From 52a1c8e2b2c209f64f6e99e0f5ce4faa709ba1c9 Mon Sep 17 00:00:00 2001 From: 1lystore Date: Mon, 15 Jun 2026 23:44:34 +0530 Subject: [PATCH] fix(security): resolve CodeQL alerts (rate-limit, XSS, ReDoS, proto-pollution) --- packages/dcp-core/src/budget.ts | 12 +++++++--- packages/dcp-relay/package.json | 11 +++++---- packages/dcp-relay/src/oauth/grant.ts | 3 ++- packages/dcp-relay/src/oauth/metadata.ts | 4 +++- packages/dcp-relay/src/relay.ts | 12 ++++++++-- packages/dcp-relay/src/safe-url.ts | 30 ++++++++++++++++++++++++ packages/dcp-vault/src/server/index.ts | 9 +++++++ pnpm-lock.yaml | 3 +++ 8 files changed, 72 insertions(+), 12 deletions(-) create mode 100644 packages/dcp-relay/src/safe-url.ts diff --git a/packages/dcp-core/src/budget.ts b/packages/dcp-core/src/budget.ts index cf3279a..4d8d272 100644 --- a/packages/dcp-core/src/budget.ts +++ b/packages/dcp-core/src/budget.ts @@ -259,9 +259,15 @@ export class BudgetEngine { if (amount < 0) { throw new VaultError('INTERNAL_ERROR', 'Budget limit cannot be negative'); } - // Guard against prototype-polluting currency keys (defensive — currency is a - // free-form string). - if (currency === '__proto__' || currency === 'constructor' || currency === 'prototype') { + // Guard both index keys against prototype pollution. `type` is a typed union + // but unchecked at runtime; `currency` is a free-form string. Reject the + // dangerous keys on BOTH so neither `config[type]` nor `[currency]` can reach + // Object.prototype. + const DANGEROUS = ['__proto__', 'constructor', 'prototype']; + if (type !== 'daily_budget' && type !== 'tx_limit' && type !== 'approval_threshold') { + throw new VaultError('INTERNAL_ERROR', 'Invalid budget type'); + } + if (DANGEROUS.includes(currency)) { throw new VaultError('INTERNAL_ERROR', 'Invalid currency'); } diff --git a/packages/dcp-relay/package.json b/packages/dcp-relay/package.json index f33072e..920051b 100644 --- a/packages/dcp-relay/package.json +++ b/packages/dcp-relay/package.json @@ -49,20 +49,21 @@ "author": "DCP Protocol", "license": "Apache-2.0", "dependencies": { - "fastify": "^5.2.1", - "@fastify/websocket": "^11.0.1", "@fastify/cors": "^10.0.2", + "@fastify/rate-limit": "^10.0.0", + "@fastify/websocket": "^11.0.1", "@noble/curves": "^1.4.0", - "pino-pretty": "^13.0.0", - "jose": "^5.9.6" + "fastify": "^5.2.1", + "jose": "^5.9.6", + "pino-pretty": "^13.0.0" }, "devDependencies": { - "vite": "^6.0.0", "@types/node": "^22.10.2", "@types/ws": "^8.5.13", "tsup": "^8.3.5", "tsx": "^4.0.0", "typescript": "^5.7.2", + "vite": "^6.0.0", "vitest": "^3.0.0", "ws": "^8.18.0" }, diff --git a/packages/dcp-relay/src/oauth/grant.ts b/packages/dcp-relay/src/oauth/grant.ts index 296c5db..5bf5ceb 100644 --- a/packages/dcp-relay/src/oauth/grant.ts +++ b/packages/dcp-relay/src/oauth/grant.ts @@ -13,6 +13,7 @@ * Pure logic: returns { status, body }. The HTTP layer just (de)serializes. */ +import { stripTrailingSlashes } from '../safe-url.js'; import type { RelayOAuthKeys } from './keys.js'; import type { AuthSessionStore, RefreshTokenStore } from './store.js'; import type { JtiReplayGuard } from './dpop.js'; @@ -48,7 +49,7 @@ const oauthError = (status: number, error: string, description?: string): GrantR }); function resourceFor(issuer: string, vaultId: string): string { - return `${issuer.replace(/\/+$/, '')}/v/${vaultId}/mcp`; + return `${stripTrailingSlashes(issuer)}/v/${vaultId}/mcp`; } /** Call the optional link-less pairing bridge, normalising "not wired" to an error. */ diff --git a/packages/dcp-relay/src/oauth/metadata.ts b/packages/dcp-relay/src/oauth/metadata.ts index 7cb4ca3..e6d6d3e 100644 --- a/packages/dcp-relay/src/oauth/metadata.ts +++ b/packages/dcp-relay/src/oauth/metadata.ts @@ -8,6 +8,8 @@ * DPoP, and bearer methods. No implicit/password/plain — OAuth 2.1 hardening. */ +import { stripTrailingSlashes } from '../safe-url.js'; + export interface AsMetadataOptions { /** Public base URL of the relay (the OAuth issuer), e.g. https://relay.example. No trailing slash. */ issuer: string; @@ -15,7 +17,7 @@ export interface AsMetadataOptions { } export function authorizationServerMetadata(opts: AsMetadataOptions): Record { - const base = opts.issuer.replace(/\/+$/, ''); + const base = stripTrailingSlashes(opts.issuer); return { issuer: base, authorization_endpoint: `${base}/oauth/authorize`, diff --git a/packages/dcp-relay/src/relay.ts b/packages/dcp-relay/src/relay.ts index a95bc5b..3fbfc48 100644 --- a/packages/dcp-relay/src/relay.ts +++ b/packages/dcp-relay/src/relay.ts @@ -20,6 +20,8 @@ import Fastify, { FastifyInstance, FastifyRequest, FastifyReply } from 'fastify' import type { FastifyPluginCallback, FastifyPluginOptions, RouteShorthandOptions } from 'fastify'; import fastifyWebsocket from '@fastify/websocket'; import fastifyCors from '@fastify/cors'; +import fastifyRateLimit from '@fastify/rate-limit'; +import { stripTrailingSlashes, jsonForScript } from './safe-url.js'; import type { WebSocket } from 'ws'; import { randomUUID } from 'crypto'; import { ed25519 } from '@noble/curves/ed25519'; @@ -211,6 +213,12 @@ export class RelayServer { fastifyCors as unknown as FastifyPluginCallback, { origin: true } ); + // Global rate limit on the public HTTP surface (defense-in-depth; the WS + // control channel and per-route guards still apply). + await this.server.register( + fastifyRateLimit as unknown as FastifyPluginCallback, + { max: 300, timeWindow: '1 minute' } + ); await this.server.register( fastifyWebsocket as unknown as FastifyPluginCallback ); @@ -523,7 +531,7 @@ export class RelayServer { * publicUrl; otherwise derives from the request (dev/local). No trailing slash. */ private baseUrl(request: FastifyRequest): string { - if (this.config.publicUrl) return this.config.publicUrl.replace(/\/+$/, ''); + if (this.config.publicUrl) return stripTrailingSlashes(this.config.publicUrl); const proto = (request.headers['x-forwarded-proto'] as string) || request.protocol || 'http'; const host = request.headers['host'] || `${this.config.host}:${this.config.port}`; return `${proto}://${host}`; @@ -770,7 +778,7 @@ export class RelayServer { `

Confirm this match code matches the one shown on your device, then approve there.

` + `
${esc(result.matchCode)}
` + `

Waiting for approval…

` + - `. + * + * JSON.stringify alone is NOT safe in a script context: a value containing + * `` can break out of the tag and inject markup. Escaping `<`, `>` and + * `&` to their \uXXXX forms keeps the payload a valid JS string while making + * tag-breakout impossible. + */ +export function jsonForScript(value: unknown): string { + return JSON.stringify(value) + .replace(//g, '\\u003e') + .replace(/&/g, '\\u0026'); +} diff --git a/packages/dcp-vault/src/server/index.ts b/packages/dcp-vault/src/server/index.ts index 45b46b9..70f069e 100644 --- a/packages/dcp-vault/src/server/index.ts +++ b/packages/dcp-vault/src/server/index.ts @@ -31,6 +31,7 @@ import Fastify, { FastifyInstance, FastifyRequest } from 'fastify'; import cors from '@fastify/cors'; +import rateLimit from '@fastify/rate-limit'; import { VaultStorage, BudgetEngine, @@ -1106,6 +1107,14 @@ async function buildServer(): Promise { methods: ['GET', 'POST', 'DELETE', 'PATCH'], }); + // Global rate limit. The vault server binds to localhost, but this is cheap + // defense-in-depth against a runaway/compromised local agent hammering routes. + // eslint-disable-next-line @typescript-eslint/no-explicit-any + await server.register(rateLimit as any, { + max: 600, + timeWindow: '1 minute', + }); + // Initialize vault storage (respects VAULT_DIR env for testing) const vaultDir = process.env.VAULT_DIR; storage = getStorage(vaultDir); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 9d15d9b..ef86100 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -160,6 +160,9 @@ importers: '@fastify/cors': specifier: ^10.0.2 version: 10.1.0 + '@fastify/rate-limit': + specifier: ^10.0.0 + version: 10.3.0 '@fastify/websocket': specifier: ^11.0.1 version: 11.2.0(bufferutil@4.1.0)(utf-8-validate@6.0.6)