test(pm-agent): a committed negative-path probe for the worker's input guards
pm-worker's command is pinned by admission with no args, so everything a caller could influence arrives in the environment — and the guards at the top of that script are what stand between an injected variable and a clone from the wrong host or a push to the wrong ref. Nothing exercised them, which makes them a claim rather than a control. probe-validation.sh asserts 17 refusals, each on BOTH a non-zero exit and the specific message, so a guard that stopped firing cannot pass by failing somewhere else. It covers a redirected git host, a real and reachable WRONG one (the egress policy permits tcp/443, so that case is not hypothetical), the right host with a smuggled path, 'main' and five other bad branch shapes, a malformed owner/repo, a bad mode, a bad issue number, and each missing credential. build-image.sh runs it inside the built image too. Writing it found three things: - the PM_BRANCH != main guard is unreachable while the pattern check above it holds, so the comment claiming it as the defence was wrong about which line does the work. Kept as an assertion independent of the regex; the comment now says so. - the EXIT trap ended in a failing test, which under set -e can replace the script's real exit code with that failure. - a provenance LOG LINE read a file through a redirect that set -o pipefail would have turned into a dead worker if the file were absent. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
This commit is contained in:
jakman85andClaude Opus 5 committed
1 parent
54418363aa
commit
8b5e950baa
6 files changed
+183
-6
No files matched your search
@@ -90,6 +90,26 @@ All offline, in the pinned toolbox (`./infra/devcontainer/dev.sh run`), never on
|
||||
kustomization, because the kustomization's comment named it by filename. Correctly
|
||||
refused: the distance between a filename in a comment and a line under `resources:` is
|
||||
one careless edit. The comment now points at it in prose.
|
||||
- **`infra/agent/probe-validation.sh` → 17 refusals, 0 failures.** A new, committed
|
||||
negative-path probe for `pm-worker`'s input guards — the things that stand between an
|
||||
injected environment variable and a clone from the wrong host or a push to the wrong ref.
|
||||
Each case asserts a non-zero exit AND the specific message, so a guard that stopped firing
|
||||
cannot pass by failing somewhere else. It refuses: an argument on the pinned command;
|
||||
`main`, a wrong prefix, `feat/x;id`, `../../etc/passwd`, too-short slugs, and a branch equal
|
||||
to the base; a redirected git host, a real-and-reachable wrong one (`github.com` — the
|
||||
egress policy permits tcp/443, so this one is genuinely reachable), and the right host with
|
||||
a smuggled path; a malformed owner or repository; an unknown mode; a non-numeric and a zero
|
||||
issue number; and each missing credential. `build-image.sh` also runs it inside the built
|
||||
image, where the interpreter is the one that will run in the cluster.
|
||||
- **Writing that probe corrected an overclaim in the code.** The `PM_BRANCH != "main"` guard
|
||||
is UNREACHABLE while the pattern check above it holds (`main` has no `feat|fix|docs|chore/`
|
||||
prefix), so the comment claiming it as the defence against pushing to main was wrong about
|
||||
which line does the work. It is kept — as an assertion that does not depend on the regex
|
||||
being right — and the comment now says so.
|
||||
- **Two more defects found by self-review of the unattended paths, both fixed:** the worker's
|
||||
`EXIT` trap ended in a failing test, which under `set -e` can replace the script's real exit
|
||||
code with that failure; and a provenance *log line* read a file through a redirect that
|
||||
`set -o pipefail` would have turned into a dead worker if the file were absent.
|
||||
- **A self-review of the trigger found and fixed one real defect** before any of this ran:
|
||||
`last_activity()` compared RFC 3339 timestamp STRINGS to find the newest comment, which is
|
||||
only correct while every offset Gitea returns is identical — and the failure mode is the
|
||||
|
||||
@@ -215,7 +215,10 @@ holds a git write token and can author code. So:
|
||||
under the pinned CLI (the first run was 27/28 and caught a real defect: the volume rule
|
||||
denied a valid `emptyDir: {}`); `check-jsonschema` on the changed ServiceDefinition; the
|
||||
canonical render reproduces `apps/wave-0/gitea.yaml` with the new policy in it; `bash -n`
|
||||
and `ast.parse` on the two entrypoints. **`./ci/gate.sh` → exit 3 (UNDETERMINED), with
|
||||
and `ast.parse` on the two entrypoints; `PM_SELFTEST=1 pm-dispatch` → 26 assertions, 0
|
||||
failures; `infra/agent/probe-validation.sh` → **17 refusals, 0 failures** against
|
||||
`pm-worker`'s input guards, including the three that matter most (a redirected git host, a
|
||||
reachable-but-wrong one, and `main` as the branch). **`./ci/gate.sh` → exit 3 (UNDETERMINED), with
|
||||
`gitea-settings` the one stage that did not run** — no `SETTINGS_API_TOKEN` on this
|
||||
workstation — and every other stage green, `spec-immutability` included. rc 3 is not a pass.
|
||||
The gate also refused an earlier revision of this change twice, both times correctly; see
|
||||
|
||||
@@ -134,6 +134,21 @@ if [ "${VERIFY}" -eq 1 ]; then
|
||||
echo "--- no Anthropic API key is baked into the image (it would outrank the subscription token) ---"
|
||||
[ -z "${ANTHROPIC_API_KEY:-}${ANTHROPIC_AUTH_TOKEN:-}" ] && echo " confirmed: neither is set"
|
||||
'
|
||||
# The worker's input guards, exercised in the image, with the interpreter and the script
|
||||
# that will actually run in the cluster. The probe is mounted read-only rather than baked
|
||||
# in: it is a build-time check, not something a worker Job has any business carrying.
|
||||
log "Probing pm-worker's input validation inside the image"
|
||||
"${ENGINE}" run --rm \
|
||||
--user 65532:65532 \
|
||||
--read-only \
|
||||
--tmpfs /workspace:rw,mode=1777 \
|
||||
--tmpfs /tmp:rw,mode=1777 \
|
||||
--network none \
|
||||
--volume "${HERE}/probe-validation.sh:/probe-validation.sh:ro,Z" \
|
||||
--entrypoint "" \
|
||||
"${IMAGE_REF}" \
|
||||
/bin/bash /probe-validation.sh
|
||||
|
||||
log "Image verified"
|
||||
fi
|
||||
|
||||
|
||||
+21
-4
@@ -73,8 +73,15 @@ esac
|
||||
[[ "${PM_BRANCH}" =~ ^(feat|fix|docs|chore)/[a-z0-9][a-z0-9-]{2,48}$ ]] \
|
||||
|| die "PM_BRANCH '${PM_BRANCH}' does not match ^(feat|fix|docs|chore)/[a-z0-9][a-z0-9-]{2,48}$"
|
||||
|
||||
# NEVER the default branch. A worker that could be handed PM_BRANCH=main would push to main
|
||||
# on its first commit, which CLAUDE.md §1 forbids in the strongest terms it has.
|
||||
# NEVER the default branch. A worker handed PM_BRANCH=main would push to main on its first
|
||||
# commit, which CLAUDE.md §1 forbids in the strongest terms it has.
|
||||
#
|
||||
# The base-branch check is the one that can fire on its own: a slug that satisfies the
|
||||
# pattern above can still equal PM_BASE_BRANCH if someone sets that to a feature branch.
|
||||
# The literal `main` check below is UNREACHABLE while the pattern holds — `main` has no
|
||||
# `feat|fix|docs|chore/` prefix — and is kept deliberately anyway, as an assertion that does
|
||||
# not depend on that regex being right. If the pattern is ever loosened, this still holds.
|
||||
# infra/agent/probe-validation.sh exercises both, and records which message each produces.
|
||||
[ "${PM_BRANCH}" != "${PM_BASE_BRANCH}" ] || die "PM_BRANCH must not be the base branch"
|
||||
[ "${PM_BRANCH}" != "main" ] || die "refusing to work on 'main' (CLAUDE.md §1)"
|
||||
|
||||
@@ -115,7 +122,13 @@ OUT=/workspace/out
|
||||
mkdir -p "$SRC" "$OUT" "$TMPDIR" "${CLAUDE_CONFIG_DIR:-/workspace/.claude}"
|
||||
|
||||
log "assignment: issue #${ISSUE} · mode ${PM_TASK_MODE} · branch ${PM_BRANCH} · model ${MODEL}"
|
||||
log "toolchain: $(cat /etc/agent-runner-versions 2>/dev/null | tr '\n' ' ')"
|
||||
# Guarded with a readability test rather than a redirect plus `2>/dev/null`: redirections are
|
||||
# applied left to right, so a failing `< file` is reported by the shell BEFORE stderr is
|
||||
# silenced. And guarded at all because `set -o pipefail` + `set -e` would otherwise let a
|
||||
# missing provenance file (this script run outside the image) kill the worker on a LOG LINE.
|
||||
VERSIONS=""
|
||||
[ -r /etc/agent-runner-versions ] && VERSIONS="$(tr '\n' ' ' < /etc/agent-runner-versions)"
|
||||
log "toolchain: ${VERSIONS:-(no provenance file; not the agent-runner image?)}"
|
||||
|
||||
# --- 2. the Gitea API, and git credentials that never touch disk in a URL ----
|
||||
# WHY GIT_ASKPASS AND NOT A CREDENTIAL IN THE REMOTE URL (or ~/.git-credentials): a token
|
||||
@@ -214,7 +227,11 @@ report_failure() {
|
||||
} > "${OUT}/failure.md"
|
||||
post_issue_comment "${OUT}/failure.md"
|
||||
}
|
||||
trap 'rc=$?; [ "$rc" -ne 0 ] && report_failure "an unhandled error (exit ${rc}); see the worker log"' EXIT
|
||||
# An `if`, not `test && report`: the EXIT trap's own exit status matters under `set -e`, and
|
||||
# a trailing failed test in it can replace the script's real exit code with that failure. The
|
||||
# `if` form always leaves the trap returning 0, so the status the script exits with is the one
|
||||
# it meant.
|
||||
trap 'rc=$?; if [ "$rc" -ne 0 ]; then report_failure "an unhandled error (exit ${rc}); see the worker log"; fi' EXIT
|
||||
|
||||
# --- 3. clone and put the branch under our feet ------------------------------
|
||||
log "cloning ${PM_GITEA_OWNER}/${PM_GITEA_REPO} from ${PM_GITEA_URL}"
|
||||
|
||||
Executable
+120
@@ -0,0 +1,120 @@
|
||||
#!/usr/bin/env bash
|
||||
###############################################################################
|
||||
# infra/agent/probe-validation.sh — prove pm-worker REFUSES hostile input.
|
||||
#
|
||||
# ./probe-validation.sh # from anywhere; needs only bash
|
||||
#
|
||||
# WHY THIS EXISTS. pm-worker's `command` is pinned by admission with no args, so everything a
|
||||
# caller could influence arrives in the ENVIRONMENT — and the guards at the top of that
|
||||
# script are what stand between an injected variable and a clone from the wrong host, a push
|
||||
# to the wrong ref, or a session with the wrong credential. Guards that nothing exercises are
|
||||
# a claim, not a control. Each case below asserts BOTH a non-zero exit AND the specific
|
||||
# message, because a check that only grepped stderr would pass a build where the guard had
|
||||
# stopped rejecting and something else had started failing instead.
|
||||
#
|
||||
# Every case must die BEFORE the script reaches any network call: the probe runs with a
|
||||
# scrubbed environment and fake credentials, and if a guard ever stopped firing, the case
|
||||
# would fail here rather than by really cloning something.
|
||||
#
|
||||
# Run offline, no cluster, no Gitea, no model. infra/agent/build-image.sh runs it inside the
|
||||
# built image as well, where the interpreter and the script are the ones that will actually
|
||||
# run in the cluster.
|
||||
###############################################################################
|
||||
set -uo pipefail
|
||||
|
||||
HERE="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
|
||||
WORKER="${PM_WORKER_PATH:-${HERE}/pm-worker}"
|
||||
[ -r "$WORKER" ] || { echo "cannot read ${WORKER}" >&2; exit 2; }
|
||||
|
||||
# A complete, VALID assignment. Each case below overrides exactly one thing, so a failure
|
||||
# names the guard rather than a pile of missing variables.
|
||||
BASE=(
|
||||
PM_TASK_MODE=implement
|
||||
PM_ISSUE_NUMBER=42
|
||||
PM_BRANCH=feat/some-real-slug
|
||||
PM_BASE_BRANCH=main
|
||||
PM_GITEA_URL=http://gitea.gitea.svc.cluster.local:3000
|
||||
PM_GITEA_OWNER=Home-Network
|
||||
PM_GITEA_REPO=home-network-k3s-impl
|
||||
PM_GITEA_USER=pm-agent
|
||||
GITEA_TOKEN=not-a-real-token
|
||||
CLAUDE_CODE_OAUTH_TOKEN=not-a-real-token
|
||||
)
|
||||
|
||||
PASS=0
|
||||
FAIL=0
|
||||
|
||||
# `env -i` so nothing from the invoking shell leaks in and accidentally satisfies a guard.
|
||||
refuses() {
|
||||
local label="$1" want="$2"; shift 2
|
||||
local out rc=0
|
||||
out="$(env -i PATH="$PATH" HOME=/tmp "${BASE[@]}" "$@" bash "$WORKER" 2>&1)" || rc=$?
|
||||
if [ "$rc" -eq 0 ]; then
|
||||
printf ' FAIL %s — exited 0; it should have been refused\n' "$label"
|
||||
FAIL=$((FAIL + 1))
|
||||
return
|
||||
fi
|
||||
if printf '%s' "$out" | grep -qF "$want"; then
|
||||
printf ' ok %s (exit %s)\n' "$label" "$rc"
|
||||
PASS=$((PASS + 1))
|
||||
else
|
||||
printf ' FAIL %s — refused (exit %s) but not for the expected reason %q:\n' "$label" "$rc" "$want"
|
||||
printf '%s\n' "$out" | tail -3 | sed 's/^/ /'
|
||||
FAIL=$((FAIL + 1))
|
||||
fi
|
||||
}
|
||||
|
||||
echo "pm-worker input-validation probe ($WORKER)"
|
||||
|
||||
# An argument on a pinned command is caller-supplied code. There is no argument surface.
|
||||
out="$(env -i PATH="$PATH" HOME=/tmp "${BASE[@]}" bash "$WORKER" --layer l2 2>&1)" || rc=$?
|
||||
if printf '%s' "$out" | grep -qF "takes no arguments"; then
|
||||
echo " ok arguments are refused (exit ${rc:-?})"; PASS=$((PASS + 1))
|
||||
else
|
||||
echo " FAIL arguments are refused — got: $(printf '%s' "$out" | tail -1)"; FAIL=$((FAIL + 1))
|
||||
fi
|
||||
|
||||
# --- the ref it may create and push ---------------------------------------
|
||||
refuses "branch 'main' (CLAUDE.md §1)" "does not match" PM_BRANCH=main
|
||||
refuses "branch with a wrong prefix" "does not match" PM_BRANCH=feature/x
|
||||
refuses "branch with a shell metacharacter" "does not match" 'PM_BRANCH=feat/x;id'
|
||||
refuses "branch with path traversal" "does not match" 'PM_BRANCH=../../etc/passwd'
|
||||
refuses "branch too short for the slug rule" "does not match" PM_BRANCH=fix/ab
|
||||
refuses "branch equal to the base branch" "must not be the base branch" \
|
||||
PM_BRANCH=feat/same-slug PM_BASE_BRANCH=feat/same-slug
|
||||
|
||||
# --- where code comes FROM and goes TO -------------------------------------
|
||||
# The namespace's egress policy permits tcp/443 to the world (the model needs it), so an
|
||||
# injected host would be REACHABLE. These are the cases that matter most.
|
||||
refuses "a redirected git host" "is not an allowed git host" \
|
||||
PM_GITEA_URL=https://evil.invalid
|
||||
refuses "a real, reachable, WRONG git host" "is not an allowed git host" \
|
||||
PM_GITEA_URL=https://github.com
|
||||
refuses "the right host on a smuggled path" "is not an allowed git host" \
|
||||
PM_GITEA_URL=http://gitea.gitea.svc.cluster.local:3000/../..
|
||||
refuses "a malformed owner" "PM_GITEA_OWNER is malformed" \
|
||||
'PM_GITEA_OWNER=../other-org'
|
||||
refuses "a malformed repository" "PM_GITEA_REPO is malformed" \
|
||||
'PM_GITEA_REPO=repo;id'
|
||||
|
||||
# --- the assignment itself -------------------------------------------------
|
||||
refuses "an unknown task mode" "must be 'implement' or 'revise'" \
|
||||
PM_TASK_MODE=merge
|
||||
refuses "a non-numeric issue number" "must be a positive integer" \
|
||||
PM_ISSUE_NUMBER=42x
|
||||
refuses "issue number zero" "must be a positive integer" \
|
||||
PM_ISSUE_NUMBER=0
|
||||
|
||||
# --- the credentials ------------------------------------------------------
|
||||
refuses "a missing model credential" "CLAUDE_CODE_OAUTH_TOKEN is not set" \
|
||||
CLAUDE_CODE_OAUTH_TOKEN=
|
||||
refuses "a missing git credential" "GITEA_TOKEN is not set" \
|
||||
GITEA_TOKEN=
|
||||
|
||||
echo
|
||||
if [ "$FAIL" -eq 0 ]; then
|
||||
echo "probe OK — ${PASS} refusal(s), 0 failure(s)"
|
||||
exit 0
|
||||
fi
|
||||
echo "probe FAILED — ${FAIL} of $((PASS + FAIL)) case(s) did not refuse as expected"
|
||||
exit 1
|
||||
@@ -151,7 +151,9 @@ nobody enumerated — on the sibling policy that was `initContainers` (measured:
|
||||
compliant `containers[0]` and a privileged init container was **admitted**, pass 3 fail 0),
|
||||
then `ephemeralContainers`, then `env`, then `envFrom`. All of them are closed here from the
|
||||
start, and `platform/kyverno/tests/pm-agent/` proves each refusal offline: **28 assertions,
|
||||
28 passing** under the pinned `kyverno` CLI.
|
||||
28 passing** under the pinned `kyverno` CLI. The worker's own input guards have their own
|
||||
offline probe — `infra/agent/probe-validation.sh`, **17 refusals** — because a guard nothing
|
||||
exercises is a claim rather than a control.
|
||||
|
||||
> The policy is applied by `platform-config/kustomization.yaml`, and it has to be: adding a
|
||||
> ClusterPolicy only to `platform/kyverno/admission/kustomization.yaml` is a **silent
|
||||
|
||||
Reference in new issue
Block a user