Skip to content

Investigate code scanning alert #63 (SSRF) — confirmed false positive, no code changes - #1691

Closed
David Pine (IEvangelist) with Copilot wants to merge 1 commit into
mainfrom
copilot/fix-code-scanning-alerts-63
Closed

David Pine (IEvangelist) with Copilot wants to merge 1 commit into
mainfrom
copilot/fix-code-scanning-alerts-63

Conversation

Copilot AI commented Sep 17, 2026 •

Copy link
Copy Markdown
Contributor

CodeQL flagged alert #63 (js/request-forgery) in .github/extensions/og-preview/lib/http-fetch.mjs:152-222, reporting that the URL used in fetchUrl()'s outgoing request depends on user-provided input.

Investigation

  • Reviewed http-fetch.mjs end-to-end, focused on the flagged lib.request(parsed, ...) call in the visit() helper.
  • Confirmed the module already implements the exact SSRF mitigations CodeQL's own rule guidance recommends, plus additional defense-in-depth:
    • parseTarget() restricts URLs to http:/https: and rejects embedded credentials.
    • resolveAddresses() resolves DNS explicitly and validates the returned records.
    • addressKind() classifies every resolved address as public, private, or loopback (IPv4 + IPv6, including IPv4-mapped addresses).
    • authorizePreview() requires an explicit OG_ALLOW_PRIVATE_NETWORK opt-in before any private-network origin is usable, and never trusts discovered/redirected URLs.
    • Before every request — including each redirect hop — hostnames are freshly resolved and re-validated against addressKind(); requests targeting non-public, unauthorized destinations are rejected.
    • The actual http(s).request() call pins the connection to the pre-validated addresses via a custom lookup callback, defeating DNS-rebinding attacks between validation and connection time.
  • Confirmed existing tests in tests/http-fetch.test.mjs already exercise these SSRF scenarios (loopback/private addresses, mixed-scope DNS answers, redirect-based bypass attempts) and assert rejection.

Conclusion

Alert #63 is a false positive. CodeQL's taint-tracking query flags any user-influenced URL reaching a network-request sink, but doesn't model this module's custom multi-step sanitizer (DNS pre-resolution + IP-range allow-listing + address-pinned request + per-redirect re-validation). No code changes were made since the flagged path is already correctly mitigated.

Copilot AI changed the title [WIP] Fix code scanning alert(s) flagged in repository Investigate code scanning alert #63 (SSRF) — confirmed false positive, no code changes Sep 17, 2026
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