mirror of
https://github.com/shankar0123/certctl.git
synced 2026-06-07 17:41:29 +00:00
d8026d5f67
Closes CodeQL alert #23 (go/request-forgery, Critical) at the structural level — by telling CodeQL what the runtime code already does — rather than via per-line `// codeql[...]` suppressions. Background. internal/service/scep_probe.go:232 calls client.Do(req) where the request URL is built from operator-supplied input. The runtime defense is two-layer: 1. validation.ValidateSafeURL(rawURL) at scep_probe.go:86 rejects non-http(s) schemes, empty hosts, literal-IP hosts in reserved ranges (loopback, link-local incl. cloud metadata 169.254.169.254, multicast, broadcast, unspecified, IPv6 link-local), and DNS names whose A/AAAA resolution returns any reserved IP. RFC 1918 is intentionally NOT blocked — see internal/validation/ssrf.go:17-21 for the design rationale. 2. validation.SafeHTTPDialContext on the http.Transport (line 254) re-resolves at dial time, applies the same reserved-IP set, and pins the dial to a literal non-reserved IP — defeating DNS rebinding between validate and dial. CodeQL's go/request-forgery query is a syntactic taint-tracking rule with no built-in knowledge of either validator, so it reports the finding even though the runtime is correctly defended. The fix. Add a Models-as-Data (MaD) extension at .github/codeql/ declaring ValidateSafeURL as a request-forgery barrier. The barrier applies to Argument[0] (the URL parameter), which means the analyzer treats every URL flowing through ValidateSafeURL as sanitized for the request-forgery taint set. After this lands: - Alert #23 dismisses at scep_probe.go:232. - The same model applies to the second site of this exact shape — webhook notifier's outbound client.Do (internal/connector/ notifier/webhook/webhook.go) — without per-line annotations. - Future code that flows operator URLs through ValidateSafeURL inherits the barrier automatically. This is the structural fix, not a band-aid: - Band-aid (rejected): `// codeql[go/request-forgery]` suppression on line 232. Suppresses one alert; doesn't teach the analyzer. Webhook notifier would need the same comment when its sibling rule landing fires. - Structural (this change): teach CodeQL via models-as-data, in config checked into the repo, that lives next to the workflow that uses it. The validators ARE sanitizers in the runtime — this PR makes the analyzer's model match reality. Files: - .github/codeql/qlpack.yml — local model pack manifest, declares extensionTargets: codeql/go-all: '*' - .github/codeql/models/request-forgery-sanitizers.model.yml — barrierModel row for validation.ValidateSafeURL Argument[0] / request-forgery taint kind / manual provenance - .github/codeql/codeql-config.yml — references the local pack + keeps security-and-quality query suite scope - .github/workflows/codeql.yml — Initialize CodeQL step picks up config-file: ./.github/codeql/codeql-config.yml. The existing `queries: security-and-quality` line stays so even if the config file fails to load, the suite scope is preserved. - docs/architecture.md::Input Validation and SSRF Protection — extended to name the egress validators (ValidateSafeURL + SafeHTTPDialContext) and the call sites (SCEP probe + webhook notifier). Closes the docs gap surfaced during the audit; the egress threat-model previously lived only in source comments. Requires CodeQL CLI ≥ 2.25.2 for the barrierModel extensible predicate (Go MaD support added 2026-04-21). github/codeql-action@v3 ships a recent enough CLI by default; if a future analysis fails with "unknown extensible predicate barrierModel", the action's CLI has regressed below 2.25.2 — pin a newer action version rather than reverting this pack. Documented inline in qlpack.yml. References: - https://codeql.github.com/docs/codeql-language-guides/customizing-library-models-for-go/ - https://github.blog/changelog/2026-04-21-codeql-now-supports-sanitizers-and-validators-in-models-as-data/
91 lines
3.7 KiB
YAML
91 lines
3.7 KiB
YAML
name: CodeQL
|
|
|
|
# Public-facing SAST baseline that complements the existing security-deep-scan
|
|
# workflow (gosec, osv-scanner, trivy, ZAP, semgrep, schemathesis, nuclei,
|
|
# testssl) with cross-file Go and JavaScript dataflow analysis. Results land
|
|
# in the repository's Security → Code scanning tab as a public signal — any
|
|
# operator/security team auditing certctl can see the scan history and
|
|
# triage state without asking.
|
|
#
|
|
# Why CodeQL in addition to gosec:
|
|
# - gosec is single-file pattern matching (catches obvious issues like
|
|
# `os/exec.Command(userInput)`); CodeQL does interprocedural taint
|
|
# tracking (catches the same issue when the userInput is laundered
|
|
# through several function calls or struct fields).
|
|
# - GitHub-native; no third-party SaaS license gate (works for BSL 1.1
|
|
# and other source-available licenses, unlike Aikido / Snyk / SonarCloud
|
|
# free tiers which require OSI-approved licenses).
|
|
# - SARIF results auto-deduplicate and persist on PRs, so reviewers see
|
|
# "this PR introduces N new findings" rather than re-running ad hoc.
|
|
#
|
|
# Findings that are intentional (e.g., the SSH connector's
|
|
# InsecureIgnoreHostKey, ACME DNS solver's intentional shell-out to operator-
|
|
# supplied scripts) get suppressed via inline `// codeql[<rule-id>]`
|
|
# comments OR via a `.github/codeql/codeql-config.yml` query-pack tweak —
|
|
# document the rationale in the same commit that adds the suppression so
|
|
# the public scan-tab readers see the threat-model justification.
|
|
|
|
on:
|
|
push:
|
|
branches: [master]
|
|
pull_request:
|
|
branches: [master]
|
|
schedule:
|
|
# Weekly Sunday 06:00 UTC, in addition to push/PR coverage. Catches
|
|
# rule-pack updates from CodeQL upstream (their Go/JS rulesets ship
|
|
# new queries on a roughly-monthly cadence).
|
|
- cron: '0 6 * * 0'
|
|
|
|
permissions:
|
|
contents: read
|
|
security-events: write # SARIF upload to GitHub code scanning
|
|
actions: read
|
|
|
|
jobs:
|
|
analyze:
|
|
name: Analyze (${{ matrix.language }})
|
|
runs-on: ubuntu-latest
|
|
timeout-minutes: 30
|
|
strategy:
|
|
fail-fast: false
|
|
matrix:
|
|
language: [go, javascript-typescript]
|
|
|
|
steps:
|
|
- name: Checkout
|
|
uses: actions/checkout@v4
|
|
|
|
- name: Set up Go
|
|
if: matrix.language == 'go'
|
|
uses: actions/setup-go@v5
|
|
with:
|
|
# Match ci.yml + release.yml + security-deep-scan.yml.
|
|
go-version: '1.25.9'
|
|
|
|
- name: Initialize CodeQL
|
|
uses: github/codeql-action/init@v3
|
|
with:
|
|
languages: ${{ matrix.language }}
|
|
# Use the security-and-quality query suite — security finds plus
|
|
# maintainability/correctness issues that the smaller security-extended
|
|
# suite skips. Comparable scope to what Aikido / SonarCloud run.
|
|
queries: security-and-quality
|
|
# Custom config + model pack at .github/codeql/. The pack declares
|
|
# internal/validation.ValidateSafeURL as a request-forgery barrier
|
|
# via Models-as-Data, dismissing the alert that fires at every site
|
|
# using the validator (scep_probe.go, webhook.go) without per-line
|
|
# `// codeql[...]` suppressions. See .github/codeql/qlpack.yml for
|
|
# the full motivation. Requires CodeQL CLI ≥ 2.25.2 for the
|
|
# barrierModel extension; codeql-action@v3 ships a recent enough
|
|
# CLI by default.
|
|
config-file: ./.github/codeql/codeql-config.yml
|
|
|
|
- name: Autobuild
|
|
uses: github/codeql-action/autobuild@v3
|
|
|
|
- name: Perform CodeQL Analysis
|
|
uses: github/codeql-action/analyze@v3
|
|
with:
|
|
category: "/language:${{ matrix.language }}"
|
|
# SARIF upload is implicit (and is what populates the Security tab).
|