From 296e1e77d1c759f791eca8f6d2830be53b1d6d58 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Antonio=20Fern=C3=A1ndez=20de=20Alba?= Date: Thu, 15 Oct 2026 10:31:19 +0200 Subject: [PATCH 1/2] fix: validate access before publishing library bump branches --- .github/workflows/ci.yml | 10 ++ RELEASE.md | 2 + scripts/create-library-version-bump-pr.sh | 44 +++++-- scripts/test-library-version-bump-pr.py | 153 ++++++++++++++++++++++ 4 files changed, 195 insertions(+), 14 deletions(-) create mode 100644 scripts/test-library-version-bump-pr.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2c5e687..5be8fcc 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -12,6 +12,16 @@ permissions: contents: read jobs: + test-library-version-bump-pr: + name: Library Bump PR Script Test + runs-on: ubuntu-latest + steps: + - name: Checkout + uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4.3.1 + + - name: Test Library Bump PR Script + run: python3 scripts/test-library-version-bump-pr.py + test-install-retries: name: Install Retry Test runs-on: ubuntu-latest diff --git a/RELEASE.md b/RELEASE.md index d5d7b53..3ee91b0 100644 --- a/RELEASE.md +++ b/RELEASE.md @@ -41,6 +41,8 @@ scripts/create-library-version-bump-pr.sh The script reads the official package sources for .NET, Java, JavaScript, Python, Python coverage, Ruby, and Go/Orchestrion. If at least one pinned default is outdated, it creates a branch, updates `action.yml` and `README.md`, creates and verifies a signed commit, pushes the branch, and opens a PR. +Before making changes, it checks that the current `gh` account has write access to this repository. Git SSH authentication and `gh` API authentication can use different accounts. It reuses existing labels and creates any missing labels before creating the branch, so a label permission failure stops before any commit or push. + The PR receives: - `library-version-bump` diff --git a/scripts/create-library-version-bump-pr.sh b/scripts/create-library-version-bump-pr.sh index 5c4e7c3..c8e16d2 100755 --- a/scripts/create-library-version-bump-pr.sh +++ b/scripts/create-library-version-bump-pr.sh @@ -39,8 +39,6 @@ for command in curl gh git ruby; do fi done -gh api user --silent >/dev/null - base_branch="main" bump_label="library-version-bump" remote="origin" @@ -48,6 +46,13 @@ repo="DataDog/test-visibility-github-action" semver_minor_label="semver-minor" semver_patch_label="semver-patch" +if [[ "$dry_run" != "true" ]]; then + if ! can_push=$(gh api "repos/$repo" --jq '.permissions.push // false') || [[ "$can_push" != "true" ]]; then + echo "The current gh account cannot access $repo with write permissions. Check gh auth status and select an account with repository access." >&2 + exit 1 + fi +fi + read_default() { ruby -ryaml -e 'puts YAML.load_file("action.yml").fetch("inputs").fetch(ARGV.fetch(0)).fetch("default")' "$1" } @@ -201,6 +206,29 @@ if git show-ref --verify --quiet "refs/heads/$branch_name"; then exit 1 fi +existing_labels=$(gh api --paginate "repos/$repo/labels" --jq '.[].name') + +ensure_label() { + local label="$1" + local description="$2" + local color="$3" + local existing_label + + while IFS= read -r existing_label; do + if [[ "$existing_label" == "$label" ]]; then + return 0 + fi + done <<< "$existing_labels" + + if ! gh label create "$label" --repo "$repo" --description "$description" --color "$color"; then + echo "Could not create required label '$label'. Check the gh account's label permissions. No branch, commit, or push was created." >&2 + return 1 + fi +} + +ensure_label "$bump_label" "Marks PRs that bump pinned library versions" "1D76DB" +ensure_label "$release_label" "Requests a $release_bump_kind test-visibility-github-action release after merge" "0E8A16" + git fetch "$remote" "$base_branch" git checkout -b "$branch_name" FETCH_HEAD @@ -211,18 +239,6 @@ git commit -S -m "chore: bump pinned library versions" git verify-commit HEAD git push -u "$remote" "$branch_name" -gh label create "$bump_label" \ - --repo "$repo" \ - --description "Marks PRs that bump pinned library versions" \ - --color "1D76DB" \ - --force - -gh label create "$release_label" \ - --repo "$repo" \ - --description "Requests a $release_bump_kind test-visibility-github-action release after merge" \ - --color "0E8A16" \ - --force - changes_markdown=$(printf -- '- %s\n' "${changes[@]}") body_file=$(mktemp) trap 'rm -f "$body_file"' EXIT diff --git a/scripts/test-library-version-bump-pr.py b/scripts/test-library-version-bump-pr.py new file mode 100644 index 0000000..ceae90b --- /dev/null +++ b/scripts/test-library-version-bump-pr.py @@ -0,0 +1,153 @@ +#!/usr/bin/env python3 +"""Check bump PR failure handling without GitHub or Git mutations.""" +import json +import os +from pathlib import Path +import shutil +import subprocess +import sys +import tempfile +import textwrap +import unittest + + +ROOT = Path(__file__).resolve().parents[1] +STUB = "#!" + sys.executable + "\n" + textwrap.dedent('''\ + import json + import os + from pathlib import Path + import sys + + tool = Path(sys.argv[0]).name + args = sys.argv[1:] + scenario = os.environ["SCENARIO"] + with open(os.environ["COMMAND_LOG"], "a") as log: + log.write(json.dumps([tool, *args]) + "\\n") + + if tool == "git": + if args[0] == "show-ref": + sys.exit(1) + elif tool == "curl": + url = args[-1] + if "maven" in url: + print("99.0.0") + else: + print(json.dumps({"versions": ["99.0.0"], "version": "99.0.0", + "info": {"version": "99.0.0"}})) + elif tool == "gh": + if args[:2] == ["api", "repos/DataDog/test-visibility-github-action"]: + if scenario == "repo_404": + print("HTTP 404: Not Found", file=sys.stderr) + sys.exit(1) + print("false" if scenario == "no_write_access" else "true") + elif args[:2] == ["api", "user"]: + print("juan-fernandez") + elif args[:2] == ["api", "repos/DataDog/orchestrion/releases/latest"]: + print("v99.0.0") + elif args[:2] == ["pr", "list"]: + pass + elif args[:3] == ["api", "--paginate", "repos/DataDog/test-visibility-github-action/labels"]: + if scenario == "label_list_failure": + sys.exit(1) + print("semver-minor") + if scenario not in ("missing_label", "label_create_failure"): + print("library-version-bump") + elif args[:2] == ["label", "create"]: + if scenario == "label_create_failure": + print("HTTP 404: Not Found", file=sys.stderr) + sys.exit(1) + elif args[:2] == ["pr", "create"]: + body = Path(args[args.index("--body-file") + 1]).read_text() + assert "### What does this PR do?" in body + assert "### Describe how to test/QA your changes" in body + assert "Go/Orchestrion:" in body + print("https://github.com/DataDog/test-visibility-github-action/pull/123") + else: + raise AssertionError(args) +''') + + +class BumpPRTests(unittest.TestCase): + def run_script(self, scenario, *args): + with tempfile.TemporaryDirectory() as directory: + workspace = Path(directory) + (workspace / "scripts").mkdir() + for name in ("create-library-version-bump-pr.sh", "bump-library-versions.sh"): + shutil.copy2(ROOT / "scripts" / name, workspace / "scripts" / name) + for name in ("action.yml", "README.md"): + shutil.copy2(ROOT / name, workspace / name) + commands = workspace / "commands" + commands.mkdir() + for name in ("gh", "git", "curl"): + stub = commands / name + stub.write_text(STUB) + stub.chmod(0o755) + log = workspace / "command-log" + env = dict(os.environ, PATH=f"{commands}:{os.environ['PATH']}", + SCENARIO=scenario, COMMAND_LOG=str(log)) + result = subprocess.run(["bash", "scripts/create-library-version-bump-pr.sh", *args], + cwd=workspace, env=env, text=True, capture_output=True, timeout=30) + calls = [json.loads(line) for line in log.read_text().splitlines()] + return result, calls + + def assert_no_git_mutations(self, calls): + mutating_commands = {"fetch", "checkout", "add", "commit", "push"} + self.assertFalse(any(call[0] == "git" and call[1] in mutating_commands + for call in calls), calls) + + def test_existing_labels_are_reused(self): + result, calls = self.run_script("existing_labels") + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + self.assertFalse(any(call[:3] == ["gh", "label", "create"] for call in calls)) + self.assertIn(["git", "commit", "-S", "-m", "chore: bump pinned library versions"], calls) + verify = calls.index(["git", "verify-commit", "HEAD"]) + push = next(i for i, call in enumerate(calls) if call[:2] == ["git", "push"]) + create = next(i for i, call in enumerate(calls) if call[:3] == ["gh", "pr", "create"]) + self.assertLess(verify, push) + self.assertLess(push, create) + self.assertEqual(calls[create][-4:], ["--label", "library-version-bump", "--label", "semver-minor"]) + + def test_missing_label_is_created_before_branch(self): + result, calls = self.run_script("missing_label") + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + labels = [call for call in calls if call[:3] == ["gh", "label", "create"]] + self.assertEqual(len(labels), 1) + self.assertEqual(labels[0][3], "library-version-bump") + self.assertNotIn("--force", labels[0]) + self.assertLess(calls.index(labels[0]), calls.index(["git", "fetch", "origin", "main"])) + + def test_wrong_account_stops_before_package_lookup(self): + for scenario in ("repo_404", "no_write_access"): + with self.subTest(scenario=scenario): + result, calls = self.run_script(scenario) + self.assertNotEqual(result.returncode, 0) + self.assertIn("current gh account", result.stderr) + self.assertEqual(len(calls), 1) + self.assert_no_git_mutations(calls) + + def test_label_list_failure_stops_before_branch(self): + result, calls = self.run_script("label_list_failure") + self.assertNotEqual(result.returncode, 0) + self.assert_no_git_mutations(calls) + self.assertFalse(any(call[:3] == ["gh", "label", "create"] for call in calls)) + + def test_label_creation_failure_stops_before_branch(self): + result, calls = self.run_script("label_create_failure") + self.assertNotEqual(result.returncode, 0) + self.assertIn("No branch, commit, or push was created", result.stderr) + self.assert_no_git_mutations(calls) + self.assertFalse(any(call[:3] == ["gh", "pr", "create"] for call in calls)) + + def test_dry_run_does_not_create_labels_or_pr(self): + for scenario in ("missing_label", "no_write_access"): + with self.subTest(scenario=scenario): + result, calls = self.run_script(scenario, "--dry-run") + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + self.assertIn("Dry run only", result.stdout) + self.assert_no_git_mutations(calls) + self.assertFalse(any(call[:3] in (["gh", "label", "create"], ["gh", "pr", "create"]) + for call in calls)) + + +if __name__ == "__main__": + unittest.main() From 29619755da0a4aa3a473895c0c7f43587178a247 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Antonio=20Fern=C3=A1ndez=20de=20Alba?= Date: Thu, 15 Oct 2026 10:41:09 +0200 Subject: [PATCH 2/2] chore: simplify library bump script changes --- .github/workflows/ci.yml | 10 -- RELEASE.md | 2 +- scripts/test-library-version-bump-pr.py | 153 ------------------------ 3 files changed, 1 insertion(+), 164 deletions(-) delete mode 100644 scripts/test-library-version-bump-pr.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 5be8fcc..2c5e687 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -12,16 +12,6 @@ permissions: contents: read jobs: - test-library-version-bump-pr: - name: Library Bump PR Script Test - runs-on: ubuntu-latest - steps: - - name: Checkout - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4.3.1 - - - name: Test Library Bump PR Script - run: python3 scripts/test-library-version-bump-pr.py - test-install-retries: name: Install Retry Test runs-on: ubuntu-latest diff --git a/RELEASE.md b/RELEASE.md index 3ee91b0..73063a5 100644 --- a/RELEASE.md +++ b/RELEASE.md @@ -41,7 +41,7 @@ scripts/create-library-version-bump-pr.sh The script reads the official package sources for .NET, Java, JavaScript, Python, Python coverage, Ruby, and Go/Orchestrion. If at least one pinned default is outdated, it creates a branch, updates `action.yml` and `README.md`, creates and verifies a signed commit, pushes the branch, and opens a PR. -Before making changes, it checks that the current `gh` account has write access to this repository. Git SSH authentication and `gh` API authentication can use different accounts. It reuses existing labels and creates any missing labels before creating the branch, so a label permission failure stops before any commit or push. +Before making changes, it checks that the credentials selected by `gh` have write access to this repository. Git SSH authentication and `gh` API authentication can use different accounts. It reuses existing labels and creates any missing labels before creating the branch, so a label permission failure stops before any commit or push. The PR receives: diff --git a/scripts/test-library-version-bump-pr.py b/scripts/test-library-version-bump-pr.py deleted file mode 100644 index ceae90b..0000000 --- a/scripts/test-library-version-bump-pr.py +++ /dev/null @@ -1,153 +0,0 @@ -#!/usr/bin/env python3 -"""Check bump PR failure handling without GitHub or Git mutations.""" -import json -import os -from pathlib import Path -import shutil -import subprocess -import sys -import tempfile -import textwrap -import unittest - - -ROOT = Path(__file__).resolve().parents[1] -STUB = "#!" + sys.executable + "\n" + textwrap.dedent('''\ - import json - import os - from pathlib import Path - import sys - - tool = Path(sys.argv[0]).name - args = sys.argv[1:] - scenario = os.environ["SCENARIO"] - with open(os.environ["COMMAND_LOG"], "a") as log: - log.write(json.dumps([tool, *args]) + "\\n") - - if tool == "git": - if args[0] == "show-ref": - sys.exit(1) - elif tool == "curl": - url = args[-1] - if "maven" in url: - print("99.0.0") - else: - print(json.dumps({"versions": ["99.0.0"], "version": "99.0.0", - "info": {"version": "99.0.0"}})) - elif tool == "gh": - if args[:2] == ["api", "repos/DataDog/test-visibility-github-action"]: - if scenario == "repo_404": - print("HTTP 404: Not Found", file=sys.stderr) - sys.exit(1) - print("false" if scenario == "no_write_access" else "true") - elif args[:2] == ["api", "user"]: - print("juan-fernandez") - elif args[:2] == ["api", "repos/DataDog/orchestrion/releases/latest"]: - print("v99.0.0") - elif args[:2] == ["pr", "list"]: - pass - elif args[:3] == ["api", "--paginate", "repos/DataDog/test-visibility-github-action/labels"]: - if scenario == "label_list_failure": - sys.exit(1) - print("semver-minor") - if scenario not in ("missing_label", "label_create_failure"): - print("library-version-bump") - elif args[:2] == ["label", "create"]: - if scenario == "label_create_failure": - print("HTTP 404: Not Found", file=sys.stderr) - sys.exit(1) - elif args[:2] == ["pr", "create"]: - body = Path(args[args.index("--body-file") + 1]).read_text() - assert "### What does this PR do?" in body - assert "### Describe how to test/QA your changes" in body - assert "Go/Orchestrion:" in body - print("https://github.com/DataDog/test-visibility-github-action/pull/123") - else: - raise AssertionError(args) -''') - - -class BumpPRTests(unittest.TestCase): - def run_script(self, scenario, *args): - with tempfile.TemporaryDirectory() as directory: - workspace = Path(directory) - (workspace / "scripts").mkdir() - for name in ("create-library-version-bump-pr.sh", "bump-library-versions.sh"): - shutil.copy2(ROOT / "scripts" / name, workspace / "scripts" / name) - for name in ("action.yml", "README.md"): - shutil.copy2(ROOT / name, workspace / name) - commands = workspace / "commands" - commands.mkdir() - for name in ("gh", "git", "curl"): - stub = commands / name - stub.write_text(STUB) - stub.chmod(0o755) - log = workspace / "command-log" - env = dict(os.environ, PATH=f"{commands}:{os.environ['PATH']}", - SCENARIO=scenario, COMMAND_LOG=str(log)) - result = subprocess.run(["bash", "scripts/create-library-version-bump-pr.sh", *args], - cwd=workspace, env=env, text=True, capture_output=True, timeout=30) - calls = [json.loads(line) for line in log.read_text().splitlines()] - return result, calls - - def assert_no_git_mutations(self, calls): - mutating_commands = {"fetch", "checkout", "add", "commit", "push"} - self.assertFalse(any(call[0] == "git" and call[1] in mutating_commands - for call in calls), calls) - - def test_existing_labels_are_reused(self): - result, calls = self.run_script("existing_labels") - self.assertEqual(result.returncode, 0, result.stdout + result.stderr) - self.assertFalse(any(call[:3] == ["gh", "label", "create"] for call in calls)) - self.assertIn(["git", "commit", "-S", "-m", "chore: bump pinned library versions"], calls) - verify = calls.index(["git", "verify-commit", "HEAD"]) - push = next(i for i, call in enumerate(calls) if call[:2] == ["git", "push"]) - create = next(i for i, call in enumerate(calls) if call[:3] == ["gh", "pr", "create"]) - self.assertLess(verify, push) - self.assertLess(push, create) - self.assertEqual(calls[create][-4:], ["--label", "library-version-bump", "--label", "semver-minor"]) - - def test_missing_label_is_created_before_branch(self): - result, calls = self.run_script("missing_label") - self.assertEqual(result.returncode, 0, result.stdout + result.stderr) - labels = [call for call in calls if call[:3] == ["gh", "label", "create"]] - self.assertEqual(len(labels), 1) - self.assertEqual(labels[0][3], "library-version-bump") - self.assertNotIn("--force", labels[0]) - self.assertLess(calls.index(labels[0]), calls.index(["git", "fetch", "origin", "main"])) - - def test_wrong_account_stops_before_package_lookup(self): - for scenario in ("repo_404", "no_write_access"): - with self.subTest(scenario=scenario): - result, calls = self.run_script(scenario) - self.assertNotEqual(result.returncode, 0) - self.assertIn("current gh account", result.stderr) - self.assertEqual(len(calls), 1) - self.assert_no_git_mutations(calls) - - def test_label_list_failure_stops_before_branch(self): - result, calls = self.run_script("label_list_failure") - self.assertNotEqual(result.returncode, 0) - self.assert_no_git_mutations(calls) - self.assertFalse(any(call[:3] == ["gh", "label", "create"] for call in calls)) - - def test_label_creation_failure_stops_before_branch(self): - result, calls = self.run_script("label_create_failure") - self.assertNotEqual(result.returncode, 0) - self.assertIn("No branch, commit, or push was created", result.stderr) - self.assert_no_git_mutations(calls) - self.assertFalse(any(call[:3] == ["gh", "pr", "create"] for call in calls)) - - def test_dry_run_does_not_create_labels_or_pr(self): - for scenario in ("missing_label", "no_write_access"): - with self.subTest(scenario=scenario): - result, calls = self.run_script(scenario, "--dry-run") - self.assertEqual(result.returncode, 0, result.stdout + result.stderr) - self.assertIn("Dry run only", result.stdout) - self.assert_no_git_mutations(calls) - self.assertFalse(any(call[:3] in (["gh", "label", "create"], ["gh", "pr", "create"]) - for call in calls)) - - -if __name__ == "__main__": - unittest.main()