Skip to content

fix(provider,api-express-router,logger): stop request-logger context bleed and log every endpoint's envelope#2820

Merged
forgetso merged 2 commits into
mainfrom
fix/request-logger-context-bleed
Jul 9, 2026
Merged

fix(provider,api-express-router,logger): stop request-logger context bleed and log every endpoint's envelope#2820
forgetso merged 2 commits into
mainfrom
fix/request-logger-context-bleed

Conversation

@forgetso

@forgetso forgetso commented Jul 9, 2026

Copy link
Copy Markdown
Member

Summary

  • Tasks.setLogger no longer mutates db.logger (packages/provider/src/tasks/tasks.ts) — env.getDb() returns a process-wide singleton, so overwriting db.logger on every request meant concurrent captcha submits raced and one request's user/siteKey/sessionId bindings stamped the other's DB log lines. Callers in getPoWCaptchaChallenge/getPuzzleCaptchaChallenge now pass req.logger through the constructor and drop the redundant setLogger call.
  • requestLoggerMiddleware emits Request received and Response sent envelopes on every route (packages/api-express-router/src/middlewares/requestLoggerMiddleware.ts) — previously only /frictionless had per-endpoint response logging, so getPow/PuzzleCaptchaChallenge, submitPow/PuzzleCaptchaSolution, verify.ts had no envelope in OpenObserve. Health probes (/healthz, /health, /readyz) skip the envelope so they don't drown the stream. Also mirrors x-request-id back on the outbound response.
  • Promote requestId to a top-level req_id field on the log record (packages/logger/src/logger.ts) — OpenObserve indexes top-level fields as their own columns, so WHERE req_id = '…' is now cheap. data.requestId is preserved for backwards compat. Two new unit tests.

Motivation traced in a live incident: for a puzzle solve on pronode15 at 2026-07-09T13:03:55Z, the PuzzleCaptcha record updated successfully log line for user 5G3XQqmid…'s challenge showed data_user=5HDoUo… because a concurrent request had just called setLogger and clobbered the shared db.logger. Signature validation was intact — the record was legitimately being updated for the right user — but the log output was misleading and broke user-scoped filtering.

Test plan

  • npm run -w @prosopo/logger test — 39/39 passing (37 existing + 2 new)
  • npm run -w @prosopo/logger build:tsc — clean
  • npm run -w @prosopo/api-express-router build:tsc — clean
  • npx tsc --noEmit -p packages/provider/tsconfig.json — no new errors in touched files (repo has pre-existing merge markers in blacklistRequestInspector.ts/captchaManager.ts unrelated to this PR)
  • Verify on a staging pronode: req_id appears at the top level of production_provider_node records
  • Verify concurrent puzzle solves no longer cross-tag data_user/data_sitekey/data_sessionid
  • Verify Request received / Response sent envelopes appear for /captcha/puzzle, /captcha/pow, and /api/verify routes

🤖 Generated with Claude Code

…ields leaking across concurrent captcha requests and emit request/response envelopes on every endpoint

- Tasks.setLogger no longer mutates the process-wide db.logger, which was
  causing user/siteKey/sessionId bindings from one in-flight request to
  overwrite another's on DB log lines (visible as a PuzzleCaptcha update
  for user A's challenge tagged with user B's account). Callers now pass
  the request logger through the Tasks constructor and drop the redundant
  setLogger call.
- requestLoggerMiddleware emits "Request received" and "Response sent"
  lines for every route (method, path, status, durationMs, req_id),
  excluding /healthz|/health|/readyz. Also echoes x-request-id on the
  outbound response so downstream callers can correlate without Caddy.
- Promote requestId from `data.requestId` to a top-level `req_id` field
  on the emitted JSON so OpenObserve indexes it as its own column and
  WHERE req_id = '…' becomes cheap.
- New logger unit tests cover promotion + absent-when-unset.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@forgetso forgetso enabled auto-merge (squash) July 9, 2026 15:27
Join `dataMaybeRequestId && typeof ...` onto a single line per biome's
formatter — no logic change.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@forgetso forgetso merged commit 6abff15 into main Jul 9, 2026
11 checks passed
@forgetso forgetso deleted the fix/request-logger-context-bleed branch July 9, 2026 15:48
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