fix: status.json fuer normale Nutzer lesbar machen, Lesefehler nicht mehr verschlucken
server.py legt status.json ueber mkstemp() (Modus 0600) + os.replace() an; da os.replace() den Modus der temporaeren Datei uebernimmt, war die Datei nach jedem Schreiben nur fuer root lesbar - der Dienst laeuft als root, das CLI liest aber als normaler Benutzer. os.chmod(tmp, 0o644) vor dem Rename behebt das umask-fest, auch bei wiederholten Schreibvorgaengen. ninlight-station status hat den fehlenden Lesezugriff bisher stillschweigend wie "noch nie ausgeliefert" behandelt (beides fiel durch denselben "-r"-Test). Jetzt wird zwischen "Datei fehlt" (unauffaellig) und "Datei da, aber nicht lesbar" (Fehlermeldung auf stderr) unterschieden. Neuer Test prueft, dass status.json nach dem ersten und nach einem weiteren Schreibvorgang world-readable bleibt, auch unter restriktivem umask. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
Michael Sauer
co-authored by
Claude Sonnet 5
Claude Sonnet 5
parent
cb54dac066
commit
62903a7f05
@@ -113,7 +113,19 @@ PY
|
||||
# "status" soll mit 0 enden, sobald es seine Auskunft geben konnte, auch wenn noch nie
|
||||
# ausgeliefert wurde - ohne das "|| true" waere der Exit-Code des Skripts hier der des
|
||||
# fehlgeschlagenen Tests (1), weil das die letzte Anweisung im Zweig ist.
|
||||
[ -r /run/ninlight-station/status.json ] && { echo "Zuletzt ausgeliefert:"; sed 's/^/ /' /run/ninlight-station/status.json; }
|
||||
#
|
||||
# "existiert nicht" (Station lief noch nie) und "existiert, ist aber nicht lesbar"
|
||||
# sind zwei verschiedene Zustaende und duerfen nicht beide stillschweigend im
|
||||
# "-r"-Test verschwinden: sonst faellt ein Rechte-Fehler auf status.json (siehe
|
||||
# server.py, schreibt als root) nie auf, weil die Ausgabe genauso aussieht wie
|
||||
# "noch nie ausgeliefert".
|
||||
STATUS_FILE=/run/ninlight-station/status.json
|
||||
if [ -r "$STATUS_FILE" ]; then
|
||||
echo "Zuletzt ausgeliefert:"
|
||||
sed 's/^/ /' "$STATUS_FILE"
|
||||
elif [ -e "$STATUS_FILE" ]; then
|
||||
echo "Fehler: $STATUS_FILE existiert, ist aber nicht lesbar (Rechte pruefen)" >&2
|
||||
fi
|
||||
exit 0
|
||||
;;
|
||||
*)
|
||||
|
||||
+19
-3
@@ -10,6 +10,7 @@ Standard library only - the Pi gets no extra packages.
|
||||
from __future__ import annotations
|
||||
|
||||
import argparse
|
||||
import contextlib
|
||||
import json
|
||||
import os
|
||||
import re
|
||||
@@ -85,9 +86,24 @@ class Station:
|
||||
try:
|
||||
self.state_path.parent.mkdir(parents=True, exist_ok=True)
|
||||
fd, tmp = tempfile.mkstemp(dir=self.state_path.parent, prefix=".status-")
|
||||
with os.fdopen(fd, "w") as f:
|
||||
json.dump(payload, f, indent=2)
|
||||
os.replace(tmp, self.state_path)
|
||||
try:
|
||||
with os.fdopen(fd, "w") as f:
|
||||
json.dump(payload, f, indent=2)
|
||||
# The service runs as root and mkstemp() creates the file 0600, so
|
||||
# without this an operator reading status.json as a normal user (see
|
||||
# tools/station/ninlight-station) gets "permission denied" - and since
|
||||
# os.replace() preserves the mode it arrives with, that wrong mode
|
||||
# would otherwise survive onto the live file on every single write,
|
||||
# not just the first. os.chmod() ignores the process umask (unlike the
|
||||
# mode argument to mkstemp/open), so a restrictive umask on the Pi
|
||||
# can't quietly bring the 0600 back. Set it before the rename so
|
||||
# there's never a window where the path holds a 0600 file.
|
||||
os.chmod(tmp, 0o644)
|
||||
os.replace(tmp, self.state_path)
|
||||
except BaseException:
|
||||
with contextlib.suppress(OSError):
|
||||
os.unlink(tmp)
|
||||
raise
|
||||
except OSError as e:
|
||||
print(f"warning: cannot write {self.state_path}: {e}", flush=True)
|
||||
|
||||
|
||||
@@ -1,7 +1,9 @@
|
||||
import hashlib
|
||||
import http.client
|
||||
import json
|
||||
import os
|
||||
import socket
|
||||
import stat
|
||||
import subprocess
|
||||
import sys
|
||||
import threading
|
||||
@@ -173,6 +175,43 @@ def test_state_file_records_a_missing_image(station):
|
||||
assert state["recent"][-1]["target"] == "g95_cw"
|
||||
|
||||
|
||||
def test_state_file_is_world_readable_even_under_a_restrictive_umask(tmp_path):
|
||||
"""status.json is written by a root service but read by a normal-user CLI.
|
||||
|
||||
tempfile.mkstemp() creates its file 0600 and os.replace() preserves whatever mode
|
||||
the temp file had, so the server must explicitly widen the mode before the rename -
|
||||
and must do so with chmod() rather than relying on the mode passed to mkstemp/open,
|
||||
because that one is still narrowed by the process umask. A restrictive umask (as
|
||||
root services on the Pi may well run under) must not bring the 0600 back.
|
||||
"""
|
||||
old_umask = os.umask(0o077)
|
||||
try:
|
||||
s = Station(tmp_path)
|
||||
finally:
|
||||
os.umask(old_umask)
|
||||
try:
|
||||
make_image(s.image_dir / "ninlight_evk_ota.img", "evk", "1.0.0", b"w" * 100)
|
||||
|
||||
def world_readable() -> bool:
|
||||
mode = stat.S_IMODE(s.state.stat().st_mode)
|
||||
return bool(mode & stat.S_IROTH)
|
||||
|
||||
# Written once already at startup (Station.__init__ -> write_state()).
|
||||
s.read_state(lambda _: world_readable())
|
||||
assert world_readable(), "status.json is not world-readable after the initial write"
|
||||
|
||||
# A second write (a real delivery) must not lose the mode either - e.g. if the
|
||||
# widening were only applied to the file's very first creation instead of to
|
||||
# every temp file, mode 0600 would silently come back on every later write.
|
||||
c = s.conn()
|
||||
c.request("GET", "/ninlight_evk_ota.img")
|
||||
c.getresponse().read()
|
||||
s.read_state(lambda st: st["served"] >= 1)
|
||||
assert world_readable(), "status.json lost its mode on a later overwrite"
|
||||
finally:
|
||||
s.stop()
|
||||
|
||||
|
||||
def test_recent_is_capped_at_eight(station):
|
||||
make_image(station.image_dir / "ninlight_evk_ota.img", "evk", "1.0.0", b"q" * 10)
|
||||
for _ in range(12):
|
||||
|
||||
Reference in New Issue
Block a user