Fix TOCTOU crash in rotate_log() under concurrent invocations

[pablomh]

Summary

rotate_log() checks os.path.exists(log_path) and then calls os.rename(log_path, backup_path), with no exclusion between the two steps. When multiple obsah-based CLIs (e.g. foremanctl) run concurrently and share the same log file — a normal situation, since log rotation happens unconditionally at the top of main() before any other work — one process can rename the file away in the gap between another process's exists() check and its own rename() call, crashing with an unhandled FileNotFoundError.

Live reproduction

Confirmed against the real installed package (obsah 1.10.0, unmodified) on a live Satellite host, using foremanctl auth-bundle (which shares one log file across all invocations): 150 concurrent invocations across several rounds crashed 8 of them with this exact traceback:

File ".../obsah/__init__.py", line 596, in main
    rotate_log(application_config.log_path())
  File ".../obsah/__init__.py", line 585, in rotate_log
    os.rename(log_path, backup_path)
FileNotFoundError: [Errno 2] No such file or directory: '/var/log/foremanctl/foremanctl.log' -> '/var/log/foremanctl/foremanctl.2026-08-18-07-46-18.log'

After applying this fix on the same host, 120 further concurrent invocations produced zero crashes.

Why no locking is needed here

Unlike a genuine read-modify-write (e.g. merging into a shared parameters file), log rotation doesn't need mutual exclusion. Every concurrent process wants the same outcome — "the old log is archived and a fresh one starts" — and that outcome is fully satisfied once any one process's rename succeeds. There's no per-process contribution to preserve, so the fix is simply to tolerate losing the race rather than to coordinate around it.

This is a well-established idiom rather than something new to this codebase:

• It's the textbook EAFP ("easier to ask forgiveness than permission") pattern the Python community favors over check-then-act for exactly this reason — the original code was LBYL ("look before you leap"), which is inherently racy against other processes.
• persist_args(), a few lines away in this same file, already uses os.makedirs(persist_dir, mode=0o770, exist_ok=True) to solve the identical TOCTOU shape for directory creation (exist_ok=True exists in the stdlib specifically for this). This PR applies the same idiom to os.rename().
• Ansible itself does the same thing for an analogous shared-resource cleanup race in lib/ansible/plugins/connection/ssh.py:

  with contextlib.suppress(FileNotFoundError):
      self.shm.unlink()

Testing

Added test_rotate_log_tolerates_concurrent_rotation, which reconstructs the actual race (not just a raised exception): os.path.exists() is patched so that, on the one call that matters, it returns True but also renames the file away as a side effect — simulating another process winning the race — so the real, unmocked os.rename() call fails naturally. Asserts rotate_log() doesn't raise, that the "stolen" file exists (confirming the simulated race actually happened), and that the original log file is gone (matching the real post-race state).

Co-Authored-By: Claude Sonnet 5 noreply@anthropic.com

[ehelms]

I am not seeing how to connect this comment to the test itself. Maybe drop it?

[pablomh]

Fair point — that comment was describing the production race, not this test. The test never starts another process; it patches exists() so the file is already gone when rename() runs. Dropped it.