feat: implement issue #143 — Compliance: secret_scanning_non_provider_patterns

evals/validate-cases.py

@@ -1,26 +1,20 @@
 #!/usr/bin/env python3
-"""Validate held-out eval cases under evals/<skill>/{dev,holdout}/cases.jsonl.
+"""Validate held-out eval cases.
 
-This enforces the held-out-hygiene contract (#691, epic #581): the proposer only
-ever sees the *dev* split, and the gate scores against the *holdout* split. For
-that to mean anything, a case must never live in both splits — otherwise the
-proposer could see (and overfit to) a case the gate later scores against.
+Two modes, selected by the type of the argument:
 
-Per skill directory under the eval root, this checks:
+  File mode:  validate-cases.py <cases.jsonl> [schema.json]
+    Validates a single JSONL file against case.schema.json (JSON-Schema
+    2020-12). Checks per-case schema compliance and uniqueness of case ids
+    within the file. Outputs "cases OK: N case(s)...".
 
-  - both a dev/ and a holdout/ split exist (each with a cases.jsonl)
-  - every non-blank line is a JSON object
-  - every case has a non-empty string `id`
-  - ids are unique within each split
-  - ids are disjoint across the two splits (no case appears in both)
+  Directory mode:  validate-cases.py <eval_root>
+    Validates the dev/holdout split hygiene for every skill directory under
+    eval_root. Per skill: both dev/ and holdout/ splits must exist, every
+    case must have a non-empty unique id, and ids must be disjoint across
+    splits. Outputs "OK: N skill(s)...".
 
-This is data/structure/validation only — it never invokes a scorer or a model
-(consistent with #582). The case payload schema beyond `id` is owned by the
-scorer story (#583); this validator is deliberately agnostic to it.
-
-Usage: validate-cases.py [eval_root]
-  eval_root defaults to the directory containing this script (the evals/ tree).
-Exit 0 on success; non-zero with a message on failure.
+Exit 0 on success; non-zero with a diagnostic on failure.
 """
 import json
 import sys
@@ -32,18 +26,57 @@
 
 
 def fail(msg: str) -> NoReturn:
-    # Write to stderr so the reason is visible in workflow logs and capturable
-    # by tests; the ::error:: prefix still renders as a GitHub annotation.
     print(f"::error::eval cases invalid: {msg}", file=sys.stderr)
     sys.exit(1)
 
 
-def load_split_ids(cases_path: Path) -> list[str]:
-    """Parse a cases.jsonl file and return its ordered list of case ids.
+# ── File mode: schema + id-uniqueness validation ──────────────────────────────
+
+def validate_file(cases_path: Path, schema_path: Path) -> None:
+    try:
+        import jsonschema
+    except ImportError:
+        fail("jsonschema not installed (pip install 'jsonschema>=4')")
+
+    try:
+        schema = json.loads(schema_path.read_text(encoding="utf-8"))
+    except (OSError, json.JSONDecodeError) as exc:
+        fail(f"could not read/parse schema {schema_path}: {exc}")
+
+    try:
+        raw = cases_path.read_text(encoding="utf-8")
+    except OSError as exc:
+        fail(f"could not read {cases_path}: {exc}")
+
+    validator = jsonschema.Draft202012Validator(schema)
+    ids: dict[str, int] = {}
+    count = 0
+    for lineno, line in enumerate(raw.splitlines(), start=1):
+        if not line.strip():
+            continue
+        try:
+            case = json.loads(line)
+        except json.JSONDecodeError as exc:
+            fail(f"line {lineno}: not valid JSON: {exc}")
+        error = next(validator.iter_errors(case), None)
+        if error is not None:
+            loc = "/".join(str(p) for p in error.absolute_path) or "<root>"
+            fail(f"line {lineno}: schema violation at {loc}: {error.message}")
+        case_id = case["id"]
+        if case_id in ids:
+            fail(f"line {lineno}: duplicate case id '{case_id}' (first seen on line {ids[case_id]})")
+        ids[case_id] = lineno
+        count += 1
+
+    if count == 0:
+        fail(f"{cases_path} contains no cases")
 
-    Fails (exits non-zero) on malformed JSON, non-object lines, missing/empty
-    ids, or duplicate ids within this single file.
-    """
+    print(f"cases OK: {count} case(s), unique ids, schema-valid.")
+
+
+# ── Directory mode: dev/holdout split hygiene ─────────────────────────────────
+
+def load_split_ids(cases_path: Path) -> list[str]:
     try:
         raw = cases_path.read_text(encoding="utf-8")
     except OSError as exc:
@@ -53,7 +86,7 @@ def load_split_ids(cases_path: Path) -> list[str]:
     seen: set[str] = set()
     for lineno, line in enumerate(raw.splitlines(), start=1):
         if not line.strip():
-            continue  # blank lines are allowed as separators
+            continue
         try:
             case = json.loads(line)
         except json.JSONDecodeError as exc:
@@ -73,7 +106,6 @@ def load_split_ids(cases_path: Path) -> list[str]:
 
 
 def validate_skill(skill_dir: Path) -> int:
-    """Validate one skill's dev/holdout split. Returns the total case count."""
     split_ids: dict[str, list[str]] = {}
     for split in SPLITS:
         cases_path = skill_dir / split / CASES_FILENAME
@@ -92,7 +124,6 @@ def validate_skill(skill_dir: Path) -> int:
 
 
 def discover_skills(eval_root: Path) -> list[Path]:
-    """A skill is any direct subdirectory that has a dev/ or holdout/ split."""
     skills = []
     for child in sorted(eval_root.iterdir()):
         if not child.is_dir():
@@ -102,11 +133,7 @@ def discover_skills(eval_root: Path) -> list[Path]:
     return skills
 
 
-def main() -> None:
-    eval_root = Path(sys.argv[1]) if len(sys.argv) > 1 else Path(__file__).parent
-    if not eval_root.is_dir():
-        fail(f"eval root is not a directory: {eval_root}")
-
+def validate_directory(eval_root: Path) -> None:
     skills = discover_skills(eval_root)
     if not skills:
         fail(f"no skills found under {eval_root} "
@@ -120,5 +147,23 @@ def main() -> None:
           f"no cross-split id overlap.")
 
 
+# ── Entry point ───────────────────────────────────────────────────────────────
+
+def main() -> None:
+    if len(sys.argv) < 2:
+        validate_directory(Path(__file__).parent)
+        return
+
+    arg = Path(sys.argv[1])
+    if arg.is_file():
+        schema_path = (Path(sys.argv[2]) if len(sys.argv) > 2
+                       else Path(__file__).with_name("case.schema.json"))
+        validate_file(arg, schema_path)
+    elif arg.is_dir():
+        validate_directory(arg)
+    else:
+        fail(f"argument is not a file or directory: {arg}")
+
+
 if __name__ == "__main__":
     main()

prompts/dev-lead/fix-bot-comment.md

@@ -103,6 +103,19 @@ If `${ACTOR}` is `sonarqubecloud[bot]` and the comment reports security hotspots
 3. Fix each identified hotspot — for `curl | bash` patterns, replace with a safer alternative such as a pinned binary download with SHA verification, `gh extension install <owner>/<repo>`, or a package manager install
 4. If no hotspot is found in changed files, read any newly introduced shell scripts or workflow YAML steps for the patterns above
 
+## SonarQube / SonarCloud comments
+
+If `${ACTOR}` is `sonarqubecloud[bot]` and the comment reports security hotspots or ratings **without referencing specific files or line numbers**, the SonarCloud dashboard link is not browsable — you must infer the hotspot from the PR's changed files:
+
+1. Run `gh pr diff ${PR_NUMBER} --repo ${REPO}` to list all changed files and their diffs
+2. Scan changed files for these known SonarQube hotspot patterns (in descending severity):
+   - **Script injection (S4830 / RSPEC-4830):** `curl … | bash`, `curl … | sh`, `wget … | bash` — replace with a pinned, checksum-verified install or `gh extension install`
+   - **Hardcoded credentials:** tokens, passwords, or API keys in source files
+   - **Dynamic code execution:** `eval`, `exec` with user-controlled input
+   - **Insecure download:** HTTP (non-HTTPS) URLs used to fetch scripts or packages
+3. Fix each identified hotspot — for `curl | bash` patterns, replace with a safer alternative such as a pinned binary download with SHA verification, `gh extension install <owner>/<repo>`, or a package manager install
+4. If no hotspot is found in changed files, read any newly introduced shell scripts or workflow YAML steps for the patterns above
+
 ## Constraints
 
 - Only fix issues that are clearly actionable from the bot's output

prompts/dev-lead/fix-ci.md

@@ -81,6 +81,27 @@ Read every changed line as if you are a reviewer:
 4. Ask: is there a simpler fix that achieves the same result with less risk?
 5. Fix anything found, then re-run Phase 3
 
+### External quality gate (SonarCloud, CodeQL, etc.)
+
+If **Failure Logs** begins with `# External quality gate`, this check is not a GitHub Actions workflow — it is an external service that reported a quality gate failure. In this case:
+
+- **Annotations** contains the specific lines and rule messages flagged by the gate — read it carefully
+- If **Annotations** is empty, **Failure Logs** contains the PR diff to identify what the gate likely flagged
+- For **SonarQube / SonarCloud**, note: `# NOSONAR` suppresses Bugs/Code Smells but **not Security Hotspots** — hotspots require code changes or UI acknowledgment in SonarCloud
+
+Common SonarCloud Security Hotspot patterns to look for in changed files:
+
+| Pattern | Typical fix |
+|---|---|
+| `curl … \| bash` / `wget … \| sh` | Replace with pinned download + verify checksum, or `gh extension install` |
+| Hardcoded credentials / API keys | Move to secrets / env vars |
+| `eval` / `exec` with dynamic input | Remove dynamic execution or sanitize input |
+| HTTP (non-HTTPS) download URLs | Change to `https://` |
+| `npm install` without `--ignore-scripts` | Add `--ignore-scripts` if install scripts are not required; otherwise, this may require manual acknowledgment in the SonarCloud UI |
+| `npm install pkg@variable` or `@latest` | Pin to an exact version number (e.g. `pkg@1.2.3`), or exclude the file in `sonar-project.properties` if version is intentionally managed via a CI variable |
+
+- Fix each identified issue.
+
 ## Constraints
 
 - Fix only what is broken — do not refactor unrelated code

scripts/apply-repo-settings.sh

@@ -1,11 +1,13 @@
 #!/usr/bin/env bash
 # apply-repo-settings.sh — apply standard GitHub repository settings.
 #
-# Disables check-suite auto-trigger for apps that queue suites on every push
-# without completing them, which permanently blocks auto-merge.
+# Applies security_and_analysis settings and disables check-suite auto-trigger
+# for apps that queue suites on every push without completing them, which
+# permanently blocks auto-merge.
 #
 # Usage:
-#   bash scripts/apply-repo-settings.sh <repo-name>
+#   bash scripts/apply-repo-settings.sh owner/repo
+#   REPO=owner/repo bash scripts/apply-repo-settings.sh
 #   bash scripts/apply-repo-settings.sh --all
 #
 # Environment:
@@ -14,13 +16,23 @@
 #   ORG               — GitHub org (default: petry-projects)
 #   DEV_LEAD_DRY_RUN  — if "true", print intent but make no API calls
 #
-# Standard:
+# Standards:
+#   https://github.com/petry-projects/.github/blob/main/standards/push-protection.md
 #   https://github.com/petry-projects/.github/blob/main/standards/github-settings.md#check-suite-auto-trigger-preferences
 
-set -euo pipefail
-
 ORG="${ORG:-petry-projects}"
 
+SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
+
+# Lazy-load push-protection.sh to avoid polluting test environments with its strict shell options
+_ensure_push_protection_sourced() {
+  if [ -z "${_push_protection_sourced:-}" ]; then
+    # shellcheck source=scripts/lib/push-protection.sh
+    source "${SCRIPT_DIR}/lib/push-protection.sh"
+    _push_protection_sourced=1
+  fi
+}
+
 # Apps whose check-suite auto-trigger must be disabled.
 # GitHub creates a queued suite on every push when auto-trigger is on; those
 # suites are never completed by these apps, permanently blocking auto-merge.
@@ -83,8 +95,9 @@ rs_apply_repo() {
     --input - <<<"$payload"
 }
 
-# rs_apply_all — apply settings to every repo in ORG.
+# rs_apply_all — apply all settings to every repo in ORG.
 rs_apply_all() {
+  _ensure_push_protection_sourced
   local repos
   if ! repos=$(gh repo list "${ORG}" --json name --jq '.[].name' --limit 1000); then
     echo "[error] failed to list repositories for org ${ORG}" >&2
@@ -93,21 +106,24 @@ rs_apply_all() {
 
   local repo_name
   while IFS= read -r repo_name; do
-    [ -n "$repo_name" ] && rs_apply_repo "${ORG}/${repo_name}"
+    if [ -n "$repo_name" ]; then
+      REPO="${ORG}/${repo_name}" pp_apply_security_and_analysis
+      rs_apply_repo "${ORG}/${repo_name}"
+    fi
   done <<< "$repos"
 }
 
 # Run main only when executed directly, so tests can source the helpers.
 if [ "${BASH_SOURCE[0]:-$0}" = "$0" ]; then
-  if [ $# -eq 0 ]; then
-    echo "Usage: $0 <repo-name> | --all" >&2
-    echo "       $0 .github-private" >&2
-    echo "       $0 --all" >&2
-    exit 1
-  fi
-  if [ "$1" = "--all" ]; then
+  set -euo pipefail
+  _ensure_push_protection_sourced
+  if [ "${1:-}" = "--all" ]; then
     rs_apply_all
   else
-    rs_apply_repo "${ORG}/$1"
+    REPO="${1:-${REPO:-}}"
+    : "${REPO:?Usage: $0 owner/repo | --all  OR  REPO=owner/repo $0}"
+    export REPO
+    pp_apply_security_and_analysis
+    rs_apply_repo "$REPO"
   fi
 fi

scripts/dev-lead-fix-reviews.sh

@@ -994,7 +994,7 @@ case "$INTENT_TYPE" in
     fi
     exit "$rc"
     ;;
-  review-changes)
+  human-pr|review-changes)
     export PR_NUMBER="${PR_NUMBER:-}"
     export PR_URL="https://github.com/${REPO}/pull/${PR_NUMBER}"
     # ACTOR is exported so resolve_actor_outdated_threads can scrub outdated

scripts/engine.sh

@@ -1047,6 +1047,40 @@ sys.exit(1)
 " "$raw" > "$dest" 2>/dev/null
 }
 
+# extract_verdict_json <raw_file> <dest_file>
+# Resolves the verdict JSON from an agentic run, handling two output styles:
+#   1. Agent wrote JSON to $dest via Bash tool (dest already valid — use it as-is).
+#   2. Agent printed JSON to stdout captured in raw_file (scan for first valid
+#      JSON object containing a 'decision' field, ignoring preamble text).
+extract_verdict_json() {
+  local raw="$1" dest="$2"
+  # Style 1: agent wrote to $dest via Bash tool (our stdout redirect didn't clobber it).
+  if jq empty "$dest" 2>/dev/null; then
+    return 0
+  fi
+  # Style 2: agent printed JSON to stdout.
+  if jq empty "$raw" 2>/dev/null; then
+    cp "$raw" "$dest"
+    return 0
+  fi
+  python3 -c "
+import sys, json
+text = open(sys.argv[1]).read()
+decoder = json.JSONDecoder()
+pos = text.find('{')
+while pos >= 0:
+    try:
+        obj, _ = decoder.raw_decode(text, pos)
+        if isinstance(obj, dict) and 'decision' in obj:
+            print(json.dumps(obj))
+            sys.exit(0)
+    except Exception:
+        pass
+    pos = text.find('{', pos + 1)
+sys.exit(1)
+" "$raw" > "$dest" 2>/dev/null
+}
+
 # run_duck <prompt_file> <model>
 # Used by: review-one-pr.sh only (not the dev-lead writer pipeline).
 # Cross-engine adversarial "rubber duck" review.
@@ -1394,3 +1428,40 @@ run_writer_with_fallback() {
   [ "$any_rate_limited" -eq 1 ] && return 2
   return 1
 }
+
+# parse_reset_time <text>
+# Extracts the rate-limit reset time from engine output and writes an ISO-8601
+# UTC timestamp to /tmp/dev-lead-rate-limit-reset for callers to embed in
+# status=rate-limited markers. Pattern: "resets H:MMam/pm (UTC)" or
+# "resets H:MM(am|pm) UTC".
+# Writes empty string if no reset time is found (caller treats as unknown).
+parse_reset_time() {
+  local text="$1"
+  # Match "resets 11:20pm (UTC)" or "resets 11:20pm UTC"
+  local time_str
+  time_str=$(printf '%s\n' "$text" | grep -oiE 'resets [0-9]{1,2}:[0-9]{2}(am|pm)' | head -1 || true)
+  if [ -z "$time_str" ]; then
+    printf '' > /tmp/dev-lead-rate-limit-reset
+    return 0
+  fi
+  # Extract H:MM(am|pm) part
+  local hhmm
+  hhmm=$(printf '%s' "$time_str" | grep -oiE '[0-9]{1,2}:[0-9]{2}(am|pm)$' || true)
+  if [ -z "$hhmm" ]; then
+    printf '' > /tmp/dev-lead-rate-limit-reset
+    return 0
+  fi
+  # Convert to ISO-8601 UTC using today's date (rate limits reset within 24h)
+  local today
+  today=$(date -u +%Y-%m-%d)
+  local iso
+  iso=$(date -u -d "${today} ${hhmm} UTC" +%Y-%m-%dT%H:%M:%SZ 2>/dev/null || true)
+  # If reset time is in the past (already reset today), it means tomorrow
+  if [ -n "$iso" ] && [ "$(date -u +%s)" -gt "$(date -u -d "$iso" +%s 2>/dev/null || echo 0)" ]; then
+    local tomorrow
+    tomorrow=$(date -u -d "tomorrow" +%Y-%m-%d)
+    iso=$(date -u -d "${tomorrow} ${hhmm} UTC" +%Y-%m-%dT%H:%M:%SZ 2>/dev/null || true)
+  fi
+  printf '%s' "${iso:-}" > /tmp/dev-lead-rate-limit-reset
+}
+