From 2f25d10368b54f62469e4d102828210b5057f6f7 Mon Sep 17 00:00:00 2001 From: zouantchaw <44246692+zouantchaw@users.noreply.github.com> Date: Thu, 19 Mar 2026 16:36:28 +0100 Subject: [PATCH] fix(backend): harden runtime config and verification access --- backend/command-api/src/app.js | 2 + backend/command-api/src/lib/runtime-safety.js | 44 ++++++++++++++++++ backend/command-api/src/worker-app.js | 2 + backend/command-api/test/app.test.js | 10 +++++ .../test/notification-worker.test.js | 10 +++++ backend/core-api/src/app.js | 2 + backend/core-api/src/lib/runtime-safety.js | 45 +++++++++++++++++++ .../src/services/verification-jobs.js | 43 +++++++++++++----- backend/core-api/test/app.test.js | 39 +++++++++++++++- backend/query-api/src/app.js | 2 + backend/query-api/src/lib/runtime-safety.js | 17 +++++++ backend/query-api/test/app.test.js | 10 +++++ backend/unified-api/src/app.js | 2 + backend/unified-api/src/lib/runtime-safety.js | 35 +++++++++++++++ backend/unified-api/test/app.test.js | 13 ++++++ 15 files changed, 262 insertions(+), 14 deletions(-) create mode 100644 backend/command-api/src/lib/runtime-safety.js create mode 100644 backend/core-api/src/lib/runtime-safety.js create mode 100644 backend/query-api/src/lib/runtime-safety.js create mode 100644 backend/unified-api/src/lib/runtime-safety.js diff --git a/backend/command-api/src/app.js b/backend/command-api/src/app.js index 0c6fa44a..aca81b20 100644 --- a/backend/command-api/src/app.js +++ b/backend/command-api/src/app.js @@ -6,10 +6,12 @@ import { errorHandler, notFoundHandler } from './middleware/error-handler.js'; import { healthRouter } from './routes/health.js'; import { createCommandsRouter } from './routes/commands.js'; import { createMobileCommandsRouter } from './routes/mobile.js'; +import { assertSafeRuntimeConfig } from './lib/runtime-safety.js'; const logger = pino({ level: process.env.LOG_LEVEL || 'info' }); export function createApp(options = {}) { + assertSafeRuntimeConfig(); const app = express(); app.use(requestContext); diff --git a/backend/command-api/src/lib/runtime-safety.js b/backend/command-api/src/lib/runtime-safety.js new file mode 100644 index 00000000..74bbf0ca --- /dev/null +++ b/backend/command-api/src/lib/runtime-safety.js @@ -0,0 +1,44 @@ +function runtimeEnvName() { + return `${process.env.APP_ENV || process.env.NODE_ENV || ''}`.trim().toLowerCase(); +} + +function isProtectedEnv() { + return ['staging', 'prod', 'production'].includes(runtimeEnvName()); +} + +export function assertSafeRuntimeConfig() { + if (!isProtectedEnv()) { + return; + } + + const errors = []; + + if (process.env.AUTH_BYPASS === 'true') { + errors.push('AUTH_BYPASS must be disabled'); + } + + if (`${process.env.IDEMPOTENCY_STORE || ''}`.trim().toLowerCase() === 'memory') { + errors.push('IDEMPOTENCY_STORE must not be memory'); + } + + if (errors.length > 0) { + throw new Error(`Unsafe command-api runtime config for ${runtimeEnvName()}: ${errors.join('; ')}`); + } +} + +export function assertSafeWorkerRuntimeConfig() { + if (!isProtectedEnv()) { + return; + } + + const errors = []; + const deliveryMode = `${process.env.PUSH_DELIVERY_MODE || 'live'}`.trim().toLowerCase(); + + if (deliveryMode !== 'live') { + errors.push('PUSH_DELIVERY_MODE must be live'); + } + + if (errors.length > 0) { + throw new Error(`Unsafe notification-worker runtime config for ${runtimeEnvName()}: ${errors.join('; ')}`); + } +} diff --git a/backend/command-api/src/worker-app.js b/backend/command-api/src/worker-app.js index 8ccd96ed..08b42c2c 100644 --- a/backend/command-api/src/worker-app.js +++ b/backend/command-api/src/worker-app.js @@ -1,10 +1,12 @@ import express from 'express'; import pino from 'pino'; import pinoHttp from 'pino-http'; +import { assertSafeWorkerRuntimeConfig } from './lib/runtime-safety.js'; const logger = pino({ level: process.env.LOG_LEVEL || 'info' }); export function createWorkerApp({ dispatch = async () => ({}) } = {}) { + assertSafeWorkerRuntimeConfig(); const app = express(); app.use( diff --git a/backend/command-api/test/app.test.js b/backend/command-api/test/app.test.js index ad1a91c3..dc9c6f7e 100644 --- a/backend/command-api/test/app.test.js +++ b/backend/command-api/test/app.test.js @@ -63,6 +63,16 @@ test('GET /readyz reports database not configured when no database env is presen assert.equal(res.body.status, 'DATABASE_NOT_CONFIGURED'); }); +test('createApp fails fast in protected env when auth bypass is enabled', async () => { + process.env.APP_ENV = 'staging'; + process.env.AUTH_BYPASS = 'true'; + + assert.throws(() => createApp(), /AUTH_BYPASS must be disabled/); + + delete process.env.APP_ENV; + process.env.AUTH_BYPASS = 'true'; +}); + test('command route requires idempotency key', async () => { const app = createApp(); const res = await request(app) diff --git a/backend/command-api/test/notification-worker.test.js b/backend/command-api/test/notification-worker.test.js index a4865b55..2816c9bf 100644 --- a/backend/command-api/test/notification-worker.test.js +++ b/backend/command-api/test/notification-worker.test.js @@ -12,6 +12,16 @@ test('GET /readyz returns healthy response', async () => { assert.equal(res.body.service, 'notification-worker-v2'); }); +test('createWorkerApp fails fast in protected env when push delivery is not live', async () => { + process.env.APP_ENV = 'staging'; + process.env.PUSH_DELIVERY_MODE = 'log-only'; + + assert.throws(() => createWorkerApp(), /PUSH_DELIVERY_MODE must be live/); + + delete process.env.APP_ENV; + delete process.env.PUSH_DELIVERY_MODE; +}); + test('POST /tasks/dispatch-notifications returns dispatch summary', async () => { const app = createWorkerApp({ dispatch: async () => ({ diff --git a/backend/core-api/src/app.js b/backend/core-api/src/app.js index af2f1a13..dd5e6a14 100644 --- a/backend/core-api/src/app.js +++ b/backend/core-api/src/app.js @@ -5,10 +5,12 @@ import { requestContext } from './middleware/request-context.js'; import { errorHandler, notFoundHandler } from './middleware/error-handler.js'; import { healthRouter } from './routes/health.js'; import { createCoreRouter, createLegacyCoreRouter } from './routes/core.js'; +import { assertSafeRuntimeConfig } from './lib/runtime-safety.js'; const logger = pino({ level: process.env.LOG_LEVEL || 'info' }); export function createApp() { + assertSafeRuntimeConfig(); const app = express(); app.use(requestContext); diff --git a/backend/core-api/src/lib/runtime-safety.js b/backend/core-api/src/lib/runtime-safety.js new file mode 100644 index 00000000..31aa0c71 --- /dev/null +++ b/backend/core-api/src/lib/runtime-safety.js @@ -0,0 +1,45 @@ +function runtimeEnvName() { + return `${process.env.APP_ENV || process.env.NODE_ENV || ''}`.trim().toLowerCase(); +} + +function isProtectedEnv() { + return ['staging', 'prod', 'production'].includes(runtimeEnvName()); +} + +export function assertSafeRuntimeConfig() { + if (!isProtectedEnv()) { + return; + } + + const errors = []; + + if (process.env.AUTH_BYPASS === 'true') { + errors.push('AUTH_BYPASS must be disabled'); + } + + if (process.env.UPLOAD_MOCK !== 'false') { + errors.push('UPLOAD_MOCK must be false'); + } + + if (process.env.SIGNED_URL_MOCK !== 'false') { + errors.push('SIGNED_URL_MOCK must be false'); + } + + if (process.env.LLM_MOCK !== 'false') { + errors.push('LLM_MOCK must be false'); + } + + const verificationStore = `${process.env.VERIFICATION_STORE || 'sql'}`.trim().toLowerCase(); + if (verificationStore !== 'sql') { + errors.push('VERIFICATION_STORE must be sql'); + } + + const verificationAccessMode = `${process.env.VERIFICATION_ACCESS_MODE || 'tenant'}`.trim().toLowerCase(); + if (verificationAccessMode === 'authenticated') { + errors.push('VERIFICATION_ACCESS_MODE must not be authenticated'); + } + + if (errors.length > 0) { + throw new Error(`Unsafe core-api runtime config for ${runtimeEnvName()}: ${errors.join('; ')}`); + } +} diff --git a/backend/core-api/src/services/verification-jobs.js b/backend/core-api/src/services/verification-jobs.js index ce46679b..ac70aab8 100644 --- a/backend/core-api/src/services/verification-jobs.js +++ b/backend/core-api/src/services/verification-jobs.js @@ -1,6 +1,6 @@ import { AppError } from '../lib/errors.js'; import { isDatabaseConfigured, query, withTransaction } from './db.js'; -import { requireTenantContext } from './actor-context.js'; +import { loadActorContext, requireTenantContext } from './actor-context.js'; import { invokeVertexMultimodalModel } from './llm.js'; export const VerificationStatus = Object.freeze({ @@ -95,7 +95,11 @@ async function processVerificationJobInMemory(verificationId) { } function accessMode() { - return process.env.VERIFICATION_ACCESS_MODE || 'authenticated'; + const mode = `${process.env.VERIFICATION_ACCESS_MODE || 'tenant'}`.trim().toLowerCase(); + if (mode === 'owner' || mode === 'tenant' || mode === 'authenticated') { + return mode; + } + return 'tenant'; } function providerTimeoutMs() { @@ -156,12 +160,27 @@ function toPublicJob(row) { }; } -function assertAccess(row, actorUid) { - if (accessMode() === 'authenticated') { +async function assertAccess(row, actorUid) { + if (row.owner_user_id === actorUid) { return; } - if (row.owner_user_id !== actorUid) { - throw new AppError('FORBIDDEN', 'Not allowed to access this verification', 403); + + const mode = accessMode(); + if (mode === 'authenticated') { + return; + } + + if (mode === 'owner' || !row.tenant_id) { + throw new AppError('FORBIDDEN', 'Not allowed to access this verification', 403, { + verificationId: row.id, + }); + } + + const actorContext = await loadActorContext(actorUid); + if (actorContext.tenant?.tenantId !== row.tenant_id) { + throw new AppError('FORBIDDEN', 'Not allowed to access this verification', 403, { + verificationId: row.id, + }); } } @@ -614,19 +633,19 @@ export async function createVerificationJob({ actorUid, payload }) { export async function getVerificationJob(verificationId, actorUid) { if (useMemoryStore()) { const job = loadMemoryJob(verificationId); - assertAccess(job, actorUid); + await assertAccess(job, actorUid); return toPublicJob(job); } const job = await loadJob(verificationId); - assertAccess(job, actorUid); + await assertAccess(job, actorUid); return toPublicJob(job); } export async function reviewVerificationJob(verificationId, actorUid, review) { if (useMemoryStore()) { const job = loadMemoryJob(verificationId); - assertAccess(job, actorUid); + await assertAccess(job, actorUid); if (HUMAN_TERMINAL_STATUSES.has(job.status)) { throw new AppError('CONFLICT', 'Verification already finalized', 409, { verificationId, @@ -668,7 +687,7 @@ export async function reviewVerificationJob(verificationId, actorUid, review) { } const job = result.rows[0]; - assertAccess(job, actorUid); + await assertAccess(job, actorUid); if (HUMAN_TERMINAL_STATUSES.has(job.status)) { throw new AppError('CONFLICT', 'Verification already finalized', 409, { verificationId, @@ -735,7 +754,7 @@ export async function reviewVerificationJob(verificationId, actorUid, review) { export async function retryVerificationJob(verificationId, actorUid) { if (useMemoryStore()) { const job = loadMemoryJob(verificationId); - assertAccess(job, actorUid); + await assertAccess(job, actorUid); if (job.status === VerificationStatus.PROCESSING) { throw new AppError('CONFLICT', 'Cannot retry while verification is processing', 409, { verificationId, @@ -774,7 +793,7 @@ export async function retryVerificationJob(verificationId, actorUid) { } const job = result.rows[0]; - assertAccess(job, actorUid); + await assertAccess(job, actorUid); if (job.status === VerificationStatus.PROCESSING) { throw new AppError('CONFLICT', 'Cannot retry while verification is processing', 409, { verificationId, diff --git a/backend/core-api/test/app.test.js b/backend/core-api/test/app.test.js index f2193843..fafed45b 100644 --- a/backend/core-api/test/app.test.js +++ b/backend/core-api/test/app.test.js @@ -3,7 +3,11 @@ import assert from 'node:assert/strict'; import request from 'supertest'; import { createApp } from '../src/app.js'; import { __resetLlmRateLimitForTests } from '../src/services/llm-rate-limit.js'; -import { __resetVerificationJobsForTests } from '../src/services/verification-jobs.js'; +import { + __resetVerificationJobsForTests, + createVerificationJob, + getVerificationJob, +} from '../src/services/verification-jobs.js'; beforeEach(async () => { process.env.AUTH_BYPASS = 'true'; @@ -13,7 +17,7 @@ beforeEach(async () => { process.env.MAX_SIGNED_URL_SECONDS = '900'; process.env.LLM_RATE_LIMIT_PER_MINUTE = '20'; process.env.VERIFICATION_REQUIRE_FILE_EXISTS = 'false'; - process.env.VERIFICATION_ACCESS_MODE = 'authenticated'; + process.env.VERIFICATION_ACCESS_MODE = 'tenant'; process.env.VERIFICATION_ATTIRE_PROVIDER = 'mock'; process.env.VERIFICATION_STORE = 'memory'; __resetLlmRateLimitForTests(); @@ -66,6 +70,16 @@ test('GET /readyz reports database not configured when env is absent', async () assert.equal(res.body.status, 'DATABASE_NOT_CONFIGURED'); }); +test('createApp fails fast in protected env when unsafe core flags are enabled', async () => { + process.env.APP_ENV = 'staging'; + process.env.AUTH_BYPASS = 'true'; + + assert.throws(() => createApp(), /AUTH_BYPASS must be disabled/); + + delete process.env.APP_ENV; + process.env.AUTH_BYPASS = 'true'; +}); + test('POST /core/create-signed-url requires auth', async () => { process.env.AUTH_BYPASS = 'false'; const app = createApp(); @@ -404,3 +418,24 @@ test('POST /core/verifications/:id/retry requeues verification', async () => { assert.equal(retried.status, 202); assert.equal(retried.body.status, 'PENDING'); }); + +test('verification access is denied to a different actor by default', async () => { + const created = await createVerificationJob({ + actorUid: 'owner-user', + payload: { + type: 'attire', + subjectType: 'staff', + subjectId: 'staff_1', + fileUri: 'gs://krow-workforce-dev-private/uploads/owner-user/attire.jpg', + rules: { attireType: 'shoes' }, + }, + }); + + await assert.rejects( + () => getVerificationJob(created.verificationId, 'foreign-user'), + (error) => { + assert.equal(error.code, 'FORBIDDEN'); + return true; + } + ); +}); diff --git a/backend/query-api/src/app.js b/backend/query-api/src/app.js index 43ff81da..1e363455 100644 --- a/backend/query-api/src/app.js +++ b/backend/query-api/src/app.js @@ -6,10 +6,12 @@ import { errorHandler, notFoundHandler } from './middleware/error-handler.js'; import { healthRouter } from './routes/health.js'; import { createQueryRouter } from './routes/query.js'; import { createMobileQueryRouter } from './routes/mobile.js'; +import { assertSafeRuntimeConfig } from './lib/runtime-safety.js'; const logger = pino({ level: process.env.LOG_LEVEL || 'info' }); export function createApp(options = {}) { + assertSafeRuntimeConfig(); const app = express(); app.use(requestContext); diff --git a/backend/query-api/src/lib/runtime-safety.js b/backend/query-api/src/lib/runtime-safety.js new file mode 100644 index 00000000..4b45bf08 --- /dev/null +++ b/backend/query-api/src/lib/runtime-safety.js @@ -0,0 +1,17 @@ +function runtimeEnvName() { + return `${process.env.APP_ENV || process.env.NODE_ENV || ''}`.trim().toLowerCase(); +} + +function isProtectedEnv() { + return ['staging', 'prod', 'production'].includes(runtimeEnvName()); +} + +export function assertSafeRuntimeConfig() { + if (!isProtectedEnv()) { + return; + } + + if (process.env.AUTH_BYPASS === 'true') { + throw new Error(`Unsafe query-api runtime config for ${runtimeEnvName()}: AUTH_BYPASS must be disabled`); + } +} diff --git a/backend/query-api/test/app.test.js b/backend/query-api/test/app.test.js index f2a5e9d7..26f751e2 100644 --- a/backend/query-api/test/app.test.js +++ b/backend/query-api/test/app.test.js @@ -37,6 +37,16 @@ test('GET /readyz reports database not configured when no database env is presen assert.equal(res.body.status, 'DATABASE_NOT_CONFIGURED'); }); +test('createApp fails fast in protected env when auth bypass is enabled', async () => { + process.env.APP_ENV = 'staging'; + process.env.AUTH_BYPASS = 'true'; + + assert.throws(() => createApp(), /AUTH_BYPASS must be disabled/); + + delete process.env.APP_ENV; + process.env.AUTH_BYPASS = 'true'; +}); + test('GET unknown route returns not found envelope', async () => { const app = createApp(); const res = await request(app).get('/query/unknown'); diff --git a/backend/unified-api/src/app.js b/backend/unified-api/src/app.js index a30f8657..82d23bec 100644 --- a/backend/unified-api/src/app.js +++ b/backend/unified-api/src/app.js @@ -6,10 +6,12 @@ import { errorHandler, notFoundHandler } from './middleware/error-handler.js'; import { healthRouter } from './routes/health.js'; import { createAuthRouter } from './routes/auth.js'; import { createProxyRouter } from './routes/proxy.js'; +import { assertSafeRuntimeConfig } from './lib/runtime-safety.js'; const logger = pino({ level: process.env.LOG_LEVEL || 'info' }); export function createApp(options = {}) { + assertSafeRuntimeConfig(); const app = express(); app.use(requestContext); diff --git a/backend/unified-api/src/lib/runtime-safety.js b/backend/unified-api/src/lib/runtime-safety.js new file mode 100644 index 00000000..587d8666 --- /dev/null +++ b/backend/unified-api/src/lib/runtime-safety.js @@ -0,0 +1,35 @@ +function runtimeEnvName() { + return `${process.env.APP_ENV || process.env.NODE_ENV || ''}`.trim().toLowerCase(); +} + +function isProtectedEnv() { + return ['staging', 'prod', 'production'].includes(runtimeEnvName()); +} + +export function assertSafeRuntimeConfig() { + if (!isProtectedEnv()) { + return; + } + + const errors = []; + + if (process.env.AUTH_BYPASS === 'true') { + errors.push('AUTH_BYPASS must be disabled'); + } + + if (!process.env.CORE_API_BASE_URL) { + errors.push('CORE_API_BASE_URL is required'); + } + + if (!process.env.COMMAND_API_BASE_URL) { + errors.push('COMMAND_API_BASE_URL is required'); + } + + if (!process.env.QUERY_API_BASE_URL) { + errors.push('QUERY_API_BASE_URL is required'); + } + + if (errors.length > 0) { + throw new Error(`Unsafe unified-api runtime config for ${runtimeEnvName()}: ${errors.join('; ')}`); + } +} diff --git a/backend/unified-api/test/app.test.js b/backend/unified-api/test/app.test.js index 02c42355..fb271c47 100644 --- a/backend/unified-api/test/app.test.js +++ b/backend/unified-api/test/app.test.js @@ -29,6 +29,19 @@ test('GET /readyz reports database not configured when env is absent', async () assert.equal(res.body.status, 'DATABASE_NOT_CONFIGURED'); }); +test('createApp fails fast in protected env when upstream config is unsafe', async () => { + process.env.APP_ENV = 'staging'; + process.env.AUTH_BYPASS = 'true'; + delete process.env.CORE_API_BASE_URL; + delete process.env.COMMAND_API_BASE_URL; + delete process.env.QUERY_API_BASE_URL; + + assert.throws(() => createApp(), /AUTH_BYPASS must be disabled/); + + delete process.env.APP_ENV; + process.env.AUTH_BYPASS = 'true'; +}); + test('POST /auth/client/sign-in validates payload', async () => { const app = createApp(); const res = await request(app).post('/auth/client/sign-in').send({