Repository navigation
Route release jobs through the ca-trust-guard + locked-tools composite actions - #159
Conversation
Superseded — see the current review report for commit
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| if [ "$PHYS_BAD" = 1 ]; then summary=STILL_TRUSTS_COMPROMISED; [ "$MITIGATED" = 1 ] && summary=PHYSICAL_STORE_TRUSTS_BUT_MITIGATED_BY_CLEAN_BUNDLE; fi | ||
| [ "$ERR" = 1 ] && summary=ERROR | ||
| echo "sh10-probe {\"image\":\"$img\",\"os\":\"$os\",\"summary\":\"$summary\",\"mode\":\"$SH10_MODE\",\"apply_distrust\":\"$SH10_APPLY_DISTRUST\",\"residual\":\"$([ "$PHYS_BAD" = 1 ] && echo physical-store-trusts-chain || echo none)\",\"not_covered\":\"action-tarball-downloads-before-step-1;runner-agent-dotnet-client;go(version-gated>=1.27.3);steps-running-arbitrary-node-without-step-level-NODE_OPTIONS\"}" | ||
| if [ "$ERR" = 1 ]; then echo "::error title=SH10::probe ERROR (fixture/store problem) — failing closed"; exit 1; fi |
There was a problem hiding this comment.
🟡 Suggestion (confidence: medium): this exit runs before the SH10_MODE check, so in report mode any probe ERROR still fails all 9 release jobs. Image drift is enough to trigger it: a store that goes missing (e.g. brew's etc/ca-certificates/cert.pem at L173), a changed OpenSSL error string, or a moved Git-Bash OPENSSLDIR. That conflicts with "enforcement is OFF". Consider emitting ::warning and continuing when SH10_MODE=report, and failing closed only under enforce.
| - name: SH10 runner trust-store probe (offline v3) | ||
| shell: bash | ||
| env: | ||
| SH10_MODE: report # report | enforce |
There was a problem hiding this comment.
🟡 Suggestion (confidence: high): this ~100-line probe is pasted 9 times, so switching to freeze or interim-residual means editing 18 values in sync. Miss one copy and enforcement silently differs per job. Because every job already checks out _workflows at the pinned SHA, the script could live as a hash-manifested .sh10/probe.sh, with the mode set once in workflow-level env:.
| apply_distrust() { | ||
| # Preferred mechanism (SH10-R2): point OpenSSL-based clients at a COMMITTED, hash-verified Mozilla bundle that already lacks | ||
| # the two roots (clean-ca-bundle.pem, checked by SHA256SUMS above). No download at job start, no sudo, inherited by later steps. | ||
| B="$PWD/clean-ca-bundle.pem" |
There was a problem hiding this comment.
🟡 Suggestion (confidence: medium): the committed snapshot (Mozilla as of Oct 20 2026) becomes the whole trust store for later curl/git/Python/AWS/npm traffic, and for Go on Linux via SSL_CERT_FILE. Nothing checks its age, so future Mozilla removals won't apply and new roots will be missing. Also, docker push from dockerd keeps using the system store and isn't listed in not_covered. Consider a max-age check on the bundle header and documenting the dockerd gap.
| uses: actions/checkout@v5 | ||
| with: | ||
| repository: ConductorOne/github-workflows | ||
| ref: ${{ job.workflow_sha }} |
There was a problem hiding this comment.
🟡 Suggestion (confidence: low): the PR says this uses the "same pin/ref as the existing _workflows checkouts", but every other job in this file pins via needs.determine-workflows-ref.outputs.ref (from get-workflow-version-action). This job already needs: determine-workflows-ref, so using that output keeps a single pin source. Otherwise, if job.workflow_sha ever resolves empty, checkout silently falls back to the default branch.
Superseded — see the current review report for commit
|
gontzess
changed the title
Superseded — see the current review report for commit
|
| - name: SH10 runner trust-store probe (offline v3) | ||
| shell: bash | ||
| env: | ||
| SH10_MODE: enforce # report | enforce |
There was a problem hiding this comment.
🟡 Suggestion: This commit sets SH10_MODE: enforce in all 9 probe copies. The PR description still says the initial values are report + distrust, and that enforce comes later "after a few days of clean summaries". The cited on-runner canary (run 39645149700) also ran report+distrust, so enforce on real runners has only been smoke-tested off-runner. Please either update the PR description and canary evidence to match the enforce rollout, or revert to report (docs/release-workflow.md now says enforce ships first).
Superseded — see the current review report for commit
|
Superseded — see the current review report for commit
|
| export SSL_CERT_FILE="$B" CURL_CA_BUNDLE="$B" GIT_SSL_CAINFO="$B" REQUESTS_CA_BUNDLE="$B" AWS_CA_BUNDLE="$B" NPM_CONFIG_CAFILE="$B" | ||
| git config --global http.sslCAInfo "$B"; git config --global http.sslVerify true | ||
| # Linux dockerd: per-registry CA (daemon ignores job env). Hosts the release path pushes to / pulls from. | ||
| if [ "$os" = Linux ] && command -v docker >/dev/null && [ -d /etc/docker ]; then |
There was a problem hiding this comment.
🟠 Bug: dockerd treats /etc/docker/certs.d/<host>/*.crt as roots it adds on top of the system pool (moby registry.ReadCertsDirectory appends them to tlsconfig.SystemCertPool()). So chains through the compromised intermediates that end at a system root still verify, and this leg removes no trust at all. The release image builds also go through setup-buildx-action (docker-container driver), so BuildKit pushes never read host certs.d. Either drop this leg and keep "dockerd" in not_covered with the docs saying so, or replace the daemon's system trust store and prove VERIFIED-DISTRUST against the fixture chains (high confidence).
Superseded — see the current review report for commit
|
Superseded — see the current review report for commit
|
| export SSL_CERT_FILE="$B" CURL_CA_BUNDLE="$B" GIT_SSL_CAINFO="$B" REQUESTS_CA_BUNDLE="$B" AWS_CA_BUNDLE="$B" NPM_CONFIG_CAFILE="$B" | ||
| git config --global http.sslCAInfo "$B"; git config --global http.sslVerify true | ||
| # Linux dockerd: per-registry CA (daemon ignores job env). Re-run this script after any later Docker provisioning step. | ||
| if [ "$SH10_MODE" = enforce ]; then bash ./dockerd-certs.sh "$B" || { echo "::error title=SH10::docker daemon mitigation could not be installed"; ERR=1; }; else bash ./dockerd-certs.sh "$B" || true; fi |
There was a problem hiding this comment.
🟠 Bug: Moving the leg into dockerd-certs.sh still doesn't remove any trust. Moby's registry.ReadCertsDirectory appends certs.d/<host>/*.crt to tlsconfig.SystemCertPool(), so eMudhra chains that end at a system root still verify. The image pushes also run through docker/setup-buildx-action (L1785, docker-container driver), and BuildKit never reads the host's certs.d. Enforce mode now fails releases (ERR=1) if this step can't install, so releases are gated on a mitigation that does nothing. The docs at docs/release-workflow.md:212 also still say it "covers daemon-side TLS". Either drop the leg and list dockerd/BuildKit under not_covered, or add a mechanism that replaces the pool (for example, a buildkitd config with the clean bundle plus a replaced system bundle for dockerd).
Superseded — see the current review report for commit
|
![C1 Squire [DEV]](https://avatars.githubusercontent.com/in/2846581?s=40&u=9dfdc6298bb22a241aaf41a6558bc1563c5c6466&v=4){kind=avatar}
|
Thank you for the blocking-correctness catch on the dockerd |
General PR Review: Probe runner TLS trust stores and redirect release jobs to a verified clean CA bundleBlocking Issues: 0 | Suggestions: 4 | Threads Resolved: 0 Review SummaryThe new commit only changes The probe step now claims no coverage it lacks in the docs, so I've downgraded the remaining dockerd problems to suggestions. Those problems are telemetry that doesn't match the docs, and an enforce gate tied to a helper that does nothing. The helper itself is inert for TLS trust and is not a security regression. Security IssuesNone found. Correctness IssuesNone found. Suggestions
Resolved prior findings
Prompt for AI agents |
| summary=ok; [ "${UNK:-0}" -gt 0 ] && summary="ok-with-${UNK}-unknown-cause-rejections" | ||
| if [ "$PHYS_BAD" = 1 ]; then summary=STILL_TRUSTS_COMPROMISED; [ "$MITIGATED" = 1 ] && summary=PHYSICAL_STORE_TRUSTS_BUT_MITIGATED_BY_CLEAN_BUNDLE; fi | ||
| [ "$ERR" = 1 ] && summary=ERROR | ||
| echo "sh10-probe {\"image\":\"$img\",\"os\":\"$os\",\"summary\":\"$summary\",\"mode\":\"$SH10_MODE\",\"apply_distrust\":\"$SH10_APPLY_DISTRUST\",\"residual\":\"$([ "$PHYS_BAD" = 1 ] && echo physical-store-trusts-chain || echo none)\",\"not_covered\":\"action-tarball-downloads-before-step-1;runner-agent-dotnet-client;go(version-gated>=1.27.3);js-actions-and-node-steps-without-NODE_OPTIONS=--use-openssl-ca;dockerd-on-macOS/Windows;windows-node-bundled-roots\"}" |
There was a problem hiding this comment.
🟡 Suggestion: The docs now say Linux dockerd is not covered, but this summary line still lists only dockerd-on-macOS/Windows in not_covered. dockerd-certs.sh also still prints "distrust":"dockerd-certs.d". Add Linux dockerd/BuildKit to not_covered here and in the other 8 copies, so the runtime telemetry matches docs/release-workflow.md:60.
Superseded — see the current review report for commit
|
Superseded — see the current review report for commit
|
| chain whose root remains in the host store. The default Buildx | ||
| `docker-container` driver uses its own CA store, not host `certs.d`. | ||
|
|
||
| The mitigations that actually bind the image path are base-image digests |
There was a problem hiding this comment.
🟡 Suggestion (high confidence): Base images aren't actually pinned by digest. templates/.Dockerfile-template.tmpl:1 uses FROM gcr.io/distroless/static-debian13:nonroot and templates/.Dockerfile-lambda-template.tmpl:1 uses FROM public.ecr.aws/lambda/provided:al2023, both by tag only. So the "content-addressed pulls" control this paragraph (and the probe's not_covered text mitigated-by-image-digests+immutable-tags) relies on doesn't cover base-image pulls through dockerd/BuildKit. Either pin the template FROM lines with @sha256:, or say that base-image pulls are an unmitigated residual.
Superseded — see the current review report for commit
|
Superseded by a completed review with no blocking issues; current audit evidence persisted in CI.
… runner TLS trust for the incident CA chains (#160) ## Why Following the public disclosure of compromised eMudhra intermediate certificates, GitHub-hosted runner images may still trust those intermediates until patched images roll out (actions/runner-images#15018, open). This PR ships **`ca-trust-guard`**: a self-contained composite action any job in any repo can run as step 1 (no checkout required) that (a) measures whether the runner's trust stores still accept the incident chains, (b) **denies the six incident certificates in the OS trust stores**, and (c) redirects the job's TLS clients to a committed, hash-verified clean CA bundle — with an enforce mode that fails closed unless every mitigation is **verified where it was applied**. This is preventive hardening; it does not respond to any known compromise of ConductorOne release artifacts. ## What - **`.github/actions/ca-trust-guard/action.yml`** — five one-line glue steps (probe before → deny → apply bundle → probe after → summarize); inputs `mode` (report|enforce, default enforce), `apply-distrust` (default true), `os-deny` (default true), `os-deny-elevation` (fail|warn, default fail — non-elevated/self-hosted consumers opt into `warn` knowingly). - **`cmd/ca-trust-guard`** — a stdlib-only Go command (built offline on the runner's own Go via a module-off bootstrap; PATH → toolcache → brew discovery). OpenSSL is the chain **measurement** instrument by design: Go ≥1.27.3's `crypto/x509` applies the incident SPKI distrust *unconditionally*, so a Go-native verify would read every store as distrusting and could not measure anything (a living-doc test asserts this; the README records the reasoning). Orchestration, `chains.json`, the single v3.11-tested classifier, `encoding/json` rows (openssl path+version logged per row), and summarize/enforce are Go. The positive control (every chain must verify against the pinned vulnerable roots) runs first and is binding. - **`scripts/deny-os.sh` / `deny-os.ps1`** — data-driven from `fixtures/incident-certs.json`: **4 incident roots** (emSign ECC/TLS/CS Root CA - G3, AAA Certificate Services) + **2 compromised intermediates** (EM DV TLS CA - G3B and its AAA cross-signed variant — the certificates whose keys actually leaked), each pinned by sha1+sha256 with refusal-before-mutation. Linux: `/etc/ca-certificates.conf` blacklist + `update-ca-certificates` + compiled-bundle verification. macOS: `security add-trusted-cert -d -r deny` into System.keychain (roots and intermediates). Windows: import into `Cert:\LocalMachine\Disallowed` (skipped with an explicit row when not elevated; `os-deny-elevation: fail` makes that a hard failure). - **`scripts/apply-clean-bundle.sh`** — the client-env leg (SSL_CERT_FILE/CURL_CA_BUNDLE/GIT_SSL_CAINFO/REQUESTS_CA_BUNDLE/AWS_CA_BUNDLE/NPM_CONFIG_CAFILE; Windows native-path git/curl only). - **Enforce post-condition:** a green job is *proof*, not attempt — clean-bundle rows VERIFIED-DISTRUST **and** per-OS applied-store evidence (Linux system bundle; macOS SecTrust; Windows both intermediate thumbprints in Disallowed). Residuals stay measured and reported (macOS `/etc/ssl`/brew, dockerd/BuildKit — see #159 for why the daemon class isn't coverable; the image path is bound by digest pins + immutable tags). - Fixtures: 10-payload `SHA256SUMS` (8 original + `chains.json` + `incident-certs.json`), LF-pinned. Tests: Go table tests (real-OpenSSL stores, classifier, every mode/flag/elevation combination, chains loading) + shell suites (deny dry-run/refusal/elevation/idempotency; bootstrap discovery incl. CRLF regressions), wired into verify.yaml + make test-scripts. - **Attribution:** the OS-level deny mechanics, fingerprint-keyed refusal-before-mutation, Linux verification loop, and the verify-where-applied enforce principle are adapted from gsilvestrin's ductone/multipass#951 — thank you. Follow-up (LOW): extend the deny set to all 16 impacted intermediates once their PEMs are obtained from CT logs by sha256. ## Verification - reviewer-claude gate (full review): stdlib-only Go with the v3.11 classifier contract; six-identity data-driven denial with dual-fingerprint refusal before mutation on all three OSes (all sha1/sha256 independently recomputed); five one-line glue steps; fixtures checksum-clean under an autocrlf=true clone; 12 Go tests / 646 subtests + both shell suites pass. - **Cross-repo canary** (`uses: …ca-trust-guard@<sha>` from baton-github-test): run 39667466967 surfaced two real bugs (macOS runner has no Go on PATH → toolcache/brew discovery; Windows native jq CRLF emission → normalization + CR-stripping + regression tests); fixed and re-run — [39669995046](https://github.com/ConductorOne/baton-github-test/actions/runs/39669995046) **all three OSes green**; re-confirmed at the rebased-on-main tip by [39672516187](https://github.com/ConductorOne/baton-github-test/actions/runs/39672516187) (branch now contains #157/#158; repo CI green incl. the image-lock gate): Windows log shows all 6 identities `DENIED … in Disallowed` with AFTER rows VERIFIED-DISTRUST via the natural thumbprint evidence; macOS SecTrust denied (two independent denies); Linux system bundle blacklisted; hosted Windows elevation confirmed. --------- Co-authored-by: c1-squire-dev[bot] <c1-squire-dev[bot]@users.noreply.github.com>
281977c to
1bdffc6
Compare
Superseded — see the current review report for commit
|
gontzess
changed the title
| runs-on: ubuntu-latest | ||
|
|
||
| steps: | ||
| - name: Enforce runner CA trust |
There was a problem hiding this comment.
🟡 Suggestion: The PR title and body don't match this diff. They describe an inline .sh10/ probe copied into 9 jobs, SH10_MODE/SH10_APPLY_DISTRUST env, sparse .sh10/ checkouts and a .sh10/** -text gitattributes entry, but none of that exists at head. The diff instead adopts the already-merged ca-trust-guard and locked-tools actions pinned at 99f9b2fc and deletes the scripts/ wrappers. Please update the title and body so reviewers and the merge history describe what actually ships. (Confidence: high.)
Superseded — see the current review report for commit
|
Superseded — see the current review report for commit
|
|
|
||
| steps: | ||
| - name: Enforce runner CA trust | ||
| uses: ConductorOne/github-workflows/.github/actions/ca-trust-guard@4d06ff31da09d2012770879c2c43c54dec24fe76 |
There was a problem hiding this comment.
🟡 Suggestion: This commit moves the pins to ca-trust-guard@4d06ff31 (#164) and locked-tools@562a1ea6 (#163), and I confirmed both commits are on main. The PR body still says both actions are at @99f9b2fc and points to the #160/#161 canaries as evidence. Please update the body to the new pins and link canary evidence for the #163/#164 code that now ships.
Superseded — see the current review report for commit
|
) ## Why Caller Go gates must use the version and lock identity returned by the locked-tools action, not rediscover expectations from a separate workflow checkout. The action remains the single owner of lock metadata parsing. ## What - Add `lock_path`: the absolute path of the exact lock used (action-local canonical file by default, resolved reviewed override when supplied). Preserve per-tool outputs, `versions`, and `lock_sha256`. - Wire expected Go version, path, and hash through prepare/install/source-build/result phases. Keep cross-platform single-stable-Go-version (`sort -u`) sanity in prepare. - Make `verify-locked-go.sh` consume `EXPECTED_GO_VERSION`, `EXPECTED_LOCK_SHA256`, and `LOCK_PATH`. It hashes bytes to check identity but never parses lock versions. Missing/partial expectations, drift, nonlocal toolchain mode, and version mismatch fail closed. - Restore runtime/module assertions: directive checks first, runtime version equality, then readonly/offline `GOTOOLCHAIN=local go list -m` with workspace discovery disabled. - Remove the repository Go-gate wrapper, positional lockfile mode, gon adapter and their compatibility fixtures. Go gates are strictly environment-only; positional arguments cannot supply missing expectations. **Intentional staged rollout (verifiable):** main-tip release.yaml's 12 old wrapper calls are unresolved until the coordinated caller amendment lands in #159 (branch steve.gontzes/sh10-probe-report). No live release path is affected: the v4 tag still points at e643283 (pre-dates these actions), every sampled connector consumes @v4, and code search finds zero @main/@sha callers; the failure mode in the window is fail-closed (file-not-found / missing env), never a silent pass. Do not promote v4 between this merge and #159 landing. ## Verification - `make test-scripts`: pass, including 27 integrity fixtures (one native-Windows-only skip locally), 23 image/update tests, workflow contracts, and both guard Bash suites. - `go vet ./...` and `go test -count=1 ./...`: pass with local/offline settings. - Real local Go smoke: output-derived env contract checks directives 1.13, 1.27, and 1.27.3 using actual offline `go list -m`. - Fixtures cover absolute/existing `lock_path`, preserved version/hash outputs, metadata-reader ownership, missing env, hash drift, runtime mismatch, go-list failure/disagreement, and rejection of positional lockfile fallback. - Shell syntax, image-lock gate, and `git diff --check`: pass. `.github/workflows/release.yaml` is byte-unchanged. --------- Co-authored-by: c1-squire-dev[bot] <c1-squire-dev[bot]@users.noreply.github.com>
Bootstrap guarded jobs before checkouts, retain the failure-notification exemption and conditional preflight tooling, and preserve module gates through action-local helpers. Retire forwarding wrappers and keep ordering regressions enabled for release-only changes. Co-authored-by: c1-squire-dev[bot] <c1-squire-dev[bot]@users.noreply.github.com>
Use distinct immutable guard and locked-tools refs from main, and align the expected-pin assertions and bootstrap documentation without changing job behavior. Co-authored-by: c1-squire-dev[bot] <c1-squire-dev[bot]@users.noreply.github.com>
Load local trust and tool actions from job.workflow_sha and bind module gates to installed version and lock-identity outputs. Keep optional preflight output parsing skip-safe without a version fallback, and preserve caller verification before release credentials. Co-authored-by: c1-squire-dev[bot] <c1-squire-dev[bot]@users.noreply.github.com>
7d2f6b8 to
a096b3e
Compare
Superseded — see the current review report for commit
|
| persist-credentials: false | ||
|
|
||
| - name: Enforce runner CA trust | ||
| uses: ./_workflows/.github/actions/ca-trust-guard |
There was a problem hiding this comment.
🟡 Suggestion: The PR body says both actions are pinned at @99f9b2fc and that the guard is "step 1 … before any checkout or credential". After a096b3e the diff has no remote action pins. Each job now runs ./_workflows/.github/actions/* from a _workflows checkout at job.workflow_sha, so the guard is step 2. The #160/#161 canaries cited as evidence tested the remote-pinned form. Please update the body, and link the release canary for this local-checkout form before merge. (High confidence.)
| Nine release jobs first check out `_workflows` at `${{ job.workflow_sha }}` | ||
| with a SHA-pinned checkout action, `persist-credentials: false`, and no token | ||
| input. This read-only infrastructure checkout provides the workflow's own | ||
| action implementations and canonical lock; it does not execute caller code. |
There was a problem hiding this comment.
🟡 Suggestion: "no token input" doesn't mean no credential. actions/checkout's token input defaults to ${{ github.token }}, and it is sent as a git auth header when fetching _workflows. That fetch runs before ca-trust-guard, under the unguarded trust store. The fetch is bound to the commit SHA, so content integrity holds. But the caller's read-scoped GITHUB_TOKEN still goes over a pre-guard TLS connection. The test's "github.token" in str(step) check can't see that implicit default. This isn't a regression, since the old checkouts were also unguarded. Either document this residual, or verify the commit with an unauthenticated fetch, e.g. token: '' where the repo is public. (Medium confidence.)
Superseded — see the current review report for commit
|
Explain the read-scoped GitHub token, non-persistence, pre-guard fetch, and workflow-commit integrity binding without changing checkout inputs or tests. Co-authored-by: c1-squire-dev[bot] <c1-squire-dev[bot]@users.noreply.github.com>
Superseded — see the current review report for commit
|
General PR Review: Route release jobs through the ca-trust-guard + locked-tools composite actionsBlocking Issues: 0 | Suggestions: 0 | Threads Resolved: 0 Review SummaryNew commit: Whole PR: Nine
The 6 Go module gates take their version, lock hash, and lock path from the locked-tools outputs, and the outputs they use ( Coverage: I scanned the full PR diff for security and correctness. Every guarded job declares Security IssuesNone found. Correctness IssuesNone found. SuggestionsNone. Resolved prior findings
Reviewed commit: |
Why
GitHub-hosted runner images may still trust the compromised eMudhra intermediates until patched images roll out (actions/runner-images#15018). Release jobs must (a) measure, deny, and redirect TLS trust and (b) install only hash-locked tools — before any credential. Both mechanisms ship as composite actions in this repo (ca-trust-guard, locked-tools); this PR adopts them in release.yaml and removes the per-job inline installs.
What
_workflowscheckout atjob.workflow_sha(persist-credentials: false; validate-inputs gainscontents: read(checkout uses the default read-scoped token; not persisted)), then runs ca-trust-guard and, in the 6 tool-using jobs, locked-tools via local paths (./_workflows/.github/actions/...). Workflow, action code, fixtures, and canonical tool lock are always the same commit — no self-referential action SHAs (enforced by a CI assertion)._workflowscheckout → guard → locked-tools (id: tools; preflight's stays conditional on the Go checks) → caller checkout + event-SHA verification → secrets. notify-release-failure remains exempt, with no bootstrap there.verify-locked-go.sh modulewith expected version, lock hash, and lock path taken from the locked-tools step's outputs (one reader of the lock per job); standalone version gates and the inline gon step are removed (the action asserts the runtime version; gon builds inside the action).install-tool.{sh,ps1},gon-source.sh,verify-locked-go.sh) are deleted;verify-release-checkout.sh,setup-release-buildx.sh,derive-iam-role-name.shstay.ci.yamlcovers release.yaml;test-tool-integrity.pyenforces local-path ordering, output-bound gate env, tool lists, the notify exemption, and the no-self-SHA rule.Evidence
./_workflows/.github/actions/*local-path resolution, the checkout → guard → tools ordering, and thesteps.tools.outputsgate env were exercised end-to-end (GUARD_ROOT resolves under_workflows; isolation self-check VERIFIED-DISTRUST per runner). No remote self-SHA pins remain (enforced by test).Staged rollout: safe to promote
v4only after this lands — see #165 for the window details.