diff --git a/src/adapters/webTools.test.ts b/src/adapters/webTools.test.ts index 5700c4a9..e3f39288 100644 --- a/src/adapters/webTools.test.ts +++ b/src/adapters/webTools.test.ts @@ -58,6 +58,29 @@ describe('redirect method rewriting', () => { expect(second.method).toBe('POST'); expect(second.body).toBeTruthy(); }); + + it('cancels the redirect response body before following the next hop', async () => { + const cancel = vi.fn(async () => {}); + let calls = 0; + const f = vi.fn(async () => { + calls += 1; + if (calls === 1) { + return { + status: 302, + ok: false, + statusText: 'Found', + headers: new Headers({ location: 'https://example.com/next' }), + body: { cancel }, + arrayBuffer: async () => new ArrayBuffer(0), + } as unknown as Response; + } + return new Response('ok', { status: 200, headers: { 'content-type': 'text/plain' } }); + }); + vi.stubGlobal('fetch', f); + await webFetch('https://example.com/start'); + expect(cancel).toHaveBeenCalledOnce(); + expect(f).toHaveBeenCalledTimes(2); + }); }); describe('webFetch', () => { diff --git a/src/adapters/webTools.ts b/src/adapters/webTools.ts index ed9e7155..48f73207 100644 --- a/src/adapters/webTools.ts +++ b/src/adapters/webTools.ts @@ -128,7 +128,15 @@ async function fetchWithTimeout( if ([301, 302, 303, 307, 308].includes(response.status)) { const location = response.headers.get('location'); if (!location) throw new Error('Redirect response has no location'); - await response.body?.cancel(); + // Release the redirect body before the next hop so undici does not keep + // the prior socket/buffer alive across a chain of Location responses. + if (response.body) { + try { + await response.body.cancel(); + } catch { + await response.arrayBuffer().catch(() => undefined); + } + } const next = new URL(location, current); if (carriesSensitiveRequestData && next.origin !== initialOrigin) { throw new Error('Refusing to forward credentials or request body across origins'); diff --git a/src/auth/index.ts b/src/auth/index.ts index 2fc942b9..fb60f681 100644 --- a/src/auth/index.ts +++ b/src/auth/index.ts @@ -7,6 +7,7 @@ export { runOAuthPkceFlow, loginAndSaveProfile, DEFAULT_OPENAI_CLIENT_ID, + isLoopbackRemote, type OAuthFlowResult, type OAuthFlowOptions, } from './oauthPkce.js'; diff --git a/src/auth/oauthPkce.test.ts b/src/auth/oauthPkce.test.ts new file mode 100644 index 00000000..136fc627 --- /dev/null +++ b/src/auth/oauthPkce.test.ts @@ -0,0 +1,27 @@ +import { describe, expect, it } from 'vitest'; +import { isLoopbackRemote } from './oauthPkce.js'; + +/** + * The OpenAI redirect_uri is always the advertised localhost form. Callbacks may + * still arrive over IPv4 127.0.0.1 or IPv6 ::1 on the dual-stack listener. + */ +describe('isLoopbackRemote (OpenAI PKCE callback)', () => { + const advertisedRedirectUri = (port = 1455) => `http://localhost:${port}/auth/callback`; + + it('accepts IPv4 and IPv6 localhost remotes against the advertised redirect URI', () => { + expect(advertisedRedirectUri()).toBe('http://localhost:1455/auth/callback'); + expect(isLoopbackRemote('127.0.0.1')).toBe(true); + expect(isLoopbackRemote('::1')).toBe(true); + expect(isLoopbackRemote('::ffff:127.0.0.1')).toBe(true); + }); + + it('rejects non-loopback remotes that must never drive the callback', () => { + expect(isLoopbackRemote(undefined)).toBe(false); + expect(isLoopbackRemote('')).toBe(false); + expect(isLoopbackRemote('10.0.0.1')).toBe(false); + expect(isLoopbackRemote('192.168.1.1')).toBe(false); + expect(isLoopbackRemote('8.8.8.8')).toBe(false); + expect(isLoopbackRemote('fe80::1')).toBe(false); + expect(isLoopbackRemote('2001:db8::1')).toBe(false); + }); +}); diff --git a/src/auth/oauthPkce.ts b/src/auth/oauthPkce.ts index c5f22d4c..e4f85e25 100644 --- a/src/auth/oauthPkce.ts +++ b/src/auth/oauthPkce.ts @@ -4,6 +4,7 @@ // ============================================ import { createServer, type IncomingMessage, type ServerResponse } from 'node:http'; +import { isIP } from 'node:net'; import { randomBytes, createHash } from 'node:crypto'; import { AuthProfileStore, type AuthProfile } from './oauthStore.js'; import { openBrowser } from './openBrowser.js'; @@ -26,6 +27,26 @@ const PROFILE_KEY = 'openai-gpt:default'; export const DEFAULT_OPENAI_CLIENT_ID = 'app_EMoamEEZ73f0CkXaXp7hrann'; const OAUTH_ORIGINATOR = 'openswarm'; +/** + * Accept only the advertised loopback callback forms: IPv4 127.0.0.1 and IPv6 + * ::1 (including the IPv4-mapped ::ffff:127.0.0.1 form Node reports when a + * dual-stack socket receives an IPv4 connection). Any other remote address is + * rejected so the callback server cannot be driven from off-host. + * + * The OpenAI redirect_uri stays `http://localhost:/auth/callback` (the + * registered client value); browsers may still connect via ::1 or 127.0.0.1. + */ +export function isLoopbackRemote(address: string | undefined): boolean { + if (!address) return false; + const family = isIP(address); + if (family === 4) return address === '127.0.0.1'; + if (family === 6) { + const lower = address.toLowerCase(); + return lower === '::1' || lower === '::ffff:127.0.0.1'; + } + return false; +} + // PKCE helpers function generateCodeVerifier(): string { @@ -113,6 +134,12 @@ export async function runOAuthPkceFlow(options: OAuthFlowOptions = {}): Promise< return; } + if (!isLoopbackRemote(req.socket.remoteAddress)) { + res.writeHead(403); + res.end('Forbidden'); + return; + } + const url = new URL(req.url ?? '/', `http://127.0.0.1:${port}`); if (url.pathname !== '/auth/callback') { @@ -236,8 +263,8 @@ export async function runOAuthPkceFlow(options: OAuthFlowOptions = {}): Promise< } }); - server.listen(port, '127.0.0.1', () => { - console.log(`[Auth] Callback server listening on http://127.0.0.1:${port}`); + server.listen(port, 'localhost', () => { + console.log(`[Auth] Callback server listening on http://localhost:${port} (IPv4 + IPv6 loopback)`); console.log(`[Auth] 브라우저에서 OpenAI 로그인 페이지를 엽니다...`); openBrowser(authUrl); }); diff --git a/src/issues/graphql/server.test.ts b/src/issues/graphql/server.test.ts index 0dc03e8b..db930352 100644 --- a/src/issues/graphql/server.test.ts +++ b/src/issues/graphql/server.test.ts @@ -1,6 +1,12 @@ import { afterEach, describe, expect, it } from 'vitest'; import { createServer, type IncomingMessage } from 'node:http'; -import { handleGraphQL, isGraphQLTransportAuthorized } from './server.js'; +import { + createGraphQLCostRule, + handleGraphQL, + isGraphQLRequest, + isGraphQLTransportAuthorized, +} from './server.js'; +import { validate, buildSchema, parse } from 'graphql'; function request(address: string | undefined, headers: Record = {}): IncomingMessage { return { socket: { remoteAddress: address }, headers } as unknown as IncomingMessage; @@ -8,6 +14,61 @@ function request(address: string | undefined, headers: Record = afterEach(() => { delete process.env.OPENSWARM_GRAPHQL_TOKEN; }); +describe('isGraphQLRequest exact endpoint matching', () => { + it('accepts only the exact /graphql path (with or without query/hash)', () => { + expect(isGraphQLRequest('/graphql')).toBe(true); + expect(isGraphQLRequest('/graphql?query={__typename}')).toBe(true); + expect(isGraphQLRequest('/graphql#section')).toBe(true); + }); + + it('rejects path-prefix lookalikes that previously matched startsWith', () => { + expect(isGraphQLRequest('/graphql/')).toBe(false); + expect(isGraphQLRequest('/graphql/admin')).toBe(false); + expect(isGraphQLRequest('/graphqlfoo')).toBe(false); + expect(isGraphQLRequest('/api/graphql')).toBe(false); + expect(isGraphQLRequest(undefined)).toBe(false); + expect(isGraphQLRequest('')).toBe(false); + }); +}); + +describe('GraphQL query cost limits', () => { + const schema = buildSchema(` + type Query { + a: Query + b: String + } + `); + + it('rejects a document that exceeds max depth', () => { + const query = '{ a { a { a { a { b } } } } }'; + const errors = validate(schema, parse(query), [createGraphQLCostRule({ maxDepth: 3 })]); + expect(errors.some((e) => /maximum depth/i.test(e.message))).toBe(true); + }); + + it('rejects a document that exceeds field or alias counts', () => { + const query = '{ x: b y: b z: b }'; + const errors = validate(schema, parse(query), [ + createGraphQLCostRule({ maxFieldCount: 2, maxAliasCount: 2, maxCost: 100 }), + ]); + expect(errors.some((e) => /maximum (field|alias) count/i.test(e.message))).toBe(true); + }); + + it('rejects a document that exceeds weighted execution cost', () => { + // cost accumulates as Σ depth at each field enter — a wide shallow fan-out + // of depth-1 fields exceeds a tight budget without tripping depth/alias caps. + const query = '{ a1: b a2: b a3: b a4: b }'; + const errors = validate(schema, parse(query), [ + createGraphQLCostRule({ maxDepth: 10, maxFieldCount: 20, maxAliasCount: 20, maxCost: 3 }), + ]); + expect(errors.some((e) => /maximum execution cost/i.test(e.message))).toBe(true); + }); + + it('allows a shallow bounded query', () => { + const errors = validate(schema, parse('{ b }'), [createGraphQLCostRule()]); + expect(errors).toEqual([]); + }); +}); + describe('GraphQL transport authorization', () => { it('allows a proven loopback transport without trusting Origin', () => { expect(isGraphQLTransportAuthorized(request('127.0.0.1'))).toBe(true); diff --git a/src/issues/graphql/server.ts b/src/issues/graphql/server.ts index 6555f6f8..14f9afd8 100644 --- a/src/issues/graphql/server.ts +++ b/src/issues/graphql/server.ts @@ -5,6 +5,12 @@ // ============================================ import { createSchema, createYoga } from 'graphql-yoga'; +import { + GraphQLError, + type ASTVisitor, + type ValidationContext, + type ValidationRule, +} from 'graphql'; import { typeDefs } from './typeDefs.js'; import { resolvers } from './resolvers.js'; import { registryTypeDefs } from '../../registry/graphql/typeDefs.js'; @@ -15,6 +21,12 @@ import { timingSafeEqual } from 'node:crypto'; const CORS_METHODS = 'GET, POST, OPTIONS'; const CORS_HEADERS = 'Content-Type, Authorization, X-OpenSwarm-GraphQL-Token'; +/** Depth / fan-out / alias / weighted-cost caps for untrusted GraphQL documents. */ +export const GRAPHQL_MAX_DEPTH = 12; +export const GRAPHQL_MAX_FIELD_COUNT = 120; +export const GRAPHQL_MAX_ALIAS_COUNT = 40; +export const GRAPHQL_MAX_COST = 250; + function isAllowedOrigin(origin: string): boolean { let url: URL; try { @@ -97,6 +109,75 @@ export function isGraphQLTransportAuthorized(req: IncomingMessage): boolean { return hasValidToken({ authorization, token }); } +/** + * Reject documents that nest, fan out, or alias themselves into an unbounded + * execution cost before resolvers run. + */ +export function createGraphQLCostRule(limits: { + maxDepth?: number; + maxFieldCount?: number; + maxAliasCount?: number; + maxCost?: number; +} = {}): ValidationRule { + const maxDepth = limits.maxDepth ?? GRAPHQL_MAX_DEPTH; + const maxFieldCount = limits.maxFieldCount ?? GRAPHQL_MAX_FIELD_COUNT; + const maxAliasCount = limits.maxAliasCount ?? GRAPHQL_MAX_ALIAS_COUNT; + const maxCost = limits.maxCost ?? GRAPHQL_MAX_COST; + + return function GraphQLCostRule(context: ValidationContext): ASTVisitor { + let depth = 0; + let fieldCount = 0; + let aliasCount = 0; + let cost = 0; + + return { + Field: { + enter(node) { + depth += 1; + fieldCount += 1; + if (node.alias) aliasCount += 1; + // Deeper fields cost more so nested selection sets hit the budget first. + cost += depth; + + if (depth > maxDepth) { + context.reportError(new GraphQLError( + `Query exceeds maximum depth of ${maxDepth}`, + { nodes: [node] }, + )); + } + if (fieldCount > maxFieldCount) { + context.reportError(new GraphQLError( + `Query exceeds maximum field count of ${maxFieldCount}`, + { nodes: [node] }, + )); + } + if (aliasCount > maxAliasCount) { + context.reportError(new GraphQLError( + `Query exceeds maximum alias count of ${maxAliasCount}`, + { nodes: [node] }, + )); + } + if (cost > maxCost) { + context.reportError(new GraphQLError( + `Query exceeds maximum execution cost of ${maxCost}`, + { nodes: [node] }, + )); + } + }, + leave() { + depth -= 1; + }, + }, + // Fragment fields are counted when the fragment definition itself is walked. + FragmentDefinition: { + enter() { + depth = 0; + }, + }, + }; + }; +} + // GraphQL Yoga 인스턴스 생성 (이슈 + 코드 레지스트리 스키마 머지) const yoga = createYoga({ schema: createSchema({ @@ -111,6 +192,13 @@ const yoga = createYoga({ warn: (...args: any[]) => console.warn('[GraphQL]', ...args), error: (...args: any[]) => console.error('[GraphQL]', ...args), }, + plugins: [ + { + onValidate({ addValidationRule }) { + addValidationRule(createGraphQLCostRule()); + }, + }, + ], }); /** @@ -134,11 +222,13 @@ export async function handleGraphQL( } /** - * GraphQL 경로 매칭 여부 + * Exact GraphQL endpoint match — path-prefix forms like `/graphql/admin` or + * `/graphqlfoo` must not reach the Yoga handler. */ export function isGraphQLRequest(url: string | undefined): boolean { if (!url) return false; - return url.startsWith('/graphql'); + const path = url.split(/[?#]/, 1)[0] ?? ''; + return path === '/graphql'; } export { yoga }; diff --git a/src/mcp/mcpClient.test.ts b/src/mcp/mcpClient.test.ts index cc9f8132..60458ce2 100644 --- a/src/mcp/mcpClient.test.ts +++ b/src/mcp/mcpClient.test.ts @@ -11,6 +11,7 @@ import { initMcpTools, callMcpTool, withDeadline, + sanitizeInputSchema, } from './mcpClient.js'; import type { ToolDefinition } from '../adapters/tools.js'; @@ -416,3 +417,30 @@ describe('withDeadline', () => { vi.useRealTimers(); }); }); + +describe('sanitizeInputSchema bounds', () => { + it('keeps a small object schema', () => { + const schema = { type: 'object', properties: { q: { type: 'string' } }, required: ['q'] }; + expect(sanitizeInputSchema(schema)).toEqual(schema); + }); + + it('replaces oversized serialized schemas with the empty object schema', () => { + const properties: Record = {}; + for (let i = 0; i < 40; i++) { + properties[`field_${i}`] = { type: 'string', description: 'x'.repeat(500) }; + } + const out = sanitizeInputSchema({ type: 'object', properties }); + expect(out).toEqual({ type: 'object', properties: {} }); + }); + + it('replaces schemas with too many properties', () => { + const properties: Record = {}; + for (let i = 0; i < 80; i++) properties[`p${i}`] = { type: 'string' }; + expect(sanitizeInputSchema({ type: 'object', properties })).toEqual({ type: 'object', properties: {} }); + }); + + it('rejects non-object root types', () => { + expect(sanitizeInputSchema({ type: 'string' })).toEqual({ type: 'object', properties: {} }); + expect(sanitizeInputSchema(null)).toEqual({ type: 'object', properties: {} }); + }); +}); diff --git a/src/mcp/mcpClient.ts b/src/mcp/mcpClient.ts index e7bcc0f7..2278a0a8 100644 --- a/src/mcp/mcpClient.ts +++ b/src/mcp/mcpClient.ts @@ -34,6 +34,9 @@ const MCP_JSON_PATH = join(homedir(), '.openswarm', 'mcp.json'); const MAX_MCP_TOOL_RESULT_CHARS = 20_000; const MCP_CONNECT_TIMEOUT_MS = 15_000; const MCP_OPERATION_TIMEOUT_MS = 30_000; +/** Bound remote tool input schemas before they are cached or exposed to the model. */ +const MAX_INPUT_SCHEMA_BYTES = 16_384; +const MAX_INPUT_SCHEMA_PROPERTIES = 64; const EMPTY_INPUT_SCHEMA: Record = { type: 'object', properties: {} }; interface ServerConfig { @@ -123,9 +126,46 @@ function isJsonSchemaObject(schema: unknown, depth = 0): schema is Record { +function countSchemaProperties(schema: Record, depth = 0): number { + if (depth > 8) return Number.POSITIVE_INFINITY; + let count = 0; + if (isRecord(schema.properties)) { + count += Object.keys(schema.properties).length; + for (const value of Object.values(schema.properties)) { + if (isRecord(value)) count += countSchemaProperties(value, depth + 1); + } + } + for (const keyword of ['anyOf', 'oneOf', 'allOf'] as const) { + const value = schema[keyword]; + if (!Array.isArray(value)) continue; + for (const entry of value) { + if (isRecord(entry)) count += countSchemaProperties(entry, depth + 1); + } + } + if (isRecord(schema.items)) count += countSchemaProperties(schema.items, depth + 1); + else if (Array.isArray(schema.items)) { + for (const entry of schema.items) { + if (isRecord(entry)) count += countSchemaProperties(entry, depth + 1); + } + } + if (isRecord(schema.additionalProperties)) { + count += countSchemaProperties(schema.additionalProperties, depth + 1); + } + return count; +} + +/** Drop oversized or non-object remote schemas so tool defs cannot balloon the cache. */ +export function sanitizeInputSchema(schema: unknown): Record { if (!isJsonSchemaObject(schema)) return EMPTY_INPUT_SCHEMA; if (schema.type !== undefined && schema.type !== 'object') return EMPTY_INPUT_SCHEMA; + if (countSchemaProperties(schema) > MAX_INPUT_SCHEMA_PROPERTIES) return EMPTY_INPUT_SCHEMA; + let serialized: string; + try { + serialized = JSON.stringify(schema); + } catch { + return EMPTY_INPUT_SCHEMA; + } + if (serialized.length > MAX_INPUT_SCHEMA_BYTES) return EMPTY_INPUT_SCHEMA; return schema; } diff --git a/src/support/outboundUrl.test.ts b/src/support/outboundUrl.test.ts index 813a66cd..b88fade0 100644 --- a/src/support/outboundUrl.test.ts +++ b/src/support/outboundUrl.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from 'vitest'; -import { createPublicLookup, isPrivateIp, type DnsAllResolver } from './outboundUrl.js'; +import { createPublicLookup, createPinnedPublicLookup, isPrivateIp, type DnsAllResolver } from './outboundUrl.js'; function resolverReturning(addresses: Array<{ address: string; family: number }>): DnsAllResolver { return (_hostname, _options, callback) => callback(null, addresses); @@ -46,6 +46,30 @@ describe('createPublicLookup callback contract', () => { }); }); +describe('createPinnedPublicLookup', () => { + it('returns only the pre-validated addresses without a second DNS round-trip', () => { + const addresses = [{ address: '93.184.216.34', family: 4 }]; + const lookup = createPinnedPublicLookup(addresses); + const seen: unknown[] = []; + lookup('example.com', { all: true }, (...args) => seen.push(args)); + lookup('example.com', {}, (...args) => seen.push(args)); + expect(seen).toEqual([ + [null, addresses], + [null, '93.184.216.34', 4], + ]); + }); + + it('refuses when the pinned set includes a private address', () => { + const lookup = createPinnedPublicLookup([ + { address: '93.184.216.34', family: 4 }, + { address: '127.0.0.1', family: 4 }, + ]); + let error: Error | null = null; + lookup('rebind.example', { all: true }, (err) => { error = err; }); + expect(error).toBeInstanceOf(Error); + }); +}); + describe('isPrivateIp IPv6 canonicalization', () => { it.each([ '0:0:0:0:0:0:0:1', diff --git a/src/support/outboundUrl.ts b/src/support/outboundUrl.ts index a195e5cc..8bf2ebbf 100644 --- a/src/support/outboundUrl.ts +++ b/src/support/outboundUrl.ts @@ -74,8 +74,16 @@ export function isPrivateIp(address: string): boolean { return false; } +interface ResolvedAddress { address: string; family: number } + +export interface ResolvedPublicHttpUrl { + url: URL; + /** Addresses validated in the same lookup the socket must use — prevents DNS rebinding. */ + addresses: ResolvedAddress[]; +} + /** Resolve and reject destinations that can reach the local machine or a private network. */ -export async function assertPublicHttpUrl(value: string): Promise { +export async function resolvePublicHttpUrl(value: string): Promise { const url = new URL(value); if (url.protocol !== 'https:' && url.protocol !== 'http:') throw new Error('Only HTTP(S) URLs are allowed'); if (url.username || url.password) throw new Error('Webhook URLs must not contain userinfo'); @@ -85,16 +93,35 @@ export async function assertPublicHttpUrl(value: string): Promise { } if (isIP(hostname)) { if (isPrivateIp(hostname)) throw new Error('Private network destinations are not allowed'); - return url; + const family = isIP(hostname); + return { url, addresses: [{ address: hostname, family }] }; } const addresses = await lookup(hostname, { all: true, verbatim: true }); if (addresses.length === 0 || addresses.some(({ address }) => isPrivateIp(address))) { throw new Error('Private network destinations are not allowed'); } - return url; + return { url, addresses }; } -interface ResolvedAddress { address: string; family: number } +/** Resolve and reject destinations that can reach the local machine or a private network. */ +export async function assertPublicHttpUrl(value: string): Promise { + return (await resolvePublicHttpUrl(value)).url; +} + +/** Lookup hook that returns only the pre-validated addresses — no second DNS round-trip. */ +export function createPinnedPublicLookup(addresses: readonly ResolvedAddress[]) { + return function pinnedPublicLookup( + _hostname: string, + options: { all?: boolean } | undefined, + callback: (error: Error | null, addresses: ResolvedAddress[] | string, family?: number) => void, + ): void { + if (addresses.length === 0 || addresses.some(({ address }) => isPrivateIp(address))) { + return callback(new Error('Private network destinations are not allowed'), '', 0); + } + if (options?.all) return callback(null, [...addresses]); + callback(null, addresses[0].address, addresses[0].family); + }; +} /** Injectable so the callback contract below can be tested without real DNS. */ export type DnsAllResolver = ( @@ -129,13 +156,6 @@ export function createPublicLookup(resolve: DnsAllResolver = lookupCallback as u }; } -let sharedPublicAgent: Agent | undefined; - -function publicNetworkAgent(): Agent { - sharedPublicAgent ??= new Agent({ connect: { lookup: createPublicLookup() } }); - return sharedPublicAgent; -} - /** * Fetch that refuses private destinations before connecting and pins the * resolved address, so a rebinding answer cannot move the socket after the @@ -144,15 +164,25 @@ function publicNetworkAgent(): Agent { * undici's own `fetch` is required: a dispatcher built from the npm `undici` * package is rejected by the copy of undici bundled inside Node's global * `fetch` ("invalid onError method"), which would fail every request. + * + * The connect-time lookup returns the same addresses `resolvePublicHttpUrl` + * already validated — a second DNS answer cannot steer the socket onto a + * different (still-"public") host after the check. Notification webhooks and + * other outbound callers therefore connect only to the validated public address. */ export async function publicFetch(url: string | URL, init: RequestInit = {}): Promise { - const validated = await assertPublicHttpUrl(String(url)); - // undici's Response is spec-compatible with the global one; the DOM lib types - // are structurally distinct, so cross the boundary once, here. - return undiciFetch(validated.toString(), { - ...(init as Record), - dispatcher: publicNetworkAgent(), - } as never) as unknown as Response; + const { url: validated, addresses } = await resolvePublicHttpUrl(String(url)); + const agent = new Agent({ connect: { lookup: createPinnedPublicLookup(addresses) } }); + try { + // undici's Response is spec-compatible with the global one; the DOM lib types + // are structurally distinct, so cross the boundary once, here. + return await undiciFetch(validated.toString(), { + ...(init as Record), + dispatcher: agent, + } as never) as unknown as Response; + } finally { + void agent.close(); + } } /** Cheap configuration-time check; send-time validation also resolves DNS. */ diff --git a/src/verify/runner.test.ts b/src/verify/runner.test.ts index e95fff0e..8ca558cc 100644 --- a/src/verify/runner.test.ts +++ b/src/verify/runner.test.ts @@ -4,7 +4,12 @@ import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import type { VerifyCommand } from './manifest.js'; -import { runVerify } from './runner.js'; +import { + buildVerifyToolchainPath, + runVerify, + VERIFY_ALLOWED_DEPENDENCY_DIRS, + VERIFY_TOOLCHAIN_PATH_PREFIXES, +} from './runner.js'; let root: string; let repo: string; @@ -604,3 +609,30 @@ describe('runVerify', () => { expect(evidence.rawOutputTail).toContain('tail-marker'); }); }); + +describe('buildVerifyToolchainPath', () => { + it('keeps only the explicit read-only toolchain prefixes and project dependency bins', () => { + const path = buildVerifyToolchainPath( + ['/usr/bin', '/home/user/.local/bin', '/opt/homebrew/bin', '/tmp/evil/bin'].join(':'), + '/repo', + '/repo/packages/api', + ); + const parts = path.split(':'); + for (const prefix of VERIFY_TOOLCHAIN_PATH_PREFIXES) { + expect(parts).toContain(prefix); + } + expect(parts).toContain('/repo/node_modules/.bin'); + expect(parts).toContain('/repo/packages/api/node_modules/.bin'); + expect(parts).not.toContain('/home/user/.local/bin'); + expect(parts).not.toContain('/tmp/evil/bin'); + expect(VERIFY_ALLOWED_DEPENDENCY_DIRS.has('node_modules')).toBe(true); + }); + + it('preserves nvm/fnm-style version-manager bin directories', () => { + const path = buildVerifyToolchainPath( + '/home/user/.nvm/versions/node/v22.0.0/bin:/usr/bin', + '/repo', + ); + expect(path.split(':')).toContain('/home/user/.nvm/versions/node/v22.0.0/bin'); + }); +}); diff --git a/src/verify/runner.ts b/src/verify/runner.ts index 7b071481..3ffc9caa 100644 --- a/src/verify/runner.ts +++ b/src/verify/runner.ts @@ -22,12 +22,33 @@ const OUTPUT_TAIL_BYTES = 8 * 1024; const FINGERPRINT_BYTES = 4 * 1024 * 1024; const GIT_TIMEOUT_MS = 30_000; const execFileAsync = promisify(execFile); -const DEPENDENCY_INPUTS = new Set([ + +/** Dependency manifests the sandbox may observe when comparing base vs head. */ +export const VERIFY_DEPENDENCY_INPUTS = new Set([ 'package.json', 'package-lock.json', 'npm-shrinkwrap.json', 'pnpm-lock.yaml', 'yarn.lock', 'Cargo.toml', 'Cargo.lock', 'go.mod', 'go.sum', 'requirements.txt', 'pyproject.toml', 'uv.lock', 'poetry.lock', ]); +/** Shared dependency directories the verify sandbox may bind read-only from the live tree. */ +export const VERIFY_ALLOWED_DEPENDENCY_DIRS = new Set([ + 'node_modules', '.venv-verify', '.venv', 'venv', +]); + +/** + * Read-only toolchain PATH prefixes verification may inherit. Anything else on + * the host PATH (writable home dirs, arbitrary tool installs) stays out of the + * sandbox so a malicious checkout cannot pick an unexpected binary via PATH. + */ +export const VERIFY_TOOLCHAIN_PATH_PREFIXES = [ + '/usr/bin', + '/bin', + '/usr/local/bin', + '/opt/homebrew/bin', +] as const; + +const DEPENDENCY_INPUTS = VERIFY_DEPENDENCY_INPUTS; + export interface VerifyEvidence { command: VerifyCommand; baseStatus: 'pass' | 'fail' | 'infra' | 'skipped'; @@ -69,7 +90,7 @@ async function verificationSharedPaths(projectPath: string, commands: VerifyComm if (localDirectory === '..' || localDirectory.startsWith(`..${sep}`) || isAbsolute(localDirectory)) { throw new Error(`[security] verify cwd escapes project root: ${directory}`); } - for (const name of ['node_modules', '.venv-verify', '.venv', 'venv']) { + for (const name of VERIFY_ALLOWED_DEPENDENCY_DIRS) { const dependency = join(localDirectory, name); try { await access(join(projectPath, dependency)); @@ -90,7 +111,7 @@ function isPrivateEnvironmentPath(path: string): boolean { function sharedPathSecretFilter(sharedPath: string): (path: string) => boolean { // Like the companion's secret scan, dependency payloads retain packaged // certificates (e.g. certifi/cacert.pem). Local configuration stays excluded. - return ['node_modules', '.venv', '.venv-verify', 'venv'].includes(basename(sharedPath)) + return VERIFY_ALLOWED_DEPENDENCY_DIRS.has(basename(sharedPath)) ? (path) => path.split(sep).some(isPrivateConfigurationFile) : isPrivateEnvironmentPath; } @@ -270,6 +291,52 @@ function vegaVerifyWorkspaceRoot(root: string): string | undefined { return existsSync(join(root, 'pipeline', 'path_guard.py')) ? root : undefined; } +/** + * Build the sandbox PATH from an explicit read-only toolchain allowlist plus + * project-local dependency bins (node_modules/.bin, venv/bin). Host PATH entries + * outside those prefixes are dropped. + */ +export function buildVerifyToolchainPath( + envPath: string | undefined, + root: string, + cwd: string = root, +): string { + const entries: string[] = []; + const seen = new Set(); + const add = (candidate: string): void => { + if (!candidate || seen.has(candidate)) return; + seen.add(candidate); + entries.push(candidate); + }; + + for (const prefix of VERIFY_TOOLCHAIN_PATH_PREFIXES) add(prefix); + + for (const base of [cwd, root]) { + add(join(base, 'node_modules', '.bin')); + for (const venv of VERIFY_ALLOWED_DEPENDENCY_DIRS) { + if (venv === 'node_modules') continue; + add(join(base, venv, process.platform === 'win32' ? 'Scripts' : 'bin')); + } + } + + for (const part of (envPath ?? '').split(delimiter)) { + if (!part) continue; + const normalized = part.replace(/\\/g, '/').replace(/\/+$/, ''); + const allowedPrefix = VERIFY_TOOLCHAIN_PATH_PREFIXES.some( + (prefix) => normalized === prefix || normalized.startsWith(`${prefix}/`), + ); + const projectLocal = /(?:^|\/)(?:node_modules\/\.bin|(?:\.venv-verify|\.venv|venv)\/(?:bin|Scripts))$/ + .test(normalized); + // Node version managers install under */bin — keep those so `node`/`npm` + // remain reachable without opening the entire home directory PATH. + const versionManagerBin = /(?:^|\/)(?:\.?nvm|fnm|asdf|volta|n)(?:\/|$)/.test(normalized) + && /\/bin$/.test(normalized); + if (allowedPrefix || projectLocal || versionManagerBin) add(part); + } + + return entries.join(delimiter); +} + async function runWithSandboxExecutor( command: VerifyCommand, root: string, @@ -281,13 +348,12 @@ async function runWithSandboxExecutor( const timeoutMs = command.timeoutMs ?? 300_000; try { const session = await createSession(root); - const cwdBin = join(cwd, 'node_modules', '.bin'); - const rootBin = join(root, 'node_modules', '.bin'); const relativeCwd = relative(root, cwd) || '.'; const vegaWorkspace = vegaVerifyWorkspaceRoot(root); + const toolchainPath = buildVerifyToolchainPath(process.env.PATH, root, cwd); const result = await session.execute([ `cd -- ${shellQuote(relativeCwd)}`, - `export PATH=${shellQuote(`${cwdBin}${delimiter}${rootBin}`)}:"$PATH"`, + `export PATH=${shellQuote(toolchainPath)}`, ...(vegaWorkspace ? [`export VEGA_EXTRA_PATHS=${shellQuote(vegaWorkspace)}`] : []), // Bundled VEGA toolsets intentionally use the narrower headless-workspace // contract instead of VEGA_EXTRA_PATHS. Both settings name this same @@ -358,7 +424,7 @@ async function runCommand( await Promise.all([mkdir(isolatedHome, { recursive: true }), mkdir(isolatedTmp, { recursive: true })]); const processMarker = `openswarm-verify-${randomUUID()}`; const safeEnv: NodeJS.ProcessEnv = { - PATH: env.PATH, + PATH: buildVerifyToolchainPath(env.PATH, root, cwd), HOME: isolatedHome, USERPROFILE: isolatedHome, XDG_CONFIG_HOME: join(isolatedHome, '.config'),