diff --git a/src/warden/proxy.py b/src/warden/proxy.py index 2780eb2..1f7433f 100644 --- a/src/warden/proxy.py +++ b/src/warden/proxy.py @@ -12,8 +12,9 @@ intact. Three guardrails are enforced here in code: the tool with inherited stdout/stderr so the value never enters warden's memory; sanctioned exec/file transports hold it only for their bounded handoff. The high-risk attended-login lane is stricter: it captures every client byte inside - an isolated helper session, never returns that output, requires successful - persistence to a private token helper, self-revokes, and cleans up. Audit records + an isolated helper session, never returns that output, fails closed on any + non-allowlisted login output, requires successful persistence to a private token + helper, self-revokes, and cleans up. Audit records are metadata only. * **G3 — policy gate before fetch.** The CLI runs ``check_fetch_policy`` before calling anything here; this module refuses to run an unresolved command template. @@ -44,6 +45,45 @@ _OPENBAO_TOKEN = re.compile(rb"\b(?:hvs|hvb|hvr)\.[A-Za-z0-9_-]{8,}\b") _ATTENDED_LOGIN_ROOT = ".warden-attended-login" _TOKEN_HELPER_NAME = ".vault-token" +# Attended-login output allowlist (RPF-WP-0017 criterion 4). Structure only: the +# values are never logged, returned, stored, or included in an exception. +# +# stdout: empty, or exactly one JSON object shaped like a ``bao login -format=json`` +# response (api.Secret with an ``auth`` block). +_LOGIN_JSON_TOP_TYPES: dict = { + "request_id": (str,), + "lease_id": (str,), + "lease_duration": (int,), + "renewable": (bool,), + "data": (dict, type(None)), + "warnings": (list, type(None)), + "wrap_info": (dict, type(None)), + "mount_type": (str,), + "auth": (dict,), +} +_LOGIN_JSON_AUTH_TYPES: dict = { + "client_token": (str,), + "accessor": (str,), + "policies": (list, type(None)), + "token_policies": (list, type(None)), + "identity_policies": (list, type(None)), + "metadata": (dict, type(None)), + "orphan": (bool,), + "entity_id": (str,), + "lease_duration": (int,), + "renewable": (bool,), + "mfa_requirement": (dict, type(None)), + "num_uses": (int,), +} +# stderr: empty, or exactly the OpenBao OIDC CLI prompt (browser-launch or +# open-link variant, one https URL, optional wait line). Any other byte fails closed. +_LOGIN_OIDC_STDERR = re.compile( + r"\s*Complete the login via your OIDC provider\. " + r"(?:Launching browser to:|Open the following link in your browser:)" + r"[ \t]*\n\s*https://[^\s]+[ \t]*\n" + r"\s*(?:Waiting for OIDC authentication to complete\.\.\.[ \t]*\n?\s*)?" +) + @dataclass(frozen=True) class ResolvedFetch: @@ -349,6 +389,66 @@ def _output_bytes(completed: subprocess.CompletedProcess) -> bytes: return stdout + b"\n" + stderr +def _stream_bytes(value: object) -> bytes: + if isinstance(value, str): + return value.encode("utf-8", errors="replace") + return value if isinstance(value, bytes) else b"" + + +def _reject_duplicate_keys(pairs: list) -> dict: + keys = [key for key, _ in pairs] + if len(keys) != len(set(keys)): + raise ValueError("duplicate key") + return dict(pairs) + + +def _typed_keys_allowed(obj: dict, allowed: dict) -> bool: + for key, value in obj.items(): + types = allowed.get(key) + if types is None: + return False + # bool is an int subclass; never accept it where only int is expected. + if isinstance(value, bool) and bool not in types: + return False + if not isinstance(value, types): + return False + return True + + +def _login_stdout_allowed(raw: bytes) -> bool: + if not raw.strip(): + return True + try: + parsed = json.loads( + raw.decode("utf-8"), object_pairs_hook=_reject_duplicate_keys + ) + except (UnicodeDecodeError, ValueError, RecursionError): + return False + if not isinstance(parsed, dict) or "auth" not in parsed: + return False + if not _typed_keys_allowed(parsed, _LOGIN_JSON_TOP_TYPES): + return False + auth = parsed["auth"] + return "client_token" in auth and _typed_keys_allowed(auth, _LOGIN_JSON_AUTH_TYPES) + + +def _login_stderr_allowed(raw: bytes) -> bool: + if not raw: + return True + try: + text = raw.decode("utf-8") + except UnicodeDecodeError: + return False + return _LOGIN_OIDC_STDERR.fullmatch(text) is not None + + +def _login_output_allowed(completed: subprocess.CompletedProcess) -> bool: + """True only for allowlisted login output; checks structure, never values.""" + return _login_stdout_allowed(_stream_bytes(completed.stdout)) and ( + _login_stderr_allowed(_stream_bytes(completed.stderr)) + ) + + def _revoke_contained( bao_binary: str, *, @@ -397,8 +497,10 @@ def proxy_attended_login_exec( The default home is proven writable before the OIDC client starts. Login, child, and revocation output are captured and discarded. A successful login - may return client output only after the private helper has been populated; - persistence defects and non-zero results fail closed. The reviewed child must + proceeds only when the private helper is populated and its captured output is + empty or allowlisted (one ``-format=json`` login object on stdout, the OIDC + prompt on stderr); anything else, persistence defects, and non-zero results + fail closed with self-revocation. The reviewed child must remain silent. Any possibly issued token is revoked before the isolated helper directory is removed. """ @@ -450,14 +552,19 @@ def proxy_attended_login_exec( except (OSError, ProxyError): helper_valid = False - if login.returncode != 0 or not helper_valid: + output_allowed = _login_output_allowed(login) + if login.returncode != 0 or not helper_valid or not output_allowed: revoked = _revoke_contained( resolved.argv[0], env=env, possible_output=login_output ) status = "revoked" if revoked else "revocation could not be confirmed" + reason = ( + "" if output_allowed + else " (login client produced non-allowlisted output)" + ) raise AttendedLoginError( - "attended login failed closed before command handoff; any possible " - f"issued session was contained and {status}", + "attended login failed closed before command handoff" + f"{reason}; any possible issued session was contained and {status}", exit_code=10, outcome="login_failed", ) diff --git a/tests/test_proxy.py b/tests/test_proxy.py index 065f09a..4088788 100644 --- a/tests/test_proxy.py +++ b/tests/test_proxy.py @@ -10,6 +10,7 @@ from typer.testing import CliRunner from warden.cli import app from warden.proxy import ( + AttendedLoginError, ProxyError, ResolvedFetch, caller_auth_present, @@ -370,8 +371,9 @@ def test_cli_attended_result_and_both_audits(monkeypatch, tmp_path, phase, code, raise OSError("client unavailable") if phase != "missing_helper": helper.write_bytes(sentinel) + login_json = json.dumps({"auth": {"client_token": sentinel.decode()}}) return subprocess.CompletedProcess( - argv, 1 if phase == "login_failed" else 0, sentinel, b"" + argv, 1 if phase == "login_failed" else 0, login_json.encode(), b"" ) if argv == ["reviewed-child"]: if phase == "child_start": @@ -508,8 +510,8 @@ def test_attended_login_contained_success_output_never_escapes(monkeypatch, tmp_ return subprocess.CompletedProcess( argv, 0, - stdout=json.dumps({"auth": {"client_token": sentinel}}).encode(), - stderr=b"", + stdout=json.dumps(_login_json(sentinel)).encode(), + stderr=_OIDC_PROMPT, ) if argv == ["reviewed-child"]: child_ran = True @@ -558,6 +560,159 @@ def test_attended_login_preserves_caller_warden_config_for_reviewed_child( assert seen_config == str(caller_config) +_OIDC_PROMPT = ( + b"Complete the login via your OIDC provider. Launching browser to:\n\n" + b" https://keycape.example.invalid/auth?state=x&nonce=y\n\n\n" + b"Waiting for OIDC authentication to complete...\n" +) + + +def _login_json(token: str) -> dict: + return { + "request_id": "req", + "lease_id": "", + "lease_duration": 0, + "renewable": False, + "data": None, + "warnings": None, + "mount_type": "", + "auth": { + "client_token": token, + "accessor": "acc", + "policies": ["default"], + "token_policies": ["default"], + "identity_policies": None, + "metadata": {"role": "r"}, + "orphan": True, + "entity_id": "ent", + "lease_duration": 3600, + "renewable": True, + "mfa_requirement": None, + "num_uses": 0, + }, + } + + +def _run_populated_login(monkeypatch, tmp_path, *, stdout: bytes, stderr: bytes): + """Login exits 0 with a populated private helper but the given output.""" + monkeypatch.setattr(Path, "home", lambda: tmp_path) + calls = [] + + def fake_run(argv, **kw): + calls.append(argv) + if argv[:2] == ["bao", "login"]: + helper = Path(kw["env"]["HOME"]) / ".vault-token" + helper.write_text("hvs.NONPRODUCTION_HELPER_VALUE") + helper.chmod(0o600) + return subprocess.CompletedProcess(argv, 0, stdout=stdout, stderr=stderr) + if argv[:3] == ["bao", "token", "revoke"]: + return subprocess.CompletedProcess(argv, 0, stdout=b"", stderr=b"") + raise AssertionError("reviewed child ran after non-allowlisted login output") + + monkeypatch.setattr("warden.proxy.subprocess.run", fake_run) + return calls + + +_LOGIN_SENTINEL = "hvs.NONPRODUCTION_UNEXPECTED_LOGIN" + + +@pytest.mark.parametrize( + "stdout,stderr", + [ + # unexpected stdout text + (_LOGIN_SENTINEL.encode(), b""), + # unexpected stderr text + (b"", _LOGIN_SENTINEL.encode()), + # unexpected stderr appended to an otherwise valid OIDC prompt + (b"", _OIDC_PROMPT + _LOGIN_SENTINEL.encode()), + # valid JSON followed by extra text + (json.dumps(_login_json(_LOGIN_SENTINEL)).encode() + b"\ntrailing", b""), + # two JSON objects + ((json.dumps(_login_json(_LOGIN_SENTINEL)) * 2).encode(), b""), + # unexpected top-level key + (json.dumps({**_login_json(_LOGIN_SENTINEL), "extra": _LOGIN_SENTINEL}).encode(), b""), + # unexpected auth key + (json.dumps({"auth": {"client_token": _LOGIN_SENTINEL, "x": 1}}).encode(), b""), + # wrong value type + (json.dumps({"auth": {"client_token": _LOGIN_SENTINEL, "num_uses": True}}).encode(), b""), + # JSON without auth block / non-object JSON + (json.dumps({"data": {"v": _LOGIN_SENTINEL}}).encode(), b""), + (json.dumps([_LOGIN_SENTINEL]).encode(), b""), + # duplicate keys + (b'{"auth": {"client_token": "a"}, "auth": {"client_token": "b"}}', b""), + # malformed JSON + (b'{"auth": {"client_token": "' + _LOGIN_SENTINEL.encode() + b'"', b""), + # undecodable bytes + (b"\xff\xfe", b""), + ], + ids=[ + "stdout-text", "stderr-text", "stderr-after-prompt", "json-trailing", + "json-twice", "top-extra-key", "auth-extra-key", "wrong-type", + "no-auth", "json-array", "duplicate-key", "malformed-json", "non-utf8", + ], +) +def test_attended_login_non_allowlisted_output_with_populated_helper_fails_closed( + monkeypatch, tmp_path, capsys, stdout, stderr +): + calls = _run_populated_login(monkeypatch, tmp_path, stdout=stdout, stderr=stderr) + with pytest.raises(AttendedLoginError, match="non-allowlisted output") as exc: + proxy_attended_login_exec( + ResolvedFetch(argv=["bao", "login", "-no-print"]), + child_argv=["should-not-run"], + ) + assert exc.value.exit_code == 10 + assert exc.value.outcome == "login_failed" + captured = capsys.readouterr() + assert _LOGIN_SENTINEL not in str(exc.value) + assert _LOGIN_SENTINEL not in captured.out + assert _LOGIN_SENTINEL not in captured.err + # No retry: exactly one login, then self-revocation through the populated + # helper; the reviewed child never runs. + assert calls == [ + ["bao", "login", "-no-print", "-format=json"], + ["bao", "token", "revoke", "-self"], + ] + assert not (tmp_path / ".warden-attended-login").exists() + + +@pytest.mark.parametrize( + "stdout,stderr", + [ + (b"", b""), + (b"", _OIDC_PROMPT), + (json.dumps(_login_json("hvs.NONPRODUCTION_OK")).encode() + b"\n", b""), + (json.dumps({"auth": {"client_token": "hvs.NONPRODUCTION_OK"}}).encode(), b""), + ( + b"", + b"Complete the login via your OIDC provider. Open the following link " + b"in your browser:\n\n https://keycape.example.invalid/auth?s=1\n\n\n", + ), + ], + ids=["empty", "oidc-prompt", "full-json", "minimal-json", "open-link-prompt"], +) +def test_attended_login_allowlisted_output_hands_off( + monkeypatch, tmp_path, stdout, stderr +): + monkeypatch.setattr(Path, "home", lambda: tmp_path) + calls = [] + + def fake_run(argv, **kw): + calls.append(argv) + if argv[:2] == ["bao", "login"]: + helper = Path(kw["env"]["HOME"]) / ".vault-token" + helper.write_text("hvs.NONPRODUCTION_HELPER_VALUE") + helper.chmod(0o600) + return subprocess.CompletedProcess(argv, 0, stdout=stdout, stderr=stderr) + return subprocess.CompletedProcess(argv, 0, stdout=b"", stderr=b"") + + monkeypatch.setattr("warden.proxy.subprocess.run", fake_run) + assert proxy_attended_login_exec( + ResolvedFetch(argv=["bao", "login", "-no-print"]), + child_argv=["reviewed-child"], + ) == 0 + assert calls[1] == ["reviewed-child"] + + def test_real_catalog_login_entry_is_login_lane(): from warden.routing import load_catalog e = load_catalog(_repo_catalog()).get("key-cape-oidc-login") diff --git a/workplans/WARDEN-WP-0036-attended-login-openbao-output.md b/workplans/WARDEN-WP-0036-attended-login-openbao-output.md index 44c3461..aba808c 100644 --- a/workplans/WARDEN-WP-0036-attended-login-openbao-output.md +++ b/workplans/WARDEN-WP-0036-attended-login-openbao-output.md @@ -8,7 +8,7 @@ status: finished owner: codex topic_slug: attended-login-openbao-output created: "2026-09-01" -updated: "2026-09-28" +updated: "2026-10-14" state_hub_workstream_id: "d844c96e-152d-53fa-bff6-e072125ef66c" --- @@ -66,3 +66,41 @@ OIDC, credential read or production operation was used. The earlier September 1 live operation remains historical evidence, not a live validation of this change. Automated endpoint/browser preflight and OIDC URL presentation remain optional inbox suggestions, outside these completed handoff acceptance criteria. + +### 2026-10-14 login-output fail-closed regression and fix (existing T01) + +railiance-platform's RPF-WP-0017 re-acceptance review +(`railiance-platform/reviews/RPF-WP-0017-ops-warden-reacceptance-2026-10-14.json`, +decision `reject`) found that T01's commit `b4c1d39` (2026-09-01) dropped +`login_output.strip()` from the login fail-closed condition. From then until this +fix, a login that exited 0 with a populated helper proceeded to the child whatever +it wrote to stdout/stderr. Output stayed contained (criterion 3 held) but +criterion 4 (non-allowlisted output fails closed with self-revocation) and the +criterion 5 unexpected-output coverage regressed; the existing unexpected-output +test passed only through the empty-helper persistence path. + +Fix: login output is still captured and never forwarded, and is now accepted only +when allowlisted by structure (values are never logged, returned, stored, or put +in exceptions/audit): + +- stdout: empty, or exactly one JSON object with an `auth` object containing + `client_token`; top-level keys limited to `request_id, lease_id, + lease_duration, renewable, data, warnings, wrap_info, mount_type, auth`, auth + keys to `client_token, accessor, policies, token_policies, identity_policies, + metadata, orphan, entity_id, lease_duration, renewable, mfa_requirement, + num_uses`, each type-checked; duplicate keys rejected. +- stderr: empty, or exactly the OpenBao OIDC CLI prompt ("Complete the login via + your OIDC provider. Launching browser to:" or "... Open the following link in + your browser:", one `https://` URL, optional "Waiting for OIDC authentication to + complete..."). These prompt strings were confirmed present in the local `bao` + binary; this stderr allowlist entry is an ops-warden proposal pending + railiance-platform review under RPF-WP-0017. + +Anything else (extra text, other stderr, multiple objects, unexpected keys or +types, parse/decoding failure) fails closed: self-revocation through the existing +path, helper cleanup, exit 10 / `login_failed`, no retry. Added a parametrized +populated-helper test for 13 non-allowlisted shapes and an allowlisted hand-off +test; the success test now uses a full JSON shape plus the OIDC prompt. The +September 1 T02 live operation was recorded in `9770d6a` (00:51 +0200), after +`b4c1d39` (00:13 +0200) and `8f01eef` (00:24 +0200); `proxy.py` at that time +contained the relaxed check (the installed CLI's exact revision was not recorded). No live OIDC, OpenBao call, or credential read was used for this fix.