diff --git a/.github/scripts/agent-shield/lib/severity-gate.sh b/.github/scripts/agent-shield/lib/severity-gate.sh new file mode 100644 index 000000000..525a62372 --- /dev/null +++ b/.github/scripts/agent-shield/lib/severity-gate.sh @@ -0,0 +1,90 @@ +#!/usr/bin/env bash +# Severity gate for the AgentShield reusable (issue #1436). +# +# The scanner's --min-severity only filters the report, and its exit code only +# reflects critical findings. This gate reads the scanner's JSON report and +# decides pass or fail at the configured threshold. +# +# Pure decision: no network calls. Anything it cannot verify fails closed. +# +# Report schema read here (ecc-agentshield 1.4.0 and 1.6.0, `--format json`): +# { "findings": [ { "severity": "critical|high|medium|low|info", "title": "...", +# "description": "...", "file": "...", "line": 12 } ], +# "summary": { "totalFindings": 1, ... } } +# +# Finding text comes from the scanned repository, so it is cleaned before it +# reaches the log, and `evidence` is never printed. +# +# Tested by test/workflows/agent-shield/severity-gate.bats. + +# agent_shield_severity_gate THRESHOLD REPORT_FILE +# Returns 0 when no finding is at or above THRESHOLD, 1 otherwise. Also returns +# 1 for an unknown threshold, or a report that is unreadable or off-schema. +agent_shield_severity_gate() { + local threshold="$1" report="$2" out verdict + + case "$threshold" in + critical | high | medium | low | info) ;; + *) + echo "::error::AgentShield: min-severity must be one of critical, high, medium, low, info" + return 1 + ;; + esac + + # -s: exactly one JSON document is accepted, so stray output around the report + # is treated as off-schema rather than ignored. + out=$(jq -rs --arg threshold "$threshold" ' + def levels: ["critical", "high", "medium", "low", "info"]; + def rank: . as $s | levels | index($s); + def safe: tostring + | gsub("[[:cntrl:]]+"; " ") + | gsub(":(?=:)"; ":​") + | gsub("##\\["; "#​#["); + def location: (.file // "unknown" | safe) + + (if .line then ":" + (.line | safe) else "" end); + + if length != 1 then "invalid\texpected one JSON document" + else .[0] as $report + | if ($report | type) != "object" or ($report.findings | type) != "array" then + "invalid\tfindings list missing" + elif ($report.summary.totalFindings // null) != ($report.findings | length) then + "invalid\tsummary does not match the findings list" + elif any($report.findings[]; + (type != "object") or ((.severity | type) != "string") or (.severity | rank) == null) then + "invalid\tfinding with an unknown severity" + else + ($threshold | rank) as $limit + | [$report.findings[] | select((.severity | rank) <= $limit)] as $hits + | if ($hits | length) == 0 then + "pass\t\($report.findings | length)" + else + "fail\t\($hits | length)", + ($hits | sort_by(.severity | rank)[] + | " [\(.severity | ascii_upcase)] \(.title // "untitled" | safe) (\(location))", + " \(.description // "" | safe)") + end + end + end + ' "$report" 2>/dev/null) || out="" + + verdict="${out%%$'\n'*}" + case "$verdict" in + pass$'\t'*) + echo "AgentShield: no findings at or above ${threshold} (${verdict#*$'\t'} reported)" + return 0 + ;; + fail$'\t'*) + echo "::error::AgentShield: ${verdict#*$'\t'} finding(s) at or above ${threshold}" + printf '%s\n' "${out#*$'\n'}" + return 1 + ;; + invalid$'\t'*) + echo "::error::AgentShield: cannot trust the scan report: ${verdict#*$'\t'}" + return 1 + ;; + *) + echo "::error::AgentShield: could not read the scan report" + return 1 + ;; + esac +} diff --git a/.github/workflows/agent-shield-reusable.yml b/.github/workflows/agent-shield-reusable.yml index 1cfb353bc..c9c84fdb1 100644 --- a/.github/workflows/agent-shield-reusable.yml +++ b/.github/workflows/agent-shield-reusable.yml @@ -71,7 +71,40 @@ jobs: npx --prefix="$npm_prefix" --ignore-scripts "ecc-agentshield@${AS_VERSION}" scan \ --path "$scan_target" \ --min-severity "${AS_SEVERITY}" \ - --format terminal + --format json > "$RUNNER_TEMP/agentshield-report.json" || scan_rc=$? + # Exit 2 only means the report has critical findings, which the gate + # below handles. Any other non-zero exit is a scanner failure. + case "${scan_rc:-0}" in + 0 | 2) ;; + *) + echo "::error::AgentShield scanner failed (exit ${scan_rc})" + exit "$scan_rc" + ;; + esac + + # The scanner's exit code ignores min-severity, so the threshold is enforced + # here from its JSON report (#1436). Checked out after the scan so the tooling + # is never part of the scan target, and sparse so later checks ignore it. + - name: Checkout AgentShield gate tooling + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + repository: combsmith/.github + # This reusable's own commit, so the gate matches the channel the caller + # pinned. `job.workflow_sha`, not `github.job_workflow_sha`, which is + # empty at step level (#839). + ref: ${{ job.workflow_sha }} + path: .agent-shield-tooling + sparse-checkout: .github/scripts/agent-shield + fetch-depth: 1 + persist-credentials: false + + - name: Enforce AgentShield severity threshold + env: + AS_SEVERITY: ${{ inputs.min-severity }} + run: | + # shellcheck source=/dev/null + . .agent-shield-tooling/.github/scripts/agent-shield/lib/severity-gate.sh + agent_shield_severity_gate "$AS_SEVERITY" "$RUNNER_TEMP/agentshield-report.json" # --- Org-specific structural checks --- - name: Check required agent files exist diff --git a/.github/workflows/agent-shield-tests.yml b/.github/workflows/agent-shield-tests.yml new file mode 100644 index 000000000..449f746c2 --- /dev/null +++ b/.github/workflows/agent-shield-tests.yml @@ -0,0 +1,47 @@ +# Quality gates for the AgentShield reusable and its severity gate (issue #1436). +name: AgentShield Gate Tests + +on: + pull_request: + paths: + - '.github/workflows/agent-shield-*.yml' + - '.github/scripts/agent-shield/**' + - 'test/workflows/agent-shield/**' + push: + branches: + - main + paths: + - '.github/workflows/agent-shield-*.yml' + - '.github/scripts/agent-shield/**' + - 'test/workflows/agent-shield/**' + +permissions: + contents: read + +concurrency: + group: agent-shield-tests-${{ github.ref }} + cancel-in-progress: true + +jobs: + test: + name: Lint and bats + runs-on: ubuntu-latest + timeout-minutes: 10 + steps: + - name: Checkout repository + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + fetch-depth: 1 + persist-credentials: false + + - name: Install bats, shellcheck, and jq (yq is preinstalled on the runner) + run: | + set -euo pipefail + sudo apt-get update -qq + sudo apt-get install -y --no-install-recommends bats shellcheck jq + + - name: shellcheck + run: shellcheck -x .github/scripts/agent-shield/lib/severity-gate.sh + + - name: Run bats suite + run: bats --print-output-on-failure test/workflows/agent-shield/ diff --git a/standards/canary-rings.json b/standards/canary-rings.json index ff477e3d1..4c40bb5c3 100644 --- a/standards/canary-rings.json +++ b/standards/canary-rings.json @@ -100,7 +100,7 @@ } ], "_suspect_note": "suspect_failure_classes (#668 increment 2): same shape as benign classes (`id`, `workflow` ERE, `step` ERE) plus a `guidance` discriminating question. A counted (non-benign) failure that matches a suspect class at differs=1 is triaged SUSPECT instead of REGRESSION \u2014 it still BLOCKS and still needs a human (labelled `needs-human`, NOT auto-cleared like `version_independent`), but the blocker issue renders the `guidance` so the human confirm is a 30-second check rather than a from-scratch investigation. Default-absent (this key omitted) = today's behavior, byte-identical for agents that do not opt in.", - "_confirm_note": "require_confirmation (#668 increment 3, Layer 3): opt-in human go/no-go at ONE ring boundary for a correctness-sensitive agent. When a transition sets `require_confirmation: true` and the reliability verdict is otherwise PROMOTE, the frontier holds in state AWAITING_CONFIRMATION \u2014 the scheduled promote-all never auto-advances it; a deliberate `promote --confirm` dispatch (Actions-audit-logged) does. The confirmation IS the dispatch (no state store). `--confirm` is NOT `--override`: it advances ONLY a reliability-clean AWAITING_CONFIRMATION state and can never bypass a BLOCKED gate. sync-issues files an evidence-carrying `canary-confirm:` issue (compare diff link + run stats; labelled `needs-human`) that auto-closes on promote or candidate change. dev-lead only initially; reliability still clears next/ring0/ring1 unattended. Default-absent (this key omitted) = fully autonomous, byte-identical for agents that do not opt in.", + "_confirm_note": "require_confirmation (#668 increment 3, Layer 3): opt-in human go/no-go at ONE ring boundary for a correctness-sensitive agent. When a transition sets `require_confirmation: true` and the reliability verdict is otherwise PROMOTE, the frontier holds in state AWAITING_CONFIRMATION \u2014 the scheduled promote-all never auto-advances it; a deliberate `promote --confirm` dispatch (Actions-audit-logged) does. The confirmation IS the dispatch (no state store). `--confirm` is NOT `--override`: it advances ONLY a reliability-clean AWAITING_CONFIRMATION state and can never bypass a BLOCKED gate. sync-issues files an evidence-carrying `canary-confirm:` issue (compare diff link + run stats; labelled `needs-human`) that auto-closes on promote or candidate change. dev-lead initially; agent-shield also opts in because a release can make its required check fail on findings that already exist in stable-only repos; reliability still clears next/ring0/ring1 unattended. Default-absent (this key omitted) = fully autonomous, byte-identical for agents that do not opt in.", "transitions": { "next->ring0": { "dwell_hours": 4, @@ -179,7 +179,8 @@ }, "ring1->stable": { "dwell_hours": 12, - "sample_min": 1 + "sample_min": 1, + "require_confirmation": true } } } diff --git a/test/workflows/agent-shield/severity-gate.bats b/test/workflows/agent-shield/severity-gate.bats new file mode 100644 index 000000000..218c86c1a --- /dev/null +++ b/test/workflows/agent-shield/severity-gate.bats @@ -0,0 +1,238 @@ +#!/usr/bin/env bats +# combsmith/.github#1436: the AgentShield job must fail at the configured +# min-severity, using the scanner's JSON report rather than its exit code. + +bats_require_minimum_version 1.5.0 + +REPO_ROOT="$(cd "$BATS_TEST_DIRNAME/../../.." && pwd)" +WORKFLOW="$REPO_ROOT/.github/workflows/agent-shield-reusable.yml" + +setup() { + # shellcheck source=/dev/null + . "$REPO_ROOT/.github/scripts/agent-shield/lib/severity-gate.sh" + REPORT="$BATS_TEST_TMPDIR/report.json" +} + +# report_with SEVERITY... — writes a report with one finding per severity. +report_with() { + local sev findings="[]" + for sev in "$@"; do + findings=$(jq -c --arg s "$sev" \ + '. + [{id: "x", severity: $s, category: "misconfiguration", title: ($s + " finding"), + description: "why it matters", file: "CLAUDE.md", line: 3, + evidence: "EVIDENCE-VALUE", fix: {description: "fix it", auto: false}}]' <<<"$findings") + done + jq -n --argjson f "$findings" \ + '{timestamp: "2026-10-30T00:00:00.000Z", targetPath: "/repo", findings: $f, + score: {grade: "A", numericScore: 100}, + summary: {totalFindings: ($f | length), filesScanned: 2, autoFixable: 0}}' >"$REPORT" +} + +@test "a high finding fails at the default threshold" { + report_with high + run agent_shield_severity_gate high "$REPORT" + [ "$status" -eq 1 ] + [[ "$output" == *"::error::AgentShield: 1 finding(s) at or above high"* ]] + [[ "$output" == *"[HIGH] high finding (CLAUDE.md:3)"* ]] +} + +@test "a critical finding fails at every threshold" { + report_with critical + local t + for t in critical high medium low info; do + run agent_shield_severity_gate "$t" "$REPORT" + [ "$status" -eq 1 ] + done +} + +@test "a finding below the threshold passes" { + report_with medium low info + run agent_shield_severity_gate high "$REPORT" + [ "$status" -eq 0 ] + [[ "$output" == *"no findings at or above high (3 reported)"* ]] +} + +@test "the threshold is inclusive at each level" { + local pairs=("critical high" "high medium" "medium low" "low info") pair + for pair in "${pairs[@]}"; do + report_with "${pair% *}" + run agent_shield_severity_gate "${pair% *}" "$REPORT" + [ "$status" -eq 1 ] + report_with "${pair#* }" + run agent_shield_severity_gate "${pair% *}" "$REPORT" + [ "$status" -eq 0 ] + done + report_with info + run agent_shield_severity_gate info "$REPORT" + [ "$status" -eq 1 ] +} + +@test "a clean report passes" { + report_with + run agent_shield_severity_gate high "$REPORT" + [ "$status" -eq 0 ] + [[ "$output" == *"(0 reported)"* ]] +} + +@test "findings are listed most severe first and evidence is not printed" { + report_with high critical + run agent_shield_severity_gate high "$REPORT" + [ "$status" -eq 1 ] + [[ "$output" == *"[CRITICAL]"*"[HIGH]"* ]] + [[ "$output" != *"EVIDENCE-VALUE"* ]] +} + +@test "an unknown threshold fails, even with a clean report" { + report_with + local t + for t in "" HIGH severe "high "; do + run agent_shield_severity_gate "$t" "$REPORT" + [ "$status" -eq 1 ] + [[ "$output" == *"min-severity must be one of"* ]] + done +} + +@test "a missing, empty or non-JSON report fails" { + run agent_shield_severity_gate high "$BATS_TEST_TMPDIR/missing.json" + [ "$status" -eq 1 ] + : >"$REPORT" + run agent_shield_severity_gate high "$REPORT" + [ "$status" -eq 1 ] + echo 'npm warn something' >"$REPORT" + run agent_shield_severity_gate high "$REPORT" + [ "$status" -eq 1 ] + [[ "$output" == *"::error::AgentShield:"* ]] +} + +@test "stray output around the report fails" { + report_with + cp "$REPORT" "$BATS_TEST_TMPDIR/one.json" + cat "$BATS_TEST_TMPDIR/one.json" "$BATS_TEST_TMPDIR/one.json" >"$REPORT" + run agent_shield_severity_gate high "$REPORT" + [ "$status" -eq 1 ] + [[ "$output" == *"cannot trust the scan report"* ]] +} + +@test "a report that is off-schema fails" { + local doc + for doc in \ + '[]' \ + '{}' \ + '{"summary":{"totalFindings":0}}' \ + '{"findings":{},"summary":{"totalFindings":0}}' \ + '{"findings":[]}' \ + '{"findings":[],"summary":{"totalFindings":1}}' \ + '{"findings":["high"],"summary":{"totalFindings":1}}' \ + '{"findings":[{"title":"no severity"}],"summary":{"totalFindings":1}}' \ + '{"findings":[{"severity":"severe"}],"summary":{"totalFindings":1}}' \ + '{"findings":[{"severity":3}],"summary":{"totalFindings":1}}'; do + printf '%s\n' "$doc" >"$REPORT" + run agent_shield_severity_gate info "$REPORT" + [ "$status" -eq 1 ] + [[ "$output" == *"cannot trust the scan report"* ]] + done +} + +@test "finding text cannot produce workflow commands or break out of its line" { + jq -n '{findings: [{severity: "high", + title: "a\n::error::forged ##[stop-commands]x :::y", + description: "\u001b[31m##[group]z\r\n::warning::w", + file: "x\n::notice::f", line: 1}], + summary: {totalFindings: 1}}' >"$REPORT" + run agent_shield_severity_gate high "$REPORT" + [ "$status" -eq 1 ] + [ "$(grep -c '^[[:space:]]*::' <<<"$output")" -eq 1 ] + [[ "${lines[0]}" == "::error::AgentShield: 1 finding(s) at or above high" ]] + [[ "$output" != *"##["* ]] + [ "${#lines[@]}" -eq 3 ] + [[ "${lines[1]}" != *"::"* ]] + [[ "${lines[2]}" != *"::"* ]] + [[ "$output" != *$'\033'* ]] +} + +# --- The reusable's real steps, against a stubbed scanner --- + +# run_job_steps STUB_EXIT REPORT_JSON [THRESHOLD] — runs the scan and gate steps +# in a fake checkout, with a stub npx that prints REPORT_JSON and exits STUB_EXIT. +run_job_steps() { + local stub_exit="$1" report_json="$2" threshold="${3:-high}" + local tmp="$BATS_TEST_TMPDIR" scan gate + scan=$(yq '.jobs["agent-shield"].steps[] | select(.name == "AgentShield Security Scan") | .run' "$WORKFLOW") + gate=$(yq '.jobs["agent-shield"].steps[] | select(.name == "Enforce AgentShield severity threshold") | .run' "$WORKFLOW") + [ -n "$scan" ] && [ "$scan" != "null" ] + [ -n "$gate" ] && [ "$gate" != "null" ] + + mkdir -p "$tmp/bin" "$tmp/work" "$tmp/runner-temp" + ln -sfn "$REPO_ROOT" "$tmp/work/.agent-shield-tooling" + printf '%s\n' "$report_json" >"$tmp/stub-report.json" + cat >"$tmp/bin/npx" <"$tmp/npx.args" +cat "$tmp/stub-report.json" +exit $stub_exit +STUB + chmod +x "$tmp/bin/npx" + printf '%s\n' "$scan" >"$tmp/scan.sh" + printf '%s\n' "$gate" >"$tmp/gate.sh" + + # Each step runs the way the runner runs it: its own `bash -e`, stopping the job. + run env RUNNER_TEMP="$tmp/runner-temp" AS_VERSION=1.4.0 AS_SEVERITY="$threshold" \ + PATH="$tmp/bin:$PATH" bash -c \ + 'cd "$1/work" && bash -e "$1/scan.sh" && bash -e "$1/gate.sh"' _ "$tmp" +} + +REPORT_HIGH='{"findings":[{"severity":"high","title":"t","description":"d","file":"f"}],"summary":{"totalFindings":1}}' +REPORT_CRITICAL='{"findings":[{"severity":"critical","title":"t","description":"d","file":"f"}],"summary":{"totalFindings":1}}' +REPORT_CLEAN='{"findings":[],"summary":{"totalFindings":0}}' + +@test "job: the scanner exits 0 with a high finding, and the job still fails" { + run_job_steps 0 "$REPORT_HIGH" + [ "$status" -eq 1 ] + [[ "$output" == *"1 finding(s) at or above high"* ]] +} + +@test "job: the scanner exits 2 for a critical finding, and the gate reports it" { + run_job_steps 2 "$REPORT_CRITICAL" + [ "$status" -eq 1 ] + [[ "$output" == *"[CRITICAL]"* ]] +} + +@test "job: a clean scan passes" { + run_job_steps 0 "$REPORT_CLEAN" + [ "$status" -eq 0 ] + [[ "$output" == *"no findings at or above high"* ]] +} + +@test "job: a scanner failure stops the job with the scanner's exit code" { + run_job_steps 4 "$REPORT_CLEAN" + [ "$status" -eq 4 ] + [[ "$output" == *"AgentShield scanner failed (exit 4)"* ]] +} + +@test "job: a scanner that prints no report fails" { + run_job_steps 0 "" + [ "$status" -eq 1 ] +} + +@test "job: the scan keeps --ignore-scripts and asks for JSON at the threshold" { + run_job_steps 0 "$REPORT_CLEAN" medium + [ "$status" -eq 0 ] + local args + args=$(cat "$BATS_TEST_TMPDIR/npx.args") + [[ "$args" == *"--ignore-scripts"* ]] + [[ "$args" == *"--min-severity medium"* ]] + [[ "$args" == *"--format json"* ]] +} + +@test "reusable: the gate tooling comes from the reusable's own commit, after the scan" { + local names + names=$(yq -r '.jobs["agent-shield"].steps[].name // "checkout"' "$WORKFLOW") + [ "$(sed -n 2p <<<"$names")" = "AgentShield Security Scan" ] + [ "$(sed -n 3p <<<"$names")" = "Checkout AgentShield gate tooling" ] + [ "$(sed -n 4p <<<"$names")" = "Enforce AgentShield severity threshold" ] + local step='.jobs["agent-shield"].steps[] | select(.name == "Checkout AgentShield gate tooling") | .with' + # shellcheck disable=SC2016 # a literal GitHub expression, not a shell expansion + [ "$(yq -r "$step.ref" "$WORKFLOW")" = '${{ job.workflow_sha }}' ] + [ "$(yq -r "$step.persist-credentials" "$WORKFLOW")" = "false" ] + [ "$(yq -r "$step.sparse-checkout" "$WORKFLOW")" = ".github/scripts/agent-shield" ] +} diff --git a/tests/canary_rollout.bats b/tests/canary_rollout.bats index 5589b3c39..a10fe8eba 100644 --- a/tests/canary_rollout.bats +++ b/tests/canary_rollout.bats @@ -3808,11 +3808,13 @@ _sync_gap_stub() { # evidence-carrying `canary-confirm` issue (compare diff link; needs-human). Default-absent # (transition without the key) = fully autonomous, byte-identical for opted-out agents. -@test "canary-rings.json: dev-lead ring1->stable opts into require_confirmation; opted-out agents default-off" { +@test "canary-rings.json: dev-lead and agent-shield ring1->stable opt into require_confirmation; opted-out agents default-off" { run jq -e '.agents["dev-lead"].gate.transitions["ring1->stable"].require_confirmation == true' "$RINGS" [ "$status" -eq 0 ] + run jq -e '.agents["agent-shield"].gate.transitions["ring1->stable"].require_confirmation == true' "$RINGS" + [ "$status" -eq 0 ] # Every OTHER agent's ring1->stable must NOT carry the key (absent → today's autonomous behaviour). - run jq -e '[.agents | to_entries[] | select(.key != "dev-lead") + run jq -e '[.agents | to_entries[] | select(.key != "dev-lead" and .key != "agent-shield") | .value.gate.transitions["ring1->stable"].require_confirmation] | all(. == null)' "$RINGS" [ "$status" -eq 0 ] }