fix(security): CSP functional truth, desktop crypto, doc-truth hardening (v1.24.2) - #284
Merged
Conversation
…ob: (F-01..F-04) script-src on all 5 deployment surfaces (index.html, vercel.json, public/_headers, nginx.conf x3, tauri.conf.json) has been 'self' without 'wasm-unsafe-eval' since faad8f0 (2026-05-27), silently blocking WebAssembly.instantiate in every Chromium browser in production. The advertised local-inference stack (WebLLM, ONNX Runtime Web, Transformers.js, DuckDB-WASM, Whisper-STT, Kokoro-TTS) never functioned. The same commit added an unhashed inline <script> that its own CSP-strategy comment said wasn't needed. No existing gate caught either: csp.test.ts/deploymentHeaders .test.ts only assert cross-surface consistency, and smoke-prod-build.mjs listened only for pageerror, which CSP violations never fire. Fix: 'wasm-unsafe-eval' (never the broader 'unsafe-eval') + frame-src 'self' blob: added to all 5 surfaces; the inline aurora-disabled script moved into index.tsx as a same-origin module statement. Added a 3-layer test architecture so this class of defect can't silently recur: - Layer B (new): tests/unit/cspCorrectness.test.ts — functional-directive assertions across all 5 surfaces, including a check for unhashed inline <script> tags that would have caught the 2026-05-27 defect on day one. - Layer C (hardened): scripts/smoke-prod-build.mjs now captures securitypolicyviolation DOM events + console block/refusal messages, and runs a real WebAssembly.instantiate probe in headless Chromium. Layer C surfaced a 4th, previously-masked violation after the WASM fix: zod v4's own internal JIT-capability probe (new Function("") in a try/catch, traced to the exact blocked column) is 'unsafe-eval'-classified but already handled gracefully by zod itself (jitless fallback). Per the never-'unsafe-eval' rule this is not fixed by loosening CSP — it's excluded from the violation count via a narrowly matched, documented filter and logged separately as known-benign. Verified plugin-sandbox WASM denial (workers/plugin.worker.ts) is CSP-independent and unaffected — adversarial tests in tests/unit/workers/plugin.worker.test.ts stay green. New ADR-0013 records the full decision, alternatives rejected, and the plugin- sandbox non-impact analysis. docs/DEPLOYMENT.md and docs/SECURITY-THREAT-MODEL.md cross-reference it. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Adds a "which layer catches which failure class" table to docs/CI.md documenting that Layer A (cross-surface consistency, csp.test.ts / deploymentHeaders.test.ts) was never sufficient on its own — the faad8f0 defect passed it for two months because it checks a different property than the one that broke. Layer B (cspCorrectness.test.ts) and Layer C (hardened smoke-prod-build.mjs) close that gap. Documents the accepted Tauri Layer-C gap: Playwright drives real Chromium via vite preview, not the Tauri WebView, so no runtime probe exists for the Tauri CSP specifically — Layer A+B still cover it, and building a full WebView harness was judged disproportionate given the Tauri CSP is the strictest of the 5 surfaces (a missed enforcement quirk there is more likely to be over-strict than under-enforcing). Also confirms and documents (via ci.yml inspection) that the build job's smoke:prod step has no if: gate excluding PRs. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
services/fs/fsCore.ts#deriveFileSystemCryptoKey derived the desktop
Tauri BYOK API-key encryption key from a single unsalted SHA-256
digest of publicly-derivable material
(${appDataPath}|${provider}|WorldScriptStudio|v1 — anyone with
read access to config/<provider>_key.enc.json already knows its own
parent path and the provider from the filename). This was
obfuscation, not encryption.
Fixed to PBKDF2 (600,000 iterations, SHA-256) + a random 32-byte
salt persisted alongside the ciphertext, extractable: false —
mirroring services/storage/storageEncryptionService.ts#deriveKey.
Per the locked decision, pre-existing key files are discarded, not
migrated: getApiKey now detects the legacy (no salt field) format,
removes the stale file, and dispatches a one-time notification via
appStoreRef/statusActions (the codebase's established pattern for
non-React service notifications) rather than a silent null return
indistinguishable from "no key ever set".
Two real TypeScript strictness bugs were caught by pnpm run
typecheck (the exact CI command) during verification, not present
in the original audit:
- base64ToBytes's bare `Uint8Array` return type widened to
`Uint8Array<ArrayBufferLike>`, rejected by crypto.subtle as a
BufferSource — fixed with the same `Uint8Array<ArrayBuffer>`
pattern already used in libraryBackupService.ts.
- getApiKey's `let apis: TauriApis | undefined` didn't narrow
across the retryFs() closure boundary — fixed with a properly
scoped `const apis` inside try, separate outer refs for cleanup.
Doc truth-up: README.md gains a differentiated "which mechanism
protects what" table (4 distinct paths — browser BYOK key, browser
IDB-at-rest, desktop BYOK key, library backup vault) replacing the
blanket "Web Crypto API (AES-256-GCM + PBKDF2) / encrypted at rest"
claims. Also removes a fabricated claim that desktop builds use
tauri-plugin-stronghold for OS-keychain protection — zero trace of
that dependency exists anywhere in the repo (no Cargo dep, no npm
package, no source file). CLAUDE.md de-blanketed to point at the
README table. docs/SECURITY-THREAT-MODEL.md gains a mitigations row
for fsCore.ts/settingsFsStore.ts (previously zero mentions) and a
new attack-tree section for the desktop local-file-read vector.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…s (F-08) Tauri's strict connect-src (no https: blanket) was missing endpoints for two shipped features: LanguageTool's default self-hosted port and the Hugging Face hosts WebLLM/Transformers.js resolve models from. Confirmed empirically, not guessed: - grep of node_modules/@mlc-ai/web-llm and @huggingface/transformers confirms both default to https://huggingface.co (no custom model_url/env.remoteHost override in packages/ai-core/src/). - curl -sIL against a real resolve/main/*.bin weight-file URL shows huggingface.co 302-redirects to https://us.aws.cdn.hf.co (HF's Xet CDN storage backend) for the actual bytes. Adding only huggingface.co would have left downloads silently broken. Scope widened beyond the audit's F-08 during verification: directly inspecting the web meta CSP's connect-src (not assuming) showed localhost:8010 was missing on all 5 surfaces, not just Tauri. The audit's "web https: masks the gap" reasoning doesn't apply here — LanguageTool's default URL is http://, not https://, and CSP scheme-matching is exact. LanguageTool has never worked against its own documented default local server, on any platform, under CSP enforcement, since it shipped. Fixed on all 5 surfaces, consistent with the existing 11434/1234/8000 local-service pattern; does not touch the https: scheme-source tradeoff (ADR-0004). New regression coverage in tests/unit/cspCorrectness.test.ts: port 8010 asserted across all 5 surfaces, HF hosts asserted for Tauri (web's https: already covers HTTPS hosts per ADR-0004). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Both workers/duckdbWorker.ts and workers/v2/duckdb.worker.ts loaded
DuckDB-WASM's mvp/eh bundles from unversioned
cdn.jsdelivr.net/npm/@duckdb/duckdb-wasm/dist/... URLs — floating
"latest", decoupled from the locked @duckdb/duckdb-wasm@^1.32.0 npm
dependency, and a third-party supply-chain trust dependency in an
app that markets "offline-first" and "no data is sent to a server".
worker-src 'self' blob:' had no jsdelivr allowance on any of the 5
CSP surfaces, so this was already dead code, not just a risk.
New scripts/copy-duckdb-assets.mjs copies the 4 dist assets (~72 MB)
from the pinned npm dependency into public/duckdb/ at
predev/prebuild time. Not committed to git (.gitignore'd) — build
artifacts of an existing pinned dependency, not source. Both worker
files now reference ${import.meta.env.BASE_URL}duckdb/... (absolute,
same-origin path, safe from both main-thread and nested-worker
construction contexts).
Real regression caught during verification: the new *.worker.js
files matched the SW precache globPatterns allowlist (unlike the
.wasm binaries, already excluded). Fixed by extending
vite.config.ts's globIgnores with '**/duckdb/**' — otherwise ~1.5 MB
of feature-flag-gated, lazily-loaded worker scripts would have been
precached at cold start.
Verified: bundle:budget passes (static assets don't count toward
the JS-chunk budget), smoke-prod-build.mjs passes (0 CSP
violations, wasm: ok), no cdn.jsdelivr.net references remain.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Two conflicting production URLs existed in the repo: worldscript-studio-indol.vercel.app (in-app link in GeneralSections.tsx, and the Italian locale) vs. worldscript-studio.vercel.app (README, CLAUDE.md). Verified live before touching anything: worldscript-studio.vercel.app returns 200 (title "WorldScript Studio"); worldscript-studio-indol.vercel.app returns 404. New PRODUCTION_URL constant in constants/brand.ts (single source of truth, same pattern as the existing APP_NAME/APP_FILE_SLUG). The in-app link now reads the constant instead of a hardcoded dead URL. locales/it/help.json corrected (the only locale source with the dead domain — not all 19 as originally assumed), then pnpm run i18n:bundle regenerated all bundles; only it/bundle.json actually changed (1 line). scripts/check-doc-metrics.mjs gains scanForUrlDrift() + getCanonicalProductionUrl(): scans README.md and CLAUDE.md for any *.vercel.app URL that doesn't match the constant, reusing the existing historical-section-aware scanning. This is the check that would have caught the original drift on day one. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…t-runner invocation (F-12) Storybook was built 3x per CI run: twice in the quality job's Node 22/24 matrix (pnpm run build-storybook) plus once more in the dedicated storybook job. Removed the matrix-job build — tsgo already runs separately in quality, so no Node-version-specific compile check was lost. ci.yml:2 header "StoryCraft Studio" -> "WorldScript Studio" (pre-rebrand leftover). While evaluating whether the storybook job's `test-storybook ... || true` could be made blocking, checked actual CI run logs (gh run view --log) instead of trusting the green job conclusion, which `|| true` makes meaningless for this step. Found the real bug: `error: unknown option '--max-workers=2'` -- @storybook/test-runner@^0.24.4's CLI doesn't support --max-workers (wrong case; correct is --maxWorkers), and doesn't support --retries or --screenshot-on-failure at all in this version. Every prior run failed on the first unknown flag before any story executed, printed --help text, and || true silently turned that into a green step -- this gate has produced zero real signal since it was added. Fixed the invocation to only pass supported flags (--maxWorkers=2 --junit). Kept || true for now since this is the first run where test-storybook will actually execute real stories -- no track record exists yet to judge stability against. Comment now names a concrete re-evaluation point (~10 real runs on main) instead of an open-ended "once stable." Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Both v1 (workers/duckdbWorker.ts, workers/inference.worker.ts) and v2 (workers/v2/duckdb.worker.ts, workers/v2/inference.worker.ts, workers/v2/webllm.worker.ts) worker generations are live via real, non-test call chains, confirmed by tracing every import. WS-5 (DuckDB self-hosting) had to apply the same CDN-URL fix twice because of this duplication. Consolidating onto one generation touches load-bearing code paths (analytics dual-write, local NLP/embedding inference used across RAG, cross-project search, and ProForge's memory bank) and is a properly-scoped migration in its own right, not a byproduct of this sprint. New ADR-0014 documents both generations' call sites and the duplicated-fix cost, and notes CLAUDE.md's own architecture docs already frame WorkerBus v2 as the intended target, without committing to a migration timeline. Zero functional changes. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Every continue-on-error/|| true in ci.yml now has a stated exit criterion in docs/CI.md, so none of them silently become permanent debt: Storybook test-runner (re-evaluate after ~10 real runs post the F-12 fix), e2e-deep (promote specific stable flag combos into the required e2e gate after 3 weeks stable), Lighthouse desktop (remove continue-on-error after 5 consecutive stable runs), and the coverage ratchet (intentionally, permanently advisory by design -- the one gate here without an exit criterion, documented as such so it isn't mistaken for forgotten debt). Part 2/2 (raising vitest.config.ts's coverage thresholds against freshly measured values) is deferred: this machine's ~3.7 GB RAM cannot run the full --coverage suite locally (confirmed by two separate killed attempts with zero output, consistent with this repo's own documented CI-cloud-first policy for constrained local hardware). Will be completed once CI reports real numbers for this branch, per the hard rule to never set a threshold above the actually-measured value. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThe PR hardens CSP enforcement and runtime checks, replaces unsalted filesystem encryption, self-hosts DuckDB-WASM assets, centralizes production URLs, corrects Storybook CI commands, bumps the release version, and updates related documentation, ADRs, audit records, styling, and tests. ChangesRuntime security and repository truth
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant CI
participant SmokeBuild as scripts/smoke-prod-build.mjs
participant Chromium
participant App
CI->>SmokeBuild: run production smoke test
SmokeBuild->>Chromium: launch preview browser
Chromium->>App: load built application
App-->>Chromium: emit CSP violations and run WebAssembly.instantiate
Chromium-->>SmokeBuild: return runtime results
SmokeBuild-->>CI: report pass or fail
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
🏁 CodeAnt Quality Gate ResultsCommit: ✅ Overall Status: PASSEDQuality Gate Details
|
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (5)
.github/workflows/ci.yml (1)
452-461: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMove audit history out of the workflow YAML.
This large dated explanation belongs in
docs/CI.md, which already documents the gate rationale and exit criterion. Keep only a short operational comment in.github/workflows/ci.yml.As per coding guidelines, pure JSON/YAML configuration changes should not add forced inline comments; document rationale in the response or commit description instead.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/ci.yml around lines 452 - 461, Remove the dated multi-line audit explanation from the workflow near the test-storybook command, leaving only a brief operational comment if needed; preserve the command and its existing || true behavior. Move the rationale, unsupported-flag history, and re-evaluation criterion into the existing CI gate documentation in docs/CI.md.Source: Coding guidelines
constants/index.ts (1)
1-1: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAnnotate the new public export.
Proposed annotation
+// QNBS-v3: [Expose canonical production URL / removes consumer hardcoding / preserves one URL contract] export { APP_FILE_SLUG, APP_NAME, PRODUCTION_URL } from './brand';As per coding guidelines, “Bei jeder inhaltlich relevanten Änderung in TypeScript- oder JavaScript-Dateien einen einzeiligen Kommentar im Format
// QNBS-v3: [Grund / Impact / Kreativer Mehrwert]unmittelbar bei der Änderung ergänzen.”🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@constants/index.ts` at line 1, Add the required one-line `// QNBS-v3: [Grund / Impact / Kreativer Mehrwert]` annotation immediately adjacent to the new public export in the barrel file, describing the reason, impact, and creative value of exposing `APP_FILE_SLUG`, `APP_NAME`, and `PRODUCTION_URL`.Source: Coding guidelines
tests/unit/checkDocMetrics.test.ts (1)
7-12: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd compliant QNBS-v3 annotations for the test changes.
The new import has no immediate annotation, and
QNBS-v3 (F-10):does not follow the mandated format.As per coding guidelines, “Bei jeder inhaltlich relevanten Änderung in TypeScript- oder JavaScript-Dateien einen einzeiligen Kommentar im Format
// QNBS-v3: [Grund / Impact / Kreativer Mehrwert]unmittelbar bei der Änderung ergänzen.”Also applies to: 106-108
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unit/checkDocMetrics.test.ts` around lines 7 - 12, Update the changed import and the additional modified sections in tests/unit/checkDocMetrics.test.ts with immediate one-line comments using the exact format `// QNBS-v3: [Grund / Impact / Kreativer Mehrwert]`; replace any noncompliant `QNBS-v3 (F-10):` annotation and ensure each comment accurately describes the associated test change.Source: Coding guidelines
components/settings/GeneralSections.tsx (1)
3-3: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd the required QNBS-v3 change annotations.
Both substantive TSX changes lack immediate
QNBS-v3comments.Proposed annotation
+// QNBS-v3: [Use canonical production URL / prevents stale demo links / keeps release surfaces aligned] import { APP_NAME, ICONS, PRODUCTION_URL } from '../../constants';+ {/* QNBS-v3: [Bind Live Demo to canonical URL / prevents settings-link drift / shares deployment truth] */} <a href={PRODUCTION_URL}As per coding guidelines, “Bei jeder inhaltlich relevanten TSX- oder JSX-Änderung einen einzeiligen
// QNBS-v3: …-Kommentar unmittelbar über der Änderung verwenden.”Also applies to: 451-451
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@components/settings/GeneralSections.tsx` at line 3, Add immediate single-line `// QNBS-v3: …` comments above both substantive TSX changes in `GeneralSections.tsx`, including the import change around `APP_NAME`, `ICONS`, and `PRODUCTION_URL` and the additional change at the referenced second location. Use concise descriptions of each change and keep the annotations directly adjacent.Source: Coding guidelines
scripts/check-doc-metrics.mjs (1)
90-92: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse the prescribed QNBS-v3 annotation format.
QNBS-v3 (F-10):does not match the required// QNBS-v3: [reason / impact / creative value]format, and the added scan loop has no immediate annotation.As per coding guidelines, “Bei jeder inhaltlich relevanten Änderung in TypeScript- oder JavaScript-Dateien einen einzeiligen Kommentar im Format
// QNBS-v3: [Grund / Impact / Kreativer Mehrwert]unmittelbar bei der Änderung ergänzen.”Also applies to: 190-224
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/check-doc-metrics.mjs` around lines 90 - 92, Update the annotation beside PRODUCTION_URL_ASSIGNMENT to use the required single-line format `// QNBS-v3: [reason / impact / creative value]`, preserving its explanation of the canonical production URL. Add an immediate annotation in the scan-loop change covering lines 190–224, using the same format and describing that change’s reason, impact, or creative value.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/ci.yml:
- Around line 452-470: Remove the `|| true` fallback from the `test-storybook`
command so genuine Storybook failures propagate through the workflow. If this
step must remain non-blocking temporarily, add `continue-on-error: true` at the
step level while keeping the command’s exit status intact.
In `@constants/brand.ts`:
- Around line 13-20: Replace the multi-line JSDoc above PRODUCTION_URL with the
required adjacent single-line `// QNBS-v3: ...` annotation. Keep the
PRODUCTION_URL value unchanged and move any additional rationale to
documentation only if it must be preserved.
In `@docs/adr/0014-worker-generation-duplication.md`:
- Around line 46-47: Update the ADR text around the STOP-AND-ASK rule to either
link the governing policy or explicitly record it as an ADR decision, and define
how “estimated scope” is measured so the >3× threshold is auditable.
In `@docs/audit/WS-RUN-LOG-2026-07-29.md`:
- Around line 87-88: Update the documented validation command in the audit log
to run lint and typecheck as two sequential commands using &&, or place them in
separate command blocks, so the “both clean” result is reproducible.
- Around line 3-4: Remove the host-specific absolute plan path from the document
header around the PROMPT-WSS-v1.24.x reference. Replace it with a
repository-relative reference or ticket/branch identifier, or omit the plan
reference while preserving the execution-tracking context.
In `@scripts/check-doc-metrics.mjs`:
- Around line 92-123: Extend the URL drift scan to detect scheme-less Vercel
hostnames such as worldscript-studio-indol.vercel.app, while preserving
canonical URL normalization and existing HTTPS matching. Add
locales/it/help.json to URL_CHECK_FILES so Italian help content is scanned, and
add a regression test covering the scheme-less stale hostname through
scanForUrlDrift.
In `@scripts/copy-duckdb-assets.mjs`:
- Around line 45-49: Update the alreadyCurrent logic in the asset-copy flow so
it compares file contents rather than only statSync(src).size and
statSync(dest).size; use a byte comparison, content hash, or copy
unconditionally, while preserving the existing copyFileSync and copied counter
behavior.
In `@tests/unit/checkDocMetrics.test.ts`:
- Around line 7-12: Update the declarations in check-doc-metrics.d.mts to export
getCanonicalProductionUrl and scanForUrlDrift with signatures matching the
corresponding exports in check-doc-metrics.mjs, alongside the existing
scanForDrift and stripHistoricalSections declarations.
In `@vite.config.ts`:
- Around line 72-76: Standardize the QNBS-v3 markers by adding a single-line
comment in the exact format “// QNBS-v3: [reason / impact / creative value]”
immediately above the affected symbols: the “'**/duckdb/**'” entry in
vite.config.ts, and DUCKDB_ASSET_BASE in workers/duckdbWorker.ts and
workers/v2/duckdb.worker.ts. Replace or supersede the existing multiline QNBS-v3
prose at each listed site.
---
Nitpick comments:
In @.github/workflows/ci.yml:
- Around line 452-461: Remove the dated multi-line audit explanation from the
workflow near the test-storybook command, leaving only a brief operational
comment if needed; preserve the command and its existing || true behavior. Move
the rationale, unsupported-flag history, and re-evaluation criterion into the
existing CI gate documentation in docs/CI.md.
In `@components/settings/GeneralSections.tsx`:
- Line 3: Add immediate single-line `// QNBS-v3: …` comments above both
substantive TSX changes in `GeneralSections.tsx`, including the import change
around `APP_NAME`, `ICONS`, and `PRODUCTION_URL` and the additional change at
the referenced second location. Use concise descriptions of each change and keep
the annotations directly adjacent.
In `@constants/index.ts`:
- Line 1: Add the required one-line `// QNBS-v3: [Grund / Impact / Kreativer
Mehrwert]` annotation immediately adjacent to the new public export in the
barrel file, describing the reason, impact, and creative value of exposing
`APP_FILE_SLUG`, `APP_NAME`, and `PRODUCTION_URL`.
In `@scripts/check-doc-metrics.mjs`:
- Around line 90-92: Update the annotation beside PRODUCTION_URL_ASSIGNMENT to
use the required single-line format `// QNBS-v3: [reason / impact / creative
value]`, preserving its explanation of the canonical production URL. Add an
immediate annotation in the scan-loop change covering lines 190–224, using the
same format and describing that change’s reason, impact, or creative value.
In `@tests/unit/checkDocMetrics.test.ts`:
- Around line 7-12: Update the changed import and the additional modified
sections in tests/unit/checkDocMetrics.test.ts with immediate one-line comments
using the exact format `// QNBS-v3: [Grund / Impact / Kreativer Mehrwert]`;
replace any noncompliant `QNBS-v3 (F-10):` annotation and ensure each comment
accurately describes the associated test change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 506c61b4-52c3-48cf-bdb4-310ddc5e2863
📒 Files selected for processing (36)
.github/workflows/ci.yml.gitignoreCLAUDE.mdREADME.mdcomponents/settings/GeneralSections.tsxconstants/brand.tsconstants/index.tsdocs/CI.mddocs/DEPLOYMENT.mddocs/SECURITY-THREAT-MODEL.mddocs/adr/0004-csp-connect-src-byok-tradeoff.mddocs/adr/0010-languagetool-self-hosted.mddocs/adr/0013-csp-wasm-and-blob-frames.mddocs/adr/0014-worker-generation-duplication.mddocs/audit/WS-RUN-LOG-2026-07-29.mdindex.htmlindex.tsxlocales/it/help.jsonnginx.confpackage.jsonpublic/_headerspublic/locales/it/bundle.jsonscripts/check-doc-metrics.mjsscripts/copy-duckdb-assets.mjsscripts/smoke-prod-build.mjsservices/fs/fsCore.tsservices/fs/settingsFsStore.tssrc-tauri/tauri.conf.jsontests/unit/checkDocMetrics.test.tstests/unit/cspCorrectness.test.tstests/unit/services/fs/fsCore.test.tstests/unit/services/fs/fsStores.test.tsvercel.jsonvite.config.tsworkers/duckdbWorker.tsworkers/v2/duckdb.worker.ts
…, F-10 follow-up) CI's quality job (Node 22 and 24) failed on push: TS2305, module has no exported member 'getCanonicalProductionUrl'. A hand-maintained scripts/check-doc-metrics.d.mts (added in #283) shadows the .mjs source for TypeScript's purposes and wasn't updated with the two new function signatures (getCanonicalProductionUrl, scanForUrlDrift) added in the F-10 workstream. Reproduced locally with rm -f *.tsbuildinfo && pnpm run typecheck -- the earlier "clean" claim in the run log had trusted a background task's "completed, exit code 0" notification summary without reading its actual tail output, the same trap already hit once earlier in this sprint. Fixed by adding the missing declarations; re-verified clean. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…t, WS-1 follow-up) CodeQL flagged a real high-severity finding in this PR: tests/unit/cspCorrectness.test.ts's inlineScriptContents() used /<script(...)>(...)<\/script>/g with no `i` flag. HTML tag names are case-insensitive, so an uppercase <SCRIPT> tag would silently slip past this exact detector -- the one built specifically to catch the class of defect (an unhashed inline script alongside a CSP that forbids it) that shipped 2026-05-27. A case gap here is a real completeness bug in the regression guard itself, not a lint nit. Fixed with the `i` flag on both the tag regex and the src= attribute check. Added two regression tests: an uppercase <SCRIPT> is detected, and an uppercase <SCRIPT SRC=...> is correctly not flagged as inline. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/unit/cspCorrectness.test.ts`:
- Around line 72-80: Add a single-line `// QNBS-v3: ...` rationale comment
immediately adjacent to the `inlineScriptContents` regex implementation,
explaining the case-insensitive matching requirement. Preserve the existing
JSDoc and regex behavior.
- Line 80: Update the script-matching regular expression in the html.matchAll
loop to accept optional whitespace before the closing tag’s >, using the
required </script\s*> pattern. Add a regression case in the CSP correctness
tests covering an inline script closed as </script >.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3f905d89-7b05-4562-aea8-3f6f9d7aa0d2
📒 Files selected for processing (3)
docs/audit/WS-RUN-LOG-2026-07-29.mdscripts/check-doc-metrics.d.mtstests/unit/cspCorrectness.test.ts
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
…QL follow-up) Further CodeQL "Bad HTML filtering regexp" refinement on inlineScriptContents(): browsers still parse a malformed closing tag like </script foo="bar"> or </script > as a valid </script>, so a strict `<\/script>` literal would let such a script slip past this detector undetected -- the same failure class as the case-sensitivity gap fixed in the previous commit. The closing-tag alternative `(?:\s[^>]*)?` now tolerates trailing whitespace/attributes. `\b` after the opening `<script` also stops it from matching inside an unrelated tag name like `<scripts>`. Two new regression tests: a malformed closing tag is still detected, and an unrelated <scripts> tag is not mistaken for a script open tag. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…10 follow-up) Several substantive corrections applied and verified during the PR review loop on #284, all re-tested (lint, typecheck, affected vitest suites) before accepting: - .github/workflows/ci.yml: the Storybook test-runner's `|| true` swallowed the step's real exit code, so it always showed green in the Actions UI even while genuinely failing -- literally the same visibility gap that let the F-12 flag-name bug go unnoticed for as long as it did. Moved to step-level `continue-on-error: true`, which still surfaces the true pass/fail state without blocking the job. - scripts/check-doc-metrics.mjs: the F-10 URL-drift gate excluded locales/it/help.json ("out of scope, generated content") -- but that's the actual source file where the real F-10 drift happened, and it references the host with no `https://` scheme inside a <code> tag, which the scheme-required regex would never have matched anyway. Now scans locales/it/help.json too, with an optional-scheme pattern and matching normalization. Two new regression tests cover the scheme-less case. - scripts/copy-duckdb-assets.mjs: the idempotency check only compared file size, which a same-size dependency bump could pass while leaving stale content in public/duckdb/. Added a SHA-256 content hash check (only computed once sizes already match, avoiding wasted hashing on clearly-different files). - tests/unit/cspCorrectness.test.ts: additional CodeQL rule-ID cross-reference comment. - vite.config.ts, workers/duckdbWorker.ts, workers/v2/duckdb.worker.ts, constants/brand.ts, docs/CI.md, docs/adr/0014, WS-RUN-LOG: comment condensing / precision wording, no functional change. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/check-doc-metrics.mjs`:
- Around line 92-93: Replace the affected multi-line comments near the QNBS-v3
annotations with adjacent single-line comments using exactly the format `//
QNBS-v3: [Grund / Impact / Kreativer Mehrwert]`; update all instances at the
referenced locations, preserving their relevant rationale within the three
required sections.
- Around line 92-94: Update VERCEL_URL_PATTERN to enforce hostname boundaries so
it matches only the intended worldscript-studio Vercel host, while preserving
optional schemes and trailing slash support. Add regression tests covering both
embedded/suffixed hostnames and valid scheme-less URLs.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 214a467b-6216-41c9-9b07-214b5279b4bd
📒 Files selected for processing (12)
.github/workflows/ci.ymlconstants/brand.tsdocs/CI.mddocs/adr/0014-worker-generation-duplication.mddocs/audit/WS-RUN-LOG-2026-07-29.mdscripts/check-doc-metrics.mjsscripts/copy-duckdb-assets.mjstests/unit/checkDocMetrics.test.tstests/unit/cspCorrectness.test.tsvite.config.tsworkers/duckdbWorker.tsworkers/v2/duckdb.worker.ts
🚧 Files skipped from review as they are similar to previous changes (9)
- constants/brand.ts
- tests/unit/checkDocMetrics.test.ts
- docs/adr/0014-worker-generation-duplication.md
- docs/CI.md
- scripts/copy-duckdb-assets.mjs
- workers/duckdbWorker.ts
- tests/unit/cspCorrectness.test.ts
- docs/audit/WS-RUN-LOG-2026-07-29.md
- workers/v2/duckdb.worker.ts
… 2/2) Completes WS-9: this machine can't run the full --coverage suite locally (confirmed by two killed zero-output attempts), so the threshold bump was deferred until CI reported real numbers on this PR's own commits (per this repo's CI-cloud-first policy). quality job (Node 22 and 24, identical): 79.02% statements / 66.74% branches / 73.78% functions / 80.86% lines. vitest.config.ts thresholds: L74/F67/B60/S72 -> L79/F72/B65/S77 -- capped at +5 per metric (the documented quarterly cap) rather than the full ~1pt-below-measured jump, since a straight 1pt margin would have pushed statements +6. First ratchet of Q3 2026 (prior history entries are all Q2), so the cap isn't stacking within a quarter. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
package.json bumped; src-tauri/tauri.conf.json, src-tauri/Cargo.toml, and public/sw.js's APP_VERSION synced via the existing sync-tauri-version.mjs / sync-sw-version.mjs scripts rather than hand-edited. README version badge updated (sync-readme-metrics.mjs doesn't track it). New CHANGELOG.md [1.24.2] entry lists every finding (F-01..F-14) from this sprint's audit. Tag creation and GitHub Release publishing are maintainer actions, not performed here. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Full re-run of every gate from WS-0's baseline plus the heavy stack on the final commit state: i18n, doc-metrics, suppressions, token-audit, lint, typecheck, parity:check, build + bundle:budget, smoke-prod-build -- all green. AUDIT.md gains an "Audit 2026-07-29 -- CSP / Crypto / Truth" section: full finding table (F-01..F-14, severity, resolution) plus a post-mortem answering the sprint's real question -- would a faad8f0-shaped commit fail today? Yes, in 3 independent places. The post-mortem also names two further instances of the identical "gate looks green but checks the wrong property" pattern that surfaced during this same sprint's own execution (Storybook's || true; the F-10 drift gate's own initial exclusion of the file with real incident history) -- three occurrences in one sprint, documented as one finding rather than three unrelated bugs. TODO.md gains a v1.24.2 block; ROADMAP.md synced. All three explicitly state PR #284 is open/CI-green/not-yet-merged rather than claiming release -- caught in my own first draft and corrected before committing, per this sprint's own doc-truth discipline. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
src-tauri/Cargo.toml (1)
3-3: 🩺 Stability & Availability | 🔵 TrivialVerify the required Tauri build after this metadata change.
Because
src-tauri/Cargo.tomlchanged, dispatchtauri-build.ymlon this branch and confirm it reachesFinished N bundles; the supplied web/quality records do not establish this Rust/Tauri-specific gate.As per coding guidelines, every Rust/Tauri change requires this build verification.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src-tauri/Cargo.toml` at line 3, After updating the package version in src-tauri/Cargo.toml, dispatch the tauri-build.yml workflow for this branch and verify its output reaches “Finished N bundles.” Do not treat the web or quality workflow results as a substitute for this Rust/Tauri-specific build gate.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@CHANGELOG.md`:
- Around line 10-12: Update the CHANGELOG.md v1.24.2 heading so it remains
explicitly unreleased, either by keeping its entries under [Unreleased] or
labeling the version as pending. Do not present v1.24.2 as released until the
corresponding tag exists.
In `@docs/audit/WS-RUN-LOG-2026-07-29.md`:
- Around line 535-540: Update the re-audit transcript’s docs:check output to
reflect release version 1.24.2. Rerun check-doc-metrics; if it still reports
1.24.1, update the underlying version source used by the check, then rerun the
gate and record the resulting output.
- Line 447: Add language identifiers to every newly added fenced transcript
block in WS-RUN-LOG-2026-07-29.md, using an appropriate identifier such as bash
or text. For command-only blocks, remove the leading `$` prompts or include
corresponding command output so the blocks satisfy MD014 while preserving the
transcript content.
In `@public/sw.js`:
- Line 8: Update public/sw.js at lines 8-8 by adding an adjacent one-line
QNBS-v3 rationale explaining the APP_VERSION bump’s cache invalidation impact.
Also update vitest.config.ts at lines 75-87 with an adjacent one-line QNBS-v3
rationale explaining the coverage-threshold ratchet and its CI impact; no other
changes are required.
In `@vitest.config.ts`:
- Around line 75-87: Add an adjacent one-line `// QNBS-v3: [Reason / Impact /
Creative Value]` rationale immediately beside the updated `thresholds`
configuration, describing the reason for the new coverage baseline, its impact
on CI enforcement, and the value of maintaining verified coverage quality.
Preserve the existing ratchet comments and threshold values.
---
Nitpick comments:
In `@src-tauri/Cargo.toml`:
- Line 3: After updating the package version in src-tauri/Cargo.toml, dispatch
the tauri-build.yml workflow for this branch and verify its output reaches
“Finished N bundles.” Do not treat the web or quality workflow results as a
substitute for this Rust/Tauri-specific build gate.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 13fba1c1-0476-4aad-94c6-44b00762f39d
📒 Files selected for processing (11)
AUDIT.mdCHANGELOG.mdREADME.mdROADMAP.mdTODO.mddocs/audit/WS-RUN-LOG-2026-07-29.mdpackage.jsonpublic/sw.jssrc-tauri/Cargo.tomlsrc-tauri/tauri.conf.jsonvitest.config.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- README.md
…coverage (Codecov) CodeRabbit found a real regex correctness bug in VERCEL_URL_PATTERN: without hostname boundaries it would extract "worldscript-studio. vercel.app" as a matching substring out of unrelated domains like "worldscript-studio.vercel.app.evil.com" or "notworldscript-studio. vercel.app", silently treating them as equivalent to the canonical URL instead of flagging them as different. Added lookbehind/lookahead hostname boundaries. Exported the pattern (matches the file's existing convention of exporting internals for testability) and added regression tests that assert directly on match behavior, since the higher-level scanForUrlDrift's finding count can't distinguish "correctly rejected" from "matched-and-silently-equated" -- both produce 0 findings. Two multi-line QNBS-v3 comments condensed to the required single-line format (project convention, CLAUDE.md). Codecov flagged 1 missing patch-coverage line: settingsFsStore.ts's nested catch block for when removing a stale legacy key file itself fails. Added a regression test for that cleanup-of-cleanup path. Two stale-declaration-file typecheck errors fixed along the way (scripts/check-doc-metrics.d.mts hadn't been updated for the newly exported VERCEL_URL_PATTERN; a new test helper had an implicit-any parameter) -- same class of issue as the earlier CI-caught .d.mts gap, caught locally this time before pushing. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…wave CHANGELOG.md's [1.24.2] entry presented this sprint's work as if released, contradicting every other doc's "PR #284 open, not yet merged/tagged" status -- exactly the doc-truth violation this sprint is about. Moved the entry under [Unreleased] with an explanatory comment; ROADMAP.md's reference to a "[1.24.2]" CHANGELOG heading updated to match. docs/audit/WS-RUN-LOG-2026-07-29.md: added language identifiers to all 15 fenced transcript blocks (MD040), and stripped the leading "$ " prompt from 34 "$ command -> result" annotation lines that had no separate output line, which would otherwise read as a copy-pasteable script with no shown output (MD014). Added a clarifying note where the transcript shows "latest v1.24.1": that's correct-by-design (check-doc-metrics.mjs reads git tags, not package.json; no v1.24.2 tag exists yet since tagging is a maintainer action) -- verified, not a stale-output bug. public/sw.js and vitest.config.ts: added the required single-line QNBS-v3 rationale comments adjacent to the APP_VERSION bump and the coverage-threshold change respectively (project convention, CLAUDE.md), which the WS-7/WS-9 commits had missed. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Owner
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
docs/audit/WS-RUN-LOG-2026-07-29.md (2)
216-246: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winMake the recorded CSP verification commands reproducible.
The
python3 -c "...json...['connect-src']"command is abbreviated and cannot actually inspect the CSP; the slash-separated command summary later is similarly non-executable. Record the exact commands used, or clearly label these lines as abbreviated transcript notation.Also applies to: 255-258
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/audit/WS-RUN-LOG-2026-07-29.md` around lines 216 - 246, Update the WS-4 audit entry’s CSP verification evidence, including the lines around the DoD evidence and the additionally referenced section, to use complete executable commands that inspect each relevant connect-src value; alternatively, explicitly label the existing Python and slash-separated lines as abbreviated transcript notation. Ensure the recorded commands accurately reproduce the checks described for all five surfaces and Tauri-specific hosts.
59-94: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winNarrow the zod CSP exception to the known probe.
The filter excludes every
evalviolation from any/assets/zod-file, so a future or unintended unsafe-eval in that bundle could make the smoke test pass incorrectly. Match the known probe more precisely (for example, stable source location/content) and fail on any additional zod violation.Also applies to: 99-120
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/audit/WS-RUN-LOG-2026-07-29.md` around lines 59 - 94, Narrow the zod exception in the production smoke-test violation filter to the specific known JIT probe, using stable source location or content rather than excluding all eval violations from zod assets. Ensure any other zod CSP violation remains counted and causes the smoke test to fail, while preserving the existing benign-probe exclusion and success output.
🧹 Nitpick comments (1)
docs/audit/WS-RUN-LOG-2026-07-29.md (1)
299-302: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd deterministic regression coverage for both worker generations.
The build and production smoke test do not directly prove that each worker constructs the correct same-origin asset URLs. Add focused tests for
workers/duckdbWorker.tsandworkers/v2/duckdb.worker.ts, or document a deterministic alternative that exercises both paths. Based on learnings, non-trivial changes should have deterministic tests when existing coverage is absent.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/audit/WS-RUN-LOG-2026-07-29.md` around lines 299 - 302, Add deterministic regression coverage for both workers, duckdbWorker.ts and duckdb.worker.ts, verifying that each constructs the correct same-origin asset URLs; alternatively document a deterministic test approach that exercises both paths. Update the audit entry to record the added coverage or the documented alternative instead of claiming no test is needed.Source: Learnings
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@docs/audit/WS-RUN-LOG-2026-07-29.md`:
- Around line 216-246: Update the WS-4 audit entry’s CSP verification evidence,
including the lines around the DoD evidence and the additionally referenced
section, to use complete executable commands that inspect each relevant
connect-src value; alternatively, explicitly label the existing Python and
slash-separated lines as abbreviated transcript notation. Ensure the recorded
commands accurately reproduce the checks described for all five surfaces and
Tauri-specific hosts.
- Around line 59-94: Narrow the zod exception in the production smoke-test
violation filter to the specific known JIT probe, using stable source location
or content rather than excluding all eval violations from zod assets. Ensure any
other zod CSP violation remains counted and causes the smoke test to fail, while
preserving the existing benign-probe exclusion and success output.
---
Nitpick comments:
In `@docs/audit/WS-RUN-LOG-2026-07-29.md`:
- Around line 299-302: Add deterministic regression coverage for both workers,
duckdbWorker.ts and duckdb.worker.ts, verifying that each constructs the correct
same-origin asset URLs; alternatively document a deterministic test approach
that exercises both paths. Update the audit entry to record the added coverage
or the documented alternative instead of claiming no test is needed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7cc31c40-8ff7-4730-9fdd-59691b35cf88
📒 Files selected for processing (15)
AUDIT.mdCHANGELOG.mdREADME.mdROADMAP.mdTODO.mddocs/audit/WS-RUN-LOG-2026-07-29.mdpackage.jsonpublic/sw.jsscripts/check-doc-metrics.d.mtsscripts/check-doc-metrics.mjssrc-tauri/Cargo.tomlsrc-tauri/tauri.conf.jsontests/unit/checkDocMetrics.test.tstests/unit/services/fs/fsStores.test.tsvitest.config.ts
🚧 Files skipped from review as they are similar to previous changes (14)
- public/sw.js
- scripts/check-doc-metrics.d.mts
- src-tauri/tauri.conf.json
- src-tauri/Cargo.toml
- tests/unit/services/fs/fsStores.test.ts
- tests/unit/checkDocMetrics.test.ts
- TODO.md
- vitest.config.ts
- package.json
- CHANGELOG.md
- README.md
- ROADMAP.md
- scripts/check-doc-metrics.mjs
- AUDIT.md
…ggle contrast tests/e2e/a11y.spec.ts flagged a deterministic (not flaky) WCAG AA color-contrast failure on the Writer version-control toggle button's active state: bg-[var(--sc-accent)]/20 + text-[var(--sc-accent)] measured 4.47:1 on the sepia theme (needs 4.5:1). Unrelated to the CSP/crypto sprint, but small and well-scoped, so fixed directly rather than deferred to a separate PR. Root cause: the active-state classes reused --sc-accent for both the background tint and the text color, instead of the already-vetted --nav-background-active / --nav-text-active token pair that index.css documents as independently tuned per theme specifically for this contrast requirement (e.g. sepia's pair measures 4.97:1, per the existing --sc-accent definition comment). Fixed all 3 occurrences of the identical class combo (Writer VC button desktop + mobile, and the mobile ProForge toggle, which shares the same pattern and would have the same problem even though this specific E2E spec didn't exercise it). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Follow-up to 8fc28b4 -- the finding was documented as an open follow-up in WS-11's close-out; now fixed directly per standing instruction to proactively resolve small out-of-scope issues rather than defer them. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@components/writing/WriterViewUI.tsx`:
- Line 192: Update the desktop ProForge button branch in WriterViewUI, alongside
the existing mobile conditional styling, to use the same vetted navigation color
tokens and WCAG-compliant focus styling instead of --sc-accent and
--sc-ring-focus. Keep the desktop and mobile active-state appearance consistent
across breakpoints.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: be127874-0e2a-4981-941f-dc9886e42372
📒 Files selected for processing (2)
TODO.mdcomponents/writing/WriterViewUI.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- TODO.md
…de toggles The mobile buttons were fixed for the sepia sub-4.5:1 contrast failure, but the desktop ProForge toggle (line 117) and Focus Mode toggle (line 148) used a different broken combo (bg-accent/20 + text-ring-focus, a semi-transparent accent) with the same underlying defect. Both now use the vetted --nav-background-active/--nav-text-active pair. Also adds an explicit regression test for a bare-whitespace `</script >` closing tag in cspCorrectness.test.ts — the existing `(?:\s[^>]*)?` tolerance already covered this case, but there was no test asserting it directly (only the `foo="bar"` attribute variant was tested). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
qnbs
enabled auto-merge (squash)
July 29, 2026 11:18
qnbs
added a commit
that referenced
this pull request
Jul 29, 2026
CHANGELOG.md's [Unreleased] heading -> dated [1.24.2] now that PR #284 is actually merged; AUDIT.md/TODO.md/ROADMAP.md status lines updated from "PR open, not yet merged/tagged" to "merged and released" -- the same doc-truth discipline this sprint enforced elsewhere, applied to its own closing status. WS-RUN-LOG gets a post-close addendum recording the merge (admin-squash, GitHub mergeable-state cache lag) and the one genuine CodeRabbit finding caught after WS-11's close-out commit. Co-Authored-By: Claude Sonnet 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
2 participants
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
Sprint fixing the CSP/crypto/doc-truth audit (
PROMPT-WSS-v1.24.x), base SHAf5f9c1ba(empirically re-verified — all 14 findings confirmed against current code before any change landed, see plan anddocs/audit/WS-RUN-LOG-2026-07-29.md).The headline defect (F-01/F-02):
script-srcshipped without'wasm-unsafe-eval'for two months (2026-05-27 → today), silently blockingWebAssembly.instantiatein every deployed Chromium browser — the entire advertised local-inference stack (WebLLM, ONNX Runtime Web, Transformers.js, DuckDB-WASM, Whisper-STT, Kokoro-TTS) never functioned in production. No gate caught it because the only CSP tests that existed check cross-surface consistency, never functional correctness — and the prod-build smoke test only listened forpageerror, which CSP violations never fire. Fixed with'wasm-unsafe-eval'(never the broader'unsafe-eval') +frame-src blob:on all 5 deployment surfaces, plus a new 3-layer test architecture (consistency / correctness / runtime) so this class of defect can't silently recur — full record indocs/adr/0013-csp-wasm-and-blob-frames.md.Also fixed (each with its own commit, finding ID in the subject):
storageEncryptionService.tspattern. Doc truth-up: README/CLAUDE.md no longer make a blanket "encrypted at rest" claim, and a fabricatedtauri-plugin-strongholdreference (zero trace of that dependency anywhere in the repo) is removed.connect-srcmissing LanguageTool's port and the Hugging Face hosts WebLLM/Transformers.js actually resolve models from (empirically traced viacurl— the real weight-file CDN,us.aws.cdn.hf.co, is a different host than the initialhuggingface.corequest, which would have been missed by adding only the obvious host). Scope widened during verification: the LanguageTool port turned out to be missing on all 5 surfaces, not just Tauri as originally assumed — direct inspection of the web CSP string proved it.scripts/copy-duckdb-assets.mjs; assets gitignored, not committed (~72 MB).|| true: the test-runner invocation used flags that don't exist in the installed CLI version, so it's failed on argument parsing before running a single story on every prior CI run.docs/CI.md. The coverage-threshold bump itself is pending this PR's CI-reported numbers (this machine cannot run the full--coveragesuite locally — confirmed by two killed attempts).Test plan
tests/unit/cspCorrectness.test.ts(new, 75 tests) + existingcsp.test.ts/deploymentHeaders.test.tsgreenscripts/smoke-prod-build.mjshardened with real CSP-violation + WASM-instantiate probes; passes post-fix (0 real violations, 1 documented known-benign zod JIT-probe exclusion,wasm: ok)tests/unit/services/fs/fsCore.test.ts+fsStores.test.tsextended (salt/PBKDF2 regression, legacy-payload discard path) — 40/40 passingtests/unit/checkDocMetrics.test.tsextended for the new URL-drift gate — 15/15 passingpnpm run lint/pnpm run typecheckclean throughoutpnpm run bundle:budgetpasses post-DuckDB-self-hostingvitest.config.tsthresholds per the documented ratchet rule🤖 Generated with Claude Code
Summary by CodeRabbit