feat(sdk): select AES-CTR encryption for streaming media uploads#399
Merged
Conversation
E2E fixes: - batch-download: Extend beforeAll hook timeout from 30s default to 120s — wallet login exceeds 30s in CI. Remove retry loop (was masking the timeout issue). Use clickClear() instead of Escape to dismiss selection bar after download. - streaming-playback: Extend beforeAll hook timeout to 120s for the same reason. CI optimization: - Narrow the `desktop` change filter in ci.yml to only Rust-relevant paths (src-tauri/src, vendor, crates, Cargo.toml/lock, vectors). Removes TypeScript-only paths (desktop frontend, capabilities, resources, vite config) that don't affect cargo check/test. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 123fec3c406d
- Match beforeAll hook timeout to suite timeout (180s) in streaming-playback - Rename CTR badge test to reflect conditional behavior; skip with explicit message when streaming pipeline isn't active instead of silently passing - Re-add .github/workflows/ci.yml to desktop filter so CI workflow changes trigger cargo checks Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Entire-Checkpoint: cdba1752d779
The decrypt SW was registered from /src/workers/decrypt-sw.ts with scope '/', but browsers reject this without a Service-Worker-Allowed header. Vite 7's dev server doesn't reliably send custom headers on transformed file responses. Fix: add public/decrypt-sw-dev.js wrapper that loads the real SW via importScripts(). Files in public/ are served from the root scope, so the browser allows scope: '/' without the header. Also skip the CTR badge E2E test — SDK uploadFile() hardcodes GCM encryption, so CTR mode is never triggered for new uploads. Tracked as a TODO in .planning/TODO.md. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 682dcac6efa4
…on for media uploads Entire-Checkpoint: 3327af6c8cb5
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 5a4b0c7e875c
Switch decrypt-sw-dev.js from importScripts() (classic-only) to ES module import, and register with type: 'module' in dev mode. This is more robust — importScripts() can't load ES modules if the SW source ever gains import statements. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 53a15e4b4d21
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 3cc55a32ce04
SDK uploadFile() hardcoded AES-256-GCM for all files, preventing the CTR streaming playback pipeline from activating. Now the SDK client auto-selects CTR mode for media files >256KB (video/mp4, audio/*, etc.), matching the web app's selectEncryptionMode() logic. Also fixes pre-existing type mismatches in sdk-core test mocks and un-skips the CTR badge E2E test. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Entire-Checkpoint: bd28f7731561
WalkthroughThe pull request adds support for AES-CTR streaming encryption mode selection in SDK file uploads. It introduces encryption mode detection based on file type and size, wires this mode parameter through the upload pipeline, and re-enables a corresponding E2E test. Test mocks are updated to reflect API response shape changes. Changes
Sequence DiagramsequenceDiagram
actor User
participant Client as Client.uploadFile()
participant ModeSelector as selectEncryptionMode()
participant SDKCore as sdkCore.uploadFile()
participant Crypto as Encryption Pipeline
User->>Client: Upload media file (type, size)
Client->>ModeSelector: selectEncryptionMode(mimeType, fileSize)
ModeSelector-->>Client: 'CTR' or 'GCM' based on type & size
Client->>SDKCore: uploadFile({..., encryptionMode})
SDKCore->>Crypto: Select IV: generateCtrIv() or generateIv()
SDKCore->>Crypto: Select cipher: encryptAesCtr() or encryptAesGcm()
Crypto->>Crypto: Encrypt file data
SDKCore->>SDKCore: createFileMetadata({encryptionMode})
SDKCore-->>Client: UploadResult
Client-->>User: Upload complete
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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 docstrings
🧪 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 |
Add audio/flac to STREAMING_MIME_TYPES in both the SDK client and web app's streaming-crypto service so FLAC files >256KB use AES-CTR for random-access streaming decryption. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 4f6bef90d98a
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #399 +/- ##
==========================================
+ Coverage 61.14% 61.27% +0.12%
==========================================
Files 132 133 +1
Lines 9758 9781 +23
Branches 980 985 +5
==========================================
+ Hits 5967 5993 +26
+ Misses 3577 3574 -3
Partials 214 214
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
Pull request overview
Enables the SDK upload pipeline to choose AES-256-CTR for eligible large media files so the web streaming playback pipeline (range-request decryption + CTR badge) can activate for new uploads, while also aligning sdk-core test mocks with current API/client types.
Changes:
- Add MIME-type/size-based encryption mode selection in
packages/sdkand passencryptionModethrough tosdk-core. - Update
sdk-coreupload to encrypt with AES-CTR (and CTR IV) when requested, and persist the selectedencryptionModeinto file metadata. - Fix sdk-core unit test mocks to match current DTO/result shapes; un-skip the CTR badge E2E test.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/web-e2e/tests/streaming-playback.spec.ts | Un-skips CTR badge test now that CTR uploads are enabled. |
| packages/sdk/src/client.ts | Selects encryption mode at upload time and forwards it to sdk-core. |
| packages/sdk-core/src/upload/index.ts | Adds encryptionMode param and switches between AES-GCM and AES-CTR encryption/IV generation. |
| packages/sdk-core/src/tests/vault.test.ts | Updates addToIpfs mock to match IpfsAddResult shape. |
| packages/sdk-core/src/tests/ipns.test.ts | Updates IPNS mock responses to match current DTO shapes. |
| apps/web/src/services/streaming-crypto.service.ts | Expands streaming-eligible MIME types to include audio/flac. |
| .planning/todos/done/2026-03-29-sdk-uploadfile-hardcodes-gcm-wire-ctr-encryption-for-media-uploads.md | Documents the resolved issue and implementation notes. |
Owner
Author
|
@coderabbitai please review |
|
🧠 Learnings used✅ Actions performedReview triggered.
|
Move STREAMING_MIME_TYPES, CTR_SIZE_THRESHOLD, and selectEncryptionMode to @cipherbox/sdk-core/encryption-mode so the SDK client and web app share a single source of truth. Add normalizeEncryptionMode to validate the encryptionMode param in uploadFile, coercing invalid values to GCM. Addresses PR review feedback on duplication risk and input validation. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Entire-Checkpoint: cb7f6e5552c0
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/sdk/src/client.ts (1)
1249-1276: Consider centralizingSTREAMING_MIME_TYPESto avoid drift.The SDK now maintains its own copy of
STREAMING_MIME_TYPESandCTR_SIZE_THRESHOLD, which must stay in sync with the web app'sstreaming-crypto.service.ts. While duplication is reasonable for keeping the SDK browser-agnostic, consider extracting these constants to a shared package (e.g.,@cipherbox/coreor@cipherbox/crypto) to ensure consistency and prevent accidental divergence when adding new MIME types.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/sdk/src/client.ts` around lines 1249 - 1276, The duplicated constants STREAMING_MIME_TYPES and CTR_SIZE_THRESHOLD in selectEncryptionMode should be centralized to avoid drift; refactor by moving these constants into a shared module (e.g., a new export in `@cipherbox/core` or `@cipherbox/crypto`) and import them into packages/sdk/src/client.ts, replacing the local STREAMING_MIME_TYPES and CTR_SIZE_THRESHOLD definitions and keeping selectEncryptionMode(mimeType, size) unchanged; update imports and remove the local constants so both the SDK and web app consume the same source of truth.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/sdk/src/client.ts`:
- Around line 1249-1276: The duplicated constants STREAMING_MIME_TYPES and
CTR_SIZE_THRESHOLD in selectEncryptionMode should be centralized to avoid drift;
refactor by moving these constants into a shared module (e.g., a new export in
`@cipherbox/core` or `@cipherbox/crypto`) and import them into
packages/sdk/src/client.ts, replacing the local STREAMING_MIME_TYPES and
CTR_SIZE_THRESHOLD definitions and keeping selectEncryptionMode(mimeType, size)
unchanged; update imports and remove the local constants so both the SDK and web
app consume the same source of truth.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 13d726ee-9d94-4fe9-ac46-56c68ca3aea6
📒 Files selected for processing (7)
.planning/todos/done/2026-03-29-sdk-uploadfile-hardcodes-gcm-wire-ctr-encryption-for-media-uploads.mdapps/web/src/services/streaming-crypto.service.tspackages/sdk-core/src/__tests__/ipns.test.tspackages/sdk-core/src/__tests__/vault.test.tspackages/sdk-core/src/upload/index.tspackages/sdk/src/client.tstests/web-e2e/tests/streaming-playback.spec.ts
💤 Files with no reviewable changes (1)
- packages/sdk-core/src/tests/ipns.test.ts
… exports The sdk-core mock in 5 test files used a bare factory that replaced the entire module, dropping non-mocked exports like selectEncryptionMode. Switch to importOriginal + spread so real exports survive alongside mocked functions. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 4686e944596d
…alization Cover selectEncryptionMode (MIME type allowlist, size threshold boundary) and normalizeEncryptionMode (CTR passthrough, GCM default, invalid input). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 4f9f60832913
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
uploadFile()now auto-selects AES-256-CTR encryption for media files >256KB (video/mp4, video/webm, audio/mpeg, audio/mp4, audio/flac, etc.), enabling the streaming playback pipeline (Service Worker interception, range-request decryption, encrypted badge)@cipherbox/sdk-core/encryption-mode.tswith input validationTest plan
🤖 Generated with Claude Code