feat: implement issue #252 — Compliance: secret_scanning_non_provider_patterns

scripts/compliance-remediate.sh

@@ -145,6 +145,44 @@ remediate_setting() {
   fi
 }
 
+# ---------------------------------------------------------------------------
+# Direct API: nested security_and_analysis settings
+# ---------------------------------------------------------------------------
+# The security_and_analysis settings live under a nested object on the repo
+# resource, so they can't be patched via `gh api -F key=value` (that only
+# sends flat form fields). Build the nested JSON and PATCH via stdin.
+remediate_security_analysis_setting() {
+  local repo="$1" setting="$2" expected_status="$3" check="$4"
+
+  if [[ "$DRY_RUN" == "true" ]]; then
+    skip "[DRY] Would PATCH $ORG/$repo: security_and_analysis.$setting.status=$expected_status"
+    report_direct "$repo" "$check" \
+      "DRY: would PATCH \`security_and_analysis.$setting.status=$expected_status\`"
+    return 0
+  fi
+
+  local payload
+  if ! payload=$(jq -n \
+    --arg key "$setting" \
+    --arg status "$expected_status" \
+    '{security_and_analysis: {($key): {status: $status}}}') || [[ -z "$payload" ]]; then
+    err "Failed to build payload for $ORG/$repo: security_and_analysis.$setting.status=$expected_status"
+    report_fail "$repo" "$check" \
+      "jq failed to build payload for security_and_analysis.$setting.status=$expected_status"
+    return 0
+  fi
+
+  if printf '%s' "$payload" | gh api -X PATCH "repos/$ORG/$repo" --input - > /dev/null 2>&1; then
+    ok "Fixed $ORG/$repo: security_and_analysis.$setting.status=$expected_status"
+    report_direct "$repo" "$check" \
+      "Set \`security_and_analysis.$setting.status=$expected_status\`"
+  else
+    err "Failed to patch $ORG/$repo: security_and_analysis.$setting.status=$expected_status"
+    report_fail "$repo" "$check" \
+      "PATCH security_and_analysis.$setting.status=$expected_status failed (token may lack repo-admin permission)"
+  fi
+}
+
 # ---------------------------------------------------------------------------
 # Direct API: disable check-suite auto-trigger for problematic apps
 # ---------------------------------------------------------------------------
@@ -685,6 +723,9 @@ remediate_finding() {
       report_skip "$repo" "$check" \
         "Dependabot config updates require human review — fix using standards/dependabot-policy.md"
       ;;
+    push-protection/non_provider_patterns_enabled | push-protection/secret_scanning_non_provider_patterns)
+      remediate_security_analysis_setting "$repo" "secret_scanning_non_provider_patterns" "enabled" "$check"
+      ;;
     push-protection/*)
       report_skip "$repo" "$check" \
         "Push protection settings require security_and_analysis API scope — run scripts/apply-repo-settings.sh"

test/scripts/compliance-remediate/helpers/setup.bash

@@ -0,0 +1,52 @@
+#!/usr/bin/env bash
+# Common test helpers for compliance-remediate bats suites.
+set -euo pipefail
+
+TT_REPO_ROOT="$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")/../../../.." && pwd)"
+readonly TT_REPO_ROOT
+export TT_REPO_ROOT
+
+TT_SCRIPT="${TT_REPO_ROOT}/scripts/compliance-remediate.sh"
+readonly TT_SCRIPT
+export TT_SCRIPT
+
+TT_STUBS_DIR="${TT_REPO_ROOT}/test/scripts/compliance-remediate/stubs"
+readonly TT_STUBS_DIR
+export TT_STUBS_DIR
+
+tt_make_tmpdir() {
+  TT_TMP="$(mktemp -d)"
+  export TT_TMP
+}
+
+tt_cleanup_tmpdir() {
+  if [[ -n "${TT_TMP:-}" && -d "${TT_TMP}" ]]; then
+    rm -rf "${TT_TMP}"
+  fi
+}
+
+# Install the fake `gh` binary on PATH for the test.
+tt_install_gh_stub() {
+  local tt_tmp="${TT_TMP:?TT_TMP must be set (call tt_make_tmpdir first)}"
+  local stubs_dir="${TT_STUBS_DIR:?TT_STUBS_DIR must be set}"
+  local stub_dir="${tt_tmp}/bin"
+  mkdir -p "$stub_dir"
+  cp "${stubs_dir}/gh" "$stub_dir/gh"
+  chmod +x "$stub_dir/gh"
+  PATH="${stub_dir}:${PATH}"
+  export PATH
+}
+
+# Write a findings.json file with a single finding and echo its path.
+# Args: repo, category, check
+tt_write_finding() {
+  local repo="$1" category="$2" check="$3"
+  local path="${TT_TMP}/findings.json"
+  jq -n \
+    --arg repo "$repo" \
+    --arg category "$category" \
+    --arg check "$check" \
+    '[{repo:$repo, category:$category, check:$check, severity:"warning", detail:"test", standard_ref:"x"}]' \
+    > "$path"
+  printf '%s' "$path"
+}

test/scripts/compliance-remediate/security-analysis.bats

@@ -0,0 +1,154 @@
+#!/usr/bin/env bats
+# Tests for the push-protection security_and_analysis remediation path added
+# to scripts/compliance-remediate.sh.
+#
+# Before this change, every push-protection/* finding was routed to the
+# `report_skip` catch-all. This suite pins the new behaviour: the specific
+# `secret_scanning_non_provider_patterns` finding now PATCHes the nested
+# security_and_analysis setting on the repo, and the rest of the
+# push-protection/* catch-all is preserved.
+
+bats_require_minimum_version 1.5.0
+
+load 'helpers/setup'
+
+setup() {
+  tt_make_tmpdir
+  tt_install_gh_stub
+  export GH_STUB_LOG="${TT_TMP}/gh.log"
+  export GH_STUB_STDIN_LOG="${TT_TMP}/gh.stdin"
+  : >"$GH_STUB_LOG"
+  : >"$GH_STUB_STDIN_LOG"
+}
+
+teardown() {
+  tt_cleanup_tmpdir
+}
+
+# ---------------------------------------------------------------------------
+# Apply path: non_provider_patterns_enabled (canonical audit check name)
+# is auto-remediated
+# ---------------------------------------------------------------------------
+
+@test "applies PATCH to security_and_analysis for non_provider_patterns_enabled" {
+  findings="$(tt_write_finding ".github" "push-protection" "non_provider_patterns_enabled")"
+  report_dir="${TT_TMP}/report"
+
+  GH_TOKEN=fake \
+    FINDINGS_FILE="$findings" \
+    REPORT_DIR="$report_dir" \
+    DRY_RUN=false \
+    run bash "$TT_SCRIPT"
+
+  [ "$status" -eq 0 ]
+
+  # gh was called with `api -X PATCH repos/petry-projects/.github --input -`
+  grep -qE 'api -X PATCH repos/petry-projects/\.github (.*)--input -' "$GH_STUB_LOG"
+
+  # The stdin payload must enable the specific setting via the nested object
+  grep -q '"security_and_analysis"' "$GH_STUB_STDIN_LOG"
+  grep -q '"secret_scanning_non_provider_patterns"' "$GH_STUB_STDIN_LOG"
+  grep -q '"enabled"' "$GH_STUB_STDIN_LOG"
+
+  # The finding should land in the direct-fix table, not in skipped
+  grep -q 'non_provider_patterns_enabled' "$report_dir/remediation-report.md"
+  ! grep -q 'non_provider_patterns_enabled' "$report_dir/skipped.md"
+}
+
+# Backward-compat alias: the legacy `secret_scanning_non_provider_patterns`
+# finding name is still routed to the same handler.
+@test "applies PATCH to security_and_analysis for secret_scanning_non_provider_patterns (legacy alias)" {
+  findings="$(tt_write_finding ".github" "push-protection" "secret_scanning_non_provider_patterns")"
+  report_dir="${TT_TMP}/report"
+
+  GH_TOKEN=fake \
+    FINDINGS_FILE="$findings" \
+    REPORT_DIR="$report_dir" \
+    DRY_RUN=false \
+    run bash "$TT_SCRIPT"
+
+  [ "$status" -eq 0 ]
+
+  # gh was called with `api -X PATCH repos/petry-projects/.github --input -`
+  grep -qE 'api -X PATCH repos/petry-projects/\.github (.*)--input -' "$GH_STUB_LOG"
+
+  # The stdin payload must enable the specific setting via the nested object
+  grep -q '"security_and_analysis"' "$GH_STUB_STDIN_LOG"
+  grep -q '"secret_scanning_non_provider_patterns"' "$GH_STUB_STDIN_LOG"
+  grep -q '"enabled"' "$GH_STUB_STDIN_LOG"
+
+  # The finding should land in the direct-fix table, not in skipped
+  grep -q 'secret_scanning_non_provider_patterns' "$report_dir/remediation-report.md"
+  ! grep -q 'secret_scanning_non_provider_patterns' "$report_dir/skipped.md"
+}
+
+# ---------------------------------------------------------------------------
+# Dry-run path: no API call is made, but the finding shows up as a dry-run
+# entry on the remediation report (not in the skipped report).
+# ---------------------------------------------------------------------------
+
+@test "dry-run does not call the GitHub API but records the planned change" {
+  findings="$(tt_write_finding ".github" "push-protection" "secret_scanning_non_provider_patterns")"
+  report_dir="${TT_TMP}/report"
+
+  GH_TOKEN=fake \
+    FINDINGS_FILE="$findings" \
+    REPORT_DIR="$report_dir" \
+    DRY_RUN=true \
+    run bash "$TT_SCRIPT"
+
+  [ "$status" -eq 0 ]
+
+  # No PATCH call should have been logged
+  ! grep -qE 'api -X PATCH repos/petry-projects/\.github' "$GH_STUB_LOG"
+
+  # And the finding still lands on the remediation report, not skipped
+  grep -q 'secret_scanning_non_provider_patterns' "$report_dir/remediation-report.md"
+}
+
+# ---------------------------------------------------------------------------
+# Failure path: when the PATCH fails, the finding is reported as failed
+# rather than as a successful remediation.
+# ---------------------------------------------------------------------------
+
+@test "failed PATCH is reported as a failure" {
+  findings="$(tt_write_finding ".github" "push-protection" "secret_scanning_non_provider_patterns")"
+  report_dir="${TT_TMP}/report"
+
+  GH_TOKEN=fake \
+    GH_STUB_EXIT=1 \
+    GH_STUB_STDERR="HTTP 403: forbidden" \
+    FINDINGS_FILE="$findings" \
+    REPORT_DIR="$report_dir" \
+    DRY_RUN=false \
+    run bash "$TT_SCRIPT"
+
+  # The script's main() exits non-zero when failed_count > 0
+  [ "$status" -ne 0 ]
+
+  # And the finding is in the skipped/failed report with a FAILED prefix
+  grep -q 'FAILED' "$report_dir/skipped.md"
+}
+
+# ---------------------------------------------------------------------------
+# Catch-all preserved: other push-protection/* findings still skip
+# ---------------------------------------------------------------------------
+
+@test "non-remediated push-protection findings still hit the skip path" {
+  findings="$(tt_write_finding ".github" "push-protection" "open_secret_alerts")"
+  report_dir="${TT_TMP}/report"
+
+  GH_TOKEN=fake \
+    FINDINGS_FILE="$findings" \
+    REPORT_DIR="$report_dir" \
+    DRY_RUN=false \
+    run bash "$TT_SCRIPT"
+
+  [ "$status" -eq 0 ]
+
+  # No PATCH call should be issued for this category
+  ! grep -qE 'api -X PATCH' "$GH_STUB_LOG"
+
+  # Finding should be in the skipped report
+  grep -q 'open_secret_alerts' "$report_dir/skipped.md"
+}

test/scripts/compliance-remediate/stubs/gh

@@ -0,0 +1,45 @@
+#!/usr/bin/env bash
+# Fake `gh` binary used by compliance-remediate bats tests.
+#
+# Mirrors the pattern from test/workflows/feature-ideation/stubs/gh: behaviour
+# is fully env-driven so individual tests can script success / failure without
+# per-fixture stub variants.
+#
+# Recognized vars:
+#   GH_STUB_STDOUT  — literal text emitted on stdout (default empty)
+#   GH_STUB_STDERR  — literal text emitted on stderr (default empty)
+#   GH_STUB_EXIT    — exit code (default 0)
+#   GH_STUB_LOG     — append-only log of argv invocations, one shell-quoted
+#                     line per call (via `printf '%q '`) so tests can
+#                     re-parse losslessly.
+#   GH_STUB_STDIN_LOG — if set, the stub's stdin is appended to this file
+#                       so tests can assert on the JSON payload sent to
+#                       `gh api --input -`.
+
+set -euo pipefail
+
+if [ -n "${GH_STUB_LOG:-}" ]; then
+  # shellcheck disable=SC2059
+  printf '%q ' "$@" >>"$GH_STUB_LOG"
+  printf '\n' >>"$GH_STUB_LOG"
+fi
+
+if [ -n "${GH_STUB_STDIN_LOG:-}" ]; then
+  # Drain stdin so the calling shell isn't blocked. Empty stdin is fine.
+  # Don't suppress failures — a broken log path means broken test
+  # instrumentation, which would silently weaken payload assertions.
+  cat >>"$GH_STUB_STDIN_LOG" || {
+    echo "gh stub: failed to append stdin to GH_STUB_STDIN_LOG=$GH_STUB_STDIN_LOG" >&2
+    exit 1
+  }
+fi
+
+if [ -n "${GH_STUB_STDOUT:-}" ]; then
+  printf '%s' "$GH_STUB_STDOUT"
+fi
+
+if [ -n "${GH_STUB_STDERR:-}" ]; then
+  printf '%s' "$GH_STUB_STDERR" >&2
+fi
+
+exit "${GH_STUB_EXIT:-0}"