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/agentic/agentic-server/src/server.ts b/agentic/agentic-server/src/server.ts index df3ca28797..b4a7ab9ec2 100644 --- a/agentic/agentic-server/src/server.ts +++ b/agentic/agentic-server/src/server.ts @@ -27,6 +27,7 @@ const IDENTITY_HEADERS = [ */ export const createAgenticServer = (options: AgenticServerStartOptions): express.Express => { const app = express(); + app.disable('x-powered-by'); app.use(express.json()); // When isPublic === true, strip identity headers from all incoming requests. diff --git a/graphile/graphile-cache/src/create-instance.ts b/graphile/graphile-cache/src/create-instance.ts index a522701720..e608b9a18f 100644 --- a/graphile/graphile-cache/src/create-instance.ts +++ b/graphile/graphile-cache/src/create-instance.ts @@ -46,6 +46,7 @@ export const createGraphileInstance = async ( const serv = pgl.createServ(grafserv); const handler = express(); + handler.disable('x-powered-by'); const httpServer = createServer(handler); await serv.addTo(handler, httpServer); await serv.ready(); diff --git a/graphile/graphile-presigned-url-plugin/README.md b/graphile/graphile-presigned-url-plugin/README.md index 0509fdfbbc..39ee2f2f03 100644 --- a/graphile/graphile-presigned-url-plugin/README.md +++ b/graphile/graphile-presigned-url-plugin/README.md @@ -43,3 +43,22 @@ const preset = { ], }; ``` + +### Internal vs public storage endpoint + +`client` is what the server uses to talk to storage. Presigned URLs are handed to +clients, and SigV4 signs the `Host` header, so they must be signed for the host +the client will call. When storage is reached over an internal address (e.g. an +in-cluster Service), pass a second client configured with the public endpoint: + +```typescript +s3: { + client: internalClient, // endpoint: http://minio.storage.svc.cluster.local:9000 + presignClient: publicClient, // endpoint: https://storage.example.com + publicEndpoint: 'https://storage.example.com', + bucket: 'my-uploads', +} +``` + +Without `presignClient`, URLs are signed with `client`. In the Constructive server +this is `CDN_ENDPOINT` (internal) and `CDN_PUBLIC_ENDPOINT` (public). diff --git a/graphile/graphile-presigned-url-plugin/__tests__/s3-signer.test.ts b/graphile/graphile-presigned-url-plugin/__tests__/s3-signer.test.ts new file mode 100644 index 0000000000..c20c920fb3 --- /dev/null +++ b/graphile/graphile-presigned-url-plugin/__tests__/s3-signer.test.ts @@ -0,0 +1,142 @@ +/** + * Presigned URLs are handed to clients, so they must be signed for the host the + * client calls. SigV4 signs the Host header: a URL minted for an internal + * storage host (an in-cluster Service) cannot be repointed at the public host + * afterwards — the signature only validates for the host it was signed with. + */ + +import { S3Client } from '@aws-sdk/client-s3'; +import { createHash, createHmac } from 'crypto'; + +import { generatePresignedGetUrl, generatePresignedPutUrl } from '../src/s3-signer'; +import type { S3Config } from '../src/types'; + +const INTERNAL = 'http://rustfs.constructive-infra.svc.cluster.local:9000'; +const PUBLIC = 'https://storage.example.com'; +const REGION = 'us-east-1'; +const ACCESS_KEY = 'AKIDEXAMPLE'; +const SECRET_KEY = 'wJalrXUtnFEMI/K7MDENG+bPxRfiCYEXAMPLEKEY'; +const NOW = new Date('2026-01-01T00:00:00.000Z'); + +function client(endpoint: string): S3Client { + return new S3Client({ + region: REGION, + endpoint, + forcePathStyle: true, + credentials: { accessKeyId: ACCESS_KEY, secretAccessKey: SECRET_KEY }, + }); +} + +const internalOnly: S3Config = { + client: client(INTERNAL), + bucket: 'tenant-bucket', + endpoint: INTERNAL, + region: REGION, + forcePathStyle: true, +}; + +const withPublicEndpoint: S3Config = { + ...internalOnly, + presignClient: client(PUBLIC), + publicEndpoint: PUBLIC, +}; + +const encode = (value: string) => + encodeURIComponent(value).replace(/[!'()*]/g, (c) => `%${c.charCodeAt(0).toString(16).toUpperCase()}`); +const sha256 = (value: string) => createHash('sha256').update(value).digest('hex'); +const hmac = (key: Buffer | string, value: string) => createHmac('sha256', key).update(value).digest(); + +/** + * Independent SigV4 query-signature check, as the storage server performs it: + * recompute the signature for the request a client sends to `url` (whose Host + * is `url.host`) and compare it with the one in the URL. + */ +function signatureValidates(method: string, url: string, headers: Record = {}): boolean { + const parsed = new URL(url); + const params = [...parsed.searchParams.entries()]; + const signature = parsed.searchParams.get('X-Amz-Signature'); + const amzDate = parsed.searchParams.get('X-Amz-Date')!; + const [, date, region, service] = parsed.searchParams.get('X-Amz-Credential')!.split('/'); + const signedHeaders = parsed.searchParams.get('X-Amz-SignedHeaders')!; + + const requestHeaders: Record = { host: parsed.host, ...headers }; + const canonicalQuery = params + .filter(([name]) => name !== 'X-Amz-Signature') + .map(([name, value]) => `${encode(name)}=${encode(value)}`) + .sort() + .join('&'); + const canonicalHeaders = signedHeaders + .split(';') + .map((name) => `${name}:${requestHeaders[name]}\n`) + .join(''); + const canonicalRequest = [ + method, parsed.pathname, canonicalQuery, canonicalHeaders, signedHeaders, 'UNSIGNED-PAYLOAD', + ].join('\n'); + + const scope = `${date}/${region}/${service}/aws4_request`; + const stringToSign = ['AWS4-HMAC-SHA256', amzDate, scope, sha256(canonicalRequest)].join('\n'); + const signingKey = hmac(hmac(hmac(hmac(`AWS4${SECRET_KEY}`, date), region), service), 'aws4_request'); + return hmac(signingKey, stringToSign).toString('hex') === signature; +} + +/** The same URL pointed at another host — what a post-signing host rewrite produces. */ +const rehost = (url: string, endpoint: string) => { + const parsed = new URL(url); + const target = new URL(endpoint); + parsed.protocol = target.protocol; + parsed.host = target.host; + return parsed.toString(); +}; + +beforeAll(() => { + jest.useFakeTimers({ now: NOW, advanceTimers: true }); +}); + +afterAll(() => { + jest.useRealTimers(); +}); + +describe('generatePresignedPutUrl', () => { + const putHeaders = { 'content-length': '11', 'content-type': 'text/plain' }; + + it('signs for the internal endpoint when no public endpoint is configured', async () => { + const url = await generatePresignedPutUrl(internalOnly, 'abc', 'text/plain', 11, 900); + + expect(new URL(url).origin).toBe(INTERNAL); + expect(new URL(url).pathname).toBe('/tenant-bucket/abc'); + expect(new URL(url).searchParams.get('X-Amz-Date')).toBe('20260101T000000Z'); + expect(signatureValidates('PUT', url, putHeaders)).toBe(true); + }); + + it('signs for the public endpoint when one is configured', async () => { + const url = await generatePresignedPutUrl(withPublicEndpoint, 'abc', 'text/plain', 11, 900); + + expect(new URL(url).origin).toBe(PUBLIC); + expect(new URL(url).pathname).toBe('/tenant-bucket/abc'); + expect(signatureValidates('PUT', url, putHeaders)).toBe(true); + }); + + it('a URL signed for the internal host does not validate at the public host', async () => { + const url = await generatePresignedPutUrl(internalOnly, 'abc', 'text/plain', 11, 900); + + expect(signatureValidates('PUT', rehost(url, PUBLIC), putHeaders)).toBe(false); + }); +}); + +describe('generatePresignedGetUrl', () => { + it('signs for the internal endpoint when no public endpoint is configured', async () => { + const url = await generatePresignedGetUrl(internalOnly, 'abc', 3600, 'report.pdf'); + + expect(new URL(url).origin).toBe(INTERNAL); + expect(signatureValidates('GET', url)).toBe(true); + }); + + it('signs for the public endpoint when one is configured', async () => { + const url = await generatePresignedGetUrl(withPublicEndpoint, 'abc', 3600, 'report.pdf'); + + expect(new URL(url).origin).toBe(PUBLIC); + expect(new URL(url).searchParams.get('response-content-disposition')).toBe('attachment; filename="report.pdf"'); + expect(signatureValidates('GET', url)).toBe(true); + expect(signatureValidates('GET', rehost(url, INTERNAL))).toBe(false); + }); +}); diff --git a/graphile/graphile-presigned-url-plugin/src/s3-signer.ts b/graphile/graphile-presigned-url-plugin/src/s3-signer.ts index a5e1c9c305..689bbe06a7 100644 --- a/graphile/graphile-presigned-url-plugin/src/s3-signer.ts +++ b/graphile/graphile-presigned-url-plugin/src/s3-signer.ts @@ -13,6 +13,14 @@ import type { S3Config } from './types'; const log = new Logger('graphile-presigned-url:s3'); +/** Presigned URLs go to clients, so they are signed for the client-reachable endpoint. */ +function presignTarget(s3Config: S3Config) { + return { + client: s3Config.presignClient ?? s3Config.client, + endpoint: s3Config.presignClient ? s3Config.publicEndpoint : s3Config.endpoint, + }; +} + /** * Generate a presigned PUT URL for uploading a file to S3. * @@ -41,13 +49,14 @@ export async function generatePresignedPutUrl( ContentLength: contentLength, }); + const { client, endpoint } = presignTarget(s3Config); let url: string; try { - url = await getSignedUrl(s3Config.client as any, command, { expiresIn }); + url = await getSignedUrl(client as any, command, { expiresIn }); } catch (err) { throw s3FailureError( 'PRESIGN_PUT_FAILED', - { endpoint: s3Config.endpoint, bucket: s3Config.bucket, key, contentType }, + { endpoint, bucket: s3Config.bucket, key, contentType }, err, ); } @@ -84,11 +93,12 @@ export async function generatePresignedGetUrl( } const command = new GetObjectCommand(params as any); + const { client, endpoint } = presignTarget(s3Config); let url: string; try { - url = await getSignedUrl(s3Config.client as any, command, { expiresIn }); + url = await getSignedUrl(client as any, command, { expiresIn }); } catch (err) { - throw s3FailureError('PRESIGN_GET_FAILED', { endpoint: s3Config.endpoint, bucket: s3Config.bucket, key }, err); + throw s3FailureError('PRESIGN_GET_FAILED', { endpoint, bucket: s3Config.bucket, key }, err); } log.debug(`Generated presigned GET URL for key=${key}, expires=${expiresIn}s`); return url; diff --git a/graphile/graphile-presigned-url-plugin/src/types.ts b/graphile/graphile-presigned-url-plugin/src/types.ts index 65b477ead2..6d407d0d15 100644 --- a/graphile/graphile-presigned-url-plugin/src/types.ts +++ b/graphile/graphile-presigned-url-plugin/src/types.ts @@ -186,6 +186,14 @@ export interface S3Config { bucket: string; /** S3 endpoint URL (for RustFS, MinIO, or custom S3) */ endpoint?: string; + /** + * Client used only to sign presigned URLs handed to clients, configured with + * the client-reachable `publicEndpoint`. SigV4 signs the Host header, so a URL + * must be signed for the host the client will call. Defaults to `client`. + */ + presignClient?: S3Client; + /** Endpoint `presignClient` signs for */ + publicEndpoint?: string; /** S3 region */ region?: string; /** Whether to use path-style URLs (required for path-style S3-compatible storage) */ diff --git a/graphile/graphile-settings/__tests__/presigned-url-resolver.test.ts b/graphile/graphile-settings/__tests__/presigned-url-resolver.test.ts index 49ecd6b151..b69aaff801 100644 --- a/graphile/graphile-settings/__tests__/presigned-url-resolver.test.ts +++ b/graphile/graphile-settings/__tests__/presigned-url-resolver.test.ts @@ -9,6 +9,7 @@ interface CdnOptions { awsAccessKey?: string; awsSecretKey?: string; endpoint?: string; + publicEndpoint?: string; publicUrlPrefix?: string; } @@ -18,14 +19,13 @@ async function loadResolverModule(cdn: CdnOptions | undefined) { jest.doMock('@constructive-io/graphql-env', () => ({ getEnvOptions: jest.fn(() => ({ cdn })), })); - jest.doMock('@constructive-io/s3-utils', () => ({ - createS3Client: jest.fn(() => ({ send: jest.fn() })), - })); + const createS3Client = jest.fn((config: { endpoint?: string }) => ({ endpoint: config.endpoint })); + jest.doMock('@constructive-io/s3-utils', () => ({ createS3Client })); jest.doMock('@pgpmjs/logger', () => ({ Logger: jest.fn().mockImplementation(() => ({ info: jest.fn() })), })); - return import('../src/presigned-url-resolver'); + return { ...(await import('../src/presigned-url-resolver')), createS3Client }; } const BASE_CDN: CdnOptions = { @@ -50,6 +50,30 @@ describe('getPresignedUrlS3Config', () => { })); }); + it('signs presigned URLs with the connection client when no public endpoint is set', async () => { + const { getPresignedUrlS3Config, createS3Client } = await loadResolverModule(BASE_CDN); + const config = getPresignedUrlS3Config(); + + expect(config.client).toEqual({ endpoint: 'http://localhost:9000' }); + expect(config.presignClient).toBeUndefined(); + expect(config.publicEndpoint).toBeUndefined(); + expect(createS3Client).toHaveBeenCalledTimes(1); + }); + + it('talks to storage over the endpoint and signs presigned URLs for the public endpoint', async () => { + const { getPresignedUrlS3Config } = await loadResolverModule({ + ...BASE_CDN, + endpoint: 'http://rustfs.constructive-infra.svc.cluster.local:9000', + publicEndpoint: 'https://storage.example.com', + }); + const config = getPresignedUrlS3Config(); + + expect(config.client).toEqual({ endpoint: 'http://rustfs.constructive-infra.svc.cluster.local:9000' }); + expect(config.endpoint).toBe('http://rustfs.constructive-infra.svc.cluster.local:9000'); + expect(config.presignClient).toEqual({ endpoint: 'https://storage.example.com' }); + expect(config.publicEndpoint).toBe('https://storage.example.com'); + }); + it('caches the initialized S3 configuration', async () => { const { getPresignedUrlS3Config } = await loadResolverModule(BASE_CDN); diff --git a/graphile/graphile-settings/src/presigned-url-resolver.ts b/graphile/graphile-settings/src/presigned-url-resolver.ts index 5e12f141e6..b83e2fbf6d 100644 --- a/graphile/graphile-settings/src/presigned-url-resolver.ts +++ b/graphile/graphile-settings/src/presigned-url-resolver.ts @@ -6,6 +6,10 @@ * initializes an S3Client on first use. * * Follows the same lazy-init pattern as upload-resolver.ts. + * + * `cdn.endpoint` (CDN_ENDPOINT) is the host the server talks to; presigned URLs + * are signed for `cdn.publicEndpoint` (CDN_PUBLIC_ENDPOINT) when it is set, so a + * cluster-internal storage host never reaches a client. */ import { getEnvOptions } from '@constructive-io/graphql-env'; @@ -42,7 +46,7 @@ export function getPresignedUrlS3Config(): S3Config { ); } - const { bucketName, awsRegion, awsAccessKey, awsSecretKey, endpoint, publicUrlPrefix } = cdn; + const { bucketName, awsRegion, awsAccessKey, awsSecretKey, endpoint, publicEndpoint, publicUrlPrefix } = cdn; if (!awsAccessKey || !awsSecretKey) { throw new Error( @@ -59,23 +63,25 @@ export function getPresignedUrlS3Config(): S3Config { } log.info( - `[presigned-url-resolver] Initializing: bucket=${bucketName} endpoint=${endpoint}`, + `[presigned-url-resolver] Initializing: bucket=${bucketName} endpoint=${endpoint} ` + + `publicEndpoint=${publicEndpoint ?? endpoint}`, ); - const client = createS3Client({ + const connect = (url: string | undefined) => createS3Client({ provider: (cdn.provider || 'minio') as any, region: awsRegion, accessKeyId: awsAccessKey, secretAccessKey: awsSecretKey, - ...(endpoint ? { endpoint } : {}), + ...(url ? { endpoint: url } : {}), }); s3Config = { - client, + client: connect(endpoint), bucket: bucketName, region: awsRegion, publicUrlPrefix, ...(endpoint ? { endpoint, forcePathStyle: true } : {}), + ...(publicEndpoint ? { presignClient: connect(publicEndpoint), publicEndpoint } : {}), }; return s3Config; diff --git a/graphql/dev-server/src/server.ts b/graphql/dev-server/src/server.ts index b4e2d5f181..31a203fd23 100644 --- a/graphql/dev-server/src/server.ts +++ b/graphql/dev-server/src/server.ts @@ -1,7 +1,7 @@ import { getEnvOptions } from '@constructive-io/graphql-env'; import type { ConstructiveOptions } from '@constructive-io/graphql-types'; import { Logger } from '@pgpmjs/logger'; -import { cors, healthz, poweredBy } from '@pgpmjs/server-utils'; +import { cors, healthz } from '@pgpmjs/server-utils'; import express from 'express'; import { createGraphileInstance, type GraphileCacheEntry } from 'graphile-cache'; import { getPgPool } from 'pg-cache'; @@ -47,9 +47,9 @@ export const createDevServer = async ( }); const app = express(); + app.disable('x-powered-by'); healthz(app); cors(app, serverOpts.origin ?? opts.server?.origin); - app.use(poweredBy('constructive')); app.use((req, res, next) => instance.handler(req, res, next)); const httpServer = await new Promise((resolve, reject) => { 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/explorer/src/server.ts b/graphql/explorer/src/server.ts index 26035d5e12..ae906bcd53 100644 --- a/graphql/explorer/src/server.ts +++ b/graphql/explorer/src/server.ts @@ -1,7 +1,7 @@ import { getEnvOptions } from '@constructive-io/graphql-env'; import type { ConstructiveOptions } from '@constructive-io/graphql-types'; import { middleware as parseDomains } from '@constructive-io/url-domains'; -import { cors, healthz, poweredBy } from '@pgpmjs/server-utils'; +import { cors, healthz } from '@pgpmjs/server-utils'; import express, { Express, NextFunction, Request, Response } from 'express'; import { createGraphileInstance, graphileCache, GraphileCacheEntry } from 'graphile-cache'; import type { GraphileConfig } from 'graphile-config'; @@ -56,11 +56,11 @@ export const GraphQLExplorer = (rawOpts: ConstructiveOptions = {}): Express => { }; const app = express(); + app.disable('x-powered-by'); healthz(app); cors(app, server.origin); app.use(parseDomains()); - app.use(poweredBy('constructive')); app.use(async (req: Request, res: Response, next: NextFunction) => { if (req.urlDomains?.subdomains.length === 1) { 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__/server.integration.test.ts b/graphql/server-test/__tests__/server.integration.test.ts index bd26d87da5..44aaedc584 100644 --- a/graphql/server-test/__tests__/server.integration.test.ts +++ b/graphql/server-test/__tests__/server.integration.test.ts @@ -575,6 +575,31 @@ describe('Cookie-authenticated GraphQL CSRF flow', () => { }); }); + it('sets the csrf_token cookie with Secure and SameSite on HTTPS requests', async () => { + const res = await request + .get('/graphql') + .set('Host', host) + .set('X-Forwarded-Proto', 'https'); + + const setCookie = res.headers['set-cookie']; + const cookies = (Array.isArray(setCookie) ? setCookie : [setCookie]) + .filter((cookie): cookie is string => typeof cookie === 'string'); + const csrfCookie = cookies.find((cookie) => cookie.startsWith('csrf_token=')); + + expect(csrfCookie).toBeDefined(); + expect(csrfCookie).toMatch(/;\s*Secure(;|$)/); + expect(csrfCookie).toContain('SameSite=Lax'); + // Double-submit design: the SPA reads the token via document.cookie, so + // this cookie must remain readable (not HttpOnly). + expect(csrfCookie).not.toMatch(/;\s*HttpOnly(;|$)/); + }); + + it('does not send an x-powered-by header', async () => { + const res = await request.get('/graphql').set('Host', host); + + expect(res.headers['x-powered-by']).toBeUndefined(); + }); + it('keeps genuinely unknown routes on the HTML 404 path', async () => { const res = await request .get('/this-route-does-not-exist') 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__/cookie.test.ts b/graphql/server/src/middleware/__tests__/cookie.test.ts index fdecda2337..8102331936 100644 --- a/graphql/server/src/middleware/__tests__/cookie.test.ts +++ b/graphql/server/src/middleware/__tests__/cookie.test.ts @@ -20,7 +20,7 @@ describe('cookie utilities', () => { it('returns default config when no authSettings provided', () => { const config = getSessionCookieConfig(); expect(config).toEqual({ - secure: false, // NODE_ENV is 'test' + secure: true, // Secure by default; cookieSecure: false is the explicit opt-out sameSite: 'lax', domain: undefined, httpOnly: true, @@ -29,6 +29,11 @@ describe('cookie utilities', () => { }); }); + it('honors the cookieSecure: false opt-out for plain-HTTP deployments', () => { + const config = getSessionCookieConfig({ cookieSecure: false }); + expect(config.secure).toBe(false); + }); + it('uses authSettings values when provided', () => { const authSettings: AuthSettings = { cookieSecure: true, diff --git a/graphql/server/src/middleware/__tests__/csrf-integration.test.ts b/graphql/server/src/middleware/__tests__/csrf-integration.test.ts index 7f3df0eaf5..f87801a90e 100644 --- a/graphql/server/src/middleware/__tests__/csrf-integration.test.ts +++ b/graphql/server/src/middleware/__tests__/csrf-integration.test.ts @@ -7,7 +7,7 @@ describe('CSRF middleware integration', () => { const csrf = createCsrfMiddleware({ cookieOptions: { httpOnly: false, - secure: false, + secure: true, sameSite: 'lax', }, }); @@ -43,6 +43,8 @@ describe('CSRF middleware integration', () => { expect.any(String), expect.objectContaining({ httpOnly: false, + secure: true, + sameSite: 'lax', }) ); done(); 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/cookie.ts b/graphql/server/src/middleware/cookie.ts index bba9c1e37c..18ce75a200 100644 --- a/graphql/server/src/middleware/cookie.ts +++ b/graphql/server/src/middleware/cookie.ts @@ -34,7 +34,7 @@ export const getSessionCookieConfig = ( } return { - secure: authSettings?.cookieSecure ?? process.env.NODE_ENV === 'production', + secure: authSettings?.cookieSecure ?? true, sameSite: (authSettings?.cookieSamesite as 'strict' | 'lax' | 'none') ?? 'lax', domain: authSettings?.cookieDomain ?? undefined, httpOnly: authSettings?.cookieHttponly ?? true, @@ -48,7 +48,7 @@ export const getSessionCookieConfig = ( */ export const getDeviceTokenCookieConfig = (authSettings?: AuthSettings): CookieConfig => { return { - secure: authSettings?.cookieSecure ?? process.env.NODE_ENV === 'production', + secure: authSettings?.cookieSecure ?? true, sameSite: (authSettings?.cookieSamesite as 'strict' | 'lax' | 'none') ?? 'lax', domain: authSettings?.cookieDomain ?? undefined, httpOnly: true, 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..176f74bbb2 100644 --- a/graphql/server/src/server.ts +++ b/graphql/server/src/server.ts @@ -5,7 +5,7 @@ import { getEnvOptions } from '@constructive-io/graphql-env'; import type { ConstructiveOptions } from '@constructive-io/graphql-types'; import { middleware as parseDomains } from '@constructive-io/url-domains'; import { Logger } from '@pgpmjs/logger'; -import { healthz, poweredBy, svcCache, trustProxy } from '@pgpmjs/server-utils'; +import { healthz, svcCache, trustProxy } from '@pgpmjs/server-utils'; import { PgpmOptions } from '@pgpmjs/types'; import cookieParser from 'cookie-parser'; import express, { Express, NextFunction, Request, RequestHandler, Response } from 'express'; @@ -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'; @@ -96,6 +96,7 @@ class Server { const observabilityEnabled = isGraphqlObservabilityEnabled(effectiveOpts.server?.host); const app = express(); + app.disable('x-powered-by'); const api = createApiMiddleware(effectiveOpts); const authenticate = createAuthenticateMiddleware(effectiveOpts); const requestLogger = createRequestLogger({ observabilityEnabled }); @@ -153,7 +154,6 @@ class Server { } } - app.use(poweredBy('constructive')); app.use(cookieParser()); app.use(cors(fallbackOrigin)); app.use('/graphql', graphqlUpload.graphqlUploadExpress({ @@ -194,7 +194,7 @@ class Server { const csrf = createCsrfMiddleware({ cookieOptions: { httpOnly: false, // SPA clients need to read this via document.cookie - secure: process.env.NODE_ENV === 'production', + secure: true, // browsers accept Secure cookies on http://localhost; cookieSecure: false opts out for plain-HTTP deployments sameSite: 'lax' } }); @@ -213,7 +213,9 @@ class Server { csrf.protect(req as any, res as any, next); }; const csrfSetToken: RequestHandler = (req: Request, res: Response, next: NextFunction) => { - csrf.setToken(req as any, res as any, next); + csrf.setToken(req as any, res as any, next, { + secure: req.api?.authSettings?.cookieSecure ?? true + }); }; app.use(csrfSetToken); // Set CSRF token cookie on all requests app.use('/graphql', csrfProtect); // Enforce CSRF on GraphQL mutations @@ -230,7 +232,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/csrf/__tests__/csrf.test.ts b/packages/csrf/__tests__/csrf.test.ts index 71befb43fd..c52b5559f3 100644 --- a/packages/csrf/__tests__/csrf.test.ts +++ b/packages/csrf/__tests__/csrf.test.ts @@ -99,6 +99,21 @@ describe('createCsrfMiddleware', () => { expect(next).toHaveBeenCalled(); }); + it('should apply per-request cookie option overrides', () => { + const csrf = createCsrfMiddleware(); + const req = createMockReq(); + const res = createMockRes(); + const next = jest.fn(); + + csrf.setToken(req, res, next, { secure: false }); + + expect(res.cookie).toHaveBeenCalledWith( + 'csrf_token', + expect.any(String), + expect.objectContaining({ secure: false }) + ); + }); + it('should use custom cookie name', () => { const csrf = createCsrfMiddleware({ cookieName: 'my_csrf' }); const req = createMockReq(); diff --git a/packages/csrf/src/middleware.ts b/packages/csrf/src/middleware.ts index c8dbe74fca..27a102392c 100644 --- a/packages/csrf/src/middleware.ts +++ b/packages/csrf/src/middleware.ts @@ -7,7 +7,7 @@ const DEFAULT_CONFIG: Required = { fieldName: '_csrf', cookieOptions: { httpOnly: true, - secure: process.env.NODE_ENV === 'production', + secure: true, sameSite: 'lax', maxAge: 86400, path: '/', @@ -37,7 +37,8 @@ export interface CsrfMiddlewareResult { setToken: ( req: CsrfRequest, res: CsrfResponse, - next: (err?: Error) => void + next: (err?: Error) => void, + cookieOptions?: CookieOptions ) => void; getToken: (req: CsrfRequest) => string | undefined; generateToken: () => string; @@ -57,12 +58,13 @@ export function createCsrfMiddleware(config: CsrfConfig = {}): CsrfMiddlewareRes const setToken = ( req: CsrfRequest, res: CsrfResponse, - next: (err?: Error) => void + next: (err?: Error) => void, + cookieOptions?: CookieOptions ): void => { const existingToken = req.cookies[cfg.cookieName]; if (!existingToken) { const token = generateToken(cfg.tokenLength); - res.cookie(cfg.cookieName, token, cfg.cookieOptions); + res.cookie(cfg.cookieName, token, { ...cfg.cookieOptions, ...cookieOptions }); } next(); }; 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/packages/oauth/src/middleware/express.ts b/packages/oauth/src/middleware/express.ts index 607d2d76a4..28230504c1 100644 --- a/packages/oauth/src/middleware/express.ts +++ b/packages/oauth/src/middleware/express.ts @@ -64,7 +64,7 @@ export function createOAuthMiddleware(config: OAuthMiddlewareConfig): OAuthRoute res.cookie(clientConfig.stateCookieName!, state, { httpOnly: true, - secure: process.env.NODE_ENV === 'production', + secure: clientConfig.stateCookieSecure ?? true, maxAge: (clientConfig.stateCookieMaxAge || 600) * 1000, sameSite: 'lax', }); diff --git a/packages/oauth/src/oauth-client.ts b/packages/oauth/src/oauth-client.ts index cbdd348e50..4e01e7caa3 100644 --- a/packages/oauth/src/oauth-client.ts +++ b/packages/oauth/src/oauth-client.ts @@ -17,6 +17,7 @@ export class OAuthClient { callbackPath: '/auth/{provider}/callback', stateCookieName: 'oauth_state', stateCookieMaxAge: 600, + stateCookieSecure: true, ...config, }; } diff --git a/packages/oauth/src/types.ts b/packages/oauth/src/types.ts index db4ef07437..7c1af0f4ce 100644 --- a/packages/oauth/src/types.ts +++ b/packages/oauth/src/types.ts @@ -31,6 +31,7 @@ export interface OAuthClientConfig { callbackPath?: string; stateCookieName?: string; stateCookieMaxAge?: number; + stateCookieSecure?: boolean; } export interface TokenResponse { diff --git a/packages/server-utils/src/utils.ts b/packages/server-utils/src/utils.ts index d099ae085a..31d8453b24 100644 --- a/packages/server-utils/src/utils.ts +++ b/packages/server-utils/src/utils.ts @@ -1,4 +1,4 @@ -import { Express, NextFunction,Request, Response } from 'express'; +import { Express,Request, Response } from 'express'; const UUID_RE = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i; @@ -13,15 +13,6 @@ export const healthz = (app: Express): void => { }); }; -export const poweredBy = (name: string) => { - return async (req: Request, res: Response, next: NextFunction): Promise => { - res.set({ - 'X-Powered-By': name, - }); - return next(); - }; -}; - export const trustProxy = (app: Express, trustProxy?: boolean): void => { if (trustProxy) { app.set('trust proxy', (ip: string) => { 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/__tests__/merge.test.ts b/pgpm/env/__tests__/merge.test.ts index 8924d0b4ec..a1cf43c98f 100644 --- a/pgpm/env/__tests__/merge.test.ts +++ b/pgpm/env/__tests__/merge.test.ts @@ -179,6 +179,26 @@ describe('getEnvOptions', () => { expect(result).toMatchSnapshot(); }); + it('maps CDN_ENDPOINT and CDN_PUBLIC_ENDPOINT to separate settings', () => { + tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'pgpm-env-')); + writeConfig(tempDir, {}); + + const { cdn } = getEnvOptions({}, tempDir, { + CDN_ENDPOINT: 'http://rustfs.constructive-infra.svc.cluster.local:9000', + CDN_PUBLIC_ENDPOINT: 'https://storage.example.com' + }); + + expect(cdn.endpoint).toBe('http://rustfs.constructive-infra.svc.cluster.local:9000'); + expect(cdn.publicEndpoint).toBe('https://storage.example.com'); + }); + + it('leaves cdn.publicEndpoint unset by default', () => { + tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'pgpm-env-')); + writeConfig(tempDir, {}); + + expect(getEnvOptions({}, tempDir, {}).cdn.publicEndpoint).toBeUndefined(); + }); + it('replaces array fields with later values (overrides win)', () => { tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'pgpm-env-replace-')); writeConfig(tempDir, { diff --git a/pgpm/env/src/env.ts b/pgpm/env/src/env.ts index 5ec9b056a9..babedeb040 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, @@ -60,6 +61,7 @@ export const getEnvVars = (env: NodeJS.ProcessEnv = process.env): PgpmOptions => AWS_SECRET_KEY, AWS_SECRET_ACCESS_KEY, CDN_ENDPOINT, + CDN_PUBLIC_ENDPOINT, CDN_PUBLIC_URL_PREFIX, DEPLOYMENT_USE_TX, @@ -134,6 +136,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 }), @@ -149,6 +152,7 @@ export const getEnvVars = (env: NodeJS.ProcessEnv = process.env): PgpmOptions => ...((AWS_ACCESS_KEY || AWS_ACCESS_KEY_ID) && { awsAccessKey: AWS_ACCESS_KEY || AWS_ACCESS_KEY_ID }), ...((AWS_SECRET_KEY || AWS_SECRET_ACCESS_KEY) && { awsSecretKey: AWS_SECRET_KEY || AWS_SECRET_ACCESS_KEY }), ...(CDN_ENDPOINT && { endpoint: CDN_ENDPOINT }), + ...(CDN_PUBLIC_ENDPOINT && { publicEndpoint: CDN_PUBLIC_ENDPOINT }), ...(CDN_PUBLIC_URL_PREFIX && { publicUrlPrefix: CDN_PUBLIC_URL_PREFIX }), }, deployment: { diff --git a/pgpm/types/src/pgpm.ts b/pgpm/types/src/pgpm.ts index 194c9e6e09..18f64efb47 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; } /** @@ -143,6 +149,11 @@ export interface CDNOptions { awsSecretKey?: string; /** S3-compatible API endpoint URL (RustFS, MinIO, R2, DO Spaces, GCS, etc.) */ endpoint?: string; + /** + * Client-reachable endpoint that presigned URLs are signed for. Set it when + * `endpoint` is an internal host (e.g. an in-cluster Service); defaults to `endpoint`. + */ + publicEndpoint?: string; /** Public URL prefix for generating download URLs (e.g., CDN domain, S3 public URL) */ publicUrlPrefix?: string; } @@ -383,7 +394,8 @@ export const pgpmDefaults: PgpmOptions = { host: 'localhost', port: 3000, trustProxy: false, - strictAuth: false + strictAuth: false, + exposeErrors: false }, cdn: { provider: 'minio',