FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

feat(apply): enforced the update denylist in the writer itself · bauer-group/XPD-RPIImage@c83a09c · GitHub

Repository navigation

Commit c83a09c

Browse files
committed
feat(apply): enforced the update denylist in the writer itself
Two guards that the updater will depend on, both placed so they cannot be forgotten rather than documented so they can be remembered. * BGRPI_DENY_GLOBS in apply-lib.sh, checked inside bg_install - the one function through which every module writes. Identity and access (shadow, passwd, sudoers, pam), operator-owned state (the 05- overrides our own helper writes, wpa_supplicant PSKs, rfkill saved state, static resolv.conf, hostname) and the two files where a typo is an unbootable device rather than a failed service (cmdline.txt, fstab). Device context only: /etc/hostname is exactly what bgrpiimage-base is for at build time. * _validate_apply_contract in generate.py asserts every rendered payload file is referenced by its module's apply.sh. A renderer that starts emitting a file nobody installs ships it to every device in /opt/bgrpiimage where it never takes effect, and the symptom appears weeks later as "we shipped that fix, why is the bus still dead" - which is how the SamplePoint and TransmitQueueLength bugs presented. Deviates from the plan, deliberately. The plan called for a per-module apply.manifest.json carrying files, modes, classes and on_change actions. That is a second description of what apply.sh already says, kept in sync by hand - the exact duplication that produced the CAN constant drift between generate.py and bgrpiimage-setup. Enforcing the denylist inside the writer is strictly stronger than describing it in data a module could omit, and a textual reference check gets the coverage assertion for none of the upkeep. The PLATFORM/SITE split the manifest was to carry is expressed as the denylist: SITE keys are simply not writable on a device. Adds tests/test-apply-guards.sh, because every one of these is a negative. Negatives rot silently - a refactor that drops a condition passes every other test in the suite, since nothing it does is wrong, it just stops declining. 27 assertions covering each denied path, the same paths staying writable at build time, the paths we do own staying writable on a device, mask being build-only, and dry-run writing nothing. The contract check was negative-tested too: a stray rendered file is reported, and the clean tree is not.
1 parent 5502fc8 commit c83a09c

5 files changed

Lines changed: 213 additions & 1 deletion

File tree

‎.github/workflows/build.yml‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,13 @@ jobs:
137137
python scripts/generate.py config/variants/canbus-plattform.json >/dev/null
138138
bash tests/test-apply-parity.sh
139139
140+
- name: 🧪 Apply guard test
141+
# Every guard in apply-lib.sh is a negative - "this must NOT
142+
# happen" - and negatives rot silently: a refactor that drops a
143+
# condition passes every other test in the suite, because nothing
144+
# it does is wrong, it just stops declining.
145+
run: bash tests/test-apply-guards.sh
146+
140147
- name: 🧪 Apply idempotence test
141148
# Applies every module twice against a scratch root in device
142149
# context and asserts the second run changes nothing and queues no

‎Makefile‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ render: ## resolve env vars + render generated module files
4141
$(PY) scripts/generate.py $(CONFIG) $(if $(ENV_FILE),--env-file $(ENV_FILE),)
4242

4343
.PHONY: test
44-
test: test-selfupdate test-parity test-idempotence ## run all tests (needs docker)
44+
test: test-selfupdate test-parity test-idempotence test-guards ## run all tests (needs docker)
4545

4646
.PHONY: test-selfupdate
4747
test-selfupdate: ## exercise `bgrpiimage-setup update --self`
@@ -55,6 +55,10 @@ test-parity: render ## assert apply.sh still produces what the chroot scripts di
5555
test-idempotence: render ## assert applying a release twice is a no-op
5656
bash tests/test-apply-idempotence.sh
5757

58+
.PHONY: test-guards
59+
test-guards: ## assert the apply-lib guards actually refuse
60+
bash tests/test-apply-guards.sh
61+
5862
.PHONY: bootstrap
5963
bootstrap: ## clone/update CustomPiOS into ./CustomPiOS
6064
bash scripts/bootstrap.sh

‎scripts/generate.py‎

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1762,6 +1762,51 @@ def _semantic_validate(cfg: dict[str, Any]) -> None:
17621762
)
17631763

17641764

1765+
def _validate_apply_contract(cfg: dict[str, Any]) -> list[str]:
1766+
"""Assert every rendered payload file is actually installed by its module.
1767+
1768+
The failure this catches is quiet and expensive: a renderer starts
1769+
emitting a file, nobody adds it to apply.sh, and the file rides along in
1770+
/opt/bgrpiimage on every device without ever reaching the path that makes
1771+
it do something. Nothing errors. The setting simply has no effect, and the
1772+
symptom shows up weeks later as "we shipped that fix, why is the bus still
1773+
dead" - which is exactly how the SamplePoint and TransmitQueueLength bugs
1774+
presented before them.
1775+
1776+
Deliberately a textual reference check rather than a manifest. A manifest
1777+
would be a second description of the same thing, kept in sync by hand -
1778+
the duplication that produced the CAN constant drift between generate.py
1779+
and bgrpiimage-setup. Here the apply script stays the single description,
1780+
and this only asserts it mentions everything the renderer produced.
1781+
"""
1782+
problems: list[str] = []
1783+
for module in ACTIVE_MODULES:
1784+
if not _module_enabled(module, cfg):
1785+
continue
1786+
apply_sh = MODULES_DIR / module / "apply.sh"
1787+
if not apply_sh.exists():
1788+
continue
1789+
body = apply_sh.read_text(encoding="utf-8")
1790+
gen = MODULES_DIR / module / "filesystem" / "root" / "opt" / "bgrpiimage" / module
1791+
if not gen.is_dir():
1792+
continue
1793+
for path in sorted(gen.rglob("*")):
1794+
if not path.is_file():
1795+
continue
1796+
rel = path.relative_to(gen)
1797+
if rel.as_posix() == "apply.sh":
1798+
continue # staged by the generator, not consumed by the script
1799+
# A file may be named outright, or covered by its directory when
1800+
# the module mirrors a whole tree with bg_install_tree.
1801+
names = {rel.as_posix(), path.name}
1802+
parent = rel.parent.as_posix()
1803+
if parent != ".":
1804+
names.add(parent)
1805+
if not any(n in body for n in names):
1806+
problems.append(f"{module}: renders {rel.as_posix()} but apply.sh never mentions it")
1807+
return problems
1808+
1809+
17651810
def _window_minutes(start_hhmm: str, end_hhmm: str) -> int:
17661811
"""Minutes between two HH:MM timestamps, wrapping past midnight if needed."""
17671812
def to_min(s: str) -> int:
@@ -1988,6 +2033,19 @@ def main() -> int:
19882033

19892034
render_variant_config(resolved)
19902035
console.print(table)
2036+
2037+
# After rendering, not before: the check compares what was actually
2038+
# produced against what the apply scripts reference.
2039+
contract_problems = _validate_apply_contract(resolved)
2040+
if contract_problems:
2041+
_error_panel(
2042+
"apply contract violated",
2043+
"\n".join(contract_problems),
2044+
"a rendered file that no apply.sh installs reaches devices but "
2045+
"never takes effect - add it to the module's apply.sh, or stop "
2046+
"rendering it",
2047+
)
2048+
return 1
19912049
console.print(
19922050
f" [green]{total}[/] artifact{'s' if total != 1 else ''} written to "
19932051
f"[dim]src/modules/*/filesystem/root/[/]\n"

‎src/modules/bgrpiimage-common/apply-lib.sh‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,58 @@ bg_intent() {
116116
printf '%s\n' "$line" >> "$BGRPI_INTENTS"
117117
}
118118

119+
# ---------------------------------------------------------------------------
120+
# What an update may never write
121+
# ---------------------------------------------------------------------------
122+
# Enforced here, in the one function that writes, rather than trusted to each
123+
# module. A denylist nine scripts are supposed to respect is a code-review
124+
# convention and will be violated within two releases; a denylist the writer
125+
# enforces is a property of the system.
126+
#
127+
# DEVICE CONTEXT ONLY. Several of these are written legitimately at image
128+
# build time - /etc/hostname and /etc/hosts are exactly what bgrpiimage-base
129+
# is for. The list is about what an UPDATE to a configured, deployed machine
130+
# may touch, and the answer for all of it is "not this".
131+
#
132+
# Three kinds of entry:
133+
# identity and access shadow/passwd/sudoers/pam - the only two ways onto
134+
# these boxes are sudo and su, and they are each
135+
# other's only fallback. An update that breaks both
136+
# is a truck roll with an rpiboot jumper.
137+
# operator-owned state the 05- overrides this project's own helper writes,
138+
# wpa_supplicant PSKs, rfkill saved state, static
139+
# resolv.conf, hostname - site data, not platform.
140+
# boot and storage cmdline.txt and fstab, where one typo is an
141+
# unbootable device rather than a failed service.
142+
BGRPI_DENY_GLOBS=(
143+
'/etc/shadow' '/etc/passwd' '/etc/group' '/etc/gshadow'
144+
'/etc/subuid' '/etc/subgid'
145+
'/etc/sudoers' '/etc/sudoers.d/*'
146+
'/etc/pam.d/*'
147+
'/etc/ssh/ssh_host_*'
148+
'/home/*' '/root/*'
149+
'/etc/hostname' '/etc/hosts'
150+
'/etc/resolv.conf'
151+
'/etc/wpa_supplicant/*'
152+
'/var/lib/systemd/rfkill/*'
153+
'/etc/systemd/network/05-bgrpiimage-*'
154+
'/etc/docker/daemon.json'
155+
'/boot/firmware/cmdline.txt'
156+
'/etc/fstab' '/etc/crypttab'
157+
'/etc/apt/sources.list' '/etc/apt/sources.list.d/*' '/etc/apt/keyrings/*'
158+
'/etc/machine-id'
159+
)
160+
161+
bg_denied() {
162+
local dest="$1" g
163+
bg_is_image && return 1
164+
for g in "${BGRPI_DENY_GLOBS[@]}"; do
165+
# shellcheck disable=SC2053 # glob match on the right is the point
166+
[[ "$dest" == $g ]] && return 0
167+
done
168+
return 1
169+
}
170+
119171
# ---------------------------------------------------------------------------
120172
# Writing
121173
# ---------------------------------------------------------------------------
@@ -131,6 +183,9 @@ bg_install() {
131183
BGRPI_LAST_CHANGED=0
132184

133185
[[ -r "$src" ]] || bg_die "bg_install: source not readable: $src"
186+
if bg_denied "$dest"; then
187+
bg_die "refusing to write $dest on a running system - see BGRPI_DENY_GLOBS"
188+
fi
134189

135190
if [[ -f "$target" ]] && [[ "$(_bg_sha "$src")" == "$(_bg_sha "$target")" ]]; then
136191
# Content matches; still assert the mode, because a wrong mode is a

‎tests/test-apply-guards.sh‎

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
#!/usr/bin/env bash
2+
# Asserts the guards in apply-lib.sh actually refuse.
3+
#
4+
# These are the guards that stand between an update and a truck roll, and
5+
# every one of them is a negative: "this must NOT happen". Negatives rot
6+
# silently - a refactor that drops a condition passes every other test in the
7+
# suite, because nothing it does is wrong, it just stops declining. So they
8+
# get their own assertions.
9+
#
10+
# bash tests/test-apply-guards.sh # or: make test-guards
11+
set -uo pipefail
12+
13+
if [ -z "${BGRPI_TEST_INNER:-}" ]; then
14+
root=$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)
15+
command -v docker >/dev/null || { echo "docker is required" >&2; exit 1; }
16+
mount="$root"
17+
case "$(uname -s)" in
18+
MINGW*|MSYS*|CYGWIN*) mount=$(cygpath -w "$root"); export MSYS_NO_PATHCONV=1 ;;
19+
esac
20+
exec docker run --rm -e BGRPI_TEST_INNER=1 \
21+
-v "${mount}:/repo:ro" debian:trixie-slim \
22+
bash /repo/tests/test-apply-guards.sh
23+
fi
24+
25+
PASS=0; FAIL=0
26+
ok() { echo " PASS $*"; PASS=$((PASS+1)); }
27+
bad() { echo " FAIL $*"; FAIL=$((FAIL+1)); }
28+
29+
LIB=/repo/src/modules/bgrpiimage-common/apply-lib.sh
30+
mkdir -p /scratch/etc /stub
31+
printf 'payload\n' > /src.txt
32+
for c in systemctl networkctl udevadm; do
33+
printf '#!/bin/sh\nexit 0\n' > "/stub/$c"; chmod +x "/stub/$c"
34+
done
35+
export PATH="/stub:$PATH"
36+
37+
# Run one bg_install in a subshell and report its exit status.
38+
try_install() { # ctx dest
39+
( export BGRPI_CTX="$1" BGRPI_ROOT=/scratch BGRPI_INTENTS=/i.txt
40+
# shellcheck disable=SC1090
41+
. "$LIB"
42+
bg_install /src.txt "$2" 0644 ) >/dev/null 2>&1
43+
}
44+
45+
echo "=== denylist: refused on a device, allowed while building an image ==="
46+
for p in /etc/shadow /etc/passwd /etc/sudoers.d/010-bgrpiimage-admin \
47+
/etc/pam.d/su /etc/ssh/ssh_host_rsa_key /etc/hostname /etc/hosts \
48+
/etc/resolv.conf /etc/wpa_supplicant/wpa_supplicant-wlan0.conf \
49+
/var/lib/systemd/rfkill/0:wlan \
50+
/etc/systemd/network/05-bgrpiimage-can0.network \
51+
/etc/docker/daemon.json /boot/firmware/cmdline.txt /etc/fstab \
52+
/etc/apt/sources.list.d/docker.list /etc/machine-id; do
53+
if try_install device "$p"; then bad "device write to $p was ALLOWED"; else ok "device refuses $p"; fi
54+
done
55+
56+
echo "=== the same paths are legitimate at build time ==="
57+
for p in /etc/hostname /etc/hosts; do
58+
if try_install image "$p"; then ok "image may write $p"; else bad "image write to $p was refused"; fi
59+
done
60+
61+
echo "=== paths this project owns stay writable on a device ==="
62+
for p in /etc/systemd/network/40-can0.network /etc/systemd/network/70-can0.link \
63+
/etc/apt/apt.conf.d/50unattended-upgrades /etc/issue \
64+
/etc/update-motd.d/10-bgrpiimage /etc/bgrpiimage-release; do
65+
if try_install device "$p"; then ok "device may write $p"; else bad "device write to $p was refused"; fi
66+
done
67+
68+
echo "=== unit masking is build-only ==="
69+
mask_rc() { # ctx -> "refused" | "did it"
70+
( export BGRPI_CTX="$1" BGRPI_ROOT=/scratch BGRPI_INTENTS=/i.txt
71+
# shellcheck disable=SC1090
72+
. "$LIB"
73+
bg_unit_mask NetworkManager 2>&1 ) | grep -q 'refusing to mask' && echo refused || echo did
74+
}
75+
[ "$(mask_rc device)" = "refused" ] && ok "device refuses to mask a unit" || bad "device masked a unit"
76+
[ "$(mask_rc image)" = "did" ] && ok "image may mask a unit" || bad "image refused to mask"
77+
78+
echo "=== dry run writes nothing ==="
79+
rm -rf /scratch; mkdir -p /scratch
80+
( export BGRPI_CTX=device BGRPI_ROOT=/scratch BGRPI_DRY_RUN=1 BGRPI_INTENTS=/i.txt
81+
# shellcheck disable=SC1090
82+
. "$LIB"
83+
bg_install /src.txt /etc/issue 0644 ) >/dev/null 2>&1
84+
if [ -e /scratch/etc/issue ]; then bad "dry run created a file"; else ok "dry run created nothing"; fi
85+
86+
echo
87+
echo "================ $PASS passed, $FAIL failed ================"
88+
[ "$FAIL" -eq 0 ]

0 commit comments

Comments
 (0)

Back | FazBrowse Home | New Git URL