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/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..eacf1ce 100644 --- a/src/paude/backends/proxy_config.py +++ b/src/paude/backends/proxy_config.py @@ -170,3 +170,62 @@ 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 active authentication modes.""" + from paude.providers import get_provider + + provider_targets = { + key + for provider_name in credential_providers + for key in get_provider(provider_name).secret_env_vars + } + 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 + } + optional_provider_targets = provider_targets - required + return required | (config_targets - optional_provider_targets) + + +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..daa6590 100644 --- a/src/paude/container/proxy_runner.py +++ b/src/paude/container/proxy_runner.py @@ -4,8 +4,11 @@ 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.proxy_inspect import ProxyInspectionError, ProxyInspector from paude.container.runner import ContainerRunner @@ -15,6 +18,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. @@ -24,6 +82,7 @@ class ProxyRunner: def __init__(self, runner: ContainerRunner) -> None: self._runner = runner + self._inspector = ProxyInspector(runner) @property def _engine(self) -> ContainerEngine: @@ -52,7 +111,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 +238,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: @@ -186,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, @@ -228,3 +312,74 @@ 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) + self._require_running_candidate(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/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/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_allow_domain.py b/tests/test_allow_domain.py index c1bf36f..8d19b69 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,19 @@ 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[: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="") + + 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 +144,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_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 6042e0c..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,21 +1801,89 @@ 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) # --------------------------------------------------------------------------- -# 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 strip_ansi(result.stdout) + + class TestBlockedDomainsCLI: """Tests for the blocked-domains CLI subcommand.""" @@ -1903,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_podman_proxy.py b/tests/test_podman_proxy.py index 4bf0fda..c769d73 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,17 @@ 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[: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="") + + mock_runner.engine.run.side_effect = run return mock_runner @@ -895,10 +907,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 +923,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 +1084,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..bdf3447 100644 --- a/tests/test_proxy_secrets.py +++ b/tests/test_proxy_secrets.py @@ -2,15 +2,25 @@ from __future__ import annotations +import json +import subprocess from unittest.mock import MagicMock, patch import pytest +from paude.agents.cursor import CursorAgent from paude.backends.podman.helpers import proxy_secret_name, proxy_secret_prefix -from paude.backends.proxy_config import ProxyCredentials +from paude.backends.podman.proxy_credentials import ProxyCredentialManager +from paude.backends.proxy_config import ( + ProxyCredentials, + proxy_credential_targets, + required_proxy_credential_targets, +) 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 +345,308 @@ 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 + + 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.""" + + 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], + ) + + 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 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] 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()