Commit 9bf2402
authored
auth: serve /.well-known/did/nostr/<pubkey>.json (JavaScriptSolidServer#407) (JavaScriptSolidServer#408)
* auth: serve /.well-known/did/nostr/<pubkey>.json (JavaScriptSolidServer#407)
JSS now publishes did:nostr DID documents at the spec-canonical
HTTP-resolution path for any local account whose profile carries
a Nostr Multikey verificationMethod. Each pod becomes its own
authoritative DID resolver — closing the loop that was forcing
the IdP "Sign in with Schnorr" flow to fall back to typed-username
hint (JavaScriptSolidServer#403/JavaScriptSolidServer#405).
Pieces:
- src/idp/well-known-did-nostr.js: Fastify handler. Lazily builds
a pubkey → accountId index by scanning <DATA_ROOT>/.idp/accounts/_webid_index.json
and reading each account's profile for f-form Multikey + JsonWebKey
entries. 5-min TTL; rebuild on miss. Production hook on LDP write
path is a follow-up.
- Generates a CID-shaped DID doc (`@context` per spec, `type:DIDNostr`,
`alsoKnownAs:[<webId>]` from the account record, Multikey VM
derived deterministically from the pubkey). Headers per spec:
Content-Type application/did+json (or +ld+json for .jsonld alias),
Cache-Control max-age=3600, Nostr-Timestamp, Last-Modified.
- Accepts <pubkey>.json, <pubkey>.jsonld, and bare <pubkey> on the
same handler. 400 on non-hex / wrong-length, 404 when no local
account claims the pubkey.
- src/server.js: register the route directly (before the LDP wildcard
GET /* handler). Registering inside the IdP plugin let the wildcard
swallow the dynamic-segment + .json path before our route could
match.
- src/auth/nostr.js: extracted extractNostrPubkeysFromProfile() —
enumerates every Nostr-shaped pubkey in a profile (Multikey or
JWK x-coord). Used by the index rebuild.
- src/auth/nostr.js: verifyNostrAuth's DID-doc fallback now passes
the request's own host as the first resolver via buildResolverList(),
ahead of the configured external resolver. Same-pod sign-ins
resolve with no third-party hop.
- src/auth/did-nostr.js: resolveDidNostrToWebId now accepts an array
of resolver URLs, tries each in order, returns the first hit.
verifyWebIdBacklink gains a same-origin shortcut: a DID doc served
from the same origin as the WebID is authoritative for that origin
and doesn't need a bidirectional sameAs check (which a JSS profile
doesn't carry by default — it asserts the pubkey via
verificationMethod, not via sameAs). This is the key change that
makes the flow zero-typing for local users without requiring
changes to the profile shape.
Tests: 6 integration tests (well-known endpoint live under a real
JSS server) + 4 unit tests for extractNostrPubkeysFromProfile.
Full suite 710 → 720 pass, no regressions.
Closes JavaScriptSolidServer#407. Refs JavaScriptSolidServer#403/JavaScriptSolidServer#405 (typed-username fallback this supersedes
for local users), JavaScriptSolidServer#4/JavaScriptSolidServer#386 (cross-protocol unification).
* Address copilot pass 1 on JavaScriptSolidServer#408
Eight findings, two of them genuine security bugs.
1. SSRF gadget in buildResolverList (JavaScriptSolidServer#408 line 521). The new
"try request host first" did:nostr resolution fed
request-controlled Host / X-Forwarded-* headers into a fetch().
An attacker could craft headers to force outbound fetches to
arbitrary internal hosts. Eliminated entirely by switching local
resolution to an in-process function call:
`resolveDidNostrLocally(pubkey)` — no HTTP, no SSRF surface.
resolveDidNostrToWebId reverts to single-resolver signature
(the existing external nostr.social fallback).
2. SSRF on did:nostr external fetches (line 118). The existing
resolveDidNostrToWebId / verifyWebIdBacklink fetches had no
SSRF protection. Now both go through validateExternalUrl with
blockPrivateIPs/resolveDNS/requireHttps-in-prod (matching the
LWS-CID verifier's policy). The DEFAULT_DID_RESOLVER is
trusted, but operators can configure others.
3. Index didn't filter by `authentication` membership (line 79).
A VM present in verificationMethod but intentionally NOT in
authentication (revocation pending, assertion-only, etc.)
would still get a published DID doc that asserted authentication
— defeating the user's exclusion. Now collects authentication
IDs first and indexes only matching VMs. New test: a key
pushed into verificationMethod without authentication
membership returns 404.
4. Last-Modified always now() (line 170). Now reflects the
underlying profile file's mtime so conditional GET / cache
freshness work correctly. Index value bumped to
`{ accountId, mtimeMs }`.
5. readJsonOrEmpty swallowed all errors (line 53). Now returns
null only on ENOENT; logs other errors via console.error so
operational issues (parse error, perms) aren't silent.
6. dataRoot parameter was misleading vs accounts.js (line 67).
Documented the constraint in the doc comment: the parameter
only differs meaningfully from process.env.DATA_ROOT in
non-default deployments, and findById/account-index lookups
always go through DATA_ROOT.
7. 400 message claimed lowercase but regex accepted uppercase
(line 143). Now: lowercase pubkey before regex check, regex
itself is lowercase-only, message just says "64 hex chars".
8. Stale "Fetch with timeout" JSDoc above sameOrigen (line 56).
Removed.
Test count: 10 → 11 in the new module. Full suite: 720, no
regressions.
* Address copilot pass 2 on JavaScriptSolidServer#408
Seven findings, all real.
1. Circular import nostr.js ↔ well-known-did-nostr.js. Extracted
the shared helpers (extractNostrPubkeysFromProfile,
decodeFFormSecp256k1) to src/auth/nostr-keys.js. Both callers
now import from there one-directionally. nostr.js re-exports
extractNostrPubkeysFromProfile for back-compat with the
existing test.
2. Indexer didn't validate CID semantics — could publish a DID doc
asserting `pubkey → webId` even when the underlying profile
was inconsistent. Now mirrors the resource-side checks before
indexing:
- profile @id MUST equal account.webId (no fragment-swap)
- VM controller MUST be in expected controller set
- VM MUST be referenced from authentication (already had this)
Profiles failing any check are skipped with no index entry.
3. Duplicate-pubkey "first-write wins" was silent. Now tracks every
account that claims each pubkey, drops ambiguous ones from the
index, and logs loudly via console.error. Resolution returns 404
for ambiguous keys instead of an arbitrary pick.
4. resolveDidNostrLocally fired even with IdP disabled, hitting
<DATA_ROOT>/.idp/accounts on every NIP-98 request. Gated behind
request.idpEnabled so non-IdP deployments don't touch IdP
storage.
5. readJsonOrEmpty doc said "null only on ENOENT" but actually
returned null on any error (after logging non-ENOENT). Fixed
the doc to match: ENOENT → silent null, other errors → null
with console.error so the operational issue surfaces.
6. dataRoot parameter was misleading — only affected profile reads,
not the account-index path which derives from process.env.DATA_ROOT.
Removed the parameter entirely; everything now reads from the
env. Single source of truth.
7. PR description mentioned "array of resolver URLs" but the
implementation reverted to single resolverUrl in pass 1. Will
update the PR body separately.
Test count: 11 → 11 in module (covered by existing
authentication-membership test). Full suite: 720 → 721 pass.
* Address copilot pass 3 on JavaScriptSolidServer#408
Three findings, all real.
1. /.well-known/* bypasses the WAC preHandler (correct — it's a
public namespace). But that means the wildcard write handlers
(PUT/POST/PATCH/DELETE /*) would accept unauthenticated writes
under /.well-known/did/nostr/<anything>, creating files on disk
that the GET handler would then ignore (it only reads the
account index). Storage abuse vector.
Fix: register explicit method handlers for the namespace that
return 405 Method Not Allowed with an Allow header. Fastify's
route specificity beats the wildcard, so writes never reach the
LDP layer.
2. HEAD requests fell through to the wildcard HEAD /* handler,
which looked for an on-disk file and returned 404 even when
GET returned 200. Inconsistent.
Fix: register HEAD with the same handler as GET so headers
(Content-Type, Cache-Control, Last-Modified) match.
3. The integration test hard-coded idpIssuer to
`http://127.0.0.1` (no port) while the helper bound to an
OS-assigned ephemeral port. The mismatch was harmless for the
GET-only tests we had, but oidc-provider behavior depends on
the issuer being accurate, and the divergence from every
other IdP test in the suite was a footgun.
Fix: switched to the established pattern from test/idp.test.js
— pick an available port up front via a tiny net.createServer
helper, build baseUrl, pass it as idpIssuer, listen on that
port. No more lying about the port.
Tests: added two new ones to lock the new behavior in:
- HEAD returns 200 with the same headers as GET, empty body
- PUT/POST/PATCH/DELETE all return 405 with Allow header
Total: 11 → 13 in module, 721 → 723 in full suite.
* Address copilot pass 4 on JavaScriptSolidServer#408
Five findings, all real.
1. Non-IdP deployments paid the IdP/accounts module startup cost
(transitively bcryptjs etc.) just by loading the NIP-98 verifier
in src/auth/nostr.js — and at server startup via the static
import in src/server.js. Both now lazy-load the well-known
module:
- src/auth/nostr.js: dynamic import inside the
`if (request.idpEnabled)` branch
- src/server.js: same logic moved into an async
fastify.register(...) plugin so the dynamic import lives
inside the IdP-only path without making createServer async
Cost only paid on IdP deployments now.
2. 404/400 error responses omitted Nostr-Timestamp and used
different cache policies than 200, with no documentation of
the divergence. Aligned to a documented per-status policy:
- 200 Cache-Control: max-age=3600 + Last-Modified (profile mtime)
- 404 Cache-Control: max-age=60 (short TTL — newly added
keys surface fast)
- 400 Cache-Control: no-store (malformed; never cache)
Nostr-Timestamp is now set on EVERY response (per the did:nostr
spec recommendation that clients correlate the resolver clock
with the answer). Last-Modified stays 200-only — there's no
"underlying resource" mtime for an error response.
3. Test used TEST_DATA_DIR='./data' which is also JSS's default
data root — running the suite would clobber a developer's
local pod data, and could race with other suites that use the
shared helper's `./data`. Switched to a dedicated
'./test-data-well-known-did-nostr' that's isolated to this
suite and removed in after().
4. Suite mutated process.env.DATA_ROOT but never restored it,
leaking the test value into anything that ran after.
Captures the original in `originalDataRoot` (including
undefined → unset) and restores in after(). Mirrors the
pattern in test/idp-change-password.test.js.
5. rebuildPubkeyIndex() called findById(accountId) without a
try/catch. A single corrupt or unreadable account JSON would
throw mid-loop and abort the entire index rebuild — turning
this endpoint AND in-process NIP-98 local resolution into
500s for every user until the bad file was found. Now wrapped
per-account: log + skip, keep going for the rest.
Tests: existing 13 still pass; the 404/400 tests now also assert
the new Nostr-Timestamp + Cache-Control headers. Full suite 723/723.
* Address copilot pass 5 on JavaScriptSolidServer#408
One finding, real bug.
Profiles with a relative subject (`"@id": "#me"`) hit a silent
correctness gap in `collectAuthenticationIds()`: it re-derived the
base URL from `profile['@id']`, which `stripHashIfAny()` couldn't
turn into a usable absolute. Authentication entries stayed
relative, the later `authIds.has(vmId)` check could never match
even when the VM was authenticated, and the indexer would skip
the account — looking like "no local mapping" rather than a bug.
Fix: caller passes the already-validated absolute subject as the
base. The validation in rebuildPubkeyIndex (line 122) absolutizes
the subject against `account.webId`, so by the time we get here
we have a known-absolute string. Pass that down instead of
re-deriving.
Added a regression test that writes a profile with a relative
`@id` AND a relative `authentication` entry; if the base is
honored, the VM is published and the test passes. Test count
13 → 14 in module, 723 → 724 in full suite.
* Address copilot pass 6 on JavaScriptSolidServer#408
Four findings, three code changes + a PR-description update.
1. Root-level pods were never indexed. The indexer hard-coded
`<DATA_ROOT>/<podName>/profile/card.jsonld`, but single-user
root pods store the profile at `<DATA_ROOT>/profile/card.jsonld`
with no podName subdirectory — even though the seeded account
record has `podName: 'me'`. So `dataRoot/me/profile/card.jsonld`
would silently miss, and root-pod Nostr keys never made it into
the index.
Fix: derive the on-disk profile path from the account WebID's
pathname instead of from `podName`. WebID pathname is
`/profile/card.jsonld` for root and `/alice/profile/card.jsonld`
for named — joining either with dataRoot yields the actual
on-disk path. podName isn't even read anymore.
2. Nostr-Timestamp had inconsistent semantics across status codes:
200 used the profile mtime, 400/404 used the current time.
That defeats the spec-recommended "correlate the resolver's
clock with the answer" purpose. Aligned: Nostr-Timestamp is
ALWAYS the resolver's clock at answer time. Last-Modified
stays 200-only and continues to track the underlying profile
mtime (which is what conditional-GET clients actually want).
3. PR description still referenced "array of resolver URLs" and
"same-origin sameAs check" that didn't match the shipped code.
Updated PR body via REST patch (`gh pr edit` hit a deprecated-
Projects-classic GraphQL error). Now matches what's in the
tree: single resolverUrl, same-origin DID-doc shortcut.
4. Test suite only covered named-pod layouts. Added a regression
test that:
- writes a profile at <TEST_DATA_DIR>/profile/card.jsonld
with a Nostr Multikey VM
- synthesizes a matching account record with podName='me'
(intentionally divergent from the on-disk layout)
- injects it into _webid_index.json
- hits the well-known endpoint and asserts the DID doc
comes back with alsoKnownAs pointing at the root-pod WebID
Test count: 14 → 15 in module, 724 → 725 in full suite.
* Address copilot pass 7 on JavaScriptSolidServer#408
One finding (the others on this round were stale repeats of
already-fixed items from earlier passes).
The pass-6 commit derived the on-disk profile path from the
account WebID's pathname:
profilePath = path.join(dataRoot, webIdUrl.pathname);
Copilot flagged that `webIdUrl.pathname` starts with `/`. The
specific claim — "path.join discards dataRoot" — is wrong (Node's
path.join keeps both segments, that's path.resolve's behavior).
But the underlying concern is real: if an account record ever
contained a webId whose pathname has `..` segments, the join +
read would happily traverse outside DATA_ROOT.
Operators are the only writers to account records, so this is
defense-in-depth rather than a remote-attacker vector. Still cheap
to harden:
- strip leading `/` so the pathname is treated as a relative segment
- resolve dataRoot and the joined path to absolute
- assert the result is dataRootAbs OR starts with dataRootAbs + sep
- skip + log loudly if not
Added a regression test that injects a malicious account record
with `webId: <baseUrl>/../../../etc/passwd#me`, hits the endpoint,
and asserts:
- 404 for the unrelated query (request doesn't 500)
- the evil account is silently skipped (no traversal occurred —
if it had, fs.readFile would have been called on /etc/passwd
and either thrown or returned binary, both of which would
propagate as a 500 from the handler)
Test count: 15 → 16 in module, 725 → 726 in full suite.
* Address copilot pass 8 on JavaScriptSolidServer#408 — SSRF via redirect chain
One finding, real security bug.
`fetchWithTimeout()` left fetch's default redirect behavior on,
which means an attacker-controlled (or compromised) DID resolver
could 30x-redirect to a private IP — e.g. 169.254.169.254 (cloud
metadata) or any internal-network host — and the follow-up request
would bypass the initial validateExternalUrl check entirely. Same
class of bug for the WebID-backlink fetch.
Replaced the plain timeout wrapper with `fetchWithRedirectGuard`,
mirroring the established pattern in src/auth/cid-doc-fetch.js so
the four pieces of hardening live in one place per call site:
- Manual redirect handling (redirect: 'manual'), capped at
MAX_REDIRECTS = 5. Anything past that throws.
- SSRF re-validation on EVERY hop — not just the initial URL.
An allowed origin redirecting to 169.254.169.254 now fails
the per-hop validateExternalUrl, not the initial one.
- Cross-origin redirects refused. Open-redirect on the resolver
or WebID origin can't bounce us into an arbitrary host.
- Body-size cap (1 MB default) before reading. A resolver
serving an unbounded stream can't pin RAM.
Also:
- The DID-doc same-origin shortcut now uses the FINAL post-
redirect URL, not the initially-requested URL, so a same-
origin redirect doesn't accidentally compare against the
wrong origin.
- 4xx/5xx responses no longer try to JSON-parse the body.
- Both call sites swallow fetch failures into null/false the
same way (uniform error semantics for the caller).
New tests spin up a tiny http server that:
- 302s to a different host → resolver returns null (cross-
origin redirect refused)
- 302s in a self-loop → resolver returns null after the cap
(redirect-chain refusal)
Test count: 12 → 14 in did-nostr.test.js, 726 → 728 in full suite.
* Address copilot pass 9 on JavaScriptSolidServer#408
Six findings — three real bugs (JavaScriptSolidServer#3, JavaScriptSolidServer#4, JavaScriptSolidServer#5), two test bugs (JavaScriptSolidServer#1, JavaScriptSolidServer#2),
one stale comment (JavaScriptSolidServer#6).
1. The pass-8 SSRF/redirect tests passed for the wrong reason.
`validateExternalUrl()` blocks loopback unconditionally when
`blockPrivateIPs: true`, so every request to 127.0.0.1 was
short-circuiting on the SSRF guard before the redirect handler
ever fired. Cross-origin / hop-cap behavior was never asserted.
Fix: exported `fetchWithRedirectGuard` with a `_validateUrl`
test seam, and rewrote the tests to call it directly with a
permissive stub. Now each guarantee is explicitly observable:
- cross-origin redirect → "cross-origin redirect refused"
- chain length > MAX_REDIRECTS → "too many redirects"
- body > maxBytes → "response too large"
- validator called once per hop (≥6 = 1 initial + 5 hops)
- happy path returns the body
2. The pass-7 path-containment integration test passed for the
wrong reason too — reading /etc/passwd fails JSON parsing and
the indexer's per-account try/catch turns that into a 404
regardless of containment. Also, WHATWG URL parsing strips
`..` segments, so the production code-path can't actually
reach the containment branch via a URL-parsed webId.
Fix: extracted the containment logic into
`profilePathFromWebId(dataRoot, webId, accountId)` and added
focused unit tests with raw inputs that bypass URL parsing.
The check stays in production as defense-in-depth (any future
caller that bypasses `new URL()` is still safe), and the test
suite now actually validates each branch.
3. Failure caching regression: my pass-8 refactor moved the
network/redirect/SSRF/parse failure paths from the outer
try/catch (which set `failureTtl: true`, FAILURE_CACHE_TTL=1m)
into a local catch (which forgot the flag, defaulting to
CACHE_TTL=5m). A transient resolver hiccup would get pinned
in cache for 5 minutes instead of 1. Added the flag back to
every transient-failure cache write. "No linkage" stays as
a 5-minute cache (it's a valid steady-state answer).
4. `findAccountByNostrPubkey()` could trigger a thundering herd
on TTL expiry — every concurrent request started its own full
disk scan. Now wraps `rebuildPubkeyIndex()` in an in-flight
promise (`rebuildInFlight`) so only ONE rebuild runs at a
time and every other caller awaits its result. Cleared on
completion (success or failure) so the next TTL miss can
start a fresh one.
5. `rebuildPubkeyIndex()` had no per-profile size cap. A user
can write their own profile, and TTL-expired rebuilds are
triggered by attacker-controlled NIP-98 traffic, so an
adversarially-large profile could pin the event loop on
JSON.parse during a rebuild loop. Added MAX_PROFILE_BYTES
(64 KB — generous for a real profile, hard wall for an
attack). Stat the file first, log + skip if oversized.
6. `src/auth/nostr.js` module docstring still described the
resolution chain as 3 steps (CID VM → external resolver →
fallback), missing the local-index step inserted in this PR.
Updated to the 4-step chain: CID VM → local index (IdP-only)
→ external resolver → did:nostr fallback. Noted that the
external path is now SSRF + redirect hardened.
Tests: 12 → 16 in did-nostr (added 5 unit tests + dropped 2
flawed integration tests), 16 → 16 in well-known module test
(removed the bogus integration containment test, replaced with
6 focused unit tests on the extracted helper). Full suite
728 → 736 pass.
* Address copilot pass 10 on JavaScriptSolidServer#408
Two findings, both real.
1. The pass-3 405 handlers only covered the single-segment
`/.well-known/did/nostr/:pubkeyAndExt` route. Fastify's
`:pubkeyAndExt` parameter matches ONE path component — a
request like `PUT /.well-known/did/nostr/a/b` falls through
to the wildcard `PUT /*` handler. Because `/.well-known/*`
intentionally bypasses the WAC preHandler, those wildcard
write handlers would happily accept an unauthenticated write
under this namespace. Storage-abuse vector reopened for
multi-segment paths.
Fix: register the 405 handler for the whole namespace —
`/.well-known/did/nostr`, the trailing-slash form, the
single-segment param route, AND `/.well-known/did/nostr/*`.
Loop over all four to cover empty, exact, single-segment,
and arbitrary-depth paths with the same method.
2. The DID-resolution cache was an unbounded `Map` keyed by
attacker-controlled NIP-98 pubkeys. A flood of unique pubkeys
would grow memory without limit since entries are only
evicted on subsequent lookups (lazy expiry) — never on
insertion. RAM exhaustion DoS for any pod with public NIP-98
traffic.
Fix: bounded LRU with the same `setCacheEntry` pattern
src/auth/cid-doc-fetch.js uses. Re-insert on set to bump
to MRU; drop `cache.keys().next().value` (the oldest entry)
while size > CACHE_MAX_ENTRIES (10k → bounded at a few MB
worst case). Replaced every `cache.set` with
`setCacheEntry`. Exposed `_cacheSizeForTests` and
`_CACHE_MAX_FOR_TESTS` so the bound is observable from tests.
Tests:
- "blocks writes to multi-segment paths under the namespace"
drives PUT/POST/PATCH/DELETE against the empty path, the
trailing-slash form, two-segment, and four-segment paths,
asserts 405 for all of them.
- "evicts oldest entries past the LRU cap" populates the
cache with several unique pubkeys via the failure-cache
path and asserts `cacheSize <= CACHE_MAX_ENTRIES`.
Test count: 16 → 18 in did-nostr (+ cache test, no removals),
17 → 18 in well-known module (+ multi-segment test).
Full suite: 736 → 738 pass.
* Address copilot pass 11 on JavaScriptSolidServer#408
Four findings, all real.
1. The pass-10 405 method blocks were registered inside the
`if (idpEnabled)` branch, which left non-IdP deployments wide
open: /.well-known/* unconditionally bypasses WAC, so the
wildcard write handlers (PUT/POST/PATCH/DELETE /*) would still
accept unauthenticated writes under /.well-known/did/nostr/...
and create files on disk on a non-IdP pod. Anyone could plant
a doc.
Fix: register the 405 routes UNCONDITIONALLY, before the
idpEnabled branch. The GET/HEAD generation stays IdP-only
(it actually reads the IdP accounts index — non-IdP pods
have nothing to serve), but the writes are blocked
regardless of whether GET succeeds.
2. `findAccountByNostrPubkey()` called `findById(entry.accountId)`
without a try/catch — only the rebuild loop had per-account
error handling. A corrupt account JSON could turn DID-doc
requests AND every in-process `resolveDidNostrLocally` call
from src/auth/nostr.js into 500s. Now wrapped: log once,
return a cache miss, keep going for everyone else.
3. The JWK pubkey extraction in `extractNostrPubkeysFromProfile`
ignored the y-coordinate. The verifier in src/auth/nostr.js
(jwkMatchesNostrPubkey) requires y to match the BIP-340
canonical (even-y) point for the declared x — every secp256k1
x has TWO valid points, and accepting either lets an attacker
plant a JWK at someone else's WebID. The indexer was happily
indexing keys the verifier would later reject, surfacing as
401s on advertised pubkeys ("indexed but unauthenticated").
Fix: shared `pubkeyFromValidatedJwk()` helper in nostr-keys.js
that mirrors the verifier's check (decompress 0x02||x, compare
declared y against the canonical y). Used by the indexer.
Indexing and verification now agree.
4. Cache hits in resolveDidNostrToWebId returned `cached.webId`
without re-inserting into the Map — Map preserves insertion
order, so a frequently-hit entry could still be evicted as
"oldest" once the cache reached the cap. Defeats the LRU
intent.
Fix: re-insert via `setCacheEntry(cacheKey, cached)` on hit
so hits bump entries to MRU.
Tests added:
- "returns 405 for PUT/POST/PATCH/DELETE under the namespace"
in a non-IdP server. Drives the namespace empty path, single,
and two-segment paths × all four write methods.
- "finds JsonWebKey entries when y matches the BIP-340
canonical point" — happy path with a derived correct y.
- "rejects JsonWebKey entries with mismatched y" — same x,
bogus y, asserts the indexer drops it.
- "rejects JsonWebKey entries missing y" — y absent, asserts
rejection (was previously accepted).
- The previous "x-coord only" test was wrong (used
`y: 'irrelevant'`) — replaced with the canonical-y variant.
Test count: 18 → 25 in module test (+ 3 new + 1 reshape - 1 dropped),
738 → 741 in full suite.
* Address copilot pass 12 on JavaScriptSolidServer#408 — same-origin shortcut
One finding, real security gap.
The same-origin shortcut in resolveDidNostrToWebId returned a
WebID without verifying the WebID profile actually claimed the
pubkey. The intuition behind the shortcut — "the doc and the
WebID are at the same origin, so the doc is authoritative" —
breaks on multi-tenant origins. On a pod hosting multiple
users, Mallory who controls
`<host>/.well-known/did/nostr/<MallorysPubkey>.json` can
publish a DID doc with `alsoKnownAs` pointing at Alice's
WebID on the same host, and the same-origin check would
accept it. Mallory now resolves as Alice.
Also flagged: even single-tenant pods can have writable
/.well-known/ misconfigurations that produce the same gap.
Fix:
- Drop the same-origin shortcut entirely. Always run
verifyWebIdBacklink.
- Extend verifyWebIdBacklink to accept TWO linkage shapes:
(1) CID v1 — a `verificationMethod` containing this Nostr
pubkey AND referenced from `authentication`. Mirrors
the resource-side verifier (LWS10-CID) and what JSS
profiles ship.
(2) owl:sameAs / schema:sameAs to did:nostr:<pubkey> — the
original linkage shape, kept for backward compat.
Either is sufficient. CID-VM is checked first so JSS
profiles (which use verificationMethod, not sameAs)
succeed without the shortcut.
- Inside checkCidVmBacklink, use the same
`extractNostrPubkeysFromProfile` the indexer uses, so the
JWK y-validation from pass-11 applies here too. JSON-LD
profiles with relative subjects ('@id': '#me') are handled
by absolutizing both `vm.id` and authentication entries
against the profile subject.
- Removed the now-unused `sameOrigin` helper and `foundAtUrl`
threading that fed into it.
Tests: three unit tests on `_checkCidVmBacklinkForTests`
(exposed via test seam):
- happy: VM matches pubkey AND is in authentication → true
- attack: pubkey not in profile (multi-tenant scenario) → false
- revoked: VM in verificationMethod but NOT in authentication
→ false (matches the auth-membership rule the indexer
enforces)
Test count: 18 → 21 in did-nostr, 741 → 744 in full suite.
* Address copilot pass 13 on JavaScriptSolidServer#408
Two findings, one code bug and one PR-description drift.
1. Cache key was `pubkey.toLowerCase()` and ignored `resolverUrl`.
Two resolvers can legitimately disagree about the same
pubkey — operator-run resolvers can each have their own
view, alsoKnownAs values may differ, one might have a doc
while another doesn't. With pubkey-only keying:
- a `null` cached after a miss against resolver A would
suppress a real result against resolver B for 5 minutes
- a webId cached from one resolver could be returned for
a query against a totally different resolver
Cross-resolver cache poisoning either way.
Fix: cache key is now `${resolverUrl}::${pubkey.toLowerCase()}`.
All cache.get / cache.delete / setCacheEntry call sites already
reference the single `cacheKey` variable, so the change is
localized.
Regression test: drives two failing lookups for the SAME pubkey
against TWO different resolver URLs and asserts the cache size
grows by 2 (not 1) — proving the entries are keyed independently.
2. PR description still mentioned the same-origin shortcut that
pass-12 removed. Updated PR body via REST patch to describe
the always-verify behavior: resolveDidNostrToWebId always
runs the backlink check, which accepts CID-VM linkage
(verificationMethod referenced from authentication) OR the
legacy owl:sameAs shape.
Test count: 21 → 22 in did-nostr, 744 → 745 in full suite.
* Address copilot pass 14 on JavaScriptSolidServer#408
Three findings — one HTTP-semantics bug, two test-clarity nits.
1. OPTIONS still misadvertised methods. The wildcard `OPTIONS /*`
handler returned `Allow: GET, HEAD, PUT, DELETE, PATCH, OPTIONS,
POST` for every URL — including `/.well-known/did/nostr/*`,
where writes are explicitly 405'd. CORS preflights and any
client doing capability discovery would believe the namespace
accepts writes, then get a 405 on the actual call. Misleading
and inconsistent.
Fix: register an explicit OPTIONS handler for the same
patterns as the 405s (empty path, trailing-slash form,
single-segment param, and `/*` subtree). Returns 204 with
`Allow: GET, HEAD, OPTIONS` — matches the method blocks.
Regression test asserts `Allow` is consistent across
`''`, `/`, `/x`, and `/a/b` and explicitly checks that
PUT/POST/DELETE/PATCH are NOT advertised.
2 + 3. Two unit-test names didn't match what they asserted —
"refuses an unparseable-then-resolved-outside path" actually
verified that the path stays INSIDE dataRoot, and "rejects a
path that resolves outside" actually verified the inverse
invariant (every URL-parseable webId resolves inside).
The mismatch was a holdover from when I expected URL parsing
NOT to normalize `..` segments — once I confirmed it does,
the assertions were correct but the names were stale.
Renamed to:
- "keeps a relative dataRoot + plausible webId inside the
absolute dataRoot" (was: "refuses an unparseable-then-
resolved-outside path (defense-in-depth)")
- "every URL-parseable webId with `..` segments still
resolves inside dataRoot" (was: "rejects a path that
resolves outside an absolute dataRoot")
Comments updated to match.
Test count: 25 → 26 in module test (+ OPTIONS regression),
745 → 746 in full suite.
* Address copilot pass 15 on JavaScriptSolidServer#408
Three findings — one production gap, two correctness gaps in
checkCidVmBacklink.
1. The alsoKnownAs filter only accepted `https://` URLs, despite
the comment saying "HTTP(S)". In non-production deployments
(test pods, dev fixtures, JSS-on-localhost) the SSRF guard
permits http but the resolver pre-filtered them out before any
fetch. Effect: http WebIDs were never resolvable even when the
SSRF policy would have allowed them.
Fix: accept both http and https. The SSRF layer enforces
`requireHttps: NODE_ENV === 'production'`, so http is still
refused in prod — just no longer at the wrong layer.
2. checkCidVmBacklink built the URL base for absolutizing
relative IDs from `profile['@id']` only. If the profile uses a
relative subject (e.g. `"@id": "#me"`) AND has absolute VM IDs
(a common mixed shape), `base` was empty and the
authentication-membership check fell through to raw-string
comparison — silently false-negative.
Fix: caller now passes the FETCHED document URL down as a
secondary base. Preference order: absolute @id → docUrl → "".
`verifyWebIdBacklink` plumbs `backlinkRes.url` through (the
final post-redirect URL, not the initial WebID).
3. checkCidVmBacklink didn't validate the VM's `controller`
against the profile's expected controller set, even though the
docstring claimed it mirrored the resource-side verifier. A
profile could declare a VM whose controller pointed at a
completely different origin and CID-VM backlink would still
accept it — disagreeing with the resource-side LWS10-CID
verifier that DOES check this.
Fix: collect the profile-level `controller` (with subject
fallback for CID v1 self-control) and require the VM's
controller to be in that set. If the VM has NO explicit
controller, accept only when the VM ID's origin matches the
subject's origin (so an attacker can't plant a controller-less
VM at a fragment of someone else's profile).
Tests added:
- "handles relative @id when docUrl is supplied" — relative
subject + absolute VM IDs + docUrl base → match.
- "rejects when VM controller is not in expected set" — VM
declared with a foreign-origin controller → no match
(matches the resource-side verifier).
Test count: 22 → 24 in did-nostr, 746 → 748 in full suite.
* Address copilot pass 16 on JavaScriptSolidServer#408
Two findings, both real.
1. Pubkey input validation was length-only. Since the pubkey
comes off an attacker-controlled NIP-98 event and is
interpolated into both the resolver URL path
(`<resolverUrl>/<pubkey>.json`) and the cache key, characters
like `/` would turn the resolution into an arbitrary-path
fetch on the resolver origin and create misleading cache
entries. e.g. pubkey = `<31 chars>/<32 chars>` of total
length 64 would request `<resolverUrl>/<31 chars>/<32 chars>.json`
— a different path entirely.
Fix: require `/^[0-9a-f]{64}$/i.test(pubkey)`. Also
normalize via toLowerCase once up front so cache and URL
are case-stable.
Regression test: drives a 64-char pubkey containing `/`
against an unreachable resolver and asserts a clean null
(no request, no cache entry). Plus too-short, too-long,
non-string variants.
2. verifyWebIdBacklink returned `false` uniformly for every
non-success case — both transient backlink-fetch failures
(network / SSRF refusal / redirect cap / timeout / 5xx)
AND the steady-state "fetched OK but no linkage" answer.
The caller cached both as steady-state nulls (5-minute
CACHE_TTL) instead of failures (1-minute FAILURE_CACHE_TTL).
A WebID host having a 30-second blip pinned a null answer
for 5 minutes.
Fix: tri-state semantics for verifyWebIdBacklink.
- `true` — linkage found
- `false` — fetched OK, no linkage (verified absence)
- throws TransientBacklinkError — fetch failed transiently
Caller catches the new error and caches with `failureTtl: true`.
5xx now also classifies as transient (it's "try again",
not "no"). 4xx stays as verified absence.
Removed the outer try/catch's swallow-all-into-false
pattern; transient errors now surface to the caller for
the right TTL classification.
Test count: 24 → 26 in did-nostr (+ 2 input-validation tests),
748 → 750 in full suite.
* Address copilot pass 17 on JavaScriptSolidServer#408
Two findings, both real.
1. checkCidVmBacklink had a permissive branch that accepted a
verificationMethod with NO explicit `controller`, based only
on the VM ID and profile subject sharing an origin. That
made backlink looser than the resource-side LWS10-CID
verifier (in src/auth/nostr.js) and the well-known indexer
(in src/idp/well-known-did-nostr.js), neither of which
accepts a controller-less VM. Result: did:nostr resolution
could approve a binding the resource-side verifier would
later reject — a key would "work" via DID resolution but
401 on direct CID verification, surfacing as inconsistent
binding rules across the stack.
Fix: drop the origin-fallback branch. CID v1 is now
uniformly strict — VM MUST declare an explicit `controller`
AND that controller must intersect the profile's expected
controller set. The resource-side verifier already enforced
this; backlink now matches.
Test added: a profile with a controller-less VM (everything
else CID-correct) → backlink returns false. Documents the
strictness alignment so future regressions are caught.
2. Profile read/parse failures in rebuildPubkeyIndex were
silently swallowed (`catch { continue; }`). When a local
account's profile is unreadable or malformed JSON, the
well-known endpoint returns 404 with zero log output —
"why isn't my pubkey publishing?" was undebuggable
without grepping silence.
Fix: rate-limited per-account log on stat/read/parse
failures. First occurrence per account per hour fires a
`console.error` with accountId + profilePath + error code/
message. Tracker is bounded (10k entries) so it can't grow
without limit. Cleared by `_resetIndexForTests`.
Test added: synthesize an account with a malformed JSON
profile, hit the well-known endpoint, capture
console.error, assert the diagnostic mentions both the
accountId and the profile path.
Test count: 26 → 27 in did-nostr (+ controller-less reject),
27 → 28 in well-known module (+ profile-failure log), 750 → 752
in full suite.
* Address copilot pass 18 on JavaScriptSolidServer#408 — drop unused test scaffolding
One finding, cleanup.
The "Same-origin shortcut removed" describe block set up a local
HTTP server (with `http`, `server`, `port`, `mode` vars and a
listen/close lifecycle) but the tests inside it call
`_checkCidVmBacklinkForTests` with in-memory objects and never
hit the server. The scaffolding was a leftover from an earlier
attempt to drive the live resolver against loopback —
abandoned once I confirmed validateExternalUrl
unconditionally blocks 127.0.0.1.
Removed: http import, server creation, handler bodies, listen/
close in before/after, port plumbing. Kept the on-curve key
derivation (still needed by the in-memory tests) and the
clearCache() reset.
Net: same 3 tests, less setup, faster, no port allocation.
* Address copilot pass 19 on JavaScriptSolidServer#408
Three findings, all real.
1. Stale type comment on `pubkeyIndex`. The Map values are
`{ accountId, mtimeMs }` (since pass-2), not bare accountIds.
Updated to reflect the current shape (and the new addition
below).
2. `findAccountByNostrPubkey()` re-read the account JSON from
disk on EVERY lookup via `findById(accountId)`. That's the
NIP-98 auth hot path (every signed request via
`resolveDidNostrLocally`) AND every DID-doc request. The
webId we need is already known at index-build time — there's
no reason to re-fetch it per request.
Fix: store `webId` directly on the index entry (now
`{ accountId, webId, mtimeMs }`). The lookup hot path now
answers from RAM with zero filesystem I/O. The rebuild
loop still uses `findById` (it's the only place that needs
the full account record), and that's already wrapped in
per-account try/catch so a corrupt file can't break the
rebuild.
3. `verifyWebIdBacklink()` only treated 5xx as transient.
Two more 4xx codes are conventionally transient and should
match:
- 408 Request Timeout — server timed itself out, retry
- 429 Too Many Requests — rate-limited, definitely retry
Pre-fix, both got pinned as "verified absence" with the
full 5-minute CACHE_TTL. Now classified as transient and
re-tried after the 1-minute FAILURE_CACHE_TTL.
Other 4xx (404, 410, etc.) stay as verified absence — the
host answered authoritatively that the resource doesn't
exist; caching that for the steady-state TTL is correct.
No new tests — the existing transient-vs-absence tests cover
the classification logic; 408/429 are the same shape as 5xx.
The webId-on-index change is exercised by every existing
well-known integration test (DID-doc generation reads
`account.webId` from the lookup result).
* Address copilot pass 20 on JavaScriptSolidServer#408
Three findings.
1. The OPTIONS handler for /.well-known/did/nostr/* returned a
bare 204 with only `Allow: GET, HEAD, OPTIONS`. CORS preflights
from a browser at a different origin would refuse to follow up
because no Access-Control-* headers were set — the read-only
namespace was effectively non-CORS-able.
Fix: call getCorsHeaders(request.headers.origin) for the full
CORS header set, then override `Access-Control-Allow-Methods`
with the restricted GET/HEAD/OPTIONS list. ACAO/ACAH/ACAC/
max-age all match what the rest of the server returns.
Test now also asserts:
- access-control-allow-methods has GET, HEAD, OPTIONS
- access-control-allow-methods does NOT have PUT (or any
write method)
- access-control-allow-origin and -headers are present
Driven with `Origin: https://other.example` so the
cross-origin behavior is explicit.
2. The non-IdP describe block called `createServer` (which
mutates process.env.DATA_ROOT) but didn't save/restore the
original value in after(). Subsequent tests in the same
process saw the test's DATA_ROOT — exactly the kind of
cross-suite leakage that surfaced in the full-suite run as
a flaky failure in nostr-cid-vm.test.js (which passed in
isolation).
Fix: capture `originalDataRoot` at suite scope, restore
(or delete if originally undefined) in after(). Mirrors the
existing pattern in the first describe block.
3. The "evicts oldest entries past the LRU cap" test had a stale
"Skip this on CI where it'd be too slow" comment that didn't
match the test (the test isn't skipped and only adds 5
entries). Renamed to
"cache stays at-or-under CACHE_MAX_ENTRIES after a burst of
misses" and trimmed the comment to match what's actually
asserted.
Test count: same 752 (the OPTIONS test was reshaped, not
duplicated).1 parent d459644 commit 9bf2402
7 files changed
Lines changed: 2052 additions & 102 deletions
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
14 | 14 | | |
15 | 15 | | |
16 | 16 | | |
17 | | - | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
18 | 22 | | |
19 | | - | |
20 | | - | |
| 23 | + | |
| 24 | + | |
21 | 25 | | |
22 | 26 | | |
23 | 27 | | |
24 | 28 | | |
25 | 29 | | |
26 | 30 | | |
27 | 31 | | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
28 | 36 | | |
29 | 37 | | |
| 38 | + | |
| 39 | + | |
30 | 40 | | |
31 | 41 | | |
32 | 42 | | |
33 | 43 | | |
34 | 44 | | |
35 | 45 | | |
36 | 46 | | |
37 | | - | |
38 | | - | |
39 | | - | |
40 | | - | |
41 | | - | |
42 | 47 | | |
43 | 48 | | |
44 | 49 | | |
| |||
282 | 287 | | |
283 | 288 | | |
284 | 289 | | |
285 | | - | |
286 | | - | |
287 | | - | |
| 290 | + | |
| 291 | + | |
| 292 | + | |
| 293 | + | |
| 294 | + | |
| 295 | + | |
| 296 | + | |
| 297 | + | |
| 298 | + | |
| 299 | + | |
| 300 | + | |
| 301 | + | |
| 302 | + | |
| 303 | + | |
| 304 | + | |
| 305 | + | |
| 306 | + | |
| 307 | + | |
| 308 | + | |
| 309 | + | |
| 310 | + | |
| 311 | + | |
| 312 | + | |
| 313 | + | |
288 | 314 | | |
289 | 315 | | |
290 | 316 | | |
| |||
599 | 625 | | |
600 | 626 | | |
601 | 627 | | |
602 | | - | |
603 | | - | |
604 | | - | |
605 | | - | |
606 | | - | |
607 | | - | |
608 | | - | |
609 | | - | |
610 | | - | |
611 | | - | |
612 | | - | |
613 | | - | |
614 | | - | |
615 | | - | |
616 | | - | |
617 | | - | |
618 | | - | |
619 | 628 | | |
620 | 629 | | |
621 | 630 | | |
| |||
0 commit comments