fix(run): regenerate persisted authority key on demand when missing#218
Merged
Conversation
The persisted authority-config path (added in #174) read the configured key_file directly but never generated it when absent, despite the doc comments promising on-demand generation and referencing an ensure_authority_key helper that was never implemented. When key_file lives in a volatile location (e.g. $XDG_RUNTIME_DIR, a tmpfs cleared on logout), the persisted config keeps pointing at it after a reboot even though the file is gone, so 'firma run' failed closed with 'failed to read key file … No such file or directory'. Implement ensure_authority_key (idempotent: reuse an existing key so issued tokens survive restarts; mint one only when missing) and call it from persisted_authority_config. Add a full-stack smoke test that scaffolds a persisted config, wipes the key, and runs 'firma run -- echo hello' end to end — the coverage #174 lacked, since its e2e scenarios always run against a freshly-minted key in a tempdir and never exercise a stale persisted config.
CI runners and macOS lack the bubblewrap sandbox 'firma run' needs, so the end-to-end smoke test failed with 'backend error (bwrap): bubblewrap is not installed'. Restrict it to Linux and skip cleanly when the sandbox is absent; the always-running regenerates_missing_key_file unit test still guards the fix on every platform. The smoke test runs fully wherever bwrap is present.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes FIR-404 — https://linear.app/firma-ai/issue/FIR-404
Summary
firma runfails closed withfailed to read key file … No such file or directorywhen the persisted[authority].key_fileno longer exists on disk — e.g. it lives in$XDG_RUNTIME_DIR(a tmpfs cleared on logout/reboot) and the machine has since rebooted.Root cause
The persisted authority-config path added in #174 reads the configured
key_filedirectly but never generates it when absent, despite:persisted_authority_config’s doc claiming “The key is generated on demand if the configured path has none yet.”generate_authority_key’s doc referencing anensure_authority_keyhelper that was never implemented.Before #174, autostart always minted a fresh ephemeral key in the per-run marker dir, so a missing configured key never mattered. #174 switched to reading the persisted key without regenerating it.
Fix
ensure_authority_key— idempotent: reuse an existing key so issued tokens survive restarts; mint one (creating the parent dir) only when the secret is missing. Called frompersisted_authority_config.Test
crates/firma/tests/run_echo_smoke.rs: scaffolds a real persisted config, wipes the key to mimic the tmpfs wipe, and runsfirma run -- echo helloend to end — asserting the sandboxed command executes and the key is regenerated. Skips gracefully only when the host lacks unprivileged user namespaces.regenerates_missing_key_fileunit test onpersisted_authority_config.#174’s e2e scenarios always run against a freshly-minted key in a tempdir and are
#[ignore]d, so they never exercise a stale persisted config — this adds the missing coverage.Verification
authority autostart failed); patched → printshello, key regenerated.cargo clippy -p firma-run -p firma --testsclean;cargo fmt --checkclean; new + existing tests pass.