Skip to content

test(permissions): a member limited to owned records deletes only what they or their team own - #4969

Merged
simlarsen merged 5 commits into
masterfrom
claude/owned-scope-monitor-delete
Oct 18, 2026
Merged

simlarsen merged 5 commits into
masterfrom
claude/owned-scope-monitor-delete

Conversation

@simlarsen

@simlarsen simlarsen commented Oct 18, 2026 •

Copy link
Copy Markdown
Contributor

What this holds

A team permission scoped to Owned "only applies to resources where this team, or the user acting, is listed as an owner" (Permissions docs, Scope). A delete has to keep to that exactly as a list and an update do: a member whose monitor permissions are all scoped to Owned deletes the monitors they or one of their teams own, and no other.

Master already does this. Since #4526 a delete is narrowed by the same record rule as a read and an update (BasePermission.addRecordScopeToQuery, asked by DeletePermission.checkDeletePermission). Since #4552 nobody can add themselves or their team as an owner of a record they may not read. But no test held a delete of an owned-scope record end to end, by id, through the routes. This PR adds those tests. Tests only: no product change.

Tests added

OwnedRecordsDeletePostgres (100 tests, new step in Postgres Schema Drift)

Every request goes through the route, the real user middleware (it reads the member's teams, permission rows, custom role copies and API keys from the tables), the real MonitorService with its delete hooks, and the real permission pipeline, query serializer and TypeORM, on a migrated Postgres. Only the session token, caches, SSO lookups, audit trail, workflow and realtime triggers, and Valkey locks are stood in for.

  • Each grant a member may hold scoped to Owned: Monitor Admin, Monitor Member, Read + Delete Monitor, Project Member, the All Operational Resources read and delete permissions, and a custom role's Monitor Admin (the copies written by CustomRoleSync). For each one:
    • a monitor they or their team own is deleted, both by POST /monitor/:id/delete-item and by DELETE /monitor/:id;
    • a monitor owned by another user, by another team or by nobody is answered 404 Monitor not found. on both routes, and stays with its owner rows;
    • the list holds only owned monitors;
    • MonitorService.deleteBy and hardDeleteBy with the member's own props remove only the owned monitors.
  • Grants of two scopes:
    • a delete scoped to Owned beside a read of every monitor: other monitors are refused with "You do not have permission to delete this monitor." (the member may read them);
    • a delete of every monitor beside a read scoped to Owned: other monitors are answered 404, because a delete needs the read;
    • a grant over the whole project beside the owned one: every monitor is reached (the broader grant wins, as the docs say).
  • Becoming an owner first: adding themselves (/monitor-owner-user) or their team (/monitor-owner-team) as an owner of a monitor they may not read is refused, and no owner row is written. Moving an owner row of their own monitor onto another monitor is refused. Removing another team's owners of a monitor they may not read is refused. The monitor still can't be deleted.
  • Records of a monitor or a status page: a status timeline entry or a monitor test is deleted only on an owned monitor. An announcement or an announcement template is deleted only when one of its status pages is owned (on two pages, on none, on others).
  • Whoever else asks:
    • a viewer of every monitor deletes none;
    • a member of another project deletes none;
    • a request without a session deletes nothing;
    • another project's monitor is answered 404;
    • a delete across projects (is-multi-tenant-query) is refused even for an owned monitor;
    • an API key holds no scope (docs: keys do not support Owned), so its grant reaches every monitor.
  • Every other kind with owners of its own (the 26 other OwnerTableRegistry kinds: incidents, alerts, status pages, dashboards, services, hosts, clusters, databases, queues, workflows…), through BaseAPI and a plain service: by id, owned records go and others are answered 404 and stay; by query, only owned records go. Rows are filled from the migrated schema's own column list, so a new kind is swept without editing the suite.

OwnedRecordsDeleteEveryKind (294 tests, Common Test)

For every kind the Owned scope narrows, it checks the query DeletePermission.checkDeletePermission runs with, for a member holding every scope-limitable delete and read permission of that kind (and the read of the record it is read through), all scoped to Owned. The query reaches the owned record and not another. The same grants over the whole project reach every record. A kind that no scope-limitable permission deletes is refused. The kinds swept:

  • the 27 kinds with owner tables;
  • the @OwnedThrough records, through the record they belong to;
  • the @CanAccessIfCanReadOn records read through a kind with owners, through that record (its owner rows among them).

The two kinds read through a list of status pages are pinned exactly, with their Postgres test above. New kinds join the sweep automatically.

Proof the tests catch a regression (mutation checks, each restored)

  • Put back the old delete rule (DeletePermission narrowed by labels only): 74/100 Postgres tests and 131/294 guard tests fail.
  • Make the owner narrowing a no-op (OwnedScopePermission.addOwnedRecordsToQuery): 83/100 Postgres tests (lists and hard deletes included) and 74/294 guard tests fail.
  • Skip the parent read on create (CreatePermission.checkParentPermission): exactly the two "becoming an owner first" tests fail.

What changes for existing customers

Nothing. Tests and a CI step only.

What I ran

  • Both suites on a throwaway Postgres 15 with every registered migration (100/100 and 294/294), before and after merging master.
  • The guard with billing off, and with CI's billing settings (BILLING_ENABLED=true and the plan definitions from test-setup.sh).
  • The guard with ee/ hidden.
  • npm run compile-tests in packages/Common (all three programs).
  • npx jest Guard in packages/Common (132 suites).
  • Tests/Ops (47 suites, including PostgresSuitesRunInCi).
  • prettier and eslint on every changed file; the branch ReDoS checker (one new, linear pattern).

No failures seen on master in what I ran.

Review

A second pass took the branch and tried to get a monitor delete past the Owned scope some other way, on a freshly migrated Postgres through the real routes.

What I tried, and what holds on master:

  • A member who reads more monitors than they manage. The common "see every monitor, manage your own" shape - an Owned-scoped delete (Monitor Admin, Monitor Member, or Delete Monitor) beside a broader read (Monitor Viewer over the project, the Viewer role, Read Monitor, or a read limited to a label) - still deletes only the monitors the member or their team own, by POST /monitor/:id/delete-item, by DELETE /monitor/:id, and by deleteBy query. A monitor they may read but not own is refused ("You do not have permission to delete this monitor."). Covered for a custom role's copies too. These are the tests added in this pass (10), and both mutation checks catch them: with the old labels-only delete rule, and with the owner narrowing made a no-op, all ten fail.
  • Other callers and shapes were already covered by the suite and stay refused: a viewer, a member of another project, no session, another project's monitor, a delete across projects, and an API key's scope.

Follow-up (not a change here, pre-existing on master): who may add or remove a resource's owners is gated on reading the resource, not yet narrowed by the same Owned scope. So a member whose management grant is Owned-scoped but who can read more monitors can still name themselves an owner of a monitor they do not own. This is a change to owner-management scope across every kind that carries owners - a product decision of its own - so it is tracked as a separate hardening task, not folded into this tests-only change.

All 110 Postgres tests and 294 guard tests pass before and after merging master (billing on and off for the guard); compile-tests is clean; npx jest Guard is 132 suites green; prettier, eslint and the ReDoS check are clean.

🤖 Generated with Claude Code

valeriamustafaeva and others added 3 commits October 18, 2026 01:34
…t they or their team own

A delete by a member whose grants are scoped to Owned keeps to the records
they or one of their teams own, as a list and an update do. Two suites hold
that:

- OwnedRecordsDeleteEveryKind (Common Test): for every kind the Owned
  scope narrows - the kinds with owners of their own, the records that
  belong to one, and the records read through one - the query a delete runs
  with reaches the owned records only, and the same grants over the whole
  project reach every record.
- OwnedRecordsDeletePostgres (Postgres Schema Drift): through the monitor
  routes, the real user middleware and MonitorService's hooks on a migrated
  Postgres - by id (both routes) and by query, for every grant a member may
  hold scoped to Owned (a custom role's included), mixed scopes, owner rows
  that cannot be added to or taken from a monitor the member may not read,
  a monitor's status timeline and tests, a status page's announcements and
  templates, the requests of a viewer, another project, no session, across
  projects and an API key, and every other kind with owners of its own.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…hey hold, formatted

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@simlarsen

simlarsen commented Oct 18, 2026 •

Copy link
Copy Markdown
Contributor Author

✅ Snyk checks have passed. No issues have been found so far.

Status Scan Engine Critical High Medium Low Total (0)
✅ Open Source Security 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

valeriamustafaeva and others added 2 commits October 18, 2026 02:26
…y the ones they own

A grant scoped to Owned beside a broader read - Monitor Viewer over the
whole project, the Viewer role, Read Monitor, or a read limited to a label
- is a common 'see everything, manage your own' shape. The owned-records
delete suite now holds that shape too: through the routes and by query, a
member who may read a monitor they do not own is still refused its delete,
and deletes only the monitors they or their team own. Covered for a custom
role's copies as well.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@simlarsen
simlarsen merged commit 1ddd048 into master Oct 18, 2026
5 of 62 checks passed
@simlarsen
simlarsen deleted the claude/owned-scope-monitor-delete branch October 18, 2026 02:39
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