fix: install.sh loop abort, ACME poll sudo gate, /static/ sub-paths
- install.sh: the traversal-chmod loop assigned _d but looped over the never-set $d; under set -u every fresh install aborted with "d: unbound variable" at that line. Loop over $_d. - acme collector: the self-heal normalize (sudo chmod g+rwX) now runs only when a no-sudo group-read-bit probe detects a lost bit — acme.sh re-hardens the tree 600 on every run, so the steady-state poll makes no sudo call. The group bit (not daemon readability) is what the two-user model keeps for the WebUI user. - lib.acme: new get_acme_home() accessor (ACME_HOME env, default data/acme), reused by _run_acme; _summarize_acme_output preserves a "Permission denied" line even when it is not among the final two, so the collector's actionable-error matcher keeps firing. - nginx template: emit location /static/ for any is_management path (not only '/'); the SPA references /static/... at the domain root regardless of the management backend path. - tests: probe, summarizer, and nginx-subpath cases in test_state.py, test_acme.py, test_nginx.py.
This commit is contained in:
@@ -3,9 +3,11 @@
|
|||||||
import logging
|
import logging
|
||||||
import os
|
import os
|
||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
|
from stat import S_IRGRP
|
||||||
from typing import Any
|
from typing import Any
|
||||||
|
|
||||||
from lib import schema
|
from lib import schema
|
||||||
|
from lib.acme import get_acme_home
|
||||||
from lib.common import load_json
|
from lib.common import load_json
|
||||||
from lib.state import PROJECT_DIR, _now_iso, register_collector
|
from lib.state import PROJECT_DIR, _now_iso, register_collector
|
||||||
|
|
||||||
@@ -157,6 +159,25 @@ def _friendly_acme_error(exc: Exception) -> str:
|
|||||||
return text
|
return text
|
||||||
|
|
||||||
|
|
||||||
|
def _acme_home_needs_normalize() -> bool:
|
||||||
|
"""Cheap no-sudo probe: has any ACME_HOME file lost its group-read bit?
|
||||||
|
|
||||||
|
acme.sh re-hardens its tree (``chmod 600``) on every run, so the daemon's
|
||||||
|
self-heal (``normalize_acme_home``) is only needed after a run by another
|
||||||
|
user (e.g. a manual run as the WebUI user) stripped group read. The probe
|
||||||
|
checks the group bit — not the daemon's own readability — because group
|
||||||
|
read is what the two-user model keeps for the WebUI user; a file the
|
||||||
|
daemon can read but the group cannot must still be healed.
|
||||||
|
"""
|
||||||
|
try:
|
||||||
|
for p in get_acme_home().rglob("*"):
|
||||||
|
if p.is_file() and not (p.stat().st_mode & S_IRGRP):
|
||||||
|
return True
|
||||||
|
except OSError:
|
||||||
|
return True
|
||||||
|
return False
|
||||||
|
|
||||||
|
|
||||||
def _collect_acme() -> schema.AcmeState:
|
def _collect_acme() -> schema.AcmeState:
|
||||||
"""Collect ACME certificate list and email.
|
"""Collect ACME certificate list and email.
|
||||||
|
|
||||||
@@ -171,16 +192,18 @@ def _collect_acme() -> schema.AcmeState:
|
|||||||
# `status.error` so the poll diff still detects recovery.
|
# `status.error` so the poll diff still detects recovery.
|
||||||
cert_error: str | None = None
|
cert_error: str | None = None
|
||||||
try:
|
try:
|
||||||
# Self-heal ACME_HOME permissions before listing, exactly like the
|
# Self-heal ACME_HOME permissions before listing, but only when the
|
||||||
# handler preflight (_run_acme_preflight). acme.sh dot-sources
|
# probe detects a lost group-read bit — the steady-state poll then
|
||||||
# account.conf on startup; a prior run by another user (e.g. a manual
|
# makes no sudo call. acme.sh dot-sources account.conf on startup; a
|
||||||
# run as the WebUI user) can leave it owner-only and make `--list`
|
# prior run by another user (e.g. a manual run as the WebUI user) can
|
||||||
# exit 2. The startup normalize only covers the first collection, so
|
# leave it owner-only and make `--list` exit 2. The startup normalize
|
||||||
# the poll must normalize too or a mid-lifetime ownership flip would
|
# only covers the first collection, so the poll must probe too or a
|
||||||
# blank the cert list until the next issue/renew or daemon restart.
|
# mid-lifetime ownership flip would blank the cert list until the
|
||||||
|
# next issue/renew or daemon restart.
|
||||||
from daemon.handlers.acme import normalize_acme_home
|
from daemon.handlers.acme import normalize_acme_home
|
||||||
from lib.acme import list_certs
|
from lib.acme import list_certs
|
||||||
|
|
||||||
|
if _acme_home_needs_normalize():
|
||||||
normalize_acme_home()
|
normalize_acme_home()
|
||||||
certs = list_certs()
|
certs = list_certs()
|
||||||
except Exception as exc:
|
except Exception as exc:
|
||||||
|
|||||||
+17
-3
@@ -32,6 +32,11 @@ _ACME_ENVIRON = {
|
|||||||
_WEBROOT = PROJECT_DIR / "data" / "acme" / "www"
|
_WEBROOT = PROJECT_DIR / "data" / "acme" / "www"
|
||||||
|
|
||||||
|
|
||||||
|
def get_acme_home() -> Path:
|
||||||
|
"""Resolve the ACME home directory (``ACME_HOME`` env, default ``data/acme``)."""
|
||||||
|
return Path(os.environ.get("ACME_HOME", str(_ACME_HOME)))
|
||||||
|
|
||||||
|
|
||||||
def _find_acme() -> str:
|
def _find_acme() -> str:
|
||||||
"""Locate the acme.sh binary on the system.
|
"""Locate the acme.sh binary on the system.
|
||||||
|
|
||||||
@@ -87,7 +92,7 @@ def _run_acme(args: list[str]) -> str:
|
|||||||
acme_bin = _find_acme()
|
acme_bin = _find_acme()
|
||||||
|
|
||||||
# Check for ACME_HOME env var (set by systemd in production)
|
# Check for ACME_HOME env var (set by systemd in production)
|
||||||
acme_home_env = os.environ.get("ACME_HOME", str(_ACME_HOME))
|
acme_home_env = str(get_acme_home())
|
||||||
|
|
||||||
cmd: list[str] = [
|
cmd: list[str] = [
|
||||||
acme_bin,
|
acme_bin,
|
||||||
@@ -143,14 +148,22 @@ def _summarize_acme_output(output: str) -> str:
|
|||||||
in the final lines (e.g. "The retryafter=86400 value is too large
|
in the final lines (e.g. "The retryafter=86400 value is too large
|
||||||
(> 600), will not retry anymore."). Strips per-line timestamps and
|
(> 600), will not retry anymore."). Strips per-line timestamps and
|
||||||
the "Please check log file" pointer so the summary stays toast-
|
the "Please check log file" pointer so the summary stays toast-
|
||||||
sized. The full transcript remains in the log and acme.sh.log.
|
sized. A "Permission denied" diagnostic is preserved even when it
|
||||||
|
is not among the final lines — the actionable-error matcher in
|
||||||
|
daemon/collectors/acme.py keys off it. The full transcript remains
|
||||||
|
in the log and acme.sh.log.
|
||||||
"""
|
"""
|
||||||
lines = [line.strip() for line in output.strip().splitlines() if line.strip()]
|
lines = [line.strip() for line in output.strip().splitlines() if line.strip()]
|
||||||
lines = [re.sub(r"^\[[^\]]*\] ", "", line) for line in lines]
|
lines = [re.sub(r"^\[[^\]]*\] ", "", line) for line in lines]
|
||||||
lines = [line for line in lines if not line.startswith("Please check log file")]
|
lines = [line for line in lines if not line.startswith("Please check log file")]
|
||||||
if not lines:
|
if not lines:
|
||||||
return "(no output)"
|
return "(no output)"
|
||||||
return "; ".join(lines[-2:])
|
tail = list(lines[-2:])
|
||||||
|
for line in reversed(lines):
|
||||||
|
if "Permission denied" in line and line not in tail:
|
||||||
|
tail.insert(0, line)
|
||||||
|
break
|
||||||
|
return "; ".join(tail)
|
||||||
|
|
||||||
|
|
||||||
def set_email(email: str) -> None:
|
def set_email(email: str) -> None:
|
||||||
@@ -649,6 +662,7 @@ __all__ = [
|
|||||||
"days_until_expiry",
|
"days_until_expiry",
|
||||||
"deploy",
|
"deploy",
|
||||||
"find_cert_dir",
|
"find_cert_dir",
|
||||||
|
"get_acme_home",
|
||||||
"get_cert_info",
|
"get_cert_info",
|
||||||
"get_cert_paths",
|
"get_cert_paths",
|
||||||
"get_email",
|
"get_email",
|
||||||
|
|||||||
+3
-3
@@ -253,9 +253,9 @@ mkdir -p /etc/dnsmasq
|
|||||||
chmod -R a+rX "${PROJECT_DIR}/webui/static"
|
chmod -R a+rX "${PROJECT_DIR}/webui/static"
|
||||||
# ...and traversal (x only) up the parent chain, so repo-in-$HOME installs work.
|
# ...and traversal (x only) up the parent chain, so repo-in-$HOME installs work.
|
||||||
_d="${PROJECT_DIR}"
|
_d="${PROJECT_DIR}"
|
||||||
while [[ "$d" != "/" && -n "$d" ]]; do
|
while [[ "$_d" != "/" && -n "$_d" ]]; do
|
||||||
chmod a+x "$d" 2>/dev/null || true
|
chmod a+x "$_d" 2>/dev/null || true
|
||||||
d="$(dirname "$d")"
|
_d="$(dirname "$_d")"
|
||||||
done
|
done
|
||||||
# Set ownership: daemon owns project dir in prod, repo owner keeps ownership in dev.
|
# Set ownership: daemon owns project dir in prod, repo owner keeps ownership in dev.
|
||||||
# The top-level .git (directory or worktree pointer file) is left untouched so
|
# The top-level .git (directory or worktree pointer file) is left untouched so
|
||||||
|
|||||||
@@ -53,7 +53,7 @@ server {
|
|||||||
{% endif %}
|
{% endif %}
|
||||||
|
|
||||||
{% for ppath, pcfg in paths.items() %}
|
{% for ppath, pcfg in paths.items() %}
|
||||||
{% if pcfg.is_management and ppath == '/' %}
|
{% if pcfg.is_management %}
|
||||||
# SPA static assets — served from disk, no Flask round-trip.
|
# SPA static assets — served from disk, no Flask round-trip.
|
||||||
# no-cache: browsers revalidate every load; unchanged files are 304s.
|
# no-cache: browsers revalidate every load; unchanged files are 304s.
|
||||||
location /static/ {
|
location /static/ {
|
||||||
|
|||||||
@@ -277,3 +277,35 @@ class TestHasAutoRenew:
|
|||||||
with patch.object(acme, "_ACME_HOME", acme_dir):
|
with patch.object(acme, "_ACME_HOME", acme_dir):
|
||||||
result = acme._has_auto_renew("nonexistent.com")
|
result = acme._has_auto_renew("nonexistent.com")
|
||||||
assert result is False
|
assert result is False
|
||||||
|
|
||||||
|
|
||||||
|
class TestSummarizeAcmeOutput:
|
||||||
|
def test_last_two_lines(self):
|
||||||
|
out = (
|
||||||
|
"[2026-09-04] line one\n[2026-09-04] retry failed\n[2026-09-04] giving up\n"
|
||||||
|
)
|
||||||
|
assert acme._summarize_acme_output(out) == "retry failed; giving up"
|
||||||
|
|
||||||
|
def test_strips_timestamps_and_log_pointer(self):
|
||||||
|
out = "[ts] work\nPlease check log file /x/acme.sh.log\n[ts] done\n"
|
||||||
|
assert acme._summarize_acme_output(out) == "work; done"
|
||||||
|
|
||||||
|
def test_empty_returns_placeholder(self):
|
||||||
|
assert acme._summarize_acme_output("") == "(no output)"
|
||||||
|
|
||||||
|
def test_preserves_permission_denied_outside_tail(self):
|
||||||
|
out = (
|
||||||
|
"[ts] starting\n"
|
||||||
|
"[ts] /data/acme/account.conf: Permission denied\n"
|
||||||
|
"[ts] step three\n"
|
||||||
|
"[ts] step four\n"
|
||||||
|
)
|
||||||
|
summary = acme._summarize_acme_output(out)
|
||||||
|
# The permission line is not among the final two, but the
|
||||||
|
# actionable-error matcher (daemon/collectors/acme.py) keys off it.
|
||||||
|
assert "account.conf: Permission denied" in summary
|
||||||
|
assert summary.count("; ") == 2 # capped at three lines
|
||||||
|
|
||||||
|
def test_permission_denied_in_tail_not_duplicated(self):
|
||||||
|
out = "[ts] ok\n[ts] account.conf: Permission denied\n"
|
||||||
|
assert acme._summarize_acme_output(out) == "ok; account.conf: Permission denied"
|
||||||
|
|||||||
@@ -349,6 +349,26 @@ class TestGenerateServerConf:
|
|||||||
out = nginx.generate_server_conf(cfg)
|
out = nginx.generate_server_conf(cfg)
|
||||||
assert "location /static/" not in out
|
assert "location /static/" not in out
|
||||||
|
|
||||||
|
def test_management_static_location_on_subpath(self, temp_data_dir):
|
||||||
|
# The SPA references /static/... at the domain root regardless of the
|
||||||
|
# management backend path, so the block is emitted for any
|
||||||
|
# is_management path, not only '/'.
|
||||||
|
cfg = {
|
||||||
|
"domain": "mgmt.example.com",
|
||||||
|
"paths": {
|
||||||
|
"/app": {
|
||||||
|
"backend": {"host": "127.0.0.1", "port": 9090, "proto": "http"},
|
||||||
|
"is_management": True,
|
||||||
|
},
|
||||||
|
},
|
||||||
|
"force_ssl": True,
|
||||||
|
"cert": "acme",
|
||||||
|
}
|
||||||
|
out = nginx.generate_server_conf(cfg)
|
||||||
|
assert "location /static/ {" in out
|
||||||
|
static_root = str(nginx.PROJECT_DIR / "webui" / "static")
|
||||||
|
assert f"alias {static_root}/;" in out
|
||||||
|
|
||||||
def test_websocket_path(self, temp_data_dir):
|
def test_websocket_path(self, temp_data_dir):
|
||||||
cfg = {
|
cfg = {
|
||||||
"domain": "mgmt.example.com",
|
"domain": "mgmt.example.com",
|
||||||
|
|||||||
+78
-2
@@ -1,6 +1,7 @@
|
|||||||
"""Tests for lib/state.py — state store and collect functions."""
|
"""Tests for lib/state.py — state store and collect functions."""
|
||||||
|
|
||||||
import json
|
import json
|
||||||
|
import os
|
||||||
from unittest.mock import patch
|
from unittest.mock import patch
|
||||||
|
|
||||||
import daemon.collectors.acme
|
import daemon.collectors.acme
|
||||||
@@ -267,6 +268,9 @@ class TestAcmeCollectNonFatal:
|
|||||||
patch.object(
|
patch.object(
|
||||||
daemon.collectors.acme, "_get_acme_email", return_value="a@b.c"
|
daemon.collectors.acme, "_get_acme_email", return_value="a@b.c"
|
||||||
),
|
),
|
||||||
|
patch(
|
||||||
|
"daemon.collectors.acme._acme_home_needs_normalize", return_value=True
|
||||||
|
),
|
||||||
patch("daemon.handlers.acme.normalize_acme_home"),
|
patch("daemon.handlers.acme.normalize_acme_home"),
|
||||||
patch(
|
patch(
|
||||||
"lib.acme.list_certs",
|
"lib.acme.list_certs",
|
||||||
@@ -290,6 +294,9 @@ class TestAcmeCollectNonFatal:
|
|||||||
patch.object(
|
patch.object(
|
||||||
daemon.collectors.acme, "_get_acme_email", return_value="a@b.c"
|
daemon.collectors.acme, "_get_acme_email", return_value="a@b.c"
|
||||||
),
|
),
|
||||||
|
patch(
|
||||||
|
"daemon.collectors.acme._acme_home_needs_normalize", return_value=True
|
||||||
|
),
|
||||||
patch("daemon.handlers.acme.normalize_acme_home"),
|
patch("daemon.handlers.acme.normalize_acme_home"),
|
||||||
patch("lib.acme.list_certs", return_value=[]),
|
patch("lib.acme.list_certs", return_value=[]),
|
||||||
patch.object(
|
patch.object(
|
||||||
@@ -316,6 +323,9 @@ class TestAcmeCollectNonFatal:
|
|||||||
patch.object(
|
patch.object(
|
||||||
daemon.collectors.acme, "_get_acme_email", return_value="a@b.c"
|
daemon.collectors.acme, "_get_acme_email", return_value="a@b.c"
|
||||||
),
|
),
|
||||||
|
patch(
|
||||||
|
"daemon.collectors.acme._acme_home_needs_normalize", return_value=True
|
||||||
|
),
|
||||||
patch("daemon.handlers.acme.normalize_acme_home", side_effect=_norm),
|
patch("daemon.handlers.acme.normalize_acme_home", side_effect=_norm),
|
||||||
patch("lib.acme.list_certs", side_effect=_list),
|
patch("lib.acme.list_certs", side_effect=_list),
|
||||||
patch.object(
|
patch.object(
|
||||||
@@ -324,12 +334,78 @@ class TestAcmeCollectNonFatal:
|
|||||||
):
|
):
|
||||||
result = _collect_acme()
|
result = _collect_acme()
|
||||||
|
|
||||||
# The poll must normalize ACME_HOME perms before listing, so a
|
# The poll must normalize ACME_HOME perms before listing when the
|
||||||
# mid-lifetime ownership flip self-heals without a restart.
|
# probe detects a lost group-read bit, so a mid-lifetime ownership
|
||||||
|
# flip self-heals without a restart.
|
||||||
assert order == ["normalize", "list"]
|
assert order == ["normalize", "list"]
|
||||||
assert result["certs"] == [{"domain": "example.com"}]
|
assert result["certs"] == [{"domain": "example.com"}]
|
||||||
assert result["status"] == {"error": None}
|
assert result["status"] == {"error": None}
|
||||||
|
|
||||||
|
def test_no_normalize_when_probe_clean(self):
|
||||||
|
from daemon.collectors.acme import _collect_acme
|
||||||
|
|
||||||
|
order: list[str] = []
|
||||||
|
|
||||||
|
def _norm():
|
||||||
|
order.append("normalize")
|
||||||
|
|
||||||
|
def _list():
|
||||||
|
order.append("list")
|
||||||
|
return []
|
||||||
|
|
||||||
|
with (
|
||||||
|
patch.object(
|
||||||
|
daemon.collectors.acme, "_get_acme_email", return_value="a@b.c"
|
||||||
|
),
|
||||||
|
patch(
|
||||||
|
"daemon.collectors.acme._acme_home_needs_normalize", return_value=False
|
||||||
|
),
|
||||||
|
patch("daemon.handlers.acme.normalize_acme_home", side_effect=_norm),
|
||||||
|
patch("lib.acme.list_certs", side_effect=_list),
|
||||||
|
patch.object(
|
||||||
|
daemon.collectors.acme, "_parse_account_conf", return_value=_ACCOUNT
|
||||||
|
),
|
||||||
|
):
|
||||||
|
result = _collect_acme()
|
||||||
|
|
||||||
|
# Steady state: the probe sees group-read bits intact, so the poll
|
||||||
|
# must not pay for a sudo normalize.
|
||||||
|
assert order == ["list"]
|
||||||
|
assert result["certs"] == []
|
||||||
|
assert result["status"] == {"error": None}
|
||||||
|
|
||||||
|
|
||||||
|
class TestAcmeHomeProbe:
|
||||||
|
"""_acme_home_needs_normalize probes the group-read bit without sudo."""
|
||||||
|
|
||||||
|
def test_flags_file_without_group_read(self, tmp_path, monkeypatch):
|
||||||
|
from daemon.collectors.acme import _acme_home_needs_normalize
|
||||||
|
|
||||||
|
monkeypatch.setenv("ACME_HOME", str(tmp_path))
|
||||||
|
(tmp_path / "account.conf").write_text("x")
|
||||||
|
os.chmod(tmp_path / "account.conf", 0o600)
|
||||||
|
assert _acme_home_needs_normalize() is True
|
||||||
|
|
||||||
|
def test_clean_when_group_read_set(self, tmp_path, monkeypatch):
|
||||||
|
from daemon.collectors.acme import _acme_home_needs_normalize
|
||||||
|
|
||||||
|
monkeypatch.setenv("ACME_HOME", str(tmp_path))
|
||||||
|
(tmp_path / "account.conf").write_text("x")
|
||||||
|
os.chmod(tmp_path / "account.conf", 0o640)
|
||||||
|
assert _acme_home_needs_normalize() is False
|
||||||
|
|
||||||
|
def test_clean_on_empty_home(self, tmp_path, monkeypatch):
|
||||||
|
from daemon.collectors.acme import _acme_home_needs_normalize
|
||||||
|
|
||||||
|
monkeypatch.setenv("ACME_HOME", str(tmp_path))
|
||||||
|
assert _acme_home_needs_normalize() is False
|
||||||
|
|
||||||
|
def test_clean_on_missing_home(self, tmp_path, monkeypatch):
|
||||||
|
from daemon.collectors.acme import _acme_home_needs_normalize
|
||||||
|
|
||||||
|
monkeypatch.setenv("ACME_HOME", str(tmp_path / "does-not-exist"))
|
||||||
|
assert _acme_home_needs_normalize() is False
|
||||||
|
|
||||||
def test_permission_error_is_actionable(self):
|
def test_permission_error_is_actionable(self):
|
||||||
from daemon.collectors.acme import _collect_acme
|
from daemon.collectors.acme import _collect_acme
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user