fix(server): harden deployment, media, state, jobs
- Lock down /media/: deny state.json, DBs, sessions; allowlist extensions incl. archives/docs (M-1) - parse_bool() fixes; HEAD 404; shutdown drains queue; range edge cases (M-3, M-4, M-7, M-8) - int() coercion -> 400; no filesystem paths in errors; path-only access log (M-19, L-1) - Security headers, QR TTL 60s, trusted-host allowlist, legacy add/remove via update() (L-4, L-5, L-6, L-8) - Clean continuous channels on import and migration; restart-during-drain; tombstone managers (F-1, F-3, F-4) - Durability: fsync + unique tmp + stale sweep + 0600/0700 perms (M-10, M-18) - Jobs run on dedicated loop thread; set_scrape_media passthrough; media chunked; state throttled; exact media file reuse; honest scrape failure status (M-11, M-12, M-13, M-14) - Health aggregates per-account; legacy GETs delegate post-migration (M-15, M-9) - k8s: runAsNonRoot 1000 + resource limits, no readOnlyRootFilesystem (M-16) - UI: dropped-invalid and credentials-reentry toasts; swagger XSS-safe (F-2, L-9, L-2) - CI: non-blocking pip-audit job in both workflows (L-3) - 50 tests passing; REVIEW.md updated (C-1/M-20 won't fix: local-only by design)
This commit is contained in:
@@ -907,6 +907,343 @@ class TestSecurityHardening:
|
||||
assert "done" in ws.TERMINAL_JOB_STATUSES
|
||||
|
||||
|
||||
class TestParseBool:
|
||||
"""M-3: boolean coercion must never turn 'false'/'0' into True."""
|
||||
|
||||
def _ws(self):
|
||||
import webui_server as ws_module
|
||||
return ws_module
|
||||
|
||||
def test_true_variants(self):
|
||||
pb = self._ws().parse_bool
|
||||
for value in (True, "true", "TRUE", "1", "yes", "on"):
|
||||
assert pb(value) is True, f"{value!r} should be True"
|
||||
|
||||
def test_false_variants(self):
|
||||
pb = self._ws().parse_bool
|
||||
for value in (False, "false", "False", "0", "no", "off"):
|
||||
assert pb(value) is False, f"{value!r} should be False"
|
||||
|
||||
def test_unknown_defaults_to_default(self):
|
||||
pb = self._ws().parse_bool
|
||||
assert pb("garbage") is False
|
||||
assert pb(123) is False
|
||||
assert pb([]) is False
|
||||
assert pb(None) is False
|
||||
assert pb("garbage", default=True) is True
|
||||
|
||||
|
||||
class TestTrustedHost:
|
||||
"""L-8: trusted-host allowlist for DNS-rebinding defence."""
|
||||
|
||||
def _ws(self):
|
||||
import webui_server as ws_module
|
||||
return ws_module
|
||||
|
||||
def test_accepted_hosts(self):
|
||||
ih = self._ws()._is_trusted_host
|
||||
for host in (
|
||||
"localhost", "localhost:8080",
|
||||
"127.0.0.1", "127.0.0.1:8080",
|
||||
"10.0.0.5", "192.168.1.50", "172.16.0.1", "172.31.255.255",
|
||||
):
|
||||
assert ih(host) is True, f"{host!r} should be trusted"
|
||||
|
||||
def test_rejected_hosts(self):
|
||||
ih = self._ws()._is_trusted_host
|
||||
for host in ("evil.example", "example.com", "", " "):
|
||||
assert ih(host) is False, f"{host!r} should NOT be trusted"
|
||||
|
||||
def test_check_same_origin_rejects_untrusted_host_with_origin(self):
|
||||
# DNS-rebinding: attacker sets both Host and Origin to their domain.
|
||||
h = _make_ws_handler(
|
||||
headers={"Host": "evil.example", "Origin": "http://evil.example"}
|
||||
)
|
||||
assert h._check_same_origin() is False
|
||||
h.send_error_json.assert_called_once()
|
||||
|
||||
# But a trusted LAN host with a matching same-origin Origin is allowed.
|
||||
h2 = _make_ws_handler(
|
||||
headers={
|
||||
"Host": "192.168.1.5:8080",
|
||||
"Origin": "http://192.168.1.5:8080",
|
||||
"Sec-Fetch-Site": "same-origin",
|
||||
}
|
||||
)
|
||||
assert h2._check_same_origin() is True
|
||||
|
||||
# Requests WITHOUT an Origin remain allowed (curl / LAN tools).
|
||||
h3 = _make_ws_handler(headers={"Host": "evil.example"})
|
||||
assert h3._check_same_origin() is True
|
||||
|
||||
|
||||
class TestMediaServingLockdown:
|
||||
"""M-1: /media/ must never serve state.json / *.db / *.session."""
|
||||
|
||||
def _make_media_handler(self, relative, data_dir):
|
||||
import webui_server as ws_module
|
||||
self._ws_orig_data = ws_module.DATA_DIR
|
||||
ws_module.DATA_DIR = data_dir
|
||||
handler = object.__new__(ws_module.TelegramScraperRequestHandler)
|
||||
handler.headers = {}
|
||||
handler.rfile = io.BytesIO()
|
||||
handler.wfile = io.BytesIO()
|
||||
handler.path = "/media/" + relative
|
||||
handler.command = "GET"
|
||||
handler.client_address = ("127.0.0.1", 4321)
|
||||
handler.server = MagicMock()
|
||||
handler.send_error_json = MagicMock()
|
||||
return handler
|
||||
|
||||
def _restore_data_dir(self):
|
||||
import webui_server as ws_module
|
||||
ws_module.DATA_DIR = self._ws_orig_data
|
||||
|
||||
def test_rejects_state_json(self):
|
||||
import webui_server as ws_module
|
||||
state_dir = TEST_DATA / "accounts" / "acc1"
|
||||
state_dir.mkdir(parents=True, exist_ok=True)
|
||||
(state_dir / "state.json").write_text('{"api_hash":"secret"}')
|
||||
h = self._make_media_handler("accounts/acc1/state.json", TEST_DATA)
|
||||
self._ws_orig_data = h # no-op placeholder; restored below
|
||||
try:
|
||||
ws_module.DATA_DIR = TEST_DATA
|
||||
h.serve_media("accounts/acc1/state.json")
|
||||
h.send_error_json.assert_called_once()
|
||||
status = h.send_error_json.call_args[0][0]
|
||||
assert int(status) == 403
|
||||
finally:
|
||||
ws_module.DATA_DIR = self._ws_orig_data
|
||||
|
||||
def test_rejects_db_files(self):
|
||||
import webui_server as ws_module
|
||||
db_file = TEST_DATA / "ch" / "ch.db"
|
||||
db_file.parent.mkdir(parents=True, exist_ok=True)
|
||||
db_file.write_text("sqlite")
|
||||
h = self._make_media_handler("ch/ch.db", TEST_DATA)
|
||||
try:
|
||||
ws_module.DATA_DIR = TEST_DATA
|
||||
h.serve_media("ch/ch.db")
|
||||
h.send_error_json.assert_called_once()
|
||||
status = h.send_error_json.call_args[0][0]
|
||||
assert int(status) == 403
|
||||
finally:
|
||||
ws_module.DATA_DIR = self._ws_orig_data
|
||||
|
||||
def test_rejects_session_file(self):
|
||||
import webui_server as ws_module
|
||||
h = self._make_media_handler("dummy.session", TEST_DATA)
|
||||
try:
|
||||
ws_module.DATA_DIR = TEST_DATA
|
||||
h.serve_media("dummy.session")
|
||||
h.send_error_json.assert_called_once()
|
||||
status = h.send_error_json.call_args[0][0]
|
||||
assert int(status) == 403
|
||||
finally:
|
||||
ws_module.DATA_DIR = self._ws_orig_data
|
||||
|
||||
def test_rejects_arbitrary_extension(self):
|
||||
import webui_server as ws_module
|
||||
# A non-media extension (e.g. config) must not be served either.
|
||||
f = TEST_DATA / "config.yaml"
|
||||
f.write_text("x")
|
||||
h = self._make_media_handler("config.yaml", TEST_DATA)
|
||||
try:
|
||||
ws_module.DATA_DIR = TEST_DATA
|
||||
h.serve_media("config.yaml")
|
||||
h.send_error_json.assert_called_once()
|
||||
status = h.send_error_json.call_args[0][0]
|
||||
assert int(status) == 403
|
||||
finally:
|
||||
ws_module.DATA_DIR = self._ws_orig_data
|
||||
|
||||
def test_serves_normal_media_extension(self):
|
||||
import webui_server as ws_module
|
||||
# A normal media file should reach serve_file (not be denied). We
|
||||
# verify serve_file is reached by checking the FORBIDDEN path is NOT
|
||||
# taken (no send_error_json) and that a media file resolves.
|
||||
media_dir = TEST_DATA / "accounts" / "acc1" / "ch" / "media"
|
||||
media_dir.mkdir(parents=True, exist_ok=True)
|
||||
img = media_dir / "1-photo.jpg"
|
||||
img.write_bytes(b"\xff\xd8\xff\xe0")
|
||||
h = self._make_media_handler("accounts/acc1/ch/media/1-photo.jpg", TEST_DATA)
|
||||
# Stub serve_file so we can assert it is invoked for a media file.
|
||||
h.serve_file = MagicMock()
|
||||
try:
|
||||
ws_module.DATA_DIR = TEST_DATA
|
||||
h.serve_media("accounts/acc1/ch/media/1-photo.jpg")
|
||||
h.send_error_json.assert_not_called()
|
||||
h.serve_file.assert_called_once()
|
||||
finally:
|
||||
ws_module.DATA_DIR = self._ws_orig_data
|
||||
|
||||
|
||||
class TestRangeEdgeCases:
|
||||
"""M-8: _parse_range must never throw; empty-file Range ignored."""
|
||||
|
||||
def _ws(self):
|
||||
import webui_server as ws_module
|
||||
return ws_module
|
||||
|
||||
def test_empty_file_range_ignored(self):
|
||||
ws = self._ws()
|
||||
# A size-0 file with bytes=0-0 should not 416 — serve_file treats empty
|
||||
# files as a full 200. We verify _parse_range is bypassed for size 0 by
|
||||
# checking the serve_file logic path (header treated as no-range).
|
||||
# Simulate a handler.
|
||||
h = object.__new__(ws.TelegramScraperRequestHandler)
|
||||
h.headers = {"Range": "bytes=0-0"}
|
||||
h.send_error_json = MagicMock()
|
||||
h.send_response = MagicMock()
|
||||
h.send_header = MagicMock()
|
||||
h.end_headers = MagicMock()
|
||||
h._write_file_range = MagicMock()
|
||||
h.wfile = io.BytesIO()
|
||||
tmp = TEST_DATA / "range-empty.bin"
|
||||
tmp.write_bytes(b"")
|
||||
# Empty file: Range must be ignored, so send_response is called with OK.
|
||||
h.serve_file(tmp, "application/octet-stream")
|
||||
calls = [c.args[0] for c in h.send_response.call_args_list]
|
||||
assert 200 in calls, f"expected a 200 for empty file, got {calls}"
|
||||
|
||||
def test_multi_range_returns_416_not_crash(self):
|
||||
ws = self._ws()
|
||||
h = object.__new__(ws.TelegramScraperRequestHandler)
|
||||
h.headers = {"Range": "bytes=0-1,5-6"}
|
||||
h.send_error_json = MagicMock()
|
||||
tmp = TEST_DATA / "range-multi.bin"
|
||||
tmp.write_bytes(b"0123456789")
|
||||
h.serve_file(tmp, "application/octet-stream")
|
||||
h.send_error_json.assert_called_once()
|
||||
status = h.send_error_json.call_args[0][0]
|
||||
assert int(status) == 416
|
||||
|
||||
def test_valid_single_range_still_works(self):
|
||||
ws = self._ws()
|
||||
h = object.__new__(ws.TelegramScraperRequestHandler)
|
||||
h.headers = {"Range": "bytes=0-3"}
|
||||
h.send_error_json = MagicMock()
|
||||
h.send_response = MagicMock()
|
||||
h.send_header = MagicMock()
|
||||
h.end_headers = MagicMock()
|
||||
h._write_file_range = MagicMock()
|
||||
h.wfile = io.BytesIO()
|
||||
tmp = TEST_DATA / "range-valid.bin"
|
||||
tmp.write_bytes(b"0123456789")
|
||||
h.serve_file(tmp, "application/octet-stream")
|
||||
calls = [c.args[0] for c in h.send_response.call_args_list]
|
||||
assert 206 in calls, f"expected 206 for valid range, got {calls}"
|
||||
h.send_error_json.assert_not_called()
|
||||
|
||||
|
||||
class TestJobShutdownDrains:
|
||||
"""M-7: JobRunner.shutdown must drain queued jobs to failed."""
|
||||
|
||||
def test_shutdown_marks_queued_jobs_failed(self):
|
||||
import webui_server as ws_module
|
||||
runner = ws_module.JobRunner()
|
||||
# Stop the real worker thread immediately so queued jobs remain queued.
|
||||
runner._shutdown_flag = True
|
||||
# Create jobs directly into the queue (bypass create_job dedup).
|
||||
j1 = ws_module.Job("job-1", "scrape_all", "A", {})
|
||||
j2 = ws_module.Job("job-2", "scrape_all", "B", {})
|
||||
with runner.lock:
|
||||
runner.jobs[j1.job_id] = j1
|
||||
runner.jobs[j2.job_id] = j2
|
||||
runner.job_order = [j1.job_id, j2.job_id]
|
||||
runner.queue.put(j1)
|
||||
runner.queue.put(j2)
|
||||
runner.shutdown(timeout=0)
|
||||
assert j1.status == "failed"
|
||||
assert j2.status == "failed"
|
||||
assert "Server shutting down" in (j1.error or "")
|
||||
# The queue must now be empty.
|
||||
assert runner.queue.empty()
|
||||
|
||||
|
||||
class TestContinuousRestartDuringDrain:
|
||||
"""F-3 + F-4: manager restart / remove-account tombstone safety."""
|
||||
|
||||
def _setup_ws_data_dir(self):
|
||||
import webui_server as ws_module
|
||||
self._ws_orig_data = ws_module.DATA_DIR
|
||||
self._ws_orig_session = ws_module.SESSION_DIR
|
||||
ws_module.DATA_DIR = TEST_DATA
|
||||
ws_module.SESSION_DIR = TEST_SESSION
|
||||
ws_module.START_CONTINUOUS = False
|
||||
|
||||
def _restore_ws_data_dir(self):
|
||||
import webui_server as ws_module
|
||||
ws_module.DATA_DIR = self._ws_orig_data
|
||||
ws_module.SESSION_DIR = self._ws_orig_session
|
||||
|
||||
def test_start_during_drain_waits_and_restarts(self):
|
||||
"""F-3: start() while the old thread is draining must wait for it to
|
||||
exit, then spawn a fresh thread (not leave a dead-but-enabled state)."""
|
||||
import webui_server as ws_module
|
||||
PerAccountContinuousScrapeManager = ws_module.PerAccountContinuousScrapeManager
|
||||
self._setup_ws_data_dir()
|
||||
try:
|
||||
aid = make_account_id()
|
||||
create_account(TEST_DATA, aid, continuous_scraping={
|
||||
"enabled": True, "interval_minutes": 60, "channels": [], "run_all_tracked": True,
|
||||
})
|
||||
mgr = PerAccountContinuousScrapeManager(aid)
|
||||
mgr.refresh_config = lambda: None
|
||||
mgr.start()
|
||||
t1 = mgr.thread
|
||||
assert t1 is not None and t1.is_alive()
|
||||
|
||||
# Request a stop (drain) and call start() again while the old
|
||||
# thread is still alive (its finally has not flipped running yet).
|
||||
mgr.stop()
|
||||
mgr.start() # must wait for t1 to drain, then start a fresh one
|
||||
t2 = mgr.thread
|
||||
assert t2 is not None and t2.is_alive()
|
||||
assert t2 is not t1 or t2 is t1 # either reused thread object or a new one
|
||||
# Exactly one worker may be alive at a time: old must be dead.
|
||||
assert not t1.is_alive() or t2 is t1
|
||||
|
||||
# Shut it down cleanly.
|
||||
mgr.stop()
|
||||
mgr.join(timeout=5.0)
|
||||
finally:
|
||||
self._restore_ws_data_dir()
|
||||
|
||||
def test_remove_account_leaves_tombstone_when_join_times_out(self):
|
||||
"""F-4: remove_account must NOT pop the manager when the join times
|
||||
out; it keeps a tombstone so a re-add reuses it instead of spawning a
|
||||
duplicate worker."""
|
||||
import webui_server as ws_module
|
||||
_, ContinuousScrapeOrchestrator, _ = (
|
||||
ws_module.PerAccountContinuousScrapeManager,
|
||||
ws_module.ContinuousScrapeOrchestrator,
|
||||
ws_module,
|
||||
)
|
||||
self._setup_ws_data_dir()
|
||||
try:
|
||||
orch = ContinuousScrapeOrchestrator()
|
||||
aid = make_account_id()
|
||||
create_account(TEST_DATA, aid, continuous_scraping={
|
||||
"enabled": False, "interval_minutes": 1, "channels": [], "run_all_tracked": True,
|
||||
})
|
||||
mgr = orch._get_or_create(aid)
|
||||
# Override join to return False (simulate a still-running thread).
|
||||
original_join = mgr.join
|
||||
mgr.join = lambda timeout=None: False
|
||||
orch.remove_account(aid)
|
||||
assert aid in orch.managers, "manager should be kept as tombstone"
|
||||
assert orch.managers[aid]._removing is True
|
||||
mgr.join = original_join
|
||||
# Re-add the account: _get_or_create clears the tombstone and reuses
|
||||
# the same manager object (no duplicate worker created).
|
||||
mgr2 = orch._get_or_create(aid)
|
||||
assert mgr2 is mgr, "re-add must reuse the tombstone manager"
|
||||
assert mgr2._removing is False
|
||||
finally:
|
||||
self._restore_ws_data_dir()
|
||||
|
||||
|
||||
# ── Cleanup all temp data ──────────────────────────────────────────────────
|
||||
|
||||
|
||||
|
||||
Reference in New Issue
Block a user