Skip to content

Pin zstd build to immutable commit SHA, not just tag - #1

Open
smn wants to merge 2 commits into
masterfrom
pin-upstream-zstd-sha
Open

Pin zstd build to immutable commit SHA, not just tag#1
smn wants to merge 2 commits into
masterfrom
pin-upstream-zstd-sha

Conversation

@smn

@smn smn commented Jun 1, 2026

Copy link
Copy Markdown

Summary

  • Pin the zstd build to commit `f8745da6ff1ad1e7bab384bd1f9d742439278e99` (the commit `v1.5.7` currently resolves to) instead of trusting the mutable tag alone.
  • Add a supply-chain guard in `build_deps.sh` that aborts the build if upstream has moved the tag, so a re-tagging attack cannot slip a different commit into our NIF.
  • Keep `ZSTD_TAG="v1.5.7"` for readability; it is now verified against `ZSTD_SHA` before we check out and build.

Why

Git tags are mutable. If an attacker (or a well-intentioned maintainer) re-points `v1.5.7` at a different commit upstream, our previous `git checkout v1.5.7` would silently build that new commit. Pinning to an immutable commit SHA and verifying the tag-to-SHA mapping before building eliminates that vector.

What changed

  • `build_deps.sh`
    • New `ZSTD_SHA` constant alongside `ZSTD_TAG`.
    • `checkout_lib` now: fetches the pinned commit explicitly if missing, verifies the tag still resolves to it (aborts with a clear error if not), checks out by SHA (detached HEAD) instead of by tag, and re-verifies `HEAD` matches the pin as a belt-and-braces check.
  • `README.md` — documents the SHA pin, how to find the SHA for a new tag (`git ls-remote ... 'v1.5.7^{}'`), and that bumping zstd means updating both `ZSTD_TAG` and `ZSTD_SHA`.

Test plan

  • `bash -n build_deps.sh` — syntax OK.
  • Full `./build_deps.sh` run from a clean state — clones, checks out the pinned commit, `git rev-parse HEAD` matches the pin exactly, builds `libzstd.a` successfully.
  • Simulated a moved tag (set `ZSTD_SHA` to a different real commit while the tag still pointed at the original) — the guard fired and refused to build, with the expected error message.
  • CI green on this PR.

Notes for bumping zstd in future

```sh

Find the commit a tag resolves to:

git ls-remote https://github.com/facebook/zstd.git 'v1.5.8^{}'
```

Update both `ZSTD_TAG` and `ZSTD_SHA` in `build_deps.sh` in the same commit.

@smn
smn requested review from Aratramba and shaunsephton June 1, 2026 13:11
shaunsephton
shaunsephton previously approved these changes Jun 1, 2026
@smn

smn commented Jun 1, 2026

Copy link
Copy Markdown
Author

Have created a PR for this upstream as well silviucpp#26

@smn

smn commented Jun 1, 2026

Copy link
Copy Markdown
Author

going to leave this PR open until upstream ships (if it does) just to make sure we have better visibility on drift between the fork & upstream.

Adopt the maintainer's minimal structure: drop ZSTD_BRANCH and ZSTD_TAG
for a single immutable ZSTD_SHA, clone with --no-tags so no mutable tag
refs land on disk, and check out the pinned commit by SHA. Removes the
extra tag-verification/HEAD checks the maintainer asked to drop.

Pin stays on the real v1.5.7 commit
f8745da6ff1ad1e7bab384bd1f9d742439278e99 (the upstream comment's
794ea1b0... is the v1.5.7 merge's first parent, i.e. v1.5.6). A plain
clone fetches all branches, so the release commit is present for checkout
without needing --branch.
@smn

smn commented Jul 15, 2026

Copy link
Copy Markdown
Author

Upstream merged!

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