Skip to content

Conversation

@TheJACKedViking
Copy link
Collaborator

Potential fix for https://github.com/Enflame-Media/happy-server/security/code-scanning/5

In general, to fix incomplete escaping/encoding, ensure that all characters that are special in the target context are consistently escaped. Here, for an HTTP quoted-string used in the Server-Timing header, both the backslash (\) and the double quote (") must be escaped. Doing only " is incomplete.

The best targeted fix without changing functionality is to update the formatTimingEntry function so that, when entry.desc is present, we first escape backslashes, then escape double quotes on the already-escaped string. The order matters: we must escape \ before ", otherwise we might double-escape backslashes that we introduce during escaping. A simple and correct implementation is:

const escapedDesc = entry.desc.replace(/\\/g, '\\\\').replace(/"/g, '\\"');
result += `;desc="${escapedDesc}"`;

This change is localized to formatTimingEntry in src/middleware/timing.ts around line 79. No new imports or additional helper functions are required; we just modify the string construction to perform both replacements.

Suggested fixes powered by Copilot Autofix. Review carefully before merging.

…ng or encoding

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
Copilot AI review requested due to automatic review settings December 26, 2025 10:50
@coderabbitai
Copy link

coderabbitai bot commented Dec 26, 2025

Caution

Review failed

The pull request is closed.

Walkthrough

The middleware now properly escapes both backslashes and quotes in Server-Timing header descriptions. When a timing entry includes a description, the code generates a safely escaped string suitable for HTTP quoted-string values, then includes this escaped version in the desc parameter of the Server-Timing header.

Changes

Cohort / File(s) Summary
Server-Timing Header Escaping
src/middleware/timing.ts
Enhanced description escaping to handle both backslashes and quotes for proper HTTP quoted-string formatting in Server-Timing headers

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐰 A backslash and a quote walked through the wire,
But my escape sequences? They're climbing higher!
Now Server-Timing descriptions dance so clean,
Properly quoted—the safest you've seen!

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch alert-autofix-5

📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 980494c and 5cad9cd.

📒 Files selected for processing (1)
  • src/middleware/timing.ts

Comment @coderabbitai help to get the list of available commands and usage tips.

@TheJACKedViking TheJACKedViking marked this pull request as ready for review December 26, 2025 10:50
@TheJACKedViking TheJACKedViking merged commit b1d04e5 into cloudflare-workers Dec 26, 2025
10 of 11 checks passed
@TheJACKedViking TheJACKedViking deleted the alert-autofix-5 branch December 26, 2025 10:51
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR addresses a security vulnerability (code scanning alert #5) related to incomplete string escaping in the Server-Timing HTTP header. The fix ensures that both backslashes and double quotes are properly escaped when formatting timing descriptions, preventing potential header injection or parsing issues.

  • Implements proper HTTP quoted-string escaping by escaping backslashes first, then double quotes
  • Updates the comment to better describe the escaping purpose
  • Refactors the escaping logic into a separate variable for clarity

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

TheJACKedViking added a commit that referenced this pull request Jan 9, 2026
Addresses multiple issues: improves connection state recovery after hibernation (Fix slopus#10), ensures proper cleanup of pending auth alarms (Fix #3), corrects RPC response format handling (HAP-689, Fix #4), and fixes session deletion updates (Fix slopus#8). Updates documentation and references to new project structure, updates formatting scripts to use oxfmt, and bumps several dev dependencies.
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