What the developer wrote
Authored by Nicolas Dorier
Fix flaky CanMonetizeTest
What changed, and why it matters
This commit fixes a flaky automated test for BTCPay Server's subscription/monetization feature. The code change removes a guard that previously prevented a subscriber's plan from being restarted if it had already started. The test change waits for a background activation event and reloads the page before checking the UI. On its own, the patch looks like a timing/test-stability fix rather than a clear security fix, but removing the 'already started' guard could, in theory, allow duplicate plan activations or repeated credits if the event handler runs more than once.
Treat as a reliability/test fix unless additional vendor or researcher disclosure confirms security impact. Review TryStartPlan for idempotency and audit whether repeated credits or activations can create duplicate subscriptions, grant extra service time, or bypass trial limits. If the service is not idempotent, the removed guard should be replaced with a safer synchronization mechanism rather than simply deleted.
Security signals we found
Removal of a state guard (`!checkout.PlanStarted`) around plan activation
Background event-driven activation now always re-attempted on credit
Test change suggests previous behavior was non-deterministic / race-prone
No explicit security framing by vendor in commit or title
Evidence from the diff
In SubscriptionHostedService.cs, the commit removes the conditional if (!checkout.PlanStarted) around await TryStartPlan(subCtx, checkout, sub). Previously, when an invoice was credited to a subscriber, the plan was only started on first credit; now TryStartPlan is invoked unconditionally. The test is updated to wait for the SubscriptionEvent.SubscriberActivated event and reload the page before asserting the call-to-action is gone. The diff does not explain why the guard was removed, and there is no vendor statement that this is a security issue. The change could be benign (idempotent plan start) or could mask/repair a race where a plan fails to start on the first credit and needs a second attempt. Without more context, security relevance is speculative.
Changed components
BTCPayServer.Plugins.Subscriptions.SubscriptionHostedServiceBTCPayServer.Tests.MonetizationTests.CanMonetizeTestInspect captured patch +10 / −7
diff --git a/BTCPayServer.Tests/MonetizationTests.cs b/BTCPayServer.Tests/MonetizationTests.cs
index 84b2950..9280e45 100644
--- a/BTCPayServer.Tests/MonetizationTests.cs
+++ b/BTCPayServer.Tests/MonetizationTests.cs
@@ -6,6 +6,7 @@ using BTCPayServer.Events;
using BTCPayServer.Plugins.Monetization;
using BTCPayServer.Services;
using BTCPayServer.Views.Manage;
+using Microsoft.Playwright;
using Xunit;
using Xunit.Abstractions;
using static Microsoft.Playwright.Assertions;
@@ -126,8 +127,14 @@ public class MonetizationTests(ITestOutputHelper helper) : UnitTestBase(helper)
await s.LogIn("normal-guest@gmail.com");
await portal.AssertCallToAction(SubscriptionTests.PortalPMO.CallToAction.Danger);
await portal.ClickCallToAction();
- await s.PayInvoice(mine: true, clickRedirect: true);
- await s.FindAlertMessage();
+
+ await s.Server.WaitForEvent<SubscriptionEvent.SubscriberActivated>(async () =>
+ {
+ await s.PayInvoice(mine: true, clickRedirect: true);
+ await s.FindAlertMessage();
+ });
+ await s.FastReloadAsync();
+ await s.Page.WaitForLoadStateAsync(LoadState.DOMContentLoaded);
await portal.AssertNoCallToAction();
await s.GoToUrl("/");
diff --git a/BTCPayServer/Plugins/Subscriptions/SubscriptionHostedService.cs b/BTCPayServer/Plugins/Subscriptions/SubscriptionHostedService.cs
index 1a1e71e..665b2ae 100644
--- a/BTCPayServer/Plugins/Subscriptions/SubscriptionHostedService.cs
+++ b/BTCPayServer/Plugins/Subscriptions/SubscriptionHostedService.cs
@@ -420,11 +420,7 @@ public class SubscriptionHostedService(
{
checkout.CreditedByInvoice += diff;
await subCtx.CreditSubscriber(sub, $"Credit purchase (Inv: {invoice.Id})", diff);
-
- if (!checkout.PlanStarted)
- {
- await TryStartPlan(subCtx, checkout, sub);
- }
+ await TryStartPlan(subCtx, checkout, sub);
}
else
{
Why this scored 23/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.