Phase 9 part A

.github/workflows/ci.yml

@@ -58,6 +58,10 @@ jobs:
         run: |
           shellcheck --version
           shellcheck scripts/*.sh .githooks/commit-msg exercises/*/*.sh
+      - name: no credentials on command lines
+        run: |
+          scripts/ci-check-credential-args.sh --self-test
+          scripts/ci-check-credential-args.sh
       - name: commit-msg hook cases
         run: bash scripts/test-commit-msg.sh
       - name: i18n key parity

deploy/argocd/project.yaml

@@ -1,8 +1,10 @@
 # The Argo CD project RootCause's Application belongs to (spec section 13.2).
 # It limits what a bad commit on main can make Argo CD do: one source repo,
-# the two RootCause namespaces on this cluster only, no cluster-wide objects
-# at all (the namespaces come from scripts/k8s-up.sh), and only the kinds
-# deploy/k8s uses. Argo CD's core install has no default project, so an
+# the two RootCause namespaces on this cluster only, and only the kinds
+# deploy/k8s uses. The one cluster-wide kind allowed is the admission policy
+# for sandbox pods and its binding, so self-heal restores the policy if it is
+# deleted or weakened (spec section 13.2); the namespaces still come from
+# scripts/k8s-up.sh. Argo CD's core install has no default project, so an
 # Application outside this one syncs nothing.
 apiVersion: argoproj.io/v1alpha1
 kind: AppProject
@@ -18,7 +20,11 @@ spec:
       namespace: rootcause-system
     - server: https://kubernetes.default.svc
       namespace: sandboxes
-  clusterResourceWhitelist: []
+  clusterResourceWhitelist:
+    - group: admissionregistration.k8s.io
+      kind: ValidatingAdmissionPolicy
+    - group: admissionregistration.k8s.io
+      kind: ValidatingAdmissionPolicyBinding
   namespaceResourceWhitelist:
     - group: ""
       kind: Service

deploy/k8s/k3d.yaml

@@ -11,6 +11,11 @@ metadata:
 servers: 1
 agents: 2
 image: rancher/k3s:v1.37.1-k3s1@sha256:ca7f37d993d82ef0dcdcfecb2e0e2618ea541dbaffc620c8cedebe01a82acd0d
+# The Kubernetes API is published on the host's 127.0.0.1 only, like the
+# dashboard. k3d's default is 0.0.0.0. This applies when the cluster is
+# created; an existing cluster keeps its old port until it is recreated.
+kubeAPI:
+  hostIP: "127.0.0.1"
 ports:
   - port: 127.0.0.1:7300:30730
     nodeFilters:

internal/session/engine.go

@@ -164,9 +164,22 @@ const (
 
 // specFor builds the sandbox spec for an exercise session.
 func (e *Engine) specFor(ex *exercise.Exercise, sessionID string) runtime.SandboxSpec {
-	labels := map[string]string{labelSession: sessionID}
+	database := ""
 	if e.st != nil {
-		labels[labelDatabase] = e.st.Database()
+		database = e.st.Database()
+	}
+	return SandboxSpec(ex, sessionID, database)
+}
+
+// SandboxSpec builds the spec a session's sandbox is created from: the
+// exercise's image, limits, network, capabilities, init and tmpfs volumes,
+// and the labels the reaper reads. database labels the database that owns the
+// session; "" leaves the label out. The engine creates every sandbox from
+// it.
+func SandboxSpec(ex *exercise.Exercise, sessionID, database string) runtime.SandboxSpec {
+	labels := map[string]string{labelSession: sessionID}
+	if database != "" {
+		labels[labelDatabase] = database
 	}
 	var tmpfs []runtime.Tmpfs
 	for _, t := range ex.Tmpfs {
@@ -333,7 +346,7 @@ func (e *Engine) runCheck(ctx context.Context, ex *exercise.Exercise, spec runti
 		return e.execScript(ctx, sandboxID, ex.Dir(), "check.sh", env)
 	}
 
-	rs := runnerSpec(spec)
+	rs := RunnerSpec(spec)
 	_ = e.rt.Destroy(ctx, rs.Name) // a runner left by an interrupted check
 	runner, err := e.rt.Create(ctx, rs)
 	if err != nil {
@@ -349,12 +362,12 @@ func (e *Engine) runCheck(ctx context.Context, ex *exercise.Exercise, spec runti
 	return e.execScript(ctx, runner.ID, ex.Dir(), "check.sh", runnerEnv)
 }
 
-// runnerSpec derives the runner container's spec from the sandbox's: the same
+// RunnerSpec derives the runner container's spec from the sandbox's: the same
 // image and locked-down limits, no added capabilities, the internal network
 // (the loader requires it for runner checks), and a label marking it as a
 // runner. It keeps the sandbox's labels, so the reaper treats an orphaned
 // runner like its sandbox.
-func runnerSpec(sandbox runtime.SandboxSpec) runtime.SandboxSpec {
+func RunnerSpec(sandbox runtime.SandboxSpec) runtime.SandboxSpec {
 	labels := make(map[string]string, len(sandbox.Labels)+1)
 	for k, v := range sandbox.Labels {
 		labels[k] = v

internal/session/runcheck_test.go

@@ -54,7 +54,7 @@ func TestRunnerSpec(t *testing.T) {
 		Caps:   []string{"SETUID"},
 		Labels: map[string]string{labelSession: "s-1", labelDatabase: "rootcause_test"},
 	}
-	got := runnerSpec(sandbox)
+	got := RunnerSpec(sandbox)
 	if got.Name != "rootcause-s-1-runner" || got.Image != "img" || got.Network != "internal" {
 		t.Errorf("runner name/image/network = %q/%q/%q", got.Name, got.Image, got.Network)
 	}
@@ -67,7 +67,7 @@ func TestRunnerSpec(t *testing.T) {
 		}
 	}
 	if _, ok := sandbox.Labels[labelRunner]; ok {
-		t.Error("runnerSpec changed the sandbox's own labels")
+		t.Error("RunnerSpec changed the sandbox's own labels")
 	}
 }
 

internal/session/selftest.go

@@ -177,7 +177,7 @@ func (e *Engine) assertNoRunner(ctx context.Context, sandboxName string) error {
 		return err
 	}
 	for _, b := range boxes {
-		if b.Name == runnerSpec(runtime.SandboxSpec{Name: sandboxName}).Name {
+		if b.Name == RunnerSpec(runtime.SandboxSpec{Name: sandboxName}).Name {
 			return fmt.Errorf("runner container %s left behind after the check", b.Name)
 		}
 	}

scripts/ci-check-credential-args.sh

@@ -0,0 +1,60 @@
+#!/usr/bin/env bash
+# Fails if a script passes a credential to a command line, where any process
+# on the machine can read it (ps, /proc/<pid>/cmdline). It looks for curl-style
+# user and header options whose value is built from a command substitution or
+# from a variable named like a token or password:
+#
+#   -u "user:$(cat token-file)"      --user "x:$GITHUB_TOKEN"
+#   -H "Authorization: Bearer $1"    --header "Authorization: token $t"
+#
+# Pass such values on stdin instead (curl --config -).
+#
+# Usage: scripts/ci-check-credential-args.sh [file...]   (default: the other scripts/*.sh)
+#        scripts/ci-check-credential-args.sh --self-test
+set -euo pipefail
+
+root="$(cd "$(dirname "$0")/.." && pwd)"
+cd "$root"
+
+pattern='(^|[[:space:]])(-u|--user|-H|--header)[[:space:]]+"([^"]*\$\(|[^"]*\$\{?[A-Za-z_]*([Tt][Oo][Kk][Ee][Nn]|[Pp][Aa][Ss][Ss][Ww][Oo][Rr][Dd])|[Aa]uthorization:)'
+
+if [[ "${1:-}" == --self-test ]]; then
+	# The line that once put the owner's token on curl's command line, and
+	# the forms the pattern must also catch, then two it must leave alone.
+	# They are literal text, never expanded.
+	# shellcheck disable=SC2016
+	bad=(
+		'curl -fsS -u "DIZAMU:$(tr -d "[:space:]" < ~/.config/rootcause/ghcr-token)" https://ghcr.io/token'
+		'curl -sS -u "${GITHUB_ACTOR:-x}:$GITHUB_TOKEN" https://ghcr.io/token'
+		'curl -fsS -H "Authorization: Bearer $1" https://ghcr.io/v2/'
+		'curl --header "Authorization: token $t" https://api.github.com/'
+	)
+	# shellcheck disable=SC2016
+	good=(
+		'curl -fsSL --retry 3 -o "$tmp/$name" "$url"'
+		'curl -sS -I -H "Accept: $accept" "https://$registry/v2/"'
+	)
+	for line in "${bad[@]}"; do
+		grep -qE -- "$pattern" <<<"$line" || { echo "self-test: not caught: $line" >&2; exit 1; }
+	done
+	for line in "${good[@]}"; do
+		if grep -qE -- "$pattern" <<<"$line"; then echo "self-test: false alarm: $line" >&2; exit 1; fi
+	done
+	echo "self-test: ${#bad[@]} caught, ${#good[@]} left alone"
+	exit 0
+fi
+
+files=("$@")
+if [[ ${#files[@]} -eq 0 ]]; then
+	# Every script but this one, whose comments and self-test hold the
+	# patterns on purpose.
+	for f in scripts/*.sh; do
+		[[ "$f" == scripts/ci-check-credential-args.sh ]] || files+=("$f")
+	done
+fi
+if hits="$(grep -nHE -- "$pattern" "${files[@]}")"; then
+	echo "credential on a command line (pass it on stdin, e.g. curl --config -):" >&2
+	echo "$hits" >&2
+	exit 1
+fi
+echo "no credential arguments in ${#files[@]} files"

scripts/ci-release-image.sh

@@ -20,6 +20,15 @@ cd "$root"
 
 die() { echo "ci-release-image: $*" >&2; exit 1; }
 
+# curl_with <curl config line> <curl args...>: runs curl with one config line
+# (a user or a header) read from stdin, so a credential never reaches a
+# command line, where other processes could read it.
+curl_with() {
+	local line="$1"
+	shift
+	curl --config - "$@" <<<"$line"
+}
+
 # bearer prints a pull token for the repository, from GITHUB_TOKEN. GHCR
 # answers 401 or 403 both when the token may not read the package and when
 # the package does not exist yet (before the first release); bearer then
@@ -28,7 +37,7 @@ bearer() {
 	[[ -n "${GITHUB_TOKEN:-}" ]] || die "GITHUB_TOKEN is not set"
 	local body code
 	body="$(mktemp)"
-	code="$(curl -sS -o "$body" -w '%{http_code}' -u "${GITHUB_ACTOR:-x}:$GITHUB_TOKEN" \
+	code="$(curl_with "user = \"${GITHUB_ACTOR:-x}:$GITHUB_TOKEN\"" -sS -o "$body" -w '%{http_code}' \
 		"https://$registry/token?service=$registry&scope=repository:$repo:pull")" || { rm -f "$body"; die "GHCR token request failed"; }
 	case "$code" in
 	200) python3 -c 'import json, sys; print(json.load(sys.stdin).get("token", ""))' <"$body" ;;
@@ -43,8 +52,8 @@ bearer() {
 manifest_digest() {
 	local code headers
 	headers="$(mktemp)"
-	code="$(curl -sS -o /dev/null -D "$headers" -w '%{http_code}' -I \
-		-H "Authorization: Bearer $1" -H "Accept: $accept" \
+	code="$(curl_with "header = \"Authorization: Bearer $1\"" -sS -o /dev/null -D "$headers" -w '%{http_code}' -I \
+		-H "Accept: $accept" \
 		"https://$registry/v2/$repo/manifests/$2")"
 	case "$code" in
 	200) tr -d '\r' <"$headers" | awk -F': ' 'tolower($1) == "docker-content-digest" { print $2 }' ;;
@@ -57,7 +66,7 @@ manifest_digest() {
 # release_tags <token> prints the repository's vX.Y.Z tags.
 release_tags() {
 	local out
-	out="$(curl -fsS -H "Authorization: Bearer $1" "https://$registry/v2/$repo/tags/list?n=1000")" || return 0
+	out="$(curl_with "header = \"Authorization: Bearer $1\"" -fsS "https://$registry/v2/$repo/tags/list?n=1000")" || return 0
 	python3 -c 'import json, sys; print("\n".join(json.load(sys.stdin).get("tags") or []))' <<<"$out" |
 		grep -E '^v[0-9]+\.[0-9]+\.[0-9]+$' || true
 }

scripts/k8s-gitops-prove.sh

@@ -69,11 +69,15 @@ app() { kc get application rootcause -n argocd -o jsonpath="$1"; }
 
 echo "== 1. the Application"
 if ! kc get application rootcause -n argocd >/dev/null 2>&1; then
+	if ! kc get crd applications.argoproj.io >/dev/null 2>&1; then
+		echo "note  Argo CD is not installed in this cluster"
+	fi
 	fail "no Application rootcause in argocd (run scripts/k8s-up.sh without --dev)"
 	echo
 	echo "k8s-gitops-prove: $failures check(s) failed"
 	exit 1
 fi
+section1_failures="$failures"
 src="$(app '{.spec.project} {.spec.source.repoURL} {.spec.source.targetRevision} {.spec.source.path}')"
 if [[ "$src" == "rootcause $repo_url main deploy/k8s" ]]; then
 	pass "project rootcause, $repo_url, main, deploy/k8s"
@@ -99,6 +103,19 @@ if [[ "$state" == "Synced Healthy $main_sha" ]]; then
 else
 	fail "state '$state', main is $main_sha"
 fi
+# When section 1 fails, Argo CD's own conditions usually say why (a refused
+# spec, an unreachable repository, a manifest that does not render).
+if [[ "$failures" -gt "$section1_failures" ]]; then
+	conditions="$(app '{range .status.conditions[*]}{.type}: {.message}{"\n"}{end}')"
+	if [[ -n "$conditions" ]]; then
+		echo "Argo CD conditions:"
+		while IFS= read -r line; do
+			echo "      $line"
+		done <<<"$conditions"
+	else
+		echo "note  the Application reports no conditions"
+	fi
+fi
 
 echo "== 2. the release"
 revision="$(app '{.status.sync.revision}')"

scripts/k8s-prove.sh

@@ -14,6 +14,9 @@
 #      probes repeated from the real session pod's terminal. It plays on its
 #      own database, rootcause_cluster_test, through a second API pod, and the
 #      real database must be unchanged afterwards (working rule 10).
+#   4. Nothing of the cluster listens beyond the host's 127.0.0.1: every port
+#      the k3d node containers publish (the dashboard and the Kubernetes API)
+#      is bound to 127.0.0.1.
 #
 # The probe pods, the rootcause-probe namespace, the second API pod, its
 # Secret and the test database are removed at the end.
@@ -73,6 +76,8 @@ get nodes - no
 list namespaces - no
 create namespaces - no
 create clusterrolebindings.rbac.authorization.k8s.io - no
+delete validatingadmissionpolicies.admissionregistration.k8s.io - no
+patch validatingadmissionpolicybindings.admissionregistration.k8s.io - no
 impersonate serviceaccounts - no
 impersonate users - no
 * * - no
@@ -272,6 +277,23 @@ else
 	fail "sessions: $testdb has $played, rootcause went from $real_before to $real_after"
 fi
 
+echo "== 4. published ports"
+# docker ps prints each published port as "IP:hostport->port/proto".
+published="$(docker ps --filter "label=k3d.cluster=rootcause" --format '{{.Names}} {{.Ports}}')"
+# The address may be IPv6 in brackets ([::]:port); "]" goes first in the set.
+bindings="$(grep -oE '[][0-9a-f.:]+:[0-9]+->[0-9]+/[a-z]+' <<<"$published" || true)"
+if [[ -z "$bindings" ]]; then
+	fail "no published ports found for the k3d cluster"
+fi
+while read -r b; do
+	[[ -n "$b" ]] || continue
+	if [[ "$b" == 127.0.0.1:* ]]; then
+		pass "published on the host's loopback only: $b"
+	else
+		fail "published beyond 127.0.0.1: $b (deploy/k8s/k3d.yaml binds 127.0.0.1 for a new cluster; recreate it)"
+	fi
+done <<<"$bindings"
+
 echo
 if [[ "$failures" -gt 0 ]]; then
 	echo "k8s-prove: $failures check(s) failed"