opensource/sim
3
0
Fork 0
mirror of https://github.com/simstudioai/sim.git synced 2026-09-24 15:45:35 +08:00
Code Issues Packages Projects Releases Wiki Activity

fix(scripts): make the sql Date-binding audit precise and crash-proof (#6340)

Browse Source 
* fix(scripts): make the sql Date-binding audit precise and crash-proof

Resolve the drizzle `sql` tag from its import binding, scope Date bindings
lexically, tolerate unparseable files, accept the allow annotation above a
multi-line template, and scan the root scripts directory.

* fix(scripts): honor shadowed bindings and defaulted destructured Dates

* fix(scripts): audit drizzle sql tags bound through a dynamic import

* chore(scripts): drop the sql Date-binding unit tests and the exports that served them

* chore(scripts): drop the script unit tests and the exports that served them
This commit is contained in:
Waleed
2026-08-06 14:30:58 -07:00
committed by GitHub
parent 0ae09c1518
commit 8c49d35a9c
7 changed files with 431 additions and 403 deletions
Download Patch File Download Diff File Expand all files Collapse all files
scripts/check-migrations-safety.test.ts
-200
View File
@@ -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([])
})
})
scripts/check-migrations-safety.ts
+1 -1
View File
@@ -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: <reason>` 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<string>()
scripts/check-sql-date-binding.test.ts
-62
View File
@@ -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<boolean>\`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'])
})
})
scripts/check-sql-date-binding.ts
+427 -100
View File
@@ -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<string, unknown> {
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<string>): 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<string> {
const names = new Set<string>()
let changed = true
interface Scope {
parent: Scope | null
dateNames: Set<string>
localNames: Set<string>
}
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<string>): 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<string, Set<string>> {
const fields = new Map<string, Set<string>>()
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
}
}
}
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
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]))
}
}
for (const child of getChildNodes(node)) visit(child)
}
visit(program)
return fields
}
/** Re-run until stable so `const b = a` chains resolve regardless of declaration order. */
while (changed) {
changed = false
visit(program)
/** Local names the `sql` tag is reachable through, resolved from `drizzle-orm` imports. */
interface SqlBindings {
tags: Set<string>
namespaces: Set<string>
}
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)
}
return names
visit(program)
return bindings
}
function isSqlIdentifier(node: unknown): boolean {
return isSyntaxNode(node) && node.type === 'Identifier' && node.name === 'sql'
}
/** Matches `` sql`…` `` and `` sql<T>`…` `` (the generic wraps the tag in TSInstantiationExpression). */
function isSqlTag(node: unknown): boolean {
if (isSqlIdentifier(node)) return true
/**
* `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 (
isSyntaxNode(node) &&
node.type === 'TSInstantiationExpression' &&
isSqlIdentifier(node.expression)
typeof value === 'string' &&
(value === DRIZZLE_MODULE || value.startsWith(`${DRIZZLE_MODULE}/`))
)
}
function isSqlParamCall(node: SyntaxNode): boolean {
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)
}
}
/** `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<T>`…` `` (the generic wraps the tag in TSInstantiationExpression). */
function isSqlTag(node: unknown, bindings: SqlBindings): boolean {
if (isSqlReference(node, bindings)) return true
return (
isSyntaxNode(node) &&
node.type === 'TSInstantiationExpression' &&
isSqlReference(node.expression, bindings)
)
}
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<string> => {
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<string>()
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)) {
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'
)
}
}
for (const child of getChildNodes(node)) visit(child)
}
visit(program)
return violations
if (node.type === 'CallExpression' && isSqlParamCall(node, bindings)) {
const args = Array.isArray(node.arguments) ? node.arguments : []
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, 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,
})
}
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} <reason> on the preceding line.`
`\n${ALLOW_ANNOTATION} <reason> on the line above the expression, its sql template, or` +
`\nits enclosing statement.`
)
process.exit(1)
}
scripts/check-tool-request-boundary.test.ts
-37
View File
@@ -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([])
})
})
scripts/check-tool-request-boundary.ts
+1 -1
View File
@@ -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,