chore: Initial project setup with configuration, documentation, and core library#1
chore: Initial project setup with configuration, documentation, and core library#1mergify[bot] merged 52 commits intomainfrom
Conversation
Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
…en privileges Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
…dates Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
… licenses Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
…dency checks Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
…remove fuzz workflow Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
…mments Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
…lines Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
…rity Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
…mands Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 56 out of 57 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| /// Enumerate all privileges on the token. | ||
| pub fn enumerate_token_privileges( | ||
| token: &OwnedHandle, | ||
| ) -> Result<Vec<PrivilegeInfo>, TokenPrivilegeError> { | ||
| // First call to get required buffer size | ||
| let mut return_length = 0_u32; | ||
|
|
||
| // SAFETY: First call with null buffer to query the required size. | ||
| // Expected to fail with ERROR_INSUFFICIENT_BUFFER, which we handle. | ||
| let size_result = unsafe { | ||
| GetTokenInformation( | ||
| token.0, | ||
| windows::Win32::Security::TokenPrivileges, | ||
| None, | ||
| 0, | ||
| &raw mut return_length, | ||
| ) | ||
| }; |
There was a problem hiding this comment.
The initial GetTokenInformation size query assumes the call fails with an insufficient-buffer error, but it doesn’t validate the actual error code. If it fails for another reason, this can lead to confusing QueryFailed(Other) or potentially proceeding with an incorrect return_length. Check the returned error and only treat it as a size query success-path when it’s the expected insufficient-buffer condition; otherwise propagate the real OS error.
Welcome to Codecov 🎉Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests. ℹ️ You can also turn on project coverage checks and project coverage reporting on Pull Request comment Thanks for integrating Codecov - We've got you covered ☂️ |
|
@Mergifyio queue |
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 56 out of 57 changed files in this pull request and generated 13 comments.
Comments suppressed due to low confidence (1)
docs/src/safety-contract.md:187
- This section says
OwnedHandleispub(crate)and “never exposed outside ffi.rs”, but insrc/ffi.rsit’s declaredpub struct OwnedHandle. Either make the typepub(crate)(preferred) or adjust this text so the safety contract matches the code.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| toolchain: | ||
| - stable | ||
| - stable minus 2 releases | ||
| - "1.85.0" # MSRV |
There was a problem hiding this comment.
The compatibility workflow hard-codes MSRV as 1.85.0, but this crate declares rust-version = "1.91" and multiple docs state MSRV 1.91. As written, the compat job will fail to build. Align the workflow’s MSRV entry with the crate’s actual MSRV (or update rust-version/docs if 1.85 is intended).
| - "1.85.0" # MSRV | |
| - "1.91" # MSRV |
| - mdformat-gfm | ||
| - mdformat-config | ||
| - mdformat-footnote | ||
| - mdformat-front-matters |
There was a problem hiding this comment.
The mdformat hook lists mdformat-front-matters, but the commonly used plugin (and the one referenced in mise.toml) is mdformat-frontmatter. This mismatch will cause pre-commit to fail installing the formatter dependencies.
| - mdformat-front-matters | |
| - mdformat-frontmatter |
| Check if the current process is running with elevated (Administrator) privileges. | ||
|
|
||
| Returns `true` if the process token has `TokenElevationTypeFull` (elevated via UAC) or `TokenElevationTypeDefault` (UAC disabled and user is an admin). | ||
|
|
There was a problem hiding this comment.
The is_elevated docs describe TokenElevationTypeFull / TokenElevationTypeDefault, but the implementation queries TOKEN_ELEVATION and returns TokenIsElevated != 0. Update this section to match the actual Win32 field being queried to avoid misleading consumers.
| ### Minimal Dependencies | ||
|
|
||
| The crate has a single runtime dependency: `thiserror` for error derivation. The `windows` crate is only pulled in on Windows targets via `[target.'cfg(windows)'.dependencies]`. | ||
|
|
||
| Dev dependencies (`proptest`, `tempfile`) are not included in production builds. | ||
|
|
There was a problem hiding this comment.
This page claims dev dependencies include tempfile, but Cargo.toml currently only lists proptest under [dev-dependencies]. Either add tempfile or remove it from this statement to keep docs accurate.
| Every push to `main` and every pull request runs through: | ||
|
|
||
| 1. **Quality gate** -- `rustfmt` check and Clippy with `-D warnings`. | ||
| 2. **Test suite** -- `cargo nextest run` on Ubuntu. | ||
| 3. **Cross-platform tests** -- Linux, macOS, and Windows runners. | ||
| 4. **Coverage** -- `cargo-llvm-cov` with Codecov upload. |
There was a problem hiding this comment.
The CI pipeline description here doesn’t match the actual workflows: ci.yml runs full tests on Windows and only stub tests on Ubuntu, and there are no macOS runners. Please update these bullets to reflect the real CI jobs to avoid confusing contributors.
| let byte_len = return_length as usize; | ||
| let u64_len = byte_len.div_ceil(size_of::<u64>()); | ||
| let mut buffer = vec![0_u64; u64_len]; |
There was a problem hiding this comment.
size_of::<u64>() is used without importing size_of (and without qualifying it), so this file won’t compile. Use std::mem::size_of::<u64>() (or add an explicit use std::mem::size_of;).
| /// RAII wrapper for Win32 `HANDLE` that calls `CloseHandle` on drop. | ||
| pub struct OwnedHandle(HANDLE); | ||
|
|
There was a problem hiding this comment.
OwnedHandle is declared pub, but the docs claim it is pub(crate) and “never exposed outside ffi.rs”. Even though ffi isn’t publicly exported today, making the type pub(crate) better enforces the intended boundary and prevents accidental exposure via future re-exports.
| #[cfg(target_os = "windows")] | ||
| match result { | ||
| Ok(_) | ||
| | Err( | ||
| token_privilege::TokenPrivilegeError::InvalidPrivilegeName { .. } | ||
| | token_privilege::TokenPrivilegeError::LookupFailed { .. }, | ||
| ) => {} | ||
| Err(other) => prop_assert!(false, "unexpected error variant: {other}"), | ||
| } |
There was a problem hiding this comment.
On Windows these proptests treat only InvalidPrivilegeName and LookupFailed as expected errors. However is_privilege_enabled can also legitimately return OpenTokenFailed and CheckFailed (and has_privilege can return OpenTokenFailed/QueryFailed). Consider accepting all documented TokenPrivilegeError variants here to avoid flaky failures in restricted/CI environments.
| - Non-Windows stub tests verify `Err(UnsupportedPlatform)` on all platforms | ||
| - `SeChangeNotifyPrivilege` is the reliable test privilege (enabled on all Windows processes by default) | ||
| - Coverage target: 85% line coverage (enforced by `just coverage-check`) | ||
| - Dev dependencies: `proptest` for property-based testing, `tempfile` |
There was a problem hiding this comment.
AGENTS.md lists tempfile as a dev-dependency, but Cargo.toml currently only includes proptest under [dev-dependencies]. Please update this line (or add the dependency) to keep the repo guidance consistent.
| - Dev dependencies: `proptest` for property-based testing, `tempfile` | |
| - Dev dependencies: `proptest` for property-based testing |
| format-files +FILES: | ||
| @{{ mise_exec }} prettier --write --config .prettierrc.json {{ FILES }} |
There was a problem hiding this comment.
The format-files recipe injects the +FILES parameter directly into a bash command (@{{ mise_exec }} prettier --write --config .prettierrc.json {{ FILES }}) without any quoting or escaping. If this recipe is invoked from pre-commit hooks (as indicated by the comment) with filenames controlled by an attacker (e.g., via a maliciously named file in a PR), shell metacharacters in the filename can break out of the intended prettier command and execute arbitrary commands on the developer’s machine. To fix this, ensure that file arguments are passed as properly escaped/quoted positional parameters (e.g., via a helper script or just’s shell-escaping features) so that filenames cannot be interpreted by the shell as additional commands or arguments.
46a27a6 to
44f968a
Compare
…atibility Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
I, UncleSp1d3r <unclesp1d3r@evilbitlabs.io>, hereby add my Signed-off-by to this commit: f3e1822 I, UncleSp1d3r <unclesp1d3r@evilbitlabs.io>, hereby add my Signed-off-by to this commit: d72ad7e I, UncleSp1d3r <unclesp1d3r@evilbitlabs.io>, hereby add my Signed-off-by to this commit: 1b74c50 I, UncleSp1d3r <unclesp1d3r@evilbitlabs.io>, hereby add my Signed-off-by to this commit: 0751432 I, UncleSp1d3r <unclesp1d3r@evilbitlabs.io>, hereby add my Signed-off-by to this commit: 44f968a Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io> Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
|
@Mergifyio queue |
Merge Queue StatusRule:
This pull request spent 6 seconds in the queue, with no time running CI. Required conditions to merge
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 55 out of 56 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ## RAII Handle Wrapper | ||
|
|
||
| `OwnedHandle` is a `pub(crate)` newtype around `HANDLE` that implements `Drop` to call `CloseHandle`. This ensures handles are closed on all code paths: | ||
|
|
||
| - Normal return | ||
| - Early `?` propagation | ||
| - Panic unwinding | ||
|
|
||
| The handle is never `Copy` or `Clone`, preventing aliased access. It is never exposed outside `ffi.rs`. |
There was a problem hiding this comment.
The safety contract states OwnedHandle is pub(crate) and never exposed outside ffi.rs, but in code it’s declared pub struct OwnedHandle(...). Even though the ffi module itself is private, making OwnedHandle explicitly pub(crate) (or updating the doc) would better enforce/communicate the intended encapsulation.
| let byte_len = return_length as usize; | ||
| let u64_len = byte_len.div_ceil(size_of::<u64>()); | ||
| let mut buffer = vec![0_u64; u64_len]; | ||
|
|
There was a problem hiding this comment.
size_of::<u64>() is used here but size_of isn’t in scope, so this won’t compile. Use std::mem::size_of::<u64>() (or import std::mem::size_of) when computing u64_len.
This pull request introduces important project configuration and legal files. The main changes are the addition of detailed licensing information and a comprehensive
.coderabbitai.yamlconfiguration file to guide code review, labeling, and documentation practices.Project configuration enhancements:
.coderabbitai.yamlwith extensive instructions for code review, labeling, path filters, code generation, and pre-merge checks, tailored for safety-focused, idiomatic Rust development.Legal and licensing additions:
LICENSE-APACHEcontaining the full Apache License 2.0 text.LICENSE-MITcontaining the full MIT License text.