Skip to content

Remove line breaks outright in logSafe - #109

Merged
akumar-99 merged 1 commit into
mainfrom
fix/log-injection-alert
Oct 22, 2026
Merged

akumar-99 merged 1 commit into
mainfrom
fix/log-injection-alert

Conversation

@akumar-99

Copy link
Copy Markdown
Contributor

What and why

Bug fix, no spec. Closes code-scanning alert #27 (log injection, src/server.ts), which stayed open after #108.

CodeQL's log-injection rule only treats a replace as sanitising when it swaps line breaks for an empty string (StringReplaceSanitizer in LogInjectionQuery.qll). #108 swapped them for a space, so the rule never counted it.

Changes

  • logSafe now removes \r and \n outright. Other control characters still become spaces, and the 200-character cap is unchanged.
  • A rejected request's reason now logs a\nb as ab rather than a b.

Checks

  • bun run typecheck && bun run test:coverage pass (100% lines and functions)
  • bun run web:typecheck && bun run web:test pass (no web change)
  • Each acceptance criterion has a test (no spec; the existing log-forging test covers the change)
  • Figma "AsyncUp v2 — Product Design" matches what was built, or no UI change
  • Docs (docs/guide/) updated for anything people will notice, and bun run docs:build passes (nothing to update)
  • Website (site/) updated if a feature it describes changed, and bun run site:build passes (nothing to update)
  • Layout checked from 390px to 1440px, or no UI change

CodeQL's log-injection rule only treats a replace as sanitising when it swaps line breaks for an empty string, so alert #27 stayed open after #108.
@akumar-99
akumar-99 merged commit b6681b8 into main Oct 22, 2026
6 checks passed
@akumar-99
akumar-99 deleted the fix/log-injection-alert branch October 22, 2026 06:06
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