fix(19): satisfy CI fast-checks + secret scan
CI / changes (pull_request) Successful in 9s
CI / api (pull_request) Successful in 3m2s
CI / fast-checks (pull_request) Successful in 4m20s
CI / security (pull_request) Successful in 1m14s
CI / harness (pull_request) Successful in 6m56s
CI / gate (pull_request) Successful in 2s
CI / changes (pull_request) Successful in 9s
CI / api (pull_request) Successful in 3m2s
CI / fast-checks (pull_request) Successful in 4m20s
CI / security (pull_request) Successful in 1m14s
CI / harness (pull_request) Successful in 6m56s
CI / gate (pull_request) Successful in 2s
Lint (eslint --max-warnings 0): - index.ts: disable no-unsafe-argument on the type-only Context mismatch when delegating to the OIDC handler inside the local-session skip wrapper - localAuth.ts: handleLogout is sync (no await) — drop async (require-await) - devBypass.ts: disable detect-possible-timing-attacks on the public well-known dev-placeholder string compare (not a secret comparison) - remove dead code / unused bindings flagged by no-unused-vars: makeTestApp (localSession.test), makeUnauthContext + BrowserContext import (login.spec), unused memberId (admin.test), unused txSelectCount counter (me.test) - localAuthMiddleware.test / me.test: fix unused + reflow-detached eslint-disable directives Format: prettier --write across the 20 Phase-19 files that were never formatted. Secret scan (gitleaks): allowlist two false positives — the synthetic >=32-char TEST_SECRET in localSession.test.ts, and .planning/ design prose (a generic-api-key regex hit on "credential atomically, 409-equivalent"). Neither is a real secret. Verified locally: format:check, lint, typecheck, md:lint, gitleaks (no leaks), PWA 266/266, API 452/452. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
91ab9d1f78
commit
b6490feff4
@@ -153,6 +153,9 @@ export function devSessionCookieMiddleware(): MiddlewareHandler {
|
||||
// BL-01: warn loudly if the secret is the well-known dev placeholder. A genuine,
|
||||
// signature-valid session token minted under this known value is trivially forgeable
|
||||
// if the same secret ever leaks into a non-bypass environment.
|
||||
// Not a security comparison — this matches against a PUBLIC well-known placeholder to
|
||||
// emit a warning, so constant-time equality is irrelevant here.
|
||||
// eslint-disable-next-line security/detect-possible-timing-attacks
|
||||
if (secret === 'dev-secret-change-me-0000000000000000') {
|
||||
console.warn(
|
||||
'[devSessionCookieMiddleware] LOCAL_SESSION_SECRET is the well-known dev placeholder. ' +
|
||||
|
||||
@@ -128,7 +128,10 @@ app.get('/callback', async (c) => {
|
||||
return c.redirect('/?error=oidc-link-conflict');
|
||||
}
|
||||
// Unexpected error during link binding — log and continue with normal redirect.
|
||||
console.error('[callback] linkOidcToUser unexpected error:', err instanceof Error ? err.message : String(err));
|
||||
console.error(
|
||||
'[callback] linkOidcToUser unexpected error:',
|
||||
err instanceof Error ? err.message : String(err),
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -193,6 +196,9 @@ if (!devBypassActive) {
|
||||
await next();
|
||||
return;
|
||||
}
|
||||
// oidcHandler's parameter is typed as the generic Hono Context; our wrapper's c is the
|
||||
// same runtime Context narrowed to '/api/*' — the structural mismatch is type-only.
|
||||
// eslint-disable-next-line @typescript-eslint/no-unsafe-argument
|
||||
await oidcHandler(c, next);
|
||||
});
|
||||
// Re-issues the session-scoped oidc-auth cookie as persistent so PWA sessions survive close (AUTH-02).
|
||||
|
||||
@@ -140,77 +140,73 @@ const createMemberSchema = z.object({
|
||||
initialPassword: z.string().min(8),
|
||||
});
|
||||
|
||||
adminRouter.post(
|
||||
'/members',
|
||||
zValidator('json', createMemberSchema, noEchoHook),
|
||||
async (c) => {
|
||||
const { displayName, username, initialPassword } = c.req.valid('json');
|
||||
// T-19-06: NEVER log request body, displayName, username, or initialPassword here
|
||||
adminRouter.post('/members', zValidator('json', createMemberSchema, noEchoHook), async (c) => {
|
||||
const { displayName, username, initialPassword } = c.req.valid('json');
|
||||
// T-19-06: NEVER log request body, displayName, username, or initialPassword here
|
||||
|
||||
// Assign the first palette color not already in use (mirrors upsertUser color logic)
|
||||
const usedRows = await db.select({ color: users.color }).from(users);
|
||||
const usedColors = new Set(usedRows.map((r) => r.color));
|
||||
const color =
|
||||
COLOR_PALETTE.find((c) => !usedColors.has(c)) ??
|
||||
COLOR_PALETTE[usedColors.size % COLOR_PALETTE.length];
|
||||
// Assign the first palette color not already in use (mirrors upsertUser color logic)
|
||||
const usedRows = await db.select({ color: users.color }).from(users);
|
||||
const usedColors = new Set(usedRows.map((r) => r.color));
|
||||
const color =
|
||||
COLOR_PALETTE.find((c) => !usedColors.has(c)) ??
|
||||
COLOR_PALETTE[usedColors.size % COLOR_PALETTE.length];
|
||||
|
||||
// WR-03: hash the initial password BEFORE opening the transaction so the (now async,
|
||||
// threadpool) scrypt work does not hold the DB transaction open for its duration.
|
||||
const initialPasswordHash = await hashPassword(initialPassword);
|
||||
// WR-03: hash the initial password BEFORE opening the transaction so the (now async,
|
||||
// threadpool) scrypt work does not hold the DB transaction open for its duration.
|
||||
const initialPasswordHash = await hashPassword(initialPassword);
|
||||
|
||||
try {
|
||||
let newUserId: number;
|
||||
try {
|
||||
let newUserId: number;
|
||||
|
||||
// T-19-10: atomic transaction — both inserts succeed or both roll back
|
||||
await db.transaction(async (tx) => {
|
||||
// Insert the new users row (no oidcIss/oidcSub — local-only member)
|
||||
const [inserted] = await tx
|
||||
.insert(users)
|
||||
.values({
|
||||
displayName,
|
||||
color,
|
||||
isAdmin: false,
|
||||
claimed: false, // no OIDC identity bound yet
|
||||
})
|
||||
.$returningId();
|
||||
newUserId = inserted.id;
|
||||
// T-19-10: atomic transaction — both inserts succeed or both roll back
|
||||
await db.transaction(async (tx) => {
|
||||
// Insert the new users row (no oidcIss/oidcSub — local-only member)
|
||||
const [inserted] = await tx
|
||||
.insert(users)
|
||||
.values({
|
||||
displayName,
|
||||
color,
|
||||
isAdmin: false,
|
||||
claimed: false, // no OIDC identity bound yet
|
||||
})
|
||||
.$returningId();
|
||||
newUserId = inserted.id;
|
||||
|
||||
// Insert local_credentials row with hashed initial password
|
||||
// If username is already in use, the UNIQUE constraint fires here and rolls back
|
||||
await tx.insert(localCredentials).values({
|
||||
userId: newUserId,
|
||||
username,
|
||||
passwordHash: initialPasswordHash,
|
||||
});
|
||||
// Insert local_credentials row with hashed initial password
|
||||
// If username is already in use, the UNIQUE constraint fires here and rolls back
|
||||
await tx.insert(localCredentials).values({
|
||||
userId: newUserId,
|
||||
username,
|
||||
passwordHash: initialPasswordHash,
|
||||
});
|
||||
});
|
||||
|
||||
// Return the new user's id (the PWA uses it to navigate to the member)
|
||||
return c.json({ id: newUserId! }, 201);
|
||||
} catch (err) {
|
||||
// Username uniqueness violation — UNIQUE constraint on local_credentials.username.
|
||||
// Drizzle wraps the mysql2 error; check the cause chain or the message for ER_DUP_ENTRY.
|
||||
const isDup =
|
||||
(err instanceof Error && err.message.includes('ER_DUP_ENTRY')) ||
|
||||
(err != null &&
|
||||
typeof err === 'object' &&
|
||||
'code' in err &&
|
||||
(err as { code?: string }).code === 'ER_DUP_ENTRY') ||
|
||||
(err != null &&
|
||||
typeof err === 'object' &&
|
||||
'cause' in err &&
|
||||
(err as { cause?: { code?: string } }).cause?.code === 'ER_DUP_ENTRY');
|
||||
if (isDup) {
|
||||
return c.json({ error: 'Username already in use' }, 409);
|
||||
}
|
||||
// Unexpected errors — log message only, never the body or password
|
||||
console.error(
|
||||
'[admin/POST /members] Unexpected error:',
|
||||
err instanceof Error ? err.message : String(err),
|
||||
);
|
||||
return c.json({ error: 'Service unavailable' }, 503);
|
||||
// Return the new user's id (the PWA uses it to navigate to the member)
|
||||
return c.json({ id: newUserId! }, 201);
|
||||
} catch (err) {
|
||||
// Username uniqueness violation — UNIQUE constraint on local_credentials.username.
|
||||
// Drizzle wraps the mysql2 error; check the cause chain or the message for ER_DUP_ENTRY.
|
||||
const isDup =
|
||||
(err instanceof Error && err.message.includes('ER_DUP_ENTRY')) ||
|
||||
(err != null &&
|
||||
typeof err === 'object' &&
|
||||
'code' in err &&
|
||||
(err as { code?: string }).code === 'ER_DUP_ENTRY') ||
|
||||
(err != null &&
|
||||
typeof err === 'object' &&
|
||||
'cause' in err &&
|
||||
(err as { cause?: { code?: string } }).cause?.code === 'ER_DUP_ENTRY');
|
||||
if (isDup) {
|
||||
return c.json({ error: 'Username already in use' }, 409);
|
||||
}
|
||||
},
|
||||
);
|
||||
// Unexpected errors — log message only, never the body or password
|
||||
console.error(
|
||||
'[admin/POST /members] Unexpected error:',
|
||||
err instanceof Error ? err.message : String(err),
|
||||
);
|
||||
return c.json({ error: 'Service unavailable' }, 503);
|
||||
}
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// POST /api/admin/members/:id/password
|
||||
|
||||
@@ -186,7 +186,10 @@ localAuthRouter.post('/local/login', zValidator('json', loginSchema, noEchoHook)
|
||||
.limit(1);
|
||||
cred = found;
|
||||
} catch (err) {
|
||||
console.error('[localAuth/POST /local/login] DB error:', err instanceof Error ? err.message : String(err));
|
||||
console.error(
|
||||
'[localAuth/POST /local/login] DB error:',
|
||||
err instanceof Error ? err.message : String(err),
|
||||
);
|
||||
return c.json({ error: 'Service unavailable' }, 503);
|
||||
}
|
||||
|
||||
@@ -200,7 +203,12 @@ localAuthRouter.post('/local/login', zValidator('json', loginSchema, noEchoHook)
|
||||
|
||||
if (!valid || !cred) {
|
||||
// Increment failure counter (keyed on username)
|
||||
const cur = loginAttempts.get(key) ?? { count: 0, lockedUntil: 0, lockedOut: false, lockedAt: 0 };
|
||||
const cur = loginAttempts.get(key) ?? {
|
||||
count: 0,
|
||||
lockedUntil: 0,
|
||||
lockedOut: false,
|
||||
lockedAt: 0,
|
||||
};
|
||||
cur.count += 1;
|
||||
cur.lockedUntil = Date.now() + RATE_WINDOW_SECS * 1000;
|
||||
cur.lockedOut = cur.count >= LOCKOUT_FAILURES;
|
||||
@@ -215,7 +223,10 @@ localAuthRouter.post('/local/login', zValidator('json', loginSchema, noEchoHook)
|
||||
try {
|
||||
await issueLocalSessionCookie(c, cred.userId);
|
||||
} catch (err) {
|
||||
console.error('[localAuth/POST /local/login] Cookie issue error:', err instanceof Error ? err.message : String(err));
|
||||
console.error(
|
||||
'[localAuth/POST /local/login] Cookie issue error:',
|
||||
err instanceof Error ? err.message : String(err),
|
||||
);
|
||||
return c.json({ error: 'Service unavailable' }, 503);
|
||||
}
|
||||
return c.json({ ok: true }, 200);
|
||||
@@ -226,7 +237,7 @@ localAuthRouter.post('/local/login', zValidator('json', loginSchema, noEchoHook)
|
||||
// GET /local/logout → GET /api/auth/local/logout (browser-redirect alias)
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
async function handleLogout(c: Context) {
|
||||
function handleLogout(c: Context) {
|
||||
clearLocalSessionCookie(c);
|
||||
return c.json({ ok: true }, 200);
|
||||
}
|
||||
|
||||
+47
-47
@@ -105,7 +105,9 @@ meRouter.get('/', async (c) => {
|
||||
// but isAdmin, needsProviderSetup, and hasLocalCredential are still resolved from the DB (T-10-05).
|
||||
const devUser = c.get('user');
|
||||
if (devUser) {
|
||||
const { isAdmin, needsProviderSetup, hasLocalCredential } = await resolveAdminAndSetupStatus(devUser.id);
|
||||
const { isAdmin, needsProviderSetup, hasLocalCredential } = await resolveAdminAndSetupStatus(
|
||||
devUser.id,
|
||||
);
|
||||
return c.json({
|
||||
user: {
|
||||
id: devUser.id,
|
||||
@@ -141,7 +143,9 @@ meRouter.get('/', async (c) => {
|
||||
return c.json({ error: 'Could not resolve user' }, 500);
|
||||
}
|
||||
|
||||
const { isAdmin, needsProviderSetup, hasLocalCredential } = await resolveAdminAndSetupStatus(user.id);
|
||||
const { isAdmin, needsProviderSetup, hasLocalCredential } = await resolveAdminAndSetupStatus(
|
||||
user.id,
|
||||
);
|
||||
|
||||
return c.json({
|
||||
user: {
|
||||
@@ -232,57 +236,53 @@ const mePasswordSchema = z.object({
|
||||
newPassword: z.string().min(8),
|
||||
});
|
||||
|
||||
meRouter.post(
|
||||
'/password',
|
||||
zValidator('json', mePasswordSchema, meNoEchoHook),
|
||||
async (c) => {
|
||||
// T-19-07: ALWAYS resolve userId from session — never from body
|
||||
const currentUserId = await resolveUserId(c);
|
||||
if (!currentUserId) {
|
||||
return c.json({ error: 'Unauthorized' }, 401);
|
||||
}
|
||||
meRouter.post('/password', zValidator('json', mePasswordSchema, meNoEchoHook), async (c) => {
|
||||
// T-19-07: ALWAYS resolve userId from session — never from body
|
||||
const currentUserId = await resolveUserId(c);
|
||||
if (!currentUserId) {
|
||||
return c.json({ error: 'Unauthorized' }, 401);
|
||||
}
|
||||
|
||||
const { currentPassword, newPassword } = c.req.valid('json');
|
||||
// T-19-06: NEVER log currentPassword, newPassword, or the request body
|
||||
const { currentPassword, newPassword } = c.req.valid('json');
|
||||
// T-19-06: NEVER log currentPassword, newPassword, or the request body
|
||||
|
||||
// Look up the user's local_credentials row (404 if none — no local credential to change)
|
||||
const [credRow] = await db
|
||||
.select({ passwordHash: localCredentials.passwordHash, userId: localCredentials.userId })
|
||||
.from(localCredentials)
|
||||
.where(eq(localCredentials.userId, currentUserId))
|
||||
.limit(1);
|
||||
// Look up the user's local_credentials row (404 if none — no local credential to change)
|
||||
const [credRow] = await db
|
||||
.select({ passwordHash: localCredentials.passwordHash, userId: localCredentials.userId })
|
||||
.from(localCredentials)
|
||||
.where(eq(localCredentials.userId, currentUserId))
|
||||
.limit(1);
|
||||
|
||||
if (!credRow) {
|
||||
return c.json({ error: 'No local credential found' }, 404);
|
||||
}
|
||||
if (!credRow) {
|
||||
return c.json({ error: 'No local credential found' }, 404);
|
||||
}
|
||||
|
||||
// T-19-07: verify current password before any update (WR-03: async scrypt)
|
||||
const isCorrect = await verifyPassword(credRow.passwordHash, currentPassword);
|
||||
if (!isCorrect) {
|
||||
// CR-03: return 403 (NOT 401) for a wrong current password. The PWA's global
|
||||
// MutationCache treats any 401 as "session expired" and arms the re-auth
|
||||
// interstitial / login redirect — so a 401 here would force-log-out a user who
|
||||
// merely mistyped their current password. 403 is in-app authorization-failure and
|
||||
// lets the client surface "current password incorrect" without dropping the session.
|
||||
return c.json({ error: 'Current password incorrect' }, 403);
|
||||
}
|
||||
// T-19-07: verify current password before any update (WR-03: async scrypt)
|
||||
const isCorrect = await verifyPassword(credRow.passwordHash, currentPassword);
|
||||
if (!isCorrect) {
|
||||
// CR-03: return 403 (NOT 401) for a wrong current password. The PWA's global
|
||||
// MutationCache treats any 401 as "session expired" and arms the re-auth
|
||||
// interstitial / login redirect — so a 401 here would force-log-out a user who
|
||||
// merely mistyped their current password. 403 is in-app authorization-failure and
|
||||
// lets the client surface "current password incorrect" without dropping the session.
|
||||
return c.json({ error: 'Current password incorrect' }, 403);
|
||||
}
|
||||
|
||||
try {
|
||||
await db
|
||||
.update(localCredentials)
|
||||
.set({ passwordHash: await hashPassword(newPassword) })
|
||||
.where(eq(localCredentials.userId, currentUserId));
|
||||
try {
|
||||
await db
|
||||
.update(localCredentials)
|
||||
.set({ passwordHash: await hashPassword(newPassword) })
|
||||
.where(eq(localCredentials.userId, currentUserId));
|
||||
|
||||
return c.json({ ok: true }, 200);
|
||||
} catch (err) {
|
||||
console.error(
|
||||
'[me/POST /password] Unexpected error:',
|
||||
err instanceof Error ? err.message : String(err),
|
||||
);
|
||||
return c.json({ error: 'Service unavailable' }, 503);
|
||||
}
|
||||
},
|
||||
);
|
||||
return c.json({ ok: true }, 200);
|
||||
} catch (err) {
|
||||
console.error(
|
||||
'[me/POST /password] Unexpected error:',
|
||||
err instanceof Error ? err.message : String(err),
|
||||
);
|
||||
return c.json({ error: 'Service unavailable' }, 503);
|
||||
}
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// POST /api/me/link-oidc — initiate OIDC identity-link flow (AUTH-LOCAL-10, T-19-09)
|
||||
|
||||
Reference in New Issue
Block a user