Redact sensitive headers in HTTP request/response logs#469
Conversation
Added redactSensitiveHeaders funciton to mask sensitive headers in HTTP dumps
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughDebug HTTP request and response dumps in ChangesSensitive header redaction in debug logging
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cmd/vulnx/clis/common.go`:
- Around line 1047-1051: Move the redactSensitiveHeaders() function from
cmd/vulnx/clis/common.go to a shared location in pkg/service/vulnx.go, export it
by capitalizing the first letter to make it RedactSensitiveHeaders(), and then
apply this exported function to redact the req.Dump() output before logging it
in the doRequest() function. This ensures the X-PDCP-Key header and other
sensitive information are masked across all debug logging paths, not just in the
CLI callbacks.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 275d4d04-c47e-424f-b8f2-b60abd863a8c
📒 Files selected for processing (1)
cmd/vulnx/clis/common.go
Added a function to redact sensitive headers from HTTP dumps to prevent leaking credentials.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pkg/service/vulnx.go`:
- Line 209: The debug request dump is currently using gologger.Print().Msgf()
instead of the appropriate debug log level. Replace gologger.Print().Msgf() with
gologger.Debug().Msgf() in the redactSensitiveHeaders call to maintain
consistency with coding guidelines and ensure debug logs are properly filterable
by log level.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 60cfd372-9737-44db-8e31-770ed494c146
📒 Files selected for processing (1)
pkg/service/vulnx.go
There was a problem hiding this comment.
🧹 Nitpick comments (1)
pkg/service/vulnx.go (1)
230-232: ⚡ Quick winBuild the redaction pattern from
xPDCPHeaderKey.The request code sets auth headers via
xPDCPHeaderKey, but the redactor hard-codes the same string. Tie the regex to the constant so future header-name changes cannot silently bypass redaction.As per coding guidelines, "Define constants at package level when used across functions".
Proposed fix
func redactSensitiveHeaders(dump []byte) string { - re := regexp.MustCompile(`(?i)(X-PDCP-Key:\s*).*`) + re := regexp.MustCompile(`(?i)(` + regexp.QuoteMeta(xPDCPHeaderKey) + `:\s*).*`) return re.ReplaceAllString(string(dump), "${1}[REDACTED]") }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/service/vulnx.go` around lines 230 - 232, In the redactSensitiveHeaders function, replace the hard-coded header name "X-PDCP-Key:" in the regex pattern with the xPDCPHeaderKey constant. Build the regex pattern dynamically using the constant value (e.g., fmt.Sprintf or string concatenation) so that the redaction pattern automatically stays in sync with the xPDCPHeaderKey constant definition, preventing silent bypass of redaction if the header name is ever changed.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@pkg/service/vulnx.go`:
- Around line 230-232: In the redactSensitiveHeaders function, replace the
hard-coded header name "X-PDCP-Key:" in the regex pattern with the
xPDCPHeaderKey constant. Build the regex pattern dynamically using the constant
value (e.g., fmt.Sprintf or string concatenation) so that the redaction pattern
automatically stays in sync with the xPDCPHeaderKey constant definition,
preventing silent bypass of redaction if the header name is ever changed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 94a78e6c-5eb3-4bfc-97ae-60bbde024ab4
📒 Files selected for processing (1)
pkg/service/vulnx.go
|
@mirackayikci merge conflict |
|
@coderabbitai create an issue to track this pr |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
gh issue create \
--repo "projectdiscovery/vulnx" \
--title "Redact sensitive headers (X-PDCP-Key) in debug HTTP request/response dumps" \
--assignee "dogancanbakir" \
--body "## Summary
This issue tracks the work introduced in PR `#469` to prevent leaking the \`X-PDCP-Key\` authentication header in debug logs.
## Problem
When the \`--debug-req\` or \`--debug-resp\` flags are enabled, \`httputil.DumpRequestOut\` exposes the \`X-PDCP-Key\` authentication header in plaintext. This can leak the API key into CI logs or shared terminal output, creating a security risk.
## Proposed Solution
Implement a \`redactSensitiveHeaders\` helper function that uses a case-insensitive regular expression to replace the value of the \`X-PDCP-Key\` header with \`[REDACTED]\` in HTTP request/response dumps before they are emitted to debug logs.
## Affected Files
- \`cmd/vulnx/clis/common.go\` — sanitize dumps in \`ensureVulnxClientInitialized\`
- \`pkg/service/vulnx.go\` — sanitize dumps in \`doRequest\`
## Acceptance Criteria
- [ ] The \`X-PDCP-Key\` header value is replaced with \`[REDACTED]\` in all debug log output.
- [ ] All other debug information (headers, body, etc.) remains intact.
- [ ] A dedicated constant (\`xPDCPHeaderKey\`) is used in the regex pattern to avoid hardcoded strings.
- [ ] No regression in existing debug functionality.
## References
- PR: `#469`
- Requested by: `@dogancanbakir`"Length of output: 1642
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cmd/vulnx/clis/common.go`:
- Around line 1047-1050: The redaction logic in redactSensitiveHeaders only
matches X-Api-Key, so debug dumps can still expose X-PDCP-Key credentials.
Update sensitiveHeaderRe in common.go to cover both header names and keep
redactSensitiveHeaders using that shared pattern so any dump containing either
header is replaced with [REDACTED].
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f5dca276-6b11-447a-b31b-53a73216e5cd
📒 Files selected for processing (1)
cmd/vulnx/clis/common.go
2 participants
When --debug-req/--debug-resp is enabled, httputil.DumpRequestOut includes the X-PDCP-Key authentication header in plaintext, which can leak the API key into CI logs or shared terminal output. This PR masks the key value in debug request/response dumps while keeping the rest intact for debugging
Summary by CodeRabbit