From ca660a058570905e6e83e3a2e06de9b0cfd69198 Mon Sep 17 00:00:00 2001 From: johannesjo Date: Wed, 13 May 2026 02:10:46 +0200 Subject: [PATCH] fix(supersync): harden production server deploy --- packages/super-sync-server/README.md | 9 ++ packages/super-sync-server/scripts/deploy.sh | 134 +++++++++++++++++- packages/super-sync-server/src/server.ts | 42 +++++- .../services/operation-download.service.ts | 6 +- .../src/sync/websocket.routes.ts | 7 +- .../tests/migration-sql.spec.ts | 29 ++++ .../tests/operation-download.service.spec.ts | 5 +- .../tests/server-security.spec.ts | 17 +++ .../tests/websocket.routes.spec.ts | 21 ++- 9 files changed, 249 insertions(+), 21 deletions(-) diff --git a/packages/super-sync-server/README.md b/packages/super-sync-server/README.md index 8787a87529..1a92f5c1c5 100644 --- a/packages/super-sync-server/README.md +++ b/packages/super-sync-server/README.md @@ -67,6 +67,15 @@ changes, and raise `MIGRATION_TIMEOUT` (seconds, default `900`) if a large table requires more time. Exit code `124` from `deploy.sh` means the migration timed out — re-run after the blocking transaction clears. +If a deploy was interrupted after Prisma recorded the +`20260512000000_add_full_state_sequence_index_drop_redundant_indexes` migration +as failed, later deploys can stop with `P3009`. Prisma can also stop this +specific migration with `P3018` because it contains several `CREATE/DROP INDEX +CONCURRENTLY` statements, which cannot run in one transaction block. `deploy.sh` +handles both cases: it resolves the failed row when needed, applies the +concurrent index statements one at a time outside Prisma migrate, marks the +migration applied, and retries `migrate deploy`. + If `DATABASE_URL` points to an external PostgreSQL server, set `POSTGRES_SERVICE=` to the empty value. `deploy.sh` then starts only the app/proxy services with compose dependencies disabled so the bundled Postgres diff --git a/packages/super-sync-server/scripts/deploy.sh b/packages/super-sync-server/scripts/deploy.sh index 31bd655443..ecb9bd631e 100755 --- a/packages/super-sync-server/scripts/deploy.sh +++ b/packages/super-sync-server/scripts/deploy.sh @@ -148,6 +148,7 @@ echo "" # loudly instead of hanging this script forever. Exit code 124 = timed out. MIGRATION_TIMEOUT="${MIGRATION_TIMEOUT:-900}" MIGRATOR_RUN="docker compose $COMPOSE_FILES run --rm --no-deps --interactive=false -T supersync" +FULL_STATE_INDEX_MIGRATION="20260512000000_add_full_state_sequence_index_drop_redundant_indexes" echo "==> Verifying database connectivity from the supersync image..." set +e timeout "$POSTGRES_WAIT_TIMEOUT" \ @@ -169,11 +170,134 @@ fi echo " Database reachable" echo "" echo "==> Applying database migrations before app restart (timeout: ${MIGRATION_TIMEOUT}s)..." -set +e -timeout "$MIGRATION_TIMEOUT" \ - $MIGRATOR_RUN sh -ec 'echo " Migrator container started"; npx prisma migrate deploy' -MIGRATE_STATUS=$? -set -e + +MIGRATE_LOG="" +MIGRATE_STATUS=0 + +run_migrate_deploy() { + MIGRATE_LOG="$(mktemp "${TMPDIR:-/tmp}/supersync-migrate.XXXXXX")" + set +e + timeout "$MIGRATION_TIMEOUT" \ + $MIGRATOR_RUN sh -ec 'echo " Migrator container started"; npx prisma migrate deploy' 2>&1 | tee "$MIGRATE_LOG" + MIGRATE_STATUS=${PIPESTATUS[0]} + set -e +} + +is_recoverable_full_state_index_migration_failure() { + [ -n "$MIGRATE_LOG" ] && + grep -q 'P3009' "$MIGRATE_LOG" && + grep -q "$FULL_STATE_INDEX_MIGRATION" "$MIGRATE_LOG" +} + +is_full_state_index_transaction_block_failure() { + [ -n "$MIGRATE_LOG" ] && + grep -q 'P3018' "$MIGRATE_LOG" && + grep -q "$FULL_STATE_INDEX_MIGRATION" "$MIGRATE_LOG" && + grep -q 'cannot run inside a transaction block' "$MIGRATE_LOG" +} + +run_concurrent_index_sql() { + local sql="$1" + local execute_status + + set +e + # Use the same supersync container and DATABASE_URL as `migrate deploy`. + # This avoids marking the Prisma migration applied in one database after + # running the out-of-band index SQL against another. + printf '%s\n' "$sql" | timeout "$MIGRATION_TIMEOUT" \ + $MIGRATOR_RUN sh -ec 'npx prisma db execute --schema prisma/schema.prisma --stdin' + execute_status=${PIPESTATUS[1]} + set -e + + if [ "$execute_status" -eq 124 ]; then + echo "" + echo "ERROR: concurrent index SQL timed out after ${MIGRATION_TIMEOUT}s." + echo " A long-running transaction may be blocking CREATE/DROP INDEX CONCURRENTLY." + exit 1 + fi + if [ "$execute_status" -ne 0 ]; then + echo "" + echo "ERROR: concurrent index SQL failed (exit $execute_status)." + exit "$execute_status" + fi +} + +apply_full_state_index_migration_outside_prisma() { + echo "" + echo "==> Applying $FULL_STATE_INDEX_MIGRATION outside Prisma migrate..." + echo " Prisma cannot run this multi-statement CONCURRENTLY migration in one transaction block." + + run_concurrent_index_sql 'DROP INDEX CONCURRENTLY IF EXISTS "operations_user_id_full_state_server_seq_idx";' + run_concurrent_index_sql "CREATE INDEX CONCURRENTLY \"operations_user_id_full_state_server_seq_idx\" ON \"operations\"(\"user_id\", \"server_seq\") WHERE \"op_type\" IN ('SYNC_IMPORT', 'BACKUP_IMPORT', 'REPAIR');" + run_concurrent_index_sql 'DROP INDEX CONCURRENTLY IF EXISTS "operations_user_id_op_type_idx";' + run_concurrent_index_sql 'DROP INDEX CONCURRENTLY IF EXISTS "operations_user_id_entity_type_entity_id_idx";' + run_concurrent_index_sql 'DROP INDEX CONCURRENTLY IF EXISTS "operations_user_id_server_seq_idx";' +} + +resolve_failed_full_state_index_migration() { + local resolve_status + + echo "" + echo "==> Resolving failed index-only migration $FULL_STATE_INDEX_MIGRATION..." + echo " Marking it rolled back so Prisma can retry the idempotent migration SQL." + set +e + timeout "$POSTGRES_WAIT_TIMEOUT" \ + $MIGRATOR_RUN npx prisma migrate resolve --rolled-back "$FULL_STATE_INDEX_MIGRATION" + resolve_status=$? + set -e + + if [ "$resolve_status" -eq 124 ]; then + echo "" + echo "ERROR: prisma migrate resolve timed out after ${POSTGRES_WAIT_TIMEOUT}s." + exit 1 + fi + if [ "$resolve_status" -ne 0 ]; then + echo "" + echo "ERROR: prisma migrate resolve failed (exit $resolve_status)." + exit "$resolve_status" + fi +} + +resolve_applied_full_state_index_migration() { + local resolve_status + + echo "" + echo "==> Marking $FULL_STATE_INDEX_MIGRATION as applied after out-of-band SQL..." + set +e + timeout "$POSTGRES_WAIT_TIMEOUT" \ + $MIGRATOR_RUN npx prisma migrate resolve --applied "$FULL_STATE_INDEX_MIGRATION" + resolve_status=$? + set -e + + if [ "$resolve_status" -eq 124 ]; then + echo "" + echo "ERROR: prisma migrate resolve --applied timed out after ${POSTGRES_WAIT_TIMEOUT}s." + exit 1 + fi + if [ "$resolve_status" -ne 0 ]; then + echo "" + echo "ERROR: prisma migrate resolve --applied failed (exit $resolve_status)." + exit "$resolve_status" + fi +} + +run_migrate_deploy +if [ "$MIGRATE_STATUS" -ne 0 ] && [ "$MIGRATE_STATUS" -ne 124 ] && + is_recoverable_full_state_index_migration_failure; then + resolve_failed_full_state_index_migration + echo "" + echo "==> Retrying database migrations after resolving $FULL_STATE_INDEX_MIGRATION..." + run_migrate_deploy +fi +if [ "$MIGRATE_STATUS" -ne 0 ] && [ "$MIGRATE_STATUS" -ne 124 ] && + is_full_state_index_transaction_block_failure; then + apply_full_state_index_migration_outside_prisma + resolve_applied_full_state_index_migration + echo "" + echo "==> Retrying database migrations after applying $FULL_STATE_INDEX_MIGRATION..." + run_migrate_deploy +fi + if [ "$MIGRATE_STATUS" -eq 124 ]; then echo "" echo "ERROR: prisma migrate deploy timed out after ${MIGRATION_TIMEOUT}s." diff --git a/packages/super-sync-server/src/server.ts b/packages/super-sync-server/src/server.ts index 87e4bab0ee..eb9284001d 100644 --- a/packages/super-sync-server/src/server.ts +++ b/packages/super-sync-server/src/server.ts @@ -29,6 +29,37 @@ const escapeHtml = (unsafe: string): string => { .replace(/'/g, '''); }; +const SENSITIVE_QUERY_PARAMS = [ + 'authorization', + 'jwt', + 'logintoken', + 'password', + 'passkeyrecoverytoken', + 'resetpasswordtoken', + 'token', +] as const; + +const SENSITIVE_QUERY_PARAM_SET = new Set(SENSITIVE_QUERY_PARAMS); + +const SENSITIVE_QUERY_PARAM_PATTERN = new RegExp( + `([?&](?:${SENSITIVE_QUERY_PARAMS.join('|')})=)[^&\\s]*`, + 'gi', +); + +export const sanitizeRequestUrlForLog = (rawUrl: string): string => { + try { + const url = new URL(rawUrl, 'http://localhost'); + for (const key of Array.from(url.searchParams.keys())) { + if (SENSITIVE_QUERY_PARAM_SET.has(key.toLowerCase())) { + url.searchParams.set(key, 'redacted'); + } + } + return `${url.pathname}${url.search}`; + } catch { + return rawUrl.replace(SENSITIVE_QUERY_PARAM_PATTERN, '$1redacted'); + } +}; + const generatePrivacyHtml = (privacy?: PrivacyConfig): void => { const publicDir = path.join(__dirname, '../../public'); const templatePath = path.join(publicDir, 'privacy.template.html'); @@ -113,10 +144,13 @@ export const createServer = ( // validation messages or auth failures that are safe and actionable. fastifyServer.setErrorHandler((error: FastifyError, req, reply) => { const statusCode = error.statusCode ?? 500; - Logger.error( - `Request failed ${statusCode} ${req.method} ${req.url}: ${error.name}: ${error.message}`, - error.stack, - ); + const sanitizedUrl = sanitizeRequestUrlForLog(req.url); + const logMessage = `Request failed ${statusCode} ${req.method} ${sanitizedUrl}: ${error.name}: ${error.message}`; + if (statusCode >= 500) { + Logger.error(logMessage, error.stack); + } else { + Logger.warn(logMessage); + } if (statusCode >= 500) { return reply.status(500).send({ statusCode: 500, diff --git a/packages/super-sync-server/src/sync/services/operation-download.service.ts b/packages/super-sync-server/src/sync/services/operation-download.service.ts index b6f050922d..2f919ca909 100644 --- a/packages/super-sync-server/src/sync/services/operation-download.service.ts +++ b/packages/super-sync-server/src/sync/services/operation-download.service.ts @@ -30,6 +30,8 @@ const OPERATION_DOWNLOAD_SELECT = { syncImportReason: true, } as const; +const DOWNLOAD_TRANSACTION_TIMEOUT_MS = 60000; + type OperationDownloadRow = { id: string; serverSeq: number; @@ -281,8 +283,8 @@ export class OperationDownloadService { snapshotVectorClock, }; }, - { timeout: 30000 }, - ); // 30s - consistent with other sync transactions + { timeout: DOWNLOAD_TRANSACTION_TIMEOUT_MS }, + ); // Matches other sync transactions; stays below Fastify's 80s request timeout. } /** diff --git a/packages/super-sync-server/src/sync/websocket.routes.ts b/packages/super-sync-server/src/sync/websocket.routes.ts index ece5ea06fb..dbc9251a7b 100644 --- a/packages/super-sync-server/src/sync/websocket.routes.ts +++ b/packages/super-sync-server/src/sync/websocket.routes.ts @@ -4,6 +4,9 @@ import { getWsConnectionService } from './services/websocket-connection.service' import { Logger } from '../logger'; import { CLIENT_ID_REGEX, MAX_CLIENT_ID_LENGTH } from './sync.const'; +export const WS_CONNECTION_RATE_LIMIT_MAX = 120; +export const WS_CONNECTION_RATE_LIMIT_WINDOW = '1 minute'; + export const wsRoutes = async (fastify: FastifyInstance): Promise => { fastify.get( '/ws', @@ -11,8 +14,8 @@ export const wsRoutes = async (fastify: FastifyInstance): Promise => { websocket: true, config: { rateLimit: { - max: 10, - timeWindow: '1 minute', + max: WS_CONNECTION_RATE_LIMIT_MAX, + timeWindow: WS_CONNECTION_RATE_LIMIT_WINDOW, }, }, }, diff --git a/packages/super-sync-server/tests/migration-sql.spec.ts b/packages/super-sync-server/tests/migration-sql.spec.ts index ab5a216342..ffb1316a30 100644 --- a/packages/super-sync-server/tests/migration-sql.spec.ts +++ b/packages/super-sync-server/tests/migration-sql.spec.ts @@ -75,6 +75,35 @@ describe('performance migrations', () => { expect(deployScript).toContain('prisma db execute'); expect(deployScript).toContain(migrationCommand); expect(deployScript).toContain('Migrator container started'); + expect(deployScript).toContain( + 'FULL_STATE_INDEX_MIGRATION="20260512000000_add_full_state_sequence_index_drop_redundant_indexes"', + ); + expect(deployScript).toContain('is_recoverable_full_state_index_migration_failure'); + expect(deployScript).toContain("grep -q 'P3009'"); + expect(deployScript).toContain('is_full_state_index_transaction_block_failure'); + expect(deployScript).toContain("grep -q 'P3018'"); + expect(deployScript).toContain("grep -q 'cannot run inside a transaction block'"); + expect(deployScript).toContain('run_concurrent_index_sql'); + expect(deployScript).toContain( + 'Use the same supersync container and DATABASE_URL as `migrate deploy`', + ); + expect(deployScript).not.toContain('psql -v ON_ERROR_STOP=1'); + expect(deployScript).toContain('prisma db execute --schema prisma/schema.prisma'); + expect(deployScript).toContain( + 'CREATE INDEX CONCURRENTLY \\"operations_user_id_full_state_server_seq_idx\\"', + ); + expect(deployScript).toContain( + 'migrate resolve --rolled-back "$FULL_STATE_INDEX_MIGRATION"', + ); + expect(deployScript).toContain( + 'migrate resolve --applied "$FULL_STATE_INDEX_MIGRATION"', + ); + expect(deployScript).toContain( + 'Retrying database migrations after resolving $FULL_STATE_INDEX_MIGRATION', + ); + expect(deployScript).toContain( + 'Retrying database migrations after applying $FULL_STATE_INDEX_MIGRATION', + ); expect(deployScript).toContain(externalDbStartCommand); expect(deployScript).toContain('RUN_MIGRATIONS_ON_STARTUP'); expect(deployScript.indexOf(migrationCommand)).toBeLessThan( diff --git a/packages/super-sync-server/tests/operation-download.service.spec.ts b/packages/super-sync-server/tests/operation-download.service.spec.ts index 9176b09395..1e460040a1 100644 --- a/packages/super-sync-server/tests/operation-download.service.spec.ts +++ b/packages/super-sync-server/tests/operation-download.service.spec.ts @@ -291,8 +291,10 @@ describe('OperationDownloadService', () => { it('should select only download response fields inside atomic reads', async () => { let capturedTx: any; + let capturedOptions: any; - vi.mocked(prisma.$transaction).mockImplementation(async (fn: any) => { + vi.mocked(prisma.$transaction).mockImplementation(async (fn: any, options: any) => { + capturedOptions = options; capturedTx = { operation: { findFirst: vi.fn().mockResolvedValue(null), @@ -308,6 +310,7 @@ describe('OperationDownloadService', () => { await service.getOpsSinceWithSeq(1, 0); + expect(capturedOptions).toEqual({ timeout: 60000 }); expect(capturedTx.operation.findMany).toHaveBeenCalledWith( expect.objectContaining({ select: EXPECTED_OPERATION_DOWNLOAD_SELECT, diff --git a/packages/super-sync-server/tests/server-security.spec.ts b/packages/super-sync-server/tests/server-security.spec.ts index bd2fd263b7..4f533eae3e 100644 --- a/packages/super-sync-server/tests/server-security.spec.ts +++ b/packages/super-sync-server/tests/server-security.spec.ts @@ -1,8 +1,25 @@ import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; import Fastify, { FastifyInstance } from 'fastify'; import helmet from '@fastify/helmet'; +import { sanitizeRequestUrlForLog } from '../src/server'; describe('Server Security Configuration', () => { + describe('request URL log sanitization', () => { + it('should redact sensitive query params without removing non-sensitive context', () => { + expect( + sanitizeRequestUrlForLog( + '/api/sync/ws?token=secret-jwt&clientId=B_AEh6&limit=10', + ), + ).toBe('/api/sync/ws?token=redacted&clientId=B_AEh6&limit=10'); + }); + + it('should redact sensitive query params case-insensitively', () => { + expect(sanitizeRequestUrlForLog('/reset-password?resetPasswordToken=secret')).toBe( + '/reset-password?resetPasswordToken=redacted', + ); + }); + }); + describe('Content Security Policy', () => { let app: FastifyInstance; diff --git a/packages/super-sync-server/tests/websocket.routes.spec.ts b/packages/super-sync-server/tests/websocket.routes.spec.ts index b7c0794d36..3d7102c23c 100644 --- a/packages/super-sync-server/tests/websocket.routes.spec.ts +++ b/packages/super-sync-server/tests/websocket.routes.spec.ts @@ -1,5 +1,9 @@ import { describe, it, expect, beforeEach, vi } from 'vitest'; import { CLIENT_ID_REGEX, MAX_CLIENT_ID_LENGTH } from '../src/sync/sync.const'; +import { + WS_CONNECTION_RATE_LIMIT_MAX, + WS_CONNECTION_RATE_LIMIT_WINDOW, +} from '../src/sync/websocket.routes'; /** * Tests the WebSocket route validation logic from websocket.routes.ts. @@ -44,9 +48,8 @@ async function simulateWsHandler( socket: { close: ReturnType }, ): Promise<'accepted' | 'rejected'> { // Dynamic import to pick up the vi.mock above - const { getWsConnectionService } = await import( - '../src/sync/services/websocket-connection.service' - ); + const { getWsConnectionService } = + await import('../src/sync/services/websocket-connection.service'); try { const { token, clientId } = query; @@ -97,6 +100,13 @@ describe('WebSocket Route Validation', () => { }); }); + describe('route rate limit', () => { + it('should tolerate reconnect bursts after deploys and restarts', () => { + expect(WS_CONNECTION_RATE_LIMIT_MAX).toBe(120); + expect(WS_CONNECTION_RATE_LIMIT_WINDOW).toBe('1 minute'); + }); + }); + describe('CLIENT_ID_REGEX', () => { it('should accept alphanumeric characters', () => { expect(CLIENT_ID_REGEX.test('abc123')).toBe(true); @@ -131,10 +141,7 @@ describe('WebSocket Route Validation', () => { describe('handler validation flow', () => { it('should reject when token is missing', async () => { - const result = await simulateWsHandler( - { clientId: 'valid_client' }, - mockSocket, - ); + const result = await simulateWsHandler({ clientId: 'valid_client' }, mockSocket); expect(result).toBe('rejected'); expect(mockSocket.close).toHaveBeenCalledWith(4001, 'Missing token');