feat: support client-provided session credentials

[Kukks]

Summary

• Allow clients to provide a token when creating a session via POST /lnurl/session
• Server derives sessionId = SHA-256(token).slice(0, 32) — same token always produces the same LNURL
• If a session with the derived ID already exists, the old SSE connection is torn down and replaced
• Session hijack is impossible by construction: different tokens always produce different sessionIds

How it works

Clients can optionally send a JSON body with POST /lnurl/session:

{
  "token": "a-deterministic-hex-string-at-least-32-chars"
}

If omitted, the server generates random credentials as before (fully backward compatible).

The server derives sessionId from the token using SHA-256(tokenBytes).slice(0, 32). The wallet never sends a sessionId — only the token.

Validation:

• token must be a hex string >= 32 characters
• Returns 400 with descriptive error for invalid values

Companion PR

• fix: extract LNURL session into app-level provider with reusable sessions arkade-os/wallet#559 — wallet derives deterministic token from private key using HMAC-SHA256

Test plan

• All 38 tests pass, including:
  • Derives sessionId from client-provided token
  • Same LNURL for same token across reconnects
  • Reconnect replaces old SSE connection
  • Authenticate with client-provided token
  • Reject short token (< 32 chars)
  • Reject non-hex token
  • Different tokens produce different sessionIds (hijack impossible)

[arkanaai]

🔴 CRITICAL: Session hijacking via public sessionId — request changes

Finding 1 — Session takeover (CRITICAL, money-losing bug)

src/session-manager.ts:25-28

if (this.sessions.has(id)) {
  this.destroy(id);
}

The sessionId is embedded in the LNURL callback URL in plaintext (/lnurl/{sessionId} — see server.ts:94). Every payer who receives the LNURL can decode bech32 and extract the sessionId. This is by design for the LNURL protocol.

The attack: an attacker who decodes the LNURL calls POST /lnurl/session with the victim's sessionId and their own token. The server:

1. Destroys the legitimate wallet's SSE session (and any pending invoice)
2. Creates a new session with the attacker's token
3. The attacker now controls the session — when the next payer pays, the attacker provides their own Lightning invoice

This is an invoice substitution attack. The attacker steals payments meant for the victim.

Fix: On reconnect, the provided token MUST match the existing session's token before tearing it down:

if (this.sessions.has(id)) {
  const existing = this.sessions.get(id)!;
  if (existing.token !== token) {
    // Return an error — caller doesn't own this session
    return null; // or throw
  }
  this.destroy(id);
}

And in server.ts, handle the null/error by returning 403.

---

Finding 2 — Asymmetric credential provision allows confusing states

src/session-manager.ts:22-23

const id = providedId || randomBytes(16).toString("hex");
const token = providedToken || randomBytes(32).toString("hex");

A client can provide sessionId without token (or vice versa). This creates a session with a deterministic ID but a random token — meaning the wallet can never reconnect with the same token. The "stable endpoint" feature silently degrades to broken behavior.

Fix: In server.ts, validate that either both are provided or neither:

if ((providedId == null) !== (providedToken == null)) {
  res.status(400).json({ error: "sessionId and token must both be provided or both omitted" });
  return;
}

---

Finding 3 — No hex format validation

src/server.ts:78-84

The PR description says sessionId and token should be "hex strings", and the companion wallet PR (arkade-os/wallet#559) derives them via SHA-256 (which produces hex). But the server only validates length, not format.

A non-hex sessionId with URL-special characters (e.g., /, ?, #, %) could break URL routing or cause ambiguous path matching in GET /lnurl/:id and GET /lnurl/:id/callback.

Fix: Add hex validation:

const HEX_RE = /^[0-9a-f]+$/i;
if (providedId != null && !HEX_RE.test(providedId)) {
  res.status(400).json({ error: "sessionId must be a hex string" });
  return;
}

---

Finding 4 — destroy() on reconnect drops pending invoices silently

src/session-manager.ts:25-28 (after fixing Finding 1)

When a legitimate wallet reconnects, destroy() rejects any pendingInvoice with "Session closed". The payer gets an error even though the wallet is still alive — it just reconnected. Consider draining the pending invoice to the new session instead of rejecting it, or at minimum document this as expected behavior.

---

Finding 5 — Missing test: attacker token on existing session

The test suite covers the happy path (reconnect with same creds) but doesn't test the adversarial case: reconnecting with the same sessionId but a different token. After fixing Finding 1, add a test that verifies this returns 403 and the original session is NOT destroyed.

---

Non-blocking notes

• package-lock.json (3891 lines added): This repo uses pnpm per engines in package.json. Should this be pnpm-lock.yaml instead? A package-lock.json committed alongside pnpm can cause confusion.
• The 6 new tests are well-structured and cover the main happy paths. Good work there.
• The SSE teardown-on-reconnect design is sound once the token check is added.

---

Bottom line: Finding 1 is a money-losing vulnerability that must be fixed before merge. An attacker with the public LNURL can hijack any reusable session. Findings 2 and 3 are lower severity but should also be addressed.

[arkanaai]

🟡 Re-review after aade372f — server-side sessionId derivation

The refactor to derive sessionId = SHA-256(token)[:32] is a clean design improvement. It eliminates the session hijacking vector (my Finding 1), the asymmetric credential issue (Finding 2), and the hex validation gap (Finding 3). The .gitignore addition addresses the package-lock.json note. The adversarial test covers the anti-hijack case (Finding 5). Well done — the core design is now sound.

One remaining bug from my original review (Finding 4) is now exploitable due to the reconnection feature:

---

🔴 BUG: Stale close handler destroys new session on reconnect

src/session-manager.ts:31-33 (new code) + destroy() at line 135

Trace through the reconnect flow:

1. Wallet connects with token T. Session A created with id = derive(T). Handler registered: resA.on("close", () => this.destroy(id)).
2. Wallet reconnects (same token). destroy(id) tears down session A from the map — but resA is not ended and its close handler remains registered.
3. Session B created with the same id, stored in the map. resB.on("close", ...) registered.
4. Old TCP connection resA eventually closes (client drop, timeout, etc.) → resA's stale close handler fires → this.destroy(id) → kills session B.

The wallet's active session is destroyed by a ghost handler from the previous connection.

Fix — guard close handler so it only destroys the session it owns:

// In create(), replace the close handler:
sseRes.on("close", () => {
  const current = this.sessions.get(id);
  if (current?.sseRes === sseRes) {
    this.destroy(id);
  }
});

And in the reconnect path, explicitly end the old SSE response to prevent the orphaned connection from lingering:

if (existing) {
  if (existing.token !== token) return null;
  existing.sseRes.end();   // <-- close old SSE cleanly
  this.destroy(id);
}

Add a test: reconnect with the same token, then close the old connection and verify the new session remains active and can still handle invoice requests.

---

Note: merge ordering with arkade-os/wallet#559

The wallet PR sends { token } only (no sessionId), which aligns with this refactored API. However, the wallet PR will break against the current main (which doesn't have this code). Merge this PR first, then the wallet PR.

---

All other findings from my original review are resolved. This is the last blocking issue.

[arkanaai]

See detailed re-review comment. One blocking bug remains: stale SSE close handler destroys the new session on reconnect. All previous critical findings are resolved.