fix(config): never mint a sealing key over existing sealed secrets

Merged
BN
8d ago
+650-117

What & why

Fixes #653.

[secrets] values in config.toml are sealed with a data-encryption key that lives in the engine's credential store. When that store could not supply the key, the engine treated the key as missing and minted a new random one. A new key cannot open values sealed with the old one, so the engine home was bricked in both directions:

  • while the store stayed locked, the load failed with store envelope authentication failed;
  • after unlocking, it failed with config-secrets key conflict, because a minted secrets.key disagreed with the keychain copy.

Incident. macstudio, reached only over SSH. The locked login keychain let the engine mint over its sealed secrets. launchd respawned the engine 3,513 times. preloop status printed only Error: engine exited before becoming ready: exit status: 1, because the engine's stderr goes to engine.log, not the terminal. An orphan secrets.key from Oct 10 also disagreed with the keychain key.

Mechanism. The default file store migrates from the keychain on a miss, and it reports itself available even when the keychain is locked. A locked keychain therefore made the lookup return nothing, and the "no entry" branch minted a key into the file store. The store.available() branch that writes secrets.key is reached only by a store that reports itself unavailable, so the two paths need separate handling. The fix covers both.

Changes

  • Never mint over sealed values. When any sealed:v1: value exists for the config directory, a key is returned only if it opens them, and no key is minted. A locked or empty store fails the load and names the store, the reason, and the unlock command (security unlock-keychain ~/Library/Keychains/login.keychain-db on macOS). A genuinely fresh home still mints as before.
  • Settle store/secrets.key disagreements by evidence. Each key is trial-decrypted against the sealed values. The key that opens them wins. The superseded file is renamed to secrets.key.orphan-<unix-ts>, never deleted. If neither key opens them, the error names both. With no sealed values, the store key wins and the file is retired the same way.
  • Write path. write_config_to also counts sealed values already in the file on disk, so a write cannot re-key a config whose store is unreadable.
  • CLI auto-start. A failed background start now includes the tail of this attempt's engine.log (from the offset where the attempt began), so the unlock instruction reaches the terminal. This is the only CLI change.
  • Docs. docs/setup.md (startup behavior with sealed secrets), docs/self-hosting.md (state table row for secrets.key), CHANGELOG.md (Fixed).

Not changed: the sealing format, the credential-store backends, and launchd/plist handling.

Not covered. A home that a pre-fix build already re-keyed (a key minted into the file store while the keychain was locked) is not repaired automatically. It now fails with an explicit does not open the sealed config secrets error that names the source to fix, instead of an authentication failure. The keychain entry is not touched.

Protocol surface

  • I confirm this change touches the runner protocol interface: NO (config-file key handling and CLI startup diagnostics only)

Required gates

  • just test-ci passes locally: not run. Scope was the targeted tests and clippy below, per the change assignment. Run the full gate before relying on this change beyond the targeted tests.
  • No protocol or wire shapes changed, so no runner-watch conformance run applies.
  • Property tests: not applicable to the touched code.
  • No new wire fields or events.
  • No secrets, credentials, or deployment paths introduced. Test fixtures use dummy keys.

Verification performed

  • cargo test -p preloop-runner-server --locked --lib config::tests::: 45 passed, 0 failed. New or changed by this PR:
    • locked_store_never_mints_over_sealed_secrets
    • locked_store_never_mints_on_the_write_path
    • fresh_home_with_a_locked_store_mints_the_file_key
    • reachable_store_without_an_entry_never_mints_over_sealed_secrets
    • unopened_key_file_is_kept_out_of_an_empty_store
    • orphan_key_file_is_renamed_aside_when_the_store_key_opens_the_secrets
    • file_key_is_used_when_the_store_key_does_not_open_the_secrets
    • conflicting_keys_without_sealed_values_prefer_the_store
    • conflicting_keys_that_neither_open_the_secrets_fail_naming_both (replaces conflicting_store_and_file_keys_fail_closed, whose "fail closed on any conflict" rule no longer holds)
    • sealed_values_are_found_in_raw_text_even_when_the_file_is_damaged
  • cargo test -p preloop-cli --locked engine_log_tail: 1 passed (engine_log_tail_quotes_only_this_starts_output).
  • cargo clippy -p preloop-runner-server --locked --all-targets -- -D warnings: clean.
  • cargo clippy -p preloop-cli --locked --all-targets -- -D warnings: clean.
  • cargo fmt --check (pinned toolchain) for preloop-runner-server and preloop-cli: clean.

Checklist

  • Tests added or updated for the changed behavior
  • Docs updated (docs/setup.md, docs/self-hosting.md)
  • Changelog entry under Unreleased / Fixed

Fixes #653

Devin Review

Summary by cubic

Fixes #653. The engine no longer mints a new sealing key over existing sealed [secrets] values. A locked or empty credential store previously looked like a missing key, so the engine minted a fresh key that could not open values sealed with the old one, bricking the engine home with store envelope authentication failed and, after unlocking, config-secrets key conflict. Now the load fails with an error that names the store, the reason, and the unlock command (security unlock-keychain on macOS). A genuinely fresh home without sealed values still mints as before.

When the store and a secrets.key file disagree, the key that opens the sealed values wins; the superseded file is renamed secrets.key.orphan-<unix-ts>, never deleted. If neither opens them, the error names both. The write path also counts sealed values already on disk, so a write cannot re-key a config whose store is unreadable. A failed background engine start now prints the tail of this attempt's engine.log, so the error reaches the terminal.

Migration

  • Homes re-keyed by a pre-fix build are not repaired automatically; they now fail with an explicit does not open the sealed config secrets error rather than an authentication failure.

Summary by CodeRabbit

  • Bug Fixes
    • Prevented key creation or replacement when sealed secrets cannot be safely unlocked. Conflicting legacy key files are preserved under a timestamped name rather than deleted.
    • Background engine startup errors now include the log path and recent relevant log lines.
  • Documentation
    • Clarified how credential keys are stored, migrated, and handled when locked or conflicting.