diff --git a/server/src/__tests__/agent-auth-jwt.test.ts b/server/src/__tests__/agent-auth-jwt.test.ts index e084b45b21..c5fe3f8b95 100644 --- a/server/src/__tests__/agent-auth-jwt.test.ts +++ b/server/src/__tests__/agent-auth-jwt.test.ts @@ -9,6 +9,7 @@ describe("agent local JWT", () => { const issuerEnv = "PAPERCLIP_AGENT_JWT_ISSUER"; const audienceEnv = "PAPERCLIP_AGENT_JWT_AUDIENCE"; const disableLegacyFallbackEnv = "PAPERCLIP_AGENT_JWT_DISABLE_LEGACY_FALLBACK"; + const instanceIdEnv = "PAPERCLIP_INSTANCE_ID"; const originalEnv = { secret: process.env[secretEnv], @@ -17,6 +18,7 @@ describe("agent local JWT", () => { issuer: process.env[issuerEnv], audience: process.env[audienceEnv], disableLegacyFallback: process.env[disableLegacyFallbackEnv], + instanceId: process.env[instanceIdEnv], }; beforeEach(() => { @@ -26,6 +28,7 @@ describe("agent local JWT", () => { delete process.env[issuerEnv]; delete process.env[audienceEnv]; delete process.env[disableLegacyFallbackEnv]; + delete process.env[instanceIdEnv]; vi.useFakeTimers(); }); @@ -43,6 +46,8 @@ describe("agent local JWT", () => { else process.env[audienceEnv] = originalEnv.audience; if (originalEnv.disableLegacyFallback === undefined) delete process.env[disableLegacyFallbackEnv]; else process.env[disableLegacyFallbackEnv] = originalEnv.disableLegacyFallback; + if (originalEnv.instanceId === undefined) delete process.env[instanceIdEnv]; + else process.env[instanceIdEnv] = originalEnv.instanceId; }); it("creates and verifies a token", () => { @@ -160,6 +165,82 @@ describe("agent local JWT", () => { }); }); + // --- Instance isolation (PAP-12899) --------------------------------------- + // A worktree/fork control-plane instance runs under a distinct + // PAPERCLIP_INSTANCE_ID but deliberately shares PAPERCLIP_AGENT_JWT_SECRET + // with its source instance (provisioning copies the secret). Before this + // change, a fork-minted run JWT validated successfully against the live plane + // (reads worked; writes then failed on missing heartbeat_runs FK rows). These + // tests pin the boundary that keeps fork tokens out of the live plane. + + it("stamps the minting instance id into the token claims", () => { + process.env[instanceIdEnv] = "default"; + vi.setSystemTime(new Date("2026-01-01T00:00:00.000Z")); + const token = createLocalAgentJwt("agent-1", "company-1", "claude_local", "run-1"); + const claims = verifyLocalAgentJwt(token!); + expect(claims?.instance_id).toBe("default"); + }); + + it("rejects a fork/worktree-minted token on the live control plane", () => { + vi.setSystemTime(new Date("2026-01-01T00:00:00.000Z")); + + // Mint on a worktree/fork instance (distinct instance id, SAME secret). + process.env[instanceIdEnv] = "pap-12899-worktree"; + const forkToken = createLocalAgentJwt("agent-1", "company-1", "claude_local", "run-1"); + expect(forkToken).not.toBeNull(); + // Sanity: it verifies on the instance that minted it. + expect(verifyLocalAgentJwt(forkToken!)?.company_id).toBe("company-1"); + + // Now switch to the live control plane (same shared secret, "default" + // instance) and confirm the fork token no longer authenticates — neither + // its instance-scoped signature nor the legacy master-secret fallback + // matches, so reads and writes are both refused. + process.env[instanceIdEnv] = "default"; + expect(verifyLocalAgentJwt(forkToken!)).toBeNull(); + }); + + it("keeps live-plane heartbeat tokens authenticating across mint/verify", () => { + process.env[instanceIdEnv] = "default"; + vi.setSystemTime(new Date("2026-01-01T00:00:00.000Z")); + const token = createLocalAgentJwt("agent-1", "company-1", "claude_local", "run-1", "user-1"); + const claims = verifyLocalAgentJwt(token!); + expect(claims).toMatchObject({ + sub: "agent-1", + company_id: "company-1", + run_id: "run-1", + instance_id: "default", + }); + }); + + it("rejects a token whose instance_id claim is forged to match the live plane", () => { + vi.setSystemTime(new Date("2026-01-01T00:00:00.000Z")); + + // Mint a fork token, then tamper the instance_id claim to impersonate the + // live "default" instance. The signature was bound to the fork instance's + // derived key, so re-encoding the claim cannot make it validate on the + // live plane — the claim check is defense-in-depth, the key is the boundary. + process.env[instanceIdEnv] = "pap-12899-worktree"; + const forkToken = createLocalAgentJwt("agent-1", "company-1", "claude_local", "run-1"); + const [headerB64, claimsB64, signature] = forkToken!.split("."); + const claims = JSON.parse(Buffer.from(claimsB64, "base64url").toString("utf8")); + claims.instance_id = "default"; + const tamperedClaimsB64 = Buffer.from(JSON.stringify(claims), "utf8").toString("base64url"); + const tampered = `${headerB64}.${tamperedClaimsB64}.${signature}`; + + process.env[instanceIdEnv] = "default"; + expect(verifyLocalAgentJwt(tampered)).toBeNull(); + }); + + it("still rejects the master-secret legacy fallback once it is disabled (full instance isolation)", () => { + vi.setSystemTime(new Date("2026-01-01T00:00:00.000Z")); + process.env[disableLegacyFallbackEnv] = "true"; + process.env[instanceIdEnv] = "default"; + // The legacy fallback signs with the raw shared secret and is therefore + // instance-agnostic; disabling it closes that residual cross-instance hole. + const legacyToken = craftLegacyMasterSecretToken(process.env[secretEnv]!, "company-1"); + expect(verifyLocalAgentJwt(legacyToken)).toBeNull(); + }); + it("defaults TTL to 1h when PAPERCLIP_AGENT_JWT_TTL_SECONDS is unset", () => { delete process.env[ttlEnv]; vi.setSystemTime(new Date("2026-01-01T00:00:00.000Z")); diff --git a/server/src/__tests__/agent-auth-middleware.test.ts b/server/src/__tests__/agent-auth-middleware.test.ts index fa2763518e..54c4fe150b 100644 --- a/server/src/__tests__/agent-auth-middleware.test.ts +++ b/server/src/__tests__/agent-auth-middleware.test.ts @@ -106,6 +106,14 @@ function createApp(db: any) { assertCompanyAccess(req, req.params.companyId); res.json({ ok: true }); }); + app.get("/companies/:companyId/issues/:issueId", (req, res) => { + assertCompanyAccess(req, req.params.companyId); + res.json({ id: req.params.issueId, readable: true }); + }); + app.patch("/companies/:companyId/issues/:issueId", (req, res) => { + assertCompanyAccess(req, req.params.companyId); + res.json({ id: req.params.issueId, writable: true }); + }); app.use(errorHandler); return app; } @@ -132,7 +140,11 @@ function craftAgentJwtWithoutResponsibleClaim(input: { const headerB64 = Buffer.from(JSON.stringify(header), "utf8").toString("base64url"); const claimsB64 = Buffer.from(JSON.stringify(claims), "utf8").toString("base64url"); const signingInput = `${headerB64}.${claimsB64}`; - const signingKey = createHmac("sha256", input.secret).update(`jwt:${input.companyId}`).digest("hex"); + // Sign with the same per-instance, per-company key the server derives. The + // instance defaults to "default" (beforeEach clears PAPERCLIP_INSTANCE_ID), + // matching the live control plane this middleware test exercises. This helper + // only omits the responsible_user_id claim — it is not a cross-instance token. + const signingKey = createHmac("sha256", input.secret).update(`jwt:default:${input.companyId}`).digest("hex"); const signature = createHmac("sha256", signingKey).update(signingInput).digest("base64url"); return `${signingInput}.${signature}`; } @@ -140,10 +152,14 @@ function craftAgentJwtWithoutResponsibleClaim(input: { describe("agent auth middleware", () => { const originalSecret = process.env.PAPERCLIP_AGENT_JWT_SECRET; const originalTtl = process.env.PAPERCLIP_AGENT_JWT_TTL_SECONDS; + const originalInstanceId = process.env.PAPERCLIP_INSTANCE_ID; beforeEach(() => { process.env.PAPERCLIP_AGENT_JWT_SECRET = "auth-middleware-secret"; process.env.PAPERCLIP_AGENT_JWT_TTL_SECONDS = "3600"; + // Pin the control-plane instance so mint/verify (and the hand-crafted + // legacy token helper) all derive keys under the "default" live instance. + delete process.env.PAPERCLIP_INSTANCE_ID; }); afterEach(() => { @@ -151,6 +167,8 @@ describe("agent auth middleware", () => { else process.env.PAPERCLIP_AGENT_JWT_SECRET = originalSecret; if (originalTtl === undefined) delete process.env.PAPERCLIP_AGENT_JWT_TTL_SECONDS; else process.env.PAPERCLIP_AGENT_JWT_TTL_SECONDS = originalTtl; + if (originalInstanceId === undefined) delete process.env.PAPERCLIP_INSTANCE_ID; + else process.env.PAPERCLIP_INSTANCE_ID = originalInstanceId; }); it("uses the signed responsible_user_id claim and keeps the signed run id authoritative", async () => { @@ -239,6 +257,36 @@ describe("agent auth middleware", () => { }); }); + it("rejects fork-minted run JWTs before issue reads or writes reach live issue data", async () => { + const companyId = randomUUID(); + const agentId = randomUUID(); + const runId = randomUUID(); + const issueId = randomUUID(); + const { db } = createDbState({ + agent: { id: agentId, companyId }, + run: { id: runId, companyId, agentId, responsibleUserId: "user-claim" }, + }); + + process.env.PAPERCLIP_INSTANCE_ID = "pap-12899-worktree"; + const forkToken = createLocalAgentJwt(agentId, companyId, "codex_local", runId, "user-claim"); + expect(forkToken).not.toBeNull(); + + process.env.PAPERCLIP_INSTANCE_ID = "default"; + const app = createApp(db); + const readRes = await request(app) + .get(`/companies/${companyId}/issues/${issueId}`) + .set("Authorization", `Bearer ${forkToken}`) + .set("X-Paperclip-Run-Id", runId); + const writeRes = await request(app) + .patch(`/companies/${companyId}/issues/${issueId}`) + .set("Authorization", `Bearer ${forkToken}`) + .set("X-Paperclip-Run-Id", runId) + .send({ title: "should not write" }); + + expect(readRes.status).toBe(401); + expect(writeRes.status).toBe(401); + }); + it("populates agent-key actors from the key responsible user binding", async () => { const companyId = randomUUID(); const agentId = randomUUID(); diff --git a/server/src/agent-auth-jwt.ts b/server/src/agent-auth-jwt.ts index 7b38119435..4cb25e97ab 100644 --- a/server/src/agent-auth-jwt.ts +++ b/server/src/agent-auth-jwt.ts @@ -1,4 +1,5 @@ import { createHmac, timingSafeEqual } from "node:crypto"; +import { resolvePaperclipInstanceId } from "./home-paths.js"; interface JwtHeader { alg: string; @@ -15,6 +16,7 @@ export interface LocalAgentJwtClaims { exp: number; iss?: string; aud?: string; + instance_id?: string; jti?: string; } @@ -41,24 +43,44 @@ function jwtConfig() { ttlSeconds: parseNumber(process.env.PAPERCLIP_AGENT_JWT_TTL_SECONDS, 60 * 60), issuer: process.env.PAPERCLIP_AGENT_JWT_ISSUER ?? "paperclip", audience: process.env.PAPERCLIP_AGENT_JWT_AUDIENCE ?? "paperclip-api", + // The control-plane instance this process belongs to. The live plane runs as + // "default"; every worktree/fork instance gets a distinct id (its worktree + // name) even though it deliberately shares PAPERCLIP_AGENT_JWT_SECRET with + // the source instance. Folding this into the signing-key derivation is what + // prevents a fork-minted token from authenticating against the live plane. + instanceId: resolvePaperclipInstanceId(), disableLegacyFallback: parseBooleanEnv(process.env.PAPERCLIP_AGENT_JWT_DISABLE_LEGACY_FALLBACK), }; } /** - * Derive a per-company signing key from the master JWT secret and a companyId. + * Derive a per-instance, per-company signing key from the master JWT secret, + * the control-plane instanceId, and a companyId. * - * In a multi-tenant deployment this ensures that a JWT signed for company A - * cannot be reused to authenticate as an agent in company B, even if the raw - * token leaks. The instance-wide master secret is never used to sign new - * tokens — it is retained only as a verification fallback so that tokens - * issued before this change continue to validate. + * Two isolation properties fall out of this derivation: + * - Per-company: a JWT signed for company A cannot be reused to authenticate + * as an agent in company B, even if the raw token leaks. + * - Per-instance: a JWT minted by a worktree/fork control-plane instance + * cannot authenticate against the live plane, even though forks + * deliberately share the same master secret (it is copied into worktree + * envs by provisioning). The live plane derives its key from its own + * instanceId ("default"), so a fork token — signed under the fork's + * instanceId — never matches. See PAP-12896 for the incident this closes. + * + * The instance-wide master secret is never used to sign new tokens — it is + * retained only as a verification fallback so that tokens issued before this + * change continue to validate. NOTE: that legacy fallback is instance-agnostic + * (it signs with the raw shared secret), so complete cryptographic instance + * isolation additionally requires disabling it once outstanding legacy tokens + * have expired (set PAPERCLIP_AGENT_JWT_DISABLE_LEGACY_FALLBACK=true). Normal + * fork-minted run tokens are already rejected without that step because they + * are signed with the derived key, not the raw master secret. * * The derivation domain-separates with the `jwt:` prefix so the same master * secret can safely be reused for other HMAC purposes without key reuse. */ -function deriveCompanySigningKey(masterSecret: string, companyId: string): string { - return createHmac("sha256", masterSecret).update(`jwt:${companyId}`).digest("hex"); +function deriveCompanySigningKey(masterSecret: string, companyId: string, instanceId: string): string { + return createHmac("sha256", masterSecret).update(`jwt:${instanceId}:${companyId}`).digest("hex"); } function base64UrlEncode(value: string) { @@ -110,6 +132,7 @@ export function createLocalAgentJwt( exp: now + config.ttlSeconds, iss: config.issuer, aud: config.audience, + instance_id: config.instanceId, }; const header = { @@ -118,9 +141,10 @@ export function createLocalAgentJwt( }; const signingInput = `${base64UrlEncode(JSON.stringify(header))}.${base64UrlEncode(JSON.stringify(claims))}`; - // Sign with the per-company derived key so a leaked token cannot be reused - // across tenants. - const signingKey = deriveCompanySigningKey(config.secret, companyId); + // Sign with the per-instance, per-company derived key so a leaked token + // cannot be reused across tenants and a fork-minted token cannot authenticate + // against a different control-plane instance. + const signingKey = deriveCompanySigningKey(config.secret, companyId, config.instanceId); const signature = signPayload(signingKey, signingInput); return `${signingInput}.${signature}`; @@ -145,18 +169,24 @@ export function verifyLocalAgentJwt(token: string): LocalAgentJwtClaims | null { if (!claimedCompanyId) return null; const signingInput = `${headerB64}.${claimsB64}`; - // Try the per-company derived key first (current tokens). Fall back to the - // raw master secret so tokens issued before per-company derivation existed - // continue to verify — this preserves backward compatibility for any - // outstanding tokens (TTL bounds the legacy window naturally). + // Try the per-instance, per-company derived key first (current tokens), + // deriving under THIS control plane's own instanceId. A token minted by a + // worktree/fork instance was signed under a different instanceId, so it will + // not match here — that is the boundary that keeps fork tokens out of the + // live plane (PAP-12896/PAP-12899). Fall back to the raw master secret so + // tokens issued before per-company derivation existed continue to verify — + // this preserves backward compatibility for any outstanding tokens (TTL + // bounds the legacy window naturally). // // Operators should set `PAPERCLIP_AGENT_JWT_DISABLE_LEGACY_FALLBACK=true` // approximately one JWT TTL (~1h by default, see PAPERCLIP_AGENT_JWT_TTL_SECONDS) // after deploying per-company signing. Once set, the master-secret fallback - // is disabled and only tokens validating under the per-company derived key - // are accepted — closing the window in which a leaked master secret could - // be used to forge tokens with arbitrary future `exp` values for any tenant. - const perCompanyKey = deriveCompanySigningKey(config.secret, claimedCompanyId); + // is disabled and only tokens validating under the per-instance/per-company + // derived key are accepted — closing the window in which a leaked master + // secret could be used to forge tokens with arbitrary future `exp` values for + // any tenant, and completing cryptographic isolation between control-plane + // instances (the raw-secret fallback is instance-agnostic). + const perCompanyKey = deriveCompanySigningKey(config.secret, claimedCompanyId, config.instanceId); const perCompanySig = signPayload(perCompanyKey, signingInput); let signatureOk = safeCompare(signature, perCompanySig); if (!signatureOk && !config.disableLegacyFallback) { @@ -186,6 +216,15 @@ export function verifyLocalAgentJwt(token: string): LocalAgentJwtClaims | null { if (issuer && issuer !== config.issuer) return null; if (audience && audience !== config.audience) return null; + // Enforce the minting instance when the claim is present. The instance-scoped + // signing key above is the real cryptographic boundary; this claim check is + // defense-in-depth that yields a clean, cheap rejection (and, once legacy + // tokens have aged out, guards the master-secret fallback path too). Legacy + // tokens minted before this claim existed omit it and are still accepted, so + // enforcement is conditional — matching how iss/aud are handled above. + const instanceClaim = typeof claims.instance_id === "string" ? claims.instance_id : undefined; + if (instanceClaim && instanceClaim !== config.instanceId) return null; + return { sub, company_id: companyId, @@ -196,6 +235,7 @@ export function verifyLocalAgentJwt(token: string): LocalAgentJwtClaims | null { exp, ...(issuer ? { iss: issuer } : {}), ...(audience ? { aud: audience } : {}), + ...(instanceClaim ? { instance_id: instanceClaim } : {}), jti: typeof claims.jti === "string" ? claims.jti : undefined, }; }