Skip to content

docs(nip98): document that the u-tag host is deliberately not compared - #435

Merged
v0l merged 1 commit into
LNVPS:masterfrom
mptfire:audit/nip98-docs
Nov 2, 2026
Merged

v0l merged 1 commit into
LNVPS:masterfrom
mptfire:audit/nip98-docs

Conversation

@mptfire

@mptfire mptfire commented Nov 1, 2026

Copy link
Copy Markdown
Contributor

Docs/comment-only change; found by the same independent audit (2026-11-01).

Problem

The code comment in lnvps_api_common/src/nip98.rs claims the NIP-98 u tag check covers "host and path". It doesn't: the host comparison was removed deliberately in 0448d16 (this deployment answers on several hostnames — clearnet, Tor onion, UAT — and no single configured public_url covers them), but the comment was never updated. Anyone auditing or building on the code is told a control exists that does not.

The same section of API_DOCUMENTATION.md ("NIP-98 requirements") is silent on it, so client authors can assume host binding is enforced.

Change

  • The comment now states the actual check (path only), why the host check was dropped, the bounds that remain (60-second window, single-use event cache, optional payload binding), and the residual gap: a phishing flow that obtains an event signed for an attacker-controlled host with a colliding path is not defended against.
  • API_DOCUMENTATION.md documents the same under NIP-98 requirements.
  • Also spells out the restore path if that tradeoff ever needs revisiting: a host allowlist (several configured entries), not the old single public_url check that broke multi-homed clients.

No code behavior changes; no tests affected.

The in-code comment claimed the u tag check covers host AND path, but the
host comparison was removed deliberately in 0448d16 (multi-homed
deployment: clearnet, Tor onion, UAT). Replace the comment with the actual
tradeoff and its bounds, and state the same in the NIP-98 requirements
section of the API documentation so client authors do not rely on host
binding.

Also spells out the restore path if the phishing tradeoff ever matters: a
host ALLOWLIST (several entries), not the old single public_url check.

Found by the 2026-11-01 security audit (audit LNVM-014, design note).
@v0l
v0l marked this pull request as ready for review November 2, 2026 10:32
@v0l
v0l enabled auto-merge November 2, 2026 10:33
@v0l
v0l merged commit c15bf0b into LNVPS:master Nov 2, 2026
5 of 6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants