Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 20 additions & 15 deletions src/components/organisms/ConversationView/ConversationView.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
'use client';

import React, { useState, useEffect, useCallback } from 'react';
import React, { useState, useEffect, useCallback, useRef } from 'react';
import ChatWindow from '@/components/organisms/ChatWindow';
import ErrorBoundary from '@/components/ErrorBoundary';
import {
Expand Down Expand Up @@ -54,6 +54,12 @@ export default function ConversationView({
const [sending, setSending] = useState(false);
const [hasMore, setHasMore] = useState(false);
const [cursor, setCursor] = useState<number | null>(null);
// Mirror cursor into a ref so loadMessages can read the latest value
// without depending on `cursor` (which would trigger a reload on every
// cursor change). loadMore=true paths must read cursorRef.current to
// avoid the stale-closure bug where pagination always re-fetched from
// the start because the useCallback closure captured cursor=null.
const cursorRef = useRef<number | null>(null);
const [error, setError] = useState<string | null>(null);
const [participantName, setParticipantName] = useState('Unknown User');

Expand Down Expand Up @@ -137,7 +143,7 @@ export default function ConversationView({

const result = await messageService.getMessageHistory(
conversationId,
loadMore ? cursor : null,
loadMore ? cursorRef.current : null,
50
);

Expand Down Expand Up @@ -187,6 +193,7 @@ export default function ConversationView({

setHasMore(result.has_more);
setCursor(result.cursor);
cursorRef.current = result.cursor;
} catch (err: unknown) {
setError(
err instanceof Error ? err.message : 'Failed to load messages'
Expand All @@ -195,10 +202,10 @@ export default function ConversationView({
setLoading(false);
}
},
// cursor is read fresh via closure for the loadMore=true path but we
// deliberately don't depend on it — a cursor change alone shouldn't
// trigger a reload, only a conversationId change should.
// eslint-disable-next-line react-hooks/exhaustive-deps
// cursor is read via cursorRef.current — depending on the cursor state
// would trigger a reload on every page-load (cursor changes after each
// call), so we use a ref to read the latest value without invalidating
// the callback identity.
[conversationId]
);

Expand Down Expand Up @@ -248,16 +255,14 @@ export default function ConversationView({

if (result.queued) {
// Offline OR send-failed-and-queued. Show the optimistic bubble.
console.log(
'[ConversationView] message queued (offline/retry):',
result.message.id
);
logger.debug('message queued (offline/retry)', {
id: result.message.id,
});
addPending(result.message.id, content);
} else {
console.log(
'[ConversationView] message sent, appending optimistic:',
result.message.id
);
logger.debug('message sent, appending optimistic', {
id: result.message.id,
});
// Optimistically append the sent message so it appears immediately.
// Supabase free tier can have read-after-write latency — an immediate
// loadMessages() query may return stale results (empty if first message).
Expand Down Expand Up @@ -285,7 +290,7 @@ export default function ConversationView({
err instanceof Error
? err.message
: 'Failed to send message. Please try again.';
console.error('[ConversationView] sendMessage failed:', msg, err);
logger.error('sendMessage failed', { message: msg, error: err });
setError(msg);
} finally {
setSending(false);
Expand Down
10 changes: 8 additions & 2 deletions src/lib/messaging/__tests__/key-derivation.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -106,13 +106,19 @@ describe('KeyDerivationService', () => {
expect(keyPair.publicKeyJwk.y).toBeDefined();
});

it('should produce extractable private key for operations', async () => {
it('should produce non-extractable private key for XSS resistance', async () => {
const password = 'TestPassword123!';
const salt = service.generateSalt();

const keyPair = await service.deriveKeyPair({ password, salt });

expect(keyPair.privateKey.extractable).toBe(true);
// Batch 8 security contract: KeyDerivationService.importPrivateKey
// imports the in-memory ECDH private key with extractable=false so a
// script holding a reference to derivedKeys.privateKey cannot
// exfiltrate raw key material via exportKey('jwk', ...). Operations
// (deriveKey/deriveBits for ECDH) still work because they don't
// require extractability.
expect(keyPair.privateKey.extractable).toBe(false);
});

it('should return base64-encoded salt in result', async () => {
Expand Down
16 changes: 12 additions & 4 deletions src/lib/messaging/encryption.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,18 @@ const logger = createLogger('messaging:encryption');

export class EncryptionService {
/**
* Generate an ECDH P-256 key pair for asymmetric encryption
* Task: T046
* Generate an ECDH P-256 key pair for asymmetric encryption.
*
* @returns Promise<KeyPair> - Public and private CryptoKey objects
* @deprecated Production messaging derives keys via `KeyDerivationService`
* (Argon2id from password). This method exists ONLY for unit-test fixtures
* that need a quick extractable keypair to round-trip JWK and assert
* non-extractable storage behavior. Do NOT call from production code:
* the returned private key is extractable, so XSS reading a reference to
* it can call exportKey('jwk', ...) and exfiltrate raw key material. If
* you need a key pair in production, use KeyDerivationService.deriveKeyPair
* which returns a non-extractable private key.
*
* @returns Promise<KeyPair> - Public and private CryptoKey objects (extractable)
* @throws EncryptionError if Web Crypto API is unavailable
*/
async generateKeyPair(): Promise<KeyPair> {
Expand All @@ -43,7 +51,7 @@ export class EncryptionService {
name: CRYPTO_PARAMS.ALGORITHM,
namedCurve: CRYPTO_PARAMS.CURVE,
},
true, // extractable
true, // extractable — see @deprecated note above
['deriveBits', 'deriveKey']
);

Expand Down
56 changes: 36 additions & 20 deletions src/lib/messaging/key-derivation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -116,21 +116,16 @@ export class KeyDerivationService {
// Step 2: Reduce seed to valid P-256 scalar
const privateKeyScalar = reduceScalar(seed);

// Step 3: Import as ECDH private key
const privateKey = await this.importPrivateKey(privateKeyScalar);
// Step 3: Import as non-extractable ECDH private key. importPrivateKey
// also returns the public-only JWK (computed from the same private
// scalar via noble-curves), so we do NOT need to exportKey() the
// private key — that would require importing extractable, which
// would let XSS exfiltrate raw key material.
const { privateKey, publicKeyJwk } =
await this.importPrivateKey(privateKeyScalar);

// Step 4: Export and reimport to get public key
const privateKeyJwk = await crypto.subtle.exportKey('jwk', privateKey);

// Create public-only JWK (remove d parameter)
const publicKeyJwk: JsonWebKey = {
kty: privateKeyJwk.kty,
crv: privateKeyJwk.crv,
x: privateKeyJwk.x,
y: privateKeyJwk.y,
};

// Import public key
// Step 4: Import the public-only JWK as a separate CryptoKey
// (extractable=true is fine here; public keys aren't sensitive).
const publicKey = await crypto.subtle.importKey(
'jwk',
publicKeyJwk,
Expand Down Expand Up @@ -220,13 +215,18 @@ export class KeyDerivationService {
}

/**
* Import raw private key bytes as CryptoKey
* Uses JWK format for better browser compatibility
* Import raw private key bytes as CryptoKey + computed public-key JWK.
* Uses JWK format for better browser compatibility.
*
* Returns the public-only JWK (x, y, no d) alongside the imported private
* CryptoKey so the caller doesn't need to round-trip the private key
* through exportKey() — that would require importing it as extractable
* (which defeats the XSS-resistance guarantee on the in-memory copy).
* @private
*/
private async importPrivateKey(
privateKeyBytes: Uint8Array
): Promise<CryptoKey> {
): Promise<{ privateKey: CryptoKey; publicKeyJwk: JsonWebKey }> {
try {
// Always use the JWK path with noble-curves to compute the public key.
// The PKCS#8 path used to be the primary, but Firefox/WebKit reject the
Expand Down Expand Up @@ -257,17 +257,33 @@ export class KeyDerivationService {
y,
};

// Import the JWK as a CryptoKey (works in all browsers)
return await crypto.subtle.importKey(
// Import the JWK as a NON-EXTRACTABLE CryptoKey.
// The in-memory private key only ever needs deriveKey/deriveBits for
// ECDH; we never need exportKey() on it. Importing non-extractable
// closes the XSS exfiltration path: a script with a handle to
// keyManagementService.derivedKeys.privateKey can still USE the key
// for ECDH from the same origin, but cannot extract raw key material
// for offline decryption of stolen ciphertext.
const privateKey = await crypto.subtle.importKey(
'jwk',
jwk,
{
name: CRYPTO_PARAMS.ALGORITHM,
namedCurve: CRYPTO_PARAMS.CURVE,
},
true,
false,
['deriveKey', 'deriveBits']
);

// Public-only JWK (no `d`) for the caller to attach to keyPair.publicKeyJwk.
const publicKeyJwk: JsonWebKey = {
kty: 'EC',
crv: CRYPTO_PARAMS.CURVE,
x,
y,
};

return { privateKey, publicKeyJwk };
} catch (error) {
if (error instanceof KeyDerivationError) {
throw error;
Expand Down
2 changes: 1 addition & 1 deletion src/lib/payments/payment-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -294,7 +294,7 @@ export async function getPaymentHistory(
status: item.status as PaymentActivity['status'],
charged_amount: item.charged_amount ?? 0,
charged_currency: item.charged_currency as Currency,
customer_email: (item.intent as any).customer_email,
customer_email: (item.intent as { customer_email: string }).customer_email,
webhook_verified: item.webhook_verified,
created_at: item.created_at,
}));
Expand Down
Loading
Loading