Skip to content

fix: mask sensitive headers in API request logs#14732

Open
AUTHENSOR wants to merge 1 commit into
Comfy-Org:masterfrom
AUTHENSOR:fix/mask-auth-token-in-api-logs
Open

fix: mask sensitive headers in API request logs#14732
AUTHENSOR wants to merge 1 commit into
Comfy-Org:masterfrom
AUTHENSOR:fix/mask-auth-token-in-api-logs

Conversation

@AUTHENSOR

@AUTHENSOR AUTHENSOR commented Jul 2, 2026

Copy link
Copy Markdown

fix: mask sensitive headers in API request logs

Summary

The API request logger (comfy_api_nodes/util/request_logger.py) writes
request/response details to persistent plaintext files in
temp/api_logs/. Without masking, the Authorization header — which carries
the user's Comfy API bearer token for paid nodes (Grok, Bria, Runway, Gemini,
Rodin) — is written verbatim to every log file. These files are never
cleaned up or rotated, so tokens accumulate on disk indefinitely.

Fix: mask Authorization, X-API-Key, Cookie, Set-Cookie, and
Proxy-Authorization headers before writing to log files. Non-sensitive
headers pass through unchanged.

The vulnerability

# comfy_api_nodes/util/client.py:646-705 (the caller)
payload_headers = {"Accept": "application/json"}
payload_headers.update(get_comfy_api_headers(cfg.node_cls))
# payload_headers now contains: {"Authorization": "Bearer <token>", ...}

request_logger.log_request_response(
    ...
    request_headers=dict(payload_headers),  # ← full headers including token
)

# comfy_api_nodes/util/request_logger.py:103-104 (before fix)
if request_headers:
    log_content.append(f"Headers:\n{_format_data_for_logging(request_headers)}")
    # ↑ writes {"Authorization": "Bearer sk-xxx"} to plaintext .log file

Every API request creates a new log file in temp/api_logs/ containing the
bearer token in plaintext. These files:

  • Never expire or rotate (no cleanup logic exists)
  • Accumulate indefinitely (one per request, hundreds over weeks of use)
  • Are plaintext (no encryption)

Exposure scenarios

  • User shares their ComfyUI directory for debugging
  • User uploads logs for support (the log files are designed for this purpose)
  • A backup system captures the temp directory
  • Another process on a shared system reads temp files
  • A crash report or error tracker captures file paths/content

The fix

# after — mask sensitive headers before formatting
_SENSITIVE_HEADERS = frozenset(
    {"authorization", "proxy-authorization", "x-api-key", "cookie", "set-cookie"}
)

def _mask_sensitive_headers(headers):
    masked = dict(headers)
    for key in masked:
        if key.lower() in _SENSITIVE_HEADERS:
            masked[key] = "***"
    return masked

# applied at both request and response header logging
if request_headers:
    log_content.append(f"Headers:\n{_format_data_for_logging(_mask_sensitive_headers(request_headers))}")

Non-sensitive headers (Accept, Content-Type, User-Agent) pass through
unchanged — the logs remain useful for debugging.

Tests

9 tests in tests/test/test_request_logger.py:

  • Masking behavior: Authorization, X-API-Key, Cookie, Set-Cookie
    are all masked to ***
  • Case-insensitivity: authorization and AUTHORIZATION both masked
  • Non-sensitive headers unchanged: Accept, Content-Type, User-Agent
    pass through
  • Non-mutation: the original headers dict is not modified (returns copy)
  • Edge cases: None and {} handled
  • End-to-end: a real log file written by log_request_response does not
    contain the token, but does contain Authorization: ***
$ python -m pytest tests/test/test_request_logger.py -v
============================== 9 passed in 0.02s ==============================

Impact

  • Credential exposure: the Comfy API bearer token (used for paid API
    nodes) is written to persistent plaintext log files that never expire
  • Affected users: anyone using Comfy API nodes (Grok, Bria, Runway,
    Gemini, Rodin, etc.) — the token is logged on every request
  • Severity: medium — requires secondary access (reading temp files), but
    the log files are designed to be human-readable and shareable for debugging

Scope

Two files: comfy_api_nodes/util/request_logger.py (the fix) and
tests/test/test_request_logger.py (new test file). Diff: +175/−13.

API Node PR Checklist

Scope

  • Is API Node Change

Pricing & Billing

  • Need pricing update
  • No pricing update

If Need pricing update:

  • Metronome rate cards updated
  • Auto‑billing tests updated and passing

QA

  • QA done
  • QA not required

Comms

  • Informed Kosinkadink

The API request logger writes request/response details to persistent
plaintext files in the temp/api_logs directory. Without masking, the
Authorization header (which carries the user's Comfy API bearer token for
paid nodes like Grok, Bria, Runway, Gemini, and Rodin) is written verbatim
to every log file. These files are never cleaned up, so tokens accumulate
on disk indefinitely.

Fix: mask Authorization, X-API-Key, Cookie, Set-Cookie, and
Proxy-Authorization headers before writing to log files. Non-sensitive
headers pass through unchanged.

9 tests: masking behavior, case-insensitivity, non-mutation of original,
and end-to-end verification that the token does not appear in the log file.

Signed-off-by: John Kearney <johndanielkearney@gmail.com>
@coderabbitai

coderabbitai Bot commented Jul 2, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 7d4c3ed1-0c9a-49c8-8a35-c2e1595e1e2c

📥 Commits

Reviewing files that changed from the base of the PR and between 35c1470 and f67f4ac.

📒 Files selected for processing (2)
  • comfy_api_nodes/util/request_logger.py
  • tests/test/test_request_logger.py

📝 Walkthrough

Walkthrough

This pull request adds masking for sensitive HTTP headers (Authorization, Proxy-Authorization, X-API-Key, Cookie, Set-Cookie) in the request logger used by comfy_api_nodes, via a new _mask_sensitive_headers helper applied before request and response headers are written to plaintext log files. Filename sanitization logic is reformatted without behavior changes, and the module's __main__ demonstration block is expanded with additional example invocations. A new test module validates masking behavior and confirms sensitive values do not appear in generated log files.

Changes

Cohort / File(s) Summary
comfy_api_nodes/util/request_logger.py Adds _SENSITIVE_HEADERS set and _mask_sensitive_headers() helper; applies masking to request and response header logging; reformats filename sanitization; expands __main__ demo block
tests/test/test_request_logger.py New test module covering header masking behavior and log-file output verification

Sequence Diagram(s)

See hidden layer diagram for header masking flow.

Related PRs: None identified.

Suggested labels: security, tests

Suggested reviewers: (none identified from provided context)

Poem: A rabbit hopped through logs at night, and found tokens shining much too bright. With a wave of paw, three stars appear — ***, ***, hiding what should not be here. Now Bearer tokens sleep unseen, safe within the log, so clean.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: masking sensitive headers in API request logs.
Description check ✅ Passed The description is directly related to the PR and explains the masking fix, risks, and tests.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

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.

1 participant