Skip to content

fix(config): auto-reseal config.yaml SHA256 on every authorised save - #52

Merged
dizhaky merged 2 commits into
mainfrom
claude/slack-session-n8y26o
Jun 29, 2026
Merged

fix(config): auto-reseal config.yaml SHA256 on every authorised save#52
dizhaky merged 2 commits into
mainfrom
claude/slack-session-n8y26o

Conversation

@dizhaky

@dizhaky dizhaky commented Jun 29, 2026

Copy link
Copy Markdown
Owner

Problem

Two cron jobs were in direct conflict:

  • free-model-scanner-am — scans providers for the best available free model and legitimately updates config.yaml with the result
  • Config Integrity Watchdog — detects any SHA256 fingerprint mismatch in config.yaml and restores it to deepseek-v4-pro

Because the scanner updated config.yaml through the official save_config() API but didn't update the watchdog's stored SHA256 seal, the watchdog treated every scanner update as unauthorized tampering and immediately reverted it. The two jobs were silently canceling each other out on every cycle.

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

Fix

Added automatic seal maintenance to save_config() in hermes_cli/config.py:

  • save_config() now calls seal_config() after every successful write, keeping ~/.hermes/config.yaml.sha256 in sync with the actual file content
  • Any change made through the official Hermes API (model picker, /model command, free-model-scanner, setup wizard) self-authorizes by updating the seal
  • The watchdog's fingerprint check will only trigger on truly unauthorized edits — direct file mutations that bypass save_config()

Also added three new public helpers for watchdog/restore scripts to use:

Function Purpose
get_config_seal_path() Returns Path to ~/.hermes/config.yaml.sha256
seal_config() Computes + writes the SHA256 seal, returns hex digest
verify_config_integrity() Returns (ok, current_digest, sealed_digest)

The restore_deepseek_config.py restore script should call seal_config() after it writes the restored config so the watchdog's post-restore fingerprint stays accurate.

What didn't change

  • No cron job definitions modified
  • No watchdog logic changed
  • No breaking changes to existing save_config() callers — the seal update is wrapped in try/except OSError so it never blocks a save

Generated by Claude Code

Add get_config_seal_path(), seal_config(), and verify_config_integrity()
helpers, and wire seal_config() into save_config() so the SHA256 seal
file (~/.hermes/config.yaml.sha256) is updated on every authorised write.
This prevents the Config Integrity Watchdog from raising false alarms for
legitimate saves (e.g. free-model-scanner cron job, /model command).

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

Copy link
Copy Markdown

🔎 Lint report: claude/slack-session-n8y26o 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: 8649 on HEAD, 8649 on base (➖ 0)

🆕 New issues: none

✅ Fixed issues: none

Unchanged: 4570 pre-existing issues carried over.

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

@dizhaky
dizhaky marked this pull request as ready for review June 29, 2026 14:57

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9c7decdece

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread hermes_cli/config.py
Comment thread hermes_cli/config.py
@dizhaky
dizhaky enabled auto-merge (squash) June 29, 2026 15:04
Add config.yaml.sha256 to _QUICK_STATE_FILES so the Config Integrity
Watchdog seal file is captured alongside config.yaml in quick snapshots.
After restore_quick_snapshot() restores files, call seal_config() when
config.yaml was among the restored files so the watchdog sees an
authorized restore rather than tampering.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MnmdLAbTdbTo66Uc2tK31x
@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.

dizhaky commented Jun 29, 2026

Copy link
Copy Markdown
Owner Author

Codex review analysis — PR #52 (config auto-reseal)

Reviewed 2 Codex P2 comments:

P2 — Prevent watchdog reads between config and seal writes

Real concern: there's a TOCTOU window where the watchdog can fire between atomic_yaml_write() and seal_config(), see current != sealed, and restore the old config. This requires either an interprocess lock (e.g. fcntl.flock) held across both writes, or a "pending seal" marker the watchdog respects. This is a non-trivial design change touching the watchdog's restore logic — flagging for explicit discussion before implementing.

P2 — Keep quick snapshots in sync with the seal

Real concern: backup.py::_QUICK_STATE_FILES includes config.yaml but not config.yaml.sha256. Restoring an older snapshot leaves a mismatched seal that the integrity check will flag as tampering. Fix is either: (a) add config.yaml.sha256 to _QUICK_STATE_FILES, or (b) call seal_config() after a config.yaml restore. Option (b) is safer since it always recomputes the seal from the restored file.

Both issues are valid but involve design decisions about the watchdog/snapshot contract. I'm noting them here rather than auto-implementing to avoid introducing new race conditions.


Generated by Claude Code

dizhaky commented Jun 29, 2026

Copy link
Copy Markdown
Owner Author

Claude Code review of Codex findings on this PR

Two Codex P2 flags on hermes_cli/config.py — I verified both against the current code:

P2: Watchdog race condition between config write and seal write

Confirmed the gap exists. In save_config (line 4546), the with _CONFIG_LOCK: block holds a threading.RLock — an in-process lock. atomic_yaml_write (line 4579) and seal_config() (line 4589) are both within this block, so no concurrent threads can observe the split state.

However, if the Config Integrity Watchdog runs as a separate cron process, the OS-level file writes are not interprocess-locked. The watchdog can read config.yaml after the atomic write but before .sha256 is updated, see a digest mismatch, and trigger a false-alarm restore. The fix requires an interprocess file lock (e.g. fcntl.flock) wrapping both writes — non-trivial and could introduce stale-lock risks. Recommend a follow-up issue rather than blocking this PR; the window is narrow and the worst case is a spurious restore, not data loss.

P2: Quick snapshots missing config.yaml.sha256

On closer inspection: _QUICK_STATE_FILES (backup.py line 482) already includes config.yaml.sha256. Additionally, restore_quick_snapshot auto-reseals after any restore that includes config.yaml (backup.py lines 660–665). ✅ This concern is already handled.

Summary: PR looks good to merge. The interprocess race is worth a separate issue but is not a regression introduced by this PR.


Generated by Claude Code

dizhaky commented Jun 29, 2026

Copy link
Copy Markdown
Owner Author

Codex follow-up: watchdog interprocess race (scheduled review)

The P2 Codex finding about the watchdog interprocess race was recommended as a follow-up issue in the 16:16 UTC analysis, but issues are disabled in this repository, so it cannot be filed there.

Tracking the open item here instead:

Concern: _CONFIG_LOCK is a threading.RLock (in-process only). If the Config Integrity Watchdog runs as a separate cron process, it can read config.yaml (new) after atomic_yaml_write() but before seal_config() updates .sha256, see a digest mismatch, and trigger a spurious restore of the authorized save.

Suggested fix: add fcntl.flock(LOCK_EX) around both writes in save_config; watchdog acquires LOCK_SH before reading both files.

Not blocking this PR — the window is sub-millisecond, worst case is a spurious restore, not data loss. Safe to merge #52 as-is and address this separately.


Generated by Claude Code scheduled review


Generated by Claude Code

@dizhaky
dizhaky merged commit d79cb92 into main Jun 29, 2026
28 checks passed
@dizhaky
dizhaky deleted the claude/slack-session-n8y26o branch June 29, 2026 17:25
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