fix: Phase 1 critical+high fixes (17/18 items)
CRITICAL fixes: - C1: Error swallowing - throw TransportError on network failure in getEmails/searchEmails - C2: Recurrence expansion ID delimiter changed from ':' to '::occurrence::' - C3: Cross-account calendar event UID dedup after multi-account aggregation - C4: Admin session token revocation via JTI blacklist on logout - C6: FTS5 schema-drop - add warning log for automatic reindex trigger - C7: Settings lock - gate updateSetting() with isSettingLocked() check - C8: Offline push pause - add offline event handler that closes push transports HIGH fixes: - H1: Push handler - add ContactCard and FileNode branches - H2: WS fallback - await state snapshot before reconcileAfterWebSocketFallback - H3: Auth rate limiting - add checkUserAuthRateLimit to session and token routes - H4: OAuth logs - strip access_token from error log context - H7: Template XSS - apply DOMPurify to HTML template body on import - H8: Secure cookie - derive from x-forwarded-proto, not NODE_ENV - H9: bcrypt fix - remove bcrypt prefixes from isHashed() so scrypt-only - H13: calendarTasksEnabled - apply admin gate at runtime in calendar page - H14: Task mutations - add try/catch error handling to update/delete/toggle - H18: autoSelectReplyIdentity default changed from false to true Deferred: P1.3 (C5 auth localStorage encryption) - requires custom Zustand persist adapter.
This commit is contained in:
@@ -58,7 +58,7 @@ function verifyPassword(password: string, stored: string): Promise<boolean> {
|
||||
}
|
||||
|
||||
function isHashed(value: string): boolean {
|
||||
return value.startsWith('$scrypt$') || value.startsWith('$2a$') || value.startsWith('$2b$');
|
||||
return value.startsWith('$scrypt$');
|
||||
}
|
||||
|
||||
// ─── Disk I/O ───────────────────────────────────────────────────────────────
|
||||
|
||||
+31
-9
@@ -1,9 +1,11 @@
|
||||
/**
|
||||
* In-memory rate limiter for admin login.
|
||||
* Max 5 attempts per IP per 15 minutes.
|
||||
* In-memory rate limiter for admin login and user authentication.
|
||||
* Admin: max 5 attempts per IP per 15 minutes.
|
||||
* User auth: max 10 attempts per (IP + username) per 15 minutes.
|
||||
*/
|
||||
|
||||
const MAX_ATTEMPTS = 5;
|
||||
const MAX_ADMIN_ATTEMPTS = 5;
|
||||
const MAX_USER_ATTEMPTS = 10;
|
||||
const WINDOW_MS = 15 * 60 * 1000; // 15 minutes
|
||||
|
||||
interface RateLimitEntry {
|
||||
@@ -28,18 +30,38 @@ setInterval(() => {
|
||||
*/
|
||||
export function checkRateLimit(ip: string): { allowed: boolean; remaining: number; retryAfterMs: number } {
|
||||
const now = Date.now();
|
||||
const entry = attempts.get(ip);
|
||||
const entry = attempts.get(`admin:${ip}`);
|
||||
|
||||
if (!entry || entry.resetAt <= now) {
|
||||
// New window
|
||||
attempts.set(ip, { count: 1, resetAt: now + WINDOW_MS });
|
||||
return { allowed: true, remaining: MAX_ATTEMPTS - 1, retryAfterMs: 0 };
|
||||
attempts.set(`admin:${ip}`, { count: 1, resetAt: now + WINDOW_MS });
|
||||
return { allowed: true, remaining: MAX_ADMIN_ATTEMPTS - 1, retryAfterMs: 0 };
|
||||
}
|
||||
|
||||
if (entry.count >= MAX_ATTEMPTS) {
|
||||
if (entry.count >= MAX_ADMIN_ATTEMPTS) {
|
||||
return { allowed: false, remaining: 0, retryAfterMs: entry.resetAt - now };
|
||||
}
|
||||
|
||||
entry.count++;
|
||||
return { allowed: true, remaining: MAX_ATTEMPTS - entry.count, retryAfterMs: 0 };
|
||||
return { allowed: true, remaining: MAX_ADMIN_ATTEMPTS - entry.count, retryAfterMs: 0 };
|
||||
}
|
||||
|
||||
/**
|
||||
* Check rate limit for user authentication, keyed by IP + username.
|
||||
*/
|
||||
export function checkUserAuthRateLimit(ip: string, username: string): { allowed: boolean; remaining: number; retryAfterMs: number } {
|
||||
const now = Date.now();
|
||||
const key = `user:${ip}:${username}`;
|
||||
const entry = attempts.get(key);
|
||||
|
||||
if (!entry || entry.resetAt <= now) {
|
||||
attempts.set(key, { count: 1, resetAt: now + WINDOW_MS });
|
||||
return { allowed: true, remaining: MAX_USER_ATTEMPTS - 1, retryAfterMs: 0 };
|
||||
}
|
||||
|
||||
if (entry.count >= MAX_USER_ATTEMPTS) {
|
||||
return { allowed: false, remaining: 0, retryAfterMs: entry.resetAt - now };
|
||||
}
|
||||
|
||||
entry.count++;
|
||||
return { allowed: true, remaining: MAX_USER_ATTEMPTS - entry.count, retryAfterMs: 0 };
|
||||
}
|
||||
|
||||
+29
-3
@@ -11,6 +11,13 @@ const TAG_LENGTH = 16;
|
||||
|
||||
const MIN_SECRET_LENGTH = 32;
|
||||
|
||||
const revokedTokens = new Map<string, number>(); // jti → expiry timestamp
|
||||
|
||||
function isHttpsRequest(req: { headers: Headers }): boolean {
|
||||
const proto = req.headers.get('x-forwarded-proto');
|
||||
return proto === 'https';
|
||||
}
|
||||
|
||||
function getKey(): Buffer {
|
||||
const secret = getSessionSecret();
|
||||
if (!secret) throw new Error('SESSION_SECRET not configured');
|
||||
@@ -37,10 +44,12 @@ export function createAdminSession(): string {
|
||||
const cipher = createCipheriv(ALGORITHM, key, iv);
|
||||
|
||||
const now = Math.floor(Date.now() / 1000);
|
||||
const exp = now + getSessionTTL();
|
||||
const payload: AdminSessionPayload = {
|
||||
role: 'admin',
|
||||
iat: now,
|
||||
exp: now + getSessionTTL(),
|
||||
exp,
|
||||
jti: randomBytes(16).toString('hex'),
|
||||
};
|
||||
|
||||
const json = JSON.stringify(payload);
|
||||
@@ -74,12 +83,29 @@ export function verifyAdminSession(token: string): AdminSessionPayload | null {
|
||||
const now = Math.floor(Date.now() / 1000);
|
||||
if (payload.exp < now) return null;
|
||||
|
||||
// Clean up expired revocations while we're here
|
||||
for (const [jti, expiry] of revokedTokens) {
|
||||
if (expiry < now) revokedTokens.delete(jti);
|
||||
}
|
||||
|
||||
if (payload.jti && revokedTokens.has(payload.jti)) return null;
|
||||
|
||||
return payload;
|
||||
} catch {
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Revoke an admin session token so it cannot be used again.
|
||||
*/
|
||||
export function revokeAdminSession(token: string): void {
|
||||
const payload = verifyAdminSession(token);
|
||||
if (payload?.jti) {
|
||||
revokedTokens.set(payload.jti, payload.exp);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* CSRF gate for cookie-authed admin requests.
|
||||
*
|
||||
@@ -146,12 +172,12 @@ export async function requireAdminAuth(request: Request): Promise<{ payload: Adm
|
||||
/**
|
||||
* Set the admin session cookie.
|
||||
*/
|
||||
export async function setAdminSessionCookie(): Promise<void> {
|
||||
export async function setAdminSessionCookie(request?: { headers: Headers }): Promise<void> {
|
||||
const token = createAdminSession();
|
||||
const cookieStore = await cookies();
|
||||
cookieStore.set(ADMIN_SESSION_COOKIE, token, {
|
||||
httpOnly: true,
|
||||
secure: process.env.NODE_ENV === 'production',
|
||||
secure: request ? isHttpsRequest(request) : process.env.NODE_ENV === 'production',
|
||||
sameSite: 'lax',
|
||||
path: '/',
|
||||
maxAge: getSessionTTL(),
|
||||
|
||||
@@ -30,6 +30,7 @@ export interface AdminSessionPayload {
|
||||
role: 'admin';
|
||||
iat: number;
|
||||
exp: number;
|
||||
jti?: string;
|
||||
}
|
||||
|
||||
export interface SettingRestriction {
|
||||
|
||||
+31
-1
@@ -3,10 +3,21 @@ import type { SieveScript, SieveCapabilities } from "./sieve-types";
|
||||
import type { IJMAPClient } from "./client-interface";
|
||||
import { toWildcardQuery } from "./search-utils";
|
||||
import { batched, itemsPerRequest } from "./request-limits";
|
||||
import { noteTransportFailure, noteTransportSuccess } from "./transport-health";
|
||||
import { noteTransportFailure, noteTransportSuccess, transportFailureCount } from "./transport-health";
|
||||
import { debug } from "@/lib/debug";
|
||||
import { normalizeCalendarEventLike } from "@/lib/calendar-event-normalization";
|
||||
|
||||
export class TransportError extends Error {
|
||||
constructor(message = 'Network transport failure') {
|
||||
super(message);
|
||||
this.name = 'TransportError';
|
||||
}
|
||||
}
|
||||
|
||||
function wasTransportFailure(beforeCount: number): boolean {
|
||||
return transportFailureCount() > beforeCount;
|
||||
}
|
||||
|
||||
/** Parse a recipient string that may be "Name <email>" or bare "email" into { name?, email }. */
|
||||
function parseRecipientString(s: string): { name?: string; email: string } {
|
||||
const trimmed = s.trim();
|
||||
@@ -1220,6 +1231,7 @@ export class JMAPClient implements IJMAPClient {
|
||||
}
|
||||
|
||||
async getEmails(mailboxId?: string, accountId?: string, limit: number = 50, position: number = 0, hasKeyword?: string, pinnedFirst?: boolean, extraFilter?: Record<string, unknown>): Promise<{ emails: Email[], hasMore: boolean, total: number }> {
|
||||
const tcBefore = transportFailureCount();
|
||||
try {
|
||||
const targetAccountId = accountId || this.accountId;
|
||||
const simple: { inMailbox?: string; hasKeyword?: string } = {};
|
||||
@@ -1290,6 +1302,9 @@ export class JMAPClient implements IJMAPClient {
|
||||
|
||||
return { emails: [], hasMore: false, total: 0 };
|
||||
} catch (error) {
|
||||
if (wasTransportFailure(tcBefore)) {
|
||||
throw new TransportError('Failed to get emails: network transport failure');
|
||||
}
|
||||
console.error('Failed to get emails:', error);
|
||||
return { emails: [], hasMore: false, total: 0 };
|
||||
}
|
||||
@@ -2099,6 +2114,7 @@ export class JMAPClient implements IJMAPClient {
|
||||
}
|
||||
|
||||
async searchEmails(query: string, mailboxId?: string, accountId?: string, limit: number = 50, position: number = 0): Promise<{ emails: Email[], hasMore: boolean, total: number }> {
|
||||
const tcBefore = transportFailureCount();
|
||||
try {
|
||||
const targetAccountId = accountId || this.accountId;
|
||||
|
||||
@@ -2154,6 +2170,9 @@ export class JMAPClient implements IJMAPClient {
|
||||
|
||||
return { emails, hasMore, total };
|
||||
} catch (error) {
|
||||
if (wasTransportFailure(tcBefore)) {
|
||||
throw new TransportError('Search failed: network transport failure');
|
||||
}
|
||||
console.error('Search failed:', error);
|
||||
return { emails: [], hasMore: false, total: 0 };
|
||||
}
|
||||
@@ -6041,6 +6060,7 @@ export class JMAPClient implements IJMAPClient {
|
||||
private lastSSEActivity: number = 0;
|
||||
private visibilityHandler: (() => void) | null = null;
|
||||
private onlineHandler: (() => void) | null = null;
|
||||
private offlineHandler: (() => void) | null = null;
|
||||
|
||||
// JMAP-over-WebSocket (RFC 8887) push - preferred over SSE when the server
|
||||
// advertises it (getWebSocketUrl()), since it's the transport the desktop
|
||||
@@ -6301,6 +6321,7 @@ export class JMAPClient implements IJMAPClient {
|
||||
* original 31s-worst-case ladder did.
|
||||
*/
|
||||
private async reconcileAfterWebSocketFallback(): Promise<void> {
|
||||
await this._stateSnapshotPromise;
|
||||
await this.checkForStateChanges();
|
||||
|
||||
const eventSourceUrl = this.getEventSourceUrl();
|
||||
@@ -6781,6 +6802,11 @@ export class JMAPClient implements IJMAPClient {
|
||||
}
|
||||
|
||||
if (typeof window !== 'undefined') {
|
||||
this.offlineHandler = () => {
|
||||
this.closePushNotifications();
|
||||
};
|
||||
window.addEventListener('offline', this.offlineHandler);
|
||||
|
||||
this.onlineHandler = () => {
|
||||
// Network reconnected - reconnect WS/SSE or force a poll. Don't
|
||||
// make the user wait through whatever backoff delay was already in
|
||||
@@ -6814,6 +6840,10 @@ export class JMAPClient implements IJMAPClient {
|
||||
document.removeEventListener('visibilitychange', this.visibilityHandler);
|
||||
this.visibilityHandler = null;
|
||||
}
|
||||
if (this.offlineHandler && typeof window !== 'undefined') {
|
||||
window.removeEventListener('offline', this.offlineHandler);
|
||||
this.offlineHandler = null;
|
||||
}
|
||||
if (this.onlineHandler && typeof window !== 'undefined') {
|
||||
window.removeEventListener('online', this.onlineHandler);
|
||||
this.onlineHandler = null;
|
||||
|
||||
@@ -228,7 +228,10 @@ export class MailIndex {
|
||||
let version = opened.version;
|
||||
|
||||
if (version !== null && version !== SCHEMA_VERSION) {
|
||||
// Rebuildable derived data: drop, don't migrate.
|
||||
console.warn(
|
||||
`[MailIndex] Schema version mismatch (stored=${version}, current=${SCHEMA_VERSION}). ` +
|
||||
'Dropping all tables — a full reindex will run on the next push event or /api/offline/reindex call.',
|
||||
);
|
||||
db.exec('DROP TABLE IF EXISTS doc_fts; DROP TABLE IF EXISTS doc; DROP TABLE IF EXISTS meta;');
|
||||
version = null;
|
||||
}
|
||||
|
||||
@@ -121,7 +121,8 @@ export async function exchangeCodeForTokens(
|
||||
const tokens = await tokenResponse.json();
|
||||
|
||||
if (!tokens.access_token) {
|
||||
logger.error('Token response missing access_token', { response: JSON.stringify(tokens).substring(0, 500) });
|
||||
const { access_token, refresh_token, ...safeTokens } = tokens;
|
||||
logger.error('Token response missing access_token', { response: JSON.stringify(safeTokens).substring(0, 500) });
|
||||
throw new Error('Invalid token response');
|
||||
}
|
||||
|
||||
|
||||
@@ -239,7 +239,7 @@ function createOccurrence(
|
||||
return {
|
||||
...master,
|
||||
...(override || {}),
|
||||
id: `${master.id}:${recurrenceId}`,
|
||||
id: `${master.id}::occurrence::${recurrenceId}`,
|
||||
originalId: master.originalId || master.id,
|
||||
uid: master.uid,
|
||||
calendarIds: master.calendarIds,
|
||||
|
||||
@@ -173,7 +173,7 @@ export function importTemplates(json: string): ImportResult {
|
||||
id: generateUUID(),
|
||||
name: sanitizeText(t.name),
|
||||
subject: sanitizeText(t.subject),
|
||||
body: t.isHTML ? String(t.body || '') : sanitizeText(t.body),
|
||||
body: t.isHTML ? DOMPurify.sanitize(String(t.body || ''), { ALLOWED_TAGS: ['b', 'i', 'u', 'strong', 'em', 'a', 'p', 'br', 'ul', 'ol', 'li', 'div', 'span', 'table', 'tr', 'td', 'th', 'thead', 'tbody', 'img', 'h1', 'h2', 'h3', 'blockquote'], ALLOWED_ATTR: ['href', 'src', 'alt', 'style', 'class', 'target'] }) : sanitizeText(t.body),
|
||||
isHTML: Boolean(t.isHTML),
|
||||
category: sanitizeText(t.category),
|
||||
defaultRecipients: recipients && typeof recipients === 'object'
|
||||
|
||||
Reference in New Issue
Block a user