Skip to content

fix(core/inlines): handle incomplete string escaping or encoding#5195

Merged
marcoscaceres merged 10 commits into
mainfrom
alert-autofix-51
Apr 17, 2026
Merged

fix(core/inlines): handle incomplete string escaping or encoding#5195
marcoscaceres merged 10 commits into
mainfrom
alert-autofix-51

Conversation

@marcoscaceres

Copy link
Copy Markdown
Contributor

Potential fix for https://github.com/speced/respec/security/code-scanning/51

Use a position-specific operation instead of generic first-occurrence replacement.
In src/core/inlines.js, inside inlineXrefMatches, replace:

  • return matched.replace("\\", "");

with logic that removes exactly the intended leading escape backslash from the original token, i.e. the one immediately after {{. The most precise no-behavior-expansion change is:

  • return matched.slice(0, 2) + matched.slice(3);

This preserves all other backslashes and characters, and only removes the escape marker at the expected location.

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

@marcoscaceres

Copy link
Copy Markdown
Contributor Author

@copilot see the CI error about the commit message. Can you fix that please?

Copilot AI commented Apr 14, 2026

Copy link
Copy Markdown
Contributor

@copilot see the CI error about the commit message. Can you fix that please?

I've added a new commit (f7463a9) with the proper conventional commits format: fix(inlines): use position-specific removal of leading escape backslash.

The CI "Check PR title" checks the PR title (not just commit messages), which still shows the old title. The PR title needs to be manually updated to fix(inlines): use position-specific removal of leading escape backslash for that check to pass.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • https://api.github.com/graphql
    • Triggering command: /usr/bin/gh gh auth status (http block)
  • https://api.github.com/repos/speced/respec/pulls/5195
    • Triggering command: /usr/bin/curl curl -s -X PATCH -H Authorization: token ****** -H Accept: application/vnd.github.v3+json REDACTED -d {"title":"fix(inlines): use position-specific removal of leading escape backslash"} (http block)

If you need me to access, download, or install something from one of these locations, you can either:

@marcoscaceres marcoscaceres changed the title Potential fix for code scanning alert no. 51: Incomplete string escaping or encoding fix: Incomplete string escaping or encoding Apr 14, 2026
@marcoscaceres marcoscaceres marked this pull request as ready for review April 15, 2026 09:02
marcoscaceres and others added 3 commits April 17, 2026 18:03
…ing or encoding

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
The code is self-documenting; the comment restated what the slice
call does and was generated by Copilot Autofix.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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/code-scanning finding by making inline IDL reference “escape” handling ({{\...}}) remove a specific escape marker backslash instead of doing a generic first-occurrence replacement.

Changes:

  • Update inlineXrefMatches() to remove the escape backslash using slicing rather than String.prototype.replace().

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

Comment thread src/core/inlines.js Outdated
…with optional whitespace

Agent-Logs-Url: https://github.com/speced/respec/sessions/97502a36-d132-4a00-871b-332e1c7b958d

Co-authored-by: marcoscaceres <870154+marcoscaceres@users.noreply.github.com>
@sidvishnoi sidvishnoi changed the title fix: Incomplete string escaping or encoding fix(core/inlines): handle incomplete string escaping or encoding Apr 17, 2026
@marcoscaceres

Copy link
Copy Markdown
Contributor Author

@copilot can you check and fix the ci issues?

Copilot AI and others added 2 commits April 17, 2026 18:23
…d-lang

Agent-Logs-Url: https://github.com/speced/respec/sessions/0f559468-adb1-48f7-915f-f1ccc0c6e77d

Co-authored-by: marcoscaceres <870154+marcoscaceres@users.noreply.github.com>
… in worker

Agent-Logs-Url: https://github.com/speced/respec/sessions/0f559468-adb1-48f7-915f-f1ccc0c6e77d

Co-authored-by: marcoscaceres <870154+marcoscaceres@users.noreply.github.com>

Copilot AI commented Apr 17, 2026

Copy link
Copy Markdown
Contributor

@copilot can you check and fix the ci issues?

Investigated and fixed in d73b750.

The CI failure in "Core — Highlight" was caused by importScripts() being blocked in blob workers for cross-origin URLs (blob worker origin is "null", making requests to http://localhost:9876/... cross-origin).

Root cause: The fix applied earlier for loading respec-highlight.js (fetch in main thread, inline into worker blob) wasn't applied to the highlight-load-lang action for custom languages.

Fix:

  • worker/respec-worker.js: when langScript content is provided in the message, create a same-origin blob URL and use importScripts(blobURL) instead of importScripts(langURL). Wrapped in try/catch so errors don't prevent postMessage from being called.
  • tests/spec/core/highlight.html: fetch the language script in the main thread (same-origin) and send it as langScript to the worker, bypassing the cross-origin restriction.

Comment thread worker/respec-worker.js Fixed
@marcoscaceres

Copy link
Copy Markdown
Contributor Author

@copilot can you fix ci and address any remaining comments?

…e langScript content

Agent-Logs-Url: https://github.com/speced/respec/sessions/5d432fb4-fa31-498c-91cb-d47b9d5aa895

Co-authored-by: marcoscaceres <870154+marcoscaceres@users.noreply.github.com>

Copilot AI commented Apr 17, 2026

Copy link
Copy Markdown
Contributor

@copilot can you fix ci and address any remaining comments?

Fixed in b51e27a. The remaining CodeQL alert (#52) was about importScripts(langURL) using a user-provided URL as a redirect target. Removed the else fallback entirely — the worker now requires langScript content to be provided and throws if it's missing, which also removes the unsafe URL-based path. CodeQL scan now shows 0 alerts.

@marcoscaceres marcoscaceres enabled auto-merge (squash) April 17, 2026 19:28
@marcoscaceres marcoscaceres merged commit f351bbb into main Apr 17, 2026
9 checks passed
@marcoscaceres marcoscaceres deleted the alert-autofix-51 branch April 17, 2026 20:00
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.

5 participants