Skip to content

[PM-39239] Surface shareable access URLs in bw send output#1235

Open
adudek-bw wants to merge 1 commit into
tools/pm-34719/add-send-commands-to-rust-cli-2from
tools/pm-39239/surface-shareable-access-urls
Open

[PM-39239] Surface shareable access URLs in bw send output#1235
adudek-bw wants to merge 1 commit into
tools/pm-34719/add-send-commands-to-rust-cli-2from
tools/pm-39239/surface-shareable-access-urls

Conversation

@adudek-bw

Copy link
Copy Markdown
Contributor

🎟️ Tracking

https://bitwarden.atlassian.net/browse/PM-39239

📔 Objective

For Rust CLI

bw send get --text and the default output of bw send create now emit a shareable access URL that a user can paste to a recipient, instead of the raw access_id.

Previously --text emitted only the raw access_id and the create default output omitted the URL entirely — both because bw had no access to the configured web-vault base URL.

bw send get --text and the default output of bw send create now emit a
shareable access URL (<web-vault>/#/send/<access_id>/<url_b64_key>) instead
of the raw access_id. The web-vault base is derived from the client's
configured API URL, so self-hosted and cloud both work without hardcoding
bitwarden.com.
@adudek-bw adudek-bw requested review from a team as code owners July 9, 2026 12:15
@adudek-bw adudek-bw requested a review from djsmith85 July 9, 2026 12:15
@github-actions

github-actions Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

🤖 Bitwarden Claude Code Review

Overall Assessment: REQUEST CHANGES

Reviewed the change that makes bw send get --text and the default bw send create output emit a shareable access URL instead of a raw access_id. The URL construction (build_access_url, to_url_b64) and the URL-safe base64 mapping are correct and well-tested, matching the legacy SendResponse fragment shape. The main concern is how the web-vault base is derived from the API URL.

Code Review Details
  • ⚠️ : Access URL is a broken link for cloud users — web_vault_from_api_url only strips /api, so the cloud default https://api.bitwarden.com yields https://api.bitwarden.com/#/send/..., which does not serve the web-vault SPA.
    • crates/bw/src/tools/send.rs:770

Comment on lines +770 to +779
/// Pure derivation of the web-vault base from an API URL. Strips a trailing `/api` (the suffix the
/// CLI appends when building `api_url` from a server URL) and any surrounding slashes. Split out
/// from [`web_vault_url`] so it can be unit-tested without a live client.
fn web_vault_from_api_url(api_url: &str) -> String {
let trimmed = api_url.trim_end_matches('/');
trimmed
.strip_suffix("/api")
.unwrap_or(trimmed)
.trim_end_matches('/')
.to_string()

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ IMPORTANT: Access URL is a broken link for cloud users (the default case).

Details and fix

web_vault_from_api_url only strips a trailing /api. For the cloud default, the client's api_url is https://api.bitwarden.com (see main.rs:213 and the default when --server is omitted at login). That host has no /api suffix, so it is returned untouched and the assembled URL becomes:

https://api.bitwarden.com/#/send/<access_id>/<url_key>

The API host does not serve the web-vault SPA, so this is not a working recipient link. The web_vault_from_api_url_leaves_non_api_url_untouched test pins this behavior, but the resulting URL is broken for the most common (cloud) case. The same applies to self-hosted deployments that use a distinct API subdomain (e.g. api_url = https://api.example.com).

The legacy CLI does not derive the web vault from the API URL — getSendUrl()/getWebVaultUrl() return the actual configured vault/send host (for cloud, https://vault.bitwarden.com / https://send.bitwarden.com). Consider deriving the web-vault base from a configured web-vault URL (the web_vault config field already exists but is not wired into the client) rather than inverting api_url, and special-casing the cloud api.bitwarden.com host so it maps to the vault host instead of emitting a link to the API origin.

At minimum, the single-domain self-hosted assumption (api_url == <web-vault>/api) should be documented as a known limitation and the cloud default handled.

@sonarqubecloud

sonarqubecloud Bot commented Jul 9, 2026

Copy link
Copy Markdown

@github-actions

github-actions Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

🔍 SDK Breaking Change Detection

SDK Version: tools/pm-39239/surface-shareable-access-urls (8a751a6)

⚠️ If breaking changes are detected, a corresponding pull request addressing them must be ready for merge in the affected client repository.

Client Status Details
typescript ❌ Breaking changes detected Compilation failed with new SDK version. A corresponding pull request addressing the breaking changes must be ready for merge in bitwarden/clients. - View Details

Breaking change detection uses the build of the SDK from this branch, including any incompatibities pre-existing on or merged into this branch. Check the workflow logs to confirm.
Results update as workflows complete.

@codecov

codecov Bot commented Jul 9, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 56.52174% with 30 lines in your changes missing coverage. Please review.
✅ Project coverage is 85.08%. Comparing base (30e952e) to head (8a751a6).

Files with missing lines Patch % Lines
crates/bw/src/tools/send.rs 56.52% 30 Missing ⚠️
Additional details and impacted files
@@                                Coverage Diff                                 @@
##           tools/pm-34719/add-send-commands-to-rust-cli-2    #1235      +/-   ##
==================================================================================
- Coverage                                           85.13%   85.08%   -0.05%     
==================================================================================
  Files                                                 467      465       -2     
  Lines                                               65249    64850     -399     
==================================================================================
- Hits                                                55549    55178     -371     
+ Misses                                               9700     9672      -28     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.


/// Build the shareable Send access URL from a decrypted [`SendView`].
///
/// Format: `<web-vault>/#/send/<access_id>/<url_b64_key>`.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❓ I need to look at this more closely, but this look dangerous and might behave differently across environments.

@itsadrago can you please take a look at this and determine if this will be verifiable before a production release? I think a testing strategy was established during PM-39449.

See also

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My main takeaway from 39449 was that the US production env - and only that env - has a vanity URL that redirects to the vault, but hitting the vault link directly works too. Since that is the case I think the links produced by this code will work as-is, we just won't get the vanity URL in US prod. It would still be good to get confirmation from @itsadrago, that was a messy ticket :D

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mcamirault is correct. The links should work as-is, but we lose the vanity URL for the US prod env. I believe the CLI client doesn't have an authoritative source for the correct sends hostname and domain to use and so we have to derive it. We need a configuration service in this repo similar to DefaultEnvironmentService in the clients repo, but that's probably out of the scope of this work

Comment on lines +742 to +745
let web_vault = web_vault_url(client);
let url_key = to_url_b64(key);

Ok(format!("{web_vault}/#/send/{access_id}/{url_key}"))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While the approach itself is likely okay, it does bare risk of being brittle and needed in multiple places. I've started a discussion with the rest of the platform team, to see if there's a way to provide that url in a similar manner as the TS-clients currently do https://github.com/bitwarden/clients/blob/main/libs/common/src/platform/services/default-environment.service.ts#L441

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants