From 112f87a236fe7371867f3f8a111468ff08379697 Mon Sep 17 00:00:00 2001 From: Ben Browning <56071+bbrowning@users.noreply.github.com> Date: Tue, 29 Sep 2026 10:10:24 -0400 Subject: [PATCH 1/2] Enforce required provider secrets on create and upgrade Providers declare required_secret_env_vars, but they were only checked when mutating allowed domains/endpoints. `paude create --provider anthropic-oauth` without CLAUDE_CODE_OAUTH_TOKEN exported built a session whose proxy had no token, so the agent failed to authenticate with no hint why. The same applied to ANTHROPIC_API_KEY and OPENAI_API_KEY. Add check_required_secrets() to the provider registry, with an optional per-provider auth_hint, and call it from create (after dry-run, before any build) and upgrade (before the manifest save, teardown, or stopping a running session). start/connect are unchanged since an existing proxy keeps its bindings; the missing-proxy recreate gap is logged as PROXY-001. Co-Authored-By: Claude Opus 5.5 --- KNOWN_ISSUES.md | 17 +++++++++++ README.md | 5 +++- src/paude/cli/create.py | 8 +++++ src/paude/cli/upgrade.py | 43 +++++++++++++++++++++++---- src/paude/providers/__init__.py | 8 ++++- src/paude/providers/base.py | 30 +++++++++++++++++++ tests/test_cli.py | 49 ++++++++++++++++++++++++++++++- tests/test_providers.py | 41 +++++++++++++++++++++++++- tests/test_upgrade.py | 52 +++++++++++++++++++++++++++++++-- 9 files changed, 240 insertions(+), 13 deletions(-) diff --git a/KNOWN_ISSUES.md b/KNOWN_ISSUES.md index 2b306115..7992fb8c 100644 --- a/KNOWN_ISSUES.md +++ b/KNOWN_ISSUES.md @@ -448,6 +448,23 @@ registry records them as docker. A fix would either treat a shim `docker` as podman for capabilities, or reject/redirect `--backend=docker` when the shim is detected. +### PROXY-001: recreating a missing proxy on `start`/`connect` drops unset credentials + +**Status**: Open +**Severity**: Low +**Discovered**: 2026-09-29 while enforcing required provider secrets on `create`/`upgrade` + +`paude create` and `paude upgrade` now refuse to run when a provider's +`required_secret_env_vars` are unset on the host. `start` and `connect` are not +checked, which is correct when the proxy container still exists (it keeps its +original credential bindings). But when the proxy is missing, +`PodmanProxyManager.start_if_needed` recreates it from +`gather_proxy_credentials()`, which silently skips unset host variables. A +session recreated that way comes up with no `CLAUDE_CODE_OAUTH_TOKEN` (or API +key) binding, even though the Podman secret from `create` may still exist. A +fix would either reuse the session's existing credential secrets when +recreating, or run `check_required_secrets` in that branch only. + ## Agent Limitations Issues caused by upstream agent behavior, not paude bugs. diff --git a/README.md b/README.md index 22db0d04..be8e0a54 100644 --- a/README.md +++ b/README.md @@ -122,7 +122,10 @@ the real token never reaches the agent container (the agent only sees a `paude-proxy-managed` sentinel). Because the token does not rotate, one token can be shared across all your sessions — the same value must be present on later `start`/`connect`/`upgrade`. When it expires, re-run `claude setup-token`, export -the new value, and upgrade (or recreate) the session. +the new value, and upgrade (or recreate) the session. `paude create` and +`paude upgrade` fail up front if a provider's required variable (here +`CLAUDE_CODE_OAUTH_TOKEN`; `ANTHROPIC_API_KEY` or `OPENAI_API_KEY` for the +API-key providers) is not set. The token is used only by the official `claude` binary running in the session (including when Gas City's `gc` spawns it). Sharing one subscription seat across diff --git a/src/paude/cli/create.py b/src/paude/cli/create.py index 7e422319..f5730f15 100644 --- a/src/paude/cli/create.py +++ b/src/paude/cli/create.py @@ -327,6 +327,14 @@ def session_create( ) raise typer.Exit() + from paude.providers import check_required_secrets + + try: + check_required_secrets(resolved.providers) + except ValueError as e: + typer.echo(f"Error: {e}", err=True) + raise typer.Exit(1) from None + if ssh_key and not host: typer.echo( "Error: --ssh-key requires --host.", diff --git a/src/paude/cli/upgrade.py b/src/paude/cli/upgrade.py index be249093..a79cfb52 100644 --- a/src/paude/cli/upgrade.py +++ b/src/paude/cli/upgrade.py @@ -349,14 +349,15 @@ def session_upgrade( err=True, ) - # Auto-stop if running - if session is not None and session.status == "running": - typer.echo(f"Stopping session '{name}'...", err=True) - backend_obj.stop_session(name) - try: if isinstance(backend_obj, PodmanBackend): - _upgrade_podman(name, backend_obj, True, overrides) + _upgrade_podman( + name, + backend_obj, + True, + overrides, + stop_running=session is not None and session.status == "running", + ) else: typer.echo("Unsupported backend for upgrade.", err=True) raise typer.Exit(1) @@ -612,6 +613,8 @@ def _upgrade_podman( backend: PodmanBackend, rebuild: bool, overrides: UpgradeOverrides, + *, + stop_running: bool = False, ) -> None: """Upgrade a Podman/Docker session in place. @@ -630,6 +633,13 @@ def _upgrade_podman( state, created_at = _resolve_upgrade_state(name, backend) _apply_overrides(state, overrides) + _require_provider_secrets(name, state.spec.credential_providers) + + # Stopped only once the preflight checks pass, so a refused upgrade leaves + # a running session running. + if stop_running: + typer.echo(f"Stopping session '{name}'...", err=True) + backend.stop_session(name) # Persist the fully-resolved config BEFORE any destructive step, so an # interrupt from here on can be finished by re-running the upgrade. @@ -669,6 +679,27 @@ def _upgrade_podman( _recreate_session(name, backend, state, images, config) +def _require_provider_secrets(name: str, credential_providers: list[str]) -> None: + """Refuse to rebuild when the host lacks a provider's required secrets. + + The proxy is recreated from the host environment, so rebuilding without + them would leave the session unable to authenticate. Checked before the + manifest is saved or anything is torn down. + """ + from paude.providers import check_required_secrets + + try: + check_required_secrets(credential_providers) + except ValueError as e: + typer.echo(f"Error: {e}", err=True) + typer.echo( + f"Session '{name}' was not modified. Export the missing " + f"variables and re-run 'paude upgrade {name}'.", + err=True, + ) + raise typer.Exit(1) from None + + def _recreate_session( name: str, backend: PodmanBackend, diff --git a/src/paude/providers/__init__.py b/src/paude/providers/__init__.py index 5a05b6ef..a61098bb 100644 --- a/src/paude/providers/__init__.py +++ b/src/paude/providers/__init__.py @@ -5,11 +5,17 @@ resolve_agent_provider, supported_providers, ) -from paude.providers.base import ProviderConfig, get_provider, list_providers +from paude.providers.base import ( + ProviderConfig, + check_required_secrets, + get_provider, + list_providers, +) __all__ = [ "AgentProviderConfig", "ProviderConfig", + "check_required_secrets", "get_provider", "list_providers", "resolve_agent_provider", diff --git a/src/paude/providers/base.py b/src/paude/providers/base.py index bf7dcd7f..0ab3a9bd 100644 --- a/src/paude/providers/base.py +++ b/src/paude/providers/base.py @@ -2,6 +2,8 @@ from __future__ import annotations +import os +from collections.abc import Iterable, Mapping from dataclasses import dataclass, field @@ -17,6 +19,7 @@ class ProviderConfig: 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. + auth_hint: How to obtain the required secrets, shown when missing. passthrough_env_prefixes: Host env var prefixes to forward. domain_aliases: Domain aliases to auto-include in allowed-domains. """ @@ -26,6 +29,7 @@ class ProviderConfig: 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) + auth_hint: str = "" passthrough_env_prefixes: list[str] = field(default_factory=list) domain_aliases: list[str] = field(default_factory=list) @@ -73,6 +77,7 @@ class ProviderConfig: # `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"], + auth_hint="run `claude setup-token` on the host and export the token", domain_aliases=["claude"], ), "cursor": ProviderConfig( @@ -112,3 +117,28 @@ def get_provider(name: str) -> ProviderConfig: def list_providers() -> list[str]: """List all registered provider names.""" return sorted(_PROVIDERS.keys()) + + +def check_required_secrets( + provider_names: Iterable[str], + environ: Mapping[str, str] | None = None, +) -> None: + """Fail if any provider's required secret env vars are unset on the host. + + Raises: + ValueError: Naming each missing variable and the provider needing it. + """ + env = os.environ if environ is None else environ + problems: list[str] = [] + for name in dict.fromkeys(provider_names): + provider = get_provider(name) + missing = [key for key in provider.required_secret_env_vars if not env.get(key)] + if not missing: + continue + hint = f"; {provider.auth_hint}" if provider.auth_hint else "" + problems.append(f"{', '.join(missing)} (required by provider '{name}'{hint})") + if problems: + raise ValueError( + "Missing required credentials in the host environment: " + + "; ".join(problems) + ) diff --git a/tests/test_cli.py b/tests/test_cli.py index 5fb92536..bee345f0 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -493,6 +493,50 @@ def test_gascity_claude_codex_swap_to_anthropic_oauth(self): assert "claude -> anthropic-oauth" in out assert "codex -> chatgpt" in out + @patch("paude.cli.create_podman.create_podman_session") + @patch("paude.cli.create._prepare_session_create") + def test_create_without_oauth_token_fails(self, mock_prepare, mock_create): + """create refuses to build a session whose proxy would lack the token.""" + result = runner.invoke( + app, + ["create", "--agent", "claude", "--provider", "anthropic-oauth"], + env={"CLAUDE_CODE_OAUTH_TOKEN": None}, + ) + assert result.exit_code == 1 + output = result.stdout + (result.stderr or "") + assert "CLAUDE_CODE_OAUTH_TOKEN" in output + assert "claude setup-token" in output + mock_prepare.assert_not_called() + mock_create.assert_not_called() + + @patch("paude.cli.create_podman.create_podman_session") + @patch("paude.cli.create._prepare_session_create") + def test_create_with_oauth_token_proceeds(self, mock_prepare, mock_create): + mock_prepare.return_value = ([], [], {}, False) + result = runner.invoke( + app, + ["create", "--agent", "claude", "--provider", "anthropic-oauth"], + env={"CLAUDE_CODE_OAUTH_TOKEN": "sk-ant-oat01-test"}, + ) + assert result.exit_code == 0 + mock_create.assert_called_once() + + def test_dry_run_without_oauth_token_still_works(self): + """Dry-run only previews the config, so it does not require secrets.""" + result = runner.invoke( + app, + [ + "create", + "--agent", + "claude", + "--provider", + "anthropic-oauth", + "--dry-run", + ], + env={"CLAUDE_CODE_OAUTH_TOKEN": None}, + ) + assert result.exit_code == 0 + def test_codex_anthropic_oauth_rejected(self): """anthropic-oauth is not a valid provider for codex.""" result = runner.invoke( @@ -708,6 +752,7 @@ def test_explicit_mappings_and_extra_credentials_pass_to_create( "--agent-provider", "claude=anthropic,codex=openai", ], + env={"ANTHROPIC_API_KEY": "sk-ant", "OPENAI_API_KEY": "sk-oai"}, ) assert result.exit_code == 0 mock_create.assert_called_once() @@ -727,7 +772,9 @@ def test_single_agent_extra_provider_is_allowed(self, mock_prepare, mock_create) """An extra credential provider need not map to an agent.""" mock_prepare.return_value = ([], [], {}, False) result = runner.invoke( - app, ["create", "--agents", "claude", "--providers", "vertex,openai"] + app, + ["create", "--agents", "claude", "--providers", "vertex,openai"], + env={"OPENAI_API_KEY": "sk-oai"}, ) assert result.exit_code == 0 mock_create.assert_called_once() diff --git a/tests/test_providers.py b/tests/test_providers.py index 7d73d771..03a1541d 100644 --- a/tests/test_providers.py +++ b/tests/test_providers.py @@ -11,7 +11,12 @@ resolve_agent_provider, supported_providers, ) -from paude.providers.base import ProviderConfig, get_provider, list_providers +from paude.providers.base import ( + ProviderConfig, + check_required_secrets, + get_provider, + list_providers, +) class TestProviderRegistry: @@ -273,3 +278,37 @@ def test_results_are_sorted(self) -> None: for agent_name in AGENT_PROVIDERS: providers = supported_providers(agent_name) assert providers == sorted(providers) + + +class TestCheckRequiredSecrets: + """Tests for host-side validation of required provider secrets.""" + + def test_present_secret_passes(self) -> None: + check_required_secrets( + ["anthropic-oauth"], environ={"CLAUDE_CODE_OAUTH_TOKEN": "tok"} + ) + + def test_providers_without_required_secrets_pass(self) -> None: + check_required_secrets(["vertex", "chatgpt", "cursor", "google"], environ={}) + + def test_missing_oauth_token_names_var_and_hint(self) -> None: + with pytest.raises(ValueError, match="Missing required") as exc: + check_required_secrets(["anthropic-oauth"], environ={}) + message = str(exc.value) + assert "CLAUDE_CODE_OAUTH_TOKEN" in message + assert "anthropic-oauth" in message + assert "claude setup-token" in message + + def test_empty_value_counts_as_missing(self) -> None: + with pytest.raises(ValueError, match="OPENAI_API_KEY"): + check_required_secrets(["openai"], environ={"OPENAI_API_KEY": ""}) + + def test_reports_every_missing_provider(self) -> None: + with pytest.raises(ValueError, match="Missing required") as exc: + check_required_secrets( + ["anthropic", "vertex", "openai"], + environ={"ANTHROPIC_API_KEY": "k"}, + ) + message = str(exc.value) + assert "OPENAI_API_KEY" in message + assert "ANTHROPIC_API_KEY" not in message diff --git a/tests/test_upgrade.py b/tests/test_upgrade.py index d7c96400..3345082a 100644 --- a/tests/test_upgrade.py +++ b/tests/test_upgrade.py @@ -120,7 +120,7 @@ def test_upgrade_already_up_to_date_with_rebuild( def test_upgrade_auto_stops_running_session( self, mock_find: MagicMock, mock_upgrade_podman: MagicMock ) -> None: - """Session is running, upgrade should call stop_session first.""" + """A running session is handed to _upgrade_podman to stop after preflight.""" mock_backend = MagicMock() mock_backend.get_session.return_value = _make_session( "test-session", status="running", version="0.1.0" @@ -133,7 +133,7 @@ def test_upgrade_auto_stops_running_session( result = runner.invoke(app, ["upgrade", "test-session"]) assert result.exit_code == 0 - mock_backend.stop_session.assert_called_once_with("test-session") + assert mock_upgrade_podman.call_args.kwargs["stop_running"] is True @patch("paude.cli.upgrade._upgrade_podman") @patch("paude.cli.upgrade.find_session_backend") @@ -519,10 +519,16 @@ def test_upgrade_podman_preserves_volume( from paude.cli.upgrade import _upgrade_podman + up.backend.stop_session = MagicMock() # type: ignore[method-assign] _upgrade_podman( - "test-session", up.backend, rebuild=False, overrides=_NO_OVERRIDES + "test-session", + up.backend, + rebuild=False, + overrides=_NO_OVERRIDES, + stop_running=True, ) + up.backend.stop_session.assert_called_once_with("test-session") # Old container and proxy container removed up.runner.remove_container.assert_any_call("paude-test-session", force=True) up.runner.remove_container.assert_any_call( @@ -701,6 +707,38 @@ def test_upgrade_podman_removes_proxy( # Network removed up.networks.remove_network.assert_called_once_with("paude-net-test-session") + @patch("paude.container.ImageManager") + @patch("paude.config.detector.detect_config", return_value=None) + def test_upgrade_podman_missing_required_secret_changes_nothing( + self, + mock_detect_config: MagicMock, + mock_image_manager_class: MagicMock, + monkeypatch: pytest.MonkeyPatch, + ) -> None: + """Switching to anthropic-oauth without the token on the host fails + before the manifest is written, images are built, or anything is torn + down, so the existing session stays intact.""" + import typer + + from paude import upgrade_state + from paude.cli.upgrade import _upgrade_podman + + up = _upgrade_backend(self._make_container_labels()) + overrides = UpgradeOverrides(agent_providers={"claude": "anthropic-oauth"}) + + monkeypatch.delenv("CLAUDE_CODE_OAUTH_TOKEN", raising=False) + up.backend.stop_session = MagicMock() # type: ignore[method-assign] + with pytest.raises(typer.Exit): + _upgrade_podman( + "test-session", up.backend, True, overrides, stop_running=True + ) + + assert upgrade_state.load("test-session") is None + mock_image_manager_class.assert_not_called() + up.runner.remove_container.assert_not_called() + up.create_session.assert_not_called() + up.backend.stop_session.assert_not_called() + @patch("paude.mounts.build_mounts", return_value=[]) @patch("paude.cli.helpers._prepare_session_create") @patch("paude.container.ImageManager") @@ -1700,6 +1738,14 @@ def _run( "paude.backends.podman.helpers.find_container_by_session_name", return_value={"Labels": labels}, ), + patch.dict( + "os.environ", + { + "ANTHROPIC_API_KEY": "sk-ant", + "CLAUDE_CODE_OAUTH_TOKEN": "sk-ant-oat", + "OPENAI_API_KEY": "sk-oai", + }, + ), ): mock_image_manager = MagicMock() mock_image_manager.ensure_default_image.return_value = "paude:latest" From f331e34c56c7e8f11837699b11de04edc3941a36 Mon Sep 17 00:00:00 2001 From: Ben Browning <56071+bbrowning@users.noreply.github.com> Date: Tue, 29 Sep 2026 10:55:13 -0400 Subject: [PATCH 2/2] Set dummy OPENAI_API_KEY in codex openai swap test Upgrade now refuses to add a provider whose required secrets are missing from the host environment. The chatgpt->openai swap test adds the openai provider, so it failed in CI where OPENAI_API_KEY is unset. Provide a dummy key so the test exercises the swap itself. Co-Authored-By: Claude Opus 5.5 --- tests/integration/test_upgrade_podman.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/integration/test_upgrade_podman.py b/tests/integration/test_upgrade_podman.py index 04446bd7..c3a6d940 100644 --- a/tests/integration/test_upgrade_podman.py +++ b/tests/integration/test_upgrade_podman.py @@ -407,8 +407,11 @@ def test_upgrade_swap_codex_chatgpt_to_openai( unique_session_name: str, podman_test_image: str, podman_proxy_image: str, + monkeypatch: pytest.MonkeyPatch, ) -> None: """Swapping codex chatgpt->openai strips config and clears auth.json.""" + # Upgrade refuses to add a provider whose required secret is unset. + monkeypatch.setenv("OPENAI_API_KEY", "sk-test-dummy") backend = PodmanBackend() try: