From 15c3aba306f35953f965acb9f83a83b5fff81dbe Mon Sep 17 00:00:00 2001 From: Adam Soltys Date: Sat, 22 Aug 2026 09:54:53 -0700 Subject: nwc: 404 missing connections + require key rotation to revive retired ones Damus's one-click setup derives its NWC secret deterministically from the user's nsec, so after the credential invalidation its re-mint reused the same retired pubkey and could never recover; meanwhile GET /app/:pubkey crashed into a 500 on missing records instead of the 404 the client needs to know to re-create the connection. We store client-supplied connection secrets (verified: all 2744 Damus app records in prod carry one), so a disclosed DB means disclosed secrets and re-authorizing the same pubkey would re-authorize the leaked bearer credential. POST /app therefore rejects any pubkey that is quarantined or predates nwc:credential-cutoff with an explicit 403 telling the client to mint a fresh secret; a fresh secret yields a fresh pubkey and passes. Owner posts refresh the record's created epoch so live connections stay clear of the cutoff, and GET /apps drops stale set members instead of 500ing the listing. Damus needs a versioned derivation salt so one-click can mint a genuinely new secret; PR submitted separately. Co-Authored-By: Claude Fable 5 --- routes/users.ts | 33 +++++++++++++++++++++++++++++++-- 1 file changed, 31 insertions(+), 2 deletions(-) (limited to 'routes') diff --git a/routes/users.ts b/routes/users.ts index e301d708..20afc5d8 100644 --- a/routes/users.ts +++ b/routes/users.ts @@ -1074,6 +1074,10 @@ export default { const { pubkey } = req.params; const { user } = req; const app = await g(`app:${pubkey}`); + // Missing records must 404, not crash into a 500: clients (Damus one-click + // setup) probe this endpoint and treat 404 as "not configured yet, create + // one" — anything else aborts their whole connection flow. + if (!app) return res.code(404).send({ error: "connection not found" }); if (app.uid !== user.id) fail("unauthorized"); const lud16 = `${user.username}@${host}`; @@ -1097,7 +1101,11 @@ export default { async apps(req, res) { const { user } = req; const pubkeys = await db.sMembers(`${user.id}:apps`); - const apps = await Promise.all(pubkeys.map((p) => g(`app:${p}`))); + // Drop set members whose app record was deleted (e.g. an invalidation + // sweep) so one stale pubkey can't 500 the whole listing. + const apps = (await Promise.all(pubkeys.map((p) => g(`app:${p}`)))).filter( + Boolean, + ); const lud16 = `${user.username}@${host}`; @@ -1148,6 +1156,24 @@ export default { let app = await g(`app:${pubkey}`); if (app && uid !== app.uid) fail("Unauthorized"); + // Rotation is mandatory after a credential disclosure: the NWC pubkey is + // derived from the bearer secret, so re-registering a retired or + // quarantined pubkey would re-authorize the exact secret that was + // disclosed (we store client-supplied secrets, so they're part of any DB + // leak). Reject it and make the client mint a fresh secret — a new secret + // yields a new pubkey and never trips this check. + const credentialCutoff = Number(await g("nwc:credential-cutoff")) || 0; + const retired = + (app && + credentialCutoff > 0 && + (!Number(app.created) || Number(app.created) < credentialCutoff)) || + (await db.sIsMember("nwc:quarantined", pubkey)); + if (retired) + return res.code(403).send({ + error: + "This connection's key was retired for security reasons; connect again with a newly generated secret", + }); + const validRenewals = new Set([ "daily", "weekly", @@ -1189,7 +1215,10 @@ export default { secret, }; - if (!app?.created) app.created = Date.now(); + // Refresh the creation epoch on every owner post: this record passed the + // retirement check above, so the owner updating it (or minting it fresh) + // asserts it's live and keeps it clear of the credential cutoff. + app.created = Date.now(); await s(`app:${pubkey}`, app); await db.sAdd(`${uid}:apps`, pubkey); -- cgit v1.2.3