From 4c0f3d30489a1c13f7a7815966a11ff3afce1092 Mon Sep 17 00:00:00 2001 From: Christopher <131207452+PetrefiedThunder@users.noreply.github.com> Date: Mon, 26 Oct 2026 01:32:34 -0700 Subject: [PATCH 1/4] fix(security): never print full partner API key in bootstrap script (code-scanning #13 #14 #15) --- gateway/docs/constant-test-loop.md | 52 ++++---- gateway/docs/partner-api-key-bootstrap.md | 12 +- gateway/scripts/partner_api_key_bootstrap.py | 81 ++++++++++-- gateway/tests/test_partner_bootstrap_flags.py | 116 ++++++++++++++---- 4 files changed, 192 insertions(+), 69 deletions(-) diff --git a/gateway/docs/constant-test-loop.md b/gateway/docs/constant-test-loop.md index 24e1867d..4b9ad812 100644 --- a/gateway/docs/constant-test-loop.md +++ b/gateway/docs/constant-test-loop.md @@ -68,69 +68,65 @@ looks like the trust plane failing. Set `CI_SMOKE_AGENT_KEY` for a pre-provisioned agent credential. Optionally provide `CI_SMOKE_WALLET_ID` and `CI_SMOKE_KEY_ID` for faster startup (the script will fetch them from the API if not provided): ```bash -# Provision an agent key once (using bootstrap key) and extract credentials in one pipeline +# Provision an agent key once (using bootstrap key). The minted secret is +# never printed; --key-file writes it (mode 0600, path must not already exist). export BOOTSTRAP_KEY="amw_live_..." -# Provision once; keep every identity field from the same response. -# Disable shell tracing while handling credentials. Do not print this JSON. set +x +KEY_FILE="${TMPDIR:-/tmp}/amw-ci-smoke-key.$$" PROVISION_JSON="$(python scripts/partner_api_key_bootstrap.py \ --api-url https://api.thisisatest.tech \ --agent-id ci-smoke-agent \ --key-name constant-test-loop \ --budget-credits 5000 \ - --json)" -export CI_SMOKE_AGENT_KEY="$(printf '%s' "$PROVISION_JSON" | jq -er .api_key)" + --json \ + --key-file "$KEY_FILE")" +export CI_SMOKE_AGENT_KEY="$(cat "$KEY_FILE")" export CI_SMOKE_WALLET_ID="$(printf '%s' "$PROVISION_JSON" | jq -er .wallet_id)" export CI_SMOKE_KEY_ID="$(printf '%s' "$PROVISION_JSON" | jq -er .key_id)" +rm -f "$KEY_FILE" unset PROVISION_JSON -# Or use a restrictive temporary file if needed -KEY_FILE="$(mktemp)" -umask 077 -python scripts/partner_api_key_bootstrap.py \ - --api-url https://api.thisisatest.tech \ - --agent-id ci-smoke-agent \ - --key-name constant-test-loop \ - --budget-credits 5000 \ - --json > "$KEY_FILE" -chmod 600 "$KEY_FILE" -export CI_SMOKE_AGENT_KEY="$(jq -r .api_key "$KEY_FILE")" -export CI_SMOKE_WALLET_ID="$(jq -r .wallet_id "$KEY_FILE")" -export CI_SMOKE_KEY_ID="$(jq -r .key_id "$KEY_FILE")" -rm "$KEY_FILE" # Clean up immediately - # Run the constant test API_URL=https://api.thisisatest.tech python scripts/constant_test_loop.py ``` ## Machine-Readable Bootstrap Output -`partner_api_key_bootstrap.py --json` prints JSON to stdout so the minted key can be piped directly: +`partner_api_key_bootstrap.py --json` prints JSON to stdout with `api_key` +replaced by `api_key_masked`. Persist the secret with `--key-file` (do not +pre-create the path; the script refuses an existing file): ```bash -# Pipe to jq to extract the agent API key +KEY_FILE="${TMPDIR:-/tmp}/amw-ci-smoke-key.$$" + +# JSON identity fields on stdout; full key only in KEY_FILE BOOTSTRAP_KEY="..." python scripts/partner_api_key_bootstrap.py \ --api-url https://api.thisisatest.tech \ --agent-id ci-smoke-agent \ --key-name test-key \ --budget-credits 5000 \ - --json | jq -r .api_key + --json \ + --key-file "$KEY_FILE" -# Or pipe directly to gh secret set +# Or print only the key-file path, then load it into a secret store +KEY_FILE="${TMPDIR:-/tmp}/amw-ci-smoke-key.$$" BOOTSTRAP_KEY="..." python scripts/partner_api_key_bootstrap.py \ --api-url https://api.thisisatest.tech \ --agent-id ci-smoke-agent \ --key-name constant-test-loop \ --budget-credits 5000 \ - --json | jq -r .api_key | \ - gh secret set CI_SMOKE_AGENT_KEY --repo agentmiddleware/agent-middleware + --key-only \ + --key-file "$KEY_FILE" +gh secret set CI_SMOKE_AGENT_KEY --repo agentmiddleware/agent-middleware < "$KEY_FILE" +rm -f "$KEY_FILE" ``` ### Security Properties - Human/status text goes to stderr (never stdout in `--json` mode) - Bootstrap/admin key is never printed to stdout or stderr +- The minted agent key is never printed to stdout or stderr; `--key-file` stores it - Agent key is never logged or printed during constant test execution - All secrets read from environment variables, never hardcoded @@ -147,9 +143,9 @@ pytest tests/test_constant_test_loop.py -v ``` Covers: -- `--json` mode produces pipeable stdout +- `--json` mode produces pipeable stdout (`api_key_masked`, not `api_key`) - Bootstrap key never leaks to stdout or stderr -- `jq -r .api_key` pipeline works +- `--key-file` persists the minted secret at mode 0600 - Constant test loop runs against local instance - Self-provisioning when no key provided - Agent key never logged during execution diff --git a/gateway/docs/partner-api-key-bootstrap.md b/gateway/docs/partner-api-key-bootstrap.md index 061aadcc..63683581 100644 --- a/gateway/docs/partner-api-key-bootstrap.md +++ b/gateway/docs/partner-api-key-bootstrap.md @@ -20,13 +20,16 @@ Never commit `VALID_API_KEYS` to git. export API_URL="${PUBLIC_URL:-https://api.thisisatest.tech}" export BOOTSTRAP_KEY=... # from secret manager / Railway variables — not a demo string -# One-shot provisioner (prints agent key once to stdout): +# One-shot provisioner. The minted secret is never printed; it is written +# to --key-file (created mode 0600, refused if the path already exists). +# Without --key-file, stdout shows only a masked form. uv run --with-requirements requirements.txt \ python scripts/partner_api_key_bootstrap.py \ --api-url "$API_URL" \ --sponsor-name "Partner Co" \ --agent-id "partner-agent-001" \ - --budget-credits 1000 + --budget-credits 1000 \ + --key-file "$HOME/.config/amw/partner-agent-001.key" ``` Optional bounds on what you mint (all default to unlimited): @@ -43,10 +46,11 @@ uv run --with-requirements requirements.txt \ python scripts/partner_api_key_bootstrap.py \ --api-url "$API_URL" \ --sponsor-wallet-id "$SPONSOR_WALLET_ID" \ - --agent-wallet-id "$AGENT_WALLET_ID" + --agent-wallet-id "$AGENT_WALLET_ID" \ + --key-file "$HOME/.config/amw/partner-agent-001.key" ``` -Hand the **agent** key (and wallet id) to the partner over a secure channel. +Read the agent key from that file and hand it (and the wallet id) to the partner over a secure channel. Revoke when the engagement ends: ```bash diff --git a/gateway/scripts/partner_api_key_bootstrap.py b/gateway/scripts/partner_api_key_bootstrap.py index d3f49dd0..573b34d6 100644 --- a/gateway/scripts/partner_api_key_bootstrap.py +++ b/gateway/scripts/partner_api_key_bootstrap.py @@ -4,7 +4,10 @@ Uses a bootstrap/admin key (VALID_API_KEYS) only to provision: sponsor wallet → agent wallet → DB-scoped agent API key. -The agent secret is printed once. Requires network access to --api-url. +The minted agent secret is never printed to stdout or stderr. Pass --key-file +PATH to write it (created mode 0600, fails if PATH exists); otherwise only a +masked form is shown (first 8 chars of the key prefix area + "..." + last 4). +Requires network access to --api-url. Does not mint keys without a bootstrap secret (fail closed). Resume: pass --sponsor-wallet-id / --agent-wallet-id to skip earlier creates @@ -28,6 +31,32 @@ def _require(condition: bool, message: str) -> None: raise SystemExit(f"error: {message}") +def _mask_api_key(key: str) -> str: + """Return a display form that is not the full secret. + + Uses the first 8 characters of the key prefix area, an ellipsis, and + the last 4 characters. + """ + return f"{key[:8]}...{key[-4:]}" + + +def _write_key_file(path: str, api_key: str) -> None: + """Write the minted key to PATH created 0600; refuse if it already exists.""" + flags = os.O_CREAT | os.O_EXCL | os.O_WRONLY + try: + fd = os.open(path, flags, 0o600) + except FileExistsError: + raise SystemExit(f"error: --key-file {path} already exists") + except OSError as exc: + raise SystemExit(f"error: cannot write --key-file {path}: {exc}") from exc + try: + os.write(fd, api_key.encode("utf-8")) + except OSError as exc: + raise SystemExit(f"error: cannot write --key-file {path}: {exc}") from exc + finally: + os.close(fd) + + def _require_safe_api_url(api_url: str) -> str: """Require HTTPS for remote hosts; allow http:// for loopback only.""" base = api_url.strip().rstrip("/") @@ -92,7 +121,8 @@ def provision( Args: json_mode: If True, print only machine-readable status to stderr (never the api_key or bootstrap_key). Human text goes to stderr so - `jq -r .api_key` works on stdout JSON output. + stdout JSON stays pipeable. The minted secret is never in that JSON; + use --key-file to persist it. """ base = _require_safe_api_url(api_url) headers = { @@ -153,7 +183,7 @@ def provision( key_body["max_uses"] = max_uses key = _post(client, "/v1/api-keys", key_body) api_key = key.get("api_key") - _require(bool(api_key), "api_key missing (shown only once)") + _require(bool(api_key), "api_key missing (write with --key-file; never printed)") return { "api_url": base, @@ -217,15 +247,27 @@ def main() -> int: parser.add_argument( "--json", action="store_true", - help="Print full JSON including api_key (default: human summary)", + help=( + "Print JSON to stdout with api_key replaced by api_key_masked " + "(and key_file when --key-file is given). Default: human summary." + ), ) parser.add_argument( "--key-only", action="store_true", help=( - "Print only the minted api_key to stdout. Lets the secret be " - "piped straight into a secret store without a jq dependency, and " - "without the surrounding metadata landing wherever stdout goes." + "Print only the --key-file path, or the masked key when --key-file " + "is omitted. Never prints the full minted secret." + ), + ) + parser.add_argument( + "--key-file", + default=None, + metavar="PATH", + help=( + "Write the minted api_key to PATH (created mode 0600 with " + "O_CREAT|O_EXCL|O_WRONLY; refused if PATH already exists). " + "Stdout prints PATH in --key-only mode, and JSON includes key_file." ), ) args = parser.parse_args() @@ -233,6 +275,9 @@ def main() -> int: if args.key_only and args.json: raise SystemExit("error: --key-only and --json are mutually exclusive") + if args.key_file and os.path.exists(args.key_file): + raise SystemExit(f"error: --key-file {args.key_file} already exists") + if args.agent_wallet_id and not args.sponsor_wallet_id: raise SystemExit( "error: --agent-wallet-id resume requires --sponsor-wallet-id " @@ -255,21 +300,33 @@ def main() -> int: json_mode=args.json, ) + api_key = str(result["api_key"]) + masked = _mask_api_key(api_key) + key_file = args.key_file + if key_file: + _write_key_file(key_file, api_key) + if args.key_only: - print(result["api_key"]) + print(key_file if key_file else masked) elif args.json: # Machine-readable: JSON to stdout, all status to stderr. - # Never print bootstrap_key. This lets `jq -r .api_key` work. - print(json.dumps(result, indent=2)) + # Never print bootstrap_key or the full minted api_key. + payload = {k: v for k, v in result.items() if k != "api_key"} + payload["api_key_masked"] = masked + if key_file: + payload["key_file"] = key_file + print(json.dumps(payload, indent=2)) else: - # Human-readable summary to stdout (legacy behavior). + # Human-readable summary to stdout. Masked key only. print("Partner API key bootstrap OK") print(f" api_url: {result['api_url']}") print(f" sponsor_wallet_id: {result['sponsor_wallet_id']}") print(f" agent_wallet_id: {result['agent_wallet_id']}") print(f" key_id: {result['key_id']}") print(f" key_prefix: {result['key_prefix']}") - print(f" api_key (once): {result['api_key']}") + print(f" api_key (masked): {masked}") + if key_file: + print(f" key_file: {key_file}") return 0 diff --git a/gateway/tests/test_partner_bootstrap_flags.py b/gateway/tests/test_partner_bootstrap_flags.py index 5c36bce7..33cabafe 100644 --- a/gateway/tests/test_partner_bootstrap_flags.py +++ b/gateway/tests/test_partner_bootstrap_flags.py @@ -1,16 +1,26 @@ """Operator bounds on partner_api_key_bootstrap: --daily-limit, --expires-in-days, --max-uses reach the right request bodies, and stay -absent when not requested (so older servers see unchanged payloads).""" +absent when not requested (so older servers see unchanged payloads). + +CLI modes never print the minted secret; --key-file writes it at mode 0600. +""" from __future__ import annotations import json +import stat import sys import pytest import scripts.partner_api_key_bootstrap as bootstrap +# Test-generated minted secret. Long enough that the masked form is not a +# substring of the full key and the full key is not a substring of the mask. +MINTED_KEY = "b2a_test_generated_key_wxyz" +MASKED_KEY = "b2a_test...wxyz" +BOOTSTRAP_SECRET = "synthetic-bootstrap-secret" + class _FakeResponse: def __init__(self, payload: dict): @@ -44,7 +54,7 @@ def post(self, path, json): if path == "/v1/billing/wallets/agent": return _FakeResponse({"wallet_id": "wal_agent"}) return _FakeResponse( - {"api_key": "b2a_test", "key_id": "key_1", "key_prefix": "b2a_test"} + {"api_key": MINTED_KEY, "key_id": "key_1", "key_prefix": "b2a_test"} ) @@ -72,6 +82,26 @@ def factory(**kwargs): return result, dict(captured["client"].posts) +def _cli_args(*extra: str) -> list[str]: + return [ + "partner_api_key_bootstrap.py", + "--api-url", + "https://api.example.test", + "--sponsor-name", + "Synthetic Partner", + "--agent-id", + "synthetic-agent", + *extra, + ] + + +def _run_main(monkeypatch, extra: list[str]) -> int: + monkeypatch.setenv("BOOTSTRAP_KEY", BOOTSTRAP_SECRET) + monkeypatch.setattr(bootstrap.httpx, "Client", _FakeClient) + monkeypatch.setattr(sys, "argv", _cli_args(*extra)) + return bootstrap.main() + + def test_bounds_flags_reach_request_bodies(monkeypatch): result, posts = _provision( monkeypatch, @@ -82,7 +112,7 @@ def test_bounds_flags_reach_request_bodies(monkeypatch): assert posts["/v1/billing/wallets/agent"]["daily_limit"] == 250.0 assert posts["/v1/api-keys"]["expires_in_days"] == 30 assert posts["/v1/api-keys"]["max_uses"] == 1000 - assert result["api_key"] == "b2a_test" + assert result["api_key"] == MINTED_KEY def test_bounds_default_to_absent(monkeypatch): @@ -92,33 +122,69 @@ def test_bounds_default_to_absent(monkeypatch): assert "max_uses" not in posts["/v1/api-keys"] +def test_mask_form_is_prefix8_ellipsis_last4(): + assert bootstrap._mask_api_key(MINTED_KEY) == MASKED_KEY + assert MASKED_KEY == f"{MINTED_KEY[:8]}...{MINTED_KEY[-4:]}" + assert MINTED_KEY not in MASKED_KEY + + @pytest.mark.parametrize("mode", ["--key-only", "--json", None]) -def test_cli_output_modes_keep_status_and_bootstrap_key_out_of_stdout( - monkeypatch, capsys, mode -): - monkeypatch.setenv("BOOTSTRAP_KEY", "synthetic-bootstrap-secret") - monkeypatch.setattr(bootstrap.httpx, "Client", _FakeClient) - arguments = [ - "partner_api_key_bootstrap.py", - "--api-url", - "https://api.example.test", - "--sponsor-name", - "Synthetic Partner", - "--agent-id", - "synthetic-agent", - ] - if mode: - arguments.append(mode) - monkeypatch.setattr(sys, "argv", arguments) - assert bootstrap.main() == 0 +def test_cli_output_modes_never_print_full_key(monkeypatch, capsys, mode): + extra = [mode] if mode else [] + assert _run_main(monkeypatch, extra) == 0 captured = capsys.readouterr() - assert "synthetic-bootstrap-secret" not in captured.out + captured.err + combined = captured.out + captured.err + assert MINTED_KEY not in combined + assert BOOTSTRAP_SECRET not in combined assert "[created]" in captured.err assert "[created]" not in captured.out + assert MASKED_KEY in captured.out if mode == "--key-only": - assert captured.out == "b2a_test\n" + assert captured.out == f"{MASKED_KEY}\n" elif mode == "--json": - assert json.loads(captured.out)["api_key"] == "b2a_test" + payload = json.loads(captured.out) + assert "api_key" not in payload + assert payload["api_key_masked"] == MASKED_KEY + assert "key_file" not in payload else: assert captured.out.startswith("Partner API key bootstrap OK\n") - assert "api_key (once): b2a_test\n" in captured.out + assert f"api_key (masked): {MASKED_KEY}\n" in captured.out + + +@pytest.mark.parametrize("mode", ["--key-only", "--json", None]) +def test_cli_key_file_writes_0600_and_omits_full_key_from_stdio( + monkeypatch, capsys, tmp_path, mode +): + key_path = tmp_path / "minted.key" + extra = ["--key-file", str(key_path)] + if mode: + extra.append(mode) + assert _run_main(monkeypatch, extra) == 0 + captured = capsys.readouterr() + combined = captured.out + captured.err + assert MINTED_KEY not in combined + assert BOOTSTRAP_SECRET not in combined + assert str(key_path) in captured.out + assert key_path.read_text(encoding="utf-8") == MINTED_KEY + assert stat.S_IMODE(key_path.stat().st_mode) == 0o600 + if mode == "--key-only": + assert captured.out == f"{key_path}\n" + assert MASKED_KEY not in captured.out + elif mode == "--json": + payload = json.loads(captured.out) + assert "api_key" not in payload + assert payload["api_key_masked"] == MASKED_KEY + assert payload["key_file"] == str(key_path) + else: + assert f"api_key (masked): {MASKED_KEY}\n" in captured.out + assert f"key_file: {key_path}\n" in captured.out + + +def test_key_file_refuses_existing_path(monkeypatch, capsys, tmp_path): + key_path = tmp_path / "minted.key" + key_path.write_text("preexisting", encoding="utf-8") + with pytest.raises(SystemExit, match="already exists"): + _run_main(monkeypatch, ["--key-file", str(key_path)]) + captured = capsys.readouterr() + assert MINTED_KEY not in captured.out + captured.err + assert key_path.read_text(encoding="utf-8") == "preexisting" From 0f915f960d82fd6b57bc5b368a36b6e8baa3ab53 Mon Sep 17 00:00:00 2001 From: Christopher <131207452+PetrefiedThunder@users.noreply.github.com> Date: Mon, 26 Oct 2026 04:01:56 -0700 Subject: [PATCH 2/4] fix(security): print only non-secret identifiers in partner bootstrap Remove the masked-key derivation so no stdout/stderr output derives from the minted secret. stdout carries identity fields only (key id, wallet ids, key prefix); the secret flows solely to --key-file. Align the docs recipes and doc-contract test with the new output, and generate test fixtures at runtime. Signed-off-by: Christopher <131207452+PetrefiedThunder@users.noreply.github.com> --- gateway/docs/constant-test-loop.md | 6 +- gateway/docs/partner-api-key-bootstrap.md | 2 +- gateway/scripts/partner_api_key_bootstrap.py | 61 +++++++++------- .../tests/test_onboarding_doc_contracts.py | 21 +++++- gateway/tests/test_partner_bootstrap_flags.py | 72 ++++++++++++------- 5 files changed, 103 insertions(+), 59 deletions(-) diff --git a/gateway/docs/constant-test-loop.md b/gateway/docs/constant-test-loop.md index 4b9ad812..6ae001e2 100644 --- a/gateway/docs/constant-test-loop.md +++ b/gateway/docs/constant-test-loop.md @@ -93,8 +93,8 @@ API_URL=https://api.thisisatest.tech python scripts/constant_test_loop.py ## Machine-Readable Bootstrap Output -`partner_api_key_bootstrap.py --json` prints JSON to stdout with `api_key` -replaced by `api_key_masked`. Persist the secret with `--key-file` (do not +`partner_api_key_bootstrap.py --json` prints JSON identity fields to stdout +without the secret. Persist the secret with `--key-file` (do not pre-create the path; the script refuses an existing file): ```bash @@ -143,7 +143,7 @@ pytest tests/test_constant_test_loop.py -v ``` Covers: -- `--json` mode produces pipeable stdout (`api_key_masked`, not `api_key`) +- `--json` mode produces pipeable stdout (identity fields only, never `api_key`) - Bootstrap key never leaks to stdout or stderr - `--key-file` persists the minted secret at mode 0600 - Constant test loop runs against local instance diff --git a/gateway/docs/partner-api-key-bootstrap.md b/gateway/docs/partner-api-key-bootstrap.md index 63683581..d3aee41b 100644 --- a/gateway/docs/partner-api-key-bootstrap.md +++ b/gateway/docs/partner-api-key-bootstrap.md @@ -22,7 +22,7 @@ export BOOTSTRAP_KEY=... # from secret manager / Railway variables — not a d # One-shot provisioner. The minted secret is never printed; it is written # to --key-file (created mode 0600, refused if the path already exists). -# Without --key-file, stdout shows only a masked form. +# Without --key-file, stdout shows only non-secret identifiers (key id). uv run --with-requirements requirements.txt \ python scripts/partner_api_key_bootstrap.py \ --api-url "$API_URL" \ diff --git a/gateway/scripts/partner_api_key_bootstrap.py b/gateway/scripts/partner_api_key_bootstrap.py index 573b34d6..7ad1ed8a 100644 --- a/gateway/scripts/partner_api_key_bootstrap.py +++ b/gateway/scripts/partner_api_key_bootstrap.py @@ -5,8 +5,8 @@ sponsor wallet → agent wallet → DB-scoped agent API key. The minted agent secret is never printed to stdout or stderr. Pass --key-file -PATH to write it (created mode 0600, fails if PATH exists); otherwise only a -masked form is shown (first 8 chars of the key prefix area + "..." + last 4). +PATH to write it (created mode 0600, fails if PATH exists); stdout shows +only non-secret identifiers (key id, wallet ids, key prefix). Requires network access to --api-url. Does not mint keys without a bootstrap secret (fail closed). @@ -31,15 +31,6 @@ def _require(condition: bool, message: str) -> None: raise SystemExit(f"error: {message}") -def _mask_api_key(key: str) -> str: - """Return a display form that is not the full secret. - - Uses the first 8 characters of the key prefix area, an ellipsis, and - the last 4 characters. - """ - return f"{key[:8]}...{key[-4:]}" - - def _write_key_file(path: str, api_key: str) -> None: """Write the minted key to PATH created 0600; refuse if it already exists.""" flags = os.O_CREAT | os.O_EXCL | os.O_WRONLY @@ -183,7 +174,9 @@ def provision( key_body["max_uses"] = max_uses key = _post(client, "/v1/api-keys", key_body) api_key = key.get("api_key") - _require(bool(api_key), "api_key missing (write with --key-file; never printed)") + _require( + bool(api_key), "api_key missing (write with --key-file; never printed)" + ) return { "api_url": base, @@ -248,7 +241,7 @@ def main() -> int: "--json", action="store_true", help=( - "Print JSON to stdout with api_key replaced by api_key_masked " + "Print JSON identity fields to stdout without the secret " "(and key_file when --key-file is given). Default: human summary." ), ) @@ -256,8 +249,8 @@ def main() -> int: "--key-only", action="store_true", help=( - "Print only the --key-file path, or the masked key when --key-file " - "is omitted. Never prints the full minted secret." + "Print only the --key-file path, or the key_id when --key-file " + "is omitted. Never prints the minted secret." ), ) parser.add_argument( @@ -300,31 +293,45 @@ def main() -> int: json_mode=args.json, ) + # Non-secret identifiers for stdout. The minted secret below flows + # only to --key-file and never to print/log. + api_url = str(result["api_url"]) + sponsor_wallet_id = str(result["sponsor_wallet_id"]) + agent_wallet_id = str(result["agent_wallet_id"]) + wallet_id = str(result["wallet_id"]) + agent_id = str(result["agent_id"]) + key_id = result.get("key_id") + key_prefix = result.get("key_prefix") api_key = str(result["api_key"]) - masked = _mask_api_key(api_key) key_file = args.key_file if key_file: _write_key_file(key_file, api_key) if args.key_only: - print(key_file if key_file else masked) + print(key_file if key_file else key_id) elif args.json: # Machine-readable: JSON to stdout, all status to stderr. - # Never print bootstrap_key or the full minted api_key. - payload = {k: v for k, v in result.items() if k != "api_key"} - payload["api_key_masked"] = masked + # Never print bootstrap_key or the minted api_key. + payload: dict[str, Any] = { + "api_url": api_url, + "sponsor_wallet_id": sponsor_wallet_id, + "agent_wallet_id": agent_wallet_id, + "wallet_id": wallet_id, + "agent_id": agent_id, + "key_id": key_id, + "key_prefix": key_prefix, + } if key_file: payload["key_file"] = key_file print(json.dumps(payload, indent=2)) else: - # Human-readable summary to stdout. Masked key only. + # Human-readable summary to stdout. Non-secret identifiers only. print("Partner API key bootstrap OK") - print(f" api_url: {result['api_url']}") - print(f" sponsor_wallet_id: {result['sponsor_wallet_id']}") - print(f" agent_wallet_id: {result['agent_wallet_id']}") - print(f" key_id: {result['key_id']}") - print(f" key_prefix: {result['key_prefix']}") - print(f" api_key (masked): {masked}") + print(f" api_url: {api_url}") + print(f" sponsor_wallet_id: {sponsor_wallet_id}") + print(f" agent_wallet_id: {agent_wallet_id}") + print(f" key_id: {key_id}") + print(f" key_prefix: {key_prefix}") if key_file: print(f" key_file: {key_file}") return 0 diff --git a/gateway/tests/test_onboarding_doc_contracts.py b/gateway/tests/test_onboarding_doc_contracts.py index 9b03de7c..56cec674 100644 --- a/gateway/tests/test_onboarding_doc_contracts.py +++ b/gateway/tests/test_onboarding_doc_contracts.py @@ -7,8 +7,10 @@ import json from pathlib import Path import re +import secrets import shutil import subprocess +import uuid ROOT = Path(__file__).resolve().parents[1] @@ -64,15 +66,25 @@ def test_review_claim_names_local_receipt_exception(): def test_ci_recipe_exports_one_provisioned_identity(tmp_path): doc = read("constant-test-loop.md") recipe = doc.split("# Provision an agent key once", 1)[1].split( - "# Or use a restrictive", 1 + "# Run the constant test", 1 )[0] recipe = "# Provision an agent key once" + recipe + # Runtime-generated values only: the fake provisioner writes the secret + # to --key-file and prints identity JSON (no api_key on stdout). + agent_key = f"agt-{secrets.token_hex(16)}" + wallet_id = f"wal-{uuid.uuid4().hex[:12]}" + key_id = f"key-{uuid.uuid4().hex[:12]}" fake_python = tmp_path / "python" fake_python.write_text( "#!/bin/sh\n" 'count=0; [ ! -f "$COUNT_FILE" ] || count=$(cat "$COUNT_FILE")\n' 'count=$((count + 1)); printf "%s" "$count" > "$COUNT_FILE"\n' - 'printf \'{"api_key":"synthetic-%s","wallet_id":"wallet-%s","key_id":"key-%s"}\\n\' "$count" "$count" "$count"\n' + 'key_file=""; prev=""; for arg in "$@"; do\n' + ' if [ "$prev" = "--key-file" ]; then key_file="$arg"; fi\n' + ' prev="$arg"\n' + "done\n" + 'if [ -n "$key_file" ]; then printf "%s" "$FAKE_AGENT_KEY" > "$key_file"; fi\n' + 'printf \'{"wallet_id":"%s","key_id":"%s"}\\n\' "$FAKE_WALLET_ID" "$FAKE_KEY_ID"\n' ) fake_python.chmod(0o755) jq = shutil.which("jq") @@ -80,6 +92,9 @@ def test_ci_recipe_exports_one_provisioned_identity(tmp_path): env = { "PATH": f"{tmp_path}:{Path(jq).parent}:/usr/bin:/bin", "COUNT_FILE": str(tmp_path / "count"), + "FAKE_AGENT_KEY": agent_key, + "FAKE_WALLET_ID": wallet_id, + "FAKE_KEY_ID": key_id, } result = subprocess.run( [ @@ -93,7 +108,7 @@ def test_ci_recipe_exports_one_provisioned_identity(tmp_path): capture_output=True, check=True, ) - assert result.stdout.splitlines() == ["synthetic-1", "wallet-1", "key-1"] + assert result.stdout.splitlines() == [agent_key, wallet_id, key_id] assert (tmp_path / "count").read_text() == "1" diff --git a/gateway/tests/test_partner_bootstrap_flags.py b/gateway/tests/test_partner_bootstrap_flags.py index 33cabafe..7b1e42cb 100644 --- a/gateway/tests/test_partner_bootstrap_flags.py +++ b/gateway/tests/test_partner_bootstrap_flags.py @@ -8,18 +8,37 @@ from __future__ import annotations import json +import secrets import stat import sys +import time +import uuid import pytest import scripts.partner_api_key_bootstrap as bootstrap -# Test-generated minted secret. Long enough that the masked form is not a -# substring of the full key and the full key is not a substring of the mask. -MINTED_KEY = "b2a_test_generated_key_wxyz" -MASKED_KEY = "b2a_test...wxyz" -BOOTSTRAP_SECRET = "synthetic-bootstrap-secret" +# Runtime-generated fixtures only (no hardcoded secrets or ids). Distinct +# "mnt-"/"kpfx-" alphabets keep the minted secret's head/tail out of the +# server-provided prefix shown on stdout; the guard loop below removes any +# residual hex collision so leak assertions stay meaningful every run. +MINTED_KEY = f"mnt-{secrets.token_hex(16)}-{time.time_ns():x}" +BOOTSTRAP_SECRET = f"boot-{secrets.token_hex(16)}-{time.time_ns():x}" +KEY_ID = f"key-{uuid.uuid4().hex[:12]}" +KEY_PREFIX = f"kpfx-{secrets.token_hex(3)}" +WAL_SPONSOR = f"wal-{uuid.uuid4().hex[:12]}" +WAL_AGENT = f"wal-{uuid.uuid4().hex[:12]}" +while ( + MINTED_KEY[:8] in KEY_PREFIX + or MINTED_KEY[-4:] in KEY_PREFIX + or MINTED_KEY[-4:] in KEY_ID + or MINTED_KEY[-4:] in WAL_AGENT + or MINTED_KEY[-4:] in WAL_SPONSOR +): + KEY_PREFIX = f"kpfx-{secrets.token_hex(3)}" + KEY_ID = f"key-{uuid.uuid4().hex[:12]}" + WAL_SPONSOR = f"wal-{uuid.uuid4().hex[:12]}" + WAL_AGENT = f"wal-{uuid.uuid4().hex[:12]}" class _FakeResponse: @@ -50,11 +69,11 @@ def get(self, path): def post(self, path, json): self.posts.append((path, json)) if path == "/v1/billing/wallets/sponsor": - return _FakeResponse({"wallet_id": "wal_sponsor"}) + return _FakeResponse({"wallet_id": WAL_SPONSOR}) if path == "/v1/billing/wallets/agent": - return _FakeResponse({"wallet_id": "wal_agent"}) + return _FakeResponse({"wallet_id": WAL_AGENT}) return _FakeResponse( - {"api_key": MINTED_KEY, "key_id": "key_1", "key_prefix": "b2a_test"} + {"api_key": MINTED_KEY, "key_id": KEY_ID, "key_prefix": KEY_PREFIX} ) @@ -102,6 +121,13 @@ def _run_main(monkeypatch, extra: list[str]) -> int: return bootstrap.main() +def _assert_no_secret_leak(combined: str) -> None: + assert MINTED_KEY not in combined + assert MINTED_KEY[:8] not in combined + assert MINTED_KEY[-4:] not in combined + assert BOOTSTRAP_SECRET not in combined + + def test_bounds_flags_reach_request_bodies(monkeypatch): result, posts = _provision( monkeypatch, @@ -122,33 +148,30 @@ def test_bounds_default_to_absent(monkeypatch): assert "max_uses" not in posts["/v1/api-keys"] -def test_mask_form_is_prefix8_ellipsis_last4(): - assert bootstrap._mask_api_key(MINTED_KEY) == MASKED_KEY - assert MASKED_KEY == f"{MINTED_KEY[:8]}...{MINTED_KEY[-4:]}" - assert MINTED_KEY not in MASKED_KEY - - @pytest.mark.parametrize("mode", ["--key-only", "--json", None]) def test_cli_output_modes_never_print_full_key(monkeypatch, capsys, mode): extra = [mode] if mode else [] assert _run_main(monkeypatch, extra) == 0 captured = capsys.readouterr() combined = captured.out + captured.err - assert MINTED_KEY not in combined - assert BOOTSTRAP_SECRET not in combined + _assert_no_secret_leak(combined) assert "[created]" in captured.err assert "[created]" not in captured.out - assert MASKED_KEY in captured.out if mode == "--key-only": - assert captured.out == f"{MASKED_KEY}\n" + assert captured.out == f"{KEY_ID}\n" elif mode == "--json": payload = json.loads(captured.out) assert "api_key" not in payload - assert payload["api_key_masked"] == MASKED_KEY + assert "api_key_masked" not in payload assert "key_file" not in payload + assert payload["key_id"] == KEY_ID + assert payload["wallet_id"] == WAL_AGENT + assert payload["key_prefix"] == KEY_PREFIX else: assert captured.out.startswith("Partner API key bootstrap OK\n") - assert f"api_key (masked): {MASKED_KEY}\n" in captured.out + assert f"key_id: {KEY_ID}\n" in captured.out + assert f"key_prefix: {KEY_PREFIX}\n" in captured.out + assert "api_key" not in captured.out @pytest.mark.parametrize("mode", ["--key-only", "--json", None]) @@ -162,21 +185,20 @@ def test_cli_key_file_writes_0600_and_omits_full_key_from_stdio( assert _run_main(monkeypatch, extra) == 0 captured = capsys.readouterr() combined = captured.out + captured.err - assert MINTED_KEY not in combined - assert BOOTSTRAP_SECRET not in combined + _assert_no_secret_leak(combined) assert str(key_path) in captured.out assert key_path.read_text(encoding="utf-8") == MINTED_KEY assert stat.S_IMODE(key_path.stat().st_mode) == 0o600 if mode == "--key-only": assert captured.out == f"{key_path}\n" - assert MASKED_KEY not in captured.out elif mode == "--json": payload = json.loads(captured.out) assert "api_key" not in payload - assert payload["api_key_masked"] == MASKED_KEY + assert "api_key_masked" not in payload assert payload["key_file"] == str(key_path) + assert payload["key_id"] == KEY_ID else: - assert f"api_key (masked): {MASKED_KEY}\n" in captured.out + assert "api_key" not in captured.out assert f"key_file: {key_path}\n" in captured.out From d9941fd1016758e58d2018e9bf8b8dff52b6b227 Mon Sep 17 00:00:00 2001 From: Christopher <131207452+PetrefiedThunder@users.noreply.github.com> Date: Mon, 26 Oct 2026 07:16:37 -0700 Subject: [PATCH 3/4] fix(gateway): reserve key file before minting and harden bootstrap writes - reserve key file with O_CREAT|O_EXCL 0600 before minting; unlink on mint failure - short-write loop, fsync, close+unlink on write error - drop response body from SystemExit; report status only - --key-only requires --key-file (argparse error) - docs: mkdir -p -m 700 ~/.config/amw before writing key Signed-off-by: Christopher <131207452+PetrefiedThunder@users.noreply.github.com> --- gateway/docs/partner-api-key-bootstrap.md | 2 + gateway/scripts/partner_api_key_bootstrap.py | 99 +++++++++---- gateway/tests/test_constant_test_loop.py | 21 ++- gateway/tests/test_partner_bootstrap_flags.py | 139 +++++++++++++++++- 4 files changed, 226 insertions(+), 35 deletions(-) diff --git a/gateway/docs/partner-api-key-bootstrap.md b/gateway/docs/partner-api-key-bootstrap.md index d3aee41b..439b2f7e 100644 --- a/gateway/docs/partner-api-key-bootstrap.md +++ b/gateway/docs/partner-api-key-bootstrap.md @@ -20,6 +20,8 @@ Never commit `VALID_API_KEYS` to git. export API_URL="${PUBLIC_URL:-https://api.thisisatest.tech}" export BOOTSTRAP_KEY=... # from secret manager / Railway variables — not a demo string +mkdir -p -m 700 ~/.config/amw + # One-shot provisioner. The minted secret is never printed; it is written # to --key-file (created mode 0600, refused if the path already exists). # Without --key-file, stdout shows only non-secret identifiers (key id). diff --git a/gateway/scripts/partner_api_key_bootstrap.py b/gateway/scripts/partner_api_key_bootstrap.py index 15209724..b3b3ad76 100644 --- a/gateway/scripts/partner_api_key_bootstrap.py +++ b/gateway/scripts/partner_api_key_bootstrap.py @@ -5,8 +5,8 @@ sponsor wallet → agent wallet → DB-scoped agent API key. The minted agent secret is never printed to stdout or stderr. Pass --key-file -PATH to write it (created mode 0600, fails if PATH exists); stdout shows -only non-secret identifiers (key id, wallet ids, key prefix). +PATH to write it (reserved mode 0600 before minting, fails if PATH exists); +stdout shows only non-secret identifiers (key id, wallet ids, key prefix). Requires network access to --api-url. Does not mint keys without a bootstrap secret (fail closed). @@ -31,21 +31,41 @@ def _require(condition: bool, message: str) -> None: raise SystemExit(f"error: {message}") -def _write_key_file(path: str, api_key: str) -> None: - """Write the minted key to PATH created 0600; refuse if it already exists.""" +def _reserve_key_file(path: str) -> int: + """Create PATH exclusively (mode 0600) and return its fd; refuse if it exists.""" flags = os.O_CREAT | os.O_EXCL | os.O_WRONLY try: - fd = os.open(path, flags, 0o600) + return os.open(path, flags, 0o600) except FileExistsError: raise SystemExit(f"error: --key-file {path} already exists") except OSError as exc: raise SystemExit(f"error: cannot write --key-file {path}: {exc}") from exc + + +def _write_key_fd(fd: int, path: str, api_key: str) -> None: + """Write the full key to the reserved fd, fsync, then close. + + On any error the fd is closed and PATH is unlinked before raising, + so a partial key file is never left behind. + """ + data = api_key.encode("utf-8") try: - os.write(fd, api_key.encode("utf-8")) + view = memoryview(data) + while view: + written = os.write(fd, view) + view = view[written:] + os.fsync(fd) except OSError as exc: + try: + os.close(fd) + except OSError: + pass + try: + os.unlink(path) + except OSError: + pass raise SystemExit(f"error: cannot write --key-file {path}: {exc}") from exc - finally: - os.close(fd) + os.close(fd) def _require_safe_api_url(api_url: str) -> str: @@ -85,7 +105,7 @@ def _bootstrap_key(cli_value: str | None) -> str: def _post(client: httpx.Client, path: str, body: dict[str, Any]) -> dict[str, Any]: resp = client.post(path, json=body) if resp.status_code >= 400: - raise SystemExit(f"error: POST {path} → {resp.status_code}: {resp.text[:500]}") + raise SystemExit(f"error: bootstrap request failed (HTTP {resp.status_code})") data = resp.json() _require(isinstance(data, dict), f"POST {path} returned non-object JSON") return data @@ -256,8 +276,8 @@ def main() -> int: "--key-only", action="store_true", help=( - "Print only the --key-file path, or the key_id when --key-file " - "is omitted. Never prints the minted secret." + "Print only the --key-file path. Requires --key-file. " + "Never prints the minted secret." ), ) parser.add_argument( @@ -275,8 +295,8 @@ def main() -> int: if args.key_only and args.json: raise SystemExit("error: --key-only and --json are mutually exclusive") - if args.key_file and os.path.exists(args.key_file): - raise SystemExit(f"error: --key-file {args.key_file} already exists") + if args.key_only and not args.key_file: + parser.error("--key-only requires --key-file (the key is never printed)") if args.agent_wallet_id and not args.sponsor_wallet_id: raise SystemExit( @@ -284,21 +304,39 @@ def main() -> int: "(for operator record-keeping)" ) - result = provision( - api_url=args.api_url, - bootstrap_key=_bootstrap_key(args.bootstrap_key), - sponsor_name=args.sponsor_name, - agent_id=args.agent_id, - budget_credits=args.budget_credits, - initial_credits=args.initial_credits, - key_name=args.key_name, - sponsor_wallet_id=args.sponsor_wallet_id, - agent_wallet_id=args.agent_wallet_id, - daily_limit=args.daily_limit, - expires_in_days=args.expires_in_days, - max_uses=args.max_uses, - json_mode=args.json, - ) + bootstrap_key = _bootstrap_key(args.bootstrap_key) + + # Reserve the key file before minting so a minted key is never discarded + # because its destination cannot be created. + key_fd = _reserve_key_file(args.key_file) if args.key_file else None + + try: + result = provision( + api_url=args.api_url, + bootstrap_key=bootstrap_key, + sponsor_name=args.sponsor_name, + agent_id=args.agent_id, + budget_credits=args.budget_credits, + initial_credits=args.initial_credits, + key_name=args.key_name, + sponsor_wallet_id=args.sponsor_wallet_id, + agent_wallet_id=args.agent_wallet_id, + daily_limit=args.daily_limit, + expires_in_days=args.expires_in_days, + max_uses=args.max_uses, + json_mode=args.json, + ) + except BaseException: + if key_fd is not None: + try: + os.close(key_fd) + except OSError: + pass + try: + os.unlink(args.key_file) + except OSError: + pass + raise # Non-secret identifiers for stdout. The minted secret below flows # only to --key-file and never to print/log. @@ -312,10 +350,11 @@ def main() -> int: api_key = str(result["api_key"]) key_file = args.key_file if key_file: - _write_key_file(key_file, api_key) + assert key_fd is not None # reserved above whenever --key-file is given + _write_key_fd(key_fd, key_file, api_key) if args.key_only: - print(key_file if key_file else key_id) + print(key_file) elif args.json: # Machine-readable: JSON to stdout, all status to stderr. # Never print bootstrap_key or the minted api_key. diff --git a/gateway/tests/test_constant_test_loop.py b/gateway/tests/test_constant_test_loop.py index cdc5153c..b7ebf5dc 100644 --- a/gateway/tests/test_constant_test_loop.py +++ b/gateway/tests/test_constant_test_loop.py @@ -655,7 +655,7 @@ def test_off_loopback_runs_require_a_pinned_tool_and_approved_payload(): assert "b2a_placeholder" not in result.stdout + result.stderr -def test_bootstrap_key_only_pipes_without_jq(): +def test_bootstrap_key_only_pipes_without_jq(tmp_path): """--key-only and --json both write stdout, so they cannot both apply.""" result = subprocess.run( [ @@ -673,6 +673,23 @@ def test_bootstrap_key_only_pipes_without_jq(): assert result.returncode != 0 assert "mutually exclusive" in result.stderr + # --key-only without --key-file is an argparse error (exit 2): the minted + # key is never printed, so a missing destination would discard it. + result = subprocess.run( + [ + sys.executable, + str(BOOTSTRAP_SCRIPT), + "--api-url", + "https://api.example.org", + "--key-only", + ], + capture_output=True, + text=True, + env={"PATH": "/usr/bin:/bin", "BOOTSTRAP_KEY": "placeholder"}, + ) + assert result.returncode == 2 + assert "requires --key-file" in result.stderr + # --key-only must not have opened a path around the fail-closed check. result = subprocess.run( [ @@ -681,6 +698,8 @@ def test_bootstrap_key_only_pipes_without_jq(): "--api-url", "https://api.example.org", "--key-only", + "--key-file", + str(tmp_path / "agent.key"), ], capture_output=True, text=True, diff --git a/gateway/tests/test_partner_bootstrap_flags.py b/gateway/tests/test_partner_bootstrap_flags.py index 7b1e42cb..04d797f6 100644 --- a/gateway/tests/test_partner_bootstrap_flags.py +++ b/gateway/tests/test_partner_bootstrap_flags.py @@ -8,11 +8,13 @@ from __future__ import annotations import json +import os import secrets import stat import sys import time import uuid +from pathlib import Path import pytest @@ -148,7 +150,7 @@ def test_bounds_default_to_absent(monkeypatch): assert "max_uses" not in posts["/v1/api-keys"] -@pytest.mark.parametrize("mode", ["--key-only", "--json", None]) +@pytest.mark.parametrize("mode", ["--json", None]) def test_cli_output_modes_never_print_full_key(monkeypatch, capsys, mode): extra = [mode] if mode else [] assert _run_main(monkeypatch, extra) == 0 @@ -157,9 +159,7 @@ def test_cli_output_modes_never_print_full_key(monkeypatch, capsys, mode): _assert_no_secret_leak(combined) assert "[created]" in captured.err assert "[created]" not in captured.out - if mode == "--key-only": - assert captured.out == f"{KEY_ID}\n" - elif mode == "--json": + if mode == "--json": payload = json.loads(captured.out) assert "api_key" not in payload assert "api_key_masked" not in payload @@ -210,3 +210,134 @@ def test_key_file_refuses_existing_path(monkeypatch, capsys, tmp_path): captured = capsys.readouterr() assert MINTED_KEY not in captured.out + captured.err assert key_path.read_text(encoding="utf-8") == "preexisting" + + +# Fixed, obviously synthetic values for the reserve/mint/write tests below. +FIXED_KEY = "fixed-test-key-value-001" +FIXED_BODY_SENTINEL = "fixed-sentinel-response-body-001" + + +def test_reserve_key_file_uses_exclusive_0600_create(monkeypatch, tmp_path): + key_path = tmp_path / "minted.key" + real_open = os.open + seen: dict[str, int] = {} + + def _record_open(path, flags, mode=0o777): + seen["flags"] = flags + seen["mode"] = mode + return real_open(path, flags, mode) + + monkeypatch.setattr(os, "open", _record_open) + fd = bootstrap._reserve_key_file(str(key_path)) + os.close(fd) + assert seen["flags"] == (os.O_CREAT | os.O_EXCL | os.O_WRONLY) + assert seen["mode"] == 0o600 + + +def test_existing_key_file_skips_mint(monkeypatch, tmp_path): + key_path = tmp_path / "minted.key" + key_path.write_text("fixed-preexisting-content", encoding="utf-8") + + def _no_mint(**kwargs): + raise AssertionError("provision must not run when --key-file exists") + + monkeypatch.setenv("BOOTSTRAP_KEY", BOOTSTRAP_SECRET) + monkeypatch.setattr(bootstrap, "provision", _no_mint) + monkeypatch.setattr(sys, "argv", _cli_args("--key-file", str(key_path))) + with pytest.raises(SystemExit, match="already exists"): + bootstrap.main() + assert key_path.read_text(encoding="utf-8") == "fixed-preexisting-content" + + +def test_failed_mint_removes_reserved_key_file(monkeypatch, tmp_path): + key_path = tmp_path / "minted.key" + + def _fail(**kwargs): + raise SystemExit("error: fixed-mint-failure") + + monkeypatch.setenv("BOOTSTRAP_KEY", BOOTSTRAP_SECRET) + monkeypatch.setattr(bootstrap, "provision", _fail) + monkeypatch.setattr(sys, "argv", _cli_args("--key-file", str(key_path))) + with pytest.raises(SystemExit, match="fixed-mint-failure"): + bootstrap.main() + assert not key_path.exists() + + +def test_write_key_fd_handles_short_writes_and_fsyncs(monkeypatch, tmp_path): + key_path = tmp_path / "minted.key" + fd = bootstrap._reserve_key_file(str(key_path)) + real_write = os.write + real_fsync = os.fsync + fsynced: list[int] = [] + + def _short_write(target_fd, data): + if target_fd != fd: + return real_write(target_fd, data) + return real_write(target_fd, bytes(data[:1])) + + def _record_fsync(target_fd): + fsynced.append(target_fd) + return real_fsync(target_fd) + + monkeypatch.setattr(os, "write", _short_write) + monkeypatch.setattr(os, "fsync", _record_fsync) + bootstrap._write_key_fd(fd, str(key_path), FIXED_KEY) + assert key_path.read_text(encoding="utf-8") == FIXED_KEY + assert fsynced == [fd] + assert stat.S_IMODE(key_path.stat().st_mode) == 0o600 + + +def test_write_key_fd_failure_removes_file_and_closes_fd(monkeypatch, tmp_path): + key_path = tmp_path / "minted.key" + fd = bootstrap._reserve_key_file(str(key_path)) + real_write = os.write + + def _fail_write(target_fd, data): + if target_fd == fd: + raise OSError("fixed-write-failure") + return real_write(target_fd, data) + + monkeypatch.setattr(os, "write", _fail_write) + with pytest.raises(SystemExit, match="fixed-write-failure"): + bootstrap._write_key_fd(fd, str(key_path), FIXED_KEY) + assert not key_path.exists() + with pytest.raises(OSError, match="Bad file descriptor"): + os.fstat(fd) + + +def test_post_error_omits_response_body(): + class _ErrorResponse: + status_code = 502 + text = FIXED_BODY_SENTINEL + + class _ErrorClient: + def post(self, path, json): + return _ErrorResponse() + + with pytest.raises(SystemExit) as excinfo: + bootstrap._post(_ErrorClient(), "/v1/api-keys", {"key_name": "fixed-name"}) + message = str(excinfo.value) + assert "502" in message + assert FIXED_BODY_SENTINEL not in message + + +def test_key_only_without_key_file_is_argparse_error(monkeypatch, capsys): + def _no_mint(**kwargs): + raise AssertionError("provision must not run without --key-file") + + monkeypatch.setenv("BOOTSTRAP_KEY", BOOTSTRAP_SECRET) + monkeypatch.setattr(bootstrap, "provision", _no_mint) + monkeypatch.setattr(sys, "argv", _cli_args("--key-only")) + with pytest.raises(SystemExit) as excinfo: + bootstrap.main() + assert excinfo.value.code == 2 + assert "requires --key-file" in capsys.readouterr().err + + +def test_bootstrap_doc_creates_config_dir_before_key_file(): + doc = ( + Path(__file__).resolve().parents[1] / "docs" / "partner-api-key-bootstrap.md" + ).read_text(encoding="utf-8") + mkdir_at = doc.index("mkdir -p -m 700 ~/.config/amw") + key_file_at = doc.index("--key-file") + assert mkdir_at < key_file_at From 67527bc5d668e2bf05bf597c140d68cff19aef23 Mon Sep 17 00:00:00 2001 From: Christopher <131207452+PetrefiedThunder@users.noreply.github.com> Date: Mon, 26 Oct 2026 23:01:38 -0700 Subject: [PATCH 4/4] fix(gateway): require key file, keep key on failed secret set, runtime test values Delete KEY_FILE only after gh secret set succeeds; require --key-file before minting; print key_id on key-file write failure; use secrets/uuid4 in new tests. Signed-off-by: Christopher <131207452+PetrefiedThunder@users.noreply.github.com> --- gateway/docs/constant-test-loop.md | 3 +- gateway/docs/partner-api-key-bootstrap.md | 4 +- gateway/scripts/partner_api_key_bootstrap.py | 19 ++- gateway/tests/test_constant_test_loop.py | 10 +- gateway/tests/test_partner_bootstrap_flags.py | 131 +++++++++++++++--- 5 files changed, 138 insertions(+), 29 deletions(-) diff --git a/gateway/docs/constant-test-loop.md b/gateway/docs/constant-test-loop.md index 6ae001e2..b2de888a 100644 --- a/gateway/docs/constant-test-loop.md +++ b/gateway/docs/constant-test-loop.md @@ -118,8 +118,7 @@ BOOTSTRAP_KEY="..." python scripts/partner_api_key_bootstrap.py \ --budget-credits 5000 \ --key-only \ --key-file "$KEY_FILE" -gh secret set CI_SMOKE_AGENT_KEY --repo agentmiddleware/agent-middleware < "$KEY_FILE" -rm -f "$KEY_FILE" +gh secret set CI_SMOKE_AGENT_KEY --repo agentmiddleware/agent-middleware < "$KEY_FILE" && rm -f "$KEY_FILE" ``` ### Security Properties diff --git a/gateway/docs/partner-api-key-bootstrap.md b/gateway/docs/partner-api-key-bootstrap.md index 439b2f7e..6bd6b2f4 100644 --- a/gateway/docs/partner-api-key-bootstrap.md +++ b/gateway/docs/partner-api-key-bootstrap.md @@ -24,7 +24,9 @@ mkdir -p -m 700 ~/.config/amw # One-shot provisioner. The minted secret is never printed; it is written # to --key-file (created mode 0600, refused if the path already exists). -# Without --key-file, stdout shows only non-secret identifiers (key id). +# --key-file is required: stdout shows only non-secret identifiers (key id). +# Parent dir must exist before --key-file is written. +install -d -m 700 "$HOME/.config/amw" uv run --with-requirements requirements.txt \ python scripts/partner_api_key_bootstrap.py \ --api-url "$API_URL" \ diff --git a/gateway/scripts/partner_api_key_bootstrap.py b/gateway/scripts/partner_api_key_bootstrap.py index b3b3ad76..5c581a95 100644 --- a/gateway/scripts/partner_api_key_bootstrap.py +++ b/gateway/scripts/partner_api_key_bootstrap.py @@ -285,9 +285,10 @@ def main() -> int: default=None, metavar="PATH", help=( - "Write the minted api_key to PATH (created mode 0600 with " + "Required: write the minted api_key to PATH (created mode 0600 with " "O_CREAT|O_EXCL|O_WRONLY; refused if PATH already exists). " - "Stdout prints PATH in --key-only mode, and JSON includes key_file." + "Stdout prints PATH in --key-only mode, and JSON includes key_file. " + "The minted key is never printed." ), ) args = parser.parse_args() @@ -304,6 +305,9 @@ def main() -> int: "(for operator record-keeping)" ) + if not args.key_file: + parser.error("--key-file is required (the minted key is never printed)") + bootstrap_key = _bootstrap_key(args.bootstrap_key) # Reserve the key file before minting so a minted key is never discarded @@ -351,7 +355,16 @@ def main() -> int: key_file = args.key_file if key_file: assert key_fd is not None # reserved above whenever --key-file is given - _write_key_fd(key_fd, key_file, api_key) + try: + _write_key_fd(key_fd, key_file, api_key) + except SystemExit: + print( + f"error: failed to write --key-file {key_file} for " + f"key_id={key_id} agent_id={agent_id}; the key is active " + "and should be revoked", + file=sys.stderr, + ) + raise if args.key_only: print(key_file) diff --git a/gateway/tests/test_constant_test_loop.py b/gateway/tests/test_constant_test_loop.py index b7ebf5dc..9b2005a4 100644 --- a/gateway/tests/test_constant_test_loop.py +++ b/gateway/tests/test_constant_test_loop.py @@ -9,6 +9,7 @@ import json import os +import secrets import signal import socket import subprocess @@ -657,6 +658,7 @@ def test_off_loopback_runs_require_a_pinned_tool_and_approved_payload(): def test_bootstrap_key_only_pipes_without_jq(tmp_path): """--key-only and --json both write stdout, so they cannot both apply.""" + bootstrap_secret = secrets.token_urlsafe(32) result = subprocess.run( [ sys.executable, @@ -668,7 +670,7 @@ def test_bootstrap_key_only_pipes_without_jq(tmp_path): ], capture_output=True, text=True, - env={"PATH": "/usr/bin:/bin", "BOOTSTRAP_KEY": "placeholder"}, + env={"PATH": "/usr/bin:/bin", "BOOTSTRAP_KEY": bootstrap_secret}, ) assert result.returncode != 0 assert "mutually exclusive" in result.stderr @@ -685,12 +687,13 @@ def test_bootstrap_key_only_pipes_without_jq(tmp_path): ], capture_output=True, text=True, - env={"PATH": "/usr/bin:/bin", "BOOTSTRAP_KEY": "placeholder"}, + env={"PATH": "/usr/bin:/bin", "BOOTSTRAP_KEY": bootstrap_secret}, ) assert result.returncode == 2 assert "requires --key-file" in result.stderr # --key-only must not have opened a path around the fail-closed check. + key_file = tmp_path / "agent.key" result = subprocess.run( [ sys.executable, @@ -699,7 +702,7 @@ def test_bootstrap_key_only_pipes_without_jq(tmp_path): "https://api.example.org", "--key-only", "--key-file", - str(tmp_path / "agent.key"), + str(key_file), ], capture_output=True, text=True, @@ -707,6 +710,7 @@ def test_bootstrap_key_only_pipes_without_jq(tmp_path): ) assert result.returncode != 0 assert "BOOTSTRAP_KEY" in result.stderr + assert not key_file.exists() def test_unusable_tool_pin_exits_as_configuration_not_invariant_failure( diff --git a/gateway/tests/test_partner_bootstrap_flags.py b/gateway/tests/test_partner_bootstrap_flags.py index 04d797f6..18d2f58c 100644 --- a/gateway/tests/test_partner_bootstrap_flags.py +++ b/gateway/tests/test_partner_bootstrap_flags.py @@ -151,8 +151,11 @@ def test_bounds_default_to_absent(monkeypatch): @pytest.mark.parametrize("mode", ["--json", None]) -def test_cli_output_modes_never_print_full_key(monkeypatch, capsys, mode): - extra = [mode] if mode else [] +def test_cli_output_modes_never_print_full_key(monkeypatch, capsys, tmp_path, mode): + key_path = tmp_path / f"{uuid.uuid4()}.key" + extra = ["--key-file", str(key_path)] + if mode: + extra.append(mode) assert _run_main(monkeypatch, extra) == 0 captured = capsys.readouterr() combined = captured.out + captured.err @@ -163,7 +166,7 @@ def test_cli_output_modes_never_print_full_key(monkeypatch, capsys, mode): payload = json.loads(captured.out) assert "api_key" not in payload assert "api_key_masked" not in payload - assert "key_file" not in payload + assert payload["key_file"] == str(key_path) assert payload["key_id"] == KEY_ID assert payload["wallet_id"] == WAL_AGENT assert payload["key_prefix"] == KEY_PREFIX @@ -172,6 +175,7 @@ def test_cli_output_modes_never_print_full_key(monkeypatch, capsys, mode): assert f"key_id: {KEY_ID}\n" in captured.out assert f"key_prefix: {KEY_PREFIX}\n" in captured.out assert "api_key" not in captured.out + assert f"key_file: {key_path}\n" in captured.out @pytest.mark.parametrize("mode", ["--key-only", "--json", None]) @@ -204,17 +208,18 @@ def test_cli_key_file_writes_0600_and_omits_full_key_from_stdio( def test_key_file_refuses_existing_path(monkeypatch, capsys, tmp_path): key_path = tmp_path / "minted.key" - key_path.write_text("preexisting", encoding="utf-8") + preexisting = f"preexisting-{secrets.token_urlsafe(16)}" + key_path.write_text(preexisting, encoding="utf-8") with pytest.raises(SystemExit, match="already exists"): _run_main(monkeypatch, ["--key-file", str(key_path)]) captured = capsys.readouterr() assert MINTED_KEY not in captured.out + captured.err - assert key_path.read_text(encoding="utf-8") == "preexisting" + assert key_path.read_text(encoding="utf-8") == preexisting -# Fixed, obviously synthetic values for the reserve/mint/write tests below. -FIXED_KEY = "fixed-test-key-value-001" -FIXED_BODY_SENTINEL = "fixed-sentinel-response-body-001" +# Runtime-generated values for the reserve/mint/write tests below. +WRITE_KEY = secrets.token_urlsafe(32) +BODY_SENTINEL = secrets.token_urlsafe(32) def test_reserve_key_file_uses_exclusive_0600_create(monkeypatch, tmp_path): @@ -236,7 +241,8 @@ def _record_open(path, flags, mode=0o777): def test_existing_key_file_skips_mint(monkeypatch, tmp_path): key_path = tmp_path / "minted.key" - key_path.write_text("fixed-preexisting-content", encoding="utf-8") + preexisting_content = f"preexisting-{secrets.token_urlsafe(16)}" + key_path.write_text(preexisting_content, encoding="utf-8") def _no_mint(**kwargs): raise AssertionError("provision must not run when --key-file exists") @@ -246,19 +252,20 @@ def _no_mint(**kwargs): monkeypatch.setattr(sys, "argv", _cli_args("--key-file", str(key_path))) with pytest.raises(SystemExit, match="already exists"): bootstrap.main() - assert key_path.read_text(encoding="utf-8") == "fixed-preexisting-content" + assert key_path.read_text(encoding="utf-8") == preexisting_content def test_failed_mint_removes_reserved_key_file(monkeypatch, tmp_path): key_path = tmp_path / "minted.key" + failure_marker = f"mint-failure-{uuid.uuid4().hex}" def _fail(**kwargs): - raise SystemExit("error: fixed-mint-failure") + raise SystemExit(f"error: {failure_marker}") monkeypatch.setenv("BOOTSTRAP_KEY", BOOTSTRAP_SECRET) monkeypatch.setattr(bootstrap, "provision", _fail) monkeypatch.setattr(sys, "argv", _cli_args("--key-file", str(key_path))) - with pytest.raises(SystemExit, match="fixed-mint-failure"): + with pytest.raises(SystemExit, match=failure_marker): bootstrap.main() assert not key_path.exists() @@ -281,8 +288,8 @@ def _record_fsync(target_fd): monkeypatch.setattr(os, "write", _short_write) monkeypatch.setattr(os, "fsync", _record_fsync) - bootstrap._write_key_fd(fd, str(key_path), FIXED_KEY) - assert key_path.read_text(encoding="utf-8") == FIXED_KEY + bootstrap._write_key_fd(fd, str(key_path), WRITE_KEY) + assert key_path.read_text(encoding="utf-8") == WRITE_KEY assert fsynced == [fd] assert stat.S_IMODE(key_path.stat().st_mode) == 0o600 @@ -292,14 +299,16 @@ def test_write_key_fd_failure_removes_file_and_closes_fd(monkeypatch, tmp_path): fd = bootstrap._reserve_key_file(str(key_path)) real_write = os.write + write_failure = f"write-failure-{uuid.uuid4().hex}" + def _fail_write(target_fd, data): if target_fd == fd: - raise OSError("fixed-write-failure") + raise OSError(write_failure) return real_write(target_fd, data) monkeypatch.setattr(os, "write", _fail_write) - with pytest.raises(SystemExit, match="fixed-write-failure"): - bootstrap._write_key_fd(fd, str(key_path), FIXED_KEY) + with pytest.raises(SystemExit, match=write_failure): + bootstrap._write_key_fd(fd, str(key_path), WRITE_KEY) assert not key_path.exists() with pytest.raises(OSError, match="Bad file descriptor"): os.fstat(fd) @@ -308,17 +317,18 @@ def _fail_write(target_fd, data): def test_post_error_omits_response_body(): class _ErrorResponse: status_code = 502 - text = FIXED_BODY_SENTINEL + text = BODY_SENTINEL class _ErrorClient: def post(self, path, json): return _ErrorResponse() + key_name = f"key-{uuid.uuid4()}" with pytest.raises(SystemExit) as excinfo: - bootstrap._post(_ErrorClient(), "/v1/api-keys", {"key_name": "fixed-name"}) + bootstrap._post(_ErrorClient(), "/v1/api-keys", {"key_name": key_name}) message = str(excinfo.value) assert "502" in message - assert FIXED_BODY_SENTINEL not in message + assert BODY_SENTINEL not in message def test_key_only_without_key_file_is_argparse_error(monkeypatch, capsys): @@ -334,6 +344,87 @@ def _no_mint(**kwargs): assert "requires --key-file" in capsys.readouterr().err +@pytest.mark.parametrize("mode", ["--json", None]) +def test_missing_key_file_exits_and_skips_mint(monkeypatch, capsys, mode): + bootstrap_secret = secrets.token_urlsafe(32) + sponsor_name = f"sponsor-{uuid.uuid4()}" + agent_id = f"agent-{uuid.uuid4()}" + key_name = f"key-{uuid.uuid4()}" + argv = [ + "partner_api_key_bootstrap.py", + "--api-url", + "https://api.example.test", + "--sponsor-name", + sponsor_name, + "--agent-id", + agent_id, + "--key-name", + key_name, + ] + if mode: + argv.append(mode) + + def _no_mint(**kwargs): + raise AssertionError("provision must not run without --key-file") + + monkeypatch.setenv("BOOTSTRAP_KEY", bootstrap_secret) + monkeypatch.setattr(bootstrap, "provision", _no_mint) + monkeypatch.setattr(sys, "argv", argv) + with pytest.raises(SystemExit) as excinfo: + bootstrap.main() + assert excinfo.value.code == 2 + err = capsys.readouterr().err + assert "--key-file is required" in err + assert bootstrap_secret not in err + + +def test_write_failure_after_mint_prints_key_id_not_secret( + monkeypatch, capsys, tmp_path +): + bootstrap_secret = secrets.token_urlsafe(32) + sponsor_name = f"sponsor-{uuid.uuid4()}" + agent_id = f"agent-{uuid.uuid4()}" + key_name = f"key-{uuid.uuid4()}" + key_path = tmp_path / f"{uuid.uuid4()}.key" + failure_marker = f"write-failure-{uuid.uuid4().hex}" + + def _fail_write(fd, path, api_key): + raise SystemExit(f"error: {failure_marker}") + + monkeypatch.setenv("BOOTSTRAP_KEY", bootstrap_secret) + monkeypatch.setattr(bootstrap.httpx, "Client", _FakeClient) + monkeypatch.setattr(bootstrap, "_write_key_fd", _fail_write) + monkeypatch.setattr( + sys, + "argv", + [ + "partner_api_key_bootstrap.py", + "--api-url", + "https://api.example.test", + "--sponsor-name", + sponsor_name, + "--agent-id", + agent_id, + "--key-name", + key_name, + "--key-file", + str(key_path), + ], + ) + with pytest.raises(SystemExit, match=failure_marker): + bootstrap.main() + captured = capsys.readouterr() + combined = captured.out + captured.err + assert KEY_ID in captured.err + assert agent_id in captured.err + assert "active" in captured.err + assert "revoke" in captured.err.lower() + assert MINTED_KEY not in combined + assert MINTED_KEY[:8] not in combined + assert MINTED_KEY[-4:] not in combined + assert bootstrap_secret not in combined + + def test_bootstrap_doc_creates_config_dir_before_key_file(): doc = ( Path(__file__).resolve().parents[1] / "docs" / "partner-api-key-bootstrap.md"