feat: migrate KeePass additional URLs / AndroidApp as Bitwarden login URIs#29
Conversation
…t custom fields
KeePass(XC) additional URLs (KP2A_URL, KP2A_URL_N) and AndroidApp package ids
were copied verbatim into Bitwarden custom fields, where they were inert. They
now fold into login.uris with per-URI match modes reproducing KeePassXC:
plain -> base domain, double-quoted -> exact, trailing-path wildcard ->
starts-with, host/interior wildcard -> regex (best-effort whole-URL pattern,
warned), AndroidApp -> androidapp://. Non-web schemes (keepassxc/cmd/kdbx/file)
and unresolved {REF:...} URLs are dropped.
Two orthogonal knobs: --uri-match / KP2BW_URI_MATCH (plain-tier match; default
'domain' = faithful KeePassXC host-based matching, 'default' defers to the
account default), and --interpret-uri-syntax / KP2BW_INTERPRET_URI_SYNTAX
(quote/wildcard interpretation; default on). URI order is stabilised by
attribute suffix so re-run content-diffs stay quiet.
New uri_mapping module (pure resolver + classifier) with fixtures covering the
match table, drops, dedup, literal mode, and the match-name override.
Covers the new-import path. Existing-item auto-migrate + a Bitwarden-only
--migrate-uris pass are the next slice.
|
Warning Review limit reached
More reviews will be available in 2 hours, 16 minutes, and 29 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAhoy, matey! Here be a proper treasure map o' this here PR: A brand-new Sequence DiagramsequenceDiagram
actor User
participant CLI as kp2bw CLI
participant Converter as Converter
participant UriMapping as uri_mapping
participant BwServe as BitwardenServeClient
rect rgba(184, 134, 11, 0.5)
Note over CLI,UriMapping: Normal import flow (with --uri-match, --interpret-uri-syntax)
User->>CLI: kp2bw --uri-match domain --interpret-uri-syntax
CLI->>Converter: Converter(uri_match=domain, interpret_uri_syntax=True)
Converter->>UriMapping: build_login_uris(primary_url, additional_urls, android_packages)
UriMapping-->>Converter: list[BwUri] with match modes
Converter-->>CLI: BwItem with login.uris
CLI->>BwServe: create/update item
BwServe-->>User: Aye, items synced!
end
rect rgba(34, 139, 34, 0.5)
Note over CLI,BwServe: --migrate-uris one-shot (Bitwarden-only)
User->>CLI: kp2bw --migrate-uris
CLI->>BwServe: migrate_url_fields_to_uris(plain_match, interpret_syntax)
BwServe->>UriMapping: remap_item_fields_to_uris(fields, uris)
UriMapping-->>BwServe: (new_fields, new_uris, changed)
alt Legacy fields present
BwServe->>BwServe: update_item(PUT)
end
BwServe-->>CLI: MigrateResult(scanned, migrated)
CLI-->>User: Shiver me timbers! All upgrades done!
end
rect rgba(220, 20, 60, 0.5)
Note over CLI,UriMapping: --report-uris collision detection
User->>CLI: kp2bw --report-uris bitwarden
CLI->>BwServe: list_items()
BwServe-->>CLI: items with login.uris
CLI->>UriMapping: collision_groups(uris)
UriMapping-->>CLI: dict[registrable_domain, list[str]]
CLI-->>User: Collision report (ready fer spreadin' the word)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 6 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (6 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
📦 Test this PR (archived)
This PR has been merged. You can still test the final state: uvx -p 3.14 --from 'git+https://github.com/kjanat/kp2bw@bfded651cae5fabffc8344e993f0ae044ce23400' kp2bw --help📋 PR Details
Merge commit: bfded65 📋 Example usage# Show version
uvx -p 3.14 --from 'git+https://github.com/kjanat/kp2bw@bfded651cae5fabffc8344e993f0ae044ce23400' kp2bw --version
# Migrate KeePass DB (interactive password prompts)
uvx -p 3.14 --from 'git+https://github.com/kjanat/kp2bw@bfded651cae5fabffc8344e993f0ae044ce23400' kp2bw vault.kdbx
# Non-interactive with env vars
KP2BW_KEEPASS_PASSWORD=kppass KP2BW_BITWARDEN_PASSWORD=bwpass uvx -p 3.14 --from 'git+https://github.com/kjanat/kp2bw@bfded651cae5fabffc8344e993f0ae044ce23400' kp2bw vault.kdbx -y🤖 Archived on merge |
…porting A Bitwarden-only one-shot pass (no KeePass) that re-folds legacy KP2A_URL*/AndroidApp custom fields into login URIs on existing items, for users who imported before URL folding and don't want to re-run a full migration. (Re-running a normal migration already upgrades them via the update path; this is the no-re-import alternative.) - uri_mapping.remap_item_fields_to_uris: pure transform — drop the URL/app fields, append derived URIs to the item's existing ones (de-duplicated). - bw_serve.migrate_url_fields_to_uris: list in-scope login items, apply the remap, PUT only the changed ones; skips non-login items. Idempotent. - cli --migrate-uris (env KP2BW_MIGRATE_URIS): Bitwarden-only dispatch like --strip-ids; honours --uri-match / --interpret-uri-syntax and -o/-c. - Tests: remap pure cases + a client-double for the bw_serve pass. Docs updated.
Primary URLs now migrate with match=0 (base domain) instead of null, matching
the new configurable URI-match default ('domain', faithful to KeePassXC). The
e2e golden snapshots predated the change; regenerated to the new shape. Behavior
is restorable per-run with --uri-match default.
… URL/AndroidApp keys Flip the plain-tier match default from base-domain (0) to null (defer to the Bitwarden account default). Evidence: Bitwarden's own export format writes match:null, and a real 1847-item migrated vault is 100% null -- defaulting to 0 would split-brain newly-folded URIs against the existing ones. --uri-match domain still forces base-domain (KeePassXC-faithful) for those who want it. - uri_mapping: plain_match defaults to None; widen AndroidApp to also catch the no-underscore AndroidApp1 variant; fold the plain URL/URL_n convention beside KP2A_URL*. Free-text URL labels (API Url, Alt. URL, Website, ...) stay as custom fields, since folding those would wrongly autofill metadata endpoints. - cli/convert: --uri-match defaults to 'default' (null). - Restore the e2e goldens to null, committing over the earlier 0 patch (53d025d): the null default reproduces pre-change behaviour, so the goldens equal master and the e2e stays green. The URL folding is covered by uri_mapping/migrate_uris unit fixtures. Widening the e2e *fixture* to exercise folding end-to-end is deferred: it needs a golden regen against live Vaultwarden, which this dev box can't run (Docker port-forward and bw-serve teardown are both broken here).
…eport) Print which logins all surface together under Bitwarden's base-domain matching, so users with many subdomains can see what to switch to Host match. Read-only -- it lists, changes nothing. - uri_mapping: uri_host, registrable_domain (curated two-level public-suffix heuristic so e.g. 10bis.co.il stays whole), and collision_groups (group hosts by registrable domain, keep only multi-host groups). - convert.collect_keepass_uris: gather entry.url + additional-URL fields from a KeePass db without running a migration. - cli --report-uris (env KP2BW_REPORT_URIS): 'keepass' reads the db (no bw CLI needed), 'bitwarden' reads the live vault honouring -o/-c. Prints a header via the console and plain group lines for clean copy/paste. - Tests for the helpers; docs in README/CHANGELOG/.env.example.
KP2BW_KEEPASS_FILE="" (or any empty-string export) shadowed the .env value -- load_dotenv(override=False) counts an empty export as "set" -- yielding a baffling "KeePass database path is required" when .env clearly had it. _load_dotenv now fills any key that is unset OR empty from the file, while a real non-empty shell variable still wins (documented CLI > env > default precedence preserved for meaningful values). Regression test in cli_env_test.
…n/hang) On POSIX `bw` is commonly a node launcher that spawns a worker; teardown signalled only the tracked PID, leaving the worker orphaned -- it kept the port and, when kp2bw's stdout was a pipe, held it open so the parent pipeline never reached EOF (a multi-minute "still running" hang) and leaked bw serve across runs. bw serve now starts in its own session (start_new_session=True) and teardown signals the whole process group -- but ONLY when the process leads its own group (getpgid == pid); otherwise a single-PID kill, so it never signals kp2bw's own process group (which would kill the caller -- caught because it hung the test suite mid-fix). Windows teardown (taskkill /T + port reap) is unchanged. - bw_serve: start_new_session on the Popen; terminate_serve group-signal with the own-group-leader guard + single-PID fallback. - tests: new bw_serve_teardown_test drives the group-kill path; the existing command test covers the single-PID fallback. Verified live on the affected machine -- 6.3s clean close, zero orphaned bw serve.
…> login URIs) Add a "Multi URL" fixture entry and regenerate the golden vaults so the e2e covers the full per-URI mapping against real Vaultwarden 1.36.0: - plain additional URLs (KP2A_URL / KP2A_URL_n / URL_1) -> match unset - double-quoted -> exact (3) - trailing-path wildcard -> starts-with (2) - host wildcard -> regex (4), stored/round-tripped faithfully through Bitwarden - AndroidApp (bare) and the no-underscore AndroidApp1 -> androidapp:// - keepassxc:// dropped; the KP2A/URL/AndroidApp fields no longer survive as custom fields This is the end-to-end validation deferred earlier (it was blocked by the bw serve teardown hang, now fixed). The e2e's idempotency pass (snapshot 1 == snapshot 2) covers determinism, so the regenerated goldens are stable for CI's compare leg.
coderabbitai
Bot
added
bug
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/kp2bw/convert.py (2)
761-767:⚠️ Potential issue | 🟠 Major | ⚡ Quick winREF merges still lose the shiny new URI aliases, arr.
When a REF entry collapses into its referent, this branch appends only
kp_entry.url. AnyKP2A_URL*,URL_*, orAndroidApp*fields on the REF entry never reach_add_bw_entry_to_entries_dict(), so they vanish instead of being merged into the shared login. Reuse the same URL/app extraction plusbuild_login_uris()here and dedupe against the referent’s existing URIs.🤖 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/kp2bw/convert.py` around lines 761 - 767, The current code only appends kp_entry.url when merging a REF entry into its referent, which causes other URL fields like KP2A_URL*, URL_*, and AndroidApp* to be lost. Instead of directly appending just the single BwUri, extract all URL and app fields from the REF entry (using the same extraction logic as _add_bw_entry_to_entries_dict()), build the complete URI list using build_login_uris(), and then merge only the new URIs that don't already exist in ref_item["login"]["uris"] to avoid duplicates.
919-921:⚠️ Potential issue | 🟠 Major | ⚡ Quick winURI match-mode changes are invisible to the update diff, matey.
_login_differs()compares only the URI strings. A re-run that changesmatchfromnullto0(or any other mode change with the same URI text) is treated as unchanged, so the in-place upgrade path never fires. Compare(uri, match)pairs instead. Based on PR objectives, a normal re-run is meant to upgrade existing imports in place.💡 Suggested change
- ex_uris = [u.get("uri", "") for u in (existing.get("uris") or [])] - de_uris = [u.get("uri", "") for u in (desired.get("uris") or [])] + ex_uris = [ + (u.get("uri", ""), u.get("match")) + for u in (existing.get("uris") or []) + ] + de_uris = [ + (u.get("uri", ""), u.get("match")) + for u in (desired.get("uris") or []) + ] return ex_uris != de_uris🤖 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/kp2bw/convert.py` around lines 919 - 921, The `_login_differs()` function currently compares only the URI strings and ignores the match mode, so changes to the match mode are treated as unchanged. Modify the list comprehensions for both ex_uris and de_uris to include both the "uri" and "match" fields from each object (e.g., by creating tuples containing both values or by comparing the full objects), so that differences in either the URI string or the match mode are properly detected by the inequality comparison.
🤖 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 `@CHANGELOG.md`:
- Around line 13-19: The CHANGELOG.md entry for the bw serve teardown fix
currently implies that only POSIX systems (Linux/macOS) received changes by
stating at the end that "Windows teardown (taskkill /T + port reap) is
unchanged." However, the Windows teardown behavior was also modified as part of
this fix. Update the changelog entry to clearly call out what the Windows
teardown change was, rather than presenting it as unchanged. This ensures the
entry accurately reflects that both POSIX and Windows teardown behavior were
improved in this release.
In `@src/kp2bw/cli.py`:
- Around line 719-723: The validation check for report_uris does not normalize
the value before comparing it against the expected strings "keepass" and
"bitwarden", so environment variables with different casing (like "KEEPASS") or
whitespace (like " bitwarden ") will fail validation even though they represent
valid intent. After the _with_env call assigns report_uris, normalize the value
by converting it to lowercase and stripping leading/trailing whitespace before
performing the validation check on line 720.
In `@src/kp2bw/convert.py`:
- Around line 126-150: The collect_keepass_uris function currently returns raw
URL strings without applying the same transformations that build_login_uris
performs during migration, causing the --report-uris report to preview different
URIs than what actually gets written. Add uri_match and interpret_uri_syntax
parameters to collect_keepass_uris, then instead of appending raw entry.url and
custom field values directly to the uris list, pass them through
build_login_uris with these settings to ensure the reported URIs match exactly
what the migration will produce.
In `@src/kp2bw/uri_mapping.py`:
- Around line 273-300: Remove the raw URL content (`s!r`) from all logger.debug
and logger.warning calls in this block. The function performs validation checks
(for non-web schemes, unresolved references, illegal characters, invalid
quoted-exact patterns, and invalid wildcards) and logs rejection reasons - keep
only the descriptive reason messages in the log statements without interpolating
the sensitive vault URL data. Update each of the five logger.debug calls that
currently include the URL and the logger.warning call about wildcard migration
to omit the `{s!r}` variable while keeping the descriptive context.
In `@tests/e2e_vaultwarden_test.py`:
- Around line 342-360: The Multi URL fixture entry setup currently relies only
on golden snapshot comparison for validation, meaning regressions could occur
when KP2BW_SNAPSHOT_GOLDEN is false. Add explicit named assertions that verify
both the successful migration of login.uris (with proper match modes applied)
and the removal of KP2A_URL*, KP2A_URL_*, AndroidApp*, and AndroidApp* custom
fields from the multi entry. These assertions should run unconditionally in
_assert_comprehensive_seed() or as a separate check, not only when comparing
against golden snapshots, to ensure URI folding, match modes, and field removal
are properly validated in all test paths.
In `@tests/migrate_uris_test.py`:
- Around line 50-54: The assertion in the update_item method only checks for
fields starting with "KP2A_URL" but does not catch "AndroidApp*" legacy fields.
Extend the condition that detects legacy fields to also check for field names
starting with "AndroidApp" in addition to "KP2A_URL", so that any regression
involving either type of legacy field will be caught and the assertion will fail
appropriately.
---
Outside diff comments:
In `@src/kp2bw/convert.py`:
- Around line 761-767: The current code only appends kp_entry.url when merging a
REF entry into its referent, which causes other URL fields like KP2A_URL*,
URL_*, and AndroidApp* to be lost. Instead of directly appending just the single
BwUri, extract all URL and app fields from the REF entry (using the same
extraction logic as _add_bw_entry_to_entries_dict()), build the complete URI
list using build_login_uris(), and then merge only the new URIs that don't
already exist in ref_item["login"]["uris"] to avoid duplicates.
- Around line 919-921: The `_login_differs()` function currently compares only
the URI strings and ignores the match mode, so changes to the match mode are
treated as unchanged. Modify the list comprehensions for both ex_uris and
de_uris to include both the "uri" and "match" fields from each object (e.g., by
creating tuples containing both values or by comparing the full objects), so
that differences in either the URI string or the match mode are properly
detected by the inequality comparison.
🪄 Autofix (Beta)
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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 973aedca-2d52-4c88-a5db-8020deadb1c1
📒 Files selected for processing (15)
.env.exampleCHANGELOG.mdREADME.mdsrc/kp2bw/bw_serve.pysrc/kp2bw/cli.pysrc/kp2bw/convert.pysrc/kp2bw/uri_mapping.pytests/__snapshots__/vault_after_update.jsontests/__snapshots__/vault_initial.jsontests/bw_serve_teardown_test.pytests/cli_env_test.pytests/e2e_vaultwarden_test.pytests/migrate_uris_test.pytests/test_script_adapters.pytests/uri_mapping_test.py
📜 Review details
⏰ Context from checks skipped due to timeout of 18000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: e2e (2026.5.0, false)
- GitHub Check: e2e (latest, true)
🧰 Additional context used
📓 Path-based instructions (18)
**/*.{toml,json,jsonc,py}
📄 CodeRabbit inference engine (AGENTS.md)
Use dprint for formatting (config .dprint.jsonc) — run
dprint fmt; nevertomli/taplooruv's own output for TOML
Files:
tests/bw_serve_teardown_test.pytests/__snapshots__/vault_after_update.jsontests/migrate_uris_test.pytests/__snapshots__/vault_initial.jsontests/cli_env_test.pytests/e2e_vaultwarden_test.pytests/test_script_adapters.pysrc/kp2bw/uri_mapping.pysrc/kp2bw/bw_serve.pysrc/kp2bw/convert.pysrc/kp2bw/cli.pytests/uri_mapping_test.py
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Ruff runs in preview mode, target
py314
Files:
tests/bw_serve_teardown_test.pytests/migrate_uris_test.pytests/cli_env_test.pytests/e2e_vaultwarden_test.pytests/test_script_adapters.pysrc/kp2bw/uri_mapping.pysrc/kp2bw/bw_serve.pysrc/kp2bw/convert.pysrc/kp2bw/cli.pytests/uri_mapping_test.py
tests/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Tests are executable scripts (
main()+ assertions), not pytest collection
tests/**/*.py: Test scripts must usemain()function entry point withAssertionErrorfor checks; do not adopt pytest collection framework
E2E command wrappers must redact sensitive arguments and output before logging to prevent secret exposure
E2E tests must validate idempotency: second run of the same operation must not duplicate items or state
Fixture changes require coordinated updates to docker-compose setup, Dockerfile definitions, and test assertions
Do not convert test scripts to framework-only style without preserving direct script execution capability viamain()and command-line execution
Do not print raw secrets or session data in helper command output; redact before logging
Do not edit fixture content ad hoc without coordinating assertion updates and docker-compose/Dockerfile modifications
Do not treat e2e rerun duplicates as acceptable behavior; idempotency violations must be treated as test failures
Files:
tests/bw_serve_teardown_test.pytests/migrate_uris_test.pytests/cli_env_test.pytests/e2e_vaultwarden_test.pytests/test_script_adapters.pytests/uri_mapping_test.py
**
⚙️ CodeRabbit configuration file
**: # PROJECT KNOWLEDGE BASEGenerated: 2026-02-26 Commit:
59ba3c2Branch:masterOVERVIEW
KeePass to Bitwarden migration CLI. Python runtime package + workspace stubs package. Core flow migrates
entries/folders/attachments/passkeys withbw serveas primary transport.STRUCTURE
kp2bw/ ├── src/kp2bw/ # Runtime package (CLI + conversion + bw transport + types) │ └── AGENTS.md ├── specs/ # OpenAPI spec for Bitwarden vault management API │ └── vault-management-api.json ├── tests/ # Script-style smoke + docker e2e + fixture contract │ └── AGENTS.md ├── packages/pykeepass-stubs/ # Separate stubs release stream │ └── AGENTS.md ├── scripts/ # Release/version checks + codegen for github-script │ └── AGENTS.md └── .github/workflows/ # CI orchestration (release + integration + codegen drift) └── AGENTS.mdWHERE TO LOOK
Task Location Notes CLI flags, prompts, envs src/kp2bw/cli.pyEntrypoint kp2bw.cli:mainandpython -m kp2bwhandoffConversion orchestration src/kp2bw/convert.py3-phase top-level flow; item+attachment migration logic Bitwarden HTTP transport src/kp2bw/bw_serve.pybw servelifecycle, dedup index, batch create, attachment uploadWorkflow policy details .github/workflows/AGENTS.mdTrigger matrix, cross-workflow dependencies, output contracts Release version gating scripts/version-check-shared.mjsNormalizes release/tag prefixes; drives workflow gates Main package publishing .github/workflows/publish.yml...
Files:
tests/bw_serve_teardown_test.pytests/__snapshots__/vault_after_update.jsontests/migrate_uris_test.pytests/__snapshots__/vault_initial.jsontests/cli_env_test.pytests/e2e_vaultwarden_test.pytests/test_script_adapters.pysrc/kp2bw/uri_mapping.pyCHANGELOG.mdsrc/kp2bw/bw_serve.pysrc/kp2bw/convert.pysrc/kp2bw/cli.pyREADME.mdtests/uri_mapping_test.py
tests/**
⚙️ CodeRabbit configuration file
tests/**: # TESTS KNOWLEDGE BASEOVERVIEW
Tests are script-executable smoke/e2e checks, with dockerized Vaultwarden integration and tracked fixtures.
WHERE TO LOOK
Task Location Notes bw command resolution tests/bw_serve_command_test.pyCross-platform argv wrapping + process teardown logic Windows bw.cmd live smoke tests/windows_bw_cmd_smoke.pyGated by KP2BW_RUN_WIN_CMD_SMOKE=1; Windows CI onlyPackage smoke validation tests/smoke_test.pyVerifies built artifact behavior, metadata, entry points Stubs package validation tests/stubs_smoke_test.pyVerifies partialmarker + type-check consumptionFull migration e2e tests/e2e_vaultwarden_test.pyRich seed + idempotency + golden snapshots Snapshot normalization tests/_snapshot.pyScrubs volatile fields, hashes attachments, golden diff Golden snapshots tests/__snapshots__/*.jsonCommitted expected vault state (pinned bw owns them) Integration infra tests/docker-compose.ymlVaultwarden + test container orchestration Vaultwarden image seeding tests/Dockerfile.vaultwardenPinned image + DB/key fixtures Test runner image tests/Dockerfile.testuv + lockfile-pinned bw CLI + test deps Reusable bw setup .github/actions/setup-bwInstalls a chosen @bitwarden/cli(matrix-able)Fixture retention rules tests/fixtures/.gitignoreAllowlist keeps required DB/certs tracked CONVENTIONS
- No pytest collection model; scripts use
main()andAssertionError...
Files:
tests/bw_serve_teardown_test.pytests/__snapshots__/vault_after_update.jsontests/migrate_uris_test.pytests/__snapshots__/vault_initial.jsontests/cli_env_test.pytests/e2e_vaultwarden_test.pytests/test_script_adapters.pytests/uri_mapping_test.py
tests/__snapshots__/*.json
📄 CodeRabbit inference engine (tests/AGENTS.md)
tests/__snapshots__/*.json: Golden snapshot files must contain committed expected vault state as pinned by bw CLI version; manage alongside CLI version updates
Golden snapshots are gated byKP2BW_SNAPSHOT_GOLDEN=1environment variable; pinned CLI matrix leg owns golden comparison, latest canary leg runs behavioral checks only
Golden snapshots must be regenerated and reviewed when rootpackage.json,bun.lock, ortests/Dockerfile.vaultwardenpinned image changes viaKP2BW_UPDATE_SNAPSHOTS=1flag
Files:
tests/__snapshots__/vault_after_update.jsontests/__snapshots__/vault_initial.json
tests/e2e_vaultwarden_test.py
📄 CodeRabbit inference engine (tests/AGENTS.md)
Full migration e2e test must include rich seed data, idempotency validation, and golden snapshot comparison in
e2e_vaultwarden_test.py
Files:
tests/e2e_vaultwarden_test.py
tests/test_script_adapters.py
📄 CodeRabbit inference engine (AGENTS.md)
tests/test_script_adapters.py:tests/test_script_adapters.pyprovides pytest wrappers sopytestcollects tests; script files remain the source-of-truth
Heavy adapters are opt-in: setKP2BW_RUN_PACKAGING_TESTS=1and/orKP2BW_RUN_E2E_TESTS=1
Files:
tests/test_script_adapters.py
{src/**/*.py,packages/**/*.py}
📄 CodeRabbit inference engine (AGENTS.md)
{src/**/*.py,packages/**/*.py}: Intra-package imports are relative (from .module import X)
Never use root logger calls (logging.info(...)etc.); use module logger
Files:
src/kp2bw/uri_mapping.pysrc/kp2bw/bw_serve.pysrc/kp2bw/convert.pysrc/kp2bw/cli.py
src/kp2bw/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
src/kp2bw/**/*.py: Never use bareException; use project exceptions (BitwardenClientError,ConversionError)
Do not rewrite valid Python 3.14 comma-formexcept X, Y:syntax to tuple form
src/kp2bw/**/*.py: Use module logger withlogger = logging.getLogger(__name__), never root logger calls
Keep sensitive values out of logs - no rawbwcommands/output, session/password values, or decrypted vault content
Raise project exceptions (BitwardenClientError,ConversionError) instead of bareException
Keep relative imports for local modules (e.g.,from .exceptions import ConversionError)
Python 3.14 comma-form multi-except is valid; do not rewrite it to tuple form just for style
Files:
src/kp2bw/uri_mapping.pysrc/kp2bw/bw_serve.pysrc/kp2bw/convert.pysrc/kp2bw/cli.py
src/kp2bw/**
⚙️ CodeRabbit configuration file
src/kp2bw/**: # SOURCE PACKAGE KNOWLEDGE BASEOVERVIEW
Runtime package for CLI argument handling, KeePass parsing, Bitwarden conversion, and transport.
STRUCTURE
src/kp2bw/ ├── cli.py # CLI parsing, prompts, env handling, run mode selection ├── convert.py # Conversion orchestrator and entry transformation pipeline ├── bw_serve.py # bw serve process lifecycle + HTTP CRUD + attachment upload ├── bw_types.py # Hand-written TypedDict types (supplements generated types) ├── _bw_api_types.py # Auto-generated from specs/vault-management-api.json (DO NOT EDIT) ├── _console.py # Shared Rich Console instance (stderr) ├── exceptions.py # BitwardenClientError, ConversionError ├── __main__.py # python -m kp2bw handoff └── __init__.py # __version__ from installed metadataWHERE TO LOOK
Task Location Notes Add/adjust CLI flags src/kp2bw/cli.pymain()and argument parser are hereChange import behavior src/kp2bw/convert.pyTop-level convert()flow and migration phasesTune dedup/idempotency src/kp2bw/bw_serve.pyExisting item index + batch create behavior Attachment behavior src/kp2bw/bw_serve.pyAsync upload path and multipart logic Map KeePass fields src/kp2bw/convert.pyEntry/custom field/TOTP/passkey mapping API type definitions src/kp2bw/bw_types.pyHand-written TypedDicts supplementing codegen Regenerate API types scripts/generate-bw-types.shRun after editing specs/vault-management-api.jsonError contract src/kp2bw/exceptions.pyKeep custom exception taxonomy ...
Files:
src/kp2bw/uri_mapping.pysrc/kp2bw/bw_serve.pysrc/kp2bw/convert.pysrc/kp2bw/cli.py
**/CHANGELOG.md
📄 CodeRabbit inference engine (Custom checks)
**/CHANGELOG.md: If any source code files (excluding tests, docs, CI, markdown, or comments-only changes) are modified, CHANGELOG.md MUST also be modified in the same PR with an entry describing the changes (Added, Changed, Fixed, Removed).
If a version bump is detected in source code modifications, CHANGELOG.md MUST contain a new section header matching the exact new version number in the format: '## [X.Y.Z] - YYYY-MM-DD' (with date TBD acceptable before release).
If NO version bump is detected in source code modifications, the changes in the PR MUST be added under the existing '## [Unreleased]' section in CHANGELOG.md.
Files:
CHANGELOG.md
src/kp2bw/bw_serve.py
📄 CodeRabbit inference engine (AGENTS.md)
src/kp2bw/bw_serve.py:bw serveis localhost-only and password is passed via env var, not CLI arg
Never log rawbwcommand strings or raw command output
Bitwarden HTTP transport (bw servelifecycle, dedup index, batch create, attachment upload) is insrc/kp2bw/bw_serve.py
src/kp2bw/bw_serve.py: Do not reintroduce subprocess-per-op transport for new work
bw serveteardown on Windows is port-based to reliably kill orphanednodegrandchildren: useterminate_serve(port=)/close()withparse_listening_pidsand_kill_port_listeners
Include response body viaformat_http_errorinbw serveHTTP error logging instead of opaque error messages
Files:
src/kp2bw/bw_serve.py
src/kp2bw/{convert,bw_serve}.py
📄 CodeRabbit inference engine (src/kp2bw/AGENTS.md)
src/kp2bw/{convert,bw_serve}.py: Do not break idempotency: a re-run with no KeePass changes must issue noPUTand upload no attachment, controlled via_content_differs/ upload-if-missing gate
Reconcile attachments by content hash, not filename: upload if item lacks file, re-upload if bytes changed; delete stale copy only after replacement uploads (upload-then-delete), skip deletion on upload failure
Files:
src/kp2bw/bw_serve.pysrc/kp2bw/convert.py
src/kp2bw/convert.py
📄 CodeRabbit inference engine (AGENTS.md)
Conversion orchestration (3-phase flow, item+attachment migration logic) is in
src/kp2bw/convert.py
src/kp2bw/convert.py: Dedup keys on stable identity using KeePass entry UUID inKP2BW_IDcustom field, not on(folder, title)
Fold KeePass metadata (tags and expiry) into a YAMLKP2BW_METAtext field using PyYAMLsafe_dump(allow_unicode=False)to escape control chars and line-break code points
For existing-item sync, match by UUID first, then fall back to legacy(folder, name)matching for unstamped login items, then create new; preserve id/favorite/folder/org and union collectionIds on update
Files:
src/kp2bw/convert.py
src/kp2bw/{convert,cli}.py
📄 CodeRabbit inference engine (src/kp2bw/AGENTS.md)
Oversize custom fields (value over
MAX_BW_ITEM_LENGTH, 10k) are offloaded to<key>.txtattachment instead of inline field, unless--include-oversize-secretsis set
Files:
src/kp2bw/convert.pysrc/kp2bw/cli.py
src/kp2bw/cli.py
📄 CodeRabbit inference engine (AGENTS.md)
CLI flags, prompts, envs are defined in
src/kp2bw/cli.pyAlways write a full DEBUG log to a per-user file via
_configure_loggingincli.py, independent of console verbosity; useKP2BW_LOG_FILE/KP2BW_LOG_DIRoverrides or%LOCALAPPDATA%/kp2bw/logs
Files:
src/kp2bw/cli.py
src/kp2bw/{cli,__main__}.py
📄 CodeRabbit inference engine (src/kp2bw/AGENTS.md)
Maintain behavior parity for
kp2bw.cli:mainandpython -m kp2bw
Files:
src/kp2bw/cli.py
🧠 Learnings (1)
📚 Learning: 2026-02-23T21:45:13.630Z
Learnt from: kjanat
Repo: kjanat/kp2bw PR: 2
File: src/kp2bw/bw_serve.py:306-310
Timestamp: 2026-02-23T21:45:13.630Z
Learning: In src/kp2bw/bw_serve.py, for Bitwarden CLI 'bw serve' Vault Management API endpoints (e.g., /list/object/folders, /list/object/items, /list/object/org-collections), the HTTP response has the shape {"success": true, "data": {"object": "list", "data": [...]}}. Since the outer 'data' is a wrapper object, extract the inner list with payload.get("data", {}).get("data", []) after calling _request(). Example: payload = _request(...); items = payload.get("data", {}).get("data", []) to obtain the actual array.
Applied to files:
src/kp2bw/bw_serve.py
🪛 ast-grep (0.43.0)
tests/bw_serve_teardown_test.py
[error] 26-31: Use of unsanitized data to create processes
Context: subprocess.Popen(
[sys.executable, "-c", "import time; time.sleep(30)"],
stdin=subprocess.DEVNULL,
stderr=subprocess.PIPE,
start_new_session=True,
)
Note: [CWE-78].
(os-system-unsanitized-data)
[error] 26-31: Command coming from incoming request
Context: subprocess.Popen(
[sys.executable, "-c", "import time; time.sleep(30)"],
stdin=subprocess.DEVNULL,
stderr=subprocess.PIPE,
start_new_session=True,
)
Note: [CWE-20].
(subprocess-from-request)
src/kp2bw/bw_serve.py
[error] 350-355: Command coming from incoming request
Context: subprocess.run(
["taskkill", "/F", "/T", "/PID", str(process.pid)],
check=False,
capture_output=True,
stdin=subprocess.DEVNULL,
)
Note: [CWE-20].
(subprocess-from-request)
[error] 350-355: Use of unsanitized data to create processes
Context: subprocess.run(
["taskkill", "/F", "/T", "/PID", str(process.pid)],
check=False,
capture_output=True,
stdin=subprocess.DEVNULL,
)
Note: [CWE-78].
(os-system-unsanitized-data)
[error] 726-738: Command coming from incoming request
Context: subprocess.Popen(
cmd,
stdin=subprocess.DEVNULL,
stderr=subprocess.PIPE,
cwd=self._bw_cwd,
env=env,
# POSIX: run bw serve in its own session/process group so teardown
# can kill the launcher and its node worker together (see
# terminate_serve). Without this an orphaned worker keeps the port
# and, when our stdout is a pipe, holds it open -> the parent
# pipeline hangs. Ignored on Windows (taskkill /T handles the tree).
start_new_session=True,
)
Note: [CWE-20].
(subprocess-from-request)
[error] 726-738: Use of unsanitized data to create processes
Context: subprocess.Popen(
cmd,
stdin=subprocess.DEVNULL,
stderr=subprocess.PIPE,
cwd=self._bw_cwd,
env=env,
# POSIX: run bw serve in its own session/process group so teardown
# can kill the launcher and its node worker together (see
# terminate_serve). Without this an orphaned worker keeps the port
# and, when our stdout is a pipe, holds it open -> the parent
# pipeline hangs. Ignored on Windows (taskkill /T handles the tree).
start_new_session=True,
)
Note: [CWE-78].
(os-system-unsanitized-data)
🪛 Checkov (3.2.530)
tests/__snapshots__/vault_after_update.json
[low] 199-200: Base64 High Entropy String
(CKV_SECRET_6)
tests/__snapshots__/vault_initial.json
[low] 177-178: Base64 High Entropy String
(CKV_SECRET_6)
🪛 LanguageTool
CHANGELOG.md
[uncategorized] ~15-~15: Use a comma before ‘so’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...'s stdout was a pipe, held the pipe open so the parent pipeline never reached EOF (...
(COMMA_COMPOUND_SENTENCE_2)
[uncategorized] ~33-~33: Do not use a comma before “so” when introducing a dependent clause.
Context: ...`). It changes nothing -- it just prints, so you can decide which entries to swit...
(NO_COMMA_BEFORE_SO)
[uncategorized] ~49-~49: Use a comma before “and” if it connects two independent clauses (unless they are closely connected and short).
Context: ... a normal re-run (the change is detected and the item updated in place); for users w...
(COMMA_COMPOUND_SENTENCE_2)
README.md
[uncategorized] ~135-~135: Use a comma before “and” if it connects two independent clauses (unless they are closely connected and short).
Context: ...normal migration (the change is detected and the items are updated in place), or — i...
(COMMA_COMPOUND_SENTENCE_2)
🔍 Remote MCP Context7
Based on the PR summary and AI-generated analysis provided, here is my review of the relevant context for this PR:
Code Integration & Pattern Validation Summary
URI Handling Implementation
The PR introduces a comprehensive URI migration system through src/kp2bw/uri_mapping.py with the following key features:
Match Mode Mapping:
- Maps KeePass/KeePassXC URL syntax to Bitwarden
login.urismatch modes:- Plain URLs → base domain matching (mode 0, or null for account defaults)
- Double-quoted URLs → exact match (mode 3)
- Trailing-path wildcards → starts-with matching (mode 2)
- Host/interior wildcards → regex matching (mode 4)
- AndroidApp package IDs →
androidapp://URIs
Non-web Scheme Handling: The implementation properly drops unsupported schemes (keepassxc://, cmd://, kdbx://, file://) and unresolved {REF:…} references, preventing invalid entries from being stored.
Process Teardown Improvements
The terminate_serve refactoring in src/kp2bw/bw_serve.py addresses cross-platform robustness:
POSIX Process Group Handling:
- Uses
start_new_session=Truewhen spawningbw servesubprocess to ensure launcher and worker share the same process group - Targets the full process group with
SIGTERM/SIGKILLrather than single-PID signalling - Falls back to single-PID when process group determination fails
- This pattern prevents orphaned background processes and pipe-hang issues
Windows Differentiation:
- Distinguishes between shell-spawned vs direct subprocess termination
- Uses
taskkill /T(tree termination) vs escalated terminate/kill for robustness
Environment Variable Handling
The .env file loading logic in src/kp2bw/cli.py implements proper variable precedence:
Precedence Chain: CLI args > shell env vars > .env file > defaults
- Empty environment variables no longer shadow
.enventries - Only missing or truly empty variables are filled from
.envfile - Non-empty shell environment variables maintain priority
This follows standard CLI/environment patterns and prevents accidental overrides.
Migration Path Integration
The PR provides two upgrade mechanisms for existing Bitwarden vaults:
Normal Re-run: Existing imports trigger updates via _content_differs when login.uris change, allowing incremental adoption by simply re-importing with new settings.
Dedicated --migrate-uris Pass: A Bitwarden-only migration (BitwardenServeClient.migrate_url_fields_to_uris) transforms legacy KP2A_URL*/AndroidApp custom fields into login URIs without requiring KeePass access, enabling in-place updates for users with lost/inaccessible source vaults.
Configuration Flexibility
Two independent settings provide fine-grained control:
--uri-match/KP2BW_URI_MATCH: Controls plain-tier match mode (defaults tonull/account settings;domainforces base-domain matching)--interpret-uri-syntax/KP2BW_INTERPRET_URI_SYNTAX: Enables/disables quote/wildcard interpretation (default enabled; disabled = literal import)
Collision Detection
The --report-uris feature provides read-only analysis showing which logins surface together under base-domain matching, helping users identify and resolve potential autofill collisions.
Test Coverage
Comprehensive testing includes:
uri_mapping_test.py: Match-mode parsing, attribute key detection, syntax interpretation, deduplication, collision groupingmigrate_uris_test.py: Legacy field → URI transformation with item scans/migration countsbw_serve_teardown_test.py: POSIX process-group termination regression testcli_env_test.py: Environment variable vs.envprecedence verification- End-to-end validation against real Vaultwarden with deterministic seed data
Quality Assurance
All code quality gates pass:
- Linting: ruff
- Type checking: pyright, mypy
- Formatting: dprint
- Test results: 16 passed / 4 skipped
🔇 Additional comments (10)
README.md (1)
88-159: LGTM!.env.example (1)
34-71: LGTM!src/kp2bw/bw_serve.py (1)
25-26: LGTM!Also applies to: 118-127, 348-371, 373-410, 733-739, 1026-1067
src/kp2bw/cli.py (1)
10-26: LGTM!Also applies to: 89-111, 247-314, 434-548, 716-718, 724-735, 752-835, 880-881
tests/uri_mapping_test.py (1)
1-261: LGTM!tests/migrate_uris_test.py (1)
1-49: LGTM!Also applies to: 57-83
tests/bw_serve_teardown_test.py (1)
1-55: LGTM!tests/cli_env_test.py (1)
107-150: LGTM!tests/test_script_adapters.py (1)
42-56: LGTM!src/kp2bw/uri_mapping.py (1)
333-335:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPrimary URLs dodge the drop filter, matey.
build_login_uris()validates additional URLs via_classify_additional_url(), but the mainprimary_urlis appended blindly. A primarykeepassxc://...,file://...,{REF:...}, or otherwise invalid value will still land inlogin.uris, even though this module documents those inputs as dropped. Feed the primary URL through the same validation path with syntax interpretation disabled.💡 Suggested change
- primary = primary_url.strip() - if primary: - _add(BwUri(uri=primary, match=plain_match)) + primary = primary_url.strip() + if primary: + _add( + _classify_additional_url( + primary, + plain_match=plain_match, + interpret_syntax=False, + ) + )> Likely an incorrect or invalid review comment.
- convert: REF merge & collect_keepass_uris now run entries through build_login_uris (primary + KP2A_URL*/URL_*/AndroidApp*), so report and resolved-REF items match migration output; dedup against existing. - uri_mapping: drop raw URL values from always-on debug/warn logs. - cli: normalise --report-uris via strip().lower(), keep raw for errors. - tests: widen migrate_uris legacy-field guard to URL*/AndroidApp*; add custom_properties to REF double; assert Multi URL fold in e2e seed.
There was a problem hiding this comment.
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 `@tests/e2e_vaultwarden_test.py`:
- Around line 689-702: The dict comprehension on line 689 that creates
multi_uris masks duplicate URIs since duplicate keys are silently collapsed. Add
an assertion before the dict creation to verify that the raw list from
_login(multi)["uris"] contains no duplicate URI values. You can do this by
extracting the list into a variable, then asserting that the number of unique
URI values equals the total number of URIs in the list. Keep the existing
dict-based assertion unchanged to verify the match values are correct.
In `@tests/migrate_uris_test.py`:
- Around line 53-54: The guard checking for legacy URL/app fields in the test is
too broad because startswith("URL") will match non-legacy field names like
"URLAlias" even when migration is correct. Replace the overly-broad prefix
checks in the condition (specifically the "URL" and "AndroidApp" parts within
the startswith tuple) with the exact same key predicates and rules used in the
production migration mapping code, so the test only fails on genuine legacy
fields that should have been migrated, not on unrelated fields that happen to
start with those strings.
🪄 Autofix (Beta)
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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: b4632d79-8c88-4e48-8c04-b82b01746949
📒 Files selected for processing (6)
src/kp2bw/cli.pysrc/kp2bw/convert.pysrc/kp2bw/uri_mapping.pytests/convert_ref_resolution_test.pytests/e2e_vaultwarden_test.pytests/migrate_uris_test.py
📜 Review details
⏰ Context from checks skipped due to timeout of 18000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: e2e (false, 2026.5.0)
- GitHub Check: e2e (true, latest)
- GitHub Check: autofix
- GitHub Check: cmd-shim (bun)
- GitHub Check: cmd-shim (npm)
- GitHub Check: Analyze (actions)
- GitHub Check: Analyze (python)
- GitHub Check: Analyze (javascript-typescript)
🧰 Additional context used
📓 Path-based instructions (14)
**/*.{toml,json,jsonc,py}
📄 CodeRabbit inference engine (AGENTS.md)
Use dprint for formatting (config .dprint.jsonc) — run
dprint fmt; nevertomli/taplooruv's own output for TOML
Files:
tests/migrate_uris_test.pytests/convert_ref_resolution_test.pytests/e2e_vaultwarden_test.pysrc/kp2bw/convert.pysrc/kp2bw/cli.pysrc/kp2bw/uri_mapping.py
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Ruff runs in preview mode, target
py314
Files:
tests/migrate_uris_test.pytests/convert_ref_resolution_test.pytests/e2e_vaultwarden_test.pysrc/kp2bw/convert.pysrc/kp2bw/cli.pysrc/kp2bw/uri_mapping.py
tests/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Tests are executable scripts (
main()+ assertions), not pytest collection
tests/**/*.py: Test scripts must usemain()function entry point withAssertionErrorfor checks; do not adopt pytest collection framework
E2E command wrappers must redact sensitive arguments and output before logging to prevent secret exposure
E2E tests must validate idempotency: second run of the same operation must not duplicate items or state
Fixture changes require coordinated updates to docker-compose setup, Dockerfile definitions, and test assertions
Do not convert test scripts to framework-only style without preserving direct script execution capability viamain()and command-line execution
Do not print raw secrets or session data in helper command output; redact before logging
Do not edit fixture content ad hoc without coordinating assertion updates and docker-compose/Dockerfile modifications
Do not treat e2e rerun duplicates as acceptable behavior; idempotency violations must be treated as test failures
Files:
tests/migrate_uris_test.pytests/convert_ref_resolution_test.pytests/e2e_vaultwarden_test.py
**
⚙️ CodeRabbit configuration file
**: # PROJECT KNOWLEDGE BASEGenerated: 2026-02-26 Commit:
59ba3c2Branch:masterOVERVIEW
KeePass to Bitwarden migration CLI. Python runtime package + workspace stubs package. Core flow migrates
entries/folders/attachments/passkeys withbw serveas primary transport.STRUCTURE
kp2bw/ ├── src/kp2bw/ # Runtime package (CLI + conversion + bw transport + types) │ └── AGENTS.md ├── specs/ # OpenAPI spec for Bitwarden vault management API │ └── vault-management-api.json ├── tests/ # Script-style smoke + docker e2e + fixture contract │ └── AGENTS.md ├── packages/pykeepass-stubs/ # Separate stubs release stream │ └── AGENTS.md ├── scripts/ # Release/version checks + codegen for github-script │ └── AGENTS.md └── .github/workflows/ # CI orchestration (release + integration + codegen drift) └── AGENTS.mdWHERE TO LOOK
Task Location Notes CLI flags, prompts, envs src/kp2bw/cli.pyEntrypoint kp2bw.cli:mainandpython -m kp2bwhandoffConversion orchestration src/kp2bw/convert.py3-phase top-level flow; item+attachment migration logic Bitwarden HTTP transport src/kp2bw/bw_serve.pybw servelifecycle, dedup index, batch create, attachment uploadWorkflow policy details .github/workflows/AGENTS.mdTrigger matrix, cross-workflow dependencies, output contracts Release version gating scripts/version-check-shared.mjsNormalizes release/tag prefixes; drives workflow gates Main package publishing .github/workflows/publish.yml...
Files:
tests/migrate_uris_test.pytests/convert_ref_resolution_test.pytests/e2e_vaultwarden_test.pysrc/kp2bw/convert.pysrc/kp2bw/cli.pysrc/kp2bw/uri_mapping.py
tests/**
⚙️ CodeRabbit configuration file
tests/**: # TESTS KNOWLEDGE BASEOVERVIEW
Tests are script-executable smoke/e2e checks, with dockerized Vaultwarden integration and tracked fixtures.
WHERE TO LOOK
Task Location Notes bw command resolution tests/bw_serve_command_test.pyCross-platform argv wrapping + process teardown logic Windows bw.cmd live smoke tests/windows_bw_cmd_smoke.pyGated by KP2BW_RUN_WIN_CMD_SMOKE=1; Windows CI onlyPackage smoke validation tests/smoke_test.pyVerifies built artifact behavior, metadata, entry points Stubs package validation tests/stubs_smoke_test.pyVerifies partialmarker + type-check consumptionFull migration e2e tests/e2e_vaultwarden_test.pyRich seed + idempotency + golden snapshots Snapshot normalization tests/_snapshot.pyScrubs volatile fields, hashes attachments, golden diff Golden snapshots tests/__snapshots__/*.jsonCommitted expected vault state (pinned bw owns them) Integration infra tests/docker-compose.ymlVaultwarden + test container orchestration Vaultwarden image seeding tests/Dockerfile.vaultwardenPinned image + DB/key fixtures Test runner image tests/Dockerfile.testuv + lockfile-pinned bw CLI + test deps Reusable bw setup .github/actions/setup-bwInstalls a chosen @bitwarden/cli(matrix-able)Fixture retention rules tests/fixtures/.gitignoreAllowlist keeps required DB/certs tracked CONVENTIONS
- No pytest collection model; scripts use
main()andAssertionError...
Files:
tests/migrate_uris_test.pytests/convert_ref_resolution_test.pytests/e2e_vaultwarden_test.py
tests/e2e_vaultwarden_test.py
📄 CodeRabbit inference engine (tests/AGENTS.md)
Full migration e2e test must include rich seed data, idempotency validation, and golden snapshot comparison in
e2e_vaultwarden_test.py
Files:
tests/e2e_vaultwarden_test.py
{src/**/*.py,packages/**/*.py}
📄 CodeRabbit inference engine (AGENTS.md)
{src/**/*.py,packages/**/*.py}: Intra-package imports are relative (from .module import X)
Never use root logger calls (logging.info(...)etc.); use module logger
Files:
src/kp2bw/convert.pysrc/kp2bw/cli.pysrc/kp2bw/uri_mapping.py
src/kp2bw/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
src/kp2bw/**/*.py: Never use bareException; use project exceptions (BitwardenClientError,ConversionError)
Do not rewrite valid Python 3.14 comma-formexcept X, Y:syntax to tuple form
src/kp2bw/**/*.py: Use module logger withlogger = logging.getLogger(__name__), never root logger calls
Keep sensitive values out of logs - no rawbwcommands/output, session/password values, or decrypted vault content
Raise project exceptions (BitwardenClientError,ConversionError) instead of bareException
Keep relative imports for local modules (e.g.,from .exceptions import ConversionError)
Python 3.14 comma-form multi-except is valid; do not rewrite it to tuple form just for style
Files:
src/kp2bw/convert.pysrc/kp2bw/cli.pysrc/kp2bw/uri_mapping.py
src/kp2bw/convert.py
📄 CodeRabbit inference engine (AGENTS.md)
Conversion orchestration (3-phase flow, item+attachment migration logic) is in
src/kp2bw/convert.py
src/kp2bw/convert.py: Dedup keys on stable identity using KeePass entry UUID inKP2BW_IDcustom field, not on(folder, title)
Fold KeePass metadata (tags and expiry) into a YAMLKP2BW_METAtext field using PyYAMLsafe_dump(allow_unicode=False)to escape control chars and line-break code points
For existing-item sync, match by UUID first, then fall back to legacy(folder, name)matching for unstamped login items, then create new; preserve id/favorite/folder/org and union collectionIds on update
Files:
src/kp2bw/convert.py
src/kp2bw/{convert,bw_serve}.py
📄 CodeRabbit inference engine (src/kp2bw/AGENTS.md)
src/kp2bw/{convert,bw_serve}.py: Do not break idempotency: a re-run with no KeePass changes must issue noPUTand upload no attachment, controlled via_content_differs/ upload-if-missing gate
Reconcile attachments by content hash, not filename: upload if item lacks file, re-upload if bytes changed; delete stale copy only after replacement uploads (upload-then-delete), skip deletion on upload failure
Files:
src/kp2bw/convert.py
src/kp2bw/{convert,cli}.py
📄 CodeRabbit inference engine (src/kp2bw/AGENTS.md)
Oversize custom fields (value over
MAX_BW_ITEM_LENGTH, 10k) are offloaded to<key>.txtattachment instead of inline field, unless--include-oversize-secretsis set
Files:
src/kp2bw/convert.pysrc/kp2bw/cli.py
src/kp2bw/**
⚙️ CodeRabbit configuration file
src/kp2bw/**: # SOURCE PACKAGE KNOWLEDGE BASEOVERVIEW
Runtime package for CLI argument handling, KeePass parsing, Bitwarden conversion, and transport.
STRUCTURE
src/kp2bw/ ├── cli.py # CLI parsing, prompts, env handling, run mode selection ├── convert.py # Conversion orchestrator and entry transformation pipeline ├── bw_serve.py # bw serve process lifecycle + HTTP CRUD + attachment upload ├── bw_types.py # Hand-written TypedDict types (supplements generated types) ├── _bw_api_types.py # Auto-generated from specs/vault-management-api.json (DO NOT EDIT) ├── _console.py # Shared Rich Console instance (stderr) ├── exceptions.py # BitwardenClientError, ConversionError ├── __main__.py # python -m kp2bw handoff └── __init__.py # __version__ from installed metadataWHERE TO LOOK
Task Location Notes Add/adjust CLI flags src/kp2bw/cli.pymain()and argument parser are hereChange import behavior src/kp2bw/convert.pyTop-level convert()flow and migration phasesTune dedup/idempotency src/kp2bw/bw_serve.pyExisting item index + batch create behavior Attachment behavior src/kp2bw/bw_serve.pyAsync upload path and multipart logic Map KeePass fields src/kp2bw/convert.pyEntry/custom field/TOTP/passkey mapping API type definitions src/kp2bw/bw_types.pyHand-written TypedDicts supplementing codegen Regenerate API types scripts/generate-bw-types.shRun after editing specs/vault-management-api.jsonError contract src/kp2bw/exceptions.pyKeep custom exception taxonomy ...
Files:
src/kp2bw/convert.pysrc/kp2bw/cli.pysrc/kp2bw/uri_mapping.py
src/kp2bw/cli.py
📄 CodeRabbit inference engine (AGENTS.md)
CLI flags, prompts, envs are defined in
src/kp2bw/cli.pyAlways write a full DEBUG log to a per-user file via
_configure_loggingincli.py, independent of console verbosity; useKP2BW_LOG_FILE/KP2BW_LOG_DIRoverrides or%LOCALAPPDATA%/kp2bw/logs
Files:
src/kp2bw/cli.py
src/kp2bw/{cli,__main__}.py
📄 CodeRabbit inference engine (src/kp2bw/AGENTS.md)
Maintain behavior parity for
kp2bw.cli:mainandpython -m kp2bw
Files:
src/kp2bw/cli.py
🔇 Additional comments (12)
src/kp2bw/cli.py (1)
719-723: LGTM!Also applies to: 790-796
tests/convert_ref_resolution_test.py (1)
100-103: LGTM!src/kp2bw/uri_mapping.py (5)
1-47: LGTM!
49-189: LGTM!
192-258: LGTM!
273-307: Arrr, the scurvy URL leakage be properly scuttled!The previous review flagged raw vault URLs spillin' into the always-on DEBUG log. Ye've fixed it proper — all log statements now emit only the drop reason without interpolatin' the sensitive URI values. The comment at lines 273-275 makes the intent crystal clear for future maintainers. Excellent work, matey! 🏴☠️
310-442: LGTM!src/kp2bw/convert.py (5)
125-143: LGTM!
146-181: Yarr, the report now shows the true treasure map!The previous review flagged that
--report-uris keepasswas previewin' raw strings instead of the post-migration URIs. Ye've threadeduri_matchandinterpret_uri_syntaxthrough and now process every entry viabuild_login_uris— so the collision report previews exactly what migration would write, includin' quote/wildcard transforms and dropped non-web schemes. The report and migration be sailin' in lockstep now! 🏴☠️
199-200: LGTM!Also applies to: 225-226, 243-244, 320-326
450-461: LGTM!Also applies to: 514-515
793-808: LGTM!
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
- e2e: assert Multi URL has no duplicate URI values before the dict-based match assertion (dict comprehension would silently collapse dupes). - migrate_uris + e2e: replace broad startswith(KP2A_URL/URL/AndroidApp) legacy-field guard with is_url_attribute_key (the production predicate), so decoys like 'URLAlias' no longer count as un-migrated.
1 participant
What
KeePass(XC) additional URLs (
KP2A_URL,KP2A_URL_1, …) andAndroidApppackage ids were copied verbatim into Bitwarden custom fields, where they're inert. They now become real entries inlogin.uris, so one login autofills across every site and app it covered in KeePass.Per-URI match modes (reproducing KeePassXC)
matchis per-URI in Bitwarden, so each URL maps to the mode that reproduces its KeePassXC shape:match"…"host/*keepassxc:///cmd:///kdbx:///file://,{REF:…}AndroidApp=com.x.yandroidapp://com.x.yWhy best-effort on regex: Bitwarden runs one regex against the whole URL (unanchored, case-insensitive), whereas KeePassXC regexes host and path separately. A byte-for-byte copy of KeePassXC's internal regex wouldn't behave equivalently — the translation targets the match set, and complex wildcards log a review warning.
Config (two orthogonal knobs)
--uri-match/KP2BW_URI_MATCH— plain-tier match. Defaultdomain(faithful KeePassXC replication);defaultdefers to your Bitwarden account default;host/exact/… to tighten. Quoted/wildcard URLs keep their own modes regardless.--interpret-uri-syntax/--no-…/KP2BW_INTERPRET_URI_SYNTAX— whether quote/wildcard syntax is honored (default on; off = literal import).Upgrading existing imports
Two paths, both honouring the knobs and
-o/-c:KP2A_*fields, so_content_differsfires and the update replaces fields+login in place (KP2BW_IDmatches the right item). No extra step.--migrate-uris/KP2BW_MIGRATE_URIS— a Bitwarden-only one-shot pass (no KeePass) that re-folds the legacy fields into URIs on every existing login item, for users who don't want to re-import. Idempotent.Tests / gates
uri_mapping_test.py(match table, drops, dedup, literal mode, override, remap) +migrate_uris_test.py(client-double for the bw-only pass). ruff · ty · basedpyright (0/0/0) · dprint · pytest 16 passed / 4 skipped.