Skip to content

Commit abd29cf

Browse files
cfsmp3claude
andcommitted
feat: Verify binary commit SHA before running tests
Add verification that the downloaded CCExtractor binary's embedded Git commit SHA matches the expected commit from the test record. This prevents race conditions where an old artifact might be used due to GitHub API caching or other timing issues. Changes: - Add _verify_binary_commit() function that runs binary with --version and parses the "Git commit:" line from output - Call verification after artifact extraction for Linux tests - Fail test with clear error message if commit SHA doesn't match - Skip verification for Windows (can't run .exe on Linux server) The binary already reports its commit via --version: $ ccextractor --version CCExtractor detailed version info Version: 0.96 Git commit: 7a9acb7bd2cb1ae492b4a67d14c9e1687a0f607f ... Tests added for all verification code paths. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 parent 53b3d7f commit abd29cf

File tree

2 files changed

+173
-2
lines changed

2 files changed

+173
-2
lines changed

mod_ci/controllers.py

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
import os
77
import re
88
import shutil
9+
import subprocess
910
import time
1011
import zipfile
1112
from collections import defaultdict
@@ -304,6 +305,61 @@ def mark_test_failed(db, test, repository, message: str) -> None:
304305
log.error(f"Failed to mark test {test.id} as failed: {e}")
305306

306307

308+
def _verify_binary_commit(binary_path: str, expected_commit: str, log) -> tuple[bool, str]:
309+
"""
310+
Verify that the binary's embedded commit SHA matches the expected commit.
311+
312+
Runs the binary with --version and parses the output to extract the Git commit.
313+
This ensures we're testing the correct binary and prevents race conditions.
314+
315+
:param binary_path: Path to the ccextractor binary
316+
:type binary_path: str
317+
:param expected_commit: The expected Git commit SHA
318+
:type expected_commit: str
319+
:param log: Logger instance
320+
:return: Tuple of (success, message)
321+
:rtype: tuple[bool, str]
322+
"""
323+
try:
324+
result = subprocess.run(
325+
[binary_path, '--version'],
326+
capture_output=True,
327+
text=True,
328+
timeout=30
329+
)
330+
331+
# Parse output for Git commit line
332+
# Example output: " Git commit: 7a9acb7bd2cb1ae492b4a67d14c9e1687a0f607f"
333+
for line in result.stdout.split('\n'):
334+
if 'Git commit:' in line:
335+
binary_commit = line.split('Git commit:')[1].strip()
336+
if binary_commit == expected_commit:
337+
log.info(f"Binary commit verified: {binary_commit}")
338+
return True, f"Binary commit verified: {binary_commit}"
339+
else:
340+
error_msg = (f"Binary commit mismatch! Expected {expected_commit}, "
341+
f"but binary reports {binary_commit}")
342+
log.error(error_msg)
343+
return False, error_msg
344+
345+
error_msg = f"Could not find Git commit in binary --version output: {result.stdout[:500]}"
346+
log.error(error_msg)
347+
return False, error_msg
348+
349+
except subprocess.TimeoutExpired:
350+
error_msg = "Binary --version timed out after 30 seconds"
351+
log.error(error_msg)
352+
return False, error_msg
353+
except FileNotFoundError:
354+
error_msg = f"Binary not found at {binary_path}"
355+
log.error(error_msg)
356+
return False, error_msg
357+
except Exception as e:
358+
error_msg = f"Error verifying binary commit: {e}"
359+
log.error(error_msg)
360+
return False, error_msg
361+
362+
307363
def start_test(compute, app, db, repository: Repository.Repository, test, bot_token) -> None:
308364
"""
309365
Start a VM instance and run the tests.
@@ -470,6 +526,22 @@ def start_test(compute, app, db, repository: Repository.Repository, test, bot_to
470526
with zipfile.ZipFile(zip_path, 'r') as artifact_zip:
471527
artifact_zip.extractall(base_folder)
472528

529+
# Verify the binary's embedded commit SHA matches expected commit
530+
# This prevents race conditions where an old artifact might be used
531+
if test.platform == TestPlatform.linux:
532+
binary_path = os.path.join(base_folder, 'ccextractor')
533+
# Make binary executable
534+
os.chmod(binary_path, 0o755)
535+
verified, verify_msg = _verify_binary_commit(binary_path, test.commit, log)
536+
if not verified:
537+
mark_test_failed(db, test, repository, verify_msg)
538+
return
539+
else:
540+
# Windows binaries can't be verified on Linux server
541+
# The binary name is ccextractorwinfull.exe
542+
# TODO: Add verification in Windows VM startup script
543+
log.info(f"Skipping binary verification for Windows (cannot run .exe on Linux)")
544+
473545
artifact_saved = True
474546
break
475547

tests/test_ci/test_controllers.py

Lines changed: 101 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,9 @@
66
from flask import g
77

88
from mod_auth.models import Role
9-
from mod_ci.controllers import (Workflow_builds, get_info_for_pr_comment,
10-
progress_type_request, start_platforms)
9+
from mod_ci.controllers import (Workflow_builds, _verify_binary_commit,
10+
get_info_for_pr_comment, progress_type_request,
11+
start_platforms)
1112
from mod_ci.models import BlockedUsers
1213
from mod_customized.models import CustomizedTest
1314
from mod_home.models import CCExtractorVersion, GeneralData
@@ -2047,3 +2048,101 @@ def generate_header(data, event, ci_key=None):
20472048
'utf-8'), g.github['ci_key'] if ci_key is None else ci_key)
20482049
headers = generate_git_api_header(event, sig)
20492050
return headers
2051+
2052+
2053+
class TestVerifyBinaryCommit(BaseTestCase):
2054+
"""Test the _verify_binary_commit function."""
2055+
2056+
@mock.patch('mod_ci.controllers.subprocess.run')
2057+
def test_verify_binary_commit_success(self, mock_run):
2058+
"""Test successful commit verification."""
2059+
mock_log = MagicMock()
2060+
expected_commit = "abc123def456"
2061+
2062+
# Mock successful --version output
2063+
mock_run.return_value = MagicMock(
2064+
stdout="CCExtractor detailed version info\n"
2065+
" Version: 0.96\n"
2066+
f" Git commit: {expected_commit}\n"
2067+
" Compilation date: 2025-12-23\n"
2068+
)
2069+
2070+
success, message = _verify_binary_commit("/path/to/ccextractor", expected_commit, mock_log)
2071+
2072+
self.assertTrue(success)
2073+
self.assertIn("verified", message)
2074+
mock_log.info.assert_called()
2075+
2076+
@mock.patch('mod_ci.controllers.subprocess.run')
2077+
def test_verify_binary_commit_mismatch(self, mock_run):
2078+
"""Test commit verification fails on mismatch."""
2079+
mock_log = MagicMock()
2080+
expected_commit = "abc123def456"
2081+
actual_commit = "different789"
2082+
2083+
mock_run.return_value = MagicMock(
2084+
stdout=f" Git commit: {actual_commit}\n"
2085+
)
2086+
2087+
success, message = _verify_binary_commit("/path/to/ccextractor", expected_commit, mock_log)
2088+
2089+
self.assertFalse(success)
2090+
self.assertIn("mismatch", message)
2091+
self.assertIn(expected_commit, message)
2092+
self.assertIn(actual_commit, message)
2093+
mock_log.error.assert_called()
2094+
2095+
@mock.patch('mod_ci.controllers.subprocess.run')
2096+
def test_verify_binary_commit_no_git_line(self, mock_run):
2097+
"""Test verification fails when Git commit line is missing."""
2098+
mock_log = MagicMock()
2099+
2100+
mock_run.return_value = MagicMock(
2101+
stdout="CCExtractor version info\n Version: 0.96\n"
2102+
)
2103+
2104+
success, message = _verify_binary_commit("/path/to/ccextractor", "abc123", mock_log)
2105+
2106+
self.assertFalse(success)
2107+
self.assertIn("Could not find Git commit", message)
2108+
mock_log.error.assert_called()
2109+
2110+
@mock.patch('mod_ci.controllers.subprocess.run')
2111+
def test_verify_binary_commit_timeout(self, mock_run):
2112+
"""Test verification handles timeout."""
2113+
import subprocess
2114+
mock_log = MagicMock()
2115+
2116+
mock_run.side_effect = subprocess.TimeoutExpired(cmd="test", timeout=30)
2117+
2118+
success, message = _verify_binary_commit("/path/to/ccextractor", "abc123", mock_log)
2119+
2120+
self.assertFalse(success)
2121+
self.assertIn("timed out", message)
2122+
mock_log.error.assert_called()
2123+
2124+
@mock.patch('mod_ci.controllers.subprocess.run')
2125+
def test_verify_binary_commit_file_not_found(self, mock_run):
2126+
"""Test verification handles missing binary."""
2127+
mock_log = MagicMock()
2128+
2129+
mock_run.side_effect = FileNotFoundError()
2130+
2131+
success, message = _verify_binary_commit("/path/to/ccextractor", "abc123", mock_log)
2132+
2133+
self.assertFalse(success)
2134+
self.assertIn("not found", message)
2135+
mock_log.error.assert_called()
2136+
2137+
@mock.patch('mod_ci.controllers.subprocess.run')
2138+
def test_verify_binary_commit_generic_exception(self, mock_run):
2139+
"""Test verification handles unexpected exceptions."""
2140+
mock_log = MagicMock()
2141+
2142+
mock_run.side_effect = Exception("Unexpected error")
2143+
2144+
success, message = _verify_binary_commit("/path/to/ccextractor", "abc123", mock_log)
2145+
2146+
self.assertFalse(success)
2147+
self.assertIn("Error verifying", message)
2148+
mock_log.error.assert_called()

0 commit comments

Comments
 (0)