winget-cli

commit 8ee3db1188140b6cb71ca1b2f2328016b7470092
parent 6559f65f66d7895a403d6212b20519498b020dd6
Author: yao-msft <50888816+yao-msft@users.noreply.github.com>
Date:   Tue,  5 Sep 2023 11:37:52 -0700

Give System and Admins full access to state folders (#3471)


Diffstat:
M
src/AppInstallerCLITests/Runtime.cpp
 | 
11
+++++++++--
M
src/AppInstallerCommonCore/Runtime.cpp
 | 
19
+++++++++++++++++--

2 files changed, 26 insertions(+), 4 deletions(-)
diff --git a/src/AppInstallerCLITests/Runtime.cpp b/src/AppInstallerCLITests/Runtime.cpp
@@ -79,8 +79,15 @@ TEST_CASE("ApplyACL_BothOwners", "[runtime]")
     details.ACL[ACEPrincipal::CurrentUser] = ACEPermissions::ReadExecute;
     details.ACL[ACEPrincipal::System] = ACEPermissions::All;
 
-    // Both cannot be owners
-    REQUIRE_THROWS_HR(details.ApplyACL(), HRESULT_FROM_WIN32(ERROR_INVALID_STATE));
+    if (IsRunningAsSystem())
+    {
+        // Both cannot be owners
+        REQUIRE_THROWS_HR(details.ApplyACL(), HRESULT_FROM_WIN32(ERROR_INVALID_STATE));
+    }
+    else
+    {
+        REQUIRE_NOTHROW(details.ApplyACL());
+    }
 }
 
 TEST_CASE("ApplyACL_CurrentUserOwner_SystemAll", "[runtime]")
diff --git a/src/AppInstallerCommonCore/Runtime.cpp b/src/AppInstallerCommonCore/Runtime.cpp
@@ -254,6 +254,7 @@ namespace AppInstaller::Runtime
         // Configuring permissions for both CurrentUser and SYSTEM while not having owner set as one of them is not valid because
         // below we use only the owner permissions in the case of running as SYSTEM.
         if ((hasCurrentUser && hasSystem) &&
+            IsRunningAsSystem() &&
             (!Owner || (Owner.value() != ACEPrincipal::CurrentUser && Owner.value() != ACEPrincipal::System)))
         {
             THROW_HR(HRESULT_FROM_WIN32(ERROR_INVALID_STATE));
@@ -441,7 +442,12 @@ namespace AppInstaller::Runtime
             if (path == PathName::SecureSettingsForWrite)
             {
                 result.SetOwner(ACEPrincipal::Admins);
-                result.ACL[ACEPrincipal::CurrentUser] = ACEPermissions::ReadExecute;
+                // When running as system, we do not set current user permissions to avoid permission conflicts.
+                if (!IsRunningAsSystem())
+                {
+                    result.ACL[ACEPrincipal::CurrentUser] = ACEPermissions::ReadExecute;
+                }
+                result.ACL[ACEPrincipal::System] = ACEPermissions::All;
             }
             else
             {
@@ -500,12 +506,16 @@ namespace AppInstaller::Runtime
             result.Path = GetPathToAppDataDir(s_AppDataDir_State, forDisplay);
             result.Path /= GetRuntimePathStateName();
             result.SetOwner(ACEPrincipal::CurrentUser);
+            result.ACL[ACEPrincipal::System] = ACEPermissions::All;
+            result.ACL[ACEPrincipal::Admins] = ACEPermissions::All;
             break;
         case PathName::StandardSettings:
         case PathName::UserFileSettings:
             result.Path = GetPathToAppDataDir(s_AppDataDir_Settings, forDisplay);
             result.Path /= GetRuntimePathStateName();
             result.SetOwner(ACEPrincipal::CurrentUser);
+            result.ACL[ACEPrincipal::System] = ACEPermissions::All;
+            result.ACL[ACEPrincipal::Admins] = ACEPermissions::All;
             break;
         case PathName::SecureSettingsForRead:
         case PathName::SecureSettingsForWrite:
@@ -518,7 +528,12 @@ namespace AppInstaller::Runtime
             if (path == PathName::SecureSettingsForWrite)
             {
                 result.SetOwner(ACEPrincipal::Admins);
-                result.ACL[ACEPrincipal::CurrentUser] = ACEPermissions::ReadExecute;
+                // When running as system, we do not set current user permissions to avoid permission conflicts.
+                if (!IsRunningAsSystem())
+                {
+                    result.ACL[ACEPrincipal::CurrentUser] = ACEPermissions::ReadExecute;
+                }
+                result.ACL[ACEPrincipal::System] = ACEPermissions::All;
             }
             else
             {