Fix: Strip query string from pctx.Path in ext_proc and ext_authz

[JoshSag]

Fixes #881.

The problem

pipeline.Context.Path diverged by listener mode: the proxies parse the request
target (r.URL.Path — query-free, percent-decoded), while ext_proc and ext_authz
passed the raw target through, query string included. Full table, timeline, and
impact in #881.

The change

Run the raw request target through url.ParseRequestURI — the same parser net/http
runs for the proxy listeners — at pctx construction in both Envoy-fed listeners
(shared helper httpx.PathOnly). Path is now identical across all four listener
modes, decoding included: /api/hello%20world?x=1 yields /api/hello world
everywhere. Targets that parser rejects (which net/http answers with 400 before any
proxy-mode pipeline runs) keep a plain query-strip fallback — no worse than today.

Two behavior notes for Envoy-mode deployments, both alignments with proxy-sidecar's
longstanding semantics:

• Decoding affects matching: /%68ealthz now matches a /healthz bypass/policy
  pattern, as it always has under the proxies.

• The raw query — which routinely carries tokens and secrets — no longer reaches the
  ibac judge LLM; the flip side is that policy loses query visibility entirely until
  the Query field lands (follow-up below).

• ext_proc: Path: httpx.PathOnly(getHeader(headers, ":path")) at all four
  construction sites.

• ext_authz: path := httpx.PathOnly(httpReq.GetPath()).

• pipeline.Context.Path doc comment now states the invariant, so plugins may
  match/log/policy-feed Path without stripping a query themselves.

• inference-parser's one-line defensive strip is kept (defense in depth for
  contexts constructed outside a listener; the failure mode it guards is silent),
  with its comment updated to reflect the new invariant. The other existing strips
  (bypass matcher, tool-prune) are untouched.

No behavior change for the forward/reverse proxy listeners.

Evidence

New table-driven tests TestExtProc_PathMatchesProxyListeners and
TestCheck_PathMatchesProxyListeners drive each fixed listener through a capture
plugin and pin all three behaviors per listener — query strip (/api/x?secret=1),
percent-decoding (/api/hello%20world?secret=1&b=2), and the unparseable-target
fallback (/a%zz?secret=1) — asserting pctx.Path holds exactly what the proxy
listeners produce for the same wire bytes.

On main (before):

--- FAIL: TestExtProc_PathMatchesProxyListeners
    inbound  pctx.Path = ["/api/x?secret=1"], want ["/api/x"]
    outbound pctx.Path = ["/api/hello%20world?secret=1&b=2"], want ["/api/hello world"]
--- FAIL: TestCheck_PathMatchesProxyListeners   (ext_authz)
    inbound  pctx.Path = ["/api/x?secret=1"], want [/api/x]
    outbound pctx.Path = ["/api/x?secret=1"], want [/api/x]

On this branch: both pass, and the same capture-plugin probe run against the forward
and reverse proxy listeners confirms they already produced these values. Full
go vet ./... && go test -count=1 -race ./listener/... ./pipeline/... ./plugins/...
green.

Proposed follow-up (not in this PR)

Plugins that legitimately need query parameters currently have no channel for them —
the proxies drop the query on the floor. A follow-up could add a Query string field
to pipeline.Context, populated by all four listeners (from r.URL.RawQuery / the
request target), so query-aware plugins opt in explicitly instead of parsing Path.
This PR deliberately only restores the invariant; adding the field is a separate,
additive decision.

Assisted-By: Claude (Anthropic AI) noreply@anthropic.com

Summary by CodeRabbit

• Bug Fixes

  • Request paths now consistently exclude query strings across authorization and external-processing listeners.
  • Percent-encoded URL paths are decoded consistently before being passed through request pipelines.
  • Unparseable request targets retain their query-stripped path.
  • Inference parsing now handles listener-provided paths reliably, including requests containing query parameters.

• Documentation

  • Clarified pipeline path behavior and documented normalization and fallback handling.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Team

Run ID: 2af51398-ca04-4707-b519-2bfe7c5882e7

📥 Commits

Reviewing files that changed from the base of the PR and between 6e1e66b and 29d8be3.

📒 Files selected for processing (6)

• authbridge/authlib/listener/extauthz/server.go
• authbridge/authlib/listener/extauthz/server_path_test.go
• authbridge/authlib/listener/extproc/server.go
• authbridge/authlib/listener/extproc/server_path_test.go
• authbridge/authlib/listener/httpx/path.go
• authbridge/authlib/pipeline/context.go

🚧 Files skipped from review as they are similar to previous changes (1)

• authbridge/authlib/pipeline/context.go

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.

---

📝 Walkthrough

Walkthrough

Ext_authz and ext_proc now populate pipeline.Context.Path with the decoded URL path without its query string. Shared parsing handles invalid targets with a query-strip fallback. Tests cover inbound and outbound paths.

Changes

Path normalization

Layer / File(s)	Summary
Shared path normalization contract authbridge/authlib/listener/httpx/path.go, authbridge/authlib/pipeline/context.go	httpx.PathOnly parses and decodes request targets, removes queries, and falls back to plain query stripping for invalid targets. Context.Path documentation describes this contract.
Ext_authz path normalization and tests authbridge/authlib/listener/extauthz/server.go, authbridge/authlib/listener/extauthz/server_path_test.go	Check uses httpx.PathOnly. Tests cover inbound and outbound pipelines, query removal, percent-decoding, and invalid-target fallback.
Ext_proc path normalization and tests authbridge/authlib/listener/extproc/server.go, authbridge/authlib/listener/extproc/server_path_test.go, authbridge/authlib/plugins/inferenceparser/plugin.go	All four request handlers normalize :path. Tests cover the same path cases. Inference parser documentation describes query-free listener paths.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to 29d8b

Ext_authz and ext_proc now provide query-free, percent-decoded paths consistently across listener modes, with malformed targets retaining the documented query-strip fallback. Current coverage exercises the changed behaviors and no merge-blocking risk remains.

Suggested reviewers: huang195, kellyaa

Sequence Diagram(s)

sequenceDiagram
  participant EnvoyOrAuthRequest
  participant Listener
  participant httpxPathOnly
  participant Pipeline
  EnvoyOrAuthRequest->>Listener: provide request target
  Listener->>httpxPathOnly: normalize target
  httpxPathOnly-->>Listener: decoded path without query
  Listener->>Pipeline: store Context.Path

Loading

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 7 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	The changes satisfy issue #881. ext_proc and ext_authz now use shared httpx.PathOnly normalization, which removes queries, decodes percent encoding, and preserves a fallback for unparseable targets. T…
Out of Scope Changes check	✅ Passed	The changes remain within scope. The shared helper, listener updates, documentation, and tests directly support consistent pipeline.Context.Path behavior. No unrelated code changes are present.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: removing query strings from pctx.Path in both ext_proc and ext_authz.

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[abigailgold]

1. Suggestion: Now that the query-free invariant is guaranteed at the listener level, consider a follow-up to either remove the now-redundant defensive strips in toolprune.pathOnly and bypass.Matcher.Match, or consolidate all three pathOnly-shaped helpers (new extproc/extauthz ones plus the two pre-existing ones) behind one shared implementation, so their edge-case behavior (trailing slash, fragment handling) can't drift independently the way the original bug did.
2. Observation (not a fix needed): The security impact on ibac's judge prompt (§3.5) is, in my assessment, the most important consequence of this fix and is somewhat underweighted relative to the functional-mismatch framing in the PR body — worth the reviewers' explicit attention when discussing the PR, even though no additional code change is required.

[JoshSag]

@abigailgold Thanks — both points acted on.

1. Done for the two new helpers: 29d8be3a (pushed) hoists them into a shared
   httpx.PathOnly, so the pair defining the invariant can't drift. The pre-existing
   plugin strips I'd leave to the same follow-up as the Query field, with one
   constraint: plugin code sees the already-decoded pctx.Path, so it must never re-run
   the parser (that double-decodes) — per site the choice is delete-as-redundant or
   keep-as-plain-strip. bypass.Matcher's strip stays either way; it's canonicalization,
   not a workaround.

2. Agreed — the body's behavior-notes bullet now leads with the judge-LLM leak fix.

Assisted-By: Claude (Anthropic AI) noreply@anthropic.com