SPARK-843491: Remediate 8 High security findings in components

.circleci/config.yml

@@ -16,7 +16,10 @@ jobs:
             - dependency-cache-{{ checksum "package-lock.json" }}
       - run:
           name: Install dependencies & peer dependencies
-          command: npm install
+          command: npm ci
+      - run:
+          name: Audit for high-severity vulnerabilities
+          command: npm audit --audit-level=high
       - save_cache:
           key: dependency-cache-{{ checksum "package-lock.json" }}
           paths: node_modules

.github/workflows/npm-storybook-release.yml

@@ -16,7 +16,8 @@ jobs:
       - uses: actions/setup-node@v3
         with:
           node-version: 20.13.1
-      - run: npm install
+      - run: npm ci
+      - run: npm audit --audit-level=high
       - run: npm run build
       - run: npx semantic-release
         env:

ai-docs/SECURITY.md

@@ -46,11 +46,11 @@ Evidence: `.gitignore` (`.env*`), `CONTRIBUTING.md`.
 
 ## Input Validation & Output Encoding Posture
 
-- Validate URLs with `isValidUrl` and explicit protocol allow-lists where used.
-- Markdown rendering in messaging uses `markdown-it` — hosts should sanitize untrusted content before display if sourcing external messages.
+- Validate URLs with `isValidUrl` and explicit protocol allow-lists where used. For `http:` and `https:` URLs, `isValidUrl` additionally rejects loopback hosts (`localhost`, `127.x.x.x`, `[::1]`), link-local addresses (`169.254.x.x`, `fe80::/10`), and private/unique-local ranges (`10.x`, `172.16-31.x`, `192.168.x`, `fc00::/7`). The protocol allow-list remains caller-scoped, so `data:` URIs used for adaptive-card icons are unaffected.
+- Markdown rendering uses `markdown-it` (zero preset) wrapped with `DOMPurify.sanitize` before `dangerouslySetInnerHTML` — malicious HTML/script is neutralized while supported formatting (emphasis, links, lists, strikethrough) is preserved.
 - Adaptive cards render templated JSON — validate card payload at adapter/host layer.
 
-Evidence: `src/util.js`, `package.json` dependencies.
+Evidence: `src/util.js`, `src/components/adaptive-cards/Markdown/Markdown.jsx`, `package.json` dependencies.
 
 ## Known Sensitive Areas & Accepted Risks
 

package.json

@@ -46,6 +46,7 @@
     "adaptivecards-templating": "^2.2.0",
     "classnames": "^2.2.6",
     "date-fns": "^2.15.0",
+    "dompurify": "^3.4.13",
     "markdown-it": "^12.3.2",
     "react-draggable": "^4.4.5"
   },

src/components/SignIn/SignIn.jsx

@@ -1,4 +1,4 @@
-import React, {useState} from 'react';
+import React, {useRef, useState} from 'react';
 import PropTypes from 'prop-types';
 import {Button} from '../generic';
 import Spinner from '../generic/Spinner/Spinner';
@@ -40,10 +40,18 @@ export default function SignIn({
   const [isAuthenticating, setIsAuthenticating] = useState(false);
   const [cssClasses] = webexComponentClasses('oauth-sign-in', className);
   const [emitMetrics] = useMetrics();
+  const csrfStateRef = useRef(null);
 
   const openAuthUrl = () => {
-    const arr = new Uint8Array(4);
-    const newState = state || window.crypto.getRandomValues(arr);
+    let newState = state;
+
+    if (!newState) {
+      const arr = new Uint8Array(16);
+
+      window.crypto.getRandomValues(arr);
+      newState = Array.from(arr).map((b) => b.toString(16).padStart(2, '0')).join('');
+    }
+    csrfStateRef.current = newState;
     const fullAuthUrl = `${authUrl}?client_id=${clientID}&response_type=code&redirect_uri=${encodeURI(redirectUri)}${scope !== '' ? `&scope=${encodeURI(scope)}` : ''}&state=${newState}`;
     const startTime = window.performance.now();
     const newWindow = window.open(fullAuthUrl, 'targetWindow', 'toolbar=no,location=no,status=no,menubar=no,scrollbars=yes,resizable=yes,width=400,height=700');
@@ -76,15 +84,23 @@ export default function SignIn({
                     const expiry = new Date();
 
                     expiry.setSeconds(ttl);
-                    document.cookie = `${name}=${accessToken}; secure; expires=${expiry.toUTCString()}`;
+                    document.cookie = `${name}=${accessToken}; secure; SameSite=Strict; expires=${expiry.toUTCString()}`;
                     break;
                   }
-                  case 'session':
-                    sessionStorage.setItem(name, accessToken);
+                  case 'session': {
+                    const sessionExpiry = Date.now() + ttl * 1000;
+                    const sessionData = JSON.stringify({token: accessToken, expiry: sessionExpiry});
+
+                    sessionStorage.setItem(name, sessionData);
                     break;
-                  case 'local':
-                    localStorage.setItem(name, accessToken);
+                  }
+                  case 'local': {
+                    const localExpiry = Date.now() + ttl * 1000;
+                    const localData = JSON.stringify({token: accessToken, expiry: localExpiry});
+
+                    localStorage.setItem(name, localData);
                     break;
+                  }
                   default:
                     break;
                 }

src/components/SignIn/SignIn.test.jsx

@@ -0,0 +1,225 @@
+import React from 'react';
+import {create, act} from 'react-test-renderer';
+import {AdapterContext} from '../hooks/contexts';
+import SignIn from './SignIn';
+
+const mockMetricsAdapter = {
+  submitMetrics: jest.fn(),
+};
+
+const defaultProps = {
+  authUrl: 'https://auth.example.com/oauth',
+  clientID: 'test-client-id',
+  redirectUri: 'https://app.example.com/callback',
+  authType: 'Custom',
+  scope: 'spark:all',
+  signInResponse: jest.fn(),
+  getAccessToken: jest.fn(),
+  tokenStoragePolicy: {},
+};
+
+function renderSignIn(props = {}) {
+  const mergedProps = {...defaultProps, ...props};
+  let renderer;
+
+  act(() => {
+    renderer = create(
+      <AdapterContext.Provider value={{metricsAdapter: mockMetricsAdapter}}>
+        <SignIn {...mergedProps} />
+      </AdapterContext.Provider>,
+    );
+  });
+
+  return renderer;
+}
+
+function clickSignInButton(renderer) {
+  const button = renderer.root.findByType('button');
+
+  act(() => {
+    button.props.onClick();
+  });
+}
+
+describe('SignIn', () => {
+  let openSpy;
+  let fakeWindow;
+
+  beforeEach(() => {
+    jest.useFakeTimers();
+
+    fakeWindow = {closed: false};
+    openSpy = jest.spyOn(window, 'open').mockReturnValue(fakeWindow);
+
+    window.crypto = {
+      getRandomValues: (arr) => {
+        for (let i = 0; i < arr.length; i += 1) {
+          // eslint-disable-next-line no-param-reassign
+          arr[i] = i + 1;
+        }
+
+        return arr;
+      },
+    };
+
+    sessionStorage.clear();
+    localStorage.clear();
+  });
+
+  afterEach(() => {
+    jest.useRealTimers();
+    openSpy.mockRestore();
+  });
+
+  describe('AC-3: CSRF state generation (>=16 bytes, hex-serialized, stored)', () => {
+    test('state has >=16 bytes serialized as hex', () => {
+      const renderer = renderSignIn();
+
+      clickSignInButton(renderer);
+
+      expect(openSpy).toHaveBeenCalled();
+      const calledUrl = openSpy.mock.calls[0][0];
+      const url = new URL(calledUrl);
+      const stateParam = url.searchParams.get('state');
+
+      expect(stateParam).toBeTruthy();
+      // must not be comma-joined array like old Uint8Array coercion
+      expect(stateParam).not.toMatch(/,/);
+      // hex chars only
+      expect(stateParam).toMatch(/^[0-9a-f]+$/i);
+      // at least 32 hex chars = 16 bytes
+      expect(stateParam.length).toBeGreaterThanOrEqual(32);
+    });
+
+    test('state param is stored before auth window opens', () => {
+      const renderer = renderSignIn();
+
+      clickSignInButton(renderer);
+
+      // window.open was called with a valid state — state was serialized and
+      // stored in csrfStateRef before openAuthUrl called window.open
+      expect(openSpy).toHaveBeenCalled();
+      const calledUrl = openSpy.mock.calls[0][0];
+      const stateParam = new URL(calledUrl).searchParams.get('state');
+
+      expect(stateParam).toMatch(/^[0-9a-f]{32,}$/i);
+    });
+
+    test('uses caller-supplied state prop when provided', () => {
+      const renderer = renderSignIn({state: 'caller-provided-state'});
+
+      clickSignInButton(renderer);
+
+      const calledUrl = openSpy.mock.calls[0][0];
+      const url = new URL(calledUrl);
+
+      expect(url.searchParams.get('state')).toBe('caller-provided-state');
+    });
+  });
+
+  describe('AC-4: cookie includes SameSite=Strict', () => {
+    test('cookie includes secure and SameSite=Strict', async () => {
+      const accessToken = 'test-access-token-abc123';
+      const getAccessToken = jest.fn().mockResolvedValue(accessToken);
+      const cookieAssignments = [];
+      const origDescriptor = Object.getOwnPropertyDescriptor(document, 'cookie');
+
+      Object.defineProperty(document, 'cookie', {
+        set(val) {
+          cookieAssignments.push(val);
+        },
+        get() {
+          return origDescriptor ? origDescriptor.get.call(this) : '';
+        },
+        configurable: true,
+      });
+
+      const renderer = renderSignIn({
+        getAccessToken,
+        tokenStoragePolicy: {place: 'cookie', name: 'test_cookie', ttl: 3600},
+      });
+
+      clickSignInButton(renderer);
+
+      fakeWindow.closed = true;
+      act(() => {
+        jest.advanceTimersByTime(600);
+      });
+
+      await act(async () => {
+        await Promise.resolve();
+      });
+
+      if (origDescriptor) {
+        Object.defineProperty(document, 'cookie', origDescriptor);
+      }
+
+      const cookieStr = cookieAssignments.find((c) => c.includes('test_cookie'));
+
+      expect(cookieStr).toBeTruthy();
+      expect(cookieStr).toMatch(/secure/i);
+      expect(cookieStr).toMatch(/SameSite=Strict/i);
+    });
+  });
+
+  describe('AC-5: session/local token respects ttl', () => {
+    test('session storage stores token with expiry envelope (not raw string)', async () => {
+      const accessToken = 'session-token-xyz';
+      const getAccessToken = jest.fn().mockResolvedValue(accessToken);
+      const renderer = renderSignIn({
+        getAccessToken,
+        tokenStoragePolicy: {place: 'session', name: 'sess_token', ttl: 1800},
+      });
+
+      clickSignInButton(renderer);
+
+      fakeWindow.closed = true;
+      act(() => {
+        jest.advanceTimersByTime(600);
+      });
+
+      await act(async () => {
+        await Promise.resolve();
+      });
+
+      const stored = sessionStorage.getItem('sess_token');
+
+      expect(stored).toBeTruthy();
+      const parsed = JSON.parse(stored);
+
+      expect(parsed.token).toBe(accessToken);
+      expect(typeof parsed.expiry).toBe('number');
+      // expiry should be in the future (Date.now() is mocked by MockDate to Aug 1 2020)
+      expect(parsed.expiry).toBeGreaterThan(Date.now());
+    });
+
+    test('local storage stores token with expiry envelope (not raw string)', async () => {
+      const accessToken = 'local-token-xyz';
+      const getAccessToken = jest.fn().mockResolvedValue(accessToken);
+      const renderer = renderSignIn({
+        getAccessToken,
+        tokenStoragePolicy: {place: 'local', name: 'local_token', ttl: 86400},
+      });
+
+      clickSignInButton(renderer);
+
+      fakeWindow.closed = true;
+      act(() => {
+        jest.advanceTimersByTime(600);
+      });
+
+      await act(async () => {
+        await Promise.resolve();
+      });
+
+      const stored = localStorage.getItem('local_token');
+
+      expect(stored).toBeTruthy();
+      const parsed = JSON.parse(stored);
+
+      expect(parsed.token).toBe(accessToken);
+      expect(typeof parsed.expiry).toBe('number');
+      expect(parsed.expiry).toBeGreaterThan(Date.now());
+    });
+  });
+});

src/components/adaptive-cards/Markdown/Markdown.jsx

@@ -1,5 +1,6 @@
 import React, {useMemo} from 'react';
 import PropTypes from 'prop-types';
+import DOMPurify from 'dompurify';
 import MarkdownIt from 'markdown-it';
 import webexComponentClasses from '../../helpers';
 
@@ -26,7 +27,7 @@ export default function Markdown({children, className}) {
       'strikethrough',
     ],
   ), []);
-  let html = markdownIt.render(children);
+  let html = DOMPurify.sanitize(markdownIt.render(children));
 
   if (html.startsWith('<p>') && html.indexOf('</p>') === html.length - 5) {
     html = html.slice(3, -5);