Skip to content

Blink: warn that a stored custodial API key is readable by the server operator - #152

Merged
Kukks merged 1 commit into
Kukks:masterfrom
pretyflaco:blink/custodial-key-server-trust-warning
Oct 1, 2026
Merged

Kukks merged 1 commit into
Kukks:masterfrom
pretyflaco:blink/custodial-key-server-trust-warning

Conversation

@pretyflaco

@pretyflaco pretyflaco commented Aug 26, 2026 •

Copy link
Copy Markdown
Contributor

What

Adds a server-trust warning to the custodial (API key) path in the Blink plugin — on the setup tab and in the README — stating that BTCPay stores the API key in the store's Lightning settings, so whoever operates the server can read it and use it for whatever the key is scoped to (a WRITE key can spend the account's balance).

The warning is deliberately proportionate. A user consciously selects the key's scopes in the Blink dashboard, so this is not "you didn't know what the key does" — it is a reminder of the server-storage consequence, which is the part that isn't visible at key-creation time: on a shared or public-registration BTCPay instance the key sits on a host the tenant may not operate. It leads with the two mitigations that actually apply here:

  • Minimize scope — READ + RECEIVE to receive; add WRITE only if BTCPay must also pay invoices. A leaked receive-only key cannot spend.
  • Revoke — a key can be rotated/revoked from the dashboard at any time.

The non-custodial ln-address= path stores no credential and is explicitly noted as unaffected.

Why

BTCPay's Flint (Spark) plugin recently added an equivalent disclosure for the same class of exposure — a server-stored spend credential readable by the instance admin — in sethforprivacy/flint#45 (raised in sethforprivacy/flint#43). This applies the same fiduciary principle to Blink's custodial path. The exposure is genuinely less critical here than in the seed case, because a Blink API key is scope-limitable and revocable where a wallet seed is neither, and the warning says so rather than overstating it.

Changes

  • Views/Shared/Blink/LNPaymentMethodSetupTab.cshtml — warning alert after the API-key setup instructions.
  • README.md — matching note under the custodial account section.

Docs/UI copy only; no behavioural change.

Summary by CodeRabbit

  • Documentation
    • Added security guidance for custodial Blink API keys, including storage visibility and server operator access.
    • Clarified scope permissions, including the spending capability of WRITE keys.
    • Recommended limiting key scopes and revoking keys when no longer needed.
    • Documented the credential-free non-custodial setup option.

… operator

The custodial (API key) path stores the key in the store's Lightning
settings on the server, so on a shared or public-registration instance the
operator can read it and use it for whatever the key is scoped to — a WRITE
key can spend the account's balance. Add a proportionate warning to the
setup tab and README: minimize scope (READ+RECEIVE to receive, WRITE only to
pay), treat the key as exposed on a server you do not operate, and note it is
revocable from the dashboard. The non-custodial ln-address path stores no
credential and is unaffected.
@coderabbitai

coderabbitai Bot commented Aug 26, 2026 •

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 145686e1-44f7-46a7-b19d-93d4a8b42a16

📥 Commits

Reviewing files that changed from the base of the PR and between 2b951b6 and 854c6b4.

📒 Files selected for processing (2)
  • Plugins/BTCPayServer.Plugins.Blink/README.md
  • Plugins/BTCPayServer.Plugins.Blink/Views/Shared/Blink/LNPaymentMethodSetupTab.cshtml

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The Blink README and custodial account setup UI now warn about API-key storage, server-operator access, scopes, WRITE spending authority, key revocation, and the credential-free non-custodial flow.

Changes

Blink credential security warnings

Layer / File(s) Summary
Credential warning documentation
Plugins/BTCPayServer.Plugins.Blink/README.md, Plugins/BTCPayServer.Plugins.Blink/Views/Shared/Blink/LNPaymentMethodSetupTab.cshtml
The README and setup alert describe API-key storage, operator access, recommended scopes, WRITE key spending authority, key revocation, and the non-custodial ln-address= flow.

Estimated code review effort: 1 (Trivial) | ~3 minutes

Merge Risk: ⚪ Minimal · up to 854c6

This PR adds a server-trust warning to the Blink custodial API-key setup guidance and README without changing behavior; no actionable merge-blocking risk remains after normal checks and review.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: warning that stored custodial Blink API keys are readable by the server operator.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0 files. (2 skipped: 2 unsupported.)

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@Kukks
Kukks merged commit 1b71357 into Kukks:master Oct 1, 2026
4 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