From e77d9a18f6d770d95edfefcc802d255b571eab07 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 20 Aug 2026 10:40:29 +0000 Subject: [PATCH 1/2] feat(scripts): let CI configure the smoke loop entirely through env MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --tool and --other-tool already fall back to $CI_SMOKE_TOOL and $CI_SMOKE_OTHER_TOOL, but --tool-args had no equivalent. A CI job could therefore satisfy every part of the off-loopback guard except the payload, and had to reach for argv to supply the one remaining piece — which for a secret-adjacent value is the wrong place, and which makes the guard read as an obstacle rather than a check. --tool-args now falls back to $CI_SMOKE_TOOL_ARGS. This is not cosmetic: PR 327 wires the loop into CI and its production step invokes it with neither flag, so the guard added in #326 exits 2 on every push to main. Configuring that step through secrets is only possible once the payload has an environment path. An explicitly empty payload set through the environment counts as a decision, exactly as --tool-args '{}' does on the command line, so it satisfies the guard rather than falling through to "unset". Malformed values are ConfigurationError (exit 2), never SmokeTestFailure: invalid JSON, or valid JSON that is not an object, means the loop was configured wrong rather than that the trust plane broke. The message names which source was bad, since --tool-args and $CI_SMOKE_TOOL_ARGS fail identically otherwise. A list or scalar is rejected up front rather than forwarded as the tool's arguments and refused downstream, where it would surface as a tool error and read as the product failing. Dropping argparse's type=json.loads is part of that: it exits 2 for bad JSON too, but with usage text that never says what was wrong with the payload. Verified against production with a placeholder credential: with only CI_SMOKE_TOOL and CI_SMOKE_TOOL_ARGS set, the run reaches real authentication instead of either guard, and a malformed value reports "$CI_SMOKE_TOOL_ARGS is not valid JSON" at exit 2. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Ri576RW3ocgqpx1xYN9vBm --- docs/constant-test-loop.md | 15 +++++++ scripts/constant_test_loop.py | 46 +++++++++++++++++-- tests/test_constant_test_loop.py | 77 ++++++++++++++++++++++++++++++++ 3 files changed, 134 insertions(+), 4 deletions(-) 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..267458c2 100644 --- a/tests/test_constant_test_loop.py +++ b/tests/test_constant_test_loop.py @@ -652,3 +652,80 @@ 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(): + """--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. + """ + 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.""" + 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.thisisatest.tech", + "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 placeholder credential + # 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 From 31064f07e240fee20fba3ce893946c0399962d88 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 20 Aug 2026 10:46:24 +0000 Subject: [PATCH 2/2] test: stop the guard test touching production, and scrub ambient CI_SMOKE MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit test_environment_alone_satisfies_the_off_loopback_guard pointed at api.thisisatest.tech, so every pytest run — including pull-request CI — opened a real HTTPS session to production, and a slow reject would surface as TimeoutExpired rather than a configuration assertion. That is precisely the unattended production traffic the guard under test exists to make deliberate. It now targets a reserved .invalid host (RFC 6761): still non-loopback, so both guards apply, but it never resolves. test_tool_args_resolve_from_flag_environment_or_neither asserted that _resolve_tool_args(None) returns None without clearing CI_SMOKE_TOOL_ARGS. The resolver reads the real environment, so an ambient value made the assertion fail even though the flag path was correct. Running the suite under a deliberately hostile environment to check that second fix surfaced a third problem, which the new fallback made reachable: test_constant_test_loop_never_logs_api_key passes **os.environ through to the subprocess, so an exported CI_SMOKE_TOOL_ARGS is forwarded as the tool's arguments and the run dies with "_write_note() got an unexpected keyword argument 'ambient'" — a message that points at the tool rather than at the stray variable. The fragility predates this branch (an ambient CI_SMOKE_WALLET_ID would mislead the same way) but only became reachable once the payload gained an environment path. That test now strips CI_SMOKE_* before setting the three values it means to set, matching what the other subprocess tests here already do. Verified with CI_SMOKE_TOOL_ARGS, CI_SMOKE_TOOL and CI_SMOKE_WALLET_ID all exported to junk: 19 passed. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Ri576RW3ocgqpx1xYN9vBm --- tests/test_constant_test_loop.py | 29 ++++++++++++++++++++++------- 1 file changed, 22 insertions(+), 7 deletions(-) diff --git a/tests/test_constant_test_loop.py b/tests/test_constant_test_loop.py index 267458c2..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, @@ -654,12 +658,16 @@ def test_unusable_tool_pin_exits_as_configuration_not_invariant_failure( assert "configuration error" in result.stderr -def test_tool_args_resolve_from_flag_environment_or_neither(): +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. @@ -706,12 +714,19 @@ def test_malformed_tool_args_are_a_configuration_error(monkeypatch): def test_environment_alone_satisfies_the_off_loopback_guard(): - """A CI job configured purely through secrets must get past the 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.thisisatest.tech", + "API_URL": "https://api.invalid", "CI_SMOKE_AGENT_KEY": "b2a_placeholder", "CI_SMOKE_TOOL": "partner.echo", "CI_SMOKE_TOOL_ARGS": '{"text": "ci"}', @@ -724,8 +739,8 @@ def test_environment_alone_satisfies_the_off_loopback_guard(): text=True, timeout=60, ) - # It gets past configuration and fails on the placeholder credential - # instead, which is the point: neither guard message appears. + # 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