Skip to content

refactor: only register callback routes when their processor is active#597

Open
Anshumancanrock wants to merge 4 commits intocameri:mainfrom
Anshumancanrock:refactor/callback-routes-conditional-registration
Open

refactor: only register callback routes when their processor is active#597
Anshumancanrock wants to merge 4 commits intocameri:mainfrom
Anshumancanrock:refactor/callback-routes-conditional-registration

Conversation

@Anshumancanrock
Copy link
Copy Markdown
Collaborator

Description

The /callbacks/opennode, /callbacks/lnbits, and /callbacks/zebedee routes were always registered regardless of which payment processor was active. Each controller then had to check settings.payments.processor and return 403 if it didn't match.

This PR moves that check to the route layer so each route is now only registered when its processor is the configured one, the same way /callbacks/nodeless works after #584.

Related Issue

Follows up on #584 (as approved by @cameri).

Motivation and Context

The in-controller processor check was redundant , if a route isn't registered, it can't be hit. Keeping dead routes open is unnecessary exposure, and the scattered paymentProcessor !== 'x' checks were noise that could easily be missed when reading the controller logic.

How Has This Been Tested?

  • Removed the now-redundant processor-check tests from each controller spec
  • Updated test/unit/routes/callbacks.spec.ts to stub createSettings so the OpenNode route registers correctly during the test

Screenshots (if appropriate):

N/A

Types of changes

  • Non-functional change (docs, style, minor refactor)
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my code changes.
  • I added a changeset, or this is docs-only and I added an empty changeset.
  • All new and existing tests passed.

@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented May 2, 2026

🦋 Changeset detected

Latest commit: 6834e5a

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
nostream Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@coveralls
Copy link
Copy Markdown
Collaborator

coveralls commented May 2, 2026

Coverage Status

coverage: 64.979% (+2.2%) from 62.753% — Anshumancanrock:refactor/callback-routes-conditional-registration into cameri:main

@Anshumancanrock Anshumancanrock force-pushed the refactor/callback-routes-conditional-registration branch 2 times, most recently from a6da7c6 to ca9454e Compare May 2, 2026 17:11
@Anshumancanrock Anshumancanrock force-pushed the refactor/callback-routes-conditional-registration branch from ca9454e to 4a932d4 Compare May 2, 2026 17:15
@Anshumancanrock Anshumancanrock requested a review from cameri May 3, 2026 02:48
const requireProcessor = (name: string) =>
(_req: Request, res: Response, next: NextFunction) => {
const settings = createSettings()
if (settings.payments?.processor !== name) {
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

should we also check here if payments are enabled, not just which processor is configured?

@cameri
Copy link
Copy Markdown
Owner

cameri commented May 3, 2026

PR title does not quite match the change: callbacks are always registered, but guarded by a reusable middleware that checks the payment processor is configured before allowing the request.
Changeset has same issue.

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.

3 participants