Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 25 additions & 0 deletions .github/claude-skip-actors
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
# Repo-owned skip-actors list: ADR 0002's ratified skip-actor exception — the
# actors for whom the claude-review and claude-security-review lanes skip
# entirely. One actor login per line; `#` comments and blank lines are
# ignored. scripts/read-skip-actors.sh joins the entries into the
# comma-separated form the ci-workflows reusable workflows take, and it is the
# ONLY statement of the list: both caller workflows read it through that
# script, and both evidence guards (verify-claude-review-skill.sh,
# verify-security-review-evidence.sh) default from it. The list used to be
# restated verbatim at five sites, and the restatement drifted exactly as
# restatements do — 2b4d8abf added cursor[bot] to both workflow lines and left
# verify-security-review-evidence.sh's copy behind.
#
# ADDING an actor here admits it to a PRIVILEGED skip on security-sensitive
# paths and is an ADR 0002 decision, not a config tweak. REMOVING an actor is
# never a one-line change either: the security lane fails CLOSED for an actor
# the reusable's `allowed_bots` does not permit, so un-skipping for real means
# widening `allowed_bots` upstream first (ADR 0002, "skip-actors and the
# action's allowed_bots are different levers"). Removing `dependabot[bot]`
# additionally requires mirroring CLAUDE_CODE_OAUTH_TOKEN into the separate
# Dependabot secrets store — see the reusable's `skip-actors` input docs.
dependabot[bot]
claude[bot]
melodic-ai[bot]
melodic-standards-sync[bot]
cursor[bot]
15 changes: 14 additions & 1 deletion .github/workflows/claude-review.yml
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,19 @@ jobs:
persist-credentials: false
- name: Run claude-review skill-evidence guard tests
run: bash scripts/verify-claude-review-skill.sh.test.sh
# The skip-actors exception lives in .github/claude-skip-actors — the one
# statement of ADR 0002's ratified list, read through its one parser. The
# value used to be restated inline here (and at four sibling sites), and
# the restatement drifted (2b4d8abf).
# Plain assignment first, never inside the echo: a command substitution
# in echo's argument returns ECHO's status under bash -e, so a parser
# failure would emit `list=` and the step would stay green. The
# assignment form fails the step the moment the read fails.
- name: Read the ratified skip-actors list
id: skip-actors
run: |
list="$(scripts/read-skip-actors.sh)"
echo "list=$list" >> "$GITHUB_OUTPUT"
- name: Verify claude-review invoked the skill
env:
GH_TOKEN: ${{ github.token }}
Expand All @@ -74,5 +87,5 @@ jobs:
EVENT_HEAD_SHA: ${{ github.event.pull_request.head.sha }}
LANE_RESULT: ${{ needs.review.result }}
REVIEWER_LOGINS: claude[bot]
SKIP_ACTORS: dependabot[bot],claude[bot],melodic-ai[bot],melodic-standards-sync[bot],cursor[bot]
SKIP_ACTORS: ${{ steps.skip-actors.outputs.list }}
run: bash scripts/verify-claude-review-skill.sh
85 changes: 77 additions & 8 deletions .github/workflows/claude-security-review.yml
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,64 @@ concurrency:
cancel-in-progress: false

jobs:
# The ratified skip-actors list lives in .github/claude-skip-actors — the
# one statement of ADR 0002's skip-actor exception, formerly restated at
# five sites (which drifted: 2b4d8abf). A workflow-call `with:` value cannot
# read a file, so this job reads the list and hands it across `needs`.
#
# It reads the PR's BASE copy of the list, the same discipline as the
# reusable's `paths-file` — and that is CONTENT protection only. On a
# pull_request event this workflow file, scripts/read-skip-actors.sh, and
# the checkout-with-base composite all come from the PR HEAD, so a head
# that rewrites the parser can still feed the lane a different list. That
# is the pre-existing trust boundary, not a regression (the literal used to
# live in this same head-controlled file), and it stays bounded by the
# reusable's allowed_bots gate on who can trigger a privileged run; the
# base read closes the cheap one-line list edit, not a determined head
# rewrite. A base that predates the file falls back to the head copy (the
# adopting PR itself), announced with a ::notice rather than silently. A
# failed read here skips the lane, which a ruleset would read as success —
# the evidence job below fails closed on that shape.
skip-actors:
runs-on: ubuntu-24.04
timeout-minutes: 5
outputs:
list: ${{ steps.read.outputs.list }}
steps:
- name: Check out
uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
with:
persist-credentials: false
- name: Fetch base
uses: ./.github/actions/checkout-with-base
# Each fallback is its own announced branch, never a silent 2>/dev/null
# catch-all: an unresolvable base ref is a hard failure (on pull_request
# the composite above fetched it, so its absence means something is
# broken, not merely pre-adoption), while the list file absent at a
# resolvable base is the one legitimate head-copy fallback — the
# adopting PR. The parser reads are plain assignments so a read failure
# fails the step under bash -e.
- name: Read the ratified skip-actors list
id: read
env:
BASE_REF: ${{ github.base_ref }}
run: |
src=".github/claude-skip-actors"
if [ -z "$BASE_REF" ] || ! git rev-parse --verify --quiet "origin/$BASE_REF" >/dev/null; then
echo "::error::base ref '${BASE_REF:-<empty>}' is unavailable; refusing to guess which skip-actors list applies"
exit 1
fi
if git cat-file -e "origin/$BASE_REF:$src" 2>/dev/null; then
git show "origin/$BASE_REF:$src" >"$RUNNER_TEMP/claude-skip-actors"
list="$(scripts/read-skip-actors.sh "$RUNNER_TEMP/claude-skip-actors")"
else
echo "::notice::$src absent at origin/$BASE_REF (pre-adoption base); reading the head copy"
list="$(scripts/read-skip-actors.sh)"
fi
echo "list=$list" >> "$GITHUB_OUTPUT"

security-review:
needs: skip-actors
permissions:
contents: read # checkout + read the diff
pull-requests: write # post the security review
Expand All @@ -37,14 +94,16 @@ jobs:
paths-file: .github/claude-security-paths
# Stated explicitly, never left to the reusable's default: this list IS
# ADR 0002's skip-actor exception, and the exception must be readable and
# reviewable in the repo it applies to. An inherited default silently
# rewrites the exception whenever ci-workflows changes it — which is how
# `claude[bot]` and `melodic-ai[bot]` entered it (#1766 dropped this line
# while re-pinning to a version whose default had widened).
# reviewable in the repo it applies to — it lives in
# .github/claude-skip-actors, read by the `skip-actors` job above. An
# inherited default silently rewrites the exception whenever ci-workflows
# changes it — which is how `claude[bot]` and `melodic-ai[bot]` entered
# it (#1766 dropped this line while re-pinning to a version whose default
# had widened).
#
# DO NOT delete this line at a re-pin, and do not "simplify" it away.
# Deleting it is not a no-op even when the value matches the upstream
# default of the moment: the four actors here are ADR 0002's 2026-08-04
# default of the moment: the actors in that file are ADR 0002's
# ratified baseline, and inheriting instead re-delegates that decision to
# whatever ci-workflows ships next. Every actor in an inherited default
# satisfies this repo's REQUIRED `security-review / security-review`
Expand All @@ -64,7 +123,7 @@ jobs:
# not. runner-policy rejects inputs outside the reviewed contract but
# cannot REQUIRE one, so dropping this line re-widens the exception with
# nothing said. That residual gap is standards#308.
skip-actors: dependabot[bot],claude[bot],melodic-ai[bot],melodic-standards-sync[bot],cursor[bot]
skip-actors: ${{ needs.skip-actors.outputs.list }}
# One named secret (least privilege), never `secrets: inherit`.
secrets:
CLAUDE_CODE_OAUTH_TOKEN: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }}
Expand All @@ -86,7 +145,7 @@ jobs:
# pin carries the outputs needs a version they can compare against, and the
# guard's own message says the same thing the same way.
security-review-evidence:
needs: security-review
needs: [skip-actors, security-review]
if: >-
always() &&
github.event_name == 'pull_request' &&
Expand All @@ -104,6 +163,16 @@ jobs:
uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
with:
persist-credentials: false
# A failed skip-actors read skips the lane, and a ruleset reads a skipped
# required check as success — so this supplement reddens that shape
# rather than letting a broken read double as an off switch.
- name: Fail closed when the skip-actors read failed
if: needs.skip-actors.result != 'success'
env:
SKIP_ACTORS_RESULT: ${{ needs.skip-actors.result }}
run: |
echo "::error::the skip-actors read job concluded '$SKIP_ACTORS_RESULT'; the security lane cannot have received the ratified list"
exit 1
# Self-test first, so a broken guard cannot mask the regression it exists
# to catch — the shape #2517 was: the guard agreed with itself while
# failing every pull request.
Expand All @@ -122,5 +191,5 @@ jobs:
LANE_REVIEW_RAN: ${{ needs.security-review.outputs.review-ran }}
LANE_REVIEW_FAILED: ${{ needs.security-review.outputs.review-failed }}
LANE_FAILURE_CLASS: ${{ needs.security-review.outputs.failure-class }}
SKIP_ACTORS: dependabot[bot],claude[bot],melodic-ai[bot],melodic-standards-sync[bot],cursor[bot]
SKIP_ACTORS: ${{ needs.skip-actors.outputs.list }}
run: bash scripts/verify-security-review-evidence.sh
54 changes: 54 additions & 0 deletions scripts/read-skip-actors.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
#!/usr/bin/env bash
# Print the ratified review skip-actors list in the comma-separated, no-spaces
# form the ci-workflows reusable workflows take.
#
# scripts/read-skip-actors.sh [<file>]
#
# The list itself lives in .github/claude-skip-actors (one actor per line) —
# the single statement of ADR 0002's skip-actor exception; this script owns
# the one parse of it. Consumers: the claude-security-review caller reads the
# lane's `skip-actors` input through it, the claude-review caller reads its
# evidence guard's SKIP_ACTORS through it, and both evidence guards default
# from it when SKIP_ACTORS is not in the environment. <file> overrides the
# committed path so a caller can hand in a copy read from another ref (the
# security caller reads the PR's BASE copy, mirroring the reusable's
# `paths-file` discipline).
#
# FAIL CLOSED ON SHAPE: a missing file, an empty active set, or an entry that
# could corrupt the comma-joined form (embedded comma or whitespace) exits 2
# with nothing on stdout. Printing an empty or mangled list would silently
# rewrite the exception, which is the drift this file replaced.
#
# Exit: 0 list printed; 2 usage, unreadable file, or a malformed entry.
set -uo pipefail

SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" || exit 2
# shellcheck source=lib/read-list.sh
. "$SCRIPT_DIR/lib/read-list.sh" || exit 2

if [[ $# -gt 1 ]]; then
echo "usage: $(basename "$0") [<file>]" >&2
exit 2
fi
file="${1:-$SCRIPT_DIR/../.github/claude-skip-actors}"

actors=()
# `inline`: entries are actor logins, never regexes, and a login cannot
# contain `#`.
read_list::into actors "$file" --comments inline || exit 2

if [[ ${#actors[@]} -eq 0 ]]; then
echo "read-skip-actors: $file names no actors; an empty exception must be an explicit consumer decision, not a parsed-away file" >&2
exit 2
fi
for actor in "${actors[@]}"; do
if [[ "$actor" == *[,[:space:]]* ]]; then
echo "read-skip-actors: malformed entry (comma or whitespace) in $file: $actor" >&2
exit 2
fi
done

(
IFS=,
printf '%s\n' "${actors[*]}"
)
66 changes: 66 additions & 0 deletions scripts/read-skip-actors.test.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
#!/usr/bin/env bash
# Unit tests for read-skip-actors.sh. Synthetic list files exercise the parse
# and every fail-closed shape; one case reads the real committed list so the
# published form stays a live assertion, not an example.
set -uo pipefail

SELF_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
SCRIPT="$SELF_DIR/read-skip-actors.sh"

# shellcheck source=lib/test-harness.sh
. "$SELF_DIR/lib/test-harness.sh"

TMP="$(mktemp -d)"
trap 'rm -rf "$TMP"' EXIT

write_list() {
printf '%s\n' "$@" >"$TMP/list"
}

# --- comments and blanks are ignored; entries join with commas --------------
write_list "# ratified" "" "dependabot[bot]" "cursor[bot] # org-installed" ""
if out="$(bash "$SCRIPT" "$TMP/list" 2>&1)" && [[ "$out" == "dependabot[bot],cursor[bot]" ]]; then
ok "joins active entries with commas, dropping comments and blanks"
else
fail "expected 'dependabot[bot],cursor[bot]', got: $out"
fi

# --- a single entry prints with no trailing comma ---------------------------
write_list "claude[bot]"
if out="$(bash "$SCRIPT" "$TMP/list" 2>&1)" && [[ "$out" == "claude[bot]" ]]; then
ok "a single entry prints bare"
else
fail "expected 'claude[bot]', got: $out"
fi

# --- fail-closed shapes exit 2 with nothing on stdout -----------------------
expect_exit_2() {
local label="$1"
shift
local out status=0
out="$(bash "$SCRIPT" "$@" 2>/dev/null)" || status=$?
if [[ "$status" -eq 2 && -z "$out" ]]; then
ok "$label exits 2 with empty stdout"
else
fail "$label should exit 2 with empty stdout, got status $status, out: $out"
fi
}

expect_exit_2 "a missing file" "$TMP/no-such-file"
write_list "# only commentary" ""
expect_exit_2 "an empty active set" "$TMP/list"
write_list "dependabot[bot],cursor[bot]"
expect_exit_2 "an embedded comma" "$TMP/list"
write_list "depend abot[bot]"
expect_exit_2 "embedded whitespace" "$TMP/list"
write_list "claude[bot]"
expect_exit_2 "extra arguments" "$TMP/list" surplus

# --- the committed list parses and carries the ratified five ----------------
if out="$(bash "$SCRIPT" 2>&1)" && [[ "$out" == *"cursor[bot]"* && "$out" != *" "* ]]; then
ok "the committed .github/claude-skip-actors parses to a spaceless list naming cursor[bot]"
else
fail "committed list should parse and name cursor[bot], got: $out"
fi

test_harness::report
11 changes: 9 additions & 2 deletions scripts/verify-claude-review-skill.sh
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@
# Environment:
# GITHUB_EVENT_NAME pull_request expected; anything else is not applicable
# GITHUB_ACTOR PR author login
# SKIP_ACTORS comma-separated actors exempt from review
# SKIP_ACTORS comma-separated actors exempt from review (default:
# .github/claude-skip-actors via read-skip-actors.sh)
# LANE_RESULT needs.review.result
# GITHUB_REPOSITORY owner/repo
# PR_NUMBER pull request number
Expand All @@ -29,7 +30,13 @@

set -euo pipefail

SKIP_ACTORS="${SKIP_ACTORS:-dependabot[bot],claude[bot],melodic-ai[bot],melodic-standards-sync[bot],cursor[bot]}"
# The default is the ratified list in .github/claude-skip-actors, read through
# its one parser — never a restated literal: the sibling security guard's
# literal drifted behind the workflow lines exactly that way (2b4d8abf added
# cursor[bot] everywhere but there).
if [[ -z "${SKIP_ACTORS:-}" ]]; then
SKIP_ACTORS="$("$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)/read-skip-actors.sh")" || exit 2
fi
REVIEWER_LOGINS="${REVIEWER_LOGINS:-claude[bot]}"

usage() {
Expand Down
11 changes: 9 additions & 2 deletions scripts/verify-security-review-evidence.sh
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,8 @@
# Environment:
# GITHUB_EVENT_NAME pull_request expected; anything else is not applicable
# GITHUB_ACTOR PR author login
# SKIP_ACTORS comma-separated actors exempt from review
# SKIP_ACTORS comma-separated actors exempt from review (default:
# .github/claude-skip-actors via read-skip-actors.sh)
# LANE_RESULT needs.security-review.result
# LANE_RELEVANT the lane's `relevant` output
# LANE_REVIEW_RAN the lane's `review-ran` output
Expand All @@ -44,7 +45,13 @@

set -euo pipefail

SKIP_ACTORS="${SKIP_ACTORS:-dependabot[bot],claude[bot],melodic-ai[bot],melodic-standards-sync[bot]}"
# The default is the ratified list in .github/claude-skip-actors, read through
# its one parser — never a restated literal: this guard's literal is the copy
# that drifted (2b4d8abf added cursor[bot] to both workflow lines and left it
# out here).
if [[ -z "${SKIP_ACTORS:-}" ]]; then
SKIP_ACTORS="$("$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)/read-skip-actors.sh")" || exit 2
fi

usage() {
cat <<'EOF'
Expand Down
Loading