Skip to content

test(sdk): isolate fixtures from ambient umask - #775

Merged
mldangelo-oai merged 3 commits into
openai:mainfrom
Hughhhhcoder:codex/codex-security-umask-fixtures
Sep 1, 2026
Merged

test(sdk): isolate fixtures from ambient umask#775
mldangelo-oai merged 3 commits into
openai:mainfrom
Hughhhhcoder:codex/codex-security-umask-fixtures

Conversation

@Hughhhhcoder

@Hughhhhcoder Hughhhhcoder commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes #207.

Several POSIX tests treat directories they create as trusted/private fixtures but inherited the invoking account umask. Under 0002, those directories became 0775 and were correctly rejected by the SDK secure-ancestry checks. This change gives only those test-owned fixtures an explicit private mode without relaxing production validation or changing process-global state.

Changes

  • create the affected test-owned repository, credential, state, campaign, publication, and alias-parent fixtures with mode 0700
  • normalize existing test-owned credential ancestors where recursive setup may already have created them
  • keep explicit unsafe/shared-parent fixtures unchanged
  • leave every product-created scan directory and ancestor untouched

Testing

  • low-umask regression set spanning the 9 changed files — 23 pass, 0 fail
  • pnpm run types — pass
  • pnpm run format — pass
  • git diff --check — pass

The current tree also exposes separate low-umask failures in post-start scan flows. This patch deliberately does not chmod or otherwise repair those product-created ancestors, so those failures remain visible for separate diagnosis.

Risk and rollout

Test-only change. It does not install a preload, mutate the runner umask, alter product-created paths, or change production behavior. POSIX mode normalization is skipped where appropriate on Windows.

Public disclosure review

  • No customer, partner, prospect, or user identities, data, or identifying details are included.
  • No credentials, personal data, private source, scan findings, or nonpublic links or tickets are included.
  • I reviewed the branch name, title, description, commits, changes, comments, logs, screenshots, attachments, and links for public disclosure.
@Hughhhhcoder

Copy link
Copy Markdown
Contributor Author

@codex review

@github-actions github-actions Bot added the skip-release-notes Omit internal changes from generated release notes label Sep 1, 2026
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 1, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-01T04:54:41.750941Z 27dd772 Manual request
🔒 Security Review Completed 2026-09-01T04:54:30.890842Z 27dd772 New commits
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 507f23a56c

ℹ️ 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".

Comment thread sdk/typescript/tests-ts/setup.ts Outdated
@Hughhhhcoder

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2b9268da8d

ℹ️ 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".

Comment thread sdk/typescript/tests-ts/compact-diff-scan.test.ts Outdated
@Hughhhhcoder

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Another round soon, please!

Reviewed commit: 27dd772517

ℹ️ 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".

@mldangelo-oai mldangelo-oai left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for tracking this down and iterating on the review feedback! The final fixture-only fix keeps the production permission checks intact. I independently verified 23 focused tests across the nine changed files under both 0002 and 0022, including the unsafe-parent cases. Looks good to merge once CI passes.

@mldangelo-oai
mldangelo-oai merged commit ef8c8d6 into openai:main Sep 1, 2026
41 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

skip-release-notes Omit internal changes from generated release notes

2 participants