Conversation
bolinfest
force-pushed
the
pr10964
branch
3 times, most recently
from
8f93792 to
d9a6866
Compare
![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: d9a6866735
ℹ️ 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".
| let initial_mode = match modes.first().copied() { | ||
| Some(mode) => mode, | ||
| None => { | ||
| modes.push(WebSearchModeRequirement::Disabled); | ||
| WebSearchModeRequirement::Disabled | ||
| } | ||
| }; | ||
| let mut accepted = modes.into_iter().collect::<std::collections::BTreeSet<_>>(); | ||
| accepted.insert(WebSearchModeRequirement::Disabled); | ||
| let accepted_for_error: Vec<WebSearchMode> = | ||
| accepted.iter().copied().map(Into::into).collect(); | ||
| let initial_value = Some(WebSearchMode::from(initial_mode)); | ||
| let requirement_source_for_error = requirement_source.clone(); | ||
| let constrained = Constrained::new(initial_value, move |candidate| { |
There was a problem hiding this comment.
When allowed_web_search_modes is present, this code forces an initial value to the first entry and the validator rejects None, which makes the config look explicitly set even when the user never chose a mode. That bypasses the sandbox-based default selection in resolve_web_search_mode_for_turn, so an allowlist like ["cached", "live"] will always default to cached (or whatever is listed first), even in danger-full-access where the previous default was live. If the allowlist is meant to only constrain valid modes, this is a behavioral regression; consider allowing None and letting per-turn resolution pick the preferred allowed mode instead of treating list order as a forced default.
Useful? React with 👍 / 👎.
bolinfest
force-pushed
the
pr10964
branch
2 times, most recently
from
b549a43 to
d9b9a74
Compare
Example: ```toml # This means that "live" is not allowed; "disabled" is allowed even though not listed explicitly. allowed_web_search_modes = ["cached"] ``` ### Why - Support admin/MDM/requirements.toml constraints over web-search behavior, independent of user config or sandbox defaults. - Ensure per-turn config resolution and review-mode overrides can never crash when constraints are present. - Note that `allowed_web_search_modes = ["cached"]` denies `"live"`, even if `--yolo` is used. See `resolve_web_search_mode_for_turn()`. ### What - Add `allowed_web_search_modes` to requirements parsing and app-server v2 `ConfigRequirements` (`allowedWebSearchModes`), with schema/TS fixture updates. - Introduce TOML-only `WebSearchModeRequirement` for requirements allowlists; accept `disabled|cached|live`, and treat an empty list as `[disabled]`. - Convert the allowlist to a `ConstrainedWithSource<Option<WebSearchMode>>`, always permitting `Disabled` while enforcing membership for other values. - Make `Config.web_search_mode` a constrained field, update call sites/tests, and surface constraint violations via warnings/fallback. - Extend TUI `/debug-config` output to display `allowed_web_search_modes`. ### Safety - Avoid `expect()` on constrained `web_search_mode` mutation in session per-turn config and review-thread setup; warn and keep the constrained value instead.
3 participants
This PR makes it possible to disable live web search via an enterprise config even if the user is running in
--yolomode (though cached web search will still be available). To do this, create/etc/codex/requirements.tomlas follows:Or set
requirements_toml_base64MDM as explained on https://developers.openai.com/codex/security/#locations.Why
requirements.tomlconstraints on web-search behavior, independent of user config and per-turn sandbox defaults.What
allowed_web_search_modesto requirements parsing and surface it in app-server v2ConfigRequirements(allowedWebSearchModes), with fixtures updated.WebSearchModeRequirement) and normalize semantics:disabledis always implicitly allowed (even if not listed).["disabled"].Config.web_search_modeaConstrained<WebSearchMode>and apply requirements viaConstrainedWithSource<WebSearchMode>.resolve_web_search_mode_for_turn) to:Live → Cached → DisabledwhenSandboxPolicy::DangerFullAccessis active (subject to requirements), unless the user preference is explicitlyDisabled./debug-configand app-server mapping to display normalizedallowed_web_search_modes(including implicitdisabled).SandboxPolicy::ReadOnly(sinceDangerFullAccesslegitimately preferslivewhen allowed).