security(dci): stop DCI Administrator group from granting system admin

[gonzalesedwin1123]

Do not merge yet — held for human review.

Summary

spp_dci.group_dci_admin (the group gating raw DCI payloads and PII fields in every spp_dci_* view) was defined with implied_ids linking base.group_system. In Odoo, implied_ids grants the implied groups to members — it is not a membership precondition, though the record's own comment ("Members must already be system administrators") shows it was believed to be one. So assigning the scoped DCI/PII-visibility role silently made that user a full Settings/System administrator. The pinning test added with the group (test_dci_admin_group_implies_system) locked the escalation into CI. The commit that introduced this (a5826347) is in the released 2026.07 tag, so deployed databases carry the escalation.

Fix

• Remove the escalation: drop base.group_system from group_dci_admin.implied_ids and correct the record comment.
• Preserve admin visibility the documented way: link group_dci_admin into spp_security.group_spp_admin.implied_ids — the exact extension pattern documented in spp_security/security/groups_admin.xml (base.group_system → group_spp_admin → domain groups). System/OpenSPP admins keep seeing the gated PII fields; DCI Administrator grants nothing beyond itself. spp_security is already in spp_dci's dependency closure (spp_dci → spp_registry → spp_security). The extension record sits outside the noupdate block so upgrades apply it.
• Migration for released databases: the group record is noupdate="1", so the XML fix never reaches installed DBs — migrations/19.0.2.0.2/post-migration.py strips the link via the ORM and logs how many users held the group. Odoo 19 computes effective membership as a transitive closure of implied_ids (res_groups.py: all_implied_ids), so removing the link retroactively revokes system admin from affected users; there is no stored per-user expansion to clean.
• Version bump 19.0.2.0.1 → 19.0.2.0.2 + HISTORY.md fragment.

No changes needed in the five consumer modules (spp_dci_client_crvs/_dr/_ibr/_sr, spp_dci_server): their groups="spp_dci.group_dci_admin" view gates and arch-assertion tests are unaffected.

Tests (TDD — red commit first)

• test_dci_admin_does_not_grant_system (replaces the escalation-pinning test): base.group_system absent from implied_ids and all_implied_ids (closure — blocks reintroduction through an intermediate group); behavioral check that a user granted the group does not has_group('base.group_system').
• test_spp_admin_implies_dci_admin: admins keep PII visibility (closure + behavioral).
• test_migration_removes_escalation: recreates the released vulnerable state, runs the migration's migrate(), asserts the link and its closure effects are gone.
• Green: spp_dci 22/22, plus consumer suites spp_dci_client_crvs 98, spp_dci_client_dr 144, spp_dci_client_ibr 103, spp_dci_client_sr 134, spp_dci_server 332 — all 0 failed. Lint (ruff/prettier/pylint-odoo/OCA checks) clean; security audit 0 errors / 0 warnings.

Operational note for deployments

On 2026.07 installs, anyone granted DCI Administrator has been a de-facto system admin since the grant. The migration revokes the privilege on upgrade, but operators should audit actions taken by those users while escalated (settings, user/group management, module installs) — removal does not undo what was done.

Reviewed

Adversarial staff review: fix confirmed correct and complete for the stated vulnerability, verified against Odoo 19 source. Confirmed: the <data noupdate="1"> + bare extension-record XML structure is standard and matches ~15 sibling modules; load order guarantees spp_security.group_spp_admin exists; Odoo's MigrationManager runs the 19.0.2.0.2 script on a 19.0.2.0.1 → 19.0.2.0.2 upgrade (inclusive upper bound) and post-migration runs after the non-noupdate extension record is applied; Odoo 19 does not materialize implied groups into res_groups_users_rel, and res.groups.write() invalidates the registry group and ACL caches, so the ORM unlink revokes the escalation immediately; the group did not exist in v17 openspp-modules, so no legacy materialized-membership residue is possible. Review polish applied: the migration also refreshes the record's stale comment (the noupdate flag would otherwise preserve the misleading "must already be system administrators" text on upgraded DBs), and the migration test pins the operator-facing warning with assertLogs. One out-of-scope sibling finding was raised to the maintainers separately.

Related

Sibling escalation of the same class (implied_ids linking base.group_system), found by this PR's review: #329 (spp_key_management.group_key_admin). A repo-wide sweep found only these two instances; #334 tracks a CI guard to prevent a third.

Merge order & release-note note

Per the cross-PR interaction analysis (2026-07-24, internal/plans/security-prs-interaction-analysis.md): no hard ordering — every combination with the other open security PRs commutes. Soft preference: land this early (or in the same release as #336/#265) — until the implication is stripped, DCI admins still transit base.group_system and pass every new system-admin gate those PRs add, so this PR is what makes their exclusions effective.

Release notes must state: DCI Administrator no longer grants system admin, and all DCI-server administration (bearer tokens, sender registries, subscriptions, server keys, the settings block) is base.group_system-gated — deployments whose DCI operator held only this group must grant base.group_system explicitly. #269/#326 add further system-only fields in the same window. The migration logs a warning, but upgrade-log warnings are easy to miss.

[gemini-code-assist]

Caution

The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (19.0-staging-sec-batch1@cb7f4c6). Learn more about missing BASE report.

Additional details and impacted files

@@                    Coverage Diff                     @@
##             19.0-staging-sec-batch1     #327   +/-   ##
==========================================================
  Coverage                           ?   78.18%           
==========================================================
  Files                              ?      225           
  Lines                              ?    14929           
  Branches                           ?        0           
==========================================================
  Hits                               ?    11672           
  Misses                             ?     3257           
  Partials                           ?        0           

Flag	Coverage Δ
spp_base_common	91.07% <ø> (?)
spp_dci	89.59% <ø> (?)
spp_dci_client	89.25% <ø> (?)
spp_dci_client_compliance	99.25% <ø> (?)
spp_dci_client_crvs	92.51% <ø> (?)
spp_dci_client_dr	85.77% <ø> (?)
spp_dci_client_ibr	92.47% <ø> (?)
spp_dci_client_sr	98.10% <ø> (?)
spp_dci_compliance	93.01% <ø> (?)
spp_dci_demo	94.28% <ø> (?)
spp_dci_indicators	96.23% <ø> (?)
spp_dci_server	90.36% <ø> (?)
spp_dci_server_social	89.57% <ø> (?)
spp_programs	65.27% <ø> (?)
spp_registry	86.94% <ø> (?)
spp_security	69.56% <ø> (?)
spp_starter_sp_mis	86.66% <ø> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[gonzalesedwin1123]

Follow-up filed from the review: #334 — CI guard against res.groups implying base.group_system outside spp_security, to prevent another instance of this privilege-escalation class (this fix + #329 were the two found).

[gemini-code-assist]

Caution

The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased.