Skip to content

fix: fail closed without vss auth - #665

Open
ben-kaufman wants to merge 5 commits into
masterfrom
fix/fail-closed-unauthenticated-vss
Open

fix: fail closed without vss auth#665
ben-kaufman wants to merge 5 commits into
masterfrom
fix/fail-closed-unauthenticated-vss

Conversation

@ben-kaufman

@ben-kaufman ben-kaufman commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

This PR fails closed when LNURL-auth is missing so wallet backups cannot fall back to unauthenticated VSS.

Description

Removes the unauthenticated VSS client fallback. If LNURL-auth is not configured, backup setup and Lightning node build now error instead of using a public encryption key or fixed-header VSS.

Env currently always sets an LNURL-auth URL, so this is a footgun fix rather than a user-visible change on production configs.

Related: synonymdev/audit#55 (BITKIT-SEC-003). Library-side rejection is in synonymdev/vss-rust-client-ffi.

Linked Issues/Tasks

Screenshot / Video

N/A

QA Notes

Manual Tests

  • 1. Fresh wallet → backup/Lightning start with current Env LNURL-auth URL: setup still succeeds.
  • 2. regression: empty LNURL-auth URL → VSS setup and node build fail instead of using unauthenticated VSS.

Automated Checks

  • Added ChannelMigrationPersistenceTests coverage for retaining migration data on setup failure, clearing it after success, and preserving a newer pending migration.
  • SwiftFormat and translation validation pass locally.

Made with Cursor

@greptile-apps

greptile-apps Bot commented Aug 12, 2026

Copy link
Copy Markdown

Greptile Summary

The PR removes unauthenticated VSS fallback behavior so backup and Lightning-node initialization fail closed when LNURL-auth is unavailable.

  • Requires LNURL-auth parameters when initializing the backup VSS client.
  • Requires a non-empty LNURL-auth URL before building the Lightning node and during stale-monitor recovery.
  • Adds a shared vssAuthRequired service error and documents the security change.

Confidence Score: 5/5

The PR appears safe to merge and consistently enforces fail-closed VSS authentication across the changed setup paths.

The backup client and Lightning builder now reject missing LNURL-auth rather than selecting unauthenticated construction, while preserving authenticated setup and stale-monitor recovery behavior.

Important Files Changed

Filename Overview
Bitkit/Services/LightningService.swift Removes fixed-header VSS node construction and rejects empty LNURL-auth configuration before either normal or stale-monitor-recovery builds.
Bitkit/Services/VssBackupClient.swift Removes unauthenticated backup-client fallback and consistently reports the shared authentication-required error.
Bitkit/Utilities/Errors.swift Adds localized handling for the new vssAuthRequired service error.
changelog.d/next/fail-closed.security.md Accurately records that wallet backups no longer fall back to unauthenticated VSS.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[VSS or Lightning setup] --> B{LNURL-auth configured?}
  B -- No --> C[Throw vssAuthRequired]
  B -- Yes --> D[Build authenticated VSS client or node]
  D --> E{Build succeeds?}
  E -- Yes --> F[Setup complete]
  E -- DangerousValue --> G[Enable stale monitor recovery]
  G --> H[Retry authenticated node build]
  E -- Other error --> I[Propagate setup failure]
Loading

Reviews (1): Last reviewed commit: "fix: fail closed without vss auth" | Re-trigger Greptile

jvsena42
jvsena42 previously approved these changes Aug 13, 2026

@jvsena42 jvsena42 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Self-contained PR, no issues found

Comment thread Bitkit/Services/LightningService.swift
Comment thread Bitkit/Services/VssBackupClient.swift Outdated
jvsena42
jvsena42 previously approved these changes Aug 13, 2026
@jvsena42

Copy link
Copy Markdown
Member

@ben-kaufman conflict

ben-kaufman and others added 4 commits August 17, 2026 07:34
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
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