No minimum bound on subscriptions.interval_seconds lets the keeper drain a payer's on-chain allowance every poll cycle

[abayomicornelius]

Overview

SubscriptionService::create (src/services/subscription.rs:48-101) only validates that interval_seconds is positive — there is no lower bound at all:

if req.interval_seconds <= 0 {
    return Err(AppError::Validation("interval_seconds must be positive".into()));
}

The database migration matches this — interval_seconds BIGINT NOT NULL CHECK (interval_seconds > 0) (migrations/006_add_subscriptions.sql:24) — so nothing anywhere in the stack stops a client from creating a subscription with interval_seconds = 1.

The keeper background loop (main.rs:134-168) polls for due work every keeper_poll_interval_secs (default 60, configurable, floored at 5 via .max(5) at main.rs:138). SubscriptionService::run_due_executions selects every active subscription whose next_execution_at <= NOW() (subscription.rs:203-214) and, on a successful execution, reschedules next_execution_at = Utc::now() + ChronoDuration::seconds(sub.interval_seconds) (subscription.rs:263-264). With interval_seconds = 1, a subscription becomes "due" again essentially immediately after each execution — meaning it will be picked up and executed on every single keeper pass for as long as it keeps succeeding, i.e. once every keeper_poll_interval_secs (as often as every 5 seconds, if an operator has tuned the poll interval down), indefinitely, with no cap.

Since execute_subscription moves real funds against a pre-granted on-chain allowance (per this file's own extensive doc comments: "the keeper only pays the transaction fee; whether the call actually succeeds ... is entirely gated by the on-chain authorization the payer already granted"), a subscription created (accidentally via a client bug, or deliberately by a malicious/compromised payer account, or by a griefer targeting a recipient they don't like by spamming tiny payments to them) with an unreasonably small interval will have the keeper draining that pre-authorized allowance far faster than any sane recurring-payment product should allow, and hammering the Stellar network/Soroban RPC with one signed submission every few seconds per such subscription, for as long as it remains active and funded.

This is a plain missing-bound validation gap, but a consequential one given what actually consumes the field: unlike a display-only or cosmetic parameter, interval_seconds directly controls the rate of real on-chain fund movement by an autonomous background process.

Requirements

• Add a minimum bound on interval_seconds at creation time — e.g. a MIN_SUBSCRIPTION_INTERVAL_SECS constant (a reasonable floor might be measured in minutes/hours depending on the intended product use case for "subscriptions," but even a conservative floor like 60s materially closes the worst-case drain rate) — enforced in SubscriptionService::create, mirroring how MAX_CONSECUTIVE_FAILURES/KEEPER_BATCH_LIMIT are already defined as named constants in this same file.
• Add the equivalent CHECK constraint at the database level (CHECK (interval_seconds >= <min>)) so the floor holds even against a future code path that bypasses the service layer.
• Consider whether an upper bound is also worth enforcing (extremely large but finite values are less operationally dangerous than tiny ones, but are handled separately — see the companion issue on interval_seconds near i64::MAX causing a chrono::Duration panic, which this issue's fix does not itself resolve).

Acceptance Criteria

• SubscriptionService::create rejects interval_seconds below the configured minimum with AppError::Validation.
• A new migration adds the matching CHECK constraint to the subscriptions table.
• A test asserts interval_seconds = 1 (or any value below the chosen floor) is rejected at creation, and that a value at/above the floor succeeds.
• The chosen minimum is documented (as a comment on the constant, and ideally in docs/API.md) so it's an intentional product decision, not an arbitrary magic number.

Additional Notes

Edge cases

• The floor should be validated against first_execution_at/rescheduling too, not just the initial creation value — since interval_seconds is immutable after creation in the current API (there's no update endpoint, only cancel), a creation-time check is sufficient today, but worth a note if an "update subscription" endpoint is ever added.

Testing strategy

• A straightforward unit test on SubscriptionService::create analogous to the existing positive/negative-amount validation tests already present in sibling services (e.g. services/payment_request.rs's test module).

Cross-references

• Distinct from the companion issue about interval_seconds near i64::MAX panicking chrono::Duration::seconds() — that's an upper-bound crash/DoS bug; this is a lower-bound business-logic/fund-safety gap. Both point at the same field but are different bug classes requiring different fixes (one needs a floor, the other needs a ceiling well below the panic threshold).

[guptakumarranjeet150]

Hello! I would like to work on this issue.

I would love to take this on. My planned approach:

• Build out the handlers for No minimum bound on subscriptions.interval_seconds lets the keeper drain a payer's on-chain allowance every poll cycle ensuring proper HTTP status codes and error propagation.
• Add integration test suites for standard invocations and boundary/error conditions.

Excited to work on this—please assign to me and I will submit a clean PR shortly!