Tools/pm 39238/complete file send lifecycle by adudek-bw · Pull Request #1303 · bitwarden/sdk-internal

adudek-bw · 2026-07-28T11:23:36Z

🎟️ Tracking

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

📔 Objective

Completes the file-send lifecycle for the Rust CLI bw send, which PR #1176 (PM-34719) stubbed out with "not yet implemented" errors.

🚨 Breaking Changes

Changes to the WASM/UniFFI binding surface, safe today (no live consumers — the clients repo routes file-send creation to the legacy service and does not call these):

SendClient::create_file_send gains a file_buffer: Vec parameter; CreateFileSendResponse gains encrypted_file_buffer: Vec.
SendClient::upload_send_file gains file_upload_type and upload_url parameters.

The cross-repo TypeScript compatibility check will confirm no client breakage.

…ub.com:bitwarden/sdk-internal into tools/pm-34719/add-send-commands-to-rust-cli-2

Co-authored-by: John Harrington <84741727+harr1424@users.noreply.github.com>

- Trim heavy doc-comments on private items in bitwarden-send/edit.rs - Rename `--output-file` to `--output` on `bw send get` (PR #1178 freed the long form) - Replace `PM-34719` TODO references with follow-up tickets (PM-39238 file-send lifecycle, PM-39239 access URLs, PM-39240 encodedJson) - Delete stale `--deleteInDays` validation TODO (now enforced at clap level via `value_parser`) - Soften text/file disambiguation TODO to a behavior note (matches legacy CLI) - Update `bw send receive` stub to reference sibling ticket PM-34718 - Minor wording fix in tests/send.rs

Replace "type-immutability enforcement" with an explicit "reject text↔file type changes" so readers don't confuse it with Rust's `mut`.

- Move SendArgs, SendCommands, and the 8 Send*Args structs from tools/mod.rs to tools/send.rs so the arg definitions live alongside their `impl BwCommand for ...` blocks. `pub use send::SendArgs` preserves the `tools::SendArgs` external import path. - Replace the custom `parse_delete_in_days` Result-returning parser with a `PossibleValuesParser`-based combinator. clap now surfaces the allowed values in `--help` output: [possible values: 1, 2, 3, 7, 14, 30] - Update the matching integration test to assert on clap's "possible values" error wording.

clippy::implied_bounds_in_impls flagged the `+ Clone` as redundant — `TypedValueParser`'s supertrait already requires `Clone`.

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.

Implements bw send create --file: encrypt the local file under the send key and upload via SendClient::upload_send_file, which now dispatches Direct (multipart POST) vs Azure (single-blob PUT to the pre-signed URL, renew-on-expiry). Adds a premium precondition check (reads the premium claim from the access-token JWT) before file create, and adds premium to bitwarden_core::auth::JwtToken. Verifies text/file disambiguation and re-points the get --output stub to PM-34718.

Address code review: send the encrypted buffer length as SendRequestModel.file_length (matching legacy's encrypt-then-create ordering) instead of None, and stop setting SendFileModel.size to the plaintext length on create (leave None; the server derives it from the uploaded blob). create_file_send now takes the plaintext file_buffer, encrypts it internally under the recorded send key, sets file_length, and returns the ciphertext for upload.

…te-file-send-lifecycle # Conflicts: # crates/bw/src/tools/send.rs # crates/bw/tests/send.rs

aikido-pr-checks · 2026-07-28T11:24:38Z

+) -> color_eyre::eyre::Result<bitwarden_send::SendView> {
+    // Read the plaintext before creating the send so a read failure aborts before we register a
+    // send that would then have no content.
+    let bytes = std::fs::read(path).wrap_err_with(|| format!("Could not read file {path}"))?;


Path traversal attack possible - critical severity
The application constructs file paths using untrusted data, potentially leading to a path traversal vulnerability. An attacker could manipulate these inputs to access, create, or overwrite sensitive files.

Show fix

Suggested change

let bytes = std::fs::read(path).wrap_err_with(|| format!("Could not read file {path}"))?;

// Prevent path traversal attacks by rejecting paths containing '..'.

let path_buf = std::path::Path::new(path);

if path_buf.components().any(|c| c == std::path::Component::ParentDir) {

return Err(eyre!("Invalid input: {}", path_buf.display()));

}

let bytes = std::fs::read(path_buf).wrap_err_with(|| format!("Could not read file {path}"))?;

_{Reply @AikidoSec ignore: [REASON] to ignore this issue.}
_{More info}

@AikidoSec ignore: local CLI argument, not a trust-boundary crossing — the operating user already has filesystem access to any path they pass here.

✅ Based on your feedback, we ignored this issue because of the following reason:

local CLI argument, not a trust-boundary crossing — the operating user already has filesystem access to any path they pass here.

github-actions · 2026-07-28T12:13:28Z

🔍 SDK Breaking Change Detection

SDK Version: tools/pm-39238/complete-file-send-lifecycle (638c6f4)

⚠️ 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	✅ No breaking changes detected	Compilation passed with new SDK version - View Details
android	✅ No breaking changes detected	Compilation passed with new SDK version - 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.

dani-garcia

Looks pretty good, but some of the comments are pretty wordy and unneeded. It would be good to trim them down.

codecov · 2026-07-28T12:30:28Z

Codecov Report

❌ Patch coverage is 91.10379% with 108 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.06%. Comparing base (b2c68b4) to head (bd00b41).

Files with missing lines	Patch %	Lines
crates/bw/src/tools/send.rs	40.42%	56 Missing ⚠️
crates/bw/src/tools/receive.rs	88.86%	51 Missing ⚠️
crates/bitwarden-send/src/access.rs	99.64%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1303      +/-   ##
==========================================
+ Coverage   85.90%   86.06%   +0.16%     
==========================================
  Files         493      495       +2     
  Lines       71347    72535    +1188     
==========================================
+ Hits        61290    62429    +1139     
- Misses      10057    10106      +49

☔ 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.

Addresses PR feedback flagging duplicate/unnecessary comments.

harr1424

This looks good, I agree on the above regarding some of these comments, though.

…com:bitwarden/sdk-internal into tools/pm-39238/complete-file-send-lifecycle

aikido-pr-checks · 2026-07-29T13:18:15Z

+) -> color_eyre::eyre::Result<bitwarden_send::SendView> {
+    // Read the plaintext before creating the send so a read failure aborts before we register a
+    // send that would then have no content.
+    let bytes = std::fs::read(path).wrap_err_with(|| format!("Could not read file {path}"))?;


Path traversal attack possible - critical severity
The application constructs file paths using untrusted data, potentially leading to a path traversal vulnerability. An attacker could manipulate these inputs to access, create, or overwrite sensitive files.

Show fix

Suggested change

let bytes = std::fs::read(path).wrap_err_with(|| format!("Could not read file {path}"))?;

// Prevent path traversal attacks by rejecting paths containing '..'.

let path_buf = std::path::Path::new(path);

if path_buf.components().any(|c| c == std::path::Component::ParentDir) {

return Err(eyre!("Invalid input: {}", path_buf.display()));

}

let bytes = std::fs::read(path).wrap_err_with(|| format!("Could not read file {path}"))?;

_{Reply @AikidoSec ignore: [REASON] to ignore this issue.}
_{More info}

@AikidoSec ignore: local CLI argument, not a trust-boundary crossing — the operating user already has filesystem access to any path they pass here.

✅ Based on your feedback, we ignored this issue because of the following reason:

local CLI argument, not a trust-boundary crossing — the operating user already has filesystem access to any path they pass here.

@adudek-bw yes, the user has access to the filesystem, but if the path contains "../../../" for example it could still write where they're not expecting it to be. This should still be fixed.

harr1424

The comments seem much more focused now! I found some error output which production users might find strange; should we consider referencing the internal Jira tickets using code comments as opposed to command output?

harr1424 · 2026-07-29T13:35:24Z

            // be a worse UX than an explicit "not implemented" error.
            Some(SendCommands::Get(args)) if args.output_path.is_some() => Err(eyre!(
-                "`--output` on `bw send get` is not yet implemented (tracked under PM-39238)."
+                "`--output` on `bw send get` is not yet implemented (tracked under PM-34718)."


❓ Will this output show in our production release? If so, does it make sense to reference an internal Jira ticket in the output itself?

harr1424 · 2026-07-29T13:35:42Z

❓ Will this output show in our production release? If so, does it make sense to reference an internal Jira ticket in the output itself?

harr1424 · 2026-07-29T13:35:47Z

❓ Will this output show in our production release? If so, does it make sense to reference an internal Jira ticket in the output itself?

Adds SendAccessKey (bitwarden-send), the anonymous-decrypt primitive that derives a Send's symmetric key from the URL fragment without any account key store or logged-in user. Wires up bw receive and its bw send receive alias: URL/API/identity resolution, password resolution (--password/--passwordenv/--passwordfile with interactive fallback gated on BW_NOINTERACTION), the password and email-OTP send-access token negotiation flow, and file download/decrypt/save via a new --output flag (replacing the previous --obj path flag to match bw send get's existing convention). Adds --fullObject for dumping the decrypted Send as JSON. Leaves bw send get --output as an explicit not-yet-implemented error with an expanded comment, since no owner-scoped download endpoint exists server-side.

…com:bitwarden/sdk-internal into tools/pm-39238/complete-file-send-lifecycle

# Conflicts: # crates/bw/src/tools/send.rs

SendFileTemplate derived Default, which gave send_type: 0 (the text discriminant) since that's u8::default() — the derive silently ignored the "1 = file" comment. bw send template send.file emitted type: 0 instead of 1. Replaced the derive with a manual Default impl and added a regression test pinning the discriminant.

adudek-bw and others added 27 commits June 8, 2026 09:55

Add Send functionality to rust CLI

7ee42d5

PR fixes

dcc75e1

Merge branch 'main' into tools/pm-34719/add-send-commands-to-rust-cli-2

176c1c9

PR fixes

b6871b3

Merge branch 'tools/pm-34719/add-send-commands-to-rust-cli-2' of gith…

6a674f2

…ub.com:bitwarden/sdk-internal into tools/pm-34719/add-send-commands-to-rust-cli-2

Merge branch 'main' into tools/pm-34719/add-send-commands-to-rust-cli-2

b5f47fd

PR fix

8092c14

PR fix

5038c1a

Merge branch 'main' into tools/pm-34719/add-send-commands-to-rust-cli-2

358c840

PR fix

47b4865

Merge branch 'tools/pm-34719/add-send-commands-to-rust-cli-2' of gith…

2aac235

…ub.com:bitwarden/sdk-internal into tools/pm-34719/add-send-commands-to-rust-cli-2

Merge branch 'main' into tools/pm-34719/add-send-commands-to-rust-cli-2

2718bef

PR fixes

4e7f043

Update crates/bw/tests/send.rs

1113cf1

Co-authored-by: John Harrington <84741727+harr1424@users.noreply.github.com>

Update crates/bw/tests/send.rs

a7456e8

Co-authored-by: John Harrington <84741727+harr1424@users.noreply.github.com>

Clarify type-immutability comment on bw send edit

e98b2fe

Replace "type-immutability enforcement" with an explicit "reject text↔file type changes" so readers don't confuse it with Rust's `mut`.

Merge branch 'main' into tools/pm-34719/add-send-commands-to-rust-cli-2

ceace58

merge main

f496e22

Merge branch 'main' into tools/pm-34719/add-send-commands-to-rust-cli-2

af1d1cf

Drop redundant Clone bound from delete_in_days_parser

30e952e

clippy::implied_bounds_in_impls flagged the `+ Clone` as redundant — `TypedValueParser`'s supertrait already requires `Clone`.

PR fixes

34e7de4

Merge remote-tracking branch 'origin/main' into tools/pm-39238/comple…

1cca1f4

…te-file-send-lifecycle # Conflicts: # crates/bw/src/tools/send.rs # crates/bw/tests/send.rs

adudek-bw requested review from a team as code owners July 28, 2026 11:23

adudek-bw requested a review from dani-garcia July 28, 2026 11:23

aikido-pr-checks Bot reviewed Jul 28, 2026

View reviewed changes

Merge branch 'main' into tools/pm-39238/complete-file-send-lifecycle

638c6f4

dani-garcia previously approved these changes Jul 28, 2026

View reviewed changes

Comment thread crates/bitwarden-core/src/auth/jwt_token.rs Outdated

Comment thread crates/bw/src/tools/send.rs Outdated

adudek-bw added 2 commits July 28, 2026 10:10

Merge branch 'main' into tools/pm-39238/complete-file-send-lifecycle

959af75

chore(bw): trim redundant review comments in send.rs and jwt_token.rs

943e126

Addresses PR feedback flagging duplicate/unnecessary comments.

harr1424 previously approved these changes Jul 29, 2026

View reviewed changes

Merge branch 'tools/pm-39238/complete-file-send-lifecycle' of github.…

5fc85b7

…com:bitwarden/sdk-internal into tools/pm-39238/complete-file-send-lifecycle

adudek-bw dismissed stale reviews from harr1424 and dani-garcia via 5fc85b7 July 29, 2026 13:17

aikido-pr-checks Bot reviewed Jul 29, 2026

View reviewed changes

adudek-bw requested review from dani-garcia and harr1424 July 29, 2026 13:31

harr1424 reviewed Jul 29, 2026

View reviewed changes

PR fixes

e4edf56

adudek-bw requested review from harr1424 and withinfocus July 30, 2026 16:41

adudek-bw added 3 commits July 30, 2026 12:42

Merge branch 'main' into tools/pm-39238/complete-file-send-lifecycle

a4495a6

Merge branch 'main' into tools/pm-39238/complete-file-send-lifecycle

e88d175

dani-garcia previously approved these changes Jul 31, 2026

View reviewed changes

adudek-bw added 2 commits July 31, 2026 16:13

Merge branch 'tools/pm-39238/complete-file-send-lifecycle' of github.…

a2e4d4c

…com:bitwarden/sdk-internal into tools/pm-39238/complete-file-send-lifecycle

Merge branch 'main' into tools/pm-39238/complete-file-send-lifecycle

516466a

# Conflicts: # crates/bw/src/tools/send.rs

adudek-bw dismissed dani-garcia’s stale review via 516466a July 31, 2026 20:25

adudek-bw requested a review from dani-garcia July 31, 2026 20:25

harr1424 approved these changes Jul 31, 2026

View reviewed changes

-    let bytes = std::fs::read(path).wrap_err_with(|| format!("Could not read file {path}"))?;
+    // Prevent path traversal attacks by rejecting paths containing '..'.
+    let path_buf = std::path::Path::new(path);
+    if path_buf.components().any(|c| c == std::path::Component::ParentDir) {
+        return Err(eyre!("Invalid input: {}", path_buf.display()));
+    }
+    let bytes = std::fs::read(path_buf).wrap_err_with(|| format!("Could not read file {path}"))?;

Uh oh!

Conversation

adudek-bw commented Jul 28, 2026

🎟️ Tracking

📔 Objective

🚨 Breaking Changes

Uh oh!

aikido-pr-checks Bot Jul 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adudek-bw Jul 28, 2026

Choose a reason for hiding this comment

Uh oh!

aikido-pr-checks Bot Jul 28, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Jul 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🔍 SDK Breaking Change Detection

Uh oh!

dani-garcia left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

codecov Bot commented Jul 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

harr1424 left a comment

Choose a reason for hiding this comment

Uh oh!

aikido-pr-checks Bot Jul 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adudek-bw Jul 29, 2026

Choose a reason for hiding this comment

Uh oh!

aikido-pr-checks Bot Jul 29, 2026

Choose a reason for hiding this comment

Uh oh!

withinfocus Jul 29, 2026

Choose a reason for hiding this comment

Uh oh!

harr1424 left a comment

Choose a reason for hiding this comment

Uh oh!

harr1424 Jul 29, 2026

Choose a reason for hiding this comment

Uh oh!

harr1424 Jul 29, 2026

Choose a reason for hiding this comment

Uh oh!

harr1424 Jul 29, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

aikido-pr-checks Bot Jul 28, 2026 •

edited

Loading

github-actions Bot commented Jul 28, 2026 •

edited

Loading

codecov Bot commented Jul 28, 2026 •

edited

Loading

aikido-pr-checks Bot Jul 29, 2026 •

edited

Loading