From 903b116121f4cec03bafc58e754951cc5bb26ada Mon Sep 17 00:00:00 2001 From: Dan Lynch Date: Sat, 3 Oct 2026 23:52:22 +0000 Subject: [PATCH] fix(graphql-server): mask internal errors regardless of NODE_ENV; map SQLSTATE 42501 to FORBIDDEN Unknown/internal errors are now always masked behind a reference id; raw detail (and grafast explain) is only returned under an explicit server.exposeErrors / SERVER_EXPOSE_ERRORS opt-in. Native privilege refusals (42501) surface as the registered FORBIDDEN code with the registry message instead of the table name. --- .../references/environment-configuration.md | 1 + .../__snapshots__/merge.test.ts.snap | 1 + .../seed/error-masking/schema.sql | 25 +++++ .../error-masking.integration.test.ts | 101 ++++++++++++++++++ .../__tests__/upload.integration.test.ts | 14 ++- .../middleware/__tests__/mask-error.test.ts | 35 +++++- graphql/server/src/middleware/auth.ts | 4 +- .../server/src/middleware/error-handler.ts | 39 ++++--- graphql/server/src/middleware/graphile.ts | 12 +-- graphql/server/src/middleware/mask-error.ts | 67 +++++++++--- graphql/server/src/server.ts | 4 +- packages/errors/README.md | 3 +- packages/errors/__tests__/parse.test.ts | 12 +++ packages/errors/src/pg.ts | 9 +- .../__snapshots__/merge.test.ts.snap | 1 + pgpm/env/src/env.ts | 2 + pgpm/types/src/pgpm.ts | 9 +- 17 files changed, 285 insertions(+), 54 deletions(-) create mode 100644 graphql/server-test/__fixtures__/seed/error-masking/schema.sql create mode 100644 graphql/server-test/__tests__/error-masking.integration.test.ts diff --git a/.agents/skills/pgpm/references/environment-configuration.md b/.agents/skills/pgpm/references/environment-configuration.md index 9aa0541096..527abaf8e2 100644 --- a/.agents/skills/pgpm/references/environment-configuration.md +++ b/.agents/skills/pgpm/references/environment-configuration.md @@ -123,6 +123,7 @@ const deployOptions = getDeploymentEnvOptions(); | `SERVER_TRUST_PROXY` | Trust proxy headers | | `SERVER_ORIGIN` | Server origin URL | | `SERVER_STRICT_AUTH` | Strict authentication mode | +| `SERVER_EXPOSE_ERRORS` | Return raw internal errors to clients (local debugging only; default `false`, masked) | ### CDN/Storage diff --git a/graphql/env/__tests__/__snapshots__/merge.test.ts.snap b/graphql/env/__tests__/__snapshots__/merge.test.ts.snap index 796744fd63..77e81439cd 100644 --- a/graphql/env/__tests__/__snapshots__/merge.test.ts.snap +++ b/graphql/env/__tests__/__snapshots__/merge.test.ts.snap @@ -88,6 +88,7 @@ exports[`getEnvOptions merges pgpm defaults, graphql defaults, config, env, and "user": "env-user", }, "server": { + "exposeErrors": false, "host": "localhost", "port": 5000, "strictAuth": false, diff --git a/graphql/server-test/__fixtures__/seed/error-masking/schema.sql b/graphql/server-test/__fixtures__/seed/error-masking/schema.sql new file mode 100644 index 0000000000..a6553c2d3f --- /dev/null +++ b/graphql/server-test/__fixtures__/seed/error-masking/schema.sql @@ -0,0 +1,25 @@ +-- Error-masking fixture: a table the anonymous role holds no grant on, a +-- mutation refused with a registered public code, and a query that fails with +-- an unexpected database error. +-- +-- Compose after app-schemas/simple-pets/schema.sql and scoped/test-data.sql. + +CREATE TABLE "simple-pets-public".vault_items ( + id serial PRIMARY KEY, + secret text NOT NULL +); +REVOKE ALL ON "simple-pets-public".vault_items FROM anonymous, authenticated, PUBLIC; + +CREATE FUNCTION "simple-pets-public".invite_member(address text) +RETURNS boolean AS $$ +BEGIN + RAISE EXCEPTION 'INVITE_ADDRESS_REQUIRED'; +END; +$$ LANGUAGE plpgsql VOLATILE; + +CREATE FUNCTION "simple-pets-public".vault_ratio() +RETURNS integer AS $$ +BEGIN + RETURN 1 / 0; +END; +$$ LANGUAGE plpgsql STABLE; diff --git a/graphql/server-test/__tests__/error-masking.integration.test.ts b/graphql/server-test/__tests__/error-masking.integration.test.ts new file mode 100644 index 0000000000..37d2e5d571 --- /dev/null +++ b/graphql/server-test/__tests__/error-masking.integration.test.ts @@ -0,0 +1,101 @@ +/** + * What an anonymous client of a public API sees when a request fails, over + * real HTTP through the scoped-routing server. + * + * Masking must hold whatever NODE_ENV says: the suite runs with NODE_ENV + * `development` — what an unset NODE_ENV reads as — so a deployment that + * forgets to set it still never returns raw PostgreSQL errors. + * + * Run tests: + * pnpm test -- --testPathPattern=error-masking + */ +import path from 'path'; +import type supertest from 'supertest'; + +import { getConnections, seed } from '../src'; + +jest.setTimeout(30000); + +const sharedSeedRoot = path.join(__dirname, '..', '..', '..', '__fixtures__', 'seed'); +const localSeedRoot = path.join(__dirname, '..', '__fixtures__', 'seed', 'error-masking'); +const pgpmWorkspace = path.join(sharedSeedRoot, '..', '..'); +const metaSchemas = [ + 'catalog_private', + 'routing_public', + 'apps_public', + 'metaschema_public', + 'metaschema_modules_public' +]; + +const HOST = 'app.test.constructive.io'; +const MASKED = /^An unexpected error occurred\. Reference: [0-9a-f]{16}$/; + +const nodeEnv = process.env.NODE_ENV; +let request: supertest.Agent; +let teardown: () => Promise; + +const post = async (query: string) => { + const res = await request.post('/graphql').set('Host', HOST).send({ query }); + expect(res.status).toBe(200); + return res.body as { + data?: unknown; + errors?: { message: string; extensions?: Record }[]; + extensions?: unknown; + }; +}; + +beforeAll(async () => { + process.env.NODE_ENV = 'development'; + ({ request, teardown } = await getConnections( + { + schemas: ['simple-pets-public', 'simple-pets-pets-public'], + authRole: 'anonymous', + server: { useRouting: true, api: { isPublic: true, metaSchemas } } + }, + [ + seed.pgpm(pgpmWorkspace), + seed.sqlfile([ + path.join(sharedSeedRoot, 'app-schemas', 'simple-pets', 'schema.sql'), + path.join(sharedSeedRoot, 'scoped', 'test-data.sql'), + path.join(localSeedRoot, 'schema.sql') + ]) + ] + )); +}); + +afterAll(async () => { + await teardown(); + process.env.NODE_ENV = nodeEnv; +}); + +describe('error masking for anonymous callers', () => { + it('answers a read of a table anon holds no grant on with FORBIDDEN, naming nothing', async () => { + const body = await post('{ vaultItems { nodes { id secret } } }'); + + expect(body.errors).toHaveLength(1); + expect(body.errors![0].extensions?.code).toBe('FORBIDDEN'); + expect(body.errors![0].message).toBe('You do not have permission to do that.'); + expect(body.extensions).toBeUndefined(); + expect(JSON.stringify(body)).not.toMatch(/vault_items|simple-pets|permission denied/); + }); + + it('passes a registered Constructive error code through unchanged', async () => { + const body = await post('mutation { inviteMember(input: { address: "" }) { clientMutationId } }'); + + expect(body.errors).toHaveLength(1); + expect(body.errors![0].message).toBe('INVITE_ADDRESS_REQUIRED'); + expect(body.errors![0].extensions?.code).toBe('INVITE_ADDRESS_REQUIRED'); + }); + + it('masks an unexpected database error behind a reference id', async () => { + const body = await post('{ vaultRatio }'); + + expect(body.errors).toHaveLength(1); + expect(body.errors![0].message).toMatch(MASKED); + expect(body.errors![0].extensions).toEqual({ + code: 'INTERNAL_SERVER_ERROR', + errorId: expect.any(String) + }); + expect(JSON.stringify(body)).not.toMatch(/division by zero/); + }); +}); diff --git a/graphql/server-test/__tests__/upload.integration.test.ts b/graphql/server-test/__tests__/upload.integration.test.ts index 013190a9b1..4f675a001f 100644 --- a/graphql/server-test/__tests__/upload.integration.test.ts +++ b/graphql/server-test/__tests__/upload.integration.test.ts @@ -222,11 +222,11 @@ const DELETE_APP_BUCKET = ` * Assert that a mutation was denied specifically by RLS (not by some other error). * * 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). + * 1. A privilege refusal — a missing grant or an RLS refusal (SQLSTATE + * 42501) is surfaced as FORBIDDEN, or the mutation matched no row + * ("No values were ..."). + * 2. A masked internal error — code INTERNAL_SERVER_ERROR (the raw + * message is only logged server-side). * 3. The mutation silently affects 0 rows and returns null or an * object with all-null fields (RLS USING clause filtered the row). * @@ -244,9 +244,7 @@ function expectRlsDenied( // Reject GraphQL validation errors — these indicate a bug in the test expect(code).not.toBe('GRAPHQL_VALIDATION_FAILED'); expect( - msg.includes('permission denied') || - msg.includes('new row violates row-level security') || - msg.includes('insufficient_privilege') || + code === 'FORBIDDEN' || msg.includes('No values were') || code === 'INTERNAL_SERVER_ERROR' ).toBe(true); diff --git a/graphql/server/src/middleware/__tests__/mask-error.test.ts b/graphql/server/src/middleware/__tests__/mask-error.test.ts index e89c631c44..d84d52157b 100644 --- a/graphql/server/src/middleware/__tests__/mask-error.test.ts +++ b/graphql/server/src/middleware/__tests__/mask-error.test.ts @@ -60,8 +60,10 @@ const run = async (query: string, variables?: Record) => { describe('maskError', () => { const nodeEnv = process.env.NODE_ENV; + // Masking must not depend on NODE_ENV: a deployment that leaves it unset (which + // reads as development) still masks. beforeAll(() => { - process.env.NODE_ENV = 'production'; + process.env.NODE_ENV = 'development'; }); afterAll(() => { @@ -114,4 +116,35 @@ describe('maskError', () => { expect(result.extensions?.code).toBe('INTERNAL_SERVER_ERROR'); expect(result.extensions?.errorId).toEqual(expect.any(String)); }); + + it('passes an internal error through only when exposeErrors is opted into', async () => { + const [error] = (await execute({ schema, document: parse('mutation{ brokenField }') })).errors ?? []; + + const result = maskError(error, { exposeErrors: true }) as { message: string }; + + expect(result.message).toBe('relation "internal_secrets" does not exist'); + }); + + it('surfaces a native privilege refusal as FORBIDDEN without naming the table', () => { + 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 = maskError(error) as { message: string; extensions?: Record }; + + expect(result.message).toBe('You do not have permission to do that.'); + expect(result.message).not.toContain('agent_thread'); + expect(result.extensions?.code).toBe('FORBIDDEN'); + expect(result.extensions?.class).toBe('public'); + expect(result.extensions?.errorId).toBeUndefined(); + }); + + it('passes a registered public code through unchanged', () => { + const pgError = Object.assign(new Error('INVITE_ADDRESS_REQUIRED'), { code: 'P0001' }); + const error = new GraphQLError(pgError.message, { path: ['submitInvite'], originalError: pgError }); + + const result = maskError(error) as { message: string; extensions?: Record }; + + expect(result.message).toBe('INVITE_ADDRESS_REQUIRED'); + expect(result.extensions?.code).toBe('INVITE_ADDRESS_REQUIRED'); + }); }); diff --git a/graphql/server/src/middleware/auth.ts b/graphql/server/src/middleware/auth.ts index ef6da3f3a2..532f953ea2 100644 --- a/graphql/server/src/middleware/auth.ts +++ b/graphql/server/src/middleware/auth.ts @@ -1,7 +1,6 @@ import './types'; // for Request type import { errors } from '@constructive-io/errors'; -import { getNodeEnv } from '@pgpmjs/env'; import { Logger } from '@pgpmjs/logger'; import { PgpmOptions } from '@pgpmjs/types'; import { NextFunction, Request, RequestHandler, Response } from 'express'; @@ -11,7 +10,6 @@ import pgQueryContext from 'pg-query-context'; import { respondWithGraphQLError } from '../errors/graphql-response'; const log = new Logger('auth'); -const isDev = () => getNodeEnv() === 'development'; /** Default cookie name for session tokens. */ const SESSION_COOKIE_NAME = 'constructive_session'; @@ -127,7 +125,7 @@ export const createAuthenticateMiddleware = ( respondWithGraphQLError( res, errors.INTERNAL_FAILURE({ - details: isDev() ? e.message : 'authentication failed', + details: opts.server?.exposeErrors ? e.message : 'authentication failed', }) ); return; diff --git a/graphql/server/src/middleware/error-handler.ts b/graphql/server/src/middleware/error-handler.ts index 012bc53bac..9de0ad4cda 100644 --- a/graphql/server/src/middleware/error-handler.ts +++ b/graphql/server/src/middleware/error-handler.ts @@ -19,8 +19,8 @@ const wantsJson = (req: Request): boolean => { || Boolean(req.is('json')); }; -const sanitizeMessage = (error: Error): string => { - if (isDevelopment()) return error.message; +const sanitizeMessage = (error: Error, exposeErrors: boolean): string => { + if (exposeErrors) 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'; @@ -40,12 +40,12 @@ const isCsrfError = (err: Error): boolean => { return typeof code === 'string' && code.startsWith('CSRF_'); }; -const categorizeError = (err: Error): ErrorResponse => { +const categorizeError = (err: Error, exposeErrors: boolean): ErrorResponse => { if (isApiError(err)) { return { statusCode: err.statusCode, code: err.code, - message: sanitizeMessage(err), + message: sanitizeMessage(err, exposeErrors), logLevel: err.statusCode >= 500 ? 'error' : 'warn', }; } @@ -54,12 +54,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: sanitizeMessage(err, exposeErrors), 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: sanitizeMessage(err, exposeErrors), logLevel: 'error' }; } - return { statusCode: 500, code: 'INTERNAL_ERROR', message: sanitizeMessage(err), logLevel: 'error' }; + return { statusCode: 500, code: 'INTERNAL_ERROR', message: sanitizeMessage(err, exposeErrors), logLevel: 'error' }; }; const sendResponse = (req: Request, res: Response, { statusCode, code, message }: ErrorResponse): void => { @@ -88,16 +88,21 @@ const logError = (err: Error, req: Request, level: 'warn' | 'error'): void => { } }; -export const errorHandler: ErrorRequestHandler = (err: Error, req: Request, res: Response, _next: NextFunction): void => { - if (res.headersSent) { - log.warn({ event: 'headers_already_sent', requestId: req.requestId, path: req.path, errorMessage: err.message }); - return; - } - - const response = categorizeError(err); - logError(err, req, response.logLevel); - sendResponse(req, res, response); -}; +/** + * Express error handler. Unexpected errors reach the client as a generic + * message unless `exposeErrors` (`server.exposeErrors`) is explicitly set. + */ +export const createErrorHandler = ({ exposeErrors = false }: { exposeErrors?: boolean } = {}): ErrorRequestHandler => + (err: Error, req: Request, res: Response, _next: NextFunction): void => { + if (res.headersSent) { + log.warn({ event: 'headers_already_sent', requestId: req.requestId, path: req.path, errorMessage: err.message }); + return; + } + + const response = categorizeError(err, exposeErrors); + logError(err, req, response.logLevel); + sendResponse(req, res, response); + }; export const notFoundHandler = (req: Request, res: Response, _next: NextFunction): void => { log.warn({ event: 'route_not_found', path: req.path, method: req.method, requestId: req.requestId }); diff --git a/graphql/server/src/middleware/graphile.ts b/graphql/server/src/middleware/graphile.ts index e888d0acff..4c0c129b6d 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'; @@ -28,8 +27,6 @@ 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 // ============================================================================= @@ -82,6 +79,7 @@ const buildPreset = async ( roleName: string, introspectionRole: string | undefined, graphileOptions: ConstructiveOptions['graphile'], + exposeErrors: boolean, databaseSettings?: DatabaseSettings, apiId?: string, compute?: ComputeConfig @@ -131,10 +129,10 @@ const buildPreset = async ( graphiqlPath: '/graphiql', graphiql: true, graphiqlOnGraphQLGET: false, - maskError + maskError: (error) => maskError(error, { exposeErrors }) }, grafast: { - explain: process.env.NODE_ENV === 'development', + explain: exposeErrors, context: (requestContext: Partial) => { // In grafserv/express/v4, the request is available at requestContext.expressv4.req const req = (requestContext as { expressv4?: { req?: Request } })?.expressv4?.req; @@ -281,6 +279,7 @@ const buildPreset = async ( export const graphile = (opts: ConstructiveOptions): RequestHandler => { const observabilityEnabled = isGraphqlObservabilityEnabled(opts.server?.host); + const exposeErrors = opts.server?.exposeErrors ?? false; return async (req: Request, res: Response, next: NextFunction) => { const label = reqLabel(req); @@ -372,6 +371,7 @@ export const graphile = (opts: ConstructiveOptions): RequestHandler => { roleName, opts.api?.introspectionRole, opts.graphile, + exposeErrors, api.databaseSettings, api.apiId, compute @@ -416,7 +416,7 @@ export const graphile = (opts: ConstructiveOptions): RequestHandler => { respondWithGraphQLError( res, errors.INTERNAL_FAILURE({ - details: isDev() ? e?.message ?? String(e) : 'An unexpected error occurred' + details: exposeErrors ? e?.message ?? String(e) : 'An unexpected error occurred' }) ); return; diff --git a/graphql/server/src/middleware/mask-error.ts b/graphql/server/src/middleware/mask-error.ts index 958afd228d..3bb5c677cd 100644 --- a/graphql/server/src/middleware/mask-error.ts +++ b/graphql/server/src/middleware/mask-error.ts @@ -1,7 +1,12 @@ import crypto from 'node:crypto'; -import { classify, type ErrorContext, parse } from '@constructive-io/errors'; -import { getNodeEnv } from '@pgpmjs/env'; +import { + classify, + type ErrorContext, + format, + INSUFFICIENT_PRIVILEGE_SQLSTATE, + parse, +} from '@constructive-io/errors'; import { Logger } from '@pgpmjs/logger'; import { type GraphQLError, type GraphQLFormattedError } from 'graphql'; @@ -65,26 +70,59 @@ const BAD_USER_INPUT = 'BAD_USER_INPUT'; */ export const normalizeError = ( error: GraphQLError, -): { code: string | null; context: ErrorContext; class: 'public' | 'internal' } => { +): { + code: string | null; + context: ErrorContext; + class: 'public' | 'internal'; + sqlState?: string; +} => { 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 }; + return { code: parsed.code, context: parsed.context, class: parsed.class, sqlState: parsed.sqlState }; }; /** - * Production-aware error handling backed by `@constructive-io/errors`. + * The message a public error is surfaced with. A native privilege refusal names + * the table, function, or policy it hit ("permission denied for table x"), so it + * is answered with the registry's message for its code instead. + */ +const publicMessage = ( + error: GraphQLError, + code: string | null, + context: ErrorContext, + sqlState: string | undefined, +): string => + code && sqlState === INSUFFICIENT_PRIVILEGE_SQLSTATE && error.message !== code + ? format(code, context) + : error.message; + +export interface MaskErrorOptions { + /** + * Return internal errors to the client unmasked (enriched with their code) + * instead of behind a reference id. Local debugging only — wired from + * `server.exposeErrors`, never inferred from NODE_ENV, so a deployment that + * forgets to set NODE_ENV still masks. + */ + exposeErrors?: boolean; +} + +/** + * Client-facing 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. + * 2. Surface public (registered/allowlisted) errors as-is; a native privilege + * refusal (SQLSTATE 42501) surfaces as `FORBIDDEN` with the registry message. + * 3. Mask every internal/unknown error behind a reference ID and log the + * original — unless `exposeErrors` was explicitly opted into. */ -export const maskError = (error: GraphQLError): GraphQLError | GraphQLFormattedError => { - const { code, context, class: errorClass } = normalizeError(error); +export const maskError = ( + error: GraphQLError, + { exposeErrors = false }: MaskErrorOptions = {}, +): GraphQLError | GraphQLFormattedError => { + const { code, context, class: errorClass, sqlState } = normalizeError(error); // Lift the structured code onto extensions for every recognized error so // clients always receive a machine-readable code (`extensions` is read-only @@ -108,11 +146,12 @@ export const maskError = (error: GraphQLError): GraphQLError | GraphQLFormattedE } as GraphQLFormattedError; } - if (isPublicCode(effectiveCode) || getNodeEnv() === 'development') { + const surfaced = isPublicCode(effectiveCode); + if (surfaced || exposeErrors) { // Note: grafserv strips originalError and internal extensions before // serializing to the client, so returning the enriched error is safe. return { - message: error.message, + message: surfaced ? publicMessage(error, code, context, sqlState) : error.message, ...(error.locations ? { locations: error.locations } : {}), ...(error.path ? { path: error.path } : {}), extensions, @@ -125,6 +164,8 @@ export const maskError = (error: GraphQLError): GraphQLError | GraphQLFormattedE return { message: `An unexpected error occurred. Reference: ${errorId}`, + ...(error.locations ? { locations: error.locations } : {}), + ...(error.path ? { path: error.path } : {}), extensions: { code: 'INTERNAL_SERVER_ERROR', errorId diff --git a/graphql/server/src/server.ts b/graphql/server/src/server.ts index af82eeb42c..ab668c62cb 100644 --- a/graphql/server/src/server.ts +++ b/graphql/server/src/server.ts @@ -33,7 +33,7 @@ import { createAuthenticateMiddleware } from './middleware/auth'; import { createCaptchaMiddleware } from './middleware/captcha'; import { parseCookieValue, SESSION_COOKIE_NAME } from './middleware/cookie'; import { cors } from './middleware/cors'; -import { errorHandler, notFoundHandler } from './middleware/error-handler'; +import { createErrorHandler, notFoundHandler } from './middleware/error-handler'; import { favicon } from './middleware/favicon'; import { createFlushMiddleware, flushService } from './middleware/flush'; import { createFnRouter } from './middleware/fn'; @@ -230,7 +230,7 @@ class Server { // Error handling - MUST be LAST app.use(notFoundHandler); // Catches unmatched routes (404) - app.use(errorHandler); // Catches all thrown errors + app.use(createErrorHandler({ exposeErrors: effectiveOpts.server?.exposeErrors })); // Catches all thrown errors this.app = app; this.debugSampler = observabilityEnabled ? startDebugSampler(effectiveOpts) : null; diff --git a/packages/errors/README.md b/packages/errors/README.md index 4668324e57..0e513384b2 100644 --- a/packages/errors/README.md +++ b/packages/errors/README.md @@ -39,7 +39,8 @@ throw errors.ACCOUNT_EXISTS(); - `parse()` recovers structure from the code's precedence: structured `DETAIL` JSON → GraphQL `extensions.code` → a leading ALL_CAPS token in the message (legacy DB `RAISE`, incl. `CODE (arg, arg)` positional args) → native - SQLSTATE constraint mapping. + SQLSTATE mapping (class-23 constraint violations, and `42501` + insufficient privilege — a missing grant or RLS refusal — as `FORBIDDEN`). - The registry has two layers, merged at lookup time (curated wins): - **Generated** (`src/generated/registry.generated.ts`) — every code raised via `EXCEPTION`/`THROW` across constructive-db (deploy sources + generated output), diff --git a/packages/errors/__tests__/parse.test.ts b/packages/errors/__tests__/parse.test.ts index 7af662c9dc..a786d6ad60 100644 --- a/packages/errors/__tests__/parse.test.ts +++ b/packages/errors/__tests__/parse.test.ts @@ -46,6 +46,18 @@ describe('parse', () => { expect(result.context.constraint).toBe('users_email_key'); }); + it('maps a native privilege refusal (42501) to FORBIDDEN', () => { + const result = parse({ message: 'permission denied for table agent_thread', code: '42501' }); + expect(result.code).toBe('FORBIDDEN'); + expect(result.class).toBe('public'); + expect(result.context).toEqual({}); + }); + + it('keeps a registered code raised with SQLSTATE 42501', () => { + const result = parse({ message: 'STEP_UP_REQUIRED', code: '42501' }); + expect(result.code).toBe('STEP_UP_REQUIRED'); + }); + it('classifies unknown codes as internal (masked)', () => { const result = parse({ message: 'DATA_INVARIANT_BROKEN', code: 'P0001' }); expect(result.code).toBe('DATA_INVARIANT_BROKEN'); diff --git a/packages/errors/src/pg.ts b/packages/errors/src/pg.ts index 59fa173686..856f0a78dd 100644 --- a/packages/errors/src/pg.ts +++ b/packages/errors/src/pg.ts @@ -1,10 +1,15 @@ import type { PgErrorFields } from './types'; +/** SQLSTATE `insufficient_privilege`: a missing grant or a row-level security refusal. */ +export const INSUFFICIENT_PRIVILEGE_SQLSTATE = '42501'; + /** - * 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 privilege refusals (a missing + * grant or an RLS policy) as `FORBIDDEN`. */ export const SQLSTATE_TO_CODE: Record = { + [INSUFFICIENT_PRIVILEGE_SQLSTATE]: 'FORBIDDEN', 23505: 'UNIQUE_VIOLATION', 23503: 'FOREIGN_KEY_VIOLATION', 23502: 'NOT_NULL_VIOLATION', diff --git a/pgpm/env/__tests__/__snapshots__/merge.test.ts.snap b/pgpm/env/__tests__/__snapshots__/merge.test.ts.snap index 3d7f12db7e..4e59ca9b83 100644 --- a/pgpm/env/__tests__/__snapshots__/merge.test.ts.snap +++ b/pgpm/env/__tests__/__snapshots__/merge.test.ts.snap @@ -62,6 +62,7 @@ exports[`getEnvOptions merges defaults, config, env, and overrides 1`] = ` "user": "env-user", }, "server": { + "exposeErrors": false, "host": "localhost", "port": 9999, "strictAuth": false, diff --git a/pgpm/env/src/env.ts b/pgpm/env/src/env.ts index 5ec9b056a9..265de03acd 100644 --- a/pgpm/env/src/env.ts +++ b/pgpm/env/src/env.ts @@ -45,6 +45,7 @@ export const getEnvVars = (env: NodeJS.ProcessEnv = process.env): PgpmOptions => SERVER_TRUST_PROXY, SERVER_ORIGIN, SERVER_STRICT_AUTH, + SERVER_EXPOSE_ERRORS, PGHOST, PGPORT, @@ -134,6 +135,7 @@ export const getEnvVars = (env: NodeJS.ProcessEnv = process.env): PgpmOptions => ...(SERVER_TRUST_PROXY && { trustProxy: parseEnvBoolean(SERVER_TRUST_PROXY) }), ...(SERVER_ORIGIN && { origin: SERVER_ORIGIN }), ...(SERVER_STRICT_AUTH && { strictAuth: parseEnvBoolean(SERVER_STRICT_AUTH) }), + ...(SERVER_EXPOSE_ERRORS && { exposeErrors: parseEnvBoolean(SERVER_EXPOSE_ERRORS) }), }, pg: { ...(PGHOST && { host: PGHOST }), diff --git a/pgpm/types/src/pgpm.ts b/pgpm/types/src/pgpm.ts index 194c9e6e09..07e1184fdc 100644 --- a/pgpm/types/src/pgpm.ts +++ b/pgpm/types/src/pgpm.ts @@ -120,6 +120,12 @@ export interface ServerOptions { origin?: string; /** Whether to enforce strict authentication */ strictAuth?: boolean; + /** + * Return internal error details (raw database messages) to clients instead + * of a masked reference id. Local debugging only; never set on a deployed + * server. Independent of NODE_ENV. + */ + exposeErrors?: boolean; } /** @@ -383,7 +389,8 @@ export const pgpmDefaults: PgpmOptions = { host: 'localhost', port: 3000, trustProxy: false, - strictAuth: false + strictAuth: false, + exposeErrors: false }, cdn: { provider: 'minio',