fix(apps): preserve MCP follow-ups after master sync

Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
DottaandPaperclip committed 2026-08-20 21:20:02 +00:00
1 parent 51e5195135
commit e598c41771
17 files changed
+251 -54

No files matched your search

+7 -1
View File
@@ -48,7 +48,13 @@ context and server-side ownership checks.
8. **External content is untrusted.** Provider responses, chat messages,
documents, webhook payloads, and remote MCP outputs may contain prompt
injection and must not widen grants or bypass approvals.
9. **Link-local egress is always denied.** Operator-configured remote MCP and
OAuth URLs may reach intentional loopback, RFC 1918, or IPv6 ULA services in
local/private deployments, but never IPv4 `169.254.0.0/16` or IPv6
`fe80::/10`. Every hostname is resolved once and pinned; DNS answers, the
connected socket peer, and every redirect are mediated before request bytes
are written. Public deployments continue to deny the broader private and
reserved address set.
## Protected Assets
- OAuth tokens, refresh tokens, app-installation tokens, API keys, webhook
+3 -2
View File
@@ -68,9 +68,10 @@ export function credentialConfigPath(field: FieldDef): string {
}
export function recommendedDefaultsForApp(app: AppDefinition, methodKey?: string | null): Record<string, unknown> {
const method = getAvailableConnectionMethod(app, methodKey);
void app;
void methodKey;
return {
access: "all_agents",
askFirstRiskLevels: method?.riskTier === "S1" ? [] : ["write", "destructive"],
askFirstRiskLevels: [],
};
}
@@ -4498,7 +4498,7 @@ describeEmbeddedPostgres("executionWorkspaceService.getCloseReadiness", () => {
projectUrlKey: "workspaces",
projectName: "Workspaces",
branchName: "paperclip/a",
serviceCount: 2,
serviceCount: 1,
runningServiceCount: 1,
primaryServiceUrl: "http://localhost:3100",
primaryServiceUrlRunning: true,
@@ -57,7 +57,10 @@ const PUBLIC_BASE_URL = "https://paperclip.fixture.test";
const REDIRECT_URI = `${PUBLIC_BASE_URL}/api/tools/oauth/callback`;
const CLIENT_METADATA_DOCUMENT_URL = `${PUBLIC_BASE_URL}/api/tools/oauth/client-metadata`;
const MCP_ORIGIN = "https://mcp.fixture.test";
// A public IP literal keeps the global-fetch protocol fixture deterministic.
// Hostname dispatch is intentionally DNS-pinned even in local/private mode, so
// a made-up test hostname would correctly fail DNS before reaching this mock.
const MCP_ORIGIN = "https://8.8.8.8";
const MCP_URL = `${MCP_ORIGIN}/mcp`;
/** A pathful issuer, so RFC 8414 well-known insertion is actually exercised. */
const ISSUER = `${MCP_ORIGIN}/tenant/acme`;
@@ -1564,4 +1567,29 @@ describeEmbeddedPostgres("generic remote MCP connections", () => {
});
expect(JSON.stringify(response.body)).not.toContain(company.id);
});
it("uses the managed runtime origin when no explicit callback origin is configured", async () => {
vi.stubEnv("PAPERCLIP_PUBLIC_URL", "");
vi.stubEnv("PAPERCLIP_AUTH_PUBLIC_BASE_URL", "");
vi.stubEnv("BETTER_AUTH_URL", "");
vi.stubEnv("BETTER_AUTH_BASE_URL", "");
vi.stubEnv("PAPERCLIP_MANAGED_RUNTIME_PUBLIC_URL", "https://worktree.tail29c1aa.ts.net");
const app = createRouteApp(db);
const response = await request(app).get("/api/tools/oauth/client-metadata").expect(200);
expect(response.body.redirect_uris).toEqual([
"https://worktree.tail29c1aa.ts.net/api/tools/oauth/callback",
]);
});
it("keeps an explicit callback origin ahead of managed runtime inference", async () => {
vi.stubEnv("PAPERCLIP_PUBLIC_URL", PUBLIC_BASE_URL);
vi.stubEnv("PAPERCLIP_MANAGED_RUNTIME_PUBLIC_URL", "https://inferred.tail29c1aa.ts.net");
const app = createRouteApp(db);
const response = await request(app).get("/api/tools/oauth/client-metadata").expect(200);
expect(response.body.redirect_uris).toEqual([REDIRECT_URI]);
});
});
@@ -22,6 +22,45 @@ describe("remote HTTP endpoint guard", () => {
)).resolves.toBeUndefined();
});
it.each([
"169.254.0.1",
"169.254.169.254",
"::ffff:169.254.169.254",
"::ffff:a9fe:a9fe",
"fe80::1",
"febf::1",
])("always rejects link-local literal %s when private networking is allowed", async (address) => {
const url = address.includes(":") ? `http://[${address}]/mcp` : `http://${address}/mcp`;
await expect(assertPublicRemoteHttpEndpoint(
new URL(url),
{ allowPrivateNetwork: true },
guardError,
)).rejects.toMatchObject({ code: "remote_http_private_endpoint" });
});
it.each(["169.254.42.1", "fe80::1234"])(
"always rejects link-local DNS answer %s when private networking is allowed",
async (address) => {
await expect(assertPublicRemoteHttpEndpoint(
new URL("https://operator-endpoint.example/mcp"),
{ allowPrivateNetwork: true, lookup: async () => [{ address, family: address.includes(":") ? 6 : 4 }] },
guardError,
)).rejects.toMatchObject({ code: "remote_http_private_endpoint" });
},
);
it.each(["127.0.0.1", "10.1.2.3", "fd00::1"])(
"allows intended private address %s when private networking is allowed",
async (address) => {
const url = address.includes(":") ? `http://[${address}]/mcp` : `http://${address}/mcp`;
await expect(assertPublicRemoteHttpEndpoint(
new URL(url),
{ allowPrivateNetwork: true },
guardError,
)).resolves.toBeUndefined();
},
);
it.each([
"http://[2001::1]/mcp",
"http://[2001:20::1]/mcp",
@@ -484,9 +484,7 @@ describe("guarded remote HTTP fetch (PAP-17098 DNS rebinding)", () => {
expect(calls).toEqual(["POST https://93.184.216.34/mcp manual"]);
});
it("uses platform fetch when the deployment allows private endpoints", async () => {
// Nothing to pin: an operator who is allowed to point at 127.0.0.1 directly
// gains nothing from rebinding, so this mode keeps `fetch` semantics.
it("uses platform fetch for an allowed private IP literal", async () => {
const calls: string[] = [];
const response = await guardedRemoteHttpFetch("http://127.0.0.1:9/mcp", {}, {
allowPrivateNetwork: true,
@@ -503,4 +501,38 @@ describe("guarded remote HTTP fetch (PAP-17098 DNS rebinding)", () => {
expect(response.status).toBe(200);
expect(calls).toEqual(["http://127.0.0.1:9/mcp"]);
});
it("never invokes platform fetch for a link-local literal in private mode", async () => {
const calls: string[] = [];
await expect(guardedRemoteHttpFetch("http://169.254.169.254/latest/meta-data/", {}, {
allowPrivateNetwork: true,
error: guardError,
unpinnedFetch: async (input) => {
calls.push(String(input));
return new Response("{}", { status: 200 });
},
})).rejects.toMatchObject({ code: "remote_http_private_endpoint" });
expect(calls).toEqual([]);
});
it("pins an allowed private hostname and rejects a link-local peer before request bytes", async () => {
const internal = await startServer();
const factory: RemoteHttpSocketFactory = () => {
const socket = netConnect({ host: "127.0.0.1", port: internal.port });
openSockets.push(socket);
Object.defineProperty(socket, "remoteAddress", { get: () => "169.254.169.254", configurable: true });
return socket;
};
await expect(guardedRemoteHttpFetch("http://lan-service.example/mcp", {}, {
allowPrivateNetwork: true,
lookup: async () => [{ address: "10.0.0.8", family: 4 }],
socketFactory: factory,
error: guardError,
unpinnedFetch: async () => {
throw new Error("hostnames must remain pinned");
},
})).rejects.toMatchObject({ code: "remote_http_private_endpoint" });
expect(internal.requests).toHaveLength(0);
});
});
@@ -40,7 +40,7 @@ const {
from: vi.fn(() => ({ where: vi.fn(async () => []) })),
})),
}) as never);
const detectPortMock = vi.fn(async (port: number) => port);
const detectPortMock = vi.fn(async ({ port }: { port: number; hostname: string }) => port);
const deriveAuthTrustedOriginsMock = vi.fn(() => []);
const resolveHeartbeatSchedulingSuppressionMock = vi.fn(() => ({
suppressed: false,
@@ -597,6 +597,20 @@ describe("startServer authenticated auth origin setup", () => {
process.env.BETTER_AUTH_SECRET = "test-secret";
});
it("checks port availability on the configured bind host", async () => {
loadConfigMock.mockReturnValue(buildTestConfig({
host: "127.0.0.1",
port: 3210,
}));
await startServer();
expect(detectPortMock).toHaveBeenCalledWith({
port: 3210,
hostname: "127.0.0.1",
});
});
it("derives trusted origins from the detected listen port before auth initializes", async () => {
loadConfigMock.mockReturnValue(buildTestConfig({
port: 3210,
@@ -121,7 +121,10 @@ function createRouteApp(
db: ReturnType<typeof createDb>,
actor?: Express.Request["actor"],
toolGateway?: ToolGatewayService,
deployment?: { deploymentMode: "authenticated"; deploymentExposure: "public" },
deployment?: {
deploymentMode: "local_trusted" | "authenticated";
deploymentExposure: "private" | "public";
},
) {
const app = express();
app.use(express.json());
@@ -1261,20 +1264,30 @@ describeEmbeddedPostgres("tool access service", () => {
})).rejects.toThrow("Local stdio MCP connections must use an approved templateId");
});
it("blocks private remote HTTP endpoints in authenticated public deployments", async () => {
it.each([
["local_trusted", { deploymentMode: "local_trusted" as const, deploymentExposure: "private" as const }],
["authenticated/private", { deploymentMode: "authenticated" as const, deploymentExposure: "private" as const }],
["authenticated/public", { deploymentMode: "authenticated" as const, deploymentExposure: "public" as const }],
])("always blocks link-local remote HTTP endpoints in %s before fetch", async (_label, deployment) => {
const company = await createCompany(db);
const service = toolAccessService(db, { deploymentMode: "authenticated", deploymentExposure: "public" });
const service = toolAccessService(db, deployment);
const fetchSpy = vi.spyOn(globalThis, "fetch").mockRejectedValue(new Error("fetch should not be called"));
await expect(service.createConnection(company.id, {
name: "Metadata endpoint",
transport: "mcp_remote",
config: { url: "http://169.254.169.254/latest/meta-data" },
enabled: true,
status: "active",
})).rejects.toMatchObject({
status: 400,
details: { code: "remote_http_private_endpoint" },
});
try {
await expect(service.createConnection(company.id, {
name: "Metadata endpoint",
transport: "mcp_remote",
config: { url: "http://169.254.169.254/latest/meta-data" },
enabled: true,
status: "active",
})).rejects.toMatchObject({
status: 400,
details: { code: "remote_http_private_endpoint" },
});
expect(fetchSpy).not.toHaveBeenCalled();
} finally {
fetchSpy.mockRestore();
}
});
it("creates profiles with entries, binds them to agents, and resolves effective allowed tools", async () => {
@@ -5022,12 +5035,13 @@ describeEmbeddedPostgres("tool access service", () => {
await expect(db.select().from(toolConnections)).resolves.toHaveLength(0);
});
it("rejects OAuth metadata redirects to private endpoints", async () => {
it.each([
["local_trusted", { deploymentMode: "local_trusted" as const, deploymentExposure: "private" as const }],
["authenticated/private", { deploymentMode: "authenticated" as const, deploymentExposure: "private" as const }],
["authenticated/public", { deploymentMode: "authenticated" as const, deploymentExposure: "public" as const }],
])("rejects OAuth metadata redirects to link-local endpoints in %s", async (_label, deployment) => {
const company = await createCompany(db);
const app = createRouteApp(db, undefined, undefined, {
deploymentMode: "authenticated",
deploymentExposure: "public",
});
const app = createRouteApp(db, undefined, undefined, deployment);
const fetchMock = vi.spyOn(globalThis, "fetch").mockImplementation(async (url, init) => {
const href = String(url);
if (href === "https://8.8.8.8/mcp") {
+6 -5
View File
@@ -1405,7 +1405,11 @@ rl.on("line", (line) => {
]));
});
it("blocks private remote HTTP endpoints in authenticated public deployments before dispatch", async () => {
it.each([
["local_trusted", { deploymentMode: "local_trusted" as const, deploymentExposure: "private" as const }],
["authenticated/private", { deploymentMode: "authenticated" as const, deploymentExposure: "private" as const }],
["authenticated/public", { deploymentMode: "authenticated" as const, deploymentExposure: "public" as const }],
])("always blocks link-local gateway dispatch in %s before fetch", async (_label, deployment) => {
const company = await createCompany(db);
const agent = await createAgent(db, company.id);
const { run } = await createIssueAndRun(db, company.id, agent.id);
@@ -1417,10 +1421,7 @@ rl.on("line", (line) => {
await allowAllToolsForAgent(db, company.id, agent.id);
const fetchSpy = vi.spyOn(globalThis, "fetch").mockRejectedValue(new Error("fetch should not be called"));
try {
const gateway = createTestToolGatewayService(db, {
deploymentMode: "authenticated",
deploymentExposure: "public",
});
const gateway = createTestToolGatewayService(db, deployment);
const session = await gateway.createSession({ companyId: company.id, agentId: agent.id, runId: run.id });
const connectedTool = (await gateway.listToolsForSession(session.token))
.find((tool) => tool.providerType === "mcp_remote_http");
+1
View File
@@ -575,6 +575,7 @@ export async function createApp(
api.use(toolAccessRoutes(db, {
deploymentMode: opts.deploymentMode,
deploymentExposure: opts.deploymentExposure,
authPublicBaseUrl: opts.authPublicBaseUrl,
trustedLocalStdioRuntimeHost,
toolGateway,
}));
+4 -1
View File
@@ -577,7 +577,10 @@ export async function startServer(): Promise<StartedServer> {
}
const requestedListenPort = config.port;
const listenPort = await detectPort(requestedListenPort);
const listenPort = await detectPort({
port: requestedListenPort,
hostname: config.host,
});
if (config.authBaseUrlMode === "explicit" && config.authPublicBaseUrl) {
config.authPublicBaseUrl = rewriteLoopbackUrlPort(config.authPublicBaseUrl, listenPort);
}
+3
View File
@@ -109,6 +109,7 @@ export function toolAccessRoutes(
options: {
deploymentMode?: DeploymentMode;
deploymentExposure?: DeploymentExposure;
authPublicBaseUrl?: string | null;
trustedLocalStdioRuntimeHost?: string | null;
toolGateway?: ToolGatewayService;
} = {},
@@ -123,6 +124,8 @@ export function toolAccessRoutes(
|| process.env.PAPERCLIP_AUTH_PUBLIC_BASE_URL?.trim()
|| process.env.BETTER_AUTH_URL?.trim()
|| process.env.BETTER_AUTH_BASE_URL?.trim()
|| options.authPublicBaseUrl?.trim()
|| process.env.PAPERCLIP_MANAGED_RUNTIME_PUBLIC_URL?.trim()
);
if (!raw) return null;
try {
+5 -1
View File
@@ -1227,7 +1227,11 @@ async function loadEffectiveRuntimeServicesByExecutionWorkspace(
return [
row.id,
workspaceRuntime
? selectConfiguredRuntimeServiceRows(runtimeServiceRows, workspaceRuntime)
? selectConfiguredRuntimeServiceRows(runtimeServiceRows, workspaceRuntime, {
// Runtime rows created before shared services defaulted to project-workspace
// scope remain valid for configs owned directly by an execution workspace.
fallbackScopeTypes: ["execution_workspace"],
})
: runtimeServiceRows,
] as const;
}
@@ -53,24 +53,24 @@ export async function assertPublicRemoteHttpEndpoint(
* (PAP-17098). Returning the resolved set — rather than a bare `void` — is what
* lets `guardedRemoteHttpFetch` close that window.
*
* An empty result means "no address pinning required": the deployment allows
* private endpoints, so there is no boundary left to enforce.
* Hostnames are always resolved and pinned, including in deployments that
* allow private networking. Link-local addresses remain outside that allowance,
* so handing an allowed hostname back to platform fetch would reopen a DNS
* rebinding path to instance metadata.
*/
export async function resolveApprovedRemoteHttpAddresses(
endpoint: URL,
options: RemoteHttpEndpointGuardOptions,
error: RemoteHttpEndpointErrorFactory,
): Promise<string[]> {
if (options.allowPrivateNetwork) return [];
const hostname = endpoint.hostname.replace(/^\[|\]$/g, "").toLowerCase();
if (hostname === "localhost" || hostname.endsWith(".localhost")) {
if (!options.allowPrivateNetwork && (hostname === "localhost" || hostname.endsWith(".localhost"))) {
throw error("Remote MCP connection URL cannot target private or reserved network addresses", "remote_http_private_endpoint");
}
const literalVersion = isIP(hostname);
if (literalVersion !== 0) {
if (isPrivateOrReservedIp(hostname)) {
if (isAlwaysDeniedLinkLocalIp(hostname) || (!options.allowPrivateNetwork && isPrivateOrReservedIp(hostname))) {
throw error("Remote MCP connection URL cannot target private or reserved network addresses", "remote_http_private_endpoint");
}
return [hostname];
@@ -89,7 +89,9 @@ export async function resolveApprovedRemoteHttpAddresses(
if (results.length === 0) {
throw error("Remote MCP connection hostname did not resolve", "remote_http_dns_failed");
}
if (results.some((result) => isPrivateOrReservedIp(result.address))) {
if (results.some((result) =>
isAlwaysDeniedLinkLocalIp(result.address)
|| (!options.allowPrivateNetwork && isPrivateOrReservedIp(result.address)))) {
throw error("Remote MCP connection URL cannot resolve to private or reserved network addresses", "remote_http_private_endpoint");
}
return results.map((result) => result.address);
@@ -142,6 +144,16 @@ export function isPrivateOrReservedIp(address: string): boolean {
return true;
}
/** Link-local egress is denied in every deployment mode. */
export function isAlwaysDeniedLinkLocalIp(address: string): boolean {
const normalized = normalizeIpAddress(address);
if (isIP(normalized) === 4) {
const octets = parseIpv4Address(normalized);
return octets !== null && octets[0] === 169 && octets[1] === 254;
}
return isIP(normalized) === 6 && /^fe[89ab]/.test(normalized);
}
function isPrivateOrReservedIpv4(address: string): boolean {
const octets = parseIpv4Address(address);
if (!octets) return true;
+10 -10
View File
@@ -6,6 +6,7 @@ import { connect as tlsConnect, type TLSSocket } from "node:tls";
import { createBrotliDecompress, createGunzip, createInflate } from "node:zlib";
import {
isAlwaysDeniedLinkLocalIp,
isPrivateOrReservedIp,
normalizeIpAddress,
resolveApprovedRemoteHttpAddresses,
@@ -56,8 +57,7 @@ export type GuardedRemoteHttpFetchOptions = RemoteHttpEndpointGuardOptions & {
/** Deadline for response headers, and idle deadline between body chunks. */
responseTimeoutMs?: number;
/**
* Platform `fetch`, used only when the deployment allows private endpoints and
* there is therefore no egress boundary to pin against.
* Platform `fetch`, used only for IP literals, which cannot be rebound.
*/
unpinnedFetch?: typeof fetch;
};
@@ -84,12 +84,8 @@ export type GuardedRemoteHttpFetchOptions = RemoteHttpEndpointGuardOptions & {
* 5. never follows redirects — it behaves as `redirect: "manual"` so the caller
* re-runs the whole guard against every `Location` it decides to follow.
*
* Two cases need no pinning and keep platform `fetch` semantics:
*
* - the deployment allows private endpoints, where the guard is a documented
* no-op and rebinding cannot reach anything an operator could not reach by
* typing the private URL in directly; and
* - the URL already carries an IP literal, where no name resolution happens on
* URLs that already carry an IP literal need no pinning and keep platform
* `fetch` semantics: no name resolution happens on
* either side of the guard, so there is no second answer to disagree with the
* first. `URL` has already normalised the literal (`0x7f.1`, `::ffff:7f00:1`)
* by the time the guard classifies it.
@@ -103,7 +99,7 @@ export async function guardedRemoteHttpFetch(
const approved = await resolveApprovedRemoteHttpAddresses(endpoint, options, options.error);
const literalHost = isIP(endpoint.hostname.replace(/^\[|\]$/g, "")) !== 0;
const platformFetch = options.unpinnedFetch ?? fetch;
if (approved.length === 0 || literalHost) {
if (literalHost) {
try {
return await platformFetch(endpoint.toString(), { ...init, redirect: "manual" });
} catch (error) {
@@ -237,7 +233,11 @@ async function openVerifiedSocket(input: {
}
const peer = raw.remoteAddress ? normalizeIpAddress(raw.remoteAddress) : null;
if (!peer || isPrivateOrReservedIp(peer) || !approvedSet.has(peer)) {
const peerDenied = peer && (
isAlwaysDeniedLinkLocalIp(peer)
|| (!options.allowPrivateNetwork && isPrivateOrReservedIp(peer))
);
if (!peer || peerDenied || !approvedSet.has(peer)) {
raw.destroy();
throw options.error(
"Remote MCP connection resolved to an address that was not approved",
@@ -147,6 +147,33 @@ describe("selectConfiguredRuntimeServiceRows", () => {
]);
});
it("can fall back to legacy execution-workspace scope for directly configured services", () => {
const projectScopedHistory = runtimeServiceRow({
serviceName: "worker",
command: "pnpm worker",
});
const legacyExecutionScopedWeb = runtimeServiceRow({
executionWorkspaceId: randomUUID(),
scopeType: "execution_workspace",
scopeId: randomUUID(),
serviceName: "web",
command: "pnpm dev",
});
const selected = selectConfiguredRuntimeServiceRows(
[projectScopedHistory, legacyExecutionScopedWeb],
{ services: [{ name: "web", command: "pnpm dev" }] },
{ fallbackScopeTypes: ["execution_workspace"] },
);
expect(selected).toEqual([
expect.objectContaining({
id: legacyExecutionScopedWeb.id,
configIndex: 0,
}),
]);
});
it("selects the row for the configured port instead of newer history on another port", () => {
const previousPort = runtimeServiceRow({
port: 42013,
@@ -34,6 +34,9 @@ export function selectCurrentRuntimeServiceRows(rows: WorkspaceRuntimeServiceRow
export function selectConfiguredRuntimeServiceRows(
rows: WorkspaceRuntimeServiceRow[],
workspaceRuntime: Record<string, unknown> | null | undefined,
options?: {
fallbackScopeTypes?: WorkspaceRuntimeServiceRow["scopeType"][];
},
) {
const availableRows = selectCurrentRuntimeServiceRows(rows).map((row) => ({
...row,
@@ -52,9 +55,18 @@ export function selectConfiguredRuntimeServiceRows(
: command.lifecycle === "shared"
? "project_workspace"
: "run";
const matchedRow = matchWorkspaceRuntimeServiceToCommand(
command,
availableRows.filter((row) => row.scopeType === expectedScope),
const candidateScopes = [
expectedScope,
...(options?.fallbackScopeTypes ?? []).filter((scopeType) => scopeType !== expectedScope),
];
const matchedRow = candidateScopes.reduce<(typeof availableRows)[number] | null>(
(match, scopeType) =>
match
?? matchWorkspaceRuntimeServiceToCommand(
command,
availableRows.filter((row) => row.scopeType === scopeType),
),
null,
);
if (!matchedRow) continue;
selectedRows.push({