Add KeePassXC and pass external fallback providers#63
Conversation
Closes #49. Extends the reduced-security `--external-fallback` path beyond `1password` and `bitwarden` to also support `keepassxc` and `pass`. Both reuse the existing validated absolute-path execution model (owner-only, no `~`, no relative paths, 0755-or-tighter, bounded output, process-group timeout) so no new attack surface is introduced. KeePassXC: - requires `fallbackProviderDatabase` (absolute .kdbx path) in ~/.apw/config.json - reads the master password from APW_KEEPASSXC_PASSWORD and feeds it to keepassxc-cli over stdin; never written to disk or cached - runs `search` then `show --show-protected --attributes UserName,Password,URL` and maps the result into the existing external_cli payload shape pass (passwordstore.org): - relies on gpg-agent for the unlock, so APW never handles the key - discovers entries with `pass find <host>`, preferring an entry whose leaf name equals the host over a substring match - parses `pass show` output: line 1 is the password, `user:`/`username:` /`login:` and `url:`/`website:` lines supply the rest `run_external_provider_command` gains an optional stdin-feeding variant used only by KeePassXC; existing callers are unchanged. Provider failures map to typed APW errors: missing entry -> no_results, malformed CLI output -> proto_invalid_response, missing config/database -> invalid_config. Fallback payloads keep transport "external_cli" / securityMode "reduced_external_cli" and nothing is cached. Adds 8 tests (pass tree parsing, pass/keepassxc happy paths, and the typed-error paths for missing entry, missing master password, and missing database). README.md and docs/SECURITY_POSTURE_AND_TESTING.md document the per-provider setup and security posture.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6ce21db765
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
pheidon
left a comment
pheidon
left a comment
There was a problem hiding this comment.
I cannot approve this yet. The pass find tree parser over-counts top-level depth because a root row like └── web is treated as depth 1 instead of depth 0. That leaves the previous root on the stack when a later top-level sibling appears, so a real listing with multiple roots can synthesize invalid entry paths such as old-root/new-root/example.com and then call pass show on the wrong path.
Please fix the depth calculation, for example by subtracting the branch marker level with a saturating adjustment, and add a regression test with multiple top-level pass find roots where the matching host is under the second root. CI is otherwise green, but this parser bug blocks approval.
athena-omt
added
area:infra
…ack-providers-49 # Conflicts: # docs/SECURITY_POSTURE_AND_TESTING.md
|
Requested-change follow-up is addressed and ready for re-review.
Verification recorded on the PR:
|
4 participants
Summary
Extends the reduced-security
--external-fallbackpath beyond1passwordandbitwardento also supportkeepassxcandpass, per #49.Closes #49.
Both new providers reuse the existing validated absolute-path execution model
(owner-only, no
~, no relative paths,0755-or-tighter mode, bounded output,process-group timeout). Fallback payloads keep
transport: "external_cli"/securityMode: "reduced_external_cli"and nothing is cached.KeePassXC
fallbackProviderDatabasewith an absolute.kdbxpath.APW_KEEPASSXC_PASSWORDand feeds it tokeepassxc-cliover stdin.searchthenshow --show-protected --attributes UserName,Password,URL.pass
gpg-agentfor unlock; APW never handles the master key.pass find <host>tree output into terminal entry candidates only.substring fallback.
synthesize paths like
old-root/web/example.com.Verification
cargo test --manifest-path rust/Cargo.toml pass_pick_entry_- passed, 5 testscargo test --manifest-path rust/Cargo.toml login_- relevant unit tests passed before the sandbox entered socket e2e coverage; the e2e socket bind is sandbox-blocked herecargo fmt --manifest-path rust/Cargo.toml -- --check- passedbash scripts/ci/run-fast-checks.sh- passedgit diff --check- passedNotes
passCLIs are not available in this environment. Happy paths are exercised against shell-script simulators; a maintainer should still smoke-test against real CLIs on macOS before release.