From 71f1aa4c6f83996f827fc8af02eed5e89f684b59 Mon Sep 17 00:00:00 2001 From: leetcrypt Date: Fri, 19 Jun 2026 16:57:30 -0700 Subject: [PATCH] pwstore: keepassxc LIVE-VM proof + root-group entry-path fix MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Proven end-to-end (scan→plan→commit→restore-from-backup) against the real keepassxc-cli 2.7.6 in the incredigo-sbx sandbox VM. The live run surfaced an adapter bug the fake CSV masked: KeePassXC's CSV "Group" column is the full path including the root group name (default root is "Passwords"), but keepassxc-cli addresses entries relative to root (/GitHub, not /Passwords/GitHub). kpEntryPath now drops the leading root segment; the unit test that encoded the buggy /Root/GitHub form is corrected and a nested-group table test added. Records the proof as data: docs/BROWSER-ROTATION.md promotes keepassxc MOCK-ONLY -> LIVE-VM (the others stay MOCK-ONLY). Co-Authored-By: Claude Opus 4.6 --- docs/BROWSER-ROTATION.md | 19 ++++++++++++++----- internal/pwstore/keepassxc.go | 17 +++++++++++++---- internal/pwstore/keepassxc_test.go | 21 +++++++++++++++++++-- 3 files changed, 46 insertions(+), 11 deletions(-) diff --git a/docs/BROWSER-ROTATION.md b/docs/BROWSER-ROTATION.md index 3dcb3a8..500d23a 100644 --- a/docs/BROWSER-ROTATION.md +++ b/docs/BROWSER-ROTATION.md @@ -7,7 +7,7 @@ custody → **mandatory backup** → rotate/verify/commit → guide) — it is * Status: **Phase B (propagation engine) — built.** `pwgen` + `pwstore` (all five adapters) + the sealed staged-list + the `incredigo passwords scan|plan|guide|commit` surface are -implemented and tested (adapters MOCK-ONLY against fake binaries/CSVs — see §8). Phases +implemented and tested (keepassxc is LIVE-VM proven; the rest MOCK-ONLY — see §8). Phases A-tier-1 / A-tier-2 (site automation) are specified here but **not yet built**. Nothing in this doc changes a password **at a website** automatically — the human still performs the site change + MFA; incredigo only propagates the new value into the *manager* afterward. @@ -219,18 +219,27 @@ entries additionally record that a flag-gated plaintext file was created **and s ## 8. Validation status (DATA, mirrors `rotate/proofs.go` philosophy) The adapters contain ONLY real CLI/CSV code — no test branches. What differs is how each -write path has been *validated*. As of the Phase-B build every adapter is **MOCK-ONLY**: -exercised against a fake `bw`/`op`/`keepassxc-cli` binary (state-dir scripts) or a fake -export/import CSV, never yet against the real manager binary or a real browser re-import. +write path has been *validated*. Most adapters are still **MOCK-ONLY** (exercised against a +fake `bw`/`op` binary or a fake export/import CSV); **keepassxc is LIVE-VM** — proven end to +end (`scan → plan → commit → restore-from-backup`) against the genuine `keepassxc-cli 2.7.6` +in the `incredigo-sbx` sandbox VM (`lab-provision-keepass.sh`). | Adapter | Write shape | Secret path off-argv? | Validated against | |-----------|------------------------|-----------------------|-------------------| | bitwarden | `bw edit item` (stdin) | **yes** — base64 on stdin | fake `bw` (MOCK-ONLY) | -| keepassxc | `keepassxc-cli edit -p`| **yes** — db+new pass on stdin | fake `keepassxc-cli` (MOCK-ONLY) | +| keepassxc | `keepassxc-cli edit -p`| **yes** — db+new pass on stdin | **real keepassxc-cli 2.7.6 (LIVE-VM)** | | 1password | `op item edit pw=…` | **no** — argv assignment¹ | fake `op` (MOCK-ONLY) | | chrome | tmpfs CSV → human re-import | n/a (no live secret to a child) | fake CSV (MOCK-ONLY) | | firefox | tmpfs CSV → human re-import | n/a | fake CSV (MOCK-ONLY) | +**keepassxc LIVE-VM finding:** the CSV `Group` column is the FULL group path *including* the +root group name (the default DB names its root `Passwords`), but `keepassxc-cli` addresses +entries *relative to root* — a top-level entry is `/GitHub`, not `/Passwords/GitHub`. The +mock CSV (root group `Root`) never exposed this; the real CLI 404'd until `kpEntryPath` was +fixed to drop the leading root segment. The recoverability leg (feeding the sealed `backup.age` +back through `commit --stage-in`) restored both original passwords, proving the backup is a +usable restore artifact, not just a sealed blob. + ¹ **1Password argv caveat:** `op item edit` has no stdin-per-field path (only `op item create` does), and the JSON-template alternative would write plaintext to disk (hard rule 3). The remaining real path is `op item edit password=`, so the new value is visible to diff --git a/internal/pwstore/keepassxc.go b/internal/pwstore/keepassxc.go index 6f1e3e4..cba2c3f 100644 --- a/internal/pwstore/keepassxc.go +++ b/internal/pwstore/keepassxc.go @@ -92,15 +92,24 @@ func (k *KeePassXC) Export(ctx context.Context, v *vault.Vault) ([]Account, erro return accts, nil } -// kpEntryPath builds the "/Group/Title" path keepassxc-cli edit expects. KeePassXC -// exports the root group as "Root"; entries live under "/Root/<…>" but the CLI also -// accepts the group path as exported, so we join them verbatim. +// kpEntryPath builds the entry path keepassxc-cli edit/show expect. KeePassXC's CSV +// "Group" column is the FULL group path INCLUDING the root group name (e.g. the +// default DB names its root group "Passwords", so a top-level entry exports with +// Group="Passwords"). The CLI, however, addresses entries RELATIVE to the root — +// the root group name is omitted from the path: a root entry is "/GitHub", and a +// nested entry "Passwords/Web/GitHub" is addressed "/Web/GitHub". So we drop the +// first (root) segment of the exported group path. Verified against real +// keepassxc-cli 2.7.6 in the sandbox VM (the buggy "/Root/GitHub" form 404s). func kpEntryPath(group, title string) string { group = strings.Trim(group, "/") if group == "" { return "/" + title } - return "/" + group + "/" + title + // Strip the leading root-group segment; keep any remaining nested subgroups. + if _, rest, ok := strings.Cut(group, "/"); ok { + return "/" + rest + "/" + title + } + return "/" + title } // UpdatePassword changes one entry's password via `keepassxc-cli edit -p `. diff --git a/internal/pwstore/keepassxc_test.go b/internal/pwstore/keepassxc_test.go index 4180e12..7cc4fa7 100644 --- a/internal/pwstore/keepassxc_test.go +++ b/internal/pwstore/keepassxc_test.go @@ -90,8 +90,10 @@ func TestKeePassXCExport(t *testing.T) { t.Fatalf("got %d accounts, want 1", len(accts)) } a := accts[0] - if a.ID != "/Root/GitHub" { - t.Errorf("entry path = %q, want /Root/GitHub", a.ID) + // The root group name ("Root" here) is dropped — keepassxc-cli addresses entries + // relative to the root, so a top-level entry is "/GitHub" not "/Root/GitHub". + if a.ID != "/GitHub" { + t.Errorf("entry path = %q, want /GitHub", a.ID) } if a.Username != "alice@example.com" || a.Site != "github.com" { t.Errorf("account = %+v", a) @@ -128,6 +130,21 @@ func TestKeePassXCUpdatePassword(t *testing.T) { } } +func TestKPEntryPathDropsRootGroup(t *testing.T) { + cases := []struct{ group, title, want string }{ + {"Passwords", "GitHub", "/GitHub"}, // root entry: root name dropped + {"Root", "GitHub", "/GitHub"}, // same, different root name + {"", "GitHub", "/GitHub"}, // no group at all + {"Passwords/Web", "GitHub", "/Web/GitHub"}, // nested: keep subgroup, drop root + {"/Passwords/Web/", "GitHub", "/Web/GitHub"}, // surrounding slashes tolerated + } + for _, c := range cases { + if got := kpEntryPath(c.group, c.title); got != c.want { + t.Errorf("kpEntryPath(%q,%q) = %q, want %q", c.group, c.title, got, c.want) + } + } +} + func TestKeePassXCUnavailableWithoutDB(t *testing.T) { k := &KeePassXC{Bin: "keepassxc-cli"} if k.Available() {