From 8af2054ec02df8c20ff049854c34331160a97bed Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 2 Jul 2026 06:07:01 +0000 Subject: [PATCH] fix: validate failure status values on the PUT endpoints (#135) Reject statuses outside pending|accepted|rejected with a 400 on both the single-failure and batch status routes, and reject a non-array filenames on the batch route, so junk values can no longer distort computeStats. Adds handler tests for both routes. Co-Authored-By: Claude Fable 5 https://claude.ai/code/session_01BxexPASV1UbMj8ede48RLu --- .../2026-07-02_fix-validate-failure-status.md | 27 ++++++++ src/handler.js | 11 +++ tests/node/handler.test.js | 69 +++++++++++++++++++ 3 files changed, 107 insertions(+) create mode 100644 CHANGES/2026-07-02_fix-validate-failure-status.md diff --git a/CHANGES/2026-07-02_fix-validate-failure-status.md b/CHANGES/2026-07-02_fix-validate-failure-status.md new file mode 100644 index 0000000..330bc3a --- /dev/null +++ b/CHANGES/2026-07-02_fix-validate-failure-status.md @@ -0,0 +1,27 @@ +# fix: validate failure status values on the PUT endpoints + +**Date:** 2026-07-02 +**Type:** Fix + +## Intent +Closes #135. `PUT /api/scans/:scanId/failures/:filename/status` and `PUT /api/scans/:scanId/failures/batch` accepted any string as a status and passed it straight to `projects.updateFailureStatus` / `batchUpdateStatus`. Since `computeStats` only counts `pending | accepted | rejected`, junk statuses were stored silently and distorted scan stats. Both routes now reject unknown statuses with a 400, and the batch route additionally rejects a non-array `filenames`. + +### Prompts summary +1. Implement GitHub issue #135: validate status values on the two failure-status PUT endpoints, add tests, commit, push. + +## Changes + +### `src/handler.js` +- Added `VALID_STATUSES` set (`pending`, `accepted`, `rejected`) near the two routes. +- Both PUT routes return `400 invalid status: ` when the body's status is not in the set. +- The batch route returns `400 filenames must be an array` when `filenames` is not an array. + +### `tests/node/handler.test.js` +- Added a `putJson` helper and a `PUT failure status validation` suite: bogus status returns 400 on both routes (and leaves stats untouched), non-array `filenames` returns 400, and valid statuses return 200 with updated stats. Scans are seeded via `projects.createScanFromResults` against the tmp data dir. + +## Files modified + +| File | Change | +|------|--------| +| `src/handler.js` | Validate status against a whitelist on both PUT routes; reject non-array `filenames` on batch | +| `tests/node/handler.test.js` | New tests for status/filenames validation and stats updates | diff --git a/src/handler.js b/src/handler.js index 54071d6..0182694 100644 --- a/src/handler.js +++ b/src/handler.js @@ -366,9 +366,14 @@ function createRouter() { else res.status(404).json({ error: 'project not found' }); }); + const VALID_STATUSES = new Set(['pending', 'accepted', 'rejected']); + router.put('/api/scans/:scanId/failures/:filename/status', (req, res) => { const body = req.body; if (!body || !body.status) return res.status(400).json({ error: 'status required' }); + if (!VALID_STATUSES.has(body.status)) { + return res.status(400).json({ error: `invalid status: ${body.status}` }); + } const stats = projects.updateFailureStatus(req.params.scanId, req.params.filename, body.status); if (stats) res.json(stats); else res.status(404).json({ error: 'not found' }); @@ -379,6 +384,12 @@ function createRouter() { if (!body || !body.status || !body.filenames) { return res.status(400).json({ error: 'status and filenames required' }); } + if (!VALID_STATUSES.has(body.status)) { + return res.status(400).json({ error: `invalid status: ${body.status}` }); + } + if (!Array.isArray(body.filenames)) { + return res.status(400).json({ error: 'filenames must be an array' }); + } const stats = projects.batchUpdateStatus(req.params.scanId, body.filenames, body.status); if (stats) res.json(stats); else res.status(404).json({ error: 'not found' }); diff --git a/tests/node/handler.test.js b/tests/node/handler.test.js index 9a24502..5300fea 100644 --- a/tests/node/handler.test.js +++ b/tests/node/handler.test.js @@ -37,6 +37,16 @@ async function postJson(pathname, body) { return { status: res.status, data }; } +async function putJson(pathname, body) { + const res = await fetch(baseUrl + pathname, { + method: 'PUT', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify(body), + }); + const data = await res.json().catch(() => null); + return { status: res.status, data }; +} + describe('POST /api/projects path validation', () => { it('rejects path that does not exist', async () => { const res = await postJson('/api/projects', { path: '/definitely/not/a/real/dir/xyz123' }); @@ -129,3 +139,62 @@ describe('GET /api/scans/:id pagination params', () => { assert.equal(data.failures.length, 3); }); }); + +describe('PUT failure status validation', () => { + let scan; + + beforeEach(() => { + scan = projects.createScanFromResults( + 'proj1', 'MyProject', '/tmp/proj', [], + [ + { filename: 'a.png', module: ':app', profile: 'baseline', status: 'pending' }, + { filename: 'b.png', module: ':app', profile: 'baseline', status: 'pending' }, + ] + ); + }); + + it('rejects an invalid status on the single-failure route', async () => { + const res = await putJson(`/api/scans/${scan.id}/failures/a.png/status`, { status: 'bogus' }); + assert.equal(res.status, 400); + assert.match(res.data.error, /invalid status: bogus/); + const stats = projects.getScan(scan.id).stats; + assert.equal(stats.pending, 2); + }); + + it('rejects an invalid status on the batch route', async () => { + const res = await putJson(`/api/scans/${scan.id}/failures/batch`, { + status: 'bogus', + filenames: ['a.png', 'b.png'], + }); + assert.equal(res.status, 400); + assert.match(res.data.error, /invalid status: bogus/); + const stats = projects.getScan(scan.id).stats; + assert.equal(stats.pending, 2); + }); + + it('rejects non-array filenames on the batch route', async () => { + const res = await putJson(`/api/scans/${scan.id}/failures/batch`, { + status: 'accepted', + filenames: 'a.png', + }); + assert.equal(res.status, 400); + assert.match(res.data.error, /filenames must be an array/); + }); + + it('accepts a valid status on the single-failure route and updates stats', async () => { + const res = await putJson(`/api/scans/${scan.id}/failures/a.png/status`, { status: 'accepted' }); + assert.equal(res.status, 200); + assert.equal(res.data.accepted, 1); + assert.equal(res.data.pending, 1); + }); + + it('accepts a valid status on the batch route and updates stats', async () => { + const res = await putJson(`/api/scans/${scan.id}/failures/batch`, { + status: 'rejected', + filenames: ['a.png', 'b.png'], + }); + assert.equal(res.status, 200); + assert.equal(res.data.rejected, 2); + assert.equal(res.data.pending, 0); + }); +});