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
5 changes: 3 additions & 2 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
"mime-types": "^2.1.35",
"n3": "^1.26.0",
"oidc-provider": "^9.6.0",
"secure-json-parse": "^2.7.0",
"sql.js": "^1.13.0",
"tar-stream": "^3.2.0"
},
Expand Down
38 changes: 31 additions & 7 deletions src/auth/nostr.js
Original file line number Diff line number Diff line change
Expand Up @@ -243,17 +243,41 @@ export async function verifyNostrAuth(request) {

// Validate payload hash if present and request has body
const payloadTag = getTagValue(event, 'payload');
if (payloadTag && request.body) {
let bodyString;
if (typeof request.body === 'string') {
bodyString = request.body;
// Validate whenever a payload tag is present AND a body was provided —
// keyed on `!== undefined`, not truthiness, so a JSON body that parses
// to a falsy value (`null`, `false`, `0`, `""`) is still hash-checked
// rather than silently skipping the integrity guard (Copilot review on
// #573). Fastify leaves request.body `undefined` when no body was sent.
if (payloadTag && request.body !== undefined) {
// Hash the EXACT bytes the client signed. NIP-98's `payload` tag is
// sha256(request body) over the wire bytes. crypto.update() accepts a
// string (encoded UTF-8) or a Buffer (raw bytes), so we pass each
// through in its native form — never a lossy conversion.
let bodyData;
if (typeof request.rawBody === 'string') {
// application/json raw wire string captured by the parser (#565),
// because by this point request.body is already a parsed object and
// the original bytes are gone. Re-serializing the object (the old
// fallback) only matched when the client happened to send minified
// JSON in Node's exact key order — pretty-printed or differently-
// escaped bodies 401'd despite a valid signature. (JSON is UTF-8 by
// spec, so the captured string round-trips losslessly.)
bodyData = request.rawBody;
} else if (typeof request.body === 'string') {
bodyData = request.body;
} else if (Buffer.isBuffer(request.body)) {
bodyString = request.body.toString();
// Hash the Buffer DIRECTLY. A .toString() round-trip UTF-8-mangles
// binary / non-UTF-8 bodies (e.g. an image PUT) and would cause
// false mismatches against the raw-byte hash the client signed.
bodyData = request.body;
} else {
bodyString = JSON.stringify(request.body);
// No raw bytes captured (shouldn't happen for HTTP requests:
// application/json sets rawBody, other types stay a Buffer). Keep a
// deterministic fallback rather than throwing.
bodyData = JSON.stringify(request.body);
}

const expectedHash = crypto.createHash('sha256').update(bodyString).digest('hex');
const expectedHash = crypto.createHash('sha256').update(bodyData).digest('hex');
if (payloadTag.toLowerCase() !== expectedHash.toLowerCase()) {
return { webId: null, error: 'Payload hash mismatch' };
}
Expand Down
37 changes: 37 additions & 0 deletions src/server.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import Fastify from 'fastify';
import sjson from 'secure-json-parse';
import rateLimit from '@fastify/rate-limit';
import { readFile } from 'fs/promises';
import { readFileSync } from 'fs';
Expand Down Expand Up @@ -269,7 +270,43 @@ export function createServer(options = {}) {
done(null, body);
});

// Override the default application/json parser so the NIP-98 payload-hash
// check (src/auth/nostr.js) can verify against the EXACT bytes the client
// signed, not a re-serialization of the parsed object (#565). The default
// parser discards the raw bytes once it produces an object, so capturing
// req.rawBody here is the only point they still exist. Behaviour otherwise
// mirrors Fastify 4's defaultJsonParser exactly — empty body → 400,
// secure-json-parse (same prototype-pollution protection JSS gets today),
// 400 on malformed — so no other request path changes. (Must
// removeContentTypeParser first: Fastify throws on a duplicate type.)
fastify.removeContentTypeParser('application/json');
fastify.addContentTypeParser('application/json', { parseAs: 'string' }, (req, body, done) => {
req.rawBody = body;
if (body === '' || body == null) {
// Match Fastify's FST_ERR_CTP_EMPTY_JSON_BODY exactly (code +
// message + status), so the error-response shape — which surfaces
// err.code — is identical to the default parser's.
const err = new Error("Body cannot be empty when content-type is set to 'application/json'");
err.code = 'FST_ERR_CTP_EMPTY_JSON_BODY';
err.statusCode = 400;
return done(err, undefined);
}
let json;
try {
// The malformed-JSON path already mirrors Fastify's default: it
// sets statusCode 400 on the raw parser error without adding an FST
// code (the default does the same), so no code is set here.
json = sjson.parse(body);
} catch (err) {
err.statusCode = 400;
return done(err, undefined);
}
done(null, json);
});

// Attach server config to requests
// Raw request body for the application/json parser to stash (#565).
fastify.decorateRequest('rawBody', null);
fastify.decorateRequest('connegEnabled', null);
fastify.decorateRequest('notificationsEnabled', null);
fastify.decorateRequest('idpEnabled', null);
Expand Down
182 changes: 182 additions & 0 deletions test/nip98-payload-hash.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,182 @@
/**
* NIP-98 payload-hash verifies the RAW request bytes, not a re-serialization
* of the parsed body (#565).
*
* The bug: src/auth/nostr.js hashed `JSON.stringify(request.body)` — the
* parsed-then-recompacted body — so a NIP-98 client that sent pretty-printed
* (or otherwise non-minified) JSON 401'd with "Payload hash mismatch" even
* though signature/URL/method/timestamp were all valid. NIP-98's `payload`
* tag is sha256 over the exact body bytes on the wire.
*
* The fix has two halves, tested here:
* 1. server.js captures the raw application/json bytes as request.rawBody
* (the default parser discards them once it produces an object), while
* otherwise mirroring Fastify's default parser — empty → 400,
* secure-json-parse (prototype-pollution safe), 400 on malformed.
* 2. nostr.js hashes request.rawBody when present, never a re-serialization.
*/

import { describe, it, before, after } from 'node:test';
import assert from 'node:assert';
import { createServer } from '../src/server.js';
import { verifyNostrAuth } from '../src/auth/nostr.js';
import { generateSecretKey, getPublicKey, nip98Token } from '../src/nostr/event.js';
import fs from 'fs-extra';

const DATA_DIR = './test-data-nip98-payload';

describe('application/json parser: rawBody capture + preserved semantics (#565)', () => {
let server;
let originalDataRoot;

before(async () => {
originalDataRoot = process.env.DATA_ROOT;
await fs.remove(DATA_DIR);
await fs.ensureDir(DATA_DIR);
server = createServer({
logger: false,
root: DATA_DIR,
public: true, // open writes so we exercise the parser without auth
forceCloseConnections: true,
});
await server.ready();
});

after(async () => {
if (server) await server.close();
if (originalDataRoot === undefined) delete process.env.DATA_ROOT;
else process.env.DATA_ROOT = originalDataRoot;
await fs.remove(DATA_DIR);
});

it('accepts a valid (pretty-printed) JSON body — parser stays functional', async () => {
const res = await server.inject({
method: 'PUT',
url: '/public/pretty.json',
headers: { 'content-type': 'application/json' },
payload: JSON.stringify({ a: 1, b: 'two' }, null, 2),
});
assert.ok(res.statusCode === 200 || res.statusCode === 201 || res.statusCode === 204,
`pretty JSON PUT should succeed, got ${res.statusCode}`);
});

it('rejects malformed JSON with 400 (mirrors Fastify default, not 500)', async () => {
const res = await server.inject({
method: 'PUT',
url: '/public/bad.json',
headers: { 'content-type': 'application/json' },
payload: '{ not valid json',
});
assert.strictEqual(res.statusCode, 400);
});

it('empty JSON body errors with Fastify\'s FST_ERR_CTP_EMPTY_JSON_BODY code (response-shape fidelity)', async () => {
const res = await server.inject({
method: 'PUT',
url: '/public/empty.json',
headers: { 'content-type': 'application/json' },
payload: '',
});
assert.strictEqual(res.statusCode, 400);
assert.strictEqual(res.json().code, 'FST_ERR_CTP_EMPTY_JSON_BODY',
'empty-body error must carry Fastify\'s standard code so the response shape matches the default parser');
});

it('blocks __proto__ prototype pollution (secure-json-parse preserved)', async () => {
const res = await server.inject({
method: 'PUT',
url: '/public/proto.json',
headers: { 'content-type': 'application/json' },
payload: '{ "__proto__": { "polluted": true } }',
});
// secure-json-parse throws on __proto__ → our parser surfaces 400.
assert.strictEqual(res.statusCode, 400, 'a __proto__ payload must be rejected');
assert.strictEqual({}.polluted, undefined, 'Object.prototype must not be polluted');
});
});

describe('NIP-98 payload hash uses the raw request bytes (#565)', () => {
const sk = generateSecretKey();
const pk = getPublicKey(sk);

// A pretty-printed body whose minified re-serialization differs byte-for-byte.
const entry = { name: 'alice', nested: { x: 1, y: 2 }, tags: ['a', 'b'] };
const prettyBody = JSON.stringify(entry, null, 2);
const compactBody = JSON.stringify(entry);
const parsed = JSON.parse(prettyBody);

const url = 'http://localhost:9999/pool/entries/' + pk + '.json';

// Build a mock Fastify request whose URL/host/method match what was signed,
// so verification reaches the payload-hash check.
function mockRequest(token, { rawBody, body }) {
return {
method: 'PUT',
url: '/pool/entries/' + pk + '.json',
protocol: 'http',
hostname: 'localhost:9999',
headers: { host: 'localhost:9999', authorization: `Nostr ${token}` },
rawBody,
body,
};
}

it('a pretty-printed body with rawBody captured passes the payload check', async () => {
// Client signed sha256(prettyBody) — exactly the bytes it sent.
const token = nip98Token(url, 'PUT', sk, prettyBody);
const result = await verifyNostrAuth(mockRequest(token, { rawBody: prettyBody, body: parsed }));
assert.notStrictEqual(result.error, 'Payload hash mismatch',
`pretty body should pass the payload check; got error: ${result.error}`);
assert.ok(result.webId && result.webId.startsWith('did:nostr:'),
`should resolve to the did:nostr identity; got ${JSON.stringify(result)}`);
});

it('control: the same body WITHOUT rawBody (object only) still mismatches — proves rawBody is the fix', async () => {
const token = nip98Token(url, 'PUT', sk, prettyBody);
// No rawBody → the old code path hashes JSON.stringify(parsed) = compact.
const result = await verifyNostrAuth(mockRequest(token, { rawBody: undefined, body: parsed }));
assert.strictEqual(result.error, 'Payload hash mismatch',
'without the captured raw bytes, a pretty body must still fail (the bug)');
});

it('a compact body still verifies (no regression for already-working clients)', async () => {
const token = nip98Token(url, 'PUT', sk, compactBody);
const result = await verifyNostrAuth(mockRequest(token, { rawBody: compactBody, body: parsed }));
assert.notStrictEqual(result.error, 'Payload hash mismatch',
`compact body should still pass; got error: ${result.error}`);
});

it('a falsy JSON body (false) is still hash-checked, not skipped (Copilot review on #573)', async () => {
// The body literally parses to `false`. A WRONG payload tag must still
// be caught — under the old `&& request.body` truthiness guard this
// validation was skipped entirely for falsy bodies.
const falsyRaw = 'false';
const wrongToken = nip98Token(url, 'PUT', sk, 'a-different-body');
const mismatch = await verifyNostrAuth(mockRequest(wrongToken, { rawBody: falsyRaw, body: false }));
assert.strictEqual(mismatch.error, 'Payload hash mismatch',
'a falsy body with a wrong payload tag must be rejected, not skipped');

// And the matching payload for the same falsy body passes.
const goodToken = nip98Token(url, 'PUT', sk, falsyRaw);
const ok = await verifyNostrAuth(mockRequest(goodToken, { rawBody: falsyRaw, body: false }));
assert.notStrictEqual(ok.error, 'Payload hash mismatch',
`a falsy body with the correct payload should pass; got ${ok.error}`);
});

it('a binary / non-UTF-8 Buffer body hashes the raw bytes (Copilot review on #573)', async () => {
// Bytes that are NOT valid UTF-8 — a .toString() round-trip would
// mangle them (replacement chars) and break the hash. The client
// signs sha256 over the raw bytes; the server must do the same.
const binary = Buffer.from([0xff, 0xfe, 0x00, 0x01, 0x80, 0xc3, 0x28]);
// Sanity: this fixture genuinely changes bytes under a UTF-8 string
// round-trip, so the old `.toString()` path WOULD have mismatched.
assert.ok(!Buffer.from(binary.toString('utf8'), 'utf8').equals(binary),
'fixture must be lossy under a UTF-8 round-trip');
const token = nip98Token(url, 'PUT', sk, binary);
// Non-JSON body → no rawBody; request.body is the raw Buffer (the `*`
// content-type parser keeps it as a Buffer).
const result = await verifyNostrAuth(mockRequest(token, { rawBody: undefined, body: binary }));
assert.notStrictEqual(result.error, 'Payload hash mismatch',
`binary body must hash raw bytes, not a UTF-8 round-trip; got error: ${result.error}`);
});
});