The ledger id is a UUID, and only an authenticated request could have found it
`ledger_entries.id` is a uuid. The ancestry route parsed it with Number.parseInt, so every real row would have returned 400 "invalid ledger id" — the endpoint had never successfully served anything. The unauthenticated probe that "proved the route was live" returned 401 from requireAuth BEFORE the handler ran, so it could not have seen this. A route-existence check and an acceptance test are not the same evidence, which is exactly why the acceptance step demands a real authenticated 200 against a real row rather than a 401. Fixed to a UUID match, and the test asserts the real production id shape passes while '1' and a traversal string do not. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CQJeAG8vcDoL5zkiaJyVb8
This commit is contained in:
@@ -33,9 +33,16 @@ const LEDGER_COLUMNS = 'id, sport, game_date, player_key, player_name, stat, sid
|
|||||||
* Ancestry for the published Read behind one ledger row. Additive: no existing
|
* Ancestry for the published Read behind one ledger row. Additive: no existing
|
||||||
* response changes, and nothing here is consulted by any other surface.
|
* response changes, and nothing here is consulted by any other surface.
|
||||||
*/
|
*/
|
||||||
|
// `ledger_entries.id` is a UUID. The first version of this route parsed it as
|
||||||
|
// an integer, which would have rejected every real row with a 400 — and the
|
||||||
|
// unauthenticated probe returned 401 from the middleware before the handler ran,
|
||||||
|
// so a route-existence check could never have found it. Only an authenticated
|
||||||
|
// request against a real row does.
|
||||||
|
const UUID_RE = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i;
|
||||||
|
|
||||||
router.get('/ledger/:id', requireAuth, async (req, res) => {
|
router.get('/ledger/:id', requireAuth, async (req, res) => {
|
||||||
const id = Number.parseInt(req.params.id, 10);
|
const id = String(req.params.id || '');
|
||||||
if (!Number.isFinite(id) || id <= 0) {
|
if (!UUID_RE.test(id)) {
|
||||||
return res.status(400).json({ error: 'invalid ledger id' });
|
return res.status(400).json({ error: 'invalid ledger id' });
|
||||||
}
|
}
|
||||||
try {
|
try {
|
||||||
|
|||||||
@@ -230,6 +230,21 @@ describe('the ancestry product contract', () => {
|
|||||||
expect(routeSrc).toMatch(/router\.get\('\/ledger\/:id', requireAuth/);
|
expect(routeSrc).toMatch(/router\.get\('\/ledger\/:id', requireAuth/);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test('the ledger id is treated as a UUID, which is what the column actually is', () => {
|
||||||
|
// The first version parsed it with Number.parseInt, so every real row would
|
||||||
|
// have 400'd. The unauthenticated probe returns 401 from the middleware
|
||||||
|
// BEFORE the handler runs, so a route-existence check could never see it —
|
||||||
|
// only an authenticated request against a real row can.
|
||||||
|
expect(routeSrc).toMatch(/UUID_RE\s*=\s*\/\^\[0-9a-f\]\{8\}/);
|
||||||
|
expect(routeSrc).not.toMatch(/Number\.parseInt\(req\.params\.id/);
|
||||||
|
const m = routeSrc.match(/const UUID_RE = (\/.*\/i);/);
|
||||||
|
// eslint-disable-next-line no-eval
|
||||||
|
const re = eval(m[1]);
|
||||||
|
expect(re.test('1426ed64-e35f-4908-bb14-13d66c77643b')).toBe(true);
|
||||||
|
expect(re.test('1')).toBe(false);
|
||||||
|
expect(re.test('../../etc/passwd')).toBe(false);
|
||||||
|
});
|
||||||
|
|
||||||
test('it is keyed on a stable product identifier, not a raw natural key', () => {
|
test('it is keyed on a stable product identifier, not a raw natural key', () => {
|
||||||
expect(routeSrc).toMatch(/ledger_entries/);
|
expect(routeSrc).toMatch(/ledger_entries/);
|
||||||
expect(routeSrc).not.toMatch(/read_natural_key/);
|
expect(routeSrc).not.toMatch(/read_natural_key/);
|
||||||
|
|||||||
Reference in New Issue
Block a user