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.

[acul71]

@sumanjeet0012
This is a focused, correct fix for an important Circuit Relay v2 security bug (#1342). The author responded thoroughly to @acul71's review: negative regression test, public get_reservation() API, spec-aligned status code 204, and protobuf enum update. All CI checks pass. The remaining concern is protobuf gencode regeneration that downgrades from 6.32.1 to 4.25.1 — this should be corrected to match the rest of the repository before merge. Unrelated QUIC and redis changes should ideally be cleaned up in commit history. Once the protobuf tooling issue is resolved and @acul71 re-approves, this PR should be ready to merge.

FULL AI REVIEW:

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 at bf536027)
Related issue: #1342
Review date: 2026-06-14
Prior review: AI-PR-REVIEW-1343-0.md (2026-06-05, before maintainer feedback and follow-up commits)

---

1. Summary of Changes

This PR fixes a security-relevant bug in Circuit Relay v2 (libp2p/relay/circuit_v2/). In CircuitV2Protocol._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.

Fixes #1342

Since the initial review (2026-06-05), the author addressed @acul71's requested changes with three additional commits:

File	Change
libp2p/relay/circuit_v2/protocol.py	Destination reservation enforced via can_accept_connection(peer_id=peer_id); source limits checked separately via get_reservation(source_addr); NO_RESERVATION rejection path; internal call sites migrated from _reservations.get to get_reservation
libp2p/relay/circuit_v2/protocol_buffer.py	StatusCode.NO_RESERVATION = 204 (spec-aligned)
libp2p/relay/circuit_v2/pb/circuit.proto	Adds NO_RESERVATION = 204 to protobuf enum
libp2p/relay/circuit_v2/pb/circuit_pb2.py / .pyi	Regenerated from updated .proto
libp2p/relay/circuit_v2/resources.py	New public get_reservation(peer_id) -> Reservation | None helper
newsfragments/1342.bugfix.rst	User-facing release note
tests/core/relay/test_circuit_v2_protocol.py	New negative test: CONNECT without destination reservation returns NO_RESERVATION
tests/core/relay/test_circuit_v2_transport.py	Happy-path test updated: destination RESERVE before source dial
libp2p/transport/quic/transport.py	Unrelated logger formatting fixes
tests/interop/perf/test_perf_test_helpers.py	Unrelated redis mock for perf test import

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

Wire behavior change: CONNECT requests to destinations without active reservations are now correctly rejected with NO_RESERVATION (204). Previously, a source with its own reservation could bypass destination opt-in.

---

2. Branch Sync Status and Merge Conflicts

Branch Sync Status

• Status: ℹ️ Ahead of origin/main (no divergence)
• Details: 0 6 — branch is 0 commits behind and 6 commits ahead of origin/main
  • 4 author commits + 1 merge commit (Merge branch 'main' into relay-fix by @acul71) + 1 additional author commit

Merge Conflict Analysis

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

---

3. Strengths

• Correct root-cause fix: Destination admission now checks peer_id (destination) instead of source_addr, directly closing the opt-in bypass in Circuit Relay: Source peer can connect to destination peer without destination peer's reservation #1342.
• Maintainer feedback addressed: The June 14 follow-up commits resolve all substantive items from @acul71's review (negative test, get_reservation API, spec-aligned NO_RESERVATION = 204, protobuf enum update, Fixes #1342 in PR body).
• Clear separation of concerns: Destination reservation requirement and source per-reservation connection limits are distinct checks with appropriate status codes and messages.
• Regression test locks in the fix: test_circuit_v2_connect_fails_without_reservation asserts StatusCode.NO_RESERVATION (204) and stream reset — the integration test alone would not catch a revert to checking source_addr.
• Newsfragment present: newsfragments/1342.bugfix.rst correctly references issue Circuit Relay: Source peer can connect to destination peer without destination peer's reservation #1342 with user-facing wording.
• CI green: All GitHub Actions checks pass (core, lint, interop, docs, Windows matrix) on the latest push.
• Local lint/typecheck: make lint and make typecheck pass with no errors or warnings.

---

4. Issues Found

Critical

None. The core security fix is correct and the negative test prevents silent regression.

Major

• File: libp2p/relay/circuit_v2/pb/circuit_pb2.py
• Line(s): 1–19
• Issue: Protobuf regeneration downgraded the checked-in gencode from Protobuf Python 6.32.1 (on main, with _runtime_version.ValidateProtobufRuntimeVersion) to 4.25.1 (without runtime version validation). This creates inconsistency with every other _pb2.py file on main and removes the project's runtime version guard for this module.
• Suggestion: Regenerate circuit_pb2.py / circuit_pb2.pyi using the same protoc / protobuf toolchain as the rest of the repo (matching main's 6.32.1 output), changing only the NO_RESERVATION enum addition. Alternatively, manually add only the enum delta to the existing 6.x gencode without a full downgrade.

libp2p/relay/circuit_v2/pb/circuit_pb2.py — lines 1–19

# -*- coding: utf-8 -*-
# Generated by the protocol buffer compiler.  DO NOT EDIT!
# source: libp2p/relay/circuit_v2/pb/circuit.proto
# Protobuf Python Version: 4.25.1
"""Generated protocol buffer code."""
from google.protobuf import descriptor as _descriptor
from google.protobuf import descriptor_pool as _descriptor_pool
from google.protobuf import symbol_database as _symbol_database
from google.protobuf.internal import builder as _builder
# @@protoc_insertion_point(imports)

Compare with main (still 6.32.1 with ValidateProtobufRuntimeVersion).

Minor

• File: PR commit history / scope

• Line(s): N/A

• Issue: The PR bundles unrelated fixes (libp2p/transport/quic/transport.py logger changes, tests/interop/perf/test_perf_test_helpers.py redis mock) alongside the relay security fix. These complicate review and bisection.

• Suggestion: Squash or split before merge; keep relay fix commits focused. The author's own TODO ("Clean up commit history") remains unchecked.

• File: GitHub PR description

• Line(s): N/A

• Issue: PR body still describes NO_RESERVATION = 103 in the "How was it fixed?" section, but the implementation now correctly uses 204 per spec.

• Suggestion: Update the PR description to reflect the final status code value.

• File: libp2p/relay/circuit_v2/resources.py

• Line(s): 449–465

• Issue: RelayResourceManager.can_accept_connection() still reads _reservations directly while new call sites use get_reservation(). Behavior is equivalent (expiry is checked inside Reservation.can_accept_connection()), but the API surface is slightly inconsistent.

• Suggestion: Optionally refactor can_accept_connection() to delegate to get_reservation() for a single code path.

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

• Line(s): N/A

• Issue: "Add or update documentation related to these changes" remains unchecked. No module doc or example updates were added (acceptable for a targeted bugfix, but the checkbox is stale).

• Suggestion: Mark complete or add a brief note in docs/examples.circuit_relay that destination reservation is mandatory before CONNECT.

---

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 enforced before CONNECT proceeds. Negative test prevents regression. ✅

Post-fix assessment:

• Risk: None identified in the fix itself. Residual process risk: protobuf gencode downgrade could mask version mismatches at import time for this module only.
• Impact: None from the security fix.
• Mitigation: Regenerate protobuf with the project's standard toolchain; maintainer re-review after protobuf fix.

---

6. Documentation and Examples

Item	Status
Newsfragment (1342.bugfix.rst)	✅ Present, user-facing, ends with newline
Issue reference (Fixes #1342)	✅ Present in PR body
Protobuf spec alignment (NO_RESERVATION = 204)	✅ Updated in .proto, protocol_buffer.py, and tests
Module docstrings / API docs	⚠️ get_reservation() has a docstring; no broader relay docs updated
Examples (docs/examples.circuit_relay)	⚠️ Not updated — existing examples may already show reservation flow

No blocking documentation gaps for a targeted security bugfix.

---

7. Newsfragment Requirement

✅ Satisfied.

• Issue linked: Circuit Relay: Source peer can connect to destination peer without destination peer's reservation #1342 (Fixes #1342 in PR body; auto-close configured)
• Newsfragment file: newsfragments/1342.bugfix.rst
• Type: bugfix — correct
• Content: User-facing, describes impact without implementation detail
• Newline: Present

---

8. Tests and Validation

New / Updated Tests

• test_circuit_v2_connect_fails_without_reservation — asserts NO_RESERVATION (204) when destination has no reservation ✅
• test_circuit_v2_transport.py — destination RESERVE step added to happy-path integration test ✅
• Gap (non-blocking): No test for source RESOURCE_LIMIT_EXCEEDED when source reservation slots are exhausted

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: 1 (local) — 1 failure unrelated to this PR
• Summary: 2795 passed, 1 failed, 16 skipped in ~112s
• Failed test: tests/core/network/test_connection_management.py::TestExtractIpFromMultiaddr::test_invalid_dns
  • Error: AssertionError: assert ['127.0.0.1'] == [] — local DNS resolved an "invalid" hostname to 127.0.0.1
  • Assessment: Environment-specific / flaky; not introduced by this PR (no changes in test_connection_management.py or connection management code). CI core matrix passes on the PR branch.
• Relay tests: test_circuit_v2_connect_fails_without_reservation and updated transport test pass locally.

CI (GitHub Actions)

All checks pass on the latest push, including tox (3.10–3.13, core/lint/interop/docs) and Windows matrix.

Maintainer Feedback Status

@acul71 requested changes on 2026-06-05. The author force-pushed follow-up commits on 2026-06-14 addressing:

Requested change	Status
Negative test for NO_RESERVATION	✅ Addressed
Fixes #1342 in PR body	✅ Addressed
Public get_reservation() instead of _reservations access	✅ Addressed
Spec-aligned NO_RESERVATION = 204 in protobuf	✅ Addressed (with protobuf version concern noted above)
Commit history cleanup	⚠️ Still 6 commits; unrelated QUIC/redis changes included

Awaiting re-review from @acul71.

---

9. Recommendations for Improvement

1. Regenerate protobuf gencode with the project's standard protoc/protobuf version (6.x, matching main) — only add the NO_RESERVATION enum; do not downgrade runtime version validation.
2. Squash or split commits before merge: isolate relay security fix from QUIC logger and redis mock changes.
3. Update PR description to reflect NO_RESERVATION = 204 (not 103).
4. Optional: Add a test for source RESOURCE_LIMIT_EXCEEDED when source reservation slots are full.
5. Optional: Refactor RelayResourceManager.can_accept_connection() to use get_reservation() internally.

---

10. Questions for the Author

1. Was the protobuf gencode regenerated with a local protoc 4.x install? Can you regenerate with the same toolchain as main (6.32.1) to avoid removing _runtime_version.ValidateProtobufRuntimeVersion?
2. Should the QUIC logger fix and redis mock be moved to a separate PR, or squashed into a clearly labeled "CI/misc" commit?
3. Is requiring only destination reservation (not source) the intended long-term behavior? This matches the Circuit Relay v2 spec, but differs from the pre-fix accidental behavior.

---

11. Overall Assessment

Metric	Rating
Quality Rating	Good
Security Impact	High (positive) — fixes a real opt-in bypass
Merge Readiness	Needs re-review — substantive maintainer feedback addressed; protobuf gencode downgrade should be fixed before merge
Confidence	High

Summary: This is a focused, correct fix for an important Circuit Relay v2 security bug (#1342). The author responded thoroughly to @acul71's review: negative regression test, public get_reservation() API, spec-aligned status code 204, and protobuf enum update. All CI checks pass. The remaining concern is protobuf gencode regeneration that downgrades from 6.32.1 to 4.25.1 — this should be corrected to match the rest of the repository before merge. Unrelated QUIC and redis changes should ideally be cleaned up in commit history. Once the protobuf tooling issue is resolved and @acul71 re-approves, this PR should be ready to merge.