fix(sandbox): reject unsafe mount credential configurations - #4255
fix(sandbox): reject unsafe mount credential configurations#4255seratch wants to merge 1 commit into
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: 87a4bf6788
ℹ️ 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".
87a4bf6 to
670ec3c
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 670ec3cc5d
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fdd7fa31d4
ℹ️ 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".
fdd7fa3 to
b0df561
Compare
seratch
changed the title
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b0df5611f9
ℹ️ 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".
b0df561 to
8db5444
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8db5444fa2
ℹ️ 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".
8db5444 to
a4a9b5d
Compare
a4a9b5d to
74f7034
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a4a9b5d25d
ℹ️ 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".
| def _configured_mount_authority_fields(mount: Mount) -> tuple[str, ...]: | ||
| mount_type = _canonical_mount_type(type(mount)) or mount.type | ||
| fields = [ | ||
| name | ||
| for name in _AUTHORITY_FIELDS_BY_MOUNT_TYPE.get(mount_type, ()) | ||
| if getattr(mount, name, None) is not None |
There was a problem hiding this comment.
When an in-container rclone mount places credentials in another config-rendered string, this classifier accepts it as credentialless; for example, S3Mount(s3_provider="AWS\naccess_key_id = ...\nsecret_access_key = ...") passes validation, and _rclone_required_lines() inserts those lines verbatim into the sandbox config while serialization retains the secret. Other endpoint/account fields are rendered similarly, so reject CR/LF or otherwise safely encode every value written into rclone configuration before allowing the mount.
AGENTS.md reference: AGENTS.md:L137-L137
Useful? React with 👍 / 👎.
| if "sandbox" in state_json: | ||
| if not isinstance(state_json["sandbox"], Mapping): | ||
| state_json["sandbox"] = {} | ||
| _raise_invalid_run_state_sandbox_envelope() | ||
| sanitized_sandbox, _redacted = sanitize_run_state_sandbox_mount_authority( |
There was a problem hiding this comment.
When this SDK receives a snapshot from a future unsupported schema that changes the sandbox envelope, this block interprets the future payload before _build_run_state_from_json() checks $schemaVersion. A new non-mapping shape therefore raises an invalid envelope error and clears the caller's payload instead of producing the documented unsupported-version UserError; validate the schema version first and sanitize only layouts this reader supports.
AGENTS.md reference: AGENTS.md:L136-L136
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 74f703410f
ℹ️ 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".
| opaque_fields = _OPAQUE_STRATEGY_AUTHORITY_FIELDS.get( | ||
| strategy_type, | ||
| _ALL_OPAQUE_STRATEGY_AUTHORITY_FIELDS, | ||
| ) |
There was a problem hiding this comment.
When a custom MountStrategyBase stores authority in any field outside the hard-coded set, such as api_token, this sanitizer leaves it untouched; validation likewise sees no configured authority, so both session-state and RunState serialization retain the secret verbatim. This is distinct from inherited built-in mount fields: a registered custom strategy's own credential fields still bypass the boundary. Reject unknown strategy payloads or redact their entire opaque configuration rather than enumerating two known field names.
AGENTS.md reference: AGENTS.md:L137-L137
Useful? React with 👍 / 👎.
1 participant
This pull request fixes cloud mount credentials crossing into model-controlled sandbox processes or durable sandbox state.
Credential-bearing in-container and unknown-boundary mount configurations now fail before provider, filesystem, or helper side effects. Credentialless helpers, Docker volumes, provider-native mounts, and Vercel's explicit trusted create-only opt-in remain supported.
Sandbox session and RunState serialization remove mount authority and require an exact trusted configuration match before rebinding external credentials. Provider failures, cancellation paths, snapshots, temporary credential files, and partial Docker startup cleanup are also handled without exposing credential values.
Document changes will be done separately.