Skip to content

fix: omit authorization from request debug dumps#8768

Open
officialasishkumar wants to merge 1 commit into
NousResearch:mainfrom
officialasishkumar:fix/redact-request-dump-auth
Open

fix: omit authorization from request debug dumps#8768
officialasishkumar wants to merge 1 commit into
NousResearch:mainfrom
officialasishkumar:fix/redact-request-dump-auth

Conversation

@officialasishkumar

@officialasishkumar officialasishkumar commented Apr 13, 2026

Copy link
Copy Markdown

What does this PR do?

Removes the Authorization header from request debug dump files written by _dump_api_request_debug().

The dump is meant to help diagnose provider request failures, but it does not need to persist credential material. Omitting the header entirely avoids exposing even partially masked API keys in request_dump_*.json files while preserving the request URL, content type, body, and error context needed for debugging.

Related Issue

Fixes #8518

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✨ New feature (non-breaking change that adds functionality)
  • 🔒 Security fix
  • 📝 Documentation update
  • ✅ Tests (adding or improving test coverage)
  • ♻️ Refactor (no behavior change)
  • 🎯 New skill (bundled or hub)

Changes Made

  • Removed API key extraction from _dump_api_request_debug() in run_agent.py.
  • Removed Authorization from the persisted debug dump header payload.
  • Added a regression test that writes a dump and verifies no full or partial API key material appears in the serialized JSON.

How to Test

  1. Run: 
    python -m pytest tests/run_agent/test_run_agent.py::TestApiRequestDebugDump tests/run_agent/test_run_agent.py::TestMaskApiKey -q -o addopts=''
  2. Confirm the dump contains only Content-Type under request headers.
  3. Confirm the serialized dump does not contain Authorization or API key fragments.

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: Ubuntu Linux 6.8 x86_64

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — or N/A
  • I've updated cli-config.yaml.example if I added/changed config keys — or N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — or N/A
  • I've updated tool descriptions/schemas if I changed tool behavior — or N/A

For New Skills

N/A

Screenshots / Logs

Targeted test output:

4 passed, 1 warning in 1.47s

@alt-glitch alt-glitch added type/security Security vulnerability or hardening P0 Critical — data loss, security, crash loop comp/agent Core agent loop, run_agent.py, prompt builder labels Apr 28, 2026
@alt-glitch

alt-glitch commented Apr 28, 2026

Copy link
Copy Markdown
Collaborator

Likely duplicate of #8533 — both strip Authorization header from _dump_api_request_debug() debug dumps to fix #8518.

@alt-glitch alt-glitch mentioned this pull request May 2, 2026
Closed
1 task
@zons-zhaozhy zons-zhaozhy mentioned this pull request May 2, 2026
Closed
@egilewski

egilewski commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

Needs rework before this can close #8518.

I checked current upstream/main at e7a7872a874837ca36105ca3e90db464cf7c125a and PR head 0dad3b8b6d43bddf9180f4e19a22924c5e044043. Current main now writes request debug dumps through agent/agent_runtime_helpers.py, while this PR changes the older run_agent.py dump implementation.

Validation:

  • A synthetic request-dump probe on current main reproduced the exposure: the dump includes a masked request.headers.Authorization value and still serializes body credential fields such as api_key and nested x_api_key-style values.
  • The same probe on the patched PR-base replay confirms the PR removes the dump Authorization header, but body credential fields still serialize, including nested credential-bearing values. That leaves the core request-body leakage from [Security] API key exposed in request debug dumps via incomplete masking #8518 only partially addressed.
  • python -m pytest tests/run_agent/test_run_agent.py::TestApiRequestDebugDump tests/run_agent/test_run_agent.py::TestMaskApiKey -q -o addopts="" -p no:cacheprovider passed on the PR replay (4 passed).
  • CodeRabbit ran successfully against the uncommitted PR diff with base e7a7872a874837ca36105ca3e90db464cf7c125a and reported No findings.

Recommendation: rebase onto current main, move the fix to agent/agent_runtime_helpers.py, and apply recursive redaction/removal for credential-shaped keys and Authorization-bearing values before serializing the dump body. This appears to need the same broader treatment discussed on #8533 rather than only dropping the top-level request header.

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

Reviewers

No reviews

Assignees

No one assigned

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

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Security] API key exposed in request debug dumps via incomplete masking

3 participants

@officialasishkumar @alt-glitch @egilewski