From b6e48420e86bda7dbb5f558cbe7f085dca0b0cd5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Neo=20=E5=AD=AB?= Date: Tue, 4 Aug 2026 20:00:10 +0800 Subject: [PATCH] test: guard the process-globals rounds 20-21 introduced MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A fourth test sweep took three layers no previous sweep had: the installed-wheel surface, mutation testing of five more invariant families, and process-global isolation. The first two found nothing to fix; the third found a defect in the test I was writing. **Installed-wheel E2E (8/8).** Built a wheel, installed it outside the repo, and exercised what a pip user actually gets: the package + 5 submodules import, the registry resolves from the INSTALLED copy (INV-MCP-2), the MCP gate obtains a real governance decision (INV-MCP-1), `coerce_bool` rejects string truthiness (INV-COERCE), the egress guard refuses every metadata spelling (INV-EGRESS), the `sentinel` console script runs, 4 tool handlers import, and promotion is refused with no witnessed gate (INV-OPS-7). Every security invariant holds on the real pip path, not just in a checkout. **Mutation testing, 6/6 caught** across five families never mutated before β€” INV-PROMOTE-3 (both the missing-safety-dims check and its enabling switch), INV-SANDBOX-3, INV-GOV-4 (ledger truncation AND the anchored tail-hash comparison), INV-PLAY-1. With round 23 that is 12/12 over 9 families. Same self-checking harness: a control mutation runs first and nothing is reported unless it fails. **The isolation guards, and the bug in them.** Rounds 20-21 added three process-globals, each a way for one test to weaken a check for everything after it. The new file asserts none leaks. Positive-controlling it found that my behavioural assertion was too weak: assert out["ok"] is False # PASSED with the witness deliberately leaked With the witness leaked the gate correctly OPENS, the call reaches the control plane, and the handler returns ok:False with `upstream_error` β€” so the loose assertion passed for the wrong reason while the gate was disabled. The environment assertion caught the planted leak; the behavioural one did not. It now asserts the error KIND. Two assertions that look like they test the same thing can differ in strength, and the weaker one passing is how a real leak would ship. That positive control also surfaced a self-harm risk worth stating plainly: with the witness leaked, the handler issued a genuine `CreateHarnessEndpoint`. It was rejected (`UnrecognizedClientException`) only because the ambient credentials happened to be invalid. On a machine with working credentials, a test in a suite that advertises ZERO AWS would have created a production endpoint. The control plane is now stubbed with an object whose every attribute raises, so a leak is caught by assertion rather than by a network round trip. Verified: no botocore frame appears, the stub intercepts, and the leak is still detected. The file is `zz_`-prefixed so the default alphabetical order collects it last β€” these assertions are about the state the rest of the suite leaves behind. Under this repo's `pytest-randomly` default that becomes a sample rather than a guarantee, which the module docstring says outright rather than pretending otherwise. Tested: 3715 passed / 6 skipped in BOTH fixed and random order; installed-wheel E2E 8/8; scenario E2E 25/25; README CLI E2E 13/13; IaC E2E tsc 0 / 8-of-8 stack tests / synth 9 stacks; ruff clean; both mypy gates clean; make ci green. --- README.md | 8 +- docs/FIDELITY-REPORT.md | 2 +- docs/ROADMAP.md | 8 +- tests/test_zz_process_isolation.py | 130 +++++++++++++++++++++++++++++ 4 files changed, 139 insertions(+), 9 deletions(-) create mode 100644 tests/test_zz_process_isolation.py diff --git a/README.md b/README.md index 2d2ef66..1f1b218 100644 --- a/README.md +++ b/README.md @@ -13,7 +13,7 @@ python bedrock-agentcore version - tests + tests coverage milestones hardening @@ -99,7 +99,7 @@ Honest build status per capability β€” mirrors the self-audit. | **Tools** | `nvd_lookup` / `epss_kev` / `attack_lookup` / `web_search` | 🟑 **reference stubs** (offline-safe, contract-tested) | `tools/`, `tests/test_tool_handlers.py` | | **Tools** | `siem_query` / `asset_lookup` / `enrich_ioc` / `ops_query` β€” backend-pluggable | 🟒 **built + tested** (offline mock default; `*_LIVE`=1 switches to a real stdlib-HTTP client β€” env-driven URL + bearer, timeouts, all failuresβ†’`upstream_error` with no silent fallback β€” proven end-to-end against an in-process 127.0.0.1 mock server, zero external network) | `tools/{siem_query,asset_lookup,enrich_ioc,ops_query}/`, `tests/test_*_live.py` | -🟒 built & validated Β· 🟑 built, partial Β· 🟠 designed with loadable config Β· βšͺ design narrative only. **3711 offline tests pass** (+6 skipped when optional deps absent). +🟒 built & validated Β· 🟑 built, partial Β· 🟠 designed with loadable config Β· βšͺ design narrative only. **3715 offline tests pass** (+6 skipped when optional deps absent). ## πŸš€ Quickstart @@ -247,7 +247,7 @@ Borrowed patterns (see [`docs/BLUEPRINT.md`](docs/BLUEPRINT.md)): supervisorβ†’s | [`docs/GOVERNANCE.md`](docs/GOVERNANCE.md) | Registry dual-gate, HITL, sandbox hooks, and tag-guard controls | | [`docs/COMPLIANCE.md`](docs/COMPLIANCE.md) | Capability β†’ SOC 2 / ISO 27001 / NIST CSF 2.0 control mapping (anchors machine-verified) | | [`docs/OBSERVABILITY.md`](docs/OBSERVABILITY.md) | Logging (`logutil`), metrics (token/latency/tool-call/error/eval), the OTEL/Transaction-Search path | -| [`docs/TESTING.md`](docs/TESTING.md) | The 3711-test offline suite: layout, determinism, how to run | +| [`docs/TESTING.md`](docs/TESTING.md) | The 3715-test offline suite: layout, determinism, how to run | | [`docs/FIDELITY-REPORT.md`](docs/FIDELITY-REPORT.md) | The self-audit β€” real vs. built vs. designed, with limits stated | | [`docs/ROADMAP.md`](docs/ROADMAP.md) | Delivered milestones (M0–M12) and what's next | | [**API reference (live)**](https://aws-samples.github.io/sample-sentinel-harness/) | Rendered `sentinel_harness` API docs (pdoc β†’ GitHub Pages) | @@ -285,7 +285,7 @@ sentinel-harness/ β”œβ”€β”€ iac-cdk/ L3 CDK stacks (9; guardrail/identity/obs/vpc live) 🟒 β”œβ”€β”€ iac-terraform/ deployable Terraform mirror (validate-clean) 🟒 β”œβ”€β”€ docs/ QUICKSTART Β· ARCHITECTURE Β· BLUEPRINT Β· SETUP Β· HARNESSES Β· GOVERNANCE Β· TESTING Β· FIDELITY-REPORT Β· ROADMAP -β”œβ”€β”€ tests/ offline unit + config tests (3711) 🟒 +β”œβ”€β”€ tests/ offline unit + config tests (3715) 🟒 └── .github/workflows/ CI incl. a customer-name / secret gate ``` diff --git a/docs/FIDELITY-REPORT.md b/docs/FIDELITY-REPORT.md index 5e17a17..f56ea08 100644 --- a/docs/FIDELITY-REPORT.md +++ b/docs/FIDELITY-REPORT.md @@ -29,7 +29,7 @@ This is real, not narration: - **Layer 3 foundation ships runnable, tested code.** The dual-gate tool/skill registry, PreToolUse sandbox hook, and Agent Factory (fleet provision, dry-run, cross-env tag-guard) are built and unit-tested. - **Native IaC, not hand-rolled resources.** The gateway / registry / memory / harness stacks use the native `AWS::BedrockAgentCore::*` CloudFormation types. Per the README status matrix, the **Gateway and Memory CFN types are registered**; the Registry type is not yet in CFN (see limitations). - **Config path works.** `pip install -e .` succeeds (`[tool.setuptools] packages = ["sentinel_harness", "intake"]`), the `sentinel` console script works, and `sentinel create ` loads real config via `sentinel_harness/loader.py` (systemPrompt file read, `bedrockModelConfig` / `agentCoreGateway` / `managedMemoryConfiguration` mapping, `${ENV}` expansion, `@gateway/tool` allowedTools grammar). -- **Scale.** 3711 offline tests pass (+6 skipped when optional deps absent) across 137 test files, with 37 evidence JSON artifacts, 22 scenarios, 20 tools (incl. a 7-tool deterministic detection-engineering suite), an `iac-cdk` project (9 stacks synth-green) and an `iac-terraform` mirror (`validate`-clean). +- **Scale.** 3715 offline tests pass (+6 skipped when optional deps absent) across 137 test files, with 37 evidence JSON artifacts, 22 scenarios, 20 tools (incl. a 7-tool deterministic detection-engineering suite), an `iac-cdk` project (9 stacks synth-green) and an `iac-terraform` mirror (`validate`-clean). - **Clean anonymization.** No real account IDs (only the `000000000000` placeholder), no customer or company names, no secrets. The CI secret-and-name scan is self-non-matching and fails the build on any hit. ## 4. Live controls retained for demos (us-east-1) diff --git a/docs/ROADMAP.md b/docs/ROADMAP.md index b5ddd1d..85676b1 100644 --- a/docs/ROADMAP.md +++ b/docs/ROADMAP.md @@ -51,7 +51,7 @@ live) Β· 🟑 skeleton / partial Β· πŸ”΄ gap. | `specialists/` | `cve-intel` (docker-build + live-validated on AgentCore Runtime) + `attack-mapper` / `threat-hunt` (real graph/plan builders) + `adversarial-reviewer` (agent_a2a + local_a2a + two-stage Dockerfile + contract test) | βœ… | all four specialists shipped | | `longrunning/` | `bas-runner` (BAS case-gen + detection-replay) + `detonation` (full simulated microVM lifecycle + orchestrator) | 🟩 | both built + tested; detonation stays an honest SIMULATED no-op | | `iac-cdk/lib/` | 9 synth-green stacks β€” `gateway` / `registry` / `memory` / `network` / `identity` / `guardrail` / `observability` / `harness` / `runtime` (+ `iam`); `iac-terraform/` mirror is `terraform validate`-clean | βœ… | `guardrail` / `identity` / `observability` LIVE-deployed (us-east-1); the Registry + `runtime` custom-resource/raw-CfnResource stacks synth clean but fail on deploy until their CFN types are GA (both control-plane APIs are separately live-verified β€” Registry via `registry_live.py`, `CreateAgentRuntime` via a real arm64 microVM that served a live A2A call, HTTP 200, real Bedrock model, on a non-prod test account, then torn down β€” `evidence/live_a2a_runtime_result.json`) | -| `tests/` | 137 files, **3711 offline passing** (+6 skipped) | βœ… | add tests with each new module | +| `tests/` | 137 files, **3715 offline passing** (+6 skipped) | βœ… | add tests with each new module | | `evidence/` | 37 evidence sets | βœ… | add one per milestone | ### 0.3 Fit score (vs. a full three-layer SecOps agent program) @@ -181,8 +181,8 @@ Each milestone gives: **goal / files / reused APIs / acceptance (live evidence) Suggest one feature branch per milestone. ### M0 β€” Environment & baseline reproduction (half a day) -**Goal:** on a fresh machine, get all 3711 offline tests green and reproduce β‰₯1 live scenario. -- [ ] `uv sync` + `uv run pytest -q` β†’ 3711 passing (+6 skipped) (offline). +**Goal:** on a fresh machine, get all 3715 offline tests green and reproduce β‰₯1 live scenario. +- [ ] `uv sync` + `uv run pytest -q` β†’ 3715 passing (+6 skipped) (offline). - [ ] Configure `SENTINEL_EXECUTION_ROLE_ARN` / `SENTINEL_REGION` / `AWS_PROFILE` (non-prod) β€” see `docs/SETUP.md`. - [ ] Run `scenarios/scenario_cve_triage.py`; compare `evidence/cve_triage_result.json` shape. - [ ] Run `scenarios/scenario_hitl_resume.py`; reproduce pauseβ†’approveβ†’resume. @@ -419,7 +419,7 @@ hand-off reuses the live-capable M1/M2 engine (driven offline here, labeled a wi (`make deploy`, cost note, `make destroy`) + the no-lock-in export. β€” `docs/QUICKSTART.md` - [x] `tests/smoke/`: offline acceptance suite (default offline; `SENTINEL_SMOKE_LIVE=1` opt-in for live). β€” `tests/smoke/` -**Acceptance:** `make test` β†’ 3711 offline tests green; `make seed-registry` β†’ dual-gate `ok`; +**Acceptance:** `make test` β†’ 3715 offline tests green; `make seed-registry` β†’ dual-gate `ok`; `make create-harnesses` (DRY_RUN=1) β†’ 8 harnesses validate offline with zero AWS; `sentinel export` β†’ valid compilable Strands Python; `make smoke` β†’ the offline acceptance suite green. A fresh non-prod account can then run `make deploy` (free-tier foundation) and the live scenarios; `make destroy` tears it all down. diff --git a/tests/test_zz_process_isolation.py b/tests/test_zz_process_isolation.py new file mode 100644 index 0000000..796efd5 --- /dev/null +++ b/tests/test_zz_process_isolation.py @@ -0,0 +1,130 @@ +""" +Process-global state must not leak between tests. +================================================ +Rounds 20-21 introduced three process-globals, and each is a way for one test to weaken a +security check for every test that runs after it: + + SENTINEL_PROMOTION_GATE_WITNESSED (env) INV-OPS-7 β€” a leak DISABLES the promotion + gate process-wide, which is the exact + failure the witness exists to prevent + the sentinel_harness.metric log handler INV-METRIC-1 β€” a stacking bug duplicates + every emitted metric line + logger.propagate = False pre-existing; it silently broke a `caplog` + assertion in round 20 depending on whether + an earlier test had called + configure_logging() + +The file name starts with `zz_` so pytest collects it LAST in the default alphabetical +order: these assertions are about the state the rest of the suite leaves behind, so running +them first would prove nothing. + +Under `pytest-randomly` (this repo's default) collection order is shuffled, so these become +a sample rather than a guarantee β€” still useful, and the `-p no:randomly` CI leg gives the +ordered run. Recorded rather than worked around: an ordering-dependent check that pretends +to be absolute is worse than one that says what it covers. + +A NOTE ON THE SECOND ASSERTION, which was wrong first +----------------------------------------------------- +The behavioural check originally read `assert out["ok"] is False`. With the witness +deliberately leaked, that PASSED β€” for the wrong reason. The gate had correctly opened, the +call proceeded to AWS, and it failed on `NoCredentialsError`, so `ok` was False with +`error == "upstream_error"`. The environment assertion caught the planted leak; the +behavioural one did not. + +So it now asserts the error KIND. Two assertions that look like they test the same thing +can differ in strength, and the weaker one passing is how a real leak would ship. +""" +from __future__ import annotations + +import logging +import os + +WITNESS_ENV = "SENTINEL_PROMOTION_GATE_WITNESSED" + + +def test_the_promotion_witness_did_not_leak(): + """A leaked witness means every later caller in this process can promote unattended.""" + value = os.environ.get(WITNESS_ENV) + assert value is None, ( + f"{WITNESS_ENV} leaked into the process environment as {value!r}. Some test set " + "it without cleanup, and INV-OPS-7's tool-side promotion gate is now open for " + "everything that runs afterwards. Use the `gate_witnessed` fixture (monkeypatch, " + "auto-reverted) or `agent_loop._with_promotion_witness`, which restores the prior " + "value in a `finally`." + ) + + +def test_the_promotion_gate_still_refuses_at_the_end_of_the_suite(monkeypatch): + """The behavioural half: ask the real tool after everything else has run. + + Asserts the error KIND, not merely `ok is False`. With a leaked witness the gate opens, + the call reaches the control plane and returns `ok: False` with `upstream_error` β€” so + the loose version of this assertion passed while the gate was disabled. + + The control plane is stubbed, deliberately. Positive-controlling this test surfaced a + real hazard in an earlier draft: with a leaked witness the gate opens and the handler + issues a genuine `CreateHarnessEndpoint`. It was rejected + (`UnrecognizedClientException`) only because the ambient credentials were invalid β€” on + a machine with working credentials, a test in a suite that advertises ZERO AWS would + have created a production endpoint. Stubbing makes that impossible rather than + unlikely. + """ + import importlib.util + import pathlib + + handler_path = (pathlib.Path(__file__).resolve().parent.parent + / "tools" / "harness_ops" / "handler.py") + spec = importlib.util.spec_from_file_location("_isolation_harness_ops", handler_path) + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + + class _RefuseToCall: + """Any control-plane call is a test failure, not a network round trip.""" + + def __getattr__(self, name): + def _boom(**_kwargs): + raise AssertionError( + f"the promotion gate let a real control-plane call through: " + f"{name}(...). The witness leaked." + ) + return _boom + + from sentinel_harness import core + monkeypatch.setattr(core, "_control", _RefuseToCall()) + + out = module.handler( + {"action": "promote_endpoint", + "params": {"harness_id": "h-isolation", "endpoint_name": "prod"}}, + None, + ) + assert out["ok"] is False, f"promotion was ALLOWED at end of suite: {out}" + assert out.get("error") == "validation_error", ( + f"the promotion was refused, but not BY THE GATE β€” error={out.get('error')!r}, " + f"message={str(out.get('message'))[:160]!r}. An `upstream_error` here means the " + "gate OPENED and the call reached the control plane, i.e. the witness leaked and " + "only the absence of credentials stopped it." + ) + assert "human-approval gate" in str(out.get("message")), out.get("message") + + +def test_the_metric_handler_did_not_stack(): + """`logutil.get_metric_sink` is idempotent by design (one tagged handler). A stacking + regression would duplicate every metric line, which reads as double the token spend.""" + logger = logging.getLogger("sentinel_harness.metric") + tagged = [h for h in logger.handlers + if getattr(h, "_sentinel_metric_handler", False)] + assert len(tagged) <= 1, ( + f"{len(tagged)} tagged metric handlers are attached: {tagged}. " + "get_metric_sink must reuse the one handler, not append another." + ) + + +def test_no_test_left_the_repo_registry_path_redirected(): + """`SENTINEL_REGISTRY_PATH` overrides the governance registry. Left set, it points every + later caller β€” including the MCP server's approved-set load β€” at whatever file the last + test used.""" + value = os.environ.get("SENTINEL_REGISTRY_PATH") + assert value is None, ( + f"SENTINEL_REGISTRY_PATH leaked as {value!r}; the registry every later test reads " + "is whatever that path holds. Set it with monkeypatch.setenv, never os.environ." + )