Follow-up from two privilege-escalation fixes: PR #327 (spp_dci.group_dci_admin) and PR #329 (spp_key_management.group_key_admin).
Type: tooling / hardening (prevent a recurring vulnerability class)
Problem
Two separate groups shipped in released versions with implied_ids linking base.group_system. Because Odoo's implied_ids grants the implied groups to a group's members (it is not a membership precondition), each silently promoted a scoped role to full Settings/System administrator. Both were written by authors who believed the link meant "members must already be system admins" — the exact opposite of its effect. A repo-wide sweep found only these two, but nothing prevents a third.
Proposed guard
Add a CI/pre-commit check (or a res.groups load-time assertion) that fails when any res.groups record outside spp_security — or outside an explicitly allowlisted set of intentional role records — declares implied_ids linking base.group_system (or base.group_erp_manager). This makes the escalation class impossible to reintroduce quietly.
Notes for whoever implements:
- Sweep must be multi-line-aware: the
eval="[Command.link(ref('base.group_system'))]" list often wraps across lines, so a single-line grep misses instances.
- The legitimate cases to allowlist:
spp_security's own admin wiring and spp_user_roles' "System Admin" global role.
Related: #327, #329.
Follow-up from two privilege-escalation fixes: PR #327 (
spp_dci.group_dci_admin) and PR #329 (spp_key_management.group_key_admin).Type: tooling / hardening (prevent a recurring vulnerability class)
Problem
Two separate groups shipped in released versions with
implied_idslinkingbase.group_system. Because Odoo'simplied_idsgrants the implied groups to a group's members (it is not a membership precondition), each silently promoted a scoped role to full Settings/System administrator. Both were written by authors who believed the link meant "members must already be system admins" — the exact opposite of its effect. A repo-wide sweep found only these two, but nothing prevents a third.Proposed guard
Add a CI/pre-commit check (or a
res.groupsload-time assertion) that fails when anyres.groupsrecord outsidespp_security— or outside an explicitly allowlisted set of intentional role records — declaresimplied_idslinkingbase.group_system(orbase.group_erp_manager). This makes the escalation class impossible to reintroduce quietly.Notes for whoever implements:
eval="[Command.link(ref('base.group_system'))]"list often wraps across lines, so a single-line grep misses instances.spp_security's own admin wiring andspp_user_roles' "System Admin" global role.Related: #327, #329.