fix: asking for reprompt by soraxas · Pull Request #356 · doy/rbw

soraxas · 2026-07-09T09:03:50Z

No description provided.

…fields

…EntryData and rename structures around

Display logic has been transferred inside db, but it's temporary as it needs to be decoupled too.

…tr> instead of FromStr

…ecoupling

written with AI, verified and fixed by OI (Organic Intelligence)

…id downcasting errors

exchange_refresh_token and exchange_refresh_token_async were calling json_with_path() directly on the response without first checking the HTTP status. A 4xx from the identity server (e.g. {"error":"invalid_grant"}) was therefore parsed as ConnectRefreshTokenRes, failing with a cryptic "missing field `access_token`" JSON error instead of surfacing the actual HTTP status.

Bitwarden (and Vaultwarden) rotate the refresh token on every exchange: alongside the new access_token, the server returns a new refresh_token and immediately invalidates the old one. ConnectRefreshTokenRes only captured access_token, so the stored refresh token became stale after the first refresh cycle. The next sync would attempt to use the already-rotated token and receive HTTP 400 invalid_grant, surfacing as "api request returned error: 400". Fix: capture refresh_token in ConnectRefreshTokenRes, thread it back through with_exchange_refresh_token_async (return type expands from (Option<String>, T) to (Option<String>, Option<String>, T)), and persist it in Db via a new update_refresh_token method. All six public action functions (sync, add, edit, remove, list_folders, create_folder) and their callers in both the agent and the rbw binary are updated. Also adds body logging to client::sync() on non-2xx responses so future failures surface the server's error body alongside the status code.

Signed-off-by: Tin Lai <tin@tinyiu.com>

Folder names are encrypted with the local user vault key, even for organization entries. Passing the entry context sent org/item keys to the agent, which caused invalid MAC failures during list/search for entries in folders.

Copilot

Pull request overview

This PR is a broad refactor of rbw that modernizes the agent/client architecture (moving more logic into the daemon and making components async), updates crypto/session handling, and introduces optional SSH-signature confirmation via pinentry.

Changes:

Refactors the agent into a new async agent module with cached DB state, deadline-based lock/sync timers, and broadcast-based notifications.
Reworks pinentry and locked-memory handling (new LockedVec), and threads new crypto/session parameter structs through login/unlock flows.
Adds optional confirm_ssh behavior for SSH-agent signing plus Nix flake/devshell support and various cleanup/formatting updates.

Reviewed changes

Copilot reviewed 41 out of 44 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
`src/search.rs`	Adds a shared `Needle` type with parsing/display for search/find inputs.
`src/pwgen.rs`	Formatting-only simplifications in password generation.
`src/protocol.rs`	Fixes protocol version calculation; refactors environment variable handling/types.
`src/pinentry.rs`	Major rewrite of pinentry interaction: streaming parser, percent decode helpers, and new confirm flow.
`src/locked.rs`	Replaces the old locked vector type with `LockedVec` based on a fixed-size locked buffer.
`src/lib.rs`	Exposes the new `search` module.
`src/json.rs`	Simplifies JSON-with-path helpers; async path now relies on `From<reqwest::Error>`.
`src/identity.rs`	Switches identity derivation to use `CryptoParameters` and `LockedVec`.
`src/error.rs`	Adds new error variants and `From` conversions (I/O, UTF-8, reqwest).
`src/edit.rs`	Refactors editor invocation; improves error propagation and reduces allocations.
`src/dirs.rs`	Makes directory/file helpers fallible (`Result<PathBuf>`) instead of panicking.
`src/config.rs`	Adds `confirm_ssh` config knob; updates load/save paths to new `dirs` results.
`src/cipherstring.rs`	Updates locked decrypt return types and cleans up symmetric/asymmetric handling.
`src/bin/rbw/sock.rs`	Updates socket path retrieval for new fallible `dirs` APIs.
`src/bin/rbw/main.rs`	Makes client CLI async; refactors command wiring and completion generation.
`src/bin/rbw/actions.rs`	Simplifies client/agent request sending; adjusts log-path handling for fallible dirs.
`src/bin/rbw-agent/sock.rs`	Updates agent socket binding to use fallible `dirs` APIs.
`src/bin/rbw-agent/notifications.rs`	Replaces per-subscriber channel list with `tokio::sync::broadcast` and adds disconnect signaling.
`src/bin/rbw-agent/main.rs`	Replaces old agent/state/timeout modules with the new `agent` implementation and signal handling.
`src/bin/rbw-agent/debugger.rs`	Formatting-only adjustments to tracing/core-dump error messages.
`src/bin/rbw-agent/daemon.rs`	Updates pid/log file paths for new fallible `dirs` APIs.
`src/bin/rbw-agent/agent/ssh_agent.rs`	Reworks SSH agent integration to call into new `Agent` and adds optional confirmation.
`src/bin/rbw-agent/agent/mod.rs`	New central async `Agent` implementation (state, deadlines, notifications, request routing).
`src/bin/rbw-agent/agent/actions.rs`	New agent action handlers (login/unlock/sync/crypto/reprompt/ssh-key lookup).
`src/bin/rbw-agent/timeout.rs`	Removed (replaced by deadline-based logic in the new agent).
`src/bin/rbw-agent/state.rs`	Removed (state moved into the new agent internals).
`src/bin/rbw-agent/agent.rs`	Removed (replaced by `src/bin/rbw-agent/agent/mod.rs`).
`src/bin/rbw-agent/actions.rs`	Removed (replaced by `src/bin/rbw-agent/agent/actions.rs`).
`src/base64.rs`	Minor formatting cleanup.
`src/api/mod.rs`	New API models/enums and sync/login parsing logic (replacing older API layer).
`src/api/client.rs`	New async HTTP client implementation (reqwest/axum-based SSO callback flow).
`src/actions.rs`	Reworks public actions layer around new async API client and new session/crypto structs.
`shell.nix`	Adds a Nix dev shell definition for Rust tooling and pinentry.
`flake.nix`	Adds flake wiring (currently exposes a placeholder default package).
`flake.lock`	Locks nixpkgs input.
`README.md`	Documents fork status and the new `confirm_ssh` option.
`CHANGELOG.md`	Records the refactor, new options, and dependency removals.
`Cargo.toml`	Bumps version, adjusts deps/features (e.g., removes arrayvec/is-terminal/tokio-stream).
`Cargo.lock`	Updates lockfile for dependency changes.
`.rustfmt.toml`	Updates rustfmt settings (newline style).
`.github/workflows/tests.yaml`	Minor formatting tweak.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

+    pinentry.command("CONFIRM").await?;
+
+    pinentry.wait().await?;
+
+    Ok(true)


+        // NOTE: This panics if the line is > 4096 bytes
+        v.push(b);


        self.env_vars
            .iter()
-            .map(|(var, val)| (var.0.clone(), val.0.clone()))
-            .filter(|(var, _)| (*ENVIRONMENT_VARIABLES_OS).contains(var))
+            .map(|(var, val)| (var.0.as_os_str(), val.0.as_os_str()))
+            .filter(|(var, _)| (ENVIRONMENT_VARIABLES_OS).contains(&var.to_os_string()))
            .collect()


+    pinentry.command("SETTITLE rbw").await?;
+    pinentry.command(&format!("SETPROMPT {prompt}")).await?;
+    pinentry.command(&format!("SETDESC {desc}")).await?;
+
+    if let Some(err) = err {


+  outputs =
+    { self, nixpkgs }:
+    {
+
+      packages.x86_64-linux.hello = nixpkgs.legacyPackages.x86_64-linux.hello;
+
+      packages.x86_64-linux.default = self.packages.x86_64-linux.hello;
+      devShells.x86_64-linux.default = import ./shell.nix { pkgs = nixpkgs.legacyPackages.x86_64-linux; };
+
+    };


deade1e added 30 commits May 8, 2026 12:25

improve readability

76e9556

split display_short and remove duplicated code

46c0571

dedup notes printing code

6d50048

add temporary get_field

271e0d7

split display_field in two parts

bada984

shrink fields variable assignment

1e0a68d

move DecryptedData and DecryptedField up and rename fields to custom_…

8d17949

…fields

remove duplicated structure DecryptedData and substitute it with db::…

e1c953b

…EntryData and rename structures around

decouple Entry from commands

3e4231b

Display logic has been transferred inside db, but it's temporary as it needs to be decoupled too.

small comment to remember commenting about the pinentry on README

4981782

decouple fields decrypting from decrypt_cipher fn

7fa8201

improve decrypt_fields name

1f79cdd

add gdb to shell

ce1fb68

move decrypt_field down

1df225b

partially remove duplicated code inside decrypt_cipher

933c218

add some comments and improve naming of decrypt fns

5662ebb

add a nice comment on get_fields

e3505ac

decouple dynamic_field extraction logic

9ff38ef

make FieldType contain the Custom occurrence and just have it From<&s…

79a87d0

…tr> instead of FromStr

pre-validate totp_rs::Algorithm and improve overall readability

a65e2dd

dedup folder creation logic

55acedb

move display procedures back into commands.rs. still need dedup and d…

4facca0

…ecoupling

improve naming and spacing of display fns

c59878c

fix missing message in case no field is found in display_entry_field

6c034b6

small comment regarding "short"

c212ee7

improve naming and comment

9894742

move --full printing logic into Display trait impl for Entry

bb21e48

enable copying to clipboard and add some useful comments around

4dce25c

remove val_display_or_store

5728b02

make difference between Encrypted and Decrypted entries

4983b70

deade1e and others added 26 commits June 7, 2026 15:38

line

8fea98c

add a bunch of log::trace and release db guard to fix notifications bug

a57130a

fix missing re-evaluation of deadlines

7267c18

update CHANGELOG.md

8d5f4cd

written with AI, verified and fixed by OI (Organic Intelligence)

avoid some clones in error conversion

61d3e1e

error_model -> model

d992731

add specific error for unavailability of crypto parameters in db

c03b221

add specific error for unavailability of session parameters

67d8f50

re-use error for missing config email

2e534c4

add specific error for unavailability of protected keys in db and avo…

2f390cb

…id downcasting errors

remove duplicated retry logic by implementing with_retry

c258925

remove superfluous error context

daad9fd

remove superfluous deserializer impl

48f4a78

add a comment for a security weakness

df2c731

Merge branch 'refactor'

7a57d48

trigger workflow

febc2d0

fix clippy warnings

80d73a8

move guard up to make clippy happy

d0e5ad0

add cargo-deny

1baf275

update dependencies to fix vulns

adb7931

bump version to 1.16.0-rc1

09e0741

move Needle into search.rs

4df9835

surface error when it happens

9a967be

Signed-off-by: Tin Lai <tin@tinyiu.com>

Decrypt folder names with user key

5e1abcc

Folder names are encrypted with the local user vault key, even for organization entries. Passing the entry context sent org/item keys to the agent, which caused invalid MAC failures during list/search for entries in folders.

Copilot AI review requested due to automatic review settings July 9, 2026 09:03

soraxas closed this Jul 9, 2026

Copilot started reviewing on behalf of soraxas July 9, 2026 09:09 View session

Copilot AI reviewed Jul 9, 2026

View reviewed changes

Navigation Menu

fix: asking for reprompt - #356

fix: asking for reprompt#356
soraxas wants to merge 274 commits into
doy:mainfrom
soraxas:fix-asking-for-reprompt

soraxas commented Jul 9, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

soraxas commented Jul 9, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants