Fix warden access proxy for catalog fetch commands with shell pipes.

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.
This commit is contained in:
2026-07-07 16:42:26 +02:00
parent 7d915a2eb5
commit be3b4a2a86
4 changed files with 137 additions and 27 deletions

View File

@@ -964,7 +964,7 @@ def _access_proxy(
err.print("[yellow]Proxying ungated[/yellow] (--no-policy; gate not enforced).") err.print("[yellow]Proxying ungated[/yellow] (--no-policy; gate not enforced).")
try: 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: except ProxyError as e:
err.print(f"[red]{e}[/red]") err.print(f"[red]{e}[/red]")
raise typer.Exit(2) raise typer.Exit(2)
@@ -979,9 +979,9 @@ def _access_proxy(
if not child_argv: if not child_argv:
err.print("[red]--exec needs a command after `--`[/red], e.g. `-- npm publish`.") err.print("[red]--exec needs a command after `--`[/red], e.g. `-- npm publish`.")
raise typer.Exit(2) 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: else:
rc = proxy_fetch(argv) rc = proxy_fetch(resolved)
except ProxyError as e: except ProxyError as e:
err.print(f"[red]{e}[/red]") err.print(f"[red]{e}[/red]")
raise typer.Exit(5) raise typer.Exit(5)

View File

@@ -25,6 +25,7 @@ import os
import re import re
import shlex import shlex
import subprocess import subprocess
from dataclasses import dataclass
from datetime import datetime, timezone from datetime import datetime, timezone
from pathlib import Path from pathlib import Path
from typing import List, Optional from typing import List, Optional
@@ -34,18 +35,43 @@ from warden.routing.models import RouteEntry
_PLACEHOLDER = re.compile(r"<[^>]+>") _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): class ProxyError(Exception):
"""Raised when a proxy fetch cannot be performed safely.""" """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( def resolve_fetch_command(
entry: RouteEntry, entry: RouteEntry,
*, *,
domain: Optional[str] = None, domain: Optional[str] = None,
field: Optional[str] = None, field: Optional[str] = None,
path: Optional[str] = None, path: Optional[str] = None,
) -> List[str]: ) -> ResolvedFetch:
"""Build the concrete argv for an entry's fetch, or raise if under-specified. """Build the concrete fetch command for an entry, or raise if under-specified.
Starts from the catalog ``fetch_command`` template (with ``<path_template>`` Starts from the catalog ``fetch_command`` template (with ``<path_template>``
inlined), substitutes ``<domain>``/``<FIELD>`` and an explicit ``--path`` override, inlined), substitutes ``<domain>``/``<FIELD>`` 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 " "Supply --domain/--field (and --path for owner-side names) — warden will not "
"guess owner-confirmed resource names." "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: 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) 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. """Run the owner's tool, streaming its output straight to the caller.
stdout/stderr are **inherited** (``None``), never piped — the secret value flows 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). subsystem → caller and is never read into warden's memory, buffer, or log (G2).
Returns the tool's exit code. Returns the tool's exit code.
""" """
completed = subprocess.run( # noqa: S603 — argv is shlex-split from a validated template env = _caller_env()
argv, if resolved.argv is not None:
stdout=None, completed = subprocess.run( # noqa: S603 — argv is shlex-split from a validated template
stderr=None, resolved.argv,
stdin=None, stdout=None,
env=_caller_env(), stderr=None,
check=False, 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 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. """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`) 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: if not env_var:
raise ProxyError("--exec requires --field (the env var name to inject), e.g. NPM_AUTH_TOKEN") raise ProxyError("--exec requires --field (the env var name to inject), e.g. NPM_AUTH_TOKEN")
fetched = subprocess.run( # noqa: S603 env = _caller_env()
argv, stdout=subprocess.PIPE, stderr=None, stdin=None, if resolved.argv is not None:
env=_caller_env(), check=False, text=True, 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: if fetched.returncode != 0:
raise ProxyError( raise ProxyError(
f"fetch failed (exit {fetched.returncode}) — check caller auth and the path." f"fetch failed (exit {fetched.returncode}) — check caller auth and the path."

View File

@@ -107,8 +107,8 @@ def test_proxy_fetch_runs_fully_offline_against_double(tmp_path):
fetch_command="bao kv get -field=<FIELD> <path_template>", fetch_command="bao kv get -field=<FIELD> <path_template>",
exec_capable=True, 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)) 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. # 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) assert result.stdout.strip().startswith(SYNTHETIC_PREFIX)

View File

@@ -11,6 +11,7 @@ from typer.testing import CliRunner
from warden.cli import app from warden.cli import app
from warden.proxy import ( from warden.proxy import (
ProxyError, ProxyError,
ResolvedFetch,
caller_auth_present, caller_auth_present,
proxy_exec, proxy_exec,
proxy_fetch, proxy_fetch,
@@ -45,10 +46,11 @@ def _entry(**over) -> RouteEntry:
# --- resolve_fetch_command ------------------------------------------------- # --- resolve_fetch_command -------------------------------------------------
def test_resolve_builds_argv(): 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" _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(): 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)) 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) ----------------------------- # --- G2: transit-only fetch (inherited stdout) -----------------------------
def test_proxy_fetch_inherits_stdout_never_pipes(monkeypatch): 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) return subprocess.CompletedProcess(argv, 0)
monkeypatch.setattr("warden.proxy.subprocess.run", fake_run) 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 assert rc == 0
# The value must never enter warden's memory — stdout is inherited, not piped. # The value must never enter warden's memory — stdout is inherited, not piped.
assert calls["stdout"] is None assert calls["stdout"] is None
assert calls.get("stderr") 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 --- # --- G1 + inject: exec injects value into child env, adds no warden token ---
def test_proxy_exec_injects_only_into_child_env(monkeypatch): 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.setattr("warden.proxy.subprocess.run", fake_run)
monkeypatch.delenv("NPM_AUTH_TOKEN", raising=False) 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 assert rc == 0
# Value injected into child env (trailing newline stripped)… # Value injected into child env (trailing newline stripped)…
assert seen_env["NPM_AUTH_TOKEN"] == "SECRETVAL" 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} 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(): def test_proxy_exec_requires_env_var():
with pytest.raises(ProxyError, match="requires --field"): 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 ---------------------------------------------- # --- G1 caller auth detection ----------------------------------------------