fix: critical bugs + security hardening

Phase 1 (critical bugs):
- Fix firewall import string-to-list bug (system_import.py)
- Add rich rules removal in firewall config apply (handlers/firewall.py)

Phase 2 (security hardening):
- Restrict sudo wildcards to specific paths (sudoers.d/vacuum-walld)
- Fix TOCTOU: use /run/vacuum-wall/ for temp files (nginx, dnsmasq, network handlers)
- Remove unnecessary sudo from wg genkey/pubkey (handlers/wireguard.py)

Phase 3 (validation):
- Validate poll intervals > 0 (daemon/server.py)
- Restrict sysctl to whitelisted parameters (handlers/network.py)

Phase 4 (defensive programming):
- Enforce shell=False in run() and run_proc() (lib/common.py)
- Track issuance tasks for graceful shutdown (handlers/acme.py)
- Add nginx template marker consistency tests (tests/test_system_import.py)
This commit is contained in:
2026-07-10 17:05:37 +00:00
parent 803258cf18
commit 05524f3756
27 changed files with 1805 additions and 521 deletions
+1 -1
View File
@@ -462,7 +462,7 @@ class TestProxyDomains:
mock_post.return_value = {"domain": "ex.com"}
resp = client.post(
"/api/proxy/domains",
json={"domain": "ex.com", "backend_host": "10.0.0.1", "backend_port": 80},
json={"domain": "ex.com", "backend": "webui"},
)
assert resp.status_code == 200
+632 -25
View File
@@ -59,13 +59,27 @@ class TestGetConfig:
class TestSaveConfig:
def test_saves_and_reloads(self, temp_data_dir):
cfg = {
"domains": {"example.com": {"backend": {"host": "localhost", "port": 80}}}
"backends": {
"myapp": {
"label": "My App",
"paths": {
"/": {
"backend": {
"host": "localhost",
"port": 80,
"proto": "http",
}
}
},
}
},
"domains": {"example.com": {"backend": "myapp"}},
}
nginx.save_config(cfg)
loaded = nginx.get_config()
assert loaded["domains"]["example.com"]["backend"] == "myapp"
assert (
loaded["domains"]["example.com"]["paths"]["/"]["backend"]["host"]
== "localhost"
loaded["backends"]["myapp"]["paths"]["/"]["backend"]["host"] == "localhost"
)
@@ -76,46 +90,76 @@ class TestGetDomains:
def test_returns_domain_list(self, temp_data_dir):
cfg = {
"domains": {
"example.com": {
"backend": {"host": "localhost", "port": 8080, "proto": "http"},
"force_ssl": True,
"backends": {
"myapp": {
"label": "My App",
"paths": {
"/": {
"backend": {
"host": "localhost",
"port": 8080,
"proto": "http",
}
},
"/api": {
"backend": {
"host": "localhost",
"port": 8081,
"proto": "http",
}
},
},
}
}
},
"domains": {
"example.com": {"backend": "myapp", "force_ssl": True},
},
}
nginx.save_config(cfg)
result = nginx.get_domains()
assert len(result) == 1
assert len(result) == 2
assert result[0]["domain"] == "example.com"
assert result[0]["backend_name"] == "myapp"
assert result[0]["path"] == "/"
class TestAddDomain:
@patch("lib.nginx.get_config")
def test_add_domain(self, mock_get, temp_data_dir):
mock_get.return_value = {
def test_add_domain(self, temp_data_dir):
cfg = {
"backends": {
"myapp": {
"label": "My App",
"paths": {
"/": {
"backend": {
"host": "10.0.0.5",
"port": 8080,
"proto": "http",
}
}
},
}
},
"domains": {},
"management": None,
"ssl": {"protocols": "TLSv1.2 TLSv1.3"},
}
nginx.add_domain("example.com", "10.0.0.5", 8080)
cfg = nginx.get_config()
assert "example.com" in cfg["domains"]
assert (
cfg["domains"]["example.com"]["paths"]["/"]["backend"]["host"] == "10.0.0.5"
)
assert cfg["domains"]["example.com"]["paths"]["/"]["backend"]["port"] == 8080
nginx.save_config(cfg)
nginx.add_domain("example.com", "myapp")
loaded = nginx.get_config()
assert "example.com" in loaded["domains"]
assert loaded["domains"]["example.com"]["backend"] == "myapp"
assert loaded["domains"]["example.com"]["force_ssl"] is True
@patch("lib.nginx.get_config")
def test_duplicate_domain_raises(self, mock_get, temp_data_dir):
mock_get.return_value = {
"domains": {
"example.com": {"backend": {"host": "x", "port": 80, "proto": "http"}}
},
"management": None,
"backends": {"myapp": {}},
"domains": {"example.com": {"backend": "myapp"}},
"ssl": {"protocols": "TLSv1.2 TLSv1.3"},
}
nginx.save_config(mock_get.return_value)
with pytest.raises(ValueError):
nginx.add_domain("example.com", "10.0.0.5", 8080)
nginx.add_domain("example.com", "myapp")
class TestRemoveDomain:
@@ -380,3 +424,566 @@ class TestHashPasswordFallback:
result = nginx._hash_password("test")
assert isinstance(result, str)
assert len(result) > 0
# ---------------------------------------------------------------------------
# Backend CRUD and resolution tests (daemon handler)
# NOTE: The daemon handler modules cannot be called as-is because they
# require sudo and live service paths. Instead we test through `lib.nginx`
# public API where possible, and unit-test the handler-internal functions
# by importing them directly.
# ---------------------------------------------------------------------------
class TestResolvePaths:
def test_resolves_from_backend(self, temp_data_dir):
"""Paths resolve from backends[domain_cfg['backend']].paths."""
backends = {
"myapp": {
"label": "My App",
"paths": {
"/": {"backend": {"host": "a", "port": 80, "proto": "http"}},
"/api": {"backend": {"host": "b", "port": 8080, "proto": "http"}},
},
}
}
domain_cfg = {"backend": "myapp"}
resolved = nginx._resolve_paths(domain_cfg, backends)
assert resolved == backends["myapp"]["paths"]
def test_returns_empty_when_backend_missing(self, temp_data_dir):
"""Empty dict when backend ref not found in backends."""
resolved = nginx._resolve_paths({"backend": "nonexistent"}, {})
assert resolved == {}
def test_returns_empty_when_no_backend_key(self, temp_data_dir):
"""Empty dict when domain has no backend key."""
resolved = nginx._resolve_paths({}, {"myapp": {"paths": {"/": {}}}})
assert resolved == {}
class TestResolveAuth:
def test_domain_auth_wins(self, temp_data_dir):
"""Domain-level auth overrides backend auth."""
backends = {
"myapp": {"auth": {"user": "backend", "htpasswd": "/backend/.htpasswd"}}
}
domain_cfg = {
"backend": "myapp",
"auth": {"user": "domain", "htpasswd": "/domain/.htpasswd"},
}
resolved = nginx._resolve_auth(domain_cfg, backends)
assert resolved == {"user": "domain", "htpasswd": "/domain/.htpasswd"}
def test_domain_auth_null_disables(self, temp_data_dir):
"""Domain auth set to None disables all auth."""
backends = {
"myapp": {"auth": {"user": "backend", "htpasswd": "/backend/.htpasswd"}}
}
domain_cfg = {"backend": "myapp", "auth": None}
resolved = nginx._resolve_auth(domain_cfg, backends)
assert resolved is None
def test_backend_auth_applies(self, temp_data_dir):
"""Backend auth applies when domain has no auth key."""
backends = {
"myapp": {"auth": {"user": "backend", "htpasswd": "/backend/.htpasswd"}}
}
domain_cfg = {"backend": "myapp"}
resolved = nginx._resolve_auth(domain_cfg, backends)
assert resolved == {"user": "backend", "htpasswd": "/backend/.htpasswd"}
def test_no_auth_when_absent_everywhere(self, temp_data_dir):
"""None when neither domain nor backend define auth."""
backends = {"myapp": {"paths": {}}}
domain_cfg = {"backend": "myapp"}
resolved = nginx._resolve_auth(domain_cfg, backends)
assert resolved is None
class TestBackendCRUD:
"""Test daemon handler backend CRUD operations directly."""
def test_validate_paths_valid(self, temp_data_dir):
"""_validate_paths accepts correct schemas."""
from daemon.handlers.nginx import _validate_paths
paths = {
"/": {"backend": {"host": "a", "port": 80, "proto": "http"}},
"/ws": {
"backend": {"host": "b", "port": 9091, "proto": "http"},
"is_websocket": True,
},
}
_validate_paths(paths) # no exception
def test_validate_paths_missing_host(self, temp_data_dir):
"""_validate_paths raises when backend.host missing."""
from daemon.handlers.nginx import _validate_paths
with pytest.raises(ValueError, match="missing 'host'"):
_validate_paths({"/": {"backend": {"port": 80, "proto": "http"}}})
def test_validate_paths_missing_port(self, temp_data_dir):
"""_validate_paths raises when backend.port missing."""
from daemon.handlers.nginx import _validate_paths
with pytest.raises(ValueError, match="missing 'port'"):
_validate_paths({"/": {"backend": {"host": "a", "proto": "http"}}})
def test_validate_paths_missing_proto(self, temp_data_dir):
"""_validate_paths raises when backend.proto missing."""
from daemon.handlers.nginx import _validate_paths
with pytest.raises(ValueError, match="missing 'proto'"):
_validate_paths({"/": {"backend": {"host": "a", "port": 80}}})
def test_validate_paths_no_backend(self, temp_data_dir):
"""_validate_paths raises when path lacks backend dict."""
from daemon.handlers.nginx import _validate_paths
with pytest.raises(ValueError, match="missing 'backend'"):
_validate_paths({"/": {"something": "else"}})
@patch("daemon.handlers.nginx._get_config")
@patch("daemon.handlers.nginx._save_config")
def test_add_backend(self, mock_save, mock_get, temp_data_dir):
"""_add_backend creates a new backend entry."""
from daemon.handlers.nginx import _add_backend
mock_get.return_value = {
"backends": {},
"domains": {},
"ssl": {},
}
_add_backend(
"test",
"Test Label",
{
"/": {"backend": {"host": "a", "port": 80, "proto": "http"}},
},
)
saved_cfg = mock_save.call_args[0][0]
assert "test" in saved_cfg["backends"]
assert saved_cfg["backends"]["test"]["label"] == "Test Label"
@patch("daemon.handlers.nginx._get_config")
@patch("daemon.handlers.nginx._save_config")
def test_add_backend_duplicate_raises(self, mock_save, mock_get, temp_data_dir):
"""_add_backend raises ValueError for duplicate name."""
from daemon.handlers.nginx import _add_backend
mock_get.return_value = {
"backends": {"test": {"label": "Existing"}},
"domains": {},
"ssl": {},
}
with pytest.raises(ValueError, match="already exists"):
_add_backend(
"test",
"New Label",
{
"/": {"backend": {"host": "a", "port": 80, "proto": "http"}},
},
)
@patch("daemon.handlers.nginx._get_config")
@patch("daemon.handlers.nginx._save_config")
def test_update_backend(self, mock_save, mock_get, temp_data_dir):
"""_update_backend modifies label and paths."""
from daemon.handlers.nginx import _update_backend
mock_get.return_value = {
"backends": {
"test": {
"label": "Old",
"paths": {
"/": {"backend": {"host": "a", "port": 80, "proto": "http"}}
},
}
},
"domains": {},
"ssl": {},
}
_update_backend(
"test",
label="New",
paths={
"/api": {"backend": {"host": "b", "port": 9000, "proto": "http"}},
},
)
saved_cfg = mock_save.call_args[0][0]
assert saved_cfg["backends"]["test"]["label"] == "New"
assert "/api" in saved_cfg["backends"]["test"]["paths"]
@patch("daemon.handlers.nginx._get_config")
def test_update_backend_notfound_raises(self, mock_get, temp_data_dir):
"""_update_backend raises KeyError for unknown backend."""
from daemon.handlers.nginx import _update_backend
mock_get.return_value = {"backends": {}, "domains": {}, "ssl": {}}
with pytest.raises(KeyError):
_update_backend("nonexistent", label="X")
@patch("daemon.handlers.nginx._get_config")
def test_update_backend_builtin_raises(self, mock_get, temp_data_dir):
"""_update_backend raises ValueError for builtin backend."""
from daemon.handlers.nginx import _update_backend
mock_get.return_value = {
"backends": {"webui": {"label": "WebUI", "builtin": True, "paths": {}}},
"domains": {},
"ssl": {},
}
with pytest.raises(ValueError, match="Cannot modify"):
_update_backend("webui", label="Hacked")
@patch("daemon.handlers.nginx._get_config")
@patch("daemon.handlers.nginx._save_config")
def test_remove_backend(self, mock_save, mock_get, temp_data_dir):
"""_remove_backend deletes non-builtin backend."""
from daemon.handlers.nginx import _remove_backend
mock_get.return_value = {
"backends": {"test": {"label": "Test", "paths": {}}},
"domains": {},
"ssl": {},
}
_remove_backend("test")
saved_cfg = mock_save.call_args[0][0]
assert "test" not in saved_cfg["backends"]
@patch("daemon.handlers.nginx._get_config")
def test_remove_backend_notfound_raises(self, mock_get, temp_data_dir):
"""_remove_backend raises KeyError for unknown backend."""
from daemon.handlers.nginx import _remove_backend
mock_get.return_value = {"backends": {}, "domains": {}, "ssl": {}}
with pytest.raises(KeyError):
_remove_backend("nonexistent")
@patch("daemon.handlers.nginx._get_config")
def test_remove_backend_builtin_raises(self, mock_get, temp_data_dir):
"""_remove_backend raises ValueError for builtin backend."""
from daemon.handlers.nginx import _remove_backend
mock_get.return_value = {
"backends": {"webui": {"label": "WebUI", "builtin": True, "paths": {}}},
"domains": {},
"ssl": {},
}
with pytest.raises(ValueError, match="Cannot remove"):
_remove_backend("webui")
@patch("daemon.handlers.nginx._get_config")
def test_remove_backend_referenced_raises_conflict(self, mock_get, temp_data_dir):
"""_remove_backend raises ConflictError when domains reference it."""
from daemon.handlers.nginx import _remove_backend
from daemon.server import ConflictError
mock_get.return_value = {
"backends": {"myapp": {"label": "App", "paths": {}}},
"domains": {"example.com": {"backend": "myapp"}},
"ssl": {},
}
with pytest.raises(ConflictError, match="referenced by domain"):
_remove_backend("myapp")
class TestMigration:
def test_ensure_webui_backend_creates(self, temp_data_dir):
"""_ensure_webui_backend creates webui backend if missing."""
cfg = {}
nginx._ensure_webui_backend(cfg)
assert "webui" in cfg["backends"]
assert cfg["backends"]["webui"]["_migrated"] is True
assert cfg["backends"]["webui"]["label"] == "Vacuum Wall WebUI"
assert cfg["backends"]["webui"]["builtin"] is True
def test_ensure_webui_backend_skips_migrated(self, temp_data_dir):
"""_ensure_webui_backend skips if _migrated is true."""
cfg = {
"backends": {
"webui": {
"label": "Custom",
"_migrated": True,
"paths": {},
}
}
}
nginx._ensure_webui_backend(cfg)
# Label unchanged — not recreated
assert cfg["backends"]["webui"]["label"] == "Custom"
def test_migrate_mgmt_domains_application_webui(self, temp_data_dir):
"""Domains with application='webui' and mgmt paths get backend='webui'."""
cfg = {
"backends": {"webui": {"_migrated": True, "paths": {}}},
"domains": {
"mgmt.example.com": {
"application": "webui",
"paths": {
"/": {"is_management": True},
"/ws": {"is_websocket": True},
},
"auth": {"user": "admin"},
}
},
}
nginx._migrate_mgmt_domains(cfg)
dom = cfg["domains"]["mgmt.example.com"]
assert dom["backend"] == "webui"
assert "application" not in dom
assert "paths" not in dom
assert "auth" not in dom
def test_migrate_mgmt_domains_strips_application_only(self, temp_data_dir):
"""application='webui' stripped even when paths don't match mgmt shape."""
cfg = {
"backends": {"webui": {"_migrated": True, "paths": {}}},
"domains": {
"odd.example.com": {
"application": "webui",
"paths": {"/": {"is_management": True}},
}
},
}
nginx._migrate_mgmt_domains(cfg)
dom = cfg["domains"]["odd.example.com"]
assert "application" not in dom
assert "backend" not in dom # paths didn't match full mgmt shape
assert "paths" in dom # not removed
def test_migrate_mgmt_domains_detects_webui_paths(self, temp_data_dir):
"""Domains matching webui path shape get migrated to backend."""
cfg = {
"backends": {"webui": {"_migrated": True, "paths": {}}},
"domains": {
"mgmt.example.com": {
"paths": {
"/": {
"backend": {
"host": "127.0.0.1",
"port": 9090,
"proto": "http",
},
"is_management": True,
},
"/ws": {
"backend": {
"host": "127.0.0.1",
"port": 9091,
"proto": "http",
},
"is_websocket": True,
},
}
}
},
}
nginx._migrate_mgmt_domains(cfg)
dom = cfg["domains"]["mgmt.example.com"]
assert dom["backend"] == "webui"
assert "paths" not in dom
def test_migrate_mgmt_domains_skips_non_mgmt(self, temp_data_dir):
"""Non-management domains are left untouched."""
cfg = {
"backends": {"webui": {"_migrated": True, "paths": {}}},
"domains": {
"app.example.com": {
"paths": {
"/": {
"backend": {"host": "10.0.0.1", "port": 80, "proto": "http"}
}
}
}
},
}
nginx._migrate_mgmt_domains(cfg)
dom = cfg["domains"]["app.example.com"]
assert "backend" not in dom
assert "paths" in dom
def test_migrate_config_full(self, temp_data_dir):
"""_migrate_config runs _ensure_webui_backend then _migrate_mgmt_domains."""
cfg = {
"domains": {
"mgmt.example.com": {
"application": "webui",
"paths": {
"/": {"is_management": True},
"/ws": {"is_websocket": True},
},
}
}
}
result = nginx._migrate_config(cfg)
assert result is cfg
assert "webui" in cfg["backends"]
assert cfg["domains"]["mgmt.example.com"]["backend"] == "webui"
class TestDomainSwap:
"""Domain can change which backend it references."""
def test_swap_backend(self, temp_data_dir):
"""update_domain allows changing backend field."""
cfg = {
"backends": {
"webui": {"label": "WebUI", "paths": {}},
"myapp": {"label": "App", "paths": {}},
},
"domains": {"example.com": {"backend": "webui", "force_ssl": True}},
"ssl": {"protocols": "TLSv1.2 TLSv1.3"},
}
nginx.save_config(cfg)
nginx.update_domain("example.com", backend="myapp")
loaded = nginx.get_config()
assert loaded["domains"]["example.com"]["backend"] == "myapp"
def test_swap_backend_nonexistent_raises(self, temp_data_dir):
"""update_domain raises ValueError when new backend not found."""
cfg = {
"backends": {"webui": {"label": "WebUI", "paths": {}}},
"domains": {"example.com": {"backend": "webui"}},
"ssl": {"protocols": "TLSv1.2 TLSv1.3"},
}
nginx.save_config(cfg)
with pytest.raises(ValueError, match="Backend 'nonexistent' not found"):
nginx.update_domain("example.com", backend="nonexistent")
class TestGetDomainsWithBackends:
"""get_domains resolves paths from backends, includes backend_name."""
def test_flattens_by_backend_paths(self, temp_data_dir):
"""Each backend path becomes a separate entry."""
cfg = {
"backends": {
"myapp": {
"label": "My App",
"paths": {
"/": {"backend": {"host": "a", "port": 80, "proto": "http"}},
"/api": {
"backend": {"host": "b", "port": 8080, "proto": "http"}
},
"/ws": {
"backend": {"host": "c", "port": 9091, "proto": "http"},
"is_websocket": True,
},
},
}
},
"domains": {"example.com": {"backend": "myapp", "force_ssl": True}},
"ssl": {},
}
nginx.save_config(cfg)
result = nginx.get_domains()
assert len(result) == 3
paths = {r["path"] for r in result}
assert paths == {"/", "/api", "/ws"}
for r in result:
assert r["backend_name"] == "myapp"
assert r["domain"] == "example.com"
def test_skips_domains_without_backend(self, temp_data_dir):
"""Domains without a backend key are skipped."""
cfg = {
"backends": {},
"domains": {
"good.com": {"backend": "webui"},
"bad.com": {"some": "orphan"},
},
"ssl": {},
}
nginx.save_config(cfg)
result = nginx.get_domains()
domains = {r["domain"] for r in result}
assert "good.com" in domains
assert "bad.com" not in domains
def test_includes_management_and_websocket_flags(self, temp_data_dir):
"""is_management and is_websocket flags propagate to entries."""
cfg = {
"backends": {
"webui": {
"label": "WebUI",
"paths": {
"/": {
"backend": {
"host": "127.0.0.1",
"port": 9090,
"proto": "http",
},
"is_management": True,
},
"/ws": {
"backend": {
"host": "127.0.0.1",
"port": 9091,
"proto": "http",
},
"is_websocket": True,
},
},
}
},
"domains": {"mgmt.local": {"backend": "webui"}},
"ssl": {},
}
nginx.save_config(cfg)
result = nginx.get_domains()
entries_by_path = {r["path"]: r for r in result}
assert entries_by_path["/"]["is_management"] is True
assert entries_by_path["/ws"]["is_websocket"] is True
class TestGenerateServerConfWithBackends:
"""generate_server_conf resolves paths via backends parameter."""
def test_with_backends_param(self, temp_data_dir):
"""When backends provided, paths resolve from backend."""
backends = {
"myapp": {
"paths": {
"/": {
"backend": {"host": "10.0.0.1", "port": 8080, "proto": "http"}
}
}
}
}
domain_cfg = {"domain": "example.com", "backend": "myapp", "force_ssl": True}
out = nginx.generate_server_conf(domain_cfg, backends)
assert "proxy_pass http://10.0.0.1:8080;" in out
class TestPatchConfigPreservesBackends:
"""PATCH_NGINX_CONFIG (deep_merge) does not overwrite backends fully."""
def test_ssl_patch_keeps_backends(self, temp_data_dir):
"""Patching ssl settings preserves backends dict (deep_merge semantics)."""
from lib.common import deep_merge
current = {
"backends": {"myapp": {"label": "App", "paths": {}}},
"domains": {"example.com": {"backend": "myapp"}},
"ssl": {"protocols": "TLSv1.2"},
}
patch = {"ssl": {"protocols": "TLSv1.3"}}
merged = deep_merge(current, patch)
assert merged["backends"]["myapp"]["label"] == "App"
assert merged["domains"]["example.com"]["backend"] == "myapp"
assert merged["ssl"]["protocols"] == "TLSv1.3"
+27
View File
@@ -642,3 +642,30 @@ class TestCfgsEqual:
a = {"dhcp": {"ranges": [{"start": "1.2.3.4"}]}}
b = {"dhcp": {"ranges": [{"start": "1.2.3.4"}]}}
assert system_import._cfgs_equal(a, b)
# ──────────────────────────────────────────────────────────────────────
# Template marker consistency
# ──────────────────────────────────────────────────────────────────────
class TestTemplateMarker:
"""Verify nginx templates contain the expected auto-generated marker."""
def test_acme_challenge_has_marker(self):
content = (
Path(__file__).resolve().parent.parent
/ "system"
/ "nginx"
/ "acme-challenge.conf"
).read_text()
assert "# Auto-generated by Vacuum Wall" in content
def test_server_block_has_marker(self):
content = (
Path(__file__).resolve().parent.parent
/ "system"
/ "nginx"
/ "server_block.conf"
).read_text()
assert "# Auto-generated by Vacuum Wall" in content