Repository navigation
fix: redact binary payloads in debug request logging - #154
Merged
Merged
Conversation
Uploaded file bytes (and other raw content) were dumped verbatim by the DEBUG "Request options" log line. Harmless when nothing consumes it, but pytest's log capture prints every DEBUG record for a failed test, so a failing contract test involving a file upload flooded the CI log with megabytes of raw PDF bytes.
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The new redaction behavior needs regression coverage for raw and multipart payloads.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Redacts binary request payloads from DEBUG logs to prevent oversized CI output.
Changes:
- Adds recursive redaction for
bytesandbytearray. - Applies redaction to logged request options.
File summaries
| File | Description |
|---|---|
src/landingai_ade/_base_client.py |
Redacts binary values before request-option logging. |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 1
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+501
to
+502
| _redact_binary_for_logging( | ||
| model_dump( |
pyright strict flagged the recursive redact helper because narrowing an Any-typed value via isinstance loses element types; switch to object with explicit casts on each container branch. Also short-circuit via a _contains_binary scan so the common case (no bytes anywhere, as in every non-file request) returns the original object instead of allocating a new dict/list/tuple every call, which was tripping the tracemalloc leak test (pytest's log capture retains each call's redacted container). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
🔵 Needs a closer look
The logging regression needs automated coverage for content and multipart payloads.
Review details
Suppressed comments (1)
src/landingai_ade/_base_client.py:520
- The regression being fixed is not covered by an automated test, even though
_build_requestalready has extensive coverage intests/test_client.py. Please capture the DEBUG record for requests with directcontentbytes and nested multipartfilesbytes/bytearrays, and assert that the payload is absent while the<N bytes>placeholders are present; otherwise this logging leak can be reintroduced unnoticed.
_redact_binary_for_logging(
- Files reviewed: 1/1 changed files
- Comments generated: 0 new
- Review effort level: Balanced
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
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.
The DEBUG "Request options" log line dumped uploaded file bytes verbatim. Harmless normally, but pytest's log capture prints every DEBUG record for a failed test, so a failing contract test flooded the CI log with megabytes of raw binary (e.g. https://github.com/landing-ai/ade-python/actions/runs/34082925303/job/101621522084).
Redacts
bytes/bytearrayvalues (fromfilesandcontent) to a<N bytes>placeholder before logging.