stop the driver hanging on a terminal and on a launcher
ai-sandbox-autophase could sit forever, printing nothing, in two places that are both before the first unit -- which is why both looked like a script that "does nothing". The rc probe ran `timeout 15 bash -ic` in front of the user's terminal. An interactive bash calls tcsetpgrp() to take that terminal, is in its background, and is stopped for trying; a stopped process does not take SIGTERM, so the timeout waited in front of it and the driver waited on the command substitution behind it. It now runs in its own session (setsid), with stdin closed, under `timeout -k`, and reads the definition from a temp file rather than a pipe, because a grandchild an rc file leaves behind holds a pipe's EOF open long after the shell it came from has been reaped. A launcher is called *with* the run, and a function ending at `--env NAME` -- the shape everyone writes for typing by hand, including every example this repo printed -- discards the flags and the in-container command. The container then starts the image's default harness with no prompt and, with --no-interactive gone, the terminal attached: an agent waiting for keystrokes behind a pipe nobody reads. `ap_forwards_args` checks the captured definition, --status and the preflight name it with the one-line fix, and every child now starts with stdin on /dev/null so a launcher cannot put the sandbox back on the keyboard. Aliases and whole command lines need no forwarding: the shell appends the words to those. Tests: a launcher fixture that drops its arguments, one that leaves a process behind, and shape assertions on ap_probe_rc. LLM-Generated: true
This commit is contained in:
@@ -51,6 +51,9 @@ sandbox-pi() {
|
||||
printf 'LAUNCH argc=%s cmd=%s\n' "\${#argv0[@]}" "\${cmd[0]:-none}"
|
||||
local a
|
||||
for a in "\${argv0[@]}"; do printf 'A:%s\n' "\$a"; done
|
||||
# what this child was started with on stdin: the driver closes it, so an agent inside
|
||||
# the sandbox cannot end up reading the terminal of an unattended run
|
||||
printf 'STDIN:%s\n' "\$(readlink /proc/self/fd/0)"
|
||||
} >> "\$trace"
|
||||
(( \${#cmd[@]} )) || return 0
|
||||
"\${cmd[@]}"
|
||||
@@ -59,6 +62,25 @@ EOF
|
||||
cat > "$WORK/rc/aliases.bash" <<'EOF'
|
||||
alias sandbox-alias='sandbox-pi'
|
||||
EOF
|
||||
# The launcher exactly as it is often written for interactive use -- a fixed policy, and
|
||||
# nothing after it. It works when *you* type its name, and drops the whole in-container
|
||||
# command when something appends one, which is every run the driver makes. Kept as a
|
||||
# fixture so the driver is checked to name it instead of hanging on it.
|
||||
cat > "$WORK/rc/stubby.bash" <<'EOF'
|
||||
stubby-pi() {
|
||||
export AI_API_TOKEN='CANARY-STUBBY-4f2c81'
|
||||
ai-sandbox --allow api.example.com --env AI_API_TOKEN
|
||||
}
|
||||
EOF
|
||||
# An rc file that leaves a grandchild behind, the way one that starts an agent or a
|
||||
# helper daemon does. The definition probe must not wait for it: reading a definition is
|
||||
# not a conversation with the file's process tree.
|
||||
cat > "$WORK/rc/daemon.bash" <<'EOF'
|
||||
daemon-pi() {
|
||||
sleep 10 &
|
||||
ai-sandbox "$@"
|
||||
}
|
||||
EOF
|
||||
cat > "$WORK/rc/broken.bash" <<'EOF'
|
||||
broken-sandbox() {
|
||||
local -a cmd=()
|
||||
@@ -147,6 +169,7 @@ GTRACE="$WORK/git.trace"
|
||||
MARKER="$WORK/token.marker"
|
||||
PROMPTS="$WORK/prompts.log"
|
||||
: > "$TRACE"; : > "$GTRACE"; : > "$MARKER"; : > "$PROMPTS"
|
||||
printf 'the driver stdin, which no child is allowed to see\n' > "$WORK/stdin.marker"
|
||||
DEF_RC="$WORK/rc/sandbox.bash:$WORK/rc/aliases.bash:$WORK/rc/00-nothing.bash"
|
||||
|
||||
# A prefix assignment in front of a shell function sticks around for the rest of the
|
||||
@@ -156,8 +179,11 @@ ap_run() { # [args...]
|
||||
local cmd="${T_CMD-sandbox-pi}" rc="${T_RC-$DEF_RC}" lib="${T_LIB-$LIB_FIX}"
|
||||
local validate="${T_VALIDATE-sandbox}" sargs="${T_SANDBOX_ARGS---no-tty --no-interactive}"
|
||||
local push="${T_PUSH-1}" pre="${T_PREFLIGHT-1}" notes="${T_NOTES-1}" gfail="${T_GITFAIL-}"
|
||||
local ppush="${T_PHASE_PUSH-}" dryout="${T_DRYOUT-}" plant="${T_PLANT-}"
|
||||
unset T_CMD T_RC T_LIB T_VALIDATE T_SANDBOX_ARGS T_PUSH T_PREFLIGHT T_NOTES T_GITFAIL T_PHASE_PUSH T_DRYOUT T_PLANT
|
||||
local ppush="${T_PHASE_PUSH-}" dryout="${T_DRYOUT-}" plant="${T_PLANT-}" stdin_src="${T_STDIN-$WORK/stdin.marker}"
|
||||
unset T_CMD T_RC T_LIB T_VALIDATE T_SANDBOX_ARGS T_PUSH T_PREFLIGHT T_NOTES T_GITFAIL T_PHASE_PUSH T_DRYOUT T_PLANT T_STDIN
|
||||
# stdin is a file with a known name, never the test runner's own: the launcher records
|
||||
# what it was started with, and that is how "a child never inherits the terminal" is
|
||||
# checked. A driver that forwards stdin would show up as this path in the trace.
|
||||
OUT="$(env "PATH=$WORK/bin:$PATH" \
|
||||
"AISB_AP_LIB=$lib" "AISB_AP_RC_FILES=$rc" "AISB_AP_PI_CMD=$cmd" \
|
||||
"AISB_AP_VALIDATE=$validate" "AISB_AP_SANDBOX_ARGS=$sargs" \
|
||||
@@ -166,7 +192,7 @@ ap_run() { # [args...]
|
||||
"FAKE_PI_PROMPTS=$PROMPTS" "REAL_GIT=$REAL_GIT" "PHASE_NOTIFY=0" \
|
||||
${gfail:+"FAKE_GIT_FAIL=$gfail"} ${ppush:+"PHASE_PUSH=$ppush"} ${dryout:+"AISB_AP_DRYOUT=$dryout"} \
|
||||
${plant:+"FAKE_PI_PLANT_HOOK=$plant"} \
|
||||
"$APBIN" "$@" 2>&1)" && RC=0 || RC=$?
|
||||
"$APBIN" "$@" 2>&1 < "$stdin_src")" && RC=0 || RC=$?
|
||||
return 0
|
||||
}
|
||||
gt() { grep -c "^$1" "$GTRACE" 2>/dev/null || true; } # count of git calls by verb
|
||||
@@ -210,6 +236,67 @@ T_CMD='' ap_run --status
|
||||
assert_rc 1 "an explicitly empty launcher is an error, not a silent fallback"
|
||||
assert_contains "no sandbox launcher" "and says so"
|
||||
|
||||
# Extract one function from the driver itself, so a test asks the real code a question
|
||||
# instead of restating its answer (the way aisb_fn() does for bin/ai-sandbox).
|
||||
ap_fn() { # <name>
|
||||
awk -v fn="$1" '$0 ~ "^" fn "\\(\\) \\{" {f=1} f {print} f && /^\}$/ {exit}' "$APBIN"
|
||||
}
|
||||
|
||||
t_name "a launcher that drops its arguments is named, and never hangs a run"
|
||||
# The shape every interactive sandbox-pi takes: a fixed policy and no "$@". Appended to,
|
||||
# it runs the image's default harness with no prompt, so an unattended run of the real
|
||||
# thing sat in front of an agent waiting for keystrokes, behind a pipe that showed
|
||||
# nothing. The driver has to say so, in the preview and before the first unit.
|
||||
: > "$TRACE"
|
||||
T_CMD=stubby-pi T_RC="$WORK/rc/stubby.bash" ap_run --status
|
||||
assert_rc 0 "--status with a launcher that takes no arguments"
|
||||
assert_contains "drops the arguments appended to it" "the preview says so"
|
||||
assert_contains "End the function with" "and says how to fix it"
|
||||
assert_absent "CANARY-STUBBY" "without repeating what the function exports"
|
||||
assert_absent "export AI_API_TOKEN" "and without printing its body"
|
||||
T_CMD=stubby-pi T_RC="$WORK/rc/stubby.bash" ap_run --max-units 1
|
||||
assert_rc 1 "a run with it is refused, not left waiting"
|
||||
assert_contains "preflight failed" "at the preflight, before any unit"
|
||||
assert_contains "ignores its arguments" "with the reason named"
|
||||
assert_contains 'ai-sandbox --allow ... --env NAME "$@"' "and the fix written out"
|
||||
if grep -q 'cmd=pi' "$TRACE"; then _bad "no unit runs"; else _ok "no unit runs"; fi
|
||||
# the fixture launcher in the tests forwards "\$@", and must not be nagged about it
|
||||
ap_run --status
|
||||
assert_absent "drops the arguments" "a launcher that forwards them is not flagged"
|
||||
|
||||
t_name "reading the rc files cannot be stopped, and cannot be held open"
|
||||
# An interactive bash started from a terminal takes that terminal: it calls tcsetpgrp(),
|
||||
# gets SIGTTOU because it is in the background, and stops -- and a stopped process does
|
||||
# not take SIGTERM, so the probe waited on it forever with the driver waiting behind it.
|
||||
# Everything a run does starts in this function, previews included, so the whole script
|
||||
# looked wedged with nothing printed. The three defences are asserted here by shape;
|
||||
# stopping a process is not something a test can arrange without a terminal of its own.
|
||||
probe_src="$(ap_fn ap_probe_rc)"
|
||||
for want in 'AP_SETSID' '< /dev/null' 'timeout -k 5 15' 'mktemp' '> "$tmp"'; do
|
||||
if printf '%s' "$probe_src" | grep -qF -- "$want"; then
|
||||
_ok "the probe is bounded, file-backed and off the terminal ($want)"
|
||||
else
|
||||
_bad "the probe is bounded, file-backed and off the terminal" "missing: $want"
|
||||
fi
|
||||
done
|
||||
if grep -qF 'AP_SETSID=(setsid)' "$APBIN"; then
|
||||
_ok "and its own session, so it has no controlling terminal to be stopped by"
|
||||
else
|
||||
_bad "and its own session, so it has no controlling terminal to be stopped by" "no AP_SETSID=(setsid) in $APBIN"
|
||||
fi
|
||||
# and the consequence a user would feel: an rc file that leaves a process behind must not
|
||||
# be able to hold the read open. sleep 10 in the fixture is the stand-in for the agent or
|
||||
# helper daemon a real rc file starts; if the probe reads a pipe instead of a file, this
|
||||
# test does not fail fast -- it fails 10 seconds later, which is the whole point.
|
||||
: > "$TRACE"
|
||||
start=$SECONDS
|
||||
T_CMD=daemon-pi T_RC="$WORK/rc/daemon.bash" ap_run --status
|
||||
elapsed=$(( SECONDS - start ))
|
||||
assert_rc 0 "--status with an rc file that backgrounds a process"
|
||||
assert_contains "function, from $WORK/rc/daemon.bash" "resolved all the same"
|
||||
(( elapsed < 8 )) && _ok "without waiting for what it left behind (${elapsed}s)" \
|
||||
|| _bad "without waiting for what it left behind" "took ${elapsed}s"
|
||||
|
||||
t_name "a child is the launcher, then --, then the harness command"
|
||||
: > "$TRACE"
|
||||
PROJ_A="$WORK/pa"; mkproj "$PROJ_A" 1 >/dev/null
|
||||
@@ -232,6 +319,15 @@ if grep -q '^LAUNCH argc=[0-9]* cmd=pi$' "$TRACE"; then
|
||||
else
|
||||
_bad "pi is the in-container command" "$(grep '^LAUNCH' "$TRACE" | head -3)"
|
||||
fi
|
||||
# Every child starts with stdin on /dev/null, whatever this test's own stdin is: the
|
||||
# flag that normally keeps podman off the terminal (--no-interactive) is a flag, and a
|
||||
# launcher is free to drop it.
|
||||
if grep -q '^STDIN:/dev/null$' "$TRACE"; then
|
||||
_ok "the child's stdin is /dev/null, not the caller's"
|
||||
else
|
||||
_bad "the child's stdin is /dev/null, not the caller's" "$(grep '^STDIN' "$TRACE" | head -3)"
|
||||
fi
|
||||
if grep -qF "stdin.marker" "$TRACE"; then _bad "the run's own stdin never reaches a child"; else _ok "the run's own stdin never reaches a child"; fi
|
||||
argc="$(grep '^LAUNCH argc=.* cmd=pi$' "$TRACE" | head -1 | sed 's/.*argc=\([0-9]*\).*/\1/')"
|
||||
[[ -n "$argc" && "$argc" -ge 8 ]] && _ok "the whole harness command line reached the launcher ($argc arguments)" \
|
||||
|| _bad "the whole harness command line reached the launcher" "argc=$argc"
|
||||
|
||||
Reference in New Issue
Block a user