Deze tests toetsten als root iets anders dan bedoeld
Some checks failed
Deploy server-up (dev) / deploy (push) Failing after 16m6s

De CI-runner draait als root, en drie tests gingen daar onderuit omdat ze
stilzwijgend van een gewone gebruiker uitgingen:

- rechten intrekken met chmod 000 houdt root niet tegen, dus er werd niets
  overgeslagen; de weigering komt nu uit het inpakken zelf
- `backups.os` ís de os-module, dus het onderscheppen van chown ving ook wat
  tarfile als root zelf chownt; er wordt nu op het exacte doelpad getoetst
- het geval "draait al als niet-root" liep als root juist de hele afdaal-tak in
  en zou daar echt een gebruiker aanmaken; de uid wordt nu nagebootst

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Q9eqpADJSRs49SoGGr4NAy
This commit is contained in:
Ramon 2026-08-02 22:08:57 +02:00
parent fc2e80fbda
commit 94364973ff
2 changed files with 37 additions and 11 deletions

View file

@ -331,14 +331,18 @@ def test_een_onleesbaar_bestand_laat_de_rest_van_de_backup_staan(env, monkeypatc
_d, appdata = _stack_met_appdata(env) _d, appdata = _stack_met_appdata(env)
(appdata / "umami" / "tweede.txt").write_bytes(b"deze wel") (appdata / "umami" / "tweede.txt").write_bytes(b"deze wel")
# Geen namaak: een bestand waar de eigenaar zelf niet in kan kijken. Precies # Rechten intrekken werkt niet als toets: root leest ook een bestand met
# wat je krijgt als een container zijn data als een andere uid wegschrijft. # modus 000, en de CI draait als root. De weigering komt daarom van het
onleesbaar = appdata / "umami" / "bestand.txt" # inpakken zelf — dat is precies waar hij vandaan zou komen.
onleesbaar.chmod(0o000) echt = backups.tarfile.TarFile.add
try:
def weiger_een(self, name, *a, **kw):
if str(name).endswith("bestand.txt"):
raise PermissionError("Permission denied")
return echt(self, name, *a, **kw)
monkeypatch.setattr(backups.tarfile.TarFile, "add", weiger_een)
meta = backups.create("umami") meta = backups.create("umami")
finally:
onleesbaar.chmod(0o644)
assert meta["skipped"] == ["bestand.txt"], meta assert meta["skipped"] == ["bestand.txt"], meta
with backups.tarfile.open(backups.backup_dir() / meta["file"]) as tar: with backups.tarfile.open(backups.backup_dir() / meta["file"]) as tar:
@ -374,13 +378,17 @@ def test_teruggezette_appdata_krijgt_de_puid_van_de_stack(env, monkeypatch):
meta = backups.create("umami") meta = backups.create("umami")
gevraagd = [] gevraagd = []
# `backups.os` ís de os-module, dus dit vangt ook wat tarfile zelf chownt —
# als root doet die dat namelijk wél. Daarom toetsen we op het exacte
# doelpad in plaats van op "alles wat langskwam".
monkeypatch.setattr(backups.os, "chown", monkeypatch.setattr(backups.os, "chown",
lambda p, u, g, **kw: gevraagd.append((str(p), u, g))) lambda p, u, g, **kw: gevraagd.append((str(p), u, g)))
ok, msg = backups.restore("umami", meta["file"]) ok, msg = backups.restore("umami", meta["file"])
assert ok, msg assert ok, msg
assert gevraagd, "er is helemaal niet gechownd" doel = str(appdata / "umami")
assert all(u == 1005 and g == 1006 for _p, u, g in gevraagd), gevraagd assert (doel, 1005, 1006) in gevraagd, \
f"de teruggezette appdata kreeg niet de PUID van de stack: {gevraagd}"
def test_zonder_puid_valt_hij_terug_op_ons_eigen_account(env): def test_zonder_puid_valt_hij_terug_op_ons_eigen_account(env):

View file

@ -74,13 +74,14 @@ def draai(env_extra: dict, nep=None, argv=("echo", "APP-GESTART")):
# ── Het mag nooit weigeren ─────────────────────────────────────────────────── # ── Het mag nooit weigeren ───────────────────────────────────────────────────
# Deze gevallen stoppen vóór het script naar zijn eigen uid kijkt, dus ze
# gedragen zich hetzelfde of je nu root bent of niet.
@pytest.mark.parametrize("env_extra, waarom", [ @pytest.mark.parametrize("env_extra, waarom", [
({}, "zonder SU_UID"), ({}, "zonder SU_UID"),
({"SU_UID": "0"}, "expliciet root"), ({"SU_UID": "0"}, "expliciet root"),
({"SU_UID": ""}, "lege SU_UID"), ({"SU_UID": ""}, "lege SU_UID"),
({"SU_UID": "abc"}, "geen getal"), ({"SU_UID": "abc"}, "geen getal"),
({"SU_UID": "1000", "SU_GID": "x"}, "gid geen getal"), ({"SU_UID": "1000", "SU_GID": "x"}, "gid geen getal"),
({"SU_UID": "1000"}, "al niet-root"),
]) ])
def test_de_app_start_hoe_dan_ook(env_extra, waarom): def test_de_app_start_hoe_dan_ook(env_extra, waarom):
r = draai(env_extra) r = draai(env_extra)
@ -88,6 +89,23 @@ def test_de_app_start_hoe_dan_ook(env_extra, waarom):
assert "APP-GESTART" in r.stdout, f"{waarom}: de app is niet gestart" assert "APP-GESTART" in r.stdout, f"{waarom}: de app is niet gestart"
def test_wie_al_niet_root_is_zakt_niet_verder_af(nep_omgeving):
"""Zonder root valt er niets klaar te zetten en niets af te zakken.
De uid wordt hier nagebootst in plaats van aan de omgeving overgelaten: de
CI-runner draait wél als root, en dan zou deze test iets heel anders toetsen
dan waarvoor hij bedoeld is of erger, echt een gebruiker aanmaken.
"""
(nep_omgeving["bin"] / "id").write_text("#!/bin/sh\necho 1000\n", encoding="utf-8")
(nep_omgeving["bin"] / "id").chmod(0o755)
r = draai({"SU_UID": "990", "SU_GID": "990"}, nep=nep_omgeving)
assert r.returncode == 0, r.stderr
assert "APP-GESTART" in r.stdout
assert "wordt genegeerd" in r.stderr
assert nep_omgeving["log"].read_text() == "", "er is toch iets gewijzigd"
def test_onzin_wordt_gemeld_en_niet_stil_genegeerd(): def test_onzin_wordt_gemeld_en_niet_stil_genegeerd():
r = draai({"SU_UID": "abc"}) r = draai({"SU_UID": "abc"})
assert "getallen" in r.stderr assert "getallen" in r.stderr