mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-10 20:50:08 +02:00
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Apps give those agents governed access to external tools. > - Remote MCP setup needs secure endpoint validation and durable credentials. > - PostHog needs both browser sign-in and personal API key setup paths. > - This pull request adds the shared remote MCP foundation and the PostHog definition. > - The benefit is a secure and reusable base for later app connection work. ## Linked Issues or Issue Description Refs #11965 This is stack 1 of 11. It replaces the first reviewable part of #11965. ## What Changed - Add guarded remote MCP setup and credential handling. - Add PostHog OAuth and API key connection methods. - Add focused server, shared contract, and UI coverage. - Keep the migration replay-safe and idempotent. - Give the late-close security regression the same 10-second CI headroom as the adjacent real-timer handshake test. - Synchronize fake-timer handshake tests at the exact ensure-session boundary so real filesystem setup cannot race the fake deadline. - Drive PTY overflow coverage only after listener registration so scheduling cannot reorder the test fixture. ## Verification - pnpm exec vitest run packages/adapter-utils/src/acpx-engine/execute.test.ts server/src/__tests__/plugin-worker-manager.test.ts (220 passed; affected cases also passed five focused stress repetitions) - `pnpm exec vitest run packages/adapter-utils/src/acpx-engine/execute.test.ts -t "never leaks a sandbox-provided value from a late close rejection into logs or the result"` (1 passed) - `pnpm exec vitest run packages/adapter-utils/src/acpx-engine/execute.test.ts -t "never promotes a late ensureSession resolution|closes a late-resolving real handle exactly once"` (2 passed) - `pnpm -r typecheck` - `pnpm --filter @paperclipai/server exec vitest run src/__tests__/tool-access-service.test.ts` - `pnpm --filter @paperclipai/db check:migrations` - `pnpm build` ## Risks - Remote endpoint validation can reject configurations that previously passed without checks. - OAuth configuration errors can block setup until the operator corrects the provider settings. - The migration uses guarded statements so repeated execution is safe. - The test-only synchronization changes do not affect runtime behavior; they remove filesystem/fake-clock and listener-registration races observed under parallel CI load. > I checked `ROADMAP.md`. This stack continues the existing app connection work from #11965 and does not duplicate another planned item. ## Model Used OpenAI Codex, GPT-5. The runtime model ID and context window were not exposed. The model used reasoning, tool use, and code execution. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing>
149 lines
5.3 KiB
TypeScript
149 lines
5.3 KiB
TypeScript
/**
|
|
* Header-name/value safety for user-supplied remote MCP credentials (PAP-17087).
|
|
*
|
|
* Both the guided "Connect your own MCP server" flow and the paste-config escape
|
|
* hatch let an operator name arbitrary request headers for an arbitrary endpoint.
|
|
* Those names reach a real outbound `fetch`, so they are validated here — once,
|
|
* in shared code — instead of at each call site:
|
|
*
|
|
* - only RFC 9110 `token` characters, so a name can never smuggle a separator,
|
|
* whitespace, or CR/LF into the request line;
|
|
* - never a hop-by-hop, framing, routing, or ambient-credential header, because
|
|
* those either belong to the transport or would let a pasted config redirect
|
|
* the request or attach a browser cookie;
|
|
* - values must stay printable single-line, which blocks header/response
|
|
* splitting through a value that carries `\r\n`.
|
|
*
|
|
* `Authorization` is deliberately allowed: it is the header the bearer-key path
|
|
* uses, and its value is stored as a Paperclip secret like every other one.
|
|
*/
|
|
|
|
/** RFC 9110 field-name = token. */
|
|
const HTTP_TOKEN_PATTERN = /^[!#$%&'*+\-.^_`|~0-9A-Za-z]+$/;
|
|
|
|
const MAX_HEADER_NAME_LENGTH = 128;
|
|
const MAX_HEADER_VALUE_LENGTH = 8_192;
|
|
|
|
/**
|
|
* Headers Paperclip refuses to project from a user-supplied config.
|
|
*
|
|
* `connection`/`keep-alive`/`te`/`trailer`/`transfer-encoding`/`upgrade` are
|
|
* hop-by-hop (RFC 9110 §7.6.1) and belong to the fetch implementation.
|
|
* `content-length`/`host` frame and route the request. `cookie` would attach
|
|
* ambient browser-style credentials that Paperclip cannot scope or rotate.
|
|
* `proxy-*` targets an intermediary rather than the MCP server.
|
|
*/
|
|
const FORBIDDEN_HEADER_NAMES = new Set([
|
|
"connection",
|
|
"content-length",
|
|
"cookie",
|
|
"cookie2",
|
|
"expect",
|
|
"host",
|
|
"keep-alive",
|
|
"proxy-authenticate",
|
|
"proxy-authorization",
|
|
"proxy-connection",
|
|
"set-cookie",
|
|
"set-cookie2",
|
|
"te",
|
|
"trailer",
|
|
"transfer-encoding",
|
|
"upgrade",
|
|
"via",
|
|
]);
|
|
|
|
/** Prefixes reserved for the transport or for intermediaries. */
|
|
const FORBIDDEN_HEADER_PREFIXES = ["proxy-", "sec-", "http2-"];
|
|
|
|
export type McpRemoteHeaderRejection =
|
|
| "empty"
|
|
| "too_long"
|
|
| "invalid_characters"
|
|
| "forbidden"
|
|
| "value_too_long"
|
|
| "value_control_characters";
|
|
|
|
export interface McpRemoteHeaderCheck {
|
|
ok: boolean;
|
|
reason?: McpRemoteHeaderRejection;
|
|
}
|
|
|
|
const OK: McpRemoteHeaderCheck = { ok: true };
|
|
|
|
/**
|
|
* Is `name` a header Paperclip is willing to send on a user-configured remote
|
|
* MCP request? Returns the specific rejection reason so callers can produce an
|
|
* actionable, UI-safe message.
|
|
*/
|
|
export function checkMcpRemoteHeaderName(name: string): McpRemoteHeaderCheck {
|
|
const trimmed = name.trim();
|
|
if (!trimmed) return { ok: false, reason: "empty" };
|
|
if (trimmed.length > MAX_HEADER_NAME_LENGTH) return { ok: false, reason: "too_long" };
|
|
if (!HTTP_TOKEN_PATTERN.test(trimmed)) return { ok: false, reason: "invalid_characters" };
|
|
const lower = trimmed.toLowerCase();
|
|
if (FORBIDDEN_HEADER_NAMES.has(lower)) return { ok: false, reason: "forbidden" };
|
|
if (FORBIDDEN_HEADER_PREFIXES.some((prefix) => lower.startsWith(prefix))) {
|
|
return { ok: false, reason: "forbidden" };
|
|
}
|
|
return OK;
|
|
}
|
|
|
|
/**
|
|
* Is `value` safe to send as a header value? Rejects CR/LF and other control
|
|
* characters (header splitting) and absurdly long values.
|
|
*/
|
|
export function checkMcpRemoteHeaderValue(value: string): McpRemoteHeaderCheck {
|
|
if (value.length > MAX_HEADER_VALUE_LENGTH) return { ok: false, reason: "value_too_long" };
|
|
// Reject C0/C1 controls and DEL. A tab is legal in a field value per RFC 9110
|
|
// but has no legitimate use in a credential, so it is rejected too.
|
|
if (/[\u0000-\u001f\u007f-\u009f]/.test(value)) {
|
|
return { ok: false, reason: "value_control_characters" };
|
|
}
|
|
return OK;
|
|
}
|
|
|
|
export function isSafeMcpRemoteHeaderName(name: string): boolean {
|
|
return checkMcpRemoteHeaderName(name).ok;
|
|
}
|
|
|
|
export function isSafeMcpRemoteHeaderValue(value: string): boolean {
|
|
return checkMcpRemoteHeaderValue(value).ok;
|
|
}
|
|
|
|
/** A UI-safe explanation for a rejected header. Never echoes the value. */
|
|
export function mcpRemoteHeaderRejectionMessage(
|
|
headerName: string,
|
|
reason: McpRemoteHeaderRejection,
|
|
): string {
|
|
switch (reason) {
|
|
case "empty":
|
|
return "Header names cannot be blank.";
|
|
case "too_long":
|
|
return `Header name "${headerName.slice(0, MAX_HEADER_NAME_LENGTH)}" is too long.`;
|
|
case "invalid_characters":
|
|
return `"${headerName.slice(0, MAX_HEADER_NAME_LENGTH)}" is not a valid header name. Use letters, digits, and dashes.`;
|
|
case "forbidden":
|
|
return `Paperclip manages the "${headerName}" header and cannot send a custom value for it.`;
|
|
case "value_too_long":
|
|
return `The value for "${headerName}" is too long.`;
|
|
case "value_control_characters":
|
|
return `The value for "${headerName}" contains line breaks or control characters.`;
|
|
}
|
|
}
|
|
|
|
/**
|
|
* `credentialValues` keys use a `headers.<Name>` config path. Extract the header
|
|
* name, or `null` when the path is not a header path.
|
|
*/
|
|
export function mcpRemoteHeaderNameFromConfigPath(configPath: string): string | null {
|
|
if (!configPath.startsWith("headers.")) return null;
|
|
const name = configPath.slice("headers.".length).trim();
|
|
return name.length > 0 ? name : null;
|
|
}
|
|
|
|
export const MCP_REMOTE_HEADER_LIMITS = {
|
|
maxNameLength: MAX_HEADER_NAME_LENGTH,
|
|
maxValueLength: MAX_HEADER_VALUE_LENGTH,
|
|
} as const;
|