diff --git a/.devcontainer/Dockerfile b/.devcontainer/Dockerfile index 6e81662..02524de 100644 --- a/.devcontainer/Dockerfile +++ b/.devcontainer/Dockerfile @@ -1,22 +1,20 @@ -# syntax=docker/dockerfile:1.28 +# syntax=docker/dockerfile:1.28@sha256:bb22d9815c728170f72750f4e5b0d672e06176142e1d602c7e66c050100b7e5b # Single-stage build: develop (the devcontainer image). Built with the repo # root as context (see compose.yml). The repo-root Dockerfile is reserved for # an instance's own runtime image -- see docs/TEMPLATE.md's "Contents" and # "Instance extension points". -ARG DEBIAN_VERSION=trixie - # renovate: datasource=github-releases depName=j178/prek -ARG PREK_VERSION=0.5.7 +ARG PREK_VERSION=0.5.9 # renovate: datasource=npm depName=@anthropic-ai/claude-code -ARG CLAUDE_CODE_VERSION=2.1.310 +ARG CLAUDE_CODE_VERSION=2.1.316 # renovate: datasource=github-releases depName=edouard-claude/snip ARG SNIP_VERSION=0.25.2 # renovate: datasource=github-releases depName=cli/cli -ARG GH_VERSION=2.103.2 +ARG GH_VERSION=2.103.3 # uv itself isn't a project dependency manager here -- it's only the # mechanism scripts/develop.sh uses to install an exact, checksum-verified @@ -24,7 +22,7 @@ ARG GH_VERSION=2.103.2 # package. Left on PATH afterward; harmless, and an instance that adds # Python as its own application runtime can reuse it directly. # renovate: datasource=github-releases depName=astral-sh/uv -ARG UV_VERSION=0.13.5 +ARG UV_VERSION=0.13.6 # python3 is infrastructure tooling, not an application runtime this # template assumes: prek needs an interpreter to build the venv for any @@ -37,7 +35,7 @@ ARG PYTHON_VERSION=3.14.8 # (the clear-thought MCP server in .mcp.json) is the only thing that # needs it. # renovate: datasource=node-version depName=node -ARG NODE_VERSION=24.22.1 +ARG NODE_VERSION=24.22.2 ARG SSL_CERT_FILE=/etc/ssl/certs/ca-certificates.crt ARG SSL_CERT_DIR=/etc/ssl/certs @@ -51,7 +49,11 @@ ARG SSL_CERT_DIR=/etc/ssl/certs # this stage does install are infrastructure tooling only -- see the ARG # comments above. ######################################## -FROM mcr.microsoft.com/devcontainers/base:${DEBIAN_VERSION} AS develop +# A literal tag plus digest, not an ARG: Renovate's dockerfile manager then +# bumps the tag and re-pins the digest together (pinDigests in +# .github/renovate.json). The syntax frontend on line 1 is pinned the same +# way. Digests are of the multi-arch index (amd64 and arm64). +FROM mcr.microsoft.com/devcontainers/base:trixie@sha256:71b6fe7c4e07ec747a94a72b2a17f2f8c286eaa7b1c59862888df764efb8b6c6 AS develop ARG PREK_VERSION ARG CLAUDE_CODE_VERSION ARG SNIP_VERSION diff --git a/.devcontainer/README.md b/.devcontainer/README.md index f8933a4..c06a683 100644 --- a/.devcontainer/README.md +++ b/.devcontainer/README.md @@ -5,7 +5,9 @@ (features, mounts, editor settings). - `Dockerfile` — the devcontainer image (a single `develop` stage), built with the repo root as context; `Dockerfile.dockerignore` holds its - build-context excludes. The top-level `Dockerfile` is reserved for an + build-context excludes. Its `FROM` and `# syntax=` lines are pinned by + digest, written literally so Renovate can update tag and digest + together. The top-level `Dockerfile` is reserved for an instance's runtime image (`../docs/TEMPLATE.md`). - `compose.yml` — the dev service (`myapp`), built from `Dockerfile`'s `develop` stage. An instance that needs backing services diff --git a/.github/renovate.json b/.github/renovate.json index 21a6241..858e4df 100644 --- a/.github/renovate.json +++ b/.github/renovate.json @@ -9,11 +9,29 @@ "labels": ["security"] }, "osvVulnerabilityAlerts": true, + "internalChecksFilter": "strict", "packageRules": [ { "matchUpdateTypes": ["patch", "minor"], "groupName": "patch and minor updates", - "automerge": true + "automerge": true, + "description": "Automerged updates wait 3 days after release (the age Renovate's own security:minimumReleaseAgeNpm preset uses), so a hijacked release is usually yanked or flagged before it lands in main. The top-level internalChecksFilter strict (the docs' recommendation with minimumReleaseAge) makes a still-pending version be skipped, not merged.", + "minimumReleaseAge": "3 days" + }, + { + "description": "The python-version datasource has no release timestamps, so under the default minimumReleaseAgeBehaviour (timestamp-required) a Python patch would stay pending forever. It is raised without the age check and, since nothing vouches for its age, is not automerged.", + "matchDatasources": ["python-version"], + "matchDepNames": ["python"], + "groupName": "python patch", + "minimumReleaseAgeBehaviour": "timestamp-optional", + "automerge": false + }, + { + "description": "The github-runners datasource has no release timestamps either, so the 3 day age check would keep a runner image bump pending forever. Raised without the age check and not automerged.", + "matchDatasources": ["github-runners"], + "groupName": "runner images", + "minimumReleaseAgeBehaviour": "timestamp-optional", + "automerge": false }, { "matchUpdateTypes": ["major"], @@ -101,6 +119,18 @@ "datasourceTemplate": "docker", "depNameTemplate": "quay.io/skopeo/stable" }, + { + "description": "The syft image pinned by tag and digest in the Makefile; Renovate updates both.", + "customType": "regex", + "managerFilePatterns": [ + "/^Makefile$/" + ], + "matchStrings": [ + "SYFT_IMAGE := anchore/syft:(?v[0-9][^@\\s]*)@(?sha256:[a-f0-9]{64})" + ], + "datasourceTemplate": "docker", + "depNameTemplate": "anchore/syft" + }, { "description": "The hadolint image pinned in the hadolint hook.", "customType": "regex", diff --git a/.github/scripts/README.md b/.github/scripts/README.md index 30bf032..6d2d1fa 100644 --- a/.github/scripts/README.md +++ b/.github/scripts/README.md @@ -62,7 +62,9 @@ `has_make_target.sh` runs no recipe, but make still evaluates `$(shell ...)` and `include` while parsing the Makefile. `test_release_scripts.py` tests both, `verify_release_tag.sh`, and that - `../../scripts/develop.sh` never nests `fetch` inside `retry`; + `../../scripts/develop.sh` never nests `fetch` inside `retry`, verifies + downloads with `fetch_checked`/`install_claude`, and runs no installer script, and + that `release.yml` gates its privileged jobs on the default branch; `scripts-tests` runs it. Keep `assemble_dist.py` clean under `ruff check --select ALL` (bar `COM812` and `CPY001`), so instances that select `PL` etc. need no per-file ignores. @@ -104,18 +106,51 @@ `test_release_scripts.py`. - `run_claude.sh` — invokes `claude -p` for one issue-moderation stage inside the devcontainer; shared by all four Claude-invoking - `../workflows/moderate-*.yml` jobs. See `../workflows/README.md`'s - "Issue moderation" section. + `../workflows/moderate-*.yml` jobs. Takes the API key from its + environment only (never a file), scrubs it from Claude's subprocesses, + appends `MODERATION_EXTRA_TOOLS` and passes inline deny rules. The + workflows run the trusted copy installed at `.moderation-tools/` by + `moderate_prepare.sh`, never the one in the tree Claude edits. See + `../workflows/README.md`'s "Issue moderation" section and + `https://github.com/craftainer/template-base/blob/main/docs/adrs/0001-harden-the-issue-moderation-workflows.md`. +- `moderate_lib.sh` — sourced by the moderation scripts below: `is_sha`, + `is_ancestor` (compare API), `check_result_tree` (the commits a patch + produced against the path allow-list and no symlink or submodule), + `agent_file` (a result file must be a regular file really under + `.moderation-out/`) and the size caps. +- `moderate_check_paths.sh` — the path allow-list: reads NUL-separated + paths on stdin and fails on any under `.github/`, `.devcontainer/`, + `.claude/`, a `Makefile`/`*.mk`, `Dockerfile*`, `compose*.y*ml`/`docker-compose*.y*ml`, + `.pre-commit-config.yaml`, `scripts/release*` and the other files that + decide what runs with credentials. +- `moderate_prepare.sh` / `moderate_verify_branch.sh` — run on the runner + from a snapshot of the default branch before Claude starts: install the + trusted launcher, prompts and hook config, and refuse a repro/plan + branch that differs from the default branch in a denied path. +- `moderate_package_result.sh` — the Claude job's last step: reads the + agent's commits through a fresh bare repository and its note or triage + file only if a regular file under `.moderation-out/`, and writes + `meta.json` and `patch.diff` for the `apply` job. +- `moderate_apply_result.sh` / `moderate_apply_triage.sh` — the `apply` + jobs' scripts (fix/build, and triage): validate the artifact, then push, + open the PR or comment, and label. The only scripts that write; they + never run Claude or anything from the artifact. +- `test_moderate_apply_result.py` / `test_moderate_apply_triage.py` / + `test_moderate_tools.py` — fake-`gh`/`git` tests of the two apply scripts + (argument order, validation, the allow-list, failure paths), and tests of + the allow-list, the packager (real git, symlinks, rewritten history) and + the launcher; `scripts-tests` runs them. - `moderate_cleanup.sh` — closes any open PR for a moderation branch and deletes the branch; used by `../workflows/moderate-cleanup.yml` (issue closed) and `reset_issue_state.sh` (issue reopened). Idempotent. - `reset_issue_state.sh` — strips stage labels back to the bare `bug`/`enhancement` label and reconciles any leftover branch/PR, on - issue reopen; used by `../workflows/moderate-bug-triage.yml` and + issue reopen; used by the `apply` jobs of + `../workflows/moderate-bug-triage.yml` and `../workflows/moderate-feature-triage.yml`. - `prompts/` — the Claude prompt for each moderation stage (`bug-triage.md`, `bug-fix.md`, `feature-triage.md`, - `feature-build.md`), read by `run_claude.sh`. + `feature-build.md`), read by `run_claude.sh` from `.moderation-tools/`. ## Do diff --git a/.github/scripts/moderate_apply_result.sh b/.github/scripts/moderate_apply_result.sh index af6c43f..dade130 100755 --- a/.github/scripts/moderate_apply_result.sh +++ b/.github/scripts/moderate_apply_result.sh @@ -8,6 +8,13 @@ # moderate-bug-fix.yml / moderate-feature-build.yml. set -euo pipefail +# shellcheck source=moderate_lib.sh +. "$(dirname "${BASH_SOURCE[0]}")/moderate_lib.sh" + +# Needs, from the workflow's env: GH_TOKEN, GITHUB_REPOSITORY and +# DEFAULT_BRANCH (github.event.repository.default_branch). +: "${DEFAULT_BRANCH:?DEFAULT_BRANCH must be set}" + result_dir="$1" branch_prefix="$2" # "feature" or "bug" branch_suffix="$3" # "plan" or "repro" @@ -44,6 +51,11 @@ if ! [[ "$issue_number" =~ ^[0-9]+$ ]]; then echo "::error::issue_number in meta.json isn't numeric: $issue_number" >&2 exit 1 fi +# The artifact must belong to the issue this run is for. +if [ -n "${ISSUE_NUMBER:-}" ] && [ "$issue_number" != "$ISSUE_NUMBER" ]; then + echo "::error::issue_number in meta.json ($issue_number) is not this run's issue ($ISSUE_NUMBER)" >&2 + exit 1 +fi # Re-adding the confirm label during a run queues a second run behind the # first (the workflow's concurrency group serializes them). If the ready @@ -60,25 +72,41 @@ outcome="$(jq -r '.outcome' "$meta")" # Everything that can fail on a "built" result. Explicit `|| return 1`, not # errexit: the caller runs this in an `if !` condition, which suspends it. +# Nothing from meta.json reaches git or gh unvalidated: base_sha must be a full +# hex sha, the branch name is built from fixed parts plus a numeric issue +# number, and every ref follows a `--`. apply_built() { local remote_sha pr_url + is_sha "$base_sha" || { echo "::error::base_sha is not a 40-character sha" >&2; return 1; } + if [ -f "$result_dir/patch.diff" ] && [ "$(wc -c < "$result_dir/patch.diff")" -gt "$MAX_PATCH_BYTES" ]; then + echo "::error::patch.diff is over $MAX_PATCH_BYTES bytes" >&2 + return 1 + fi + + # The branch is bot-owned: triage's apply step creates `/-` + # (the repro/plan branch this result builds on) and only the apply scripts + # write to it afterwards. The lease pins the tip read here, so a push by + # anyone in between still fails. base_sha must be that tip or an ancestor of + # it (an earlier attempt's commits sit on top); with no such branch it must + # be an ancestor of the default branch. A tip that does not descend from + # base_sha is foreign work: refuse. + remote_sha="$(git ls-remote -- origin "refs/heads/$branch" | cut -f1)" || return 1 + if [ -n "$remote_sha" ]; then + is_sha "$remote_sha" || return 1 + is_ancestor "$base_sha" "$remote_sha" || return 1 + else + is_ancestor "$base_sha" "$DEFAULT_BRANCH" || return 1 + fi + git config user.name "claude-moderator[bot]" || return 1 git config user.email "claude-moderator[bot]@users.noreply.github.com" || return 1 - git fetch --depth=1 origin "$base_sha" || return 1 - git checkout -B "$branch" FETCH_HEAD || return 1 - git am "$result_dir/patch.diff" || return 1 - - # The branch is bot-owned: triage's Claude run creates `/-` - # (the repro/plan branch this result builds on) and only this script writes - # to it afterwards. The lease pins the tip read here, so a push by anyone - # in between still fails. A tip other than base_sha must be a descendant of - # it (an earlier attempt's commits), otherwise it's foreign work: refuse. - remote_sha="$(git ls-remote origin "refs/heads/$branch" | cut -f1)" || return 1 - if [ -n "$remote_sha" ] && [ "$remote_sha" != "$base_sha" ]; then - git fetch --depth=100 origin "$base_sha" "$remote_sha" || return 1 - git merge-base --is-ancestor "$base_sha" "$remote_sha" || return 1 - fi - git push --force-with-lease="$branch:$remote_sha" origin "$branch" || return 1 + git fetch --depth=1 -- origin "$base_sha" || return 1 + git checkout -B "$branch" FETCH_HEAD -- || return 1 + git am -- "$result_dir/patch.diff" || return 1 + # The applied commits, not the patch text, are what gets checked: renames, + # mode changes and binary files all show up in the diff. + check_result_tree "$base_sha" || return 1 + git push --force-with-lease="$branch:$remote_sha" -- origin "$branch" || return 1 pr_url="$(gh pr list --head "$branch" --state open --json number,url --jq '.[0].url // ""')" || return 1 if [ -n "$pr_url" ]; then @@ -86,7 +114,7 @@ apply_built() { else gh pr create --title "$subject" --body "$body -$closes_verb #$issue_number" --base main --head "$branch" || return 1 +$closes_verb #$issue_number" --base "$DEFAULT_BRANCH" --head "$branch" || return 1 fi # Only now that the PR exists does the stage move on. @@ -96,8 +124,8 @@ $closes_verb #$issue_number" --base main --head "$branch" || return 1 if [ "$outcome" = built ]; then base_sha="$(jq -r '.base_sha' "$meta")" branch="$branch_prefix/$issue_number-$branch_suffix" - subject="$(jq -r '.subject' "$meta")" - body="$(jq -r '.body' "$meta")" + subject="$(jq -r --argjson n "$MAX_TITLE_CHARS" '(.subject // "") | .[0:$n]' "$meta")" + body="$(jq -r --argjson n "$MAX_TEXT_CHARS" '(.body // "") | .[0:$n] | gsub("@"; "@\u200b")' "$meta")" if ! apply_built; then fail "Applying the automated result (patch, push, PR or relabel) failed." @@ -105,7 +133,7 @@ if [ "$outcome" = built ]; then else # Not built: the stage stays at "ready" on purpose, so a maintainer's # retry (re-adding the confirm label) re-runs the build. - body="$(jq -r '.body' "$meta")" + body="$(jq -r --argjson n "$MAX_TEXT_CHARS" '(.body // "") | .[0:$n] | gsub("@"; "@\u200b")' "$meta")" gh issue edit "$issue_number" --add-label "needs-human" || true gh issue comment "$issue_number" --body "$body" fi diff --git a/.github/scripts/moderate_apply_triage.sh b/.github/scripts/moderate_apply_triage.sh new file mode 100755 index 0000000..0af529b --- /dev/null +++ b/.github/scripts/moderate_apply_triage.sh @@ -0,0 +1,136 @@ +#!/usr/bin/env bash +# Applies one triage job's packaged result (outcome metadata and, for a repro +# or plan, a git patch -- data, never executed as code): pushes the repro/plan +# branch, comments on the issue and sets the stage label. The privileged half +# of the triage split described in ../workflows/README.md's "Issue moderation" +# section: the job that ran Claude on the untrusted issue text held a +# read-only token, so everything with write effect happens here, from the +# default branch's checkout, on values validated below. +# +# Usage: moderate_apply_triage.sh +# Env: GH_TOKEN, GITHUB_REPOSITORY, DEFAULT_BRANCH, ISSUE_NUMBER (this run's +# issue); GITHUB_SERVER_URL/GITHUB_RUN_ID are optional (run link on failure). +set -euo pipefail + +# shellcheck source=moderate_lib.sh +. "$(dirname "${BASH_SOURCE[0]}")/moderate_lib.sh" + +: "${DEFAULT_BRANCH:?DEFAULT_BRANCH must be set}" +: "${GITHUB_REPOSITORY:?GITHUB_REPOSITORY must be set}" + +kind="$1" +result_dir="$2" + +# The action names Claude may report, and the label each one sets. The labels +# come from this table, never from the result. +case "$kind" in + bug) + prefix=bug suffix=repro + declare -A label_for=([needs-info]="bug:needs-info" [repro-ready]="bug:repro-ready" [cannot-reproduce]="needs-human") + branch_action=repro-ready + only_prefix= + ;; + feature) + prefix=feature suffix=plan + declare -A label_for=([not-a-fit]="enhancement:not-a-fit" [plan-ready]="enhancement:plan-ready" [cannot-plan]="needs-human") + branch_action=plan-ready + only_prefix=docs/plans/ + ;; + *) + echo "::error::unknown kind '$kind' (expected 'bug' or 'feature')" >&2 + exit 1 + ;; +esac + +meta="$result_dir/meta.json" + +# Failure path: needs-human first (the signal that matters), then a +# best-effort comment. The run link only appears when all of its parts exist. +# Targets $issue_number, which is this run's issue (ISSUE_NUMBER) until the +# artifact's own number has been validated against it. +issue_number="${ISSUE_NUMBER:-}" +fail() { + if [ -n "$issue_number" ]; then + gh issue edit "$issue_number" --add-label "needs-human" || true + local run_url="" + if [ -n "${GITHUB_SERVER_URL:-}" ] && [ -n "${GITHUB_RUN_ID:-}" ]; then + run_url=" See $GITHUB_SERVER_URL/$GITHUB_REPOSITORY/actions/runs/$GITHUB_RUN_ID." + fi + gh issue comment "$issue_number" --body "$1$run_url Reopen the issue to restart triage." || true + fi + exit 1 +} + +# The artifact is untrusted data: a missing or malformed meta.json, or one +# for another issue, is refused loudly. The fixed message never echoes it. +meta_issue="$(jq -r '.issue_number' "$meta")" || meta_issue="" +if ! [[ "$meta_issue" =~ ^[0-9]+$ ]]; then + echo "::error::meta.json is missing, malformed, or its issue_number isn't numeric" >&2 + fail "Automated triage produced an unreadable result." +fi +if [ -n "$issue_number" ] && [ "$meta_issue" != "$issue_number" ]; then + echo "::error::issue_number in meta.json ($meta_issue) is not this run's issue ($issue_number)" >&2 + fail "Automated triage produced a result for a different issue." +fi +issue_number="$meta_issue" +branch="$prefix/$issue_number-$suffix" + +action="$(jq -r '(.action // "") | tostring' "$meta")" +# Claude's words are posted as text: capped, and `@` broken so it cannot ping. +comment="$(jq -r --argjson n "$MAX_TEXT_CHARS" '(.comment // "") | .[0:$n] | gsub("@"; "@​")' "$meta")" + +if ! [[ "$action" =~ ^[a-z-]{1,30}$ ]] || [ -z "${label_for[$action]+set}" ] || [ -z "$comment" ]; then + # Includes "failed" (no usable result), an action not in the table and an + # empty comment: nothing from the result is acted on. + gh issue edit "$issue_number" --add-label "needs-human" || true + gh issue comment "$issue_number" --body "Automated triage ended without a usable result. A maintainer needs to look at this issue." + exit 0 +fi + +# Everything that can fail on a repro/plan result. Explicit `|| return 1`, not +# errexit: the caller runs this in an `if !` condition, which suspends it. +apply_branch() { + local base_sha remote_sha patch="$result_dir/patch.diff" + base_sha="$(jq -r '.base_sha // ""' "$meta")" + is_sha "$base_sha" || { echo "::error::base_sha is not a 40-character sha" >&2; return 1; } + [ -f "$patch" ] || { echo "::error::a repro/plan result without a patch" >&2; return 1; } + if [ "$(wc -c < "$patch")" -gt "$MAX_PATCH_BYTES" ]; then + echo "::error::patch.diff is over $MAX_PATCH_BYTES bytes" >&2 + return 1 + fi + # Triage builds on the default branch, so the base must be part of it. + is_ancestor "$base_sha" "$DEFAULT_BRANCH" || return 1 + + # A branch left by an earlier attempt of this same run is overwritten only + # when it descends from the base; anything else is foreign work. + remote_sha="$(git ls-remote -- origin "refs/heads/$branch" | cut -f1)" || return 1 + if [ -n "$remote_sha" ]; then + is_sha "$remote_sha" || return 1 + is_ancestor "$base_sha" "$remote_sha" || return 1 + fi + + git config user.name "claude-moderator[bot]" || return 1 + git config user.email "claude-moderator[bot]@users.noreply.github.com" || return 1 + git fetch --depth=1 -- origin "$base_sha" || return 1 + git checkout -B "$branch" FETCH_HEAD -- || return 1 + git am -- "$patch" || return 1 + check_result_tree "$base_sha" "$only_prefix" || return 1 + git push --force-with-lease="$branch:$remote_sha" -- origin "$branch" || return 1 +} + +link="" +if [ "$action" = "$branch_action" ]; then + if ! apply_branch; then + fail "Applying the automated triage result (patch or push) failed." + fi + link=" + +Branch diff: ${GITHUB_SERVER_URL:-https://github.com}/$GITHUB_REPOSITORY/compare/$DEFAULT_BRANCH...$branch" +fi + +if ! gh issue comment "$issue_number" --body "$comment$link"; then + fail "Posting the triage comment failed." +fi +if ! gh issue edit "$issue_number" --add-label "${label_for[$action]}"; then + fail "Setting the triage label failed." +fi diff --git a/.github/scripts/moderate_check_paths.sh b/.github/scripts/moderate_check_paths.sh new file mode 100755 index 0000000..4da2335 --- /dev/null +++ b/.github/scripts/moderate_check_paths.sh @@ -0,0 +1,30 @@ +#!/usr/bin/env bash +# Path allow-list for anything the issue-moderation automation (Claude, via a +# patch) is allowed to change. Reads NUL-separated repository paths on stdin; +# prints each denied path to stderr and exits 1 if there was any. +# +# Denied: everything that decides what runs with credentials later -- CI and +# its scripts, the devcontainer and image build, the hook configuration, the +# release and Make machinery, and Claude's own configuration. A fix that really +# needs one of these is not built automatically: it becomes a `needs-human` +# note for a maintainer. See ../workflows/README.md's "Issue moderation". +set -euo pipefail + +denied=0 +while IFS= read -r -d '' path; do + case "$path" in + .github/* | .devcontainer/* | .claude/* | .vscode/* | .mcp.json | \ + .pre-commit-config.yaml | .pre-commit-config.yml | .gitattributes | .gitmodules | \ + Makefile | */Makefile | GNUmakefile | */GNUmakefile | makefile | */makefile | *.mk | \ + scripts/release* | scripts/develop.sh | scripts/post-create.sh | \ + scripts/develop.d/* | scripts/post-create.d/* | \ + Dockerfile* | */Dockerfile* | dockerfile* | */dockerfile* | *.Dockerfile | *.dockerfile | \ + compose*.y*ml | */compose*.y*ml | docker-compose*.y*ml | */docker-compose*.y*ml) + echo "::error::denied path: $path" >&2 + denied=1 + ;; + *) ;; + esac +done + +exit "$denied" diff --git a/.github/scripts/moderate_lib.sh b/.github/scripts/moderate_lib.sh new file mode 100755 index 0000000..34f7583 --- /dev/null +++ b/.github/scripts/moderate_lib.sh @@ -0,0 +1,75 @@ +#!/usr/bin/env bash +# shellcheck shell=bash +# Helpers shared by the issue-moderation scripts that handle a result produced +# by Claude (untrusted data): moderate_package_result.sh, moderate_verify_branch.sh, +# moderate_apply_result.sh and moderate_apply_triage.sh. Sourced, never run +# directly. See ../workflows/README.md's "Issue moderation" section and +# ../../docs/adrs/0001-harden-the-issue-moderation-workflows.md. + +MODERATE_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +# Caps on what the (untrusted) result may put on the repository. +MAX_PATCH_BYTES=2097152 +MAX_TEXT_CHARS=10000 +MAX_TITLE_CHARS=200 + +# is_sha : a full 40-character lowercase hex object name, nothing else +# (so it can never be read as a git option). +is_sha() { + [[ "${1:-}" =~ ^[0-9a-f]{40}$ ]] +} + +# is_ancestor : is the same commit as, or an ancestor +# of, the second argument. Asks the compare API instead of local history, so a +# shallow checkout and a long branch both work. +is_ancestor() { + local status + status="$(gh api "repos/$GITHUB_REPOSITORY/compare/$1...$2" --jq .status)" || return 1 + [ "$status" = ahead ] || [ "$status" = identical ] +} + +# check_result_tree : every path that differs between and HEAD in +# the current repository passes moderate_check_paths.sh, and none is a symlink +# or a submodule. Prints the offenders to stderr. With , every +# path must also start with it (the plan stage may change only docs/plans/). +check_result_tree() { + local base="$1" only_prefix="${2:-}" tmp meta path mode bad=0 + local -a paths=() + tmp="$(mktemp)" || return 1 + if ! git diff --raw -z --no-renames "$base" HEAD -- > "$tmp"; then + rm -f "$tmp" + return 1 + fi + while IFS= read -r -d '' meta && IFS= read -r -d '' path; do + read -r _ mode _ <<<"$meta" + case "$mode" in + 120000 | 160000) + echo "::error::$path is a symlink or submodule, which the automation never accepts" >&2 + bad=1 + ;; + *) ;; + esac + paths+=("$path") + if [ -n "$only_prefix" ] && [[ "$path" != "$only_prefix"* ]]; then + echo "::error::$path is outside $only_prefix, the only place this stage may change" >&2 + bad=1 + fi + done < "$tmp" + rm -f "$tmp" + [ "$bad" = 0 ] || return 1 + [ "${#paths[@]}" -gt 0 ] || return 0 + printf '%s\0' "${paths[@]}" | "$MODERATE_DIR/moderate_check_paths.sh" +} + +# agent_file : succeeds only for a regular, non-symlink file +# that really lives under /.moderation-out. The agent controls that +# directory, and a symlink in it would be read here on the runner, outside the +# container, and could carry a runner file into an issue comment. +agent_file() { + local file="$1" workspace="$2" real root + [ -f "$file" ] && [ ! -L "$file" ] || return 1 + real="$(realpath -e -- "$file")" || return 1 + # The directory name is appended to the resolved workspace, not resolved + # itself, so a `.moderation-out` symlinked elsewhere fails the prefix test. + root="$(realpath -e -- "$workspace")/.moderation-out" || return 1 + [[ "$real" == "$root/"* ]] +} diff --git a/.github/scripts/moderate_package_result.sh b/.github/scripts/moderate_package_result.sh index 984a7bc..0838d24 100755 --- a/.github/scripts/moderate_package_result.sh +++ b/.github/scripts/moderate_package_result.sh @@ -1,40 +1,103 @@ #!/usr/bin/env bash -# Packages a worker job's result -- a git patch plus outcome metadata, -# never anything executed -- for the privileged "apply" job of the same -# workflow to consume via a job-to-job artifact. See -# ../workflows/README.md's "Issue moderation" section: keeping -# push/PR/label-write capability out of this job (the one that runs -# Claude against untrusted issue text) is what closes CodeQL's -# actions/untrusted-checkout alert -- this job never holds a -# write-scoped GITHUB_TOKEN. +# Packages a Claude job's result -- a git patch plus outcome metadata, never +# anything executed -- for the privileged "apply" job of the same workflow to +# consume via a job-to-job artifact. See ../workflows/README.md's "Issue +# moderation" section: keeping push/PR/label-write capability out of the job +# that runs Claude against untrusted issue text is what closes CodeQL's +# actions/untrusted-checkout alert -- this job never holds a write-scoped +# GITHUB_TOKEN. +# +# Runs on the runner, from the trusted snapshot of the default branch (never +# from the tree Claude edited), and does not trust that tree's git config: the +# commits are fetched into a fresh bare repository and read there. +# +# Usage: moderate_package_result.sh +# mode build fix/build: .moderation-out/outcome.md explains a non-result +# mode triage triage: .moderation-out/triage.json = {"action","comment"} +# Needs GITHUB_ISSUE_NUMBER in the environment. set -euo pipefail +# shellcheck source=moderate_lib.sh +. "$(dirname "${BASH_SOURCE[0]}")/moderate_lib.sh" + base_sha="$1" out_dir="$2" -outcome_marker="$3" +workspace="$3" +mode="$4" + +case "$mode" in build | triage) ;; *) + echo "::error::unknown mode '$mode'" >&2 + exit 1 + ;; +esac +[[ "${GITHUB_ISSUE_NUMBER:-}" =~ ^[0-9]+$ ]] || { echo "::error::GITHUB_ISSUE_NUMBER is not numeric" >&2; exit 1; } mkdir -p "$out_dir" +safe="$(mktemp -d)" +trap 'rm -rf "$safe"' EXIT +git init -q --bare "$safe" -if [ -n "$(git log "${base_sha}..HEAD" --oneline)" ]; then - outcome=built - subject="$(git log -1 --format=%s HEAD)" - body="$(git log -1 --format=%b HEAD)" - git format-patch "${base_sha}..HEAD" --binary --stdout > "$out_dir/patch.diff" -elif [ -f "$outcome_marker" ]; then - outcome=needs-human - subject="" - body="$(cat "$outcome_marker")" -else +# Number of new commits on top of base_sha in the agent's repository (0 when +# the history was rewritten so that base_sha is no longer an ancestor). +commits=0 +if is_sha "$base_sha" \ + && git --git-dir="$safe" -c protocol.file.allow=always fetch -q --no-tags --no-write-fetch-head \ + -- "$workspace" "+HEAD:refs/heads/result" \ + && git --git-dir="$safe" merge-base --is-ancestor "$base_sha" refs/heads/result; then + commits="$(git --git-dir="$safe" rev-list --count "$base_sha..refs/heads/result")" +fi + +# Patch, when there is something to apply and it is small enough. +have_patch=0 +too_large=0 +if [ "$commits" -gt 0 ]; then + git --git-dir="$safe" format-patch "$base_sha..refs/heads/result" --binary --stdout > "$out_dir/patch.diff" + if [ "$(wc -c < "$out_dir/patch.diff")" -gt "$MAX_PATCH_BYTES" ]; then + rm -f "$out_dir/patch.diff" + too_large=1 + else + have_patch=1 + fi +fi + +if [ "$mode" = build ]; then outcome=failed subject="" body="The automated run ended without a commit or an explanation (it may have crashed or hit its turn/time limit)." + marker="$workspace/.moderation-out/outcome.md" + if [ "$too_large" = 1 ]; then + outcome=needs-human + body="The automated change is larger than the ${MAX_PATCH_BYTES}-byte limit for an unattended run." + elif [ "$have_patch" = 1 ]; then + outcome=built + subject="$(git --git-dir="$safe" log -1 --format=%s refs/heads/result)" + body="$(git --git-dir="$safe" log -1 --format=%b refs/heads/result)" + elif agent_file "$marker" "$workspace"; then + outcome=needs-human + body="$(head -c 20000 "$marker")" + fi + jq -n \ + --arg issue "$GITHUB_ISSUE_NUMBER" \ + --arg base_sha "$base_sha" \ + --arg outcome "$outcome" \ + --arg subject "$subject" \ + --arg body "$body" \ + '{issue_number: $issue, base_sha: $base_sha, outcome: $outcome, subject: $subject, body: $body}' \ + > "$out_dir/meta.json" +else + action=failed + comment="" + result="$workspace/.moderation-out/triage.json" + if [ "$too_large" = 0 ] && agent_file "$result" "$workspace" \ + && jq -e '(.action | type == "string") and (.comment | type == "string")' "$result" > /dev/null 2>&1; then + action="$(jq -r '.action' "$result")" + comment="$(jq -r '.comment[0:20000]' "$result")" + fi + jq -n \ + --arg issue "$GITHUB_ISSUE_NUMBER" \ + --arg base_sha "$base_sha" \ + --arg action "$action" \ + --arg comment "$comment" \ + '{issue_number: $issue, base_sha: $base_sha, action: $action, comment: $comment}' \ + > "$out_dir/meta.json" fi - -jq -n \ - --arg issue "$GITHUB_ISSUE_NUMBER" \ - --arg base_sha "$base_sha" \ - --arg outcome "$outcome" \ - --arg subject "$subject" \ - --arg body "$body" \ - '{issue_number: $issue, base_sha: $base_sha, outcome: $outcome, subject: $subject, body: $body}' \ - > "$out_dir/meta.json" diff --git a/.github/scripts/moderate_prepare.sh b/.github/scripts/moderate_prepare.sh new file mode 100755 index 0000000..811ca98 --- /dev/null +++ b/.github/scripts/moderate_prepare.sh @@ -0,0 +1,29 @@ +#!/usr/bin/env bash +# Run on the runner, from the trusted snapshot of the default branch, in the +# workspace the Claude step will use. Installs the trusted copies of what the +# Claude step executes or reads as configuration, and the scratch directory +# Claude writes its result to, then hides both from git so an agent's +# `git add -A` cannot commit them: +# +# .moderation-tools/run_claude.sh the launcher +# .moderation-tools/prompts/ the stage prompts +# .moderation-tools/pre-commit-config.yaml the hook config prek runs +# .moderation-out/ Claude's result files +# +# The tree Claude edits is never the source of any of these, so a changed +# `.pre-commit-config.yaml` or run_claude.sh in it is never executed with the +# API key present. See ../workflows/README.md's "Issue moderation". +# +# Usage: moderate_prepare.sh (cwd = the workspace) +set -euo pipefail + +trusted="$1" + +mkdir -p .moderation-tools/prompts .moderation-out +install -m 755 "$trusted/.github/scripts/run_claude.sh" .moderation-tools/run_claude.sh +cp -- "$trusted"/.github/scripts/prompts/*.md .moderation-tools/prompts/ +cp -- "$trusted/.pre-commit-config.yaml" .moderation-tools/pre-commit-config.yaml + +exclude="$(git rev-parse --git-path info/exclude)" +mkdir -p "$(dirname "$exclude")" +printf '%s\n' '/.moderation-tools/' '/.moderation-out/' >> "$exclude" diff --git a/.github/scripts/moderate_verify_branch.sh b/.github/scripts/moderate_verify_branch.sh new file mode 100755 index 0000000..acde903 --- /dev/null +++ b/.github/scripts/moderate_verify_branch.sh @@ -0,0 +1,26 @@ +#!/usr/bin/env bash +# Run on the runner, from the trusted snapshot of the default branch, before +# Claude starts on a repro/plan branch: the branch must not differ from its +# merge base with the default branch in any path moderate_check_paths.sh denies +# (devcontainer, image build, hook config, scripts that run with credentials). +# The branch was created by moderate_apply_triage.sh from a checked patch, so +# this only fails when someone else pushed to it; then no Claude step runs and +# the packaging step reports the note below as a `needs-human` outcome. +# +# Usage (cwd = the checkout of the branch): moderate_verify_branch.sh +set -euo pipefail + +# shellcheck source=moderate_lib.sh +. "$(dirname "${BASH_SOURCE[0]}")/moderate_lib.sh" + +trusted_sha="$1" +is_sha "$trusted_sha" || { echo "::error::trusted sha is not a full sha" >&2; exit 1; } + +merge_base="$(git merge-base "$trusted_sha" HEAD)" +if ! check_result_tree "$merge_base"; then + mkdir -p .moderation-out + printf '%s\n' \ + "The branch changes a path the automation does not build on (CI, devcontainer, hook or release configuration, a symlink or a submodule), so no automated run was started. A maintainer needs to look at the branch." \ + > .moderation-out/outcome.md + exit 1 +fi diff --git a/.github/scripts/prompts/bug-fix.md b/.github/scripts/prompts/bug-fix.md index df7bb6d..01451c0 100644 --- a/.github/scripts/prompts/bug-fix.md +++ b/.github/scripts/prompts/bug-fix.md @@ -17,8 +17,14 @@ the bug and no fix yet. - Make the failing test pass without weakening or deleting it (no loosening assertions, no skip/xfail markers, no deleting the test and writing a different one that happens to pass). -- Run the full check suite: `prek run --all-files --hook-stage - manual` (see docs/TEMPLATE.md's "Checks" section). Fix anything it +- Run the full check suite with the trusted hook configuration (never + the repository's own, which you must not rely on): + `prek run --config .moderation-tools/pre-commit-config.yaml + --all-files --hook-stage manual` (see docs/TEMPLATE.md's "Checks" + section). Do not change CI, devcontainer, hook or release files + (`.github/`, `.devcontainer/`, `.claude/`, `.pre-commit-config.yaml`, + Makefiles, `*.mk`, Dockerfiles, compose files, `scripts/release*`, + `scripts/develop.sh`): a result touching any of them is rejected. Fix anything it flags -- lint, types, coverage, the rest of the test suite -- don't just make the one new test pass at the expense of breaking others. - Make exactly one commit with a Conventional Commits message (e.g. @@ -28,7 +34,7 @@ the bug and no fix yet. If the check suite still fails after a genuine attempt, or the fix isn't converging: do not commit. Instead, write a plain-text explanation of -what you tried and where it got stuck to `.moderation-outcome.md` at the -repository root (this becomes a comment on the issue, and the issue gets +what you tried and where it got stuck to `.moderation-out/outcome.md` (the +directory exists and is not part of the repository; this becomes a comment on the issue, and the issue gets labeled `needs-human` instead of a PR being opened). Leave the branch as checked out either way. diff --git a/.github/scripts/prompts/bug-triage.md b/.github/scripts/prompts/bug-triage.md index 5a07ad2..591c7ba 100644 --- a/.github/scripts/prompts/bug-triage.md +++ b/.github/scripts/prompts/bug-triage.md @@ -18,19 +18,25 @@ actionable: do the steps plausibly lead from the expected to the actual behavior given how the code actually works? Is anything missing, ambiguous, or contradictory? -Then do exactly one of the following three things. Always end with both a -comment and a label -- never leave the issue untouched. +This step cannot push, comment or label: it has a read-only token. A +separate, privileged step does that afterwards from the result you leave +behind. You leave it by writing `.moderation-out/triage.json` (the +directory exists and is not part of the repository), valid JSON with +exactly two string fields: -## A. Missing or contradictory information - -- Comment with specific, concrete follow-up questions -- name exactly - what's missing or inconsistent, don't just ask for "more details": + {"action": "needs-info" | "repro-ready" | "cannot-reproduce", + "comment": ""} - gh issue comment "$GITHUB_ISSUE_NUMBER" --body "..." +Escape newlines in `comment` as `\n`. Then do exactly one of the +following three things. Always end with that file -- never leave the issue +without a result. -- Label it and stop. Do not create a branch or write any code: +## A. Missing or contradictory information - gh issue edit "$GITHUB_ISSUE_NUMBER" --add-label "bug:needs-info" +- Set `action` to `needs-info`. In `comment`, ask specific, concrete + follow-up questions -- name exactly what's missing or inconsistent, + don't just ask for "more details". +- Do not create a branch or write any code. ## B. Actionable @@ -38,34 +44,31 @@ comment and a label -- never leave the issue untouched. the current HEAD. - Write a failing test that reproduces the bug, and only that -- no fix. It must fail for the reason the report describes, not some unrelated - error (a typo, a missing fixture, a bad import). + error (a typo, a missing fixture, a bad import). Only add or change + files outside CI, devcontainer, hook and release configuration + (`.github/`, `.devcontainer/`, `.claude/`, `.pre-commit-config.yaml`, + Makefiles, `*.mk`, Dockerfiles, compose files, `scripts/release*`, + `scripts/develop.sh`): a result touching any of them is rejected. - Run the project's test command, scoped to the new test (see this repo's docs/TEMPLATE.md and its own instance-specific checks - documentation for how tests are normally run), and confirm it fails for - the expected reason. If a genuine, on-target failing test doesn't come - together after a reasonable attempt, fall back to case C instead of - committing a misleading or flaky test. -- Commit with a Conventional Commits message (e.g. `test: reproduce - bug in #`) and push: - - git push -u origin "bug/$GITHUB_ISSUE_NUMBER-repro" + documentation for how tests are normally run). Hooks run with the + trusted configuration, never the repository's own: -- Look up this repo's slug and comment a link to the branch's diff - against the default branch, plus a short summary of the failing test, - asking a maintainer to add the `bug:confirm` label once the reporter - has said this matches what they're seeing: + prek run --config .moderation-tools/pre-commit-config.yaml --all-files - repo=$(gh repo view --json nameWithOwner -q .nameWithOwner) - gh issue comment "$GITHUB_ISSUE_NUMBER" --body "... https://github.com/$repo/compare/main...bug/$GITHUB_ISSUE_NUMBER-repro ..." - -- Label it: - - gh issue edit "$GITHUB_ISSUE_NUMBER" --add-label "bug:repro-ready" + Confirm the new test fails for the expected reason. If a genuine, + on-target failing test doesn't come together after a reasonable + attempt, fall back to case C instead of committing a misleading or + flaky test. +- Commit with a Conventional Commits message (e.g. `test: reproduce + bug in #`). Do not push; there is no push access. +- Set `action` to `repro-ready`. In `comment`, summarise the failing + test and ask a maintainer to add the `bug:confirm` label once the + reporter has said this matches what they're seeing. The link to the + branch is added for you. ## C. Can't reproduce after a genuine attempt -- Comment explaining specifically what you tried and why it didn't - reproduce. -- Label it and stop. Do not push a branch: - - gh issue edit "$GITHUB_ISSUE_NUMBER" --add-label "needs-human" +- Set `action` to `cannot-reproduce`. In `comment`, explain specifically + what you tried and why it didn't reproduce. +- Do not commit anything. diff --git a/.github/scripts/prompts/feature-build.md b/.github/scripts/prompts/feature-build.md index 5c6fa07..0db759e 100644 --- a/.github/scripts/prompts/feature-build.md +++ b/.github/scripts/prompts/feature-build.md @@ -19,8 +19,14 @@ under `docs/plans/`. fold anything ADR-worthy into `docs/adrs/` (only if it involved a significant, reversible-at-cost decision) or into product docs under `docs/`, then delete the plan file -- a finished plan doesn't linger. -- Run the full check suite: `prek run --all-files --hook-stage - manual` (see docs/TEMPLATE.md's "Checks" section). Fix anything it +- Run the full check suite with the trusted hook configuration (never + the repository's own, which you must not rely on): + `prek run --config .moderation-tools/pre-commit-config.yaml + --all-files --hook-stage manual` (see docs/TEMPLATE.md's "Checks" + section). Do not change CI, devcontainer, hook or release files + (`.github/`, `.devcontainer/`, `.claude/`, `.pre-commit-config.yaml`, + Makefiles, `*.mk`, Dockerfiles, compose files, `scripts/release*`, + `scripts/develop.sh`): a result touching any of them is rejected. Fix anything it flags. - Make exactly one commit with a Conventional Commits message (e.g. `feat: `) -- its subject and body become the PR title and body @@ -30,6 +36,7 @@ under `docs/plans/`. If the check suite still fails after a genuine attempt, or the implementation isn't converging with the plan: do not commit. Instead, write a plain-text explanation of what you tried and where it got stuck -to `.moderation-outcome.md` at the repository root (this becomes a +to `.moderation-out/outcome.md` (the directory exists and is not part of the +repository; this becomes a comment on the issue, and the issue gets labeled `needs-human` instead of a PR being opened). Leave the branch as checked out either way. diff --git a/.github/scripts/prompts/feature-triage.md b/.github/scripts/prompts/feature-triage.md index cdcceac..16a4dc5 100644 --- a/.github/scripts/prompts/feature-triage.md +++ b/.github/scripts/prompts/feature-triage.md @@ -17,16 +17,25 @@ the request fits: does it duplicate existing functionality, contradict a recorded ADR decision, or belong in a different architectural layer than proposed? -Then do exactly one of the following two things. Always end with both a -comment and a label -- never leave the issue untouched. +This step cannot push, comment or label: it has a read-only token. A +separate, privileged step does that afterwards from the result you leave +behind. You leave it by writing `.moderation-out/triage.json` (the +directory exists and is not part of the repository), valid JSON with +exactly two string fields: -## A. Doesn't fit + {"action": "not-a-fit" | "plan-ready" | "cannot-plan", + "comment": ""} + +Escape newlines in `comment` as `\n`. Then do exactly one of the +following two things. Always end with that file -- never leave the issue +without a result. -- Comment explaining specifically why (cite the duplicated feature, the - contradicted ADR, or the layering mismatch). -- Label it and stop. Do not create a branch: +## A. Doesn't fit - gh issue edit "$GITHUB_ISSUE_NUMBER" --add-label "enhancement:not-a-fit" +- Set `action` to `not-a-fit`. In `comment`, explain specifically why + (cite the duplicated feature, the contradicted ADR, or the layering + mismatch). +- Do not create a branch. ## B. Fits @@ -38,25 +47,16 @@ comment and a label -- never leave the issue untouched. Base the Goal/Approach on the issue's Problem/Proposed solution/Alternatives fields, plus whatever repo context is needed to make the approach concrete and reviewable. Reference any relevant ADR - instead of repeating its reasoning. Leave Status as `Draft`. + instead of repeating its reasoning. Leave Status as `Draft`. Change + nothing else. - Commit with a Conventional Commits message (e.g. `docs: plan for - #`) and push: - - git push -u origin "feature/$GITHUB_ISSUE_NUMBER-plan" - -- Comment on the issue with the plan's Goal/Approach summary and a link - to the plan file on that branch: - - repo=$(gh repo view --json nameWithOwner -q .nameWithOwner) - gh issue comment "$GITHUB_ISSUE_NUMBER" --body "... https://github.com/$repo/blob/feature/$GITHUB_ISSUE_NUMBER-plan/docs/plans/ ..." - - Ask a maintainer to add the `enhancement:confirm` label once the plan looks right. -- Label it: - - gh issue edit "$GITHUB_ISSUE_NUMBER" --add-label "enhancement:plan-ready" + #`). Do not push; there is no push access. +- Set `action` to `plan-ready`. In `comment`, summarise the plan's + Goal/Approach and ask a maintainer to add the `enhancement:confirm` + label once the plan looks right. The link to the branch is added for + you. If you cannot produce a coherent plan (the request is too vague even after reading the repo, or genuinely needs a maintainer decision first), -comment explaining what's blocking a plan and label -`gh issue edit "$GITHUB_ISSUE_NUMBER" --add-label "needs-human"` instead -of case B. Do not push a branch in that case. +set `action` to `cannot-plan` and explain in `comment` what's blocking a +plan. Do not commit anything in that case. diff --git a/.github/scripts/run_claude.sh b/.github/scripts/run_claude.sh index cad96de..680d7ef 100755 --- a/.github/scripts/run_claude.sh +++ b/.github/scripts/run_claude.sh @@ -2,7 +2,15 @@ # Runs `claude -p` for one moderation stage, inside the devcontainer (see # ../workflows/README.md's "Issue moderation" section). Shared by all four # Claude-invoking moderation workflows so the model/effort/output-format -# pinning and the git identity used for commits stay in one place. +# pinning, the git identity and the permission hardening stay in one place. +# +# The workflow copies this file from a trusted snapshot of the default branch +# to .moderation-tools/run_claude.sh before it starts (moderate_prepare.sh); it +# never runs the copy that sits in the checked-out tree. +# +# The Anthropic API key arrives ONLY in the environment of this process +# (devcontainers/ci's `env:` input -> `devcontainer exec --remote-env`). It is +# never written to a file in the workspace. set -euo pipefail prompt_file="$1" @@ -10,18 +18,77 @@ allowed_tools="$2" max_turns="$3" timeout_seconds="$4" -export ANTHROPIC_API_KEY -ANTHROPIC_API_KEY="$(cat .secrets/claude.txt)" +if [ -z "${ANTHROPIC_API_KEY:-}" ]; then + echo "::error::ANTHROPIC_API_KEY is not set in the environment of run_claude.sh" >&2 + exit 1 +fi +[[ "$max_turns" =~ ^[0-9]+$ && "$timeout_seconds" =~ ^[0-9]+$ ]] || { + echo "::error::max turns and timeout must be integers" >&2 + exit 1 +} + +# An instance's optional extra tools (the MODERATION_EXTRA_TOOLS repository +# variable) arrive as data in the environment, never spliced into a command. +if [ -n "${MODERATION_EXTRA_TOOLS:-}" ]; then + allowed_tools="$allowed_tools,$MODERATION_EXTRA_TOOLS" +fi # Every commit this pipeline makes carries this identity, distinct from the # github-actions[bot] identity ../scripts/template_sync.py uses, so a -# Claude-authored commit is visibly attributable to this pipeline. -git config user.name "claude-moderator[bot]" -git config user.email "claude-moderator[bot]@users.noreply.github.com" +# Claude-authored commit is visibly attributable to this pipeline. Set through +# the environment, not `git config`, so nothing is written to .git/config. +export GIT_AUTHOR_NAME="claude-moderator[bot]" +export GIT_COMMITTER_NAME="claude-moderator[bot]" +export GIT_AUTHOR_EMAIL="claude-moderator[bot]@users.noreply.github.com" +export GIT_COMMITTER_EMAIL="claude-moderator[bot]@users.noreply.github.com" + +# Strip credentials from the environment of everything Claude starts (Bash +# commands, hooks, MCP servers); only claude itself keeps the key. +export CLAUDE_CODE_SUBPROCESS_ENV_SCRUB=1 + +# Network allow-list for the sandbox below: comma-separated hosts, set by the +# workflow (github.com/api.github.com for triage's `gh issue view`, github.com +# for prek's hook-repo clones, plus the optional MODERATION_EXTRA_DOMAINS +# repository variable). The Anthropic API is not on it: claude itself is not +# sandboxed, only the commands it starts. +domains_json="" +IFS=',' read -ra domains <<< "${MODERATION_ALLOWED_DOMAINS:-}" +for d in "${domains[@]}"; do + [ -n "$d" ] || continue + [[ "$d" =~ ^[A-Za-z0-9*.-]+$ ]] || { echo "::error::bad domain '$d' in MODERATION_ALLOWED_DOMAINS" >&2; exit 1; } + domains_json="$domains_json${domains_json:+,}\"$d\"" +done +[ -n "$domains_json" ] || { echo "::error::MODERATION_ALLOWED_DOMAINS is empty" >&2; exit 1; } + +# Inline `--settings` is the command-line level of Claude Code's settings +# precedence, above the project's own .claude/settings*.json, and leaves no +# file for the agent to edit mid-run. +# +# permissions.deny outranks every allow rule and keeps the FILE tools (which +# the sandbox does not cover) away from the process environment +# (/proc//environ holds the key), this tooling directory and .git. +# +# sandbox: Claude Code's built-in sandbox (bubblewrap + socat on Linux) wraps +# every Bash command and the processes it starts -- prek hooks, tests, code the +# agent wrote. failIfUnavailable makes claude exit at startup when it cannot +# start, and allowUnsandboxedCommands=false removes the retry-outside-the- +# sandbox escape, so a runner or container that cannot sandbox fails closed +# instead of running unsandboxed. enableWeakerNestedSandbox is deliberately NOT +# set: it would bind-mount the container's /proc, exposing the claude process's +# environment. autoAllowBashIfSandboxed=false keeps --allowedTools in force for +# Bash. Network is limited to the allow-list above (strictAllowlist: refuse, +# do not prompt); the key is unset in every sandboxed command, and the +# credential locations of the container user are unreadable. +settings='{"permissions":{"deny":["Read(//proc/**)","Read(//sys/**)","Read(~/.claude/**)","Read(~/.config/gh/**)","Edit(./.moderation-tools/**)","Edit(./.git/**)"]},' +settings="$settings"'"sandbox":{"enabled":true,"failIfUnavailable":true,"allowUnsandboxedCommands":false,"autoAllowBashIfSandboxed":false,' +settings="$settings"'"network":{"allowedDomains":['"$domains_json"'],"strictAllowlist":true},' +settings="$settings"'"filesystem":{"allowWrite":["~/.cache"],"denyRead":["~/.claude","~/.config/gh","~/.ssh","~/.aws","~/.netrc","~/.git-credentials"]},' +settings="$settings"'"credentials":{"envVars":[{"name":"ANTHROPIC_API_KEY","mode":"deny"}]}}}' timeout "${timeout_seconds}s" claude -p "$(cat "$prompt_file")" \ --model claude-sonnet-5 \ --effort medium \ --output-format json \ --max-turns "$max_turns" \ + --settings "$settings" \ --allowedTools "$allowed_tools" diff --git a/.github/scripts/test_moderate_apply_result.py b/.github/scripts/test_moderate_apply_result.py index af133b8..4e8b025 100644 --- a/.github/scripts/test_moderate_apply_result.py +++ b/.github/scripts/test_moderate_apply_result.py @@ -29,24 +29,37 @@ + FAKE_COMMON + """[ "$1 $2" != "issue view" ] || printf '%s\\n' $FAKE_LABELS [ "$1 $2" != "pr list" ] || printf '%s\\n' "${FAKE_PR_URL:-}" +if [ "$1" = api ]; then + status=ahead + IFS='|' + for sub in ${FAKE_DIVERGED:-}; do + case "$CALL" in *"$sub"*) status=diverged;; esac + done + unset IFS + printf '%s\\n' "$status" +fi """ ) FAKE_GIT = ( "#!/usr/bin/env bash\nTOOL=git\n" + FAKE_COMMON + """[ "$1" != ls-remote ] || { [ -z "${FAKE_REMOTE_SHA:-}" ] || printf '%s\\trefs/heads/x\\n' "$FAKE_REMOTE_SHA"; } +[ "$1 $2" != "diff --raw" ] || printf '%b' "${FAKE_DIFF-:100644 100644 a b M\\0tests/test_x.py\\0}" """ ) ARGS = ["bug", "repro", "bug:repro-ready", "bug:fixed", "Fixes"] BRANCH = "bug/7-repro" READY = "bug bug:repro-ready" +BASE = "a1" * 20 +REMOTE = "d4" * 20 SETUP = [ "git config user.name claude-moderator[bot]", "git config user.email claude-moderator[bot]@users.noreply.github.com", - "git fetch --depth=1 origin abc123", - f"git checkout -B {BRANCH} FETCH_HEAD", - "git am {result}/patch.diff", + f"git fetch --depth=1 -- origin {BASE}", + f"git checkout -B {BRANCH} FETCH_HEAD --", + "git am -- {result}/patch.diff", + f"git diff --raw -z --no-renames {BASE} HEAD --", ] PR_CREATE = [ "gh pr create --title fix: the thing --body details", @@ -56,12 +69,15 @@ SWAP = "gh issue edit 7 --remove-label bug:repro-ready --remove-label needs-human --add-label bug:fixed" NEEDS_HUMAN = "gh issue edit 7 --add-label needs-human" VIEW = "gh issue view 7 --json labels --jq .labels[].name" +LSREMOTE = f"git ls-remote -- origin refs/heads/{BRANCH}" +VS_DEFAULT = f"gh api repos/o/r/compare/{BASE}...main --jq .status" +VS_REMOTE = f"gh api repos/o/r/compare/{BASE}...{REMOTE} --jq .status" LIST = f'gh pr list --head {BRANCH} --state open --json number,url --jq .[0].url // ""' @unittest.skipUnless(shutil.which("jq"), "jq not installed (the script parses meta.json with it)") class ApplyResultTest(unittest.TestCase): - """Argument order, success ordering and the failure path.""" + """Argument order, validation, success ordering and the failure path.""" def setUp(self) -> None: """Put fake ``gh`` and ``git`` first on PATH inside a temp dir.""" @@ -84,14 +100,16 @@ def run_script( remote_sha: str = "", pr_url: str = "", run_env: dict[str, str] | None = None, + meta_extra: dict[str, str] | None = None, ) -> subprocess.CompletedProcess[str]: """Run the script against a canned ``meta.json``.""" meta = { "issue_number": "7", - "base_sha": "abc123", + "base_sha": BASE, "outcome": outcome, "subject": "fix: the thing", "body": "details", + **(meta_extra or {}), } (self.result / "meta.json").write_text(json.dumps(meta)) env = { @@ -105,6 +123,8 @@ def run_script( "GITHUB_SERVER_URL": "https://github.com", "GITHUB_REPOSITORY": "o/r", "GITHUB_RUN_ID": "99", + "DEFAULT_BRANCH": "main", + "ISSUE_NUMBER": "7", **(run_env or {}), } return subprocess.run( # noqa: S603 -- fixed argv, trusted script @@ -122,12 +142,12 @@ def log(self) -> list[str]: return path.read_text().splitlines() if path.exists() else [] def setup_calls(self) -> list[str]: - """Return the expected git calls up to and including ``git am``.""" + """Return the expected git calls up to and including the diff check.""" return [c.format(result=self.result) for c in SETUP] def push(self, expected: str = "") -> str: """Return the expected push call, leased on ``expected``.""" - return f"git push --force-with-lease={BRANCH}:{expected} origin {BRANCH}" + return f"git push --force-with-lease={BRANCH}:{expected} -- origin {BRANCH}" def assert_failed_cleanly(self, proc: subprocess.CompletedProcess[str], log: list[str]) -> None: """Assert needs-human came before the comment, and the stage never moved.""" @@ -144,22 +164,13 @@ def test_happy_path(self) -> None: self.assertEqual(proc.returncode, 0, proc.stderr) self.assertEqual( self.log(), - [ - VIEW, - *self.setup_calls(), - f"git ls-remote origin refs/heads/{BRANCH}", - self.push(), - LIST, - ] - + PR_CREATE - + [SWAP], + [VIEW, LSREMOTE, VS_DEFAULT, *self.setup_calls(), self.push(), LIST, *PR_CREATE, SWAP], ) def test_am_fails(self) -> None: """A failing ``git am`` stops before any push or PR; the stage stays ready.""" proc = self.run_script(fail="git am") log = self.log() - self.assertEqual(log[: len(self.setup_calls()) + 1], [VIEW, *self.setup_calls()]) self.assertFalse([c for c in log if c.startswith(("git push", "gh pr"))]) self.assert_failed_cleanly(proc, log) @@ -190,20 +201,11 @@ def test_label_edit_fails_after_pr(self) -> None: self.assertTrue([c for c in log if c.startswith("gh issue comment 7")]) # The retry: PR already open, remote tip is the earlier attempt's SHA. (self.tmp / "log.txt").unlink() - retry = self.run_script(remote_sha="def456", pr_url="https://github.com/o/r/pull/8") + retry = self.run_script(remote_sha=REMOTE, pr_url="https://github.com/o/r/pull/8") self.assertEqual(retry.returncode, 0, retry.stderr) self.assertEqual( self.log(), - [ - VIEW, - *self.setup_calls(), - f"git ls-remote origin refs/heads/{BRANCH}", - "git fetch --depth=100 origin abc123 def456", - "git merge-base --is-ancestor abc123 def456", - self.push("def456"), - LIST, - SWAP, - ], + [VIEW, LSREMOTE, VS_REMOTE, *self.setup_calls(), self.push(REMOTE), LIST, SWAP], ) def test_comment_fails_inside_fail(self) -> None: @@ -228,10 +230,10 @@ def test_fail_omits_run_link_without_env(self) -> None: def test_retry_after_partial_first_run(self) -> None: """Remote branch at an earlier attempt's SHA and PR already open: leased push, no new PR.""" - proc = self.run_script(remote_sha="def456", pr_url="https://github.com/o/r/pull/8") + proc = self.run_script(remote_sha=REMOTE, pr_url="https://github.com/o/r/pull/8") log = self.log() self.assertEqual(proc.returncode, 0, proc.stderr) - self.assertIn(self.push("def456"), log) + self.assertIn(self.push(REMOTE), log) self.assertNotIn( "--force", " ".join(c for c in log if c.startswith("git push")).replace("--force-with-lease", ""), @@ -252,18 +254,79 @@ def test_retry_after_failure_clears_needs_human(self) -> None: def test_retry_refuses_foreign_branch_tip(self) -> None: """A remote tip that doesn't descend from the base SHA is never overwritten.""" - proc = self.run_script(remote_sha="def456", fail="git merge-base") + proc = self.run_script(remote_sha=REMOTE, run_env={"FAKE_DIVERGED": f"...{REMOTE}"}) log = self.log() - self.assertFalse([c for c in log if c.startswith(("git push", "gh pr"))]) + self.assertFalse([c for c in log if c.startswith(("git fetch", "git push", "gh pr"))]) self.assert_failed_cleanly(proc, log) def test_remote_branch_at_base_sha_is_pushed_with_lease(self) -> None: - """First run: the triage branch sits at the base SHA, so no ancestry check is needed.""" - proc = self.run_script(remote_sha="abc123") + """First run: the triage branch sits at the base SHA, so the lease pins that tip.""" + proc = self.run_script(remote_sha=BASE) log = self.log() self.assertEqual(proc.returncode, 0, proc.stderr) - self.assertIn(self.push("abc123"), log) - self.assertFalse([c for c in log if c.startswith("git merge-base")]) + self.assertIn(self.push(BASE), log) + + def test_base_not_on_default_branch_without_a_remote_branch(self) -> None: + """No bot branch to build on: the base must be an ancestor of the default branch.""" + proc = self.run_script(run_env={"FAKE_DIVERGED": "...main"}) + log = self.log() + self.assertFalse([c for c in log if c.startswith(("git fetch", "git push", "gh pr"))]) + self.assert_failed_cleanly(proc, log) + + def test_base_sha_must_be_a_full_hex_sha(self) -> None: + """Option-like, short or uppercase values never reach git (no fetch, no api call).""" + for bad in ("--upload-pack=touch /tmp/x", "abc123", "A1" * 20, f"{BASE}\n--x", ""): + with self.subTest(base_sha=bad): + (self.tmp / "log.txt").unlink(missing_ok=True) + proc = self.run_script(meta_extra={"base_sha": bad}) + log = self.log() + self.assertFalse([c for c in log if c.startswith(("git fetch", "git am", "gh api"))]) + self.assert_failed_cleanly(proc, log) + + def test_denied_path_in_the_patch_is_never_pushed(self) -> None: + """A patch touching CI, the hook config or the release scripts is applied locally only.""" + for path in ( + ".github/workflows/x.yml", + ".github/scripts/run_claude.sh", + "scripts/release.sh", + "Makefile", + "image.mk", + ".pre-commit-config.yaml", + ".devcontainer/Dockerfile", + ): + with self.subTest(path=path): + (self.tmp / "log.txt").unlink(missing_ok=True) + diff = f":100644 100644 a b M\\0tests/ok.py\\0:100644 100644 a b M\\0{path}\\0" + proc = self.run_script(run_env={"FAKE_DIFF": diff}) + log = self.log() + self.assertFalse([c for c in log if c.startswith(("git push", "gh pr"))]) + self.assertIn("denied path", proc.stderr) + self.assert_failed_cleanly(proc, log) + + def test_symlink_or_submodule_in_the_patch_is_never_pushed(self) -> None: + """A symlink (120000) or gitlink (160000) is rejected whatever its path.""" + for mode in ("120000", "160000"): + with self.subTest(mode=mode): + (self.tmp / "log.txt").unlink(missing_ok=True) + proc = self.run_script(run_env={"FAKE_DIFF": f":000000 {mode} a b A\\0src/link\\0"}) + log = self.log() + self.assertFalse([c for c in log if c.startswith(("git push", "gh pr"))]) + self.assert_failed_cleanly(proc, log) + + def test_ordinary_paths_pass(self) -> None: + """Source, tests and docs (also next to look-alike names) are accepted.""" + diff = "".join( + f":100644 100644 a b M\\0{p}\\0" + for p in ("src/a.py", "tests/test_a.py", "docs/Makefile.md", "scripts/other.sh", ".githubx/y") + ) + proc = self.run_script(run_env={"FAKE_DIFF": diff}) + self.assertEqual(proc.returncode, 0, proc.stderr) + + def test_result_for_another_issue_is_refused(self) -> None: + """An artifact whose issue_number isn't this run's issue changes nothing.""" + proc = self.run_script(run_env={"ISSUE_NUMBER": "8"}) + self.assertEqual(proc.returncode, 1) + self.assertEqual(self.log(), []) def test_not_built_outcome_comments_and_flags_needs_human(self) -> None: """A non-built outcome posts Claude's note, adds needs-human and keeps the ready label.""" @@ -271,6 +334,13 @@ def test_not_built_outcome_comments_and_flags_needs_human(self) -> None: self.assertEqual(proc.returncode, 0, proc.stderr) self.assertEqual(self.log(), [VIEW, NEEDS_HUMAN, "gh issue comment 7 --body details"]) + def test_note_cannot_ping_users(self) -> None: + """An @ in the model's note is broken so the comment notifies nobody.""" + self.run_script(outcome="needs-human", meta_extra={"body": "cc @octocat"}) + comment = next(c for c in self.log() if c.startswith("gh issue comment 7")) + self.assertNotIn("@octocat", comment) + self.assertIn("@​octocat", comment) + def test_already_processed_is_a_noop(self) -> None: """Without the ready label the script only reads the labels.""" proc = self.run_script(labels="bug") diff --git a/.github/scripts/test_moderate_apply_triage.py b/.github/scripts/test_moderate_apply_triage.py new file mode 100644 index 0000000..11083df --- /dev/null +++ b/.github/scripts/test_moderate_apply_triage.py @@ -0,0 +1,269 @@ +"""Tests for ``moderate_apply_triage.sh`` (the privileged apply step of the triage workflows).""" + +from __future__ import annotations + +import json +import os +import shutil +import subprocess +import tempfile +import unittest +from pathlib import Path + +from test_moderate_apply_result import BASE, FAKE_GH, FAKE_GIT, REMOTE + +SCRIPTS = Path(__file__).resolve().parent +NEEDS_HUMAN = "gh issue edit 7 --add-label needs-human" +COMMENT = "gh issue comment 7 --body looks reproducible" + + +def setup_calls(branch: str, result: Path) -> list[str]: + """Return the git calls that build, then check, the branch from the patch.""" + return [ + "git config user.name claude-moderator[bot]", + "git config user.email claude-moderator[bot]@users.noreply.github.com", + f"git fetch --depth=1 -- origin {BASE}", + f"git checkout -B {branch} FETCH_HEAD --", + f"git am -- {result}/patch.diff", + f"git diff --raw -z --no-renames {BASE} HEAD --", + ] + + +@unittest.skipUnless(shutil.which("jq"), "jq not installed (the script parses meta.json with it)") +class ApplyTriageTest(unittest.TestCase): + """What a triage result may do: comment, one fixed label, and a checked branch push.""" + + def setUp(self) -> None: + """Put fake ``gh`` and ``git`` first on PATH inside a temp dir.""" + tmp = tempfile.TemporaryDirectory() + self.addCleanup(tmp.cleanup) + self.tmp = Path(tmp.name) + for name, body in (("gh", FAKE_GH), ("git", FAKE_GIT)): + exe = self.tmp / "bin" / name + exe.parent.mkdir(exist_ok=True) + exe.write_text(body) + exe.chmod(0o755) + self.result = self.tmp / "result" + self.result.mkdir() + # The fake `git am` never reads it, but the script requires one to exist. + (self.result / "patch.diff").write_text("") + + def run_script( + self, + action: str, + kind: str = "bug", + comment: str = "looks reproducible", + fail: str = "", + remote_sha: str = "", + run_env: dict[str, str] | None = None, + meta_extra: dict[str, str] | None = None, + ) -> subprocess.CompletedProcess[str]: + """Run the script against a canned ``meta.json``.""" + meta = { + "issue_number": "7", + "base_sha": BASE, + "action": action, + "comment": comment, + **(meta_extra or {}), + } + (self.result / "meta.json").write_text(json.dumps(meta)) + env = { + **os.environ, + "PATH": f"{self.tmp / 'bin'}{os.pathsep}{os.environ['PATH']}", + "FAKE_LOG": str(self.tmp / "log.txt"), + "FAKE_FAIL": fail, + "FAKE_REMOTE_SHA": remote_sha, + "GITHUB_SERVER_URL": "https://github.com", + "GITHUB_REPOSITORY": "o/r", + "GITHUB_RUN_ID": "99", + "DEFAULT_BRANCH": "main", + "ISSUE_NUMBER": "7", + **(run_env or {}), + } + return subprocess.run( # noqa: S603 -- fixed argv, trusted script + [str(SCRIPTS / "moderate_apply_triage.sh"), kind, str(self.result)], + env=env, + capture_output=True, + text=True, + check=False, + timeout=30, + ) + + def log(self) -> list[str]: + """Return the recorded fake-command calls, in order.""" + path = self.tmp / "log.txt" + return path.read_text().splitlines() if path.exists() else [] + + def test_needs_info_only_comments_and_labels(self) -> None: + """No branch work at all: the comment, then the table's label.""" + proc = self.run_script("needs-info") + self.assertEqual(proc.returncode, 0, proc.stderr) + self.assertEqual(self.log(), [COMMENT, "gh issue edit 7 --add-label bug:needs-info"]) + + def test_cannot_reproduce_flags_needs_human(self) -> None: + """The give-up outcome maps to needs-human, still with its comment.""" + proc = self.run_script("cannot-reproduce") + self.assertEqual(proc.returncode, 0, proc.stderr) + self.assertEqual(self.log(), [COMMENT, NEEDS_HUMAN]) + + def test_repro_ready_pushes_the_checked_branch_first(self) -> None: + """Branch built from the patch, checked and pushed (leased on absence) before the comment.""" + proc = self.run_script("repro-ready") + self.assertEqual(proc.returncode, 0, proc.stderr) + branch = "bug/7-repro" + self.assertEqual( + self.log(), + [ + f"gh api repos/o/r/compare/{BASE}...main --jq .status", + f"git ls-remote -- origin refs/heads/{branch}", + *setup_calls(branch, self.result), + f"git push --force-with-lease={branch}: -- origin {branch}", + "gh issue comment 7 --body looks reproducible", + "", + f"Branch diff: https://github.com/o/r/compare/main...{branch}", + "gh issue edit 7 --add-label bug:repro-ready", + ], + ) + + def test_feature_plan_ready(self) -> None: + """The feature kind uses its own branch and label.""" + diff = ":100644 100644 a b A\\0docs/plans/2026-11-x.md\\0" + proc = self.run_script("plan-ready", kind="feature", run_env={"FAKE_DIFF": diff}) + self.assertEqual(proc.returncode, 0, proc.stderr) + log = self.log() + self.assertIn("git push --force-with-lease=feature/7-plan: -- origin feature/7-plan", log) + self.assertEqual(log[-1], "gh issue edit 7 --add-label enhancement:plan-ready") + + def test_unknown_action_acts_on_nothing_from_the_result(self) -> None: + """An action outside the kind's table, or a bad shape, only flags needs-human.""" + for action in ("not-a-fit", "repro-ready; rm -rf /", "failed", "", "REPRO-READY", "x]"): + with self.subTest(action=action): + (self.tmp / "log.txt").unlink(missing_ok=True) + proc = self.run_script(action) + log = self.log() + self.assertEqual(proc.returncode, 0, proc.stderr) + self.assertEqual(log[0], NEEDS_HUMAN) + self.assertEqual(len(log), 2, log) + self.assertTrue( + log[1].startswith("gh issue comment 7 --body Automated triage ended") + ) + + def test_result_labels_are_ignored(self) -> None: + """Only the fixed table decides the label; a label field in the result is never read.""" + self.run_script("needs-info", meta_extra={"label": "claude:ok", "labels": "x"}) + edits = [c for c in self.log() if c.startswith("gh issue edit")] + self.assertEqual(edits, ["gh issue edit 7 --add-label bug:needs-info"]) + + def test_denied_path_blocks_the_push(self) -> None: + """A repro touching the workflows is applied locally only; the issue gets needs-human.""" + diff = ":100644 100644 a b M\\0.github/workflows/release.yml\\0" + proc = self.run_script("repro-ready", run_env={"FAKE_DIFF": diff}) + log = self.log() + self.assertEqual(proc.returncode, 1) + self.assertFalse([c for c in log if c.startswith("git push")]) + self.assertIn(NEEDS_HUMAN, log) + self.assertNotIn("gh issue edit 7 --add-label bug:repro-ready", log) + + def test_bad_base_sha_never_reaches_git(self) -> None: + """A non-hex base (including an option) fails before any fetch or api call.""" + for bad in ("--upload-pack=x", "abc", ""): + with self.subTest(base_sha=bad): + (self.tmp / "log.txt").unlink(missing_ok=True) + proc = self.run_script("repro-ready", meta_extra={"base_sha": bad}) + log = self.log() + self.assertEqual(proc.returncode, 1) + self.assertFalse([c for c in log if c.startswith(("git", "gh api"))]) + self.assertIn(NEEDS_HUMAN, log) + + def test_base_not_on_default_branch(self) -> None: + """A base that is not an ancestor of the default branch is refused.""" + proc = self.run_script("repro-ready", run_env={"FAKE_DIVERGED": "...main"}) + self.assertEqual(proc.returncode, 1) + self.assertFalse([c for c in self.log() if c.startswith(("git push", "git fetch"))]) + + def test_foreign_branch_is_not_overwritten(self) -> None: + """An existing branch that does not descend from the base is left alone.""" + proc = self.run_script( + "repro-ready", remote_sha=REMOTE, run_env={"FAKE_DIVERGED": f"...{REMOTE}"} + ) + self.assertEqual(proc.returncode, 1) + self.assertFalse([c for c in self.log() if c.startswith(("git push", "git fetch"))]) + + def test_push_failure_flags_needs_human(self) -> None: + """A failed push neither comments the result nor sets the stage label.""" + proc = self.run_script("repro-ready", fail="git push") + log = self.log() + self.assertEqual(proc.returncode, 1) + self.assertNotIn("gh issue edit 7 --add-label bug:repro-ready", log) + self.assertLess( + log.index(NEEDS_HUMAN), + next(i for i, c in enumerate(log) if c.startswith("gh issue comment 7")), + ) + + def test_comment_cannot_ping_users(self) -> None: + """An @ in the model's comment is broken so it notifies nobody.""" + self.run_script("needs-info", comment="cc @octocat") + comment = next(c for c in self.log() if c.startswith("gh issue comment 7")) + self.assertNotIn("@octocat", comment) + + def test_result_for_another_issue_is_refused(self) -> None: + """An artifact for a different issue changes nothing on it; this run's issue gets needs-human.""" + proc = self.run_script("needs-info", run_env={"ISSUE_NUMBER": "8"}) + self.assertEqual(proc.returncode, 1) + log = self.log() + self.assertEqual(log[0], "gh issue edit 8 --add-label needs-human") + self.assertTrue(log[1].startswith("gh issue comment 8 --body ")) + self.assertEqual(len(log), 2) + + def test_unreadable_meta_flags_needs_human(self) -> None: + """A missing, malformed or non-numeric meta.json is loud, not silent.""" + for body in (None, "not json", '{"issue_number": "7; x"}', "{}"): + with self.subTest(meta=body): + (self.tmp / "log.txt").unlink(missing_ok=True) + self.run_script("needs-info") + meta = self.result / "meta.json" + if body is None: + meta.unlink() + else: + meta.write_text(body) + (self.tmp / "log.txt").unlink(missing_ok=True) + env = { + **os.environ, + "PATH": f"{self.tmp / 'bin'}{os.pathsep}{os.environ['PATH']}", + "FAKE_LOG": str(self.tmp / "log.txt"), + "GITHUB_REPOSITORY": "o/r", + "DEFAULT_BRANCH": "main", + "ISSUE_NUMBER": "7", + } + proc = subprocess.run( # noqa: S603 -- fixed argv, trusted script + [str(SCRIPTS / "moderate_apply_triage.sh"), "bug", str(self.result)], + env=env, + capture_output=True, + text=True, + check=False, + timeout=30, + ) + self.assertEqual(proc.returncode, 1, proc.stderr) + log = self.log() + self.assertEqual(log[0], NEEDS_HUMAN) + self.assertEqual(len(log), 2) + + def test_plan_may_only_change_docs_plans(self) -> None: + """A plan touching anything outside docs/plans/ is never pushed.""" + diff = ":100644 100644 a b M\\0docs/plans/x.md\\0:100644 100644 a b M\\0src/app.py\\0" + proc = self.run_script("plan-ready", kind="feature", run_env={"FAKE_DIFF": diff}) + log = self.log() + self.assertEqual(proc.returncode, 1) + self.assertIn("outside docs/plans/", proc.stderr) + self.assertFalse([c for c in log if c.startswith("git push")]) + self.assertIn(NEEDS_HUMAN, log) + self.assertNotIn("gh issue edit 7 --add-label enhancement:plan-ready", log) + + def test_repro_is_not_limited_to_docs_plans(self) -> None: + """The repro branch keeps only the deny-list (tests anywhere are fine).""" + proc = self.run_script("repro-ready") + self.assertEqual(proc.returncode, 0, proc.stderr) + + +if __name__ == "__main__": + unittest.main() diff --git a/.github/scripts/test_moderate_tools.py b/.github/scripts/test_moderate_tools.py new file mode 100644 index 0000000..6ed9244 --- /dev/null +++ b/.github/scripts/test_moderate_tools.py @@ -0,0 +1,310 @@ +"""Tests for the moderation helpers that run next to Claude: path allow-list, packaging, launcher.""" + +from __future__ import annotations + +import json +import os +import shutil +import subprocess +import tempfile +import unittest +from pathlib import Path + +SCRIPTS = Path(__file__).resolve().parent +GIT_ENV = { + "GIT_AUTHOR_NAME": "t", + "GIT_AUTHOR_EMAIL": "t@example.com", + "GIT_COMMITTER_NAME": "t", + "GIT_COMMITTER_EMAIL": "t@example.com", +} + + +def run( + argv: list[str], cwd: Path | None = None, env: dict[str, str] | None = None, stdin: bytes = b"" +) -> subprocess.CompletedProcess[bytes]: + """Run a fixed argv with a merged environment.""" + return subprocess.run( # noqa: S603 -- fixed argv, trusted scripts + argv, + cwd=cwd, + env={**os.environ, **GIT_ENV, **(env or {})}, + input=stdin, + capture_output=True, + check=False, + timeout=60, + ) + + +class CheckPathsTest(unittest.TestCase): + """``moderate_check_paths.sh`` denies what runs with credentials and allows the rest.""" + + def verdict(self, *paths: str) -> int: + """Return the script's exit status for these paths.""" + data = b"".join(p.encode() + b"\0" for p in paths) + return run([str(SCRIPTS / "moderate_check_paths.sh")], stdin=data).returncode + + def test_denied(self) -> None: + """Each sensitive path is refused, alone and mixed in with fine ones.""" + denied = [ + ".github/workflows/release.yml", + ".github/scripts/run_claude.sh", + ".github/renovate.json", + ".devcontainer/Dockerfile", + ".devcontainer/stack/x/compose.yml", + ".claude/settings.json", + ".mcp.json", + ".pre-commit-config.yaml", + "Makefile", + "sub/Makefile", + "image.mk", + "deep/dir/build.mk", + "scripts/release.sh", + "scripts/release-publish.sh", + "scripts/develop.sh", + "scripts/post-create.d/x.sh", + "Dockerfile", + "app/Dockerfile.dev", + "compose.yml", + "svc/compose.override.yaml", + ".gitattributes", + ".gitmodules", + ] + for path in denied: + with self.subTest(path=path): + self.assertEqual(self.verdict(path), 1) + self.assertEqual(self.verdict("src/ok.py", path), 1) + + def test_allowed(self) -> None: + """Ordinary source, tests and docs pass, including look-alike names.""" + allowed = [ + "src/app/main.py", + "tests/test_a.py", + "docs/plans/2026-11-x.md", + "docs/Makefile.md", + "scripts/other.sh", + ".githubx/y", + "github/workflows/x.yml", + "README.md", + ] + self.assertEqual(self.verdict(*allowed), 0) + + def test_empty_input(self) -> None: + """No paths is fine.""" + self.assertEqual(self.verdict(), 0) + + def test_newline_in_a_path(self) -> None: + """Paths are NUL-separated, so a newline inside one cannot hide a denied name.""" + self.assertEqual(self.verdict("a\n.github/x"), 0) + self.assertEqual(self.verdict(".github/x\nb"), 1) + + +@unittest.skipUnless(shutil.which("jq") and shutil.which("git"), "jq and git are required") +class PackageResultTest(unittest.TestCase): + """``moderate_package_result.sh`` reads the agent's repository without trusting it.""" + + def setUp(self) -> None: + """Create a workspace repository with one base commit.""" + tmp = tempfile.TemporaryDirectory() + self.addCleanup(tmp.cleanup) + self.tmp = Path(tmp.name) + self.ws = self.tmp / "ws" + self.ws.mkdir() + self.git("init", "-q", "-b", "main") + (self.ws / "a.txt").write_text("base\n") + self.git("add", "a.txt") + self.git("commit", "-q", "-m", "base") + self.base = self.git("rev-parse", "HEAD").stdout.decode().strip() + (self.ws / ".moderation-out").mkdir() + self.out = self.tmp / "out" + + def git(self, *args: str) -> subprocess.CompletedProcess[bytes]: + """Run git in the workspace.""" + proc = run(["git", *args], cwd=self.ws) + self.assertEqual(proc.returncode, 0, proc.stderr) + return proc + + def commit(self, name: str, message: str = "fix: it\n\nbody text") -> None: + """Add a file and commit it.""" + (self.ws / name).write_text("new\n") + self.git("add", name) + self.git("commit", "-q", "-m", message) + + def package(self, mode: str, base: str | None = None) -> dict[str, str]: + """Run the script and return meta.json.""" + proc = run( + [ + str(SCRIPTS / "moderate_package_result.sh"), + base or self.base, + str(self.out), + str(self.ws), + mode, + ], + env={"GITHUB_ISSUE_NUMBER": "7"}, + ) + self.assertEqual(proc.returncode, 0, proc.stderr) + return json.loads((self.out / "meta.json").read_text()) + + def test_build_with_a_commit(self) -> None: + """A commit becomes a patch plus the commit's subject and body.""" + self.commit("b.txt") + meta = self.package("build") + self.assertEqual(meta["outcome"], "built") + self.assertEqual(meta["subject"], "fix: it") + self.assertEqual(meta["body"], "body text") + self.assertEqual(meta["base_sha"], self.base) + self.assertIn("b.txt", (self.out / "patch.diff").read_text()) + + def test_build_with_an_explanation(self) -> None: + """No commit but an outcome note: needs-human with the note.""" + (self.ws / ".moderation-out" / "outcome.md").write_text("stuck here") + meta = self.package("build") + self.assertEqual((meta["outcome"], meta["body"]), ("needs-human", "stuck here")) + self.assertFalse((self.out / "patch.diff").exists()) + + def test_build_with_nothing(self) -> None: + """No commit and no note: failed.""" + self.assertEqual(self.package("build")["outcome"], "failed") + + def test_symlinked_note_is_not_read(self) -> None: + """A note that is a symlink to a runner file never reaches the comment.""" + secret = self.tmp / "secret.txt" + secret.write_text("TOPSECRET") + (self.ws / ".moderation-out" / "outcome.md").symlink_to(secret) + meta = self.package("build") + self.assertEqual(meta["outcome"], "failed") + self.assertNotIn("TOPSECRET", json.dumps(meta)) + + def test_symlinked_output_directory_is_not_read(self) -> None: + """A `.moderation-out` that points elsewhere is not read either.""" + elsewhere = self.tmp / "elsewhere" + elsewhere.mkdir() + (elsewhere / "outcome.md").write_text("TOPSECRET") + shutil.rmtree(self.ws / ".moderation-out") + (self.ws / ".moderation-out").symlink_to(elsewhere) + meta = self.package("build") + self.assertEqual(meta["outcome"], "failed") + self.assertNotIn("TOPSECRET", json.dumps(meta)) + + def test_rewritten_history_gives_no_patch(self) -> None: + """When the base is no longer an ancestor of HEAD, nothing is packaged as a patch.""" + self.git("checkout", "-q", "--orphan", "other") + self.git("rm", "-rq", "--cached", ".") + self.commit("c.txt", "feat: unrelated") + meta = self.package("build") + self.assertEqual(meta["outcome"], "failed") + self.assertFalse((self.out / "patch.diff").exists()) + + def test_base_sha_must_be_hex(self) -> None: + """A malformed base is reported as failed and never passed to git.""" + meta = self.package("build", base="--upload-pack=x") + self.assertEqual(meta["outcome"], "failed") + + def test_triage_result(self) -> None: + """A valid triage.json is carried over with the commit's patch.""" + self.commit("t.py") + (self.ws / ".moderation-out" / "triage.json").write_text( + json.dumps({"action": "repro-ready", "comment": "it fails"}) + ) + meta = self.package("triage") + self.assertEqual((meta["action"], meta["comment"]), ("repro-ready", "it fails")) + self.assertTrue((self.out / "patch.diff").exists()) + + def test_triage_result_bad_shapes(self) -> None: + """Invalid JSON, wrong types and a symlinked file all become `failed`.""" + target = self.ws / ".moderation-out" / "triage.json" + for content in ("not json", '{"action": 1, "comment": "x"}', '{"action": "x"}'): + with self.subTest(content=content): + target.write_text(content) + self.assertEqual(self.package("triage")["action"], "failed") + target.unlink() + other = self.tmp / "o.json" + other.write_text(json.dumps({"action": "needs-info", "comment": "from outside"})) + target.symlink_to(other) + self.assertEqual(self.package("triage")["action"], "failed") + + +class RunClaudeTest(unittest.TestCase): + """``run_claude.sh`` takes the key from the environment only and hardens the claude call.""" + + def setUp(self) -> None: + """Put a recording fake ``claude`` first on PATH.""" + tmp = tempfile.TemporaryDirectory() + self.addCleanup(tmp.cleanup) + self.tmp = Path(tmp.name) + exe = self.tmp / "bin" / "claude" + exe.parent.mkdir() + exe.write_text( + "#!/usr/bin/env bash\n" + '{ printf "ARG:%s\\n" "$@"; printf "KEY:%s\\n" "${ANTHROPIC_API_KEY:-}";\n' + ' printf "SCRUB:%s\\n" "${CLAUDE_CODE_SUBPROCESS_ENV_SCRUB:-}";\n' + ' printf "AUTHOR:%s\\n" "${GIT_AUTHOR_NAME:-}"; } > "$FAKE_OUT"\n' + ) + exe.chmod(0o755) + self.prompt = self.tmp / "prompt.md" + self.prompt.write_text("do the thing") + self.out = self.tmp / "out.txt" + + def launch(self, env: dict[str, str]) -> subprocess.CompletedProcess[bytes]: + """Run the launcher with a clean-ish environment.""" + base = {k: v for k, v in os.environ.items() if k not in {"ANTHROPIC_API_KEY"}} + path = f"{self.tmp / 'bin'}{os.pathsep}{base['PATH']}" + return subprocess.run( # noqa: S603 -- fixed argv, trusted script + [str(SCRIPTS / "run_claude.sh"), str(self.prompt), "Read,Grep", "5", "30"], + cwd=self.tmp, + env={**base, "PATH": path, "FAKE_OUT": str(self.out), **env}, + capture_output=True, + check=False, + timeout=60, + ) + + def test_key_only_from_the_environment(self) -> None: + """The key reaches claude, the call is hardened, and no key file is involved.""" + proc = self.launch( + { + "ANTHROPIC_API_KEY": "sk-test", + "MODERATION_EXTRA_TOOLS": "Bash(uv run pytest *)", + "MODERATION_ALLOWED_DOMAINS": "github.com,api.github.com", + } + ) + self.assertEqual(proc.returncode, 0, proc.stderr) + lines = self.out.read_text().splitlines() + self.assertIn("KEY:sk-test", lines) + self.assertIn("SCRUB:1", lines) + self.assertIn("AUTHOR:claude-moderator[bot]", lines) + self.assertEqual(lines[lines.index("ARG:--allowedTools") + 1], "ARG:Read,Grep,Bash(uv run pytest *)") + settings = json.loads(lines[lines.index("ARG:--settings") + 1].removeprefix("ARG:")) + self.assertIn("Read(//proc/**)", settings["permissions"]["deny"]) + sandbox = settings["sandbox"] + self.assertIs(sandbox["enabled"], True) + self.assertIs(sandbox["failIfUnavailable"], True) + self.assertIs(sandbox["allowUnsandboxedCommands"], False) + self.assertIs(sandbox["autoAllowBashIfSandboxed"], False) + self.assertNotIn("enableWeakerNestedSandbox", sandbox) + self.assertEqual(sandbox["network"]["allowedDomains"], ["github.com", "api.github.com"]) + self.assertIs(sandbox["network"]["strictAllowlist"], True) + self.assertIn({"name": "ANTHROPIC_API_KEY", "mode": "deny"}, sandbox["credentials"]["envVars"]) + self.assertEqual(lines[lines.index("ARG:-p") + 1], "ARG:do the thing") + + def test_missing_or_bad_domains_fail_before_claude_runs(self) -> None: + """No network allow-list, or a malformed host, means claude never starts.""" + for domains in ("", ",", 'x.com","evil', "a b"): + with self.subTest(domains=domains): + proc = self.launch({"ANTHROPIC_API_KEY": "k", "MODERATION_ALLOWED_DOMAINS": domains}) + self.assertEqual(proc.returncode, 1) + self.assertFalse(self.out.exists()) + + def test_missing_key_fails_before_claude_runs(self) -> None: + """Without the key in the environment the launcher stops; claude is never started.""" + proc = self.launch({"MODERATION_ALLOWED_DOMAINS": "github.com"}) + self.assertEqual(proc.returncode, 1) + self.assertFalse(self.out.exists()) + + def test_no_key_file_and_no_git_config_in_the_launcher(self) -> None: + """The launcher never reads `.secrets/` nor writes `git config` into the tree.""" + text = (SCRIPTS / "run_claude.sh").read_text() + code = "\n".join(line for line in text.splitlines() if not line.lstrip().startswith("#")) + self.assertNotIn(".secrets", code) + self.assertNotIn("git config", code) + + +if __name__ == "__main__": + unittest.main() diff --git a/.github/scripts/test_release_scripts.py b/.github/scripts/test_release_scripts.py index 56bd045..9d9cb2b 100644 --- a/.github/scripts/test_release_scripts.py +++ b/.github/scripts/test_release_scripts.py @@ -3,8 +3,10 @@ from __future__ import annotations import contextlib +import hashlib import io import itertools +import json import os import re import shutil @@ -610,74 +612,53 @@ def test_workflow_uses_the_script(self) -> None: self.assertNotIn("commits/refs/tags", text) -class DevelopScriptRetryTest(unittest.TestCase): - """``scripts/develop.sh`` never nests ``fetch``'s retries inside ``retry``'s.""" +class DevelopScriptTest(unittest.TestCase): + """``scripts/develop.sh``: bounded retries, and no unverified download is ever run.""" + + text = (SCRIPTS.parents[1] / "scripts" / "develop.sh").read_text() def test_retried_steps_do_not_download(self) -> None: - """A function passed to ``retry`` neither calls ``fetch`` nor ``download_installer``.""" - text = (SCRIPTS.parents[1] / "scripts" / "develop.sh").read_text() - retried = re.findall(r"^retry (\w+)", text, re.MULTILINE) - self.assertGreaterEqual(len(retried), 3) + """A function passed to ``retry`` never calls ``fetch`` (retries would multiply).""" + retried = re.findall(r"^retry (\w+)", self.text, re.MULTILINE) + self.assertGreaterEqual(len(retried), 1) for name in retried: - body = re.search(rf"^{name}\(\) \{{\n(.*?)^\}}", text, re.MULTILINE | re.DOTALL) - self.assertIsNotNone(body, name) - self.assertNotRegex(body.group(1), r"\b(fetch|download_installer)\b", name) # type: ignore[union-attr] - - def test_failed_installer_download_names_the_installer(self) -> None: - """A permanent download failure says which installer failed, and still aborts.""" - text = (SCRIPTS.parents[1] / "scripts" / "develop.sh").read_text() - parts = [] - for name in ("tmp_path", "fetch", "download_installer"): - body = re.search(rf"^{name}\(\) \{{\n.*?^\}}\n", text, re.MULTILINE | re.DOTALL) + body = re.search(rf"^{name}\(\) \{{\n(.*?)^\}}", self.text, re.MULTILINE | re.DOTALL) self.assertIsNotNone(body, name) - parts.append(body.group(0)) # type: ignore[union-attr] - with tempfile.TemporaryDirectory() as tmp: - curl = Path(tmp) / "curl" - curl.write_text("#!/usr/bin/env bash\necho 'curl: (22) 404' >&2\nexit 22\n") - curl.chmod(0o755) - script = ( - 'set -euo pipefail\ncleanup_paths=()\ntmp_result=""\ninstaller_script=""\n' - + "".join(parts) - + 'download_installer uv "https://example.invalid/uv-installer.sh"\n' - + "echo not-reached\n" + self.assertNotRegex(body.group(1), r"\bfetch(_checked)?\b", name) # type: ignore[union-attr] + + def test_no_vendor_installer_script_is_run(self) -> None: + """uv, prek and Claude Code are verified binaries, never a downloaded install script.""" + self.assertNotRegex(self.text, r"installer\.sh|claude\.ai/install|/install\.sh") + self.assertNotIn("download_installer", self.text) + for tool in ("uv", "prek"): + self.assertRegex( + self.text, rf'fetch_checked "https://github\.com/[^"]*/\${{{tool}_asset}}\.tar\.gz"' ) - result = subprocess.run( # noqa: S603 -- fixed argv - ["bash", "-c", script], # noqa: S607 - env={**os.environ, "PATH": f"{tmp}{os.pathsep}{os.environ['PATH']}"}, - capture_output=True, - text=True, - check=False, - timeout=30, - ) - self.assertEqual(result.returncode, 22, result.stderr) - self.assertNotIn("not-reached", result.stdout) - self.assertIn("uv installer", result.stderr) - self.assertIn("https://example.invalid/uv-installer.sh", result.stderr) - self.assertIn("curl exit 22", result.stderr) - text_calls = re.findall(r"^download_installer (\S+) ", text, re.MULTILINE) - self.assertEqual(text_calls, ["uv", "prek", "claude"]) + self.assertIn("install_claude ", self.text) + self.assertRegex(self.text, r"(?m)^claude_signing_fpr=[0-9A-F]{40}$") - def run_download(self, curl_body: str) -> tuple[subprocess.CompletedProcess[str], int]: - """Run ``download_installer`` against a fake ``curl``; return the process and curl calls.""" - text = (SCRIPTS.parents[1] / "scripts" / "develop.sh").read_text() + def run_bash( + self, functions: tuple[str, ...], body: str, curl_body: str + ) -> tuple[subprocess.CompletedProcess[str], int]: + """Run ``body`` with ``develop.sh``'s helper functions and a fake ``curl``; return it and the curl call count.""" parts = [] - for name in ("tmp_path", "fetch", "download_installer"): - body = re.search(rf"^{name}\(\) \{{\n.*?^\}}\n", text, re.MULTILINE | re.DOTALL) - self.assertIsNotNone(body, name) - parts.append(body.group(0)) # type: ignore[union-attr] + for name in functions: + found = re.search(rf"^{name}\(\) \{{\n.*?^\}}\n", self.text, re.MULTILINE | re.DOTALL) + self.assertIsNotNone(found, name) + parts.append(found.group(0)) # type: ignore[union-attr] with tempfile.TemporaryDirectory() as tmp: - for name, body in ( + for name, content in ( ("curl", f'#!/usr/bin/env bash\necho x >> "{tmp}/calls"\n{curl_body}\n'), ("sleep", "#!/usr/bin/env bash\nexit 0\n"), ): path = Path(tmp) / name - path.write_text(body) + path.write_text(content) path.chmod(0o755) + (Path(tmp) / "calls").touch() script = ( - 'set -euo pipefail\ncleanup_paths=()\ntmp_result=""\ninstaller_script=""\n' + 'set -euo pipefail\ncleanup_paths=()\ntmp_result=""\nfetch_label=""\n' + "".join(parts) - + 'download_installer uv "https://example.invalid/uv-installer.sh"\n' - + 'echo "installed: $(cat "$installer_script")"\n' + + body.replace("TMP", tmp) ) result = subprocess.run( # noqa: S603 -- fixed argv ["bash", "-c", script], # noqa: S607 @@ -690,35 +671,297 @@ def run_download(self, curl_body: str) -> tuple[subprocess.CompletedProcess[str] calls = (Path(tmp) / "calls").read_text().count("x") return result, calls + def run_fetch(self, curl_body: str) -> tuple[subprocess.CompletedProcess[str], int]: + """Run one labelled ``fetch`` against a fake ``curl``.""" + return self.run_bash( + ("tmp_path", "fetch"), + 'fetch_label="uv tarball" fetch "https://example.invalid/uv.tar.gz" -o TMP/out\necho not-reached\n', + curl_body, + ) + + def test_failed_download_names_what_failed(self) -> None: + """A permanent download failure says which download failed, and still aborts.""" + result, calls = self.run_fetch("echo 'curl: (22) 404' >&2\nexit 22") + self.assertEqual(result.returncode, 22, result.stderr) + self.assertEqual(calls, 1) + self.assertNotIn("not-reached", result.stdout) + self.assertIn("uv tarball", result.stderr) + self.assertIn("https://example.invalid/uv.tar.gz", result.stderr) + self.assertIn("curl exit 22", result.stderr) + def test_transient_curl_failures_are_retried_by_fetch(self) -> None: """DNS (6) and connect (7) failures retry inside fetch, not abort.""" for status in (6, 7): with self.subTest(status=status): - # Fail twice, then write the installer to the -o target. + # Fail twice, then write the file to the -o target. body = ( f'n=$(wc -l < "$(dirname "$0")/calls")\n' f"if [ $n -le 2 ]; then echo 'curl: ({status})' >&2; exit {status}; fi\n" 'while [ $# -gt 0 ]; do [ "$1" = -o ] && out=$2; shift; done\n' 'echo script > "$out"\n' ) - result, calls = self.run_download(body) + result, calls = self.run_fetch(body) + self.assertIn("not-reached", result.stdout) self.assertEqual(result.returncode, 0, result.stderr) self.assertEqual(calls, 3) - self.assertIn("installed: script", result.stdout) def test_persistent_curl_failure_gives_up_after_bounded_retries(self) -> None: - """A DNS failure that never clears aborts after 4 tries, naming the installer.""" - result, calls = self.run_download("exit 6") + """A DNS failure that never clears aborts after 4 tries, naming the download.""" + result, calls = self.run_fetch("exit 6") self.assertEqual(result.returncode, 6, result.stderr) self.assertEqual(calls, 4) - self.assertIn("uv installer", result.stderr) - - def test_empty_installer_download_fails(self) -> None: - """A download that leaves an empty file fails instead of installing nothing.""" - body = 'while [ $# -gt 0 ]; do [ "$1" = -o ] && out=$2; shift; done\n: > "$out"\n' - result, _ = self.run_download(body) - self.assertNotEqual(result.returncode, 0) - self.assertIn("installer is empty", result.stderr) + self.assertIn("uv tarball", result.stderr) + + def run_fetch_checked(self, checksum_line: str) -> subprocess.CompletedProcess[str]: + """Run ``fetch_checked`` against a fake ``curl`` serving ``tool.tar.gz`` and ``checksum_line``.""" + curl_body = ( + 'while [ $# -gt 0 ]; do case "$1" in -o) out=$2;; http*) url=$1;; esac; shift; done\n' + f'case "$url" in *.sha256) printf \'%s\\n\' \'{checksum_line}\' > "$out";; *) printf payload > "$out";; esac\n' + ) + result, _ = self.run_bash( + ("tmp_path", "fetch", "fetch_checked"), + 'tmp_path -d\nfetch_checked "https://example.invalid/tool.tar.gz" "$tmp_result"\necho verified\n', + curl_body, + ) + return result + + def test_fetch_checked_accepts_a_matching_checksum(self) -> None: + """The file is accepted when the published checksum for its own name matches.""" + digest = hashlib.sha256(b"payload").hexdigest() + result = self.run_fetch_checked(f"{digest} tool.tar.gz") + self.assertEqual(result.returncode, 0, result.stderr) + self.assertIn("verified", result.stdout) + + def test_fetch_checked_rejects_a_wrong_or_missing_checksum(self) -> None: + """A mismatching checksum, or one listed for another name, aborts before anything is used.""" + digest = hashlib.sha256(b"payload").hexdigest() + for line in (f"{'0' * 64} tool.tar.gz", f"{digest} other.tar.gz", ""): + with self.subTest(line=line): + result = self.run_fetch_checked(line) + self.assertNotEqual(result.returncode, 0) + self.assertNotIn("verified", result.stdout) + + @staticmethod + def gpg_key(home: Path, name: str) -> tuple[str, bytes]: + """Create a throwaway signing key in ``home``; return its fingerprint and armored public key.""" + env = {**os.environ, "GNUPGHOME": str(home)} + base = ["gpg", "--batch", "--quiet", "--pinentry-mode", "loopback", "--passphrase", ""] + subprocess.run( # noqa: S603 -- fixed argv + [ + *base, + "--quick-generate-key", + f"{name} <{name}@example.invalid>", + "ed25519", + "sign", + "never", + ], + env=env, + check=True, + capture_output=True, + timeout=60, + ) + colons = subprocess.run( # noqa: S603 -- fixed argv + ["gpg", "--batch", "--with-colons", "--list-keys", name], # noqa: S607 + env=env, + check=True, + capture_output=True, + text=True, + timeout=60, + ).stdout + fpr = next(line.split(":")[9] for line in colons.splitlines() if line.startswith("fpr")) + armored = subprocess.run( # noqa: S603 -- fixed argv + ["gpg", "--batch", "--armor", "--export", name], # noqa: S607 + env=env, + check=True, + capture_output=True, + timeout=60, + ).stdout + return fpr, armored + + def run_install_claude( + self, + *, + pin: str = "good", + signer: str = "good", + manifest_version: str = "1.2.3", + manifest_checksum: str | None = None, + binary: bytes = b"claude-binary", + ) -> subprocess.CompletedProcess[str]: + """Run ``install_claude`` against a fake ``curl`` serving a key, a manifest signed by ``signer``, and ``binary``. + + ``pin`` and ``signer`` pick which of two throwaway keys (``good``, + ``other``) is the pinned fingerprint and which signs the manifest; + the served key is always ``good``. + """ + with tempfile.TemporaryDirectory() as tmp: + root = Path(tmp) + homes = {} + for name in ("good", "other"): + homes[name] = root / f"gh-{name}" + homes[name].mkdir(mode=0o700) + try: + keys = {name: self.gpg_key(homes[name], name) for name in homes} + if manifest_checksum is None: + manifest_checksum = hashlib.sha256(b"claude-binary").hexdigest() + manifest = json.dumps( + { + "version": manifest_version, + "platforms": {"linux-x64": {"checksum": manifest_checksum}}, + } + ) + served = root / "served" + served.mkdir() + (served / "key.asc").write_bytes(keys["good"][1]) + (served / "manifest.json").write_text(manifest) + (served / "claude").write_bytes(binary) + subprocess.run( # noqa: S603 -- fixed argv + [ + "gpg", + "--batch", + "--quiet", + "--local-user", + keys[signer][0], + "--detach-sign", + "-o", + str(served / "manifest.json.sig"), + str(served / "manifest.json"), + ], # noqa: S607 + env={**os.environ, "GNUPGHOME": str(homes[signer])}, + check=True, + capture_output=True, + timeout=60, + ) + finally: + for home in homes.values(): + subprocess.run( # noqa: S603 -- fixed argv + ["gpgconf", "--homedir", str(home), "--kill", "all"], # noqa: S607 + check=False, + capture_output=True, + timeout=30, + ) + curl_body = ( + 'while [ $# -gt 0 ]; do case "$1" in -o) out=$2;; http*) url=$1;; esac; shift; done\n' + f'name="${{url##*/}}"; [ "$name" = claude-code.asc ] && name=key.asc\n' + f'cp "{served}/$name" "$out"\n' + ) + result, _ = self.run_bash( + ("tmp_path", "fetch", "install_claude"), + f"claude_signing_fpr={keys[pin][0]}\n" + "claude_release_base=https://example.invalid/releases\n" + "claude_key_url=https://example.invalid/claude-code.asc\n" + "sudo() { echo SUDO-RAN; }\n" + 'tmp_path -d\ninstall_claude 1.2.3 linux-x64 /cfg "$tmp_result"\n', + curl_body, + ) + return result + + @unittest.skipUnless(shutil.which("gpg") and shutil.which("jq"), "needs gpg and jq") + def test_install_claude_installs_a_verified_release(self) -> None: + """The control: a pinned key, a manifest it signed and a matching binary install.""" + result = self.run_install_claude() + self.assertEqual(result.returncode, 0, result.stderr) + self.assertIn("SUDO-RAN", result.stdout) + + @unittest.skipUnless(shutil.which("gpg") and shutil.which("jq"), "needs gpg and jq") + def test_install_claude_fails_closed(self) -> None: + """Each broken link in the chain stops the install before the binary is run.""" + digest = hashlib.sha256(b"claude-binary").hexdigest() + cases = { + "served key is not the pinned fingerprint": ({"pin": "other"}, "fingerprint"), + "manifest signed by another key": ({"signer": "other"}, "not signed by"), + "manifest is for another version": ({"manifest_version": "9.9.9"}, "expected 1.2.3"), + "manifest checksum is not hex": ({"manifest_checksum": "zz" * 32}, "no valid checksum"), + "manifest checksum is too short": ( + {"manifest_checksum": digest[:40]}, + "no valid checksum", + ), + "binary does not match the manifest": ({"binary": b"tampered"}, "FAILED"), + } + for label, (kwargs, message) in cases.items(): + with self.subTest(label): + result = self.run_install_claude(**kwargs) # type: ignore[arg-type] + self.assertNotEqual(result.returncode, 0) + self.assertNotIn("SUDO-RAN", result.stdout) + self.assertIn(message, result.stderr + result.stdout) + + +class ReleaseWorkflowGuardTest(unittest.TestCase): + """``release.yml`` runs only from the default branch.""" + + text = (SCRIPTS.parent / "workflows" / "release.yml").read_text() + guard = "github.ref == format('refs/heads/{0}', github.event.repository.default_branch)" + + def job(self, name: str) -> str: + """Return the text of top-level job ``name``.""" + found = re.search( + rf"^ {name}:\n(.*?)(?=^ [a-z][\w-]*:\n|\Z)", self.text, re.MULTILINE | re.DOTALL + ) + self.assertIsNotNone(found, name) + return found.group(1) # type: ignore[union-attr] + + def test_version_job_fails_first_on_another_ref(self) -> None: + """The first step of the job every other job needs refuses a non-default ref.""" + version = self.job("version") + first_step = version.split("steps:\n", 1)[1].split("\n\n", 1)[0] + self.assertIn("refs/heads/${{ github.event.repository.default_branch }}", first_step) + self.assertIn("exit 1", first_step) + + def test_every_job_with_write_permissions_or_secrets_is_gated(self) -> None: + """Jobs holding ``contents: write``, ``packages: write``, ``id-token`` or secrets check the ref.""" + for name in ("build", "release", "publish-packages"): + with self.subTest(job=name): + self.assertIn(self.guard, self.job(name)) + # Nothing else gets write permissions or secrets without needing the version job. + for match in re.finditer( + r"^ ([a-z][\w-]*):\n", self.text.split("\njobs:\n", 1)[1], re.MULTILINE + ): + name = match.group(1) + if name != "version": + self.assertRegex(self.job(name), r"needs: ", name) + + def test_privileged_jobs_declare_the_release_environment(self) -> None: + """Jobs with write permissions, an OIDC token or secrets run in ``release``, and no others.""" + # Any write permission (including write-all), any OIDC token, and any + # reference to secrets other than GITHUB_TOKEN (dotted, bracketed, + # `secrets: inherit`, or the whole context). + privileged = re.compile( + r"[\w-]+: write\b|write-all|secrets\s*:\s*inherit" + r"|secrets\s*\[|secrets\s*\.\s*(?!GITHUB_TOKEN\b)" + ) + names = re.findall(r"^ ([a-z][\w-]*):\n", self.text.split("\njobs:\n", 1)[1], re.MULTILINE) + found = [] + for name in names: + # Comments mention these keys too; only the YAML itself counts. + body = "\n".join( + line for line in self.job(name).splitlines() if not line.lstrip().startswith("#") + ) + has_env = re.search(r"^ environment: release$", body, re.MULTILINE) is not None + with self.subTest(job=name): + self.assertEqual(has_env, bool(privileged.search(body))) + if has_env: + found.append(name) + self.assertTrue(found, "no job declares environment: release") + + def test_environment_is_a_plain_name_not_an_expression(self) -> None: + """A dispatcher-controlled expression could pick an unrestricted environment.""" + matches = list(re.finditer(r"^[ \t]+environment:[ \t]*(.*)$", self.text, re.MULTILINE)) + self.assertTrue(matches, "no environment: line found") + for match in matches: + self.assertEqual(match.group(1).strip(), "release") + + def test_environment_is_documented_for_instances(self) -> None: + """The setup step and the allowed-secrets move are in the instance-facing docs.""" + docs = SCRIPTS.parent.parent / "docs" / "TEMPLATE.md" + text = docs.read_text() + self.assertIn("Release environment", text) + self.assertIn("gh secret set NAME --env release", text) + readme = (SCRIPTS.parent / "workflows" / "README.md").read_text() + self.assertIn("environment: release", readme) + + def test_release_is_created_at_the_dispatched_sha(self) -> None: + """The tag and release target ``github.sha``, the tip of the (default) branch dispatched.""" + self.assertIn("TARGET_SHA: ${{ github.sha }}", self.text) + self.assertIn('--target "$TARGET_SHA"', self.text) if __name__ == "__main__": diff --git a/.github/template-sync-manifest.yml b/.github/template-sync-manifest.yml index 3a43381..361a723 100644 --- a/.github/template-sync-manifest.yml +++ b/.github/template-sync-manifest.yml @@ -108,6 +108,7 @@ replace: - scripts/image-scan.sh - scripts/image-dast.sh - scripts/skopeo.sh + - scripts/skopeo-dir.sh - scripts/test-image-scripts.sh # template-react's own layer on top of docker's: the runtime image (an diff --git a/.github/template-sync-state.json b/.github/template-sync-state.json index a38a593..96d6131 100644 --- a/.github/template-sync-state.json +++ b/.github/template-sync-state.json @@ -1,4 +1,4 @@ { "template_repo": "craftainer/template-docker", - "synced_tag": "v1.8.2" + "synced_tag": "v1.8.3" } diff --git a/.github/workflows/README.md b/.github/workflows/README.md index 2b9a02a..e12ecd6 100644 --- a/.github/workflows/README.md +++ b/.github/workflows/README.md @@ -6,7 +6,15 @@ [`devcontainers/ci`](https://github.com/devcontainers/ci)), under a per-run `COMPOSE_PROJECT_NAME` that a final step tears down. Runs as a two-leg `amd64`/`arm64` matrix (see "Architecture matrix" below). -- `release.yml` — manually triggered. Takes a release channel +- `release.yml` — manually triggered, and only from the default branch: + the `version` job's first step fails on any other ref, and `build`, + `release` and `publish-packages` repeat the check in their own `if:`. + Those three jobs (the ones with `contents: write`, `packages: write`, + `id-token: write` or registry/npm secrets) also declare `environment: + release`: the server-side rule, with deployment branches limited to the + default branch and the registry/npm secrets held in that environment + (setup: `../../docs/TEMPLATE.md`, "Release environment"). + Takes a release channel (`alpha`/`beta`/`rc`/`full`) and a SemVer 2 bump (`major`/`minor`/`patch`/`none`), computes the next tag via `../scripts/compute_next_version.py`, then drives the instance's @@ -95,6 +103,57 @@ plan on a branch, and asks a maintainer to confirm (the `enhancement:confirm` label) before a second stage implements it and opens a PR. +**Every Claude job is read-only; a separate `apply` job writes.** All four +Claude-invoking workflows have the same shape (the threat model, the +alternatives and the sources are in +[`https://github.com/craftainer/template-base/blob/main/docs/adrs/0001-harden-the-issue-moderation-workflows.md`](https://github.com/craftainer/template-base/blob/main/docs/adrs/0001-harden-the-issue-moderation-workflows.md)): + +- The Claude job (`triage`/`fix`/`build`) holds `contents: read` (triage + also `issues: read`, for `gh issue view`) and nothing else. Its only + output is a `moderation-result` artifact (`meta.json` plus an optional + `patch.diff`): data, nothing executable. +- The `apply` job runs no Claude and no code from the issue, the repro/plan + branch or the artifact. It checks out the default branch, validates + every value it reads and only then pushes, comments and labels. Triage + now has an `apply` job too: Claude no longer comments, labels or pushes + the repro/plan branch itself; `moderate_apply_triage.sh` does, from + Claude's `action` (checked against a fixed table of three names per + kind, each mapped to one fixed label) and `comment` (capped at 10000 + characters, `@` broken so it pings nobody, and a branch-diff link added + by the script). +- What `apply` validates: the issue number is the run's issue; `base_sha` + is 40 lowercase hex characters, never reaches git as an option (refs + follow `--`), and is an ancestor of the default branch or, for a fix or + build, of the bot branch's current tip (checked with the compare API, so + there is no clone depth limit); the patch is at most 2 MiB; and the + commits the patch produces pass `moderate_check_paths.sh`. That + allow-list refuses any change under `.github/`, `.devcontainer/`, + `.claude/` or `.vscode/`, to `.mcp.json`, `.pre-commit-config.yaml`, + `.gitattributes`, `.gitmodules`, a `Makefile` or `*.mk`, a `Dockerfile*` + or `compose*.y*ml`, `scripts/release*`, `scripts/develop.sh`, + `scripts/post-create.sh` and their `.d/` directories, plus any symlink or + submodule. A fix that needs one of those becomes a `needs-human` note, + not a PR. +- The default branch is the base for the PR (`--base` is the repository's + default branch, not a hard-coded `main`). + +**Claude never runs from its own tree.** The job snapshots the default +branch (`git archive` into `$RUNNER_TEMP/trusted`, outside the +devcontainer's bind mount) and everything with credentials comes from that +snapshot: `moderate_prepare.sh` copies the launcher, the prompts and the +`prek` configuration to `.moderation-tools/` (hidden from git through +`.git/info/exclude`), and `moderate_package_result.sh` runs from the +snapshot too. It reads the commits through a fresh bare repository, not +through the agent's `.git/config`, and reads Claude's note or triage file +only if it is a regular file really under `.moderation-out/` (a symlink +there is ignored, since the runner would otherwise follow it to a runner +file). Fix/build additionally run `moderate_verify_branch.sh` before +Claude starts: the repro/plan branch must not differ from its merge base +with the default branch in any denied path, because the devcontainer and +image build read the branch's own `.devcontainer/`, `Dockerfile` and +`scripts/develop.sh`. If it does, no Claude step runs and the result is a +`needs-human` note. + **Fix/build is one workflow with two jobs, to keep Claude out of any job that can push/PR/relabel.** `moderate-bug-fix.yml` / `moderate-feature-build.yml` start on `issues: [labeled]` for the @@ -108,8 +167,9 @@ and runs Claude against the issue's untrusted title/body/comments, but never holds write-scoped credentials. Claude commits locally (or, if it can't converge, writes an explanation to `.moderation-outcome.md`) and the job packages that as a `moderation-result` artifact -(`../scripts/moderate_package_result.sh`: a `git format-patch` diff plus a -small JSON outcome record — data, nothing executable). The `apply` job +(`../scripts/moderate_package_result.sh`, run from the trusted snapshot: a +`git format-patch` diff plus a small JSON outcome record — data, nothing +executable). The `apply` job (`needs:` the first one, its own job-level `permissions: contents: write, issues: write, pull-requests: write`) runs from the default branch's checkout, downloads that artifact and does the actual `git @@ -137,11 +197,12 @@ under the concurrency group. `apply` is idempotent, so that retry is safe after a partial first run: - The branch (`/-`) is bot-owned: triage's - Claude run creates it and only `apply` writes to it afterwards. A - retry's `git am` makes new SHAs, so `apply` pushes with + `apply` job creates it from the validated patch and only the `apply` + jobs write to it afterwards. A retry's `git am` makes new SHAs, so + `apply` pushes with `--force-with-lease=:`: it fails if - anyone pushed in between, and it refuses to overwrite a tip that is not - the base SHA or a descendant of it (foreign work). + anyone pushed in between, and it refuses to overwrite a tip that does + not descend from the base SHA (foreign work). - An open PR for the branch (`gh pr list --head`) is reused, never duplicated. - The stage label swaps in one `gh issue edit` (`*-ready` to @@ -156,9 +217,6 @@ after a partial first run: re-added repeatedly during a run, each queued run can post its own not-built comment (the ready label is still present, so the re-check below does not skip them). -- The foreign-work check fetches with `--depth=100`. A remote tip more - than 100 commits ahead of the base SHA is misjudged as foreign and - fails with the generic "Applying the automated result failed" message. **The confirm label means "run attempted".** `gate` consumes it even when the run is then skipped (missing `ANTHROPIC_API_KEY`, untrusted author @@ -194,9 +252,9 @@ and a usage alert at 80% of it) and storing that key as the `ANTHROPIC_API_KEY` repository secret, then running `moderate-setup.yml` once to create the labels below. -Two independent label sequences track state, applied by `gh issue edit` -inside each stage's Claude prompt (`../scripts/prompts/`) or by a -deterministic workflow step: +Two independent label sequences track state, applied by the `apply` jobs +(`gh issue edit`) from a fixed action-to-label table, or by a deterministic +workflow step: - Bug: `bug` → (`bug:needs-info` | `bug:repro-ready`) → `bug:fixed`; a maintainer adds `bug:confirm` to start the fix (the label is consumed when the run starts). @@ -237,31 +295,95 @@ issue carried when closed, reopening it strips every `bug:*`/ `enhancement:*` stage label back to the bare `bug`/`enhancement` label (`../scripts/reset_issue_state.sh`) and re-runs triage as if new, rather than resuming stale state — the repo may have changed, or a prior -branch/PR may already be gone via cleanup. +branch/PR may already be gone via cleanup. The reset edits labels and +deletes branches, so it runs in the triage workflow's `apply` job, ahead +of applying the new result, not in the read-only Claude job. **Environment.** All four Claude-invoking jobs run `claude` the same way `checks.yml` runs `prek`: inside the actual devcontainer (`develop` stage) via `devcontainers/ci`, so Claude runs the exact CLI build pinned -in `.devcontainer/Dockerfile`'s `CLAUDE_CODE_VERSION`. Each writes the -`ANTHROPIC_API_KEY` secret to `.secrets/claude.txt` first (matching -`.secrets/README.md`'s "one file per secret" convention) and removes it -in an `if: always()` step; `../scripts/run_claude.sh` reads it, sets a -dedicated `claude-moderator[bot]` git identity for the commits this -pipeline makes, and invokes `claude -p --model claude-sonnet-5 ---effort medium --max-turns --allowedTools ` under a -`timeout`. Every job caps wall-clock (`timeout-minutes:` 10 for triage, -20 for fix/build) and turns (15 for triage, 40 for fix/build) so a -runaway prompt fails closed instead of burning CI/API budget; the actual -spend cap lives in the Anthropic Console (a dedicated workspace/key with -a monthly limit), since `claude` itself has no such flag. - -**Extra tools.** The read-only Claude jobs are limited to `git`, `gh`, -file tools and `prek run`. An instance that needs Claude to run more -(e.g. a targeted repro) sets the optional `MODERATION_EXTRA_TOOLS` -repository variable to extra comma-separated Claude Code `--allowedTools` -entries, e.g. `Bash(uv run pytest *)`; it's appended to each read-only -job's tool list and never applies to the `apply` jobs, which don't -run Claude. +in `.devcontainer/Dockerfile`'s `CLAUDE_CODE_VERSION`. The +`ANTHROPIC_API_KEY` secret is never written to a file: the step lists it +(and `MODERATION_EXTRA_TOOLS`, `MODERATION_ALLOWED_DOMAINS`, `GITHUB_ISSUE_NUMBER` and, for triage, +`GH_TOKEN`) under `with: env:`, which `devcontainers/ci` hands to +`devcontainer exec --remote-env`, so it exists only in the environment of +the one `run_claude.sh` command. **A step-level `env:` alone does not reach +the container**: the action passes only the names in `with: env:` (or all +of the runner's variables with `inheritEnv: true`) to the command, while +the step's own `env:` stays in the action process, which is why +`GITHUB_TOKEN` there is only the image build's `github_token` secret. (Before +this hardening the moderation steps relied on the step `env:` for +`GH_TOKEN`; `checks.yml` was already correct.) `run_claude.sh` sets a +dedicated `claude-moderator[bot]` git identity through `GIT_AUTHOR_*` / +`GIT_COMMITTER_*` (nothing is written to `.git/config`), sets +`CLAUDE_CODE_SUBPROCESS_ENV_SCRUB=1` so Bash commands, hooks and MCP +servers Claude starts do not get the key, passes inline `--settings` +deny rules (the command-line level of Claude Code's settings precedence, +so a project `.claude/settings.json` cannot loosen them) for `/proc`, +`/sys`, `~/.claude`, `~/.config/gh`, `.moderation-tools/` and `.git/`, and +invokes `claude -p --model claude-sonnet-5 --effort medium +--max-turns --allowedTools ` under a `timeout`. Every job caps +wall-clock (`timeout-minutes:` 10 for triage, 20 for fix/build) and turns +(15 for triage, 40 for fix/build) so a runaway prompt fails closed instead +of burning CI/API budget; the actual spend cap lives in the Anthropic +Console (a dedicated workspace/key with a monthly limit), since `claude` +itself has no such flag. + +**Tool limits.** `--allowedTools` is an allow-list for an agent that is +already in a hostile prompt, not a sandbox, so it is narrow and the +privileged work is elsewhere. Every Claude job gets `Read`, `Grep`, `Glob`, +`Write`, `Edit` and these Bash patterns, each matched as a whole command +(compound commands are split and each part must match): + +- git, local subcommands only: `status`, `diff`, `log`, `show`, `add`, + `commit`, `restore`, `rm`, `mv`, `ls-files`, `rev-parse` (triage also + `checkout -b` and `switch -c`). No `git *`: nothing can reach `-c`, + `config`, `push`, `remote`, `fetch`, `clone` or an alias. +- `prek run --config .moderation-tools/pre-commit-config.yaml *` for + triage-bug, fix and build: the hook configuration is the trusted copy, + not the tree's own `.pre-commit-config.yaml`. Only the config is + trusted: its `local`/`script` hooks run repository scripts from the tree + Claude edits, so those scripts are agent-editable and the sandbox, not + the config copy, is what contains them. +- `gh issue view *` and `gh repo view *`, triage only (read-only token). + No `gh issue comment`/`edit`, no `gh pr`, no `gh api`. + +**Extra tools.** An instance that needs Claude to run more (e.g. a +targeted repro) sets the optional `MODERATION_EXTRA_TOOLS` repository +variable to extra comma-separated Claude Code `--allowedTools` entries, +e.g. `Bash(uv run pytest *)`. It reaches `run_claude.sh` as an +environment variable (never spliced into a shell command), is appended +to each Claude job's tool list and never applies to the `apply` jobs, +which don't run Claude. Whatever it adds widens what a prompt injection +can do inside the job, so treat it like the API key's scope. + +**Sandbox (fail closed).** Code Claude writes and then runs through `prek` +(a test, a hook) would otherwise run as the same user as `claude`, with +network access, and could read the claude process's environment. +`run_claude.sh` therefore turns on Claude Code's built-in sandbox +() through the inline +`--settings`: `sandbox.enabled`, `failIfUnavailable: true` (claude exits at +startup if bubblewrap cannot run), `allowUnsandboxedCommands: false` (no +retry outside the sandbox), `autoAllowBashIfSandboxed: false` (the +`--allowedTools` list still decides which Bash commands run), +`network.allowedDomains` from `MODERATION_ALLOWED_DOMAINS` with +`strictAllowlist: true` (triage: `github.com,api.github.com`, fix/build: +`github.com`, plus the optional `MODERATION_EXTRA_DOMAINS` repository +variable, e.g. a package registry), `credentials.envVars` denying +`ANTHROPIC_API_KEY` in every sandboxed command, `filesystem.denyRead` for +the container user's credential locations and `allowWrite` of `~/.cache`. +`enableWeakerNestedSandbox` is deliberately not set: it would bind-mount the +container's `/proc` and expose the claude process's environment. The +sandbox covers Bash and what it starts, not the file tools, which the +`permissions.deny` rules cover. The image installs `bubblewrap` and `socat` +(`scripts/develop.sh`). If the devcontainer cannot create the sandbox +(for example no user namespaces), the Claude step fails and the issue gets a +`needs-human` note; moderation never runs unsandboxed. **Unverified until a +real run:** this repository has no key, so whether bubblewrap works inside +the devcontainer on a GitHub-hosted runner, and that the sandboxed commands +cannot see the claude process's `/proc//environ`, are checked only by +the procedure in the pull request that introduced this (an agent-run +command must fail to read that file and to reach an arbitrary host). A single `moderate-issue-` `concurrency:` group, shared across the two triage workflows, the two worker (fix/build) workflows, and @@ -341,6 +463,16 @@ the digest are those of the tested artifact: ## Release variables and secrets +The secrets below belong in the `release` environment (Settings → +Environments), not in repository secrets: `build`, `release` and +`publish-packages` declare `environment: release`, and the environment's +deployment branches are limited to the default branch, so a run from +another ref can't read them. The workflow reads them as `secrets.NAME` +either way, and an environment secret wins over a repository secret of the +same name. Variables stay repository-level. Configure the environment +before the first release that uses it: a missing one is created by GitHub +with no rules. + `release.yml` passes these to the Makefile targets as environment variables of the same name; unset ones are empty, and a target should skip cleanly when the registry isn't configured. @@ -383,6 +515,9 @@ cleanly when the registry isn't configured. `make finalize` can push to the repository's own GHCR packages with `GITHUB_TOKEN` (no other job has it; the token is in `build`'s job env, so every per-arch target can use it). Leave `checks.yml` at `contents: read`. +- Keep `environment: release` on exactly the `release.yml` jobs that hold + `contents: write`, `packages: write`, `id-token: write` or registry/npm + secrets (`build`, `release`, `publish-packages`), and none elsewhere. - Keep `id-token: write` on `release.yml`'s `publish-packages` job only (PyPI, npm and crates.io trusted publishing via GitHub OIDC). GitHub permissions are per job, not per step, and that job's `if:` on the diff --git a/.github/workflows/moderate-bug-fix.yml b/.github/workflows/moderate-bug-fix.yml index b80d3b1..6a40069 100644 --- a/.github/workflows/moderate-bug-fix.yml +++ b/.github/workflows/moderate-bug-fix.yml @@ -70,8 +70,10 @@ jobs: # branch doesn't exist; the `fix` job's own trust condition # still gates whether that's used. - id: resolve-sha + env: + ISSUE_NUMBER: ${{ github.event.issue.number }} run: | - sha=$(gh api "repos/${{ github.repository }}/commits/bug/${{ github.event.issue.number }}-repro" --jq .sha 2>/dev/null || true) + sha=$(gh api "repos/$GITHUB_REPOSITORY/commits/bug/${ISSUE_NUMBER}-repro" --jq .sha 2>/dev/null || true) echo "sha=$sha" >> "$GITHUB_OUTPUT" fix: @@ -82,47 +84,85 @@ jobs: # opt-in `claude:ok`. if: >- ${{ needs.gate.outputs.has-key == 'true' && + needs.gate.outputs.repro-sha != '' && contains(github.event.issue.labels.*.name, 'bug:repro-ready') && (contains(fromJSON('["OWNER","MEMBER","COLLABORATOR"]'), github.event.issue.author_association) || contains(github.event.issue.labels.*.name, 'claude:ok')) }} # renovate: datasource=github-runners depName=ubuntu runs-on: ${{ vars.CI_RUNNER || 'ubuntu-24.04' }} timeout-minutes: 20 + permissions: + contents: read env: GITHUB_ISSUE_NUMBER: ${{ github.event.issue.number }} steps: + # The whole history, so the branch can be compared with the default + # branch below. The tip is pinned to the SHA the gate job resolved. - name: Check out repro branch uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: persist-credentials: false + fetch-depth: 0 ref: ${{ needs.gate.outputs.repro-sha }} + # A snapshot outside the workspace (and so outside the devcontainer's + # bind mount) of the default branch: everything run on the runner or + # copied into the container comes from it, never from the repro + # branch's own tree. `issues` events run on the default branch's + # latest commit. + - name: Snapshot the trusted tooling + env: + TRUSTED_SHA: ${{ github.sha }} + run: | + mkdir "$RUNNER_TEMP/trusted" + git archive "$TRUSTED_SHA" | tar -x -C "$RUNNER_TEMP/trusted" + "$RUNNER_TEMP/trusted/.github/scripts/moderate_prepare.sh" "$RUNNER_TEMP/trusted" + + # The devcontainer and image build below read the branch's own + # `.devcontainer/`, `Dockerfile`, compose and `scripts/develop.sh`. + # The branch must not differ from its merge base with the default + # branch in any of them (or in any other path the apply step denies); + # otherwise no Claude step runs and the result is a `needs-human` note. + - name: Verify the branch against the path allow-list + id: verify + continue-on-error: true + env: + TRUSTED_SHA: ${{ github.sha }} + run: '"$RUNNER_TEMP/trusted/.github/scripts/moderate_verify_branch.sh" "$TRUSTED_SHA"' + # Unique per run so concurrent runs never collide on # container/volume/network names, and so the teardown step below tears # down exactly this run's stack. Mirrors checks.yml. - name: Set compose project name run: echo "COMPOSE_PROJECT_NAME=ci-moderate-${{ github.run_id }}-${{ github.run_attempt }}" >> "$GITHUB_ENV" - - name: Write Claude API key - run: | - install -m 600 /dev/null .secrets/claude.txt - printf '%s' "${{ secrets.ANTHROPIC_API_KEY }}" > .secrets/claude.txt - + # The API key reaches the container only through `with: env:` (-> + # `devcontainer exec --remote-env`), i.e. the environment of this one + # command. A step-level `env:` alone is NOT forwarded into the + # container. There is no GitHub token in the container at all: this + # job's prompt has no `gh` access. - name: Run Claude bug fix + if: steps.verify.outcome == 'success' uses: devcontainers/ci@513af61f4de4f75d37e4438f184ba4358f0fc1ca # v0.3.1900000450 + env: + # Build-time only (compose.yml's `github_token` build secret): not + # in the `with: env:` list below, so not forwarded into the container. + GITHUB_TOKEN: ${{ github.token }} + ANTHROPIC_API_KEY: ${{ secrets.ANTHROPIC_API_KEY }} + MODERATION_EXTRA_TOOLS: ${{ vars.MODERATION_EXTRA_TOOLS }} + # Hosts the sandbox lets Claude's commands reach (everything else is refused). + MODERATION_ALLOWED_DOMAINS: github.com${{ vars.MODERATION_EXTRA_DOMAINS && format(',{0}', vars.MODERATION_EXTRA_DOMAINS) || '' }} with: runCmd: >- - .github/scripts/run_claude.sh - .github/scripts/prompts/bug-fix.md - "Read,Grep,Glob,Write,Edit,Bash(git *),Bash(prek run *)${{ vars.MODERATION_EXTRA_TOOLS && format(',{0}', vars.MODERATION_EXTRA_TOOLS) || '' }}" + .moderation-tools/run_claude.sh + .moderation-tools/prompts/bug-fix.md + "Read,Grep,Glob,Write,Edit,Bash(git status *),Bash(git diff *),Bash(git log *),Bash(git show *),Bash(git add *),Bash(git commit *),Bash(git restore *),Bash(git rm *),Bash(git mv *),Bash(git ls-files *),Bash(git rev-parse *),Bash(prek run --config .moderation-tools/pre-commit-config.yaml *)" 40 900 - env: - GITHUB_TOKEN: ${{ github.token }} - GITHUB_ISSUE_NUMBER: ${{ github.event.issue.number }} - - - name: Remove Claude API key - if: always() - run: rm -f .secrets/claude.txt + env: | + ANTHROPIC_API_KEY + MODERATION_EXTRA_TOOLS + MODERATION_ALLOWED_DOMAINS + GITHUB_ISSUE_NUMBER # Always packages/uploads, even if Claude's step above failed or # timed out, so the apply job still gets a "failed" outcome and @@ -132,10 +172,11 @@ jobs: env: RESULT_SHA: ${{ needs.gate.outputs.repro-sha }} run: | - .github/scripts/moderate_package_result.sh \ + "$RUNNER_TEMP/trusted/.github/scripts/moderate_package_result.sh" \ "$RESULT_SHA" \ moderation-result \ - .moderation-outcome.md + "$GITHUB_WORKSPACE" \ + build - name: Upload result artifact if: always() @@ -171,11 +212,14 @@ jobs: timeout-minutes: 5 env: GH_TOKEN: ${{ github.token }} + ISSUE_NUMBER: ${{ github.event.issue.number }} + DEFAULT_BRANCH: ${{ github.event.repository.default_branch }} steps: - name: Check out repository uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: persist-credentials: false + ref: ${{ github.sha }} # The `fix` job uploads in an `if: always()` step, so a missing # artifact means it died before packaging (runner crash, upload diff --git a/.github/workflows/moderate-bug-triage.yml b/.github/workflows/moderate-bug-triage.yml index a14d440..4ea54ed 100644 --- a/.github/workflows/moderate-bug-triage.yml +++ b/.github/workflows/moderate-bug-triage.yml @@ -1,7 +1,11 @@ # Claude-driven first triage of a bug report -- see # ../workflows/README.md's "Issue moderation" section for the full state -# machine. -# Dormant (job shows Skipped, not Failed) unless the ANTHROPIC_API_KEY +# machine and the threat model (docs/adrs/0001-harden-the-issue-moderation-workflows.md). +# One workflow, three jobs: `gate` (is the feature configured), `triage` +# (Claude against the issue's untrusted text, with a READ-ONLY token) and +# `apply` (no Claude, no untrusted code: validates the result artifact, then +# pushes the repro branch, comments and labels). +# Dormant (jobs show Skipped, not Failed) unless the ANTHROPIC_API_KEY # repository secret is configured. name: Moderate bug triage @@ -9,7 +13,7 @@ on: issues: types: [opened, reopened] -# Least privilege: only the `triage` job gets write scopes. +# Least privilege: every job declares its own scopes below. permissions: {} concurrency: @@ -40,25 +44,33 @@ jobs: # renovate: datasource=github-runners depName=ubuntu runs-on: ${{ vars.CI_RUNNER || 'ubuntu-24.04' }} timeout-minutes: 10 + # Read-only: this job runs Claude against the issue's (untrusted) text, so + # whatever a prompt injection reaches must not be able to push, comment or + # label. `issues: read` is for `gh issue view`. permissions: - contents: write - issues: write - pull-requests: write + contents: read + issues: read env: - GH_TOKEN: ${{ github.token }} GITHUB_ISSUE_NUMBER: ${{ github.event.issue.number }} steps: - name: Check out repository uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: persist-credentials: false + fetch-depth: 0 + ref: ${{ github.sha }} - # Whatever state this issue was in before it was closed, reopening it - # restarts triage from scratch rather than resuming stale state -- see - # ../workflows/README.md. - - name: Reset stale triage state on reopen - if: github.event.action == 'reopened' - run: .github/scripts/reset_issue_state.sh bug "$GITHUB_ISSUE_NUMBER" + # A snapshot outside the workspace (and so outside the devcontainer's + # bind mount) of the default branch: everything run on the runner or + # copied in below comes from it, never from the tree Claude edits. + # `issues` events run on the default branch's latest commit. + - name: Snapshot the trusted tooling + env: + TRUSTED_SHA: ${{ github.sha }} + run: | + mkdir "$RUNNER_TEMP/trusted" + git archive "$TRUSTED_SHA" | tar -x -C "$RUNNER_TEMP/trusted" + "$RUNNER_TEMP/trusted/.github/scripts/moderate_prepare.sh" "$RUNNER_TEMP/trusted" # Unique per run so concurrent runs never collide on # container/volume/network names, and so the teardown step below tears @@ -66,28 +78,54 @@ jobs: - name: Set compose project name run: echo "COMPOSE_PROJECT_NAME=ci-moderate-${{ github.run_id }}-${{ github.run_attempt }}" >> "$GITHUB_ENV" - - name: Write Claude API key - run: | - install -m 600 /dev/null .secrets/claude.txt - printf '%s' "${{ secrets.ANTHROPIC_API_KEY }}" > .secrets/claude.txt - + # The API key reaches the container only through `with: env:` (-> `devcontainer + # exec --remote-env`), i.e. the environment of this one command. A + # step-level `env:` alone is NOT forwarded into the container. - name: Run Claude bug triage uses: devcontainers/ci@513af61f4de4f75d37e4438f184ba4358f0fc1ca # v0.3.1900000450 + env: + # Build-time only (compose.yml's `github_token` build secret): not + # in the `with: env:` list below, so not forwarded into the container. + GITHUB_TOKEN: ${{ github.token }} + ANTHROPIC_API_KEY: ${{ secrets.ANTHROPIC_API_KEY }} + GH_TOKEN: ${{ github.token }} + MODERATION_EXTRA_TOOLS: ${{ vars.MODERATION_EXTRA_TOOLS }} + # Hosts the sandbox lets Claude's commands reach (everything else is refused). + MODERATION_ALLOWED_DOMAINS: github.com,api.github.com${{ vars.MODERATION_EXTRA_DOMAINS && format(',{0}', vars.MODERATION_EXTRA_DOMAINS) || '' }} with: runCmd: >- - gh auth setup-git && - .github/scripts/run_claude.sh - .github/scripts/prompts/bug-triage.md - "Read,Grep,Glob,Write,Edit,Bash(git *),Bash(gh issue *),Bash(gh repo view:*),Bash(prek run *)${{ vars.MODERATION_EXTRA_TOOLS && format(',{0}', vars.MODERATION_EXTRA_TOOLS) || '' }}" + .moderation-tools/run_claude.sh + .moderation-tools/prompts/bug-triage.md + "Read,Grep,Glob,Write,Edit,Bash(git status *),Bash(git diff *),Bash(git log *),Bash(git show *),Bash(git add *),Bash(git commit *),Bash(git checkout -b *),Bash(git switch -c *),Bash(git restore *),Bash(git rm *),Bash(git mv *),Bash(git ls-files *),Bash(git rev-parse *),Bash(gh issue view *),Bash(gh repo view *),Bash(prek run --config .moderation-tools/pre-commit-config.yaml *)" 15 120 + env: | + ANTHROPIC_API_KEY + GH_TOKEN + MODERATION_EXTRA_TOOLS + MODERATION_ALLOWED_DOMAINS + GITHUB_ISSUE_NUMBER + + # Always packages/uploads, even if Claude's step above failed or + # timed out, so the apply job still gets a "failed" outcome and + # can comment on the issue instead of it going silently stuck. + - name: Package result for the apply job + if: always() env: - GITHUB_TOKEN: ${{ github.token }} - GH_TOKEN: ${{ github.token }} - GITHUB_ISSUE_NUMBER: ${{ github.event.issue.number }} + BASE_SHA: ${{ github.sha }} + run: | + "$RUNNER_TEMP/trusted/.github/scripts/moderate_package_result.sh" \ + "$BASE_SHA" \ + moderation-result \ + "$GITHUB_WORKSPACE" \ + triage - - name: Remove Claude API key + - name: Upload result artifact if: always() - run: rm -f .secrets/claude.txt + uses: actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9 # v7.0.2 + with: + name: moderation-result + path: moderation-result + retention-days: 1 - name: Tear down devcontainer stack if: always() @@ -95,3 +133,70 @@ jobs: docker compose \ -p "$COMPOSE_PROJECT_NAME" \ down --volumes --remove-orphans + + # Privileged half: consumes only the data artifact above (outcome metadata + # and a git patch, never executed) and does the push/comment/label. Runs on + # the default branch's workflow definition and checkout (an `issues` + # event), never runs Claude, and validates everything it reads. + # `always()` so a failed or timed-out Claude step still gets its + # "failed" outcome applied; skipped (gate closed, no API key, issue not + # trusted) means there is nothing to apply. + apply: + needs: triage + if: ${{ !cancelled() && needs.triage.result != 'skipped' }} + permissions: + contents: write + issues: write + pull-requests: write + # renovate: datasource=github-runners depName=ubuntu + runs-on: ${{ vars.CI_RUNNER || 'ubuntu-24.04' }} + timeout-minutes: 5 + env: + GH_TOKEN: ${{ github.token }} + ISSUE_NUMBER: ${{ github.event.issue.number }} + DEFAULT_BRANCH: ${{ github.event.repository.default_branch }} + steps: + - name: Check out repository + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + ref: ${{ github.sha }} + + # The `triage` job uploads in an `if: always()` step, so a missing + # artifact means it died before packaging (runner crash, upload + # error) -- handled by the "Report" step below rather than failing + # silently, hence continue-on-error here. + - name: Download moderation result + id: download + continue-on-error: true + uses: actions/download-artifact@9000827ccba6bdab643e8b6fd33ac0654aef8333 # v8.0.2 + with: + name: moderation-result + path: moderation-result + + # Checkout doesn't persist credentials; git push/fetch use gh's + # credential helper, fed by GH_TOKEN. + - name: Configure git credentials + run: gh auth setup-git + + # Whatever state this issue was in before it was closed, reopening it + # restarts triage from scratch rather than resuming stale state -- see + # ../workflows/README.md. Runs here, not in the Claude job, because it + # edits labels and deletes branches. + - name: Reset stale triage state on reopen + if: github.event.action == 'reopened' + run: .github/scripts/reset_issue_state.sh bug "$ISSUE_NUMBER" + + - name: Report a missing moderation result + if: steps.download.outcome == 'failure' + env: + RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} + DOWNLOAD_OUTCOME: ${{ steps.download.outcome }} + run: | + gh issue edit "$ISSUE_NUMBER" --add-label needs-human || true + gh issue comment "$ISSUE_NUMBER" --body "No result artifact (missing or download error; download step: $DOWNLOAD_OUTCOME). The run likely failed before packaging one. See $RUN_URL. Reopen the issue to restart triage." || true + exit 1 + + - name: Apply result + if: steps.download.outcome == 'success' + run: .github/scripts/moderate_apply_triage.sh bug moderation-result diff --git a/.github/workflows/moderate-feature-build.yml b/.github/workflows/moderate-feature-build.yml index 8fb1e45..aa27c09 100644 --- a/.github/workflows/moderate-feature-build.yml +++ b/.github/workflows/moderate-feature-build.yml @@ -70,8 +70,10 @@ jobs: # branch doesn't exist; the `build` job's own trust condition # still gates whether that's used. - id: resolve-sha + env: + ISSUE_NUMBER: ${{ github.event.issue.number }} run: | - sha=$(gh api "repos/${{ github.repository }}/commits/feature/${{ github.event.issue.number }}-plan" --jq .sha 2>/dev/null || true) + sha=$(gh api "repos/$GITHUB_REPOSITORY/commits/feature/${ISSUE_NUMBER}-plan" --jq .sha 2>/dev/null || true) echo "sha=$sha" >> "$GITHUB_OUTPUT" build: @@ -82,47 +84,85 @@ jobs: # opt-in `claude:ok`. if: >- ${{ needs.gate.outputs.has-key == 'true' && + needs.gate.outputs.plan-sha != '' && contains(github.event.issue.labels.*.name, 'enhancement:plan-ready') && (contains(fromJSON('["OWNER","MEMBER","COLLABORATOR"]'), github.event.issue.author_association) || contains(github.event.issue.labels.*.name, 'claude:ok')) }} # renovate: datasource=github-runners depName=ubuntu runs-on: ${{ vars.CI_RUNNER || 'ubuntu-24.04' }} timeout-minutes: 20 + permissions: + contents: read env: GITHUB_ISSUE_NUMBER: ${{ github.event.issue.number }} steps: + # The whole history, so the branch can be compared with the default + # branch below. The tip is pinned to the SHA the gate job resolved. - name: Check out plan branch uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: persist-credentials: false + fetch-depth: 0 ref: ${{ needs.gate.outputs.plan-sha }} + # A snapshot outside the workspace (and so outside the devcontainer's + # bind mount) of the default branch: everything run on the runner or + # copied into the container comes from it, never from the plan + # branch's own tree. `issues` events run on the default branch's + # latest commit. + - name: Snapshot the trusted tooling + env: + TRUSTED_SHA: ${{ github.sha }} + run: | + mkdir "$RUNNER_TEMP/trusted" + git archive "$TRUSTED_SHA" | tar -x -C "$RUNNER_TEMP/trusted" + "$RUNNER_TEMP/trusted/.github/scripts/moderate_prepare.sh" "$RUNNER_TEMP/trusted" + + # The devcontainer and image build below read the branch's own + # `.devcontainer/`, `Dockerfile`, compose and `scripts/develop.sh`. + # The branch must not differ from its merge base with the default + # branch in any of them (or in any other path the apply step denies); + # otherwise no Claude step runs and the result is a `needs-human` note. + - name: Verify the branch against the path allow-list + id: verify + continue-on-error: true + env: + TRUSTED_SHA: ${{ github.sha }} + run: '"$RUNNER_TEMP/trusted/.github/scripts/moderate_verify_branch.sh" "$TRUSTED_SHA"' + # Unique per run so concurrent runs never collide on # container/volume/network names, and so the teardown step below tears # down exactly this run's stack. Mirrors checks.yml. - name: Set compose project name run: echo "COMPOSE_PROJECT_NAME=ci-moderate-${{ github.run_id }}-${{ github.run_attempt }}" >> "$GITHUB_ENV" - - name: Write Claude API key - run: | - install -m 600 /dev/null .secrets/claude.txt - printf '%s' "${{ secrets.ANTHROPIC_API_KEY }}" > .secrets/claude.txt - + # The API key reaches the container only through `with: env:` (-> + # `devcontainer exec --remote-env`), i.e. the environment of this one + # command. A step-level `env:` alone is NOT forwarded into the + # container. There is no GitHub token in the container at all: this + # job's prompt has no `gh` access. - name: Run Claude feature build + if: steps.verify.outcome == 'success' uses: devcontainers/ci@513af61f4de4f75d37e4438f184ba4358f0fc1ca # v0.3.1900000450 + env: + # Build-time only (compose.yml's `github_token` build secret): not + # in the `with: env:` list below, so not forwarded into the container. + GITHUB_TOKEN: ${{ github.token }} + ANTHROPIC_API_KEY: ${{ secrets.ANTHROPIC_API_KEY }} + MODERATION_EXTRA_TOOLS: ${{ vars.MODERATION_EXTRA_TOOLS }} + # Hosts the sandbox lets Claude's commands reach (everything else is refused). + MODERATION_ALLOWED_DOMAINS: github.com${{ vars.MODERATION_EXTRA_DOMAINS && format(',{0}', vars.MODERATION_EXTRA_DOMAINS) || '' }} with: runCmd: >- - .github/scripts/run_claude.sh - .github/scripts/prompts/feature-build.md - "Read,Grep,Glob,Write,Edit,Bash(git *),Bash(prek run *)${{ vars.MODERATION_EXTRA_TOOLS && format(',{0}', vars.MODERATION_EXTRA_TOOLS) || '' }}" + .moderation-tools/run_claude.sh + .moderation-tools/prompts/feature-build.md + "Read,Grep,Glob,Write,Edit,Bash(git status *),Bash(git diff *),Bash(git log *),Bash(git show *),Bash(git add *),Bash(git commit *),Bash(git restore *),Bash(git rm *),Bash(git mv *),Bash(git ls-files *),Bash(git rev-parse *),Bash(prek run --config .moderation-tools/pre-commit-config.yaml *)" 40 900 - env: - GITHUB_TOKEN: ${{ github.token }} - GITHUB_ISSUE_NUMBER: ${{ github.event.issue.number }} - - - name: Remove Claude API key - if: always() - run: rm -f .secrets/claude.txt + env: | + ANTHROPIC_API_KEY + MODERATION_EXTRA_TOOLS + MODERATION_ALLOWED_DOMAINS + GITHUB_ISSUE_NUMBER # Always packages/uploads, even if Claude's step above failed or # timed out, so the apply job still gets a "failed" outcome and @@ -132,10 +172,11 @@ jobs: env: RESULT_SHA: ${{ needs.gate.outputs.plan-sha }} run: | - .github/scripts/moderate_package_result.sh \ + "$RUNNER_TEMP/trusted/.github/scripts/moderate_package_result.sh" \ "$RESULT_SHA" \ moderation-result \ - .moderation-outcome.md + "$GITHUB_WORKSPACE" \ + build - name: Upload result artifact if: always() @@ -171,11 +212,14 @@ jobs: timeout-minutes: 5 env: GH_TOKEN: ${{ github.token }} + ISSUE_NUMBER: ${{ github.event.issue.number }} + DEFAULT_BRANCH: ${{ github.event.repository.default_branch }} steps: - name: Check out repository uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: persist-credentials: false + ref: ${{ github.sha }} # The `build` job uploads in an `if: always()` step, so a missing # artifact means it died before packaging (runner crash, upload diff --git a/.github/workflows/moderate-feature-triage.yml b/.github/workflows/moderate-feature-triage.yml index 1c3ae58..ab5dad0 100644 --- a/.github/workflows/moderate-feature-triage.yml +++ b/.github/workflows/moderate-feature-triage.yml @@ -1,7 +1,11 @@ # Claude-driven first triage of a feature request -- see # ../workflows/README.md's "Issue moderation" section for the full state -# machine. -# Dormant (job shows Skipped, not Failed) unless the ANTHROPIC_API_KEY +# machine and the threat model (docs/adrs/0001-harden-the-issue-moderation-workflows.md). +# One workflow, three jobs: `gate` (is the feature configured), `triage` +# (Claude against the issue's untrusted text, with a READ-ONLY token) and +# `apply` (no Claude, no untrusted code: validates the result artifact, then +# pushes the plan branch, comments and labels). +# Dormant (jobs show Skipped, not Failed) unless the ANTHROPIC_API_KEY # repository secret is configured. name: Moderate feature triage @@ -9,7 +13,7 @@ on: issues: types: [opened, reopened] -# Least privilege: only the `triage` job gets write scopes. +# Least privilege: every job declares its own scopes below. permissions: {} concurrency: @@ -40,25 +44,33 @@ jobs: # renovate: datasource=github-runners depName=ubuntu runs-on: ${{ vars.CI_RUNNER || 'ubuntu-24.04' }} timeout-minutes: 10 + # Read-only: this job runs Claude against the issue's (untrusted) text, so + # whatever a prompt injection reaches must not be able to push, comment or + # label. `issues: read` is for `gh issue view`. permissions: - contents: write - issues: write - pull-requests: write + contents: read + issues: read env: - GH_TOKEN: ${{ github.token }} GITHUB_ISSUE_NUMBER: ${{ github.event.issue.number }} steps: - name: Check out repository uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: persist-credentials: false + fetch-depth: 0 + ref: ${{ github.sha }} - # Whatever state this issue was in before it was closed, reopening it - # restarts triage from scratch rather than resuming stale state -- see - # ../workflows/README.md. - - name: Reset stale triage state on reopen - if: github.event.action == 'reopened' - run: .github/scripts/reset_issue_state.sh enhancement "$GITHUB_ISSUE_NUMBER" + # A snapshot outside the workspace (and so outside the devcontainer's + # bind mount) of the default branch: everything run on the runner or + # copied in below comes from it, never from the tree Claude edits. + # `issues` events run on the default branch's latest commit. + - name: Snapshot the trusted tooling + env: + TRUSTED_SHA: ${{ github.sha }} + run: | + mkdir "$RUNNER_TEMP/trusted" + git archive "$TRUSTED_SHA" | tar -x -C "$RUNNER_TEMP/trusted" + "$RUNNER_TEMP/trusted/.github/scripts/moderate_prepare.sh" "$RUNNER_TEMP/trusted" # Unique per run so concurrent runs never collide on # container/volume/network names, and so the teardown step below tears @@ -66,28 +78,54 @@ jobs: - name: Set compose project name run: echo "COMPOSE_PROJECT_NAME=ci-moderate-${{ github.run_id }}-${{ github.run_attempt }}" >> "$GITHUB_ENV" - - name: Write Claude API key - run: | - install -m 600 /dev/null .secrets/claude.txt - printf '%s' "${{ secrets.ANTHROPIC_API_KEY }}" > .secrets/claude.txt - + # The API key reaches the container only through `with: env:` (-> `devcontainer + # exec --remote-env`), i.e. the environment of this one command. A + # step-level `env:` alone is NOT forwarded into the container. - name: Run Claude feature triage uses: devcontainers/ci@513af61f4de4f75d37e4438f184ba4358f0fc1ca # v0.3.1900000450 + env: + # Build-time only (compose.yml's `github_token` build secret): not + # in the `with: env:` list below, so not forwarded into the container. + GITHUB_TOKEN: ${{ github.token }} + ANTHROPIC_API_KEY: ${{ secrets.ANTHROPIC_API_KEY }} + GH_TOKEN: ${{ github.token }} + MODERATION_EXTRA_TOOLS: ${{ vars.MODERATION_EXTRA_TOOLS }} + # Hosts the sandbox lets Claude's commands reach (everything else is refused). + MODERATION_ALLOWED_DOMAINS: github.com,api.github.com${{ vars.MODERATION_EXTRA_DOMAINS && format(',{0}', vars.MODERATION_EXTRA_DOMAINS) || '' }} with: runCmd: >- - gh auth setup-git && - .github/scripts/run_claude.sh - .github/scripts/prompts/feature-triage.md - "Read,Grep,Glob,Write,Edit,Bash(git *),Bash(gh issue *),Bash(gh repo view:*)${{ vars.MODERATION_EXTRA_TOOLS && format(',{0}', vars.MODERATION_EXTRA_TOOLS) || '' }}" + .moderation-tools/run_claude.sh + .moderation-tools/prompts/feature-triage.md + "Read,Grep,Glob,Write,Edit,Bash(git status *),Bash(git diff *),Bash(git log *),Bash(git show *),Bash(git add *),Bash(git commit *),Bash(git checkout -b *),Bash(git switch -c *),Bash(git restore *),Bash(git rm *),Bash(git mv *),Bash(git ls-files *),Bash(git rev-parse *),Bash(gh issue view *),Bash(gh repo view *)" 15 120 + env: | + ANTHROPIC_API_KEY + GH_TOKEN + MODERATION_EXTRA_TOOLS + MODERATION_ALLOWED_DOMAINS + GITHUB_ISSUE_NUMBER + + # Always packages/uploads, even if Claude's step above failed or + # timed out, so the apply job still gets a "failed" outcome and + # can comment on the issue instead of it going silently stuck. + - name: Package result for the apply job + if: always() env: - GITHUB_TOKEN: ${{ github.token }} - GH_TOKEN: ${{ github.token }} - GITHUB_ISSUE_NUMBER: ${{ github.event.issue.number }} + BASE_SHA: ${{ github.sha }} + run: | + "$RUNNER_TEMP/trusted/.github/scripts/moderate_package_result.sh" \ + "$BASE_SHA" \ + moderation-result \ + "$GITHUB_WORKSPACE" \ + triage - - name: Remove Claude API key + - name: Upload result artifact if: always() - run: rm -f .secrets/claude.txt + uses: actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9 # v7.0.2 + with: + name: moderation-result + path: moderation-result + retention-days: 1 - name: Tear down devcontainer stack if: always() @@ -95,3 +133,70 @@ jobs: docker compose \ -p "$COMPOSE_PROJECT_NAME" \ down --volumes --remove-orphans + + # Privileged half: consumes only the data artifact above (outcome metadata + # and a git patch, never executed) and does the push/comment/label. Runs on + # the default branch's workflow definition and checkout (an `issues` + # event), never runs Claude, and validates everything it reads. + # `always()` so a failed or timed-out Claude step still gets its + # "failed" outcome applied; skipped (gate closed, no API key, issue not + # trusted) means there is nothing to apply. + apply: + needs: triage + if: ${{ !cancelled() && needs.triage.result != 'skipped' }} + permissions: + contents: write + issues: write + pull-requests: write + # renovate: datasource=github-runners depName=ubuntu + runs-on: ${{ vars.CI_RUNNER || 'ubuntu-24.04' }} + timeout-minutes: 5 + env: + GH_TOKEN: ${{ github.token }} + ISSUE_NUMBER: ${{ github.event.issue.number }} + DEFAULT_BRANCH: ${{ github.event.repository.default_branch }} + steps: + - name: Check out repository + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + ref: ${{ github.sha }} + + # The `triage` job uploads in an `if: always()` step, so a missing + # artifact means it died before packaging (runner crash, upload + # error) -- handled by the "Report" step below rather than failing + # silently, hence continue-on-error here. + - name: Download moderation result + id: download + continue-on-error: true + uses: actions/download-artifact@9000827ccba6bdab643e8b6fd33ac0654aef8333 # v8.0.2 + with: + name: moderation-result + path: moderation-result + + # Checkout doesn't persist credentials; git push/fetch use gh's + # credential helper, fed by GH_TOKEN. + - name: Configure git credentials + run: gh auth setup-git + + # Whatever state this issue was in before it was closed, reopening it + # restarts triage from scratch rather than resuming stale state -- see + # ../workflows/README.md. Runs here, not in the Claude job, because it + # edits labels and deletes branches. + - name: Reset stale triage state on reopen + if: github.event.action == 'reopened' + run: .github/scripts/reset_issue_state.sh enhancement "$ISSUE_NUMBER" + + - name: Report a missing moderation result + if: steps.download.outcome == 'failure' + env: + RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} + DOWNLOAD_OUTCOME: ${{ steps.download.outcome }} + run: | + gh issue edit "$ISSUE_NUMBER" --add-label needs-human || true + gh issue comment "$ISSUE_NUMBER" --body "No result artifact (missing or download error; download step: $DOWNLOAD_OUTCOME). The run likely failed before packaging one. See $RUN_URL. Reopen the issue to restart triage." || true + exit 1 + + - name: Apply result + if: steps.download.outcome == 'success' + run: .github/scripts/moderate_apply_triage.sh feature moderation-result diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 1691b62..5905159 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -15,6 +15,24 @@ # existing release). A GitHub release can therefore exist for a version whose # `publish-packages` failed: re-run the failed jobs to finish publishing, and # do not delete the release. +# +# Default branch only: any user with write access can dispatch this workflow +# on any branch, and `build` runs that branch's Makefile with `contents: +# write`, `packages: write` and the registry secrets. So the `version` job +# fails unless the dispatched ref is the default branch, every job that +# holds write permissions or secrets (`build`, `release`, `publish-packages`) +# is also gated by the same condition itself, and the release is tagged at +# `github.sha`, which for a dispatch on the default branch is its tip. +# These guards stop an accidental dispatch from a feature branch, but they +# are not a security boundary: GitHub runs this file from the dispatched +# ref, so a writer can delete them on their own branch. The server-side +# rule that holds against that is the `release` environment: `build`, +# `release` and `publish-packages` declare `environment: release`, whose +# deployment branches are limited to the default branch, and the registry +# and npm secrets live in that environment (not at repository level), so +# a run from another ref is refused the job and cannot read them. The +# checks here stay as the fail-fast. Setup: docs/TEMPLATE.md, "Release +# environment". name: Release on: @@ -56,6 +74,17 @@ jobs: prerelease: ${{ steps.version.outputs.prerelease }} arches: ${{ steps.arches.outputs.arches }} steps: + # First step, before anything is checked out or run from the ref. + - name: Refuse to release from a non-default branch + env: + REF: ${{ github.ref }} + DEFAULT_REF: refs/heads/${{ github.event.repository.default_branch }} + run: | + if [ "$REF" != "$DEFAULT_REF" ]; then + echo "::error::Release runs only from the default branch ($DEFAULT_REF); this run is on $REF." + exit 1 + fi + - name: Check out repository uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: @@ -79,6 +108,7 @@ jobs: build: needs: version + if: ${{ github.ref == format('refs/heads/{0}', github.event.repository.default_branch) }} strategy: fail-fast: false matrix: @@ -89,13 +119,12 @@ jobs: # is the separate `publish-packages` job below. `packages: write` lets # `make publish` push to this repository's own GHCR packages # (ghcr.io//...) with `GITHUB_TOKEN` (actor - # `GITHUB_ACTOR`, a default Actions env var) when the OCI_REGISTRY_* - # secrets are unset and OCI_REGISTRY is ghcr.io (the Makefile's - # REGISTRY_CREDS; any other registry needs the secrets). The + # `GITHUB_ACTOR`) when the OCI_REGISTRY_* secrets are unset. The # `release` job has it too, for `make finalize`'s manifest lists. permissions: contents: write packages: write + environment: release # renovate: datasource=github-runners depName=ubuntu runs-on: ${{ matrix.arch == 'arm64' && (vars.CI_RUNNER_ARM64 || 'ubuntu-24.04-arm') || (vars.CI_RUNNER_AMD64 || 'ubuntu-24.04') }} env: @@ -255,6 +284,7 @@ jobs: release: needs: [version, release-check] + if: ${{ github.ref == format('refs/heads/{0}', github.event.repository.default_branch) }} # Two runs for the same tag (a double dispatch, or a re-run beside a # newer run) must not race on the draft deletion, finalize and release # creation below: the second waits and then sees the published release @@ -270,6 +300,7 @@ jobs: permissions: contents: write packages: write + environment: release # renovate: datasource=github-runners depName=ubuntu runs-on: ${{ vars.CI_RUNNER || 'ubuntu-24.04' }} steps: @@ -372,16 +403,17 @@ jobs: # secrets live in this job alone, and a job-level `if` on the # `*_PUBLISH` repository variables means an image-only instance never # starts it: it never requests an OIDC token and never has a package - # secret in an environment. (`vars` is allowed in `jobs..if`; + # secret in its process environment. (`vars` is allowed in `jobs..if`; # `secrets` is not, which is why the variables are the opt-in switch.) # The assembled `dist/` is downloaded first, so `make publish-packages` # uploads exactly what `make build` produced. publish-packages: needs: [version, release] - if: ${{ vars.PYPI_PUBLISH != '' || vars.NPM_PUBLISH != '' || vars.CARGO_PUBLISH != '' }} + if: ${{ github.ref == format('refs/heads/{0}', github.event.repository.default_branch) && (vars.PYPI_PUBLISH != '' || vars.NPM_PUBLISH != '' || vars.CARGO_PUBLISH != '') }} permissions: contents: read id-token: write + environment: release # renovate: datasource=github-runners depName=ubuntu runs-on: ${{ vars.CI_RUNNER || 'ubuntu-24.04' }} env: diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 0f7ba68..8c4a05f 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -47,7 +47,7 @@ repos: stages: [pre-commit, manual] - repo: https://github.com/compilerla/conventional-pre-commit - rev: v4.4.0 + rev: v4.5.0 hooks: # No "manual" stage here, unlike every other hook in this file: this # one needs the actual commit-message file git passes at the @@ -189,7 +189,7 @@ repos: # RUST_VERSION) to this entry. - id: check-dockerfile-versions name: check-dockerfile-versions (ARG defaults match across Dockerfiles) - entry: bash scripts/check-dockerfile-versions.sh DEBIAN_VERSION NODE_VERSION + entry: bash scripts/check-dockerfile-versions.sh SSL_CERT_FILE SSL_CERT_DIR NODE_VERSION language: system # Also when the check itself, or this entry (the ARG list), changes. files: ^((\.devcontainer/)?Dockerfile|scripts/check-dockerfile-versions\.sh|\.pre-commit-config\.yaml)$ @@ -230,6 +230,6 @@ repos: name: image-scripts-test (Makefile and scripts/ behaviour) entry: bash scripts/test-image-scripts.sh language: system - files: ^(Makefile|scripts/(check-dockerfile-versions|image-scan|image-dast|image-smoke|skopeo|test-image-scripts)\.sh)$ + files: ^(Makefile|scripts/(check-dockerfile-versions|image-scan|image-dast|image-smoke|skopeo|skopeo-dir|test-image-scripts)\.sh)$ pass_filenames: false stages: [pre-commit, manual] diff --git a/Dockerfile b/Dockerfile index 949ca64..496d4f3 100644 --- a/Dockerfile +++ b/Dockerfile @@ -6,7 +6,7 @@ # the bundle is built with the same Node the devcontainer tests against -- # enforced by scripts/check-dockerfile-versions.sh. # renovate: datasource=node-version depName=node -ARG NODE_VERSION=24.22.1 +ARG NODE_VERSION=24.22.2 ARG DEBIAN_VERSION=trixie # renovate: datasource=docker depName=nginxinc/nginx-unprivileged diff --git a/Makefile b/Makefile index 261860e..ab1a93a 100644 --- a/Makefile +++ b/Makefile @@ -16,7 +16,7 @@ -include image.mk -.PHONY: repo-check release-arches build sbom release-assets publish publish-packages verify-published release-check finalize image-smoke check-arch +.PHONY: repo-check release-arches build sbom release-assets publish publish-packages verify-published release-check finalize image-smoke image-scan image-dast check-arch .DEFAULT_GOAL := release-arches DOCKERFILE ?= Dockerfile @@ -31,9 +31,14 @@ IMAGE_SMOKE_PORT ?= 8000 IMAGE_SMOKE_PATH ?= /health/live IMAGE_SMOKE_RUN_ARGS ?= IMAGE_SMOKE_TIMEOUT ?= 60 +# image-scan: empty = blocking locally, advisory when CI=true; 1 blocks, 0 warns. +IMAGE_SCAN_STRICT ?= -# renovate: datasource=docker depName=anchore/syft -SYFT_VERSION := v1.52.0 +# Pinned by tag and digest like SKOPEO_IMAGE; Renovate updates both together +# (a customManager in .github/renovate.json). The SBOM tool runs as root with +# dist/ (the image tar) mounted read-write, so a retagged image must not run. +# Apache-2.0. +SYFT_IMAGE := anchore/syft:v1.52.0@sha256:500e2d872ac019436926e8322b4fc1f39441d94d21f6f4046c6ff29b30e8cb02 # Pinned by tag and digest; Renovate updates both together. Apache-2.0. The # `-immutable` tag, not `vX.Y.Z`: quay.io rebuilds the plain tags daily and @@ -71,7 +76,7 @@ OCI_NAME = $(shell printf '%s' '$(or $(OCI_IMAGE_NAME),$(REPO))' | tr '[:upper:] export OCI_DESCRIPTION export IMAGE_SMOKE_RUN_ARGS IMAGE_SMOKE_PORT IMAGE_SMOKE_PATH IMAGE_SMOKE_TIMEOUT RELEASE_VERSION export IMAGE_TAR_PREFIX = $(ASSET_PREFIX) -export SKOPEO_IMAGE ARCHES +export SKOPEO_IMAGE ARCHES IMAGE_SCAN_STRICT # Only set in CI, by crazy-max/ghaction-github-runtime in release.yml. ifdef ACTIONS_RUNTIME_TOKEN @@ -117,11 +122,12 @@ build: check-arch repo-check $(BUILD) --output type=oci,dest=$(ASSET).tar . sbom: check-arch repo-check - docker run --rm -v "$(CURDIR)/dist:/dist" anchore/syft:$(SYFT_VERSION) \ + docker run --rm -v "$(CURDIR)/dist:/dist" $(SYFT_IMAGE) \ oci-archive:/$(ASSET).tar -o spdx-json=/$(ASSET).spdx.json -# Each arch is smoke-tested here, natively on its own runner, before publish. -release-assets: check-arch repo-check image-smoke +# Each arch is smoke-tested, Trivy-scanned and ZAP-scanned here, natively on +# its own runner, before publish. +release-assets: check-arch repo-check image-smoke image-scan image-dast test -f $(ASSET).tar && test -f $(ASSET).spdx.json # Registry credentials for publish and finalize, as shell that sets $user and @@ -155,7 +161,7 @@ else @$(REPO_GUARD) @test -f $(ASSET).tar || { echo "$(ASSET).tar not found; run 'make build' first." >&2; exit 1; } @$(REGISTRY_CREDS); \ - auth="$$(mktemp -d)" && chmod 0777 "$$auth" && trap 'rm -rf "$$auth"' EXIT && \ + auth="$$(bash scripts/skopeo-dir.sh)" && trap 'rm -rf "$$auth"' EXIT && \ printf '%s' "$$pass" | bash scripts/skopeo.sh -i -v "$$auth:/auth" -- \ login --authfile /auth/auth.json -u "$$user" --password-stdin "$(OCI_REGISTRY)" && \ bash scripts/skopeo.sh -v "$$auth:/auth:ro" -- \ @@ -192,6 +198,15 @@ verify-published: image-smoke: repo-check bash scripts/image-smoke.sh +# Run by release-assets (so before publish) on the arch's tar in dist/. Trivy +# is advisory (warning + job summary) unless IMAGE_SCAN_STRICT=1; ZAP follows +# .zap/rules.tsv. Both need Docker and pull their pinned images by digest. +image-scan: + bash scripts/image-scan.sh + +image-dast: + bash scripts/image-dast.sh + # No recipe by design: image.mk adds steps as prerequisites, so Make never # sees two recipes for one target. Runs the host-arch smoke test again # inside the devcontainer, with the instance's backing services up. diff --git a/docs/TEMPLATE.md b/docs/TEMPLATE.md index 764d6a2..75d2928 100644 --- a/docs/TEMPLATE.md +++ b/docs/TEMPLATE.md @@ -172,6 +172,117 @@ already establishes for `ruff`/`mypy`/`pytest`/`pip-audit`. triggers with a label trigger and in-workflow jobs). Don't suppress an audit to get green. +### Supply chain and release hardening + +Findings from the OWASP review (A03/A08), and what each now does: + +- **Release runs only from the default branch.** `release.yml`'s first + step fails unless `github.ref` is `refs/heads/`, and + `build`, `release` and `publish-packages` (the jobs holding `contents: + write`, `packages: write`, the OIDC token or registry secrets) repeat + the check as a job-level `if:`. This stops an accidental dispatch from another branch, but + it is not a security boundary: GitHub runs the workflow file from the + dispatched ref, so a writer can delete the checks on their own branch. + The server-side rule is the `release` environment (next bullet). + The tag and release target `github.sha`, which for a dispatch on the + default branch is its tip. Instances get this through template sync. + GitHub's [events documentation](https://docs.github.com/en/actions/reference/workflows-and-actions/events-that-trigger-workflows) + defines `GITHUB_REF` for a dispatch as the "branch or tag that received + dispatch", so the check has to be explicit; the + [secure-use guide](https://docs.github.com/en/actions/reference/security/secure-use) + asks for the least privilege `GITHUB_TOKEN` and warns that anyone with + write access can read repository secrets. +- **Release environment (setup step for each instance).** `release.yml`'s + `build`, `release` and `publish-packages` jobs declare `environment: + release`. An admin must configure it once per repository, **before the + first release that carries this workflow**: GitHub creates a + referenced-but-missing environment with no rules and no secrets, so an + unconfigured one protects nothing. Settings → Environments → `release`: + Deployment branches and tags → *Selected branches and tags* → add the + default branch (e.g. `main`) as a branch rule; or with the API, + `PUT repos///environments/release` with + `deployment_branch_policy: {protected_branches: false, + custom_branch_policies: true}`, then `POST + .../environments/release/deployment-branch-policies` with `name=` and `type=branch`. Then move the release secrets + (`OCI_REGISTRY_USERNAME`, `OCI_REGISTRY_PASSWORD`, `NPM_TOKEN`, + `CARGO_REGISTRY_TOKEN`) from repository secrets into the environment + (`gh secret set NAME --env release`, then `gh secret delete NAME`). An + *organization* secret of the same name stays readable by any workflow + in a repository it is shared with: delete it and recreate it as an + environment secret instead. A run + from any other ref is then refused the privileged jobs and cannot read + them, even if the in-workflow default-branch check was deleted on that + branch. The `jobs..environment` secrets are read as `secrets.NAME` + exactly like repository secrets, so no step changes (an environment + secret overrides a repository one of the same name); repository + *variables* (`PYPI_PUBLISH`, `OCI_REGISTRY`, ...) stay as they are. + `release-check`, `verify-published` and `version` hold no write + permission or secret and do not use the environment. Syncing the + workflow into an instance does not create or configure the environment; + until the steps above are done, nothing fails and nothing is protected. + The `build` job is a matrix, so each arch leg, `release` and + `publish-packages` is its own deployment: use the branch rule only. + Required reviewers or a wait timer would ask for approval once per job + and leg. If the repository + is private it needs a plan that supports environments. Trusted + publishers (npm, PyPI, crates.io) have an optional *environment* field, + and it must be set to `release`: that field is what binds publishing to + the environment, and without it a branch workflow with `id-token: write` + can still publish by OIDC. Set it once `release.yml` with the + environment is on the default branch. The registries only report a bad + config when you publish. Residual: a writer's branch workflow can + request `packages: write` itself, so the environment does not gate GHCR + pushes; mitigate with the package's GHCR Actions-access setting, pinning + consumed images and features by digest, and an org ruleset requiring + review for `.github/workflows/**`. See the [GitHub documentation](https://docs.github.com/en/actions/how-tos/deploy/configure-and-manage-deployments/manage-environments) + and [npm's](https://docs.npmjs.com/trusted-publishers). +- **Renovate waits 3 days before automerging.** The "patch and minor + updates" group sets `minimumReleaseAge: "3 days"` (the age Renovate's + own `security:minimumReleaseAgeNpm` preset uses) with + a top-level `internalChecksFilter: "strict"` (the + [documented recommendation](https://docs.renovatebot.com/key-concepts/minimum-release-age/) + alongside it), so a version still inside the window is skipped, not + merged. Which updates automerge is unchanged, except one: Renovate 42 + holds an update without a release timestamp forever + (`minimumReleaseAgeBehaviour` defaults to `timestamp-required`), and the + `python-version` and `github-runners` datasources have none, so Python + patches and runner image bumps are raised as their own PRs, without the + age check, and are not automerged. + Digest-only updates (including the Dockerfile's base image) were never + in the automerged group. +- **Installers are verified, not run blind.** `scripts/develop.sh` no + longer runs vendor install scripts. `uv` and `prek` are installed from + their release tarballs, checked against the `.sha256` file published + beside each asset; Claude Code's binary is checked against its + `manifest.json`, whose GPG signature is verified against the pinned + release-signing-key fingerprint (the vendor's + [documented verification](https://code.claude.com/docs/en/setup)). The + versions stay in the Dockerfile ARGs, so Renovate needs no new + manager. A checksum fetched from the same release as the file it + covers guards against a corrupted or swapped download, and against a + mirror or CDN serving something else; it does not help if the vendor's + release itself is compromised (the signed Claude Code manifest does + cover that). That is the same assurance node, `snip` and `gh` already + had. Bumping the signing key is a manual edit of `claude_signing_fpr` + when Anthropic rotates it. +- **The devcontainer base image and Dockerfile frontend are pinned by + digest.** `.devcontainer/Dockerfile` has `FROM + mcr.microsoft.com/devcontainers/base:trixie@sha256:...` and a pinned + `# syntax=` line, both written literally (the former `DEBIAN_VERSION` + ARG is gone), so Renovate's `dockerfile` manager can keep tag and digest + together. To move to another Debian release, change the tag; Renovate + re-pins the digest. +- **Accepted (Low): the MCP server's transitive dependencies.** + `.mcp.json` pins `@waldzellai/clear-thought-onepointfive` to an exact + version, but `npx` has no lockfile support, the package ships no + `npm-shrinkwrap.json`, and its own dependencies are ranges (`zod`, + `marked`, `@modelcontextprotocol/sdk`, ...). Those resolve at each + session start in the credentialed devcontainer. Mitigations are the + exact top-level pin, Renovate bumping it, and the 3-day age gate on + that bump; the transitive set is not pinned. An instance that wants it + pinned vendors the server (a lockfile and `npm ci`) instead. + Run everything at once with: ```bash @@ -198,10 +309,10 @@ unchanged across every instance), then drives these targets: | Target | Runs | Where | Env available | |---|---|---|---| | `release-arches` | once, first | host runner | none; must print a JSON array on stdout, e.g. `["amd64"]` | -| `build`, `sbom`, `release-assets`, `publish` | once **per arch**, in that order | host runner for that arch | `RELEASE_VERSION`, `RELEASE_TAG`, `RELEASE_ARCH`, `OCI_REGISTRY`, `OCI_IMAGE_NAME`, `OCI_REGISTRY_USERNAME`, `OCI_REGISTRY_PASSWORD`, all `GITHUB_*` (`GITHUB_TOKEN` has `packages: write`); no package-registry variables or secrets, and no OIDC token | +| `build`, `sbom`, `release-assets`, `publish` | once **per arch**, in that order | host runner for that arch | `RELEASE_VERSION`, `RELEASE_TAG`, `RELEASE_ARCH`, `OCI_REGISTRY`, `OCI_IMAGE_NAME`, `OCI_REGISTRY_USERNAME`, `OCI_REGISTRY_PASSWORD`, all `GITHUB_*` (`GITHUB_TOKEN` has `packages: write`); the registry credentials come from the `release` environment; no package-registry variables or secrets, and no OIDC token | | `release-check` | once, after all arches; gates the GitHub release and `publish-packages` (not the per-arch image `publish`, which already ran in `build`) | **inside the devcontainer** (`devcontainers/ci`), stack up | `RELEASE_VERSION`, `RELEASE_TAG`, `GITHUB_REPOSITORY`; **not** `RELEASE_ARCH` (it runs once, not per arch) and no `ARCHES`: to check each arch's files in `dist/`, iterate over `make -s release-arches` | | `finalize` | once, after `release-check`; then the GitHub release is created | host runner | same as per-arch minus `RELEASE_ARCH` (`GITHUB_TOKEN` also has `packages: write`) | -| `publish-packages` | once, after the GitHub release exists, **only if** `PYPI_PUBLISH`, `NPM_PUBLISH` or `CARGO_PUBLISH` is set | host runner (own job, with `id-token: write`) | `RELEASE_VERSION`, `RELEASE_TAG`, `PYPI_PUBLISH`, `NPM_PUBLISH`, `CARGO_PUBLISH`, all `GITHUB_*`, the OIDC token, `NPM_TOKEN` and `CARGO_REGISTRY_TOKEN` (bootstrap secrets; empty when unset, so treat empty as absent); `dist/` holds every arch leg's files | +| `publish-packages` | once, after the GitHub release exists, **only if** `PYPI_PUBLISH`, `NPM_PUBLISH` or `CARGO_PUBLISH` is set | host runner (own job, with `id-token: write`, in the `release` environment) | `RELEASE_VERSION`, `RELEASE_TAG`, `PYPI_PUBLISH`, `NPM_PUBLISH`, `CARGO_PUBLISH`, all `GITHUB_*`, the OIDC token, `NPM_TOKEN` and `CARGO_REGISTRY_TOKEN` (bootstrap secrets; empty when unset, so treat empty as absent); `dist/` holds every arch leg's files | | `verify-published` | once, after `publish-packages` succeeded, **only if** a `*_PUBLISH` variable is set | host runner (own job, no secrets, no OIDC token) | `RELEASE_VERSION`, `RELEASE_TAG`, `PYPI_PUBLISH`, `NPM_PUBLISH`, `CARGO_PUBLISH`; `dist/` as for `publish-packages` | What each target is for: `build` produces the release artifact(s) (an @@ -248,6 +359,19 @@ fail naming the missing secret. Trade-off: the token is in the job env, so every target (`build`, `sbom`, `release-assets`), not only `publish`, can write those packages. +**Accepted residual: the GHCR push is gated by the branch, not by a +secret.** With the `OCI_REGISTRY_*` secrets unset, nothing secret stands +between a `Release` dispatch and the push: whoever can dispatch `Release` +on a branch can run that branch's `Makefile` with a `packages: write` +`GITHUB_TOKEN` and publish `ghcr.io//:-` before +any review. The control is template-base's `release` environment, whose +deployment-branch rule allows only the default branch (see template-base's +[Release environment](https://github.com/craftainer/template-base/blob/main/docs/TEMPLATE.md) +bullet in `docs/TEMPLATE.md`); it arrives here with the template-base sync +(template-base#67) and needs no change to the `Makefile`. Until an instance +has synced and created the environment, treat write access to the repository +as the right to publish its GHCR image. + **Upgrading a package instance.** Before this contract, `publish` also uploaded packages, once per arch. Now `publish` is image-only and gets no `*_PUBLISH` variables, secrets or OIDC token. An instance that publishes @@ -398,7 +522,8 @@ these files over base: | `.dockerignore` | `merge` | baseline build-context excludes for the runtime image (including `node_modules`, `coverage`, `target` and `dist`); the instance appends its own | | `scripts/image-smoke.sh` | `replace` | loads the image tar from `dist/`, runs it and polls a URL | | `scripts/runner-setup.sh` | `replace` | optional: creates the group-0 runtime user and its writable directories for the `runner` stage | -| `scripts/image-scan.sh` | `replace` | the Trivy scan the `image-scan` hook runs | +| `scripts/skopeo-dir.sh` | `replace` | the temp directory for skopeo's container: 0700, or 0777 under rootless/userns Docker; `SKOPEO_DIR_MODE=0700\|0777` overrides the detection (set 0777 if `publish` or `image-smoke` hit a permission error on Docker Desktop or a remote daemon) | +| `scripts/image-scan.sh` | `replace` | the Trivy scan the `image-scan` hook and `release-assets` run (advisory in CI unless `IMAGE_SCAN_STRICT=1`) | | `scripts/image-dast.sh` | `replace` | the ZAP baseline scan the `image-dast` hook runs | | `.zap/rules.tsv` | `ignore` | the instance's ZAP rule levels, seeded with a commented header | | `scripts/test-image-scripts.sh` | `replace` | offline tests of the Makefile and `scripts/` logic, run by the `image-scripts-test` hook | @@ -440,7 +565,7 @@ defaults) and every variable is `?=`, so set them in `image.mk`: | `IMAGE_SMOKE_PORT`, `IMAGE_SMOKE_PATH` | `8000`, `/health/live` | URL polled for a 2xx | | `IMAGE_SMOKE_RUN_ARGS`, `IMAGE_SMOKE_TIMEOUT` | empty, `60` | extra `docker run` words, whitespace-separated with no embedded spaces or quotes (passed through the environment); seconds to wait | -`SYFT_VERSION` (the SBOM tool image tag) and `SKOPEO_IMAGE` (tag and +`SYFT_IMAGE` and `SKOPEO_IMAGE` (the SBOM tool and skopeo, each tag and digest) are pinned in the Makefile for Renovate; don't override them. The image name comes from `GITHUB_REPOSITORY` (the repository name), never a hardcoded one. Inside the devcontainer, where `release-check` runs and @@ -495,7 +620,16 @@ instance-tests: uv run pytest --cov-report=xml:dist/coverage.xml ``` -`release-assets` depends on `image-smoke`, so each arch is booted +`release-assets` depends on `image-smoke`, `image-scan` and `image-dast` +(in that order), so each arch is booted, Trivy-scanned (advisory in CI unless +`IMAGE_SCAN_STRICT=1`, see the `image-scan` hook below) and ZAP-scanned (blocking per `.zap/rules.tsv`) +natively on its own runner in the build job before `publish`, on the tar +`make build` just wrote. The hosted runner already has Docker and both +scanners pull their images by digest (Docker Hub, `ghcr.io`), so +`release.yml` is unchanged. When `CI=true` and there is no tar for the +arch, both scripts print a `::warning::` (plus a plain line on stderr, +because prek hides a passing hook's output) instead of skipping silently. +`release-check` (the devcontainer run) does not scan. Each arch is booted natively on its own runner in the build job before `publish`; the `release-check` run inside the devcontainer smoke-tests the host arch again with the backing services up. `image-smoke` converts the arch's @@ -542,7 +676,7 @@ packages and writable directories there instead of editing the script. ### Dockerfile version check The `check-dockerfile-versions` hook runs -`scripts/check-dockerfile-versions.sh DEBIAN_VERSION` whenever either +`scripts/check-dockerfile-versions.sh SSL_CERT_FILE SSL_CERT_DIR` whenever either Dockerfile, the script or `.pre-commit-config.yaml` (where the ARG list lives) changes: each named ARG must have the same default in `.devcontainer/Dockerfile` and `Dockerfile` (the runtime must match the @@ -570,9 +704,14 @@ in its `.pre-commit-config.yaml`; ARGs not listed aren't checked. `RELEASE_ARCH`, else the host's; without `RELEASE_VERSION`, the newest `dist/*-.tar`), never stale tars of other versions. With no such tar it skips. Otherwise `scripts/image-scan.sh` runs a - pinned Trivy image on that tar and fails on HIGH or CRITICAL + pinned Trivy image on that tar and reports HIGH or CRITICAL findings that have a fix (`--severity HIGH,CRITICAL --ignore-unfixed - --exit-code 1`). Trivy can't read the OCI-layout tar `make build` + --exit-code 2`; Trivy errors exit 1). Locally (the manual hook) findings + block. When `CI=true` they are advisory: a `::warning::` annotation, an + entry in `$GITHUB_STEP_SUMMARY` when set, exit 0. `IMAGE_SCAN_STRICT` + overrides either way (`1` blocks, `0` is advisory); the Makefile exports + it, so set it in the environment or as `IMAGE_SCAN_STRICT := 1` in + `image.mk`. Trivy errors fail in every mode. Trivy can't read the OCI-layout tar `make build` writes, so the script unpacks it and scans the directory, selecting `linux/` from the tar's name so a buildx attestation manifest is never scanned instead of the image. The database is cached in the `trivy-cache` Docker @@ -598,8 +737,8 @@ in its `.pre-commit-config.yaml`; ARGs not listed aren't checked. the smoke URL, then runs the pinned ZAP baseline image (`ghcr.io/zaproxy/zaproxy:@sha256:…`, bumped by Renovate) in the app container's network namespace. The scan is passive only (headers, - cookies, information leaks; OWASP A02, A07) and writes `dist/dast/report.html` and - `report.json`. Rule levels come from the instance-owned `.zap/rules.tsv` + cookies, information leaks; OWASP A02, A07) and writes `dist/dast--report.html` and + `dist/dast--report.json` (arch-qualified, so the release assets stay unique per leg). Rule levels come from the instance-owned `.zap/rules.tsv` (`id`, `IGNORE|WARN|FAIL`, `(name)`, tab-separated; seeded with a commented header and no rules). A rule not listed is a WARN. Exit 0 and 2 (warnings only) pass; 1 (a rule at FAIL) and 3 (ZAP error) fail. The spider @@ -663,6 +802,20 @@ Further sync behaviour, detailed in `.github/scripts/README.md`: - The moderation `apply` jobs download the `moderation-result` artifact strictly, so a lost artifact fails the job instead of passing silently (see `.github/workflows/README.md`, "Issue moderation"). +- The issue-moderation workflows (`moderate-*.yml`) keep Claude read-only + and do every write in a separate `apply` job, now for triage as well as + fix/build (see `docs/adrs/0001-harden-the-issue-moderation-workflows.md` + in template-base and `.github/workflows/README.md`). What an instance + sees: `ANTHROPIC_API_KEY` is passed through the process environment only, + never `.secrets/claude.txt`; Claude's git and `gh` tools are narrowed, and + `MODERATION_EXTRA_TOOLS` is the way to add more; a fix or plan that + changes CI, devcontainer, hook, `Makefile`/`*.mk`, `Dockerfile*`, + `compose*.y*ml` or release files ends as a `needs-human` note instead of a + PR; and the PR base is the default branch. Claude's commands run in Claude + Code's sandbox (the image installs `bubblewrap` and `socat`) with a network + allow-list; `MODERATION_EXTRA_DOMAINS` adds hosts (e.g. a package + registry), and the pipeline fails closed if the sandbox cannot start. An instance whose Makefile or + hook configuration the agent needs to change does that by hand. - The scripts under `.github/scripts/` run on the runner's system `python3`. Base ships `.github/scripts/ruff.toml` (`replace` tier), pinned to that Python's `target-version`, and `test_script_syntax.py` guards it with diff --git a/scripts/README.md b/scripts/README.md index a2e6d9e..d2a9b20 100644 --- a/scripts/README.md +++ b/scripts/README.md @@ -7,14 +7,19 @@ copying its shape into a second script. Prefer the hook below over either. Every download goes through its `fetch` helper (bounded curl retries with backoff; GitHub URLs authenticated when the optional - `github_token` BuildKit secret is present) and the installers - that download for themselves run through `retry` (the installer script - itself is fetched once, by `fetch`, so the two layers of retries do not - multiply; a failed download names the installer and URL before the script - aborts, an empty one is rejected, and `fetch` itself retries DNS and + `github_token` BuildKit secret is present) and the one tool that + downloads for itself (`uv python install`) runs through `retry` (never + around a `fetch`, so the two layers of retries do not multiply; a failed + download names what failed and the URL before the script aborts, and + `fetch` itself retries DNS and connect failures (curl exit 6/7) and truncated transfers (18/52/55/56) up to 4 times; timeouts are left to curl's own `--retry`), so CI's shared-IP HTTP 429s don't fail the image build. Local builds need no token. + No vendor install script is run: `uv` and `prek` are release tarballs + checked against the `.sha256` published beside them (`fetch_checked`), + and Claude Code's binary is checked against its GPG-signed + `manifest.json` (key fingerprint pinned in `claude_signing_fpr`); see + `../docs/TEMPLATE.md`'s "Supply chain and release hardening". - `post-create.sh` — the devcontainer's `postCreateCommand`: git `safe.directory`, `prek install`, then every `post-create.d/*.sh`. - `develop.d/`, `post-create.d/`, `runner-setup.d/` — instance hook @@ -26,11 +31,14 @@ `runner-setup.d/*.sh`. An instance whose runner image doesn't call it (an nginx runner, say) ignores it and `runner-setup.d/`. - `image-scan.sh` — the manual-stage `image-scan` hook: Trivy over the - tar for the current `RELEASE_VERSION`/arch in `dist/`. + tar for the current `RELEASE_VERSION`/arch in `dist/`. Findings block locally; with `CI=true` they are advisory + (`::warning::`, `$GITHUB_STEP_SUMMARY`, exit 0). `IMAGE_SCAN_STRICT=1|0` overrides. Also run by + `make release-assets` with `image-dast.sh`. With `CI=true`, a skip for lack + of a tar is a `::warning::`. - `image-dast.sh` — the manual-stage `image-dast` hook: starts the image from the tar in `dist/` as `image-smoke.sh` does and runs the pinned ZAP baseline scan (passive) in its network namespace, with rule levels from the - instance-owned `.zap/rules.tsv`; reports go to `dist/dast/`. + instance-owned `.zap/rules.tsv`; reports go to `dist/dast--report.{html,json}`. - `test-image-scripts.sh` — the `image-scripts-test` hook: offline tests of the Makefile's repo derivation, the `publish`/`finalize` registry credentials (secrets, the ghcr.io `GITHUB_TOKEN` fallback, the non-ghcr @@ -43,7 +51,18 @@ test. Needs `SKOPEO_IMAGE`, which the Makefile exports; usage in the script's header. - `skopeo.sh` — runs the pinned skopeo image as the caller's user with - `dist/` mounted at `/dist`; used by `make publish` and `image-smoke.sh`. + `dist/` mounted at `/dist`; used by `make publish`, `image-smoke.sh` and + `image-dast.sh`. +- `skopeo-dir.sh [TEMPLATE]` — prints a new temp directory skopeo's container + can write to: 0700 normally, 0777 only when `docker info` reports rootless + or userns-remapped Docker (the container's uid then isn't the owner). Used + for `make publish`'s authfile directory and the work directories of + `image-smoke.sh` and `image-dast.sh`. `image-dast.sh` widens its directory + to 0777 only after the image tar is gone, because ZAP runs as uid 1000. + `SKOPEO_DIR_MODE=0700|0777` overrides the detection (Docker Desktop, remote + daemons; anything else is an error). If skopeo fails with a permission + error on a 0700 directory, set `SKOPEO_DIR_MODE=0777`; `image-smoke.sh` and + `image-dast.sh` say so on failure. - `check-dockerfile-versions.sh ARG...` — fails when a named `ARG` default differs between `.devcontainer/Dockerfile` and the top-level `Dockerfile`. - `develop.d/10-corepack.sh` — enables pnpm via Corepack in the diff --git a/scripts/develop.sh b/scripts/develop.sh index 5aeedb1..6a19448 100644 --- a/scripts/develop.sh +++ b/scripts/develop.sh @@ -16,7 +16,7 @@ node_version=$6 gh_version=$7 # Network resilience. CI runners share IPs, so unauthenticated downloads -# from GitHub, the Claude Code installer and the registries intermittently +# from GitHub, Claude Code's release host and the registries intermittently # fail with HTTP 429. Two measures, neither of which changes what is # installed: # @@ -42,7 +42,7 @@ gh_version=$7 # failed checksum or a kill never leaves the bearer-token header file or # a download directory behind. # - retry: reruns a step a bounded number of times with doubling delays, for -# installers that do their own downloads inside the script and so can't be +# tools that do their own downloads (`uv python install`) and so can't be # wrapped by fetch. Never put a `fetch` inside a `retry`: their retries # would multiply. cleanup_paths=() @@ -87,7 +87,7 @@ fetch() { # and looping again would multiply that. At most 4 tries here, each with # curl's own retries inside: bounded, and `retry` never wraps a fetch. A transfer curl can tell is cut short (a Content-Length # mismatch is exit 18, a cut chunked body 56) is therefore re-fetched - # here, not by re-running an installer on a broken file. Callers must write + # here, not by unpacking a broken file. Callers must write # to a file (-o): a re-run after a partial transfer would otherwise # duplicate stdout. local attempt=1 status=0 @@ -130,68 +130,125 @@ retry() { done } -# Installers are downloaded to a file and then run, never piped straight -# from curl: a retried download would otherwise re-emit the script into a -# shell that is already executing its first, truncated part. The file is -# world-readable because the installer runs as the vscode user. +# Nothing here pipes a download into a shell or runs a vendor's install +# script: uv, prek and the Claude Code CLI are fetched as the release's own +# binaries and verified before anything runs, like node, snip and gh below. +# uv and prek publish a `.sha256` beside each release asset, which +# fetch_checked verifies. Claude Code signs its per-release manifest.json +# (SHA256 per platform) with an Anthropic GPG key, so install_claude checks +# the key's fingerprint against the pin below, then the signature, then the +# binary against the manifest -- the vendor's documented verification +# (https://code.claude.com/docs/en/setup, "Binary integrity and code +# signing"). # -# download_installer : labels fetch's failure message -# (a variable assignment scoped to that one call), since `set -e` would -# otherwise abort without saying which installer failed. fetch itself reports, -# so it still runs under `set -e` (not inside an `||` list, which would -# disable it). -installer_script="" -download_installer() { - rm -f "$installer_script" - tmp_path - installer_script="$tmp_result" - chmod a+r "$installer_script" - fetch_label="$1 installer" fetch "$2" -o "$installer_script" - # An empty 200 response would "succeed" at installing nothing. - if [ ! -s "$installer_script" ]; then - echo "downloaded $1 installer is empty: $2" >&2 - return 1 - fi +# fetch_checked : downloads and .sha256 into and +# aborts unless the file matches the checksum listed for its own name. +fetch_checked() { + local url=$1 dir=$2 name + name="${url##*/}" + fetch_label="$name" fetch "$url" -o "${dir}/${name}" + fetch_label="$name checksum" fetch "${url}.sha256" -o "${dir}/${name}.sha256" + # awk selects the line for this exact name (a missing one fails), so + # sha256sum never sees another asset's line or an empty input. + (cd "$dir" && awk -v n="$name" '$2 == n { print; found = 1 } END { exit !found }' "${name}.sha256" | sha256sum -c -) } -# The installers do their own downloads. prek's comes only from github.com, -# so it gets the token (PREK_GITHUB_TOKEN, passed through the environment -# so it never appears in an argument list). uv's tries releases.astral.sh -# first, a third-party host, so UV_GITHUB_TOKEN is deliberately not set; the -# Claude installer downloads from claude.ai, not GitHub. -# -# Each installer script is downloaded once, by `fetch`, which has its own -# retries; only running it is wrapped in `retry`, for the downloads the -# installer does itself. Wrapping the download in `retry` too would multiply -# the two layers of retries (up to 4 x 4 x 6 attempts for one script). -run_uv_installer() { - sudo -u vscode env HOME=/home/vscode INSTALLER_NO_MODIFY_PATH=1 sh "$installer_script" -} +# Fingerprint of Anthropic's Claude Code release signing key (see the docs +# above); a key whose primary fingerprint differs is rejected. +claude_signing_fpr=31DDDE24DDFAB679F42D7BD2BAA929FF1A7ECACE +claude_release_base=https://downloads.claude.ai/claude-code-releases +claude_key_url=https://downloads.claude.ai/keys/claude-code.asc -# An empty PREK_GITHUB_TOKEN is the same as unset to the installer (it tests -# `[ -n "${AUTH_TOKEN:-}" ]`), so a local build sends no header. -run_prek_installer() { - PREK_GITHUB_TOKEN="${GITHUB_TOKEN:-}" sudo --preserve-env=PREK_GITHUB_TOKEN -u vscode \ - env HOME=/home/vscode INSTALLER_NO_MODIFY_PATH=1 sh "$installer_script" +# install_claude : is the +# manifest's platform key (linux-x64, linux-arm64). is a scratch +# directory that the vscode user can traverse. Installs as vscode. +install_claude() { + local version=$1 platform=$2 config_dir=$3 dir=$4 + local key="${dir}/claude-code.asc" gnupg_home fpr status manifest_version checksum + tmp_path -d + gnupg_home="$tmp_result" + fetch_label="Claude Code signing key" fetch "$claude_key_url" -o "$key" + fpr="$(gpg --batch --homedir "$gnupg_home" --show-keys --with-colons "$key" | + awk -F: '$1 == "fpr" && !seen++ { print $10 }')" + if [ "$fpr" != "$claude_signing_fpr" ]; then + echo "Claude Code signing key has fingerprint '${fpr}', expected ${claude_signing_fpr}" >&2 + return 1 + fi + gpg --batch --quiet --homedir "$gnupg_home" --import "$key" + fetch_label="Claude Code manifest" fetch "${claude_release_base}/${version}/manifest.json" -o "${dir}/manifest.json" + fetch_label="Claude Code manifest signature" fetch "${claude_release_base}/${version}/manifest.json.sig" -o "${dir}/manifest.json.sig" + # A bad, missing or unknown-key signature makes gpg exit non-zero. + status="$(gpg --batch --homedir "$gnupg_home" --status-fd 1 --verify "${dir}/manifest.json.sig" "${dir}/manifest.json" 2> /dev/null)" || { + echo "Claude Code manifest is not signed by ${claude_signing_fpr}" >&2 + return 1 + } + # VALIDSIG's 12th field is the signing key's primary fingerprint. + printf '%s\n' "$status" | + awk -v fpr="$claude_signing_fpr" '$1 == "[GNUPG:]" && $2 == "VALIDSIG" && $12 == fpr { ok = 1 } END { exit !ok }' || { + echo "Claude Code manifest is not signed by ${claude_signing_fpr}" >&2 + return 1 + } + manifest_version="$(jq -er '.version' "${dir}/manifest.json")" + if [ "$manifest_version" != "$version" ]; then + echo "Claude Code manifest is for ${manifest_version}, expected ${version}" >&2 + return 1 + fi + checksum="$(jq -er --arg p "$platform" '.platforms[$p].checksum' "${dir}/manifest.json")" + if [[ ! $checksum =~ ^[0-9a-f]{64}$ ]]; then + echo "Claude Code manifest has no valid checksum for ${platform}" >&2 + return 1 + fi + fetch_label="Claude Code binary" fetch "${claude_release_base}/${version}/${platform}/claude" -o "${dir}/claude" + (cd "$dir" && printf '%s claude\n' "$checksum" | sha256sum -c -) + chmod a+rx "${dir}/claude" + # The verified binary installs itself (to ~/.local/share/claude and a + # ~/.local/bin/claude link) without fetching anything else. + sudo -u vscode env HOME=/home/vscode CLAUDE_CONFIG_DIR="$config_dir" \ + "${dir}/claude" install "$version" } -# CLAUDE_CONFIG_DIR matches compose.yml's, so anything the installer writes -# lands where Claude Code will look for it at runtime. -run_claude_installer() { - sudo -u vscode env HOME=/home/vscode CLAUDE_CONFIG_DIR=/home/vscode/.claude \ - bash "$installer_script" "$claude_code_version" +run_uv_python_install() { + sudo -u vscode env HOME=/home/vscode /usr/local/bin/uv python install "$python_version" } -# xz-utils: Node's own release tarballs are .tar.xz. +# xz-utils: Node's own release tarballs are .tar.xz. gnupg and jq: verifying +# the Claude Code manifest's signature and reading its checksum. bubblewrap +# and socat: the dependencies of Claude Code's built-in sandbox on Linux, +# which the issue-moderation workflows require (run_claude.sh fails closed +# without it). apt-get update -apt-get install -y --no-install-recommends xz-utils +apt-get install -y --no-install-recommends gnupg jq xz-utils bubblewrap socat apt-get clean rm -rf /var/lib/apt/lists/* -download_installer uv "https://releases.astral.sh/github/uv/releases/download/${uv_version}/uv-installer.sh" -retry run_uv_installer -ln -s /home/vscode/.local/bin/uv /usr/local/bin/uv -ln -s /home/vscode/.local/bin/uvx /usr/local/bin/uvx +# One arch mapping for every tool below: Debian's name, the Rust triple +# prefix of uv's and prek's release assets, Node's, and Claude Code's +# manifest platform key. +dpkg_arch="$(dpkg --print-architecture)" +case "$dpkg_arch" in + amd64) + rust_arch=x86_64 + node_arch=x64 + claude_platform=linux-x64 + ;; + arm64) + rust_arch=aarch64 + node_arch=arm64 + claude_platform=linux-arm64 + ;; + *) + echo "unsupported architecture: $dpkg_arch" >&2 + exit 1 + ;; +esac + +uv_asset="uv-${rust_arch}-unknown-linux-gnu" +tmp_path -d +uv_tmpdir="$tmp_result" +fetch_checked "https://github.com/astral-sh/uv/releases/download/${uv_version}/${uv_asset}.tar.gz" "$uv_tmpdir" +tar -xzf "${uv_tmpdir}/${uv_asset}.tar.gz" -C "$uv_tmpdir" "${uv_asset}/uv" "${uv_asset}/uvx" +install -m755 -t /usr/local/bin "${uv_tmpdir}/${uv_asset}/uv" "${uv_tmpdir}/${uv_asset}/uvx" +rm -rf "$uv_tmpdir" # python3 isn't otherwise a dependency of this template -- it's # infrastructure tooling only: prek (like pre-commit) needs *some* @@ -202,7 +259,8 @@ ln -s /home/vscode/.local/bin/uvx /usr/local/bin/uvx # rather than an unpinned apt package; `uv python find` below resolves the # path so it can be symlinked to a plain, unversioned `python3` on PATH -- # prek's own hook-venv creation looks for that directly, not via `uv run`. -sudo -u vscode env HOME=/home/vscode /usr/local/bin/uv python install "$python_version" +# uv downloads the build itself, so a failed download is retried here. +retry run_uv_python_install python_bin="$(sudo -u vscode env HOME=/home/vscode /usr/local/bin/uv python find "$python_version")" ln -s "$python_bin" /usr/local/bin/python3 ln -s "$python_bin" /usr/local/bin/python @@ -213,15 +271,6 @@ ln -s "$python_bin" /usr/local/bin/python # against that release's published SHASUMS256.txt) rather than a # devcontainer feature, so every tool this stage installs is pinned the # same way, in the same place -- see .devcontainer/Dockerfile's NODE_VERSION ARG. -node_dpkg_arch="$(dpkg --print-architecture)" -case "$node_dpkg_arch" in - amd64) node_arch=x64 ;; - arm64) node_arch=arm64 ;; - *) - echo "unsupported architecture for Node.js install: $node_dpkg_arch" >&2 - exit 1 - ;; -esac node_asset="node-v${node_version}-linux-${node_arch}" tmp_path -d node_tmpdir="$tmp_result" @@ -239,24 +288,33 @@ ln -s "/usr/local/lib/nodejs/${node_asset}/bin/npx" /usr/local/bin/npx # project dependency of any particular language's package manager -- every # instance, regardless of language, is expected to run # `prek run --all-files --hook-stage manual` (see checks.yml). -download_installer prek "https://github.com/j178/prek/releases/download/v${prek_version}/prek-installer.sh" -retry run_prek_installer -ln -s /home/vscode/.local/bin/prek /usr/local/bin/prek +prek_asset="prek-${rust_arch}-unknown-linux-gnu" +tmp_path -d +prek_tmpdir="$tmp_result" +fetch_checked "https://github.com/j178/prek/releases/download/v${prek_version}/${prek_asset}.tar.gz" "$prek_tmpdir" +tar -xzf "${prek_tmpdir}/${prek_asset}.tar.gz" -C "$prek_tmpdir" "${prek_asset}/prek" +install -m755 -t /usr/local/bin "${prek_tmpdir}/${prek_asset}/prek" +rm -rf "$prek_tmpdir" # Created here, owned by vscode, because a fresh claude-config volume # (compose.yml) copies this directory's ownership and mode on first mount; -# left to the installer it could be missing or root-owned. 0700: it holds +# left to the install it could be missing or root-owned. 0700: it holds # Claude Code's credentials. sudo -u vscode install -d -m 0700 /home/vscode/.claude -download_installer claude https://claude.ai/install.sh -retry run_claude_installer +# CLAUDE_CONFIG_DIR matches compose.yml's, so anything the install writes +# lands where Claude Code will look for it at runtime. +tmp_path -d +claude_tmpdir="$tmp_result" +chmod a+rx "$claude_tmpdir" +install_claude "$claude_code_version" "$claude_platform" /home/vscode/.claude "$claude_tmpdir" +rm -rf "$claude_tmpdir" ln -s /home/vscode/.local/bin/claude /usr/local/bin/claude # For the snip Claude Code PreToolUse hook -- see .claude/README.md. Not on # PyPI/npm, so fetched as a release tarball and checksum-verified against # the project's own published checksums.txt instead of trusting a # curl-pipe-to-sh installer. -snip_arch="$(dpkg --print-architecture)" +snip_arch="$dpkg_arch" snip_asset="snip_${snip_version}_linux_${snip_arch}.tar.gz" tmp_path -d snip_tmpdir="$tmp_result" @@ -275,7 +333,7 @@ ln -s /home/vscode/.local/bin/snip /usr/local/bin/snip # against the release's own checksums file, like snip above. Its login # state lives in the gh-config volume (.devcontainer/compose.yml), so a # rebuild doesn't require `gh auth login` again. -gh_arch="$(dpkg --print-architecture)" +gh_arch="$dpkg_arch" gh_asset="gh_${gh_version}_linux_${gh_arch}" tmp_path -d gh_tmpdir="$tmp_result" diff --git a/scripts/image-dast.sh b/scripts/image-dast.sh index d7172ec..8d75dcc 100755 --- a/scripts/image-dast.sh +++ b/scripts/image-dast.sh @@ -23,7 +23,9 @@ # # ZAP joins the app container's network namespace (`--network container:`), # so it reaches the app on 127.0.0.1:PORT and nothing is exposed on the host -# network. Reports go to dist/dast/ (report.html, report.json). +# network. Reports go to dist/dast--report.{html,json}: flat and +# arch-qualified, because dist/ is merged across the arch legs and attached to +# the release. # # Exit codes of zap-baseline.py (https://www.zaproxy.org/docs/docker/baseline-scan/): # 0 nothing found, 2 warnings only: both pass. 1 at least one rule at FAIL, @@ -55,7 +57,12 @@ else done fi if [[ -z "$tar" ]]; then - echo "image-dast: no linux/$arch image tar in dist/${RELEASE_VERSION:+ for $RELEASE_VERSION}, skipping" + msg="image-dast: no linux/$arch image tar in dist/${RELEASE_VERSION:+ for $RELEASE_VERSION}, skipping" + echo "$msg" + if [[ "${CI:-}" == true ]]; then + echo "::warning title=image-dast skipped::$msg; nothing was scanned" + echo "$msg; nothing was scanned" >&2 + fi exit 0 fi @@ -117,17 +124,20 @@ trap cleanup EXIT # shares the workspace but not this process's /tmp). echo "image-dast: loading $tar (linux/$arch)" tar_dir="$(cd "$(dirname "$tar")" && pwd)" -work="$(mktemp -d "$tar_dir/.image-dast.XXXXXX")" -# 0777: the container's user (ZAP runs as uid 1000) must be able to write -# the reports here, also with rootless or userns-remapped Docker. -chmod 0777 "$work" +# Private (0700, or 0777 only under rootless/userns Docker; see skopeo-dir.sh) +# while it holds the image tar. +work="$(bash "$root/scripts/skopeo-dir.sh" "$tar_dir/.image-dast.XXXXXX")" image="image-dast-$RANDOM$RANDOM:dast" SKOPEO_DIST_DIR="$tar_dir" bash "$root/scripts/skopeo.sh" -- \ --override-os linux --override-arch "$arch" \ copy "oci-archive:/dist/$(basename "$tar")" \ - "docker-archive:/dist/$(basename "$work")/image.tar:$image" + "docker-archive:/dist/$(basename "$work")/image.tar:$image" \ + || { echo "error: skopeo failed; if this is a permission error, retry with SKOPEO_DIR_MODE=0777 (see scripts/skopeo-dir.sh)." >&2; exit 1; } docker load -i "$work/image.tar" rm -f "$work/image.tar" +# 0777 from here on, with the image tar gone: ZAP runs as uid 1000, not the +# caller, and must be able to write its reports (and read rules.tsv) here. +chmod 0777 "$work" # ${run_args[@]+...}: an empty array is unbound under `set -u` on bash < 4.4. cid="$(docker run -d -p "127.0.0.1::${port}" ${run_args[@]+"${run_args[@]}"} "$image")" @@ -172,13 +182,13 @@ docker run --rm --network "container:$cid" -v "$work:/zap/wrk:rw" "$zap" \ zap-baseline.py "${zap_args[@]}" || zap_status=$? if [[ -f "$work/report.html" || -f "$work/report.json" ]]; then - rm -rf dist/dast - mkdir -p dist/dast + # Flat and arch-qualified: dist/ is merged across arch legs (assemble_dist.py + # rejects one path from two arches) and every file in it is a release asset. for f in report.html report.json; do - if [[ -f "$work/$f" ]]; then cp "$work/$f" "dist/dast/$f"; fi + if [[ -f "$work/$f" ]]; then cp "$work/$f" "dist/dast-$arch-$f"; fi done fi -echo "image-dast: reports in dist/dast/" +echo "image-dast: reports in dist/dast-$arch-report.{html,json}" case "$zap_status" in 0) echo "image-dast: no findings" ;; diff --git a/scripts/image-scan.sh b/scripts/image-scan.sh index 1a7bf49..ca3ada7 100755 --- a/scripts/image-scan.sh +++ b/scripts/image-scan.sh @@ -5,6 +5,12 @@ # host's) selects the arch; RELEASE_VERSION selects dist/*--.tar, # and when unset the newest dist/*-.tar is used, so tars of older # versions or other arches left in dist/ are never scanned. +# Findings block locally. When CI=true they are advisory: printed as a +# `::warning::` annotation and appended to $GITHUB_STEP_SUMMARY when set, exit +# 0. IMAGE_SCAN_STRICT overrides either way (1 blocks, 0 is advisory); the +# Makefile passes it through from the environment or image.mk. Trivy errors +# (no DB, bad tar) fail in every mode. In CI a skip for lack of a tar is also a +# `::warning::`, since a passing prek hook hides its output. # Run by the manual-stage `image-scan` pre-commit hook; needs Docker and # network access (the vulnerability DB is cached in the trivy-cache volume). # @@ -39,7 +45,12 @@ else if [[ -n "$newest" ]]; then tars=("$newest"); fi fi if [[ ${#tars[@]} -eq 0 ]]; then - echo "image-scan: no linux/$arch image tar in dist/${RELEASE_VERSION:+ for $RELEASE_VERSION}, skipping" + msg="image-scan: no linux/$arch image tar in dist/${RELEASE_VERSION:+ for $RELEASE_VERSION}, skipping" + echo "$msg" + if [[ "${CI:-}" == true ]]; then + echo "::warning title=image-scan skipped::$msg; nothing was scanned" + echo "$msg; nothing was scanned" >&2 + fi exit 0 fi @@ -48,6 +59,11 @@ fi work="$(mktemp -d dist/.image-scan.XXXXXX)" trap 'rm -rf "$work"' EXIT +strict="${IMAGE_SCAN_STRICT:-}" +if [[ -z "$strict" ]]; then + if [[ "${CI:-}" == true ]]; then strict=0; else strict=1; fi +fi + status=0 for t in "${tars[@]}"; do name="$(basename "$t" .tar)" @@ -65,8 +81,31 @@ for t in "${tars[@]}"; do mount=("$layout:/image:ro") input=/image fi + # --exit-code 2: findings (2) are told apart from Trivy errors (1). + rc=0 docker run --rm -v trivy-cache:/root/.cache -v "${mount[0]}" "$trivy" \ image --input "$input" --platform "linux/$arch" \ - --severity HIGH,CRITICAL --ignore-unfixed --exit-code 1 || status=1 + --severity HIGH,CRITICAL --ignore-unfixed --exit-code 2 \ + >"$work/trivy.out" 2>&1 || rc=$? + cat "$work/trivy.out" + case "$rc" in + 0) ;; + 2) + if [[ "$strict" == 1 ]]; then + echo "image-scan: HIGH/CRITICAL fixable vulnerabilities in $t (linux/$arch); IMAGE_SCAN_STRICT=0 makes this advisory" >&2 + else + echo "::warning title=image-scan::HIGH/CRITICAL fixable vulnerabilities in $t (linux/$arch); IMAGE_SCAN_STRICT=1 makes this blocking" + fi + if [[ -n "${GITHUB_STEP_SUMMARY:-}" ]]; then + { + printf '### image-scan: %s (linux/%s)\n\n```\n' "$t" "$arch" + cat "$work/trivy.out" + printf '```\n\n' + } >>"$GITHUB_STEP_SUMMARY" + fi + if [[ "$strict" == 1 ]]; then status=1; fi + ;; + *) status=1 ;; + esac done exit "$status" diff --git a/scripts/image-smoke.sh b/scripts/image-smoke.sh index a67110c..d988bb7 100755 --- a/scripts/image-smoke.sh +++ b/scripts/image-smoke.sh @@ -82,18 +82,17 @@ trap cleanup EXIT : "${SKOPEO_IMAGE:?SKOPEO_IMAGE is required (set by the Makefile)}" echo "image-smoke: loading $tar (linux/$arch)" tar_dir="$(cd "$(dirname "$tar")" && pwd)" -work="$(mktemp -d "$tar_dir/.image-smoke.XXXXXX")" -# mktemp makes the directory 0700; with rootless or userns-remapped Docker -# the container's user can't write into it. It only exists inside dist/, for -# the length of this script. -chmod 0777 "$work" +# 0700 unless rootless or userns-remapped Docker needs 0777 for skopeo's user +# (see skopeo-dir.sh). It only exists inside dist/, for the length of this script. +work="$(bash "$(dirname "${BASH_SOURCE[0]}")/skopeo-dir.sh" "$tar_dir/.image-smoke.XXXXXX")" # Run (and later remove) a private reference, so cleanup never touches an # image that already existed. image="image-smoke-$RANDOM$RANDOM:smoke" SKOPEO_DIST_DIR="$tar_dir" bash "$(dirname "${BASH_SOURCE[0]}")/skopeo.sh" -- \ --override-os linux --override-arch "$arch" \ copy "oci-archive:/dist/$(basename "$tar")" \ - "docker-archive:/dist/$(basename "$work")/image.tar:$image" + "docker-archive:/dist/$(basename "$work")/image.tar:$image" \ + || { echo "error: skopeo failed; if this is a permission error, retry with SKOPEO_DIR_MODE=0777 (see scripts/skopeo-dir.sh)." >&2; exit 1; } docker load -i "$work/image.tar" # ${run_args[@]+...}: an empty array is unbound under `set -u` on bash < 4.4. @@ -116,7 +115,7 @@ while ((SECONDS < deadline)); do echo "image-smoke: container exited before answering" >&2 exit 1 fi - if curl -fsS -o /dev/null "$url" 2>/dev/null; then + if curl -fsS --max-time 5 -o /dev/null "$url" 2>/dev/null; then echo "image-smoke: $url answered 2xx" exit 0 fi diff --git a/scripts/skopeo-dir.sh b/scripts/skopeo-dir.sh new file mode 100755 index 0000000..7f0c9a9 --- /dev/null +++ b/scripts/skopeo-dir.sh @@ -0,0 +1,34 @@ +#!/usr/bin/env bash +# Prints the path of a new temporary directory that skopeo.sh's container can +# write to, with the least permission that works. skopeo.sh runs the container +# as the caller's uid:gid, so a plain `mktemp -d` (0700, owned by the caller) +# is enough on a rootful daemon. Under rootless or userns-remapped Docker the +# container's uid maps to a subordinate uid that is not the owner, so the +# directory has to be 0777 there (a known residual on shared hosts: it is +# short-lived and the caller removes it). +# +# SKOPEO_DIR_MODE=0700|0777 overrides the detection (Docker Desktop, remote +# daemons); any other value is an error. +# +# Usage: skopeo-dir.sh [MKTEMP_TEMPLATE] +set -euo pipefail + +case "${SKOPEO_DIR_MODE:-}" in + "" | 0700 | 0777) ;; + *) echo "skopeo-dir: SKOPEO_DIR_MODE must be 0700 or 0777, got '$SKOPEO_DIR_MODE'" >&2; exit 2 ;; +esac + +dir="$(mktemp -d "${1:-${TMPDIR:-/tmp}/skopeo.XXXXXX}")" + +if [[ -n "${SKOPEO_DIR_MODE:-}" ]]; then + chmod "$SKOPEO_DIR_MODE" "$dir" +else + # SecurityOptions lists e.g. "name=seccomp,profile=builtin name=rootless"; + # an unreachable daemon or a missing format field falls through to 0700. + opts="$(docker info --format '{{range .SecurityOptions}}{{.}} {{end}}' 2>/dev/null || true)" + case "$opts" in + *name=rootless* | *name=userns*) chmod 0777 "$dir" ;; + esac +fi + +printf '%s\n' "$dir" diff --git a/scripts/test-image-scripts.sh b/scripts/test-image-scripts.sh index 37cf6a2..0dc5435 100755 --- a/scripts/test-image-scripts.sh +++ b/scripts/test-image-scripts.sh @@ -2,7 +2,7 @@ # Offline tests for the template-owned image logic that has no other check: # the Makefile's repo derivation, check-dockerfile-versions.sh, image-smoke.sh's # missing-tar failure, skopeo.sh's argument handling and image-dast.sh's -# skip, ZAP wiring and exit-code mapping. Needs no Docker daemon +# skip, skopeo-dir.sh's mode, ZAP wiring and exit-code mapping. Needs no Docker daemon # (docker is stubbed) and writes only to a temp directory. Run by the # `image-scripts-test` hook. set -euo pipefail @@ -114,6 +114,31 @@ out="$(sk -- copy a b)" || fail "skopeo.sh: no docker args should work" out="$(sk -i -- login)" || fail "skopeo.sh: docker args should work" [[ "$out" == *" -i img login" ]] || fail "skopeo.sh: unexpected command: $out" +# skopeo-dir.sh: 0700 on a rootful daemon (or none reachable), 0777 only when +# `docker info` reports rootless or userns. +sd="$tmp/skdir" +mkdir -p "$sd/bin" +cat >"$sd/bin/docker" <<'EOF_STUB' +#!/usr/bin/env bash +[[ "$1" == info ]] || exit 1 +printf '%s' "${STUB_SECOPTS:-}" +[[ "${STUB_SECOPTS:-}" != FAIL ]] || exit 1 +EOF_STUB +chmod +x "$sd/bin/docker" +skdir_mode() { + local d + d="$(PATH="$sd/bin:$PATH" TMPDIR="$sd" STUB_SECOPTS="$1" SKOPEO_DIR_MODE="${2:-}" bash "$root/scripts/skopeo-dir.sh" 2>/dev/null)" || { echo error; return; } + stat -c %a "$d" + rm -rf "$d" +} +expect_eq "skopeo-dir: rootful" 700 "$(skdir_mode 'name=seccomp,profile=builtin ')" +expect_eq "skopeo-dir: no daemon" 700 "$(skdir_mode FAIL)" +expect_eq "skopeo-dir: rootless" 777 "$(skdir_mode 'name=seccomp,profile=builtin name=rootless ')" +expect_eq "skopeo-dir: userns" 777 "$(skdir_mode 'name=userns ')" +expect_eq "skopeo-dir: override to 0777" 777 "$(skdir_mode '' 0777)" +expect_eq "skopeo-dir: override to 0700 beats rootless" 700 "$(skdir_mode 'name=rootless ' 0700)" +expect_eq "skopeo-dir: invalid override" error "$(skdir_mode '' 0755)" + # Makefile publish/finalize credentials, with docker stubbed (it records its # argv and the stdin of any --password-stdin login): the OCI_REGISTRY_* secrets # win; with them unset ghcr.io falls back to GITHUB_ACTOR/GITHUB_TOKEN; any @@ -165,6 +190,64 @@ for target in publish finalize; do [[ "$(creds_rc)" != 0 && "$out" == *OCI_REGISTRY_PASSWORD* ]] || fail "$target username without password should fail naming OCI_REGISTRY_PASSWORD (rc=$(creds_rc)): $out" done +# image-scan.sh with docker stubbed (Trivy's exit code and output come from +# the environment): findings (2) are advisory by default (a ::warning:: and a +# job-summary entry, exit 0), IMAGE_SCAN_STRICT=1 makes them fail, a Trivy +# error (1) always fails, and a skip is a loud warning only when CI=true. +sc="$tmp/scan" +mkdir -p "$sc/scripts" "$sc/dist" "$sc/bin" +cp "$root/scripts/image-scan.sh" "$sc/scripts/" +cat >"$sc/bin/docker" <<'EOF_STUB' +#!/usr/bin/env bash +echo "Total: 3 (HIGH: 3)" +exit "${TRIVY_EXIT:-0}" +EOF_STUB +chmod +x "$sc/bin/docker" +scan_env() { (cd "$sc" && env -u CI -u GITHUB_STEP_SUMMARY -u IMAGE_SCAN_STRICT PATH="$sc/bin:$PATH" "$@" bash scripts/image-scan.sh); } +rc=0 +out="$(scan_env CI=true 2>"$sc/err")" || rc=$? +expect_eq "image-scan: skip exit" 0 "$rc" +[[ "$out" == *"::warning title=image-scan skipped"* && "$(cat "$sc/err")" == *"nothing was scanned"* ]] || fail "image-scan: CI skip should warn on stdout and stderr: $out" +out="$(scan_env 2>&1)" +[[ "$out" != *"::warning"* && "$out" == *skipping* ]] || fail "image-scan: a skip outside CI should not warn: $out" +printf 'x' >"$sc/manifest.json" +tar -cf "$sc/dist/app-1.0.0-$host.tar" -C "$sc" manifest.json +rc=0 +out="$(scan_env TRIVY_EXIT=0 RELEASE_VERSION=1.0.0)" || rc=$? +[[ "$rc" == 0 && "$out" != *"::warning"* ]] || fail "image-scan: clean scan should pass quietly (rc=$rc): $out" +rc=0 +: >"$sc/summary" +out="$(scan_env CI=true TRIVY_EXIT=2 RELEASE_VERSION=1.0.0 GITHUB_STEP_SUMMARY="$sc/summary")" || rc=$? +[[ "$rc" == 0 && "$out" == *"::warning title=image-scan::"* ]] || fail "image-scan: findings in CI should warn and pass (rc=$rc): $out" +[[ "$(cat "$sc/summary")" == *"Total: 3"* ]] || fail "image-scan: findings not in the step summary: $(cat "$sc/summary")" +rc=0 +out="$(scan_env TRIVY_EXIT=2 RELEASE_VERSION=1.0.0 2>&1)" || rc=$? +[[ "$rc" == 1 && "$out" != *"::warning"* ]] || fail "image-scan: findings locally should block without an annotation (rc=$rc): $out" +rc=0 +out="$(scan_env TRIVY_EXIT=2 RELEASE_VERSION=1.0.0 IMAGE_SCAN_STRICT=0)" || rc=$? +expect_eq "image-scan: local findings with IMAGE_SCAN_STRICT=0" 0 "$rc" +rc=0 +out="$(scan_env CI=true TRIVY_EXIT=2 RELEASE_VERSION=1.0.0 IMAGE_SCAN_STRICT=1)" || rc=$? +expect_eq "image-scan: CI findings with IMAGE_SCAN_STRICT=1" 1 "$rc" +rc=0 +out="$(scan_env CI=true TRIVY_EXIT=1 RELEASE_VERSION=1.0.0)" || rc=$? +expect_eq "image-scan: trivy error" 1 "$rc" + +# IMAGE_SCAN_STRICT reaches the scripts through make: from the environment and +# from image.mk, and `release-assets` runs image-scan and image-dast. +ms="$tmp/mkstrict" +mkdir -p "$ms" +printf 'show:\n\t@echo "[$$IMAGE_SCAN_STRICT]"\n' >"$ms/show.mk" +out="$(cd "$ms" && env -u IMAGE_SCAN_STRICT GITHUB_REPOSITORY=o/r make -s -f "$root/Makefile" -f show.mk show)" +expect_eq "make: IMAGE_SCAN_STRICT default" "[]" "$out" +out="$(cd "$ms" && IMAGE_SCAN_STRICT=1 GITHUB_REPOSITORY=o/r make -s -f "$root/Makefile" -f show.mk show)" +expect_eq "make: IMAGE_SCAN_STRICT from the environment" "[1]" "$out" +printf 'IMAGE_SCAN_STRICT := 0\n' >"$ms/image.mk" +out="$(cd "$ms" && env -u IMAGE_SCAN_STRICT GITHUB_REPOSITORY=o/r make -s -f "$root/Makefile" -f show.mk show)" +expect_eq "make: IMAGE_SCAN_STRICT from image.mk" "[0]" "$out" +out="$(cd "$ms" && GITHUB_REPOSITORY=o/r RELEASE_ARCH=amd64 RELEASE_VERSION=1.0.0 ARCHES=amd64 make -n -f "$root/Makefile" release-assets)" +[[ "$out" == *"bash scripts/image-scan.sh"* && "$out" == *"bash scripts/image-dast.sh"* ]] || fail "make: release-assets should run image-scan and image-dast: $out" + # image-dast.sh in a scratch copy of the repo (it works relative to its own # repo root), with docker and curl stubbed: skips without a tar, wires ZAP to # the app container's network and the rules file, and maps zap-baseline.py's @@ -172,7 +255,7 @@ done dd="$tmp/dast" mkdir -p "$dd/scripts" "$dd/dist" "$dd/.zap" "$dd/bin" cp "$root/Makefile" "$dd/" -cp "$root/scripts/image-dast.sh" "$root/scripts/skopeo.sh" "$dd/scripts/" +cp "$root/scripts/image-dast.sh" "$root/scripts/skopeo.sh" "$root/scripts/skopeo-dir.sh" "$dd/scripts/" printf '# header\n10038\tFAIL\t(CSP)\n' >"$dd/.zap/rules.tsv" cat >"$dd/bin/docker" <<'EOF_STUB' #!/usr/bin/env bash @@ -201,6 +284,9 @@ dast() { } out="$(dast env)" || fail "image-dast: no tar should skip: $out" [[ "$out" == *"skipping"* ]] || fail "image-dast: wrong skip message: $out" +[[ "$out" != *"::warning"* ]] || fail "image-dast: a skip outside CI should not warn: $out" +out="$(dast env CI=true)" || fail "image-dast: a CI skip should still pass" +[[ "$out" == *"::warning title=image-dast skipped"* && "$out" == *$'\n'"image-dast: no linux"*"nothing was scanned" ]] || fail "image-dast: CI skip should warn on stdout and stderr: $out" touch "$dd/dist/app-1.0.0-$host.tar" for pair in 0:ok 2:ok 1:fail 3:fail; do : >"$dd/log"