test: create private fixture directories with an explicit mode - #215
Open
rohanpoudel2 wants to merge 3 commits into
Open
test: create private fixture directories with an explicit mode#215rohanpoudel2 wants to merge 3 commits into
rohanpoudel2 wants to merge 3 commits into
Conversation
The POSIX test suite silently depended on the ambient process umask being at least as restrictive as 0022. Several fixtures that stand in for private or trusted scan ancestors were created with a bare `mkdir`, so under the common collaborative umask 0002 they became mode 0775 and the SDK's group-writable ancestor checks correctly rejected them. Six tests failed for a reason unrelated to what they were asserting. Create those fixtures with an explicit `mode: 0o700`, matching the neighbouring `mkdir` calls in the same tests, and restore the private mode on the linked Git worktree that `git worktree add` creates under the ambient umask. The production ancestry check is deliberately unchanged. Verified with the full suite under umask 0002, 0022, 0007 and 0077. Fixes openai#207
github-actions
Bot
added
the
skip-release-notes
Collaborator
|
Codex Review: Didn't find any major issues. Keep it up! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
Security review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #207
Problem
The POSIX test suite silently depends on the ambient process umask being at least as restrictive as
0022. Several fixtures that stand in for private or trusted scan ancestors are created with a baremkdir, so under the common collaborative umask0002they become mode0775. The SDK's group-writable ancestor check then correctly rejects them, and six tests fail for a reason unrelated to what they assert:Reproduced on macOS 15 (Darwin 25.5.0), Node 24.11.1, Bun 1.3.14, at
a8fc009:The six failures match the ones reported in #207 exactly:
runtime directories and plugin Python boundary > validates explicit output directories and creates private temporary pathscanonical scan contract > accepts a scan directory beneath a symlinked parentCLI authentication > keeps delegated credentials in the configured Codex homeCodexSecurity orchestration > preflights local inputs without initializing runtime or credentialsCodexSecurity orchestration > rejects output inside normal and linked Git worktrees before runtime initializationCodexSecurity orchestration > keeps a private preflight snapshot isolated from persistent credentialsChange
Test-only. The production ancestry check is deliberately left alone, per the reporter's suggestion.
Each affected fixture is now created with an explicit
mode: 0o700, matching the neighbouringmkdircalls that already do this in the same tests (for examplecaller-ownedandscanDira few lines away).mkdirapplies the mode to every directory it creates, including withrecursive: true, and0o700carries no group- or world-writable bits for a umask to strip, so the fixtures are now umask-independent.One fixture needed a different treatment: the linked worktree in
rejects output inside normal and linked Git worktreesis created bygit worktree add, which honours the ambient umask and takes no mode argument, so the test restores the private mode with an explicitchmodafterwards.Verification
Full suite, same commit, four umasks:
0002002200070077pnpm run typesandpnpm run formatare both clean.Note on
restricted-rootWhile fixing this I found a seventh fixture with the same defect that #207 does not list:
restricted-rootinruntime.test.ts. It only surfaces once the other fixtures in that test are fixed, because an earlier assertion in the same test aborted first. It is included here.