refactor(env): stop mutating os.environ in resolve_env
ResolvedEnv.forwarded now carries name->value pairs instead of names whose values had been side-loaded into os.environ. The Docker backend collects the dict (plus the renamed OAuth token) and passes it via subprocess.run(env=...) so docker run -e NAME forwards by-name from the child's environment, not the parent's. Values are excluded from the dataclass repr (forwarded on ResolvedEnv, forwarded_env on DockerBottlePlan) so accidental logging cannot leak secret or interpolated values. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
@@ -119,13 +119,13 @@ class DockerBottleBackend(BottleBackend["DockerBottlePlan", "DockerBottleCleanup
|
|||||||
|
|
||||||
proxy_plan = self._proxy.prepare(bottle, slug, stage_dir)
|
proxy_plan = self._proxy.prepare(bottle, slug, stage_dir)
|
||||||
resolved = resolve_env(manifest, spec.agent_name)
|
resolved = resolve_env(manifest, spec.agent_name)
|
||||||
|
# Everything that should reach the bottle by-name (so its value
|
||||||
|
# never lands on argv or in env_file) goes into one dict. The
|
||||||
|
# rename from CLAUDE_BOTTLE_OAUTH_TOKEN to CLAUDE_CODE_OAUTH_TOKEN
|
||||||
|
# happens here; nothing mutates the host os.environ.
|
||||||
|
forwarded_env: dict[str, str] = dict(resolved.forwarded)
|
||||||
if spec.forward_oauth_token:
|
if spec.forward_oauth_token:
|
||||||
# Forward by-name so the value never lands on argv or in
|
forwarded_env["CLAUDE_CODE_OAUTH_TOKEN"] = os.environ["CLAUDE_BOTTLE_OAUTH_TOKEN"]
|
||||||
# env_file; the parent-side rename from CLAUDE_BOTTLE_OAUTH_TOKEN
|
|
||||||
# to CLAUDE_CODE_OAUTH_TOKEN happens in `_run_agent_container`
|
|
||||||
# via a per-subprocess env dict, so global os.environ stays
|
|
||||||
# untouched and `prepare` remains side-effect-free.
|
|
||||||
resolved.forwarded.append("CLAUDE_CODE_OAUTH_TOKEN")
|
|
||||||
self._write_env_file(resolved, env_file)
|
self._write_env_file(resolved, env_file)
|
||||||
prompt_file.write_text(agent.prompt)
|
prompt_file.write_text(agent.prompt)
|
||||||
|
|
||||||
@@ -142,7 +142,7 @@ class DockerBottleBackend(BottleBackend["DockerBottlePlan", "DockerBottleCleanup
|
|||||||
derived_image=derived_image,
|
derived_image=derived_image,
|
||||||
runtime_image=runtime_image,
|
runtime_image=runtime_image,
|
||||||
env_file=env_file,
|
env_file=env_file,
|
||||||
forwarded_env=tuple(resolved.forwarded),
|
forwarded_env=forwarded_env,
|
||||||
prompt_file=prompt_file,
|
prompt_file=prompt_file,
|
||||||
proxy_plan=proxy_plan,
|
proxy_plan=proxy_plan,
|
||||||
allowlist_summary=allowlist_summary,
|
allowlist_summary=allowlist_summary,
|
||||||
@@ -232,12 +232,11 @@ class DockerBottleBackend(BottleBackend["DockerBottlePlan", "DockerBottleCleanup
|
|||||||
|
|
||||||
info(f"starting container {plan.container_name} from {plan.runtime_image}")
|
info(f"starting container {plan.container_name} from {plan.runtime_image}")
|
||||||
|
|
||||||
# Rename CLAUDE_BOTTLE_OAUTH_TOKEN -> CLAUDE_CODE_OAUTH_TOKEN in the
|
# Inject forwarded values (secrets, interpolated host vars, the
|
||||||
# child docker process's env (not in our own), so `-e CLAUDE_CODE_OAUTH_TOKEN`
|
# renamed OAuth token) into the docker-run child's env so the
|
||||||
# forwards by-name without the value landing on argv.
|
# `-e NAME` flags above pick them up — without touching our own
|
||||||
child_env: dict[str, str] | None = None
|
# os.environ or putting values on argv.
|
||||||
if plan.spec.forward_oauth_token:
|
child_env: dict[str, str] = {**os.environ, **plan.forwarded_env}
|
||||||
child_env = {**os.environ, "CLAUDE_CODE_OAUTH_TOKEN": os.environ["CLAUDE_BOTTLE_OAUTH_TOKEN"]}
|
|
||||||
|
|
||||||
name_idx = docker_args.index("--name") + 1
|
name_idx = docker_args.index("--name") + 1
|
||||||
for candidate in docker_mod.container_name_candidates(plan.container_name):
|
for candidate in docker_mod.container_name_candidates(plan.container_name):
|
||||||
|
|||||||
@@ -8,7 +8,7 @@ further resolution; show_plan-style rendering is the `print` method.
|
|||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
import sys
|
import sys
|
||||||
from dataclasses import dataclass
|
from dataclasses import dataclass, field
|
||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
|
|
||||||
from ...log import info
|
from ...log import info
|
||||||
@@ -42,7 +42,11 @@ class DockerBottlePlan(BottlePlan):
|
|||||||
derived_image: str # "" -> no derived image
|
derived_image: str # "" -> no derived image
|
||||||
runtime_image: str # image actually launched (derived or base)
|
runtime_image: str # image actually launched (derived or base)
|
||||||
env_file: Path # docker --env-file: NAME=VALUE literals
|
env_file: Path # docker --env-file: NAME=VALUE literals
|
||||||
forwarded_env: tuple[str, ...] # docker -e <NAME>: forwarded by-name
|
# name -> value for vars forwarded into the docker-run child process
|
||||||
|
# via subprocess env (so values never land on argv or in a file).
|
||||||
|
# repr=False keeps secret/interpolated/OAuth values out of any
|
||||||
|
# accidental log of the plan dataclass.
|
||||||
|
forwarded_env: dict[str, str] = field(repr=False)
|
||||||
prompt_file: Path
|
prompt_file: Path
|
||||||
proxy_plan: PipelockProxyPlan
|
proxy_plan: PipelockProxyPlan
|
||||||
allowlist_summary: str
|
allowlist_summary: str
|
||||||
|
|||||||
+23
-18
@@ -2,11 +2,11 @@
|
|||||||
backend-neutral ResolvedEnv describing how the bottle should receive
|
backend-neutral ResolvedEnv describing how the bottle should receive
|
||||||
each variable:
|
each variable:
|
||||||
|
|
||||||
- `forwarded` — names whose values have been placed into this
|
- `forwarded` — name→value pairs the backend must inject into the
|
||||||
process's env (from a tty prompt for `secret`, from the matching
|
launched process's environment directly (e.g. via
|
||||||
host var for `interpolated`). The backend is expected to pass
|
`subprocess.run(env=...)`), so the value never appears on argv,
|
||||||
these to the bottle by-name so the value never appears on argv,
|
in a file, or in a log line. The dict is excluded from the
|
||||||
in a file, or in a log line.
|
dataclass-generated repr so accidental logging doesn't leak it.
|
||||||
- `literals` — name→value pairs that the manifest carries verbatim.
|
- `literals` — name→value pairs that the manifest carries verbatim.
|
||||||
The backend serializes these however its launcher accepts env
|
The backend serializes these however its launcher accepts env
|
||||||
(an env-file, an API payload, etc.).
|
(an env-file, an API payload, etc.).
|
||||||
@@ -22,6 +22,8 @@ Critical rules:
|
|||||||
interpolated env var. Both are treated as potentially sensitive:
|
interpolated env var. Both are treated as potentially sensitive:
|
||||||
nothing about their value (other than presence) ever lands on
|
nothing about their value (other than presence) ever lands on
|
||||||
disk, in a log line, or on argv.
|
disk, in a log line, or on argv.
|
||||||
|
- resolve_env does NOT mutate the host process's os.environ; the
|
||||||
|
backend builds a child env dict and passes it to its launcher.
|
||||||
- Errors mention only the variable NAME, never any portion of the value.
|
- Errors mention only the variable NAME, never any portion of the value.
|
||||||
"""
|
"""
|
||||||
|
|
||||||
@@ -43,11 +45,15 @@ _INTERPOLATED_RE = re.compile(r"^\$\{([A-Za-z_][A-Za-z0-9_]*)\}$")
|
|||||||
class ResolvedEnv:
|
class ResolvedEnv:
|
||||||
"""Backend-neutral env resolution result.
|
"""Backend-neutral env resolution result.
|
||||||
|
|
||||||
`forwarded` names have already been exported into os.environ by
|
`forwarded` maps name→value for entries whose values must not land
|
||||||
resolve_env; the backend forwards by-name. `literals` carry their
|
on argv or in a file; backends inject them into the launched
|
||||||
values verbatim and are serialized by the backend."""
|
process's environment directly. `repr=False` so a stray `repr(...)`
|
||||||
|
or log of the dataclass doesn't dump secret values.
|
||||||
|
|
||||||
forwarded: list[str] = field(default_factory=list[str])
|
`literals` carry values verbatim and are serialized by the backend
|
||||||
|
however its launcher accepts non-secret env."""
|
||||||
|
|
||||||
|
forwarded: dict[str, str] = field(default_factory=dict[str, str], repr=False)
|
||||||
literals: dict[str, str] = field(default_factory=dict[str, str])
|
literals: dict[str, str] = field(default_factory=dict[str, str])
|
||||||
|
|
||||||
|
|
||||||
@@ -110,11 +116,13 @@ def _read_secret_silent(name: str, prompt_body: str) -> str:
|
|||||||
|
|
||||||
def resolve_env(manifest: Manifest, agent: str) -> ResolvedEnv:
|
def resolve_env(manifest: Manifest, agent: str) -> ResolvedEnv:
|
||||||
"""Iterate the agent's env entries:
|
"""Iterate the agent's env entries:
|
||||||
- secret: always prompt; export into this process; mark forwarded
|
- secret: prompt at runtime; carry value in forwarded
|
||||||
- interpolated: copy host value; export under target name; mark forwarded
|
- interpolated: read $HOST_VAR from os.environ; carry value in forwarded
|
||||||
- literal: include in the literals map verbatim
|
- literal: include in the literals map verbatim
|
||||||
"""
|
|
||||||
forwarded: list[str] = []
|
The host process's os.environ is read but never mutated; the
|
||||||
|
backend injects forwarded values via its launcher's env parameter."""
|
||||||
|
forwarded: dict[str, str] = {}
|
||||||
literals: dict[str, str] = {}
|
literals: dict[str, str] = {}
|
||||||
bottle = manifest.bottle_for(agent)
|
bottle = manifest.bottle_for(agent)
|
||||||
for name, raw in bottle.env.items():
|
for name, raw in bottle.env.items():
|
||||||
@@ -123,9 +131,7 @@ def resolve_env(manifest: Manifest, agent: str) -> ResolvedEnv:
|
|||||||
kind = env_entry_kind(raw)
|
kind = env_entry_kind(raw)
|
||||||
if kind == "secret":
|
if kind == "secret":
|
||||||
prompt_body = env_entry_secret_prompt(raw)
|
prompt_body = env_entry_secret_prompt(raw)
|
||||||
value = _read_secret_silent(name, prompt_body)
|
forwarded[name] = _read_secret_silent(name, prompt_body)
|
||||||
os.environ[name] = value
|
|
||||||
forwarded.append(name)
|
|
||||||
elif kind == "interpolated":
|
elif kind == "interpolated":
|
||||||
host_var = env_entry_interpolated_from(raw)
|
host_var = env_entry_interpolated_from(raw)
|
||||||
host_value = os.environ.get(host_var, "")
|
host_value = os.environ.get(host_var, "")
|
||||||
@@ -134,8 +140,7 @@ def resolve_env(manifest: Manifest, agent: str) -> ResolvedEnv:
|
|||||||
f"env entry {name} is interpolated from ${host_var}, "
|
f"env entry {name} is interpolated from ${host_var}, "
|
||||||
f"but ${host_var} is unset or empty in the host environment."
|
f"but ${host_var} is unset or empty in the host environment."
|
||||||
)
|
)
|
||||||
os.environ[name] = host_value
|
forwarded[name] = host_value
|
||||||
forwarded.append(name)
|
|
||||||
else: # literal
|
else: # literal
|
||||||
literals[name] = raw
|
literals[name] = raw
|
||||||
return ResolvedEnv(forwarded=forwarded, literals=literals)
|
return ResolvedEnv(forwarded=forwarded, literals=literals)
|
||||||
|
|||||||
Reference in New Issue
Block a user