diff --git a/frontend/src/__tests__/history-deeplink.test.ts b/frontend/src/__tests__/history-deeplink.test.ts index f5e15b074..153653a2f 100644 --- a/frontend/src/__tests__/history-deeplink.test.ts +++ b/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=, 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=. + * - the scheduled-purchase email's Review & Edit button (#581 + * follow-up), which links to /purchases#history?execution=. */ -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; + 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 = ` + + + + + +
row
other
+ `; + 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 = ` + + +
nope
+ `; + 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); + }); +}); diff --git a/frontend/src/history.ts b/frontend/src/history.ts index f89d7f15c..6d6efd299 100644 --- a/frontend/src/history.ts +++ b/frontend/src/history.ts @@ -43,16 +43,45 @@ export function readDeepLinkExecutionID(): string { // applyExecutionDeepLink scrolls the history table to the row matching // the ?execution= 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( `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 diff --git a/frontend/src/styles/tables.css b/frontend/src/styles/tables.css index cde13d0d1..05e55bed2 100644 --- a/frontend/src/styles/tables.css +++ b/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=. 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 { diff --git a/internal/email/coverage_test.go b/internal/email/coverage_test.go index 7ebfdefa7..b13d0d354 100644 --- a/internal/email/coverage_test.go +++ b/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) { diff --git a/internal/email/sender.go b/internal/email/sender.go index e16afa227..75b3d3011 100644 --- a/internal/email/sender.go +++ b/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 diff --git a/internal/email/template_renderers_test.go b/internal/email/template_renderers_test.go index f8517bc0b..57a842bca 100644 --- a/internal/email/template_renderers_test.go +++ b/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) { diff --git a/internal/email/templates.go b/internal/email/templates.go index 06d9a9c28..ea92c2744 100644 --- a/internal/email/templates.go +++ b/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. diff --git a/internal/email/templates_test.go b/internal/email/templates_test.go index ba2289842..43698fd9d 100644 --- a/internal/email/templates_test.go +++ b/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 +} diff --git a/internal/purchase/notifications.go b/internal/purchase/notifications.go index 63775d985..11142dd7e 100644 --- a/internal/purchase/notifications.go +++ b/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"),