diff --git a/docs/constant-test-loop.md b/docs/constant-test-loop.md index 73099cfc..62ad6132 100644 --- a/docs/constant-test-loop.md +++ b/docs/constant-test-loop.md @@ -50,6 +50,21 @@ python scripts/constant_test_loop.py \ --tool-args '{"text": "constant test loop"}' ``` +Every flag has an environment equivalent, so a CI job can be configured +entirely through secrets without putting anything in argv: + +| Flag | Environment variable | +|---|---| +| `--api-url` | `API_URL` | +| `--tool` | `CI_SMOKE_TOOL` | +| `--other-tool` | `CI_SMOKE_OTHER_TOOL` | +| `--tool-args` | `CI_SMOKE_TOOL_ARGS` (a JSON object) | + +A flag beats the corresponding variable when both are set. A malformed +`CI_SMOKE_TOOL_ARGS` — invalid JSON, or valid JSON that is not an object — +exits 2 as a configuration error rather than 1, so a payload typo never +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 diff --git a/scripts/constant_test_loop.py b/scripts/constant_test_loop.py index 3bef059b..4f096428 100755 --- a/scripts/constant_test_loop.py +++ b/scripts/constant_test_loop.py @@ -582,6 +582,41 @@ def _validate_api_url(url: str) -> str: return url +def _resolve_tool_args(cli_value: str | None) -> dict | None: + """Resolve the governed tool's payload from the flag or the environment. + + ``--tool`` and ``--other-tool`` already fall back to $CI_SMOKE_TOOL and + $CI_SMOKE_OTHER_TOOL. Without the same fallback here, a CI job could + configure everything the off-loopback guard demands *except* the payload, + and would have to reach for argv to supply the one remaining piece — + which for a secret-adjacent value is the wrong place. + + Returns a dict or None. Anything unparseable is a ConfigurationError + (exit 2), never an invariant failure: a malformed payload means the loop + was configured wrong, not that the trust plane broke. + """ + raw = cli_value + source = "--tool-args" + if raw is None: + raw = os.environ.get("CI_SMOKE_TOOL_ARGS", "").strip() or None + source = "$CI_SMOKE_TOOL_ARGS" + if raw is None: + return None + + try: + parsed = json.loads(raw) + except json.JSONDecodeError as exc: + raise ConfigurationError(f"{source} is not valid JSON: {exc}") from exc + if not isinstance(parsed, dict): + # A list or scalar would be forwarded as the tool's arguments and + # rejected downstream as a tool error, which reads as the product + # failing rather than the payload being the wrong shape. + raise ConfigurationError( + f"{source} must be a JSON object, got {type(parsed).__name__}" + ) + return parsed + + def _require_deliberate_production_target( api_url: str, args: argparse.Namespace ) -> None: @@ -639,17 +674,20 @@ def main(argv: list[str] | None = None) -> int: ) parser.add_argument( "--tool-args", - type=json.loads, default=None, help=( - "JSON object of arguments for the governed tool. Required off " - "loopback so the payload is one you approved for repeated " - "invocation; pass '{}' if the tool needs none." + "JSON object of arguments for the governed tool, or set " + "$CI_SMOKE_TOOL_ARGS. Required off loopback so the payload is one " + "you approved for repeated invocation; pass '{}' if the tool " + "needs none." ), ) args = parser.parse_args(argv) try: + # Resolved before the production guard reads args.tool_args, so an + # env-configured payload satisfies it exactly as the flag does. + args.tool_args = _resolve_tool_args(args.tool_args) # Validate API URL (applies to both --api-url and $API_URL) api_url = _validate_api_url(args.api_url or _get_api_url()) _require_deliberate_production_target(api_url, args) diff --git a/tests/test_constant_test_loop.py b/tests/test_constant_test_loop.py index fb025f35..5dad8381 100644 --- a/tests/test_constant_test_loop.py +++ b/tests/test_constant_test_loop.py @@ -364,11 +364,15 @@ def test_constant_test_loop_never_logs_api_key(test_server): wallet_id = provision_data["wallet_id"] key_id = provision_data["key_id"] - # Run with explicit credentials + # Run with explicit credentials. Ambient CI_SMOKE_* is stripped first: + # inheriting the whole environment lets a developer's exported + # CI_SMOKE_TOOL_ARGS reach the tool as its arguments, and the resulting + # "unexpected keyword argument" points at the tool rather than the stray + # variable. The same applies to an ambient wallet or key id. result = subprocess.run( [sys.executable, str(CONSTANT_TEST_SCRIPT), "--api-url", base_url], env={ - **os.environ, + **{k: v for k, v in os.environ.items() if not k.startswith("CI_SMOKE")}, "CI_SMOKE_AGENT_KEY": agent_key, "CI_SMOKE_WALLET_ID": wallet_id, "CI_SMOKE_KEY_ID": key_id, @@ -652,3 +656,91 @@ def test_unusable_tool_pin_exits_as_configuration_not_invariant_failure( ) assert result.returncode == 2, result.stdout + result.stderr assert "configuration error" in result.stderr + + +def test_tool_args_resolve_from_flag_environment_or_neither(monkeypatch): + """--tool-args needs the env fallback --tool and --other-tool already have. + + A CI job can otherwise satisfy every part of the off-loopback guard except + the payload, and would have to put that one remaining piece in argv. + """ + # Cleared explicitly: the "neither set" case reads the real environment, + # so an ambient CI_SMOKE_TOOL_ARGS would return a payload and fail the + # assertion even though the flag path is correct. + monkeypatch.delenv("CI_SMOKE_TOOL_ARGS", raising=False) + resolve = _load_loop()["_resolve_tool_args"] + + # The flag wins, and is parsed. + assert resolve('{"text": "from flag"}') == {"text": "from flag"} + + # Neither set: None, so the guard can still refuse off loopback. + assert resolve(None) is None + + +def test_tool_args_environment_fallback(monkeypatch): + monkeypatch.setenv("CI_SMOKE_TOOL_ARGS", '{"text": "from env"}') + resolve = _load_loop()["_resolve_tool_args"] + + assert resolve(None) == {"text": "from env"} + # An explicit flag still overrides the environment. + assert resolve('{"text": "flag wins"}') == {"text": "flag wins"} + + # An explicitly empty payload is a decision, not an absence: it must + # satisfy the guard rather than falling through to None. + monkeypatch.setenv("CI_SMOKE_TOOL_ARGS", "{}") + assert _load_loop()["_resolve_tool_args"](None) == {} + + +def test_malformed_tool_args_are_a_configuration_error(monkeypatch): + """Exit 2, not 1: a bad payload means misconfiguration, not a broken plane.""" + module = _load_loop() + resolve = module["_resolve_tool_args"] + ConfigurationError = module["ConfigurationError"] + + with pytest.raises(ConfigurationError, match="not valid JSON"): + resolve("{not json") + + # A list or scalar would be forwarded as the tool's arguments and rejected + # downstream as a tool error, which reads as the product failing. + with pytest.raises(ConfigurationError, match="must be a JSON object"): + resolve('["a", "b"]') + with pytest.raises(ConfigurationError, match="must be a JSON object"): + resolve("42") + + monkeypatch.setenv("CI_SMOKE_TOOL_ARGS", "[1, 2]") + module = _load_loop() + with pytest.raises(module["ConfigurationError"], match=r"\$CI_SMOKE_TOOL_ARGS"): + module["_resolve_tool_args"](None) + + +def test_environment_alone_satisfies_the_off_loopback_guard(): + """A CI job configured purely through secrets must get past the guard. + + Pointed at a reserved ``.invalid`` host (RFC 6761), which is non-loopback + — so both guards apply — but never resolves. Naming a real deployment + here would open an HTTPS session to it on every pytest run, including + pull-request CI, which is exactly the unattended production traffic this + guard exists to make deliberate. + """ + env = {k: v for k, v in os.environ.items() if not k.startswith("CI_SMOKE")} + env.update( + { + "PATH": os.environ.get("PATH", "/usr/bin:/bin"), + "API_URL": "https://api.invalid", + "CI_SMOKE_AGENT_KEY": "b2a_placeholder", + "CI_SMOKE_TOOL": "partner.echo", + "CI_SMOKE_TOOL_ARGS": '{"text": "ci"}', + } + ) + result = subprocess.run( + [sys.executable, str(CONSTANT_TEST_SCRIPT)], + env=env, + capture_output=True, + text=True, + timeout=60, + ) + # It gets past configuration and fails on the unresolvable host instead, + # which is the point: neither guard message appears. + assert "refusing to auto-select a tool" not in result.stderr + assert "refusing to send derived arguments" not in result.stderr + assert "b2a_placeholder" not in result.stdout + result.stderr