From e38c8fde3b87c8db30800b9ec961da786cf64e9d Mon Sep 17 00:00:00 2001 From: wendyamoni-creator Date: Mon, 27 Jul 2026 09:28:52 +0000 Subject: [PATCH 1/2] fix: only invalidate cache tags when mutation response body reports success MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit request() was invalidating NEWSLETTER/STATISTICS cache tags for any POST/DELETE that returned HTTP 2xx, without inspecting the parsed payload's success field. newsletterSubscribe / newsletterUnsubscribe can return { success: false, message: '...' } with a 200 status, so a rejected subscription was still busting the statistics cache — causing an unnecessary refetch and a brief stale flicker for unrelated data. Fix: in the mutation cache-invalidation block, derive 'succeeded' from the response body: - no success field present → treat as success (non-envelope endpoints) - success: true → invalidate as before - success: false → skip invalidation entirely Tests added to 'Cache invalidation strategy' suite: • success:false 200 response → cache NOT invalidated • success:true 200 response → cache IS invalidated • no success field present → cache IS invalidated (non-envelope) Closes #1162 --- frontend/src/lib/api/__tests__/client.test.ts | 78 +++++++++++++++++++ frontend/src/lib/api/client.ts | 18 ++++- 2 files changed, 92 insertions(+), 4 deletions(-) diff --git a/frontend/src/lib/api/__tests__/client.test.ts b/frontend/src/lib/api/__tests__/client.test.ts index 5dcf3d6c..a00bf1d7 100644 --- a/frontend/src/lib/api/__tests__/client.test.ts +++ b/frontend/src/lib/api/__tests__/client.test.ts @@ -549,6 +549,84 @@ describe('API Client', () => { const second = await api.getFeaturedMarkets(); expect(second).toEqual([{ id: 1, title: 'Resolved Market' }]); }); + + it('does not invalidate cache when mutation returns success:false (#1162)', async () => { + // Prime a statistics cache entry. + (global.fetch as jest.Mock).mockResolvedValueOnce({ + ok: true, + text: async () => JSON.stringify({ total_markets: 10 }), + }); + await api.getStatistics(); + expect(global.fetch).toHaveBeenCalledTimes(1); + + // newsletterSubscribe returns 200 with success:false (business-logic failure). + (global.fetch as jest.Mock).mockResolvedValueOnce({ + ok: true, + text: async () => JSON.stringify({ success: false, message: 'Email already subscribed' }), + }); + await api.newsletterSubscribe({ email: 'existing@example.com' }); + expect(global.fetch).toHaveBeenCalledTimes(2); + + // Cache must NOT be invalidated — next getStatistics should return cached value. + const cachedStats = await api.getStatistics(); + expect(cachedStats).toEqual({ total_markets: 10 }); + // fetch should not have been called again (still 2 calls total). + expect(global.fetch).toHaveBeenCalledTimes(2); + }); + + it('invalidates cache when mutation returns success:true (#1162)', async () => { + // Prime a statistics cache entry. + (global.fetch as jest.Mock).mockResolvedValueOnce({ + ok: true, + text: async () => JSON.stringify({ total_markets: 10 }), + }); + await api.getStatistics(); + expect(global.fetch).toHaveBeenCalledTimes(1); + + // newsletterSubscribe succeeds. + (global.fetch as jest.Mock).mockResolvedValueOnce({ + ok: true, + text: async () => JSON.stringify({ success: true, message: 'Subscribed!' }), + }); + await api.newsletterSubscribe({ email: 'new@example.com' }); + expect(global.fetch).toHaveBeenCalledTimes(2); + + // Cache MUST be invalidated — next getStatistics should hit the network. + (global.fetch as jest.Mock).mockResolvedValueOnce({ + ok: true, + text: async () => JSON.stringify({ total_markets: 11 }), + }); + const freshStats = await api.getStatistics(); + expect(freshStats).toEqual({ total_markets: 11 }); + expect(global.fetch).toHaveBeenCalledTimes(3); + }); + + it('invalidates cache for mutations that do not use success envelope (#1162)', async () => { + // Prime a markets cache entry. + (global.fetch as jest.Mock).mockResolvedValueOnce({ + ok: true, + text: async () => JSON.stringify([{ id: 1, title: 'Market' }]), + }); + await api.getFeaturedMarkets(); + expect(global.fetch).toHaveBeenCalledTimes(1); + + // resolveMarket returns { invalidated_keys: N } (no success field). + (global.fetch as jest.Mock).mockResolvedValueOnce({ + ok: true, + text: async () => JSON.stringify({ invalidated_keys: 2 }), + }); + await api.resolveMarket(1); + expect(global.fetch).toHaveBeenCalledTimes(2); + + // Cache MUST be invalidated — next getFeaturedMarkets hits the network. + (global.fetch as jest.Mock).mockResolvedValueOnce({ + ok: true, + text: async () => JSON.stringify([{ id: 1, title: 'Resolved' }]), + }); + const freshMarkets = await api.getFeaturedMarkets(); + expect(freshMarkets).toEqual([{ id: 1, title: 'Resolved' }]); + expect(global.fetch).toHaveBeenCalledTimes(3); + }); }); describe('DELETE requests', () => { diff --git a/frontend/src/lib/api/client.ts b/frontend/src/lib/api/client.ts index 7c904c5c..adc73211 100644 --- a/frontend/src/lib/api/client.ts +++ b/frontend/src/lib/api/client.ts @@ -307,11 +307,21 @@ async function request( // On mutations, invalidate only the affected resource tags instead of // the entire cache. Fall back to a full clear for untagged mutations. + // + // Guard: some endpoints return { success: false, message: '...' } with a + // 200 status to signal business-logic failure (e.g. newsletterSubscribe). + // Only bust the cache when the mutation actually succeeded — i.e. when the + // response has no `success` field (non-envelope endpoints) or when + // `success` is explicitly `true`. if (method === "POST" || method === "DELETE") { - if (options.cacheTags?.length) { - apiCache.invalidateByTags(options.cacheTags); - } else { - apiCache.invalidateByPattern('.*'); + const bodyObj = (typeof data === 'object' && data !== null) ? data as Record : null; + const succeeded = bodyObj === null || !('success' in bodyObj) || bodyObj['success'] === true; + if (succeeded) { + if (options.cacheTags?.length) { + apiCache.invalidateByTags(options.cacheTags); + } else { + apiCache.invalidateByPattern('.*'); + } } } From 20cb545c4b129ec6b8449d515f051256245a233c Mon Sep 17 00:00:00 2001 From: wendyamoni-creator Date: Mon, 27 Jul 2026 10:06:41 +0000 Subject: [PATCH 2/2] fix: remove style-src 'unsafe-inline' from CSP (only needed for Skeleton's inline styles) Replace Skeleton component's inline style attribute with CSS classes and CSS utility classes in Statistics.css. Remove 'unsafe-inline' from style-src in both next.config.js and proxy.ts middleware CSP definitions. This eliminates the CSS-based data exfiltration risk that 'unsafe-inline' permits, since only one component (Skeleton.tsx) previously required it. Co-authored-by: automated --- frontend/next.config.js | 2 +- frontend/src/__tests__/proxy.test.ts | 68 +++++++++++++++++++ .../src/__tests__/security-headers.test.ts | 24 ++++--- frontend/src/components/Skeleton.css | 2 + frontend/src/components/Skeleton.tsx | 10 --- frontend/src/components/Statistics.css | 19 ++++++ frontend/src/components/Statistics.tsx | 6 +- .../components/__tests__/Skeleton.test.tsx | 14 ++-- frontend/src/proxy.ts | 2 +- 9 files changed, 120 insertions(+), 27 deletions(-) create mode 100644 frontend/src/__tests__/proxy.test.ts diff --git a/frontend/next.config.js b/frontend/next.config.js index 6c53c37b..263946ac 100644 --- a/frontend/next.config.js +++ b/frontend/next.config.js @@ -74,7 +74,7 @@ const nextConfig = { }, { key: 'Content-Security-Policy', - value: "default-src 'self'; script-src 'self' 'strict-dynamic'; style-src 'self' 'unsafe-inline'; img-src 'self' data: https:; font-src 'self' data:; connect-src 'self' https:; frame-ancestors 'none'; base-uri 'self'; form-action 'self'; object-src 'none'; upgrade-insecure-requests", + value: "default-src 'self'; script-src 'self' 'strict-dynamic'; style-src 'self'; img-src 'self' data: https:; font-src 'self' data:; connect-src 'self' https:; frame-ancestors 'none'; base-uri 'self'; form-action 'self'; object-src 'none'; upgrade-insecure-requests", }, ], }, diff --git a/frontend/src/__tests__/proxy.test.ts b/frontend/src/__tests__/proxy.test.ts new file mode 100644 index 00000000..265a3787 --- /dev/null +++ b/frontend/src/__tests__/proxy.test.ts @@ -0,0 +1,68 @@ +jest.mock('next/server', () => ({ + NextResponse: { + next: jest.fn(() => ({ + headers: new Map(), + })), + }, + NextRequest: jest.fn(), +})); + +import { proxy } from '../proxy'; + +describe('Proxy CSP', () => { + const originalCrypto = global.crypto; + + beforeAll(() => { + (global as any).crypto = { + randomUUID: jest.fn(() => 'test-nonce-uuid'), + }; + }); + + afterAll(() => { + (global as any).crypto = originalCrypto; + }); + + function makeRequest(headers: Record = {}) { + return { + headers: new Headers(headers), + }; + } + + it('should not include unsafe-inline in style-src', () => { + const headers = new Map(); + const mockNextResponse = { + headers: { + set: (key: string, value: string) => headers.set(key, value), + get: (key: string) => headers.get(key), + }, + }; + + jest.requireMock('next/server').NextResponse.next.mockReturnValue(mockNextResponse as any); + + const request = makeRequest(); + proxy(request as any); + + const csp = headers.get('Content-Security-Policy') ?? ''; + const styleSrcMatch = csp.match(/style-src\s+([^;]+)/); + expect(styleSrcMatch).toBeDefined(); + expect(styleSrcMatch?.[1]).not.toContain('unsafe-inline'); + }); + + it('should include style-src self in CSP', () => { + const headers = new Map(); + const mockNextResponse = { + headers: { + set: (key: string, value: string) => headers.set(key, value), + get: (key: string) => headers.get(key), + }, + }; + + jest.requireMock('next/server').NextResponse.next.mockReturnValue(mockNextResponse as any); + + const request = makeRequest(); + proxy(request as any); + + const csp = headers.get('Content-Security-Policy') ?? ''; + expect(csp).toContain("style-src 'self'"); + }); +}); \ No newline at end of file diff --git a/frontend/src/__tests__/security-headers.test.ts b/frontend/src/__tests__/security-headers.test.ts index 26bd757a..ac8f55ae 100644 --- a/frontend/src/__tests__/security-headers.test.ts +++ b/frontend/src/__tests__/security-headers.test.ts @@ -36,14 +36,22 @@ describe('Security Headers', () => { expect(header.value).toContain('camera=()'); }); - it('should have Content-Security-Policy header configured', () => { - const header = headers.find((h: any) => h.key === 'Content-Security-Policy'); - expect(header).toBeDefined(); - expect(header.value).toContain("default-src 'self'"); - expect(header.value).toContain("script-src 'self'"); - expect(header.value).toContain("style-src 'self'"); - expect(header.value).toContain("frame-ancestors 'none'"); - }); +it('should have Content-Security-Policy header configured', () => { + const header = headers.find((h: any) => h.key === 'Content-Security-Policy'); + expect(header).toBeDefined(); + expect(header.value).toContain("default-src 'self'"); + expect(header.value).toContain("script-src 'self'"); + expect(header.value).toContain("style-src 'self'"); + expect(header.value).toContain("frame-ancestors 'none'"); + }); + + it('should not have unsafe-inline in style-src', () => { + const header = headers.find((h: any) => h.key === 'Content-Security-Policy'); + expect(header).toBeDefined(); + const styleSrcMatch = header.value.match(/style-src\s+([^;]+)/); + expect(styleSrcMatch).toBeDefined(); + expect(styleSrcMatch[1]).not.toContain('unsafe-inline'); + }); it('should have all required security headers', () => { const requiredHeaders = [ diff --git a/frontend/src/components/Skeleton.css b/frontend/src/components/Skeleton.css index 4223577b..84544799 100644 --- a/frontend/src/components/Skeleton.css +++ b/frontend/src/components/Skeleton.css @@ -1,5 +1,7 @@ /* Skeleton Loading Styles */ .skeleton { + width: 100%; + height: 1rem; background: linear-gradient(90deg, #f3f4f6 25%, #e5e7eb 50%, #f3f4f6 75%); background-size: 200% 100%; animation: shimmer 1.5s infinite; diff --git a/frontend/src/components/Skeleton.tsx b/frontend/src/components/Skeleton.tsx index da4bf08e..6cfcbc44 100644 --- a/frontend/src/components/Skeleton.tsx +++ b/frontend/src/components/Skeleton.tsx @@ -2,29 +2,19 @@ import React from 'react'; import './Skeleton.css'; interface SkeletonProps { - width?: string; - height?: string; className?: string; variant?: 'text' | 'rectangular' | 'circular'; 'aria-label'?: string; } export const Skeleton: React.FC = ({ - width = '100%', - height = '1rem', className = '', variant = 'text', 'aria-label': ariaLabel = 'Loading content' }) => { - const style = { - width, - height, - }; - return (
{

Total Markets

{loading ? ( - + ) : (

{displayValues.totalMarkets} @@ -72,7 +72,7 @@ export const Statistics: React.FC = () => {

Total Volume

{loading ? ( - + ) : (

{displayValues.totalVolume} @@ -82,7 +82,7 @@ export const Statistics: React.FC = () => {

Active Users

{loading ? ( - + ) : (

{displayValues.activeUsers} diff --git a/frontend/src/components/__tests__/Skeleton.test.tsx b/frontend/src/components/__tests__/Skeleton.test.tsx index cc73c2ed..d411811e 100644 --- a/frontend/src/components/__tests__/Skeleton.test.tsx +++ b/frontend/src/components/__tests__/Skeleton.test.tsx @@ -11,10 +11,10 @@ describe('Skeleton', () => { expect(skeleton).toHaveAttribute('aria-label', 'Loading content'); }); - it('renders with custom dimensions', () => { - render(); + it('renders with default dimensions via CSS', () => { + render(); const skeleton = screen.getByRole('status'); - expect(skeleton).toHaveStyle({ width: '200px', height: '50px' }); + expect(skeleton).toHaveClass('skeleton'); }); it('renders with different variants', () => { @@ -37,4 +37,10 @@ describe('Skeleton', () => { render(); expect(screen.getByRole('status')).toHaveClass('custom-skeleton'); }); -}); \ No newline at end of file + + it('applies size utility classes', () => { + render(); + const skeleton = screen.getByRole('status'); + expect(skeleton).toHaveClass('stat-skeleton--markets'); + }); +}); diff --git a/frontend/src/proxy.ts b/frontend/src/proxy.ts index 1f8366f4..d141461f 100644 --- a/frontend/src/proxy.ts +++ b/frontend/src/proxy.ts @@ -18,7 +18,7 @@ export function proxy(request: NextRequest) { const cspHeader = [ "default-src 'self'", `script-src 'self' 'nonce-${nonce}' 'strict-dynamic'`, - "style-src 'self' 'unsafe-inline'", + "style-src 'self'", "img-src 'self' data: https:", "font-src 'self' data:", `connect-src ${connectSrc}`,