From baa1d987159784922b8e04e447515efe773d108f Mon Sep 17 00:00:00 2001
From: Ryan Duguid <152749594+ryanduguid@users.noreply.github.com>
Date: Thu, 13 Aug 2026 02:31:09 +1000
Subject: [PATCH 1/3] ci: fail closed on API compatibility checks

---
 .github/workflows/api-diff.yml    |  19 ++---
 scripts/api-diff/api-diff.sh      | 115 +++++++--------------------
 scripts/api-diff/api-diff.test.sh | 124 ++++++++++--------------------
 3 files changed, 74 insertions(+), 184 deletions(-)

diff --git a/.github/workflows/api-diff.yml b/.github/workflows/api-diff.yml
index 3ba967cc4..543deaa2f 100644
--- a/.github/workflows/api-diff.yml
+++ b/.github/workflows/api-diff.yml
@@ -12,32 +12,23 @@ concurrency:
   cancel-in-progress: true
 
 jobs:
-  test-conventional-commit-logic:
-    runs-on: ubuntu-latest
-    permissions:
-      contents: read
-    steps:
-    - name: Checkout code
-      uses: actions/checkout@v4
-
-    - name: Run conventional commit logic unit tests
-      run: ./scripts/api-diff/api-diff.test.sh
-
   api-diff:
     runs-on: ubuntu-latest
-    needs: test-conventional-commit-logic
+    timeout-minutes: 20
     permissions:
       contents: read
-      pull-requests: write
 
     steps:
     - name: Checkout code
-      uses: actions/checkout@v4
+      uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4.2.2
       with:
         fetch-depth: 0
 
     - name: Make script executable
       run: chmod +x scripts/api-diff/api-diff.sh
 
+    - name: Test fail-closed behaviour
+      run: bash scripts/api-diff/api-diff.test.sh
+
     - name: Run API diff check
       run: ./scripts/api-diff/api-diff.sh
diff --git a/scripts/api-diff/api-diff.sh b/scripts/api-diff/api-diff.sh
index b5c09823c..0a72b5f69 100755
--- a/scripts/api-diff/api-diff.sh
+++ b/scripts/api-diff/api-diff.sh
@@ -1,20 +1,19 @@
 #!/bin/bash
 
 # Script to check API diffs using oasdiff
-# Usage: ./scripts/api-diff/api-diff.sh [--fail-on-breaking] [filename.yaml]
+# Usage: ./scripts/api-diff/api-diff.sh [filename.yaml]
 # Assumes you have Docker installed and the repo is checked out with master branch available
 
-set -e  # Exit on error
-set -o pipefail  # Catch errors in pipes
+set -euo pipefail
 
 # Change to repo root
 cd "$(dirname "$0")/../.."
 
 # Configuration
-DOCKER_IMAGE="${OASDIFF_DOCKER_IMAGE:-tufin/oasdiff:latest}"
+DOCKER_IMAGE="${OASDIFF_DOCKER_IMAGE:-tufin/oasdiff:v1.28.0@sha256:86830f988eaafcf589acb2794ee5ab78e3300ded071d6517bf085469300cbf36}"
 
 # Detect base branch from GitHub Actions environment or fallback to local defaults
-if [ -n "$GITHUB_BASE_REF" ]; then
+if [ -n "${GITHUB_BASE_REF:-}" ]; then
     # In GitHub Actions PR, use the base ref (e.g., "master")
     BASE_BRANCH="${BASE_BRANCH:-origin/$GITHUB_BASE_REF}"
 else
@@ -22,76 +21,17 @@ else
     BASE_BRANCH="${BASE_BRANCH:-origin/master}"
 fi
 
-FAIL_ON_BREAKING=false
 TARGET_FILE=""
-DRY_RUN=false
-
-detect_breaking_commit_marker() {
-    local commits="$1"
-
-    # Conventional Commits breaking indicators:
-    # 1) An exclamation mark in the type/scope header, e.g. feat!: ... or feat(api)!: ...
-    # 2) A BREAKING CHANGE footer in the commit body
-    if echo "$commits" | grep -Eiq '^[[:space:]]*[a-z]+(\([^)]+\))?!:'; then
-        return 0
-    fi
-
-    if echo "$commits" | grep -Eiq 'BREAKING[ -]CHANGE:'; then
-        return 0
-    fi
-
-    return 1
-}
-
-get_commit_messages() {
-    # For tests and local overrides
-    if [ -n "$COMMIT_MESSAGES" ]; then
-        echo "$COMMIT_MESSAGES"
-        return 0
-    fi
-
-    # In GitHub Actions PRs, scan all commit subjects + bodies in the PR range.
-    if [ -n "$GITHUB_BASE_REF" ]; then
-        git log --format='%s%n%b%n----' "origin/$GITHUB_BASE_REF..HEAD" 2>/dev/null || true
-        return 0
-    fi
-
-    # Local default: inspect HEAD commit only.
-    git log -1 --format='%s%n%b' 2>/dev/null || true
-}
 
 # Parse arguments
 for arg in "$@"; do
-    if [ "$arg" = "--fail-on-breaking" ]; then
-        FAIL_ON_BREAKING=true
-    elif [ "$arg" = "--dry-run" ]; then
-        DRY_RUN=true
-    elif [[ "$arg" == *.yaml ]]; then
+    if [[ "$arg" == *.yaml ]] && [ -z "$TARGET_FILE" ]; then
         TARGET_FILE="$arg"
-    fi
-done
-
-# If --fail-on-breaking not explicitly set, determine based on conventional commit markers.
-if [ "$FAIL_ON_BREAKING" = false ]; then
-    COMMIT_TEXT=$(get_commit_messages)
-    if detect_breaking_commit_marker "$COMMIT_TEXT"; then
-        echo "Detected conventional commit breaking marker ('!' in header or 'BREAKING CHANGE:' footer), allowing breaking changes"
-        FAIL_ON_BREAKING=false
     else
-        echo "No conventional commit breaking marker found, failing on breaking changes"
-        FAIL_ON_BREAKING=true
+        echo "Error: unsupported argument '$arg'" >&2
+        exit 2
     fi
-fi
-
-if [ "$DRY_RUN" = true ]; then
-    if [ "$FAIL_ON_BREAKING" = true ]; then
-        echo "Mode: Failing on breaking changes"
-    else
-        echo "Mode: Allowing breaking changes"
-    fi
-    echo "Dry run mode, exiting after commit message check"
-    exit 0
-fi
+done
 
 echo "Starting API diff check..."
 
@@ -101,12 +41,14 @@ if [ ! -f "xero_accounting.yaml" ]; then
     exit 1
 fi
 
-# Fetch master if not already done
-git fetch "${BASE_BRANCH%%/*}" "${BASE_BRANCH##*/}" 2>/dev/null || echo "Warning: Could not fetch ${BASE_BRANCH}"
+# Refresh and verify the exact base. A missing or stale base must not turn this
+# compatibility check into a pass.
+git fetch "${BASE_BRANCH%%/*}" "${BASE_BRANCH##*/}"
+git rev-parse --verify "${BASE_BRANCH}^{commit}" >/dev/null
 
 # Create temp directory for master branch files (outside repo to avoid overlap with /current mount)
 TEMP_DIR=$(mktemp -d)
-trap "rm -rf $TEMP_DIR" EXIT
+trap 'rm -rf "$TEMP_DIR"' EXIT
 
 # Get list of xero*.yaml files (excluding any master_*.yaml files)
 if [ -n "$TARGET_FILE" ]; then
@@ -127,6 +69,7 @@ else
 fi
 
 BREAKING_CHANGES_FOUND=false
+EXECUTION_FAILED=false
 FILES_WITH_BREAKING_CHANGES=()
 TOTAL_FILES=0
 PROCESSED_FILES=0
@@ -143,16 +86,13 @@ for file in $files; do
     echo "========== $file =========="
 
     # Get the file from master branch
-    if ! git show "$BASE_BRANCH:$file" > "$TEMP_DIR/$file" 2>/dev/null; then
+    if ! git cat-file -e "$BASE_BRANCH:$file" 2>/dev/null; then
         echo "ℹ️  New file (does not exist in master branch)"
+        PROCESSED_FILES=$((PROCESSED_FILES + 1))
         continue
     fi
 
-    # Verify the temp file was created
-    if [ ! -f "$TEMP_DIR/$file" ]; then
-        echo "❌ Failed to create temp file"
-        continue
-    fi
+    git show "$BASE_BRANCH:$file" > "$TEMP_DIR/$file"
 
     # Note: oasdiff has some non-deterministic behavior in change counts due to
     # unordered map iteration in Go. Error counts are consistent, but warning
@@ -170,7 +110,8 @@ for file in $files; do
     if [ $CHANGELOG_EXIT -eq 0 ]; then
         echo "✓ Changelog generated successfully"
     else
-        echo "⚠ Could not generate changelog (exit code: $CHANGELOG_EXIT)"
+        echo "❌ Could not generate changelog (exit code: $CHANGELOG_EXIT)"
+        EXECUTION_FAILED=true
     fi
 
     # Run breaking changes check
@@ -201,6 +142,11 @@ echo "Processed: $PROCESSED_FILES/$TOTAL_FILES files"
 echo "========================================"
 
 # Summary
+if [ "$EXECUTION_FAILED" = true ]; then
+    echo "❌ API diff execution failed"
+    exit 1
+fi
+
 if [ "$BREAKING_CHANGES_FOUND" = true ]; then
     echo ""
     echo "❌ Breaking changes detected in the following files:"
@@ -212,15 +158,10 @@ if [ "$BREAKING_CHANGES_FOUND" = true ]; then
         fi
     done
 
-    if [ "$FAIL_ON_BREAKING" = true ]; then
-        echo ""
-        echo "Exiting with error due to breaking changes"
-        exit 1
-    else
-        echo ""
-        echo "Note: Not failing build (use --fail-on-breaking to fail on breaking changes)"
-    fi
+    echo ""
+    echo "Exiting with error due to breaking changes"
+    exit 1
 else
     echo ""
     echo "✓ No breaking changes detected across all files"
-fi
\ No newline at end of file
+fi
diff --git a/scripts/api-diff/api-diff.test.sh b/scripts/api-diff/api-diff.test.sh
index bce73e8a9..e0ddd1e5f 100755
--- a/scripts/api-diff/api-diff.test.sh
+++ b/scripts/api-diff/api-diff.test.sh
@@ -1,91 +1,49 @@
-#!/bin/bash
+#!/usr/bin/env bash
 
-# Unit test for api-diff.sh conventional commit logic
-# Tests commit message detection and FAIL_ON_BREAKING setting
+set -euo pipefail
 
-set -e
+cd "$(dirname "$0")/../.."
 
-echo "=== Unit Test: api-diff.sh Conventional Commit Logic ==="
-echo
+FAKE_BIN=$(mktemp -d)
+trap 'rm -rf "$FAKE_BIN"' EXIT
 
-TESTS_PASSED=0
-TESTS_FAILED=0
-
-SCRIPT_PATH="scripts/api-diff/api-diff.sh"
-
-# Helper function to test script with commit messages
-test_commit_messages() {
-    local commit_messages="$1"
-    local expected_mode="$2"  # "allow-breaking-changes" or "block-breaking-changes"
-    local test_name="$3"
-
-    echo "Testing: $test_name (commit: $commit_messages)"
-
-    # Run the script in dry-run mode with COMMIT_MESSAGES set
-    local output
-    output=$(COMMIT_MESSAGES="$commit_messages" "$SCRIPT_PATH" --dry-run 2>&1)
-
-    if [[ "$expected_mode" == "allow-breaking-changes" ]]; then
-        if echo "$output" | grep -q "Mode: Allowing breaking changes"; then
-            echo "  ✓ PASS: Correctly allows breaking changes"
-            TESTS_PASSED=$((TESTS_PASSED + 1))
-        else
-            echo "  ✗ FAIL: Expected to allow breaking changes, but output was:"
-            echo "$output"
-            TESTS_FAILED=$((TESTS_FAILED + 1))
-        fi
-    elif [[ "$expected_mode" == "block-breaking-changes" ]]; then
-        if echo "$output" | grep -q "Mode: Failing on breaking changes"; then
-            echo "  ✓ PASS: Correctly fails on breaking changes"
-            TESTS_PASSED=$((TESTS_PASSED + 1))
-        else
-            echo "  ✗ FAIL: Expected to fail on breaking changes, but output was:"
-            echo "$output"
-            TESTS_FAILED=$((TESTS_FAILED + 1))
-        fi
+cat > "$FAKE_BIN/docker" <<'EOF'
+#!/usr/bin/env bash
+if [[ " $* " == *" changelog "* ]] && [ "${FAKE_DOCKER_MODE:-pass}" = "changelog-error" ]; then
+    echo "simulated changelog error" >&2
+    exit 2
+fi
+if [[ " $* " == *" breaking "* ]] && [ "${FAKE_DOCKER_MODE:-pass}" = "breaking" ]; then
+    echo "simulated breaking change" >&2
+    exit 1
+fi
+exit 0
+EOF
+chmod +x "$FAKE_BIN/docker"
+
+run_check() {
+    local expected_exit=$1
+    local mode=$2
+    shift 2
+
+    set +e
+    PATH="$FAKE_BIN:$PATH" \
+        BASE_BRANCH=origin/master \
+        FAKE_DOCKER_MODE="$mode" \
+        OASDIFF_DOCKER_IMAGE=test-image \
+        bash scripts/api-diff/api-diff.sh "$@" >/dev/null 2>&1
+    local actual_exit=$?
+    set -e
+
+    if [ "$actual_exit" -ne "$expected_exit" ]; then
+        echo "Expected exit $expected_exit for mode '$mode', got $actual_exit" >&2
+        exit 1
     fi
-    echo
 }
 
-# Test cases: test_commit_messages "commit_messages" "expected_mode" "test_name"
-# expected_mode: "allow-breaking-changes" = allows breaking changes, "block-breaking-changes" = fails on breaking
+run_check 0 pass xero_accounting.yaml
+run_check 1 breaking xero_accounting.yaml
+run_check 1 changelog-error xero_accounting.yaml
+run_check 2 pass --unsupported
 
-echo "--- Commit messages that SHOULD allow breaking changes ---"
-test_commit_messages "feat!: remove deprecated endpoint" "allow-breaking-changes" "Header with ! marker"
-test_commit_messages "feat(api)!: remove deprecated endpoint" "allow-breaking-changes" "Header with scope and ! marker"
-test_commit_messages "feat: refactor API\n\nBREAKING CHANGE: response schema updated" "allow-breaking-changes" "BREAKING CHANGE footer"
-test_commit_messages "fix: patch bug\n\nSome details\nBREAKING CHANGE: removed old field" "allow-breaking-changes" "BREAKING CHANGE footer after body"
-
-echo "--- Commit messages that SHOULD fail on breaking changes ---"
-test_commit_messages "feat: add optional field" "block-breaking-changes" "Normal feature commit"
-test_commit_messages "fix: resolve null issue" "block-breaking-changes" "Normal fix commit"
-test_commit_messages "chore: update docs" "block-breaking-changes" "Chore commit"
-test_commit_messages "docs: mention breaking behaviour in description" "block-breaking-changes" "Contains word breaking but no marker"
-
-echo "--- Test override with --fail-on-breaking ---"
-# Test that --fail-on-breaking overrides commit message logic
-echo "Testing override: commit with breaking marker and --fail-on-breaking"
-output=$(COMMIT_MESSAGES="feat!: breaking api update" "$SCRIPT_PATH" --dry-run --fail-on-breaking 2>&1)
-if echo "$output" | grep -q "Mode: Failing on breaking changes"; then
-    echo "  ✓ PASS: --fail-on-breaking overrides commit logic"
-    TESTS_PASSED=$((TESTS_PASSED + 1))
-else
-    echo "  ✗ FAIL: --fail-on-breaking did not override, output:"
-    echo "$output"
-    TESTS_FAILED=$((TESTS_FAILED + 1))
-fi
-echo
-
-echo "========================================"
-echo "Test Results:"
-echo "  Passed: $TESTS_PASSED"
-echo "  Failed: $TESTS_FAILED"
-echo "========================================"
-
-if [ $TESTS_FAILED -gt 0 ]; then
-    echo "❌ Some tests failed!"
-    exit 1
-else
-    echo "✅ All tests passed!"
-    exit 0
-fi
+echo "API diff fail-closed tests passed"

From 8d62cdabf8736d381fcc63ba95355426ca561ed8 Mon Sep 17 00:00:00 2001
From: Ryan Duguid <152749594+ryanduguid@users.noreply.github.com>
Date: Thu, 13 Aug 2026 02:53:31 +1000
Subject: [PATCH 2/3] test: cover unset Actions environment

---
 scripts/api-diff/api-diff.sh      | 2 +-
 scripts/api-diff/api-diff.test.sh | 3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/scripts/api-diff/api-diff.sh b/scripts/api-diff/api-diff.sh
index 0a72b5f69..6df24460a 100755
--- a/scripts/api-diff/api-diff.sh
+++ b/scripts/api-diff/api-diff.sh
@@ -153,7 +153,7 @@ if [ "$BREAKING_CHANGES_FOUND" = true ]; then
     for file in "${FILES_WITH_BREAKING_CHANGES[@]}"; do
         echo "   - $file"
         # Output GitHub Actions annotation
-        if [ -n "$GITHUB_ACTIONS" ]; then
+        if [ -n "${GITHUB_ACTIONS:-}" ]; then
             echo "::warning file=${file}::Breaking changes detected in this API spec file"
         fi
     done
diff --git a/scripts/api-diff/api-diff.test.sh b/scripts/api-diff/api-diff.test.sh
index e0ddd1e5f..d02963e32 100755
--- a/scripts/api-diff/api-diff.test.sh
+++ b/scripts/api-diff/api-diff.test.sh
@@ -27,7 +27,8 @@ run_check() {
     shift 2
 
     set +e
-    PATH="$FAKE_BIN:$PATH" \
+    env -u GITHUB_ACTIONS \
+        PATH="$FAKE_BIN:$PATH" \
         BASE_BRANCH=origin/master \
         FAKE_DOCKER_MODE="$mode" \
         OASDIFF_DOCKER_IMAGE=test-image \

From 3311e7b524af463bd8f88925593737cba6886782 Mon Sep 17 00:00:00 2001
From: Ryan Duguid <152749594+ryanduguid@users.noreply.github.com>
Date: Thu, 13 Aug 2026 03:52:42 +1000
Subject: [PATCH 3/3] docs: describe the pinned oasdiff image

---
 scripts/api-diff/README.md | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/scripts/api-diff/README.md b/scripts/api-diff/README.md
index 75d9e6f47..4824603ba 100644
--- a/scripts/api-diff/README.md
+++ b/scripts/api-diff/README.md
@@ -23,9 +23,11 @@ Main script that compares OpenAPI specifications against the master branch.
 ```
 
 **Environment Variables:**
-- `OASDIFF_DOCKER_IMAGE` - Docker image to use (default: `tufin/oasdiff:latest`)
+- `OASDIFF_DOCKER_IMAGE` - Docker image to use (default: oasdiff 1.28.0 pinned by image digest in `api-diff.sh`)
 - `BASE_BRANCH` - Branch to compare against (default: `origin/master`)
 
+When updating oasdiff, verify the release tag and the image manifest digest from the publisher, then update both the tag and digest together. Do not replace the default with a mutable tag such as `latest`.
+
 ### `api-diff.test.sh`
 Unit tests for conventional commit breaking marker detection used in GitHub Actions.