Skip to content

fix(scan): strip a profile login shell override from the scan runtime - #865

Open
kevin9327 wants to merge 1 commit into
openai:mainfrom
kevin9327:fix/scan-profile-login-shell
Open

kevin9327 wants to merge 1 commit into
openai:mainfrom
kevin9327:fix/scan-profile-login-shell

Conversation

@kevin9327

Copy link
Copy Markdown
Contributor

Summary

scanRuntimeCodexConfig builds the Codex config a scan runs under. It pins a set of execution keys and, because a selected profile takes precedence over the top-level table, it also deletes those same keys from every configured profile so a profile cannot put them back:

delete profile["approval_policy"];
delete profile["approvals_reviewer"];
delete profile["default_permissions"];
delete profile["permissions"];
delete profile["sandbox_mode"];

allow_login_shell is pinned to false in the returned config next to approval_policy and default_permissions, but it is the one pinned key with no matching per-profile delete. A configured profiles.<name>.allow_login_shell = true therefore survives hardening, and the profile layer is what the scan runtime ends up applying — resolveCodexProfile in config.ts deep-merges the selected profile over the top-level config, and scanApprovalPolicy already reads selectedScanProfile(config)?.["approval_policy"] for exactly that reason.

So a scan started with a profile that sets allow_login_shell = true runs with the login shell the scan runtime is supposed to disable, while the emitted top-level config still reads allow_login_shell = false.

The override reaches this function through ordinary public CLI surface — codex-security scan . --codex 'profile="p"' --codex 'profiles.p.allow_login_shell=true' is accepted; validateOverrides only owns plugins, marketplaces and features.plugins.

Changes

  • sdk/typescript/src/api.ts: add delete profile["allow_login_shell"]; to the per-profile strip loop in scanRuntimeCodexConfig, alongside the five keys already removed there.
  • sdk/typescript/tests-ts/api-preflight-config.test.ts: regression test covering the selected profile and an unselected one.

Testing

Observed through the exported SDK function, before and after the change, with the same input.

Before, on unmodified main:

$ node -e "import('./dist/api.js').then(({scanRuntimeCodexConfig}) => console.log(JSON.stringify(
    scanRuntimeCodexConfig({ profile: 'p', profiles: { p: {
      allow_login_shell: true, sandbox_mode: 'danger-full-access', approval_policy: 'never' } } })
  , null, 2)))"
{
  "profile": "p",
  "profiles": { "p": { "allow_login_shell": true } },
  "approval_policy": "never",
  "approvals_reviewer": "auto_review",
  "allow_login_shell": false,
  ...
}

sandbox_mode and approval_policy were stripped from the profile; allow_login_shell: true was not, and it contradicts the pinned top-level false.

New test against unmodified main:

$ bun test --timeout 30000 ./tests-ts/api-preflight-config.test.ts
error: expect(received).toEqual(expected)
@@ -1,5 +1,7 @@
  {
-   "other": {},
+   "other": {
+     "allow_login_shell": true,
+   },
    "selected": {
+     "allow_login_shell": true,
      "model": "profile-model",
(fail) CodexSecurity preflight configuration > removes a profile login shell override from every configured profile
 16 pass
 1 fail

After the change, same input:

{ "allow_login_shell": false, "profiles": { "p": {} } }

Checks run:

  • bun test --timeout 30000 ./tests-ts/api-preflight-config.test.ts — 17 pass, 0 fail
  • bun test --timeout 30000 ./tests-ts/config.test.ts — 22 pass, 1 skip, 3 fail. The 3 failures are pre-existing on this Windows machine and unrelated to this change: all three abort with Windows credential-home ancestor allows another identity to replace the directory, and they fail identically with the change stashed (22 pass / 3 fail on unmodified main).
  • tsc --noEmit (lint) — clean
  • tsc -p tsconfig.ci.json (build:ci) — clean
  • prettier --check on both changed files — All matched files use Prettier code style!

Risk and rollout

No public CLI surface change: no new command, flag, environment variable, or default. The pinned allow_login_shell: false in the returned config is unchanged; the only difference is that a profile can no longer contradict it, which matches how the other five hardened keys already behave.

Compatibility: a user who set profiles.<name>.allow_login_shell = true and relied on it taking effect during a scan will see that setting ignored during scans. It is already ignored for policy generation, which resolves the profile through resolveCodexProfile before hardening, so this brings scans in line with the existing behaviour of the neighbouring path. Profiles are otherwise untouched, and configs with no profiles table are unaffected.

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.

`scanRuntimeCodexConfig` pins `allow_login_shell: false` for the scan
runtime and deletes the sibling execution and permission keys from every
configured profile so a selected profile cannot restore them. It never
deleted `allow_login_shell`, so `profiles.<name>.allow_login_shell = true`
survived hardening and, by the profile precedence this SDK applies in
`resolveCodexProfile`, won over the pinned top-level value.

Delete it alongside the other five keys.
@chatgpt-codex-connector

Copy link
Copy Markdown

Note

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@github-actions github-actions Bot added the bug Something isn't working label Sep 10, 2026
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