Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 44 additions & 0 deletions .github/scripts/generate-checksums.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
#!/usr/bin/env bash
# Generates a <file>.sha256 checksum file next to each given release
# artifact (FR-012). Uses sha256sum if present, falling back to
# `shasum -a 256` (macOS has no sha256sum by default) -- no additional
# dependency beyond what the runner/OS already provides, per plan.md's
# Key Design Decision #6.
#
# The checksum file references its artifact by basename, not the full
# invocation path: release assets are downloaded flat, side-by-side, so a
# checksum file baked with the build machine's absolute path would fail
# `sha256sum -c`/`shasum -a 256 -c` for every downloader.
set -euo pipefail

usage() {
echo "usage: $0 <file> [file...]" >&2
exit 2
}

[ $# -ge 1 ] || usage

# Detect the available hash tool once, not per-file, and fail fast with a
# clear message if neither exists (rather than each file silently hitting
# a "command not found" only when it's its turn to be hashed).
if command -v sha256sum >/dev/null 2>&1; then
sha_bin=(sha256sum)
elif command -v shasum >/dev/null 2>&1; then
sha_bin=(shasum -a 256)
else
echo "error: neither sha256sum nor shasum found" >&2
exit 1
fi

for file in "$@"; do
if [ ! -f "$file" ]; then
echo "error: $file not found" >&2
exit 1
fi
dir="$(dirname "$file")"
base="$(basename "$file")"
# `--`: without it, a basename starting with `-` (e.g. `-artifact`) would
# be parsed as an option by sha256sum/shasum instead of a filename.
(cd "$dir" && "${sha_bin[@]}" -- "$base") > "${file}.sha256"
echo "wrote ${file}.sha256"
done
144 changes: 144 additions & 0 deletions .github/scripts/tests/test-generate-checksums.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,144 @@
#!/usr/bin/env bash
# Fixture-based tests for generate-checksums.sh (T011, Constitution I). Run
Comment thread
rsenna marked this conversation as resolved.
# manually:
# bash .github/scripts/tests/test-generate-checksums.sh
set -euo pipefail

script_dir="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
script="$script_dir/../generate-checksums.sh"

pass=0
fail=0

check() {
local desc="$1" ok="$2"
if [ "$ok" = "0" ]; then
pass=$((pass + 1))
else
fail=$((fail + 1))
echo "FAIL: $desc"
fi
}

verify_checksum() {
# Portable sha256sum -c across macOS (shasum) and Linux (sha256sum).
# `--`: a dash-prefixed checksum filename (e.g. "-dash-artifact.sha256")
# would otherwise be parsed as an option, same footgun the script under
# test guards against for the artifact filename itself.
local checksum_file="$1"
if command -v sha256sum >/dev/null 2>&1; then
(cd "$(dirname "$checksum_file")" && sha256sum -c -- "$(basename "$checksum_file")") >/dev/null 2>&1
else
(cd "$(dirname "$checksum_file")" && shasum -a 256 -c -- "$(basename "$checksum_file")") >/dev/null 2>&1
fi
}

checksum_records_basename() {
# `sha256sum -c` only verifies that the hash matches SOME file with the
# recorded name -- it doesn't confirm that name is the one we expect. A
# bug that wrote e.g. second-artifact.sha256 with third-artifact's
# record (whose content still happens to exist and match) would pass
# verify_checksum silently. Cross-check the filename field itself.
local checksum_file="$1" expected_basename="$2" recorded
recorded="$(awk '{print $NF}' "$checksum_file")"
[ "$recorded" = "$expected_basename" ]
}

check_file_exists() {
local desc="$1" path="$2"
if [ -f "$path" ]; then
check "$desc" 0
else
check "$desc" 1
fi
}

tmp="$(mktemp -d)"
trap 'rm -rf "$tmp"' EXIT

# --- happy path: checksum file is created and verifies against its artifact ---
printf 'iklo release artifact fixture\n' > "$tmp/iklo-v0.1.0-x86_64-unknown-linux-gnu"
"$script" "$tmp/iklo-v0.1.0-x86_64-unknown-linux-gnu"
check_file_exists "checksum file created" "$tmp/iklo-v0.1.0-x86_64-unknown-linux-gnu.sha256"
if verify_checksum "$tmp/iklo-v0.1.0-x86_64-unknown-linux-gnu.sha256"; then
check "checksum verifies against its real artifact (sha256sum -c)" 0
else
check "checksum verifies against its real artifact (sha256sum -c)" 1
fi

# --- checksum file references the artifact by basename, not an absolute path ---
# (release assets are downloaded flat, side-by-side -- a checksum file
# containing the build machine's absolute path would fail `sha256sum -c`
# for every downloader.)
check "checksum file references artifact by basename, not absolute path" \
"$(grep -q "$tmp" "$tmp/iklo-v0.1.0-x86_64-unknown-linux-gnu.sha256" && echo 1 || echo 0)"

# --- negative: tampering the artifact after the fact must fail verification ---
printf 'tampered content\n' > "$tmp/iklo-v0.1.0-x86_64-unknown-linux-gnu"
if verify_checksum "$tmp/iklo-v0.1.0-x86_64-unknown-linux-gnu.sha256"; then
fail=$((fail + 1))
echo "FAIL: tampered artifact should NOT verify, but it did"
else
pass=$((pass + 1))
fi

# --- multiple artifacts in ONE invocation (exercises the for-loop, not
# just two separate single-arg calls) -- content-verified, not just
# existence, so a regression that attributes the wrong checksum to the
# wrong file (or writes an invalid one) would be caught. ---
Comment thread
coderabbitai[bot] marked this conversation as resolved.
printf 'second artifact\n' > "$tmp/second-artifact"
printf 'third artifact\n' > "$tmp/third-artifact"
"$script" "$tmp/second-artifact" "$tmp/third-artifact"
check_file_exists "second checksum file created (multi-arg call)" "$tmp/second-artifact.sha256"
check_file_exists "third checksum file created (multi-arg call)" "$tmp/third-artifact.sha256"
if verify_checksum "$tmp/second-artifact.sha256"; then
check "second artifact's checksum verifies (multi-arg call)" 0
else
check "second artifact's checksum verifies (multi-arg call)" 1
fi
if verify_checksum "$tmp/third-artifact.sha256"; then
check "third artifact's checksum verifies (multi-arg call)" 0
else
check "third artifact's checksum verifies (multi-arg call)" 1
fi
if checksum_records_basename "$tmp/second-artifact.sha256" "second-artifact"; then
check "second artifact's checksum records the correct basename (not third's)" 0
else
check "second artifact's checksum records the correct basename (not third's)" 1
fi
if checksum_records_basename "$tmp/third-artifact.sha256" "third-artifact"; then
check "third artifact's checksum records the correct basename (not second's)" 0
else
check "third artifact's checksum records the correct basename (not second's)" 1
fi

# --- dash-prefixed basename: regression test for the `--` fix (without
# it, sha256sum/shasum would parse "-artifact" as an option and fail) ---
printf 'dash-prefixed artifact\n' > "$tmp/-dash-artifact"
"$script" "$tmp/-dash-artifact"
check_file_exists "dash-prefixed-basename checksum file created" "$tmp/-dash-artifact.sha256"
if verify_checksum "$tmp/-dash-artifact.sha256"; then
check "dash-prefixed-basename checksum verifies" 0
else
check "dash-prefixed-basename checksum verifies" 1
fi

# --- missing argument fails ---
if "$script" >/dev/null 2>&1; then
fail=$((fail + 1))
echo "FAIL (expected failure but succeeded): no arguments"
else
pass=$((pass + 1))
fi

# --- nonexistent file fails ---
if "$script" "$tmp/does-not-exist" >/dev/null 2>&1; then
fail=$((fail + 1))
echo "FAIL (expected failure but succeeded): nonexistent file"
else
pass=$((pass + 1))
fi

echo "----"
echo "pass=$pass fail=$fail"
[ "$fail" -eq 0 ]
17 changes: 11 additions & 6 deletions .github/workflows/release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,9 @@ on:
tags:
- 'v[0-9]*'

# Least-privilege: through T009 this workflow only reads the repo -- it
# validates the tag, tests, and builds, but publishes nothing.
# Least-privilege: through T011 this workflow only reads the repo -- it
# validates the tag, tests, builds, packages, and checksums, but
# publishes nothing.
# `contents: write` is deferred until T012, the task that actually creates
# the GitHub Release -- granting it here would hand a compromised build
# step needless write access to the repo via the default token.
Expand All @@ -17,6 +18,11 @@ jobs:
release:
name: Release
runs-on: ubuntu-latest
env:
# Single platform for now, per spec.md's Assumptions; a matrix
# build is a later expansion. Job-level so the packaging and
# checksum steps can't drift out of sync with each other.
TARGET: x86_64-unknown-linux-gnu
steps:
- name: Checkout
uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
Expand Down Expand Up @@ -61,10 +67,9 @@ jobs:
# consume. Per plan.md's Key Design Decision #7, the actual GitHub
# Release is created once, atomically, in T012 -- this step only
# prepares the asset; it does not create or upload to a Release.
# Single platform for now (ubuntu-latest / ${{ env.TARGET }}), per
# spec.md's Assumptions; a matrix build is a later expansion.
env:
TARGET: x86_64-unknown-linux-gnu
run: |
mkdir -p dist
cp target/release/iklo "dist/iklo-${GITHUB_REF_NAME}-${TARGET}"

- name: Generate SHA-256 checksums (FR-012)
run: .github/scripts/generate-checksums.sh "dist/iklo-${GITHUB_REF_NAME}-${TARGET}"
49 changes: 44 additions & 5 deletions specs/005-ci-release-versioning/tasks.md
Original file line number Diff line number Diff line change
Expand Up @@ -217,11 +217,50 @@ created with the packaged CLI binary attached.
exact packaging commands from the workflow (with
`GITHUB_REF_NAME=v0.1.0` standing in for the real tag-push env var),
confirmed the staged file is present, executable, and correctly named.
- [ ] **T011** [US2] Generate and publish SHA-256 checksums for every
release artifact (FR-012, SC-007). Include a test verifying each
published checksum file actually matches its artifact's content (e.g.
`sha256sum -c` against the built binary), not just that a checksum file
exists.
- [x] **T011** [US2] Generate SHA-256 checksums for every release
artifact (FR-012, SC-007). Actual publishing (upload to a GitHub
Release asset) happens in T012's atomic release-creation call, per
plan.md's Key Design Decision #7 -- same staging-now/publish-in-T012
split as T010. Include a test verifying each generated checksum file
actually matches its artifact's content (e.g. `sha256sum -c` against
the built binary), not just that a checksum file exists.
**Done 2026-08-13**: `.github/scripts/generate-checksums.sh` --
`sha256sum` if present, `shasum -a 256` fallback (macOS has no
`sha256sum` by default, detected once up front, not per-file), no
additional dependency, per Key Design Decision #6. Writes
`<file>.sha256` referencing the artifact by basename (not the build
machine's absolute path -- release assets download flat, side-by-side,
so an absolute-path checksum file would fail verification for every
downloader). Test-first (Constitution I):
`.github/scripts/tests/test-generate-checksums.sh` written and
confirmed red (script didn't exist) before implementing; green after
(14/14 assertions) -- happy path, checksum-references-basename-not-path,
a genuine `sha256sum -c` verification against real content, a negative
case (tampered artifact must fail verification), multiple artifacts in
ONE invocation with each one's checksum individually content-verified
AND each checksum file's recorded filename field individually asserted
correct (not just that `-c` passes and not just file existence --
catches the for-loop attributing the wrong checksum to the wrong
file), a dash-prefixed-basename regression case, missing-argument and
nonexistent-file failure cases.
**Fix after cubic-dev-ai review**: a basename starting with `-` (e.g.
`-artifact`) was parsed as an option by `sha256sum`/`shasum` instead of
a filename -- added `--` before the filename in both the script and the
test helper's own verification call (the test helper had the identical
bug, caught only once the dash-prefixed regression case was added).
**Fix after coderabbitai review**: `sha256sum -c` only confirms the
hash matches *some* file with the recorded name, not that the name is
the one actually expected -- a `checksum_records_basename` helper now
cross-checks each `.sha256` file's recorded filename field directly,
so a bug that attributed one artifact's record to another's file (with
content that happened to still verify) would be caught. Also replaced
every `$([ -f ... ]; echo $?)` command-substitution existence check
with an explicit `check_file_exists` helper for readability/consistency
with the rest of the suite's if/else style.
Wired into `release.yml` immediately after T010's packaging step.
Verified end-to-end locally against the real release binary: built,
packaged, checksummed, then `shasum -a 256 -c` against the generated
file reported `OK`.
- [ ] **T012** [US2] Ensure any failure — tag format, build, tests,
packaging, checksum, or note generation — stops the workflow and avoids
publishing a partial/invalid release (FR-008); reject re-publishing an
Expand Down