From a4544ee7ac8c6489bbc88e03f7a5f1d342c3e3bd Mon Sep 17 00:00:00 2001 From: ratovarius Date: Fri, 18 Sep 2026 19:09:32 -0300 Subject: [PATCH 1/2] fix(release): preserve credential and comment context --- crates/google-workspace-cli/src/auth.rs | 34 +++++++++++++++++-- .../src/helpers/docs/read.rs | 21 +++++++++--- .../src/helpers/docs/read_tests.rs | 22 ++++++++++++ 3 files changed, 71 insertions(+), 6 deletions(-) diff --git a/crates/google-workspace-cli/src/auth.rs b/crates/google-workspace-cli/src/auth.rs index 6b27e3f92..6182dc46f 100644 --- a/crates/google-workspace-cli/src/auth.rs +++ b/crates/google-workspace-cli/src/auth.rs @@ -367,8 +367,19 @@ async fn load_credentials_with_loader( ); } - // 2. Encrypted credentials - if enc_path.exists() { + // 2. Encrypted credentials. Inspect symlink metadata so dangling symlinks + // and inaccessible paths are treated as credential failures, not absence. + let encrypted_present = match std::fs::symlink_metadata(enc_path) { + Ok(_) => true, + Err(error) if error.kind() == std::io::ErrorKind::NotFound => false, + Err(_) => { + anyhow::bail!( + "Failed to inspect saved credentials at {}. Credentials and token caches have been preserved; no fallback credentials were used.", + crate::output::sanitize_for_terminal(&enc_path.display().to_string()) + ) + } + }; + if encrypted_present { // A read, decryption, or keyring failure does not mean the files are // disposable. Stop here so a retry cannot silently select another account. // Do not render backend error details, which may contain sensitive data. @@ -910,6 +921,25 @@ mod tests { } } + #[cfg(unix)] + #[tokio::test] + #[serial_test::serial] + async fn test_load_credentials_dangling_encrypted_symlink_blocks_fallback() { + use std::os::unix::fs::symlink; + + let dir = tempfile::tempdir().unwrap(); + let enc_path = dir.path().join("credentials.enc"); + let fallback_path = dir.path().join("credentials.json"); + let fallback_json = r#"{"client_id":"fallback","client_secret":"secret","refresh_token":"refresh","type":"authorized_user"}"#; + std::fs::write(&fallback_path, fallback_json).unwrap(); + symlink(dir.path().join("missing-encrypted"), &enc_path).unwrap(); + + let error = load_credentials_inner(None, &enc_path, &fallback_path) + .await + .expect_err("dangling encrypted credential symlink must block fallback"); + assert!(error.to_string().contains("saved credentials")); + } + #[tokio::test] #[serial_test::serial] async fn test_load_credentials_keyring_failure_preserves_files_and_blocks_adc() { diff --git a/crates/google-workspace-cli/src/helpers/docs/read.rs b/crates/google-workspace-cli/src/helpers/docs/read.rs index 33a06e2d9..b70824614 100644 --- a/crates/google-workspace-cli/src/helpers/docs/read.rs +++ b/crates/google-workspace-cli/src/helpers/docs/read.rs @@ -306,33 +306,46 @@ pub(super) fn normalize_with_comments(document: &Value) -> Result std::collections::HashMap> { let mut anchors = std::collections::HashMap::new(); - collect_comment_anchors_recursive(document, &mut anchors); + collect_comment_anchors_recursive(document, None, &mut anchors); anchors } fn collect_comment_anchors_recursive( value: &Value, + current_tab_id: Option, anchors: &mut std::collections::HashMap>, ) { match value { Value::Object(object) => { + let tab_id = object + .get("tabId") + .and_then(Value::as_str) + .map(String::from) + .or(current_tab_id); if let Some(comment_anchors) = object.get("commentAnchors").and_then(Value::as_object) { for (id, anchor) in comment_anchors { - let ranges = anchor + let mut ranges = anchor .get("ranges") .and_then(Value::as_array) .cloned() .unwrap_or_default(); + if let Some(tab_id) = tab_id.as_deref() { + for range in &mut ranges { + if range.get("tabId").is_none() { + range["tabId"] = Value::String(tab_id.to_string()); + } + } + } anchors.insert(id.clone(), ranges); } } for child in object.values() { - collect_comment_anchors_recursive(child, anchors); + collect_comment_anchors_recursive(child, tab_id.clone(), anchors); } } Value::Array(array) => { for child in array { - collect_comment_anchors_recursive(child, anchors); + collect_comment_anchors_recursive(child, current_tab_id.clone(), anchors); } } _ => {} diff --git a/crates/google-workspace-cli/src/helpers/docs/read_tests.rs b/crates/google-workspace-cli/src/helpers/docs/read_tests.rs index ca8b4c07b..b2b751116 100644 --- a/crates/google-workspace-cli/src/helpers/docs/read_tests.rs +++ b/crates/google-workspace-cli/src/helpers/docs/read_tests.rs @@ -215,6 +215,28 @@ fn comment_anchor_resolution_respects_document_segments() { assert_eq!(output["comments"][1]["referencedText"], json!(["header"])); } +#[test] +fn comment_anchor_without_tab_id_uses_enclosing_tab() { + let mut input = legacy(); + input["comments"] = json!([{"commentId": "c2", "anchorId": "a2"}]); + input["tabs"] = json!([ + {"tabProperties": {"tabId": "tab-1"}, "documentTab": {"body": {"content": []}}}, + {"tabProperties": {"tabId": "tab-2"}, "documentTab": { + "body": {"content": [{ + "startIndex": 1, "endIndex": 7, + "paragraph": {"elements": [{ + "startIndex": 1, "endIndex": 7, + "textRun": {"content": "second"} + }]} + }]}, + "commentAnchors": {"a2": {"ranges": [{"startIndex": 1, "endIndex": 7}]}} + }} + ]); + + let output = read::normalize_with_comments(&input).unwrap(); + assert_eq!(output["comments"][0]["referencedText"], json!(["second"])); +} + #[test] fn request_rejects_partial_masks_lossy_views_and_parameter_bypasses() { for params in [ From 33ebe36c0471cea4055b9ac925a32584ca2e6c49 Mon Sep 17 00:00:00 2001 From: ratovarius Date: Fri, 18 Sep 2026 19:17:59 -0300 Subject: [PATCH 2/2] fix(release): close PR 22 review findings --- .changeset/release-review-fixes.md | 5 +++ crates/google-workspace-cli/src/auth.rs | 39 ++++++++++++++++++- .../src/helpers/docs/read.rs | 4 +- 3 files changed, 46 insertions(+), 2 deletions(-) create mode 100644 .changeset/release-review-fixes.md diff --git a/.changeset/release-review-fixes.md b/.changeset/release-review-fixes.md new file mode 100644 index 000000000..8433d1de8 --- /dev/null +++ b/.changeset/release-review-fixes.md @@ -0,0 +1,5 @@ +--- +"@googleworkspace/cli": patch +--- + +Harden credential fallback and preserve document tab context when resolving comment anchors. \ No newline at end of file diff --git a/crates/google-workspace-cli/src/auth.rs b/crates/google-workspace-cli/src/auth.rs index 6182dc46f..20f87c091 100644 --- a/crates/google-workspace-cli/src/auth.rs +++ b/crates/google-workspace-cli/src/auth.rs @@ -352,6 +352,23 @@ async fn load_credentials_with_loader( enc_path: &std::path::Path, default_path: &std::path::Path, load_encrypted: impl FnOnce(&std::path::Path) -> anyhow::Result, +) -> anyhow::Result { + load_credentials_with_loaders( + env_file, + enc_path, + default_path, + |path| std::fs::symlink_metadata(path), + load_encrypted, + ) + .await +} + +async fn load_credentials_with_loaders( + env_file: Option<&str>, + enc_path: &std::path::Path, + default_path: &std::path::Path, + metadata: impl FnOnce(&std::path::Path) -> std::io::Result, + load_encrypted: impl FnOnce(&std::path::Path) -> anyhow::Result, ) -> anyhow::Result { // 1. Explicit env var — plaintext file (User or Service Account) if let Some(path) = env_file { @@ -369,7 +386,7 @@ async fn load_credentials_with_loader( // 2. Encrypted credentials. Inspect symlink metadata so dangling symlinks // and inaccessible paths are treated as credential failures, not absence. - let encrypted_present = match std::fs::symlink_metadata(enc_path) { + let encrypted_present = match metadata(enc_path) { Ok(_) => true, Err(error) if error.kind() == std::io::ErrorKind::NotFound => false, Err(_) => { @@ -940,6 +957,26 @@ mod tests { assert!(error.to_string().contains("saved credentials")); } + #[tokio::test] + async fn test_load_credentials_metadata_failure_blocks_fallback() { + let dir = tempfile::tempdir().unwrap(); + let enc_path = dir.path().join("credentials.enc"); + let fallback_path = dir.path().join("credentials.json"); + let error = load_credentials_with_loaders( + None, + &enc_path, + &fallback_path, + |_| Err(std::io::Error::from(std::io::ErrorKind::PermissionDenied)), + |_| Ok(String::new()), + ) + .await + .expect_err("credential metadata failures must block fallback"); + assert!(error + .to_string() + .contains("Failed to inspect saved credentials")); + assert!(!error.to_string().contains("No credentials found")); + } + #[tokio::test] #[serial_test::serial] async fn test_load_credentials_keyring_failure_preserves_files_and_blocks_adc() { diff --git a/crates/google-workspace-cli/src/helpers/docs/read.rs b/crates/google-workspace-cli/src/helpers/docs/read.rs index b70824614..8b7e97523 100644 --- a/crates/google-workspace-cli/src/helpers/docs/read.rs +++ b/crates/google-workspace-cli/src/helpers/docs/read.rs @@ -318,7 +318,9 @@ fn collect_comment_anchors_recursive( match value { Value::Object(object) => { let tab_id = object - .get("tabId") + .get("tabProperties") + .and_then(|properties| properties.get("tabId")) + .or_else(|| object.get("tabId")) .and_then(Value::as_str) .map(String::from) .or(current_tab_id);