diff --git a/app/sovran_systemsos_web/server.py b/app/sovran_systemsos_web/server.py index c8cd4e8..494d4af 100644 --- a/app/sovran_systemsos_web/server.py +++ b/app/sovran_systemsos_web/server.py @@ -3742,8 +3742,18 @@ async def api_backup_drives(): async def _monitor_backup_subprocess(proc: asyncio.subprocess.Process) -> None: - """Mark status FAILED if backup subprocess exits unexpectedly.""" + """Drain stderr, then mark status FAILED if backup subprocess exits unexpectedly.""" + stderr_chunks: list[bytes] = [] + + async def _drain_stderr() -> None: + if proc.stderr is not None: + async for line in proc.stderr: + stderr_chunks.append(line) + + drain_task = asyncio.create_task(_drain_stderr()) rc = await proc.wait() + await drain_task + if rc == 0: return @@ -3752,7 +3762,9 @@ async def _monitor_backup_subprocess(proc: asyncio.subprocess.Process) -> None: if status in {"SUCCESS", "FAILED"}: return - msg = f"[{time.strftime('%Y-%m-%d %H:%M:%S')}] ERROR: Backup subprocess exited unexpectedly (code {rc})." + stderr_text = b"".join(stderr_chunks).decode("utf-8", errors="replace").strip() + detail = f" — stderr: {stderr_text}" if stderr_text else "" + msg = f"[{time.strftime('%Y-%m-%d %H:%M:%S')}] ERROR: Backup subprocess exited unexpectedly (code {rc}).{detail}" await loop.run_in_executor(None, _append_backup_log, msg) await loop.run_in_executor(None, _write_backup_status, "FAILED") @@ -3808,11 +3820,25 @@ async def api_backup_run(target: str = ""): env = dict(os.environ) env["BACKUP_TARGET"] = selected_target + bash_path = shutil.which("bash") + if bash_path is None: + no_bash_msg = ( + f"[{time.strftime('%Y-%m-%d %H:%M:%S')}] ERROR: Cannot start backup:" + " interpreter 'bash' not found on PATH." + " Ensure pkgs.bash is in the sovran-hub-web service PATH." + ) + await loop.run_in_executor(None, _append_backup_log, no_bash_msg) + await loop.run_in_executor(None, _write_backup_status, "FAILED") + raise HTTPException( + status_code=500, + detail="Backup interpreter (bash) not available. Check service PATH configuration.", + ) + try: proc = await asyncio.create_subprocess_exec( - "/usr/bin/env", "bash", BACKUP_SCRIPT, + bash_path, BACKUP_SCRIPT, stdout=asyncio.subprocess.DEVNULL, - stderr=asyncio.subprocess.DEVNULL, + stderr=asyncio.subprocess.PIPE, env=env, ) except Exception as exc: diff --git a/app/tests/test_manual_backup_workflow.py b/app/tests/test_manual_backup_workflow.py index 109bc8c..27ada9d 100644 --- a/app/tests/test_manual_backup_workflow.py +++ b/app/tests/test_manual_backup_workflow.py @@ -1,3 +1,4 @@ +import asyncio import unittest from pathlib import Path @@ -6,6 +7,7 @@ REPO_ROOT = Path(__file__).resolve().parents[2] BACKUP_SCRIPT = REPO_ROOT / "app" / "sovran_systemsos_web" / "scripts" / "sovran-hub-backup.sh" SERVER_FILE = REPO_ROOT / "app" / "sovran_systemsos_web" / "server.py" SUPPORT_JS = REPO_ROOT / "app" / "sovran_systemsos_web" / "static" / "js" / "support.js" +NIX_HUB_FILE = REPO_ROOT / "modules" / "core" / "sovran-hub.nix" class ManualBackupWorkflowTests(unittest.TestCase): @@ -36,6 +38,104 @@ class ManualBackupWorkflowTests(unittest.TestCase): self.assertIn("asyncio.create_task(_monitor_backup_subprocess(proc))", server_source) self.assertIn("result === \"success\" || result === \"failed\"", support_source) + # ── Regression tests for exit-code-127 / missing-interpreter bug ────────── + + def test_nix_service_path_includes_bash_and_gawk(self): + """Regression: pkgs.bash and pkgs.gawk must appear in the sovran-hub-web + service path so that the backup shell script and its awk calls can be + resolved at runtime without producing exit code 127.""" + nix_source = NIX_HUB_FILE.read_text() + self.assertIn( + "pkgs.bash", + nix_source, + "pkgs.bash must be declared in the sovran-hub-web systemd service path", + ) + self.assertIn( + "pkgs.gawk", + nix_source, + "pkgs.gawk must be declared in the sovran-hub-web systemd service path", + ) + + def test_server_resolves_bash_via_shutil_which(self): + """Regression: server must locate bash with shutil.which() rather than + relying on /usr/bin/env bash, so a missing interpreter is caught early + with a clear diagnostic instead of a cryptic exit-code-127 failure.""" + source = SERVER_FILE.read_text() + self.assertIn( + 'shutil.which("bash")', + source, + "server.py must resolve bash via shutil.which to catch missing interpreter", + ) + self.assertNotIn( + '"/usr/bin/env", "bash"', + source, + "server.py must not use /usr/bin/env bash (relies on PATH, causes code 127)", + ) + + def test_server_logs_actionable_message_when_bash_missing(self): + """Regression: when bash is absent the server must write an actionable log + entry and set FAILED status before returning an HTTP error.""" + source = SERVER_FILE.read_text() + self.assertIn( + "bash_path is None", + source, + "server.py must check that bash_path is not None before launching", + ) + self.assertIn( + "interpreter", + source, + "server.py missing-bash error message must reference 'interpreter'", + ) + + def test_server_captures_stderr_from_backup_subprocess(self): + """Regression: the backup subprocess must use stderr=PIPE so that any + startup error (e.g. code 127 from a missing command) is captured and + surfaced in the backup log rather than being silently discarded.""" + source = SERVER_FILE.read_text() + self.assertIn( + "stderr=asyncio.subprocess.PIPE", + source, + "backup subprocess must use stderr=PIPE to capture diagnostic output", + ) + self.assertIn( + "stderr_text", + source, + "_monitor_backup_subprocess must capture and log stderr content", + ) + + def test_monitor_includes_stderr_detail_in_failed_message(self): + """Regression: _monitor_backup_subprocess must append captured stderr to + the FAILED log entry so the UI shows what went wrong (e.g. 'bash: not + found') rather than only the raw exit code.""" + source = SERVER_FILE.read_text() + self.assertIn("stderr_text", source) + self.assertIn('if stderr_text else ""', source) + + def test_exit_code_127_subprocess_stderr_drain(self): + """Behavioral regression: a subprocess that exits 127 must have its + stderr drained without deadlock by the async drain loop.""" + + async def _run(): + proc = await asyncio.create_subprocess_exec( + "bash", "-c", "echo 'bash: command not found' >&2; exit 127", + stdout=asyncio.subprocess.DEVNULL, + stderr=asyncio.subprocess.PIPE, + ) + chunks: list[bytes] = [] + + async def _drain(): + async for line in proc.stderr: + chunks.append(line) + + drain_task = asyncio.create_task(_drain()) + rc = await proc.wait() + await drain_task + return rc, b"".join(chunks).decode() + + rc, stderr_out = asyncio.run(_run()) + self.assertEqual(rc, 127) + self.assertIn("bash: command not found", stderr_out) + if __name__ == "__main__": unittest.main() diff --git a/modules/core/sovran-hub.nix b/modules/core/sovran-hub.nix index af01ed7..a21e4ee 100644 --- a/modules/core/sovran-hub.nix +++ b/modules/core/sovran-hub.nix @@ -382,6 +382,8 @@ in }; path = [ + pkgs.bash + pkgs.gawk pkgs.qrencode pkgs.curl pkgs.iproute2