diff --git a/README.md b/README.md index abb17f8..39cd981 100644 --- a/README.md +++ b/README.md @@ -54,6 +54,34 @@ The action exits successfully without committing when the staged diff is empty, so a reproducible build that produces identical bytes is a no-op rather than an empty commit. +### Outputs + +| Output | Description | +|---|---| +| `sha` | The oid of the commit, or empty when nothing was committed. | + +Use it to name the commit rather than resolving the branch again. GitHub can +still resolve a branch to its previous tip seconds after the mutation lands, +so a workflow dispatched on the branch NAME may check out the commit before +this one and act on the wrong tree: + +```yaml + - uses: pkghaus/signed-commit@v1 + id: commit + with: + workdir: . + branch: master + message: "..." + token: ${{ github.token }} + + - run: gh workflow run release.yml -f sha="$SHA" + env: + SHA: ${{ steps.commit.outputs.sha }} +``` + +An empty `sha` means the tree held no changes, which is not the same as a +commit that failed: a failure exits non-zero and emits nothing at all. + ### The token is required, with no default A composite action does not inherit the caller's secrets, and an input default diff --git a/action.yml b/action.yml index e8da8a2..3734663 100644 --- a/action.yml +++ b/action.yml @@ -25,10 +25,21 @@ inputs: default that silently resolved to the wrong token would be worse than a required argument. required: true +outputs: + sha: + description: >- + The oid of the commit this created, or empty when the tree held no + changes and nothing was committed. A caller that has to act on the + commit must name it with this rather than resolve the branch again: + GitHub can still resolve a branch to its previous tip seconds after the + mutation, and a workflow dispatched on that name then reads the wrong + tree. + value: ${{ steps.commit.outputs.sha }} runs: using: composite steps: - - shell: bash + - id: commit + shell: bash env: WORKDIR: ${{ inputs.workdir }} BRANCH: ${{ inputs.branch }} diff --git a/commit-branch.sh b/commit-branch.sh index c9235e6..9b16e2e 100755 --- a/commit-branch.sh +++ b/commit-branch.sh @@ -62,8 +62,23 @@ if [ "${#PATHSPEC[@]}" -gt 0 ]; then else git add -A fi +# The oid of the commit this makes, for a caller that has to name it. Without +# it a caller can only ask GitHub to resolve the branch by name afterwards, and +# that is a race: a workflow dispatched seconds after this mutation can still +# resolve the branch to the PREVIOUS tip and then act on the wrong tree. +# pkghaus/packages lost an ouch release to exactly that on 2026-09-13. +# +# Empty when nothing was committed, so a caller can tell the two apart rather +# than inferring it from a tip that did not move. Silent when GITHUB_OUTPUT is +# unset, which is every local run and the whole test suite. +emit_sha() { # oid + [ -n "${GITHUB_OUTPUT:-}" ] || return 0 + printf 'sha=%s\n' "$1" >> "$GITHUB_OUTPUT" +} + if git diff --cached --quiet HEAD -- "${PATHSPEC[@]}" 2>/dev/null; then log "nothing changed on $BRANCH" + emit_sha "" exit 0 fi @@ -140,7 +155,15 @@ if ! curl -sS --fail-with-body -X POST "$API/graphql" \ exit 1 fi -python3 - "$response" <<'PY' +# stdout carries the oid and nothing else, so the substitution below captures +# it; every diagnostic goes to stderr as before. +# +# What keeps an unsigned commit from reaching the caller is the exit status, +# not the order of the prints: a non-zero exit here makes the assignment fail, +# and set -e stops the script before emit_sha runs. Printing the oid after the +# signature check is belt-and-braces on top of that, which is why moving it +# earlier does not break the tests. +oid="$(python3 - "$response" <<'PY' import json, sys d = json.load(open(sys.argv[1])) if d.get("errors"): @@ -153,4 +176,7 @@ print(f'committed {c["oid"][:12]} signature={sig.get("state")} valid={sig.get("i if not sig.get("isValid"): print("FATAL: GitHub did not sign the commit", file=sys.stderr) raise SystemExit(1) +print(c["oid"]) PY +)" +emit_sha "$oid" diff --git a/tests/lint-action.sh b/tests/lint-action.sh index 8ff35f4..b751118 100755 --- a/tests/lint-action.sh +++ b/tests/lint-action.sh @@ -60,6 +60,20 @@ assert not missing, f"inputs never passed to the step: {sorted(missing)}" # shell metacharacters cannot become shell. assert "${{" not in step["run"], "expression interpolated inside run:" +# Every output must read from the step that actually runs, or it is wired to +# nothing and resolves to the empty string forever -- which looks exactly like +# a commit that changed nothing. The step therefore needs an id, and each +# output's value has to name it. +outputs = doc.get("outputs", {}) +if outputs: + step_id = step.get("id") + assert step_id, "outputs are declared but the step has no id to read from" + for name, spec in outputs.items(): + value = str(spec.get("value", "")) + assert f"steps.{step_id}.outputs." in value, \ + f"output {name!r} does not read from steps.{step_id}.outputs" + print(f" ok {len(outputs)} output(s) wired to step id {step_id!r}") + with open(sys.argv[1], "w") as out: out.write("#!/usr/bin/env bash\nset -euo pipefail\n") out.write(step["run"]) diff --git a/tests/run.sh b/tests/run.sh index d010099..e2d09cb 100755 --- a/tests/run.sh +++ b/tests/run.sh @@ -38,7 +38,7 @@ eq() { # label expected actual # assertions vanish rather than fail -- which is exactly what happened when # these groups were first lifted out of pkghaus/apt: eq() was left behind and # the suite still printed "all tests passed". Bump this when adding one. -EXPECTED_ASSERTIONS=8 +EXPECTED_ASSERTIONS=13 echo "the signed-commit payload describes every change, deletions included" ( @@ -133,9 +133,15 @@ echo "a tree with no changes makes no commit" git -c user.name=t -c user.email=t@example.invalid commit -qm base mkdir -p "$work/bin" printf '#!/bin/sh\ntouch %s/called\n' "$work" > "$work/bin/curl"; chmod +x "$work/bin/curl" - PATH="$work/bin:$PATH" "$ROOT/commit-branch.sh" "$repo" archive "test" >/dev/null 2>&1 + out="$work/gh-output"; : > "$out" + GITHUB_OUTPUT="$out" PATH="$work/bin:$PATH" \ + "$ROOT/commit-branch.sh" "$repo" archive "test" >/dev/null 2>&1 if [ -f "$work/called" ]; then no "an unchanged tree must not call the API" "curl was called" else ok "an unchanged tree must not call the API"; fi + # Empty rather than absent. A caller distinguishes "nothing to commit" from + # "committed" by reading this; an absent key and an empty one look the same + # to a workflow expression, but only one of them is written on purpose. + eq "and it reports an empty sha rather than none" "sha=" "$(cat "$out")" exit $((fail > 0)) ) || fail=$((fail + 1)) @@ -180,6 +186,80 @@ FAKE exit $((fail > 0)) ) || fail=$((fail + 1)) +echo "the commit oid reaches the caller, which is how a dispatch names the right tree" +( + work="$(mktemp -d)" + export GITHUB_TOKEN=fake GITHUB_REPOSITORY=pkghaus/packages + # A real oid from the incident this output exists to prevent: the ouch bump + # whose release was dispatched against the branch NAME and resolved to the + # tip before it. + oid=0fe3b0a783f5ec2041b2b90299b4be61123b15fd + + repo="$work/repo"; mkdir -p "$repo"; cd "$repo" + git init -q -b master . + printf 'VERSION=0.8.2\n' > package.conf + git add -A + git -c user.name=t -c user.email=t@example.invalid commit -qm base + printf 'VERSION=0.8.3\n' > package.conf + + mkdir -p "$work/bin" + cat > "$work/bin/curl" < "$out" + GITHUB_OUTPUT="$out" PATH="$work/bin:$PATH" \ + "$ROOT/commit-branch.sh" "$repo" master "bump" >/dev/null 2>&1 + + # The FULL oid. The log line prints oid[:12] and emitting that instead + # would be a tag pointing at nothing, so the length is the assertion. + eq "the full oid is emitted, not the 12-character log form" \ + "sha=$oid" "$(cat "$out")" + + # Every local run and this whole suite has no GITHUB_OUTPUT. Appending to + # an unset path would abort the script after the commit had already landed. + PATH="$work/bin:$PATH" "$ROOT/commit-branch.sh" "$repo" master "bump" \ + >/dev/null 2>&1 && rc=0 || rc=$? + eq "an unset GITHUB_OUTPUT is not a failure" "0" "${rc:-1}" + + cd /; rm -rf "$work" + exit $((fail > 0)) +) || fail=$((fail + 1)) + +echo "a commit GitHub refused to sign emits no oid at all" +( + work="$(mktemp -d)" + export GITHUB_TOKEN=fake GITHUB_REPOSITORY=pkghaus/packages + + repo="$work/repo"; mkdir -p "$repo"; cd "$repo" + git init -q -b master . + printf 'one\n' > f.txt + git add -A + git -c user.name=t -c user.email=t@example.invalid commit -qm base + printf 'two\n' > f.txt + + mkdir -p "$work/bin" + cat > "$work/bin/curl" <<'FAKE' +#!/bin/sh +echo '{"data":{"createCommitOnBranch":{"commit":{"oid":"bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb","signature":{"isValid":false,"state":"UNSIGNED"}}}}}' +FAKE + chmod +x "$work/bin/curl" + + out="$work/gh-output"; : > "$out" + GITHUB_OUTPUT="$out" PATH="$work/bin:$PATH" \ + "$ROOT/commit-branch.sh" "$repo" master "test" >/dev/null 2>&1 && rc=0 || rc=$? + + eq "the run fails" "1" "${rc:-0}" + # The oid is printed after the signature check for exactly this reason: a + # caller that tagged this would be naming an unsigned commit. + eq "and nothing is handed to the caller" "" "$(cat "$out")" + + cd /; rm -rf "$work" + exit $((fail > 0)) +) || fail=$((fail + 1)) + echo ran="$(wc -l < "$TALLY")" if [ "$ran" -ne "$EXPECTED_ASSERTIONS" ]; then