Skip to content

fix: Sanitize middleware logging and add tracing headers#754

Merged
thostetler merged 1 commit intoadsabs:masterfrom
thostetler:legacy-app-detection-ssr-fix
Jan 14, 2026
Merged

fix: Sanitize middleware logging and add tracing headers#754
thostetler merged 1 commit intoadsabs:masterfrom
thostetler:legacy-app-detection-ssr-fix

Conversation

@thostetler
Copy link
Member

Tracing headers (X-Original-Uri, X-Forwarded-For, X-Amzn-Trace-Id) were not being forwarded from middleware to the bootstrap API, breaking distributed tracing. Additionally, middleware logs were exposing raw usernames and were vulnerable to log injection.

  • Forward TRACING_HEADERS to bootstrap API calls in initSession
  • Add src/utils/logging.ts with getUserLogId() for privacy-safe user correlation (hashed)
  • Add sanitizeHeaderValue() to prevent log injection attacks
  • Enhanced structured logging throughout middleware with timing info
  • Added tests for tracing header forwarding

@thostetler thostetler requested a review from shinyichen January 7, 2026 17:41
@thostetler thostetler marked this pull request as ready for review January 7, 2026 17:41
@codecov
Copy link

codecov bot commented Jan 7, 2026

Codecov Report

❌ Patch coverage is 87.55556% with 28 lines in your changes missing coverage. Please review.
✅ Project coverage is 61.0%. Comparing base (1467923) to head (39498ef).

Files with missing lines Patch % Lines
src/utils/logging.ts 63.3% 18 Missing ⚠️
src/middleware.ts 93.4% 10 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##           master    #754     +/-   ##
========================================
+ Coverage    60.8%   61.0%   +0.2%     
========================================
  Files         300     300             
  Lines       34871   34965     +94     
  Branches     1486    1506     +20     
========================================
+ Hits        21193   21317    +124     
+ Misses      13644   13614     -30     
  Partials       34      34             
Files with missing lines Coverage Δ
src/middlewares/initSession.ts 91.4% <100.0%> (+0.8%) ⬆️
src/middleware.ts 93.9% <93.4%> (-0.7%) ⬇️
src/utils/logging.ts 63.3% <63.3%> (ø)

... and 3 files with indirect coverage changes

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

- Replaced username logging with hashed userId in public-facing middleware logs
- Added sanitization for User-Agent and tracing headers to prevent log injection
- Forward tracing headers (X-Original-Uri, X-Forwarded-For, X-Amzn-Trace-Id) to bootstrap API
- Enhanced logging throughout middleware with structured data and timing info
- Added tests for tracing header forwarding
@thostetler thostetler force-pushed the legacy-app-detection-ssr-fix branch from a9a709a to 39498ef Compare January 14, 2026 15:56
@thostetler thostetler merged commit cf445b1 into adsabs:master Jan 14, 2026
5 checks passed
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