Skip to content

Conversation

@michaelmalave
Copy link
Contributor

@michaelmalave michaelmalave commented Nov 6, 2025

Summary

Implements URL sanitization for logging to prevent sensitive OAuth parameters from appearing in logs. Adds a utility to remove sensitive query parameters from URLs before they are written to error logs and HTTP request logs, improving security posture without affecting functionality.

Type of Change

  • fix: Bug fix or issue (patch semvar update)
  • feat: Introduces a new feature to the codebase (minor semvar update)
  • perf: Performance improvement
  • docs: Documentation only changes
  • tests: Adding missing tests or correcting existing tests
  • chore: Code cleanup tasks, dependency updates, or other changes

Testing

Run and verify all URL sanitizer tests pass:

pnpm install
pnpm test --grep "URL Sanitizer"

All 25 unit tests pass, covering:

  • Sensitive OAuth parameters
  • Absolute and relative URLs
  • Edge cases (malformed URLs, encoded parameters, parameters without values)
  • Real-world OAuth scenarios

Additional Context

Changes:

  • Added lib/url-sanitizer.js module to sanitize URLs by removing sensitive OAuth query parameters
  • Updated logEventCtxError() in lib/server.js to sanitize URLs before error logging
  • Replaced morgan('tiny') with custom format that sanitizes URLs in HTTP request logs

Impact:

  • Prevents sensitive OAuth data from appearing in logs
  • No functional impact: sanitization only affects log output; actual request processing uses unsanitized URLs
  • Downstream projects (e.g., mcp-heroku-com) unaffected as they don't depend on log content
  • OAuth flows continue to work correctly using actual request URLs and query parameters

Sanitized Parameters: Removes sensitive OAuth query parameters from logged URLs while preserving non-sensitive query parameters (e.g., scope, response_type).

Related Issue

Closes #W-20006706

@michaelmalave michaelmalave requested a review from a team as a code owner November 6, 2025 20:17
@michaelmalave michaelmalave changed the title fix: adding pre logging oath url scrubbing and testing to ensure sensitive details are not passed fix: adding pre logging oath url scrubbing to ensure sensitive data not stored Nov 6, 2025
Copy link
Contributor

@k80bowman k80bowman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@michaelmalave michaelmalave merged commit 017173b into main Nov 7, 2025
1 check passed
@michaelmalave michaelmalave deleted the mm/fix/oauth-url-sanitization branch November 7, 2025 16:49
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.

2 participants