fix(setup): initialize secrets crypto for env-var security option#706
fix(setup): initialize secrets crypto for env-var security option#706
Conversation
The "Environment variable" option in the setup wizard's security step generated a master key but never initialized `secrets_crypto`, causing subsequent API key saves to fail silently. Fix by: 1. Creating SecretsCrypto from the generated key (matching keychain path) 2. Storing the key hex in settings for write_bootstrap_env to persist 3. Auto-writing SECRETS_MASTER_KEY to ~/.ironclaw/.env 4. Using inject_single_var for thread-safe env overlay 5. Fixing misleading message (shell profiles don't work, only .env) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a critical bug in the setup wizard where selecting the "Environment variable" option for master key storage failed to properly initialize the secrets cryptography context. This oversight prevented subsequent steps, such as API key storage, from functioning correctly. The changes ensure that the crypto context is immediately available, the generated master key is persistently stored in the Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request correctly fixes a bug where the secrets crypto context was not initialized when using the environment variable security option during setup. The changes ensure that secrets_crypto is properly instantiated, allowing subsequent API key encryption to succeed. The master key is also now correctly persisted to ~/.ironclaw/.env for future runs. Additionally, the code is improved by replacing an unsafe std::env::set_var call with a thread-safe equivalent. The new test case effectively validates the fix. The changes are well-implemented and address the issue thoroughly.
Note: Security Review did not run due to the size of the PR.
Summary
Closes #666.
secrets_cryptowhen the user picks "Environment variable" in the setup wizard's security step, so subsequent API key saves workSECRETS_MASTER_KEYto~/.ironclaw/.envviawrite_bootstrap_envso the key persists across restartsinject_single_var(thread-safe overlay) instead ofunsafe set_var.envfiles work (not shell profiles)Root cause
Option 1 (env var) in
step_security()generated the master key and printed it, but never setself.secrets_crypto. This left itNone, causinginit_secrets_context()to fail when trying to encrypt API keys in later wizard steps.Test plan
test_env_var_security_initializes_crypto-- verifies generated key produces validSecretsCryptocargo clippy --all --all-features-- zero warningscargo test --all-features-- all lib tests passGenerated with Claude Code