fix(security): sovereign-audit hardening pass — RCE, multi-tenant, reliability

Reasoning-based audit fixes (all verified by typecheck, attack paths re-traced):

- build-time RCE: validate spec.dependencies to npm-registry semver only
  (no git/url/file specifiers) + --ignore-scripts in runner Dockerfile.
- container hardening fail-CLOSED: harden unless RUNNER_DISABLE_HARDENING=1,
  no longer gated on a fragile NODE_ENV string compare.
- secret env keys validated (UPPER_SNAKE, reject NODE_*/PATH/LD_*).
- cross-org image-tag collision: qualify tag with serverId.
- /iterate now enforces suspension + daily-build limits like /servers.
- preview SSE: clear keepalive in finally + on client close (timer/FD leak).
- SMS OTP: atomic attempt counter (lt(attempts,MAX) in UPDATE) — brute-force race.
- getSession orders membership by createdAt (deterministic primary org).
- template scopes aggregated from real tool scopes (was hardcoded mcp:read).
- template category filter pushed into WHERE (was applied after LIMIT).
- support admin reply/status: 404 on unknown ticket; status change now audited.
- build worker: queue defaultJobOptions, docker build/run/stop timeouts,
  old-container teardown in finally (no orphan on post-deploy DB failure).
- nginx: HSTS, X-Frame-Options DENY, nosniff, Referrer-Policy.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@
This commit is contained in:
Marco Sadjadi
2026-05-29 20:56:30 +02:00
parent 092290bb38
commit 9d5386ccba
12 changed files with 338 additions and 136 deletions

View File

@@ -17,7 +17,14 @@ let queue: Queue<BuildJobData> | null = null;
export function getBuildQueue(): Queue<BuildJobData> {
if (!queue) {
queue = new Queue<BuildJobData>('build', { connection: getRedis() });
queue = new Queue<BuildJobData>('build', {
connection: getRedis(),
// Explicit job lifecycle. attempts:1 because a build is non-idempotent
// (allocates a host port, runs a container, spends an LLM call) — a blind
// BullMQ retry would double-spend; users re-run via /iterate instead.
// removeOnComplete/Fail caps Redis growth. (GEN-007)
defaultJobOptions: { attempts: 1, removeOnComplete: 100, removeOnFail: 500 },
});
}
return queue;
}

View File

@@ -219,7 +219,8 @@ export async function serverRoutes(app: FastifyInstance): Promise<void> {
if (choice.provider !== 'anthropic' || !config.ANTHROPIC_API_KEY) {
return reply.code(409).send({
error: 'streaming_unavailable',
detail: 'Streaming preview is only available for Anthropic-backed tiers. Use POST /v1/servers/preview instead.',
detail:
'Streaming preview is only available for Anthropic-backed tiers. Use POST /v1/servers/preview instead.',
});
}
@@ -254,7 +255,10 @@ export async function serverRoutes(app: FastifyInstance): Promise<void> {
// open as long as bytes flow; comments are SSE-noop but count as bytes.
const keepalive = setInterval(() => reply.raw.write(`: ping\n\n`), 15_000);
const abort = new AbortController();
req.raw.on('close', () => abort.abort());
req.raw.on('close', () => {
abort.abort();
clearInterval(keepalive);
});
// `resolved` is set inside the awaited handlers below — by the time
// streamSpecFromAnthropic returns, exactly one of onSpec/onError will
@@ -263,95 +267,105 @@ export async function serverRoutes(app: FastifyInstance): Promise<void> {
// ended without either handler running (which would be a programming
// bug, not a runtime path).
let resolved = false;
await streamSpecFromAnthropic(
parsed.data.prompt,
{
apiKey: config.ANTHROPIC_API_KEY,
model: choice.model,
maxTokens: choice.maxTokens,
signal: abort.signal,
},
{
onText: (delta) => send('text', delta),
onSpec: async ({ spec, source }) => {
const previewId = await cacheSpec(spec);
send('spec', {
previewId,
source,
plan,
modelDisplayName: choice.displayName,
modelBadge: choice.displayBadge,
upgradeHint: plan === 'hobby',
spec: {
name: spec.name,
description: spec.description,
tools: spec.tools.map((t) => ({
name: t.name,
description: t.description,
inputSchema: t.inputSchema,
})),
requiredSecrets: spec.requiredSecrets,
scopes: spec.scopes,
},
});
app.log.info(
{
try {
await streamSpecFromAnthropic(
parsed.data.prompt,
{
apiKey: config.ANTHROPIC_API_KEY,
model: choice.model,
maxTokens: choice.maxTokens,
signal: abort.signal,
},
{
onText: (delta) => send('text', delta),
onSpec: async ({ spec, source }) => {
const previewId = await cacheSpec(spec);
send('spec', {
previewId,
tools: spec.tools.length,
prompt: parsed.data.prompt.slice(0, 200),
model: choice.displayName,
},
'preview_spec_ready',
);
resolved = true;
},
onError: (err) => {
if (err instanceof SpecTruncatedError) {
app.log.warn(
source,
plan,
modelDisplayName: choice.displayName,
modelBadge: choice.displayBadge,
upgradeHint: plan === 'hobby',
spec: {
name: spec.name,
description: spec.description,
tools: spec.tools.map((t) => ({
name: t.name,
description: t.description,
inputSchema: t.inputSchema,
})),
requiredSecrets: spec.requiredSecrets,
scopes: spec.scopes,
},
});
app.log.info(
{
reason: err.message,
previewId,
tools: spec.tools.length,
prompt: parsed.data.prompt.slice(0, 200),
model: choice.displayName,
},
'preview_spec_truncated',
'preview_spec_ready',
);
send('error', {
error: 'spec_too_large',
detail:
'The spec for this prompt exceeded the maximum response size. Split it into fewer tools or describe one capability per prompt.',
});
} else if (err instanceof SpecValidationError) {
app.log.warn(
{
zod_message: err.message,
prompt: parsed.data.prompt.slice(0, 200),
model: choice.displayName,
},
'preview_spec_invalid',
);
send('error', { error: 'spec_invalid', detail: err.message });
} else if (err instanceof BannedPatternError) {
send('error', { error: 'banned_pattern', detail: err.message });
} else if (err instanceof SpecTimeoutError) {
send('error', {
error: 'preview_timeout',
detail: 'Spec generation took too long. Try a shorter, more specific prompt.',
});
} else {
app.log.error(err);
send('error', { error: 'preview_failed', detail: err.message });
}
resolved = true;
resolved = true;
},
onError: (err) => {
if (err instanceof SpecTruncatedError) {
app.log.warn(
{
reason: err.message,
prompt: parsed.data.prompt.slice(0, 200),
model: choice.displayName,
},
'preview_spec_truncated',
);
send('error', {
error: 'spec_too_large',
detail:
'The spec for this prompt exceeded the maximum response size. Split it into fewer tools or describe one capability per prompt.',
});
} else if (err instanceof SpecValidationError) {
app.log.warn(
{
zod_message: err.message,
prompt: parsed.data.prompt.slice(0, 200),
model: choice.displayName,
},
'preview_spec_invalid',
);
send('error', { error: 'spec_invalid', detail: err.message });
} else if (err instanceof BannedPatternError) {
send('error', { error: 'banned_pattern', detail: err.message });
} else if (err instanceof SpecTimeoutError) {
send('error', {
error: 'preview_timeout',
detail: 'Spec generation took too long. Try a shorter, more specific prompt.',
});
} else {
app.log.error(err);
send('error', { error: 'preview_failed', detail: err.message });
}
resolved = true;
},
},
},
);
);
if (!resolved) {
app.log.error({ prompt: parsed.data.prompt.slice(0, 200) }, 'preview_stream_unresolved');
send('error', { error: 'preview_failed', detail: 'stream ended without a final event' });
if (!resolved) {
app.log.error({ prompt: parsed.data.prompt.slice(0, 200) }, 'preview_stream_unresolved');
send('error', { error: 'preview_failed', detail: 'stream ended without a final event' });
}
} catch (err) {
// If the stream itself rejects (e.g. cacheSpec/Redis throws inside onSpec,
// or a network error before either handler runs) we must still tear down
// the keepalive timer and close the socket — otherwise the interval keeps
// writing to a dead connection forever, leaking a timer + FD per failure. (SRV-004)
app.log.error({ err, prompt: parsed.data.prompt.slice(0, 200) }, 'preview_stream_threw');
if (!resolved) send('error', { error: 'preview_failed', detail: 'spec generation failed' });
} finally {
clearInterval(keepalive);
reply.raw.end();
}
clearInterval(keepalive);
reply.raw.end();
});
app.post('/v1/servers', { preHandler: requireAuth }, async (req, reply) => {
@@ -574,6 +588,32 @@ export async function serverRoutes(app: FastifyInstance): Promise<void> {
.limit(1);
if (!server) return reply.code(404).send({ error: 'not_found' });
// iterate queues a full paid LLM build exactly like POST /v1/servers, so it
// must enforce the same suspension + daily-build gates. Without these a
// suspended (non-paying) or rate-capped org could generate unlimited builds
// by hitting iterate instead of create. (SRV-003)
const billing = await getOrgBilling(user.orgId);
if (billing.suspended) {
return reply.code(402).send({
error: 'subscription_suspended',
detail:
billing.suspendedReason === 'payment_failed'
? 'Your subscription is paused due to a payment issue. Update your payment method in /settings/billing.'
: 'Your subscription is paused. Visit /settings/billing for details.',
suspendedReason: billing.suspendedReason,
});
}
const iterateRl = await checkDailyLimit('build', user.userId, BUILD_DAILY_LIMIT[billing.plan]);
if (!iterateRl.ok) {
return reply.code(429).send({
error: 'rate_limited',
detail: `Daily build limit reached for plan "${billing.plan}" (${BUILD_DAILY_LIMIT[billing.plan]}/day). Resets in ${Math.ceil(iterateRl.resetIn / 3600)}h.`,
plan: billing.plan,
limit: BUILD_DAILY_LIMIT[billing.plan],
resetIn: iterateRl.resetIn,
});
}
const nextVersion = server.currentVersion + 1;
const [build] = await db
.insert(builds)

View File

@@ -250,6 +250,15 @@ export async function supportRoutes(app: FastifyInstance): Promise<void> {
const body = NewMessageBody.safeParse(req.body);
if (!body.success) return reply.code(400).send({ error: 'invalid_input' });
// Confirm the ticket exists first — otherwise the insert below hits a raw
// FK violation (500) instead of a clean 404. (SUP-002)
const [ticket] = await db
.select({ id: supportTickets.id })
.from(supportTickets)
.where(eq(supportTickets.id, parsed.data.id))
.limit(1);
if (!ticket) return reply.code(404).send({ error: 'not_found' });
await db.insert(supportMessages).values({
ticketId: parsed.data.id,
authorUserId: user.userId,
@@ -282,12 +291,22 @@ export async function supportRoutes(app: FastifyInstance): Promise<void> {
'/v1/admin/support/tickets/:id/status',
{ preHandler: requireAdmin },
async (req, reply) => {
const user = req.user!;
const Params = z.object({ id: z.string().uuid() });
const parsed = Params.safeParse(req.params);
if (!parsed.success) return reply.code(400).send({ error: 'invalid_id' });
const body = StatusBody.safeParse(req.body);
if (!body.success) return reply.code(400).send({ error: 'invalid_input' });
// 404 on unknown ticket instead of a silent no-op `UPDATE ... WHERE id=?`
// that returns ok:true and masks the bad id. (SUP-002)
const [ticket] = await db
.select({ id: supportTickets.id })
.from(supportTickets)
.where(eq(supportTickets.id, parsed.data.id))
.limit(1);
if (!ticket) return reply.code(404).send({ error: 'not_found' });
await db
.update(supportTickets)
.set({
@@ -297,6 +316,17 @@ export async function supportRoutes(app: FastifyInstance): Promise<void> {
})
.where(eq(supportTickets.id, parsed.data.id));
// Status changes were previously unaudited, unlike admin replies — close
// the compliance-trail gap. (SUP-002)
await audit({
orgId: user.orgId,
userId: user.userId,
action: 'support.status_changed',
resourceType: 'support_ticket',
resourceId: parsed.data.id,
metadata: { status: body.data.status },
});
return reply.send({ ok: true });
},
);

View File

@@ -194,10 +194,15 @@ export async function templateRoutes(app: FastifyInstance): Promise<void> {
toolsSchema: server.toolsSchema,
generatedCode: build.generatedCode,
requiredSecrets: parsed.data.secretHints,
scopes: (server.toolsSchema as Array<{ scopes?: string[] }>).reduce<string[]>(
() => ['mcp:read'],
[],
),
// Aggregate the distinct scopes actually declared by the server's tools
// (deduped), falling back to read-only. The previous reduce ignored its
// input and hardcoded ['mcp:read'] for every template regardless of what
// its tools did. (TPL-003)
scopes: (() => {
const tools = (server.toolsSchema as Array<{ scopes?: string[] }> | null) ?? [];
const all = [...new Set(tools.flatMap((t) => t.scopes ?? []))];
return all.length > 0 ? all : ['mcp:read'];
})(),
allowedDomains: parsed.data.allowedDomains ?? null,
})
.returning();
@@ -310,13 +315,19 @@ export async function templateRoutes(app: FastifyInstance): Promise<void> {
.from(templates)
.leftJoin(users, eq(users.id, templates.ownerUserId))
.leftJoin(organizations, eq(organizations.id, templates.ownerOrgId))
.where(eq(templates.status, 'public'))
// Category filter belongs in the WHERE, BEFORE limit — filtering in JS
// after `.limit(50)` meant `?category=x` searched only the 50 newest
// public templates (any category), returning far fewer than `limit`. (TPL-008)
.where(
and(
eq(templates.status, 'public'),
parsed.data.category ? eq(templates.category, parsed.data.category) : undefined,
),
)
.orderBy(desc(templates.createdAt))
.limit(parsed.data.limit);
const filtered = parsed.data.category
? rows.filter((r) => r.template.category === parsed.data.category)
: rows;
const filtered = rows;
// Single grouped query — was N+1 (one COUNT per template). On a 100-row
// listing that's 101 round-trips → p95 latency cliff once the marketplace