Skip to content

Commit a60d35a

Browse files
traskCopilot
andcommitted
Detect query strings leaked into server URL paths
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
1 parent 33ecbfd commit a60d35a

3 files changed

Lines changed: 93 additions & 1 deletion

File tree

‎tools/http/runner/README.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,8 @@ given.
3434
The shared server contract uses `/users/456` only for its query request. The
3535
HTTP policy uses that path to report a missing conditionally required
3636
`url.query` attribute without flagging the query-free `/users/123` request.
37+
It matches the path before any `?` and separately reports query strings
38+
included in `url.path`.
3739

3840
The span-name policy checks the HTTP method token separately from its target.
3941
When a server span name has a target but no `http.route`, it reports

‎tools/http/runner/src/http_conformance/policies/http_span_validation.rego‎

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,10 +82,28 @@ deny contains _http_span_finding(
8282
),
8383
) if {
8484
_http_span_kind(input.sample.span) == "server"
85-
_http_attr_value(input.sample.span, "url.path") == "/users/456"
85+
path := _http_attr_value(input.sample.span, "url.path")
86+
is_string(path)
87+
split(path, "?")[0] == "/users/456"
8688
not _http_has_attr(input.sample.span, "url.query")
8789
}
8890

91+
deny contains _http_span_finding(
92+
"http_url_path_format",
93+
"violation",
94+
input.sample.span,
95+
{"attribute_key": "url.path", "kind": "server"},
96+
sprintf(
97+
"Server span '%v' includes a query string in 'url.path'; query strings belong in 'url.query'.",
98+
[input.sample.span.name],
99+
),
100+
) if {
101+
_http_span_kind(input.sample.span) == "server"
102+
path := _http_attr_value(input.sample.span, "url.path")
103+
is_string(path)
104+
contains(path, "?")
105+
}
106+
89107
deny contains _http_span_finding(
90108
"recommended_attribute_not_present",
91109
"violation",

‎tools/http/runner/tests/test_http_policy.py‎

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
_HTTP_POLICY_IDS = {
2222
"http_route_not_present",
2323
"http_span_name_format",
24+
"http_url_path_format",
2425
"required_attribute_not_present",
2526
}
2627

@@ -147,6 +148,37 @@ def policy_advice(
147148
path=_QUERY_PATH,
148149
query=_QUERY_STRING,
149150
),
151+
_server_span(
152+
"GET /users/{leaked_query}",
153+
route="/users/{leaked_query}",
154+
path=f"{_QUERY_PATH}?{_QUERY_STRING}",
155+
),
156+
_server_span(
157+
"GET /users/{leaked_with_query}",
158+
route="/users/{leaked_with_query}",
159+
path=f"{_QUERY_PATH}?{_QUERY_STRING}",
160+
query=_QUERY_STRING,
161+
),
162+
_server_span(
163+
"GET /users/{query_free}",
164+
route="/users/{query_free}",
165+
path="/users/123",
166+
),
167+
_server_span(
168+
"GET /users/{prefix_match}",
169+
route="/users/{prefix_match}",
170+
path=f"{_QUERY_PATH}7",
171+
),
172+
_server_span(
173+
"GET /users/{encoded_question_mark}",
174+
route="/users/{encoded_question_mark}",
175+
path=f"{_QUERY_PATH}%3Fname",
176+
),
177+
_server_span(
178+
"GET /users/{other_query}",
179+
route="/users/{other_query}",
180+
path=f"/users/123?{_QUERY_STRING}",
181+
),
150182
]
151183
),
152184
encoding="utf-8",
@@ -281,3 +313,43 @@ def test_contract_query_request_with_url_query_has_no_finding(
281313
policy_advice: dict[tuple[str, str], dict[str, dict[str, Any]]],
282314
) -> None:
283315
assert policy_advice[("server", "GET /users/{with_query}")] == {}
316+
317+
318+
@pytest.mark.parametrize(
319+
("target", "expected"),
320+
[
321+
(
322+
"leaked_query",
323+
{"required_attribute_not_present", "http_url_path_format"},
324+
),
325+
("leaked_with_query", {"http_url_path_format"}),
326+
("other_query", {"http_url_path_format"}),
327+
],
328+
)
329+
def test_query_string_in_url_path_reports_a_leak(
330+
policy_advice: dict[tuple[str, str], dict[str, dict[str, Any]]],
331+
target: str,
332+
expected: set[str],
333+
) -> None:
334+
advice = policy_advice[("server", f"GET /users/{{{target}}}")]
335+
336+
assert set(advice) == expected
337+
assert advice["http_url_path_format"]["context"] == {
338+
"attribute_key": "url.path",
339+
"kind": "server",
340+
}
341+
if "required_attribute_not_present" in expected:
342+
assert advice["required_attribute_not_present"]["context"] == {
343+
"attribute_key": "url.query",
344+
"kind": "server",
345+
}
346+
347+
348+
@pytest.mark.parametrize(
349+
"target", ["query_free", "prefix_match", "encoded_question_mark"]
350+
)
351+
def test_other_paths_do_not_require_url_query(
352+
policy_advice: dict[tuple[str, str], dict[str, dict[str, Any]]],
353+
target: str,
354+
) -> None:
355+
assert policy_advice[("server", f"GET /users/{{{target}}}")] == {}

0 commit comments

Comments
 (0)