fix(windows): allow archive publication under pinned directories - #226
metaphorics wants to merge 3 commits into
Conversation
|
Warning Review limit reachedNext included review available in 59 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (3)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![amazon-q-developer[bot]](https://avatars.githubusercontent.com/in/1220912?s=60&v=4){kind=small}
There was a problem hiding this comment.
The PR correctly fixes the Windows ERROR_SHARING_VIOLATION by conditionally allowing FILE_SHARE_WRITE for directories while maintaining read-only sharing for archives. The implementation maintains critical security invariants by never sharing DELETE permissions, preventing held directories from being renamed and held archives from being modified. The new test coverage validates both the fix and the security guarantees. The changes are minimal, focused, and ready for merge.
You can now have the agent implement changes and create commits directly on your pull request's source branch. Simply comment with /q followed by your request in natural language to ask the agent to make changes.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 65005c8661
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
1 participant
Change
Publish staged archives through an existing synchronous file handle using a same-parent, non-replacing native rename. Keep FILE_SHARE_READ-only pins on directories and archives. Hold the staging file exclusively until publication so neither its contents nor its parent can change during the operation.
The native call uses a validated leaf name, null RootDirectory, aligned UTF-16 storage, and DELETE access on the staging handle. It avoids reopening the write-denying parent. Native errors become Win32 I/O errors; unsafe code remains inside the Windows cache-guard module and its existing optional capability.
Add Windows-native PR CI and a regression covering a supplementary Unicode filename, denial of a directory writer before any child exists, blocked directory relocation and archive mutation, and successful operations after releasing the guards.
Review correction
The initial directory-write-sharing approach was rejected after a valid P1 reparse-point finding. a1cf753 restored read-only sharing and introduced handle-based publication; 15b2b09 keeps the staging identity exclusive. The finding is replied to and resolved.
Validation
The earlier Windows release run 35511205071 reproduced the archive sharing violation after the ownership fix passed. The corrected native job now passes. Full release-build validation has been dispatched separately and is not yet claimed successful.
Stack
Depends on #225. Ready for review after native Windows validation; not merged.
Post-Deploy Monitoring & Validation
Require Windows native boundary and the full Windows CLI release job to pass before merge. Writable handles to held directories, mutation of held archives, or relocation of held directories remain security regressions. Do not add write/delete sharing, broad retries, or skip negative tests to make a job green.