security: AppSec audit remediation (0.11.0)

.github/dependabot.yml

@@ -0,0 +1,10 @@
+version: 2
+
+updates:
+  # Weekly rather than daily: this repo has a single npm tree at the root, and a
+  # daily cadence produces more PR noise than a maintainer team this size can
+  # triage without starting to rubber stamp them.
+  - package-ecosystem: npm
+    directory: /
+    schedule:
+      interval: weekly

.github/workflows/ci.yml

@@ -42,3 +42,23 @@ jobs:
       - name: Test
         run: npm test
 
+  audit:
+    name: Dependency audit
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+      - uses: actions/setup-node@v4
+        with: { node-version: '22.x', cache: npm }
+      - run: npm ci
+      # Production tree gates the build; the dev tree is advisory so a vitest
+      # advisory cannot wedge every PR on an unrelated change.
+      #
+      # Moderate rather than high, since the production tree is now at zero and a
+      # threshold only holds the line it is set at. It was `high`, and that is exactly
+      # how three moderate `qs` advisories sat in the shipped tree while this job
+      # reported green: reachable through Express's query parser, so on the request path
+      # of every deployment using the Express adapter.
+      - name: Audit production dependencies
+        run: npm audit --omit=dev --audit-level=moderate
+      - name: Audit all dependencies (advisory)
+        run: npm audit --audit-level=moderate || true

.github/workflows/codeql.yml

@@ -0,0 +1,55 @@
+name: CodeQL
+
+# Separate from ci.yml deliberately. CodeQL takes minutes rather than seconds, and
+# a weekly schedule only makes sense on its own workflow; folding it into the PR
+# job would make every pull request wait on an analysis that rarely changes its
+# answer between commits.
+on:
+  push:
+    branches: [main, master, develop]
+  pull_request:
+    branches: [main, master, develop]
+  schedule:
+    # Monday 07:00 UTC. The scheduled run is the one that matters: it re-analyses
+    # unchanged code against updated queries, which is how a newly published
+    # vulnerability class gets found in code nobody has touched.
+    - cron: '0 7 * * 1'
+
+concurrency:
+  group: codeql-${{ github.ref }}
+  cancel-in-progress: true
+
+jobs:
+  analyze:
+    name: Analyze (${{ matrix.language }})
+    runs-on: ubuntu-latest
+    permissions:
+      # Least privilege: the analysis needs to read the tree and write findings,
+      # and nothing else.
+      actions: read
+      contents: read
+      security-events: write
+    strategy:
+      fail-fast: false
+      matrix:
+        language: ['javascript-typescript']
+
+    steps:
+      - uses: actions/checkout@v4
+
+      - name: Initialize CodeQL
+        uses: github/codeql-action/init@v3
+        with:
+          languages: ${{ matrix.language }}
+          # security-extended over the default pack. The default set is tuned to
+          # keep false positives near zero on any repository; this one is an
+          # authentication library, so a quieter scan is the wrong trade.
+          queries: security-extended
+
+      - name: Autobuild
+        uses: github/codeql-action/autobuild@v3
+
+      - name: Perform CodeQL analysis
+        uses: github/codeql-action/analyze@v3
+        with:
+          category: /language:${{ matrix.language }}

CHANGELOG.md

@@ -7,7 +7,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 All packages in this workspace share a single version.
 
-## [Unreleased]
+## [0.11.0] - 2026-09-24
+
+Minor rather than patch, and the reason is one line of behaviour: the session cookie now
+carries `Secure` by default. A browser will not send a `Secure` cookie over `http://`, so a
+deployment that terminates TLS nowhere loses its sessions on upgrade. That is a real break
+even though the change is strictly more secure, and it is the case to read before adopting.
+
+The other changes are additive or bug fixes. `InMemoryChallengeStore` and
+`InMemorySessionStore` gained an optional constructor argument, and both now drop records
+past their retention bound, so a consumer holding a `challenge_id` past its TTL sees `null`
+where it previously saw a stale record.
 
 ### Added
 
@@ -139,6 +149,85 @@ All packages in this workspace share a single version.
 
 ### Fixed
 
+- **A malformed NIP-98 `u` tag is a mismatch, not an unhandled exception** (#33).
+  `exactUrlMatch()` called `new URL()` on the `u` tag with no guard, and that tag is
+  attacker-supplied, so a completion carrying `u: "not-a-url"` threw `TypeError` out of
+  `verifyNip98Completion()` instead of returning `NAP_COMPLETE_URL_MISMATCH`. On an
+  unauthenticated endpoint that cost three things at once: the adapter answered `500` rather
+  than the uniform `401`, the response skipped the `padAuthResponse()` floor that makes
+  failures indistinguishable (RFC §15), and the throw happened before `logFailure()` so the
+  request produced no audit record at all. A valid signature is needed to reach the check,
+  but any throwaway key will do.
+
+  Half the added tests exist to stop the fix going the other way: this is the audience
+  binding, so making the function total by loosening the comparison would be an
+  authentication bypass. A trailing slash, a different path, host, scheme or port, and
+  userinfo must all still fail. `nap-java` was unaffected, having always caught here.
+
+- **`writeNapCookieSuccess` now defaults to a protected cookie, and merges caller options
+  over those defaults** (#34). The default path emitted `session=TOKEN; Path=/`, with no
+  `HttpOnly`, `Secure` or `SameSite`, so the access token was readable by any script on the
+  page, travelled in cleartext, and rode along on cross-site requests. The helper's whole
+  stated purpose is keeping that credential away from script, and `toPublicSessionView`
+  already omits `access_token` from `GET /auth/session` on the assumption of an `HttpOnly`
+  cookie the default did not produce.
+
+  The second half was worse: partial options replaced the attributes rather than adding to
+  them, so `{ domain: '.example.com' }` (a caller setting one attribute, the case a real
+  deployment hits) silently dropped all three protections. Options are now spread over
+  `{ httpOnly: true, secure: true, sameSite: 'lax', path: '/' }`, which also leaves an
+  explicit `httpOnly: false` winning for local development. `nap-java` already defaulted
+  this way, so the divergence where one deployment was safe on the JVM and not on Node is
+  closed. Both adapters fixed, with the partial-options case covered as a regression test.
+
+- **The in-memory stores evict** (#35). `InMemoryChallengeStore` and `InMemorySessionStore`
+  never removed a record: challenges were marked expired and sessions stamped `revoked_at`,
+  and both stayed resident for the life of the process. Both maps are filled by
+  unauthenticated traffic, and the outstanding-challenge caps do not help because they count
+  only records still in `issued`, so they bound concurrency rather than memory.
+
+  The retention bounds are deliberate rather than plain expiry. A challenge is kept until
+  `result_cache_until` when it was redeemed, because that window is what makes a client
+  retry idempotent under RFC §13.3. A session is kept until `expires_at` or
+  `refresh_expires_at`, whichever is later, because `getByRefreshToken()` answers for
+  revoked sessions so a replay stays recognisable, and evicting at the access window would
+  turn a detected reuse into a merely unknown token. Both sweep on the write path as well as
+  the read path: sweeping only on reads left a server that takes logins and serves no
+  guarded requests growing without bound, which is the shape of the attack rather than an
+  edge case.
+
+- **`/auth/session` reads expiry from the server's clock** (#38). Both adapters built the
+  handler's guard options without `clock`, so a deployment on an injected clock judged
+  expiry by the wall clock on exactly that one endpoint, while `/auth/logout` two functions
+  away passed it correctly. A session live on the injected clock answered `401`. The route
+  options now come from one builder, so the call sites cannot drift apart again.
+
+### Security
+
+- **Production dependency advisories cleared, and scanning added to CI** (#36). `npm audit
+  --omit=dev` went from four high-severity findings to none. Two of them bore directly on
+  controls this repository implements: Fastify's `request.protocol` and `request.host`
+  spoofing sits underneath `createRequestDerivedBaseUrlResolver()`, and `body-parser`
+  silently disabling size enforcement on an invalid limit sits underneath the 1 kB cap
+  `createNapExpressJsonParser()` applies to an unauthenticated endpoint.
+
+  `vitest` moved 2 to 4 and `testcontainers` 10 to 12, both semver-major, clearing the
+  critical advisory on the test runner. CI now audits the production tree as a gate and the
+  full tree advisorily, which is the split that keeps it tuned: a dev-only advisory should
+  not wedge every unrelated pull request. CodeQL and Dependabot added, and `SECURITY.md`
+  records the controls that are repository settings rather than files.
+
+  **Amended after CI ran.** "None" above was true at `--audit-level=high`, which is where the
+  gate was set, and three moderate `qs` advisories were sitting under it the whole time:
+  GHSA-4mjr-xmp4-gh2g, GHSA-q8mj-m7cp-5q26 and GHSA-x5fp-wj9c-mxmx. `qs` is Express's query
+  parser, so they are on the request path of every deployment using the Express adapter, not
+  a build-time concern.
+
+  Express pins `qs` at `~6.14.0` and `~` locks the minor, so no upgrade of Express reaches
+  the fix. An `overrides` entry scoped to `express` pulls it to 6.16.0. The production gate
+  now runs at `--audit-level=moderate`, because a threshold only holds the line it is set at,
+  and the production tree is at zero rather than at "nothing above high".
+
 - **`maxSessionLifetimeSeconds` now clamps the tokens it issues**, so the ceiling is a wall
   rather than an estimate. It previously gated only the *decision* to refresh: a refresh one
   second before the ceiling minted a full-length access token, and guarded requests kept

README.md

@@ -120,6 +120,9 @@ on it and version skew surfaces as confusing `verifyEvent` failures.
 
 ## Documentation
 
+- [UPGRADING.md](UPGRADING.md) — what breaks between releases and what to do about it.
+  Read this before taking 0.11.0: the session cookie now defaults to `Secure`, which ends
+  sessions on any deployment serving over plain `http`.
 - [docs/tutorials/](docs/tutorials/README.md) — the tutorial series, in order, with what each
   one gets you.
 - [docs/NAP-v2-RFC.md](docs/NAP-v2-RFC.md) — the protocol specification. The authority.

SECURITY.md

@@ -0,0 +1,72 @@
+# Security
+
+## Reporting a vulnerability
+
+Do not open a public issue for an exploitable vulnerability in NAP. Report it privately
+through GitHub's [private vulnerability reporting][pvr] on this repository, or to the
+maintainer directly.
+
+NAP is an authentication protocol implementation, so a flaw here is a flaw in every
+deployment that depends on it. Please include the version, whether the issue is in the
+protocol or in one adapter, and a reproduction if you have one.
+
+[pvr]: https://docs.github.com/en/code-security/security-advisories/guidance-on-reporting-and-writing/privately-reporting-a-security-vulnerability
+
+## Automated checks
+
+Three run in CI and are visible in the repository:
+
+| Check | Where | Gates a PR? |
+| --- | --- | --- |
+| Dependency audit (production tree) | `.github/workflows/ci.yml` | Yes, at `--audit-level=high` |
+| Dependency audit (all, incl. dev) | `.github/workflows/ci.yml` | No, advisory only |
+| CodeQL (`security-extended`) | `.github/workflows/codeql.yml` | Findings surface in the Security tab |
+
+The production/dev split is deliberate. A dev-only advisory (a test runner, a bundler)
+should not wedge every unrelated pull request, because a check developers learn to click
+past has negative value. The production tree is small, actionable, and should always be
+green.
+
+## Settings that cannot live in this file
+
+These are repository settings rather than files, so they have to be enabled in the GitHub
+UI. They are listed here because a control nobody wrote down is a control nobody turns
+back on after it is disabled.
+
+**Secret scanning and push protection**
+`Settings > Code security and analysis > Secret scanning`, both the scan and push
+protection. A manual scan of the tree found nothing committed as of the 0.10.1 audit,
+which is the right moment to turn the guard on rather than the reason to skip it.
+
+**Private vulnerability reporting**
+`Settings > Code security and analysis > Private vulnerability reporting`. Without it a
+reporter's only options are a public issue or nothing, and the first is worse.
+
+**Dependabot alerts and security updates**
+`Settings > Code security and analysis`. `.github/dependabot.yml` schedules version
+updates; alerts are the separate switch that surfaces a CVE between scheduled runs.
+
+**Branch protection on the default branch**
+Require the build and audit checks to pass before merge. Without this the CI jobs are
+advisory in practice no matter what they return. Note that `validate` is a matrix job, so
+it reports one check per Node version (`Validate (Node 20.19.0)` and `Validate (Node 22.x)`
+today) rather than a single `Validate`; select the ones the UI actually lists rather than
+typing a name. `Dependency audit` is a single check.
+
+## Scope notes for anyone auditing this repository
+
+Worth knowing before you start, from the 0.10.1 audit:
+
+- **The audience binding is the highest-severity surface.** `createAudienceHostAllowlist()`
+  and `createRequestDerivedBaseUrlResolver()` decide what every NIP-98 proof is checked
+  against, from a client-supplied `Host` header. Both refuse an empty allowlist at wiring
+  time rather than per request; that is deliberate and should stay that way.
+- **The voucher extension reaches the network.** `nap-voucher` makes outbound calls to
+  mint URLs that arrive in the request body. The ordering in `resolver.ts` (allowlist
+  first, always) is a security property, not a style choice.
+- **Retention is not yet solved for SQL stores.** The in-memory stores evict; the Postgres
+  store has no `DELETE` path. See the open issue on store retention.
+- **The response floor is load-bearing.** `padAuthResponse()` exists so a failed
+  authentication cannot be distinguished by latency. Anything that returns early, throws,
+  or answers on a different schedule undermines it, which is how the malformed `u` tag bug
+  (a 500 escaping the floor, unaudited) mattered more than it first looked.

UPGRADING.md

@@ -0,0 +1,76 @@
+# Upgrading
+
+## 0.10.1 to 0.11.0
+
+One change breaks a working deployment, and it is the one that sounds harmless: the session
+cookie now carries `Secure` by default. Everything else is additive or a bug fix.
+
+No data migration. All packages in the workspace share a version, so upgrade them together.
+
+### Before you deploy
+
+**1. If anything serves over plain `http://`, say so explicitly.**
+
+`writeNapCookieSuccess()` used to default to a session cookie with no `HttpOnly`, no
+`Secure`, and no `SameSite`. It now defaults to all three, and a browser will not send a
+`Secure` cookie over `http://`. A deployment terminating TLS nowhere stops authenticating
+the moment it upgrades, and it does so silently: the login succeeds, the cookie is set, and
+the next request simply arrives without it.
+
+Local development and anything genuinely behind plain http needs the escape hatch:
+
+```ts
+// Cookie options are the second argument, after the cookie name.
+writeNapCookieSuccess('nap_session', { secure: false })
+```
+
+Production should terminate TLS instead. The escape hatch is per-attribute, so turning off
+`secure` leaves `httpOnly` and `sameSite` in place.
+
+**2. If you pass cookie options, re-read them.**
+
+Partial options used to *replace* the defaults rather than merge with them, so passing
+`{ maxAge }` alone silently dropped every security attribute. They now merge, which means
+an option you pass explicitly still wins, but the ones you leave out are no longer discarded.
+
+Worth an actual look: if you were compensating for the old behaviour by restating
+attributes you did not otherwise care about, those restatements are now redundant rather
+than load-bearing.
+
+**3. Expect expired challenges and sessions to disappear.**
+
+`InMemoryChallengeStore` and `InMemorySessionStore` now evict records past their retention
+bound, where they previously grew without limit. Code holding a `challenge_id` past its TTL
+now sees `null` where it used to see a stale record. The bound is derived from the
+record itself (`result_cache_until` for a redeemed challenge, `expires_at` otherwise), so
+RFC §13.3 retry safety is preserved: a redeemed challenge still inside its result-cache
+window survives.
+
+Both constructors take an optional `{ clock }`. If you inject a clock into
+`NapServerOptions`, pass the same one here. A store sweeping on a different clock from the
+server either keeps records the server has written off or drops ones it still considers
+live.
+
+This only affects the in-memory stores. The SQL stores still retain expired rows (nap#39),
+which is tracked separately.
+
+### Deploying alongside `nap-java`
+
+Both implementations now put the access token in the cookie and authenticate with
+`getByAccessToken()`. Before this release they disagreed, so a cookie minted by one server
+could not be presented to the other.
+
+If both run against one session store, deploy `nap` 0.11.0 and `nap-java` 0.9.0 together.
+A mixed fleet mid-rollout rejects cookies issued by the other side, which presents as users
+being logged out at random rather than once.
+
+### Verifying afterwards
+
+```bash
+curl -si https://your-host/auth/session | grep -i set-cookie
+```
+
+You want `HttpOnly`, `Secure`, and `SameSite` on that line. If sessions stop working right
+after the upgrade and that line is present, the likely cause is the first item above: the
+cookie is being set correctly and withheld on the next request because the origin is not
+`https`.

examples/merchant-app/package.json

@@ -1,7 +1,7 @@
 {
   "name": "@imani/nap-example-merchant-app",
   "private": true,
-  "version": "0.10.1",
+  "version": "0.11.0",
   "type": "module",
   "description": "The runnable example that docs/tutorials/ is built around.",
   "exports": {
@@ -16,21 +16,21 @@
     "db:down": "docker compose down -v"
   },
   "dependencies": {
-    "@imani/nap-adapter-express": "0.10.1",
-    "@imani/nap-client-nip46": "0.10.1",
-    "@imani/nap-client-web": "0.10.1",
-    "@imani/nap-core": "0.10.1",
-    "@imani/nap-react": "0.10.1",
-    "@imani/nap-server": "0.10.1",
-    "@imani/nap-store-postgres": "0.10.1",
+    "@imani/nap-adapter-express": "0.11.0",
+    "@imani/nap-client-nip46": "0.11.0",
+    "@imani/nap-client-web": "0.11.0",
+    "@imani/nap-core": "0.11.0",
+    "@imani/nap-react": "0.11.0",
+    "@imani/nap-server": "0.11.0",
+    "@imani/nap-store-postgres": "0.11.0",
     "express": "^4.21.2",
     "nostr-tools": "^2.23.0",
     "pg": "^8.13.1",
     "react": "^19.0.0",
     "react-dom": "^19.0.0"
   },
   "devDependencies": {
-    "@imani/nap-client-http": "0.10.1",
+    "@imani/nap-client-http": "0.11.0",
     "@types/express": "^5.0.0",
     "@types/pg": "^8.11.10",
     "@types/react": "^19.0.0",