What the developer wrote
Authored by Chukwuleta Tobechi
resolve code rabbit
What changed, and why it matters
This commit makes two small fixes in the BTCPay Server Subscriptions plugin. It changes a test helper so two UI form-filling branches are mutually exclusive, and it tightens when a subscription plan change can be scheduled and updates statistics when a plan actually changes. There is no clear security vulnerability in the diff itself; it looks like a routine bug-fix or code-review follow-up.
Treat as a normal functional fix. Review the subscription plan-change flow for edge cases such as scheduling changes on already-expired or non-normal-phase subscribers, but no immediate security action is indicated by this diff alone.
Security signals we found
Business-logic change around subscription plan migration timing
Addition of state preconditions (PeriodEnd > now, Phase == Normal) before scheduling a plan change
Statistics update added after plan swap
Evidence from the diff
The patch modifies SubscriptionHostedService.cs in two places. First, when a subscription enters Expired/Grace phase and a pending new plan is activated, it now records the previous plan ID, swaps in the new plan, and calls UpdatePlanStats for both the old and new plan IDs. Second, it restricts AtPeriodEnd plan changes to subscribers whose PeriodEnd is in the future and whose phase is Normal. The test file change turns a sequential ‘if’ into ‘else if’, making PlanChanges and PlanChangesWithTiming branches mutually exclusive.
Changed components
BTCPayServer/Plugins/Subscriptions/SubscriptionHostedService.csBTCPayServer.Tests/SubscriptionTests.csInspect captured patch +13 / −6
diff --git a/BTCPayServer.Tests/SubscriptionTests.cs b/BTCPayServer.Tests/SubscriptionTests.cs
index fca8041..bca2b2e 100644
--- a/BTCPayServer.Tests/SubscriptionTests.cs
+++ b/BTCPayServer.Tests/SubscriptionTests.cs
@@ -1262,8 +1262,7 @@ public class SubscriptionTests(ITestOutputHelper testOutputHelper) : UnitTestBas
await s.Page.Locator($"#PlanChanges_{i}__SelectedType").SelectOptionAsync(new[] { PlanChanges[i].ToString() });
}
}
-
- if (PlanChangesWithTiming is not null)
+ else if (PlanChangesWithTiming is not null)
{
for (var i = 0; i < PlanChangesWithTiming.Length; i++)
{
diff --git a/BTCPayServer/Plugins/Subscriptions/SubscriptionHostedService.cs b/BTCPayServer/Plugins/Subscriptions/SubscriptionHostedService.cs
index 0117a83..2d58644 100644
--- a/BTCPayServer/Plugins/Subscriptions/SubscriptionHostedService.cs
+++ b/BTCPayServer/Plugins/Subscriptions/SubscriptionHostedService.cs
@@ -272,11 +272,15 @@ public class SubscriptionHostedService(
if (newPhase is PhaseTypes.Expired or PhaseTypes.Grace && m is { NewPlan: not null, NewPlanId: not null } && m.NewPlanId != m.PlanId)
{
+ var prevPlanId = m.PlanId;
var prevPlan = m.Plan;
(m.PlanId, m.Plan) = (m.NewPlanId, m.NewPlan);
(m.NewPlanId, m.NewPlan) = (null, null);
if (!m.Plan.FeaturesLoaded)
await ctx.PlanFeatures.FetchPlanFeaturesAsync(m.Plan);
+
+ await UpdatePlanStats(ctx, prevPlanId);
+ await UpdatePlanStats(ctx, m.PlanId);
}
if (newPhase is PhaseTypes.Expired)
@@ -662,10 +666,14 @@ public class SubscriptionHostedService(
if (planChangeRecord.Timing == PlanChangeData.ChangeTiming.AtPeriodEnd)
{
- portal.Subscriber.NewPlanId = planId;
- portal.Subscriber.NewPlan = plan;
- await ctx.SaveChangesAsync();
- return new PlanMigrationResult.Scheduled();
+ if (portal.Subscriber.PeriodEnd is not null && portal.Subscriber.PeriodEnd > DateTimeOffset.UtcNow
+ && portal.Subscriber.Phase == SubscriberData.PhaseTypes.Normal)
+ {
+ portal.Subscriber.NewPlanId = planId;
+ portal.Subscriber.NewPlan = plan;
+ await ctx.SaveChangesAsync();
+ return new PlanMigrationResult.Scheduled();
+ }
}
}
Why this scored 28/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.