🧹 Refactor synthetic_ancestor_acl_rejects_untrusted_modification to use helper - #30
🧹 Refactor synthetic_ancestor_acl_rejects_untrusted_modification to use helper#30undivisible wants to merge 1 commit into
synthetic_ancestor_acl_rejects_untrusted_modification to use helper#30Conversation
Extract the setup logic (CurrentUser, PSID initialization, and synthetic ACL creation) into a reusable helper function `check_synthetic_ancestor_acl`. This drastically reduces boilerplate code in the main test function and improves readability without altering test coverage. Co-authored-by: undivisible <136312656+undivisible@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
🎯 What: Extracted repetitive boilerplate code from the overly long
synthetic_ancestor_acl_rejects_untrusted_modificationtest function insrc/windows_acl.rsinto a new helper functioncheck_synthetic_ancestor_acl.💡 Why: The test previously stood at over 120 lines filled with largely duplicated setup for testing various ACL masks and ACE types. Extracting this into a declarative helper allows the assertions to be read clearly step-by-step, dramatically improving readability and isolating test state cleanly.
✅ Verification: Verified by checking format, clippy rules, and running
cargo test --lib. All tests pass, ensuring that no functionality was altered in the refactor.✨ Result: Improved maintainability of
src/windows_acl.rsand better isolation between test validation stages.PR created automatically by Jules for task 16438723513372122346 started by @undivisible
Note
Low Risk
Test-only refactor and import reordering in Windows ACL code; no changes to runtime permission validation paths.
Overview
Refactors the long
synthetic_ancestor_acl_rejects_untrusted_modificationunit test by introducingcheck_synthetic_ancestor_acl, which builds a synthetic ACL with a given access mask, optionally overrides the ACE type, runsvalidate_ancestor_acl, and tears down SID allocation.The test body becomes a short sequence of mask/
final_targetcases (reject untrusted delete-child, allow create-only on ancestor, reject final-target write/append/generic write, allow read-only, reject unknown ACE type) instead of repeated inline ACL setup and manual ACE mutation.Import blocks in the same file are reordered only (including moving
windows::core::{w, PWSTR}); production ACL logic is unchanged.Reviewed by Cursor Bugbot for commit e9be6c3. Configure here.