fix(nostr): NIP-98 payload hash verifies raw request bytes, not a re-serialization — closes JavaScriptSolidServer#565 (JavaScriptSolidServer#573)

melvincarvalho · web-flow · commit e31293b2c285 · 2026-06-16T10:29:08.000+02:00
* fix(nostr): NIP-98 payload hash verifies raw request bytes, not a re-serialization (JavaScriptSolidServer#565) src/auth/nostr.js hashed JSON.stringify(request.body) — the parsed- then-recompacted body — to check the NIP-98 `payload` tag. NIP-98 defines that tag as sha256 over the EXACT body bytes on the wire, so any client sending pretty-printed (or differently-escaped / different key-order) JSON 401'd with 'Payload hash mismatch' despite a valid signature, URL, method, and timestamp. It silently forced every NIP-98 client to send minified JSON in Node's exact serialization — not a real spec requirement, and the bug only bit Nostr auth (DPoP has no payload check), making valid signatures look broken. Root cause: by the time the auth check runs, Fastify's default application/json parser has already turned the body into an object — the raw bytes are gone, so the re-serialization was the best the code could do, and it only matched by coincidence when the client minified. Fix (two halves): - server.js overrides the application/json content-type parser to capture the raw bytes as request.rawBody before parsing. It otherwise mirrors Fastify 4's defaultJsonParser exactly — empty body → 400, secure-json-parse (the SAME prototype-pollution protection JSS gets today; not plain JSON.parse), 400 on malformed — so no other request path changes. secure-json-parse promoted to a direct dependency for this. - nostr.js hashes request.rawBody when present, never a re-serialization. Other content types are unaffected: they stay Buffers via the '*' parser and already hashed raw bytes correctly; only application/json (parsed to an object) was broken. Tests (test/nip98-payload-hash.test.js, 6): - parser: pretty JSON accepted, malformed → 400, __proto__ → 400 + Object.prototype not polluted (proves secure parsing preserved) - payload hash: a pretty body with rawBody passes AND resolves to the did:nostr identity; the same body WITHOUT rawBody still mismatches (control — proves rawBody is the fix); a compact body still verifies (no regression). Full suite: 968/968 (the global JSON-parser override regressed nothing). Closes JavaScriptSolidServer#565. * review fix (JavaScriptSolidServer#573): hash Buffer bodies directly, not via a lossy toString() Copilot caught a real correctness bug in the pre-existing Buffer branch: `request.body.toString()` UTF-8-decodes the bytes, which mangles binary / non-UTF-8 bodies (an image PUT, etc.) — so the server's hash wouldn't match the raw-byte sha256 a NIP-98 client signed. crypto.update() accepts a Buffer directly, so we now pass the Buffer through unconverted and only ever hash native forms (string → UTF-8, Buffer → raw bytes). New test: a deliberately non-UTF-8 Buffer body (with a sanity check that it's genuinely lossy under a UTF-8 round-trip) verifies against its raw-byte payload hash — would have failed under the old .toString() path. 7/7 in the file; full suite green. * review pass-2 fix (JavaScriptSolidServer#573): hash-check payload for falsy-parsed bodies too Copilot pass-2 (separate from the Buffer fix): the `if (payloadTag && request.body)` guard skipped payload validation when a JSON body parsed to a falsy value (null/false/0/""), so a client could sign a payload tag, send a falsy body, and the integrity check silently didn't run. Guard on `request.body !== undefined` instead — Fastify leaves body undefined only when no body was sent — so falsy-but-present bodies are still verified while genuinely bodyless requests stay skipped. New test: a `false` body with a WRONG payload tag is now rejected (was skipped), and with the correct tag passes. 8/8 in file; full suite green. * review pass-3 fix (JavaScriptSolidServer#573): empty-JSON error carries FST_ERR_CTP_EMPTY_JSON_BODY code Copilot pass-3: the custom parser's empty-body 400 was a plain Error with no err.code, but JSS's error handler emits code: err.code, so the response shape drifted from Fastify's default (which uses FST_ERR_CTP_EMPTY_JSON_BODY) — undercutting the 'mirrors the default exactly' claim. Set err.code = 'FST_ERR_CTP_EMPTY_JSON_BODY' with the same message/status. The malformed-JSON path already matched (Fastify sets statusCode on the raw parser error without an FST code; so do we) — documented that in a comment so it doesn't look like an oversight. New test: an empty application/json body returns 400 with code === 'FST_ERR_CTP_EMPTY_JSON_BODY'. 9/9 in file; full suite green.
diff --git a/package-lock.json b/package-lock.json
diff --git a/package.json b/package.json
@@ -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"
   },
diff --git a/src/auth/nostr.js b/src/auth/nostr.js
@@ -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' };
     }
diff --git a/src/server.js b/src/server.js
@@ -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';
@@ -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);
diff --git a/test/nip98-payload-hash.test.js b/test/nip98-payload-hash.test.js
@@ -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}`);
+  });
+});
