Skip to content

security(spp_grm_cel): escalation/routing rules evaluate as superuser — GRM officer can seize every ticket [Severity: High] #379

Description

@gonzalesedwin1123

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:542ticket.sudo()._on_sla_breach():564-567 apply_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-39apply_routing — superuser.

check_escalations (grm_escalation_rule.py:439-444) searches all is_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.

Suggested fix (mirror #364)

Add a system-managed eval_as_user_id to both rule models:

  • force-set in create() (never pop() — see fix(spp_alerts): evaluate alert rules as their owner, not the elevated cron #364's HIGH-1: popping lets default_<field> in the context forge it via default_get);
  • 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.

Related: #266, #364, #374.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions