From f6b14c52109e0d55b919f179a38c8a4c8f8868f9 Mon Sep 17 00:00:00 2001 From: FrameBridge contributors Date: Tue, 27 Oct 2026 16:54:03 -0400 Subject: [PATCH] Verify owned TTY cleanup before reporting stop --- .../framebridge_expanded/_tty_backend.py | 5 +- .../expanded/framebridge_expanded/runtime.py | 16 +- source/expanded/framebridge_expanded/tty.py | 133 +++++++- .../framebridge_expanded/tty_cleanup.py | 143 +++++++++ .../framebridge_expanded/tty_sudo_control.py | 120 +++++++- source/tests/test_runtime_tty_cleanup.py | 80 +++++ source/tests/test_tty_cleanup.py | 290 ++++++++++++++++++ source/tests/test_tty_sudo.py | 6 +- 8 files changed, 759 insertions(+), 34 deletions(-) create mode 100644 source/expanded/framebridge_expanded/tty_cleanup.py create mode 100644 source/tests/test_runtime_tty_cleanup.py create mode 100644 source/tests/test_tty_cleanup.py diff --git a/source/expanded/framebridge_expanded/_tty_backend.py b/source/expanded/framebridge_expanded/_tty_backend.py index dcad4c1..6288923 100644 --- a/source/expanded/framebridge_expanded/_tty_backend.py +++ b/source/expanded/framebridge_expanded/_tty_backend.py @@ -212,7 +212,10 @@ class RealTerminalMixin: def shutdown(self): with self._lock: + errors=[] for sid in list(self._real): try:self.abort(sid) - except (TerminalDenied,SudoControlError):pass + except (TerminalDenied,SudoControlError) as error: + errors.append({'session_id':sid,'error':str(error)}) super().shutdown() + return {'cleanup_verified':not self._real,'errors':errors} diff --git a/source/expanded/framebridge_expanded/runtime.py b/source/expanded/framebridge_expanded/runtime.py index cc5a3f2..9bb78ca 100644 --- a/source/expanded/framebridge_expanded/runtime.py +++ b/source/expanded/framebridge_expanded/runtime.py @@ -468,6 +468,17 @@ def stop(state): connection = disconnect(state) if not connection.get("ok"): raise RuntimeError("The background ChatGPT connection could not be stopped safely; local services were left unchanged.") + from . import tty + + def verify_tty_stop(): + outcome = tty.stop(state) + if (not isinstance(outcome, dict) or outcome.get("ok") is not True + or outcome.get("cleanup_verified") is not True): + raise RuntimeError("TTY cleanup could not be verified; inspect this profile's terminal status before retrying.") + + # Verify terminal cleanup before stopping local services, then again after + # their termination to cover a terminal opened during the stop operation. + verify_tty_stop() from .autostart import mark_manual_stop mark_manual_stop(state) rec = _read(state / "manager.json") @@ -478,7 +489,10 @@ def stop(state): for child in reversed(_read(state / "worker.json").get("processes", [])): _terminate(None, child) _atomic(state / "ready.json", {"ready": False}) - return status(state) + verify_tty_stop() + result = status(state) + result["tty_cleanup_verified"] = True + return result def main(): parser = argparse.ArgumentParser() diff --git a/source/expanded/framebridge_expanded/tty.py b/source/expanded/framebridge_expanded/tty.py index b55a5db..37b6556 100644 --- a/source/expanded/framebridge_expanded/tty.py +++ b/source/expanded/framebridge_expanded/tty.py @@ -64,6 +64,7 @@ _core, _server = _load_captured() TerminalDenied = _core.TerminalDenied from ._tty_backend import RealTerminalMixin from .tty_sudo_control import SudoControlError +from . import tty_cleanup @dataclass(frozen=True) @@ -192,6 +193,7 @@ class PortableManager(RealTerminalMixin, _core.Manager): idle_timeout_s=600) self.max_sessions = limits.max_sessions self._last_used = {} + self.cleanup_daemon = None @staticmethod def _local_context(context): @@ -213,9 +215,13 @@ class PortableManager(RealTerminalMixin, _core.Manager): if program != "bash": raise TerminalDenied("PROGRAM_NOT_SUPPORTED") self._size(rows, cols) - result = super().open("bash", rows, cols) - self._last_used[result["session_id"]] = time.monotonic() - return result + self._record_cleanup(operation_pending=True) + try: + result = super().open("bash", rows, cols) + self._last_used[result["session_id"]] = time.monotonic() + return result + finally: + self._record_cleanup() def reap(self): super().reap() @@ -238,8 +244,57 @@ class PortableManager(RealTerminalMixin, _core.Manager): raise TerminalDenied("SESSION_NOT_FOUND") result = super().abort(sid) self._last_used.pop(sid, None) + self._record_cleanup() return result + def sudo(self, sid, mode): + if mode == "status": + return super().sudo(sid, mode) + self._record_cleanup(operation_pending=True) + try: + return super().sudo(sid, mode) + finally: + self._record_cleanup() + + def _cleanup_records(self): + keys = ("session_id", "pid", "birth", "server_pid", "server_birth", + "unit_name", "control_group", "pane_control_group") + records = [] + source = {} + if self._real: + source.update({sid: session._sudo_record for sid, session in self._real.items()}) + controller = getattr(self, "_sudo", None) + if controller is not None: + source.update(controller._records) + for record in source.values(): + records.append({key: record.get(key) for key in keys}) + if record.get("pending_old_record"): + records.append({key: record["pending_old_record"].get(key) for key in keys}) + return records + + def _record_cleanup(self, *, verified=False, errors=(), operation_pending=False): + if self.cleanup_daemon is not None: + tty_cleanup.write(self.profile, self.cleanup_daemon, self._cleanup_records(), + verified=verified, errors=errors, + operation_pending=operation_pending or self._cleanup_uncertain()) + + def _cleanup_uncertain(self): + controller = getattr(self, "_sudo", None) + return bool(controller is not None and controller._uncertain_launches) + + def shutdown(self): + with self._lock: + outcome = {"cleanup_verified": False, "errors": [{"error": "TTY_INTERNAL_ERROR"}]} + try: + outcome = super().shutdown() + if self._cleanup_records() or self._cleanup_uncertain(): + outcome["cleanup_verified"] = False + if not outcome["errors"]: + outcome["errors"] = [{"error": "TTY_CLEANUP_PENDING"}] + finally: + self._record_cleanup(verified=outcome["cleanup_verified"], errors=outcome["errors"]) + return outcome + def status(self): result = super().status() for key in ("owner_device_connected", "owner_signature_required"): @@ -297,6 +352,13 @@ class PortableServer(_server.LocalServer): os.umask(old) p.chmod(0o600) + def server_close(self): + try: + self.manager.shutdown() + finally: + socketserver.UnixStreamServer.server_close(self) + self._socket_path.unlink(missing_ok=True) + _SHAPES = { "status": (set(), set()), @@ -415,13 +477,19 @@ def serve(state=None): if not stat.S_ISSOCK(st.st_mode) or st.st_uid != os.geteuid(): raise TerminalDenied("TTY_SOCKET_UNTRUSTED") path.unlink() + try: + tty_cleanup.require_prior_cleanup(state) + except tty_cleanup.CleanupUnverified: + raise TerminalDenied("TTY_CLEANUP_PENDING") from None manager = PortableManager(state, limits, home) + manager.cleanup_daemon = tty_cleanup.daemon_identity(_identity(os.getpid())) + manager._record_cleanup() def stopped(_signum, _frame): raise SystemExit(0) signal.signal(signal.SIGTERM, stopped) signal.signal(signal.SIGINT, stopped) pidfile = state / "tty-process.json" - _write_json(pidfile, {**_identity(os.getpid()), "socket": str(path), "state_dir": str(state)}) + _write_json(pidfile, {**manager.cleanup_daemon, "socket": str(path), "state_dir": str(state)}) try: with PortableServer(path, manager) as server: print(json.dumps({"ok": True, "ready": True, "socket": str(path), @@ -436,6 +504,10 @@ def start(state=None): current = status(state) if current.get("ok"): return current + try: + tty_cleanup.require_prior_cleanup(state) + except tty_cleanup.CleanupUnverified: + return {"ok": False, "error": "TTY_CLEANUP_PENDING", "cleanup_verified": False} env = os.environ.copy() root = str(Path(__file__).resolve().parents[1]) env["PYTHONPATH"] = root + (os.pathsep + env["PYTHONPATH"] if env.get("PYTHONPATH") else "") @@ -458,19 +530,48 @@ def start(state=None): def stop(state=None): state = state_path(state) p = state / "tty-process.json" - if not p.is_file() or p.is_symlink(): - return {"ok": True, "state": "STOPPED", "already_stopped": True} + expected = None + already = True + death_known = False try: - record = json.loads(p.read_text()) - if not _alive(record): - return {"ok": True, "state": "STOPPED", "already_stopped": True} - os.kill(record["pid"], signal.SIGTERM) - deadline = time.monotonic() + 10 - while _alive(record) and time.monotonic() < deadline: - time.sleep(0.1) - return {"ok": not _alive(record), "state": "STOPPED" if not _alive(record) else "STOP_TIMEOUT"} - except (ValueError, OSError, KeyError): - return {"ok": False, "error": "TTY_STOP_FAILED"} + receipt = state / tty_cleanup.NAME + if not p.exists() and not p.is_symlink() and not receipt.exists() and not receipt.is_symlink(): + return {"ok": True, "state": "STOPPED", "already_stopped": True, + "cleanup_verified": True} + outcome = tty_cleanup.read(state) + if p.exists() or p.is_symlink(): + record = tty_cleanup.read_private(p) + if "boot" not in record: + raise tty_cleanup.CleanupUnverified() + expected = tty_cleanup.daemon_identity(record) + if outcome["daemon"] != expected: + raise tty_cleanup.CleanupUnverified() + if expected["boot"] == tty_cleanup.current_boot() and _alive(record): + already = False + fd = os.pidfd_open(record["pid"]) + try: + if _alive(record): + signal.pidfd_send_signal(fd, signal.SIGTERM) + finally: + os.close(fd) + deadline = time.monotonic() + 10 + while _alive(record) and time.monotonic() < deadline: + time.sleep(0.1) + if _alive(record): + return {"ok": False, "state": "STOP_TIMEOUT", "cleanup_verified": False} + elif outcome["daemon"]["boot"] == tty_cleanup.current_boot() and _alive(outcome["daemon"]): + raise tty_cleanup.CleanupUnverified() + death_known = True + outcome = tty_cleanup.read(state) + if expected is not None and outcome["daemon"] != expected: + raise tty_cleanup.CleanupUnverified() + if not outcome["cleanup_verified"] and outcome["daemon"]["boot"] == tty_cleanup.current_boot(): + raise tty_cleanup.CleanupUnverified() + return {"ok": True, "state": "STOPPED", "already_stopped": already, + "cleanup_verified": True} + except (tty_cleanup.CleanupUnverified, ValueError, OSError, KeyError, TypeError, AttributeError): + return {"ok": False, "state": "STOPPED" if death_known else "STOP_UNVERIFIED", + "error": "TTY_CLEANUP_UNVERIFIED", "cleanup_verified": False} finally: child = _STARTED_CHILDREN.get(str(state)) if child is not None: diff --git a/source/expanded/framebridge_expanded/tty_cleanup.py b/source/expanded/framebridge_expanded/tty_cleanup.py new file mode 100644 index 0000000..934bb7e --- /dev/null +++ b/source/expanded/framebridge_expanded/tty_cleanup.py @@ -0,0 +1,143 @@ +"""Private evidence for one TTY daemon's cleanup; never authorizes process kills.""" +from __future__ import annotations +import json +import os +import re +from pathlib import Path +import stat +import tempfile + +NAME = "tty-cleanup.json" + + +class CleanupUnverified(RuntimeError): + pass + + +def current_boot(): + return Path("/proc/sys/kernel/random/boot_id").read_text().strip() + + +def daemon_identity(record): + return {"pid": record["pid"], "birth": record["birth"], "uid": record["uid"], + "boot": record.get("boot", current_boot())} + + +def _session_valid(record): + keys = {"session_id", "pid", "birth", "server_pid", "server_birth", "unit_name", + "control_group", "pane_control_group"} + if type(record) is not dict or set(record) != keys: + return False + if (type(record["session_id"]) is not str or not re.fullmatch(r"[a-f0-9]{32}", record["session_id"]) + or any(type(record[key]) is not int or record[key] <= 0 for key in ("pid", "server_pid")) + or any(type(record[key]) is not str or not record[key].isdigit() for key in ("birth", "server_birth")) + or type(record["unit_name"]) is not str + or not re.fullmatch(r"framebridge-tty-[a-f0-9]{12}-" + record["session_id"] + r"-[a-f0-9]{8}\.service", record["unit_name"])): + return False + for key in ("control_group", "pane_control_group"): + group = record[key] + if type(group) is not str or not group.startswith("/") or ".." in Path(group).parts or "\0" in group: + return False + server = Path(record["control_group"]) + pane = Path(record["pane_control_group"]) + return (server.name == record["unit_name"] and (pane == server or ( + pane.parent == server.parent and re.fullmatch(r"tmux-spawn-[a-f0-9-]{36}\.scope", pane.name)))) + + +def read_private(path): + fd = os.open(path, os.O_RDONLY | os.O_NOFOLLOW | os.O_NONBLOCK) + with os.fdopen(fd, "r") as stream: + info = os.fstat(stream.fileno()) + if (not stat.S_ISREG(info.st_mode) or info.st_uid != os.geteuid() + or stat.S_IMODE(info.st_mode) != 0o600 or info.st_size > 1048576): + raise CleanupUnverified() + return json.load(stream) + + +def read(state): + try: + value = read_private(Path(state) / NAME) + daemon = value["daemon"] + if (type(value) is not dict or value.get("schema") != 1 + or value.get("state_dir") != str(Path(state).absolute()) + or type(value.get("cleanup_verified")) is not bool + or type(value.get("sessions")) is not list + or type(value.get("errors")) is not list + or type(value.get("operation_pending")) is not bool + or set(daemon) != {"pid", "birth", "uid", "boot"} + or type(daemon["pid"]) is not int or daemon["pid"] <= 0 + or type(daemon["birth"]) is not str or not daemon["birth"].isdigit() + or daemon["uid"] != os.geteuid() + or type(daemon["boot"]) is not str + or not re.fullmatch(r"[a-f0-9]{8}(-[a-f0-9]{4}){3}-[a-f0-9]{12}", daemon["boot"]) + or any(not _session_valid(record) for record in value["sessions"]) + or (value["cleanup_verified"] and (value["sessions"] or value["errors"] + or value["operation_pending"]))): + raise CleanupUnverified() + return value + except (OSError, ValueError, KeyError, TypeError): + raise CleanupUnverified() from None + + +def write(state, daemon, sessions, *, verified=False, errors=(), operation_pending=False): + state = Path(state) + value = {"schema": 1, "state_dir": str(state.absolute()), "daemon": daemon, + "cleanup_verified": verified, "sessions": sessions, + "errors": list(errors), "operation_pending": operation_pending} + fd, name = tempfile.mkstemp(prefix=".tty-cleanup-", dir=state) + try: + with os.fdopen(fd, "w") as stream: + json.dump(value, stream, ensure_ascii=True) + stream.write("\n") + os.replace(name, state / NAME) + finally: + Path(name).unlink(missing_ok=True) + + +def _gone(pid, birth, uid): + if type(pid) is not int or pid <= 0 or type(birth) is not str or not birth.isdigit(): + return False + try: + path = Path("/proc") / str(pid) + fields = (path / "stat").read_text().rsplit(")", 1)[1].split() + return fields[19] != birth or (path.stat().st_uid == uid and fields[0] in {"Z", "X"}) + except FileNotFoundError: + return True + except (OSError, IndexError): + return False + + +def require_prior_cleanup(state): + """Permit restart only after prior evidence is clean or identities are gone. + + Persisted identities are read-only evidence, never targets for a later kill. + """ + state = Path(state) + if not (state / NAME).exists() and not (state / NAME).is_symlink(): + if (state / "tty-process.json").exists() or (state / "tty-process.json").is_symlink(): + raise CleanupUnverified() + return + value = read(state) + # A different kernel boot cannot retain any process or user cgroup from + # this daemon. Do not compare old PID numbers with this boot's processes. + if value["daemon"]["boot"] != current_boot() or value["cleanup_verified"]: + return + daemon = value["daemon"] + if value["operation_pending"] or not _gone(daemon["pid"], daemon["birth"], daemon["uid"]): + raise CleanupUnverified() + for record in value["sessions"]: + try: + if not _gone(record["pid"], record["birth"], daemon["uid"]): + raise CleanupUnverified() + if not _gone(record["server_pid"], record["server_birth"], daemon["uid"]): + raise CleanupUnverified() + for group in {record["control_group"], record["pane_control_group"]}: + if not group.startswith("/") or ".." in Path(group).parts: + raise CleanupUnverified() + root = Path("/sys/fs/cgroup") / group.lstrip("/") + if root.exists(): + files = [root / "cgroup.procs", *root.glob("**/cgroup.procs")] + if any(path.read_text().strip() for path in files): + raise CleanupUnverified() + except (OSError, KeyError, TypeError, AttributeError): + raise CleanupUnverified() from None diff --git a/source/expanded/framebridge_expanded/tty_sudo_control.py b/source/expanded/framebridge_expanded/tty_sudo_control.py index 5fa4068..0dd1547 100644 --- a/source/expanded/framebridge_expanded/tty_sudo_control.py +++ b/source/expanded/framebridge_expanded/tty_sudo_control.py @@ -11,6 +11,7 @@ import os from pathlib import Path import pwd import re +import signal import shutil import shlex import stat @@ -122,6 +123,7 @@ class SudoConsoleController: self.timeout = float(timeout) self.prefix = "framebridge-tty-" + hashlib.sha256(str(self.base).encode()).hexdigest()[:12] + "-" self._records = {} + self._uncertain_launches = set() self._lock = threading.RLock() def _run(self, command, *, data=None, optional=False): @@ -359,8 +361,12 @@ class SudoConsoleController: command.append("--setenv=" + key + "=" + value) command += [self.tmux, "-D", "-S", str(socket), "-f", "/dev/null"] launched = False + verified_record = None try: launched = True + # A failed partial launch must never disappear from shutdown + # evidence, even if its ordinary rollback also fails. + self._uncertain_launches.add(unit_name) self._run(command) deadline = time.monotonic() + self.timeout while time.monotonic() < deadline: @@ -382,26 +388,37 @@ class SudoConsoleController: deadline = time.monotonic() + self.timeout while time.monotonic() < deadline: current = self._inspect(record) + verified_record = current if current["no_new_privs"] != expected: _fail("TTY_SUDO_PRIVILEGE_STATE_MISMATCH") if not current["busy"]: + self._uncertain_launches.discard(unit_name) return current time.sleep(0.05) _fail("TTY_SUDO_CONSOLE_NOT_IDLE") except BaseException: - if launched: + # Only a fully inspected record can prove rollback. Incomplete + # launches retain uncertainty even if an ordinary stop was sent. + if verified_record is not None: + try: + self._close(verified_record) + self._uncertain_launches.discard(unit_name) + except SudoControlError: + pass + elif launched: try: unit = self._unit(unit_name) proc, _, _, _ = self._pane(record) scope = self._pane_scope(self._pane_group(proc["pid"]), unit.get("ControlGroup")) if scope: - self._run([self.systemctl, "--user", "stop", scope], optional=True) + self._run([self.systemctl, "--user", "--no-block", "stop", scope], optional=True) except SudoControlError: pass - self._run([self.systemctl, "--user", "stop", unit_name], optional=True) - socket.unlink(missing_ok=True) - marker.unlink(missing_ok=True) - rcfile.unlink(missing_ok=True) + self._run([self.systemctl, "--user", "--no-block", "stop", unit_name], optional=True) + if unit_name not in self._uncertain_launches: + socket.unlink(missing_ok=True) + marker.unlink(missing_ok=True) + rcfile.unlink(missing_ok=True) raise def launch(self, session_id, cwd=None, rows=24, cols=80, mode="off"): @@ -433,6 +450,34 @@ class SudoConsoleController: _fail("TTY_SUDO_ELEVATED_PROCESS_PRESENT") return {**record, "busy_reason": None} + def _close_members(self, record): + members = self._scope_members_existing(record) + result = {} + for pid in members: + process = _proc(pid) + if process is None: + # An exited member can disappear between cgroup and /proc reads. + if pid in self._scope_members_existing(record): + _fail("TTY_SUDO_PROCESS_STATE_UNVERIFIED") + continue + if any(uid != self.uid for uid in process["uids"]): + _fail("TTY_SUDO_ELEVATED_PROCESS_PRESENT") + if process["state"] not in {"Z", "X"}: + result[pid] = process + return result + + def _scope_members_existing(self, record): + members = set() + for group in {record["control_group"], record["pane_control_group"]}: + path = Path("/sys/fs/cgroup") / group.lstrip("/") + if path.exists(): + try: + members.update(self._scope(group)) + except SudoControlError: + if path.exists(): + raise + return members + def _close(self, record, require_idle=False): try: current = self._idle(record) if require_idle else self._inspect(record) @@ -444,17 +489,60 @@ class SudoConsoleController: if current["busy_reason"] in {"TTY_SUDO_ELEVATED_PROCESS_PRESENT", "TTY_SUDO_PROCESS_STATE_UNVERIFIED"}: _fail(current["busy_reason"]) if record.get("unit_name"): + # Close this private PTY before stopping its sibling tmux-spawn + # scope: interactive Bash ignores TERM and that scope may otherwise + # wait the user's default 90-second systemd stop timeout. + self._tmux(record, "kill-session", "-t", record["name"], optional=True) if current.get("pane_scope_unit"): - self._run([self.systemctl, "--user", "stop", current["pane_scope_unit"]], optional=True) - self._run([self.systemctl, "--user", "stop", record["unit_name"]], optional=True) + self._run([self.systemctl, "--user", "--no-block", "stop", + current["pane_scope_unit"]], optional=True) + self._run([self.systemctl, "--user", "--no-block", "stop", + record["unit_name"]], optional=True) deadline = time.monotonic() + self.timeout + escalate_at = time.monotonic() + min(1.0, self.timeout / 3) + escalated = False while time.monotonic() < deadline: - remaining = set() - for group in {current["control_group"], current["pane_control_group"]}: - if (Path("/sys/fs/cgroup") / group.lstrip("/")).exists(): - remaining.update(self._scope(group)) - if not remaining and _proc(current["pid"]) is None: + remaining = self._close_members(current) + shell = _proc(current["pid"]) + server = _proc(current["server_pid"]) + shell_alive = (shell and shell["birth"] == current["birth"] + and shell["state"] not in {"Z", "X"}) + server_alive = (server and server["birth"] == current["server_birth"] + and server["state"] not in {"Z", "X"}) + if not remaining and not shell_alive and not server_alive: break + if not escalated and time.monotonic() >= escalate_at: + # Revalidate the private service identity and all recorded + # cgroup members before escalating. Never signal root jobs, + # unverified processes or a reused PID. + self._terminated(current) + for identity in remaining.values(): + try: + fd = os.pidfd_open(identity["pid"]) + except ProcessLookupError: + continue + except (AttributeError, OSError): + _fail("TTY_SUDO_PIDFD_UNAVAILABLE") + try: + live = _proc(identity["pid"]) + if (not live or live["birth"] != identity["birth"] + or live["state"] in {"Z", "X"}): + continue + if any(uid != self.uid for uid in live["uids"]): + _fail("TTY_SUDO_ELEVATED_PROCESS_PRESENT") + group = self._pane_group(live["pid"]) + if not any(group == owned or group.startswith(owned + "/") + for owned in {current["control_group"], current["pane_control_group"]}): + _fail("TTY_SUDO_CGROUP_UNEXPECTED") + try: + signal.pidfd_send_signal(fd, signal.SIGKILL) + except ProcessLookupError: + pass + except (AttributeError, OSError): + _fail("TTY_SUDO_PIDFD_SIGNAL_FAILED") + finally: + os.close(fd) + escalated = True time.sleep(0.05) else: _fail("TTY_SUDO_SCOPE_STILL_RUNNING") @@ -505,7 +593,11 @@ class SudoConsoleController: except BaseException as error: previous = _proc(current["pid"]) if previous and previous["birth"] == current["birth"] and previous["state"] not in {"Z", "X"}: - self._close(candidate) + try: + self._close(candidate) + except BaseException: + self._uncertain_launches.add(candidate["unit_name"]) + raise raise # The former Bash is gone. Keep the verified replacement and # retain old ownership metadata until all old scopes are empty. diff --git a/source/tests/test_runtime_tty_cleanup.py b/source/tests/test_runtime_tty_cleanup.py new file mode 100644 index 0000000..132a974 --- /dev/null +++ b/source/tests/test_runtime_tty_cleanup.py @@ -0,0 +1,80 @@ +"""Runtime stop must propagate terminal cleanup evidence.""" +from contextlib import ExitStack +from pathlib import Path +import unittest +from unittest.mock import call, patch + +from framebridge_expanded import runtime, tty + + +class RuntimeTerminalCleanupTests(unittest.TestCase): + def setUp(self): + stack = ExitStack() + self.addCleanup(stack.close) + self.state = Path("/unused-framebridge-runtime-test") + stack.enter_context(patch.object(runtime, "state_path", return_value=self.state)) + stack.enter_context(patch.object(runtime, "load_config", return_value={})) + self.disconnect = stack.enter_context(patch( + "framebridge_expanded.connection_background.disconnect", return_value={"ok": True})) + self.manual = stack.enter_context(patch("framebridge_expanded.autostart.mark_manual_stop")) + self.terminal = stack.enter_context(patch.object( + tty, "stop", return_value={"ok": True, "cleanup_verified": True})) + self.manager = {"pid": 101} + self.child = {"pid": 102} + self.worker = {"pid": 103} + self.read = stack.enter_context(patch.object(runtime, "_read", side_effect=[ + self.manager, {"processes": [self.child]}, {"processes": [self.worker]}])) + self.terminate = stack.enter_context(patch.object(runtime, "_terminate")) + self.atomic = stack.enter_context(patch.object(runtime, "_atomic")) + self.status = stack.enter_context(patch.object( + runtime, "status", return_value={"ok": True, "running": False, "ready": False})) + + def test_failed_terminal_cleanup_leaves_local_services_running(self): + self.terminal.return_value = {"ok": False, "state": "STOPPED", "cleanup_verified": False} + with self.assertRaisesRegex(RuntimeError, "TTY cleanup could not be verified"): + runtime.stop(self.state) + self.disconnect.assert_called_once_with(self.state) + self.manual.assert_not_called() + self.terminate.assert_not_called() + self.atomic.assert_not_called() + self.status.assert_not_called() + + def test_cleanup_requires_both_explicit_true_fields(self): + for outcome in ({"ok": True}, {"ok": True, "cleanup_verified": False}, + {"ok": False, "cleanup_verified": True}, None): + with self.subTest(outcome=outcome): + self.terminal.return_value = outcome + with self.assertRaises(RuntimeError): + runtime.stop(self.state) + self.terminate.assert_not_called() + + def test_normal_stop_verifies_before_and_after_terminating_services(self): + result = runtime.stop(self.state) + self.assertEqual(self.terminal.call_args_list, [call(self.state), call(self.state)]) + self.terminate.assert_has_calls([ + call(None, self.manager), call(None, self.child), call(None, self.worker)]) + self.manual.assert_called_once_with(self.state) + self.assertTrue(result["tty_cleanup_verified"]) + self.assertFalse(result["running"]) + + def test_late_cleanup_failure_cannot_return_global_success(self): + self.terminal.side_effect = [ + {"ok": True, "cleanup_verified": True}, + {"ok": False, "cleanup_verified": False}] + with self.assertRaisesRegex(RuntimeError, "TTY cleanup could not be verified"): + runtime.stop(self.state) + self.assertTrue(self.terminate.called) + self.atomic.assert_called_once_with(self.state / "ready.json", {"ready": False}) + self.status.assert_not_called() + + def test_failed_disconnect_preserves_existing_stop_guard(self): + self.disconnect.return_value = {"ok": False} + with self.assertRaisesRegex(RuntimeError, "background ChatGPT connection"): + runtime.stop(self.state) + self.terminal.assert_not_called() + self.manual.assert_not_called() + self.terminate.assert_not_called() + + +if __name__ == "__main__": + unittest.main() diff --git a/source/tests/test_tty_cleanup.py b/source/tests/test_tty_cleanup.py new file mode 100644 index 0000000..92a8b19 --- /dev/null +++ b/source/tests/test_tty_cleanup.py @@ -0,0 +1,290 @@ +"""Owned real-console cleanup, daemon outcome and fail-closed evidence regressions.""" +import json +import os +from pathlib import Path +import signal +import subprocess +import sys +import tempfile +import time +import unittest +from unittest import mock +import uuid + +ROOT = Path(__file__).resolve().parents[1] +sys.path.insert(0, str(ROOT / "expanded")) +os.environ["FRAMEBRIDGE_EXPANDED_PREFIX"] = str(ROOT) +from framebridge_expanded import tty, tty_cleanup, tty_sudo_control as control + + +class CleanupTests(unittest.TestCase): + def setUp(self): + self.temp = tempfile.TemporaryDirectory(prefix="fb-cleanup-qa-", dir=Path("/run/user") / str(os.getuid())) + self.state = Path(self.temp.name) + self.addCleanup(self.temp.cleanup) + self.addCleanup(tty.stop, self.state) + + def dead_daemon(self): + child = subprocess.Popen([sys.executable, "-c", "import time;time.sleep(10)"]) + identity = tty._identity(child.pid) + child.terminate() + child.wait() + return tty_cleanup.daemon_identity(identity) + + def pidfile(self, daemon): + tty._write_json(self.state / "tty-process.json", {key: daemon[key] for key in ("pid", "birth", "uid", "boot")}) + + def assert_gone(self, record): + for pid, birth in ((record["pid"], record["birth"]), (record["server_pid"], record["server_birth"])): + process = control._proc(pid) + self.assertFalse(process and process["birth"] == birth and process["state"] not in {"Z", "X"}) + controller = control.SudoConsoleController(tty.runtime_path(self.state), timeout=5) + self.assertEqual(controller._scope_members_existing(record), set()) + + def test_real_close_and_stop_verify_owned_process_death(self): + self.assertTrue(tty.start(self.state)["ok"]) + for close_first in (True, False): + opened = tty.request(self.state, {"authorized": True, "op": "open"}) + self.assertTrue(opened["ok"], opened) + receipt = tty_cleanup.read(self.state) + self.assertEqual(len(receipt["sessions"]), 1) + record = receipt["sessions"][0] + if close_first: + closed = tty.request(self.state, {"authorized": True, "op": "close", "session_id": record["session_id"]}) + self.assertTrue(closed["ok"], closed) + self.assertTrue(closed["result"]["cleanup_verified"]) + stopped = tty.stop(self.state) + self.assertTrue(stopped["ok"], stopped) + self.assertTrue(stopped["cleanup_verified"]) + self.assert_gone(record) + self.assertTrue(tty.stop(self.state)["cleanup_verified"]) + if close_first: + self.assertTrue(tty.start(self.state)["ok"]) + + def test_failed_shutdown_cannot_become_success_by_stop_or_restart(self): + limits = tty.Limits() + home = self.state / "home"; home.mkdir() + manager = tty.PortableManager(self.state, limits, home) + manager.cleanup_daemon = self.dead_daemon() + manager._record_cleanup() + self.addCleanup(manager.shutdown) + manager.open() + self.pidfile(manager.cleanup_daemon) + with mock.patch.object(manager._sudo, "close", side_effect=control.SudoControlError("TTY_SUDO_SCOPE_STILL_RUNNING")): + result = manager.shutdown() + self.assertFalse(result["cleanup_verified"]) + before = (self.state / tty_cleanup.NAME).read_bytes() + for _ in range(2): + result = tty.stop(self.state) + self.assertFalse(result["ok"], result) + self.assertFalse(result["cleanup_verified"]) + self.assertEqual(result["error"], "TTY_CLEANUP_UNVERIFIED") + self.assertEqual(tty.start(self.state)["error"], "TTY_CLEANUP_PENDING") + self.assertEqual((self.state / tty_cleanup.NAME).read_bytes(), before) + self.assertTrue(control._proc(tty_cleanup.read(self.state)["sessions"][0]["pid"])) + self.assertTrue(manager.shutdown()["cleanup_verified"]) + self.assertTrue(tty.stop(self.state)["cleanup_verified"]) + + def test_missing_or_other_daemon_receipt_does_not_verify_stop(self): + expected = self.dead_daemon(); self.pidfile(expected) + self.assertFalse(tty.stop(self.state)["cleanup_verified"]) + other = self.dead_daemon() + tty_cleanup.write(self.state, other, [], verified=True) + self.assertFalse(tty.stop(self.state)["cleanup_verified"]) + + def test_untrusted_or_tampered_receipt_fails_closed(self): + daemon = self.dead_daemon(); self.pidfile(daemon) + path = self.state / tty_cleanup.NAME + tty_cleanup.write(self.state, daemon, [], verified=True) + path.chmod(0o644) + self.assertFalse(tty.stop(self.state)["cleanup_verified"]) + path.chmod(0o600) + value = json.loads(path.read_text()); value["daemon"]["uid"] += 1 + tty._write_json(path, value) + self.assertFalse(tty.stop(self.state)["cleanup_verified"]) + value["daemon"] = daemon; value["state_dir"] += "-other" + tty._write_json(path, value) + self.assertFalse(tty.stop(self.state)["cleanup_verified"]) + path.unlink(); path.symlink_to(self.state / "tty-process.json") + self.assertFalse(tty.stop(self.state)["cleanup_verified"]) + self.assertEqual(tty.start(self.state)["error"], "TTY_CLEANUP_PENDING") + + def test_inflight_operation_receipt_is_preserved_on_restart(self): + daemon = self.dead_daemon() + tty_cleanup.write(self.state, daemon, [], operation_pending=True) + before = (self.state / tty_cleanup.NAME).read_bytes() + self.assertEqual(tty.start(self.state)["error"], "TTY_CLEANUP_PENDING") + self.assertEqual((self.state / tty_cleanup.NAME).read_bytes(), before) + + def test_pending_receipt_allows_restart_only_after_natural_identity_death(self): + daemon = self.dead_daemon() + child = subprocess.Popen([sys.executable, "-c", "import time;time.sleep(10)"]) + identity = tty._identity(child.pid) + sid = uuid.uuid4().hex + unit = "framebridge-tty-" + "a" * 12 + "-" + sid + "-" + "b" * 8 + ".service" + record = {"session_id": sid, "unit_name": unit, "pid": child.pid, "birth": identity["birth"], + "server_pid": daemon["pid"], "server_birth": daemon["birth"], + "control_group": "/absent-fb-cleanup-qa/" + unit, + "pane_control_group": "/absent-fb-cleanup-qa/" + unit} + tty_cleanup.write(self.state, daemon, [record], errors=[{"error": "TTY_SUDO_SCOPE_STILL_RUNNING"}]) + try: + self.assertEqual(tty.start(self.state)["error"], "TTY_CLEANUP_PENDING") + finally: + child.terminate(); child.wait() + self.assertTrue(tty.start(self.state)["ok"]) + self.assertTrue(tty.stop(self.state)["cleanup_verified"]) + + def test_prelaunch_invalid_arguments_do_not_poison_profile(self): + home = self.state / "home"; home.mkdir() + manager = tty.PortableManager(self.state, tty.Limits(), home) + manager.cleanup_daemon = self.dead_daemon() + manager._record_cleanup() + self.addCleanup(manager.shutdown) + with mock.patch.object(manager._sudo, "_run") as run: + with self.assertRaises(control.SudoControlError): + manager._sudo.launch(uuid.uuid4().hex, cwd=self.state / "absent") + with self.assertRaises(tty.TerminalDenied): + manager.open(rows=0) + run.assert_not_called() + self.assertFalse(manager._cleanup_uncertain()) + self.assertTrue(manager.shutdown()["cleanup_verified"]) + self.assertTrue(tty.start(self.state)["ok"]) + self.assertTrue(tty.stop(self.state)["cleanup_verified"]) + + def test_previous_boot_receipts_recover_without_signaling_old_pid_numbers(self): + daemon = tty_cleanup.daemon_identity(tty._identity(os.getpid())) + daemon["boot"] = str(uuid.uuid4()) + for verified, pending in ((True, False), (False, False), (False, True)): + with self.subTest(verified=verified, pending=pending): + tty_cleanup.write(self.state, daemon, [], verified=verified, operation_pending=pending) + self.pidfile(daemon) + with mock.patch.object(os, "pidfd_open") as opening: + self.assertTrue(tty.stop(self.state)["cleanup_verified"]) + opening.assert_not_called() + self.assertTrue(tty.start(self.state)["ok"]) + self.assertTrue(tty.stop(self.state)["cleanup_verified"]) + + def test_verified_launch_rollback_does_not_poison_profile(self): + home = self.state / "home"; home.mkdir() + manager = tty.PortableManager(self.state, tty.Limits(), home) + manager.cleanup_daemon = self.dead_daemon(); manager._record_cleanup() + self.addCleanup(manager.shutdown) + original = manager._sudo._inspect + injected = False + def mismatch(record): + nonlocal injected + current = original(record) + if not injected: + injected = True + return {**current, "no_new_privs": 0} + return current + with mock.patch.object(manager._sudo, "_inspect", side_effect=mismatch): + with self.assertRaises(control.SudoControlError) as error: + manager.open() + self.assertEqual(error.exception.code, "TTY_SUDO_PRIVILEGE_STATE_MISMATCH") + self.assertFalse(manager._cleanup_uncertain()) + self.assertTrue(manager.shutdown()["cleanup_verified"]) + self.assertTrue(manager.open()["session_id"]) + self.assertTrue(manager.shutdown()["cleanup_verified"]) + + def test_partial_launch_and_failed_rollback_remain_unverified(self): + home = self.state / "home"; home.mkdir() + manager = tty.PortableManager(self.state, tty.Limits(), home) + manager.cleanup_daemon = self.dead_daemon() + manager._record_cleanup(); self.pidfile(manager.cleanup_daemon) + original = manager._sudo._run + def failed(command, **kwargs): + if command[0] == manager._sudo.systemd_run: + original(command, **kwargs) + raise control.SudoControlError("TTY_SUDO_BACKEND_TIMEOUT") + if command[0] == manager._sudo.systemctl and "stop" in command: + raise control.SudoControlError("TTY_SUDO_BACKEND_TIMEOUT") + return original(command, **kwargs) + try: + with mock.patch.object(manager._sudo, "_run", side_effect=failed): + with self.assertRaises(control.SudoControlError): + manager.open() + self.assertTrue(manager._sudo._uncertain_launches) + self.assertFalse(manager.shutdown()["cleanup_verified"]) + self.assertTrue(tty_cleanup.read(self.state)["operation_pending"]) + self.assertFalse(tty.stop(self.state)["cleanup_verified"]) + self.assertEqual(tty.start(self.state)["error"], "TTY_CLEANUP_PENDING") + finally: + for unit in manager._sudo._uncertain_launches: + manager._sudo._unit(unit) # Narrow ownership validation first. + original([manager._sudo.systemctl, "--user", "stop", unit], optional=True) + + def test_real_close_kills_hup_and_term_ignoring_owned_job(self): + controller = control.SudoConsoleController(tty.runtime_path(self.state), timeout=5) + record = controller.launch(uuid.uuid4().hex, cwd=self.state) + self.addCleanup(lambda: controller.close(record) if record["session_id"] in controller._records else None) + marker = self.state / "child.pid" + script = "import os,signal,time;signal.signal(signal.SIGHUP,signal.SIG_IGN);signal.signal(signal.SIGTERM,signal.SIG_IGN);open(" + repr(str(marker)) + ",'w').write(str(os.getpid()));time.sleep(30)" + import shlex + command = "nohup " + shlex.quote(sys.executable) + " -c " + shlex.quote(script) + " >/dev/null 2>&1 & disown" + controller._tmux(record, "send-keys", "-t", record["pane"], "-l", command) + controller._tmux(record, "send-keys", "-t", record["pane"], "Enter") + deadline = time.monotonic() + 3 + while not marker.exists() and time.monotonic() < deadline: + time.sleep(0.03) + self.assertTrue(marker.exists()) + child = control._proc(int(marker.read_text())) + calls = [] + original = controller._run + def run(command, **kwargs): + calls.append(command) + return original(command, **kwargs) + started = time.monotonic() + with mock.patch.object(controller, "_run", side_effect=run): + self.assertTrue(controller.close(record)["closed"]) + self.assertLess(time.monotonic() - started, 5) + killed = control._proc(child["pid"]) + self.assertFalse(killed and killed["birth"] == child["birth"] and killed["state"] not in {"Z", "X"}) + self.assert_gone(record) + kill_index = next(i for i, call in enumerate(calls) if "kill-session" in call) + stops = [(i, call) for i, call in enumerate(calls) if "stop" in call] + self.assertTrue(stops) + self.assertTrue(all(i > kill_index and "--no-block" in call for i, call in stops)) + + +class PidfdGuardTests(unittest.TestCase): + def test_reused_pid_after_pidfd_open_is_not_signaled(self): + controller = control.SudoConsoleController.__new__(control.SudoConsoleController) + controller.uid = os.getuid() + controller.timeout = 0.06 + controller.systemctl = "systemctl" + record = {"session_id": "a" * 32, "unit_name": "owned.service", "name": "private-session", + "pid": 123456, "birth": "1", "server_pid": 123457, "server_birth": "2", + "control_group": "/private/owned.service", "pane_control_group": "/private/own.scope", + "pane_scope_unit": "own.scope", "busy_reason": None} + member = {"pid": 123458, "birth": "3", "uids": (os.getuid(),) * 4, "state": "S"} + opened = False + def pidfd_open(pid): + nonlocal opened + self.assertEqual(pid, member["pid"]) + opened = True + return 987 + def process(pid): + if pid == member["pid"]: + return {**member, "birth": "4" if opened else "3"} + return None + with (mock.patch.object(controller, "_inspect", return_value=record), + mock.patch.object(controller, "_tmux"), + mock.patch.object(controller, "_run"), + mock.patch.object(controller, "_close_members", return_value={member["pid"]: member}), + mock.patch.object(controller, "_terminated", return_value=record), + mock.patch.object(controller, "_pane_group") as group, + mock.patch.object(control, "_proc", side_effect=process), + mock.patch.object(os, "pidfd_open", side_effect=pidfd_open), + mock.patch.object(os, "close") as closing, + mock.patch.object(signal, "pidfd_send_signal") as sending): + with self.assertRaises(control.SudoControlError): + controller._close(record) + self.assertTrue(opened) + sending.assert_not_called() + group.assert_not_called() + closing.assert_called_once_with(987) + + +if __name__ == "__main__": + unittest.main() diff --git a/source/tests/test_tty_sudo.py b/source/tests/test_tty_sudo.py index d2c7891..19978f1 100644 --- a/source/tests/test_tty_sudo.py +++ b/source/tests/test_tty_sudo.py @@ -254,8 +254,10 @@ class PortableSudoLimitsTests(SudoControlTests): self.assertEqual(error.exception.code, "TTY_SUDO_SIZE_INVALID") def test_actual_nnp_mismatch_rolls_back_candidate(self): - with mock.patch.object(self.controller, "_inspect", - return_value={"no_new_privs": 0, "busy": False}): + original = self.controller._inspect + def mismatch(record): + return {**original(record), "no_new_privs": 0} + with mock.patch.object(self.controller, "_inspect", side_effect=mismatch): with self.assertRaises(control.SudoControlError) as error: self.controller.launch(uuid.uuid4().hex, mode="off") self.assertEqual(error.exception.code, "TTY_SUDO_PRIVILEGE_STATE_MISMATCH")