Follow-up discovered while fixing the Force Unlock authorization gap (PR #336). Deliberately left out of that PR's scope because the correct fix is different in kind and higher-risk.
Severity: Low–Medium (residual lock-clearing bypass)
Problem
PR #336 gates the manual escape hatch action_force_unlock to base.group_system. But is_locked on spp.cycle / spp.program is also cleared by the async completion callbacks on the manager models:
spp_programs/models/managers/cycle_manager_base.py — mark_import_as_done/_failed, mark_prepare_entitlement_as_done/_failed, mark_check_eligibility_as_done/_failedspp_programs/models/managers/program_manager.py — mark_enroll_eligible_as_done/_failedspp_programs/models/managers/payment_manager.py — mark_job_as_done/_failedspp_programs/models/managers/entitlement_manager_base.py — mark_job_as_done/_failed
These are public methods and reachable via RPC/call_kw. A user with access to the manager model and the target cycle could call e.g. mark_import_as_done(cycle, "…") directly to clear an active operation lock — the same effect PR #336 blocks on action_force_unlock, via a different door.
Why it was not fixed in PR #336
These callbacks must run in the initiating (non-admin) user's context — the async job executes as whoever started the operation, and the callback is how the pipeline releases its own lock. Applying the base.group_system guard used for action_force_unlock would break normal operation. The correct fix is a different design:
- a job-identity / one-shot token so only the real on_done/on_error callback can invoke the unlock, or
- moving the unlock into a trusted callback context not directly RPC-addressable, or
- splitting "do the completion work" from "release the lock" so the release is not a standalone public method.
Proposed
Decide on one of the above and implement it as its own reviewed change, with tests that a non-admin cannot clear a lock by calling a mark_* method out of band while still allowing the genuine job callback to release it.
Related: #336.
Follow-up discovered while fixing the Force Unlock authorization gap (PR #336). Deliberately left out of that PR's scope because the correct fix is different in kind and higher-risk.
Severity: Low–Medium (residual lock-clearing bypass)
Problem
PR #336 gates the manual escape hatch
action_force_unlocktobase.group_system. Butis_lockedonspp.cycle/spp.programis also cleared by the async completion callbacks on the manager models:spp_programs/models/managers/cycle_manager_base.py—mark_import_as_done/_failed,mark_prepare_entitlement_as_done/_failed,mark_check_eligibility_as_done/_failedspp_programs/models/managers/program_manager.py—mark_enroll_eligible_as_done/_failedspp_programs/models/managers/payment_manager.py—mark_job_as_done/_failedspp_programs/models/managers/entitlement_manager_base.py—mark_job_as_done/_failedThese are public methods and reachable via RPC/
call_kw. A user with access to the manager model and the target cycle could call e.g.mark_import_as_done(cycle, "…")directly to clear an active operation lock — the same effect PR #336 blocks onaction_force_unlock, via a different door.Why it was not fixed in PR #336
These callbacks must run in the initiating (non-admin) user's context — the async job executes as whoever started the operation, and the callback is how the pipeline releases its own lock. Applying the
base.group_systemguard used foraction_force_unlockwould break normal operation. The correct fix is a different design:Proposed
Decide on one of the above and implement it as its own reviewed change, with tests that a non-admin cannot clear a lock by calling a
mark_*method out of band while still allowing the genuine job callback to release it.Related: #336.