From 9b68743cc931d7e300d40c0149cc7cc314f61b52 Mon Sep 17 00:00:00 2001 From: marcelsafin <179933638+marcelsafin@users.noreply.github.com> Date: Wed, 29 Jul 2026 02:59:02 +0200 Subject: [PATCH] fix(workflows): make security requirements sync deterministic Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../scripts/check_security_requirements.py | 12 +++++++-- .github/workflows/security.yml | 2 +- tests/test_security_workflow.py | 26 +++++++++++++++---- 3 files changed, 32 insertions(+), 8 deletions(-) diff --git a/.github/scripts/check_security_requirements.py b/.github/scripts/check_security_requirements.py index 24ebd23789..18f8053528 100644 --- a/.github/scripts/check_security_requirements.py +++ b/.github/scripts/check_security_requirements.py @@ -29,12 +29,20 @@ def _dependency_diff_refs() -> tuple[str, str]: def _dependency_inputs_changed() -> bool: base_ref, head_ref = _dependency_diff_refs() try: + merge_base = subprocess.run( + ["git", "merge-base", base_ref, head_ref], + check=True, + cwd=REPO_ROOT, + stderr=subprocess.PIPE, + stdout=subprocess.PIPE, + text=True, + ).stdout.strip() result = subprocess.run( [ "git", "diff", "--name-only", - base_ref, + merge_base, head_ref, "--", *DEPENDENCY_INPUTS, @@ -77,6 +85,7 @@ def main() -> int: generated_requirements = Path(generated_requirements_env) generated_requirements.parent.mkdir(parents=True, exist_ok=True) + generated_requirements.write_bytes(COMMITTED_REQUIREMENTS.read_bytes()) subprocess.run( [ @@ -87,7 +96,6 @@ def main() -> int: "--extra", "test", "--universal", - "--upgrade", "--generate-hashes", "--quiet", "--no-header", diff --git a/.github/workflows/security.yml b/.github/workflows/security.yml index 959aec6741..f9eb6fd060 100644 --- a/.github/workflows/security.yml +++ b/.github/workflows/security.yml @@ -34,7 +34,7 @@ jobs: - name: Check committed audit requirements are current env: DEPENDENCY_DIFF_BASE: ${{ github.event.pull_request.base.sha || github.event.before || '' }} - DEPENDENCY_DIFF_HEAD: ${{ github.sha }} + DEPENDENCY_DIFF_HEAD: ${{ github.event.pull_request.head.sha || github.sha }} GENERATED_REQUIREMENTS: ${{ runner.temp }}/security-audit-requirements.txt run: python .github/scripts/check_security_requirements.py diff --git a/tests/test_security_workflow.py b/tests/test_security_workflow.py index a9e3f591bd..5be4904d68 100644 --- a/tests/test_security_workflow.py +++ b/tests/test_security_workflow.py @@ -30,7 +30,7 @@ f"--quiet --no-header --output-file {COMMITTED_AUDIT_REQUIREMENTS}" ) WORKFLOW_SYNC_COMPILE_TEST_EXTRA_DEPS = ( - "uv pip compile pyproject.toml --extra test --universal --upgrade --generate-hashes " + "uv pip compile pyproject.toml --extra test --universal --generate-hashes " "--quiet --no-header --output-file" ) WORKFLOW_SYNC_SCRIPT = "python .github/scripts/check_security_requirements.py" @@ -99,7 +99,9 @@ def test_dependency_audit_uses_committed_requirements_for_prs_and_pushes(self): assert sync_check["env"]["DEPENDENCY_DIFF_BASE"] == ( "${{ github.event.pull_request.base.sha || github.event.before || '' }}" ) - assert sync_check["env"]["DEPENDENCY_DIFF_HEAD"] == "${{ github.sha }}" + assert sync_check["env"]["DEPENDENCY_DIFF_HEAD"] == ( + "${{ github.event.pull_request.head.sha || github.sha }}" + ) assert sync_check["run"] == WORKFLOW_SYNC_SCRIPT assert committed_audit["run"] == LOCAL_PIP_AUDIT @@ -239,10 +241,14 @@ def test_committed_audit_requirements_are_hashed(self): def test_sync_script_skips_when_dependency_inputs_are_unchanged(self, monkeypatch, capsys): sync_script = _load_sync_script() + commands = [] def fake_run(command, **kwargs): + commands.append(command) + if command[:2] == ["git", "merge-base"]: + return subprocess.CompletedProcess(command, 0, stdout="base123\n", stderr="") assert command == [ - "git", "diff", "--name-only", "HEAD^", "HEAD", "--", + "git", "diff", "--name-only", "base123", "HEAD", "--", "pyproject.toml", ".github/security-audit-requirements.txt", ] assert kwargs["check"] is True @@ -251,16 +257,21 @@ def fake_run(command, **kwargs): monkeypatch.setattr(sync_script.subprocess, "run", fake_run) assert sync_script.main() == 0 + assert commands[0] == ["git", "merge-base", "HEAD^", "HEAD"] assert "sync check skipped" in capsys.readouterr().out def test_sync_script_uses_github_diff_refs_when_available(self, monkeypatch): sync_script = _load_sync_script() monkeypatch.setenv("DEPENDENCY_DIFF_BASE", "abc123") monkeypatch.setenv("DEPENDENCY_DIFF_HEAD", "def456") + commands = [] def fake_run(command, **_kwargs): + commands.append(command) + if command[:2] == ["git", "merge-base"]: + return subprocess.CompletedProcess(command, 0, stdout="merge123\n", stderr="") assert command == [ - "git", "diff", "--name-only", "abc123", "def456", "--", + "git", "diff", "--name-only", "merge123", "def456", "--", "pyproject.toml", ".github/security-audit-requirements.txt", ] return subprocess.CompletedProcess(command, 0, stdout="", stderr="") @@ -268,6 +279,7 @@ def fake_run(command, **_kwargs): monkeypatch.setattr(sync_script.subprocess, "run", fake_run) assert sync_script._dependency_inputs_changed() is False + assert commands[0] == ["git", "merge-base", "abc123", "def456"] def test_sync_script_compiles_and_compares_when_dependency_inputs_changed( self, monkeypatch, tmp_path @@ -284,10 +296,13 @@ def test_sync_script_compiles_and_compares_when_dependency_inputs_changed( monkeypatch.setenv("GENERATED_REQUIREMENTS", str(generated_requirements)) def fake_run(command, **kwargs): - if command[0] == "git": + if command[:2] == ["git", "merge-base"]: + return subprocess.CompletedProcess(command, 0, stdout="base123\n", stderr="") + if command[:2] == ["git", "diff"]: return subprocess.CompletedProcess(command, 0, stdout="pyproject.toml\n", stderr="") compile_commands.append(command) assert kwargs["check"] is True + assert generated_requirements.read_text(encoding="utf-8") == "pytest==1\n" generated_requirements.write_text("pytest==1\n", encoding="utf-8") return subprocess.CompletedProcess(command, 0) @@ -297,6 +312,7 @@ def fake_run(command, **kwargs): assert len(compile_commands) == 1 compile_command = " ".join(compile_commands[0]) assert WORKFLOW_SYNC_COMPILE_TEST_EXTRA_DEPS in compile_command + assert "--upgrade" not in compile_commands[0] assert "--output-file" in compile_commands[0] assert str(generated_requirements) in compile_commands[0]