diff --git a/doc/architecture/paperclip-runner-daytona-networking.md b/doc/architecture/paperclip-runner-daytona-networking.md index 169212b945a..769206bff82 100644 --- a/doc/architecture/paperclip-runner-daytona-networking.md +++ b/doc/architecture/paperclip-runner-daytona-networking.md @@ -53,6 +53,10 @@ runner instead; it can still reuse the image's compatible provider CLI. Explicit remote runner artifacts undergo the same checks. This metadata gate does not change the PRP version or invalidate existing durable session manifests. +The replacement artifact uses the controller's server-owned binary resolver, +including the vendored binary in packaged deployments. It must not depend on a +source checkout or a local Rust build directory being present on the server. + For a controller deployed before this capability check, operators can set `PAPERCLIP_RUNNER_REMOTE_BINARY_PATH` in the **server process environment** to a current runner binary built for the sandbox's OS and architecture. This bypasses diff --git a/doc/connections/CHAT-CONNECTOR-UX.md b/doc/connections/CHAT-CONNECTOR-UX.md index daae0d48859..33da582280b 100644 --- a/doc/connections/CHAT-CONNECTOR-UX.md +++ b/doc/connections/CHAT-CONNECTOR-UX.md @@ -221,6 +221,13 @@ HTTPS termination at a trusted deployment boundary must not create false URL-dri warnings for the internal HTTP hop; continue detecting genuine host, port, and path changes. Keep callback verification evidence separate for distinct surfaces. +On a claimed Cloud instance, callback health can use the public host forwarded +by the gateway after the provider authenticates the request. These hints are +diagnostic evidence only: they must not affect authentication, routing, or the +configured callback URL. Self-hosted instances continue using the request URL. +Preserve gateway observations in dedicated diagnostic headers when another +provider proxy replaces standard forwarded headers before reaching the tenant. + ## Apply and verify For a requested redesign, identify the relevant principles and fix the concrete diff --git a/server/src/__tests__/chat-channels.integration.test.ts b/server/src/__tests__/chat-channels.integration.test.ts index 6bb26879725..91f67c80af4 100644 --- a/server/src/__tests__/chat-channels.integration.test.ts +++ b/server/src/__tests__/chat-channels.integration.test.ts @@ -12976,6 +12976,8 @@ describeEmbeddedPostgres("chat channel control-plane integration", () => { const body = JSON.stringify({ type: "url_verification", challenge: "proxy-check" }); const request = signedSlackWebhookRequest({ url: `http://paperclip.example${path}`, contentType: "application/json", body }); request.headers.set("x-forwarded-host", "untrusted.example"); + request.headers.set("x-paperclip-cloud-forwarded-host", "untrusted.example"); + request.headers.set("x-paperclip-cloud-forwarded-proto", "https"); request.headers.set("x-forwarded-proto", "https"); await service.handleWebhook(endpoint.publicId, "slack", request); await expect(service.get(endpoint.id)).resolves.toMatchObject({ setup: { callbacksNeedUpdate: false, callbackSurfaces: { events: { status: "current" } } } }); @@ -12987,6 +12989,81 @@ describeEmbeddedPostgres("chat channel control-plane integration", () => { } }); + it("records the public Cloud proxy host for authenticated Slack callback health", async () => { + const fixture = await seedCompany(); + const { endpoint, service } = await configuredSlackEndpoint(fixture); + const canonicalOrigin = vi.spyOn(cloudRuntimeIdentity, "runtimeCanonicalOrigin").mockReturnValue("https://paperclip.example"); + const path = `/api/chat-webhooks/${endpoint.publicId}/slack`; + const cases = [ + ["events", "application/json", JSON.stringify({ type: "url_verification", challenge: "cloud-proxy-check" })], + ["interactivity", "application/x-www-form-urlencoded", new URLSearchParams({ payload: JSON.stringify({ type: "block_actions", team: { id: "T-PAPERCLIP" } }) }).toString()], + ["slashCommands", "application/x-www-form-urlencoded", new URLSearchParams({ command: "/maya-paperclip", team_id: "T-PAPERCLIP" }).toString()], + ] as const; + try { + for (const prefix of ["x-forwarded", "x-paperclip-cloud-forwarded"]) { + for (const [surface, contentType, body] of cases) { + const request = signedSlackWebhookRequest({ url: `http://tenant.internal:3100${path}`, contentType, body }); + request.headers.set("x-forwarded-host", "tenant.up.railway.app"); + request.headers.set(`${prefix}-host`, "paperclip.example"); + request.headers.set(`${prefix}-proto`, "https"); + expect((await service.handleWebhook(endpoint.publicId, "slack", request)).ok).toBe(true); + await expect(service.get(endpoint.id)).resolves.toMatchObject({ setup: { + callbacksNeedUpdate: false, callbackSurfaces: { [surface]: { status: "current" } }, + } }); + // The proxy hint is evidence, not a rewrite to the adapter request. + expect(request.url).toBe(`http://tenant.internal:3100${path}`); + } + } + canonicalOrigin.mockReturnValue("https://moved.example"); + await expect(service.get(endpoint.id)).resolves.toMatchObject({ setup: { + callbacksNeedUpdate: true, callbackSurfaces: { events: { status: "stale" }, interactivity: { status: "stale" }, slashCommands: { status: "stale" } }, + } }); + } finally { + await service.shutdown(); + canonicalOrigin.mockRestore(); + } + }); + + it("does not let rejected callbacks or malformed proxy hints establish healthy Slack callbacks", async () => { + const fixture = await seedCompany(); + const { endpoint, runtime, service } = await configuredSlackEndpoint(fixture); + const providerRuntime = runtime.endpoints.get(endpoint.id)!; + const canonicalOrigin = vi.spyOn(cloudRuntimeIdentity, "runtimeCanonicalOrigin").mockReturnValue("https://paperclip.example"); + const path = `/api/chat-webhooks/${endpoint.publicId}/slack`; + const body = JSON.stringify({ type: "url_verification", challenge: "proxy-hint-check" }); + const send = async (host: string, protocol = "https", signed = true) => { + const request = signedSlackWebhookRequest({ url: `http://tenant.internal:3100${path}`, contentType: "application/json", body }); + request.headers.set("x-paperclip-cloud-forwarded-host", host); + request.headers.set("x-paperclip-cloud-forwarded-proto", protocol); + if (!signed) request.headers.delete("x-slack-signature"); + // The fake provider runtime supplies the adapter's authentication result. + providerRuntime.webhookResponse = new Response(signed ? "accepted" : "rejected", { status: signed ? 202 : 401 }); + return service.handleWebhook(endpoint.publicId, "slack", request); + }; + try { + // A real callback on an old public hostname must still warn. + await send("old.example"); + await expect(service.get(endpoint.id)).resolves.toMatchObject({ setup: { callbacksNeedUpdate: true } }); + expect((await send("paperclip.example", "https", false)).status).toBe(401); + await expect(service.get(endpoint.id)).resolves.toMatchObject({ setup: { callbacksNeedUpdate: true } }); + for (const host of ["paperclip.example, proxy.example", "paperclip.example/path", "user@paperclip.example", "paperclip.example?query=1", "paperclip.example#fragment", "paperclip.example:8443"]) { + await send(host); + await expect(service.get(endpoint.id)).resolves.toMatchObject({ setup: { callbacksNeedUpdate: true } }); + } + await send("paperclip.example", "javascript"); + await expect(service.get(endpoint.id)).resolves.toMatchObject({ setup: { callbacksNeedUpdate: true } }); + await send("paperclip.example"); + await expect(service.get(endpoint.id)).resolves.toMatchObject({ setup: { callbacksNeedUpdate: false } }); + for (const host of ["paperclip.example:443", "PAPERCLIP.EXAMPLE:443", "paperclip.example.", "paperclip.example.:443"]) { + await send(host); + await expect(service.get(endpoint.id)).resolves.toMatchObject({ setup: { callbacksNeedUpdate: false } }); + } + } finally { + await service.shutdown(); + canonicalOrigin.mockRestore(); + } + }); + it("tracks Slack callback surfaces independently and reports public URL drift", async () => { const fixture = await seedCompany(); const { endpoint, runtime, service } = diff --git a/server/src/services/chat-channels.ts b/server/src/services/chat-channels.ts index 9c3f23f2b19..0d24757d4b4 100644 --- a/server/src/services/chat-channels.ts +++ b/server/src/services/chat-channels.ts @@ -513,6 +513,7 @@ function canonicalCallbackUrl(value: string): string | null { try { const url = new URL(value); if (url.protocol !== "https:" && url.protocol !== "http:") return null; + url.hostname = url.hostname.replace(/\.$/, ""); url.username = ""; url.password = ""; url.search = ""; @@ -544,10 +545,36 @@ type SlackCallbackInspection = { isUrlVerification: boolean; }; +// Cloud records the public request host in dedicated headers so provider edges +// cannot replace it with their upstream host. Only use callback-health evidence on a +// claimed Cloud instance, after provider authentication succeeded. It must not +// change the Request passed to the adapter, signature verification, routing, +// board identity, or the configured URL used to generate callbacks. +function slackCallbackObservationUrl(request: Request): string | null { + const directUrl = canonicalCallbackUrl(request.url); + if (!directUrl || !runtimeCanonicalOrigin()) return directUrl; + const hasCloudEvidence = request.headers.has("x-paperclip-cloud-forwarded-host"); + const host = request.headers.get(hasCloudEvidence ? "x-paperclip-cloud-forwarded-host" : "x-forwarded-host")?.trim(); + const protocol = request.headers.get(hasCloudEvidence ? "x-paperclip-cloud-forwarded-proto" : "x-forwarded-proto")?.trim(); + if (!host || /[\s/@\\?#,]/.test(host) || (protocol !== "https" && protocol !== "http")) return directUrl; + try { + const origin = new URL(`${protocol}://${host}`); + // Accept one authority only, never credentials, paths, queries, or lists. + if (!origin.host || origin.username || origin.password || origin.pathname !== "/" || origin.search || origin.hash) return directUrl; + const observed = new URL(directUrl); + observed.protocol = origin.protocol; + observed.host = origin.host; + observed.port = origin.port; + return canonicalCallbackUrl(observed.toString()); + } catch { + return directUrl; + } +} + async function inspectSlackCallback( request: Request, ): Promise { - const url = canonicalCallbackUrl(request.url); + const url = slackCallbackObservationUrl(request); if (!url) return null; const contentType = request.headers.get("content-type")?.toLowerCase() ?? ""; let body: string; diff --git a/server/src/services/native-runtime/native-session-executor.test.ts b/server/src/services/native-runtime/native-session-executor.test.ts index 39362e92f92..4f21c1153e7 100644 --- a/server/src/services/native-runtime/native-session-executor.test.ts +++ b/server/src/services/native-runtime/native-session-executor.test.ts @@ -292,6 +292,7 @@ import { } from "./native-session-executor.js"; beforeEach(() => { + state.resolveRunnerBinary.mockReset().mockReturnValue("/tmp/paperclip-runnerd"); state.resolveCurrentWakeCommentsBinding.mockReset().mockResolvedValue(null); state.assertCurrentWakeCommentsRead.mockReset().mockResolvedValue(undefined); }); @@ -10059,11 +10060,12 @@ describe("runnerd provider runtime wiring", () => { } }); - it.each([false, true])("uses shared Codex and replaces only a stale runner image (stale=%s)", async (staleRunner) => { + it.each(["current", "stale", "missing"])("uses shared Codex and the server-owned replacement artifact (image=%s)", async (image) => { + const needsReplacement = image !== "current"; // The mocked remote executes metadata probes; artifact staging only needs bytes. // Keep this regression independent of a locally compiled Rust runner binary. const controllerArtifact = join(isolatedStateDirectory, "paperclip-runnerd"); - if (staleRunner) { + if (needsReplacement) { await writeFile(controllerArtifact, "fixture runner artifact"); state.resolveRunnerBinary.mockReturnValueOnce(controllerArtifact); } @@ -10078,7 +10080,7 @@ describe("runnerd provider runtime wiring", () => { binaryName: "paperclip-runnerd", packageName: "@paperclipai/paperclip-runner", binaryContractVersion: 2, - durableSessionCapabilities: staleRunner && command.command === "/usr/local/bin/paperclip-runnerd" + durableSessionCapabilities: image === "stale" && command.command === "/usr/local/bin/paperclip-runnerd" ? undefined : ["unlimited_runtime", "connection_lease_renewal"], prpTransportModes: ["listen_ws"], @@ -10095,7 +10097,7 @@ describe("runnerd provider runtime wiring", () => { } else if (script === "uname -s; uname -m") { stdout = `${process.platform === "darwin" ? "Darwin" : "Linux"}\n${process.arch === "arm64" ? "arm64" : "x86_64"}\n`; } else if (script.includes("command -v paperclip-runnerd")) { - stdout = "/usr/local/bin/paperclip-runnerd\n"; + stdout = image === "missing" ? "" : "/usr/local/bin/paperclip-runnerd\n"; } else if (script.includes("command -v codex")) { stdout = script.includes("/opt/paperclip-runner/bin/codex") ? "/opt/paperclip-runner/bin/codex\n" @@ -10140,7 +10142,7 @@ describe("runnerd provider runtime wiring", () => { await expect(transport.controlPlaneRegistration({})).rejects.toThrow( "reached-preinstalled-codex-verification", ); - if (staleRunner) { + if (needsReplacement) { expect(transport.runnerBinary).toBe(controllerArtifact); expect(syncIn).toHaveBeenCalledTimes(1); expect(syncIn).toHaveBeenCalledWith([expect.objectContaining({