Banned contract keeps emitting outbound SUBSCRIBE renewals while node holds an active subscription

[sanity]

Problem

A contract that is banned while this node already holds an active subscription is not torn down by the ban path, so the node keeps emitting outbound SUBSCRIBE renewal wire requests for it until the ban TTL expires.

Ring::apply_ban_decisions (crates/core/src/ring.rs ~L800) only calls ban_list.ban() / ban_list.unban() on a BanTriggered / BanLifted decision. It does not clear the contract's entry from active_subscriptions / local interest, so the contract remains in the renewal set.

The subscription-renewal loop (crates/core/src/ring.rs ~L1255, spawning crate::operations::subscribe::run_renewal_subscribe at ~L1323) iterates contracts_needing_renewal and, for each, spawns a renewal driver that emits an outbound SUBSCRIBE request using the same machinery as a client-initiated SUBSCRIBE. This renewal path does not pass through operations::reject_if_contract_banned — that egress gate (#4300) lives only at the four start_client_* originator entry points, not at the connection-maintenance renewal scheduler. So a banned-but-still-subscribed contract keeps sending outbound SUBSCRIBE renewals on every maintenance cycle until the ban's TTL lifts.

Scope / severity

This is genuine outbound network egress for a banned contract, so it is adjacent to #4300. It is out of #4300's named scope for two reasons:

• It does not push contract state. The state-egress paths (delegate-driven UPDATE fan-out, NeighborHosting overlap sync, InterestSync::Summaries stale repair, and the four client originator entry points) are all gated by feat(governance): phase 7 — repeat-offender ban TTL with wire-boundary drop #4299 / feat(governance): egress self-block for banned contracts at originator entry-points #4371. A SUBSCRIBE renewal only re-registers interest; it carries no state.
• It is time-bounded: it self-resolves when the ban TTL expires (default 1 hour) and the next BanLifted is processed, after which the subscription is legitimate again — or the subscription itself expires on its own lease.

So this is a lower-severity, bounded egress leak rather than a hole in the state-egress block, which is why it's a follow-up rather than part of #4371.

Proposed fix

Either:

1. Gate the renewal loop on the ban list — skip run_renewal_subscribe for any contract currently in ContractBanList (e.g. add an is_banned check alongside the existing can_request_subscription spam check at ~L1277), or
2. Actively unsubscribe / drop interest for a contract when it is banned — have apply_ban_decisions clear the contract from active_subscriptions / local interest (and ideally send an Unsubscribe) on BanTriggered, so it leaves the renewal set entirely.

Option 2 is the more thorough fix (it also stops the node from holding a subscription it has decided is harmful); option 1 is the smaller, more contained change.

References #4300 and #4371.

[AI-assisted - Claude]

[github-actions]

🤖 Auto-labeled

Applied labels:

• T-bug (98% confidence)
• P-medium (88% confidence)
• E-medium (80% confidence)
• A-networking (92% confidence)
• A-contracts (85% confidence)

Reasoning: The report describes a defect where banning a contract does not remove it from active_subscriptions, causing outbound SUBSCRIBE renewals to continue — this is a bug. The issue is a bounded outbound egress leak (resolves when ban TTL expires) and does not appear to be critical, so P-medium is appropriate (affects some users and the network behavior). Fixes range from a small check in the renewal loop to modifying ban handling to clear subscriptions; that suggests moderate effort (E-medium). The code paths implicated (Ring, subscription renewal, ban list) are networking/peer protocol and contract/subscription management concerns, so A-networking and A-contracts apply.

Previous labels: none

If these labels are incorrect, please update them. This helps improve the auto-labeling system.

[sanity]

Issue Priority Assessment

Generated: 2026-06-16T13:59:41.035160800+00:00
Issue hash: 5f592876

Value Assessment

This issue is confirmed still open and unfixed. Here is my assessment:

Relevance: The bug is present in the current code. The work from PR #4300/#4371 added reject_if_contract_banned to all four client-originator entry points (start_client_subscribe, start_directed_subscribe, and the GET/PUT/UPDATE equivalents), but the maintenance renewal path — run_renewal_subscribe called from the ring's subscription renewal loop around L1372 — has no equivalent check. apply_ban_decisions (ring.rs ~L800) still does not clear active_subscriptions on a BanTriggered event. The bug is exactly as described.

User Impact: The impact is narrow but genuine. Only nodes that both (a) hold an active subscription to a contract and (b) subsequently receive a BanTriggered decision for that same contract will exhibit the behavior. When that happens, the node emits outbound SUBSCRIBE renewal wire requests for a contract it has locally decided is harmful, potentially advertising continued interest to peers. Because the ban self-resolves after its TTL (default one hour), and because renewals carry no state payload, this is a bounded data-plane leak rather than a security hole. However, it undermines the semantic integrity of the ban mechanism and wastes network capacity on contracts the node has explicitly rejected.

Strategic Alignment: Freenet's contract-ban system is part of its abuse-resistance and network hygiene story. The issue was filed as a follow-up to the #4300 egress-gate work, meaning the team already has momentum here and the architectural context is fresh. Option 2 from the proposed fix (clearing the subscription in apply_ban_decisions) is the more principled resolution and would also remove the subscription from memory, making the fix both a correctness improvement and a minor resource fix. Option 1 (adding an is_banned guard in the renewal loop) is a one-liner and lower risk. Either way the scope is small and well-understood.

Blocking and Urgency: This does not appear to block other in-progress work. It is genuinely medium-priority: the self-bounding nature of the bug (TTL expiry) means no node is permanently misconfigured, and the absence of a state-egress vector means it is not a privacy or integrity issue in the way #4371 was. The right time to address it is soon after #4371 stabilizes, while the ban-list and subscription-renewal subsystems are still fresh in whoever picks it up — but there is no acute urgency.

Cost Assessment

The implementation picture is complete. Here's my assessment:

---

Relevance Check: The issue is fully actionable. The code is exactly where the issue says it is — apply_ban_decisions at line ~849 and the renewal loop at ~L1304 in ring.rs. The ContractBanList::is_banned method exists and takes a &ContractInstanceId. The ring.contract_ban_list field is accessible in the renewal loop's scope (the loop already calls ring.open_connections(), ring.contracts_needing_renewal(), and ring.can_request_subscription()). Nothing about the codebase has drifted from the issue description.

Scope: This is a single-file change to crates/core/src/ring.rs. Option 1 (gate the renewal loop) adds one is_banned check alongside the existing can_request_subscription guard at line ~1326, using the already-accessible ring.contract_ban_list. Option 2 (actively unsubscribe on ban) additionally touches apply_ban_decisions and requires passing op_manager or a channel into that function so it can enqueue unsubscribes — a slightly wider change. Neither option touches other modules. The contract_ban_list module and subscribe operation need no changes under either option.

Complexity: Option 1 is genuinely small: a one-liner if ring.contract_ban_list.is_banned(contract.id()) { skipped += 1; continue; } inserted before the can_request_subscription check. The ContractKey::id() accessor already exists. Option 2 is moderately more involved because apply_ban_decisions currently takes only &ContractBanList and a slice of decisions — passing an unsubscribe mechanism in requires threading an additional parameter (a tokio::sync::mpsc sender or similar). That said, the ring struct already has access to op_manager in the maintenance loop where apply_ban_decisions is called, so it's a parameter-threading exercise rather than an architectural change. There are no tricky edge cases: the ban list is thread-safe, is_banned does atomic reads, and the contract key type is Copy.

External Dependencies: None. This is pure in-codebase Rust code with no third-party integrations, service calls, or coordination required.

Risk: Very low for Option 1. It is strictly additive — banned contracts simply get skipped in the renewal loop, which is already the correct behavior for any egress-banned contract. The worst regression would be failing to renew a subscription that shouldn't have been banned, but that's already a risk accepted by the ban mechanism itself. Option 2 carries slightly more risk because actively unsubscribing could race with a BanLifted decision or interfere with the normal subscription lifecycle, but the renewal TTL semantics already handle re-subscription after a ban lifts (the next BanLifted would let the subscription re-establish normally).

Testing Effort: A unit test or simulation test can exercise this by banning a contract that has an active subscription entry and asserting that contracts_needing_renewal() items for that contract are skipped (or that run_renewal_subscribe is not called). The existing contract_ban_list unit tests in ring/contract_ban_list.rs show the testing patterns. A simulation-level test would be ideal (following the project's philosophy), but given this is a bounded maintenance loop and not a routing correctness issue, a focused unit test on the renewal loop's ban-check path is probably sufficient for CI. Overall testing effort is low — the DST harness and simulation framework exist, the ban list is easy to set up, and the invariant to assert is straightforward.

---

Generated by issue-prioritizer