Skip to content

ci: build release tags without workflow caches - #138

Merged
andersalm merged 3 commits into
developfrom
fix/0.7.1-release-tag-builds
Oct 1, 2026
Merged

andersalm merged 3 commits into
developfrom
fix/0.7.1-release-tag-builds

Conversation

@andersalm

Copy link
Copy Markdown
Contributor

A manual dispatch on a version tag could publish a GitHub Release, and tag builds could reuse workflow caches. Publication now requires a push to a v* tag. Tag workflow refs and every nonempty manual checkout override build without workflow cache restore/save or buildx layer-cache import/export. The override rule also covers a branch-selected dispatch that checks out a tag.

Branch pushes, PRs, and manual dispatch without an override keep their existing caches. All jobs stay on GitHub-hosted runners with the same eight required check names and release dependencies. Kubo can still download and verify fresh bytes through its existing retry path.

Validation: three focused tests cover 12 event/ref cases across all nine cache actions and publication, execute the actual buildx shell with a recording Docker function, and check runner/name/dependency invariants. The tests run in source-gate. Actionlint 1.7.12 and the four required repo gates pass. Independent source review accepted commit 96d9e8d2; CI and Irzhy review remain merge gates.

Closes #137. Part of #93. This carries the release guard from #120 and adds cache-free tag builds. Release execution proof remains with #93; these checks create no tags or releases and perform no signing.

@irzhywau irzhywau left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Security review — #138 ci: build release tags without workflow caches

@andersalm — security review of head 96d9e8d2 against develop. Review only; no changes were made to the branch.

Verdict: approve the diff. It correctly closes the cache-poisoning path into release builds raised on #120 (F2), and the manual-dispatch publication hole. One Medium, pre-existing governance gap is adjacent to this PR's threat model (F1); it is not a blocker for this diff. F2–F3 are Low. A fix plan at the end separates what belongs in this PR, in follow-up PRs, and in repository settings.

Severity scale: Critical exploitable now with direct impact · High realistic path to code execution or artifact integrity loss · Medium needs an extra condition · Low hardening · Info no action.


F1 — Medium (pre-existing, outside the diff) · Release publication relies only on write access, and the published tarballs are unsigned

Where: ci.yml release job (if: github.event_name == 'push' && startsWith(github.ref, 'refs/tags/v'), permissions: contents: write). Repository state checked at review time:

  • GET /repos/Elacity/elastos-runtime/rulesets → [] (no tag rulesets);
  • legacy tag protection → none;
  • no deployment environments.

Threat. The publication gate is "a v* tag was pushed". Branch protection on main (1 review) does not cover tags, so an account with write access can tag a commit that never went through review. CI then builds it and publishes it as a public GitHub Release (v0.7.1 is currently Latest). The tarballs carry only a .sha256 produced by the same job, with no publisher-DID signature, so a downloader cannot tell a rogue release from a real one.

Why Medium, not High:

  • No new capability. On GitHub, the write role can already create, edit and delete releases and upload assets directly. So the CI tag path gives a write account, or a stolen write token, nothing it lacks today; it only makes such a release look CI-built.
  • Narrow audience. GitHub Releases are not the documented install or update channel. install.sh and elastos update use the signed Carrier release, which needs the publisher key and is unaffected.
  • The real gap is release governance plus unsigned public binaries, not this workflow.

Fix: a tag ruleset alone does not close this, because write users can still edit release assets by hand. Recommended:

  1. Sign what is published: sign the tarballs, or the .sha256 list, with the publisher DID or Sigstore, and document how to verify them. This is the change that makes a rogue upload detectable.
  2. Release environment: add environment: release with required reviewers to the release job. That gives a second human gate and an audit record for CI-built releases.
  3. Tag ruleset on refs/tags/v*: restrict creation, update and deletion to release managers, and block moving a tag. Note that manual release editing stays possible for the write role. Narrowing that means reducing who holds write access.
  4. Ancestry check in the release job:
    - uses: actions/checkout@v5
      with: { fetch-depth: 0 }
    - name: tag must be on main
      run: git merge-base --is-ancestor "$GITHUB_SHA" origin/main || { echo "::error::tag is not on main"; exit 1; }

F2 — Low (pre-existing) · Cache-free is not pinned: release jobs still run mutable third-party actions

Where: jobs in the release dependency set use tag-pinned actions: actions/checkout@v5, actions/setup-node@v4, dtolnay/rust-toolchain@1.91.0, actions/upload-artifact@v5, actions/download-artifact@v5, and softprops/action-gh-release@v2 (with contents: write).

Threat. Removing caches stops the cache path, but code executed in the release build can still change under the same tag if an upstream action is compromised or its tag is moved. That is the remaining supply-chain input into release binaries.

Fix: pin these actions to full commit SHAs with a version comment (uses: actions/checkout@<sha> # v5.x.y). Let Dependabot (package-ecosystem: github-actions) keep them current. At minimum, pin the actions used by source-home-linux, source-home-macos and release.


F3 — Low · The policy test can miss a new cache path

Where: scripts/ci-release-policy-test.py. Cache steps are found with uses: (Swatinem/rust-cache|actions/cache)@, guarded by the len(caches) == 9 assertion.

Threat. This test is the regression guard for the property "tag builds restore nothing". It would not notice a cache introduced in another form:

  • actions/cache/restore@… / actions/cache/save@…;
  • cache: input on actions/setup-node (or other setup-* actions);
  • docker/build-push-action cache-from/cache-to;
  • a second type=gha buildx invocation.

The test would stay green while a tag build silently restores state again.

Fix: widen the detector, and fail when any of these appear without the CI_USE_CACHE guard:

CACHE_RE = re.compile(r"uses: (Swatinem/rust-cache|actions/cache(/restore|/save)?)@|^\s+cache(-from|-to)?:|type=gha", re.M)

Add one negative self-test: an injected unguarded actions/cache/restore@v4 step must fail the check.

Note (Info): evaluate() uses eval with empty builtins. That is not a sandbox, but the input is this repo's own ci.yml, which already executes in CI with the same trust. Acceptable. Add a comment so it is never pointed at external input.


Checked and fine

  • CI_USE_CACHE is false for tag refs and for any manual dispatch with a non-empty ref override. That also stops a dispatch on main from writing main-scoped caches built from an arbitrary ref, which closes a second poisoning path.
  • All nine cache steps plus the buildx type=gha import/export are guarded. The test executes the real buildx shell with a stubbed docker. I ran the test locally: Ran 3 tests … OK.
  • Publication now needs github.event_name == 'push'; a manual dispatch on a v* ref can no longer publish.
  • All jobs stay on GitHub-hosted runners, and the release needs list and check names are unchanged and asserted.
  • Kubo is downloaded fresh on tags and verified by its existing checksum path.

Fix plan

In this PR (recommended before merge, small): widen the policy test (F3)

  • scripts/ci-release-policy-test.py: replace the cache-step detector with one that also matches actions/cache/restore@, actions/cache/save@, a cache: / cache-from: / cache-to: input on any step, and type=gha. Assert that every match is guarded by env.CI_USE_CACHE == 'true' (or CI_USE_CACHE in shell).
  • Add a negative self-test: inject an unguarded uses: actions/cache/restore@v4 step into an in-memory copy of the workflow and assert the check fails.
  • Add a comment on evaluate(): it uses eval and must only read this repo's own ci.yml.

Follow-up PR (author): pin actions in the release path (F2)

  • Pin to full commit SHAs, with a version comment, every action used by source-home-linux, source-home-macos and release: actions/checkout, actions/setup-node, dtolnay/rust-toolchain, actions/upload-artifact, actions/download-artifact, softprops/action-gh-release.
  • Add .github/dependabot.yml with package-ecosystem: github-actions so the pins stay current.
  • Extend ci-release-policy-test.py: every uses: in those three jobs must match @[0-9a-f]{40}.

Follow-up PR (author): tag ancestry guard (F1, workflow part)

  • In the release job, before download-artifact: check out with fetch-depth: 0 and run git merge-base --is-ancestor "$GITHUB_SHA" origin/main, failing otherwise.
  • Add a policy-test case asserting the step exists and precedes publication.

Repository settings (repo admin, not code) (F1)

  • A tag ruleset on refs/tags/v*: restrict creation, update and deletion to release managers, and block moving a tag.
  • A release deployment environment with required reviewers. Then add environment: release to the release job, which can go in the ancestry-guard PR.
  • Review who holds write access, since that role can still edit release assets by hand.

Design follow-up (issue under #93): signed GitHub Release assets (F1, root cause)

  • Sign the tarballs, or the .sha256 list, with the publisher DID (same key and trust as Carrier releases) or with Sigstore keyless signing tied to this workflow. Document the verification command next to the release notes.
  • Acceptance: a tampered asset uploaded by hand fails the documented verification.

Merge guidance: this PR can merge once its own CI is green. The F3 test widening is recommended in it. The other items are tracked separately and don't block it.

@andersalm
andersalm merged commit 2b93cfa into develop Oct 1, 2026
9 checks passed
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