From f4b652e182a614ae1b43e17c8c8f957cfc7b12e6 Mon Sep 17 00:00:00 2001 From: Ramon Date: Sun, 2 Aug 2026 14:49:18 +0200 Subject: [PATCH] Eigenaar meenemen bij het verhuizen MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `shutil.copy2` neemt de rechten mee maar niet de eigenaar, en Server Up draait als root. Verhuisde appdata werd daardoor root:root, waarna een container die als PUID 1000 draait er niet meer in kon. - elk bestand, elke map, elke symlink én de hoofdmap krijgen de uid/gid van het origineel - lukt chownen niet (je draait niet als root), dan gaat de verhuizing door met een waarschuwing in plaats van te struikelen - tests voor beide gevallen, en voor het feit dat de uid van de bron komt Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Q9eqpADJSRs49SoGGr4NAy --- CHANGELOG.md | 8 +++++ server-up/core/paden.py | 34 +++++++++++++++++++++ tests/test_paden.py | 66 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 108 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 009ab89..275caac 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -34,6 +34,14 @@ dan blijft het origineel staan en is de melding de fout. Het omschrijven kijkt naar hele pad-segmenten, zodat `/opt/serverup` vervangen `/opt/serverup-oud` met rust laat. +**Rechten én eigenaar gaan mee.** `shutil.copy2` neemt de rechten over maar niet +de eigenaar, en Server Up draait als root — verhuisde appdata werd dus root:root, +waarna een container die als PUID 1000 draait niet meer bij zijn eigen gegevens +kon. De linuxserver-images zetten `/config` bij het starten terug, maar postgres, +Nextcloud en Immich lopen er gewoon op vast. De verhuizing chownt nu elk bestand, +elke map, elke symlink én de hoofdmap zelf naar de eigenaar van het origineel. +Lukt dat niet, dan gaat de verhuizing door met een waarschuwing in het log. + Jobs kregen daarvoor een `voortgang()`; de terminal toont de balk boven het log. # v0.8.82-beta — "Poort {port} is vrij" stond er letterlijk diff --git a/server-up/core/paden.py b/server-up/core/paden.py index 621be45..caa4122 100644 --- a/server-up/core/paden.py +++ b/server-up/core/paden.py @@ -127,6 +127,25 @@ def past_het(bron: Path, doel: Path) -> tuple[bool, str]: # ── Verhuizen ──────────────────────────────────────────────────────────────── +def _neem_eigenaar_over(bron: Path, doel: Path, mislukt: list) -> None: + """Geef het gekopieerde dezelfde eigenaar en rechten als het origineel. + + `shutil.copy2` neemt de rechten mee maar níét de eigenaar. Server Up draait + als root, dus zonder dit wordt alles root:root — en een container die als + PUID 1000 draait kan dan niet meer bij zijn eigen appdata. Bij de + linuxserver-images valt dat nog mee (die zetten `/config` bij het starten + terug), maar postgres, Nextcloud en Immich lopen er gewoon op vast. + """ + try: + st = os.stat(bron, follow_symlinks=False) + os.chown(doel, st.st_uid, st.st_gid, follow_symlinks=False) + except (OSError, NotImplementedError) as e: + # Draai je niet als root, dan is alles toch al van jou; dan is dit geen + # fout maar een no-op. Alleen echte gevallen verzamelen we. + if not mislukt: + mislukt.append(str(e)) + + def verhuis(bron: Path, doel: Path, log_fn=None, voortgang_fn=None, verwijder_bron: bool = True) -> int: """Kopieer alles van bron naar doel; verwijder de bron pas als het klopt. @@ -134,6 +153,9 @@ def verhuis(bron: Path, doel: Path, log_fn=None, voortgang_fn=None, `voortgang_fn(gedaan_bytes, totaal_bytes, label)` wordt tijdens het kopiëren aangeroepen. Retourneert het aantal gekopieerde bestanden. + Rechten én eigenaar gaan mee: een verhuisde appdata-map die opeens van root + is, breekt elke container die als een gewone gebruiker draait. + Verwijderen gebeurt alleen als aantal én omvang overeenkomen. Klopt er iets niet, dan blijft de bron staan en volgt er een fout — een half gelukte verhuizing waarbij het origineel al weg is, is het ergste dat hier kan @@ -153,6 +175,12 @@ def verhuis(bron: Path, doel: Path, log_fn=None, voortgang_fn=None, f"{diskspace.leesbaar(totaal)}") doel.mkdir(parents=True, exist_ok=True) + # Ook de hoofdmap zelf: staat die op root, dan kan een container er niet in + # schrijven al klopt alles eronder. + chown_mislukt: list = [] + shutil.copystat(bron, doel) + _neem_eigenaar_over(bron, doel, chown_mislukt) + gedaan = kopieerd = 0 for p in sorted(bron.rglob("*")): rel = p.relative_to(bron) @@ -175,9 +203,15 @@ def verhuis(bron: Path, doel: Path, log_fn=None, voortgang_fn=None, kopieerd += 1 if voortgang_fn: voortgang_fn(gedaan, totaal, str(rel)) + _neem_eigenaar_over(p, uit, chown_mislukt) except OSError as e: raise PadFout(f"Kopiëren van {rel} mislukte: {e}") from e + if chown_mislukt and log_fn: + log_fn(f"Let op: eigenaar kon niet worden overgenomen " + f"({chown_mislukt[0]}). Controleer de rechten als een app niet " + f"meer bij zijn gegevens kan.") + # Controleren vóór we iets weggooien. n_doel, b_doel = meet(doel) if n_doel < aantal or b_doel < totaal: diff --git a/tests/test_paden.py b/tests/test_paden.py index 8ec4965..e8aed3d 100644 --- a/tests/test_paden.py +++ b/tests/test_paden.py @@ -113,6 +113,72 @@ def test_zelfde_map_doet_niets(env, tmp_path): assert (bron / "a.txt").exists() +def test_eigenaar_en_rechten_gaan_mee(env, tmp_path, monkeypatch): + """`shutil.copy2` neemt de rechten mee maar niet de eigenaar. + + Server Up draait als root, dus zonder chown werd verhuisde appdata root:root + en kwam een container die als PUID 1000 draait er niet meer in. De test kan + zelf niet chownen (dat mag alleen root), dus we leggen vast wát er gevraagd + wordt. + """ + from core import paden + bron = _vul(tmp_path / "oud", {"a.txt": "hallo", "sub/b.txt": "wereld"}) + os.chmod(bron / "a.txt", 0o640) + doel = tmp_path / "nieuw" + + gevraagd = {} + monkeypatch.setattr(paden.os, "chown", + lambda p, u, g, **kw: gevraagd.__setitem__(str(p), (u, g))) + + paden.verhuis(bron, doel) + + eigen = (os.getuid(), os.getgid()) + assert gevraagd.get(str(doel)) == eigen, "de hoofdmap zelf blijft van root" + assert gevraagd.get(str(doel / "a.txt")) == eigen + assert gevraagd.get(str(doel / "sub")) == eigen + assert gevraagd.get(str(doel / "sub" / "b.txt")) == eigen + # De rechten komen van copy2/copystat en horen ook te kloppen. + assert os.stat(doel / "a.txt").st_mode & 0o777 == 0o640 + + +def test_de_eigenaar_komt_van_de_bron(env, tmp_path, monkeypatch): + """De uid/gid moet van het origineel komen, niet van wie er toevallig + kopieert. Chownen kan alleen root, dus we leggen de vraag vast.""" + from core import paden + bron = _vul(tmp_path, {"a.txt": "x"}) / "a.txt" + doel = tmp_path / "b.txt" + doel.write_text("x") + + st = os.stat(bron) + # Een echte stat_result met een andere eigenaar, zodat copystat blijft werken. + nep = os.stat_result((st.st_mode, st.st_ino, st.st_dev, st.st_nlink, + 1000, 1000, st.st_size, + int(st.st_atime), int(st.st_mtime), int(st.st_ctime))) + monkeypatch.setattr(paden.os, "stat", lambda p, **kw: nep) + gevraagd = [] + monkeypatch.setattr(paden.os, "chown", + lambda p, u, g, **kw: gevraagd.append((u, g))) + + paden._neem_eigenaar_over(bron, doel, []) + assert gevraagd == [(1000, 1000)] + + +def test_chown_die_niet_mag_stopt_de_verhuizing_niet(env, tmp_path, monkeypatch): + """Draai je niet als root, dan is alles toch al van jou. Dat mag geen fout + zijn — wel een melding, want dan kán het misgaan.""" + from core import paden + bron = _vul(tmp_path / "oud", {"a.txt": "hallo"}) + doel = tmp_path / "nieuw" + monkeypatch.setattr(paden.os, "chown", + lambda *a, **k: (_ for _ in ()).throw(PermissionError("mag niet"))) + + regels = [] + n = paden.verhuis(bron, doel, log_fn=regels.append) + + assert n == 1 and (doel / "a.txt").read_text() == "hallo" + assert any("eigenaar" in r for r in regels), "geen waarschuwing over de rechten" + + def test_symlinks_blijven_symlinks(env, tmp_path): """Een link volgen zou de inhoud dupliceren of buiten de boom kunnen wijzen.""" from core import paden