Follow-up from the batch-2 review of PR #353 (security(roles): scope program/CR roles to Tier-3 registry read, drop registry-search menu). The fix is correct and complete; two verification/hardening items were deferred.
Severity: Low (test hardening + a verification check)
1. Assert the registry-search menu is gone via load_menus
#353's tests assert the four roles hold group_registry_read and not group_registry_viewer, which is a strong proxy (both the leaf menu_registry_search and the Registry app root are gated on viewer). But per the repo pitfall, has_group / _visible_menu_ids can diverge from real UI pruning — only load_menus applies ancestor-invisibility pruning. Add a test asserting spp_registry_search.menu_registry_search (and the Registry app root) is absent from load_menus(False) for a user holding each of the four scoped roles.
2. Verify no program/CR screen depends on the dropped viewer-only reads
Moving the roles off Tier-2 viewer also dropped read on viewer-only ACLs beyond the menu (e.g. spp_consent.*, spp_banking res.partner.bank, and if installed spp_dci_client_* / spp_irrigation). Registrant-core read is preserved, and this tightening is aligned — but confirm no program/CR screen these four roles actually open renders a consent/bank field that now raises AccessError. One-pass check; add a regression test if any screen is affected.
References
Batch-2 review: internal/plans/batch2-fix-review.md (#353 section). Scoped roles: Program Viewer, CR Requestor, CR Local Validator, CR HQ Validator.
Follow-up from the batch-2 review of PR #353 (
security(roles): scope program/CR roles to Tier-3 registry read, drop registry-search menu). The fix is correct and complete; two verification/hardening items were deferred.Severity: Low (test hardening + a verification check)
1. Assert the registry-search menu is gone via
load_menus#353's tests assert the four roles hold
group_registry_readand notgroup_registry_viewer, which is a strong proxy (both the leafmenu_registry_searchand the Registry app root are gated on viewer). But per the repo pitfall,has_group/_visible_menu_idscan diverge from real UI pruning — onlyload_menusapplies ancestor-invisibility pruning. Add a test assertingspp_registry_search.menu_registry_search(and the Registry app root) is absent fromload_menus(False)for a user holding each of the four scoped roles.2. Verify no program/CR screen depends on the dropped viewer-only reads
Moving the roles off Tier-2 viewer also dropped read on viewer-only ACLs beyond the menu (e.g.
spp_consent.*,spp_bankingres.partner.bank, and if installedspp_dci_client_*/spp_irrigation). Registrant-core read is preserved, and this tightening is aligned — but confirm no program/CR screen these four roles actually open renders a consent/bank field that now raisesAccessError. One-pass check; add a regression test if any screen is affected.References
Batch-2 review:
internal/plans/batch2-fix-review.md(#353 section). Scoped roles: Program Viewer, CR Requestor, CR Local Validator, CR HQ Validator.