From 0d5ff26a8ffa68d54cd0a4d1c9514dd527f3cee1 Mon Sep 17 00:00:00 2001 From: Anso Date: Sat, 8 Aug 2026 17:47:19 -0400 Subject: [PATCH] fix(fleet): reject negative or non-finite remote reclaimableBytes in prune estimate (#1803) A remote node returning HTTP 200 with a negative reclaimableBytes value passed the typeof-only validation in the fleet prune estimate fan-out and was folded into the per-node sum and the fleet total, silently shrinking the estimate. Extend the invalid-response check to reject non-finite and negative values, matching the existing guard for destructive prune plans. Adds route-level tests covering negative, mixed partial, non-finite (1e999 to Infinity), and zero-valued remote estimates. --- .../fleet-action-card-endpoints.test.ts | 129 ++++++++++++++++++ backend/src/routes/fleet.ts | 9 +- 2 files changed, 137 insertions(+), 1 deletion(-) diff --git a/backend/src/__tests__/fleet-action-card-endpoints.test.ts b/backend/src/__tests__/fleet-action-card-endpoints.test.ts index dd220ed2..0f7ec1a0 100644 --- a/backend/src/__tests__/fleet-action-card-endpoints.test.ts +++ b/backend/src/__tests__/fleet-action-card-endpoints.test.ts @@ -598,6 +598,135 @@ describe('POST /api/fleet/prune/estimate', () => { } }); + it('rejects a negative remote reclaimableBytes without folding it into the totals', async () => { + estimateManagedReclaim.mockResolvedValue({ reclaimableBytes: 0 }); + const remoteId = db.addNode({ + name: 'remote-negative', + type: 'remote', + api_url: 'http://remote-negative.example:1852', + api_token: 'tok', + compose_dir: '/app/compose', + is_default: false, + }); + try { + vi.spyOn(globalThis, 'fetch').mockResolvedValue(new Response( + JSON.stringify({ reclaimableBytes: -5 }), + { status: 200, headers: { 'Content-Type': 'application/json' } }, + )); + const res = await request(app) + .post('/api/fleet/prune/estimate') + .set('Authorization', authHeader) + .send({ targets: ['images'], scope: 'managed' }); + expect(res.status).toBe(200); + const remote = res.body.perNode.find((n: { nodeId: number }) => n.nodeId === remoteId); + expect(remote.reachable).toBe(false); + expect(remote.partial).toBeUndefined(); + expect(remote.reclaimableBytes).toBe(0); + expect(remote.error).toBe('Invalid response from remote node'); + expect(res.body.totalBytes).toBe(0); + } finally { + db.deleteNode(remoteId); + } + }); + + it('rejects a negative remote target without discarding a successful sibling target', async () => { + estimateManagedReclaim.mockResolvedValue({ reclaimableBytes: 0 }); + const remoteId = db.addNode({ + name: 'remote-negative-partial', + type: 'remote', + api_url: 'http://remote-negative-partial.example:1852', + api_token: 'tok', + compose_dir: '/app/compose', + is_default: false, + }); + try { + vi.spyOn(globalThis, 'fetch').mockImplementation(async (_url, init) => { + const body = JSON.parse(String(init?.body ?? '{}')) as { target?: string }; + return new Response( + JSON.stringify({ reclaimableBytes: body.target === 'images' ? 42 : -5 }), + { status: 200, headers: { 'Content-Type': 'application/json' } }, + ); + }); + const res = await request(app) + .post('/api/fleet/prune/estimate') + .set('Authorization', authHeader) + .send({ targets: ['images', 'volumes'], scope: 'managed' }); + expect(res.status).toBe(200); + const remote = res.body.perNode.find((n: { nodeId: number }) => n.nodeId === remoteId); + expect(remote.reachable).toBe(true); + expect(remote.partial).toBe(true); + expect(remote.reclaimableBytes).toBe(42); + expect(remote.error).toBe('Invalid response from remote node'); + expect(res.body.totalBytes).toBe(42); + } finally { + db.deleteNode(remoteId); + } + }); + + it('rejects a non-finite remote reclaimableBytes value', async () => { + estimateManagedReclaim.mockResolvedValue({ reclaimableBytes: 0 }); + const remoteId = db.addNode({ + name: 'remote-infinity', + type: 'remote', + api_url: 'http://remote-infinity.example:1852', + api_token: 'tok', + compose_dir: '/app/compose', + is_default: false, + }); + try { + // 1e999 overflows JSON.parse to Infinity; JSON.stringify would emit null + // instead, so the raw body must be provided verbatim. + vi.spyOn(globalThis, 'fetch').mockResolvedValue(new Response( + '{"reclaimableBytes":1e999}', + { status: 200, headers: { 'Content-Type': 'application/json' } }, + )); + const res = await request(app) + .post('/api/fleet/prune/estimate') + .set('Authorization', authHeader) + .send({ targets: ['images'], scope: 'managed' }); + expect(res.status).toBe(200); + const remote = res.body.perNode.find((n: { nodeId: number }) => n.nodeId === remoteId); + expect(remote.reachable).toBe(false); + expect(remote.partial).toBeUndefined(); + expect(remote.reclaimableBytes).toBe(0); + expect(remote.error).toBe('Invalid response from remote node'); + expect(res.body.totalBytes).toBe(0); + } finally { + db.deleteNode(remoteId); + } + }); + + it('accepts a zero remote reclaimableBytes as a valid estimate', async () => { + estimateManagedReclaim.mockResolvedValue({ reclaimableBytes: 0 }); + const remoteId = db.addNode({ + name: 'remote-zero', + type: 'remote', + api_url: 'http://remote-zero.example:1852', + api_token: 'tok', + compose_dir: '/app/compose', + is_default: false, + }); + try { + vi.spyOn(globalThis, 'fetch').mockResolvedValue(new Response( + JSON.stringify({ reclaimableBytes: 0 }), + { status: 200, headers: { 'Content-Type': 'application/json' } }, + )); + const res = await request(app) + .post('/api/fleet/prune/estimate') + .set('Authorization', authHeader) + .send({ targets: ['images'], scope: 'managed' }); + expect(res.status).toBe(200); + const remote = res.body.perNode.find((n: { nodeId: number }) => n.nodeId === remoteId); + expect(remote.reachable).toBe(true); + expect(remote.partial).toBeUndefined(); + expect(remote.reclaimableBytes).toBe(0); + expect(remote.error).toBeUndefined(); + expect(res.body.totalBytes).toBe(0); + } finally { + db.deleteNode(remoteId); + } + }); + it('keeps successful local bytes when a later target rejects without timing out', async () => { estimateManagedReclaim.mockImplementation(async (target: string) => { if (target === 'images') return { reclaimableBytes: 4096 }; diff --git a/backend/src/routes/fleet.ts b/backend/src/routes/fleet.ts index 459b2a61..f36e0f98 100644 --- a/backend/src/routes/fleet.ts +++ b/backend/src/routes/fleet.ts @@ -2427,7 +2427,14 @@ fleetRouter.post('/prune/estimate', authMiddleware, async (req: Request, res: Re return { bytes: 0, error: errBody.error || `Remote returned ${response.status}` }; } const remote = (await response.json().catch(() => null)) as { reclaimableBytes?: number } | null; - if (!remote || typeof remote.reclaimableBytes !== 'number') { + // Remote nodes are an untrusted boundary: reject non-finite or + // negative values so a bad estimate cannot shrink the fleet total. + if ( + !remote + || typeof remote.reclaimableBytes !== 'number' + || !Number.isFinite(remote.reclaimableBytes) + || remote.reclaimableBytes < 0 + ) { return { bytes: 0, error: 'Invalid response from remote node' }; } return { bytes: remote.reclaimableBytes };