feat(config): guard against config wipe on parse failure

[Societus]

Body

When config.yaml has a YAML syntax error, the gateway falls back to .env/gateway.json defaults and keeps running. The problem is that any subsequent save_config() call — a model switch, a statusbar toggle — writes those bare defaults back to disk, destroying the user's custom_providers, mcp_servers, matrix config, everything that wasn't in the fallback. This is particularly nasty for credential hygiene: _load_cfg() swallows the parse error and returns {}, so any TUI-triggered config write calls save_config({}), which normalizes against DEFAULT_CONFIG and writes defaults to disk — stripping every custom provider and mass-orphaning their credentials in auth.json at once.

This adds an environment flag (HERMES_CONFIG_PARSE_FAILED) that the gateway sets on parse failure. save_config() and save_config_value() check it and refuse to write. The gateway keeps running on fallback defaults, sessions keep working, but the broken config stays untouched on disk. The flag clears on the next successful load.

For recovery, hermes config recover reads the broken YAML and error, calls an LLM directly to fix the syntax, validates the output, and writes the repaired config through write_raw_config() — a bypass that skips the guard intentionally. The TUI gateway also gets config.recover_status and config.recover_write RPC methods for frontend integration.

Changes

• gateway/config.py: sets/clears guard flag, saves config.yaml.broken / config.yaml.error
• hermes_cli/config.py: save_config() checks guard; adds write_raw_config() for recovery
• cli.py: save_config_value() checks guard
• utils.py: atomic_yaml_write() gains raw_text parameter
• hermes_cli/main.py: adds hermes config recover --model <spec> subcommand
• tui_gateway/server.py: adds config.recover_status and config.recover_write RPC methods

Testing

65 existing config/gateway tests pass. Guard verified: flag blocks save_config, file mtime unchanged.

[teknium1]

Thanks for targeting a real destructive-config failure mode. Current main still has a fresh-process variant: hermes_cli/config.py:6937-6993 falls back to defaults when no last-known-good config exists, while tui_gateway/server.py:13929-13944 loads and saves that config during tools.configure.

Problems

• The new flag is set only by gateway/config.py:778. The TUI mutation path does not call load_gateway_config(); its sole call is the handoff flow at tui_gateway/server.py:6360. A fresh TUI process with invalid YAML can therefore still reach save_config() without the guard.
• _config_recover includes complete raw YAML in an OpenAI request. Config files can contain API keys (cli.py:3657), so this may disclose credentials to the repair provider.
• The diff adds recovery RPC methods but no ui-tui/ client integration, leaving them unused.

Suggested changes

• Put parse-validity refusal at the shared config write boundary, rather than in a gateway-local environment flag.
• Add a fresh-process TUI regression that asserts malformed-config bytes are unchanged after tools.configure.
• Split or redact-and-confirm the LLM recovery flow before sending config contents externally.

Automated hermes-sweeper review.

[teknium1]

This flag is set only when load_gateway_config() runs. tui_gateway/server.py's tools.configure path calls shared load_config() then save_config() directly, so a fresh TUI process with invalid YAML never receives this guard. Put parse-validity refusal in the shared write boundary and cover that path with an E2E regression.

[ruangraung]

Hey @Societus — thanks for putting this PR together, it targets a genuinely destructive failure mode. I hit it in production today, and the trigger is exactly the fresh-process gap @teknium1 flagged in review. Wanted to share my reproduction + evidence in case it helps move this forward.

What happened

Ran hermes update on a Linux VPS with a systemd-linger dashboard. Config was at _config_version: 33 (current latest) before the pull, so migrate_config() ran zero version-step migrations — the migration pass was not the writer. Restarted the dashboard service afterward to pick up the new code. The fresh process booted, and a save_config call (triggered by a dashboard API request, not an unconditional boot-path write) read config.yaml, but read_raw_config() returned {} — a transient parse/read failure on the file the update had just touched. save_config({}) then proceeded: the preserve set (built from read_raw_config() at config.py:7222-7224) was empty, so _strip_default_values dropped every non-default section — providers, fallback_providers, secrets, mcp_servers, platform_toolsets, all of it.

What was lost: all custom providers, fallback provider chains, secrets provider configuration, MCP server configs, platform toolset assignments, and plugin enable/disable state. The dashboard UI showed every tab blank — plugins, MCPs, memory provider, compression engine, all reset to defaults. Recovery was possible only because a pre-update state snapshot had captured the intact config + env files minutes before the pull.

Forensic evidence

The gateway boot on new code showed a degraded max_iterations value — 60 instead of the configured 90 (and DEFAULT_CONFIG also has 90), indicating the config was already stripped by that boot:

2026-07-15 04:13:49,415 INFO gateway.run: Agent budget: max_iterations=60 (agent.max_turns from config.yaml, or HERMES_MAX_ITERATIONS from .env, or default 90)

The config.yaml inode Birth timestamp matched the moment I began manually rebuilding the file (after noticing the wipe), confirming the original file had been overwritten. No explicit save_config log line exists (the function doesn't log its writes), but the timeline aligns: update → restart → degraded config visible at boot → I notices wipe.

Why this PR's current guard wouldn't have caught it

The HERMES_CONFIG_PARSE_FAILED flag is set only by gateway/config.py. The dashboard boot path does not go through load_gateway_config() — it reaches save_config() via HTTP request handlers without the guard ever being set. This is the exact gap @teknium1 identified: "A fresh TUI process with invalid YAML can therefore still reach save_config() without the guard."

Code-level root cause (verified on current main)

• read_raw_config() (config.py:6737-6742) catches any exception and returns {} — no last-known-good fallback. The LKG retention from fix(config): retain last-known-good config when config.yaml fails to parse #60591 (fe25806a6) was added to load_config() only; read_raw_config() was left unguarded.
• save_config() calls require_readable_config_before_write() then atomic_yaml_write(). But that guard only checks the file is readable (stat() + read(1)), not that it parses to non-empty. A file that exists and is readable but parses to {} sails through.
• save_config builds its preserve set from read_raw_config() (config.py:7222-7224). If that read returned {}, the preserve set is empty, and _strip_default_values strips every non-default section.

Suggested fix (prototyped locally)

Implementing @teknium1's review note: move parse-validity refusal into save_config itself — the shared write boundary. After require_readable_config_before_write, if the file exists and is non-empty but read_raw_config() returned {}, refuse to write:

# Fail-closed guard: if config.yaml exists and is non-empty on
# disk but read_raw_config() returned {}, the file failed to
# parse. Proceeding would build an empty preserve-set, causing
# _strip_default_values to drop every non-default section.
try:
    _existing_size = config_path.stat().st_size
except OSError:
    _existing_size = 0
if _existing_size > 0 and not read_raw_config():
    logger.warning(
        "Refusing to save config: %s is non-empty (%d bytes) but "
        "read_raw_config() returned empty — the file likely failed "
        "to parse. Writing now would wipe all non-default sections. "
        "Fix the YAML or retry.",
        config_path,
        _existing_size,
    )
    return

Regression test simulates the defect — monkeypatches read_raw_config to return {} while a real config with providers/fallback_providers sits on disk, then asserts save_config leaves the file bytes unchanged:

def test_save_config_refuses_when_read_returns_empty_on_nonempty_file(_isolated_config):
    config_path = _isolated_config / "config.yaml"
    original_content = (
        "model:\n  default: test-model\n  provider: test-provider\n"
        "providers:\n  test-provider:\n    api: https://example.com/v1\n"
        "    key_env: TEST_API_KEY\n    name: Test\n"
        "fallback_providers:\n  - provider: test-provider\n    model: fallback-model\n"
        "_config_version: 33\n"
    )
    config_path.write_text(original_content, encoding="utf-8")
    original_bytes = config_path.read_bytes()

    # Simulate transient parse failure
    monkeypatch.setattr(cfg_mod, "read_raw_config", lambda: {})
    save_config({"model": {"default": "new-model", "provider": "new-provider"}})

    assert config_path.read_bytes() == original_bytes, (
        "save_config overwrote a non-empty config.yaml when "
        "read_raw_config() returned {} — the wipe guard failed!"
    )

3/3 new tests pass. 151/151 existing config tests still pass with the guard in place — no regressions.

Offer to help

Happy to implement this as a focused addition to this PR, or as a follow-up if you'd prefer to keep this PR scoped to the gateway-local flag. Either way, I can provide the full guard implementation + test file. Just let me know what works best!

Environment: Linux (Debian 13) VPS with systemd-linger dashboard.

Hermes Agent v0.18.2 (2026.7.7.2) · upstream 6997dc81
Install method: git
Python: 3.11.15
OpenAI SDK: 2.24.0

[Societus]

@ruangraung — solid reproduction and root-cause analysis. I was checking some update on this when I got the initial sweeper notification, but your comment added some good detail and an expanded on it a bit. I verified every claim against current main: read_raw_config() swallows exceptions to {}, require_readable_config_before_write() only checks file readability via stat + read(1) with no parse validity check, and the tools.configure path at tui_gateway/server.py:14116 calls save_config() without going through load_gateway_config(), so the gateway-local flag in this PR never fires on that path.

I implemented your suggested guard at the shared write boundary. Both save_config() and set_config_value() now refuse to write when config.yaml is non-empty on disk but read_raw_config() returns {}. Three regression tests cover the wipe scenario, the fresh-install edge case, and the set_config_value path. 121/121 config tests pass.

The commit is in def52cc, credited to you as co-author since the reproduction, root-cause analysis, and guard prototype were all yours.

The credential redaction concern @teknium1 raised on the LLM recovery flow is a separate issue I'll address in a follow-up so it doesn't block this one.

[Societus]

I double checked the failing test slices 4,5 and 8 and found no impact from the commits towards the PR. Loading and saving config portions of gateway/config.py, hermes_cli/config.py, cli.py, utils.py, and scripts/release.py should be safe. The failing tests all live in tests/agent/test_compression_concurrent_fork.py, a compression lock test added after this PR opened. It has a timing race on a threading.Event that's unrelated to config parsing.

[ruangraung]

@Societus I dug into the CI failure myself before posting, and I think the cause is different from what we assumed, and it's actually our guard, not a pre-existing test.

The failing tests are not test_compression_concurrent_fork.py. In run 29447346642 the three red slices fail on:

• tests/hermes_cli/test_web_server_profile_unification.py (3 failures, KeyError: 'platforms')
• tests/hermes_cli/test_web_server_messaging_profiles.py (2 failures, assert telegram["enabled"] is True → False)
• tests/hermes_cli/test_web_server_skills_profiles.py (2 failures, toolset/skills scoping)

Root cause: the guard itself. The job logs show the guard firing right before each failure:

WARNING hermes_cli.config:config.py:7285 Refusing to save config:
.../config.yaml is non-empty (3 bytes) but read_raw_config() returned empty
— the file likely failed to parse. Writing now would overwrite all custom
settings with defaults. Fix the YAML syntax, then retry.

Those profile-scoped dashboard tests create a fresh {} config (3 bytes: {} + newline) and then call save_config() to populate it with platforms / telegram / toolsets. The guard sees a non-empty file and read_raw_config() == {} and refuses, so the write never lands and the assertion fails.

Why it misfires: read_raw_config() returns {} for both "genuinely unparseable" and "valid-but-empty {}". The guard can't tell them apart, so it blocks a legitimate write to a valid empty config. I confirmed these same slices are green on main (run 29464923384), so this is a regression the guard introduced, not a pre-existing flake.

Suggested fix: refuse only on a real parse error, not on a valid-empty result. In the guard, attempt the parse inline and only refuse when YAML actually raises:

import yaml
try:
    with open(config_path, "r", encoding="utf-8") as _f:
        yaml.safe_load(_f)
except (yaml.YAMLError, OSError, UnicodeDecodeError):
    logger.warning(
        "Refusing to save config: %s failed to parse — writing now "
        "would wipe all custom settings. Fix the YAML syntax, then retry.",
        config_path,
    )
    return
# a valid `{}` parses fine and falls through to the normal write path

That keeps the original protection (a garbage/unreadable config won't be wiped) while letting a fresh {} config get populated normally.

Heads up on our side too: my local fail-closed patch uses the same if _existing_size > 0 and not read_raw_config() condition, so it has the identical blind spot, it just hasn't bitten us because our real config.yaml is never {}. Once this is corrected upstream I'll align my local patch to the inline-parse version.

Happy to sketch the exact diff + a regression test (a {} config that should still be writable) if that helps. Great work getting the shared-boundary guard in, this is just the last sharp edge before it's safe to merge.

[Societus]

@ruangraung However you'd like to proceed, please feel free to do so, unfortunately yesterday was my last day to have any tinkering time for the week so anything I do next is going to be probably a week or more out.

[ruangraung]

@Societus thank you for the handoff, and nice work on the guard intent and the secret-redaction hardening, good catches.

I've opened a follow-up PR (#NEW) that fixes the CI regression. The issue was that not read_raw_config() can't distinguish a valid empty {} config from an unparseable file — both return {}. The fix replaces the emptiness check with a real inline fast_safe_load() parse: refuse only when the file is non-empty and parsing actually raises an exception.

Applied to both save_config() and set_config_value() (your original scope). 8 regression tests + 59 dashboard profile tests pass cleanly.

When you're back next week, we'd love your review. Happy to cherry-pick your original commits if that's cleaner, or absorb any feedback.

[kshitijk4poor]

Closing — the write-guard half of this PR is now superseded by #96169 (salvage of #71385): the shared require_readable_config_before_write guard parses config.yaml and fail-closes on unparseable / non-mapping roots for every write path (config set/unset, save_config, auth writers, tui-gateway _save_cfg), snapshotting a .corrupt.*.bak before refusing. That mechanism also avoids the cross-process fragility of the HERMES_CONFIG_PARSE_FAILED env flag approach here (per repo policy, behavioral flags don't go through env vars — the guard now checks the file itself at write time, so no flag is needed).

You get credit for being the first to identify and attack this bug class back in April — the "save_config({}) after a swallowed parse error mass-destroys custom_providers" diagnosis in your description is exactly the failure mode the merged fix closes, and #65975/#71385 both built on the trail this PR started.

The hermes config recover LLM-repair feature is a separate idea and isn't covered by the merged work — if you're still interested, that would be welcome as a fresh, scoped PR against current main (the guard now snapshots .corrupt.*.bak files, which gives recover a clean input to work from). Note the unrelated docs/code-review-graph-integration.md file in this branch would need to be dropped.