Skip to content

fix(acp): forward allowlisted client headers on all ACP paths - #384

Open
rehmanmuradali wants to merge 2 commits into
mainfrom
fix/acp-forward-client-headers
Open

fix(acp): forward allowlisted client headers on all ACP paths#384
rehmanmuradali wants to merge 2 commits into
mainfrom
fix/acp-forward-client-headers

Conversation

@rehmanmuradali

@rehmanmuradali rehmanmuradali commented Jul 29, 2026

Copy link
Copy Markdown

Summary

  • get_headers(agent) is called from several ACP paths (send_message_stream, create_task, cancel/interrupt) without threading the inbound client headers.
  • As a result filter_request_headers(None) returned {}, so no allowlisted client x-* headers ever reached the agent.
  • Fall back to the inbound request headers when callers don't pass them, so the existing allowlist actually forwards them.

Test plan

  • Trigger an ACP path that previously dropped headers (e.g. send_message_stream) and confirm allowlisted client x-* headers now reach the agent.
  • Confirm existing behavior is preserved when request_headers is explicitly passed.

Made with Cursor

Greptile Summary

This PR fixes a header-forwarding gap in the ACP service: get_headers(agent) was called without request_headers on several code paths (send_message_stream, create_task, cancel/interrupt), causing filter_request_headers(None) to return {} and silently drop all allowlisted client x-* headers.

  • Core fix (agent_acp_service.py): When request_headers is None, fall back to dict(self._request.headers) before filtering, so the existing security allowlist (x-* prefix, BLOCKED_HEADERS exclusion) now correctly forwards headers on all ACP code paths.
  • Tests (test_agent_acp_service.py): Two new unit tests cover the fallback case (no explicit headers → inbound x-* forwarded, credentials blocked) and the explicit-empty-dict case (no fallback triggered, only x-request-id remains).

Confidence Score: 5/5

Safe to merge — minimal, well-targeted fallback that activates the already-existing security filter on code paths that previously bypassed it entirely.

The fix is four lines in one function, backed by two new unit tests that verify both the fallback and the explicit-override cases. The security allowlist was already correct — the only gap was that it was never reached when callers omitted request_headers. No new untested code paths are introduced.

Files Needing Attention: No files require special attention.

Important Files Changed

Filename Overview
agentex/src/domain/services/agent_acp_service.py Four-line fix in get_headers: falls back to dict(self._request.headers) when request_headers is None, preserving existing security filtering via filter_request_headers.
agentex/tests/unit/services/test_agent_acp_service.py Two new tests covering the fallback-to-inbound-headers path and the explicit-empty-dict (no-fallback) path; assertions correctly check credential blocking and allowlist behavior.

Sequence Diagram

sequenceDiagram
    participant Caller as ACP Caller
    participant GH as get_headers(agent)
    participant FR as filter_request_headers()
    participant RQ as self._request.headers

    Caller->>GH: get_headers(agent) [no request_headers]
    Note over GH: request_headers is None
    GH->>RQ: dict(self._request.headers)
    RQ-->>GH: raw inbound headers
    GH->>FR: filter_request_headers(inbound_headers)
    Note over FR: Allow x-* only, block credentials
    FR-->>GH: "safe x-* subset"
    GH->>GH: merge filtered + delegation + auth headers
    GH-->>Caller: final headers dict
Loading

Reviews (3): Last reviewed commit: "test(acp): cover get_headers inbound-hea..." | Re-trigger Greptile

Copilot AI review requested due to automatic review settings July 29, 2026 12:19
@rehmanmuradali
rehmanmuradali requested a review from a team as a code owner July 29, 2026 12:19

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes ACP header propagation by ensuring allowlisted client x-* headers are forwarded to agent ACP servers on ACP paths where get_headers(agent) was previously called without explicit request_headers.

Changes:

  • Add a fallback in AgentACPService.get_headers() to use inbound Request.headers when request_headers is not provided.
  • Preserve existing behavior when request_headers is explicitly passed (including opting out by passing {}).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread agentex/src/domain/services/agent_acp_service.py
@rehmanmuradali
rehmanmuradali marked this pull request as draft July 29, 2026 12:22
@rehmanmuradali
rehmanmuradali marked this pull request as ready for review July 29, 2026 12:26
rehmanmuradali and others added 2 commits July 30, 2026 10:23
Several ACP paths call get_headers(agent) without threading the inbound
client headers, so filter_request_headers(None) returned {} and no
allowlisted client x-* headers reached the agent. Fall back to the
inbound request headers so the existing allowlist forwards them.

Co-authored-by: Cursor <cursoragent@cursor.com>
Assert that omitting request_headers forwards allowlisted inbound x-*
headers (dropping sensitive ones), and that passing {} forwards none.

Co-authored-by: Cursor <cursoragent@cursor.com>
@rehmanmuradali
rehmanmuradali force-pushed the fix/acp-forward-client-headers branch from 90ddd41 to 775527b Compare July 30, 2026 06:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants