Broaden elevated process limitations

doc/release-notes/1.31/user-scope-elevation-guards.md

@@ -0,0 +1,15 @@
+## Bug Fixes
+
+### Consistent elevation checks for user-scope package operations
+
+Added elevated-process protections for user-scope MSI uninstall and repair, portable upgrades, and font installation and removal. Repair and uninstall-previous operations reject unsupported elevation before preparing the action. These operations report a consistent error directing callers to retry from a non-administrator context.
+
+Font protection also covers initial and forced installations, which can inspect or replace existing font registrations.
+
+Fresh portable installations remain supported from an elevated process and no longer reuse preexisting installation metadata or cleanup records.
+
+The checks apply to the elevated half of a split UAC token. Existing behavior with UAC disabled is unchanged.
+
+### Minor Bug Fixes
+
+* Fixed installing new dependencies during an upgrade and applying upgrade protections when an installed dependency needs an update.

src/AppInstallerCLICore/PortableInstaller.cpp

@@ -259,11 +259,10 @@ namespace AppInstaller::CLI::Portable
         }
     }
 
-    // TODO: Optimize by applying the difference between expected and desired state.
-    void PortableInstaller::ApplyDesiredState()
+    void PortableInstaller::RemoveExpectedState()
     {
         std::filesystem::path existingIndexPath = InstallLocation / GetPortableIndexFileName();
-        if (std::filesystem::exists(existingIndexPath))
+        if (UsesExistingInstallData() && !InstallLocation.empty() && std::filesystem::exists(existingIndexPath))
         {
             bool deleteIndex = false;
             {
@@ -297,7 +296,7 @@ namespace AppInstaller::CLI::Portable
 
             if (deleteIndex)
             {
-                std::filesystem::remove(existingIndexPath);
+                SQLiteStorageBase::DeleteSQLiteDatabase(existingIndexPath);
                 AICLI_LOG(CLI, Info, << "Portable index deleted: " << existingIndexPath);
             }
         }
@@ -324,16 +323,34 @@ namespace AppInstaller::CLI::Portable
             }
         }
 
-        // Check if existing install location differs from the target install location for proper cleanup.
-        if (!TargetInstallLocation.empty() && TargetInstallLocation != InstallLocation)
+        if (!UsesExistingInstallData())
         {
-            RemoveInstallDirectory();
+            SQLiteStorageBase::DeleteSQLiteDatabase(TargetInstallLocation / GetPortableIndexFileName());
+        }
+    }
+
+    // TODO: Optimize by applying the difference between expected and desired state.
+    void PortableInstaller::ApplyDesiredState()
+    {
+        if (!UsesExistingInstallData())
+        {
+            // Replace the selected index slot without opening any preexisting database or journal.
+            SQLiteStorageBase::DeleteSQLiteDatabase(TargetInstallLocation / GetPortableIndexFileName());
+        }
+        else
+        {
+            RemoveExpectedState();
+
+            if (TargetInstallLocation != InstallLocation)
+            {
+                RemoveInstallDirectory();
+            }
         }
 
         if (RecordToIndex)
         {
             std::filesystem::path targetIndexPath = TargetInstallLocation / GetPortableIndexFileName();
-            PortableIndex targetIndex = std::filesystem::exists(targetIndexPath) ?
+            PortableIndex targetIndex = UsesExistingInstallData() && std::filesystem::exists(targetIndexPath) ?
                 PortableIndex::Open(targetIndexPath.u8string(), SQLiteStorageBase::OpenDisposition::ReadWrite) :
                 PortableIndex::CreateNew(targetIndexPath.u8string());
 
@@ -366,35 +383,42 @@ namespace AppInstaller::CLI::Portable
         return true;
     }
 
-    void PortableInstaller::Install(Workflow::OperationType operation)
+    void PortableInstaller::Install()
     {
+        THROW_HR_IF(E_UNEXPECTED, !SupportsInstall());
+
         // If the operation is an install, the ARP entry should be created first so that a catastrophic failure
         // leaves the system in a state where an uninstall may be possible
-        if (operation == Workflow::OperationType::Install)
+        if (!UsesExistingInstallData())
         {
             RegisterARPEntry();
         }
 
-        CreateTargetInstallDirectory();
+        const bool targetInstallDirectoryCreated = CreateTargetInstallDirectory();
 
         ApplyDesiredState();
 
+        InstallLocation = TargetInstallLocation;
+        InstallDirectoryCreated = targetInstallDirectoryCreated;
+
         if (!InstallDirectoryAddedToPath)
         {
             AddToPathVariable(GetPortableLinksLocation(GetScope()));
         }
 
         // If the operation is an upgrade, the ARP entry should be created last so that a catastrophic failure
         // leaves the system in a state where an upgrade can be re-attempted
-        if (operation == Workflow::OperationType::Upgrade)
+        if (UsesExistingInstallData())
         {
             RegisterARPEntry();
         }
     }
 
     void PortableInstaller::Uninstall()
     {
-        ApplyDesiredState();
+        THROW_HR_IF(E_UNEXPECTED, !SupportsUninstall() || (!UsesExistingInstallData() && TargetInstallLocation.empty()));
+
+        RemoveExpectedState();
 
         RemoveInstallDirectory();
 
@@ -412,15 +436,25 @@ namespace AppInstaller::CLI::Portable
         AICLI_LOG(CLI, Info, << "PortableARPEntry deleted.");
     }
 
-    void PortableInstaller::CreateTargetInstallDirectory()
+    bool PortableInstaller::CreateTargetInstallDirectory()
     {
-        if (std::filesystem::create_directories(TargetInstallLocation))
+        const bool created = std::filesystem::create_directories(TargetInstallLocation);
+        if (created)
         {
             AICLI_LOG(Core, Info, << "Created target install directory: " << TargetInstallLocation);
-            CommitToARPEntry(PortableValueName::InstallDirectoryCreated, true);
         }
 
+        const bool targetInstallDirectoryCreated = created || (TargetInstallLocation == InstallLocation && InstallDirectoryCreated);
+        if (!UsesExistingInstallData())
+        {
+            InstallLocation = TargetInstallLocation;
+            InstallDirectoryCreated = targetInstallDirectoryCreated;
+        }
+
+        CommitToARPEntry(PortableValueName::InstallDirectoryCreated, targetInstallDirectoryCreated);
         CommitToARPEntry(PortableValueName::InstallLocation, TargetInstallLocation);
+
+        return targetInstallDirectoryCreated;
     }
 
     void PortableInstaller::RemoveInstallDirectory()
@@ -533,7 +567,7 @@ namespace AppInstaller::CLI::Portable
     {
         const auto& indexPath = InstallLocation / GetPortableIndexFileName();
 
-        if (std::filesystem::exists(indexPath))
+        if (!InstallLocation.empty() && std::filesystem::exists(indexPath))
         {
             PortableIndex portableIndex = PortableIndex::Open(indexPath.u8string(), SQLiteStorageBase::OpenDisposition::ReadWrite);
             m_expectedEntries = portableIndex.GetAllPortableFiles();
@@ -576,9 +610,17 @@ namespace AppInstaller::CLI::Portable
         }
     }
 
-    PortableInstaller::PortableInstaller(Manifest::ScopeEnum scope, Utility::Architecture arch, const std::string& productCode) :
+    PortableInstaller::PortableInstaller(Manifest::ScopeEnum scope, Utility::Architecture arch, const std::string& productCode, Workflow::OperationType operation) :
+        m_operation(operation),
         m_portableARPEntry(PortableARPEntry(scope, arch, productCode))
     {
+        THROW_HR_IF(E_INVALIDARG, !SupportsInstall() && !SupportsUninstall());
+
+        if (!UsesExistingInstallData())
+        {
+            return;
+        }
+
         if (ARPEntryExists())
         {
             DisplayName = GetStringValue(PortableValueName::DisplayName);
@@ -604,6 +646,12 @@ namespace AppInstaller::CLI::Portable
 
     void PortableInstaller::RegisterARPEntry()
     {
+        if (!UsesExistingInstallData())
+        {
+            // Stale registration values must not become installed state for a later operation.
+            Registry::Key::DeleteTree(m_portableARPEntry.GetKey(), {});
+        }
+
         CommitToARPEntry(PortableValueName::WinGetPackageIdentifier, WinGetPackageIdentifier);
         CommitToARPEntry(PortableValueName::WinGetSourceIdentifier, WinGetSourceIdentifier);
         CommitToARPEntry(PortableValueName::UninstallString, "winget uninstall --product-code " + GetProductCode());

src/AppInstallerCLICore/PortableInstaller.h

@@ -17,7 +17,7 @@ namespace AppInstaller::CLI::Portable
     // Object representation of the metadata and functionality required for installing a Portable package. 
     struct PortableInstaller
     {
-        // These values are initialized based on the values from the entry in ARP
+        // Existing ARP values are loaded only for upgrade and uninstall operations.
         std::string DisplayName;
         std::string DisplayVersion;
         std::string HelpLink;
@@ -37,14 +37,17 @@ namespace AppInstaller::CLI::Portable
         // If we fail to create a symlink, add install directory to PATH variable
         bool InstallDirectoryAddedToPath = false;
 
-        bool IsUpdate = false;
         bool Purge = false;
         bool RecordToIndex = false;
 
         // This is the incoming target install location determined from the context args.
         std::filesystem::path TargetInstallLocation;
 
-        PortableInstaller(Manifest::ScopeEnum scope, Utility::Architecture arch, const std::string& productCode);
+        PortableInstaller(Manifest::ScopeEnum scope, Utility::Architecture arch, const std::string& productCode, Workflow::OperationType operation);
+
+        bool IsUpdate() const { return m_operation == Workflow::OperationType::Upgrade; }
+
+        bool UsesExistingInstallData() const { return m_operation != Workflow::OperationType::Install; }
 
         bool VerifyExpectedState();
 
@@ -57,9 +60,10 @@ namespace AppInstaller::CLI::Portable
         {
             m_expectedEntries = m_desiredEntries;
             m_desiredEntries = {};
+            InstallLocation = TargetInstallLocation;
         }
 
-        void Install(AppInstaller::CLI::Workflow::OperationType operation = Workflow::OperationType::Install);
+        void Install();
 
         void Uninstall();
 
@@ -94,11 +98,15 @@ namespace AppInstaller::CLI::Portable
         AppInstaller::Manifest::AppsAndFeaturesEntry GetAppsAndFeaturesEntry();
 
     private:
+        Workflow::OperationType m_operation;
         PortableARPEntry m_portableARPEntry;
         std::vector<AppInstaller::Portable::PortableFileEntry> m_desiredEntries;
         std::vector<AppInstaller::Portable::PortableFileEntry> m_expectedEntries;
         std::stringstream m_stream;
 
+        bool SupportsInstall() const { return !UsesExistingInstallData() || IsUpdate(); }
+        bool SupportsUninstall() const { return !UsesExistingInstallData() || m_operation == Workflow::OperationType::Uninstall; }
+
         std::string GetStringValue(PortableValueName valueName);
         std::filesystem::path GetPathValue(PortableValueName valueName);
         bool GetBoolValue(PortableValueName valueName);
@@ -107,10 +115,11 @@ namespace AppInstaller::CLI::Portable
         void RegisterARPEntry();
 
         void ApplyDesiredState();
+        void RemoveExpectedState();
         void InstallFile(AppInstaller::Portable::PortableFileEntry& desiredState);
         void RemoveFile(AppInstaller::Portable::PortableFileEntry& desiredState);
 
-        void CreateTargetInstallDirectory();
+        bool CreateTargetInstallDirectory();
         void RemoveInstallDirectory();
 
         void AddToPathVariable(std::filesystem::path value);

src/AppInstallerCLICore/Resources.h

@@ -23,6 +23,7 @@ namespace AppInstaller::CLI::Resource
         WINGET_DEFINE_RESOURCE_STRINGID(AcceptSourceAgreementsArgumentDescription);
         WINGET_DEFINE_RESOURCE_STRINGID(AdjoinedNotFlagError);
         WINGET_DEFINE_RESOURCE_STRINGID(AdjoinedNotFoundError);
+        WINGET_DEFINE_RESOURCE_STRINGID(AdminContextActionProhibited);
         WINGET_DEFINE_RESOURCE_STRINGID(AdminSettingDisabled);
         WINGET_DEFINE_RESOURCE_STRINGID(AdminSettingDisableDescription);
         WINGET_DEFINE_RESOURCE_STRINGID(AdminSettingEnabled);
@@ -499,8 +500,6 @@ namespace AppInstaller::CLI::Resource
         WINGET_DEFINE_RESOURCE_STRINGID(NestedInstallerNotFound);
         WINGET_DEFINE_RESOURCE_STRINGID(NestedInstallerNotSpecified);
         WINGET_DEFINE_RESOURCE_STRINGID(NestedInstallerNotSupported);
-        WINGET_DEFINE_RESOURCE_STRINGID(NoAdminRepairForUserScopePackage);
-        WINGET_DEFINE_RESOURCE_STRINGID(NoAdminUninstallForUserScopePackage);
         WINGET_DEFINE_RESOURCE_STRINGID(NoApplicableInstallers);
         WINGET_DEFINE_RESOURCE_STRINGID(NoExperimentalFeaturesMessage);
         WINGET_DEFINE_RESOURCE_STRINGID(NoInstalledFontFound);

src/AppInstallerCLICore/Workflows/DependenciesFlow.cpp

@@ -341,6 +341,15 @@ namespace AppInstaller::CLI::Workflow
                 dependencyContext.Add<Execution::Data::InstalledPackageVersion>(itr->second.InstalledPackageVersion);
                 dependencyContext.Add<Execution::Data::Installer>(itr->second.Installer);
 
+                if (itr->second.InstalledPackageVersion)
+                {
+                    dependencyContext.SetFlags(Execution::ContextFlag::InstallerExecutionUseUpdate);
+                }
+                else
+                {
+                    dependencyContext.ClearFlags(Execution::ContextFlag::InstallerExecutionUseUpdate);
+                }
+
                 if (WI_IsFlagSet(context.GetFlags(), Execution::ContextFlag::InstallerDownloadOnly))
                 {
                     dependencyContext.Add<Execution::Data::DownloadDirectory>(context.Get<Execution::Data::DownloadDirectory>() / L"Dependencies");

src/AppInstallerCLICore/Workflows/DependenciesFlow.h

@@ -40,9 +40,10 @@ namespace AppInstaller::CLI::Workflow
     void GetDependenciesInfoForUninstall(Execution::Context& context);
 
     // Builds the dependency graph and creates the sub contexts for each package dependency.
+    // Each dependency's installed version determines whether its operation is an install or upgrade.
     // Required Args: None
     // Inputs: Manifest, Installer and DependencySource
-    // Outputs: Dependencies
+    // Outputs: PackageSubContexts
     struct CreateDependencySubContexts : public WorkflowTask
     {
         CreateDependencySubContexts(

src/AppInstallerCLICore/Workflows/FontFlow.cpp

@@ -4,6 +4,7 @@
 #include "pch.h"
 #include "FontFlow.h"
 #include "TableOutput.h"
+#include "WorkflowBase.h"
 #include <winget/Fonts.h>
 #include <AppInstallerRuntime.h>
 using namespace AppInstaller::Utility;
@@ -15,6 +16,13 @@ namespace AppInstaller::CLI::Workflow
 
     namespace
     {
+        Manifest::ScopeEnum GetFontInstallScope(Execution::Context& context)
+        {
+            return context.Args.Contains(Args::Type::InstallScope) ?
+                Manifest::ConvertToScopeEnum(context.Args.GetArg(Args::Type::InstallScope)) :
+                Manifest::ScopeEnum::User;
+        }
+
         struct InstalledFontFamiliesTableLine
         {
             InstalledFontFamiliesTableLine(Utility::LocIndString familyName, int faceCount)
@@ -189,17 +197,25 @@ namespace AppInstaller::CLI::Workflow
         }
     }
 
-    void FontInstallImpl(Execution::Context& context)
+    void EnsureSupportForFontInstall(Execution::Context& context)
     {
-        context.Reporter.Info() << Resource::String::InstallFlowStartingPackageInstall << std::endl;
+        if (context.Get<Data::Installer>().value().EffectiveInstallerType() == Manifest::InstallerTypeEnum::Font &&
+            GetFontInstallScope(context) != Manifest::ScopeEnum::Machine)
+        {
+            context << EnsureNotRunningWithNonDefaultFullToken;
+        }
+    }
 
-        // We will default to User scope.
-        Manifest::ScopeEnum scope = Manifest::ScopeEnum::User;
-        if (context.Args.Contains(Execution::Args::Type::InstallScope))
+    void FontInstallImpl(Execution::Context& context)
+    {
+        const auto scope = GetFontInstallScope(context);
+        if (scope != Manifest::ScopeEnum::Machine)
         {
-            scope = Manifest::ConvertToScopeEnum(context.Args.GetArg(Execution::Args::Type::InstallScope));
+            AICLI_RETURN_IF_TERMINATED(context << EnsureNotRunningWithNonDefaultFullToken);
         }
 
+        context.Reporter.Info() << Resource::String::InstallFlowStartingPackageInstall << std::endl;
+
         Fonts::FontContext fontContext;
         fontContext.InstallerSource = InstallerSource::WinGet;
         fontContext.Scope = scope;
@@ -286,17 +302,23 @@ namespace AppInstaller::CLI::Workflow
 
     void FontUninstallImpl(Execution::Context& context)
     {
-        context.Reporter.Info() << Resource::String::UninstallFlowStartingPackageUninstall << std::endl;
-
         try
         {
             // We will default to installed scope.
-            auto scope = Manifest::ConvertToScopeEnum(context.Get<Execution::Data::InstalledPackageVersion>()->GetMetadata()[Repository::PackageVersionMetadata::InstalledScope]);
+            const auto installedScope = Manifest::ConvertToScopeEnum(context.Get<Execution::Data::InstalledPackageVersion>()->GetMetadata()[Repository::PackageVersionMetadata::InstalledScope]);
+            auto scope = installedScope;
             if (context.Args.Contains(Execution::Args::Type::InstallScope))
             {
                 scope = Manifest::ConvertToScopeEnum(context.Args.GetArg(Execution::Args::Type::InstallScope));
             }
 
+            if (installedScope != Manifest::ScopeEnum::Machine || scope != Manifest::ScopeEnum::Machine)
+            {
+                AICLI_RETURN_IF_TERMINATED(context << EnsureNotRunningWithNonDefaultFullToken);
+            }
+
+            context.Reporter.Info() << Resource::String::UninstallFlowStartingPackageUninstall << std::endl;
+
             Fonts::FontContext fontContext;
             fontContext.InstallerSource = InstallerSource::WinGet;
             if (context.Args.Contains(Execution::Args::Type::Manifest))