diff --git a/apps/review-service/package.json b/apps/review-service/package.json index 703a9fba..70d8b180 100644 --- a/apps/review-service/package.json +++ b/apps/review-service/package.json @@ -5,7 +5,7 @@ "scripts": { "build": "nest build", "dev": "nest start --watch", - "lint": "tsc --noEmit && prettier --single-quote --check package.json migrations/README.md src/**/*.ts ../../docs/superpowers/plans/2026-08-04-guided-review-service-slice.md", + "lint": "tsc --noEmit && prettier --single-quote --check package.json migrations/README.md 'src/**/*.ts' ../../docs/superpowers/plans/2026-08-04-guided-review-service-slice.md", "test": "vitest run --passWithNoTests --no-file-parallelism", "typecheck": "tsc --noEmit" }, diff --git a/apps/review-service/src/http-boundary.test.ts b/apps/review-service/src/http-boundary.test.ts index dfde5e26..a88c608a 100644 --- a/apps/review-service/src/http-boundary.test.ts +++ b/apps/review-service/src/http-boundary.test.ts @@ -1,11 +1,14 @@ +import { randomBytes } from 'node:crypto'; import { HttpException } from '@nestjs/common'; import { describe, expect, it } from 'vitest'; import { requireHistoryLimit, requireRitualPath, - requireWorkspaceHeader, + requireTrustedWorkspaceContext, toReviewHttpException, + type ReviewTrustedRequestBinding, } from './http-boundary'; +import { signReviewTestContext } from './review-context.test-helper'; import { ReviewCompletionConflictError, ReviewValidationError, @@ -13,23 +16,470 @@ import { import { ReviewPersistenceError } from './postgres-review-repository'; const WORKSPACE_ID = '018f47b2-c1d2-4a30-8c17-221fb579c042'; +const OTHER_WORKSPACE_ID = '018f47b2-c1d2-4a30-8c17-221fb579c043'; +const SECRET = randomBytes(32).toString('base64url'); +const NOW_SECONDS = 1_786_334_400; +const HISTORY_BINDING = { + method: 'GET', + path: '/v1/reviews/completions', +} as const; +const DAILY_PLANNING_BINDING = { + method: 'POST', + path: '/v1/reviews/daily-planning/completions', +} as const; +const WEEKLY_REVIEW_BINDING = { + method: 'POST', + path: '/v1/reviews/weekly-review/completions', +} as const; +const BASE64URL_ALPHABET = + 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789-_'; function response(error: HttpException): unknown { return error.getResponse(); } +function signature( + issuedAt: string, + binding: { method: 'GET' | 'POST'; path: string } = HISTORY_BINDING, + workspaceId = WORKSPACE_ID, +): string { + return signReviewTestContext({ + secret: SECRET, + workspaceId, + issuedAt, + binding, + }); +} + +function expectTrustedContextRejection( + headers: { workspaceId: unknown; issuedAt: unknown; signature: unknown }, + secret: unknown, + requestBinding: ReviewTrustedRequestBinding, + nowSeconds: number, + status: number, + code: string, +): void { + const operation = () => + requireTrustedWorkspaceContext(headers, secret, requestBinding, nowSeconds); + expect(operation).toThrow(HttpException); + + let thrown: unknown; + try { + operation(); + } catch (error) { + thrown = error; + } + expect(thrown).toBeInstanceOf(HttpException); + expect(response(thrown as HttpException)).toMatchObject({ status, code }); +} + describe('Review HTTP boundary', () => { - it('accepts only bounded workspace, ritual, and history values', () => { - expect(requireWorkspaceHeader(WORKSPACE_ID.toUpperCase())).toBe( - WORKSPACE_ID, - ); + it('accepts fresh signed workspace context and bounded ritual/history values', () => { + const issuedAt = String(NOW_SECONDS - 30); + expect( + requireTrustedWorkspaceContext( + { + workspaceId: WORKSPACE_ID.toUpperCase(), + issuedAt, + signature: signature(issuedAt), + }, + SECRET, + HISTORY_BINDING, + NOW_SECONDS, + ), + ).toBe(WORKSPACE_ID); expect(requireRitualPath('weekly-review')).toBe('weekly-review'); expect(requireHistoryLimit(undefined)).toBe(50); expect(requireHistoryLimit('100')).toBe(100); }); + it('accepts a signed completion request binding', () => { + const issuedAt = String(NOW_SECONDS); + expect( + requireTrustedWorkspaceContext( + { + workspaceId: WORKSPACE_ID, + issuedAt, + signature: signature(issuedAt, DAILY_PLANNING_BINDING), + }, + SECRET, + DAILY_PLANNING_BINDING, + NOW_SECONDS, + ), + ).toBe(WORKSPACE_ID); + }); + + it('rejects replaying a history signature as a completion request', () => { + const issuedAt = String(NOW_SECONDS); + expectTrustedContextRejection( + { + workspaceId: WORKSPACE_ID, + issuedAt, + signature: signature(issuedAt, HISTORY_BINDING), + }, + SECRET, + DAILY_PLANNING_BINDING, + NOW_SECONDS, + 401, + 'invalid_gateway_context', + ); + }); + + it('rejects replaying one completion signature on another completion path', () => { + const issuedAt = String(NOW_SECONDS); + expectTrustedContextRejection( + { + workspaceId: WORKSPACE_ID, + issuedAt, + signature: signature(issuedAt, DAILY_PLANNING_BINDING), + }, + SECRET, + WEEKLY_REVIEW_BINDING, + NOW_SECONDS, + 401, + 'invalid_gateway_context', + ); + }); + + it('accepts the exact maximum context age', () => { + const issuedAt = String(NOW_SECONDS - 60); + expect( + requireTrustedWorkspaceContext( + { + workspaceId: WORKSPACE_ID, + issuedAt, + signature: signature(issuedAt), + }, + SECRET, + HISTORY_BINDING, + NOW_SECONDS, + ), + ).toBe(WORKSPACE_ID); + }); + + it('accepts the exact maximum future clock skew', () => { + const issuedAt = String(NOW_SECONDS + 5); + expect( + requireTrustedWorkspaceContext( + { + workspaceId: WORKSPACE_ID, + issuedAt, + signature: signature(issuedAt), + }, + SECRET, + HISTORY_BINDING, + NOW_SECONDS, + ), + ).toBe(WORKSPACE_ID); + }); + + it('rejects a non-canonical base64url alias for the same signature bytes', () => { + const issuedAt = String(NOW_SECONDS); + const canonical = signature(issuedAt); + const finalIndex = BASE64URL_ALPHABET.indexOf(canonical.at(-1) ?? ''); + expect(finalIndex).toBeGreaterThanOrEqual(0); + expect(finalIndex % 4).toBe(0); + const aliasCharacter = BASE64URL_ALPHABET[finalIndex + 1]; + expect(aliasCharacter).toBeDefined(); + const nonCanonical = `${canonical.slice(0, -1)}${aliasCharacter}`; + expect(Buffer.from(nonCanonical, 'base64url')).toEqual( + Buffer.from(canonical, 'base64url'), + ); + + expect(() => + requireTrustedWorkspaceContext( + { + workspaceId: WORKSPACE_ID, + issuedAt, + signature: nonCanonical, + }, + SECRET, + HISTORY_BINDING, + NOW_SECONDS, + ), + ).toThrow(HttpException); + }); + + it.each([ + { + name: 'stale timestamp', + headers: { + workspaceId: WORKSPACE_ID, + issuedAt: String(NOW_SECONDS - 61), + signature: signature(String(NOW_SECONDS - 61)), + }, + secret: SECRET, + binding: HISTORY_BINDING, + nowSeconds: NOW_SECONDS, + status: 401, + code: 'invalid_gateway_context', + }, + { + name: 'future timestamp', + headers: { + workspaceId: WORKSPACE_ID, + issuedAt: String(NOW_SECONDS + 6), + signature: signature(String(NOW_SECONDS + 6)), + }, + secret: SECRET, + binding: HISTORY_BINDING, + nowSeconds: NOW_SECONDS, + status: 401, + code: 'invalid_gateway_context', + }, + { + name: 'forged signature', + headers: { + workspaceId: WORKSPACE_ID, + issuedAt: String(NOW_SECONDS), + signature: 'A'.repeat(43), + }, + secret: SECRET, + binding: HISTORY_BINDING, + nowSeconds: NOW_SECONDS, + status: 401, + code: 'invalid_gateway_context', + }, + { + name: 'signature bound to another workspace', + headers: { + workspaceId: WORKSPACE_ID, + issuedAt: String(NOW_SECONDS), + signature: signature( + String(NOW_SECONDS), + HISTORY_BINDING, + OTHER_WORKSPACE_ID, + ), + }, + secret: SECRET, + binding: HISTORY_BINDING, + nowSeconds: NOW_SECONDS, + status: 401, + code: 'invalid_gateway_context', + }, + { + name: 'unsupported request method', + headers: { + workspaceId: WORKSPACE_ID, + issuedAt: String(NOW_SECONDS), + signature: signature(String(NOW_SECONDS)), + }, + secret: SECRET, + binding: { method: 'DELETE', path: '/v1/reviews/completions' }, + nowSeconds: NOW_SECONDS, + status: 401, + code: 'invalid_gateway_context', + }, + { + name: 'unsupported request path', + headers: { + workspaceId: WORKSPACE_ID, + issuedAt: String(NOW_SECONDS), + signature: signature(String(NOW_SECONDS)), + }, + secret: SECRET, + binding: { method: 'POST', path: '/v1/reviews/completions' }, + nowSeconds: NOW_SECONDS, + status: 401, + code: 'invalid_gateway_context', + }, + { + name: 'short verifier secret', + headers: { + workspaceId: WORKSPACE_ID, + issuedAt: String(NOW_SECONDS), + signature: signature(String(NOW_SECONDS)), + }, + secret: 'too-short', + binding: HISTORY_BINDING, + nowSeconds: NOW_SECONDS, + status: 503, + code: 'gateway_context_unavailable', + }, + { + name: 'missing verifier secret', + headers: { + workspaceId: WORKSPACE_ID, + issuedAt: String(NOW_SECONDS), + signature: signature(String(NOW_SECONDS)), + }, + secret: undefined, + binding: HISTORY_BINDING, + nowSeconds: NOW_SECONDS, + status: 503, + code: 'gateway_context_unavailable', + }, + { + name: 'missing workspace header', + headers: { + workspaceId: undefined, + issuedAt: String(NOW_SECONDS), + signature: signature(String(NOW_SECONDS)), + }, + secret: SECRET, + binding: HISTORY_BINDING, + nowSeconds: NOW_SECONDS, + status: 401, + code: 'invalid_gateway_context', + }, + { + name: 'non-string workspace header', + headers: { + workspaceId: 123, + issuedAt: String(NOW_SECONDS), + signature: signature(String(NOW_SECONDS)), + }, + secret: SECRET, + binding: HISTORY_BINDING, + nowSeconds: NOW_SECONDS, + status: 401, + code: 'invalid_gateway_context', + }, + { + name: 'invalid workspace UUID', + headers: { + workspaceId: 'not-a-uuid', + issuedAt: String(NOW_SECONDS), + signature: signature(String(NOW_SECONDS)), + }, + secret: SECRET, + binding: HISTORY_BINDING, + nowSeconds: NOW_SECONDS, + status: 401, + code: 'invalid_gateway_context', + }, + { + name: 'missing issued-at header', + headers: { + workspaceId: WORKSPACE_ID, + issuedAt: undefined, + signature: signature(String(NOW_SECONDS)), + }, + secret: SECRET, + binding: HISTORY_BINDING, + nowSeconds: NOW_SECONDS, + status: 401, + code: 'invalid_gateway_context', + }, + { + name: 'non-string issued-at header', + headers: { + workspaceId: WORKSPACE_ID, + issuedAt: 123, + signature: signature(String(NOW_SECONDS)), + }, + secret: SECRET, + binding: HISTORY_BINDING, + nowSeconds: NOW_SECONDS, + status: 401, + code: 'invalid_gateway_context', + }, + { + name: 'nonnumeric issued-at header', + headers: { + workspaceId: WORKSPACE_ID, + issuedAt: 'not-a-timestamp', + signature: signature(String(NOW_SECONDS)), + }, + secret: SECRET, + binding: HISTORY_BINDING, + nowSeconds: NOW_SECONDS, + status: 401, + code: 'invalid_gateway_context', + }, + { + name: 'missing signature header', + headers: { + workspaceId: WORKSPACE_ID, + issuedAt: String(NOW_SECONDS), + signature: undefined, + }, + secret: SECRET, + binding: HISTORY_BINDING, + nowSeconds: NOW_SECONDS, + status: 401, + code: 'invalid_gateway_context', + }, + { + name: 'non-string signature header', + headers: { + workspaceId: WORKSPACE_ID, + issuedAt: String(NOW_SECONDS), + signature: 123, + }, + secret: SECRET, + binding: HISTORY_BINDING, + nowSeconds: NOW_SECONDS, + status: 401, + code: 'invalid_gateway_context', + }, + { + name: 'wrong-length signature', + headers: { + workspaceId: WORKSPACE_ID, + issuedAt: String(NOW_SECONDS), + signature: 'A'.repeat(42), + }, + secret: SECRET, + binding: HISTORY_BINDING, + nowSeconds: NOW_SECONDS, + status: 401, + code: 'invalid_gateway_context', + }, + { + name: 'invalid base64url signature characters', + headers: { + workspaceId: WORKSPACE_ID, + issuedAt: String(NOW_SECONDS), + signature: '!'.repeat(43), + }, + secret: SECRET, + binding: HISTORY_BINDING, + nowSeconds: NOW_SECONDS, + status: 401, + code: 'invalid_gateway_context', + }, + { + name: 'non-integer verifier clock', + headers: { + workspaceId: WORKSPACE_ID, + issuedAt: String(NOW_SECONDS), + signature: signature(String(NOW_SECONDS)), + }, + secret: SECRET, + binding: HISTORY_BINDING, + nowSeconds: Number.NaN, + status: 503, + code: 'gateway_context_unavailable', + }, + { + name: 'negative verifier clock', + headers: { + workspaceId: WORKSPACE_ID, + issuedAt: String(NOW_SECONDS), + signature: signature(String(NOW_SECONDS)), + }, + secret: SECRET, + binding: HISTORY_BINDING, + nowSeconds: -1, + status: 503, + code: 'gateway_context_unavailable', + }, + ])( + 'fails closed for $name', + ({ headers, secret, binding, nowSeconds, status, code }) => { + expectTrustedContextRejection( + headers, + secret, + binding, + nowSeconds, + status, + code, + ); + }, + ); + it.each([ - () => requireWorkspaceHeader('not-a-workspace'), () => requireRitualPath('execute'), () => requireHistoryLimit('101'), ])('returns bounded problems for invalid boundary input', (operation) => { diff --git a/apps/review-service/src/http-boundary.ts b/apps/review-service/src/http-boundary.ts index 51a0c894..c93c0962 100644 --- a/apps/review-service/src/http-boundary.ts +++ b/apps/review-service/src/http-boundary.ts @@ -1,3 +1,4 @@ +import { createHmac, timingSafeEqual } from 'node:crypto'; import { HttpException } from '@nestjs/common'; import { ReviewCompletionConflictError, @@ -17,6 +18,31 @@ export interface ReviewProblemDetails { code: string; } +/** Signed tenant authority emitted by the authenticated gateway boundary. */ +export interface ReviewTrustedWorkspaceContextHeaders { + workspaceId: unknown; + issuedAt: unknown; + signature: unknown; +} + +/** Server-owned request identity bound into each Review gateway context. */ +export interface ReviewTrustedRequestBinding { + method: unknown; + path: unknown; +} + +const UNIX_SECONDS_PATTERN = /^(?:0|[1-9]\d{0,12})$/u; +const BASE64URL_SHA256_PATTERN = /^[A-Za-z0-9_-]{43}$/u; +const MINIMUM_GATEWAY_SECRET_BYTES = 32; +const MAXIMUM_CONTEXT_AGE_SECONDS = 60; +const MAXIMUM_FUTURE_SKEW_SECONDS = 5; +const REVIEW_HISTORY_PATH = '/v1/reviews/completions'; +const REVIEW_COMPLETION_PATHS = new Set([ + '/v1/reviews/daily-planning/completions', + '/v1/reviews/daily-shutdown/completions', + '/v1/reviews/weekly-review/completions', +]); + function problemException( status: number, title: string, @@ -31,17 +57,133 @@ function problemException( return new HttpException(problem, status); } -/** Requires a tenant UUIDv4 exclusively from the trusted workspace header. */ -export function requireWorkspaceHeader(value: string | undefined): string { +/** Rejects malformed, forged, stale, or future trusted context with a credential-free 401 problem. */ +function invalidGatewayContext(): never { + throw problemException( + 401, + 'Trusted gateway context is invalid', + 'invalid_gateway_context', + ); +} + +/** Reports verifier configuration that cannot authenticate context as a bounded 503 problem. */ +function unavailableGatewayContext(): never { + throw problemException( + 503, + 'Trusted gateway context is unavailable', + 'gateway_context_unavailable', + ); +} + +/** + * Requires verifier key material that is long enough to authenticate gateway + * workspace assertions. The returned value is safe to pass to the HMAC verifier. + */ +export function requireReviewGatewayContextSecret(secret: unknown): string { + if ( + typeof secret !== 'string' || + Buffer.byteLength(secret, 'utf8') < MINIMUM_GATEWAY_SECRET_BYTES + ) { + return unavailableGatewayContext(); + } + return secret; +} + +/** Accepts only the exact method/path combinations exposed by Review. */ +function requireReviewRequestBinding(binding: ReviewTrustedRequestBinding): { + method: 'GET' | 'POST'; + path: string; +} { + if (binding.method === 'GET' && binding.path === REVIEW_HISTORY_PATH) { + return { method: 'GET', path: REVIEW_HISTORY_PATH }; + } + if ( + binding.method === 'POST' && + typeof binding.path === 'string' && + REVIEW_COMPLETION_PATHS.has(binding.path) + ) { + return { method: 'POST', path: binding.path }; + } + return invalidGatewayContext(); +} + +/** + * Computes the SHA-256 HMAC over the Review-specific request-bound context. + * The workspace ID must already be normalized to lowercase UUIDv4 form, so the + * gateway signs that normalized identifier plus the exact HTTP method and path. + */ +function workspaceContextDigest( + workspaceId: string, + issuedAt: string, + method: 'GET' | 'POST', + path: string, + secret: string, +): Buffer { + return createHmac('sha256', secret) + .update( + `life-os.review-context.v1\n${workspaceId}\n${issuedAt}\n${method}\n${path}`, + 'utf8', + ) + .digest(); +} + +/** + * Verifies the short-lived, method-and-path-bound workspace context created + * after gateway authentication. A browser-selected workspace header is never + * an authorization input, and a history context cannot be replayed as a write. + */ +export function requireTrustedWorkspaceContext( + headers: ReviewTrustedWorkspaceContextHeaders, + secret: unknown, + requestBinding: ReviewTrustedRequestBinding, + nowSeconds = Math.floor(Date.now() / 1000), +): string { + const verifiedSecret = requireReviewGatewayContextSecret(secret); + if (!Number.isSafeInteger(nowSeconds) || nowSeconds < 0) { + return unavailableGatewayContext(); + } + const { method, path } = requireReviewRequestBinding(requestBinding); + if ( + typeof headers.workspaceId !== 'string' || + typeof headers.issuedAt !== 'string' || + typeof headers.signature !== 'string' || + !UNIX_SECONDS_PATTERN.test(headers.issuedAt) || + !BASE64URL_SHA256_PATTERN.test(headers.signature) + ) { + return invalidGatewayContext(); + } + + let workspaceId: string; try { - return requireReviewWorkspaceId(value); + workspaceId = requireReviewWorkspaceId(headers.workspaceId); } catch { - throw problemException( - 400, - 'A valid x-workspace-id header is required', - 'invalid_workspace', - ); + return invalidGatewayContext(); + } + + const issuedAtSeconds = Number(headers.issuedAt); + if ( + issuedAtSeconds > nowSeconds + MAXIMUM_FUTURE_SKEW_SECONDS || + issuedAtSeconds < nowSeconds - MAXIMUM_CONTEXT_AGE_SECONDS + ) { + return invalidGatewayContext(); + } + + const expected = workspaceContextDigest( + workspaceId, + headers.issuedAt, + method, + path, + verifiedSecret, + ); + const actual = Buffer.from(headers.signature, 'base64url'); + if ( + actual.length !== expected.length || + actual.toString('base64url') !== headers.signature || + !timingSafeEqual(actual, expected) + ) { + return invalidGatewayContext(); } + return workspaceId; } /** Requires a supported ritual kind from the bounded route parameter. */ diff --git a/apps/review-service/src/main.test.ts b/apps/review-service/src/main.test.ts index 3f5cee32..c01e2a7b 100644 --- a/apps/review-service/src/main.test.ts +++ b/apps/review-service/src/main.test.ts @@ -1,6 +1,8 @@ +import { randomBytes } from 'node:crypto'; import { HttpException } from '@nestjs/common'; -import { describe, expect, it } from 'vitest'; -import { ReviewController } from './main'; +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; +import { requireReviewServiceConfiguration, ReviewController } from './main'; +import { signReviewTestContext } from './review-context.test-helper'; import { ReviewService, type ReviewCompletionRecord, @@ -10,6 +12,8 @@ import { const WORKSPACE_ID = '018f47b2-c1d2-4a30-8c17-221fb579c042'; const IDEMPOTENCY_KEY = 'd1191b96-b7f4-4d8f-b1f7-9e2838686d5f'; const COMPLETION_ID = '3f044b68-c515-4a52-8862-38af0047b88d'; +const GATEWAY_SECRET = randomBytes(32).toString('base64url'); +let previousGatewaySecret: string | undefined; class InMemoryReviewRepository implements ReviewRepository { readonly records: ReviewCompletionRecord[] = []; @@ -39,7 +43,51 @@ function body() { }; } +function trustedContext( + method: 'GET' | 'POST', + path: string, + workspaceId = WORKSPACE_ID, +): readonly [string, string] { + const issuedAt = String(Math.floor(Date.now() / 1000)); + const signature = signReviewTestContext({ + secret: GATEWAY_SECRET, + workspaceId, + issuedAt, + binding: { method, path }, + }); + return [issuedAt, signature] as const; +} + +function expectGatewayContextUnavailable(operation: () => unknown): void { + let thrown: unknown; + try { + operation(); + } catch (error) { + thrown = error; + } + expect(thrown).toBeInstanceOf(HttpException); + const exception = thrown as HttpException; + expect(exception.getStatus()).toBe(503); + expect(exception.getResponse()).toMatchObject({ + status: 503, + code: 'gateway_context_unavailable', + }); +} + describe('Review controller', () => { + beforeEach(() => { + previousGatewaySecret = process.env.REVIEW_GATEWAY_CONTEXT_SECRET; + process.env.REVIEW_GATEWAY_CONTEXT_SECRET = GATEWAY_SECRET; + }); + + afterEach(() => { + if (previousGatewaySecret === undefined) { + delete process.env.REVIEW_GATEWAY_CONTEXT_SECRET; + } else { + process.env.REVIEW_GATEWAY_CONTEXT_SECRET = previousGatewaySecret; + } + }); + it('exposes health and all three guided completion routes', async () => { const repository = new InMemoryReviewRepository(); const service = new ReviewService( @@ -53,26 +101,96 @@ describe('Review controller', () => { status: 'ok', service: 'review-service', }); + + const [dailyPlanningIssuedAt, dailyPlanningSignature] = trustedContext( + 'POST', + '/v1/reviews/daily-planning/completions', + ); await expect( - controller.completeDailyPlanning(WORKSPACE_ID, body()), + controller.completeDailyPlanning( + WORKSPACE_ID, + dailyPlanningIssuedAt, + dailyPlanningSignature, + body(), + ), ).resolves.toMatchObject({ ritualKind: 'daily-planning' }); + + const [dailyShutdownIssuedAt, dailyShutdownSignature] = trustedContext( + 'POST', + '/v1/reviews/daily-shutdown/completions', + ); await expect( - controller.completeDailyShutdown(WORKSPACE_ID, body()), + controller.completeDailyShutdown( + WORKSPACE_ID, + dailyShutdownIssuedAt, + dailyShutdownSignature, + body(), + ), ).resolves.toMatchObject({ ritualKind: 'daily-shutdown' }); + + const [weeklyReviewIssuedAt, weeklyReviewSignature] = trustedContext( + 'POST', + '/v1/reviews/weekly-review/completions', + ); await expect( - controller.completeWeeklyReview(WORKSPACE_ID, body()), + controller.completeWeeklyReview( + WORKSPACE_ID, + weeklyReviewIssuedAt, + weeklyReviewSignature, + body(), + ), ).resolves.toMatchObject({ ritualKind: 'weekly-review' }); + + const [historyIssuedAt, historySignature] = trustedContext( + 'GET', + '/v1/reviews/completions', + ); await expect( - controller.listCompletions(WORKSPACE_ID, '10'), + controller.listCompletions( + WORKSPACE_ID, + historyIssuedAt, + historySignature, + '10', + ), ).resolves.toHaveLength(3); }); + it('keeps startup and readiness fail-closed for unsafe gateway secrets', () => { + const controller = new ReviewController( + new ReviewService(new InMemoryReviewRepository()), + ); + + delete process.env.REVIEW_GATEWAY_CONTEXT_SECRET; + expectGatewayContextUnavailable(() => + requireReviewServiceConfiguration(process.env), + ); + expectGatewayContextUnavailable(() => controller.ready()); + + process.env.REVIEW_GATEWAY_CONTEXT_SECRET = 'too-short'; + expectGatewayContextUnavailable(() => + requireReviewServiceConfiguration(process.env), + ); + expectGatewayContextUnavailable(() => controller.ready()); + + process.env.REVIEW_GATEWAY_CONTEXT_SECRET = GATEWAY_SECRET; + expect(() => requireReviewServiceConfiguration(process.env)).not.toThrow(); + expect(controller.ready()).toEqual({ + status: 'ready', + service: 'review-service', + }); + }); + it('fails closed before the domain for invalid workspace ownership', async () => { const controller = new ReviewController( new ReviewService(new InMemoryReviewRepository()), ); + const [issuedAt, signature] = trustedContext( + 'POST', + '/v1/reviews/daily-planning/completions', + 'invalid', + ); await expect( - controller.completeDailyPlanning('invalid', body()), + controller.completeDailyPlanning('invalid', issuedAt, signature, body()), ).rejects.toBeInstanceOf(HttpException); }); }); diff --git a/apps/review-service/src/main.ts b/apps/review-service/src/main.ts index 421273f0..dd87b31f 100644 --- a/apps/review-service/src/main.ts +++ b/apps/review-service/src/main.ts @@ -12,8 +12,10 @@ import { import { NestFactory } from '@nestjs/core'; import { requireHistoryLimit, - requireWorkspaceHeader, + requireReviewGatewayContextSecret, + requireTrustedWorkspaceContext, toReviewHttpException, + type ReviewTrustedRequestBinding, } from './http-boundary'; import { type ReviewCompletionRecord, @@ -41,42 +43,84 @@ export class ReviewController { return { status: 'ok', service: 'review-service' }; } + /** Returns readiness only when signed workspace authority can be verified. */ + @Get('ready') + ready(): { status: 'ready'; service: 'review-service' } { + requireReviewGatewayContextSecret( + process.env.REVIEW_GATEWAY_CONTEXT_SECRET, + ); + return { status: 'ready', service: 'review-service' }; + } + /** Records a completed daily planning ritual. */ @Post('reviews/daily-planning/completions') async completeDailyPlanning( - @Headers('x-workspace-id') workspaceHeader: string | undefined, + @Headers('x-life-os-workspace-id') workspaceId: string | undefined, + @Headers('x-life-os-context-issued-at') issuedAt: string | undefined, + @Headers('x-life-os-context-signature') signature: string | undefined, @Body() body: unknown, ): Promise { - return await this.complete(workspaceHeader, 'daily-planning', body); + const trustedWorkspaceId = this.trustedWorkspaceId( + { workspaceId, issuedAt, signature }, + { + method: 'POST', + path: '/v1/reviews/daily-planning/completions', + }, + ); + return await this.complete(trustedWorkspaceId, 'daily-planning', body); } /** Records a completed daily shutdown ritual. */ @Post('reviews/daily-shutdown/completions') async completeDailyShutdown( - @Headers('x-workspace-id') workspaceHeader: string | undefined, + @Headers('x-life-os-workspace-id') workspaceId: string | undefined, + @Headers('x-life-os-context-issued-at') issuedAt: string | undefined, + @Headers('x-life-os-context-signature') signature: string | undefined, @Body() body: unknown, ): Promise { - return await this.complete(workspaceHeader, 'daily-shutdown', body); + const trustedWorkspaceId = this.trustedWorkspaceId( + { workspaceId, issuedAt, signature }, + { + method: 'POST', + path: '/v1/reviews/daily-shutdown/completions', + }, + ); + return await this.complete(trustedWorkspaceId, 'daily-shutdown', body); } /** Records a completed Monday-anchored weekly review ritual. */ @Post('reviews/weekly-review/completions') async completeWeeklyReview( - @Headers('x-workspace-id') workspaceHeader: string | undefined, + @Headers('x-life-os-workspace-id') workspaceId: string | undefined, + @Headers('x-life-os-context-issued-at') issuedAt: string | undefined, + @Headers('x-life-os-context-signature') signature: string | undefined, @Body() body: unknown, ): Promise { - return await this.complete(workspaceHeader, 'weekly-review', body); + const trustedWorkspaceId = this.trustedWorkspaceId( + { workspaceId, issuedAt, signature }, + { + method: 'POST', + path: '/v1/reviews/weekly-review/completions', + }, + ); + return await this.complete(trustedWorkspaceId, 'weekly-review', body); } /** Lists deterministic immutable completion history for one workspace. */ @Get('reviews/completions') async listCompletions( - @Headers('x-workspace-id') workspaceHeader: string | undefined, + @Headers('x-life-os-workspace-id') workspaceId: string | undefined, + @Headers('x-life-os-context-issued-at') issuedAt: string | undefined, + @Headers('x-life-os-context-signature') signature: string | undefined, @Query('limit') limit: string | undefined, ): Promise { try { + const trustedWorkspaceId = this.trustedWorkspaceId( + { workspaceId, issuedAt, signature }, + { method: 'GET', path: '/v1/reviews/completions' }, + ); return await this.reviewService.list( - requireWorkspaceHeader(workspaceHeader), + trustedWorkspaceId, requireHistoryLimit(limit), ); } catch (error) { @@ -84,17 +128,30 @@ export class ReviewController { } } + /** Returns workspace authority only when the signed context matches the exact Review route. */ + private trustedWorkspaceId( + headers: { + workspaceId: string | undefined; + issuedAt: string | undefined; + signature: string | undefined; + }, + requestBinding: ReviewTrustedRequestBinding, + ): string { + return requireTrustedWorkspaceContext( + headers, + process.env.REVIEW_GATEWAY_CONTEXT_SECRET, + requestBinding, + ); + } + + /** Records one ritual only for a workspace ID already accepted by the trusted-context verifier. */ private async complete( - workspaceHeader: string | undefined, + workspaceId: string, ritualKind: ReviewRitualKind, body: unknown, ): Promise { try { - return await this.reviewService.complete( - requireWorkspaceHeader(workspaceHeader), - ritualKind, - body, - ); + return await this.reviewService.complete(workspaceId, ritualKind, body); } catch (error) { throw toReviewHttpException(error); } @@ -118,8 +175,16 @@ export class ReviewController { }) export class AppModule {} +/** Verifies required security configuration before Review accepts traffic. */ +export function requireReviewServiceConfiguration( + env: NodeJS.ProcessEnv, +): void { + requireReviewGatewayContextSecret(env.REVIEW_GATEWAY_CONTEXT_SECRET); +} + /** Boots the versioned review service on its configured public port. */ async function bootstrap(): Promise { + requireReviewServiceConfiguration(process.env); const app = await NestFactory.create(AppModule); app.setGlobalPrefix('v1'); app.enableShutdownHooks(); diff --git a/apps/review-service/src/review-context.test-helper.ts b/apps/review-service/src/review-context.test-helper.ts new file mode 100644 index 00000000..43b6e940 --- /dev/null +++ b/apps/review-service/src/review-context.test-helper.ts @@ -0,0 +1,29 @@ +import { createHmac } from 'node:crypto'; + +/** HTTP request identity included in a signed Review gateway context. */ +export interface ReviewContextTestBinding { + readonly method: 'GET' | 'POST'; + readonly path: string; +} + +/** + * Produces the canonical Review gateway-context HMAC used by boundary tests. + * + * Test callers provide the same normalized workspace, issue time, method, and + * path that production verification binds. Centralizing this payload prevents + * tests from silently diverging when the request-bound authority contract + * changes. + */ +export function signReviewTestContext(input: { + readonly secret: string; + readonly workspaceId: string; + readonly issuedAt: string; + readonly binding: ReviewContextTestBinding; +}): string { + return createHmac('sha256', input.secret) + .update( + `life-os.review-context.v1\n${input.workspaceId.toLowerCase()}\n${input.issuedAt}\n${input.binding.method}\n${input.binding.path}`, + 'utf8', + ) + .digest('base64url'); +} diff --git a/apps/review-service/src/review-controller-authority.test.ts b/apps/review-service/src/review-controller-authority.test.ts new file mode 100644 index 00000000..c0ce1a58 --- /dev/null +++ b/apps/review-service/src/review-controller-authority.test.ts @@ -0,0 +1,243 @@ +import { randomBytes } from 'node:crypto'; +import { HttpException } from '@nestjs/common'; +import { afterEach, describe, expect, it, vi } from 'vitest'; +import { ReviewController } from './main'; +import { signReviewTestContext } from './review-context.test-helper'; +import type { ReviewService } from './review-domain'; + +const WORKSPACE_ID = '11111111-1111-4111-8111-111111111111'; +const CONTEXT_SECRET = randomBytes(32).toString('base64url'); + +interface RouteHeaders { + readonly workspaceId: string | undefined; + readonly issuedAt: string | undefined; + readonly signature: string | undefined; +} + +interface ReviewServiceSpies { + readonly complete: ReturnType; + readonly list: ReturnType; +} + +interface RouteCase { + readonly name: string; + readonly serviceMethod: keyof ReviewServiceSpies; + readonly method: 'GET' | 'POST'; + readonly path: string; + readonly invoke: ( + controller: ReviewController, + headers: RouteHeaders, + ) => Promise; +} + +const ROUTES: readonly RouteCase[] = [ + { + name: 'completeDailyPlanning', + serviceMethod: 'complete', + method: 'POST', + path: '/v1/reviews/daily-planning/completions', + invoke: (controller, headers) => + controller.completeDailyPlanning( + headers.workspaceId, + headers.issuedAt, + headers.signature, + {}, + ), + }, + { + name: 'completeDailyShutdown', + serviceMethod: 'complete', + method: 'POST', + path: '/v1/reviews/daily-shutdown/completions', + invoke: (controller, headers) => + controller.completeDailyShutdown( + headers.workspaceId, + headers.issuedAt, + headers.signature, + {}, + ), + }, + { + name: 'completeWeeklyReview', + serviceMethod: 'complete', + method: 'POST', + path: '/v1/reviews/weekly-review/completions', + invoke: (controller, headers) => + controller.completeWeeklyReview( + headers.workspaceId, + headers.issuedAt, + headers.signature, + {}, + ), + }, + { + name: 'listCompletions', + serviceMethod: 'list', + method: 'GET', + path: '/v1/reviews/completions', + invoke: (controller, headers) => + controller.listCompletions( + headers.workspaceId, + headers.issuedAt, + headers.signature, + '10', + ), + }, +]; + +/** Creates observable Review service boundaries without persistence. */ +function serviceSpies(): ReviewServiceSpies { + return { + complete: vi.fn().mockResolvedValue({}), + list: vi.fn().mockResolvedValue([]), + }; +} + +/** Creates a controller whose domain calls can prove tenant authority flow. */ +function controllerWith(service: ReviewServiceSpies): ReviewController { + return new ReviewController(service as unknown as ReviewService); +} + +/** Produces a request-bound HMAC gateway assertion at one issue time. */ +function signedHeaders( + route: RouteCase, + issuedAtSeconds: number, +): RouteHeaders { + const issuedAt = String(issuedAtSeconds); + const signature = signReviewTestContext({ + secret: CONTEXT_SECRET, + workspaceId: WORKSPACE_ID, + issuedAt, + binding: { method: route.method, path: route.path }, + }); + return { workspaceId: WORKSPACE_ID, issuedAt, signature }; +} + +/** Returns the bounded HTTP status from an expected rejected route call. */ +async function rejectedStatus(operation: Promise): Promise { + try { + await operation; + } catch (error) { + expect(error).toBeInstanceOf(HttpException); + return (error as HttpException).getStatus(); + } + throw new Error( + 'Expected Review route to reject untrusted workspace context', + ); +} + +afterEach(() => { + delete process.env.REVIEW_GATEWAY_CONTEXT_SECRET; + vi.restoreAllMocks(); +}); + +describe.sequential('Review controller tenant authority contract', () => { + it('rejects a browser-selected workspace header without signed context', async () => { + process.env.REVIEW_GATEWAY_CONTEXT_SECRET = CONTEXT_SECRET; + const service = serviceSpies(); + const controller = controllerWith(service); + const headers: RouteHeaders = { + workspaceId: WORKSPACE_ID, + issuedAt: undefined, + signature: undefined, + }; + + for (const route of ROUTES) { + vi.clearAllMocks(); + expect(await rejectedStatus(route.invoke(controller, headers))).toBe(401); + expect(service[route.serviceMethod], route.name).not.toHaveBeenCalled(); + } + }); + + it('passes only the verified workspace to every Review domain route', async () => { + process.env.REVIEW_GATEWAY_CONTEXT_SECRET = CONTEXT_SECRET; + const nowSeconds = Math.floor(Date.now() / 1000); + const service = serviceSpies(); + const controller = controllerWith(service); + + for (const route of ROUTES) { + vi.clearAllMocks(); + await route.invoke(controller, signedHeaders(route, nowSeconds)); + expect(service[route.serviceMethod], route.name).toHaveBeenCalledTimes(1); + expect(service[route.serviceMethod].mock.calls[0]?.[0], route.name).toBe( + WORKSPACE_ID, + ); + } + }); + + it('rejects untrusted contexts before every Review domain call', async () => { + const nowSeconds = Math.floor(Date.now() / 1000); + const service = serviceSpies(); + const controller = controllerWith(service); + + for (const route of ROUTES) { + const fresh = signedHeaders(route, nowSeconds); + const expired = signedHeaders(route, nowSeconds - 120); + const future = signedHeaders(route, nowSeconds + 120); + const tamperedDigest = Buffer.from(fresh.signature ?? '', 'base64url'); + const firstTamperedByte = tamperedDigest.at(0); + if (firstTamperedByte === undefined) { + throw new Error('Expected a SHA-256 gateway signature'); + } + tamperedDigest[0] = firstTamperedByte ^ 0xff; + const invalidContexts = [ + { + name: 'missing', + headers: { ...fresh, workspaceId: undefined }, + status: 401, + secretConfigured: true, + }, + { + name: 'expired', + headers: expired, + status: 401, + secretConfigured: true, + }, + { + name: 'future', + headers: future, + status: 401, + secretConfigured: true, + }, + { + name: 'tampered', + headers: { + ...fresh, + signature: tamperedDigest.toString('base64url'), + }, + status: 401, + secretConfigured: true, + }, + { + name: 'malformed', + headers: { ...fresh, workspaceId: 'not-a-uuid' }, + status: 401, + secretConfigured: true, + }, + { + name: 'secret-unconfigured', + headers: fresh, + status: 503, + secretConfigured: false, + }, + ] as const; + + for (const invalid of invalidContexts) { + vi.clearAllMocks(); + if (invalid.secretConfigured) { + process.env.REVIEW_GATEWAY_CONTEXT_SECRET = CONTEXT_SECRET; + } else { + delete process.env.REVIEW_GATEWAY_CONTEXT_SECRET; + } + expect( + await rejectedStatus(route.invoke(controller, invalid.headers)), + `${route.name}:${invalid.name}`, + ).toBe(invalid.status); + expect( + service[route.serviceMethod], + `${route.name}:${invalid.name}`, + ).not.toHaveBeenCalled(); + } + } + }); +});