fix(sandbox): strip query string from L7 path matching#614
Open
johntmyers wants to merge 1 commit intomainfrom
Open
fix(sandbox): strip query string from L7 path matching#614johntmyers wants to merge 1 commit intomainfrom
johntmyers wants to merge 1 commit intomainfrom
Conversation
Previously, L7 policy path rules received the full request URI including the query string. This caused exact path rules like `path: /api/v1/download` to silently fail when clients added query parameters (e.g., `/api/v1/download?slug=foo`), because Rego's glob.match treats the query string as part of the last path segment. This fix splits the request target into path and query components during HTTP parsing. Path rules now match only the path component, and query strings are passed through transparently to the upstream server. This matches user expectations: a path rule controls which endpoints are reachable, not which query parameters are allowed. Changes: - Add `query` field to L7Request and L7RequestInfo structs - Split path/query in parse_http_request before L7 policy evaluation - Pass query string to Rego input for future query param filtering - Add l7_query field to L7_REQUEST log output - Add tests for query string splitting and path matching with query params - Document path matching behavior in policy-schema.md Refs: /discussions/607
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fix L7 policy path matching to ignore query strings, so exact path rules like
path: /api/v1/downloadcorrectly match requests with query parameters (e.g.,/api/v1/download?slug=foo&version=1.0).Related Issue
Refs: #607
Changes
queryfield toL7RequestandL7RequestInfostructs to hold the query string separately from the pathparse_http_requestl7_queryfield to L7_REQUEST log output for observabilityTesting
Smoke tested locally with httpbin.org:
GET /getGET /get?foo=bar&baz=quxGET /headersGET /anythingGET /anything?test=1mise run pre-commitpasses (excluding pre-existing license issues)Checklist