Handle invalid security config safely across managed, project, and session layers#15064
Handle invalid security config safely across managed, project, and session layers#15064evawong-oai wants to merge 8 commits intomainfrom
Conversation
|
All contributors have signed the CLA ✍️ ✅ |
|
@codex review |
|
@codex review |
![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: a0f95b69b8
ℹ️ 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".
| fn is_invalid_security_managed_config_entry(dropped_entry: &str) -> bool { | ||
| matches!( | ||
| managed_entry_top_level_key(dropped_entry), | ||
| "approval_policy" | "sandbox_mode" | ||
| ) |
There was a problem hiding this comment.
is_invalid_security_managed_config_entry only checks the top-level path segment. If sanitization drops profiles.<name>.approval_policy or profiles.<name>.sandbox_mode, the top-level key is profiles, so the code logs and continues instead of failing closed. That lets malformed profile-level managed security controls slip through, which undermines the intended fail-closed behavior.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in 064bcae60. Managed config security detection now checks the leaf field name instead of only the top level path, so dropped entries like profiles.team.approval_policy and profiles.team.sandbox_mode fail closed as intended. I also extended the managed preferences regression to cover those profile scoped cases, and CARGO_TARGET_DIR=/tmp/codex-target-eva cargo test -p codex-core --lib managed_preferences_ passes.
|
@codex address that feedback |
|
I have read the CLA Document and I hereby sign the CLA |
|
recheck |
|
Codex couldn't complete this request. Try again later. |
evawong-oai
force-pushed
the
codex/eva-pr14818
branch
from
064bcae to
0611acc
Compare
evawong-oai
changed the title
evawong-oai
changed the base branch from
greg/mdm-managed-config-sanitize
to
main
2 participants
Summary
This follow up aligns config loading across managed, project, and session sources.
For security related config, malformed inputs degrade safely by failing closed or restricting permissions, while non security config remains fail soft.
For managed config, malformed MDM security controls still fail closed while non security entries are sanitized and ignored.
For project
.codex/config.tomland session flags, malformedapproval_policyorsandbox_modenow fail closed, while malformed non security entries are ignored and valid entries continue to apply. The session path preserves partial overrides that depend on existing config, such as MCP enablement patches.This PR also keeps the macOS config service tests isolated from host managed preferences.
Validation
cargo test -p codex-utils-absolute-pathcargo test -p codex-configCARGO_TARGET_DIR=/tmp/codex-target-eva cargo test -p codex-core --lib config::service::tests::CARGO_TARGET_DIR=/tmp/codex-target-eva cargo test -p codex-core --lib config_loader::tests::Notes
Follow up to #14818.
Full
codex-corestill has unrelatedjs_replsandbox failures in this local environment.