Skip to content

Commit 34b2ffb

Browse files
wshallwshallwshallwshallclaude
authored
test(quality): the username-as-access-key screen could be narrowed to silence (BACKLOG #1226) (#595)
The screen and its wiring already shipped. What was missing is the row's PROOF CLAUSE: nothing stopped the report being emptied by a one-line edit that looks like reclassifying a field. MEASURED, NOT READ. The dispatch said limb 2's absence rested on reading the test module plus two history searches, with no executed mutation. So I ran one. Moving "uploader" from ACCESS_KEY_NAMES into LABEL_NAMES: the suite 10 passed -> 10 passed, unchanged the live report 8 candidates -> 7; excluded 80 -> 81 api/app.py save(uploader=...) DISAPPEARS from the report entirely The uploads site stops being surfaced for judgement and is silently recorded as "excluded as correct". Nothing reds. Being absent from a report is indistinguishable from having been adjudicated correct, which is the whole defect. *** A DISJOINTNESS TEST DOES NOT CATCH IT, and that is the trap. *** The mutation REMOVES the name from one set and ADDS it to the other, so the two stay disjoint throughout. Membership has to be PINNED -- the device tests/test_private_paths_stay_ignored.py already uses. THREE TESTS, TWO ROUTES, AND THEY DISCRIMINATE RATHER THAN OVERLAPPING. Each set-level guard fires on its own route only, which is what shows they are two tests and not one wearing two names: mutation pin overlap proof clause move uploader (remove + add) FIRES silent FIRES add uploader to LABEL_NAMES only silent FIRES FIRES Both scored by applying the mutation, running, and restoring from a byte copy, hash-verified identical each time. THE PROOF CLAUSE IS ASSERTED ON THE SHIPPED SCREEN'S OUTPUT, not on its constants, and that is deliberate: the two set-level tests each cover one narrowing route, while the report assertion covers the OUTCOME regardless of route -- a rule change or a scope change would red it too. IT DOES NOT CALL THE SITE A DEFECT, which the row is explicit about. `uploader=identity.username` may well be correct code -- a sibling `uploader_id` can carry the key. The screen emits CANDIDATES, never verdicts, and several reported sites are correct. What is asserted is that a reader still gets to SEE it and decide. That constraint pulls against the other one -- the report must stay reviewable AND must not become a verdict -- and dropping either gives the wrong artifact. I DID NOT REBUILD THE SCREEN. The dispatch warned that a builder reading only the row's opening would, and it is already on main and already wired into ci.yml and pre-commit. My own screen confirmed it: 5 BACKLOG-form commits postdating the 2026-08-11 filing. VERIFIED, scope named: pytest 13 passed, tests/test_username_access_key_screen.py (10 -> 13) ruff format --check, ruff check, mypy -- run separately, each with its own exit code, all clean. mypy flagged my own helper for a missing return annotation and I fixed it rather than leaving a known error on the grounds that CI does not cover tests/. the shipped screen run directly, before and after each mutation NOT a full-suite run Co-authored-by: wshallwshall <mefordev@messagefoundry.org> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
1 parent 0849620 commit 34b2ffb

1 file changed

Lines changed: 88 additions & 0 deletions

File tree

tests/test_username_access_key_screen.py

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ class this screen exists to catch.
3636
import subprocess
3737
import sys
3838
from pathlib import Path
39+
from types import ModuleType
3940

4041
ROOT = Path(__file__).resolve().parents[1]
4142
SCREEN = ROOT / "scripts" / "quality" / "username_access_key_screen.py"
@@ -249,3 +250,90 @@ def test_the_screen_is_wired_in_BOTH_places_like_its_sibling() -> None:
249250
assert "--baseline" in ci and "--baseline" in pre, (
250251
"wired WITHOUT --baseline, which installs a step that can never fail"
251252
)
253+
254+
255+
# ---------------------------------------------------------------------------------------------
256+
# BACKLOG #1226, THE PROOF CLAUSE. The screen can be silently narrowed to nothing, and every
257+
# assertion above stays GREEN while it happens.
258+
#
259+
# MEASURED, not reasoned. Move "uploader" from ACCESS_KEY_NAMES into LABEL_NAMES -- a one-line
260+
# edit that looks like reclassifying a field -- and:
261+
# the suite 10 passed, unchanged
262+
# the live report 8 candidates -> 7, excluded 80 -> 81
263+
# app.py save(uploader=...) DISAPPEARS from the report
264+
# The uploads site stops being surfaced for judgement and is silently recorded as
265+
# "excluded as correct" instead. Nothing reds. That is the defect this clause closes.
266+
#
267+
# A DISJOINTNESS TEST DOES NOT CATCH IT, which is the trap worth naming: the mutation REMOVES the
268+
# name from one set and ADDS it to the other, so the two stay disjoint throughout. Membership has
269+
# to be PINNED, the same device `test_the_pinned_list_has_not_silently_shrunk` uses in
270+
# tests/test_private_paths_stay_ignored.py.
271+
# ---------------------------------------------------------------------------------------------
272+
273+
274+
def _load_screen() -> ModuleType:
275+
"""Import the screen as a module so its constants can be asserted on directly.
276+
277+
It lives under scripts/ rather than in an installed package, so it is loaded by path -- the
278+
same device tests/test_scan_forbidden.py uses for the leak scanner.
279+
"""
280+
import importlib.util
281+
282+
spec = importlib.util.spec_from_file_location("username_access_key_screen", SCREEN)
283+
assert spec is not None and spec.loader is not None
284+
mod = importlib.util.module_from_spec(spec)
285+
spec.loader.exec_module(mod)
286+
return mod
287+
288+
289+
#: Access-key names whose REMOVAL would empty part of the report. Pinned rather than derived: this
290+
#: list existing is the point, and a name leaving it must be a reviewed edit rather than a side
291+
#: effect of reclassifying a field. "at least these" -- adding is free, removing must red.
292+
_PINNED_ACCESS_KEY_NAMES = frozenset({"owner", "uploader", "user", "key"})
293+
294+
295+
def test_the_access_key_names_have_not_silently_shrunk() -> None:
296+
screen = _load_screen()
297+
missing = _PINNED_ACCESS_KEY_NAMES - screen.ACCESS_KEY_NAMES
298+
assert not missing, (
299+
f"{sorted(missing)} left ACCESS_KEY_NAMES. Each one is a shape that scopes a RESOURCE by "
300+
"username -- the class this screen exists for. Removing one narrows the report with no "
301+
"other signal: the suite stays green and the sites it covered are recorded as excluded."
302+
)
303+
304+
305+
def test_no_access_key_name_is_also_an_audit_label() -> None:
306+
"""The two sets must not overlap, and this is the WEAKER of the two guards on purpose.
307+
308+
It cannot catch the measured mutation -- moving a name from one set to the other keeps them
309+
disjoint. It catches the sloppier edit that adds a name to LABEL_NAMES without removing it,
310+
where the exclusion would silently win.
311+
"""
312+
screen = _load_screen()
313+
both = screen.ACCESS_KEY_NAMES & screen.LABEL_NAMES
314+
assert not both, (
315+
f"{sorted(both)} is in BOTH ACCESS_KEY_NAMES and LABEL_NAMES. The exclusion wins, so those "
316+
"sites vanish from the report while the name still reads as covered."
317+
)
318+
319+
320+
def test_the_uploads_site_stays_REVIEWABLE_rather_than_absent() -> None:
321+
"""THE PROOF CLAUSE ITSELF, and it is deliberately about the REPORT, not about the sets.
322+
323+
The item's requirement is that the uploads report stay reviewable -- each hit adjudicated as
324+
correctly-keyed rather than ABSENT. `save(uploader=...)` in api/app.py is the site that carries
325+
that: `uploader=identity.username` may well be correct code (a sibling `uploader_id` can carry
326+
the key), and this test does NOT assert it is a defect. It asserts the reader still gets to see
327+
it and decide.
328+
329+
Asserted on the SHIPPED SCREEN's output rather than on its constants, so ANY narrowing that
330+
removes the site reds here -- a set edit, a rule change, a scope change. The two set-level tests
331+
above cover one route each; this covers the outcome regardless of route.
332+
"""
333+
proc = run()
334+
assert proc.returncode == 0, proc.stderr
335+
assert "save(uploader=...)" in proc.stdout, (
336+
"the uploads site is no longer surfaced for judgement. The screen has gone quiet on the "
337+
"shape BACKLOG #1226 is about, and being absent from the report is indistinguishable from "
338+
f"having been adjudicated correct.\n{proc.stdout}"
339+
)

0 commit comments

Comments
 (0)