Skip to content

[PM-24618] Integrate CLI sends with server branch #18106

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
harr1424 wants to merge 22 commits into
base: main
Choose a base branch
from
+847 −19
Draft

[PM-24618] Integrate CLI sends with server branch #18106

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
6983929
WIP explore existing Send attributes and logic
harr1424 Dec 24, 2025
68323dd
Merge branch 'main' into PM-24618-CLI-reveal-email-CLI-flag
harr1424 Dec 26, 2025
913a6ad
WIP progress checkpoint Send refactor
harr1424 Dec 26, 2025
e8794cf
send creation integrated with server and returns object containing em…
harr1424 Dec 27, 2025
e2659d6
Claude! Review my code!
harr1424 Dec 29, 2025
705bc2b
WIP respond to Claude & add tests
harr1424 Dec 30, 2025
5ea4414
Merge branch 'main' into PM-24618-CLI-reveal-email-CLI-flag
harr1424 Dec 30, 2025
0fd81fa
improve mutual exclusion check for create with emails and password
harr1424 Dec 30, 2025
07eb16d
respond to review comments
harr1424 Dec 30, 2025
51f920d
Merge branch 'main' into PM-24618-CLI-reveal-email-CLI-flag
harr1424 Jan 9, 2026
f31aca2
[PM-21774] Adjust icon and tooltip for protected Sends on the Sends l…
mcamirault Jan 9, 2026
4d486aa
Merge branch 'main' into tools/pm-21774/send-list-page-updates
mcamirault Jan 9, 2026
94b4b11
[PM-21774] Update Sent table UI stories
mcamirault Jan 9, 2026
06bc0d5
[PM-21774] Fix Send table UI story
mcamirault Jan 9, 2026
0f611a7
[PM-21774] Merge changes from main and resolve conflicts
mcamirault Jan 12, 2026
70a222b
merge origin/tools/pm-21774/send-list-page-updates
harr1424 Jan 15, 2026
42691b7
Merge remote-tracking branch 'origin/main' into PM-23108-CLI-Add-emai…
harr1424 Jan 15, 2026
ff81b34
fix eslint issue
harr1424 Jan 15, 2026
c8c7f8b
resolve eslint errors
harr1424 Jan 15, 2026
02a3b6b
Merge branch 'main' into PM-24618-CLI-reveal-email-CLI-flag
harr1424 Jan 15, 2026
d27a9b4
fix strict typing errors
harr1424 Jan 15, 2026
6014a7d
Merge branch 'main' into PM-24618-CLI-reveal-email-CLI-flag
harr1424 Jan 15, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions apps/cli/src/tools/send/commands/create.command.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { getUserId } from "@bitwarden/common/auth/services/account.service";
import { BillingAccountProfileStateService } from "@bitwarden/common/billing/abstractions/account/billing-account-profile-state.service";
import { EnvironmentService } from "@bitwarden/common/platform/abstractions/environment.service";
import { SendType } from "@bitwarden/common/tools/send/enums/send-type";
import { AuthType } from "@bitwarden/common/tools/send/models/domain/send";
import { SendApiService } from "@bitwarden/common/tools/send/services/send-api.service.abstraction";
import { SendService } from "@bitwarden/common/tools/send/services/send.service.abstraction";
import { NodeUtils } from "@bitwarden/node/node-utils";
Expand Down Expand Up @@ -87,6 +88,15 @@ export class SendCreateCommand {

req.key = null;
req.maxAccessCount = maxAccessCount;
req.emails = emails;
Copy link
Contributor

@claude claude bot Dec 29, 2025

Choose a reason for hiding this comment

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

⚠️ IMPORTANT: authType calculated but never assigned to encrypted send before API call

Details and fix

Lines 93-99 calculate req.authType based on whether emails or password is provided, but this value is never transferred to encSend before the API save at line 154.

Looking at line 152: encSend.emails = emails && emails.join(","); - the emails are assigned, but authType is not.

Fix needed around line 152:

encSend.deletionDate = sendView.deletionDate;
encSend.expirationDate = sendView.expirationDate;
encSend.emails = emails && emails.join(",");
encSend.authType = req.authType;  // Add this

Impact: The server receives sends without the authType field, potentially breaking email-based OTP authentication or causing it to default incorrectly.


if (emails != null && emails.length > 0) {
Copy link
Contributor

@claude claude bot Dec 29, 2025

Choose a reason for hiding this comment

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

⚠️ IMPORTANT: Unsafe array length check on potentially undefined value

Details and fix

Line 82 extracts emails with ?? undefined, but line 93 checks emails.length > 0 without first verifying emails is not undefined. This will throw TypeError: Cannot read property 'length' of undefined if emails is undefined.

Fix:

if (emails != null && emails.length > 0) {
  req.authType = AuthType.Email;
} else if (password != null && password.trim().length > 0) {
  req.authType = AuthType.Password;
} else {
  req.authType = AuthType.None;
}

Why: JavaScript/TypeScript will throw when accessing .length on undefined. The nullish coalescing on line 82 explicitly returns undefined, making this a runtime error waiting to happen.

Copy link
Contributor Author

@harr1424 harr1424 Dec 29, 2025

Choose a reason for hiding this comment

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

Loose equality operator != will handle undefined as intended here, which is likely why the suggested "fix" is identical to the existing code.

req.authType = AuthType.Email;
} else if (password != null && password.trim().length > 0) {
req.authType = AuthType.Password;
} else {
req.authType = AuthType.None;
}

const hasPremium$ = this.accountService.activeAccount$.pipe(
switchMap(({ id }) => this.accountProfileService.hasPremiumFromAnySource$(id)),
Expand Down
4 changes: 4 additions & 0 deletions apps/cli/src/tools/send/models/send.response.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// @ts-strict-ignore
import { Utils } from "@bitwarden/common/platform/misc/utils";
import { SendType } from "@bitwarden/common/tools/send/enums/send-type";
import { AuthType } from "@bitwarden/common/tools/send/models/domain/send";
import { SendView } from "@bitwarden/common/tools/send/models/view/send.view";

import { BaseResponse } from "../../../models/response/base.response";
Expand Down Expand Up @@ -92,6 +93,7 @@ export class SendResponse implements BaseResponse {
emails?: Array<string>;
disabled: boolean;
hideEmail: boolean;
authType: AuthType;

constructor(o?: SendView, webVaultUrl?: string) {
if (o == null) {
Expand All @@ -116,8 +118,10 @@ export class SendResponse implements BaseResponse {
this.deletionDate = o.deletionDate;
this.expirationDate = o.expirationDate;
this.passwordSet = o.password != null;
this.emails = o.emails ?? [];
Copy link
Contributor

@claude claude bot Dec 29, 2025

Choose a reason for hiding this comment

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

❌ CRITICAL: Missing authType assignment in constructor causes data loss

Details and fix

The constructor (lines 98-132) creates a SendResponse from a SendView but doesn't copy the authType field. This is added at line 124, but the corresponding field in the toView static method (line 36-58) is also missing authType assignment.

Current state:

  • Line 124: this.authType = o.authType; ✓ (constructor copies it)
  • Line 55: view.emails = send.emails ?? [];
  • Line 58: Missing view.authType = send.authType;

Fix needed in toView method around line 57:

view.emails = send.emails ?? [];
view.disabled = send.disabled;
view.hideEmail = send.hideEmail;
view.authType = send.authType;  // Add this line
return view;

Impact: The toView method is used in create.command.ts line 147. Without copying authType, sends lose their authentication type during the create flow.

this.disabled = o.disabled;
this.hideEmail = o.hideEmail;
this.authType = o.authType;

if (o.type === SendType.Text && o.text != null) {
this.text = new SendTextResponse(o.text);
Expand Down
5 changes: 2 additions & 3 deletions apps/cli/src/tools/send/send.program.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,9 +59,7 @@ export class SendProgram extends BaseProgram {
new Option(
"--email <email>",
"optional emails to access this Send. Can also be specified in JSON.",
)
.argParser(parseEmail)
.hideHelp(),
).argParser(parseEmail),
)
.option("-a, --maxAccessCount <amount>", "The amount of max possible accesses.")
.option("--hidden", "Hide <data> in web by default. Valid only if --file is not set.")
Expand Down Expand Up @@ -328,6 +326,7 @@ export class SendProgram extends BaseProgram {
file: sendFile,
text: sendText,
type: type,
emails: options.email ?? undefined,
});

return Buffer.from(JSON.stringify(template), "utf8").toString("base64");
Expand Down
4 changes: 4 additions & 0 deletions libs/common/src/tools/send/models/data/send.data.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
// FIXME: Update this file to be type safe and remove this and next line
// @ts-strict-ignore
import { AuthType } from "@bitwarden/common/tools/send/models/domain/send";

import { SendType } from "../../enums/send-type";
import { SendResponse } from "../response/send.response";

Expand All @@ -24,6 +26,7 @@ export class SendData {
emails: string;
disabled: boolean;
hideEmail: boolean;
authType: AuthType;

constructor(response?: SendResponse) {
if (response == null) {
Expand All @@ -45,6 +48,7 @@ export class SendData {
this.emails = response.emails;
this.disabled = response.disable;
this.hideEmail = response.hideEmail;
this.authType = response.authType;

switch (this.type) {
case SendType.Text:
Expand Down
6 changes: 5 additions & 1 deletion libs/common/src/tools/send/models/domain/send.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import { UserKey } from "../../../../types/key";
import { SendType } from "../../enums/send-type";
import { SendData } from "../data/send.data";

import { Send } from "./send";
import { AuthType, Send } from "./send";
import { SendText } from "./send-text";

describe("Send", () => {
Expand Down Expand Up @@ -42,6 +42,7 @@ describe("Send", () => {
emails: null!,
disabled: false,
hideEmail: true,
authType: AuthType.None,
};

mockContainerService();
Expand Down Expand Up @@ -94,6 +95,7 @@ describe("Send", () => {
emails: null!,
disabled: false,
hideEmail: true,
authType: AuthType.None,
});
});

Expand All @@ -118,6 +120,7 @@ describe("Send", () => {
send.password = "password";
send.disabled = false;
send.hideEmail = true;
send.authType = AuthType.None;

const encryptService = mock<EncryptService>();
const keyService = mock<KeyService>();
Expand Down Expand Up @@ -157,6 +160,7 @@ describe("Send", () => {
password: "password",
disabled: false,
hideEmail: true,
authType: AuthType.None,
});
});
});
10 changes: 10 additions & 0 deletions libs/common/src/tools/send/models/domain/send.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,14 @@ import { SendView } from "../view/send.view";
import { SendFile } from "./send-file";
import { SendText } from "./send-text";

export const AuthType = Object.freeze({
Email: 0,
Password: 1,
None: 2,
} as const);

export type AuthType = (typeof AuthType)[keyof typeof AuthType];

export class Send extends Domain {
id: string;
accessId: string;
Expand All @@ -33,6 +41,7 @@ export class Send extends Domain {
emails: string;
disabled: boolean;
hideEmail: boolean;
authType: AuthType;

constructor(obj?: SendData) {
super();
Expand Down Expand Up @@ -63,6 +72,7 @@ export class Send extends Domain {
this.deletionDate = obj.deletionDate != null ? new Date(obj.deletionDate) : null;
this.expirationDate = obj.expirationDate != null ? new Date(obj.expirationDate) : null;
this.hideEmail = obj.hideEmail;
this.authType = obj.authType;

switch (this.type) {
case SendType.Text:
Expand Down
4 changes: 4 additions & 0 deletions libs/common/src/tools/send/models/response/send.response.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
// FIXME: Update this file to be type safe and remove this and next line
// @ts-strict-ignore
import { AuthType } from "@bitwarden/common/tools/send/models/domain/send";

import { BaseResponse } from "../../../../models/response/base.response";
import { SendType } from "../../enums/send-type";
import { SendFileApi } from "../api/send-file.api";
Expand All @@ -23,6 +25,7 @@ export class SendResponse extends BaseResponse {
emails: string;
disable: boolean;
hideEmail: boolean;
authType: AuthType;

constructor(response: any) {
super(response);
Expand All @@ -41,6 +44,7 @@ export class SendResponse extends BaseResponse {
this.emails = this.getResponseProperty("Emails");
this.disable = this.getResponseProperty("Disabled") || false;
this.hideEmail = this.getResponseProperty("HideEmail") || false;
this.authType = this.getResponseProperty("AuthType");

const text = this.getResponseProperty("Text");
if (text != null) {
Expand Down
10 changes: 9 additions & 1 deletion libs/common/src/tools/send/models/view/send.view.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { Utils } from "../../../../platform/misc/utils";
import { SymmetricCryptoKey } from "../../../../platform/models/domain/symmetric-crypto-key";
import { DeepJsonify } from "../../../../types/deep-jsonify";
import { SendType } from "../../enums/send-type";
import { Send } from "../domain/send";
import { AuthType, Send } from "../domain/send";

import { SendFileView } from "./send-file.view";
import { SendTextView } from "./send-text.view";
Expand All @@ -29,6 +29,7 @@ export class SendView implements View {
emails: string[] = [];
disabled = false;
hideEmail = false;
authType: AuthType = null;

constructor(s?: Send) {
if (!s) {
Expand All @@ -46,6 +47,13 @@ export class SendView implements View {
this.disabled = s.disabled;
this.password = s.password;
this.hideEmail = s.hideEmail;
this.authType = s.authType;
this.emails = s.emails
? s.emails
.split(",")
.map((e) => e.trim())
.filter((e) => e)
: [];
}

get urlB64Key(): string {
Expand Down
Loading