Skip to content

chore: promote staging to staging-promote/695e6fa1-24607103256 (2026-04-18 15:11 UTC)#2651

Open
ironclaw-ci[bot] wants to merge 1 commit intostaging-promote/695e6fa1-24607103256from
staging-promote/fcb295ba-24607448875
Open

chore: promote staging to staging-promote/695e6fa1-24607103256 (2026-04-18 15:11 UTC)#2651
ironclaw-ci[bot] wants to merge 1 commit intostaging-promote/695e6fa1-24607103256from
staging-promote/fcb295ba-24607448875

Conversation

@ironclaw-ci
Copy link
Copy Markdown
Contributor

@ironclaw-ci ironclaw-ci bot commented Apr 18, 2026 •
edited by github-actions bot
Loading

Auto-promotion from staging CI

Batch range: a53eac5c2dec6b6cd5c08189086093fde64aa9cb..fcb295bad4eb0431fde626a4461446b2b571ef56
Promotion branch: staging-promote/fcb295ba-24607448875
Base: staging-promote/695e6fa1-24607103256
Triggered by: Staging CI batch at 2026-04-18 15:11 UTC

Commits in this batch (118):

Current commits in this promotion (1)

Current base: staging-promote/695e6fa1-24607103256
Current head: staging-promote/fcb295ba-24607448875
Current range: origin/staging-promote/695e6fa1-24607103256..origin/staging-promote/fcb295ba-24607448875

Auto-updated by staging promotion metadata workflow

Waiting for gates:

  • Tests: pending
  • E2E: pending
  • Claude Code review: pending (will post comments on this PR)

Auto-created by staging-ci workflow

@ilblackdragon
fix(secrets): auto-generate master key via .env on every startup (#1820…
fcb295b 
…) (#2648)

* fix(secrets): auto-generate master key via .env fallback on every startup (#1820)

The "secrets store is not available" failure happened when a user reached
`require_secrets_store` (web settings → save API key) but `SecretsConfig::resolve()`
had returned `KeySource::None` because neither `SECRETS_MASTER_KEY` nor an OS
keychain entry existed. This is the normal state on headless Linux, inside a
container without secret-service, or after a partial onboarding that wrote
`ONBOARD_COMPLETED=true` but didn't persist a key.

The wizard's quick-mode `auto_setup_security()` already handled this chain
correctly — env var → keychain read → keychain write → generate + persist to
`~/.ironclaw/.env` as `SECRETS_MASTER_KEY=…` — but that code only ran during
onboarding. If the user's gate slipped past `check_onboard_needed()`, nothing
re-ran it.

Move the chain into `SecretsConfig::resolve()` so it runs on every startup:

- `resolve()` generates and persists a key via `upsert_bootstrap_vars_to()`
  (the same writer the wizard uses) when keychain is unavailable, and
  injects it into the process env overlay so the current run sees it.
- `auto_setup_security()` collapses to a thin caller: `SecretsConfig::resolve()`
  → build `SecretsCrypto` → mirror source/hex into settings → print status.
  This removes the duplicate chain and keeps one persistence format (`.env`)
  and one `KeySource` (`Env`) for the keychain-unavailable path.

Regression test `resolve_persists_generated_key_when_keychain_empty`
drives `resolve_with_env_path()` with a tempfile and asserts the `.env`
carries the generated key. Skips gracefully when the host keychain
already holds a key (developer machines) so we don't wipe a real key.

Supersedes #2312, which added a second persistence format
(`~/.ironclaw/master.key`) + `KeySource::File` variant + `Keystore`
trait alongside the existing `.env` path, with a known
divergence-on-recovery gap between the two stores. Option B here
reuses the single existing path instead.

* ci: re-trigger regression check with skip-regression-check label
@github-actions github-actions bot added scope: setup Onboarding / setup size: L 200-499 changed lines risk: high Safety, secrets, auth, or critical infrastructure contributor: core 20+ merged PRs labels Apr 18, 2026
@claude
Copy link
Copy Markdown

claude bot commented Apr 18, 2026

Code review

Found 5 issues:

  1. [CRITICAL:100] String interpolation error in test assertion
    String placeholder not formatted in error message. The test assertion will print literal {contents} instead of the actual file contents, making test failures impossible to debug.

    https://github.com/anthropics/ironclaw/blob/e7290d8f02987d2ef9ddd6cdffb53bf57098ab7b/src/config/secrets.rs#L180-L185

    // Current (wrong):
".env must carry the generated key; got: {contents}"

// Should be:
".env must carry the generated key; got: {}",contents

  2. [HIGH:100] Race condition in auto_generate_and_persist() when called concurrently
    Multiple concurrent calls to SecretsConfig::resolve() could each generate and persist different master keys. The .env write is not atomic, and the two-step process (file write → env inject) is not transactional. This could cause the persisted key and process env to diverge, leading to decrypt failures. The code should ensure auto-generation happens at most once per process startup using OnceLock or similar synchronization.

    https://github.com/anthropics/ironclaw/blob/e7290d8f02987d2ef9ddd6cdffb53bf57098ab7b/src/config/secrets.rs#L76-L106

  3. [HIGH:75] Missing idempotency test for resolve()
    The regression test calls resolve_with_env_path() only once. If resolve() is called twice (e.g., during config reloads), it should return the same key both times, but the test doesn't verify this. The test should call resolve_with_env_path() twice on the same temp .env and assert identical keys.

    https://github.com/anthropics/ironclaw/blob/e7290d8f02987d2ef9ddd6cdffb53bf57098ab7b/src/config/secrets.rs#L149-L212

  4. [MEDIUM:75] Silent fallback when keychain storage fails
    When store_master_key() fails (line 84), the function silently falls back to writing to .env with only a debug log on success (line 88). If the keychain fails due to transient issues, users won't know the key ended up in the less-secure .env file. Should log info! or debug! when falling back to .env persistence.

    https://github.com/anthropics/ironclaw/blob/e7290d8f02987d2ef9ddd6cdffb53bf57098ab7b/src/config/secrets.rs#L84-L90

  5. [MEDIUM:75] Synchronous file I/O blocks async context on startup
    upsert_bootstrap_vars_to() is a synchronous blocking read-modify-write operation on ~/.ironclaw/.env that now runs on every startup when keychain is unavailable. This blocks the async executor thread during startup, adding latency before the database is available. For large .env files, the line-by-line rebuild is also inefficient.

    https://github.com/anthropics/ironclaw/blob/e7290d8f02987d2ef9ddd6cdffb53bf57098ab7b/src/config/secrets.rs#L92-L99

Secondary findings (lower priority):

  • [MEDIUM:50] Error context lost in wizard: The wizard's error message at line 1210 is a generic "secrets resolve returned no master key" which masks the actual failure reason from the underlying ConfigError. Should preserve and propagate the context.

  • [LOW:50] Test cleanup: delete_master_key() error is silently ignored (line 196). If deletion fails, the test could leave a stray key in the developer's real keychain.

🤖 Generated with Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

contributor: core 20+ merged PRs risk: high Safety, secrets, auth, or critical infrastructure scope: setup Onboarding / setup size: L 200-499 changed lines staging-promotion

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@ilblackdragon