Skip to content

fix(security): demo auto-admin requires an explicit runtime opt-in - #434

Open
mptfire wants to merge 1 commit into
LNVPS:masterfrom
mptfire:audit/demo-admin-guard
Open

mptfire wants to merge 1 commit into
LNVPS:masterfrom
mptfire:audit/demo-admin-guard

Conversation

@mptfire

@mptfire mptfire commented Nov 1, 2026

Copy link
Copy Markdown

Found by the same independent audit (2026-11-01).

What

GET /api/admin/v1/me/roles under the demo feature auto-assigned the admin role to any authenticated key that had none — one request, self-service admin. The feature is non-default, but it ships in-tree and a release build that accidentally enables it (or a demo deployment facing the internet) is trivially compromised, since any Nostr key can authenticate to the admin surface.

The grant now requires two conditions: the demo feature compiled in and LNVPS_DEMO_OPEN_ADMIN=1 in the process environment. The binary also warns at startup when the feature is compiled in.

Behavior

  • Demo deployments: set LNVPS_DEMO_OPEN_ADMIN=1 in the environment and the old behavior is back.
  • Any other build (including one that accidentally shipped --features demo without the variable): the endpoint reports the key's (empty) roles and promotes nothing — fails closed.

Validation

  • New regression test: without the env var, an authenticated key with no roles is reported as having none and is not promoted (the grant path itself needs a database — MockDb's role functions are stubs — so the positive grant path is covered by the e2e suite in CI).
  • Full lnvps_api_admin suite green (258 tests).
  • API_CHANGELOG.md documents the change and the opt-in variable.

Alternative considered

Deleting the block entirely. Kept (gated) instead because a demo deployment presumably wants the no-bootstrap flow; the env var preserves that while making accidental exposure require two independent mistakes.

The demo feature made GET /api/admin/v1/me/roles assign the admin role to
any authenticated key that had none. The feature is non-default but ships
in-tree, so a release build that accidentally enabled it handed admin to
whoever knocked.

The grant now additionally requires LNVPS_DEMO_OPEN_ADMIN=1 in the
environment, and the binary warns at startup when the feature is
compiled in. Demo deployments set the variable and keep working;
everything else fails closed.

Found by the 2026-11-01 security audit (LNVM-006).
@mptfire
mptfire force-pushed the audit/demo-admin-guard branch from a6adf99 to c224287 Compare November 2, 2026 06:35
@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
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.

1 participant