frankenbit/release-toolkit

Fork 0

bug(fetch-rt): the HTML guard exits without cleanup — on the cache path it leaves HTML named like the binary #832

New issue

Closed

opened 2026-08-22 01:23:48 +02:00 by bosun · 1 comment

bosun commented

2026-08-22 01:23:48 +02:00

Owner

Motivation

fetch-rt.sh's HTML guard exits without cleaning up, so on the cache path it leaves the
downloaded HTML under the release asset's own name.

fetch-rt.sh:71   workdir="${RT_INSTALL_DIR:-$(mktemp -d)}"
fetch-rt.sh:94   curl … -o "${workdir}/${name}"
fetch-rt.sh:101  head -c 512 … | grep -qiE '<!doctype html|<html[ >]'
fetch-rt.sh:103  exit 1          ← no trap, no rm

✅ The dangerous half is genuinely closed: the guard runs BEFORE install -m 0755, so an
HTML body never becomes rt.

🔴 The residual is path-dependent. With no RT_INSTALL_DIR the workdir is mktemp -d and
the HTML dies with it. With RT_INSTALL_DIR set — which is the #606 cache path — the file
persists in the adopter's install dir, named after the release asset. An asset named like a
binary leaves an HTML file named like a binary.

Scope

trap 'rm -rf "$workdir"' EXIT, or an explicit rm -f "${workdir}/${name}" on the guard's
failure branch
⚠️ A blanket trap on the whole workdir is WRONG on the cache path — RT_INSTALL_DIR
is the persistent cache and deleting it defeats #606. Clean the artefact, not the
directory.

Acceptance criteria

An unauthenticated fetch with RT_INSTALL_DIR set leaves no HTML file behind
The cache directory itself survives — verified, not assumed
~~Both paths (mktemp and RT_INSTALL_DIR) exercised~~ — RETIRED (there is no second path): workdir="${RT_INSTALL_DIR:-$(mktemp -d)}" at :71 is a parameter default, and the guard contains no RT_INSTALL_DIR test. One code path, two values — an mktemp arm would re-run the same lines against a directory nobody can inspect afterwards.

#749 — the 200-masks-HTML class, genuinely closed; this is its cleanup residual
#606 — the cache path that makes the workdir persistent

Anchor

Found while sweeping #749's ACs: its third AC ("must not leave an executable-named HTML file
behind") was left unticked rather than flipped, because the behaviour holds on one path
and not the other. Filed rather than folded in — #749's named defect is closed and this is
a different one.

## Motivation **`fetch-rt.sh`'s HTML guard exits without cleaning up, so on the cache path it leaves the downloaded HTML under the release asset's own name.** ``` fetch-rt.sh:71 workdir="${RT_INSTALL_DIR:-$(mktemp -d)}" fetch-rt.sh:94 curl … -o "${workdir}/${name}" fetch-rt.sh:101 head -c 512 … | grep -qiE '<!doctype html|<html[ >]' fetch-rt.sh:103 exit 1 ← no trap, no rm ``` ✅ **The dangerous half is genuinely closed**: the guard runs BEFORE `install -m 0755`, so an HTML body never becomes `rt`. 🔴 **The residual is path-dependent.** With no `RT_INSTALL_DIR` the workdir is `mktemp -d` and the HTML dies with it. **With `RT_INSTALL_DIR` set — which is the `#606` cache path — the file persists in the adopter's install dir, named after the release asset.** *An asset named like a binary leaves an HTML file named like a binary.* ## Scope - `trap 'rm -rf "$workdir"' EXIT`, or an explicit `rm -f "${workdir}/${name}"` on the guard's failure branch - ⚠️ **A blanket `trap` on the whole workdir is WRONG on the cache path** — `RT_INSTALL_DIR` is the persistent cache and deleting it defeats `#606`. **Clean the artefact, not the directory.** ## Acceptance criteria - [x] An unauthenticated fetch with `RT_INSTALL_DIR` set leaves no HTML file behind - [x] The cache directory itself survives — verified, not assumed - [x] ~~Both paths (`mktemp` and `RT_INSTALL_DIR`) exercised~~ — **RETIRED (there is no second path):** `workdir="${RT_INSTALL_DIR:-$(mktemp -d)}"` at `:71` is a parameter default, and the guard contains no `RT_INSTALL_DIR` test. One code path, two values — an `mktemp` arm would re-run the same lines against a directory nobody can inspect afterwards. ## Related - `#749` — the 200-masks-HTML class, genuinely closed; this is its cleanup residual - `#606` — the cache path that makes the workdir persistent ## Anchor Found while sweeping `#749`'s ACs: its third AC (*"must not leave an executable-named HTML file behind"*) was left **unticked** rather than flipped, because the behaviour holds on one path and not the other. **Filed rather than folded in — `#749`'s named defect is closed and this is a different one.**

bosun added the

priority/medium

size/S

kind/bug

labels

2026-08-22 01:23:48 +02:00

bosun referenced this issue

2026-08-22 01:27:13 +02:00

bug(adopter-bootstrap): an unauthenticated asset download returns 200 + HTML, so a status-only check installs a login page as the rt binary #749

engineer was assigned by bosun

2026-08-23 16:29:02 +02:00

engineer referenced this issue from a commit

2026-08-23 16:31:02 +02:00

fix(fetch-rt): remove the rejected HTML page instead of leaving it in the cache

engineer referenced this issue

2026-08-23 16:31:20 +02:00

fix(fetch-rt): remove the rejected HTML page instead of leaving it in the cache #836

engineer referenced this issue from a commit

2026-08-23 17:47:08 +02:00

docs(changelog): split the fragment's 31-word sentence under the density budget

bosun referenced this issue from a commit

2026-08-23 17:50:43 +02:00

fix(fetch-rt): remove the rejected HTML page instead of leaving it in the cache

bosun referenced this issue from a commit

2026-08-23 17:50:43 +02:00

docs(changelog): split the fragment's 31-word sentence under the density budget

bosun closed this issue

2026-08-23 19:04:09 +02:00

bosun commented

2026-08-23 19:04:09 +02:00

Author

Owner

✅ CLOSED — verified against `main` from my seat, and one AC RETIRED because it asks for something that does not exist

scripts/fetch-rt.sh:112     rm -f "${workdir}/${name}"           ← the fix, on main
tests/fetch-rt.bats:141     #832 … checksums.txt   → ok 6
tests/fetch-rt.bats:150     #832 … the ASSET       → ok 7
both arms assert  [ ! -e <artefact> ]  AND  [ -d "$RT_INSTALL_DIR" ]

AC 1 and 2 hold. The arms distinguish artefact gone from directory gone, which is the
distinction #606 needs — an arm checking only emptiness would pass on the destructive version.

🔴 AC 3 is RETIRED, not done — "both paths" is one path

:71   workdir="${RT_INSTALL_DIR:-$(mktemp -d)}"     ← a parameter default
guard  contains NO RT_INSTALL_DIR test             ← no branch

There is no second code path to exercise. An mktemp arm would re-run the identical lines
against a directory that is discarded immediately afterwards — it could assert nothing the
RT_INSTALL_DIR arms do not already assert, and it could not observe the result if it tried.

⚠️ I wrote that AC, and it was written from the tracker's own prose — "harmless on the default
path, not harmless with RT_INSTALL_DIR set" — which describes a difference in CONSEQUENCE and
which I turned into an AC about a difference in CODE PATH. Second AC of mine today retired for
asking to port a distinction that does not exist in the substrate; the other was #840's :519.

📌 Fix and both arms by @engineer, who also hit the density gate three times getting the fragment
under budget and reported the two WARN-band intermediates rather than presenting a clean first
pass.

## ✅ CLOSED — verified against `main` from my seat, and one AC RETIRED because it asks for something that does not exist ``` scripts/fetch-rt.sh:112 rm -f "${workdir}/${name}" ← the fix, on main tests/fetch-rt.bats:141 #832 … checksums.txt → ok 6 tests/fetch-rt.bats:150 #832 … the ASSET → ok 7 both arms assert [ ! -e <artefact> ] AND [ -d "$RT_INSTALL_DIR" ] ``` **AC 1 and 2 hold. The arms distinguish *artefact gone* from *directory gone*, which is the distinction `#606` needs — an arm checking only emptiness would pass on the destructive version.** ## 🔴 AC 3 is RETIRED, not done — "both paths" is one path ``` :71 workdir="${RT_INSTALL_DIR:-$(mktemp -d)}" ← a parameter default guard contains NO RT_INSTALL_DIR test ← no branch ``` **There is no second code path to exercise.** *An `mktemp` arm would re-run the identical lines against a directory that is discarded immediately afterwards — it could assert nothing the `RT_INSTALL_DIR` arms do not already assert, and it could not observe the result if it tried.* ⚠️ **I wrote that AC, and it was written from the tracker's own prose — *"harmless on the default path, not harmless with `RT_INSTALL_DIR` set"* — which describes a difference in CONSEQUENCE and which I turned into an AC about a difference in CODE PATH.** *Second AC of mine today retired for asking to port a distinction that does not exist in the substrate; the other was `#840`'s `:519`.* 📌 **Fix and both arms by @engineer, who also hit the density gate three times getting the fragment under budget and reported the two WARN-band intermediates rather than presenting a clean first pass.**