mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-27 19:35:13 +01:00
feat(shared): validate operator-supplied remote MCP headers
The guided "Connect your own MCP server" flow and the paste-config escape hatch both let an operator name arbitrary request headers for an arbitrary endpoint, and those names reach a real outbound fetch. Validate them once in shared code rather than at each call site: RFC 9110 token names only, no hop-by-hop/framing/routing/ambient-credential headers, and no control characters in values. Also add the generic-auth inputs the guided flow needs (`authMode`, a preregistered `oauthClient`) and the auth-challenge shape the wizard branches on, so a URL-only connection can describe what the endpoint asked for. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
1 parent
860eb96c1e
commit
ca0f7bc2eb
8 files changed
+472
-4
No files matched your search
@@ -865,6 +865,9 @@ export {
|
||||
mcpConnectionCredentialRefSchema,
|
||||
createToolApplicationSchema,
|
||||
connectToolAppSchema,
|
||||
GENERIC_MCP_AUTH_MODES,
|
||||
genericMcpAuthModeSchema,
|
||||
genericMcpOAuthClientSchema,
|
||||
reconnectToolAppSchema,
|
||||
finishToolAppSchema,
|
||||
updateToolApplicationSchema,
|
||||
|
||||
@@ -61,6 +61,68 @@ describe("tool access validators", () => {
|
||||
}).success).toBe(false);
|
||||
});
|
||||
|
||||
// PAP-17087: the guided generic flow and paste-config both reach the connect
|
||||
// endpoint, so unsafe header names/values are rejected once at this boundary.
|
||||
it("accepts generic advanced-authentication input for a pasted URL", () => {
|
||||
const parsed = connectToolAppSchema.safeParse({
|
||||
link: "https://mcp.example.test/mcp",
|
||||
authMode: "custom_headers",
|
||||
credentialValues: {
|
||||
"headers.X-Api-Key": "phx_abc123",
|
||||
"headers.X-PostHog-Project-Id": "12345",
|
||||
},
|
||||
});
|
||||
expect(parsed.success).toBe(true);
|
||||
|
||||
const manualClient = connectToolAppSchema.safeParse({
|
||||
link: "https://mcp.example.test/mcp",
|
||||
authMode: "oauth",
|
||||
oauthClient: { clientId: "client-abc", clientSecret: "shhh" },
|
||||
});
|
||||
expect(manualClient.success).toBe(true);
|
||||
});
|
||||
|
||||
it("rejects header credentials Paperclip refuses to send", () => {
|
||||
for (const configPath of ["headers.Host", "headers.Cookie", "headers.Transfer-Encoding", "headers.Sec-Fetch-Mode"]) {
|
||||
const parsed = connectToolAppSchema.safeParse({
|
||||
link: "https://mcp.example.test/mcp",
|
||||
credentialValues: { [configPath]: "value" },
|
||||
});
|
||||
expect(parsed.success, configPath).toBe(false);
|
||||
}
|
||||
});
|
||||
|
||||
it("rejects header names and values that could split the outbound request", () => {
|
||||
const badName = connectToolAppSchema.safeParse({
|
||||
link: "https://mcp.example.test/mcp",
|
||||
credentialValues: { "headers.X-Bad\r\nX-Injected": "value" },
|
||||
});
|
||||
expect(badName.success).toBe(false);
|
||||
|
||||
const badValue = connectToolAppSchema.safeParse({
|
||||
link: "https://mcp.example.test/mcp",
|
||||
credentialValues: { "headers.X-Api-Key": "abc\r\nX-Injected: 1" },
|
||||
});
|
||||
expect(badValue.success).toBe(false);
|
||||
if (!badValue.success) {
|
||||
// The message names the header but must never echo the rejected value.
|
||||
const message = badValue.error.issues[0]?.message ?? "";
|
||||
expect(message).toContain("X-Api-Key");
|
||||
expect(message).not.toContain("X-Injected");
|
||||
}
|
||||
});
|
||||
|
||||
it("keeps generic advanced authentication off the curated gallery path", () => {
|
||||
expect(connectToolAppSchema.safeParse({
|
||||
galleryKey: "posthog",
|
||||
authMode: "bearer",
|
||||
}).success).toBe(false);
|
||||
expect(connectToolAppSchema.safeParse({
|
||||
galleryKey: "posthog",
|
||||
oauthClient: { clientId: "client-abc" },
|
||||
}).success).toBe(false);
|
||||
});
|
||||
|
||||
it("accepts secret references for connection credentials", () => {
|
||||
const parsed = createToolConnectionSchema.safeParse({
|
||||
applicationId: "11111111-1111-4111-8111-111111111111",
|
||||
|
||||
@@ -30,6 +30,12 @@ import {
|
||||
TOOL_RUNTIME_KINDS,
|
||||
TOOL_RUNTIME_SLOT_STATUSES,
|
||||
} from "../constants.js";
|
||||
import {
|
||||
checkMcpRemoteHeaderName,
|
||||
checkMcpRemoteHeaderValue,
|
||||
mcpRemoteHeaderNameFromConfigPath,
|
||||
mcpRemoteHeaderRejectionMessage,
|
||||
} from "../mcp-remote-headers.js";
|
||||
import { jsonSchemaSchema } from "./plugin.js";
|
||||
|
||||
export const toolApplicationTypeSchema = z.enum(TOOL_APPLICATION_TYPES);
|
||||
@@ -258,6 +264,73 @@ export const disableToolStdioCommandTemplateSchema = z.object({
|
||||
|
||||
export type DisableToolStdioCommandTemplate = z.infer<typeof disableToolStdioCommandTemplateSchema>;
|
||||
|
||||
/**
|
||||
* How an operator says a generic remote MCP endpoint authenticates (PAP-17087).
|
||||
*
|
||||
* `auto` is the default and the only value the simple path sends: Paperclip
|
||||
* probes the endpoint and branches on what it finds. The rest are the explicit
|
||||
* choices behind "Advanced authentication", where the operator already knows.
|
||||
*/
|
||||
export const GENERIC_MCP_AUTH_MODES = ["auto", "none", "bearer", "custom_headers", "oauth"] as const;
|
||||
|
||||
export const genericMcpAuthModeSchema = z.enum(GENERIC_MCP_AUTH_MODES);
|
||||
|
||||
export type GenericMcpAuthMode = z.infer<typeof genericMcpAuthModeSchema>;
|
||||
|
||||
/**
|
||||
* A preregistered OAuth client an operator pasted in because the authorization
|
||||
* server supports neither CIMD nor dynamic registration. The secret is write-only:
|
||||
* it becomes a Paperclip secret ref and is never read back.
|
||||
*/
|
||||
export const genericMcpOAuthClientSchema = z.object({
|
||||
clientId: z.string().trim().min(1).max(4096),
|
||||
clientSecret: z.string().min(1).max(16384).optional(),
|
||||
}).strict();
|
||||
|
||||
export type GenericMcpOAuthClient = z.infer<typeof genericMcpOAuthClientSchema>;
|
||||
|
||||
/**
|
||||
* Reject `headers.*` credential paths whose header name Paperclip refuses to
|
||||
* send, and any value that could split the outbound request. This runs at the
|
||||
* API boundary so both the guided wizard and normalized paste-config go through
|
||||
* exactly one gate; the service re-checks when it projects the headers.
|
||||
*/
|
||||
function rejectUnsafeHeaderCredentials(
|
||||
credentialValues: Record<string, string>,
|
||||
ctx: z.RefinementCtx,
|
||||
path: (string | number)[],
|
||||
) {
|
||||
for (const [configPath, value] of Object.entries(credentialValues)) {
|
||||
const headerName = mcpRemoteHeaderNameFromConfigPath(configPath);
|
||||
if (configPath.startsWith("headers.") && !headerName) {
|
||||
ctx.addIssue({
|
||||
code: z.ZodIssueCode.custom,
|
||||
path: [...path, configPath],
|
||||
message: "Header names cannot be blank.",
|
||||
});
|
||||
continue;
|
||||
}
|
||||
if (!headerName) continue;
|
||||
const nameCheck = checkMcpRemoteHeaderName(headerName);
|
||||
if (!nameCheck.ok) {
|
||||
ctx.addIssue({
|
||||
code: z.ZodIssueCode.custom,
|
||||
path: [...path, configPath],
|
||||
message: mcpRemoteHeaderRejectionMessage(headerName, nameCheck.reason!),
|
||||
});
|
||||
continue;
|
||||
}
|
||||
const valueCheck = checkMcpRemoteHeaderValue(value);
|
||||
if (!valueCheck.ok) {
|
||||
ctx.addIssue({
|
||||
code: z.ZodIssueCode.custom,
|
||||
path: [...path, configPath],
|
||||
message: mcpRemoteHeaderRejectionMessage(headerName, valueCheck.reason!),
|
||||
});
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
export const connectToolAppSchema = z.object({
|
||||
galleryKey: z.string().trim().min(1).max(120).optional(),
|
||||
connectionMethodKey: z.string().trim().min(1).max(120).optional(),
|
||||
@@ -266,8 +339,25 @@ export const connectToolAppSchema = z.object({
|
||||
credentialValues: z.record(z.string().trim().min(1).max(200), z.string().min(1)).optional(),
|
||||
configValues: z.record(z.string().trim().min(1).max(200), z.unknown()).optional(),
|
||||
applicationId: z.string().uuid().optional(),
|
||||
authMode: genericMcpAuthModeSchema.optional(),
|
||||
oauthClient: genericMcpOAuthClientSchema.optional(),
|
||||
}).superRefine((value, ctx) => {
|
||||
if (value.configValues) rejectSensitiveConfigKeys(value.configValues, ctx, ["configValues"]);
|
||||
if (value.credentialValues) rejectUnsafeHeaderCredentials(value.credentialValues, ctx, ["credentialValues"]);
|
||||
if (value.authMode && value.galleryKey) {
|
||||
ctx.addIssue({
|
||||
code: z.ZodIssueCode.custom,
|
||||
path: ["authMode"],
|
||||
message: "Authentication mode selection applies to a pasted URL, not a gallery app",
|
||||
});
|
||||
}
|
||||
if (value.oauthClient && value.galleryKey) {
|
||||
ctx.addIssue({
|
||||
code: z.ZodIssueCode.custom,
|
||||
path: ["oauthClient"],
|
||||
message: "Preregistered OAuth client credentials apply to a pasted URL, not a gallery app",
|
||||
});
|
||||
}
|
||||
}).refine(
|
||||
(value) => Boolean(value.galleryKey) !== Boolean(value.link),
|
||||
{ message: "Provide exactly one of galleryKey or link" },
|
||||
|
||||
Reference in new issue
Block a user