Skip to content

sec: ASVS packet G -- authorization and data protection (BACKLOG #1151, #1153, #1154, #1184, #1186, #1187, #1188) - #944

Merged
wshallwshall merged 13 commits into
mainfrom
claude/asvs-phi-brief-03d670
Sep 7, 2026
Merged

sec: ASVS packet G -- authorization and data protection (BACKLOG #1151, #1153, #1154, #1184, #1186, #1187, #1188)#944
wshallwshall merged 13 commits into
mainfrom
claude/asvs-phi-brief-03d670

Conversation

@wshallwshall

@wshallwshall wshallwshall commented Sep 6, 2026

Copy link
Copy Markdown
Collaborator

ASVS packet G, authorization and data protection. Nine rows assigned; seven worked, two belong to other open PRs.

Every row stays OPEN. Each carries Closing-act: scorecard-rescore, which lives in the separate vault repository, so no engine commit can close one. Verified with parse_items rather than a hand-rolled scan.

Every finding here is written in the conditional. There are zero deployed instances, so nothing below is a live exposure.

What landed

Row Cell Change
#1151 8.1.1 docs/SECURITY.md said "Monitoring dashboards stay global"; the code narrows all five monitoring reads for a scoped caller. Corrected to state both halves, plus GET /metrics as the exception in the other direction. Derived by a guard that executes each route.
#1153 8.2.4 Pinned the six classes in the "Attributes not consumed at this release" paragraph, so narrowing it reds. Repaired the item's measured guard defect: the absence pattern could not see the engine's own time-window evaluator.
#1154 8.3.2 The two AD map editors revoked nothing while every sibling authorization-value mutator did. Both now revoke directory sessions and audit the count.
#1184 14.2.1 Titled defect no longer reproduces. Corrected the row's stale banner rather than manufacturing work.
#1186 14.2.4 docs/PHI.md said nothing is written to browser storage; the console deliberately writes localStorage column preferences, and _auth.py is written to preserve them. Corrected and bound two ways.
#1187 14.2.6 metadata returned complete on every list row beside a masked summary. Now masked until revealed.
#1188 14.2.7 docs/PHI.md section 2's legend promises every residual tier is listed in section 8. It was false in two places, not the one the row names. Both fixed, with a guard binding the promise in both directions.

Not in this PR

A ledger defect on main, found here and NOT fixed here

Merging origin/main surfaced this, and it needs its own change by whoever holds the number.

Item #1147 no longer parses as a backlog item on main. Measured with parse_items: origin/main has 442 items, my pre-merge tree had 443, and #1147 is the entire difference. Commit 642225f78 (PR #855, about #1146) deleted the ## 1147. heading line and put #1146's re-verification prose in its place, so #1147's body is now orphaned under #1146. It is not in the archive either — a positive control in the same run matched 237 headings there, so the pattern works.

This PR adopts main's state rather than fixing it: restoring the heading belongs in its own commit, and the ledger gate names repo-security-review-afbae3 / claude/asvs-scoring-status-9bcb93 as holding that number.

Verification

ruff format --check and ruff check clean. mypy strict clean over messagefoundry (268 files).

CI on 12202224f before the last two commits: 1 failed, 11880 passed, 846 skipped — the one failure was mine and is fixed in dd69113d6 (two new doc-drift guards were unclassified in test_tooling_partition.py; both are engine-subject, so they go in _STAYS_WITHOUT_IMPORTING, not the tooling manifest).

Every new guard was mutation-tested and each mutation is recorded in its commit message. Two mutation harnesses were themselves wrong on the first attempt and are documented rather than quietly fixed:

  • the first #1188 mutation deleted the section 2 row along with the section 8 row, removing the obligation and the evidence together, so the test passed and looked like a working control
  • an early duplicate-guard keyed on a 40-character prefix shared with six sibling banners, so it fired a false positive

Read the hosted-runner legs before mergingwindows-service-smoke and the server-DB jobs never run in a Builder's environment.

🤖 Generated with Claude Code

The one red is BACKLOG #1304, not this branch

repo harness tests (windows-2025) is red. Recorded here rather than silently re-rolled, because #1304 asks for exactly that: "DO NOT rerun until green without recording that you did: a manufactured green and an earned one are indistinguishable afterwards."

Cite the reason, not the test name — the name changes every time. All three recent runs fail with the identical non-assertion reason, raised inside except subprocess.TimeoutExpired:

AssertionError: PWSH LAUNCH TIMED OUT after 45s (BACKLOG #1304).
This is a PROCESS LAUNCH that never returned. It is NOT an assertion failure and NOT
evidence that the gate's behaviour changed: no gate logic ran.
Run Commit Failing test Reason
34048598814 main 744a7a434 test_the_launch_timeout_diagnostic_does_not_fire_on_an_ordinary_denial pwsh launch timeout
34060631827 main 172b1327c test_the_ordering_switch_did_not_leak_into_rules_3_and_3d, test_gits_own_dash_C_path_argument_does_not_become_a_false_deny pwsh launch timeout
34061247588 this branch b2dc3a3fa test_a_dash_C_must_not_shadow_the_git_dir_that_actually_decides pwsh launch timeout

Four distinct test names, three runs, one mode. The gate script never launched, so repository content cannot have influenced the outcome.

It is not "inherited from main" either, and that phrasing would send a reader hunting for a main-side breakage that does not exist. Main oscillates with no code change in the window: 744a7a434 red, 642225f78 green, 172b1327c red, ebdfa44a6 green. #1304 records the same chronology from 2026-08-21. It is a filed, load-correlated infrastructure flake that hits main and PR branches alike.

Every mechanism by which this diff could reach that job was attacked and failed. The job runs -m tooling off tests/tooling_manifest.txt (153 entries) and none of the nine test files changed here is in it — so the two new files cannot have shifted the --dist loadfile distribution. Collection counts are identical at 3345 on both main and this branch. This branch's run was the fastest of the three (1242s vs 1290s and 1274s), refuting a load-from-bloat mechanism. Locally, all 223 tests in both gate files pass on this branch.

PRs 881 and 841 touch those same gate files but are both OPEN and DIRTY, so they are neither a repair in flight nor a cause — only a future conflict risk.

If it reds again on a different gate test name with the same launch-timeout reason, that is confirmation, not escalation.

wshallwshall and others added 7 commits September 6, 2026 13:09
…BACKLOG #1151)

BACKLOG #1151's 2026-08-20 pass names "the monitoring-scope correction, stating
BOTH halves" as its first build subject. This is that subject.

docs/SECURITY.md's per-channel-scoping blockquote -- the public statement of the
engine's data-specific access rule, which is what ASVS 8.1.1 grades -- asserted
"Monitoring dashboards stay global." The code outgrew that sentence. Measured by
executing the API, not by reading it: for a channel-scoped caller list_channels,
list_connections, list_events, graph_edges and active_alerts are all narrowed,
and the shared outbound is suppressed outright rather than relabelled -- its
dashboard row, its graph node and its live status all disappear. So the doc
claimed a caller sees MORE than the code gives them.

The correction states both halves rather than deleting the false one, which
would have removed the only public statement about monitoring-plane scoping:

* narrowed:  GET /channels, /connections, /events, /graph/edges, /alerts/active,
             plus 403 on /connections/{name}/events and /connections/{name}/metadata
* global:    GET /stats, GET /metrics/history, and the outbox_by_status field of
             the /ws/stats frame -- aggregate counters with no connection identity.
             Its sibling connections_html field IS scoped, so one frame carries
             both rules.
* neither:   GET /metrics. The Prometheus exposition is keyed by connection and
             destination and is NOT narrowed, so a first deployment would let any
             monitoring:read holder read every connection's series regardless of
             scope. Stated rather than glossed, tracked as #1152. Writing only
             "aggregate counts stay global" would have traded one false sentence
             for a quieter one.

The blockquote's four-permission enumeration becomes an "at least" form with the
rule named (SDS-3.6): the scoped set is whichever route narrows on
Identity.can_access_channel.

tests/test_monitoring_scope_doc_drift.py keeps it true. The narrowed and global
route sets are module constants checked against BOTH the code and the doc in one
run: an executed request per route with an unscoped operator in the same run as
the positive control, then an exact backticked-token comparison against the
blockquote. Eight planted-mutation controls -- the retired sentence re-asserted,
and each classified route deleted in turn -- red the guard, and the whole file
reds against the pre-fix doc.

A name-presence scan was rejected as the instrument after measuring it: over the
live app it misses _control_guard (the connections:control routes enforce the
scope through a helper, so their bodies name no primitive) and it misses the
per-property rule inside ws_stats.

PROVENANCE: this work was built on 2026-08-22 as commit e0cdcebf7 and never
reached a remote -- it sat on a local branch. Cherry-picked here unchanged and
re-verified at 744a7a4: the guard's 11 tests pass against the current tree.

The row STAYS OPEN. Its closing act is a scorecard re-score, which lives in the
separate vault repository and no engine commit can perform. This lands the first
of the item's named build subjects; the per-route data-rule derivation over both
route modules, the bidirectional extra-constraints comparison and the rest of the
item's list remain unallocated.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…(BACKLOG #1154)

ASVS 8.3.2 asks that changes to values an authorization decision reads apply
immediately. The AD group maps are such values: they resolve to role sets and to
channel scope, which is exactly what the decision reads.

Both setters wrote the map and revoked nothing. set_ad_group_scope_map's own
docstring said so in as many words -- "Takes effect on each AD user's next
login" -- while every sibling authorization-value mutator in the same file
already revoked: set_roles, set_channel_scope, custom-role update and delete,
disable, password reset. So an administrator narrowing a group's roles or its
channel scope would, on a first deployment, leave every already-running
directory session on the pre-edit mapping for as long as that session stayed
alive. Nothing alerted and nothing reverted, which is the requirement's
alternative arm, so neither arm was met.

Both setters now call _revoke_ad_sessions() and record the count in their audit
row, so an operator can see the blast radius of a map edit without
reconstructing it.

Scope, and why it is not narrower: the revoke covers AD-provider accounts and
not local ones. Working out which principals a given map edit actually affects
would mean re-binding to the directory, and both an added and a removed mapping
change an outcome, so the affected set is not derivable from the entries alone.
Local accounts read neither map, so they are left alone.

It is deliberately NOT counted against the reconciler's mass-revoke breaker.
That breaker exists to catch a directory the engine cannot read; this is an
administrator's own step-up-gated edit, and suppressing it would be the control
firing on the wrong event.

The test parametrises both setters and carries a LOCAL account as the control --
the half that makes it mean something, since a revoke-everything implementation
would satisfy the AD assertion just as well. It asserts both preconditions
before acting, so it cannot pass on an empty session table. Mutation executed:
replacing both _revoke_ad_sessions() calls with a literal zero reds both arms by
name. The two maps take different right-hand values, and the role map has a
foreign key onto the roles table, so the entry is parametrised too -- a channel
name there fails the constraint rather than the assertion, which is how the
first draft of this test failed.

Gates: ruff format + ruff check clean, mypy strict clean over messagefoundry
(268 files), 197 passed across test_ad_group_scope, test_ad_session_reconcile,
test_auth_service, test_api_auth, test_auth_session_lifecycle, test_step_up and
test_channel_rbac.

The row STAYS OPEN. Its closing act is a scorecard re-score in the separate
vault repository, which no engine commit can perform, and this lands one of the
item's four named builds. Still unallocated: the channel-scope re-diff in the
reconciler, requester re-validation at approval release, per-frame
authorization on the stats socket, re-resolution inside the streaming export
loop, and the bounded age on the extension's offline policy cache.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ACKLOG #1187)

ASVS 14.2.6 asks that complete data be masked in the interface unless the user
specifically views it. The reveal mechanism for that shipped under this row in
PR #514: authorization and reveal became two decisions, and `summary` is
display-masked on list surfaces and complete on the one route that reveals it.

`metadata` was left out, and field_authz.py said so in its own comment -- it
called the field "the obvious next candidate" and named the reason it mattered:
"masking one field while the same identifiers return complete one field over is
a partial control that reads as a whole one." PHI_FIELDS rates `metadata` on the
same view_summary tier as `summary`, for the same reason: it carries
ingest-derived MRN and patient-name PHI. So on a first deployment every list and
search surface would return that identifier complete on every row, beside a
masked summary, which is exactly the census-off-a-screen exposure the verb
describes.

The comment was an accurate description of the gap, so this closes the gap
rather than re-describing it.

`mask_for_display` reads the composed-summary grammar and `metadata` does not
follow it -- its values are code- and operator-attached and its mechanism is
documented as TBD. That is handled rather than overlooked: an unrecognized part
is masked WHOLE, so an unknown shape degrades to the fixed-width mask instead of
being passed through. A JSON blob therefore masks to nothing readable, which is
the correct direction for a field whose grammar is not yet fixed, and the detail
route's reveal still returns it complete.

Three tests, each pinning a different half:
* masked on a list surface, COMPLETE on a reveal -- the second assertion is the
  one that matters, because it is what keeps this a mask rather than a
  withholding and keeps the console able to do its job
* an unknown grammar fails closed, asserted directly because it is the case that
  decides whether extending the mask set to this field is safe at all
* the detail route's reveal set is compared to MASKED_UNTIL_REVEALED, read from
  the source rather than restated. Adding a property to the mask set and
  forgetting the reveal would hide it everywhere with no way to see it -- a
  product break no masking test would catch, since masking is what they all
  assert.

Two existing fail-closed tests now reveal `metadata` too. That is not a
weakening: each carries a comment saying it reveals deliberately so its positive
control observes the COMPLETE value, "because this test is about the gate" and a
masked value would leave it unable to tell a working gate from a working mask.
Extending the reveal keeps that stated design true as the mask set grows.

Mutations executed: dropping `metadata` from MASKED_UNTIL_REVEALED reds all
three new tests; dropping it from the route's reveal set reds the drift test by
name.

Gates: ruff format + ruff check clean, mypy strict clean over messagefoundry
(268 files), 97 passed across test_api, test_field_authz,
test_field_authz_fail_closed and test_content_search, and 422 passed + 3 skipped
across the full web console suite.

The row STAYS OPEN, and the larger half is untouched: the raw body still
materializes and renders on the detail page with no reveal step
(api/app.py sets `raw` unconditionally; the console renders it into a `pre`),
and the console has no reveal control at all. That is cross-cutting across the
API model, four console surfaces sharing one handler, and the audit
discriminator, and it is named here rather than folded in. The closing act is a
scorecard re-score in the separate vault repository, which no engine commit can
perform.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ys so (BACKLOG #1186)

ASVS 14.2.4 asks that the named control domains be implemented as defined in the
documentation for the specific data's protection level. BACKLOG #1186 records
the general shape of the failure: a sentence in the classification document
asserts a posture the shipped code refutes, drifts undetected because nothing
reads it, and gets corrected by hand with no guard left behind.

This is an instance, found and closed. docs/PHI.md's "No PHI in browser storage"
bullet said "Nothing is written to localStorage/sessionStorage/IndexedDB". Two
shipped artifacts refute it, and the second does so deliberately:

* messagefoundry_webconsole/static/app.js has read and written per-table column
  widths and visibility under an `mfcols:v2:` localStorage prefix since
  2026-07-07, keyed by pathname and table ordinal
* messagefoundry_webconsole/_auth.py sends `Clear-Site-Data: "cache"` and NOT
  `"storage"` on logout, its comment saying in as many words that `"storage"`
  would wipe the deliberately-persistent, PHI-free column preferences

So the document denied a behaviour another part of the product was written to
protect. Measured: no token of that sentence is read anywhere under tests/,
scripts/, messagefoundry/ or packaging/ -- which is why it could drift.

The correction keeps the PHI claim, which is true, and replaces the absolute
claim with what is actually stored plus the argument for why it is PHI-free. The
retraction is written in place rather than the old sentence being deleted, the
same way the #1184 retraction two bullets down is. The bullet also gets split:
it was carrying two unrelated assertions, storage and search-term-in-a-URL, in
one paragraph.

tests/test_browser_storage_doc_drift.py binds it, and binds the CLAIM rather
than the absence. A test asserting "the console writes no storage" would have to
be deleted the day a legitimate preference lands -- which is how the original
sentence became false in the first place. Instead the document names the
prefixes it permits and the guard compares that list to the source in BOTH
directions: an undeclared prefix reds, and a declared prefix nothing writes reds
too, so a permission cannot outlive its feature. The remaining absolute half of
the claim, sessionStorage and IndexedDB, stays absolute and stays measured. A
liveness receipt asserts the console source exists and names localStorage at
all, so none of the comparisons can pass vacuously.

Mutations executed: renaming the prefix in the doc reds both directions;
appending a sessionStorage read to app.js reds the forbidden-API arm by name.

Gates: ruff format + ruff check clean, mypy strict clean over messagefoundry
(268 files), 5 passed in the new guard plus 422 passed + 3 skipped across the
full web console suite.

The row STAYS OPEN. Its closing act is a scorecard re-score in the separate
vault repository, which no engine commit can perform, and this lands one
instance plus its binding. Still unallocated from the item's list: the retention
gate-behaviour doc binding, the generalised backend-posture claim guard, the IDE
protection-level inventory with its row-set guard, the operator-declared
volume-encryption setting, and the scope note on whether the IDE extension is
inside the scored posture.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…hat can see the clock (BACKLOG #1153)

ASVS 8.2.4 asks for adaptive controls on a consumer's environmental and
contextual attributes, applied at session start and during an existing session,
"as defined in the application's documentation".

That clause makes the documentation the measure, which makes NARROWING the
documentation the cheapest way to appear to pass: shorten the not-consumed list
and the shipped controls suddenly match what the application "defines". The item
names that as its first disqualified move, and nothing prevented it. This is the
mechanical defence, which the item's own research says does not exist today.

Two halves, and the second is the reason this is not a string comparison.

First: the six attribute classes in docs/SECURITY.md's "Attributes not consumed
at this release" paragraph are pinned one test each. Narrowing reds; widening
does not, which is the right asymmetry. A class leaves that list honestly only
when the control is BUILT, and that commit changes the tuple beside the code.

Second: the engine SHIPS a complete timezone-aware time-of-day and day-of-week
window evaluator -- ActiveWindow and Schedule in config/models.py, used to start
and stop connections on a calendar and to pace alert rules. BACKLOG #1153
measured that the recorded time-of-day absence pattern matched NONE of that
evaluator's spellings. An absence pattern that cannot match the thing where the
thing demonstrably exists returns a zero meaning "my pattern is wrong", and that
is indistinguishable from "the engine has no clock".

So the positive control asserts the evaluator exists and is findable BEFORE any
absence is asserted, and the absence is then asserted where it actually bears on
the verb: no authorization decision reads a time window. "Time of day is not
consumed" never meant the engine has no clock; it means no allow or deny reads
one. Measured both sides in one run: zero hits for those four symbols inside
messagefoundry/auth/, against eleven for the same symbols across pipeline/ and
config/.

Mutations executed: dropping geolocation from the paragraph reds that arm by
name; planting the four symbols in an auth module reds all four absence arms.

Also corrected, same subject, in __main__.py: the comment justifying the shipped
admin_new_ip_step_up default ended "preserving the ASVS 8.1.3/8.1.4/8.2.4 N/A
keystone". That asserts a grade the record does not carry -- this item's own
filed text records 8.2.4 as PARTIAL, not not-applicable. A source comment
claiming a cell is N/A is a false premise in a distributed artifact, where a
later assessor reads it as authority for a decision nobody made. The comment's
real reasons are kept and one measured reason is added: on the shipped loopback
bind the flip changes nothing at all, because _same_host folds 127.0.0.1 and ::1
into one host. No behaviour changes.

Gates: ruff format + ruff check clean, mypy strict clean over messagefoundry
(268 files), 11 passed in the new guard, 52 passed with test_security_doc_drift,
and 166 passed across test_admin_new_ip, test_cli, test_phi_logging_inventory
and test_security_posture_defaults.

The row STAYS OPEN, and deliberately so: it needs an owner ruling on three
questions before anything else can move -- whether this verb's "consumer" ranges
over ingest peers, whether the verb's parenthetical is a coverage floor or
exemplary, and which posture the shipped-default rule grades. Nothing here
touches those. Its closing act is a scorecard re-score in the separate vault
repository, which no engine commit can perform.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…e premise (BACKLOG #1151, #1153, #1154, #1184, #1186, #1187)

One banner per item, at the top of each blockquote. Every row STAYS OPEN: each
carries Closing-act: scorecard-rescore, which lives in the separate vault
repository and no engine commit can perform. Verified with parse_items rather
than a hand-rolled scan -- all six still read as open, and the status check
reports 680 items each declaring exactly one status.

Five banners record work that landed in this PR: the monitoring-scope correction
and its derived guard (#1151), the not-consumed-attributes pin with an
instrument that can see the engine's own window evaluator (#1153),
revoke-on-mutation parity for the two AD map editors (#1154), the browser-storage
correction and its two-way binding (#1186), and metadata masked until revealed
(#1187). Each names what is still unallocated, so the row's remainder does not
have to be re-derived.

The sixth is a correction, not a report. #1184's titled defect NO LONGER
REPRODUCES: no GET or HEAD route on either plane declares the PHI search needle,
all five surfaces the row requires to be priced are paid, and the guard that
keeps it passes at HEAD. Its scoring banner and "What holds it short today"
paragraph still describe the needle as bound, which is a record asserting an
exposure the code does not have -- the shape CLAUDE.md section 0 forbids. The new
banner says so first and points at what actually remains, none of which is this
row's titled defect. No engine work was manufactured for it.

Three of the nine rows in this packet are deliberately NOT touched here, and each
is reported in the PR body instead: #1152 and #1185 have their own open PRs (832
and 874), and #1188's claim is held by a stranded worktree, so its work was
parked rather than forced.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…s masked (BACKLOG #1187)

CI red on test (windows-2022, py3.14), and it was mine. Whose failure it was,
established before touching anything: that job PASSED on main at 744a7a4, the
exact commit this branch is based on, and failed here. Main's own red at that
commit is two different jobs (repo harness tests, CI gate), so this was not
inherited.

Three tests in tests/test_field_authz_metadata.py pinned the pre-masking
behaviour. They grade the AUTHORIZATION gate (SEC-006 / CWE-213), and each
carries a holder assertion as its positive control: a non-holder gets None, a
holder gets the value. Adding metadata to MASKED_UNTIL_REVEALED means a holder's
un-revealed read now returns a MASK, which is also non-None -- so the controls
stopped distinguishing "the gate released" from "the mask ran".

Fixed the same way the two fail-closed tests in the parent commit were: the
holder controls pass `revealed`, so they observe the complete value and keep
grading the gate. Every non-holder assertion is untouched, because masking
changes nothing for a caller who was never authorized.

Two assertions ADDED rather than adjusted, so this ends up stronger than what it
replaced:

* the masked state is now asserted explicitly on the summary tier -- not None
  and not the complete value -- because it is a third outcome distinct from both
  released and withheld, and nothing named it before
* count_exposed must return zero for a MASKED metadata value. That is the
  substantive half. The counter skips masked properties by design, which is what
  makes it mean "PHI actually put on screen" rather than "PHI the caller could
  have asked for". Without this line the counter would over-report every list
  surface, and an operator reading the PHI-exposure audit could not tell a page
  of masks from a page of identifiers.

Mutation executed: dropping metadata from MASKED_UNTIL_REVEALED now reds this
file in two places, naming the unrevealed-complete-value case. Before this
commit that mutation left the file green.

HOW IT WAS MISSED, since the mechanism matters more than the fix: the parent
commit ran the suites judged relevant by subject and never grepped for the
changed SYMBOL. A file named for the exact field being changed was one directory
listing away. The sweep is now done properly -- every test file referencing
redact_unauthorized, MASKED_UNTIL_REVEALED, count_exposed, mask_for_display,
PHI_FIELDS or either AD map setter was enumerated and run: 130 passed, 149
skipped (the skips are server-DB suites CI runs in dedicated jobs). The one
skipped metadata assertion, in test_postgres_store.py, reads the store directly
and references redact_unauthorized zero times, so masking sits above it.

Gates: ruff format + ruff check clean, 130 passed across every suite touching the
changed symbols.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@github-actions github-actions Bot added the ci-red A required check went red. Attribute it before retrying. label Sep 6, 2026
wshallwshall and others added 4 commits September 6, 2026 16:10
…t (BACKLOG #1153, #1186)

CI red, one failure across all three test legs, and it was mine:
test_tooling_partition.py::test_every_non_engine_test_is_classified named
test_adaptive_attributes_doc_drift.py and test_browser_storage_doc_drift.py as
unclassified. Neither imports the engine, so both fall to that guard, and it is
working exactly as designed -- a new non-engine test must be placed
deliberately.

Both go in _STAYS_WITHOUT_IMPORTING rather than the tooling manifest, because
both are engine-SUBJECT: they read shipped source off disk and assert about it.

* test_adaptive_attributes_doc_drift reads messagefoundry/config/models.py (the
  ActiveWindow/Schedule evaluator, as its positive control) and rglobs
  messagefoundry/auth/**.py (the absence assertion). The absence arm is the
  reason it cannot be tooling: what it guards against is a time-window read
  appearing inside auth/, which arrives as an ENGINE diff and does not trip the
  tooling job's path gate.
* test_browser_storage_doc_drift reads messagefoundry_webconsole/static/app.js.
  That is shipped product source by this file's own stated rule -- the
  _webconsole arm of _ENGINE_IMPORT exists to say the console counts as the
  product -- so its subject is engine source even though it imports nothing. It
  fires when the console writes a storage prefix docs/PHI.md does not name,
  which arrives as a console diff and does not trip the tooling gate either.

Listed as tooling, each would be deselected on the engine legs AND unreached by
the tooling job's path gate, so the change that breaks it would face nothing --
the "gate resting on a false premise" failure this file's own docstring names,
and the reason its rule is "when ambiguous, leave it off" the manifest.

WHAT THIS TEST DOES AND DOES NOT VERIFY, stated because it matters for review:
it checks that a file is classified SOMEWHERE, not that the bucket is right.
Both buckets turn it green. The placement argument above is a judgment no guard
checks, which is why each entry carries its reason inline in the house style --
a reviewer can check the claim, and that is the whole point of the list.

The third new guard, test_monitoring_scope_doc_drift.py, needs no entry: it
executes the API and so imports the engine, which classifies it already.

CI evidence for the rest of the tree, from the same run this fixes
(test (ubuntu-latest, py3.14) on 1220222): 1 failed, 11880 passed, 846
skipped. The one failure is this. Notably test_connscale_smoke, which failed
once on my contended local box, PASSED on all three CI platforms -- I have no
confirmed cause for the local failure and am not asserting one.

Gates: ruff format + ruff check clean, test_tooling_partition 11 passed.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… a guard says so (BACKLOG #1188)

ASVS 14.2.7 grades retention against a data classification, so the classification
document is the thing measured. docs/PHI.md section 2's vocabulary legend makes a
completeness claim about itself: the `UNBOUNDED -- honest gap` form promises
"every one of these is also listed in section 8", and the two partial forms
`orphan-only` and `dead-only` each promise "the residual stays listed in
section 8".

That claim was FALSE in two places, and the second was not in the research:

* File-connector output / spill dirs carried `UNBOUNDED -- honest gap` in
  section 2 with NO section 8 row at all
* the `reference.name` / `reference.version` / `reference.key` columns carried
  `orphan-only` while section 8 named only the `reference.value` they key

The second was found by writing the check and reading what it reported, rather
than by trusting the row's own account of the defect -- which named one instance.
Deriving the failure set before writing the assertions is the only reason it is
in this commit.

Fixes match the shape of each gap. The spill dirs get their own section 8 row,
stating conditionally that on a first deployment those directories would
accumulate plaintext PL-1 bodies with no engine-managed deletion, and that the
cover is operator-owned. The key columns do NOT get a duplicate row: they ride
the same purge as the value they key, so the existing `reference.value` row now
names them, with the PL-4-versus-PL-2 asymmetry stated rather than smoothed over.

The guard binds the legend's own promise. For every section 2 Retention cell
carrying one of the three residual forms, at least one backticked identifier from
its Location must appear in the section 8 honest-gaps table. The join is on
backticked tokens, not whole cells, because the two tables word the same tier
differently on purpose -- section 2 carries provenance, section 8 carries the
reason -- so a whole-cell comparison would red on prose that is not drift.

Two liveness receipts, because this guard's failure mode is passing vacuously:
the section 8 slicer asserts it parsed at least seven lines, and the walk asserts
it actually exercised each of the three forms it claims to cover. A section 2
table that stopped using a form would otherwise make this pass by finding
nothing.

NO ENGINE BEHAVIOUR CHANGES, and none should: the 2026-07-30 no-auto-bound ruling
rules out giving these tiers a window. An automatic unlink of File-connector
output would be the engine deleting data an operator or a downstream system owns,
and pointing purge_reference_snapshots at declared sets would delete data the
engine is still serving. This pins the classification the verb measures against
without asserting deletion coverage that does not exist, and it must not be read
as moving the cell to pass.

Mutations executed, both on the restored tree: deleting only the section 8 spill
row reds the guard naming that tier, and reverting the reference row to its
pre-fix wording reds it naming the key columns. The FIRST attempt at the first
mutation was itself wrong and is worth recording -- it deleted the section 2 row
along with the section 8 row, removing the obligation and the evidence together,
so the test passed and looked like a working control.

PROVENANCE: built earlier this session, then parked at
refs/parked/asvs-1188-phi-honest-gaps because the row's claim was held by a
worktree with no live session. That claim's own note said "release on merge" for
PR 935, which merged at 16:20Z; the owner force-released it. Restored and
re-verified here: the parked work predates this branch's other PHI.md edit, so
both were confirmed present and the only removed line is the reference row this
change replaces.

Gates: ruff format + ruff check clean, 42 passed across
test_retention_classification_drift and test_phi_at_rest_inventory, and
test_tooling_partition still green (this file imports the engine, so it needs no
new classification entry).

The row STAYS OPEN. Its closing act is a scorecard re-score in the separate vault
repository, which no engine commit can perform. Untouched: the still-declared
reference.value tier, the permanently-pending queue.payload rows, and the absence
of any filesystem sweep for the spill directories -- all three remain real gaps,
now merely honest ones.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…e cell (BACKLOG #1188)

One banner, at the top of the item's blockquote. The row STAYS OPEN: verified
with parse_items rather than a hand-rolled scan.

The banner leads with the finding that was not in the row's research -- the
section 2 legend's completeness claim was false in TWO places, and the second was
found by deriving the failure set before writing any assertion rather than by
seeding the check from the row's own account of the defect.

It also says plainly what this does NOT do. The three tiers are unchanged and
still unreached; making a gap honest is a smaller thing than closing it, and the
closing act is a scorecard re-score no engine commit can perform. That sentence
is in the banner because the tempting misread of this change is that documenting
a gap moved the cell.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@wshallwshall wshallwshall changed the title sec: ASVS packet G -- authorization and data protection (BACKLOG #1151, #1153, #1154, #1184, #1186, #1187) sec: ASVS packet G -- authorization and data protection (BACKLOG #1151, #1153, #1154, #1184, #1186, #1187, #1188) Sep 6, 2026
@wshallwshall
wshallwshall added this pull request to the merge queue Sep 7, 2026
@github-actions github-actions Bot added the unread Green and mergeable, but not marked read. Read it, then add: reviewed label Sep 7, 2026
@github-actions

github-actions Bot commented Sep 7, 2026

Copy link
Copy Markdown

@wshallwshall

This pull request is green and nobody has marked it read.

Every check that has reported is passing and the reviewed label is absent, so
a reviewer has read this is the only thing between it and a merge. Nothing else reports that, which is
why this comment exists (BACKLOG #1413).

To clear it, IN THIS ORDER:

  1. If the branch is behind main, update it FIRST: gh pr update-branch --repo MEFORORG/MessageFoundry 944.
    That push fires synchronize, and synchronize REMOVES the reviewed label. Labelling
    before updating throws the label away and costs a round trip.
  2. Read the diff.
  3. gh pr edit 944 --repo MEFORORG/MessageFoundry --add-label reviewed

DO NOT TRIAGE THIS FROM mergeStateStatus. It returns one value with precedence, so BEHIND,
DIRTY and UNSTABLE each mask the missing check -- the requirement is invisible until you act on
something else. This check never reads that field; see scripts/ci/check_unread_prs.py.

A seat can find every pull request in this state in one call: gh pr list --label unread.

Adding reviewed withdraws unread: the label event re-evaluates this pull request,
and so does the next completion of a watched workflow. Those are the two edges that clear it, so a
change neither of them reports -- converting to a draft, say -- can leave the label standing until one
of them next happens.

The reviewed label is a PROCESS gate. It records that a step happened. It does not
establish that an independent party looked, and nothing here should be read as saying it does.

Merged via the queue into main with commit b2200dc Sep 7, 2026
44 checks passed
@wshallwshall
wshallwshall deleted the claude/asvs-phi-brief-03d670 branch September 7, 2026 17:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci-red A required check went red. Attribute it before retrying. unread Green and mergeable, but not marked read. Read it, then add: reviewed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant