From 866da85c26d96821c00917306b04098e1896f1c8 Mon Sep 17 00:00:00 2001 From: "pulse-triage[bot]" <249995291+pulse-triage[bot]@users.noreply.github.com> Date: Sun, 4 Oct 2026 08:31:57 +0100 Subject: [PATCH] Validate History response ownership before caching Reject mismatched resource, range and metric envelopes and malformed samples at the shared client boundary. Preserve explicit Kubernetes compatibility, real access failures, valid empty history and measured zero. Exercise drawer, single-metric and row-cache consumers, and inspect desktop and phone-emulated browser renders. Change-source: pulse-maintainer --- .../v6/internal/subsystems/api-contracts.md | 27 ++ .../internal/subsystems/unified-resources.md | 18 ++ .../browser-tests/history-response.cjs | 276 ++++++++++++++++++ .../1b57341f2dd28b0d.json | 45 +++ .../src/api/__tests__/chartsApi.test.ts | 40 ++- .../__tests__/chartsHistoryResponse.test.ts | 189 ++++++++++++ frontend-modern/src/api/charts.ts | 88 +++++- .../HistoryResponse.contract.test.tsx | 240 +++++++++++++++ 8 files changed, 908 insertions(+), 15 deletions(-) create mode 100644 frontend-modern/browser-tests/history-response.cjs create mode 100644 frontend-modern/browser-verification/1b57341f2dd28b0d.json create mode 100644 frontend-modern/src/api/__tests__/chartsHistoryResponse.test.ts create mode 100644 frontend-modern/src/components/Workloads/__tests__/HistoryResponse.contract.test.tsx diff --git a/docs/release-control/v6/internal/subsystems/api-contracts.md b/docs/release-control/v6/internal/subsystems/api-contracts.md index 84d54e0b95..f28f290b1a 100644 --- a/docs/release-control/v6/internal/subsystems/api-contracts.md +++ b/docs/release-control/v6/internal/subsystems/api-contracts.md @@ -20,6 +20,33 @@ ## Purpose +### Metrics History responses are bound to the issued selection + +The shared History client admits HTTP200 JSON only when the response resource +type, resource ID, range and single/all-metric shape match the issued request. +Validation uses a captured selection, not a subsequently mutated params object. +It preserves only the server's explicit compatibility rules: Kubernetes types +use the `k8s` wire token, legacy pod IDs receive the `k8s:` prefix and IDs are +trimmed. An omitted range accepts the empty or 24h default echo; explicit ranges +and metrics must match exactly. No host, node or disk alias is guessed. + +Metrics containers and point arrays must have the declared shape, finite +readings/aggregate bounds and representable timestamps. Zero and observed empty +arrays/maps are valid; omitted source remains compatible, and declared store, +memory, live and mock_synthetic provenance is preserved. Finite but unusable +window bounds still reach the existing observation-based window fallback. +Rejected bodies produce a fixed diagnostic without their IDs, values or text; +body status claims are not HTTP status. Real transport/access failures remain +unchanged. No endpoint, server permission, collector or wire schema changes. + +`chartsHistoryResponse.test.ts` covers the response matrix, legitimate protocol +variants and mutable input; existing `chartsApi.test.ts` uses actual-shaped +responses to verify request construction. `HistoryResponse.contract.test.tsx` +exercises the real client with production drawer, shared chart-state and row +warming consumers. The connected browser fixture verifies actual History +components/CSS on desktop Chromium and phone WebKit, not native identity, +appliance history, workload safety or released availability. + ### Discovery detail reads preserve final access status `getDiscovery` retains final HTTP401/403 in a typed client error before any diff --git a/docs/release-control/v6/internal/subsystems/unified-resources.md b/docs/release-control/v6/internal/subsystems/unified-resources.md index 01bb486de0..930352d3b4 100644 --- a/docs/release-control/v6/internal/subsystems/unified-resources.md +++ b/docs/release-control/v6/internal/subsystems/unified-resources.md @@ -23,6 +23,24 @@ and sort the complete canonical change table while startup and ingestion wait. ## Purpose +### History consumers do not accept a mismatched or malformed success body + +The common metrics History client validates the issued selection and response +shape before a drawer, shared chart or opportunistic row-history cache can accept +it. An invalid first HTTP200 is unavailable, not an observed empty series or a +current-value substitute. A rejected same-target refresh may retain only that +target's previously validated result with the existing refresh-failure notice; +rejected values never populate remount or row caches. A valid subsequent read, +including measured zero or observed empty history, restores normal presentation. +HTTP401/403 still withdraws cached evidence rather than becoming format failure. + +The mounted `HistoryResponse.contract.test.tsx` uses the actual client admission +boundary, not a mocked ChartsAPI result. `browser-tests/history-response.cjs` +drives the production PBS-service and disk History fixture with wrong-target, +wrong-type/range/metric, malformed, denied, valid-zero and empty bodies, keyboard +retry/inspection and remounts. These synthetic controls do not diagnose #1723, +establish native panels/identity or prove #2439 thaw or workload liveness. + ### Guest discovery evidence and UI state have exact owners The guest drawer's passive discovery lookup changes only when its explicit diff --git a/frontend-modern/browser-tests/history-response.cjs b/frontend-modern/browser-tests/history-response.cjs new file mode 100644 index 0000000000..7c17a290ac --- /dev/null +++ b/frontend-modern/browser-tests/history-response.cjs @@ -0,0 +1,276 @@ +// Actual History consumers, API client and CSS; synthetic same-origin bodies. +// This verifies response admission, not a native appliance or shipped repair. +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); +const { chromium, webkit } = require('playwright'); + +(async () => { + const root = '/workspace/frontend-modern'; + const artifacts = path.join(root, 'node_modules/history-response-proof'); + fs.mkdirSync(artifacts, { recursive: true }); + process.chdir(root); + const { createServer } = await import(path.join(root, 'node_modules/vite/dist/node/index.js')); + const server = await createServer({ + root, + configFile: path.join(root, 'vite.config.ts'), + cacheDir: fs.mkdtempSync(path.join(artifacts, 'vite-')), + server: { host: '127.0.0.1', port: 5269, strictPort: true }, + }); + const playwright = require('playwright/package.json').version; + assert.equal( + playwright, + JSON.parse(fs.readFileSync('/workspace/tests/integration/package-lock.json', 'utf8')).packages[ + 'node_modules/@playwright/test' + ].version, + ); + const results = []; + let browser; + try { + await server.listen(); + for (const [name, engine, phone] of [ + ['chromium-desktop', chromium, false], + ['webkit-phone', webkit, true], + ]) { + browser = await engine.launch( + engine === chromium + ? { headless: true, channel: 'chromium', args: ['--no-sandbox'] } + : { headless: true }, + ); + const page = await browser.newPage({ + viewport: phone ? { width: 390, height: 844 } : { width: 1365, height: 900 }, + hasTouch: phone, + isMobile: phone, + }); + await page.clock.install({ time: new Date('2026-10-04T07:30:00Z') }); + let mode = 'wrong-target'; + let value = 42; + const errors = [], + requests = [], + checks = [], + screenshots = [], + offOrigin = []; + page.on('pageerror', (error) => errors.push(error.message)); + await page.route('**/*', async (route) => { + const url = new URL(route.request().url()); + if (url.origin !== 'http://127.0.0.1:5269') { + offOrigin.push(url.origin); + return route.abort(); + } + if (!url.pathname.startsWith('/api/')) return route.continue(); + if (url.pathname === '/api/metrics-store/history') { + const query = Object.fromEntries(url.searchParams); + requests.push({ ...query, mode }); + if (mode === 'denied') + return route.fulfill({ status: 403, json: { error: 'fixture-private-body' } }); + const points = [0, 1].map((i) => ({ + timestamp: 1791098940000 + i * 60_000, + value: value - 1 + i, + min: value - 1 + i, + max: value - 1 + i, + })); + const response = { + resourceType: mode === 'wrong-type' ? 'node' : query.resourceType, + resourceId: mode === 'wrong-target' ? 'fixture-private-predecessor' : query.resourceId, + range: mode === 'wrong-range' ? '7d' : query.range, + start: 1791098940000, + end: 1791099000000, + source: 'store', + }; + return route.fulfill({ + json: { + ...response, + ...(query.metric + ? { + metric: mode === 'wrong-metric' ? 'cpu' : query.metric, + points: mode === 'malformed' ? null : mode === 'empty' ? [] : points, + } + : { + metrics: + mode === 'malformed' + ? [] + : mode === 'empty' + ? {} + : { cpu: points, temperature: points }, + }), + }, + }); + } + return route.fulfill({ + json: + url.pathname === '/api/license/runtime-capabilities' + ? { + capabilities: [], + limits: [], + max_history_days: 7, + hosted_mode: false, + runtime: { build: 'community' }, + blocked_capabilities: [], + } + : { enabled: false, data: [] }, + }); + }); + await page.goto('http://127.0.0.1:5269/browser-tests/history-access.html'); + if (phone) await page.evaluate(() => document.documentElement.classList.add('dark')); + const service = page.getByTestId('service'); + const disk = page.getByTestId('disk'); + const plots = () => service.locator('[data-testid="guest-history-plot"] path'); + const chartDescription = () => + disk + .locator('canvas') + .getAttribute('aria-describedby') + .then((id) => disk.locator(`#${id}`).innerText()); + const check = async (label, action) => { + await action(); + checks.push(label); + }; + const capture = async (state) => { + const dimensions = await page.evaluate(() => ({ + width: innerWidth, + scroll: document.documentElement.scrollWidth, + })); + assert.ok(dimensions.scroll <= dimensions.width + 1, JSON.stringify(dimensions)); + const file = `${name}-${state}.png`; + await page.screenshot({ + path: path.join(artifacts, file), + fullPage: true, + animations: 'disabled', + }); + screenshots.push({ + state, + file, + dimensions, + serviceText: await service.innerText(), + diskDescription: await chartDescription(), + }); + }; + await service.getByText('Failed to load history data', { exact: true }).waitFor(); + await disk.getByText('Failed to load history data', { exact: true }).waitFor(); + await check( + 'wrong-target HTTP200 is unavailable, not empty or previous-host samples', + async () => { + assert.equal(await plots().count(), 0); + assert.ok(!(await service.innerText()).includes('No stored history')); + assert.ok(!(await page.locator('body').innerText()).includes('fixture-private')); + assert.ok(!(await chartDescription()).includes('42')); + }, + ); + await capture('wrong-target'); + mode = 'correct'; + await service.getByRole('button', { name: 'Retry history' }).focus(); + await page.keyboard.press('Enter'); + await service.getByText('42.0%', { exact: true }).waitFor(); + await page.clock.runFor(30_000); + await check( + 'keyboard retry and valid single-metric polling restore bound evidence', + async () => { + assert.ok((await chartDescription()).includes('42°C')); + assert.equal(await plots().count(), 2); + }, + ); + mode = 'wrong-range'; + value = 97; + await service.getByRole('button', { name: 'Refresh history' }).click(); + await service + .getByText('History refresh failed. Showing previously loaded history.', { exact: true }) + .waitFor(); + await page.clock.runFor(30_000); + await check('wrong-range refresh keeps only labelled validated evidence', async () => { + assert.ok((await service.innerText()).includes('42.0%')); + assert.ok(!(await service.innerText()).includes('97.0%')); + assert.ok((await disk.innerText()).includes('last successful result')); + assert.ok(!(await chartDescription()).includes('97°C')); + }); + await capture('retained-valid'); + await service.getByRole('button', { name: 'Hide service History' }).click(); + await service.getByRole('button', { name: 'Show service History' }).click(); + await service + .getByText('History refresh failed. Showing previously loaded history.', { exact: true }) + .waitFor(); + await check('remount cache has no rejected range response', async () => { + assert.ok((await service.innerText()).includes('42.0%')); + assert.ok(!(await service.innerText()).includes('97.0%')); + }); + for (const bad of ['wrong-type', 'malformed', 'wrong-metric']) { + mode = bad; + await service.getByRole('button', { name: 'Retry history' }).click(); + // wrong-metric affects the single-metric reader only. + if (bad === 'wrong-metric') await service.getByText('97.0%', { exact: true }).waitFor(); + else + await service + .getByText('History refresh failed. Showing previously loaded history.', { + exact: true, + }) + .waitFor(); + await page.clock.runFor(30_000); + await check( + `${bad} cannot replace the single-metric chart's validated samples`, + async () => { + assert.ok(!(await chartDescription()).includes('97°C')); + if (bad !== 'wrong-metric') assert.ok(!(await service.innerText()).includes('97.0%')); + }, + ); + } + mode = 'denied'; + await service.getByRole('button', { name: 'Refresh history' }).click(); + await service + .getByText('Access denied. Check your permissions and license plan.', { exact: true }) + .waitFor(); + await page.clock.runFor(30_000); + await check('HTTP403 still withdraws validated history and inspection', async () => { + assert.equal(await plots().count(), 0); + assert.equal(await service.getByRole('slider').count(), 0); + assert.ok(!(await service.innerText()).includes('97.0%')); + assert.ok(!(await chartDescription()).includes('42°C')); + }); + await capture('denied'); + mode = 'correct'; + value = 0; + const retry = service.getByRole('button', { name: 'Retry history' }); + await retry.focus(); + await page.keyboard.press('Enter'); + await service.getByText('0.0%', { exact: true }).waitFor(); + await page.clock.runFor(30_000); + const canvas = disk.getByRole('img', { name: 'Temperature chart' }); + await canvas.focus(); + await page.keyboard.press('End'); + await check('measured zero is accepted, including keyboard canvas inspection', async () => { + assert.ok((await chartDescription()).includes('0°C')); + assert.equal(await plots().count(), 2); + }); + await capture('zero-recovered'); + mode = 'empty'; + await service.getByRole('button', { name: 'Refresh history' }).click(); + await service.getByText('No stored history in this range', { exact: true }).first().waitFor(); + await page.clock.runFor(30_000); + await check('valid empty success is distinct from the initial invalid body', async () => { + assert.equal(await plots().count(), 0); + assert.ok(!(await service.innerText()).includes('Failed to load')); + assert.ok((await chartDescription()).includes('No 1-hour history data is available.')); + }); + assert.deepEqual(errors, []); + assert.deepEqual(offOrigin, []); + results.push({ + browser: name, + version: browser.version(), + checks, + requests, + screenshots, + errors, + offOrigin, + }); + await browser.close(); + browser = null; + } + fs.writeFileSync( + path.join(artifacts, 'result.json'), + JSON.stringify({ result: 'passed', playwright, results }, null, 2), + ); + } finally { + if (browser) await browser.close(); + await server.close(); + } +})().catch((error) => { + console.error(error); + process.exitCode = 1; +}); diff --git a/frontend-modern/browser-verification/1b57341f2dd28b0d.json b/frontend-modern/browser-verification/1b57341f2dd28b0d.json new file mode 100644 index 0000000000..212e7d0505 --- /dev/null +++ b/frontend-modern/browser-verification/1b57341f2dd28b0d.json @@ -0,0 +1,45 @@ +{ + "version": 1, + "result": "passed", + "base_sha": "9d5fd627f01cb176e5f28b1923a3407fc0867c3a", + "content_sha256": { + "frontend-modern/src/api/charts.ts": "4a05bd62662ed2e812e02eaaa4f3da7470276e9cd4cfd260ab00ae9a69e6387d" + }, + "changed_paths": [ + "frontend-modern/src/api/charts.ts" + ], + "routes": [ + "Existing PBS service History and disk History, through browser-tests/history-access.html production-component fixture" + ], + "states": [ + "Initial HTTP200 with wrong target unavailable, not empty", + "Valid response and measured zero recovery", + "Wrong range/type/metric or malformed response cannot replace validated evidence", + "Same-target remount retention with warning", + "Real HTTP403 withdrawal", + "Valid observed empty history" + ], + "interactions": [ + "Keyboard retry using Enter", + "History hide/remount", + "Polling refresh", + "Keyboard End canvas inspection" + ], + "viewports": [ + { + "width": 1365, + "height": 900 + }, + { + "width": 390, + "height": 844 + } + ], + "verified_at": "2026-10-04T07:31:31Z", + "browsers": [ + "Chromium desktop", + "WebKit phone emulation; not native iOS" + ], + "command": "pulse-worker-browser frontend-modern/browser-tests/history-response.cjs", + "proof": "Production History components, real ChartsAPI and API client, synthetic same-origin HTTP; no native appliance or shipped acceptance." +} diff --git a/frontend-modern/src/api/__tests__/chartsApi.test.ts b/frontend-modern/src/api/__tests__/chartsApi.test.ts index 2d6dd66691..16df3541c0 100644 --- a/frontend-modern/src/api/__tests__/chartsApi.test.ts +++ b/frontend-modern/src/api/__tests__/chartsApi.test.ts @@ -78,7 +78,15 @@ describe('ChartsAPI', () => { }); it('builds metrics history query params including maxPoints', async () => { - apiFetchJSONMock.mockResolvedValueOnce({} as any); + apiFetchJSONMock.mockResolvedValueOnce({ + resourceType: 'agent', + resourceId: 'agent-1', + metric: 'cpu', + range: '14d', + start: 1, + end: 2, + points: [], + }); await ChartsAPI.getMetricsHistory({ resourceType: 'agent', @@ -94,7 +102,15 @@ describe('ChartsAPI', () => { }); it('builds canonical physical-disk history queries for 30m live I/O charts', async () => { - apiFetchJSONMock.mockResolvedValueOnce({} as any); + apiFetchJSONMock.mockResolvedValueOnce({ + resourceType: 'disk', + resourceId: 'SERIAL884006359727', + metric: 'diskread', + range: '30m', + start: 1, + end: 2, + points: [], + }); await ChartsAPI.getMetricsHistory({ resourceType: 'disk', @@ -109,7 +125,15 @@ describe('ChartsAPI', () => { }); it('passes through agent metrics history requests', async () => { - apiFetchJSONMock.mockResolvedValueOnce({} as any); + apiFetchJSONMock.mockResolvedValueOnce({ + resourceType: 'agent', + resourceId: 'agent-1', + metric: 'cpu', + range: '24h', + start: 1, + end: 2, + points: [], + }); await ChartsAPI.getMetricsHistory({ resourceType: 'agent', @@ -124,7 +148,15 @@ describe('ChartsAPI', () => { }); it('maps canonical kubernetes resource types to the backend k8s history token', async () => { - apiFetchJSONMock.mockResolvedValueOnce({} as any); + apiFetchJSONMock.mockResolvedValueOnce({ + resourceType: 'k8s', + resourceId: 'k8s:cluster-a:pod:api-1', + metric: 'cpu', + range: '24h', + start: 1, + end: 2, + points: [], + }); await ChartsAPI.getMetricsHistory({ resourceType: 'pod', diff --git a/frontend-modern/src/api/__tests__/chartsHistoryResponse.test.ts b/frontend-modern/src/api/__tests__/chartsHistoryResponse.test.ts new file mode 100644 index 0000000000..639fcb9ae6 --- /dev/null +++ b/frontend-modern/src/api/__tests__/chartsHistoryResponse.test.ts @@ -0,0 +1,189 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { ChartsAPI, type MetricsHistoryParams, type ResourceType } from '@/api/charts'; +import { apiFetchJSON } from '@/utils/apiClient'; + +vi.mock('@/utils/apiClient', () => ({ apiFetchJSON: vi.fn() })); + +const request: MetricsHistoryParams = { + resourceType: 'agent', + resourceId: 'pbs-current', + range: '24h', +}; +const point = { timestamp: 1_700_000_000_000, value: 0, min: 0, max: 0 }; +const envelope = () => ({ + ...request, + start: point.timestamp, + end: point.timestamp + 60_000, + source: 'store', +}); +const all = () => ({ ...envelope(), metrics: { cpu: [point] } }); +const single = () => ({ ...envelope(), metric: 'cpu', points: [point] }); +const read = async (body: unknown, params = request) => { + vi.mocked(apiFetchJSON).mockResolvedValueOnce(body); + return ChartsAPI.getMetricsHistory(params); +}; + +beforeEach(() => vi.mocked(apiFetchJSON).mockReset()); + +describe('metrics History response admission', () => { + it.each([ + ['another resource', { resourceId: 'private-predecessor' }], + ['another type', { resourceType: 'node' }], + ['another range', { range: '1h' }], + ['absent resource', { resourceId: undefined }], + ['absent type', { resourceType: undefined }], + ['absent range', { range: undefined }], + ['non-string resource', { resourceId: 1 }], + ['unknown source', { source: 'private-source' }], + ['null source', { source: null }], + ['absent start', { start: undefined }], + ['string end', { end: '1700000060000' }], + ['nonfinite end', { end: Infinity }], + ['null metrics', { metrics: null }], + ['array metrics', { metrics: [] }], + ['missing metrics', { metrics: undefined }], + ['null series', { metrics: { cpu: null } }], + ['object series', { metrics: { cpu: point } }], + ['mixed shapes', { points: [], metric: 'cpu' }], + ])('rejects %s rather than resolving empty or caching it', async (_label, change) => { + await expect(read({ ...all(), ...change })).rejects.toThrow( + 'Invalid metrics history response.', + ); + }); + + it.each([null, [], 'private body', 42, {}])('rejects a non-envelope body %#', async (body) => { + await expect(read(body)).rejects.toThrow('Invalid metrics history response.'); + }); + + it.each([ + null, + 1, + {}, + { ...point, timestamp: '1700000000000' }, + { ...point, timestamp: 9e15 }, + { ...point, value: NaN }, + { ...point, value: null }, + { ...point, min: undefined }, + { ...point, max: Infinity }, + ])('rejects malformed samples in either response shape %#', async (invalid) => { + await expect(read({ ...all(), metrics: { cpu: [point, invalid] } })).rejects.toThrow( + 'Invalid metrics history response.', + ); + await expect( + read({ ...single(), points: [point, invalid] }, { ...request, metric: 'cpu' }), + ).rejects.toThrow('Invalid metrics history response.'); + }); + + it.each([ + { metric: 'memory' }, + { metric: undefined }, + { points: null }, + { points: undefined }, + { points: {} }, + { metrics: {} }, + ])('requires the requested single-metric shape %#', async (change) => { + await expect(read({ ...single(), ...change }, { ...request, metric: 'cpu' })).rejects.toThrow( + 'Invalid metrics history response.', + ); + }); + + it.each(['store', 'memory', 'live', 'mock_synthetic', undefined])( + 'preserves valid samples and source %s without manufacturing points', + async (source) => { + const body = { ...all(), source }; + expect(await read(body)).toBe(body); + expect(await read({ ...single(), source }, { ...request, metric: 'cpu' })).toEqual({ + ...single(), + source, + }); + }, + ); + + it('accepts observed empty responses, zero readings and the existing invalid-window fallback', async () => { + for (const metrics of [{}, { cpu: [] }, { cpu: [point] }]) { + const body = { ...all(), start: 0, end: 0, metrics }; + expect(await read(body)).toBe(body); + } + expect(await read({ ...single(), points: [] }, { ...request, metric: 'cpu' })).toMatchObject({ + points: [], + }); + }); + + it.each(['', '24h'])( + 'accepts the default range echo %s only when range was omitted', + async (range) => { + expect( + await read({ ...all(), range }, { resourceType: 'agent', resourceId: 'pbs-current' }), + ).toMatchObject({ range }); + await expect( + read({ ...all(), range: '1h' }, { resourceType: 'agent', resourceId: 'pbs-current' }), + ).rejects.toThrow(); + }, + ); + + it.each([ + 'node', + 'agent', + 'vm', + 'system-container', + 'oci-container', + 'app-container', + 'storage', + 'docker-host', + 'disk', + ] as ResourceType[])( + 'keeps the canonical %s type and server ID trimming', + async (resourceType) => { + const body = { ...all(), resourceType }; + expect(await read(body, { ...request, resourceType, resourceId: ' pbs-current ' })).toBe( + body, + ); + }, + ); + + it.each(['k8s-cluster', 'k8s-node', 'k8s-deployment', 'pod'] as ResourceType[])( + 'accepts the wire k8s token for %s, not an unrelated ID', + async (resourceType) => { + const body = { ...all(), resourceType: 'k8s', resourceId: 'k8s:cluster-a:pod:uid' }; + for (const resourceId of ['cluster-a:pod:uid', 'k8s:cluster-a:pod:uid']) { + expect(await read(body, { ...request, resourceType, resourceId })).toBe(body); + } + await expect( + read(body, { ...request, resourceType, resourceId: 'cluster-b:pod:uid' }), + ).rejects.toThrow(); + }, + ); + + it('does not treat body status claims as HTTP denial or expose wrong-target details', async () => { + await expect( + read({ ...all(), resourceId: 'private-resource', status: 403, error: 'private-message' }), + ).rejects.toMatchObject({ + message: 'Invalid metrics history response.', + }); + }); + + it.each([401, 403, 503])( + 'preserves real transport error %s without reclassifying it', + async (status) => { + const error = Object.assign(new Error('transport'), { status }); + vi.mocked(apiFetchJSON).mockRejectedValueOnce(error); + await expect(ChartsAPI.getMetricsHistory(request)).rejects.toBe(error); + }, + ); + + it('binds validation to the issued selection even if a caller mutates its params', async () => { + let complete!: (value: unknown) => void; + vi.mocked(apiFetchJSON).mockImplementationOnce( + () => + new Promise((resolve) => { + complete = resolve; + }), + ); + const params = { ...request }; + const promise = ChartsAPI.getMetricsHistory(params); + params.resourceId = 'successor'; + params.range = '1h'; + complete(all()); + expect(await promise).toMatchObject(request); + }); +}); diff --git a/frontend-modern/src/api/charts.ts b/frontend-modern/src/api/charts.ts index ddcf8df72f..931d9712b7 100644 --- a/frontend-modern/src/api/charts.ts +++ b/frontend-modern/src/api/charts.ts @@ -284,6 +284,68 @@ export interface AllMetricsHistoryResponse { source?: 'store' | 'memory' | 'live' | 'mock_synthetic'; } +const isHistoryObject = (value: unknown): value is Record => + typeof value === 'object' && value !== null && !Array.isArray(value); + +const isFiniteHistoryNumber = (value: unknown): value is number => + typeof value === 'number' && Number.isFinite(value); + +const isHistoryPointList = (value: unknown): value is AggregatedMetricPoint[] => + Array.isArray(value) && + value.every( + (point: unknown) => + isHistoryObject(point) && + isFiniteHistoryNumber(point.timestamp) && + Number.isFinite(new Date(point.timestamp).getTime()) && + isFiniteHistoryNumber(point.value) && + isFiniteHistoryNumber(point.min) && + isFiniteHistoryNumber(point.max), + ); + +// A successful transport is not evidence for the requested selection. Validate +// before any drawer, chart or hover cache can accept the response. Keep errors +// fixed: a mismatched body can contain another resource's private details. +function parseMetricsHistoryResponse( + value: unknown, + params: MetricsHistoryParams, +): SingleMetricHistoryResponse | AllMetricsHistoryResponse { + const resourceType = toMetricsHistoryAPIResourceType(params.resourceType); + const requestedId = params.resourceId.trim(); + // This is the server's one explicit ID compatibility rule, not an alias + // search: legacy Kubernetes pod IDs receive the canonical k8s: prefix. + const resourceId = + resourceType === 'k8s' && requestedId.includes(':pod:') && !requestedId.startsWith('k8s:') + ? `k8s:${requestedId}` + : requestedId; + if ( + !isHistoryObject(value) || + value.resourceType !== resourceType || + value.resourceId !== resourceId || + (params.range ? value.range !== params.range : value.range !== '' && value.range !== '24h') || + !isFiniteHistoryNumber(value.start) || + !isFiniteHistoryNumber(value.end) || + (value.source !== undefined && + !['store', 'memory', 'live', 'mock_synthetic'].includes(value.source as string)) + ) { + throw new Error('Invalid metrics history response.'); + } + if (params.metric) { + if (value.metric !== params.metric || 'metrics' in value || !isHistoryPointList(value.points)) { + throw new Error('Invalid metrics history response.'); + } + return value as unknown as SingleMetricHistoryResponse; + } + if ( + 'metric' in value || + 'points' in value || + !isHistoryObject(value.metrics) || + !Object.values(value.metrics).every(isHistoryPointList) + ) { + throw new Error('Invalid metrics history response.'); + } + return value as unknown as AllMetricsHistoryResponse; +} + export type TimeRange = '5m' | '15m' | '30m' | '1h' | '4h' | '12h' | '24h' | '7d' | '30d'; export class ChartsAPI { @@ -396,25 +458,29 @@ export class ChartsAPI { static async getMetricsHistory( params: MetricsHistoryParams, ): Promise { + const selection = { ...params }; const searchParams = new URLSearchParams({ - resourceType: toMetricsHistoryAPIResourceType(params.resourceType), - resourceId: params.resourceId, + resourceType: toMetricsHistoryAPIResourceType(selection.resourceType), + resourceId: selection.resourceId, }); - if (params.metric) { - searchParams.set('metric', params.metric); + if (selection.metric) { + searchParams.set('metric', selection.metric); } - if (params.range) { - searchParams.set('range', params.range); + if (selection.range) { + searchParams.set('range', selection.range); } if ( - typeof params.maxPoints === 'number' && - Number.isFinite(params.maxPoints) && - params.maxPoints > 0 + typeof selection.maxPoints === 'number' && + Number.isFinite(selection.maxPoints) && + selection.maxPoints > 0 ) { - searchParams.set('maxPoints', Math.round(params.maxPoints).toString()); + searchParams.set('maxPoints', Math.round(selection.maxPoints).toString()); } const url = `${this.baseUrl}/metrics-store/history?${searchParams.toString()}`; - return params.signal ? apiFetchJSON(url, { signal: params.signal }) : apiFetchJSON(url); + const response: unknown = selection.signal + ? await apiFetchJSON(url, { signal: selection.signal }) + : await apiFetchJSON(url); + return parseMetricsHistoryResponse(response, selection); } /** diff --git a/frontend-modern/src/components/Workloads/__tests__/HistoryResponse.contract.test.tsx b/frontend-modern/src/components/Workloads/__tests__/HistoryResponse.contract.test.tsx new file mode 100644 index 0000000000..f2a43447a8 --- /dev/null +++ b/frontend-modern/src/components/Workloads/__tests__/HistoryResponse.contract.test.tsx @@ -0,0 +1,240 @@ +import { cleanup, fireEvent, render, screen } from '@solidjs/testing-library'; +import { createSignal } from 'solid-js'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { apiFetchJSON } from '@/utils/apiClient'; +import { resetCreateNonSuspendingQueryCacheForTest } from '@/hooks/createNonSuspendingQuery'; +import { useHistoryChartState } from '@/components/shared/useHistoryChartState'; +import type { WorkloadGuest } from '@/types/workloads'; +import { GuestDrawerHistory } from '../GuestDrawerHistory'; +import { useWorkloadTableMetricHistory } from '../useWorkloadTableMetricHistory'; + +// Mock the transport, not ChartsAPI: every production consumer uses the real +// response-admission boundary and its existing retention/error policy. +vi.mock('@/utils/apiClient', () => ({ apiFetchJSON: vi.fn() })); +vi.mock('@/stores/license', () => ({ + loadRuntimeCapabilities: vi.fn(), + maxHistoryDays: () => 7, + isRangeLocked: () => false, +})); + +const target = { resourceType: 'agent' as const, resourceId: 'pbs-current' }; +const body = (value = 42) => ({ + ...target, + range: '24h', + start: 1_700_000_000_000, + end: 1_700_000_060_000, + source: 'store', + metrics: { + cpu: [0, 1].map((i) => ({ + timestamp: 1_700_000_000_000 + i * 60_000, + value, + min: value, + max: value, + })), + }, +}); +const transport = vi.mocked(apiFetchJSON); +const settle = () => vi.advanceTimersByTimeAsync(0); +const plots = (container: HTMLElement) => + container.querySelectorAll('[data-testid="guest-history-plot"] path'); + +beforeEach(() => { + vi.useFakeTimers(); + transport.mockReset(); + vi.spyOn(console, 'error').mockImplementation(() => {}); +}); +afterEach(() => { + cleanup(); + resetCreateNonSuspendingQueryCacheForTest(); + vi.restoreAllMocks(); + vi.useRealTimers(); +}); + +describe('production History consumers reject unbound response evidence', () => { + it.each([ + ['wrong resource', { resourceId: 'private-predecessor' }], + ['wrong range', { range: '1h' }], + ['wrong type', { resourceType: 'node' }], + ['malformed metrics', { metrics: [] }], + ])( + 'keeps initial %s unavailable, offers retry and admits real zero/empty success', + async (_label, change) => { + transport.mockResolvedValueOnce({ ...body(97), ...change, error: 'private diagnostics' }); + const view = render(() => ( + + )); + await settle(); + expect(screen.getByRole('status', { name: 'History refresh status' })).toHaveTextContent( + 'Failed to load history data', + ); + expect(plots(view.container)).toHaveLength(0); + expect(view.container).not.toHaveTextContent('97.0%'); + expect(view.container).not.toHaveTextContent('99.0%'); + expect(view.container).not.toHaveTextContent('No stored history'); + expect(view.container).not.toHaveTextContent('private'); + transport.mockResolvedValueOnce(body(0)); + fireEvent.click(screen.getByRole('button', { name: 'Retry history' })); + await settle(); + expect(plots(view.container)).toHaveLength(1); + expect(view.container).toHaveTextContent('0.0%'); + transport.mockResolvedValueOnce({ ...body(), metrics: {} }); + fireEvent.click(screen.getByRole('button', { name: 'Refresh history' })); + await settle(); + expect(plots(view.container)).toHaveLength(0); + expect(view.container).toHaveTextContent('No stored history in this range'); + expect(screen.getByRole('status', { name: 'History refresh status' })).toBeEmptyDOMElement(); + }, + ); + + it('retains only its validated same-target result on an invalid refresh, including remount, then recovers', async () => { + transport.mockResolvedValueOnce(body()); + const view = render(() => ); + await settle(); + transport.mockResolvedValueOnce({ ...body(97), resourceId: 'wrong-pbs' }); + fireEvent.click(screen.getByRole('button', { name: 'Refresh history' })); + await settle(); + expect(plots(view.container)).toHaveLength(1); + expect(view.container).toHaveTextContent('42.0%'); + expect(view.container).not.toHaveTextContent('97.0%'); + expect(view.container).toHaveTextContent( + 'History refresh failed. Showing previously loaded history.', + ); + view.unmount(); + transport.mockImplementationOnce(() => new Promise(() => {})); + const remount = render(() => ); + expect(remount.container).toHaveTextContent('42.0%'); + expect(remount.container).not.toHaveTextContent('97.0%'); + await settle(); + transport.mockResolvedValueOnce(body(12)); + await vi.advanceTimersByTimeAsync(30_000); + expect(remount.container).toHaveTextContent('12.0%'); + expect(remount.container).not.toHaveTextContent('42.0%'); + expect(screen.getByRole('status', { name: 'History refresh status' })).toBeEmptyDOMElement(); + }); + + it('does not cache an invalid first response as empty history on remount', async () => { + transport.mockResolvedValueOnce({ ...body(97), metrics: null }); + const first = render(() => ); + await settle(); + first.unmount(); + transport.mockImplementationOnce(() => new Promise(() => {})); + const second = render(() => ); + expect(second.container).not.toHaveTextContent('No stored history'); + expect(second.container).toHaveTextContent('Failed to load history data'); + expect(plots(second.container)).toHaveLength(0); + }); + + it('keeps true denial separate from invalid HTTP200 and withdraws previously validated evidence', async () => { + transport.mockResolvedValueOnce(body()); + const view = render(() => ); + await settle(); + transport.mockResolvedValueOnce({ ...body(97), resourceId: 'wrong', status: 403 }); + fireEvent.click(screen.getByRole('button', { name: 'Refresh history' })); + await settle(); + expect(view.container).toHaveTextContent('previously loaded history'); + transport.mockRejectedValueOnce(Object.assign(new Error('private denial'), { status: 403 })); + fireEvent.click(screen.getByRole('button', { name: 'Retry history' })); + await settle(); + expect(plots(view.container)).toHaveLength(0); + expect(view.container).not.toHaveTextContent('42.0%'); + expect(view.container).toHaveTextContent( + 'Access denied. Check your permissions and license plan.', + ); + }); + + it('protects the shared single-metric chart across initial, refresh and selection failures', async () => { + const valid = (value = 42) => { + const { metrics, ...response } = body(value); + return { ...response, metric: 'cpu', points: metrics.cpu }; + }; + const [resourceId, setResourceId] = createSignal(target.resourceId); + function Probe() { + const chart = useHistoryChartState( + { + get resourceId() { + return resourceId(); + }, + resourceType: 'agent', + metric: 'cpu', + range: '24h', + }, + { getCanvas: () => undefined, getContainer: () => undefined }, + ); + return ( + + {JSON.stringify({ + values: chart.data().map((p) => p.value), + error: chart.error(), + refresh: chart.refreshFailed(), + })} + + ); + } + transport.mockResolvedValueOnce({ ...valid(97), metric: 'memory' }); + const view = render(() => ); + await settle(); + expect(view.container).toHaveTextContent('Failed to load history data'); + expect(view.container).not.toHaveTextContent('97'); + transport.mockResolvedValueOnce(valid()); + await vi.advanceTimersByTimeAsync(30_000); + expect(view.container).toHaveTextContent('"values":[42,42]'); + transport.mockResolvedValueOnce({ ...valid(97), points: null }); + await vi.advanceTimersByTimeAsync(30_000); + expect(view.container).toHaveTextContent('"values":[42,42]'); + expect(view.container).toHaveTextContent('"refresh":true'); + transport.mockResolvedValueOnce(valid(97)); + setResourceId('pbs-replacement'); + await settle(); + expect(view.container).toHaveTextContent('"values":[]'); + expect(view.container).toHaveTextContent('Failed to load history data'); + }); + + it('keeps wrong-target or malformed hover warming out of the row cache and permits a later valid request', async () => { + const guest = { + id: 'vm-current', + name: 'vm-current', + type: 'qemu', + workloadType: 'vm', + metricsTarget: { resourceType: 'vm', resourceId: 'metrics-current' }, + } as WorkloadGuest; + const [active, setActive] = createSignal(guest); + function Probe() { + const reader = useWorkloadTableMetricHistory({ + activeGuest: active, + enabled: () => false, + onDemand: () => true, + range: () => '24h', + selectedNode: () => null, + }); + return ( + + {JSON.stringify(active() ? reader.getGuestMetricSeries(active()!, 'cpu') : [])} + + ); + } + transport.mockResolvedValueOnce({ ...body(97), resourceType: 'vm', resourceId: 'wrong-vm' }); + const view = render(() => ); + await settle(); + expect(view.container).not.toHaveTextContent('97'); + transport.mockResolvedValueOnce({ + ...body(97), + resourceType: 'vm', + resourceId: 'metrics-current', + metrics: [], + }); + setActive(null); + setActive(guest); + await settle(); + expect(view.container).not.toHaveTextContent('97'); + transport.mockResolvedValueOnce({ + ...body(0), + resourceType: 'vm', + resourceId: 'metrics-current', + }); + setActive(null); + setActive(guest); + await settle(); + expect(view.container).toHaveTextContent('"value":0'); + expect(transport).toHaveBeenCalledTimes(3); + }); +});