security: staging batch 3 (#272 #342 #341 #262 #269 #325 #326 #335) - #513
security: staging batch 3 (#272 #342 #341 #262 #269 #325 #326 #335)#513gonzalesedwin1123 wants to merge 10 commits into
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## 19.0 #513 +/- ##
==========================================
- Coverage 76.88% 74.51% -2.38%
==========================================
Files 703 624 -79
Lines 45739 43931 -1808
==========================================
- Hits 35166 32733 -2433
- Misses 10573 11198 +625 Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
ci-full verdict on
|
| Job | baseline | staging e428de54 |
diff |
|---|---|---|---|
| SP-MIS Demo | 11 failed, 11 errors of 7386 tests | 11 failed, 11 errors of 7433 tests | test-level failure set identical (2 setUpClass errors: spp_analytics TestAggregationIntegrationDemo, spp_programs TestEntitlementManager in-kind); SQL-error set identical (40); Odoo-error set identical modulo random IDs in messages |
| GRM Demo | 0 failed, 1 error of 3609 | 0 failed, 1 error of 3632 | SQL-error set (31) and Odoo-error set (23) identical |
| DRIMS Sri Lanka Demo | 1 failed, 1 error of 2690 | 1 failed, 1 error of 2707 | SQL-error set (25) and Odoo-error set (15) identical |
The test-count increases (+47 / +23 / +17) are this batch's added tests running in those stacks. Zero failures only present on staging.
Note: this PR is not yet ready to merge — the merge-turn adversarial review returned NEEDS-CHANGES on #262 (two HIGH, fix in progress) and #341 (design; recommendation is to revert it out of this batch). A follow-up comment will land here when the staging head changes; the verdict above will be re-run on the new head.
ci-full verdict on the FINAL head
|
| Job | baseline | staging 3f367f24 |
diff |
|---|---|---|---|
| SP-MIS Demo | 11 failed, 11 errors of 7386 tests | 11 failed, 11 errors of 7434 tests | identical test-level failures (the two pre-existing setUpClass errors in spp_analytics and spp_programs in-kind), identical SQL-error and Odoo-error sets |
| GRM Demo | 0 failed, 1 error of 3609 | 0 failed, 1 error of 3633 | identical SQL/Odoo-error sets |
| DRIMS Sri Lanka Demo | 1 failed, 1 error of 2690 | 1 failed, 1 error of 2710 | identical SQL/Odoo-error sets |
Test-count deltas (+48 / +24 / +20) are the batch's own added tests, including #514's six.
This PR's own CI on 3f367f24: 52/52 green (Trivy skipped as on every PR). Together with the identity sweep, version chain and -u upgrade gate recorded in the description, all merge gates are satisfied.
Merge with a MERGE COMMIT (not squash).
For the human review — what is already verified, and where a second pair of eyes helps mostAlready verified mechanically on Merge-turn adversarial review (nine read-only staff-engineer passes on the staged composition, one per squash plus one on the whole batch) — condensed:
Where a human adds the most value:
Whoever merges: merge commit, not squash — the ten squash commits are the audit trail. |
1 participant
Security staging batch 3: seven reviewed security fixes plus one review-driven fix-up, squash-merged one at a time into
19.0-staging-sec-batch3(cut from19.0ate85e79e2, which is still the tip of19.0, so this PR applies with zero drift andgit merge-treeis conflict-free).⚠ Merge with a MERGE COMMIT, not squash
The ten squash commits below are the reviewed units and the tree-identity audit is keyed on them. Squashing this PR would collapse them and break the post-merge audit. (
allow_merge_commitis on for the repo; batch 2's #422 landed the same way.)Contents (first-parent order, oldest first)
meas CEL context identifier (DCI dotted-validation false positive)search_registrantsgroups=on the stored partner impact indicators and the impact O2M; impact-read check on the affected-registrant action, gated stat button, eligibility method privatisedNet effect: 8 modules, 69 files.
spp_dci_indicatorsgains tests only (version unchanged at 19.0.1.0.2).spp_demoandspp_farmer_registry_demoare byte-identical to19.0.Why #341 is reverted and #514 exists
A merge-turn adversarial review of the staged composition (nine read-only reviewers, record in the internal plans folder
batch3-merge-turn-review.md) returned SHIP-WITH-NITS on six PRs and the composition, and NEEDS-CHANGES on two:res.partnerimpact indicators had no field-levelgroups=, so any internal user could stillsearch_reada per-registrant victim list over RPC; andspp_hazard_programsexposed the impacted-registrant list to any program reader via a stat button and a public method. Fixed in security(hazard): gate stored registrant impact indicators with field-level groups (fix-up for #262) #514 (the review's second claim, that partner creation would break for users without impact read, was refuted: stored computes run as superuser). Verified in Odoo 19 source that fieldgroups=is enforced on domains,order=,read_group, export and relational traversal.ir_module_module.demo, which is always False on OpenSPP's own stack (docker/odoo.conf.templatesetswithout_demo = True) so every evaluation instance lost its demo personas, while stock-Odoo production installs have demo on and keptsppadmin/demolive; the forced-u spp_demoalso reset demo logins and passwords on every upgrade. Reverted (Revert "security(demo): deactivate default-credential demo users on production install + drop Production/Stable (#341)" #515); to be redesigned around an explicit opt-in and re-landed in batch 4 with security(demo): archive default-credential demo users in spp_mis_demo_v2/spp_grm_demo/spp_drims_sl(_demo) #356.Verification on the final head
3f367f24--match-head-commit; each refresh onto staging verified byte-identical to the reviewed change set (fix(cel): recognise 'me' as a context identifier (dotted DCI validation false-positive) #342's five hand-laddered spp_cel_domain metadata files confirmed by CI's pre-commit; Revert "security(demo): deactivate default-credential demo users on production install + drop Production/Stable (#341)" #515 re-verified as the exact inverse after its refresh).19.0; migration dirspp_dci_client_compliance/19.0.1.0.2matches its manifest.-uupgrade gate (PASSED, run 2): DB seeded ate85e79e2with the 8 modules on a production-style install, upgraded to3f367f24: exit 0, 0 errors; alllatest_versions equal the staging manifests; thebase.group_userread row onspp.hazard.impactis gone after upgrade; 2 planted data sources carrying the well-known compliance token (one archived) were purged and a re-keyed one kept (security(dci): purge stale compliance bearer token on upgrade #326's migration). Evidence: internal plansbatch3-upgrade-test-evidence.md.e428de54passed by failing-test-set diff (comment below). Run 34445289320 on3f367f24is in progress; its verdict will be posted as a comment.After merge
Post-merge audit (
--remerge-diffempty, staging tree == merge tree) before the branch auto-deletes; archive tagarchive/sec-batch3-stagingby merge SHA; held-out-PR sweep (#274 stays held until after batch 4); follow-ups from the review filed as issues.