fix(#411): narrow verifySession allowlist — prevent false-positive infra error classification
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 <noreply@anthropic.com>
This commit is contained in:
@@ -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");
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user