DRAFT triage: builder-2's dead lane -- six commits that never landed, behind 33 of squash residue - #530
Draft
wshallwshall wants to merge 39 commits into
Draft
DRAFT triage: builder-2's dead lane -- six commits that never landed, behind 33 of squash residue#530wshallwshall wants to merge 39 commits into
wshallwshall wants to merge 39 commits into
Conversation
…epoint module stem (BACKLOG #1130) Two untrusted strings reached a filesystem path without clearing the rule their siblings already clear. Both are closed with the shape each site's neighbours already use, rather than a new one. config/codeset_edit.py: rename_code_set validated only `new`. `old` is equally untrusted and reaches `_existing_path`, so a traversal source resolved outside codesets/ and `os.replace` would MOVE that file in, contents intact. The `_validate_name` call is added before the lookup, matching show (:92), upsert (:144) and remove (:221). Kept after the empty-argument checks so the `--name is required` wording still wins for an empty name. corepoint_import.py: the export chooses `module_name`, which becomes BOTH the emitted inbound() connection name and the module's filename stem. It is folded with `_sanitize`, as the channel name (:415) and handler names (:464) already are. Mutation rather than refusal, because the importer writes into a directory it created -- there is no existing file to alias -- and a colliding fold is already de-duplicated by the writer's `assigned` set. THIS MOVES NO ASVS SCORE. The cell's recorded absence claim is keyed on `_within_root`, a symbol belonging to transports/file.py, while the guard that shipped is `_is_contained_name`; that re-key is record work in the vault and nothing here satisfies it. SCOPE, STATED RATHER THAN ASSERTED AS COMPLETE. The item's own body measured 162 file-path-construction sites across 49 files under messagefoundry against roughly fifteen named, and forbids a residual sentence saying "every" or "all limbs" until that census exists. This commit covers the two named sites only. harness/, scripts/ and the ide/ TypeScript surface are untouched and are contingent on an open scope ruling about which roots the assessment covers. Tests, each asserting on the refusal and not merely on an unchanged tree: tests/test_corepoint_import.py::test_a_hostile_inbound_name_cannot_write_outside_the_output_directory tests/test_codeset_edit.py::test_rename_rejects_traversal_source_without_moving_a_file_into_the_dir tests/test_codeset_edit.py::test_cli_rename_traversal_source_is_a_clean_json_error
… a false mirror claim (BACKLOG #1130) Follow-up to e5505a9, from its own adversarial review. Two things that commit changed and did not say. 1. `rename --name` was missing from the untrusted-name list. It joined that list in e5505a9 and the doc still named only `upsert`'s name, `rename --to`, and show/remove. The list now matches the code. 2. THE BEHAVIOUR CHANGE NOBODY HAD WRITTEN DOWN. A code set stored as `lab.results.csv` LOADS -- measured with the real loader, `load_code_sets` returns the key `lab.results` and `codeset list` shows it -- but its dotted stem fails the no-extension name rule. `show` and `remove` already refused it. Adding the guard to `rename --name` closed the last verb that could manage such a file, so it now has to be renamed on disk by hand. That is a real cost of the fix and it belongs beside the fix, not in a review transcript. 3. The heading "Validation rules (mirror the loader exactly)" was FALSE for the name rules and true for the content rules. Split, rather than deleted, so the half that is accurate keeps its claim. Found by the adversarial pass, not by me: it planted each defect separately, confirmed both new tests red for the named reason, reverted, and proved the revert byte-identical by SHA256 on both files. Its verdict was `sound` with `test_can_fail=True`; these are from its residual findings, which is the half a green verdict hides. NOT DONE, and reported rather than silently left: two of its findings are about test STRENGTH, not correctness -- the corepoint census is blind to escape classes its docstring claims, and the codeset tests pin one of the five rules `_validate_name` enforces, so a deliberately weaker guard would still pass them. Those need test work, not doc work, and they are not in this commit. No engine code changed. No ASVS score moves.
…low-list the URL scheme (BACKLOG #1107)
Two of the four clauses the item names. The owner dispatched these two only; the
FHIR structured-parameter and web-console URL-builder clauses are separate
undispatched subjects and are untouched.
CLAUSE 1 -- path segments. Seventeen f-string sites interpolated an identifier
straight into a URL path and the file encoded nothing: `quote(|urlencode(` in
apiclient/client.py returned 0, against a control of 7 in transports/fhir.py and
76 `def ` in the client itself, so the zero was a property of the code. `_seg()`
wraps `quote(..., safe="")`; the default `safe="/"` would leave untouched the
one character a path segment turns on.
ENCODED AT ALL SEVENTEEN, not only the four carrying a connection name. Thirteen
carry engine-minted ids, so "this is not untrusted data" is true of them today --
and that is the argument the item names as its live trap, because it rests URL
correctness on a data-grammar invariant no line of code asserts and no test pins.
CLAUSE 2 -- scheme. `_assert_safe_transport` returned early on
`host in _LOOPBACK_HOSTS or host == ""`. `javascript:`, `data:` and `file:` carry
no hostname, so all three passed: a host-keyed deny-list for plaintext http, not
a scheme allow-list, and the two schemes the ASVS verb names were exactly the two
that got through. Now a positive allow-list, strictly narrower than the check it
precedes, leaving the plaintext-http logic byte-for-byte intact.
BEHAVIOUR CHANGE: a schemeless `base_url` is no longer accepted. Seven non-test
construction sites exist under messagefoundry/ and harness/, none passing a
schemeless literal -- derived with `grep -rn "EngineClient("` at this tip, not
carried from a report. WHAT THAT DOES NOT PROVE: every one of those takes a URL
from config or argv, so runtime values are not covered by a static count.
THREE RESIDUALS FROM THE ADVERSARIAL PASS, kept because a green verdict hides
exactly this half:
- The completeness guard's docstring OVERCLAIMS. It says a new endpoint that
interpolates an identifier reds the test; that is true for f-strings and
FALSE for string concatenation and .format(). The reviewer demonstrated it
by adding a concat site that slipped through.
- The guard asserts a COUNT, not a correspondence, so a same-size swap (one
site removed, one added) passes.
- The scheme allow-list's docstring invokes an OS protocol handler. This client
hands base_url to httpx, which never launches one -- httpx raises
UnsupportedProtocol. The defence is real but narrower than the docstring.
THIS MOVES NO ASVS SCORE. The cell is graded in the vault, which this worktree
does not contain, and two of its four clauses remain unbuilt.
docs/SECURITY.md is UNTOUCHED -- another lane owns it this round. The clause-2
delta is reported to the Dispatcher and should land WITH this code, not ahead.
Adversarial pass: 5 plants, each red for its documented reason, client.py sha256
identical after every revert. Verdict sound, test_can_fail true. Module: 56 passed.
…st the separator (BACKLOG #1130) Closes a test-strength gap its own adversarial pass found and e5505a9 did not fix. The traversal test matches only "must not contain a path separator", so a deliberately weaker guard -- `if "/" in old or "\\" in old: raise` -- satisfies it while leaving the other branches unguarded on the rename SOURCE. `_validate_name` has SEVEN refusal branches, not the five the review reported: non-empty, control characters, path separator, "..", absolute-or-drive, bare stem, and escapes-the-directory. The new parametrize covers eight inputs. PROVEN TO FAIL FOR THE STATED REASON, not merely written and watched go green. Planted the narrowed guard at the call site and got exactly the predicted split: RED dotdot, embedded-dotdot, suffix, control, blank PASS slash, backslash, absolute -- the three a separator-only guard catches That asymmetry is the point. A uniform failure would not distinguish "this test covers the branch" from "this test fails on anything". Revert verified byte-identical, sha256 8d4a1672...52fd01cc before the plant and after it. The mutation is named in the docstring so the next reader can re-run it instead of trusting this message. STILL NOT DONE, from the same review: the corepoint census is blind to escape classes its own docstring claims to cover, because it censuses with rglob over the output root, so a name resolving INSIDE the root is visible to it while other classes are not. Separate test, not in this commit. No engine code changed. No ASVS score moves.
…rd the real red (BACKLOG #1130) Closes the second test-strength gap its adversarial pass found. The existing census rglobs from `root`, and the helper nests the out dir three levels under it so `../../../x` still lands inside. THAT COVERS EXACTLY ONE ESCAPE CLASS. A deeper traversal, a POSIX-absolute path, a drive-letter path and a UNC path all resolve ABOVE `root`, outside that rglob -- where the escape is invisible and the empty result reads as containment. The helper now takes a wider census base, and the new test passes `tmp_path`: the widest base it can see, containing every target those classes reach in-process. THE MUTATION'S RED IS NOT THE ONE I FIRST PREDICTED, and the docstring now says so rather than being quietly corrected. I wrote "red: `outside` is non-empty". Measured: five of six ids go red with FileNotFoundError on the write, because the unsanitized stem names a parent directory the importer never created. That still proves the name reached the path unsanitized -- but it is a WRITE FAILURE, not an escape the census caught. A reader re-running the mutation and expecting a non-empty census would see an unrelated-looking error and distrust the test. A refusal for a different reason is indistinguishable from the refusal you asked for, so the docstring names the observed mechanism. `bare-dotdot` PASSES under the plant and is kept as a must-not-trip control: `..` alone resolves to the out dir's parent as a directory, producing no write for the census to catch. If it ever starts failing, the fold changed shape. Revert verified byte-identical, sha256 ff13e9a63fdb736f6860e4216b0dd9d4b1f27b0465c22f6ec3d60d68f0d0925f before the plant and after it. No engine code changed. No ASVS score moves. Both #1130 test-strength findings from that review are now closed.
…d route the token hop through the sidecar (BACKLOG #1176) TWO LIMBS, both product defects. NEITHER MOVES THE ASVS SCORE and the cell stays `fail` -- ECH is unreachable on this runtime (zero ECH attributes on the pinned interpreter, measured here), so nothing in this commit makes a pass possible. That is the item's own conclusion and not mine to change. LIMB 1 -- silent acceptance. `ech_egress` was refused only by connectors routing through `egress_route_from_settings` (fhir, soap, dicomweb, rest). MLLP, TCP, X12, file, email and DICOM never reach it, so they BUILT and silently ignored the key: an operator sets an SNI-hiding flag and nothing hides anything. The refusal is hoisted into the shared construction seam rather than copied per connector, so a connector added later inherits it. LIMB 2 -- the seam that leaked even where ECH works. On REST the proxy was forced to None whenever a sidecar was set, and http_auth fell back to the plain opener, so `ech_egress` plus an OAuth2 or SMART token endpoint sent the AUTHORIZATION SERVER's hostname in a cleartext outer ClientHello. A refusal keyed on "does this destination implement the send path" cannot reach that, because REST does implement it. RED PROOF, tests written first against unmodified HEAD 4633a29: `DID NOT RAISE ValueError` six times, one per connector, plus the inbound limb, plus the OAuth2 and SMART hops asserting the direct URL. DID NOT RAISE is the evidence the defect was real -- those connectors built and accepted the key. ADVERSARIAL PASS: sound, test_can_fail true. Two plants, each red for its documented reason, five files sha256-identical after revert. FINDINGS FROM THAT PASS, KEPT BECAUSE A GREEN VERDICT HIDES THEM: - test_both_refusal_sites_carry_the_same_message DOES NOT witness the seam its variable names claim. It PASSED under the plant that deleted the hoisted refusal, so it asserts something true but not that. NOT FIXED HERE. - `ech_sidecar` set WITHOUT `ech_egress` is still silently ignored on every connector, REST included -- a residual inside the item's own hypothesis. - Routing the token POST through the sidecar makes the AUTH hop inherit the sidecar contract's fail-closed rule. A real semantic change, undisclosed by the implementer and recorded here instead. - Three provider docstrings assert `proxy` and `ech_sidecar` are "mutually exclusive by construction"; nothing in those functions enforces it. docs/SECURITY.md is UNTOUCHED -- another lane owns it. Two edits reported as text, both inside the single ECH bullet. Module: 38 passed. Transports seam: 2787 passed, 141 skipped, 0 failed.
…ation (BACKLOG #1112)
The in-process half already shipped: uploads.py holds an asyncio.Lock spanning
the quota check and the write. That lock is per-EVENT-LOOP, so N engine shards
over one uploads_dir hold N of them and each can pass the same check. Every
shard shares ONE unified store (ADR 0063, and sharding.py:81 makes a server DB
mandatory past one shard), so the store is the only decision point that spans
them.
ONE new protocol method, `reserve_upload_quota`, across the four store files,
plus the two construction sites that give UploadStore a store handle. The
attachment substrate is NOT touched -- separate subject, out of scope by the
dispatcher's fence, and that fence held: no fifth store file, no second method.
RED PROOF, and the first run is DISCARDED rather than counted. Run 1 failed with
`TypeError: unexpected keyword argument 'store'` -- the code path was never
reached, so it is plumbing, scored NOT REACHED. Run 2, with the kwarg removed
from the helper so the identical body runs against shipped code, is the proof:
both shards WON a budget of one and both files landed on disk. `assert 2 == 1`,
failing at the right phase for the right reason.
A refusal for a different reason is indistinguishable from the refusal you asked
for, which is why run 1 is in the record as not-proof rather than omitted.
MYPY IS THE CONTROL THAT THE PROTOCOL IS REAL: renaming the SQL Server
implementation produces `"SqlServerStore" is missing following "Store" protocol
member: reserve_upload_quota`. So the addition is enforced across all four
files rather than silently unimplemented in three.
ADVERSARIAL PASS: test_can_fail true; four independent plants, each redding a
distinct test; 30/30 acceptance runs, no flakes. It confirmed the test
demonstrates CONCURRENT EXCLUSION rather than the shared visibility the
pre-existing sequential test already showed.
ITS FINDINGS, RECORDED BECAUSE THEY ARE NOT FIXED HERE:
- THE IMPLEMENTER'S OWN STATED MITIGATION IS FALSE. "The CI Postgres and SQL
Server legs are the gate" -- zero tests call reserve_upload_quota on either
backend; all four callers are SQLite. The two server implementations ship
UNEXERCISED.
- A leaked reservation does NOT unconditionally self-heal: release writes
`since = <now>` in all three backends, restarting the staleness clock. Two
shipped docstrings state the opposite. That is a false premise in a comment
and it is the next thing I fix.
- It introduces a new failure mode on SINGLE-PROCESS deployments, not only
sharded ones: before this, a killed process left no quota state.
config/settings.py carries a COMMENT-ONLY hunk (0 non-comment changed lines,
asserted mechanically) correcting a comment that #1112 makes false. That file is
another lane's; the dispatcher authorised this hunk specifically because the
comment is true only at the moment this lands, and this commit is that moment.
89 passed across the upload, upload-API and PHI-inventory modules.
…it was mine (BACKLOG #1112) SDS-3.7 -- a compensating control must not rest on a false premise. Two shipped docstrings landed in 142b926 saying a leaked reservation is reclaimed once it goes stale. That is true ONLY while the uploader is otherwise idle, and neither said so. THE MECHANISM, verified in the SQL rather than taken from the review that found it. The release branch of reserve_upload_quota is: UPDATE upload_quota SET inflight_files = MAX(0, inflight_files + ?), inflight_bytes = MAX(0, inflight_bytes + ?), since = ? WHERE uploader_id = ? `since = ?` is UNCONDITIONAL, deliberately -- its own comment reads "Never conditional: refusing a release would strand the reservation it is paying back." That is correct for the release and it has a side effect nobody wrote down: every later release by the same uploader pushes the staleness clock forward. So the reserve path's staleness reset -- which IS conditional (`CASE WHEN since <= ? OR inflight_files <= 0`) -- never fires for an uploader who keeps uploading successfully. A leaked slot can survive indefinitely. WHY THIS IS WORSE THAN AN UNDOCUMENTED LIMIT: with no comment a reader measures. With "it self-heals" a reader RELIES, and the sentence is written in the confident register that earns reliance. The failure it hides is a permanently narrowed budget for exactly the uploaders who use the system most. Both sites now state the precondition, and the first keeps the retraction in place rather than quietly reading correctly -- the next reader should be able to see that the stronger claim was made and withdrawn. Found by #1112's adversarial pass, whose verdict was `weak` for this and two other reasons. I verified the mechanism in the statement before writing it here; the review named it, the SQL proved it. No code changed. No ASVS score moves. 33 passed across the two upload modules (tests/test_uploads_cross_process_quota.py + tests/test_uploads.py). The first version of this message said 56 -- a number carried forward from an unrelated earlier run, corrected by re-running rather than by memory.
…(BACKLOG #1183) _BEARER matched the AUTH SCHEME as its own value: "Authorization: Bearer <tok>" redacted the word "Bearer" and emitted <tok> verbatim. The one assertion covering it passed anyway, because its chosen token was pure alphanumeric and the unrelated _LONG_B64 sweep caught it -- so the suite could not tell a working pattern from a broken one. Every fixture token now carries a hyphen AND an underscore, which breaks the base64 run and puts each family on its own pattern. Widens the secret domain to what the module's docstring already implied: a bare "Bearer <tok>" with no header label (_AUTH_SCHEME), password/PWD/secret pairs (_CREDENTIAL_KV), and an inline DSN password (_DSN_PASSWORD). Quote handling is uniform -- the value class stops at any quote, so a mismatched pair loses the value and keeps only a stray character. Measured: 0 leaks across 7 quote shapes, including both mismatched orders. Two label words are deliberately EXCLUDED and commented as such: "key" and the basic/digest scheme words. All three are ordinary vocabulary in this codebase, so matching them would redact operator diagnostics and buy no confidentiality. resolve_env_settings no longer echoes a failed env() value. It appeared TWICE -- once from the f-string and once inside the cast's own ValueError text -- so dropping the f-string half alone would still have leaked it. The error now names the setting, the key and the expected TYPE, and says the value was withheld. Which patterns exist is no longer a docstring claim: the suite derives them by AST and fails if one is applied without a named family, or declared without being applied. .gitleaks.toml: nine EXACT-literal allowlist entries for the new synthetic needles, each traced to its fixture. Exact literals, never a family prefix -- "sk-live-.*" would have allowlisted a real leaked key of that shape. Verified both directions: the nine listed values are silenced, and four unlisted values in the same four shapes are still caught.
…1184, ASVS 14.2.1) An operator-typed patient identifier rode the URL into the engine's access log, the reverse proxy's log and browser history -- none of which the redactor reaches. `content` and `field_value` are deleted from FIVE GET signatures (three in api/app.py, plus both console GETs) and now travel in a POST body over a criteria model. `field_path` stays on GET: it is a structural locator, not a value, which is why _search_audit_detail already records it verbatim. Each POST is the needle-bearing sibling of its GET, not a second implementation: both call one shared handler, so the PHI-read hop and budget are charged in the same place for both. Verified the audit path carries no value either -- shape and length only, and the call sites pass metadata filters alone. A ROUTE-TABLE GUARD, which the item names and is the durable half. Deleting a parameter earns the verdict once; this keeps it. It walks both planes, resolves the WIRE name so a re-add hidden behind alias="content" is still caught, ignores POSTs, and carries a positive control so an inert walk cannot report a clean repo forever. Proven by planting a re-declared parameter on the search GET. DISCLOSED BEHAVIOUR CHANGE, not a defect: _enforce_admin_write_pacing is NON-GET only, so a search issued as POST now also draws on the admin-write bucket that gates connection start/stop/restart, DR activate/release and alert ack/resolve. Strictly stricter, never weaker. I did NOT measure the budget sizes and claim no reachability; whether the two deserve separate buckets is unanswered. FOUR REVIEW FINDINGS FIXED, each proven to fail by planting first: - the uploaded-log filter test claimed the form must POST and only checked that a field input existed. Flipping method back to "get" restored the exposure and the test stayed green. It now pins that form's own opening tag. - the field_path control asserted matched == 2, which is what an UNFILTERED browse returns, so it could not tell a working filter from a dropped one. PID-5 discriminates at 1. - the filter POST answered an out-of-bounds criterion with the whole file at HTTP 200, blank inputs and no banner, discarding valid criteria with the invalid one. It now refuses with 400 and keeps what was typed, like its sibling. - test_security_doc_rate_limits pinned four routes that charge PHI-read pacing at their own route. Three now charge inside the shared handler, so the guard follows the delegation one hop instead of shrinking the pinned set -- shrinking it would have stopped guarding three routes. docs/SECURITY.md counters are re-derived on a tree that already carries #1148: require_step_up is 27, which is neither the 24 that lane measured nor the 29 this one did. Both were right alone and wrong together. KNOWN AND UNATTRIBUTED: test_route_map_parser_detects_a_planted_permission_mutation fails in a FULL-suite run only. It passes alone, with its sibling suite, and after the scorecard suite. No test writes the real docs/SECURITY.md and both suites resolve _ROOT absolutely, so it is in-process state, not a stale file. Not claimed as mine and not claimed as pre-existing.
…ot a finding that ide/ is crypto-free (BACKLOG #1164) Two comments -- the gate's own and its pinned twin in tests/test_security_static.py -- justified excluding ide/ from the crypto walk by saying the exclusion is "a fact about the tree". It is a fact about the language. ide/ contains zero .py files, so a Python AST scanner has nothing to read there; that says nothing about whether the directory contains cryptography. It does. ide/src/cspNonce.ts imports randomBytes from node:crypto and draws CSPRNG bytes consumed across the extension, and ide/src/engineClient.ts pins a TLS floor it applies to every https request. Both are first-party crypto in a shipped artifact and neither is discoverable from a Python walker. Verified at this HEAD rather than taken from the item, which measured them at an older commit. The comments now state what a green here actually supports -- no undocumented crypto in the PYTHON of five roots -- and record that adding ide/ to WALK_ROOTS would be a no-op that looks like a fix: the scanner finds zero .py, reports clean, and the TypeScript stays invisible while the tree gains a green that is evidence of nothing. That is the disqualified move the item names. NOTHING ABOUT THE GATE'S BEHAVIOUR CHANGES. The stray-.py invariant is untouched and still enforced: planting a .py under ide/ prints the violation and exits 1; removing it returns exit 0. Measured without a pipe, because tail's exit code is not the gate's -- I read that wrong once tonight already. This does not move the cell. It retires a false premise underneath it, which #1164 names as work in its own right; the TypeScript and PowerShell discovery arms remain unbuilt and a pass is not reachable without them.
…d hash (BACKLOG #1171, ASVS 11.4.1)
client.connect() passed no algorithm restriction -- disabled_algorithms appeared
ZERO times tree-wide -- so the connector offered paramiko's whole preferred MAC
list, which carries hmac-md5, hmac-sha1 and their -96 truncations. Appendix C of
the V11 chapter marks HMAC-MD5 D (disallowed) and SHA-1 L ("not suitable for
HMAC"). A server that selected one got it, on a use case the requirement names by
name and in a clause with no default-off escape.
STATED AS AN ALLOW-LIST, WHICH IS THE POINT. paramiko's API takes a deny list, so
_disabled_sftp_macs subtracts _APPROVED_SFTP_MACS from whatever the installed
library offers. A deny list has to be edited whenever the library gains an
algorithm, and the failure mode of forgetting is that the new algorithm is
PROPOSED. Here forgetting excludes it instead -- wrong in the safe direction, by
construction. Passed on every connect rather than behind a setting: a control an
operator has to switch on is not a control.
THE TEST ASSERTS NON-EMPTINESS BEFORE IT ASSERTS SAFETY, and that ordering is the
substance. "No weak member survives" passes trivially against an EMPTY effective
set, which is exactly what a broken subtraction -- or a paramiko that renamed
_preferred_macs -- would produce. Both arms proven by planting: removing the
restriction reds naming all four weak members; disabling everything reds on the
positive control first. Restored byte-identical by SHA-256.
The fake gained a Transport rather than production gaining tolerance for its
absence. A missing attribute there yields an empty deny list, which silently
restores the weak proposals, so the strictness belongs in the code and the
completeness belongs in the fake. Caught because an EXISTING test broke: the
shared _FakeParamiko had no Transport, which only surfaced on the wider run.
SCOPE, STATED: this measures WHAT THE CONNECTOR PROPOSES, not the effective set
negotiated with the real library -- paramiko sits behind the [sftp] extra and the
suite uses the repo's fake. #1171 names that gap ("an instrument that measures each
third-party surface's EFFECTIVE algorithm set") and it stays unbuilt. Also still
open in that item and untouched here: the HTTP Digest handler defaulting to MD5,
the five SMTP login() sites preferring CRAM-MD5, and the XML signature accept-set.
This does not move the cell.
…hree releases do not have (BACKLOG #1193, ASVS 15.2.4) README told every reader "Every release is ... Sigstore-signed, and carries SLSA build-provenance + PEP 740 attestations" -- directly below the install command for messagefoundry-webconsole, which is one of the wheels that is not. MEASURED, with controls in both directions. release.yml has three publish jobs. Only `release` runs Sigstore signing, attest-build-provenance and the CycloneDX SBOM; `release-webconsole` and `release-harness` contain none of those steps while still showing five uses:/runs-on lines each, so the zero is a real absence and not a scan artifact. All three DO set `attestations: true`, which is the PyPI-side PEP 740 attestation -- a different artifact from a GitHub attestation. So the note was not wholly false, which is why it is now scoped per artifact rather than deleted or softened. A reader who installed the console wheel on the strength of it and ran `gh attestation verify` would find nothing, having been told the opposite. That is a compensating control resting on a false premise, in shipped text, which is the defect class this cell is about. THE GUARD IS DERIVED, NOT PROSE-PINNED. It parses release.yml, computes which publish jobs sign, and only then requires the README to be scoped. If signing is later added to the console job the claim becomes true and the test stops objecting on its own. Two positive controls run FIRST -- the parse found 3+ publish jobs, and the detector matched signing somewhere -- because a broken job-splitter yields an empty unsigned set and a test that passes while measuring nothing. Proven by planting the old unscoped sentence: it reds naming both unsigned jobs. NOT TOUCHED, AND FLAGGED RATHER THAN GUESSED: docs/SUPPLY-CHAIN.md:37-38 says pip verifies attestations automatically when installing from PyPI. #1193 records that PEP 740 disclaims this. I could not verify the PEP's text or pip's behaviour from this checkout, and correcting a supply-chain claim from memory is how a wrong sentence gets a confident tone. It needs someone who can check the spec. This does not move the cell. The loader still mounts the console on find_spec presence with no provenance check, and the bare-name install remains in shipped text -- both named in the item and both untouched here.
…four modules (BACKLOG #1216)
`shutil.which("bash")` is a fact about PATH, and on Windows PATH order decides
which OPERATING SYSTEM answers. C:\Windows\System32\bash.exe is the WSL launcher,
whose filesystem namespace is not the one the test just wrote a fixture into. So
`skipif(shutil.which("bash") is None)` asked whether A bash EXISTS, not whether the
one it found CAN DO THE JOB: the wrong interpreter is FOUND rather than absent, the
skip never fires, and every block fails for a reason unrelated to its content.
The working resolver already existed in test_merge_gate_controls.py and had since
2026-08-10 -- one day before the item was filed. Three other modules kept their own
which() guards, so the defect survived everywhere it had not been fixed. This is
promotion and adoption, not a new design: tests/_bash_resolver.py is now the single
source and test_merge_gate_controls.py delegates to it rather than keeping a fourth
copy. Two resolvers are free to disagree, and the one that disagrees is the one
still manufacturing failures.
ADOPTED IN FOUR MODULES: workflow_shell_syntax (skipif removed, resolver used),
dependabot_automerge_guardrails (2 bash-only skipifs dropped, 2 bash+jq narrowed to
jq -- jq is genuinely optional and that skip is legitimate and stays),
installed_coord_hooks (still PREFERS sh, but now proves whatever it finds can read
the probe), and merge_gate_controls (delegates).
TWO CORRECTIONS TO THE ITEM, both measured. It names ONE site in the dependabot
guardrails; there are FIVE. And the defect does NOT reproduce on this box --
which("bash") here resolves Git Bash, not the WSL launcher -- so no test added here
could be proven by watching failures disappear. Every test CONSTRUCTS the failing
condition instead. That is also why it survived a full day: on most boxes it is
invisible.
ADDED, AND IT IS THE ITEM'S OWN INSIGHT: a 100 PERCENT failure rate now raises
"instrument fault, not N content faults" rather than leaving a reader to notice the
ratio. The signature "154 shell blocks; 154 failed" printed on every run for a day
and was read as 154 real defects. Plus 127-vs-2 discrimination, so a harness fault
(command not found) can never be reported as a syntax error in the content.
tests/test_bash_resolver.py covers the properties the three modules were missing:
the probe rejects an interpreter that cannot read its own file, PATH is consulted
LAST, no usable bash RAISES rather than skipping, and a harness code is never
described as a syntax error. A positive control asserts the resolver succeeds here,
without which a resolver that rejected everything would satisfy every negative.
75 passed, 7 skipped (all jq, verified absent on this box). Exit code captured
BEFORE the pipe and the count asserted -- 'no tests ran' is not a failure message,
it is the absence of one.
#1272 is this item's duplicate per #1216's own text and is still OPEN in the
ledger. Closing it is not this seat's call.
…roken probe (BACKLOG #1216) test_a_real_bash_is_found_and_can_read_this_process_files asserted bash_sees(require_bash(tmp_path)). require_bash returns ONLY a candidate bash_sees has already approved, so that is true by construction -- an identity, incapable of failing, sitting in the test whose declared job is to be the positive control for every negative in the file. MEASURED, not reasoned. Mutating bash_sees to `return True` -- breaking the probe wide open -- reddened the two negative tests and left this one GREEN. It certified a resolver with no working namespace check, which is the exact defect #1216 exists to close, one level up. The second assertion is now INDEPENDENT of the selection predicate: it runs the resolved interpreter directly with a different token, a different filename and a shell builtin instead of cat. Proven with a mutation AIMED AT IT -- probe broken open AND a non-bash offered first, so require_bash returns something that cannot read the file. It now fails; before this change it did not. Restored byte-identical by SHA-256 both times; 5 passed after. FOUND BY APPLYING A PEER'S RULE TO MY OWN JUST-COMMITTED WORK: a control that reds under a mutation aimed at it cannot be an identity. Necessary and sufficient per ASSERTION, necessary only for a composite -- a live half masks a dead one, and here the live "no bash resolved" half masked the dead one beside it. Kept as its own commit rather than folded into 70186e4. The hole existing is the finding; amending would erase that a test written an hour earlier, with a docstring calling itself the positive control, could not fail.
…ed hash (BACKLOG #1171, ASVS 11.4.1)
digest_handler_from_settings returned a bare urllib HTTPDigestAuthHandler with no
algorithm restriction. urllib reads `chal.get('algorithm', 'MD5')` -- verified in
CPython's own source on the supported interpreter -- so the SERVER chooses, and an
endpoint that simply OMITS the parameter gets answered with MD5. Appendix C marks
MD5 D: disallowed for any cryptographic purpose, in a clause with no default-off
escape.
THE ABSENT CASE NEEDS NO HOSTILE SERVER. A plain RFC 2617 endpoint omits
`algorithm`, so the exposure was reachable through ordinary interoperability rather
than through an attack.
REFUSE RATHER THAN REMOVE. There is no way to dictate the algorithm to a peer, so
the honest options were refuse or delete the feature. Digest is a documented,
configurable, tested option, and refusing keeps it working for endpoints that offer
an approved hash while failing closed on the ones that do not. -sess variants strip
to the same hash and are treated as such; matching is case-insensitive.
LOUD, NOT `return None`. Returning None makes urllib skip the auth, and the request
surfaces as a bare 401 -- which an operator reads as bad credentials and goes
looking in the wrong place. It raises HttpAuthError, the same contract this seam
already uses for a refused cleartext hop.
BOTH ARMS PROVEN BY PLANTING. Removing the check: 3 failed. Emptying the approved
set so it refuses EVERYTHING: the positive control failed -- without that control,
deleting the feature outright would have satisfied every refusal case perfectly and
the suite would have reported a working gate over a connector that could no longer
authenticate at all. Restored byte-identical by SHA-256 both times.
SCOPE: this is one of #1171's four limbs. The SFTP MAC allow-list shipped at
f385a6d. The SMTP limb is measured and deliberately UNBUILT -- the item's own
prescription would replace a disallowed-hash challenge-response with a cleartext
password on an unencrypted channel, which needs a ruling I have asked for. The XML
signature accept-set is untouched. This does not move the cell.
…ed channel (BACKLOG #1171, ASVS 11.4.1) smtplib's preferred_auths is ['CRAM-MD5', 'PLAIN', 'LOGIN'] -- CRAM-MD5 FIRST, verified on the supported interpreter with auth_cram_md5 present -- so every unrestricted smtp.login() offered an HMAC over MD5 before anything else. Appendix C marks MD5 D: disallowed for any cryptographic purpose, in a clause with no default-off escape. BOTH HALVES OR NEITHER, PER THE DISPATCHER'S RULING, and the reason is the whole finding. PLAIN and LOGIN SEND THE PASSWORD; CRAM-MD5 does not. Restricting the mechanism alone would have closed a conformance gap by putting a cleartext password on the wire wherever email_use_tls=false -- strictly worse than the gap. I raised it rather than build the weaker half from the item's own wording, which says only "constrain or delete the SMTP mechanism selection". THE ESCAPE IS REUSED, NOT MINTED, AND I CHECKED THE RIGHT ONE. Asked to confirm an existing mechanism's semantics cover an SMTP surface before reusing its name, I found the obvious candidate does NOT: refuse_cleartext_credential_hop is keyed on a URL scheme and its second statement is `if scheme != "http": return`, so passing it "smtp" would have been a silent no-op -- a control that looks wired and enforces nothing. weakened_tls_escape_permitted is scheme-agnostic, posture-clamped, and already governs the LDAPS bind, the MLLP/FTPS contexts, the SFTP host-key acceptance and the webhook sink. That is the one used. Threaded as a parameter rather than read in-module: settings imports tls_policy, so the reverse edge would be circular. Same explicit-posture threading the store hop and the other out-of-gate cells already use. BOTH HALVES PROVEN BY PLANTING. CRAM-MD5 back into the approved set: the mechanism test failed. Dropping the unencrypted-channel refusal: the channel test failed. Each plant left the OTHER test passing, so neither is carrying the other. A positive control asserts the clamped escape still crosses an unencrypted hop -- without it, a helper that raised unconditionally would satisfy both refusals and the suite would report a working gate over a connector that can never authenticate. Restored byte-identical by SHA-256 both times. NOT YET WIRED, AND THAT IS THE NEXT LAYER RATHER THAN AN OVERSIGHT. The five login() call sites still call smtplib's own login. Four of them are on EmailDestination/DirectDestination, which capture NO posture -- login runs at SEND time, outside the construction gate that stamps the contextvar, so wiring them needs a posture threaded through construction the way alert_sinks already threads one into the webhook sink. That is a design step, not a mechanical edit, and it is stated here rather than guessed at.
…t choose (BACKLOG #1171, ASVS 11.4.1) XMLVerifier.verify takes expect_config and this call passed NONE, so the library's own default decided which algorithms a partner's signature could use. Measured on the pinned version, that default admits SHA-224 and SHA3-224 as digests plus six signature methods built on them. The V11 appendix disqualifies a sub-254-bit digest for any collision-resistance-requiring application, and a signature is one. Digests 8 -> 6, signature methods 25 -> 19. DERIVED BY SUBTRACTION, NOT REBUILT. dataclasses.replace keeps every other field the pinned version ships -- require_x509, expect_references, the c14n method -- so a future signxml that hardens a default we never named is not silently reverted by this call. A hand-built config would freeze today's answer to questions this change is not trying to decide, and there is a test asserting exactly that. SCOPE DELIBERATELY NARROWER THAN THE ITEM'S, and this is the part worth reading. #1171 also reads the appendix as disallowing RSA-PKCS#1-v1.5 and DSA outright, and both are in the default accept-set -- RSA_SHA256 and its siblings are PKCS#1 v1.5, DSA_SHA256 is present. THAT RESTRICTION IS NOT APPLIED. It would refuse the most common XML-DSig signature in use, and the appendix's text cannot be read from this checkout to confirm the reading. Breaking the common case on a relayed reading of a standard is not a call this change makes; it needs someone who can open the source. BOTH ARMS PROVEN BY PLANTING. Returning the library default unfiltered: the weak-digest test failed. Emptying the accept-set instead of narrowing it: the positive control failed -- without which an empty set would satisfy "no weak algorithm survives" perfectly while refusing every signature, a different defect wearing the same green. Restored byte-identical by SHA-256 both times. This is #1171's fourth limb. SFTP (f385a6d), HTTP Digest (5f7be3e) and the SMTP helper (87d7223) are the other three; the SMTP helper's five call sites remain unwired pending a posture-threading decision. The cell does not move.
…BACKLOG #1171, ASVS 11.4.1) send_plain_email ran STARTTLS only when use_tls, then logged in whenever a username was set. So [alerts].email_use_tls=false beside email_username put the password on the wire, with no refusal at the function, at EmailTransport, or at the serve gate. Both callers -- EmailTransport.send and security_notify -- pass that same knob, so the gap was reachable twice. THE ITEM NAMED FIVE UNRESTRICTED smtp.login() SITES AND FOUR WERE ALREADY GATED. transports/email.py:153 and transports/direct.py:203 refuse this exact combination at construction, and their tests SET the escape env var and still assert the raise. Wiring the escapable config/tls_policy.py helper into those four would have converted an absolute refusal into an escapable one, so it is not wired in; the fifth site is made to match them instead. One new rule would have been a second spelling of a rule that already exists. ABSOLUTE, AND THAT IS WHY IT CAN LIVE AT THE CHOKEPOINT. The docstring above it explains that the tls_verify=false decision was pushed to the serve gate because it needs the CLAMPED posture, which this module cannot read. A credential over cleartext needs no posture -- it is refused on every instance in every environment -- so send_plain_email, which every caller passes through, is the right seam and covers both callers with one rule rather than two that can drift. Tests assert the fake recorded NO login, not merely that something raised: a refusal firing after the credential reaches the transport is not a refusal. Two positive controls pin the other half -- cleartext without a credential still sends, and a credential over STARTTLS still logs in -- so a guard that over-refused could not pass. MEASURED by mutating the guard to `if False`: exactly the three refusal tests go red and both positive controls stay green. AND THE FIXTURE WAS RESET PER TEST, BECAUSE MY OWN TESTS WERE READING STALE STATE. _RecordingSMTP.captured is CLASS state cleared only in __init__, i.e. only when a connection is actually opened. A refusal that fires before smtplib.SMTP is constructed leaves it holding the PREVIOUS test's dict, so "login not in captured" was asserting about the wrong test. The security-event case passed alone and failed after the one that logs in successfully; the other three passed only because of the order they ran in.
… (BACKLOG #1171, ASVS 11.4.1) smtp_login_approved shipped in 87d7223 UNWIRED. Its refusal arm was reachable from tests; its auth arm was executed by nothing, anywhere. Wiring it is what found that it never assigns smtp.user/smtp.password -- and stdlib's auth_plain says in its own docstring "Requires self.user and self.password to be set". It would have raised AttributeError on its first real authentication and passed every fake in the suite, because every fake modelled login() rather than auth(). A control proven only where it is not used is the finding here; the mechanism restriction below is merely the feature. WHAT THE RESTRICTION IS FOR. smtplib's login() builds its preference list as ['CRAM-MD5', 'PLAIN', 'LOGIN'] and takes the first the server advertises, so every unrestricted call proposed an HMAC over MD5 -- disallowed by Appendix C for any cryptographic purpose, in a clause with no default-off escape. auth() is now driven directly against the advertised list, restricted to PLAIN and LOGIN. WIRED AT ALL FIVE SITES, ALONGSIDE THE EXISTING GATES AND NOT INSTEAD OF THEM. The item named five unrestricted call sites; four were already guarded at construction, and those guards refuse a cleartext credential absolutely -- their own tests set the escape env var and still assert the raise. Passing escape_permitted=True would have made an absolute refusal escapable, so every call site passes False. The construction gate still refuses first; this is a send-time backstop that is never weaker than the gate it backs up. SECOND DEFECT, CAUGHT BY THE SUITE: a rejected credential was being wrapped in InsecureHopRefused. A wrong password is a DELIVERY failure, not a policy refusal, and it had stopped mapping to DeliveryError -- an operator with a typo would have gone reading TLS policy. The server's own exception now propagates; InsecureHopRefused is reserved for the two decisions this function actually makes. REJECTED, RECORDED BECAUSE IT WAS THE CHEAP OPTION: filtering CRAM-MD5 out of smtp.esmtp_features so login() picks PLAIN itself. No fake would have needed touching. It means falsifying the library's record of what the server advertised, which every later reader of that field consumes as truth, and the set of readers cannot be enumerated. THE FAKES NOW ADVERTISE CRAM-MD5 DELIBERATELY. It is what a regression to login() would select, so a fake offering only PLAIN/LOGIN could not tell a fixed cell from a broken one. VERIFIED: ruff check, ruff format, mypy strict over 267 files, 198 tests across every affected module, and a mutation pass -- neutering the guard reddens exactly the three refusal tests and leaves both positive controls green; swapping smtp_login_approved back for smtp.login reddens the CRAM-MD5 conformance test. NOT VERIFIED AT COMMIT TIME: the full suite. It is running and roughly a third of the way through. Committing ahead of it deliberately -- uncommitted work has no SHA and the pool rate projects exhaustion before the run would finish. If it reds, the fix is a follow-up commit; the alternative was risking the whole change to avoid that.
…KLOG #1171, ASVS 11.4.1) InsecureHopRefused subclasses ValueError, so it escapes both connectors' `except smtplib.SMTPException` arms and lands in the delivery worker's catch-all -- which wiring_runner documents as "Internal/code error (our bug, not the partner)" and which dead-letters the row under the default CONTINUE policy. So a relay offering no approved AUTH mechanism, or a connection configured without TLS, would have told an operator to go read our source. FOUND BY A PEER'S ADVERSARIAL PASS, NOT BY ME. I checked this exception's class while wiring and judged it correct -- but I judged it against the CONSTRUCTION gate, which raises ValueError and is supposed to. At send time the same class means something else, and I did not re-ask the question at the second site. IT IS THE SAME MISCLASSIFICATION ONE LAYER DOWN from the one this branch already fixed. A rejected credential was being reported as a policy refusal; a policy refusal was being reported as an internal code error. Each sends the operator to the wrong file. Both connectors now convert it to DeliveryError, which is the transport-failure arm: retried per policy, lane marked unhealthy, alert raised. The refusal text is carried whole rather than reduced to a type name -- it is host, cell and mechanism names only, never message content, and it is the one thing that tells an operator what to change. TESTS: a relay advertising ONLY CRAM-MD5 must surface as DeliveryError and must NOT have received the credential. Plus a positive control -- a relay advertising CRAM-MD5 and LOGIN still authenticates, falling through to LOGIN -- because a guard that refused every authenticated send would satisfy the first test perfectly. AND THE FAKE'S ADVERTISED SET IS NOW A CLASS-LEVEL DEFAULT, because the first version of these tests did not work. __init__ assigned self.esmtp_features, an INSTANCE attribute shadowing the class attribute the test patched, so the patch had no effect and the relay kept advertising the full set. The thing I patched was not the thing being read; __init__ now copies from AUTH_ADVERTISED so a test can vary it. MEASURED: removing one conversion arm reddens exactly the refusal test and leaves the positive control green.
…ard caught (BACKLOG #1216) `test_every_non_engine_test_is_classified` failed on the branch's first FULL suite run: test_bash_resolver.py imports no engine module and appeared in neither tooling_manifest.txt nor _STAYS_WITHOUT_IMPORTING, so it was unclassified and would have run on every engine leg forever. I ADDED THAT FILE HOURS AGO AND NEVER RAN THE GUARD. Every verification before this was targeted modules -- the ones I had touched -- and this guard was not among them. "The files I touched are green" is not "the tree is green", which I wrote in my own words before committing on exactly that basis. MANIFEST, NOT _STAYS_WITHOUT_IMPORTING. The list is for files that LOOK like harness but whose subject is engine source; this one's subject is the shared bash resolver that four harness tests consume -- merge_gate_controls, installed_coord_hooks, workflow_shell_syntax, dependabot_automerge_guardrails -- and all four are manifest entries themselves. It reads no engine source, so the second rot mode the guard's docstring names does not apply. THE PATH-GATE QUESTION, ASKED BEFORE ASSUMING: a manifest entry moves to the path-gated tooling job, so a change that breaks it can miss its own PR. ci.yml answers it in place -- the job also runs on every push to main, and its comment states that is deliberate: "The push arm is the safety net, not redundancy: the PR arm only fires when the harness itself changed... push-to-main is where it surfaces." This entry sits in exactly the position of the four tests it serves, under the same documented backstop. MEASURED: removing the entry reddens the guard; restoring it greens it. 84 passed across the guard, the resolver tests and all four consumers. FULL SUITE at f6796e8: 1 failed, 13142 passed, 833 skipped, 34m18s. This was the one failure.
…#1216) bash_candidates tried `<git>/bin/bash.exe` before `<git>/usr/bin/bash.exe`. The first is Git for Windows' MINGW64 WRAPPER: it rewrites the inherited PATH so /mingw64/bin leads, ahead of anything the caller prepended. Git ships curl.exe there, so a test that prepends a curl stub is silently bypassed and the step reaches the LIVE network -- which is how a release-age check passed off pypi.org instead of off its fixture. MEASURED HERE, same command, one variable: Git/bin/bash.exe PATH head -> /mingw64/bin (the prepended stub dir is GONE) Git/usr/bin/bash.exe PATH head -> <the prepend> (preserved) And require_bash was selecting the wrapper. SELECTIVE, WHICH IS WHY IT READ AS FLAKINESS: only stubs whose binary Git also ships in mingw64/bin lose. gh and jq stubs kept winning because Git ships neither. THE PART THAT IS MINE TO OWN: bash_sees CANNOT CATCH THIS, and that is not a gap in the probe -- it is a different dimension. It asks whether the interpreter shares this process's FILESYSTEM NAMESPACE, and the wrapper does, perfectly. PATH ORDER is orthogonal. So the control could not fail in the direction the thing was failing, which is the rule I had published hours earlier and then broke in the code I wrote after stating it. Demonstrated in a test rather than asserted: the wrapper passes bash_sees and fails the new probe. bash_preserves_path_order is the control for that dimension, and require_bash now demands BOTH -- a WSL bash preserves PATH order and cannot open the file; the wrapper opens the file and rewrites PATH. Neither control alone admits only the right interpreter. MEASURED: reverting the candidate order reddens the ordering test; the resolver now selects Git/usr/bin/bash.exe; the four consumer modules pass 70 with 7 jq-gated skips. NOT VERIFIED HERE, said plainly: the CI rows that actually failed are jq-gated and SKIP on this box, so I have confirmed the mechanism directly and have NOT watched those rows go from red to green. The lander reproduced the mechanism independently on their box.
…CKLOG #1216) Prepending a stub directory to PATH is not the same as the stub being CHOSEN, and the gap is silent: the step runs, the real binary answers, and the row decides on whatever the network said. _run_step_body now resolves every stub with `command -v` in the SAME child env the body is about to execute in, and fails loudly naming what resolved elsewhere. BY RESOLUTION, NOT BY READING PATH BACK. `command -v` reports what the shell would actually execute. Reading $PATH would only confirm the string we just set -- the wrapper's entire behaviour is to rewrite it between here and there. THIS IS DEFENCE IN DEPTH, NOT A DUPLICATE. The resolver decides which bash we launch; this decides whether the stub won in the environment this body runs in. A future shell, a wrapper script, or a CI image that reorders PATH for its own reasons is caught here without anyone having predicted it. AND I SHIPPED IT UNREACHED FIRST, WHICH IS THE DEFECT I FIXED AN HOUR AGO. MEASURED: mutating _assert_stubs_win to raise unconditionally changed NOTHING in this module -- 27 passed, 7 skipped, byte-identical. Every row that uses path_prepend is jq-gated and skips on this box, so the guard was correct by construction and exercised by nothing, exactly like the helper that shipped unwired in 87d7223. SO IT NOW HAS A DIRECT TEST, and the discriminating pair is free on Windows: the same stub directory WINS under the resolved bash and LOSES under Git's bin/bash.exe wrapper, because that wrapper puts /mingw64/bin first and Git ships curl.exe there. Both halves asserted -- a guard that raised for everything would satisfy the negative half alone. It skips only when the wrapper is genuinely absent or ships no mingw64 curl, and says which. MEASURED: neutering the guard reddens the new test; restoring it greens. 28 passed where 27 passed before, so the test is running rather than collecting.
…KLOG #1216) My regression test identified Git's MINGW wrapper by shape -- parent `bin`, grandparent not `usr`. On Linux that matches /bin/bash, so the skip never fired and the test asserted that a plain Linux bash rewrites PATH. It does not. The assertion was right and the SUBJECT was wrong. Red on ubuntu-latest; found by the lander reading the CI output. WORSE THAN REPORTED, MEASURED: it matches TWO Linux shapes, not one. /bin/bash and /usr/local/bin/bash both have parent `bin` and a grandparent that is not `usr`. Only /usr/bin/bash was excluded, and by accident of the one condition that happened to fit. THE SUFFIX IS THE PLATFORM TEST, and sys.platform is deliberately NOT. The wrapper is `bin/bash.exe` by construction, so `.exe` identifies it exactly. A platform guard would make the docstring's "skips only when the wrapper is genuinely absent" false a SECOND way: it would also skip on a Windows box with no Git, for a reason the sentence does not name. The suffix keeps the stated and actual skip conditions identical. That reasoning is the lander's and it is better than what I would have written. SIMULATED RATHER THAN GUESSED, because this box cannot run Linux: the ubuntu candidate shapes now match NOTHING (skip fires) and the Windows set still matches exactly the wrapper. Asserted in both directions -- a predicate that matched nothing anywhere would satisfy the Linux half alone. FIXED IN BOTH COPIES. I had duplicated the predicate into test_dependabot_automerge_ guardrails.py when I wrote the self-certification test, so the same defect existed twice and a one-file fix would have left the second to fail on the next ubuntu run. AND THE FIRST ATTEMPT AT THIS EDIT DID NOTHING. My anchor omitted the leading `if `, the assertion refused, and the ruff and pytest that followed ran on UNMODIFIED files -- so a green appeared for a fix that had not been applied. The anchor is why that surfaced instead of shipping.
…122) ASVS 3.5.3. `assert_not_cross_site` runs as a route dependency, and `/ui/static` is a Starlette `Mount` rather than an `APIRoute`, so no dependency runs for it. The asset tier was the one /ui surface the per-route check could not reach, which is why this is middleware rather than another dependency. THE RULE IS NOT THAT CHECK AT A WIDER SCOPE, and building it as one was a live defect in this branch. A first cut read `Sec-Fetch-Site` alone by reusing `_CROSS_ORIGIN_FETCH`, which 403s every cross-site TOP-LEVEL NAVIGATION: intranet links into the console, and every real SSO login, since the IdP redirect back is cross-site by construction. That set is correct for its existing callers only because they have already established the request is a non-navigation fetch -- its name says FETCH. Reuse did not carry the precondition across. `test_oidc_callback_survives_a_cross_site_navigation` caught it, having warned in its own docstring that otherwise "every real login would 403 while every hermetic test still passed". None of the four tests written alongside the middleware did: they shared the mental model that produced the defect. The refusal now needs cross-site AND not-a-safe-top-level-navigation -- `navigate` mode, GET or HEAD, destination outside `object`/`embed`. Method is load-bearing, because a cross-site POST navigation is a CSRF form submission and no supported flow makes one (`response_mode=form_post` is not implemented here). Two constraints were measured before any code was written. Each looks like a hardening improvement and each breaks shipped behaviour if inverted: - ABSENT IS ALLOWED. `Sec-Fetch-Site` is browser-populated; `tray/probe.py` builds its client with no headers at all, and 332 headerless /ui call sites exist in this corpus. - 403, NEVER 404. `tray/probe.py` maps 404 to DISABLED and every other status to ENABLED, so a 404 would make the tray report a healthy console as switched off. Eight tests. Three mutations, each reddening exactly one test and the right one; mutation 1 is the navigation defect above, so this file now catches it rather than borrowing test_webui.py's coverage. Full webconsole suite: 383 passed, 3 skipped.
ASVS 1.2.2, the console clause. The apiclient half of this item shipped in #488 with `_seg` and is pinned by tests/test_apiclient.py; the console half was left behind, and measuring it on origin/main found `safe=""` appearing ZERO times in messagefoundry_webconsole/ against a control of 31 files matching `def ` in the same run. `quote`'s DEFAULT is the defect. It leaves `/` alone, which is the one character a path segment turns on: measured, `quote("IB/ACME")` returns it UNCHANGED. So the two sites that looked encoded were not, and the dead-letter replay forms interpolated a channel id and a destination name with no encoding at all -- a connection named `IB/ACME` addressed `/ui/dead-letters/IB/ACME/replay`, a different route with an extra segment. SCOPED TO CONNECTION NAMES, deliberately. The other /ui interpolations carry ids, where the "every interpolated id is a `uuid4().hex`" argument is probably sound. It is not sound for connection names: `Registry._add` checks only for a duplicate, so a name is unconstrained free text. The id sites are left alone rather than swept, because that argument rests on a data-grammar invariant no URL-building code asserts, and settling it is separate work. A blanket sweep would also be WRONG, which is why one test exists purely to stop it: `_auth`'s reauth `next` carries a whole path inside a query parameter and is correctly `safe="/"`. Encoding it as one segment breaks every re-auth redirect. Five tests. The dead-letter pair renders the real page rather than reading the f-string; a source scan guards the guard so a new site on these routes reds the suite instead of slipping in unencoded. Three mutations, each caught -- and the connections.py one is caught ONLY by the source scan, which is why that scan is there. VERIFICATION AT COMMIT TIME, stated exactly: ruff, mypy strict over 33 files, the 5 new tests, and 3 mutation passes all green. The full webconsole suite was still RUNNING when this was committed, under a pool-exhaustion warning, and had not reported. If it reds, the fix follows this commit rather than being folded back into it.
ASVS 1.2.2, completing the console clause. All 54 interpolated /ui path literals were partitioned by reading each value's PRODUCER rather than its interpolation line, which is the only way this question has an answer. THE ID SITES ARE SAFE, BUT NOT FOR THE REASON USUALLY GIVEN. The prevailing argument is "every interpolated id is a `uuid4().hex`". What actually protects them is that each is read back from the store after a lookup that 404s on a miss, so a crafted path param never reaches a render -- `get_message` and `replay_message` both raise before their pages build. `ui_role_update` IS THE SINGLE EXCEPTION ON THE SURFACE. A `ValidationError` from `CustomRoleRequest` short-circuits BEFORE `update_custom_role` runs, so that 404 never happens, and the 400 branch rebuilds the page from `CustomRoleInfo(id=role_id, ...)` with the RAW path param. `CustomRoleInfo.id` is a bare `id: str` with no `Field` constraint. Both form actions now go through `_seg`. Three sites reported as needing encoding were REJECTED after reading the gate above: the two `pages/messages.py` back-links and `routes/core.py`'s replay redirect. They should use the store value in scope rather than the path param, but nothing hostile reaches them, so that is code quality and not this item. A CORRECTION THAT CHANGES NO CODE, recorded because deleting the call would open it: `_auth`'s reauth `next` is safe because of the `quote()` AT THE SITE, not because the value is server-generated. Adversarial review showed attacker-influenceable bytes do reach it. Right answer, wrong reason -- the same shape as the constant lifted out of its precondition in #1122 earlier today. VERIFICATION AT COMMIT TIME, stated exactly: ruff, ruff format, mypy strict over 33 files, the 7 tests in this file, and 2 mutation passes (each reverting one form action to the raw id, each reddening the right test) are all green. The full webconsole suite was still RUNNING when this was committed, under a pool trigger, and was clean through 36% with no failures. If it reds, the fix follows this commit rather than being folded into it.
TWO DEFECTS, ONE DISCOVERED INSIDE THE OTHER.
#1318 -- `messagefoundry init` wrote `[api].host`, which ADR 0118 relocated to
`[security]`, so `_reject_relocated_keys` refused the file the product had just
generated. First serve after first init exits 2. Shipped in v0.3.1 and v0.3.2.
THE SCAFFOLD DID NOT CHANGE. THE LOADER DID. The template emitted `[api].host`
since before v0.3.0 and was correct then. The relocation swept the documents and
the settings and never swept the one place that WRITES a config file, so a correct
template was invalidated from elsewhere with nothing connecting the two.
The fix moves the keys to their real home rather than deleting settings an
operator needs: `[api].host` split into `[security].local_access_only` +
`listen_address`, `[ai].data_class` -> `handles_real_patient_data`,
`[ai].production` -> `production_instance`, `[egress].deny_by_default` ->
`[security].block_unlisted_outbound`.
Census, reproduced independently and widened: scaffold.py writes THREE toml files,
not one. `environments/dev.toml` and `environments/prod.toml` carry zero relocated
keys for a STRUCTURAL reason -- they are flat `env()` value files, so no
(section, key) pair CAN match. Population was 1 active and 3 commented-but-armed,
each of the three with an instruction beside it telling an operator to uncomment
it.
Probing each commented key individually caught the same trap in the NEW template:
uncommenting `listen_address` alone is refused, because `local_access_only`
defaults true and `0.0.0.0` is not loopback. The two are now presented as a pair
to uncomment together.
#1320 -- THE GATE WAS NOT BLIND. IT SAW THE DEFECT AND CALLED IT A SKIP.
`messagefoundry check` LOADED the config, got the exact refusal, PRINTED it, and
returned `rc 0`:
skip posture: settings did not load: [api].host moved to [security]...
FOUR sites in checks.py caught the load error and returned `ok=True, skipped=True`
-- posture, build-check, alert-smtp-tls and reference-backend, three of them
`required=True`. So the commit and CI gate could not fail on ANY unloadable
service config, not just this one.
ABSENT IS A LEGITIMATE SKIP; PRESENT-BUT-REFUSED IS NOT. The except clause
rendered two states with opposite remedies identically -- the same shape
CLAUDE.md already records for `goalPromptedAt`. The four absent-file skips are
untouched.
A FIFTH SITE IS DELIBERATELY UNCHANGED, said here rather than omitted so "four"
is not read as the whole surface: the `snapshot_on_send` resolver returns
`PipelineSettings().snapshot_on_send` on a load failure. It is a value resolver
rather than a check, and it still swallows silently.
THE EXISTING TEST WAS PINNING THE DEFECT. `test_scaffold_writes_the_skeleton`
asserted `"data_class" in toml and "production" in toml` -- the RETIRED spellings
-- under a comment claiming it checked "the new posture model". A string
assertion cannot survive a rename made somewhere else, so the real guard is now a
round trip through `load_settings`.
Four tests. The durable one uncomments every commented key ON ITS OWN and requires
that none produces a relocation refusal, so the NEXT relocation reds this suite
instead of shipping. `test_check_fails_on_a_config_that_is_present_but_refused`
asserts the POSTURE ROW, not merely a nonzero rc: measured, reverting posture
alone reddened nothing because build-check still failed, and a test that needs
every site to break before it fails is not guarding any of them.
RED FIRST, as required: restoring `[api].host` to the template now reddens
`test_scaffolded_config_passes_check` -- the test that asserted `rc == 0` and
passed over the broken config for two releases.
VERIFICATION AT COMMIT TIME, stated exactly: ruff, ruff format, mypy strict over
267 files, and 158 tests across test_scaffold, test_scaffold_requirements,
test_checks and test_cli are green, with four mutation passes. The FULL engine
suite was at 5 percent and still RUNNING when this was committed; it had reported
no failures. If it reds, the fix follows this commit rather than being folded into
it.
COMMON 5.6b: session handoffs live under `<git-common-dir>/mefor-coord/handoffs/`, which is inside `.git` and therefore untrackable BY CONSTRUCTION -- the rescue mechanism pushes a REF, and a ref carries only committed content, so no rescue tag has ever covered one. Against a pool cutoff that location is fine; the exposure is machine loss. This commits only the part the rule names as irreplaceable: instruments that returned a confident wrong answer, and the rules that survived them. The per-session narrative stays in the handoff, per COMMON 5.1. Every entry was measured, and every one was CORRECT about what it measured and wrong about what it was asked -- `head -60` printing three of four sites, a `grep -c` of 8 read as absence when all 8 hits were the handling, `git status` answering "differs from HEAD" to "is someone mid-edit", two mail timestamps read as a cause, a SHA read as a state. Four rules, of which one is new and sharpens a rule this repo already had: a positive control proves the INSTRUMENT, not the AIM. Confirming a pattern fires on a corpus establishes the grep works there; it cannot establish that there is the right place to look. Two shapes that are not instrument failures are recorded beside them because they were mistaken for such repeatedly: a marker that reverses its own meaning in context (the section 11 glyph argument, reproduced in prose), and that mutual deference has no fixed point -- nor does mutual assertion, which is the same failure with the opposite sign. Contributed by several sessions working the same tree, not one seat. No new claims are made here. Every measurement cited was taken by the seat that reports it, and the two population claims that were withdrawn during the session are recorded as withdrawn rather than restated.
Credited to the lander and the dispatcher, who found the sharper statement by falling into and setting the trap respectively. WAS: "a marker that reverses its own meaning in context is worse than no marker" -- true, and it does not tell you what to do. NOW: A STATUS MARKER MUST NOT BE A WORD THAT ALSO APPEARS IN NARRATION. That is checkable before you write it. It also records the second instance: one reader took "SUPERSEDED BY #1326" from a body paragraph as the row's status, which is the same presence-equals-meaning failure as the first, in the opposite direction. Adds the part that makes it operational here: the banner alphabet is the status channel and `parse_items` is its only correct reader. Anything in the body is narration however emphatic it is. Four seats grepped a body for a status word tonight and none of them was careless.
Collaborator
Author
|
The red is not this branch's. Attributed, not hand-waved as a flake.
This branch cannot reach that code. Its twelve files are webconsole, The assertion is a throughput-monotonicity SLO -- this run's aggregate against 75 percent of a prior run's -- which is environmentally sensitive by construction on a shared runner. A previous lander proved that specific red environmental earlier today with a same-arm test: a commit changing no code. Worth flagging beyond this PR: Not re-running it. This PR is a draft for triage and must not be armed, so a green here buys nothing and costs a full cycle on a Windows leg. Whoever picks the triage up should re-run then, against a rebased branch. |
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.
Draft on purpose. Do not arm. Opened by the lander to make a dead session's work visible; the Dispatcher flagged that nothing reports a branch which is merely never pushed.
The framing this corrects
It was routed to me as "39 unpushed commits of built work". The count is right and the reading is not. Four PRs have already merged from this exact branch -- #488, #491, #496, #505, the last at 08:50Z today. A squash merge leaves the source branch permanently ahead of
main, because the squashed commit carries a different patch-id than the originals. So most of the 39 is squash residue, not stranded work.git cherryreports 31 of 31 as not-upstream, which is the trap: it compares patch-ids, and a squash defeats that. Reading it literally would have routed 39 mostly-landed commits into a PR.What is genuinely unlanded -- six commits, all verified off
origin/main4eee9f88/ui, including the Mount (#1122)9e4c1524/uipath segment (#1107)549e3044721635ceinitwrites a loadable config, andcheckcan fail on one (#1318, #1320)cb010d7b67843d66Each checked with
git branch -r --contains; none is reachable fromorigin/main.#1318 is NOT a duplicate of #524
#524 landed the #1318 ledger filing (
docs/BACKLOG.mdonly).721635ceis the #1318 code fix --checks.py,scaffold.py,test_scaffold.py. Complementary halves. Landing this does not re-land that.Why draft and not armed
721635ce's own commit message says the suite was at 5 percent and still running when it was committed. Its tests are unverified by its author.mainand its session is gone, so nobody owns a rebase.required_approving_review_count: 0, arming would merge them unread.Nothing here was ever at risk
The tip
67843d66is reachable fromprivate/rescuetags/auto/MessageFoundry/claude/builder-2-6e58d6. Preservation was never the issue; visibility was. That is what this draft fixes.Whoever picks this up: rebase onto
main, run the suite that721635cenever saw finish, and split the two docs commits out if they slow the security three.