mirror of
https://github.com/shankar0123/certctl.git
synced 2026-06-07 22:01:36 +00:00
shankar0123
e06447b763
Reverts:482e952ci(codeql): rewire local model pack discovery — fix1122f5asilent no-op1122f5aci(codeql): teach analyzer about ValidateSafeURL SSRF barrier Net: drops .github/codeql/ entirely; restores the codeql.yml workflow and the docs/architecture.md::Input Validation and SSRF Protection section to their pre-1122f5a state. Alert #23 (go/request-forgery, Critical) at internal/service/scep_probe.go:232 stays OPEN to be resolved later. Why this revert exists. The original Option A (model pack barrier declaration) was the right idea on paper — teach the analyzer that internal/validation.ValidateSafeURL sanitizes the URL argument so the request-forgery taint trace stops there. Two iterations in (1122f5a+482e952), the pack still wasn't loading: -1122f5aused `packs: { go: ['./'] }` in codeql-config.yml. That field expects pack names, not paths; the local pack silently never registered. CodeQL ran clean but emitted the same alert. -482e952restructured into .github/codeql/certctl-models/ + named the pack + added `additional-packs: .github/codeql` to the action init step. Surface looked correct against the pattern I'd researched (vscode-codeql, CodeQL docs). But: Warning: Unexpected input(s) 'additional-packs', valid inputs are [..., packs, ...] A fatal error occurred: 'shankar0123/certctl-models' not found in the registry 'https://ghcr.io/v2/'. `additional-packs` is not a valid input on github/codeql- action/init@v3 (verified directly against init/action.yml on that branch). Without a valid path-resolver input, the CLI fell back to the public registry, where the pack obviously isn't published. CodeQL run #56 fatal-errored. The next iteration would have been: codeql-workspace.yml at the repo root, OR convert to a query pack referenced via `queries: ./path`, OR publish to GHCR, OR drop MaD and write custom QL. Each is its own incremental commit with its own failure modes I can't pre-validate without a CI push, against a `barrierModel` feature for Go that's too new (added 2026-04-21) to have shipped public examples to copy from. Honest cost-benefit. The runtime at scep_probe.go:232 is correct on day one — `ValidateSafeURL` rejects reserved-IP targets at the service entry; `SafeHTTPDialContext` re-resolves at dial time and pins to a literal non-reserved IP, defeating DNS rebinding. CodeQL is reporting a known-class false positive on a known-good sanitizer pattern. The cost of teaching CodeQL about a 2-site validator (this + webhook notifier's client.Do) — multiple iterations of pack-discovery infrastructure, a `.github/codeql/` tree to maintain, version-tracking against codeql-action and CodeQL-CLI updates — exceeds the benefit of silencing those 2 alerts. The right path forward, when capacity exists: either land a short justified `// codeql[go/request-forgery]` annotation at each of the 2 sites with a comment block citing ValidateSafeURL + SafeHTTPDialContext, OR dismiss alert #23 in the GitHub Security UI as "won't fix — false positive" with the same justification in the dismissal comment. Both are real fixes for the underlying problem (analyzer's model differs from runtime reality at known-safe call sites). Neither requires new CI infrastructure. Until then, the alert stays open. The Security tab is a public signal — anyone reviewing the certctl repo sees that we've left this finding visible rather than hidden it via config. That's itself a security-posture statement. Specific files restored: - .github/workflows/codeql.yml: drops `config-file:` and `additional-packs:` from Initialize CodeQL step. Workflow is byte-equivalent to its pre-1122f5a state (verified). - .github/codeql/: directory removed (3 files: qlpack.yml, codeql-config.yml, certctl-models/models/*.model.yml). - docs/architecture.md::Input Validation and SSRF Protection: drops the "Outbound HTTP egress" paragraph that was added in1122f5a. The original section's coverage of shell input validators + network-scanner reserved-IP filter remains intact — that's what was there before. Other commits between1122f5aand now (c4157fd— encryption-key fix + H-1 regression guard) are PRESERVED. They're unrelated to CodeQL and remain valid.
82 lines
3.1 KiB
YAML
82 lines
3.1 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
|
|
|
|
- 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).
|