fix(services): block subscription termination while an earned period is unbilled

src/core/ServicesLifecycle.sol

@@ -58,12 +58,34 @@ abstract contract ServicesLifecycle is Base {
     // ═══════════════════════════════════════════════════════════════════════════
 
     /// @notice Terminate a service
+    /// @dev Owner path only. Blocks while a fully-elapsed subscription period is still unbilled and
+    ///      owed to operators, so the owner cannot terminate inside the keeper's billing-latency
+    ///      window and reclaim (via withdrawRemainingEscrow) escrow operators already earned — the
+    ///      period becomes permanently unbillable once status != Active. `billSubscription` is
+    ///      permissionless, so anyone can clear this in one call, then terminate. The gate lives here
+    ///      (not in `_terminateService`) so the non-payment remedy `terminateServiceForNonPayment` —
+    ///      which fires precisely when escrow cannot cover a period and so can never be billed — is
+    ///      not deadlocked. Conditions mirror the eligibility gates in
+    ///      `PaymentsBilling._billSubscriptionImpl` (subscription, within TTL, full interval elapsed)
+    ///      and MUST be kept in lockstep; settlement itself cannot run here because billing lives in a
+    ///      separate facet unreachable by internal call. A non-empty operator set is a conservative
+    ///      "fees are owed" proxy — an all-inactive set just costs one harmless cursor-advancing bill.
     function terminateService(uint64 serviceId) external nonReentrant {
         Types.Service storage svc = _getService(serviceId);
         if (svc.owner != msg.sender) {
             revert Errors.NotServiceOwner(serviceId, msg.sender);
         }
 
+        if (svc.status == Types.ServiceStatus.Active && svc.pricing == Types.PricingModel.Subscription) {
+            uint64 interval = _blueprintConfigs[svc.blueprintId].subscriptionInterval;
+            bool withinTtl = svc.ttl == 0 || block.timestamp <= svc.createdAt + svc.ttl;
+            bool periodElapsed = interval != 0 && block.timestamp >= svc.lastPaymentAt + interval;
+            bool hasOperators = _serviceOperatorSet[serviceId].length() != 0;
+            if (withinTtl && periodElapsed && hasOperators) {
+                revert Errors.SubscriptionPeriodUnbilled(serviceId);
+            }
+        }
+
         _terminateService(serviceId);
     }
 

src/libraries/Errors.sol

@@ -262,6 +262,10 @@ library Errors {
     ///         Indicates a service-creation path that skipped `initSubscriptionBaseline`.
     error SubscriptionBaselineNotInitialized(uint64 serviceId);
 
+    /// @notice Termination blocked: a fully-elapsed subscription period is still unbilled and owed to
+    ///         operators. Call the permissionless `billSubscription(serviceId)` first, then terminate.
+    error SubscriptionPeriodUnbilled(uint64 serviceId);
+
     /// @notice EventDriven services are funded per-job via `msg.value`; upfront paymentAmount
     ///         at request is rejected so funds aren't collected only to be locked.
     error UpfrontPaymentNotAllowedForEventDriven();

test/exploit/Exploit_ServicesLifecycle_TerminateStealsUnbilledPeriod.t.sol

@@ -0,0 +1,116 @@
+// SPDX-License-Identifier: MIT
+pragma solidity ^0.8.26;
+
+import { BaseTest } from "../BaseTest.sol";
+import { Types } from "../../src/libraries/Types.sol";
+import { Errors } from "../../src/libraries/Errors.sol";
+
+/// @title Exploit_ServicesLifecycle_TerminateStealsUnbilledPeriod
+/// @notice Regression for the terminate-gate fix. A subscription owner used to be able to terminate
+///         the service while a FULLY-ELAPSED billing period was still unbilled, then sweep the entire
+///         escrow — including the period operators already served (the period becomes permanently
+///         unbillable once status != Active).
+///
+/// Invariant restored: operators must be paid for every subscription period they actually served
+/// before the customer can reclaim escrow for that period.
+///
+/// Fix: owner-path `terminateService` reverts `SubscriptionPeriodUnbilled` while a fully-elapsed,
+/// operator-owed, still-billable period exists. `billSubscription` is permissionless, so anyone can
+/// settle that period first; only then can the owner terminate and reclaim the genuinely-unserved
+/// tail. The gate is on the owner path only, so the non-payment remedy is not deadlocked.
+contract Exploit_ServicesLifecycle_TerminateStealsUnbilledPeriod is BaseTest {
+    uint256 constant SUBSCRIPTION_RATE = 1 ether;
+    uint64 constant SUBSCRIPTION_INTERVAL = 30 days;
+
+    function test_Regression_TerminateBlockedUntilEarnedPeriodSettled() public {
+        uint256 startTime = 1_000_000;
+        vm.warp(startTime);
+
+        Types.BlueprintConfig memory config = Types.BlueprintConfig({
+            membership: Types.MembershipModel.Fixed,
+            pricing: Types.PricingModel.Subscription,
+            minOperators: 1,
+            maxOperators: 5,
+            subscriptionRate: SUBSCRIPTION_RATE,
+            subscriptionInterval: SUBSCRIPTION_INTERVAL,
+            eventRate: 0
+        });
+
+        vm.prank(developer);
+        uint64 blueprintId = _createBlueprintWithConfigAsSender("ipfs://terminate-steal", address(0), config);
+
+        // Operator stakes and registers to actually serve the subscription.
+        vm.prank(operator1);
+        staking.registerOperator{ value: 5 ether }();
+        vm.prank(operator1);
+        staking.setDelegationMode(Types.DelegationMode.Open);
+        _directRegisterOperator(operator1, blueprintId, "");
+
+        // Customer (service owner = user1) funds 6 periods of escrow.
+        uint256 escrowAmount = SUBSCRIPTION_RATE * 6;
+        address[] memory operators = new address[](1);
+        operators[0] = operator1;
+        address[] memory callers = new address[](0);
+
+        vm.prank(user1);
+        uint64 requestId = tangle.requestService{ value: escrowAmount }(
+            blueprintId, operators, "", callers, 0, address(0), escrowAmount, Types.ConfidentialityPolicy.Any
+        );
+
+        vm.prank(operator1);
+        tangle.approveService(_approve(requestId));
+
+        uint64 serviceId = 0;
+        assertTrue(tangle.isServiceActive(serviceId), "service should be active");
+
+        // --- Period 1: served AND billed (the honest case). ---
+        vm.warp(startTime + 31 days);
+        tangle.billSubscription(serviceId);
+
+        uint256 escrowAfterPeriod1 = tangle.getServiceEscrow(serviceId).balance;
+        // ~5 periods of escrow remain after billing the first period (TWAP dust tolerance).
+        assertApproxEqAbs(escrowAfterPeriod1, SUBSCRIPTION_RATE * 5, 1, "5 periods should remain");
+
+        // --- Period 2: operator serves a FULL second interval, but no keeper has billed yet
+        //     (the normal latency window between period end and billing). ---
+        vm.warp(startTime + 62 days);
+        assertGe(block.timestamp, startTime + 62 days, "period 2 fully elapsed");
+
+        uint256 owedToOperatorsForPeriod2 = SUBSCRIPTION_RATE;
+
+        // ── FIX: the owner CANNOT terminate while the earned period is unbilled. ──
+        vm.prank(user1);
+        vm.expectRevert(abi.encodeWithSelector(Errors.SubscriptionPeriodUnbilled.selector, serviceId));
+        tangle.terminateService(serviceId);
+        assertTrue(tangle.isServiceActive(serviceId), "service still active: termination blocked");
+
+        // Anyone (here, an operator acting as keeper) settles the earned period permissionlessly.
+        vm.prank(operator2);
+        tangle.billSubscription(serviceId);
+
+        uint256 escrowAfterSettle = tangle.getServiceEscrow(serviceId).balance;
+        // The earned period-2 fee left escrow to operators, dropping the balance by ~one period.
+        assertApproxEqAbs(
+            escrowAfterPeriod1 - escrowAfterSettle,
+            owedToOperatorsForPeriod2,
+            1e12,
+            "billing settled ~1 earned period out of escrow to operators"
+        );
+
+        // ── Now the owner CAN terminate, and reclaims only the genuinely-unserved tail (~4 periods). ──
+        uint256 ownerBalanceBefore = user1.balance;
+        vm.prank(user1);
+        tangle.terminateService(serviceId);
+        assertFalse(tangle.isServiceActive(serviceId), "service terminated after settlement");
+
+        vm.prank(user1);
+        tangle.withdrawRemainingEscrow(serviceId);
+        uint256 refundedToOwner = user1.balance - ownerBalanceBefore;
+
+        // Owner gets only the post-settlement remainder — one full period SHORT of the pre-termination
+        // escrow, exactly the earned value the exploit used to steal.
+        uint256 honestMaxRefund = escrowAfterPeriod1 - owedToOperatorsForPeriod2;
+        assertApproxEqAbs(refundedToOwner, honestMaxRefund, 1e12, "owner refunded only the unserved tail");
+        assertLt(refundedToOwner, escrowAfterPeriod1 - 1e12, "owner did NOT sweep the earned period");
+    }
+}