fix(bitwarden): support slashes in custom fields - #693
Conversation
📝 WalkthroughWalkthroughBitwarden references now split at the first ChangesBitwarden reference parsing
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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. Comment |
Greptile SummaryThe PR updates Bitwarden reference parsing to split only at the first slash, preserving subsequent slashes in custom field names.
Confidence Score: 5/5The PR appears safe to merge, with the parser, backend argument handling, regression test, and documentation aligned. The first slash remains the item/field delimiter while the complete remainder is passed as the custom field name, preserving existing references and enabling the intended slash-containing fields. Important Files Changed
Reviews (1): Last reviewed commit: "fix(bitwarden): support slashes in custo..." | Re-trigger Greptile |
There was a problem hiding this comment.
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/bitwarden.bats`:
- Around line 218-237: Update the test setup after item_id is assigned in "fnox
get retrieves custom field containing a slash" to register deletion through the
test teardown or exit-trap mechanism, ensuring delete_test_bw_item runs even
when run, assert_success, or assert_output fails; remove the success-only
cleanup call.
🪄 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: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: d20106d7-bfa6-401e-89b2-2b372900b806
📒 Files selected for processing (3)
crates/fnox-core/src/providers/bitwarden.rsdocs/providers/bitwarden.mdtest/bitwarden.bats
| @test "fnox get retrieves custom field containing a slash" { | ||
| create_bitwarden_config | ||
|
|
||
| item_info=$(create_test_bw_item) | ||
| item_id=$(echo "$item_info" | cut -d'|' -f1) | ||
| item_name=$(echo "$item_info" | cut -d'|' -f2) | ||
|
|
||
| cat >>"${FNOX_CONFIG_FILE}" <<EOF | ||
|
|
||
| [secrets.INVALID_ITEM] | ||
| [secrets.TEST_SLASH_FIELD] | ||
| provider = "bitwarden" | ||
| value = "nonexistent-item-$(date +%s)" | ||
| value = "$item_name/API/Key" | ||
| EOF | ||
|
|
||
| # Try to get non-existent secret | ||
| run "$FNOX_BIN" get INVALID_ITEM | ||
| assert_failure | ||
| assert_output --partial "cli_failed" | ||
| run "$FNOX_BIN" get TEST_SLASH_FIELD | ||
| assert_success | ||
| assert_output "slash-field-value-${BATS_TEST_NUMBER:-0}" | ||
|
|
||
| delete_test_bw_item "$item_id" | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Clean up the test item after test failures.
The test deletes the Bitwarden item only after run, assert_success, and assert_output pass. If any step fails, the generated item remains in the vault and can pollute later test runs. Register cleanup as soon as item_id is available through the test teardown or exit-trap path.
🤖 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 `@test/bitwarden.bats` around lines 218 - 237, Update the test setup after
item_id is assigned in "fnox get retrieves custom field containing a slash" to
register deletion through the test teardown or exit-trap mechanism, ensuring
delete_test_bw_item runs even when run, assert_success, or assert_output fails;
remove the success-only cleanup call.
Instruction counts
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.
|
### 🚀 Features - **(bitwarden)** support custom fields by [@jdx](https://github.com/jdx) in [#690](#690) ### 🐛 Bug Fixes - **(bitwarden)** support slashes in custom fields by [@jdx](https://github.com/jdx) in [#693](#693) - **(ci)** update Infisical project bootstrap by [@jdx](https://github.com/jdx) in [#675](#675) ### 📚 Documentation - **(sync)** clarify personal age provider setup by [@jdx](https://github.com/jdx) in [#692](#692) ### 🔍 Other Changes - run perf jobs on bamboo by [@jdx](https://github.com/jdx) in [#672](#672) - build perf binaries on bamboo by [@jdx](https://github.com/jdx) in [#674](#674) ### 📦️ Dependency Updates - update jdx/mise-action action to v4.2.3 by [@renovate[bot]](https://github.com/renovate[bot]) in [#677](#677) - update rust crate keepass to v0.13.18 by [@renovate[bot]](https://github.com/renovate[bot]) in [#678](#678) - update rust crate schemars to v1.2.2 by [@renovate[bot]](https://github.com/renovate[bot]) in [#679](#679) - update dependency github:jdx/tak to v0.0.5 by [@renovate[bot]](https://github.com/renovate[bot]) in [#676](#676) - update rust crate jsonwebtoken to v11 by [@renovate[bot]](https://github.com/renovate[bot]) in [#685](#685) - update zizmorcore/zizmor-action action to v0.6.1 by [@renovate[bot]](https://github.com/renovate[bot]) in [#680](#680) - update rust crate base64 to 0.23 by [@renovate[bot]](https://github.com/renovate[bot]) in [#683](#683) - update rust crate usage-lib to v4.1.0 by [@renovate[bot]](https://github.com/renovate[bot]) in [#684](#684) - update dependency @anthropic-ai/claude-code to v2.1.220 by [@renovate[bot]](https://github.com/renovate[bot]) in [#681](#681) - update rust crate usage-lib to v5 by [@renovate[bot]](https://github.com/renovate[bot]) in [#687](#687) - update rust crate rmcp to v3 by [@renovate[bot]](https://github.com/renovate[bot]) in [#688](#688)
Summary
bwandrbwRoot cause
Bitwarden references were split on every
/and rejected unless they contained at most two segments. A reference such asitem/A/Btherefore failed before either Bitwarden backend ran, even though both backends can resolve a custom field namedA/B.The parser now treats only the first slash as the item/field delimiter and preserves the remainder as the exact custom field name.
Fixes the follow-up in discussion #689.
Validation
cargo test -p fnox-core providers::bitwardenmise run buildmise run test:cargomise run lintmise run test:bats -- test/bitwarden.bats(vault-backed tests skipped withoutBW_SESSION)The full Bats suite was also run; unrelated environment-dependent tests failed in keychain, GitHub lease, signal, and file-secret coverage.
AI-assisted — Tool: Codex; model: unavailable/GPT-5; version: unavailable.
Note
Low Risk
Localized parser change in the Bitwarden provider with regression tests; no auth or broader secret-handling changes.
Overview
Bitwarden reference parsing no longer rejects values with more than one
/or splits on every slash. Only the first/separates item from field; everything after it is the field name (e.g.Database/API/Key→ itemDatabase, fieldAPI/Key). The previousInvalid secret reference formatpath for extra segments is removed.get_secretuses a newparse_referencehelper (split_oncewith default fieldpasswordwhen no slash).bwcustom-field JSON lookup andrbw--fieldargs now receive the full slash-containing name.Docs note that custom field names may contain
/. Tests add unit coverage for parsing and slash fields, vault-backed Bats foritem/API/Key, and drop the Bats case that expected failure oninvalid/format/with/too/many/slashes.Reviewed by Cursor Bugbot for commit 0904efd. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit
/.