feat(12.1): AES-CTR streaming encryption for media files#135
Merged
Conversation
Phase 12.1: AES-CTR Streaming Encryption - Implementation decisions documented - Phase boundary established - Desktop FUSE CTR deferred to gap-closing phase Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
AES-CTR streaming encryption research covering: - Service Worker transparent decrypt proxy pattern - AES-256-CTR nonce/counter management for random access - Streaming upload with TransformStream - IPFS byte-range limitations and workarounds - Custom media player architecture Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Phase 12.1: AES-CTR Streaming Encryption - 4 plans in 3 waves - 2 parallel (upload pipeline + service worker), 2 sequential - Ready for execution Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- AES_CTR_IV_SIZE (16), AES_CTR_NONCE_SIZE (8), AES_CTR_LENGTH (64), AES_CTR_ALGORITHM constants - generateCtrIv() with 8-byte random nonce + 8-byte zero counter - Exported through utils/index.ts barrel Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- encryptAesCtr() for full-buffer AES-256-CTR encryption via Web Crypto - decryptAesCtr() for full-buffer decryption - decryptAesCtrRange() for random-access byte-range decryption with computed counter offset - BigInt counter arithmetic for 64-bit counter space - All functions exported from @cipherbox/crypto barrel Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Round-trip tests for various sizes (empty, 1B, 15B, 16B, 17B, 1MB) - CTR output size verification (no auth tag, same size as input) - Range decrypt at block-aligned and unaligned offsets - Single-byte range, full-file range, cross-block range - Boundary clamping and out-of-range handling - Key/IV uniqueness verification - Input validation (wrong key size, wrong IV size, invalid range) - generateCtrIv format (16 bytes, zero counter, random nonce) - Large file (1MB) middle-range decrypt - CryptoError code assertions - 35 test cases, all passing Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Tasks completed: 3/3 - CTR constants and IV generation - CTR encrypt, decrypt, and range-decrypt functions - CTR encryption test suite (35 tests) SUMMARY: .planning/phases/12.1-aes-ctr-streaming-encryption/12.1-01-SUMMARY.md Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add streaming-crypto.service.ts with selectEncryptionMode() and encryptFileCtr() - selectEncryptionMode: returns CTR for media files >256KB, GCM for all others - encryptFileCtr: streams file in 1MB chunks with AES-256-CTR, counter derived from byte offset - Add encryptionMode field to EncryptedFileResult type Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Intercepts /decrypt-stream/* URLs from media elements - Fetches encrypted content from API, caches full file in memory - Decrypts arbitrary byte ranges with AES-CTR counter offset math - Returns proper 200/206 responses with Content-Range headers - Auth token and API base URL managed via postMessage - Separate tsconfig.sw.json for WebWorker lib types Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- encryptFile() dispatches to CTR or GCM based on selectEncryptionMode() - UploadedFile type carries encryptionMode through pipeline - useFolder handleAddFile/handleAddFiles pass encryptionMode to createFileMetadata - useDropUpload passes encryptionMode from upload result to folder registration - Barrel export streaming-crypto.service from services/index.ts Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Tasks completed: 2/2 - Streaming crypto service with mode selection - Mode-aware file encryption and upload pipeline SUMMARY: .planning/phases/12.1-aes-ctr-streaming-encryption/12.1-02-SUMMARY.md Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ootstrap - sw-registration.ts: registerDecryptSW, sendToSW, isSwActive, waitForSW, updateSwToken, setSwApiBase, registerStream, unregisterStream - build-sw.mjs: post-build script compiles decrypt-sw.ts to dist/decrypt-sw.js using Vite lib-mode (IIFE, minified) - main.tsx: registers SW on startup, syncs API base URL and auth token - package.json: build script includes SW compilation step Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Tasks completed: 2/2 - Service Worker for AES-CTR decrypt proxy - SW registration, build pipeline, and app bootstrap SUMMARY: .planning/phases/12.1-aes-ctr-streaming-encryption/12.1-03-SUMMARY.md Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- download.service.ts: CTR decryption support via encryptionMode field - useStreamingPreview: SW-based streaming media preview hook - decrypt-sw.ts: progress postMessage during encrypted file fetch - Fallback to blob URL when SW not active or file is GCM Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Dual-hook pattern: useStreamingPreview for CTR, useFilePreview for GCM - Decrypt progress bar with percentage during SW encrypted content fetch - ENCRYPTED badge on video screen when streaming CTR playback active - Stream cleanup on dialog close via streaming.cleanup() - Falls back to blob URL when SW not active or file is GCM Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Dual-hook pattern: useStreamingPreview for CTR, useFilePreview for GCM - Decrypt progress bar with percentage during SW encrypted content fetch - Web Audio API visualization works with SW-proxied audio source - Stream cleanup on dialog close via streaming.cleanup() - Falls back to blob URL when SW not active or file is GCM Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Tasks completed: 3/3 - Mode-aware download service and streaming preview hook - Video player dialog with streaming decrypt support - Audio player dialog with streaming decrypt support SUMMARY: .planning/phases/12.1-aes-ctr-streaming-encryption/12.1-04-SUMMARY.md Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughCompletes Phase 12.1 by implementing AES-256-CTR streaming encryption infrastructure: adds CTR crypto primitives and tests, introduces mode-aware file encryption in the web app, builds a Service Worker-based transparent decrypt proxy for media playback, and integrates streaming preview support into video/audio players with progress UI and upload/download propagation of encryptionMode. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client (Browser)
participant Main as Main App
participant Crypto as Streaming Crypto
participant Upload as Upload Service
participant API as IPFS/API
Client->>Main: Upload file
Main->>Crypto: selectEncryptionMode(file)
Crypto-->>Main: 'CTR' or 'GCM'
alt CTR
Main->>Crypto: encryptFileCtr(file, userPublicKey)
Crypto->>Crypto: generateCtrIv(), generate fileKey
loop per 1MB chunk
Crypto->>Crypto: encryptAesCtr(chunk, fileKey, iv)
end
Crypto->>Crypto: wrapKey(fileKey)
Crypto-->>Main: EncryptedFileResult {cid?, iv, wrappedKey, encryptionMode:'CTR'}
else GCM
Main->>Crypto: encryptFileGcm(...)
Crypto-->>Main: EncryptedFileResult {..., encryptionMode:'GCM'}
end
Main->>Upload: uploadFile(encrypted, encryptionMode)
Upload->>API: POST /upload {ciphertext, metadata{encryptionMode}}
API-->>Upload: response {cid, ...}
Upload-->>Main: UploadedFile {cid, wrappedKey, iv, encryptionMode}
sequenceDiagram
participant Viewer as VideoPlayerDialog
participant Hook as useStreamingPreview
participant Meta as IPNS Metadata API
participant SW as Service Worker (decrypt-sw)
participant Media as HTML5 Media Element
Viewer->>Hook: open preview(item)
Hook->>SW: isSwReady() / waitForSW()
Hook->>Meta: resolveFileMetadata(ipnsName)
Meta-->>Hook: {encryptionMode:'CTR', iv, cid, wrappedKey}
Hook->>Hook: unwrapKey(wrappedKey, vaultKey)
Hook->>SW: registerStream({ipnsName, fileKey, iv, cid, mimeType, totalSize})
Hook-->>Viewer: streamUrl = /decrypt-stream/{ipnsName}
Viewer->>Media: set src = streamUrl
Media->>SW: GET /decrypt-stream/{ipnsName} [Range: bytes=0-...]
SW->>API: fetch encrypted bytes (range)
API-->>SW: encrypted bytes
SW->>SW: decryptAesCtrRange(encrypted, fileKey, iv, start, end)
SW-->>Media: HTTP 206 {Content-Range, decrypted bytes}
SW-->>Viewer: postMessage({fetch-progress: X%})
Viewer->>Viewer: update decrypt progress UI
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/web/src/components/file-browser/AudioPlayerDialog.tsx (1)
395-401:⚠️ Potential issue | 🟡 MinorMissing
e.preventDefault()on arrow key handling in audio progress slider.Unlike
VideoPlayerDialogwhich callse.preventDefault()forArrowRight/ArrowLeft, the audio progress bar omits it. This may cause the page to scroll horizontally while the user tries to seek.Add preventDefault
onKeyDown={(e) => { const audio = audioRef.current; if (!audio || !duration) return; - if (e.key === 'ArrowRight') + if (e.key === 'ArrowRight') { + e.preventDefault(); audio.currentTime = Math.min(duration, audio.currentTime + 5); - else if (e.key === 'ArrowLeft') + } else if (e.key === 'ArrowLeft') { + e.preventDefault(); audio.currentTime = Math.max(0, audio.currentTime - 5); + } }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/components/file-browser/AudioPlayerDialog.tsx` around lines 395 - 401, The onKeyDown handler in AudioPlayerDialog (attached to the audio progress slider) updates audio.currentTime for 'ArrowRight'/'ArrowLeft' but doesn't call e.preventDefault(), allowing the page to scroll; update the handler (the onKeyDown function that references audioRef.current and duration) to call e.preventDefault() before adjusting audio.currentTime for both ArrowRight and ArrowLeft so key presses don't trigger default scrolling behavior..planning/ROADMAP.md (1)
57-57:⚠️ Potential issue | 🟡 MinorPhase 12.6 checkbox is unchecked but marked Complete in the progress table (line 385).
Line 57 shows
- [ ] **Phase 12.6: Per-File IPNS Metadata Split**while the progress table on line 385 showsComplete | 2026-02-17. The individual plan checkboxes (lines 272-276) are also unchecked. These appear stale from before 12.6 was completed.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.planning/ROADMAP.md at line 57, Update the roadmap to mark Phase 12.6 as completed: change the checklist item "- [ ] **Phase 12.6: Per-File IPNS Metadata Split**" to checked ("- [x]") and likewise mark the related individual plan checkboxes referenced around the block for lines 272-276 as checked, and remove or update any "INSERTED" tag so the single-phase checkbox, the per-file plan checkboxes, and the progress table entry for Phase 12.6 are consistent with the "Complete | 2026-02-17" status.
🧹 Nitpick comments (13)
apps/web/scripts/build-sw.mjs (1)
19-24: Consider enabling sourcemaps for the Service Worker bundle.Debugging minified SW code in DevTools can be painful. Generating a sourcemap alongside the minified output would help with production issue diagnosis.
💡 Optional: enable sourcemaps
build: { lib: { entry: path.resolve(root, 'src/workers/decrypt-sw.ts'), formats: ['iife'], name: 'DecryptSW', }, outDir: path.resolve(root, 'dist'), emptyOutDir: false, minify: true, + sourcemap: true, rollupOptions: {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/scripts/build-sw.mjs` around lines 19 - 24, The Service Worker bundle defined under the build.lib block (entry: 'src/workers/decrypt-sw.ts', formats: ['iife'], name: 'DecryptSW') should emit a sourcemap; enable sourcemaps by adding the appropriate sourcemap option to the Vite build configuration (e.g., set sourcemap: true on the build or build.lib config) so the minified SW output produces an accompanying .map file for easier debugging in DevTools.apps/web/package.json (1)
8-8: Add TypeScript type-checking for the Service Worker to the build pipeline.The Service Worker TypeScript is not type-checked during the build.
tsc -bonly checks project references intsconfig.json(currently onlytsconfig.node.json), excluding the SW. Thetsconfig.sw.jsonexists but is unused, andscripts/build-sw.mjsperforms only transpilation via Vite without type-checking.Add
tsc -p tsconfig.sw.json --noEmitto the build chain to ensure the SW code is type-checked:Add SW type-check to build
- "build": "tsc -b && vite build && node scripts/build-sw.mjs", + "build": "tsc -b && tsc -p tsconfig.sw.json --noEmit && vite build && node scripts/build-sw.mjs",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/package.json` at line 8, The package.json build pipeline currently runs "tsc -b && vite build && node scripts/build-sw.mjs" but misses type-checking the Service Worker; update the build pipeline to run TypeScript type-check against tsconfig.sw.json by invoking tsc -p tsconfig.sw.json --noEmit (before the SW transpilation step) so the Service Worker is type-checked; ensure the addition is ordered so tsconfig.sw.json is validated prior to running node scripts/build-sw.mjs and reference the existing scripts/build-sw.mjs, the package.json "build" script, and tsconfig.sw.json when making this change.apps/web/src/workers/decrypt-sw.ts (3)
17-23:fileKeystored as hex string in SW memory — document the trust boundary.The
StreamContextholds the raw AES key material (as hex). This is the correct approach for the SW decrypt proxy, but it means the SW process holds decryption keys for all active streams. Consider adding a brief comment noting the security invariant: keys are only held while a stream is registered and must be cleared viaunregister-streamwhen playback ends.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/workers/decrypt-sw.ts` around lines 17 - 23, The StreamContext currently stores raw AES key material in the service worker (fileKey as hex) which creates a trust boundary; update the comment near the StreamContext type in decrypt-sw.ts to explicitly document that fileKey contains hex-encoded AES-256 keys held in SW memory only for the lifetime of a registered stream, that keys must be cleared when playback ends, and reference the unregister-stream message/handler to ensure streams remove keys from SW memory; mention the security invariant that the SW is trusted for active streams and keys must not be persisted beyond unregister-stream.
240-313: No timeout on the IPFS fetch — consider addingAbortSignal.timeout().If the IPFS gateway is slow or unresponsive,
fetch(fetchUrl, ...)will hang indefinitely with no way for the user to recover. A reasonable timeout (e.g., 60s) would let the SW return an error so the UI can show a failure state.Sketch
+ const controller = new AbortController(); + const timeoutId = setTimeout(() => controller.abort(), 60_000); try { - response = await fetch(fetchUrl, { headers: fetchHeaders }); + response = await fetch(fetchUrl, { headers: fetchHeaders, signal: controller.signal }); } catch { return new Response('Failed to fetch encrypted content', { status: 502 }); + } finally { + clearTimeout(timeoutId); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/workers/decrypt-sw.ts` around lines 240 - 313, The fetch to the IPFS gateway (where fetchUrl is used to populate encrypted via encryptedCache.get(cacheKey)) has no timeout; add an AbortController/AbortSignal.timeout(60000) (or similar) and pass its signal to fetch(fetchUrl, { headers: fetchHeaders, signal }) so the request is aborted after ~60s; catch the abort error in the existing try/catch and return a meaningful Response (e.g., 504 Gateway Timeout) and call postToClients with a fetch-error/fetch-timeout payload (include fileMetaIpnsName: cacheKey and the error) so the UI can recover. Ensure you clean up any controller if needed and keep the other response-status handling (401/response.ok) unchanged.
55-65: UnboundedencryptedCachecan exhaust SW memory.Each played media file is cached in full with no eviction. A user who previews several large videos will accumulate all of them in memory until each corresponding
unregister-streammessage is sent. Consider adding a cap (e.g., LRU eviction or max entry count) as a safety net against callers that fail to unregister.Sketch: simple LRU cap
+const MAX_CACHE_ENTRIES = 5; + /** In-memory cache of fetched encrypted files (full file per CID) */ const encryptedCache = new Map<string, Uint8Array>(); + +function cacheSet(key: string, data: Uint8Array): void { + if (encryptedCache.size >= MAX_CACHE_ENTRIES) { + // Evict oldest entry (Map preserves insertion order) + const oldest = encryptedCache.keys().next().value; + if (oldest !== undefined) encryptedCache.delete(oldest); + } + encryptedCache.set(key, data); +}Then replace
encryptedCache.set(cacheKey, encrypted)on line 308 withcacheSet(cacheKey, encrypted).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/workers/decrypt-sw.ts` around lines 55 - 65, The encryptedCache Map currently stores full files with no eviction, risking service-worker memory exhaustion; implement a capped LRU cache wrapper around encryptedCache (e.g., replace the Map with an LRU implementation or create helper functions cacheGet/cacheSet that enforce a maxEntries limit and evict least-recently-used items) and update all uses to call cacheGet/cacheSet instead of encryptedCache.get/encryptedCache.set; ensure the eviction interacts correctly with stream lifecycle (streamRegistry and the "unregister-stream" message) so entries removed by LRU are cleaned up or ignored by StreamContext consumers.apps/web/src/components/file-browser/AudioPlayerDialog.tsx (1)
67-73: Same dual-hook double-fetch concern as VideoPlayerDialog.The
blobPreviewactivates beforestreaming.isCtrresolves, causing redundant download for CTR files. Same fix as suggested inVideoPlayerDialog.tsxapplies here — gate on!streaming.loading.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/components/file-browser/AudioPlayerDialog.tsx` around lines 67 - 73, In AudioPlayerDialog the blobPreview useFilePreview call initializes while streaming.isCtr is still unresolved, causing duplicate downloads; update the open condition passed to useFilePreview (the blobPreview declaration) to also require streaming finished loading (e.g., add && !streaming.loading) so the preview is only requested after streaming.isCtr is resolved; reference: blobPreview, useFilePreview, streaming.isCtr, streaming.loading in AudioPlayerDialog.apps/web/src/components/file-browser/VideoPlayerDialog.tsx (2)
282-289:streamingobject reference inuseCallbackdeps recreateshandleCloseevery render.
streamingis a new object on every render (returned from the hook). This makes theuseCallbackmemoization ineffective and causeshandleCloseto be a new function every render.Use the stable `cleanup` reference instead
const handleClose = useCallback(() => { if (videoRef.current) { videoRef.current.pause(); } setIsPlaying(false); - streaming.cleanup(); + streamingCleanup(); onClose(); - }, [onClose, streaming]); + }, [onClose, streamingCleanup]);Add a stable reference above:
const streamingCleanup = streaming.cleanup;Or depend on
streaming.cleanupdirectly since it's auseCallbackwith[]deps inside the hook.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/components/file-browser/VideoPlayerDialog.tsx` around lines 282 - 289, The handleClose useCallback currently depends on the streaming object which is recreated each render, breaking memoization; update handleClose to depend on a stable cleanup reference instead (e.g., capture streaming.cleanup into a const like streamingCleanup or reference streaming.cleanup directly) so the dependency array uses streamingCleanup (or streaming.cleanup) rather than streaming; keep existing behavior: call videoRef.current.pause(), setIsPlaying(false), streamingCleanup(), and onClose() inside handleClose and include onClose and streamingCleanup in the dependency array.
62-68: Dual-hook activation causes unnecessary blob download for CTR-eligible files.When the dialog opens for a CTR-eligible file,
streaming.isCtris initiallyfalse(resolved asynchronously after metadata fetch), soblobPreviewactivates and starts downloading/decrypting the full file. OnceisCtrresolves totrue, the blob download is cancelled — but the work has already started.Consider gating
blobPreviewon an explicit "streaming check completed" signal (e.g.,!streaming.loadingcombined with!streaming.isCtr) to avoid the redundant fetch:Suggested approach
// Blob URL preview (GCM fallback or when SW not ready) const blobPreview = useFilePreview({ - open: open && (!isStreamingCandidate || !streaming.isSwReady || !streaming.isCtr), + open: open && (!isStreamingCandidate || !streaming.isSwReady || (!streaming.isCtr && !streaming.loading)), item, mimeType, folderKey, });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/components/file-browser/VideoPlayerDialog.tsx` around lines 62 - 68, The blob preview hook is being activated too early because streaming.isCtr is resolved async; update the activation condition passed to useFilePreview so the hook only runs when the dialog is open and either the file is not a streaming candidate or the streaming check has finished and confirmed non-CTR. Concretely, change the boolean expression for the open argument (currently using open && (!isStreamingCandidate || !streaming.isSwReady || !streaming.isCtr)) to require streaming.check completion (use streaming.loading) and only call useFilePreview when open && (!isStreamingCandidate || (!streaming.loading && !streaming.isCtr)), referencing useFilePreview, blobPreview, isStreamingCandidate, streaming.loading and streaming.isCtr.apps/web/src/services/streaming-crypto.service.ts (2)
83-89:importKeyacceptsBufferSourcedirectly — thenew Uint8Array(...).bufferpattern is unnecessary.
crypto.subtle.importKey('raw', ...)acceptsUint8Arraydirectly as aBufferSource. The intermediate copy +.bufferaccess works but is less idiomatic.Simplification
const cryptoKey = await crypto.subtle.importKey( 'raw', - new Uint8Array(fileKey).buffer as ArrayBuffer, + fileKey, { name: AES_CTR_ALGORITHM }, false, ['encrypt'] );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/services/streaming-crypto.service.ts` around lines 83 - 89, The importKey call unnecessarily creates an intermediate ArrayBuffer; update the crypto.subtle.importKey call in streaming-crypto.service.ts to pass a BufferSource directly (e.g., the existing Uint8Array or fileKey if it is already a Uint8Array) instead of new Uint8Array(fileKey).buffer, keeping the same algorithm name AES_CTR_ALGORITHM and usages (encrypt) so the signature of importKey(..., BufferSource, { name: AES_CTR_ALGORITHM }, false, ['encrypt']) is preserved.
74-141:encryptFileCtrruns on the main thread — guideline recommends Web Workers for large file encryption.This function processes files > 256 KB (potentially hundreds of MB). While
crypto.subtle.encryptis async, the chunk combination (lines 118-125) and the overall orchestration happen on the main thread. For very large files this could cause jank.Consider offloading to a Web Worker in a follow-up, consistent with how the existing GCM path may handle large files. As per coding guidelines, "Use Web Workers for large file encryption to avoid blocking the UI thread — never use synchronous crypto operations on large files in the main thread."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/services/streaming-crypto.service.ts` around lines 74 - 141, encryptFileCtr performs CPU- and memory-heavy chunking and final concatenation on the main thread which can cause UI jank for large files; move the heavy work into a Web Worker: create a worker that accepts the File (or ArrayBuffer/ReadableStream), iv, fileKey (or key material), CHUNK_SIZE and performs the per-chunk AES-CTR counter calculation, crypto.subtle.importKey (inside worker), crypto.subtle.encrypt for each chunk and the final concatenation, then posts back the ciphertext and iv; update encryptFileCtr to spawn the worker, send transferable buffers (or stream) to it, await the worker result, then perform wrapKey and clearBytes on the main thread (or wrapKey can also be done in-worker if desirable), and ensure you keep the same outputs (ciphertext Uint8Array, iv hex, wrappedKey hex, sizes, encryptionMode) and handle worker errors/timeouts.apps/web/src/hooks/useStreamingPreview.ts (3)
193-211:handleDownloadfires async work without tracking loading/cancellation state.The download runs as a fire-and-forget promise chain. If the user triggers download, closes the dialog, and the download completes,
setErrorwill be called on an unmounted component (React 18 suppresses the warning but it's still a no-op). More importantly, there's no loading indicator or way to cancel. Consider gatingsetErrorbehind a mounted ref check.As per coding guidelines, "Inside async functions that reference React refs, always re-check for null after each
awaitand never use non-null assertions on refs in async code."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/hooks/useStreamingPreview.ts` around lines 193 - 211, handleDownload currently fires a fire-and-forget promise and may call setError on an unmounted component and has no loading/cancel flow; update the useStreamingPreview hook to track loading and cancellation: add a mounted/ref flag and an isLoading state used by handleDownload, create/propagate an AbortController or cancel token into downloadFileFromIpns, and in the cleanup effect set the mounted flag and call abort; inside the .then/.catch of handleDownload check the mounted ref before calling setError or triggering triggerBrowserDownload, and ensure you reset isLoading in finally only when still mounted; modify calls to downloadFileFromIpns to accept the abort signal so the download can be cancelled.
61-65: SW readiness is only checked whenopenchanges — no re-check if SW activates later.If the SW hasn't claimed the page yet when the dialog first opens,
swReadystaysfalsefor the lifetime of that dialog instance. The user would need to close and re-open the dialog. This is likely acceptable as a known limitation (first page load), but consider documenting it or usingwaitForSW()(from sw-registration.ts) to await readiness with a timeout.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/hooks/useStreamingPreview.ts` around lines 61 - 65, The current useEffect only checks service worker readiness when open changes, so swReady can remain false if the SW activates later; update the effect in useStreamingPreview.ts to, when open is true, call waitForSW() (from sw-registration.ts) with a reasonable timeout and setSwReady(true) if it resolves (or setSwReady(false) on timeout/error), and ensure you cancel/ignore the result if the component unmounts or open becomes false to avoid state updates on an unmounted component; keep references to the existing isSwActive() check for immediate truthy cases and use waitForSW() for waiting behavior.
142-143: Hex-encoded file key string cannot be zeroed from memory.
fileKeyHexis a JS string containing the raw key in hex. Unlike theUint8Arraycleared on Line 143, strings are immutable in JS and persist until GC. This is an inherent limitation of thepostMessagecommunication pattern with the SW, but worth noting since the key material remains in the main thread's heap.Consider whether the SW could accept a
Uint8ArrayviapostMessage(structured clone transfers the buffer) instead of hex, so the main thread could zero it immediately. This would require changes insw-registration.tsanddecrypt-sw.ts.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/hooks/useStreamingPreview.ts` around lines 142 - 143, The code creates a hex string (fileKeyHex) from sensitive bytes (bytesToHex) which cannot be zeroed; instead send the raw Uint8Array (fileKey) to the service worker via postMessage using structured clone/transferable and remove creation of fileKeyHex. Update useStreamingPreview to postMessage the ArrayBuffer/Uint8Array (and pass the buffer as a transferable) then immediately call clearBytes(fileKey); update the SW side handlers in sw-registration.ts and decrypt-sw.ts to accept an ArrayBuffer/Uint8Array payload and perform decryption on that binary buffer; and adjust any related types/signatures to reflect the binary key instead of a hex string so no immutable string copy of the key remains in the main thread.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/web/src/hooks/useStreamingPreview.ts`:
- Around line 100-169: The effect that registers a stream (calls registerStream
with fileMetaIpnsName and fileKeyHex and sets ipnsNameRef.current) only marks
cancelled on cleanup, so if the component unmounts while open=true the service
worker stream is never unregistered; modify the main useEffect cleanup to also
call unregisterStream with the current ipnsNameRef.current (or the same
item.fileMetaIpnsName) and clear ipnsNameRef.current, ensuring you only call
unregisterStream when a stream was registered (e.g., when isCtr or
ipnsNameRef.current is set) and still handle the existing cancelled flag and
state resets the same way.
In `@apps/web/src/main.tsx`:
- Around line 50-53: The subscription to useAuthStore uses a full-state listener
and only calls updateSwToken when state.accessToken is truthy, so on logout
(accessToken -> null) the SW keeps the old token; change the subscription to
listen specifically to accessToken changes and always call updateSwToken with
the new value (including null/undefined) so the service worker is explicitly
cleared on logout; locate useAuthStore.subscribe and replace the full-state
listener with a selector for state => state.accessToken and a handler that calls
updateSwToken(accessToken) unconditionally.
In `@apps/web/src/services/streaming-crypto.service.ts`:
- Around line 60-67: The docstring is incorrect: although CHUNK_SIZE bounds each
crypto.subtle.encrypt call, the implementation accumulates every chunk in
encryptedChunks and then concatenates them (the code around encryptedChunks and
the final combination at lines where the buffer is allocated), so peak memory
grows with file size. Fix by either updating the comment to remove the "~2MB
bounded" claim or change the implementation to stream chunks out instead of
accumulating: replace encryptedChunks with an incremental write/flush (e.g., a
WritableStream, an upload append/callback, or piping a ReadableStream) that
writes each encrypted chunk as produced and avoids the final full-size
concatenation; ensure CHUNK_SIZE remains for per-encrypt calls and remove the
final buffer allocation/concatenation logic.
In `@apps/web/src/workers/decrypt-sw.ts`:
- Around line 315-339: The Range parsing fails to handle suffix ranges like
"bytes=-500"; update the logic that reads request.headers.get('Range') and the
regex (or parsing) used in the block around rangeHeader / start / end /
isRangeRequest so it accepts both "start-end" and "-suffix" forms, set
isRangeRequest=true for suffix matches, compute start = Math.max(0,
ctx.totalSize - suffixLength) and end = ctx.totalSize - 1 for "bytes=-N", keep
the existing clamping (if end >= ctx.totalSize ...) and the 416 Response when
start > end, and ensure the new parsing still supports the original
"bytes=START-" and "bytes=START-END" cases.
In `@packages/crypto/src/__tests__/aes-ctr.test.ts`:
- Around line 71-88: The test "should encrypt and decrypt 1MB data" is timing
out under the default 5s; update the test declaration to supply an explicit,
larger timeout (e.g., pass a third argument like 20000 ms to the it(...) call)
so the heavy work in generateFileKey, generateCtrIv, encryptAesCtr and
decryptAesCtr can complete; do the same for the other large 1MB range test (the
"1MB range" test) to avoid similar CI timeouts.
---
Outside diff comments:
In @.planning/ROADMAP.md:
- Line 57: Update the roadmap to mark Phase 12.6 as completed: change the
checklist item "- [ ] **Phase 12.6: Per-File IPNS Metadata Split**" to checked
("- [x]") and likewise mark the related individual plan checkboxes referenced
around the block for lines 272-276 as checked, and remove or update any
"INSERTED" tag so the single-phase checkbox, the per-file plan checkboxes, and
the progress table entry for Phase 12.6 are consistent with the "Complete |
2026-02-17" status.
In `@apps/web/src/components/file-browser/AudioPlayerDialog.tsx`:
- Around line 395-401: The onKeyDown handler in AudioPlayerDialog (attached to
the audio progress slider) updates audio.currentTime for
'ArrowRight'/'ArrowLeft' but doesn't call e.preventDefault(), allowing the page
to scroll; update the handler (the onKeyDown function that references
audioRef.current and duration) to call e.preventDefault() before adjusting
audio.currentTime for both ArrowRight and ArrowLeft so key presses don't trigger
default scrolling behavior.
---
Nitpick comments:
In `@apps/web/package.json`:
- Line 8: The package.json build pipeline currently runs "tsc -b && vite build
&& node scripts/build-sw.mjs" but misses type-checking the Service Worker;
update the build pipeline to run TypeScript type-check against tsconfig.sw.json
by invoking tsc -p tsconfig.sw.json --noEmit (before the SW transpilation step)
so the Service Worker is type-checked; ensure the addition is ordered so
tsconfig.sw.json is validated prior to running node scripts/build-sw.mjs and
reference the existing scripts/build-sw.mjs, the package.json "build" script,
and tsconfig.sw.json when making this change.
In `@apps/web/scripts/build-sw.mjs`:
- Around line 19-24: The Service Worker bundle defined under the build.lib block
(entry: 'src/workers/decrypt-sw.ts', formats: ['iife'], name: 'DecryptSW')
should emit a sourcemap; enable sourcemaps by adding the appropriate sourcemap
option to the Vite build configuration (e.g., set sourcemap: true on the build
or build.lib config) so the minified SW output produces an accompanying .map
file for easier debugging in DevTools.
In `@apps/web/src/components/file-browser/AudioPlayerDialog.tsx`:
- Around line 67-73: In AudioPlayerDialog the blobPreview useFilePreview call
initializes while streaming.isCtr is still unresolved, causing duplicate
downloads; update the open condition passed to useFilePreview (the blobPreview
declaration) to also require streaming finished loading (e.g., add &&
!streaming.loading) so the preview is only requested after streaming.isCtr is
resolved; reference: blobPreview, useFilePreview, streaming.isCtr,
streaming.loading in AudioPlayerDialog.
In `@apps/web/src/components/file-browser/VideoPlayerDialog.tsx`:
- Around line 282-289: The handleClose useCallback currently depends on the
streaming object which is recreated each render, breaking memoization; update
handleClose to depend on a stable cleanup reference instead (e.g., capture
streaming.cleanup into a const like streamingCleanup or reference
streaming.cleanup directly) so the dependency array uses streamingCleanup (or
streaming.cleanup) rather than streaming; keep existing behavior: call
videoRef.current.pause(), setIsPlaying(false), streamingCleanup(), and onClose()
inside handleClose and include onClose and streamingCleanup in the dependency
array.
- Around line 62-68: The blob preview hook is being activated too early because
streaming.isCtr is resolved async; update the activation condition passed to
useFilePreview so the hook only runs when the dialog is open and either the file
is not a streaming candidate or the streaming check has finished and confirmed
non-CTR. Concretely, change the boolean expression for the open argument
(currently using open && (!isStreamingCandidate || !streaming.isSwReady ||
!streaming.isCtr)) to require streaming.check completion (use streaming.loading)
and only call useFilePreview when open && (!isStreamingCandidate ||
(!streaming.loading && !streaming.isCtr)), referencing useFilePreview,
blobPreview, isStreamingCandidate, streaming.loading and streaming.isCtr.
In `@apps/web/src/hooks/useStreamingPreview.ts`:
- Around line 193-211: handleDownload currently fires a fire-and-forget promise
and may call setError on an unmounted component and has no loading/cancel flow;
update the useStreamingPreview hook to track loading and cancellation: add a
mounted/ref flag and an isLoading state used by handleDownload, create/propagate
an AbortController or cancel token into downloadFileFromIpns, and in the cleanup
effect set the mounted flag and call abort; inside the .then/.catch of
handleDownload check the mounted ref before calling setError or triggering
triggerBrowserDownload, and ensure you reset isLoading in finally only when
still mounted; modify calls to downloadFileFromIpns to accept the abort signal
so the download can be cancelled.
- Around line 61-65: The current useEffect only checks service worker readiness
when open changes, so swReady can remain false if the SW activates later; update
the effect in useStreamingPreview.ts to, when open is true, call waitForSW()
(from sw-registration.ts) with a reasonable timeout and setSwReady(true) if it
resolves (or setSwReady(false) on timeout/error), and ensure you cancel/ignore
the result if the component unmounts or open becomes false to avoid state
updates on an unmounted component; keep references to the existing isSwActive()
check for immediate truthy cases and use waitForSW() for waiting behavior.
- Around line 142-143: The code creates a hex string (fileKeyHex) from sensitive
bytes (bytesToHex) which cannot be zeroed; instead send the raw Uint8Array
(fileKey) to the service worker via postMessage using structured
clone/transferable and remove creation of fileKeyHex. Update useStreamingPreview
to postMessage the ArrayBuffer/Uint8Array (and pass the buffer as a
transferable) then immediately call clearBytes(fileKey); update the SW side
handlers in sw-registration.ts and decrypt-sw.ts to accept an
ArrayBuffer/Uint8Array payload and perform decryption on that binary buffer; and
adjust any related types/signatures to reflect the binary key instead of a hex
string so no immutable string copy of the key remains in the main thread.
In `@apps/web/src/services/streaming-crypto.service.ts`:
- Around line 83-89: The importKey call unnecessarily creates an intermediate
ArrayBuffer; update the crypto.subtle.importKey call in
streaming-crypto.service.ts to pass a BufferSource directly (e.g., the existing
Uint8Array or fileKey if it is already a Uint8Array) instead of new
Uint8Array(fileKey).buffer, keeping the same algorithm name AES_CTR_ALGORITHM
and usages (encrypt) so the signature of importKey(..., BufferSource, { name:
AES_CTR_ALGORITHM }, false, ['encrypt']) is preserved.
- Around line 74-141: encryptFileCtr performs CPU- and memory-heavy chunking and
final concatenation on the main thread which can cause UI jank for large files;
move the heavy work into a Web Worker: create a worker that accepts the File (or
ArrayBuffer/ReadableStream), iv, fileKey (or key material), CHUNK_SIZE and
performs the per-chunk AES-CTR counter calculation, crypto.subtle.importKey
(inside worker), crypto.subtle.encrypt for each chunk and the final
concatenation, then posts back the ciphertext and iv; update encryptFileCtr to
spawn the worker, send transferable buffers (or stream) to it, await the worker
result, then perform wrapKey and clearBytes on the main thread (or wrapKey can
also be done in-worker if desirable), and ensure you keep the same outputs
(ciphertext Uint8Array, iv hex, wrappedKey hex, sizes, encryptionMode) and
handle worker errors/timeouts.
In `@apps/web/src/workers/decrypt-sw.ts`:
- Around line 17-23: The StreamContext currently stores raw AES key material in
the service worker (fileKey as hex) which creates a trust boundary; update the
comment near the StreamContext type in decrypt-sw.ts to explicitly document that
fileKey contains hex-encoded AES-256 keys held in SW memory only for the
lifetime of a registered stream, that keys must be cleared when playback ends,
and reference the unregister-stream message/handler to ensure streams remove
keys from SW memory; mention the security invariant that the SW is trusted for
active streams and keys must not be persisted beyond unregister-stream.
- Around line 240-313: The fetch to the IPFS gateway (where fetchUrl is used to
populate encrypted via encryptedCache.get(cacheKey)) has no timeout; add an
AbortController/AbortSignal.timeout(60000) (or similar) and pass its signal to
fetch(fetchUrl, { headers: fetchHeaders, signal }) so the request is aborted
after ~60s; catch the abort error in the existing try/catch and return a
meaningful Response (e.g., 504 Gateway Timeout) and call postToClients with a
fetch-error/fetch-timeout payload (include fileMetaIpnsName: cacheKey and the
error) so the UI can recover. Ensure you clean up any controller if needed and
keep the other response-status handling (401/response.ok) unchanged.
- Around line 55-65: The encryptedCache Map currently stores full files with no
eviction, risking service-worker memory exhaustion; implement a capped LRU cache
wrapper around encryptedCache (e.g., replace the Map with an LRU implementation
or create helper functions cacheGet/cacheSet that enforce a maxEntries limit and
evict least-recently-used items) and update all uses to call cacheGet/cacheSet
instead of encryptedCache.get/encryptedCache.set; ensure the eviction interacts
correctly with stream lifecycle (streamRegistry and the "unregister-stream"
message) so entries removed by LRU are cleaned up or ignored by StreamContext
consumers.
Tauri/FUSE-T client only implements GCM decryption. CTR-encrypted media files will fail on desktop mount. Noted for Phase 11 or future insertion. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (2)
.planning/phases/12.1-aes-ctr-streaming-encryption/12.1-VERIFICATION.md (2)
146-150: Consider documenting the CTR authentication trade-off in the verification report.The Anti-Patterns section only searches for code markers (TODO/FIXME) but doesn't document significant design trade-offs. While not an "anti-pattern" per se, the decision to use CTR without authentication for media files (vs. authenticated GCM for other files) represents a notable architectural divergence that impacts security properties. Consider adding a note acknowledging this conscious trade-off for transparency in the verification record.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.planning/phases/12.1-aes-ctr-streaming-encryption/12.1-VERIFICATION.md around lines 146 - 150, The report omits documenting the deliberate CTR-without-authentication choice; update .planning/phases/12.1-aes-ctr-streaming-encryption/12.1-VERIFICATION.md by adding a short subsection (e.g., "Design trade-offs: CTR vs GCM" under or after the "Anti-Patterns Found" table) that states the system uses CTR without authentication for media files while using authenticated GCM for other files, explains the rationale, lists the security impact (loss of integrity/authentication for CTR-encrypted media), and notes any mitigations or accepted risks so the decision is explicit in the verification record.
183-187: Expand Known Gaps to include CTR authentication limitation.The Known Gaps section only mentions desktop FUSE compatibility but omits the fundamental security trade-off: AES-256-CTR provides no authentication or integrity checking, unlike AES-256-GCM's AEAD properties. This should be documented as a known limitation:
Gap Severity Details CTR-encrypted media lacks tamper detection/integrity Low-Med AES-256-CTR provides only confidentiality, not authentication. Bit flips in ciphertext will cause corresponding plaintext corruption without detection. Acceptable for media streaming use case per threat model. This ensures stakeholders understand that CTR-encrypted files cannot detect tampering or corruption, which is a deliberate trade-off for byte-range streaming capability.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.planning/phases/12.1-aes-ctr-streaming-encryption/12.1-VERIFICATION.md around lines 183 - 187, Add a new Known Gaps table row titled "CTR-encrypted media lacks tamper detection/integrity" (severity: Low-Med) immediately alongside the existing "Desktop FUSE client has no CTR decryption support" entry and use the details: "AES-256-CTR provides only confidentiality, not authentication. Bit flips in ciphertext will cause corresponding plaintext corruption without detection. Acceptable for media streaming use case per threat model." Ensure the new row matches the table formatting and placement in the Known Gaps section so readers see the CTR authentication limitation alongside the FUSE/desktop note.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.planning/phases/12.1-aes-ctr-streaming-encryption/12.1-VERIFICATION.md:
- Around line 146-150: The report omits documenting the deliberate
CTR-without-authentication choice; update
.planning/phases/12.1-aes-ctr-streaming-encryption/12.1-VERIFICATION.md by
adding a short subsection (e.g., "Design trade-offs: CTR vs GCM" under or after
the "Anti-Patterns Found" table) that states the system uses CTR without
authentication for media files while using authenticated GCM for other files,
explains the rationale, lists the security impact (loss of
integrity/authentication for CTR-encrypted media), and notes any mitigations or
accepted risks so the decision is explicit in the verification record.
- Around line 183-187: Add a new Known Gaps table row titled "CTR-encrypted
media lacks tamper detection/integrity" (severity: Low-Med) immediately
alongside the existing "Desktop FUSE client has no CTR decryption support" entry
and use the details: "AES-256-CTR provides only confidentiality, not
authentication. Bit flips in ciphertext will cause corresponding plaintext
corruption without detection. Acceptable for media streaming use case per threat
model." Ensure the new row matches the table formatting and placement in the
Known Gaps section so readers see the CTR authentication limitation alongside
the FUSE/desktop note.
- Fix CI: add 30s timeout to 1MB AES-CTR tests that exceeded 5s default - Fix stream leak: unregister SW stream on unmount in useStreamingPreview - Fix stale token: clear SW auth token on logout (was retained indefinitely) - Fix dual-hook: gate blob preview on !streaming.loading to avoid redundant fetch - Fix useCallback deps: use stable streaming.cleanup ref instead of object - Fix a11y: add e.preventDefault() to AudioPlayerDialog arrow key handler - Add SW robustness: LRU cache cap (5), 60s fetch timeout, suffix range parsing - Add SW type-check to build pipeline (tsc -p tsconfig.sw.json --noEmit) - Fix doc: correct misleading ~2MB bounded memory claim in streaming-crypto - Fix ROADMAP: mark Phase 12.6 checkboxes as complete (matched progress table) - Update verification: document CTR vs GCM trade-off and tamper detection gap Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Owner
Author
CodeRabbit Review — All Items AddressedCommit Outside Diff Range (2) — Both Fixed
Nitpick Comments — Addressed
All tests pass (223 crypto, 22 web). Build succeeds with new SW type-check. |
5 tasks
Merged
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
1 participant
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
/decrypt-stream/*URLs, returning proper 200/206 responses with Content-Range headersDetails
4 plans across 3 waves:
packages/crypto/src/aes/encrypt-ctr.ts,decrypt-ctr.tsapps/web/src/services/streaming-crypto.service.ts,file-crypto.service.tsapps/web/src/workers/decrypt-sw.ts,lib/sw-registration.tsapps/web/src/hooks/useStreamingPreview.ts,VideoPlayerDialog.tsx,AudioPlayerDialog.tsxVerification: 5/5 must-haves verified against codebase. 223 tests pass (35 new CTR tests). Web app builds cleanly.
Test plan
encryptionMode: 'CTR')pnpm test— 223 tests pass including 35 new CTR tests🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests