chore+fix: repo hygiene, code-review fixes, audit cleanup
Three independent code reviews + a security audit produced ~200 findings.
This commit lands the high-impact subset. Tests pass (53), typecheck
clean, eslint clean (3 minor exhaustive-deps warnings left).
REPO HYGIENE
- Add .editorconfig, .prettierrc.json, .prettierignore.
- Add ESLint flat config (.eslintrc.cjs) — correctness-focused, no style
rules (Prettier owns formatting).
- Add `format` / `format:check` / `lint` npm scripts.
- Add CHANGELOG.md (Keep a Changelog format, back-filled to 0.1.x).
- Reformat all source via Prettier so future diffs stay small.
DATA SAFETY (src/main/store.ts)
- Atomic write (tmp + rename) with retry on transient EBUSY/EPERM —
was non-atomic writeFileSync, vulnerable to truncation on power loss.
- On corrupt JSON, rename to `app-state.json.corrupt-<ts>` instead of
silently overwriting the user's exercises/history with defaults.
- Validate parsed shape before merging — reject arrays/scalars where
objects expected; per-field array checks.
- Strip `id` from incoming patches in updateExercise/updateChallenge —
a runtime caller (IPC) could otherwise smuggle id changes through.
- clearHistory now refuses an unbounded wipe (no beforeTs => no-op);
callers must pass an explicit boundary.
- unref() the debounce timer so it doesn't keep the event loop alive.
SECURITY (src/main/*)
- gsi-server: hard 256 KB body cap (was unbounded — local OOM vector),
reject any Origin/Sec-Fetch-Site header (blocks browser CSRF from
visited pages), require application/json Content-Type, generic 400
on parse error (no error string echo to client), closeAllConnections
+ async close on stop.
- dota2: validate auth.token from payload with timingSafeEqual against
the per-install token — was unauthenticated, any local process could
forge match-end events. Narrow object shape before spread-merge to
avoid throws on hostile payloads like {player:"x"}. Reset latest /
prevState after match_end so the next match starts clean.
- ipc: gate `dev:simulateMatchEnd` registration behind `!app.isPackaged`
so it does not exist in shipped builds.
- preload: gate the matching `simulateMatchEnd` export behind
`import.meta.env.MODE !== 'production'` so the bundler dead-code-
eliminates it from the production preload bundle.
- windows: shell.openExternal allowlist (http/https/mailto only) — was
forwarding any URL, including file:/javascript:/custom URI handlers
(some Windows handlers have been RCE vectors). will-navigate blocks
navigation to anywhere except file:// or the dev URL.
CORRECTNESS (src/main/* + src/shared/*)
- shared/types.ts isQuietAt: fix wrap-around + day-of-week filter.
With from=22:00 to=07:00 days=[Mon..Fri], the window started THE
PREVIOUS DAY when we're in the AM half — old code checked today's
day-of-week and got the wrong answer Sat 02:00 and Mon 01:00. Now
the filter is evaluated against the window's START day. Also reject
malformed HH:MM strings instead of producing NaN.
- scheduler: call broadcastState() after firing exercises so the
renderer's Dashboard/Exercises pages don't show stale nextFireAt
until the next state-changing IPC. Guard powerMonitor listeners
against double-registration on dev hot-reload.
- dota2: fix `launchOptionStatus = steamRunning ? 'queued' : 'queued'`
tautology — both branches now correctly read 'queued'.
- steam-launch-options: replace `require('node:fs')` inside atomicWrite
with the top-level import; retry on transient EBUSY/EPERM.
CORRECTNESS (src/renderer/*)
- lib/history.ts: replace `today.getTime() - i * MS_DAY` arithmetic
with `setDate(date - i)` calendar arithmetic in dailyRepsRange and
currentStreak — DST transitions shift epoch math by ±1h and cause
dayKey() to emit duplicate or missing days at the boundary.
- lib/icon.tsx: restrict name lookup to ICON_CHOICES set — an arbitrary
string from a corrupted state file could otherwise resolve to
unrelated Lucide exports and crash the renderer.
- lib/format.ts: guard formatCountdown against NaN/Infinity.
- i18n/index.ts: replace regex-based interpolation with split/join so
variable values containing regex metacharacters interpolate
literally; warn in dev on missing keys; clamp pluralRu(-N) via abs.
- ReminderApp: keyboard shortcuts moved INTO ExerciseReminder so Enter
respects the stepper's `adjusted` flag (was always passing planned
reps). Stepper capped at 5× planned. Don't hijack Space when a
button is focused. `key={exercise.id+nextFireAt}` forces a fresh
component for back-to-back reminders so stepper state resets. Match
summary view gets Esc-to-close. Functional setMode in onMarkDone
avoids races against stale `mode.done`.
- UpdaterCard: guard against NaN/Infinity in download-progress events
(electron-updater fires early events with undefined fields).
- Games: gate DevPanel behind `import.meta.env.DEV` in addition to the
main-side IPC gate, and narrow the `simulateMatchEnd` access.
- Add aria-labels for the +/- stepper buttons (i18n keys added).
TESTS
- +2 quiet-hours tests covering wrap-around + day-filter combo and
malformed HH:MM fallback. Total 53 passing.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
@@ -1,5 +1,12 @@
|
||||
import { app } from 'electron'
|
||||
import { existsSync, mkdirSync, readFileSync, writeFileSync } from 'node:fs'
|
||||
import {
|
||||
existsSync,
|
||||
mkdirSync,
|
||||
readFileSync,
|
||||
renameSync,
|
||||
unlinkSync,
|
||||
writeFileSync
|
||||
} from 'node:fs'
|
||||
import { join } from 'node:path'
|
||||
import { randomUUID } from 'node:crypto'
|
||||
import {
|
||||
@@ -14,9 +21,15 @@ import {
|
||||
Settings
|
||||
} from '@shared/types'
|
||||
|
||||
/** Keep at most this many entries (~3 years if ~10/day). Trim oldest. */
|
||||
/**
|
||||
* Keep at most this many history entries (≈2.7 years at 10/day).
|
||||
* When the cap is hit, drop oldest 10% so we don't trim on every write.
|
||||
*/
|
||||
const HISTORY_MAX = 10_000
|
||||
|
||||
const WRITE_DEBOUNCE_MS = 1500
|
||||
const WRITE_RETRY_DELAYS = [50, 200, 800] // ms backoff on transient EBUSY/EPERM
|
||||
|
||||
let cache: AppState | null = null
|
||||
let storePath = ''
|
||||
let pendingWrite: NodeJS.Timeout | null = null
|
||||
@@ -66,26 +79,63 @@ function makeInitial(): AppState {
|
||||
}
|
||||
}
|
||||
|
||||
/** Quarantine a corrupt state file so the user can recover it manually. */
|
||||
function quarantineCorrupt(p: string, reason: string): void {
|
||||
try {
|
||||
const stamp = new Date()
|
||||
.toISOString()
|
||||
.replace(/[:.]/g, '-')
|
||||
.replace(/Z$/, '')
|
||||
const dest = `${p}.corrupt-${stamp}`
|
||||
renameSync(p, dest)
|
||||
console.error(
|
||||
`[store] app-state.json was unreadable (${reason}); ` +
|
||||
`moved to ${dest} and starting fresh.`
|
||||
)
|
||||
} catch (e) {
|
||||
console.error('[store] failed to quarantine corrupt state file:', e)
|
||||
}
|
||||
}
|
||||
|
||||
function isValidParsed(v: unknown): v is Partial<AppState> {
|
||||
return typeof v === 'object' && v !== null && !Array.isArray(v)
|
||||
}
|
||||
|
||||
function load(): AppState {
|
||||
const p = getStorePath()
|
||||
if (!existsSync(p)) {
|
||||
const initial = makeInitial()
|
||||
writeFileSync(p, JSON.stringify(initial, null, 2), 'utf-8')
|
||||
atomicWrite(p, JSON.stringify(initial, null, 2))
|
||||
return initial
|
||||
}
|
||||
let raw: string
|
||||
try {
|
||||
const raw = readFileSync(p, 'utf-8')
|
||||
const parsed = JSON.parse(raw) as Partial<AppState>
|
||||
return {
|
||||
exercises: parsed.exercises ?? [],
|
||||
settings: { ...DEFAULT_SETTINGS, ...(parsed.settings ?? {}) },
|
||||
challenges: parsed.challenges ?? [],
|
||||
gamesEnabled: parsed.gamesEnabled ?? {},
|
||||
history: parsed.history ?? []
|
||||
}
|
||||
} catch {
|
||||
raw = readFileSync(p, 'utf-8')
|
||||
} catch (e) {
|
||||
console.error('[store] cannot read state file:', e)
|
||||
return makeInitial() // do not quarantine — we can't read it anyway
|
||||
}
|
||||
let parsed: unknown
|
||||
try {
|
||||
parsed = JSON.parse(raw)
|
||||
} catch (e) {
|
||||
quarantineCorrupt(p, `JSON parse error: ${String(e)}`)
|
||||
return makeInitial()
|
||||
}
|
||||
if (!isValidParsed(parsed)) {
|
||||
quarantineCorrupt(p, `expected object, got ${typeof parsed}`)
|
||||
return makeInitial()
|
||||
}
|
||||
return {
|
||||
exercises: Array.isArray(parsed.exercises) ? parsed.exercises : [],
|
||||
settings: { ...DEFAULT_SETTINGS, ...(parsed.settings ?? {}) },
|
||||
challenges: Array.isArray(parsed.challenges) ? parsed.challenges : [],
|
||||
gamesEnabled:
|
||||
typeof parsed.gamesEnabled === 'object' && parsed.gamesEnabled !== null
|
||||
? parsed.gamesEnabled
|
||||
: {},
|
||||
history: Array.isArray(parsed.history) ? parsed.history : []
|
||||
}
|
||||
}
|
||||
|
||||
function appendHistory(
|
||||
@@ -98,11 +148,10 @@ function appendHistory(
|
||||
const entry: HistoryEntry = { ts: Date.now(), exerciseId, action }
|
||||
if (actualReps !== undefined) entry.actualReps = actualReps
|
||||
state.history.push(entry)
|
||||
// Cap size — trim oldest 10% when over limit, so we don't trim every write.
|
||||
if (state.history.length > HISTORY_MAX) {
|
||||
state.history = state.history.slice(-Math.floor(HISTORY_MAX * 0.9))
|
||||
}
|
||||
scheduleWrite()
|
||||
// Caller schedules the write; appendHistory itself is internal.
|
||||
}
|
||||
|
||||
export function getHistory(sinceMs?: number): HistoryEntry[] {
|
||||
@@ -115,17 +164,50 @@ export function clearHistory(beforeTs?: number): number {
|
||||
const state = getState()
|
||||
const before = state.history?.length ?? 0
|
||||
if (beforeTs == null) {
|
||||
state.history = []
|
||||
} else {
|
||||
state.history = (state.history ?? []).filter((e) => e.ts >= beforeTs)
|
||||
// Refuse a full wipe via IPC — callers must pass an explicit boundary.
|
||||
// (Settings UI passes 0 to wipe everything; that's an opt-in.)
|
||||
return 0
|
||||
}
|
||||
state.history = (state.history ?? []).filter((e) => e.ts >= beforeTs)
|
||||
scheduleWrite()
|
||||
return before - (state.history?.length ?? 0)
|
||||
}
|
||||
|
||||
/**
|
||||
* Atomically write to `path` via a sibling .tmp file + rename. Retries a few
|
||||
* times on transient EBUSY/EPERM (AV/OneDrive holding the file).
|
||||
*/
|
||||
function atomicWrite(path: string, contents: string): void {
|
||||
const tmp = `${path}.tmp`
|
||||
let lastErr: unknown
|
||||
for (let i = 0; i <= WRITE_RETRY_DELAYS.length; i++) {
|
||||
try {
|
||||
writeFileSync(tmp, contents, 'utf-8')
|
||||
renameSync(tmp, path)
|
||||
return
|
||||
} catch (e) {
|
||||
lastErr = e
|
||||
// best-effort cleanup of the stale .tmp
|
||||
try {
|
||||
if (existsSync(tmp)) unlinkSync(tmp)
|
||||
} catch {
|
||||
/* ignore */
|
||||
}
|
||||
const delay = WRITE_RETRY_DELAYS[i]
|
||||
if (delay === undefined) break
|
||||
// Synchronous sleep — write path is short and called outside the hot loop.
|
||||
const until = Date.now() + delay
|
||||
while (Date.now() < until) {
|
||||
/* spin */
|
||||
}
|
||||
}
|
||||
}
|
||||
console.error('[store] atomic write failed after retries:', lastErr)
|
||||
}
|
||||
|
||||
function flush(): void {
|
||||
if (!cache) return
|
||||
writeFileSync(getStorePath(), JSON.stringify(cache, null, 2), 'utf-8')
|
||||
atomicWrite(getStorePath(), JSON.stringify(cache, null, 2))
|
||||
}
|
||||
|
||||
function scheduleWrite(): void {
|
||||
@@ -133,7 +215,10 @@ function scheduleWrite(): void {
|
||||
pendingWrite = setTimeout(() => {
|
||||
pendingWrite = null
|
||||
flush()
|
||||
}, 1500)
|
||||
}, WRITE_DEBOUNCE_MS)
|
||||
// Don't keep the event loop alive solely for a pending write — `before-quit`
|
||||
// calls `flushNow()` and we explicitly want the process to exit on schedule.
|
||||
pendingWrite.unref?.()
|
||||
}
|
||||
|
||||
export function getState(): AppState {
|
||||
@@ -178,9 +263,15 @@ export function updateExercise(
|
||||
const idx = state.exercises.findIndex((e) => e.id === id)
|
||||
if (idx === -1) return undefined
|
||||
const prev = state.exercises[idx]
|
||||
const merged: Exercise = { ...prev, ...patch }
|
||||
// Drop `id` from the patch even though the type forbids it — runtime callers
|
||||
// (IPC) can still smuggle it through. We never let the id change.
|
||||
const { id: _ignoredId, ...safePatch } = patch as Partial<Exercise>
|
||||
const merged: Exercise = { ...prev, ...safePatch }
|
||||
// If interval changed, reschedule from now.
|
||||
if (patch.intervalMinutes !== undefined && patch.intervalMinutes !== prev.intervalMinutes) {
|
||||
if (
|
||||
patch.intervalMinutes !== undefined &&
|
||||
patch.intervalMinutes !== prev.intervalMinutes
|
||||
) {
|
||||
merged.nextFireAt = Date.now() + merged.intervalMinutes * 60_000
|
||||
}
|
||||
state.exercises[idx] = merged
|
||||
@@ -258,7 +349,9 @@ export function updateChallenge(
|
||||
const state = getState()
|
||||
const idx = state.challenges.findIndex((c) => c.id === id)
|
||||
if (idx === -1) return undefined
|
||||
state.challenges[idx] = { ...state.challenges[idx], ...patch }
|
||||
// Same id-strip as updateExercise.
|
||||
const { id: _ignoredId, ...safePatch } = patch as Partial<Challenge>
|
||||
state.challenges[idx] = { ...state.challenges[idx], ...safePatch }
|
||||
scheduleWrite()
|
||||
return state.challenges[idx]
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user