review roundup (low): input size caps, parseStrictJson brace-slice, logger drops message-less events, +6 more

[devswha]

Summary

Low-severity findings from the adversarial review, grouped to avoid issue noise. None are crashes or security holes in the single-user CLI threat model; each is a latent footgun or robustness gap. Split out individually if any is worth its own fix.

---

G1 — No size cap on input file / stdin reads (memory exhaustion)

src/loader.js:129 (loadInputText → bare readFileSync(path, 'utf8')), src/cli/input.js:61-63 (stdin data += chunk, unbounded). A multi-GB file or cat huge | patina loads the whole input into a JS string before any processing. Self-inflicted for a local CLI, but there is genuinely no guard. Confidence: CERTAIN.

G2 — parseStrictJson naive brace-slice breaks on prose containing braces

src/scoring.js:60-79. After optional code-fence stripping, extraction is indexOf('{') … lastIndexOf('}'). Unfenced model output like Here is the result for {A}: {"overall": 20} slices from the {A} brace and JSON.parse throws; {"overall": 20} note: use {x} slices to the trailing {x}. Bounded impact (caught → one temp-0 re-ask → degrades to overall: null), but chatty non-fence models can spuriously null a score. Confidence: CERTAIN.

G3 — Logger silently drops any event lacking message

src/logger.js:32 — if (!fields.message) return;. The structured event name is accepted but never emitted; a caller logging logger.warn('some.event') or logger.error('x', { code }) without message produces zero output. Currently every call site passes message, so it's latent, but the contract is fragile (an error path could swallow its own diagnostic). Confidence: CERTAIN.

G4 — --max-failure-rate values in (1, ~2) silently read as a tiny percent

src/cli/args.js:559 — const ratio = n > 1 ? n / 100 : n;. --max-failure-rate 1.5 becomes 0.015 (1.5%), so with the 4-file warm-up the breaker stops after roughly the first failure. The value is genuinely ambiguous (ratio vs percent), but 1.5 almost certainly does not mean "stop at 1.5% failures." Consider warning on the (1,2) band or documenting the heuristic at the boundary. Confidence: CERTAIN.

G5 — Duplicate config merge when cwd === homedir (latent)

src/config.js:39 iterates [resolve(homedir(), '.patina.yaml'), resolve(cwd(), '.patina.yaml')]; when run from $HOME both resolve to the same path and the file is deepMerged twice. Currently idempotent (scalars last-write-wins; additive lists deduped via Set), so benign today — but any future additive key holding non-primitives would silently double-apply. Confidence: CERTAIN.

G6 — selectIndependentEvidence is O(n²) in match count

src/features/translationese.js:154-170 — for each ranked match, selected.some(kept => overlaps(...)) scans all kept; when most matches don't overlap, selected grows to N → O(N²). Measured ~160ms at ~6000 matches ('당신 '.repeat(6000), lang:'ko'). Far milder than the post-editese ReDoS, but an uncapped quadratic on attacker-influenceable input; the input is already sorted, so a sweep-line would make it linear. Confidence: CERTAIN.

G7 — stageOcrImages leaks its 0700 temp dir on an early throw

src/ocr.js:280-353 — mkdtempSync creates the dir; the caller cleans it up in a finally only because the function returns dir. The per-candidate work is individually try/caught, so a throw is unlikely today, but any throw between mkdtempSync and return leaks the temp dir permanently (caller never receives dir). Latent footgun; wrap the body in its own try/finally. Confidence: CERTAIN (mechanism), low reachability.

G8 — Backend isAuthenticated() false-positives can misdirect OCR fallback / --list-backends

src/backends/kimi-cli.js:23-28 returns true if ~/.kimi/config.toml merely exists; src/backends/gemini-cli.js:22-29 returns true if GEMINI_API_KEY is any non-empty string. A stale/empty config or invalid key makes patina pick/advertise a backend that then fails at invoke time (it still surfaces the failure, but can mis-order selectOcrBackends fallback and mislead the auth-status column). Confidence: LIKELY.

G9 — browser-diff --serve: socket error after listen is silently swallowed

src/browser-diff.js — server.on('error', rejectServer) rejects the outer promise, but once the server is listening that promise is already resolved, so a runtime socket error during a long --serve session is a no-op (rejecting a settled promise does nothing) — no log, no surface. Not a crash (the event has a listener). Confidence: LIKELY.

---

All locations were traced directly against current main. No code changes made — review only.

[devswha]

All nine roundup findings are fixed and merged to main, split across the PRs that own the relevant files:

• G1 — input file / stdin size caps → fix(batch): route file-read errors through the circuit breaker + cap input size (#503, #508 G1) #511 (loadInputText MAX_INPUT_BYTES + byte-counted stdin cap, typed inputError)
• G2 — parseStrictJson brace-slice → fix(features): ReDoS regex, MATTR window/null guards, robust JSON, linear evidence (#502, #507, #508 G2/G6) #513 (string-aware balanced {...} scan, first parseable object wins)
• G3 — logger drops message-less events → fix: low-severity robustness footguns (#508 G3, G5, G7, G9) #514 (falls back to the structured event name)
• G4 — --max-failure-rate (1,2) silent tiny-percent → fix(batch): reject empty --suffix=/--outdir= and warn on ambiguous failure-rate (#504, #508 G4) #512 (one-line stderr heads-up, behavior unchanged)
• G5 — duplicate config merge when cwd === homedir → fix: low-severity robustness footguns (#508 G3, G5, G7, G9) #514 (dedupe the user-config path list)
• G6 — selectIndependentEvidence O(n^2) → fix(features): ReDoS regex, MATTR window/null guards, robust JSON, linear evidence (#502, #507, #508 G2/G6) #513 (covered-position bitmap sweep, same selection)
• G7 — stageOcrImages leaks its 0700 temp dir on early throw → fix: low-severity robustness footguns (#508 G3, G5, G7, G9) #514 (try/catch removes the dir on throw)
• G8 — backend isAuthenticated() false-positives → fix(backends): single shared timeout budget, fall-through fallback, stricter auth (#506, #508 G8) #510 (kimi requires non-empty config.toml; gemini requires a non-blank GEMINI_API_KEY)
• G9 — browser-diff --serve swallows post-listen socket errors → fix: low-severity robustness footguns (#508 G3, G5, G7, G9) #514 (post-listen error handler surfaces + shuts down)

Each fix ships with focused unit tests. Full lint + unit + e2e suites pass on the combined result.