Serve a first boot page that loads nothing from a third party #14
No reviewers
Labels
No labels
accessibility
bug
documentation
duplicate
enhancement
good first issue
help wanted
invalid
question
wontfix
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
Keel-Linux/inithooks!14
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "fix/no-third-party-script"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
The page the appliance serves on first boot, before anybody has logged in,
loaded three scripts from two other hosts. It now loads none.
Closes #13.
What was there
Measured on the
wordpress-democontainer, 2026-09-28, on the page that isactually served (
/var/lib/inithooks/turnkey-init-fence/htdocs/index.html):Two are the ones the issue names, appended by
29tagidat first boot. Thethird is not in the issue: a jQuery from
ajax.googleapis.com, in thepackaged page itself, that nothing on the page uses: the inline script
below it is plain
document.getElementById. It was there to support theremote script. That is exactly why the test parses the page for every host
it would fetch from instead of grepping for a hostname: a hostname grep is
what let this one sit next to the two that were noticed.
What the tag was for, established before removing it
The issue asked for this, so it was settled first rather than after.
Introduced in 2015 (
52e7c8e, "include script support for AJAX updates")and extracted into
29tagida week later (dc9fd92):.jslet TurnKey change the fence page's content after an imagehad shipped, without shipping anything;
.directis a beacon. It has no content to return; requesting itis the point. It told TurnKey the fence page had been hit directly, broken
down by build, version and appliance: which is the same triple the URL
path announces.
The same pair was once applied to the web control panel's index as well
(
webcp/$build/$version/$app.js); that half was deleted in 2016(
50101fc).Both endpoints answer 404 today:
So what every Keel appliance shipped was a beacon to a service that no
longer exists, and a page that is unverifiable by construction:
keel verifycan attest to every byte in the layer and say nothing about ascript fetched at view time.
What depended on it
Only itself, which is what the issue asked to establish up front.
tagid_versionandtagid_buildexisted solely to fill in thoseURLs. Both are gone, with
tagid_is_taggedandtagid_render_scripts.tagid_buildwas the only reader of/etc/apt/apt.conf.d/01turnkeyin this repository: it parsed the build tags out of the apt User-Agent.
Keel-Linux/common#6 removes that file, so the two changes fit together;
a test asserts the hook is fine without it.
bin/secalerts.shreads the same file, independently, for the versionit POSTs to
hub.turnkeylinux.org. It is guarded(
[ -e "$f" ] && ...) and falls back toturnkey-version. Measured thatit survives the file's removal and does not trip its own
set -e. It isuntouched here; that it talks to the Hub at all is a separate subject.
tagid_app_namestays. The@APP_NAME@substitution is the hook'sother job and is not the tag.
29tagidstill does it and still reloadsthe fence.
Why this is not a pure deletion
HTDOCSis/var/lib/inithooks/turnkey-init-fence/htdocs, and/varsurvives.
29tagidused to copy the packaged pages only when that directorydid not exist, so an index tagged by an older image stays tagged, and this
hook is the only thing that ever looks at it again. The measurement above is
that page.
The first version of this branch stripped remote
<script>elements fromit with a regular expression. Review measured an unquoted
src, astylesheet, an image and a frame getting past it. So
29tagidno longerjudges the page at all: on every run, under
turnkey-inittoo, it rebuildsthe
/varcopy from the packaged pages beside the served directory, namesthe appliance in it, and swaps it in. A page the package did not ship is
not served, however it is written. Nothing in the tree writes into that
copy but this hook. If the rebuild fails, the copy is removed and the fence
serves the packaged pages (
fence_htdocs), and the hook exits 1 with a<3>message. The commit message has the three part justification for thechange of approach.
How it is asserted
Not by reading the hook's source, and not by grepping for
ajax.turnkeylinux.org. Every page the fence serves is judged by twodetectors that share no parser:
tests/remote_loads.py(HTMLParser, everyattribute and style construct that fetches, 48 cases of its own) and a tag
level regular expression that knows no HTML and catches what HTMLParser
misses on malformed markup. Each hostile page the review measured, and two
parser differentials, are served as inherited pages and must be gone.
Tests
tests/test-tagid.bats: 27 tests; against the first version of the branch,8 fail. 125 bats in all,
29tagid29/29,lib/tagid.sh1/1, total 99.55;Python 196 passed, 3 skipped.
Changelog:
2.3.6+keel8, after #19 (keel6) and #18 (keel7).Noticed, not changed
The page still carries
<a href>links towww.turnkeylinux.org(theappliance product page and the contact page) and calls the appliance a
TurnKey one in its prose. Those are branding and vocabulary, not a script
and not a fetch (nothing is requested until an operator clicks), so they
are out of this issue's scope and left for the vocabulary work.
Test plan
tests/coverage.sh: six files, both touched ones at 100 percentpytest: 187 passed, 3 skipped, unchangedshellcheck -S warningon the hook and the library (the library'sSC2148 is pre-existing and matches
lib/ipconfig.shandlib/init-fence.sh, which are sourced the same way)ajax.turnkeylinux.orgendpoints probed, 404bin/secalerts.shchecked against a missing01turnkeyhost's
core.tar.zstis in a known-inconsistent state and a repair ispending, so the build lock was deliberately not taken. The measurement
above was taken read-only with
lxc-attach.Fits with Keel-Linux/common#6, which removes the
01turnkeythis hookstopped reading. Org-wide context: Keel-Linux/tracker#15.
Review of
fix/no-third-party-script. Findings ordered by severity, each says whether I got it by running something or by reading. No CRITICAL and no HIGH.Checked first, because the branch rests on them
Both endpoints are gone.
core.js,core.direct,wordpress.jsandwordpress.directunderhttps://ajax.turnkeylinux.org/initfence/iso/19.0-trixie-amd64/all answer 404. Confirmed.The strip cannot truncate the page. This is the claim I most wanted to fail, because
docs/traps.mdhas a gpg entry that is exactly this shape, and perl does not check write errors on many paths. It holds. Driven against/dev/full:perl reports a short write and exits non-zero on both the streaming path and the flush path, so
firstboot.d/29tagid:37-38takes theelsebranch, removes the temp file and leaves the index untouched. The redirection also targets$INITFENCE_INDEX.tmp, never the index, so even a failure before perl starts cannot touch it. Test 9 covers the perl-exits-1 case; the short-write case is the one that would have bitten and it behaves the same.The survival-across-upgrade reasoning is right.
HTDOCSis/var/lib/inithooks/turnkey-init-fence/htdocs(default/turnkey-init-fence),29tagid:22-25copies the packaged page only when that directory is absent, andfab-apply-overlayonly ever adds files. So an index tagged by an older image does survive and this hook is the only thing that looks at it again. Deleting the generator alone would not have been enough.bin/secalerts.shsurvives. Lines 53-55 are[ -e "$f" ] && turnkey_version=$(...)followed by[ -n "$turnkey_version" ] || turnkey_version=$(turnkey-version). Under#!/bin/bash -ea failing&&list does not terminate the script — confirmed by running the shape — so with01turnkeygone it falls through toturnkey-version, which is still installed. It is guarded and it does survive.27/27 of
tests/test-tagid.batspass here on bats 1.11.1, and the suite usesrun !throughout, so its refutations really do refute.MEDIUM —
lib/tagid.sh:25andtests/test-tagid.bats:56-64: the stripper and the detector share a blind spot, so the test cannot catch the stripper's gapThe strength claimed for the test is that it parses the page for every host it would fetch from rather than grepping a hostname — which is what caught the jQuery. Two things narrow that in practice, and they narrow it in the same direction, so nothing fails.
Driving the function directly:
The
<link>,<img>and<iframe>cases are by design — the function is named for scripts. Butloaded_hosts()at line 56 is what asserts the broader property, and it matches only double-quoted attributes ((src|href)[[:space:]]*=[[:space:]]*"[^"]+"). So the one form the stripper misses is also the one form the detector cannot see, and the three element types the stripper does not handle are only ever tested on pages that do not contain them: the inherited-page test at line 96 usesscript_hosts, notloaded_hosts, and test 2 runs on a fresh tree.The scenario: an image whose
/varindex came from a build that carried a remote<link rel="stylesheet">or a remote<img>— a rebranded fence page, a downstream edit, or simply the next thing somebody appends to that page for the same reason TurnKey appended the scripts. It survives the strip, it serves from the fence before anybody has logged in, and the suite stays green. The fix is small in both places: widen the element set the strip handles, or rename the function and say plainly that scripts are the only class defended; and makeloaded_hosts()accept'...'and bare attribute values so the detector is strictly wider than the defence rather than exactly as narrow.Nothing that has ever actually been written to that page slips through — the two generated tags and the packaged jQuery were all double-quoted
<script src>, and I checked all three are stripped. That is why this is MEDIUM and not higher.LOW —
lib/tagid.sh:25: a remote script with a body leaves its body behind as page textThe optional
</script>in the pattern only matches when it follows the opening tag immediately. The fetch is removed, which is the point, so this is cosmetic — but the residue lands in the rendered page as visible text and an orphan closing tag, on the first thing an operator sees.LOW —
firstboot.d/29tagid:6: the strip never runs underturnkey-init[ -n "$_TURNKEY_INIT" ] && exit 0is inherited and correct for the@APP_NAME@half. It now also means the new defence is skipped on exactly the path an operator takes when the first boot did not complete — anddocs/traps.mdentry 1 is a first boot that hangs forever, hit three times in this project. On that machine the fence is up, the index in/varis the tagged one, and runningturnkey-initdoes not strip it. Worth either moving the strip above the early exit or saying in the comment why it does not need to be.LOW —
firstboot.d/29tagid:35: the temporary file is written inside the directory the fence serves$HTDOCS/index.html.tmpis underHTDOCS, andbin/simplehttpd.pyserves that directory statically. During the write, and permanently if the hook is killed between the redirect and themv,/index.html.tmpis fetchable. The content is the stripped page so nothing leaks, andrm -fcovers the ordinary failure — a kill is what it does not cover.mktempin the parent directory, or a name the server will not serve, costs nothing.LOW — an already-initialised machine keeps its tagged page
The strip runs from a first-boot hook, so a machine that has already completed init and is then upgraded to
2.3.6+keel6keeps the two<script src>tags in/varfor good. Impact is small — the fence is disabled by97turnkey-init-fence-disableand both endpoints 404 — but "the first boot page carries no third party script" indebian/changelogis true of new machines only, and the changelog is where someone will look for the answer.Noted, not a finding
.directbeacon reading is right: no content to return, the request is the payload, and the URL path carries build, version and appliance.<a href>links towww.turnkeylinux.orgleft in the page are correctly out of scope — I confirmed nothing is requested until a click.tests/test-tagid.batsusesrun !correctly throughout. The rest of this repository does not: 20 bare negations in other suites assert nothing, 17 of them intests/test-ipconfig.bats, where 11 of the 13 rejection cases ofipconfig_ip6_syntaxare inert — an IPv6 validator whose negative tests do not run, in a project with adocs/traps.mdentry about IPv6 literals. Out of scope here, list handed over separately, wants its own issue.Approve. The removal is complete for everything that page has ever carried, the inherited-copy problem was found rather than assumed, and the temp-file path holds up under the failure mode that would actually have caused the trap it cites. The MEDIUM is that the guarantee stated is wider than the regex delivers and the test cannot tell the difference; I would widen the detector at least, so the next remote element added to that page fails something.
Review addressed in
7ab1826ande314934, then master merged in (c3371c8). 29tagid no longer strips anything. It rebuilds the /var copy from the packaged pages next to the served directory, runs under turnkey-init as well, and falls back to the packaged pages if it fails. The commit message has the justification. Two detectors that share no parser now judge the page. The changelog is now keel7, so this lands after #18.