Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions CHANGES/2026-07-02_fix-validate-failure-status.md
Original file line number Diff line number Diff line change
@@ -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: <value>` 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 |
11 changes: 11 additions & 0 deletions src/handler.js
Original file line number Diff line number Diff line change
Expand Up @@ -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' });
Expand All @@ -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' });
Expand Down
69 changes: 69 additions & 0 deletions tests/node/handler.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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' });
Expand Down Expand Up @@ -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);
});
});