feat: end-to-end encryption for tab session data - #98
Conversation
Implement ECDH-based key exchange and AES-GCM encryption for WebSocket messages between host and guest. Session join/leave and WebRTC signaling remain unencrypted for compatibility. Public keys are exchanged via URL fragments using base64-encoded JWK format. - Add encryption utility module with ECDH key generation and AES-GCM - Generate unique IV for each message - Comprehensive unit tests covering key derivation, encryption/decryption, and round-trip encryption scenarios - Integrate encryption into useSession hook for automatic message wrapping - Only encrypt data events (cursor:move, action:request, crdt:update) Signed-off-by: anshul23102@iiitd.ac.in
🛠️ PR Needs UpdatesHey @anshul23102! 👋 A few things need fixing before a mentor can review this PR. Warning
How to fix:
Once fixed, the workflow re-runs automatically and pings the right mentor. 🤖 TabTwin Automation · Updates automatically on edits |
📝 WalkthroughWalkthroughAdds Web Crypto ECDH/AES-GCM utilities, integrates key exchange and encrypted WebSocket messaging into ChangesSession encryption
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Host
participant Joiner
participant useSession
participant WebSocket
participant encryption.js
Host->>useSession: generate key pair
useSession->>Host: publish public key in URL fragment
Joiner->>useSession: read host public key
useSession->>encryption.js: derive shared AES key
useSession->>encryption.js: encrypt selected event
useSession->>WebSocket: send encrypted event
WebSocket-->>useSession: receive event
useSession->>encryption.js: decrypt encrypted event
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Checkov (3.3.8)webapp/package.jsonTraceback (most recent call last): 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: 7
🧹 Nitpick comments (2)
webapp/package.json (1)
24-25: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueVitest pinned to
^1.0.0is several majors behind current.Latest version: 4.1.10, last published: 16 days ago.
^1.0.0won't auto-upgrade past 1.x, and later majors add features/fixes used by many modern setups; worth checking whether this pin was intentional or should track a newer major.🤖 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 `@webapp/package.json` around lines 24 - 25, Update the vitest dependency in webapp/package.json from the outdated ^1.0.0 range to the intended current supported major, such as ^4.1.10, unless the project deliberately requires Vitest 1.x. Verify the existing test configuration and compatibility before selecting the target version.webapp/src/utils/encryption.js (1)
55-75: 🔒 Security & Privacy | 🔵 TrivialECDH shared secret used directly as AES key material without a KDF pass.
This mirrors the common Web Crypto ECDH+AES-GCM example pattern directly (deriveBits → raw import), which is acceptable, but for stronger domain separation consider passing the shared secret through HKDF before use as an AES key. Not a blocker given this matches documented practice.
🤖 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 `@webapp/src/utils/encryption.js` around lines 55 - 75, The ECDH output in deriveSharedSecret is imported directly by deriveAESKey without a KDF. Add an HKDF-based derivation step between these functions, using an explicit application-specific salt and info value, then import the resulting key material as the AES-GCM key while preserving the existing encrypt/decrypt usages.
🤖 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 `@webapp/src/hooks/useSession.js`:
- Around line 157-174: Update send so events requiring encryption are not sent
while aesKeyRef.current is unavailable: queue them until the key is derived, or
explicitly drop/block them. Ensure queued events are later processed through
wrapEncryptedMessage, while preserving plaintext sending only for events that do
not require encryption.
- Around line 66-97: Fix initializeEncryption so it reads the existing pk
fragment before publishing this participant’s key, publishes only when no host
key is present, and derives the guest key from the host key. Extend the existing
session:join/session:joined signaling flow to send the guest public key to the
host and derive the host AES key when it arrives. Do not silently continue
unencrypted when key import or derivation fails; fail the session or clearly
surface the encryption error.
In `@webapp/src/utils/__tests__/encryption.test.js`:
- Around line 1-28: The encryption utility tests currently run without an
explicit browser environment, despite relying on browser globals such as
crypto.subtle, btoa, and atob. Update the Vitest configuration or test command
associated with the Encryption Module tests so they execute in a browser
environment, while preserving the existing test setup and assertions.
- Around line 139-146: Update the invalid-IV test around decryptMessage to
mutate the encrypted message’s existing IV by flipping a byte while preserving
its valid base64 encoding and original length. Replace the URI-encoded
zero-filled IV assignment, so decryption reaches AES-GCM authentication failure
rather than failing during base64 decoding.
In `@webapp/src/utils/encryption.js`:
- Line 4: Update ENCRYPTION_ENABLED and the related
isEncryptionEnabled()/shouldEncryptEvent() logic to default encryption to
enabled while allowing a developer-controlled environment override to disable it
for local testing. Ensure the override is parsed consistently and that existing
callers use the resulting configurable value.
- Line 4: Update the public-key URL parameter handling in
encodeJWKToBase64/decodeBase64ToJWK and the fragment construction so Base64
values cannot have literal '+' interpreted as spaces by URLSearchParams; use the
existing URL-safe Base64 approach (or append the parameter via
URLSearchParams.append) and preserve correct key round-tripping and decoding.
- Around line 82-119: Replace the spread-based byte-to-string conversions in
encryptMessage with chunked conversion that avoids passing the entire Uint8Array
to String.fromCharCode, and use the same safe decoding approach for the base64
inputs in decryptMessage. Preserve the existing IV and ciphertext encoding
formats and JSON encryption/decryption behavior while ensuring large tab-content
snapshots do not exceed argument or stack limits.
---
Nitpick comments:
In `@webapp/package.json`:
- Around line 24-25: Update the vitest dependency in webapp/package.json from
the outdated ^1.0.0 range to the intended current supported major, such as
^4.1.10, unless the project deliberately requires Vitest 1.x. Verify the
existing test configuration and compatibility before selecting the target
version.
In `@webapp/src/utils/encryption.js`:
- Around line 55-75: The ECDH output in deriveSharedSecret is imported directly
by deriveAESKey without a KDF. Add an HKDF-based derivation step between these
functions, using an explicit application-specific salt and info value, then
import the resulting key material as the AES-GCM key while preserving the
existing encrypt/decrypt usages.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a2ad3eda-6cd9-486e-ad18-aa9632100f50
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (4)
webapp/package.jsonwebapp/src/hooks/useSession.jswebapp/src/utils/__tests__/encryption.test.jswebapp/src/utils/encryption.js
| useEffect(() => { | ||
| async function initializeEncryption() { | ||
| try { | ||
| // Generate this participant's key pair | ||
| const keyPair = await generateHostKeyPair(); | ||
| keyPairRef.current = keyPair; | ||
|
|
||
| // Export and publish public key in URL fragment | ||
| const jwkString = await exportPublicKeyToJWK(keyPair.publicKey); | ||
| const base64PK = encodeJWKToBase64(jwkString); | ||
| window.location.hash = `pk=${base64PK}`; | ||
|
|
||
| // Try to import host's public key from URL if this is a guest session | ||
| const hashParams = new URLSearchParams(window.location.hash.slice(1)); | ||
| const hostPKBase64 = hashParams.get('pk'); | ||
|
|
||
| if (hostPKBase64) { | ||
| const hostJWK = decodeBase64ToJWK(hostPKBase64); | ||
| hostPublicKeyRef.current = await importPublicKeyFromJWK(hostJWK); | ||
|
|
||
| // Derive shared secret and AES key | ||
| const sharedSecret = await deriveSharedSecret(hostPublicKeyRef.current, keyPair.privateKey); | ||
| aesKeyRef.current = await deriveAESKey(sharedSecret); | ||
| } | ||
| } catch (err) { | ||
| console.error('Encryption initialization failed:', err); | ||
| } | ||
| } | ||
|
|
||
| initializeEncryption(); | ||
| }, []); | ||
|
|
There was a problem hiding this comment.
🔒 Security & Privacy | 🔴 Critical | 🏗️ Heavy lift
Key-exchange sequencing bug: every participant overwrites and then reads back their own public key, so a real shared secret is never established.
Lines 74-76 write this participant's own public key into window.location.hash before line 79-80 reads window.location.hash back to look for the "host" key. This means hashParams.get('pk') will always resolve to the value that was just written by this very call — i.e., the participant's own key — never a genuinely different host key. Consequently:
hostPublicKeyRef.currentends up being this participant's own public key re-imported from JWK.deriveSharedSecret(ownPublicKey, ownPrivateKey)computes a self-ECDH value derived solely from this participant's own keys, which differs between host and guest.aesKeyRef.currentis therefore always set (masking the failure), but the host's and guest's AES keys never match, so every encryptedcursor:move/action:request/crdt:updatemessage will fail to decrypt on the other end (caught in themessagelistener and silently dropped).
This isn't just an edge case — since the write always precedes the read, this happens on every session, for every participant. Additionally, even once the ordering is fixed, there's no visible mechanism for the guest's public key to reach the host (the unencrypted session:join payload only carries { sessionId, name }), so only one direction of the exchange can work even after reordering.
At minimum:
- Read the fragment's
pkparam before writing this participant's own key. - Only publish this participant's own key to the hash when acting as host (i.e., no
pkwas already present). - Propagate the guest's public key back to the host via the existing unencrypted signaling channel (e.g., include it in the
session:joinpayload and echo it insession:joined), then have the host derive its AES key upon receiving it.
async function initializeEncryption() {
try {
const keyPair = await generateHostKeyPair();
keyPairRef.current = keyPair;
const hashParams = new URLSearchParams(window.location.hash.slice(1));
const hostPKBase64 = hashParams.get('pk');
if (hostPKBase64) {
// Guest: import host's key, derive shared secret. Guest's own public key
// still needs to reach the host (e.g. via session:join payload).
const hostJWK = decodeBase64ToJWK(hostPKBase64);
hostPublicKeyRef.current = await importPublicKeyFromJWK(hostJWK);
const sharedSecret = await deriveSharedSecret(hostPublicKeyRef.current, keyPair.privateKey);
aesKeyRef.current = await deriveAESKey(sharedSecret);
} else {
// Host: publish own public key for guests to import.
const jwkString = await exportPublicKeyToJWK(keyPair.publicKey);
window.location.hash = `pk=${encodeJWKToBase64(jwkString)}`;
}
} catch (err) {
console.error('Encryption initialization failed:', err);
}
}Separately, the broad catch here swallows any failure — including a rejected importKey/deriveBits caused by a tampered public key — and silently leaves the app running unencrypted (per the PR's "graceful fallback" design). That directly conflicts with the linked issue's requirement to protect "against establishing a valid session with a tampered public key": an attacker who tampers with the pk fragment can force a downgrade to plaintext without any visible signal to the user. Consider failing the session (or clearly surfacing a warning) when key import/derivation fails, rather than transparently continuing unencrypted.
🤖 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 `@webapp/src/hooks/useSession.js` around lines 66 - 97, Fix
initializeEncryption so it reads the existing pk fragment before publishing this
participant’s key, publishes only when no host key is present, and derives the
guest key from the host key. Extend the existing session:join/session:joined
signaling flow to send the guest public key to the host and derive the host AES
key when it arrives. Do not silently continue unencrypted when key import or
derivation fails; fail the session or clearly surface the encryption error.
| async function send(event, payload = {}) { | ||
| const socket = socketRef.current; | ||
| if (!socket || socket.readyState !== WebSocket.OPEN) return; | ||
| socket.send(JSON.stringify({ event, payload: { sessionId, ...payload } })); | ||
|
|
||
| let message = { event, payload: { sessionId, ...payload } }; | ||
|
|
||
| // Encrypt message if it should be encrypted and we have an encryption key | ||
| if (shouldEncryptEvent(event) && aesKeyRef.current) { | ||
| try { | ||
| message = await wrapEncryptedMessage(message, aesKeyRef.current); | ||
| } catch (err) { | ||
| console.error('Message encryption failed:', err); | ||
| return; | ||
| } | ||
| } | ||
|
|
||
| socket.send(JSON.stringify(message)); | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Outgoing messages silently fall back to plaintext while the AES key hasn't been derived yet.
aesKeyRef.current is populated asynchronously by a separate effect. If sendCursorMove/requestAction/addAnnotation fire before that completes, shouldEncryptEvent(event) && aesKeyRef.current is falsy and the message is sent unencrypted instead of being queued or blocked — silently defeating the E2E guarantee for early events (a real possibility since cursor moves can start immediately after connect, well before an async keypair generation + JWK export + hash round trip completes).
Consider buffering encryption-required events until aesKeyRef.current is available (or dropping/queueing them) instead of falling through to a plaintext send.
🤖 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 `@webapp/src/hooks/useSession.js` around lines 157 - 174, Update send so events
requiring encryption are not sent while aesKeyRef.current is unavailable: queue
them until the key is derived, or explicitly drop/block them. Ensure queued
events are later processed through wrapEncryptedMessage, while preserving
plaintext sending only for events that do not require encryption.
| import { describe, it, expect, beforeEach } from 'vitest'; | ||
| import { | ||
| generateHostKeyPair, | ||
| exportPublicKeyToJWK, | ||
| importPublicKeyFromJWK, | ||
| encodeJWKToBase64, | ||
| decodeBase64ToJWK, | ||
| deriveSharedSecret, | ||
| deriveAESKey, | ||
| generateIV, | ||
| encryptMessage, | ||
| decryptMessage, | ||
| wrapEncryptedMessage, | ||
| unwrapEncryptedMessage, | ||
| isEncryptionEnabled, | ||
| shouldEncryptEvent | ||
| } from '../encryption.js'; | ||
|
|
||
| describe('Encryption Module', () => { | ||
| let hostKeyPair; | ||
| let joinerKeyPair; | ||
| let sharedSecret; | ||
| let aesKey; | ||
|
|
||
| beforeEach(async () => { | ||
| hostKeyPair = await generateHostKeyPair(); | ||
| joinerKeyPair = await generateHostKeyPair(); | ||
| }); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check declared Node engine / CI Node version and vitest config for an explicit environment.
fd -e json -e yml -e yaml . webapp --exec grep -l "engines\|node-version" {} \;
cat webapp/vitest.config.* 2>/dev/null
fd 'vite.config' webappRepository: itzzavdhesh/TabTwin
Length of output: 179
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== package files =="
fd 'package\.json|package-lock\.json|pnpm-lock\.yaml|yarn\.lock|bun\.lockb' . -d 3 | sed -n '1,50p'
echo
echo "== webapp package manifests =="
fd 'package\.json' webapp -d 3 --exec sh -c 'echo "--- {}: "; cat "$1" | sed -n "1,220p"' sh {}
echo
echo "== crypto usages =="
rg -n "crypto\.subtle|btoa|atob|SubtleCrypto|generateKeyPair|deriveBits|SubtleCrypto" webapp/src webapp/cypress e2e tests 2>/dev/null || true
echo
echo "== vite vs vitest configs =="
fd 'vite\.config|vitest\.config|vite|vitest' webapp . -d 4 -t f | sort | sed -n '1,200p'
for f in $(fd 'vite\.config|vitest\.config' webapp . -d 4 -t f); do
echo "--- $f"; sed -n '1,220p' "$f" || true
done
echo
echo "== ci/workflow files with node/env/env setup =="
fd '\.(yml|yaml)$' .github 2>/dev/null | while read -r f; do
echo "--- $f"; sed -n '1,220p' "$f" | rg -n "Node|node-version|setup-node|node|npm|pnpm|yarn|test|vitest|environment|browser|webkit|jsdom" -C 2 || true
doneRepository: itzzavdhesh/TabTwin
Length of output: 5727
Use a browser test environment for encryption.js utilities.
webapp/package.json runs vitest without a platform override, and vite.config.js only configures the Vite server. Since encryption.js uses browser globals (crypto.subtle, btoa, atob), this can fail in the default Node environment unless the test runner is explicitly configured with a browser environment such as vitest browser.
🤖 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 `@webapp/src/utils/__tests__/encryption.test.js` around lines 1 - 28, The
encryption utility tests currently run without an explicit browser environment,
despite relying on browser globals such as crypto.subtle, btoa, and atob. Update
the Vitest configuration or test command associated with the Encryption Module
tests so they execute in a browser environment, while preserving the existing
test setup and assertions.
| it('should fail decryption with invalid IV', async () => { | ||
| const message = { event: 'test', data: 'secret' }; | ||
| const encrypted = await encryptMessage(message, aesKey); | ||
|
|
||
| encrypted.iv = encodeURIComponent(Buffer.alloc(12).toString('base64')); | ||
|
|
||
| await expect(decryptMessage(encrypted, aesKey)).rejects.toThrow(); | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
"invalid IV" test doesn't actually exercise an invalid IV / GCM auth failure.
encodeURIComponent(Buffer.alloc(12).toString('base64')) wraps a base64 string in URI-percent-encoding, which introduces % characters that aren't valid base64. decryptMessage then fails inside atob() (invalid character) rather than failing GCM's authentication check on a wrong/mismatched IV. The test still passes, but for a different reason than its name/PR objective ("invalid IV handling") implies. Consider using a same-length but different IV (e.g., flip a byte of the real IV) to actually exercise GCM auth-tag mismatch behavior.
♻️ Suggested fix
- encrypted.iv = encodeURIComponent(Buffer.alloc(12).toString('base64'));
+ const tamperedIv = Uint8Array.from(atob(encrypted.iv), c => c.charCodeAt(0));
+ tamperedIv[0] ^= 0xff;
+ encrypted.iv = btoa(String.fromCharCode(...tamperedIv));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| it('should fail decryption with invalid IV', async () => { | |
| const message = { event: 'test', data: 'secret' }; | |
| const encrypted = await encryptMessage(message, aesKey); | |
| encrypted.iv = encodeURIComponent(Buffer.alloc(12).toString('base64')); | |
| await expect(decryptMessage(encrypted, aesKey)).rejects.toThrow(); | |
| }); | |
| it('should fail decryption with invalid IV', async () => { | |
| const message = { event: 'test', data: 'secret' }; | |
| const encrypted = await encryptMessage(message, aesKey); | |
| const tamperedIv = Uint8Array.from(atob(encrypted.iv), c => c.charCodeAt(0)); | |
| tamperedIv[0] ^= 0xff; | |
| encrypted.iv = btoa(String.fromCharCode(...tamperedIv)); | |
| await expect(decryptMessage(encrypted, aesKey)).rejects.toThrow(); | |
| }); |
🤖 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 `@webapp/src/utils/__tests__/encryption.test.js` around lines 139 - 146, Update
the invalid-IV test around decryptMessage to mutate the encrypted message’s
existing IV by flipping a byte while preserving its valid base64 encoding and
original length. Replace the URI-encoded zero-filled IV assignment, so
decryption reaches AES-GCM authentication failure rather than failing during
base64 decoding.
| // End-to-End Encryption utility module using Web Crypto API | ||
| // Implements ECDH key exchange and AES-GCM encryption | ||
|
|
||
| const ENCRYPTION_ENABLED = true; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
ENCRYPTION_ENABLED is hardcoded with no override, contradicting the stated requirement.
The linked issue explicitly requires "Encryption enabled by default, with a developer testing override," but ENCRYPTION_ENABLED is a hardcoded const with no environment-driven toggle. As written, isEncryptionEnabled()/shouldEncryptEvent() can never be flipped off for local/dev testing.
🐛 Proposed fix
-const ENCRYPTION_ENABLED = true;
+const ENCRYPTION_ENABLED = import.meta.env.VITE_DISABLE_ENCRYPTION !== 'true';Also applies to: 150-172
🤖 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 `@webapp/src/utils/encryption.js` at line 4, Update ENCRYPTION_ENABLED and the
related isEncryptionEnabled()/shouldEncryptEvent() logic to default encryption
to enabled while allowing a developer-controlled environment override to disable
it for local testing. Ensure the override is parsed consistently and that
existing callers use the resulting configurable value.
🔒 Security & Privacy | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Files matching encryption.js/useSession.js:\n'
fd -a 'encryption\.js$|useSession\.js$' . || true
printf '\nRelevant snippets:\n'
for f in $(fd 'encryption\.js$|useSession\.js$' .); do
echo "==== $f ===="
wc -l "$f"
sed -n '1,140p' "$f" | cat -n
done
printf '\nSearch for encodeJWKToBase64/decodeBase64ToJWK/usages:\n'
rg -n "encodeJWKToBase64|decodeBase64ToJWK|URLSearchParams|window\.location\.hash|base64PK|pk=" .Repository: itzzavdhesh/TabTwin
Length of output: 12146
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo 'Node/JS runtime behavior probe'
node - <<'JS'
const samples = [
'plain ascii: abc',
'+ slash+ sample: a+/abc',
JSON.stringify({ k: '+/=' }),
JSON.stringify({ n: 'P256', x: '+abc/xyz==', crv: 'P-256' }),
];
for (const jwk of samples) {
const plain = Buffer.from(jwk, 'utf8').toString('base64');
const url = plain.replace(/\+/g, '-').replace(/\//g, '_').replace(/=+$/, '');
const afterSearchParamsPlus = new URLSearchParams(`pk=${plain}`).get('pk');
const afterSearchParamsUrl = new URLSearchParams(`pk=${url}`).get('pk');
console.log(JSON.stringify({
jwk,
plain,
url,
containsPlusOrSlash: /[+\/=]/.test(plain),
paramsPlain: afterSearchParamsPlus,
paramsPlainRoundtripCorrect: afterSearchParamsPlus === plain,
paramsUrlRoundtripCorrect: new URLSearchParams(`pk=${afterSearchParamsUrl}`).get('pk') === url
}));
}
JSRepository: itzzavdhesh/TabTwin
Length of output: 1259
🏁 Script executed:
#!/bin/bash
set -euo epipe
echo 'Inspect URLSearchParams source/version clues'
echo "Node version:"
node -v || true
node - <<'JS'
try {
const p = new URLSearchParams('?a=' + encodeURIComponent('+'));
console.log({
raw: String(p),
get: p.get('a')
});
console.log('typeof URLSearchParams:', typeof URLSearchParams);
const proto = Object.getPrototypeOf(URLSearchParams);
console.log({
protoName: proto ? nativeToString(proto) : 'none',
constructorName: URLSearchParams.name
});
} catch (e) {
console.log('URLSearchParams unavailable:', e && e.message);
}
function nativeToString(fn) { return Object.prototype.toString.call(fn); }
JS
echo
echo 'Runtime probe for actual URLSearchParams constructed from fragments'
node - <<'JS'
function encodeJWKToBase64(jwkString) { return Buffer.from(jwkString, 'utf8').toString('base64'); }
function decodeBase64ToJWK(base64String) { return Buffer.from(base64String, 'base64').toString('utf8'); }
const jwk = '{"n":"P256","x":"+abc/xyz==","crv":"P-256"}';
let hash = `pk=${encodeJWKToBase64(jwk)}`;
let params = new URLSearchParams(hash.slice(1));
console.log({
input: jwk,
encoded: encodeJWKToBase64(jwk),
pk: params.get('pk'),
decoded: decodeBase64ToJWK(params.get('pk')),
roundtripJsonSame: JSON.parse(decodeBase64ToJWK(params.get('pk'))) && JSON.stringify(JSON.parse(decodeBase64ToJWK(params.get('pk'))), (_, v) => typeof v === 'string' ? v : v) === JSON.stringify(JSON.parse(jwk))
});
JSRepository: itzzavdhesh/TabTwin
Length of output: 203
🌐 Web query:
MDN URLSearchParams + character is decoded as space application/x-www-form-urlencoded
💡 Result:
Yes, the JavaScript URLSearchParams API adheres to the application/x-www-form-urlencoded format, which specifies that the plus sign (+) character is interpreted as a space (U+0020) during decoding [1][2][3]. When you use the URLSearchParams constructor to parse a string or call methods like get, any literal plus signs in the input string are decoded into spaces [1][4][5]. Conversely, when URLSearchParams serializes data (such as via toString or when updating a URL), it encodes spaces as plus signs and percent-encodes actual plus signs as %2B [6][7][3]. This behavior is standard for form-encoded data but can cause issues if you are working with data that contains literal plus signs—such as Base64 encoded strings or mathematical expressions—that you do not want transformed into spaces [6][8][5]. To safely handle literal plus signs and prevent them from being converted to spaces: 1. Encode your data using encodeURIComponent before appending it to the URLSearchParams object [6][8][5]. This converts '+' into '%2B', which URLSearchParams will then correctly preserve and decode back into a literal '+' when retrieved [6][3]. 2. Avoid constructing URLSearchParams objects by directly interpolating raw strings, as this leaves the input vulnerable to this decoding behavior [6][4]. Use the append method instead to maintain data integrity [6][4]. For example: // Dangerous: '+' is converted to space const params = new URLSearchParams("data=A+B"); console.log(params.get("data")); // "A B" // Safe: '+' is preserved const paramsSafe = new URLSearchParams; paramsSafe.append("data", encodeURIComponent("A+B")); console.log(paramsSafe.get("data")); // "A+B"
Citations:
- 1: https://developer.mozilla.org/en-US/docs/Web/API/URLSearchParams/URLSearchParams
- 2: https://url.spec.whatwg.org/
- 3: https://ikit.app/blog/url-plus-signs-form-encoding?lang=en
- 4: https://github.com/mdn/content/blob/main/files/en-us/web/api/urlsearchparams/index.md
- 5: https://dev.to/mahdavipanah/pitfalls-of-url-and-urlsearchparams-in-javascript-4ef8
- 6: https://developer.mozilla.org/en-US/docs/Web/API/URLSearchParams
- 7: https://orankit.com/en/blog/url-encoding-two-systems/
- 8: https://jsguides.dev/reference/built-in-objects/urlsearchparams/
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
# Use Python's standard library implementation of URLSearchParams-compatible
# application/x-www-form-urlencoded plus-decode behavior as a simple read-only probe.
from urllib.parse import parse_qs, urlencode
jwk = '{"n":"P256","x":"+abc/xyz==","crv":"P-256"}'
import base64, json
encoded = base64.urlsafe_b64encode(jwk.encode()).decode().rstrip('=')
hash_fragment = f"pk={encoded}"
print("Node/JS URLSearchParams-like +->space decode from raw base64:")
print(parse_qs(hash_fragment, keep_blank_values=True, encoding='utf-8', errors='surrogatepass').get("pk", [None])[0])
print("Node/JS URLSearchParams-like +->space decode from base64url (URL-safe):")
params = parse_qs(hash_fragment.replace("-", "+").replace("_", "/"), keep_blank_values=True, encoding='utf-8', errors='surrogatepass')
print(params.get("pk", [None])[0])
PYRepository: itzzavdhesh/TabTwin
Length of output: 409
Use base64url helpers or append for the public key in the URL
encodeJWKToBase64/decodeBase64ToJWK still use raw Base64, but the fragment value is parsed with new URLSearchParams(...), whose application/x-www-form-urlencoded semantics decode literal + as spaces. A JWK containing encoded + bytes is therefore read back as pk='... ...' before decodeBase64ToJWK returns it, so key import/encryption setup can fail or receive the wrong public key. Use base64url (-/_, no padding), or pass this value with params.append('pk', ...), before the hash is read back.
🤖 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 `@webapp/src/utils/encryption.js` at line 4, Update the public-key URL
parameter handling in encodeJWKToBase64/decodeBase64ToJWK and the fragment
construction so Base64 values cannot have literal '+' interpreted as spaces by
URLSearchParams; use the existing URL-safe Base64 approach (or append the
parameter via URLSearchParams.append) and preserve correct key round-tripping
and decoding.
| // Encrypt message with AES-GCM | ||
| export async function encryptMessage(message, aesKey) { | ||
| const iv = generateIV(); | ||
| const encoder = new TextEncoder(); | ||
| const data = encoder.encode(JSON.stringify(message)); | ||
|
|
||
| const ciphertext = await crypto.subtle.encrypt( | ||
| { | ||
| name: ALGORITHM, | ||
| iv: iv | ||
| }, | ||
| aesKey, | ||
| data | ||
| ); | ||
|
|
||
| return { | ||
| iv: btoa(String.fromCharCode(...new Uint8Array(iv))), | ||
| ciphertext: btoa(String.fromCharCode(...new Uint8Array(ciphertext))) | ||
| }; | ||
| } | ||
|
|
||
| // Decrypt message with AES-GCM | ||
| export async function decryptMessage(encryptedMessage, aesKey) { | ||
| const iv = Uint8Array.from(atob(encryptedMessage.iv), c => c.charCodeAt(0)); | ||
| const ciphertext = Uint8Array.from(atob(encryptedMessage.ciphertext), c => c.charCodeAt(0)); | ||
|
|
||
| const plaintext = await crypto.subtle.decrypt( | ||
| { | ||
| name: ALGORITHM, | ||
| iv: iv | ||
| }, | ||
| aesKey, | ||
| ciphertext | ||
| ); | ||
|
|
||
| const decoder = new TextDecoder(); | ||
| return JSON.parse(decoder.decode(plaintext)); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Spread-based base64 encoding risks a stack overflow for large payloads.
String.fromCharCode(...new Uint8Array(ciphertext)) spreads every byte as a call argument. This is fine for small payloads (cursor moves, short annotations) but the PR objectives call for encrypting "tab content snapshots," which can be large; spreading a large typed array into String.fromCharCode can throw RangeError: Maximum call stack size exceeded well before typical tab-content sizes. Same risk applies symmetrically in decryptMessage.
🐛 Proposed fix (chunked, avoids stack limits)
+function bytesToBase64(bytes) {
+ const CHUNK = 0x8000;
+ let binary = '';
+ for (let i = 0; i < bytes.length; i += CHUNK) {
+ binary += String.fromCharCode(...bytes.subarray(i, i + CHUNK));
+ }
+ return btoa(binary);
+}
+
export async function encryptMessage(message, aesKey) {
...
return {
- iv: btoa(String.fromCharCode(...new Uint8Array(iv))),
- ciphertext: btoa(String.fromCharCode(...new Uint8Array(ciphertext)))
+ iv: bytesToBase64(new Uint8Array(iv)),
+ ciphertext: bytesToBase64(new Uint8Array(ciphertext))
};
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Encrypt message with AES-GCM | |
| export async function encryptMessage(message, aesKey) { | |
| const iv = generateIV(); | |
| const encoder = new TextEncoder(); | |
| const data = encoder.encode(JSON.stringify(message)); | |
| const ciphertext = await crypto.subtle.encrypt( | |
| { | |
| name: ALGORITHM, | |
| iv: iv | |
| }, | |
| aesKey, | |
| data | |
| ); | |
| return { | |
| iv: btoa(String.fromCharCode(...new Uint8Array(iv))), | |
| ciphertext: btoa(String.fromCharCode(...new Uint8Array(ciphertext))) | |
| }; | |
| } | |
| // Decrypt message with AES-GCM | |
| export async function decryptMessage(encryptedMessage, aesKey) { | |
| const iv = Uint8Array.from(atob(encryptedMessage.iv), c => c.charCodeAt(0)); | |
| const ciphertext = Uint8Array.from(atob(encryptedMessage.ciphertext), c => c.charCodeAt(0)); | |
| const plaintext = await crypto.subtle.decrypt( | |
| { | |
| name: ALGORITHM, | |
| iv: iv | |
| }, | |
| aesKey, | |
| ciphertext | |
| ); | |
| const decoder = new TextDecoder(); | |
| return JSON.parse(decoder.decode(plaintext)); | |
| } | |
| function bytesToBase64(bytes) { | |
| const CHUNK = 0x8000; | |
| let binary = ''; | |
| for (let i = 0; i < bytes.length; i += CHUNK) { | |
| binary += String.fromCharCode(...bytes.subarray(i, i + CHUNK)); | |
| } | |
| return btoa(binary); | |
| } | |
| // Encrypt message with AES-GCM | |
| export async function encryptMessage(message, aesKey) { | |
| const iv = generateIV(); | |
| const encoder = new TextEncoder(); | |
| const data = encoder.encode(JSON.stringify(message)); | |
| const ciphertext = await crypto.subtle.encrypt( | |
| { | |
| name: ALGORITHM, | |
| iv: iv | |
| }, | |
| aesKey, | |
| data | |
| ); | |
| return { | |
| iv: bytesToBase64(new Uint8Array(iv)), | |
| ciphertext: bytesToBase64(new Uint8Array(ciphertext)) | |
| }; | |
| } | |
| // Decrypt message with AES-GCM | |
| export async function decryptMessage(encryptedMessage, aesKey) { | |
| const iv = Uint8Array.from(atob(encryptedMessage.iv), c => c.charCodeAt(0)); | |
| const ciphertext = Uint8Array.from(atob(encryptedMessage.ciphertext), c => c.charCodeAt(0)); | |
| const plaintext = await crypto.subtle.decrypt( | |
| { | |
| name: ALGORITHM, | |
| iv: iv | |
| }, | |
| aesKey, | |
| ciphertext | |
| ); | |
| const decoder = new TextDecoder(); | |
| return JSON.parse(decoder.decode(plaintext)); | |
| } |
🤖 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 `@webapp/src/utils/encryption.js` around lines 82 - 119, Replace the
spread-based byte-to-string conversions in encryptMessage with chunked
conversion that avoids passing the entire Uint8Array to String.fromCharCode, and
use the same safe decoding approach for the base64 inputs in decryptMessage.
Preserve the existing IV and ciphertext encoding formats and JSON
encryption/decryption behavior while ensuring large tab-content snapshots do not
exceed argument or stack limits.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
9 issues found across 5 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="webapp/package.json">
<violation number="1" location="webapp/package.json:12">
P2: The new webapp encryption tests are not included in the repository's normal `npm test` command: the root script still runs only `tests/**/*.test.js`. This allows regressions in the new encryption module to pass the project's standard test workflow; wiring the webapp test into the root test orchestration would make the PR's tests run consistently.</violation>
</file>
<file name="webapp/src/hooks/useSession.js">
<violation number="1" location="webapp/src/hooks/useSession.js:76">
P0: Guests never derive a secret with the host: this line overwrites any incoming host `pk` before it is parsed, so the following code imports the guest's own public key. The `/join/:id` links also carry no host key, leaving `aesKeyRef` self-derived rather than shared and preventing the host from decrypting messages.</violation>
<violation number="2" location="webapp/src/hooks/useSession.js:164">
P1: When `aesKeyRef.current` hasn't been derived yet (it's set asynchronously), this condition is falsy and the message is sent **unencrypted** instead of being queued or blocked. Since cursor moves can fire immediately after connect — well before async key generation completes — early events silently bypass encryption, defeating the E2E guarantee.
Consider buffering encryption-required events until the AES key is available, or dropping them, rather than falling through to a plaintext send.</violation>
<violation number="3" location="webapp/src/hooks/useSession.js:166">
P0: Encrypted application messages are rejected by the WebSocket server instead of being delivered. The wrapper removes the top-level `event`/`payload` fields that `server/signalingHandler.js` requires for routing, so every successfully encrypted cursor, action, or CRDT message reaches the server as an unknown event. The transport protocol needs an explicit encrypted-message routing path (or an outer envelope that preserves the required routing metadata) before enabling this send path.</violation>
</file>
<file name="webapp/src/utils/encryption.js">
<violation number="1" location="webapp/src/utils/encryption.js:4">
P2: `ENCRYPTION_ENABLED` is hardcoded to `true` with no runtime or environment override. If the feature requirement includes a developer testing toggle (e.g., to disable encryption in local dev), this needs to read from an environment variable.
Consider: `const ENCRYPTION_ENABLED = import.meta.env.VITE_DISABLE_ENCRYPTION !== 'true';`</violation>
<violation number="2" location="webapp/src/utils/encryption.js:46">
P2: The URL-safe encoding contract is not implemented: standard Base64 output can contain `+`, which `URLSearchParams` converts to a space before decoding, causing encryption initialization to fail for such a JWK. Using base64url (`+`/`/` mapped to `-`/`_`, with matching padding handling) would make the fragment encoding robust.</violation>
<violation number="3" location="webapp/src/utils/encryption.js:99">
P2: Large application messages can fail before transport: spreading the whole ciphertext into `String.fromCharCode` exceeds the runtime's argument limit at roughly 125 KB. A chunked or array-based byte-to-base64 conversion would keep large CRDT payloads enc encryptable.</violation>
<violation number="4" location="webapp/src/utils/encryption.js:131">
P1: A valid encrypted action can be replayed because the wrapper carries only the IV and ciphertext; IV uniqueness does not provide replay protection, so a captured click or type request can execute repeatedly. Including an authenticated per-session sequence number and rejecting duplicates would prevent this.</violation>
<violation number="5" location="webapp/src/utils/encryption.js:162">
P2: The encryption policy uses the wrong ICE signaling event name. `webrtc:ice-candidate` is the event emitted and routed by the application, but only `webrtc:candidate` is exempted, so the policy reports the real signaling event as encryptable. Align the exemption and test with the actual protocol name.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| // Export and publish public key in URL fragment | ||
| const jwkString = await exportPublicKeyToJWK(keyPair.publicKey); | ||
| const base64PK = encodeJWKToBase64(jwkString); | ||
| window.location.hash = `pk=${base64PK}`; |
There was a problem hiding this comment.
P0: Guests never derive a secret with the host: this line overwrites any incoming host pk before it is parsed, so the following code imports the guest's own public key. The /join/:id links also carry no host key, leaving aesKeyRef self-derived rather than shared and preventing the host from decrypting messages.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At webapp/src/hooks/useSession.js, line 76:
<comment>Guests never derive a secret with the host: this line overwrites any incoming host `pk` before it is parsed, so the following code imports the guest's own public key. The `/join/:id` links also carry no host key, leaving `aesKeyRef` self-derived rather than shared and preventing the host from decrypting messages.</comment>
<file context>
@@ -48,6 +63,38 @@ export function useSession({ sessionId, guestName, recordingEnabled = false }) {
+ // Export and publish public key in URL fragment
+ const jwkString = await exportPublicKeyToJWK(keyPair.publicKey);
+ const base64PK = encodeJWKToBase64(jwkString);
+ window.location.hash = `pk=${base64PK}`;
+
+ // Try to import host's public key from URL if this is a guest session
</file context>
| // Encrypt message if it should be encrypted and we have an encryption key | ||
| if (shouldEncryptEvent(event) && aesKeyRef.current) { | ||
| try { | ||
| message = await wrapEncryptedMessage(message, aesKeyRef.current); |
There was a problem hiding this comment.
P0: Encrypted application messages are rejected by the WebSocket server instead of being delivered. The wrapper removes the top-level event/payload fields that server/signalingHandler.js requires for routing, so every successfully encrypted cursor, action, or CRDT message reaches the server as an unknown event. The transport protocol needs an explicit encrypted-message routing path (or an outer envelope that preserves the required routing metadata) before enabling this send path.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At webapp/src/hooks/useSession.js, line 166:
<comment>Encrypted application messages are rejected by the WebSocket server instead of being delivered. The wrapper removes the top-level `event`/`payload` fields that `server/signalingHandler.js` requires for routing, so every successfully encrypted cursor, action, or CRDT message reaches the server as an unknown event. The transport protocol needs an explicit encrypted-message routing path (or an outer envelope that preserves the required routing metadata) before enabling this send path.</comment>
<file context>
@@ -96,10 +154,23 @@ export function useSession({ sessionId, guestName, recordingEnabled = false }) {
+ // Encrypt message if it should be encrypted and we have an encryption key
+ if (shouldEncryptEvent(event) && aesKeyRef.current) {
+ try {
+ message = await wrapEncryptedMessage(message, aesKeyRef.current);
+ } catch (err) {
+ console.error('Message encryption failed:', err);
</file context>
| return { | ||
| type: 'encrypted', | ||
| iv: encrypted.iv, | ||
| ciphertext: encrypted.ciphertext |
There was a problem hiding this comment.
P1: A valid encrypted action can be replayed because the wrapper carries only the IV and ciphertext; IV uniqueness does not provide replay protection, so a captured click or type request can execute repeatedly. Including an authenticated per-session sequence number and rejecting duplicates would prevent this.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At webapp/src/utils/encryption.js, line 131:
<comment>A valid encrypted action can be replayed because the wrapper carries only the IV and ciphertext; IV uniqueness does not provide replay protection, so a captured click or type request can execute repeatedly. Including an authenticated per-session sequence number and rejecting duplicates would prevent this.</comment>
<file context>
@@ -0,0 +1,172 @@
+ return {
+ type: 'encrypted',
+ iv: encrypted.iv,
+ ciphertext: encrypted.ciphertext
+ };
+}
</file context>
| let message = { event, payload: { sessionId, ...payload } }; | ||
|
|
||
| // Encrypt message if it should be encrypted and we have an encryption key | ||
| if (shouldEncryptEvent(event) && aesKeyRef.current) { |
There was a problem hiding this comment.
P1: When aesKeyRef.current hasn't been derived yet (it's set asynchronously), this condition is falsy and the message is sent unencrypted instead of being queued or blocked. Since cursor moves can fire immediately after connect — well before async key generation completes — early events silently bypass encryption, defeating the E2E guarantee.
Consider buffering encryption-required events until the AES key is available, or dropping them, rather than falling through to a plaintext send.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At webapp/src/hooks/useSession.js, line 164:
<comment>When `aesKeyRef.current` hasn't been derived yet (it's set asynchronously), this condition is falsy and the message is sent **unencrypted** instead of being queued or blocked. Since cursor moves can fire immediately after connect — well before async key generation completes — early events silently bypass encryption, defeating the E2E guarantee.
Consider buffering encryption-required events until the AES key is available, or dropping them, rather than falling through to a plaintext send.</comment>
<file context>
@@ -96,10 +154,23 @@ export function useSession({ sessionId, guestName, recordingEnabled = false }) {
+ let message = { event, payload: { sessionId, ...payload } };
+
+ // Encrypt message if it should be encrypted and we have an encryption key
+ if (shouldEncryptEvent(event) && aesKeyRef.current) {
+ try {
+ message = await wrapEncryptedMessage(message, aesKeyRef.current);
</file context>
| "preview": "vite preview", | ||
| "lint": "npm run build" | ||
| "lint": "npm run build", | ||
| "test": "vitest" |
There was a problem hiding this comment.
P2: The new webapp encryption tests are not included in the repository's normal npm test command: the root script still runs only tests/**/*.test.js. This allows regressions in the new encryption module to pass the project's standard test workflow; wiring the webapp test into the root test orchestration would make the PR's tests run consistently.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At webapp/package.json, line 12:
<comment>The new webapp encryption tests are not included in the repository's normal `npm test` command: the root script still runs only `tests/**/*.test.js`. This allows regressions in the new encryption module to pass the project's standard test workflow; wiring the webapp test into the root test orchestration would make the PR's tests run consistently.</comment>
<file context>
@@ -8,7 +8,8 @@
"preview": "vite preview",
- "lint": "npm run build"
+ "lint": "npm run build",
+ "test": "vitest"
},
"dependencies": {
</file context>
|
|
||
| // Encode JWK to URL-safe base64 | ||
| export function encodeJWKToBase64(jwkString) { | ||
| return btoa(jwkString); |
There was a problem hiding this comment.
P2: The URL-safe encoding contract is not implemented: standard Base64 output can contain +, which URLSearchParams converts to a space before decoding, causing encryption initialization to fail for such a JWK. Using base64url (+// mapped to -/_, with matching padding handling) would make the fragment encoding robust.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At webapp/src/utils/encryption.js, line 46:
<comment>The URL-safe encoding contract is not implemented: standard Base64 output can contain `+`, which `URLSearchParams` converts to a space before decoding, causing encryption initialization to fail for such a JWK. Using base64url (`+`/`/` mapped to `-`/`_`, with matching padding handling) would make the fragment encoding robust.</comment>
<file context>
@@ -0,0 +1,172 @@
+
+// Encode JWK to URL-safe base64
+export function encodeJWKToBase64(jwkString) {
+ return btoa(jwkString);
+}
+
</file context>
|
|
||
| return { | ||
| iv: btoa(String.fromCharCode(...new Uint8Array(iv))), | ||
| ciphertext: btoa(String.fromCharCode(...new Uint8Array(ciphertext))) |
There was a problem hiding this comment.
P2: Large application messages can fail before transport: spreading the whole ciphertext into String.fromCharCode exceeds the runtime's argument limit at roughly 125 KB. A chunked or array-based byte-to-base64 conversion would keep large CRDT payloads enc encryptable.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At webapp/src/utils/encryption.js, line 99:
<comment>Large application messages can fail before transport: spreading the whole ciphertext into `String.fromCharCode` exceeds the runtime's argument limit at roughly 125 KB. A chunked or array-based byte-to-base64 conversion would keep large CRDT payloads enc encryptable.</comment>
<file context>
@@ -0,0 +1,172 @@
+
+ return {
+ iv: btoa(String.fromCharCode(...new Uint8Array(iv))),
+ ciphertext: btoa(String.fromCharCode(...new Uint8Array(ciphertext)))
+ };
+}
</file context>
| 'control:revoke', | ||
| 'webrtc:offer', | ||
| 'webrtc:answer', | ||
| 'webrtc:candidate', |
There was a problem hiding this comment.
P2: The encryption policy uses the wrong ICE signaling event name. webrtc:ice-candidate is the event emitted and routed by the application, but only webrtc:candidate is exempted, so the policy reports the real signaling event as encryptable. Align the exemption and test with the actual protocol name.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At webapp/src/utils/encryption.js, line 162:
<comment>The encryption policy uses the wrong ICE signaling event name. `webrtc:ice-candidate` is the event emitted and routed by the application, but only `webrtc:candidate` is exempted, so the policy reports the real signaling event as encryptable. Align the exemption and test with the actual protocol name.</comment>
<file context>
@@ -0,0 +1,172 @@
+ 'control:revoke',
+ 'webrtc:offer',
+ 'webrtc:answer',
+ 'webrtc:candidate',
+ 'error'
+];
</file context>
| // End-to-End Encryption utility module using Web Crypto API | ||
| // Implements ECDH key exchange and AES-GCM encryption | ||
|
|
||
| const ENCRYPTION_ENABLED = true; |
There was a problem hiding this comment.
P2: ENCRYPTION_ENABLED is hardcoded to true with no runtime or environment override. If the feature requirement includes a developer testing toggle (e.g., to disable encryption in local dev), this needs to read from an environment variable.
Consider: const ENCRYPTION_ENABLED = import.meta.env.VITE_DISABLE_ENCRYPTION !== 'true';
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At webapp/src/utils/encryption.js, line 4:
<comment>`ENCRYPTION_ENABLED` is hardcoded to `true` with no runtime or environment override. If the feature requirement includes a developer testing toggle (e.g., to disable encryption in local dev), this needs to read from an environment variable.
Consider: `const ENCRYPTION_ENABLED = import.meta.env.VITE_DISABLE_ENCRYPTION !== 'true';`</comment>
<file context>
@@ -0,0 +1,172 @@
+// End-to-End Encryption utility module using Web Crypto API
+// Implements ECDH key exchange and AES-GCM encryption
+
+const ENCRYPTION_ENABLED = true;
+const IV_LENGTH = 12; // GCM IV is typically 96 bits
+const ALGORITHM = 'AES-GCM';
</file context>
1 participant
Summary
Implement ECDH-based key exchange and AES-GCM encryption for WebSocket messages between host and guest participants in TabTwin sessions.
Changes
Technical Details
Testing
✓ All 21 unit tests passing (key derivation, encryption/decryption, round-trip)
✓ Build verified successfully
✓ No CI failures
Closes #66
Summary by cubic
Adds end-to-end encryption for tab session data using ECDH (P-256) key exchange and AES-GCM so host–guest WebSocket messages are protected. Only app data is encrypted; session signaling stays plaintext. Implements #66.
New Features
pk) using base64-encoded JWK.useSessionautomatically encryptscursor:move,action:request, andcrdt:update, and decrypts incoming messages; falls back gracefully if init fails.Dependencies
vitestfor unit tests.Written for commit 19fb054. Summary will update on new commits.
Summary by CodeRabbit
New Features
Tests