From bbf179503ca5f301c2812059325e5344e91deea0 Mon Sep 17 00:00:00 2001 From: Schalli Date: Wed, 9 Sep 2026 10:50:37 +0200 Subject: [PATCH] fix(quick-260909-eor): forTenant() bindet Mandantenkontext auf dieselbe Verbindung MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit WINDOWS #20: set_config() lief auf einer anderen Postgres-Verbindung als die eigentliche Abfrage, weil die interaktive Callback-Form von $transaction verwendet wurde. Ersetzt durch die Array-Form, die set_config und Abfrage als eine Transaktion auf einer Verbindung ausfuehrt (Prismas empfohlenes Muster fuer RLS-ueber-Extensions). Injektionsfestigkeit (T-02-05) bleibt ueber ein getaggtes $executeRaw-Template statt $executeRawUnsafe erhalten. - prisma-tenant.extension.spec.ts: prueft die Form des Aufrufs (Array mit zwei Eintraegen, Rueckgabewert ist der zweite Eintrag) ohne laufende Datenbank - rls-scratch-check.mjs: neues Werkzeug, das eine Wegwerf-Datenbank anlegt und live misst — gleiche Backend-Verbindung, gesetzter Kontext, keine Fremdmandanten-Zeilen, keine Zeilen ohne Kontext. Alle 5 Pruefungen bestehen gegen die lokale Datenbank. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01FYZcd3SSmo14QTqWx2KKzU --- apps/api/scripts/rls-scratch-check.mjs | 241 ++++++++++++++++++ .../prisma/prisma-tenant.extension.spec.ts | 116 +++++++++ .../api/src/prisma/prisma-tenant.extension.ts | 66 ++++- 3 files changed, 414 insertions(+), 9 deletions(-) create mode 100644 apps/api/scripts/rls-scratch-check.mjs create mode 100644 apps/api/src/prisma/prisma-tenant.extension.spec.ts diff --git a/apps/api/scripts/rls-scratch-check.mjs b/apps/api/scripts/rls-scratch-check.mjs new file mode 100644 index 0000000..c75205f --- /dev/null +++ b/apps/api/scripts/rls-scratch-check.mjs @@ -0,0 +1,241 @@ +#!/usr/bin/env node +// WINDOWS #20 (260909-eor) — richtet sich eine eigene Wegwerf-Datenbank ein +// und misst dort live, ob das reparierte forTenant()-Muster (Aufgabe 1) +// tatsaechlich das tut, was es behauptet. Aufgabe 2 erweitert dieses +// Werkzeug um einen zweiten Abschnitt fuer die auth_lookup_*-Funktionen. +// +// Ruehrt die Datenbank "tessera" NICHT an (T-EOR-07): der Name der +// Wegwerf-Datenbank ist fest im Werkzeug verdrahtet, nicht ueber eine +// Umgebungsvariable steuerbar, damit ein Tippfehler nicht in der echten +// Datenbank landet. Verbindungsangaben kommen ausschliesslich ueber +// TESSERA_SCRATCH_ADMIN_URL (Verbindung zu einer Wartungsdatenbank wie +// "postgres" mit Rechten, um eine neue Datenbank/Rolle anzulegen und wieder +// abzuraeumen). Ohne diese Variable bricht das Werkzeug mit einer Anleitung +// ab, statt eine Vorgabe zu raten. +// +// Nutzt ausschliesslich @prisma/client (bereits Abhaengigkeit der API) — +// kein neues Paket. Fuer DDL (CREATE DATABASE/ROLE mit festen, im Werkzeug +// hartkodierten Namen) ist Interpolation unvermeidlich, da PostgreSQL +// Identifier nicht parametrisieren kann; es fliesst dabei nirgends +// Nutzereingabe ein. +// +// Dupliziert bewusst das forTenant()-Verbindungsmuster statt die +// TypeScript-Quelle unter apps/api/src zu importieren — dasselbe Vorgehen +// wie im bestehenden apps/api/scripts/rls-preflight.mjs, weil ein reines +// Node-Skript ohne Build-Schritt kein .ts importieren kann. +// +// Meldet je Pruefung eine Zeile und beendet sich mit Rueckgabewert 1, sobald +// eine Pruefung scheitert. Gibt kein Kennwort und keine vollstaendige +// Verbindungszeichenkette aus. + +import { PrismaClient } from '@prisma/client'; + +const ADMIN_ENV_VAR = 'TESSERA_SCRATCH_ADMIN_URL'; +const SCRATCH_DB_NAME = 'tessera_rls_scratch'; +const SCRATCH_ROLE_NAME = 'tessera_rls_scratch_role'; +const SCRATCH_ROLE_PASSWORD = 'scratch_only_local_never_reused'; + +function fail(message) { + console.error(`FEHLER: ${message}`); + process.exit(1); +} + +function parseAdminUrl() { + const raw = process.env[ADMIN_ENV_VAR]; + if (!raw) { + fail( + `${ADMIN_ENV_VAR} ist nicht gesetzt. Beispiel: ` + + `${ADMIN_ENV_VAR}="postgresql://tessera:tessera_dev@172.19.0.2:5432/postgres" ` + + `node apps/api/scripts/rls-scratch-check.mjs`, + ); + } + return raw; +} + +function urlForDatabase(adminUrl, dbName) { + const url = new URL(adminUrl); + url.pathname = `/${dbName}`; + return url; +} + +async function withAdminPrisma(adminUrl, fn) { + const prisma = new PrismaClient({ datasourceUrl: adminUrl }); + try { + return await fn(prisma); + } finally { + await prisma.$disconnect(); + } +} + +async function setupScratchDatabase(adminUrl) { + await withAdminPrisma(adminUrl, async (admin) => { + await admin.$executeRawUnsafe( + `SELECT pg_terminate_backend(pid) FROM pg_stat_activity WHERE datname = '${SCRATCH_DB_NAME}' AND pid <> pg_backend_pid()`, + ); + await admin.$executeRawUnsafe(`DROP DATABASE IF EXISTS ${SCRATCH_DB_NAME}`); + await admin.$executeRawUnsafe(`DROP ROLE IF EXISTS ${SCRATCH_ROLE_NAME}`); + await admin.$executeRawUnsafe(`CREATE DATABASE ${SCRATCH_DB_NAME}`); + await admin.$executeRawUnsafe( + `CREATE ROLE ${SCRATCH_ROLE_NAME} WITH LOGIN NOSUPERUSER NOBYPASSRLS NOCREATEDB NOCREATEROLE PASSWORD '${SCRATCH_ROLE_PASSWORD}'`, + ); + }); + + const scratchAdminUrl = urlForDatabase(adminUrl, SCRATCH_DB_NAME).toString(); + await withAdminPrisma(scratchAdminUrl, async (db) => { + await db.$executeRawUnsafe(` + CREATE TABLE probe ( + id serial PRIMARY KEY, + "tenantId" text NOT NULL, + label text NOT NULL + ); + `); + await db.$executeRawUnsafe(` + CREATE OR REPLACE FUNCTION current_tenant_id() RETURNS TEXT AS $$ + SELECT current_setting('app.current_tenant', true); + $$ LANGUAGE sql STABLE; + `); + await db.$executeRawUnsafe(`ALTER TABLE probe ENABLE ROW LEVEL SECURITY;`); + await db.$executeRawUnsafe(`ALTER TABLE probe FORCE ROW LEVEL SECURITY;`); + await db.$executeRawUnsafe(` + CREATE POLICY tenant_isolation_policy ON probe + USING ("tenantId" = current_tenant_id()); + `); + await db.$executeRawUnsafe(`GRANT USAGE ON SCHEMA public TO ${SCRATCH_ROLE_NAME}`); + await db.$executeRawUnsafe( + `GRANT SELECT, INSERT, UPDATE, DELETE ON probe TO ${SCRATCH_ROLE_NAME}`, + ); + await db.$executeRawUnsafe( + `GRANT USAGE, SELECT ON ALL SEQUENCES IN SCHEMA public TO ${SCRATCH_ROLE_NAME}`, + ); + await db.$executeRawUnsafe( + `GRANT EXECUTE ON FUNCTION current_tenant_id() TO ${SCRATCH_ROLE_NAME}`, + ); + await db.$executeRawUnsafe( + `INSERT INTO probe ("tenantId", label) VALUES ('TENANT-A', 'a-row'), ('TENANT-B', 'b-row')`, + ); + }); +} + +async function teardownScratchDatabase(adminUrl) { + await withAdminPrisma(adminUrl, async (admin) => { + await admin.$executeRawUnsafe( + `SELECT pg_terminate_backend(pid) FROM pg_stat_activity WHERE datname = '${SCRATCH_DB_NAME}' AND pid <> pg_backend_pid()`, + ); + await admin.$executeRawUnsafe(`DROP DATABASE IF EXISTS ${SCRATCH_DB_NAME}`); + await admin.$executeRawUnsafe(`DROP ROLE IF EXISTS ${SCRATCH_ROLE_NAME}`); + }); +} + +function report(results, kennung, passed, detail) { + const status = passed ? 'bestanden' : 'FEHLGESCHLAGEN'; + console.log(`${kennung}: ${status} — ${detail}`); + results.push({ kennung, passed, detail }); +} + +/** + * Repliziert exakt das reparierte forTenant()-Muster aus + * apps/api/src/prisma/prisma-tenant.extension.ts: set_config und die + * eigentliche Abfrage als Array-Form von $transaction, also auf einer + * gemeinsamen Verbindung. + */ +async function forTenantQuery(prisma, tenantId, queryFn) { + const setTenantContext = prisma.$executeRaw`SELECT set_config('app.current_tenant', ${tenantId}, true)`; + const [, result] = await prisma.$transaction([setTenantContext, queryFn(prisma)]); + return result; +} + +/** + * Aufgabe 1 — misst die fuenf im Plan genannten Verhaltensweisen von + * forTenant() unter der Rolle ohne BYPASSRLS. + */ +async function runForTenantChecks(scratchRoleUrl, results) { + const prisma = new PrismaClient({ datasourceUrl: scratchRoleUrl }); + + try { + // 1+2: gleiche Verbindung UND gesetzter Kontext — als zwei Teilmessungen + // einer einzigen Array-Transaktion, im selben Format wie die urspruengliche + // Fehlerreproduktion (Backend-PID beim set_config-Schritt vs. Backend-PID + // bei der eigentlichen Abfrage; siehe Kopfkommentar von + // prisma-tenant.extension.ts: "inside tx"/"actual qry"). + const [setStepRow, queryStepRow] = await prisma.$transaction([ + prisma.$queryRaw`SELECT pg_backend_pid() AS pid, set_config('app.current_tenant', 'TENANT-A', true) AS applied`, + prisma.$queryRaw`SELECT pg_backend_pid() AS pid, current_tenant_id() AS t`, + ]).then(([setRows, queryRows]) => [setRows[0], queryRows[0]]); + + report( + results, + 'gleiche-backend-verbindung', + setStepRow.pid === queryStepRow.pid, + `set_config-Schritt pg_backend_pid()=${setStepRow.pid}, Abfrage-Schritt pg_backend_pid()=${queryStepRow.pid}`, + ); + report( + results, + 'mandantenkontext-waehrend-abfrage-gesetzt', + queryStepRow.t === 'TENANT-A', + `current_tenant_id() waehrend der eigentlichen Abfrage=${JSON.stringify(queryStepRow.t)}`, + ); + + // 3+4: forTenant(A) liefert ausschliesslich Zeilen von A, keine von B. + const rowsForA = await forTenantQuery(prisma, 'TENANT-A', (tx) => + tx.$queryRaw`SELECT "tenantId" FROM probe ORDER BY id`, + ); + const onlyA = rowsForA.length > 0 && rowsForA.every((r) => r.tenantId === 'TENANT-A'); + report( + results, + 'nur-eigene-mandanten-zeilen', + onlyA, + `forTenant(TENANT-A) liefert ${rowsForA.length} Zeile(n): ${JSON.stringify(rowsForA.map((r) => r.tenantId))}`, + ); + + const leaksB = rowsForA.some((r) => r.tenantId === 'TENANT-B'); + report( + results, + 'keine-fremdmandanten-zeilen', + !leaksB, + leaksB ? 'Zeile von TENANT-B sichtbar unter forTenant(TENANT-A)' : 'keine Zeile von TENANT-B sichtbar', + ); + + // 5: ungebundener Zugriff derselben Rolle liefert null Zeilen. + const unbound = await prisma.$queryRaw`SELECT "tenantId" FROM probe`; + report( + results, + 'ungebunden-liefert-null-zeilen', + unbound.length === 0, + `ungebundener SELECT liefert ${unbound.length} Zeile(n)`, + ); + } finally { + await prisma.$disconnect(); + } +} + +async function main() { + const adminUrl = parseAdminUrl(); + const results = []; + + console.log(`Richte Wegwerf-Datenbank "${SCRATCH_DB_NAME}" ein...`); + await setupScratchDatabase(adminUrl); + + try { + const scratchRoleUrl = urlForDatabase(adminUrl, SCRATCH_DB_NAME); + scratchRoleUrl.username = SCRATCH_ROLE_NAME; + scratchRoleUrl.password = SCRATCH_ROLE_PASSWORD; + + await runForTenantChecks(scratchRoleUrl.toString(), results); + } finally { + console.log(`Raeume Wegwerf-Datenbank "${SCRATCH_DB_NAME}" ab...`); + await teardownScratchDatabase(adminUrl); + } + + const allPassed = results.every((r) => r.passed); + console.log( + allPassed + ? `Alle ${results.length} Pruefungen bestanden.` + : `${results.filter((r) => !r.passed).length} von ${results.length} Pruefungen fehlgeschlagen.`, + ); + process.exit(allPassed ? 0 : 1); +} + +main().catch((err) => { + console.error('FEHLER beim Ausfuehren der Wegwerf-Pruefung:', err.message); + process.exit(1); +}); diff --git a/apps/api/src/prisma/prisma-tenant.extension.spec.ts b/apps/api/src/prisma/prisma-tenant.extension.spec.ts new file mode 100644 index 0000000..b761e1b --- /dev/null +++ b/apps/api/src/prisma/prisma-tenant.extension.spec.ts @@ -0,0 +1,116 @@ +import { readFileSync } from 'node:fs'; +import { join } from 'node:path'; +import { describe, expect, it, vi } from 'vitest'; +import { forTenant } from './prisma-tenant.extension'; + +/** + * Prueft ohne laufende Datenbank die FORM des Aufrufs, nicht seinen mit + * Produktionscode erzeugten Inhalt (WINDOWS #20, Lehre aus dem + * tautologischen Test in STATE.md — Pitfall "Tautologischer Test"): ein + * vorgetaeuschter Client zeichnet auf, dass `$transaction` mit einem Feld + * aus zwei Eintraegen aufgerufen wird, dass der Rueckgabewert der zweite + * Eintrag ist, und dass `query` waehrend des Aufbaus dieses Feldes genau + * einmal beruehrt wird. + */ + +const EXTENSION_SOURCE_PATH = join(__dirname, 'prisma-tenant.extension.ts'); + +/** + * Der Kopfkommentar der Datei erklaert absichtlich das ALTE, defekte + * Verhalten (inklusive $executeRawUnsafe und der interaktiven + * $transaction(async ...)-Form) als Beleg fuer die Reparatur. Eine + * Volltextpruefung auf den Quelltext wuerde deshalb faelschlich fehlschlagen + * — Block- und Zeilenkommentare muessen vor der Pruefung des tatsaechlichen + * Codes herausgefiltert werden (gleiches Vorgehen wie in + * auth-lookup-functions.spec.ts fuer die Migrations-SQL). + */ +function stripComments(source: string): string { + return source + .replace(/\/\*[\s\S]*?\*\//g, '') + .replace(/^\s*\/\/.*$/gm, ''); +} + +describe('forTenant() — Array-Form von $transaction (WINDOWS #20)', () => { + it('ruft $transaction mit einem Feld aus genau zwei Eintraegen auf', async () => { + const transactionCalls: unknown[] = []; + const fakeQueryResult = { id: 'row-1' }; + + const fakePrisma: any = { + $transaction: vi.fn((arg: unknown) => { + transactionCalls.push(arg); + expect(Array.isArray(arg)).toBe(true); + expect((arg as unknown[]).length).toBe(2); + return Promise.resolve(['set_config-result', fakeQueryResult]); + }), + $extends: (config: any) => { + // Reproduziert nur den Teil der echten $extends-API, den + // $allOperations braucht — kein echter Prisma-Client noetig. + return { + async __invoke(args: unknown, query: (args: unknown) => unknown) { + return config.query.$allOperations({ args, query }); + }, + }; + }, + $executeRaw: vi.fn((_strings: TemplateStringsArray, ..._values: unknown[]) => { + return 'set-config-promise'; + }), + }; + + const scoped = forTenant(fakePrisma, 'tenant-a') as any; + + let queryCallCount = 0; + const query = (args: unknown) => { + queryCallCount += 1; + return fakeQueryResult; + }; + + const result = await scoped.__invoke({ where: { id: 'row-1' } }, query); + + expect(fakePrisma.$transaction).toHaveBeenCalledTimes(1); + expect(transactionCalls).toHaveLength(1); + expect((transactionCalls[0] as unknown[]).length).toBe(2); + + // Rueckgabewert ist der ZWEITE Eintrag des Felds (der Query-Aufruf), + // nicht das Ergebnis von set_config. + expect(result).toBe(fakeQueryResult); + + // query() wurde beim Aufbau des Felds genau einmal beruehrt. + expect(queryCallCount).toBe(1); + }); + + it('setzt den Mandantenkontext ueber ein getaggtes $executeRaw-Template, nicht ueber zusammengebauten Text', async () => { + const fakePrisma: any = { + $transaction: vi.fn((arg: unknown) => Promise.resolve(['set-config-result', 'query-result'])), + $extends: (config: any) => ({ + async __invoke(args: unknown, query: (args: unknown) => unknown) { + return config.query.$allOperations({ args, query }); + }, + }), + $executeRaw: vi.fn((strings: TemplateStringsArray, ...values: unknown[]) => { + // Ein getaggtes Template liefert ein Array von Textstuecken plus die + // interpolierten Werte getrennt — genau das ist der Beleg dafuer, + // dass der Mandantenwert parametrisiert und nicht in den SQL-Text + // eingebaut wird. + expect(Array.isArray(strings)).toBe(true); + expect(values).toContain('tenant-with-quote-\' OR 1=1'); + return 'set-config-promise'; + }), + }; + + const scoped = forTenant(fakePrisma, "tenant-with-quote-' OR 1=1") as any; + await scoped.__invoke({}, () => 'query-result'); + + expect(fakePrisma.$executeRaw).toHaveBeenCalledTimes(1); + }); + + it('verwendet im tatsaechlichen Code (ohne Kommentare) kein $executeRawUnsafe mehr', () => { + const source = stripComments(readFileSync(EXTENSION_SOURCE_PATH, 'utf-8')); + expect(source).not.toContain('$executeRawUnsafe'); + }); + + it('nutzt im tatsaechlichen Code die Array-Form von $transaction (kein interaktiver async-Callback)', () => { + const source = stripComments(readFileSync(EXTENSION_SOURCE_PATH, 'utf-8')); + expect(source).toMatch(/\$transaction\(\s*\[/); + expect(source).not.toMatch(/\$transaction\(\s*async/); + }); +}); diff --git a/apps/api/src/prisma/prisma-tenant.extension.ts b/apps/api/src/prisma/prisma-tenant.extension.ts index 952a459..9f8230b 100644 --- a/apps/api/src/prisma/prisma-tenant.extension.ts +++ b/apps/api/src/prisma/prisma-tenant.extension.ts @@ -4,20 +4,68 @@ import { PrismaClient } from '@prisma/client'; * Creates a tenant-scoped Prisma client that sets the app.current_tenant * PostgreSQL session variable before every query via RLS. * - * Uses parameterized set_config to prevent SQL injection (T-02-05). + * WARUM DIE ARRAY-FORM VON $transaction PFLICHT IST (WINDOWS #20, gemessen + * 2026-09-09, siehe .planning/quick/260909-eor-.../260909-eor-PLAN.md): + * + * Die vorherige Fassung nutzte die INTERAKTIVE Callback-Form + * (`prisma.$transaction(async (tx) => { await tx.$executeRawUnsafe(...); return query(args); })`) + * und rief `query(args)` — die eigentliche Datenbankoperation — auf dem + * AEUSSEREN `prisma`-Client auf, nicht auf `tx`. Ein Nachbau dieses exakten + * Musters gegen die lokale Datenbank ergab: + * + * inside tx : {"pid":254999,"t":"TENANT-A"} + * actual qry : {"pid":255000,"t":null} + * SAME CONNECTION? false + * + * `set_config('app.current_tenant', ..., true)` mit `local=true` gilt nur + * transaktions- UND verbindungslokal. Die interaktive Callback-Form haelt + * fuer `tx` eine eigene Verbindung; `query(args)` lief auf einer ANDEREN, + * unter Postgres-Poolern austauschbaren Verbindung und sah den Kontext nie. + * Unter einer Rolle ohne BYPASSRLS waere die Folge nicht "zu viele Zeilen", + * sondern NULL Zeilen — die Policy vergleicht gegen NULL. + * + * Die Array-Form (`prisma.$transaction([a, b])`) fuehrt alle Eintraege als + * EINE Transaktion auf EINER Verbindung aus — das ist das von Prisma selbst + * fuer RLS-ueber-Extensions vorgesehene Muster. `query(args)` sieht damit + * denselben Kontext, den `set_config` unmittelbar zuvor auf derselben + * Verbindung gesetzt hat. + * + * GRENZFAELLE — benannter Vorbehalt fuer Etappe 2 (nicht stillschweigend + * uebergangen, siehe 260909-eor-SUMMARY.md): + * + * - Ruft aufrufender Code selbst `$transaction` auf einem mit `forTenant()` + * gebundenen Client auf: `$transaction` ist keine Modell-Operation und + * laeuft NICHT durch `$allOperations`. Der Mandantenkontext wird in einem + * solchen Fall nicht automatisch gesetzt — jede einzelne im + * `$transaction`-Array enthaltene Modell-Operation dispatcht zwar durch + * `$allOperations` (weil sie auf dem extended Client aufgerufen wird) und + * bekommt dadurch ihre EIGENE Ein-Element-Transaktion mit eigenem + * `set_config` — mehrere solche Operationen liefen dann aber auf + * MEHREREN Teiltransaktionen statt einer gemeinsamen, was Atomaritaet + * ueber die gesamte aeussere Transaktion hinweg verletzen kann. Heute + * ruft kein `forTenant()`-Aufrufer eine verschachtelte `$transaction` auf + * (gemessen: alle 9 tatsaechlichen mandantengebundenen Abfragen in + * `ldap.service.ts` sind Einzeloperationen) — vor jedem neuen + * `forTenant()`-Aufruf mit eigener Transaktion in Etappe 2 erneut pruefen. + * - `$queryRaw`/`$executeRaw` DIREKT auf dem gebundenen Client laufen + * weiterhin normal durch `$allOperations` (Prisma behandelt sie wie jede + * andere Operation) und werden daher korrekt an dieselbe Verbindung + * gebunden wie `set_config`. + * + * Parametrisiert ueber ein getaggtes `$executeRaw`-Template (kein + * `$executeRawUnsafe` mit zusammengebautem Text mehr) — die + * Injektionsfestigkeit aus T-02-05 bleibt beim Umbau erhalten. */ export function forTenant(prisma: PrismaClient, tenantId: string) { return prisma.$extends({ query: { $allOperations({ args, query }: { args: any; query: (args: any) => any }) { - return (prisma as any).$transaction(async (tx: any) => { - // Use parameterized query to avoid SQL injection - await tx.$executeRawUnsafe( - `SELECT set_config('app.current_tenant', $1, true)`, - tenantId, - ); - return query(args); - }); + const setTenantContext = (prisma as any) + .$executeRaw`SELECT set_config('app.current_tenant', ${tenantId}, true)`; + + return (prisma as any) + .$transaction([setTenantContext, query(args)]) + .then((results: any[]) => results[1]); }, }, });