diff --git a/.github/workflows/tag-release.yml b/.github/workflows/tag-release.yml index 9ae14ed..a4417a0 100644 --- a/.github/workflows/tag-release.yml +++ b/.github/workflows/tag-release.yml @@ -220,6 +220,7 @@ jobs: env: NEXT_TAG: ${{ steps.compute.outputs.next_tag }} TAG_PREFIX: ${{ inputs.tag-prefix }} + GH_TOKEN: ${{ steps.app-token.outputs.token }} run: | PREFIX="${TAG_PREFIX:-v}" VERSION="${NEXT_TAG#"${PREFIX}"}" @@ -248,6 +249,13 @@ jobs: set -euo pipefail + # Manifest file path — the unique set of modified paths, consumed by + # tag-release.yml's Git Data API tree builder. Overridable so parallel + # test runs and concurrent script invocations don't clobber one + # another through a shared /tmp file (the default preserves prior + # behavior). + BUMP_MODIFIED_FILE="${BUMP_MODIFIED_FILE:-/tmp/bump.modified}" + # Strict allowlist: identifiers, integer indices, bracket-quoted string keys # (npm/composer style: kebab-case, @scope/pkg, dotted paths), and the [] # iterator (applies-to-every). The jq [*] form is NOT valid jq grammar and @@ -332,6 +340,7 @@ jobs: fi write_value "$file_path" "$field" "$path_expr" "$version" + printf '%s\n' "$file_path" >> "$BUMP_MODIFIED_FILE" printf '| `%s` | `%s` | `%s` -> `%s` | updated |\n' \ "$file_path" "$disp" "$current" "$version" return 0 @@ -343,6 +352,12 @@ jobs: version="${2:-${VERSION:-}}" : "${version:?usage: $0 | VERSION=X $0 [config]}" + # Manifest of unique modified paths (consumed by tag-release.yml's + # Git Data API path to build POST /git/trees). Truncate-or-create at + # start so the file is always a fresh, well-defined snapshot of THIS + # invocation's modifications. De-dup happens at end of apply pass. + : > "$BUMP_MODIFIED_FILE" + if [ ! -f "$config" ]; then echo "No $config found, skipping version file bump" exit 2 @@ -384,6 +399,14 @@ jobs: fi done < <(jq -c '.files[]' "$config") + # De-dup manifest in place: a single file may appear N times if + # .version-bump.json has multiple entries against it (see test + # `multi-entry: two path_expr entries against same file write both`). + # sort -u keeps the contract that /tmp/bump.modified is a unique set. + if [ -s "$BUMP_MODIFIED_FILE" ]; then + sort -u "$BUMP_MODIFIED_FILE" -o "$BUMP_MODIFIED_FILE" + fi + if [ "$changed" -eq 1 ]; then exit 0 else @@ -399,21 +422,98 @@ jobs: case "$BUMP_EXIT" in 0) - git config user.name "github-actions[bot]" - git config user.email "41898282+github-actions[bot]@users.noreply.github.com" - for entry in $(jq -r '.files[] | .path' .version-bump.json); do - git add "$entry" - done - git commit -m "chore(release): bump version files to $VERSION" - git push origin HEAD:main - RESULT="Committed and pushed version file updates" + # --- Git Data API rewrite (spec §3.1) --- + # BASE_SHA is the workflow's runtime view of the repo HEAD — + # the SHA actions/checkout checked out. Reading refs/heads/main + # fresh here would race with concurrent pushes (spec §2). + REPO="${GITHUB_REPOSITORY}" + BASE_SHA="${GITHUB_SHA}" + + # Step 4 (spec): read parent tree SHA from BASE_SHA's commit. + PARENT_TREE_SHA=$(gh api "repos/${REPO}/git/commits/${BASE_SHA}" --jq '.tree.sha') + + # Steps 3+5: create blobs for each modified file, accumulate + # tree[] entries, then create the tree. The manifest path must + # match the one bump_version_files wrote — same default, same + # BUMP_MODIFIED_FILE override if the step env sets it. + MANIFEST="${BUMP_MODIFIED_FILE:-/tmp/bump.modified}" + TREE_ENTRIES='[]' + while IFS= read -r modified_path; do + [ -z "$modified_path" ] && continue + blob_content_b64=$(base64 < "$modified_path" | tr -d '\n') + blob_sha=$(jq -nc \ + --arg c "$blob_content_b64" \ + '{content: $c, encoding: "base64"}' \ + | gh api -X POST "repos/${REPO}/git/blobs" --input - --jq '.sha') + TREE_ENTRIES=$(jq --arg p "$modified_path" --arg s "$blob_sha" \ + '. + [{path: $p, mode: "100644", type: "blob", sha: $s}]' \ + <<<"$TREE_ENTRIES") + done < "$MANIFEST" + + TREE_SHA=$(jq -nc \ + --arg base "$PARENT_TREE_SHA" \ + --argjson entries "$TREE_ENTRIES" \ + '{base_tree: $base, tree: $entries}' \ + | gh api -X POST "repos/${REPO}/git/trees" --input - --jq '.sha') + + # Step 6: create commit. NO author/committer fields — GitHub + # auto-signs with the bot/web-flow key only when the request is + # authenticated as the App AND the body has no identity overrides + # (spec §3.3). + NEW_COMMIT_SHA=$(jq -nc \ + --arg msg "chore(release): bump version files to $VERSION" \ + --arg tree "$TREE_SHA" \ + --arg parent "$BASE_SHA" \ + '{message: $msg, tree: $tree, parents: [$parent]}' \ + | gh api -X POST "repos/${REPO}/git/commits" --input - --jq '.sha') + + # Acceptance criterion #2 (spec §6): bump commit MUST verify. + # Verify the commit OBJECT *before* advancing main. A commit is + # addressable by SHA the instant POST /git/commits returns — + # independent of any ref — so this check needs no branch update + # first. Verifying first means a verification failure (or a + # failed lookup) leaves main untouched: no dangling unverified + # commit for a later rerun to pick up via the no-bump path. + # Note jq path: GET /git/commits/{sha} returns the raw Git commit + # object with `verification` at the TOP level. The higher-level + # /commits/{sha} endpoint (no `git/` prefix) wraps it as + # `.commit.verification` — do NOT confuse the two. + VERIFIED=$(gh api "repos/${REPO}/git/commits/${NEW_COMMIT_SHA}" \ + --jq '.verification.verified') + REASON=$(gh api "repos/${REPO}/git/commits/${NEW_COMMIT_SHA}" \ + --jq '.verification.reason') + if [ "$VERIFIED" != "true" ]; then + echo "::error::Bump commit ${NEW_COMMIT_SHA} did not auto-sign (verified=${VERIFIED} reason=${REASON})" + echo "::error::This means the API rewrite is not satisfying the required_signatures rule — investigate before proceeding." + exit 1 + fi + + # Step 7: fast-forward main to the verified commit. force=false + # (default). 422 = non-fast-forward = main moved past BASE_SHA + # after checkout → fail the run loudly (spec §3.1 step 7, + # race-detection backstop). + gh api -X PATCH "repos/${REPO}/git/refs/heads/main" \ + -f sha="$NEW_COMMIT_SHA" \ + -F force=false + + # Export TAG_TARGET_SHA for the Create-and-push-tag step. + echo "TAG_TARGET_SHA=${NEW_COMMIT_SHA}" >> "$GITHUB_ENV" + RESULT="Committed and pushed version file updates via Git Data API (sha: ${NEW_COMMIT_SHA}, verified: true, reason: ${REASON})" ;; 1) echo "::error::Bump version files step failed due to .version-bump.json error" exit 1 ;; 2) - RESULT="No changes — config absent, all up to date, or all entries skipped" + # No bump created. The commit to tag is exactly the commit that + # "Compute release" semver-analyzed — i.e. GITHUB_SHA. Reading + # live refs/heads/main here instead would race with concurrent + # pushes (spec §2) and could tag a commit that was never + # analyzed, publishing it under a bump computed from a different + # tree. GITHUB_SHA also covers rerun-after-partial-failure: a + # fresh dispatch resolves GITHUB_SHA to current main HEAD anyway. + echo "TAG_TARGET_SHA=${GITHUB_SHA}" >> "$GITHUB_ENV" + RESULT="No changes — config absent, all up to date, or all entries skipped (tag will point at analyzed commit: ${GITHUB_SHA})" ;; *) echo "::error::bump-version-files.sh exited with unexpected code $BUMP_EXIT" @@ -436,11 +536,54 @@ jobs: - name: Create and push tag env: NEXT_TAG: ${{ steps.compute.outputs.next_tag }} + GH_TOKEN: ${{ steps.app-token.outputs.token }} run: | - git config user.name "github-actions[bot]" - git config user.email "41898282+github-actions[bot]@users.noreply.github.com" + # --- Git Data API rewrite (spec §3.2) --- + # TAG_TARGET_SHA is set by the prior "Bump version files" step: + # - case 0: the new bump commit's SHA (returned by POST /git/commits) + # - case 2: current refs/heads/main SHA (read via GET /git/ref/heads/main) + # Either way, NEVER from local checkout state (spec §2). + REPO="${GITHUB_REPOSITORY}" + + if [ -z "${TAG_TARGET_SHA:-}" ]; then + echo "::error::TAG_TARGET_SHA not set by Bump version files step — workflow contract violated" + exit 1 + fi - git tag -a "$NEXT_TAG" -m "Release $NEXT_TAG" - git push origin "$NEXT_TAG" + # Create tag OBJECT. NO tagger field — GitHub auto-signs with the + # bot/web-flow key only when authenticated as the App AND the body + # has no identity overrides (spec §3.3). + TAG_OBJECT_SHA=$(jq -nc \ + --arg tag "$NEXT_TAG" \ + --arg msg "Release $NEXT_TAG" \ + --arg sha "$TAG_TARGET_SHA" \ + '{tag: $tag, message: $msg, object: $sha, type: "commit"}' \ + | gh api -X POST "repos/${REPO}/git/tags" --input - --jq '.sha') + + # Create tag REF. 422 = ref already exists → fail loudly (spec §4 + # row 3): re-creating an existing tag is not a workflow-level + # decision; operator must intentionally delete/replace before rerun. + gh api -X POST "repos/${REPO}/git/refs" \ + -f ref="refs/tags/${NEXT_TAG}" \ + -f sha="$TAG_OBJECT_SHA" + + # Report tag verification status (acceptance criterion #3 OR #4 + # per spec §6 — the canary run records which path locks in for v2.5.3). + # Note jq path: GET /git/tags/{sha} returns the raw Git tag object + # with `verification` at the TOP level (same convention as + # /git/commits/{sha} — see Bump step's verification check). + TAG_VERIFIED=$(gh api "repos/${REPO}/git/tags/${TAG_OBJECT_SHA}" --jq '.verification.verified') + TAG_REASON=$(gh api "repos/${REPO}/git/tags/${TAG_OBJECT_SHA}" --jq '.verification.reason') + + { + echo "### Tag created" + echo "" + echo "**Tag:** \`$NEXT_TAG\`" + echo "**Tag object SHA:** \`$TAG_OBJECT_SHA\`" + echo "**Target commit SHA:** \`$TAG_TARGET_SHA\`" + echo "**Tag object verification:** \`verified=$TAG_VERIFIED\` \`reason=$TAG_REASON\`" + echo "" + echo "Tag created via Git Data API. If the caller workflow chains a publish step, it will pick up from here." + } >> "$GITHUB_STEP_SUMMARY" - echo "Tagged and pushed $NEXT_TAG - release.yml will pick up from here." + echo "Tag $NEXT_TAG created via Git Data API (verified=$TAG_VERIFIED reason=$TAG_REASON)." diff --git a/scripts/bump-version-files.sh b/scripts/bump-version-files.sh index 0b3a824..a2da211 100755 --- a/scripts/bump-version-files.sh +++ b/scripts/bump-version-files.sh @@ -21,6 +21,13 @@ set -euo pipefail +# Manifest file path — the unique set of modified paths, consumed by +# tag-release.yml's Git Data API tree builder. Overridable so parallel +# test runs and concurrent script invocations don't clobber one +# another through a shared /tmp file (the default preserves prior +# behavior). +BUMP_MODIFIED_FILE="${BUMP_MODIFIED_FILE:-/tmp/bump.modified}" + # Strict allowlist: identifiers, integer indices, bracket-quoted string keys # (npm/composer style: kebab-case, @scope/pkg, dotted paths), and the [] # iterator (applies-to-every). The jq [*] form is NOT valid jq grammar and @@ -105,6 +112,7 @@ bump_entry() { fi write_value "$file_path" "$field" "$path_expr" "$version" + printf '%s\n' "$file_path" >> "$BUMP_MODIFIED_FILE" printf '| `%s` | `%s` | `%s` -> `%s` | updated |\n' \ "$file_path" "$disp" "$current" "$version" return 0 @@ -116,6 +124,12 @@ config="${1:-.version-bump.json}" version="${2:-${VERSION:-}}" : "${version:?usage: $0 | VERSION=X $0 [config]}" +# Manifest of unique modified paths (consumed by tag-release.yml's +# Git Data API path to build POST /git/trees). Truncate-or-create at +# start so the file is always a fresh, well-defined snapshot of THIS +# invocation's modifications. De-dup happens at end of apply pass. +: > "$BUMP_MODIFIED_FILE" + if [ ! -f "$config" ]; then echo "No $config found, skipping version file bump" exit 2 @@ -157,6 +171,14 @@ while IFS= read -r row; do fi done < <(jq -c '.files[]' "$config") +# De-dup manifest in place: a single file may appear N times if +# .version-bump.json has multiple entries against it (see test +# `multi-entry: two path_expr entries against same file write both`). +# sort -u keeps the contract that /tmp/bump.modified is a unique set. +if [ -s "$BUMP_MODIFIED_FILE" ]; then + sort -u "$BUMP_MODIFIED_FILE" -o "$BUMP_MODIFIED_FILE" +fi + if [ "$changed" -eq 1 ]; then exit 0 else diff --git a/tests/bump-version-files.bats b/tests/bump-version-files.bats index 1aa2593..515afaf 100644 --- a/tests/bump-version-files.bats +++ b/tests/bump-version-files.bats @@ -11,6 +11,10 @@ setup() { REPO_ROOT="$BATS_TEST_DIRNAME/.." cp "$REPO_ROOT"/tests/fixtures/bump-version-files/targets/*.json "$TMPDIR/" cd "$TMPDIR" + # Per-test manifest path. The script defaults to a shared /tmp/bump.modified; + # pinning it inside this test's unique TMPDIR keeps parallel/concurrent runs + # from clobbering one another (teardown's rm -rf cleans it up). + export BUMP_MODIFIED_FILE="$TMPDIR/bump.modified" } teardown() { @@ -395,3 +399,56 @@ JSON [ "$(cat server.json)" = "$ORIG" ] [[ "$output" =~ "skipped (invalid path_expr)" ]] } + +# === Manifest emission: bump.modified contract (#issue-cross-agent-12) === +# Manifest must be a UNIQUE PATH SET — see spec §3.1 step 2. The Git Data API +# rewrite of tag-release.yml consumes this file to build POST /git/trees; +# duplicates would produce duplicate tree[] entries with undefined behavior. +# Each test reads $BUMP_MODIFIED_FILE (a per-test path exported in setup), +# not the shared /tmp default, so concurrent runs stay isolated. + +@test "manifest: bump.modified is unique path set after multi-entry-same-file run" { + run_bumper "valid/multi-entry-same-file.json" "1.2.3" + [ "$status" -eq 0 ] + [ -f "$BUMP_MODIFIED_FILE" ] + # Exactly one line: server.json (despite TWO entries targeting it) + [ "$(wc -l < "$BUMP_MODIFIED_FILE")" -eq 1 ] + [ "$(cat "$BUMP_MODIFIED_FILE")" = "server.json" ] +} + +@test "manifest: bump.modified contains all unique modified paths in mixed run" { + run_bumper "valid/mixed-old-and-new.json" "1.2.3" + [ "$status" -eq 0 ] + # mixed-old-and-new.json bumps package.json (legacy field) AND server.json (path_expr). + # `< file` redirect makes the assertion fail loudly if the manifest is missing, + # rather than passing a stray empty string into the comparison. + expected=$(printf 'package.json\nserver.json\n') + [ "$(sort < "$BUMP_MODIFIED_FILE")" = "$expected" ] +} + +@test "manifest: bump.modified is empty when no entries are updated (idempotent rerun)" { + cp "$REPO_ROOT/tests/fixtures/bump-version-files/valid/legacy-field.json" .version-bump.json + bash "$REPO_ROOT/scripts/bump-version-files.sh" .version-bump.json 1.2.3 # First run: bumps + run bash "$REPO_ROOT/scripts/bump-version-files.sh" .version-bump.json 1.2.3 # Second run: no-op + [ "$status" -eq 2 ] + [ -f "$BUMP_MODIFIED_FILE" ] + [ ! -s "$BUMP_MODIFIED_FILE" ] # File exists but is empty +} + +@test "manifest: bump.modified is truncated on each invocation (no cross-run pollution)" { + # Run 1: bump multi-entry-same-file (writes "server.json") + run_bumper "valid/multi-entry-same-file.json" "1.2.3" + [ "$status" -eq 0 ] + # Run 2: idempotent rerun of legacy-field. First bump 0.0.0 -> 9.9.9 succeeds; + # second bump 9.9.9 -> 9.9.9 is a no-op (exit 2). The manifest after run 2 + # must still be truncated (empty) AND must NOT carry server.json across. + cp "$REPO_ROOT/tests/fixtures/bump-version-files/valid/legacy-field.json" .version-bump.json + bash "$REPO_ROOT/scripts/bump-version-files.sh" .version-bump.json 9.9.9 # First bump + run bash "$REPO_ROOT/scripts/bump-version-files.sh" .version-bump.json 9.9.9 # Idempotent rerun + [ "$status" -eq 2 ] + # File-existence guard prevents this from passing vacuously when the + # script doesn't yet create the manifest (red phase): grep on a missing + # file returns 1, and `! grep` would silently succeed without it. + [ -f "$BUMP_MODIFIED_FILE" ] + ! grep -q server.json "$BUMP_MODIFIED_FILE" +} diff --git a/tests/tag-release-invariants.bats b/tests/tag-release-invariants.bats new file mode 100644 index 0000000..1ce6487 --- /dev/null +++ b/tests/tag-release-invariants.bats @@ -0,0 +1,127 @@ +#!/usr/bin/env bats + +# tag-release-invariants.bats — static checks on the rewritten Git Data API +# blocks in .github/workflows/tag-release.yml. +# +# Spec §2 invariant: the rewritten "Bump version files" and "Create and push +# tag" steps must never depend on a post-checkout local-state read for +# deciding what SHA to use as a commit parent, what SHA to update main to, +# or what SHA to tag. The single load-bearing SHA value is $GITHUB_SHA, +# captured once. + +setup() { + REPO_ROOT="$BATS_TEST_DIRNAME/.." + WORKFLOW="$REPO_ROOT/.github/workflows/tag-release.yml" +} + +# Extract the YAML body of a step by name, up to the next " - name:" boundary. +extract_step_body() { + local name="$1" + awk -v marker=" - name: $name" ' + index($0, marker) == 1 { inside=1; next } + inside && /^ - name: / { inside=0 } + inside { print } + ' "$WORKFLOW" +} + +@test "invariant: 'Bump version files' step contains no post-checkout local-state git reads" { + body=$(extract_step_body "Bump version files") + [ -n "$body" ] + # Spec §2 forbids ALL post-checkout local-state git reads — the same set + # in both the bump step and the tag step. `git log` is included even though + # the bump step doesn't currently call it, to prevent future regressions + # (e.g., "look up the parent commit's date" via local git instead of API). + for forbidden in 'git rev-parse' 'git symbolic-ref' 'git for-each-ref' 'git describe' 'git log'; do + if printf '%s' "$body" | grep -qF "$forbidden"; then + printf 'VIOLATION: "%s" appears in Bump version files step (spec §2)\n' "$forbidden" + printf '%s\n' "$body" | grep -nF "$forbidden" + false + fi + done +} + +@test "invariant: 'Create and push tag' step contains no post-checkout local-state git reads" { + body=$(extract_step_body "Create and push tag") + [ -n "$body" ] + for forbidden in 'git rev-parse' 'git symbolic-ref' 'git for-each-ref' 'git describe' 'git log'; do + if printf '%s' "$body" | grep -qF "$forbidden"; then + printf 'VIOLATION: "%s" appears in Create and push tag step (spec §2)\n' "$forbidden" + printf '%s\n' "$body" | grep -nF "$forbidden" + false + fi + done +} + +@test "invariant: 'Bump version files' step uses GITHUB_SHA as commit parent" { + body=$(extract_step_body "Bump version files") + # After Phase B rewrite, BASE_SHA must be derived from GITHUB_SHA, not from + # GET /git/ref/heads/main (which would race per spec §2). Skip this check + # if the step still uses git push (Phase A only — pre-Phase-B baseline). + if printf '%s' "$body" | grep -qF 'git push origin HEAD:main'; then + skip "Phase A baseline — bump step still uses git push; check applies after Phase B" + fi + # Assert the full load-bearing chain, not merely that GITHUB_SHA appears + # somewhere — a stray `echo "$GITHUB_SHA"` must NOT satisfy this test: + # GITHUB_SHA -> BASE_SHA -> jq --arg parent -> commit payload + printf '%s' "$body" | grep -qF 'BASE_SHA="${GITHUB_SHA}"' || { + echo "VIOLATION: bump step does not bind BASE_SHA to GITHUB_SHA (spec §2 / §3.1 step 1)" + false + } + # `--` ends grep option parsing so the leading `--arg` is treated as a + # pattern, not a flag. + printf '%s' "$body" | grep -qF -- '--arg parent "$BASE_SHA"' || { + echo "VIOLATION: commit-create jq call does not wire its parent arg from BASE_SHA" + false + } + printf '%s' "$body" | grep -qF 'parents: [$parent]' || { + echo "VIOLATION: commit payload does not build parents from the \$parent arg" + false + } +} + +@test "invariant: 'Bump version files' step never reads a live ref for the tag target" { + body=$(extract_step_body "Bump version files") + [ -n "$body" ] + # Spec §2 / review Finding 1: the no-bump (exit 2) path must tag the + # semver-analyzed commit (GITHUB_SHA), never a fresh `GET /git/ref/heads/...` + # read — that races with concurrent pushes and can tag an unanalyzed commit. + # The discriminator is the singular/plural ref endpoint form: the legitimate + # fast-forward PATCH uses the PLURAL `git/refs/heads/main`; only the + # SINGULAR-`ref` GET form (`git/ref/heads/`) is forbidden here. + if printf '%s' "$body" | grep -qF 'git/ref/heads/'; then + echo "VIOLATION: Bump step performs a live 'GET .../git/ref/heads/' read (spec §2 / Finding 1)" + printf '%s' "$body" | grep -nF 'git/ref/heads/' + false + fi +} + +@test "invariant: bump-commit and tag payloads omit author/committer/tagger fields" { + # Strip comment-only lines first — the workflow code's own commentary + # legitimately mentions these field names (e.g., "NO author/committer + # fields — auto-signs"). The test enforces that the words appear ONLY + # in comments, never in code positions where they could become real + # identity overrides via a jq object literal, gh api -f/-F/--field flag, + # or YAML key. + body_bump=$(extract_step_body "Bump version files" | grep -v '^[[:space:]]*#') + body_tag=$(extract_step_body "Create and push tag" | grep -v '^[[:space:]]*#') + if printf '%s' "$body_bump" | grep -qF 'git push origin HEAD:main'; then + skip "Phase A baseline — checks apply after Phase B" + fi + # Word-boundary match catches every escape hatch: "author", 'author', + # author: (jq literal), -f author=, -F author=, --field author=. + # If a real workflow change needs one of these field names in code + # for a non-identity-override reason, the test must be updated + # deliberately, with a comment explaining why. + for field in 'author' 'committer'; do + if printf '%s' "$body_bump" | grep -qE "\\b${field}\\b"; then + echo "VIOLATION: '$field' appears in Bump step (non-comment context) — disables auto-signing if used as a payload field name (spec §3.3)" + printf '%s' "$body_bump" | grep -nE "\\b${field}\\b" + false + fi + done + if printf '%s' "$body_tag" | grep -qE '\btagger\b'; then + echo "VIOLATION: 'tagger' appears in Tag step (non-comment context) — disables auto-signing if used as a payload field name (spec §3.3)" + printf '%s' "$body_tag" | grep -nE '\btagger\b' + false + fi +}