From 7017b7edff3e420ecb5ae415c5dfa5f3da9fb72f Mon Sep 17 00:00:00 2001 From: Remigiusz Antczak Date: Sun, 15 Nov 2026 04:46:15 +0100 Subject: [PATCH 1/5] Tool hub: verify snyk-agent-scan, keep it route-only with a never-run test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Upstream (PyPI and GitHub, fetched 2026-11-15): Apache-2.0, 0.6.11, requires SNYK_TOKEN, sends component information to the Agent Scan API, no offline mode documented, and can start stdio MCP servers. A key and the network are required, so the registry row stays route-only. tool-hub.md now states the verified reason, standards-index.md logs the sources, and a new offline test asserts the tool is never invoked and emits no finding rows even with SNYK_TOKEN set and uvx on PATH. No-Mechanism-Reason: the new scripts/test-review-checks-hub-snyk-agent-scan.sh is the mechanism for the tool-hub.md wording change. size-budget-raise: .claude/skills/deep-code-review/references/tool-hub.md 59805→59884 snyk-agent-scan row states the verified key and network requirement Tests: 111/111 Co-Authored-By: Claude Opus 5.5 --- .../deep-code-review/references/tool-hub.md | 2 +- changelog.d/hub-snyk-agent-scan.md | 5 ++++ docs/standards-index.md | 1 + .../review-checks-hub-snyk-agent-scan.sh | 2 ++ .../test-review-checks-hub-snyk-agent-scan.sh | 27 +++++++++++++++++++ 5 files changed, 36 insertions(+), 1 deletion(-) create mode 100644 changelog.d/hub-snyk-agent-scan.md create mode 100644 scripts/ci-tests.d/review-checks-hub-snyk-agent-scan.sh create mode 100755 scripts/test-review-checks-hub-snyk-agent-scan.sh diff --git a/.claude/skills/deep-code-review/references/tool-hub.md b/.claude/skills/deep-code-review/references/tool-hub.md index 89aeb0f28..f5d8d2ecc 100644 --- a/.claude/skills/deep-code-review/references/tool-hub.md +++ b/.claude/skills/deep-code-review/references/tool-hub.md @@ -77,7 +77,7 @@ The agent sandbox denies installs and most network, so the owner provisions scan | `detekt` | Not routed: Kotlin static analysis, Apache-2.0 (https://github.com/detekt/detekt LICENSE, on the OSI list at https://opensource.org/license/apache-2-0, fetched 2026-11-12). The CLI docs (https://detekt.dev/docs/gettingstarted/cli, fetched 2026-11-12) give `--input`, `--report sarif:PATH` (the XML-like format is `checkstyle`) and exits 0 clean, 1 unexpected error, 2 issues found, 3 invalid config. `Main.kt` and `CliArgs.kt` hold no HTTP, update or telemetry code, but the analysis core, the Kotlin compiler and plugin loading were not read, so its network-free behavior is `unverified` (not checked = not routed); it is also a code-quality linter whose default rule sets are not security rules | not run | not routed | not routed | | `security-code-scan` | Not routed: a Roslyn analyzer (NuGet package or Visual Studio extension, LGPL-3.0 per the repository page, https://github.com/security-code-scan/security-code-scan, fetched 2026-11-12) that runs inside a project build; the stand-alone runner was not verified here and a build needs a NuGet restore (network). It cannot scan changed files only, so it is not routed; `devskim` is the routed .NET source scanner | not run | not routed | not routed | | `bearer` | Not routed: Bearer CLI is under the Elastic License 2.0, which forbids offering the software to third parties as a hosted or managed service (https://github.com/Bearer/bearer, fetched 2026-11-12), and that license is not on the OSI approved-licenses list (https://opensource.org/licenses, fetched 2026-11-12), so it is not routed | not run | not routed | not routed | -| `snyk-agent-scan` | MCP and skill configs | not run | unverified: docs show no offline mode (it calls the Agent Scan API and can start MCP servers) | route-only | +| `snyk-agent-scan` | MCP and skill configs | not run | needs a `SNYK_TOKEN` and calls the Agent Scan API (no offline mode documented; it can also start MCP servers): never run by the hub (checked 2026-11-15, Apache-2.0, 0.6.11) | route-only | | `opengrep` | SAST fallback: stands in for `semgrep` when `semgrep` is absent and `opengrep` is installed; same rules dir (repo config, else the bundled rules), never the registry pack; rows keep tool `semgrep` and the closing line says `opengrep` ran | `opengrep scan --config DIR --json --quiet -- FILES` (env `SEMGREP_SEND_METRICS=off`) | unverified (its README shows `scan` and `-f` only; `--config` and `--json` are inherited from semgrep, not confirmed); a crash is `not run`, never clean | route-only, local | | `scorecard` | OpenSSF Scorecard for the `origin` GitHub repo; checks scoring 0-4 become rows (rule `scorecard-`, file `.`); -1 (inconclusive) is ignored; a report with no checks, or only -1 checks, is `not run` (`not-checked` row), never a clean score. Opt-in: `REVIEW_SCORECARD=1` plus `GITHUB_TOKEN`; no token is `not run` | `scorecard --repo=github.com/OWNER/REPO --format=json` (token passed as `GITHUB_AUTH_TOKEN`) | verified (`scorecard --help`, 2026-11-12) | route-only, opt-in, network | | `nuclei` | Conservative, not passive: sends real requests to one running app. Opt-in `REVIEW_DAST_URL`, exact localhost origin only unless `REVIEW_DAST_ALLOW_REMOTE=1`. Allowlist: info and low http templates tagged `tech` or `exposure`, minus `default-login`, `xss`, `ssrf`, `oast`, `fuzz`, `fuzzing`, `takeover`, `cve`, `intrusive`, `dos`, `bruteforce`, `rce`, `sqli`. A failed `curl` probe of the target, or zero templates listed by `nuclei -tl`, is `NOT CHECKED` (an empty scan never reads as clean) | `nuclei -u URL -jsonl -silent -nc -ni -duc -rl 10 -s info,low -type http -tags tech,exposure -etags ...` | flags verified (`nuclei -h`, 2026-11-12); the tag choice is a conservative judgment, not a nuclei-certified safe set | route-only, opt-in, network | diff --git a/changelog.d/hub-snyk-agent-scan.md b/changelog.d/hub-snyk-agent-scan.md new file mode 100644 index 000000000..4df0f888c --- /dev/null +++ b/changelog.d/hub-snyk-agent-scan.md @@ -0,0 +1,5 @@ +Tool hub: verify snyk-agent-scan and keep it route-only + +Upstream (PyPI and GitHub, fetched 2026-11-15) shows Apache-2.0, version 0.6.11, a required SNYK_TOKEN and calls to the Agent Scan API, with no documented offline mode. The hub therefore never runs it; tool-hub.md states the verified reason and a new offline test asserts it is not invoked and emits no finding rows even with SNYK_TOKEN set. + +size-budget-raise: .claude/skills/deep-code-review/references/tool-hub.md 59805→59884 snyk-agent-scan row states the verified key and network requirement diff --git a/docs/standards-index.md b/docs/standards-index.md index d300460f2..a7027a5a5 100644 --- a/docs/standards-index.md +++ b/docs/standards-index.md @@ -2016,3 +2016,4 @@ Verification date: **2026-11-05**. Raw-byte sha256 of each fetch and the item id | cfn-lint — repository page and JSON formatter | https://github.com/aws-cloudformation/cfn-lint | Fetched 2026-11-13 (src/cfnlint/formatters/json.py via raw.githubusercontent.com): exit-code bitmask 0/2/4/6/8/10/12/14, `--format json`, per-match keys; MIT-0. | | rubocop — CLI reference, formatters and Security cops | https://docs.rubocop.org/rubocop/latest/usage/cli_reference.html | Fetched 2026-11-13 (also /latest/formatters.html and /latest/cops_security.html): exit codes 0/1/2, `--only`, JSON keys and severity levels, Security cop names; `--force-default-config` was not on the page (unverified). | | terrascan — repository page | https://github.com/tenable/terrascan | Fetched 2026-11-13: archived Nov 20, 2025, no longer maintained; first scan downloads policies; exit codes 0/1/3/4/5. | +| snyk-agent-scan — PyPI page and GitHub README | https://pypi.org/project/snyk-agent-scan/ | Fetched 2026-11-15 (also https://github.com/snyk/agent-scan): Apache-2.0; 0.6.11 (uploaded 2026-11-06); `uvx snyk-agent-scan@latest [path]`, `--json` (v0.6+ `scan_path_responses`); requires `SNYK_TOKEN`, sends component info to the Agent Scan API, no offline mode documented, can run stdio MCP servers. Needs a key: kept route-only, never run. | diff --git a/scripts/ci-tests.d/review-checks-hub-snyk-agent-scan.sh b/scripts/ci-tests.d/review-checks-hub-snyk-agent-scan.sh new file mode 100644 index 000000000..039d2e68b --- /dev/null +++ b/scripts/ci-tests.d/review-checks-hub-snyk-agent-scan.sh @@ -0,0 +1,2 @@ +# review_checks.sh hub routing for snyk-agent-scan: route-only, never run (needs SNYK_TOKEN); stub binaries. +bash scripts/test-review-checks-hub-snyk-agent-scan.sh diff --git a/scripts/test-review-checks-hub-snyk-agent-scan.sh b/scripts/test-review-checks-hub-snyk-agent-scan.sh new file mode 100755 index 000000000..199c69506 --- /dev/null +++ b/scripts/test-review-checks-hub-snyk-agent-scan.sh @@ -0,0 +1,27 @@ +#!/usr/bin/env bash +# snyk-agent-scan (verified 2026-11-15: Apache-2.0, 0.6.11, needs SNYK_TOKEN and calls the Agent Scan API, can start MCP servers) +# stays route-only: even with a changed .mcp.json, uvx and snyk-agent-scan on PATH, and SNYK_TOKEN set, it is never run and emits no finding rows. +set -euo pipefail +ROOT=$(cd "$(dirname "$0")/.." && pwd) +RC=${REVIEW_CHECKS:-$ROOT/.claude/skills/deep-code-review/scripts/review_checks.sh} +T=$(mktemp -d "${TMPDIR:-/tmp}/snyk.XXXXXX") && [ -n "$T" ] && [ -d "$T" ] || { echo "no temp dir" >&2; exit 1; } +trap 'cd /; rm -r -- "$T"' EXIT; fail=0 +ok() { echo "ok - $1"; }; bad() { echo "FAIL - $1"; fail=1; } +export PERUN_HEAVY_DIR="$T/heavy" HUB_LOG="$T/hub.log" SNYK_TOKEN=fake-token-example +mkdir -p "$T/repo" "$T/s" "$T/base" "$T/sys" && cd "$T/repo" && : >"$HUB_LOG" +git init -q . && case "$(git rev-parse --show-toplevel)" in "$(cd "$T" && pwd -P)"/*) ;; *) echo "refusing: repo not under temp dir" >&2; exit 1 ;; esac +G() { git -c user.email=t@example.com -c user.name=t "$@"; } +G commit -q --allow-empty -m base && git branch base +echo '{}' >.mcp.json; mkdir s1; echo '# s' >s1/SKILL.md; G add . && G commit -q -m f +for t in uvx snyk-agent-scan; do printf '#!/bin/sh\necho "%s $*" >>"$HUB_LOG"\necho '"'"'{"scan_path_responses":[{"issues":[{"code":"E001","message":"m"}]}]}'"'"'\n' "$t" >"$T/s/$t"; chmod +x "$T/s/$t"; done +printf '#!/bin/sh\necho %s\n' "'{\"results\":[],\"errors\":[]}'" >"$T/base/semgrep" +printf '#!/bin/sh\necho %s\n' "'{\"Results\":[]}'" >"$T/base/trivy" +printf '#!/bin/sh\nexit 0\n' >"$T/base/gitleaks"; chmod +x "$T/base/"* +for b in bash env git grep sed head tail cat rm mkdir mktemp dirname basename python3 perl timeout gtimeout jq true sort uniq paste tr cut wc awk date cp mv ls find touch tee readlink sleep; do + p=$(type -P "$b") || continue; ln -s "$p" "$T/sys/$b" +done +PATH="$T/s:$T/base:$T/sys" bash "$RC" --base base 2>"$T/err" >"$T/out" || true +[ ! -s "$HUB_LOG" ] && ok "uvx and snyk-agent-scan never invoked" || bad "uvx and snyk-agent-scan never invoked" +jq -e 'all(.[]; .tool!="snyk-agent-scan" or .rule=="not-checked")' "$T/out" >/dev/null && ok "no finding rows" || bad "no finding rows" +grep -q '^NOT CHECKED: snyk-agent-scan (unverified' "$T/err" && ok "reported NOT CHECKED" || bad "reported NOT CHECKED" +exit $fail From 8fe425da9e5a0782dc53e68493d0e7c3d5a24dde Mon Sep 17 00:00:00 2001 From: Remigiusz Antczak Date: Sun, 15 Nov 2026 04:48:24 +0100 Subject: [PATCH 2/5] Test: hub receipt names uninstalled checkov and hadolint review_checks.sh already prints NOT CHECKED rows, a "skipped (not installed)" list and a "not run" ran/wired line for matching tools that are missing, and prints no clean verdict. Add the missing regression test: a diff touching .tf, .py and a Dockerfile with only a bandit stub on PATH must list checkov and hadolint in all three places. Tests: 5/5 Co-Authored-By: Claude Opus 5.5 --- changelog.d/hub-receipt-not-installed.md | 2 ++ .../ci-tests.d/review-checks-hub-receipt.sh | 2 ++ scripts/test-review-checks-hub-receipt.sh | 29 +++++++++++++++++++ 3 files changed, 33 insertions(+) create mode 100644 changelog.d/hub-receipt-not-installed.md create mode 100644 scripts/ci-tests.d/review-checks-hub-receipt.sh create mode 100644 scripts/test-review-checks-hub-receipt.sh diff --git a/changelog.d/hub-receipt-not-installed.md b/changelog.d/hub-receipt-not-installed.md new file mode 100644 index 000000000..32bc62f21 --- /dev/null +++ b/changelog.d/hub-receipt-not-installed.md @@ -0,0 +1,2 @@ +### Added +- Regression test: a diff with .tf, .py and a Dockerfile and only bandit installed makes review_checks.sh name checkov and hadolint as not installed, with no clean verdict. diff --git a/scripts/ci-tests.d/review-checks-hub-receipt.sh b/scripts/ci-tests.d/review-checks-hub-receipt.sh new file mode 100644 index 000000000..0bab2e588 --- /dev/null +++ b/scripts/ci-tests.d/review-checks-hub-receipt.sh @@ -0,0 +1,2 @@ +# review_checks.sh: receipt names uninstalled tools (checkov, hadolint) when only bandit is present. +bash scripts/test-review-checks-hub-receipt.sh diff --git a/scripts/test-review-checks-hub-receipt.sh b/scripts/test-review-checks-hub-receipt.sh new file mode 100644 index 000000000..c4d67b727 --- /dev/null +++ b/scripts/test-review-checks-hub-receipt.sh @@ -0,0 +1,29 @@ +#!/usr/bin/env bash +# review_checks.sh receipt: a diff touching .tf, .py and a Dockerfile with only bandit installed must name checkov and hadolint +# as not installed (NOT CHECKED rows, "skipped (not installed)" in the summary, "not run" in the ran/wired line), never a silent clean. +set -euo pipefail +ROOT=$(cd "$(dirname "$0")/.." && pwd) +RC=${REVIEW_CHECKS:-$ROOT/.claude/skills/deep-code-review/scripts/review_checks.sh} +T=$(mktemp -d "${TMPDIR:-/tmp}/hubreceipt.XXXXXX") && [ -n "$T" ] && [ -d "$T" ] || { echo "no temp dir" >&2; exit 1; } +trap 'cd /; rm -r -- "$T"' EXIT; fail=0 +ok() { echo "ok - $1"; }; bad() { echo "FAIL - $1"; fail=1; } +export PERUN_HEAVY_DIR="$T/heavy" DCR_CACHE_ROOT="$T/nocache" DCR_USER_BIN= TMPDIR="$T/tmp" HOME="$T/home" +mkdir -p "$T/repo" "$T/bin" "$T/sys" "$T/tmp" "$T/home" && cd "$T/repo" +git init -q . && case "$(git rev-parse --show-toplevel)" in "$(cd "$T" && pwd -P)"/*) ;; *) echo "refusing: repo not under temp dir" >&2; exit 1 ;; esac +G() { git -c user.email=t@example.com -c user.name=t "$@"; } +printf 'x\n' >README.md; G add . && G commit -q -m base && git branch base +printf 'resource "aws_s3_bucket" "b" {}\n' >main.tf; printf 'print(1)\n' >a.py; printf 'FROM alpine:3.19\n' >Dockerfile +G add . && G commit -q -m code +printf '#!/bin/sh\necho %s\n' "'{\"results\":[],\"metrics\":{}}'" >"$T/bin/bandit"; chmod +x "$T/bin/bandit" +for b in bash env git grep sed head tail cat rm mkdir mktemp dirname basename python3 perl timeout gtimeout jq true sort uniq paste tr cut wc awk date cp mv ls find touch tee readlink sleep xargs; do + p=$(type -P "$b") || continue; ln -s "$p" "$T/sys/$b" +done +PATH="$T/bin:$T/sys" bash "$RC" --base base 2>"$T/err" >"$T/out" || true +for t in checkov hadolint; do + jq -e --arg t "$t" 'any(.[]; .tool==$t and .rule=="not-checked")' "$T/out" >/dev/null && grep -q "NOT CHECKED: $t (install: " "$T/err" \ + && ok "$t: not-checked row and NOT CHECKED line" || bad "$t receipt" + grep -qE "skipped \(not installed\):.*\b$t\b" "$T/err" && grep -qE "ran [0-9]+ / wired [0-9]+ scanners; not run:.*\b$t\b" "$T/err" \ + && ok "$t: in skipped and not-run summary" || bad "$t summary" +done +! grep -qiE 'clean|no findings' "$T/err" && ok "no clean verdict while tools did not run" || bad "clean verdict" +exit $fail From f9f1841f7064b62990b6284185a61a072006c13e Mon Sep 17 00:00:00 2001 From: Remigiusz Antczak Date: Sun, 15 Nov 2026 04:53:31 +0100 Subject: [PATCH 3/5] Semgrep: insecure-defaults (fail-open) rules for Python and JS/TS Add templates/semgrep-taint/defaults.yml with six rules: py debug-enabled, py cors-credentials-wildcard, py and js jwt-unverified (signature check off or alg none), py and js auth-fails-open (auth helper returns True / next() from its exception handler). Secret-env-fallback, TLS-verify-off and the JS CORS-with-credentials check already exist in py.yml and js.yml, so they are not repeated. scripts/test_semgrep_defaults.py runs seeded bad fixtures (17 annotated findings, recall 100%) and clean twins plus test-path copies (0 hits); it prints SKIP without semgrep, like the sibling semgrep tests. Registered in scripts/ci-tests.d, changelog.d entry added. Tests: 1/1 Co-Authored-By: Claude Opus 5.5 --- .../templates/semgrep-taint/defaults.yml | 99 +++++++++++++ changelog.d/semgrep-insecure-defaults.md | 2 + scripts/ci-tests.d/semgrep-defaults.sh | 2 + scripts/test_semgrep_defaults.py | 134 ++++++++++++++++++ 4 files changed, 237 insertions(+) create mode 100644 .claude/skills/deep-code-review/templates/semgrep-taint/defaults.yml create mode 100644 changelog.d/semgrep-insecure-defaults.md create mode 100644 scripts/ci-tests.d/semgrep-defaults.sh create mode 100644 scripts/test_semgrep_defaults.py diff --git a/.claude/skills/deep-code-review/templates/semgrep-taint/defaults.yml b/.claude/skills/deep-code-review/templates/semgrep-taint/defaults.yml new file mode 100644 index 000000000..599c7e493 --- /dev/null +++ b/.claude/skills/deep-code-review/templates/semgrep-taint/defaults.yml @@ -0,0 +1,99 @@ +# Bundled insecure-default (fail-open config) rules. Already covered elsewhere, so not repeated here: secret-env-fallback and tls-verify-off (py.yml, js.yml), cors-credentials-any-origin (js.yml). +# Added: py debug-enabled, py cors-credentials-wildcard, py/js jwt-unverified, py/js auth-fails-open. Test paths are skipped. +# ponytail: auth-fails-open only sees a try/except (catch) inside a function whose name says auth, token, permission, allowed or verify, and only a literal return True / next(). +rules: +- id: perun.py.debug-enabled + languages: [python] + severity: WARNING + message: Debug mode is switched on in code (DEBUG = True or app.run(debug=True)). It exposes stack traces and an interactive console in production. Read it from the environment and default it to off. + pattern-either: + - pattern: DEBUG = True + - pattern: $APP.run(..., debug=True, ...) + - pattern: $APP.debug = True + paths: + exclude: ["test", "tests", "fixtures", "mocks", "e2e", "test_*", "*_test.py", "conftest.py"] +- id: perun.py.cors-credentials-wildcard + languages: [python] + severity: WARNING + message: CORS allows every origin and also allows credentials. Any site can then make authenticated requests as the user. List the trusted origins explicitly. + pattern-either: + - pattern: $A.add_middleware(CORSMiddleware, ..., allow_origins=["*"], ..., allow_credentials=True, ...) + - pattern: $A.add_middleware(CORSMiddleware, ..., allow_credentials=True, ..., allow_origins=["*"], ...) + - pattern: CORS($A, ..., origins="*", ..., supports_credentials=True, ...) + - pattern: CORS($A, ..., supports_credentials=True, ..., origins="*", ...) + - patterns: + - pattern: CORS_ALLOW_ALL_ORIGINS = True + - pattern-either: + - pattern-inside: | + CORS_ALLOW_CREDENTIALS = True + ... + - pattern-inside: | + ... + CORS_ALLOW_CREDENTIALS = True + paths: + exclude: ["test", "tests", "fixtures", "mocks", "e2e", "test_*", "*_test.py", "conftest.py"] +- id: perun.py.jwt-unverified + languages: [python] + severity: ERROR + message: JWT signature checking is off or the none algorithm is allowed, so anyone can forge a token. Verify with a fixed algorithm list and the real key. + pattern-either: + - pattern: '$J.decode(..., options={..., "verify_signature": False, ...}, ...)' + - pattern: $J.decode(..., verify=False, ...) + - pattern: $J.decode(..., algorithms=[..., "none", ...], ...) + - pattern: $J.encode(..., algorithm="none", ...) + paths: + exclude: ["test", "tests", "fixtures", "mocks", "e2e", "test_*", "*_test.py", "conftest.py"] +- id: perun.js.jwt-unverified + languages: [javascript, typescript] + severity: ERROR + message: The JWT none algorithm is allowed, so anyone can forge a token. Verify with a fixed algorithm list and the real key. + pattern-either: + - pattern: '$F(..., {..., algorithms: [..., "none", ...], ...})' + - pattern: '$F(..., {..., algorithm: "none", ...})' + paths: + exclude: ["test", "tests", "__tests__", "spec", "fixtures", "mocks", "__mocks__", "e2e", "*.test.*", "*.spec.*"] +- id: perun.py.auth-fails-open + languages: [python] + severity: WARNING + message: An auth check returns True from its exception handler, so any error (a bad token, a down key service) grants access. Deny on error. + patterns: + - pattern-inside: | + def $F(...): + ... + - metavariable-regex: + metavariable: $F + regex: (?i).*(?:auth|token|permission|allowed|verify).* + - pattern: | + try: + ... + except $E: + ... + return True + paths: + exclude: ["test", "tests", "fixtures", "mocks", "e2e", "test_*", "*_test.py", "conftest.py"] +- id: perun.js.auth-fails-open + languages: [javascript, typescript] + severity: WARNING + message: An auth check lets the request through from its catch block (it returns true or calls next()), so any error grants access. Deny on error. + patterns: + - pattern-either: + - pattern-inside: | + function $F(...) { + ... + } + - pattern-inside: | + const $F = (...) => { + ... + } + - metavariable-regex: + metavariable: $F + regex: (?i).*(?:auth|token|permission|allowed|verify).* + - pattern-either: + - pattern: | + try { ... } catch ($E) { ... return true; } + - pattern: | + try { ... } catch ($E) { ... return next(); } + - pattern: | + try { ... } catch ($E) { ... next(); } + paths: + exclude: ["test", "tests", "__tests__", "spec", "fixtures", "mocks", "__mocks__", "e2e", "*.test.*", "*.spec.*"] diff --git a/changelog.d/semgrep-insecure-defaults.md b/changelog.d/semgrep-insecure-defaults.md new file mode 100644 index 000000000..279435dd4 --- /dev/null +++ b/changelog.d/semgrep-insecure-defaults.md @@ -0,0 +1,2 @@ +### Added +- Bundled semgrep rules for fail-open defaults (templates/semgrep-taint/defaults.yml): `perun.py.debug-enabled`, `perun.py.cors-credentials-wildcard`, `perun.py.jwt-unverified`, `perun.js.jwt-unverified`, `perun.py.auth-fails-open`, `perun.js.auth-fails-open`. Secret-env-fallback, TLS-verify-off and JS CORS-with-credentials were already covered and are not repeated. Tested by scripts/test_semgrep_defaults.py with seeded bad fixtures and clean twins. diff --git a/scripts/ci-tests.d/semgrep-defaults.sh b/scripts/ci-tests.d/semgrep-defaults.sh new file mode 100644 index 000000000..17a3f7c77 --- /dev/null +++ b/scripts/ci-tests.d/semgrep-defaults.sh @@ -0,0 +1,2 @@ +# Bundled insecure-default (fail-open config) semgrep rules in defaults.yml: seeded bad fixtures (recall 100%) and clean twins (0 hits); live part SKIPs without semgrep. +python3 scripts/test_semgrep_defaults.py diff --git a/scripts/test_semgrep_defaults.py b/scripts/test_semgrep_defaults.py new file mode 100644 index 000000000..6325729d8 --- /dev/null +++ b/scripts/test_semgrep_defaults.py @@ -0,0 +1,134 @@ +#!/usr/bin/env python3 +"""Tests for the bundled insecure-default rules in templates/semgrep-taint/defaults.yml. + +Each rule has seeded bad fixtures marked `ruleid: ` on the line before and a clean twin that must stay silent: recall 100%, zero hits +on the twins. The live check needs semgrep and prints SKIP without it (CI installs none). All names and values are fictional.""" +import json, os, re, shutil, subprocess, sys, tempfile + +ROOT = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) +CFG = os.path.join(ROOT, ".claude", "skills", "deep-code-review", "templates", "semgrep-taint", "defaults.yml") +src = open(CFG).read() +IDS = [l.split("id: ")[1] for l in src.splitlines() if l.startswith("- id: ")] +assert len(IDS) == 6 and src.count(" severity: ") == 6, IDS + +BAD_PY = '''import jwt +from flask_cors import CORS +# ruleid: perun.py.debug-enabled +DEBUG = True +# ruleid: perun.py.debug-enabled +app.run(host="0.0.0.0", debug=True) +# ruleid: perun.py.cors-credentials-wildcard +app.add_middleware(CORSMiddleware, allow_origins=["*"], allow_credentials=True) +# ruleid: perun.py.cors-credentials-wildcard +CORS(app, origins="*", supports_credentials=True) +# ruleid: perun.py.jwt-unverified +jwt.decode(tok, options={"verify_signature": False}) +# ruleid: perun.py.jwt-unverified +jwt.decode(tok, key, algorithms=["HS256", "none"]) +# ruleid: perun.py.jwt-unverified +jwt.encode(claims, None, algorithm="none") +def is_authenticated(tok): + # ruleid: perun.py.auth-fails-open + try: + check(tok) + except Exception: + log.warning("auth backend down") + return True + return False +''' +CLEAN_PY = '''import os, jwt +from flask_cors import CORS +DEBUG = os.environ.get("ACME_DEBUG") == "1" +app.run(host="127.0.0.1", debug=False) +app.add_middleware(CORSMiddleware, allow_origins=["https://app.example.com"], allow_credentials=True) +app.add_middleware(CORSMiddleware, allow_origins=["*"], allow_credentials=False) +CORS(app, origins="https://app.example.com", supports_credentials=True) +jwt.decode(tok, key, algorithms=["HS256"]) +jwt.decode(tok, options={"verify_signature": True}) +jwt.encode(claims, key, algorithm="HS256") +def is_authenticated(tok): + try: + check(tok) + except Exception: + return False + return True +def parse_number(s): + try: + return int(s) + except ValueError: + return True +''' +BAD_CORS_DJANGO = '''# ruleid: perun.py.cors-credentials-wildcard +CORS_ALLOW_ALL_ORIGINS = True +CORS_ALLOW_CREDENTIALS = True +''' +CLEAN_CORS_DJANGO = '''CORS_ALLOW_ALL_ORIGINS = True +CORS_ALLOW_CREDENTIALS = False +''' +BAD_JS = '''// ruleid: perun.js.jwt-unverified +jwt.verify(tok, key, { algorithms: ["HS256", "none"] }); +// ruleid: perun.js.jwt-unverified +jwt.sign(claims, "", { algorithm: "none" }); +async function verifyToken(req, res, next) { + // ruleid: perun.js.auth-fails-open + try { + await check(req.headers.authorization); + return next(); + } catch (e) { + log(e); + return next(); + } +} +const isAuthorized = (req) => { + // ruleid: perun.js.auth-fails-open + try { + return check(req); + } catch (e) { + return true; + } +}; +''' +CLEAN_JS = '''jwt.verify(tok, key, { algorithms: ["HS256"] }); +jwt.sign(claims, key, { algorithm: "HS256" }); +async function verifyToken(req, res, next) { + try { + await check(req.headers.authorization); + return next(); + } catch (e) { + return res.status(401).end(); + } +} +const parseBody = (req) => { + try { + return JSON.parse(req.body); + } catch (e) { + return true; + } +}; +''' +BAD = {"app/bad.py": BAD_PY, "app/bad_cors.py": BAD_CORS_DJANGO, "app/bad.js": BAD_JS, "app/bad.ts": BAD_JS} +CLEAN = {"app/clean.py": CLEAN_PY, "app/clean_cors.py": CLEAN_CORS_DJANGO, "app/clean.js": CLEAN_JS, "app/clean.ts": CLEAN_JS} +SKIPPED = {"tests/bad.py": BAD_PY, "app/bad.spec.ts": BAD_JS} + +if not shutil.which("semgrep"): + print("SKIP: semgrep not installed; the live fixture check did not run") + sys.exit(0) +d = tempfile.mkdtemp() +try: + want = set() + for rel, text in {**BAD, **CLEAN, **SKIPPED}.items(): + p = os.path.join(d, rel); os.makedirs(os.path.dirname(p), exist_ok=True); open(p, "w").write(text) + for rel, text in BAD.items(): + lines = text.splitlines() + want |= {(rel, i + 2, l.split("ruleid: ")[1]) for i, l in enumerate(lines) if "ruleid: " in l} + run = subprocess.run(["semgrep", "--config", CFG, "--metrics=off", "--disable-version-check", "--json", "--quiet", "--", "."], + cwd=d, capture_output=True, text=True) + out = json.loads(run.stdout) + got = {(r["path"], r["start"]["line"], re.search(r"perun\.(?:py|js)\..*$", r["check_id"])[0]) for r in out["results"]} + # a multi-line try is reported at its first line; the annotation sits on the line before it + assert got == want, f"missing {sorted(want - got)} extra {sorted(got - want)}" + ids = {i for _, _, i in got} + assert ids == set(IDS), f"rules without a bad fixture: {set(IDS) - ids}" +finally: + shutil.rmtree(d) +print(f"ok: {len(want)} bad findings, 0 on clean twins and test paths") From d0a689ef8b672c73450f0017dc5acd1838c6f615 Mon Sep 17 00:00:00 2001 From: Remigiusz Antczak Date: Sun, 15 Nov 2026 04:54:52 +0100 Subject: [PATCH 4/5] CI: run the insecure-defaults semgrep fixtures in real-hub scripts/test_semgrep_defaults.py printed SKIP when semgrep was not on PATH, and CI never put it there for that script, so the new rules were never checked in CI. The real-hub job now runs the script with the hash-locked semgrep venv first on PATH, tees the output, and fails if SKIP appears. Tests: 1/1 Co-Authored-By: Claude Opus 5.5 --- .github/workflows/ci.yml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 4c9714ff9..6af7bf9d8 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -561,6 +561,11 @@ jobs: mkdir -p "$RUNNER_TEMP/bin" for t in semgrep pysemgrep njsscan; do ln -s "$RUNNER_TEMP/venv-semgrep/bin/$t" "$RUNNER_TEMP/bin/$t"; done echo "$RUNNER_TEMP/bin" >> "$GITHUB_PATH" + - name: Run insecure-defaults semgrep fixtures (fail on SKIP, so the live check cannot silently skip) + run: | + set -euo pipefail + PATH="$RUNNER_TEMP/venv-semgrep/bin:$PATH" python3 scripts/test_semgrep_defaults.py | tee "$RUNNER_TEMP/semgrep-defaults.log" + if grep -q SKIP "$RUNNER_TEMP/semgrep-defaults.log"; then echo "semgrep fixture check skipped; failing"; exit 1; fi - name: Install rubocop 1.69.2 (pinned, private GEM_HOME) run: | set -euo pipefail From 39fd016606ebaa0dd8dd07e97a50ef7b71dc5676 Mon Sep 17 00:00:00 2001 From: Remi Antczak <140408771+remigiusz-antczak@users.noreply.github.com> Date: Sun, 15 Nov 2026 04:57:48 +0100 Subject: [PATCH 5/5] Hub: fold same file:line findings from several tools into one corroborated row review_checks.sh now merges rows that several tools report on the same file and line into one row that names every tool (corroborated by: ...) and keeps the highest severity named in the message prefix, first row on a tie. Rows carry no category field, so the key is file:line. Different lines, a tool's own extra rules on a line, not-checked rows and summary rows never fold. Tests: 10/10 Co-Authored-By: Claude Opus 5.5 --- .../deep-code-review/scripts/review_checks.sh | 17 ++++++ changelog.d/hub-corroboration-dedup.md | 2 + scripts/ci-tests.d/review-checks-hub-dedup.sh | 2 + scripts/test-review-checks-hub-dedup.sh | 52 +++++++++++++++++++ 4 files changed, 73 insertions(+) create mode 100644 changelog.d/hub-corroboration-dedup.md create mode 100644 scripts/ci-tests.d/review-checks-hub-dedup.sh create mode 100644 scripts/test-review-checks-hub-dedup.sh diff --git a/.claude/skills/deep-code-review/scripts/review_checks.sh b/.claude/skills/deep-code-review/scripts/review_checks.sh index 9732d383c..eeadc49d5 100755 --- a/.claude/skills/deep-code-review/scripts/review_checks.sh +++ b/.claude/skills/deep-code-review/scripts/review_checks.sh @@ -1574,6 +1574,23 @@ for t in data: k = key(t) or fkey(t) if not k or len(groups[k]) <= 3: out.append(row(t)) elif k not in done: done.add(k); out.append(summ(k, groups[k]) if key(t) else fsumm(k, groups[k])) +# Corroboration: several tools on the same file:line fold into one row (the highest severity named in the message prefix wins, the first row on a tie) that names every tool. Rows carry no category field, so the key is file:line; one tool's other rules on that line stay their own rows, and not-checked, line-0 and summary rows never fold. +SEV = {s: i for i, s in enumerate(("info", "low", "warning", "medium", "error", "high", "critical"))} +sv = lambda r: SEV.get((re.match(r"(\w+):", r["message"].lower()) or [0, ""])[1], -1) +fold, out2 = {}, [] +for r in out: + k = (os.path.normpath(r["file"]), r["line"]) + if r["rule"] == "not-checked" or r["rule"].endswith("-summary") or r["line"] <= 0 or r["file"] in ("", "-") or "_lines" in r: out2.append(r); continue + g = fold.get(k) + if g is None: fold[k] = g = {"best": r, "tools": [r["tool"]]}; out2.append(r) + elif r["tool"] in g["tools"]: out2.append(r) + else: + g["tools"].append(r["tool"]) + if sv(r) > sv(g["best"]): out2[out2.index(g["best"])] = r; g["best"] = r +out = out2 +for g in fold.values(): + if len(g["tools"]) > 1: + b = g["best"]; b["tools"] = g["tools"]; t = f" [corroborated by: {', '.join(g['tools'])}]"; b["message"] += t; b["text"] += t ML = [r.pop("_lines", None) for r in out] # per-file summary rows: every member line; popped here in every mode so the key never prints ud = sys.argv[11] # `git diff -U0` of BASE...HEAD; "" under --full/--history or when a git read above failed (no "new" key) if ud: diff --git a/changelog.d/hub-corroboration-dedup.md b/changelog.d/hub-corroboration-dedup.md new file mode 100644 index 000000000..fbe95e8c4 --- /dev/null +++ b/changelog.d/hub-corroboration-dedup.md @@ -0,0 +1,2 @@ +### Changed +- review_checks.sh folds findings from several tools on the same file and line into one row that names every tool ("corroborated by: ..."), keeping the highest severity named in the message prefix. Different lines, one tool's own rows, not-checked rows and summary rows never fold. scripts/test-review-checks-hub-dedup.sh covers it. diff --git a/scripts/ci-tests.d/review-checks-hub-dedup.sh b/scripts/ci-tests.d/review-checks-hub-dedup.sh new file mode 100644 index 000000000..fe794ddfd --- /dev/null +++ b/scripts/ci-tests.d/review-checks-hub-dedup.sh @@ -0,0 +1,2 @@ +# Registers the review_checks.sh hub corroboration dedup test (same file:line from several tools folds to one row). +bash scripts/test-review-checks-hub-dedup.sh diff --git a/scripts/test-review-checks-hub-dedup.sh b/scripts/test-review-checks-hub-dedup.sh new file mode 100644 index 000000000..47ca3f4ca --- /dev/null +++ b/scripts/test-review-checks-hub-dedup.sh @@ -0,0 +1,52 @@ +#!/usr/bin/env bash +# review_checks.sh hub corroboration: semgrep, zizmor and actionlint rows on the same file:line fold into one row naming all three tools, +# keeping the highest severity (message prefix); a different line stays its own row, and one tool on two lines gives two rows. Stub tools on a +# hermetic PATH, fictional workflow content only. +set -euo pipefail +ROOT=$(cd "$(dirname "$0")/.." && pwd) +RC=${REVIEW_CHECKS:-$ROOT/.claude/skills/deep-code-review/scripts/review_checks.sh} +T=$(mktemp -d "${TMPDIR:-/tmp}/hubdedup.XXXXXX") && [ -n "$T" ] && [ -d "$T" ] || { echo "no temp dir" >&2; exit 1; } +trap 'cd /; rm -r -- "$T"' EXIT +fail=0 +export DCR_CACHE_ROOT="$T/nocache" DCR_USER_BIN= PERUN_HEAVY_DIR="$T/heavy" REVIEW_LEASE_WAIT=0 REVIEW_SEMGREP_REGISTRY=0 +unset REVIEW_OSV REVIEW_OSV_DB_DIR OSV_SCANNER_LOCAL_DB_CACHE_DIRECTORY REVIEW_SEMGREP_CONFIG REVIEW_FIXTURE_ROWS REVIEW_TEST_CMD +G() { git -c user.email=t@example.com -c user.name=t "$@"; } +mkdir -p "$T/bin" "$T/zbin" "$T/sys" +for b in bash env git grep sed head tail cat rm mkdir mktemp dirname basename python3 perl timeout gtimeout jq true sort uniq paste tr cut wc awk date cp mv ls find touch tee readlink sleep cmp nice uname; do + p=$(type -P "$b") || continue; ln -s "$p" "$T/sys/$b" # only the system binaries review_checks.sh needs, never the host PATH +done +cat >"$T/bin/semgrep" <<'S' +#!/usr/bin/env bash +cat "$SG_JSON" +S +cat >"$T/bin/zizmor" <<'S' +#!/usr/bin/env bash +cat "$ZZ_JSON"; exit 13 +S +cat >"$T/bin/actionlint" <<'S' +#!/usr/bin/env bash +cat "$AL_JSON"; exit 1 +S +chmod +x "$T/bin/"* +WF=.github/workflows/ci.yml +mkdir -p "$T/repo" && cd "$T/repo" && git init -q . && G commit -q --allow-empty -m base && git branch base +mkdir -p .github/workflows && printf 'name: acme-ci\non: push\njobs:\n build:\n runs-on: ubuntu-latest\n steps:\n - uses: actions/checkout@v4\n - run: make test\n - run: make lint\n - run: make pack\n' >"$WF" +G add -A && G commit -q -m change +# sg LINE...: one semgrep row per line, message "low: ..."; zz ROW0...: zizmor rows (0-based); al LINE...: actionlint rows, message "error: ..." +sg() { local r= l; for l in "$@"; do r+="${r:+,}{\"check_id\":\"acme-rule\",\"path\":\"$WF\",\"start\":{\"line\":$l},\"extra\":{\"message\":\"low: mutable tag\"}}"; done; printf '{"results":[%s],"errors":[],"paths":{"scanned":["%s"]}}\n' "$r" "$WF"; } +zz() { local r= l; for l in "$@"; do r+="${r:+,}{\"ident\":\"unpinned-uses\",\"desc\":\"d\",\"locations\":[{\"symbolic\":{\"kind\":\"Primary\",\"key\":{\"Local\":{\"verbatim_path\":\"$WF\"}}},\"concrete\":{\"location\":{\"start_point\":{\"row\":$l}}}}]}"; done; printf '[%s]\n' "$r"; } +al() { local r= l; for l in "$@"; do r+="${r:+,}{\"message\":\"error: bad step\",\"filepath\":\"$WF\",\"line\":$l,\"kind\":\"expression\"}"; done; printf '[%s]\n' "$r"; } +go() { PATH="$T/bin:$T/sys" SG_JSON="$T/sg.json" ZZ_JSON="$T/zz.json" AL_JSON="$T/al.json" bash "$RC" --base base >"$T/out" 2>"$T/err" || true; } +at() { jq -c --argjson l "$1" '[.[] | select(.line==$l and .rule!="not-checked")]' "$T/out"; } +eq() { if [ "$1" = "$2" ]; then echo "ok - $3"; else echo "FAIL - $3 (got $1, want $2)"; fail=1; fi; } + +sg 5 >"$T/sg.json"; zz 4 >"$T/zz.json"; al 5 >"$T/al.json"; go # all three on line 5 +eq "$(at 5 | jq length)" 1 "3 tools, same line: one row" +eq "$(at 5 | jq -r '.[0].tools | sort | join(",")')" "actionlint,semgrep,zizmor" "the row names all 3 tools" +eq "$(at 5 | jq -r '.[0].message | test("corroborated by: ")')" true "message lists the tools" +eq "$(at 5 | jq -r '.[0].tool')" actionlint "max severity kept (error beats low)" + +sg 5 9 >"$T/sg.json"; zz 99 >"$T/zz.json"; al 99 >"$T/al.json"; go # only semgrep has rows on 5 and 9 +eq "$(at 5 | jq length)$(at 9 | jq length)" 11 "one tool, 2 lines: 2 rows" +eq "$(jq '[.[] | select(.tool=="semgrep")] | length' "$T/out")" 2 "no merge across lines" +exit $fail