diff --git a/CHANGELOG.md b/CHANGELOG.md index 17246ea..6a9cef1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,12 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). +## [Unreleased] + +### Fixed + +- 🔒 **Process log retention never reached files whose in-memory record was gone** — `_cleanup_expired()`'s log-file deletion only ran when a log file *also* had a matching in-memory `BackgroundProcess` record, but that registry is in-memory and resets on every restart, while the JSONL log files themselves live on disk (often a persistent volume) and survive restarts. Any log file older than the last restart was therefore permanently unreachable by that path regardless of age. Confirmed this can retain plaintext secrets indefinitely if a command echoes one to stdout. `_cleanup_expired()` now also sweeps the log directory by file mtime directly (rate-limited to once per 5 minutes), independent of any in-memory record. + ## [0.11.34] - 2026-04-08 ### Added diff --git a/open_terminal/main.py b/open_terminal/main.py index 01c0796..854e16e 100644 --- a/open_terminal/main.py +++ b/open_terminal/main.py @@ -345,7 +345,7 @@ def _set_session_cwd(session_id: str | None, path: str): _session_cwds[session_id] = (path, time.time()) -from open_terminal.utils.log import log_process, read_log +from open_terminal.utils.log import log_process, read_log, sweep_expired_log_files_rate_limited @@ -375,6 +375,17 @@ def _cleanup_expired(): except OSError: pass + # The loop above only reaches a log file while its BackgroundProcess + # record is still in _processes -- an in-memory dict that resets on + # every restart. Log files live on disk (often a persistent volume) + # and survive restarts, so a file whose in-memory record is gone was + # otherwise permanently unreachable by this function regardless of + # age. Sweep the log directory by mtime directly as a backstop, + # independent of any in-memory record. + sweep_expired_log_files_rate_limited( + os.path.join(LOG_DIR, "processes"), PROCESS_LOG_RETENTION + ) + def _get_process(process_id: str) -> BackgroundProcess: _cleanup_expired() diff --git a/open_terminal/utils/log.py b/open_terminal/utils/log.py index c7e02aa..6b35c8c 100644 --- a/open_terminal/utils/log.py +++ b/open_terminal/utils/log.py @@ -14,6 +14,60 @@ from open_terminal.env import MAX_PROCESS_LOG_SIZE, LOG_FLUSH_INTERVAL, LOG_FLUSH_BUFFER +_last_disk_sweep = 0.0 +_DISK_SWEEP_INTERVAL = 300 # rate-limit: at most once per 5 minutes + + +def sweep_expired_log_files( + processes_dir: str, retention: float, now: Optional[float] = None +) -> list[str]: + """Delete ``*.jsonl`` files in *processes_dir* older than *retention* seconds. + + Complements the in-memory-record-based cleanup in ``main.py``'s + ``_cleanup_expired()``: that path only reaches a log file while its + ``BackgroundProcess`` record is still in memory, but that registry + resets on every process restart while the log files themselves live + on a persistent volume. A log file older than the last restart is + therefore permanently unreachable by that path alone, regardless of + age. This reads mtimes directly off disk instead. + + Returns the list of paths actually deleted (for observability/tests). + """ + now = time.time() if now is None else now + deleted: list[str] = [] + try: + entries = os.listdir(processes_dir) + except OSError: + return deleted + for name in entries: + if not name.endswith(".jsonl"): + continue + path = os.path.join(processes_dir, name) + try: + if now - os.path.getmtime(path) > retention: + os.remove(path) + deleted.append(path) + except OSError: + pass + return deleted + + +def sweep_expired_log_files_rate_limited(processes_dir: str, retention: float) -> list[str]: + """Rate-limited wrapper around :func:`sweep_expired_log_files`. + + ``_cleanup_expired()`` runs on every process-status-check request, + which can be frequent under active polling -- an unconditional + directory listing on every call would add avoidable I/O. This limits + the actual disk scan to at most once per *_DISK_SWEEP_INTERVAL* + regardless of call frequency. + """ + global _last_disk_sweep + now = time.time() + if now - _last_disk_sweep < _DISK_SWEEP_INTERVAL: + return [] + _last_disk_sweep = now + return sweep_expired_log_files(processes_dir, retention, now=now) + class BoundedLogWriter: """Async wrapper that rotates the log file when it exceeds a size limit.