Skip to content

fix(test): keep a developer's live OpenCodex credentials out of the test sandbox - #5830

Merged
lidge-jun merged 2 commits into
devfrom
codex/test-sandbox-drops-live-tokens
Sep 25, 2026
Merged

lidge-jun merged 2 commits into
devfrom
codex/test-sandbox-drops-live-tokens

Conversation

@lidge-jun

Copy link
Copy Markdown
Owner

Summary

On a Windows machine with OpenCodex installed, the test suite failed tests that pass on the hosted runners. The install stores its data-plane token as a user environment variable (OPENCODEX_API_AUTH_TOKEN), so every shell on that machine carries it. The test sandbox rewrote HOME and the config directories but passed the rest of the environment through, so tests that assume no credential read the live token instead: installLaunchd baked it into the plist, and the byte-identical no-op path turned into a bootstrap.

  • createIsolatedTestEnvironment (the bun run test path) and tests/preload.ts (bare bun test) now drop OPENCODEX_API_AUTH_TOKEN, OPENCODEX_ADMIN_AUTH_TOKEN, and OCX_API_TOKEN_FILE from the sandbox. The list is exported once as LIVE_INSTALL_CREDENTIAL_ENV. Other inherited variables are unchanged.
  • tests/clients/link-compensation.test.ts and tests/clients/client-link-tunnel.test.ts asserted POSIX mode 0600 on Windows, where the files are protected by NTFS ACL hardening and stat reports 0666. They now skip that check on Windows, the same way link-store and client-link-state already do.

Verification

The CI Windows leg (9 shards, BUN_TEST_BATCH_SIZE=6, 480 s batches, Bun 1.4.0) was run sequentially on a Windows 11 machine with OpenCodex installed and OPENCODEX_API_AUTH_TOKEN set in the user environment:

  • On dev 04ba23d, 13 unique failures appeared:
    • 10 came from the inherited token: 5 in launchd-repair, 2 in cursor-integration-status, 1 in loopback-companion-client-targets, and 2 in claude-cli.
    • 2 were the mode-bit checks.
    • 1 was Codex Log Guard inspection > repeat inspection is memoized and invalidated by a write. It passes alone and is the mtime-granularity case AGENTS.md already lists; it is not changed here.
  • On this branch, every file that failed passes on that machine with the token still set (118 + 30 + 99 tests). A clean rerun of the leg passed shards 1–4 in full (12,739 tests, 0 failures) before it was stopped. Shards 5–9 were not rerun.
  • bun test tests/ci-workflows/test-runner.test.ts tests/update/update-stop-first.test.ts tests/ci-workflows/test-home-guard.test.ts: 104 pass, including the new drops the developer's live install credentials case. bun x tsc --noEmit passes.
  • Squash-merged without waiting for PR CI at the maintainer's request. The hosted runners never set these variables, so their behavior is unchanged.

Checklist

  • Scope stays focused and avoids unrelated cleanup.
  • Docs or release notes were updated when needed. (none needed; test harness only)
  • Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults. The change removes live credentials from the test environment and logs no values.

… test sandbox

A Windows install stores its data-plane token as a user environment variable, so the sandbox inherited OPENCODEX_API_AUTH_TOKEN from every shell on that machine. installLaunchd then baked the live token into the plist, the byte-identical check failed, and five launchd repair tests failed only on a developer machine. createIsolatedTestEnvironment and the preload now drop OPENCODEX_API_AUTH_TOKEN, OPENCODEX_ADMIN_AUTH_TOKEN, and OCX_API_TOKEN_FILE.
link-compensation and client-link-tunnel asserted 0600 on files Windows protects with NTFS ACLs, where stat reports 0666. The other link tests already guard this the same way.
@lidge-jun
lidge-jun requested a review from Ingwannu as a code owner September 25, 2026 07:06
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 25, 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-25T07:09:37.783473Z 9628819 PR opened
ℹ️ 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.

@lidge-jun
lidge-jun merged commit 47065fd into dev Sep 25, 2026
8 checks passed
@lidge-jun
lidge-jun deleted the codex/test-sandbox-drops-live-tokens branch September 25, 2026 07:06
@github-actions

Copy link
Copy Markdown
Contributor

✅ Deterministic PR hygiene checks passed.

@github-actions github-actions Bot added the bug Something isn't working label Sep 25, 2026

@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: 9628819553

ℹ️ 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 scripts/test.ts
}
writeTestTempOwner(root, baseEnv[TEST_RUN_ID_ENV]);
const inherited = { ...baseEnv };
for (const name of LIVE_INSTALL_CREDENTIAL_ENV) delete inherited[name];

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Strip credential keys case-insensitively on Windows

When a Windows environment contains one of these variables with noncanonical casing, such as opencodex_api_auth_token, spreading baseEnv creates an ordinary case-sensitive object and these exact-name deletions leave the credential in isolated.env; runTestLane then passes it to Bun.spawn, where Windows treats it as the same variable and the sandbox can again consume the developer's live token. Filter the copied keys by an uppercased name and cover a mixed-case input in the regression test.

AGENTS.md reference: scripts/AGENTS.md:L14-L17

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant