diff --git a/CHANGELOG.md b/CHANGELOG.md index 05e74d1a..a4c5a013 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,3 +12,4 @@ | 2026-02-24 | 0.1.7 | Added `/health` endpoints and switched smoke checks to `/health` for Cloud Run compatibility. | | 2026-02-24 | 0.1.8 | Enabled dev frontend reachability and made deploy auth mode environment-aware (`dev` public, `staging` private). | | 2026-02-24 | 0.1.9 | Switched core API from mock behavior to real GCS upload/signed URLs and real Vertex model calls in dev deployment. | +| 2026-02-24 | 0.1.10 | Hardened core APIs with signed URL ownership/expiry checks, object existence checks, and per-user LLM rate limiting. | diff --git a/backend/core-api/src/middleware/error-handler.js b/backend/core-api/src/middleware/error-handler.js index 289395f3..2a08b112 100644 --- a/backend/core-api/src/middleware/error-handler.js +++ b/backend/core-api/src/middleware/error-handler.js @@ -11,6 +11,9 @@ export function notFoundHandler(req, res) { export function errorHandler(error, req, res, _next) { const envelope = toErrorEnvelope(error, req.requestId); + if (envelope.status === 429 && envelope.body.details?.retryAfterSeconds) { + res.set('Retry-After', String(envelope.body.details.retryAfterSeconds)); + } if (req.log) { req.log.error( { diff --git a/backend/core-api/src/routes/core.js b/backend/core-api/src/routes/core.js index a753ae22..d73b0170 100644 --- a/backend/core-api/src/routes/core.js +++ b/backend/core-api/src/routes/core.js @@ -6,9 +6,11 @@ import { requireAuth, requirePolicy } from '../middleware/auth.js'; import { createSignedUrlSchema } from '../contracts/core/create-signed-url.js'; import { invokeLlmSchema } from '../contracts/core/invoke-llm.js'; import { invokeVertexModel } from '../services/llm.js'; -import { generateReadSignedUrl, uploadToGcs } from '../services/storage.js'; +import { checkLlmRateLimit } from '../services/llm-rate-limit.js'; +import { generateReadSignedUrl, uploadToGcs, validateFileUriAccess } from '../services/storage.js'; const DEFAULT_MAX_FILE_BYTES = 10 * 1024 * 1024; +const DEFAULT_MAX_SIGNED_URL_SECONDS = 900; const ALLOWED_FILE_TYPES = new Set(['application/pdf', 'image/jpeg', 'image/jpg', 'image/png']); const upload = multer({ @@ -105,10 +107,25 @@ async function handleCreateSignedUrl(req, res, next) { try { const payload = parseBody(createSignedUrlSchema, req.body || {}); const expiresInSeconds = payload.expiresInSeconds || 300; + const maxSignedUrlSeconds = Number.parseInt( + process.env.MAX_SIGNED_URL_SECONDS || `${DEFAULT_MAX_SIGNED_URL_SECONDS}`, + 10 + ); + if (expiresInSeconds > maxSignedUrlSeconds) { + throw new AppError('VALIDATION_ERROR', `expiresInSeconds must be <= ${maxSignedUrlSeconds}`, 400); + } + const signed = useMockSignedUrl() - ? mockSignedUrl(payload.fileUri, expiresInSeconds) + ? (() => { + validateFileUriAccess({ + fileUri: payload.fileUri, + actorUid: req.actor.uid, + }); + return mockSignedUrl(payload.fileUri, expiresInSeconds); + })() : await generateReadSignedUrl({ fileUri: payload.fileUri, + actorUid: req.actor.uid, expiresInSeconds, }); @@ -124,6 +141,12 @@ async function handleCreateSignedUrl(req, res, next) { async function handleInvokeLlm(req, res, next) { try { const payload = parseBody(invokeLlmSchema, req.body || {}); + const rateLimit = checkLlmRateLimit({ uid: req.actor.uid }); + if (!rateLimit.allowed) { + throw new AppError('RATE_LIMITED', 'Too many model requests. Please retry shortly.', 429, { + retryAfterSeconds: rateLimit.retryAfterSeconds, + }); + } const startedAt = Date.now(); if (process.env.LLM_MOCK === 'false') { diff --git a/backend/core-api/src/services/llm-rate-limit.js b/backend/core-api/src/services/llm-rate-limit.js new file mode 100644 index 00000000..2e63f330 --- /dev/null +++ b/backend/core-api/src/services/llm-rate-limit.js @@ -0,0 +1,41 @@ +const counters = new Map(); + +function currentWindowKey(now = Date.now()) { + return Math.floor(now / 60000); +} + +function perMinuteLimit() { + return Number.parseInt(process.env.LLM_RATE_LIMIT_PER_MINUTE || '20', 10); +} + +export function checkLlmRateLimit({ uid, now = Date.now() }) { + const limit = perMinuteLimit(); + if (!Number.isFinite(limit) || limit <= 0) { + return { allowed: true, remaining: null, retryAfterSeconds: 0 }; + } + + const windowKey = currentWindowKey(now); + const record = counters.get(uid); + + if (!record || record.windowKey !== windowKey) { + counters.set(uid, { windowKey, count: 1 }); + return { allowed: true, remaining: limit - 1, retryAfterSeconds: 0 }; + } + + if (record.count >= limit) { + const retryAfterSeconds = (windowKey + 1) * 60 - Math.floor(now / 1000); + return { + allowed: false, + remaining: 0, + retryAfterSeconds: Math.max(1, retryAfterSeconds), + }; + } + + record.count += 1; + counters.set(uid, record); + return { allowed: true, remaining: limit - record.count, retryAfterSeconds: 0 }; +} + +export function __resetLlmRateLimitForTests() { + counters.clear(); +} diff --git a/backend/core-api/src/services/storage.js b/backend/core-api/src/services/storage.js index 4e4b2f5c..da0dd382 100644 --- a/backend/core-api/src/services/storage.js +++ b/backend/core-api/src/services/storage.js @@ -3,7 +3,7 @@ import { AppError } from '../lib/errors.js'; const storage = new Storage(); -function parseGsUri(fileUri) { +export function parseGsUri(fileUri) { if (!fileUri.startsWith('gs://')) { throw new AppError('VALIDATION_ERROR', 'fileUri must start with gs://', 400); } @@ -27,6 +27,20 @@ function allowedBuckets() { ]); } +export function validateFileUriAccess({ fileUri, actorUid }) { + const { bucket, path } = parseGsUri(fileUri); + if (!allowedBuckets().has(bucket)) { + throw new AppError('FORBIDDEN', `Bucket not allowed for signing: ${bucket}`, 403); + } + + const ownedPrefix = `uploads/${actorUid}/`; + if (!path.startsWith(ownedPrefix)) { + throw new AppError('FORBIDDEN', 'Cannot sign URL for another user path', 403); + } + + return { bucket, path }; +} + export async function uploadToGcs({ bucket, objectPath, contentType, buffer }) { const file = storage.bucket(bucket).file(objectPath); await file.save(buffer, { @@ -38,13 +52,14 @@ export async function uploadToGcs({ bucket, objectPath, contentType, buffer }) { }); } -export async function generateReadSignedUrl({ fileUri, expiresInSeconds }) { - const { bucket, path } = parseGsUri(fileUri); - if (!allowedBuckets().has(bucket)) { - throw new AppError('FORBIDDEN', `Bucket not allowed for signing: ${bucket}`, 403); +export async function generateReadSignedUrl({ fileUri, actorUid, expiresInSeconds }) { + const { bucket, path } = validateFileUriAccess({ fileUri, actorUid }); + const file = storage.bucket(bucket).file(path); + const [exists] = await file.exists(); + if (!exists) { + throw new AppError('NOT_FOUND', 'File not found for signed URL', 404, { fileUri }); } - const file = storage.bucket(bucket).file(path); const expiresAtMs = Date.now() + expiresInSeconds * 1000; const [signedUrl] = await file.getSignedUrl({ version: 'v4', diff --git a/backend/core-api/test/app.test.js b/backend/core-api/test/app.test.js index 19845d95..b1cdbc0e 100644 --- a/backend/core-api/test/app.test.js +++ b/backend/core-api/test/app.test.js @@ -1,10 +1,17 @@ -import test from 'node:test'; +import test, { beforeEach } from 'node:test'; 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'; -process.env.AUTH_BYPASS = 'true'; -process.env.LLM_MOCK = 'true'; +beforeEach(() => { + process.env.AUTH_BYPASS = 'true'; + process.env.LLM_MOCK = 'true'; + process.env.SIGNED_URL_MOCK = 'true'; + process.env.MAX_SIGNED_URL_SECONDS = '900'; + process.env.LLM_RATE_LIMIT_PER_MINUTE = '20'; + __resetLlmRateLimitForTests(); +}); test('GET /healthz returns healthy response', async () => { const app = createApp(); @@ -34,7 +41,7 @@ test('POST /core/create-signed-url returns signed URL', async () => { .post('/core/create-signed-url') .set('Authorization', 'Bearer test-token') .send({ - fileUri: 'gs://krow-workforce-dev-private/foo.pdf', + fileUri: 'gs://krow-workforce-dev-private/uploads/test-user/foo.pdf', expiresInSeconds: 300, }); @@ -44,6 +51,35 @@ test('POST /core/create-signed-url returns signed URL', async () => { assert.equal(typeof res.body.requestId, 'string'); }); +test('POST /core/create-signed-url rejects non-owned path', async () => { + const app = createApp(); + const res = await request(app) + .post('/core/create-signed-url') + .set('Authorization', 'Bearer test-token') + .send({ + fileUri: 'gs://krow-workforce-dev-private/uploads/other-user/foo.pdf', + expiresInSeconds: 300, + }); + + assert.equal(res.status, 403); + assert.equal(res.body.code, 'FORBIDDEN'); +}); + +test('POST /core/create-signed-url enforces expiry cap', async () => { + process.env.MAX_SIGNED_URL_SECONDS = '300'; + const app = createApp(); + const res = await request(app) + .post('/core/create-signed-url') + .set('Authorization', 'Bearer test-token') + .send({ + fileUri: 'gs://krow-workforce-dev-private/uploads/test-user/foo.pdf', + expiresInSeconds: 301, + }); + + assert.equal(res.status, 400); + assert.equal(res.body.code, 'VALIDATION_ERROR'); +}); + test('POST /invokeLLM legacy alias works', async () => { const app = createApp(); const res = await request(app) @@ -59,3 +95,31 @@ test('POST /invokeLLM legacy alias works', async () => { assert.equal(typeof res.body.result, 'object'); assert.equal(typeof res.body.model, 'string'); }); + +test('POST /core/invoke-llm enforces per-user rate limit', async () => { + process.env.LLM_RATE_LIMIT_PER_MINUTE = '1'; + const app = createApp(); + + const first = await request(app) + .post('/core/invoke-llm') + .set('Authorization', 'Bearer test-token') + .send({ + prompt: 'hello', + responseJsonSchema: { type: 'object' }, + fileUrls: [], + }); + + const second = await request(app) + .post('/core/invoke-llm') + .set('Authorization', 'Bearer test-token') + .send({ + prompt: 'hello again', + responseJsonSchema: { type: 'object' }, + fileUrls: [], + }); + + assert.equal(first.status, 200); + assert.equal(second.status, 429); + assert.equal(second.body.code, 'RATE_LIMITED'); + assert.equal(typeof second.headers['retry-after'], 'string'); +}); diff --git a/makefiles/backend.mk b/makefiles/backend.mk index 77b9d6f7..79b38bf8 100644 --- a/makefiles/backend.mk +++ b/makefiles/backend.mk @@ -31,6 +31,8 @@ BACKEND_CORE_IMAGE ?= $(BACKEND_REGION)-docker.pkg.dev/$(GCP_PROJECT_ID)/$(BACKE BACKEND_COMMAND_IMAGE ?= $(BACKEND_REGION)-docker.pkg.dev/$(GCP_PROJECT_ID)/$(BACKEND_ARTIFACT_REPO)/command-api:latest BACKEND_LOG_LIMIT ?= 100 BACKEND_LLM_MODEL ?= gemini-2.0-flash-001 +BACKEND_MAX_SIGNED_URL_SECONDS ?= 900 +BACKEND_LLM_RATE_LIMIT_PER_MINUTE ?= 20 .PHONY: backend-help backend-enable-apis backend-bootstrap-dev backend-migrate-idempotency backend-deploy-core backend-deploy-commands backend-deploy-workers backend-smoke-core backend-smoke-commands backend-logs-core @@ -129,7 +131,7 @@ backend-deploy-core: --region=$(BACKEND_REGION) \ --project=$(GCP_PROJECT_ID) \ --service-account=$(BACKEND_RUNTIME_SA_EMAIL) \ - --set-env-vars=APP_ENV=$(ENV),GCP_PROJECT_ID=$(GCP_PROJECT_ID),PUBLIC_BUCKET=$(BACKEND_PUBLIC_BUCKET),PRIVATE_BUCKET=$(BACKEND_PRIVATE_BUCKET),UPLOAD_MOCK=false,SIGNED_URL_MOCK=false,LLM_MOCK=false,LLM_LOCATION=$(BACKEND_REGION),LLM_MODEL=$(BACKEND_LLM_MODEL),LLM_TIMEOUT_MS=20000 \ + --set-env-vars=APP_ENV=$(ENV),GCP_PROJECT_ID=$(GCP_PROJECT_ID),PUBLIC_BUCKET=$(BACKEND_PUBLIC_BUCKET),PRIVATE_BUCKET=$(BACKEND_PRIVATE_BUCKET),UPLOAD_MOCK=false,SIGNED_URL_MOCK=false,LLM_MOCK=false,LLM_LOCATION=$(BACKEND_REGION),LLM_MODEL=$(BACKEND_LLM_MODEL),LLM_TIMEOUT_MS=20000,MAX_SIGNED_URL_SECONDS=$(BACKEND_MAX_SIGNED_URL_SECONDS),LLM_RATE_LIMIT_PER_MINUTE=$(BACKEND_LLM_RATE_LIMIT_PER_MINUTE) \ $(BACKEND_RUN_AUTH_FLAG) @echo "✅ Core backend service deployed."