fix: replace flaky timing-based test with deterministic assertion
All checks were successful
ci/woodpecker/push/api Pipeline was successful
All checks were successful
ci/woodpecker/push/api Pipeline was successful
The constant-time comparison test used Date.now() deltas with a 10ms threshold which is unreliable in CI. Replace with deterministic tests that verify both same-length and different-length key rejection paths work correctly. The actual timing-safe behavior is guaranteed by Node's crypto.timingSafeEqual which the guard uses. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -113,34 +113,24 @@ describe("ApiKeyGuard", () => {
|
||||
const validApiKey = "test-api-key-12345";
|
||||
vi.mocked(mockConfigService.get).mockReturnValue(validApiKey);
|
||||
|
||||
const startTime = Date.now();
|
||||
const context1 = createMockExecutionContext({
|
||||
"x-api-key": "wrong-key-short",
|
||||
// Verify that same-length keys are compared properly (exercises timingSafeEqual path)
|
||||
// and different-length keys are rejected before comparison
|
||||
const sameLength = createMockExecutionContext({
|
||||
"x-api-key": "test-api-key-12344", // Same length, one char different
|
||||
});
|
||||
const differentLength = createMockExecutionContext({
|
||||
"x-api-key": "short", // Different length
|
||||
});
|
||||
|
||||
try {
|
||||
guard.canActivate(context1);
|
||||
} catch {
|
||||
// Expected to fail
|
||||
}
|
||||
const shortKeyTime = Date.now() - startTime;
|
||||
// Both should throw, proving the comparison logic handles both cases
|
||||
expect(() => guard.canActivate(sameLength)).toThrow("Invalid API key");
|
||||
expect(() => guard.canActivate(differentLength)).toThrow("Invalid API key");
|
||||
|
||||
const startTime2 = Date.now();
|
||||
const context2 = createMockExecutionContext({
|
||||
"x-api-key": "test-api-key-12344", // Very close to correct key
|
||||
// Correct key should pass
|
||||
const correct = createMockExecutionContext({
|
||||
"x-api-key": validApiKey,
|
||||
});
|
||||
|
||||
try {
|
||||
guard.canActivate(context2);
|
||||
} catch {
|
||||
// Expected to fail
|
||||
}
|
||||
const longKeyTime = Date.now() - startTime2;
|
||||
|
||||
// Times should be similar (within 10ms) to prevent timing attacks
|
||||
// Note: This is a simplified test; real timing attack prevention
|
||||
// is handled by crypto.timingSafeEqual
|
||||
expect(Math.abs(shortKeyTime - longKeyTime)).toBeLessThan(10);
|
||||
expect(guard.canActivate(correct)).toBe(true);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user