Fix: Circuit Relay v2 connect destination peer without active reservation.

[sumanjeet0012]

What was wrong?

Fixes #1342

In CircuitV2Protocol._handle_connect(), the admission gate that should verify the destination peer has an active reservation was accidentally checking the source peer's reservation instead:

# Before — wrong variable
if not self.resource_manager.can_accept_connection(peer_id=source_addr):

peer_id is the destination (from msg.peer); source_addr is the source (from stream.muxed_conn.peer_id). Passing source_addr meant: if the source has a reservation, the check passes — regardless of whether the destination ever reserved. A source peer could therefore reach any relay-connected destination that had never opted in.

How was it fixed?

The single incorrect gate was replaced with two clearly-separated checks:

1. Destination admission — can_accept_connection(peer_id=peer_id): rejects with NO_RESERVATION if the destination has no active reservation.
2. Source connection limit — checked independently against source_addr: rejects with RESOURCE_LIMIT_EXCEEDED if the source has exhausted its own slot count.

NO_RESERVATION = 103 was also added to StatusCode so the rejection path has a semantically accurate status code.

# After — correct
if not self.resource_manager.can_accept_connection(peer_id=peer_id):
    await self._send_status(stream, StatusCode.NO_RESERVATION,
        "Destination peer has no active reservation on this relay", ...)
    await stream.reset()
    return

source_reservation = self.resource_manager._reservations.get(source_addr)
if source_reservation and not source_reservation.can_accept_connection():
    await self._send_status(stream, StatusCode.RESOURCE_LIMIT_EXCEEDED,
        "Source peer has exceeded its connection limit", ...)
    await stream.reset()
    return

To-Do

• Clean up commit history
• Add or update documentation related to these changes
• Add entry to the release notes

[acul71]

AI PR Review: #1343 — Fix Circuit Relay v2 connect destination peer without active reservation

PR: #1343
Author: @sumanjeet0012
Reviewed branch: relay-fix (checked out locally)
Related issue: #1342
Review date: 2026-06-05

---

1. Summary of Changes

This PR fixes a security-relevant bug in Circuit Relay v2 (libp2p/relay/circuit_v2/). In _handle_connect(), the relay admission gate checked the source peer's reservation (source_addr) instead of the destination peer's reservation (peer_id). That allowed a source with its own reservation to reach any relay-connected destination that had never sent RESERVE, bypassing the protocol's opt-in model described in #1342.

Changes:

File	Change
libp2p/relay/circuit_v2/protocol.py	Destination reservation enforced via can_accept_connection(peer_id=peer_id); source connection limits enforced separately; new NO_RESERVATION rejection path
libp2p/relay/circuit_v2/protocol_buffer.py	Adds StatusCode.NO_RESERVATION = 103
newsfragments/1342.bugfix.rst	User-facing release note
tests/core/relay/test_circuit_v2_transport.py	Integration test updated to have destination make a RESERVE before source dials

Architecture context: Affects the relay hop protocol handler (CircuitV2Protocol) and its interaction with RelayResourceManager reservation tracking. No breaking API changes to public host/transport interfaces.

Breaking changes: None for public APIs. Wire behavior change: CONNECT requests to destinations without reservations are now correctly rejected (previously incorrectly accepted in some cases).

---

2. Branch Sync Status and Merge Conflicts

Branch Sync Status

• Status: ℹ️ Ahead of origin/main (PR commits only; no divergence)
• Details: 0 3 — branch is 0 commits behind and 3 commits ahead of origin/main
  • 2 feature commits from author + 1 merge commit (Merge branch 'main' into relay-fix by @acul71)

Merge Conflict Analysis

✅ No merge conflicts detected. The PR branch merges cleanly into origin/main.

---

3. Strengths

• Correct root-cause fix: Swapping source_addr → peer_id in the destination admission check directly addresses the vulnerability described in #1342.
• Clear separation of concerns: Destination reservation requirement and source per-reservation connection limits are now distinct checks with appropriate status codes and messages.
• Semantically accurate status code: Adding NO_RESERVATION is aligned with the Circuit Relay v2 spec semantics (even though py-libp2p's numeric values differ from proto3 spec values — see Issues Found).
• Newsfragment present: newsfragments/1342.bugfix.rst correctly references issue #1342 with user-facing wording.
• Test fix exposes prior test gap: The transport integration test now performs a destination RESERVE step, reflecting real protocol usage (destination must opt in before being reachable).
• CI green: All GitHub Actions checks pass (core, lint, interop, docs, Windows matrix).
• Local validation: make lint, make typecheck, make test, and make linux-docs all pass on the checked-out branch.

---

4. Issues Found

Critical

None. The core fix is correct and addresses the reported vulnerability.

Major

• File: tests/core/relay/test_circuit_v2_transport.py

• Line(s): 377–390 (new reservation step); no corresponding negative test

• Issue: The test update makes the happy path protocol-correct but does not assert that CONNECT is rejected when the destination lacks a reservation. Before this PR, the integration test implicitly relied on the buggy behavior (no destination RESERVE yet dial succeeded). A dedicated negative test would lock in the fix and prevent regression.

• Suggestion: Add a test (in test_circuit_v2_protocol.py or test_circuit_v2_transport.py) that sends a CONNECT for a destination without reservation and asserts the relay responds with StatusCode.NO_RESERVATION and resets the stream.

• File: GitHub PR description

• Line(s): N/A

• Issue: Issue #1342 is referenced narratively ("Issue #1342") but the PR body does not use a closing keyword (Fixes #1342, Closes #1342, etc.). This prevents automatic issue closure on merge and is inconsistent with project convention.

• Suggestion: Add Fixes #1342 to the PR description.

Minor

• File: libp2p/relay/circuit_v2/protocol.py
• Line(s): 704–705
• Issue: Source connection limit check accesses the private attribute self.resource_manager._reservations directly instead of a public API on RelayResourceManager (which already exposes can_accept_connection, has_reservation, etc.).
• Suggestion: Add a small public helper (e.g. get_reservation(peer_id) -> Reservation | None or can_source_accept_connection(peer_id) -> bool) to avoid reaching into _reservations.

libp2p/relay/circuit_v2/protocol.py — lines 703–705

        # Separately enforce the source peer's per-reservation connection limit.
        source_reservation = self.resource_manager._reservations.get(source_addr)
        if source_reservation and not source_reservation.can_accept_connection():

• File: libp2p/relay/circuit_v2/pb/circuit.proto

• Line(s): 44–54

• Issue: NO_RESERVATION is added to the Python StatusCode IntEnum (103) but not to the protobuf Status.Code enum. create_status() casts the int directly onto the protobuf object, so this works at runtime, but the .proto definition remains out of sync with both the Python enum and the updated spec enum values (NO_RESERVATION = 204 in proto3). Pre-existing drift, but this PR introduces a new code only in the Python layer.

• Suggestion: Consider updating circuit.proto (and regenerating pb2) in a follow-up; document the numeric mapping if interop with go/rust/js implementations is a goal.

• File: PR author To-Do list (PR body)

• Line(s): N/A

• Issue: Author notes remaining tasks: commit history cleanup and documentation updates are unchecked.

• Suggestion: Squash or rebase commits before merge; no user-facing docs strictly required for this internal fix, but a brief note in relay module docstring or docs/examples.circuit_relay about destination reservation being mandatory would help.

---

5. Security Review

This PR closes a security vulnerability rather than introducing one.

• Risk (pre-fix): Unauthorized relayed delivery — any source peer with a reservation could reach destinations that never opted in via RESERVE, turning the relay into an unsolicited traffic vector.
• Impact (pre-fix): High — violates Circuit Relay v2 opt-in model; enables reaching peers without consent.
• Mitigation (this PR): Destination reservation is now enforced before CONNECT proceeds. ✅

Post-fix assessment:

• Risk: Residual — no negative test means regression could go unnoticed; status code numeric mismatch with other implementations could cause confusing interop errors (not a bypass).
• Impact: None from the fix itself.
• Mitigation: Add negative test; consider proto alignment for wire compatibility.

---

6. Documentation and Examples

Item	Status
Newsfragment (1342.bugfix.rst)	✅ Present, user-facing, ends with newline
Issue reference	⚠️ Referenced but no Fixes #1342 keyword
Module docstrings / API docs	❌ Not updated (acceptable for small bugfix; author flagged as TODO)
Examples (docs/examples.circuit_relay)	❌ Not updated — existing examples may already show reservation flow

No blocking documentation gaps for a targeted bugfix, but a negative-path test would serve as living documentation of the security invariant.

---

7. Newsfragment Requirement

✅ Satisfied.

• Issue linked: #1342 (referenced in PR body and commit messages)
• Newsfragment file: newsfragments/1342.bugfix.rst
• Type: bugfix — correct for this change
• Content: User-facing, describes impact without implementation detail
• Newline: Present

Non-blocker: Add Fixes #1342 to PR body for auto-close on merge.

---

8. Tests and Validation

New / Updated Tests

• test_circuit_v2_transport.py: Destination RESERVE step added before source dial — fixes the test to match correct protocol flow.
• Gap: No test for CONNECT rejection when destination has no reservation.

Linting (make lint)

• Exit code: 0 ✅
• Result: All pre-commit hooks passed (ruff, mypy-local, pyrefly-local, mdformat, path audit, etc.)
• Errors/Warnings: None

Type Checking (make typecheck)

• Exit code: 0 ✅
• Result: mypy and pyrefly passed
• Errors/Warnings: None

Test Execution (make test)

• Exit code: 0 ✅
• Summary: 2795 passed, 16 skipped in ~105s
• Failures/Errors: None
• Notable warnings: Standard pytest duration report only; no relay-related failures

Documentation Build (make linux-docs)

• Exit code: 0 ✅
• Result: Sphinx HTML build succeeded (104 sources, -W warnings-as-errors mode)
• Errors/Warnings: None fatal

CI (GitHub Actions)

All checks pass including tox (3.10–3.13, core/lint/interop/docs), Windows matrix, and Read the Docs preview.

---

9. Recommendations for Improvement

1. Add a negative unit/integration test asserting NO_RESERVATION when destination has no active reservation — highest-value follow-up.
2. Add Fixes #1342 to the PR description.
3. Replace _reservations direct access with a public RelayResourceManager method.
4. Clean up commit history (squash the two fix commits; drop or clarify the merge commit) per author's own TODO.
5. Optional follow-up: Align circuit.proto Status enum with spec (including NO_RESERVATION) for interop clarity.

---

10. Questions for the Author

1. Was the absence of a negative test intentional, or would you add one asserting StatusCode.NO_RESERVATION before merge?
2. Should source peers without a reservation be allowed to initiate CONNECT (current behavior after this PR), or should the relay require both source and destination reservations? The pre-fix code accidentally required source reservation; this PR only mandates destination reservation per spec.
3. Is NO_RESERVATION = 103 the intended wire value for py-libp2p interop, given other implementations may use 204 per the proto3 spec update?

---

11. Overall Assessment

Metric	Rating
Quality Rating	Good
Security Impact	High (positive) — fixes a real opt-in bypass
Merge Readiness	Needs fixes — minor: negative test + PR body keyword; non-blocking: commit cleanup
Confidence	High

Summary: This is a focused, correct fix for an important Circuit Relay v2 security bug (#1342). The logic change in _handle_connect() is sound, the newsfragment is in place, and all local/CI validation passes. The main gap is test coverage for the rejection path — the updated integration test would still pass even if the destination check were reverted to source_addr, so a negative test is strongly recommended before merge. With that addition and a Fixes #1342 line in the PR body, this should be ready to merge.