Skip to content

security(spp_farmer_registry_cr, spp_studio_change_requests): CR detail models lack record rules — unscoped RPC access (same class as #261) [Severity: High] #423

Description

@gonzalesedwin1123

Surfaced during the batch-2 review of PR #261 (security(cr): add record rules to CR detail models). #261 fixed the missing-ir.rule class only for the group_cr_user detail models in spp_change_request_v2 and spp_cr_type_assign_program. The identical vulnerability class remains live in two sibling modules under their own groups.

Severity: High (same class as #261 — unscoped RPC read/write of other users' CR detail data)

Problem

CR detail models (spp.cr.detail.*) inherit an abstract base, so the parent spp.change.request ownership/area ir.rules do not cascade to them. Each concrete detail model ships an ACL granting write/create to a group but, without its own ir.rule, the row access is unscoped — any member of that group can read/tamper with detail rows of change requests they do not own, over RPC.

#261 closed this for group_cr_user. Still open:

  • spp_farmer_registry_cr — the farm CR detail models (group_registry_officer). Confirm each concrete spp.cr.detail.* model in this module carries ownership + area rules 1:1 (mirroring security(cr): add record rules to CR detail models (ownership + area) #261), TransientModel wizards exempt.
  • spp_studio_change_requestsspp.cr.detail.generic (group_studio_editor_officer).

Fix

Per-module ownership analysis (the group models differ from group_cr_user, so #261's rules can't be reused verbatim): add user/validator/manager ir.rules scoped through the parent change request, plus a global area rule, for every concrete detail model. Extend spp_change_request_v2's completeness test pattern (which iterates spp.cr.detail.% and asserts every ACL-granted op has a carrying rule) to cover these modules, or add per-module equivalents.

Notes

  • spp_change_request_v2's completeness test deliberately scopes to group_cr_user-reachable models and ignores these, so CI stays green today — this must be filed/fixed separately (it is).
  • Version bump + migration considerations per the standard security-fix checklist.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions