diff --git a/crates/nuncio-cli/src/args.rs b/crates/nuncio-cli/src/args.rs index 96bba71..77a3314 100644 --- a/crates/nuncio-cli/src/args.rs +++ b/crates/nuncio-cli/src/args.rs @@ -308,6 +308,10 @@ pub enum MailSubcommand { /// Message body text content. #[arg(short, long, help = "Message body text content")] body: String, + /// Identifier of the account to send from (defaults to the first + /// configured account when omitted). + #[arg(short, long, help = "Identifier of the account to send from")] + account: Option, }, /// Full-text search across all cached email messages. Search { diff --git a/crates/nuncio-cli/src/runner.rs b/crates/nuncio-cli/src/runner.rs index 68c34f3..e1fbb93 100644 --- a/crates/nuncio-cli/src/runner.rs +++ b/crates/nuncio-cli/src/runner.rs @@ -322,8 +322,14 @@ impl HeadlessRunner { MailSubcommand::Sync => self.handle_sync(json_mode).await, MailSubcommand::List { folder } => self.handle_list_folder(folder, json_mode).await, MailSubcommand::Read { id } => self.handle_read_message(id, json_mode).await, - MailSubcommand::Send { to, subject, body } => { - self.handle_send_email(to, subject, body, json_mode).await + MailSubcommand::Send { + to, + subject, + body, + account, + } => { + self.handle_send_email(to, subject, body, account.as_deref(), json_mode) + .await } MailSubcommand::Search { query } => self.handle_search(query, json_mode).await, MailSubcommand::Mark { id, read, unread } => { @@ -695,6 +701,7 @@ impl HeadlessRunner { to: &str, subject: &str, body: &str, + account: Option<&str>, json_mode: bool, ) -> String { let mut client = match self.connect_mail_client().await { @@ -710,6 +717,7 @@ impl HeadlessRunner { body_text: body.to_string(), body_html: None, attachments: Vec::new(), + account_id: account.map(str::to_string), }) .await { @@ -3165,6 +3173,7 @@ mod tests { to: "alice@nuncio.mx".to_string(), subject: "Quarterly Roadmap".to_string(), body: "Let's discuss the roadmap.".to_string(), + account: None, }, }, true, @@ -3182,6 +3191,35 @@ mod tests { assert_eq!(recorded_send.to, "alice@nuncio.mx"); assert_eq!(recorded_send.subject, "Quarterly Roadmap"); assert_eq!(recorded_send.body_text, "Let's discuss the roadmap."); + assert_eq!(recorded_send.account_id, None); + + // `mail send --account ` threads the explicit selector through + // to the daemon over the wire, proving the CLI flag is not merely + // parsed but actually reaches `SendMessageRequest.account_id`. + let mail_send_with_account = runner + .execute_command( + &Commands::Mail { + action: MailSubcommand::Send { + to: "alice@nuncio.mx".to_string(), + subject: "Quarterly Roadmap".to_string(), + body: "Let's discuss the roadmap.".to_string(), + account: Some("acct-work".to_string()), + }, + }, + true, + ) + .await; + assert!(mail_send_with_account.contains(r#""sent":true"#)); + + let recorded_send_with_account = send_probe + .lock() + .unwrap_or_else(|e| e.into_inner()) + .clone() + .expect("stub daemon received a second send_message request"); + assert_eq!( + recorded_send_with_account.account_id, + Some("acct-work".to_string()) + ); } /// Reference-client proof: boots a stub `nuncio.v1.Calendar` gRPC server diff --git a/crates/nuncio-proto/proto/nuncio/v1/descriptor.bin b/crates/nuncio-proto/proto/nuncio/v1/descriptor.bin index 2304090..609afaa 100644 Binary files a/crates/nuncio-proto/proto/nuncio/v1/descriptor.bin and b/crates/nuncio-proto/proto/nuncio/v1/descriptor.bin differ diff --git a/crates/nuncio-proto/proto/nuncio/v1/nuncio.proto b/crates/nuncio-proto/proto/nuncio/v1/nuncio.proto index 07eba08..fb96d27 100644 --- a/crates/nuncio-proto/proto/nuncio/v1/nuncio.proto +++ b/crates/nuncio-proto/proto/nuncio/v1/nuncio.proto @@ -321,7 +321,11 @@ service Mail { // SendMessage composes and sends a real outbound email over SMTP, built // from the configured account's SMTP endpoint // (`AccountConfig.smtp_host`/`smtp_port`) and its keyring-stored - // password. Returns successfully ONLY when + // password. When `account_id` is set, that specific account is used as + // the sender, and a request naming an account that does not exist fails + // rather than silently falling back to a different account. When + // `account_id` is omitted, the first configured account is used. Returns + // successfully ONLY when // the SMTP transport genuinely accepted the message for delivery -- // never a fabricated success. A transport/authentication failure surfaces // as a `Status::internal` (or `Status::invalid_argument` for a malformed @@ -469,6 +473,10 @@ message SendMessageRequest { optional string body_html = 5; // Optional file attachments. repeated Attachment attachments = 6; + // Optional identifier of the account to send from. When omitted, the + // first configured account is used (preserving prior behavior); when + // set, the named account must exist or the request fails. + optional string account_id = 7; } // SendMessageResponse. A successful response means the SMTP transport diff --git a/crates/nunciod/src/grpc.rs b/crates/nunciod/src/grpc.rs index 97083d4..f5cb81a 100644 --- a/crates/nunciod/src/grpc.rs +++ b/crates/nunciod/src/grpc.rs @@ -1355,6 +1355,7 @@ impl Mail for MailGrpcService { .into_iter() .map(map_attachment_from_proto) .collect(), + account_id: req.account_id, }; let message_id = if let Some(sender) = &self.overrides.message_sender { @@ -3295,6 +3296,7 @@ mod tests { body_text: "Body".to_string(), body_html: None, attachments: Vec::new(), + account_id: None, }) .await .expect_err("missing bearer token must be rejected"); @@ -3552,6 +3554,7 @@ mod tests { body_text: "Let's discuss the roadmap.".to_string(), body_html: None, attachments: Vec::new(), + account_id: None, } } @@ -3839,6 +3842,7 @@ mod tests { body_text: "Injected send body".to_string(), body_html: None, attachments: Vec::new(), + account_id: None, })) .await .expect("send_message succeeds") diff --git a/crates/nunciod/src/send.rs b/crates/nunciod/src/send.rs index afcd079..995b68a 100644 --- a/crates/nunciod/src/send.rs +++ b/crates/nunciod/src/send.rs @@ -12,13 +12,13 @@ //! //! # Account selection //! -//! The `nuncio.v1.Mail` service (like every other RPC on it -- -//! `ListMessages`, `GetMessage`, `SearchMessages`, etc.) does not yet -//! disambiguate between multiple configured accounts; it operates over a -//! single, global store. Consistent with that existing scope, `SendMessage` -//! sends from the first account returned by [`DatabaseEngine::list_accounts`]. -//! Supporting an explicit multi-account choice is future work, alongside -//! the rest of the `Mail` service's single-account scope. +//! `SendMessage` accepts an optional `account_id`. When the caller supplies +//! one, that specific account is resolved and used as the sender -- a +//! request naming an account that does not exist fails honestly rather than +//! silently falling back to a different identity. When `account_id` is +//! omitted, the account resolves to the first one returned by +//! [`DatabaseEngine::list_accounts`], preserving behavior for callers that +//! predate explicit selection. //! //! # Testability //! @@ -61,6 +61,13 @@ pub enum SendError { /// No account is configured to send mail from. #[error("no account is configured to send mail from")] NoAccountConfigured, + /// The caller specified an `account_id` that does not match any + /// configured account. Distinct from [`SendError::NoAccountConfigured`] + /// (which means zero accounts exist at all): this means at least one + /// account exists, just not the one requested, so the request must fail + /// rather than silently sending from a different account. + #[error("no account configured with id {0:?}")] + AccountNotFound(String), /// Failed to read the account's credential from the secret vault. #[error("failed to read account credential from the secret vault: {0}")] Vault(#[from] VaultError), @@ -90,6 +97,9 @@ pub struct ComposeRequest { pub body_html: Option, /// File attachments. pub attachments: Vec, + /// Identifier of the account to send from. `None` preserves the prior + /// "first configured account" behavior. + pub account_id: Option, } /// Send `message` through `sender`. Pure seam shared by production and @@ -121,14 +131,24 @@ fn build_smtp_sender( } /// Resolves the account to send mail from. See the module-level "Account -/// selection" doc above for why this is currently the first configured -/// account rather than an explicit per-request choice. -async fn resolve_sending_account(db: &DatabaseEngine) -> Result { +/// selection" doc above: an explicit `account_id` must match a configured +/// account or the request fails; `None` preserves the "first configured +/// account" behavior. +async fn resolve_sending_account( + db: &DatabaseEngine, + account_id: Option<&str>, +) -> Result { let accounts = db.list_accounts().await?; - accounts - .into_iter() - .next() - .ok_or(SendError::NoAccountConfigured) + match account_id { + Some(id) => accounts + .into_iter() + .find(|a| a.id == id) + .ok_or_else(|| SendError::AccountNotFound(id.to_string())), + None => accounts + .into_iter() + .next() + .ok_or(SendError::NoAccountConfigured), + } } /// Generates a daemon-local identifier for a sent message, for reference in @@ -154,7 +174,7 @@ pub async fn send_message_for_account( secrets: &SecretManager, request: ComposeRequest, ) -> Result { - let config = resolve_sending_account(db).await?; + let config = resolve_sending_account(db, request.account_id.as_deref()).await?; let password = secrets.get_secret(&config.keyring_secret_key)?; let sender = build_smtp_sender(&config, &password)?; @@ -186,7 +206,7 @@ pub async fn send_message_with_injected_sender( sender: &dyn MessageSender, request: ComposeRequest, ) -> Result { - let config = resolve_sending_account(db).await?; + let config = resolve_sending_account(db, request.account_id.as_deref()).await?; let outbound = OutboundMessage { from: config.email_address, @@ -286,6 +306,7 @@ mod tests { body_text: Some("Body".to_string()), body_html: None, attachments: Vec::new(), + account_id: None, }, ) .await @@ -314,6 +335,7 @@ mod tests { body_text: Some("Body".to_string()), body_html: None, attachments: Vec::new(), + account_id: None, }, ) .await @@ -352,6 +374,7 @@ mod tests { body_text: Some("Body".to_string()), body_html: None, attachments: Vec::new(), + account_id: None, }, ) .await @@ -392,6 +415,7 @@ mod tests { body_text: Some("Body via injected sender.".to_string()), body_html: None, attachments: Vec::new(), + account_id: None, }, ) .await @@ -423,6 +447,7 @@ mod tests { body_text: Some("Body".to_string()), body_html: None, attachments: Vec::new(), + account_id: None, }, ) .await @@ -430,4 +455,75 @@ mod tests { assert!(matches!(err, SendError::NoAccountConfigured)); assert!(mock.sent_messages().is_empty()); } + + /// Proves that with multiple configured accounts, an explicit + /// `account_id` selects that SPECIFIC account as the sender -- not + /// whichever account happens to be first -- by asserting the captured + /// outbound message's `From:` matches the requested account. + #[tokio::test] + async fn send_message_with_injected_sender_uses_the_explicitly_selected_account() { + let (db, _dir) = DatabaseEngine::connect_ephemeral() + .await + .expect("ephemeral db"); + + let first = sample_account("acct-first"); + let second = sample_account("acct-second"); + db.save_account(&first).await.expect("save first account"); + db.save_account(&second).await.expect("save second account"); + + let mock = MockMessageSender::new(); + send_message_with_injected_sender( + &db, + &mock, + ComposeRequest { + to: "bob@nuncio.mx".to_string(), + cc: None, + subject: "Hi".to_string(), + body_text: Some("Body".to_string()), + body_html: None, + attachments: Vec::new(), + account_id: Some(second.id.clone()), + }, + ) + .await + .expect("send succeeds via explicitly selected account"); + + let sent = mock.sent_messages(); + assert_eq!(sent.len(), 1); + assert_eq!(sent[0].from, second.email_address); + assert_ne!(sent[0].from, first.email_address); + } + + /// An `account_id` naming an account that does not exist must fail with + /// the specific [`SendError::AccountNotFound`] variant -- never a silent + /// fallback to the first configured account, and never confusable with + /// [`SendError::NoAccountConfigured`] (which means zero accounts exist). + #[tokio::test] + async fn send_message_with_injected_sender_rejects_unknown_account_id() { + let (db, _dir) = DatabaseEngine::connect_ephemeral() + .await + .expect("ephemeral db"); + + let config = sample_account("acct-only"); + db.save_account(&config).await.expect("save account"); + + let mock = MockMessageSender::new(); + let err = send_message_with_injected_sender( + &db, + &mock, + ComposeRequest { + to: "bob@nuncio.mx".to_string(), + cc: None, + subject: "Hi".to_string(), + body_text: Some("Body".to_string()), + body_html: None, + attachments: Vec::new(), + account_id: Some("acct-does-not-exist".to_string()), + }, + ) + .await + .expect_err("unknown account id must fail"); + assert!(matches!(err, SendError::AccountNotFound(id) if id == "acct-does-not-exist")); + assert!(mock.sent_messages().is_empty()); + } } diff --git a/crates/nunciod/tests/spine_e2e_test.rs b/crates/nunciod/tests/spine_e2e_test.rs index f4d6253..4a2c844 100644 --- a/crates/nunciod/tests/spine_e2e_test.rs +++ b/crates/nunciod/tests/spine_e2e_test.rs @@ -305,6 +305,7 @@ async fn full_mail_spine_round_trips_over_authenticated_grpc_with_mocked_protoco body_text: "Let's discuss the roadmap.".to_string(), body_html: None, attachments: Vec::new(), + account_id: None, }) .await .expect("send_message succeeds")