From 1d136fea5ed62051e2a13530753257c15e35ee0b Mon Sep 17 00:00:00 2001 From: CTO Date: Wed, 24 Jun 2026 06:33:22 +0000 Subject: [PATCH] fix(subscriptions): avoid credential-derived redaction metadata Co-Authored-By: Paperclip --- .../schema/user_subscription_credentials.ts | 2 +- .../src/validators/subscription-credential.ts | 2 +- .../subscription-credentials-routes.test.ts | 3 +- .../subscription-credentials-service.test.ts | 68 ++++++++++++++++--- .../src/services/subscription-credentials.ts | 33 +++------ 5 files changed, 71 insertions(+), 37 deletions(-) diff --git a/packages/db/src/schema/user_subscription_credentials.ts b/packages/db/src/schema/user_subscription_credentials.ts index 7d928dec757..49303db710c 100644 --- a/packages/db/src/schema/user_subscription_credentials.ts +++ b/packages/db/src/schema/user_subscription_credentials.ts @@ -29,7 +29,7 @@ export const userSubscriptionCredentials = pgTable( material: jsonb("material").$type>().notNull(), // SHA-256 of the plaintext credential, for change detection / audit only. valueSha256: text("value_sha256").notNull(), - // Non-sensitive redacted preview metadata (e.g. token length, suffix hint). + // Safe metadata derived from credential kind only; never from plaintext. redactedMetadata: jsonb("redacted_metadata").$type>(), status: text("status").notNull().default("active"), lastTestedAt: timestamp("last_tested_at", { withTimezone: true }), diff --git a/packages/shared/src/validators/subscription-credential.ts b/packages/shared/src/validators/subscription-credential.ts index 853924fd9df..e66fd320998 100644 --- a/packages/shared/src/validators/subscription-credential.ts +++ b/packages/shared/src/validators/subscription-credential.ts @@ -71,7 +71,7 @@ export const subscriptionCredentialReadModelSchema = z.object({ credentialKind: subscriptionCredentialKindSchema, status: subscriptionCredentialStatusSchema, testStatus: subscriptionCredentialTestStatusSchema, - // Non-sensitive hints only (e.g. material length, last 4 chars of a token). + // Safe metadata only; must not be derived from plaintext credential material. redactedMetadata: z.record(z.string(), z.unknown()).nullable(), lastTestedAt: z.string().datetime().nullable(), lastResolvedAt: z.string().datetime().nullable(), diff --git a/server/src/__tests__/subscription-credentials-routes.test.ts b/server/src/__tests__/subscription-credentials-routes.test.ts index ad4b0d9b77d..c5667a93b49 100644 --- a/server/src/__tests__/subscription-credentials-routes.test.ts +++ b/server/src/__tests__/subscription-credentials-routes.test.ts @@ -55,7 +55,7 @@ const REDACTED_CREDENTIAL = { credentialKind: "claude_oauth_token", status: "active", testStatus: "untested", - redactedMetadata: { kind: "claude_oauth_token", materialLength: 40, tokenSuffix: "abcd" }, + redactedMetadata: { kind: "claude_oauth_token", materialFormat: "token" }, lastTestedAt: null, lastResolvedAt: null, createdAt: "2026-01-01T00:00:00.000Z", @@ -81,6 +81,7 @@ describe("subscription credential routes", () => { expect(res.status).toBe(200); expect(res.body).toEqual([REDACTED_CREDENTIAL]); + expect(Object.keys(res.body[0].redactedMetadata).sort()).toEqual(["kind", "materialFormat"]); expect(mockSvc.list).toHaveBeenCalledWith("company-1", "user-1"); }); diff --git a/server/src/__tests__/subscription-credentials-service.test.ts b/server/src/__tests__/subscription-credentials-service.test.ts index 4f6da333f7b..ec00c0b79e1 100644 --- a/server/src/__tests__/subscription-credentials-service.test.ts +++ b/server/src/__tests__/subscription-credentials-service.test.ts @@ -100,20 +100,20 @@ function makeFakeDb(options: { describe("subscriptionCredentialService", () => { beforeEach(() => { vi.clearAllMocks(); - mockSecretProvider.createSecret.mockResolvedValue({ + mockSecretProvider.createSecret.mockImplementation(async ({ value }: { value: string }) => ({ material: ENCRYPTED_MATERIAL, - valueSha256: sha256Hex("plain-token"), - }); + valueSha256: sha256Hex(value), + })); mockSecretProvider.resolveVersion.mockResolvedValue("plain-token"); }); it("encrypts credential material before persistence and returns only a redacted read model", async () => { + const credentialMaterial = "plain-secret-1234"; const returnedRow = makeCredentialRow({ material: ENCRYPTED_MATERIAL, redactedMetadata: { kind: "claude_oauth_token", - materialLength: "plain-token".length, - tokenSuffix: "oken", + materialFormat: "token", }, }); const { db, state } = makeFakeDb({ insertReturningRow: returnedRow }); @@ -124,23 +124,71 @@ describe("subscriptionCredentialService", () => { userId: returnedRow.userId, provider: returnedRow.provider as SubscriptionCredentialProvider, credentialKind: "claude_oauth_token", - material: "plain-token", + material: credentialMaterial, }); expect(getSecretProvider).toHaveBeenCalledWith("local_encrypted"); - expect(mockSecretProvider.createSecret).toHaveBeenCalledWith({ value: "plain-token" }); + expect(mockSecretProvider.createSecret).toHaveBeenCalledWith({ value: credentialMaterial }); expect(state.insertedValues).toMatchObject({ material: ENCRYPTED_MATERIAL, - valueSha256: sha256Hex("plain-token"), + valueSha256: sha256Hex(credentialMaterial), }); - expect(state.insertedValues).not.toMatchObject({ material: "plain-token" }); + expect(state.insertedValues).not.toMatchObject({ material: credentialMaterial }); expect(result).toMatchObject({ id: returnedRow.id, provider: "claude", credentialKind: "claude_oauth_token", - redactedMetadata: expect.objectContaining({ tokenSuffix: "oken" }), + redactedMetadata: { kind: "claude_oauth_token", materialFormat: "token" }, }); expect(result).not.toHaveProperty("material"); + expect(JSON.stringify(result)).not.toContain(credentialMaterial); + expect(JSON.stringify(result)).not.toContain("1234"); + expect(state.insertedValues?.redactedMetadata).toEqual({ + kind: "claude_oauth_token", + materialFormat: "token", + }); + expect(JSON.stringify(state.insertedValues?.redactedMetadata)).not.toContain("1234"); + }); + + it("does not derive JSON key names into redacted metadata", async () => { + const codexJson = JSON.stringify({ + accessToken: "access-secret", + refreshToken: "refresh-secret", + accountId: "acct-1", + }); + const returnedRow = makeCredentialRow({ + provider: "codex", + credentialKind: "codex_auth_json", + redactedMetadata: { + kind: "codex_auth_json", + materialFormat: "json", + }, + }); + const { db, state } = makeFakeDb({ insertReturningRow: returnedRow }); + const svc = subscriptionCredentialService(db); + + const result = await svc.upsert({ + companyId: returnedRow.companyId, + userId: returnedRow.userId, + provider: "codex", + credentialKind: "codex_auth_json", + material: codexJson, + }); + + expect(result.redactedMetadata).toEqual({ + kind: "codex_auth_json", + materialFormat: "json", + }); + expect(state.insertedValues?.redactedMetadata).toEqual({ + kind: "codex_auth_json", + materialFormat: "json", + }); + const metadataJson = JSON.stringify(state.insertedValues?.redactedMetadata); + expect(metadataJson).not.toContain("accessToken"); + expect(metadataJson).not.toContain("refreshToken"); + expect(metadataJson).not.toContain("accountId"); + expect(metadataJson).not.toContain("access-secret"); + expect(metadataJson).not.toContain("refresh-secret"); }); it("resolves decrypted material through the secret provider and records a redacted resolution timestamp", async () => { diff --git a/server/src/services/subscription-credentials.ts b/server/src/services/subscription-credentials.ts index 85e11afccd2..99947e2b4d6 100644 --- a/server/src/services/subscription-credentials.ts +++ b/server/src/services/subscription-credentials.ts @@ -56,29 +56,14 @@ function assertKindMatchesProvider( } } -// Build non-sensitive redacted metadata for read models. Only safe hints (no -// secret material) are stored or returned. -function buildRedactedMetadata( - kind: SubscriptionCredentialKind, - material: string, -): Record { - const length = material.length; - if (kind === "claude_oauth_token") { - const suffix = material.length >= 4 ? material.slice(-4) : null; - return { kind, materialLength: length, tokenSuffix: suffix }; - } - // JSON credential documents: surface only the top-level key names so the UI - // can hint at shape without exposing values. - let jsonKeys: string[] | null = null; - try { - const parsed = JSON.parse(material) as unknown; - if (parsed && typeof parsed === "object" && !Array.isArray(parsed)) { - jsonKeys = Object.keys(parsed as Record).sort(); - } - } catch { - jsonKeys = null; - } - return { kind, materialLength: length, jsonKeys }; +// Build redacted metadata from the declared credential kind only. Do not derive +// this record from plaintext credential material; even suffixes, lengths, or +// JSON key names are treated as credential-derived data. +function buildRedactedMetadata(kind: SubscriptionCredentialKind): Record { + return { + kind, + materialFormat: kind === "claude_oauth_token" ? "token" : "json", + }; } function toReadModel(row: CredentialRow): SubscriptionCredentialReadModel { @@ -156,7 +141,7 @@ export function subscriptionCredentialService(db: Db) { const provider = getSecretProvider(ENCRYPTION_PROVIDER); const prepared = await provider.createSecret({ value: input.material }); const now = new Date(); - const redactedMetadata = buildRedactedMetadata(input.credentialKind, input.material); + const redactedMetadata = buildRedactedMetadata(input.credentialKind); const [row] = await db .insert(userSubscriptionCredentials)