Skip to content

Commit de54ff7

Browse files
marcelsafinCopilot
andauthored
fix(workflows): make security requirements sync deterministic (#3832)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent f4a9b89 commit de54ff7

3 files changed

Lines changed: 32 additions & 8 deletions

File tree

.github/scripts/check_security_requirements.py

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,12 +29,20 @@ def _dependency_diff_refs() -> tuple[str, str]:
2929
def _dependency_inputs_changed() -> bool:
3030
base_ref, head_ref = _dependency_diff_refs()
3131
try:
32+
merge_base = subprocess.run(
33+
["git", "merge-base", base_ref, head_ref],
34+
check=True,
35+
cwd=REPO_ROOT,
36+
stderr=subprocess.PIPE,
37+
stdout=subprocess.PIPE,
38+
text=True,
39+
).stdout.strip()
3240
result = subprocess.run(
3341
[
3442
"git",
3543
"diff",
3644
"--name-only",
37-
base_ref,
45+
merge_base,
3846
head_ref,
3947
"--",
4048
*DEPENDENCY_INPUTS,
@@ -77,6 +85,7 @@ def main() -> int:
7785

7886
generated_requirements = Path(generated_requirements_env)
7987
generated_requirements.parent.mkdir(parents=True, exist_ok=True)
88+
generated_requirements.write_bytes(COMMITTED_REQUIREMENTS.read_bytes())
8089

8190
subprocess.run(
8291
[
@@ -87,7 +96,6 @@ def main() -> int:
8796
"--extra",
8897
"test",
8998
"--universal",
90-
"--upgrade",
9199
"--generate-hashes",
92100
"--quiet",
93101
"--no-header",

.github/workflows/security.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ jobs:
3434
- name: Check committed audit requirements are current
3535
env:
3636
DEPENDENCY_DIFF_BASE: ${{ github.event.pull_request.base.sha || github.event.before || '' }}
37-
DEPENDENCY_DIFF_HEAD: ${{ github.sha }}
37+
DEPENDENCY_DIFF_HEAD: ${{ github.event.pull_request.head.sha || github.sha }}
3838
GENERATED_REQUIREMENTS: ${{ runner.temp }}/security-audit-requirements.txt
3939
run: python .github/scripts/check_security_requirements.py
4040

tests/test_security_workflow.py

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@
3030
f"--quiet --no-header --output-file {COMMITTED_AUDIT_REQUIREMENTS}"
3131
)
3232
WORKFLOW_SYNC_COMPILE_TEST_EXTRA_DEPS = (
33-
"uv pip compile pyproject.toml --extra test --universal --upgrade --generate-hashes "
33+
"uv pip compile pyproject.toml --extra test --universal --generate-hashes "
3434
"--quiet --no-header --output-file"
3535
)
3636
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):
9999
assert sync_check["env"]["DEPENDENCY_DIFF_BASE"] == (
100100
"${{ github.event.pull_request.base.sha || github.event.before || '' }}"
101101
)
102-
assert sync_check["env"]["DEPENDENCY_DIFF_HEAD"] == "${{ github.sha }}"
102+
assert sync_check["env"]["DEPENDENCY_DIFF_HEAD"] == (
103+
"${{ github.event.pull_request.head.sha || github.sha }}"
104+
)
103105
assert sync_check["run"] == WORKFLOW_SYNC_SCRIPT
104106
assert committed_audit["run"] == LOCAL_PIP_AUDIT
105107

@@ -239,10 +241,14 @@ def test_committed_audit_requirements_are_hashed(self):
239241

240242
def test_sync_script_skips_when_dependency_inputs_are_unchanged(self, monkeypatch, capsys):
241243
sync_script = _load_sync_script()
244+
commands = []
242245

243246
def fake_run(command, **kwargs):
247+
commands.append(command)
248+
if command[:2] == ["git", "merge-base"]:
249+
return subprocess.CompletedProcess(command, 0, stdout="base123\n", stderr="")
244250
assert command == [
245-
"git", "diff", "--name-only", "HEAD^", "HEAD", "--",
251+
"git", "diff", "--name-only", "base123", "HEAD", "--",
246252
"pyproject.toml", ".github/security-audit-requirements.txt",
247253
]
248254
assert kwargs["check"] is True
@@ -251,23 +257,29 @@ def fake_run(command, **kwargs):
251257
monkeypatch.setattr(sync_script.subprocess, "run", fake_run)
252258

253259
assert sync_script.main() == 0
260+
assert commands[0] == ["git", "merge-base", "HEAD^", "HEAD"]
254261
assert "sync check skipped" in capsys.readouterr().out
255262

256263
def test_sync_script_uses_github_diff_refs_when_available(self, monkeypatch):
257264
sync_script = _load_sync_script()
258265
monkeypatch.setenv("DEPENDENCY_DIFF_BASE", "abc123")
259266
monkeypatch.setenv("DEPENDENCY_DIFF_HEAD", "def456")
267+
commands = []
260268

261269
def fake_run(command, **_kwargs):
270+
commands.append(command)
271+
if command[:2] == ["git", "merge-base"]:
272+
return subprocess.CompletedProcess(command, 0, stdout="merge123\n", stderr="")
262273
assert command == [
263-
"git", "diff", "--name-only", "abc123", "def456", "--",
274+
"git", "diff", "--name-only", "merge123", "def456", "--",
264275
"pyproject.toml", ".github/security-audit-requirements.txt",
265276
]
266277
return subprocess.CompletedProcess(command, 0, stdout="", stderr="")
267278

268279
monkeypatch.setattr(sync_script.subprocess, "run", fake_run)
269280

270281
assert sync_script._dependency_inputs_changed() is False
282+
assert commands[0] == ["git", "merge-base", "abc123", "def456"]
271283

272284
def test_sync_script_compiles_and_compares_when_dependency_inputs_changed(
273285
self, monkeypatch, tmp_path
@@ -284,10 +296,13 @@ def test_sync_script_compiles_and_compares_when_dependency_inputs_changed(
284296
monkeypatch.setenv("GENERATED_REQUIREMENTS", str(generated_requirements))
285297

286298
def fake_run(command, **kwargs):
287-
if command[0] == "git":
299+
if command[:2] == ["git", "merge-base"]:
300+
return subprocess.CompletedProcess(command, 0, stdout="base123\n", stderr="")
301+
if command[:2] == ["git", "diff"]:
288302
return subprocess.CompletedProcess(command, 0, stdout="pyproject.toml\n", stderr="")
289303
compile_commands.append(command)
290304
assert kwargs["check"] is True
305+
assert generated_requirements.read_text(encoding="utf-8") == "pytest==1\n"
291306
generated_requirements.write_text("pytest==1\n", encoding="utf-8")
292307
return subprocess.CompletedProcess(command, 0)
293308

@@ -297,6 +312,7 @@ def fake_run(command, **kwargs):
297312
assert len(compile_commands) == 1
298313
compile_command = " ".join(compile_commands[0])
299314
assert WORKFLOW_SYNC_COMPILE_TEST_EXTRA_DEPS in compile_command
315+
assert "--upgrade" not in compile_commands[0]
300316
assert "--output-file" in compile_commands[0]
301317
assert str(generated_requirements) in compile_commands[0]
302318

0 commit comments

Comments
 (0)