feat(security): per-company JWT signing keys for multi-tenant isolation (#5864)
## Thinking Path > - Paperclip orchestrates AI agents for zero-human companies > - Agents authenticate to the server with a JWT signed by the deployment's master secret > - In a multi-tenant deployment, all agents from every tenant are signed with the *same* key, so a leak (CI/staging dump, hostile contractor with infra access, supply-chain) lets the attacker mint tokens for *any* tenant > - The same master secret also issued tokens with a 48-hour TTL, giving any leaked token a two-day window of validity even after rotation > - This pull request derives a per-company signing key via `HMAC-SHA256(master, "jwt:<companyId>")` and reduces the default TTL to 1h; the verifier tries the per-company key first and falls back to the master secret only for tokens issued before this change so no agent gets locked out on deploy > - The benefit is multi-tenant key isolation (a leak of one company's derived key cannot forge tokens for another) and a tighter blast-radius on any leaked token, with zero local-first impact (single-tenant deploys derive their one company's key the same way and continue to work unchanged) ## Linked Issues or Issue Description Refs #5288 — a separate key-hygiene finding in the same module (`agent-auth-jwt.ts` falls back to `BETTER_AUTH_SECRET` as the JWT signing secret). Related agent-JWT trust-model concern, but not fixed by this PR — the master-secret fallback selection is unchanged here. No existing issue covers this PR's problem directly — described in-PR: - In a multi-tenant deployment, agents from every tenant get JWTs signed with the *same* master key, so a single leak (CI/staging dump, hostile contractor, supply chain) lets the attacker mint tokens for *any* tenant. - The same master secret issued tokens with a 48-hour TTL, giving any leaked token a two-day validity window even after rotation. - Fix: derive a per-company signing key via `HMAC-SHA256(master, "jwt:<companyId>")` and reduce the default TTL to 1h, with a master-secret verification fallback so pre-existing tokens are not locked out on deploy. ## What Changed - **`server/src/agent-auth-jwt.ts`** - New `deriveCompanySigningKey(masterSecret, companyId)` — `HMAC-SHA256` with domain-separated input (`jwt:<companyId>`) so the master secret can be safely reused for other HMAC purposes in the future without cross-protocol risk. - `signAgentJwt` always signs with the derived per-company key. - `verifyAgentJwt` reads `company_id` from the token's (untrusted) claim payload, looks up the candidate derived key, and verifies. If that fails AND a master secret is set, it falls back to verifying with the raw master secret — pre-existing tokens validate until they expire. Verification still fails if the signature doesn't bind. - Default TTL: `60 * 60 * 48` → `60 * 60`. Existing `PAPERCLIP_AGENT_JWT_TTL_SECONDS` override still wins. - **`PAPERCLIP_AGENT_JWT_DISABLE_LEGACY_FALLBACK`** (optional, default off) — operators set this ~one TTL after deploying to sunset the master-secret verification fallback entirely, closing the window in which a leaked master secret could forge arbitrary-`exp` tokens for any tenant. - **`server/src/__tests__/agent-auth-jwt.test.ts`** (6 new cases) - Per-company isolation via tamper: token for company A fails when verified for company B. - Legacy-token verification path: tokens signed with the raw master secret still verify. - Default TTL is 1h. - Legacy fallback toggle: master-secret tokens accepted when unset, rejected when enabled, and per-company tokens unaffected either way. ## Verification - `pnpm --filter @paperclipai/server run typecheck` — clean. - `npx vitest run agent-auth-jwt` — 11/11 pass (6 new + 5 existing). - Manual: token signed for company A under per-company key fails when verified against company B's derived key. ## Risks - **Backward-compatible verification**, so no agent gets locked out on deploy — but operators relying on hot-swapping the master secret should note that pre-existing tokens *will* keep validating against the master key until their TTL elapses, unless `PAPERCLIP_AGENT_JWT_DISABLE_LEGACY_FALLBACK=true` is set to end the fallback window explicitly. - **TTL reduction is a default, not a hard cap.** Operators who relied on the 48h window can override via env. If 1h is too aggressive for upstream taste, happy to gate the change behind an env var. - **No new required env vars.** Single-tenant local-first deploys derive one company's key the same way and behave identically to today. - **Domain-separated HMAC input** (`jwt:<companyId>`) means the master secret can be safely reused for other future HMAC purposes without cross-protocol risk. ## Model Used Claude Opus 4.7 (1M context), extended thinking mode; rebase + legacy-fallback sunset documentation by Claude Fable 5 (1M context). ## Checklist - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] Thinking path traces from project context to this change - [x] Model used specified - [x] Checked ROADMAP.md — part of the multi-tenant hardening initiative - [x] Tests run locally and pass (`agent-auth-jwt` 11/11) - [x] Added per-company-isolation, legacy-fallback, and TTL-default tests - [x] No UI changes - [x] Documented risks above - [x] Will address all Greptile and reviewer comments before merge Part of the multi-tenant hardening initiative — see also #3967 (cross-tenant 404 oracle) and #5865 (plugin tables `company_id`). --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
committed by
GitHub
parent
606e74d11f
commit
70357b961f
@@ -1,3 +1,4 @@
|
||||
import { createHmac } from "node:crypto";
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import { createLocalAgentJwt, verifyLocalAgentJwt } from "../agent-auth-jwt.js";
|
||||
|
||||
@@ -7,6 +8,7 @@ describe("agent local JWT", () => {
|
||||
const ttlEnv = "PAPERCLIP_AGENT_JWT_TTL_SECONDS";
|
||||
const issuerEnv = "PAPERCLIP_AGENT_JWT_ISSUER";
|
||||
const audienceEnv = "PAPERCLIP_AGENT_JWT_AUDIENCE";
|
||||
const disableLegacyFallbackEnv = "PAPERCLIP_AGENT_JWT_DISABLE_LEGACY_FALLBACK";
|
||||
|
||||
const originalEnv = {
|
||||
secret: process.env[secretEnv],
|
||||
@@ -14,6 +16,7 @@ describe("agent local JWT", () => {
|
||||
ttl: process.env[ttlEnv],
|
||||
issuer: process.env[issuerEnv],
|
||||
audience: process.env[audienceEnv],
|
||||
disableLegacyFallback: process.env[disableLegacyFallbackEnv],
|
||||
};
|
||||
|
||||
beforeEach(() => {
|
||||
@@ -22,6 +25,7 @@ describe("agent local JWT", () => {
|
||||
process.env[ttlEnv] = "3600";
|
||||
delete process.env[issuerEnv];
|
||||
delete process.env[audienceEnv];
|
||||
delete process.env[disableLegacyFallbackEnv];
|
||||
vi.useFakeTimers();
|
||||
});
|
||||
|
||||
@@ -37,6 +41,8 @@ describe("agent local JWT", () => {
|
||||
else process.env[issuerEnv] = originalEnv.issuer;
|
||||
if (originalEnv.audience === undefined) delete process.env[audienceEnv];
|
||||
else process.env[audienceEnv] = originalEnv.audience;
|
||||
if (originalEnv.disableLegacyFallback === undefined) delete process.env[disableLegacyFallbackEnv];
|
||||
else process.env[disableLegacyFallbackEnv] = originalEnv.disableLegacyFallback;
|
||||
});
|
||||
|
||||
it("creates and verifies a token", () => {
|
||||
@@ -97,4 +103,119 @@ describe("agent local JWT", () => {
|
||||
process.env[audienceEnv] = "paperclip-api";
|
||||
expect(verifyLocalAgentJwt(token!)).toBeNull();
|
||||
});
|
||||
|
||||
it("does not verify a token across companies (per-company isolation)", () => {
|
||||
vi.setSystemTime(new Date("2026-01-01T00:00:00.000Z"));
|
||||
const tokenA = createLocalAgentJwt("agent-1", "company-A", "claude_local", "run-1");
|
||||
expect(tokenA).not.toBeNull();
|
||||
|
||||
// A token whose body claims company-A must verify successfully under its
|
||||
// own company-A derived key.
|
||||
expect(verifyLocalAgentJwt(tokenA!)?.company_id).toBe("company-A");
|
||||
|
||||
// Tamper: forge a token by copying tokenA's header+signature and swapping
|
||||
// the claim's company_id to company-B. The signature was bound to the
|
||||
// company-A derived key over the original claims; once we re-encode with a
|
||||
// different company_id (or rebind to company-B's key) verification must
|
||||
// fail because the signature is over the original signing input.
|
||||
const [headerB64, claimsB64, signature] = tokenA!.split(".");
|
||||
const claims = JSON.parse(Buffer.from(claimsB64, "base64url").toString("utf8"));
|
||||
claims.company_id = "company-B";
|
||||
const tamperedClaimsB64 = Buffer.from(JSON.stringify(claims), "utf8").toString("base64url");
|
||||
const tampered = `${headerB64}.${tamperedClaimsB64}.${signature}`;
|
||||
expect(verifyLocalAgentJwt(tampered)).toBeNull();
|
||||
});
|
||||
|
||||
it("accepts legacy tokens signed with the master secret (backward compat)", () => {
|
||||
vi.setSystemTime(new Date("2026-01-01T00:00:00.000Z"));
|
||||
const masterSecret = process.env[secretEnv]!;
|
||||
|
||||
// Hand-craft a token signed directly with the master secret, simulating a
|
||||
// JWT issued before per-company derivation existed.
|
||||
const now = Math.floor(Date.now() / 1000);
|
||||
const header = { alg: "HS256", typ: "JWT" };
|
||||
const claims = {
|
||||
sub: "agent-legacy",
|
||||
company_id: "company-legacy",
|
||||
adapter_type: "claude_local",
|
||||
run_id: "run-legacy",
|
||||
iat: now,
|
||||
exp: now + 3600,
|
||||
iss: "paperclip",
|
||||
aud: "paperclip-api",
|
||||
};
|
||||
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 legacySig = createHmac("sha256", masterSecret).update(signingInput).digest("base64url");
|
||||
const legacyToken = `${signingInput}.${legacySig}`;
|
||||
|
||||
const verified = verifyLocalAgentJwt(legacyToken);
|
||||
expect(verified).toMatchObject({
|
||||
sub: "agent-legacy",
|
||||
company_id: "company-legacy",
|
||||
adapter_type: "claude_local",
|
||||
run_id: "run-legacy",
|
||||
});
|
||||
});
|
||||
|
||||
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"));
|
||||
const token = createLocalAgentJwt("agent-1", "company-1", "claude_local", "run-1");
|
||||
const claims = verifyLocalAgentJwt(token!);
|
||||
expect(claims).not.toBeNull();
|
||||
expect(claims!.exp - claims!.iat).toBe(60 * 60);
|
||||
});
|
||||
|
||||
// Helper: hand-craft a token signed with the raw master secret (legacy path).
|
||||
function craftLegacyMasterSecretToken(masterSecret: string, companyId: string) {
|
||||
const now = Math.floor(Date.now() / 1000);
|
||||
const header = { alg: "HS256", typ: "JWT" };
|
||||
const claims = {
|
||||
sub: "agent-legacy",
|
||||
company_id: companyId,
|
||||
adapter_type: "claude_local",
|
||||
run_id: "run-legacy",
|
||||
iat: now,
|
||||
exp: now + 3600,
|
||||
iss: "paperclip",
|
||||
aud: "paperclip-api",
|
||||
};
|
||||
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 legacySig = createHmac("sha256", masterSecret).update(signingInput).digest("base64url");
|
||||
return `${signingInput}.${legacySig}`;
|
||||
}
|
||||
|
||||
it("accepts master-secret-signed tokens when PAPERCLIP_AGENT_JWT_DISABLE_LEGACY_FALLBACK is unset", () => {
|
||||
vi.setSystemTime(new Date("2026-01-01T00:00:00.000Z"));
|
||||
delete process.env[disableLegacyFallbackEnv];
|
||||
const legacyToken = craftLegacyMasterSecretToken(process.env[secretEnv]!, "company-legacy");
|
||||
const verified = verifyLocalAgentJwt(legacyToken);
|
||||
expect(verified).not.toBeNull();
|
||||
expect(verified!.company_id).toBe("company-legacy");
|
||||
});
|
||||
|
||||
it("rejects master-secret-signed tokens when PAPERCLIP_AGENT_JWT_DISABLE_LEGACY_FALLBACK is enabled", () => {
|
||||
vi.setSystemTime(new Date("2026-01-01T00:00:00.000Z"));
|
||||
process.env[disableLegacyFallbackEnv] = "true";
|
||||
const legacyToken = craftLegacyMasterSecretToken(process.env[secretEnv]!, "company-legacy");
|
||||
expect(verifyLocalAgentJwt(legacyToken)).toBeNull();
|
||||
});
|
||||
|
||||
it("still verifies per-company-signed tokens when PAPERCLIP_AGENT_JWT_DISABLE_LEGACY_FALLBACK is enabled", () => {
|
||||
vi.setSystemTime(new Date("2026-01-01T00:00:00.000Z"));
|
||||
process.env[disableLegacyFallbackEnv] = "true";
|
||||
const token = createLocalAgentJwt("agent-1", "company-1", "claude_local", "run-1");
|
||||
expect(token).not.toBeNull();
|
||||
const verified = verifyLocalAgentJwt(token!);
|
||||
expect(verified).toMatchObject({
|
||||
sub: "agent-1",
|
||||
company_id: "company-1",
|
||||
adapter_type: "claude_local",
|
||||
run_id: "run-1",
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user