From afec590b34a3680d3b52e302325ad721a3876853 Mon Sep 17 00:00:00 2001 From: Fabian von Feilitzsch Date: Thu, 30 Jul 2026 12:21:16 -0400 Subject: [PATCH] :seedling: Add skill script execution probe and ADR 0007 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Answers, per cluster, whether an agent can stage a script in a writable directory and execute it — and whether that directory sits inside the git worktree the harness commits and pushes. The original concern was that skills mount read-only and possibly noexec, so executable content would be unusable. Measured on CRI-O 1.35: the ImageVolume is read-only at the superblock but noexec does not reach the container, because CRI-O sets it on the lowerdir and overlayfs does not inherit MS_NOEXEC. So execution is not the constraint — placement is. Agents do not execute from the mount anyway; they write a script somewhere writable and run it from there. Given a skill that said only "write the script to a file and run it", the model wrote into the git worktree and left /tmp empty. The watcher did not commit it only because its extension allowlist happens to exclude .sh; it does include .md, .json, .yaml and .txt. ADR 0007 records the resulting convention and why it belongs in the harness rather than in each SkillCard. The probe runs standalone against any cluster — no CRDs, controller or LLM required — and also probes a real Sandbox pod when one exists, so the two can be compared. Refs #70 Co-Authored-By: Claude Opus 5 (1M context) Signed-off-by: Fabian von Feilitzsch --- .gitignore | 7 + README.md | 8 +- changes/unreleased/70-skill-exec-probe.yaml | 7 + ...0007-skill-script-execution-and-staging.md | 216 ++++++++++ hack/probe/probe-pod.yaml | 45 ++ hack/probe/probe.sh | 405 ++++++++++++++++++ hack/probe/run-probe.sh | 257 +++++++++++ 7 files changed, 943 insertions(+), 2 deletions(-) create mode 100644 changes/unreleased/70-skill-exec-probe.yaml create mode 100644 docs/adr/0007-skill-script-execution-and-staging.md create mode 100644 hack/probe/probe-pod.yaml create mode 100755 hack/probe/probe.sh create mode 100755 hack/probe/run-probe.sh diff --git a/.gitignore b/.gitignore index 7545d59d..d68a3ad0 100644 --- a/.gitignore +++ b/.gitignore @@ -31,3 +31,10 @@ go.work # Local scratch files .scratch/ + +# Local dev cluster state: kubeconfig, profile markers, probe results +.dev/ + +# Python bytecode +__pycache__/ +*.pyc diff --git a/README.md b/README.md index 155397c4..46b6772b 100644 --- a/README.md +++ b/README.md @@ -33,7 +33,7 @@ CI pipeline) resolves application metadata before creating the CR. - **Agent Sandbox** is a hard dependency for workload execution - **Git credentials** stay in the harness — the agent does not receive push credentials -- **Skills** are OCI artifacts mounted via ImageVolumes (K8s 1.33+) +- **Skills** are OCI artifacts mounted read-only via ImageVolumes (K8s 1.33+) - **Workspaces** are ephemeral — git is the persistence layer - **ACP over HTTP** (via `goose serve`) provides real-time observability and human-in-the-loop interaction @@ -56,9 +56,13 @@ agentic-controller/ ## Platform requirements -- Kubernetes 1.33+ (ImageVolume GA) +- Kubernetes 1.33+ (ImageVolume beta, enabled by default) - OpenShift 4.20+ - Agent Sandbox v0.5.x +- A container runtime that implements ImageVolume: CRI-O 1.31+ or containerd + 2.0+. cri-dockerd does not implement it. + +For a local cluster to develop against, see [docs/local-development.md](docs/local-development.md). ## Related projects diff --git a/changes/unreleased/70-skill-exec-probe.yaml b/changes/unreleased/70-skill-exec-probe.yaml new file mode 100644 index 00000000..8199e755 --- /dev/null +++ b/changes/unreleased/70-skill-exec-probe.yaml @@ -0,0 +1,7 @@ +kind: enhancement + +description: > + Add a probe that determines, for a given cluster, whether an agent can stage + a script in a writable directory and execute it, and whether that directory + is inside the git worktree the harness commits and pushes. Records the + resulting staging convention in ADR 0007. diff --git a/docs/adr/0007-skill-script-execution-and-staging.md b/docs/adr/0007-skill-script-execution-and-staging.md new file mode 100644 index 00000000..1b468023 --- /dev/null +++ b/docs/adr/0007-skill-script-execution-and-staging.md @@ -0,0 +1,216 @@ +# ADR 0007: Skill Script Execution and Staging + +**Status:** Proposed +**Date:** 2026-07-28 +**Authors:** Fabian von Feilitzsch + +> Numbering note: 0004, 0005, and 0006 are claimed by open PRs (#47, #35, #53). + +## Context + +Skills are packaged as OCI images and mounted into agent pods as Kubernetes +ImageVolumes at `/opt/skills//` (`agentrun_controller.go`, +`resolveSkillVolumes`). As skills grow beyond pure prose into procedures with +concrete commands, the question arises: **can an agent execute a script that +ships with a skill?** + +The concern was that skill mounts are read-only and possibly mounted `noexec`, +which would make executable payloads unusable. The concern was well-founded on +paper: + +- CRI-O passes `[]string{"ro", "noexec", "nosuid", "nodev"}` to + `Store().MountImage()` in `mountImage()` — verified identical on every branch + from `release-1.31` through `main`. +- KEP-4639 dropped `noexec` as a *conformance requirement* when retargeting beta + in v1.34, but that only means runtimes are no longer obliged to apply it. + CRI-O still does. +- containerd's `container_image_mount_linux.go` shows no such flags, so the + behaviour is **runtime-dependent, not merely version-dependent**. + +Two things resolved the question, one by reframing and one by measurement. + +**The reframe.** Agents do not execute scripts *from* the skill mount. They +either emit the script text from `SKILL.md` into a writable directory, or copy +it off the mount — then run it from there. The skill mount is a **read** surface, +not an **exec** surface. `noexec` blocks `execve()`; it never blocks reads. So +the mount's exec semantics do not gate the approach at all. What gates it is +whether some writable directory in the pod is executable. + +**The measurement.** `hack/probe/probe.sh` was run on minikube with CRI-O +1.35.0, Kubernetes v1.34.0, ImageVolume gate BETA/enabled, against both a +hand-written pod and a real controller-created Sandbox. Both agreed: + +| Location | per-mount opts | superblock | noexec reaches container | write | execve | +|---|---|---|---|---|---| +| `/opt/skills/` | `rw,relatime` | `ro,seclabel,...` | **no** | EROFS | allowed | +| `/tmp` | `rw,relatime` (overlay) | `rw` | no | ok | ok | +| `/workspace` | `rw,relatime` (xfs) | `rw` | no | ok | ok | + +The skill mount line, verbatim: + +``` +1267 1259 0:180 / /opt/skills/probe-skill rw,relatime - overlay overlay ro,seclabel,lowerdir=... +``` + +**`noexec` does not reach the container even under CRI-O.** CRI-O applies it to +the lowerdir image-store mount, then layers an `overlay` on top; overlayfs does +not inherit `MS_NOEXEC` from its lower mount. Read-only *is* enforced, but at the +superblock rather than in the per-mount flags — which is why writes fail with +EROFS while the per-mount options read `rw`. + +Independently measured kernel semantics (ubi9-minimal, a real `noexec` tmpfs) +that hold wherever `noexec` *is* applied: `./x.sh` fails, while `sh x.sh`, +`bash x.sh`, `sh < x.sh`, `. x.sh` and `python3 x.py` all succeed — an +interpreter only reads the file. Compiled binaries fail. `chmod` on a read-only +mount fails with EROFS regardless. + +## Decision + +**1. Skills are a read surface. Agents stage scripts before executing them.** + +Executing directly from `/opt/skills/` is not a supported pattern, even though it +currently happens to work under CRI-O. Relying on it would couple skill authoring +to a container-runtime implementation detail that KEP-4639 explicitly stopped +requiring, and that containerd and CRI-O already disagree about. + +**2. The staging directory is `/tmp`, not `/workspace`.** + +`/workspace` is the git worktree. The harness commits and force-pushes it, and +the filesystem watcher introduced in PR #53 auto-commits during a run — so a +script staged there can land on the user's branch. `/tmp` is writable, +executable, and outside the worktree. Both were measured exec-capable; the +difference is the commit risk, not the capability. + +**3. Skill authoring guidance must state this explicitly.** + +This is **measured, not predicted**. A real run — claude-sonnet-5 via goose, the +PR #53 harness, Hub-resolved application, coolstore cloned to `/workspace/repo` — +was given a skill that said only: + +> 1. Write the verification script above to a file. +> 2. Make the file executable. +> 3. Run it. + +with no mention of location. The model did: + +``` +tool: write · /workspace/repo/verify.sh +tool: shell · chmod +x /workspace/repo/verify.sh && /workspace/repo/verify.sh +``` + +It wrote **into the git worktree**. `/tmp` was left empty. Execution succeeded, +confirming again that the exec surface is not the constraint — placement is. + +Left unsaid, the wrong behaviour is the default behaviour. The convention has to +be stated in the skill text itself; it will not be inferred. + +**3b. The rule belongs in the harness, not in each skill or Agent prompt.** + +Where the rule lives matters as much as its content. It is a property of the +execution environment — the working directory is a git worktree that gets pushed +— not of any particular skill, so requiring every SkillCard author (or every +Agent author) to restate it is how it gets forgotten. The harness injects it into +every prompt as a `## Working Environment` preamble in `buildPrompt`, ahead of +the Agent prompt, the playbook context, the skill, and the stage task. + +**Verified.** Rerunning the identical experiment — same skill, same silent +instructions, same model, only the harness changed: + +``` +before: tool: write · /workspace/repo/verify.sh + tool: shell · chmod +x /workspace/repo/verify.sh && /workspace/repo/verify.sh + +after: tool: shell · cat > /tmp/verify.sh << 'EOF' +``` + +Zero writes into the repository. The proposed patch is 22 lines in +`harness/cmd/migration-harness/main.go` and applies to PR #53. + +**3a. The current safety net is incidental, and should not be relied on.** + +The staged `verify.sh` was *not* committed — but only because the filesystem +watcher's `ShouldStageNewFile` allowlists by extension, and `.sh` happens not to +be listed. The base set is `.md .json .yaml .yml .xml .properties .txt`, and the +Java image adds `.java .gradle .kts .kt .groovy`. + +So a stray `.sh` is safe by accident, while a stray `.md`, `.json`, `.yaml` or +`.txt` scratch file **would** be auto-committed and pushed to the user's branch — +and those are exactly the extensions an agent is most likely to use for notes, +plans, and intermediate output. The allowlist is protecting the wrong things by +coincidence. Staging outside the worktree is the actual fix. + +For payloads too large to inline in `SKILL.md`, ship the file in the skill and +copy it out — reading from the mount always works: + +```sh +cp /opt/skills//helper.sh /tmp/ && chmod +x /tmp/helper.sh && /tmp/helper.sh +``` + +**4. Skills do not ship compiled binaries.** + +Binaries cannot run from a `noexec` mount at all, must be built per +architecture, and would make skills non-portable across the runtimes we intend to +support. Tools belong in the agent image, which is where the air-gap and image +composition story already lives (ADR 0001). + +## Alternatives Considered + +**Bake the executable bit into the OCI layer and execute in place.** Works today +under CRI-O, and `skillctl` already preserves modes via `tar.FileInfoHeader` +(`pkg/oci/build.go`). Rejected: it depends on `noexec` not being applied, which +is a runtime implementation detail rather than a guarantee. A hardened cluster, +a policy layer such as OpenShell (PR #47, which explicitly covers "binary +restrictions"), or a future CRI-O change would break every such skill at once. + +**Have the harness stage `/opt/skills/*/scripts/` into a writable exec directory +at startup.** Robust, and it would make the agent's natural `./foo.sh` reflex +work. Rejected as unnecessary: agents already write scripts themselves, so this +adds a harness mechanism to solve a problem that does not exist. Worth revisiting +only if skills ever need to ship executable payloads. + +**Mount skills exec-capable.** Not available: `ImageVolumeSource` exposes only +`reference` and `pullPolicy`. There is no mount-options knob. + +## Consequences + +- Skill authors write scripts as *content*, and the invocation convention is + staging to `/tmp`. This must appear in authoring documentation and ideally in + the stage skills themselves. +- The platform is insensitive to whether a given cluster applies `noexec` to + image volumes, which removes a runtime-dependent variable from the support + matrix. +- `hack/probe/run-probe.sh` re-answers the question on any cluster. It should be re-run + against real OpenShift before dev preview, and whenever the target platform + version moves — the finding here is from minikube/CRI-O 1.35.0 and is evidence, + not a guarantee. +- The only outcome that would invalidate this decision is + `BLOCKED_NO_EXEC_SURFACE`: a cluster where nothing writable is executable. + Not observed, but plausible on hardened clusters that mount emptyDir `noexec`. + +## Appendix: the injected rules + +Verified text, added to `buildPrompt` in the harness ahead of every other +context layer. Reproduced here so the decision survives independently of any +particular branch. + +```go +const stagingRules = `## Working Environment + +Your working directory is a git repository. When this stage finishes it is +committed and pushed to the user's branch, so anything you leave in it ships to +the user. + +Write ephemeral files to /tmp, never into the repository working tree: + - scripts you need to run: write them to /tmp, make them executable there, + and run them from there + - scratch notes, plans, logs, and intermediate output + +Only files the user actually asked for belong in the repository. This applies +even when a skill's instructions do not say where to put something. +` +``` + +## References + +- `hack/probe/probe.sh`, `hack/probe/run-probe.sh` — the probe +- KEP-4639 (OCI VolumeSource); CRI-O `server/container_create_linux.go` diff --git a/hack/probe/probe-pod.yaml b/hack/probe/probe-pod.yaml new file mode 100644 index 00000000..697f6dd4 --- /dev/null +++ b/hack/probe/probe-pod.yaml @@ -0,0 +1,45 @@ +--- +# Standalone probe pod for the staging-directory execution question. +# +# Deliberately depends on NOTHING: no CRDs, no controller, no Agent Sandbox, no +# LLM. It reproduces the shape of a controller-created sandbox pod -- same agent +# base image, an ImageVolume-mounted skill at /opt/skills/, an emptyDir +# workspace -- so its mount semantics are directly comparable, while being +# answerable on a bare cluster in about 30 seconds. +# +# The probe itself is NOT baked in. It is piped in at run time: +# kubectl exec -i imagevolume-probe -- sh -s < hack/probe/probe.sh +# so iterating on the probe never requires rebuilding or reloading an image. +# +# Image placeholders are substituted by hack/probe/run-probe.sh. Applying this +# file directly uses the defaults below. +apiVersion: v1 +kind: Pod +metadata: + name: imagevolume-probe + labels: + app.kubernetes.io/name: imagevolume-probe + app.kubernetes.io/component: diagnostic +spec: + restartPolicy: Never + # Override the stub entrypoint: we want a bare long-running process, not the + # stub's banner logic, so the probe measures the environment and nothing else. + containers: + - name: agent + image: quay.io/konveyor/agentic-controller-agent:e2e + command: ["sleep", "infinity"] + volumeMounts: + - name: skill + mountPath: /opt/skills/probe-skill + readOnly: true + - name: workspace + mountPath: /workspace + volumes: + # The mount under test. Any skill image works -- we are measuring how the + # kubelet/CRI mounts an ImageVolume, not the contents. + - name: skill + image: + reference: quay.io/konveyor/skills:maven-migration + # Matches the controller's workspace volume (agentrun_controller.go:332). + - name: workspace + emptyDir: {} diff --git a/hack/probe/probe.sh b/hack/probe/probe.sh new file mode 100755 index 00000000..aaf4cc54 --- /dev/null +++ b/hack/probe/probe.sh @@ -0,0 +1,405 @@ +#!/bin/sh +# Staging-directory execution probe. +# +# Agents that use scripts embedded in a SKILL.md do not execute them from the +# read-only skill mount -- they write the script to a writable directory and run +# it from there. This probe answers the question that approach actually depends +# on: is there a writable directory in this pod that we can execute from, and is +# it outside the git worktree the harness commits and pushes? +# +# Runs INSIDE a pod. Delivered with: +# kubectl exec -i -- sh -s < hack/probe/probe.sh +# +# POSIX sh only -- the agent image is ubi-minimal (no bash or awk guaranteed). +# Emits "RESULT key=value" lines. ALWAYS exits 0: the verdict lives in the +# output, not the exit code, so a hostile environment yields evidence rather +# than a dead container. + +PROBE_SCHEMA=konveyor.staging-exec-probe/v1 + +# Directories an agent might plausibly stage a script into, in preference order. +# +# HOME is often "/" in these images (the agent user has no real home), so filter +# it out: "/" is the container root, not a staging directory, and probing it +# produces a meaningless row plus an empty result key. +_raw_staging="${PROBE_STAGING_DIRS:-/tmp /workspace /workspace/.konveyor ${HOME:-}}" +STAGING_DIRS="" +for _d in ${_raw_staging}; do + [ -n "${_d}" ] || continue + [ "${_d}" = "/" ] && continue + case " ${STAGING_DIRS} " in *" ${_d} "*) continue ;; esac + STAGING_DIRS="${STAGING_DIRS} ${_d}" +done + +SKILLS_DIR="${PROBE_SKILLS_DIR:-/opt/skills}" + +# ---------------------------------------------------------------- helpers --- + +# Collapse to a single line and truncate. Values are diagnostic strings, not +# data -- lossy is fine, multi-line output corrupting the format is not. +clean() { + tr '\n\r\t' ' ' 2>/dev/null | cut -c1-160 +} + +emit() { + printf 'RESULT %s=%s\n' "$1" "$2" +} + +emit_raw() { + printf 'RESULT %s=%s\n' "$1" "$(printf '%s' "$2" | clean)" +} + +have() { + command -v "$1" >/dev/null 2>&1 +} + +# Is $1 inside a git worktree? Staging there risks the harness committing and +# force-pushing the script to the user's branch. +# +# git is normally present in the agent image, but ubi-minimal does not ship it. +# When it is missing we must NOT default to "yes" -- that would wrongly condemn +# /tmp. Fall back to path containment against the workspace root. +in_worktree() { + _d=$1 + if have git; then + _top=$(git -C "$_d" rev-parse --show-toplevel 2>/dev/null) + [ -n "$_top" ] && return 0 + return 1 + fi + _ws=${PROBE_WORKSPACE:-/workspace} + case "$_d" in + "$_ws" | "$_ws"/*) return 0 ;; + esac + return 1 +} + +# Find the mount covering $1 and echo "|||". +# +# /proc/self/mountinfo: +# field 5 mountpoint +# field 6 per-mount options <- MS_NOEXEC lives HERE +# "-" separator, then fstype, source, super options +# +# Both option sets matter, for different flags: +# noexec is per-mount only. Reading it from the super options is the single +# easiest way to get this whole question wrong. +# ro can come from EITHER. A k8s ImageVolume under CRI-O looks like +# ... rw,relatime - overlay overlay ro,seclabel,lowerdir=... +# i.e. per-mount says rw while the superblock says ro -- and writes +# fail with EROFS. Checking only field 6 reports it as writable. +# +# Longest matching mountpoint wins; on ties the LAST entry wins, because that is +# what the kernel resolves to when several mounts share a mountpoint. +covering_mount() { + _target=$1 + _best_len=-1 + _best="" + while IFS= read -r _line; do + # shellcheck disable=SC2086 + set -- $_line + _mp=$5 + _opts=$6 + # After the "-" separator: fstype, source, super options. + _fstype="" + _superopts="" + _seen_dash=0 + _after=0 + for _f in "$@"; do + if [ "$_seen_dash" = 1 ]; then + _after=$((_after + 1)) + [ "$_after" = 1 ] && _fstype=$_f + [ "$_after" = 3 ] && { _superopts=$_f; break; } + continue + fi + [ "$_f" = "-" ] && _seen_dash=1 + done + + _match=0 + case "$_mp" in + /) _match=1 ;; + *) + case "$_target" in + "$_mp") _match=1 ;; + "$_mp"/*) _match=1 ;; + esac + ;; + esac + [ "$_match" = 1 ] || continue + + _len=${#_mp} + # >= not > : later entries win ties (overmounts). + if [ "$_len" -ge "$_best_len" ]; then + _best_len=$_len + _best="$_mp|$_opts|$_fstype|$_superopts" + fi + done < /proc/self/mountinfo + [ -n "$_best" ] && printf '%s' "$_best" +} + +# Read-only if EITHER the per-mount options or the superblock say so. +mount_is_ro() { + has_opt "$1" ro || has_opt "$2" ro +} + +# Does the option list contain a given flag? Options are comma-separated. +has_opt() { + case ",$1," in + *",$2,"*) return 0 ;; + esac + return 1 +} + +# ------------------------------------------------------------ fingerprint --- +# +# Without this, results are uninterpretable: an OpenShift SCC overrides the +# image's USER, so the UID a check ran as is not knowable from the manifest. + +emit schema "$PROBE_SCHEMA" +emit uid "$(id -u 2>/dev/null)" +emit gid "$(id -g 2>/dev/null)" +emit groups "$(id -G 2>/dev/null | clean)" +emit_raw kernel "$(uname -srm 2>/dev/null)" +emit hostname "${HOSTNAME:-$(hostname 2>/dev/null)}" +emit cwd "$(pwd 2>/dev/null)" + +if [ -r /etc/os-release ]; then + # shellcheck disable=SC1091 + . /etc/os-release 2>/dev/null + emit os "${ID:-unknown}-${VERSION_ID:-unknown}" +else + emit os unknown +fi + +# The only SELinux signal reliably available from inside a container. An empty +# value means SELinux is not labelling this process; it does NOT prove SELinux +# is disabled cluster-wide. +if [ -r /proc/self/attr/current ]; then + emit_raw selinux_label "$(cat /proc/self/attr/current 2>/dev/null)" +else + emit selinux_label unavailable +fi + +# Distinguishes a SKIPPED check from a FAILED one. A missing python3 must never +# be reported as "python cannot execute here". +for _t in sh bash python3 env cp chmod git stat; do + if have "$_t"; then emit "tool_$_t" yes; else emit "tool_$_t" no; fi +done + +# ------------------------------------------------- staging directory probe --- +# +# THE primary question. For each candidate: can we write a script there, mark it +# executable, and execve it -- and if we do, does it land inside the git worktree +# that the harness commits and force-pushes to the user's branch? + +BEST_DIR="" +BEST_DIR_SAFE="" +ANY_EXEC=no + +for dir in $STAGING_DIRS; do + key=$(printf '%s' "$dir" | tr -c 'a-zA-Z0-9' '_') + + if [ ! -d "$dir" ]; then + emit "stage${key}_present" no + continue + fi + emit "stage${key}_present" yes + + mi=$(covering_mount "$dir") + if [ -n "$mi" ]; then + mp=$(printf '%s' "$mi" | cut -d'|' -f1) + opts=$(printf '%s' "$mi" | cut -d'|' -f2) + fstype=$(printf '%s' "$mi" | cut -d'|' -f3) + superopts=$(printf '%s' "$mi" | cut -d'|' -f4) + emit "stage${key}_mount" "$mp" + emit "stage${key}_opts" "$opts" + emit "stage${key}_superopts" "$superopts" + emit "stage${key}_fstype" "$fstype" + # noexec is per-mount only -- never read it from the super options. + if has_opt "$opts" noexec; then + emit "stage${key}_noexec" yes + else + emit "stage${key}_noexec" no + fi + if mount_is_ro "$opts" "$superopts"; then + emit "stage${key}_ro" yes + else + emit "stage${key}_ro" no + fi + else + emit "stage${key}_mount" unknown + fi + + f="$dir/.probe-exec-$$" + if printf '#!/bin/sh\necho PROBE_EXEC_OK\n' > "$f" 2>/dev/null; then + emit "stage${key}_write" ok + else + emit "stage${key}_write" denied + continue + fi + + if chmod +x "$f" 2>/dev/null; then + emit "stage${key}_chmod" ok + else + emit "stage${key}_chmod" denied + fi + + # The capability the agent actually needs. + if out=$("$f" 2>&1); then + emit "stage${key}_exec" ok + ANY_EXEC=yes + else + emit "stage${key}_exec" denied + emit_raw "stage${key}_exec_err" "$out" + fi + + # Fallback if execve is blocked: an interpreter only READS the file, so + # noexec does not stop it. + if out=$(sh "$f" 2>&1); then + emit "stage${key}_interp" ok + else + emit "stage${key}_interp" denied + emit_raw "stage${key}_interp_err" "$out" + fi + + # Can a compiled binary run here? Copy a real ELF rather than shipping one: + # avoids committing a per-architecture blob to the repo. + src="" + for cand in /bin/echo /usr/bin/echo /bin/true; do + [ -x "$cand" ] && { src=$cand; break; } + done + if [ -n "$src" ] && cp "$src" "$dir/.probe-bin-$$" 2>/dev/null; then + chmod +x "$dir/.probe-bin-$$" 2>/dev/null + if "$dir/.probe-bin-$$" probe >/dev/null 2>&1; then + emit "stage${key}_binexec" ok + else + emit "stage${key}_binexec" denied + fi + rm -f "$dir/.probe-bin-$$" 2>/dev/null + else + emit "stage${key}_binexec" skipped + fi + + # Would staging here leak onto the user's branch? The harness commits and + # force-pushes the worktree, and PR #53 adds an fsnotify watcher that + # auto-commits mid-run. + if in_worktree "$dir"; then + emit "stage${key}_in_worktree" yes + if have git; then + emit "stage${key}_git_worktree" \ + "$(git -C "$dir" rev-parse --show-toplevel 2>/dev/null)" + if git -C "$dir" check-ignore -q "$f" 2>/dev/null; then + emit "stage${key}_git_ignored" yes + else + emit "stage${key}_git_ignored" no + fi + else + emit "stage${key}_git_worktree" "path-containment (git absent)" + fi + else + emit "stage${key}_in_worktree" no + fi + + rm -f "$f" 2>/dev/null +done + +# Pick the recommended staging dir: first that can execute, preferring one that +# is not inside a git worktree. +for dir in $STAGING_DIRS; do + key=$(printf '%s' "$dir" | tr -c 'a-zA-Z0-9' '_') + [ -d "$dir" ] || continue + f="$dir/.probe-pick-$$" + printf '#!/bin/sh\nexit 0\n' > "$f" 2>/dev/null || continue + chmod +x "$f" 2>/dev/null + if "$f" >/dev/null 2>&1; then + [ -z "$BEST_DIR" ] && BEST_DIR=$dir + if ! in_worktree "$dir"; then + [ -z "$BEST_DIR_SAFE" ] && BEST_DIR_SAFE=$dir + fi + fi + rm -f "$f" 2>/dev/null +done + +emit staging_exec_any "$ANY_EXEC" +emit staging_best "${BEST_DIR:-none}" +emit staging_best_outside_git "${BEST_DIR_SAFE:-none}" + +# ---------------------------------------------------- skill mount (record) --- +# +# No longer decision-driving -- agents stage scripts elsewhere -- but cheap, and +# it tells us whether skills could ever ship executable payloads directly. +# Reading from the mount always works even under noexec, which is what makes +# "cp from the mount, then exec" a valid alternative to inlining script text. + +if [ -d "$SKILLS_DIR" ]; then + emit skills_present yes + emit_raw skills_list "$(ls -1 "$SKILLS_DIR" 2>/dev/null | tr '\n' ',')" + + for sd in "$SKILLS_DIR"/*; do + [ -d "$sd" ] || continue + name=$(basename "$sd") + skey=$(printf '%s' "$name" | tr -c 'a-zA-Z0-9' '_') + + mi=$(covering_mount "$sd") + if [ -n "$mi" ]; then + opts=$(printf '%s' "$mi" | cut -d'|' -f2) + fstype=$(printf '%s' "$mi" | cut -d'|' -f3) + superopts=$(printf '%s' "$mi" | cut -d'|' -f4) + emit "skill_${skey}_opts" "$opts" + emit "skill_${skey}_superopts" "$superopts" + emit "skill_${skey}_fstype" "$fstype" + # A CRI-O ImageVolume reports per-mount rw with a ro superblock, so + # these two lines legitimately disagree. noexec is per-mount only. + if has_opt "$opts" noexec; then + emit "skill_${skey}_noexec" yes + else + emit "skill_${skey}_noexec" no + fi + if mount_is_ro "$opts" "$superopts"; then + emit "skill_${skey}_ro" yes + else + emit "skill_${skey}_ro" no + fi + fi + + # noexec never blocks reads -- confirm, since the copy-then-exec path + # depends on it. + if [ -f "$sd/SKILL.md" ] && head -c 1 "$sd/SKILL.md" >/dev/null 2>&1; then + emit "skill_${skey}_readable" yes + else + emit "skill_${skey}_readable" no + fi + + # Writes here are EXPECTED to fail. EROFS (read-only fs) vs EPERM (not + # the owner) is a fork in the remediation road, so capture the text. + if touch "$sd/.probe-write-$$" 2>/dev/null; then + emit "skill_${skey}_write" ok + rm -f "$sd/.probe-write-$$" 2>/dev/null + else + err=$(touch "$sd/.probe-write-$$" 2>&1) + emit "skill_${skey}_write" denied + emit_raw "skill_${skey}_write_err" "$err" + fi + done +else + emit skills_present no +fi + +# ------------------------------------------------------------- the verdict --- +# +# Only BLOCKED_NO_EXEC_SURFACE invalidates the write-to-temp-and-run approach. + +if [ "$ANY_EXEC" != yes ]; then + VERDICT=BLOCKED_NO_EXEC_SURFACE + REMEDIATION="no writable+executable directory found; agents cannot stage scripts anywhere" +elif [ -n "$BEST_DIR_SAFE" ]; then + VERDICT="OK_USE_${BEST_DIR_SAFE}" + REMEDIATION="stage scripts in ${BEST_DIR_SAFE} (writable, executable, outside the git worktree)" +else + VERDICT=OK_BUT_COMMIT_RISK + REMEDIATION="only exec-capable dirs are inside the git worktree; staging there risks committing scripts to the user branch" +fi + +emit verdict "$VERDICT" +emit_raw remediation "$REMEDIATION" + +exit 0 diff --git a/hack/probe/run-probe.sh b/hack/probe/run-probe.sh new file mode 100755 index 00000000..5d62f24d --- /dev/null +++ b/hack/probe/run-probe.sh @@ -0,0 +1,257 @@ +#!/usr/bin/env bash +# Run the staging-directory execution probe against a cluster. +# +# Answers: can an agent write a script to a writable directory in its pod and +# execute it -- and is that directory outside the git worktree the harness +# commits and force-pushes? +# +# Probes two places and compares them: +# 1. a standalone pod (no CRDs, no controller) -- fast, always available +# 2. the real controller-created Sandbox for an AgentRun, if one exists +# Divergence between the two is itself a finding: it would mean the controller's +# mount construction differs from a hand-written pod. +# +# Portable across macOS and Linux: no `timeout`, no GNU-only sed/date/stat. +set -euo pipefail + +SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)" +REPO_ROOT="$(cd "${SCRIPT_DIR}/../.." && pwd)" + +# If DEV_KUBECONFIG is set, target that cluster. Without it kubectl falls +# through to ~/.kube/config -- i.e. whatever cluster you happen to be pointed +# at -- and this script CREATES PODS. +# +# Deliberately conditional: running against an arbitrary cluster (a real +# OpenShift, say) is the important use, so an explicitly-set KUBECONFIG with no +# DEV_KUBECONFIG is honoured as-is. The connectivity check below prints the +# target before anything is created. +if [ -n "${DEV_KUBECONFIG:-}" ]; then + export KUBECONFIG="${DEV_KUBECONFIG}" +fi + +PROBE_POD="${PROBE_POD:-imagevolume-probe}" +PROBE_NS="${PROBE_NS:-default}" +AGENT_RUN="${AGENT_RUN:-dev-run}" +AGENT_IMG="${DEV_AGENT_IMG:-quay.io/konveyor/agentic-controller-agent:e2e}" +SKILL_IMG="${PROBE_SKILL_IMG:-quay.io/konveyor/skills:maven-migration}" +RESULTS_DIR="${DEV_RESULTS_DIR:-${REPO_ROOT}/.dev/results}" +WAIT_SECONDS="${PROBE_WAIT_SECONDS:-120}" +KEEP="${PROBE_KEEP:-0}" + +log() { printf '\033[1;34m==>\033[0m %s\n' "$*"; } +warn() { printf '\033[1;33mWARN:\033[0m %s\n' "$*" >&2; } +err() { printf '\033[1;31mERROR:\033[0m %s\n' "$*" >&2; } + +kube() { kubectl -n "${PROBE_NS}" "$@"; } + +# ------------------------------------------------------- cluster attribution +# +# A verdict without this is not portable. "Scripts execute fine" means nothing +# unless you know which runtime produced it -- CRI-O and containerd handle image +# mounts differently, so a containerd answer does not transfer to OpenShift. + +collect_cluster_facts() { + log "Cluster attribution" + + # Show what we are about to create pods on, and stop if it is unreachable. + # Proceeding would otherwise bury the real cause under a wall of + # "connection refused" from every subsequent kubectl call. + local ctx endpoint + ctx="$(kubectl config current-context 2>/dev/null || echo unknown)" + endpoint="$(kubectl config view --minify -o jsonpath='{.clusters[0].cluster.server}' 2>/dev/null || true)" + printf ' context: %s\n' "${ctx}" + printf ' endpoint: %s\n' "${endpoint:-unknown}" + printf ' kubeconfig: %s\n' "${KUBECONFIG:-$HOME/.kube/config}" + + if ! kubectl get --raw /version >/dev/null 2>&1; then + err "cannot reach the cluster at ${endpoint:-} (context '${ctx}')." + err "This script creates pods -- refusing to continue against an unreachable cluster." + err "Point KUBECONFIG (or DEV_KUBECONFIG) at the cluster you want to probe." + exit 1 + fi + + local server + server="$(kubectl version -o json 2>/dev/null \ + | tr -d ' \n' | sed -n 's/.*"serverVersion".*"gitVersion":"\([^"]*\)".*/\1/p' || true)" + printf ' kubernetes: %s\n' "${server:-unknown}" + printf 'RESULT cluster_k8s=%s\n' "${server:-unknown}" >> "${RAW_OUT}" + + local runtimes + runtimes="$(kubectl get nodes \ + -o jsonpath='{range .items[*]}{.metadata.name}{"="}{.status.nodeInfo.containerRuntimeVersion}{"\n"}{end}' \ + 2>/dev/null || true)" + if [ -z "${runtimes}" ]; then + warn "could not read node runtime versions" + fi + while IFS= read -r line; do + [ -n "${line}" ] || continue + printf ' runtime: %s\n' "${line}" + printf 'RESULT cluster_runtime=%s\n' "${line#*=}" >> "${RAW_OUT}" + + case "${line#*=}" in + docker://*) + warn "node runs the docker runtime (cri-dockerd), which does not implement" + warn "ImageVolume at all -- the skill-mount rows will be unavailable." + ;; + cri-o://*) + # CRI-O added ImageVolume support in 1.31. + local ver="${line#*cri-o://}" + case "${ver}" in + 1.2*|1.30.*) warn "cri-o ${ver} predates ImageVolume support (needs >= 1.31)" ;; + esac + ;; + containerd://1.*) + warn "containerd 1.x does not implement ImageVolume (needs >= 2.0)" + ;; + esac + done </dev/null || true)" + if [ -z "${metrics}" ]; then + warn "could not read kubelet metrics; feature-gate state unknown" + return 0 + fi + + state="$(printf '%s\n' "${metrics}" \ + | grep '^kubernetes_feature_enabled{name="ImageVolume"' | head -1 || true)" + if [ -z "${state}" ]; then + printf ' ImageVolume: not reported by kubelet (likely GA or removed gate)\n' + return 0 + fi + + printf ' ImageVolume: %s\n' "${state}" + printf 'RESULT cluster_imagevolume_gate=%s\n' "${state}" >> "${RAW_OUT}" + case "${state}" in + *"} 0") warn "ImageVolume feature gate is DISABLED on the kubelet -- skill mounts will fail" ;; + esac +} + +# --------------------------------------------------------------- probe pod + +dump_failure_context() { + local pod=$1 + err "probe pod '${pod}' did not become ready; dumping context" + kube describe pod "${pod}" 2>&1 | sed 's/^/ /' || true + printf '\n --- recent events ---\n' + kube get events --sort-by=.lastTimestamp 2>&1 | tail -25 | sed 's/^/ /' || true + printf '\n --- security context / SCC ---\n' + kube get pod "${pod}" -o jsonpath='{.spec.securityContext}{"\n"}' 2>/dev/null | sed 's/^/ /' || true + kube get pod "${pod}" -o jsonpath='{.metadata.annotations.openshift\.io/scc}{"\n"}' 2>/dev/null | sed 's/^/ /' || true +} + +start_probe_pod() { + log "Creating standalone probe pod '${PROBE_POD}'" + kube delete pod "${PROBE_POD}" --ignore-not-found --wait=true >/dev/null 2>&1 || true + + # Pipe through sed rather than sed -i: BSD and GNU disagree on -i's argument. + sed -e "s|quay.io/konveyor/agentic-controller-agent:e2e|${AGENT_IMG}|" \ + -e "s|quay.io/konveyor/skills:maven-migration|${SKILL_IMG}|" \ + "${SCRIPT_DIR}/probe-pod.yaml" \ + | kube apply -f - >/dev/null + + log "Waiting for probe pod (up to ${WAIT_SECONDS}s)" + if ! kube wait --for=condition=Ready "pod/${PROBE_POD}" --timeout="${WAIT_SECONDS}s" >/dev/null 2>&1; then + dump_failure_context "${PROBE_POD}" + return 1 + fi +} + +# Deliver the probe over stdin. Nothing is baked into any image, so iterating on +# probe.sh needs no rebuild -- and this works identically against the standalone +# pod and a real Sandbox. +run_probe_in() { + local pod=$1 container=${2:-} label=$3 + local args=(exec -i "${pod}") + [ -n "${container}" ] && args+=(-c "${container}") + args+=(-- sh -s) + + log "Probing ${label} (pod/${pod})" + if ! kube "${args[@]}" < "${SCRIPT_DIR}/probe.sh" > "${RESULTS_DIR}/${label}.txt" 2>&1; then + warn "probe exec failed against ${label}; partial output retained" + fi + sed "s/^RESULT /RESULT ${label}./" "${RESULTS_DIR}/${label}.txt" >> "${RAW_OUT}" 2>/dev/null || true +} + +summarize() { + local label=$1 file="${RESULTS_DIR}/$1.txt" + [ -f "${file}" ] || return 0 + printf '\n %s\n' "${label}" + grep -E '^RESULT (verdict|remediation|staging_best|staging_exec_any|cluster_)' "${file}" 2>/dev/null \ + | sed 's/^RESULT / /' || true + printf ' --- per-directory ---\n' + grep -E '^RESULT stage_.*_(noexec|exec|in_worktree)=' "${file}" 2>/dev/null \ + | sed 's/^RESULT / /' || true + printf ' --- skill mount (informational) ---\n' + grep -E '^RESULT skill_.*_(noexec|ro|readable)=' "${file}" 2>/dev/null \ + | sed 's/^RESULT / /' || true +} + +main() { + mkdir -p "${RESULTS_DIR}" + RAW_OUT="${RESULTS_DIR}/probe-raw.txt" + : > "${RAW_OUT}" + + collect_cluster_facts + check_imagevolume_gate + + if start_probe_pod; then + run_probe_in "${PROBE_POD}" "" standalone + else + err "standalone probe unavailable" + fi + + # The real thing: whatever the controller actually built. + local sandbox + sandbox="$(kube get agentrun "${AGENT_RUN}" -o jsonpath='{.status.sandboxName}' 2>/dev/null || true)" + if [ -n "${sandbox}" ] && kube get pod "${sandbox}" >/dev/null 2>&1; then + run_probe_in "${sandbox}" agent sandbox + else + log "No Sandbox pod for AgentRun '${AGENT_RUN}' -- skipping the in-system probe." + log " (create an AgentRun, then re-run with AGENT_RUN=, to compare" + log " the standalone result against a real controller-created pod)" + fi + + printf '\n' + log "Results" + summarize standalone + summarize sandbox + + # Disagreement means the controller's pod shape differs from the hand-written + # one -- worth knowing before trusting either number. + if [ -f "${RESULTS_DIR}/standalone.txt" ] && [ -f "${RESULTS_DIR}/sandbox.txt" ]; then + local a b + a="$(grep '^RESULT verdict=' "${RESULTS_DIR}/standalone.txt" 2>/dev/null || true)" + b="$(grep '^RESULT verdict=' "${RESULTS_DIR}/sandbox.txt" 2>/dev/null || true)" + printf '\n' + if [ -n "${a}" ] && [ "${a}" = "${b}" ]; then + log "Standalone and Sandbox verdicts AGREE: ${a#RESULT verdict=}" + else + warn "Standalone and Sandbox verdicts DIFFER:" + warn " standalone: ${a#RESULT verdict=}" + warn " sandbox: ${b#RESULT verdict=}" + warn "The controller's mount construction differs from the hand-written pod." + fi + fi + + printf '\n raw output: %s\n' "${RAW_OUT}" + + if [ "${KEEP}" != "1" ]; then + kube delete pod "${PROBE_POD}" --ignore-not-found --wait=false >/dev/null 2>&1 || true + else + log "Keeping probe pod (PROBE_KEEP=1): kubectl -n ${PROBE_NS} exec -it ${PROBE_POD} -- sh" + fi +} + +main "$@"