feat(core-api): harden signed urls and llm rate limits
This commit is contained in:
@@ -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(
|
||||
{
|
||||
|
||||
@@ -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') {
|
||||
|
||||
41
backend/core-api/src/services/llm-rate-limit.js
Normal file
41
backend/core-api/src/services/llm-rate-limit.js
Normal file
@@ -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();
|
||||
}
|
||||
@@ -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',
|
||||
|
||||
@@ -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');
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user