docs(asvs): correct a false published attestation, declare the scope boundary, and gate a closed cell against silent re-scoring - #156
Merged
Conversation
… attestation that was not true
Two public documents, one cause: the assessment method never wrote down where the
software ends and the substrate begins, so a platform-property requirement had no
principled home and churned between verdicts -- and separately, a published
attestation claimed more than the record supports.
ASVS-ASSESSMENT-METHOD.md
* Section 2 now states the boundary the "assessed as source" phrase already implied:
the subject is three SOFTWARE ARTIFACTS, not the host, hypervisor, CPU, firmware
or network. Where a requirement's VERB names a property of that substrate, rule 1
applies. This is ASVS's own principle and is now quoted verbatim from 0x03 at the
v5.0.0 tag rather than paraphrased -- "excludes requirements ... where
configuration is outside the application's responsibility", with the DNS example.
* Two guards, because a scope clause grows to swallow inconvenient cells: reporting
on a platform property is not providing it AND is not disqualifying either -- ask
what the verb requires, and of what.
* Explicitly declines the fork route. 0x03 does say organizations are "strongly
encouraged" to fork, but its omission examples are technology-not-used
(GraphQL/WebSockets/SOAP) and forking changes what you claim conformance TO.
Rule 1 against a declared scope is narrower and leaves the target unchanged.
* New section 2.1: what an out-of-scope cell does NOT buy. 4.0's "may still claim
full ASVS compliance" clause was DROPPED in 5.0, and OWASP retains normative
authority over which requirements sit at which level -- so an L3 claim omitting an
L3 requirement is non-conformant on OWASP's own terms.
* 11.7.1 moves from the worked example of a rule-3 fail to the worked example of a
rule-1 na, with the objection it has to answer stated rather than omitted.
Secure_Development_Standards.md
* "verified against OWASP ASVS 5.0 Level 3" -> "assessed against ASVS 5.0 using
Level 3 as the target -- an assessment IN PROGRESS". The old wording was
unsupportable three ways: the survey is incomplete and unread cells are explicitly
not passes; open requirements exist including at L2; and an L3 requirement is
scoped out, which under 5.0 does not preserve an L3 claim.
* The correction is stated in the document rather than made quietly, because the
claim was PUBLISHED. No figure is restated here, deliberately, so this page cannot
go stale against the record.
* Notes that no accredited L3 pathway exists to appeal to: OWASP certifies nobody,
and the one ASVS-based scheme accredits testing FIRMS at L1-L2.
…the four causes When 11.7.1 was scoped out under rule 1, the fail count went 3 -> 2 with ZERO lines of engine code changed. A reader comparing only the totals across those two days concludes a defect was fixed. Nothing was. The rationale sits on the cell and the boundary sits in section 2, but neither is visible to someone reading a number. New section 2.2 names the four causes a bucket total moves, of which exactly one is an improvement: a control shipped on by default (real); a cell read for the first time (the survey advanced -- unverified -> pass is a DISCOVERY, not a fix); a scope boundary stated (identical code, smaller denominator); a rule applied more carefully (more accurate, and these move DOWN too). Two obligations, both cheap: never report a total as a trend without naming which cause moved it, and when a count improves, state what would have had to happen for it to MEAN improvement and whether that happened -- the negative-control discipline applied to a metric. Written down because it cuts AGAINST us more often than for us. The survey is incomplete, so most future movement is cause 2, and the aggregate will get worse before it gets better as unverified cells resolve into real verdicts. That is the survey working, and it should be reported as such rather than defended against. Sits directly under 2.1 (an out-of-scope cell buys no Level 3 claim), because the two failure modes are the same one seen from opposite ends: 2.1 stops a scoped-out cell becoming a conformance claim, 2.2 stops it becoming a trend line. Prompted by the ASVS-11.7.1-summary session, whose objection -- that the rule-3 observations about the code stay literally true even after the verdict moves -- is what made the gap visible.
…t stands still Section 2.2 as first written had a blind spot: it teaches a reader to interrogate a number that MOVED, and the worse class hides in stillness. Code changes, an evidence anchor drifts off the line it was pinned to, and the recorded verdict quietly stops describing the code -- with the total perfectly stable. Stability reads as "nothing to see", which is why nobody goes looking. Not hypothetical, and not cause 4. Cause 4 is deliberate: someone re-read a cell. This is an evidence pointer breaking on its own. Measured here on 2026-08-02: SEVEN anchors across six cells had drifted and not one bucket total moved. It is catchable only by a gate, which is what ADR 0156's drift gate is for. Adds a fifth cause too: THE STANDARD ITSELF CAN MOVE. The 345 denominator is pinned to ASVS v5.0.0 by digest; a 5.0.x patch would change requirement text and counts with zero code change and zero assessment work -- and it is the cause most easily mistaken for the survey advancing. Third obligation, symmetric to the first two: a stable count is not evidence of a stable posture unless the anchors were re-verified in the same pass. Report the drift check with the total, or you are publishing the freshness of the last check rather than of the software. And state the pinned ASVS version with any total, so a denominator change surfaces as a version change instead of as progress. Both additions from the ASVS-11.7.1-summary session, which read section 2.2 out of the git object rather than trusting the relay and found the gap in it.
…e ADR that rejected this ruling Owner ruling: 11.7.1 is na, LOCKED as of 2026-08-02. Three changes make that true rather than merely written down. 1. THE CLOSURE IS NOW A GATE, NOT PROSE. load_scorecard refuses a cell carrying `decision_closed = true` whose verdict has moved off `decision_closed_verdict`, and refuses a closure with no pin at all (a closure with nothing to compare against is a comment, not a control). Deliberately an error, not a warning: the cost of a false stop is one conversation with the owner; the cost of a silent re-score is a posture document that disagrees with the record and is found months later. Prose was already tried here and did not hold -- four passes each believed they were doing careful work, and a rationale they could read never stopped them. Four tests, and the two that matter were PROVED RED before being trusted: with the guard disabled both closure tests fail with "DID NOT RAISE ScorecardError", while the green-half test (verdict still matches its pin) and the reach control (a cell that never opted in is untouched) keep passing. Verified against the real vault data too: exit 0, 345 cells, 750 anchors, 86 absence claims. 2. ADR 0152's REJECTION OF THIS RULING IS ANSWERED IN PLACE. Its "N/A deferred" paragraph is the strongest attack on what just landed and is the record of one of the four verdict moves -- the one that went the other way. Its stated ground is FALSIFIED: "ASVS's enumerated grounds are absent functionality and external processes" is not what ASVS says. 0x03 at the v5.0.0 tag excludes requirements "where configuration is outside the application's responsibility", DNS example -- a responsibility exclusion, the exact category the paragraph denies exists. That error survived because the pinned corpus is REQUIREMENTS-ONLY and holds no chapter prose, so no check against it could ever catch a wrong claim about the standard's text. I re-asserted it myself before catching it. The adopted ruling also does not rest on the ground that paragraph rejects: not "the platform provides it", but rule 1 against a positively-declared scope of three SOFTWARE ARTIFACTS. And its "strictly weaker than measuring the property" assumed measuring was achievable -- research since establishes no attestation on any platform asserts memory encryption as a property. 3. ADOPTER GUIDANCE CORRECTED. SYSTEM-REQUIREMENTS.md and CONFIGURATION.md told adopters to "disclose 11.7.1 as Partial" -- pre-filling a verdict in someone else's assessment, and one this project no longer holds either. Both now say to assess it against their own deployment, with the correction stated rather than made quietly. Recorded and not buried: the fail count moved 3 -> 2 with ZERO engine code changed. The posture did not improve; the scope got stated. NOT mirrored to the vault yet -- the vault mirrors engine MAIN, and this is a branch. The f6d6f38b guard fires when this reaches main, not before.
…closure Regression this renderer caused, inside one session. 11.7.1 was closed while it was a `fail`, so its STOP text surfaced in the Open-cells table -- the highest-traffic face of the record. The same ruling then moved it to `na`, it fell out of `open_states`, and the rendered record went SILENT about the one cell that had just been ruled on. A reader of ASVS-CURRENT.md would have seen nothing at all. Adds a "Closed by owner decision -- do not re-score" section that renders on `decision_closed`, independent of verdict, and promotes decision_closed/_on/_by onto the Cell so the renderer can see them rather than re-parsing raw TOML. Two tests, and the second is the one that matters: a closed `na` cell renders (the regression), AND the section is absent when nothing is closed (a control on the rule's REACH -- without it the first test passes just as well for a renderer that emits the heading unconditionally). Found by the ASVS-11.7.1-summary session's grounding audit, which flagged that the Cell record models no closure fields at all; checking that turned up the live case.
…ol is a false premise PHI.md contradicted itself about one control, three hundred lines apart. Section 2 (:211-214) records plainly that there is NO [security].volume_encryption_declared setting at HEAD and that nothing in the engine checks FDE is on. Section 10's residual text (:584-586) then offered "the documented restricted-service-account + volume-encryption posture" as the compensating control for the in-use-memory residual, phrased as though it were something the product supplies. Both halves are operator-asserted and engine-unchecked. The claim is now qualified at the point of use rather than only in the section nobody reads alongside it, and it points at section 2's own statement so the two cannot drift apart again. Deliberately weakened, not removed: the posture IS worth doing and does mitigate where an operator applied it. What was wrong is the implication that the engine knows. CLAUDE.md section 11: a compensating control must not rest on a false premise. Found by the ASVS-11.7.1-summary session's grounding audit while checking what the 11.7.1 record would inherit -- verified against the code before acting (grep for the setting returns nothing).
… -- and the stale one was the Fail
The ledger carried three verdict statements for 11.7.1 and they did not agree. After
the 2026-08-02 owner ruling the picture inverted from what I expected: the two saying
N/A (:415, :429) are now RIGHT, and the one saying Fail (:6390) is the stale one.
:6390 said "13.3.3/11.7.1 remain **Fail**". Neither is a Fail on the record.
11.7.1 is `na`; 13.3.3 is `unverified` -- never read against the requirement
text, which is explicitly NOT a verdict. Both re-derived from the scorecard
before writing, not assumed.
:415 verdict right, REASON wrong. "Unachievable for pure-Python on-prem" is not
the ground -- the engine does ship ADR 0152 rungs 1-2; they REPORT ON the
platform property rather than PROVIDE it. The ground is that the verb names a
CPU/firmware property, outside the declared scope of three software
artifacts. A right answer for a wrong reason is the thing that gets
re-derived into a wrong answer later.
:429 left standing but qualified: "residuals are closed" is a STAFFING judgement,
not a coverage claim, and reads as a posture summary it cannot support while
most cells have never been examined.
Left alone deliberately: :245, :523, :6384, :6388, :6394 are build-history and
deferral records, not verdict claims. An over-broad sweep is its own defect.
None of these three sites matches the ledger gate's `^#{2,3} (\d+)\.` heading regex,
so nothing here was gate-protected and nothing would have caught the contradiction.
…te one The most reusable finding from today's sweep, and it is not a verdict. A false statement about ASVS -- "ASVS reserves non-applicable for functionality-based exclusions" -- survived TWO independent assessors and reached a signed-adjacent risk-acceptance block. Not because anyone was careless: because every one of them verified against the pinned corpus, and the corpus is REQUIREMENTS-ONLY. It carries no chapter prose, no assessment guidance, no definitions. A claim about what the standard SAYS outside a requirement's own text is structurally uncheckable against it, and the check comes back clean every time. 0x03 says the opposite of what was asserted: it excludes requirements "where configuration is outside the application's responsibility". One fetch of the chapter settled it; no amount of corpus checking could have. New section 2.1a states the rule and the instruction that follows from it: to cite ASVS prose, fetch the chapter at the v5.0.0 tag and quote it verbatim -- never paraphrase from memory, from an earlier assessment, or from another agent, and never read a green corpus check as evidence about a claim the corpus cannot represent. Sits between 2.1 (an out-of-scope cell buys no L3 claim) and 2.2 (a count movement is not a posture movement) because all three are the same family: things a green check does not tell you. Named as the finding most worth keeping by the ASVS-11.7.1-summary session, which is also the session that independently hit it.
… a consult item for 5.4.3's premise
Groundwork, not a finished feature: the route, page and settings are not in this commit.
Filing a new ledger item here, not building one -- hence no claim.
1. THE PREDICATE (messagefoundry_webconsole/_external.py). Pure -- no settings import,
no request, no I/O. What decides whether an interstitial appears is the part that
has to be right, so it is isolated and tested on its own.
The test is a DECLARED DOMAIN LIST, not same-origin, because ASVS says "outside the
application's CONTROL" and control is organisational: an operator's own AD FS is a
different host, a different origin, and squarely inside their control.
Two details the research named as the ways a naive version is WORSE than none:
* A suffix test without a label boundary is a hole -- `evilhospital.example` ends
with `hospital.example`. Matched on a dot boundary instead.
* The displayed host must be what the browser RESOLVES. A Cyrillic homograph
renders identically to the Latin form, so showing decoded Unicode shows the
operator a lie. We display punycode and flag when the two differ.
Secure-by-default in three places: an empty domain list makes every absolute http(s)
URL external; an unparseable host is external; a non-navigable scheme is external.
Failing toward SHOWING the interstitial is the safe direction.
14 tests. The two that matter are the lookalike and the homograph; two reach
controls exist so a predicate that fires on everything cannot pass as working.
2. A NEW LEDGER ITEM (number from alloc.ps1, never grepped) recording a consult we owe
ourselves: 5.4.3's `na` rests on antivirus being enterprise-provided, and this tests
that premise where it is least likely to hold -- files the engine PULLS over
SFTP/FTPS from outside the domain, where there is no inbound delivery event for a
perimeter scanner and on-access coverage depends on whether the integration host's
working directory is watched at all.
USED THE REAL CUSTOMER DOMAIN IN THE FIRST DRAFT AND CAUGHT IT BEFORE COMMITTING.
Fixture domains are now RFC 2606 `.example`. Worth recording: the `forbidden-content`
leak guard PASSED on that draft, so it does not cover customer DOMAINS -- only the
manual diff scan caught it. That is a gap in the gate, not a near-miss to feel relieved
about.
…th a cancel, before an off-site hop Completes the control the predicate in 8552edc only decided. Default-on; the escape is opt-in, named at startup, and documented as lowering security. THE SHAPE, and why the GET/POST split is the interesting part. `GET /ui/oidc/start` used to mint a PKCE flow and 303 to the IdP in one step. It now renders the interstitial and stages NOTHING; the flow starts only when the operator confirms via POST. Three things fall out: * The destination is never carried by the page -- only the HOST is shown, and the POST posts back to us with no URL. An interstitial that carried its target would itself be an open redirect, which is strictly worse than having none. * The bounded flow cache REJECTS when full, so a GET that minted a flow was a login-DoS lever. It no longer is. * The confirm is a POST behind the console's Sec-Fetch-Site check, closing the standing hole where any external page could begin a federated sign-in by linking to the start leg. INTERNAL IS INTERNAL. ASVS asks about destinations outside the application's CONTROL, and control is organisational, not topological -- an operator's own AD FS is a different host, a different origin, and squarely theirs. So `organization_domains` decides, matched on a LABEL boundary. Empty is the STRICT position: every absolute destination is interstitialed, including your own IdP. SEAM v17. The policy reaches the route as CONFIG on UiDeps, for the same reason `oidc_enabled` does: `create_managed_app` attaches the AuthService inside the lifespan, long after `mount_ui` has fixed the route table. Console SUPPORTED_ENGINE_SEAMS and the golden snapshot move with it; the golden diff was reviewed before regenerating and is exactly the four fields plus the bump. THREE GATES CAUGHT REAL THINGS AND ALL THREE WERE FIXED PROPERLY, NOT SUPPRESSED: * The rate-limit doc gate reads throttle shapes by walking DECORATED route functions. My first draft hoisted the start body into a plain helper -- limiter still working, gate blind to it. Restructured so the decorated POST holds the branch, with a comment saying why it must stay there. * The 8.1.3/8.1.4 contextual-input inventory flagged `external_link_allowlist`. It decides whether a NOTIFICATION is shown, not whether a request is authorized, so it is recorded in the reviewed-non-inputs set WITH that reasoning -- it is still a security-relevant setting, and those are different questions. * `test_serve_loopback_emits_no_new_stderr` caught a start-time note that fired on every stock boot. Removed: start-time output is for a posture WEAKER than the default, never for the default itself. Startup warnings name allowlist entries INDIVIDUALLY, never a count -- "3 destinations exempted" is the shape of message that lets an entry nobody intended sit for a year. 9 route tests on top of the 14 predicate tests. The load-bearing one asserts the page carries no destination URL to post back; the negative-branch tests (internal IdP, interstitial off, allowlisted) genuinely do NOT render it, so the positives are not passing vacuously. Settings documented in CONFIGURATION.md, including that declaring your domains is the correct fix for an over-eager interstitial and the allowlist is the waiver.
…enings Caught by `test_every_security_bool_at_its_insecure_value_is_reported`, which iterates SecuritySettings.model_fields and fails on any security bool that can sit at its insecure value without security_loosenings() naming it. `external_link_interstitial` was exactly that: an operator could turn the ASVS 3.7.3 control OFF and GET /security/posture would not say so. That completeness floor is the right shape and it worked -- a new switch cannot be added without either being reported or being explicitly exempted with a reason. Also registers `external_link_allowlist`, which the floor does NOT pin because it is a LIST rather than a bool. Left unregistered it would have been an unreported loosening by omission, and the floor would never have said a word. Entries are named individually, not counted: "3 destinations are exempt" tells a posture review nothing about WHICH, and that is how an entry nobody intended survives one. Two suite failures remain and neither is mine: test_version and test_gate_installed_parity compare the venv's INSTALLED metadata (0.3.0) against the source __version__ (0.3.2). A stale editable install in a venv shared by several sessions -- not repaired here, because reinstalling into it would change the environment under them.
…luation-03eb26 # Conflicts: # docs/ASVS-ASSESSMENT-METHOD.md
…after the record said `na` Same coupling that bit 11.7.1 and the same fix. The rubric teaches by worked example, so a stale example does not merely sit there — it TEACHES the superseded verdict to the next assessor, in the one document written to stop assessors disagreeing. Rewritten as the rule-1 case it now is, and the old reasoning is KEPT rather than deleted: "a scan hook exists but its only shipped implementation is _no_scan and there is no configuration key" is still TRUE of the code. It answers rule 3's question, and rule 1 runs first. An example that just swaps its verdict teaches nothing about why. Also records, in the rubric itself, that this `na` is WEAKER than 11.7.1's: the engine ships a scan seam, so this is a control the product COULD implement, which makes the verdict conditional on the enterprise actually covering those ingest paths. A reader who takes this row as a template for the next platform-delegated cell needs that qualifier, not just the verdict. Found while resolving the merge with main, not by looking for it.
wshallwshall
enabled auto-merge (squash)
August 3, 2026 02:15
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
wshallwshall
disabled auto-merge
August 3, 2026 02:48
… of closing it
Two defects in my own 3.7.3 build, and one false claim in its commit message. Found by
the coordinator reading the actual CI rather than my local subset.
1. ⛔ ASVS 3.5.1 — THE NEW POST ROUTE HAD NO ORIGIN GUARD. `099fdcd7`'s message said
"the confirm is a POST behind the console's Sec-Fetch-Site check". THAT WAS FALSE.
The leg checks Sec-Fetch-MODE (is this a navigation), not Sec-Fetch-SITE (did it come
from us) — and a cross-site <form method=post> IS a navigation, so it sailed through.
The split therefore RELOCATED the drive-by sign-in hole from GET to POST rather than
closing it: a foreign page could still mint a flow and bounce the operator to the IdP.
`assert_same_origin` is now the first statement of the POST handler. It precedes the
rate limiter deliberately and does not violate the module's "audit-writing branches
sit behind the limiter" rule — it raises 403 and writes no audit row.
The module docstring's "no same-origin assertion on either leg" carve-out was written
for the CALLBACK, where the IdP's redirect is legitimately cross-site. It never
applied to a start leg reached from our own page, and now says so.
2. ASVS 7.4.4 — `leaving_site()` suppressed the shared chrome with `nav=Markup("")`,
stranding a signed-in operator on a page with no sign-out. Its sibling interstitial
`oidc_landing()` uses `minimal_nav()` for exactly this reason; now so does this one.
3. THE TESTS COULD NOT HAVE CAUGHT EITHER. All nine sent NO Sec-Fetch headers, so the
guard would never have fired — they would have passed just as happily with it
deleted. Three added that supply the header a real browser sends, including a REACH
CONTROL so a guard that 403s everything cannot pass.
⭐ AND WRITING THOSE TESTS FALSIFIED A SECOND CLAIM OF MINE. A cross-site GET with an
EXTERNAL IdP renders the interstitial (200) and is NOT refused — correctly, because that
page stages nothing. But with an INTERNAL IdP the GET delegates straight to the minting
leg, so FOR INTERNAL-IDP DEPLOYMENTS THE GET STILL MINTS A FLOW, exactly as before this
change. The bounded-flow-cache DoS lever is closed for external IdPs and merely
origin-guarded for internal ones. `099fdcd7` claimed the split closed it outright; it
did not. Both behaviours are now asserted so the asymmetry cannot be forgotten.
Note for the record: the coordinator's cited test paths (test_ui_origin_guard.py,
test_ui_logout_affordance.py) exist in neither this branch nor main, and neither
assertion message appears anywhere in tests/. The FINDINGS were right and I verified
each against the code directly; the file references were not.
…mpared the two The through-line of this whole session, filed so it outlives it: the ASVS risk-acceptance register is UNGATED PROSE. No check has ever compared the cell ids in its signed sign-off blocks against the verdict of record, and a manual cross-check of all eight blocks found 29 entries that are not carried residuals -- 22 of them `unverified`, i.e. a signed acceptance of a risk nobody ever assessed, present in every single block.⚠️ Filed with the count explicitly marked NOT INDEPENDENTLY CONFIRMED. It is one measurement by one session, and the gate is precisely what would reproduce it, so the item says to treat the number as the REASON to build the check and never as an established fact. Caveat raised by the coordinator and it is the right one -- a finding about unverified claims should not itself be an unverified claim. Design notes carry the three lessons this session paid for: print what you scanned (a gate whose regex stopped matching is indistinguishable from a clean one); prove it red in both directions before trusting green; and REPORT, never auto-correct -- the lists sit inside SIGNED acceptances and rewriting signed content to satisfy a checker is a worse defect than the drift. ⛔ Filed, NOT built. A new security-doc gate can block merges, so it is the owner's green-light like any enforcing control. Number from alloc.ps1. Explicitly out of scope for the item, and stated in it: what the 2026-07-14 signature actually covered. That is not a fact an agent can recover -- it is a question about what a human agreed to, and only the signer can answer it.
…flow — and a correction I owe
RETRACTION FIRST, because it is committed and pushed. `27499a14`'s message says the
coordinator's cited test paths "exist in neither this branch nor main, and neither
assertion message appears anywhere in tests/". THAT IS FALSE. Both files exist:
packaging/messagefoundry-webconsole/tests/test_ui_origin_guard.py
packaging/messagefoundry-webconsole/tests/test_ui_logout_affordance.py
Both assertion messages are in them. Its citation was the full path straight from the
CI log and was correct in substance AND provenance. I grepped `tests/` -- the engine
dir -- and concluded from its absence there. I made that error inside the very message
correcting the coordinator for a provenance error, which is the sharpest possible
demonstration of the rule I was invoking: VERIFY BEATS INFER, including when you are
the one doing the correcting.
THE GAP THAT CAUSED IT, and it invalidates several "full suite green" claims I made
tonight: pyproject's `testpaths = ["tests"]` means a bare `pytest` NEVER RUNS the
webconsole package's own suite -- 14 files, 344 tests. My 10,301-passed runs were
missing all of them. So the local/CI gap is not "those tests are CI-only"; it is that
my invocation silently excluded a whole package.
Running them proved the 27499a1 fixes rather than merely arguing for them: 21 pass on
the two files that were failing.
THE THIRD REGRESSION WAS ALSO REAL, and I had denied it for the same wrong reason.
`test_oidc_start_redirects_to_the_idp_and_sets_the_flow_cookie` DOES exist and did
assert 303 on the GET. Three tests updated to the intended design rather than to
whatever made them pass:
* the redirect/PKCE/cookie test now asserts the GET renders the interstitial and sets
NO cookie, then POSTs for the 303 -- the split is asserted, not stepped around.
* the AC-8 availability test targets the POST, because "still reaches the IdP" is a
property of the leg that attempts.
* the full round trip TRAVERSES the interstitial (GET 200 -> POST 303) rather than
shortcutting to the POST. Its value is walking the path a browser walks.
344 passed, 3 skipped across the whole webconsole package.
wshallwshall
added a commit
that referenced
this pull request
Aug 4, 2026
…edirect assertion actually assert it
CodeQL flagged `assert "login.microsoftonline.com" in body` as incomplete URL
substring sanitization. It is a false positive on the SINK — that line asserts the
interstitial RENDERS a host, it is not a redirect gate — and the coordinator confirmed
the real question independently: production has no `X in url` check anywhere in
routes/oidc.py, so no open-redirect exists.
Restructured rather than suppressed, because the rule was pointing at something true
about the TEST even though it was wrong about the risk: `host in page` passes if the
host appears ANYWHERE — a stray comment, an unrelated attribute — which is the same
laxness the rule exists to warn about. Now anchored to the element that renders it:
`<code>host</code>`. Stronger assertion, no sink.
Also strengthened the one that matters most in this file. The open-redirect test used
to assert a string was ABSENT ("the IdP URL does not appear") plus a substring match on
the form action. A string-absence check passes for the wrong reason the moment the
markup changes shape. It now PARSES the Continue form's action and asserts the actual
property: no scheme, no netloc, exact path, empty query — there is nothing in it for an
attacker to steer.
Proved it can go red in both directions before trusting it: an absolute action and a
destination smuggled into the query are each rejected, current markup passes.
⛔ Not suppressed deliberately. A suppression is a claim someone reads later and takes
on trust; a standing ignored red is the "green gate that lies" pattern this session has
spent itself unpicking. Restructuring cost one line more and leaves nothing to trust.
Cherry-picked here to land it at last, and to clear the CodeQL alert this PR's own
hostname substitution surfaced. The substring shape at line 67 has been in main all
along; changing that line re-attributed the pre-existing "Incomplete URL substring
sanitization" alert to this PR as a new high-severity finding. The real fix already
existed on this commit, so applying it resolves both.
Conflict resolution: took this commit's structure (element-anchored assertion and the
parsed form action) and kept the reserved `idp.example.com` host from the earlier commit
in this PR, so the vendor endpoint does not come back.
Recovered work: authored ~9 hours before PR #156 merged but dropped by a push boundary,
not a review decision -- #156's merged head 9f85b46 is this commit's parent. When found
it was reachable from a single local branch ref with no remote copy.
wshallwshall
added a commit
that referenced
this pull request
Aug 4, 2026
…ng) + coordination-fact corrections (#164) * test(ui): use a reserved IdP host in the interstitial tests, not a real vendor endpoint `login.microsoftonline.com` was the only real IdP hostname anywhere in the repo -- no accounts.google.com, no *.okta.com, no auth0.com, no onelogin, no pingidentity. That singling-out is what makes it read as a hint about the deployment's actual identity provider rather than as generic test data, and no customer or site operational detail belongs in a public repo. `idp.example.com` is already this repo's OIDC placeholder for this feature (tests/test_settings.py configures the whole block with it), so this makes the interstitial tests consistent with the settings tests rather than inventing a convention. Behaviour-preserving, and measured rather than assumed: the external-link classifier special-cases no host at all -- no allowlist, no example.com handling, no loopback or same-origin shortcut -- so an RFC 2606 reserved name classifies exactly as the real hostname did. Both files: 27 passed. Also drops the vendor name from one docstring; the sentence's point was that the IdP is trusted and is not the hospital, which does not need the vendor to make it. * chore(coord): drop the Claude plan-usage tooling — internal to how we work, not product This repo is a healthcare integration engine. Tooling and documentation about the maintainer's own Anthropic plan consumption (5-hour / weekly pools, per-model weekly buckets, a statusLine collector) is internal development process, not something an open-source integration engine should carry. Removed set is exactly the inverse of one commit: 41a8c49 added these same five paths (three scripts, one test, one 52-line doc section) and nothing else, so this is a clean excision rather than untangling. The subsystem was also dead. The statusLine executor is only reached from the terminal TUI's render tree, so it never runs under the desktop app -- its heartbeat has never been written once and latest.json has never existed, which means the reader could only ever return UNKNOWN. What shipped was a tool that does not run, documented with a boundary claim ("never runs under claude -p or the SDK") that is false: the boundary is the terminal TUI, and the desktop app is a third category the docs never named. tests/test_coord_usage.py is deleted in THIS commit, not a later one. Its skip gate is `pwsh missing or os.name != "nt"`, so it skips on ubuntu but RUNS on both Windows legs, where it hard-asserts returncode == 0 with no existence check on the script paths. Deleting the scripts without the test would red two required contexts -- and with auto-merge armed and strict branch protection, a red required leg blocks every open PR in the repo, not just this one. docs/WORKTREES.md keeps everything else: the section is excised in place rather than the file moved or ignored, because ~12 files point readers at it and its PR-triage material is the best contributor documentation here. 728 -> 676 lines. Nothing that other sessions depend on is touched: alloc, claim, presence, occupancy, lock, overlap, session-registry, install-coordination and install-git-hooks all stay. install-coordination.ps1 wires three hook scripts and never the statusLine key, so the coordination bundle is unaffected. Verified: no residual reference to any removed path anywhere in the tree. The material is not lost -- `git show 41a8c49:<path>` retrieves any of it. * fix(coord): two load-bearing coordination facts were wrong — correct them at every site Both were inferences recorded as measurements in the doc other sessions treat as the source of record, and both taught the wrong lesson. 1. "Hooks cannot call MCP" is FALSE. `type: "mcp_tool"` is a documented hook handler on every hook event, with the tool's output treated like command-hook stdout. Probed 2026-08-03: a command control in the same UserPromptSubmit array fired and reached the model, while an mcp_tool naming ccd_session_mgmt AND one naming a deliberately nonexistent server both produced nothing — and with no MCP server connected on this box, "not surfaced", "not addressable" and "errored invisibly" are the same bytes. Recorded as UNTESTED, not impossible, with the re-run condition named. If it works, announce collapses to one hook entry. 2. "A registry id passed to send_message fails silently" is FALSE — it errors loudly. Measured by calling send_message with a valid id belonging to no session: `Session <id> not found.`, nothing delivered. The namespaces carry different UUIDs by construction, so that is the path a registry id takes. Getting the id wrong is self-announcing; the doc had every session braced for a failure mode that does not occur. Fixed at all four sites, including the instruction text the hook prints into every session — the one that was actively teaching it. tests/test_announce_hook.py + test_announce_wiring.py: 59 passed. * docs(drift): announce-on-join is LIVE — the status table was a claim about the past The row said "MERGED, NOT INSTALLED — inert by accident. It has never run." Re-established 2026-08-03 by both methods the section itself prescribes: `install-coordination.ps1 -Status` reports UserPromptSubmit -> announce-session.ps1 INSTALLED, and the state dir holds 47 markers / 28 receipt files / 23 delivery logs, newest line `out=ANNOUNCED peers=3 reach=3 sent=3 checks=2 ms=616`. Rewritten as a two-column table — the defect on 2026-08-02, the re-verification on 2026-08-03 — so the incident and its lesson survive instead of being deleted along with the stale status. The lesson is the valuable half: merging a hook does not install one, and the missing-script notice built to catch that class lives inside the shim, so it cannot fire when the shim is not wired at all. Also corrects the interval: ~13 hours after #133 merged, not "weeks", and adds the closing rule that a row not re-established by receipt is a claim about the past, not a status. * test(console): retire the CodeQL substring sink — and make the open-redirect assertion actually assert it CodeQL flagged `assert "login.microsoftonline.com" in body` as incomplete URL substring sanitization. It is a false positive on the SINK — that line asserts the interstitial RENDERS a host, it is not a redirect gate — and the coordinator confirmed the real question independently: production has no `X in url` check anywhere in routes/oidc.py, so no open-redirect exists. Restructured rather than suppressed, because the rule was pointing at something true about the TEST even though it was wrong about the risk: `host in page` passes if the host appears ANYWHERE — a stray comment, an unrelated attribute — which is the same laxness the rule exists to warn about. Now anchored to the element that renders it: `<code>host</code>`. Stronger assertion, no sink. Also strengthened the one that matters most in this file. The open-redirect test used to assert a string was ABSENT ("the IdP URL does not appear") plus a substring match on the form action. A string-absence check passes for the wrong reason the moment the markup changes shape. It now PARSES the Continue form's action and asserts the actual property: no scheme, no netloc, exact path, empty query — there is nothing in it for an attacker to steer. Proved it can go red in both directions before trusting it: an absolute action and a destination smuggled into the query are each rejected, current markup passes. ⛔ Not suppressed deliberately. A suppression is a claim someone reads later and takes on trust; a standing ignored red is the "green gate that lies" pattern this session has spent itself unpicking. Restructuring cost one line more and leaves nothing to trust. Cherry-picked here to land it at last, and to clear the CodeQL alert this PR's own hostname substitution surfaced. The substring shape at line 67 has been in main all along; changing that line re-attributed the pre-existing "Incomplete URL substring sanitization" alert to this PR as a new high-severity finding. The real fix already existed on this commit, so applying it resolves both. Conflict resolution: took this commit's structure (element-anchored assertion and the parsed form action) and kept the reserved `idp.example.com` host from the earlier commit in this PR, so the vendor endpoint does not come back. Recovered work: authored ~9 hours before PR #156 merged but dropped by a push boundary, not a review decision -- #156's merged head 9f85b46 is this commit's parent. When found it was reachable from a single local branch ref with no remote copy.
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
2 participants
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.
docs/Secure_Development_Standards.mdonmainright now publishes:The ASVS survey is at 35.9% examined (124/345), 221 cells unverified. "Verified against ASVS 5.0 Level 3" is not supportable, and it is a published security attestation in a public repository.
b94cc832corrects it to "assessed against OWASP ASVS 5.0 using Level 3 as the target" and adds a dated note recording that the previous wording was wrong and why.That correction was written but never pushed. Had the worktree been lost, the fix would have gone with it and the false claim would have stayed live. That alone justified pushing the branch.
What is here
b94cc832d3eb4e57e4a27c4ba2a50f3d7b41ec0ba5e7b179Touches
scripts/asvs/scorecard.py(+61) andtests/test_asvs_scorecard.py(+83), so this is not docs-only.Why it also matters for 11.7.1
The owner has ruled 11.7.1 is
na. The rubric's Rule 1 delegates applicability to §2, andmain's §2 does not declare a platform-substrate boundary — so onmainas it stands, Rule 1 cannot delivernaandASSESSMENT-METHOD.md:65still readsfailunder Rule 3.b94cc832adds exactly that boundary. Declare the boundary, then apply the rule — this must land before any 11.7.1 verdict sweep, or the sweep enumerates edits against a tree that lacks the justification.Verification done by the coordinator
main: 8 files, 320 insertions, 8 deletions — nothing reverted. (Two-dot reads 1,306 deletions; that is the pre-squash staleness artifact documented in docs(worktrees): the pre-squash merge-base trap — corrected, the numbers were real but the pairing was not #149, not a proposed change.)git merge-tree --write-treeagainstmain: clean.main— this was not already landed.Not verified by me
The
featcommit's gate behaviour, the ADR 0152 changes, and the PHI/CONFIGURATION edits. Review before arming.🤖 Generated with Claude Code