From 399d5a31c889a59b8a5480a9c87683ecbbab44be Mon Sep 17 00:00:00 2001 From: Jason Woltje Date: Mon, 16 Feb 2026 15:42:10 -0600 Subject: [PATCH] =?UTF-8?q?fix(#411):=20narrow=20verifySession=20allowlist?= =?UTF-8?q?=20=E2=80=94=20prevent=20false-positive=20infra=20error=20class?= =?UTF-8?q?ification?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace broad "expired" and "unauthorized" substring matches with specific patterns to prevent infrastructure errors from being misclassified as auth errors: - "expired" -> "token expired", "session expired", or exact match "expired" - "unauthorized" -> exact match "unauthorized" only This prevents TLS errors like "certificate has expired" and DB auth errors like "Unauthorized: Access denied for user" from being silently swallowed as 401 responses. Co-Authored-By: Claude Opus 4.6 --- apps/api/src/auth/auth.service.spec.ts | 54 ++++++++++++++++++++++++++ apps/api/src/auth/auth.service.ts | 8 ++-- 2 files changed, 59 insertions(+), 3 deletions(-) diff --git a/apps/api/src/auth/auth.service.spec.ts b/apps/api/src/auth/auth.service.spec.ts index 56d0f56..4e88469 100644 --- a/apps/api/src/auth/auth.service.spec.ts +++ b/apps/api/src/auth/auth.service.spec.ts @@ -485,6 +485,60 @@ describe("AuthService", () => { expect(result).toBeNull(); }); + it("should return null for 'session expired' auth error", async () => { + const auth = service.getAuth(); + const mockGetSession = vi.fn().mockRejectedValue(new Error("Session expired")); + auth.api = { getSession: mockGetSession } as any; + + const result = await service.verifySession("expired-session"); + + expect(result).toBeNull(); + }); + + it("should return null for bare 'unauthorized' (exact match)", async () => { + const auth = service.getAuth(); + const mockGetSession = vi.fn().mockRejectedValue(new Error("unauthorized")); + auth.api = { getSession: mockGetSession } as any; + + const result = await service.verifySession("unauth-token"); + + expect(result).toBeNull(); + }); + + it("should return null for bare 'expired' (exact match)", async () => { + const auth = service.getAuth(); + const mockGetSession = vi.fn().mockRejectedValue(new Error("expired")); + auth.api = { getSession: mockGetSession } as any; + + const result = await service.verifySession("expired-token"); + + expect(result).toBeNull(); + }); + + it("should re-throw 'certificate has expired' as infrastructure error (not auth)", async () => { + const auth = service.getAuth(); + const mockGetSession = vi + .fn() + .mockRejectedValue(new Error("certificate has expired")); + auth.api = { getSession: mockGetSession } as any; + + await expect(service.verifySession("any-token")).rejects.toThrow( + "certificate has expired" + ); + }); + + it("should re-throw 'Unauthorized: Access denied for user' as infrastructure error (not auth)", async () => { + const auth = service.getAuth(); + const mockGetSession = vi + .fn() + .mockRejectedValue(new Error("Unauthorized: Access denied for user")); + auth.api = { getSession: mockGetSession } as any; + + await expect(service.verifySession("any-token")).rejects.toThrow( + "Unauthorized: Access denied for user" + ); + }); + it("should return null when a non-Error value is thrown", async () => { const auth = service.getAuth(); const mockGetSession = vi.fn().mockRejectedValue("string-error"); diff --git a/apps/api/src/auth/auth.service.ts b/apps/api/src/auth/auth.service.ts index d396def..0d659f4 100644 --- a/apps/api/src/auth/auth.service.ts +++ b/apps/api/src/auth/auth.service.ts @@ -130,10 +130,12 @@ export class AuthService { const msg = error.message.toLowerCase(); const isExpectedAuthError = msg.includes("invalid token") || - msg.includes("expired") || + msg.includes("token expired") || + msg.includes("session expired") || msg.includes("session not found") || - msg.includes("unauthorized") || - msg.includes("invalid session"); + msg.includes("invalid session") || + msg === "unauthorized" || + msg === "expired"; if (!isExpectedAuthError) { // Infrastructure or unexpected — propagate as 500