Skip to content

PPNA-10000 Redact sensitive values from HTTP/auth debug logs - #38

Merged
ashbork merged 6 commits into
masterfrom
PPNA-10000-debug-logging-cleanup
Aug 26, 2026
Merged

ashbork merged 6 commits into
masterfrom
PPNA-10000-debug-logging-cleanup

Conversation

@DareYouS

Copy link
Copy Markdown
Contributor

No description provided.

@DareYouS
DareYouS requested a review from a team as a code owner August 13, 2026 09:27
Comment thread piwik_pro_log_analytics/import_logs.py Fixed
Comment thread piwik_pro_log_analytics/import_logs.py Fixed

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds redaction safeguards for credentials and sensitive HTTP values in debug logs.

Changes:

  • Adds recursive payload and URL query redaction helpers.
  • Applies redaction to authentication, request, response, and HTTP error logging.
  • Adds tests and changelog documentation.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
piwik_pro_log_analytics/import_logs.py Implements and applies log redaction.
tests/test_main.py Tests sensitive-value masking.
CHANGELOG.md Documents the security change.
Suppressed comments (6)

piwik_pro_log_analytics/import_logs.py:218

  • The URL sanitizer preserves parts.netloc, so credentials in userinfo remain visible; for example, https://user:password@example.com/?token=x still logs user:password. Redact the userinfo portion before rebuilding the URL.
            parts.netloc,

piwik_pro_log_analytics/import_logs.py:203

  • This sanitizer is not applied to RedirectHandlerWithLogging at line 1797, which still logs newurl verbatim. A redirect Location containing token_auth, cookies, or another sensitive query parameter therefore bypasses this PR's redaction; apply this helper to that URL before logging it.
def _redact_url_component_for_log(value):
    """Redact sensitive query-string parameters in a URL or path+query string."""

piwik_pro_log_analytics/import_logs.py:1862

  • The same HTTP response is logged again without redaction by the --debug-tracker path at line 2452. That second log call bypasses this protection and can emit sensitive response fields verbatim; apply _redact_sensitive_for_log there as well.
        logging.debug("Response '%s'" % _redact_sensitive_for_log(result))

piwik_pro_log_analytics/import_logs.py:1834

  • These sanitizer calls execute before logging.debug checks the log level, so normal non-debug imports now decode, parse, traverse, and re-serialize every JSON request body. For large batched tracking payloads this adds avoidable work on every request; guard the debug block before computing the redacted values.
        logging.debug(  # codeql[py/clear-text-logging-sensitive-data]
            "Request data '%s'" % _redact_sensitive_for_log(data)

piwik_pro_log_analytics/import_logs.py:1862

  • This parses, traverses, and serializes every JSON response even when debug logging is disabled, because function arguments are evaluated eagerly. Guard the sanitizer with the debug-level check so production requests do not pay this cost.
        logging.debug("Response '%s'" % _redact_sensitive_for_log(result))

piwik_pro_log_analytics/import_logs.py:1827

  • Although this request log is sanitized, Configuration logs the same user-supplied piwik_url and piwik_api_url verbatim at lines 1325 and 1332 before any request is made. Query credentials or URL userinfo therefore still reach debug output; route those two URL log arguments through _redact_url_component_for_log too.
        logging.debug("Request url '%s'" % _redact_url_component_for_log(url))

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread piwik_pro_log_analytics/import_logs.py
The OAuth token request body carries client_secret, so redacting it in
place still leaves the secret one key-name heuristic away from the log
(and CodeQL rightly flags the flow). Log only the body type and size.
@DareYouS
DareYouS force-pushed the PPNA-10000-debug-logging-cleanup branch from 79f062f to 5868d08 Compare August 13, 2026 13:45
@DareYouS
DareYouS requested a balanced review from Copilot August 13, 2026 13:46

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

Suppressed comments (2)

piwik_pro_log_analytics/import_logs.py:198

  • The current implementation still exposes credentials that are valid JSON scalars. For example, a token value of "12345" is parsed as an integer and serialized back as 12345, so the authentication debug log at line 1475 leaks it. Fail closed unless a top-level payload is a keyed object or array that can actually be scrubbed.
        try:
            parsed = json.loads(value)
        except (TypeError, ValueError):
            return _REDACTED
        try:
            return json.dumps(_redact_sensitive_for_log(parsed, as_payload=False))
        except (TypeError, ValueError):
            return _REDACTED

piwik_pro_log_analytics/import_logs.py:1876

  • This redaction runs eagerly even when debug logging is disabled. Consequently every successful API response is parsed, recursively copied, and serialized here, then parsed again by _call_api; large responses pay this overhead during normal runs. Guard the transformation with the debug-level check.
        logging.debug("Response '%s'" % _redact_sensitive_for_log(result))

Comment thread piwik_pro_log_analytics/import_logs.py
Comment thread piwik_pro_log_analytics/import_logs.py
@ashbork
ashbork merged commit 89f97d1 into master Aug 26, 2026
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants