From 0c07b8269bb3426749251888f1b3a41b57b8e68b Mon Sep 17 00:00:00 2001 From: Rongxin Liu <10591665+rongxin-liu@users.noreply.github.com> Date: Tue, 29 Sep 2026 18:18:06 +0100 Subject: [PATCH 1/2] Bound the prompt hook's cost, and add a kill switch - Read the typescript bounded (first 64K + last 1M) instead of the whole file into a variable, so the prompt after a failed command no longer scales with how much it printed: 35 MB took 2.4 s, now 0.1 s, and 350 MB would have taken 24 s. - Run each helper under timeout (5 s, then SIGKILL), so a slow or stuck helper cannot stall the prompt; today's helpers can't block, but the framework accepts helpers in any language. - HELP50_DISABLED in the environment disables help50 at login and is reported by help50 is-enabled/status. Set as an organization-wide Codespaces secret, it turns help50 off for everyone at their next login without rebuilding an image; set by one user, it's a persistent personal opt-out. Smoke tests cover all three. --- etc/profile.d/help50.sh | 15 +++++++++++---- opt/cs50/bin/help50 | 8 +++++++- tests/smoke.sh | 37 +++++++++++++++++++++++++++++++++++++ 3 files changed, 55 insertions(+), 5 deletions(-) diff --git a/etc/profile.d/help50.sh b/etc/profile.d/help50.sh index 3d21308..03fcc3b 100644 --- a/etc/profile.d/help50.sh +++ b/etc/profile.d/help50.sh @@ -55,8 +55,15 @@ function _help50() { # https://tldp.org/LDP/abs/html/exitcodes.html if [[ $status -ne 0 && $status -ne 130 && $status -ne 148 ]]; then - # Read typescript from disk - local typescript=$(cat $HELP50) + # Read typescript from disk, bounded: at most the first 64K (where the command line is + # echoed) and the last 1M (where errors tend to be), so that a program that printed a + # great deal before failing doesn't stall the prompt while the whole file is read + local typescript + if [[ $(stat -c %s "$HELP50" 2> /dev/null || echo 0) -gt $((65536 + 1048576)) ]]; then + typescript=$(head -c 65536 "$HELP50"; echo; echo "[... output omitted ...]"; tail -c 1048576 "$HELP50") + else + typescript=$(cat "$HELP50") + fi # Remove script's own output (if this is user's first command) typescript=$(echo "$typescript" | sed '1{/^Script started on .*/d}') @@ -110,10 +117,10 @@ function _help50() { typescript="$after_first" fi - # Try to get help + # Try to get help, giving each helper a few seconds at most, lest a slow or stuck helper stall the prompt for helper in $HELPERS/*; do if [[ -f $helper && -x $helper ]]; then - local help=$($helper $argv <<< "$typescript") + local help=$(timeout -k 1 5 $helper $argv <<< "$typescript") if [[ -n "$help" ]]; then break fi diff --git a/opt/cs50/bin/help50 b/opt/cs50/bin/help50 index 2bf77d3..e1c9615 100755 --- a/opt/cs50/bin/help50 +++ b/opt/cs50/bin/help50 @@ -5,7 +5,13 @@ function _disable() { } function _is-enabled() { - if [[ -f /tmp/help50.lock ]]; then + + # Kill switch: set in the environment (e.g., an organization-wide Codespaces secret) to + # turn help50 off for everyone at their next login, without rebuilding an image + if [[ -n "$HELP50_DISABLED" ]]; then + echo "disabled (HELP50_DISABLED is set)" + return 1 + elif [[ -f /tmp/help50.lock ]]; then echo disabled return 1 else diff --git a/tests/smoke.sh b/tests/smoke.sh index c8511cf..e89f54e 100755 --- a/tests/smoke.sh +++ b/tests/smoke.sh @@ -54,6 +54,43 @@ run "$IMAGE" bash --login -c ' test "$(cat /tmp/cmd)" = ./slow || exit 1 ' +echo "- the prompt hook stays fast after a failed command printed a huge amount of output" +run "$IMAGE" bash --login -c ' + export HELP50=$(mktemp) + _helpless() { printf "%s" "$1" > /tmp/output; } + . /etc/profile.d/help50.sh + + # 35 MB (4 million lines) of output, then an error, as script(1) records it. + # Reading the whole file into a variable took ~2.4 s here and scaled linearly. + { printf "$ ./huge\r\n"; seq 1 4000000 | sed "s/$/\r/"; printf "Error: boom\r\n"; } > "$HELP50" + size=$(stat -c %s "$HELP50") + set -o history; history -s ./huge; set +o history + start=$(date +%s%N); false; _help50; elapsed=$(( ($(date +%s%N) - start) / 1000000 )) + echo " hook took ${elapsed} ms for a ${size}-byte typescript" + test "$elapsed" -lt 1000 && + test "$(tail -n 1 /tmp/output)" = "Error: boom" || exit 1 +' + +echo "- a helper that hangs cannot stall the prompt" +run --user root "$IMAGE" bash --login -c ' + printf "#!/bin/bash\ncat > /dev/null\nsleep 60\n" > /opt/cs50/lib/help50/zz_hang && chmod 755 /opt/cs50/lib/help50/zz_hang + su ubuntu -c "bash --login -c '"'"' + export HELP50=\$(mktemp); . /etc/profile.d/help50.sh + printf \"\$ ./x\\r\\nsome error\\r\\n\" > \"\$HELP50\" + set -o history; history -s ./x; set +o history + start=\$(date +%s); false; _help50; elapsed=\$(( \$(date +%s) - start )) + echo \"hook took \${elapsed} s with a hung helper\"; test \"\$elapsed\" -lt 15 + '"'"'" +' + +echo "- HELP50_DISABLED in the environment keeps help50 from starting, and says so" +run "$IMAGE" bash --login -c 'help50 is-enabled | grep -qx enabled' +run --env HELP50_DISABLED=1 "$IMAGE" bash --login -c ' + out=$(help50 is-enabled); test $? -eq 1 && [[ "$out" == *HELP50_DISABLED* ]] || exit 1' +# In an interactive shell on a pty (script provides one), help50 starts by default but not when disabled +run "$IMAGE" bash -c 'echo "help50 status; exit" | script -qc "bash --login -i" /dev/null' | grep -q '^started' +run --env HELP50_DISABLED=1 "$IMAGE" bash -c 'echo "help50 status; exit" | script -qc "bash --login -i" /dev/null' | grep -q '^stopped' + echo "- help50 COMMAND runs COMMAND, with its exit status" run "$IMAGE" bash --login -c 'help50 true && ! help50 false && test "$(help50 echo x)" = x' run "$IMAGE" bash --login -c 'help50 valgrind python x.py < /dev/null; test $? -eq 1' 2>&1 | grep -q 'does not support Python' From 888add19ae0e8685015e8e176721ccb2c5cf41d5 Mon Sep 17 00:00:00 2001 From: Rongxin Liu <10591665+rongxin-liu@users.noreply.github.com> Date: Tue, 29 Sep 2026 18:51:23 +0100 Subject: [PATCH 2/2] Treat false-y HELP50_DISABLED as unset, and document ctl-c under timeout - HELP50_DISABLED=0 (or false, no, off, case-insensitively) now counts as unset, so that an admin who sets the org secret to 0 to turn help50 back on gets what they asked for, rather than every student staying disabled with no error. The is-enabled message now shows the value and says to unset it. Smoke tests cover the false-y values and that the lock file is still honored when the environment doesn't disable. - Note in the prompt hook that timeout runs each helper in its own process group, so ctl-c no longer reaches a stuck helper; the timeout itself is the bound. --foreground would restore ctl-c but stop timeout from killing the helper's children, which would give back the hang this is meant to remove. --- etc/profile.d/help50.sh | 6 +++++- opt/cs50/bin/help50 | 16 +++++++++++----- tests/smoke.sh | 8 +++++++- 3 files changed, 23 insertions(+), 7 deletions(-) diff --git a/etc/profile.d/help50.sh b/etc/profile.d/help50.sh index 03fcc3b..13684c8 100644 --- a/etc/profile.d/help50.sh +++ b/etc/profile.d/help50.sh @@ -117,7 +117,11 @@ function _help50() { typescript="$after_first" fi - # Try to get help, giving each helper a few seconds at most, lest a slow or stuck helper stall the prompt + # Try to get help, giving each helper a few seconds at most, lest a slow or stuck helper stall + # the prompt. Note that timeout runs the helper in its own process group, so ctl-c at the + # terminal no longer reaches the helper (it did before); the timeout itself is the bound. + # Not --foreground, which would restore ctl-c but stop timeout from killing the helper's + # children, so an orphaned child holding stdout open could stall the prompt indefinitely. for helper in $HELPERS/*; do if [[ -f $helper && -x $helper ]]; then local help=$(timeout -k 1 5 $helper $argv <<< "$typescript") diff --git a/opt/cs50/bin/help50 b/opt/cs50/bin/help50 index e1c9615..f296249 100755 --- a/opt/cs50/bin/help50 +++ b/opt/cs50/bin/help50 @@ -7,11 +7,17 @@ function _disable() { function _is-enabled() { # Kill switch: set in the environment (e.g., an organization-wide Codespaces secret) to - # turn help50 off for everyone at their next login, without rebuilding an image - if [[ -n "$HELP50_DISABLED" ]]; then - echo "disabled (HELP50_DISABLED is set)" - return 1 - elif [[ -f /tmp/help50.lock ]]; then + # turn help50 off for everyone at their next login, without rebuilding an image. Values + # that read as false (0, false, no, off) count as unset, so that setting the secret to 0 + # re-enables help50 just as deleting it would, rather than silently keeping it off + case "${HELP50_DISABLED,,}" in + ""|0|false|no|off) ;; + *) + echo "disabled (HELP50_DISABLED=$HELP50_DISABLED; unset it to re-enable)" + return 1 + ;; + esac + if [[ -f /tmp/help50.lock ]]; then echo disabled return 1 else diff --git a/tests/smoke.sh b/tests/smoke.sh index e89f54e..3431339 100755 --- a/tests/smoke.sh +++ b/tests/smoke.sh @@ -86,7 +86,13 @@ run --user root "$IMAGE" bash --login -c ' echo "- HELP50_DISABLED in the environment keeps help50 from starting, and says so" run "$IMAGE" bash --login -c 'help50 is-enabled | grep -qx enabled' run --env HELP50_DISABLED=1 "$IMAGE" bash --login -c ' - out=$(help50 is-enabled); test $? -eq 1 && [[ "$out" == *HELP50_DISABLED* ]] || exit 1' + out=$(help50 is-enabled); test $? -eq 1 && [[ "$out" == *HELP50_DISABLED=1* && "$out" == *unset* ]] || exit 1' +# Values that read as false count as unset, so that setting the secret to 0 re-enables help50, as deleting it would +for value in 0 false FALSE no off ""; do + run --env HELP50_DISABLED="$value" "$IMAGE" bash --login -c 'help50 is-enabled | grep -qx enabled' +done +# The lock file (help50 disable) is still honored when the environment doesn't disable +run --env HELP50_DISABLED=0 "$IMAGE" bash --login -c 'help50 disable && ! help50 is-enabled && help50 enable && help50 is-enabled' > /dev/null # In an interactive shell on a pty (script provides one), help50 starts by default but not when disabled run "$IMAGE" bash -c 'echo "help50 status; exit" | script -qc "bash --login -i" /dev/null' | grep -q '^started' run --env HELP50_DISABLED=1 "$IMAGE" bash -c 'echo "help50 status; exit" | script -qc "bash --login -i" /dev/null' | grep -q '^stopped'