-
Notifications
You must be signed in to change notification settings - Fork 514
[Bugfix][Router] Log request headers as a lazy argument so they can be redacted #1097
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,61 @@ | ||
| """The session-extraction debug line must not defeat TokenRedactionFilter. | ||
|
|
||
| TokenRedactionFilter only inspects ``record.args`` for a Starlette ``Headers`` | ||
| object, so a call site that formats the headers into the message string leaves | ||
| nothing for it to redact. | ||
| """ | ||
|
|
||
| import logging | ||
|
|
||
| from starlette.datastructures import Headers | ||
|
|
||
| from vllm_router import log | ||
|
|
||
|
|
||
| def test_formatted_headers_are_not_redacted_but_a_lazy_arg_is(): | ||
| redaction = log.TokenRedactionFilter() | ||
| headers = Headers( | ||
| {"authorization": "Bearer super-secret-token", "host": "example.invalid"} | ||
| ) | ||
|
|
||
| formatted = logging.LogRecord( | ||
| name="test", | ||
| level=logging.DEBUG, | ||
| pathname="test.py", | ||
| lineno=1, | ||
| msg=f"Debug session extraction - Request headers: {dict(headers)}", | ||
| args=None, | ||
| exc_info=None, | ||
| ) | ||
| redaction.filter(formatted) | ||
| assert "super-secret-token" in formatted.getMessage(), ( | ||
| "a pre-formatted dict carries nothing in record.args, so the filter " | ||
| "has nothing to act on; this is what the call site must avoid" | ||
| ) | ||
|
|
||
| lazy = logging.LogRecord( | ||
| name="test", | ||
| level=logging.DEBUG, | ||
| pathname="test.py", | ||
| lineno=1, | ||
| msg="Debug session extraction - Request headers: %s", | ||
| args=(headers,), | ||
| exc_info=None, | ||
| ) | ||
| redaction.filter(lazy) | ||
| assert "super-secret-token" not in lazy.getMessage() | ||
| assert "Bearer ****" in lazy.getMessage() | ||
|
|
||
|
|
||
| def test_session_extraction_logs_headers_as_a_lazy_arg(): | ||
| """The call site itself, read from source, must pass the Headers object.""" | ||
| import inspect | ||
|
|
||
| from vllm_router.services.request_service import request as request_module | ||
|
|
||
| source = inspect.getsource(request_module.route_general_request) | ||
| assert 'logger.debug("Debug session extraction - Request headers: %s"' in source, ( | ||
| "the headers must reach the logger as a lazy argument, not formatted " | ||
| "into the message, or TokenRedactionFilter cannot redact them" | ||
| ) | ||
| assert "Request headers: {dict(request.headers)}" not in source | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -607,7 +607,10 @@ async def route_general_request( | |
| f"Debug session extraction - Router type: {type(request.app.state.router).__name__}" | ||
| ) | ||
| logger.debug(f"Debug session extraction - Session key config: {session_key}") | ||
| logger.debug(f"Debug session extraction - Request headers: {dict(request.headers)}") | ||
| # Pass the Headers object as a lazy argument rather than a formatted dict, | ||
| # so TokenRedactionFilter can find it in record.args and redact | ||
| # Authorization and Cookie before this line is emitted. | ||
| logger.debug("Debug session extraction - Request headers: %s", request.headers) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Passing |
||
| logger.debug(f"Debug session extraction - Extracted session ID: {session_id}") | ||
|
|
||
| logger.info( | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using
inspect.getsourceand string matching to assert on the implementation details ofroute_general_requestis highly fragile. This test will fail if:\n1. The log message is formatted with single quotes instead of double quotes.\n2. A code formatter (likeblackorruff) wraps the line differently.\n3. The source code is not available (e.g., when running from compiled.pycfiles or in certain CI/CD environments).\n\nInstead of inspecting the source code, a more robust approach is to mock the logger or usepytest'scaplogfixture to verify that the logged message is actually redacted whenroute_general_requestis executed. If invokingroute_general_requestis too complex due to dependencies, we can at least make the string matching more flexible (e.g., using regular expressions or AST parsing via theastmodule) to avoid failing on simple formatting changes.