test(sandbox): a static AST guard pins the codec and worker import boundary (BACKLOG #346) - #252
Closed
wshallwshall wants to merge 2 commits into
Closed
test(sandbox): a static AST guard pins the codec and worker import boundary (BACKLOG #346)#252wshallwshall wants to merge 2 commits into
wshallwshall wants to merge 2 commits into
Conversation
…ry (BACKLOG #346) The sandbox's import boundary (DEFAULT_FORBIDDEN_MODULES in pipeline/sandbox.py -- socket/ssl/asyncio, the I/O- and secret-bearing messagefoundry.* subpackages, cryptography) is enforced only at RUNTIME and only inside the off-by-default [sandbox].mode=subprocess child. Nothing statically pins that the two modules which run inside that boundary -- _sandbox_codec.py and _sandbox_worker.py -- do not themselves import a forbidden module. Both are clean today; a future edit reintroducing a forbidden import would make mode=subprocess DOA on first deployment while the default-mode suite stayed green -- the failure inverts, hitting the most security-conscious installs hardest and quietest. This is defence-in-depth test coverage, not a code change: neither sandbox.py nor the codec is touched. tests/test_sandbox_import_boundary.py walks the two files' own ast import nodes (ast.Import/ast.ImportFrom, including nested/function-level and relative imports resolved to absolute) and asserts none resolves under a DEFAULT_FORBIDDEN_MODULES prefix. The forbidden set is imported from the runtime constant, never copied, so the guard tracks whatever the sandbox forbids. It ships with a positive control (each static import form the walker handles is seen, including the load-bearing from-parent alias-append) and a negative control (benign messagefoundry.* imports raise zero flags). Scope is the two files' DIRECT imports, deliberately not a transitive walk: importing the codec pulls asyncio/cryptography/store/transports/auth into sys.modules, so a transitive walker would red on clean shipped code and prove nothing. sandbox.py is out of scope per BACKLOG #346 even though the worker child imports it; the docstring records that residual for the owner. Falsified: planting `import socket` into the real _sandbox_codec.py reddens the live guard naming it; removing the walker's alias-append reddens only the alias-append positive-control case; an over-broad matcher reddens the negative control. All plants restored before commit.
… landed (BACKLOG #346) The #346 banner alone: OPEN -> SHIPPED, pointing at tests/test_sandbox_import_boundary.py (the static ast guard added in the preceding commit). The completeness wording is softened from "every forbidden import form is seen" to "each static import form the walker handles" -- a static walker cannot see dynamic importlib/__import__ forms, and CLAUDE.md section 11 prefers a bounded claim to an enumeration. Only the #346 banner line changed; the ranked table, the four census distribution lines, and every other item's banner are untouched. The census was NOT recomputed.
wshallwshall
enabled auto-merge (squash)
August 6, 2026 14:16
wshallwshall
added a commit
that referenced
this pull request
Aug 7, 2026
…251 #252 #257 #260 #273) (#274) * feat(asvs): prove an absence claim bites, not just that its pattern matches (BACKLOG #1006) `check_absences` admits an ASVS absence claim on `re.search(a.pattern, a.mutation)` -- one TOML field matched against another. That proves the mutation is well-formed; it never proves the mutation BITES. A reintroduction raised into a swallowing handler, written to a field nobody reads, or behind a flag nobody branches on satisfies every failure mode `check_absences` has and changes nothing observable. A green gate that is not evidence. Add an opt-in `--prove-absences` mode (`scripts/asvs/scorecard.py`) that executes the claim rather than grepping it: - Two optional `Absence` fields, `mutation_path` and `observable` (a pytest node id). When both are set the mode copies the tree to a scratch dir, runs the observable (baseline must be green), appends the mutation, and requires the observable to go RED -- and to fail as a test failure (exit 1). It fails closed on every other code: an already-red baseline, an uncollectable node, or a mutation that only breaks import is a PROVE-ERROR, never a proof. A claim that reddens nothing is UNPROVEN and fails the mode. - A coarse same-file static backstop screens claims carrying `mutation_path` but no `observable`: a `raise` landing in a file whose every handler swallows. It is a screen, not a proof (it cannot see a swallow in a caller), documented as such. - The whole pass runs in a TemporaryDirectory scratch copy, so it never mutates the tracked tree and never trips the committed-tree scan on itself. Both fields default empty and load without being refused: the vault's ~81 existing absence claims carry neither and must stay loadable (ADR 0156 §7). Absent means "not yet proven by execution", surfaced by the mode, never "proven vacuous". Review hardening carried in this change (the mode's own helpers): - `_scratch_ignore` refuses `.env*`, `*.db` (+ WAL sidecars) and `docs/security` when copying the tree. The vault runs this module against the REAL tree (ADR 0156 §7); a scratch copy carrying those would spill secrets / the local store / vault posture data into a world-default temp dir, which CLAUDE.md §9 forbids. The public-repo path never sees them; this is defence for the eventual vault run. - `_is_within_tree` refuses a `mutation_path` that is absolute or contains `..` before anything is applied, so an authored path cannot escape the scratch copy. Tests (tests/test_asvs_scorecard.py): eight fixture tests drive `prove_absences` directly (proved / UNPROVEN / already-red baseline / collection-error / two static backstop arms including a re-raise reach control / root-untouched / load round-trip), plus three that drive the CLI contract CI depends on -- `main([..., "--prove-absences"])` exit 0 on a biting fixture and 1 on a non-biting one, and `main([...])` without `--corpus` exit 2 -- plus the secrets-exclusion and path-traversal guards. Every new test was falsified (broken on purpose, watched red, restored). MessageFoundry is a not-deployed beta: the mode is opt-in, the default `verify` path is byte-unchanged, and no authored claim carries an `observable` yet, so nothing new is blocked by this alone today. Wiring the mode over the vault claims and backfilling their observables is the owner's follow-up. * docs(backlog): flip #1006 to shipped -- the absence gate can now prove behaviour (BACKLOG #1006) Flip the #1006 banner from filed to shipped. It is written as a capability claim, not a closure claim: the `--prove-absences` mode CAN catch a well-formed-but-vacuous reintroduction once a claim carries an `observable`, but the default `verify` path is byte-unchanged and no authored claim carries one yet, so nothing new is blocked by this alone today -- the honest present-tense state for a not-deployed beta. Banner lines of #1006 ONLY. The ranked table, the four census distribution lines, and every other item's banner are untouched. The status census was NOT recomputed. * test(connscale): dynamic contiguous inbound-port allocation, drop the flaky marker (BACKLOG #1014) The connscale SQLite smoke test hard-coded base_port=41000 and needs 24 contiguous inbound ports, so two worktrees running the suite at once contended for the same fixed block; a @pytest.mark.flaky(reruns=2) marker retried past the collision, relabelling a determinate resource conflict as CI noise. On the first parallel run it would keep masking exactly this class. Replace the fixed block with _free_contiguous_ports(), which anchors an n-wide block at a RANDOM base inside a bounded window, probes each port with a no-REUSEADDR bind, and returns the range only when all n bind. The random anchor over a wide window de-correlates concurrent worktrees; a genuine future collision now surfaces as a red, not a masked retry. Contiguity is asserted at the acquisition site and the allocator fails loudly -- never a silent fixed fallback -- via two branches: an up-front width guard when the block cannot fit the window, and a post-loop raise when no free block is found after `tries` attempts. The window is [20000,30000): the lower bound sits ABOVE the sibling MLLP fixed-port band (other tests bind fixed inbound ports in the 11xxx-19xxx range, e.g. 15099/19601), and the upper bound stays BELOW the OS ephemeral floors (Linux 32768+, Windows/macOS 49152+) so a kernel-assigned ephemeral port -- the sink/API ports, or any unrelated connection -- can never land in the block after it is probed. Drop the @pytest.mark.flaky marker: the collision was the cause, so keeping it would re-hide the class this removes. Add three helper tests -- contiguity and in-window, post-loop exhaustion (tries=0), and the width guard -- each pinned to its branch (match=) and falsified by mutation. Test-only change; no product code is touched. * backlog: close #1014 -- dynamic connscale port allocation ships, flaky marker dropped Flip #1014's status banner from open (filed) to shipped: the dynamic contiguous inbound-port allocation and the flaky-marker removal land in the same branch (commit 3450c3f). This edits the #1014 banner line ONLY. The ranked table and the four census distribution lines are untouched, and the census was NOT recomputed. * feat(anon): structural PHI-shape detectors + coverage report + token-floor signal on the leak-check (BACKLOG #331) The fail-closed leak-check verified only that MAPPED fields were pseudonymized; PHI sitting in a field no rule mapped would pass the check clean on first deployment (a real MRN is not a denylisted string). Scoped to the fields anonymize did NOT rewrite, add: - high-precision structural detectors over unmapped fields: dashed SSN, punctuated NANP phone, and CX MR/MRN-typed identifier. Deliberately narrow, to avoid the mass false-positives a broad digit-run search produces on HL7 bodies dense with dates/order-numbers/set-ids (ADR 0030 section 5). - LeakReport: an unmapped-field coverage report (addresses only, never a value) plus token_tables_live / token_floor_reason. Reasons name the shape + field ADDRESS only, so a raised LeakError / log line never carries PHI. - token_floor_failure() folded into the fail-closed decision under the require_live_denylist opt-IN lever (default off, so token-less CI/OSS/fork runs stay green with the structural detectors as the live backstop). The whole structural block is mirrored byte-identical into tee/anon/leak.py; a new engine/tee leak_report parity test pins it. Each detector was falsified (removed it, watched the unmapped-PHI dataset slip through, restored); a false-positive guard proves a benign unmapped field (14-digit EVN timestamp, order number, coded observation id) does not trip. Docs are written to the shipped DEFAULT behaviour, not an overclaim: the coverage report and token-floor reason are RECORDED and surfaced on a refusal or via the on_report hook (not an unconditional clean-path catch-all), and the strict refusal is opt-IN. anon/__init__.py (both copies) necessarily changed to export LeakReport/leak_report/coverage_clause and wire the lever. ADR 0030 section 5 / section 7 / Consequences amended (the "deferred" phrasing was stale against the shipped code). NOT-DEPLOYED beta: worded as "would let unmapped PHI through on first deployment", no present-tense exposure claim. * docs(backlog): flip #331 banner to SHIPPED, worded to the default behaviour (BACKLOG #331) Banner-only flip of #331 to SHIPPED. Worded to the shipped DEFAULT behaviour, not an overclaim: the coverage report and token_floor_reason are RECORDED and surfaced on a refusal or via the on_report hook (not an unconditional clean-path catch-all), and require_live_denylist is the strict opt-IN lever (default off). Census NOT recomputed: only the #331 banner line changed. The ranked table, the four census distribution lines, and every other item's banner are untouched. * test(sandbox): a static ast guard pins the codec+worker import boundary (BACKLOG #346) The sandbox's import boundary (DEFAULT_FORBIDDEN_MODULES in pipeline/sandbox.py -- socket/ssl/asyncio, the I/O- and secret-bearing messagefoundry.* subpackages, cryptography) is enforced only at RUNTIME and only inside the off-by-default [sandbox].mode=subprocess child. Nothing statically pins that the two modules which run inside that boundary -- _sandbox_codec.py and _sandbox_worker.py -- do not themselves import a forbidden module. Both are clean today; a future edit reintroducing a forbidden import would make mode=subprocess DOA on first deployment while the default-mode suite stayed green -- the failure inverts, hitting the most security-conscious installs hardest and quietest. This is defence-in-depth test coverage, not a code change: neither sandbox.py nor the codec is touched. tests/test_sandbox_import_boundary.py walks the two files' own ast import nodes (ast.Import/ast.ImportFrom, including nested/function-level and relative imports resolved to absolute) and asserts none resolves under a DEFAULT_FORBIDDEN_MODULES prefix. The forbidden set is imported from the runtime constant, never copied, so the guard tracks whatever the sandbox forbids. It ships with a positive control (each static import form the walker handles is seen, including the load-bearing from-parent alias-append) and a negative control (benign messagefoundry.* imports raise zero flags). Scope is the two files' DIRECT imports, deliberately not a transitive walk: importing the codec pulls asyncio/cryptography/store/transports/auth into sys.modules, so a transitive walker would red on clean shipped code and prove nothing. sandbox.py is out of scope per BACKLOG #346 even though the worker child imports it; the docstring records that residual for the owner. Falsified: planting `import socket` into the real _sandbox_codec.py reddens the live guard naming it; removing the walker's alias-append reddens only the alias-append positive-control case; an over-broad matcher reddens the negative control. All plants restored before commit. * backlog: flip #346 to SHIPPED -- the static ast import-boundary guard landed (BACKLOG #346) The #346 banner alone: OPEN -> SHIPPED, pointing at tests/test_sandbox_import_boundary.py (the static ast guard added in the preceding commit). The completeness wording is softened from "every forbidden import form is seen" to "each static import form the walker handles" -- a static walker cannot see dynamic importlib/__import__ forms, and CLAUDE.md section 11 prefers a bounded claim to an enumeration. Only the #346 banner line changed; the ranked table, the four census distribution lines, and every other item's banner are untouched. The census was NOT recomputed. * fix(tls): route four insecure-TLS escape cells through the ADR-0092 clamp (BACKLOG #329) LDAPS (auth/ldap.py), SFTP host-key (transports/remotefile.py), the webhook sink (pipeline/alert_sinks.py) and the AI-broker (transports/ai_broker.py) read the raw MEFOR_ALLOW_INSECURE_TLS escape directly; on an enforcing-PHI instance each would otherwise honour the env var on first deployment. Each now routes through the ADR-0092 weakened_tls_escape helper: SFTP is built in-gate so it uses _here(); the other three are built outside the hop scope, so the instance posture is threaded explicitly through AuthService / notifier_from_settings / ai_broker_from_settings (additive, default None = byte-identical for existing callers). The fifth cell the item names (direct.py) was already clamped in #323, so this converts the remaining four. Docs (CONNECTIONS/DEPLOYMENT/ PHI) corrected from 'not clamped'/'unclamped' to clamped. * docs(backlog): flip #329 to shipped -- four insecure-TLS cells clamped (BACKLOG #329) Banner line only (leaves the 2026-08-03 amendment note); census not recomputed. * fix(dev): anchor setup-leak-gate.ps1 to its own checkout, not the cwd (BACKLOG #1063) `$repo` came from `git rev-parse --show-toplevel`, which resolves against the CURRENT directory rather than the path the script was handed. Invoked by absolute `-File` path from another worktree -- the ordinary shape on a clone carrying dozens of them -- it armed the CALLER's checkout and printed CONFIGURED about that one, while the checkout the operator named kept no token source and went on failing closed. An absolute `-File` invocation is naming the checkout to act on; it must not then consult a different one. Now `Split-Path -Parent (Split-Path -Parent $PSScriptRoot)`, the form postgres.ps1:37 and sqlserver.ps1:56 in the same directory already use, plus an assert that the derived root actually carries scripts/security/ -- a wrong root should say so where it is derived rather than surface later as a confusing scanner failure. Tested by the DIVERGENCE, which is the only shape that can fail: two temp checkouts that both carry scripts/security/, the script invoked by absolute path while the shell stands in the other one. A test run from inside the target passes with the bug still in, because cwd and script root are then the same directory. Reverted to the old line, the same test reports "the named checkout was not armed" -- the negative control was run, not assumed. The fixture copies only the three files the script reaches for, never the whole of scripts/security/: a maintainer running this suite has the real token list sitting in that directory, and a copytree would sweep it into a temp dir. Also corrects this item's own prose. It called alloc.ps1:51 "byte-equivalent"; it is not -- alloc.ps1 carries --path-format=absolute and this script does not. The defect is identical, the bytes are not, and "byte-equivalent" is the kind of claim a later reader greps for and then trusts. * fix(coord): anchor alloc.ps1 and claim.ps1 to their own checkout (BACKLOG #1060) Both took `$repo` from an unanchored `git rev-parse --show-toplevel`, which resolves against the CURRENT directory rather than the path the script was handed. Invoked by absolute `-File` path from worktree A while intending to commit from worktree B, the claim was recorded to A; the ledger gate then refused B's commit -- correctly, it fails closed -- but far from the cause and with a message about the wrong thing, and it cost a number. `git -C $PSScriptRoot`, not `Split-Path`. The recorded `worktree` value has THREE readers: ledger_check.py:227, this script's own `-List`, and prune-merged.ps1's orphan-claim release, whose comment at :787 names the producing command -- "records `worktree = $repo` from `git rev-parse --path-format=absolute`" -- and matches on the full normalised path because a false positive there hands a live session's key to someone else. All three fold separators, so `Split-Path` would not have broken anything; it would have silently falsified that comment, in a destructive tool, for no gain. THE FILING NAMED ONE OF FOUR CWD-DERIVED READS, and the other three are measured in the negative control below: * the `branch` recorded with the claim was the CALLER's branch; * the floor's boundary was parsed from the CALLER's scripts/hooks/ledger_check.py; * the floor's WORKING-TREE term read the CALLER's docs/BACKLOG.md. The third is not friction and the item's severity paragraph is corrected in the same commit. That term exists to catch a number written but committed NOWHERE. Reading the caller's tree makes a number drafted in the target worktree invisible, so the allocator hands it out as free and two items share it -- both owned by that worktree, so owns() passes and the ledger gate never fires. The silent collision the docstring says this script exists to prevent, reached through the script. Narrow, since anything committed on any ref is still caught by the all-refs term, but a correctness hole rather than friction. claim.ps1:54 carried the same construct and was never filed -- found by inspection here, fixed in the same commit. Its enforcing hook, claim_check.py, reads the repo from cwd and is RIGHT to: a commit hook's cwd IS the committing worktree. Hook right, tool wrong, and only the tool can be invoked from somewhere else. Both scripts now print a NOTE when the shell is standing somewhere else. Anchoring is correct but surprising, and the item's other half -- showing the recorded worktree -- was already built (`claimed by:` / `by :`); what was missing is saying so when it diverges, instead of leaving it to surface as a refused commit later. Silent on the ordinary same-tree invocation, so it stays worth reading. THE FIX TURNED TWO SANDBOXED TEST FILES INTO WRITERS ON THE LIVE REGISTRY, which is worse than the red suite it also caused, and is the reason those fixtures changed here. test_coord_claim_{refresh,liveness}.py ran the REAL scripts/coord/claim.ps1 with cwd set to a temp repo -- scoped to a throwaway registry purely by ambient cwd, and one of them said so ("it scopes itself to the cwd's repo"). Once the script stopped consulting cwd, the passing half of the run wrote real claims into this clone's shared registry: two strays, `k` and a date-shaped key, were created and removed by hand. Both fixtures now stage and COMMIT a copy of the script inside the temp repo, so the sandbox is structural rather than ambient, and a linked worktree of the fixture carries its own copy -- which is how the peer-holds-the-key tests still produce a claim recorded against the peer. test_ledger_check.py already did exactly this for alloc.ps1, which is why it was the one that did not break. Tested by the DIVERGENCE, with -ShowFloor so no numbers are burned: allocation is a one-way door and a test that allocated would leave permanent holes in the shared registry for every worktree of this clone. Two temp checkouts draft different numbers and carry different PUBLIC_BACKLOG_FLOOR stubs; the caller's number is deliberately HIGHER, because the floor is a maximum and an equal or lower one would pass with the bug in. Reverted to the old lines, the same test reports floor 7777, boundary 1900 and a watermark under Caller/.git -- three independent signals, all pointing at the wrong tree. * docs(backlog): record the test-isolation trap #1060's fix walked into (BACKLOG #1060) A cwd-dependence that reads as a defect in the tool can be load-bearing ISOLATION in its tests. Both claim test files were scoped to a throwaway registry purely by ambient cwd -- one said so in a docstring -- so anchoring the script turned the passing half of the run into a writer on this clone's shared registry before the rest of it went red. Recorded in the item rather than only in the commit message, because #1057 and #1059 are the remaining instances of the same class and will hit the same trap: check what a test is isolated BY before changing what the code reads. * docs(CONNECTIONS): repoint the serial/ASTM decline at the archive; #27 is closed The connector-parity row for Serial (RS-232) / ASTM E1381/E1394/E1318 cited the decline as ([BACKLOG.md](BACKLOG.md) #27). Item 27 is closed and lives at docs/archive/backlog/BACKLOG-CLOSED.md:994; it is not in the live ledger. The pointer sent a reader to the wrong file. This is the second half of a designated two-marker pair. The archived item's own banner names both markers -- "marker landed in PR #411 (CLAUDE.md section 12 + docs/CONNECTIONS.md Serial row)" -- and commit 8a14602 repointed the CLAUDE.md half while logging this one as still carrying the decline, because that commit was scoped to section 12. The two halves disagreed about where #27 lives until now. Form: an anchored link, matching the sibling convention already used in this same directory for this same target (docs/AOAG-DEPLOYMENT.md:389 and :476). The cell already opens with "declined-by-design (v0.2+)", so the citation's only job is to resolve; restating "closed" in the cell would duplicate a fact the cell asserts two clauses earlier. Relative path: (archive/backlog/BACKLOG-CLOSED.md), NOT (docs/archive/...). The link is repo-relative from inside docs/. CLAUDE.md is at the repo root and correctly uses the docs/-prefixed form; copying that form here would resolve to docs/docs/archive/... and 404. Verified, not assumed: - The anchor slug was derived by a rule first replayed against three anchors already committed in the repo (#100, #101, #52) -- 3 of 3 exact -- then applied to #27's heading, then confirmed to match exactly one real "## " heading in the target file. A bogus anchor was run through the same check and found nothing, so the check can report a miss. - Item locations come from parse_items imported from scripts/docs/backlog_status_check.py, per CLAUDE.md section 11 -- not a hand-rolled scan of the banner alphabet. - backlog_status_check.py still reports 363 items, unchanged. - Read from origin/main throughout; the primary checkout runs behind. NOT changed, deliberately, with the reason: - docs/BACKLOG.md:574 -- bare number inside the section headed "Value & priority analysis (recorded 2026-06-19) - superseded". A superseded snapshot is a historical record; it has no path to rot. - docs/testing/FEATURE-COVERAGE-PLAN.md:41 -- names the features in prose and carries no number or path at all. Nothing to rot; adding a pointer would be new scope, not a repair. - docs/testing/master-test-plan/00-strategy-and-governance.md:699 -- bare #26/#27 that resolve to nothing rather than to wrong content. Repairing one link here would leave a single correct relative link among 28 broken root-relative ones in the same file; it belongs in the doc-set-wide sweep that class needs. - docs/BACKLOG.md:906 -- a real defect, but larger than a pointer repair and in a file several sessions are editing. Reported separately for a decision. A wider scan (127 path-bearing BACKLOG citations) found roughly 90 more naming the live ledger for an archived item. Not touched here: the staleness is currently uniform, and repointing a subset would assert by contrast that the untouched siblings are live. That class needs one pass, not a trickle.
Collaborator
Author
|
Superseded by merge train #274, which merged as Your commits went to Verified before closing, by the instrument that actually answers the question. #274 was squash-merged, so The question is "is this PR's own change present in Ran for all seven train members; all seven came back already-applied. Nothing was lost. Reopen if you disagree -- but please check the reverse-apply result first rather than |
auto-merge was automatically disabled
August 7, 2026 20:13
Pull request was closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes BACKLOG #346. Test-only.
A static AST guard pins the sandbox codec plus worker import boundary, so a future import that
crosses it fails at test time rather than at runtime in a sandboxed worker.
VERIFIED 3/3: the live guard, a POSITIVE control, and a NEGATIVE control. The negative control is the
part that matters -- it proves the guard can actually fail, rather than passing because it inspects
nothing. Commit messages glyph-free; banner gate OK.