Merge pull request #1623 from DeusData/fix/ancestor-appcontainer-sids

fix(windows): tolerate AppContainer SIDs on ancestors, keep the runtime dir strict

src/daemon/ipc.c

@@ -3987,8 +3987,45 @@ static bool win_sid_trusted(win_security_t *security, PSID sid) {
            win_sid_is_trusted_installer((const uint8_t *)sid, (size_t)sid_length);
 }
 
+/* AppContainer identities: package SIDs (S-1-15-2-*) and capability SIDs
+ * (S-1-15-3-*), under the APP_PACKAGE identifier authority (15).
+ *
+ * These are tolerated on ANCESTOR components only — never on the private
+ * runtime directory itself, which keeps demanding the exact current user.
+ *
+ * Why they are admissible there: a sandboxed package's ACE on %LOCALAPPDATA%
+ * grants that package, and a process cannot select which AppContainer it runs
+ * in — the identity is stamped by the OS at process creation from the package
+ * it was launched from. So such an ACE cannot be exercised by arbitrary local
+ * code the way a live local group can. What it does permit is the packaged
+ * application itself; that is the residual risk this exemption accepts, and it
+ * is the same trust already extended to whatever installed that package.
+ *
+ * Why BOTH forms: capability SIDs alone are not enough. The most common real
+ * ACE of this shape is `S-1-15-2-*` — a package SID. On reported machines it
+ * resolves through HKCR\...\AppContainer\Mappings to Anthropic Claude
+ * Desktop, an application many of our users run and cannot be asked to
+ * uninstall. Covering only S-1-15-3-* leaves exactly that case failing.
+ *
+ * Grounded in #1533 (four independent reproductions across four SID classes)
+ * and #1574. Approach and the ancestor-only boundary follow @mlandolfi90's
+ * PR #1447, extended to package SIDs. */
+static bool win_sid_is_app_container(const uint8_t *sid, size_t sid_length) {
+    if (!windows_sid_valid(sid, sid_length) || sid[1] < 1U) {
+        return false;
+    }
+    /* identifier authority must be exactly 15 (APP_PACKAGE_AUTHORITY) */
+    if (sid[2] != 0U || sid[3] != 0U || sid[4] != 0U || sid[5] != 0U || sid[6] != 0U ||
+        sid[7] != 15U) {
+        return false;
+    }
+    uint32_t first = win_sid_read_u32_le(sid + 8U);
+    return first == 2U || first == 3U;
+}
+
 static bool win_bounded_sid_trusted(win_security_t *security, const uint8_t *sid,
-                                    size_t sid_capacity, bool creator_owner_inherit_only) {
+                                    size_t sid_capacity, bool creator_owner_inherit_only,
+                                    bool ancestor) {
     if (!security || !sid || sid_capacity < 8U || sid[1] > 15U) {
         return false;
     }
@@ -4004,7 +4041,8 @@ static bool win_bounded_sid_trusted(win_security_t *security, const uint8_t *sid
              * user, so such an ACE only ever grants to us. Default Windows
              * profile/temp ACLs (and GitHub runner profiles) carry it, and
              * rejecting it locked real current-user directories out. */
-            security->is_well_known_sid((PSID)sid, WinCreatorOwnerRightsSid));
+            security->is_well_known_sid((PSID)sid, WinCreatorOwnerRightsSid) ||
+            (ancestor && win_sid_is_app_container(sid, sid_length)));
 }
 
 static bool win_file_owner_secure(win_security_t *security, HANDLE file,
@@ -4039,7 +4077,8 @@ static DWORD win_private_mutation_rights(void) {
            DELETE | WRITE_DAC | WRITE_OWNER | ACCESS_SYSTEM_SECURITY;
 }
 
-static bool win_file_acl_secure(win_security_t *security, HANDLE file, DWORD mutation) {
+static bool win_file_acl_secure(win_security_t *security, HANDLE file, DWORD mutation,
+                                bool ancestor) {
     PACL dacl = NULL;
     PSECURITY_DESCRIPTOR descriptor = NULL;
     DWORD status = security->get_security_info(file, SE_FILE_OBJECT, DACL_SECURITY_INFORMATION,
@@ -4080,7 +4119,8 @@ static bool win_file_acl_secure(win_security_t *security, HANDLE file, DWORD mut
         const uint8_t *sid = (const uint8_t *)&ace->SidStart;
         size_t sid_capacity = (size_t)header->AceSize - sid_offset;
         bool creator_owner_inherit_only = (header->AceFlags & INHERIT_ONLY_ACE) != 0U;
-        if (!win_bounded_sid_trusted(security, sid, sid_capacity, creator_owner_inherit_only)) {
+        if (!win_bounded_sid_trusted(security, sid, sid_capacity, creator_owner_inherit_only,
+                                     ancestor)) {
             /* Name the untrusted identity class so a harness/profile ACL leak
              * (an inherited Users / Authenticated Users / Everyone ACE) is
              * distinguishable from a genuinely hostile grant. */
@@ -4113,9 +4153,9 @@ static bool win_file_acl_secure(win_security_t *security, HANDLE file, DWORD mut
 }
 
 static bool win_file_security_secure(win_security_t *security, HANDLE file,
-                                     bool require_current_user, DWORD mutation) {
+                                     bool require_current_user, DWORD mutation, bool ancestor) {
     return win_file_owner_secure(security, file, require_current_user) &&
-           win_file_acl_secure(security, file, mutation);
+           win_file_acl_secure(security, file, mutation, ancestor);
 }
 
 static bool win_runtime_directory_secure(const wchar_t *runtime_dir) {
@@ -4179,7 +4219,7 @@ static bool win_runtime_directory_secure(const wchar_t *runtime_dir) {
     }
     bool final_private =
         secure_result == ERROR_SUCCESS &&
-        win_file_security_secure(&security, directory, true, win_private_mutation_rights());
+        win_file_security_secure(&security, directory, true, win_private_mutation_rights(), false);
     (void)CloseHandle(directory);
     win_security_destroy(&security);
     return valid_handle && owner_ok && final_private;
@@ -4202,7 +4242,7 @@ static bool win_directory_component_secure(win_security_t *security, const wchar
     bool valid = GetFileInformationByHandle(directory, &info) != 0 &&
                  (info.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY) != 0 &&
                  (info.dwFileAttributes & FILE_ATTRIBUTE_REPARSE_POINT) == 0 &&
-                 win_file_security_secure(security, directory, false, mutation);
+                 win_file_security_secure(security, directory, false, mutation, true);
     (void)CloseHandle(directory);
     return valid;
 }

tests/test_windows_bundle_contract.sh

@@ -415,7 +415,7 @@ require(
         needle in read("src/daemon/ipc.c")
         for needle in (
             "win_directory_component_secure",
-            "win_file_security_secure(security, directory, false, mutation)",
+            "win_file_security_secure(security, directory, false, mutation, true)",
             "win_private_mutation_rights()",
             "~((DWORD)FILE_ADD_SUBDIRECTORY)",
             "FILE_ADD_FILE",
@@ -429,6 +429,30 @@ require(
     "src/daemon/ipc.c must enforce the shared cross-account ancestor trust policy",
 )
 
+# ── 6b. AppContainer tolerance is ANCESTOR-ONLY ─────────────────────────────
+# Package (S-1-15-2-*) and capability (S-1-15-3-*) SIDs are admitted on ancestor
+# components so a machine running a sandboxed desktop app (Claude Desktop stamps
+# its package SID on %LOCALAPPDATA%) is not locked out. The private runtime
+# directory must keep demanding the exact current user: it is passed
+# ancestor=false and that is the whole boundary.
+require(
+    all(
+        needle in read("src/daemon/ipc.c")
+        for needle in (
+            "win_sid_is_app_container",
+            "ancestor && win_sid_is_app_container",
+            # both AppContainer forms, not capability alone
+            "first == 2U || first == 3U",
+            # APP_PACKAGE identifier authority
+            "sid[7] != 15U",
+            # the private runtime directory is validated with ancestor=false
+            "win_private_mutation_rights(), false)",
+        )
+    ),
+    "AppContainer SIDs must be tolerated on ancestors ONLY, covering both package "
+    "and capability forms, with the private runtime directory still strict",
+)
+
 # On Windows subprocess supervision receives a non-NULL lpApplicationName, so a
 # literal `git` would not use PATH. Resolve only git.exe beneath inherited
 # absolute PATH entries and never permit the current-directory search implied by