fix: harden snapshot keys and client IDs for Checkmarx findings

docs/security/partner-checkmarx-938.md

@@ -0,0 +1,78 @@
+# Partner Checkmarx follow-up (#938)
+
+This records source remediation for the six portal-view findings in
+[issue #938](https://github.com/lightapi/portal-view/issues/938), excluding the
+false-positive rows. Original assessment:
+[partner report](https://github.com/networknt/networknt-doc/blob/master/src/security/partner-checkmarx-assessment-2026-10-28.md).
+The partner's scanned revision and deployed artifact remain unknown.
+
+| Assessment / worksheet rows | Source disposition |
+| --- | --- |
+| A03 / 4–5 | Both comparison cell dictionaries use null prototypes. Arbitrary IDs, including __proto__, constructor, prototype and toString, remain own data keys without changing dictionary prototypes. Regression tests cover top-level/detail cells and structured-clone worker transfer; null prototypes protect construction, not the transferred object. This removes reliance on API UUID validation for this primitive. |
+| A09 / 24 | Chat client message IDs use the shared secureRandomUUID helper. The draft remains visible with an error if cryptographic generation is unavailable, including expired-session renewal. |
+| A09 / 26 | Generated legacy JSON-RPC correlation IDs use a per-page sequence, avoiding both weak randomness and a Web Crypto dependency. Explicit caller JSON-RPC IDs remain unchanged. |
+| A09 / 27 | Knowledge command idempotency keys use the same helper, one key per invocation. Commands requiring no idempotency key do not request one; missing Web Crypto rejects keyed commands before dispatch. |
+| A10 / 25 | History confirms the old ProfileMenu generator was OAuth state, not a correlation ID. The historical generator was already replaced by cryptographic generation. Sign-in now uses secureRandomUUID, including the getRandomValues fallback. Menu tests exercise the real helper, secure state storage/redirect and a visible retryable failure without Web Crypto. |
+
+## Cryptographic IDs
+
+The shared UUID helper prefers native crypto.randomUUID, otherwise generates 16
+bytes with crypto.getRandomValues and sets the UUID v4 version/variant bits. This
+preserves chat's UUID format in contexts where randomUUID is unavailable. It
+throws when neither cryptographic API is available, rather than falling back to
+Math.random or a timestamp. A failed native generator is propagated without
+silent downgrade. The separate createIdempotencyKey form helper already uses
+Web Crypto and has a different opaque-key format; it is unchanged. clientMessageId
+is a compatibility re-export of secureRandomUUID, not another implementation.
+The migration covers the assessed chat, Knowledge and OAuth paths; other direct
+randomUUID call sites are outside this change. It does not claim repository-wide
+ID-generator consolidation.
+
+JSON-RPC IDs only correlate responses within one page. A monotonically increasing
+per-page sequence is sufficient; it is not used as an idempotency key, credential
+or OAuth state. Legacy-shaped query and command calls therefore remain usable
+without Web Crypto.
+
+## Snapshot worker boundary
+
+Null-prototype cell dictionaries prevent prototype setter execution while the
+worker builds the comparison. Structured clone restores Object.prototype on the
+receiving side, but preserves dangerous names such as __proto__ as own data
+properties. The UI only reads cells by the snapshot IDs already populated by the
+builder; it does not add new untrusted keys to transferred cells. Tests verify
+that both regular and detail values survive transfer as own properties. This
+change does not promise that null prototypes survive worker transfer or that
+subsequent arbitrary writes to ordinary objects are safe.
+
+## Historical OAuth evidence
+
+- [c14f56b](https://github.com/lightapi/portal-view/commit/c14f56ba382fdfc069fa57db88966700fea3e014)
+  introduced Math.random OAuth state in ProfileMenu.
+- [d239afd](https://github.com/lightapi/portal-view/commit/d239afd6aedb6ce52af1c1bcebc96f98717269b9)
+  moved sign-in to src/utils/signIn.ts, initially retaining the weak generator.
+- [7f200a3](https://github.com/lightapi/portal-view/commit/7f200a3f93c0beeda53f6102db1732293b6cf4d3)
+  replaced it with crypto.randomUUID. Current ProfileMenu delegates to that helper;
+  Entra SSO delegates to MSAL instead.
+
+The current Dashboard callback compares returned state with portal_auth_state,
+removes it after a match and signs out after mismatch. This is source inspection;
+no live OAuth/provider or server callback validation was exercised here. History
+establishes the original purpose but does not identify the partner's exact scan
+revision. Do not describe the historical weak state generator as a false positive.
+
+## Server idempotency boundary
+
+A09 concerns weak client-generated keys. Keys are not credentials; randomizing
+one does not replace authenticated authorization or establish actor isolation.
+This frontend PR does not change or qualify backend authorization/replay policy.
+Deployment owners must verify the intended authenticated owner/operation scope,
+cross-owner rejection and replay behavior against their deployed backend. These
+are backend acceptance criteria, not conclusions established by frontend tests.
+
+## Deployment closure
+
+Deploy the repaired frontend, identify the backend/identity provider artifacts,
+and rerun the partner scan with row-specific evidence. Source fixes and synthetic
+frontend tests do not demonstrate deployed producer guarantees, live OAuth state
+validation or backend cross-owner replay isolation. No live services, databases or
+credentials were changed for this work.

src/components/Header/ProfileMenu.security.test.tsx

@@ -0,0 +1,64 @@
+import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
+import { render, screen, fireEvent, waitFor } from '@testing-library/react';
+import ProfileMenu from './ProfileMenu';
+import { publishTestConfig } from '../../test/runtimeConfigFixture';
+
+vi.mock('../../authConfig', () => ({ loginRequest: { scopes: ['openid', 'profile'] } }));
+vi.mock('../../contexts/UserContext', () => ({
+  changePassword: vi.fn(), getOrders: vi.fn(), getPayment: vi.fn(), getProfile: vi.fn(),
+  userHost: vi.fn(), createOrg: vi.fn(), signOut: vi.fn(), signUp: vi.fn(), updateRoles: vi.fn(),
+  useUserDispatch: () => vi.fn(), useUserState: () => ({ isAuthenticated: false, roles: [] }),
+}));
+vi.mock('react-router-dom', () => ({ useNavigate: () => vi.fn() }));
+vi.mock('../PortalActions/ActionDisplayToggle', () => ({ ActionDisplayToggle: () => null }));
+
+const realLocation = window.location;
+let assigned: string | null;
+beforeEach(() => {
+  publishTestConfig({ authentication: { mode: 'oauth2', signInUrl: '/signin?client_id=example' } });
+  assigned = null;
+  localStorage.clear();
+  Object.defineProperty(window, 'location', { configurable: true, value: {
+    origin: 'http://portal.example.test', set href(value: string) { assigned = value; },
+  } });
+});
+afterEach(() => {
+  Object.defineProperty(window, 'location', { configurable: true, value: realLocation });
+  localStorage.clear();
+  vi.unstubAllGlobals();
+  vi.restoreAllMocks();
+});
+
+function clickSignIn() {
+  fireEvent.click(screen.getByRole('button', { name: 'Account menu' }));
+  fireEvent.click(screen.getByRole('menuitem', { name: 'Sign In' }));
+}
+
+describe('ProfileMenu sign-in security', () => {
+  it.each(['native', 'bytes'])('uses real cryptographic OAuth state through the menu (%s)', async mode => {
+    const weak = vi.spyOn(Math, 'random');
+    const native = vi.fn(() => '00000000-0000-4000-8000-000000000001');
+    const bytes = vi.fn((value: Uint8Array) => value.fill(5));
+    vi.stubGlobal('crypto', mode === 'native' ? { randomUUID: native } : { getRandomValues: bytes });
+    render(<ProfileMenu />);
+    clickSignIn();
+    await waitFor(() => expect(assigned).not.toBeNull());
+    const expected = mode === 'native' ? '00000000-0000-4000-8000-000000000001' : '05050505-0505-4505-8505-050505050505';
+    expect(new URL(assigned!).searchParams.get('state')).toBe(expected);
+    expect(localStorage.getItem('portal_auth_state')).toBe(expected);
+    expect(mode === 'native' ? native : bytes).toHaveBeenCalledOnce();
+    expect(weak).not.toHaveBeenCalled();
+  });
+  it('shows a visible error without state storage or redirect when crypto is unavailable, then allows retry', async () => {
+    vi.stubGlobal('crypto', undefined);
+    render(<ProfileMenu />);
+    clickSignIn();
+    expect(await screen.findByRole('alert')).toHaveTextContent('Unable to sign in securely');
+    expect(assigned).toBeNull();
+    expect(localStorage.getItem('portal_auth_state')).toBeNull();
+    vi.stubGlobal('crypto', { randomUUID: () => '00000000-0000-4000-8000-000000000002' });
+    clickSignIn();
+    await waitFor(() => expect(screen.queryByRole('alert')).not.toBeInTheDocument());
+    expect(new URL(assigned!).searchParams.get('state')).toBe('00000000-0000-4000-8000-000000000002');
+  });
+});

src/components/Header/ProfileMenu.tsx

@@ -1,6 +1,6 @@
 import { useState } from "react";
 import { Person as AccountIcon } from "@mui/icons-material";
-import { IconButton, Menu, MenuItem, ListSubheader } from "@mui/material";
+import { IconButton, Menu, MenuItem, ListSubheader, Alert } from "@mui/material";
 import { useTheme } from "@mui/material/styles";
 import { useNavigate } from "react-router-dom";
 import type { IPublicClientApplication } from "@azure/msal-browser";
@@ -30,11 +30,20 @@ function ProfileMenuContent({
 }) {
   const theme = useTheme();
   const [profileMenu, setProfileMenu] = useState<null | HTMLElement>(null);
+  const [signInError, setSignInError] = useState<string | null>(null);
   const userDispatch = useUserDispatch();
   const { isAuthenticated, userId, email, roles } = useUserState();
   const navigate = useNavigate();
 
-  const handleSignIn = () => signIn(msalInstance);
+  const handleSignIn = async () => {
+    setSignInError(null);
+    try {
+      await signIn(msalInstance);
+    } catch {
+      setProfileMenu(null);
+      setSignInError('Unable to sign in securely. Try a supported browser or contact your administrator.');
+    }
+  };
 
   const handleMenuClose = () => {
     setProfileMenu(null);
@@ -47,6 +56,7 @@ function ProfileMenuContent({
 
   return (
     <>
+      {signInError && <Alert severity="error">{signInError}</Alert>}
       <IconButton
         aria-haspopup="true"
         color="inherit"

src/pages/genai/Chat.test.tsx

@@ -395,3 +395,23 @@ it('offers only advertised workspaces and submits a task without repository bund
   act(() => socket.receive({ type: 'executionResult', turnId: 'turn-a', state: 'COMPLETED', text: 'It loads at startup.', workspace: { workspaceId: 'personal', taskId: 'task-one', checkpointDigest: 'sha256:'+'b'.repeat(64) } }));
   expect(screen.getByLabelText('Existing task ID')).toHaveValue('task-one');
 });
+
+it.each([false, true])('keeps the draft and reports missing Web Crypto before send/renewal (expired=%s)', async expired => {
+  const refresh = vi.fn();
+  const user = userEvent.setup(); render(<Chat />); const socket = await connect(user);
+  act(() => {
+    socket.readyState = 1; socket.onopen?.();
+    socket.receive({ type: 'session', session_id: 'session-a' });
+    if (expired) socket.receive({ type: 'authentication_context', expiresAt: Math.floor(Date.now() / 1000) - 1 });
+  });
+  vi.stubGlobal('fetch', refresh);
+  vi.stubGlobal('crypto', undefined);
+  try {
+    fireEvent.change(screen.getByPlaceholderText('Type your message here...'), { target: { value: 'unsent draft' } });
+    await user.click(screen.getByRole('button', { name: 'Send message' }));
+    expect(screen.getByText('Secure random ID generation is unavailable')).toBeInTheDocument();
+    expect(socket.send).not.toHaveBeenCalled();
+    expect(refresh).not.toHaveBeenCalled();
+    expect(screen.getByPlaceholderText('Type your message here...')).toHaveValue('unsent draft');
+  } finally { vi.unstubAllGlobals(); }
+});

src/pages/genai/Chat.tsx

@@ -346,12 +346,12 @@ export default function Chat() {
     const handleSend = () => {
         if (!input.trim() || !ws.current || ws.current.readyState !== WebSocket.OPEN || !connected || !sessionReady || !turnTypes.includes(mode)) return;
         setSendError('');
-        if (authenticationExpiresAt.current !== null && Date.now() >= authenticationExpiresAt.current * 1000) {
-            draftId.current ||= clientMessageId();
-            void reauthenticate();
-            return;
-        }
         try {
+            if (authenticationExpiresAt.current !== null && Date.now() >= authenticationExpiresAt.current * 1000) {
+                draftId.current ||= clientMessageId();
+                void reauthenticate();
+                return;
+            }
             const id = draftId.current || clientMessageId();
             const payload = mode === 'coding'
                 ? codingSource === 'workspace'

src/pages/genai/codingRequest.test.ts

@@ -1,5 +1,5 @@
-import { describe, expect, it } from 'vitest';
-import { codingPayload, emptyCodingInput, importCodingRequest } from './codingRequest';
+import { afterEach, describe, expect, it, vi } from 'vitest';
+import { clientMessageId, codingPayload, emptyCodingInput, importCodingRequest } from './codingRequest';
 const input = { ...emptyCodingInput, artifactUri: 'file:///spool/repo.bundle', digest: 'sha256:' + 'a'.repeat(64), size: '42', baseRevision: 'b'.repeat(40) };
 describe('typed coding request', () => {
   it('builds the fixed implementation authority and imports a generated smoke request', () => {
@@ -18,3 +18,15 @@ describe('typed coding request', () => {
       expect(() => importCodingRequest({ profile: 'coding', text: 'request', coding: { ...codingPayload(input), ...change } })).toThrow();
   });
 });
+
+describe('client message IDs', () => {
+  afterEach(() => vi.unstubAllGlobals());
+  it('uses Web Crypto outside secure contexts', () => {
+    vi.stubGlobal('crypto', { getRandomValues: (bytes: Uint8Array) => bytes.fill(1) });
+    expect(clientMessageId()).toBe('01010101-0101-4101-8101-010101010101');
+  });
+  it('does not generate predictable IDs without Web Crypto', () => {
+    vi.stubGlobal('crypto', undefined);
+    expect(() => clientMessageId()).toThrow('Secure random ID generation is unavailable');
+  });
+});

src/pages/genai/codingRequest.ts

@@ -1,3 +1,5 @@
+export { secureRandomUUID as clientMessageId } from '../../utils/secureRandomUUID';
+
 export type CodingInput = {
   artifactUri: string; digest: string; size: string; baseRevision: string;
   workspaceRoot: string; maximumPatchBytes: string; maximumChangedFiles: string;
@@ -6,19 +8,6 @@ export const emptyCodingInput: CodingInput = {
   artifactUri: '', digest: '', size: '', baseRevision: '', workspaceRoot: '/workspace/repository',
   maximumPatchBytes: '65536', maximumChangedFiles: '1',
 };
-/** crypto.randomUUID is secure-context only; getRandomValues is not. The chat page
- *  also serves ws:// over plain http, where the former is undefined. */
-export function clientMessageId(): string {
-  const source = globalThis.crypto;
-  if (typeof source?.randomUUID === 'function') return source.randomUUID();
-  const bytes = new Uint8Array(16);
-  if (typeof source?.getRandomValues === 'function') source.getRandomValues(bytes);
-  else for (let index = 0; index < bytes.length; index += 1) bytes[index] = Math.floor(Math.random() * 256);
-  bytes[6] = (bytes[6] & 0x0f) | 0x40;
-  bytes[8] = (bytes[8] & 0x3f) | 0x80;
-  const hex = Array.from(bytes, byte => byte.toString(16).padStart(2, '0')).join('');
-  return `${hex.slice(0, 8)}-${hex.slice(8, 12)}-${hex.slice(12, 16)}-${hex.slice(16, 20)}-${hex.slice(20)}`;
-}
 const positive = (text: string, name: string) => {
   const value = Number(text);
   if (!Number.isSafeInteger(value) || value <= 0) throw new Error(`${name} must be a positive safe integer.`);

src/pages/genai/knowledgeApi.test.ts

@@ -33,7 +33,32 @@ describe('Knowledge command API', () => {
         });
     });
 
-    it('does not send an idempotency key for ordinary aggregate updates', async () => {
+    it('uses cryptographic UUID bytes when randomUUID is unavailable', async () => {
+        vi.stubGlobal('crypto', { getRandomValues: (bytes: Uint8Array) => bytes.fill(3) });
+        await knowledgeCommand('requestKnowledgeBaseReindex', { knowledgeBaseId: 'kb-a' });
+        expect(mockedFetchClient).toHaveBeenCalledWith('/portal/command', expect.objectContaining({
+            headers: { 'Idempotency-Key': '03030303-0303-4303-8303-030303030303' },
+        }));
+    });
+
+    it('does not send idempotent commands without Web Crypto', async () => {
+        vi.stubGlobal('crypto', undefined);
+        await expect(knowledgeCommand('createKnowledgeBase', {})).rejects.toThrow('Secure random ID generation is unavailable');
+        expect(mockedFetchClient).not.toHaveBeenCalled();
+    });
+
+    it('generates separate keys for separate invocations', async () => {
+        const uuid = vi.fn().mockReturnValueOnce('00000000-0000-4000-8000-000000000001')
+            .mockReturnValueOnce('00000000-0000-4000-8000-000000000002');
+        vi.stubGlobal('crypto', { randomUUID: uuid });
+        await knowledgeCommand('createKnowledgeBase', { name: 'first' });
+        await knowledgeCommand('createKnowledgeBase', { name: 'second' });
+        expect(mockedFetchClient.mock.calls.map(call => call[1].headers['Idempotency-Key']))
+            .toEqual(['00000000-0000-4000-8000-000000000001', '00000000-0000-4000-8000-000000000002']);
+    });
+
+    it('does not send an idempotency key or need randomness for ordinary aggregate updates', async () => {
+        vi.stubGlobal('crypto', undefined);
         const data = {
             knowledgeBaseId: '018f0000-0000-7000-8000-000000000002',
             environment: 'dev', aggregateVersion: 1,

src/pages/genai/knowledgeApi.ts

@@ -1,3 +1,4 @@
+import { secureRandomUUID } from '../../utils/secureRandomUUID';
 import fetchClient from '../../utils/fetchClient';
 
 const API = { host: 'lightapi.net', service: 'genai', version: '0.1.0' } as const;
@@ -40,9 +41,7 @@ export async function knowledgeQuery<T>(action: string, data: Record<string, unk
 export async function knowledgeCommand(action: string, data: Record<string, unknown>) {
     const idempotent = IDEMPOTENT_KNOWLEDGE_COMMANDS.has(action);
     const idempotencyKey = idempotent
-        ? (typeof crypto !== 'undefined' && crypto.randomUUID
-            ? crypto.randomUUID()
-            : `${Date.now()}-${Math.random().toString(36).slice(2)}`)
+        ? secureRandomUUID()
         : undefined;
     return fetchClient('/portal/command', {
         method: 'POST',

src/pages/snapshot/snapshotComparison.test.ts

@@ -10,6 +10,29 @@ const IDS = [
 ];
 
 describe('snapshot comparison', () => {
+  it.each(['__proto__', 'constructor', 'prototype', 'toString'])('treats snapshot ID %s as an ordinary own key, including worker transfer', id => {
+    const first = { ...snapshot(0, [entry('nested', { leaf: 'first' }, 'map')]), snapshotId: id };
+    const second = snapshot(1, [entry('nested', { leaf: 'second' }, 'map')]);
+    const model = buildSnapshotComparison([first, second]);
+    for (const rows of [model.rows, model.detailRows]) {
+      expect(Object.getPrototypeOf(rows[0].cells)).toBeNull();
+      expect(Object.hasOwn(rows[0].cells, id)).toBe(true);
+      expect(rows[0].cells[id]?.value).toEqual(rows === model.rows ? { leaf: 'first' } : 'first');
+      expect(rows[0].status).toBe('valueChanged');
+    }
+    // Structured clone may restore Object.prototype, but must keep dangerous keys as data properties.
+    const transferred = structuredClone(model);
+    expect(Object.getPrototypeOf(transferred.rows[0].cells)).toBe(Object.prototype);
+    expect(Object.getOwnPropertyDescriptor(transferred.rows[0].cells, id)).toEqual(expect.objectContaining({
+      value: expect.objectContaining({ value: { leaf: 'first' } }),
+    }));
+    expect(Object.hasOwn(transferred.rows[0].cells, id)).toBe(true);
+    expect(transferred.rows[0].cells[id]?.value).toEqual({ leaf: 'first' });
+    expect(Object.hasOwn(transferred.detailRows[0].cells, id)).toBe(true);
+    expect(transferred.detailRows[0].cells[id]?.value).toBe('first');
+    expect(Object.prototype).not.toHaveProperty('leaf');
+  });
+
   it('ignores map order, preserves list order, and detects source-only changes', () => {
     const model = buildSnapshotComparison([
       snapshot(0, [entry('map', { b: 2, a: 1 }, 'map'), entry('list', ['a', 'b'], 'list'), entry('source', true, 'boolean', 'environment')]),