-
Notifications
You must be signed in to change notification settings - Fork 132
fix: harden security headers and webhook body handling #555
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,6 +8,82 @@ import { | |
| poolExhaustedCounter, | ||
| } from "./promMetrics"; | ||
|
|
||
| function buildPrismaClient(url: string): PrismaClient { | ||
| return new PrismaClient({ | ||
| datasources: { db: { url } }, | ||
| log: [ | ||
| { level: "query", emit: "event" }, | ||
| { level: "error", emit: "stdout" }, | ||
| { level: "warn", emit: "stdout" }, | ||
| ], | ||
| }); | ||
| } | ||
|
|
||
| function applyPrismaClientMiddleware(client: PrismaClient): void { | ||
| client.$use(async (params: Prisma.MiddlewareParams, next: Prisma.MiddlewareFn) => { | ||
| const tracer = trace.getTracer("prisma"); | ||
| const spanName = `prisma.${params.model ?? "raw"}.${params.action}`; | ||
| return tracer.startActiveSpan(spanName, async (span) => { | ||
| span.setAttributes({ | ||
| "db.system": "postgresql", | ||
| "db.operation": params.action, | ||
| ...(params.model ? { "db.prisma.model": params.model } : {}), | ||
| }); | ||
| try { | ||
| const result = await next(params); | ||
| span.setStatus({ code: SpanStatusCode.OK }); | ||
| return result; | ||
| } catch (err) { | ||
| span.setStatus({ code: SpanStatusCode.ERROR, message: String(err) }); | ||
| throw err; | ||
| } finally { | ||
| span.end(); | ||
| } | ||
| }); | ||
| }); | ||
|
|
||
| client.$use(async (params: Prisma.MiddlewareParams, next: Prisma.MiddlewareFn) => { | ||
| const end = poolAcquireHistogram.startTimer({ | ||
| model: params.model ?? "raw", | ||
| action: params.action, | ||
| }); | ||
| try { | ||
| return await next(params); | ||
| } finally { | ||
| end(); | ||
| } | ||
| }); | ||
|
|
||
| client.$use(async (params: Prisma.MiddlewareParams, next: Prisma.MiddlewareFn) => { | ||
| let lastError: unknown; | ||
| for (let attempt = 1; attempt <= MAX_RETRIES; attempt++) { | ||
| try { | ||
| return await next(params); | ||
| } catch (err) { | ||
| if (!isPoolExhaustionError(err)) { | ||
| throw err; | ||
| } | ||
| poolExhaustedCounter.inc(); | ||
| if (attempt < MAX_RETRIES) { | ||
| lastError = err; | ||
| const backoff = BASE_BACKOFF_MS * 2 ** (attempt - 1); | ||
| logger.warn("Prisma connection pool exhausted, retrying", { | ||
| model: params.model, | ||
| action: params.action, | ||
| attempt, | ||
| maxRetries: MAX_RETRIES, | ||
| backoffMs: backoff, | ||
| }); | ||
| await new Promise((r) => setTimeout(r, backoff)); | ||
| } else { | ||
| throw err; | ||
| } | ||
| } | ||
| } | ||
| throw lastError; | ||
| }); | ||
| } | ||
|
|
||
| // B-056: Validate URL assignments at boot to prevent runtime/migration confusion. | ||
| // DATABASE_URL → direct PostgreSQL only (used by prisma migrate) | ||
| // PRISMA_ACCELERATE_URL → prisma:// or prisma+postgres:// protocol (runtime connection pooling) | ||
|
|
@@ -31,8 +107,6 @@ if (config.prismaAccelerateUrl && !ACCELERATE_PROTOCOL_RE.test(config.prismaAcce | |
| ); | ||
| } | ||
|
|
||
| const useAccelerate = Boolean(config.prismaAccelerateUrl); | ||
|
|
||
| // #386: When routing through Prisma Accelerate, the proxy enforces its own | ||
| // query timeout (default 10 s, configurable via ACCELERATE_QUERY_TIMEOUT_MS in | ||
| // the Accelerate dashboard). If Accelerate cancels a query before PostgreSQL | ||
|
|
@@ -65,151 +139,91 @@ function appendStatementTimeout(url: string, timeoutMs: number): string { | |
| } | ||
| } | ||
|
|
||
| const databaseUrl = useAccelerate | ||
| ? config.prismaAccelerateUrl! | ||
| : appendStatementTimeout(config.databaseUrl, STATEMENT_TIMEOUT_MS); | ||
| // Retry config for connection pool exhaustion (Prisma Accelerate) | ||
| const MAX_RETRIES = 3; | ||
| const BASE_BACKOFF_MS = 200; | ||
|
|
||
| function resolveDatabaseUrls(): { runtimeUrl: string; replicaUrl: string; useAccelerate: boolean } { | ||
| const configuredDatabaseUrl = process.env.DATABASE_URL || config.databaseUrl; | ||
| const configuredAccelerateUrl = process.env.PRISMA_ACCELERATE_URL || config.prismaAccelerateUrl; | ||
| const configuredReplicaUrl = process.env.DATABASE_URL_REPLICA || config.databaseUrlReplica; | ||
|
|
||
| const latestStatementTimeoutMs = parseInt(process.env.DB_STATEMENT_TIMEOUT_MS ?? "9000", 10); | ||
| const useAccelerate = Boolean(configuredAccelerateUrl); | ||
| const runtimeUrl = useAccelerate | ||
| ? configuredAccelerateUrl! | ||
| : appendStatementTimeout(configuredDatabaseUrl, latestStatementTimeoutMs); | ||
| const replicaUrl = configuredReplicaUrl || runtimeUrl; | ||
|
|
||
| return { runtimeUrl, replicaUrl, useAccelerate }; | ||
| } | ||
|
|
||
| let basePrisma: PrismaClient = buildPrismaClient(resolveDatabaseUrls().runtimeUrl); | ||
| let basePrismaReplica: PrismaClient = buildPrismaClient(resolveDatabaseUrls().replicaUrl); | ||
| let currentRuntimeUrl = resolveDatabaseUrls().runtimeUrl; | ||
| let currentReplicaUrl = resolveDatabaseUrls().replicaUrl; | ||
| let currentUseAccelerate = resolveDatabaseUrls().useAccelerate; | ||
|
|
||
| applyPrismaClientMiddleware(basePrisma); | ||
| applyPrismaClientMiddleware(basePrismaReplica); | ||
|
|
||
| logger.info( | ||
| `[database] Runtime connection: ${useAccelerate ? "Prisma Accelerate (pooled)" : "direct PostgreSQL"}`, | ||
| `[database] Runtime connection: ${currentUseAccelerate ? "Prisma Accelerate (pooled)" : "direct PostgreSQL"}`, | ||
| ); | ||
| logger.info( | ||
| "[database] Migration connection: direct PostgreSQL via DATABASE_URL " + | ||
| "(run prisma migrate against DATABASE_URL, never against PRISMA_ACCELERATE_URL)", | ||
| ); | ||
|
|
||
| const basePrisma = new PrismaClient({ | ||
| datasources: { db: { url: databaseUrl } }, | ||
| log: [ | ||
| { level: "query", emit: "event" }, | ||
| { level: "error", emit: "stdout" }, | ||
| { level: "warn", emit: "stdout" }, | ||
| ], | ||
| }); | ||
|
|
||
| const replicaUrl = config.databaseUrlReplica || databaseUrl; | ||
|
|
||
| const basePrismaReplica = new PrismaClient({ | ||
| datasources: { db: { url: replicaUrl } }, | ||
| log: [ | ||
| { level: "query", emit: "event" }, | ||
| { level: "error", emit: "stdout" }, | ||
| { level: "warn", emit: "stdout" }, | ||
| ], | ||
| }); | ||
|
|
||
| // Retry config for connection pool exhaustion (Prisma Accelerate) | ||
| const MAX_RETRIES = 3; | ||
| const BASE_BACKOFF_MS = 200; | ||
|
|
||
| function isPoolExhaustionError(err: unknown): boolean { | ||
| if (err instanceof Prisma.PrismaClientKnownRequestError) { | ||
| return err.code === "P2024"; | ||
| } | ||
| return false; | ||
| } | ||
|
|
||
| // OTel: wrap every Prisma query in a span so traces link DB calls to parent spans | ||
| basePrisma.$use(async (params, next) => { | ||
| const tracer = trace.getTracer("prisma"); | ||
| const spanName = `prisma.${params.model ?? "raw"}.${params.action}`; | ||
| return tracer.startActiveSpan(spanName, async (span) => { | ||
| span.setAttributes({ | ||
| "db.system": "postgresql", | ||
| "db.operation": params.action, | ||
| ...(params.model ? { "db.prisma.model": params.model } : {}), | ||
| }); | ||
| try { | ||
| const result = await next(params); | ||
| span.setStatus({ code: SpanStatusCode.OK }); | ||
| return result; | ||
| } catch (err) { | ||
| span.setStatus({ code: SpanStatusCode.ERROR, message: String(err) }); | ||
| throw err; | ||
| } finally { | ||
| span.end(); | ||
| } | ||
| }); | ||
| }); | ||
| function refreshPrismaClientsIfNeeded(): void { | ||
| const resolved = resolveDatabaseUrls(); | ||
| if ( | ||
| resolved.runtimeUrl === currentRuntimeUrl && | ||
| resolved.replicaUrl === currentReplicaUrl && | ||
| resolved.useAccelerate === currentUseAccelerate | ||
| ) { | ||
| return; | ||
| } | ||
|
|
||
| // #388: Track per-query acquisition + execution latency for Prometheus. | ||
| // The "wait" component is estimated as the time before `next()` returns minus | ||
| // the query execution time; since Prisma doesn't expose a direct wait signal, | ||
| // we instrument total elapsed time labelled by model/action. | ||
| basePrisma.$use(async (params, next) => { | ||
| const end = poolAcquireHistogram.startTimer({ | ||
| model: params.model ?? "raw", | ||
| action: params.action, | ||
| logger.info("[database] Refreshing Prisma clients with updated connection settings", { | ||
| runtimeUrl: resolved.runtimeUrl, | ||
| replicaUrl: resolved.replicaUrl, | ||
| useAccelerate: resolved.useAccelerate, | ||
| }); | ||
|
Comment on lines
+195
to
199
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win Do not log raw database URLs.
Proposed change logger.info("[database] Refreshing Prisma clients with updated connection settings", {
- runtimeUrl: resolved.runtimeUrl,
- replicaUrl: resolved.replicaUrl,
+ runtimeConnection: resolved.useAccelerate ? "Prisma Accelerate (pooled)" : "direct PostgreSQL",
+ runtimeUrlChanged: resolved.runtimeUrl !== currentRuntimeUrl,
+ replicaUrlChanged: resolved.replicaUrl !== currentReplicaUrl,
useAccelerate: resolved.useAccelerate,
});🤖 Prompt for AI Agents |
||
| try { | ||
| return await next(params); | ||
| } finally { | ||
| end(); | ||
| } | ||
| }); | ||
|
|
||
| // Connection pool exhaustion retry middleware: retry with exponential backoff | ||
| // when Prisma Accelerate returns P2024 (connection pool timeout). | ||
| // Retries up to MAX_RETRIES-1 times (attempt 1 = first try, attempt 4 throws). | ||
| basePrisma.$use(async (params, next) => { | ||
| let lastError: unknown; | ||
| for (let attempt = 1; attempt <= MAX_RETRIES; attempt++) { | ||
| try { | ||
| return await next(params); | ||
| } catch (err) { | ||
| if (!isPoolExhaustionError(err)) { | ||
| throw err; | ||
| } | ||
| // #388: count every pool exhaustion regardless of retry outcome | ||
| poolExhaustedCounter.inc(); | ||
| if (attempt < MAX_RETRIES) { | ||
| lastError = err; | ||
| const backoff = BASE_BACKOFF_MS * 2 ** (attempt - 1); | ||
| logger.warn("Prisma connection pool exhausted, retrying", { | ||
| model: params.model, | ||
| action: params.action, | ||
| attempt, | ||
| maxRetries: MAX_RETRIES, | ||
| backoffMs: backoff, | ||
| }); | ||
| await new Promise((r) => setTimeout(r, backoff)); | ||
| } else { | ||
| throw err; | ||
| } | ||
| } | ||
| } | ||
| throw lastError; | ||
| }); | ||
| const previousBasePrisma = basePrisma; | ||
| const previousReplicaPrisma = basePrismaReplica; | ||
|
|
||
| basePrismaReplica.$use(async (params, next) => { | ||
| let lastError: unknown; | ||
| for (let attempt = 1; attempt <= MAX_RETRIES; attempt++) { | ||
| try { | ||
| return await next(params); | ||
| } catch (err) { | ||
| if (!isPoolExhaustionError(err)) { | ||
| throw err; | ||
| } | ||
| if (attempt < MAX_RETRIES) { | ||
| lastError = err; | ||
| const backoff = BASE_BACKOFF_MS * 2 ** (attempt - 1); | ||
| logger.warn("Prisma replica connection pool exhausted, retrying", { | ||
| model: params.model, | ||
| action: params.action, | ||
| attempt, | ||
| maxRetries: MAX_RETRIES, | ||
| backoffMs: backoff, | ||
| }); | ||
| await new Promise((r) => setTimeout(r, backoff)); | ||
| } else { | ||
| throw err; | ||
| } | ||
| } | ||
| } | ||
| throw lastError; | ||
| }); | ||
| basePrisma = buildPrismaClient(resolved.runtimeUrl); | ||
| basePrismaReplica = buildPrismaClient(resolved.replicaUrl); | ||
| currentRuntimeUrl = resolved.runtimeUrl; | ||
| currentReplicaUrl = resolved.replicaUrl; | ||
| currentUseAccelerate = resolved.useAccelerate; | ||
|
|
||
| applyPrismaClientMiddleware(basePrisma); | ||
| applyPrismaClientMiddleware(basePrismaReplica); | ||
|
|
||
| void previousBasePrisma.$disconnect().catch((err: unknown) => { | ||
| logger.warn("[database] Failed to disconnect previous Prisma client", { error: err }); | ||
| }); | ||
| void previousReplicaPrisma.$disconnect().catch((err: unknown) => { | ||
| logger.warn("[database] Failed to disconnect previous Prisma replica client", { error: err }); | ||
| }); | ||
|
Comment on lines
+201
to
+218
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift Swap clients only after the replacements connect successfully.
Also applies to: 272-275 🤖 Prompt for AI Agents |
||
|
|
||
| logger.info( | ||
| `[database] Runtime connection: ${currentUseAccelerate ? "Prisma Accelerate (pooled)" : "direct PostgreSQL"}`, | ||
| ); | ||
| } | ||
|
|
||
| export const prisma = useAccelerate ? basePrisma.$extends(withAccelerate()) : basePrisma; | ||
| export const prismaReplica = useAccelerate | ||
| export let prisma = currentUseAccelerate ? basePrisma.$extends(withAccelerate()) : basePrisma; | ||
| export let prismaReplica = currentUseAccelerate | ||
| ? basePrismaReplica.$extends(withAccelerate()) | ||
| : basePrismaReplica; | ||
|
|
||
|
|
@@ -255,6 +269,9 @@ export async function connectWithRetry(): Promise<void> { | |
| let lastError: unknown; | ||
| for (let attempt = 1; attempt <= maxRetries; attempt++) { | ||
| try { | ||
| refreshPrismaClientsIfNeeded(); | ||
| prisma = currentUseAccelerate ? basePrisma.$extends(withAccelerate()) : basePrisma; | ||
| prismaReplica = currentUseAccelerate ? basePrismaReplica.$extends(withAccelerate()) : basePrismaReplica; | ||
| await Promise.all([basePrisma.$connect(), basePrismaReplica.$connect()]); | ||
| if (attempt > 1) { | ||
| logger.info("[database] Connected after retry", { attempt }); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟠 Major
🧩 Analysis chain
🌐 Web query:
For@prisma/extension-accelerate3.0.1, should a Prisma Client using a prisma:// or prisma+postgres:// connection URL be extended with withAccelerate(), and should that decision be made per PrismaClient instance?💡 Result:
Yes, a Prisma Client instance using a prisma:// or prisma+postgres:// connection URL (for Prisma Accelerate) should generally be extended with the withAccelerate extension [1][2]. Extending with withAccelerate provides two primary benefits [1]: 1. It enables access to the cacheStrategy parameter within model queries, allowing for per-query caching configurations (such as ttl and swr) [1][3]. 2. It facilitates the use of advanced Accelerate features, such as withAccelerateInfo to inspect cache status [3]. While the Prisma Client will route queries through the Accelerate connection pool automatically if the connection string starts with prisma:// or prisma+postgres:// [1][4], applying the extension is standard practice to leverage the full suite of Accelerate's features [1][2]. Regarding instance management, this decision should be made per PrismaClient instance [1][4]. Because the extension is applied using the.$extends method on a specific instance, you must apply it to every instance that you intend to use with the Accelerate-specific features [1][3]. If you have multiple instances of PrismaClient, each one must be explicitly extended to utilize the Accelerate extension [1][5]. Note that for migration operations or other administrative tasks that require a direct database connection, you should typically use a separate, non-extended PrismaClient instance configured with your direct database URL, rather than the Accelerate URL [2][6].
Citations:
Per-client Accelerate flag for URL-specific configuration.
Derive
runtimeUseAccelerateandreplicaUseAccelerateindependently from their resolved URLs usingACCELERATE_PROTOCOL_RE, rather than relying on a single globaluseAccelerateflag derived fromPRISMA_ACCELERATE_URL. ApplyingwithAccelerate()to a client initialized with a direct database URL (e.g.,postgresql://) when a global flag is true can cause configuration errors or runtime failures, as the extension must match the specific connection protocol of the instance.Ensure the initialization at lines 225–227 and 272–274 consumes the specific flag for each client instance.
🤖 Prompt for AI Agents