Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ PYTEST := uv run --no-project --python 3.13 --with pytest --with hypothesis --wi

.DEFAULT_GOAL := help
.PHONY: help ci typecheck test lint synth deploy deploy-endpoints seed-registry create-harnesses \
smoke reset destroy demo clean dist
smoke reset destroy demo clean dist sync-action-pins
# `dist` was added as a target a few rounds ago and NOT declared here. It went unnoticed
# because tests/test_makefile.py's KEY_TARGETS list had drifted to 13 of the 16 targets, so
# the PHONY check never covered it. Both are fixed together.
Expand Down Expand Up @@ -81,6 +81,15 @@ test: ## Run the offline test suite (hermetic, no AWS).
lint: ## Static-check the Python with ruff.
uv run --no-project --python 3.13 --with ruff ruff check .

sync-action-pins: ## Re-derive the INV-CI-3 pin table + version comments from GitHub (needs `gh`).
# The supported way to fix `test_action_pin_comments.py` after a SHA bump. Dependabot bumps
# Actions weekly and cannot update the authoritative table, so without this the only route was
# hand-copying a 40-hex SHA and hand-resolving it — the manual work the guard exists to remove.
# Dry-run first, on purpose: this rewrites a test module, so the diff gets read before it lands.
uv run python scripts/sync_action_pins.py
@echo ""
@echo "Dry run above. To apply: uv run python scripts/sync_action_pins.py --write"

synth: ## CDK synth the 9 Layer-3 stacks locally (offline, no deploy).
cd iac-cdk && npx cdk synth

Expand Down
1 change: 1 addition & 0 deletions docs/INVARIANTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -876,6 +876,7 @@ the packaged install path.
| **INV-SUPPLY-1** | Every dependency manifest in the repo is covered by a Dependabot ecosystem AND reachable by CI's vulnerability audit. Dependabot declared three ecosystems (pip at the root, npm under `iac-cdk/`, github-actions) while the repo also ships **five container manifests** — `specialists/{adversarial-reviewer,attack-mapper,cve-intel,threat-hunt}/requirements.txt` and `longrunning/bas-runner/requirements.txt` — plus a Dockerfile beside each. Those are the deps that run in the DEPLOYED Runtime images, and they were patched by nobody and audited by nothing: Dependabot's entire commit history in this repo touches only `.github/workflows/` (every commit that ever changed a container requirements.txt is a human one, including `3e1dacf`, a manual "bump bedrock-agentcore across all 4 specialists"), and `pip-audit` built its audit set from `pyproject.toml`'s `project.dependencies` alone. Measured consequence: each of the four specialists carried **19 known advisories in 2 packages** — `litellm` (12) and `starlette` (7) — both TRANSITIVE via `strands-agents[a2a,litellm]==1.9.1` and `fastapi==0.139.0`, so neither was visible as a line in any file. `longrunning/bas-runner` came back clean, the negative control proving the audit discriminated rather than reporting red for everything. Fixed in three places: `directories:`-based pip + docker Dependabot entries covering all five (grouped into one PR because `test_specialist_containers.py` asserts the four specialists ship a byte-identical set, so staggered PRs would each break that invariant); a BLOCKING container-audit step in `supply-chain.yml` whose loop propagates per-file failure (`exit $status` — a loop swallowing the code would print advisories and still report green); and the upgrade itself to `strands-agents==1.50.2` / `fastapi==0.141.1`, verified to pull `litellm 1.91.1` + `starlette 1.4.1` and to keep all six APIs the code imports (`Agent`, `LiteLLMModel`, `A2AServer`, `MCPClient`, `BedrockAgentCoreApp`, `mcp.server.Server`) plus INV-MCP-5's 1.x decorator surface — 19 advisories to **0**, re-measured by running CI's actual loop. Also removed the SIXTH copy of one fact: `ci.yml`'s `real-stack` job re-typed `strands-agents==1.9.1` under a comment claiming it was "the SAME version the specialist containers pin" — a claim, not a fact, and already stale, so the job verifying the shipped stack verified a version nothing shipped; it now installs `-r specialists/cve-intel/requirements.txt`, the INV-CI-1 fix applied again. Recorded method failure: the audit-coupling assertion first asked `"requirements.txt" in text` and passed **vacuously** — pip-audit's own step writes a temp `audit-requirements.txt`, so the substring was satisfied while the container audit did not exist; it now parses the YAML and requires a pip-audit invocation naming a real `(specialists\|longrunning)/…/requirements.txt`. A substring standing in for a structural question, this time inside the guard written to catch it. Vulnerability freedom is deliberately NOT asserted offline (it needs a live advisory DB and would make the suite time-dependent); the offline guard asserts nothing is left OUT of the audit's reach. | `.github/dependabot.yml` · `.github/workflows/supply-chain.yml` · `specialists/*/requirements.txt` | `test_dependabot_covers_every_manifest.py` |
| **INV-CI-4** | Every CI job bounds its runtime, and `concurrency` is decided per workflow SEMANTICS rather than uniformly. Two runtime protections were missing across all six workflows. **(1)** No job declared `timeout-minutes` — all 14 inherited GitHub's default of **360 minutes**, so a hung job (a network read with no timeout, a test waiting on a lock, an `npm install` against a degraded registry) would hold a runner for six hours; on a public repo that is the whole concurrency budget, so one wedged job blocks every other PR's CI. Values are calibrated to MEASURED durations from five real runs, not guessed: `test` 321-345s -> 15min, `codeql analyze` 65-94s -> 20, `iac` 74-88s -> 10, `real-stack` 57-63s -> 10, `pip-audit` 25-90s -> 15 (it grew when the container audit landed, INV-SUPPLY-1), `mypy` 20s -> 10, `bandit` 13s -> 5, `secret-and-name scan` 5s -> 3. The guard asserts each timeout stays within 2x-40x its measured seconds, because a tolerance is calibrated to a MAGNITUDE — INV-DOC-9 records the cost of borrowing one across magnitudes — so too tight turns variance into flakes and too loose is decoration. That band immediately caught two of my own values (bandit 10min = 46x, secret-scan 5min = 60x), which were tightened rather than the guard being relaxed. **(2)** Five of six workflows had no `concurrency` block, so superseded runs kept going — pushing twice to a PR ran the full matrix twice while only the last verdict was ever read. Crucially this is NOT a uniform fix: `ci`/`codeql`/`supply-chain`/`scorecard` cancel, but **`release.yml` must NEVER cancel**. It publishes, and cancelling midway leaves a state no retry cleanly repairs — a GitHub Release created and tagged while PyPI never received the upload, or an attestation signed for artifacts that were never published. Two tags pushed in quick succession is exactly when `cancel-in-progress: true` would fire, i.e. precisely when it does the most damage; so release groups by tag (serialising a re-run of the same tag) with `cancel-in-progress: false`, and that asymmetry is asserted because "add concurrency everywhere" is the obvious cleanup that would break it. **A negative result, recorded so it is not tidied up:** `docs.yml` keys its group on the literal `pages` with no `github.ref`, so in principle a PR's docs build can cancel main's in-flight Pages deployment. Adding a ref is wrong twice — measured across 40 historical docs runs (19 push/main + 21 pull_request) the conclusion was `success` every time, **zero cancellations**, and a global group is the POINT since Pages deployment is a singleton and keying on ref would PERMIT the concurrent deploys the single group prevents. The exemption therefore has its own guard (the group must stay ref-independent), the rule this repo applies to lint-excluded directories applied to a config exception. Also **verified and deliberately left alone**: permissions were ALREADY least-privilege — every workflow declares top-level `contents: read` (or `read-all`) and only the four jobs needing more widen it themselves — so this round changed nothing there, but the property is now asserted in both directions (no top-level write; the set of write-scoped jobs matches a recorded map) since a future job could otherwise quietly inherit write access. Guard mutation-tested 9/9, including the two dangerous cleanups. | `.github/workflows/*.yml` job timeouts · concurrency · permissions | `test_workflow_runtime_guards.py` |
| **INV-SUPPLY-2** | A version bound that exists for a BREAKAGE is declared where Dependabot reads it. `mcp>=1.0,<2` was stated in three places — a long `pyproject.toml` comment, INV-MCP-5 in this document, and `test_mcp_version_bound.py` — and **Dependabot reads none of them**, so it proposed lifting the bound twice in one week: **PR #59** widened `pyproject.toml` to `mcp>=1.0,<3`, and **PR #60** — the very first PR produced by the container coverage added in INV-SUPPLY-1 — pinned all four DEPLOYED specialists to `mcp==2.0.0`. Both re-verified against the real 2.0.0 release rather than trusted from the record, because a bound whose evidence has expired is worse than no bound: `from mcp.server import Server` still imports (an import check is not a compatibility check — INV-MCP-5's own recorded lesson), but `Server.list_tools` / `.call_tool` are GONE and `create_server()` raises `AttributeError: 'Server' object has no attribute 'list_tools'`, so `sentinel mcp serve` cannot start. The part that could have been missed: that is the SERVER surface, which the specialists never touch — they use the CLIENT surface, so "the same bound applies" was an assumption needing its own measurement. It held for an INDEPENDENT reason: `from mcp.client.streamable_http import streamablehttp_client` also raises ImportError on 2.0, and all four `specialists/*/agent_a2a.py` import it to reach the Gateway. **Two separate breakages behind one version number**, and PR #60 would have shipped both into the containers. Fixed by declaring the bound in `.github/dependabot.yml` with its evidence and its lift procedure — generalising what `iac-cdk`'s TypeScript bound already did right (INV-IAC), i.e. "a fix applied to one call site is not an invariant" applied to a supply-chain declaration. The non-obvious detail the guard encodes: **`ignore` is PER-UPDATE-BLOCK, not global**, so the root `pip` entry's ignore does not cover the container `pip` entry — which is exactly why PR #60 existed — and a guard checking only "mcp is ignored somewhere" would have passed while the dangerous PR was open. So every block of the relevant ecosystem is checked. Also asserted: each ignore must carry a VERSION RANGE (a bare `ignore: mcp` silently blocks 1.x security patches too, turning a compatibility bound into an unmaintained dependency), the pyproject bound and the ignore must describe the SAME boundary (widening to `<3` while the ignore stays at `>=2.0.0` is the worst combination — the repo permits a version Dependabot has stopped warning about), and the file must record HOW to re-verify so the bound can be lifted deliberately rather than becoming permanent by default. Mutation-tested 7/7, including the exact PR #59 and PR #60 conditions. | `.github/dependabot.yml` ignore blocks · `pyproject.toml` bounds | `test_breaking_bounds_are_told_to_dependabot.py` |
| **INV-CI-5** | A guard that demands a machine-derived table SHIPS a working way to derive it, and its failure message names that tool. INV-CI-3 works — it blocked Dependabot **PR #61**, which bumped the three `github/codeql-action` sub-actions to a SHA the `_AUTHORITATIVE` table did not know — but it was **not actionable**. Its message said to resolve each SHA against GitHub and that "`SENTINEL_VERIFY_ACTION_PINS=1` does this". **That was false**, measured by reproducing PR #61 in the working tree: the offline layer reported `2 failed` while the online layer **PASSED**, because it iterates only entries already in the table and therefore never sees a brand-new SHA. So the only remaining route was a human hand-copying a 40-hex SHA and hand-resolving it against the API — precisely the manual work INV-CI-3 exists to eliminate, on Dependabot's weekly cadence. A guard that converts the labour it prevents into a mandatory ceremony has traded one defect for another; "the guards are code too, so they get invariants" (round 18) is why this one exists. Fixed with `scripts/sync_action_pins.py` + `make sync-action-pins`, which re-derives table and comments from the AUTHORITATIVE direction (SHA -> the tag pointing at it, never comment -> table, since reading the comment would launder a stale label into the source of truth — INV-CI-3's defect verbatim). Verified end-to-end against PR #61's exact state: guard fails -> one command -> guard passes, table gained `d1ba80a13dd9 = v4.37.5` and dropped the superseded entry, 3 comments rewritten; and on a clean tree it reports `nothing to do` for all 12 pins, the negative control proving tool and guard agree. It REFUSES to write when any SHA is unresolvable (verified live: `rc=1`, an explicit UNRESOLVED line, zero writes) because a partial table is worse than a stale one — the guard would pass on the resolved entries while the suspicious pin stayed unrecorded. **Two of my own defects are recorded here.** (1) The tool's first ranking picked among tags by counting dot-separated components, and `actions/deploy-pages` has three tags on one commit (`v5.0.0`, the moving alias `v5`, and the historical re-tag `v3.0.2-node.24`); the count chose `v3.0.2-node.24`, so the tool proposed rewriting a CORRECT comment into a misleading one — the guard's own defect class, reintroduced by its remediation. Now ranked semantically and pinned by a test over the real ambiguous tag set. (2) Three of this module's own assertions initially SURVIVED mutation: `"/tags" in <all strings>` was satisfied by the resolver's **docstring** while the code queried `/commits` (documentation standing in for implementation); `"refusing to write" in main()` was satisfied by a SECOND refuse-to-write path when the first was deleted; and locating the gate by `any(Name == "unresolved")` accepted `if False and unresolved:` — a guard must test whether a gate can FIRE, not whether it exists. All three now resolve structurally (call arguments only, branch-scoped strings, exact-condition match). Mutation-tested 10/10. | `scripts/sync_action_pins.py` · `Makefile` · INV-CI-3 failure messages | `test_pin_sync_tool.py` |

---

Expand Down
14 changes: 14 additions & 0 deletions scripts/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
"""Repository maintenance tooling — not part of the shipped package.

`__init__.py` is REQUIRED here, not decorative. Without it `scripts/` resolves as a namespace
package, and INV-PKG-1 records what that costs: the repo once shipped a top-level `litellm/`
namespace package while `litellm` is also a PyPI dependency every specialist installs, so a regular
installed package silently outranked it and `litellm.gateway` stopped importing in the only
environment it targeted. `scripts` is a plausible name for an installed package too, so the same
trap applies.

Nothing here is imported by `sentinel_harness`; these are maintainer entry points run via
`make <target>`. They are deliberately excluded from the sdist for the same reason `.github/` is —
CI and maintenance tooling is not source, and a downstream packager (conda-forge, Debian, Fedora)
should not have their build depend on it. See `tests/test_sdist_contents.py`.
"""
Loading