diff --git a/.changeset/fuzzy-dingos-key.md b/.changeset/fuzzy-dingos-key.md new file mode 100644 index 0000000000..2eec4a85e8 --- /dev/null +++ b/.changeset/fuzzy-dingos-key.md @@ -0,0 +1,5 @@ +--- +"eve": patch +--- + +Prevent connection token cache collisions when a principal issuer or ID contains `:` or `%`. diff --git a/packages/eve/src/public/channels/eve-forwarded-principal.integration.test.ts b/packages/eve/src/public/channels/eve-forwarded-principal.integration.test.ts index 5f144772d8..d2e66a7fb2 100644 --- a/packages/eve/src/public/channels/eve-forwarded-principal.integration.test.ts +++ b/packages/eve/src/public/channels/eve-forwarded-principal.integration.test.ts @@ -158,7 +158,7 @@ describe("eveChannel forwarded principal → runtime principal", () => { type: "user", }); // The audit attribute never enters Connect token-cache keying. - expect(principalKey(principal)).toBe("user:slack:slack:U123"); + expect(principalKey(principal)).toBe("user:slack:slack%3AU123"); }); it("resolves the transport service principal (and fails Connect) without forwarding", async () => { diff --git a/packages/eve/src/runtime/connections/principal.test.ts b/packages/eve/src/runtime/connections/principal.test.ts index 08fd9957ac..99869cef3f 100644 --- a/packages/eve/src/runtime/connections/principal.test.ts +++ b/packages/eve/src/runtime/connections/principal.test.ts @@ -59,6 +59,36 @@ describe("principalKey", () => { it("keys an issuerless native Vercel user by its user id", () => { expect(principalKey({ id: "user_123", type: "user" })).toBe("user:user_123"); }); + + it("does not alias an issuer with an issuerless id containing the separator", () => { + const issued = principalKey({ id: "U123", issuer: "slack", type: "user" }); + const issuerless = principalKey({ id: "slack:U123", type: "user" }); + + expect(issued).not.toBe(issuerless); + expect(issuerless).toBe("user:slack%3AU123"); + }); + + it("does not alias a separator in the issuer with one in the id", () => { + const separatorInIssuer = principalKey({ + id: "U123", + issuer: "slack:webhook", + type: "user", + }); + const separatorInId = principalKey({ id: "webhook:U123", issuer: "slack", type: "user" }); + + expect(separatorInIssuer).not.toBe(separatorInId); + expect(separatorInIssuer).toBe("user:slack%3Awebhook:U123"); + expect(separatorInId).toBe("user:slack:webhook%3AU123"); + }); + + it("does not alias a literal percent-encoded separator with a separator", () => { + const encoded = principalKey({ id: "a%3Ab", type: "user" }); + const separator = principalKey({ id: "a:b", type: "user" }); + + expect(encoded).not.toBe(separator); + expect(encoded).toBe("user:a%253Ab"); + expect(separator).toBe("user:a%3Ab"); + }); }); describe("resolveConnectionPrincipal", () => { diff --git a/packages/eve/src/runtime/connections/principal.ts b/packages/eve/src/runtime/connections/principal.ts index a504ce608c..9eb073081a 100644 --- a/packages/eve/src/runtime/connections/principal.ts +++ b/packages/eve/src/runtime/connections/principal.ts @@ -27,15 +27,23 @@ import { isVercelOidcIssuer } from "#shared/vercel-project.js"; * would otherwise alias to the same cache slot. * - `{ type: "user", id }` → `"user:${id}"`. This is the native * Vercel Connect user projection. + * + * `%` and `:` are percent-encoded within each user segment so the + * separator cannot make distinct principals render to the same key. */ export function principalKey(principal: ConnectionPrincipal): string { if (principal.type === "app") { return "app"; } + const id = encodePrincipalKeySegment(principal.id); if (principal.issuer === undefined) { - return `user:${principal.id}`; + return `user:${id}`; } - return `user:${principal.issuer}:${principal.id}`; + return `user:${encodePrincipalKeySegment(principal.issuer)}:${id}`; +} + +function encodePrincipalKeySegment(value: string): string { + return value.replaceAll("%", "%25").replaceAll(":", "%3A"); } /**