diff --git a/package.json b/package.json index e36f32afc48..b69411907e0 100644 --- a/package.json +++ b/package.json @@ -31,9 +31,9 @@ "check:cron-parity": "bun run scripts/check-cron-parity.ts", "check:api-validation:strict": "bun run scripts/check-api-validation-contracts.ts --check --enforce-boundary-baseline", "check:realtime-prune": "bun run scripts/check-realtime-prune-graph.ts", - "check:tool-request-boundary": "bun test scripts/check-tool-request-boundary.test.ts && bun run scripts/check-tool-request-boundary.ts", + "check:tool-request-boundary": "bun run scripts/check-tool-request-boundary.ts", "check:tool-registry-boundary": "bun run scripts/check-tool-registry-boundary.ts", - "check:sql-date-binding": "bun test scripts/check-sql-date-binding.test.ts && bun run scripts/check-sql-date-binding.ts", + "check:sql-date-binding": "bun run scripts/check-sql-date-binding.ts", "check:zustand-v5": "bun run scripts/check-zustand-v5-selectors.ts", "check:react-query": "bun run scripts/check-react-query-patterns.ts --check", "check:client-boundary": "bun run scripts/check-client-boundary-imports.ts --check", diff --git a/scripts/check-migrations-safety.test.ts b/scripts/check-migrations-safety.test.ts deleted file mode 100644 index af966d6e8dd..00000000000 --- a/scripts/check-migrations-safety.test.ts +++ /dev/null @@ -1,200 +0,0 @@ -/** - * Run with: bun test scripts/check-migrations-safety.test.ts - * (Root scripts are bun-native and not part of the turbo/vitest workspaces.) - */ -import { describe, expect, test } from 'bun:test' -import { lintSql } from './check-migrations-safety.ts' - -const rules = (sql: string) => lintSql(sql).map((f) => `${f.tier}:${f.rule}`) - -describe('additive / safe', () => { - test('nullable add column passes', () => { - expect(lintSql('ALTER TABLE "webhook" ADD COLUMN "provider_config" json;')).toEqual([]) - }) - - test('NOT NULL with DEFAULT passes', () => { - expect(lintSql('ALTER TABLE "user" ADD COLUMN "flag" boolean DEFAULT false NOT NULL;')).toEqual( - [] - ) - }) - - test('CREATE TABLE plus index and FK on that new table passes', () => { - const sql = `CREATE TABLE "kb" ("id" text PRIMARY KEY NOT NULL, "user_id" text NOT NULL); ---> statement-breakpoint -CREATE INDEX "kb_user_id_idx" ON "kb" USING btree ("user_id"); ---> statement-breakpoint -ALTER TABLE "kb" ADD CONSTRAINT "kb_user_fk" FOREIGN KEY ("user_id") REFERENCES "user"("id");` - expect(lintSql(sql)).toEqual([]) - }) - - test('CONCURRENTLY index after a COMMIT breakpoint passes', () => { - const sql = `COMMIT; ---> statement-breakpoint -SET lock_timeout = 0; ---> statement-breakpoint -CREATE INDEX CONCURRENTLY IF NOT EXISTS "idx_x" ON "embedding" ("kb_id");` - expect(lintSql(sql)).toEqual([]) - }) -}) - -describe('hard errors', () => { - test('ADD COLUMN NOT NULL without default', () => { - expect(rules('ALTER TABLE "user" ADD COLUMN "email" text NOT NULL;')).toEqual([ - 'error:add-not-null-no-default', - ]) - }) - - test('RENAME column', () => { - expect(rules('ALTER TABLE "marketplace" RENAME COLUMN "executions" TO "views";')).toEqual([ - 'error:rename', - ]) - }) - - test('CREATE INDEX on existing table without CONCURRENTLY', () => { - expect(rules('CREATE INDEX "idx_y" ON "embedding" ("kb_id");')).toEqual([ - 'error:index-not-concurrent', - ]) - }) - - test('CONCURRENTLY index without IF NOT EXISTS', () => { - const sql = `COMMIT; ---> statement-breakpoint -CREATE INDEX CONCURRENTLY "idx_z" ON "embedding" ("kb_id");` - expect(rules(sql)).toEqual(['error:concurrent-index-not-idempotent']) - }) - - test('CONCURRENTLY index without a preceding COMMIT', () => { - expect( - rules('CREATE INDEX CONCURRENTLY IF NOT EXISTS "idx_z" ON "embedding" ("kb_id");') - ).toEqual(['error:concurrent-index-no-commit']) - }) - - test('ADD FOREIGN KEY on existing table without NOT VALID', () => { - expect( - rules( - 'ALTER TABLE "session" ADD CONSTRAINT "s_fk" FOREIGN KEY ("uid") REFERENCES "user"("id");' - ) - ).toEqual(['error:constraint-not-valid']) - }) -}) - -describe('annotate tier', () => { - const drop = 'ALTER TABLE "webhook" DROP COLUMN "secret";' - - test('DROP COLUMN unannotated fails', () => { - expect(rules(drop)).toEqual(['error:drop-column']) - }) - - test('DROP COLUMN annotated passes', () => { - const sql = `-- migration-safe: secret read removed in v0.6.1 (#1234), shipped two deploys ago\n${drop}` - expect(lintSql(sql)).toEqual([]) - }) - - test('annotation tolerates an intervening statement-breakpoint line', () => { - const sql = `ALTER TABLE "webhook" ADD COLUMN "provider_config" json; ---> statement-breakpoint --- migration-safe: secret read removed in v0.6.1 (#1234) -${drop}` - expect(lintSql(sql)).toEqual([]) - }) - - test('dangling annotation with empty reason fails', () => { - const sql = `-- migration-safe:\n${drop}` - const found = lintSql(sql) - expect(found).toHaveLength(1) - expect(found[0].tier).toBe('error') - expect(found[0].message).toContain('no reason') - }) - - test('annotation on the wrong statement does not bleed', () => { - const sql = `-- migration-safe: removing secret -ALTER TABLE "webhook" ADD COLUMN "x" json; ---> statement-breakpoint -${drop}` - expect(rules(sql)).toEqual(['error:drop-column']) - }) - - test('type change and DROP TABLE are annotate-tier', () => { - expect( - rules( - 'ALTER TABLE "user_table_rows" ALTER COLUMN "order_key" SET DATA TYPE text COLLATE "C";' - ) - ).toEqual(['error:alter-type']) - expect(rules('DROP TABLE "marketplace_execution" CASCADE;')).toEqual(['error:drop-table']) - }) -}) - -describe('warnings (non-blocking)', () => { - test('UPDATE backfill warns but does not error', () => { - const found = lintSql('UPDATE "user_table_definitions" SET "schema" = \'{}\' WHERE id = \'1\';') - expect(found.map((f) => f.tier)).toEqual(['warn']) - }) - - test('UPDATE without WHERE flags the whole-table note', () => { - const found = lintSql('UPDATE "user" SET "active" = true;') - expect(found[0].tier).toBe('warn') - expect(found[0].message).toContain('no WHERE') - }) -}) - -describe('review fixes', () => { - test('RENAME CONSTRAINT is metadata-only — not flagged', () => { - expect( - lintSql('ALTER TABLE "permission_group" RENAME CONSTRAINT "old_fk" TO "new_fk";') - ).toEqual([]) - }) - - test('ALTER INDEX ... RENAME is metadata-only — not flagged', () => { - expect(lintSql('ALTER INDEX "old_idx" RENAME TO "new_idx";')).toEqual([]) - }) - - test('table RENAME TO is still a hard error', () => { - expect(rules('ALTER TABLE "marketplace" RENAME TO "listings";')).toEqual(['error:rename']) - }) - - test('plain DROP INDEX is a hard error (ACCESS EXCLUSIVE lock)', () => { - expect(rules('DROP INDEX "permission_group_workspace_name_unique";')).toEqual([ - 'error:drop-index-not-concurrent', - ]) - }) - - test('DROP INDEX CONCURRENTLY after a COMMIT passes clean', () => { - const sql = `COMMIT; ---> statement-breakpoint -DROP INDEX CONCURRENTLY IF EXISTS "stale_idx";` - expect(lintSql(sql)).toEqual([]) - }) - - test('DROP INDEX CONCURRENTLY without IF EXISTS is not idempotent', () => { - const sql = `COMMIT; ---> statement-breakpoint -DROP INDEX CONCURRENTLY "stale_idx";` - expect(rules(sql)).toEqual(['error:concurrent-drop-index-not-idempotent']) - }) - - test('DROP INDEX CONCURRENTLY without a preceding COMMIT errors', () => { - expect(rules('DROP INDEX CONCURRENTLY IF EXISTS "stale_idx";')).toEqual([ - 'error:concurrent-drop-index-no-commit', - ]) - }) - - test('alter-type does not match TYPE inside a string default', () => { - expect(lintSql(`ALTER TABLE "x" ALTER COLUMN "y" SET DEFAULT 'change TYPE later';`)).toEqual([]) - }) -}) - -describe('parser robustness', () => { - test('semicolon inside a string literal does not split', () => { - expect(lintSql(`ALTER TABLE "x" ADD COLUMN "y" text DEFAULT 'a;b' NOT NULL;`)).toEqual([]) - }) - - test('dollar-quoted DO block is one statement; FK on a new table is suppressed', () => { - const sql = `CREATE TABLE "jobs" ("id" text PRIMARY KEY NOT NULL, "wid" text NOT NULL); ---> statement-breakpoint -DO $$ BEGIN - ALTER TABLE "jobs" ADD CONSTRAINT "jobs_fk" FOREIGN KEY ("wid") REFERENCES "workspace"("id"); -EXCEPTION WHEN duplicate_object THEN null; -END $$;` - expect(lintSql(sql)).toEqual([]) - }) -}) diff --git a/scripts/check-migrations-safety.ts b/scripts/check-migrations-safety.ts index 5008b48e367..0c7fd3dd302 100644 --- a/scripts/check-migrations-safety.ts +++ b/scripts/check-migrations-safety.ts @@ -330,7 +330,7 @@ const ANNOTATE_GUIDANCE = 'is a contract-phase op. Confirm the old code no longer reads/writes it (it must have shipped in an earlier deploy — not this same PR), then acknowledge with a `-- migration-safe: ` comment on the line above.' /** Lint a single migration's SQL. Returns only actionable findings. */ -export function lintSql(content: string): Finding[] { +function lintSql(content: string): Finding[] { const lines = content.split('\n') const statements = parseStatements(content) const createdTables = new Set() diff --git a/scripts/check-sql-date-binding.test.ts b/scripts/check-sql-date-binding.test.ts deleted file mode 100644 index 834aed257fd..00000000000 --- a/scripts/check-sql-date-binding.test.ts +++ /dev/null @@ -1,62 +0,0 @@ -import { describe, expect, test } from 'bun:test' -import { findSqlDateBindingViolations } from './check-sql-date-binding' - -describe('sql Date binding audit', () => { - test('rejects every unbound Date form that reaches a raw template', () => { - const violations = findSqlDateBindingViolations(` - const now = new Date() - const threshold = new Date(now.getTime() - 1000) - const alias = threshold - function scan(cutoff: Date, since: Date | null) { - const inline = sql\`col < \${new Date(cursor.ts)}\` - const local = sql\`col < \${now}\` - const chained = sql\`col < \${alias}\` - const generic = sql\`col < \${threshold}\` - const annotated = sql\`col < \${cutoff}\` - const nullable = sql\`col < \${since}\` - const fallback = sql\`col < \${since ?? now}\` - const unencoded = sql.param(now) - } - `) - - expect(violations.map((violation) => violation.expression)).toEqual([ - 'new Date(cursor.ts)', - 'now', - 'alias', - 'threshold', - 'cutoff', - 'since', - 'since ?? now', - 'now', - ]) - }) - - test('accepts column-bound params, non-Date values, and annotated exceptions', () => { - expect( - findSqlDateBindingViolations(` - const now = new Date() - const bound = sql\`col < \${sql.param(now, asyncJobs.startedAt)}\` - const fragment = sql\`col < \${sql.param(new Date(), table.createdAt)}\` - const columns = sql\`\${table.startedAt} < \${table.endedAt}\` - const scalars = sql\`col < \${MAX_INT32} AND name = \${name}\` - const notSql = other\`col < \${now}\` - // sql-date-bound: raw text column, no timestamp encoding applies - const excused = sql\`col < \${now}\` - `) - ).toEqual([]) - }) - - test('rejects annotation markers that are malformed or incidental', () => { - const violations = findSqlDateBindingViolations(` - const now = new Date() - // sql-date-bound: - const bareMarker = sql\`col < \${now}\` - const label = 'sql-date-bound: not a comment' - const incidental = sql\`col < \${now}\` - // trailing marker sql-date-bound: reason - const misplaced = sql\`col < \${now}\` - `) - - expect(violations.map((violation) => violation.expression)).toEqual(['now', 'now', 'now']) - }) -}) diff --git a/scripts/check-sql-date-binding.ts b/scripts/check-sql-date-binding.ts index a85a619b1f5..abbacb1f9c2 100644 --- a/scripts/check-sql-date-binding.ts +++ b/scripts/check-sql-date-binding.ts @@ -8,6 +8,10 @@ * context, so an interpolated `Date` skips that mapping, reaches the now-identity serializer * unchanged, and the wire encoder throws `ERR_INVALID_ARG_TYPE`. Binding through * `sql.param(date, table.column)` restores the column mapping. + * + * Only drizzle's tag is audited. postgres-js's own client tag (`const sql = postgres(url)`) + * serializes Dates correctly, so the tag is resolved to a `drizzle-orm` import binding rather + * than matched by the identifier name. */ import { readdirSync, readFileSync } from 'node:fs' import { dirname, extname, join, relative, resolve } from 'node:path' @@ -16,10 +20,12 @@ import { parse } from '@babel/parser' const SCRIPT_DIR = dirname(fileURLToPath(import.meta.url)) const ROOT = resolve(SCRIPT_DIR, '..') -const SCAN_DIRS = [join(ROOT, 'apps'), join(ROOT, 'packages')] +const SCAN_DIRS = [join(ROOT, 'apps'), join(ROOT, 'packages'), join(ROOT, 'scripts')] const SKIP_DIRS = new Set(['node_modules', '.next', '.turbo', 'coverage', 'dist', 'build', 'out']) const SOURCE_EXTENSIONS = new Set(['.ts', '.tsx', '.mts', '.cts']) const ALLOW_ANNOTATION = '// sql-date-bound:' +const DRIZZLE_MODULE = 'drizzle-orm' +const STATEMENT_TYPE = /(Statement|Declaration)$/ interface Violation { file: string @@ -28,6 +34,12 @@ interface Violation { reason: string } +/** Result of auditing one file; `parseError` marks a file the parser could not read. */ +interface FileAnalysis { + violations: Violation[] + parseError?: string +} + interface SyntaxNode extends Record { type: string start?: number | null @@ -78,102 +90,262 @@ function isDateAnnotation(annotation: unknown): boolean { } /** `new Date(...)` plus the expression forms that trivially forward one. */ -function isDateExpression(node: unknown, dateNames: ReadonlySet): boolean { +function isDateExpression(node: unknown, isDateName: (name: string) => boolean): boolean { if (!isSyntaxNode(node)) return false const current = unwrap(node) if (current.type === 'NewExpression') return isSyntaxNode(current.callee) && current.callee.name === 'Date' if (current.type === 'Identifier') - return typeof current.name === 'string' && dateNames.has(current.name) + return typeof current.name === 'string' && isDateName(current.name) if (current.type === 'ConditionalExpression') return ( - isDateExpression(current.consequent, dateNames) || - isDateExpression(current.alternate, dateNames) + isDateExpression(current.consequent, isDateName) || + isDateExpression(current.alternate, isDateName) ) if (current.type === 'LogicalExpression') - return isDateExpression(current.left, dateNames) || isDateExpression(current.right, dateNames) + return isDateExpression(current.left, isDateName) || isDateExpression(current.right, isDateName) return false } /** - * Names bound to a `Date` anywhere in the file. The pass over-approximates scope — a name - * declared in one function marks same-named bindings elsewhere — which keeps the audit - * blind-spot-free at the cost of flagging a shadowed non-Date, resolvable by renaming. + * Lexical scope for Date-typed bindings; lookups walk the parent chain. + * + * `localNames` holds every name the scope binds, Date-typed or not, so a lookup + * stops at the nearest declaring scope instead of reaching past a shadow. */ -function collectDateNames(program: SyntaxNode): Set { - const names = new Set() - let changed = true +interface Scope { + parent: Scope | null + dateNames: Set + localNames: Set +} + +const createScope = (parent: Scope | null): Scope => ({ + parent, + dateNames: new Set(), + localNames: new Set(), +}) + +/** + * Resolves `name` to the nearest scope that binds it. + * + * `dateNames` is consulted before `localNames` at every level so the resolution + * fixpoint still converges: a local binding not *yet* proven to be a Date blocks + * the walk on this pass, and a later pass finds it once proven. + */ +function hasDateName(scope: Scope, name: string): boolean { + for (let current: Scope | null = scope; current; current = current.parent) { + if (current.dateNames.has(name)) return true + if (current.localNames.has(name)) return false + } + return false +} + +/** Records every identifier a binding pattern introduces, however nested. */ +function collectBoundNames(node: unknown, into: Set): void { + if (!isSyntaxNode(node)) return + if (node.type === 'Identifier') { + if (typeof node.name === 'string') into.add(node.name) + return + } + if (node.type === 'ObjectPattern' && Array.isArray(node.properties)) { + for (const property of node.properties) { + if (!isSyntaxNode(property)) continue + collectBoundNames(property.type === 'RestElement' ? property.argument : property.value, into) + } + return + } + if (node.type === 'ArrayPattern' && Array.isArray(node.elements)) { + for (const element of node.elements) collectBoundNames(element, into) + return + } + if (node.type === 'AssignmentPattern') collectBoundNames(node.left, into) + if (node.type === 'RestElement') collectBoundNames(node.argument, into) +} + +const FUNCTION_TYPES = new Set([ + 'FunctionDeclaration', + 'FunctionExpression', + 'ArrowFunctionExpression', + 'ObjectMethod', + 'ClassMethod', + 'ClassPrivateMethod', + 'TSDeclareFunction', +]) + +/** Field names declared `Date` on an interface or object-type alias, keyed by type name. */ +function collectDateTypeFields(program: SyntaxNode): Map> { + const fields = new Map>() + + const membersOf = (node: unknown): SyntaxNode[] => { + if (!isSyntaxNode(node)) return [] + if (node.type === 'TSTypeLiteral' && Array.isArray(node.members)) + return node.members.filter(isSyntaxNode) + if (node.type === 'TSInterfaceBody' && Array.isArray(node.body)) + return node.body.filter(isSyntaxNode) + return [] + } const visit = (node: SyntaxNode) => { - const isBinding = - node.type === 'VariableDeclarator' || - node.type === 'ClassProperty' || - node.type === 'PropertyDefinition' - if (isBinding && isSyntaxNode(node.id ?? node.key)) { - const target = (node.id ?? node.key) as SyntaxNode - if ( - target.type === 'Identifier' && - typeof target.name === 'string' && - (isDateAnnotation(target.typeAnnotation ?? node.typeAnnotation) || - isDateExpression(node.init ?? node.value, names)) - ) { - if (!names.has(target.name)) { - names.add(target.name) - changed = true - } + const name = isSyntaxNode(node.id) ? node.id.name : undefined + if (typeof name === 'string') { + const members = + node.type === 'TSInterfaceDeclaration' + ? membersOf(node.body) + : node.type === 'TSTypeAliasDeclaration' + ? membersOf(node.typeAnnotation) + : [] + for (const member of members) { + if (member.type !== 'TSPropertySignature' || !isDateAnnotation(member.typeAnnotation)) + continue + const key = isSyntaxNode(member.key) ? member.key.name : undefined + if (typeof key !== 'string') continue + const existing = fields.get(name) + if (existing) existing.add(key) + else fields.set(name, new Set([key])) } } - if ( - (node.type === 'Identifier' && isDateAnnotation(node.typeAnnotation)) || - (node.type === 'TSPropertySignature' && isDateAnnotation(node.typeAnnotation)) - ) { - const name = isSyntaxNode(node.key) ? node.key.name : node.name - if (typeof name === 'string' && !names.has(name)) { - names.add(name) - changed = true + for (const child of getChildNodes(node)) visit(child) + } + visit(program) + return fields +} + +/** Local names the `sql` tag is reachable through, resolved from `drizzle-orm` imports. */ +interface SqlBindings { + tags: Set + namespaces: Set +} + +function collectSqlBindings(program: SyntaxNode): SqlBindings { + const bindings: SqlBindings = { tags: new Set(), namespaces: new Set() } + + const visit = (node: SyntaxNode) => { + if (node.type === 'ImportDeclaration' && isSyntaxNode(node.source)) { + const source = node.source.value + const isDrizzle = + typeof source === 'string' && + (source === DRIZZLE_MODULE || source.startsWith(`${DRIZZLE_MODULE}/`)) + if (isDrizzle && Array.isArray(node.specifiers)) { + for (const specifier of node.specifiers) { + if (!isSyntaxNode(specifier) || !isSyntaxNode(specifier.local)) continue + const local = specifier.local.name + if (typeof local !== 'string') continue + if (specifier.type === 'ImportNamespaceSpecifier') bindings.namespaces.add(local) + else if ( + specifier.type === 'ImportSpecifier' && + isSyntaxNode(specifier.imported) && + specifier.imported.name === 'sql' + ) + bindings.tags.add(local) + } } } + if (node.type === 'VariableDeclarator' && isDrizzleImportCall(node.init)) + bindDynamicImport(node.id, bindings) + for (const child of getChildNodes(node)) visit(child) } + visit(program) + return bindings +} - /** Re-run until stable so `const b = a` chains resolve regardless of declaration order. */ - while (changed) { - changed = false - visit(program) +/** + * `import('drizzle-orm')`, with or without an `await`. + * + * Babel parses a dynamic import as a `CallExpression` whose callee is `Import`; + * the `ImportExpression` spelling is accepted too so a parser upgrade cannot + * silently reopen the hole this closes. + */ +function isDrizzleImportCall(node: unknown): boolean { + if (!isSyntaxNode(node)) return false + const current = node.type === 'AwaitExpression' ? unwrapAwait(node) : node + if (!isSyntaxNode(current)) return false + const isImport = + current.type === 'ImportExpression' || + (current.type === 'CallExpression' && + isSyntaxNode(current.callee) && + current.callee.type === 'Import') + if (!isImport) return false + const args = Array.isArray(current.arguments) ? current.arguments : [] + const source = isSyntaxNode(current.source) ? current.source : args.find(isSyntaxNode) + const value = source?.value + return ( + typeof value === 'string' && + (value === DRIZZLE_MODULE || value.startsWith(`${DRIZZLE_MODULE}/`)) + ) +} + +const unwrapAwait = (node: SyntaxNode): unknown => + isSyntaxNode(node.argument) ? node.argument : undefined + +/** + * Binds `const { sql } = await import('drizzle-orm')` and its namespace form. + * + * Without this a file importing the tag dynamically resolves no tag at all, so + * the whole file is skipped rather than audited — a silent hole, not a warning. + */ +function bindDynamicImport(target: unknown, bindings: SqlBindings): void { + if (!isSyntaxNode(target)) return + if (target.type === 'Identifier' && typeof target.name === 'string') { + bindings.namespaces.add(target.name) + return + } + if (target.type !== 'ObjectPattern' || !Array.isArray(target.properties)) return + for (const property of target.properties) { + if (!isSyntaxNode(property) || property.type !== 'ObjectProperty') continue + const key = isSyntaxNode(property.key) ? property.key.name : undefined + if (key !== 'sql') continue + const raw = isSyntaxNode(property.value) ? property.value : undefined + const local = raw?.type === 'AssignmentPattern' && isSyntaxNode(raw.left) ? raw.left : raw + if (local?.type === 'Identifier' && typeof local.name === 'string') + bindings.tags.add(local.name) } - return names } -function isSqlIdentifier(node: unknown): boolean { - return isSyntaxNode(node) && node.type === 'Identifier' && node.name === 'sql' +/** `sql`, an aliased import of it, or `namespace.sql`. */ +function isSqlReference(node: unknown, bindings: SqlBindings): boolean { + if (!isSyntaxNode(node)) return false + const current = unwrap(node) + if (current.type === 'Identifier') + return typeof current.name === 'string' && bindings.tags.has(current.name) + return ( + current.type === 'MemberExpression' && + current.computed !== true && + isSyntaxNode(current.object) && + current.object.type === 'Identifier' && + typeof current.object.name === 'string' && + bindings.namespaces.has(current.object.name) && + isSyntaxNode(current.property) && + current.property.name === 'sql' + ) } /** Matches `` sql`…` `` and `` sql`…` `` (the generic wraps the tag in TSInstantiationExpression). */ -function isSqlTag(node: unknown): boolean { - if (isSqlIdentifier(node)) return true +function isSqlTag(node: unknown, bindings: SqlBindings): boolean { + if (isSqlReference(node, bindings)) return true return ( isSyntaxNode(node) && node.type === 'TSInstantiationExpression' && - isSqlIdentifier(node.expression) + isSqlReference(node.expression, bindings) ) } -function isSqlParamCall(node: SyntaxNode): boolean { +function isSqlParamCall(node: SyntaxNode, bindings: SqlBindings): boolean { const callee = isSyntaxNode(node.callee) ? unwrap(node.callee) : undefined return Boolean( callee && callee.type === 'MemberExpression' && isSyntaxNode(callee.property) && callee.property.name === 'param' && - isSqlIdentifier(callee.object) + isSqlReference(callee.object, bindings) ) } /** - * A violation is excused only when the preceding line is a line comment whose - * text is exactly the documented annotation followed by a non-empty reason. - * Matching the marker anywhere on the line would let unrelated code — or a - * bare marker with no justification — silently disable the audit. + * A violation is excused only when the line above one of its anchors is a line comment whose + * text is exactly the documented annotation followed by a non-empty reason. Matching the + * marker anywhere on the line would let unrelated code — or a bare marker with no + * justification — silently disable the audit. */ function isAllowAnnotation(line: string | undefined): boolean { const trimmed = (line ?? '').trim() @@ -181,52 +353,195 @@ function isAllowAnnotation(line: string | undefined): boolean { return trimmed.slice(ALLOW_ANNOTATION.length).trim().length > 0 } -export function findSqlDateBindingViolations(source: string, file = 'source.ts'): Violation[] { - const syntaxTree = parse(source, { - sourceFilename: file, - sourceType: 'unambiguous', - errorRecovery: true, - plugins: [...(extname(file) === '.tsx' ? (['jsx'] as const) : []), 'typescript'], - }) - const program = syntaxTree.program as unknown as SyntaxNode - const dateNames = collectDateNames(program) - const lines = source.split('\n') - const violations: Violation[] = [] +interface BindingCandidate { + scope: Scope + name: string + annotation?: unknown + init?: unknown +} - const report = (node: SyntaxNode, reason: string) => { - if (typeof node.start !== 'number' || typeof node.end !== 'number' || !node.loc) return - const line = node.loc.start.line - if (isAllowAnnotation(lines[line - 2])) return - violations.push({ file, line, expression: source.slice(node.start, node.end), reason }) +interface CheckSite { + node: SyntaxNode + scope: Scope + /** Lines an allow annotation may sit above: the expression, its template, its statement. */ + anchors: number[] + reason: string +} + +const TEMPLATE_REASON = + 'a Date interpolated into a raw sql template has no encoder; bind it with sql.param(date, table.column)' +const PARAM_REASON = 'sql.param(date) has no encoder; pass the column as the second argument' + +function analyzeSource(source: string, file = 'source.ts'): FileAnalysis { + let program: SyntaxNode + try { + const syntaxTree = parse(source, { + sourceFilename: file, + sourceType: 'unambiguous', + errorRecovery: true, + plugins: [ + ...(extname(file) === '.tsx' ? (['jsx'] as const) : []), + 'typescript', + 'decorators', + ], + }) + program = syntaxTree.program as unknown as SyntaxNode + } catch (error) { + return { + violations: [], + parseError: error instanceof Error ? error.message : String(error), + } } - const visit = (node: SyntaxNode) => { - if (node.type === 'TaggedTemplateExpression' && isSqlTag(node.tag)) { + const bindings = collectSqlBindings(program) + if (bindings.tags.size === 0 && bindings.namespaces.size === 0) return { violations: [] } + + const dateTypeFields = collectDateTypeFields(program) + const rootScope: Scope = createScope(null) + const candidates: BindingCandidate[] = [] + const checks: CheckSite[] = [] + + /** Field names typed `Date` on an inline object type or a named interface/type alias. */ + const dateFieldsOf = (annotation: unknown): Set => { + if (!isSyntaxNode(annotation)) return new Set() + if (annotation.type === 'TSTypeAnnotation') return dateFieldsOf(annotation.typeAnnotation) + if (annotation.type === 'TSTypeLiteral' && Array.isArray(annotation.members)) { + const names = new Set() + for (const member of annotation.members) { + if (!isSyntaxNode(member)) continue + if (member.type !== 'TSPropertySignature' || !isDateAnnotation(member.typeAnnotation)) + continue + const key = isSyntaxNode(member.key) ? member.key.name : undefined + if (typeof key === 'string') names.add(key) + } + return names + } + if (annotation.type === 'TSTypeReference' && isSyntaxNode(annotation.typeName)) { + const name = annotation.typeName.name + if (typeof name === 'string') return dateTypeFields.get(name) ?? new Set() + } + return new Set() + } + + /** Binds `{ since }: { since: Date }` — a destructured Date is still an unbound Date. */ + const bindPattern = (pattern: unknown, annotation: unknown, scope: Scope) => { + if (!isSyntaxNode(pattern) || pattern.type !== 'ObjectPattern') return + const fields = dateFieldsOf(annotation) + if (fields.size === 0 || !Array.isArray(pattern.properties)) return + for (const property of pattern.properties) { + if (!isSyntaxNode(property) || property.type !== 'ObjectProperty') continue + const key = isSyntaxNode(property.key) ? property.key.name : undefined + const raw = isSyntaxNode(property.value) ? property.value : undefined + const value = raw?.type === 'AssignmentPattern' && isSyntaxNode(raw.left) ? raw.left : raw + if (typeof key !== 'string' || !fields.has(key)) continue + if (value?.type === 'Identifier' && typeof value.name === 'string') + scope.dateNames.add(value.name) + } + } + + const bindParameters = (fn: SyntaxNode, scope: Scope) => { + if (!Array.isArray(fn.params)) return + for (const raw of fn.params) { + if (!isSyntaxNode(raw)) continue + const param = raw.type === 'AssignmentPattern' && isSyntaxNode(raw.left) ? raw.left : raw + collectBoundNames(param, scope.localNames) + if (param.type === 'Identifier' && typeof param.name === 'string') { + if (isDateAnnotation(param.typeAnnotation)) scope.dateNames.add(param.name) + } else if (param.type === 'ObjectPattern') { + bindPattern(param, param.typeAnnotation, scope) + } + } + } + + const lines = source.split('\n') + + const visit = (node: SyntaxNode, parentScope: Scope, parentStatementLine: number) => { + let scope = parentScope + if (FUNCTION_TYPES.has(node.type)) { + scope = createScope(parentScope) + bindParameters(node, scope) + } + const statementLine = + STATEMENT_TYPE.test(node.type) && node.loc ? node.loc.start.line : parentStatementLine + + if (node.type === 'VariableDeclarator' && isSyntaxNode(node.id)) { + const target = node.id + collectBoundNames(target, scope.localNames) + if (target.type === 'Identifier' && typeof target.name === 'string') { + candidates.push({ + scope, + name: target.name, + annotation: target.typeAnnotation ?? node.typeAnnotation, + init: node.init, + }) + } else if (target.type === 'ObjectPattern') { + bindPattern(target, target.typeAnnotation, scope) + } + } + + if (node.type === 'TaggedTemplateExpression' && isSqlTag(node.tag, bindings)) { const quasi = isSyntaxNode(node.quasi) ? node.quasi : undefined const expressions = Array.isArray(quasi?.expressions) ? quasi.expressions : [] + const tagLine = node.loc?.start.line for (const expression of expressions) { - if (isDateExpression(expression, dateNames)) { - report( - expression as SyntaxNode, - 'a Date interpolated into a raw sql template has no encoder; bind it with sql.param(date, table.column)' - ) - } + if (!isSyntaxNode(expression) || !expression.loc) continue + checks.push({ + node: expression, + scope, + anchors: [expression.loc.start.line, tagLine ?? statementLine, statementLine], + reason: TEMPLATE_REASON, + }) } } - if (node.type === 'CallExpression' && isSqlParamCall(node)) { + + if (node.type === 'CallExpression' && isSqlParamCall(node, bindings)) { const args = Array.isArray(node.arguments) ? node.arguments : [] - if (args.length === 1 && isDateExpression(args[0], dateNames)) { - report( - args[0] as SyntaxNode, - 'sql.param(date) has no encoder; pass the column as the second argument' - ) + const argument = args.length === 1 && isSyntaxNode(args[0]) ? args[0] : undefined + if (argument?.loc) { + checks.push({ + node: argument, + scope, + anchors: [argument.loc.start.line, node.loc?.start.line ?? statementLine, statementLine], + reason: PARAM_REASON, + }) } } - for (const child of getChildNodes(node)) visit(child) + + for (const child of getChildNodes(node)) visit(child, scope, statementLine) + } + visit(program, rootScope, 1) + + /** Re-run until stable so `const b = a` chains resolve regardless of declaration order. */ + let changed = true + while (changed) { + changed = false + for (const candidate of candidates) { + if (hasDateName(candidate.scope, candidate.name)) continue + const isDate = + isDateAnnotation(candidate.annotation) || + isDateExpression(candidate.init, (name) => hasDateName(candidate.scope, name)) + if (isDate) { + candidate.scope.dateNames.add(candidate.name) + changed = true + } + } + } + + const violations: Violation[] = [] + for (const check of checks) { + const { node } = check + if (typeof node.start !== 'number' || typeof node.end !== 'number' || !node.loc) continue + if (!isDateExpression(node, (name) => hasDateName(check.scope, name))) continue + if (check.anchors.some((line) => isAllowAnnotation(lines[line - 2]))) continue + violations.push({ + file, + line: node.loc.start.line, + expression: source.slice(node.start, node.end), + reason: check.reason, + }) } - visit(program) - return violations + return { violations } } function collectSources(dir: string, found: string[] = []): string[] { @@ -241,9 +556,20 @@ function collectSources(dir: string, found: string[] = []): string[] { function main(): void { const files = SCAN_DIRS.flatMap((dir) => collectSources(dir)) - const violations = files.flatMap((file) => - findSqlDateBindingViolations(readFileSync(file, 'utf8'), file) - ) + const violations: Violation[] = [] + const skipped: { file: string; parseError: string }[] = [] + + for (const file of files) { + const analysis = analyzeSource(readFileSync(file, 'utf8'), file) + if (analysis.parseError) skipped.push({ file, parseError: analysis.parseError }) + violations.push(...analysis.violations) + } + + if (skipped.length > 0) { + console.warn(`⚠ ${skipped.length} file(s) could not be parsed and were not audited:`) + for (const entry of skipped) + console.warn(` ${relative(ROOT, entry.file)} ${entry.parseError}`) + } if (violations.length > 0) { console.error('Unbound Date values reach postgres-js through raw sql templates:') @@ -256,7 +582,8 @@ function main(): void { `\nDrizzle replaces postgres-js's temporal serializers with an identity function and maps` + `\ntimestamps itself, so a Date outside column context is never serialized. Bind through` + `\nthe column: sql.param(date, table.column). Annotate a genuine exception with` + - `\n${ALLOW_ANNOTATION} on the preceding line.` + `\n${ALLOW_ANNOTATION} on the line above the expression, its sql template, or` + + `\nits enclosing statement.` ) process.exit(1) } diff --git a/scripts/check-tool-request-boundary.test.ts b/scripts/check-tool-request-boundary.test.ts deleted file mode 100644 index 6eeb7dd8618..00000000000 --- a/scripts/check-tool-request-boundary.test.ts +++ /dev/null @@ -1,37 +0,0 @@ -import { describe, expect, test } from 'bun:test' -import { findToolRequestBoundaryViolations } from './check-tool-request-boundary' - -describe('tool request boundary audit', () => { - test('rejects direct and aliased ToolConfig request execution', () => { - const violations = findToolRequestBoundaryViolations(` - const direct = mistralParserTool.request.body(params) - const computed = tool.request['headers'](params) - const typedRequest = (customTool as ToolConfig).request - const typed = typedRequest[\`method\`](params) - const optional = tool.request?.url - const requestConfig = customTool.request - const url = requestConfig.url - const { body } = requestConfig - `) - - expect(violations.map((violation) => violation.expression)).toEqual([ - 'mistralParserTool.request.body', - "tool.request['headers']", - 'typedRequest[\`method\`]', - 'tool.request?.url', - 'requestConfig.url', - 'body', - ]) - }) - - test('allows declarations and ordinary request objects', () => { - expect( - findToolRequestBoundaryViolations(` - const tool = { request: { url: '/api/tool', headers: () => ({}) } } - const request = options.request - request.headers.get('authorization') - incomingRequest.headers.get('authorization') - `) - ).toEqual([]) - }) -}) diff --git a/scripts/check-tool-request-boundary.ts b/scripts/check-tool-request-boundary.ts index 043f666f96a..f2acb388525 100644 --- a/scripts/check-tool-request-boundary.ts +++ b/scripts/check-tool-request-boundary.ts @@ -148,7 +148,7 @@ function isLikelyToolIdentifier(expression: SyntaxNode): boolean { ) } -export function findToolRequestBoundaryViolations(source: string, file = 'source.ts'): Violation[] { +function findToolRequestBoundaryViolations(source: string, file = 'source.ts'): Violation[] { const extension = extname(file) const syntaxTree = parse(source, { sourceFilename: file,