fix(set): preserve existing remote key name by jdx · Pull Request #712 · jdx/fnox

jdx · 2026-08-12T21:52:10Z

Summary

preserve an existing remote provider key name when fnox set updates a secret
keep --key-name as the highest-precedence override
fall back to the environment key when creating a secret or switching providers
repair the GCP Secret Manager integration test to assert and clean up the configured key

Root cause

fnox set reused the existing provider but resolved remote key names only from --key-name or the environment key. It then persisted that fallback, replacing a custom value in the config and writing to the wrong remote secret.

Validation

mise run test:cargo
mise run lint
mise run build
mise run test:bats -- test/gcp_secret_manager.bats (10 credential-gated tests skipped because GCP credentials are unavailable)

Fixes #711

AI-assisted — Tool: Codex; model: openai/gpt-5; version: unavailable.

Note

Medium Risk
Changes how remote secrets are named and written on every set for storage providers, and Vault put_secret now has complex patch/CAS paths where a bug could corrupt or mis-route secret data.

Overview
Fixes fnox set so updates to remote-backed secrets keep the configured provider value (custom remote key) instead of overwriting it with the env var name and writing to the wrong backend object. --key-name still wins; switching providers falls back to the env key. Only secrets that belong to the file/profile being written supply that reuse—inherited parent config no longer steers a child override.

HashiCorp Vault put_secret is reworked: secret/field references, kv patch for field updates (siblings preserved), values via piped stdin (so @ and - are literal), plus create/retry/CAS handling for missing paths, races, and soft-deleted KV v2 metadata.

CI: Windows mise-action sets fetch_from_github: false. New test/set_remote_key.bats, expanded Vault BATS, and a GCP SM create test fix (generated secret name + cleanup).

^{Reviewed by Cursor Bugbot for commit 34a6052. Bugbot is set up for automated code reviews on this repo. Configure here.}

Summary by CodeRabbit

New Features
- Secret updates now reuse an existing provider key when applicable.
- Explicit key names take precedence, with environment-based keys used as a fallback.
- Vault secrets can be updated by field while preserving other fields.
- Secret references support nested paths and default fields.
Bug Fixes
- Improved handling when switching secrets between remote providers.
- Prevented inherited keys from being reused for local overrides.
- Preserved dry-run behavior during remote key resolution.
- Improved Vault secret creation, recreation, concurrent updates, and cleanup.
- Secret Manager creation and cleanup now work reliably with generated names.

coderabbitai · 2026-08-12T21:52:40Z

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

@coderabbitai resume to resume automatic reviews.
@coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

▶️ Resume reviews
🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited), Organization UI (inherited)

Review profile: CHILL

Plan: Pro Plus

Run ID: 66bb54dd-16b1-478f-8106-90b639826bcc

📥 Commits

Reviewing files that changed from the base of the PR and between 251884f and a59baa0.

📒 Files selected for processing (1)

crates/fnox-core/src/providers/vault.rs

📝 Walkthrough

Walkthrough

The set command resolves remote keys from the write target and profile. It reuses compatible target-owned keys and falls back to the environment key. Vault now supports field-specific secret updates. Tests cover key resolution, GCP cleanup, and Vault field preservation.

Changes

Remote key and Vault updates

Layer / File(s)	Summary
Set target and key resolution `src/commands/set.rs`	The command resolves the write target before provider operations. It filters inherited secrets, selects the write-profile provider, and applies explicit-key, same-provider, and environment-key precedence.
Set resolution validation `src/commands/set.rs`, `test/set_remote_key.bats`, `test/gcp_secret_manager.bats`	Tests cover key reuse, dry-run behavior, provider switching, inherited-key exclusion, write-profile settings, and cleanup of generated GCP secret names.
Vault secret reference and field updates `crates/fnox-core/src/providers/vault.rs`, `test/vault.bats`	Vault parses bare and field-qualified references. Field-qualified writes use `patch`, while bare keys retain whole-secret `put` behavior. Integration tests verify sibling fields remain unchanged, deleted secrets can be recreated, and concurrent field creation preserves sibling fields.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Mergeability Score: ⚪ Minimal · up to a59ba

This change preserves configured remote key names when updating secrets and adds related provider coverage; no actionable merge-blocking risk remains after normal checks and review.

Sequence Diagram(s)

sequenceDiagram
  participant User
  participant SetCommand
  participant WriteProfile
  participant RemoteProvider
  participant VaultCLI
  User->>SetCommand: invoke fnox set
  SetCommand->>WriteProfile: resolve target and provider
  SetCommand->>RemoteProvider: resolve compatible remote key
  SetCommand->>VaultCLI: write whole secret or selected field
  VaultCLI-->>SetCommand: return write result

Poem

I’m a rabbit with keys in a row,
The write profile tells where to go.
Matching keys hop into place,
Vault patches one field with grace.
GCP names stay known for cleanup bright.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the pull request's primary change: preserving existing remote key names during updates.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

greptile-apps · 2026-08-12T21:54:12Z

Greptile Summary

The PR preserves existing remote key names during fnox set and adds field-aware Vault writes with missing-secret and concurrency recovery. The Vault recovery remains incomplete for permanently destroyed KV v2 versions.

Resolves providers and existing remote keys from the write target/profile.
Uses Vault patch operations to preserve sibling fields and stdin to preserve literal values.
Adds remote-key, GCP Secret Manager, and Vault integration coverage.

Confidence Score: 4/5

The PR is not yet safe to merge because permanently destroyed Vault field references still cannot be recreated.

The recovery path classifies destroyed KV v2 versions as soft-deleted and attempts a CAS write against the destroyed current version, after which the fallback patch still targets a missing secret.

Files Needing Attention: crates/fnox-core/src/providers/vault.rs, test/vault.bats

Important Files Changed

Filename	Overview
src/commands/set.rs	Preserves target-owned remote key names while retaining explicit key-name precedence and write-profile provider resolution.
crates/fnox-core/src/providers/vault.rs	Adds shared field-reference parsing and race-aware Vault writes, but destroyed KV v2 versions remain unrecreatable.
test/vault.bats	Covers sibling preservation, soft-delete recreation, literal values, and concurrent creation, but not permanently destroyed versions.
test/set_remote_key.bats	Exercises existing-key preservation, dry runs, inherited secrets, and write-profile resolution.
test/gcp_secret_manager.bats	Aligns creation assertions and cleanup with the configured remote secret name.

_{Reviews (8): Last reviewed commit: "fix(deps): stabilize windows mise setup" | Re-trigger Greptile}

coderabbitai

Actionable comments posted: 1

🧹 Nitpick comments (1)

src/commands/set.rs (1)
368-409: 🗄️ Data Integrity & Integration | 🔵 Trivial | 🏗️ Heavy lift

Cover the command wiring, not only the helper.

These tests call resolve_remote_key_name directly. They do not verify that SetCommand::run selects the existing provider, passes the resolved name to put_secret, and persists the name during an update. Add a mocked-provider regression test for an existing same-provider secret and a dry-run update.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/commands/set.rs` around lines 368 - 409, Add regression tests covering
SetCommand::run command wiring, not just resolve_remote_key_name: mock the
provider for an existing same-provider secret and verify the selected provider
and resolved name are passed to put_secret, then add a dry-run update case
verifying the resolved name is persisted without performing the write. Reuse the
existing test helpers and provider-mocking symbols in the module.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/commands/set.rs`:
- Around line 109-115: Update the provider-resolution logic in the set command
around existing_secret and provider_name_to_use to use the effective
write-profile selected by --write-profile, rather than the active --profile.
Apply that write-profile scope to both existing-secret and provider lookups
while preserving explicit --provider behavior, and add a regression test
covering distinct --profile and --write-profile values.

---

Nitpick comments:
In `@src/commands/set.rs`:
- Around line 368-409: Add regression tests covering SetCommand::run command
wiring, not just resolve_remote_key_name: mock the provider for an existing
same-provider secret and verify the selected provider and resolved name are
passed to put_secret, then add a dry-run update case verifying the resolved name
is persisted without performing the write. Reuse the existing test helpers and
provider-mocking symbols in the module.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited), Organization UI (inherited)

Review profile: CHILL

Plan: Pro Plus

Run ID: e1570743-7a11-4bcf-8fbe-f61e3ce4fbd9

📥 Commits

Reviewing files that changed from the base of the PR and between f978acc and e6480a7.

📒 Files selected for processing (2)

src/commands/set.rs
test/gcp_secret_manager.bats

github-actions · 2026-08-12T21:58:02Z

Instruction counts

benchmark	trend	instructions	Δ	wall (min)	Δ
schema	`██▇▇█▁▆▆▄▄▇▇▅▂▂▇▄▅▅▆▃`	7,115,712 → 7,112,875	-0.04%	3.37 → 3.21ms	-4.78%
usage	`▄▄▃▃▃▁███▇▇▇▇▆▆▇▇▇▇▇█`	10,915,997 → 10,917,065	+0.01%	3.53 → 3.51ms	-0.34%

No instruction-count regression above 1.25%.

_{Only instruction counts gate. Wall clock is shown for context — on identical hardware it moves 4-20% run to run.}

_{Measured by tak — instruction-counted CLI benchmarks, stored in this repository's git notes.}

_{34a60527bd1f vs f978acc9d477 · measured on the runner, not pushed to the history.}

coderabbitai

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@test/vault.bats`:
- Around line 136-159: Register failure-safe cleanup immediately after
create_test_vault_secret assigns secret_name, using the test’s established
teardown or trap mechanism to call delete_test_vault_secret with that name.
Ensure cleanup runs when any subsequent command or assertion in this test fails,
while preserving the existing final cleanup behavior without causing duplicate
deletion.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited), Organization UI (inherited)

Review profile: CHILL

Plan: Pro Plus

Run ID: 6ec05def-142f-46a4-929c-fcf356c4987f

📥 Commits

Reviewing files that changed from the base of the PR and between e6480a7 and 63f2bfa.

📒 Files selected for processing (4)

crates/fnox-core/src/providers/vault.rs
src/commands/set.rs
test/set_remote_key.bats
test/vault.bats

coderabbitai

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@crates/fnox-core/src/providers/vault.rs`:
- Around line 252-257: Update the missing-secret fallback in the vault write
flow around execute_vault_command to use kv put with -cas=0, preventing
overwrite of a concurrently created secret. If that create-only write fails,
retry the original kv patch operation and propagate any resulting error. Add an
integration test covering the concurrent-creation race.
- Around line 251-260: Update the Vault write flow around execute_vault_command
to prevent secret values beginning with @ from being interpreted as file paths
and values equal to - from being read from process stdin. Escape leading @
values before constructing patch_args or put_args, and route - values through a
controlled stdin pipe; add regression tests covering both behaviors.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited), Organization UI (inherited)

Review profile: CHILL

Plan: Pro Plus

Run ID: a2c33223-aa90-4fe5-a0f0-9857babb22c2

📥 Commits

Reviewing files that changed from the base of the PR and between 63f2bfa and b8d633f.

📒 Files selected for processing (2)

crates/fnox-core/src/providers/vault.rs
test/vault.bats

🚧 Files skipped from review as they are similar to previous changes (1)

test/vault.bats

cursor

Cursor Bugbot has reviewed your changes and found 1 potential issue.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit 251884f. Configure here.}

@jdx

### 🐛 Bug Fixes - **(set)** preserve existing remote key name by [@jdx](https://github.com/jdx) in [#712](#712) ### 📚 Documentation - fix sponsor logo sizing by [@jdx](https://github.com/jdx) in [#714](#714) ### 🛡️ Security - **(deps)** update dependency @anthropic-ai/claude-code to v2.1.221 by [@renovate[bot]](https://github.com/renovate[bot]) in [#705](#705) ### 📦️ Dependency Updates - simplify cargo version requirements by [@jdx](https://github.com/jdx) in [#694](#694) - update rust crate usage-lib to v5.1.0 by [@renovate[bot]](https://github.com/renovate[bot]) in [#695](#695) - update rust crate google-cloud-secretmanager-v1 to v1.12.0 by [@renovate[bot]](https://github.com/renovate[bot]) in [#706](#706) - update jdx/mise-action action to v4.2.4 by [@renovate[bot]](https://github.com/renovate[bot]) in [#697](#697) - update rust crate aho-corasick to v1.1.5 by [@renovate[bot]](https://github.com/renovate[bot]) in [#698](#698) - update rust crate data-encoding to v2.11.1 by [@renovate[bot]](https://github.com/renovate[bot]) in [#700](#700) - update rust crate clap to v4.6.5 by [@renovate[bot]](https://github.com/renovate[bot]) in [#699](#699) - update rust crate ignore to v0.4.32 by [@renovate[bot]](https://github.com/renovate[bot]) in [#701](#701) - update rust crate keepass to v0.13.20 by [@renovate[bot]](https://github.com/renovate[bot]) in [#702](#702) - update zizmorcore/zizmor-action action to v0.6.2 by [@renovate[bot]](https://github.com/renovate[bot]) in [#704](#704) - update rust crate rustls to v0.23.43 by [@renovate[bot]](https://github.com/renovate[bot]) in [#703](#703) - update rust crate base64 to v0.23.1 by [@renovate[bot]](https://github.com/renovate[bot]) in [#707](#707) - update rust crate globset to v0.4.20 by [@renovate[bot]](https://github.com/renovate[bot]) in [#708](#708) - update dependency @anthropic-ai/claude-code to v2.1.222 by [@renovate[bot]](https://github.com/renovate[bot]) in [#710](#710) - update rust crate ignore to v0.4.33 by [@renovate[bot]](https://github.com/renovate[bot]) in [#709](#709) - lock file maintenance by [@renovate[bot]](https://github.com/renovate[bot]) in [#715](#715)

fix(set): preserve existing remote key name

e6480a7

greptile-apps Bot reviewed Aug 12, 2026

View reviewed changes

Comment thread src/commands/set.rs

cursor Bot reviewed Aug 12, 2026

View reviewed changes

Comment thread src/commands/set.rs

coderabbitai Bot reviewed Aug 12, 2026

View reviewed changes

Comment thread src/commands/set.rs Outdated

fix(set): scope remote key reuse to write target

63f2bfa

coderabbitai Bot reviewed Aug 12, 2026

View reviewed changes

Comment thread test/vault.bats

greptile-apps Bot reviewed Aug 12, 2026

View reviewed changes

Comment thread crates/fnox-core/src/providers/vault.rs Outdated

cursor Bot reviewed Aug 12, 2026

View reviewed changes

Comment thread crates/fnox-core/src/providers/vault.rs Outdated

fix(vault): create missing field references

b8d633f

coderabbitai Bot reviewed Aug 12, 2026

View reviewed changes

Comment thread crates/fnox-core/src/providers/vault.rs

Comment thread crates/fnox-core/src/providers/vault.rs Outdated

fix(vault): harden field writes

251884f

cursor Bot reviewed Aug 12, 2026

View reviewed changes

Comment thread crates/fnox-core/src/providers/vault.rs

fix(vault): recreate deleted field paths

a59baa0

greptile-apps Bot reviewed Aug 12, 2026

View reviewed changes

Comment thread crates/fnox-core/src/providers/vault.rs

jdx added 3 commits August 12, 2026 22:38

fix(vault): recognize missing patch responses

e8d3c57

test(vault): assert literal dash value

e32a6f7

fix(deps): stabilize windows mise setup

34a6052

greptile-apps Bot reviewed Aug 12, 2026

View reviewed changes

Comment thread crates/fnox-core/src/providers/vault.rs

jdx merged commit 660eb06 into main Aug 13, 2026
19 checks passed

jdx deleted the codex/fix-set-existing-remote-key branch August 13, 2026 00:21

mise-en-dev mentioned this pull request Aug 13, 2026

chore: release v1.33.1 #696

Merged

BrewTestBot mentioned this pull request Aug 17, 2026

fnox 1.33.1 Homebrew/homebrew-core#299235

Merged

github-actions Bot mentioned this pull request Aug 17, 2026

chore: bump bundled fnox to 1.33.1 fullerzz/fnox-py#30

Open

Uh oh!

Conversation

jdx commented Aug 12, 2026 • edited by cursor Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Root cause

Validation

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Aug 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviews paused

Walkthrough

Changes

Sequence Diagram(s)

❌ Failed checks (1 warning)

Uh oh!

greptile-apps Bot commented Aug 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 4/5

Important Files Changed

Uh oh!

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions Bot commented Aug 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Instruction counts

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

cursor Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

jdx commented Aug 12, 2026 •

edited by cursor Bot

Loading

coderabbitai Bot commented Aug 12, 2026 •

edited

Loading

greptile-apps Bot commented Aug 12, 2026 •

edited

Loading

github-actions Bot commented Aug 12, 2026 •

edited

Loading