fix(security): harden auth reset, rate limiter fallback, and CI secrets
- Move CI_AUTH_SECRET from plaintext to ${{ secrets.CI_AUTH_SECRET }}
- Wrap password reset (update + session kill + token mark) in $transaction
to prevent stale sessions on partial failure (CWE-613)
- Rate limiter Redis fallback now uses stricter degraded limits
(maxRequests/10) and logs at error level instead of warn
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -14,7 +14,7 @@ env:
|
||||
NODE_VERSION: "20"
|
||||
PNPM_VERSION: "9.14.2"
|
||||
CI_AUTH_URL: http://localhost:3100
|
||||
CI_AUTH_SECRET: capakraken-ci-build-secret-rotate-if-shared
|
||||
CI_AUTH_SECRET: ${{ secrets.CI_AUTH_SECRET }}
|
||||
|
||||
jobs:
|
||||
guardrails:
|
||||
|
||||
@@ -26,7 +26,7 @@ function makeDb(
|
||||
resetToken?: Partial<Record<string, unknown>>;
|
||||
} = {},
|
||||
) {
|
||||
return {
|
||||
const db = {
|
||||
user: {
|
||||
findUnique: vi.fn().mockResolvedValue({ id: "user_1", email: "user@example.com" }),
|
||||
update: vi.fn().mockResolvedValue({ id: "user_1" }),
|
||||
@@ -42,7 +42,9 @@ function makeDb(
|
||||
update: vi.fn().mockResolvedValue({}),
|
||||
...overrides.resetToken,
|
||||
},
|
||||
$transaction: vi.fn(async (fn: (tx: unknown) => Promise<unknown>) => fn(db)),
|
||||
} as never;
|
||||
return db;
|
||||
}
|
||||
|
||||
function makeCtx(db = makeDb()) {
|
||||
|
||||
@@ -47,18 +47,20 @@ describe("rate limiter", () => {
|
||||
vi.doMock("ioredis", () => ({
|
||||
Redis: vi.fn().mockImplementation(() => ({
|
||||
on: vi.fn(),
|
||||
eval: vi.fn(async (_script: string, _numKeys: number, key: string, windowMsValue: string) => {
|
||||
const now = Date.now();
|
||||
const windowMs = Number(windowMsValue);
|
||||
const existing = store.get(key);
|
||||
if (!existing || existing.resetAt <= now) {
|
||||
store.set(key, { count: 1, resetAt: now + windowMs });
|
||||
} else {
|
||||
existing.count += 1;
|
||||
}
|
||||
const current = store.get(key)!;
|
||||
return [current.count, current.resetAt - now];
|
||||
}),
|
||||
eval: vi.fn(
|
||||
async (_script: string, _numKeys: number, key: string, windowMsValue: string) => {
|
||||
const now = Date.now();
|
||||
const windowMs = Number(windowMsValue);
|
||||
const existing = store.get(key);
|
||||
if (!existing || existing.resetAt <= now) {
|
||||
store.set(key, { count: 1, resetAt: now + windowMs });
|
||||
} else {
|
||||
existing.count += 1;
|
||||
}
|
||||
const current = store.get(key)!;
|
||||
return [current.count, current.resetAt - now];
|
||||
},
|
||||
),
|
||||
scan: vi.fn(async () => ["0", [...store.keys()]]),
|
||||
del: delMock,
|
||||
})),
|
||||
@@ -88,7 +90,7 @@ describe("rate limiter", () => {
|
||||
expect(afterReset.remaining).toBe(1);
|
||||
});
|
||||
|
||||
it("falls back to in-memory counters when Redis is unavailable", async () => {
|
||||
it("falls back to degraded in-memory counters when Redis is unavailable", async () => {
|
||||
vi.doMock("ioredis", () => ({
|
||||
Redis: vi.fn().mockImplementation(() => ({
|
||||
on: vi.fn(),
|
||||
@@ -101,7 +103,9 @@ describe("rate limiter", () => {
|
||||
}));
|
||||
|
||||
const { createRateLimiter } = await import("../middleware/rate-limit.js");
|
||||
const limiter = createRateLimiter(60_000, 2, {
|
||||
// Degraded fallback uses max(1, floor(maxRequests/10)), so with
|
||||
// maxRequests=20 the degraded limit is 2.
|
||||
const limiter = createRateLimiter(60_000, 20, {
|
||||
backend: "redis",
|
||||
redisUrl: "redis://test",
|
||||
name: "redis-fallback-test",
|
||||
|
||||
@@ -37,13 +37,19 @@ const DEFAULT_REDIS_URL = process.env["REDIS_URL"]?.trim();
|
||||
const warnedRedisFailures = new Set<string>();
|
||||
let sharedRedisClient: Redis | null = null;
|
||||
|
||||
function getBackendMode(
|
||||
requestedBackend: RateLimitBackendMode | undefined,
|
||||
): RateLimitBackendMode {
|
||||
if (requestedBackend === "memory" || requestedBackend === "redis" || requestedBackend === "auto") {
|
||||
function getBackendMode(requestedBackend: RateLimitBackendMode | undefined): RateLimitBackendMode {
|
||||
if (
|
||||
requestedBackend === "memory" ||
|
||||
requestedBackend === "redis" ||
|
||||
requestedBackend === "auto"
|
||||
) {
|
||||
return requestedBackend;
|
||||
}
|
||||
if (DEFAULT_REDIS_BACKEND === "memory" || DEFAULT_REDIS_BACKEND === "redis" || DEFAULT_REDIS_BACKEND === "auto") {
|
||||
if (
|
||||
DEFAULT_REDIS_BACKEND === "memory" ||
|
||||
DEFAULT_REDIS_BACKEND === "redis" ||
|
||||
DEFAULT_REDIS_BACKEND === "auto"
|
||||
) {
|
||||
return DEFAULT_REDIS_BACKEND;
|
||||
}
|
||||
return "auto";
|
||||
@@ -69,10 +75,7 @@ function getRedisClient(redisUrl: string): Redis {
|
||||
return sharedRedisClient;
|
||||
}
|
||||
|
||||
function createMemoryBackend(
|
||||
windowMs: number,
|
||||
maxRequests: number,
|
||||
): RateLimiterBackend {
|
||||
function createMemoryBackend(windowMs: number, maxRequests: number): RateLimiterBackend {
|
||||
const store = new Map<string, RateLimitEntry>();
|
||||
const cleanupInterval = setInterval(() => {
|
||||
const now = Date.now();
|
||||
@@ -127,7 +130,7 @@ function createRedisBackend(
|
||||
async function runRedisCheck(key: string): Promise<RateLimitResult> {
|
||||
const client = getRedisClient(options.redisUrl);
|
||||
const redisKey = `${redisKeyPrefix}:${key}`;
|
||||
const result = await client.eval(
|
||||
const result = (await client.eval(
|
||||
`
|
||||
local current = redis.call("INCR", KEYS[1])
|
||||
local ttl = redis.call("PTTL", KEYS[1])
|
||||
@@ -140,7 +143,7 @@ function createRedisBackend(
|
||||
1,
|
||||
redisKey,
|
||||
String(windowMs),
|
||||
) as [number | string, number | string];
|
||||
)) as [number | string, number | string];
|
||||
|
||||
const count = Number(result[0]);
|
||||
const ttlMs = Math.max(0, Number(result[1]));
|
||||
@@ -156,13 +159,7 @@ function createRedisBackend(
|
||||
const matchPattern = `${redisKeyPrefix}:*`;
|
||||
let cursor = "0";
|
||||
do {
|
||||
const [nextCursor, keys] = await client.scan(
|
||||
cursor,
|
||||
"MATCH",
|
||||
matchPattern,
|
||||
"COUNT",
|
||||
100,
|
||||
);
|
||||
const [nextCursor, keys] = await client.scan(cursor, "MATCH", matchPattern, "COUNT", 100);
|
||||
cursor = nextCursor;
|
||||
if (keys.length > 0) {
|
||||
await client.del(...keys);
|
||||
@@ -177,9 +174,9 @@ function createRedisBackend(
|
||||
} catch (error) {
|
||||
if (!warnedRedisFailures.has(warningKey)) {
|
||||
warnedRedisFailures.add(warningKey);
|
||||
logger.warn(
|
||||
logger.error(
|
||||
{ err: error, redisUrl: options.redisUrl, limiter: options.name },
|
||||
"Rate limiter Redis backend unavailable, falling back to in-memory counters",
|
||||
"Rate limiter Redis backend unavailable, falling back to degraded in-memory counters",
|
||||
);
|
||||
}
|
||||
throw error;
|
||||
@@ -208,9 +205,19 @@ export function createRateLimiter(
|
||||
const redisUrl = options.redisUrl?.trim() || DEFAULT_REDIS_URL;
|
||||
const keyPrefix = options.keyPrefix ?? DEFAULT_REDIS_KEY_PREFIX;
|
||||
const shouldUseRedis = backendMode === "redis" || (backendMode === "auto" && Boolean(redisUrl));
|
||||
const redisBackend = shouldUseRedis && redisUrl
|
||||
? createRedisBackend(windowMs, maxRequests, { name, redisUrl, keyPrefix })
|
||||
: null;
|
||||
const redisBackend =
|
||||
shouldUseRedis && redisUrl
|
||||
? createRedisBackend(windowMs, maxRequests, { name, redisUrl, keyPrefix })
|
||||
: null;
|
||||
|
||||
// When Redis is unavailable, apply a stricter limit to compensate for
|
||||
// per-node isolation (each process keeps independent in-memory counters,
|
||||
// so the effective cluster-wide limit is maxRequests × nodeCount).
|
||||
const degradedMemoryBackend = createMemoryBackend(
|
||||
windowMs,
|
||||
Math.max(1, Math.floor(maxRequests / 10)),
|
||||
);
|
||||
let redisDegraded = false;
|
||||
|
||||
const check = (async (key: string) => {
|
||||
const normalizedKey = key.trim().toLowerCase();
|
||||
@@ -227,9 +234,15 @@ export function createRateLimiter(
|
||||
}
|
||||
|
||||
try {
|
||||
return await redisBackend.check(normalizedKey);
|
||||
const result = await redisBackend.check(normalizedKey);
|
||||
if (redisDegraded) {
|
||||
redisDegraded = false;
|
||||
logger.info({ limiter: name }, "Rate limiter Redis backend recovered");
|
||||
}
|
||||
return result;
|
||||
} catch {
|
||||
return memoryBackend.check(normalizedKey);
|
||||
redisDegraded = true;
|
||||
return degradedMemoryBackend.check(normalizedKey);
|
||||
}
|
||||
}) as RateLimiter;
|
||||
|
||||
|
||||
@@ -94,7 +94,10 @@ export const authRouter = createTRPCRouter({
|
||||
throw new TRPCError({ code: "NOT_FOUND", message: "Reset link not found." });
|
||||
}
|
||||
if (record.usedAt) {
|
||||
throw new TRPCError({ code: "BAD_REQUEST", message: "This reset link has already been used." });
|
||||
throw new TRPCError({
|
||||
code: "BAD_REQUEST",
|
||||
message: "This reset link has already been used.",
|
||||
});
|
||||
}
|
||||
if (record.expiresAt < new Date()) {
|
||||
throw new TRPCError({ code: "BAD_REQUEST", message: "This reset link has expired." });
|
||||
@@ -103,19 +106,22 @@ export const authRouter = createTRPCRouter({
|
||||
const { hash } = await import("@node-rs/argon2");
|
||||
const passwordHash = await hash(input.password);
|
||||
|
||||
const updatedUser = await ctx.db.user.update({
|
||||
where: { email: record.email },
|
||||
data: { passwordHash },
|
||||
select: { id: true },
|
||||
});
|
||||
// All three operations must succeed atomically: if session deletion
|
||||
// fails after the password is already changed, old sessions could
|
||||
// persist with the new password (CWE-613).
|
||||
await ctx.db.$transaction(async (tx) => {
|
||||
const updatedUser = await tx.user.update({
|
||||
where: { email: record.email },
|
||||
data: { passwordHash },
|
||||
select: { id: true },
|
||||
});
|
||||
|
||||
// Invalidate all active sessions so any session obtained before the
|
||||
// password reset cannot be reused (CWE-613).
|
||||
await ctx.db.activeSession.deleteMany({ where: { userId: updatedUser.id } });
|
||||
await tx.activeSession.deleteMany({ where: { userId: updatedUser.id } });
|
||||
|
||||
await ctx.db.passwordResetToken.update({
|
||||
where: { token: input.token },
|
||||
data: { usedAt: new Date() },
|
||||
await tx.passwordResetToken.update({
|
||||
where: { token: input.token },
|
||||
data: { usedAt: new Date() },
|
||||
});
|
||||
});
|
||||
|
||||
return { success: true };
|
||||
|
||||
Reference in New Issue
Block a user