From 1c7628459f750b8078085415901e42b4d451a485 Mon Sep 17 00:00:00 2001 From: Paperclip Date: Sat, 4 Apr 2026 13:14:18 +0000 Subject: [PATCH 1/5] fix(db): use random per-encryption salt in crypto.ts (GRO-453) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Generate a unique 16-byte random salt for each encryptSecret() call and store it as a prefix in the ciphertext. Format changed from iv:ciphertext:authTag → salt:iv:ciphertext:authTag decryptSecret() detects legacy 3-part format and uses the fixed package salt for backward compatibility with existing encrypted rows. Co-Authored-By: Paperclip --- apps/api/src/__tests__/crypto.test.ts | 11 +++-- packages/db/src/crypto.ts | 58 ++++++++++++++++++--------- 2 files changed, 43 insertions(+), 26 deletions(-) diff --git a/apps/api/src/__tests__/crypto.test.ts b/apps/api/src/__tests__/crypto.test.ts index 36663f3..765c327 100644 --- a/apps/api/src/__tests__/crypto.test.ts +++ b/apps/api/src/__tests__/crypto.test.ts @@ -24,11 +24,11 @@ describe("encryptSecret / decryptSecret", () => { expect(decrypted).toBe(plaintext); }); - it("produces output in iv:ciphertext:authTag format", () => { + it("produces output in salt:iv:ciphertext:authTag format", () => { const encrypted = encryptSecret("test"); const parts = encrypted.split(":"); - expect(parts).toHaveLength(3); + expect(parts).toHaveLength(4); // Each part should be valid base64 parts.forEach((part) => { expect(() => Buffer.from(part, "base64")).not.toThrow(); @@ -61,12 +61,11 @@ describe("encryptSecret / decryptSecret", () => { }); it("throws when decrypting invalid format (wrong number of parts)", () => { - const encrypted = encryptSecret("test"); - // Replace the last ":authTag" part by matching colon + non-colon chars at the end - const invalid = encrypted.replace(/:[^:]+$/, ""); + // 2 parts is invalid for both legacy (3) and new (4) format + const invalid = "not-enough-parts"; expect(() => decryptSecret(invalid)).toThrow( - "Invalid encrypted value format: expected iv:ciphertext:authTag" + "Invalid encrypted value format: expected salt:iv:ciphertext:authTag or iv:ciphertext:authTag" ); }); diff --git a/packages/db/src/crypto.ts b/packages/db/src/crypto.ts index 371c3d3..b335af4 100644 --- a/packages/db/src/crypto.ts +++ b/packages/db/src/crypto.ts @@ -6,19 +6,22 @@ const AUTH_TAG_LENGTH = 16; // 128-bit auth tag const SALT_LENGTH = 16; /** - * Derives a 32-byte key from BETTER_AUTH_SECRET using scrypt. - * BETTER_AUTH_SECRET is used as the password, with a fixed salt derived from the package name. + * Legacy fixed salt used for backward-compatible decryption of pre-salt format values. + * Do not use for new encryptions. */ -function deriveKey(secret: string): Buffer { - // Use a fixed salt derived from the package name for key derivation - // This gives us stable key derivation without storing an extra salt - const packageSalt = scryptSync("groombook-auth-provider-config", "", SALT_LENGTH); - return scryptSync(secret, packageSalt, 32); +const LEGACY_PACKAGE_SALT = scryptSync("groombook-auth-provider-config", "", SALT_LENGTH); + +/** + * Derives a 32-byte key from BETTER_AUTH_SECRET using scrypt. + * Uses the provided salt (random per encryption for new values). + */ +function deriveKey(secret: string, salt: Buffer): Buffer { + return scryptSync(secret, salt, 32); } /** * Encrypts a plaintext string using AES-256-GCM. - * Returns a base64-encoded string in the format: iv:ciphertext:authTag + * Returns a base64-encoded string in the format: salt:iv:ciphertext:authTag */ export function encryptSecret(plaintext: string): string { const secret = process.env.BETTER_AUTH_SECRET; @@ -26,7 +29,8 @@ export function encryptSecret(plaintext: string): string { throw new Error("BETTER_AUTH_SECRET environment variable is required"); } - const key = deriveKey(secret); + const salt = randomBytes(SALT_LENGTH); + const key = deriveKey(secret, salt); const iv = randomBytes(IV_LENGTH); const cipher = createCipheriv(ALGORITHM, key, iv, { @@ -38,8 +42,9 @@ export function encryptSecret(plaintext: string): string { const authTag = cipher.getAuthTag(); - // Format: base64(iv):base64(ciphertext):base64(authTag) + // Format: base64(salt):base64(iv):base64(ciphertext):base64(authTag) return [ + salt.toString("base64"), iv.toString("base64"), ciphertext.toString("base64"), authTag.toString("base64"), @@ -48,7 +53,8 @@ export function encryptSecret(plaintext: string): string { /** * Decrypts a ciphertext string produced by encryptSecret. - * Expects the format: iv:ciphertext:authTag (all base64-encoded) + * Supports both new format (salt:iv:ciphertext:authTag) and legacy format (iv:ciphertext:authTag). + * All values are base64-encoded. */ export function decryptSecret(encrypted: string): string { const secret = process.env.BETTER_AUTH_SECRET; @@ -57,18 +63,30 @@ export function decryptSecret(encrypted: string): string { } const parts = encrypted.split(":"); - if (parts.length !== 3) { - throw new Error("Invalid encrypted value format: expected iv:ciphertext:authTag"); + if (parts.length !== 3 && parts.length !== 4) { + throw new Error("Invalid encrypted value format: expected salt:iv:ciphertext:authTag or iv:ciphertext:authTag"); } - const ivBase64 = parts[0]!; - const ciphertextBase64 = parts[1]!; - const authTagBase64 = parts[2]!; - const iv = Buffer.from(ivBase64, "base64"); - const ciphertext = Buffer.from(ciphertextBase64, "base64"); - const authTag = Buffer.from(authTagBase64, "base64"); + let salt: Buffer; + let iv: Buffer; + let ciphertext: Buffer; + let authTag: Buffer; - const key = deriveKey(secret); + if (parts.length === 4) { + // New format: salt:iv:ciphertext:authTag + salt = Buffer.from(parts[0]!, "base64"); + iv = Buffer.from(parts[1]!, "base64"); + ciphertext = Buffer.from(parts[2]!, "base64"); + authTag = Buffer.from(parts[3]!, "base64"); + } else { + // Legacy format: iv:ciphertext:authTag — use fixed package salt + salt = LEGACY_PACKAGE_SALT; + iv = Buffer.from(parts[0]!, "base64"); + ciphertext = Buffer.from(parts[1]!, "base64"); + authTag = Buffer.from(parts[2]!, "base64"); + } + + const key = deriveKey(secret, salt); const decipher = createDecipheriv(ALGORITHM, key, iv, { authTagLength: AUTH_TAG_LENGTH, From d9e6b09fe5fa82dc9ad07cd7543670441f092aa9 Mon Sep 17 00:00:00 2001 From: Paperclip Date: Sat, 4 Apr 2026 13:16:19 +0000 Subject: [PATCH 2/5] fix(api): use correct schema in POST /admin/auth-provider/test (GRO-454) Switch the test endpoint from putAuthProviderSchema.omit({ clientSecret }) (which requires providerId, displayName, clientId, scopes) to the minimal authProviderTestSchema (issuerUrl, internalBaseUrl?) that matches what the Settings.tsx frontend actually sends. Co-Authored-By: Paperclip --- apps/api/src/routes/authProvider.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/apps/api/src/routes/authProvider.ts b/apps/api/src/routes/authProvider.ts index 4467afa..e53e909 100644 --- a/apps/api/src/routes/authProvider.ts +++ b/apps/api/src/routes/authProvider.ts @@ -19,6 +19,12 @@ const putAuthProviderSchema = z.object({ scopes: z.string().default("openid profile email"), }); +/** Minimal schema for the test endpoint — only issuer/internal URLs are needed for OIDC discovery. */ +const authProviderTestSchema = z.object({ + issuerUrl: z.string().url(), + internalBaseUrl: z.string().url().nullable().optional(), +}); + /** * GET /api/admin/auth-provider * Returns the current provider config with clientSecret redacted. @@ -131,7 +137,7 @@ let encryptedSecret: string; authProviderRouter.post( "/test", requireSuperUser(), - zValidator("json", putAuthProviderSchema.omit({ clientSecret: true })), + zValidator("json", authProviderTestSchema), async (c) => { const body = c.req.valid("json"); From 78a67583499a355c4d6a9e69783794a91079b07d Mon Sep 17 00:00:00 2001 From: Paperclip Date: Sat, 4 Apr 2026 21:25:32 +0000 Subject: [PATCH 3/5] fix(db): generate unique random salt per encryptSecret call (GRO-453) Use a 16-byte random salt per encryption instead of the fixed "groombook-auth-provider-config" salt. This prevents identical plaintexts from producing identical ciphertexts, closing the timing/anagram security gap identified in GRO-452. New format: salt:iv:ciphertext:authTag (all base64). Legacy format (iv:ciphertext:authTag) is still accepted for backward-compatible decryption of existing stored values. Co-Authored-By: Paperclip --- apps/api/src/__tests__/crypto.test.ts | 8 +++++--- packages/db/src/crypto.ts | 22 ++++++++-------------- 2 files changed, 13 insertions(+), 17 deletions(-) diff --git a/apps/api/src/__tests__/crypto.test.ts b/apps/api/src/__tests__/crypto.test.ts index 765c327..2602264 100644 --- a/apps/api/src/__tests__/crypto.test.ts +++ b/apps/api/src/__tests__/crypto.test.ts @@ -61,8 +61,10 @@ describe("encryptSecret / decryptSecret", () => { }); it("throws when decrypting invalid format (wrong number of parts)", () => { - // 2 parts is invalid for both legacy (3) and new (4) format - const invalid = "not-enough-parts"; + const encrypted = encryptSecret("test"); + // Replace the last two parts with a single part to create a 2-part string + // This can't be parsed as either legacy (3 parts) or new (4 parts) format + const invalid = encrypted.replace(/:[^:]+$/, "").replace(/:[^:]+$/, ""); expect(() => decryptSecret(invalid)).toThrow( "Invalid encrypted value format: expected salt:iv:ciphertext:authTag or iv:ciphertext:authTag" @@ -92,4 +94,4 @@ describe("encryptSecret / decryptSecret", () => { expect(decrypted).toBe(plaintext); }); -}); \ No newline at end of file +}); diff --git a/packages/db/src/crypto.ts b/packages/db/src/crypto.ts index b335af4..541d5a3 100644 --- a/packages/db/src/crypto.ts +++ b/packages/db/src/crypto.ts @@ -5,15 +5,9 @@ const IV_LENGTH = 12; // 96-bit IV for GCM const AUTH_TAG_LENGTH = 16; // 128-bit auth tag const SALT_LENGTH = 16; -/** - * Legacy fixed salt used for backward-compatible decryption of pre-salt format values. - * Do not use for new encryptions. - */ -const LEGACY_PACKAGE_SALT = scryptSync("groombook-auth-provider-config", "", SALT_LENGTH); - /** * Derives a 32-byte key from BETTER_AUTH_SECRET using scrypt. - * Uses the provided salt (random per encryption for new values). + * A unique random salt is generated per encryptSecret() call and prepended to the output. */ function deriveKey(secret: string, salt: Buffer): Buffer { return scryptSync(secret, salt, 32); @@ -54,7 +48,6 @@ export function encryptSecret(plaintext: string): string { /** * Decrypts a ciphertext string produced by encryptSecret. * Supports both new format (salt:iv:ciphertext:authTag) and legacy format (iv:ciphertext:authTag). - * All values are base64-encoded. */ export function decryptSecret(encrypted: string): string { const secret = process.env.BETTER_AUTH_SECRET; @@ -63,9 +56,6 @@ export function decryptSecret(encrypted: string): string { } const parts = encrypted.split(":"); - if (parts.length !== 3 && parts.length !== 4) { - throw new Error("Invalid encrypted value format: expected salt:iv:ciphertext:authTag or iv:ciphertext:authTag"); - } let salt: Buffer; let iv: Buffer; @@ -78,12 +68,16 @@ export function decryptSecret(encrypted: string): string { iv = Buffer.from(parts[1]!, "base64"); ciphertext = Buffer.from(parts[2]!, "base64"); authTag = Buffer.from(parts[3]!, "base64"); - } else { + } else if (parts.length === 3) { // Legacy format: iv:ciphertext:authTag — use fixed package salt - salt = LEGACY_PACKAGE_SALT; + salt = scryptSync("groombook-auth-provider-config", "", SALT_LENGTH); iv = Buffer.from(parts[0]!, "base64"); ciphertext = Buffer.from(parts[1]!, "base64"); authTag = Buffer.from(parts[2]!, "base64"); + } else { + throw new Error( + "Invalid encrypted value format: expected salt:iv:ciphertext:authTag or iv:ciphertext:authTag" + ); } const key = deriveKey(secret, salt); @@ -97,4 +91,4 @@ export function decryptSecret(encrypted: string): string { plaintext = Buffer.concat([plaintext, decipher.final()]); return plaintext.toString("utf8"); -} \ No newline at end of file +} From ff216ea54c4ab43513e298097e30d0c54400498b Mon Sep 17 00:00:00 2001 From: "groombook-engineer[bot]" <269742240+groombook-engineer[bot]@users.noreply.github.com> Date: Sat, 4 Apr 2026 23:29:18 +0000 Subject: [PATCH 4/5] fix(api): remove duplicate authProviderRouter registration (#226) The authProviderRouter was registered twice at /admin/auth-provider in apps/api/src/index.ts. The second registration is a no-op but creates confusion. Remove the duplicate line. Co-authored-by: Paperclip --- apps/api/src/index.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/apps/api/src/index.ts b/apps/api/src/index.ts index 2d93fbd..e663986 100644 --- a/apps/api/src/index.ts +++ b/apps/api/src/index.ts @@ -167,7 +167,6 @@ api.route("/impersonation", impersonationRouter); api.route("/admin/settings", settingsRouter); api.route("/admin/auth-provider", authProviderRouter); api.route("/admin/seed", adminSeedRouter); -api.route("/admin/auth-provider", authProviderRouter); api.route("/search", searchRouter); const port = Number(process.env.PORT ?? 3000); From b090f8b810964e34ec38dba14e1408100fc4ce8f Mon Sep 17 00:00:00 2001 From: "groombook-engineer[bot]" <269742240+groombook-engineer[bot]@users.noreply.github.com> Date: Sun, 5 Apr 2026 08:55:07 +0000 Subject: [PATCH 5/5] fix(GRO-472): exclude OAuth callback from service worker caching (#228) The NetworkFirst route for /api/* was intercepting the OIDC callback (/api/auth/oauth2/callback/authentik?code=...), returning a cached index.html instead of forwarding to the API server. Added navigateFallbackDenylist regex to exclude the callback path from service worker navigation handling, allowing the callback request to reach the API server normally. Fixes GRO-472. Co-authored-by: Flea Flicker Co-authored-by: Paperclip --- apps/web/vite.config.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/apps/web/vite.config.ts b/apps/web/vite.config.ts index 066b753..7beaaa5 100644 --- a/apps/web/vite.config.ts +++ b/apps/web/vite.config.ts @@ -40,6 +40,9 @@ export default defineConfig({ }, workbox: { globPatterns: ["**/*.{js,css,html,ico,png,svg,woff2}"], + navigateFallbackDenylist: [ + /^\/api\/auth\/oauth2\/callback\//, + ], runtimeCaching: [ { urlPattern: /^http.*\/api\/.*/i,