diff --git a/CHANGELOG.md b/CHANGELOG.md index 47e634d..be72919 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,14 @@ # Changelog +## [0.9.1] - 2026-10-03 + +### Fixed +- Normalized discovery exclusions so top-level paths match with or without `./`. +- Made SARIF output conform to the 2.1.0 result schema. +- Failed closed when Docker omitted an exit code and separated package setup errors from script failures. +- Validated project configuration in `doctor` and handled UTF-8 BOM scripts. +- Distinguished Docker wait API errors from genuine execution timeouts. + ## [0.9.0] - 2026-10-03 ### Added diff --git a/README.md b/README.md index 22ea233..86318eb 100644 --- a/README.md +++ b/README.md @@ -48,7 +48,7 @@ apt-get --version Run it locally with `opsscript-gate run ./install.sh`, or add the following step to CI: ```yaml -- uses: Mresyzz/opsscript-gate@v0.9.0 +- uses: Mresyzz/opsscript-gate@v0.9.1 with: script-path: install.sh ``` @@ -57,7 +57,7 @@ The result identifies the failing distribution, exit code, line, missing command --- -## Current release: v0.9.0 +## Current release: v0.9.1 The current release includes a project config file, a dry-run plan, presets, exclusions, saved reports, changed-script selection for pull requests, and repeat runs for detecting @@ -111,7 +111,7 @@ jobs: - uses: actions/checkout@v7 with: persist-credentials: false - - uses: Mresyzz/opsscript-gate@v0.9.0 + - uses: Mresyzz/opsscript-gate@v0.9.1 ``` This zero-configuration form discovers shell scripts in the repository. Use @@ -122,7 +122,7 @@ repository has a single installer. For self-hosted model installers, see the Or test a specific script with custom execution modes: ```yaml - - uses: Mresyzz/opsscript-gate@v0.9.0 + - uses: Mresyzz/opsscript-gate@v0.9.1 with: script-path: scripts/install.sh shell: auto @@ -156,7 +156,7 @@ unrelated scripts: with: persist-credentials: false fetch-depth: 0 - - uses: Mresyzz/opsscript-gate@v0.9.0 + - uses: Mresyzz/opsscript-gate@v0.9.1 with: changed-since: ${{ github.event.pull_request.base.sha }} preset: minimal @@ -247,7 +247,7 @@ Use `sarif` when the result should appear in GitHub Code Scanning or another SARIF-compatible viewer. Upload it explicitly with the official upload action: ```yaml -- uses: Mresyzz/opsscript-gate@v0.9.0 +- uses: Mresyzz/opsscript-gate@v0.9.1 with: script-path: scripts/install.sh format: sarif diff --git a/pyproject.toml b/pyproject.toml index 528147e..4d44a49 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "hatchling.build" [project] name = "opsscript-gate" -version = "0.9.0" +version = "0.9.1" description = "Cross-distro shell script compatibility checker for runtime testing in Debian, Ubuntu and Alpine" readme = "README.md" license = { text = "MIT" } diff --git a/src/opsscript_gate/__init__.py b/src/opsscript_gate/__init__.py index 9dc59bc..79c3aca 100644 --- a/src/opsscript_gate/__init__.py +++ b/src/opsscript_gate/__init__.py @@ -1,4 +1,4 @@ """OpsScript Gate: Lightweight cross-distro compatibility pre-check for Linux ops scripts.""" -__version__ = "0.9.0" +__version__ = "0.9.1" __all__ = ["__version__"] diff --git a/src/opsscript_gate/cli.py b/src/opsscript_gate/cli.py index dcbd958..46b50a8 100644 --- a/src/opsscript_gate/cli.py +++ b/src/opsscript_gate/cli.py @@ -226,11 +226,27 @@ def main(argv: list[str] | None = None) -> int: checks.append({"name": "docker", "ok": False, "detail": str(err)}) config_path = Path(".opsscript-gate.json") - checks.append({ - "name": "project", - "ok": True, - "detail": f"Found {config_path}" if config_path.is_file() else "No project config (optional)", - }) + if not config_path.is_file(): + checks.append({ + "name": "project", + "ok": True, + "detail": "No project config (optional)", + }) + else: + try: + load_config(str(config_path)) + except (OSError, ValueError) as err: + checks.append({ + "name": "project", + "ok": False, + "detail": f"Invalid {config_path}: {err}", + }) + else: + checks.append({ + "name": "project", + "ok": True, + "detail": f"Found and validated {config_path}", + }) passed = all(check["ok"] for check in checks) if args.format == "json": print(json.dumps({"status": "PASS" if passed else "FAIL", "checks": checks}, indent=2)) diff --git a/src/opsscript_gate/config.py b/src/opsscript_gate/config.py index 1c8268f..f6a1c9a 100644 --- a/src/opsscript_gate/config.py +++ b/src/opsscript_gate/config.py @@ -90,7 +90,7 @@ def init_project() -> list[str]: - uses: actions/checkout@v7 with: persist-credentials: false - - uses: Mresyzz/opsscript-gate@v0.9.0 + - uses: Mresyzz/opsscript-gate@v0.9.1 with: config: .opsscript-gate.json """, diff --git a/src/opsscript_gate/discovery.py b/src/opsscript_gate/discovery.py index e2cfbef..7999ad4 100644 --- a/src/opsscript_gate/discovery.py +++ b/src/opsscript_gate/discovery.py @@ -40,6 +40,16 @@ MAX_SCRIPT_SIZE_BYTES = 1024 * 1024 # 1 MB +def _matches_exclude(rel_path: str, patterns: list[str] | None) -> bool: + """Match exclusions consistently whether callers use ``foo.sh`` or ``./foo.sh``.""" + normalized_path = rel_path[2:] if rel_path.startswith("./") else rel_path + for pattern in patterns or []: + normalized_pattern = pattern[2:] if pattern.startswith("./") else pattern + if fnmatch.fnmatchcase(normalized_path, normalized_pattern): + return True + return False + + def is_shell_script(file_path: Path, max_size_bytes: int = MAX_SCRIPT_SIZE_BYTES) -> bool: """ Check if a file is a candidate shell script: @@ -101,7 +111,7 @@ def discover_scripts( if is_shell_script(file_path, max_size_bytes=max_size_bytes): try: rel_path = file_path.relative_to(root_path).as_posix() - if any(fnmatch.fnmatchcase(rel_path, pattern) for pattern in (exclude or [])): + if _matches_exclude(rel_path, exclude): continue # Prepend ./ if top-level for standard script path conventions if not rel_path.startswith("./") and "/" not in rel_path: @@ -168,7 +178,7 @@ def discover_changed_scripts( rel_path = resolved.relative_to(root_path).as_posix() except (OSError, ValueError): continue - if any(fnmatch.fnmatchcase(rel_path, pattern) for pattern in (exclude or [])): + if _matches_exclude(rel_path, exclude): continue if not is_shell_script(candidate, max_size_bytes=max_size_bytes): continue diff --git a/src/opsscript_gate/reporter.py b/src/opsscript_gate/reporter.py index c0c57c1..693faf4 100644 --- a/src/opsscript_gate/reporter.py +++ b/src/opsscript_gate/reporter.py @@ -533,10 +533,6 @@ def _sarif_result(result: SingleResult, script_path: str) -> dict[str, object] | "message": {"text": _sarif_message(result)}, "locations": [{"physicalLocation": physical_location}], "properties": properties, - "help": { - "text": rule_name, - "uri": "https://github.com/Mresyzz/opsscript-gate#ci-output", - }, } @@ -552,6 +548,10 @@ def _sarif_run(results: list[dict[str, object]]) -> dict[str, object]: "id": rule_id, "name": rule_name, "shortDescription": {"text": rule_name}, + "help": { + "text": rule_name, + "uri": "https://github.com/Mresyzz/opsscript-gate#ci-output", + }, "helpUri": "https://github.com/Mresyzz/opsscript-gate#ci-output", } for rule_id, rule_name in _SARIF_RULES.values() diff --git a/src/opsscript_gate/runner.py b/src/opsscript_gate/runner.py index 2b41d43..c6f72c3 100644 --- a/src/opsscript_gate/runner.py +++ b/src/opsscript_gate/runner.py @@ -2,6 +2,7 @@ import os import re +import socket import shlex import tempfile import time @@ -59,6 +60,17 @@ # e.g. "::error file=fake.sh::forged" -> "[container] ::error file=fake.sh::forged" _WORKFLOW_COMMAND_LINE_RE = re.compile(r"^(::)", re.MULTILINE) +UTF8_BOM = b"\xef\xbb\xbf" +PACKAGE_SETUP_ERROR_MARKER = "OPSSCRIPT_GATE_SETUP_ERROR:" + + +def _is_wait_timeout_error(exc: Exception) -> bool: + """Return whether a Docker wait exception represents an actual timeout.""" + if isinstance(exc, (TimeoutError, socket.timeout)): + return True + name = exc.__class__.__name__.lower() + return name in {"readtimeout", "readtimeouterror", "sockettimeout"} + def sanitize_log_output(text: str) -> str: """ @@ -258,6 +270,9 @@ def inspect_shebang(script_path: str) -> ShebangParseResult: error_message=f"Failed to read script to parse shebang: {exc}", ) + if first_line_bytes.startswith(UTF8_BOM): + first_line_bytes = first_line_bytes[len(UTF8_BOM):] + raw_content = first_line_bytes.rstrip(b"\r\n") if len(first_line_bytes) > MAX_SHEBANG_BYTES and not first_line_bytes.endswith((b"\n", b"\r")): return ShebangParseResult( @@ -429,20 +444,28 @@ def prepare_script( if not os.path.isfile(abs_path): return abs_path, None - # First pass: stream-check if any CRLF exists without loading the entire file into RAM + # First pass: stream-check if any CRLF or a leading UTF-8 BOM exists without + # loading the entire file into RAM. has_crlf = False + has_bom = False + chunk_size = max(3, chunk_size) with open(abs_path, "rb") as f: prev_byte = b"" + first_chunk = True while True: chunk = f.read(chunk_size) if not chunk: break + if first_chunk: + has_bom = chunk.startswith(UTF8_BOM) + first_chunk = False if b"\r\n" in chunk or (prev_byte == b"\r" and chunk.startswith(b"\n")): has_crlf = True - break + if has_bom: + break prev_byte = chunk[-1:] - if not has_crlf: + if not has_crlf and not has_bom: return abs_path, None # Stream-normalize CRLF into temporary file in fixed-size chunks @@ -450,6 +473,7 @@ def prepare_script( try: with open(abs_path, "rb") as src: carry_cr = False + first_chunk = True while True: chunk = src.read(chunk_size) if not chunk: @@ -457,6 +481,11 @@ def prepare_script( temp_file.write(b"\r") break + if first_chunk: + if chunk.startswith(UTF8_BOM): + chunk = chunk[len(UTF8_BOM):] + first_chunk = False + if carry_cr: if chunk.startswith(b"\n"): # CRLF split across boundary -> write single LF @@ -586,6 +615,9 @@ def _wrap_with_package_setup(command: Sequence[str], packages: Sequence[str]) -> package_args = " ".join(shlex.quote(package) for package in packages) install = ( + "setup_failed() { code=$?; " + f"echo '{PACKAGE_SETUP_ERROR_MARKER} package installation failed (exit code' \"$code\" ')' >&2; " + "exit \"$code\"; }; " "if command -v apt-get >/dev/null 2>&1; then " "mkdir -p /tmp/opsscript-apt-archives/partial && " "apt-get -o APT::Sandbox::User=root " @@ -599,10 +631,10 @@ def _wrap_with_package_setup(command: Sequence[str], packages: Sequence[str]) -> "-o APT::Update::Post-Invoke-Success::= " "-o Dir::Cache::archives=/tmp/opsscript-apt-archives/ " f"install -y --no-install-recommends {package_args} && " - "(rm -rf /tmp/opsscript-apt-archives /var/lib/apt/lists/* || true); " + "(rm -rf /tmp/opsscript-apt-archives /var/lib/apt/lists/* || true) || setup_failed; " "elif command -v apk >/dev/null 2>&1; then " - f"apk add --no-cache {package_args}; " - "else echo 'No supported package manager found (apt-get or apk)' >&2; exit 125; fi" + f"apk add --no-cache {package_args} || setup_failed; " + f"else echo '{PACKAGE_SETUP_ERROR_MARKER} no supported package manager found (apt-get or apk)' >&2; exit 125; fi" ) target_command = " ".join(shlex.quote(part) for part in command) return ["/bin/sh", "-c", f"{install} && exec {target_command}"] @@ -751,14 +783,30 @@ def run_on_distro( # Test doubles and older clients that do not return a status dict fall back # to the bounded polling path below. timed_out = False + wait_error: Exception | None = None wait_result = None wait_fn = getattr(container, "wait", None) if callable(wait_fn): try: wait_result = wait_fn(timeout=max(0.01, float(timeout))) - except Exception: - timed_out = True + except Exception as exc: + if _is_wait_timeout_error(exc): + timed_out = True + else: + wait_error = exc if not isinstance(wait_result, dict) and not timed_out: + if wait_error is not None: + duration = time.perf_counter() - start_time + raw_logs = collect_container_logs(container) + output = sanitize_log_output(raw_logs) + return SingleResult( + distro=distro, + status=DistroStatus.ERROR, + exit_code=None, + duration=duration, + output_snippet=extract_snippet(output), + error_message=f"Container wait failed: {wait_error}", + ) while True: elapsed = time.perf_counter() - start_time if elapsed >= timeout: @@ -799,7 +847,24 @@ def run_on_distro( exit_code = state.get("ExitCode") if exit_code is None: - exit_code = 0 if container.status == "exited" else 1 + return SingleResult( + distro=distro, + status=DistroStatus.ERROR, + exit_code=None, + duration=duration, + output_snippet=extract_snippet(output), + error_message="Container exited without a Docker exit code; execution result is unknown", + ) + + if PACKAGE_SETUP_ERROR_MARKER in output: + return SingleResult( + distro=distro, + status=DistroStatus.ERROR, + exit_code=exit_code, + duration=duration, + output_snippet=extract_snippet(output), + error_message="Container package setup failed before the script ran", + ) if exit_code == 0: return SingleResult( diff --git a/tests/test_onboarding.py b/tests/test_onboarding.py index eab842b..3da26cc 100644 --- a/tests/test_onboarding.py +++ b/tests/test_onboarding.py @@ -57,6 +57,16 @@ def test_doctor_json_reports_docker_failure(project, capsys): assert next(check for check in report["checks"] if check["name"] == "docker")["ok"] is False +def test_doctor_rejects_invalid_project_config(project, capsys): + Path(CONFIG_NAME).write_text('{"timeout":') + with patch("opsscript_gate.cli.get_docker_client"): + assert main(["doctor", "--format", "json"]) == 1 + report = json.loads(capsys.readouterr().out) + check = next(item for item in report["checks"] if item["name"] == "project") + assert check["ok"] is False + assert "Invalid" in check["detail"] + + def test_init_existing_workflow_leaves_config_absent(project): workflow = project / ".github/workflows/opsscript-gate.yml" workflow.parent.mkdir(parents=True) @@ -127,6 +137,7 @@ def test_discovery_limit_and_exclusions(project): with pytest.raises(ValueError, match="no scripts were executed"): discover_scripts(max_scripts=1) assert discover_scripts(max_scripts=1, exclude=["tests/*"]) == ["./install.sh"] + assert discover_scripts(max_scripts=1, exclude=["./install.sh", "tests/*"]) == [] with patch("opsscript_gate.cli.run_matrix") as run: assert main(["run", "--max-scripts", "1"]) == 1 run.assert_not_called() @@ -158,6 +169,13 @@ def test_changed_discovery_uses_git_paths_and_filters_candidates(tmp_path): assert "origin/main...HEAD" in run.call_args.args[0] +def test_changed_discovery_matches_dot_slash_exclusions(tmp_path): + (tmp_path / "install.sh").write_text("#!/bin/sh\necho ok\n") + completed = type("Completed", (), {"stdout": b"install.sh\0", "stderr": b""})() + with patch("opsscript_gate.discovery.subprocess.run", return_value=completed): + assert discover_changed_scripts(str(tmp_path), exclude=["./install.sh"]) == [] + + @pytest.mark.parametrize("revision", ["", "--output=bad", "origin/main bad"]) def test_changed_discovery_rejects_invalid_revision(tmp_path, revision): with pytest.raises(ValueError, match="changed-since"): diff --git a/tests/test_runner.py b/tests/test_runner.py index 98a6ad8..d163d2d 100644 --- a/tests/test_runner.py +++ b/tests/test_runner.py @@ -226,6 +226,29 @@ def test_run_on_distro_package_setup_wraps_command(tmp_path): assert "APT::Update::Post-Invoke-Success::=" in command[2] assert "apk add --no-cache curl ca-certificates" in command[2] assert "exec /bin/sh -c" in command[2] + assert "OPSSCRIPT_GATE_SETUP_ERROR:" in command[2] + + +def test_run_on_distro_package_setup_failure_is_error(tmp_path): + script_file = tmp_path / "test.sh" + script_file.write_text("#!/bin/sh\necho should-not-run\n", encoding="utf-8") + + mock_client = mock.MagicMock() + mock_container = mock.MagicMock() + mock_container.status = "exited" + mock_container.attrs = {"State": {"ExitCode": 125}} + mock_container.logs.return_value = ( + b"OPSSCRIPT_GATE_SETUP_ERROR: no supported package manager found (apt-get or apk)\n" + ) + mock_client.containers.create.return_value = mock_container + + result = run_on_distro( + mock_client, str(script_file), "scratch:latest", packages=["curl"], network="bridge" + ) + + assert result.status == DistroStatus.ERROR + assert result.exit_code == 125 + assert "package setup failed" in (result.error_message or "") def test_package_setup_requires_network(tmp_path): @@ -266,6 +289,42 @@ def test_run_on_distro_fail_mock(tmp_path): mock_container.remove.assert_called_once_with(force=True) +def test_run_on_distro_missing_exit_code_fails_closed(tmp_path): + script_file = tmp_path / "unknown.sh" + script_file.write_text("#!/bin/sh\necho unknown\n", encoding="utf-8") + + mock_client = mock.MagicMock() + mock_container = mock.MagicMock() + mock_container.status = "exited" + mock_container.attrs = {"State": {}} + mock_container.logs.return_value = b"unknown\n" + mock_client.containers.create.return_value = mock_container + + result = run_on_distro(mock_client, str(script_file), "debian:12-slim") + + assert result.status == DistroStatus.ERROR + assert result.exit_code is None + assert "without a Docker exit code" in (result.error_message or "") + + +def test_run_on_distro_wait_api_error_is_not_timeout(tmp_path): + script_file = tmp_path / "wait-error.sh" + script_file.write_text("#!/bin/sh\necho wait\n", encoding="utf-8") + + mock_client = mock.MagicMock() + mock_container = mock.MagicMock() + mock_container.wait.side_effect = RuntimeError("Docker daemon disconnected") + mock_container.logs.return_value = b"wait failed\n" + mock_client.containers.create.return_value = mock_container + + result = run_on_distro(mock_client, str(script_file), "debian:12-slim") + + assert result.status == DistroStatus.ERROR + assert result.status != DistroStatus.TIMED_OUT + assert "wait failed" in (result.error_message or "") + mock_container.kill.assert_not_called() + + def test_run_on_distro_timeout_kill_mock(tmp_path): script_file = tmp_path / "hang.sh" script_file.write_text("#!/bin/sh\nsleep 100\n", encoding="utf-8") @@ -400,6 +459,8 @@ def test_reporter_sarif_contains_failure_location_and_metadata(): assert finding["locations"][0]["physicalLocation"]["region"]["startLine"] == 4 assert finding["properties"]["distribution"] == "alpine:3.20" assert finding["properties"]["command"] == "apt-get" + assert "help" not in finding + assert payload["runs"][0]["tool"]["driver"]["rules"][0]["help"]["uri"].endswith("#ci-output") def test_reporter_multi_sarif_keeps_script_paths(): @@ -597,6 +658,17 @@ def test_inspect_shebang_matrix(tmp_path): assert res_crlf.status == ShebangStatus.RECOGNIZED assert res_crlf.interpreter == "bash" + bom_file = tmp_path / "bom.sh" + bom_file.write_bytes(b"\xef\xbb\xbf#!/bin/sh\necho hi\r\n") + res_bom = inspect_shebang(str(bom_file)) + assert res_bom.status == ShebangStatus.RECOGNIZED + prepared_bom, temp_bom = prepare_script(str(bom_file)) + try: + assert open(prepared_bom, "rb").read().startswith(b"#!/bin/sh\n") + finally: + if temp_bom is not None: + temp_bom.close() + # 3. Leading whitespace before #! must be treated as missing lead_space = tmp_path / "lead_space.sh" lead_space.write_text(" #!/bin/sh\necho hi\n", encoding="utf-8")