From 2ea3f760da76a066567c408906abe9954f6e64c3 Mon Sep 17 00:00:00 2001 From: Ben Browning <56071+bbrowning@users.noreply.github.com> Date: Thu, 27 Aug 2026 22:52:48 +0000 Subject: [PATCH 1/3] Preserve proxy credentials during domain updates Preserve current Podman secret bindings and Docker credential values when domain-only changes recreate the proxy. Add explicit credential refresh, atomic durable domain state, and rollback ordering so failed replacements leave the working proxy intact. Document the behavior and cover inspection, refresh, recovery, and failure paths across both backends. --- README.md | 31 +++ src/paude/backends/base.py | 10 +- src/paude/backends/podman/backend.py | 30 ++- src/paude/backends/podman/proxy.py | 85 +++++-- .../backends/podman/proxy_credentials.py | 123 ++++++++++ src/paude/backends/podman/proxy_state.py | 114 +++++++++ src/paude/backends/podman/resources.py | 7 +- src/paude/backends/proxy_config.py | 53 ++++ src/paude/cli/domains.py | 73 +++++- src/paude/cli/help.py | 11 + src/paude/container/proxy_inspect.py | 94 ++++++++ src/paude/container/proxy_runner.py | 142 ++++++++++- tests/test_allow_domain.py | 73 +++++- tests/test_cli.py | 70 +++++- tests/test_podman_proxy.py | 99 +++++++- tests/test_podman_session.py | 6 + tests/test_proxy_secrets.py | 228 +++++++++++++++++- tests/test_proxy_state.py | 70 ++++++ tests/test_session_resources.py | 25 ++ tests/test_upgrade.py | 3 + 20 files changed, 1298 insertions(+), 49 deletions(-) create mode 100644 src/paude/backends/podman/proxy_state.py create mode 100644 src/paude/container/proxy_inspect.py create mode 100644 tests/test_proxy_state.py diff --git a/README.md b/README.md index d5ba6f1..f279352 100644 --- a/README.md +++ b/README.md @@ -257,6 +257,37 @@ an upgrade is interrupted (e.g. `Ctrl-C`) you can simply re-run existing session in place, e.g. `paude upgrade SESSION --add-agent codex`. See [Session Management](docs/SESSIONS.md) for the per-agent persistence paths. +### Updating allowed domains safely + +Change a running session's egress policy with `paude allowed-domains`: + +```bash +paude allowed-domains SESSION --add .example.com +paude allowed-domains SESSION --remove .example.com +paude allowed-domains SESSION --replace default .example.com +``` + +Domain-only updates preserve every credential binding already attached to the +proxy; they do not re-read possibly missing or stale values from the invoking +shell. Paude preflights the replacement and retains the existing proxy until +the replacement is running and the new domains are committed. A failed update +restores the old proxy and policy instead of leaving the session without an +authenticated route. Committed domains survive proxy recovery and are used by +subsequent backups and upgrades. + +To deliberately replace credentials whose fresh values are available in the +current environment, add `--refresh-credentials` to a domain mutation. Fresh +values replace matching bindings while unrelated credentials remain attached: + +```bash +export CLAUDE_CODE_OAUTH_TOKEN=new-setup-token +paude allowed-domains SESSION --add .example.com --refresh-credentials +``` + +If a required binding is neither attached nor supplied for an explicit +refresh, the command fails before changing the working proxy and names the +missing environment variable. + ### Backing up a session To guard a long-running session against loss, snapshot it to a portable bundle: diff --git a/src/paude/backends/base.py b/src/paude/backends/base.py index 4739fcf..0561b7e 100644 --- a/src/paude/backends/base.py +++ b/src/paude/backends/base.py @@ -235,12 +235,20 @@ def get_proxy_blocked_log(self, name: str) -> str | None: """ ... - def update_allowed_domains(self, name: str, domains: list[str]) -> None: + def update_allowed_domains( + self, + name: str, + domains: list[str], + *, + refresh_credentials: bool = False, + ) -> None: """Update allowed domains for a session. Args: name: Session name. domains: New list of allowed domains. + refresh_credentials: Replace bindings for credentials currently + supplied by the host while preserving all other bindings. """ ... diff --git a/src/paude/backends/podman/backend.py b/src/paude/backends/podman/backend.py index 066bdfe..bea94a5 100644 --- a/src/paude/backends/podman/backend.py +++ b/src/paude/backends/podman/backend.py @@ -413,16 +413,38 @@ def get_proxy_blocked_log(self, name: str) -> str | None: require_session(self._runner, name) return self._proxy.get_blocked_log(name) - def update_allowed_domains(self, name: str, domains: list[str]) -> None: + def update_allowed_domains( + self, + name: str, + domains: list[str], + *, + refresh_credentials: bool = False, + ) -> None: """Update allowed domains for a session.""" require_session(self._runner, name) composition = get_session_composition(self._runner, name) from paude.backends.podman.helpers import get_session_credential_providers + from paude.backends.proxy_config import ( + ProxyCredentials, + proxy_credential_targets, + required_proxy_credential_targets, + ) - proxy_creds = self._setup.gather_proxy_credentials( - composition, get_session_credential_providers(self._runner, name) + providers = get_session_credential_providers(self._runner, name) + refresh = ( + self._setup.gather_proxy_credentials(composition, providers) + if refresh_credentials + else ProxyCredentials(chatgpt_oauth_mode="chatgpt" in providers) + ) + self._proxy.update_domains( + name, + domains, + credentials=refresh, + credential_targets=proxy_credential_targets(composition), + required_credentials=required_proxy_credential_targets( + composition, providers + ), ) - self._proxy.update_domains(name, domains, credentials=proxy_creds) def exec_in_session(self, name: str, command: str) -> tuple[int, str, str]: """Execute a command inside a running session's container.""" diff --git a/src/paude/backends/podman/proxy.py b/src/paude/backends/podman/proxy.py index ae024ee..7fe52b1 100644 --- a/src/paude/backends/podman/proxy.py +++ b/src/paude/backends/podman/proxy.py @@ -21,6 +21,7 @@ proxy_container_name, ) from paude.backends.podman.proxy_credentials import ProxyCredentialManager +from paude.backends.podman.proxy_state import ProxyStateStore from paude.backends.proxy_config import CA_CERT_CONTAINER_PATH as CA_CERT_CONTAINER_PATH from paude.backends.proxy_config import ( PROXY_BLOCKED_LOG_PATH, @@ -98,6 +99,7 @@ def __init__( self._proxy_runner = ProxyRunner(runner) self._ca_cert = CACertDistributor(runner) self._credentials = ProxyCredentialManager(runner) + self._state = ProxyStateStore(runner) def _create_credential_secrets( self, @@ -145,12 +147,23 @@ def get_config_from_labels( return None domains = [d for d in domains_str.split(",") if d] + durable_domains = self.read_domain_state(session_name, proxy_image) + if durable_domains is not None: + domains = durable_domains otel_ports_str = labels.get(PAUDE_LABEL_OTEL_PORTS, "") otel_ports = [int(p) for p in otel_ports_str.split(",") if p] return (proxy_image, domains, otel_ports) + def read_domain_state( + self, session_name: str, proxy_image: str | None + ) -> list[str] | None: + """Read the committed domain override for a session, if one exists.""" + if not proxy_image: + return None + return self._state.read(auth_volume_name(session_name), proxy_image) + def start_if_needed( self, session_name: str, @@ -414,8 +427,10 @@ def update_domains( session_name: str, domains: list[str], credentials: ProxyCredentials | Mapping[str, str] | None = None, + credential_targets: set[str] | None = None, + required_credentials: set[str] | None = None, ) -> None: - """Update allowed domains for a session.""" + """Update domains using preserved credentials and a rollback-safe swap.""" pname = proxy_container_name(session_name) if not self._runner.container_exists(pname): raise ValueError( @@ -445,28 +460,62 @@ def update_domains( agent_ip = derive_agent_ip(proxy_ip) if proxy_ip else None dns = _get_host_dns(self._runner.engine) - secret_refs = self._create_credential_secrets(session_name, credentials) - credential_env = self._credential_env(credentials) + if credentials is None: + credentials = ProxyCredentials() + elif not isinstance(credentials, ProxyCredentials): + credentials = ProxyCredentials(environment=dict(credentials)) + previous_domains = self._state.read(auth_vol, proxy_image) + prepared = self._credentials.prepare_update( + session_name, + pname, + credentials, + credential_targets or set(), + required_credentials or set(), + ) + credential_env = self._credential_env(prepared.credentials) print( f"Updating proxy domains for session '{session_name}'...", file=sys.stderr, ) - self._proxy_runner.recreate_session_proxy( - name=pname, - image=proxy_image, - network=nname, - dns=dns, - allowed_domains=domains, - ip=proxy_ip, - otel_ports=otel_ports, - ca_volume=ca_vol, - credentials=credentials, - allowed_clients=agent_ip, - secret_refs=secret_refs, - credential_env=credential_env, - auth_volume=auth_vol, - ) + swap = None + try: + swap = self._proxy_runner.swap_session_proxy( + name=pname, + image=proxy_image, + network=nname, + dns=dns, + allowed_domains=domains, + ip=proxy_ip, + otel_ports=otel_ports, + ca_volume=ca_vol, + credentials=prepared.credentials, + allowed_clients=agent_ip, + secret_refs=prepared.secret_refs, + credential_env=credential_env, + auth_volume=auth_vol, + ) + self._state.write(auth_vol, proxy_image, domains) + swap.commit() + except Exception as primary: + rollback_failures: list[str] = [] + if swap is not None: + try: + self._state.restore(auth_vol, proxy_image, previous_domains) + except Exception as exc: + rollback_failures.append(f"state restore failed: {exc}") + try: + swap.rollback() + except Exception as exc: + rollback_failures.append(f"proxy restore failed: {exc}") + self._credentials.rollback_update(prepared) + if rollback_failures: + raise ProxyStartError( + f"Proxy update failed: {primary}; " + "; ".join(rollback_failures) + ) from primary + raise + + self._credentials.commit_update(prepared) # Verify CA cert survived the recreate (same named volume = same cert). # If the cert is missing or changed, redistribute to the agent. diff --git a/src/paude/backends/podman/proxy_credentials.py b/src/paude/backends/podman/proxy_credentials.py index 9ac490d..3f4856e 100644 --- a/src/paude/backends/podman/proxy_credentials.py +++ b/src/paude/backends/podman/proxy_credentials.py @@ -2,21 +2,35 @@ from __future__ import annotations +import secrets from collections.abc import Mapping +from dataclasses import dataclass, field from paude.backends.podman.helpers import proxy_secret_name, proxy_secret_prefix from paude.backends.proxy_config import ( PROXY_CHATGPT_AUTH_STATE_ENV, ProxyCredentials, ) +from paude.container.proxy_inspect import ProxyInspectionError, ProxyInspector from paude.container.runner import ContainerRunner +@dataclass +class PreparedProxyCredentials: + """Credential arguments plus the resources needed for commit or rollback.""" + + credentials: ProxyCredentials + secret_refs: list[str] = field(default_factory=list) + staged_secrets: list[str] = field(default_factory=list) + superseded_secrets: list[str] = field(default_factory=list) + + class ProxyCredentialManager: """Manages podman secrets for proxy credential injection.""" def __init__(self, runner: ContainerRunner) -> None: self._runner = runner + self._inspector = ProxyInspector(runner) def create_secrets( self, @@ -66,3 +80,112 @@ def remove_secrets(self, session_name: str) -> None: names = self._runner.list_secrets_by_prefix(proxy_secret_prefix(session_name)) for sname in names: self._runner.remove_secret(sname) + + def prepare_update( + self, + session_name: str, + proxy_name: str, + refresh: ProxyCredentials, + credential_targets: set[str], + required_targets: set[str], + ) -> PreparedProxyCredentials: + """Preserve current bindings and overlay only explicit refresh values.""" + targets = credential_targets | set(refresh.environment) + if self._runner.engine.supports_secrets: + return self._prepare_podman_update( + session_name, proxy_name, refresh, required_targets + ) + + environment = self._inspector.environment(proxy_name) + preserved = {key: environment[key] for key in targets if key in environment} + preserved.update(refresh.environment) + self._require_targets(set(preserved), required_targets) + return PreparedProxyCredentials( + credentials=ProxyCredentials( + environment=preserved, + chatgpt_oauth_mode=refresh.chatgpt_oauth_mode, + ) + ) + + def _prepare_podman_update( + self, + session_name: str, + proxy_name: str, + refresh: ProxyCredentials, + required_targets: set[str], + ) -> PreparedProxyCredentials: + refs = self._inspector.secret_refs(proxy_name) + refs_by_target: dict[str, str] = {} + names_by_target: dict[str, str] = {} + for ref in refs: + target = self._secret_target(ref) + if target is None: + continue + if target in refs_by_target: + raise ProxyInspectionError( + f"Proxy '{proxy_name}' has duplicate credential target {target}." + ) + refs_by_target[target] = ref + names_by_target[target] = ref.split(",", 1)[0] + + self._require_targets( + set(refs_by_target) | set(refresh.environment), required_targets + ) + + staged: list[str] = [] + superseded: list[str] = [] + candidate_refs = [ + ref for ref in refs if self._secret_target(ref) not in refresh.environment + ] + try: + for target, value in refresh.environment.items(): + secret_name = ( + f"{proxy_secret_name(session_name, target)}-" + f"update-{secrets.token_hex(4)}" + ) + self._runner.create_secret_from_value(secret_name, value) + staged.append(secret_name) + candidate_refs.append(f"{secret_name},type=env,target={target}") + previous = names_by_target.get(target) + if previous and previous.startswith(proxy_secret_prefix(session_name)): + superseded.append(previous) + except Exception: + for secret_name in staged: + self._runner.remove_secret(secret_name) + raise + + return PreparedProxyCredentials( + credentials=ProxyCredentials(chatgpt_oauth_mode=refresh.chatgpt_oauth_mode), + secret_refs=candidate_refs, + staged_secrets=staged, + superseded_secrets=superseded, + ) + + def rollback_update(self, prepared: PreparedProxyCredentials) -> None: + """Remove generation secrets created for an update that did not commit.""" + for secret_name in prepared.staged_secrets: + self._runner.remove_secret(secret_name) + + def commit_update(self, prepared: PreparedProxyCredentials) -> None: + """Remove only the old bindings superseded by an explicit refresh.""" + for secret_name in prepared.superseded_secrets: + self._runner.remove_secret(secret_name) + + @staticmethod + def _secret_target(ref: str) -> str | None: + for part in ref.split(","): + if part.startswith("target="): + target = part.partition("=")[2] + return target or None + return None + + @staticmethod + def _require_targets(present: set[str], required: set[str]) -> None: + missing = sorted(required - present) + if missing: + names = ", ".join(missing) + raise ValueError( + "Cannot update proxy domains because required credential " + f"bindings are missing: {names}. Supply them and use " + "--refresh-credentials." + ) diff --git a/src/paude/backends/podman/proxy_state.py b/src/paude/backends/podman/proxy_state.py new file mode 100644 index 0000000..6891ccf --- /dev/null +++ b/src/paude/backends/podman/proxy_state.py @@ -0,0 +1,114 @@ +"""Durable mutable proxy configuration stored in the session auth volume.""" + +from __future__ import annotations + +import json + +from paude.container.runner import ContainerRunner + +_STATE_PATH = "/data/auth/allowed-domains.json" +_STATE_SCHEMA = "allowed-domains.v1" +_MISSING_EXIT = 3 + + +class ProxyStateError(RuntimeError): + """Durable proxy state could not be read or written safely.""" + + +class ProxyStateStore: + """Read and atomically write non-secret proxy state through the engine.""" + + def __init__(self, runner: ContainerRunner) -> None: + self._runner = runner + + def read(self, volume: str, image: str) -> list[str] | None: + """Return committed domains, or ``None`` for a legacy absent record.""" + result = self._runner.engine.run( + "run", + "--rm", + "-v", + f"{volume}:/data/auth:ro", + "--entrypoint", + "sh", + image, + "-c", + f"test -e {_STATE_PATH} || exit {_MISSING_EXIT}; cat {_STATE_PATH}", + check=False, + ) + if result.returncode == _MISSING_EXIT: + return None + if result.returncode != 0: + raise ProxyStateError( + "Could not read durable allowed-domain state: " + f"{result.stderr.strip() or 'container helper failed'}" + ) + try: + record = json.loads(result.stdout) + except (json.JSONDecodeError, TypeError) as exc: + raise ProxyStateError("Durable allowed-domain state is corrupt.") from exc + if ( + not isinstance(record, dict) + or record.get("schema") != _STATE_SCHEMA + or not isinstance(record.get("domains"), list) + or not all(isinstance(item, str) for item in record["domains"]) + ): + raise ProxyStateError("Durable allowed-domain state is corrupt.") + return list(record["domains"]) + + def write(self, volume: str, image: str, domains: list[str]) -> None: + """Atomically commit a versioned allowed-domain record.""" + payload = json.dumps( + {"schema": _STATE_SCHEMA, "domains": domains}, separators=(",", ":") + ) + script = ( + "umask 077; " + f"tmp={_STATE_PATH}.tmp.$$; " + f'cat > "$tmp" && mv -f "$tmp" {_STATE_PATH}' + ) + result = self._runner.engine.run( + "run", + "--rm", + "-i", + "-v", + f"{volume}:/data/auth", + "--entrypoint", + "sh", + image, + "-c", + script, + check=False, + input=payload, + ) + if result.returncode != 0: + raise ProxyStateError( + "Could not commit durable allowed-domain state: " + f"{result.stderr.strip() or 'container helper failed'}" + ) + + def restore( + self, + volume: str, + image: str, + previous: list[str] | None, + ) -> None: + """Restore the record captured before a failed proxy transaction.""" + if previous is not None: + self.write(volume, image, previous) + return + result = self._runner.engine.run( + "run", + "--rm", + "-v", + f"{volume}:/data/auth", + "--entrypoint", + "sh", + image, + "-c", + f"rm -f {_STATE_PATH}", + check=False, + ) + if result.returncode != 0: + raise ProxyStateError( + "Could not restore durable allowed-domain state: " + f"{result.stderr.strip() or 'container helper failed'}" + ) diff --git a/src/paude/backends/podman/resources.py b/src/paude/backends/podman/resources.py index c60bca2..080eab9 100644 --- a/src/paude/backends/podman/resources.py +++ b/src/paude/backends/podman/resources.py @@ -30,6 +30,7 @@ from __future__ import annotations import sys +from dataclasses import replace from typing import TYPE_CHECKING from paude.backends.labels import LabeledSession, read_labels @@ -90,7 +91,11 @@ def labels(self, name: str) -> LabeledSession | None: container = find_container_by_session_name(self._runner, name) if container is None: return None - return read_labels(container.get("Labels", {}) or {}) + view = read_labels(container.get("Labels", {}) or {}) + domains = self._proxy.read_domain_state(name, view.spec.proxy_image) + if domains is None: + return view + return replace(view, spec=replace(view.spec, allowed_domains=domains)) # -- rebuild ---------------------------------------------------------- diff --git a/src/paude/backends/proxy_config.py b/src/paude/backends/proxy_config.py index 7d97b91..e521d6a 100644 --- a/src/paude/backends/proxy_config.py +++ b/src/paude/backends/proxy_config.py @@ -170,3 +170,56 @@ def gather_proxy_credentials( chatgpt_oauth_mode = "chatgpt" in effective_providers return ProxyCredentials(environment=creds, chatgpt_oauth_mode=chatgpt_oauth_mode) + + +def proxy_credential_targets( + agent_config: AgentConfig | AgentComposition | Agent, +) -> set[str]: + """Return every environment credential paude may bind to a proxy. + + Domain-only updates use this allow-list when reading Docker's inspected + environment. Keeping the list explicit prevents ordinary proxy settings + from being replayed as credentials while still preserving credentials for + providers other than the session's primary provider. + """ + from paude.providers import get_provider, list_providers + + targets = {"GH_TOKEN", PROXY_GCP_ADC_ENV} + targets.update( + key + for provider_name in list_providers() + for key in get_provider(provider_name).secret_env_vars + ) + targets.update( + key for config in _agent_configs(agent_config) for key in config.secret_env_vars + ) + return targets + + +def required_proxy_credential_targets( + agent_config: AgentConfig | AgentComposition | Agent, + credential_providers: list[str], +) -> set[str]: + """Return credential targets required by the session's selected providers.""" + from paude.providers import get_provider + + required = { + key + for provider_name in credential_providers + for key in get_provider(provider_name).secret_env_vars + } + required.update( + key for config in _agent_configs(agent_config) for key in config.secret_env_vars + ) + return required + + +def _agent_configs( + agent_config: AgentConfig | AgentComposition | Agent, +) -> list[AgentConfig]: + """Normalize an agent, composition, or config to its configs.""" + if hasattr(agent_config, "agents"): + return [agent.config for agent in agent_config.agents] + if hasattr(agent_config, "config"): + return [agent_config.config] + return [agent_config] diff --git a/src/paude/cli/domains.py b/src/paude/cli/domains.py index e67491e..b512a85 100644 --- a/src/paude/cli/domains.py +++ b/src/paude/cli/domains.py @@ -101,7 +101,13 @@ def _list_domains(backend_obj: Backend, name: str) -> None: typer.echo(f" {domain}") -def _add_domains(backend_obj: Backend, name: str, add: list[str]) -> None: +def _add_domains( + backend_obj: Backend, + name: str, + add: list[str], + *, + refresh_credentials: bool = False, +) -> None: """Add domains to the current allowed list. Args: @@ -127,12 +133,20 @@ def _add_domains(backend_obj: Backend, name: str, add: list[str]) -> None: merged.append(d) seen.add(d) - backend_obj.update_allowed_domains(name, merged) + backend_obj.update_allowed_domains( + name, merged, refresh_credentials=refresh_credentials + ) added_count = len(merged) - len(current) typer.echo(f"Added {added_count} domain(s) to session '{name}'.") -def _remove_domains(backend_obj: Backend, name: str, remove: list[str]) -> None: +def _remove_domains( + backend_obj: Backend, + name: str, + remove: list[str], + *, + refresh_credentials: bool = False, +) -> None: """Remove domains from the current allowed list. Args: @@ -160,12 +174,20 @@ def _remove_domains(backend_obj: Backend, name: str, remove: list[str]) -> None: ) raise typer.Exit(1) - backend_obj.update_allowed_domains(name, remaining) + backend_obj.update_allowed_domains( + name, remaining, refresh_credentials=refresh_credentials + ) removed_count = len(current) - len(remaining) typer.echo(f"Removed {removed_count} domain(s) from session '{name}'.") -def _replace_domains(backend_obj: Backend, name: str, replace: list[str]) -> None: +def _replace_domains( + backend_obj: Backend, + name: str, + replace: list[str], + *, + refresh_credentials: bool = False, +) -> None: """Replace all domains for a session. Args: @@ -174,7 +196,9 @@ def _replace_domains(backend_obj: Backend, name: str, replace: list[str]) -> Non replace: New domain list. """ expanded = _expand_domains_or_exit(replace) - backend_obj.update_allowed_domains(name, expanded) + backend_obj.update_allowed_domains( + name, expanded, refresh_credentials=refresh_credentials + ) typer.echo(f"Replaced domains for session '{name}' ({len(expanded)} domain(s)).") @@ -200,19 +224,50 @@ def allowed_domains_cmd( help="Container backend (auto-detected from session if not specified).", ), ] = None, + refresh_credentials: Annotated[ + bool, + typer.Option( + "--refresh-credentials", + help=( + "Replace proxy credentials supplied by the current host " + "environment; preserve all other bindings." + ), + ), + ] = False, ) -> None: """Manage allowed egress domains for a session.""" _check_domains_mutual_exclusivity(add, remove, replace) + if refresh_credentials and not any((add, remove, replace)): + typer.echo( + "Error: --refresh-credentials requires --add, --remove, or --replace.", + err=True, + ) + raise typer.Exit(1) backend_obj = _resolve_backend_for_domains(name, backend) try: if add: - _add_domains(backend_obj, name, add) + _add_domains( + backend_obj, + name, + add, + refresh_credentials=refresh_credentials, + ) elif remove: - _remove_domains(backend_obj, name, remove) + _remove_domains( + backend_obj, + name, + remove, + refresh_credentials=refresh_credentials, + ) elif replace: - _replace_domains(backend_obj, name, replace) + _replace_domains( + backend_obj, + name, + replace, + refresh_credentials=refresh_credentials, + ) else: _list_domains(backend_obj, name) except NotImplementedError as e: diff --git a/src/paude/cli/help.py b/src/paude/cli/help.py index a8ffffd..114312c 100644 --- a/src/paude/cli/help.py +++ b/src/paude/cli/help.py @@ -139,9 +139,20 @@ class HelpSection: "paude allowed-domains NAME --replace default .example.com", "Replace entire list", ), + ( + "paude allowed-domains NAME --add .example.com --refresh-credentials", + "Update domains and refresh supplied credentials", + ), ("paude blocked-domains NAME", "Show blocked domains"), ("paude blocked-domains NAME --raw", "Show raw proxy log"), ), + text=( + "Domain-only updates preserve the proxy's current credential bindings" + " and durable policy. Replacement is fail-closed: the old proxy is" + " restored if the candidate cannot start or commit. Use" + " --refresh-credentials only when fresh replacement values are present" + " in the current environment; unrelated bindings remain attached." + ), ), HelpSection( title="Examples", diff --git a/src/paude/container/proxy_inspect.py b/src/paude/container/proxy_inspect.py new file mode 100644 index 0000000..6b9888f --- /dev/null +++ b/src/paude/container/proxy_inspect.py @@ -0,0 +1,94 @@ +"""Strict inspection of the credential state attached to a proxy container.""" + +from __future__ import annotations + +import json + +from paude.container.runner import ContainerRunner + + +class ProxyInspectionError(RuntimeError): + """A proxy inspection failed or returned malformed data.""" + + +class ProxyInspector: + """Read proxy state without collapsing failures into absent values.""" + + def __init__(self, runner: ContainerRunner) -> None: + self._runner = runner + + def secret_refs(self, name: str) -> list[str]: + """Return the exact ``--secret`` refs used to create a Podman proxy.""" + result = self._runner.engine.run( + "inspect", "-f", "{{json .Config.CreateCommand}}", name, check=False + ) + if result.returncode != 0: + raise ProxyInspectionError( + f"Could not inspect credential bindings for proxy '{name}': " + f"{result.stderr.strip() or 'container inspect failed'}" + ) + try: + command = json.loads(result.stdout) + except (json.JSONDecodeError, TypeError) as exc: + raise ProxyInspectionError( + f"Proxy '{name}' has malformed create-command inspection data." + ) from exc + if not isinstance(command, list) or not all( + isinstance(arg, str) for arg in command + ): + raise ProxyInspectionError( + f"Proxy '{name}' has malformed create-command inspection data." + ) + + refs: list[str] = [] + index = 0 + while index < len(command): + arg = command[index] + if arg == "--secret": + if index + 1 >= len(command): + raise ProxyInspectionError( + f"Proxy '{name}' has a malformed --secret binding." + ) + refs.append(command[index + 1]) + index += 2 + continue + if arg.startswith("--secret="): + refs.append(arg.partition("=")[2]) + index += 1 + return refs + + def environment(self, name: str) -> dict[str, str]: + """Return a proxy's configured environment, failing on bad inspection.""" + result = self._runner.engine.run( + "inspect", "-f", "{{json .Config.Env}}", name, check=False + ) + if result.returncode != 0: + raise ProxyInspectionError( + f"Could not inspect credential bindings for proxy '{name}': " + f"{result.stderr.strip() or 'container inspect failed'}" + ) + try: + entries = json.loads(result.stdout) + except (json.JSONDecodeError, TypeError) as exc: + raise ProxyInspectionError( + f"Proxy '{name}' has malformed environment inspection data." + ) from exc + if not isinstance(entries, list) or not all( + isinstance(entry, str) and "=" in entry for entry in entries + ): + raise ProxyInspectionError( + f"Proxy '{name}' has malformed environment inspection data." + ) + return dict(entry.split("=", 1) for entry in entries) + + def running(self, name: str) -> bool: + """Return whether the proxy is running, failing if state is unreadable.""" + result = self._runner.engine.run( + "inspect", "-f", "{{.State.Running}}", name, check=False + ) + state = result.stdout.strip() + if result.returncode != 0 or state not in {"true", "false"}: + raise ProxyInspectionError( + f"Could not inspect running state for proxy '{name}'." + ) + return state == "true" diff --git a/src/paude/container/proxy_runner.py b/src/paude/container/proxy_runner.py index 6ac62b5..d503641 100644 --- a/src/paude/container/proxy_runner.py +++ b/src/paude/container/proxy_runner.py @@ -4,6 +4,8 @@ import time from collections.abc import Mapping +from dataclasses import dataclass +from secrets import token_hex from paude.container.engine import ContainerEngine from paude.container.runner import ContainerRunner @@ -15,6 +17,61 @@ class ProxyStartError(Exception): pass +@dataclass +class ProxySwap: + """A started candidate proxy whose retained predecessor can be restored.""" + + runner: ContainerRunner + name: str + backup_name: str + network: str + ip: str | None + old_running: bool + old_renamed: bool = False + old_disconnected: bool = False + candidate_created: bool = False + + def commit(self) -> None: + """Remove the retained predecessor after all update state is durable.""" + self._run_checked("remove retained proxy", "rm", "-f", self.backup_name) + + def rollback(self) -> None: + """Remove the candidate and restore the predecessor's identity/state.""" + failures: list[str] = [] + if self.candidate_created: + self._try(failures, "remove candidate proxy", "rm", "-f", self.name) + if self.old_disconnected: + args = ["network", "connect"] + if self.ip: + args.extend(["--ip", self.ip]) + args.extend([self.network, self.backup_name]) + self._try(failures, "reconnect retained proxy", *args) + if self.old_renamed: + self._try( + failures, + "restore retained proxy name", + "rename", + self.backup_name, + self.name, + ) + if self.old_running: + self._try(failures, "restart retained proxy", "start", self.name) + if failures: + raise ProxyStartError("; ".join(failures)) + + def _try(self, failures: list[str], operation: str, *args: str) -> None: + try: + self._run_checked(operation, *args) + except ProxyStartError as exc: + failures.append(str(exc)) + + def _run_checked(self, operation: str, *args: str) -> None: + result = self.runner.engine.run(*args, check=False) + if result.returncode != 0: + detail = result.stderr.strip() or "container engine command failed" + raise ProxyStartError(f"Failed to {operation}: {detail}") + + class ProxyRunner: """Proxy container lifecycle operations. @@ -52,7 +109,14 @@ def _connect_bridge_if_needed(self, container_name: str) -> None: if self._engine.supports_multi_network_create: return bridge = self._engine.default_bridge_network - self._engine.run("network", "connect", bridge, container_name, check=False) + result = self._engine.run( + "network", "connect", bridge, container_name, check=False + ) + if result.returncode != 0: + raise ProxyStartError( + "Failed to connect proxy to the bridge network: " + f"{result.stderr.strip() or 'container engine command failed'}" + ) def _build_env_args( self, @@ -172,7 +236,11 @@ def create_session_proxy( if result.returncode != 0: raise ProxyStartError(f"Failed to create proxy: {result.stderr}") - self._connect_bridge_if_needed(name) + try: + self._connect_bridge_if_needed(name) + except Exception: + self._engine.run("rm", "-f", name, check=False) + raise return name def start_session_proxy(self, name: str) -> None: @@ -228,3 +296,73 @@ def recreate_session_proxy( self.start_session_proxy(name) return name + + def swap_session_proxy( + self, + name: str, + image: str, + network: str, + dns: str | None = None, + allowed_domains: list[str] | None = None, + ip: str | None = None, + otel_ports: list[int] | None = None, + ca_volume: str | None = None, + credentials: Mapping[str, str] | None = None, + allowed_clients: str | None = None, + secret_refs: list[str] | None = None, + credential_env: Mapping[str, str] | None = None, + auth_volume: str | None = None, + ) -> ProxySwap: + """Start a replacement while retaining the old proxy for rollback.""" + swap = ProxySwap( + runner=self._runner, + name=name, + backup_name=f"{name}-rollback-{token_hex(4)}", + network=network, + ip=ip, + old_running=self._runner.container_running(name), + ) + try: + if swap.old_running: + swap._run_checked("stop current proxy", "stop", "-t", "1", name) + swap._run_checked( + "retain current proxy", + "rename", + name, + swap.backup_name, + ) + swap.old_renamed = True + swap._run_checked( + "release current proxy address", + "network", + "disconnect", + network, + swap.backup_name, + ) + swap.old_disconnected = True + self.create_session_proxy( + name=name, + image=image, + network=network, + dns=dns, + allowed_domains=allowed_domains, + ip=ip, + otel_ports=otel_ports, + ca_volume=ca_volume, + credentials=credentials, + allowed_clients=allowed_clients, + secret_refs=secret_refs, + credential_env=credential_env, + auth_volume=auth_volume, + ) + swap.candidate_created = True + self.start_session_proxy(name) + except Exception as primary: + try: + swap.rollback() + except Exception as rollback: + raise ProxyStartError( + f"Proxy replacement failed: {primary}; rollback failed: {rollback}" + ) from primary + raise + return swap diff --git a/tests/test_allow_domain.py b/tests/test_allow_domain.py index c1bf36f..1e14e38 100644 --- a/tests/test_allow_domain.py +++ b/tests/test_allow_domain.py @@ -2,6 +2,7 @@ from __future__ import annotations +import json from unittest.mock import MagicMock, patch import pytest @@ -86,9 +87,17 @@ def test_recreates_proxy_with_new_domains(self, mock_dns: MagicMock) -> None: mock_runner.engine.binary = "podman" mock_runner.engine.supports_multi_network_create = True mock_runner.engine.default_bridge_network = "podman" - mock_runner.engine.run.return_value = MagicMock( - returncode=0, stdout="", stderr="" - ) + + def run(*args: str, **_kwargs: object) -> MagicMock: + if args[:3] == ("inspect", "-f", "{{json .Config.CreateCommand}}"): + return MagicMock( + returncode=0, stdout=json.dumps(["podman", "create"]), stderr="" + ) + if args and args[0] == "run" and any("test -e" in arg for arg in args): + return MagicMock(returncode=3, stdout="", stderr="") + return MagicMock(returncode=0, stdout="", stderr="") + + mock_runner.engine.run.side_effect = run # Both main and proxy containers exist mock_runner.container_exists.return_value = True mock_runner.get_container_image.return_value = "proxy:latest" @@ -133,3 +142,61 @@ def test_raises_value_error_when_no_proxy(self) -> None: with pytest.raises(ValueError, match="no proxy"): backend.update_allowed_domains("my-session", [".example.com"]) + + @patch( + "paude.backends.podman.helpers.get_session_credential_providers", + return_value=["anthropic-oauth"], + ) + @patch("paude.backends.podman.backend.get_session_composition") + def test_default_update_never_gathers_ambient_credentials( + self, + mock_composition: MagicMock, + mock_providers: MagicMock, + ) -> None: + runner = MagicMock() + runner.container_exists.return_value = True + backend = make_backend(runner) + backend._proxy = MagicMock() + backend._setup.gather_proxy_credentials = MagicMock() # type: ignore[method-assign] + composition = MagicMock() + composition.agents = [] + mock_composition.return_value = composition + + backend.update_allowed_domains("my-session", [".example.com"]) + + backend._setup.gather_proxy_credentials.assert_not_called() + credentials = backend._proxy.update_domains.call_args.kwargs["credentials"] + assert credentials.environment == {} + + @patch( + "paude.backends.podman.helpers.get_session_credential_providers", + return_value=["anthropic-oauth"], + ) + @patch("paude.backends.podman.backend.get_session_composition") + def test_explicit_refresh_gathers_a_host_overlay( + self, + mock_composition: MagicMock, + mock_providers: MagicMock, + ) -> None: + from paude.backends.proxy_config import ProxyCredentials + + runner = MagicMock() + runner.container_exists.return_value = True + backend = make_backend(runner) + backend._proxy = MagicMock() + composition = MagicMock() + composition.agents = [] + mock_composition.return_value = composition + fresh = ProxyCredentials(environment={"CLAUDE_CODE_OAUTH_TOKEN": "fresh"}) + backend._setup.gather_proxy_credentials = MagicMock( # type: ignore[method-assign] + return_value=fresh + ) + + backend.update_allowed_domains( + "my-session", [".example.com"], refresh_credentials=True + ) + + backend._setup.gather_proxy_credentials.assert_called_once_with( + composition, ["anthropic-oauth"] + ) + assert backend._proxy.update_domains.call_args.kwargs["credentials"] is fresh diff --git a/tests/test_cli.py b/tests/test_cli.py index 6042e0c..6b0fd84 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -1812,10 +1812,78 @@ def test_help_shows_cp_command(self): # --------------------------------------------------------------------------- -# blocked-domains subcommand +# allowed-domains / blocked-domains subcommands # --------------------------------------------------------------------------- +class TestAllowedDomainsCLI: + """Tests for credential-safe allowed-domain mutation options.""" + + @patch("paude.cli.domains._resolve_backend_for_domains") + def test_refresh_credentials_is_explicitly_forwarded( + self, mock_resolve: MagicMock + ) -> None: + backend = MagicMock() + backend.get_allowed_domains.return_value = [".pypi.org"] + mock_resolve.return_value = backend + + result = runner.invoke( + app, + [ + "allowed-domains", + "my-session", + "--add", + ".example.com", + "--refresh-credentials", + ], + ) + + assert result.exit_code == 0 + backend.update_allowed_domains.assert_called_once_with( + "my-session", + [".pypi.org", ".example.com"], + refresh_credentials=True, + ) + + @patch("paude.cli.domains._resolve_backend_for_domains") + def test_domain_mutation_preserves_credentials_by_default( + self, mock_resolve: MagicMock + ) -> None: + backend = MagicMock() + backend.get_allowed_domains.return_value = [".pypi.org"] + mock_resolve.return_value = backend + + result = runner.invoke( + app, ["allowed-domains", "my-session", "--add", ".example.com"] + ) + + assert result.exit_code == 0 + backend.update_allowed_domains.assert_called_once_with( + "my-session", + [".pypi.org", ".example.com"], + refresh_credentials=False, + ) + + @patch("paude.cli.domains._resolve_backend_for_domains") + def test_refresh_credentials_requires_a_mutation( + self, mock_resolve: MagicMock + ) -> None: + result = runner.invoke( + app, ["allowed-domains", "my-session", "--refresh-credentials"] + ) + + assert result.exit_code == 1 + output = result.stdout + (result.stderr or "") + assert "requires --add, --remove, or --replace" in output + mock_resolve.assert_not_called() + + def test_help_documents_refresh_credentials(self) -> None: + result = runner.invoke(app, ["allowed-domains", "--help"]) + + assert result.exit_code == 0 + assert "--refresh-credentials" in result.stdout + + class TestBlockedDomainsCLI: """Tests for the blocked-domains CLI subcommand.""" diff --git a/tests/test_podman_proxy.py b/tests/test_podman_proxy.py index 4bf0fda..3c03bf1 100644 --- a/tests/test_podman_proxy.py +++ b/tests/test_podman_proxy.py @@ -2,6 +2,7 @@ from __future__ import annotations +import json from unittest.mock import MagicMock, patch import pytest @@ -14,7 +15,8 @@ auth_volume_name, ca_volume_name, ) -from paude.backends.proxy_config import derive_agent_ip +from paude.backends.podman.proxy_credentials import PreparedProxyCredentials +from paude.backends.proxy_config import ProxyCredentials, derive_agent_ip from paude.container.proxy_runner import ProxyStartError @@ -29,7 +31,15 @@ def _make_mock_runner(engine_binary: str = "podman") -> MagicMock: mock_runner.engine.default_bridge_network = ( "podman" if engine_binary == "podman" else "bridge" ) - mock_runner.engine.run.return_value = MagicMock(returncode=0, stdout="", stderr="") + + def run(*args: str, **_kwargs: object) -> MagicMock: + if args[:3] == ("inspect", "-f", "{{json .Config.CreateCommand}}"): + return MagicMock(returncode=0, stdout=json.dumps([engine_binary, "create"])) + if args and args[0] == "run" and any("test -e" in arg for arg in args): + return MagicMock(returncode=3, stdout="", stderr="") + return MagicMock(returncode=0, stdout="", stderr="") + + mock_runner.engine.run.side_effect = run return mock_runner @@ -895,10 +905,14 @@ def test_update_domains_passes_credentials_as_secrets( credentials={"ANTHROPIC_API_KEY": "sk-real-key"}, ) - # Secrets should have been created - mock_runner.create_secret_from_value.assert_called_once_with( - "paude-proxy-cred-test-session-anthropic-api-key", "sk-real-key" + # Refresh uses a generation-specific secret, leaving the old binding + # available until the proxy transaction commits. + mock_runner.create_secret_from_value.assert_called_once() + staged_name, staged_value = mock_runner.create_secret_from_value.call_args.args + assert staged_name.startswith( + "paude-proxy-cred-test-session-anthropic-api-key-update-" ) + assert staged_value == "sk-real-key" # Check --secret in the recreate/create call engine_calls = mock_runner.engine.run.call_args_list @@ -907,10 +921,7 @@ def test_update_domains_passes_credentials_as_secrets( call_args = create_call[0][0] secret_indices = [i for i, a in enumerate(call_args) if a == "--secret"] secret_vals = [call_args[i + 1] for i in secret_indices] - assert ( - "paude-proxy-cred-test-session-anthropic-api-key," - "type=env,target=ANTHROPIC_API_KEY" in secret_vals - ) + assert f"{staged_name},type=env,target=ANTHROPIC_API_KEY" in secret_vals class TestUpdateDomainsCaResilience: @@ -1071,6 +1082,76 @@ def test_update_domains_warns_when_ca_cert_missing_after_recreate( mock_runner.inject_file.assert_not_called() +class TestUpdateDomainTransaction: + """Durable state and retained-proxy swap commit or roll back together.""" + + @patch("paude.backends.podman.proxy._get_host_dns", return_value=None) + def test_state_write_failure_restores_proxy_state_and_staged_secrets( + self, mock_dns: MagicMock + ) -> None: + runner = _make_mock_runner() + runner.container_exists.return_value = True + runner.get_container_image.return_value = "proxy:latest" + network = MagicMock() + network.get_network_gateway.return_value = "10.89.0.1" + manager = PodmanProxyManager(runner, network) + manager.get_config_from_labels = MagicMock( # type: ignore[method-assign] + return_value=("proxy:latest", [".old.example"], []) + ) + prepared = PreparedProxyCredentials(credentials=ProxyCredentials()) + manager._credentials = MagicMock() + manager._credentials.prepare_update.return_value = prepared + manager._credentials.credential_env.return_value = {} + manager._state = MagicMock() + manager._state.read.return_value = [".old.example"] + manager._state.write.side_effect = RuntimeError("write failed") + swap = MagicMock() + manager._proxy_runner = MagicMock() + manager._proxy_runner.swap_session_proxy.return_value = swap + + with pytest.raises(RuntimeError, match="write failed"): + manager.update_domains("test-session", [".new.example"]) + + manager._state.restore.assert_called_once_with( + "paude-auth-test-session", "proxy:latest", [".old.example"] + ) + swap.rollback.assert_called_once_with() + manager._credentials.rollback_update.assert_called_once_with(prepared) + swap.commit.assert_not_called() + + @patch("paude.backends.podman.proxy._get_host_dns", return_value=None) + def test_success_commits_state_before_removing_retained_proxy( + self, mock_dns: MagicMock + ) -> None: + runner = _make_mock_runner() + runner.container_exists.return_value = True + runner.get_container_image.return_value = "proxy:latest" + network = MagicMock() + network.get_network_gateway.return_value = "10.89.0.1" + manager = PodmanProxyManager(runner, network) + manager.get_config_from_labels = MagicMock( # type: ignore[method-assign] + return_value=("proxy:latest", [".old.example"], []) + ) + prepared = PreparedProxyCredentials(credentials=ProxyCredentials()) + manager._credentials = MagicMock() + manager._credentials.prepare_update.return_value = prepared + manager._credentials.credential_env.return_value = {} + manager._state = MagicMock() + manager._state.read.return_value = [".old.example"] + swap = MagicMock() + manager._proxy_runner = MagicMock() + manager._proxy_runner.swap_session_proxy.return_value = swap + events = MagicMock() + events.attach_mock(manager._state.write, "write") + events.attach_mock(swap.commit, "commit") + + manager.update_domains("test-session", [".new.example"]) + + assert [call[0] for call in events.mock_calls] == ["write", "commit"] + manager._credentials.commit_update.assert_called_once_with(prepared) + swap.rollback.assert_not_called() + + class TestSourceIpFiltering: """Tests for source IP filtering (allowed_clients) in proxy creation.""" diff --git a/tests/test_podman_session.py b/tests/test_podman_session.py index b3ec9a2..4b5ab05 100644 --- a/tests/test_podman_session.py +++ b/tests/test_podman_session.py @@ -1532,6 +1532,9 @@ def test_start_session_recreates_missing_proxy( mock_network = MagicMock() backend = make_backend(mock_runner, mock_network) + backend._proxy.read_domain_state = MagicMock( # type: ignore[method-assign] + return_value=None + ) backend.start_session("my-session") # Proxy should be recreated via engine.run (create + start) @@ -1605,6 +1608,9 @@ def container_exists(name: str) -> bool: mock_network = MagicMock() backend = make_backend(mock_runner, mock_network) + backend._proxy.read_domain_state = MagicMock( # type: ignore[method-assign] + return_value=None + ) backend.connect_session("my-session") # Proxy should be recreated via engine.run (create + start) diff --git a/tests/test_proxy_secrets.py b/tests/test_proxy_secrets.py index 1df1bca..fe5bfb4 100644 --- a/tests/test_proxy_secrets.py +++ b/tests/test_proxy_secrets.py @@ -2,15 +2,20 @@ from __future__ import annotations +import json +import subprocess from unittest.mock import MagicMock, patch import pytest from paude.backends.podman.helpers import proxy_secret_name, proxy_secret_prefix +from paude.backends.podman.proxy_credentials import ProxyCredentialManager from paude.backends.proxy_config import ProxyCredentials from paude.container.engine import ContainerEngine -from paude.container.proxy_runner import ProxyRunner +from paude.container.proxy_inspect import ProxyInspectionError +from paude.container.proxy_runner import ProxyRunner, ProxyStartError from paude.container.runner import ContainerRunner +from tests.fakes import FakeTransport, make_engine, recorded_commands class TestProxySecretName: @@ -335,3 +340,224 @@ def test_remove_credential_secrets(self) -> None: assert runner.remove_secret.call_count == 2 runner.remove_secret.assert_any_call("paude-proxy-cred-sess-api-key") runner.remove_secret.assert_any_call("paude-proxy-cred-sess-gh-token") + + +def _result( + returncode: int = 0, stdout: str = "", stderr: str = "" +) -> subprocess.CompletedProcess[str]: + return subprocess.CompletedProcess([], returncode, stdout, stderr) + + +class TestCredentialPreservingUpdates: + """Credential update preparation reads the live binding, not ambient state.""" + + def test_podman_preserves_exact_attached_refs_without_rewriting(self) -> None: + command = [ + "podman", + "create", + "--secret", + "paude-proxy-cred-sess-claude-code-oauth-token," + "type=env,target=CLAUDE_CODE_OAUTH_TOKEN", + "--secret", + "old-gh,type=env,target=GH_TOKEN", + ] + transport = FakeTransport( + results={"inspect -f": _result(stdout=json.dumps(command))} + ) + runner = ContainerRunner(make_engine("podman", transport=transport)) + + prepared = ProxyCredentialManager(runner).prepare_update( + "sess", + "paude-proxy-sess", + ProxyCredentials(), + {"CLAUDE_CODE_OAUTH_TOKEN", "GH_TOKEN"}, + {"CLAUDE_CODE_OAUTH_TOKEN"}, + ) + + assert prepared.secret_refs == [ + "paude-proxy-cred-sess-claude-code-oauth-token," + "type=env,target=CLAUDE_CODE_OAUTH_TOKEN", + "old-gh,type=env,target=GH_TOKEN", + ] + commands = recorded_commands(runner.engine) + assert not any( + command[1:3] in (["secret", "create"], ["secret", "rm"]) + for command in commands + ) + + def test_podman_refresh_stages_new_generation_and_retains_unrelated(self) -> None: + command = [ + "podman", + "create", + "--secret", + "paude-proxy-cred-sess-claude-code-oauth-token," + "type=env,target=CLAUDE_CODE_OAUTH_TOKEN", + "--secret", + "old-gh,type=env,target=GH_TOKEN", + ] + transport = FakeTransport( + results={"inspect -f": _result(stdout=json.dumps(command))} + ) + runner = ContainerRunner(make_engine("podman", transport=transport)) + manager = ProxyCredentialManager(runner) + + prepared = manager.prepare_update( + "sess", + "paude-proxy-sess", + ProxyCredentials(environment={"CLAUDE_CODE_OAUTH_TOKEN": "fresh"}), + {"CLAUDE_CODE_OAUTH_TOKEN", "GH_TOKEN"}, + {"CLAUDE_CODE_OAUTH_TOKEN"}, + ) + + assert "old-gh,type=env,target=GH_TOKEN" in prepared.secret_refs + assert ( + "paude-proxy-cred-sess-claude-code-oauth-token," + "type=env,target=CLAUDE_CODE_OAUTH_TOKEN" not in prepared.secret_refs + ) + assert len(prepared.staged_secrets) == 1 + assert any( + ref + == f"{prepared.staged_secrets[0]},type=env,target=CLAUDE_CODE_OAUTH_TOKEN" + for ref in prepared.secret_refs + ) + assert ( + "paude-proxy-cred-sess-claude-code-oauth-token" + in prepared.superseded_secrets + ) + manager.commit_update(prepared) + assert [ + "podman", + "secret", + "rm", + "paude-proxy-cred-sess-claude-code-oauth-token", + ] in recorded_commands(runner.engine) + + def test_missing_required_binding_fails_before_any_mutation(self) -> None: + transport = FakeTransport( + results={"inspect -f": _result(stdout=json.dumps(["podman", "create"]))} + ) + runner = ContainerRunner(make_engine("podman", transport=transport)) + + with pytest.raises(ValueError, match="CLAUDE_CODE_OAUTH_TOKEN"): + ProxyCredentialManager(runner).prepare_update( + "sess", + "paude-proxy-sess", + ProxyCredentials(), + {"CLAUDE_CODE_OAUTH_TOKEN"}, + {"CLAUDE_CODE_OAUTH_TOKEN"}, + ) + + assert len(recorded_commands(runner.engine)) == 1 + + def test_inspect_failure_is_not_treated_as_no_credentials(self) -> None: + transport = FakeTransport( + results={"inspect -f": _result(returncode=1, stderr="inspect failed")} + ) + runner = ContainerRunner(make_engine("podman", transport=transport)) + + with pytest.raises(ProxyInspectionError, match="inspect failed"): + ProxyCredentialManager(runner).prepare_update( + "sess", + "paude-proxy-sess", + ProxyCredentials(), + {"CLAUDE_CODE_OAUTH_TOKEN"}, + {"CLAUDE_CODE_OAUTH_TOKEN"}, + ) + + assert len(recorded_commands(runner.engine)) == 1 + + def test_docker_preserves_current_values_and_overlays_only_refresh(self) -> None: + current = [ + "CLAUDE_CODE_OAUTH_TOKEN=old", + "GH_TOKEN=unrelated", + "ALLOWED_DOMAINS=.old.example", + ] + transport = FakeTransport( + results={"inspect -f": _result(stdout=json.dumps(current))} + ) + runner = ContainerRunner(make_engine("docker", transport=transport)) + + prepared = ProxyCredentialManager(runner).prepare_update( + "sess", + "paude-proxy-sess", + ProxyCredentials(environment={"CLAUDE_CODE_OAUTH_TOKEN": "fresh"}), + {"CLAUDE_CODE_OAUTH_TOKEN", "GH_TOKEN"}, + {"CLAUDE_CODE_OAUTH_TOKEN"}, + ) + + assert prepared.credentials.environment == { + "CLAUDE_CODE_OAUTH_TOKEN": "fresh", + "GH_TOKEN": "unrelated", + } + assert "ALLOWED_DOMAINS" not in prepared.credentials.environment + + +class TestRollbackProxySwap: + """A candidate failure restores the retained proxy and its fixed address.""" + + def test_start_failure_restores_old_name_network_and_running_state(self) -> None: + engine = MagicMock() + engine.supports_multi_network_create = True + engine.default_bridge_network = "podman" + starts = 0 + + def run(*args: str, **_kwargs: object) -> MagicMock: + nonlocal starts + if args == ("start", "paude-proxy-sess"): + starts += 1 + if starts == 1: + return MagicMock(returncode=1, stdout="", stderr="boom") + return MagicMock(returncode=0, stdout="", stderr="") + + engine.run.side_effect = run + runner = MagicMock(spec=ContainerRunner) + runner.engine = engine + runner.container_running.return_value = True + + with pytest.raises(ProxyStartError, match="Failed to start proxy"): + ProxyRunner(runner).swap_session_proxy( + name="paude-proxy-sess", + image="proxy:latest", + network="paude-net-sess", + ip="10.89.0.2", + ) + + commands = [call.args for call in engine.run.call_args_list] + rename_out = next(i for i, cmd in enumerate(commands) if cmd[0] == "rename") + disconnect = next( + i for i, cmd in enumerate(commands) if cmd[:2] == ("network", "disconnect") + ) + candidate_create = next( + i for i, cmd in enumerate(commands) if cmd[0] == "create" + ) + candidate_start = next( + i for i, cmd in enumerate(commands) if cmd == ("start", "paude-proxy-sess") + ) + candidate_remove = next( + i for i, cmd in enumerate(commands) if cmd[:2] == ("rm", "-f") + ) + reconnect = next( + i for i, cmd in enumerate(commands) if cmd[:2] == ("network", "connect") + ) + rename_back = max(i for i, cmd in enumerate(commands) if cmd[0] == "rename") + restart = max( + i for i, cmd in enumerate(commands) if cmd == ("start", "paude-proxy-sess") + ) + assert ( + rename_out + < disconnect + < candidate_create + < candidate_start + < candidate_remove + < reconnect + < rename_back + < restart + ) + assert commands[reconnect] == ( + "network", + "connect", + "--ip", + "10.89.0.2", + "paude-net-sess", + commands[rename_out][2], + ) diff --git a/tests/test_proxy_state.py b/tests/test_proxy_state.py new file mode 100644 index 0000000..8e04228 --- /dev/null +++ b/tests/test_proxy_state.py @@ -0,0 +1,70 @@ +"""Tests for durable allowed-domain proxy state.""" + +from __future__ import annotations + +import json +import subprocess +from unittest.mock import MagicMock + +import pytest + +from paude.backends.podman.proxy_state import ProxyStateError, ProxyStateStore +from paude.container.runner import ContainerRunner +from tests.fakes import FakeTransport, make_engine + + +def _result( + returncode: int = 0, stdout: str = "", stderr: str = "" +) -> subprocess.CompletedProcess[str]: + return subprocess.CompletedProcess([], returncode, stdout, stderr) + + +class TestProxyStateStore: + """The auth-volume record distinguishes absence, data, and failure.""" + + def test_absent_legacy_record_is_none(self) -> None: + runner = ContainerRunner( + make_engine(transport=FakeTransport(default_result=_result(returncode=3))) + ) + + assert ProxyStateStore(runner).read("auth", "proxy:latest") is None + + def test_reads_empty_list_as_committed_data(self) -> None: + payload = json.dumps({"schema": "allowed-domains.v1", "domains": []}) + runner = ContainerRunner( + make_engine(transport=FakeTransport(default_result=_result(stdout=payload))) + ) + + assert ProxyStateStore(runner).read("auth", "proxy:latest") == [] + + @pytest.mark.parametrize( + "result", + [ + _result(returncode=1, stderr="helper failed"), + _result(stdout="not-json"), + _result(stdout='{"schema":"wrong","domains":[]}'), + ], + ) + def test_read_failure_never_falls_back_to_stale_labels( + self, result: subprocess.CompletedProcess[str] + ) -> None: + runner = ContainerRunner( + make_engine(transport=FakeTransport(default_result=result)) + ) + + with pytest.raises(ProxyStateError): + ProxyStateStore(runner).read("auth", "proxy:latest") + + def test_write_uses_atomic_versioned_payload(self) -> None: + runner = MagicMock(spec=ContainerRunner) + runner.engine.run.return_value = MagicMock(returncode=0, stdout="", stderr="") + + ProxyStateStore(runner).write("auth", "proxy:latest", [".new.example"]) + + call = runner.engine.run.call_args + assert any("mv -f" in arg for arg in call.args) + assert json.loads(call.kwargs["input"]) == { + "schema": "allowed-domains.v1", + "domains": [".new.example"], + } + assert "auth:/data/auth" in call.args diff --git a/tests/test_session_resources.py b/tests/test_session_resources.py index 0db015d..3d4675c 100644 --- a/tests/test_session_resources.py +++ b/tests/test_session_resources.py @@ -240,6 +240,31 @@ def test_labels_reads_the_whole_spec_in_one_fetch( assert view.spec.agent == "codex" assert runner.list_containers.call_count == 1 + def test_labels_prefers_committed_domains_including_empty( + self, + resources: SessionResources, + runner: MagicMock, + proxy: MagicMock, + ) -> None: + runner.list_containers.return_value = [ + { + "Id": "abc", + "Labels": { + PAUDE_LABEL_SESSION: SESSION, + PAUDE_LABEL_AGENT: "codex", + "paude.io/proxy-image": "proxy:latest", + "paude.io/allowed-domains": ".stale.example", + }, + } + ] + proxy.read_domain_state.return_value = [] + + view = resources.labels(SESSION) + + assert view is not None + assert view.spec.allowed_domains == [] + proxy.read_domain_state.assert_called_once_with(SESSION, "proxy:latest") + def test_labels_is_none_for_a_session_with_no_container( self, resources: SessionResources, runner: MagicMock ) -> None: diff --git a/tests/test_upgrade.py b/tests/test_upgrade.py index 88965c4..69edbec 100644 --- a/tests/test_upgrade.py +++ b/tests/test_upgrade.py @@ -360,6 +360,9 @@ def _upgrade_backend( volumes = MagicMock(spec=VolumeManager) networks = MagicMock(spec=NetworkManager) backend = make_backend(runner, network_manager=networks, volume_manager=volumes) + backend._proxy.read_domain_state = MagicMock( # type: ignore[method-assign] + return_value=None + ) create_session = MagicMock(return_value=_upgraded_session()) start = MagicMock() backend.create_session = create_session # type: ignore[method-assign] From 8dc2caf9b448d53447573657034c5f9064927588 Mon Sep 17 00:00:00 2001 From: Ben Browning <56071+bbrowning@users.noreply.github.com> Date: Thu, 27 Aug 2026 23:37:47 +0000 Subject: [PATCH 2/3] Validate proxy replacements during domain updates Separate credentials that can be supplied from those required by the active provider mode. This lets Cursor browser OAuth update domains without CURSOR_API_KEY. Inspect each replacement after initialization. If it has stopped, roll back and preserve the retained working proxy. --- src/paude/backends/proxy_config.py | 16 +++-- src/paude/container/proxy_runner.py | 17 ++++++ src/paude/providers/base.py | 8 +++ tests/test_allow_domain.py | 2 + tests/test_podman_proxy.py | 2 + tests/test_proxy_secrets.py | 91 ++++++++++++++++++++++++++++- 6 files changed, 130 insertions(+), 6 deletions(-) diff --git a/src/paude/backends/proxy_config.py b/src/paude/backends/proxy_config.py index e521d6a..eacf1ce 100644 --- a/src/paude/backends/proxy_config.py +++ b/src/paude/backends/proxy_config.py @@ -200,18 +200,24 @@ def required_proxy_credential_targets( agent_config: AgentConfig | AgentComposition | Agent, credential_providers: list[str], ) -> set[str]: - """Return credential targets required by the session's selected providers.""" + """Return credential targets required by the active authentication modes.""" from paude.providers import get_provider - required = { + provider_targets = { key for provider_name in credential_providers for key in get_provider(provider_name).secret_env_vars } - required.update( + required = { + key + for provider_name in credential_providers + for key in get_provider(provider_name).required_secret_env_vars + } + config_targets = { key for config in _agent_configs(agent_config) for key in config.secret_env_vars - ) - return required + } + optional_provider_targets = provider_targets - required + return required | (config_targets - optional_provider_targets) def _agent_configs( diff --git a/src/paude/container/proxy_runner.py b/src/paude/container/proxy_runner.py index d503641..daa6590 100644 --- a/src/paude/container/proxy_runner.py +++ b/src/paude/container/proxy_runner.py @@ -8,6 +8,7 @@ from secrets import token_hex from paude.container.engine import ContainerEngine +from paude.container.proxy_inspect import ProxyInspectionError, ProxyInspector from paude.container.runner import ContainerRunner @@ -81,6 +82,7 @@ class ProxyRunner: def __init__(self, runner: ContainerRunner) -> None: self._runner = runner + self._inspector = ProxyInspector(runner) @property def _engine(self) -> ContainerEngine: @@ -254,6 +256,20 @@ def start_session_proxy(self, name: str) -> None: raise ProxyStartError(f"Failed to start proxy: {result.stderr}") time.sleep(1) + def _require_running_candidate(self, name: str) -> None: + """Fail when a started replacement did not survive initialization.""" + try: + running = self._inspector.running(name) + except ProxyInspectionError as exc: + raise ProxyStartError( + f"Failed to verify replacement proxy startup: {exc}" + ) from exc + if not running: + raise ProxyStartError( + "Replacement proxy exited during initialization; " + "the previous proxy will be restored." + ) + def recreate_session_proxy( self, name: str, @@ -357,6 +373,7 @@ def swap_session_proxy( ) swap.candidate_created = True self.start_session_proxy(name) + self._require_running_candidate(name) except Exception as primary: try: swap.rollback() diff --git a/src/paude/providers/base.py b/src/paude/providers/base.py index f985438..bf7dcd7 100644 --- a/src/paude/providers/base.py +++ b/src/paude/providers/base.py @@ -14,6 +14,9 @@ class ProviderConfig: display_name: Human-readable name (e.g., "Vertex AI"). passthrough_env_vars: Host env vars to forward to container (non-secret). secret_env_vars: Host env vars to deliver securely. + required_secret_env_vars: Secure env vars required for this provider's + proxy-backed authentication mode. A secret may be optional when the + provider supports an alternative login flow. passthrough_env_prefixes: Host env var prefixes to forward. domain_aliases: Domain aliases to auto-include in allowed-domains. """ @@ -22,6 +25,7 @@ class ProviderConfig: display_name: str passthrough_env_vars: list[str] = field(default_factory=list) secret_env_vars: list[str] = field(default_factory=list) + required_secret_env_vars: list[str] = field(default_factory=list) passthrough_env_prefixes: list[str] = field(default_factory=list) domain_aliases: list[str] = field(default_factory=list) @@ -44,6 +48,7 @@ class ProviderConfig: name="openai", display_name="OpenAI", secret_env_vars=["OPENAI_API_KEY"], + required_secret_env_vars=["OPENAI_API_KEY"], domain_aliases=["openai"], ), "chatgpt": ProviderConfig( @@ -56,6 +61,7 @@ class ProviderConfig: name="anthropic", display_name="Anthropic", secret_env_vars=["ANTHROPIC_API_KEY"], + required_secret_env_vars=["ANTHROPIC_API_KEY"], domain_aliases=["claude"], ), "anthropic-oauth": ProviderConfig( @@ -66,11 +72,13 @@ class ProviderConfig: # `Authorization: Bearer` header. The agent only ever sees the # `paude-proxy-managed` sentinel (set per-agent via extra_env_vars). secret_env_vars=["CLAUDE_CODE_OAUTH_TOKEN"], + required_secret_env_vars=["CLAUDE_CODE_OAUTH_TOKEN"], domain_aliases=["claude"], ), "cursor": ProviderConfig( name="cursor", display_name="Cursor", + # Optional: Cursor also supports browser OAuth inside the container. secret_env_vars=["CURSOR_API_KEY"], domain_aliases=["cursor"], ), diff --git a/tests/test_allow_domain.py b/tests/test_allow_domain.py index 1e14e38..8d19b69 100644 --- a/tests/test_allow_domain.py +++ b/tests/test_allow_domain.py @@ -93,6 +93,8 @@ def run(*args: str, **_kwargs: object) -> MagicMock: return MagicMock( returncode=0, stdout=json.dumps(["podman", "create"]), stderr="" ) + if args[:3] == ("inspect", "-f", "{{.State.Running}}"): + return MagicMock(returncode=0, stdout="true\n", stderr="") if args and args[0] == "run" and any("test -e" in arg for arg in args): return MagicMock(returncode=3, stdout="", stderr="") return MagicMock(returncode=0, stdout="", stderr="") diff --git a/tests/test_podman_proxy.py b/tests/test_podman_proxy.py index 3c03bf1..c769d73 100644 --- a/tests/test_podman_proxy.py +++ b/tests/test_podman_proxy.py @@ -35,6 +35,8 @@ def _make_mock_runner(engine_binary: str = "podman") -> MagicMock: def run(*args: str, **_kwargs: object) -> MagicMock: if args[:3] == ("inspect", "-f", "{{json .Config.CreateCommand}}"): return MagicMock(returncode=0, stdout=json.dumps([engine_binary, "create"])) + if args[:3] == ("inspect", "-f", "{{.State.Running}}"): + return MagicMock(returncode=0, stdout="true\n", stderr="") if args and args[0] == "run" and any("test -e" in arg for arg in args): return MagicMock(returncode=3, stdout="", stderr="") return MagicMock(returncode=0, stdout="", stderr="") diff --git a/tests/test_proxy_secrets.py b/tests/test_proxy_secrets.py index fe5bfb4..bdf3447 100644 --- a/tests/test_proxy_secrets.py +++ b/tests/test_proxy_secrets.py @@ -8,9 +8,14 @@ import pytest +from paude.agents.cursor import CursorAgent from paude.backends.podman.helpers import proxy_secret_name, proxy_secret_prefix from paude.backends.podman.proxy_credentials import ProxyCredentialManager -from paude.backends.proxy_config import ProxyCredentials +from paude.backends.proxy_config import ( + ProxyCredentials, + proxy_credential_targets, + required_proxy_credential_targets, +) from paude.container.engine import ContainerEngine from paude.container.proxy_inspect import ProxyInspectionError from paude.container.proxy_runner import ProxyRunner, ProxyStartError @@ -491,6 +496,51 @@ def test_docker_preserves_current_values_and_overlays_only_refresh(self) -> None } assert "ALLOWED_DOMAINS" not in prepared.credentials.environment + def test_podman_cursor_browser_auth_preserves_unrelated_binding(self) -> None: + command = [ + "podman", + "create", + "--secret", + "old-gh,type=env,target=GH_TOKEN", + ] + transport = FakeTransport( + results={"inspect -f": _result(stdout=json.dumps(command))} + ) + runner = ContainerRunner(make_engine("podman", transport=transport)) + cursor = CursorAgent().config + required = required_proxy_credential_targets(cursor, ["cursor"]) + + prepared = ProxyCredentialManager(runner).prepare_update( + "sess", + "paude-proxy-sess", + ProxyCredentials(), + proxy_credential_targets(cursor), + required, + ) + + assert required == set() + assert prepared.secret_refs == ["old-gh,type=env,target=GH_TOKEN"] + + def test_docker_cursor_browser_auth_preserves_unrelated_binding(self) -> None: + current = ["GH_TOKEN=unrelated", "ALLOWED_DOMAINS=.old.example"] + transport = FakeTransport( + results={"inspect -f": _result(stdout=json.dumps(current))} + ) + runner = ContainerRunner(make_engine("docker", transport=transport)) + cursor = CursorAgent().config + required = required_proxy_credential_targets(cursor, ["cursor"]) + + prepared = ProxyCredentialManager(runner).prepare_update( + "sess", + "paude-proxy-sess", + ProxyCredentials(), + proxy_credential_targets(cursor), + required, + ) + + assert required == set() + assert prepared.credentials.environment == {"GH_TOKEN": "unrelated"} + class TestRollbackProxySwap: """A candidate failure restores the retained proxy and its fixed address.""" @@ -561,3 +611,42 @@ def run(*args: str, **_kwargs: object) -> MagicMock: "paude-net-sess", commands[rename_out][2], ) + + def test_candidate_exit_after_successful_start_restores_old_proxy(self) -> None: + engine = MagicMock() + engine.supports_multi_network_create = True + engine.default_bridge_network = "podman" + + def run(*args: str, **_kwargs: object) -> MagicMock: + if args[:3] == ("inspect", "-f", "{{.State.Running}}"): + return MagicMock(returncode=0, stdout="false\n", stderr="") + return MagicMock(returncode=0, stdout="", stderr="") + + engine.run.side_effect = run + runner = MagicMock(spec=ContainerRunner) + runner.engine = engine + runner.container_running.return_value = True + + with pytest.raises(ProxyStartError, match="exited during initialization"): + ProxyRunner(runner).swap_session_proxy( + name="paude-proxy-sess", + image="proxy:latest", + network="paude-net-sess", + ip="10.89.0.2", + ) + + commands = [call.args for call in engine.run.call_args_list] + rename_out = next(i for i, cmd in enumerate(commands) if cmd[0] == "rename") + backup_name = commands[rename_out][2] + candidate_start = next( + i for i, cmd in enumerate(commands) if cmd == ("start", "paude-proxy-sess") + ) + state_check = next(i for i, cmd in enumerate(commands) if cmd[0] == "inspect") + candidate_remove = next( + i + for i, cmd in enumerate(commands) + if cmd == ("rm", "-f", "paude-proxy-sess") + ) + rename_back = max(i for i, cmd in enumerate(commands) if cmd[0] == "rename") + assert candidate_start < state_check < candidate_remove < rename_back + assert ("rm", "-f", backup_name) not in commands From a53f16c8f90118676519eb786b8b215036007fe6 Mon Sep 17 00:00:00 2001 From: Ben Browning <56071+bbrowning@users.noreply.github.com> Date: Mon, 31 Aug 2026 17:24:32 +0000 Subject: [PATCH 3/3] Stabilize rendered CLI test output Run every pytest Make target with one explicit color environment and centralize ANSI normalization for rendered CLI assertions. Cover the Make child environment and shared normalizer so inherited terminal settings cannot mask styling-sensitive tests. --- Makefile | 9 +++-- tests/ansi.py | 12 ++++++ tests/test_ansi.py | 26 +++++++++++++ tests/test_cli.py | 88 +++++++++++++++++++++--------------------- tests/test_makefile.py | 81 ++++++++++++++++++++++++++++++++++++++ tests/test_workflow.py | 14 ++----- 6 files changed, 172 insertions(+), 58 deletions(-) create mode 100644 tests/ansi.py create mode 100644 tests/test_ansi.py create mode 100644 tests/test_makefile.py diff --git a/Makefile b/Makefile index 1a36592..544fdea 100644 --- a/Makefile +++ b/Makefile @@ -54,6 +54,7 @@ help: # Detect native architecture for builds NATIVE_ARCH := $(shell uname -m | sed 's/x86_64/amd64/') +PYTEST := env -u NO_COLOR FORCE_COLOR=1 TERM=xterm-256color uv run pytest # Build images locally (native arch, for development) build: @@ -66,19 +67,19 @@ run: # Run unit tests (default, fast - integration tests excluded via pyproject.toml) test: - uv run pytest --cov=paude --cov-report=term-missing + $(PYTEST) --cov=paude --cov-report=term-missing # Run all integration tests (requires infrastructure) test-integration: - uv run pytest tests/integration/ -v -m integration + $(PYTEST) tests/integration/ -v -m integration # Run all tests (unit + integration, for CI) test-all: - uv run pytest -o "addopts=-v" --cov=paude --cov-report=term-missing + $(PYTEST) -o "addopts=-v" --cov=paude --cov-report=term-missing # Run Podman integration tests test-podman: - uv run pytest tests/integration/ -v -m podman + $(PYTEST) tests/integration/ -v -m podman # Development targets install: diff --git a/tests/ansi.py b/tests/ansi.py new file mode 100644 index 0000000..8f89783 --- /dev/null +++ b/tests/ansi.py @@ -0,0 +1,12 @@ +"""Helpers for assertions against terminal-rendered output.""" + +from __future__ import annotations + +import re + +_ANSI_SGR = re.compile(r"\x1b\[[0-9;]*m") + + +def strip_ansi(text: str) -> str: + """Remove ANSI Select Graphic Rendition sequences from text.""" + return _ANSI_SGR.sub("", text) diff --git a/tests/test_ansi.py b/tests/test_ansi.py new file mode 100644 index 0000000..f430902 --- /dev/null +++ b/tests/test_ansi.py @@ -0,0 +1,26 @@ +"""Tests for terminal-output assertion helpers.""" + +import pytest + +from tests.ansi import strip_ansi + + +@pytest.mark.parametrize( + ("text", "expected"), + [ + pytest.param("plain text", "plain text", id="plain"), + pytest.param("\x1b[31merror\x1b[0m", "error", id="color"), + pytest.param("-\x1b[1;36m-option\x1b[0m", "--option", id="split-option"), + ], +) +def test_strip_ansi(text: str, expected: str) -> None: + """ANSI styling is removed without changing the rendered text.""" + assert strip_ansi(text) == expected + + +def test_strip_ansi_is_idempotent() -> None: + """Normalizing output more than once does not alter it further.""" + styled = "\x1b[1mimportant\x1b[0m" + normalized = strip_ansi(styled) + + assert strip_ansi(normalized) == normalized diff --git a/tests/test_cli.py b/tests/test_cli.py index 6b0fd84..3ea108e 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -2,7 +2,6 @@ from __future__ import annotations -import re import subprocess from pathlib import Path from unittest.mock import MagicMock, patch @@ -12,6 +11,7 @@ from paude.backends import Session from paude.cli import _parse_copy_path, app +from tests.ansi import strip_ansi runner = CliRunner() @@ -26,8 +26,9 @@ def test_help_shows_help(flag): """Help flag shows help and exits 0.""" result = runner.invoke(app, [flag]) + output = strip_ansi(result.stdout) assert result.exit_code == 0 - assert "Run AI coding agents in isolated containers" in result.stdout + assert "Run AI coding agents in isolated containers" in output @pytest.mark.parametrize( @@ -130,7 +131,7 @@ def test_forward_port_recognized_on_attach_commands(command): """--forward-port is accepted by both attach commands.""" result = runner.invoke(app, [command, "--help"]) assert result.exit_code == 0 - assert "--forward-port" in _strip_ansi(result.stdout) + assert "--forward-port" in strip_ansi(result.stdout) @pytest.mark.parametrize("command", ["connect", "start"]) @@ -245,7 +246,7 @@ def test_allowed_domains_multiple_values(): def test_help_shows_dry_run_option(): """--help shows --dry-run option.""" result = runner.invoke(app, ["--help"]) - assert "--dry-run" in result.stdout + assert "--dry-run" in strip_ansi(result.stdout) def test_args_option(): @@ -409,7 +410,7 @@ def test_claude_anthropic_oauth_dry_run(self): ], ) assert result.exit_code == 0 - assert "provider: anthropic-oauth" in _strip_ansi(result.stdout) + assert "provider: anthropic-oauth" in strip_ansi(result.stdout) def test_gascity_claude_codex_swap_to_anthropic_oauth(self): """The user's flow: claude + gascity on anthropic-oauth, codex on chatgpt.""" @@ -425,7 +426,7 @@ def test_gascity_claude_codex_swap_to_anthropic_oauth(self): ], ) assert result.exit_code == 0 - out = _strip_ansi(result.stdout) + out = strip_ansi(result.stdout) assert "gascity -> anthropic-oauth" in out assert "claude -> anthropic-oauth" in out assert "codex -> chatgpt" in out @@ -465,7 +466,7 @@ def test_agents_providers_dry_run(self): ], ) assert result.exit_code == 0 - out = _strip_ansi(result.stdout) + out = strip_ansi(result.stdout) assert "agents: gascity, claude, codex" in out assert "credential providers: vertex, chatgpt" in out # Derived per-agent providers. @@ -480,13 +481,13 @@ def test_agents_repeatable_option(self): ["create", "--agents", "gascity", "--agents", "claude", "--dry-run"], ) assert result.exit_code == 0 - assert "agents: gascity, claude" in _strip_ansi(result.stdout) + assert "agents: gascity, claude" in strip_ansi(result.stdout) def test_singular_agent_alias_dry_run(self): """--agent still resolves to a single-item agents list.""" result = runner.invoke(app, ["create", "--agent", "gascity", "--dry-run"]) assert result.exit_code == 0 - assert "agents: gascity" in _strip_ansi(result.stdout) + assert "agents: gascity" in strip_ansi(result.stdout) @patch("paude.dry_run.show_dry_run") def test_single_gascity_install_is_exact(self, mock_show: MagicMock): @@ -502,7 +503,7 @@ def test_duplicate_agents_rejected(self): app, ["create", "--agents", "claude,claude,codex", "--dry-run"] ) assert result.exit_code != 0 - assert "Duplicate agent" in _strip_ansi(result.output) + assert "Duplicate agent" in strip_ansi(result.output) def test_agent_and_agents_conflict(self): """Passing both --agent and --agents fails with a clear message.""" @@ -510,7 +511,7 @@ def test_agent_and_agents_conflict(self): app, ["create", "--agent", "claude", "--agents", "codex", "--dry-run"] ) assert result.exit_code != 0 - assert "not both" in _strip_ansi(result.output) + assert "not both" in strip_ansi(result.output) def test_provider_and_providers_are_independent(self): """Primary mapping shorthand can use an explicit credential set.""" @@ -526,7 +527,7 @@ def test_provider_and_providers_are_independent(self): ], ) assert result.exit_code == 0 - out = _strip_ansi(result.output) + out = strip_ansi(result.output) assert "credential providers: vertex, openai" in out assert "claude -> vertex" in out @@ -543,7 +544,7 @@ def test_provider_and_agent_provider_conflict(self): ], ) assert result.exit_code != 0 - assert "not both" in _strip_ansi(result.output) + assert "not both" in strip_ansi(result.output) @pytest.mark.parametrize( "mapping", @@ -555,7 +556,7 @@ def test_malformed_agent_provider_rejected(self, mapping: str): ["create", "--agent-provider", mapping, "--dry-run"], ) assert result.exit_code != 0 - assert "expected AGENT=PROVIDER" in _strip_ansi(result.output) + assert "expected AGENT=PROVIDER" in strip_ansi(result.output) def test_duplicate_agent_provider_mapping_rejected(self): result = runner.invoke( @@ -570,7 +571,7 @@ def test_duplicate_agent_provider_mapping_rejected(self): ], ) assert result.exit_code != 0 - assert "Duplicate provider mapping" in _strip_ansi(result.output) + assert "Duplicate provider mapping" in strip_ansi(result.output) def test_unknown_agent_rejected(self): """An unknown agent name in --agents is rejected.""" @@ -588,7 +589,7 @@ def test_multi_agent_real_create_passes_full_composition( mock_prepare.return_value = ([], [], {}, False) result = runner.invoke(app, ["create", "--agents", "claude,codex,gascity"]) assert result.exit_code == 0 - out = _strip_ansi(result.output) + out = strip_ansi(result.output) assert "multi-agent creation is not yet supported" not in out mock_create.assert_called_once() assert mock_create.call_args.kwargs["agent_name"] == "claude" @@ -609,7 +610,7 @@ def test_single_agent_real_create_no_warning(self, mock_prepare, mock_create): mock_prepare.return_value = ([], [], {}, False) result = runner.invoke(app, ["create", "--agents", "claude"]) assert result.exit_code == 0 - assert "multi-agent creation is not yet supported" not in _strip_ansi( + assert "multi-agent creation is not yet supported" not in strip_ansi( result.output ) mock_create.assert_called_once() @@ -619,13 +620,13 @@ def test_empty_agent_rejected_cleanly(self): result = runner.invoke(app, ["create", "--agent", "", "--dry-run"]) assert result.exit_code != 0 assert result.exception is None or not isinstance(result.exception, IndexError) - assert "Agent name cannot be empty" in _strip_ansi(result.output) + assert "Agent name cannot be empty" in strip_ansi(result.output) def test_empty_provider_rejected_cleanly(self): """An explicit empty --provider fails with a clean error, not a silent default.""" result = runner.invoke(app, ["create", "--provider", "", "--dry-run"]) assert result.exit_code != 0 - assert "Provider name cannot be empty" in _strip_ansi(result.output) + assert "Provider name cannot be empty" in strip_ansi(result.output) @patch("paude.cli.create_podman.create_podman_session") @patch("paude.cli.create._prepare_session_create") @@ -687,7 +688,7 @@ def test_extra_credential_provider_shown_in_dry_run(self): ], ) assert result.exit_code == 0 - out = _strip_ansi(result.stdout) + out = strip_ansi(result.stdout) assert "credential providers: vertex, chatgpt, openai" in out assert "codex -> chatgpt" in out @@ -713,23 +714,18 @@ def test_no_command_accepts_github_token(args): assert "No such option" in result.output -def _strip_ansi(text: str) -> str: - """Remove ANSI escape codes from text.""" - return re.sub(r"\x1b\[[0-9;]*m", "", text) - - class TestCreateHostFlag: """Tests for --host and --ssh-key CLI flags.""" def test_host_flag_recognized(self): """--host flag is accepted by the create command.""" result = runner.invoke(app, ["create", "--help"]) - assert "--host" in _strip_ansi(result.stdout) + assert "--host" in strip_ansi(result.stdout) def test_ssh_key_flag_recognized(self): """--ssh-key flag is accepted by the create command.""" result = runner.invoke(app, ["create", "--help"]) - assert "--ssh-key" in _strip_ansi(result.stdout) + assert "--ssh-key" in strip_ansi(result.stdout) def test_ssh_key_without_host_rejected(self): """--ssh-key requires --host.""" @@ -786,18 +782,19 @@ def test_bare_paude_shows_list(): def test_help_shows_commands(): """Help shows commands section.""" result = runner.invoke(app, ["--help"]) + output = strip_ansi(result.stdout) assert result.exit_code == 0 - assert "create" in result.stdout - assert "start" in result.stdout - assert "stop" in result.stdout - assert "list" in result.stdout + assert "create" in output + assert "start" in output + assert "stop" in output + assert "list" in output def test_help_shows_extra_sections(): """Help includes extra reference sections as Rich panels.""" result = runner.invoke(app, ["--help"]) assert result.exit_code == 0 - output = result.stdout + output = strip_ansi(result.stdout) assert "Workflow" in output assert "Syncing Code" in output assert "Copying Files" in output @@ -819,19 +816,21 @@ def test_help_shows_extra_sections(): def test_subcommand_help(command, description): """Subcommand --help shows its own help, not main help.""" result = runner.invoke(app, [command, "--help"]) + output = strip_ansi(result.stdout) assert result.exit_code == 0 - assert command in result.stdout.lower() - assert description in result.stdout - assert "paude - Run Claude Code" not in result.stdout + assert command in output.lower() + assert description in output + assert "paude - Run Claude Code" not in output def test_remote_help(): """'remote --help' shows subcommand help.""" result = runner.invoke(app, ["remote", "--help"]) + output = strip_ansi(result.stdout) assert result.exit_code == 0 - assert "remote" in result.stdout.lower() - assert "git" in result.stdout.lower() or "ACTION" in result.stdout - assert "paude - Run Claude Code" not in result.stdout + assert "remote" in output.lower() + assert "git" in output.lower() or "ACTION" in output + assert "paude - Run Claude Code" not in output class TestRemoteCommand: @@ -1157,9 +1156,10 @@ def test_subcommand_runs_without_main_execution(): # This test verifies that subcommands don't trigger podman checks # by confirming they complete without the "podman required" error result = runner.invoke(app, ["stop", "--help"]) + output = strip_ansi(result.stdout) assert result.exit_code == 0 - assert "Stop a session" in result.stdout - assert "podman is required" not in result.stdout + assert "Stop a session" in output + assert "podman is required" not in output # Tests for connect command multi-backend search behavior @@ -1801,14 +1801,14 @@ def test_cp_help(self): result = runner.invoke(app, ["cp", "--help"]) assert result.exit_code == 0 - assert "Copy files between local and a session" in result.stdout + assert "Copy files between local and a session" in strip_ansi(result.stdout) def test_help_shows_cp_command(self): """Main help shows cp command.""" result = runner.invoke(app, ["--help"]) assert result.exit_code == 0 - assert "cp" in result.stdout + assert "cp" in strip_ansi(result.stdout) # --------------------------------------------------------------------------- @@ -1881,7 +1881,7 @@ def test_help_documents_refresh_credentials(self) -> None: result = runner.invoke(app, ["allowed-domains", "--help"]) assert result.exit_code == 0 - assert "--refresh-credentials" in result.stdout + assert "--refresh-credentials" in strip_ansi(result.stdout) class TestBlockedDomainsCLI: @@ -1971,7 +1971,7 @@ def test_proxy_not_running_error(self, mock_resolve: MagicMock) -> None: def test_help_includes_blocked_domains() -> None: """Help output includes blocked-domains command.""" result = runner.invoke(app, ["--help"]) - assert "blocked-domains" in result.stdout + assert "blocked-domains" in strip_ansi(result.stdout) class TestDetectDevScriptDir: diff --git a/tests/test_makefile.py b/tests/test_makefile.py new file mode 100644 index 0000000..a29e997 --- /dev/null +++ b/tests/test_makefile.py @@ -0,0 +1,81 @@ +"""Tests for Make command boundaries.""" + +from __future__ import annotations + +import os +import subprocess +from pathlib import Path + +import pytest + +PROJECT_ROOT = Path(__file__).resolve().parents[1] + + +@pytest.mark.parametrize( + ("target", "expected_args"), + [ + pytest.param( + "test", + "run pytest --cov=paude --cov-report=term-missing", + id="unit", + ), + pytest.param( + "test-all", + "run pytest -o addopts=-v --cov=paude --cov-report=term-missing", + id="all", + ), + pytest.param( + "test-integration", + "run pytest tests/integration/ -v -m integration", + id="integration", + ), + pytest.param( + "test-podman", + "run pytest tests/integration/ -v -m podman", + id="podman", + ), + ], +) +def test_pytest_target_enforces_color_environment( + tmp_path: Path, + target: str, + expected_args: str, +) -> None: + """Every pytest target gives its child a deterministic color environment.""" + bin_dir = tmp_path / "bin" + bin_dir.mkdir() + invocation_log = tmp_path / "uv-invocation" + uv = bin_dir / "uv" + uv.write_text( + "#!/bin/sh\n" + 'printf \'%s\\n\' "${NO_COLOR+x}" "${FORCE_COLOR-}" ' + '"${TERM-}" "$*" >"$UV_INVOCATION_LOG"\n' + ) + uv.chmod(0o755) + env = os.environ.copy() + env.update( + { + "NO_COLOR": "1", + "FORCE_COLOR": "0", + "TERM": "dumb", + "UV_INVOCATION_LOG": str(invocation_log), + "PATH": f"{bin_dir}{os.pathsep}{env['PATH']}", + } + ) + + result = subprocess.run( + ["make", target], + cwd=PROJECT_ROOT, + env=env, + check=False, + capture_output=True, + text=True, + ) + + assert result.returncode == 0, result.stderr + assert invocation_log.read_text().splitlines() == [ + "", + "1", + "xterm-256color", + expected_args, + ] diff --git a/tests/test_workflow.py b/tests/test_workflow.py index d52cf25..fbb643b 100644 --- a/tests/test_workflow.py +++ b/tests/test_workflow.py @@ -19,6 +19,7 @@ reset_session, status_sessions, ) +from tests.ansi import strip_ansi class TestGetContainerBranch: @@ -1323,13 +1324,6 @@ def test_reset_exec_failure( reset_session("test", force=True) -def _strip_ansi(text: str) -> str: - """Remove ANSI escape codes from text.""" - import re - - return re.sub(r"\x1b\[[0-9;]*m", "", text) - - class TestHarvestCli: """Tests for harvest CLI command.""" @@ -1340,7 +1334,7 @@ def test_harvest_help(self) -> None: runner = CliRunner() result = runner.invoke(app, ["harvest", "--help"]) - output = _strip_ansi(result.output) + output = strip_ansi(result.output) assert result.exit_code == 0 assert "harvest" in output.lower() assert "--branch" in output @@ -1357,7 +1351,7 @@ def test_reset_help(self) -> None: runner = CliRunner() result = runner.invoke(app, ["reset", "--help"]) - output = _strip_ansi(result.output) + output = strip_ansi(result.output) assert result.exit_code == 0 assert "--branch" in output assert "--force" in output @@ -1374,6 +1368,6 @@ def test_status_help(self) -> None: runner = CliRunner() result = runner.invoke(app, ["status", "--help"]) - output = _strip_ansi(result.output) + output = strip_ansi(result.output) assert result.exit_code == 0 assert "status" in output.lower()