diff --git a/.claude/skills/deep-code-review/scripts/review_checks.sh b/.claude/skills/deep-code-review/scripts/review_checks.sh index 0f4184277..3a67e5f64 100755 --- a/.claude/skills/deep-code-review/scripts/review_checks.sh +++ b/.claude/skills/deep-code-review/scripts/review_checks.sh @@ -1090,8 +1090,9 @@ while IFS=';' read -r hn hs hm hre _ hh; do fi continue fi + # solhint 6.x throws RangeError (exit 255, no output) on a file outside its cwd, so the canary runs from $td if [ "$hn" = solhint ]; then # canary: the config's `extends` value is unverified, so a config that loads no rules would read as clean; the throwaway tx.origin contract must raise avoid-tx-origin first - hubcmd solhint "$td/canary.sol"; printf 'pragma solidity ^0.8.0;\ncontract C { function f() public view returns (bool) { return tx.origin == msg.sender; } }\n' >"$td/canary.sol" + hubcmd solhint canary.sol; cmd=(bash -c 'cd "$1" && shift && exec "$@"' _ "$td" "${cmd[@]}"); printf 'pragma solidity ^0.8.0;\ncontract C { function f() public view returns (bool) { return tx.origin == msg.sender; } }\n' >"$td/canary.sol" run_one solhint "${cmd[@]}" || continue grep -qE '"ruleId"[[:space:]]*:[[:space:]]*"avoid-tx-origin"' "$out" || { skip solhint "canary: the throwaway config raised no avoid-tx-origin row, so its rules may not be loaded"; continue; } fi diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ff5b2e601..aac2645d9 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -515,7 +515,7 @@ jobs: - name: Allowlist guard run: REQUIRE_GITLEAKS=1 bash scripts/test_gitleaks_allowlist.sh - # The review_checks.sh stub tests never run a real scanner. This job installs the real tfsec, ansible-lint, rubocop and cfn-lint + # The review_checks.sh stub tests never run a real scanner. This job installs the real tfsec, ansible-lint, rubocop, cfn-lint, pmd, solhint, hadolint, bandit, ruff and actionlint # (pinned; no advisory-DB download) and checks each is RAN with its rule id on one canary. bundler-audit, kubescape and retire need a downloaded DB, so they stay stub-only. real-hub: runs-on: ubuntu-latest @@ -546,6 +546,39 @@ jobs: gem install --no-document --install-dir "$RUNNER_TEMP/gems" --bindir "$RUNNER_TEMP/gems/bin" rubocop -v 1.69.2 echo "GEM_HOME=$RUNNER_TEMP/gems" >> "$GITHUB_ENV" echo "$RUNNER_TEMP/gems/bin" >> "$GITHUB_PATH" + - name: Install bandit 1.9.4 and ruff 0.17.5 (pinned, venv) + run: | + set -euo pipefail + python3 -m venv "$RUNNER_TEMP/venv2" + "$RUNNER_TEMP/venv2/bin/pip" install --quiet bandit==1.9.4 ruff==0.17.5 + echo "$RUNNER_TEMP/venv2/bin" >> "$GITHUB_PATH" + - name: Install solhint 6.2.5 (pinned npm) + run: | + set -euo pipefail + npm install --silent --no-audit --no-fund --prefix "$RUNNER_TEMP/npm" solhint@6.2.5 + echo "$RUNNER_TEMP/npm/node_modules/.bin" >> "$GITHUB_PATH" + - name: Install hadolint 2.15.1 (pinned sha256) + run: | + set -euo pipefail + mkdir -p "$RUNNER_TEMP/bin" + curl --proto '=https' --tlsv1.2 -fsSL -o "$RUNNER_TEMP/bin/hadolint" https://github.com/hadolint/hadolint/releases/download/v2.15.1/hadolint-linux-x86_64 + echo "c7187db94eeeeca956519a6af171adc31453941a1e777961f6e680f697c8c507 $RUNNER_TEMP/bin/hadolint" | sha256sum -c - + chmod +x "$RUNNER_TEMP/bin/hadolint" + - name: Install actionlint 1.7.12 (pinned sha256) + run: | + set -euo pipefail + cd "$RUNNER_TEMP" + curl --proto '=https' --tlsv1.2 -fsSL -O https://github.com/rhysd/actionlint/releases/download/v1.7.12/actionlint_1.7.12_linux_amd64.tar.gz + echo "8aca8db96f1b94770f1b0d72b6dddcb1ebb8123cb3712530b08cc387b349a3d8 actionlint_1.7.12_linux_amd64.tar.gz" | sha256sum -c - + tar -xzf actionlint_1.7.12_linux_amd64.tar.gz -C "$RUNNER_TEMP/bin" actionlint + - name: Install PMD 7.29.0 (pinned sha256; the runner's preinstalled Java) + run: | + set -euo pipefail + cd "$RUNNER_TEMP" + curl --proto '=https' --tlsv1.2 -fsSL -O https://github.com/pmd/pmd/releases/download/pmd_releases%2F7.29.0/pmd-dist-7.29.0-bin.zip + echo "d27177c185b837fa57f097e482d1253468aa035379c5eb2671fa0a279109b179 pmd-dist-7.29.0-bin.zip" | sha256sum -c - + unzip -q pmd-dist-7.29.0-bin.zip + echo "$RUNNER_TEMP/pmd-bin-7.29.0/bin" >> "$GITHUB_PATH" - name: Real-tool canaries run: REQUIRE_REAL_HUB=1 bash scripts/test-review-checks-real-hub.sh diff --git a/changelog.d/hub-real-smoke-2.md b/changelog.d/hub-real-smoke-2.md new file mode 100644 index 000000000..e6df88ce0 --- /dev/null +++ b/changelog.d/hub-real-smoke-2.md @@ -0,0 +1,2 @@ +### Changed +- The real-hub CI job now also installs pinned, checksum-verified real pmd, solhint, hadolint, bandit, ruff and actionlint and asserts each is reported as ran with its expected rule id on a fictional canary file, extending the tfsec, ansible-lint, rubocop and cfn-lint smoke so a changed flag or report shape in these six scanners fails CI instead of passing on stubs alone. diff --git a/changelog.d/solhint-canary-cwd.md b/changelog.d/solhint-canary-cwd.md new file mode 100644 index 000000000..e7d847cd5 --- /dev/null +++ b/changelog.d/solhint-canary-cwd.md @@ -0,0 +1,3 @@ +### Fixed +- The review hub's solhint canary now runs from its throwaway directory. solhint 6.x aborts with a RangeError (exit 255, no output) on a file outside its working directory, so the canary never raised avoid-tx-origin and solhint was reported as not run. The fail-closed guard is unchanged. +- test-review-checks-real-hub.sh no longer prints "Broken pipe" when it shows a tool's raw output after a failure. diff --git a/scripts/test-review-checks-hub-pmd-solhint.sh b/scripts/test-review-checks-hub-pmd-solhint.sh index 74b5ba9ee..7cde3c669 100755 --- a/scripts/test-review-checks-hub-pmd-solhint.sh +++ b/scripts/test-review-checks-hub-pmd-solhint.sh @@ -31,7 +31,7 @@ echo "solhint $*" >>"$HUB_LOG" # without --config the real tool would read the project's .solhint.json (plugins = code): the stub flags that as a finding if [ "$1" != --config ] && [ -f .solhint.json ]; then printf '{"runs":[{"results":[{"ruleId":"plugin-loaded","message":{"text":"m"},"locations":[{"physicalLocation":{"artifactLocation":{"uri":"contracts/T.sol"},"region":{"startLine":1}}}]}]}]}'; exit 1; fi # the canary scan (a throwaway contract in the review temp dir): a rule row, or none when the config loads no rules -case "${*: -1}" in */canary.sol) if [ "${CANARY:-ok}" = ok ]; then printf '{"runs":[{"results":[{"ruleId":"avoid-tx-origin","message":{"text":"m"},"locations":[{"physicalLocation":{"artifactLocation":{"uri":"canary.sol"},"region":{"startLine":2}}}]}]}]}'; else printf '{"runs":[{"results":[]}]}'; fi; exit 1 ;; esac +case "${*: -1}" in canary.sol) if [ "${CANARY:-ok}" = ok ]; then printf '{"runs":[{"results":[{"ruleId":"avoid-tx-origin","message":{"text":"m"},"locations":[{"physicalLocation":{"artifactLocation":{"uri":"canary.sol"},"region":{"startLine":2}}}]}]}]}'; else printf '{"runs":[{"results":[]}]}'; fi; exit 1 ;; esac printf '%s' "$J_sol" exit "${SOL_RC:-1}" S diff --git a/scripts/test-review-checks-real-hub.sh b/scripts/test-review-checks-real-hub.sh index bca5c292e..f5a44bf0d 100755 --- a/scripts/test-review-checks-real-hub.sh +++ b/scripts/test-review-checks-real-hub.sh @@ -1,5 +1,5 @@ #!/usr/bin/env bash -# Real-tool smoke for the review_checks.sh hub: runs the REAL tfsec, ansible-lint, rubocop and cfn-lint (no stubs) on one tiny +# Real-tool smoke for the review_checks.sh hub: runs the REAL tfsec, ansible-lint, rubocop, cfn-lint, pmd, solhint, hadolint, bandit, ruff (ruff-security) and actionlint (no stubs) on one tiny # known-bad canary each, in a throwaway repo, and asserts each is reported as RAN (stderr "analyzers ran:", no "not run:" line) # with its expected rule id in the JSON rows. The stub tests (scripts/test-review-checks-hub-*.sh) pin routing and failure # handling; this pins the real flags, SARIF/JSON shapes and exit codes. Not covered: bundler-audit, kubescape and retire need a @@ -10,8 +10,9 @@ set -euo pipefail ROOT=$(cd "$(dirname "$0")/.." && pwd) RC=${REVIEW_CHECKS:-$ROOT/.claude/skills/deep-code-review/scripts/review_checks.sh} fail=0; tools= -for t in tfsec ansible-lint rubocop cfn-lint; do - if command -v "$t" >/dev/null 2>&1; then tools+=" $t" +bin() { [ "$1" = ruff-security ] && echo ruff || echo "$1"; } # the hub tool name ruff-security runs the ruff binary +for t in tfsec ansible-lint rubocop cfn-lint pmd solhint hadolint bandit ruff-security actionlint; do + if command -v "$(bin "$t")" >/dev/null 2>&1; then tools+=" $t" elif [ "${REQUIRE_REAL_HUB:-}" = 1 ]; then echo "FAIL - $t not installed (REQUIRE_REAL_HUB=1)"; fail=1 else echo "skip - $t not installed"; fi done @@ -21,21 +22,31 @@ trap 'cd /; rm -r -- "$T"' EXIT mkdir "$T/repo" && cd "$T/repo" || exit 1 G() { git -c user.email=t@example.com -c user.name=t "$@"; } G init -q . && printf 'x\n' >README.md && G add . && G commit -q -m base && G branch base -mkdir -p infra playbooks app +mkdir -p infra playbooks app docker .github/workflows contracts printf 'resource "aws_s3_bucket" "acme" {\n bucket = "acme-canary"\n}\n' >infra/main.tf printf -- '---\n- hosts: all\n tasks:\n - ansible.builtin.shell: echo hello\n' >playbooks/site.yml printf 'eval(params)\n' >app/canary.rb # Security/Eval skips a plain string literal argument (cop source: pattern $!str), so the argument must not be one printf 'AWSTemplateFormatVersion: "2010-09-09"\nResources:\n Acme:\n Type: AWS::S3::Bucket\n Properties:\n NoSuchProperty: 1\n' >infra/stack.yaml +printf 'public class Canary {\n void f() throws Exception {\n javax.crypto.spec.SecretKeySpec k = new javax.crypto.spec.SecretKeySpec("0123456789abcdef".getBytes(), "AES");\n }\n}\n' >app/Canary.java +printf 'pragma solidity ^0.8.0;\ncontract Canary { function f() public view returns (bool) { return tx.origin == msg.sender; } }\n' >contracts/Canary.sol +printf 'FROM ubuntu:latest\n' >docker/Dockerfile +printf 'import subprocess\n\nsubprocess.call("ls", shell=True)\n' >app/canary.py +printf 'on: push\njobs:\n acme:\n steps:\n - run: echo hello\n' >.github/workflows/canary.yml # no runs-on: actionlint syntax-check G add . && G commit -q -m canaries rc=0; bash "$RC" --base base >"$T/out" 2>"$T/err" || rc=$? echo "review_checks exit $rc; stderr tail:"; tail -n 15 "$T/err" # tool -> rule id regex (rubocop and ansible-lint ids are exact; tfsec and cfn-lint match the rule family, unpinned ids) -want() { case $1 in tfsec) echo '^aws-s3-' ;; ansible-lint) echo '^no-changed-when' ;; rubocop) echo '^Security/Eval$' ;; cfn-lint) echo '^E3' ;; esac; } +want() { case $1 in tfsec) echo '^aws-s3-' ;; ansible-lint) echo '^no-changed-when' ;; rubocop) echo '^Security/Eval$' ;; cfn-lint) echo '^E3' ;; + pmd) echo '^(HardCodedCryptoKey|InsecureCryptoIv)$' ;; solhint) echo '^avoid-tx-origin$' ;; hadolint) echo '^DL3007$' ;; bandit) echo '^B602$' ;; ruff-security) echo '^S602$' ;; actionlint) echo '^syntax-check$' ;; esac; } # tool -> its canary path (a row must be on this file, not just carry the rule id) -cany() { case $1 in tfsec) echo infra/main.tf ;; ansible-lint) echo playbooks/site.yml ;; rubocop) echo app/canary.rb ;; cfn-lint) echo infra/stack.yaml ;; esac; } +cany() { case $1 in tfsec) echo infra/main.tf ;; ansible-lint) echo playbooks/site.yml ;; rubocop) echo app/canary.rb ;; cfn-lint) echo infra/stack.yaml ;; + pmd) echo app/Canary.java ;; solhint) echo contracts/Canary.sol ;; hadolint) echo docker/Dockerfile ;; bandit|ruff-security) echo app/canary.py ;; actionlint) echo .github/workflows/canary.yml ;; esac; } # the hub's own capture is private to its run: on a failure, rerun the tool's hub argv here and show its raw output head raw() { case $1 in tfsec) (cd infra && tfsec . --format sarif --no-colour --soft-fail --no-module-downloads) ;; ansible-lint) ansible-lint --offline --nocolor -q -f sarif ./playbooks/site.yml ;; - rubocop) rubocop --force-default-config --fail-level fatal --format json --cache false -- ./app/canary.rb ;; cfn-lint) cfn-lint --format json -- ./infra/stack.yaml ;; esac; } + rubocop) rubocop --force-default-config --fail-level fatal --format json --cache false -- ./app/canary.rb ;; cfn-lint) cfn-lint --format json -- ./infra/stack.yaml ;; + pmd) pmd check --no-cache --no-progress -R category/java/security.xml -f sarif -d ./app/Canary.java ;; solhint) printf '{"extends":"solhint:recommended"}\n' >"$T/solhint.json"; solhint --config "$T/solhint.json" -f sarif --noPoster --disc ./contracts/Canary.sol ;; + hadolint) hadolint -f json --no-fail ./docker/Dockerfile ;; bandit) bandit -q -f json --exit-zero ./app/canary.py ;; ruff-security) ruff check --select S --output-format json --no-cache --exit-zero ./app/canary.py ;; + actionlint) actionlint -format '{{json .}}' ./.github/workflows/canary.yml ;; esac; } for t in $tools; do re=$(want "$t"); cf=$(cany "$t") if grep -qE "analyzers ran:.*\b$t\b" "$T/err" && ! grep -q "not run: $t" "$T/err" && ! grep -q "NOT CHECKED: $t" "$T/err" \ @@ -43,7 +54,7 @@ for t in $tools; do echo "ok - $t: RAN, rule matching $re reported" else echo "FAIL - $t: not RAN or no rule matching $re; its rows: $(jq -c --arg t "$t" '[.[]|select(.tool==$t)|{file,rule}]' "$T/out" 2>&1 | head -c 400)"; fail=1 - echo "--- raw $t output (first 40 lines, exit via rerun) ---"; { raw "$t" 2>&1 || echo "[exit $?]"; } | head -n 40 || true; echo "--- end raw $t ---" + echo "--- raw $t output (first 40 lines, exit via rerun) ---"; { raw "$t" 2>&1 || echo "[exit $?]"; } >"$T/raw" || true; head -n 40 "$T/raw"; echo "--- end raw $t ---" fi done [ "$fail" = 0 ] && echo "PASS: test-review-checks-real-hub"