diff --git a/scripts/compliance-remediate.sh b/scripts/compliance-remediate.sh index d5058e454..6f02541ba 100644 --- a/scripts/compliance-remediate.sh +++ b/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" diff --git a/test/scripts/compliance-remediate/helpers/setup.bash b/test/scripts/compliance-remediate/helpers/setup.bash new file mode 100644 index 000000000..87a4281b0 --- /dev/null +++ b/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" +} diff --git a/test/scripts/compliance-remediate/security-analysis.bats b/test/scripts/compliance-remediate/security-analysis.bats new file mode 100644 index 000000000..434e4487f --- /dev/null +++ b/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" +} diff --git a/test/scripts/compliance-remediate/stubs/gh b/test/scripts/compliance-remediate/stubs/gh new file mode 100644 index 000000000..e94580ea0 --- /dev/null +++ b/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}"