refactor(agent): move placeholder env injection into agent_provision_plan
The has_provider_auth check and egress-placeholder injection were duplicated in both backends. Move them into agent_provision_plan so the provisioner owns that decision entirely: - Replace has_provider_auth: bool param with manifest_egress_routes, compute has_provider_auth internally from the route roles. - Inject CLAUDE_CODE_OAUTH_TOKEN=egress-placeholder inside the plan when has_provider_auth, alongside the existing nonessential-traffic vars. Backends no longer touch the placeholder env. - Remove placeholder_env from AgentProviderRuntime; expose placeholder_env_for() for print_util's hide-from-summary logic. Assisted-by: Claude Code
This commit is contained in:
@@ -34,7 +34,6 @@ class AgentProviderRuntime:
|
|||||||
image: str
|
image: str
|
||||||
dockerfile: str
|
dockerfile: str
|
||||||
auth_role: str
|
auth_role: str
|
||||||
placeholder_env: str
|
|
||||||
prompt_mode: PromptMode
|
prompt_mode: PromptMode
|
||||||
bypass_args: tuple[str, ...]
|
bypass_args: tuple[str, ...]
|
||||||
resume_args: tuple[str, ...]
|
resume_args: tuple[str, ...]
|
||||||
@@ -100,7 +99,6 @@ _RUNTIMES = {
|
|||||||
image="bot-bottle-claude:latest",
|
image="bot-bottle-claude:latest",
|
||||||
dockerfile=str(_REPO_ROOT / "Dockerfile.claude"),
|
dockerfile=str(_REPO_ROOT / "Dockerfile.claude"),
|
||||||
auth_role="claude_code_oauth",
|
auth_role="claude_code_oauth",
|
||||||
placeholder_env="CLAUDE_CODE_OAUTH_TOKEN",
|
|
||||||
prompt_mode="append_file",
|
prompt_mode="append_file",
|
||||||
bypass_args=("--dangerously-skip-permissions",),
|
bypass_args=("--dangerously-skip-permissions",),
|
||||||
resume_args=("--continue",),
|
resume_args=("--continue",),
|
||||||
@@ -112,7 +110,6 @@ _RUNTIMES = {
|
|||||||
image="bot-bottle-codex:latest",
|
image="bot-bottle-codex:latest",
|
||||||
dockerfile=str(_REPO_ROOT / "Dockerfile.codex"),
|
dockerfile=str(_REPO_ROOT / "Dockerfile.codex"),
|
||||||
auth_role="",
|
auth_role="",
|
||||||
placeholder_env="",
|
|
||||||
prompt_mode="read_prompt_file",
|
prompt_mode="read_prompt_file",
|
||||||
bypass_args=("--dangerously-bypass-approvals-and-sandbox",),
|
bypass_args=("--dangerously-bypass-approvals-and-sandbox",),
|
||||||
resume_args=("resume", "--last"),
|
resume_args=("resume", "--last"),
|
||||||
@@ -125,6 +122,13 @@ def runtime_for(template: str) -> AgentProviderRuntime:
|
|||||||
return _RUNTIMES[template]
|
return _RUNTIMES[template]
|
||||||
|
|
||||||
|
|
||||||
|
def placeholder_env_for(template: str) -> str:
|
||||||
|
"""Return the provider auth placeholder env var name, or empty string."""
|
||||||
|
if template == PROVIDER_CLAUDE:
|
||||||
|
return "CLAUDE_CODE_OAUTH_TOKEN"
|
||||||
|
return ""
|
||||||
|
|
||||||
|
|
||||||
def agent_provision_plan(
|
def agent_provision_plan(
|
||||||
*,
|
*,
|
||||||
template: str,
|
template: str,
|
||||||
@@ -133,10 +137,13 @@ def agent_provision_plan(
|
|||||||
guest_home: str = "/home/node",
|
guest_home: str = "/home/node",
|
||||||
guest_env: dict[str, str] | None = None,
|
guest_env: dict[str, str] | None = None,
|
||||||
forward_host_credentials: bool = False,
|
forward_host_credentials: bool = False,
|
||||||
has_provider_auth: bool = False,
|
manifest_egress_routes: tuple[EgressRoute, ...] = (),
|
||||||
host_env: dict[str, str] | None = None,
|
host_env: dict[str, str] | None = None,
|
||||||
) -> AgentProvisionPlan:
|
) -> AgentProvisionPlan:
|
||||||
runtime = runtime_for(template)
|
runtime = runtime_for(template)
|
||||||
|
has_provider_auth = bool(runtime.auth_role) and any(
|
||||||
|
runtime.auth_role in r.roles for r in manifest_egress_routes
|
||||||
|
)
|
||||||
resolved_guest_env = dict(guest_env or {})
|
resolved_guest_env = dict(guest_env or {})
|
||||||
env_vars: dict[str, str] = {}
|
env_vars: dict[str, str] = {}
|
||||||
dirs: list[AgentProvisionDir] = []
|
dirs: list[AgentProvisionDir] = []
|
||||||
@@ -193,6 +200,7 @@ def agent_provision_plan(
|
|||||||
"guest, but Codex did not accept it"
|
"guest, but Codex did not accept it"
|
||||||
)))
|
)))
|
||||||
if template == PROVIDER_CLAUDE and has_provider_auth:
|
if template == PROVIDER_CLAUDE and has_provider_auth:
|
||||||
|
env_vars["CLAUDE_CODE_OAUTH_TOKEN"] = "egress-placeholder"
|
||||||
env_vars["CLAUDE_CODE_DISABLE_NONESSENTIAL_TRAFFIC"] = "1"
|
env_vars["CLAUDE_CODE_DISABLE_NONESSENTIAL_TRAFFIC"] = "1"
|
||||||
env_vars["DISABLE_ERROR_REPORTING"] = "1"
|
env_vars["DISABLE_ERROR_REPORTING"] = "1"
|
||||||
|
|
||||||
|
|||||||
@@ -168,19 +168,6 @@ def resolve_plan(
|
|||||||
# never lands on argv or in env_file) goes into one dict. Nothing
|
# never lands on argv or in env_file) goes into one dict. Nothing
|
||||||
# mutates the host os.environ.
|
# mutates the host os.environ.
|
||||||
forwarded_env: dict[str, str] = dict(resolved.forwarded)
|
forwarded_env: dict[str, str] = dict(resolved.forwarded)
|
||||||
# Some provider CLIs refuse to start without *some* credential
|
|
||||||
# env var even when egress will strip + re-inject the real
|
|
||||||
# Authorization header. For those providers, auth_role names the
|
|
||||||
# route marker that enables a non-secret placeholder env. Codex is
|
|
||||||
# intentionally absent here: it should use its device/ChatGPT login
|
|
||||||
# state, and an OPENAI_API_KEY placeholder would force API-key auth.
|
|
||||||
has_provider_auth = any(
|
|
||||||
provider_runtime.auth_role
|
|
||||||
and provider_runtime.auth_role in r.roles
|
|
||||||
for r in egress_manifest_routes(bottle)
|
|
||||||
)
|
|
||||||
if has_provider_auth and provider_runtime.placeholder_env:
|
|
||||||
forwarded_env[provider_runtime.placeholder_env] = "egress-placeholder"
|
|
||||||
_write_env_file(resolved, env_file)
|
_write_env_file(resolved, env_file)
|
||||||
prompt_file.write_text(agent.prompt)
|
prompt_file.write_text(agent.prompt)
|
||||||
|
|
||||||
@@ -191,7 +178,7 @@ def resolve_plan(
|
|||||||
state_dir=agent_dir,
|
state_dir=agent_dir,
|
||||||
guest_home=os.environ.get("BOT_BOTTLE_CONTAINER_HOME", "/home/node"),
|
guest_home=os.environ.get("BOT_BOTTLE_CONTAINER_HOME", "/home/node"),
|
||||||
forward_host_credentials=provider.forward_host_credentials,
|
forward_host_credentials=provider.forward_host_credentials,
|
||||||
has_provider_auth=has_provider_auth,
|
manifest_egress_routes=egress_manifest_routes(bottle),
|
||||||
host_env=dict(os.environ),
|
host_env=dict(os.environ),
|
||||||
)
|
)
|
||||||
guest_env = dict(agent_provision.guest_env)
|
guest_env = dict(agent_provision.guest_env)
|
||||||
|
|||||||
@@ -9,7 +9,7 @@ from __future__ import annotations
|
|||||||
|
|
||||||
from typing import Sequence
|
from typing import Sequence
|
||||||
|
|
||||||
from ..agent_provider import runtime_for
|
from ..agent_provider import placeholder_env_for
|
||||||
from ..log import info
|
from ..log import info
|
||||||
|
|
||||||
|
|
||||||
@@ -41,5 +41,5 @@ def visible_agent_env_names(
|
|||||||
think a real key is entering the agent, so hide only the active
|
think a real key is entering the agent, so hide only the active
|
||||||
provider-owned placeholder.
|
provider-owned placeholder.
|
||||||
"""
|
"""
|
||||||
hidden = {runtime_for(agent_provider_template).placeholder_env}
|
hidden = {placeholder_env_for(agent_provider_template)}
|
||||||
return sorted({name for name in env_names if name and name not in hidden})
|
return sorted({name for name in env_names if name and name not in hidden})
|
||||||
|
|||||||
@@ -100,20 +100,6 @@ def resolve_plan(
|
|||||||
git_gate_dir.mkdir(parents=True, exist_ok=True)
|
git_gate_dir.mkdir(parents=True, exist_ok=True)
|
||||||
git_gate_plan = GitGate().prepare(bottle, slug, git_gate_dir)
|
git_gate_plan = GitGate().prepare(bottle, slug, git_gate_dir)
|
||||||
|
|
||||||
# Some provider CLIs refuse to start without *some* credential
|
|
||||||
# env var even when egress will strip + re-inject the real
|
|
||||||
# Authorization header. For those providers, auth_role names the
|
|
||||||
# route marker that enables a non-secret placeholder env. Codex is
|
|
||||||
# intentionally absent here: it should use its device/ChatGPT login
|
|
||||||
# state, and an OPENAI_API_KEY placeholder would force API-key auth.
|
|
||||||
has_provider_auth = any(
|
|
||||||
provider_runtime.auth_role
|
|
||||||
and provider_runtime.auth_role in r.roles
|
|
||||||
for r in egress_manifest_routes(bottle)
|
|
||||||
)
|
|
||||||
if has_provider_auth and provider_runtime.placeholder_env:
|
|
||||||
guest_env[provider_runtime.placeholder_env] = "egress-placeholder"
|
|
||||||
|
|
||||||
# Prompt file is always written (mode 0o600) so the in-VM
|
# Prompt file is always written (mode 0o600) so the in-VM
|
||||||
# path always exists. Content is the agent's `prompt`
|
# path always exists. Content is the agent's `prompt`
|
||||||
# field (markdown body) — empty for agents with no prompt.
|
# field (markdown body) — empty for agents with no prompt.
|
||||||
@@ -148,7 +134,7 @@ def resolve_plan(
|
|||||||
guest_home=os.environ.get("BOT_BOTTLE_GUEST_HOME", "/home/node"),
|
guest_home=os.environ.get("BOT_BOTTLE_GUEST_HOME", "/home/node"),
|
||||||
guest_env=guest_env,
|
guest_env=guest_env,
|
||||||
forward_host_credentials=provider.forward_host_credentials,
|
forward_host_credentials=provider.forward_host_credentials,
|
||||||
has_provider_auth=has_provider_auth,
|
manifest_egress_routes=egress_manifest_routes(bottle),
|
||||||
host_env=dict(os.environ),
|
host_env=dict(os.environ),
|
||||||
)
|
)
|
||||||
merged_guest_env = dict(agent_provision.guest_env)
|
merged_guest_env = dict(agent_provision.guest_env)
|
||||||
|
|||||||
@@ -13,7 +13,7 @@ from bot_bottle.agent_provider import (
|
|||||||
agent_provision_plan,
|
agent_provision_plan,
|
||||||
runtime_for,
|
runtime_for,
|
||||||
)
|
)
|
||||||
from bot_bottle.egress import CODEX_HOST_CREDENTIAL_TOKEN_REF
|
from bot_bottle.egress import CODEX_HOST_CREDENTIAL_TOKEN_REF, EgressRoute
|
||||||
|
|
||||||
|
|
||||||
def _jwt(exp: int) -> str:
|
def _jwt(exp: int) -> str:
|
||||||
@@ -24,15 +24,13 @@ def _jwt(exp: int) -> str:
|
|||||||
|
|
||||||
|
|
||||||
class TestAgentProviderRuntime(unittest.TestCase):
|
class TestAgentProviderRuntime(unittest.TestCase):
|
||||||
def test_claude_keeps_oauth_placeholder(self):
|
def test_claude_has_auth_role(self):
|
||||||
runtime = runtime_for("claude")
|
runtime = runtime_for("claude")
|
||||||
self.assertEqual("claude_code_oauth", runtime.auth_role)
|
self.assertEqual("claude_code_oauth", runtime.auth_role)
|
||||||
self.assertEqual("CLAUDE_CODE_OAUTH_TOKEN", runtime.placeholder_env)
|
|
||||||
|
|
||||||
def test_codex_does_not_inject_openai_api_key_placeholder(self):
|
def test_codex_has_no_auth_role(self):
|
||||||
runtime = runtime_for("codex")
|
runtime = runtime_for("codex")
|
||||||
self.assertEqual("", runtime.auth_role)
|
self.assertEqual("", runtime.auth_role)
|
||||||
self.assertEqual("", runtime.placeholder_env)
|
|
||||||
|
|
||||||
def test_codex_plan_declares_home_state(self):
|
def test_codex_plan_declares_home_state(self):
|
||||||
with tempfile.TemporaryDirectory(prefix="bb-provider.") as tmp:
|
with tempfile.TemporaryDirectory(prefix="bb-provider.") as tmp:
|
||||||
@@ -81,18 +79,19 @@ class TestAgentProviderRuntime(unittest.TestCase):
|
|||||||
self.assertEqual(1, len(plan.verify))
|
self.assertEqual(1, len(plan.verify))
|
||||||
self.assertIn("CODEX_HOME=/run/codex-home", plan.verify[0].argv)
|
self.assertIn("CODEX_HOME=/run/codex-home", plan.verify[0].argv)
|
||||||
|
|
||||||
def test_claude_with_provider_auth_disables_nonessential_traffic(self):
|
def test_claude_with_provider_auth_sets_placeholder_and_disables_nonessential_traffic(self):
|
||||||
with tempfile.TemporaryDirectory(prefix="bb-provider.") as tmp:
|
with tempfile.TemporaryDirectory(prefix="bb-provider.") as tmp:
|
||||||
plan = agent_provision_plan(
|
plan = agent_provision_plan(
|
||||||
template="claude",
|
template="claude",
|
||||||
dockerfile="/tmp/Dockerfile.claude",
|
dockerfile="/tmp/Dockerfile.claude",
|
||||||
state_dir=Path(tmp),
|
state_dir=Path(tmp),
|
||||||
has_provider_auth=True,
|
manifest_egress_routes=(EgressRoute(
|
||||||
|
host="api.anthropic.com",
|
||||||
|
roles=("claude_code_oauth",),
|
||||||
|
),),
|
||||||
)
|
)
|
||||||
self.assertEqual(
|
self.assertEqual("egress-placeholder", plan.env_vars["CLAUDE_CODE_OAUTH_TOKEN"])
|
||||||
"1",
|
self.assertEqual("1", plan.env_vars["CLAUDE_CODE_DISABLE_NONESSENTIAL_TRAFFIC"])
|
||||||
plan.env_vars["CLAUDE_CODE_DISABLE_NONESSENTIAL_TRAFFIC"],
|
|
||||||
)
|
|
||||||
self.assertEqual("1", plan.env_vars["DISABLE_ERROR_REPORTING"])
|
self.assertEqual("1", plan.env_vars["DISABLE_ERROR_REPORTING"])
|
||||||
|
|
||||||
def test_codex_forward_host_credentials_populates_egress_routes(self):
|
def test_codex_forward_host_credentials_populates_egress_routes(self):
|
||||||
|
|||||||
Reference in New Issue
Block a user