Skip to content

security: batch 2 — CR + programs chain (#264, #261, #338, #336, #353, #343, #365) - #422

Open
gonzalesedwin1123 wants to merge 7 commits into
19.0from
19.0-staging-sec-batch2
Open

security: batch 2 — CR + programs chain (#264, #261, #338, #336, #353, #343, #365)#422
gonzalesedwin1123 wants to merge 7 commits into
19.0from
19.0-staging-sec-batch2

Conversation

@gonzalesedwin1123

Copy link
Copy Markdown
Member

Lands security batch 2 — the change-request + programs chain, seven interdependent fixes validated together on a staging branch so 19.0 is never exposed to an unverified intermediate state.

⚠ Merge with a MERGE COMMIT — not squash

Squashing collapses seven distinct security fixes into one opaque commit. The merge commit preserves one commit per fix (each carrying its (#NNN) reference) for bisect and attribution. The repo default is squash — change it at merge time. (Batch 1 / #399 was accidentally squashed; do not repeat.)

What's in it

Seven PRs, hard-ordered because the CR module version chain and a migration dir depend on it: #264#261#338#336#353#343#365.

PR Fix Module → version
#264 Route + apply the same single field for dynamic approvals (freeze proposed change post-submit) spp_change_request_v2 → 3.1.2
#261 Record rules (ownership + area) on every CR detail model spp_change_request_v2 3.1.3, spp_cr_type_assign_program 1.0.2
#338 Validate program access on the assign-program detail (+ apply-sink re-check + preview redaction) spp_cr_type_assign_program → 1.0.3
#336 Enforce system-admin authz on Force Unlock (field guard, not just the button) spp_programs 2.2.1, spp_program_geofence 1.0.1, spp_farmer_registry_demo 2.1.2
#353 Scope program/CR roles to Tier-3 registry read; drop the registry-search menu spp_change_request_v2 3.1.4 (+migration), spp_programs 2.2.2 (+migration)
#343 Writable selected_field no longer bypasses CR conflict/duplicate checks (fail-closed) spp_change_request_v2 → 3.1.5
#365 Server-side manager authorization on CR apply (RPC/UI/wizards/API v2) spp_change_request_v2 → 3.1.6

Review

Every fix was independently adversarially reviewed against its exact vulnerability, Odoo 19 core semantics, and test coverage before merging into staging (internal/plans/batch2-fix-review.md): six ACCURATE-AND-COMPLETE with follow-ups only. The one CONCERNS finding — #338 rested on a false "module unreleased" premise, leaving a residual on already-deployed DBs — was fixed in-branch (apply-time sink re-check bound to the requester's company scope + preview redaction + corrected rationale) and that delta separately reviewed.

Verification (on the merged staging tree)

  • Version chain — every module strictly above 19.0: CR 3.1.1→3.1.6, programs 2.2.0→2.2.2, assign_program 1.0.1→1.0.3, geofence 1.0.0→1.0.1, farmer_demo 2.1.1→2.1.2. Migration dirs match manifests (CR 19.0.3.1.4, programs 19.0.2.2.2).
  • Structural / file union — the delta vs 19.0 is confined to exactly the 5 batch-2 modules, no riders (18 CR + 19 programs + 10 assign_program + 4 geofence + 5 farmer_demo files).
  • Per-PR CI — each PR was squash-merged into staging pinned with --match-head-commit <CI'd head> (manifest: internal/plans/batch2-manifest.tsv), green on all module test jobs at that head. Each rebase was verified footprint-clean with its code files byte-identical to the reviewed head — only manifest/HISTORY/generated-README metadata was merged.
  • Batch CI — a "Full Test Suite" workflow_dispatch on the staging head is running (linked in checks).
  • Note on check_version_chain.py: run post-merge it reports false "collision" errors, because the seven branches are now stacked (each rebased onto its predecessors) rather than independent off 19.0 — the tool's pre-merge model. The merged-tree checks above are the authoritative post-merge verification.

🔴 Release notes — behavior changes

  1. security(roles): scope program/CR roles to Tier-3 registry read (drop registry-search menu) #353 — Program Viewer / CR Requestor / CR Local Validator / CR HQ Validator lose the Registry Search menu (moved from Tier-2 viewer to Tier-3 read; registrant read unchanged). Migrations revoke the viewer group from existing users on upgrade.
  2. security(programs): enforce system-admin authorization on Force Unlock #336 — Force Unlock on cycles/programs now requires base.group_system server-side; program officers/managers/cycle-approvers can no longer clear an active operation lock via RPC.
  3. security(spp_change_request_v2): enforce manager authorization on CR apply (server-side) #365 — operator action: the API v2 change-request $apply endpoint now requires the endpoint user to hold group_cr_manager. Deployments applying CRs via API must grant that role.
  4. security(cr): route and apply the same single field for dynamic approvals #264 / security(cr): writable selected_field bypasses CR conflict checks #343 — dynamic-approval CRs: the routed field is frozen post-submit and conflict/duplicate detection derives from the actual diff. Latent today (no shipped CR type enables dynamic approval), engages when one is configured.

After merging

Plan & manifest: internal/plans/security-staging-merge-plan.md, internal/plans/batch2-manifest.tsv, internal/plans/batch2-fix-review.md.

@codecov

codecov Bot commented Aug 14, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 91.87817% with 16 lines in your changes missing coverage. Please review.
✅ Project coverage is 72.76%. Comparing base (0820667) to head (c739519).

Files with missing lines Patch % Lines
spp_change_request_v2/models/conflict_mixin.py 92.30% 4 Missing ⚠️
...rmer_registry_demo/models/farmer_demo_generator.py 0.00% 3 Missing ⚠️
...rams/models/managers/entitlement_manager_inkind.py 0.00% 2 Missing ⚠️
spp_programs/models/managers/payment_manager.py 50.00% 2 Missing ⚠️
spp_change_request_v2/models/change_request.py 97.29% 1 Missing ⚠️
...ge_request_v2/models/change_request_detail_base.py 95.65% 1 Missing ⚠️
spp_change_request_v2/strategies/field_mapping.py 90.90% 1 Missing ⚠️
...p_cr_type_assign_program/details/assign_program.py 90.00% 1 Missing ⚠️
...ograms/models/managers/entitlement_manager_base.py 80.00% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             19.0     #422      +/-   ##
==========================================
+ Coverage   72.24%   72.76%   +0.51%     
==========================================
  Files         419      462      +43     
  Lines       29813    31545    +1732     
==========================================
+ Hits        21539    22953    +1414     
- Misses       8274     8592     +318     
Flag Coverage Δ
spp_api_v2_change_request 66.53% <ø> (ø)
spp_api_v2_cycles 71.03% <ø> (ø)
spp_api_v2_entitlements 70.23% <ø> (ø)
spp_api_v2_programs 92.22% <ø> (ø)
spp_api_v2_simulation 71.19% <ø> (?)
spp_base_common 91.07% <ø> (ø)
spp_case_entitlements 100.00% <ø> (?)
spp_case_programs 100.00% <ø> (?)
spp_change_request_v2 78.13% <94.30%> (+0.29%) ⬆️
spp_cr_type_assign_program 92.50% <95.00%> (+0.42%) ⬆️
spp_dci_compliance 93.01% <ø> (?)
spp_dci_demo 94.28% <ø> (ø)
spp_farmer_registry_cr 61.24% <ø> (ø)
spp_farmer_registry_demo 61.02% <0.00%> (-0.05%) ⬇️
spp_mis_demo_v2 70.38% <ø> (ø)
spp_program_geofence 97.02% <100.00%> (?)
spp_programs 65.47% <89.79%> (+0.19%) ⬆️
spp_registry 87.22% <ø> (+0.07%) ⬆️
spp_security 69.56% <ø> (ø)

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

Files with missing lines Coverage Δ
...r_type_assign_program/strategies/assign_program.py 94.82% <100.00%> (+1.07%) ⬆️
spp_program_geofence/models/eligibility_manager.py 96.26% <100.00%> (ø)
spp_programs/models/cycle.py 66.11% <100.00%> (+0.63%) ⬆️
spp_programs/models/managers/cycle_manager_base.py 72.65% <100.00%> (ø)
...pp_programs/models/managers/eligibility_manager.py 87.70% <100.00%> (+3.96%) ⬆️
...ograms/models/managers/entitlement_manager_cash.py 64.45% <100.00%> (ø)
spp_programs/models/managers/program_manager.py 86.39% <100.00%> (ø)
spp_programs/models/programs.py 87.81% <100.00%> (+0.34%) ⬆️
spp_change_request_v2/models/change_request.py 84.14% <97.29%> (+0.30%) ⬆️
...ge_request_v2/models/change_request_detail_base.py 73.75% <95.65%> (+4.26%) ⬆️
... and 7 more

... and 44 files with indirect coverage changes

🚀 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant