Skip to content

fix: address open code scanning alerts - #1688

Merged
David Pine (IEvangelist) merged 7 commits into
mainfrom
ievangelist-code-scanning-fixes
Sep 17, 2026
Merged

David Pine (IEvangelist) merged 7 commits into
mainfrom
ievangelist-code-scanning-fixes

Conversation

@IEvangelist

@IEvangelist David Pine (IEvangelist) commented Sep 17, 2026 •

Copy link
Copy Markdown
Member

Summary

Addresses all 19 open CodeQL findings inventoried on main at 26d319119c731f652297780648754d4549873825, plus follow-up findings exposed by PR analysis. Fresh CodeQL analysis reports zero findings in both languages for current head 8944e691. Full integration CI is still running.

  • Harden OG preview requests against mapped-IPv6 and DNS-rebinding bypasses. Pin validated DNS answers to each connection, revalidate redirects, and preserve original Host/TLS verification.
  • Require explicitly selected local origins, including host and port. OG_ALLOW_PRIVATE_NETWORK remains an opt-in for explicitly selected private origins, not arbitrary destinations discovered in fetched content. Separate private UI capabilities from revocable browse-resource capabilities.
  • Return fixed public errors, validate outbound navigation protocols, and correct module-script recognition without changing the browse sandbox.
  • Replace incomplete frontend text/script filtering and correct Markdown table delimiter escaping without a custom backtick parser.
  • Preserve useful webhook diagnostics safely: bounded, sanitized timestamps, message types, replay/in-progress message IDs, and revocation subscription ID/type/status. Full revocation bodies, callback URLs, verification credentials, and unrelated payload fields are not logged. Signature, replay, freshness, confirmation, and response behavior remain unchanged.
  • Add focused regression coverage and a dependency-free OG preview CI job. No scanner rules were disabled and no alert suppressions were added.

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 named LogInjectionPayload, 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

Code-scanning alerts Fix
39 OG network policy, canonical addresses, DNS pinning, and redirect authorization
40 Explicit HTTP(S) validation at the navigation sink
41, 60 Inline-module recognition, including whitespace and attributes on closing script tags
42 Parser-based extraction of real lifecycle controllers in tests
43, 61 Escape Markdown table delimiters and adjacent backslashes together
44 Inert search excerpt text extraction
45 Remove redundant heading tag filter; preserve slug allowlist
46, 47, 48 Fixed public OG error responses
49, 50, 51 Sanitized timestamp and replay/in-progress message-ID diagnostics
52, 53, 54, 55 Safe YouTube confirmation diagnostics
57, 58 Selected sanitized revocation fields and unknown-type diagnostics
62 Clarify the non-secret log-injection fixture name; retain credential-exclusion assertions

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):

  • Fresh CodeQL run: C# and JavaScript/TypeScript analyses both completed with 0 results and no analysis errors on merge commit 992f4ec4342447ad2f48481da084511433f4756d. Its parents confirm it includes current PR head 8944e69173c4f3214b964666f455ac9bc002d065.
  • Integration CI is still running; its results are not yet established for this head.
  • Focused .NET webhook tests: 73/73 passed, including preserved diagnostic fields, control-character handling, truncation, omitted callback URLs/credentials, and malformed revocation details.
  • API Markdown tests: 30/30 passed. Targeted type-aware ESLint and strict typechecking passed. All 801 existing enum-table cells match the original output.
  • git diff --check passed; generated icon-cache side effects were restored. Working tree clean after publication.

Earlier validation, retained for provenance only:

  • CodeQL run reported 0 results for both languages on head 643bb9c1de76ba1ec516596fad8b51ab34027c2e.
  • Full integration CI passed on that earlier head, including production frontend build, lint/unit tests, desktop/tablet/mobile Chromium E2E, AppHost build, reports, and CI gate. All 15 checks passed at that point.
  • OG Node regressions: 23/23 passed, covering DNS pinning, canonical addresses, redirects, capability isolation/revocation, proxy errors, module rewriting, and named/numeric localhost.
  • Headless browser checks passed for authorized browsing, denial of unrelated local origins, explicit reselection, preserved sandbox, blocked executable/file schemes, safe noopener navigation, and rejected forged messages. Six frontend excerpt fixtures passed without script execution or unexpected resource requests.
  • Extension syntax checks passed and the project OG preview extension reloaded successfully. The duplicate personal copy has an existing tool-name collision; no user configuration was changed.
  • No local frontend production build was run.

Default-branch alerts remain open until fixes land and default-branch analysis confirms closure.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Comment thread .github/extensions/og-preview/extension.mjs Fixed
Comment thread src/frontend/src/utils/api-markdown-shared.ts Fixed
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@aspire-repo-bot

Copy link
Copy Markdown
Contributor

Frontend HTML artifact ready

The latest frontend build uploaded the frontend-dist artifact for PR #1688. Use the VS Code button below to open this PR with GitHub Artifacts Explorer and browse the built HTML locally.

VS Code: Open PR #1688 artifacts

This comment updates automatically when a new frontend build artifact is uploaded.

Comment thread src/statichost/StaticHost/Live/Twitch/TwitchWebhookHandler.cs Outdated
Comment thread src/statichost/StaticHost/Live/LiveEndpoints.cs Outdated
Comment thread src/frontend/src/utils/api-markdown-shared.ts
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Comment thread src/statichost/StaticHost/Live/Twitch/TwitchWebhookHandler.cs Fixed
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@IEvangelist
David Pine (IEvangelist) marked this pull request as ready for review September 17, 2026 16:56
Copilot AI lite review requested due to automatic review settings September 17, 2026 16:56

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.

🟡 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.

Comment thread .github/extensions/og-preview/extension.mjs
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@IEvangelist

Copy link
Copy Markdown
Member Author

This regex also escapes pipes inside inline code spans ... renders as <code>left\|middle\|right</code>

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 left|middle|right, and the 30 focused Markdown tests pass. The separate case of a literal backslash immediately followed by a pipe inside code can display extra backslashes; that narrower limitation is explicitly documented in the PR and earlier review-thread reply. I retained the simpler helper requested in the human review rather than restoring the custom Markdown parser.

Comment thread src/statichost/StaticHost/Live/YouTube/YouTubeDiagnostics.cs Outdated
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@IEvangelist
David Pine (IEvangelist) merged commit 0a46b09 into main Sep 17, 2026
15 checks passed
@IEvangelist
David Pine (IEvangelist) deleted the ievangelist-code-scanning-fixes branch September 17, 2026 18:04
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.

4 participants