sec(frontend): escape payment field in plans and recs tables

frontend/src/__tests__/xss-purchase-payment.test.ts

@@ -0,0 +1,199 @@
+/**
+ * Regression tests for stored XSS via purchase.payment rendered unescaped
+ * inside the planned-purchases term cell (issue #1633).
+ *
+ * plans.ts:renderPlannedPurchaseRow used to emit:
+ *   <td>${purchase.term}yr ${purchase.payment.replace('-', ' ')}</td>
+ *
+ * Every sibling string field in the same row (plan_name, service,
+ * resource_type, region, status) is escaped with the existing
+ * escapeHtml() from utils.ts -- see xss-purchase-status.test.ts (#445) --
+ * but `payment` was the one field left raw. `payment` is typed as a bare
+ * `string` in api/types.ts (not a closed union like `status`), so nothing
+ * on the frontend constrains its content. The fix wraps the interpolation
+ * with escapeHtml() and defaults a missing value to '' so the unguarded
+ * .replace() call on a null/undefined payment no longer throws.
+ */
+
+import { loadPlans } from '../plans';
+
+// Use the real escapeHtml so the DOM-based escaping is exercised, not a stub.
+jest.mock('../utils', () => {
+  const actual = jest.requireActual<typeof import('../utils')>('../utils');
+  return {
+    ...actual,
+    formatDate: jest.fn((val: string) => val ? new Date(val).toLocaleDateString() : ''),
+    formatTerm: jest.fn((years: number | null | undefined) =>
+      years == null ? '' : `${years} Year${years === 1 ? '' : 's'}`),
+    formatRampSchedule: jest.fn((val: string) => val || 'Unknown'),
+    formatCurrency: jest.fn((val: number) => `$${val || 0}`),
+    populateAccountFilter: jest.fn(() => Promise.resolve()),
+  };
+});
+
+jest.mock('../api', () => ({
+  getPlans: jest.fn(),
+  getPlannedPurchases: jest.fn(),
+  getPlan: jest.fn(),
+  createPlan: jest.fn(),
+  updatePlan: jest.fn(),
+  patchPlan: jest.fn(),
+  deletePlan: jest.fn(),
+  runPlannedPurchase: jest.fn(),
+  pausePlannedPurchase: jest.fn(),
+  resumePlannedPurchase: jest.fn(),
+  deletePlannedPurchase: jest.fn(),
+  createPlannedPurchases: jest.fn(),
+  listPlanAccounts: jest.fn().mockResolvedValue([]),
+  setPlanAccounts: jest.fn().mockResolvedValue(undefined),
+  listAccounts: jest.fn().mockResolvedValue([]),
+}));
+
+jest.mock('../state', () => ({
+  getRecommendations: jest.fn().mockReturnValue([]),
+  getSelectedRecommendationIDs: jest.fn().mockReturnValue(new Set()),
+  getVisibleRecommendations: jest.fn().mockReturnValue([]),
+  setVisibleRecommendations: jest.fn(),
+  getCurrentProvider: jest.fn().mockReturnValue(''),
+  setCurrentProvider: jest.fn(),
+  getCurrentAccountIDs: jest.fn().mockReturnValue([]),
+  setCurrentAccountIDs: jest.fn(),
+  subscribeProvider: jest.fn().mockReturnValue(() => {}),
+  subscribeAccount: jest.fn().mockReturnValue(() => {}),
+  getCurrentUser: jest.fn().mockReturnValue({ id: 'u-admin', email: 'admin@example.com', groups: ['00000000-0000-5000-8000-000000000001'] }),
+  getPlansColumnFilters: jest.fn().mockReturnValue({}),
+  setPlansColumnFilter: jest.fn(),
+  clearAllPlansColumnFilters: jest.fn(),
+}));
+
+jest.mock('../history', () => ({
+  viewPlanHistory: jest.fn(),
+}));
+
+jest.mock('../commitmentOptions', () => ({
+  populateTermSelect: jest.fn(),
+  populatePaymentSelect: jest.fn(),
+  isValidCombination: jest.fn().mockReturnValue(true),
+  normalizePaymentValue: jest.fn((value: unknown) => value),
+}));
+
+jest.mock('../toast', () => ({
+  showToast: jest.fn(() => ({ dismiss: jest.fn() })),
+}));
+
+jest.mock('../confirmDialog', () => ({
+  confirmDialog: jest.fn(() => Promise.resolve(true)),
+}));
+
+jest.mock('../modal', () => ({
+  openModal: jest.fn(),
+  closeModal: jest.fn(),
+}));
+
+jest.mock('../archera', () => ({
+  openArcheraOfferModal: jest.fn(),
+}));
+
+jest.mock('../lib/skeleton', () => ({
+  showSkeletonTiles: jest.fn(),
+  showSkeletonRows: jest.fn(),
+  teardownSkeleton: jest.fn(),
+}));
+
+jest.mock('../permissions', () => ({
+  canAccess: jest.fn().mockReturnValue(true),
+}));
+
+import * as api from '../api';
+
+const SCRIPT_PAYLOAD = '<script>alert(1)</script>';
+const IMG_PAYLOAD = '"><img src=x onerror="alert(1)">';
+
+function makePurchase(payment: string | undefined) {
+  const purchase: Record<string, unknown> = {
+    id: 'pp-1',
+    plan_id: 'plan-1',
+    plan_name: 'Test Plan',
+    scheduled_date: '2024-06-01',
+    provider: 'aws',
+    service: 'ec2',
+    resource_type: 't3.medium',
+    region: 'us-east-1',
+    count: 1,
+    term: 1,
+    payment,
+    estimated_savings: 50,
+    upfront_cost: 0,
+    status: 'pending',
+    step_number: 1,
+    total_steps: 1,
+  };
+  if (payment === undefined) delete purchase['payment'];
+  return purchase;
+}
+
+describe('XSS regression: purchase.payment rendered as text, not markup (#1633)', () => {
+  beforeEach(() => {
+    document.body.innerHTML = `
+      <div id="plans-list"></div>
+      <div id="planned-purchases-list"></div>
+      <button id="new-plan-btn"></button>
+    `;
+    jest.clearAllMocks();
+    (api.getPlans as jest.Mock).mockResolvedValue({ plans: [] });
+  });
+
+  test('script-tag payload in purchase.payment does not create a <script> element', async () => {
+    (api.getPlannedPurchases as jest.Mock).mockResolvedValue({
+      purchases: [makePurchase(SCRIPT_PAYLOAD)],
+    });
+
+    await loadPlans();
+
+    // No <script> element should have been injected.
+    expect(document.querySelectorAll('script').length).toBe(0);
+
+    const list = document.getElementById('planned-purchases-list');
+    expect(list).not.toBeNull();
+    // Raw tags must not appear in the rendered markup.
+    expect(list!.innerHTML).not.toContain('<script>');
+    // The payload must appear only as escaped text content (the term cell
+    // also runs payment through .replace('-', ' '), which no-ops here since
+    // the payload has no hyphen).
+    expect(list!.textContent).toContain(SCRIPT_PAYLOAD);
+  });
+
+  test('img-onerror payload in purchase.payment does not inject a live <img> element', async () => {
+    (api.getPlannedPurchases as jest.Mock).mockResolvedValue({
+      purchases: [makePurchase(IMG_PAYLOAD)],
+    });
+
+    await loadPlans();
+
+    const list = document.getElementById('planned-purchases-list');
+    expect(list).not.toBeNull();
+    // Strongest assertion: no <img> DOM node was ever parsed out of the cell.
+    expect(list!.querySelectorAll('img').length).toBe(0);
+    expect(list!.textContent).toContain('onerror');
+  });
+
+  test('valid payment "no-upfront" still renders as readable text with hyphen replaced', async () => {
+    (api.getPlannedPurchases as jest.Mock).mockResolvedValue({
+      purchases: [makePurchase('no-upfront')],
+    });
+
+    await loadPlans();
+
+    const list = document.getElementById('planned-purchases-list');
+    expect(list).not.toBeNull();
+    expect(list!.textContent).toContain('no upfront');
+  });
+
+  test('missing purchase.payment does not throw (guarded, not just escaped)', async () => {
+    (api.getPlannedPurchases as jest.Mock).mockResolvedValue({
+      purchases: [makePurchase(undefined)],
+    });
+
+    await expect(loadPlans()).resolves.not.toThrow();
+  });
+});

frontend/src/__tests__/xss-recommendations-payment.test.ts

@@ -0,0 +1,194 @@
+/**
+ * Regression tests for stored XSS via rec.payment rendered unescaped in the
+ * Opportunities table's payment column (issue #1632).
+ *
+ * recommendations.ts:renderColumnCell used to emit:
+ *   case 'payment': return `<td>${formatPayment(rec.payment)}</td>`;
+ *
+ * while every other string-bearing arm of the same switch (provider,
+ * service, resource_type, capacity, region) is escaped with escapeHtml().
+ * formatPayment() passes any value that is not a key of
+ * PAYMENT_DISPLAY_LABELS straight through, so an unrecognised `payment`
+ * string reached innerHTML verbatim. `payment` is a bare `string` on
+ * LocalRecommendation (types.ts), so nothing on the frontend constrains
+ * its content. The fix wraps the emission site with escapeHtml().
+ */
+
+// Mock the api module
+jest.mock('../api', () => ({
+  getRecommendations: jest.fn(),
+  refreshRecommendations: jest.fn(),
+  listAccounts: jest.fn().mockResolvedValue([]),
+  listAccountsMinimal: jest.fn().mockResolvedValue([]),
+  getConfig: jest.fn().mockResolvedValue({ global: {} }),
+  listAccountServiceOverrides: jest.fn().mockResolvedValue([]),
+}));
+
+jest.mock('../api/recommendations', () => ({
+  getRecommendationDetail: jest.fn().mockResolvedValue({
+    id: 'rec-default',
+    usage_history: [],
+    confidence_bucket: 'low',
+    provenance_note: '',
+  }),
+  getRecommendationsFreshness: jest.fn().mockResolvedValue({
+    last_collected_at: new Date(Date.now() - 60 * 60 * 1000).toISOString(),
+    last_collection_error: null,
+  }),
+  refreshRecommendations: jest.fn().mockResolvedValue({}),
+}));
+
+const mockShowToast = jest.fn<{ dismiss: () => void }, [unknown]>(() => ({ dismiss: jest.fn() }));
+jest.mock('../toast', () => ({
+  showToast: (opts: unknown) => mockShowToast(opts),
+}));
+
+jest.mock('../state', () => ({
+  getCurrentProvider: jest.fn().mockReturnValue('all'),
+  setCurrentProvider: jest.fn(),
+  getCurrentAccountIDs: jest.fn().mockReturnValue([]),
+  setCurrentAccountIDs: jest.fn(),
+  getRecommendations: jest.fn().mockReturnValue([]),
+  getRecommendationByID: jest.fn().mockReturnValue(undefined),
+  setRecommendations: jest.fn(),
+  getSelectedRecommendationIDs: jest.fn().mockReturnValue(new Set()),
+  clearSelectedRecommendations: jest.fn(),
+  addSelectedRecommendation: jest.fn(),
+  removeSelectedRecommendation: jest.fn(),
+  getRecommendationsSort: jest.fn().mockReturnValue({ column: 'savings', direction: 'desc' }),
+  setRecommendationsSort: jest.fn(),
+  getRecommendationsColumnFilters: jest.fn().mockReturnValue({}),
+  setRecommendationsColumnFilter: jest.fn(),
+  clearAllRecommendationsColumnFilters: jest.fn(),
+  getVisibleRecommendations: jest.fn().mockReturnValue([]),
+  setVisibleRecommendations: jest.fn(),
+  getCostPeriod: jest.fn().mockReturnValue('monthly'),
+  setCostPeriod: jest.fn(),
+  getHiddenColumns: jest.fn().mockReturnValue(new Set()),
+  setHiddenColumns: jest.fn(),
+  getCurrentUser: jest.fn().mockReturnValue({ id: 'u-admin', email: 'admin@example.com', groups: ['00000000-0000-5000-8000-000000000001'] }),
+  subscribeProvider: jest.fn(),
+  subscribeAccount: jest.fn(),
+}));
+
+// Use the real escapeHtml so the DOM-based escaping is exercised, not a
+// stub. formatCurrency/formatTerm are stubbed for readable assertions,
+// matching recommendations.test.ts's utils mock shape.
+jest.mock('../utils', () => {
+  const actual = jest.requireActual<typeof import('../utils')>('../utils');
+  return {
+    ...actual,
+    formatCurrency: jest.fn((val: number) => `$${val || 0}`),
+    formatTerm: jest.fn((years: number | null | undefined) =>
+      years == null ? '' : `${years} Year${years === 1 ? '' : 's'}`),
+    populateAccountFilter: jest.fn(() => Promise.resolve()),
+    CURRENCY_DEFAULT_DIGITS: 0,
+  };
+});
+
+import { loadRecommendations } from '../recommendations';
+import * as api from '../api';
+
+const SCRIPT_PAYLOAD = '<script>alert(1)</script>';
+const IMG_PAYLOAD = '"><img src=x onerror="alert(1)">';
+
+function makeRec(payment: string) {
+  return {
+    id: 'rec-1',
+    provider: 'aws',
+    service: 'ec2',
+    resource_type: 't3.medium',
+    region: 'us-east-1',
+    count: 1,
+    term: 1,
+    payment,
+    savings: 100,
+    upfront_cost: 500,
+  };
+}
+
+describe('XSS regression: rec.payment rendered as text, not markup (#1632)', () => {
+  beforeEach(() => {
+    document.body.innerHTML = `
+      <div id="opportunities-tab" class="tab-content active">
+        <div id="recommendations-summary"></div>
+        <div id="recommendations-list"></div>
+      </div>
+      <div id="purchase-modal" class="hidden">
+        <div id="purchase-details"></div>
+        <div class="modal-buttons">
+          <button type="button" id="close-purchase-modal-btn">Cancel</button>
+          <button type="button" id="execute-purchase-btn" class="primary">Send for Approval</button>
+        </div>
+      </div>
+    `;
+    jest.clearAllMocks();
+    jest.useFakeTimers();
+    (api.getRecommendations as jest.Mock).mockResolvedValue({ summary: {}, recommendations: [], regions: [] });
+  });
+
+  afterEach(() => {
+    jest.useRealTimers();
+  });
+
+  test('script-tag payload in rec.payment does not create a <script> element', async () => {
+    (api.getRecommendations as jest.Mock).mockResolvedValue({
+      summary: {},
+      recommendations: [makeRec(SCRIPT_PAYLOAD)],
+      regions: ['us-east-1'],
+    });
+
+    await loadRecommendations();
+
+    expect(document.querySelectorAll('script').length).toBe(0);
+
+    const list = document.getElementById('recommendations-list');
+    expect(list).not.toBeNull();
+    expect(list!.innerHTML).not.toContain('<script>');
+    expect(list!.textContent).toContain(SCRIPT_PAYLOAD);
+  });
+
+  test('img-onerror payload in rec.payment does not inject a live <img> element', async () => {
+    (api.getRecommendations as jest.Mock).mockResolvedValue({
+      summary: {},
+      recommendations: [makeRec(IMG_PAYLOAD)],
+      regions: ['us-east-1'],
+    });
+
+    await loadRecommendations();
+
+    const list = document.getElementById('recommendations-list');
+    expect(list).not.toBeNull();
+    // Strongest assertion: no <img> DOM node was ever parsed out of the cell.
+    expect(list!.querySelectorAll('img').length).toBe(0);
+    expect(list!.textContent).toContain('onerror');
+  });
+
+  test('known payment option "all-upfront" still renders its display label', async () => {
+    (api.getRecommendations as jest.Mock).mockResolvedValue({
+      summary: {},
+      recommendations: [makeRec('all-upfront')],
+      regions: ['us-east-1'],
+    });
+
+    await loadRecommendations();
+
+    const list = document.getElementById('recommendations-list');
+    expect(list).not.toBeNull();
+    expect(list!.textContent).toContain('All Upfront');
+  });
+
+  test('unrecognised payment value falls through formatPayment as escaped text', async () => {
+    (api.getRecommendations as jest.Mock).mockResolvedValue({
+      summary: {},
+      recommendations: [makeRec('some-future-option')],
+      regions: ['us-east-1'],
+    });
+
+    await loadRecommendations();
+
+    const list = document.getElementById('recommendations-list');
+    expect(list).not.toBeNull();
+    expect(list!.textContent).toContain('some-future-option');
+  });
+});