Skip to content

feat(config): interprocess write lock — eliminate watchdog TOCTOU race - #57

Merged
github-actions[bot] merged 5 commits into
mainfrom
claude/config-write-lock
Jun 29, 2026
Merged

feat(config): interprocess write lock — eliminate watchdog TOCTOU race#57
github-actions[bot] merged 5 commits into
mainfrom
claude/config-write-lock

Conversation

@dizhaky

@dizhaky dizhaky commented Jun 29, 2026

Copy link
Copy Markdown
Owner

Problem

save_config() writes config.yaml then separately writes the SHA256 seal in seal_config(). If the Config Integrity Watchdog fires in that gap (milliseconds), it sees current != sealed, declares tampering, and triggers a false restore — resurrecting the scanner/watchdog conflict fixed in #52.

_CONFIG_LOCK (a threading lock) protects same-process concurrent writes but does nothing for the watchdog, which runs in a separate cron process.

Slack thread: https://mfc-nyc.slack.com/archives/C0BD8QBUSJF/p1782743855322599

Solution

An OS-level file lock (fcntl.flock on POSIX, msvcrt.locking on Windows) that save_config() holds exclusively from before atomic_yaml_write through the seal_config() write. No external process can observe an intermediate state.

Changes

hermes_cli/config.py

Function Description
get_config_lock_path() Returns ~/.hermes/config.yaml.lock
config_write_lock() Exclusive OS file lock context manager — hold during write + seal
config_read_lock() Shared lock context manager — for external readers (e.g. the watchdog) to acquire before checking integrity
_write_config_to_disk() Private helper extracted from save_config() to avoid fcntl.flock re-entrancy deadlock when restore_config() calls into save_config()
save_config() Now acquires config_write_lock() around write + seal — zero race window
verify_config_integrity(locked=False) New locked kwarg: when True, acquires config_read_lock() before reading, guaranteeing consistent state
restore_config(content, *, reason="") Safe restore helper: acquires lock → backs up current config → writes + reseals atomically. Replaces raw file manipulation in restore scripts like restore_deepseek_config.py

tests/hermes_cli/test_config_lock.py

17 new tests across 5 classes: lock paths, write lock lifecycle, read lock (including concurrent readers), verify_config_integrity (tamper detection, locked mode, seal update on save), and restore_config (dict/YAML string/reason in backup name/seal update after restore).

Implementation note

fcntl.flock(LOCK_EX) is not reentrant on POSIX — re-acquiring an exclusive lock from the same process deadlocks. restore_config() holding the OS lock and calling save_config() (which would re-acquire it) deadlocked in initial testing. Fixed by extracting _write_config_to_disk() as a private helper: save_config() calls config_write_lock() then _write_config_to_disk(); restore_config() acquires both _CONFIG_LOCK and config_write_lock() and calls _write_config_to_disk() directly.


Generated by Claude Code

@github-actions

github-actions Bot commented Jun 29, 2026

Copy link
Copy Markdown

🔎 Lint report: claude/config-write-lock vs origin/main

ruff

Total: 0 on HEAD, 0 on base (➖ 0)

🆕 New issues: none

✅ Fixed issues: none

Unchanged: 0 pre-existing issues carried over.

ty (type checker)

Total: 8653 on HEAD, 8650 on base (🆕 +3)

🆕 New issues (3):

Rule Count
invalid-assignment 2
unresolved-import 1
First entries
hermes_cli/config.py:39: [invalid-assignment] invalid-assignment: Object of type `None` is not assignable to `<module 'msvcrt'>`
hermes_cli/config.py:34: [invalid-assignment] invalid-assignment: Object of type `None` is not assignable to `<module 'fcntl'>`
tests/hermes_cli/test_config_lock.py:8: [unresolved-import] unresolved-import: Cannot resolve imported module `pytest`

✅ Fixed issues: none

Unchanged: 4570 pre-existing issues carried over.

Diagnostics are surfaced as warnings — this check never fails the build.

@dizhaky
dizhaky force-pushed the claude/config-write-lock branch 2 times, most recently from 2e1fe07 to 1b719fc Compare June 29, 2026 21:05
claude added 5 commits June 29, 2026 21:12
- Add config_write_lock() exclusive OS file lock (fcntl.flock / msvcrt)
- Add config_read_lock() shared lock for external readers (e.g. watchdog)
- Extract _write_config_to_disk() to avoid re-entrant lock deadlock
- Wrap save_config() write+seal in exclusive lock — zero race window
- Update verify_config_integrity(locked=False) with optional locked kwarg
- Add restore_config() safe helper: lock → backup → save → reseal
- Tests: 17 cases covering lock lifecycle, integrity, and restore

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MnmdLAbTdbTo66Uc2tK31x
…ports

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MnmdLAbTdbTo66Uc2tK31x
@dizhaky
dizhaky force-pushed the claude/config-write-lock branch from 1b719fc to 8d8c6db Compare June 29, 2026 21:13
@dizhaky
dizhaky marked this pull request as ready for review June 29, 2026 21:13
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@github-actions
github-actions Bot merged commit 455335a into main Jun 29, 2026
27 checks passed
@github-actions
github-actions Bot deleted the claude/config-write-lock branch June 29, 2026 21:13
dizhaky added a commit that referenced this pull request Jul 7, 2026
…writes (#82)

save_config()/restore_config() already keep the local .sha256 sidecar in
sync (PR #57), but the *external* git-backed baseline used by
`hermes config verify` (PR #67) was only ever updated by an explicit
`hermes config seal`. Any authorized write through save_config() — model
scanner, /model command, platform setup flows via
write_platform_config_field() — desynced that baseline, so the Config
Integrity Watchdog cron job flagged the legitimate change as tampering:
"Hermes Config Integrity Failure! The configuration hash does not match
the sealed baseline." This is the same scanner/watchdog TOCTOU conflict
documented in docs/plans/2026-07-02-scanner-watchdog-conflict-resolution.md,
just recurring one layer over in the newer git-backed mechanism.

_write_config_to_disk() now also calls the config-integrity-watchdog
skill's seal() (quietly) whenever $HERMES_DOTFILES_DIR is configured,
keeping the git-backed log current on every authorized write. Extracted
_find_core_module() (returns None instead of exiting) so this can
opportunistically no-op on machines without the watchdog set up.
restore_quick_snapshot() in backup.py gets the same treatment for
config.yaml restores from a quick snapshot.

Co-authored-by: Test <test@test.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants