Skip to content

fix: log WHIP/WHEP authentication failures for abuse detection - #379

Merged
birme merged 1 commit into
mainfrom
backend/238-log-whip-whep-auth-failures
Sep 29, 2026
Merged

birme merged 1 commit into
mainfrom
backend/238-log-whip-whep-auth-failures

Conversation

@birme

@birme birme commented Sep 29, 2026

Copy link
Copy Markdown
Contributor

Summary

  • The requireWhipAuth/requireWhepAuth hooks in src/api_whip.ts and src/api_whep.ts rejected invalid/missing bearer tokens with a 401 but emitted no log entry, making brute-force and credential-stuffing attempts invisible.
  • Add a warning-level log on each WHIP and WHEP auth failure that includes the client IP (request.ip) and request path (request.url) so abuse can be detected.
  • The token value is never logged; the path is passed through the existing sanitizeForLog helper to prevent log injection, matching the repo's defense-in-depth idiom.
  • Logging-only change: no trustProxy or other configuration was touched, keeping the diff minimal and additive.
  • Extended the existing WHIP/WHEP auth tests to assert a warning is logged on failure and that the token is never present in log output.

Test plan

  • Tests pass (npm test) — 363 passed
  • TypeScript compiles (npm run typecheck)
  • Lint clean (npm run lint) — 0 errors
  • Verified a WHIP/WHEP authentication failed warning with IP and path is emitted on auth failure, and the bearer token never appears in any log line

Closes #238

🤖 Generated with Claude Code

The requireWhipAuth/requireWhepAuth hooks rejected invalid bearer
tokens with a 401 but emitted no log entry, making brute-force and
credential-stuffing attempts invisible. Add a warning-level log on
each auth failure including the client IP and request path. The token
value is never logged, and the path is sanitised to prevent log
injection.

Closes #238

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@birme

birme commented Sep 29, 2026

Copy link
Copy Markdown
Contributor Author

Code Review

Verdict: LGTM

Summary: Adds a warning-level log on WHIP/WHEP auth failures for abuse detection (Closes #238). Correct, minimal, and safe: the token value is never logged, the only user-controllable field logged (request.url) is sanitized via sanitizeForLog, both new tests mock ./log and assert the warning fires and the token is absent, and the change is scoped strictly to logging (no trustProxy/behavioral changes). CI green (lint, pretty, ts, unittests).

Blocking

  • None.

Warnings

  • None.

Suggestions

  • Consider wrapping request.ip in sanitizeForLog defensively in case trustProxy is ever enabled (safe today — socket address, not header-derived).

Reviewed by a separate code-reviewer invocation (not the implementer). Self-authored PR: recording the verdict as a marker comment because GitHub blocks state-bearing self-review; merging via --admin per the daily-backlog-pr self-authored path (review requirement unsatisfiable for the authoring account, CI green, not conflicting/behind).

@birme
birme merged commit 87bf643 into main Sep 29, 2026
4 checks passed
@birme
birme deleted the backend/238-log-whip-whep-auth-failures branch September 29, 2026 15:44
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.

Security: Log authentication failures on WHIP/WHEP endpoints to enable abuse detection

2 participants