From b65bd5a2dc4faa7456eca34335a2b4a7a74b77d2 Mon Sep 17 00:00:00 2001 From: Tiankai Ma Date: Sun, 20 Sep 2026 12:24:30 +0800 Subject: [PATCH] ci(openapi): require the pinned server commit to be reachable from main `openapi-contract verify-source` only asserted that the server checkout is *at* the pinned commit, and CI checks out exactly the pinned SHA, so that assertion could never fail. A pull-request head SHA stays fetchable from Life-USTC/server forever, so the pin could point at a commit that was never on main while every check passed: the provenance verified, the checkout matched, and the vendored spec was byte-identical to the source. This repository hit it. api/openapi.provenance.json was pinned at fea7bb21ead65fa4da1d51d8ef36ef914a647783 ("fix(young): complete release contracts", 2026-09-15) in #57, a pull-request head that is not an ancestor of server main -- the server squash-merges, so PR heads never land there. The generated client came from a discarded branch snapshot and CI reported success until the contract was re-synced from 222eeb61. Add `openapi-contract verify-reachable SERVER_DIR`, asserting `git merge-base --is-ancestor
` against the server checkout. `verify-source` and `sync` both call it, so CI rejects an unreachable pin and the nightly sync refuses to write one. Ancestry needs real history, so the server checkouts now use `fetch-depth: 0` and fetch `refs/heads/main` explicitly. A shallow checkout stays shallow even after fetching main, so the script refuses to run there instead of guessing: a check that cannot be evaluated is worse than no check. The same refusal covers a missing main ref and a pin the checkout does not contain. scripts/openapi-contract.test.sh builds synthetic server histories and proves the check rejects an unmerged branch head -- reproducing the incident where HEAD and the vendored spec both matched the pin -- and accepts the main tip and older main commits. `make test` runs it before the Go suite. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/ci.yml | 10 ++ .github/workflows/openapi-sync.yml | 12 +- Makefile | 10 +- scripts/openapi-contract | 70 ++++++++- scripts/openapi-contract.test.sh | 237 +++++++++++++++++++++++++++++ 5 files changed, 334 insertions(+), 5 deletions(-) create mode 100755 scripts/openapi-contract.test.sh diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ac5e29e..490e327 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -25,6 +25,16 @@ jobs: repository: Life-USTC/server ref: ${{ steps.server-openapi-ref.outputs.ref }} path: .openapi-server + # The pin is checked for reachability from server main, which needs + # real history: a shallow checkout cannot answer the question, and + # `openapi-contract verify-reachable` refuses to guess. + fetch-depth: 0 + + - name: Fetch server main for the pin reachability check + run: | + git -C .openapi-server fetch --no-tags --quiet origin \ + +refs/heads/main:refs/remotes/origin/main + git -C .openapi-server rev-parse --verify refs/remotes/origin/main >/dev/null - uses: actions/setup-go@b7ad1dad31e06c5925ef5d2fc7ad053ef454303e # v7.0.0 with: diff --git a/.github/workflows/openapi-sync.yml b/.github/workflows/openapi-sync.yml index 2bc0c4f..3cdb7d4 100644 --- a/.github/workflows/openapi-sync.yml +++ b/.github/workflows/openapi-sync.yml @@ -54,6 +54,15 @@ jobs: repository: Life-USTC/server ref: ${{ steps.server.outputs.sha }} path: .openapi-server + # Needed so the sync refuses to pin a commit that never landed on + # server main (a pull-request head stays fetchable by SHA forever). + fetch-depth: 0 + + - name: Fetch server main for the pin reachability check + run: | + git -C .openapi-server fetch --no-tags --quiet origin \ + +refs/heads/main:refs/remotes/origin/main + git -C .openapi-server rev-parse --verify refs/remotes/origin/main >/dev/null - uses: actions/setup-go@b7ad1dad31e06c5925ef5d2fc7ad053ef454303e # v7.0.0 with: @@ -62,8 +71,7 @@ jobs: - name: Sync contract and generated client run: | make sync-openapi OPENAPI_SERVER_DIR=.openapi-server SERVER_COMMIT=${{ steps.server.outputs.sha }} - ./scripts/openapi-contract verify - cmp -s .openapi-server/public/openapi.generated.json api/openapi.json + make check-openapi-sync OPENAPI_SERVER_DIR=.openapi-server make generate - name: Build, test, and vet diff --git a/Makefile b/Makefile index 665b04b..527a80c 100644 --- a/Makefile +++ b/Makefile @@ -3,7 +3,7 @@ OPENAPI_SERVER_DIR ?= ../server SERVER_COMMIT ?= LDFLAGS := -ldflags "-X github.com/Life-USTC/CLI/internal/cmd/root.version=$(VERSION)" -.PHONY: build clean test lint vet install generate sync-openapi check-openapi-provenance check-openapi-sync +.PHONY: build clean test test-scripts lint vet install generate sync-openapi check-openapi-provenance check-openapi-reachability check-openapi-sync build: check-openapi-provenance generate go build $(LDFLAGS) -o life-ustc ./cmd/life-ustc @@ -12,9 +12,12 @@ clean: rm -f life-ustc rm -rf dist/ -test: +test: test-scripts go test -race ./... +test-scripts: + ./scripts/openapi-contract.test.sh + lint: golangci-lint run ./... @@ -34,5 +37,8 @@ sync-openapi: check-openapi-provenance: ./scripts/openapi-contract verify +check-openapi-reachability: + ./scripts/openapi-contract verify-reachable "$(OPENAPI_SERVER_DIR)" + check-openapi-sync: ./scripts/openapi-contract verify-source "$(OPENAPI_SERVER_DIR)" diff --git a/scripts/openapi-contract b/scripts/openapi-contract index 3cc8eaa..e1a9227 100755 --- a/scripts/openapi-contract +++ b/scripts/openapi-contract @@ -7,7 +7,7 @@ readonly spec="api/openapi.json" readonly provenance="api/openapi.provenance.json" usage() { - echo "usage: $0 {pinned-sha|verify|verify-source SERVER_DIR|sync SERVER_DIR SERVER_COMMIT}" >&2 + echo "usage: $0 {pinned-sha|verify|verify-reachable SERVER_DIR|verify-source SERVER_DIR|sync SERVER_DIR SERVER_COMMIT}" >&2 exit 2 } @@ -56,9 +56,72 @@ verify() { } } +# Resolves the ref that stands for the released history of $repository inside +# SERVER_DIR. The pin must be reachable from it, so a ref that is simply absent +# has to fail loudly: a reachability check that cannot be evaluated is worse +# than no check at all. +server_main_ref() { + local server_dir="$1" candidate + local candidates=("refs/remotes/origin/main" "refs/heads/main") + + if [[ -n "${OPENAPI_SERVER_MAIN_REF:-}" ]]; then + candidates=("$OPENAPI_SERVER_MAIN_REF") + fi + + for candidate in "${candidates[@]}"; do + if git -C "$server_dir" rev-parse --verify --quiet "${candidate}^{commit}" >/dev/null; then + printf '%s\n' "$candidate" + return 0 + fi + done + + echo "cannot check pin reachability: none of ${candidates[*]} exist in $server_dir" >&2 + echo "fetch the release branch first, e.g. git -C $server_dir fetch --no-tags origin +refs/heads/main:refs/remotes/origin/main" >&2 + exit 1 +} + +# $repository squash-merges, so every released commit is on main and a +# pull-request head commit never is. A PR head stays fetchable by SHA forever, +# so pinning one produces a client generated from a branch that was thrown +# away. Pinning is only safe when the commit is reachable from main. +verify_reachable() { + local server_dir="$1" + local commit main_ref status=0 + + commit="$(pinned_sha)" + + [[ -d "$server_dir/.git" || -f "$server_dir/.git" ]] || { + echo "cannot check pin reachability: $server_dir is not a git checkout of $repository" >&2 + exit 1 + } + [[ "$(git -C "$server_dir" rev-parse --is-shallow-repository)" == "false" ]] || { + echo "cannot check pin reachability: $server_dir is a shallow clone, so ancestry cannot be evaluated" >&2 + echo "check out $repository with fetch-depth: 0 before verifying the pin" >&2 + exit 1 + } + git -C "$server_dir" rev-parse --verify --quiet "${commit}^{commit}" >/dev/null || { + echo "cannot check pin reachability: pinned commit $commit is not present in $server_dir" >&2 + exit 1 + } + + main_ref="$(server_main_ref "$server_dir")" + git -C "$server_dir" merge-base --is-ancestor "$commit" "$main_ref" || status=$? + + if [[ "$status" -eq 1 ]]; then + echo "pinned server commit $commit is NOT reachable from $main_ref of $repository" >&2 + echo "$repository squash-merges, so this pin is a pull-request head or another commit that never landed on main; the generated client would be built from a dead branch" >&2 + echo "re-run: make sync-openapi OPENAPI_SERVER_DIR=$server_dir SERVER_COMMIT=" >&2 + exit 1 + elif [[ "$status" -ne 0 ]]; then + echo "cannot check pin reachability: git merge-base --is-ancestor $commit $main_ref failed with status $status" >&2 + exit 1 + fi +} + verify_source() { local server_dir="$1" verify + verify_reachable "$server_dir" local commit head source_spec commit="$(pinned_sha)" @@ -100,6 +163,7 @@ sync() { printf '{\n "repository": "%s",\n "commit": "%s",\n "sha256": "%s"\n}\n' \ "$repository" "$commit" "$hash" >"$provenance" verify + verify_reachable "$server_dir" } case "${1:-}" in @@ -111,6 +175,10 @@ case "${1:-}" in [[ $# -eq 1 ]] || usage verify ;; + verify-reachable) + [[ $# -eq 2 ]] || usage + verify_reachable "$2" + ;; verify-source) [[ $# -eq 2 ]] || usage verify_source "$2" diff --git a/scripts/openapi-contract.test.sh b/scripts/openapi-contract.test.sh new file mode 100755 index 0000000..8c6b370 --- /dev/null +++ b/scripts/openapi-contract.test.sh @@ -0,0 +1,237 @@ +#!/usr/bin/env bash + +# Exercises scripts/openapi-contract against synthetic Life-USTC/server +# checkouts. The point of the suite is the reachability assertion: a pull +# request head stays fetchable by SHA forever, so the contract can be pinned to +# a commit that never landed on main, and every other check in the script +# (HEAD matches the pin, vendored spec matches the source) still passes. + +set -euo pipefail + +readonly root="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +readonly script="$root/scripts/openapi-contract" +readonly workdir="$(mktemp -d "${TMPDIR:-/tmp}/openapi-contract-test.XXXXXXXX")" + +failures=0 +phase="initialization" + +cleanup() { + rm -rf -- "$workdir" +} +on_error() { + local status=$? + trap - ERR + printf 'openapi-contract test crashed: phase=%s status=%s command=%s\n' \ + "$phase" "$status" "$BASH_COMMAND" >&2 + exit "$status" +} +trap on_error ERR +trap cleanup EXIT + +pass() { + printf 'ok - %s\n' "$1" +} +fail() { + printf 'FAIL - %s\n' "$1" >&2 + if [[ -n "${2:-}" ]]; then + printf ' %s\n' "$2" >&2 + fi + failures=$((failures + 1)) +} + +git_quiet() { + git -c user.name="contract test" -c user.email="contract@test.invalid" \ + -c commit.gpgsign=false -c init.defaultBranch=main "$@" >/dev/null 2>&1 +} + +# Builds a server checkout whose main branch is a squash-merge style line of +# commits, plus one commit on a side branch that is never merged. Echoes +# " ". +make_server() { + local dir="$1" + local first main_tip side_head + + mkdir -p "$dir/public" + git_quiet init "$dir" + printf '{"openapi":"3.1.0","info":{"title":"first"}}\n' >"$dir/public/openapi.generated.json" + git_quiet -C "$dir" add -A + git_quiet -C "$dir" commit -m "feat: first release contract" + first="$(git -C "$dir" rev-parse HEAD)" + + # A pull request branched off main and was never merged, exactly like + # fea7bb21 ("fix(young): complete release contracts", 2026-09-15). + git_quiet -C "$dir" checkout -b pull-request + printf '{"openapi":"3.1.0","info":{"title":"pull request head"}}\n' >"$dir/public/openapi.generated.json" + git_quiet -C "$dir" add -A + git_quiet -C "$dir" commit -m "fix(young): complete release contracts" + side_head="$(git -C "$dir" rev-parse HEAD)" + + git_quiet -C "$dir" checkout main + printf '{"openapi":"3.1.0","info":{"title":"second"}}\n' >"$dir/public/openapi.generated.json" + git_quiet -C "$dir" add -A + git_quiet -C "$dir" commit -m "feat: squash-merged release contract" + main_tip="$(git -C "$dir" rev-parse HEAD)" + + printf '%s %s %s\n' "$main_tip" "$side_head" "$first" +} + +# Builds a consumer checkout pinned to $2 with a vendored spec copied from $3. +make_consumer() { + local dir="$1" commit="$2" source_spec="$3" + local hash + + mkdir -p "$dir/api" + cp "$source_spec" "$dir/api/openapi.json" + hash="$(sha256sum "$dir/api/openapi.json" | awk '{print $1}')" + printf '{\n "repository": "Life-USTC/server",\n "commit": "%s",\n "sha256": "%s"\n}\n' \ + "$commit" "$hash" >"$dir/api/openapi.provenance.json" +} + +# Runs the script inside a consumer checkout and reports status plus output. +run_contract() { + local consumer="$1" + shift + local status=0 + output="$(cd "$consumer" && "$script" "$@" 2>&1)" || status=$? + return "$status" +} + +phase="shell syntax" +bash -n "$script" +bash -n "${BASH_SOURCE[0]}" + +phase="fixture construction" +server="$workdir/server" +read -r main_tip side_head first_main < <(make_server "$server") +git_quiet -C "$server" checkout main + +phase="a pin that never landed on main is rejected" +consumer="$workdir/consumer-unreachable" +make_consumer "$consumer" "$side_head" "$server/public/openapi.generated.json" +if run_contract "$consumer" verify-reachable "$server"; then + fail "verify-reachable rejects a pull-request head pin" "the script accepted $side_head" +elif [[ "$output" != *"is NOT reachable from"* ]]; then + fail "verify-reachable explains why a pull-request head pin is wrong" "$output" +else + pass "verify-reachable rejects a pull-request head pin" +fi + +phase="the exact incident: HEAD and spec agree, ancestry does not" +# CLI was pinned at fea7bb21 with the server checked out at that very commit +# and the vendored spec byte-identical, and CI reported success. +consumer="$workdir/consumer-incident" +git_quiet -C "$server" checkout "$side_head" +make_consumer "$consumer" "$side_head" "$server/public/openapi.generated.json" +if run_contract "$consumer" verify-source "$server"; then + fail "verify-source rejects the pinned-at-a-PR-head incident" "the script accepted $side_head" +elif [[ "$output" != *"is NOT reachable from"* ]]; then + fail "verify-source fails on reachability, not on some other check" "$output" +else + pass "verify-source rejects the pinned-at-a-PR-head incident" +fi +git_quiet -C "$server" checkout main + +phase="a real main commit is accepted" +consumer="$workdir/consumer-tip" +make_consumer "$consumer" "$main_tip" "$server/public/openapi.generated.json" +if run_contract "$consumer" verify-reachable "$server"; then + pass "verify-reachable accepts the main tip" +else + fail "verify-reachable accepts the main tip" "$output" +fi +if run_contract "$consumer" verify-source "$server"; then + pass "verify-source accepts a checkout pinned to the main tip" +else + fail "verify-source accepts a checkout pinned to the main tip" "$output" +fi + +phase="an older main commit is accepted" +consumer="$workdir/consumer-ancestor" +make_consumer "$consumer" "$first_main" "$server/public/openapi.generated.json" +if run_contract "$consumer" verify-reachable "$server"; then + pass "verify-reachable accepts an older commit on main" +else + fail "verify-reachable accepts an older commit on main" "$output" +fi + +phase="refs/remotes/origin/main is honoured" +remote_clone="$workdir/server-clone" +git_quiet clone "file://$server" "$remote_clone" +git_quiet -C "$remote_clone" checkout "$first_main" +consumer="$workdir/consumer-remote-ref" +make_consumer "$consumer" "$first_main" "$server/public/openapi.generated.json" +if run_contract "$consumer" verify-reachable "$remote_clone"; then + pass "verify-reachable resolves main through refs/remotes/origin/main" +else + fail "verify-reachable resolves main through refs/remotes/origin/main" "$output" +fi +consumer="$workdir/consumer-remote-ref-bad" +make_consumer "$consumer" "$side_head" "$server/public/openapi.generated.json" +if run_contract "$consumer" verify-reachable "$remote_clone"; then + fail "verify-reachable still rejects a PR head against refs/remotes/origin/main" "accepted $side_head" +else + pass "verify-reachable still rejects a PR head against refs/remotes/origin/main" +fi + +phase="a check that cannot be evaluated fails loudly" +shallow="$workdir/server-shallow" +git_quiet clone --depth 1 "file://$server" "$shallow" +consumer="$workdir/consumer-shallow" +make_consumer "$consumer" "$main_tip" "$server/public/openapi.generated.json" +if run_contract "$consumer" verify-reachable "$shallow"; then + fail "verify-reachable refuses a shallow server checkout" "the script reported success" +elif [[ "$output" != *"shallow clone"* ]]; then + fail "verify-reachable names the shallow checkout as the reason" "$output" +else + pass "verify-reachable refuses a shallow server checkout" +fi + +no_main="$workdir/server-no-main" +git_quiet clone "file://$server" "$no_main" +git_quiet -C "$no_main" checkout -b detached-work +git_quiet -C "$no_main" branch -D main +git_quiet -C "$no_main" remote remove origin +consumer="$workdir/consumer-no-main" +make_consumer "$consumer" "$main_tip" "$server/public/openapi.generated.json" +if run_contract "$consumer" verify-reachable "$no_main"; then + fail "verify-reachable refuses a checkout without a main ref" "the script reported success" +elif [[ "$output" != *"cannot check pin reachability"* ]]; then + fail "verify-reachable names the missing main ref as the reason" "$output" +else + pass "verify-reachable refuses a checkout without a main ref" +fi + +consumer="$workdir/consumer-absent-commit" +make_consumer "$consumer" "0123456789abcdef0123456789abcdef01234567" "$server/public/openapi.generated.json" +if run_contract "$consumer" verify-reachable "$server"; then + fail "verify-reachable refuses a pin that the server checkout does not contain" "the script reported success" +elif [[ "$output" != *"is not present in"* ]]; then + fail "verify-reachable names the absent commit as the reason" "$output" +else + pass "verify-reachable refuses a pin that the server checkout does not contain" +fi + +phase="sync refuses to write an unreachable pin" +consumer="$workdir/consumer-sync" +make_consumer "$consumer" "$main_tip" "$server/public/openapi.generated.json" +git_quiet -C "$server" checkout "$side_head" +if run_contract "$consumer" sync "$server" "$side_head"; then + fail "sync refuses to pin a pull-request head" "the script wrote $side_head" +elif [[ "$output" != *"is NOT reachable from"* ]]; then + fail "sync fails on reachability" "$output" +else + pass "sync refuses to pin a pull-request head" +fi +git_quiet -C "$server" checkout main +if run_contract "$consumer" sync "$server" "$main_tip"; then + pass "sync accepts a commit on main" +else + fail "sync accepts a commit on main" "$output" +fi + +phase="reporting" +if [[ "$failures" -ne 0 ]]; then + printf '\n%s openapi-contract check(s) failed\n' "$failures" >&2 + exit 1 +fi +printf '\nall openapi-contract checks passed\n'