sec(purchases): remove token from scheduled purchase email query-string links

frontend/src/__tests__/history-deeplink.test.ts

@@ -1,10 +1,26 @@
 /**
- * Tests for the Purchase History deep-link parser — the suppression
- * badge on the Recommendations view links to
- * #history?execution=<id>, and the handler scrolls + highlights the
- * matching row.
+ * Tests for the Purchase History deep-link parser AND the scroll+
+ * highlight handler. Two link sources land here:
+ *   - the Recommendations view's "recently purchased" suppression
+ *     badge, which links to #history?execution=<id>.
+ *   - the scheduled-purchase email's Review & Edit button (#581
+ *     follow-up), which links to /purchases#history?execution=<id>.
  */
-import { readDeepLinkExecutionID } from '../history';
+
+jest.mock('../toast', () => ({
+  showToast: jest.fn(),
+}));
+
+jest.mock('../state', () => ({
+  subscribeProvider: jest.fn().mockReturnValue(() => {}),
+  subscribeAccount: jest.fn().mockReturnValue(() => {}),
+  getCurrentUser: jest.fn(),
+  getCurrentProvider: jest.fn().mockReturnValue(''),
+  getCurrentAccountIDs: jest.fn().mockReturnValue([]),
+}));
+
+import { applyExecutionDeepLink, readDeepLinkExecutionID } from '../history';
+import { showToast } from '../toast';
 
 describe('readDeepLinkExecutionID', () => {
   const originalHash = window.location.hash;
@@ -42,3 +58,107 @@ describe('readDeepLinkExecutionID', () => {
     expect(readDeepLinkExecutionID()).toBe('xyz789');
   });
 });
+
+describe('applyExecutionDeepLink', () => {
+  const originalHash = window.location.hash;
+  const originalPathname = window.location.pathname;
+  const showToastMock = showToast as jest.MockedFunction<typeof showToast>;
+  let scrollIntoViewMock: jest.Mock;
+  let setTimeoutSpy: jest.SpyInstance;
+
+  beforeEach(() => {
+    document.body.innerHTML = '';
+    showToastMock.mockClear();
+
+    // jsdom doesn't implement scrollIntoView — patch it on the
+    // HTMLElement prototype so the production code's call site
+    // doesn't throw, and the test can assert it ran.
+    scrollIntoViewMock = jest.fn();
+    Element.prototype.scrollIntoView = scrollIntoViewMock as unknown as typeof Element.prototype.scrollIntoView;
+
+    // Stub setTimeout so we can verify the fade-out behaviour
+    // without leaving a real timer behind that bleeds across tests.
+    setTimeoutSpy = jest.spyOn(window, 'setTimeout');
+  });
+
+  afterEach(() => {
+    window.location.hash = originalHash;
+    window.history.replaceState({}, '', originalPathname);
+    setTimeoutSpy.mockRestore();
+  });
+
+  test("returns false and skips work when no execution id in hash", () => {
+    window.location.hash = '';
+    expect(applyExecutionDeepLink()).toBe(false);
+    expect(showToastMock).not.toHaveBeenCalled();
+    expect(scrollIntoViewMock).not.toHaveBeenCalled();
+  });
+
+  test("scrolls + highlights + schedules a fade when the row exists", () => {
+    document.body.innerHTML = `
+      <table>
+        <tbody>
+          <tr data-execution-id="exec-abc-123" id="target-row"><td>row</td></tr>
+          <tr data-execution-id="exec-other"><td>other</td></tr>
+        </tbody>
+      </table>
+    `;
+    window.location.hash = '#history?execution=exec-abc-123';
+
+    expect(applyExecutionDeepLink()).toBe(true);
+
+    const row = document.getElementById('target-row') as HTMLTableRowElement;
+    expect(row.classList.contains('history-row-highlight')).toBe(true);
+    expect(scrollIntoViewMock).toHaveBeenCalledTimes(1);
+    expect(scrollIntoViewMock).toHaveBeenCalledWith({ behavior: 'smooth', block: 'center' });
+    // A fade-out timer is scheduled so the highlight class is removed
+    // and the row visually settles back to baseline styling.
+    expect(setTimeoutSpy).toHaveBeenCalledWith(expect.any(Function), 4000);
+    // No fallback toast on success.
+    expect(showToastMock).not.toHaveBeenCalled();
+  });
+
+  test("falls back to an info toast and clears the hash when the row is missing", () => {
+    document.body.innerHTML = `
+      <table><tbody>
+        <tr data-execution-id="some-other-execution"><td>nope</td></tr>
+      </tbody></table>
+    `;
+    window.history.replaceState({}, '', '/purchases');
+    window.location.hash = '#history?execution=missing-exec-id-12345678';
+
+    expect(applyExecutionDeepLink()).toBe(false);
+
+    expect(scrollIntoViewMock).not.toHaveBeenCalled();
+    expect(showToastMock).toHaveBeenCalledTimes(1);
+    const toastArg = showToastMock.mock.calls[0]?.[0] as { message: string; kind: string };
+    // Toast shows a short prefix of the execution id so the user can
+    // cross-reference against the email without a wall-of-UUID toast.
+    expect(toastArg.message).toMatch(/missing-/);
+    expect(toastArg.kind).toBe('info');
+    // The execution query param is stripped from the hash so a
+    // subsequent re-render doesn't re-fire the same toast.
+    expect(window.location.hash).not.toMatch(/execution=/);
+  });
+
+  test("CSS-escapes execution IDs containing selector metacharacters", () => {
+    // Defence in depth: an attacker can't craft an execution-id link
+    // that escapes the attribute selector — CSS.escape neutralises
+    // the closing quote / brackets. The row must still match by its
+    // literal id and the highlight class must apply.
+    const nasty = 'abc"]/script>';
+    const row = document.createElement('tr');
+    row.setAttribute('data-execution-id', nasty);
+    row.id = 'nasty-row';
+    const tbody = document.createElement('tbody');
+    tbody.appendChild(row);
+    const table = document.createElement('table');
+    table.appendChild(tbody);
+    document.body.appendChild(table);
+
+    window.location.hash = '#history?execution=' + encodeURIComponent(nasty);
+
+    expect(applyExecutionDeepLink()).toBe(true);
+    expect(row.classList.contains('history-row-highlight')).toBe(true);
+  });
+});

frontend/src/history.ts

@@ -43,16 +43,45 @@ export function readDeepLinkExecutionID(): string {
 
 // applyExecutionDeepLink scrolls the history table to the row matching
 // the ?execution=<id> hash query, if any, and flashes a highlight
-// class on it. Called after each loadHistory render so the link from
-// the Recommendations badge lands on the right row. Returns true when
-// a match was found + highlighted.
-function applyExecutionDeepLink(): boolean {
+// class on it. Called after each loadHistory render so links from
+// the Recommendations badge AND the scheduled-purchase email's
+// Review & Edit button land on the right row. Returns true when a
+// match was found + highlighted.
+//
+// Falsy paths:
+//   - No execID in URL: return false silently (the common case — no
+//     deeplink was requested).
+//   - execID present but no matching row in the rendered list (e.g.
+//     the user's date filter excludes the execution, or the row hasn't
+//     been ingested yet): surface a non-blocking toast so the user
+//     understands why the page didn't jump anywhere, and clear the
+//     hash so a follow-up loadHistory() doesn't re-toast the same
+//     miss on every re-render.
+//
+// Exported for unit-test coverage.
+export function applyExecutionDeepLink(): boolean {
   const execID = readDeepLinkExecutionID();
   if (!execID) return false;
   const row = document.querySelector<HTMLTableRowElement>(
     `tr[data-execution-id="${CSS.escape(execID)}"]`,
   );
-  if (!row) return false;
+  if (!row) {
+    // Short-prefix the ID so the toast is readable but the user can
+    // still cross-reference against the email if needed.
+    const shortID = execID.length > 8 ? `${execID.slice(0, 8)}…` : execID;
+    showToast({
+      message: `Execution ${shortID} isn't in the current view — clear filters or widen the date range to find it.`,
+      kind: 'info',
+      timeout: 8_000,
+    });
+    // Drop the ?execution= from the hash so the next loadHistory()
+    // (e.g. user changes a filter) doesn't fire this toast again.
+    if (window.location.hash) {
+      const baseHash = window.location.hash.split('?')[0] ?? '';
+      window.history.replaceState({}, '', window.location.pathname + window.location.search + baseHash);
+    }
+    return false;
+  }
   row.classList.add('history-row-highlight');
   row.scrollIntoView({ behavior: 'smooth', block: 'center' });
   // Fade the highlight after a few seconds so the row goes back to

frontend/src/styles/tables.css

@@ -33,6 +33,28 @@ tr.medium-savings {
     border-left: 3px solid var(--cudly-warn);
 }
 
+/* Deep-link target highlight: applied by applyExecutionDeepLink() in
+   history.ts when the URL hash carries ?execution=<id>. The class is
+   removed after ~4s (see history.ts) so the row fades back to normal.
+   `animation` runs once on add for a brief pulse, then `background`
+   keeps a steady highlight tint until the class is removed. */
+tr.history-row-highlight {
+    background: #fff8c4;
+    animation: history-row-flash 1.2s ease-out 1;
+}
+
+tr.history-row-highlight:hover {
+    /* Override the table-wide `tr:hover` so the deep-link highlight
+       stays visible while the user mouses over the row. */
+    background: #fff3a0;
+}
+
+@keyframes history-row-flash {
+    0%   { background: #fff080; }
+    50%  { background: #fff8c4; }
+    100% { background: #fff8c4; }
+}
+
 /* Summary sections */
 #recommendations-summary,
 #history-summary {

internal/email/coverage_test.go

@@ -513,11 +513,20 @@ func TestTemplateContent_HasRequiredSections(t *testing.T) {
 	})
 
 	t.Run("scheduledPurchaseTemplate has action links", func(t *testing.T) {
-		assert.Contains(t, scheduledPurchaseTemplate, "action=edit")
-		assert.Contains(t, scheduledPurchaseTemplate, "action=pause")
-		assert.Contains(t, scheduledPurchaseTemplate, "action=cancel")
+		// Review/edit and pause links go to the dashboard (no token in URL).
+		assert.NotContains(t, scheduledPurchaseTemplate, "action=edit")
+		assert.NotContains(t, scheduledPurchaseTemplate, "action=pause")
+		assert.NotContains(t, scheduledPurchaseTemplate, "action=cancel")
+		// Cancel link uses the direct API path with execution ID and token (#406).
+		assert.Contains(t, scheduledPurchaseTemplate, "/purchases/cancel/{{.ExecutionID}}")
 		assert.Contains(t, scheduledPurchaseTemplate, ".ApprovalToken")
 		assert.Contains(t, scheduledPurchaseTemplate, ".PlanName")
+		// Review & Edit and Pause Plan deeplinks (#581 follow-up) carry
+		// the non-sensitive ExecutionID / PlanID so the SPA can scroll
+		// the user to the relevant row instead of landing on the
+		// dashboard root.
+		assert.Contains(t, scheduledPurchaseTemplate, "/purchases#history?execution={{.ExecutionID}}")
+		assert.Contains(t, scheduledPurchaseTemplate, "/plans?plan={{.PlanID}}")
 	})
 
 	t.Run("purchaseConfirmationTemplate has history link", func(t *testing.T) {

internal/email/sender.go

@@ -374,9 +374,15 @@ func dedupeCCAgainstTo(to string, cc []string) []string {
 
 // NotificationData holds data for rendering email templates
 type NotificationData struct {
-	DashboardURL      string
-	ApprovalToken     string
-	ExecutionID       string
+	DashboardURL  string
+	ApprovalToken string
+	ExecutionID   string
+	// PlanID is the parent purchase plan's UUID. Used by the Pause Plan
+	// deeplink in scheduledPurchaseTemplate to route the user to the
+	// Plans tab with the matching plan highlighted. The plan UUID is
+	// non-sensitive (the user already needs an authenticated session
+	// cookie to act on the plan), so embedding it in the URL is safe.
+	PlanID            string
 	TotalSavings      float64
 	TotalUpfrontCost  float64
 	Recommendations   []RecommendationSummary

internal/email/template_renderers_test.go

@@ -102,6 +102,7 @@ func TestRenderScheduledPurchaseEmail(t *testing.T) {
 	data := NotificationData{
 		DashboardURL:      "https://dashboard.example.com",
 		ApprovalToken:     "approval-token-xyz",
+		ExecutionID:       "exec-render-test-001",
 		TotalSavings:      2000.00,
 		TotalUpfrontCost:  8000.00,
 		PurchaseDate:      "March 15, 2024",
@@ -123,15 +124,17 @@ func TestRenderScheduledPurchaseEmail(t *testing.T) {
 
 	require.NoError(t, err)
 	assert.Contains(t, result, data.DashboardURL)
-	assert.Contains(t, result, data.ApprovalToken)
 	assert.Contains(t, result, data.PurchaseDate)
 	assert.Contains(t, result, data.PlanName)
 	assert.Contains(t, result, "7")
 	assert.Contains(t, result, "db.r5.2xlarge")
 	assert.Contains(t, result, "mysql")
-	assert.Contains(t, result, "action=edit")
-	assert.Contains(t, result, "action=pause")
-	assert.Contains(t, result, "action=cancel")
+	// Cancel link must use the direct API path with execution ID and token.
+	assert.Contains(t, result, "/purchases/cancel/exec-render-test-001?token=approval-token-xyz")
+	// Token must not appear in any other URL (review/edit or pause).
+	assert.NotContains(t, result, "action=edit")
+	assert.NotContains(t, result, "action=pause")
+	assert.NotContains(t, result, "action=cancel")
 }
 
 func TestRenderPurchaseConfirmationEmail(t *testing.T) {

internal/email/templates.go

@@ -48,11 +48,11 @@ Estimated Monthly Savings: ${{printf "%.2f" .TotalSavings}}
 
 Actions:
 --------
-[Review & Edit] {{.DashboardURL}}?action=edit&token={{urlquery .ApprovalToken}}
+[Review & Edit] {{.DashboardURL}}/purchases#history?execution={{.ExecutionID}}
 
-[Pause Plan] {{.DashboardURL}}?action=pause&token={{urlquery .ApprovalToken}}
+[Pause Plan] {{.DashboardURL}}/plans?plan={{.PlanID}}
 
-[Cancel This Purchase] {{.DashboardURL}}?action=cancel&token={{urlquery .ApprovalToken}}
+[Cancel This Purchase] {{.DashboardURL}}/purchases/cancel/{{.ExecutionID}}?token={{urlquery .ApprovalToken}}
 
 You have {{.DaysUntilPurchase}} days to modify or cancel before automatic execution.
 

internal/email/templates_test.go

@@ -536,3 +536,80 @@ func TestSender_SendPurchaseFailedNotification_MultipleFailures(t *testing.T) {
 	require.NoError(t, err)
 	mockSNS.AssertExpectations(t)
 }
+
+// TestRenderScheduledPurchaseEmail_TokenNotInReviewEditLinks is a regression
+// test for #406: the scheduled purchase notification must not embed the
+// approval token in the Review/Edit or Pause Plan URLs. These actions
+// require an authenticated dashboard session; only the Cancel link carries
+// the token, and it must use the direct API path (not the SPA root) to
+// avoid loading third-party scripts with the token in the Referer header.
+//
+// It also covers the #581-followup deeplink shape: Review & Edit and
+// Pause Plan must include the execution / plan ID so the SPA can scroll
+// the user to the relevant row instead of dumping them at the dashboard
+// root.
+func TestRenderScheduledPurchaseEmail_TokenNotInReviewEditLinks(t *testing.T) {
+	data := NotificationData{
+		DashboardURL:      "https://dashboard.example.com",
+		ApprovalToken:     "super-secret-token",
+		ExecutionID:       "exec-abc-123",
+		PlanID:            "plan-xyz-789",
+		TotalSavings:      500.00,
+		PurchaseDate:      "March 1, 2025",
+		DaysUntilPurchase: 7,
+		PlanName:          "Test Plan",
+	}
+
+	body, err := RenderScheduledPurchaseEmail(data)
+	require.NoError(t, err)
+
+	// The cancel link must use the direct API path with the execution ID and token.
+	assert.Contains(t, body, "/purchases/cancel/exec-abc-123?token=super-secret-token")
+
+	// Review & Edit must deeplink to the Purchase History row matching
+	// ExecutionID (non-sensitive UUID; auth still required via session cookie).
+	assert.Contains(t, body, "/purchases#history?execution=exec-abc-123",
+		"Review & Edit link must deeplink to the matching execution row")
+
+	// Pause Plan must deeplink to the Plans tab with the matching plan ID.
+	assert.Contains(t, body, "/plans?plan=plan-xyz-789",
+		"Pause Plan link must deeplink to the matching plan")
+
+	// The token must not appear in the review/edit or pause lines.
+	for _, line := range splitTestLines(body) {
+		if containsAnyStr(line, "Review", "Pause") {
+			assert.NotContains(t, line, "super-secret-token",
+				"line %q must not embed the approval token", line)
+		}
+	}
+}
+
+// splitTestLines splits s on newlines, returning non-empty lines.
+func splitTestLines(s string) []string {
+	var out []string
+	start := 0
+	for i := 0; i < len(s); i++ {
+		if s[i] == '\n' {
+			if line := s[start:i]; line != "" {
+				out = append(out, line)
+			}
+			start = i + 1
+		}
+	}
+	if start < len(s) {
+		out = append(out, s[start:])
+	}
+	return out
+}
+
+// containsAnyStr reports whether s contains any of the provided substrings.
+func containsAnyStr(s string, subs ...string) bool {
+	for _, sub := range subs {
+		for i := 0; i+len(sub) <= len(s); i++ {
+			if s[i:i+len(sub)] == sub {
+				return true
+			}
+		}
+	}
+	return false
+}

internal/purchase/notifications.go

@@ -132,6 +132,8 @@ func (m *Manager) buildNotificationData(plan config.PurchasePlan, exec *config.P
 	data := email.NotificationData{
 		DashboardURL:      m.dashboardURL,
 		ApprovalToken:     exec.ApprovalToken,
+		ExecutionID:       exec.ExecutionID,
+		PlanID:            plan.ID,
 		TotalSavings:      exec.EstimatedSavings,
 		TotalUpfrontCost:  exec.TotalUpfrontCost,
 		PurchaseDate:      exec.ScheduledDate.Format("January 2, 2006"),