feat: add trusted process environment values - #4408
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: 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".
seratch
changed the title
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".
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".
96a0e64 to
b65136f
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b65136fb23
ℹ️ 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".
b65136f to
5161690
Compare
5161690 to
8f69f83
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8f69f83582
ℹ️ 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".
8f69f83 to
f531a61
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f531a61f7d
ℹ️ 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".
f531a61 to
e9e3899
Compare
e9e3899 to
7d6cf42
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7d6cf42bcb
ℹ️ 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".
7d6cf42 to
b77639e
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b77639e248
ℹ️ 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".
b77639e to
bd930d8
Compare
bd930d8 to
3e2596f
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3e2596f99e
ℹ️ 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".
3e2596f to
cbf2381
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cbf2381d9f
ℹ️ 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".
cbf2381 to
71e7794
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 71e7794f83
ℹ️ 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>
71e7794 to
da6f4a3
Compare
ErenAta16
left a comment
ErenAta16
left a comment
There was a problem hiding this comment.
The description says Daytona carries protected values, and the code says it does not. The code and its tests agree with each other, so I think the description is the thing that needs the edit, but it is worth confirming which way round it should be.
From the PR body:
Trusted runtime configuration is instead passed to
DockerSandboxClientorDaytonaSandboxClient
Docker and Daytona pass protected values only through provider-native creation environment channels.
Three things in the diff point the other way.
DaytonaSandboxClient rejects it on both entry points, with a message naming Docker as the alternative:
# create
manifest._reject_process_environment_values(
backend_id="daytona",
supported_alternative=(
"use DockerSandboxClient for protected process environment transport"
),
)
# resume
state.manifest._reject_process_environment_values(
backend_id="daytona",
...
)That is the same shape as the backends the description calls unsupported. E2BSandboxClient gets supported_alternative="use DockerSandboxClient" in the same position.
The constructor cannot take the trusted configuration. DockerSandboxClient.__init__ has
allowed_process_environment_keys: Iterable[str] = (),
process_environment_bindings: Mapping[str, str] | None = None,and DaytonaSandboxClient.__init__ has neither, so following the description literally and writing DaytonaSandboxClient(allowed_process_environment_keys={"OPENAI_API_KEY"}) is a TypeError on an unexpected keyword argument rather than a runtime rejection.
The tests pin the rejection, in tests/extensions/sandbox/test_daytona.py:
test_create_rejects_process_environment_before_provider_create
test_resume_rejects_process_environment_before_provider_lookup
pytest.raises(ValueError, match="daytona does not support ProcessEnvValue")
So the implementation is self-consistent: Docker is the only backend that carries these, and Daytona is one of the eight that refuse them.
Worth fixing before merge rather than after, because a PR body of this size usually becomes the changelog entry, and this is the paragraph a reader goes to when deciding which client to configure. If Daytona support is intended and simply not in this PR, saying "Docker today, Daytona to follow" would read the same way without sending anyone at a constructor that will not accept the argument.
I did not review the whole 3k-line diff, only the process-environment boundary, and everything else I looked at on that boundary lined up.
2 participants
This pull request adds
ProcessEnvValueas a late-bound sandbox environment reference that can survive manifest serialization without persisting the concrete secret value.A manifest only declares which sandbox environment key needs a process-environment reference. It does not declare or serialize permission to read worker process environment variables. Trusted runtime configuration is instead passed to
DockerSandboxClientorDaytonaSandboxClient:allowed_process_environment_keys={"OPENAI_API_KEY"}grants the common same-name binding.process_environment_bindings={"OPENAI_API_KEY": "PROD_OPENAI_API_KEY"}grants an explicit destination-to-source rename.The clients normalize these settings into exact destination/source bindings and apply them during sandbox create or resume. Serialized manifests therefore cannot select arbitrary
os.environkeys after being loaded by another worker.Docker and Daytona pass protected values only through provider-native creation environment channels. Unsupported backends and injected sessions reject
ProcessEnvValuebefore provider side effects. Resume with current runtime bindings recreates the backend when needed, preserves resumable workspace state, retires replaced resources, and keeps retryable cleanup identities without exposing protected values through command text or errors.This supersedes #4399.