diff --git a/CHANGELOG.md b/CHANGELOG.md index 36a1428..e11f68b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ ### Added - List commands without `--all-pages` report whether the result is complete: JSON `metadata.has_more`, plus `metadata.next_link` (Graph's `@odata.nextLink`) when more pages exist. Previously a truncated page looked identical to a complete collection. With `--all-pages`, `has_more` is false. `data` is unchanged. Human and plain output print "More results available; use --all-pages" on stderr. +- `teams message attachments delete (--chat C | --team T --channel CH [--reply R]) --message M [--index N] --yes` deletes files attached with `message send --attach` from the sender's OneDrive. Each `reference` attachment is resolved through Graph's shares API and deleted only when it is in the caller's own drive; inline images, other people's files and channel files are skipped with a reason. A file already gone counts as deleted. The message and its file card are left as they are; `message delete` is separate. ### Changed @@ -15,7 +16,6 @@ - `teams team update` and `teams channel update` no longer fail with "Failed to parse API response" (exit 1) after a successful update. Graph answers these with 204 No Content; every PATCH-based update now reads the resource back when the response has no body, so the output is unchanged. If that read fails, the error says the update succeeded (exit 1). - A POST is no longer replayed after a 5xx or a network failure once sent, which could post a message twice. It fails with exit code 8 or 7 and a hint to check before re-sending. A 429 or a connection that was never made is still retried, and read-only POSTs (`search`, `presence get-batch`) retry as before. - A POST answered 2xx whose response body cannot be read or parsed now says Graph performed it and not to re-send, instead of a bare network or parse error. - - `teams auth status` reports `user` and `tenant_id` at the top level, as `auth list` does, not only inside `token_diagnostics`. A token store that cannot be read is now `KEYRING_ERROR` (exit code 10) instead of `authenticated: false`, and a signed-out profile says why in `reason`. - A token store that cannot be read (a locked keychain, a denied prompt, an SSH session) is reported as `KEYRING_ERROR` (exit code 10) with its cause, instead of "Not authenticated" (exit code 3). Only a profile with no stored token is "Not authenticated". - A profile index that cannot be read or parsed is an error (exit code 10) instead of an empty list. `auth login` no longer rewrites it to hold only the new profile, orphaning the rest; `auth list` and `auth logout --all` no longer report no profiles as a success. @@ -23,6 +23,8 @@ - `teams config path` ignored `--config` and always reported the default path. It now reports the file in use, with `source` set to `flag` or `default`. - Config saves (`config set`, `config init`, `auth switch`) truncated the file and wrote it in place, so a failed write could leave a partial config. They now write a temporary file and rename it over the original, as `auth login` already did. - A new profile's name is limited to 1-64 of `A-Z a-z 0-9 . _ -`, not `.` or `..`. `TEAMS_CLI_TOKEN_STORE=file` maps `:`, `/` and `\` to `.` in token file names, so `customer/a` and `customer.a` shared one token and could run as the wrong account. `auth login`, `config set profiles....` and `config set default.profile` refuse another name as invalid input (exit code 2). Profiles already in the config or the token index keep working under their names. A new name whose token file name matches an existing profile's, ignoring case, is refused too. +- Attachment deletion also rejects `remoteItem` references, including local shortcuts into another drive. +- `message attachments download` and `file download --path` write through a uniquely named, exclusively created temp file in the target directory, then rename it into place. Attachments used a fixed `.part`, so a symlink there redirected the write, a stray `.part` was clobbered, and concurrent downloads to one target shared a temp file; `file download` wrote in place, through any symlink at the target, and left a truncated file on failure; a symlink at the target is now replaced. A failed write now leaves no temp file. An existing target is still overwritten and keeps its mode, and `message attachments download --path -` still streams to stdout. ## v0.9.0 - 2026-10-04 diff --git a/CLAUDE.md b/CLAUDE.md index 2b06f12..4611c1e 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -24,7 +24,7 @@ Single-crate Rust binary. Key modules: - `auth.rs` — login (PKCE, device code, client credentials), status, list, switch, logout, token - `team.rs` — list, get, create, update, delete, clone, archive, unarchive, members - `channel.rs` — list, get, create, update, delete, members - - `message.rs` — send, list, get, reply, update, delete, undelete, react, unreact, pin, unpin + - `message.rs` — send, list, get, reply, update, delete, undelete, react, unreact, pin, unpin; attachments list/download/delete (delete requires --yes and skips remote items) - `chat.rs` — list, get, create, hide, unhide, members - `presence.rs` — get, set, set-preferred, clear, clear-preferred, status, get-batch - `search.rs` — messages, users, teams diff --git a/Cargo.toml b/Cargo.toml index e67bd14..2cbdb93 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -46,10 +46,10 @@ anyhow = "1" urlencoding = "2" bytes = "1" mime_guess = "2" +tempfile = "3" [dev-dependencies] wiremock = "0.6" -tempfile = "3" assert_cmd = "2" predicates = "3" diff --git a/README.md b/README.md index 33a1730..9298321 100644 --- a/README.md +++ b/README.md @@ -395,6 +395,7 @@ teams message list --team --channel teams message list --team --channel --message-id --all-pages teams message list --chat teams message get --team --channel --message +teams message attachments delete --chat --message [--index N] --yes teams message reply --team --channel --message --mention --body "Thanks!" teams message update --team --channel --message --body "Corrected" teams message update --chat --message --body "Corrected" @@ -419,6 +420,12 @@ confirmed to match the sender's; other recipients use email. Lookup or sharing failures warn on stderr and allow upload/send to continue. Members without a usable address are reported for manual sharing from OneDrive. +`message attachments delete ... --yes` removes those uploads from your OneDrive +(`--index N` for one item, numbered as in `attachments list`). Only files in +your own OneDrive are deleted; inline images, files shared by others and +channel files are skipped with a reason. The message and its file card stay; +use `message delete` for the message. + The `--mention USER` flag on `message send` and `message reply` tags a person as a real Teams @mention — the kind that pings them, not literal `@Name` text. The flag is repeatable and `USER` may be an Entra object ID or UPN; the display name is resolved through Microsoft Graph. It works for chat sends, diff --git a/docs/attachments-spec.md b/docs/attachments-spec.md index f8c9401..db682c5 100644 --- a/docs/attachments-spec.md +++ b/docs/attachments-spec.md @@ -269,7 +269,8 @@ Ordered, each step compiles and passes tests independently. - 404 on hostedContent (expired/deleted) → exit 5, name the index and message ID. - Partial failures download what they can, then exit non-zero with an error that names the failed item and lists the paths already saved. -- Never write partial files: download to `{path}.part`, rename on success. +- Never write partial files: download to a uniquely named, exclusively created temp file + beside the target, rename on success. ### Security notes diff --git a/docs/command-reference.md b/docs/command-reference.md index 0832240..7053881 100644 --- a/docs/command-reference.md +++ b/docs/command-reference.md @@ -111,6 +111,7 @@ teams message list (--team TEAM_ID --channel CHANNEL_ID [--message-id ROOT_MESSA teams message get --team TEAM_ID --channel CHANNEL_ID (MESSAGE_ID | --message MESSAGE_ID) [--with-attachments] teams message attachments list (--team TEAM_ID --channel CHANNEL_ID [--reply REPLY_ID] | --chat CHAT_ID) (MESSAGE_ID | --message MESSAGE_ID) teams message attachments download (--team TEAM_ID --channel CHANNEL_ID [--reply REPLY_ID] | --chat CHAT_ID) (MESSAGE_ID | --message MESSAGE_ID) [--index N] [--dir DIR | --path FILE] +teams message attachments delete (--team TEAM_ID --channel CHANNEL_ID [--reply REPLY_ID] | --chat CHAT_ID) (MESSAGE_ID | --message MESSAGE_ID) [--index N] --yes teams message reply --team TEAM_ID --channel CHANNEL_ID --message-id MESSAGE_ID [--body TEXT | --stdin] [--content-type text|html] [--image PATH]... [--attach PATH]... [--mention USER]... teams message update (--team TEAM_ID --channel CHANNEL_ID | --chat CHAT_ID) (MESSAGE_ID | --message MESSAGE_ID) --body TEXT [--content-type text|html] teams message delete (--team TEAM_ID --channel CHANNEL_ID [--reply REPLY_ID] | --chat CHAT_ID) (MESSAGE_ID | --message MESSAGE_ID) --yes @@ -172,6 +173,8 @@ teams message send --chat CHAT_ID --quote MESSAGE_ID --body "Yes, that works." `message attachments` unifies the two ways Teams stores message media: inline images pasted into the compose box (Graph "hosted contents") and files attached via SharePoint/OneDrive (`reference` attachments). `list` returns an indexed inventory; `download` fetches everything downloadable by default, or one item with `--index` (add `--path FILE` for an exact destination, or `--path -` to stream to stdout). Inline images and code snippets need no scopes beyond message reads; file attachments additionally require the `Files.Read.All` delegated scope. `message get --with-attachments` embeds the same inventory under `attachment_items` in the message output. +`message attachments delete` removes files you attached (`message send --attach`) from your OneDrive. Each selected `reference` item is resolved through Graph's `/shares/{token}/driveItem` and deleted only when it is a file in your own drive (`/me/drive`); inline images, code snippets, cards, files shared by others and channel files are skipped with a `reason`, and so is a file Graph cannot find or let you see. Without `--index` every item is considered; `--index N` uses the numbering of `attachments list`. Output lists `{index, name, deleted, reason?}` per item; a file already gone counts as deleted (`reason: "already deleted"`). A Graph failure on a delete is an error, naming items deleted before it. Needs `Files.ReadWrite`, and `--yes` (without it the command exits with code 2 and sends nothing). The message itself is not changed: its file card remains, now pointing at a missing file. Use `message delete` to remove the message. + ## Chats ```bash diff --git a/docs/man/teams.1 b/docs/man/teams.1 index 9799c26..7d44f5a 100644 --- a/docs/man/teams.1 +++ b/docs/man/teams.1 @@ -168,6 +168,7 @@ teams message list (--team TEAM_ID --channel CHANNEL_ID [--message-id ROOT_MESSA teams message get --team TEAM_ID --channel CHANNEL_ID (MESSAGE_ID | --message MESSAGE_ID) [--with-attachments] teams message attachments list (--team TEAM_ID --channel CHANNEL_ID [--reply REPLY_ID] | --chat CHAT_ID) (MESSAGE_ID | --message MESSAGE_ID) teams message attachments download (--team TEAM_ID --channel CHANNEL_ID [--reply REPLY_ID] | --chat CHAT_ID) (MESSAGE_ID | --message MESSAGE_ID) [--index N] [--dir DIR | --path FILE] +teams message attachments delete (--team TEAM_ID --channel CHANNEL_ID [--reply REPLY_ID] | --chat CHAT_ID) (MESSAGE_ID | --message MESSAGE_ID) [--index N] --yes teams message reply --team TEAM_ID --channel CHANNEL_ID --message-id MESSAGE_ID [--body TEXT | --stdin] [--content-type text|html] [--image PATH]... [--attach PATH]... [--mention USER]... teams message reply --team TEAM_ID --channel CHANNEL_ID --message MESSAGE_ID --body TEXT teams message update (--team TEAM_ID --channel CHANNEL_ID | --chat CHAT_ID) (MESSAGE_ID | --message MESSAGE_ID) --body TEXT [--content-type text|html] @@ -252,6 +253,24 @@ the sender and recipient share a tenant; other recipients use email. Lookup failures, members without a usable address, and grant failures warn on stderr with manual OneDrive sharing guidance. Lookup or sharing failures do not stop upload/send. +.PP +.B message attachments delete +removes files you attached from your OneDrive. Each selected reference +attachment is resolved through /shares/{token}/driveItem and deleted only +when it lives in your own drive (/me/drive). Inline images, code snippets, +cards, files shared by others, channel files and files Graph cannot find +are skipped with a reason. Without +.B --index +every item is considered; +.B --index N +uses the numbering of +.B attachments list. +A file already gone counts as deleted. Needs Files.ReadWrite and +.B --yes +(without it the command exits with code 2). The message and its file card +remain; use +.B message delete +for the message. .SH CHAT COMMANDS .nf teams chat list [--order-by activity] diff --git a/src/api/endpoints.rs b/src/api/endpoints.rs index 67bf487..18500f4 100644 --- a/src/api/endpoints.rs +++ b/src/api/endpoints.rs @@ -324,7 +324,11 @@ pub fn channel_files_folder(team_id: &str, channel_id: &str) -> String { } pub fn drive_item(drive_id: &str, item_id: &str) -> String { - format!("{GRAPH_V1}/drives/{drive_id}/items/{item_id}") + drive_item_at(GRAPH_V1, drive_id, item_id) +} + +pub fn drive_item_at(base: &str, drive_id: &str, item_id: &str) -> String { + format!("{base}/drives/{drive_id}/items/{item_id}") } pub fn drive_item_children(drive_id: &str, item_id: &str) -> String { @@ -350,6 +354,10 @@ pub fn drive_upload_to_folder_rename(drive_id: &str, parent_id: &str, filename: ) } +pub fn me_drive_at(base: &str) -> String { + format!("{base}/me/drive") +} + pub fn me_drive_root_children() -> String { format!("{GRAPH_V1}/me/drive/root/children") } @@ -456,6 +464,10 @@ pub fn shares_drive_item_content(sharing_token: &str) -> String { format!("{GRAPH_V1}/shares/{sharing_token}/driveItem/content") } +pub fn shares_drive_item_at(base: &str, sharing_token: &str) -> String { + format!("{base}/shares/{sharing_token}/driveItem") +} + #[cfg(test)] mod tests { use super::*; @@ -489,6 +501,23 @@ mod tests { ); } + #[test] + fn attachment_delete_urls() { + let url = "https://tenant-my.sharepoint.com/personal/user/Documents/Microsoft%20Teams%20Chat%20Files/NetskopeLogs.zip"; + assert_eq!( + shares_drive_item_at(GRAPH_V1, &sharing_url_token(url)), + "https://graph.microsoft.com/v1.0/shares/u!aHR0cHM6Ly90ZW5hbnQtbXkuc2hhcmVwb2ludC5jb20vcGVyc29uYWwvdXNlci9Eb2N1bWVudHMvTWljcm9zb2Z0JTIwVGVhbXMlMjBDaGF0JTIwRmlsZXMvTmV0c2tvcGVMb2dzLnppcA/driveItem" + ); + assert_eq!( + me_drive_at(GRAPH_V1), + "https://graph.microsoft.com/v1.0/me/drive" + ); + assert_eq!( + drive_item("d1", "i1"), + "https://graph.microsoft.com/v1.0/drives/d1/items/i1" + ); + } + #[test] fn hosted_content_value_encodes_id() { // Base64 IDs can contain '+', '/', and '=' which must be percent-encoded in a path. diff --git a/src/api/files.rs b/src/api/files.rs index 2c6ec84..bc65c03 100644 --- a/src/api/files.rs +++ b/src/api/files.rs @@ -1,8 +1,8 @@ use crate::error::{Result, TeamsError}; use crate::models::common::{Page, PageResponse}; use crate::models::file::{ - DriveInviteRequest, DriveItem, DrivePermission, DriveRecipient, FilesFolder, ShareLinkRequest, - ShareLinkResponse, + Drive, DriveInviteRequest, DriveItem, DrivePermission, DriveRecipient, FilesFolder, + ShareLinkRequest, ShareLinkResponse, }; use super::client::{GraphClient, PaginationOpts}; @@ -317,6 +317,59 @@ pub async fn delete_file( .await } +/// ID of the signed-in user's OneDrive. +pub async fn my_drive_id_at(client: &GraphClient, base: &str) -> Result { + let drive: Drive = client + .get(&endpoints::me_drive_at(base), &[]) + .await + .map_err(delete_scope_hint)?; + drive.id.ok_or_else(|| TeamsError::ApiError { + status: 500, + message: "/me/drive missing id".to_string(), + }) +} + +/// Resolve a shared web URL (an attachment's `contentUrl`) to its drive item. +pub async fn shared_drive_item_at( + client: &GraphClient, + base: &str, + content_url: &str, +) -> Result { + let token = endpoints::sharing_url_token(content_url); + client + .get(&endpoints::shares_drive_item_at(base, &token), &[]) + .await +} + +/// Delete an item from a drive. A 404 is returned as `NotFound` unchanged so +/// the caller can treat an item that is already gone as deleted. +pub async fn delete_drive_item_at( + client: &GraphClient, + base: &str, + drive_id: &str, + item_id: &str, +) -> Result<()> { + client + .delete(&endpoints::drive_item_at(base, drive_id, item_id)) + .await + .map_err(|e| match e { + TeamsError::PermissionDenied(_) => delete_scope_hint(e), + other => other, + }) +} + +/// Graph masks a drive the token cannot see as 404, so `/me/drive` failing +/// with 403 or 404 usually means the Files.ReadWrite scope is missing. +fn delete_scope_hint(err: TeamsError) -> TeamsError { + let hint = "\nHint: deleting attachment uploads needs the Files.ReadWrite delegated \ + scope. Add it to your profile's scopes and run `teams auth refresh`."; + match err { + TeamsError::PermissionDenied(msg) => TeamsError::PermissionDenied(format!("{msg}{hint}")), + TeamsError::NotFound(msg) => TeamsError::NotFound(format!("{msg}{hint}")), + other => other, + } +} + /// Grant `recipients` read access to an item in the signed-in user's OneDrive, /// without sending them an email. Returns the permissions that now exist for /// them. Needs the same `Files.ReadWrite` scope as the upload itself. diff --git a/src/cli/file.rs b/src/cli/file.rs index 47661a7..199366e 100644 --- a/src/cli/file.rs +++ b/src/cli/file.rs @@ -207,12 +207,7 @@ pub async fn run( let bytes = api::files::download_file(&client, &team, &channel, &file_id).await?; if let Some(path) = path { - std::fs::write(&path, &bytes).map_err(|e| { - crate::error::TeamsError::InvalidInput(format!( - "Failed to write file '{}': {e}", - path - )) - })?; + super::message_attachments::write_atomically(std::path::Path::new(&path), &bytes)?; if format != OutputFormat::Plain { let start = Instant::now(); let result = serde_json::json!({ diff --git a/src/cli/message.rs b/src/cli/message.rs index 383cd07..868faec 100644 --- a/src/cli/message.rs +++ b/src/cli/message.rs @@ -104,7 +104,7 @@ pub enum MessageCommand { #[arg(long)] with_attachments: bool, }, - /// List or download message attachments and inline images + /// List, download or delete message attachments and inline images Attachments { #[command(subcommand)] command: super::message_attachments::AttachmentsCommand, @@ -353,6 +353,14 @@ pub async fn run( "Deleting a message needs explicit confirmation: add --yes".into(), )); } + if let MessageCommand::Attachments { + command: super::message_attachments::AttachmentsCommand::Delete { yes: false, .. }, + } = &cmd + { + return Err(TeamsError::InvalidInput( + "Deleting attachments needs explicit confirmation: add --yes".into(), + )); + } let token = auth::resolve_token(profile).await?; let client = GraphClient::new(token, &config.network)?; diff --git a/src/cli/message_attachments.rs b/src/cli/message_attachments.rs index 75218fe..cd3684b 100644 --- a/src/cli/message_attachments.rs +++ b/src/cli/message_attachments.rs @@ -4,7 +4,7 @@ use std::time::Instant; use clap::Subcommand; use crate::api::messages::MessageRef; -use crate::api::{self, GraphClient}; +use crate::api::{self, endpoints, GraphClient}; use crate::error::{Result, TeamsError}; use crate::models::attachment_inventory::{build_inventory, AttachmentItem, ItemKind}; use crate::output::{self, OutputFormat}; @@ -74,6 +74,41 @@ pub enum AttachmentsCommand { #[arg(long, requires = "index")] path: Option, }, + /// Delete files you attached from your OneDrive (the message itself is kept) + #[command( + override_usage = "teams message attachments delete (--team --channel [--reply ] | --chat ) (MESSAGE_ID | --message ) [--index ] --yes" + )] + Delete { + /// Team ID (for channel messages; requires --channel) + #[arg(long, requires = "channel", conflicts_with = "chat")] + team: Option, + /// Channel ID (for channel messages; requires --team) + #[arg(long, requires = "team", conflicts_with = "chat")] + channel: Option, + /// Chat ID (for chat messages) + #[arg(long, required_unless_present = "team")] + chat: Option, + /// Reply ID (for a reply within a channel thread) + #[arg(long, requires = "team", conflicts_with = "chat")] + reply: Option, + /// Message ID + #[arg(required_unless_present = "message", conflicts_with = "message")] + message_id: Option, + /// Message ID + #[arg( + long = "message", + alias = "message-id", + required_unless_present = "message_id", + conflicts_with = "message_id" + )] + message: Option, + /// Delete only the item with this index (from `attachments list`) + #[arg(long)] + index: Option, + /// Confirm the deletion; without it the command exits with code 2 and sends nothing + #[arg(long)] + yes: bool, + }, } pub async fn run( @@ -177,6 +212,173 @@ pub async fn run( output::print_success(format, &result, start); Ok(()) } + + AttachmentsCommand::Delete { + team, + channel, + chat, + reply, + message_id, + message, + index, + yes: _, + } => { + let start = Instant::now(); + let message_id = + super::message::resolve_id(message_id, message, "--message or ")?; + let message_ref = resolve_message_ref(team, channel, chat, reply, message_id.clone())?; + let items = fetch_inventory(client, &message_ref).await?; + let selected = select_for_delete(&items, index)?; + let results = delete_items_at(client, endpoints::GRAPH_V1, &selected).await?; + let result = serde_json::json!({ + "message_id": message_id, + "items": results, + }); + output::print_success(format, &result, start); + Ok(()) + } + } +} + +/// Like `select_items`, but every kind is selectable: items that are not +/// uploads are reported as skipped rather than refused. +fn select_for_delete( + items: &[AttachmentItem], + index: Option, +) -> Result> { + match index { + Some(i) => { + let item = items.iter().find(|it| it.index == i).ok_or_else(|| { + TeamsError::InvalidInput(format!( + "No attachment item with index {i}; the message has {} item(s). \ + Run `teams message attachments list` to see them.", + items.len() + )) + })?; + Ok(vec![item]) + } + None if items.is_empty() => Err(TeamsError::NotFound( + "Message has no attachments or inline images".into(), + )), + None => Ok(items.iter().collect()), + } +} + +/// Delete each selected file attachment that lives in the caller's own +/// OneDrive. Everything else is skipped with a reason. Stops at the first +/// Graph failure, noting what was already deleted. +async fn delete_items_at( + client: &GraphClient, + base: &str, + items: &[&AttachmentItem], +) -> Result> { + let mut my_drive: Option = None; + let mut results = Vec::new(); + for item in items { + match delete_item_at(client, base, item, &mut my_drive).await { + Ok(r) => results.push(r), + Err(e) => { + let deleted: Vec = results + .iter() + .filter(|r| r["deleted"] == true) + .map(|r| r["index"].to_string()) + .collect(); + if deleted.is_empty() { + return Err(e); + } + let note = format!( + " Deleted before the failure: item(s) {}.", + deleted.join(", ") + ); + // Rate-limit and network errors carry no message to extend. + tracing::warn!("{}", note.trim()); + return Err(match e { + TeamsError::ServerError { status, message } => TeamsError::ServerError { + status, + message: format!("{message}{note}"), + }, + TeamsError::AuthError(m) => TeamsError::AuthError(format!("{m}{note}")), + other => augment_error(other, ¬e), + }); + } + } + } + Ok(results) +} + +async fn delete_item_at( + client: &GraphClient, + base: &str, + item: &AttachmentItem, + my_drive: &mut Option, +) -> Result { + let outcome = |deleted: bool, reason: Option<&str>| { + let mut v = serde_json::json!({ + "index": item.index, + "name": item.name, + "deleted": deleted, + }); + if let Some(reason) = reason { + v["reason"] = reason.into(); + } + v + }; + + if item.kind != ItemKind::FileReference { + let reason = match item.kind { + ItemKind::HostedContent => "inline image, stored in the message, not an upload", + ItemKind::CodeSnippet => "code snippet, stored in the message, not an upload", + _ => "not a file attachment", + }; + return Ok(outcome(false, Some(reason))); + } + let Some(url) = item.content_url.as_deref() else { + return Ok(outcome(false, Some("file attachment has no contentUrl"))); + }; + + let drive_item = match api::files::shared_drive_item_at(client, base, url).await { + Ok(d) => d, + // Graph masks items the caller cannot see as 404, so a missing item + // cannot be told apart from someone else's file: never claim it deleted. + Err(TeamsError::NotFound(_)) | Err(TeamsError::PermissionDenied(_)) => { + return Ok(outcome( + false, + Some("not found or not accessible (already deleted, or not your file)"), + )); + } + Err(e) => return Err(e), + }; + + if drive_item.remote_item.is_some() { + return Ok(outcome(false, Some("shared reference to another drive"))); + } + + let my_drive_id = match my_drive { + Some(id) => id.as_str(), + None => my_drive.insert(api::files::my_drive_id_at(client, base).await?), + }; + let item_drive = drive_item + .parent_reference + .as_ref() + .and_then(|r| r.drive_id.as_deref()); + if item_drive != Some(my_drive_id) { + return Ok(outcome( + false, + Some("not in your OneDrive (shared by someone else, or a channel file)"), + )); + } + // A folder delete is recursive; attachments are files. + if drive_item.file.is_none() { + return Ok(outcome(false, Some("not a file"))); + } + let Some(item_id) = drive_item.id.as_deref() else { + return Ok(outcome(false, Some("drive item has no id"))); + }; + + match api::files::delete_drive_item_at(client, base, my_drive_id, item_id).await { + Ok(()) => Ok(outcome(true, None)), + Err(TeamsError::NotFound(_)) => Ok(outcome(true, Some("already deleted"))), + Err(e) => Err(e), } } @@ -294,6 +496,16 @@ async fn download_item( None => unique_path(dir.join(default_filename(message_id, item, content_type.as_deref()))), }; + if let Some(parent) = target.parent() { + if !parent.as_os_str().is_empty() && !parent.exists() { + std::fs::create_dir_all(parent).map_err(|e| { + TeamsError::InvalidInput(format!( + "Failed to create directory '{}': {e}", + parent.display() + )) + })?; + } + } write_atomically(&target, &bytes)?; Ok(serde_json::json!({ @@ -376,29 +588,44 @@ fn unique_path(candidate: PathBuf) -> PathBuf { .expect("unbounded suffix search always terminates") } -/// Write to `.part` then rename, so a failed download never leaves a -/// truncated file at the final path. -fn write_atomically(target: &Path, bytes: &[u8]) -> Result<()> { - if let Some(parent) = target.parent() { - if !parent.as_os_str().is_empty() && !parent.exists() { - std::fs::create_dir_all(parent).map_err(|e| { - TeamsError::InvalidInput(format!( - "Failed to create directory '{}': {e}", - parent.display() - )) - })?; - } +/// Write to a uniquely named, exclusively created temp file beside `target`, +/// then rename over it. A failed download leaves nothing behind, and an +/// existing file or symlink at a temp path is never followed or clobbered. +pub(crate) fn write_atomically(target: &Path, bytes: &[u8]) -> Result<()> { + use std::io::Write; + + let fail = |e: std::io::Error| { + TeamsError::InvalidInput(format!("Failed to write '{}': {e}", target.display())) + }; + if target.file_name().is_none() { + return Err(TeamsError::InvalidInput( + "Target path has no filename".into(), + )); + } + let dir = match target.parent() { + Some(p) if !p.as_os_str().is_empty() => p, + _ => Path::new("."), + }; + let mut builder = tempfile::Builder::new(); + builder.prefix(".teams-download-").suffix(".part"); + // Keep an existing target's mode exactly; otherwise 0666 less umask, as a + // plain create would, not tempfile's 0600. + #[cfg(unix)] + let existing = std::fs::metadata(target).ok().map(|m| m.permissions()); + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + let mode = existing.as_ref().map_or(0o666, |p| p.mode() & 0o777); + builder.permissions(std::fs::Permissions::from_mode(mode)); + } + let mut tmp = builder.tempfile_in(dir).map_err(fail)?; + #[cfg(unix)] + if let Some(perms) = existing { + tmp.as_file().set_permissions(perms).map_err(fail)?; } - let filename = target - .file_name() - .ok_or_else(|| TeamsError::InvalidInput("Target path has no filename".into()))?; - let part = target.with_file_name(format!("{}.part", filename.to_string_lossy())); - std::fs::write(&part, bytes).map_err(|e| { - TeamsError::InvalidInput(format!("Failed to write '{}': {e}", part.display())) - })?; - std::fs::rename(&part, target).map_err(|e| { - TeamsError::InvalidInput(format!("Failed to rename to '{}': {e}", target.display())) - }) + tmp.write_all(bytes).map_err(fail)?; + tmp.persist(target).map_err(|e| fail(e.error))?; + Ok(()) } fn augment_error(err: TeamsError, note: &str) -> TeamsError { @@ -461,6 +688,441 @@ mod tests { write_atomically(&no_extension, b"more").unwrap(); assert_eq!(std::fs::read(&no_extension).unwrap(), b"more"); assert!(!dir.path().join("out.part").exists()); + assert_eq!(std::fs::read_dir(dir.path()).unwrap().count(), 2); + } + + #[cfg(unix)] + #[test] + fn write_atomically_ignores_symlink_at_old_part_path() { + let dir = tempfile::tempdir().unwrap(); + let victim = dir.path().join("victim"); + std::fs::write(&victim, b"keep").unwrap(); + let part = dir.path().join("out.png.part"); + std::os::unix::fs::symlink(&victim, &part).unwrap(); + let target = dir.path().join("out.png"); + + write_atomically(&target, b"payload").unwrap(); + + assert_eq!(std::fs::read(&target).unwrap(), b"payload"); + assert_eq!(std::fs::read(&victim).unwrap(), b"keep"); + assert!(std::fs::symlink_metadata(&part) + .unwrap() + .file_type() + .is_symlink()); + } + + #[cfg(unix)] + #[test] + fn write_atomically_keeps_default_file_mode() { + use std::os::unix::fs::PermissionsExt; + let dir = tempfile::tempdir().unwrap(); + let plain = dir.path().join("plain"); + std::fs::write(&plain, b"x").unwrap(); + let target = dir.path().join("out.png"); + + write_atomically(&target, b"payload").unwrap(); + + let mode = |p: &Path| std::fs::metadata(p).unwrap().permissions().mode() & 0o777; + assert_eq!(mode(&target), mode(&plain)); + + std::fs::set_permissions(&target, std::fs::Permissions::from_mode(0o600)).unwrap(); + write_atomically(&target, b"again").unwrap(); + assert_eq!(mode(&target), 0o600); + + // Bits the umask would strip on create survive a replace. + std::fs::set_permissions(&target, std::fs::Permissions::from_mode(0o664)).unwrap(); + write_atomically(&target, b"third").unwrap(); + assert_eq!(mode(&target), 0o664); + } + + #[cfg(unix)] + #[test] + fn write_atomically_accepts_long_filename() { + let dir = tempfile::tempdir().unwrap(); + let target = dir.path().join(format!("{}.bin", "a".repeat(246))); + write_atomically(&target, b"payload").unwrap(); + assert_eq!(std::fs::read(&target).unwrap(), b"payload"); + } + + #[test] + fn write_atomically_leaves_stray_part_file_alone() { + let dir = tempfile::tempdir().unwrap(); + let part = dir.path().join("out.png.part"); + std::fs::write(&part, b"stray").unwrap(); + let target = dir.path().join("out.png"); + + write_atomically(&target, b"payload").unwrap(); + + assert_eq!(std::fs::read(&target).unwrap(), b"payload"); + assert_eq!(std::fs::read(&part).unwrap(), b"stray"); + } + + #[test] + fn write_atomically_failure_leaves_no_temp_file() { + let dir = tempfile::tempdir().unwrap(); + // A directory at the target makes the final rename fail after the write. + let target = dir.path().join("out.png"); + std::fs::create_dir(&target).unwrap(); + + assert!(write_atomically(&target, b"payload").is_err()); + + assert!(target.is_dir()); + let entries: Vec<_> = std::fs::read_dir(dir.path()) + .unwrap() + .map(|e| e.unwrap().file_name()) + .collect(); + assert_eq!(entries, vec![std::ffi::OsString::from("out.png")]); + } + + #[test] + fn write_atomically_concurrent_writers_leave_one_whole_file() { + let dir = tempfile::tempdir().unwrap(); + let target = dir.path().join("out.bin"); + let payloads = [vec![b'a'; 4 << 20], vec![b'b'; 4 << 20]]; + let barrier = std::sync::Barrier::new(payloads.len()); + + std::thread::scope(|s| { + let handles: Vec<_> = payloads + .iter() + .map(|p| { + let (target, barrier) = (&target, &barrier); + s.spawn(move || { + barrier.wait(); + write_atomically(target, p) + }) + }) + .collect(); + let results: Vec<_> = handles.into_iter().map(|h| h.join().unwrap()).collect(); + // Windows can refuse a rename onto a file another rename is replacing + // ("Access is denied"); one writer may lose there. Elsewhere both win. + let failed = results.iter().filter(|r| r.is_err()).count(); + assert!(failed == 0 || (cfg!(windows) && failed == 1), "{results:?}"); + }); + + // Either way the target is one whole payload, never a mix, and the + // losing writer's temp file is gone. + let written = std::fs::read(&target).unwrap(); + assert!(payloads.contains(&written)); + assert_eq!(std::fs::read_dir(dir.path()).unwrap().count(), 1); + } + + mod delete { + use super::*; + use crate::models::message::{ChatMessage, ChatMessageHostedContent}; + use wiremock::matchers::{method, path}; + use wiremock::{Mock, MockServer, ResponseTemplate}; + + const OWN_URL: &str = "https://tenant-my.sharepoint.com/personal/me/Documents/Microsoft%20Teams%20Chat%20Files/report.pdf"; + const FOREIGN_URL: &str = + "https://tenant.sharepoint.com/sites/team/Shared%20Documents/General/plan.docx"; + + fn test_client() -> GraphClient { + GraphClient::new( + crate::auth::token::TokenInfo { + access_token: "synthetic-test-token".into(), + expires_at: None, + token_type: "Bearer".into(), + scope: None, + refresh_token: None, + profile: "test".into(), + }, + &crate::config::NetworkConfig { + timeout: 3, + max_retries: 0, + retry_backoff_base: 1, + }, + ) + .unwrap() + } + + /// Inventory of a message with one inline image (index 0) and file + /// attachments for each of `urls` (index 1..). + fn inventory(urls: &[&str]) -> Vec { + let attachments: Vec = urls + .iter() + .enumerate() + .map(|(i, url)| { + serde_json::json!({ + "id": format!("att{i}"), + "contentType": "reference", + "contentUrl": url, + "name": format!("file{i}"), + }) + }) + .collect(); + let msg: ChatMessage = serde_json::from_value(serde_json::json!({ + "id": "m1", + "body": { + "contentType": "html", + "content": "", + }, + "attachments": attachments, + })) + .unwrap(); + let hosted: Vec = + serde_json::from_value(serde_json::json!([{"id": "aW1n"}])).unwrap(); + build_inventory(&msg, &hosted) + } + + async fn mount_me_drive(server: &MockServer, times: u64) { + Mock::given(method("GET")) + .and(path("/me/drive")) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ + "id": "my-drive", + }))) + .expect(times) + .mount(server) + .await; + } + + async fn mount_share(server: &MockServer, url: &str, drive_id: &str, item_id: &str) { + mount_share_body( + server, + url, + serde_json::json!({ + "id": item_id, + "name": "x", + "file": {"mimeType": "application/pdf"}, + "parentReference": {"driveId": drive_id, "id": "parent"}, + }), + ) + .await; + } + + async fn mount_share_body(server: &MockServer, url: &str, body: serde_json::Value) { + Mock::given(method("GET")) + .and(path(format!( + "/shares/{}/driveItem", + endpoints::sharing_url_token(url) + ))) + .respond_with(ResponseTemplate::new(200).set_body_json(body)) + .expect(1) + .mount(server) + .await; + } + + #[tokio::test] + async fn remote_reference_in_own_drive_is_never_deleted() { + let server = MockServer::start().await; + mount_share_body( + &server, + OWN_URL, + serde_json::json!({ + "id": "local-shortcut", + "file": {}, + "parentReference": {"driveId": "my-drive"}, + "remoteItem": { + "id": "foreign-item", + "file": {}, + "parentReference": {"driveId": "foreign-drive"} + } + }), + ) + .await; + mount_me_drive(&server, 0).await; + mount_delete(&server, "local-shortcut", 204, 0).await; + let items = inventory(&[OWN_URL]); + let selected = select_for_delete(&items, Some(1)).unwrap(); + let results = delete_items_at(&test_client(), &server.uri(), &selected) + .await + .unwrap(); + assert_eq!(results[0]["deleted"], false); + assert_eq!(results[0]["reason"], "shared reference to another drive"); + assert!(server + .received_requests() + .await + .unwrap() + .iter() + .all(|r| r.method.as_str() != "DELETE")); + } + + #[tokio::test] + async fn own_folder_is_never_deleted() { + let server = MockServer::start().await; + mount_me_drive(&server, 1).await; + mount_share_body( + &server, + OWN_URL, + serde_json::json!({ + "id": "folder-item", + "folder": {"childCount": 3}, + "parentReference": {"driveId": "my-drive", "id": "root"}, + }), + ) + .await; + mount_delete(&server, "folder-item", 204, 0).await; + + let items = inventory(&[OWN_URL]); + let selected = select_for_delete(&items, Some(1)).unwrap(); + let results = delete_items_at(&test_client(), &server.uri(), &selected) + .await + .unwrap(); + assert_eq!(results[0]["deleted"], false); + assert_eq!(results[0]["reason"], "not a file"); + } + + #[tokio::test] + async fn server_error_after_a_delete_keeps_the_note_and_exit_code() { + let server = MockServer::start().await; + let second = "https://tenant-my.sharepoint.com/personal/me/Documents/Microsoft%20Teams%20Chat%20Files/c.pdf"; + mount_me_drive(&server, 1).await; + mount_share(&server, OWN_URL, "my-drive", "own-item").await; + mount_share(&server, second, "my-drive", "item-c").await; + mount_delete(&server, "own-item", 204, 1).await; + mount_delete(&server, "item-c", 503, 1).await; + + let items = inventory(&[OWN_URL, second]); + let selected = select_for_delete(&items, None).unwrap(); + let err = delete_items_at(&test_client(), &server.uri(), &selected) + .await + .unwrap_err(); + assert_eq!(err.exit_code(), 8); + assert!(err.to_string().contains("item(s) 1."), "{err}"); + } + + async fn mount_delete(server: &MockServer, item_id: &str, status: u16, times: u64) { + Mock::given(method("DELETE")) + .and(path(format!("/drives/my-drive/items/{item_id}"))) + .respond_with(ResponseTemplate::new(status)) + .expect(times) + .mount(server) + .await; + } + + #[tokio::test] + async fn own_upload_is_deleted_and_others_are_skipped() { + let server = MockServer::start().await; + mount_me_drive(&server, 1).await; + mount_share(&server, OWN_URL, "my-drive", "own-item").await; + mount_share(&server, FOREIGN_URL, "team-drive", "team-item").await; + mount_delete(&server, "own-item", 204, 1).await; + Mock::given(method("DELETE")) + .and(path("/drives/team-drive/items/team-item")) + .respond_with(ResponseTemplate::new(204)) + .expect(0) + .mount(&server) + .await; + + let items = inventory(&[OWN_URL, FOREIGN_URL]); + let selected = select_for_delete(&items, None).unwrap(); + let results = delete_items_at(&test_client(), &server.uri(), &selected) + .await + .unwrap(); + + assert_eq!(results.len(), 3); + assert_eq!(results[0]["index"], 0); + assert_eq!(results[0]["deleted"], false); + assert!(results[0]["reason"] + .as_str() + .unwrap() + .contains("inline image")); + assert_eq!(results[1]["name"], "file0"); + assert_eq!(results[1]["deleted"], true); + assert!(results[1].get("reason").is_none()); + assert_eq!(results[2]["deleted"], false); + assert!(results[2]["reason"] + .as_str() + .unwrap() + .contains("not in your OneDrive")); + } + + #[tokio::test] + async fn delete_404_counts_as_already_deleted() { + let server = MockServer::start().await; + mount_me_drive(&server, 1).await; + mount_share(&server, OWN_URL, "my-drive", "own-item").await; + mount_delete(&server, "own-item", 404, 1).await; + + let items = inventory(&[OWN_URL]); + let selected = select_for_delete(&items, Some(1)).unwrap(); + let results = delete_items_at(&test_client(), &server.uri(), &selected) + .await + .unwrap(); + assert_eq!(results.len(), 1); + assert_eq!(results[0]["deleted"], true); + assert_eq!(results[0]["reason"], "already deleted"); + } + + #[tokio::test] + async fn unresolvable_share_is_skipped_without_delete() { + let server = MockServer::start().await; + Mock::given(method("GET")) + .and(path(format!( + "/shares/{}/driveItem", + endpoints::sharing_url_token(OWN_URL) + ))) + .respond_with(ResponseTemplate::new(404)) + .expect(1) + .mount(&server) + .await; + mount_me_drive(&server, 0).await; + mount_delete(&server, "own-item", 204, 0).await; + + let items = inventory(&[OWN_URL]); + let selected = select_for_delete(&items, Some(1)).unwrap(); + let results = delete_items_at(&test_client(), &server.uri(), &selected) + .await + .unwrap(); + assert_eq!(results[0]["deleted"], false); + assert!(results[0]["reason"].as_str().unwrap().contains("not found")); + } + + #[tokio::test] + async fn delete_failure_is_an_error_noting_earlier_deletes() { + let server = MockServer::start().await; + let second = "https://tenant-my.sharepoint.com/personal/me/Documents/Microsoft%20Teams%20Chat%20Files/b.pdf"; + mount_me_drive(&server, 1).await; + mount_share(&server, OWN_URL, "my-drive", "own-item").await; + mount_share(&server, second, "my-drive", "item-b").await; + mount_delete(&server, "own-item", 204, 1).await; + Mock::given(method("DELETE")) + .and(path("/drives/my-drive/items/item-b")) + .respond_with(ResponseTemplate::new(403)) + .expect(1) + .mount(&server) + .await; + + let items = inventory(&[OWN_URL, second]); + let selected = select_for_delete(&items, None).unwrap(); + let err = delete_items_at(&test_client(), &server.uri(), &selected) + .await + .unwrap_err(); + assert_eq!(err.exit_code(), 4); + let msg = err.to_string(); + assert!(msg.contains("Files.ReadWrite"), "{msg}"); + assert!( + msg.contains("Deleted before the failure: item(s) 1."), + "{msg}" + ); + } + + #[tokio::test] + async fn inline_image_only_selection_makes_no_requests() { + let server = MockServer::start().await; + let items = inventory(&[OWN_URL]); + let selected = select_for_delete(&items, Some(0)).unwrap(); + let results = delete_items_at(&test_client(), &server.uri(), &selected) + .await + .unwrap(); + assert_eq!(results.len(), 1); + assert_eq!(results[0]["deleted"], false); + assert!(server.received_requests().await.unwrap().is_empty()); + } + + #[test] + fn index_selection_matches_list_indexes() { + let items = inventory(&[OWN_URL, FOREIGN_URL]); + let picked = select_for_delete(&items, Some(2)).unwrap(); + assert_eq!(picked.len(), 1); + assert_eq!(picked[0].index, 2); + assert_eq!(picked[0].content_url.as_deref(), Some(FOREIGN_URL)); + + let err = select_for_delete(&items, Some(9)).unwrap_err(); + assert_eq!(err.exit_code(), 2); + assert!(err.to_string().contains("3 item(s)")); + + assert_eq!(select_for_delete(&[], None).unwrap_err().exit_code(), 5); + } } #[test] diff --git a/src/cli/message_media.rs b/src/cli/message_media.rs index 5193cb5..505dfa9 100644 --- a/src/cli/message_media.rs +++ b/src/cli/message_media.rs @@ -766,6 +766,7 @@ mod tests { last_modified_by: None, file: None, folder: None, + remote_item: None, download_url: None, parent_reference: None, }; diff --git a/src/models/file.rs b/src/models/file.rs index 130a848..afb687a 100644 --- a/src/models/file.rs +++ b/src/models/file.rs @@ -28,6 +28,9 @@ pub struct DriveItem { pub file: Option, #[serde(skip_serializing_if = "Option::is_none")] pub folder: Option, + /// A reference to an item in another drive, never an owned upload. + #[serde(skip_serializing_if = "Option::is_none")] + pub remote_item: Option, #[serde( rename = "@microsoft.graph.downloadUrl", skip_serializing_if = "Option::is_none" @@ -79,6 +82,12 @@ pub struct FilesFolder { pub parent_reference: Option, } +/// A drive (`GET /me/drive`); only its ID is needed. +#[derive(Debug, Clone, Deserialize)] +pub struct Drive { + pub id: Option, +} + /// Request to create a sharing link #[derive(Debug, Clone, Serialize)] #[serde(rename_all = "camelCase")] diff --git a/tests/cli.rs b/tests/cli.rs index 780c2ee..be54f7d 100644 --- a/tests/cli.rs +++ b/tests/cli.rs @@ -948,6 +948,100 @@ fn message_delete_rejects_incomplete_or_mixed_targets() { .stderr(predicate::str::contains("--team")); } +/// Like `message delete`, attachment deletion is refused without `--yes` +/// before any token is resolved, so the exit code is 2, not an auth error. +#[test] +fn message_attachments_delete_requires_yes() { + teams() + .args([ + "message", + "attachments", + "delete", + "--chat", + "19:abc@thread.v2", + "--message", + "1700000000000", + "--index", + "1", + "--output", + "json", + ]) + .assert() + .code(2) + .stdout( + predicate::str::contains("\"success\": false") + .and(predicate::str::contains("INVALID_INPUT")) + .and(predicate::str::contains("--yes")), + ); +} + +#[test] +fn message_attachments_delete_rejects_incomplete_or_mixed_targets() { + teams() + .args([ + "message", + "attachments", + "delete", + "--team", + "team-id", + "--message", + "1", + "--yes", + ]) + .assert() + .code(2) + .stderr(predicate::str::contains("--channel")); + + teams() + .args([ + "message", + "attachments", + "delete", + "--chat", + "19:abc@thread.v2", + "--team", + "team-id", + "--channel", + "channel-id", + "--message", + "1", + "--yes", + ]) + .assert() + .code(2) + .stderr(predicate::str::contains("cannot be used with")); + + teams() + .args([ + "message", + "attachments", + "delete", + "--chat", + "19:abc@thread.v2", + "--reply", + "2", + "--message", + "1", + "--yes", + ]) + .assert() + .code(2) + .stderr(predicate::str::contains("cannot be used with")); + + teams() + .args([ + "message", + "attachments", + "delete", + "--message", + "1", + "--yes", + ]) + .assert() + .code(2) + .stderr(predicate::str::contains("--chat")); +} + #[test] fn message_documented_flags_are_available() { teams()