From 5f29fd10e27bf4ce9aca136477592c5946462dc7 Mon Sep 17 00:00:00 2001 From: didericis Date: Tue, 12 May 2026 10:37:01 -0400 Subject: [PATCH] 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 --- claude_bottle/backend/docker/backend.py | 25 ++++++------- claude_bottle/backend/docker/bottle_plan.py | 8 +++- claude_bottle/env.py | 41 ++++++++++++--------- 3 files changed, 41 insertions(+), 33 deletions(-) diff --git a/claude_bottle/backend/docker/backend.py b/claude_bottle/backend/docker/backend.py index 4acd985..c6b68b5 100644 --- a/claude_bottle/backend/docker/backend.py +++ b/claude_bottle/backend/docker/backend.py @@ -119,13 +119,13 @@ class DockerBottleBackend(BottleBackend["DockerBottlePlan", "DockerBottleCleanup proxy_plan = self._proxy.prepare(bottle, slug, stage_dir) 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: - # Forward by-name so the value never lands on argv or in - # 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") + forwarded_env["CLAUDE_CODE_OAUTH_TOKEN"] = os.environ["CLAUDE_BOTTLE_OAUTH_TOKEN"] self._write_env_file(resolved, env_file) prompt_file.write_text(agent.prompt) @@ -142,7 +142,7 @@ class DockerBottleBackend(BottleBackend["DockerBottlePlan", "DockerBottleCleanup derived_image=derived_image, runtime_image=runtime_image, env_file=env_file, - forwarded_env=tuple(resolved.forwarded), + forwarded_env=forwarded_env, prompt_file=prompt_file, proxy_plan=proxy_plan, allowlist_summary=allowlist_summary, @@ -232,12 +232,11 @@ class DockerBottleBackend(BottleBackend["DockerBottlePlan", "DockerBottleCleanup info(f"starting container {plan.container_name} from {plan.runtime_image}") - # Rename CLAUDE_BOTTLE_OAUTH_TOKEN -> CLAUDE_CODE_OAUTH_TOKEN in the - # child docker process's env (not in our own), so `-e CLAUDE_CODE_OAUTH_TOKEN` - # forwards by-name without the value landing on argv. - child_env: dict[str, str] | None = None - if plan.spec.forward_oauth_token: - child_env = {**os.environ, "CLAUDE_CODE_OAUTH_TOKEN": os.environ["CLAUDE_BOTTLE_OAUTH_TOKEN"]} + # Inject forwarded values (secrets, interpolated host vars, the + # renamed OAuth token) into the docker-run child's env so the + # `-e NAME` flags above pick them up — without touching our own + # os.environ or putting values on argv. + child_env: dict[str, str] = {**os.environ, **plan.forwarded_env} name_idx = docker_args.index("--name") + 1 for candidate in docker_mod.container_name_candidates(plan.container_name): diff --git a/claude_bottle/backend/docker/bottle_plan.py b/claude_bottle/backend/docker/bottle_plan.py index 91d5bd7..5ad3da8 100644 --- a/claude_bottle/backend/docker/bottle_plan.py +++ b/claude_bottle/backend/docker/bottle_plan.py @@ -8,7 +8,7 @@ further resolution; show_plan-style rendering is the `print` method. from __future__ import annotations import sys -from dataclasses import dataclass +from dataclasses import dataclass, field from pathlib import Path from ...log import info @@ -42,7 +42,11 @@ class DockerBottlePlan(BottlePlan): derived_image: str # "" -> no derived image runtime_image: str # image actually launched (derived or base) env_file: Path # docker --env-file: NAME=VALUE literals - forwarded_env: tuple[str, ...] # docker -e : 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 proxy_plan: PipelockProxyPlan allowlist_summary: str diff --git a/claude_bottle/env.py b/claude_bottle/env.py index 0337214..a5ed287 100644 --- a/claude_bottle/env.py +++ b/claude_bottle/env.py @@ -2,11 +2,11 @@ backend-neutral ResolvedEnv describing how the bottle should receive each variable: - - `forwarded` — names whose values have been placed into this - process's env (from a tty prompt for `secret`, from the matching - host var for `interpolated`). The backend is expected to pass - these to the bottle by-name so the value never appears on argv, - in a file, or in a log line. + - `forwarded` — name→value pairs the backend must inject into the + launched process's environment directly (e.g. via + `subprocess.run(env=...)`), so the value never appears on argv, + in a file, or in a log line. The dict is excluded from the + dataclass-generated repr so accidental logging doesn't leak it. - `literals` — name→value pairs that the manifest carries verbatim. The backend serializes these however its launcher accepts env (an env-file, an API payload, etc.). @@ -22,6 +22,8 @@ Critical rules: interpolated env var. Both are treated as potentially sensitive: nothing about their value (other than presence) ever lands on 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. """ @@ -43,11 +45,15 @@ _INTERPOLATED_RE = re.compile(r"^\$\{([A-Za-z_][A-Za-z0-9_]*)\}$") class ResolvedEnv: """Backend-neutral env resolution result. - `forwarded` names have already been exported into os.environ by - resolve_env; the backend forwards by-name. `literals` carry their - values verbatim and are serialized by the backend.""" + `forwarded` maps name→value for entries whose values must not land + on argv or in a file; backends inject them into the launched + 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]) @@ -110,11 +116,13 @@ def _read_secret_silent(name: str, prompt_body: str) -> str: def resolve_env(manifest: Manifest, agent: str) -> ResolvedEnv: """Iterate the agent's env entries: - - secret: always prompt; export into this process; mark forwarded - - interpolated: copy host value; export under target name; mark forwarded + - secret: prompt at runtime; carry value in forwarded + - interpolated: read $HOST_VAR from os.environ; carry value in forwarded - 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] = {} bottle = manifest.bottle_for(agent) for name, raw in bottle.env.items(): @@ -123,9 +131,7 @@ def resolve_env(manifest: Manifest, agent: str) -> ResolvedEnv: kind = env_entry_kind(raw) if kind == "secret": prompt_body = env_entry_secret_prompt(raw) - value = _read_secret_silent(name, prompt_body) - os.environ[name] = value - forwarded.append(name) + forwarded[name] = _read_secret_silent(name, prompt_body) elif kind == "interpolated": host_var = env_entry_interpolated_from(raw) 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"but ${host_var} is unset or empty in the host environment." ) - os.environ[name] = host_value - forwarded.append(name) + forwarded[name] = host_value else: # literal literals[name] = raw return ResolvedEnv(forwarded=forwarded, literals=literals)