From 3e7e7bd2da97ea763b7935c7274acd5fc54b545d Mon Sep 17 00:00:00 2001
From: John Coffey <john@coffeylabs.org>
Date: Fri, 9 Oct 2026 16:10:36 -0700
Subject: [PATCH] Audit an administrator's access to a sharing account

AU-1.6 records access to another account through Impersonate, but it
treated any share from that account as the owner's grant for the whole
account. A share covers only the folders or calendars it names, so an
administrator could read and change everything else in an account that
had shared anything with them, without a record (GHSA-m992-gp7g-wc2j).

is_member_directly now counts only the token's own account and its
groups. Access to a sharing account is recorded like any other, once an
hour; using a share as an administrator is recorded too, which errs on
the side of the audit log.

Test 3b in the audit suite fails without this change.
---
 crates/common/src/audit.rs             |  4 +-
 crates/common/src/auth/access_token.rs | 12 +++---
 tests/src/system/audit.rs              | 60 ++++++++++++++++++++++++++
 3 files changed, 68 insertions(+), 8 deletions(-)

diff --git a/crates/common/src/audit.rs b/crates/common/src/audit.rs
index eb882266..b9f4acf1 100644
--- a/crates/common/src/audit.rs
+++ b/crates/common/src/audit.rs
@@ -308,8 +308,8 @@ impl Server {
 
     /// AU-1.6: access to another account's data through `Impersonate` (or a
     /// blob through `FetchAnyBlob`), once an hour per session's account and
-    /// target. Access through a share or group membership isn't this: the
-    /// owner granted it.
+    /// target. Group membership isn't this. A share is: it covers only what
+    /// was shared, and nothing tells which part an `Impersonate` holder read.
     pub async fn audit_foreign_access(&self, token: &AccessToken, target_id: u32, blob: bool) {
         if target_id == token.account_id() || token.is_member_directly(target_id) {
             return;
diff --git a/crates/common/src/auth/access_token.rs b/crates/common/src/auth/access_token.rs
index 0d53b524..2e1f94eb 100644
--- a/crates/common/src/auth/access_token.rs
+++ b/crates/common/src/auth/access_token.rs
@@ -599,13 +599,13 @@ impl AccessToken {
             || self.has_permission(Permission::Impersonate)
     }
 
-    /// inbuxa: AU-1.6: whether the account is reachable without
-    /// impersonation: its own, a group's it belongs to, or one shared with
-    /// it.
+    /// inbuxa: AU-1.6: whether the whole account is the token's without
+    /// impersonation: its own, or a group's it belongs to. A share doesn't
+    /// count. It grants some folders or calendars, not the account, and an
+    /// `Impersonate` holder reaches everything else in it unchecked, so
+    /// their access to a sharing account is audited too (GHSA-m992-gp7g-wc2j).
     pub fn is_member_directly(&self, account_id: u32) -> bool {
-        self.inner.account_id == account_id
-            || self.inner.member_of.contains(&account_id)
-            || self.inner.access_to.iter().any(|a| a.account_id == account_id)
+        self.inner.account_id == account_id || self.inner.member_of.contains(&account_id)
     }
 
     /// inbuxa: MA-D0: in the account only because it is a group this token
diff --git a/tests/src/system/audit.rs b/tests/src/system/audit.rs
index 203fcedf..ed17d2c5 100644
--- a/tests/src/system/audit.rs
+++ b/tests/src/system/audit.rs
@@ -221,6 +221,66 @@ pub async fn test(test: &mut TestServer) {
         "test 3"
     );
 
+    // Test 3b, AU-1.6 (GHSA-m992-gp7g-wc2j): an account that shared one
+    // folder with the administrator is still audited when the administrator
+    // reaches the rest of it. A share used to count as the owner's grant for
+    // the whole account, so nothing was recorded.
+    let share_secret = "another-secret-password-5521";
+    let sharer_id = admin
+        .create_user_account(
+            "auditshare@example.org",
+            share_secret,
+            "Audit sharer",
+            &[],
+            vec![],
+        )
+        .await
+        .id();
+    let sharer = Account::new("auditshare@example.org", share_secret, &[], "", sharer_id);
+    let mail = ["urn:ietf:params:jmap:core", "urn:ietf:params:jmap:mail"];
+    let get_mailboxes = json!([["Mailbox/get", {"accountId": sharer.id_string(), "ids": null}, "0"]]);
+    let mailboxes = sharer.jmap_request(&mail, get_mailboxes.clone()).await;
+    let inbox = mailboxes
+        .0
+        .pointer("/methodResponses/0/1/list")
+        .and_then(|list| list.as_array())
+        .and_then(|list| list.iter().find(|m| m["role"] == "inbox"))
+        .and_then(|m| m["id"].as_str())
+        .unwrap_or_else(|| panic!("test 3b: no Inbox in {}", mailboxes.0))
+        .to_string();
+    let shared = sharer
+        .jmap_request(
+            &mail,
+            json!([["Mailbox/set", {
+                "accountId": sharer.id_string(),
+                "update": {inbox: {format!("shareWith/{admin_id}"): {"mayReadItems": true}}}
+            }, "0"]]),
+        )
+        .await;
+    assert!(
+        shared
+            .0
+            .pointer("/methodResponses/0/1/updated")
+            .is_some_and(|updated| !updated.is_null()),
+        "test 3b: share: {}",
+        shared.0
+    );
+    let response = admin.jmap_request(&mail, get_mailboxes).await;
+    assert_eq!(
+        response.0.pointer("/methodResponses/0/0"),
+        Some(&json!("Mailbox/get")),
+        "test 3b: {}",
+        response.0
+    );
+    let access = admin
+        .audit(json!({"action": "accountAccess", "accountId": sharer.id_string()}))
+        .await;
+    assert_eq!(access.len(), 1, "test 3b: {access:?}");
+    assert_eq!(
+        access[0]["target"]["name"], "auditshare@example.org",
+        "test 3b"
+    );
+
     // AU-9: a plain user can't read the audit log
     let plain = Account::new(
         "audituser@example.org",
-- 
2.54.0