fix(api+schema): server-side validate schemaSnapshot to close DDL injection (HIGH-1) #14

Merged

flndrn-dev merged 1 commit from fix/security-sqli-schema-apply into main

2026-04-27 11:32:05 +02:00

flndrn-dev commented

2026-04-27 11:26:59 +02:00

(Migrated from github.com)

Summary

Closes a HIGH-severity, cross-tenant DDL/DML injection in POST/PATCH /v1/projects/:id/deployments. The route accepted schemaSnapshot as z.record(z.string(), z.unknown()) and handed it straight to services/schema-apply.ts, which interpolated table names, column names, sqlType, default, FK references, and onDelete into raw CREATE/ALTER/DROP DDL run by tx.unsafe(...) on the shared data-plane superuser connection. The schema package's isIdentifier check only fires inside the user-facing CLI builders, so a crafted JSON payload from any authenticated project owner could break out of an identifier or default literal and execute arbitrary DDL/DML across every tenant's schema.

Two-part fix:

New @briven/schema/wire.ts — Zod schema for the on-the-wire snapshot shape with strict refinements:
- every table/column/index/FK identifier matches IDENTIFIER_RE
- table names cannot start with the reserved _briven_ prefix
- sqlType is allowlisted (text|integer|bigint|boolean|timestamptz|jsonb|uuid|varchar(N)|vector(N))
- default is allowlisted (null/bool/numeric/'literal'/fn()/current_*)
- onDelete is the existing enum; version is a literal 1
- all object shapes are .strict() so unknown keys fail closed
- exported via the package index, so it travels through the existing @briven/cli/schema sub-export with no extra plumbing.
apps/api/src/routes/deployments.ts — both createSchema (POST) and patchSchema (PATCH /latest) replace the permissive z.record(z.string(), z.unknown()) with schemaSnapshotSchema. Malformed snapshots now return a structured 400 with Zod issues instead of reaching tx.unsafe.
apps/api/src/services/schema-apply.ts — the _briven_migrations insert switched from string-concat + single-quote escape to bound-parameter tx.unsafe(query, [deploymentId, deploymentId, json]). The values are server-generated today, but the raw interpolation here was a foot-gun for any future caller.

Verification

13 new wire-validator tests cover every injection vector (table-name quote breakout, column-name breakout, sqlType breakout, default literal breakout, reserved-prefix bypass, FK identifier breakout, onDelete enum bypass, index-column breakout, version mismatch); legitimate snapshots — including parametric varchar(N)/vector(N) types and the full safe-default vocabulary — still parse.
pnpm --filter @briven/schema test: 18/18 pass (5 existing + 13 new)
pnpm --filter @briven/api test: 9/9 pass
pnpm -w typecheck: 15/15 successful
pnpm -w build: 11/11 successful (CLI tarball still bundles)

Rebased on top of fix/security-hardening-phase-0 (#13) cleanly — only the import block in deployments.ts conflicted, resolved by keeping phase-0's env import removal (since ipHash no longer reads the pepper here) and adding the new schemaSnapshotSchema import.

Test plan

CI green
Smoke: POST /v1/projects/:id/deployments with a malicious table name like x" (id text); DROP SCHEMA "proj_other" CASCADE; -- → 400 validation_failed, no DDL run
Smoke: POST a legitimate snapshot → 201 + schema applied
Verify _briven_migrations insert uses bound params (check via EXPLAIN/server log)

🤖 Generated with Claude Code

## Summary Closes a HIGH-severity, cross-tenant DDL/DML injection in `POST/PATCH /v1/projects/:id/deployments`. The route accepted `schemaSnapshot` as `z.record(z.string(), z.unknown())` and handed it straight to `services/schema-apply.ts`, which interpolated table names, column names, sqlType, default, FK references, and onDelete into raw `CREATE/ALTER/DROP` DDL run by `tx.unsafe(...)` on the **shared data-plane superuser** connection. The schema package's `isIdentifier` check only fires inside the user-facing CLI builders, so a crafted JSON payload from any authenticated project owner could break out of an identifier or default literal and execute arbitrary DDL/DML across every tenant's schema. Two-part fix: 1. **New `@briven/schema/wire.ts`** — Zod schema for the on-the-wire snapshot shape with strict refinements: - every table/column/index/FK identifier matches `IDENTIFIER_RE` - table names cannot start with the reserved `_briven_` prefix - `sqlType` is allowlisted (`text|integer|bigint|boolean|timestamptz|jsonb|uuid|varchar(N)|vector(N)`) - `default` is allowlisted (null/bool/numeric/`'literal'`/`fn()`/`current_*`) - `onDelete` is the existing enum; `version` is a literal `1` - all object shapes are `.strict()` so unknown keys fail closed - exported via the package index, so it travels through the existing `@briven/cli/schema` sub-export with no extra plumbing. 2. **`apps/api/src/routes/deployments.ts`** — both `createSchema` (POST) and `patchSchema` (PATCH /latest) replace the permissive `z.record(z.string(), z.unknown())` with `schemaSnapshotSchema`. Malformed snapshots now return a structured 400 with Zod issues instead of reaching `tx.unsafe`. 3. **`apps/api/src/services/schema-apply.ts`** — the `_briven_migrations` insert switched from string-concat + single-quote escape to bound-parameter `tx.unsafe(query, [deploymentId, deploymentId, json])`. The values are server-generated today, but the raw interpolation here was a foot-gun for any future caller. ## Verification - 13 new wire-validator tests cover every injection vector (table-name quote breakout, column-name breakout, sqlType breakout, default literal breakout, reserved-prefix bypass, FK identifier breakout, onDelete enum bypass, index-column breakout, version mismatch); legitimate snapshots — including parametric `varchar(N)`/`vector(N)` types and the full safe-default vocabulary — still parse. - `pnpm --filter @briven/schema test`: 18/18 pass (5 existing + 13 new) - `pnpm --filter @briven/api test`: 9/9 pass - `pnpm -w typecheck`: 15/15 successful - `pnpm -w build`: 11/11 successful (CLI tarball still bundles) Rebased on top of `fix/security-hardening-phase-0` (#13) cleanly — only the import block in `deployments.ts` conflicted, resolved by keeping phase-0's `env` import removal (since `ipHash` no longer reads the pepper here) and adding the new `schemaSnapshotSchema` import. ## Test plan - [ ] CI green - [ ] Smoke: POST `/v1/projects/:id/deployments` with a malicious table name like `x" (id text); DROP SCHEMA "proj_other" CASCADE; --` → 400 `validation_failed`, no DDL run - [ ] Smoke: POST a legitimate snapshot → 201 + schema applied - [ ] Verify `_briven_migrations` insert uses bound params (check via `EXPLAIN`/server log) 🤖 Generated with [Claude Code](https://claude.com/claude-code)