Fix: Make contract execution safe and diagnostic

README.md

@@ -198,7 +198,7 @@ docs/           Settings reference, and the design specs behind the work
 
 ## Tests
 
-185 of them, under `tests/`. Run the lot, or a subset by pattern:
+186 of them, under `tests/`. Run the lot, or a subset by pattern:
 
 ```sh
 panama test           # everything

bin/panama

@@ -10,7 +10,7 @@
 #   edit       Open the Panama repo in Neovim
 #   doctor     Report what is actually running on this machine
 #   diagnose   Hand this machine's health and recent errors to your agent
-#   test       Run every contract under tests/ (--safe skips the hijacking ones)
+#   test       Run contract manifest entries under tests/ (--safe is hermetic only)
 #   contracts  Name the contracts that mention a given file
 #   upgrade    Re-run the installer from anywhere, interview included
 #   migrate    Apply repairs this machine has not had yet
@@ -87,10 +87,10 @@ ${BOLD}Commands:${RESET}
               Needs an agent chosen on Settings › System › Agents.
   ${GREEN}test${RESET}        Run every contract under tests/. Give it a pattern to run
               a subset: 'panama test dock' runs the ones matching 'dock'.
-              --safe skips the ones that take over the live desktop; what
+              --safe runs hermetic contracts only. External capabilities need
-              they are and why is tests/desktop-hijacking.
+              explicit --allow grants when automation invokes them.
-  ${GREEN}contracts${RESET}   Name the contracts that mention a given file, each marked
+  ${GREEN}contracts${RESET}   Name the contracts that mention a given file, each labeled
-              safe or desktop. A heuristic over the text of tests/, so it
+              with manifest capabilities. A heuristic over the text of tests/, so it
               answers "what should I run" rather than "what covers this".
   ${GREEN}upgrade${RESET}     Re-run ./install from anywhere, interview and all. For a new
               machine, or to change an answer you gave. Routine updates are
@@ -431,29 +431,43 @@ PROMPT
 }
 
 # ----------------------------------------------------------------------------
-# The desktop-hijacking ledger
+# Contract manifest
 # ----------------------------------------------------------------------------
 #
-# tests/desktop-hijacking lists, one repo-relative path per line with a '#'
+# The manifest is the runtime authority for every collected contract. An absent
-# comment saying what it does to the live session, the contracts that drive the
+# manifest is unsafe: this command must never infer that unclassified tests are
-# real shell, compositor or machine rather than a harness. Read by `test --safe`
+# hermetic.
-# to decide what to skip, and by `contracts` to mark each hit.
+CONTRACT_MANIFEST="tests/contracts.manifest"
-#
+CONTRACT_CAPABILITIES=(hermetic live-host live-compositor live-desktop network privileged)
-# Prints the paths, comments and blank lines stripped. A missing ledger prints
-# nothing: no ledger means nothing is known to hijack, which is the honest
-# reading of an absent file and keeps `--safe` from failing on a fresh checkout.
-DESKTOP_HIJACKING_LEDGER="tests/desktop-hijacking"
 
-hijacking_entries() {
+contract_manifest_entries() {
-  local ledger="$PANAMA_DIR/$DESKTOP_HIJACKING_LEDGER" line
+  local line capabilities path
-  [[ -r "$ledger" ]] || return 0
   while IFS= read -r line || [[ -n "$line" ]]; do
     line="${line%%#*}"
-    line="${line#"${line%%[![:space:]]*}"}"
+    read -r capabilities path _ <<<"$line"
-    line="${line%"${line##*[![:space:]]}"}"
+    [[ -n "${capabilities:-}" && -n "${path:-}" ]] || continue
-    [[ -n "$line" ]] && printf '%s\n' "$line"
+    printf '%s\t%s\n' "$path" "$capabilities"
-  done < "$ledger"
+  done < "$PANAMA_DIR/$CONTRACT_MANIFEST"
-  return 0
+}
+
+require_contract_manifest() {
+  [[ -r "$PANAMA_DIR/$CONTRACT_MANIFEST" ]] || {
+    err "Contract manifest is missing or unreadable: $PANAMA_DIR/$CONTRACT_MANIFEST"
+    return 1
+  }
+}
+
+test_usage() {
+  err "Usage: ${BOLD}$PROGRAM test [--safe] [--allow <capability>] [pattern]${RESET}"
+  return 2
+}
+
+is_contract_capability() {
+  local capability="$1" known
+  for known in "${CONTRACT_CAPABILITIES[@]}"; do
+    [[ "$capability" == "$known" ]] && return 0
+  done
+  return 1
 }
 
 # ----------------------------------------------------------------------------
@@ -472,93 +486,173 @@ hijacking_entries() {
 # open overlays, restart the shell, move your windows. Running the suite while
 # sitting in front of the machine used to mean losing the session for a few
 # minutes, so the honest options were "run everything" or "run nothing". --safe
-# is the third: skip exactly what tests/desktop-hijacking names, and say how
+# is the third: run only contracts the manifest classifies as hermetic, and say
-# many were skipped, so the gap is stated rather than implied.
+# which external capability coverage it skipped.
 cmd_test() {
-  local pattern="" safe=0 arg
+  local timeout_seconds="${PANAMA_TEST_TIMEOUT_SECONDS:-180}"
-  # Position-independent, because 'panama test --safe dock' and
+  [[ "$timeout_seconds" =~ ^[1-9][0-9]*$ ]] || {
-  # 'panama test dock --safe' are the same intent and nobody should have to
+    err 'PANAMA_TEST_TIMEOUT_SECONDS must be a positive integer.'
-  # remember which one this accepts.
+    return 2
-  for arg in "$@"; do
+  }
+
+  require_contract_manifest || return 1
+  cmd_test_impl "$timeout_seconds" "$@"
+}
+
+cmd_test_impl() (
+  local timeout_seconds="$1"
+  shift
+  local pattern="" safe=0 arg capability capabilities rel path
+  local -A grants=() manifest_capabilities=() skipped_counts=() missing_grants=() selected_capabilities=()
+  local -a suite=() missing_capability_list=() selected_capability_list=() capability_list=()
+
+  # Position-independent: flags can precede or follow the optional pattern.
+  while (( $# > 0 )); do
+    arg="$1"
+    shift
     case "$arg" in
       --safe) safe=1 ;;
-      *)      pattern="$arg" ;;
+      --allow)
+        (( $# > 0 )) || { test_usage; return 2; }
+        capability="$1"
+        shift
+        is_contract_capability "$capability" || {
+          err "Unknown contract capability: $capability"
+          return 2
+        }
+        [[ "$capability" != hermetic ]] || {
+          err 'hermetic contracts do not need --allow.'
+          return 2
+        }
+        grants["$capability"]=1
+        ;;
+      --*) test_usage; return 2 ;;
+      *)
+        [[ -z "$pattern" ]] || { test_usage; return 2; }
+        pattern="$arg"
+        ;;
     esac
   done
 
-  local -a suite=()
+  (( safe == 0 || ${#grants[@]} == 0 )) || {
-  local -A hijacking=()
+    err '--safe cannot be combined with --allow.'
-  local skipped=0 entry rel
+    return 2
+  }
 
-  if (( safe )); then
+  while IFS=$'\t' read -r rel capabilities; do
-    while IFS= read -r entry; do
+    manifest_capabilities["$rel"]="$capabilities"
-      hijacking["$entry"]=1
+    [[ -z "$pattern" || "$rel" == *"$pattern"* ]] || continue
-    done < <(hijacking_entries)
+    if (( safe )) && [[ "$capabilities" != hermetic ]]; then
-  fi
+      IFS=',' read -r -a capability_list <<<"$capabilities"
-
+      for capability in "${capability_list[@]}"; do
-  # Executables, plus the Python suites. Those are unittest files rather than
+        (( ++skipped_counts["$capability"] ))
-  # executables, and collecting only what has the executable bit would skip them
+      done
-  # without saying so -- which is how all three came to be run by nothing at all.
-  # A runner with a blind spot is worse than no runner, because it reports PASS.
-  while IFS= read -r path; do
-    [[ -x "$path" || "$path" == *_test.py ]] || continue
-    [[ -z "$pattern" || "$path" == *"$pattern"* ]] || continue
-    rel="tests/${path#"$PANAMA_DIR"/tests/}"
-    if (( safe )) && [[ -n "${hijacking[$rel]:-}" ]]; then
-      (( ++skipped ))
       continue
     fi
-    suite+=("$path")
+    suite+=("$rel")
-  done < <(find "$PANAMA_DIR/tests" -type f -not -path '*/fixtures/*' -not -path '*__pycache__*' | sort)
+  done < <(contract_manifest_entries)
 
   if (( ${#suite[@]} == 0 )); then
-    # "Nothing matched" and "everything that matched was skipped" are different
+    if (( safe )) && (( ${#skipped_counts[@]} > 0 )); then
-    # answers, and reporting the first for the second is how --safe would come
+      err "Every contract matching '${pattern}' needs an external capability; --safe skipped all of them."
-    # to look like a broken pattern.
-    if (( skipped > 0 )); then
-      err "Every contract matching '${pattern}' is desktop-hijacking; --safe skipped all ${skipped}."
-      printf '  What they do to the session: %s/%s\n' "$PANAMA_DIR" "$DESKTOP_HIJACKING_LEDGER" >&2
     else
       err "No contracts match '${pattern}'"
     fi
-    exit 1
+    return 1
   fi
 
+  if (( safe )); then
+    for capability in "${CONTRACT_CAPABILITIES[@]}"; do
+      [[ "$capability" == hermetic ]] && continue
+      printf 'Skipped %d %s contract(s).\n' "${skipped_counts[$capability]:-0}" "$capability"
+    done
+  else
+    for rel in "${suite[@]}"; do
+      capabilities="${manifest_capabilities[$rel]}"
+      [[ "$capabilities" == hermetic ]] && continue
+      IFS=',' read -r -a capability_list <<<"$capabilities"
+      for capability in "${capability_list[@]}"; do
+        selected_capabilities["$capability"]=1
+        [[ -n "${grants[$capability]:-}" ]] || missing_grants["$capability"]=1
+      done
+    done
+    for capability in "${CONTRACT_CAPABILITIES[@]}"; do
+      [[ "$capability" == hermetic ]] && continue
+      [[ -n "${selected_capabilities[$capability]:-}" ]] && selected_capability_list+=("$capability")
+      [[ -n "${missing_grants[$capability]:-}" ]] && missing_capability_list+=("$capability")
+    done
+    if (( ${#missing_capability_list[@]} > 0 )); then
+      if [[ -t 0 && -t 2 ]]; then
+        confirm "Run ${#suite[@]} contract(s) requiring: ${selected_capability_list[*]}?" || {
+          warn 'No contracts were run.'
+          return 1
+        }
+      else
+        err "Selected contracts require: ${missing_capability_list[*]}."
+        for capability in "${missing_capability_list[@]}"; do
+          printf '  Automation: pass --allow %s\n' "$capability" >&2
+        done
+        return 1
+      fi
+    fi
+  fi
+
+  local capture_dir stdout_file stderr_file name run_status index=0
+  local -a failed=() runner=()
+  capture_dir="$(mktemp -d)"
+  trap 'rm -rf -- "$capture_dir"' EXIT
+  trap 'rm -rf -- "$capture_dir"; exit 130' INT
+  trap 'rm -rf -- "$capture_dir"; exit 143' TERM
+
   info "Running ${#suite[@]} contract(s)"
-  local -a failed=()
+  for index in "${!suite[@]}"; do
-  local path name
+    rel="${suite[$index]}"
-  local -a runner
+    path="$PANAMA_DIR/$rel"
-  for path in "${suite[@]}"; do
+    name="${rel#tests/}"
-    name="${path#"$PANAMA_DIR"/tests/}"
+    stdout_file="$capture_dir/$index.stdout"
+    stderr_file="$capture_dir/$index.stderr"
     if [[ "$path" == *_test.py ]]; then
       runner=(python3 "$path")
     else
       runner=("$path")
     fi
-    if "${runner[@]}" >/dev/null 2>&1; then
+    run_status=0
+    timeout --signal=TERM --kill-after=5 "$timeout_seconds" \
+      "${runner[@]}" >"$stdout_file" 2>"$stderr_file" || run_status=$?
+    if (( run_status == 0 )); then
       ok "$name"
-    else
+      if [[ -s "$stderr_file" ]]; then
-      err "$name"
+        warn "$name wrote to stderr:"
-      failed+=("$name")
+        cat "$stderr_file" >&2
+      fi
+      continue
     fi
+
+    if (( run_status == 124 || run_status == 137 )); then
+      err "$name timed out after ${timeout_seconds}s"
+    else
+      err "$name failed (exit $run_status)"
+    fi
+    [[ -s "$stdout_file" ]] && {
+      printf '%s stdout:\n' "$name" >&2
+      cat "$stdout_file" >&2
+    }
+    [[ -s "$stderr_file" ]] && {
+      printf '%s stderr:\n' "$name" >&2
+      cat "$stderr_file" >&2
+    }
+    failed+=("$name")
   done
 
   header "Result"
   if (( ${#failed[@]} == 0 )); then
     ok "${#suite[@]} contract(s) passed"
-    if (( safe )); then
-      printf 'Skipped %d desktop-hijacking contract(s) (%s).\n' "$skipped" "$DESKTOP_HIJACKING_LEDGER"
-    fi
     return 0
   fi
   err "${#failed[@]} of ${#suite[@]} failed:"
   printf '    %s\n' "${failed[@]}" >&2
-  warn "Run one on its own to see why: ${BOLD}${PANAMA_DIR}/tests/<name>${RESET}"
-  if (( safe )); then
-    printf 'Skipped %d desktop-hijacking contract(s) (%s).\n' "$skipped" "$DESKTOP_HIJACKING_LEDGER"
-  fi
   return 1
-}
+)
 
 # ----------------------------------------------------------------------------
 # Command: contracts
@@ -577,8 +671,8 @@ cmd_test() {
 # a real answer -- exit 1 so a script can tell the difference -- but it is a
 # statement about this search, not about the file.
 #
-# Each hit is marked from tests/desktop-hijacking, so the output also answers
+# Each hit is labeled from tests/contracts.manifest, so the output also answers
-# "and can I run them right now".
+# which boundary the matching contract reaches.
 cmd_contracts() {
   local target="${1:-}"
   if [[ -z "$target" ]]; then
@@ -621,11 +715,12 @@ cmd_contracts() {
     suffix="${suffix#*/}"
   done
 
-  local -A hijacking=()
+  require_contract_manifest || return 1
-  local entry
+  local -A manifest_capabilities=()
-  while IFS= read -r entry; do
+  local capabilities
-    hijacking["$entry"]=1
+  while IFS=$'\t' read -r rel capabilities; do
-  done < <(hijacking_entries)
+    manifest_capabilities["$rel"]="$capabilities"
+  done < <(contract_manifest_entries)
 
   # The same collection `test` runs, so anything named here is something the
   # runner would actually execute.
@@ -643,11 +738,11 @@ cmd_contracts() {
   fi
 
   for rel in "${hits[@]}"; do
-    if [[ -n "${hijacking[$rel]:-}" ]]; then
+    [[ -n "${manifest_capabilities[$rel]:-}" ]] || {
-      printf '%s  [desktop]\n' "$rel"
+      err "Contract has no manifest capability label: $rel"
-    else
+      return 1
-      printf '%s  [safe]\n' "$rel"
+    }
-    fi
+    printf '%s  [%s]\n' "$rel" "${manifest_capabilities[$rel]}"
   done
 }
 

tests/contracts.manifest

@@ -272,6 +272,7 @@ hermetic tests/setup/projects-contract
 hermetic tests/setup/readme-contract
 hermetic tests/setup/role-contract
 hermetic tests/setup/skills-contract
+hermetic tests/setup/test-runner-contract
 hermetic tests/setup/update-command-contract
 hermetic tests/setup/user-content-contract
 hermetic tests/setup/webapp-contract

tests/setup/test-runner-contract

@@ -0,0 +1,192 @@
+#!/usr/bin/env bash
+
+# The public seam is the installed `panama` command. This fixture repository
+# proves the runner's manifest policy and diagnostics without touching the host.
+
+set -uo pipefail
+
+repo_dir="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)"
+fixture="$(mktemp -d)"
+output=""
+status=0
+
+cleanup() { rm -rf -- "$fixture"; }
+trap cleanup EXIT INT TERM
+
+fail() { printf 'test runner: %s\n' "$*" >&2; exit 1; }
+
+assert_contains() {
+    local needle="$1" haystack="$2"
+    [[ "$haystack" == *"$needle"* ]] || fail "expected output to contain: $needle\n$haystack"
+}
+
+assert_not_contains() {
+    local needle="$1" haystack="$2"
+    [[ "$haystack" != *"$needle"* ]] || fail "expected output not to contain: $needle\n$haystack"
+}
+
+assert_execution() {
+    local expected="$1" actual
+    actual="$(sort "$fixture/executions" 2>/dev/null || true)"
+    [[ "$actual" == "$expected" ]] || fail "expected executions '$expected', got '$actual'"
+}
+
+reset_executions() { : > "$fixture/executions"; }
+
+run_panama() {
+    output="$(cd "$fixture" && TMPDIR="$fixture" PANAMA_TEST_FIXTURE="$fixture" "$fixture/bin/panama" "$@" </dev/null 2>&1)"
+    status=$?
+}
+
+mkdir -p "$fixture/bin" "$fixture/tests" "$fixture/config"
+cp "$repo_dir/bin/panama" "$fixture/bin/panama"
+chmod +x "$fixture/bin/panama"
+touch "$fixture/config/subject"
+git -C "$fixture" init --quiet
+
+cat > "$fixture/tests/contracts.manifest" <<'EOF'
+# Maps the live desktop.
+live-desktop tests/desktop-contract
+hermetic tests/fail-contract
+hermetic tests/hang-contract
+# Reads a host fixture.
+live-host tests/host-contract
+# Contacts a fixture endpoint.
+network tests/network-contract
+hermetic tests/pass-contract
+hermetic tests/stderr-contract
+EOF
+
+cat > "$fixture/tests/pass-contract" <<'EOF'
+#!/usr/bin/env bash
+printf 'pass\n' >> "$PANAMA_TEST_FIXTURE/executions"
+printf 'pass stdout\n'
+# config/subject
+EOF
+
+cat > "$fixture/tests/fail-contract" <<'EOF'
+#!/usr/bin/env bash
+printf 'fail\n' >> "$PANAMA_TEST_FIXTURE/executions"
+printf 'failure stdout\n'
+printf 'failure stderr\n' >&2
+exit 7
+EOF
+
+cat > "$fixture/tests/stderr-contract" <<'EOF'
+#!/usr/bin/env bash
+printf 'stderr\n' >> "$PANAMA_TEST_FIXTURE/executions"
+printf 'warning on success\n' >&2
+EOF
+
+cat > "$fixture/tests/hang-contract" <<'EOF'
+#!/usr/bin/env bash
+printf 'hang\n' >> "$PANAMA_TEST_FIXTURE/executions"
+trap 'printf terminated >"$PANAMA_TEST_FIXTURE/terminated"; exit 124' TERM
+while :; do sleep 1; done
+EOF
+
+cat > "$fixture/tests/host-contract" <<'EOF'
+#!/usr/bin/env bash
+printf 'host\n' >> "$PANAMA_TEST_FIXTURE/executions"
+EOF
+
+cat > "$fixture/tests/desktop-contract" <<'EOF'
+#!/usr/bin/env bash
+printf 'desktop\n' >> "$PANAMA_TEST_FIXTURE/executions"
+# config/subject
+EOF
+
+cat > "$fixture/tests/network-contract" <<'EOF'
+#!/usr/bin/env bash
+printf 'network\n' >> "$PANAMA_TEST_FIXTURE/executions"
+EOF
+
+chmod +x "$fixture/tests"/{desktop,fail,host,network,pass,stderr}-contract
+: > "$fixture/executions"
+
+# --safe must select hermetic entries from the manifest, not merely omit a
+# legacy desktop list. The failing fixture makes the command nonzero, but all
+# three selected hermetic contracts still run and every other capability skips.
+run_panama test --safe
+[[ $status -ne 0 ]] || fail '--safe unexpectedly passed a failing fixture'
+assert_execution $'fail\npass\nstderr'
+assert_contains 'Skipped 1 live-host contract(s).' "$output"
+assert_contains 'Skipped 0 live-compositor contract(s).' "$output"
+assert_contains 'Skipped 1 live-desktop contract(s).' "$output"
+assert_contains 'Skipped 1 network contract(s).' "$output"
+
+reset_executions
+run_panama test desktop
+[[ $status -ne 0 ]] || fail 'non-TTY desktop run unexpectedly passed without a grant'
+assert_execution ''
+assert_contains 'pass --allow live-desktop' "$output"
+
+run_panama test --allow live-desktop desktop
+[[ $status -eq 0 ]] || fail "explicit desktop grant failed: $output"
+assert_execution 'desktop'
+
+reset_executions
+run_panama test --allow live-desktop --allow live-host host
+[[ $status -eq 0 ]] || fail "repeatable grants failed: $output"
+assert_execution 'host'
+
+reset_executions
+run_panama test --allow live-desktop network
+[[ $status -ne 0 ]] || fail 'desktop grant incorrectly allowed network'
+assert_execution ''
+assert_contains 'network' "$output"
+
+for args in '--unknown' 'pass-contract second-pattern' '--allow unknown' '--safe --allow live-desktop'; do
+    # shellcheck disable=SC2086
+    run_panama test $args
+    [[ $status -eq 2 ]] || fail "usage error did not exit 2 for: $args\n$output"
+done
+
+chmod +x "$fixture/tests/hang-contract"
+
+reset_executions
+output="$(cd "$fixture" && TMPDIR="$fixture" PANAMA_TEST_TIMEOUT_SECONDS=1 PANAMA_TEST_FIXTURE="$fixture" "$fixture/bin/panama" test hang </dev/null 2>&1)"
+status=$?
+[[ $status -ne 0 ]] || fail 'timed-out contract unexpectedly passed'
+assert_execution 'hang'
+[[ -f "$fixture/terminated" ]] || fail 'timed-out contract was not terminated with TERM'
+assert_contains 'timed out' "$output"
+
+reset_executions
+run_panama test fail
+[[ $status -ne 0 ]] || fail 'failed contract unexpectedly passed'
+assert_contains 'failure stdout' "$output"
+assert_contains 'failure stderr' "$output"
+
+reset_executions
+run_panama test stderr
+[[ $status -eq 0 ]] || fail "stderr success contract failed: $output"
+assert_contains 'warning on success' "$output"
+
+reset_executions
+run_panama test pass
+[[ $status -eq 0 ]] || fail "pass contract failed: $output"
+assert_not_contains 'pass stdout' "$output"
+
+reset_executions
+run_panama test --safe desktop
+[[ $status -ne 0 ]] || fail 'only-skipped pattern unexpectedly passed'
+assert_contains 'Every contract matching' "$output"
+assert_not_contains 'No contracts match' "$output"
+
+output="$(cd "$fixture" && "$fixture/bin/panama" contracts config/subject 2>&1)"
+status=$?
+[[ $status -eq 0 ]] || fail "contracts lookup failed: $output"
+assert_contains 'tests/desktop-contract  [live-desktop]' "$output"
+assert_contains 'tests/pass-contract  [hermetic]' "$output"
+
+mv "$fixture/tests/contracts.manifest" "$fixture/tests/contracts.manifest.missing"
+run_panama test pass
+[[ $status -ne 0 ]] || fail 'missing manifest unexpectedly allowed test execution'
+assert_contains 'contracts.manifest' "$output"
+mv "$fixture/tests/contracts.manifest.missing" "$fixture/tests/contracts.manifest"
+
+capture_dirs="$(find "$fixture" -mindepth 1 -maxdepth 1 -type d -name 'tmp.*' -print)"
+[[ -z "$capture_dirs" ]] || fail "runner leaked capture directory: $capture_dirs"
+
+printf 'test runner: PASS\n'