fix: address open code scanning alerts - #1688
Conversation
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Frontend HTML artifact readyThe latest frontend build uploaded the This comment updates automatically when a new frontend build artifact is uploaded. |
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🟡 Changes recommended
An unresolved critical browse-token race and a moderate Markdown inline-code escaping issue remain.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
This PR addresses CodeQL findings by hardening OG preview networking, sanitizing diagnostics, improving frontend parsing, and adding regression coverage.
Changes:
- Adds DNS pinning, origin authorization, capability isolation, and safe errors.
- Sanitizes webhook and verification diagnostics.
- Improves Markdown/search parsing and adds focused CI tests.
File summaries
| File | Summary |
|---|---|
tests/StaticHost.Tests/Live/TwitchWebhookHandlerTests.cs |
Tests sanitized webhook diagnostics. |
tests/StaticHost.Tests/Live/LiveTestHelpers.cs |
Provides shared live-test helpers. |
tests/StaticHost.Tests/Live/LiveEndpointsTests.cs |
Tests endpoint logging behavior. |
src/statichost/StaticHost/Live/YouTube/YouTubeDiagnostics.cs |
Adds safe verification diagnostics. |
src/statichost/StaticHost/Live/Twitch/TwitchWebhookHandler.cs |
Sanitizes Twitch diagnostics. |
src/statichost/StaticHost/Live/README.md |
Documents diagnostic behavior. |
src/statichost/StaticHost/Live/LiveEndpoints.cs |
Applies sanitized logging. |
src/frontend/tests/unit/search.vitest.test.ts |
Tests inert search extraction. |
src/frontend/tests/unit/sample-readme-headings.vitest.test.ts |
Tests heading handling. |
src/frontend/tests/unit/api-search-lifecycle.vitest.test.ts |
Tests search lifecycle parsing. |
src/frontend/tests/unit/api-markdown.vitest.test.ts |
Tests Markdown escaping behavior. |
src/frontend/src/utils/sample-readme-headings.ts |
Slugifies plain heading text. |
src/frontend/src/utils/api-markdown-shared.ts |
Escapes Markdown table delimiters; inline-code handling requires changes. |
src/frontend/src/scripts/search.ts |
Extracts inert search text. |
.github/workflows/og-preview-tests.yml |
Adds OG preview regression CI. |
.github/extensions/og-preview/ui/app.js |
Adds capability-aware requests and navigation validation. |
.github/extensions/og-preview/tests/server.test.mjs |
Tests preview server behavior. |
.github/extensions/og-preview/tests/request-access.test.mjs |
Tests access policies and capabilities. |
.github/extensions/og-preview/tests/http-fetch.test.mjs |
Tests safe outbound fetching. |
.github/extensions/og-preview/tests/fixtures/sdk.mjs |
Supplies test SDK fixtures. |
.github/extensions/og-preview/README.md |
Documents network isolation. |
.github/extensions/og-preview/lib/request-access.mjs |
Separates UI and browse capabilities. |
.github/extensions/og-preview/lib/http-fetch.mjs |
Validates destinations and pins DNS answers. |
.github/extensions/og-preview/lib/agent-readiness.mjs |
Applies safe readiness checks. |
.github/extensions/og-preview/extension.mjs |
Applies preview policies; browse tokens must be captured before awaiting network I/O. |
Review details
Suppressed comments (1)
src/frontend/src/utils/api-markdown-shared.ts:107
- This regex also escapes pipes inside inline code spans, but backslashes are literal inside code spans, so a value such as
`left|middle|right`renders as<code>left\|middle\|right</code>rather than preserving the code contents (the new test at api-markdown-vitest.test.ts:265 expects the opposite). Please protect code spans while escaping table delimiters, or adjust the contract and tests to reflect the changed output.
.replace(/\\*\|/g, (delimiter) => delimiter.replace(/[\\|]/g, '\\$&'))
- Files reviewed: 25/25 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
That is not the result inside the GFM tables this helper serves: table parsing consumes the delimiter escape before rendering inline code. The existing rendered-table regression covers |
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Summary
Addresses all 19 open CodeQL findings inventoried on
mainat26d319119c731f652297780648754d4549873825, plus follow-up findings exposed by PR analysis. Fresh CodeQL analysis reports zero findings in both languages for current head8944e691. Full integration CI is still running.OG_ALLOW_PRIVATE_NETWORKremains an opt-in for explicitly selected private origins, not arbitrary destinations discovered in fetched content. Separate private UI capabilities from revocable browse-resource capabilities.Review-driven simplification
The table helper now escapes delimiters and adjacent backslashes together, preserving existing Markdown links, ordinary escapes, and inline-code paths rather than parsing backtick runs and generating an HTML-code fallback. It is not a general Markdown normalizer: exact round-tripping of a literal backslash immediately followed by a pipe inside a code span is outside this narrower contract. Current API metadata has no pipe-bearing code, and all 801 existing enum-table cells retain their original output. Original tests remain intact; added speculative cases were narrowed to this contract.
The C# alert on the previous head originated from a non-secret test fixture named
UntrustedLogPayload: CodeQL's sensitive-name heuristic includes%trusted%. The fixture is now accurately namedLogInjectionPayload, with unchanged injection bytes and safety assertions. The separate invalid verification-token fixture and explicit secret-exclusion assertions remain. Fresh analysis confirms zero C# findings; the earlier explanation attributing the alert solely to fixture reuse was incomplete.Alert mapping
Dependabot disposition
#1686 contains 27 grouped dependency upgrades, including major-version updates; #1661 updates workflow actions. Neither fixes these source findings or is fully superseded by this PR, so both remain open. No unrelated upgrades were incorporated to manufacture supersession, and no Dependabot PRs were closed.
Third-party links and affiliations
None.
Validation
Latest revision (
8944e691):992f4ec4342447ad2f48481da084511433f4756d. Its parents confirm it includes current PR head8944e69173c4f3214b964666f455ac9bc002d065.git diff --checkpassed; generated icon-cache side effects were restored. Working tree clean after publication.Earlier validation, retained for provenance only:
643bb9c1de76ba1ec516596fad8b51ab34027c2e.noopenernavigation, and rejected forged messages. Six frontend excerpt fixtures passed without script execution or unexpected resource requests.Default-branch alerts remain open until fixes land and default-branch analysis confirms closure.