From 783dfacef5254db8975756ddc243a7f423b6f245 Mon Sep 17 00:00:00 2001 From: Dan Lynch Date: Wed, 7 Oct 2026 00:56:34 +0000 Subject: [PATCH] fix(graphql-server): never mask errors; same formatting in every environment - rename maskError -> formatError: always return the real message, with extensions.code (BAD_USER_INPUT / INTERNAL_SERVER_ERROR when none) and an errorId + server log for internal errors; no NODE_ENV check - map SQLSTATE 42501 to FORBIDDEN in @constructive-io/errors - express error handler and graphile build failures return the real message - grafast explain is opt-in via graphile.explain / GRAPHILE_EXPLAIN instead of NODE_ENV=development --- graphql/env/README.md | 3 + graphql/env/src/env.ts | 2 + .../__tests__/upload.integration.test.ts | 6 +- ...ask-error.test.ts => format-error.test.ts} | 55 +++++--- .../server/src/middleware/error-handler.ts | 22 +-- graphql/server/src/middleware/format-error.ts | 102 ++++++++++++++ graphql/server/src/middleware/graphile.ts | 11 +- graphql/server/src/middleware/mask-error.ts | 133 ------------------ .../server/src/plugins/error-events-plugin.ts | 6 +- graphql/types/src/graphile.ts | 5 + packages/errors/__tests__/parse.test.ts | 9 +- packages/errors/src/pg.ts | 9 +- 12 files changed, 179 insertions(+), 184 deletions(-) rename graphql/server/src/middleware/__tests__/{mask-error.test.ts => format-error.test.ts} (63%) create mode 100644 graphql/server/src/middleware/format-error.ts delete mode 100644 graphql/server/src/middleware/mask-error.ts diff --git a/graphql/env/README.md b/graphql/env/README.md index ce2db48788..d3dc83879c 100644 --- a/graphql/env/README.md +++ b/graphql/env/README.md @@ -107,6 +107,9 @@ PostgreSQL extensions or change the API's exposed schemas. Each cache limit must be a safe integer of at least `2`. When omitted, Grafast's upstream default for that cache remains in effect. +### Grafast Explain +- `GRAPHILE_EXPLAIN` - Allow clients to request Grafast plan/SQL output via the `x-graphql-explain` header (off unless set) + ### Feature Flags - `FEATURES_SIMPLE_INFLECTION` - Enable simple inflection plugin - `FEATURES_OPPOSITE_BASE_NAMES` - Enable opposite base names diff --git a/graphql/env/src/env.ts b/graphql/env/src/env.ts index 046d4f78fb..4a74556839 100644 --- a/graphql/env/src/env.ts +++ b/graphql/env/src/env.ts @@ -14,6 +14,7 @@ export const getGraphQLEnvVars = (env: NodeJS.ProcessEnv = process.env): Partial const scopedIntrospection = getScopedIntrospectionEnv(env); const { GRAPHILE_SCHEMA, + GRAPHILE_EXPLAIN, GRAPHILE_QUERY_CACHE_MAX_LENGTH, GRAPHILE_OPERATIONS_CACHE_MAX_LENGTH, GRAPHILE_OPERATION_PLANS_CACHE_MAX_LENGTH, @@ -88,6 +89,7 @@ export const getGraphQLEnvVars = (env: NodeJS.ProcessEnv = process.env): Partial ) }) }), + ...(GRAPHILE_EXPLAIN && { explain: parseEnvBoolean(GRAPHILE_EXPLAIN) }), ...(GRAPHILE_SCHEMA && { schema: GRAPHILE_SCHEMA.includes(',') ? GRAPHILE_SCHEMA.split(',').map(s => s.trim()) diff --git a/graphql/server-test/__tests__/upload.integration.test.ts b/graphql/server-test/__tests__/upload.integration.test.ts index 013190a9b1..6da8adb976 100644 --- a/graphql/server-test/__tests__/upload.integration.test.ts +++ b/graphql/server-test/__tests__/upload.integration.test.ts @@ -224,9 +224,8 @@ const DELETE_APP_BUCKET = ` * PostgreSQL RLS denials surface in three ways through PostGraphile: * 1. An explicit PG error — message contains "permission denied", * "new row violates row-level security", or "No values were". - * 2. A masked internal error — in production mode PostGraphile masks - * PG errors with code INTERNAL_SERVER_ERROR (the raw message is - * only logged server-side). + * 2. A structured code — `42501` reaches clients as FORBIDDEN (the raw + * message is kept), unrecognized errors as INTERNAL_SERVER_ERROR. * 3. The mutation silently affects 0 rows and returns null or an * object with all-null fields (RLS USING clause filtered the row). * @@ -248,6 +247,7 @@ function expectRlsDenied( msg.includes('new row violates row-level security') || msg.includes('insufficient_privilege') || msg.includes('No values were') || + code === 'FORBIDDEN' || code === 'INTERNAL_SERVER_ERROR' ).toBe(true); return; diff --git a/graphql/server/src/middleware/__tests__/mask-error.test.ts b/graphql/server/src/middleware/__tests__/format-error.test.ts similarity index 63% rename from graphql/server/src/middleware/__tests__/mask-error.test.ts rename to graphql/server/src/middleware/__tests__/format-error.test.ts index e89c631c44..b7a40838b8 100644 --- a/graphql/server/src/middleware/__tests__/mask-error.test.ts +++ b/graphql/server/src/middleware/__tests__/format-error.test.ts @@ -10,7 +10,7 @@ import { validate, } from 'graphql'; -import { maskError } from '../mask-error'; +import { formatError } from '../format-error'; const ResetPasswordInput = new GraphQLInputObjectType({ name: 'ResetPasswordInput', @@ -53,21 +53,11 @@ const run = async (query: string, variables?: Record) => { expect(raised.length).toBeGreaterThan(0); return raised.map( - (error) => maskError(error) as { message: string; extensions?: Record } + (error) => formatError(error) as { message: string; extensions?: Record } ); }; -describe('maskError', () => { - const nodeEnv = process.env.NODE_ENV; - - beforeAll(() => { - process.env.NODE_ENV = 'production'; - }); - - afterAll(() => { - process.env.NODE_ENV = nodeEnv; - }); - +describe('formatError', () => { it('surfaces an input field the schema does not define', async () => { const [result] = await run('mutation($i: ResetPasswordInput!){ resetPassword(input: $i) }', { i: { userId: 'role-1', roleId: 'role-1', newPassword: 'secret' }, @@ -101,17 +91,48 @@ describe('maskError', () => { extensions: { code: 'PERSISTED_QUERY_NOT_FOUND' }, }); - const result = maskError(error) as { message: string; extensions?: Record }; + const result = formatError(error) as { message: string; extensions?: Record }; expect(result.message).toBe('PersistedQueryNotFound'); expect(result.extensions?.code).toBe('PERSISTED_QUERY_NOT_FOUND'); }); - it('masks an unrecognized error raised while resolving a field', async () => { + it('surfaces an unrecognized resolver error with its real message', async () => { const [result] = await run('mutation{ brokenField }'); - expect(result.message).toMatch(/^An unexpected error occurred\. Reference: [0-9a-f]{16}$/); + expect(result.message).toBe('relation "internal_secrets" does not exist'); expect(result.extensions?.code).toBe('INTERNAL_SERVER_ERROR'); - expect(result.extensions?.errorId).toEqual(expect.any(String)); + expect(result.extensions?.errorId).toMatch(/^[0-9a-f]{16}$/); }); + + it('surfaces a permission refusal from postgres as FORBIDDEN', () => { + const pgError = Object.assign(new Error('permission denied for table agent_thread'), { + code: '42501', + }); + const error = new GraphQLError(pgError.message, { path: ['agentThreads'], originalError: pgError }); + + const result = formatError(error) as { message: string; extensions?: Record }; + + expect(result.message).toBe('permission denied for table agent_thread'); + expect(result.extensions?.code).toBe('FORBIDDEN'); + expect(result.extensions?.class).toBe('public'); + expect(result.extensions?.errorId).toBeUndefined(); + }); + + it.each(['production', 'development', 'test', undefined])( + 'formats errors identically when NODE_ENV is %s', + async (env) => { + const previous = process.env.NODE_ENV; + if (env === undefined) delete process.env.NODE_ENV; + else process.env.NODE_ENV = env; + try { + const [result] = await run('mutation{ brokenField }'); + expect(result.message).toBe('relation "internal_secrets" does not exist'); + expect(result.extensions?.code).toBe('INTERNAL_SERVER_ERROR'); + } finally { + if (previous === undefined) delete process.env.NODE_ENV; + else process.env.NODE_ENV = previous; + } + } + ); }); diff --git a/graphql/server/src/middleware/error-handler.ts b/graphql/server/src/middleware/error-handler.ts index 012bc53bac..56862f52fa 100644 --- a/graphql/server/src/middleware/error-handler.ts +++ b/graphql/server/src/middleware/error-handler.ts @@ -1,6 +1,5 @@ import './types'; -import { getNodeEnv } from '@pgpmjs/env'; import { Logger } from '@pgpmjs/logger'; import type { ErrorRequestHandler, NextFunction, Request, Response } from 'express'; @@ -10,8 +9,6 @@ import { isApiError } from '../errors/api-errors'; const log = new Logger('error-handler'); -const isDevelopment = (): boolean => getNodeEnv() === 'development'; - const wantsJson = (req: Request): boolean => { const accept = req.get('Accept') || ''; return accept.includes('application/json') @@ -19,15 +16,6 @@ const wantsJson = (req: Request): boolean => { || Boolean(req.is('json')); }; -const sanitizeMessage = (error: Error): string => { - if (isDevelopment()) return error.message; - if (isApiError(error)) return error.message; - if (error.message?.includes('ECONNREFUSED')) return 'Service temporarily unavailable'; - if (error.message?.includes('timeout') || error.message?.includes('ETIMEDOUT')) return 'Request timed out'; - if (error.message?.includes('does not exist')) return 'The requested resource does not exist'; - return 'An unexpected error occurred'; -}; - interface ErrorResponse { statusCode: number; code: string; @@ -45,7 +33,7 @@ const categorizeError = (err: Error): ErrorResponse => { return { statusCode: err.statusCode, code: err.code, - message: sanitizeMessage(err), + message: err.message, logLevel: err.statusCode >= 500 ? 'error' : 'warn', }; } @@ -54,12 +42,12 @@ const categorizeError = (err: Error): ErrorResponse => { return { statusCode: 403, code, message: err.message, logLevel: 'warn' }; } if (err.message?.includes('ECONNREFUSED') || err.message?.includes('connection terminated')) { - return { statusCode: 503, code: 'SERVICE_UNAVAILABLE', message: sanitizeMessage(err), logLevel: 'error' }; + return { statusCode: 503, code: 'SERVICE_UNAVAILABLE', message: err.message, logLevel: 'error' }; } if (err.message?.includes('timeout') || err.message?.includes('ETIMEDOUT')) { - return { statusCode: 504, code: 'GATEWAY_TIMEOUT', message: sanitizeMessage(err), logLevel: 'error' }; + return { statusCode: 504, code: 'GATEWAY_TIMEOUT', message: err.message, logLevel: 'error' }; } - return { statusCode: 500, code: 'INTERNAL_ERROR', message: sanitizeMessage(err), logLevel: 'error' }; + return { statusCode: 500, code: 'INTERNAL_ERROR', message: err.message, logLevel: 'error' }; }; const sendResponse = (req: Request, res: Response, { statusCode, code, message }: ErrorResponse): void => { @@ -84,7 +72,7 @@ const logError = (err: Error, req: Request, level: 'warn' | 'error'): void => { if (isApiError(err)) { log[level]({ event: 'api_error', code: err.code, statusCode: err.statusCode, message: err.message, ...context }); } else { - log[level]({ event: 'unexpected_error', name: err.name, message: err.message, stack: isDevelopment() ? err.stack : undefined, ...context }); + log[level]({ event: 'unexpected_error', name: err.name, message: err.message, stack: err.stack, ...context }); } }; diff --git a/graphql/server/src/middleware/format-error.ts b/graphql/server/src/middleware/format-error.ts new file mode 100644 index 0000000000..fa6938c24f --- /dev/null +++ b/graphql/server/src/middleware/format-error.ts @@ -0,0 +1,102 @@ +import crypto from 'node:crypto'; + +import { type ErrorContext, parse } from '@constructive-io/errors'; +import { Logger } from '@pgpmjs/logger'; +import { type GraphQLError, type GraphQLFormattedError } from 'graphql'; + +const formatErrorLog = new Logger('graphile:formatError'); + +/** + * An error the GraphQL layer raised about the *request*, before any resolver + * ran: an unknown input field, a value of the wrong type, a missing required + * variable. graphql-js reports variable coercion without an `extensions.code` + * (unlike parse/validation, which carry `GRAPHQL_PARSE_FAILED` / + * `GRAPHQL_VALIDATION_FAILED`), so code-based classification alone would read + * it as a server failure instead of the client's own malformed query. + * + * A request error is answered before a field is resolved, so it carries no + * response `path` — every execution error has one. Coercion wraps the inner + * complaint about the value, so `originalError` may be set, but only ever to + * another GraphQL-layer error: anything a resolver or the database threw arrives + * as a foreign error (a pg error, an `Error`). The wrap + * is recognized by name rather than by `instanceof`, because the error is raised + * by whichever copy of graphql-js grafast resolved, not by this package's. + */ +const isGraphQLLayerError = (value: unknown): boolean => + value == null || + ((value as Error).name === 'GraphQLError' && + isGraphQLLayerError((value as GraphQLError).originalError)); + +const isRequestError = (error: GraphQLError): boolean => + error.path == null && isGraphQLLayerError((error as { originalError?: unknown }).originalError); + +/** The code a request error carries when graphql-js supplied none. */ +const BAD_USER_INPUT = 'BAD_USER_INPUT'; + +/** The code any other error carries when nothing supplied one. */ +const INTERNAL_SERVER_ERROR = 'INTERNAL_SERVER_ERROR'; + +/** + * Normalize any GraphQL/database error into a canonical Constructive shape. + * + * Database errors surface through Grafast without a populated `extensions.code` + * (the semantic code lives in the message, and any SQLSTATE/DETAIL lives on the + * underlying pg error at `originalError`). We parse `originalError` first so we + * can recover the structured code, then fall back to the GraphQL error itself. + */ +export const normalizeError = ( + error: GraphQLError, +): { code: string | null; context: ErrorContext; class: 'public' | 'internal' } => { + const original = (error as { originalError?: unknown }).originalError; + const fromOriginal = original ? parse(original) : null; + const parsed = fromOriginal?.code ? fromOriginal : parse(error); + return { code: parsed.code, context: parsed.context, class: parsed.class }; +}; + +/** + * Format every GraphQL error the same way, in every environment. Nothing is + * masked: the client always receives the real message. + * + * 1. Lift the structured code onto `extensions.code`/`class`/`context` from the + * parsed error, so database errors reach clients with a machine-readable + * code instead of a bare message with empty `extensions`. + * 2. A request error graphql-js raised without a code is `BAD_USER_INPUT`. + * 3. Any other error without a code is `INTERNAL_SERVER_ERROR`. + * 4. Every internal error (unknown, or registered as internal) carries an + * `errorId` and is logged under it, so a report can be matched to the log. + */ +export const formatError = (error: GraphQLError): GraphQLFormattedError => { + const { code, context, class: errorClass } = normalizeError(error); + + // `extensions` is read-only on GraphQLError, so build a formatted error + // rather than mutating it. + const extensions: Record = { ...error.extensions }; + if (code) { + extensions.code = code; + extensions.class = errorClass; + if (Object.keys(context).length > 0) { + extensions.context = context; + } + } + + let internal = Boolean(code) && errorClass === 'internal'; + if (!code && !error.extensions?.code) { + const requestError = isRequestError(error); + extensions.code = requestError ? BAD_USER_INPUT : INTERNAL_SERVER_ERROR; + internal = !requestError; + } + + if (internal) { + const errorId = crypto.randomBytes(8).toString('hex'); + extensions.errorId = errorId; + formatErrorLog.error(`[graphql-error:${errorId}]`, error); + } + + // grafserv strips originalError before serializing to the client. + return { + message: error.message, + ...(error.locations ? { locations: error.locations } : {}), + ...(error.path ? { path: error.path } : {}), + extensions + }; +}; diff --git a/graphql/server/src/middleware/graphile.ts b/graphql/server/src/middleware/graphile.ts index e888d0acff..ce9b468e5c 100644 --- a/graphql/server/src/middleware/graphile.ts +++ b/graphql/server/src/middleware/graphile.ts @@ -4,7 +4,6 @@ import { errors } from '@constructive-io/errors'; import type { ComputeConfig } from '@constructive-io/express-context'; import { DEFAULT_REQUEST_PROTECTION, protectionPgSettings } from '@constructive-io/express-context'; import type { ConstructiveOptions } from '@constructive-io/graphql-types'; -import { getNodeEnv } from '@pgpmjs/env'; import { Logger } from '@pgpmjs/logger'; import type { NextFunction, Request, RequestHandler, Response } from 'express'; import { createGraphileInstance, graphileCache,type GraphileCacheEntry } from 'graphile-cache'; @@ -24,12 +23,10 @@ import { AuthCookiePlugin } from '../plugins/auth-cookie-plugin'; import { createErrorEventsPlugin } from '../plugins/error-events-plugin'; import { RequestProtectionPlugin } from '../plugins/request-protection-plugin'; import type { DatabaseSettings } from '../types'; +import { formatError } from './format-error'; import { makeIntrospectionWiring } from './graphile-introspection'; -import { maskError } from './mask-error'; import { observeGraphileBuild } from './observability/graphile-build-stats'; -const isDev = (): boolean => getNodeEnv() === 'development'; - // ============================================================================= // Single-Flight Pattern: In-Flight Tracking // ============================================================================= @@ -131,10 +128,10 @@ const buildPreset = async ( graphiqlPath: '/graphiql', graphiql: true, graphiqlOnGraphQLGET: false, - maskError + maskError: formatError }, grafast: { - explain: process.env.NODE_ENV === 'development', + explain: graphileOptions?.explain === true, context: (requestContext: Partial) => { // In grafserv/express/v4, the request is available at requestContext.expressv4.req const req = (requestContext as { expressv4?: { req?: Request } })?.expressv4?.req; @@ -416,7 +413,7 @@ export const graphile = (opts: ConstructiveOptions): RequestHandler => { respondWithGraphQLError( res, errors.INTERNAL_FAILURE({ - details: isDev() ? e?.message ?? String(e) : 'An unexpected error occurred' + details: e?.message ?? String(e) }) ); return; diff --git a/graphql/server/src/middleware/mask-error.ts b/graphql/server/src/middleware/mask-error.ts deleted file mode 100644 index 958afd228d..0000000000 --- a/graphql/server/src/middleware/mask-error.ts +++ /dev/null @@ -1,133 +0,0 @@ -import crypto from 'node:crypto'; - -import { classify, type ErrorContext, parse } from '@constructive-io/errors'; -import { getNodeEnv } from '@pgpmjs/env'; -import { Logger } from '@pgpmjs/logger'; -import { type GraphQLError, type GraphQLFormattedError } from 'graphql'; - -const maskErrorLog = new Logger('graphile:maskError'); - -/** - * GraphQL framework protocol codes. These originate in the GraphQL/grafast - * transport layer (not in constructive-db), so they are not Constructive domain - * codes in the `@constructive-io/errors` registry. They are always safe to - * surface — they carry no sensitive detail. Everything else (auth, account, - * resource, constraint, and every constructive-db code) is classified by the - * registry, which is the single source of truth for public vs. internal. - */ -const GRAPHQL_PROTOCOL_CODES = new Set([ - 'GRAPHQL_VALIDATION_FAILED', - 'GRAPHQL_PARSE_FAILED', - 'PERSISTED_QUERY_NOT_FOUND', - 'PERSISTED_QUERY_NOT_SUPPORTED' -]); - -/** A code is safe to surface when the registry classifies it public, or it is a - * GraphQL framework protocol code. */ -const isPublicCode = (code: string | null | undefined): boolean => - Boolean(code) && (classify(code) === 'public' || GRAPHQL_PROTOCOL_CODES.has(code as string)); - -/** - * An error the GraphQL layer raised about the *request*, before any resolver - * ran: an unknown input field, a value of the wrong type, a missing required - * variable. graphql-js reports variable coercion without an `extensions.code` - * (unlike parse/validation, which carry `GRAPHQL_PARSE_FAILED` / - * `GRAPHQL_VALIDATION_FAILED`), so code-based classification alone reads it as - * unknown and masks it — telling a client its own malformed query was a server - * failure, with a reference id pointing at nothing. - * - * A request error is answered before a field is resolved, so it carries no - * response `path` — every execution error has one. Coercion wraps the inner - * complaint about the value, so `originalError` may be set, but only ever to - * another GraphQL-layer error: anything a resolver or the database threw arrives - * as a foreign error (a pg error, an `Error`) and is masked as before. The wrap - * is recognized by name rather than by `instanceof`, because the error is raised - * by whichever copy of graphql-js grafast resolved, not by this package's. - */ -const isGraphQLLayerError = (value: unknown): boolean => - value == null || - ((value as Error).name === 'GraphQLError' && - isGraphQLLayerError((value as GraphQLError).originalError)); - -const isRequestError = (error: GraphQLError): boolean => - error.path == null && isGraphQLLayerError((error as { originalError?: unknown }).originalError); - -/** The code a surfaced request error carries when graphql-js supplied none. */ -const BAD_USER_INPUT = 'BAD_USER_INPUT'; - -/** - * Normalize any GraphQL/database error into a canonical Constructive shape. - * - * Database errors surface through Grafast without a populated `extensions.code` - * (the semantic code lives in the message, and any SQLSTATE/DETAIL lives on the - * underlying pg error at `originalError`). We parse `originalError` first so we - * can recover the structured code, then fall back to the GraphQL error itself. - */ -export const normalizeError = ( - error: GraphQLError, -): { code: string | null; context: ErrorContext; class: 'public' | 'internal' } => { - const original = (error as { originalError?: unknown }).originalError; - const fromOriginal = original ? parse(original) : null; - const parsed = fromOriginal?.code ? fromOriginal : parse(error); - return { code: parsed.code, context: parsed.context, class: parsed.class }; -}; - -/** - * Production-aware error handling backed by `@constructive-io/errors`. - * - * 1. Enrich `extensions.code`/`class`/`context` from the parsed error so clients - * always receive a machine-readable code (fixing the gap where database - * errors reached clients as a bare message with empty `extensions`). - * 2. Surface public (registered/allowlisted) errors as-is. - * 3. In development, pass everything through (enriched) for debugging. - * 4. In production, mask internal/unknown errors behind a reference ID and log - * the original. - */ -export const maskError = (error: GraphQLError): GraphQLError | GraphQLFormattedError => { - const { code, context, class: errorClass } = normalizeError(error); - - // Lift the structured code onto extensions for every recognized error so - // clients always receive a machine-readable code (`extensions` is read-only - // on GraphQLError, so we build a formatted error rather than mutating it). - const extensions: Record = { ...error.extensions }; - if (code) { - extensions.code = code; - extensions.class = errorClass; - if (Object.keys(context).length > 0) { - extensions.context = context; - } - } - - const effectiveCode = code ?? (error.extensions?.code as string | undefined); - if (!effectiveCode && isRequestError(error)) { - extensions.code = BAD_USER_INPUT; - return { - message: error.message, - ...(error.locations ? { locations: error.locations } : {}), - extensions, - } as GraphQLFormattedError; - } - - if (isPublicCode(effectiveCode) || getNodeEnv() === 'development') { - // Note: grafserv strips originalError and internal extensions before - // serializing to the client, so returning the enriched error is safe. - return { - message: error.message, - ...(error.locations ? { locations: error.locations } : {}), - ...(error.path ? { path: error.path } : {}), - extensions, - } as GraphQLFormattedError; - } - - // Mask internal/unknown errors with a reference ID. - const errorId = crypto.randomBytes(8).toString('hex'); - maskErrorLog.error(`[masked-error:${errorId}]`, error); - - return { - message: `An unexpected error occurred. Reference: ${errorId}`, - extensions: { - code: 'INTERNAL_SERVER_ERROR', - errorId - } - } as GraphQLFormattedError; -}; diff --git a/graphql/server/src/plugins/error-events-plugin.ts b/graphql/server/src/plugins/error-events-plugin.ts index b2bb061be0..8dfc8ea202 100644 --- a/graphql/server/src/plugins/error-events-plugin.ts +++ b/graphql/server/src/plugins/error-events-plugin.ts @@ -9,7 +9,7 @@ import { getOperationAST } from 'graphql'; import { escapeIdentifier, type Pool } from 'pg'; import { withPgClient } from 'pg-query-context'; -import { normalizeError } from '../middleware/mask-error'; +import { normalizeError } from '../middleware/format-error'; const log = new Logger('error-events'); @@ -19,8 +19,8 @@ const getExpressRequest = ( /** * The first structured, public-classified registry code among the errors. - * Internal/unknown errors are bugs, not refusals: they are masked and logged - * by `maskError` and never recorded as tenant events. + * Internal/unknown errors are bugs, not refusals: they are logged by + * `formatError` and never recorded as tenant events. */ const refusalCode = (errors: readonly GraphQLError[] | undefined): string | undefined => { for (const error of errors ?? []) { diff --git a/graphql/types/src/graphile.ts b/graphql/types/src/graphile.ts index ac3248aef9..3b17505503 100644 --- a/graphql/types/src/graphile.ts +++ b/graphql/types/src/graphile.ts @@ -65,6 +65,11 @@ export interface GraphileOptions { preset?: Partial; /** Explicit per-schema Grafast cache bounds used for tenant-density control. */ grafastCache?: GrafastCacheLimits; + /** + * Allow clients to request Grafast plan/SQL output with the + * `x-graphql-explain` header. Off unless set. + */ + explain?: boolean; } /** diff --git a/packages/errors/__tests__/parse.test.ts b/packages/errors/__tests__/parse.test.ts index 7af662c9dc..2393de660b 100644 --- a/packages/errors/__tests__/parse.test.ts +++ b/packages/errors/__tests__/parse.test.ts @@ -46,7 +46,14 @@ describe('parse', () => { expect(result.context.constraint).toBe('users_email_key'); }); - it('classifies unknown codes as internal (masked)', () => { + it('maps an insufficient-privilege SQLSTATE to FORBIDDEN', () => { + const result = parse({ message: 'permission denied for table agent_thread', code: '42501', table: 'agent_thread' }); + expect(result.code).toBe('FORBIDDEN'); + expect(result.class).toBe('public'); + expect(result.context.table).toBe('agent_thread'); + }); + + it('classifies unknown codes as internal', () => { const result = parse({ message: 'DATA_INVARIANT_BROKEN', code: 'P0001' }); expect(result.code).toBe('DATA_INVARIANT_BROKEN'); expect(result.known).toBe(false); diff --git a/packages/errors/src/pg.ts b/packages/errors/src/pg.ts index 59fa173686..33faef338e 100644 --- a/packages/errors/src/pg.ts +++ b/packages/errors/src/pg.ts @@ -1,15 +1,18 @@ import type { PgErrorFields } from './types'; /** - * PostgreSQL SQLSTATE codes for the native constraint violations we surface as - * public Constructive codes. + * PostgreSQL SQLSTATE codes for the native errors we surface as public + * Constructive codes: constraint violations, and `42501` (a missing grant or an + * RLS `WITH CHECK` refusal). Postgres cannot tell an anonymous caller from a + * signed-in one at that point, so `42501` is `FORBIDDEN`. */ export const SQLSTATE_TO_CODE: Record = { 23505: 'UNIQUE_VIOLATION', 23503: 'FOREIGN_KEY_VIOLATION', 23502: 'NOT_NULL_VIOLATION', 23514: 'CHECK_VIOLATION', - '23P01': 'EXCLUSION_VIOLATION' + '23P01': 'EXCLUSION_VIOLATION', + 42501: 'FORBIDDEN' }; /** SQLSTATE for a user-raised `RAISE EXCEPTION` without an explicit ERRCODE. */