fix(frontend): remediate frontend + extension audit findings#2575
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements comprehensive fixes for several critical and high-severity issues identified in a frontend and extension audit. Key improvements include redacting sensitive credentials from request history, introducing a Content Security Policy (CSP) and sanitizing external URLs to prevent DOM-XSS, persisting MV3 background session state to survive worker suspension, fixing token refresh in the browser, resolving chat abort and stream lifecycle bugs, and addressing state clobbering and race conditions in the connection and workspace stores. Additionally, several design specs, implementation plans, and robust test suites are added. The code review feedback highlights further opportunities to harden the codebase, such as wrapping initial awaits in checkOnce to prevent deadlocks on error, utilizing the merge option in Zustand's persist middleware to avoid redundant localStorage writes, adding baseline versioning to useFolderStore, wrapping storage deserialization in a try...catch block to handle malformed values, and clearing re-armed request timeouts to prevent dangling timers.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| if (prev.isChecking || checkInFlight) { | ||
| return | ||
| } | ||
| checkInFlight = true |
There was a problem hiding this comment.
If any of the asynchronous helper functions (such as getFirstRunCompleteFlag, getUserPersonaFlag, etc.) throw an error before the try block is entered, checkInFlight will remain true indefinitely. This will permanently deadlock all future health checks in the session. Consider wrapping these initial awaits in a try...catch block to ensure checkInFlight is safely reset to false on any failure.
| // Publish the post-processed hydrated state THROUGH the store so | ||
| // subscribers (already-mounted components, loading gates keyed on | ||
| // `storeHydrated`) are notified. Persist applies the raw persisted | ||
| // values via its own `set()` BEFORE this callback, but the mutations | ||
| // above (date revival, snapshot application, `storeHydrated`) happen | ||
| // afterwards and would otherwise never be broadcast. Spreading into a | ||
| // fresh object gives `set` a new reference so the update is not | ||
| // dropped as a no-op; `replace: true` matches the shape we mutated. | ||
| publishWorkspaceHydration?.({ ...state }) |
There was a problem hiding this comment.
Mutating the state inside onRehydrateStorage and then manually invoking publishWorkspaceHydration (which calls set) causes a redundant, immediate write back to localStorage right after hydration. Instead of using onRehydrateStorage for state post-processing, it is more idiomatic and efficient to perform these mutations (such as date revival, snapshot application, and setting storeHydrated: true) inside the merge option of the persist middleware. This allows Zustand to apply the fully processed state atomically, notifying subscribers correctly without triggering an extra write cycle.
| merge: (persistedState, currentState) => { | ||
| const persisted = (persistedState ?? {}) as Partial<FolderState> | ||
| const { folderApiAvailable: _legacyFolderApiAvailable, ...rest } = | ||
| persisted | ||
| return { | ||
| ...currentState, | ||
| ...rest | ||
| } | ||
| } |
There was a problem hiding this comment.
While other persisted stores in this PR have been updated to include a baseline version and migrate function to prevent future schema migration issues, useFolderStore is missing these fields. Adding version: 1 and a basic migrate handler here will ensure consistency and protect against silent state discards if the folder store's schema changes in the future.
version: 1,
migrate: (persisted) => persisted as any,
merge: (persistedState, currentState) => {
const persisted = (persistedState ?? {}) as Partial<FolderState>
const { folderApiAvailable: _legacyFolderApiAvailable, ...rest } =
persisted
return {
...currentState,
...rest
}
}
| notifyGlobal(event.key, { | ||
| oldValue: | ||
| event.oldValue == null | ||
| ? undefined | ||
| : defaultSerde.deserializer(event.oldValue), | ||
| newValue: | ||
| event.newValue == null | ||
| ? undefined | ||
| : defaultSerde.deserializer(event.newValue) | ||
| }) |
There was a problem hiding this comment.
If an external script or another tab writes a non-JSON value to localStorage under a key that matches one of our global watchers, defaultSerde.deserializer (which calls JSON.parse) will throw a SyntaxError. Since this listener is registered globally on window, this unhandled exception could disrupt the event loop. Wrapping the deserialization in a try...catch block will make the listener more resilient to malformed or corrupted storage values.
try {
notifyGlobal(event.key, {
oldValue:
event.oldValue == null
? undefined
: defaultSerde.deserializer(event.oldValue),
newValue:
event.newValue == null
? undefined
: defaultSerde.deserializer(event.newValue)
})
} catch (error) {
console.error("Failed to deserialize storage event value:", error)
}| if (timeoutId) clearTimeout(timeoutId) | ||
| timeoutId = setTimeout(() => controller.abort(), timeoutMs) |
There was a problem hiding this comment.
The re-armed timeoutId (and retryTimeoutId on line 458) are never cleared after the response body is successfully read. This leaves dangling timers in the event loop for up to 120 seconds (the default timeout for completions/RAG). While aborting a completed request's controller has no functional impact, these dangling timers can prevent Node.js processes (such as Next.js SSR or Vitest test runners) from exiting cleanly and introduce unnecessary timer overhead. Consider clearing these timeouts once the response body has been fully read.
Code Review by Qodo
Context used✅ Compliance rules (platform):
74 rules 1.
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
rmusser01
force-pushed
the
feat/frontend-extension-audit-remediation
branch
from
6157b0b to
323de42
Compare
PR Summary by Qodofix(frontend): remediate frontend + extension audit findings
AI Description
Diagram
High-Level Assessment
Files changed (109)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
apps/packages/ui/src/store/connection.tsx (1)
608-615: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
checkInFlightcan be left permanentlytrueif a pre-tryawait rejects, wedging all futurecheckOncecalls.The guard is released at every explicit exit path and after the
try/catchat Line 1037, but the awaits before thetryblock are unprotected: the persisted-flag reads here (Lines 611–615) andensurePlaceholderConfig()in the bypass branch (Line 671) run outside anytry. If any of them rejects (e.g. a transientchrome.storage/localStoragefailure),checkInFlightstaystrueforever and the guard at Line 605 short-circuits every subsequentcheckOnce, permanently disabling connection health checks for the session.Wrap the body in
try/finallyso the flag is always released, and drop the now-redundant scattered resets.🔒️ Proposed fix (try/finally guard release)
checkInFlight = true - - // Load all persisted flags upfront - const persistedFirstRun = await getFirstRunCompleteFlag() + try { + // Load all persisted flags upfront + const persistedFirstRun = await getFirstRunCompleteFlag() // ... existing body, with each early `checkInFlight = false; return` // reduced to a plain `return` ... - // Release the in-flight guard for both the normal-completion and caught-error - // paths (they converge here after the try/catch above). - checkInFlight = false + } finally { + // Always release, including if a pre-existing await above rejects. + checkInFlight = false + }🤖 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 `@apps/packages/ui/src/store/connection.tsx` around lines 608 - 615, The `checkOnce` flow in `connection.tsx` can leave `checkInFlight` stuck true if any await before the existing `try/catch` fails, which blocks all later checks. Wrap the full `checkOnce` body, including the persisted-flag reads and `ensurePlaceholderConfig()`, in a `try/finally` and release `checkInFlight` in the `finally` block so every exit path is covered. After that, remove the scattered manual resets that become redundant while keeping the existing guard and exit behavior in `checkOnce` intact.apps/packages/ui/src/hooks/chat/useChatActions.ts (1)
2971-2978: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winCompare-mode cleanup clobbers a newer turn's abort controller.
Every other cleanup site in this file (characterChatMode's finally at Line 2105, the catch block at Line 2990, sendPerModelReply's finally at Line 3157) gates
setIsProcessing/setStreaming/setAbortControllerresets behindreleaseAbortControllerIfOwned(signal)to avoid clobbering a newer overlapping turn. This Compare-mode block resets all of that state — plusactiveAbortControllerRef.current— unconditionally afterPromise.all(comparePromises)resolves. If a newer turn (double-send, regenerate-while-streaming) has taken ownership ofactiveAbortControllerRefwhile Compare's promises were still in flight, this unconditionally nulls out the newer turn's controller and flipsisProcessing/streamingoff, re-enabling Send and orphaning the newer turn's Stop button — exactly the bug this ownership pattern was introduced to prevent.🔒 Proposed fix to gate the reset on ownership
markSteeringApplied() const compareResults = await Promise.all(comparePromises) refreshHistoryFromMessages() - setIsProcessing(false) - setStreaming(false) - setAbortController(null) - activeAbortControllerRef.current = null + // Only reset shared streaming state if this turn still owns the + // controller (mirrors the pattern used elsewhere in this file). + if (releaseAbortControllerIfOwned(signal)) { + setIsProcessing(false) + setStreaming(false) + setAbortController(null) + } return aggregateChatSubmitResults(compareResults)🤖 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 `@apps/packages/ui/src/hooks/chat/useChatActions.ts` around lines 2971 - 2978, Compare-mode cleanup is resetting shared chat state unconditionally and can clobber a newer in-flight turn’s abort controller. Update the cleanup path after Promise.all(comparePromises) in useChatActions to follow the same ownership check used elsewhere in this file: gate the setIsProcessing, setStreaming, setAbortController, and activeAbortControllerRef.current resets behind releaseAbortControllerIfOwned(signal) so only the owning turn clears state. Keep the aggregateChatSubmitResults(compareResults) return flow unchanged, but avoid touching controller/state if ownership has already moved to a newer request.apps/packages/ui/src/services/tldw/request-core.ts (1)
446-458: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winCaller abort is not honored during the 401 refresh/retry.
onAbort(registered at Line 393) only aborts the originalcontroller. The retry issues its fetch with a freshretryController(Line 448/454), so if the caller'sabortSignalfires duringrefreshAuth()or the retry request, the retry proceeds and ignores the cancellation. Consider forwarding the caller signal toretryController(e.g. abort it ifabortSignal?.aborted, and haveonAbortabort both), so Stop/cancel remains effective across the retry.The body-reuse and timeout re-arm changes in this block otherwise look correct.
🛡️ Sketch
const retryController = new AbortController() + if (abortSignal) { + if (abortSignal.aborted) retryController.abort() + else abortSignal.addEventListener("abort", () => retryController.abort(), { once: true }) + } retryTimeoutId = setTimeout(() => retryController.abort(), timeoutMs)🤖 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 `@apps/packages/ui/src/services/tldw/request-core.ts` around lines 446 - 458, The 401 refresh/retry path in request-core.ts is not honoring the caller’s abort signal because retryController is isolated from the original abortSignal and onAbort only cancels the first controller. Update the retry logic in the fetch retry block so the caller’s cancellation is forwarded to retryController as well, and make the abort handler abort both controllers or otherwise propagate abort state before and during the retry request. Use the existing onAbort, controller, retryController, and abortSignal wiring to keep stop/cancel effective across refreshAuth() and the retry fetch.apps/tldw-frontend/extension/shims/wxt-browser.ts (1)
244-271: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
remove()/clear()still silently swallow storage errors, unlike the newly-fixedset().
set()was just fixed to reject on write failure instead of resolving as success (H9#3), butremove()andclear()still catch-and-ignore backend errors, resolving successfully regardless. This is an inconsistent error-handling contract across the same storage API surface.Also applies to: 273-292
🤖 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 `@apps/tldw-frontend/extension/shims/wxt-browser.ts` around lines 244 - 271, `remove()` and `clear()` in the storage area API still swallow backend failures and always resolve, which is inconsistent with the updated `set()` error behavior. Update the `remove`/`clear` paths in the same storage shim (around `getAreaBackend`, `storageOnChanged.trigger`, and the area methods) to catch backend exceptions and reject the returned promise or surface the error through the callback instead of silently ignoring it. Ensure both methods follow the same failure contract as `set()` for all backend operations.
🤖 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 `@apps/FRONTEND_AUDIT.md`:
- Around line 48-73: The Remediation status section mixes a claim that all
Critical and High findings are fixed with entries that are still partial or
residual, which is contradictory. Reword the summary and the table in
FRONTEND_AUDIT.md so the completed fixes are clearly separated from phased
follow-ups, and make sure the status language for 12102, H1, and 12103 matches
their actual remaining work. Use the existing section heading and the table rows
as the place to update the wording consistently.
In `@apps/packages/ui/src/entries/background.ts`:
- Around line 498-511: `persistSessionState` can be invoked repeatedly and its
async storage writes are currently unqueued, which can lead to out-of-order
persistence. Update the `persistSessionState` flow in `background.ts` to
serialize writes through a single promise queue so each call waits for the
previous `writePersistedSessionState` to finish before starting the next. Use
the existing `persistSessionState` helper and the `writePersistedSessionState`
call site as the main targets, and apply the same queued behavior to all
fire-and-forget callers listed in the diff.
- Around line 2286-2346: The regular ingest rehydration path in
rehydrateSessionState/resumeActiveIngestSessions leaves queued sessions with
empty jobIds stuck after restart because they are filtered out and never
reported. Add a helper similar to reportInterruptedQuickIngestUploads that scans
restored ingestSessions for non-terminal sessions that were interrupted before
job submission, and after resumeActiveIngestSessions emits a terminal failure
state with canRetry enabled so the sidepanel can recover. Use the existing
ingestSessions, resumeActiveIngestSessions, and rehydrateSessionState flow to
locate and handle these orphaned sessions.
In `@apps/packages/ui/src/hooks/chat-modes/chatModePipeline.ts`:
- Around line 962-972: The first-chunk path in chatModePipeline is setting
isProcessing back to true without confirming the current turn still owns the
shared controller, which can let a superseded turn re-toggle the UI after a
newer turn has taken over. Update the count === 0 branch to perform the same
ownership check used by releaseAbortControllerIfOwned before calling
setIsProcessing(true), and only flip the processing flag when the current signal
still owns the turn. Keep the cleanup logic around stillOwnsTurn and the shared
streaming/controller state consistent with the existing ownership handling in
chatModePipeline.
In `@apps/packages/ui/src/services/tldw/TldwApiClient.ts`:
- Around line 7944-7955: The mixin copies of createChatCompletion and
synthesizeSpeech in chat-rag.ts and models-audio.ts are now dead code and can
drift from the canonical base behavior. Remove those duplicate implementations
from the mixins and keep TldwApiClientBase as the single source of truth, then
update TldwDomainMethodOverride so those methods are excluded from the domain
interface only where the base-class versions should be used.
In `@apps/tldw-frontend/extension/shims/plasmo-storage-hook.tsx`:
- Around line 45-76: The initial load in the `useEffect` for
`plasmo-storage-hook` can overwrite a newer value from `storage.watch`, so add
an ordering guard between the `storage.get(options.key)` path and the watch
callback. Use a shared freshness flag or version token inside the effect: mark
when `watch` has applied an update, and have the `get().then(...)` handler skip
`applyValue` if that fresher watch update already landed. Keep the cleanup in
the same `useEffect` and ensure `applyValue`, `storage.get`, and `storage.watch`
remain the only entry points to this state update.
In `@apps/tldw-frontend/extension/shims/wxt-browser.ts`:
- Around line 206-243: The `set()` implementation in `wxt-browser.ts` can
partially persist earlier keys before a later `JSON.stringify`/`setItem`
failure, so `storageOnChanged.trigger()` may fire for a write that ultimately
rejects. Update the `set` method to validate/serialize every entry in `items` up
front, and only if all succeed should you write to `backend`, build `changes`,
and trigger `storageOnChanged`/resolve the callback; if any value fails, reject
without committing anything. Keep the fix localized around `set`, `backend`,
`changes`, and `writeError` handling.
In `@apps/tldw-frontend/lib/history.ts`:
- Around line 34-35: Broaden the credential redaction in history persistence:
the current SENSITIVE_BODY_KEYS check in history.ts only drops access_token and
refresh_token, so redactTokens still saves other secret-like fields from
non-auth routes. Update the redaction logic used by redactTokens and the history
entry persistence path to catch additional credential-shaped keys such as
id_token, session_token, apiKey/api_key, jwt, sessionToken, and similar
case-insensitive variants, ideally with a defensive substring/regex match for
token/secret/password. Keep the full-body drop behavior for the auth routes, but
make the non-auth body redaction more comprehensive before writing to
localStorage.
- Around line 67-82: The depth limit in redactTokens currently fails open by
returning the original subtree once recursion exceeds the limit, which can leak
sensitive data. Update redactTokens in history.ts to fail closed at the boundary
by returning a safe redacted placeholder instead of the raw value, and keep the
existing recursive handling for arrays and plain objects while preserving the
SENSITIVE_BODY_KEYS-based masking behavior.
In `@apps/tldw-frontend/next.config.mjs`:
- Around line 50-51: The hardcoded themeBootstrapScriptHash in next.config.mjs
can drift from THEME_BOOTSTRAP_SCRIPT in pages/_document.tsx and break CSP.
Update the hash generation so next.config.mjs computes it at build time from the
actual script source instead of keeping a manual constant, and wire it into the
existing CSP setup where themeBootstrapScriptHash is used.
- Around line 76-110: The security headers returned by headers() in
next.config.mjs are missing CSP violation reporting and HSTS. Update the
Content-Security-Policy entry to include a reporting target such as report-to or
report-uri so violations are observable in production, and add a
Strict-Transport-Security header in the same headers list if it is not
guaranteed by the hosting layer. Keep the changes localized to the headers()
configuration and preserve the existing headers like X-Frame-Options and
Permissions-Policy.
- Around line 62-65: The CSP in next.config.mjs is too broad because
connect-src, img-src, media-src, and frame-src still allow arbitrary
http(s)/ws(s) origins even though NEXT_PUBLIC_API_URL and
TLDW_INTERNAL_API_ORIGIN are available. Tighten the policy in the CSP builder by
pinning backend egress in the connect-src directive using the API origin
symbols, and split any external thumbnails/previews into a separate explicit
allowlist for img-src/media-src/frame-src instead of leaving generic network
schemes open.
In `@backlog/tasks/task-12093` -
Fix-javascript-URL-DOM-XSS-in-source-anchors-and-add-CSP.md:
- Around line 57-66: Update the CSP task notes to match the final shipped
policy: the current text in the task section describing `next.config.mjs` still
reflects the older header set with `'unsafe-inline'` and the follow-up wording.
Revise the description around the CSP block and the “Follow-up” note so it
matches the actual `headers()` policy, which already uses the hardened
`script-src` setup and includes the additional security headers. Keep the
references aligned with `next.config.mjs`, `async headers()`, and the CSP
summary text in this task file.
In `@backlog/tasks/task-12102` - Re-enable-frontend-type-safety-and-lint-gates.md:
- Around line 22-23: Narrow the lint-gate task description so it only mentions
the globally disabled react-hooks rules and not rules-of-hooks, since that rule
is already active outside e2e. Update the wording in the backlog note to match
the actual disables in eslint.config.mjs, and use the existing symbols
react-hooks and `@typescript-eslint/no-explicit-any` as the anchors for the fix.
In `@backlog/tasks/task-12103` -
Remove-half-wired-dead-web-auth-stack-and-dead-extension-routes-tree.md:
- Around line 19-23: Update the note to reference the correct config entrypoint,
using next.config.mjs instead of next.config.js, and mention that the
dead-looking extension/routes tree is still protected by parity tests so it
should not be framed as a simple deletion. Point the guidance at the relevant
symbols such as extension/routes/*, route-registry.tsx, app-route.tsx, and the
shared UI routes under packages/ui/src/routes/*, and add a build/test guard or
explicit quarantine note so future edits can’t silently diverge.
---
Outside diff comments:
In `@apps/packages/ui/src/hooks/chat/useChatActions.ts`:
- Around line 2971-2978: Compare-mode cleanup is resetting shared chat state
unconditionally and can clobber a newer in-flight turn’s abort controller.
Update the cleanup path after Promise.all(comparePromises) in useChatActions to
follow the same ownership check used elsewhere in this file: gate the
setIsProcessing, setStreaming, setAbortController, and
activeAbortControllerRef.current resets behind
releaseAbortControllerIfOwned(signal) so only the owning turn clears state. Keep
the aggregateChatSubmitResults(compareResults) return flow unchanged, but avoid
touching controller/state if ownership has already moved to a newer request.
In `@apps/packages/ui/src/services/tldw/request-core.ts`:
- Around line 446-458: The 401 refresh/retry path in request-core.ts is not
honoring the caller’s abort signal because retryController is isolated from the
original abortSignal and onAbort only cancels the first controller. Update the
retry logic in the fetch retry block so the caller’s cancellation is forwarded
to retryController as well, and make the abort handler abort both controllers or
otherwise propagate abort state before and during the retry request. Use the
existing onAbort, controller, retryController, and abortSignal wiring to keep
stop/cancel effective across refreshAuth() and the retry fetch.
In `@apps/packages/ui/src/store/connection.tsx`:
- Around line 608-615: The `checkOnce` flow in `connection.tsx` can leave
`checkInFlight` stuck true if any await before the existing `try/catch` fails,
which blocks all later checks. Wrap the full `checkOnce` body, including the
persisted-flag reads and `ensurePlaceholderConfig()`, in a `try/finally` and
release `checkInFlight` in the `finally` block so every exit path is covered.
After that, remove the scattered manual resets that become redundant while
keeping the existing guard and exit behavior in `checkOnce` intact.
In `@apps/tldw-frontend/extension/shims/wxt-browser.ts`:
- Around line 244-271: `remove()` and `clear()` in the storage area API still
swallow backend failures and always resolve, which is inconsistent with the
updated `set()` error behavior. Update the `remove`/`clear` paths in the same
storage shim (around `getAreaBackend`, `storageOnChanged.trigger`, and the area
methods) to catch backend exceptions and reject the returned promise or surface
the error through the callback instead of silently ignoring it. Ensure both
methods follow the same failure contract as `set()` for all backend operations.
🪄 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: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: bf71c9f1-2498-4512-a5c8-1b4fdcb32d1c
📒 Files selected for processing (101)
apps/FRONTEND_AUDIT.mdapps/packages/ui/src/components/Common/Playground/MessageSource.tsxapps/packages/ui/src/components/Knowledge/hooks/useFileSearch.tsapps/packages/ui/src/components/Knowledge/hooks/useKnowledgeSearch.tsapps/packages/ui/src/components/Notes/__tests__/notes-manager-utils.sanitize-url.test.tsapps/packages/ui/src/components/Notes/notes-manager-utils.tsapps/packages/ui/src/components/Option/Collections/ReadingList/ReadingItemCard.tsxapps/packages/ui/src/components/Option/Collections/ReadingList/ReadingItemDetail.tsxapps/packages/ui/src/components/Option/Items/ItemsWorkspace.tsxapps/packages/ui/src/components/Option/KnowledgeQA/SourceCard.tsxapps/packages/ui/src/components/Option/KnowledgeQA/SourceViewerModal.tsxapps/packages/ui/src/components/Option/Processed/index.tsxapps/packages/ui/src/components/Option/ResearchWorkspace/SourcesPane/index.tsxapps/packages/ui/src/components/Option/Watchlists/AlertsTab/AlertsTab.tsxapps/packages/ui/src/components/Option/Watchlists/ItemsTab/ItemsTab.tsxapps/packages/ui/src/components/Option/Watchlists/OutputsTab/OutputPreviewDrawer.tsxapps/packages/ui/src/components/Option/Watchlists/OutputsTab/ReportEvidencePanel.tsxapps/packages/ui/src/components/Option/Watchlists/RunsTab/RunDetailDrawer.tsxapps/packages/ui/src/components/Option/Watchlists/SourcesTab/SourcesTab.tsxapps/packages/ui/src/components/Sidepanel/Chat/hooks/useRagResultsDisplay.tsxapps/packages/ui/src/entries/__tests__/background-session-store.test.tsapps/packages/ui/src/entries/background-session-store.tsapps/packages/ui/src/entries/background.tsapps/packages/ui/src/hooks/chat-modes/__tests__/chatModePipeline.abort-lifecycle.test.tsapps/packages/ui/src/hooks/chat-modes/chatModePipeline.tsapps/packages/ui/src/hooks/chat/useChatActions.tsapps/packages/ui/src/models/ChatTldw.tsapps/packages/ui/src/models/__tests__/ChatTldw.abort-signal.test.tsapps/packages/ui/src/models/__tests__/ChatTldw.stream-transport-interrupted.test.tsapps/packages/ui/src/public/_locales/en/sources.jsonapps/packages/ui/src/services/__tests__/background-proxy.test.tsapps/packages/ui/src/services/__tests__/background-proxy.web-refresh.test.tsapps/packages/ui/src/services/background-proxy.tsapps/packages/ui/src/services/tldw/TldwApiClient.tsapps/packages/ui/src/services/tldw/TldwAuth.tsapps/packages/ui/src/services/tldw/TldwChat.tsapps/packages/ui/src/services/tldw/__tests__/TldwApiClient.sanitizer.test.tsapps/packages/ui/src/services/tldw/__tests__/TldwAuth.refresh.test.tsapps/packages/ui/src/services/tldw/__tests__/TldwChat.abort.test.tsapps/packages/ui/src/services/tldw/__tests__/request-core.refresh-timeout.test.tsapps/packages/ui/src/services/tldw/domains/chat-rag.tsapps/packages/ui/src/services/tldw/domains/models-audio.tsapps/packages/ui/src/services/tldw/request-core.tsapps/packages/ui/src/store/__tests__/connection.test.tsapps/packages/ui/src/store/__tests__/folder.test.tsapps/packages/ui/src/store/__tests__/workspace.test.tsapps/packages/ui/src/store/acp-sessions.tsapps/packages/ui/src/store/actor.tsxapps/packages/ui/src/store/connection.tsxapps/packages/ui/src/store/feedback.tsxapps/packages/ui/src/store/folder.tsxapps/packages/ui/src/store/notes-dock.tsxapps/packages/ui/src/store/persona-buddy-shell.tsapps/packages/ui/src/store/playground-session.tsxapps/packages/ui/src/store/quick-ingest-session.tsapps/packages/ui/src/store/ui-mode.tsxapps/packages/ui/src/store/workspace.tsapps/packages/ui/src/utils/__tests__/absolute-url-guard.test.tsapps/packages/ui/src/utils/__tests__/safe-external-url.test.tsapps/packages/ui/src/utils/absolute-url-guard.tsapps/packages/ui/src/utils/extract-token-from-chunk.tsapps/packages/ui/src/utils/safe-external-url.tsapps/tldw-frontend/AGENTS.mdapps/tldw-frontend/CLAUDE.mdapps/tldw-frontend/__tests__/extension/plasmo-storage-watch.test.tsxapps/tldw-frontend/__tests__/extension/wxt-browser-storage.test.tsapps/tldw-frontend/__tests__/header-runs-gating.test.tsxapps/tldw-frontend/__tests__/navigation/react-router-search-params-dynamic.test.tsxapps/tldw-frontend/__tests__/pages/admin-maintenance.test.tsxapps/tldw-frontend/__tests__/pages/content-review.test.tsxapps/tldw-frontend/components/layout/Header.tsxapps/tldw-frontend/components/layout/Layout.tsxapps/tldw-frontend/extension/routes/_RUNTIME_UNUSED.mdapps/tldw-frontend/extension/shims/plasmo-storage-hook.tsxapps/tldw-frontend/extension/shims/plasmo-storage.tsapps/tldw-frontend/extension/shims/react-router-dom.tsxapps/tldw-frontend/extension/shims/wxt-browser.tsapps/tldw-frontend/hooks/__tests__/useConfig.networking.test.tsxapps/tldw-frontend/hooks/useAuth.tsxapps/tldw-frontend/hooks/useConfig.tsxapps/tldw-frontend/hooks/useIsAdmin.tsapps/tldw-frontend/lib/__tests__/history-redaction.test.tsapps/tldw-frontend/lib/api.tsapps/tldw-frontend/lib/auth.tsapps/tldw-frontend/lib/history.tsapps/tldw-frontend/next.config.mjsapps/tldw-frontend/package.jsonapps/tldw-frontend/pages/_document.tsxbacklog/tasks/task-12091 - Fix-chat-completion-sanitizer-corrupting-successful-non-streaming-replies.mdbacklog/tasks/task-12092 - Stop-persisting-credentials-to-localStorage-request-history.mdbacklog/tasks/task-12093 - Fix-javascript-URL-DOM-XSS-in-source-anchors-and-add-CSP.mdbacklog/tasks/task-12094 - Persist-MV3-background-session-state-across-worker-suspension.mdbacklog/tasks/task-12095 - Add-URL-allowlist-before-attaching-credentials-in-extension-upload-stream-handlers.mdbacklog/tasks/task-12096 - Wire-token-refresh-into-the-web-request-path.mdbacklog/tasks/task-12097 - Fix-chat-abort-and-stream-lifecycle.mdbacklog/tasks/task-12098 - Stop-replaying-non-idempotent-chat-POST-on-stream-connection-timeout.mdbacklog/tasks/task-12099 - Fix-connection-and-workspace-store-races-and-sticky-failure-states.mdbacklog/tasks/task-12100 - Fix-web-build-browser-storage-shims.mdbacklog/tasks/task-12101 - Fix-default-request-timeouts-aborting-normal-LLM-generations.mdbacklog/tasks/task-12102 - Re-enable-frontend-type-safety-and-lint-gates.mdbacklog/tasks/task-12103 - Remove-half-wired-dead-web-auth-stack-and-dead-extension-routes-tree.md
💤 Files with no reviewable changes (9)
- apps/tldw-frontend/tests/header-runs-gating.test.tsx
- apps/tldw-frontend/components/layout/Layout.tsx
- apps/tldw-frontend/hooks/useAuth.tsx
- apps/tldw-frontend/tests/pages/content-review.test.tsx
- apps/tldw-frontend/hooks/tests/useConfig.networking.test.tsx
- apps/tldw-frontend/hooks/useConfig.tsx
- apps/tldw-frontend/components/layout/Header.tsx
- apps/tldw-frontend/hooks/useIsAdmin.ts
- apps/tldw-frontend/tests/pages/admin-maintenance.test.tsx
| ## 0.6 Remediation status (2026-07-02) | ||
|
|
||
| All Critical and High findings above were **fixed** in this pass (working tree on `dev`; not yet committed). Each fix ships with focused unit tests. Backlog tasks `task-12091`…`task-12103` track the work. | ||
|
|
||
| **Verification:** the new + affected suites all pass — 127 tests across 14 packages/ui files, 6 (C2 redaction), 11 (shim/nav), and 23 existing store/audio/client regression tests. The only red test is a **pre-existing** `workspace.ts` quota-warning test that fails identically on baseline `dev` (confirmed via `git stash` by two reviewers) and is unrelated to these changes. | ||
|
|
||
| | # | Status | Notes | | ||
| |---|--------|-------| | ||
| | C1 | ✅ Fixed | Sanitizer removed from the runtime path (`chat-rag.ts`) and base class; buggy helper deleted. | | ||
| | C2 | ✅ Fixed | Centralized redaction in `history.ts`; logout clears history. | | ||
| | H1 | ✅ Fixed | Shared `safeExternalUrl`/`openExternalUrl` at all sinks + CSP added and **tightened**: `'unsafe-inline'` dropped from `script-src` (the one trusted inline script is SHA-256-hash-allowlisted), plus `X-Content-Type-Options`/`Referrer-Policy`/`X-Frame-Options`/`Permissions-Policy`. `'unsafe-eval'` retained (WASM) as a documented follow-up. ⚠️ **Verify in a browser before merge** — confirm no CSP violations on the main routes and the dev/error overlays (I can't run the browser here). | | ||
| | H2 | ✅ Fixed | Session state persisted to `chrome.storage.session` + `chrome.alarms` backstop. Quick-ingest **remote-job polling now resumes** after a worker restart (alarm-driven, from persisted batch records); an in-flight multipart *upload* is non-resumable by design and instead reports interrupted so the UI isn't stuck, and post-restart cancel always finds the session. | | ||
| | H3 | ✅ Fixed | Origin allowlist + `sender.id` guard on `tldw:upload`/`tldw:stream`. Guard logic **consolidated** into a single canonical `absolute-url-guard.ts` (request-core + background-proxy now import it; request-core's diagnostic warnings preserved via optional hooks) — no more triplication. | | ||
| | H4 | ✅ Fixed | Web `refreshAuth` wired with single-flight; re-armable timer; FormData-retry bug fixed. | | ||
| | H5 | ✅ Fixed | Signal threaded, per-call controllers, ownership-guarded resets, early-throw + abort-path fixes. Regenerate-abort-before-first-token now discards the empty variant and restores the prior active variant/index. | | ||
| | H6 | ✅ Fixed | 5s→config-derived timeout; non-idempotent POSTs throw `StreamInterruptedError` instead of being replayed. **F10 closed:** the `stream_transport_interrupted` sentinel is now surfaced through the normal/RAG token pipeline (captured + re-emitted in `ChatTldw.stream`), so a post-first-byte truncation is finalized as *interrupted* (parity with character chat), never saved as complete. | | ||
| | H7, H8, H11 | ✅ Fixed | Functional `set` + synchronous guard (connection); hydration published via `set` (workspace); availability flag no longer persisted, with self-healing `merge` (folder). | | ||
| | H9, H10 | ✅ Fixed | Per-area isolation + memory-only `session` + scoped `clear()`; cross-instance watch bus + `useStorage` subscription; dynamic-route `useSearchParams`. | | ||
| | H12 | ✅ Fixed | Generation-endpoint timeout 10s→120s; messaging-ack decoupled (10s→130s); body read bounded; TTS timeout in `synthesizeSpeech`. | | ||
| | Config (12102) | ◑ Partial | Done: `version`/`migrate` baseline on the 8 unversioned stores; added a `typecheck` script (`tsc --noEmit`); corrected the audit's `rules-of-hooks` claim (it is already enabled). **Blocked/phased:** removing `ignoreBuildErrors` / enabling `strict` requires first clearing ~47 **pre-existing** `tsc` errors (in unrelated Watchlists components, at the current loose settings) — that cleanup is separate from audit remediation. Enabling the newer `react-hooks` rules (`set-state-in-effect` et al.) is deferred to avoid a large, noisy fix in this PR. | | ||
| | Dead code (12103) | ✅ Done | Web auth stack (`useAuth`/`useConfig`/`Header`/`Layout`/`useIsAdmin`) **deleted** (5 files + their exclusive tests; docs updated; live `lib/*`/`WebLayout` left intact). The `extension/routes/` tree was **kept** — investigation showed it's runtime-unused but parity-test-maintained (not deletable without migrating ~22 tests); documented via `_RUNTIME_UNUSED.md`. | | ||
|
|
||
| **Residuals — nearly all closed in a follow-up pass.** Fixed since the first draft: F10 partial-stream marking (H6), quick-ingest resume (H2), guard-helper consolidation (H3), regenerate-abort discard (H5), CSP `script-src` tightening + extra security headers (H1). **What genuinely remains** (each a larger effort or needing out-of-band verification, none reintroducing a defect): | ||
| - **12102** — removing `ignoreBuildErrors` / enabling TS `strict` is blocked on ~50 **pre-existing** `tsc` errors in unrelated code (measured; must be cleared first); enabling the newer `react-hooks` rules (`set-state-in-effect` et al.) is deferred to avoid a large noisy fix. A `typecheck` script was added so the team can burn the baseline down. | ||
| - **H1** — dropping `'unsafe-eval'` from the CSP needs per-feature (WASM/OCR/tokenizer) browser verification; and the tightened CSP overall should get a quick browser smoke before merge (dev/error overlays especially). | ||
| - **12103** — the `extension/routes/` mirror is runtime-unused but kept intentionally in sync by ~22 parity tests; removing it needs those tests migrated first. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Separate completed fixes from phased follow-ups.
Remediation status says all Critical/High findings were fixed, but the next rows mark config 12102 as partial and keep H1/12103 as residual follow-ups. Reword this section so the table doesn't contradict itself.
🤖 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 `@apps/FRONTEND_AUDIT.md` around lines 48 - 73, The Remediation status section
mixes a claim that all Critical and High findings are fixed with entries that
are still partial or residual, which is contradictory. Reword the summary and
the table in FRONTEND_AUDIT.md so the completed fixes are clearly separated from
phased follow-ups, and make sure the status language for 12102, H1, and 12103
matches their actual remaining work. Use the existing section heading and the
table rows as the place to update the wording consistently.
| const persistSessionState = async (): Promise<void> => { | ||
| try { | ||
| await writePersistedSessionState({ | ||
| ingestSessions: serializeIngestSessions(ingestSessions), | ||
| pendingAuthReplay: serializePendingReplay(pendingAuthReplay), | ||
| quickIngestSessions: | ||
| serializeQuickIngestSessions(quickIngestModalSessions), | ||
| quickIngestBatches: | ||
| serializeQuickIngestBatches(quickIngestBatchRecords), | ||
| }); | ||
| } catch (error) { | ||
| logBackgroundError("persist session state", error); | ||
| } | ||
| }; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win
Consider serializing persistSessionState() writes to avoid out-of-order persistence.
persistSessionState() is called fire-and-forget from many sites in rapid succession (status updates, retries, cancels, quick-ingest lifecycle events). Each call snapshots the live Maps/Sets synchronously, but the actual area.set() write is async and unqueued, so nothing guarantees writes land in invocation order if the underlying storage API ever resolves out of order under back-to-back calls. Given this module exists specifically to make session state durable across worker suspension, a simple write queue removes this risk cheaply.
♻️ Suggested fix: chain writes through a promise queue
+ let persistQueue: Promise<void> = Promise.resolve();
const persistSessionState = async (): Promise<void> => {
- try {
- await writePersistedSessionState({
- ingestSessions: serializeIngestSessions(ingestSessions),
- pendingAuthReplay: serializePendingReplay(pendingAuthReplay),
- quickIngestSessions:
- serializeQuickIngestSessions(quickIngestModalSessions),
- quickIngestBatches:
- serializeQuickIngestBatches(quickIngestBatchRecords),
- });
- } catch (error) {
- logBackgroundError("persist session state", error);
- }
+ const snapshot = {
+ ingestSessions: serializeIngestSessions(ingestSessions),
+ pendingAuthReplay: serializePendingReplay(pendingAuthReplay),
+ quickIngestSessions:
+ serializeQuickIngestSessions(quickIngestModalSessions),
+ quickIngestBatches:
+ serializeQuickIngestBatches(quickIngestBatchRecords),
+ };
+ persistQueue = persistQueue
+ .then(() => writePersistedSessionState(snapshot))
+ .catch((error) => {
+ logBackgroundError("persist session state", error);
+ });
+ return persistQueue;
};Also applies to: 786-788, 2237-2261, 3089-3090, 3131-3131, 3145-3145, 2798-2831
🤖 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 `@apps/packages/ui/src/entries/background.ts` around lines 498 - 511,
`persistSessionState` can be invoked repeatedly and its async storage writes are
currently unqueued, which can lead to out-of-order persistence. Update the
`persistSessionState` flow in `background.ts` to serialize writes through a
single promise queue so each call waits for the previous
`writePersistedSessionState` to finish before starting the next. Use the
existing `persistSessionState` helper and the `writePersistedSessionState` call
site as the main targets, and apply the same queued behavior to all
fire-and-forget callers listed in the diff.
| const resumeActiveIngestSessions = async (): Promise<void> => { | ||
| const candidates = Array.from(ingestSessions.values()).filter( | ||
| (session) => | ||
| (session.status === "queued" || session.status === "running") && | ||
| !session.awaitingAuth && | ||
| Array.isArray(session.jobIds) && | ||
| session.jobIds.length > 0 && | ||
| !activeIngestPollFunnelIds.has(session.funnelId), | ||
| ); | ||
| for (const session of candidates) { | ||
| void resumeIngestSessionPoll(session); | ||
| } | ||
| await scheduleIngestPollAlarm(); | ||
| }; | ||
|
|
||
| // Rehydrate persisted session state after a worker restart, then resume any | ||
| // interrupted ingest polling and replay pending 401 auth-recoveries. Runs at | ||
| // most once per worker lifetime. | ||
| const rehydrateSessionState = async (): Promise<void> => { | ||
| if (sessionStateHydrated) return; | ||
| sessionStateHydrated = true; | ||
| let state; | ||
| try { | ||
| state = await readPersistedSessionState(); | ||
| } catch (error) { | ||
| logBackgroundError("read session state", error); | ||
| return; | ||
| } | ||
| for (const [funnelId, value] of Object.entries(state.ingestSessions)) { | ||
| if (!ingestSessions.has(funnelId)) { | ||
| ingestSessions.set(funnelId, value as unknown as IngestSession); | ||
| } | ||
| } | ||
| for (const funnelId of state.pendingAuthReplay) { | ||
| pendingAuthReplay.add(funnelId); | ||
| } | ||
| for (const entry of state.quickIngestSessions) { | ||
| if (!quickIngestModalSessions.has(entry.sessionId)) { | ||
| quickIngestModalSessions.set(entry.sessionId, { | ||
| sessionId: entry.sessionId, | ||
| cancelled: entry.cancelled, | ||
| abortControllers: new Set(), | ||
| }); | ||
| } | ||
| } | ||
| for (const batch of state.quickIngestBatches) { | ||
| if (!quickIngestBatchRecords.has(batch.sessionId)) { | ||
| quickIngestBatchRecords.set(batch.sessionId, batch); | ||
| } | ||
| } | ||
| // Quick-ingest modal sessions restored from a previous worker that have no | ||
| // resumable remote-job batch were interrupted during their (non-resumable) | ||
| // upload phase — report those once so the sidepanel is not left stuck. | ||
| const interruptedUploadSessionIds = state.quickIngestSessions | ||
| .map((entry) => entry.sessionId) | ||
| .filter((sessionId) => !quickIngestBatchRecords.has(sessionId)); | ||
| await resumeActiveIngestSessions(); | ||
| void replayPendingAuthSessions(); | ||
| await resumeQuickIngestBatches(); | ||
| await reportInterruptedQuickIngestUploads(interruptedUploadSessionIds); | ||
| }; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
Regular ingest sessions interrupted before job-submission are never reported as failed/stuck after a worker restart.
resumeActiveIngestSessions only resumes sessions with jobIds.length > 0. A session persisted mid "Checking for existing media..."/job-submission (status queued, jobIds: [], canCancel: false, canRetry: false) survives rehydrate but is filtered out of resume, and there is no equivalent of reportInterruptedQuickIngestUploads for this funnel. The session — and its sidepanel status card — is left stuck indefinitely with no user-facing recovery path.
Consider mirroring the quick-ingest "interrupted upload" pattern: after rehydrate, detect ingestSessions with a non-terminal status but empty jobIds that aren't otherwise being resumed, and emit a terminal failure (canRetry: true) so the user can retry.
Want me to draft the equivalent reportInterruptedIngestSessions helper (or open a follow-up issue)?
🤖 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 `@apps/packages/ui/src/entries/background.ts` around lines 2286 - 2346, The
regular ingest rehydration path in
rehydrateSessionState/resumeActiveIngestSessions leaves queued sessions with
empty jobIds stuck after restart because they are filtered out and never
reported. Add a helper similar to reportInterruptedQuickIngestUploads that scans
restored ingestSessions for non-terminal sessions that were interrupted before
job submission, and after resumeActiveIngestSessions emits a terminal failure
state with canRetry enabled so the sidepanel can recover. Use the existing
ingestSessions, resumeActiveIngestSessions, and rehydrateSessionState flow to
locate and handle these orphaned sessions.
| // Only reset the shared streaming flag / controller if this turn still owns | ||
| // it. A newer in-flight turn may have replaced the controller; clobbering it | ||
| // would re-enable the send button and orphan the newer turn's Stop button. | ||
| const stillOwnsTurn = params.releaseAbortControllerIfOwned | ||
| ? params.releaseAbortControllerIfOwned(signal) | ||
| : true | ||
| if (stillOwnsTurn) { | ||
| setIsProcessing(false) | ||
| setStreaming(false) | ||
| setAbortController(null) | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
ast-grep run --pattern 'releaseAbortControllerIfOwned' --lang typescript apps/packages/ui/src/hooks/chat/useChatActions.ts
rg -n -B3 -A15 'releaseAbortControllerIfOwned' apps/packages/ui/src/hooks/chat/useChatActions.tsRepository: rmusser01/tldw_server
Length of output: 4945
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the relevant section in chatModePipeline.ts
sed -n '900,1015p' apps/packages/ui/src/hooks/chat-modes/chatModePipeline.ts
# Check whether a non-destructive ownership helper exists anywhere in the chat hooks
rg -n "isOwnerOfAbortController|releaseAbortControllerIfOwned|activeAbortControllerRef|setIsProcessing\\(true\\)" apps/packages/ui/src/hooks/chat -g '*.ts' -g '*.tsx'Repository: rmusser01/tldw_server
Length of output: 4273
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect controller ownership and turn lifecycle around the shared abort controller.
sed -n '2340,2395p' apps/packages/ui/src/hooks/chat/useChatActions.ts
# Inspect the streaming entry points around the `setIsProcessing(true)` sites.
sed -n '1700,1765p' apps/packages/ui/src/hooks/chat/useChatActions.ts
sed -n '2890,2935p' apps/packages/ui/src/hooks/chat/useChatActions.tsRepository: rmusser01/tldw_server
Length of output: 6104
Gate the first-chunk setIsProcessing(true) on turn ownership
releaseAbortControllerIfOwned only protects cleanup. The unguarded count === 0 path can still flip isProcessing back on from a superseded turn after the active turn has already reset the UI, leaving the composer stuck in a processing state with no live request. Add a non-destructive ownership check before setting the flag.
🤖 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 `@apps/packages/ui/src/hooks/chat-modes/chatModePipeline.ts` around lines 962 -
972, The first-chunk path in chatModePipeline is setting isProcessing back to
true without confirming the current turn still owns the shared controller, which
can let a superseded turn re-toggle the UI after a newer turn has taken over.
Update the count === 0 branch to perform the same ownership check used by
releaseAbortControllerIfOwned before calling setIsProcessing(true), and only
flip the processing flag when the current signal still owns the turn. Keep the
cleanup logic around stillOwnsTurn and the shared streaming/controller state
consistent with the existing ownership handling in chatModePipeline.
| // createChatCompletion and synthesizeSpeech are implemented on | ||
| // TldwApiClientBase and are intentionally excluded from the domain interface | ||
| // via TldwDomainMethodOverride, so the base-class versions are the canonical | ||
| // (type-visible) ones. The legacy duplicates in the chat-rag / models-audio | ||
| // mixins were overwriting them at runtime, which (a) re-introduced the | ||
| // non-streaming sanitizer that corrupts successful assistant replies and | ||
| // (b) dropped the generous TTS timeout. Re-apply the base implementations so | ||
| // runtime matches the declared types and the fixes take effect. | ||
| Object.assign(TldwApiClient.prototype, { | ||
| createChatCompletion: TldwApiClientBase.prototype.createChatCompletion, | ||
| synthesizeSpeech: TldwApiClientBase.prototype.synthesizeSpeech | ||
| }) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoff
Dead mixin copies of createChatCompletion/synthesizeSpeech remain a divergence hazard.
The rebind correctly restores the base implementations at runtime, but the mixin versions in chat-rag.ts and models-audio.ts still exist and are now unreachable. They already diverge subtly (base synthesizeSpeech uses TTS_REQUEST_TIMEOUT_MS; the mixin hardcodes 120000), so a future edit to a mixin copy will silently do nothing. Consider deleting the mixin implementations (and excluding them from TldwDomainMethodOverride) rather than shadowing them at runtime.
🤖 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 `@apps/packages/ui/src/services/tldw/TldwApiClient.ts` around lines 7944 -
7955, The mixin copies of createChatCompletion and synthesizeSpeech in
chat-rag.ts and models-audio.ts are now dead code and can drift from the
canonical base behavior. Remove those duplicate implementations from the mixins
and keep TldwApiClientBase as the single source of truth, then update
TldwDomainMethodOverride so those methods are excluded from the domain interface
only where the base-class versions should be used.
| "connect-src 'self' https: http: ws: wss: data: blob:", | ||
| "worker-src 'self' blob:", | ||
| "frame-src 'self' blob: data: https: http:" | ||
| ].join('; '); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== next.config.mjs ==\n'
sed -n '1,220p' apps/tldw-frontend/next.config.mjs
printf '\n== env/config references ==\n'
rg -n --hidden --glob '!**/node_modules/**' --glob '!**/.next/**' \
'NEXT_PUBLIC_API_URL|API_URL|BACKEND|baseUrl|baseURL|backend origin|connect-src|img-src|media-src|frame-src|Content-Security-Policy|csp' \
apps/tldw-frontendRepository: rmusser01/tldw_server
Length of output: 28500
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== validate-networking-config.mjs ==\n'
sed -n '1,220p' apps/tldw-frontend/scripts/validate-networking-config.mjs
printf '\n== networking docs ==\n'
sed -n '35,90p' apps/tldw-frontend/README.mdRepository: rmusser01/tldw_server
Length of output: 4457
Scope backend egress separately from remote assets NEXT_PUBLIC_API_URL / TLDW_INTERNAL_API_ORIGIN are available when this CSP is built, but connect-src/img-src/media-src/frame-src still allow arbitrary http(s)/ws(s) origins. That leaves a broad exfil path for XSS; if external thumbnails or previews are required, keep them on a separate allowlist so the backend API can stay pinned.
🤖 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 `@apps/tldw-frontend/next.config.mjs` around lines 62 - 65, The CSP in
next.config.mjs is too broad because connect-src, img-src, media-src, and
frame-src still allow arbitrary http(s)/ws(s) origins even though
NEXT_PUBLIC_API_URL and TLDW_INTERNAL_API_ORIGIN are available. Tighten the
policy in the CSP builder by pinning backend egress in the connect-src directive
using the API origin symbols, and split any external thumbnails/previews into a
separate explicit allowlist for img-src/media-src/frame-src instead of leaving
generic network schemes open.
| async headers() { | ||
| return [ | ||
| { | ||
| source: '/:path*', | ||
| headers: [ | ||
| { | ||
| key: 'Content-Security-Policy', | ||
| value: contentSecurityPolicy, | ||
| }, | ||
| // Block MIME sniffing (defense against content-type confusion XSS). | ||
| { | ||
| key: 'X-Content-Type-Options', | ||
| value: 'nosniff', | ||
| }, | ||
| // Trim referrer leakage on cross-origin navigations. | ||
| { | ||
| key: 'Referrer-Policy', | ||
| value: 'strict-origin-when-cross-origin', | ||
| }, | ||
| // Legacy clickjacking guard; complements CSP frame-ancestors 'none'. | ||
| { | ||
| key: 'X-Frame-Options', | ||
| value: 'DENY', | ||
| }, | ||
| // Least-privilege browser features. Mic is allowed for same-origin | ||
| // audio capture (realtime transcription/dictation); camera and | ||
| // geolocation are unused and denied. | ||
| { | ||
| key: 'Permissions-Policy', | ||
| value: 'camera=(), microphone=(self), geolocation=()', | ||
| }, | ||
| ], | ||
| }, | ||
| ]; | ||
| }, |
There was a problem hiding this comment.
🔒 Security & Privacy | 🔵 Trivial
Consider adding CSP violation reporting and HSTS.
No report-to/report-uri is configured, so CSP violations (including any hash mismatches from the item above) are invisible in production. A Strict-Transport-Security header is also absent from this list; if it isn't already added by the hosting/proxy layer, consider adding it here for defense-in-depth.
🤖 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 `@apps/tldw-frontend/next.config.mjs` around lines 76 - 110, The security
headers returned by headers() in next.config.mjs are missing CSP violation
reporting and HSTS. Update the Content-Security-Policy entry to include a
reporting target such as report-to or report-uri so violations are observable in
production, and add a Strict-Transport-Security header in the same headers list
if it is not guaranteed by the hosting layer. Keep the changes localized to the
headers() configuration and preserve the existing headers like X-Frame-Options
and Permissions-Policy.
| ### CSP (next.config.mjs `async headers()`, applied to `/:path*`) | ||
| ``` | ||
| default-src 'self'; base-uri 'self'; object-src 'none'; frame-ancestors 'none'; | ||
| script-src 'self' 'unsafe-inline' 'unsafe-eval' blob:; style-src 'self' 'unsafe-inline'; | ||
| img-src 'self' data: blob: https: http:; font-src 'self' data:; | ||
| media-src 'self' data: blob: https: http:; connect-src 'self' https: http: ws: wss: data: blob:; | ||
| worker-src 'self' blob:; frame-src 'self' blob: data: https: http: | ||
| ``` | ||
| `object-src 'none'` + `base-uri 'self'` + `frame-ancestors 'none'` are the hard protections. `script-src` keeps `'unsafe-inline'`/`'unsafe-eval'` (required by the `_document` theme-bootstrap inline script, antd runtime, Next.js) and `blob:` (Web Workers: OCR/diff/tokenizer). Resource directives (img/media/connect/frame) stay broad so external thumbnails, remote backends, realtime-audio WebSockets, and blob:/PDF iframe previews keep working. | ||
| **Follow-up:** tighten `script-src` to nonce/hash-based (drop `'unsafe-inline'`/`'unsafe-eval'`) — this also lets CSP block `javascript:` navigation directly. Consider adding `X-Content-Type-Options: nosniff` and `Referrer-Policy`. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Refresh the CSP notes to match the shipped policy.
These lines still describe the pre-hardened header set ('unsafe-inline' retained and the extra headers as follow-ups), but the referenced next.config.mjs already drops 'unsafe-inline', hashes the theme bootstrap script, and includes those headers. Update the task text so it reflects the final state.
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 57-57: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 58-58: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 58-58: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 64-64: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
🤖 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 `@backlog/tasks/task-12093` -
Fix-javascript-URL-DOM-XSS-in-source-anchors-and-add-CSP.md around lines 57 -
66, Update the CSP task notes to match the final shipped policy: the current
text in the task section describing `next.config.mjs` still reflects the older
header set with `'unsafe-inline'` and the follow-up wording. Revise the
description around the CSP block and the “Follow-up” note so it matches the
actual `headers()` policy, which already uses the hardened `script-src` setup
and includes the additional security headers. Keep the references aligned with
`next.config.mjs`, `async headers()`, and the CSP summary text in this task
file.
| - `apps/tldw-frontend/eslint.config.mjs:78-84,118` — **`react-hooks` rules disabled**, including `rules-of-hooks` (`:118`) which catches genuine conditional-hook crash bugs. `@typescript-eslint/no-explicit-any` is only `warn`. | ||
| - **Persisted stores lack `version`/`migrate`** (8 of 9): `playground-session`, `persona-buddy-shell`, `notes-dock`, `ui-mode`, `actor`, `quick-ingest-session`, `folder`, `feedback`, `acp-sessions`. The day someone adds `version:1` to reshape a store without a `migrate`, all users' persisted state is discarded; a field rename before then ships `undefined` into consumers. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Narrow the lint-gate description to the rules that are actually disabled.
The audit correction says rules-of-hooks is only disabled under e2e/**; the global disables are the newer react-hooks/* checks. As written, this backlog item sends readers after a rule that is already active outside e2e.
🤖 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 `@backlog/tasks/task-12102` - Re-enable-frontend-type-safety-and-lint-gates.md
around lines 22 - 23, Narrow the lint-gate task description so it only mentions
the globally disabled react-hooks rules and not rules-of-hooks, since that rule
is already active outside e2e. Update the wording in the backlog note to match
the actual disables in eslint.config.mjs, and use the existing symbols
react-hooks and `@typescript-eslint/no-explicit-any` as the anchors for the fix.
| 1. **Half-wired web auth stack.** The web `AuthProvider` (`apps/tldw-frontend/hooks/useAuth.tsx:16`) and web `ConfigProvider` (`hooks/useConfig.tsx:237`) are **never mounted** — the `ConfigProvider` in `components/AppProviders.tsx:80` is antd's, not this one; these two appear only in their own definitions and in `__tests__`. Their only consumers, `components/layout/Header.tsx` → `components/layout/Layout.tsx`, are imported by nothing (`WebLayout` uses a different shared Header). So `useAuth`/`useConfig` would throw if rendered, and `api.defaults.baseURL` is never synced from user config. It's ~500 lines of code a maintainer will mistake for live. | ||
|
|
||
| 2. **Dead `extension/routes` tree.** `apps/tldw-frontend/extension/routes/*` (`route-registry.tsx`, `app-route.tsx`, all `option-*.tsx`) is not used in the web build — pages mount `packages/ui/src/routes/*` via the `@/` alias (`next.config.js:69,85`). The two copies have diverged (e.g. the live route wraps `RouteErrorBoundary`, the dead copy doesn't); edits to the dead copy silently no-op. Note: the `extension/shims/*` in the same directory ARE live — keep those. | ||
|
|
||
| Goal: delete or clearly quarantine the dead code so future maintainers don't edit or trust it. If any piece is intended for future use, document that and add a build/test guard so it can't silently rot. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Call out the parity-test dependency and correct the config path.
This note points at next.config.js, but the surrounding audit context uses next.config.mjs; it also omits that extension/routes/* is intentionally kept in sync by parity tests and isn't safely deletable yet. Please update the path and add that guard so the ticket doesn't read like a trivial delete.
🤖 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 `@backlog/tasks/task-12103` -
Remove-half-wired-dead-web-auth-stack-and-dead-extension-routes-tree.md around
lines 19 - 23, Update the note to reference the correct config entrypoint, using
next.config.mjs instead of next.config.js, and mention that the dead-looking
extension/routes tree is still protected by parity tests so it should not be
framed as a simple deletion. Point the guidance at the relevant symbols such as
extension/routes/*, route-registry.tsx, app-route.tsx, and the shared UI routes
under packages/ui/src/routes/*, and add a build/test guard or explicit
quarantine note so future edits can’t silently diverge.
Address the Critical and High findings from the 2026-07-02 frontend/extension audit (apps/FRONTEND_AUDIT.md), plus config hardening and dead-code cleanup. Critical: - Stop the chat-completion "sanitizer" from corrupting successful non-streaming replies that mention "error"/"exception"/a file path (removed at the runtime source in domains/chat-rag.ts and the base class). - Stop persisting auth headers and login tokens to the localStorage request history; redact centrally and clear history on logout. High: - DOM-XSS: shared safeExternalUrl/openExternalUrl guard at all source-URL anchor and window.open sinks; add a CSP (script-src SHA-256-hash-allowlisted, drop 'unsafe-inline') plus X-Content-Type-Options/Referrer-Policy/X-Frame-Options/ Permissions-Policy. - MV3: persist ingest/quick-ingest/auth-replay session state and drive long polls via chrome.alarms so they survive service-worker suspension; resume remote-job polling after a restart. - Extension: origin allowlist + sender.id guard before attaching credentials on tldw:upload/tldw:stream; consolidate the guard into absolute-url-guard.ts. - Web token refresh wired with single-flight; post-refresh FormData retry fix. - Chat: Stop aborts the transport; per-call abort controllers (fixes Compare mode); ownership-guarded resets; user abort no longer saved as a complete answer; stream_transport_interrupted surfaced through the normal-mode pipeline. - Stream connection timeout no longer replays non-idempotent chat POSTs; generation request timeouts raised 10s -> 120s (and messaging-ack decoupled). - Zustand: connection-store stale-snapshot race, workspace silent-mutation hydration, folder sticky-404; storage shims (scoped clear, memory-only session, cross-instance change propagation, dynamic-route useSearchParams). Config / cleanup: - version/migrate baseline on 8 persisted stores; add a typecheck script. - Delete the dead web-auth provider stack (useAuth/useConfig/useIsAdmin/Header/ Layout); document the runtime-unused extension/routes mirror. Verification: new unit tests pass across all touched areas (~167 tests). Four pre-existing test failures are unrelated (confirmed on baseline dev). Backlog tasks task-12091..task-12103 track the work; remaining follow-ups (TS strict gate, CSP unsafe-eval, extension/routes removal) are documented in apps/FRONTEND_AUDIT.md section 0.6. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ch 1) Round-2 focused audit of character chat and TTS/STT (apps/FRONTEND_AUDIT.md "Round 2"). Fixes the safe, client-only first batch; the rest is ticketed (task-12104..task-12110). Fixed: - Character chat delete/edit hit the wrong Dexie row when a greeting shifted UI-array indices out of alignment with the Dexie history. Now address the row by stable message id (removeMessageById/updateMessageById/ deleteChatAfterMessageId), with index fallback for id-less rows. (task-12104) - Microphone could stay live after a MediaRecorder error or a double-start. Brought useAudioRecorder / useServerDictation / SpeechPlaygroundPage up to the useMicStream template: MediaRecorder onerror stops tracks + releases the capture lock, a synchronous re-entry guard prevents double-start orphans, and the acquired stream is held in a catch-reachable ref. (task-12105) - TTS playback leaks + overlapping audio: revoke the MediaSource blob URL before the streaming->buffered fallback overwrites it; useTTS.cancel() now settles the in-flight playback promise and revokes the segment URL (no orphaned generator / leaked URL); TtsClipsDrawer stops the prior clip before playing a different one; audiobook cancel aborts the in-flight chapter synthesis; useDocumentTTS detaches listeners + revokes on error. (task-12107) Ticketed for follow-up (not in this batch): - WS auth token in the URL query string (task-12106) — backend already supports subprotocol / first-message auth, but switching the client and dropping the URL token must be validated against a live server (would break voice/STT if wrong). - characterChatMode triplication + inactivity watchdog (task-12108); real-time voice hardening: barge-in audio-stop, backpressure, handshake timeout, WS unmount leaks (task-12109); character card handling: export SSRF, size caps, favorite desync, variant server-id (task-12110). Deferred within task-12104: greeting is still not persisted to Dexie, so a Dexie-sourced rehydrate shows one fewer message than a server-sourced one (cosmetic; delete/edit no longer corrupt the store). Tests: 68 passing across the touched areas (message-target-by-id, useAudioRecorder, useServerDictation, useTTS.cancel, useStreamingAudioPlayer.leak, useAudiobookGeneration.abort, TtsClipsDrawer.overlap, useDocumentTTS) + regressions. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
rmusser01
force-pushed
the
feat/frontend-extension-audit-remediation
branch
from
066502b to
c6e5a20
Compare
…tch 2)
Second remediation batch for the Round-2 audit (apps/FRONTEND_AUDIT.md).
Fixed (with tests):
- R8 (task-12109) real-time voice hardening: barge-in now stops buffered TTS and
sends one interrupt per turn; mic->WS send honors ws.bufferedAmount backpressure;
10s handshake timeout clears wedged "connecting" state; attempt-id/mounted guards
stop usePersonaLiveSession/usePersonaLiveControl leaking a WebSocket on
unmount-mid-connect.
- R9/R10 (task-12110) character cards: PNG export no longer fetches an arbitrary
attacker-controlled avatar_url (same-origin/allowlisted only, AbortController
timeout, credentials:omit, 5MB cap); avatar upload + card import size caps;
favorites reconciled to the server flag with the correct React-Query cache key;
a swiped-but-unpersisted variant no longer inherits a stale serverMessageId.
- R5 (task-12108) partial: a 60s stream-inactivity watchdog + recovery
classification is now on BOTH live character-chat paths (a source-grep guard
test verifies the shipped code got it), so a stalled character stream times out
instead of hanging. Full 3-copy consolidation deferred (large refactor while dev
is actively churning chat).
Partially done / needs live-server validation:
- R3 (task-12113) WS token moved OUT of the URL query string: persona uses the
Sec-WebSocket-Protocol ["bearer", cred] subprotocol (persona.py:3705-3712);
audio/STT + voice send {type:"auth",token} as the first message
(streaming_service.py). A charset fallback keeps a non-token-safe custom API key
from crashing new WebSocket() by reverting that case to the query token.
MUST be smoke-tested against a running backend before merge (subprotocol
handshake, single-user key charset, auth-before-config ordering, extension STT).
Deliberately deferred (documented, not silently skipped): TS-strict enablement
(blocked on ~66 pre-existing type errors), dropping CSP 'unsafe-eval' (needs WASM
browser verification), deleting the extension/routes mirror (kept in sync by ~22
parity tests; churn during dev's active route refactor).
Ticket renumbering: this branch's round-2 tickets were renumbered off 12104-12106
to 12111-12113 because teammates used 12104-12106 on origin/dev for PR-2573 work.
Tests: all round-2 follow-up + regression suites pass; one pre-existing
usePersonaLiveControl failure (confirmed identical on HEAD) is unrelated.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Resolve actionable findings from the automated PR reviewers (gemini, qodo, coderabbit). Bugs: - connection.tsx: release the checkInFlight guard on ALL paths — a throw in the pre-`try` awaits (getFirstRunCompleteFlag etc.) no longer deadlocks every future health check for the session (full-body try/catch that resets the guard). - background-proxy.ts refreshAuthDirect: throw on a failed/empty /auth/refresh so request-core sees refreshSucceeded=false and doesn't retry with a stale token (surfaces "Session expired" instead). - background.ts: pre-submission ingest sessions (queued, no jobIds) are now reported as interrupted + cleaned up on worker restart instead of silently stranded; persistSessionState writes are serialized (write-again-if-dirty) so an older snapshot can't clobber a newer one. Security: - history.ts: broaden request/response body redaction (id_token, session_token, api_key/apiKey/x-api-key, jwt, secret, password, client_secret); make the depth-limited redactor fail CLOSED (return [REDACTED] past depth 6, not raw). Shims: - plasmo-storage.ts: guard the global `storage` listener's JSON.parse; log watcher-callback errors instead of swallowing them. - plasmo-storage-hook.tsx: a `watch()` update that races the initial `get()` is no longer clobbered by the stale snapshot. Stores/tests/docs: - folder.tsx: add version/migrate baseline (consistency with the other stores). - connection.test.ts: use a fixed timestamp instead of Date.now() for determinism. - FRONTEND_AUDIT.md + tasks 12093/12102/12103: reword to match the shipped CSP (unsafe-inline dropped, hash-based, extra headers), correct the rules-of-hooks scope note (e2e-only), and note the extension/routes parity-test dependency. Verified already-addressed (no change): old history entries re-scrubbed on write; request-core dangling timers cleared in finally; wxt-browser multi-key set only emits onChanged for committed keys. Deliberately not taken (deployment/infra-sensitive): HSTS (self-hosted HTTP/LAN), CSP report-uri (needs a collector), build-time script-hash (manual constant works). Tests: 145 passing across the touched areas. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 participant
What this is
Remediation of the 2026-07-02 frontend + extension audit (
apps/FRONTEND_AUDIT.md). It fixes every Critical and High finding, plus config hardening and dead-code cleanup. Both the Next.js web UI and the WXT extension are thin shells overapps/packages/ui, so most fixes land there and benefit both.Backlog tasks TASK-12091 … TASK-12103 track each item; per-finding status is in
apps/FRONTEND_AUDIT.md§0.6.Critical
"Chat completion failed.". Removed at the real runtime source (domains/chat-rag.ts) and the base class.localStorage. Request-history was storing liveAuthorization/X-API-KEYheaders and login-response tokens, and never cleared them (even on logout). Now redacted centrally and cleared on logout.High
javascript:source/citation URLs (no CSP backstop): sharedsafeExternalUrl/openExternalUrlguard at ~17 sinks + a CSP that drops'unsafe-inline'(the one trusted inline script is SHA-256-hash-allowlisted) +X-Content-Type-Options/Referrer-Policy/X-Frame-Options/Permissions-Policy.chrome.storage.session, polls driven bychrome.alarms, remote-job polling resumes after a restart.sender.idguard before attaching the API key ontldw:upload/tldw:stream; guard logic consolidated into oneabsolute-url-guard.ts.stream_transport_interruptedsurfaced in normal-mode pipelines.clear(), memory-onlysession, cross-instance change propagation, dynamic-routeuseSearchParams.Config / cleanup
version/migratebaseline on the 8 unversioned persisted stores; added atypecheckscript.useAuth/useConfig/useIsAdmin/Header/Layout) + their exclusive tests; documented the runtime-unused (but parity-test-maintained)extension/routes/mirror.Verification
New unit tests pass across all touched areas (~167 tests). Four failing tests are pre-existing and unrelated, confirmed identical on baseline
devviagit stash:workspace.test.ts"dispatches a quota warning event when localStorage is full"react-router-shim-transition.test.tsxsources-route.test.tsx(cwd artifact)chatModePipeline.conversation-id.test.ts"treats empty model streams as failed"/,/chat,/research,/media,/audio-studio,/settings,/loginwith the console open and confirm no CSP violations, that the theme applies without flash, and that mic features work. Pay attention to the dev/error overlays (dev-only inline scripts).Known follow-ups (documented, not blocking)
ignoreBuildErrors/ enabling TSstrictis blocked on ~50 pre-existingtscerrors in unrelated code (atypecheckscript was added to measure/burn down); enabling the newerreact-hooksrules deferred.'unsafe-eval'(WASM) needs per-feature browser verification.extension/routes/removal needs its ~22 parity tests migrated first.🤖 Generated with Claude Code
Summary by cubic
Remediates all Critical/High items from the 2026‑07‑02 frontend + extension audit and hardens character chat and TTS/STT. Adds CSP/XSS guards, MV3 session resilience, safer networking/auth (incl. WS auth out of URLs), more robust chat/voice abort lifecycles, and small UI fixes.
New Features
absolute-url-guard.tspowers bothrequest-coreand the background proxy; credentials attach only to allowlisted/same‑origin requests. Persona/voice WS auth moved out of URLs into theSec-WebSocket-Protocolsubprotocol or first auth frame; 10s connect timeouts and attempt guards prevent wedged sockets (TASK-12113).WebSocket.bufferedAmountbackpressure; voice and persona streams fail fast on handshake timeout (TASK-12109).AbortControllerwith ownership‑guarded release;stream_transport_interruptedis re‑emitted so truncated answers are classified correctly.safeExternalUrl/openExternalUrladoption across sources, watchlists, search results, and notes; notes link sanitizer now strips control‑char‑obfuscated schemes.credentials: "omit"; added size caps for avatar uploads (5 MB) and character imports (10 MB); favorites reconcile to the server flag via the correct React‑Query key (TASK-12110).Bug Fixes
serverMessageId(TASK-12108/12110). Dexie edits/deletes now target messages by stable id (not index) when a greeting offsets arrays.chrome.storage.session, resume on restart, and pre‑submission sessions are marked interrupted/cleaned up; background proxy messaging‑ack timeout raised to cover long generations; session‑state writes are serialized to avoid clobbering newer snapshots.MediaRecorder.onerrorcleanup acrossuseAudioRecorder,useServerDictation, and Speech Playground; streaming→buffer fallback revokes prior blob URLs;useTTS.cancel()settles in‑flight playback and revokes URLs;TtsClipsDrawerstops an existing clip before starting another;useDocumentTTSdetaches listeners and revokes on error (TASK-12105/12107).Written for commit 4755c56. Summary will update on new commits.