feat: add trusted process environment values - #4408
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 96fc41a25d
ℹ️ 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".
96fc41a to
0a1d754
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0a1d75444c
ℹ️ 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".
0a1d754 to
bddac27
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bddac27241
ℹ️ 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".
bddac27 to
f17057a
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f17057ad66
ℹ️ 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".
f17057a to
c6d4b2b
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c6d4b2be29
ℹ️ 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".
Co-authored-by: maplexu <maplexu2010@gmail.com>
c6d4b2b to
96a0e64
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 96a0e645b3
ℹ️ 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".
| if manifest is not None: | ||
| _validate_docker_path_grants(manifest) | ||
| if manifest is not None and environment is None: | ||
| environment = await manifest.resolve_environment() |
There was a problem hiding this comment.
When ordinary create() successfully starts a container with a ProcessEnvValue and a later step such as resolve_snapshot() or session-state construction fails, _cleanup_failed_create_resources() suppresses any failure from container.remove(force=True). The caller receives only the original exception and no session or container ID, so a running orphan can retain the protected host credential indefinitely. Retry cleanup or surface a cleanup error that retains enough resource identity for deletion.
AGENTS.md reference: AGENTS.md:L169-L169
Useful? React with 👍 / 👎.
| ) | ||
|
|
||
| async def start_replacement() -> None: | ||
| resolved_environment = await state.manifest.resolve_environment() |
There was a problem hiding this comment.
When a resumed Docker state has a NoopSnapshot, this calls every custom EnvValue resolver before the immediately following branch rejects the resume. A secret-store or network resolver can therefore perform unnecessary side effects, add latency, or fail with a different error even though this snapshot shape is unconditionally unsupported. Check NoopSnapshot before resolving the environment.
AGENTS.md reference: AGENTS.md:L98-L98
Useful? React with 👍 / 👎.
| "Process environment access can only be granted for references in this " | ||
| f"manifest; no destination uses: {formatted}" | ||
| ) | ||
| trusted = self.model_copy(deep=True) |
There was a problem hiding this comment.
When a manifest combines ProcessEnvValue with a custom EnvValue backed by a non-copyable client or synchronization primitive, granting process access fails here with that object's __deepcopy__ error before the manifest can be used. EnvValue is an extension point and resolving it does not require it to be copyable, so granting authority for one process binding should copy only the manifest state needed to isolate the grant rather than deep-copying unrelated resolver internals.
Useful? React with 👍 / 👎.
This pull request adds a serializable
ProcessEnvValuereference for sandbox manifests so trusted application code can map the current SDK process environment into sandbox variables without persisting secret values or access authority. The common same-name case defaults to theEnvironmentmapping key, whilename=...supports explicit source-to-destination renaming.Runtime-only exact bindings are granted through
Manifest.with_process_environment_access(...), revoked by serialization and pickle round trips, and rebound during resume only from the current trusted manifest. Environment resolution fails before provider side effects, uses one stable snapshot per backend operation, and sanitizes downstream errors across the built-in sandbox backends.This supersedes #4399.