test: isolate release rehearsal output

[armutlutost]

Summary

Make the release rehearsal contract test use a private temporary directory and explicitly verify the command status and captured output. This prevents an unwritable fixed /tmp path from being mistaken for a successful refusal check.

Related Issue

Closes #431

Validation

• Tests added or updated
• Documentation updated if behavior changed
• Security impact considered
• Trust boundary preserved (daemon remains the only privileged executor)
• CI passes

Notes for Reviewers

The refusal assertion is unchanged. The test now cleans up its temporary output with a trap and fails clearly when the rehearsal command cannot write its output.

[vladimirrott]

Reviewed at 1dea30947c70b5ef6b0069e2d64a3685f1623e6c. Thank you for this, and welcome.

The -s check is the part that earns the change. Moving to mktemp -d stops the fixed path being pre-owned, and that alone would have closed #431 on paper. Asserting that the capture is non-empty, and printing the exit status when it is not, is what makes the next failure of this shape loud instead of silent. Capturing publish_status and testing it separately rather than leaning on the if is the same instinct: the status and the output are two facts, and the old code let one of them stand in for both.

maintainer screen returns DO NOT EXECUTE for a diff the gates run directly, so everything below ran in a container with the network off.

Clean, at your head:

$ podman run --rm --network=none -v "$PWD:/repo:z" -v "$HOME/.cargo:/cargo:O" -v "$HOME/.rustup:/rustup:O" -w /repo -e HOME=/tmp -e CARGO_HOME=/cargo -e RUSTUP_HOME=/rustup -e CARGO_NET_OFFLINE=true -e PATH=/cargo/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin localhost/sk-reh2:1 bash -c 'bash tests/release/release-rehearsal.test.sh; echo "rc=$?"'
Release rehearsal contract passed.
rc=0

Then the scenario #431 describes, against the real release_rehearsal.sh rather than a re-enactment of its shape. I staged the fixed path owned by somebody else, mode 444, holding the string the old assertion greps for, and mounted that directory as /tmp:

$ P=/home/entropia/.local/state/sysknife-maint/poisontmp; mkdir -p "$P"; printf 'rehearsal never publishes\n' > "$P/sysknife-rehearsal-publish.out"; chmod 777 "$P"; chmod 444 "$P/sysknife-rehearsal-publish.out"; ls -l "$P"
total 4
-r--r--r-- 1 entropia entropia 26 sep 15 11:36 sysknife-rehearsal-publish.out

main's copy of the test, run as an unprivileged uid against that /tmp:

$ podman run --rm --network=none --user 1000:1000 -v "$PWD:/repo:ro" -v /home/entropia/.local/state/sysknife-maint/poisontmp:/tmp:z -v "$HOME/.cargo:/cargo:O" -v "$HOME/.rustup:/rustup:O" -w /repo -e HOME=/repo -e CARGO_HOME=/cargo -e RUSTUP_HOME=/rustup -e CARGO_NET_OFFLINE=true -e PATH=/cargo/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin localhost/sk-reh2:1 bash -c 'id -u; ls -l /tmp/sysknife-rehearsal-publish.out; bash tests/release/release-rehearsal.test.sh; echo "rc=$?"'
1000
-r--r--r-- 1 root root 26 Sep 15 17:36 /tmp/sysknife-rehearsal-publish.out
tests/release/release-rehearsal.test.sh: line 38: /tmp/sysknife-rehearsal-publish.out: Permission denied
Release rehearsal contract passed.
rc=0

release_rehearsal.sh --publish was never invoked, and the contract test reported that it correctly refuses to publish. Your branch, same container, same poisoned /tmp:

Release rehearsal contract passed.
rc=0

which is a pass the run earned, because mktemp -d never touches the name somebody else owns.

Then the part I care about most, whether the new assertion can fail. I dropped the 2>&1 from your redirect, so the command still runs and the capture comes back empty:

$ git diff --stat
 tests/release/release-rehearsal.test.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
ERROR: release rehearsal never publishes; use the protected tag workflow
FAIL: rehearsal output was not captured (exit 2)
rc=1

It names the failure and carries the exit status. shellcheck --severity=warning tests/release/release-rehearsal.test.sh exits 0.

Approving.

Your checks are waiting on me, not on you

$ gh pr checks 435 --repo lacs-project/sysknife
no checks reported on the 'fix/431-private-rehearsal-output' branch

GitHub holds a first-time contributor's workflow runs until somebody with write access presses "Approve and run". That is a control I keep rather than automate, so the delay is mine. The same applies to #436. Nothing is wrong with your branch.

One line will need to move, and not because of anything you did

@vsolano9's #432 is approved and touches the same file, adding its own trap 'rm -rf "$pin_fixture"' EXIT at what becomes line 161. Yours is at 39. The two merge with no conflict:

$ git merge-tree --write-tree p432 p435
2d3ab157b24e79bf0710a86662c84677394ad431
$ git show 2d3ab157:tests/release/release-rehearsal.test.sh | grep -n 'trap\|mktemp'
38:tmp_dir="$(mktemp -d)"
39:trap 'rm -rf "$tmp_dir"' EXIT
160:pin_fixture="$(mktemp -d)"
161:trap 'rm -rf "$pin_fixture"' EXIT

Bash keeps one EXIT trap per signal. The second replaces the first, so the merged file stops cleaning up your directory and both boards stay green:

$ bash /tmp/trapdemo.sh > /tmp/trapout.txt; cat /tmp/trapout.txt; for d in $(sed 's/^[ab]=//' /tmp/trapout.txt); do [ -d "$d" ] && echo "SURVIVED: $d" || echo "removed:  $d"; done
a=/tmp/tmp.24yM4OPfvx
b=/tmp/tmp.vqjaMeTF35
SURVIVED: /tmp/tmp.24yM4OPfvx
removed:  /tmp/tmp.vqjaMeTF35

Whichever of the two lands second folds both paths into one cleanup. I am sequencing that, and I will tell you if it falls to you. Change nothing now.

Next

Once the run is approved and green, this merges as it stands.

You have two open at once already, so I am not going to hand you a third. When these land, tell me whether you would rather keep going on documentation that contradicts the binary or move to the release gates, and I will hold something in whichever direction you pick.

Last thing, an invitation rather than a condition. Somebody who reads a /tmp path and sees an ownership problem is administering machines where that reasoning pays. SysKnife's read-only surface is the cheap way to try it: sysknife doctor, or sysknife "show me failed services", both of which plan and preview before touching anything. If you run it on a box you own, I would rather hear what it got wrong than not hear.

[vladimirrott]

Your checks are running. They had been sitting at action_required since you opened this, which is the hold GitHub puts on a first contribution until someone with write access releases it, and releasing it was mine to do rather than yours.

I also brought the branch up to date with main, so the head is now 105787ee and the run is against a tree that matches what a merge would produce. Nothing in your diff changed; the only file this PR touches is tests/release/release-rehearsal.test.sh.

My approval from earlier still stands. Nothing outstanding from you on this one.