Repository navigation
fix(frontend): harden all 7 findings from the JS security/data-loss review - #65
Open
loonvault-agent[bot] wants to merge 1 commit into
Open
loonvault-agent[bot] wants to merge 1 commit into
loonvault-agent[bot] wants to merge 1 commit into
Conversation
…eview Security review of all six first-party JS files found no exploitable vulnerability but 7 anti-patterns; this lands all of them: 1. app.js: fetches had no timeout — a HANGING (not failed) live API stalled "Loading…" forever and the snapshot fallback never ran. AbortSignal.timeout (8s, feature-detected) guarantees fallback. 2. app.js: broad .catch conflated render() exceptions with fetch failures — a display bug could show "Backend is offline" (or refetch + mislabel the source) while the backend was healthy. Two-arg .then separates the error channels; every failure path now logs; render bugs get their own message. 3. md-renderer.js: safeHref's root-relative alternative accepted protocol-relative (//evil.com) and backslash (/\evil.com, normalized by browsers to //) URLs — off-site navigation through the scheme allowlist. Now /(?![/\\]). 4. viewer.js: sidebar grouping used a plain object — a doc group named __proto__/constructor hit the prototype and crashed the docs sidebar. Object.create(null). 5. md-renderer.js: bold/italic passes ran over built <a> tags and rewrote hrefs containing * (no XSS — quotes escaped — but broken links). Opening tags are now placeholder-stashed like inline code; bold inside link TEXT still renders. 6. api-worker.js: response headers passed through wholesale — a future Location/debug header would leak the execute-api host the worker exists to hide. Response headers are now allowlisted (content-type, CORS set, vary). 7. worker.js: all client headers were forwarded to the origin; the site is credential-free, so cookie/authorization are now stripped (everything else, e.g. accept-encoding, still forwards). Verified with a 39-check node suite: all prior #64 behavior (escaping, javascript: blocking, preflight forwarding, rejection statuses, apex redirect, path allowlists) plus each finding's fix and its non-regression (bold-in-link-text, accept-encoding passthrough, ACAO passthrough). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Follow-up to the JS security & data-loss review: lands all 7 findings (the review found no exploitable vulnerability; these are hardening + data-availability fixes). Each fix was empirically verified — the finding was first reproduced, then the fix confirmed, plus non-regression checks.
app.jsAbortSignal.timeout(8000), feature-detectedapp.js.catchconflatesrender()bugs with fetch failures → "Backend is offline" while the backend is healthy, or a mislabeled data source; errors swallowed silently.thenseparates error channels; all failures logged; render bugs get a distinct messagemd-renderer.jssafeHrefaccepted protocol-relative//evil.comand backslash/\evil.comURLs (verified live off-site nav)\/(?![/\\])viewer.js__proto__/constructorcrashes the docs sidebar (verified TypeError)Object.create(null)md-renderer.js*(verified<strong>inside the attribute — no XSS, but broken links)<a>tags placeholder-stashed like inline code; bold inside link text still rendersapi-worker.jsexecute-apihost (e.g. aLocationheader) the worker exists to hideworker.jscookie/authorizationstripped; everything else (e.g.accept-encoding) still forwardsVerification
39-check node suite (scratchpad, not committed): every #64 behavior re-verified (escaping,
javascript:blocking, preflight forwarding, 405/404/414/400/421/502 rejections, apex redirect, path allowlists) + each finding's repro-then-fix + non-regressions (bold-in-link-text renders,accept-encodingpassthrough, ACAO passthrough, body integrity).node --checkclean on all five files.cloudflare/worker.jsANDcloudflare/api-worker.js— both changed), thenjust deploy-frontend+ purge the Cloudflare cache.🤖 Generated with Claude Code