diff --git a/.ai/v0.9.2_issues_report.md b/.ai/v0.9.2_issues_report.md deleted file mode 100644 index c70d5ad..0000000 --- a/.ai/v0.9.2_issues_report.md +++ /dev/null @@ -1,150 +0,0 @@ -# OpsScript Gate 本地 v0.9.2 代码审查、缺陷排查与修复报告 - -**审查与修复时间**：2026-10-05 -**基准分支**：`codex/v0.9.2-hardening` -**测试套件状态**：`pytest` 182 项测试全量通过（170 passed, 12 skipped, 0 failures） - ---- - -## 概述 - -针对本地 `0.9.2` 版本在近期提交（`6260a74` 到 `cae3fbe`）及后续排查中发现的所有缺陷，进行了地毯式分析与代码修复。所有发现的问题均已完成第一性原理剖析、修复并配齐自动化回归测试。 - ---- - -## 本轮新发现的问题与修复记录 - -### 1. `discovery.py` 中 `_is_ignored_path` 误将前导 `./` 路径判定为忽略项 -* **文件位置**：`src/opsscript_gate/discovery.py` -* **问题成因**： - `_is_ignored_path` 将路径按 `/` 切分后遍历每个 `part`，直接使用 `part.startswith(".")`。当路径是以 `./` 开头（例如 `./scripts/install.sh` 或 `./run.sh`）时，首个 `part` 为单个点 `.`，`'.'.startswith(".")` 恒为 `True`，导致合法脚本被错误判定为忽略项。 -* **修复方案**： - 在切分前先剥离前导 `./`，并将 `part in (".", "..")` 显式排除在隐藏目录判断之外。 -* **测试用例**： - 新增 `tests/test_runner.py::test_is_ignored_path_dot_slash_and_hidden`。 - ---- - -### 2. `runner.py` 中 `_is_wait_timeout_error` 无法识别嵌套异常原因链 -* **文件位置**：`src/opsscript_gate/runner.py` -* **问题成因**： - Docker SDK (urllib3 / requests) 发生超时异常时，常将底层的 `ReadTimeout` 或 `SocketTimeout` 包装在 `requests.exceptions.ConnectionError` 或顶层异常中。原函数只检查了外层异常类名，导致被包装的真实超时被误报为通信崩溃（`DistroStatus.ERROR`），且未能触发超时强杀逻辑。 -* **修复方案**： - 在 `_is_wait_timeout_error` 中遍历 `__cause__` 与 `__context__` 异常链，全面识别深层超时错误。 -* **测试用例**： - 新增 `tests/test_runner.py::test_is_wait_timeout_error_cause_chain`。 - ---- - -### 3. `cli.py` 中 `--output` 指定已有目录时缺乏前置拦截 -* **文件位置**：`src/opsscript_gate/cli.py` -* **问题成因**： - 若用户误将已存在的目录传给 `--output`（例如 `--output reports/` 或 `--output .`），原逻辑在跑完所有耗时的容器矩阵测试后，才在 `publish_output` 中因 `write_text` 抛出 `IsADirectoryError` 或 `PermissionError`，严重浪费时间。 -* **修复方案**： - 在执行矩阵前增加 `if output_path.is_dir(): sys.stderr.write("Error: output must name a file, not an existing directory\n"); return 1` 提前拦截。 -* **测试用例**： - 新增 `tests/test_runner.py::test_cli_output_rejects_existing_directory_early`。 - ---- - -### 4. `remediation.py` 扩充跨发行版包管理器互踩诊断规则 -* **文件位置**：`src/opsscript_gate/remediation.py` -* **问题成因**： - 此前仅包含 Debian/Ubuntu（apt）与 Alpine（apk）的双向匹配。当脚本在 Debian/Ubuntu/Alpine 上尝试调用 `yum` 或 `dnf`，或者在 Fedora/CentOS/RHEL 系列镜像上调用 `apt`/`apk` 时，系统只能报通用缺失错误，缺乏精准指引。 -* **修复方案**： - 新增 `_match_debian_ubuntu_yum`、`_match_alpine_yum`、`_match_rhel_fedora_apt` 匹配器及专属修复建议。 -* **测试用例**： - 新增 `tests/test_runner.py::test_remediation_enterprise_distro_package_rules`。 - ---- - -### 5. `runner.py` 中保护 `container.reload()` 避免网络抖动覆盖已取得的状态码 -* **文件位置**：`src/opsscript_gate/runner.py` -* **问题成因**： - 在容器执行完毕后，`container.wait()` 已返回了包含 `StatusCode` 的字典，但随后的 `container.reload()` 调用缺乏异常防护。一旦容器刚好在退出瞬间发生通信抖动，会导致原本已成功完成的测试异常崩溃。 -* **修复方案**： - 为 `container.reload()` 增加 `try-except` 防护，优先并安全地使用已拿到的有效退出码。 - ---- - -### 6. `remediation.py` 中 `curl`/`wget` 绝对路径匹配与最小容器缺失 `sudo` 修复规则 -* **文件位置**：`src/opsscript_gate/remediation.py` -* **问题成因**： - 1. `_match_curl` 与 `_match_wget` 仅检查了命令名完全等于 `curl` / `wget`，当脚本使用 `/usr/bin/curl`、`/bin/curl` 等绝对路径调用时，无法匹配到建议规则。 - 2. 最小化 Linux 容器镜像（Debian slim、Ubuntu、Alpine 等）默认不带 `sudo`，且容器通常本身即为 root 权限。当运维脚本带有 `sudo` 时会报 `sudo: not found`，此前缺乏专属修复建议。 -* **修复方案**： - 提取命令路径中的 basename 统一进行匹配，并新增 `_match_sudo` 规则与针对性修复建议。 -* **测试用例**： - 新增 `tests/test_runner.py::test_remediation_curl_wget_path_and_sudo`。 - ---- - -### 7. `reporter.py` 中 `_sarif_path` 与 `format_github_annotations` 非前导路径穿越漏洞 -* **文件位置**：`src/opsscript_gate/reporter.py` -* **问题成因**： - 原代码仅使用 `startswith("../")` 检测路径逃逸，当相对路径为 `subdir/../../outside.sh` 时无法识别，导致逃逸路径被直接写入 SARIF 的 `artifactLocation.uri` 与 GitHub Actions `::error file=...`。 -* **修复方案**： - 引入 `os.path.normpath` 彻底归一化，凡逃逸出工作区根目录者统一安全回退至 `os.path.basename`。 -* **测试用例**： - 新增 `tests/test_runner.py::test_sarif_and_annotations_path_traversal_hardened`。 - ---- - -### 8. `cli.py` 中 `--output` 缺乏对 `.git` 目录与工程核心文件的防覆写防护 -* **文件位置**：`src/opsscript_gate/cli.py` -* **问题成因**： - 原逻辑仅保护了脚本自身与配置文件，若用户指定 `--output pyproject.toml` 或 `--output .git/config`，会导致工程配置与版本库元数据被截断覆盖。 -* **修复方案**： - 严禁向 `.git` 内部路径写入，并定义 `CRITICAL_PROJECT_FILES`（包含 `pyproject.toml`、`action.yml` 等）加入严格保护列表。 -* **测试用例**： - 新增 `tests/test_runner.py::test_cli_output_protects_git_and_critical_files`。 - ---- - -### 9. `runner.py` 中 Docker 容器 OOM-Killed 强杀状态识别 -* **文件位置**：`src/opsscript_gate/runner.py` -* **问题成因**： - 容器超出 `mem_limit` 时被 Linux 内核 OOM Killer 强杀（退出码 137），原代码仅笼统报非零退出码，丢失了内存超限关键诊断信息。 -* **修复方案**： - 读取容器 `State.OOMKilled` 标识，当为 True 时明确标注 `Container exceeded memory limit ({mem_limit}) and was killed by OOM killer`。 -* **测试用例**： - 新增 `tests/test_runner.py::test_runner_oom_killed_detection`。 - ---- - -### 10. `discovery.py` 中前导斜杠排除模式与 `..` 相对路径逃逸判定修正 -* **文件位置**：`src/opsscript_gate/discovery.py` -* **问题成因**： - 1. `_matches_exclude` 未剥离前导 `/`，导致 `/tests/*` 模式与相对路径匹配失败； - 2. `_is_ignored_path` 漏判了包含 `..` 的逃逸路径。 -* **修复方案**： - 排除模式规整化剥离前导斜杠，并将 `..` 明确划入忽略判定。 -* **测试用例**： - 新增 `tests/test_runner.py::test_matches_exclude_leading_slash_and_dotdot`。 - ---- - -### 11. 核心模块边界条件与异常路径测试覆盖盲区加固 -* **文件位置**：`src/opsscript_gate/config.py`, `src/opsscript_gate/discovery.py`, `src/opsscript_gate/reporter.py`, `src/opsscript_gate/remediation.py` -* **问题成因**： - 1. `config.py` 中的 `validate_packages` 与 `validate_memory_limit` 缺乏纯函数级边界输入断言（如类型非法、去重、空白截断、注入符号等）；`load_config` 缺乏单测覆盖超 256 KiB 上限、非字典、未知字段及互斥配置； - 2. `discovery.py` 中 `discover_changed_scripts` 缺少对 Git 缺失（`FileNotFoundError`）、Git 执行报错（`CalledProcessError`）、非法 revision 及变更数量超限的异常分支测试； - 3. `reporter.py` 中的不可见字符/控制字符消毒函数（`sanitize_display_text`, `sanitize_workflow_text`）缺乏单测；SARIF 规则映射仅测试过 `FAIL`，缺失 `TIMED_OUT` (OSG002)、`ERROR` (OSG003)、`FLAKY` (OSG004) 映射断言； - 4. `remediation.py` 中 `_match_interpreter` 缺少对 `/bin/...` 与 `/usr/bin/...` 绝对路径解释器的匹配测试。 -* **修复方案**： - 构建 `tests/test_audit_coverage.py` 补充 19 项针对性单元测试，覆盖全部上述盲区与异常边界。 -* **测试用例**： - `tests/test_audit_coverage.py`（共 19 项测试用例全部通过）。 - ---- - -## 历史排查问题的闭环确认（已在近期提交中合入） - -1. **`reporter.py` 中 `_sarif_path` 绝对路径转换**：已在 Commit `6a830e4` 中引入 `os.path.relpath` 与防止 `../` 逃逸的处理。 -2. **`discovery.py` 中 `discover_scripts` 过滤隐藏文件**：已在 Commit `9618874` 中过滤了 `filename.startswith(".")`。 -3. **`action.yml` 中 `dry-run` 大小写敏感**：已在 Commit `03b30ab` 中使用 `tr '[:upper:]' '[:lower:]'` 规整。 -4. **`cli.py` 中 `publish_output` 写文件失败吞没 stdout**：已在 Commit `03b30ab` 中调整为先打印控制台输出再写文件。 -5. **`runner.py` 中多轮执行代表性结果选择**：已在 Commit `6a830e4` 中优化为优先保留具备 `diagnostic` 的结果。 -6. **Docker 引擎模式检查**：已在 Commit `0b8a202` 中通过 `client.info()` 校验 `OSType == "linux"`。 -7. **资源上限守卫**：已在 Commit `cae3fbe`, `13f0730`, `373d5a0` 中为 `matrix`（最多32个）、`timeout`（最多3600秒）、`repeat`（最多20次）增加上限校验。 - diff --git a/.gitignore b/.gitignore index f26567d..490b734 100644 --- a/.gitignore +++ b/.gitignore @@ -50,3 +50,4 @@ venv.bak/ *~ .DS_Store Thumbs.db +.ai/ diff --git a/src/opsscript_gate/cli.py b/src/opsscript_gate/cli.py index 4add870..a70c4c4 100644 --- a/src/opsscript_gate/cli.py +++ b/src/opsscript_gate/cli.py @@ -489,7 +489,8 @@ def publish_output(output: str, filename: str | None) -> bool: try: path = Path(filename) path.parent.mkdir(parents=True, exist_ok=True) - path.write_text(output + "\n", encoding="utf-8") + with path.open("w", encoding="utf-8", newline="\n") as stream: + stream.write(output + "\n") except OSError as err: sys.stderr.write(f"Error writing report: {err}\n") return False diff --git a/src/opsscript_gate/discovery.py b/src/opsscript_gate/discovery.py index 846a18c..599ae01 100644 --- a/src/opsscript_gate/discovery.py +++ b/src/opsscript_gate/discovery.py @@ -78,6 +78,7 @@ def is_shell_script(file_path: Path, max_size_bytes: int = MAX_SCRIPT_SIZE_BYTES Check if a file is a candidate shell script: - Must be a regular file, never a symlink. - File size must not exceed max_size_bytes. + - Must be readable by the current process. - Extension is .sh or .bash, OR the first line is a recognized shell shebang. """ try: @@ -87,14 +88,13 @@ def is_shell_script(file_path: Path, max_size_bytes: int = MAX_SCRIPT_SIZE_BYTES if stat.st_size == 0 or stat.st_size > max_size_bytes: return False + with open(file_path, "rb") as f: + first_line_bytes = f.readline(256) + ext = file_path.suffix.lower() if ext in (".sh", ".bash"): return True - # If extension is not .sh/.bash, inspect the first line - with open(file_path, "rb") as f: - first_line_bytes = f.readline(256) - first_line = first_line_bytes.decode("utf-8-sig", errors="ignore").strip() return any(first_line == prefix or first_line.startswith(prefix + " ") for prefix in SHELL_SHEBANG_PREFIXES) diff --git a/src/opsscript_gate/runner.py b/src/opsscript_gate/runner.py index d33fef8..aded70d 100644 --- a/src/opsscript_gate/runner.py +++ b/src/opsscript_gate/runner.py @@ -832,6 +832,8 @@ def run_on_distro( # Line-ending defense & Windows-safe path preparation own_temp_file: tempfile.NamedTemporaryFile | None = None + container = None + start_time = time.perf_counter() if prepared_script_path is not None: mount_src = prepared_script_path else: @@ -847,35 +849,31 @@ def run_on_distro( error_message=f"Failed to read/prepare script: {exc}", ) - safe_mount_src = normalize_host_path_for_docker(mount_src) try: - container_command = _wrap_with_package_setup(command, package_list) - except (TypeError, ValueError) as exc: - return SingleResult( - distro=distro, - status=DistroStatus.ERROR, - exit_code=None, - duration=0.0, - output_snippet="", - error_message=str(exc), - ) + safe_mount_src = normalize_host_path_for_docker(mount_src) + try: + container_command = _wrap_with_package_setup(command, package_list) + except (TypeError, ValueError) as exc: + return SingleResult( + distro=distro, + status=DistroStatus.ERROR, + exit_code=None, + duration=0.0, + output_snippet="", + error_message=str(exc), + ) - # Security: Strict unprivileged options and read-only mount - volumes = { - safe_mount_src: { - "bind": "/tmp/target_script.sh", - "mode": "ro", + # Security: Strict unprivileged options and read-only mount + volumes = { + safe_mount_src: { + "bind": "/tmp/target_script.sh", + "mode": "ro", + } + } + environment = { + "DEBIAN_FRONTEND": "noninteractive", + "CI": "true", } - } - environment = { - "DEBIAN_FRONTEND": "noninteractive", - "CI": "true", - } - - container = None - start_time = time.perf_counter() - - try: create_kwargs = { "image": distro, "command": container_command, @@ -1076,13 +1074,46 @@ def run_matrix( except ValueError: mode = None + distro_list = _normalize_matrix(matrix) + abs_script = os.path.abspath(script_path) + if not os.path.exists(abs_script): + return RunReport( + results=[ + SingleResult( + distro=d, + status=DistroStatus.ERROR, + exit_code=None, + duration=0.0, + output_snippet="", + error_message=f"Target script does not exist: {abs_script}", + ) + for d in distro_list + ], + total_duration=0.0, + all_passed=False, + ) + if not os.path.isfile(abs_script): + return RunReport( + results=[ + SingleResult( + distro=d, + status=DistroStatus.ERROR, + exit_code=None, + duration=0.0, + output_snippet="", + error_message=f"Target script path is not a regular file: {abs_script}", + ) + for d in distro_list + ], + total_duration=0.0, + all_passed=False, + ) + parsed_shebang: ShebangParseResult | None = None if mode is not None and mode != ShellMode.POSIX: - if os.path.isfile(abs_script): - parsed_shebang = inspect_shebang(abs_script) + parsed_shebang = inspect_shebang(abs_script) - distro_list = _normalize_matrix(matrix) if isinstance(packages, (str, bytes)): raise ValueError("packages must be a list of package names") package_list = list(packages or []) @@ -1102,26 +1133,23 @@ def run_matrix( # Prepare script once for all parallel matrix workers shared_temp_file: tempfile.NamedTemporaryFile | None = None - prepared_script: str = abs_script - if os.path.isfile(abs_script): - try: - prepared_script, shared_temp_file = prepare_script(abs_script) - except Exception as exc: - return RunReport( - results=[ - SingleResult( - distro=d, - status=DistroStatus.ERROR, - exit_code=None, - duration=0.0, - output_snippet="", - error_message=f"Failed to read/prepare script: {exc}", - ) - for d in distro_list - ], - total_duration=0.0, - all_passed=False, - ) + try: + prepared_script, shared_temp_file = prepare_script(abs_script) + except Exception as exc: + return RunReport( + results=[ + SingleResult( + distro=d, + status=DistroStatus.ERROR, + exit_code=None, + duration=0.0, + output_snippet="", + error_message=f"Failed to read/prepare script: {exc}", + ) + for d in distro_list + ], + total_duration=0.0, + ) start_total = time.perf_counter() results: list[SingleResult] = [None] * len(distro_list) # type: ignore diff --git a/tests/test_audit_coverage.py b/tests/test_audit_coverage.py index f0025d3..979ee0c 100644 --- a/tests/test_audit_coverage.py +++ b/tests/test_audit_coverage.py @@ -289,3 +289,79 @@ def test_remediation_hint_for_interpreter(): # 不匹配时返回 None assert generate_remediation_hint(distro="ubuntu:22.04", command="/usr/bin/unknown", kind="missing_command") is None + + +# ============================================================================== +# 5. 发现的新问题与防御加固测试 +# ============================================================================== + +def test_is_shell_script_unreadable_file(tmp_path): + from opsscript_gate.discovery import is_shell_script + + unreadable = tmp_path / "denied.sh" + unreadable.write_text("#!/bin/sh\necho test\n", encoding="utf-8") + + # 模拟读权限被拒绝（PermissionError） + with mock.patch("builtins.open", side_effect=PermissionError("Permission denied")): + assert is_shell_script(unreadable) is False + + +def test_run_on_distro_cleans_up_own_temp_file_on_error(tmp_path): + import os + from opsscript_gate.runner import run_on_distro + + # 创建包含 CRLF 的脚本以触发 prepare_script 生成临时文件 + script = tmp_path / "crlf_pkg_fail.sh" + script.write_bytes(b"#!/bin/sh\r\necho hello\r\n") + + created_temp_files = [] + original_named_temp = __import__("tempfile").NamedTemporaryFile + + def spy_temp_file(*args, **kwargs): + tf = original_named_temp(*args, **kwargs) + created_temp_files.append(tf.name) + return tf + + with mock.patch("tempfile.NamedTemporaryFile", side_effect=spy_temp_file): + with mock.patch("opsscript_gate.runner._wrap_with_package_setup", side_effect=ValueError("Setup generation failed")): + result = run_on_distro( + client=mock.MagicMock(), + script_path=str(script), + distro="debian:12-slim", + packages=["curl"], + ) + assert result.status == DistroStatus.ERROR + assert "Setup generation failed" in (result.error_message or "") + + # 验证创建的临时文件在返回后已经被完全清理，没有发生泄漏 + assert len(created_temp_files) >= 1 + for temp_path in created_temp_files: + assert not os.path.exists(temp_path) + + +def test_run_matrix_directory_and_missing_script_preflight(tmp_path): + from opsscript_gate.runner import run_matrix + + missing = str(tmp_path / "does_not_exist.sh") + report_missing = run_matrix(missing, matrix=["debian:12-slim"]) + assert report_missing.all_passed is False + assert len(report_missing.results) == 1 + assert "Target script does not exist" in (report_missing.results[0].error_message or "") + + is_dir = str(tmp_path) + report_dir = run_matrix(is_dir, matrix=["debian:12-slim"]) + assert report_dir.all_passed is False + assert len(report_dir.results) == 1 + assert "not a regular file" in (report_dir.results[0].error_message or "") + + +def test_publish_output_preserves_posix_newline(tmp_path, capsys): + from opsscript_gate.cli import publish_output + + out_file = tmp_path / "report_clean.txt" + publish_output("hello\nworld", str(out_file)) + + # 验证无论在何种操作系统下写入，都严格保持标准 LF 换行，绝无 \r\n 污染 + raw_bytes = out_file.read_bytes() + assert raw_bytes == b"hello\nworld\n" + assert b"\r" not in raw_bytes