Merge branch 'v2/main' into v2/fix/2067-proxy-undici
One conflict, in `clients/tui/__tests__/App.test.tsx`: both sides added an `opts` capture to the test harness's `FakeClient` for different fields — `defaultMetadata` on v2/main (#1910), `environment.fetch` here (#2067) — so the resolution is the union of the two, keeping both rationales and both tests. Everything else auto-merged. `npm run ci` passes on the result. Signed-off-by: cliffhall <cliff@futurescale.com>
This commit is contained in:
commit
531cf84e60
5 changed files with 3739 additions and 691 deletions
|
|
@ -25,6 +25,8 @@ import {
|
|||
parseHeaderPair,
|
||||
} from "@inspector/core/mcp/node/index.js";
|
||||
import type { JsonValue } from "@inspector/core/mcp/index.js";
|
||||
import type { StrictJsonValue } from "@inspector/core/json/jsonUtils.js";
|
||||
import { isSerializableJson } from "@inspector/core/json/jsonUtils.js";
|
||||
import {
|
||||
canonicalUrlHost,
|
||||
isAllInterfacesHost,
|
||||
|
|
@ -34,7 +36,6 @@ import { consumeMethodOutcome } from "./handlers/consume-outcome.js";
|
|||
import { runMethod } from "./handlers/run-method.js";
|
||||
import {
|
||||
isOneShotMethod,
|
||||
metaValueToString,
|
||||
ONE_SHOT_METHODS,
|
||||
type MethodArgs,
|
||||
} from "./handlers/method-types.js";
|
||||
|
|
@ -480,8 +481,8 @@ function buildHandoff(
|
|||
|
||||
function parseKeyValuePair(
|
||||
value: string,
|
||||
previous: Record<string, JsonValue> = {},
|
||||
): Record<string, JsonValue> {
|
||||
previous: Record<string, StrictJsonValue> = {},
|
||||
): Record<string, StrictJsonValue> {
|
||||
const parts = value.split("=");
|
||||
const key = parts[0];
|
||||
const val = parts.slice(1).join("=");
|
||||
|
|
@ -492,13 +493,31 @@ function parseKeyValuePair(
|
|||
);
|
||||
}
|
||||
|
||||
let parsedValue: JsonValue;
|
||||
// `StrictJsonValue`: `JSON.parse` cannot produce `undefined`, and these values
|
||||
// become `_meta`, which must reach the wire exactly as written (#1910).
|
||||
let parsedValue: StrictJsonValue;
|
||||
try {
|
||||
parsedValue = JSON.parse(val) as JsonValue;
|
||||
parsedValue = JSON.parse(val) as StrictJsonValue;
|
||||
} catch {
|
||||
// Not JSON at all — a bare word or an unquoted string. Sent as a string,
|
||||
// which is what the user plainly meant.
|
||||
parsedValue = val;
|
||||
}
|
||||
|
||||
// Valid JSON syntax is not the same as sendable JSON: `1e400` parses to
|
||||
// `Infinity`, which `JSON.stringify` writes as `null`. Rejecting is better
|
||||
// than accepting the flag and silently transmitting a different value —
|
||||
// and better than falling back to the literal string, which would also not
|
||||
// be what was asked for.
|
||||
if (!isSerializableJson(parsedValue)) {
|
||||
// Names the key, never the value: the pair can carry a credential
|
||||
// (`credentials={"accessToken":"…","n":1e400}`) and this message lands in
|
||||
// stderr and CI logs.
|
||||
throw new Error(
|
||||
`Invalid value for "${key}": numbers must be finite (a literal like 1e400 overflows to Infinity and cannot be sent).`,
|
||||
);
|
||||
}
|
||||
|
||||
return { ...previous, [key as string]: parsedValue };
|
||||
}
|
||||
|
||||
|
|
@ -742,8 +761,8 @@ async function parseArgs(argv?: string[]): Promise<ParseResult> {
|
|||
promptName?: string;
|
||||
promptArgs?: Record<string, JsonValue>;
|
||||
logLevel?: LoggingLevel;
|
||||
metadata?: Record<string, JsonValue>;
|
||||
toolMetadata?: Record<string, JsonValue>;
|
||||
metadata?: Record<string, StrictJsonValue>;
|
||||
toolMetadata?: Record<string, StrictJsonValue>;
|
||||
cwd?: string;
|
||||
transport?: "sse" | "http" | "stdio";
|
||||
serverUrl?: string;
|
||||
|
|
@ -1005,22 +1024,12 @@ async function parseArgs(argv?: string[]): Promise<ParseResult> {
|
|||
promptName: options.promptName,
|
||||
promptArgs: options.promptArgs,
|
||||
logLevel: options.logLevel,
|
||||
metadata: options.metadata
|
||||
? Object.fromEntries(
|
||||
Object.entries(options.metadata).map(([key, value]) => [
|
||||
key,
|
||||
metaValueToString(value),
|
||||
]),
|
||||
)
|
||||
: undefined,
|
||||
toolMeta: options.toolMetadata
|
||||
? Object.fromEntries(
|
||||
Object.entries(options.toolMetadata).map(([key, value]) => [
|
||||
key,
|
||||
metaValueToString(value),
|
||||
]),
|
||||
)
|
||||
: undefined,
|
||||
// `--metadata`/`--tool-metadata` values are parsed as JSON, and `_meta`
|
||||
// takes any JSON — so they go through unflattened (#1910). They used to be
|
||||
// squeezed through `metaValueToString`, which sent `{"a":1}` as the
|
||||
// *string* `'{"a":1}'`.
|
||||
metadata: options.metadata,
|
||||
toolMeta: options.toolMetadata,
|
||||
appInfo: options.appInfo === true,
|
||||
format: options.format,
|
||||
};
|
||||
|
|
|
|||
|
|
@ -2,6 +2,7 @@ import { InspectorClient } from "@inspector/core/mcp/index.js";
|
|||
import { extractAppInfo } from "@inspector/core/mcp/apps.js";
|
||||
import type { AppInfo } from "@inspector/core/mcp/apps.js";
|
||||
import type { CliAppInfo } from "./method-types.js";
|
||||
import type { RequestMetadata } from "@inspector/core/mcp/types.js";
|
||||
|
||||
/**
|
||||
* Build the CLI's app-info for a tool. Never throws — failures fold into
|
||||
|
|
@ -10,7 +11,7 @@ import type { CliAppInfo } from "./method-types.js";
|
|||
export async function collectAppInfo(
|
||||
client: Pick<InspectorClient, "readResource">,
|
||||
tool: Parameters<typeof extractAppInfo>[0],
|
||||
metadata: Record<string, string> | undefined,
|
||||
metadata: RequestMetadata | undefined,
|
||||
): Promise<CliAppInfo> {
|
||||
let base: AppInfo;
|
||||
try {
|
||||
|
|
|
|||
|
|
@ -22,7 +22,7 @@ export function withConnectTimeout(
|
|||
if (settings) return { ...settings, connectionTimeout };
|
||||
return {
|
||||
headers: [],
|
||||
metadata: [],
|
||||
metadata: {},
|
||||
env: [],
|
||||
connectionTimeout,
|
||||
requestTimeout: 0,
|
||||
|
|
|
|||
|
|
@ -1,4 +1,5 @@
|
|||
import type { JsonValue } from "@inspector/core/mcp/index.js";
|
||||
import type { RequestMetadata } from "@inspector/core/mcp/types.js";
|
||||
import type { AppInfo } from "@inspector/core/mcp/apps.js";
|
||||
import type { LoggingLevel } from "@modelcontextprotocol/client";
|
||||
import type { OutputFormat } from "./format-output.js";
|
||||
|
|
@ -20,8 +21,8 @@ export type MethodArgs = {
|
|||
logLevel?: LoggingLevel;
|
||||
toolName?: string;
|
||||
toolArg?: Record<string, JsonValue>;
|
||||
toolMeta?: Record<string, string>;
|
||||
metadata?: Record<string, string>;
|
||||
toolMeta?: RequestMetadata;
|
||||
metadata?: RequestMetadata;
|
||||
appInfo?: boolean;
|
||||
format?: OutputFormat;
|
||||
/** Task id for tasks/get, tasks/cancel, tasks/result. */
|
||||
|
|
@ -110,13 +111,3 @@ export type OneShotMethod = (typeof ONE_SHOT_METHODS)[number];
|
|||
export function isOneShotMethod(method: string): method is OneShotMethod {
|
||||
return (ONE_SHOT_METHODS as readonly string[]).includes(method);
|
||||
}
|
||||
|
||||
/**
|
||||
* Encode a metadata / tool-metadata value for MethodArgs (string map).
|
||||
* Strings pass through; objects/arrays use JSON.stringify so structure is kept
|
||||
* (avoids String(obj) → "[object Object]").
|
||||
*/
|
||||
export function metaValueToString(value: JsonValue): string {
|
||||
if (typeof value === "string") return value;
|
||||
return JSON.stringify(value);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -157,10 +157,10 @@ export function sanitizeServerSettings(
|
|||
key: h.key,
|
||||
value: isSensitiveHeader(h.key) ? REDACTED : h.value,
|
||||
})),
|
||||
metadata: (settings.metadata ?? []).map((m) => ({
|
||||
key: m.key,
|
||||
value: isSensitiveHeader(m.key) ? REDACTED : m.value,
|
||||
})),
|
||||
// Metadata values may be any JSON (#1910), so the redaction has to descend:
|
||||
// a top-level key that is not itself sensitive can hold a nested one that
|
||||
// is. See `redactMetadataValue`.
|
||||
metadata: redactMetadataValue(settings.metadata ?? {}),
|
||||
env: (settings.env ?? []).map((e) => ({
|
||||
key: e.key,
|
||||
value: REDACTED,
|
||||
|
|
@ -212,6 +212,39 @@ function sanitizeInitRecord(
|
|||
return out;
|
||||
}
|
||||
|
||||
/**
|
||||
* Redact secret-bearing entries anywhere inside a `_meta` payload.
|
||||
*
|
||||
* The pre-#1910 version only had to check the top level, because a value was
|
||||
* always a string — a secret could not hide below one. Now that a value may be
|
||||
* an object or an array, `{ trace: { accessToken: "…" } }` would sail through a
|
||||
* top-level-only check: `trace` is not sensitive, so the whole subtree,
|
||||
* `accessToken` included, would print. `servers/show` output is meant to be
|
||||
* safe to paste into an issue, so the walk has to reach every key.
|
||||
*
|
||||
* A key matching {@link isSensitiveHeader} replaces its **entire** value rather
|
||||
* than being descended into, so a sensitive key holding an object cannot leak
|
||||
* through its members either. Arrays are mapped elementwise: their indices are
|
||||
* not names anyone can mark sensitive, but objects inside them can be.
|
||||
*
|
||||
* Recursion is bounded by the payload, which came from `JSON.parse` and so is a
|
||||
* finite tree with no cycles.
|
||||
*/
|
||||
function redactMetadataValue(value: unknown): unknown {
|
||||
if (Array.isArray(value)) {
|
||||
return value.map(redactMetadataValue);
|
||||
}
|
||||
if (value !== null && typeof value === "object") {
|
||||
return Object.fromEntries(
|
||||
Object.entries(value).map(([key, member]) => [
|
||||
key,
|
||||
isSensitiveHeader(key) ? REDACTED : redactMetadataValue(member),
|
||||
]),
|
||||
);
|
||||
}
|
||||
return value;
|
||||
}
|
||||
|
||||
function isSensitiveHeader(key: string): boolean {
|
||||
const k = key.toLowerCase();
|
||||
return (
|
||||
|
|
|
|||
Loading…
Reference in a new issue