Skip to content

feat: enforce tenant scoping on sqlc queries with sqlclint - #5061

Draft
disintegrator wants to merge 1 commit into
mainfrom
slqc-idor-lint
Draft

feat: enforce tenant scoping on sqlc queries with sqlclint#5061
disintegrator wants to merge 1 commit into
mainfrom
slqc-idor-lint

Conversation

@disintegrator

@disintegrator disintegrator commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Adds sqlclint, a linter that checks every production sqlc query is bounded to a tenant, and gates it in CI. Draft: the tooling is complete and verified; the audit of existing violations is barely started.

Why

.agents/skills/postgresql/SKILL.md has required every query to be scoped to a tenant for a long time, but nothing checked it. The invariant survived on reviewer attention and hand-written prose, which is a weak control for what it prevents: a query addressable by row id alone lets any caller who learns or guesses an id read or write another tenant's data.

Reviewing 1,306 queries by eye does not scale, and the cases most likely to be missed are the ones least likely to look wrong:

  • Binding organization_id on a table with a non-nullable project_id reads as scoped in review and crosses every project in the org.
  • Binding a nullable project_id looks identical to a non-nullable bind at the Go call site (both are *uuid.UUID) and silently drops rows whose project is NULL.

Neither is visible without resolving the query against the schema.

How it decides

Per table, in order:

  1. project_id is NOT NULL → require project_id.
  2. Otherwise organization_id is NOT NULL → require organization_id.
  3. Otherwise → require either column the table has.

A table with neither column inherits its parents' requirement through its foreign keys. A table that reaches no tenancy-bearing table is global and requires nothing. Across schema.sql: 171 tables — 83 require project_id, 43 organization_id, 4 either, 41 inherit or are global.

sqlc.narg never satisfies a requirement. A nullable tenancy parameter either matches nothing or, in the (@x IS NULL OR col = @x) idiom, removes the boundary entirely when NULL is passed.

Parsing, not matching

Queries are parsed with wasilibs/go-pgquery, a pure-Go (wazero/WASM, no cgo) build of libpg_query — the same parser sqlc itself uses, at the same version. sqlc's own wrapper is behind Go's internal/ rule, but the grammar underneath it is importable, so sqlclint sees exactly what sqlc sees: @name as a unary @ expression, sqlc.arg/sqlc.narg as schema-qualified function calls, $1 as a parameter reference. No preprocessing.

This is not a purity argument. Three things a text matcher gets wrong, all found by walking the real AST:

  • UPDATE/DELETE/INSERT hold their target table in a typed relation field, not a Node union — a walk keyed on the node wrapper misses the table every mutation writes to.
  • FOR UPDATE OF t represents the alias t as a RangeVar; 9 queries looked like unknown tables until the walk stopped descending into locking clauses.
  • INSERT ... SELECT supplies columns positionally from the target list; 6 queries binding project_id there read as unscoped until that was handled.

A bound also counts wherever it legitimately appears — WHERE, JOIN ... ON, EXISTS, UPDATE ... FROM, a CTE, an INSERT column list. Requiring a particular position would reject the correct SQL that scopes a child table through its parent.

Rule catalog

18 embedded markdown documents, 10 diagnostics and 8 exemption categories, with uniform frontmatter and a fixed heading sequence per kind:

sqlclint rules                       # id, kind, summary
sqlclint rules --kind exemption
sqlclint rule missing-tenant-scope   # full description

Diagnostics print see: sqlclint rule <id>. A conformance test checks both directions, so a diagnostic cannot exist without a document and a document cannot be orphaned. The exemption vocabulary is exactly the set of exemption documents, so a category cannot be invented at a call site.

Queries that genuinely cannot be bounded carry an annotation naming a category and a reason:

-- name: GetAPIKeyByKeyHash :one
-- sqlclint:ignore token-keyed -- key_hash holds a SHA-256 of a high-entropy API
-- key; this lookup is what resolves the organization, so no tenant is known yet
SELECT * FROM api_keys WHERE key_hash = @key_hash AND deleted IS FALSE;

Side effect worth knowing: sqlc carries these into the generated Go doc comment, so the justification is visible at every call site.

The ratchet

.sqlclintignore grandfathers the 304 violations that predate the check. It is generated debt, not approval. Each entry is pinned to a hash of the query body, so editing a grandfathered query re-raises it — an exemption cannot follow a name while the SQL underneath it changes. Stale entries also fail, so the file can only shrink. Structural problems (bad category, missing reason, unparseable query, unresolvable table) are never grandfathered.

What is NOT done

The audit: 2 of 116. Only SetSyncScheduleDisabled and RetrySyncSchedule are annotated, after tracing their callers.

I stopped deliberately rather than bulk-annotating. Both sit in a package full of background pollers and look like background-sweep, but the first is documented as recording "a user's explicit pause" — it is request-reachable. It is safe only because the handler resolves the config id through an organization_id-scoped lookup, making it parent-authorized. Labelling that cluster background-sweep would have documented a request-reachable unscoped write as a worker job. Each of the remaining 114 needs the same per-query caller trace; .sqlclintignore is the worklist.

Findings worth a look now

  • CloneDeploymentToolFunctions (server/internal/deployments/queries.sql) clones tool definitions keyed only on deployment_id, copying project_id from the source row. A caller passing another project's deployment id clones into that project. This reads as a real IDOR, not an exemption.
  • ~25 queries scope org-only against tables carrying a non-nullable project_id (ai_integration_configs and similar), where the natural key genuinely looks like (organization_id, provider). That is a schema-versus-rule question — either the queries should bind project_id or the column should be reconsidered — and I did not want to settle it by inventing a ninth exemption category.

Known limitation

The check proves a tenancy predicate exists and is parameterized. It cannot prove the parameter carries the authenticated tenant rather than an attacker-supplied one from the request payload. Closing that needs Go-side taint analysis in glint/ tracking contextvalues.AuthContext into repo call arguments. Out of scope here, and stated in the skill so the lint is not read as a complete IDOR guarantee.

Verification

  • mise run lint:queries → clean; --write-ignore-file regenerates.
  • New unscoped query fails; editing a grandfathered body fires modified-ignored-query; an invalid category lists the valid ids; stale entries fail; annotate → regenerate drops the count.
  • Positive controls: the 17 JOIN ... ON-only and ~102 EXISTS/subquery-scoped queries all pass.
  • Config equivalence: all-flags run and sqlclint.yaml-only run produce identical output.
  • mise run test:ci (73 tests, now covering ./sqlclint/...), go build ./..., go mod tidy clean.
  • mise run gen:sqlc-server regenerates with only the two annotation comments changing.

CI runs lint-queries on the queries path filter and always in the merge queue, mirroring lint-migrations.

🤖 Generated with Claude Code


Summary by cubic

Add sqlclint, an AST-based linter that enforces tenant scoping on all production sqlc queries and gates it in CI. Existing violations are tracked in a ratcheting .sqlclintignore, and genuine exceptions require explicit annotations with reasons.

  • New Features

    • Enforces per-table scoping: require project_id if NOT NULL, else organization_id, else either; inherits via FKs; global tables allowed.
    • Parses queries via libpg_query (same grammar as sqlc) and detects bounds across WHERE, JOIN ... ON, EXISTS, CTEs, UPDATE ... FROM, and INSERT ... SELECT.
    • Flags sqlc.narg for tenancy binds and wrong-tenant columns.
    • Adds rule catalog and docs; annotations must use known categories and include a reason: -- sqlclint:ignore <category> -- <reason>.
    • Introduces .sqlclintignore pinned to query-body hash; modified or stale entries fail; regenerate with mise run lint:queries -- --write-ignore-file.
    • CLI: sqlclint run (uses sqlclint.yaml) and sqlclint rules; task mise run lint:queries; CI job lint-queries with queries filter and always in merge queue.
    • Documents enforcement in SKILL.md; adds two parent-authorized annotations in aiintegrations.
  • Migration

    • Bind the required tenant column on new and changed queries.
    • If a query truly cannot be scoped, add -- sqlclint:ignore <category> -- <reason>; choose from documented categories.
    • Do not hand-edit .sqlclintignore; run mise run lint:queries -- --write-ignore-file after fixes or annotations.
    • Run mise run lint:queries locally before pushing; CI blocks merges on failures.

Written for commit 9deb2a1. Summary will update on new commits.

Review in cubic

The postgresql skill has required every query to be scoped to a tenant for a
long time, but nothing checked it. The invariant survived on reviewer attention
and hand-written prose, which is a weak control for the failure it prevents: a
query addressable by row id alone lets any caller who learns or guesses an id
read or write another tenant's data.

Reviewing this by eye does not scale to 1,306 queries, and the cases most likely
to be missed are the ones least likely to look wrong. A query that binds
organization_id on a project-scoped table reads as scoped and crosses every
project in the org. One that binds a nullable project_id looks equivalent to a
non-nullable bind at the Go call site and silently drops rows. Neither is
visible without resolving the query against the schema, which is what sqlclint
does.

The check parses with libpg_query, the same grammar sqlc uses, rather than
matching text. That is not a purity argument: a tenancy bound is equally real in
a WHERE clause, a JOIN condition, an EXISTS subquery or an UPDATE ... FROM, and
a text matcher either rejects the correct SQL that scopes a child through its
parent or misses the target table of every mutation, which Postgres holds in a
typed field rather than a node union.

Existing violations are grandfathered rather than fixed here. Each entry is
pinned to a hash of the query body, so the file is a ratchet and not a
suppression list: editing a grandfathered query re-raises it, and there is no
way to widen the debt without the diff showing it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@changeset-bot

changeset-bot Bot commented Aug 7, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: 9deb2a1

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@disintegrator disintegrator added enhancement New feature or request go Pull requests that update go code documentation Improvements or additions to documentation labels Aug 7, 2026
@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

atlas migrate lint on server/migrations

Status Step Result
No migration files detected  
ERD and visual diff generated View Visualization
No issues found View Report
Read the full linting report on Atlas Cloud

@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

atlas migrate lint on server/clickhouse/migrations

Status Step Result
No migration files detected  
ERD and visual diff generated View Visualization
No issues found View Report
Read the full linting report on Atlas Cloud

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation enhancement New feature or request go Pull requests that update go code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant