Skip to content

fix: strip API keys from request debug dumps (#8518)#8533

Open
EdderTalmor wants to merge 1 commit into
NousResearch:mainfrom
EdderTalmor:fix/sanitize-debug-dumps-8518
Open

fix: strip API keys from request debug dumps (#8518)#8533
EdderTalmor wants to merge 1 commit into
NousResearch:mainfrom
EdderTalmor:fix/sanitize-debug-dumps-8518

Conversation

@EdderTalmor

Copy link
Copy Markdown

Problem

_dump_api_request_debug persists raw api_kwargs into JSON files on disk. While the Authorization header is masked via _mask_api_key_for_logs(), the request body may contain api_key, x_api_key, or api_token as raw values (depending on SDK/client combination). These get written unredacted to logs_dir/request_dump_*.json.

Fix

Strip known secret keys from the body before writing:

  • api_key
  • x_api_key
  • api_token

Affected code

  • run_agent.py: _dump_api_request_debug()

_dump_api_request_debug persisted raw api_kwargs into JSON files on disk,
including any api_key present in the kwargs dict. While the Authorization
header was masked, the body contained the unredacted key.

Strip api_key, x_api_key, and api_token from the body before writing
debug dump files.
@egilewski

Copy link
Copy Markdown
Contributor

I would not merge this yet. The top-level body scrub is a useful start, but it does not close #8518 and the PR currently conflicts with current upstream/main.

Checked current upstream/main d29caf382868f8f5fb5e0c09f632f70f27e6e64e and PR head 447ee58db8860bdc796c0739112cbf04d30f3a54:

  • On current main, a focused request-dump probe against agent.agent_runtime_helpers.dump_api_request_debug still writes top-level api_key, x_api_key, and api_token sentinel values to request_dump_*.json, and the Authorization header is still partially masked rather than omitted.
  • Replaying this PR on its base removes only those top-level body keys. Nested values such as messages[0].api_key / request override fields still persist in the dump, and the partially masked Authorization header remains.
  • gh pr view 8533 --json mergeable,mergeStateStatus reports CONFLICTING / DIRTY; current main also moved this code into agent/agent_runtime_helpers.py, so the fix needs to be ported there.
  • CodeRabbit on the uncommitted PR replay completed and reported one major finding for incomplete nested credential redaction.

Recommendation: rebase/port the change to agent.agent_runtime_helpers.dump_api_request_debug, remove or fully redact the Authorization header, and use recursive, case-insensitive redaction for nested dict/list keys such as api_key, x_api_key, and api_token before writing request dumps.

Signed: GPT-5.5-xhigh in Codex

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/agent Core agent loop, run_agent.py, prompt builder P0 Critical — data loss, security, crash loop type/security Security vulnerability or hardening

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants