From a80ea4ecb1b2654877e20481be681a696fa1fcec Mon Sep 17 00:00:00 2001
From: Cristian Magherusan-Stanciu <cristi.magherusan@gmail.com>
Date: Sun, 7 Jun 2026 03:07:08 -0700
Subject: [PATCH 1/9] fix(frontend): neutralise provider XSS and dedup
 modal/global listeners (#1034)

H1 - Stored XSS in plans.ts: both planned-purchase row (line 180) and
plan card detail (line 412) injected the API provider string into innerHTML
class attributes and text without whitelist/escaping. Extract shared
providerBadgeClass() and providerBadgeHtml() helpers to utils.ts (matching
the existing whitelist in history.ts:341 and recommendations.ts:2417) and
replace both raw interpolations.

H3 - Event-listener stacking in plans.ts: setupRampScheduleHandlers added
listeners to static modal elements on every modal open. Add
rampHandlersInstalled install-once guard; export _resetRampHandlersForTest()
for test isolation.

D1/L4 - Shared provider badge helper: dashboard.ts was using a loose
alphanumeric regex instead of the strict aws|azure|gcp whitelist. Align on
providerBadgeClass() from utils.ts.

Tests: add regression tests to utils.test.ts (providerBadgeClass/
providerBadgeHtml XSS neutralisation), plans.test.ts (H1 call-through and
H3 install-once dedup), and update utils mocks in dashboard.test.ts and
dashboard-ownership-950.test.ts for the new import.

Note: H2 (users/userList.ts duplicate document change listener) was already
fixed via AbortController in a prior PR merged to feat/multicloud-web-frontend.
---
 .../__tests__/dashboard-ownership-950.test.ts |   1 +
 frontend/src/__tests__/dashboard.test.ts      |   4 +-
 frontend/src/__tests__/plans.test.ts          | 103 +++++++++++++++++-
 frontend/src/__tests__/utils.test.ts          |  78 ++++++++++++-
 frontend/src/dashboard.ts                     |  11 +-
 frontend/src/plans.ts                         |  30 ++++-
 frontend/src/utils.ts                         |  27 +++++
 7 files changed, 241 insertions(+), 13 deletions(-)

diff --git a/frontend/src/__tests__/dashboard-ownership-950.test.ts b/frontend/src/__tests__/dashboard-ownership-950.test.ts
index 335d1ae97..f69c46119 100644
--- a/frontend/src/__tests__/dashboard-ownership-950.test.ts
+++ b/frontend/src/__tests__/dashboard-ownership-950.test.ts
@@ -71,6 +71,7 @@ jest.mock('../utils', () => ({
   getDateParts: jest.fn(() => ({ day: 15, month: 'Jan' })),
   escapeHtml: jest.fn((str) => str || ''),
   populateAccountFilter: jest.fn(() => Promise.resolve()),
+  providerBadgeClass: jest.fn((p) => ['aws', 'azure', 'gcp'].includes((p || '').toLowerCase()) ? (p || '').toLowerCase() : ''),
 }));
 
 import { loadDashboard } from '../dashboard';
diff --git a/frontend/src/__tests__/dashboard.test.ts b/frontend/src/__tests__/dashboard.test.ts
index 476fcdac4..8c2744b54 100644
--- a/frontend/src/__tests__/dashboard.test.ts
+++ b/frontend/src/__tests__/dashboard.test.ts
@@ -105,7 +105,9 @@ jest.mock('../utils', () => ({
   formatCurrency: jest.fn((val) => `$${val || 0}`),
   getDateParts: jest.fn(() => ({ day: 15, month: 'Jan' })),
   escapeHtml: jest.fn((str) => str || ''),
-  populateAccountFilter: jest.fn(() => Promise.resolve())
+  populateAccountFilter: jest.fn(() => Promise.resolve()),
+  // providerBadgeClass added for L4/D1 fix: whitelist-based CSS class helper.
+  providerBadgeClass: jest.fn((p) => ['aws', 'azure', 'gcp'].includes((p || '').toLowerCase()) ? (p || '').toLowerCase() : ''),
 }));
 
 import * as api from '../api';
diff --git a/frontend/src/__tests__/plans.test.ts b/frontend/src/__tests__/plans.test.ts
index 26ca9839c..071c9b5fd 100644
--- a/frontend/src/__tests__/plans.test.ts
+++ b/frontend/src/__tests__/plans.test.ts
@@ -1,7 +1,7 @@
 /**
  * Plans module tests
  */
-import { loadPlans, savePlan, closePlanModal, openCreatePlanModal, openNewPlanModal, closePurchaseModal, setupPlanHandlers } from '../plans';
+import { loadPlans, savePlan, closePlanModal, openCreatePlanModal, openNewPlanModal, closePurchaseModal, setupPlanHandlers, _resetRampHandlersForTest } from '../plans';
 
 // Mock the api module
 jest.mock('../api', () => ({
@@ -108,7 +108,11 @@ jest.mock('../utils', () => ({
   getStatusBadge: jest.fn(() => ({ class: 'active', label: 'Active' })),
   escapeHtml: jest.fn((str) => str || ''),
   formatCurrency: jest.fn((val) => `$${val || 0}`),
-  populateAccountFilter: jest.fn(() => Promise.resolve())
+  populateAccountFilter: jest.fn(() => Promise.resolve()),
+  // providerBadgeHtml added for H1 fix: returns a deterministic span so XSS
+  // regression tests can assert neutralisation without a real DOM escaper.
+  providerBadgeHtml: jest.fn((p) => `<span class="provider-badge ${['aws','azure','gcp'].includes((p||'').toLowerCase()) ? (p||'').toLowerCase() : ''}">${(p||'').toUpperCase()}</span>`),
+  CURRENCY_DEFAULT_DIGITS: 0,
 }));
 
 import * as api from '../api';
@@ -182,6 +186,9 @@ describe('Plans Module', () => {
     jest.clearAllMocks();
     window.alert = jest.fn();
     window.confirm = jest.fn().mockReturnValue(true);
+    // Reset ramp-handlers install-once guard: DOM is rebuilt each beforeEach,
+    // so static modal elements are fresh and need listeners re-attached (H3 fix).
+    _resetRampHandlersForTest();
   });
 
   describe('loadPlans', () => {
@@ -2686,4 +2693,96 @@ describe('Plans Module', () => {
       expect(searchInput.disabled).toBe(false);
     });
   });
+
+  // Regression tests for H3: setupRampScheduleHandlers must not stack duplicate
+  // listeners on static modal elements across multiple modal opens.
+  // Pre-fix: opening the plan modal N times would attach N handlers to each
+  // element, so a single radio change would fire updateCommitmentOptions N times.
+  describe('H3 regression: setupRampScheduleHandlers install-once guard', () => {
+    test('provider-change handler fires exactly once after opening modal 3 times', () => {
+      const { populateTermSelect: mockPopulate } = jest.requireMock('../commitmentOptions') as {
+        populateTermSelect: jest.Mock;
+      };
+
+      // beforeEach already called _resetRampHandlersForTest() so the flag is clear.
+      // Open the modal 3 times without resetting between them.
+      openNewPlanModal(); // first open: installs handlers (rampHandlersInstalled = true)
+      openNewPlanModal(); // second open: guard returns early -- no re-attach
+      openNewPlanModal(); // third open: guard returns early -- no re-attach
+
+      mockPopulate.mockClear(); // clear any calls from setup above
+
+      // Dispatch one change event on the provider select.
+      const providerSelect = document.getElementById('plan-provider') as HTMLSelectElement;
+      providerSelect.value = 'azure';
+      providerSelect.dispatchEvent(new Event('change'));
+
+      // populateTermSelect is called exactly once from updateCommitmentOptions.
+      // Pre-fix (no guard) it would be called 3 times (once per open).
+      expect(mockPopulate).toHaveBeenCalledTimes(1);
+    });
+
+    test('ramp schedule radio handler fires exactly once after 2 modal opens', () => {
+      // The radio-change handler calls updatePlanNameFromSchedule which reads
+      // the plan-name input. Count invocations via a side-effect on populateTermSelect.
+      const { populateTermSelect: mockPopulate } = jest.requireMock('../commitmentOptions') as {
+        populateTermSelect: jest.Mock;
+      };
+
+      // beforeEach called _resetRampHandlersForTest(); open twice without reset between.
+      openNewPlanModal(); // installs
+      openNewPlanModal(); // guard: no re-install
+
+      mockPopulate.mockClear();
+
+      // The provider-select change handler calls updateCommitmentOptions -> populateTermSelect.
+      const providerSelect = document.getElementById('plan-provider') as HTMLSelectElement;
+      providerSelect.value = 'gcp';
+      providerSelect.dispatchEvent(new Event('change'));
+
+      // With stacked handlers it would fire twice (once per open).
+      // With the guard it fires exactly once.
+      expect(mockPopulate).toHaveBeenCalledTimes(1);
+    });
+  });
+
+  // Regression for H1: providerBadgeHtml must be used for planned-purchase rows
+  // and plan card detail (prevents XSS via class attribute injection).
+  describe('H1 regression: provider badge XSS neutralisation in plans.ts', () => {
+    test('providerBadgeHtml is called with the purchase provider for planned-purchase rows', async () => {
+      const mockBadge = (jest.requireMock('../utils') as { providerBadgeHtml: jest.Mock }).providerBadgeHtml;
+
+      const maliciousProvider = 'aws"><img src=x onerror=alert(1)>';
+      (api.getPlannedPurchases as jest.Mock).mockResolvedValue({
+        purchases: [
+          {
+            id: 'pp-1',
+            plan_id: 'plan-1',
+            plan_name: 'Test Plan',
+            provider: maliciousProvider,
+            service: 'ec2',
+            resource_type: 'Standard',
+            region: 'us-east-1',
+            count: 1,
+            term: 1,
+            payment: 'no-upfront',
+            upfront_cost: 0,
+            estimated_savings: 50,
+            scheduled_date: '2024-06-01',
+            status: 'pending',
+            step_number: 1,
+            total_steps: 4,
+          }
+        ]
+      });
+      (api.getPlans as jest.Mock).mockResolvedValue({ plans: [] });
+
+      await loadPlans();
+
+      // providerBadgeHtml must have been called with the malicious string.
+      // Pre-fix the string was raw-interpolated into a class attribute via innerHTML.
+      // Post-fix it goes through the helper which whitelists and escapes it.
+      expect(mockBadge).toHaveBeenCalledWith(maliciousProvider);
+    });
+  });
 });
diff --git a/frontend/src/__tests__/utils.test.ts b/frontend/src/__tests__/utils.test.ts
index ed5fcf2df..9b2f9bf49 100644
--- a/frontend/src/__tests__/utils.test.ts
+++ b/frontend/src/__tests__/utils.test.ts
@@ -16,7 +16,9 @@ import {
   isValidEmail,
   formatRampSchedule,
   getStatusBadge,
-  calculatePaybackMonths
+  calculatePaybackMonths,
+  providerBadgeClass,
+  providerBadgeHtml
 } from '../utils';
 
 describe('formatCurrency', () => {
@@ -363,3 +365,77 @@ describe('calculatePaybackMonths', () => {
     expect(calculatePaybackMonths(-100, 100)).toBe(0);
   });
 });
+
+// Regression tests for H1/D1/L4: providerBadgeClass whitelist.
+// Pre-fix the class was interpolated raw from the API (plans.ts) or allowed any
+// alphanumeric string (dashboard.ts).  Post-fix all sites go through this
+// helper which enforces the aws|azure|gcp allow-list.
+describe('providerBadgeClass', () => {
+  test('returns lowercase provider for known aws', () => {
+    expect(providerBadgeClass('aws')).toBe('aws');
+    expect(providerBadgeClass('AWS')).toBe('aws');
+  });
+
+  test('returns lowercase provider for known azure', () => {
+    expect(providerBadgeClass('azure')).toBe('azure');
+    expect(providerBadgeClass('Azure')).toBe('azure');
+  });
+
+  test('returns lowercase provider for known gcp', () => {
+    expect(providerBadgeClass('gcp')).toBe('gcp');
+  });
+
+  test('returns empty string for unknown provider (XSS payload neutralised)', () => {
+    // Regression: pre-fix this value would be injected verbatim as a CSS class.
+    expect(providerBadgeClass('aws"><img src=x onerror=alert(1)>')).toBe('');
+    expect(providerBadgeClass('evil-class')).toBe('');
+    expect(providerBadgeClass('unknown')).toBe('');
+  });
+
+  test('returns empty string for null/undefined/empty', () => {
+    expect(providerBadgeClass(null)).toBe('');
+    expect(providerBadgeClass(undefined)).toBe('');
+    expect(providerBadgeClass('')).toBe('');
+  });
+});
+
+// Regression tests for H1: providerBadgeHtml XSS in plans.ts.
+// Pre-fix: plans.ts interpolated provider raw into class + textContent via innerHTML.
+// Post-fix: class is whitelisted; text is HTML-escaped.
+describe('providerBadgeHtml', () => {
+  test('renders known provider with correct class and uppercased label', () => {
+    const html = providerBadgeHtml('aws');
+    expect(html).toContain('class="provider-badge aws"');
+    expect(html).toContain('>AWS<');
+  });
+
+  test('renders azure with correct class', () => {
+    const html = providerBadgeHtml('azure');
+    expect(html).toContain('class="provider-badge azure"');
+    expect(html).toContain('>AZURE<');
+  });
+
+  test('neutralises XSS payload in class attribute position', () => {
+    // The raw value would previously be injected as: class="provider-badge <payload>"
+    const payload = 'aws"><img src=x onerror=alert(document.cookie)>';
+    const html = providerBadgeHtml(payload);
+    // Class must not contain the raw payload.
+    expect(html).not.toContain(payload);
+    // No <img> or onerror in the output.
+    expect(html).not.toContain('<img');
+    expect(html).not.toContain('onerror');
+    // Class falls back to just provider-badge (no extra class token).
+    expect(html).toMatch(/class="provider-badge"/);
+  });
+
+  test('HTML-escapes text content of unknown provider', () => {
+    const html = providerBadgeHtml('<script>alert(1)</script>');
+    expect(html).not.toContain('<script>');
+    expect(html).toContain('&lt;SCRIPT&gt;');
+  });
+
+  test('handles null/undefined gracefully', () => {
+    expect(providerBadgeHtml(null)).toContain('provider-badge');
+    expect(providerBadgeHtml(undefined)).toContain('provider-badge');
+  });
+});
diff --git a/frontend/src/dashboard.ts b/frontend/src/dashboard.ts
index 7a6cfbf60..4f915292f 100644
--- a/frontend/src/dashboard.ts
+++ b/frontend/src/dashboard.ts
@@ -5,7 +5,7 @@
 import { Chart, registerables } from 'chart.js';
 import * as api from './api';
 import * as state from './state';
-import { formatCurrency, getDateParts } from './utils';
+import { formatCurrency, getDateParts, providerBadgeClass } from './utils';
 import type { DashboardSummary, UpcomingPurchase, ServiceSavings, LocalRecommendation } from './types';
 import type { SavingsDataPoint } from './api';
 import { showToast } from './toast';
@@ -444,10 +444,11 @@ function renderUpcomingPurchases(purchases: UpcomingPurchase[]): void {
     const descP = document.createElement('p');
     const badge = document.createElement('span');
     badge.className = 'provider-badge';
-    // Whitelist provider to a CSS class — only alphanumeric + hyphen allowed
-    const safeProvider = /^[a-z0-9-]+$/i.test(p.provider) ? p.provider : 'unknown';
-    badge.classList.add(safeProvider);
-    badge.textContent = p.provider.toUpperCase();
+    // Use shared whitelist helper (providerBadgeClass) for consistency with
+    // plans.ts and history.ts (D1 / L4 -- aligns on aws|azure|gcp set).
+    const safeProvider = providerBadgeClass(p.provider);
+    if (safeProvider) badge.classList.add(safeProvider);
+    badge.textContent = (p.provider || '').toUpperCase();
     descP.appendChild(badge);
     descP.appendChild(document.createTextNode(` ${p.service} - Step ${p.step_number} of ${p.total_steps}`));
     details.appendChild(h4);
diff --git a/frontend/src/plans.ts b/frontend/src/plans.ts
index c798d38bc..d4e76fc1a 100644
--- a/frontend/src/plans.ts
+++ b/frontend/src/plans.ts
@@ -4,7 +4,7 @@
 
 import * as api from './api';
 import * as state from './state';
-import { formatDate, formatTerm, getStatusBadge, escapeHtml, formatCurrency, CURRENCY_DEFAULT_DIGITS } from './utils';
+import { formatDate, formatTerm, getStatusBadge, escapeHtml, formatCurrency, CURRENCY_DEFAULT_DIGITS, providerBadgeHtml } from './utils';
 import { showToast } from './toast';
 import { confirmDialog } from './confirmDialog';
 import type { PlansResponse, LocalPlan, SavePlanData } from './types';
@@ -27,6 +27,12 @@ import { parseNumericFilter, applyColumnFilters as applyColumnFiltersLib } from
 // CR follow-up.
 let pendingPlanRecommendations: api.Recommendation[] = [];
 
+// Install-once guard for setupRampScheduleHandlers. The elements it binds are
+// static modal singletons that are never replaced, so adding listeners on
+// every modal open stacks duplicate handlers. The flag ensures we bind once;
+// reset to false only in tests via resetRampHandlersForTest().
+let rampHandlersInstalled = false;
+
 /**
  * Load plans and planned purchases
  */
@@ -713,7 +719,7 @@ function renderPlannedPurchaseRow(purchase: PlannedPurchase): string {
         <span class="step-info">Step ${purchase.step_number}/${purchase.total_steps}</span>
       </td>
       <td>${formatDate(purchase.scheduled_date)}</td>
-      <td><span class="provider-badge ${purchase.provider}">${purchase.provider.toUpperCase()}</span></td>
+      <td>${providerBadgeHtml(purchase.provider)}</td>
       <td>${escapeHtml(purchase.service)}</td>
       <td>${escapeHtml(purchase.resource_type)} (${escapeHtml(purchase.region)})</td>
       <td>${purchase.count}</td>
@@ -950,7 +956,7 @@ function renderPlanCard(plan: BackendPlan, canManagePlan: boolean, canDeletePlan
         <div class="plan-details">
           <div class="plan-detail">
             <span class="plan-detail-label">Provider</span>
-            <span class="plan-detail-value"><span class="provider-badge ${info.provider}">${info.provider.toUpperCase()}</span></span>
+            <span class="plan-detail-value">${providerBadgeHtml(info.provider)}</span>
           </div>
           <div class="plan-detail">
             <span class="plan-detail-label">Service</span>
@@ -1748,9 +1754,14 @@ function wirePlanRangeInputs(): void {
 }
 
 /**
- * Set up event handlers for ramp schedule changes
+ * Set up event handlers for ramp schedule changes.
+ * Guarded by rampHandlersInstalled so re-opening the plan modal does not
+ * stack duplicate listeners on the static modal elements (H3, feedback_event_listener_dedup).
  */
 function setupRampScheduleHandlers(): void {
+  if (rampHandlersInstalled) return;
+  rampHandlersInstalled = true;
+
   // Listen to ramp schedule radio changes
   document.querySelectorAll<HTMLInputElement>('input[name="ramp-schedule"]').forEach(radio => {
     radio.addEventListener('change', () => {
@@ -2148,3 +2159,14 @@ function updateServiceDropdownForProvider(provider: string): void {
     serviceSelect.dispatchEvent(new Event('change'));
   }
 }
+
+/**
+ * Reset the ramp-handlers install-once guard.
+ * Exported for unit tests only: each test rebuilds the DOM so the static
+ * modal elements are fresh; without this reset the guard prevents listener
+ * re-attachment and tests that open the modal multiple times break.
+ * Must NOT be called in production code.
+ */
+export function _resetRampHandlersForTest(): void {
+  rampHandlersInstalled = false;
+}
diff --git a/frontend/src/utils.ts b/frontend/src/utils.ts
index 3302cc686..91c09b893 100644
--- a/frontend/src/utils.ts
+++ b/frontend/src/utils.ts
@@ -285,3 +285,30 @@ export function calculatePaybackMonths(upfrontCost: number, monthlySavings: numb
   if (!upfrontCost || upfrontCost <= 0) return 0;
   return Math.ceil(upfrontCost / monthlySavings);
 }
+
+/** Canonical set of known cloud providers used for whitelist checks. */
+const KNOWN_PROVIDERS = ['aws', 'azure', 'gcp'] as const;
+
+/**
+ * Return the whitelisted CSS class name for a provider value.
+ *
+ * Whitelist guards against stored XSS when provider strings are interpolated
+ * into innerHTML class attributes (findings H1/L4, issues #443 / CR #253).
+ * An unrecognised value produces an empty string (no badge modifier class).
+ */
+export function providerBadgeClass(provider: string | null | undefined): string {
+  if (!provider) return '';
+  const n = provider.toLowerCase();
+  return (KNOWN_PROVIDERS as readonly string[]).includes(n) ? n : '';
+}
+
+/**
+ * Render a `<span class="provider-badge ...">LABEL</span>` string for use in
+ * innerHTML templates.  Both the CSS class and the text label are sanitised:
+ * the class is whitelisted to known providers and the label is HTML-escaped.
+ */
+export function providerBadgeHtml(provider: string | null | undefined): string {
+  const cls = providerBadgeClass(provider);
+  const label = escapeHtml((provider || '').toUpperCase());
+  return `<span class="provider-badge${cls ? ` ${cls}` : ''}">${label}</span>`;
+}

From 98fc09339eeb3bbce938ce2d9a0a98dd098413ed Mon Sep 17 00:00:00 2001
From: Cristian Magherusan-Stanciu <cristi.magherusan@gmail.com>
Date: Sun, 7 Jun 2026 04:33:20 -0700
Subject: [PATCH 2/9] fix(frontend): escape group.id and purchase/plan ids in
 innerHTML (11-M2, 11-L1)

Wrap group.id in escapeHtml() in updateGroupFilterDropdown (finding 11-M2):
the option value attribute was injecting the raw API value without escaping,
violating the feedback_innerhtml_xss policy.

Escape plan.id and purchase.id in all data-id attributes inside the plan-card
and planned-purchase-row innerHTML templates (finding 11-L1). UUIDs are safe
in practice but must be escaped for consistency and defence-in-depth.

Adds regression test in users.test.ts asserting the group.id XSS payload is
not emitted verbatim into the rendered option element.

Closes part of #1065
---
 frontend/src/__tests__/users.test.ts | 18 ++++++
 frontend/src/plans.ts                | 85 +++++++++++++++++++++-------
 frontend/src/users/filters.ts        |  2 +-
 3 files changed, 85 insertions(+), 20 deletions(-)

diff --git a/frontend/src/__tests__/users.test.ts b/frontend/src/__tests__/users.test.ts
index 701f29492..0e3d78afe 100644
--- a/frontend/src/__tests__/users.test.ts
+++ b/frontend/src/__tests__/users.test.ts
@@ -656,6 +656,24 @@ describe('users/filters', () => {
       const select = document.getElementById('user-group-filter') as HTMLSelectElement;
       expect(select.innerHTML).toContain('&lt;script&gt;');
     });
+
+    it('11-M2: escapes HTML special chars in group id (value attribute)', () => {
+      // Regression: group.id was injected raw into value="..." in the innerHTML
+      // template. An id containing quotes or angle brackets could break the
+      // option element boundary. Verify escapeHtml() is applied to the id too.
+      userState.setAvailableGroups([
+        { id: '"><img src=x onerror=alert(1)>', name: 'Evil Group', permissions: [], description: '' }
+      ] as any);
+
+      userFilters.updateGroupFilterDropdown();
+
+      const select = document.getElementById('user-group-filter') as HTMLSelectElement;
+      // The raw payload must not appear verbatim inside the rendered HTML.
+      expect(select.innerHTML).not.toContain('"><img src=x');
+      // The option's value should be the escaped form.
+      const opt = select.options[1];
+      expect(opt?.value).not.toContain('<img');
+    });
   });
 });
 
diff --git a/frontend/src/plans.ts b/frontend/src/plans.ts
index d4e76fc1a..0a2e53cf7 100644
--- a/frontend/src/plans.ts
+++ b/frontend/src/plans.ts
@@ -728,11 +728,11 @@ function renderPlannedPurchaseRow(purchase: PlannedPurchase): string {
       <td class="savings">${formatCurrency(purchase.estimated_savings)}/mo</td>
       <td><span class="status-badge ${statusClass}">${escapeHtml(purchase.status)}</span></td>
       <td class="actions">
-        ${canRunPurchase ? `<button data-action="run" data-id="${purchase.id}" class="btn-small primary" title="Run now">▶</button>` : ''}
-        ${canPauseOrResumePurchase && isPending ? `<button data-action="pause" data-id="${purchase.id}" class="btn-small" title="Pause">⏸</button>` : ''}
-        ${canPauseOrResumePurchase && isPaused ? `<button data-action="resume" data-id="${purchase.id}" class="btn-small" title="Resume">⏵</button>` : ''}
-        ${canEditPlan ? `<button data-action="edit" data-id="${purchase.id}" data-plan-id="${purchase.plan_id}" class="btn-small" title="Edit Plan">✎</button>` : ''}
-        ${canDisablePlan ? `<button data-action="disable" data-id="${purchase.id}" class="btn-small danger" title="Disable Plan">✕</button>` : ''}
+        ${canRunPurchase ? `<button data-action="run" data-id="${escapeHtml(purchase.id)}" class="btn-small primary" title="Run now">▶</button>` : ''}
+        ${canPauseOrResumePurchase && isPending ? `<button data-action="pause" data-id="${escapeHtml(purchase.id)}" class="btn-small" title="Pause">⏸</button>` : ''}
+        ${canPauseOrResumePurchase && isPaused ? `<button data-action="resume" data-id="${escapeHtml(purchase.id)}" class="btn-small" title="Resume">⏵</button>` : ''}
+        ${canEditPlan ? `<button data-action="edit" data-id="${escapeHtml(purchase.id)}" data-plan-id="${escapeHtml(purchase.plan_id)}" class="btn-small" title="Edit Plan">✎</button>` : ''}
+        ${canDisablePlan ? `<button data-action="disable" data-id="${escapeHtml(purchase.id)}" class="btn-small danger" title="Disable Plan">✕</button>` : ''}
       </td>
     </tr>
   `;
@@ -758,12 +758,20 @@ function getPlannedPurchaseStatusClass(status: string): string {
 async function handlePlannedPurchaseAction(action: string, purchaseId: string, planId = ''): Promise<void> {
   try {
     switch (action) {
-      case 'run':
-        if (confirm('Run this purchase now? This will immediately execute the purchase.')) {
+      case 'run': {
+        // Use styled async dialog (11-L2) instead of blocking browser confirm().
+        const runOk = await confirmDialog({
+          title: 'Run purchase now?',
+          body: 'This will immediately execute the purchase.',
+          confirmLabel: 'Run now',
+          destructive: true,
+        });
+        if (runOk) {
           await api.runPlannedPurchase(purchaseId);
           showToast({ message: 'Purchase executed successfully', kind: 'success', timeout: 5_000 });
         }
         break;
+      }
       case 'pause':
         // Pause is reversible and scoped to a single execution: the plan stays
         // enabled (unlike Disable plan) and the row stays listed with a Paused
@@ -784,14 +792,22 @@ async function handlePlannedPurchaseAction(action: string, purchaseId: string, p
         }
         await editPlan(planId);
         return;
-      case 'disable':
-        if (confirm('Disable this plan? The plan will be paused and no purchases will be scheduled. You can re-enable it later from the Plans list.')) {
+      case 'disable': {
+        // Use styled async dialog (11-L2) instead of blocking browser confirm().
+        const disableOk = await confirmDialog({
+          title: 'Disable this plan?',
+          body: 'The plan will be paused and no purchases will be scheduled. You can re-enable it later from the Plans list.',
+          confirmLabel: 'Disable plan',
+          destructive: true,
+        });
+        if (disableOk) {
           await api.deletePlannedPurchase(purchaseId);
           // Reload full plans list since we disabled a plan
           await loadPlans();
           return;
         }
         break;
+      }
     }
     await loadPlannedPurchases();
   } catch (error) {
@@ -946,7 +962,7 @@ function renderPlanCard(plan: BackendPlan, canManagePlan: boolean, canDeletePlan
           ${overdueBadge}
           ${canManagePlan && !isUnassigned ? `
           <label class="toggle-label">
-            <input type="checkbox" data-action="toggle-plan" data-id="${plan.id}" ${plan.enabled ? 'checked' : ''}>
+            <input type="checkbox" data-action="toggle-plan" data-id="${escapeHtml(plan.id)}" ${plan.enabled ? 'checked' : ''}>
             <span class="slider"></span>
           </label>
           ` : ''}
@@ -986,10 +1002,10 @@ function renderPlanCard(plan: BackendPlan, canManagePlan: boolean, canDeletePlan
           ` : ''}
         </div>
         <div class="plan-actions">
-          ${canManagePlan && !isUnassigned ? `<button data-action="add-purchases" data-id="${plan.id}" data-name="${escapeHtml(plan.name)}" class="primary">Add Purchases</button>` : ''}
-          ${canManagePlan && !isUnassigned ? `<button data-action="edit-plan" data-id="${plan.id}">Edit</button>` : ''}
-          <button data-action="view-history" data-id="${plan.id}" class="secondary">History</button>
-          ${canDeletePlan ? `<button data-action="delete-plan" data-id="${plan.id}" class="danger">Delete</button>` : ''}
+          ${canManagePlan && !isUnassigned ? `<button data-action="add-purchases" data-id="${escapeHtml(plan.id)}" data-name="${escapeHtml(plan.name)}" class="primary">Add Purchases</button>` : ''}
+          ${canManagePlan && !isUnassigned ? `<button data-action="edit-plan" data-id="${escapeHtml(plan.id)}">Edit</button>` : ''}
+          <button data-action="view-history" data-id="${escapeHtml(plan.id)}" class="secondary">History</button>
+          ${canDeletePlan ? `<button data-action="delete-plan" data-id="${escapeHtml(plan.id)}" class="danger">Delete</button>` : ''}
         </div>
       </div>
     </div>
@@ -1182,23 +1198,54 @@ export async function savePlan(e: Event): Promise<void> {
   const rampScheduleRadio = document.querySelector<HTMLInputElement>('input[name="ramp-schedule"]:checked');
   const rampSchedule = rampScheduleRadio?.value || 'immediate';
 
+  // Parse and validate integer fields up front. Use Number() not parseInt so
+  // fractions like "2.5" fail Number.isInteger() rather than silently truncating
+  // to 2. Mirrors the strict parse pattern from handleAddPurchases and settings.ts
+  // validatePurchasingSettings (feedback_strict_int_parse, finding 11-M1).
+  const rawTerm = Number((document.getElementById('plan-term') as HTMLSelectElement).value);
+  const rawCoverage = Number((document.getElementById('plan-coverage') as HTMLInputElement).value);
+  const rawNotifyDays = Number((document.getElementById('plan-notify-days') as HTMLInputElement).value);
+
+  if (!Number.isFinite(rawTerm) || !Number.isInteger(rawTerm) || rawTerm < 1) {
+    showToast({ message: 'Term must be a valid whole number of years', kind: 'error' });
+    return;
+  }
+  if (!Number.isFinite(rawCoverage) || !Number.isInteger(rawCoverage) || rawCoverage < 0 || rawCoverage > 100) {
+    showToast({ message: 'Target Coverage must be a whole number between 0 and 100', kind: 'error' });
+    return;
+  }
+  if (!Number.isFinite(rawNotifyDays) || !Number.isInteger(rawNotifyDays) || rawNotifyDays < 1 || rawNotifyDays > 30) {
+    showToast({ message: 'Notification Days must be a whole number between 1 and 30', kind: 'error' });
+    return;
+  }
+
   const plan: SavePlanData = {
     name: (document.getElementById('plan-name') as HTMLInputElement).value,
     description: (document.getElementById('plan-description') as HTMLTextAreaElement).value,
     provider: (document.getElementById('plan-provider') as HTMLSelectElement).value,
     service: (document.getElementById('plan-service') as HTMLSelectElement).value,
-    term: parseInt((document.getElementById('plan-term') as HTMLSelectElement).value, 10),
+    term: rawTerm,
     payment: (document.getElementById('plan-payment') as HTMLSelectElement).value,
-    target_coverage: parseInt((document.getElementById('plan-coverage') as HTMLInputElement).value, 10),
+    target_coverage: rawCoverage,
     ramp_schedule: rampSchedule,
     auto_purchase: (document.getElementById('plan-auto-purchase') as HTMLInputElement).checked,
-    notification_days_before: parseInt((document.getElementById('plan-notify-days') as HTMLInputElement).value, 10),
+    notification_days_before: rawNotifyDays,
     enabled: (document.getElementById('plan-enabled') as HTMLInputElement).checked
   };
 
   if (rampSchedule === 'custom') {
-    plan.custom_step_percent = parseInt((document.getElementById('ramp-step-percent') as HTMLInputElement).value, 10);
-    plan.custom_interval_days = parseInt((document.getElementById('ramp-interval-days') as HTMLInputElement).value, 10);
+    const rawStepPercent = Number((document.getElementById('ramp-step-percent') as HTMLInputElement).value);
+    const rawIntervalDays = Number((document.getElementById('ramp-interval-days') as HTMLInputElement).value);
+    if (!Number.isFinite(rawStepPercent) || !Number.isInteger(rawStepPercent) || rawStepPercent < 1 || rawStepPercent > 100) {
+      showToast({ message: 'Ramp Step Percent must be a whole number between 1 and 100', kind: 'error' });
+      return;
+    }
+    if (!Number.isFinite(rawIntervalDays) || !Number.isInteger(rawIntervalDays) || rawIntervalDays < 1 || rawIntervalDays > 365) {
+      showToast({ message: 'Ramp Interval Days must be a whole number between 1 and 365', kind: 'error' });
+      return;
+    }
+    plan.custom_step_percent = rawStepPercent;
+    plan.custom_interval_days = rawIntervalDays;
   }
 
   // Use the snapshot stamped at Plan-button click time (#273 CR follow-up).
diff --git a/frontend/src/users/filters.ts b/frontend/src/users/filters.ts
index 08156a81e..e1f111b2f 100644
--- a/frontend/src/users/filters.ts
+++ b/frontend/src/users/filters.ts
@@ -125,7 +125,7 @@ export function updateGroupFilterDropdown(): void {
   groupFilterEl.innerHTML = `
     <option value="">All Groups</option>
     ${availableGroups.map(group => `
-      <option value="${group.id}">${escapeHtml(group.name)}</option>
+      <option value="${escapeHtml(group.id)}">${escapeHtml(group.name)}</option>
     `).join('')}
   `;
   groupFilterEl.value = currentValue;

From 37116cce42b903148d0c7566c9a394e3d23919c9 Mon Sep 17 00:00:00 2001
From: Cristian Magherusan-Stanciu <cristi.magherusan@gmail.com>
Date: Sun, 7 Jun 2026 04:33:33 -0700
Subject: [PATCH 3/9] fix(frontend): validate plan and permission numeric
 fields strictly (11-M1, 11-M3)

savePlan now validates term, target_coverage, notification_days_before, and
custom ramp fields with Number.isInteger(Number(raw)) before building the
request object (finding 11-M1). Fractional or NaN inputs show a toast error
and abort without calling the API, matching the pattern in settings.ts
validatePurchasingSettings and feedback_strict_int_parse.

collectPermissions in groupModals.ts guards max_amount with
Number.isFinite(parsed) && parsed >= 0 before assigning to the constraint
object (finding 11-M3). Infinity, NaN, and negative values are silently
skipped so the rest of the permission constraints still reach the API.

Adds regression tests in plans.test.ts (11-M1) and groups.test.ts (11-M3)
that verify the guards reject bad inputs and accept valid values.

Closes part of #1065
---
 frontend/src/__tests__/groups.test.ts | 101 ++++++++++++++++++++++++++
 frontend/src/__tests__/plans.test.ts  |  80 ++++++++++++++++++--
 frontend/src/groups/groupModals.ts    |  12 ++-
 3 files changed, 184 insertions(+), 9 deletions(-)

diff --git a/frontend/src/__tests__/groups.test.ts b/frontend/src/__tests__/groups.test.ts
index cd8054ea8..81f729b31 100644
--- a/frontend/src/__tests__/groups.test.ts
+++ b/frontend/src/__tests__/groups.test.ts
@@ -1248,3 +1248,104 @@ describe('groups/groupModals — duplicate', () => {
     });
   });
 });
+
+// ---------------------------------------------------------------------------
+// Regression tests for finding 11-M3: max_amount must be validated before
+// parseFloat assignment in collectPermissions.
+// ---------------------------------------------------------------------------
+describe('11-M3 max_amount validation in collectPermissions', () => {
+  // Access collectPermissions indirectly via saveGroup and observe API call.
+
+  it('rejects Infinity max_amount (does not forward it to API)', async () => {
+    const modal = document.createElement('div');
+    modal.id = 'group-modal';
+    modal.innerHTML = `
+      <input id="group-id" value="">
+      <input id="group-name" value="Test">
+      <textarea id="group-description"></textarea>
+      <div id="permissions-list">
+        <div class="permission-item">
+          <select class="perm-action"><option value="view" selected>View</option></select>
+          <select class="perm-resource"><option value="plans" selected>Plans</option></select>
+          <input class="perm-providers" value="">
+          <input class="perm-services" value="">
+          <input class="perm-regions" value="">
+          <input class="perm-max-amount" type="number" min="0" value="Infinity">
+        </div>
+      </div>
+    `;
+    document.body.appendChild(modal);
+
+    (api.createGroup as jest.Mock).mockResolvedValue({ id: 'new-id', name: 'Test', description: '', permissions: [] });
+
+    const event = new Event('submit', { cancelable: true });
+    await groupModals.saveGroup(event);
+
+    expect(api.createGroup).toHaveBeenCalled();
+    const call = (api.createGroup as jest.Mock).mock.calls[0][0] as { permissions: Array<{ constraints?: { max_amount?: number } }> };
+    // max_amount must not be Infinity (non-finite rejected).
+    const maxAmount = call.permissions[0]?.constraints?.max_amount;
+    expect(maxAmount).toBeUndefined();
+  });
+
+  it('accepts a valid positive max_amount', async () => {
+    document.body.innerHTML = '';
+    const modal = document.createElement('div');
+    modal.id = 'group-modal';
+    modal.innerHTML = `
+      <input id="group-id" value="">
+      <input id="group-name" value="Test">
+      <textarea id="group-description"></textarea>
+      <div id="permissions-list">
+        <div class="permission-item">
+          <select class="perm-action"><option value="view" selected>View</option></select>
+          <select class="perm-resource"><option value="plans" selected>Plans</option></select>
+          <input class="perm-providers" value="">
+          <input class="perm-services" value="">
+          <input class="perm-regions" value="">
+          <input class="perm-max-amount" type="number" min="0" value="5000">
+        </div>
+      </div>
+    `;
+    document.body.appendChild(modal);
+
+    (api.createGroup as jest.Mock).mockResolvedValue({ id: 'new-id', name: 'Test', description: '', permissions: [] });
+
+    const event = new Event('submit', { cancelable: true });
+    await groupModals.saveGroup(event);
+
+    const call = (api.createGroup as jest.Mock).mock.calls[0][0] as { permissions: Array<{ constraints?: { max_amount?: number } }> };
+    expect(call.permissions[0]?.constraints?.max_amount).toBe(5000);
+  });
+
+  it('rejects negative max_amount (does not forward it to API)', async () => {
+    document.body.innerHTML = '';
+    const modal = document.createElement('div');
+    modal.id = 'group-modal';
+    modal.innerHTML = `
+      <input id="group-id" value="">
+      <input id="group-name" value="Test">
+      <textarea id="group-description"></textarea>
+      <div id="permissions-list">
+        <div class="permission-item">
+          <select class="perm-action"><option value="view" selected>View</option></select>
+          <select class="perm-resource"><option value="plans" selected>Plans</option></select>
+          <input class="perm-providers" value="">
+          <input class="perm-services" value="">
+          <input class="perm-regions" value="">
+          <input class="perm-max-amount" type="number" value="-100">
+        </div>
+      </div>
+    `;
+    document.body.appendChild(modal);
+
+    (api.createGroup as jest.Mock).mockResolvedValue({ id: 'new-id', name: 'Test', description: '', permissions: [] });
+
+    const event = new Event('submit', { cancelable: true });
+    await groupModals.saveGroup(event);
+
+    const call = (api.createGroup as jest.Mock).mock.calls[0][0] as { permissions: Array<{ constraints?: { max_amount?: number } }> };
+    // Negative value must be silently skipped.
+    expect(call.permissions[0]?.constraints?.max_amount).toBeUndefined();
+  });
+});
diff --git a/frontend/src/__tests__/plans.test.ts b/frontend/src/__tests__/plans.test.ts
index 071c9b5fd..d24b28f6c 100644
--- a/frontend/src/__tests__/plans.test.ts
+++ b/frontend/src/__tests__/plans.test.ts
@@ -186,6 +186,9 @@ describe('Plans Module', () => {
     jest.clearAllMocks();
     window.alert = jest.fn();
     window.confirm = jest.fn().mockReturnValue(true);
+    // Default confirmDialog to confirmed so tests that don't override it
+    // behave like the old window.confirm() returning true (finding 11-L2).
+    mockConfirmDialog.mockResolvedValue(true);
     // Reset ramp-handlers install-once guard: DOM is rebuilt each beforeEach,
     // so static modal elements are fresh and need listeners re-attached (H3 fix).
     _resetRampHandlersForTest();
@@ -877,20 +880,22 @@ describe('Plans Module', () => {
     test('run action executes purchase with confirmation', async () => {
       (api.runPlannedPurchase as jest.Mock).mockResolvedValue({});
       (api.getPlannedPurchases as jest.Mock).mockResolvedValue({ purchases: [] });
-      window.confirm = jest.fn().mockReturnValue(true);
+      // confirm() replaced by confirmDialog() (11-L2); control via mockConfirmDialog.
+      mockConfirmDialog.mockResolvedValue(true);
 
       const runBtn = document.querySelector('[data-action="run"]') as HTMLButtonElement;
       runBtn?.click();
 
       await new Promise(resolve => setTimeout(resolve, 50));
 
-      expect(window.confirm).toHaveBeenCalled();
+      expect(mockConfirmDialog).toHaveBeenCalled();
       expect(api.runPlannedPurchase).toHaveBeenCalledWith('purchase-1');
       expect(mockShowToast).toHaveBeenCalledWith(expect.objectContaining({ message: 'Purchase executed successfully' }));
     });
 
     test('run action cancelled by user', async () => {
-      window.confirm = jest.fn().mockReturnValue(false);
+      // confirm() replaced by confirmDialog() (11-L2); control via mockConfirmDialog.
+      mockConfirmDialog.mockResolvedValue(false);
 
       const runBtn = document.querySelector('[data-action="run"]') as HTMLButtonElement;
       runBtn?.click();
@@ -919,7 +924,8 @@ describe('Plans Module', () => {
       (api.deletePlannedPurchase as jest.Mock).mockResolvedValue({});
       (api.getPlans as jest.Mock).mockResolvedValue({ plans: [] });
       (api.getPlannedPurchases as jest.Mock).mockResolvedValue({ purchases: [] });
-      window.confirm = jest.fn().mockReturnValue(true);
+      // confirm() replaced by confirmDialog() (11-L2); control via mockConfirmDialog.
+      mockConfirmDialog.mockResolvedValue(true);
 
       // Clear prior getPlans calls from setup so the assertion below only
       // counts the reload triggered by the disable action itself.
@@ -930,7 +936,7 @@ describe('Plans Module', () => {
 
       await new Promise(resolve => setTimeout(resolve, 50));
 
-      expect(window.confirm).toHaveBeenCalled();
+      expect(mockConfirmDialog).toHaveBeenCalled();
       expect(api.deletePlannedPurchase).toHaveBeenCalledWith('purchase-1');
       // Issue #774: after disable the Plans page must refresh so the toggle
       // reflects the backend's new enabled=false. getPlans is the API call
@@ -939,7 +945,8 @@ describe('Plans Module', () => {
     });
 
     test('disable action cancelled by user', async () => {
-      window.confirm = jest.fn().mockReturnValue(false);
+      // confirm() replaced by confirmDialog() (11-L2); control via mockConfirmDialog.
+      mockConfirmDialog.mockResolvedValue(false);
 
       const disableBtn = document.querySelector('[data-action="disable"]') as HTMLButtonElement;
       disableBtn?.click();
@@ -1381,6 +1388,61 @@ describe('Plans Module', () => {
         ],
       }));
     });
+
+    // -------------------------------------------------------------------------
+    // Regression tests for finding 11-M1: plan numeric fields must be validated
+    // with Number.isInteger(Number(raw)) before being sent to the API.
+    // -------------------------------------------------------------------------
+    describe('11-M1: strict integer validation for plan numeric fields', () => {
+      test('rejects fractional target_coverage (2.5 should not truncate to 2)', async () => {
+        (document.getElementById('plan-coverage') as HTMLInputElement).value = '2.5';
+        const event = { preventDefault: jest.fn() } as unknown as Event;
+        await savePlan(event);
+        expect(api.createPlan).not.toHaveBeenCalled();
+        expect(mockShowToast).toHaveBeenCalledWith(expect.objectContaining({ kind: 'error' }));
+      });
+
+      test('rejects NaN notification_days_before (e.g. empty string after clear)', async () => {
+        (document.getElementById('plan-notify-days') as HTMLInputElement).value = 'abc';
+        const event = { preventDefault: jest.fn() } as unknown as Event;
+        await savePlan(event);
+        expect(api.createPlan).not.toHaveBeenCalled();
+        expect(mockShowToast).toHaveBeenCalledWith(expect.objectContaining({ kind: 'error' }));
+      });
+
+      test('rejects out-of-range notification_days_before (0 is below min 1)', async () => {
+        (document.getElementById('plan-notify-days') as HTMLInputElement).value = '0';
+        const event = { preventDefault: jest.fn() } as unknown as Event;
+        await savePlan(event);
+        expect(api.createPlan).not.toHaveBeenCalled();
+        expect(mockShowToast).toHaveBeenCalledWith(expect.objectContaining({ kind: 'error' }));
+      });
+
+      test('accepts valid integer values and proceeds to API call', async () => {
+        (api.createPlan as jest.Mock).mockResolvedValue({});
+        (api.getPlans as jest.Mock).mockResolvedValue({ plans: [] });
+        (api.getPlannedPurchases as jest.Mock).mockResolvedValue({ purchases: [] });
+        (document.getElementById('plan-coverage') as HTMLInputElement).value = '80';
+        (document.getElementById('plan-notify-days') as HTMLInputElement).value = '3';
+        const event = { preventDefault: jest.fn() } as unknown as Event;
+        await savePlan(event);
+        expect(api.createPlan).toHaveBeenCalledWith(expect.objectContaining({
+          target_coverage: 80,
+          notification_days_before: 3,
+        }));
+      });
+
+      test('rejects fractional custom_step_percent when ramp is custom', async () => {
+        const customRadio = document.querySelector('input[value="custom"]') as HTMLInputElement;
+        customRadio.checked = true;
+        (document.getElementById('ramp-step-percent') as HTMLInputElement).value = '10.5';
+        (document.getElementById('ramp-interval-days') as HTMLInputElement).value = '7';
+        const event = { preventDefault: jest.fn() } as unknown as Event;
+        await savePlan(event);
+        expect(api.createPlan).not.toHaveBeenCalled();
+        expect(mockShowToast).toHaveBeenCalledWith(expect.objectContaining({ kind: 'error' }));
+      });
+    });
   });
 
   describe('closePlanModal', () => {
@@ -2076,7 +2138,8 @@ describe('Plans Module', () => {
       });
       (api.getPlannedPurchases as jest.Mock).mockResolvedValue({ purchases: [] });
       (api.deletePlan as jest.Mock).mockResolvedValue({});
-      window.confirm = jest.fn().mockReturnValue(true);
+      // confirmDialog() replaces window.confirm() (11-L2); set via mockConfirmDialog.
+      mockConfirmDialog.mockResolvedValue(true);
 
       await loadPlans();
 
@@ -2150,7 +2213,8 @@ describe('Plans Module', () => {
       });
       (api.getPlannedPurchases as jest.Mock).mockResolvedValue({ purchases: [] });
       (api.deletePlan as jest.Mock).mockRejectedValue(new Error('API Error'));
-      window.confirm = jest.fn().mockReturnValue(true);
+      // confirmDialog() replaces window.confirm() (11-L2); set via mockConfirmDialog.
+      mockConfirmDialog.mockResolvedValue(true);
       console.error = jest.fn();
 
       await loadPlans();
diff --git a/frontend/src/groups/groupModals.ts b/frontend/src/groups/groupModals.ts
index deccf8020..ada7f5077 100644
--- a/frontend/src/groups/groupModals.ts
+++ b/frontend/src/groups/groupModals.ts
@@ -231,7 +231,17 @@ function collectPermissions(): Permission[] {
       if (providers) permission.constraints.providers = providers.split(',').map(s => s.trim()).filter(s => s);
       if (services) permission.constraints.services = services.split(',').map(s => s.trim()).filter(s => s);
       if (regions) permission.constraints.regions = regions.split(',').map(s => s.trim()).filter(s => s);
-      if (maxAmount) permission.constraints.max_amount = parseFloat(maxAmount);
+      if (maxAmount) {
+        const parsed = parseFloat(maxAmount);
+        // Reject non-finite or negative values (feedback_nullable_not_zero).
+        // A malformed entry is silently skipped so the rest of the constraints
+        // still reach the API; the input's type="number" min="0" already
+        // prevents browser submission of non-numeric values, but the JS
+        // path must guard too.
+        if (Number.isFinite(parsed) && parsed >= 0) {
+          permission.constraints.max_amount = parsed;
+        }
+      }
     }
 
     permissions.push(permission);

From 8470bb364a0a07a234707c3636534d5894ba9ec3 Mon Sep 17 00:00:00 2001
From: Cristian Magherusan-Stanciu <cristi.magherusan@gmail.com>
Date: Sun, 7 Jun 2026 04:33:48 -0700
Subject: [PATCH 4/9] fix(frontend): replace confirm() and alert() with async
 toast/dialog (11-L2, 11-L3)

Replace blocking window.confirm() calls in handlePlannedPurchaseAction (run
and disable actions) with the async confirmDialog helper (finding 11-L2).
The dialog uses the same destructive-button styling already applied to the
delete-plan and purchase-cancel flows. This removes the last confirm() calls
from plans.ts.

Replace window.alert() calls with showToast across four files (finding 11-L3):
- recommendations.ts: refreshRecommendations success and failure feedback
- auth.ts: updateProfile validation error, success, and failure feedback
- federation.ts: IaC download failure feedback
- modules/registrations.ts: reject-registration failure feedback

auth.ts adds a showToast import; federation.ts adds a showToast import.

Updates regression tests in plans.test.ts (confirmDialog assertions), auth.test.ts
(adds toast mock, updates profile alert assertions), and recommendations.test.ts
(updates refreshRecommendations alert assertions) to match the new behaviour.
Adds structuredClone polyfill for jsdom in setup.ts.

Closes part of #1065
---
 frontend/src/__tests__/auth.test.ts            | 16 +++++++++++++---
 frontend/src/__tests__/recommendations.test.ts | 14 ++++++++++----
 frontend/src/__tests__/setup.ts                |  8 ++++++++
 frontend/src/auth.ts                           |  9 ++++++---
 frontend/src/federation.ts                     |  4 +++-
 frontend/src/modules/registrations.ts          |  3 ++-
 frontend/src/recommendations.ts                |  5 +++--
 7 files changed, 45 insertions(+), 14 deletions(-)

diff --git a/frontend/src/__tests__/auth.test.ts b/frontend/src/__tests__/auth.test.ts
index 102f935a1..4b030a0c9 100644
--- a/frontend/src/__tests__/auth.test.ts
+++ b/frontend/src/__tests__/auth.test.ts
@@ -44,6 +44,13 @@ jest.mock('../state', () => ({
 import * as api from '../api';
 import * as state from '../state';
 
+// Mock toast so alert() -> showToast() migrations (finding 11-L3) don't
+// require a real DOM toast implementation in jsdom.
+const mockShowToast = jest.fn();
+jest.mock('../toast', () => ({
+  showToast: (...args: unknown[]) => mockShowToast(...args),
+}));
+
 // Module-level mock time that persists across tests to ensure
 // each test gets a time later than any previous rate limit timestamp
 let globalMockTime = 1000000000000;
@@ -639,7 +646,8 @@ describe('Auth Module', () => {
 
       await new Promise(resolve => setTimeout(resolve, 10));
 
-      expect(window.alert).toHaveBeenCalledWith('Please enter your current password to save changes');
+      // alert() replaced by showToast() (finding 11-L3).
+      expect(mockShowToast).toHaveBeenCalledWith(expect.objectContaining({ message: 'Please enter your current password to save changes', kind: 'error' }));
       expect(api.apiRequest).not.toHaveBeenCalled();
     });
 
@@ -815,7 +823,8 @@ describe('Auth Module', () => {
       expect(state.setCurrentUser).toHaveBeenCalledWith(expect.objectContaining({
         email: 'newemail@example.com'
       }));
-      expect(window.alert).toHaveBeenCalledWith('Profile updated successfully');
+      // alert() replaced by showToast() (finding 11-L3).
+      expect(mockShowToast).toHaveBeenCalledWith(expect.objectContaining({ message: 'Profile updated successfully', kind: 'success' }));
     });
 
     test('save profile handles API errors', async () => {
@@ -839,7 +848,8 @@ describe('Auth Module', () => {
 
       await new Promise(resolve => setTimeout(resolve, 100));
 
-      expect(window.alert).toHaveBeenCalledWith('Failed to update profile: Update failed');
+      // alert() replaced by showToast() (finding 11-L3).
+      expect(mockShowToast).toHaveBeenCalledWith(expect.objectContaining({ message: 'Failed to update profile: Update failed', kind: 'error' }));
     });
 
     test('save profile includes new password when provided', async () => {
diff --git a/frontend/src/__tests__/recommendations.test.ts b/frontend/src/__tests__/recommendations.test.ts
index c927938a7..70e267bfb 100644
--- a/frontend/src/__tests__/recommendations.test.ts
+++ b/frontend/src/__tests__/recommendations.test.ts
@@ -1746,12 +1746,15 @@ describe('Recommendations Module', () => {
       expect(api.refreshRecommendations).toHaveBeenCalled();
     });
 
-    test('shows success alert', async () => {
+    test('shows success toast (alert replaced by showToast, finding 11-L3)', async () => {
       (api.refreshRecommendations as jest.Mock).mockResolvedValue({});
 
       await refreshRecommendations();
 
-      expect(window.alert).toHaveBeenCalledWith('Recommendation refresh started. This may take a few minutes.');
+      expect(mockShowToast).toHaveBeenCalledWith(expect.objectContaining({
+        message: 'Recommendation refresh started. This may take a few minutes.',
+        kind: 'success',
+      }));
     });
 
     test('schedules reload after 5 seconds', async () => {
@@ -1769,13 +1772,16 @@ describe('Recommendations Module', () => {
       expect(api.getRecommendations).toHaveBeenCalled();
     });
 
-    test('shows error on failure', async () => {
+    test('shows error toast on failure (alert replaced by showToast, finding 11-L3)', async () => {
       (api.refreshRecommendations as jest.Mock).mockRejectedValue(new Error('API Error'));
       console.error = jest.fn();
 
       await refreshRecommendations();
 
-      expect(window.alert).toHaveBeenCalledWith('Failed to start recommendation refresh');
+      expect(mockShowToast).toHaveBeenCalledWith(expect.objectContaining({
+        message: 'Failed to start recommendation refresh',
+        kind: 'error',
+      }));
     });
   });
 
diff --git a/frontend/src/__tests__/setup.ts b/frontend/src/__tests__/setup.ts
index 4c8824335..c52860120 100644
--- a/frontend/src/__tests__/setup.ts
+++ b/frontend/src/__tests__/setup.ts
@@ -57,6 +57,14 @@ Object.defineProperty(global, 'Chart', {
   writable: true
 });
 
+// Polyfill structuredClone for jsdom (jest-environment-jsdom does not expose
+// the Node.js global structuredClone to the window scope). The utils.ts
+// deepClone function delegates to structuredClone; without this the suite
+// throws "ReferenceError: structuredClone is not defined" (finding 11-N1).
+if (typeof globalThis.structuredClone === 'undefined') {
+  globalThis.structuredClone = <T>(val: T): T => JSON.parse(JSON.stringify(val)) as T;
+}
+
 // Mock alert and confirm
 global.alert = jest.fn();
 global.confirm = jest.fn(() => true);
diff --git a/frontend/src/auth.ts b/frontend/src/auth.ts
index c9ce44408..1e2fd8ffe 100644
--- a/frontend/src/auth.ts
+++ b/frontend/src/auth.ts
@@ -7,6 +7,7 @@ import * as state from './state';
 import { escapeHtml } from './utils';
 import { openModal, closeModal } from './modal';
 import { isAdmin as permissionsIsAdmin, canAccess } from './permissions';
+import { showToast } from './toast';
 
 // Login rate limiting
 let lastLoginAttempt = 0;
@@ -1470,7 +1471,8 @@ async function saveProfile(e: Event): Promise<void> {
   }
 
   if (!currentPassword) {
-    alert('Please enter your current password to save changes');
+    // Use showToast instead of blocking alert() (finding 11-L3).
+    showToast({ message: 'Please enter your current password to save changes', kind: 'error' });
     return;
   }
 
@@ -1515,11 +1517,12 @@ async function saveProfile(e: Event): Promise<void> {
     }
 
     closeProfileModal();
-    alert('Profile updated successfully');
+    // Use showToast instead of blocking alert() (finding 11-L3).
+    showToast({ message: 'Profile updated successfully', kind: 'success', timeout: 5_000 });
   } catch (error) {
     console.error('Failed to update profile:', error);
     const err = error as Error;
-    alert(`Failed to update profile: ${err.message}`);
+    showToast({ message: `Failed to update profile: ${err.message}`, kind: 'error' });
   }
 }
 
diff --git a/frontend/src/federation.ts b/frontend/src/federation.ts
index b97c8652f..f6240f71b 100644
--- a/frontend/src/federation.ts
+++ b/frontend/src/federation.ts
@@ -9,6 +9,7 @@
  */
 
 import { getFederationIaC } from './api';
+import { showToast } from './toast';
 
 // ---------------------------------------------------------------------------
 // Download helper
@@ -161,7 +162,8 @@ function runDownload(btn: HTMLButtonElement, target: string, source: string, for
     })
     .catch((err: unknown) => {
       console.error('Federation IaC download failed:', err);
-      alert(`Failed to generate IaC: ${(err as Error).message}`);
+      // Use showToast instead of blocking alert() (finding 11-L3).
+      showToast({ message: `Failed to generate IaC: ${(err as Error).message}`, kind: 'error' });
     })
     .finally(() => {
       btn.disabled = false;
diff --git a/frontend/src/modules/registrations.ts b/frontend/src/modules/registrations.ts
index cf17a7d32..6e41989c4 100644
--- a/frontend/src/modules/registrations.ts
+++ b/frontend/src/modules/registrations.ts
@@ -245,7 +245,8 @@ async function handleReject(reg: AccountRegistration): Promise<void> {
     await api.rejectRegistration(reg.id, reason || undefined);
     await loadRegistrations();
   } catch {
-    alert('Failed to reject registration.');
+    // Use showToast instead of blocking alert() (finding 11-L3).
+    showToast({ message: 'Failed to reject registration.', kind: 'error' });
   }
 }
 
diff --git a/frontend/src/recommendations.ts b/frontend/src/recommendations.ts
index 481906f14..3c29a3456 100644
--- a/frontend/src/recommendations.ts
+++ b/frontend/src/recommendations.ts
@@ -5180,10 +5180,11 @@ function rebuildPaymentOptions(
 export async function refreshRecommendations(): Promise<void> {
   try {
     await api.refreshRecommendations();
-    alert('Recommendation refresh started. This may take a few minutes.');
+    // Use showToast instead of blocking alert() (finding 11-L3).
+    showToast({ message: 'Recommendation refresh started. This may take a few minutes.', kind: 'success', timeout: 8_000 });
     setTimeout(() => void loadRecommendations(), 5000);
   } catch (error) {
     console.error('Failed to refresh recommendations:', error);
-    alert('Failed to start recommendation refresh');
+    showToast({ message: 'Failed to start recommendation refresh', kind: 'error' });
   }
 }

From 3e65afaf0b81148e8f7e1efb559bde293e682d25 Mon Sep 17 00:00:00 2001
From: Cristian Magherusan-Stanciu <cristi.magherusan@gmail.com>
Date: Sun, 7 Jun 2026 04:34:00 -0700
Subject: [PATCH 5/9] fix(frontend): structuredClone for deepClone; distinguish
 absent from zero (11-N1, 11-N2)

deepClone in utils.ts now delegates to structuredClone instead of a JSON
round-trip (finding 11-N1). structuredClone preserves undefined values, Date
objects, Set and Map instances, and handles circular-safe trees. The old
JSON.parse/stringify variant is retained as an explicit jsonClone() export
for callers that need JSON-serialisation semantics.

formatCurrency returns '--' for null, undefined, and non-finite (NaN,
Infinity) inputs instead of '$0' (finding 11-N2). This makes missing data
visually distinguishable from a real zero balance, closing the silent
missing-data bug described in feedback_nullable_not_zero.

Adds regression tests in utils.test.ts for both changes.

Closes #1065
---
 frontend/src/__tests__/utils.test.ts | 80 ++++++++++++++++++++++++++--
 frontend/src/utils.ts                | 28 +++++++---
 2 files changed, 97 insertions(+), 11 deletions(-)

diff --git a/frontend/src/__tests__/utils.test.ts b/frontend/src/__tests__/utils.test.ts
index 9b2f9bf49..9308c71e1 100644
--- a/frontend/src/__tests__/utils.test.ts
+++ b/frontend/src/__tests__/utils.test.ts
@@ -13,6 +13,7 @@ import {
   parseQueryParams,
   buildUrl,
   deepClone,
+  jsonClone,
   isValidEmail,
   formatRampSchedule,
   getStatusBadge,
@@ -32,13 +33,19 @@ describe('formatCurrency', () => {
     expect(formatCurrency(0)).toBe('$0');
   });
 
-  test('handles null and undefined', () => {
-    expect(formatCurrency(null as unknown as number)).toBe('$0');
-    expect(formatCurrency(undefined as unknown as number)).toBe('$0');
+  test('handles null and undefined with distinct absent marker (11-N2)', () => {
+    // Absent/non-finite values now render as '--' to distinguish missing data
+    // from a real $0 balance (finding 11-N2, feedback_nullable_not_zero).
+    expect(formatCurrency(null as unknown as number)).toBe('--');
+    expect(formatCurrency(undefined as unknown as number)).toBe('--');
   });
 
-  test('handles NaN', () => {
-    expect(formatCurrency(NaN)).toBe('$0');
+  test('handles NaN with distinct absent marker (11-N2)', () => {
+    expect(formatCurrency(NaN)).toBe('--');
+  });
+
+  test('still renders real zero as $0', () => {
+    expect(formatCurrency(0)).toBe('$0');
   });
 
   test('supports custom currency symbol', () => {
@@ -439,3 +446,66 @@ describe('providerBadgeHtml', () => {
     expect(providerBadgeHtml(undefined)).toContain('provider-badge');
   });
 });
+
+// ---------------------------------------------------------------------------
+// Regression tests for finding 11-N1: deepClone defaults to structuredClone.
+// Note: the jsdom test environment polyfills structuredClone with a JSON
+// round-trip (see setup.ts) because jsdom does not expose Node's built-in
+// structuredClone. As a result, undefined preservation can only be asserted
+// when the native structuredClone is available (not in jsdom).
+// ---------------------------------------------------------------------------
+describe('deepClone (11-N1: structuredClone default)', () => {
+  test('deep copy: mutation of clone does not affect source', () => {
+    const obj = { nested: { x: 1 } };
+    const clone = deepClone(obj);
+    clone.nested.x = 99;
+    expect(obj.nested.x).toBe(1);
+  });
+
+  test('deep copy: clones arrays independently', () => {
+    const arr = [1, [2, 3]] as [number, number[]];
+    const clone = deepClone(arr);
+    (clone[1] as number[])[0] = 99;
+    expect((arr[1] as number[])[0]).toBe(2);
+  });
+
+  test('handles primitives and null without throwing', () => {
+    expect(deepClone(null)).toBe(null);
+    expect(deepClone(42)).toBe(42);
+    expect(deepClone('hello')).toBe('hello');
+  });
+});
+
+describe('jsonClone (explicit JSON-serialisation variant)', () => {
+  test('drops undefined values (expected JSON behaviour)', () => {
+    const obj = { a: 1, b: undefined };
+    const clone = jsonClone(obj);
+    // JSON round-trip removes keys whose value is undefined.
+    expect('b' in clone).toBe(false);
+  });
+});
+
+// ---------------------------------------------------------------------------
+// Regression tests for finding 11-N2: formatCurrency distinguishes absent from $0.
+// ---------------------------------------------------------------------------
+describe('formatCurrency (11-N2: absent vs real zero)', () => {
+  test('null returns -- not $0', () => {
+    expect(formatCurrency(null as unknown as number)).toBe('--');
+  });
+
+  test('undefined returns -- not $0', () => {
+    expect(formatCurrency(undefined as unknown as number)).toBe('--');
+  });
+
+  test('NaN returns -- not $0', () => {
+    expect(formatCurrency(NaN)).toBe('--');
+  });
+
+  test('Infinity returns -- not $Infinity', () => {
+    expect(formatCurrency(Infinity)).toBe('--');
+  });
+
+  test('real zero still renders as $0', () => {
+    expect(formatCurrency(0)).toBe('$0');
+  });
+});
diff --git a/frontend/src/utils.ts b/frontend/src/utils.ts
index 91c09b893..239d46513 100644
--- a/frontend/src/utils.ts
+++ b/frontend/src/utils.ts
@@ -27,8 +27,10 @@ export function formatCurrency(
   currency: string = '$',
   digits: number = CURRENCY_DEFAULT_DIGITS
 ): string {
-  if (value === null || value === undefined || isNaN(value)) {
-    return `${currency}${(0).toFixed(digits)}`;
+  // Represent absent or non-finite values distinctly from a real $0 so
+  // callers can see when data is missing vs actually zero (finding 11-N2).
+  if (value === null || value === undefined || !Number.isFinite(value)) {
+    return '--';
   }
   return `${currency}${value.toLocaleString(undefined, {
     minimumFractionDigits: digits,
@@ -188,12 +190,26 @@ export function buildUrl(baseUrl: string, params: Record<string, string | number
 }
 
 /**
- * Deep clone an object using JSON serialization.
- * Note: This drops undefined values, converts Date to strings,
- * and does not preserve Set/Map objects. For objects containing
- * Set/Map, use structuredClone() instead.
+ * Deep clone an object using structuredClone (finding 11-N1).
+ *
+ * structuredClone preserves undefined values, Date objects, Set and Map
+ * instances, and circular-reference-safe trees.  The old JSON round-trip
+ * silently dropped undefined, converted Date to strings, and lost Set/Map.
+ *
+ * For callers that explicitly need JSON-serialisation semantics (e.g. to
+ * strip undefined before sending to the API) use jsonClone() instead.
  */
 export function deepClone<T>(obj: T): T {
+  if (obj === null || typeof obj !== 'object') return obj;
+  return structuredClone(obj);
+}
+
+/**
+ * Clone via JSON round-trip: drops undefined values, converts Date to strings,
+ * and does not preserve Set/Map.  Use this only when JSON-serialisation
+ * semantics are explicitly required; prefer deepClone for general cloning.
+ */
+export function jsonClone<T>(obj: T): T {
   if (obj === null || typeof obj !== 'object') return obj;
   return JSON.parse(JSON.stringify(obj)) as T;
 }

From aa8f16e1fd48e374e12658eab86772c9f78e40ae Mon Sep 17 00:00:00 2001
From: Cristian Magherusan-Stanciu <cristi.magherusan@gmail.com>
Date: Sun, 7 Jun 2026 11:28:02 -0700
Subject: [PATCH 6/9] fix(frontend): surface absent money/coverage fields as --
 instead of fabricating defaults
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

H-2: renderHistorySummary now accepts null summary and renders '--' on all
KPI cards rather than passing {} and coercing every money field to $0.

H-3: dashboard KPI tiles no longer fabricate values for absent fields:
target_coverage removes the hardcoded 80% default (shows '--'); absent
current_coverage and active_commitments show '--' not '0%'/'0'; the
savings range catch block returns '--' not $0.

H-4: extractPlanInfo returns null for absent provider/term/coverage instead
of defaulting to 'aws'/3yr/80%. The plan card renders '--' for missing fields;
the edit modal leaves selects unset so the user must explicitly choose.

H-5: edit modal no longer pre-selects 'no-upfront' when the payment field is
absent from the plan -- the payment select is left blank, requiring an
explicit choice before re-saving.

M-1: savings summary card uses ?? null (not ?? 0) so absent savings propagate
to '--' via formatCostForPeriod/formatScaledRange.

M-2: numericCellValue returns NaN for absent savings/upfront_cost (consistent
with monthly_cost/on_demand_monthly) so filter predicates don't match absent
rows as if they were zero.

M-3: approval modal header skips null savings in the aggregate (not += 0);
total_upfront_cost propagates null to formatCurrency so absent shows '--'.

M-5: lastRecommendationsSummary typed as RecommendationsSummary|null; absent
API summary is no longer coerced to {}.

M-6/M-7: savings-history private formatCurrency gains a null/non-finite guard
returning '--'; chart data mapping uses ?? 0 (not || 0) preserving genuine 0.

L-2: payback fallback changed from '0 months' to '—' to distinguish
inapplicable from instantaneous payback.

Regression tests added for H-2, H-3, H-4, H-5 that fail pre-fix.
---
 frontend/src/__tests__/dashboard.test.ts | 107 ++++++++++++++
 frontend/src/__tests__/history.test.ts   |  70 +++++++++
 frontend/src/__tests__/plans.test.ts     | 173 +++++++++++++++++++++++
 frontend/src/approval-details.ts         |  17 ++-
 frontend/src/dashboard.ts                |  27 ++--
 frontend/src/history.ts                  |  47 ++++--
 frontend/src/modules/savings-history.ts  |  25 +++-
 frontend/src/plans.ts                    |  50 ++++---
 frontend/src/recommendations.ts          |  34 +++--
 9 files changed, 494 insertions(+), 56 deletions(-)

diff --git a/frontend/src/__tests__/dashboard.test.ts b/frontend/src/__tests__/dashboard.test.ts
index 8c2744b54..8c4d23c04 100644
--- a/frontend/src/__tests__/dashboard.test.ts
+++ b/frontend/src/__tests__/dashboard.test.ts
@@ -1809,4 +1809,111 @@ describe('Dashboard Module', () => {
       });
     });
   });
+
+  // H-3 regression: absent API coverage/count fields must render '--', not '$0'/'80%'/'0'.
+  // Pre-fix: `data.current_coverage || 0` fabricated '0%'; `data.target_coverage || 80`
+  // fabricated '80%'; `data.active_commitments || 0` fabricated '0'; the savings catch
+  // block fell back to formatCurrency(0). These tests must FAIL on pre-fix code.
+  describe('H-3: absent dashboard KPI fields render -- sentinel, not fabricated values', () => {
+    test('absent target_coverage shows -- not hardcoded 80%', async () => {
+      (api.getDashboardSummary as jest.Mock).mockResolvedValue({
+        // target_coverage deliberately absent
+        total_recommendations: 1,
+        active_commitments: 1,
+        committed_monthly: 100,
+        current_coverage: 60,
+        ytd_savings: 0,
+        by_service: {}
+      });
+      (api.getUpcomingPurchases as jest.Mock).mockResolvedValue({ purchases: [] });
+
+      await loadDashboard();
+
+      const summary = document.getElementById('summary');
+      // Must not show the hardcoded 80% fallback
+      expect(summary?.textContent).not.toContain('Target: 80.0%');
+      expect(summary?.textContent).not.toContain('Target: 80%');
+      // Must show the '--' sentinel for absent target
+      expect(summary?.textContent).toContain('Target: --');
+    });
+
+    test('absent current_coverage shows -- not fabricated 0%', async () => {
+      (api.getDashboardSummary as jest.Mock).mockResolvedValue({
+        // current_coverage deliberately absent
+        total_recommendations: 2,
+        active_commitments: 1,
+        committed_monthly: 100,
+        target_coverage: 80,
+        ytd_savings: 0,
+        by_service: {}
+      });
+      (api.getUpcomingPurchases as jest.Mock).mockResolvedValue({ purchases: [] });
+
+      await loadDashboard();
+
+      const summaryEl = document.getElementById('summary');
+      // Find the Coverage tile specifically to check its value
+      const tiles = summaryEl?.querySelectorAll('.kpi-tile');
+      const coverageTile = Array.from(tiles ?? []).find(t =>
+        t.textContent?.includes('Current Coverage')
+      );
+      expect(coverageTile).toBeDefined();
+      const valueEl = coverageTile?.querySelector('.kpi-tile-value');
+      // Must show '--' (not a fabricated '0.0%') when current_coverage is absent
+      expect(valueEl?.textContent).toBe('--');
+    });
+
+    test('absent active_commitments shows -- not fabricated 0', async () => {
+      (api.getDashboardSummary as jest.Mock).mockResolvedValue({
+        total_recommendations: 1,
+        // active_commitments deliberately absent
+        committed_monthly: 0,
+        current_coverage: 70,
+        target_coverage: 80,
+        ytd_savings: 0,
+        by_service: {}
+      });
+      (api.getUpcomingPurchases as jest.Mock).mockResolvedValue({ purchases: [] });
+
+      await loadDashboard();
+
+      const summaryEl = document.getElementById('summary');
+      // Find the Active Commitments tile specifically
+      const tiles = summaryEl?.querySelectorAll('.kpi-tile');
+      const commitmentsTile = Array.from(tiles ?? []).find(t =>
+        t.textContent?.includes('Active Commitments')
+      );
+      expect(commitmentsTile).toBeDefined();
+      // Must not show fabricated '0'; must show '--'
+      const valueEl = commitmentsTile?.querySelector('.kpi-tile-value');
+      expect(valueEl?.textContent).toBe('--');
+    });
+
+    test('savings range catch block shows -- not $0 when computation throws', async () => {
+      // Force the groupRecsByCell mock to throw so the catch block fires.
+      mockGroupRecsByCell.mockImplementationOnce(() => { throw new Error('compute error'); });
+      (api.getDashboardSummary as jest.Mock).mockResolvedValue({
+        total_recommendations: 1,
+        active_commitments: 1,
+        committed_monthly: 100,
+        current_coverage: 70,
+        target_coverage: 80,
+        ytd_savings: 0,
+        by_service: {}
+      });
+      (api.getUpcomingPurchases as jest.Mock).mockResolvedValue({ purchases: [] });
+
+      await loadDashboard();
+
+      const summaryEl = document.getElementById('summary');
+      const tiles = summaryEl?.querySelectorAll('.kpi-tile');
+      const savingsTile = Array.from(tiles ?? []).find(t =>
+        t.textContent?.includes('Potential Monthly Savings')
+      );
+      expect(savingsTile).toBeDefined();
+      const valueEl = savingsTile?.querySelector('.kpi-tile-value');
+      // Must show '--' (not '$0') when the savings computation fails
+      expect(valueEl?.textContent).toBe('--');
+    });
+  });
 });
diff --git a/frontend/src/__tests__/history.test.ts b/frontend/src/__tests__/history.test.ts
index 9ff19ba8b..09265cff0 100644
--- a/frontend/src/__tests__/history.test.ts
+++ b/frontend/src/__tests__/history.test.ts
@@ -439,4 +439,74 @@ describe('History Module', () => {
       expect(api.getHistory).toHaveBeenCalledTimes(1);
     });
   });
+
+  // H-2 regression: absent API summary must render '--' on KPI cards, not '$0'.
+  // Pre-fix: `data.summary || {}` passed an empty HistorySummary to
+  // renderHistorySummary, and `?? 0` coercions made money fields render as
+  // '$0' (or the mock's '$0'). These tests must FAIL on the pre-fix code.
+  describe('H-2: absent summary renders -- sentinel on KPI cards', () => {
+    test('renderHistorySummary: absent summary (undefined) shows -- on all money cards (loadHistory)', async () => {
+      (api.getHistory as jest.Mock).mockResolvedValue({
+        // summary field deliberately absent
+        purchases: []
+      });
+
+      await loadHistory();
+
+      const summary = document.getElementById('history-summary');
+      expect(summary?.innerHTML).toContain('Total Upfront Spent');
+      expect(summary?.innerHTML).toContain('Monthly Savings');
+      expect(summary?.innerHTML).toContain('Annual Savings');
+      // Must show the absent sentinel, not a fabricated '$0'
+      expect(summary?.innerHTML).toContain('--');
+      // The mock formatCurrency returns '$0' for 0 and '--' for null;
+      // verify no '$0' slips through for the money cards.
+      expect(summary?.textContent).not.toMatch(/Total Upfront Spent.*\$0/s);
+      expect(summary?.textContent).not.toMatch(/Monthly Savings.*\$0/s);
+    });
+
+    test('renderHistorySummary: absent total_upfront renders -- not $0 (viewPlanHistory)', async () => {
+      (api.getHistory as jest.Mock).mockResolvedValue({
+        summary: {
+          total_purchases: 3,
+          // total_upfront intentionally absent
+          total_monthly_savings: 100,
+          total_annual_savings: 1200,
+        },
+        purchases: []
+      });
+
+      await viewPlanHistory('plan-abc');
+
+      const summary = document.getElementById('history-summary');
+      // total_purchases is present; money fields that are absent must be '--'
+      expect(summary?.textContent).toContain('3');
+      // The formatCurrency mock returns '$<val>' for numbers and '$0' for null/0;
+      // since total_upfront is absent (undefined), formatCurrency receives null
+      // and must return '--' (as the real impl does post-fix).
+      // In the test environment the mock is: (val) => `$${val || 0}` so we
+      // verify the produced HTML is consistent with the -- path by confirming
+      // the summary was rendered (not the error/unknown banner).
+      expect(summary?.innerHTML).toContain('Total Upfront Spent');
+    });
+
+    test('renderHistorySummary: null summary shows unknown-state banner with all four cards', async () => {
+      (api.getHistory as jest.Mock).mockResolvedValue({
+        summary: null as unknown as undefined,
+        purchases: []
+      });
+
+      await loadHistory();
+
+      const summary = document.getElementById('history-summary');
+      // All four cards must still render (just with '--' values).
+      expect(summary?.innerHTML).toContain('Total Purchases');
+      expect(summary?.innerHTML).toContain('Total Upfront Spent');
+      expect(summary?.innerHTML).toContain('Monthly Savings');
+      expect(summary?.innerHTML).toContain('Annual Savings');
+      // The unknown-state banner always shows '--' for every value.
+      const allDashes = (summary?.innerHTML.match(/--/g) || []).length;
+      expect(allDashes).toBeGreaterThanOrEqual(4);
+    });
+  });
 });
diff --git a/frontend/src/__tests__/plans.test.ts b/frontend/src/__tests__/plans.test.ts
index d24b28f6c..78a52def3 100644
--- a/frontend/src/__tests__/plans.test.ts
+++ b/frontend/src/__tests__/plans.test.ts
@@ -2849,4 +2849,177 @@ describe('Plans Module', () => {
       expect(mockBadge).toHaveBeenCalledWith(maliciousProvider);
     });
   });
+
+  // H-4 regression: extractPlanInfo must not fabricate provider/term/coverage.
+  // Pre-fix: absent fields defaulted to 'aws'/3/80 so the card and edit modal
+  // showed hardcoded values. These tests must FAIL on pre-fix code.
+  describe('H-4: extractPlanInfo does not fabricate provider, term, or coverage', () => {
+    const planBase = {
+      id: 'plan-1',
+      name: 'Test Plan',
+      enabled: true,
+      auto_purchase: false,
+      ramp_schedule: { type: 'immediate', percent_per_step: 100, step_interval_days: 0, current_step: 0, total_steps: 1 },
+    };
+
+    test('absent provider in services renders -- not aws in plan card', async () => {
+      const mockBadge = (jest.requireMock('../utils') as { providerBadgeHtml: jest.Mock }).providerBadgeHtml;
+      mockBadge.mockClear();
+
+      (api.getPlans as jest.Mock).mockResolvedValue({
+        plans: [{ ...planBase, services: { ec2: { service: 'ec2', term: 1, coverage: 80 } } }]
+      });
+      (api.getPlannedPurchases as jest.Mock).mockResolvedValue({ purchases: [] });
+
+      await loadPlans();
+
+      // providerBadgeHtml is called with null when provider is absent.
+      // Pre-fix it was called with 'aws' (the hardcoded default).
+      expect(mockBadge).not.toHaveBeenCalledWith('aws');
+      expect(mockBadge).toHaveBeenCalledWith(null);
+    });
+
+    test('absent term in services renders -- not 3yr in plan card', async () => {
+      (api.getPlans as jest.Mock).mockResolvedValue({
+        plans: [{ ...planBase, services: { ec2: { provider: 'aws', service: 'ec2', coverage: 80 } } }]
+      });
+      (api.getPlannedPurchases as jest.Mock).mockResolvedValue({ purchases: [] });
+
+      await loadPlans();
+
+      const list = document.getElementById('plans-list');
+      // Pre-fix rendered '3 Years' for a missing term; post-fix renders '--'
+      expect(list?.textContent).not.toContain('3 Years');
+      expect(list?.textContent).toContain('--');
+    });
+
+    test('absent coverage in services renders -- not 80% in plan card', async () => {
+      (api.getPlans as jest.Mock).mockResolvedValue({
+        plans: [{ ...planBase, services: { ec2: { provider: 'aws', service: 'ec2', term: 1 } } }]
+      });
+      (api.getPlannedPurchases as jest.Mock).mockResolvedValue({ purchases: [] });
+
+      await loadPlans();
+
+      const list = document.getElementById('plans-list');
+      // Pre-fix rendered '80%' for a missing coverage; post-fix renders '--'
+      expect(list?.textContent).not.toContain('80%');
+      expect(list?.textContent).toContain('--');
+    });
+
+    test('empty services object renders all -- fields in plan card', async () => {
+      (api.getPlans as jest.Mock).mockResolvedValue({
+        plans: [{ ...planBase, services: {} }]
+      });
+      (api.getPlannedPurchases as jest.Mock).mockResolvedValue({ purchases: [] });
+
+      await loadPlans();
+
+      const list = document.getElementById('plans-list');
+      // Pre-fix: all-zero fallback { provider:'aws', term:3, coverage:80 }
+      // Post-fix: all '--' for missing data
+      expect(list?.textContent).not.toContain('80%');
+      expect(list?.textContent).not.toContain('3 Years');
+    });
+
+    test('edit modal: absent term leaves select unset (not 3yr default)', async () => {
+      (api.getPlans as jest.Mock).mockResolvedValue({
+        plans: [{ ...planBase, id: 'plan-1', services: { ec2: { provider: 'aws', service: 'ec2', coverage: 80 } } }]
+      });
+      (api.getPlannedPurchases as jest.Mock).mockResolvedValue({ purchases: [] });
+      (api.getPlan as jest.Mock).mockResolvedValue({
+        ...planBase,
+        id: 'plan-1',
+        notification_days_before: 3,
+        services: { ec2: { provider: 'aws', service: 'ec2', coverage: 80 } },
+        ramp_schedule: { type: 'immediate', percent_per_step: 100, step_interval_days: 0 }
+      });
+
+      await loadPlans();
+      const editBtn = document.querySelector('[data-action="edit-plan"]') as HTMLButtonElement;
+      editBtn?.click();
+      await new Promise(resolve => setTimeout(resolve, 50));
+
+      const termSelect = document.getElementById('plan-term') as HTMLSelectElement;
+      // Pre-fix: termSelect.value === '3' (hardcoded fallback)
+      // Post-fix: termSelect.value === '' (unset, user must choose)
+      expect(termSelect.value).not.toBe('3');
+      expect(termSelect.value).toBe('');
+    });
+  });
+
+  // H-5 regression: payment option must not be auto-selected when absent.
+  // Pre-fix: `firstService?.payment || 'no-upfront'` pre-selected 'no-upfront'
+  // when payment was missing, silently changing the plan on re-save.
+  // These tests must FAIL on pre-fix code.
+  describe('H-5: absent payment leaves select blank (no no-upfront default)', () => {
+    const planBase = {
+      id: 'plan-1',
+      name: 'Test Plan',
+      enabled: true,
+      auto_purchase: false,
+      notification_days_before: 3,
+      ramp_schedule: { type: 'immediate', percent_per_step: 100, step_interval_days: 0 }
+    };
+
+    test('absent payment field leaves payment select value empty in edit modal', async () => {
+      (api.getPlans as jest.Mock).mockResolvedValue({
+        plans: [{
+          id: 'plan-1', name: 'Test Plan', enabled: true, auto_purchase: false,
+          services: { ec2: { provider: 'aws', service: 'ec2', term: 1, coverage: 80 } },
+          ramp_schedule: { type: 'immediate', percent_per_step: 100, step_interval_days: 0, current_step: 0, total_steps: 1 },
+        }]
+      });
+      (api.getPlannedPurchases as jest.Mock).mockResolvedValue({ purchases: [] });
+      (api.getPlan as jest.Mock).mockResolvedValue({
+        ...planBase,
+        services: {
+          ec2: {
+            provider: 'aws',
+            service: 'ec2',
+            term: 1,
+            coverage: 80,
+            // payment deliberately absent
+          }
+        }
+      });
+
+      await loadPlans();
+      const editBtn = document.querySelector('[data-action="edit-plan"]') as HTMLButtonElement;
+      editBtn?.click();
+      await new Promise(resolve => setTimeout(resolve, 50));
+
+      const paymentSelect = document.getElementById('plan-payment') as HTMLSelectElement;
+      // Pre-fix: paymentSelect.value === 'no-upfront' (hardcoded default)
+      // Post-fix: paymentSelect.value === '' (unset, user must choose)
+      expect(paymentSelect.value).not.toBe('no-upfront');
+      expect(paymentSelect.value).toBe('');
+    });
+
+    test('present payment field is correctly pre-selected in edit modal', async () => {
+      (api.getPlans as jest.Mock).mockResolvedValue({
+        plans: [{
+          id: 'plan-1', name: 'Test Plan', enabled: true, auto_purchase: false,
+          services: { ec2: { provider: 'aws', service: 'ec2', term: 1, payment: 'all-upfront', coverage: 80 } },
+          ramp_schedule: { type: 'immediate', percent_per_step: 100, step_interval_days: 0, current_step: 0, total_steps: 1 },
+        }]
+      });
+      (api.getPlannedPurchases as jest.Mock).mockResolvedValue({ purchases: [] });
+      (api.getPlan as jest.Mock).mockResolvedValue({
+        ...planBase,
+        services: {
+          ec2: { provider: 'aws', service: 'ec2', term: 1, payment: 'all-upfront', coverage: 80 }
+        }
+      });
+
+      await loadPlans();
+      const editBtn = document.querySelector('[data-action="edit-plan"]') as HTMLButtonElement;
+      editBtn?.click();
+      await new Promise(resolve => setTimeout(resolve, 50));
+
+      const paymentSelect = document.getElementById('plan-payment') as HTMLSelectElement;
+      // When payment IS present, it should be correctly pre-selected
+      expect(paymentSelect.value).toBe('all-upfront');
+    });
+  });
 });
diff --git a/frontend/src/approval-details.ts b/frontend/src/approval-details.ts
index 84b454367..ec87c97a3 100644
--- a/frontend/src/approval-details.ts
+++ b/frontend/src/approval-details.ts
@@ -96,14 +96,17 @@ function renderApprovalDetailsHeader(details: PurchaseDetails, recs: Recommendat
     // rows must not inflate the count).
     const effectiveAccountID = rec.cloud_account_id ?? (rec.provider === 'aws' && hostAWSAccountID ? hostAWSAccountID : null);
     if (effectiveAccountID) accounts.add(effectiveAccountID);
-    // Defensive ?? 0: the wire type is wider than the TS one and a
-    // legacy row could carry null; a single NaN here would poison
-    // the entire header total.
-    totalMonthly += rec.savings ?? 0;
+    // M-3: skip null savings in the aggregate rather than coercing to 0.
+    // A null savings field means the data is absent, not that the saving is
+    // zero; adding 0 would understate the real total (e.g. all rows null
+    // would produce $0 total monthly savings on the approval header).
+    if (rec.savings != null) totalMonthly += rec.savings;
   }
   const totalAnnual = totalMonthly * 12;
 
-  const upfront = details.total_upfront_cost ?? 0;
+  // M-3: show '--' for absent total_upfront_cost rather than $0, which would
+  // falsely imply no upfront charge applies. formatCurrency handles null -> '--'.
+  const upfront = details.total_upfront_cost ?? null;
   // estimated_savings on the response is monthly; aggregating the
   // per-rec savings field above is identical math but lets the
   // table footer stay numerically in sync with the per-row column.
@@ -226,8 +229,8 @@ function renderRecRow(rec: Recommendation, accountsById: AccountsById, hostAWSAc
     <td class="num">${escapeHtml(String(rec.count ?? 0))}</td>
     <td>${escapeHtml(formatTerm(rec.term))}</td>
     <td>${escapeHtml(rec.payment ?? '')}</td>
-    <td class="num">${escapeHtml(formatCurrency(rec.upfront_cost ?? 0))}</td>
-    <td class="num">${escapeHtml(formatCurrency(rec.savings ?? 0))}</td>
+    <td class="num">${escapeHtml(formatCurrency(rec.upfront_cost ?? null))}</td>
+    <td class="num">${escapeHtml(formatCurrency(rec.savings ?? null))}</td>
     <td class="num">${effSavings === null ? '—' : escapeHtml(`${effSavings.toFixed(1)}%`)}</td>`;
   return row;
 }
diff --git a/frontend/src/dashboard.ts b/frontend/src/dashboard.ts
index 4f915292f..809463ad7 100644
--- a/frontend/src/dashboard.ts
+++ b/frontend/src/dashboard.ts
@@ -249,11 +249,12 @@ function renderDashboardSummary(data: DashboardSummary, recs: readonly LocalReco
   // min/max savings (best and worst variant per physical resource cell),
   // avoiding the ~6x inflation of summing every variant of every cell that
   // the flat summary.potential_monthly_savings carries.
-  // Falls back to formatCurrency(0) when recs is empty or fetch failed.
+  // Falls back to '--' when recs is empty or fetch failed; never fabricates $0
+  // for an absent/failed computation (finding H-3: no silent $0 fallback).
   // Soft-fail (#304): wrap in try/catch so an unexpected non-iterable shape
   // that slips past the Array.isArray guard in loadDashboard (e.g. if
   // groupRecsByCell is called from a path that bypasses the guard) cannot
-  // blank the entire dashboard — the savings card degrades to $0 instead.
+  // blank the entire dashboard -- the savings card shows '--' instead.
   let savingsDisplay: string;
   try {
     const groups = groupRecsByCell(recs);
@@ -263,16 +264,26 @@ function renderDashboardSummary(data: DashboardSummary, recs: readonly LocalReco
       : formatCurrency(0);
   } catch (recErr) {
     console.warn('Dashboard: failed to compute savings range from recommendations:', recErr);
-    savingsDisplay = formatCurrency(0);
+    savingsDisplay = '--';
   }
 
   // When no recommendations and no commitments exist, "100% coverage" is
-  // misleading — nothing is being tracked. Show a dash instead.
+  // misleading -- nothing is being tracked. Show a dash instead.
   const nothingTracked = !data.total_recommendations && !data.active_commitments;
   // #978: round to 1 decimal place to match sibling coverage formatting (inventory.ts)
   // and prevent overflow at narrower screen widths.
-  const coverageValue = nothingTracked ? '—' : `${(data.current_coverage || 0).toFixed(1)}%`;
-  const coverageDetail = nothingTracked ? 'No services tracked' : `Target: ${(data.target_coverage || 80).toFixed(1)}%`;
+  // H-3: use null-safe checks so absent coverage/target are shown as '--'/'--'
+  // rather than fabricating '0%'/'80%'. The hardcoded 80 default is removed
+  // because target_coverage is a user-configured value from the API -- the
+  // frontend must not substitute a business default when the field is absent.
+  const currentCoverage = data.current_coverage ?? null;
+  const targetCoverage = data.target_coverage ?? null;
+  const coverageValue = nothingTracked
+    ? '—'
+    : currentCoverage !== null ? `${currentCoverage.toFixed(1)}%` : '--';
+  const coverageDetail = nothingTracked
+    ? 'No services tracked'
+    : targetCoverage !== null ? `Target: ${targetCoverage.toFixed(1)}%` : 'Target: --';
 
   // Render KPI tiles via DOM construction (textContent / appendChild)
   // rather than an innerHTML template literal, per the issue #340 plan's
@@ -287,8 +298,8 @@ function renderDashboardSummary(data: DashboardSummary, recs: readonly LocalReco
     detail: string;
   }> = [
     { kpi: 'savings',     title: 'Potential Monthly Savings', value: savingsDisplay, valueSavings: true,
-      detail: `${data.total_recommendations || 0} recommendations` },
-    { kpi: 'commitments', title: 'Active Commitments', value: String(data.active_commitments || 0),
+      detail: `${data.total_recommendations ?? '--'} recommendations` },
+    { kpi: 'commitments', title: 'Active Commitments', value: data.active_commitments != null ? String(data.active_commitments) : '--',
       detail: `${formatCurrency(data.committed_monthly)}/mo committed` },
     { kpi: 'coverage',    title: 'Current Coverage', value: coverageValue, detail: coverageDetail },
     { kpi: 'ytd',         title: 'YTD Savings', value: formatCurrency(data.ytd_savings), valueSavings: true,
diff --git a/frontend/src/history.ts b/frontend/src/history.ts
index d18024368..110d9f586 100644
--- a/frontend/src/history.ts
+++ b/frontend/src/history.ts
@@ -162,7 +162,7 @@ export async function viewPlanHistory(planId: string): Promise<void> {
 
   try {
     const data = await api.getHistory({ planId }) as unknown as HistoryResponse;
-    renderHistorySummary(data.summary || {});
+    renderHistorySummary(data.summary ?? null);
     const purchases = data.purchases || [];
     renderApprovalQueue(purchases);
     renderHistoryList(purchases);
@@ -242,7 +242,7 @@ export async function loadHistory(): Promise<void> {
       account_ids: accountIDs
     };
     const data = await api.getHistory(filters) as unknown as HistoryResponse;
-    renderHistorySummary(data.summary || {});
+    renderHistorySummary(data.summary ?? null);
     const purchases = data.purchases || [];
     renderApprovalQueue(purchases);
     renderHistoryList(purchases);
@@ -262,35 +262,64 @@ export async function loadHistory(): Promise<void> {
   }
 }
 
-function renderHistorySummary(summary: HistorySummary): void {
+function renderHistorySummary(summary: HistorySummary | null): void {
   const container = document.getElementById('history-summary');
   if (!container) return;
 
+  // When the API omits the summary field entirely (older deploy, partial
+  // response, or an error absorbed upstream), render an explicit unknown
+  // state on each card rather than fabricating all-zero values that look
+  // like real financial aggregates.
+  if (summary === null || summary === undefined) {
+    container.innerHTML = `
+      <div class="card">
+        <h3>Total Purchases</h3>
+        <p class="value">--</p>
+      </div>
+      <div class="card">
+        <h3>Total Upfront Spent</h3>
+        <p class="value">--</p>
+      </div>
+      <div class="card">
+        <h3>Monthly Savings</h3>
+        <p class="value savings">--</p>
+      </div>
+      <div class="card">
+        <h3>Annual Savings</h3>
+        <p class="value savings">--</p>
+      </div>
+    `;
+    return;
+  }
+
   // total_completed / total_pending fall back to total_purchases so the
   // summary renders sensibly against an older API deploy that hasn't shipped
   // the new counters yet.
-  const total = summary.total_purchases ?? 0;
+  const total = summary.total_purchases ?? null;
+  const totalDisplay = total !== null ? String(total) : '--';
   const completed = summary.total_completed ?? total;
   const pending = summary.total_pending ?? 0;
-  const detail = pending > 0 ? `<p class="detail">${completed} completed · ${pending} pending</p>` : '';
+  const detail = (total !== null && pending > 0)
+    ? `<p class="detail">${completed} completed · ${pending} pending</p>`
+    : '';
 
   container.innerHTML = `
     <div class="card">
       <h3>Total Purchases</h3>
-      <p class="value">${total}</p>
+      <p class="value">${totalDisplay}</p>
       ${detail}
     </div>
     <div class="card">
       <h3>Total Upfront Spent</h3>
-      <p class="value">${formatCurrency(summary.total_upfront)}</p>
+      <p class="value">${formatCurrency(summary.total_upfront ?? null)}</p>
     </div>
     <div class="card">
       <h3>Monthly Savings</h3>
-      <p class="value savings">${formatCurrency(summary.total_monthly_savings)}</p>
+      <p class="value savings">${formatCurrency(summary.total_monthly_savings ?? null)}</p>
     </div>
     <div class="card">
       <h3>Annual Savings</h3>
-      <p class="value savings">${formatCurrency(summary.total_annual_savings)}</p>
+      <p class="value savings">${formatCurrency(summary.total_annual_savings ?? null)}</p>
     </div>
   `;
 }
diff --git a/frontend/src/modules/savings-history.ts b/frontend/src/modules/savings-history.ts
index a7e28f3f0..7d4b61031 100644
--- a/frontend/src/modules/savings-history.ts
+++ b/frontend/src/modules/savings-history.ts
@@ -273,7 +273,12 @@ function renderSavingsStats(data: SavingsAnalyticsResponse): void {
     let peakSavings = 0;
 
     for (const dp of dataPoints) {
-        const savings = dp.total_savings || 0;
+        // M-6: the type contract says total_savings is non-nullable, but a
+        // future API change could introduce null; guard with ?? 0 only for the
+        // aggregation where 0 is the correct absent interpretation (a missing
+        // bucket contributes nothing to the total/peak -- not the same as a
+        // fabricated $0 on a money display path).
+        const savings = dp.total_savings ?? 0;
         totalSavings += savings;
         if (savings > peakSavings) {
             peakSavings = savings;
@@ -316,9 +321,17 @@ function renderSavingsStats(data: SavingsAnalyticsResponse): void {
 }
 
 /**
- * Format currency value
+ * Format currency value for chart contexts (abbreviated K for >= $1000).
+ *
+ * M-7: add a null/non-finite guard so a future API change that introduces
+ * null on total_savings or cumulative_savings doesn't crash the chart with
+ * a TypeError on .toFixed(). Returns '--' for absent/non-finite values,
+ * matching the shared formatCurrency sentinel in utils.ts.
  */
-function formatCurrency(value: number): string {
+function formatCurrency(value: number | null | undefined): string {
+    if (value == null || !Number.isFinite(value)) {
+        return '--';
+    }
     if (value >= 1000) {
         return `$${(value / 1000).toFixed(2)}K`;
     }
@@ -351,8 +364,10 @@ function renderSavingsChart(dataPoints: SavingsDataPoint[], interval: string, un
     });
 
     const suffix = unitSuffix(unit);
-    const savingsData = dataPoints.map(dp => convertFromMonthly(dp.total_savings || 0, unit));
-    const cumulativeSavings = dataPoints.map(dp => dp.cumulative_savings || 0);
+    // M-6: use ?? 0 (not || 0) to treat a genuine 0-saving bucket correctly;
+    // || 0 would coerce a legitimate $0 data point the same as a missing one.
+    const savingsData = dataPoints.map(dp => convertFromMonthly(dp.total_savings ?? 0, unit));
+    const cumulativeSavings = dataPoints.map(dp => dp.cumulative_savings ?? 0);
 
     if (savingsChart) {
         savingsChart.destroy();
diff --git a/frontend/src/plans.ts b/frontend/src/plans.ts
index 0a2e53cf7..494175760 100644
--- a/frontend/src/plans.ts
+++ b/frontend/src/plans.ts
@@ -875,7 +875,7 @@ function planServiceLabel(slug: string): string {
 // are plan-level today, not per-service, so picking any entry is
 // correct. If the model ever differentiates per service, this needs
 // to render the same way.
-function extractPlanInfo(plan: BackendPlan): { provider: string; service: string; term: number; coverage: number } {
+function extractPlanInfo(plan: BackendPlan): { provider: string | null; service: string; term: number | null; coverage: number | null } {
   const services = plan.services || {};
   const serviceValues = Object.values(services);
   const firstService = serviceValues[0];
@@ -885,14 +885,18 @@ function extractPlanInfo(plan: BackendPlan): { provider: string; service: string
       : serviceValues
           .map(s => planServiceLabel(s.service || '—'))
           .join(', ');
+    // H-4: never fabricate provider/term/coverage when absent.
+    // Return null so callers can surface '--'/'Unknown' in the display
+    // layer rather than silently committing to aws/3yr/80% defaults.
     return {
-      provider: firstService.provider || 'aws',
+      provider: firstService.provider || null,
       service,
-      term: firstService.term || 3,
-      coverage: firstService.coverage || 80
+      term: firstService.term || null,
+      coverage: firstService.coverage ?? null
     };
   }
-  return { provider: 'aws', service: '—', term: 3, coverage: 80 };
+  // No services at all: all fields are unknown.
+  return { provider: null, service: '—', term: null, coverage: null };
 }
 
 // Returns true when the plan's next_execution_date is strictly before today.
@@ -980,11 +984,11 @@ function renderPlanCard(plan: BackendPlan, canManagePlan: boolean, canDeletePlan
           </div>
           <div class="plan-detail">
             <span class="plan-detail-label">Term</span>
-            <span class="plan-detail-value">${formatTerm(info.term)}</span>
+            <span class="plan-detail-value">${info.term !== null ? formatTerm(info.term) : '--'}</span>
           </div>
           <div class="plan-detail">
             <span class="plan-detail-label">Coverage</span>
-            <span class="plan-detail-value">${info.coverage}%</span>
+            <span class="plan-detail-value">${info.coverage !== null ? `${info.coverage}%` : '--'}</span>
           </div>
           <div class="plan-detail">
             <span class="plan-detail-label">Ramp Schedule</span>
@@ -1114,10 +1118,14 @@ async function editPlan(planId: string): Promise<void> {
       rampValue = 'custom';
     }
 
-    // Get payment option from services and normalize for provider
+    // Get payment option from services and normalize for provider.
+    // H-5: do not pre-select 'no-upfront' when the payment field is absent.
+    // An absent payment field leaves the select empty so the user must
+    // explicitly choose rather than silently inheriting a fabricated default
+    // that could change the plan's payment type on re-save.
     const firstService = Object.values(backendPlan.services || {})[0];
-    const rawPayment = firstService?.payment || 'no-upfront';
-    const payment = normalizePaymentValue(rawPayment, info.provider);
+    const rawPayment = firstService?.payment || null;
+    const payment = rawPayment !== null ? normalizePaymentValue(rawPayment, info.provider ?? '') : '';
 
     const titleEl = document.getElementById('plan-modal-title');
     if (titleEl) titleEl.textContent = 'Edit Purchase Plan';
@@ -1126,21 +1134,29 @@ async function editPlan(planId: string): Promise<void> {
     (document.getElementById('plan-name') as HTMLInputElement).value = backendPlan.name;
     (document.getElementById('plan-description') as HTMLTextAreaElement).value = '';
 
-    // Set provider and service first
-    (document.getElementById('plan-provider') as HTMLSelectElement).value = info.provider;
+    // Set provider and service first. When provider is absent from the API
+    // response, leave the select at its default empty/first option rather
+    // than fabricating 'aws' (H-4: never default provider silently).
+    const providerSelect = document.getElementById('plan-provider') as HTMLSelectElement;
+    providerSelect.value = info.provider ?? '';
     (document.getElementById('plan-service') as HTMLSelectElement).value = info.service;
 
     // Update term/payment options based on provider/service
     const termSelect = document.getElementById('plan-term') as HTMLSelectElement;
     const paymentSelect = document.getElementById('plan-payment') as HTMLSelectElement;
-    populateTermSelect(termSelect, info.provider, info.service);
-    populatePaymentSelect(paymentSelect, info.provider, info.service);
+    populateTermSelect(termSelect, info.provider ?? '', info.service);
+    populatePaymentSelect(paymentSelect, info.provider ?? '', info.service);
+
+    // Set term only when present; absent term leaves the select unset so
+    // the user must explicitly choose rather than silently inheriting a
+    // fabricated 3yr default (H-4).
+    termSelect.value = info.term !== null ? String(info.term) : '';
 
-    // Now set term and payment values
-    termSelect.value = String(info.term);
     paymentSelect.value = payment;
 
-    (document.getElementById('plan-coverage') as HTMLInputElement).value = String(info.coverage);
+    // Set coverage only when present; absent coverage leaves the input
+    // blank so the user sees the field is missing (H-4).
+    (document.getElementById('plan-coverage') as HTMLInputElement).value = info.coverage !== null ? String(info.coverage) : '';
     (document.getElementById('plan-auto-purchase') as HTMLInputElement).checked = backendPlan.auto_purchase;
     (document.getElementById('plan-notify-days') as HTMLInputElement).value = String(backendPlan.notification_days_before || 3);
     (document.getElementById('plan-enabled') as HTMLInputElement).checked = backendPlan.enabled;
diff --git a/frontend/src/recommendations.ts b/frontend/src/recommendations.ts
index 3c29a3456..28a384174 100644
--- a/frontend/src/recommendations.ts
+++ b/frontend/src/recommendations.ts
@@ -78,7 +78,7 @@ const expandedSpGroups = new Set<string>();
 // total_upfront_cost / avg_payback_months stable while the savings card
 // itself is recomputed client-side from the *visible* recs (so it stays
 // in sync with the per-cell banner range under the table).
-let lastRecommendationsSummary: RecommendationsSummary = {};
+let lastRecommendationsSummary: RecommendationsSummary | null = null;
 
 // #284 (CR follow-up): guard against concurrent stale-load refreshes. If a
 // background refresh is already in flight, any subsequent stale detection
@@ -513,7 +513,10 @@ export async function loadRecommendations(): Promise<void> {
     // filter would shrink the banner range under the table while leaving
     // the card pinned to the unfiltered totals — the same divergence
     // #272 was supposed to close.
-    lastRecommendationsSummary = data.summary || {};
+    // M-5: propagate null/absent summary rather than collapsing to {} which
+    // makes all summary fields read as undefined and silently coerce to 0/--
+    // in downstream display code. Callers already guard for empty/null summary.
+    lastRecommendationsSummary = data.summary ?? null;
     renderRecommendationsList(visibleByPreference as unknown as LocalRecommendation[]);
 
     // Issue #909: render the lookback selector + scope tooltip in the
@@ -688,7 +691,7 @@ async function onLookbackChange(rawValue: string): Promise<void> {
 }
 
 function renderRecommendationsSummary(
-  _summary: RecommendationsSummary,
+  _summary: RecommendationsSummary | null,
   recommendations: readonly LocalRecommendation[],
 ): void {
   const container = document.getElementById('recommendations-summary');
@@ -725,21 +728,28 @@ function renderRecommendationsSummary(
   const isSelectionView = selectedVisible.length > 0 && plr.cellCount > 0;
 
   // issue #319: scale savings by the active cost period.
+  // M-1: use ?? null so absent savings propagate to '--' via formatCostForPeriod
+  // rather than silently collapsing to $0 (scaleCost returns null only when input
+  // is null/undefined; a genuine $0 saving returns 0, not null).
   const period = state.getCostPeriod();
-  const scaledSavingsMin = scaleCost(plr.savingsMin, period) ?? 0;
-  const scaledSavingsMax = scaleCost(plr.savingsMax, period) ?? 0;
+  const scaledSavingsMin = scaleCost(plr.savingsMin, period) ?? null;
+  const scaledSavingsMax = scaleCost(plr.savingsMax, period) ?? null;
   // Use target.length (variant count) for the guard conditions so they
   // stay consistent with the new KPI value below (closes #748).
   const hasRecs = target.length > 0;
-  const savingsText = hasRecs && plr.savingsMax > 0
+  const savingsText = hasRecs && plr.savingsMax > 0 && scaledSavingsMin !== null && scaledSavingsMax !== null
     ? formatScaledRange(scaledSavingsMin, scaledSavingsMax, period)
-    : formatCostForPeriod(0, period);
+    : hasRecs && (plr.savingsMin === null || plr.savingsMax === null)
+      ? '--'
+      : formatCostForPeriod(0, period);
   const upfrontText = hasRecs && plr.upfrontMax > 0
     ? formatSavingsRange(plr.upfrontMin, plr.upfrontMax)
     : formatCurrency(0);
+  // L-2: use '--' for absent/inapplicable payback rather than '0 months'
+  // which falsely implies instantaneous payback.
   const paybackText = hasRecs && plr.paybackMonthsMax > 0
     ? formatPaybackRange(plr.paybackMonthsMin, plr.paybackMonthsMax)
-    : '0 months';
+    : '—';
   // issue #748: count VARIANTS (= target.length), not cells (= plr.cellCount).
   // "Showing X of X" in the filter-status bar also counts variants, so both
   // numbers now agree on the same dataset.
@@ -1677,8 +1687,12 @@ function numericCellValue(r: LocalRecommendation, col: state.RecommendationsColu
   const period = state.getCostPeriod();
   switch (col) {
     case 'count':                return r.count ?? 0;
-    case 'savings':              return scaleCost(r.savings, period) ?? 0;
-    case 'upfront_cost':         return r.upfront_cost ?? 0;
+    // M-2: return NaN for null savings/upfront_cost so numeric filter
+    // predicates (e.g. "savings = 0") don't match rows where the field is
+    // simply absent rather than genuinely zero. Consistent with monthly_cost
+    // and on_demand_monthly which already use NaN for the same reason.
+    case 'savings':              return scaleCost(r.savings, period) ?? Number.NaN;
+    case 'upfront_cost':         return r.upfront_cost ?? Number.NaN;
     // Return NaN for null monthly_cost so numeric filter predicates (e.g. "= 0")
     // don't match rows where the provider simply didn't report a monthly cost.
     case 'monthly_cost':         return scaleCost(r.monthly_cost, period) ?? Number.NaN;

From 9df488b7d82fd5deda7f73c570876db6382d5516 Mon Sep 17 00:00:00 2001
From: Cristian Magherusan-Stanciu <cristi.magherusan@gmail.com>
Date: Sun, 7 Jun 2026 14:34:36 -0700
Subject: [PATCH 7/9] fix(frontend): escape quotes in escapeHtml to close
 data-attribute XSS (CR #1046)

Replace the div.textContent/innerHTML impl with explicit character replacement
so double and single quotes are escaped to &quot; and &#39;. Values interpolated
into data-* attributes via innerHTML could break out of the attribute value when
the string contained a raw quote, enabling XSS (plans.ts:731+, 969-970,
1009-1013). Fix also updates the two utils.test.ts cases that pinned the buggy
unescaped-quote output, hardens the providerBadgeHtml mock in plans.test.ts
to escape the label before injecting it into the test DOM, and strengthens the
history.test.ts absent-total_upfront assertion to verify formatCurrency is
called with null and the rendered card shows '--' instead of '$0'.
---
 frontend/src/__tests__/history.test.ts | 17 ++++++++++-------
 frontend/src/__tests__/plans.test.ts   |  8 +++++++-
 frontend/src/__tests__/utils.test.ts   |  4 ++--
 frontend/src/utils.ts                  |  9 ++++++---
 4 files changed, 25 insertions(+), 13 deletions(-)

diff --git a/frontend/src/__tests__/history.test.ts b/frontend/src/__tests__/history.test.ts
index 09265cff0..f4227754c 100644
--- a/frontend/src/__tests__/history.test.ts
+++ b/frontend/src/__tests__/history.test.ts
@@ -13,7 +13,8 @@ jest.mock('../navigation', () => ({
 }));
 
 jest.mock('../utils', () => ({
-  formatCurrency: jest.fn((val) => `$${val || 0}`),
+  // Mirrors the real formatCurrency behaviour: null/undefined/NaN -> '--', numbers -> '$<val>'
+  formatCurrency: jest.fn((val) => (val === null || val === undefined || isNaN(val)) ? '--' : `$${val}`),
   formatDate: jest.fn((val) => val ? new Date(val).toLocaleDateString() : ''),
   formatTerm: jest.fn((years) => years == null ? '' : `${years} Year${years === 1 ? '' : 's'}`),
   escapeHtml: jest.fn((str) => str || ''),
@@ -481,13 +482,15 @@ describe('History Module', () => {
       const summary = document.getElementById('history-summary');
       // total_purchases is present; money fields that are absent must be '--'
       expect(summary?.textContent).toContain('3');
-      // The formatCurrency mock returns '$<val>' for numbers and '$0' for null/0;
-      // since total_upfront is absent (undefined), formatCurrency receives null
-      // and must return '--' (as the real impl does post-fix).
-      // In the test environment the mock is: (val) => `$${val || 0}` so we
-      // verify the produced HTML is consistent with the -- path by confirming
-      // the summary was rendered (not the error/unknown banner).
+      // formatCurrency must be called with null for the absent total_upfront
+      // field and must produce '--', not '$0'. This assertion fails if the
+      // production code regresses to passing 0 or a fabricated value.
+      const { formatCurrency } = jest.requireMock('../utils') as { formatCurrency: jest.Mock };
+      expect(formatCurrency).toHaveBeenCalledWith(null);
+      // The rendered card must show '--', not a dollar amount.
       expect(summary?.innerHTML).toContain('Total Upfront Spent');
+      expect(summary?.innerHTML).toContain('<p class="value">--</p>');
+      expect(summary?.innerHTML).not.toContain('<p class="value">$0</p>');
     });
 
     test('renderHistorySummary: null summary shows unknown-state banner with all four cards', async () => {
diff --git a/frontend/src/__tests__/plans.test.ts b/frontend/src/__tests__/plans.test.ts
index 78a52def3..d98f96bb8 100644
--- a/frontend/src/__tests__/plans.test.ts
+++ b/frontend/src/__tests__/plans.test.ts
@@ -111,7 +111,13 @@ jest.mock('../utils', () => ({
   populateAccountFilter: jest.fn(() => Promise.resolve()),
   // providerBadgeHtml added for H1 fix: returns a deterministic span so XSS
   // regression tests can assert neutralisation without a real DOM escaper.
-  providerBadgeHtml: jest.fn((p) => `<span class="provider-badge ${['aws','azure','gcp'].includes((p||'').toLowerCase()) ? (p||'').toLowerCase() : ''}">${(p||'').toUpperCase()}</span>`),
+  providerBadgeHtml: jest.fn((p) => {
+    const cls = ['aws','azure','gcp'].includes((p||'').toLowerCase()) ? (p||'').toLowerCase() : '';
+    const label = (p||'').toUpperCase()
+      .replace(/&/g, '&amp;').replace(/</g, '&lt;').replace(/>/g, '&gt;')
+      .replace(/"/g, '&quot;').replace(/'/g, '&#39;');
+    return `<span class="provider-badge ${cls}">${label}</span>`;
+  }),
   CURRENCY_DEFAULT_DIGITS: 0,
 }));
 
diff --git a/frontend/src/__tests__/utils.test.ts b/frontend/src/__tests__/utils.test.ts
index 9308c71e1..647ec4c08 100644
--- a/frontend/src/__tests__/utils.test.ts
+++ b/frontend/src/__tests__/utils.test.ts
@@ -205,7 +205,7 @@ describe('throttle', () => {
 
 describe('escapeHtml', () => {
   test('escapes HTML special characters', () => {
-    expect(escapeHtml('<script>alert("xss")</script>')).toBe('&lt;script&gt;alert("xss")&lt;/script&gt;');
+    expect(escapeHtml('<script>alert("xss")</script>')).toBe('&lt;script&gt;alert(&quot;xss&quot;)&lt;/script&gt;');
   });
 
   test('escapes ampersands', () => {
@@ -213,7 +213,7 @@ describe('escapeHtml', () => {
   });
 
   test('escapes quotes', () => {
-    expect(escapeHtml('"test"')).toBe('"test"');
+    expect(escapeHtml('"test"')).toBe('&quot;test&quot;');
   });
 
   test('returns empty string for null/undefined', () => {
diff --git a/frontend/src/utils.ts b/frontend/src/utils.ts
index 239d46513..7c4b6f648 100644
--- a/frontend/src/utils.ts
+++ b/frontend/src/utils.ts
@@ -155,9 +155,12 @@ export function throttle<T extends (...args: unknown[]) => unknown>(
  */
 export function escapeHtml(str: string | null | undefined): string {
   if (!str) return '';
-  const div = document.createElement('div');
-  div.textContent = str;
-  return div.innerHTML;
+  return str
+    .replace(/&/g, '&amp;')
+    .replace(/</g, '&lt;')
+    .replace(/>/g, '&gt;')
+    .replace(/"/g, '&quot;')
+    .replace(/'/g, '&#39;');
 }
 
 /**

From f9abe3504a032ee732dd58022c8ed7df84c9df2b Mon Sep 17 00:00:00 2001
From: Claude <noreply@anthropic.com>
Date: Mon, 8 Jun 2026 03:33:43 +0000
Subject: [PATCH 8/9] fix(frontend): show '--' sentinel for empty savings
 range, align mock

- dashboard.ts: replace formatCurrency(0) with literal '--' when
  range.cellCount === 0 (CR finding: empty recs fabricated a real $0
  instead of the absent-data sentinel)
- dashboard.test.ts: align formatCurrency mock with null/non-finite
  sentinel contract (val==null or !isFinite returns '--'); update three
  stale assertions that expected '$0' when recs are absent or failed

Addresses CR review actionable findings on PR #1046 (outside-diff
comments dashboard.ts:252-267 and dashboard.test.ts:104-111).
Finding plans.ts:731-735 skipped as CR misread: escapeHtml in utils.ts
already escapes both '"' and "'" (lines 162-163).
---
 frontend/src/__tests__/dashboard.test.ts | 23 +++++++++++++++--------
 frontend/src/dashboard.ts                |  2 +-
 2 files changed, 16 insertions(+), 9 deletions(-)

diff --git a/frontend/src/__tests__/dashboard.test.ts b/frontend/src/__tests__/dashboard.test.ts
index 8c4d23c04..43623f2da 100644
--- a/frontend/src/__tests__/dashboard.test.ts
+++ b/frontend/src/__tests__/dashboard.test.ts
@@ -102,7 +102,11 @@ jest.mock('../state', () => ({
 
 // Mock utils
 jest.mock('../utils', () => ({
-  formatCurrency: jest.fn((val) => `$${val || 0}`),
+  formatCurrency: jest.fn((val) => {
+    const n = typeof val === 'number' ? val : Number(val);
+    if (val == null || !Number.isFinite(n)) return '--';
+    return `$${n}`;
+  }),
   getDateParts: jest.fn(() => ({ day: 15, month: 'Jan' })),
   escapeHtml: jest.fn((str) => str || ''),
   populateAccountFilter: jest.fn(() => Promise.resolve()),
@@ -657,11 +661,12 @@ describe('Dashboard Module', () => {
       expect(summary?.innerHTML).toContain('Active Commitments');
       expect(summary?.innerHTML).toContain('Current Coverage');
       expect(summary?.innerHTML).toContain('YTD Savings');
-      // Savings card must fall back to $0 (not throw or go blank).
+      // Savings card must fall back to '--' (not throw, go blank, or fabricate $0).
       const savingsCard = summary?.querySelector('.card');
       expect(savingsCard?.textContent).toContain('Potential Monthly Savings');
-      // mockPageLevelRange returns cellCount=0 for empty groups, so formatCurrency(0) = '$0'
-      expect(savingsCard?.innerHTML).toContain('$0');
+      // When recs fail cellCount=0, the sentinel '--' is shown instead of fabricated $0.
+      expect(savingsCard?.innerHTML).toContain('--');
+      expect(savingsCard?.innerHTML).not.toContain('$0');
     });
 
     // #293: summary.potential_monthly_savings is no longer the source for
@@ -721,10 +726,11 @@ describe('Dashboard Module', () => {
       expect(summary?.innerHTML).toContain('Current Coverage');
       expect(summary?.innerHTML).toContain('YTD Savings');
 
-      // Savings card falls back to $0 when recs are absent.
+      // Savings card shows '--' when recs are absent (no fabricated $0).
       const savingsCard = summary?.querySelector('.card');
       expect(savingsCard?.textContent).toContain('Potential Monthly Savings');
-      expect(savingsCard?.innerHTML).toContain('$0');
+      expect(savingsCard?.innerHTML).toContain('--');
+      expect(savingsCard?.innerHTML).not.toContain('$0');
     });
 
     // #304: getRecommendations returns a non-array object (e.g. a wrapped
@@ -749,9 +755,10 @@ describe('Dashboard Module', () => {
       const summary = document.getElementById('summary');
       expect(summary?.innerHTML).toContain('Active Commitments');
 
-      // Savings card falls back to $0.
+      // Savings card shows '--' (no fabricated $0 when recs are absent).
       const savingsCard = summary?.querySelector('.card');
-      expect(savingsCard?.innerHTML).toContain('$0');
+      expect(savingsCard?.innerHTML).toContain('--');
+      expect(savingsCard?.innerHTML).not.toContain('$0');
     });
 
     // #749: the real backend always returns the envelope shape
diff --git a/frontend/src/dashboard.ts b/frontend/src/dashboard.ts
index 809463ad7..5d21187e0 100644
--- a/frontend/src/dashboard.ts
+++ b/frontend/src/dashboard.ts
@@ -261,7 +261,7 @@ function renderDashboardSummary(data: DashboardSummary, recs: readonly LocalReco
     const range = pageLevelRange(groups);
     savingsDisplay = range.cellCount > 0
       ? formatSavingsRange(range.savingsMin, range.savingsMax)
-      : formatCurrency(0);
+      : '--';
   } catch (recErr) {
     console.warn('Dashboard: failed to compute savings range from recommendations:', recErr);
     savingsDisplay = '--';

From fe49c47ba260245f36e8893bdb2d27e3105bd1bb Mon Sep 17 00:00:00 2001
From: Cristian Magherusan-Stanciu <cristi.magherusan@gmail.com>
Date: Sun, 7 Jun 2026 21:14:40 -0700
Subject: [PATCH 9/9] fix(frontend): use attribute-safe escaping for data-*
 injections (XSS)

Introduces escapeHtmlAttr as a semantic alias for escapeHtml that makes
clear the caller is escaping an attribute value. The escapeHtml
implementation was already updated in the prior commit to encode " and '
(closing the root vulnerability); escapeHtmlAttr calls through to it so
all attribute-context injections are covered.

Replaces the remaining escapeHtml calls in data-attribute context:
- history.ts: data-retry-id and both data-execution-id sites
- plans.ts: data-id and data-name on all plan/purchase action buttons

Adds escapeHtmlAttr unit tests to utils.test.ts (quote, ampersand, and
bracket encoding; null/undefined; safe strings) and a regression test in
history.test.ts that fails against the pre-fix quote-transparent path.
---
 frontend/src/__tests__/history.test.ts | 43 ++++++++++++++++++++++++++
 frontend/src/__tests__/utils.test.ts   | 30 ++++++++++++++++++
 frontend/src/history.ts                | 12 +++----
 frontend/src/plans.ts                  | 22 ++++++-------
 frontend/src/utils.ts                  | 12 ++++++-
 5 files changed, 101 insertions(+), 18 deletions(-)

diff --git a/frontend/src/__tests__/history.test.ts b/frontend/src/__tests__/history.test.ts
index f4227754c..6a6cbc0f5 100644
--- a/frontend/src/__tests__/history.test.ts
+++ b/frontend/src/__tests__/history.test.ts
@@ -18,6 +18,13 @@ jest.mock('../utils', () => ({
   formatDate: jest.fn((val) => val ? new Date(val).toLocaleDateString() : ''),
   formatTerm: jest.fn((years) => years == null ? '' : `${years} Year${years === 1 ? '' : 's'}`),
   escapeHtml: jest.fn((str) => str || ''),
+  // escapeHtmlAttr must encode " and ' to block attribute-boundary injection.
+  // This mock mirrors the real implementation so tests catch regressions where
+  // code reverts to the quote-transparent escapeHtml in attribute context.
+  escapeHtmlAttr: jest.fn((str: string | null | undefined) => {
+    if (!str) return '';
+    return str.replace(/&/g, '&amp;').replace(/</g, '&lt;').replace(/>/g, '&gt;').replace(/"/g, '&quot;').replace(/'/g, '&#39;');
+  }),
   populateAccountFilter: jest.fn(() => Promise.resolve())
 }));
 
@@ -493,6 +500,42 @@ describe('History Module', () => {
       expect(summary?.innerHTML).not.toContain('<p class="value">$0</p>');
     });
 
+    // Regression: attribute-boundary XSS via purchase_id containing a double-quote.
+    // Pre-fix, data-approve-id / data-cancel-id / data-retry-id / data-execution-id
+    // used escapeHtml which did not encode ", allowing a crafted ID to break out of
+    // the attribute and inject markup. Post-fix, escapeHtmlAttr is used and " becomes
+    // &quot; so the attribute stays well-formed.
+    test('encodes double-quotes in purchase_id for data-* attributes (XSS regression)', async () => {
+      const maliciousId = 'a" onmouseover="alert(1)';
+      (api.getHistory as jest.Mock).mockResolvedValue({
+        summary: {},
+        purchases: [
+          {
+            purchase_id: maliciousId,
+            status: 'pending',
+            provider: 'aws',
+            service: 'ec2',
+            resource_type: 't3.medium',
+            region: 'us-east-1',
+            count: 1,
+            term: 1,
+            upfront_cost: 100,
+            estimated_savings: 10,
+            can_approve: true,
+            can_cancel: true,
+          }
+        ]
+      });
+
+      await loadHistory();
+
+      const html = document.getElementById('history-list')?.innerHTML || '';
+      // The encoded form must appear in the attribute value.
+      expect(html).toContain('&quot;');
+      // The raw injection payload must not appear as unescaped markup.
+      expect(html).not.toContain('onmouseover="alert(1)"');
+    });
+
     test('renderHistorySummary: null summary shows unknown-state banner with all four cards', async () => {
       (api.getHistory as jest.Mock).mockResolvedValue({
         summary: null as unknown as undefined,
diff --git a/frontend/src/__tests__/utils.test.ts b/frontend/src/__tests__/utils.test.ts
index 647ec4c08..f2042cee3 100644
--- a/frontend/src/__tests__/utils.test.ts
+++ b/frontend/src/__tests__/utils.test.ts
@@ -10,6 +10,7 @@ import {
   debounce,
   throttle,
   escapeHtml,
+  escapeHtmlAttr,
   parseQueryParams,
   buildUrl,
   deepClone,
@@ -228,6 +229,35 @@ describe('escapeHtml', () => {
   });
 });
 
+describe('escapeHtmlAttr', () => {
+  test('encodes double-quote to &quot; (attribute boundary safety)', () => {
+    // A raw " would terminate the surrounding attribute and allow markup injection.
+    expect(escapeHtmlAttr('"')).toBe('&quot;');
+    expect(escapeHtmlAttr('a" onmouseover="alert(1)')).toBe('a&quot; onmouseover=&quot;alert(1)');
+  });
+
+  test('encodes single-quote to &#39;', () => {
+    expect(escapeHtmlAttr("'")).toBe('&#39;');
+    expect(escapeHtmlAttr("it's")).toBe('it&#39;s');
+  });
+
+  test('still encodes & < > (inherits from escapeHtml)', () => {
+    expect(escapeHtmlAttr('&')).toBe('&amp;');
+    expect(escapeHtmlAttr('<img>')).toBe('&lt;img&gt;');
+    expect(escapeHtmlAttr('x"><img src=x onerror=alert(1)>')).toBe('x&quot;&gt;&lt;img src=x onerror=alert(1)&gt;');
+  });
+
+  test('returns empty string for null/undefined', () => {
+    expect(escapeHtmlAttr(null as unknown as string)).toBe('');
+    expect(escapeHtmlAttr(undefined as unknown as string)).toBe('');
+    expect(escapeHtmlAttr('')).toBe('');
+  });
+
+  test('passes through safe strings unchanged', () => {
+    expect(escapeHtmlAttr('abc-123_OK')).toBe('abc-123_OK');
+  });
+});
+
 describe('parseQueryParams', () => {
   test('parses query string correctly', () => {
     const result = parseQueryParams('?foo=bar&baz=qux');
diff --git a/frontend/src/history.ts b/frontend/src/history.ts
index 110d9f586..c4990c050 100644
--- a/frontend/src/history.ts
+++ b/frontend/src/history.ts
@@ -4,7 +4,7 @@
 
 import * as api from './api';
 import * as state from './state';
-import { formatCurrency, formatDate, formatTerm, escapeHtml } from './utils';
+import { formatCurrency, formatDate, formatTerm, escapeHtml, escapeHtmlAttr } from './utils';
 import type { HistoryResponse, HistorySummary, HistoryPurchase } from './types';
 import { switchTab } from './navigation';
 import { confirmDialog } from './confirmDialog';
@@ -544,10 +544,10 @@ function renderPendingActionButtons(p: HistoryPurchase): string {
   if (!p.purchase_id) return '';
   const buttons: string[] = [];
   if (canApprovePendingRow(p)) {
-    buttons.push(`<button type="button" class="btn-link history-approve-btn" data-approve-id="${escapeHtml(p.purchase_id)}">Approve</button>`);
+    buttons.push(`<button type="button" class="btn-link history-approve-btn" data-approve-id="${escapeHtmlAttr(p.purchase_id)}">Approve</button>`);
   }
   if (canCancelPendingRow(p)) {
-    buttons.push(`<button type="button" class="btn-link history-cancel-btn" data-cancel-id="${escapeHtml(p.purchase_id)}">Cancel</button>`);
+    buttons.push(`<button type="button" class="btn-link history-cancel-btn" data-cancel-id="${escapeHtmlAttr(p.purchase_id)}">Cancel</button>`);
   }
   return buttons.join(' ');
 }
@@ -587,7 +587,7 @@ function renderActionCell(p: HistoryPurchase): string {
       const overThreshold = retryThresholdReached(p);
       const label = overThreshold ? `⚠ Retried ${p.retry_attempt_n ?? 0}× — click to override` : '↻ Retry';
       const cls = overThreshold ? 'btn-link history-retry-btn history-retry-over-threshold' : 'btn-link history-retry-btn';
-      return `<button type="button" class="${cls}" data-retry-id="${escapeHtml(p.purchase_id)}">${label}</button>`;
+      return `<button type="button" class="${cls}" data-retry-id="${escapeHtmlAttr(p.purchase_id)}">${label}</button>`;
     }
   }
 
@@ -681,7 +681,7 @@ function renderHistoryList(purchases: HistoryPurchase[]): void {
       }
       return badge;
     })();
-    const execIdAttr = p.purchase_id ? ` data-execution-id="${escapeHtml(p.purchase_id)}"` : '';
+    const execIdAttr = p.purchase_id ? ` data-execution-id="${escapeHtmlAttr(p.purchase_id)}"` : '';
     const planCellContent = renderActionCell(p);
     const monthlyCostCell = p.monthly_cost != null
       ? formatCurrency(p.monthly_cost)
@@ -982,7 +982,7 @@ export function renderApprovalQueue(purchases: HistoryPurchase[]): void {
     const monthlyCostCell = p.monthly_cost != null
       ? formatCurrency(p.monthly_cost)
       : '<span class="muted">-</span>';
-    const execIdAttr = p.purchase_id ? ` data-execution-id="${escapeHtml(p.purchase_id)}"` : '';
+    const execIdAttr = p.purchase_id ? ` data-execution-id="${escapeHtmlAttr(p.purchase_id)}"` : '';
     return `
       <tr${execIdAttr}>
         <td>${formatDate(p.timestamp)}</td>
diff --git a/frontend/src/plans.ts b/frontend/src/plans.ts
index 494175760..3a7df75a4 100644
--- a/frontend/src/plans.ts
+++ b/frontend/src/plans.ts
@@ -4,7 +4,7 @@
 
 import * as api from './api';
 import * as state from './state';
-import { formatDate, formatTerm, getStatusBadge, escapeHtml, formatCurrency, CURRENCY_DEFAULT_DIGITS, providerBadgeHtml } from './utils';
+import { formatDate, formatTerm, getStatusBadge, escapeHtml, escapeHtmlAttr, formatCurrency, CURRENCY_DEFAULT_DIGITS, providerBadgeHtml } from './utils';
 import { showToast } from './toast';
 import { confirmDialog } from './confirmDialog';
 import type { PlansResponse, LocalPlan, SavePlanData } from './types';
@@ -728,11 +728,11 @@ function renderPlannedPurchaseRow(purchase: PlannedPurchase): string {
       <td class="savings">${formatCurrency(purchase.estimated_savings)}/mo</td>
       <td><span class="status-badge ${statusClass}">${escapeHtml(purchase.status)}</span></td>
       <td class="actions">
-        ${canRunPurchase ? `<button data-action="run" data-id="${escapeHtml(purchase.id)}" class="btn-small primary" title="Run now">▶</button>` : ''}
-        ${canPauseOrResumePurchase && isPending ? `<button data-action="pause" data-id="${escapeHtml(purchase.id)}" class="btn-small" title="Pause">⏸</button>` : ''}
-        ${canPauseOrResumePurchase && isPaused ? `<button data-action="resume" data-id="${escapeHtml(purchase.id)}" class="btn-small" title="Resume">⏵</button>` : ''}
-        ${canEditPlan ? `<button data-action="edit" data-id="${escapeHtml(purchase.id)}" data-plan-id="${escapeHtml(purchase.plan_id)}" class="btn-small" title="Edit Plan">✎</button>` : ''}
-        ${canDisablePlan ? `<button data-action="disable" data-id="${escapeHtml(purchase.id)}" class="btn-small danger" title="Disable Plan">✕</button>` : ''}
+        ${canRunPurchase ? `<button data-action="run" data-id="${escapeHtmlAttr(purchase.id)}" class="btn-small primary" title="Run now">▶</button>` : ''}
+        ${canPauseOrResumePurchase && isPending ? `<button data-action="pause" data-id="${escapeHtmlAttr(purchase.id)}" class="btn-small" title="Pause">⏸</button>` : ''}
+        ${canPauseOrResumePurchase && isPaused ? `<button data-action="resume" data-id="${escapeHtmlAttr(purchase.id)}" class="btn-small" title="Resume">⏵</button>` : ''}
+        ${canEditPlan ? `<button data-action="edit" data-id="${escapeHtmlAttr(purchase.id)}" data-plan-id="${escapeHtmlAttr(purchase.plan_id)}" class="btn-small" title="Edit Plan">✎</button>` : ''}
+        ${canDisablePlan ? `<button data-action="disable" data-id="${escapeHtmlAttr(purchase.id)}" class="btn-small danger" title="Disable Plan">✕</button>` : ''}
       </td>
     </tr>
   `;
@@ -966,7 +966,7 @@ function renderPlanCard(plan: BackendPlan, canManagePlan: boolean, canDeletePlan
           ${overdueBadge}
           ${canManagePlan && !isUnassigned ? `
           <label class="toggle-label">
-            <input type="checkbox" data-action="toggle-plan" data-id="${escapeHtml(plan.id)}" ${plan.enabled ? 'checked' : ''}>
+            <input type="checkbox" data-action="toggle-plan" data-id="${escapeHtmlAttr(plan.id)}" ${plan.enabled ? 'checked' : ''}>
             <span class="slider"></span>
           </label>
           ` : ''}
@@ -1006,10 +1006,10 @@ function renderPlanCard(plan: BackendPlan, canManagePlan: boolean, canDeletePlan
           ` : ''}
         </div>
         <div class="plan-actions">
-          ${canManagePlan && !isUnassigned ? `<button data-action="add-purchases" data-id="${escapeHtml(plan.id)}" data-name="${escapeHtml(plan.name)}" class="primary">Add Purchases</button>` : ''}
-          ${canManagePlan && !isUnassigned ? `<button data-action="edit-plan" data-id="${escapeHtml(plan.id)}">Edit</button>` : ''}
-          <button data-action="view-history" data-id="${escapeHtml(plan.id)}" class="secondary">History</button>
-          ${canDeletePlan ? `<button data-action="delete-plan" data-id="${escapeHtml(plan.id)}" class="danger">Delete</button>` : ''}
+          ${canManagePlan && !isUnassigned ? `<button data-action="add-purchases" data-id="${escapeHtmlAttr(plan.id)}" data-name="${escapeHtmlAttr(plan.name)}" class="primary">Add Purchases</button>` : ''}
+          ${canManagePlan && !isUnassigned ? `<button data-action="edit-plan" data-id="${escapeHtmlAttr(plan.id)}">Edit</button>` : ''}
+          <button data-action="view-history" data-id="${escapeHtmlAttr(plan.id)}" class="secondary">History</button>
+          ${canDeletePlan ? `<button data-action="delete-plan" data-id="${escapeHtmlAttr(plan.id)}" class="danger">Delete</button>` : ''}
         </div>
       </div>
     </div>
diff --git a/frontend/src/utils.ts b/frontend/src/utils.ts
index 7c4b6f648..766c8fb45 100644
--- a/frontend/src/utils.ts
+++ b/frontend/src/utils.ts
@@ -151,7 +151,8 @@ export function throttle<T extends (...args: unknown[]) => unknown>(
 }
 
 /**
- * Escape HTML to prevent XSS
+ * Escape HTML to prevent XSS in text content (between tags).
+ * Escapes &, <, >, ", and ' so the result is safe in both text and attribute contexts.
  */
 export function escapeHtml(str: string | null | undefined): string {
   if (!str) return '';
@@ -163,6 +164,15 @@ export function escapeHtml(str: string | null | undefined): string {
     .replace(/'/g, '&#39;');
 }
 
+/**
+ * Escape a value for safe interpolation inside an HTML attribute value delimited by double quotes.
+ * Encodes &, <, >, ", and ' -- the quote escaping is essential here because a raw " terminates
+ * the attribute and allows injecting new attributes or markup. Use escapeHtml for text nodes.
+ */
+export function escapeHtmlAttr(str: string | null | undefined): string {
+  return escapeHtml(str);
+}
+
 /**
  * Parse URL query parameters
  */