You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Found during the staff review of PR #266 (security(grm): restrict GRM automation rules to GRM staff), out of that PR's scope. This is the same shape as #364 (alert rules evaluated by an elevated cron, fixed by binding evaluation to an owner identity) — and the impact here is larger.
Finding
Three entry points evaluate GRM rule CEL and apply rule actions with elevated rights:
spp_grm_cel/data/ir_cron.xml declares model.check_escalations() hourly with no user_id, so it runs as superuser.
spp_grm/models/grm_ticket.py:542 → ticket.sudo()._on_sla_breach() → :564-567apply_escalations(ticket) — superuser, fired from an SLA compute on ordinary ticket access.
check_escalations (grm_escalation_rule.py:439-444) searches allis_closed = False tickets with record rules bypassed, then per rule evaluates CEL and calls apply_escalation, which writes user_id, team_id, severity, priority, sends mail, and creates cases — all as superuser.
Escalation path
A GRM officer is confined by rule_spp_grm_ticket_officer (spp_grm/security/rules.xml:19-32) to tickets assigned to them or their team. But officers hold create/write on spp.grm.escalation.rule. They create one rule with an empty condition_cel (always matches — grm_escalation_rule.py:185-186), trigger_after_hours = 1, escalate_to_user_id = <themselves>. Within the hour the superuser cron reassigns every open ticket in the database to them, at which point their own record rule grants read and write on all of it.
Secondary oracle: rule matches are observable (escalation applied, counter incremented), so an officer can binary-search hidden field values with conditions like ticket.partner_id.vat == 'X', evaluated under superuser where record rules do not apply.
re-bind in write() when any targeting field changes (condition_cel, trigger_after_hours, escalate_to_*, escalate_severity, escalate_priority, active, sequence);
fail closed when no eval user resolves (do not fall back to the elevated cron);
run check_escalations' ticket search and apply_escalation's writes via with_user(eval_user), scoped to that user's companies.
Ship a post-migration.py backfilling eval_as_user_id = create_uid — and note #364's HIGH-2: a Python default on the new stored field makes _init_column pre-fill existing rows with the upgrade user before post-migration, so drop the default and make the backfill unconditional.
PR #364's implementation is a directly transferable template.
Found during the staff review of PR #266 (
security(grm): restrict GRM automation rules to GRM staff), out of that PR's scope. This is the same shape as #364 (alert rules evaluated by an elevated cron, fixed by binding evaluation to an owner identity) — and the impact here is larger.Finding
Three entry points evaluate GRM rule CEL and apply rule actions with elevated rights:
spp_grm_cel/data/ir_cron.xmldeclaresmodel.check_escalations()hourly with nouser_id, so it runs as superuser.spp_grm/models/grm_ticket.py:542→ticket.sudo()._on_sla_breach()→:564-567apply_escalations(ticket)— superuser, fired from an SLA compute on ordinary ticket access.spp_grm/controllers/grm_portal.py:51→ sudo ticket create →grm_ticket.py:30-39→apply_routing— superuser.check_escalations(grm_escalation_rule.py:439-444) searches allis_closed = Falsetickets with record rules bypassed, then per rule evaluates CEL and callsapply_escalation, which writesuser_id,team_id,severity,priority, sends mail, and creates cases — all as superuser.Escalation path
A GRM officer is confined by
rule_spp_grm_ticket_officer(spp_grm/security/rules.xml:19-32) to tickets assigned to them or their team. But officers hold create/write onspp.grm.escalation.rule. They create one rule with an emptycondition_cel(always matches —grm_escalation_rule.py:185-186),trigger_after_hours = 1,escalate_to_user_id = <themselves>. Within the hour the superuser cron reassigns every open ticket in the database to them, at which point their own record rule grants read and write on all of it.Secondary oracle: rule matches are observable (escalation applied, counter incremented), so an officer can binary-search hidden field values with conditions like
ticket.partner_id.vat == 'X', evaluated under superuser where record rules do not apply.Suggested fix (mirror #364)
Add a system-managed
eval_as_user_idto both rule models:create()(neverpop()— see fix(spp_alerts): evaluate alert rules as their owner, not the elevated cron #364's HIGH-1: popping letsdefault_<field>in the context forge it viadefault_get);write()when any targeting field changes (condition_cel,trigger_after_hours,escalate_to_*,escalate_severity,escalate_priority,active,sequence);check_escalations' ticket search andapply_escalation's writes viawith_user(eval_user), scoped to that user's companies.Ship a
post-migration.pybackfillingeval_as_user_id = create_uid— and note #364's HIGH-2: a Pythondefaulton the new stored field makes_init_columnpre-fill existing rows with the upgrade user before post-migration, so drop the default and make the backfill unconditional.PR #364's implementation is a directly transferable template.
Related: #266, #364, #374.