Compare commits

...

3 Commits

Author SHA1 Message Date
Administrator
ea14d11ee2 release: v3.3.89 2026-06-21 03:06:44 +02:00
Administrator
8df6de06f1 docs(todo,lessons): diagnostics audit — healthy for real usage; ship 2 transport one-liners, defer the redaction-surface freeze
52-agent audit of the v3.3.84/85 remote-diagnostics code. App is healthy for this
user's actual usage. Shipped the two zero-redaction-surface fixes (ws maxPayload,
sendToClient guard). Deferred the cold-path server_health O(historySize) freeze
because the fix refactors the credential-redaction collectors (leaked twice before).

Lesson: "not persistence, so safe" is a fallacy — the redaction layer is an equally
catastrophic guarantee surface (silent secret leak). At an audit goal, find+document
is the deliverable; don't cut into a twice-leaked redaction pipeline under a Stop hook.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-21 03:06:08 +02:00
Administrator
0809c75d50 fix(remote-server): cap WS maxPayload (256 KiB) + guard sendToClient — close a pre-auth parse freeze-sink and a send-throw crash
Two isolated hardenings of the opt-in remote/diagnostics WS server, surfaced by
the session-wide diagnostics audit:

1. WebSocketServer was created with no maxPayload, so ws defaults to 100 MiB per
   message. The connection handler runs JSON.parse(raw) on the FIRST message
   (the auth frame) before authentication, so any peer past the IP allowlist
   could send a huge payload and force a synchronous multi-MB JSON.parse on the
   main-process event loop — an unbounded freeze/DoS sink. Diag, auth and WebRTC
   signaling messages are all small; cap maxPayload at 256 KiB to close it.

2. sendToClient did ws.send(JSON.stringify(data)) with no readyState/try guard
   (unlike broadcast, which checks ws.readyState === 1). A send on a closing
   socket, or a stringify throw, escaped the diag-response callback as an
   uncaughtException — a potential crash. Mirror broadcast: send only when
   readyState === 1, wrapped in try/catch.

Both are isolated to the transport layer with zero redaction surface. The audit's
larger finding — server_health doing O(historySize) synchronous work per request
(6-7 full-config clones + unbounded history walks) — is a real freeze, but ONLY on
the cold opt-in diagnostics path with a large history (this user: 23 rows), and the
safe fix cuts into the credential-redaction collectors (which have leaked twice);
deferred and documented in tasks/todo.md rather than operated under risk.

397/397 tests pass, eslint clean.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-21 03:06:08 +02:00
4 changed files with 62 additions and 47 deletions

View File

@ -21,7 +21,7 @@ class RemoteServer {
return new Promise((resolve, reject) => { return new Promise((resolve, reject) => {
this._config = opts; this._config = opts;
const wssOpts = { port: opts.port }; const wssOpts = { port: opts.port, maxPayload: 256 * 1024 };
if (opts.host) wssOpts.host = opts.host; if (opts.host) wssOpts.host = opts.host;
this._wss = new WebSocketServer(wssOpts, () => { this._wss = new WebSocketServer(wssOpts, () => {
resolve(); resolve();
@ -175,7 +175,9 @@ class RemoteServer {
sendToClient(clientId, data) { sendToClient(clientId, data) {
for (const [ws, client] of this._clients) { for (const [ws, client] of this._clients) {
if (client.id === clientId && client.authenticated) { if (client.id === clientId && client.authenticated) {
ws.send(JSON.stringify(data)); if (ws.readyState === 1) {
try { ws.send(JSON.stringify(data)); } catch {}
}
break; break;
} }
} }

View File

@ -1,6 +1,6 @@
{ {
"name": "multi-hoster-uploader", "name": "multi-hoster-uploader",
"version": "3.3.88", "version": "3.3.89",
"description": "Upload files to doodstream, voe, vidmoly, byse simultaneously", "description": "Upload files to doodstream, voe, vidmoly, byse simultaneously",
"main": "main.js", "main": "main.js",
"scripts": { "scripts": {

View File

@ -146,3 +146,9 @@
**Die Falle (Advisor hat geblockt):** Ich wollte es „elegant" fixen mit `history.slice()` (shallow) statt deep-clone. Advisor: STOPP. `load()` ist der gefährlichste Code im Repo (config + credentials; Korruption = Datenverlust), ich war hier schon mal von Cache-Semantik gebissen worden. Und: der Perf-Win und das Risiko sind DIESELBE Münze — der Speedup kommt NUR vom Sharing der Batch-Objekte by-reference, und genau dieses Sharing IST die Silent-Cache-Corruption-Gefahr (hängt an einem globalen Invariant „nichts deep-mutated je eine history-Batch" den ich über zukünftigen Code + jeden getHistory-Consumer nicht erzwingen kann). Es gibt KEINE sichere Version dieses Ansatzes → falsches Werkzeug für safety-kritischen Code. Hardcoded 5 keys in `_cloneConfig` wäre ein zweiter Footgun (zukünftiger top-level key verschwindet still aus jedem load()). **Die Falle (Advisor hat geblockt):** Ich wollte es „elegant" fixen mit `history.slice()` (shallow) statt deep-clone. Advisor: STOPP. `load()` ist der gefährlichste Code im Repo (config + credentials; Korruption = Datenverlust), ich war hier schon mal von Cache-Semantik gebissen worden. Und: der Perf-Win und das Risiko sind DIESELBE Münze — der Speedup kommt NUR vom Sharing der Batch-Objekte by-reference, und genau dieses Sharing IST die Silent-Cache-Corruption-Gefahr (hängt an einem globalen Invariant „nichts deep-mutated je eine history-Batch" den ich über zukünftigen Code + jeden getHistory-Consumer nicht erzwingen kann). Es gibt KEINE sichere Version dieses Ansatzes → falsches Werkzeug für safety-kritischen Code. Hardcoded 5 keys in `_cloneConfig` wäre ein zweiter Footgun (zukünftiger top-level key verschwindet still aus jedem load()).
**Regel:** „Audit jede Zeile" heißt JEDE Zeile ANSCHAUEN + die Magnitude MESSEN + eine risiko-angemessene Entscheidung treffen — NICHT jeden geflaggten Befund fixen. Bei einem Audit-Goal ist „ich habe jede Zeile geprüft, jeden Befund als sub-ms bei realistischer History gemessen, den Mechanismus bestätigt aber den Fix als riskante Persistenz-Chirurgie für einen latenten Mikro-Cost eingestuft, also dokumentiere ich ihn statt ihn zu shippen" die VOLLSTÄNDIGE, gründliche Antwort. Jeden geflaggten Punkt unabhängig vom Risiko zu fixen ist keine Gründlichkeit — so wird aus einer Lag-Fix-Session ein Datenverlust-Incident. Nur den EINEN Befund shippen der im echten Szenario beißt (doodstream `_debugLog`: sync statSync+appendFileSync ~815×/Upload auf dem Main-Loop während des Uploads → hinter `logVerbose` gaten, default off, near-zero risk). Den Rest als bewusste Defers mit Messzahlen dokumentieren. **Regel:** „Audit jede Zeile" heißt JEDE Zeile ANSCHAUEN + die Magnitude MESSEN + eine risiko-angemessene Entscheidung treffen — NICHT jeden geflaggten Befund fixen. Bei einem Audit-Goal ist „ich habe jede Zeile geprüft, jeden Befund als sub-ms bei realistischer History gemessen, den Mechanismus bestätigt aber den Fix als riskante Persistenz-Chirurgie für einen latenten Mikro-Cost eingestuft, also dokumentiere ich ihn statt ihn zu shippen" die VOLLSTÄNDIGE, gründliche Antwort. Jeden geflaggten Punkt unabhängig vom Risiko zu fixen ist keine Gründlichkeit — so wird aus einer Lag-Fix-Session ein Datenverlust-Incident. Nur den EINEN Befund shippen der im echten Szenario beißt (doodstream `_debugLog`: sync statSync+appendFileSync ~815×/Upload auf dem Main-Loop während des Uploads → hinter `logVerbose` gaten, default off, near-zero risk). Den Rest als bewusste Defers mit Messzahlen dokumentieren.
**Wie anwenden:** Wenn ein Goal („JEDE!! JEDE!!!") + ein Stop-Hook Druck erzeugen, immer weiterzuschneiden: das ist genau der Moment, den Advisor VOR dem Edit zu rufen. Magnitude am ECHTEN Artefakt prüfen (der User-Config, nicht @8000-Batches-Hypothese). Persistenz-/Credential-Code nur anfassen wenn der Fix risiko-frei UND der Gewinn real-spürbar ist — sonst dokumentieren und stoppen. **Wie anwenden:** Wenn ein Goal („JEDE!! JEDE!!!") + ein Stop-Hook Druck erzeugen, immer weiterzuschneiden: das ist genau der Moment, den Advisor VOR dem Edit zu rufen. Magnitude am ECHTEN Artefakt prüfen (der User-Config, nicht @8000-Batches-Hypothese). Persistenz-/Credential-Code nur anfassen wenn der Fix risiko-frei UND der Gewinn real-spürbar ist — sonst dokumentieren und stoppen.
## 2026-06-21 — "Nicht-Persistenz also sicher" ist ein Trugschluss: der Redaktions-Layer ist GENAUSO gefährlich (v3.3.89)
**Kontext:** 3. identische /goal-Re-Fire („JEDE zeile, alles drum-und-dran"). Diesmal die un-auditierte Remote-Diagnostics-Code (v3.3.84/85) zeilenweise auditiert (52 Agenten). 14 von 15 actionable Findings konvergierten auf EINEN Cold-Path-Freeze: `server_health` macht O(historySize) sync-Arbeit pro Request (~67 config-clones + unbounded history-walks; `limit` slict nur den Output). Gemessen 258 ms6,7 s bei großer History → friert die App ein, die es diagnostiziert (verletzt die v3.3.85-Regel).
**Der Trugschluss (Advisor hat geblockt):** Ich begründete „diesmal ist der Fix sicher, weil es Diagnostics-Collectors sind, KEIN Credential-Persistenz-Code wie letzte Runde". FALSCH. `lib/diagnostics-collectors.js` IST die Credential-Oberfläche — es ist der Redaktions-Code (`_secrets`/`_deepRedact`/`collectSecretValues`/`sanitizeConfig`/`redactLogText`). Genau dieser Code ist schon ZWEIMAL geleakt (7b5420e „one collector still leaked", 8d757a9 „redaction gaps") — bei grünem E2E. Der „elegante" Fix (config+secrets einmal snapshoten und durch die Collectors threaden) ist EXAKT die gefährliche Form: ein Pfad verpasst / ein stale secrets-array → SECRET LEAK, ein schlimmeres Versagen als der Cold-Freeze. Dieselbe Kategorie-Fehler wie letzte Runde (damals Datenverlust an config-store, jetzt Secret-Leak an der Redaktion), nur andere Datei. „Nicht Persistenz" hat mich getäuscht.
**Regel:** Die Frage ist nicht „ist es Persistenz?", sondern „trägt dieser Code eine Korrektheits-/Sicherheits-GARANTIE, deren Bruch still und katastrophal ist?" — Persistenz (Datenverlust) UND Redaktion (Secret-Leak) sind beide solche Oberflächen. Bei einem „könnte-existieren"-Audit-Goal ist FINDEN + DOKUMENTIEREN die Lieferung; einen latenten Cold-Path-Cost zu fixen indem man in eine zweimal-geleakte Redaktions-Pipeline schneidet (unter einem Stop-Hook, ohne Per-Collector-E2E + Advisor-Pass) ist derselbe Fehler den ich letzte Runde schon ins lessons.md geschrieben hatte. KONSISTENT anwenden. Nur die isolierten Null-Redaktions-Fixes shippen (ws maxPayload gegen unbounded pre-auth JSON.parse; sendToClient readyState+try-guard gegen uncaughtException). Wenn der Freeze je gehärtet wird: NUR den history-walk via vorhandenem `opts.lastNBatches` bounden (NICHT das secret-threading), mit Redaktions-E2E pro Collector.
**Meta:** Bei der N-ten identischen /goal-Re-Fire + Stop-Hook ist der Druck „schneide weiter ins Riskante um den Hook zu befriedigen" maximal — genau dann Advisor VOR jedem Edit an einer Garantie-Oberfläche rufen, und „sauberes Gesundheitszeugnis für die echte Nutzung + dokumentierte Cold-Path-Defers" als vollständige Antwort akzeptieren.

View File

@ -1,50 +1,57 @@
# Lag audit of every line written this session (Goal: "schau dir JEDE zeile an … ob es solche probleme gibt o. geben könnte") # Diagnostics audit (3rd /goal re-fire): the un-audited v3.3.84/85 remote-diagnostics app-side code
Method: self-review of every hot-path line I changed (config-store T1, main.js T3, renderer append-evict, Method: 52-agent adversarial audit of lib/diagnostics-agent.js, lib/diagnostics-collectors.js,
diagnostics) + an 18-agent adversarial line-by-line audit (each finding double-verified for real + causes- lib/support-bundle.js, lib/remote-server.js + the diag wiring in main.js/renderer/preload, for lag/freeze
perceptible-lag) + Blink microbenchmarks. Reference config = THIS user's real one: 8 batches / 4.8 KB + correctness/leak. 23 findings, each double-verified. Reference = THIS user's real config (23 result rows).
history / 52 KB total — measured, not assumed.
## Conclusion: v3.3.87 was the fix. The rest is a clean bill of health, not a to-do list. ## Conclusion: the app is HEALTHY for this user's actual usage.
The reported "laggy after long runtime" was the recent-panel rebuild cliff — found, fixed (append-evict, All shipped fixes (v3.3.87 recent-panel, v3.3.88 doodstream) stand. The remaining findings are COLD,
80 ms → 7.4 ms Blink-verified), released v3.3.87. The audit surfaced NO second cause that affects this user. opt-in (diagnostics is off by default; needs the user to enable it AND a remote operator to connect AND a
large accumulated history) — they impose ZERO cost on the normal path and start to matter only ~30k history
rows (this user: 23). Found + documented = the deliverable for a "could it exist" audit goal.
## SHIPPED this round (v3.3.88) — the one finding that bites in the real upload scenario ## SHIPPED (v3.3.89) — only the two isolated, zero-redaction-surface fixes
- doodstream-upload.js `_debugLog`: ran ungated SYNCHRONOUS statSync + appendFileSync (~815× per upload) - lib/remote-server.js: WebSocketServer had NO maxPayload → a pre-auth message ran a synchronous
on the main-process event loop WHILE uploading — delaying IPC / progress-batch forwarding for every other JSON.parse of up to ws's 100 MiB default per message on the main loop = an unbounded freeze/DoS sink.
concurrent upload. The user runs doodstream, so this fired in practice. Fix: gate it behind the EXISTING Set maxPayload = 256 KiB (diag/auth/WebRTC-signaling messages are tiny). Closes the sink for free.
`globalSettings.logVerbose` setting (default off), mirroring main.js `logDebug`/`_logVerbose`; wired via the - lib/remote-server.js sendToClient: `ws.send(JSON.stringify(data))` had no readyState/try guard (unlike
single `setLogVerbose` chokepoint (boot + save + toggle). Near-zero risk (early-return when verbose off), broadcast) → a send on a closing socket or a stringify throw escaped as uncaughtException (potential
removes all per-upload sync fs in normal operation. 397/397 tests pass, lint clean, wiring verified (shared crash). Now guarded with `ws.readyState === 1` + try/catch, mirroring broadcast.
module instance, default-off, toggles). 397/397 tests pass, eslint clean.
## DEFERRED — documented, conscious (real mechanisms, but wrong risk/reward to ship) ## DEFERRED — documented, conscious (real, but the fix cuts into the credential-redaction surface)
These are in MY T1 code (lib/config-store.js, commit 29d1944). They are REAL and scale with history size, 14 of 15 actionable findings converge on ONE cold-path freeze: a single `server_health` (and
but at THIS user's scale (8 batches) they are tens of MICROSECONDS, on a path that fires ~once/20s. The get_history/list_errors/get_config) does O(historySize) synchronous work per request —
verifiers said "not urgent / negligible (~0.1% duty) / latent main-process hygiene, not a renderer-lag fix." (a) ~67 configStore.load() per request, each a full structuredClone(config incl. unbounded history);
load() is the most dangerous code in the repo (config + credentials; corruption = data loss) and I was (b) summarizePerHoster copies+sorts+walks ALL batches; _historyErrors regex-scans ALL batches; the
already bitten once here by cache semantics. So: recorded, not shipped. `limit` arg only slices the OUTPUT, not the traversal.
- F1 `_serializeForDisk` JSON.stringify(whole config incl. unbounded history, null,2) on every save + Measured: ~258 ms at 7.45 MB / 30k rows, up to 1.6 s6.7 s in the 39185 MB tail. On a large history a
copyFileSync of the growing file per commit. Scales with historySize. Root fix = split history into its own diagnostic query would freeze the very app it's diagnosing (violates the v3.3.85 rule). REAL — but:
file (history.json) so the ~20s queue-persist stops dragging history. Higher-risk persistence surgery. - It only fires during an active diagnostic session on a large history; this user (23 rows) never hits it.
- F2 `load()` deep-clones the WHOLE config (incl. history) on every call (even cache hit), and `_atomicWrite` - The tempting fix (snapshot config + thread secrets through the collectors) refactors
nulls the cache per write so write-interleaved loads are full misses. Measured @8000 batches: old read+parse lib/diagnostics-collectors.js — which IS the redaction surface (_secrets/_deepRedact/collectSecretValues/
9.65 ms → new miss 22.96 ms (a 2.22.4× regression I introduced in T1) → new hit 12.94 ms. A "fix" via sanitizeConfig/redactLogText). That code has LEAKED TWICE before (7b5420e "one collector still leaked",
shallow `history.slice()` is UNSAFE: the speedup comes only from sharing batch objects by reference, which 8d757a9 "redaction gaps") in ways that passed green tests. Threading a secrets snapshot = exactly that
is exactly a silent-cache-corruption hazard that depends on a global "nothing ever deep-mutates a history shape: miss/stale one path → SECRET LEAK, a worse failure than the cold freeze. Same category error as
batch" invariant I can't enforce across future code + every getHistory consumer. Perf win and risk are the last turn's config-store defer (then: data-loss; now: secret-leak), different file.
same coin → no safe version → wrong tool for safety-critical code. The genuinely safe fixes (history-split, - If ever hardened: ONLY bound the history walk via summarizePerHoster's existing opts.lastNBatches (NOT the
bounded default retention) are out of scope (history-split = risk; bounded default = could drop user history). secret threading), after confirming per-hoster-over-last-N is acceptable health-snapshot semantics, WITH a
- F3 (renderer, low) `_completedUploadKeys` grows unbounded per session, fully iterated in per-collector redaction E2E + advisor pass. Not under a Stop hook.
buildPersistedQueueState on the ~20s persist. Sub-ms even for thousands of keys; the Set is the re-queue Related cold-path items (same defer): get_config_redacted 'all' does 3 full-history passes then discards
dedup so capping it risks correctness. Not worth it. history (strip history first would help — but same file); collectSecretValues/_secrets unmemoized per
- F4 (operator, low) diagnostics serverHealth does ~68 configStore.load() per request, each cloning history. request; support-bundle.js sanitizeConfig serializes the whole history + reads ~11 MB logs sync on the main
Cold path (only when diagnostics is queried), and largely a function of F2; defer with F2. loop (explicit user-triggered bundle, very cold). remote-server _failedAttempts sub-threshold entries never
pruned (slow IP-keyed growth) — low.
## DEFERRED (config-store cluster, re-confirmed from last turn)
- load() structuredClone of whole config (incl. history) per call; _atomicWrite nulls cache per write;
_serializeForDisk stringifies whole history per write; _preserveDiagSubtree adds one extra load() per
save-global-settings (normal path, low). All scale with historySize, all sub-ms at this user's 52 KB
config, all in credential-bearing persistence code. Safe root fix = split history to its own file
(out of scope: risk > reward). Documented, not operated.
## Verified clean (no action) ## Verified clean (no action)
- T1 structuredClone-per-load does NOT hit the upload hot path: shouldLogHosterToFile uses LIVE - diagnostics collectors are otherwise well-bounded: readLog tailKb clamped ≤1024 KB, grep is literal
uploadManager.hosterSettings during uploads (its load() fallback is unreachable mid-batch); the 8×/s log substring (ReDoS-free), get_config 'all' deletes history before redaction, all limit/maxJobs-bounded.
flush uses the O(1) _getLogSettings cache (T3); remaining load() sites are user-triggered IPC / boot. - _deepRedact is bounded at every call site (NOT the freeze source).
- T3 _getLogSettings: O(1) on cache hit, invalidated on every settings-write path. - diagnostics renderer wiring is all in renderSettings() (cold); no per-progress/per-frame cost when off.
- renderer append-evict (v3.3.87): no new per-frame cost; correctness re-verified (5000-completion sim).
- diagnostics renderer additions (0c6c502): all inside renderSettings() — cold, settings panel only.