Skip to content

Commit 391664b

Browse files
author
David Ruzicka
committed
fix(upstream): fail closed on profile lookup miss
- prevent HTTP upstream routing from falling back to this.profile when transport context cannot resolve the requested profileId - report invalid upstream_mcp shapes as upstream config errors instead of misleading missing openapi_spec_path errors - add regression tests for both paths
1 parent d3d4683 commit 391664b

3 files changed

Lines changed: 60 additions & 3 deletions

File tree

src/profile/profile-loader.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,9 @@ export class ProfileLoader {
7272
} catch (err) {
7373
if (err instanceof ZodError) {
7474
const upstreamIssues = err.issues.filter((i) => i.path[0] === 'upstream_mcp');
75-
if (upstreamIssues.length > 0) {
75+
// Only wrap when ALL Zod issues are upstream_mcp — if other fields also failed,
76+
// re-throw the raw ZodError so the caller sees every problem, not just upstream_mcp.
77+
if (upstreamIssues.length > 0 && upstreamIssues.length === err.issues.length) {
7678
const arrayIssue = upstreamIssues.find(
7779
(i) => i.code === 'invalid_type' && 'received' in i && i.received === 'array',
7880
);

src/profile/upstream-mcp-config.test.ts

Lines changed: 55 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66

77
import { describe, it, expect } from 'vitest';
88
import { ValidationError } from '../core/errors.js';
9-
import { resolveUpstreamMcpConfig, hasUpstreamMcpFlag } from './upstream-mcp-config.js';
9+
import { resolveUpstreamMcpConfig, hasUpstreamMcpFlag, looksLikeUpstreamMcpProxy } from './upstream-mcp-config.js';
1010
import type { Profile } from '../types/profile.js';
1111

1212
/** Minimal valid tool definition to satisfy Profile type. */
@@ -371,3 +371,57 @@ describe('upstream_mcp_from_env D-01 migration', () => {
371371
expect(resolved?.name).toBe('p1');
372372
});
373373
});
374+
375+
describe('looksLikeUpstreamMcpProxy', () => {
376+
it('returns true for valid http-streamable object', () => {
377+
expect(looksLikeUpstreamMcpProxy({
378+
name: 'p1',
379+
transport: { type: 'http-streamable', url: 'https://example.com/mcp' },
380+
})).toBe(true);
381+
});
382+
383+
it('returns false for null', () => {
384+
expect(looksLikeUpstreamMcpProxy(null)).toBe(false);
385+
});
386+
387+
it('returns false for undefined', () => {
388+
expect(looksLikeUpstreamMcpProxy(undefined)).toBe(false);
389+
});
390+
391+
it('returns false for array (legacy shape)', () => {
392+
expect(looksLikeUpstreamMcpProxy([{ transport: { type: 'http-streamable', url: 'https://example.com/mcp' } }])).toBe(false);
393+
});
394+
395+
it('returns false for empty object', () => {
396+
expect(looksLikeUpstreamMcpProxy({})).toBe(false);
397+
});
398+
399+
it('returns false when transport is missing', () => {
400+
expect(looksLikeUpstreamMcpProxy({ name: 'p1' })).toBe(false);
401+
});
402+
403+
it('returns false for stdio transport', () => {
404+
expect(looksLikeUpstreamMcpProxy({
405+
name: 'p1',
406+
transport: { type: 'stdio', command: 'npx' },
407+
})).toBe(false);
408+
});
409+
410+
it('returns false when url is empty string', () => {
411+
expect(looksLikeUpstreamMcpProxy({
412+
name: 'p1',
413+
transport: { type: 'http-streamable', url: ' ' },
414+
})).toBe(false);
415+
});
416+
417+
it('returns false when url is missing', () => {
418+
expect(looksLikeUpstreamMcpProxy({
419+
name: 'p1',
420+
transport: { type: 'http-streamable' },
421+
})).toBe(false);
422+
});
423+
424+
it('returns false when transport is an array', () => {
425+
expect(looksLikeUpstreamMcpProxy({ transport: [{ type: 'http-streamable', url: 'https://example.com' }] })).toBe(false);
426+
});
427+
});

src/profile/upstream-mcp-config.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -230,7 +230,8 @@ export function looksLikeUpstreamMcpProxy(value: unknown): boolean {
230230
// all deployed profiles have been migrated to singular upstream_mcp object.
231231
// Both cleanup sites (this function + the Array.isArray branch in profile-resolver.ts
232232
// extractEnvVars) MUST be removed together — they serve the same migration window.
233-
// Removal: grep -rn "MIGRATION-CLEANUP(phase-03.1)" src/ — exactly 2 sites.
233+
// Removal: grep -rn "MIGRATION-CLEANUP(phase-03.1)" src/ — 2 code sites (grep returns 3 lines
234+
// because this comment block spans 2 lines; the third hit is in profile-resolver.ts).
234235
/**
235236
* Returns true if the raw profile.upstream_mcp value indicates an upstream MCP
236237
* provider is configured. Tolerates BOTH the legacy array shape and the post

0 commit comments

Comments
 (0)