fix: harden authorization checks on freezeorder and paytobuyer

[grunch]

Summary

This PR tightens the authorization logic of the freezeorder and paytobuyer admin/solver commands in bot/start.ts, bringing them to parity with the checks already enforced by settleorder and cancelorder.

Changes

• Order state validation for freezeorder: since freezing settles the hold invoice, the command now refuses to run on orders that are not in a state with funds in escrow (ACTIVE, FIAT_SENT, DISPUTE).
• Strict community comparison: order.community_id is now compared against the solver's default_community_id using strict string equality instead of a loose ObjectId comparison.
• Assigned-solver check: when a dispute exists for the order, a community solver can only act on it if they are the solver assigned to that dispute.
• Conflict-of-interest check: a solver can no longer run these commands on orders where they are themselves the buyer or the seller. These attempts are logged.

Full admins (ctx.admin.admin) are unaffected.

Testing

• npx tsc compiles cleanly.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes
  • Tightened solver authorization for freezing orders: status checks, community matching, dispute ownership, and prevention when solver is buyer or seller
  • Strengthened payout protections with the same solver checks and dispute verification
  • Ensured hold invoices are settled before marking orders frozen and surfaced settlement failures instead of silently logging them

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 8d336685-a323-4de0-ab60-c06956ef3064

📥 Commits

Reviewing files that changed from the base of the PR and between 4c40e9d and b1cc40a.

📒 Files selected for processing (2)

• bot/start.ts
• ln/hold_invoice.ts

🚧 Files skipped from review as they are similar to previous changes (1)

• bot/start.ts

---

Walkthrough

Two admin commands—freezeorder and paytobuyer—are hardened with stricter authorization and eligibility checks. freezeorder now enforces status constraints and prevents non-superadmin solvers from executing unless the order has a community, the community matches their default community, any dispute is assigned to them, and they are not a participant. paytobuyer applies similar solver authorization checks before existing payment logic. The hold-invoice settlement is performed before persisting FROZEN, and invoice-settlement failures are rethrown.

Changes

Admin command authorization hardening

Layer / File(s)	Summary
freezeorder: status eligibility and solver authorization bot/start.ts	freezeorder restricts execution to orders in ACTIVE, FIAT_SENT, or DISPUTE, and for non-superadmin solvers enforces community presence, string-equality with default_community_id, dispute ownership (dispute.solver_id), and blocks solvers who are buyer or seller (logs warnings when blocked).
freezeorder: settle hold invoice before persisting FROZEN bot/start.ts, ln/hold_invoice.ts	freezeorder now settles the hold invoice (calls settleHoldInvoice) before updating the order to FROZEN. settleHoldInvoice now rethrows on settlement failure so callers must not treat orders as frozen unless settlement succeeded.
paytobuyer: solver authorization before payout bot/start.ts	paytobuyer loads any order dispute and, for non-superadmin solvers, requires community presence and string-equality with the solver's default_community_id, requires dispute ownership by the executing solver, and blocks payout when the solver is the buyer or seller (with warning logs).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• lnp2pBot/bot#732: Related community-scoped authorization checks for checkorder/checkinvoice.
• lnp2pBot/bot#677: Related stricter order-eligibility gating for /paytobuyer and /settleorder.

Suggested reviewers

• Luquitasjeffrey
• mostronatorcoder

Poem

🐰 I twitched my nose at the command-line gate,
checking communities, disputes, and fate—
invoices settled before seals are set,
no conflicted solver can place the bet.
hops ✨

🚥 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 pull request title directly and accurately describes the main change: hardening authorization checks on two specific commands (freezeorder and paytobuyer), which is the primary focus of the changeset.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/freezeorder-paytobuyer-authz

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint install failed due to a network error.

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@bot/start.ts`:
- Around line 342-350: The handler for freezeorder sets order.status = 'FROZEN'
before calling settleHoldInvoice, which can leave an order stuck in FROZEN if
settleHoldInvoice fails (because the helper swallows/logs errors); change the
flow so you only persist order.status = 'FROZEN' after settleHoldInvoice
succeeds (or make settleHoldInvoice throw on failure and abort/rollback the
status change), i.e., locate the freezeorder flow and the settleHoldInvoice call
and move the DB update of order.status (and related save/update code) to run
after a successful settleHoldInvoice result (apply the same change to the
duplicate block around lines 383-390).

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 464354ab-2ba7-4959-bbba-b743da2df6bc

📥 Commits

Reviewing files that changed from the base of the PR and between f05189c and 4c40e9d.

📒 Files selected for processing (1)

• bot/start.ts

[mostronatorcoder]

I reviewed the current HEAD strictly. The authorization hardening looks good, and the freezeorder persistence ordering bug is fixed. But I still see one blocker in the new paytobuyer flow.

When the command is invoked on a FROZEN order, the handler now persists order.status = 'PAID_HOLD_INVOICE' before calling payToBuyer(bot, order). If payToBuyer then fails before the payment is actually sent or before the retry row is created, the order is no longer FROZEN, so:

• the admin cannot safely retry through the same FROZEN path anymore, and
• subscribe_invoice.ts also treats PAID_HOLD_INVOICE as a progressed state and will not re-enter the same freeze guard.

This is the same class of ordering problem that the PR correctly fixed for freezeorder: persist the state transition only after the irreversible side effect succeeds, or make the failure path fully transactional/idempotent. Right now the code moves the order out of FROZEN before the payout operation has proved success.

Please either:

1. keep the order in FROZEN until payToBuyer has definitely taken ownership of the retry lifecycle, or
2. prove the pre-write is safe by making the failure path impossible to lose (for example, no swallowed exceptions before pending-payment persistence, and explicit idempotent recovery for PAID_HOLD_INVOICE orders with no successful payout).

[mostronatorcoder]

I re-checked the current code path more carefully and my previous REQUEST_CHANGES review was too aggressive.

The paytobuyer pre-write to PAID_HOLD_INVOICE is already part of the existing payout model, not a new unrecoverable ordering bug introduced by this PR:

• /paytobuyer explicitly accepts both FROZEN and PAID_HOLD_INVOICE (bot/start.ts:1034-1036), so the command remains manually retryable after the pre-write.
• the handler checks PendingPayment before retrying (bot/start.ts:1073-1079), which prevents duplicating an in-flight retry lifecycle.
• the normal subscriber path already persists PAID_HOLD_INVOICE before calling payToBuyer (ln/subscribe_invoice.ts:203-212 and ln/subscribe_invoice.ts:275-276), so this state transition pattern predates the PR and matches the existing semantics.
• on non-successful payout responses, payToBuyer persists a PendingPayment retry record (ln/pay_request.ts:204-218); for the deliberate non-retry cases like AMOUNT_MISMATCH, the order is still manually re-drivable through /paytobuyer because PAID_HOLD_INVOICE is accepted.

So the blocker I raised is not actually a valid regression in this PR. The real changes here are the auth hardening plus the freezeorder fix, and those parts look good. Sorry for the noise.

[mostronatorcoder]

Re-reviewed the current HEAD after re-checking the existing payout semantics. The previous REQUEST_CHANGES I left on paytobuyer was too aggressive: PAID_HOLD_INVOICE is already the established pre-payout state in the normal subscriber path, /paytobuyer explicitly accepts both FROZEN and PAID_HOLD_INVOICE, and failed payout attempts already feed the retry lifecycle through PendingPayment or manual re-invocation. With that corrected, the actual changes in this PR look good: the auth hardening is sound and the freezeorder persistence ordering fix is the right one.