Fix: Close verification gate review findings
This commit is contained in:
@@ -9,9 +9,23 @@ repo_dir="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)"
|
||||
fixture="$(mktemp -d)"
|
||||
output=""
|
||||
status=0
|
||||
background_cli_pid=""
|
||||
background_contract_pgid=""
|
||||
|
||||
cleanup() { rm -rf -- "$fixture"; }
|
||||
trap cleanup EXIT INT TERM
|
||||
cleanup() {
|
||||
trap - EXIT INT TERM
|
||||
if [[ "$background_cli_pid" =~ ^[1-9][0-9]*$ ]]; then
|
||||
kill -TERM "$background_cli_pid" 2>/dev/null || true
|
||||
wait "$background_cli_pid" 2>/dev/null || true
|
||||
fi
|
||||
if [[ "$background_contract_pgid" =~ ^[1-9][0-9]*$ ]]; then
|
||||
kill -KILL -- "-$background_contract_pgid" 2>/dev/null || true
|
||||
fi
|
||||
rm -rf -- "$fixture"
|
||||
}
|
||||
trap cleanup EXIT
|
||||
trap 'cleanup; exit 130' INT
|
||||
trap 'cleanup; exit 143' TERM
|
||||
|
||||
fail() { printf 'test runner: %s\n' "$*" >&2; exit 1; }
|
||||
|
||||
@@ -25,6 +39,12 @@ assert_not_contains() {
|
||||
[[ "$haystack" != *"$needle"* ]] || fail "expected output not to contain: $needle\n$haystack"
|
||||
}
|
||||
|
||||
assert_before() {
|
||||
local first="$1" second="$2" haystack="$3"
|
||||
[[ "$haystack" == *"$first"*"$second"* ]] \
|
||||
|| fail "expected '$first' before '$second':\n$haystack"
|
||||
}
|
||||
|
||||
assert_execution() {
|
||||
local expected="$1" actual
|
||||
actual="$(sort "$fixture/executions" 2>/dev/null || true)"
|
||||
@@ -33,6 +53,33 @@ assert_execution() {
|
||||
|
||||
reset_executions() { : > "$fixture/executions"; }
|
||||
|
||||
wait_for_file() {
|
||||
local path="$1" attempt
|
||||
for (( attempt = 0; attempt < 100; attempt++ )); do
|
||||
[[ -s "$path" ]] && return 0
|
||||
sleep 0.05
|
||||
done
|
||||
return 1
|
||||
}
|
||||
|
||||
wait_for_process_exit() {
|
||||
local pid="$1" attempt
|
||||
for (( attempt = 0; attempt < 100; attempt++ )); do
|
||||
kill -0 "$pid" 2>/dev/null || return 0
|
||||
sleep 0.05
|
||||
done
|
||||
return 1
|
||||
}
|
||||
|
||||
wait_for_path_removal() {
|
||||
local path="$1" attempt
|
||||
for (( attempt = 0; attempt < 100; attempt++ )); do
|
||||
[[ ! -e "$path" ]] && return 0
|
||||
sleep 0.05
|
||||
done
|
||||
return 1
|
||||
}
|
||||
|
||||
run_panama() {
|
||||
output="$(cd "$fixture" && TMPDIR="$fixture" PANAMA_TEST_FIXTURE="$fixture" "$fixture/bin/panama" "$@" </dev/null 2>&1)"
|
||||
status=$?
|
||||
@@ -45,18 +92,39 @@ run_panama_with_timeout() {
|
||||
|
||||
run_panama_tty_default_no() {
|
||||
local command tty_stdout="$fixture/tty.stdout"
|
||||
local pty_state="$fixture/pty-state"
|
||||
mkdir -p "$pty_state"/{config,state,cache,data,runtime}
|
||||
chmod 700 "$pty_state/runtime"
|
||||
printf -v command 'cd %q && TMPDIR=%q PANAMA_TEST_FIXTURE=%q %q test composite > %q' \
|
||||
"$fixture" "$fixture" "$fixture" "$fixture/bin/panama" "$tty_stdout"
|
||||
output="$(python3 - "$command" <<'PY'
|
||||
output="$(
|
||||
HOME="$fixture/pty-home" \
|
||||
BASH_ENV="$fixture/pty-bash-env" \
|
||||
PANAMA_PTY_STARTUP_SENTINEL="$fixture/pty-startup-sourced" \
|
||||
python3 - "$command" "$fixture/pty-home" \
|
||||
"$pty_state/config" "$pty_state/state" "$pty_state/cache" \
|
||||
"$pty_state/data" "$pty_state/runtime" "$fixture" <<'PY'
|
||||
import errno
|
||||
import os
|
||||
import pty
|
||||
import sys
|
||||
|
||||
command = sys.argv[1]
|
||||
environment = os.environ.copy()
|
||||
environment.pop('BASH_ENV', None)
|
||||
environment.pop('ENV', None)
|
||||
environment.update({
|
||||
'HOME': sys.argv[2],
|
||||
'XDG_CONFIG_HOME': sys.argv[3],
|
||||
'XDG_STATE_HOME': sys.argv[4],
|
||||
'XDG_CACHE_HOME': sys.argv[5],
|
||||
'XDG_DATA_HOME': sys.argv[6],
|
||||
'XDG_RUNTIME_DIR': sys.argv[7],
|
||||
'TMPDIR': sys.argv[8],
|
||||
})
|
||||
pid, terminal = pty.fork()
|
||||
if pid == 0:
|
||||
os.execv('/bin/bash', ['bash', '-lc', command])
|
||||
os.execve('/bin/bash', ['bash', '--noprofile', '--norc', '-c', command], environment)
|
||||
|
||||
chunks = []
|
||||
replied = False
|
||||
@@ -78,7 +146,7 @@ _, child_status = os.waitpid(pid, 0)
|
||||
sys.stdout.buffer.write(b''.join(chunks))
|
||||
sys.exit(os.waitstatus_to_exitcode(child_status))
|
||||
PY
|
||||
)"
|
||||
)"
|
||||
status=$?
|
||||
}
|
||||
|
||||
@@ -88,6 +156,48 @@ assert_occurrences() {
|
||||
[[ "$actual" == "$expected" ]] || fail "expected $expected occurrence(s) of '$needle', got $actual\n$haystack"
|
||||
}
|
||||
|
||||
replace_manifest_line() {
|
||||
local original="$1" replacement="$2" line
|
||||
while IFS= read -r line || [[ -n "$line" ]]; do
|
||||
if [[ "$line" == "$original" ]]; then
|
||||
[[ "$replacement" == __REMOVE__ ]] || printf '%s\n' "$replacement"
|
||||
else
|
||||
printf '%s\n' "$line"
|
||||
fi
|
||||
done <<<"$valid_manifest"
|
||||
}
|
||||
|
||||
swap_manifest_contract_paths() {
|
||||
local line
|
||||
while IFS= read -r line || [[ -n "$line" ]]; do
|
||||
case "$line" in
|
||||
'live-compositor,live-desktop tests/composite-contract')
|
||||
printf '%s\n' 'live-compositor,live-desktop tests/desktop-contract'
|
||||
;;
|
||||
'live-desktop tests/desktop-contract')
|
||||
printf '%s\n' 'live-desktop tests/composite-contract'
|
||||
;;
|
||||
*) printf '%s\n' "$line" ;;
|
||||
esac
|
||||
done <<<"$valid_manifest"
|
||||
}
|
||||
|
||||
expect_manifest_rejection() {
|
||||
local label="$1" expected="$2" contents="$3"
|
||||
|
||||
printf '%s\n' "$contents" >"$fixture/tests/contracts.manifest"
|
||||
reset_executions
|
||||
run_panama test pass
|
||||
[[ $status -ne 0 ]] || fail "$label manifest unexpectedly allowed test execution"
|
||||
assert_contains "$expected" "$output"
|
||||
assert_execution ''
|
||||
|
||||
run_panama contracts config/subject
|
||||
[[ $status -ne 0 ]] || fail "$label manifest unexpectedly allowed contracts lookup"
|
||||
assert_contains "$expected" "$output"
|
||||
assert_execution ''
|
||||
}
|
||||
|
||||
mkdir -p "$fixture/bin" "$fixture/tests" "$fixture/config"
|
||||
cp "$repo_dir/bin/panama" "$fixture/bin/panama"
|
||||
chmod +x "$fixture/bin/panama"
|
||||
@@ -111,6 +221,8 @@ privileged tests/privileged-contract
|
||||
hermetic tests/stderr-contract
|
||||
EOF
|
||||
|
||||
valid_manifest="$(<"$fixture/tests/contracts.manifest")"
|
||||
|
||||
cat > "$fixture/tests/pass-contract" <<'EOF'
|
||||
#!/usr/bin/env bash
|
||||
printf 'pass\n' >> "$PANAMA_TEST_FIXTURE/executions"
|
||||
@@ -135,7 +247,13 @@ 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
|
||||
printf '%s\n' "$BASHPID" > "$PANAMA_TEST_FIXTURE/hang.pid"
|
||||
finish() {
|
||||
printf 'terminated\n' >"$PANAMA_TEST_FIXTURE/terminated"
|
||||
exit "$1"
|
||||
}
|
||||
trap 'finish 130' INT
|
||||
trap 'finish 143' TERM
|
||||
while :; do sleep 1; done
|
||||
EOF
|
||||
|
||||
@@ -147,6 +265,7 @@ EOF
|
||||
cat > "$fixture/tests/desktop-contract" <<'EOF'
|
||||
#!/usr/bin/env bash
|
||||
printf 'desktop\n' >> "$PANAMA_TEST_FIXTURE/executions"
|
||||
printf 'desktop fixture complete\n' >&2
|
||||
# config/subject
|
||||
EOF
|
||||
|
||||
@@ -172,6 +291,15 @@ chmod +x "$fixture/tests"/{composite,desktop,fail,hang,host,network,pass,privile
|
||||
# A PTY-backed default-no confirmation remains visible when stdout is redirected
|
||||
# but stdin and stderr are terminals. The fixture proves that one prompt gates
|
||||
# the selected composite capability set without running its contract.
|
||||
mkdir -p "$fixture/pty-home"
|
||||
for profile in .bash_profile .bashrc .profile; do
|
||||
cat >"$fixture/pty-home/$profile" <<'EOF'
|
||||
printf 'profile\n' >>"${PANAMA_PTY_STARTUP_SENTINEL:?}"
|
||||
EOF
|
||||
done
|
||||
cat >"$fixture/pty-bash-env" <<'EOF'
|
||||
printf 'BASH_ENV\n' >>"${PANAMA_PTY_STARTUP_SENTINEL:?}"
|
||||
EOF
|
||||
run_panama_tty_default_no
|
||||
[[ $status -ne 0 ]] || fail 'TTY default-no prompt unexpectedly ran the fixture'
|
||||
assert_execution ''
|
||||
@@ -179,6 +307,8 @@ assert_contains 'Run 1 contract(s) requiring: live-compositor live-desktop?' "$o
|
||||
assert_occurrences 'Run 1 contract(s) requiring:' "$output" 1
|
||||
assert_contains 'No contracts were run.' "$(<"$fixture/tty.stdout")"
|
||||
assert_not_contains 'Run 1 contract(s) requiring:' "$(<"$fixture/tty.stdout")"
|
||||
[[ ! -e "$fixture/pty-startup-sourced" ]] \
|
||||
|| fail 'PTY fixture sourced a shell profile or BASH_ENV'
|
||||
|
||||
# --safe must select hermetic entries from the manifest, not merely omit a
|
||||
# legacy desktop list. The failing and timed-out fixtures make the command
|
||||
@@ -204,6 +334,8 @@ 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'
|
||||
assert_contains 'Running desktop-contract [live-desktop]' "$output"
|
||||
assert_before 'Running desktop-contract [live-desktop]' 'desktop fixture complete' "$output"
|
||||
|
||||
reset_executions
|
||||
run_panama test --allow live-compositor --allow live-desktop composite
|
||||
@@ -229,6 +361,45 @@ assert_execution 'hang'
|
||||
[[ -f "$fixture/terminated" ]] || fail 'timed-out contract was not terminated with TERM'
|
||||
assert_contains 'timed out' "$output"
|
||||
|
||||
# INT/TERM ownership belongs to the exact public CLI PID, not a runner
|
||||
# subshell. The CLI must wait for the timeout process group and remove its
|
||||
# capture directory before returning the signal-derived status.
|
||||
reset_executions
|
||||
rm -f -- "$fixture/hang.pid" "$fixture/terminated"
|
||||
(
|
||||
cd "$fixture" || exit 1
|
||||
exec env TMPDIR="$fixture" PANAMA_TEST_FIXTURE="$fixture" \
|
||||
"$fixture/bin/panama" test hang
|
||||
) >"$fixture/exact-term.out" 2>&1 &
|
||||
background_cli_pid=$!
|
||||
wait_for_file "$fixture/hang.pid" \
|
||||
|| fail 'exact-PID TERM fixture never started the hang contract'
|
||||
hang_pid="$(<"$fixture/hang.pid")"
|
||||
background_contract_pgid="$(ps -o pgid= -p "$hang_pid" | tr -d '[:space:]')"
|
||||
[[ "$background_contract_pgid" =~ ^[1-9][0-9]*$ ]] \
|
||||
|| fail "could not resolve hang process group for PID $hang_pid"
|
||||
mapfile -t active_capture_dirs < <(
|
||||
find "$fixture" -mindepth 1 -maxdepth 1 -type d -name 'tmp.*' -print
|
||||
)
|
||||
(( ${#active_capture_dirs[@]} == 1 )) \
|
||||
|| fail "expected one active capture directory, got ${#active_capture_dirs[@]}"
|
||||
active_capture_dir="${active_capture_dirs[0]}"
|
||||
|
||||
kill -TERM "$background_cli_pid" \
|
||||
|| fail 'could not send TERM to the exact public CLI PID'
|
||||
term_status=0
|
||||
wait "$background_cli_pid" || term_status=$?
|
||||
background_cli_pid=""
|
||||
[[ "$term_status" -eq 143 ]] \
|
||||
|| fail "exact-PID TERM returned $term_status instead of 143: $(<"$fixture/exact-term.out")"
|
||||
wait_for_process_exit "$hang_pid" \
|
||||
|| fail "hang contract PID $hang_pid survived exact-PID TERM"
|
||||
background_contract_pgid=""
|
||||
wait_for_path_removal "$active_capture_dir" \
|
||||
|| fail "capture directory survived exact-PID TERM: $active_capture_dir"
|
||||
[[ -f "$fixture/terminated" ]] \
|
||||
|| fail 'exact-PID TERM did not reach the hang contract cleanup trap'
|
||||
|
||||
reset_executions
|
||||
run_panama test fail
|
||||
[[ $status -ne 0 ]] || fail 'failed contract unexpectedly passed'
|
||||
@@ -244,6 +415,19 @@ reset_executions
|
||||
run_panama test pass
|
||||
[[ $status -eq 0 ]] || fail "pass contract failed: $output"
|
||||
assert_not_contains 'pass stdout' "$output"
|
||||
assert_not_contains 'pass-contract [hermetic]' "$output"
|
||||
|
||||
reset_executions
|
||||
printf 'not a directory\n' >"$fixture/invalid-tmpdir"
|
||||
output="$(
|
||||
cd "$fixture" && \
|
||||
TMPDIR="$fixture/invalid-tmpdir" PANAMA_TEST_FIXTURE="$fixture" \
|
||||
"$fixture/bin/panama" test pass </dev/null 2>&1
|
||||
)"
|
||||
status=$?
|
||||
[[ $status -ne 0 ]] || fail 'invalid TMPDIR unexpectedly allowed contract execution'
|
||||
assert_contains 'Could not create contract capture directory.' "$output"
|
||||
assert_execution ''
|
||||
|
||||
reset_executions
|
||||
run_panama test --safe desktop
|
||||
@@ -258,10 +442,52 @@ assert_contains 'tests/desktop-contract [live-desktop]' "$output"
|
||||
assert_contains 'tests/composite-contract [live-compositor,live-desktop]' "$output"
|
||||
assert_contains 'tests/pass-contract [hermetic]' "$output"
|
||||
|
||||
# Both public manifest consumers fail closed on the complete format and
|
||||
# discovery set. Validation happens before selection, lookup, or contract
|
||||
# execution, so even a malformed entry unrelated to the requested pattern is
|
||||
# fatal and leaves the execution log empty.
|
||||
expect_manifest_rejection unknown-capability \
|
||||
'unknown capability hermetik on tests/pass-contract' \
|
||||
"$(replace_manifest_line 'hermetic tests/pass-contract' 'hermetik tests/pass-contract')"
|
||||
expect_manifest_rejection mixed-hermetic \
|
||||
'hermetic must appear alone on tests/pass-contract' \
|
||||
"$(replace_manifest_line 'hermetic tests/pass-contract' 'hermetic,network tests/pass-contract')"
|
||||
expect_manifest_rejection duplicate-path \
|
||||
'duplicate path tests/pass-contract' \
|
||||
"$(replace_manifest_line 'hermetic tests/pass-contract' $'hermetic tests/pass-contract\nhermetic tests/pass-contract')"
|
||||
expect_manifest_rejection stale-path \
|
||||
'stale manifest path tests/stale-contract' \
|
||||
"$(replace_manifest_line 'hermetic tests/pass-contract' 'hermetic tests/stale-contract')"
|
||||
expect_manifest_rejection missing-contract \
|
||||
'missing contract tests/pass-contract' \
|
||||
"$(replace_manifest_line 'hermetic tests/pass-contract' __REMOVE__)"
|
||||
expect_manifest_rejection extra-field \
|
||||
'manifest line is not exactly two fields' \
|
||||
"$(replace_manifest_line 'hermetic tests/pass-contract' 'hermetic tests/pass-contract unexpected')"
|
||||
expect_manifest_rejection empty-capability \
|
||||
'empty capability on tests/pass-contract' \
|
||||
"$(replace_manifest_line 'hermetic tests/pass-contract' 'hermetic, tests/pass-contract')"
|
||||
expect_manifest_rejection duplicate-capability \
|
||||
'duplicate capability hermetic on tests/pass-contract' \
|
||||
"$(replace_manifest_line 'hermetic tests/pass-contract' 'hermetic,hermetic tests/pass-contract')"
|
||||
expect_manifest_rejection unsorted-paths \
|
||||
'paths are not lexicographically sorted' \
|
||||
"$(swap_manifest_contract_paths)"
|
||||
expect_manifest_rejection uncommented-non-hermetic \
|
||||
'tests/desktop-contract is non-hermetic but lacks a directly preceding comment' \
|
||||
"$(replace_manifest_line '# Maps the live desktop.' __REMOVE__)"
|
||||
expect_manifest_rejection blank-comment \
|
||||
'tests/desktop-contract is non-hermetic but lacks a non-empty directly preceding comment' \
|
||||
"$(replace_manifest_line '# Maps the live desktop.' '#')"
|
||||
|
||||
printf '%s\n' "$valid_manifest" >"$fixture/tests/contracts.manifest"
|
||||
|
||||
mv "$fixture/tests/contracts.manifest" "$fixture/tests/contracts.manifest.missing"
|
||||
reset_executions
|
||||
run_panama test pass
|
||||
[[ $status -ne 0 ]] || fail 'missing manifest unexpectedly allowed test execution'
|
||||
assert_contains 'contracts.manifest' "$output"
|
||||
assert_execution ''
|
||||
mv "$fixture/tests/contracts.manifest.missing" "$fixture/tests/contracts.manifest"
|
||||
|
||||
capture_dirs="$(find "$fixture" -mindepth 1 -maxdepth 1 -type d -name 'tmp.*' -print)"
|
||||
|
||||
Reference in New Issue
Block a user