Conversation
![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: 8600629232
ℹ️ 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".
| for key in [CODEX_THREAD_ID_ENV_VAR, CODEX_PERMISSION_PROFILE_ENV_VAR] { | ||
| if let Some(value) = env.get(key) { | ||
| override_env.insert(key.to_string(), value.clone()); | ||
| } | ||
| } |
There was a problem hiding this comment.
When the current turn has no active named profile, inject_permission_profile_env removes CODEX_PERMISSION_PROFILE from env, but this snapshot restore path only protects the key when it is present. If a shell snapshot exports a stale profile from a parent/previous shell, sourcing the snapshot reintroduces it and no generated restore block unsets it, so shell tools see a profile that is not active; include this key in the snapshot override/unset logic even when absent from env.
Useful? React with 👍 / 👎.
| if let Some(active_permission_profile) = active_permission_profile { | ||
| env.insert( | ||
| CODEX_PERMISSION_PROFILE_ENV_VAR.to_string(), | ||
| active_permission_profile.id.clone(), |
There was a problem hiding this comment.
On Windows, environment variable names are case-insensitive, so if the inherited or policy environment already contains codex_permission_profile with a stale value, this exact-key insert leaves both spellings in the map and the spawned process can observe the stale value depending on merge/iteration order. Remove existing keys equal ignoring ASCII case before inserting the active profile value.
AGENTS.md reference: AGENTS.md:L317-L319
Useful? React with 👍 / 👎.
| let active_permission_profile = context.turn.config.permissions.active_permission_profile(); | ||
| inject_permission_profile_env(&mut env, active_permission_profile.as_ref()); |
There was a problem hiding this comment.
For remote exec-server launches, the env sent over the wire is later reduced to values that differ from local_policy_env; because this injection happens after local_policy_env is captured, an active profile is dropped whenever the local policy environment already had the same value (for example, Codex was launched from a shell with CODEX_PERMISSION_PROFILE=dev). The remote exec-server then rebuilds its own environment from its host policy and may omit the active profile entirely, so remote shell tools do not see the selected profile. Ensure this runtime-injected key is always included in the remote overlay or excluded from the baseline comparison.
AGENTS.md reference: AGENTS.md:L317-L322
Useful? React with 👍 / 👎.
| let active_permission_profile = context.turn.config.permissions.active_permission_profile(); | ||
| inject_permission_profile_env(&mut env, active_permission_profile.as_ref()); |
There was a problem hiding this comment.
When there is no active named profile, this removes CODEX_PERMISSION_PROFILE only from the locally constructed env, but remote exec-server launches rebuild the child env from env_policy on the remote host and then only overlay provided values. If the remote exec-server process was started with a stale CODEX_PERMISSION_PROFILE, the policy still inherits it and there is no overlay entry to unset it, so remote shell tools can see a profile that is not active. Exclude this key from the remote policy or send an explicit clearing mechanism for the inactive-profile case.
AGENTS.md reference: AGENTS.md:L317-L322
Useful? React with 👍 / 👎.
1 participant
Why
Shell tool owners may need to launch nested commands under the same named permission profile, including through
codex sandbox -P. Until now, child processes could observe sandbox and network metadata but could not identify the active named permission profile.The new value is intentionally informational: child processes can overwrite environment variables, so consumers must decide whether their process-tree context is trusted before using it.
What changed
CODEX_PERMISSION_PROFILEafter shell environment policy evaluation so the active profile wins over inherited or configured stale values.shell_commandand unifiedexec_command, including local, zsh-fork, and remote exec-server paths.Testing
shell_commandreceives the selected active profile.printenv CODEX_PERMISSION_PROFILE.