Split out of the review of #454, where it was raised as a non-blocking observation.
The gap
spp.program has no registry-tier ACL row at all — neither Tier-2 group_registry_viewer nor Tier-3 group_registry_read grants read on it. Meanwhile the registrant form renders program_id inside the ungated program_membership_ids list (spp_programs/views/registrant_view.xml:103).
A user holding only base.group_user + spp_registry.group_registry_read therefore hits an AccessError opening any enrolled registrant.
Why nothing is visibly broken today
The shipped roles are saved by side grants rather than by design:
- Program Viewer — via
group_programs_viewer.
- CR Requestor / Local Validator / HQ Validator — via
spp_hazard.group_hazard_viewer → spp_hazard_programs's access_spp_program_hazard_viewer. That module is auto_install on {spp_hazard, spp_programs}, and spp_change_request_v2 hard-depends on spp_hazard, so it is present wherever the program list can render.
That coupling is accidental. A future security pass that drops group_hazard_viewer from the CR roles — exactly the kind of tightening #353 did — would break their registrant form, and nothing in the roles or the tests records the dependency.
This is not a #353 regression: the Tier-2 viewer tier never had spp.program either. It predates that change, which is why it was kept out of #454 — that PR restores access #353 removed, and adding spp.program would be a new grant rather than a restoration.
Suggested fix
access_spp_program_registry_read,Program Registry Read,spp_programs.model_spp_program,spp_registry.group_registry_read,1,0,0,0
plus "spp.program" in spp_programs/tests/test_registry_read_access.py's _MODELS.
Worth pairing with a functional test
The model list in test_registry_read_access.py is hand-maintained, so it can only assert dependencies someone already thought of — it could not have caught this one. A web_read-style test that opens an enrolled registrant as a bare Tier-3 user would assert the behaviour the changelog actually claims, and would catch any missing dependency rather than only the listed ones.
Credit to @gonzalesedwin1123 for spotting this during review of #454, including the trace through spp_hazard_programs' auto-install.
Split out of the review of #454, where it was raised as a non-blocking observation.
The gap
spp.programhas no registry-tier ACL row at all — neither Tier-2group_registry_viewernor Tier-3group_registry_readgrants read on it. Meanwhile the registrant form rendersprogram_idinside the ungatedprogram_membership_idslist (spp_programs/views/registrant_view.xml:103).A user holding only
base.group_user+spp_registry.group_registry_readtherefore hits anAccessErroropening any enrolled registrant.Why nothing is visibly broken today
The shipped roles are saved by side grants rather than by design:
group_programs_viewer.spp_hazard.group_hazard_viewer→spp_hazard_programs'saccess_spp_program_hazard_viewer. That module isauto_installon{spp_hazard, spp_programs}, andspp_change_request_v2hard-depends onspp_hazard, so it is present wherever the program list can render.That coupling is accidental. A future security pass that drops
group_hazard_viewerfrom the CR roles — exactly the kind of tightening #353 did — would break their registrant form, and nothing in the roles or the tests records the dependency.This is not a #353 regression: the Tier-2 viewer tier never had
spp.programeither. It predates that change, which is why it was kept out of #454 — that PR restores access #353 removed, and addingspp.programwould be a new grant rather than a restoration.Suggested fix
plus
"spp.program"inspp_programs/tests/test_registry_read_access.py's_MODELS.Worth pairing with a functional test
The model list in
test_registry_read_access.pyis hand-maintained, so it can only assert dependencies someone already thought of — it could not have caught this one. Aweb_read-style test that opens an enrolled registrant as a bare Tier-3 user would assert the behaviour the changelog actually claims, and would catch any missing dependency rather than only the listed ones.Credit to @gonzalesedwin1123 for spotting this during review of #454, including the trace through
spp_hazard_programs' auto-install.