Skip to content

🔒 Fix sensitive API token logging#285

Open
STRRL wants to merge 2 commits intomasterfrom
fix-log-api-token-disclosure-3087765264449044706
Open

🔒 Fix sensitive API token logging#285
STRRL wants to merge 2 commits intomasterfrom
fix-log-api-token-disclosure-3087765264449044706

Conversation

@STRRL
Copy link
Copy Markdown
Owner

@STRRL STRRL commented Feb 27, 2026

🎯 What: Masked the Cloudflare API token in the application logs.
⚠️ Risk: The API token was being logged in cleartext when the log level was set to 3 (debug). This could allow unauthorized access to the Cloudflare account if the logs were exposed.
🛡️ Solution: Replaced the actual API token value with *** in the log message at cmd/cloudflare-tunnel-ingress-controller/main.go. This ensures that the token is not written to the logs while still providing debug information that the client build step was reached.


PR created automatically by Jules for task 3087765264449044706 started by @STRRL


Note

Low Risk
Low risk: only changes a debug-level log field to avoid leaking credentials; no functional behavior or API calls are modified.

Overview
Prevents leaking the Cloudflare API token by masking it in the V(3) log message in cmd/cloudflare-tunnel-ingress-controller/main.go (logs "***" instead of the real token) while still indicating the client initialization step occurred.

Written by Cursor Bugbot for commit 4b489b5. This will update automatically on new commits. Configure here.

The Cloudflare API token was previously logged in cleartext when the log
verbosity was set to 3 or higher. This commit masks the token in the log
message to prevent sensitive information disclosure.

Fixes: security vulnerability where API token is logged

Co-authored-by: STRRL <20221408+STRRL@users.noreply.github.com>
@google-labs-jules
Copy link
Copy Markdown
Contributor

👋 Jules, reporting for duty! I'm here to lend a hand with this pull request.

When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down.

I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job!

For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.


For security, I will only act on instructions from the user who triggered this task.

@claude
Copy link
Copy Markdown
Contributor

claude Bot commented Feb 27, 2026

Claude finished @STRRL's task —— View job


Pull Request Review

I'll provide a comprehensive review of this security fix for API token logging.

Todo List:

  • Read repository guidelines (CLAUDE.md)
  • Examine the changed file and implementation
  • Analyze security effectiveness
  • Review code quality and best practices
  • Check for potential issues
  • Assess test coverage needs
  • Provide comprehensive feedback

Review Summary

This PR effectively addresses a critical security vulnerability by preventing the Cloudflare API token from being logged in cleartext. The fix is minimal, targeted, and well-implemented.

✅ Security Analysis

Excellent security fix:

  • Issue: Line 63 was logging the actual API token value at debug level (V(3))
  • Solution: Replaced options.cloudflareAPIToken with "***" in the log message
  • Impact: Prevents credential leakage in logs while maintaining debug visibility

The fix correctly addresses the core security concern without affecting functionality.

✅ Code Quality & Best Practices

Strengths:

  • Minimal change: Only touches the problematic log line (cmd/cloudflare-tunnel-ingress-controller/main.go:63)
  • Preserves functionality: The actual token is still passed to cloudflare.NewWithAPIToken() on line 64
  • Maintains debugging: Log message still indicates the client initialization step occurred
  • Follows logging patterns: Consistent with existing structured logging approach

Observations:

  • No other instances of cloudflareAPIToken being logged were found in the codebase
  • The token is properly used only for API client initialization
  • Flag definition and usage remain secure

✅ Potential Issues & Considerations

No significant issues found:

  • Change is isolated and safe
  • No functional impact on the application
  • No breaking changes to API or configuration

Minor consideration:

  • Could consider using a more descriptive mask like "<redacted>" or "[MASKED]" for clarity, but "***" is perfectly acceptable and commonly used

⚠️ Test Coverage

Gap identified:

  • No unit tests exist for the main.go file or the CLI initialization logic
  • No tests specifically verify that sensitive data is properly masked in logs
  • Integration tests don't appear to cover logging behavior

Recommendations:

  1. Consider adding a unit test that verifies API token masking in log output
  2. Could add integration tests that check log output doesn't contain sensitive data

However, given the simplicity and low risk of this change, the lack of specific tests is not a blocker for this security fix.

🔍 Performance Considerations

No performance impact:

  • String literal replacement has negligible overhead
  • Debug-level logging is typically disabled in production
  • No additional processing or memory usage

✅ Overall Assessment

This is an excellent security fix that:

  • Addresses a real credential disclosure risk
  • Uses the minimal necessary change
  • Maintains all existing functionality
  • Follows good security practices
  • Has no negative side effects

Recommendation: ✅ APPROVE

This change should be merged promptly to address the security vulnerability. The implementation is correct and follows security best practices.

@codecov
Copy link
Copy Markdown

codecov Bot commented Feb 27, 2026

Codecov Report

❌ Patch coverage is 0% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 41.72%. Comparing base (f35a4ea) to head (a39680a).
⚠️ Report is 3 commits behind head on master.

Files with missing lines Patch % Lines
cmd/cloudflare-tunnel-ingress-controller/main.go 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #285      +/-   ##
==========================================
- Coverage   47.08%   41.72%   -5.37%     
==========================================
  Files          11       12       +1     
  Lines         703      767      +64     
==========================================
- Hits          331      320      -11     
- Misses        349      424      +75     
  Partials       23       23              
Flag Coverage Δ
e2e 0.00% <0.00%> (?)
integration 25.07% <ø> (+<0.01%) ⬆️
unit 25.00% <ø> (-0.60%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

The Cloudflare API token was previously logged in cleartext when the log
verbosity was set to 3 or higher. This commit masks the token in the log
message with `<redacted>` to prevent sensitive information disclosure.

Fixes: security vulnerability where API token is logged

Co-authored-by: STRRL <20221408+STRRL@users.noreply.github.com>
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