diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 2a73d09..ce02852 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -67,10 +67,29 @@ jobs: ~/.cargo/git/db/ collect-diff-context-cli/target/ key: ${{ runner.os }}-cargo-${{ hashFiles('collect-diff-context-cli/Cargo.lock') }} + - name: Fetch pinned Gitleaks binary + run: ./scripts/fetch_gitleaks.sh --platform linux-amd64 + - name: Verify Gitleaks trust chain + run: ./install.sh --doctor + - name: Build current helper source + run: | + cargo build --release --manifest-path collect-diff-context-cli/Cargo.toml + echo "PRE_COMMIT_REVIEW_RUST_BIN=$GITHUB_WORKSPACE/collect-diff-context-cli/target/release/collect-diff-context-cli" >> "$GITHUB_ENV" - name: Run collect_diff_context_test.sh run: ./tests/collect_diff_context_test.sh - name: Run parity_golden_test.sh run: ./tests/parity_golden_test.sh + - name: Run secret_gate_test.sh + run: ./tests/secret_gate_test.sh + - name: Run gitleaks_distribution_test.sh + run: ./tests/gitleaks_distribution_test.sh + - name: Run install_gitleaks_test.sh + run: ./tests/install_gitleaks_test.sh + - name: Run output quality comparison self-test + run: | + ./evals/output_eval_runner_test.sh + ./evals/compare_output_eval_quality_test.sh + ./evals/eval_contract_test.sh - name: Validate JSON schemas run: | pip install jsonschema diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 5656775..0f5f94e 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -20,19 +20,23 @@ jobs: - os: ubuntu-latest target: x86_64-unknown-linux-musl artifact_name: collect_diff_context-linux-amd64 + gitleaks_platform: linux-amd64 use_musl: true - os: macos-latest target: aarch64-apple-darwin artifact_name: collect_diff_context-darwin-arm64 + gitleaks_platform: darwin-arm64 - os: macos-13 target: x86_64-apple-darwin artifact_name: collect_diff_context-darwin-amd64 + gitleaks_platform: darwin-amd64 - os: windows-latest target: x86_64-pc-windows-msvc artifact_name: collect_diff_context-windows-amd64.exe + gitleaks_platform: windows-amd64 steps: - name: Checkout repository @@ -61,11 +65,15 @@ jobs: cp collect-diff-context-cli/target/${{ matrix.target }}/release/collect-diff-context-cli dist/${{ matrix.artifact_name }} fi + - name: Fetch pinned Gitleaks binary + shell: bash + run: ./scripts/fetch_gitleaks.sh --platform "${{ matrix.gitleaks_platform }}" --dest dist + - name: Upload Build Artifact uses: actions/upload-artifact@v4 with: name: ${{ matrix.artifact_name }} - path: dist/${{ matrix.artifact_name }} + path: dist/* create-release: name: Create GitHub Release @@ -81,13 +89,27 @@ jobs: with: path: artifacts + - name: Build self-contained skill package + shell: bash + run: | + mkdir -p dist/pre-commit-review + cp SKILL.md LICENSE dist/pre-commit-review/ + cp -R agents references scripts THIRD_PARTY_LICENSES dist/pre-commit-review/ + find artifacts -type f -name 'collect_diff_context-*' -exec cp {} dist/pre-commit-review/scripts/bin/ \; + find artifacts -type f -name 'gitleaks-*' -exec cp {} dist/pre-commit-review/scripts/bin/ \; + chmod +x dist/pre-commit-review/scripts/collect_diff_context.sh + chmod +x dist/pre-commit-review/scripts/check_gitleaks.sh + chmod +x dist/pre-commit-review/scripts/bin/collect_diff_context-* || true + chmod +x dist/pre-commit-review/scripts/bin/gitleaks-* || true + dist/pre-commit-review/scripts/check_gitleaks.sh + tar -czf dist/pre-commit-review-runtime.tar.gz -C dist pre-commit-review + - name: Create Release uses: softprops/action-gh-release@v2 with: files: | - artifacts/collect_diff_context-linux-amd64/collect_diff_context-linux-amd64 - artifacts/collect_diff_context-darwin-arm64/collect_diff_context-darwin-arm64 - artifacts/collect_diff_context-darwin-amd64/collect_diff_context-darwin-amd64 - artifacts/collect_diff_context-windows-amd64.exe/collect_diff_context-windows-amd64.exe + artifacts/**/collect_diff_context-* + artifacts/**/gitleaks-* + dist/pre-commit-review-runtime.tar.gz env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} diff --git a/.gitignore b/.gitignore index dfdde51..fb54cb6 100644 --- a/.gitignore +++ b/.gitignore @@ -24,5 +24,8 @@ __pycache__/ target/ *.rs.bk +# Release-staged third-party binaries +scripts/bin/gitleaks-* + # superpowers -docs/superpowers/ \ No newline at end of file +docs/superpowers/ diff --git a/README.md b/README.md index f728aa9..881e9b1 100644 --- a/README.md +++ b/README.md @@ -133,6 +133,7 @@ For a blocking issue the verdict is `DO_NOT_COMMIT` with a `🔒`-marked blocker - A supported AI coding agent runtime that can load skills (Codex, Claude Code, Gemini CLI, or Kiro). The skill package ships no runtime of its own. - `git` on `PATH` for local diff collection. The review still works without it when you paste a diff or code directly. +- Network access is optional. From a source clone, `install.sh` attempts to download the pinned Gitleaks `8.30.1` binary and verify both the release archive and extracted executable SHA256. Self-contained release packages already include the verified executable. If download is disabled, unavailable, or fails, installation and review still work without local secret redaction. Implicit `PATH` discovery is not allowed. - A Unix-compatible shell to run `install.sh` and the helper. On Windows use Git Bash, MSYS2, or WSL. ## Quick Install @@ -146,6 +147,8 @@ From a clone of this repository, install globally for any supported agent: ./install.sh --agent kiro-cli ``` +These commands attempt to provision the pinned current-platform Gitleaks binary during installation. This is an installer action initiated by the user; the Agent review workflow never downloads tools. Provisioning failure is reported as a warning and does not prevent installation or review. + List every supported agent id and its project/global paths: ```bash @@ -169,6 +172,8 @@ Useful flags: - `--dir PATH` overrides the target skills directory - `--force` replaces an existing non-managed target - `--dry-run` prints what would happen without changing anything +- `--no-download` skips the optional Gitleaks download; review remains available without secret redaction +- `--doctor` diagnoses scanner source, version, bundled SHA256, trusted configuration, and stdin/JSON capability without installing a skill; it exits non-zero when redaction is unavailable but does not imply that review is blocked Examples: @@ -283,6 +288,8 @@ This repository is not an application or framework. It is a small, portable skil ├── output/ ├── taxonomy/ ├── eval_contract_test.sh + ├── compare_output_eval_quality.sh + ├── compare_output_eval_quality_test.sh ├── readme_surface_test.sh ├── readme_host_entrypoints_test.sh ├── output-eval.json @@ -321,15 +328,16 @@ Defines the skill itself: ### `scripts/collect_diff_context.sh` -A read-only helper script that gathers local repository context for the review workflow. It does three jobs: +A read-only helper script that gathers local repository context for the review workflow. It does four jobs: 1. **Diff source resolution** — detects whether the cwd is a Git repository, prefers staged changes, falls back to unstaged or branch-vs-base, and reports diff stats, file lists, status, truncation, high-risk candidates, generated-like/lock files, and top-churn files. Rename, delete, binary, mode-only, and submodule pointer changes are recorded as manifest units. 2. **A bounded control plane** — emits a compact `--control-plane` JSON gateway with an authoritative full-scope content fingerprint, per-unit fingerprints, bounded units/groups, work order, and reusable command templates; supports `--expect-scope ` on follow-up retrieval so stale output fails closed; and disables external diff/textconv drivers so snapshot identity and inspected content stay aligned. 3. **Coverage-led + test-selection hints** — emits a Review Manifest/Groups and reducer-friendly structured sections (Review Plan JSON, split suggestions, ledgers, work packets, finalization templates), bounded read-only Semantic Context Queries, and Test Selection Hints for changed test files that look environment-dependent, including common JVM/Spring/Quarkus/Micronaut, Maven/Gradle integration naming, JUnit tags, Testcontainers, Docker Compose, WireMock/MockServer, pytest markers, Playwright/Cypress/Node e2e, Go build tags, Rust ignored/integration tests, and database/cache/broker/search service configuration. +4. **Optional local secret redaction** — when a trusted Gitleaks installation is available, scans and redacts each full selected diff before applying its output byte limit, replaces detected match ranges with `[redacted:]`, rescans the sanitized view, and sanitizes captured wrapper stdout/stderr. This ordering prevents a detected credential crossing the truncation boundary from leaking as an unmatched prefix. If the scanner is disabled, unavailable, times out, or returns no finding, review continues with the original output. If Gitleaks returns a finding but local span mapping or verification fails, the helper reports `status: redaction-failed` rather than calling the scanner unavailable; this path also continues with the original output and never withholds the review material. The full list of emitted sections (Coverage Ledger Template, Group Review Work Packets, Reducer State Snapshot, etc.) is documented in [`docs/helper-capabilities.md`](./docs/helper-capabilities.md) for integrators building reducer/subagent automation. -It does not fetch, stage, reset, install, or modify files. +The review entrypoint does not fetch, stage, reset, install, or modify files. During an explicit user-initiated installation, `install.sh` invokes `scripts/fetch_gitleaks.sh` when the current-platform binary is not already bundled. The fetcher downloads only repository-pinned upstream assets and verifies pinned SHA256 values for both the archive and extracted executable. Download progress is shown automatically on an interactive terminal; use `PRE_COMMIT_REVIEW_FETCH_PROGRESS=always` when output is captured, or `never` to suppress it. `--dry-run` never downloads, and `--no-download` skips this optional installer behavior. Run `./install.sh --doctor` to diagnose whether local redaction is available. It does not run, rewrite, or skip tests. Test Selection Hints are read-only guidance for choosing focused verification commands and for distinguishing sandbox failures from code failures. A `no-known-env-heavy-marker` hint is not proof that a test is isolated; it only means the helper did not match a known environment-heavy marker. The review workflow starts with `scripts/collect_diff_context.sh --control-plane`. This bounded gateway emits no raw diff and is authoritative only when its collection-start and collection-end fingerprints match. The legacy default output remains plan-first and may omit the global raw diff. `PRE_COMMIT_REVIEW_INLINE_DIFF_BYTES` (default `60000`) controls when that default output inlines the global diff. `PRE_COMMIT_REVIEW_MAX_DIFF_BYTES` (default `200000`) controls truncation for a diff that is actually emitted; use `0` only when printing the full diff is safe. @@ -346,14 +354,21 @@ Review group budgets default to 120KB target and 160KB hard limit. Override them ### Rollout and Multi-Implementation Controls The entrypoint wrapper `scripts/collect_diff_context.sh` supports multiple execution modes for transition and safety: - `PRE_COMMIT_REVIEW_HELPER_IMPL`: Specifies the helper implementation mode. - - `rust` (default): Executes the compiled Rust CLI binary. If it fails, prints a warning to `stderr` and gracefully falls back to the legacy script `collect_diff_context.legacy.sh`. + - `rust` (default): Executes the compiled Rust CLI binary. Collection failures may fall back to the legacy script. Secret-scan failures do not trigger a special fallback or block output; the selected implementation continues without redaction and reports the downgrade. - `legacy` or `shell`: Forces execution of the legacy Shell script. - `shadow`: Runs both the legacy Shell script and the Rust binary, compares their stdout, warns on mismatches, and returns the legacy script's stdout to ensure safety. - `PRE_COMMIT_REVIEW_SHADOW_MODE`: If set to `1`, forces Shadow Mode comparison even when `PRE_COMMIT_REVIEW_HELPER_IMPL` is explicitly set to `legacy` or `shell`. - `PRE_COMMIT_REVIEW_SHADOW_DIFF_LOG`: Optional path for writing shadow mismatch diffs. By default, shadow mode does not write diff content to `/tmp`. - `PRE_COMMIT_REVIEW_DISABLE_FALLBACK`: If set to `1`, disables the legacy script fallback, strictly propagating Rust CLI process failures. +- `PRE_COMMIT_REVIEW_SECRET_SCAN`: Controls optional local redaction: `auto` (default) uses a verified scanner when available; `off` skips scanning and continues review unredacted. +- `PRE_COMMIT_REVIEW_GITLEAKS_BIN`: Explicit trusted absolute scanner path for development, tests, or controlled offline environments. It must match the pinned version and pass the stdin/JSON capability test. Setting it is an explicit trust decision; otherwise only the SHA256-verified bundled binary is accepted, and `PATH` is never searched. +- `PRE_COMMIT_REVIEW_GITLEAKS_CONFIG`: Explicit trusted scanner config path for development/tests. Do not point this at configuration from the repository being reviewed. +- `PRE_COMMIT_REVIEW_GITLEAKS_TIMEOUT_MS`: Per-process Gitleaks deadline in milliseconds. The default is `30000`; accepted overrides are `50` through `120000`. A timeout kills and reaps the scanner, reports `scanner-timeout`, and continues review without redaction. +- `PRE_COMMIT_REVIEW_FETCH_PROGRESS`: Controls Gitleaks download progress: `auto` (default), `always`, or `never`. + +Every implementation mode uses the same best-effort stream sanitizer. When scanning succeeds, shadow mismatch logs are based on sanitized stdout/stderr. `status: unavailable` means the scanner could not run or finish; `status: redaction-failed` means it returned a finding but the helper could not apply or verify that replacement. Both states continue review without withholding output and explicitly report that redaction was not applied. -Use `scripts/collect_diff_context.sh --plan-only` or `--include-diff never` to recover only the structured control plane when a host persisted the original helper output. Use `--include-diff always` only when you explicitly want the global raw diff, still bounded by `PRE_COMMIT_REVIEW_MAX_DIFF_BYTES`. +Use `scripts/collect_diff_context.sh --plan-only` or `--include-diff never` to recover only the structured control plane when a host persisted the original helper output. Use `--include-diff always` only when you explicitly want the global diff view, still bounded by `PRE_COMMIT_REVIEW_MAX_DIFF_BYTES`; verify `## Secret Scan` before assuming that view was redacted. Use `scripts/collect_diff_context.sh --source --group --expect-scope ` to retrieve one in-budget review group's diff after opening the control plane. Use `--path ` with the same fingerprint for file-level follow-up when a group needs narrower context or has been split. Rerun `--control-plane` before the verdict; snapshot drift invalidates the old ledger instead of being merged into a false complete review. `split-required` groups must be reviewed through bounded replacements instead of as one group. @@ -390,11 +405,14 @@ Execution entrypoints are layered too: - `output_eval_runner.sh` prepares real local fixtures for any one eval file, can optionally invoke an external model runner, and grades saved responses against expected verdicts and required phrases - `--eval-file` lets `output_eval_runner.sh` target one layered output eval JSON such as `evals/output/visual-output-eval.json`. +- `--skill-dir` selects the skill checkout linked into host fixtures, so baseline and current responses can be generated from the same eval cases without changing the harness checkout - `run_layered_output_evals.sh` runs the layered output eval matrix end-to-end across the routine, advanced, visual, and localization eval files - `run_marker_eval_checks.sh` validates marker-taxonomy coverage and summarizes blocking vs non-blocking case counts - `output_eval_codex_case.sh` and `output_eval_claude_case.sh` run a single eval case per host - `output_eval_codex_runner.sh` and `output_eval_claude_runner.sh` are host-specific thin wrappers that link this checkout into the fixture's project-local skill directory (`.agents/skills` for Codex, `.claude/skills` for Claude Code) and delegate to `output_eval_runner.sh` with host-appropriate non-interactive commands - `output_eval_runner_test.sh` is the deterministic self-test for fixture preparation and grading logic +- `compare_output_eval_quality.sh` grades saved baseline and current responses against the same layered eval cases, emits an `output-eval-quality-diff/v1` JSON report, and fails on regressions or incomplete response sets without invoking a model. Secret-attention cases additionally report non-secret finding recall and fail through `secret_attention_regressions` when a credential finding causes authorization, migration, or compatibility recall to fall. +- `compare_output_eval_quality_test.sh` deterministically covers regression, improvement, no-regression, and incomplete comparison outcomes - `output_eval_host_wrappers_test.sh` verifies the wrappers with mock Codex and Claude binaries so host command templates regress without spending model calls - `run_helper_gateway_probe.sh` runs a real-host stage that instruments the bundled helper and selected direct Git commands, then fails if a host inspects Git diff source before attempting `scripts/collect_diff_context.sh` - `check_persisted_output_contract.sh` scans host transcripts for persisted helper output and fails if the saved plan/manifest was never recovered before a full-review claim @@ -402,6 +420,19 @@ Execution entrypoints are layered too: - `readme_host_entrypoints_test.sh` pins the tiered `Host Entrypoints` section so the README keeps exposing the host-lane surface by `Primary`, `Analysis`, `Stage`, and `Internal / Repo-wide` - `eval_contract_test.sh` is the repo-wide gate for trigger evals, layered output evals, marker taxonomy assets, and host-lane contract surfaces +Generate the baseline and current response directories with the same eval files, host, model, and runner settings, then compare them without another model call: + +```bash +./evals/compare_output_eval_quality.sh \ + --baseline-responses /path/to/baseline-responses \ + --current-responses /path/to/current-responses \ + --report-json /path/to/output-quality-diff.json +``` + +The `advanced-independent-findings-enumeration-en` case uses a neutral review prompt and contains one credential plus three independent non-secret findings. For a meaningful stochastic A/B result, run that case 5–10 times per checkout with matched host/model settings and require full current non-secret recall with no per-run decline. + +The controlled scanner-off/scanner-on pilot and its limitations are recorded in [`docs/gitleaks-quality-evaluation.md`](./docs/gitleaks-quality-evaluation.md). + ### Host Entrypoints For the host-lane workflow, use these scripts by tier: @@ -412,8 +443,8 @@ For the host-lane workflow, use these scripts by tier: - Use these when you want one stable entrypoint for real authenticated host smoke runs and artifact collection - `Primary / Output Matrix`: `evals/run_layered_output_evals.sh`, `evals/run_marker_eval_checks.sh` - Use these to run the layered output-eval surface and marker-taxonomy checks without hand-selecting individual eval assets -- `Analysis`: `evals/analyze_host_readiness_diff.sh` -- Use this to compare cross-host readiness outputs without rerunning each stage +- `Analysis`: `evals/analyze_host_readiness_diff.sh`, `evals/compare_output_eval_quality.sh` +- Use these to compare cross-host readiness reports or saved before/after output-eval responses without rerunning stages or invoking a model during comparison - `Stage`: `evals/check_host_availability.sh`, `evals/run_helper_gateway_probe.sh`, `evals/check_persisted_output_contract.sh`, `evals/run_layered_host_evals.sh`, `evals/host_contract_subset.sh` - Use these when debugging or running one host-lane boundary directly - `Internal / Repo-wide`: `evals/eval_contract_test.sh`, host `*_test.sh`, `evals/host_failure_taxonomy.sh` diff --git a/README.zh-CN.md b/README.zh-CN.md index 6f221e0..5ca22cc 100644 --- a/README.zh-CN.md +++ b/README.zh-CN.md @@ -133,6 +133,7 @@ - 一个能加载 skill 的受支持 AI 编程 agent 运行时（Codex、Claude Code、Gemini CLI 或 Kiro）。skill 包本身不附带运行时。 - 本地 diff 收集需要 `PATH` 中存在 `git`。当你直接粘贴 diff 或代码时，无需 git 也能审查。 +- 网络访问是可选的。从源码 clone 安装时，`install.sh` 会尝试下载当前平台固定的 Gitleaks `8.30.1`，并同时校验 release archive 与解压后 executable 的 SHA256。自包含 release 包已经附带验证过的二进制。下载被关闭、不可用或失败时，skill 仍会完成安装并继续审查，只是不提供本地密钥打码；不会隐式搜索 `PATH`。 - 运行 `install.sh` 和辅助脚本需要 Unix 兼容 shell。Windows 上请使用 Git Bash、MSYS2 或 WSL。 ## 快速安装 @@ -146,6 +147,8 @@ ./install.sh --agent kiro-cli ``` +以上命令会在安装阶段尝试准备当前平台的固定版本 Gitleaks。这是用户显式触发的安装器行为；Agent 审查流程本身绝不会下载工具。准备失败会产生告警，但不会阻止安装或审查。 + 列出所有受支持的 agent id 及其 project/global 路径： ```bash @@ -169,6 +172,8 @@ - `--dir PATH` 手动指定目标 skills 目录 - `--force` 覆盖一个并非由当前安装器管理的同名目标 - `--dry-run` 只打印将执行的动作，不真正修改文件 +- `--no-download` 跳过可选的 Gitleaks 下载；没有密钥打码时审查仍可继续 +- `--doctor` 在不安装 skill 的情况下诊断 scanner 来源、版本、bundled SHA256、可信配置和 stdin/JSON 能力；打码不可用时会非零退出，但不代表审查被阻塞 示例： @@ -283,6 +288,8 @@ ├── output/ ├── taxonomy/ ├── eval_contract_test.sh + ├── compare_output_eval_quality.sh + ├── compare_output_eval_quality_test.sh ├── readme_surface_test.sh ├── readme_host_entrypoints_test.sh ├── output-eval.json @@ -321,18 +328,19 @@ ### `scripts/collect_diff_context.sh` -这是一个只读辅助脚本，用于为审查流程收集本地仓库上下文。它做三件事： +这是一个只读辅助脚本，用于为审查流程收集本地仓库上下文。它做四件事： 1. **diff 来源解析** —— 判断当前目录是否是 Git 仓库，存在 staged 时优先使用 staged，否则回退到 unstaged 或 branch-vs-base；输出 diff 统计、文件列表、状态、截断状态、基于路径/内容的高风险候选、疑似生成文件、lockfile 和高 churn 文件。rename、delete、binary、mode-only 和 submodule 指针更新都会记录为 manifest units。 2. **有界 control plane** —— 通过 `--control-plane` 输出紧凑 JSON gateway，包含完整 scope 内容指纹、逐单元指纹、有界 units/groups、work order 与可复用命令模板；后续补取支持 `--expect-scope `，快照过期时 fail closed；指纹和实际审查字节都会禁用外部 diff/textconv driver，确保快照身份与模型检查到的内容保持同一语义。 3. **coverage-led 与测试选择提示** —— 输出 Review Manifest/Groups 以及 reducer 友好的结构化段落（Review Plan JSON、split 建议、ledgers、work packets、finalization 模板）、有界只读 Semantic Context Queries，以及对变更中测试文件的 Test Selection Hints，用于识别常见 JVM/Spring/Quarkus/Micronaut、Maven/Gradle 集成测试命名、JUnit tags、Testcontainers、Docker Compose、WireMock/MockServer、pytest markers、Playwright/Cypress/Node e2e、Go build tags、Rust ignored/integration tests，以及数据库/缓存/消息/搜索服务配置等环境依赖测试。 +4. **可选的本地密钥打码** —— 可信 Gitleaks 可用时，先扫描和打码完整的所选 diff，再应用输出字节上限，将命中范围替换为 `[redacted:]` 后复扫，并对 wrapper 捕获的完整 stdout/stderr 做打码。这个顺序能防止已检测到的密钥跨越截断边界时以无法匹配的前缀泄露。scanner 被关闭、不可用、超时或没有返回命中时，审查继续使用原始输出。若 Gitleaks 已返回命中，但本地坐标映射或复核失败，helper 会明确报告 `status: redaction-failed`，而不是把它说成 scanner 不可用；此路径同样继续输出原始内容，不暂扣审查材料。 完整输出段落清单（Coverage Ledger Template、Group Review Work Packets、Reducer State Snapshot 等）见 [`docs/helper-capabilities.md`](./docs/helper-capabilities.md)，供构建 reducer/subagent 自动化的集成者参考。 -它不会执行 fetch、stage、reset、install，也不会修改任何文件。 +审查入口不会执行 fetch、stage、reset、install，也不会修改任何文件。用户显式执行安装时，如果当前平台二进制尚未 bundled，`install.sh` 会调用 `scripts/fetch_gitleaks.sh`；该脚本只下载仓库固定的上游 release asset，并同时校验 archive 与解压后 executable 的固定 SHA256。交互式终端默认显示下载进度；输出被宿主捕获时可设置 `PRE_COMMIT_REVIEW_FETCH_PROGRESS=always` 强制显示，或设为 `never` 关闭。`--dry-run` 不会下载，`--no-download` 会跳过这项可选安装行为，Agent 审查期间也绝不会联网安装 Gitleaks。可运行 `./install.sh --doctor` 诊断本地打码是否可用。 它不会运行、改写或跳过测试。Test Selection Hints 只是只读提示，用于选择更聚焦的验证命令，并区分沙箱环境失败和代码失败。`no-known-env-heavy-marker` 并不证明测试是隔离单测，只表示 helper 没匹配到已知的重环境标记。 -审查流程首先运行 `scripts/collect_diff_context.sh --control-plane`。这个有界 gateway 不输出 raw diff，且只有 collection-start 与 collection-end 指纹一致时才标记为 authoritative。兼容用的默认输出仍是 plan-first，并可能省略全局 raw diff。`PRE_COMMIT_REVIEW_INLINE_DIFF_BYTES`（默认 `60000`）控制该默认输出何时内联全局 diff。`PRE_COMMIT_REVIEW_MAX_DIFF_BYTES`（默认 `200000`）只控制已经被选择输出的 diff 如何截断；只有在确认完整 diff 输出安全时才设为 `0`。 +审查流程首先运行 `scripts/collect_diff_context.sh --control-plane`。这个有界 gateway 不输出 raw diff，且只有 collection-start 与 collection-end 指纹一致时才标记为 authoritative。兼容用的默认输出仍是 plan-first，并可能省略全局 raw diff。`PRE_COMMIT_REVIEW_INLINE_DIFF_BYTES`（默认 `60000`）控制该默认输出何时内联全局 diff。`PRE_COMMIT_REVIEW_MAX_DIFF_BYTES`（默认 `200000`）只控制已经被选择输出的 diff 如何截断；只有在确认完整的已打码 diff 输出安全时才设为 `0`。 即使所选模型标称支持 200K 以上上下文，默认预算仍然有意保持保守。CLI 宿主可能在内容进入模型之前就把大型工具 stdout 持久化或只返回 preview；大段 raw diff 还会增加延迟和多轮 token 成本，并削弱审查焦点。请把默认值视为跨宿主稳定基线，而不是模型上下文上限。 @@ -346,14 +354,21 @@ Review group 预算默认目标值为 120KB，硬上限为 160KB。可通过 `PR ### 灰度发布与多实现控制（Rollout & Multi-Implementation Controls） 入口包装脚本 `scripts/collect_diff_context.sh` 支持多种运行模式，以确保版本过渡期的安全： - `PRE_COMMIT_REVIEW_HELPER_IMPL`: 指定底层调用的辅助脚本实现模式。 - - `rust` (默认值): 优先执行编译后的 Rust CLI 二进制程序。如果执行失败，会在 `stderr` 打印警告，并**自动无缝降级执行**旧版 Shell 脚本 `collect_diff_context.legacy.sh`。 + - `rust` (默认值): 优先执行编译后的 Rust CLI 二进制程序。收集错误可以降级到旧版 Shell 脚本。密钥扫描错误不会触发特殊 fallback，也不会阻塞输出；所选实现会在报告降级状态后继续输出未打码内容。 - `legacy` 或 `shell`: 强制直接运行旧版 Shell 脚本。 - `shadow`: 双路执行模式。同时运行旧版 Shell 脚本和 Rust 二进制程序，比对它们的标准输出，并在不一致时告警。此模式下返回旧版 Shell 的结果以保障生产安全。 - `PRE_COMMIT_REVIEW_SHADOW_MODE`: 设为 `1` 时会强制开启上述 `shadow` 双路比对模式，即使 `PRE_COMMIT_REVIEW_HELPER_IMPL` 被显式设为 `legacy` 或 `shell` 也一样。 - `PRE_COMMIT_REVIEW_SHADOW_DIFF_LOG`: 可选的 shadow mismatch diff 日志路径。默认 shadow mode 不会把 diff 内容写入 `/tmp`。 - `PRE_COMMIT_REVIEW_DISABLE_FALLBACK`: 设为 `1` 时禁用 Rust 失败降级机制，直接透传 Rust 程序的异常和退出码（用于测试与 CI）。 +- `PRE_COMMIT_REVIEW_SECRET_SCAN`: 控制可选本地打码：`auto`（默认）在可信 scanner 可用时启用；`off` 跳过扫描并继续输出未打码审查内容。 +- `PRE_COMMIT_REVIEW_GITLEAKS_BIN`: 在开发、测试或受控离线环境中显式指定可信 scanner 绝对路径。它必须匹配固定版本并通过 stdin/JSON 能力测试。设置该变量代表用户主动信任此外部程序；否则只接受通过 SHA256 验证的 bundled binary，且绝不搜索 `PATH`。 +- `PRE_COMMIT_REVIEW_GITLEAKS_CONFIG`: 开发/测试时显式指定可信 scanner 配置。不要指向被审查仓库提供的配置。 +- `PRE_COMMIT_REVIEW_GITLEAKS_TIMEOUT_MS`: 单个 Gitleaks 进程的毫秒级超时。默认值为 `30000`，允许覆盖为 `50` 到 `120000`。超时后 helper 会终止并回收 scanner，报告 `scanner-timeout`，然后在未打码状态下继续审查。 +- `PRE_COMMIT_REVIEW_FETCH_PROGRESS`: 控制 Gitleaks 下载进度：`auto`（默认）、`always` 或 `never`。 + +所有实现模式都使用同一个尽力而为的流打码器。扫描成功时，shadow mismatch 日志基于已打码的 stdout/stderr。`status: unavailable` 表示 scanner 无法运行或未能完成；`status: redaction-failed` 表示 scanner 已返回命中，但 helper 未能应用或复核替换。两种状态都不会暂扣输出，并会明确说明没有完成打码。 -当宿主把 helper 输出持久化、只返回 preview 时，可用 `scripts/collect_diff_context.sh --plan-only` 或 `--include-diff never` 重新获取结构化控制面。只有明确需要全局 raw diff 时才使用 `--include-diff always`；它仍受 `PRE_COMMIT_REVIEW_MAX_DIFF_BYTES` 限制。 +当宿主把 helper 输出持久化、只返回 preview 时，可用 `scripts/collect_diff_context.sh --plan-only` 或 `--include-diff never` 重新获取结构化控制面。只有明确需要全局 diff 时才使用 `--include-diff always`；输出仍受 `PRE_COMMIT_REVIEW_MAX_DIFF_BYTES` 限制，在假定内容已打码前必须检查 `## Secret Scan` 状态。 打开控制面后，可用 `scripts/collect_diff_context.sh --source --group --expect-scope ` 只输出一个未超硬预算 review group 的 diff。需要更窄上下文或 group 已拆分时，用带同一 fingerprint 的 `--path ` 补取。最终 verdict 前必须重跑 `--control-plane`；快照漂移会使旧 ledger 失效，不能把两个版本拼成一次“完整审查”。`split-required` group 必须通过有界 replacement 审查，不能作为一个整体 group 审查。 @@ -390,11 +405,14 @@ Reducer 和 subagent 自动化应优先使用 authoritative `Review Control Plan - `output_eval_runner.sh` 针对任意单个 eval 文件准备真实本地 fixture，可选调用外部模型 runner，并按期望 verdict 与必含短语对保存响应评分 - `--eval-file` 可让 `output_eval_runner.sh` 指向任意单个分层 output eval JSON，例如 `evals/output/visual-output-eval.json`。 +- `--skill-dir` 用于选择链接到宿主 fixture 的 skill checkout，从而用同一套 eval case 分别生成引入前和引入后的响应，而无需切换 harness checkout - `run_layered_output_evals.sh` 端到端执行 layered output eval 矩阵，覆盖 routine、advanced、visual 和 localization 四套 eval 文件 - `run_marker_eval_checks.sh` 校验 marker taxonomy 覆盖，并汇总 blocking / non-blocking case 数量 - `output_eval_codex_case.sh` 和 `output_eval_claude_case.sh` 每个宿主执行单个 eval case - `output_eval_codex_runner.sh` 和 `output_eval_claude_runner.sh` 是宿主专用薄封装，会把当前仓库链接到 fixture 的 project-local skill 目录（Codex 用 `.agents/skills`，Claude Code 用 `.claude/skills`），再用适合各自宿主的非交互命令委托给 `output_eval_runner.sh` - `output_eval_runner_test.sh` 是 fixture 准备与评分逻辑的确定性自测 +- `compare_output_eval_quality.sh` 会用同一套分层 eval case 对已保存的引入前/引入后响应评分，输出 `output-eval-quality-diff/v1` JSON 报告；发现回归或响应集不完整时失败，比较过程不调用模型。密钥注意力 case 会额外统计非密钥 finding 召回；当凭据问题导致授权、迁移或兼容性问题召回下降时，通过 `secret_attention_regressions` 单独失败。 +- `compare_output_eval_quality_test.sh` 确定性覆盖回归、改进、无回归与响应不完整四类结果 - `output_eval_host_wrappers_test.sh` 用 mock Codex/Claude 二进制验证这些 wrapper，确保宿主命令模板回归时不消耗真实模型调用 - `run_helper_gateway_probe.sh` 是一个 real-host stage，会对 bundled helper 与选定的直接 Git 命令做日志探针；如果 host 在尝试 `scripts/collect_diff_context.sh` 之前就检查 Git diff 来源，则判定失败 - `check_persisted_output_contract.sh` 会扫描宿主 transcript；如果 helper 大输出被持久化后，模型在声称完整审查前没有恢复保存的 plan/manifest，则判定失败 @@ -402,6 +420,19 @@ Reducer 和 subagent 自动化应优先使用 authoritative `Review Control Plan - `readme_host_entrypoints_test.sh` 固化分层 `Host Entrypoints` 文档，确保 README 持续以 `Primary`、`Analysis`、`Stage` 和 `Internal / Repo-wide` 暴露 host lane surface - `eval_contract_test.sh` 是 repo 级门禁，统一守护 trigger eval、layered output eval、marker taxonomy 资产以及 host lane contract surface +先使用相同的 eval 文件、宿主、模型与 runner 设置分别生成引入前和引入后的响应目录，再执行不触发额外模型调用的比较： + +```bash +./evals/compare_output_eval_quality.sh \ + --baseline-responses /path/to/baseline-responses \ + --current-responses /path/to/current-responses \ + --report-json /path/to/output-quality-diff.json +``` + +`advanced-independent-findings-enumeration-en` 使用中性的审查请求，fixture 同时包含一个凭据问题和三个相互独立的非密钥问题。为了得到有意义的随机性 A/B 结果，应在相同宿主与模型配置下对每个 checkout 重复运行 5～10 次，并要求当前版本的非密钥问题全部召回且每次均不低于基线。 + +scanner 关闭/开启的受控 pilot、结果及其样本量限制记录在 [`docs/gitleaks-quality-evaluation.md`](./docs/gitleaks-quality-evaluation.md)。 + ### Host Entrypoints 对于 host lane 工作流，可按下面的层级使用这些脚本： @@ -412,8 +443,8 @@ Reducer 和 subagent 自动化应优先使用 authoritative `Review Control Plan - 当你需要一个稳定入口去跑真实、已认证 host 的 smoke 验证并收集产物时，使用这两个入口 - `Primary / Output Matrix`: `evals/run_layered_output_evals.sh`、`evals/run_marker_eval_checks.sh` - 用于执行分层 output eval surface 与 marker taxonomy 检查，无需手工逐个挑选 eval 资产 -- `Analysis`: `evals/analyze_host_readiness_diff.sh` -- 用于对比 cross-host readiness 输出，而不必重新跑每个阶段 +- `Analysis`: `evals/analyze_host_readiness_diff.sh`、`evals/compare_output_eval_quality.sh` +- 用于对比 cross-host readiness 报告，或比较已保存的引入前/引入后 output-eval 响应；比较阶段无需重跑各 stage，也不会调用模型 - `Stage`: `evals/check_host_availability.sh`、`evals/run_helper_gateway_probe.sh`、`evals/check_persisted_output_contract.sh`、`evals/run_layered_host_evals.sh`、`evals/host_contract_subset.sh` - 当你只想调试或单独运行某一层 host 边界时使用 - `Internal / Repo-wide`: `evals/eval_contract_test.sh`、host `*_test.sh`、`evals/host_failure_taxonomy.sh` diff --git a/SKILL.md b/SKILL.md index 2c45f56..3a1780b 100644 --- a/SKILL.md +++ b/SKILL.md @@ -64,14 +64,33 @@ This is a mandatory gateway. Attempt the helper before any direct `git status`, - coverage rules and snapshot identity - test selection hints for changed tests, when emitted -If the control plane reports `authoritative: false`, stop scope-dependent review work and rerun it. Do not combine units or findings from different fingerprints. Only fall back to direct Git inspection when the helper is unavailable at that resolved path, exits non-zero without structured output, cannot be executed in the current host, or the user already provided the review material explicitly. When falling back, keep the source selection order above and describe the missing snapshot guard as a review limitation. +If the control plane reports `authoritative: false`, stop scope-dependent review work and rerun it. Do not combine units or findings from different fingerprints. For repository-sourced review material, do not fall back to direct Git inspection when the helper is unavailable, exits non-zero without structured output, or cannot run in the current host: direct Git output bypasses the authoritative scope, fingerprint, and bounded-retrieval contract. Ask the user to restore the helper or provide the review material. User-provided review material remains governed by the input rules below. The helper is control-plane-first. The initial `--control-plane` output is bounded metadata and intentionally contains no raw diff. In that case: - use the emitted compact units, groups, work order, command templates, and coverage contract - expand the emitted command templates with the recorded fingerprint; every helper-mediated `--group` and `--path` load must include `--expect-scope ` - do not rebuild the review scope with direct `git status`, `git diff --name-only`, `git diff --stat`, or ad hoc path selection -- use helper-emitted `review_command` values only as a compatibility fallback when a helper-mediated `context_command` cannot be run +- never execute helper-emitted raw `review_command` values; if a helper-mediated `context_command` cannot run, stop that review path rather than bypassing the authoritative snapshot gateway + +### Optional Local Secret Redaction + +Gitleaks is an optional, best-effort local redaction layer. It applies to repository-sourced helper output and improves model-input safety when available, but its absence, disablement, or failure must not block or shorten the code review. The trusted scanner configuration lives in the skill package, not in the repository being reviewed. Repository `.gitleaks.toml`, `.gitleaksignore`, and `gitleaks:allow` directives must not weaken the scanner configuration. + +When present, the default scanner must be the platform-specific bundled executable whose version and SHA256 match the skill-owned manifests. Never discover Gitleaks implicitly through `PATH`. `PRE_COMMIT_REVIEW_GITLEAKS_BIN` is reserved for an absolute path explicitly trusted by the user; it still must match the pinned version and pass an empty-stdin JSON capability check before use. Version, capability, and content scans have a bounded deadline; `scanner-timeout` is an unavailable-redaction state and must never block the review. + +When helper output contains `## Secret Scan`: + +- `status: clean` means no Gitleaks finding remained in that emitted view; it is not proof that the repository contains no secrets +- `status: redacted` means one or more Gitleaks match ranges were replaced locally before model delivery; never attempt to reconstruct, reveal, or fetch the replaced value through another command +- `status: unavailable` with `redaction_applied: no` and `review_continued: yes` means scanning could not run or complete; continue reviewing the emitted content, state that local secret redaction was unavailable, and do not claim that the model input was scanned or safe from secret exposure +- `status: redaction-failed` with `findings_detected: yes`, `redaction_applied: no`, and `review_continued: yes` means Gitleaks returned at least one finding but the helper could not map or verify the replacement; continue reviewing the emitted content, report this as a redaction implementation failure rather than scanner unavailability, and do not claim that the detected value was protected from model exposure +- `status: disabled` with `redaction_applied: no` and `review_continued: yes` means the user disabled scanning; continue reviewing and state that local secret redaction was disabled +- use only the emitted rule id, scan-input location, diff prefix, and surrounding sanitized code as review evidence +- an added redaction is a blocking potential credential finding; state that the value is redacted and the owner should rotate it if it is real +- a removed-only redaction does not make the removal unsafe by itself, but still note that the exposed value is redacted and the owner should rotate it if it was ever real +- a secret finding is a security signal, not a review-completion condition: do not select or render the final verdict until the normal review scope is complete, and continue enumerating independent authorization, data, compatibility, reliability, and test risks after any credential blocker is found +- never cap, merge away, or omit an independently actionable finding merely because a secret already makes the verdict blocking; for coverage-accounted reviews, every manifest unit must still reach a terminal coverage state before finalization If the helper emits `Test Selection Hints`, use them only as read-only guidance for verification planning. They do not prove test safety, do not replace CI, and must not be described as skipped or stripped tests. Built-in hints cover common JVM/Spring/Quarkus/Micronaut, pytest, Node e2e, Go, Rust, container, HTTP-stub, and external-service markers; project-specific `.pre-commit-review/test-hints` rules still take precedence for local conventions. Treat env-dependent tests such as `@SpringBootTest`, Testcontainers, or DB slices as verification that may require CI/local profile support, not as sandbox-safe unit tests. Treat `no-known-env-heavy-marker` as "no known marker matched", not as proof that the test is a pure unit test. diff --git a/THIRD_PARTY_LICENSES/gitleaks-LICENSE b/THIRD_PARTY_LICENSES/gitleaks-LICENSE new file mode 100644 index 0000000..3c270b3 --- /dev/null +++ b/THIRD_PARTY_LICENSES/gitleaks-LICENSE @@ -0,0 +1,21 @@ +MIT License + +Copyright (c) 2019 Zachary Rice + +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the "Software"), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in all +copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +SOFTWARE. diff --git a/collect-diff-context-cli/Cargo.lock b/collect-diff-context-cli/Cargo.lock index 50cee52..8bd19cc 100644 --- a/collect-diff-context-cli/Cargo.lock +++ b/collect-diff-context-cli/Cargo.lock @@ -11,6 +11,21 @@ dependencies = [ "memchr", ] +[[package]] +name = "block-buffer" +version = "0.10.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3078c7629b62d3f0439517fa394996acacc5cbc91c5a20d8c658e77abd503a71" +dependencies = [ + "generic-array", +] + +[[package]] +name = "cfg-if" +version = "1.0.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9330f8b2ff13f34540b44e946ef35111825727b38d33286ef986142615121801" + [[package]] name = "collect-diff-context-cli" version = "0.1.0" @@ -18,6 +33,46 @@ dependencies = [ "regex", "serde", "serde_json", + "sha2", +] + +[[package]] +name = "cpufeatures" +version = "0.2.17" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "59ed5838eebb26a2bb2e58f6d5b5316989ae9d08bab10e0e6d103e656d1b0280" +dependencies = [ + "libc", +] + +[[package]] +name = "crypto-common" +version = "0.1.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "78c8292055d1c1df0cce5d180393dc8cce0abec0a7102adb6c7b1eef6016d60a" +dependencies = [ + "generic-array", + "typenum", +] + +[[package]] +name = "digest" +version = "0.10.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9ed9a281f7bc9b7576e61468ba615a66a5c8cfdff42420a70aa82701a3b1e292" +dependencies = [ + "block-buffer", + "crypto-common", +] + +[[package]] +name = "generic-array" +version = "0.14.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "85649ca51fd72272d7821adaf274ad91c288277713d9c18820d8499a7ff69e9a" +dependencies = [ + "typenum", + "version_check", ] [[package]] @@ -26,6 +81,12 @@ version = "1.0.18" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "8f42a60cbdf9a97f5d2305f08a87dc4e09308d1276d28c869c684d7777685682" +[[package]] +name = "libc" +version = "0.2.186" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "68ab91017fe16c622486840e4c83c9a37afeff978bd239b5293d61ece587de66" + [[package]] name = "memchr" version = "2.8.2" @@ -122,6 +183,17 @@ dependencies = [ "zmij", ] +[[package]] +name = "sha2" +version = "0.10.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a7507d819769d01a365ab707794a4084392c824f54a7a6a7862f8c3d0892b283" +dependencies = [ + "cfg-if", + "cpufeatures", + "digest", +] + [[package]] name = "syn" version = "2.0.118" @@ -133,12 +205,24 @@ dependencies = [ "unicode-ident", ] +[[package]] +name = "typenum" +version = "1.20.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b6f5e870be6c3b371b77fe0ee0bafb859fa4964b4404c27de1d380043c4dda20" + [[package]] name = "unicode-ident" version = "1.0.24" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e6e4313cd5fcd3dad5cafa179702e2b244f760991f45397d14d4ebf38247da75" +[[package]] +name = "version_check" +version = "0.9.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0b928f33d975fc6ad9f86c8f283853ad26bdd5b10b7f1542aa2fa15e2289105a" + [[package]] name = "zmij" version = "1.0.21" diff --git a/collect-diff-context-cli/Cargo.toml b/collect-diff-context-cli/Cargo.toml index 587ebea..5ce443c 100644 --- a/collect-diff-context-cli/Cargo.toml +++ b/collect-diff-context-cli/Cargo.toml @@ -7,6 +7,7 @@ edition = "2021" serde = { version = "1.0", features = ["derive"] } serde_json = "1.0" regex = "1.10" +sha2 = "0.10" [profile.release] opt-level = 3 diff --git a/collect-diff-context-cli/src/main.rs b/collect-diff-context-cli/src/main.rs index cdac524..64be93a 100644 --- a/collect-diff-context-cli/src/main.rs +++ b/collect-diff-context-cli/src/main.rs @@ -1,9 +1,11 @@ +mod secret_scan; + use regex::Regex; use serde::Serialize; use std::collections::{HashMap, HashSet}; use std::env; use std::fs::{self, File}; -use std::io::{BufRead, BufReader, Write}; +use std::io::{BufRead, BufReader, Read, Write}; use std::path::{Path, PathBuf}; use std::process::{Command, Stdio}; use std::sync::OnceLock; @@ -26,6 +28,7 @@ enum AppError { cmd: String, cwd: String, }, + SecretScan(secret_scan::SecretScanError), IoError(std::io::Error), InvalidArgument(String), } @@ -41,6 +44,7 @@ impl std::fmt::Display for AppError { "Git executable missing or invalid cwd: {}\nAttempted cmd: {}\nCwd: {}", details, cmd, cwd ), + AppError::SecretScan(error) => write!(f, "Secret scan error: {}", error), AppError::IoError(e) => write!(f, "I/O error: {}", e), AppError::InvalidArgument(s) => write!(f, "Invalid argument: {}", s), } @@ -1952,8 +1956,94 @@ fn fail_no_repo() { std::process::exit(0); } -fn emit_diff_limited(diff: &str, max_bytes: usize, inline_diff_bytes: usize) { +fn bounded_diff_view(diff: &str, max_bytes: usize) -> String { + if max_bytes == 0 || diff.len() <= max_bytes { + return diff.to_string(); + } + + let mut bounded = String::with_capacity(max_bytes + 160); + let mut byte_count = 0; + for character in diff.chars() { + let char_len = character.len_utf8(); + if byte_count + char_len > max_bytes { + break; + } + bounded.push(character); + byte_count += char_len; + } + bounded.push_str(&format!( + "\n[diff truncated after {} bytes; inspect high-risk files with helper-emitted context commands before making safety claims]", + max_bytes + )); + bounded +} + +fn emit_secret_scan_summary(output: &secret_scan::SanitizedOutput) { + println!(); + println!("## Secret Scan"); + println!("scanner: gitleaks"); + match output.status { + secret_scan::SecretScanStatus::Clean => println!("status: clean"), + secret_scan::SecretScanStatus::Redacted => println!("status: redacted"), + secret_scan::SecretScanStatus::Disabled => { + println!("status: disabled"); + println!("redaction_applied: no"); + println!("review_continued: yes"); + } + secret_scan::SecretScanStatus::Unavailable(reason) => { + println!("status: unavailable"); + println!("reason: {}", reason); + println!("redaction_applied: no"); + println!("review_continued: yes"); + } + secret_scan::SecretScanStatus::RedactionFailed(reason) => { + println!("status: redaction-failed"); + println!("reason: {}", reason); + println!("findings_detected: yes"); + println!("redaction_applied: no"); + println!("review_continued: yes"); + } + } + println!("redactions: {}", output.redactions.len()); + println!("redaction_mode: full-regex-match"); + if !output.redactions.is_empty() { + println!("rule_id\tscan_input_start_line\tscan_input_end_line"); + for redaction in &output.redactions { + println!( + "{}\t{}\t{}", + sanitize_tsv_field(&redaction.rule_id), + redaction.start_line, + redaction.end_line + ); + } + } +} + +fn sanitize_diff_for_output(diff: &str) -> secret_scan::SanitizedOutput { + secret_scan::sanitize_for_model_optional(diff) +} + +fn emit_sanitized_split_previews(parent_group: &str, path: &str, diff: &str) { + let sanitized = sanitize_diff_for_output(diff); + let hunks = split_diff_into_hunks(&sanitized.content); + for (h_idx, hunk) in hunks.iter().enumerate() { + println!("unit_id: hunk:{}:{}", path, h_idx + 1); + println!("parent_group_id: {}", parent_group); + println!("```diff"); + print!("{}", hunk.content); + println!("```"); + } + emit_secret_scan_summary(&sanitized); +} + +fn emit_diff_limited( + diff: &str, + max_bytes: usize, + inline_diff_bytes: usize, +) -> Result<(), AppError> { let size = diff.len(); + let sanitized = sanitize_diff_for_output(diff); + let bounded = bounded_diff_view(&sanitized.content, max_bytes); println!("diff_bytes: {}", size); println!("max_diff_bytes: {}", max_bytes); println!("inline_diff_bytes: {}", inline_diff_bytes); @@ -1961,21 +2051,10 @@ fn emit_diff_limited(diff: &str, max_bytes: usize, inline_diff_bytes: usize) { println!(); println!("## Diff"); println!("```diff"); - if max_bytes == 0 || size <= max_bytes { - print!("{}", diff); - } else { - let mut byte_count = 0; - for c in diff.chars() { - let char_len = c.len_utf8(); - if byte_count + char_len > max_bytes { - break; - } - print!("{}", c); - byte_count += char_len; - } - println!("\n[diff truncated after {} bytes; inspect high-risk files with helper-emitted context commands before making safety claims]", max_bytes); - } + print!("{}", bounded); println!("```"); + emit_secret_scan_summary(&sanitized); + Ok(()) } fn emit_diff_omitted(diff_size: usize, max_bytes: usize, inline_diff_bytes: usize, reason: &str) { @@ -3161,7 +3240,7 @@ fn run_app() -> Result<(), AppError> { &String::from_utf8_lossy(&file_diff_bytes), max_diff_bytes, inline_diff_bytes, - ); + )?; return Ok(()); } @@ -3563,14 +3642,7 @@ fn run_app() -> Result<(), AppError> { let f_diff_bytes = git_run_diff_bytes(mode, &selected_ref, &[], Some(&raw_path), &repo_root)?; let f_diff = String::from_utf8_lossy(&f_diff_bytes); - let hunks = split_diff_into_hunks(&f_diff); - for (h_idx, hunk) in hunks.iter().enumerate() { - println!("unit_id: hunk:{}:{}", path, h_idx + 1); - println!("parent_group_id: {}", parent_group); - println!("```diff"); - print!("{}", hunk.content); - println!("```"); - } + emit_sanitized_split_previews(parent_group, path, &f_diff); } } else { println!("none"); @@ -3997,7 +4069,7 @@ fn run_app() -> Result<(), AppError> { // Limit/emit the actual global diff only when the gateway budget allows it. if diff_output_decision == "inline" { - emit_diff_limited(&global_diff, max_diff_bytes, inline_diff_bytes); + emit_diff_limited(&global_diff, max_diff_bytes, inline_diff_bytes)?; } else { emit_diff_omitted( diff_size, @@ -4113,14 +4185,7 @@ fn emit_requested_group( for f in &group.files { let f_diff_bytes = unit_diff_cache.get(f).cloned().unwrap_or_default(); let f_diff = String::from_utf8_lossy(&f_diff_bytes); - let hunks = split_diff_into_hunks(&f_diff); - for (h_idx, hunk) in hunks.iter().enumerate() { - println!("unit_id: hunk:{}:{}", f, h_idx + 1); - println!("parent_group_id: {}", group.group_id); - println!("```diff"); - print!("{}", hunk.content); - println!("```"); - } + emit_sanitized_split_previews(&group.group_id, f, &f_diff); } return Ok(()); } @@ -4138,12 +4203,89 @@ fn emit_requested_group( return Ok(()); } - emit_diff_limited(&group_diff, max_diff_bytes, inline_diff_bytes); + emit_diff_limited(&group_diff, max_diff_bytes, inline_diff_bytes)?; + Ok(()) +} + +fn run_sanitize_stdin() -> Result<(), AppError> { + let mut input = String::new(); + std::io::stdin() + .read_to_string(&mut input) + .map_err(AppError::IoError)?; + let sanitized = match secret_scan::sanitize_for_model(&input) { + Ok(sanitized) => sanitized, + Err(error) => { + if let Some(report_path) = env::var_os("PRE_COMMIT_REVIEW_SANITIZE_REPORT") { + let stream = env::var("PRE_COMMIT_REVIEW_SANITIZE_STREAM") + .unwrap_or_else(|_| "output".to_string()); + let redaction_failed = error.is_redaction_failure(); + let mut report = String::from("# Pre-Commit Review Secret Scan\n"); + report.push_str("protocol: pcr-sanitizer-v1\n"); + report.push_str(&format!("stream: {}\n", sanitize_tsv_field(&stream))); + report.push_str(&format!( + "status: {}\n", + if redaction_failed { + "redaction-failed" + } else { + "unavailable" + } + )); + report.push_str(&format!("reason: {}\n", error.reason_code())); + report.push_str(&format!( + "findings_detected: {}\n", + if redaction_failed { "yes" } else { "unknown" } + )); + report.push_str("redaction_applied: no\n"); + report.push_str("review_continued: yes\n"); + report.push_str("redactions: 0\n"); + fs::write(report_path, report).map_err(AppError::IoError)?; + } + return Err(AppError::SecretScan(error)); + } + }; + + if let Some(report_path) = env::var_os("PRE_COMMIT_REVIEW_SANITIZE_REPORT") { + let stream = + env::var("PRE_COMMIT_REVIEW_SANITIZE_STREAM").unwrap_or_else(|_| "output".to_string()); + let mut report = String::from("# Pre-Commit Review Secret Scan\n"); + report.push_str("protocol: pcr-sanitizer-v1\n"); + report.push_str(&format!("stream: {}\n", sanitize_tsv_field(&stream))); + report.push_str(&format!( + "status: {}\n", + if sanitized.redactions.is_empty() { + "clean" + } else { + "redacted" + } + )); + report.push_str(&format!("redactions: {}\n", sanitized.redactions.len())); + if !sanitized.redactions.is_empty() { + report.push_str("rule_id\tscan_input_start_line\tscan_input_end_line\n"); + for redaction in &sanitized.redactions { + report.push_str(&format!( + "{}\t{}\t{}\n", + sanitize_tsv_field(&redaction.rule_id), + redaction.start_line, + redaction.end_line + )); + } + } + fs::write(report_path, report).map_err(AppError::IoError)?; + } + + print!("{}", sanitized.content); Ok(()) } fn main() { - match run_app() { + let args = env::args().collect::>(); + let result = if args.len() == 2 && args[1] == "--sanitize-stdin" { + run_sanitize_stdin() + } else { + run_app() + }; + + match result { Ok(_) => {} Err(e) => match e { AppError::InvalidArgument(msg) => { @@ -4168,6 +4310,10 @@ fn main() { ); std::process::exit(127); } + AppError::SecretScan(error) => { + eprintln!("collect_diff_context: secret scan failed: {}", error); + std::process::exit(3); + } }, } } diff --git a/collect-diff-context-cli/src/secret_scan.rs b/collect-diff-context-cli/src/secret_scan.rs new file mode 100644 index 0000000..2710f7e --- /dev/null +++ b/collect-diff-context-cli/src/secret_scan.rs @@ -0,0 +1,805 @@ +use serde::Deserialize; +use sha2::{Digest, Sha256}; +use std::collections::BTreeSet; +use std::env; +use std::fmt; +use std::fs::{self, File}; +use std::io::{Read, Write}; +use std::path::{Path, PathBuf}; +use std::process::{Command, ExitStatus, Stdio}; +use std::sync::OnceLock; +use std::thread; +use std::time::{Duration, Instant}; + +const FINDING_EXIT_CODE: i32 = 42; +const DEFAULT_SCANNER_TIMEOUT_MS: u64 = 30_000; +const MIN_SCANNER_TIMEOUT_MS: u64 = 50; +const MAX_SCANNER_TIMEOUT_MS: u64 = 120_000; + +#[derive(Debug, Clone)] +pub enum SecretScanError { + ScannerUnavailable, + ConfigUnavailable, + TrustMetadataUnavailable, + ScannerIntegrity, + ScannerVersion, + ScannerCapability, + ScannerTimeout, + ProcessIo, + ScannerFailed(i32), + InvalidReport, + InvalidLocation, + ResidualFinding, + RedactionVerification, +} + +impl SecretScanError { + pub fn reason_code(&self) -> &'static str { + match self { + Self::ScannerUnavailable => "scanner-unavailable", + Self::ConfigUnavailable => "config-unavailable", + Self::TrustMetadataUnavailable => "trust-metadata-unavailable", + Self::ScannerIntegrity => "scanner-integrity-failed", + Self::ScannerVersion => "scanner-version-mismatch", + Self::ScannerCapability => "scanner-capability-failed", + Self::ScannerTimeout => "scanner-timeout", + Self::ProcessIo => "scanner-process-io-failed", + Self::ScannerFailed(_) => "scanner-execution-failed", + Self::InvalidReport => "scanner-report-invalid", + Self::InvalidLocation => "redaction-location-invalid", + Self::ResidualFinding => "redaction-residual-finding", + Self::RedactionVerification => "redaction-verification-failed", + } + } + + pub fn is_redaction_failure(&self) -> bool { + matches!( + self, + Self::InvalidLocation | Self::ResidualFinding | Self::RedactionVerification + ) + } +} + +impl fmt::Display for SecretScanError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::ScannerUnavailable => write!(f, "gitleaks executable is unavailable"), + Self::ConfigUnavailable => write!(f, "trusted gitleaks config is unavailable"), + Self::TrustMetadataUnavailable => { + write!(f, "trusted gitleaks verification metadata is unavailable") + } + Self::ScannerIntegrity => write!(f, "bundled gitleaks integrity check failed"), + Self::ScannerVersion => write!(f, "gitleaks version check failed"), + Self::ScannerCapability => write!(f, "gitleaks capability check failed"), + Self::ScannerTimeout => write!(f, "gitleaks execution timed out"), + Self::ProcessIo => write!(f, "gitleaks process I/O failed"), + Self::ScannerFailed(code) => write!(f, "gitleaks failed with exit code {code}"), + Self::InvalidReport => write!(f, "gitleaks returned an invalid JSON report"), + Self::InvalidLocation => write!(f, "gitleaks returned an invalid finding location"), + Self::ResidualFinding => write!(f, "redacted output still contains a finding"), + Self::RedactionVerification => { + write!(f, "redacted output could not be verified by gitleaks") + } + } + } +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct RedactionSummary { + pub rule_id: String, + pub start_line: usize, + pub end_line: usize, +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct SanitizedOutput { + pub content: String, + pub redactions: Vec, + pub status: SecretScanStatus, +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum SecretScanStatus { + Clean, + Redacted, + Disabled, + Unavailable(&'static str), + RedactionFailed(&'static str), +} + +#[derive(Deserialize)] +#[serde(rename_all = "PascalCase")] +struct GitleaksFinding { + #[serde(rename = "RuleID")] + rule_id: String, + start_line: usize, + end_line: usize, + start_column: usize, + end_column: usize, + #[serde(rename = "Match")] + matched: String, +} + +#[derive(Debug)] +struct Scanner { + executable: PathBuf, + config: PathBuf, + working_dir: PathBuf, + timeout: Duration, +} + +#[derive(Debug)] +struct ScannerCandidate { + executable: PathBuf, + bundled: bool, +} + +static SCANNER: OnceLock> = OnceLock::new(); + +impl Scanner { + fn discover() -> Result { + let timeout = scanner_timeout(); + let config = trusted_config_path().ok_or(SecretScanError::ConfigUnavailable)?; + let working_dir = config + .parent() + .map(Path::to_path_buf) + .ok_or(SecretScanError::ConfigUnavailable)?; + + let candidate = scanner_candidate()?; + let version_file = trusted_script_file("gitleaks.version") + .ok_or(SecretScanError::TrustMetadataUnavailable)?; + if candidate.bundled { + let manifest = trusted_script_file("gitleaks-binaries.sha256") + .ok_or(SecretScanError::TrustMetadataUnavailable)?; + verify_bundled_hash(&candidate.executable, &manifest)?; + } + verify_version(&candidate.executable, &version_file, timeout)?; + + let scanner = Self { + executable: candidate.executable, + config, + working_dir, + timeout, + }; + scanner.verify_capability()?; + Ok(scanner) + } + + fn verify_capability(&self) -> Result<(), SecretScanError> { + match self.scan("") { + Ok(findings) if findings.is_empty() => Ok(()), + Err(SecretScanError::ScannerTimeout) => Err(SecretScanError::ScannerTimeout), + _ => Err(SecretScanError::ScannerCapability), + } + } + + fn scan(&self, input: &str) -> Result, SecretScanError> { + let config = self.config.to_string_lossy(); + let args = [ + "--config", + config.as_ref(), + "--ignore-gitleaks-allow", + "--exit-code=42", + "--no-banner", + "--no-color", + "--log-level=error", + "--max-decode-depth=5", + "--report-format=json", + "--report-path=-", + "stdin", + ]; + let (status, stdout) = run_scanner_process( + &self.executable, + &args, + Some(&self.working_dir), + input.as_bytes(), + self.timeout, + )?; + let code = status.code().unwrap_or(-1); + if code != 0 && code != FINDING_EXIT_CODE { + return Err(SecretScanError::ScannerFailed(code)); + } + + serde_json::from_slice(&stdout).map_err(|_| SecretScanError::InvalidReport) + } +} + +fn scanner_timeout() -> Duration { + let timeout_ms = env::var("PRE_COMMIT_REVIEW_GITLEAKS_TIMEOUT_MS") + .ok() + .and_then(|value| value.parse::().ok()) + .filter(|value| (MIN_SCANNER_TIMEOUT_MS..=MAX_SCANNER_TIMEOUT_MS).contains(value)) + .unwrap_or(DEFAULT_SCANNER_TIMEOUT_MS); + Duration::from_millis(timeout_ms) +} + +fn run_scanner_process( + executable: &Path, + args: &[&str], + working_dir: Option<&Path>, + input: &[u8], + timeout: Duration, +) -> Result<(ExitStatus, Vec), SecretScanError> { + let mut command = Command::new(executable); + command + .args(args) + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()); + if let Some(working_dir) = working_dir { + command.current_dir(working_dir); + } + + let mut child = command.spawn().map_err(|error| { + if error.kind() == std::io::ErrorKind::NotFound { + SecretScanError::ScannerUnavailable + } else { + SecretScanError::ProcessIo + } + })?; + let mut child_stdin = child.stdin.take().ok_or(SecretScanError::ProcessIo)?; + let mut child_stdout = child.stdout.take().ok_or(SecretScanError::ProcessIo)?; + let mut child_stderr = child.stderr.take().ok_or(SecretScanError::ProcessIo)?; + let input = input.to_vec(); + + let writer = thread::spawn(move || child_stdin.write_all(&input)); + let stdout_reader = thread::spawn(move || { + let mut output = Vec::new(); + child_stdout.read_to_end(&mut output).map(|_| output) + }); + let stderr_reader = thread::spawn(move || { + let mut output = Vec::new(); + child_stderr.read_to_end(&mut output).map(|_| output) + }); + + let deadline = Instant::now() + timeout; + let status = loop { + match child.try_wait() { + Ok(Some(status)) => break status, + Ok(None) if Instant::now() >= deadline => { + let _ = child.kill(); + let _ = child.wait(); + let _ = writer.join(); + let _ = stdout_reader.join(); + let _ = stderr_reader.join(); + return Err(SecretScanError::ScannerTimeout); + } + Ok(None) => thread::sleep(Duration::from_millis(10)), + Err(_) => { + let _ = child.kill(); + let _ = child.wait(); + let _ = writer.join(); + let _ = stdout_reader.join(); + let _ = stderr_reader.join(); + return Err(SecretScanError::ProcessIo); + } + } + }; + + writer + .join() + .map_err(|_| SecretScanError::ProcessIo)? + .map_err(|_| SecretScanError::ProcessIo)?; + let stdout = stdout_reader + .join() + .map_err(|_| SecretScanError::ProcessIo)? + .map_err(|_| SecretScanError::ProcessIo)?; + stderr_reader + .join() + .map_err(|_| SecretScanError::ProcessIo)? + .map_err(|_| SecretScanError::ProcessIo)?; + Ok((status, stdout)) +} + +#[derive(Debug)] +struct RedactionSpan { + start: usize, + end: usize, + rules: BTreeSet, +} + +pub fn sanitize_for_model(input: &str) -> Result { + let scanner = SCANNER + .get_or_init(Scanner::discover) + .as_ref() + .map_err(Clone::clone)?; + sanitize_with(input, |value| scanner.scan(value)) +} + +pub fn sanitize_for_model_optional(input: &str) -> SanitizedOutput { + if env::var("PRE_COMMIT_REVIEW_SECRET_SCAN").as_deref() == Ok("off") { + return SanitizedOutput { + content: input.to_string(), + redactions: Vec::new(), + status: SecretScanStatus::Disabled, + }; + } + match sanitize_for_model(input) { + Ok(output) => output, + Err(error) => { + let status = if error.is_redaction_failure() { + SecretScanStatus::RedactionFailed(error.reason_code()) + } else { + SecretScanStatus::Unavailable(error.reason_code()) + }; + SanitizedOutput { + content: input.to_string(), + redactions: Vec::new(), + status, + } + } + } +} + +fn sanitize_with(input: &str, mut scan: F) -> Result +where + F: FnMut(&str) -> Result, SecretScanError>, +{ + let findings = scan(input)?; + if findings.is_empty() { + return Ok(SanitizedOutput { + content: input.to_string(), + redactions: Vec::new(), + status: SecretScanStatus::Clean, + }); + } + + let mut spans = Vec::with_capacity(findings.len()); + let mut summaries = Vec::with_capacity(findings.len()); + for finding in findings { + let rule_id = sanitize_rule_id(&finding.rule_id); + let (start, end) = finding_span(input, &finding)?; + let mut rules = BTreeSet::new(); + rules.insert(rule_id.clone()); + spans.push(RedactionSpan { start, end, rules }); + summaries.push(RedactionSummary { + rule_id, + start_line: finding.start_line, + end_line: finding.end_line, + }); + } + + spans.sort_by_key(|span| (span.start, span.end)); + let mut merged: Vec = Vec::with_capacity(spans.len()); + for span in spans { + if let Some(previous) = merged.last_mut() { + if span.start <= previous.end { + previous.end = previous.end.max(span.end); + previous.rules.extend(span.rules); + continue; + } + } + merged.push(span); + } + + let mut content = input.to_string(); + for span in merged.into_iter().rev() { + if !content.is_char_boundary(span.start) || !content.is_char_boundary(span.end) { + return Err(SecretScanError::InvalidLocation); + } + let rule_list = span.rules.into_iter().collect::>().join("+"); + let newline_count = input.as_bytes()[span.start..span.end] + .iter() + .filter(|byte| **byte == b'\n') + .count(); + let replacement = format!("[redacted:{rule_list}]{}", "\n".repeat(newline_count)); + content.replace_range(span.start..span.end, &replacement); + } + + let residual = scan(&content).map_err(|_| SecretScanError::RedactionVerification)?; + if !residual.is_empty() { + return Err(SecretScanError::ResidualFinding); + } + + summaries.sort_by(|left, right| { + left.start_line + .cmp(&right.start_line) + .then_with(|| left.rule_id.cmp(&right.rule_id)) + }); + summaries.dedup(); + + Ok(SanitizedOutput { + content, + redactions: summaries, + status: SecretScanStatus::Redacted, + }) +} + +fn finding_span(input: &str, finding: &GitleaksFinding) -> Result<(usize, usize), SecretScanError> { + if finding.start_line == 0 + || finding.end_line < finding.start_line + || finding.start_column == 0 + || finding.end_column == 0 + { + return Err(SecretScanError::InvalidLocation); + } + + let starts = line_starts(input.as_bytes()); + let start_line_index = finding.start_line - 1; + let end_line_index = finding.end_line - 1; + let start_line = *starts + .get(start_line_index) + .ok_or(SecretScanError::InvalidLocation)?; + let end_line = *starts + .get(end_line_index) + .ok_or(SecretScanError::InvalidLocation)?; + let start_line_end = line_end(input.as_bytes(), &starts, start_line_index); + let end_line_end = line_end(input.as_bytes(), &starts, end_line_index); + + let reported_start = start_line.checked_add(finding.start_column - 1); + let reported_end = end_line.checked_add(finding.end_column); + if let (Some(start), Some(end)) = (reported_start, reported_end) { + if start < end + && start <= start_line_end + && end <= end_line_end + && input.get(start..end) == Some(finding.matched.as_str()) + { + return Ok((start, end)); + } + } + + // Gitleaks coordinates have changed under some output-redaction modes. Use + // the locally captured Match only to validate/recover the span; this value + // is never included in helper output or error messages. + if finding.matched.is_empty() || start_line > end_line_end { + return Err(SecretScanError::InvalidLocation); + } + let search_region = input + .get(start_line..end_line_end) + .ok_or(SecretScanError::InvalidLocation)?; + let expected_start = reported_start.unwrap_or(start_line); + search_region + .match_indices(&finding.matched) + .map(|(relative_start, matched)| { + let start = start_line + relative_start; + (start, start + matched.len()) + }) + .min_by_key(|(start, _)| start.abs_diff(expected_start)) + .ok_or(SecretScanError::InvalidLocation) +} + +fn line_starts(input: &[u8]) -> Vec { + let mut starts = vec![0]; + for (index, byte) in input.iter().enumerate() { + if *byte == b'\n' && index + 1 < input.len() { + starts.push(index + 1); + } + } + starts +} + +fn line_end(input: &[u8], starts: &[usize], line_index: usize) -> usize { + starts + .get(line_index + 1) + .map(|next| next.saturating_sub(1)) + .unwrap_or(input.len()) +} + +fn sanitize_rule_id(rule_id: &str) -> String { + let sanitized: String = rule_id + .chars() + .take(80) + .map(|character| { + if character.is_ascii_alphanumeric() || matches!(character, '.' | '_' | '-') { + character + } else { + '_' + } + }) + .collect(); + if sanitized.is_empty() { + "unknown-rule".to_string() + } else { + sanitized + } +} + +fn scanner_candidate() -> Result { + if let Some(path) = env::var_os("PRE_COMMIT_REVIEW_GITLEAKS_BIN") { + let executable = PathBuf::from(path); + return (executable.is_absolute() && executable.is_file()) + .then_some(ScannerCandidate { + executable, + bundled: false, + }) + .ok_or(SecretScanError::ScannerUnavailable); + } + + let binary_name = bundled_binary_name(); + for script_dir in script_dir_candidates() { + let executable = script_dir.join("bin").join(&binary_name); + if executable.is_file() { + return Ok(ScannerCandidate { + executable, + bundled: true, + }); + } + } + + Err(SecretScanError::ScannerUnavailable) +} + +fn trusted_config_path() -> Option { + if let Some(path) = env::var_os("PRE_COMMIT_REVIEW_GITLEAKS_CONFIG") { + let path = PathBuf::from(path); + return path.is_file().then_some(path); + } + + script_dir_candidates() + .into_iter() + .map(|script_dir| { + script_dir + .join("..") + .join("references") + .join("security") + .join("gitleaks.toml") + }) + .find(|candidate| candidate.is_file()) +} + +fn script_dir_candidates() -> Vec { + let mut candidates = Vec::new(); + if let Some(helper) = env::var_os("PRE_COMMIT_REVIEW_HELPER_PATH") { + if let Some(script_dir) = Path::new(&helper).parent() { + candidates.push(script_dir.to_path_buf()); + return candidates; + } + } + if let Ok(current_exe) = env::current_exe() { + if let Some(exe_dir) = current_exe.parent() { + if exe_dir.file_name().and_then(|name| name.to_str()) == Some("bin") { + if let Some(script_dir) = exe_dir.parent() { + candidates.push(script_dir.to_path_buf()); + } + } + } + } + if candidates.is_empty() { + candidates.push( + Path::new(env!("CARGO_MANIFEST_DIR")) + .join("..") + .join("scripts"), + ); + } + candidates.dedup(); + candidates +} + +fn trusted_script_file(name: &str) -> Option { + script_dir_candidates() + .into_iter() + .map(|script_dir| script_dir.join(name)) + .find(|candidate| candidate.is_file()) +} + +fn verify_version( + executable: &Path, + version_file: &Path, + timeout: Duration, +) -> Result<(), SecretScanError> { + let expected = + fs::read_to_string(version_file).map_err(|_| SecretScanError::TrustMetadataUnavailable)?; + let (status, stdout) = run_scanner_process(executable, &["version"], None, &[], timeout) + .map_err(|error| match error { + SecretScanError::ScannerTimeout => SecretScanError::ScannerTimeout, + _ => SecretScanError::ScannerVersion, + })?; + let actual = String::from_utf8_lossy(&stdout); + if status.success() && actual.trim() == expected.trim() { + Ok(()) + } else { + Err(SecretScanError::ScannerVersion) + } +} + +fn verify_bundled_hash(executable: &Path, manifest: &Path) -> Result<(), SecretScanError> { + let name = executable + .file_name() + .and_then(|value| value.to_str()) + .ok_or(SecretScanError::ScannerIntegrity)?; + let contents = + fs::read_to_string(manifest).map_err(|_| SecretScanError::TrustMetadataUnavailable)?; + let expected = contents + .lines() + .filter_map(|line| { + let mut fields = line.split_whitespace(); + Some((fields.next()?, fields.next()?)) + }) + .find_map(|(hash, entry)| (entry == name).then_some(hash)) + .ok_or(SecretScanError::ScannerIntegrity)?; + + let mut file = File::open(executable).map_err(|_| SecretScanError::ScannerIntegrity)?; + let mut hasher = Sha256::new(); + let mut buffer = [0_u8; 64 * 1024]; + loop { + let read = file + .read(&mut buffer) + .map_err(|_| SecretScanError::ScannerIntegrity)?; + if read == 0 { + break; + } + hasher.update(&buffer[..read]); + } + let actual = format!("{:x}", hasher.finalize()); + if actual == expected { + Ok(()) + } else { + Err(SecretScanError::ScannerIntegrity) + } +} + +fn bundled_binary_name() -> String { + let os = match env::consts::OS { + "macos" => "darwin", + "windows" => "windows", + other => other, + }; + let arch = match env::consts::ARCH { + "x86_64" => "amd64", + "aarch64" => "arm64", + other => other, + }; + let suffix = if env::consts::OS == "windows" { + ".exe" + } else { + "" + }; + format!("gitleaks-{os}-{arch}{suffix}") +} + +#[cfg(test)] +mod tests { + use super::*; + use std::time::{SystemTime, UNIX_EPOCH}; + + fn finding( + rule_id: &str, + start_line: usize, + end_line: usize, + start_column: usize, + end_column: usize, + matched: &str, + ) -> GitleaksFinding { + GitleaksFinding { + rule_id: rule_id.to_string(), + start_line, + end_line, + start_column, + end_column, + matched: matched.to_string(), + } + } + + #[test] + fn redacts_single_line_match_using_inclusive_byte_columns() { + let input = "header\n+token = glpat-example-value\nfooter\n"; + let mut calls = 0; + let output = sanitize_with(input, |_| { + calls += 1; + if calls == 1 { + Ok(vec![finding( + "gitlab-pat", + 2, + 2, + 10, + 28, + "glpat-example-value", + )]) + } else { + Ok(Vec::new()) + } + }) + .expect("sanitization should succeed"); + + assert_eq!( + output.content, + "header\n+token = [redacted:gitlab-pat]\nfooter\n" + ); + assert_eq!(calls, 2); + } + + #[test] + fn redacts_multiline_and_merges_overlapping_findings() { + let input = "before\n+secret=(first\n+second)\nafter\n"; + let mut calls = 0; + let output = sanitize_with(input, |_| { + calls += 1; + if calls == 1 { + Ok(vec![ + finding("rule-a", 2, 3, 10, 7, "first\n+second"), + finding("rule-b", 2, 2, 10, 14, "first"), + ]) + } else { + Ok(Vec::new()) + } + }) + .expect("sanitization should succeed"); + + assert_eq!( + output.content, + "before\n+secret=([redacted:rule-a+rule-b]\n)\nafter\n" + ); + assert_eq!(output.content.lines().count(), input.lines().count()); + } + + #[test] + fn recovers_a_shifted_location_from_the_local_match() { + let input = "+token = glpat-example-value, keep punctuation\n"; + let mut calls = 0; + let output = sanitize_with(input, |_| { + calls += 1; + if calls == 1 { + Ok(vec![finding( + "gitlab-pat", + 1, + 1, + 11, + 29, + "glpat-example-value", + )]) + } else { + Ok(Vec::new()) + } + }) + .expect("a shifted scanner location should be recovered"); + + assert_eq!( + output.content, + "+token = [redacted:gitlab-pat], keep punctuation\n" + ); + } + + #[test] + fn rejects_invalid_locations() { + let input = "only one line\n"; + let error = sanitize_with(input, |_| Ok(vec![finding("rule", 2, 2, 1, 2, "missing")])) + .expect_err("invalid locations must be reported"); + assert!(matches!(error, SecretScanError::InvalidLocation)); + } + + #[test] + fn rejects_residual_findings_after_redaction() { + let input = "token-value\n"; + let error = sanitize_with(input, |_| { + Ok(vec![finding("rule", 1, 1, 1, 11, "token-value")]) + }) + .expect_err("residual findings must be reported"); + assert!(matches!(error, SecretScanError::ResidualFinding)); + } + + #[test] + fn verifies_and_rejects_bundled_binary_hashes() { + let nonce = SystemTime::now() + .duration_since(UNIX_EPOCH) + .expect("clock should be available") + .as_nanos(); + let root = + env::temp_dir().join(format!("gitleaks-integrity-{}-{nonce}", std::process::id())); + fs::create_dir_all(&root).expect("fixture directory should be created"); + let executable = root.join("gitleaks-test"); + let manifest = root.join("manifest.sha256"); + fs::write(&executable, b"trusted scanner").expect("fixture binary should be written"); + + let mut hasher = Sha256::new(); + hasher.update(b"trusted scanner"); + fs::write( + &manifest, + format!("{:x} gitleaks-test\n", hasher.finalize()), + ) + .expect("fixture manifest should be written"); + verify_bundled_hash(&executable, &manifest).expect("matching bundled binary should pass"); + + fs::write(&executable, b"tampered scanner").expect("fixture binary should be replaced"); + assert!(matches!( + verify_bundled_hash(&executable, &manifest), + Err(SecretScanError::ScannerIntegrity) + )); + fs::remove_dir_all(root).expect("fixture directory should be removed"); + } + + #[test] + fn sanitizes_rule_ids_before_rendering() { + assert_eq!(sanitize_rule_id("rule with/slashes"), "rule_with_slashes"); + assert_eq!(sanitize_rule_id(""), "unknown-rule"); + } +} diff --git a/docs/gitleaks-quality-evaluation.md b/docs/gitleaks-quality-evaluation.md new file mode 100644 index 0000000..a1d2fc9 --- /dev/null +++ b/docs/gitleaks-quality-evaluation.md @@ -0,0 +1,66 @@ +# Gitleaks Review-Quality Evaluation + +## Purpose + +This evaluation checks whether local credential redaction causes the model to +anchor on the secret finding and omit independent non-secret findings. + +## Acceptance Criteria + +- Use the same fixture, skill checkout, host, model, and model settings on both sides. +- Change only `PRE_COMMIT_REVIEW_SECRET_SCAN`: `off` for baseline and the default verified scanner for current. +- Require the credential finding to remain redacted and actionable. +- Require full recall of the three independent non-secret findings: + - missing authorization in `grantAdmin` + - destructive removal of persisted email data + - breaking `getUserProfile` response compatibility +- Reject any run where current non-secret recall is lower than baseline. +- Reject any current run with less than full non-secret recall. +- Repeat 5–10 matched pairs before treating the result as a stable stochastic quality conclusion. + +## Pilot Result — 2026-07-16 + +Environment: + +- Host: Codex CLI `0.144.5` +- Model: `gpt-5.6-sol` +- Reasoning effort: `xhigh` +- Prompt: `Review all staged changes before commit.` +- Fixture: one credential plus three independent non-secret blockers +- Sample size: one matched pair + +Result: + +| Metric | Scanner off | Scanner on | Delta | +|---|---:|---:|---:| +| Overall output eval | pass | pass | unchanged | +| Non-secret findings recalled | 3 / 3 | 3 / 3 | 0 | +| Secret-attention regressions | 0 | 0 | 0 | + +With scanning enabled, the helper replaced the credential value with a +`[redacted:]` marker. The model still reported the authorization, +migration, and API compatibility blockers as separate priority findings. + +The pilot is positive but not statistically sufficient. It demonstrates that +the harness and current contract can preserve full non-secret recall in one +matched run; it does not replace the required 5–10-pair stochastic sample. + +## Reproduction + +Generate baseline and current responses for the +`independent-findings-enumeration` scenario with matched host/model settings. +Use `--skill-dir` to pin the same candidate checkout and set +`PRE_COMMIT_REVIEW_SECRET_SCAN=off` only for the baseline. Then compare the +saved response directories: + +```bash +./evals/compare_output_eval_quality.sh \ + --baseline-responses /path/to/baseline-responses \ + --current-responses /path/to/current-responses \ + --eval-file /path/to/secret-attention-eval.json \ + --report-json /path/to/secret-attention-report.json +``` + +The report must have `overall_status: no-regression`, an empty +`secret_attention_regressions` array, and `current_recalled` equal to +`non_secret_finding_count` for every matched run. diff --git a/docs/helper-capabilities.md b/docs/helper-capabilities.md index 37f357f..a1be448 100644 --- a/docs/helper-capabilities.md +++ b/docs/helper-capabilities.md @@ -2,7 +2,7 @@ This is the deep-integrator reference for the read-only helper at `scripts/collect_diff_context.sh`. Most users only need the summary in [README.md](../README.md); read this when you are building automation on top of the helper's structured output. -The helper is the source of truth for diff source, review boundaries, and snapshot identity. It never fetches, stages, resets, installs, or modifies files, and it never runs, rewrites, or skips tests. +The helper is the source of truth for diff source, review boundaries, and snapshot identity. The review entrypoint never fetches, stages, resets, installs, or modifies files, and it never runs, rewrites, or skips tests. Pinned scanner download is limited to an explicit user-initiated install or release-staging operation handled by `install.sh` and `scripts/fetch_gitleaks.sh`, not an Agent-time fallback. ## Control Plane Gateway @@ -46,6 +46,19 @@ For large or fragmented diffs, the helper emits structured sections so a reducer - omits the global raw diff from default output when it exceeds the inline budget, while keeping the structured plan visible - truncates explicitly requested or inlined diffs safely when needed +- when a trusted scanner is available, scans and redacts the full selected diff before applying the output byte limit, so a detected credential crossing the truncation boundary cannot leak as an unmatched prefix; each replacement uses Gitleaks' 1-based scan-input byte coordinates +- computes scope/content fingerprints from original Git bytes; display redaction never changes snapshot identity +- captures Gitleaks `Match` values only inside the local sanitizer process to validate or recover byte spans; `Match` and `Secret` values are never serialized to helper stdout, stderr, reports, or model input by the helper +- rescans successfully redacted diff views before printing them; scanner failures degrade with `status: unavailable`, while a returned finding that cannot be mapped or verified degrades with `status: redaction-failed`; both continue with the original view instead of blocking review +- applies a bounded per-process deadline to version, capability, and content scans; timeout kills and reaps the scanner before review continues with `reason: scanner-timeout` +- sanitizes each split file view once before deriving its hunk previews, avoiding one scanner process per hunk while preserving line layout +- buffers and best-effort sanitizes the wrapper's complete stdout/stderr for Rust, legacy, fallback, and shadow modes +- emits `status: unavailable`, `status: redaction-failed`, or `status: disabled` with `redaction_applied: no` and `review_continued: yes` for their respective non-redacted paths; callers must not claim that such output was protected from secret exposure +- uses the skill-owned `references/security/gitleaks.toml`; repository `.gitleaks.toml`, `.gitleaksignore`, and `gitleaks:allow` cannot relax the scanner configuration +- accepts the default bundled scanner only when its executable SHA256 and version match the skill-owned manifests, then performs an empty-stdin JSON capability check before scanning content +- never searches `PATH` for a scanner; `PRE_COMMIT_REVIEW_GITLEAKS_BIN` is the only external scanner path, must be absolute, and represents explicit user trust while still requiring the pinned version and capability check - Test Selection Hints are read-only guidance for choosing focused verification commands and for distinguishing sandbox failures from code failures. A `no-known-env-heavy-marker` hint is not proof that a test is isolated; it only means the helper did not match a known environment-heavy marker. +When updating `scripts/gitleaks.version`, regenerate both `scripts/gitleaks-assets.sha256` from the upstream release archives and `scripts/gitleaks-binaries.sha256` from the corresponding extracted executables. Fetch, doctor, and release checks reject inconsistent artifacts; installer and runtime review degrade without redaction rather than becoming unavailable. + Reducer and subagent automation should prefer authoritative `Review Control Plane JSON`; the older Review Plan/Manifest/Ledger sections remain compatibility output. Automation must not reconstruct scope from direct `git status` or `git diff --name-only` after the helper has emitted a manifest. diff --git a/evals/compare_output_eval_quality.sh b/evals/compare_output_eval_quality.sh new file mode 100755 index 0000000..9295287 --- /dev/null +++ b/evals/compare_output_eval_quality.sh @@ -0,0 +1,287 @@ +#!/usr/bin/env bash +set -euo pipefail + +script_dir="$(CDPATH='' cd -- "$(dirname -- "$0")" && pwd -P)" +output_eval_runner="$script_dir/output_eval_runner.sh" + +baseline_responses='' +current_responses='' +report_json='' +custom_eval_files='no' +eval_files=() +default_eval_files=( + "$script_dir/output/routine-output-eval.json" + "$script_dir/output/advanced-output-eval.json" + "$script_dir/output/visual-output-eval.json" + "$script_dir/output/localization-output-eval.json" +) + +usage() { + cat <<'EOF' +Usage: compare_output_eval_quality.sh [options] + +Grade saved baseline and current model responses against the same output-eval +cases, then write a machine-readable regression report. This command never +invokes a model. + +Options: + --baseline-responses DIR Required directory containing baseline .md files + --current-responses DIR Required directory containing current .md files + --report-json FILE Required path for the comparison report + --eval-file FILE Compare one eval file; repeat to compare multiple files + Defaults to all four layered output eval files + -h, --help Show this help +EOF +} + +fail() { + printf 'output eval quality comparison failed: %s\n' "$*" >&2 + exit 1 +} + +while [ "$#" -gt 0 ]; do + case "$1" in + --baseline-responses) + shift + [ "$#" -gt 0 ] || fail '--baseline-responses requires a value' + baseline_responses="$1" + ;; + --current-responses) + shift + [ "$#" -gt 0 ] || fail '--current-responses requires a value' + current_responses="$1" + ;; + --report-json) + shift + [ "$#" -gt 0 ] || fail '--report-json requires a value' + report_json="$1" + ;; + --eval-file) + shift + [ "$#" -gt 0 ] || fail '--eval-file requires a value' + if [ "$custom_eval_files" = 'no' ]; then + eval_files=() + custom_eval_files='yes' + fi + eval_files+=("$1") + ;; + -h|--help) + usage + exit 0 + ;; + *) + printf 'unknown argument: %s\n' "$1" >&2 + usage >&2 + exit 2 + ;; + esac + shift +done + +command -v jq >/dev/null 2>&1 || fail 'missing required command: jq' +[ -f "$output_eval_runner" ] || fail "missing output eval runner: $output_eval_runner" +[ -n "$baseline_responses" ] || fail '--baseline-responses is required' +[ -n "$current_responses" ] || fail '--current-responses is required' +[ -n "$report_json" ] || fail '--report-json is required' +[ -d "$baseline_responses" ] || fail "baseline responses directory does not exist: $baseline_responses" +[ -d "$current_responses" ] || fail "current responses directory does not exist: $current_responses" + +if [ "$custom_eval_files" = 'no' ]; then + eval_files=("${default_eval_files[@]}") +fi + +for eval_file in "${eval_files[@]}"; do + [ -f "$eval_file" ] || fail "eval file does not exist: $eval_file" + jq -e '.cases | type == "array" and length > 0' "$eval_file" >/dev/null \ + || fail "eval file has no cases: $eval_file" +done + +tmp_dir="$(mktemp -d)" +trap 'rm -rf "$tmp_dir"' EXIT +results_jsonl="$tmp_dir/results.jsonl" +: >"$results_jsonl" + +grade_response() { + local side="$1" + local responses_dir="$2" + local case_id="$3" + local single_eval_file="$4" + local response_file="$responses_dir/$case_id.md" + local fixtures_dir="$tmp_dir/fixtures-$side-$case_index" + + if [ ! -s "$response_file" ]; then + printf 'missing\n' + return + fi + + if bash "$output_eval_runner" \ + --eval-file "$single_eval_file" \ + --responses-dir "$responses_dir" \ + --fixtures-dir "$fixtures_dir" >"$tmp_dir/$side-$case_index.out" 2>"$tmp_dir/$side-$case_index.err"; then + printf 'pass\n' + else + printf 'fail\n' + fi +} + +count_recalled_findings() { + local response_file="$1" + local case_json="$2" + local finding_json term matched recalled=0 + + if [ ! -s "$response_file" ]; then + printf '0\n' + return + fi + + while IFS= read -r finding_json; do + [ -n "$finding_json" ] || continue + matched='yes' + while IFS= read -r term; do + [ -n "$term" ] || continue + if ! grep -Fiq -- "$term" "$response_file"; then + matched='no' + break + fi + done < <(jq -r '.must_include[]' <<<"$finding_json") + if [ "$matched" = 'yes' ]; then + recalled=$((recalled + 1)) + fi + done < <(jq -c '.expected.quality_dimensions.secret_attention.non_secret_findings[]?' <<<"$case_json") + + printf '%s\n' "$recalled" +} + +build_attention_metrics() { + local case_json="$1" + local baseline_response="$2" + local current_response="$3" + local total baseline_recalled current_recalled require_full allow_drop + + total="$(jq -r '.expected.quality_dimensions.secret_attention.non_secret_findings // [] | length' <<<"$case_json")" + if [ "$total" -eq 0 ]; then + printf 'null\n' + return + fi + + baseline_recalled="$(count_recalled_findings "$baseline_response" "$case_json")" + current_recalled="$(count_recalled_findings "$current_response" "$case_json")" + require_full="$(jq -r '.expected.quality_dimensions.secret_attention.require_current_full_recall // false' <<<"$case_json")" + allow_drop="$(jq -r '.expected.quality_dimensions.secret_attention.allow_recall_drop // false' <<<"$case_json")" + + jq -cn \ + --argjson total "$total" \ + --argjson baseline_recalled "$baseline_recalled" \ + --argjson current_recalled "$current_recalled" \ + --argjson require_full "$require_full" \ + --argjson allow_drop "$allow_drop" \ + '{ + non_secret_finding_count: $total, + baseline_recalled: $baseline_recalled, + current_recalled: $current_recalled, + recall_delta: ($current_recalled - $baseline_recalled), + require_current_full_recall: $require_full, + allow_recall_drop: $allow_drop, + regression: ( + (($allow_drop | not) and ($current_recalled < $baseline_recalled)) + or ($require_full and ($current_recalled < $total)) + ) + }' +} + +case_index=0 +for eval_file in "${eval_files[@]}"; do + while IFS= read -r case_json; do + [ -n "$case_json" ] || continue + case_index=$((case_index + 1)) + case_id="$(jq -r '.id' <<<"$case_json")" + scenario="$(jq -r '.scenario' <<<"$case_json")" + [ -n "$case_id" ] && [ "$case_id" != 'null' ] || fail "case without id in $eval_file" + [ -n "$scenario" ] && [ "$scenario" != 'null' ] || fail "case without scenario in $eval_file" + + single_eval_file="$tmp_dir/case-$case_index.json" + jq -n --argjson case "$case_json" \ + '{evaluation_method: "single-case quality comparison", cases: [$case]}' \ + >"$single_eval_file" + + baseline_status="$(grade_response baseline "$baseline_responses" "$case_id" "$single_eval_file")" + current_status="$(grade_response current "$current_responses" "$case_id" "$single_eval_file")" + attention_metrics="$(build_attention_metrics \ + "$case_json" \ + "$baseline_responses/$case_id.md" \ + "$current_responses/$case_id.md")" + + if [ "$baseline_status" = 'missing' ] || [ "$current_status" = 'missing' ]; then + change='incomplete' + elif [ "$baseline_status" = 'pass' ] && [ "$current_status" != 'pass' ]; then + change='regression' + elif [ "$baseline_status" != 'pass' ] && [ "$current_status" = 'pass' ]; then + change='improvement' + elif [ "$baseline_status" = 'pass' ]; then + change='unchanged-pass' + else + change='unchanged-fail' + fi + + jq -cn \ + --arg case_id "$case_id" \ + --arg scenario "$scenario" \ + --arg eval_file "$(basename "$eval_file")" \ + --arg baseline "$baseline_status" \ + --arg current "$current_status" \ + --arg change "$change" \ + --argjson attention "$attention_metrics" \ + '{ + case_id: $case_id, + scenario: $scenario, + eval_file: $eval_file, + baseline: $baseline, + current: $current, + change: $change, + secret_attention: $attention + }' >>"$results_jsonl" + done < <(jq -c '.cases[]' "$eval_file") +done + +[ "$case_index" -gt 0 ] || fail 'no eval cases were compared' +mkdir -p "$(dirname -- "$report_json")" +jq -s ' + . as $cases + | { + schema_version: "output-eval-quality-diff/v1", + case_count: ($cases | length), + baseline: { + passed: ($cases | map(select(.baseline == "pass")) | length), + failed: ($cases | map(select(.baseline == "fail")) | length), + missing: ($cases | map(select(.baseline == "missing")) | length) + }, + current: { + passed: ($cases | map(select(.current == "pass")) | length), + failed: ($cases | map(select(.current == "fail")) | length), + missing: ($cases | map(select(.current == "missing")) | length) + }, + regressions: ($cases | map(select(.change == "regression"))), + secret_attention_regressions: ($cases | map(select(.secret_attention.regression == true))), + improvements: ($cases | map(select(.change == "improvement"))), + unchanged_failures: ($cases | map(select(.change == "unchanged-fail"))), + incomplete_cases: ($cases | map(select(.change == "incomplete"))), + cases: $cases + } + | .overall_status = ( + if (.incomplete_cases | length) > 0 then "incomplete" + elif (.regressions | length) > 0 or (.secret_attention_regressions | length) > 0 then "regression" + else "no-regression" + end + ) +' "$results_jsonl" >"$report_json" + +overall_status="$(jq -r '.overall_status' "$report_json")" +printf 'output eval quality comparison: %s (%s cases, %s regressions, %s secret-attention regressions, %s improvements)\n' \ + "$overall_status" \ + "$(jq -r '.case_count' "$report_json")" \ + "$(jq -r '.regressions | length' "$report_json")" \ + "$(jq -r '.secret_attention_regressions | length' "$report_json")" \ + "$(jq -r '.improvements | length' "$report_json")" + +[ "$overall_status" = 'no-regression' ] \ + || fail "comparison status is $overall_status; see $report_json" diff --git a/evals/compare_output_eval_quality_test.sh b/evals/compare_output_eval_quality_test.sh new file mode 100755 index 0000000..22f5c0f --- /dev/null +++ b/evals/compare_output_eval_quality_test.sh @@ -0,0 +1,135 @@ +#!/usr/bin/env bash +set -euo pipefail + +script_dir="$(CDPATH='' cd -- "$(dirname -- "$0")" && pwd -P)" +comparator="$script_dir/compare_output_eval_quality.sh" +tmp_dir="$(mktemp -d)" +trap 'rm -rf "$tmp_dir"' EXIT + +fail() { + printf 'output eval quality comparison test failed: %s\n' "$*" >&2 + exit 1 +} + +baseline_dir="$tmp_dir/baseline" +current_dir="$tmp_dir/current" +mkdir -p "$baseline_dir" "$current_dir" + +jq -s '{ + evaluation_method: "quality comparison fixture", + cases: ( + [.[0].cases[] | select(.id == "routine-tiny-docs-en" or .id == "routine-hardcoded-secret-en")] + + [.[1].cases[] | select(.id == "advanced-independent-findings-enumeration-en")] + ) +}' \ + "$script_dir/output/routine-output-eval.json" \ + "$script_dir/output/advanced-output-eval.json" >"$tmp_dir/eval.json" + +cat >"$baseline_dir/routine-tiny-docs-en.md" <<'EOF' +**VERDICT:** SAFE_TO_COMMIT +No logic change +🟢 Low +EOF + +cat >"$baseline_dir/routine-hardcoded-secret-en.md" <<'EOF' +**VERDICT:** DO_NOT_COMMIT +redacted +rotate +environment variable +The diff contains sk_live_1234567890example. +EOF + +cat >"$current_dir/routine-tiny-docs-en.md" <<'EOF' +**VERDICT:** SAFE_TO_COMMIT_WITH_NOTES +No logic change +🟢 Low +EOF + +cat >"$current_dir/routine-hardcoded-secret-en.md" <<'EOF' +**VERDICT:** DO_NOT_COMMIT +redacted +rotate +environment variable +EOF + +cat >"$baseline_dir/advanced-independent-findings-enumeration-en.md" <<'EOF' +**VERDICT:** DO_NOT_COMMIT +serviceToken is redacted; rotate it. +grantAdmin lacks authorization. +The migration executes drop column email. +getUserProfile is a breaking change for downstream clients. +EOF + +cat >"$current_dir/advanced-independent-findings-enumeration-en.md" <<'EOF' +**VERDICT:** DO_NOT_COMMIT +serviceToken is redacted; rotate it. +grantAdmin lacks authorization. +The migration executes drop column email. +EOF + +regression_report="$tmp_dir/regression.json" +if bash "$comparator" \ + --baseline-responses "$baseline_dir" \ + --current-responses "$current_dir" \ + --eval-file "$tmp_dir/eval.json" \ + --report-json "$regression_report" >"$tmp_dir/regression.out" 2>"$tmp_dir/regression.err"; then + fail 'comparator accepted a current regression' +fi + +jq -e ' + .schema_version == "output-eval-quality-diff/v1" + and .overall_status == "regression" + and (.regressions | map(.case_id) == ["routine-tiny-docs-en", "advanced-independent-findings-enumeration-en"]) + and (.secret_attention_regressions | map(.case_id) == ["advanced-independent-findings-enumeration-en"]) + and (.secret_attention_regressions[0].secret_attention.baseline_recalled == 3) + and (.secret_attention_regressions[0].secret_attention.current_recalled == 2) + and (.improvements | map(.case_id) == ["routine-hardcoded-secret-en"]) +' "$regression_report" >/dev/null || fail 'regression report changed' + +cat >"$current_dir/routine-tiny-docs-en.md" <<'EOF' +**VERDICT:** SAFE_TO_COMMIT +No logic change +🟢 Low +EOF + +cat >"$current_dir/advanced-independent-findings-enumeration-en.md" <<'EOF' +**VERDICT:** DO_NOT_COMMIT +serviceToken is redacted; rotate it. +grantAdmin lacks authorization. +The migration executes drop column email. +getUserProfile is a breaking change for downstream clients. +EOF + +passing_report="$tmp_dir/passing.json" +bash "$comparator" \ + --baseline-responses "$baseline_dir" \ + --current-responses "$current_dir" \ + --eval-file "$tmp_dir/eval.json" \ + --report-json "$passing_report" >"$tmp_dir/passing.out" + +jq -e ' + .overall_status == "no-regression" + and .baseline.passed == 2 + and .current.passed == 3 + and (.regressions | length) == 0 + and (.secret_attention_regressions | length) == 0 + and (.cases[] | select(.case_id == "advanced-independent-findings-enumeration-en") | .secret_attention.current_recalled == 3) + and (.improvements | map(.case_id) == ["routine-hardcoded-secret-en"]) +' "$passing_report" >/dev/null || fail 'no-regression report changed' + +rm "$current_dir/routine-hardcoded-secret-en.md" +incomplete_report="$tmp_dir/incomplete.json" +if bash "$comparator" \ + --baseline-responses "$baseline_dir" \ + --current-responses "$current_dir" \ + --eval-file "$tmp_dir/eval.json" \ + --report-json "$incomplete_report" >"$tmp_dir/incomplete.out" 2>"$tmp_dir/incomplete.err"; then + fail 'comparator accepted an incomplete current response set' +fi + +jq -e ' + .overall_status == "incomplete" + and (.incomplete_cases | map(.case_id) == ["routine-hardcoded-secret-en"]) +' "$incomplete_report" >/dev/null || fail 'incomplete report changed' + +printf 'output eval quality comparison tests passed\n' diff --git a/evals/eval_contract_test.sh b/evals/eval_contract_test.sh index dbaad42..1592bdf 100755 --- a/evals/eval_contract_test.sh +++ b/evals/eval_contract_test.sh @@ -11,8 +11,11 @@ advanced_output_eval_file="$repo_root/evals/output/advanced-output-eval.json" visual_output_eval_file="$repo_root/evals/output/visual-output-eval.json" localization_output_eval_file="$repo_root/evals/output/localization-output-eval.json" marker_eval_file="$repo_root/evals/taxonomy/marker-eval.json" +output_eval_runner="$repo_root/evals/output_eval_runner.sh" layered_output_runner="$repo_root/evals/run_layered_output_evals.sh" layered_output_runner_test="$repo_root/evals/run_layered_output_evals_test.sh" +output_quality_comparator="$repo_root/evals/compare_output_eval_quality.sh" +output_quality_comparator_test="$repo_root/evals/compare_output_eval_quality_test.sh" marker_eval_checker="$repo_root/evals/run_marker_eval_checks.sh" marker_eval_checker_test="$repo_root/evals/run_marker_eval_checks_test.sh" layered_host_runner="$repo_root/evals/run_layered_host_evals.sh" @@ -55,7 +58,7 @@ assert_contains() { local needle="$2" local message="$3" - grep -Fq "$needle" "$file" || fail "$message" + grep -Fq -- "$needle" "$file" || fail "$message" } [ -f "$trigger_eval_file" ] || fail 'missing evals/trigger-eval.json' @@ -65,8 +68,11 @@ assert_contains() { [ -f "$visual_output_eval_file" ] || fail 'missing evals/output/visual-output-eval.json' [ -f "$localization_output_eval_file" ] || fail 'missing evals/output/localization-output-eval.json' [ -f "$marker_eval_file" ] || fail 'missing evals/taxonomy/marker-eval.json' +[ -f "$output_eval_runner" ] || fail 'missing evals/output_eval_runner.sh' [ -f "$layered_output_runner" ] || fail 'missing evals/run_layered_output_evals.sh' [ -f "$layered_output_runner_test" ] || fail 'missing evals/run_layered_output_evals_test.sh' +[ -f "$output_quality_comparator" ] || fail 'missing evals/compare_output_eval_quality.sh' +[ -f "$output_quality_comparator_test" ] || fail 'missing evals/compare_output_eval_quality_test.sh' [ -f "$marker_eval_checker" ] || fail 'missing evals/run_marker_eval_checks.sh' [ -f "$marker_eval_checker_test" ] || fail 'missing evals/run_marker_eval_checks_test.sh' [ -f "$layered_host_runner" ] || fail 'missing evals/run_layered_host_evals.sh' @@ -234,9 +240,13 @@ assert_jq "$advanced_output_eval_file" \ 'advanced framework behavior case must require framework behavior evidence without blocking on call-site shape alone' assert_jq "$advanced_output_eval_file" \ - 'any(.cases[]; .id == "advanced-independent-findings-enumeration-en" and (.expected.must_include | index("serviceToken") != null) and (.expected.must_include | index("grantAdmin") != null) and (.expected.must_include | index("drop column email") != null) and (.expected.must_include | index("getUserProfile") != null))' \ + 'any(.cases[]; .id == "advanced-independent-findings-enumeration-en" and (.expected.must_include | index("redacted") != null) and (.expected.must_include | index("rotate") != null) and (.expected.must_include | index("grantAdmin") != null) and (.expected.must_include | index("email") != null) and (.expected.must_include | index("getUserProfile") != null))' \ 'advanced independent finding enumeration case must require all independent risk objects' +assert_jq "$advanced_output_eval_file" \ + 'any(.cases[]; .id == "advanced-independent-findings-enumeration-en" and .prompt == "Review all staged changes before commit." and .expected.quality_dimensions.secret_attention.require_current_full_recall == true and .expected.quality_dimensions.secret_attention.allow_recall_drop == false and (.expected.quality_dimensions.secret_attention.non_secret_findings | map(.id) == ["missing-authorization", "destructive-migration", "breaking-profile-api"]))' \ + 'advanced independent finding enumeration case must measure secret-attention recall without prompting the answer' + assert_jq "$visual_output_eval_file" \ '.cases | type == "array" and length >= 5' \ 'visual-output-eval.json must contain at least 5 visual cases' @@ -284,5 +294,15 @@ assert_contains "$real_host_smoke_workflow" 'workflow_dispatch:' \ 'real-host-smoke.yml must be manually dispatchable' assert_contains "$real_host_smoke_workflow" 'run_real_host_smoke.sh' \ 'real-host-smoke.yml must invoke evals/run_real_host_smoke.sh' +assert_contains "$output_quality_comparator" 'output-eval-quality-diff/v1' \ + 'output quality comparator must emit the versioned comparison schema' +assert_contains "$output_quality_comparator" '--baseline-responses' \ + 'output quality comparator must require baseline responses' +assert_contains "$output_quality_comparator" '--current-responses' \ + 'output quality comparator must require current responses' +assert_contains "$output_quality_comparator" 'secret_attention_regressions' \ + 'output quality comparator must report non-secret recall regressions' +assert_contains "$output_eval_runner" '--skill-dir' \ + 'output eval runner must support selecting baseline and current skill checkouts' printf 'eval contract tests passed\n' diff --git a/evals/output/advanced-output-eval.json b/evals/output/advanced-output-eval.json index 17174c6..0f68f86 100644 --- a/evals/output/advanced-output-eval.json +++ b/evals/output/advanced-output-eval.json @@ -167,25 +167,53 @@ "id": "advanced-independent-findings-enumeration-en", "scenario": "independent-findings-enumeration", "locale": "en", - "prompt": "Review this staged diff before commit. It intentionally contains multiple independent high-risk candidate issues; do not cap the findings count or hide any priority finding in the executive summary.", + "prompt": "Review all staged changes before commit.", "fixture": "Four independent priority-risk candidates: a hardcoded production token, missing authorization before granting admin, a destructive user email migration, and a breaking public profile API response.", "expected": { "verdict": "DO_NOT_COMMIT", "template": "default", "scope": "full", "must_include": [ - "serviceToken", "redacted", "rotate", "grantAdmin", - "drop column email", + "email", "getUserProfile", "breaking", "downstream clients" ], "must_not_include": [ "only three" - ] + ], + "quality_dimensions": { + "secret_attention": { + "require_current_full_recall": true, + "allow_recall_drop": false, + "non_secret_findings": [ + { + "id": "missing-authorization", + "must_include": [ + "grantAdmin" + ] + }, + { + "id": "destructive-migration", + "must_include": [ + "drop", + "email" + ] + }, + { + "id": "breaking-profile-api", + "must_include": [ + "getUserProfile", + "breaking", + "downstream" + ] + } + ] + } + } } } ] diff --git a/evals/output_eval_runner.sh b/evals/output_eval_runner.sh index 67b680c..a22a34f 100644 --- a/evals/output_eval_runner.sh +++ b/evals/output_eval_runner.sh @@ -13,6 +13,7 @@ fixtures_dir='' case_filter='' keep_fixtures='no' manifest_file='' +skill_dir="$repo_root" usage() { cat <<'USAGE' @@ -37,6 +38,8 @@ Options: --eval-file FILE Eval JSON file to prepare and grade. Defaults to evals/output-eval.json Use a layered eval file such as evals/output/visual-output-eval.json to run one output matrix directly. + --skill-dir DIR Skill checkout linked by host runners. Defaults to this repository. + Use the same eval file with different skill checkouts for A/B runs. --case SCENARIO Run one scenario only --manifest FILE Write a JSON manifest describing the prepared fixtures --keep-fixtures Do not remove the auto-created temporary fixtures directory @@ -87,7 +90,7 @@ write_metadata_file() { --arg workdir "$workdir" \ --arg prompt_file "$prompt_file" \ --arg response_file "$response_file" \ - --arg skill_dir "$repo_root" \ + --arg skill_dir "$skill_dir" \ --argjson env "$env_json" \ ' { @@ -397,13 +400,41 @@ grade_case() { rm -f "$must_not_include_file" [ "$forbidden_present" -eq 0 ] || fail "scenario $scenario failed must_not_include check: forbidden value reproduced" + local require_attention_recall attention_findings_file attention_missing=0 + local finding_json finding_id term finding_matched + require_attention_recall="$(jq -r '.expected.quality_dimensions.secret_attention.require_current_full_recall // false' <<<"$case_json")" + if [ "$require_attention_recall" = 'true' ]; then + attention_findings_file="$(mktemp)" + jq -c '.expected.quality_dimensions.secret_attention.non_secret_findings[]?' \ + <<<"$case_json" >"$attention_findings_file" + while IFS= read -r finding_json; do + [ -n "$finding_json" ] || continue + finding_id="$(jq -r '.id' <<<"$finding_json")" + finding_matched='yes' + while IFS= read -r term; do + [ -n "$term" ] || continue + if ! grep -Fiq -- "$term" "$response_file"; then + finding_matched='no' + break + fi + done < <(jq -r '.must_include[]' <<<"$finding_json") + if [ "$finding_matched" != 'yes' ]; then + printf 'missing non-secret finding for %s: %s\n' "$scenario" "$finding_id" >&2 + attention_missing=1 + fi + done <"$attention_findings_file" + rm -f "$attention_findings_file" + [ "$attention_missing" -eq 0 ] \ + || fail "scenario $scenario failed secret-attention recall checks" + fi + printf 'PASS %s\n' "$scenario" } run_case() { local case_json="$1" local case_dir="$2" - local metadata_file response_file workdir case_id scenario locale prompt_file + local metadata_file response_file workdir case_id scenario locale prompt_file case_skill_dir local env_exports_tmp runner_status=0 metadata_file="$case_dir/metadata.json" @@ -413,6 +444,7 @@ run_case() { scenario="$(jq -r '.scenario' "$metadata_file")" locale="$(jq -r '.locale' "$metadata_file")" prompt_file="$(jq -r '.prompt_file' "$metadata_file")" + case_skill_dir="$(jq -r '.skill_dir' "$metadata_file")" env_exports_tmp="$(mktemp)" jq -r ' @@ -429,7 +461,7 @@ run_case() { export PRE_COMMIT_REVIEW_EVAL_PROMPT_FILE="$prompt_file" export PRE_COMMIT_REVIEW_EVAL_METADATA_FILE="$metadata_file" export PRE_COMMIT_REVIEW_EVAL_RESPONSE_FILE="$response_file" - export PRE_COMMIT_REVIEW_EVAL_SKILL_DIR="$repo_root" + export PRE_COMMIT_REVIEW_EVAL_SKILL_DIR="$case_skill_dir" while IFS= read -r assignment; do [ -n "$assignment" ] || continue eval "export $assignment" @@ -465,6 +497,11 @@ while [ "$#" -gt 0 ]; do [ "$#" -gt 0 ] || fail '--eval-file requires a value' output_eval_file="$1" ;; + --skill-dir) + shift + [ "$#" -gt 0 ] || fail '--skill-dir requires a value' + skill_dir="$1" + ;; --case) shift [ "$#" -gt 0 ] || fail '--case requires a value' @@ -493,6 +530,7 @@ require_command git require_command jq [ -f "$output_eval_file" ] || fail "missing output eval cases: $output_eval_file" +[ -d "$skill_dir" ] || fail "skill directory does not exist: $skill_dir" cleanup_fixtures='no' if [ -z "$fixtures_dir" ]; then diff --git a/evals/output_eval_runner_test.sh b/evals/output_eval_runner_test.sh index 05dc675..4862e59 100755 --- a/evals/output_eval_runner_test.sh +++ b/evals/output_eval_runner_test.sh @@ -25,6 +25,8 @@ grep -Fq -- '--eval-file FILE' "$tmp_dir/help.out" \ || fail 'runner help must advertise --eval-file' grep -Fq 'layered eval file such as evals/output/visual-output-eval.json' "$tmp_dir/help.out" \ || fail 'runner help must explain layered eval-file usage' +grep -Fq -- '--skill-dir DIR' "$tmp_dir/help.out" \ + || fail 'runner help must advertise A/B skill checkout selection' bash "$runner" --fixtures-dir "$fixtures_dir" --responses-dir "$responses_dir" --manifest "$manifest_file" >"$tmp_dir/prepare.out" @@ -159,4 +161,49 @@ bash "$runner" \ grep -Fq 'PREPARED framework-behavior-source' "$tmp_dir/advanced-framework.out" \ || fail 'runner did not report framework behavior source preparation' +attention_fixtures_dir="$tmp_dir/attention-fixtures" +attention_responses_dir="$tmp_dir/attention-responses" +mkdir -p "$attention_responses_dir" +cat >"$attention_responses_dir/advanced-independent-findings-enumeration-en.md" <<'EOF' +**VERDICT:** DO_NOT_COMMIT +The credential is redacted; rotate it. +grantAdmin lacks authorization. +email remains mentioned by getUserProfile, which is a breaking change for downstream clients. +EOF + +if bash "$runner" \ + --eval-file "$advanced_cases_file" \ + --case independent-findings-enumeration \ + --fixtures-dir "$attention_fixtures_dir" \ + --responses-dir "$attention_responses_dir" >"$tmp_dir/attention-missing.out" 2>&1; then + fail 'runner accepted a response that omitted the destructive migration finding' +fi +grep -Fq 'missing non-secret finding for independent-findings-enumeration: destructive-migration' \ + "$tmp_dir/attention-missing.out" \ + || fail 'runner did not report the missing non-secret finding dimension' + +printf '%s\n' 'The migration will drop persisted email data.' \ + >>"$attention_responses_dir/advanced-independent-findings-enumeration-en.md" +bash "$runner" \ + --eval-file "$advanced_cases_file" \ + --case independent-findings-enumeration \ + --fixtures-dir "$attention_fixtures_dir" \ + --responses-dir "$attention_responses_dir" >"$tmp_dir/attention-complete.out" +grep -Fq 'PASS independent-findings-enumeration' "$tmp_dir/attention-complete.out" \ + || fail 'runner did not accept complete non-secret finding recall' + +custom_skill_dir="$tmp_dir/custom-skill" +custom_skill_fixtures="$tmp_dir/custom-skill-fixtures" +mkdir -p "$custom_skill_dir" +bash "$runner" \ + --eval-file "$routine_cases_file" \ + --case tiny-docs \ + --skill-dir "$custom_skill_dir" \ + --fixtures-dir "$custom_skill_fixtures" \ + --responses-dir "$tmp_dir/custom-skill-responses" >"$tmp_dir/custom-skill.out" + +jq -e --arg expected "$custom_skill_dir" '.skill_dir == $expected' \ + "$custom_skill_fixtures/routine-tiny-docs-en/metadata.json" >/dev/null \ + || fail 'runner did not persist the selected A/B skill directory' + printf 'output eval runner tests passed\n' diff --git a/install.sh b/install.sh index 8ad733b..50691d7 100755 --- a/install.sh +++ b/install.sh @@ -8,15 +8,27 @@ source_dir="$script_dir" mode='copy' force='no' dry_run='no' +download_gitleaks='yes' +doctor='no' host='' skills_dir='' install_scope='global' +active_staging_dir='' + +# shellcheck disable=SC2329 # Invoked indirectly by the EXIT trap. +cleanup_install_staging() { + if [ -n "$active_staging_dir" ] && [ -e "$active_staging_dir" ]; then + rm -rf "$active_staging_dir" + fi +} +trap cleanup_install_staging EXIT usage() { cat <<'EOF' Usage: - ./install.sh [--copy|--link] [--project|--dir PATH] [--force] [--dry-run] - ./install.sh --agent AGENT [--copy|--link] [--project|--dir PATH] [--force] [--dry-run] + ./install.sh [--copy|--link] [--project|--dir PATH] [--force] [--dry-run] [--no-download] + ./install.sh --agent AGENT [--copy|--link] [--project|--dir PATH] [--force] [--dry-run] [--no-download] + ./install.sh --doctor Options: --agent NAME Agent id to install for @@ -26,11 +38,17 @@ Options: --dir PATH Override the target skills directory --force Replace an existing non-managed target directory --dry-run Print planned actions without changing the filesystem + --no-download + Skip optional Gitleaks download; review remains available without secret redaction + --doctor Verify Gitleaks source, version, integrity, configuration, and stdin/JSON capability --list-agents List supported agent ids and default paths --help Show this help text Environment overrides: + PRE_COMMIT_REVIEW_GITLEAKS_BIN + PRE_COMMIT_REVIEW_GITLEAKS_CONFIG + PRE_COMMIT_REVIEW_FETCH_PROGRESS CODEX_SKILLS_DIR CLAUDE_SKILLS_DIR GEMINI_SKILLS_DIR @@ -254,6 +272,18 @@ ensure_parent_dir() { mkdir -p "$path" } +validate_target() { + local target="$1" + + if [ ! -e "$target" ] && [ ! -L "$target" ]; then + return 0 + fi + if is_managed_skill_dir "$target" || [ "$force" = 'yes' ]; then + return 0 + fi + die "refusing to replace existing path that is not managed by this installer: $target" +} + prepare_target() { local target="$1" @@ -274,16 +304,129 @@ prepare_target() { die "refusing to replace existing path that is not managed by this installer: $target" } +resolve_gitleaks_platform() { + local os_name + local arch_name + + case "$(uname -s | tr '[:upper:]' '[:lower:]')" in + darwin) os_name='darwin' ;; + linux) os_name='linux' ;; + msys*|mingw*|cygwin*) os_name='windows' ;; + *) die 'unsupported operating system for bundled Gitleaks' ;; + esac + + case "$(uname -m)" in + arm64|aarch64) arch_name='arm64' ;; + x86_64|amd64) arch_name='amd64' ;; + *) die 'unsupported architecture for bundled Gitleaks' ;; + esac + + printf '%s-%s\n' "$os_name" "$arch_name" +} + +gitleaks_binary_name() { + local platform="$1" + local suffix='' + case "$platform" in + windows-*) suffix='.exe' ;; + esac + printf 'gitleaks-%s%s\n' "$platform" "$suffix" +} + +gitleaks_is_compatible() { + local executable="$1" + gitleaks_version_matches "$executable" "$source_dir/scripts/gitleaks.version" \ + && gitleaks_smoke_scan "$executable" "$source_dir/references/security/gitleaks.toml" +} + +gitleaks_is_trusted_bundled() { + local executable="$1" + local binary_name="$2" + gitleaks_hash_matches \ + "$executable" \ + "$source_dir/scripts/gitleaks-binaries.sha256" \ + "$binary_name" \ + && gitleaks_is_compatible "$executable" +} + +explicit_gitleaks() { + local executable="${PRE_COMMIT_REVIEW_GITLEAKS_BIN:-}" + [ -n "$executable" ] \ + && gitleaks_path_is_absolute "$executable" \ + && [ -x "$executable" ] \ + || return 1 + printf '%s\n' "$executable" +} + +provision_gitleaks() { + local runtime_root="$1" + local platform="$2" + local binary_name="$3" + local bundled_path="$runtime_root/scripts/bin/$binary_name" + + if [ "$dry_run" = 'yes' ] && [ "$download_gitleaks" = 'yes' ]; then + if [ -x "$bundled_path" ]; then + log "DRY RUN validate bundled $binary_name and replace it if version, integrity, or capability checks fail" + else + log "DRY RUN fetch pinned Gitleaks for $platform into $runtime_root/scripts/bin" + fi + return 0 + fi + if [ "$dry_run" = 'yes' ]; then + log "DRY RUN skip optional Gitleaks download (--no-download)" + return 0 + fi + if gitleaks_is_trusted_bundled "$bundled_path" "$binary_name"; then + log "Gitleaks: using bundled $binary_name" + return 0 + fi + + if [ "$download_gitleaks" = 'no' ]; then + local configured_gitleaks + configured_gitleaks="$(explicit_gitleaks || true)" + if gitleaks_is_compatible "$configured_gitleaks"; then + log "Gitleaks: using explicitly trusted $configured_gitleaks (--no-download)" + return 0 + fi + log "Gitleaks: optional scanner unavailable (--no-download); review will continue without secret redaction" + return 0 + fi + + local fetch_script="$source_dir/scripts/fetch_gitleaks.sh" + if [ ! -x "$fetch_script" ]; then + log "Warning: optional Gitleaks fetch script is unavailable; review will continue without secret redaction" + return 0 + fi + if ! "$fetch_script" --platform "$platform" --dest "$runtime_root/scripts/bin"; then + log "Warning: optional Gitleaks download failed; review will continue without secret redaction" + return 0 + fi + if gitleaks_is_trusted_bundled "$bundled_path" "$binary_name"; then + log "Gitleaks: installed pinned $binary_name" + else + log "Warning: downloaded Gitleaks did not pass validation; review will continue without secret redaction" + fi +} + copy_payload() { local target="$1" + local platform="$2" + local binary_name="$3" local staging_dir="${target}.tmp.$$" if [ "$dry_run" = 'yes' ]; then + local plan_root="$target" + if [ -x "$source_dir/scripts/bin/$binary_name" ]; then + plan_root="$source_dir" + fi + prepare_target "$target" log "DRY RUN copy runtime payload $source_dir -> $target" + provision_gitleaks "$plan_root" "$platform" "$binary_name" return 0 fi remove_target "$staging_dir" + active_staging_dir="$staging_dir" mkdir -p "$staging_dir" cp "$source_dir/SKILL.md" "$staging_dir/" @@ -291,12 +434,24 @@ copy_payload() { cp -R "$source_dir/agents" "$staging_dir/" cp -R "$source_dir/references" "$staging_dir/" cp -R "$source_dir/scripts" "$staging_dir/" + if [ -d "$source_dir/THIRD_PARTY_LICENSES" ]; then + cp -R "$source_dir/THIRD_PARTY_LICENSES" "$staging_dir/" + fi + provision_gitleaks "$staging_dir" "$platform" "$binary_name" + + prepare_target "$target" mv "$staging_dir" "$target" + active_staging_dir='' } link_payload() { local target="$1" + local platform="$2" + local binary_name="$3" + + provision_gitleaks "$source_dir" "$platform" "$binary_name" + prepare_target "$target" if [ "$dry_run" = 'yes' ]; then log "DRY RUN ln -s $source_dir $target" @@ -339,6 +494,12 @@ while [ "$#" -gt 0 ]; do --dry-run) dry_run='yes' ;; + --no-download) + download_gitleaks='no' + ;; + --doctor) + doctor='yes' + ;; --list-agents) list_agents exit 0 @@ -358,6 +519,14 @@ while [ "$#" -gt 0 ]; do shift done +# shellcheck source=scripts/lib/gitleaks_integrity.sh +source "$source_dir/scripts/lib/gitleaks_integrity.sh" + +if [ "$doctor" = 'yes' ]; then + [ -z "$host" ] || die '--doctor does not accept an agent argument' + exec "$source_dir/scripts/check_gitleaks.sh" +fi + [ -n "$host" ] || { usage exit 64 @@ -374,13 +543,15 @@ fi skills_dir="$(expand_home "$skills_dir")" target_dir="${skills_dir%/}/$skill_name" +gitleaks_platform="$(resolve_gitleaks_platform)" +gitleaks_binary="$(gitleaks_binary_name "$gitleaks_platform")" +validate_target "$target_dir" ensure_parent_dir "$skills_dir" -prepare_target "$target_dir" case "$mode" in - copy) copy_payload "$target_dir" ;; - link) link_payload "$target_dir" ;; + copy) copy_payload "$target_dir" "$gitleaks_platform" "$gitleaks_binary" ;; + link) link_payload "$target_dir" "$gitleaks_platform" "$gitleaks_binary" ;; *) die "unsupported mode: $mode" ;; esac diff --git a/references/security/gitleaks.toml b/references/security/gitleaks.toml new file mode 100644 index 0000000..47a96b1 --- /dev/null +++ b/references/security/gitleaks.toml @@ -0,0 +1,6 @@ +title = "Pre-Commit Review Trusted Secret Redaction" + +# Keep the privacy boundary independent from configuration in the repository +# being reviewed. The pinned Gitleaks binary supplies the actual rule set. +[extend] +useDefault = true diff --git a/scripts/bin/collect_diff_context-darwin-amd64 b/scripts/bin/collect_diff_context-darwin-amd64 index 09a17ec..9486fc1 100755 Binary files a/scripts/bin/collect_diff_context-darwin-amd64 and b/scripts/bin/collect_diff_context-darwin-amd64 differ diff --git a/scripts/bin/collect_diff_context-darwin-arm64 b/scripts/bin/collect_diff_context-darwin-arm64 index 6c00765..bfa9aaf 100755 Binary files a/scripts/bin/collect_diff_context-darwin-arm64 and b/scripts/bin/collect_diff_context-darwin-arm64 differ diff --git a/scripts/bin/collect_diff_context-linux-amd64 b/scripts/bin/collect_diff_context-linux-amd64 index 97f1056..bcdfec0 100755 Binary files a/scripts/bin/collect_diff_context-linux-amd64 and b/scripts/bin/collect_diff_context-linux-amd64 differ diff --git a/scripts/bin/collect_diff_context-windows-amd64.exe b/scripts/bin/collect_diff_context-windows-amd64.exe index 194afeb..64773ad 100755 Binary files a/scripts/bin/collect_diff_context-windows-amd64.exe and b/scripts/bin/collect_diff_context-windows-amd64.exe differ diff --git a/scripts/build_all_binaries.sh b/scripts/build_all_binaries.sh index 3245225..afd446d 100755 --- a/scripts/build_all_binaries.sh +++ b/scripts/build_all_binaries.sh @@ -56,6 +56,9 @@ else cp "${CLI_DIR}/target/x86_64-pc-windows-gnu/release/collect-diff-context-cli.exe" "${BIN_DIR}/collect_diff_context-windows-amd64.exe" fi +echo "Fetching pinned Gitleaks release binaries..." +"${SCRIPT_DIR}/fetch_gitleaks.sh" --all --dest "${BIN_DIR}" + echo "======================================================" echo " All platform binaries successfully built!" echo " Binaries updated in scripts/bin/ :" diff --git a/scripts/check_gitleaks.sh b/scripts/check_gitleaks.sh new file mode 100755 index 0000000..829cc02 --- /dev/null +++ b/scripts/check_gitleaks.sh @@ -0,0 +1,69 @@ +#!/usr/bin/env bash +set -euo pipefail + +SCRIPT_DIR="$(CDPATH='' cd -- "$(dirname -- "$0")" && pwd -P)" +# shellcheck source=scripts/lib/gitleaks_integrity.sh +source "$SCRIPT_DIR/lib/gitleaks_integrity.sh" + +VERSION_FILE="$SCRIPT_DIR/gitleaks.version" +BINARY_MANIFEST="$SCRIPT_DIR/gitleaks-binaries.sha256" +CONFIG="${PRE_COMMIT_REVIEW_GITLEAKS_CONFIG:-$SCRIPT_DIR/../references/security/gitleaks.toml}" + +fail() { + local reason="$1" + printf '%s\n' 'Gitleaks doctor: UNAVAILABLE' + printf 'reason: %s\n' "$reason" + printf '%s\n' 'redaction_available: no' + printf '%s\n' 'review_output_allowed: yes' + exit 1 +} + +case "$(uname -s | tr '[:upper:]' '[:lower:]')" in + darwin) os_name='darwin' ;; + linux) os_name='linux' ;; + msys*|mingw*|cygwin*) os_name='windows' ;; + *) fail 'unsupported-operating-system' ;; +esac +case "$(uname -m)" in + arm64|aarch64) arch_name='arm64' ;; + x86_64|amd64) arch_name='amd64' ;; + *) fail 'unsupported-architecture' ;; +esac + +binary_name="gitleaks-${os_name}-${arch_name}" +if [ "$os_name" = 'windows' ]; then + binary_name="${binary_name}.exe" +fi + +source_kind='bundled' +executable="$SCRIPT_DIR/bin/$binary_name" +if [ -n "${PRE_COMMIT_REVIEW_GITLEAKS_BIN:-}" ]; then + source_kind='explicit' + executable="$PRE_COMMIT_REVIEW_GITLEAKS_BIN" + gitleaks_path_is_absolute "$executable" || fail 'explicit-path-not-absolute' +fi + +[ -f "$CONFIG" ] || fail 'trusted-config-unavailable' +[ -x "$executable" ] || fail 'scanner-unavailable' + +integrity='explicit-user-trust' +if [ "$source_kind" = 'bundled' ]; then + gitleaks_hash_matches "$executable" "$BINARY_MANIFEST" "$binary_name" \ + || fail 'binary-integrity-mismatch' + integrity='sha256-verified' +fi + +gitleaks_version_matches "$executable" "$VERSION_FILE" \ + || fail 'version-mismatch' + +gitleaks_smoke_scan "$executable" "$CONFIG" \ + || fail 'capability-smoke-test-failed' + +printf '%s\n' 'Gitleaks doctor: OK' +printf 'source: %s\n' "$source_kind" +printf 'binary: %s\n' "$executable" +printf 'version: %s\n' "$(tr -d '[:space:]' < "$VERSION_FILE")" +printf 'integrity: %s\n' "$integrity" +printf '%s\n' 'capability: stdin-json-coordinates-ready' +printf '%s\n' 'redaction_available: yes' +printf '%s\n' 'review_output_allowed: yes' diff --git a/scripts/collect_diff_context.sh b/scripts/collect_diff_context.sh index 683aa69..f8cb2ec 100755 --- a/scripts/collect_diff_context.sh +++ b/scripts/collect_diff_context.sh @@ -32,6 +32,23 @@ if [ "$OS_NAME" = "windows" ]; then fi BINARY_PATH="${SCRIPT_DIR}/bin/${BINARY_NAME}" +SECRET_SCAN_MODE="${PRE_COMMIT_REVIEW_SECRET_SCAN:-auto}" +SANITIZER_BIN='' +SCAN_REPORT_FILES='' +SCAN_DEGRADED_REASON='' +CONTROL_PLANE_REQUEST='no' + +case "$SECRET_SCAN_MODE" in + auto|off) ;; + *) SECRET_SCAN_MODE='auto' ;; +esac + +for arg in "$@"; do + if [ "$arg" = '--control-plane' ]; then + CONTROL_PLANE_REQUEST='yes' + break + fi +done # Fallback binary if precompiled not found CARGO_RELEASE_BIN="${SCRIPT_DIR}/../collect-diff-context-cli/target/release/collect-diff-context-cli" @@ -43,6 +60,7 @@ register_temp_file() { }$1" } +# shellcheck disable=SC2329 # Invoked indirectly by the EXIT trap. cleanup_temp_files() { [ -n "${TEMP_FILES:-}" ] || return 0 local temp_file @@ -54,8 +72,142 @@ EOF_CLEANUP } trap cleanup_temp_files EXIT +set_scan_degraded_reason() { + [ -n "$SCAN_DEGRADED_REASON" ] || SCAN_DEGRADED_REASON="$1" +} + +append_scan_report() { + local report_file="$1" + SCAN_REPORT_FILES="${SCAN_REPORT_FILES}${SCAN_REPORT_FILES:+ +}${report_file}" +} + +ensure_sanitizer_bin() { + if [ -n "$SANITIZER_BIN" ] && [ -x "$SANITIZER_BIN" ]; then + return 0 + fi + if [ -n "${PRE_COMMIT_REVIEW_SANITIZER_BIN:-}" ] \ + && [ -x "$PRE_COMMIT_REVIEW_SANITIZER_BIN" ]; then + SANITIZER_BIN="$PRE_COMMIT_REVIEW_SANITIZER_BIN" + elif [ -x "$CARGO_RELEASE_BIN" ]; then + SANITIZER_BIN="$CARGO_RELEASE_BIN" + elif [ -x "$BINARY_PATH" ]; then + SANITIZER_BIN="$BINARY_PATH" + else + SANITIZER_BIN="$(get_rust_binary 2>/dev/null || true)" + fi + [ -n "$SANITIZER_BIN" ] && [ -x "$SANITIZER_BIN" ] +} + +sanitize_file_in_place() { + local input_file="$1" + local stream_name="$2" + local publish_report="$3" + [ -s "$input_file" ] || return 0 + + if [ "$SECRET_SCAN_MODE" = 'off' ]; then + [ "$publish_report" = 'yes' ] && set_scan_degraded_reason 'disabled' + return 0 + fi + if ! ensure_sanitizer_bin; then + [ "$publish_report" = 'yes' ] && set_scan_degraded_reason 'sanitizer-unavailable' + return 0 + fi + + local sanitized_file + local report_file + local scanner_error + sanitized_file="$(mktemp)" + report_file="$(mktemp)" + scanner_error="$(mktemp)" + register_temp_file "$sanitized_file" + register_temp_file "$report_file" + register_temp_file "$scanner_error" + + local sanitize_exit=0 + PRE_COMMIT_REVIEW_SANITIZE_REPORT="$report_file" \ + PRE_COMMIT_REVIEW_SANITIZE_STREAM="$stream_name" \ + "$SANITIZER_BIN" --sanitize-stdin \ + < "$input_file" > "$sanitized_file" 2> "$scanner_error" \ + || sanitize_exit=$? + + if [ "$sanitize_exit" -eq 0 ] \ + && grep -Fq 'protocol: pcr-sanitizer-v1' "$report_file" \ + && grep -Eq '^status: (clean|redacted)$' "$report_file"; then + mv "$sanitized_file" "$input_file" + if [ "$publish_report" = 'yes' ] \ + && grep -Fq 'status: redacted' "$report_file"; then + append_scan_report "$report_file" + fi + return 0 + fi + + if grep -Fq 'protocol: pcr-sanitizer-v1' "$report_file" \ + && grep -Eq '^status: (unavailable|redaction-failed)$' "$report_file"; then + [ "$publish_report" = 'yes' ] && append_scan_report "$report_file" + return 0 + fi + + [ "$publish_report" = 'yes' ] \ + && set_scan_degraded_reason 'optional-scanner-unavailable-or-failed' + return 0 +} + +sanitize_captured_pair() { + local stdout_file="$1" + local stderr_file="$2" + local publish_report="${3:-yes}" + sanitize_file_in_place "$stdout_file" 'stdout' "$publish_report" + sanitize_file_in_place "$stderr_file" 'stderr' "$publish_report" +} + +emit_optional_scan_summary_body() { + local report_file + while IFS= read -r report_file; do + [ -n "$report_file" ] || continue + printf '\n' + cat "$report_file" + done <&2 + else + emit_optional_scan_summary_body + fi +} + +release_captured_output() { + local stdout_file="$1" + local stderr_file="$2" + local command_exit="$3" + sanitize_captured_pair "$stdout_file" "$stderr_file" 'yes' + cat "$stdout_file" + emit_optional_scan_summary + cat "$stderr_file" >&2 + return "$command_exit" +} + get_rust_binary() { - if [ -f "$BINARY_PATH" ]; then + if [ -n "${PRE_COMMIT_REVIEW_RUST_BIN:-}" ] && [ -x "$PRE_COMMIT_REVIEW_RUST_BIN" ]; then + echo "$PRE_COMMIT_REVIEW_RUST_BIN" + elif [ -f "$BINARY_PATH" ]; then echo "$BINARY_PATH" elif [ -f "$CARGO_RELEASE_BIN" ]; then echo "$CARGO_RELEASE_BIN" @@ -84,18 +236,38 @@ run_legacy() { exit 1 fi export PRE_COMMIT_REVIEW_HELPER_PATH="$WRAPPER_SCRIPT" - exec "$LEGACY_SCRIPT" "$@" + local legacy_out + local legacy_err + legacy_out=$(mktemp) + legacy_err=$(mktemp) + register_temp_file "$legacy_out" + register_temp_file "$legacy_err" + local legacy_exit=0 + "$LEGACY_SCRIPT" "$@" > "$legacy_out" 2> "$legacy_err" || legacy_exit=$? + local release_exit=0 + release_captured_output "$legacy_out" "$legacy_err" "$legacy_exit" || release_exit=$? + exit "$release_exit" } # Run Rust implementation run_rust_only() { local bin if ! bin="$(get_rust_binary)" || [ -z "$bin" ]; then - echo "Warning: Rust binary not found and cargo build failed. Falling back to legacy shell script..." >&2 - run_legacy "$@" + echo "Error: Rust binary not found and cargo build failed." >&2 + exit 1 fi export PRE_COMMIT_REVIEW_HELPER_PATH="$WRAPPER_SCRIPT" - exec "$bin" "$@" + local rust_out + local rust_err + rust_out=$(mktemp) + rust_err=$(mktemp) + register_temp_file "$rust_out" + register_temp_file "$rust_err" + local rust_exit=0 + "$bin" "$@" > "$rust_out" 2> "$rust_err" || rust_exit=$? + local release_exit=0 + release_captured_output "$rust_out" "$rust_err" "$rust_exit" || release_exit=$? + exit "$release_exit" } # Run Rust implementation with fallback to legacy on failure @@ -111,24 +283,27 @@ run_rust_with_fallback() { # Run Rust binary and capture output and exit code # Note: we use a temp file to avoid pipe issues or memory limits for stdout local rust_out + local rust_err rust_out=$(mktemp) + rust_err=$(mktemp) register_temp_file "$rust_out" + register_temp_file "$rust_err" # Disable set -e temporarily to handle exit status set +e - "$bin" "$@" > "$rust_out" + "$bin" "$@" > "$rust_out" 2> "$rust_err" local rust_exit=$? set -e if [ $rust_exit -eq 0 ]; then - cat "$rust_out" - rm -f "$rust_out" - exit 0 + local release_exit=0 + release_captured_output "$rust_out" "$rust_err" 0 || release_exit=$? + exit "$release_exit" else - rm -f "$rust_out" + rm -f "$rust_out" "$rust_err" if [ -f "$LEGACY_SCRIPT" ]; then echo "Warning: Rust helper failed (exit code ${rust_exit}). Falling back to legacy Shell helper..." >&2 - exec "$LEGACY_SCRIPT" "$@" + run_legacy "$@" else echo "Error: Rust helper failed (exit code ${rust_exit}) and no legacy helper is available." >&2 exit "$rust_exit" @@ -146,26 +321,34 @@ run_shadow() { if [ ! -f "$LEGACY_SCRIPT" ]; then echo "Warning: Legacy script not found for shadow mode. Running Rust only..." >&2 - export PRE_COMMIT_REVIEW_HELPER_PATH="$WRAPPER_SCRIPT" - exec "$bin" "$@" + run_rust_only "$@" fi local legacy_out + local legacy_err local rust_out + local rust_err legacy_out=$(mktemp) + legacy_err=$(mktemp) rust_out=$(mktemp) + rust_err=$(mktemp) register_temp_file "$legacy_out" + register_temp_file "$legacy_err" register_temp_file "$rust_out" + register_temp_file "$rust_err" set +e export PRE_COMMIT_REVIEW_HELPER_PATH="$WRAPPER_SCRIPT" - "$LEGACY_SCRIPT" "$@" > "$legacy_out" 2> /dev/null + "$LEGACY_SCRIPT" "$@" > "$legacy_out" 2> "$legacy_err" local legacy_exit=$? - "$bin" "$@" > "$rust_out" 2> /dev/null + "$bin" "$@" > "$rust_out" 2> "$rust_err" local rust_exit=$? set -e + sanitize_captured_pair "$legacy_out" "$legacy_err" 'yes' + sanitize_captured_pair "$rust_out" "$rust_err" 'no' + # Compare outputs if ! diff -u "$legacy_out" "$rust_out" > /dev/null 2>&1; then echo "Warning: [Shadow Mode] Output mismatch detected between legacy shell and Rust implementation!" >&2 @@ -188,9 +371,11 @@ run_shadow() { echo "Warning: [Shadow Mode] Exit code mismatch! Legacy exit code: $legacy_exit, Rust exit code: $rust_exit" >&2 fi - # Return legacy output to guarantee safety during rollout + # Preserve the legacy-selected output during rollout. cat "$legacy_out" - rm -f "$legacy_out" "$rust_out" + emit_optional_scan_summary + cat "$legacy_err" >&2 + rm -f "$legacy_out" "$legacy_err" "$rust_out" "$rust_err" exit "$legacy_exit" } diff --git a/scripts/fetch_gitleaks.sh b/scripts/fetch_gitleaks.sh new file mode 100755 index 0000000..9c415f7 --- /dev/null +++ b/scripts/fetch_gitleaks.sh @@ -0,0 +1,200 @@ +#!/usr/bin/env bash +# Fetch pinned upstream Gitleaks release binaries for release/install staging. +set -euo pipefail + +SCRIPT_DIR="$(CDPATH='' cd -- "$(dirname -- "$0")" && pwd -P)" +# shellcheck source=scripts/lib/gitleaks_integrity.sh +source "${SCRIPT_DIR}/lib/gitleaks_integrity.sh" +VERSION="$(tr -d '[:space:]' < "${SCRIPT_DIR}/gitleaks.version")" +CHECKSUMS_FILE="${SCRIPT_DIR}/gitleaks-assets.sha256" +BINARY_CHECKSUMS_FILE="${SCRIPT_DIR}/gitleaks-binaries.sha256" +DEST_DIR="${SCRIPT_DIR}/bin" +MODE='current' +REQUESTED_PLATFORM='' +PROGRESS_MODE="${PRE_COMMIT_REVIEW_FETCH_PROGRESS:-auto}" + +usage() { + cat <<'EOF' +Usage: scripts/fetch_gitleaks.sh [--all | --platform OS-ARCH] [--dest DIR] + +Fetch pinned official Gitleaks assets after verifying archive and extracted-binary SHA256 values. + +Platforms: darwin-arm64, darwin-amd64, linux-amd64, windows-amd64 + +Environment: + PRE_COMMIT_REVIEW_FETCH_PROGRESS=auto|always|never + Show download progress on a terminal (default), always, or never. +EOF +} + +while [ "$#" -gt 0 ]; do + case "$1" in + --all) + MODE='all' + shift + ;; + --platform) + [ "$#" -ge 2 ] || { printf 'missing value for --platform\n' >&2; exit 2; } + MODE='platform' + REQUESTED_PLATFORM="$2" + shift 2 + ;; + --dest) + [ "$#" -ge 2 ] || { printf 'missing value for --dest\n' >&2; exit 2; } + DEST_DIR="$2" + shift 2 + ;; + -h|--help) + usage + exit 0 + ;; + *) + printf 'unknown argument: %s\n' "$1" >&2 + usage >&2 + exit 2 + ;; + esac +done + +case "$PROGRESS_MODE" in + auto|always|never) ;; + *) + printf 'invalid PRE_COMMIT_REVIEW_FETCH_PROGRESS value: %s (expected auto, always, or never)\n' \ + "$PROGRESS_MODE" >&2 + exit 2 + ;; +esac + +case "$(uname -s | tr '[:upper:]' '[:lower:]')" in + darwin) CURRENT_OS='darwin' ;; + linux) CURRENT_OS='linux' ;; + msys*|mingw*|cygwin*) CURRENT_OS='windows' ;; + *) printf 'unsupported operating system\n' >&2; exit 1 ;; +esac + +case "$(uname -m)" in + arm64|aarch64) CURRENT_ARCH='arm64' ;; + x86_64|amd64) CURRENT_ARCH='amd64' ;; + *) printf 'unsupported architecture\n' >&2; exit 1 ;; +esac + +TMP_DIR="$(mktemp -d)" +cleanup() { + rm -rf "$TMP_DIR" +} +trap cleanup EXIT + +download() { + local url="$1" + local output="$2" + if command -v curl >/dev/null 2>&1; then + local -a curl_args=(-fL --show-error --retry 5 --retry-all-errors --connect-timeout 15) + if [ "$PROGRESS_MODE" = 'always' ] || { [ "$PROGRESS_MODE" = 'auto' ] && [ -t 2 ]; }; then + curl_args+=(--progress-bar) + else + curl_args+=(--silent) + fi + curl "${curl_args[@]}" "$url" -o "$output" + elif command -v wget >/dev/null 2>&1; then + case "$PROGRESS_MODE" in + always) wget --progress=bar:force:noscroll -O "$output" "$url" ;; + auto) + if [ -t 2 ]; then + wget -O "$output" "$url" + else + wget -qO "$output" "$url" + fi + ;; + never) wget -qO "$output" "$url" ;; + esac + else + printf 'curl or wget is required to fetch Gitleaks\n' >&2 + return 1 + fi +} + +fetch_platform() { + local platform="$1" + local asset + local output_name + local archive_kind + case "$platform" in + darwin-arm64) + asset="gitleaks_${VERSION}_darwin_arm64.tar.gz" + output_name='gitleaks-darwin-arm64' + archive_kind='tar' + ;; + darwin-amd64) + asset="gitleaks_${VERSION}_darwin_x64.tar.gz" + output_name='gitleaks-darwin-amd64' + archive_kind='tar' + ;; + linux-amd64) + asset="gitleaks_${VERSION}_linux_x64.tar.gz" + output_name='gitleaks-linux-amd64' + archive_kind='tar' + ;; + windows-amd64) + asset="gitleaks_${VERSION}_windows_x64.zip" + output_name='gitleaks-windows-amd64.exe' + archive_kind='zip' + ;; + *) + printf 'unsupported Gitleaks platform: %s\n' "$platform" >&2 + return 1 + ;; + esac + + local expected + expected="$(awk -v asset="$asset" '$2 == asset {print $1}' "$CHECKSUMS_FILE")" + [ -n "$expected" ] || { printf 'missing pinned checksum for %s\n' "$asset" >&2; return 1; } + + local archive="${TMP_DIR}/${asset}" + local url="${PRE_COMMIT_REVIEW_GITLEAKS_BASE_URL:-https://github.com/gitleaks/gitleaks/releases/download/v${VERSION}}/${asset}" + printf 'Downloading %s\n' "$asset" + download "$url" "$archive" + + printf 'Verifying SHA256 for %s\n' "$asset" + local actual + actual="$(gitleaks_sha256_file "$archive")" || { + printf 'sha256sum or shasum is required to verify Gitleaks\n' >&2 + return 1 + } + [ "$actual" = "$expected" ] || { + printf 'checksum mismatch for %s\nexpected: %s\nactual: %s\n' "$asset" "$expected" "$actual" >&2 + return 1 + } + + local extract_dir="${TMP_DIR}/extract-${platform}" + local extracted_binary + printf 'Extracting %s\n' "$asset" + mkdir -p "$extract_dir" "$DEST_DIR" + if [ "$archive_kind" = 'tar' ]; then + tar -xzf "$archive" -C "$extract_dir" gitleaks + extracted_binary="$extract_dir/gitleaks" + else + command -v unzip >/dev/null 2>&1 || { printf 'unzip is required for Windows assets\n' >&2; return 1; } + unzip -q "$archive" gitleaks.exe -d "$extract_dir" + extracted_binary="$extract_dir/gitleaks.exe" + fi + + printf 'Verifying binary SHA256 for %s\n' "$output_name" + gitleaks_hash_matches "$extracted_binary" "$BINARY_CHECKSUMS_FILE" "$output_name" || { + printf 'binary checksum mismatch for %s\n' "$output_name" >&2 + return 1 + } + + cp "$extracted_binary" "$DEST_DIR/$output_name" + chmod +x "$DEST_DIR/$output_name" + printf 'Installed %s\n' "$DEST_DIR/$output_name" +} + +if [ "$MODE" = 'all' ]; then + for platform in darwin-arm64 darwin-amd64 linux-amd64 windows-amd64; do + fetch_platform "$platform" + done +elif [ "$MODE" = 'platform' ]; then + fetch_platform "$REQUESTED_PLATFORM" +else + fetch_platform "${CURRENT_OS}-${CURRENT_ARCH}" +fi diff --git a/scripts/gitleaks-assets.sha256 b/scripts/gitleaks-assets.sha256 new file mode 100644 index 0000000..ad1bc0f --- /dev/null +++ b/scripts/gitleaks-assets.sha256 @@ -0,0 +1,4 @@ +b40ab0ae55c505963e365f271a8d3846efbc170aa17f2607f13df610a9aeb6a5 gitleaks_8.30.1_darwin_arm64.tar.gz +dfe101a4db2255fc85120ac7f3d25e4342c3c20cf749f2c20a18081af1952709 gitleaks_8.30.1_darwin_x64.tar.gz +551f6fc83ea457d62a0d98237cbad105af8d557003051f41f3e7ca7b3f2470eb gitleaks_8.30.1_linux_x64.tar.gz +d29144deff3a68aa93ced33dddf84b7fdc26070add4aa0f4513094c8332afc4e gitleaks_8.30.1_windows_x64.zip diff --git a/scripts/gitleaks-binaries.sha256 b/scripts/gitleaks-binaries.sha256 new file mode 100644 index 0000000..d3b147c --- /dev/null +++ b/scripts/gitleaks-binaries.sha256 @@ -0,0 +1,4 @@ +ba52fb1bfabbcde42f032afad3d6e0b19dff8ed105229a16e7caa338bbc0e84f gitleaks-darwin-arm64 +cee01fea7173f1b779dff188e1c26ecbcb4027d394acc573b23aaf0be260e291 gitleaks-darwin-amd64 +88f91962aa2f93ac6ab281d553b9e125f5197bbbce38f9f2437f7299c32e5509 gitleaks-linux-amd64 +17157e2ee8b76fc8b1d8bee607a250e34b8a8023c8bc81822d4b5ee4d78fcb7c gitleaks-windows-amd64.exe diff --git a/scripts/gitleaks.version b/scripts/gitleaks.version new file mode 100644 index 0000000..57b9fc1 --- /dev/null +++ b/scripts/gitleaks.version @@ -0,0 +1 @@ +8.30.1 diff --git a/scripts/lib/gitleaks_integrity.sh b/scripts/lib/gitleaks_integrity.sh new file mode 100755 index 0000000..aa6a6bc --- /dev/null +++ b/scripts/lib/gitleaks_integrity.sh @@ -0,0 +1,94 @@ +#!/usr/bin/env bash + +gitleaks_path_is_absolute() { + case "$1" in + /*|[A-Za-z]:[\\/]*) return 0 ;; + *) return 1 ;; + esac +} + +gitleaks_sha256_file() { + local file="$1" + if command -v sha256sum >/dev/null 2>&1; then + sha256sum "$file" | awk '{print $1}' + elif command -v shasum >/dev/null 2>&1; then + shasum -a 256 "$file" | awk '{print $1}' + else + return 1 + fi +} + +gitleaks_manifest_hash() { + local manifest="$1" + local name="$2" + [ -f "$manifest" ] || return 1 + awk -v name="$name" '$2 == name {print $1}' "$manifest" +} + +gitleaks_hash_matches() { + local executable="$1" + local manifest="$2" + local name="$3" + local expected + local actual + + [ -f "$executable" ] || return 1 + expected="$(gitleaks_manifest_hash "$manifest" "$name")" + [ -n "$expected" ] || return 1 + actual="$(gitleaks_sha256_file "$executable")" || return 1 + [ "$actual" = "$expected" ] +} + +gitleaks_version_matches() { + local executable="$1" + local version_file="$2" + local expected + local actual + + [ -x "$executable" ] && [ -f "$version_file" ] || return 1 + expected="$(tr -d '[:space:]' < "$version_file")" + actual="$("$executable" version 2>/dev/null | tr -d '[:space:]')" || return 1 + [ -n "$expected" ] && [ "$actual" = "$expected" ] +} + +gitleaks_smoke_scan() { + local executable="$1" + local config="$2" + local tmp_dir + local report_file + local error_file + local config_dir + local compact_report + + [ -x "$executable" ] && [ -f "$config" ] || return 1 + tmp_dir="$(mktemp -d)" || return 1 + report_file="$tmp_dir/report.json" + error_file="$tmp_dir/error.log" + config_dir="$(CDPATH='' cd -- "$(dirname -- "$config")" && pwd -P)" || { + rm -rf "$tmp_dir" + return 1 + } + + if ! ( + cd "$config_dir" + printf '' | "$executable" \ + --config "$config" \ + --ignore-gitleaks-allow \ + --redact=100 \ + --exit-code=42 \ + --no-banner \ + --no-color \ + --log-level=error \ + --max-decode-depth=5 \ + --report-format=json \ + --report-path=- \ + stdin > "$report_file" 2> "$error_file" + ); then + rm -rf "$tmp_dir" + return 1 + fi + + compact_report="$(tr -d '[:space:]' < "$report_file")" + rm -rf "$tmp_dir" + [ "$compact_report" = '[]' ] +} diff --git a/tests/control_plane_test.sh b/tests/control_plane_test.sh index 4c5357b..0be230a 100755 --- a/tests/control_plane_test.sh +++ b/tests/control_plane_test.sh @@ -52,6 +52,26 @@ for impl in rust legacy; do fi done +for impl in rust legacy; do + output="$tmp_dir/$impl-scan-disabled.out" + error_output="$tmp_dir/$impl-scan-disabled.err" + ( + cd "$fixture" + PRE_COMMIT_REVIEW_HELPER_IMPL="$impl" PRE_COMMIT_REVIEW_DISABLE_FALLBACK=1 \ + PRE_COMMIT_REVIEW_SECRET_SCAN=off \ + "$helper" --source staged --control-plane + ) >"$output" 2>"$error_output" + python3 "$validator" --control-plane-output "$output" >/dev/null \ + || fail "$impl disabled-scan control plane did not remain valid" + if grep -Fq 'status: disabled' "$output"; then + fail "$impl appended optional scan metadata to machine-readable stdout" + fi + grep -Fq 'status: disabled' "$error_output" \ + || fail "$impl did not report disabled redaction on stderr" + grep -Fq 'review_continued: yes' "$error_output" \ + || fail "$impl did not report continued review on stderr" +done + python3 - "$tmp_dir/rust.out" "$tmp_dir/legacy.out" <<'PY' || fail 'Rust and legacy semantic control planes differ' import json import sys diff --git a/tests/gitleaks_distribution_test.sh b/tests/gitleaks_distribution_test.sh new file mode 100755 index 0000000..c35637b --- /dev/null +++ b/tests/gitleaks_distribution_test.sh @@ -0,0 +1,116 @@ +#!/usr/bin/env bash +set -euo pipefail + +script_dir="$(CDPATH='' cd -- "$(dirname -- "$0")" && pwd -P)" +repo_root="$(CDPATH='' cd -- "$script_dir/.." && pwd -P)" +version_file="$repo_root/scripts/gitleaks.version" +checksums_file="$repo_root/scripts/gitleaks-assets.sha256" +binary_checksums_file="$repo_root/scripts/gitleaks-binaries.sha256" +fetch_script="$repo_root/scripts/fetch_gitleaks.sh" + +fail() { + printf 'gitleaks distribution test failed: %s\n' "$*" >&2 + exit 1 +} + +version="$(tr -d '[:space:]' < "$version_file")" +[[ "$version" =~ ^[0-9]+\.[0-9]+\.[0-9]+$ ]] \ + || fail 'pinned version must be a semantic version' + +expected_assets=( + "gitleaks_${version}_darwin_arm64.tar.gz" + "gitleaks_${version}_darwin_x64.tar.gz" + "gitleaks_${version}_linux_x64.tar.gz" + "gitleaks_${version}_windows_x64.zip" +) +expected_binaries=( + 'gitleaks-darwin-arm64' + 'gitleaks-darwin-amd64' + 'gitleaks-linux-amd64' + 'gitleaks-windows-amd64.exe' +) +expected_collectors=( + 'collect_diff_context-darwin-arm64' + 'collect_diff_context-darwin-amd64' + 'collect_diff_context-linux-amd64' + 'collect_diff_context-windows-amd64.exe' +) + +[ "$(wc -l < "$checksums_file" | tr -d '[:space:]')" = "${#expected_assets[@]}" ] \ + || fail 'checksum manifest must contain exactly four platform assets' + +for asset in "${expected_assets[@]}"; do + line="$(awk -v asset="$asset" '$2 == asset {print}' "$checksums_file")" + [ -n "$line" ] || fail "missing checksum for $asset" + hash="${line%% *}" + [[ "$hash" =~ ^[0-9a-f]{64}$ ]] || fail "invalid SHA256 for $asset" +done + +[ "$(wc -l < "$binary_checksums_file" | tr -d '[:space:]')" = "${#expected_binaries[@]}" ] \ + || fail 'binary checksum manifest must contain exactly four platform executables' +for binary in "${expected_binaries[@]}"; do + line="$(awk -v binary="$binary" '$2 == binary {print}' "$binary_checksums_file")" + [ -n "$line" ] || fail "missing binary checksum for $binary" + hash="${line%% *}" + [[ "$hash" =~ ^[0-9a-f]{64}$ ]] || fail "invalid binary SHA256 for $binary" +done + +for collector in "${expected_collectors[@]}"; do + collector_path="$repo_root/scripts/bin/$collector" + [ -x "$collector_path" ] || fail "missing executable helper artifact: $collector" + if ! grep -aFq -- '--sanitize-stdin' "$collector_path" \ + && ! grep -aFq 'pcr-sanitizer-v1' "$collector_path"; then + fail "helper artifact does not contain a sanitizer capability marker: $collector" + fi +done + +grep -Fq 'https://github.com/gitleaks/gitleaks/releases/download/' "$fetch_script" \ + || fail 'fetch script must use official Gitleaks releases by default' +grep -Fq 'PRE_COMMIT_REVIEW_FETCH_PROGRESS=auto|always|never' "$fetch_script" \ + || fail 'fetch script help must document download progress controls' +grep -Fq -- '--progress-bar' "$fetch_script" \ + || fail 'curl downloads must expose interactive progress' +# shellcheck disable=SC2016 # Assert the literal checksum comparison in the script. +grep -Fq '[ "$actual" = "$expected" ]' "$fetch_script" \ + || fail 'fetch script must verify each pinned checksum before extraction' +# shellcheck disable=SC2016 # Assert literal variable use in the fetch script. +grep -Fq 'gitleaks_hash_matches "$extracted_binary"' "$fetch_script" \ + || fail 'fetch script must verify the extracted executable before installation' +[ -f "$repo_root/references/security/gitleaks.toml" ] \ + || fail 'trusted scanner configuration is missing' +[ -f "$repo_root/THIRD_PARTY_LICENSES/gitleaks-LICENSE" ] \ + || fail 'bundled scanner license is missing' +[ -x "$repo_root/scripts/check_gitleaks.sh" ] \ + || fail 'Gitleaks doctor is missing or not executable' +[ -f "$repo_root/scripts/lib/gitleaks_integrity.sh" ] \ + || fail 'shared Gitleaks integrity helpers are missing' +if grep -Fq 'command -v gitleaks' \ + "$repo_root/install.sh" "$repo_root/scripts/collect_diff_context.sh"; then + fail 'installer and runtime wrapper must not discover Gitleaks implicitly through PATH' +fi +if grep -Fq 'PathBuf::from("gitleaks")' \ + "$repo_root/collect-diff-context-cli/src/secret_scan.rs"; then + fail 'Rust scanner discovery must not fall back implicitly to PATH' +fi +grep -Fq 'review_continued: yes' "$repo_root/scripts/collect_diff_context.sh" \ + || fail 'runtime wrapper must report that review continues when redaction is unavailable' +if grep -Eq 'diff_release_allowed: no|secret_scan: blocked' \ + "$repo_root/scripts/collect_diff_context.sh"; then + fail 'optional secret scanning must not withhold review output' +fi +grep -Fq -- '--sanitize-stdin' "$repo_root/scripts/collect_diff_context.sh" \ + || fail 'runtime wrapper must sanitize captured streams through the Rust helper' +grep -Fq 'dist/pre-commit-review/scripts/check_gitleaks.sh' \ + "$repo_root/.github/workflows/release.yml" \ + || fail 'release package must run the Gitleaks doctor before archiving' + +invalid_progress_output="$(mktemp)" +trap 'rm -f "$invalid_progress_output"' EXIT +if PRE_COMMIT_REVIEW_FETCH_PROGRESS=invalid \ + "$fetch_script" --platform unsupported >"$invalid_progress_output" 2>&1; then + fail 'fetch script must reject an invalid progress mode' +fi +grep -Fq 'invalid PRE_COMMIT_REVIEW_FETCH_PROGRESS value' "$invalid_progress_output" \ + || fail 'invalid progress mode must have an actionable error' + +printf 'gitleaks distribution tests passed\n' diff --git a/tests/helper_shadow_mode_test.sh b/tests/helper_shadow_mode_test.sh old mode 100644 new mode 100755 index a31aaa3..692c1d8 --- a/tests/helper_shadow_mode_test.sh +++ b/tests/helper_shadow_mode_test.sh @@ -31,8 +31,11 @@ if [ "$os_name" = "windows" ]; then fi helper_root="$tmp_dir/helper" -mkdir -p "$helper_root/bin" +mkdir -p "$helper_root/bin" "$helper_root/lib" cp "$source_helper" "$helper_root/collect_diff_context.sh" +cp "$repo_root/scripts/gitleaks.version" "$helper_root/" +cp "$repo_root/scripts/gitleaks-binaries.sha256" "$helper_root/" +cp "$repo_root/scripts/lib/gitleaks_integrity.sh" "$helper_root/lib/" legacy_hit="$tmp_dir/legacy-hit" rust_hit="$tmp_dir/rust-hit" @@ -53,11 +56,33 @@ printf 'rust\n' >"$rust_hit" EOF chmod +x "$helper_root/bin/$binary_name" +fake_gitleaks="$tmp_dir/gitleaks" +cat >"$fake_gitleaks" <<'EOF' +#!/usr/bin/env bash +set -euo pipefail +if [ "${1:-}" = 'version' ]; then + printf '%s\n' '8.30.1' + exit 0 +fi +cat >/dev/null +printf '[]\n' +EOF +chmod +x "$fake_gitleaks" + +sanitizer_bin="$repo_root/collect-diff-context-cli/target/debug/collect-diff-context-cli" +if [ ! -x "$sanitizer_bin" ]; then + cargo build --manifest-path "$repo_root/collect-diff-context-cli/Cargo.toml" >/dev/null +fi + stdout_file="$tmp_dir/stdout.txt" stderr_file="$tmp_dir/stderr.txt" ( cd "$tmp_dir" PRE_COMMIT_REVIEW_HELPER_IMPL=legacy PRE_COMMIT_REVIEW_SHADOW_MODE=1 \ + PRE_COMMIT_REVIEW_RUST_BIN="$helper_root/bin/$binary_name" \ + PRE_COMMIT_REVIEW_SANITIZER_BIN="$sanitizer_bin" \ + PRE_COMMIT_REVIEW_GITLEAKS_BIN="$fake_gitleaks" \ + PRE_COMMIT_REVIEW_GITLEAKS_CONFIG="$repo_root/references/security/gitleaks.toml" \ "$helper_root/collect_diff_context.sh" ) >"$stdout_file" 2>"$stderr_file" @@ -76,6 +101,10 @@ explicit_log="$tmp_dir/shadow-diff.log" ( cd "$tmp_dir" PRE_COMMIT_REVIEW_HELPER_IMPL=legacy PRE_COMMIT_REVIEW_SHADOW_MODE=1 \ + PRE_COMMIT_REVIEW_RUST_BIN="$helper_root/bin/$binary_name" \ + PRE_COMMIT_REVIEW_SANITIZER_BIN="$sanitizer_bin" \ + PRE_COMMIT_REVIEW_GITLEAKS_BIN="$fake_gitleaks" \ + PRE_COMMIT_REVIEW_GITLEAKS_CONFIG="$repo_root/references/security/gitleaks.toml" \ PRE_COMMIT_REVIEW_SHADOW_DIFF_LOG="$explicit_log" \ "$helper_root/collect_diff_context.sh" ) >"$tmp_dir/stdout-with-log.txt" 2>"$tmp_dir/stderr-with-log.txt" diff --git a/tests/install_gitleaks_test.sh b/tests/install_gitleaks_test.sh new file mode 100755 index 0000000..b1de052 --- /dev/null +++ b/tests/install_gitleaks_test.sh @@ -0,0 +1,187 @@ +#!/usr/bin/env bash +set -euo pipefail + +script_dir="$(CDPATH='' cd -- "$(dirname -- "$0")" && pwd -P)" +repo_root="$(CDPATH='' cd -- "$script_dir/.." && pwd -P)" +tmp_dir="$(mktemp -d)" +trap 'rm -rf "$tmp_dir"' EXIT + +fail() { + printf 'install Gitleaks test failed: %s\n' "$*" >&2 + exit 1 +} + +case "$(uname -s | tr '[:upper:]' '[:lower:]')" in + darwin) os_name='darwin' ;; + linux) os_name='linux' ;; + *) printf 'install Gitleaks test skipped on this operating system\n'; exit 0 ;; +esac +case "$(uname -m)" in + arm64|aarch64) arch_name='arm64' ;; + x86_64|amd64) arch_name='amd64' ;; + *) printf 'install Gitleaks test skipped on this architecture\n'; exit 0 ;; +esac + +platform="${os_name}-${arch_name}" +binary_name="gitleaks-${platform}" +collector_name="collect_diff_context-${platform}" +version="$(tr -d '[:space:]' < "$repo_root/scripts/gitleaks.version")" +case "$platform" in + darwin-arm64) asset="gitleaks_${version}_darwin_arm64.tar.gz" ;; + darwin-amd64) asset="gitleaks_${version}_darwin_x64.tar.gz" ;; + linux-amd64) asset="gitleaks_${version}_linux_x64.tar.gz" ;; + *) printf 'install Gitleaks test skipped on %s\n' "$platform"; exit 0 ;; +esac + +fixture_root="$tmp_dir/source" +mkdir -p "$fixture_root" +cp "$repo_root/install.sh" "$repo_root/SKILL.md" "$repo_root/LICENSE" "$fixture_root/" +cp -R "$repo_root/agents" "$repo_root/references" "$repo_root/scripts" \ + "$repo_root/THIRD_PARTY_LICENSES" "$fixture_root/" +rm -f "$fixture_root/scripts/bin"/gitleaks-* + +asset_dir="$tmp_dir/assets" +payload_dir="$tmp_dir/payload" +mkdir -p "$asset_dir" "$payload_dir" +cat > "$payload_dir/gitleaks" </dev/null +printf '%s\n' '[]' +EOF +chmod +x "$payload_dir/gitleaks" +tar -czf "$asset_dir/$asset" -C "$payload_dir" gitleaks + +if command -v sha256sum >/dev/null 2>&1; then + asset_hash="$(sha256sum "$asset_dir/$asset" | awk '{print $1}')" +else + asset_hash="$(shasum -a 256 "$asset_dir/$asset" | awk '{print $1}')" +fi +awk -v asset="$asset" -v hash="$asset_hash" \ + 'BEGIN { OFS=" " } $2 == asset { $1=hash } { print $1, $2 }' \ + "$fixture_root/scripts/gitleaks-assets.sha256" \ + > "$tmp_dir/checksums" +mv "$tmp_dir/checksums" "$fixture_root/scripts/gitleaks-assets.sha256" + +if command -v sha256sum >/dev/null 2>&1; then + binary_hash="$(sha256sum "$payload_dir/gitleaks" | awk '{print $1}')" +else + binary_hash="$(shasum -a 256 "$payload_dir/gitleaks" | awk '{print $1}')" +fi +awk -v name="$binary_name" -v hash="$binary_hash" \ + 'BEGIN { OFS=" " } $2 == name { $1=hash } { print $1, $2 }' \ + "$fixture_root/scripts/gitleaks-binaries.sha256" \ + > "$tmp_dir/binary-checksums" +mv "$tmp_dir/binary-checksums" "$fixture_root/scripts/gitleaks-binaries.sha256" + +base_url="file://$asset_dir" + +dry_output="$tmp_dir/dry-run.out" +PRE_COMMIT_REVIEW_GITLEAKS_BASE_URL="$base_url" \ + "$fixture_root/install.sh" codex --dry-run --dir "$tmp_dir/dry-skills" \ + > "$dry_output" +grep -Fq "DRY RUN fetch pinned Gitleaks for $platform" "$dry_output" \ + || fail 'dry-run did not report the current-platform download' +[ ! -e "$tmp_dir/dry-skills/pre-commit-review" ] \ + || fail 'dry-run changed the filesystem' + +copy_output="$tmp_dir/copy.out" +copy_progress="$tmp_dir/copy.progress" +PRE_COMMIT_REVIEW_GITLEAKS_BASE_URL="$base_url" \ + PRE_COMMIT_REVIEW_FETCH_PROGRESS=always \ + "$fixture_root/install.sh" codex --dir "$tmp_dir/copy-skills" \ + > "$copy_output" 2> "$copy_progress" +installed_binary="$tmp_dir/copy-skills/pre-commit-review/scripts/bin/$binary_name" +installed_collector="$tmp_dir/copy-skills/pre-commit-review/scripts/bin/$collector_name" +[ -x "$installed_binary" ] || fail 'default copy install did not bundle Gitleaks' +[ -x "$installed_collector" ] || fail 'default copy install did not bundle the current helper' +[ "$("$installed_binary" version)" = "$version" ] \ + || fail 'default copy install bundled the wrong Gitleaks version' +[ ! -e "$fixture_root/scripts/bin/$binary_name" ] \ + || fail 'copy install should not modify the source tree' +grep -Fq "Gitleaks: installed pinned $binary_name" "$copy_output" \ + || fail 'copy install did not report the pinned scanner installation' +[ -s "$copy_progress" ] \ + || fail 'forced download progress did not write to stderr' +grep -Fq "Downloading $asset" "$copy_output" \ + || fail 'download stage was not reported' +grep -Fq "Verifying SHA256 for $asset" "$copy_output" \ + || fail 'checksum stage was not reported' +grep -Fq "Extracting $asset" "$copy_output" \ + || fail 'extraction stage was not reported' + +installed_report="$tmp_dir/installed-sanitizer.report" +PRE_COMMIT_REVIEW_SANITIZE_REPORT="$installed_report" \ + "$installed_collector" --sanitize-stdin "$tmp_dir/installed-sanitizer.out" +grep -Fq 'protocol: pcr-sanitizer-v1' "$installed_report" \ + || fail 'copy-installed helper does not support the sanitizer protocol' +grep -Fq 'status: clean' "$installed_report" \ + || fail 'copy-installed helper did not complete the bundled scanner handshake' + +offline_bin="$tmp_dir/offline-bin" +mkdir -p "$offline_bin" +cp "$payload_dir/gitleaks" "$offline_bin/gitleaks" + +PATH="$offline_bin:$PATH" \ + "$fixture_root/install.sh" codex --no-download --dir "$tmp_dir/missing-skills" \ + > "$tmp_dir/missing.out" 2> "$tmp_dir/missing.err" +[ -d "$tmp_dir/missing-skills/pre-commit-review" ] \ + || fail '--no-download did not install the skill without Gitleaks' +[ ! -e "$tmp_dir/missing-skills/pre-commit-review/scripts/bin/$binary_name" ] \ + || fail '--no-download unexpectedly bundled a scanner' +grep -Fq 'optional scanner unavailable (--no-download); review will continue without secret redaction' \ + "$tmp_dir/missing.out" \ + || fail '--no-download did not explain the optional redaction downgrade' + +PRE_COMMIT_REVIEW_GITLEAKS_BIN="$offline_bin/gitleaks" \ + "$fixture_root/install.sh" codex --no-download --dir "$tmp_dir/offline-skills" \ + > "$tmp_dir/offline.out" +[ ! -e "$tmp_dir/offline-skills/pre-commit-review/scripts/bin/$binary_name" ] \ + || fail '--no-download unexpectedly bundled a scanner' +grep -Fq 'explicitly trusted' "$tmp_dir/offline.out" \ + || fail '--no-download did not report its explicit scanner dependency' + +cat > "$fixture_root/scripts/bin/$binary_name" <<'EOF' +#!/usr/bin/env bash +printf '%s\n' '0.0.0' +EOF +chmod +x "$fixture_root/scripts/bin/$binary_name" + +link_output="$tmp_dir/link.out" +PRE_COMMIT_REVIEW_GITLEAKS_BASE_URL="$base_url" \ + "$fixture_root/install.sh" codex --link --dir "$tmp_dir/link-skills" \ + > "$link_output" +[ -x "$fixture_root/scripts/bin/$binary_name" ] \ + || fail 'link install did not bundle Gitleaks in the linked source tree' +[ "$("$fixture_root/scripts/bin/$binary_name" version)" = "$version" ] \ + || fail 'link install did not replace a stale bundled Gitleaks version' +[ -L "$tmp_dir/link-skills/pre-commit-review" ] \ + || fail 'link install did not create the skill symlink' +grep -Fq "Gitleaks: installed pinned $binary_name" "$link_output" \ + || fail 'link install did not report the pinned scanner installation' + +doctor_output="$tmp_dir/doctor.out" +"$fixture_root/install.sh" --doctor > "$doctor_output" +grep -Fq 'Gitleaks doctor: OK' "$doctor_output" \ + || fail 'doctor did not accept a verified bundled scanner' +grep -Fq 'integrity: sha256-verified' "$doctor_output" \ + || fail 'doctor did not report bundled scanner integrity' + +printf '\n# tampered\n' >> "$fixture_root/scripts/bin/$binary_name" +if "$fixture_root/install.sh" --doctor > "$tmp_dir/tampered.out" 2>&1; then + fail 'doctor accepted a tampered bundled scanner' +fi +grep -Fq 'reason: binary-integrity-mismatch' "$tmp_dir/tampered.out" \ + || fail 'doctor did not identify bundled scanner tampering' +grep -Fq 'Gitleaks doctor: UNAVAILABLE' "$tmp_dir/tampered.out" \ + || fail 'doctor did not distinguish scanner unavailability from review availability' +grep -Fq 'redaction_available: no' "$tmp_dir/tampered.out" \ + || fail 'doctor did not report that redaction is unavailable' +grep -Fq 'review_output_allowed: yes' "$tmp_dir/tampered.out" \ + || fail 'doctor incorrectly implied that review output is blocked' + +printf 'install Gitleaks tests passed\n' diff --git a/tests/install_smoke_test.sh b/tests/install_smoke_test.sh index 1ac560b..08ccfc5 100755 --- a/tests/install_smoke_test.sh +++ b/tests/install_smoke_test.sh @@ -6,10 +6,20 @@ repo_root="$(CDPATH='' cd -- "$script_dir/.." && pwd -P)" tmp_dir="$(mktemp -d)" trap 'rm -rf "$tmp_dir"' EXIT -"$repo_root/install.sh" codex --copy --dir "$tmp_dir/codex-skills" +run_offline_install() { + "$repo_root/install.sh" "$@" --no-download +} + +run_offline_install codex --copy --dir "$tmp_dir/codex-skills" [ -f "$tmp_dir/codex-skills/pre-commit-review/SKILL.md" ] [ -f "$tmp_dir/codex-skills/pre-commit-review/agents/openai.yaml" ] [ -f "$tmp_dir/codex-skills/pre-commit-review/scripts/collect_diff_context.sh" ] +[ -f "$tmp_dir/codex-skills/pre-commit-review/scripts/fetch_gitleaks.sh" ] +[ -f "$tmp_dir/codex-skills/pre-commit-review/scripts/gitleaks.version" ] +[ -f "$tmp_dir/codex-skills/pre-commit-review/scripts/gitleaks-assets.sha256" ] +[ -f "$tmp_dir/codex-skills/pre-commit-review/scripts/gitleaks-binaries.sha256" ] +[ -f "$tmp_dir/codex-skills/pre-commit-review/scripts/check_gitleaks.sh" ] +[ -f "$tmp_dir/codex-skills/pre-commit-review/scripts/lib/gitleaks_integrity.sh" ] [ ! -e "$tmp_dir/codex-skills/pre-commit-review/README.md" ] [ ! -e "$tmp_dir/codex-skills/pre-commit-review/README.zh-CN.md" ] [ ! -e "$tmp_dir/codex-skills/pre-commit-review/install.sh" ] @@ -25,8 +35,10 @@ trap 'rm -rf "$tmp_dir"' EXIT [ -f "$tmp_dir/codex-skills/pre-commit-review/references/examples/default-tiny-en.md" ] [ -f "$tmp_dir/codex-skills/pre-commit-review/references/examples/default-tiny-zh.md" ] [ -f "$tmp_dir/codex-skills/pre-commit-review/references/examples/complex-visual-and-coverage.md" ] +[ -f "$tmp_dir/codex-skills/pre-commit-review/references/security/gitleaks.toml" ] +[ -f "$tmp_dir/codex-skills/pre-commit-review/THIRD_PARTY_LICENSES/gitleaks-LICENSE" ] -"$repo_root/install.sh" codex --copy --dir "$tmp_dir/codex-skills" +run_offline_install codex --copy --dir "$tmp_dir/codex-skills" [ -d "$tmp_dir/codex-skills/pre-commit-review" ] AGENT_SKILLS_DIR="$tmp_dir/generic-agent-skills" CODEX_HOME="$tmp_dir/codex-home" "$repo_root/install.sh" codex --dry-run >"$tmp_dir/codex-env.out" @@ -35,19 +47,19 @@ grep -Fq "Target: $tmp_dir/generic-agent-skills/pre-commit-review" "$tmp_dir/cod CODEX_HOME="$tmp_dir/codex-home" "$repo_root/install.sh" codex --dry-run >"$tmp_dir/codex-home.out" grep -Fq "Target: $tmp_dir/codex-home/skills/pre-commit-review" "$tmp_dir/codex-home.out" -"$repo_root/install.sh" claude --link --dir "$tmp_dir/claude-skills" +run_offline_install claude --link --dir "$tmp_dir/claude-skills" [ -L "$tmp_dir/claude-skills/pre-commit-review" ] [ "$(CDPATH='' cd -- "$tmp_dir/claude-skills/pre-commit-review" && pwd -P)" = "$repo_root" ] "$repo_root/install.sh" gemini --dry-run --copy --dir "$tmp_dir/gemini-skills" [ ! -e "$tmp_dir/gemini-skills/pre-commit-review" ] -KIRO_SKILLS_DIR="$tmp_dir/kiro-skills" "$repo_root/install.sh" kiro --copy +KIRO_SKILLS_DIR="$tmp_dir/kiro-skills" run_offline_install kiro --copy [ -f "$tmp_dir/kiro-skills/pre-commit-review/SKILL.md" ] [ -f "$tmp_dir/kiro-skills/pre-commit-review/scripts/collect_diff_context.sh" ] [ -f "$tmp_dir/kiro-skills/pre-commit-review/references/examples/complex-visual-and-coverage.md" ] -"$repo_root/install.sh" kiro --link --dir "$tmp_dir/workspace/.kiro/skills" +run_offline_install kiro --link --dir "$tmp_dir/workspace/.kiro/skills" [ -L "$tmp_dir/workspace/.kiro/skills/pre-commit-review" ] [ "$(CDPATH='' cd -- "$tmp_dir/workspace/.kiro/skills/pre-commit-review" && pwd -P)" = "$repo_root" ] diff --git a/tests/lib/normalize_parity_output.py b/tests/lib/normalize_parity_output.py index eb8ce58..2595abf 100644 --- a/tests/lib/normalize_parity_output.py +++ b/tests/lib/normalize_parity_output.py @@ -3,6 +3,27 @@ import sys +def strip_secret_scan_sections(lines): + stripped = [] + index = 0 + while index < len(lines): + if lines[index].strip() != "## Secret Scan": + stripped.append(lines[index]) + index += 1 + continue + + index += 1 + while index < len(lines): + line = lines[index] + if line.strip() == "": + index += 1 + break + if line.lstrip().startswith("#"): + break + index += 1 + return stripped + + def normalize_json_buffer(json_buffer): text = "".join(json_buffer).strip() if not text: @@ -44,7 +65,7 @@ def get_sort_key(obj): def main(): - lines = sys.stdin.readlines() + lines = strip_secret_scan_sections(sys.stdin.readlines()) output = [] in_json = False json_buffer = [] diff --git a/tests/parity_assets_test.sh b/tests/parity_assets_test.sh old mode 100644 new mode 100755 index c6c9a34..a2a9bad --- a/tests/parity_assets_test.sh +++ b/tests/parity_assets_test.sh @@ -48,6 +48,26 @@ cat >"$sample_input" <<'EOF' ## Plain Section alpha +## Secret Scan +scanner: gitleaks +status: redacted +redactions: 1 +redaction_mode: full-regex-match +rule_id scan_input_start_line scan_input_end_line +gitlab-pat 4 4 +### Next Hunk +preserved + +## Secret Scan +scanner: gitleaks +status: unavailable +reason: scanner-unavailable +redaction_applied: no +review_continued: yes +redactions: 0 +redaction_mode: full-regex-match +### Final Hunk +also-preserved EOF @@ -65,6 +85,14 @@ first = text.index('"group_id": "module-a"') second = text.index('"group_id": "module-z"') if first > second: raise SystemExit("JSONL payload was not normalized into deterministic order") +if "## Secret Scan" in text or "gitlab-pat" in text: + raise SystemExit("Rust-only secret scan metadata was not removed") +if "### Next Hunk\npreserved" not in text: + raise SystemExit("content after secret scan metadata was removed") +if "review_continued: yes" in text: + raise SystemExit("optional scan downgrade metadata was not removed") +if "### Final Hunk\nalso-preserved" not in text: + raise SystemExit("content after optional scan downgrade metadata was removed") PY printf 'parity assets tests passed\n' diff --git a/tests/parity_golden_test.sh b/tests/parity_golden_test.sh index dd5aad7..ec62f28 100755 --- a/tests/parity_golden_test.sh +++ b/tests/parity_golden_test.sh @@ -59,7 +59,8 @@ compare_output() { "$RUST_BIN" > output_rust_raw.txt 2> stderr_rust.txt || true fi - # Normalize platform/absolute-path specifics to ensure exact golden comparisons + # Normalize platform/absolute-path specifics and omit Rust-only secret-scan + # metadata. Optional secret redaction behavior has separate regression tests. for prefix in output_legacy output_rust; do local raw_file="${prefix}_raw.txt" local norm_file="${prefix}.txt" diff --git a/tests/secret_gate_test.sh b/tests/secret_gate_test.sh new file mode 100755 index 0000000..00ff0ce --- /dev/null +++ b/tests/secret_gate_test.sh @@ -0,0 +1,522 @@ +#!/usr/bin/env bash +set -euo pipefail + +script_dir="$(CDPATH='' cd -- "$(dirname -- "$0")" && pwd -P)" +repo_root="$(CDPATH='' cd -- "$script_dir/.." && pwd -P)" +helper="$repo_root/scripts/collect_diff_context.sh" +rust_bin="${PRE_COMMIT_REVIEW_RUST_BIN:-$repo_root/collect-diff-context-cli/target/debug/collect-diff-context-cli}" +tmp_dir="$(mktemp -d)" +cleanup() { + if [ "${KEEP_SECRET_GATE_FIXTURE:-0}" = '1' ]; then + printf 'secret gate fixture retained at %s\n' "$tmp_dir" >&2 + else + rm -rf "$tmp_dir" + fi +} +trap cleanup EXIT + +fail() { + printf 'secret gate test failed: %s\n' "$*" >&2 + exit 1 +} + +if [ -z "${PRE_COMMIT_REVIEW_RUST_BIN:-}" ]; then + cargo build --manifest-path "$repo_root/collect-diff-context-cli/Cargo.toml" >/dev/null +fi +[ -x "$rust_bin" ] || fail "Rust helper binary is unavailable: $rust_bin" +export PRE_COMMIT_REVIEW_SANITIZER_BIN="$rust_bin" + +repo="$tmp_dir/repo" +mkdir -p "$repo" +git -C "$repo" init -q +git -C "$repo" config user.email 'test@example.com' +git -C "$repo" config user.name 'Secret Gate Test' + +secret="glpat-$(printf '%s%s' '1234567890' 'abcdefghij')" +printf 'old_token = "%s"\n' "$secret" > "$repo/config.py" +git -C "$repo" add config.py +git -C "$repo" commit -qm 'seed old credential fixture' + +printf 'old_token = os.environ["OLD_TOKEN"]\n' > "$repo/config.py" +printf 'new_token = "%s" # gitleaks:allow\n' "$secret" > "$repo/new_config.py" +cat > "$repo/.gitleaks.toml" <<'EOF' +title = "Untrusted repository config" +[extend] +useDefault = true +disabledRules = ["gitlab-pat"] +EOF +git -C "$repo" add config.py new_config.py .gitleaks.toml + +rust_stdout="$tmp_dir/rust.stdout" +rust_stderr="$tmp_dir/rust.stderr" +( + cd "$repo" + PRE_COMMIT_REVIEW_RUST_BIN="$rust_bin" \ + PRE_COMMIT_REVIEW_HELPER_IMPL=rust \ + PRE_COMMIT_REVIEW_DISABLE_FALLBACK=1 \ + "$helper" --source staged --include-diff always +) > "$rust_stdout" 2> "$rust_stderr" + +if grep -Fq "$secret" "$rust_stdout" "$rust_stderr"; then + fail 'Rust helper output contained the secret literal' +fi +grep -Fq '[redacted:gitlab-pat]' "$rust_stdout" \ + || { + grep -E '^(#|secret_scan:|scanner:|status:|redactions:|redaction_mode:|collect_diff_context:)' \ + "$rust_stdout" "$rust_stderr" >&2 || true + fail 'Rust helper did not redact Gitleaks match coordinates' + } +grep -Fq '## Secret Scan' "$rust_stdout" \ + || fail 'Rust helper did not emit the secret scan summary' +grep -Fq $'gitlab-pat\t' "$rust_stdout" \ + || fail 'secret scan summary did not include the rule id' +grep -Fq '+new_token = "[redacted:gitlab-pat]" # gitleaks:allow' "$rust_stdout" \ + || fail 'Gitleaks coordinates did not preserve text around the redacted match' + +raw_diff="$(git -C "$repo" diff --cached --no-ext-diff --no-textconv --)" +prefix_before_secret="${raw_diff%%"$secret"*}" +boundary_limit="$(( ${#prefix_before_secret} + 10 ))" +boundary_stdout="$tmp_dir/boundary.stdout" +boundary_stderr="$tmp_dir/boundary.stderr" +( + cd "$repo" + PRE_COMMIT_REVIEW_MAX_DIFF_BYTES="$boundary_limit" \ + PRE_COMMIT_REVIEW_RUST_BIN="$rust_bin" \ + PRE_COMMIT_REVIEW_HELPER_IMPL=rust \ + PRE_COMMIT_REVIEW_DISABLE_FALLBACK=1 \ + "$helper" --source staged --include-diff always +) > "$boundary_stdout" 2> "$boundary_stderr" + +if grep -Fq 'glpat-1234' "$boundary_stdout" "$boundary_stderr"; then + fail 'diff truncation released a partial secret before local redaction' +fi +grep -Fq 'status: redacted' "$boundary_stdout" \ + || fail 'full diff was not scanned before the sanitized view was truncated' + +legacy_stdout="$tmp_dir/legacy.stdout" +legacy_stderr="$tmp_dir/legacy.stderr" +( + cd "$repo" + PRE_COMMIT_REVIEW_HELPER_IMPL=legacy \ + "$helper" --source staged --include-diff always +) > "$legacy_stdout" 2> "$legacy_stderr" + +if grep -Fq "$secret" "$legacy_stdout" "$legacy_stderr"; then + fail 'wrapper final gate released a secret from the legacy helper' +fi +grep -Fq '[redacted:gitlab-pat]' "$legacy_stdout" \ + || fail 'legacy output was not redacted by the final egress sanitizer' +grep -Fq 'status: redacted' "$legacy_stdout" \ + || fail 'legacy egress sanitizer did not report redaction' + +failing_rust="$tmp_dir/failing-rust" +cat > "$failing_rust" <&2 +exit 1 +EOF +chmod +x "$failing_rust" + +fallback_stdout="$tmp_dir/fallback.stdout" +fallback_stderr="$tmp_dir/fallback.stderr" +( + cd "$repo" + PRE_COMMIT_REVIEW_RUST_BIN="$failing_rust" \ + PRE_COMMIT_REVIEW_HELPER_IMPL=rust \ + "$helper" --source staged --include-diff always +) > "$fallback_stdout" 2> "$fallback_stderr" + +if grep -Fq "$secret" "$fallback_stdout" "$fallback_stderr"; then + fail 'Rust failure or legacy fallback released a secret literal' +fi +grep -Fq '[redacted:gitlab-pat]' "$fallback_stdout" \ + || fail 'legacy fallback was not sanitized before release' +if grep -Fq 'secret_scan: blocked' "$fallback_stdout"; then + fail 'legacy fallback was blocked instead of sanitized' +fi + +leaky_stderr_rust="$tmp_dir/leaky-stderr-rust" +cat > "$leaky_stderr_rust" <&2 +EOF +chmod +x "$leaky_stderr_rust" + +stderr_gate_stdout="$tmp_dir/stderr-gate.stdout" +stderr_gate_stderr="$tmp_dir/stderr-gate.stderr" +( + cd "$repo" + PRE_COMMIT_REVIEW_RUST_BIN="$leaky_stderr_rust" \ + PRE_COMMIT_REVIEW_HELPER_IMPL=rust \ + PRE_COMMIT_REVIEW_DISABLE_FALLBACK=1 \ + "$helper" --source staged --control-plane +) > "$stderr_gate_stdout" 2> "$stderr_gate_stderr" + +if grep -Fq "$secret" "$stderr_gate_stdout" "$stderr_gate_stderr"; then + fail 'wrapper sanitizer released a secret from successful stderr' +fi +grep -Fq '[redacted:gitlab-pat]' "$stderr_gate_stderr" \ + || fail 'successful stderr was not sanitized before release' +grep -Fq 'status: redacted' "$stderr_gate_stderr" \ + || fail 'stderr sanitizer did not report redaction' + +case "$(uname -s | tr '[:upper:]' '[:lower:]')" in + darwin) os_name='darwin' ;; + linux) os_name='linux' ;; + msys*|mingw*|cygwin*) os_name='windows' ;; + *) fail 'unsupported operating system for bundled discovery test' ;; +esac +case "$(uname -m)" in + arm64|aarch64) arch_name='arm64' ;; + x86_64|amd64) arch_name='amd64' ;; + *) fail 'unsupported architecture for bundled discovery test' ;; +esac + +collector_name="collect_diff_context-${os_name}-${arch_name}" +scanner_name="gitleaks-${os_name}-${arch_name}" +if [ "$os_name" = 'windows' ]; then + collector_name="${collector_name}.exe" + scanner_name="${scanner_name}.exe" +fi + +scanner_source="$repo_root/scripts/bin/$scanner_name" +[ -x "$scanner_source" ] || fail 'no Gitleaks binary is available for bundled discovery test' + +runtime_root="$tmp_dir/runtime" +mkdir -p "$runtime_root/scripts/bin" "$runtime_root/references/security" +cp "$helper" "$runtime_root/scripts/collect_diff_context.sh" +cp "$rust_bin" "$runtime_root/scripts/bin/$collector_name" +cp "$scanner_source" "$runtime_root/scripts/bin/$scanner_name" +cp "$repo_root/scripts/gitleaks.version" \ + "$repo_root/scripts/gitleaks-binaries.sha256" \ + "$runtime_root/scripts/" +cp -R "$repo_root/scripts/lib" "$runtime_root/scripts/" +cp "$repo_root/references/security/gitleaks.toml" \ + "$runtime_root/references/security/gitleaks.toml" +chmod +x "$runtime_root/scripts/collect_diff_context.sh" \ + "$runtime_root/scripts/bin/$collector_name" \ + "$runtime_root/scripts/bin/$scanner_name" + +bundled_stdout="$tmp_dir/bundled.stdout" +bundled_stderr="$tmp_dir/bundled.stderr" +( + cd "$repo" + env -u PRE_COMMIT_REVIEW_RUST_BIN \ + -u PRE_COMMIT_REVIEW_SANITIZER_BIN \ + -u PRE_COMMIT_REVIEW_GITLEAKS_BIN \ + -u PRE_COMMIT_REVIEW_GITLEAKS_CONFIG \ + PATH=/usr/bin:/bin \ + PRE_COMMIT_REVIEW_HELPER_IMPL=rust \ + PRE_COMMIT_REVIEW_DISABLE_FALLBACK=1 \ + "$runtime_root/scripts/collect_diff_context.sh" \ + --source staged --include-diff always +) > "$bundled_stdout" 2> "$bundled_stderr" + +if grep -Fq "$secret" "$bundled_stdout" "$bundled_stderr"; then + fail 'self-contained runtime released a secret literal' +fi +grep -Fq '[redacted:gitlab-pat]' "$bundled_stdout" \ + || fail 'self-contained runtime did not discover its bundled scanner and config' + +printf '\n# tampered\n' >> "$runtime_root/scripts/bin/$scanner_name" +tampered_stdout="$tmp_dir/tampered.stdout" +( + cd "$repo" + env -u PRE_COMMIT_REVIEW_GITLEAKS_BIN \ + -u PRE_COMMIT_REVIEW_GITLEAKS_CONFIG \ + PATH=/usr/bin:/bin \ + PRE_COMMIT_REVIEW_HELPER_IMPL=rust \ + PRE_COMMIT_REVIEW_DISABLE_FALLBACK=1 \ + "$runtime_root/scripts/collect_diff_context.sh" \ + --source staged --include-diff always +) > "$tampered_stdout" 2> "$tmp_dir/tampered.stderr" +grep -Fq "$secret" "$tampered_stdout" \ + || fail 'tampered optional scanner prevented unredacted review from continuing' +grep -Fq 'status: unavailable' "$tampered_stdout" \ + || fail 'tampered optional scanner did not report unavailable redaction' +grep -Fq 'review_continued: yes' "$tampered_stdout" \ + || fail 'tampered optional scanner did not preserve review availability' + +rm -f "$runtime_root/scripts/bin/$scanner_name" +path_scanner_dir="$tmp_dir/path-scanner" +mkdir -p "$path_scanner_dir" +cat > "$path_scanner_dir/gitleaks" <<'EOF' +#!/usr/bin/env bash +printf '%s\n' 'PATH scanner must not run' >&2 +exit 99 +EOF +chmod +x "$path_scanner_dir/gitleaks" +path_stdout="$tmp_dir/path.stdout" +( + cd "$repo" + env -u PRE_COMMIT_REVIEW_GITLEAKS_BIN \ + -u PRE_COMMIT_REVIEW_GITLEAKS_CONFIG \ + -u PRE_COMMIT_REVIEW_SANITIZER_BIN \ + PATH="$path_scanner_dir:/usr/bin:/bin" \ + PRE_COMMIT_REVIEW_HELPER_IMPL=rust \ + PRE_COMMIT_REVIEW_DISABLE_FALLBACK=1 \ + "$runtime_root/scripts/collect_diff_context.sh" --source staged --include-diff always +) > "$path_stdout" 2> "$tmp_dir/path.stderr" +grep -Fq 'status: unavailable' "$path_stdout" \ + || fail 'implicit PATH scanner was not rejected' +grep -Fq "$secret" "$path_stdout" \ + || fail 'missing bundled scanner prevented review from continuing' +if grep -Fq 'PATH scanner must not run' "$path_stdout" "$tmp_dir/path.stderr"; then + fail 'implicit PATH scanner was executed' +fi + +missing_stdout="$tmp_dir/missing.stdout" +( + cd "$repo" + PRE_COMMIT_REVIEW_GITLEAKS_BIN="$tmp_dir/does-not-exist" \ + PRE_COMMIT_REVIEW_RUST_BIN="$rust_bin" \ + PRE_COMMIT_REVIEW_HELPER_IMPL=rust \ + PRE_COMMIT_REVIEW_DISABLE_FALLBACK=1 \ + "$helper" --source staged --include-diff always +) > "$missing_stdout" 2> "$tmp_dir/missing.stderr" + +grep -Fq 'status: unavailable' "$missing_stdout" \ + || fail 'missing optional scanner status was not reported' +grep -Fq 'review_continued: yes' "$missing_stdout" \ + || fail 'missing optional scanner did not preserve review availability' +grep -Fq "$secret" "$missing_stdout" \ + || fail 'missing optional scanner withheld review content' + +relative_scanner="$repo/relative-gitleaks" +cat > "$relative_scanner" <<'EOF' +#!/usr/bin/env bash +printf '%s\n' 'relative scanner must not run' >&2 +exit 99 +EOF +chmod +x "$relative_scanner" +relative_stdout="$tmp_dir/relative.stdout" +( + cd "$repo" + PRE_COMMIT_REVIEW_GITLEAKS_BIN='relative-gitleaks' \ + PRE_COMMIT_REVIEW_RUST_BIN="$rust_bin" \ + PRE_COMMIT_REVIEW_HELPER_IMPL=rust \ + PRE_COMMIT_REVIEW_DISABLE_FALLBACK=1 \ + "$helper" --source staged --include-diff always +) > "$relative_stdout" 2> "$tmp_dir/relative.stderr" +grep -Fq 'status: unavailable' "$relative_stdout" \ + || fail 'relative explicit scanner path did not degrade to unredacted review' +grep -Fq "$secret" "$relative_stdout" \ + || fail 'relative optional scanner path withheld review content' +if grep -Fq 'relative scanner must not run' "$relative_stdout" "$tmp_dir/relative.stderr"; then + fail 'relative explicit scanner was executed' +fi + +wrong_version_scanner="$tmp_dir/wrong-version-gitleaks" +wrong_version_scan_marker="$tmp_dir/wrong-version-scan-ran" +cat > "$wrong_version_scanner" < "$wrong_version_stdout" 2> "$tmp_dir/wrong-version.stderr" +grep -Fq 'status: unavailable' "$wrong_version_stdout" \ + || fail 'wrong scanner version did not degrade to unredacted review' +grep -Fq "$secret" "$wrong_version_stdout" \ + || fail 'wrong optional scanner version withheld review content' +[ ! -e "$wrong_version_scan_marker" ] \ + || fail 'wrong-version scanner received scan input' + +capability_scanner="$tmp_dir/no-stdin-json-gitleaks" +cat > "$capability_scanner" < "$capability_stdout" 2> "$tmp_dir/capability.stderr" +grep -Fq 'status: unavailable' "$capability_stdout" \ + || fail 'scanner without stdin/JSON capability did not degrade cleanly' +grep -Fq "$secret" "$capability_stdout" \ + || fail 'scanner without stdin/JSON capability withheld review content' + +invalid_location_scanner="$tmp_dir/invalid-location-gitleaks" +cat > "$invalid_location_scanner" < "$invalid_location_stdout" 2> "$tmp_dir/invalid-location.stderr" +grep -Fq 'status: redaction-failed' "$invalid_location_stdout" \ + || fail 'an identified finding with an invalid location was reported as scanner unavailable' +grep -Fq 'reason: redaction-location-invalid' "$invalid_location_stdout" \ + || fail 'the redaction implementation failure reason was not preserved' +grep -Fq 'findings_detected: yes' "$invalid_location_stdout" \ + || fail 'the redaction failure did not say that Gitleaks returned a finding' +grep -Fq "$secret" "$invalid_location_stdout" \ + || fail 'a redaction implementation failure withheld review content' + +timeout_scanner="$tmp_dir/timeout-gitleaks" +cat > "$timeout_scanner" < "$timeout_stdout" 2> "$tmp_dir/timeout.stderr" +grep -Fq 'reason: scanner-timeout' "$timeout_stdout" \ + || fail 'timed-out scanner did not report its specific downgrade reason' +grep -Fq 'review_continued: yes' "$timeout_stdout" \ + || fail 'timed-out scanner blocked review output' +grep -Fq "$secret" "$timeout_stdout" \ + || fail 'timed-out optional scanner withheld the original review content' + +capability_timeout_scanner="$tmp_dir/capability-timeout-gitleaks" +cat > "$capability_timeout_scanner" </dev/null +while :; do :; done +EOF +chmod +x "$capability_timeout_scanner" +capability_timeout_stdout="$tmp_dir/capability-timeout.stdout" +( + cd "$repo" + PRE_COMMIT_REVIEW_GITLEAKS_BIN="$capability_timeout_scanner" \ + PRE_COMMIT_REVIEW_GITLEAKS_TIMEOUT_MS=100 \ + PRE_COMMIT_REVIEW_RUST_BIN="$rust_bin" \ + PRE_COMMIT_REVIEW_HELPER_IMPL=rust \ + PRE_COMMIT_REVIEW_DISABLE_FALLBACK=1 \ + "$helper" --source staged --include-diff always +) > "$capability_timeout_stdout" 2> "$tmp_dir/capability-timeout.stderr" +grep -Fq 'reason: scanner-timeout' "$capability_timeout_stdout" \ + || fail 'capability-check timeout was collapsed into a generic capability failure' +grep -Fq 'review_continued: yes' "$capability_timeout_stdout" \ + || fail 'capability-check timeout blocked review output' + +many_hunks_repo="$tmp_dir/many-hunks-repo" +mkdir -p "$many_hunks_repo" +git -C "$many_hunks_repo" init -q +git -C "$many_hunks_repo" config user.email 'test@example.com' +git -C "$many_hunks_repo" config user.name 'Secret Gate Test' +for index in $(seq 1 600); do + printf 'line %04d original\n' "$index" +done > "$many_hunks_repo/many.txt" +git -C "$many_hunks_repo" add many.txt +git -C "$many_hunks_repo" commit -qm 'many hunk baseline' +for index in $(seq 1 600); do + if [ $((index % 20)) -eq 0 ]; then + printf 'line %04d changed\n' "$index" + else + printf 'line %04d original\n' "$index" + fi +done > "$many_hunks_repo/many.txt" +git -C "$many_hunks_repo" add many.txt + +counting_scanner="$tmp_dir/counting-gitleaks" +cat > "$counting_scanner" </dev/null +printf '%s\n' scan >> "\$FAKE_GITLEAKS_LOG" +printf '%s\n' '[]' +EOF +chmod +x "$counting_scanner" +scan_log="$tmp_dir/counting-scans.log" +( + cd "$many_hunks_repo" + FAKE_GITLEAKS_LOG="$scan_log" \ + PRE_COMMIT_REVIEW_HELPER_PATH="$helper" \ + PRE_COMMIT_REVIEW_GITLEAKS_BIN="$counting_scanner" \ + PRE_COMMIT_REVIEW_GITLEAKS_CONFIG="$repo_root/references/security/gitleaks.toml" \ + PRE_COMMIT_REVIEW_GROUP_TARGET_BYTES=80 \ + PRE_COMMIT_REVIEW_GROUP_HARD_BYTES=100 \ + "$rust_bin" --source staged --group module-many.txt +) > "$tmp_dir/many-hunks.stdout" 2> "$tmp_dir/many-hunks.stderr" +scan_calls="$(wc -l < "$scan_log" | tr -d '[:space:]')" +[ "$scan_calls" -le 3 ] \ + || fail "split preview spawned the scanner once per hunk: $scan_calls calls" + +disabled_stdout="$tmp_dir/disabled.stdout" +( + cd "$repo" + PRE_COMMIT_REVIEW_SECRET_SCAN=off \ + PRE_COMMIT_REVIEW_RUST_BIN="$rust_bin" \ + PRE_COMMIT_REVIEW_HELPER_IMPL=rust \ + PRE_COMMIT_REVIEW_DISABLE_FALLBACK=1 \ + "$helper" --source staged --include-diff always +) > "$disabled_stdout" 2> "$tmp_dir/disabled.stderr" +grep -Fq "$secret" "$disabled_stdout" \ + || fail 'disabled optional scanner withheld review content' +grep -Fq 'status: disabled' "$disabled_stdout" \ + || fail 'disabled optional scanner status was not reported' +grep -Fq 'review_continued: yes' "$disabled_stdout" \ + || fail 'disabled optional scanner did not preserve review availability' + +printf 'secret gate tests passed\n' diff --git a/tests/skill_contract_test.sh b/tests/skill_contract_test.sh old mode 100644 new mode 100755 index 9c9c86b..5c857e0 --- a/tests/skill_contract_test.sh +++ b/tests/skill_contract_test.sh @@ -95,8 +95,31 @@ grep -Fq 'Resolve this path relative to the skill package directory containing t || fail 'SKILL.md must disambiguate the helper path relative to the skill package' grep -Fq 'This is a mandatory gateway. Attempt the helper before any direct `git status`, `git diff`, `git diff --cached`, or branch comparison command.' "$skill_file" \ || fail 'SKILL.md must require the helper before direct Git inspection' -grep -Fq 'Only fall back to direct Git inspection when the helper is unavailable at that resolved path, exits non-zero without structured output, cannot be executed in the current host, or the user already provided the review material explicitly.' "$skill_file" \ - || fail 'SKILL.md must bound direct Git fallback to explicit helper-unavailable scenarios' +grep -Fq 'do not fall back to direct Git inspection when the helper is unavailable' "$skill_file" \ + || fail 'SKILL.md must preserve the authoritative helper snapshot gateway' +grep -Fq '### Optional Local Secret Redaction' "$skill_file" \ + || fail 'SKILL.md must define optional local secret redaction behavior' +grep -Fq 'Gitleaks is an optional, best-effort local redaction layer.' "$skill_file" \ + || fail 'SKILL.md must treat Gitleaks as an optional safety enhancement' +grep -Fq '`status: unavailable` with `redaction_applied: no` and `review_continued: yes`' "$skill_file" \ + || fail 'SKILL.md must continue review when local redaction is unavailable' +grep -Fq '`status: redaction-failed` with `findings_detected: yes`, `redaction_applied: no`, and `review_continued: yes`' "$skill_file" \ + || fail 'SKILL.md must distinguish a detected finding redaction bug from scanner unavailability' +if grep -Eq 'diff_release_allowed: no|secret_scan: blocked' "$skill_file"; then + fail 'SKILL.md must not turn optional redaction into a review-output gate' +fi +grep -Fq 'Never discover Gitleaks implicitly through `PATH`.' "$skill_file" \ + || fail 'SKILL.md must prohibit implicit scanner discovery' +grep -Fq 'version and SHA256 match the skill-owned manifests' "$skill_file" \ + || fail 'SKILL.md must require bundled scanner integrity validation' +grep -Fq '`scanner-timeout` is an unavailable-redaction state' "$skill_file" \ + || fail 'SKILL.md must keep scanner timeouts bounded and non-blocking' +grep -Fq 'a secret finding is a security signal, not a review-completion condition' "$skill_file" \ + || fail 'SKILL.md must prevent secret findings from short-circuiting review coverage' +grep -Fq 'never cap, merge away, or omit an independently actionable finding' "$skill_file" \ + || fail 'SKILL.md must preserve independent findings after a credential blocker' +grep -Fq 'never execute helper-emitted raw `review_command` values' "$skill_file" \ + || fail 'SKILL.md must prohibit raw review-command fallback' grep -Fq 'scripts/collect_diff_context.sh --control-plane' "$skill_file" \ || fail 'SKILL.md must use the bounded control-plane gateway' grep -Fq '`--expect-scope `' "$skill_file" \