Skip to content

fix(security): no role escalation through user management; query cost limits - #18

Merged
BackStacked merged 1 commit into
mainfrom
claude/project-thread-dvfnaw
Oct 27, 2026
Merged

BackStacked merged 1 commit into
mainfrom
claude/project-thread-dvfnaw

Conversation

@BackStacked

Copy link
Copy Markdown
Contributor

Requested by Himanshu · project thread

Two findings from a local end-to-end security run (api in production mode on PostgreSQL + PgBouncer + Redis + worker). Both were in the 2026-10-14 audit and are still open on main.

1. Privilege escalation through user management (high once a custom role exists)

Before: assertRoles only checked that a role key exists. Reproduced locally with a custom user_manager role (user:read, user:create, user:update, role:read). That user could:

  • createUser with role owner;
  • setUserRoles on itself → owner;
  • setUserPassword on the owner (account takeover, with no role change at all).

After: AuthService refuses (FORBIDDEN) unless the actor holds every permission of the roles involved, in the same branch or organisation-wide (an org-wide grant needs org-wide roles):

  • roles being granted (createUser, setUserRoles);
  • the target user's current roles (setUserRoles, setUserPassword, disableUser, enableUser), so you cannot manage someone above you;
  • the permissions of a role being defined (upsertRole).

Owners (*:*) and admins are unaffected for everything they already hold. A new ForbiddenError in services/util.ts maps to FORBIDDEN (an existing code, so the contract doesn't change).

2. No query cost limit (DoS)

Before: only depthLimit(12). Measured locally: 1,000 aliases of trialBalance took 2.2 s, and 8 aliased invoiceDocument PDFs in one request took 7 s on the event loop (every other request waits while PDFs render).

After: a costLimit rule next to depthLimit (LIMITS in src/server.ts): at most 500 fields per operation (aliases and fragment spreads count), 20 top-level fields and 3 document / export fields. The largest of the client's 141 operations selects ~120 fields with one top-level field, so they all pass (the [static] check validates them). Larger documents still go through startDocumentJob.

Also, when introspection is off, the Did you mean "organization"? hints are removed from validation errors (they spelled out the schema).

Tests

  • New checks in test/run.ts: user-manager cannot mint an owner, promote itself or set the owner's password, but can grant a role it covers; upsertRole needs role:manage; 1,000 aliases, 21 top-level fields and 4 documents refused; no suggestions with introspection off.
  • npm run typecheck passes. TEST_BACKENDS=pg npm test: 211 passed, 0 failed (pg, PgBouncer, replicas, worker, client). MongoDB binaries are blocked in this environment, so CI's Mongo run is the first on Mongo. These changes are in the service and validation layers and don't touch storage.
  • Re-ran my smoke of all 138 root fields and the security probes against a build of this branch: 0 internal errors, and the escalation and alias checks now refuse.
  • Docs updated (02, 05, 06, 11); schema.graphql and docs/04-api-reference.md are unchanged (no SDL change); llms-full.txt regenerated.

Generated by Claude Code

… limits

- createUser / setUserRoles / setUserPassword / disableUser / enableUser / upsertRole
  now require the actor to hold every permission of the roles involved (same
  branch or organisation-wide). A custom role with user:create or user:update
  could mint an owner, promote itself, or set the owner's password.
- costLimit validation rule next to depthLimit: at most 500 fields (aliases and
  fragment spreads count), 20 top-level fields and 3 document/export fields per
  operation. 1,000 aliases of trialBalance took 2.2 s; 8 aliased invoice PDFs
  blocked the event loop for 7 s. The largest client operation has ~120 fields.
- With introspection off, 'Did you mean' hints are stripped from validation errors.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017Bj7Tb5PweikTen79CdVH4
@BackStacked BackStacked self-assigned this Oct 27, 2026
@BackStacked
BackStacked merged commit de9d4e6 into main Oct 27, 2026
1 check passed
@BackStacked
BackStacked deleted the claude/project-thread-dvfnaw branch October 27, 2026 08:51
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.

2 participants