From be3b4a2a86c0b62ad66bf36d293703af72f10399 Mon Sep 17 00:00:00 2001 From: tegwick Date: Tue, 7 Jul 2026 16:42:26 +0200 Subject: [PATCH] Fix warden access proxy for catalog fetch commands with shell pipes. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit resolve_fetch_command used shlex.split, which treated `|` as a literal argument — breaking reuse-surface-hub-write-token (kubectl | base64 -d). Piped commands now run via shell=True with inherited stdio for --fetch. --- src/warden/cli.py | 6 +-- src/warden/proxy.py | 93 +++++++++++++++++++++++++++++++++++-------- tests/test_doubles.py | 4 +- tests/test_proxy.py | 61 +++++++++++++++++++++++++--- 4 files changed, 137 insertions(+), 27 deletions(-) diff --git a/src/warden/cli.py b/src/warden/cli.py index eb1c120..de52646 100644 --- a/src/warden/cli.py +++ b/src/warden/cli.py @@ -964,7 +964,7 @@ def _access_proxy( err.print("[yellow]Proxying ungated[/yellow] (--no-policy; gate not enforced).") try: - argv = resolve_fetch_command(entry, domain=domain, field=field, path=path) + resolved = resolve_fetch_command(entry, domain=domain, field=field, path=path) except ProxyError as e: err.print(f"[red]{e}[/red]") raise typer.Exit(2) @@ -979,9 +979,9 @@ def _access_proxy( if not child_argv: err.print("[red]--exec needs a command after `--`[/red], e.g. `-- npm publish`.") raise typer.Exit(2) - rc = proxy_exec(argv, env_var=field or "", child_argv=child_argv) + rc = proxy_exec(resolved, env_var=field or "", child_argv=child_argv) else: - rc = proxy_fetch(argv) + rc = proxy_fetch(resolved) except ProxyError as e: err.print(f"[red]{e}[/red]") raise typer.Exit(5) diff --git a/src/warden/proxy.py b/src/warden/proxy.py index 65d1fff..cfa422a 100644 --- a/src/warden/proxy.py +++ b/src/warden/proxy.py @@ -25,6 +25,7 @@ import os import re import shlex import subprocess +from dataclasses import dataclass from datetime import datetime, timezone from pathlib import Path from typing import List, Optional @@ -34,18 +35,43 @@ from warden.routing.models import RouteEntry _PLACEHOLDER = re.compile(r"<[^>]+>") +@dataclass(frozen=True) +class ResolvedFetch: + """A catalog fetch command ready to run — either argv or a shell pipeline.""" + + argv: Optional[List[str]] = None + shell_cmd: Optional[str] = None + + def __post_init__(self) -> None: + if bool(self.argv) == bool(self.shell_cmd): + raise ValueError("exactly one of argv or shell_cmd must be set") + + class ProxyError(Exception): """Raised when a proxy fetch cannot be performed safely.""" +def _has_shell_pipe(cmd: str) -> bool: + """True when ``cmd`` contains an unquoted shell pipe operator.""" + in_single = in_double = False + for ch in cmd: + if ch == "'" and not in_double: + in_single = not in_single + elif ch == '"' and not in_single: + in_double = not in_double + elif ch == "|" and not in_single and not in_double: + return True + return False + + def resolve_fetch_command( entry: RouteEntry, *, domain: Optional[str] = None, field: Optional[str] = None, path: Optional[str] = None, -) -> List[str]: - """Build the concrete argv for an entry's fetch, or raise if under-specified. +) -> ResolvedFetch: + """Build the concrete fetch command for an entry, or raise if under-specified. Starts from the catalog ``fetch_command`` template (with ```` inlined), substitutes ````/```` and an explicit ``--path`` override, @@ -81,7 +107,10 @@ def resolve_fetch_command( "Supply --domain/--field (and --path for owner-side names) — warden will not " "guess owner-confirmed resource names." ) - return shlex.split(cmd) + if _has_shell_pipe(cmd): + # Catalog-reviewed pipelines (e.g. kubectl | base64 -d) need a shell. + return ResolvedFetch(shell_cmd=cmd) + return ResolvedFetch(argv=shlex.split(cmd)) def caller_auth_present(token_envs: tuple[str, ...] = ("VAULT_TOKEN", "BAO_TOKEN")) -> bool: @@ -146,25 +175,37 @@ def _caller_env() -> dict: return dict(os.environ) -def proxy_fetch(argv: List[str]) -> int: +def proxy_fetch(resolved: ResolvedFetch) -> int: """Run the owner's tool, streaming its output straight to the caller. stdout/stderr are **inherited** (``None``), never piped — the secret value flows subsystem → caller and is never read into warden's memory, buffer, or log (G2). Returns the tool's exit code. """ - completed = subprocess.run( # noqa: S603 — argv is shlex-split from a validated template - argv, - stdout=None, - stderr=None, - stdin=None, - env=_caller_env(), - check=False, - ) + env = _caller_env() + if resolved.argv is not None: + completed = subprocess.run( # noqa: S603 — argv is shlex-split from a validated template + resolved.argv, + stdout=None, + stderr=None, + stdin=None, + env=env, + check=False, + ) + else: + completed = subprocess.run( # noqa: S602 — shell_cmd is catalog-reviewed, not user input + resolved.shell_cmd, + shell=True, + stdout=None, + stderr=None, + stdin=None, + env=env, + check=False, + ) return completed.returncode -def proxy_exec(argv: List[str], *, env_var: str, child_argv: List[str]) -> int: +def proxy_exec(resolved: ResolvedFetch, *, env_var: str, child_argv: List[str]) -> int: """Fetch the value and inject it into a child command's environment only. The value transits warden's memory here (the accepted proxy tradeoff for `--exec`) @@ -175,10 +216,28 @@ def proxy_exec(argv: List[str], *, env_var: str, child_argv: List[str]) -> int: if not env_var: raise ProxyError("--exec requires --field (the env var name to inject), e.g. NPM_AUTH_TOKEN") - fetched = subprocess.run( # noqa: S603 - argv, stdout=subprocess.PIPE, stderr=None, stdin=None, - env=_caller_env(), check=False, text=True, - ) + env = _caller_env() + if resolved.argv is not None: + fetched = subprocess.run( # noqa: S603 + resolved.argv, + stdout=subprocess.PIPE, + stderr=None, + stdin=None, + env=env, + check=False, + text=True, + ) + else: + fetched = subprocess.run( # noqa: S602 + resolved.shell_cmd, + shell=True, + stdout=subprocess.PIPE, + stderr=None, + stdin=None, + env=env, + check=False, + text=True, + ) if fetched.returncode != 0: raise ProxyError( f"fetch failed (exit {fetched.returncode}) — check caller auth and the path." diff --git a/tests/test_doubles.py b/tests/test_doubles.py index 9b264fc..6a77ecc 100644 --- a/tests/test_doubles.py +++ b/tests/test_doubles.py @@ -107,8 +107,8 @@ def test_proxy_fetch_runs_fully_offline_against_double(tmp_path): fetch_command="bao kv get -field= ", exec_capable=True, ) - argv = resolve_fetch_command(entry, field="API_KEY", path="platform/x/y/z") + resolved = resolve_fetch_command(entry, field="API_KEY", path="platform/x/y/z") env = dict(os.environ, PATH=doubles_path_prepended(tmp_path)) # proxy_fetch inherits stdout; run it in a child so we can capture the stream. - result = subprocess.run(argv, capture_output=True, text=True, env=env, check=True) + result = subprocess.run(resolved.argv, capture_output=True, text=True, env=env, check=True) assert result.stdout.strip().startswith(SYNTHETIC_PREFIX) diff --git a/tests/test_proxy.py b/tests/test_proxy.py index 8c86ef0..5909fc3 100644 --- a/tests/test_proxy.py +++ b/tests/test_proxy.py @@ -11,6 +11,7 @@ from typer.testing import CliRunner from warden.cli import app from warden.proxy import ( ProxyError, + ResolvedFetch, caller_auth_present, proxy_exec, proxy_fetch, @@ -45,10 +46,11 @@ def _entry(**over) -> RouteEntry: # --- resolve_fetch_command ------------------------------------------------- def test_resolve_builds_argv(): - argv = resolve_fetch_command( + resolved = resolve_fetch_command( _entry(), domain="coulomb_social", field="NPM_AUTH_TOKEN", path="platform/x/y/z" ) - assert argv == ["bao", "kv", "get", "-field=NPM_AUTH_TOKEN", "platform/x/y/z"] + assert resolved.argv == ["bao", "kv", "get", "-field=NPM_AUTH_TOKEN", "platform/x/y/z"] + assert resolved.shell_cmd is None def test_resolve_refuses_unresolved_placeholder(): @@ -62,6 +64,18 @@ def test_resolve_refuses_non_exec_capable(): resolve_fetch_command(_entry(exec_capable=False, fetch_command=None)) +def test_resolve_piped_fetch_uses_shell_cmd(): + from warden.routing import load_catalog + + catalog = load_catalog(Path(__file__).resolve().parents[1] / "registry" / "routing" / "catalog.yaml") + entry = catalog.get("reuse-surface-hub-write-token") + resolved = resolve_fetch_command(entry) + assert resolved.argv is None + assert resolved.shell_cmd is not None + assert "| base64 -d" in resolved.shell_cmd + assert "reuse-surface-env" in resolved.shell_cmd + + # --- G2: transit-only fetch (inherited stdout) ----------------------------- def test_proxy_fetch_inherits_stdout_never_pipes(monkeypatch): @@ -72,13 +86,27 @@ def test_proxy_fetch_inherits_stdout_never_pipes(monkeypatch): return subprocess.CompletedProcess(argv, 0) monkeypatch.setattr("warden.proxy.subprocess.run", fake_run) - rc = proxy_fetch(["bao", "kv", "get", "x"]) + rc = proxy_fetch(ResolvedFetch(argv=["bao", "kv", "get", "x"])) assert rc == 0 # The value must never enter warden's memory — stdout is inherited, not piped. assert calls["stdout"] is None assert calls.get("stderr") is None +def test_proxy_fetch_shell_pipeline_inherits_stdio(monkeypatch): + calls = {} + + def fake_run(cmd, **kw): + calls.update(kw) + return subprocess.CompletedProcess(cmd, 0) + + monkeypatch.setattr("warden.proxy.subprocess.run", fake_run) + rc = proxy_fetch(ResolvedFetch(shell_cmd="kubectl get x | base64 -d")) + assert rc == 0 + assert calls["shell"] is True + assert calls["stdout"] is None + + # --- G1 + inject: exec injects value into child env, adds no warden token --- def test_proxy_exec_injects_only_into_child_env(monkeypatch): @@ -92,7 +120,11 @@ def test_proxy_exec_injects_only_into_child_env(monkeypatch): monkeypatch.setattr("warden.proxy.subprocess.run", fake_run) monkeypatch.delenv("NPM_AUTH_TOKEN", raising=False) - rc = proxy_exec(["bao", "kv", "get", "x"], env_var="NPM_AUTH_TOKEN", child_argv=["true"]) + rc = proxy_exec( + ResolvedFetch(argv=["bao", "kv", "get", "x"]), + env_var="NPM_AUTH_TOKEN", + child_argv=["true"], + ) assert rc == 0 # Value injected into child env (trailing newline stripped)… assert seen_env["NPM_AUTH_TOKEN"] == "SECRETVAL" @@ -100,9 +132,28 @@ def test_proxy_exec_injects_only_into_child_env(monkeypatch): assert "VAULT_TOKEN" not in {k for k in seen_env if k not in __import__("os").environ} +def test_proxy_exec_shell_pipeline_captures_stdout(monkeypatch): + seen_env = {} + + def fake_run(cmd, **kw): + if kw.get("shell"): + return subprocess.CompletedProcess(cmd, 0, stdout="PIPEVAL\n") + seen_env.update(kw["env"]) + return subprocess.CompletedProcess(cmd, 0) + + monkeypatch.setattr("warden.proxy.subprocess.run", fake_run) + rc = proxy_exec( + ResolvedFetch(shell_cmd="kubectl get x | base64 -d"), + env_var="REUSE_SURFACE_TOKEN", + child_argv=["true"], + ) + assert rc == 0 + assert seen_env["REUSE_SURFACE_TOKEN"] == "PIPEVAL" + + def test_proxy_exec_requires_env_var(): with pytest.raises(ProxyError, match="requires --field"): - proxy_exec(["bao"], env_var="", child_argv=["true"]) + proxy_exec(ResolvedFetch(argv=["bao"]), env_var="", child_argv=["true"]) # --- G1 caller auth detection ----------------------------------------------