Add remote_sandbox_config to our config requirements#18763
Add remote_sandbox_config to our config requirements#18763abhinav-oai merged 10 commits intomainfrom
Conversation
|
Codex Review: Something went wrong. Try again later by commenting “@codex review”. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
1 similar comment
|
Codex Review: Something went wrong. Try again later by commenting “@codex review”. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
abhinav-oai
changed the title
abhinav-oai
force-pushed
the
abhinav/remote-sandbox-config
branch
from
4d611ac to
074862b
Compare
abhinav-oai
force-pushed
the
abhinav/remote-sandbox-config
branch
from
074862b to
c90555a
Compare
|
@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: bb2015812f
ℹ️ 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".
| #[cfg(windows)] | ||
| fn local_fqdn_for_hostname(_hostname: &str) -> Option<String> { | ||
| get_computer_name(ComputerNameKind::PhysicalDnsFullyQualified) | ||
| .ok() | ||
| .and_then(|hostname| hostname.into_string().ok()) | ||
| .and_then(|hostname| normalize_fqdn_candidate(&hostname)) |
There was a problem hiding this comment.
here's the windows part @dylan-hurd-oai
we're trying to grab the FQDN hostname so we can pattern match again the hostname pattern in the requirements
|
|
||
| if let Some(requirements) = cloud_requirements.get().await.map_err(io::Error::other)? { | ||
| if let Some(mut requirements) = cloud_requirements.get().await.map_err(io::Error::other)? { | ||
| requirements.apply_remote_sandbox_config(requirements_hostname); |
There was a problem hiding this comment.
Is it possible to move this directly inside CloudRequirementsLoader so people won't forget to apply it?
| #[cfg(windows)] | ||
| use winapi_util::sysinfo::get_computer_name; | ||
|
|
||
| pub fn requirements_hostname() -> Option<String> { |
There was a problem hiding this comment.
Maybe make this field more general, just host name?
3 participants
Why
Customers need finer-grained control over allowed sandbox modes based on the host Codex is running on. For example, they may want stricter sandbox limits on devboxes while keeping a different default elsewhere.
Our current cloud requirements can target user/account groups, but they cannot vary sandbox requirements by host. That makes remote development environments awkward because the same top-level
allowed_sandbox_modeshas to apply everywhere.What
Adds a new
remote_sandbox_configsection torequirements.toml:During requirements resolution, Codex resolves the local host name once, preferring the machine FQDN when available and falling back to the cleaned kernel hostname. This host classification is best effort rather than authenticated device proof.
Each requirements source applies its first matching
remote_sandbox_configentry before it is merged with other sources. The shared merge helper keeps thatapply_remote_sandbox_configstep paired with requirements merging so new requirements sources do not have to remember the extra call.That preserves source precedence: a lower-precedence requirements file with a matching
remote_sandbox_configcannot override a higher-precedence source that already setallowed_sandbox_modes.This also wires the hostname-aware resolution through app-server, CLI/TUI config loading, config API reads, and config layer metadata so they all evaluate remote sandbox requirements consistently.
Verification
cargo test -p codex-config remote_sandbox_configcargo test -p codex-config host_namecargo test -p codex-core load_config_layers_applies_matching_remote_sandbox_configcargo test -p codex-core system_remote_sandbox_config_keeps_cloud_sandbox_modescargo test -p codex-configcargo test -p codex-coreunit tests passed;tests/all.rsintegration matrix was intentionally stopped after the relevant focused tests passedjust fix -p codex-configjust fix -p codex-corecargo check -p codex-app-server