Resolve settings path deterministically and stop reads from clobbering settings.json

MSStore.CLI.UnitTests/ConfigurationManagerUnitTests.cs

@@ -0,0 +1,135 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License.
+
+using System.Text.Json;
+using MSStore.CLI.Services;
+
+namespace MSStore.CLI.UnitTests
+{
+    [TestClass]
+    public class ConfigurationManagerUnitTests
+    {
+        public TestContext TestContext { get; set; } = null!;
+
+        private string _settingsDirectory = null!;
+        private string? _originalSettingsDirectory;
+
+        [TestInitialize]
+        public void Initialize()
+        {
+            _originalSettingsDirectory = Environment.GetEnvironmentVariable(ConfigurationManager<Configurations>.SettingsDirectoryEnvironmentVariable);
+            _settingsDirectory = Path.Combine(Path.GetTempPath(), $"msstore-cli-tests-{Guid.NewGuid()}");
+            Environment.SetEnvironmentVariable(ConfigurationManager<Configurations>.SettingsDirectoryEnvironmentVariable, _settingsDirectory);
+        }
+
+        [TestCleanup]
+        public void Cleanup()
+        {
+            Environment.SetEnvironmentVariable(ConfigurationManager<Configurations>.SettingsDirectoryEnvironmentVariable, _originalSettingsDirectory);
+
+            if (Directory.Exists(_settingsDirectory))
+            {
+                Directory.Delete(_settingsDirectory, true);
+            }
+        }
+
+        private static ConfigurationManager<Configurations> CreateConfigurationManager()
+            => new(ConfigurationsSourceGenerationContext.Default.Configurations, "settings.json", null);
+
+        [TestMethod]
+        public void ConfigurationManager_ShouldUseSettingsDirectoryEnvironmentVariable()
+        {
+            var configurationManager = CreateConfigurationManager();
+
+            configurationManager.ConfigPath.Should().Be(Path.Combine(_settingsDirectory, "settings.json"));
+        }
+
+        [TestMethod]
+        public void ConfigurationManager_ConfigPathShouldAlwaysBeRooted()
+        {
+            Environment.SetEnvironmentVariable(ConfigurationManager<Configurations>.SettingsDirectoryEnvironmentVariable, null);
+
+            var configurationManager = CreateConfigurationManager();
+
+            Path.IsPathRooted(configurationManager.ConfigPath).Should().BeTrue();
+        }
+
+        [TestMethod]
+        public void ConfigurationManager_ShouldIgnoreRelativeSettingsDirectoryEnvironmentVariable()
+        {
+            var relativeSettingsDirectory = Path.Combine("relative", "settings");
+
+            Environment.SetEnvironmentVariable(ConfigurationManager<Configurations>.SettingsDirectoryEnvironmentVariable, relativeSettingsDirectory);
+
+            var configurationManager = CreateConfigurationManager();
+
+            // The path that would have been used if the relative override had been honored, which is anchored
+            // at the current working directory and would therefore move with it.
+            var honoredOverridePath = Path.Combine(Path.GetFullPath(relativeSettingsDirectory), "settings.json");
+
+            Path.IsPathRooted(configurationManager.ConfigPath).Should().BeTrue();
+            configurationManager.ConfigPath.Should().NotBe(honoredOverridePath);
+        }
+
+        [TestMethod]
+        public async Task ConfigurationManager_LoadShouldNotWriteAnythingIfSettingsFileDoesNotExist()
+        {
+            var configurationManager = CreateConfigurationManager();
+
+            var config = await configurationManager.LoadAsync(false, CancellationToken.None);
+
+            config.SellerId.Should().BeNull();
+            Directory.Exists(_settingsDirectory).Should().BeFalse();
+            File.Exists(configurationManager.ConfigPath).Should().BeFalse();
+        }
+
+        [TestMethod]
+        public async Task ConfigurationManager_LoadShouldReturnSavedSettings()
+        {
+            var configurationManager = CreateConfigurationManager();
+
+            await configurationManager.SaveAsync(
+                new Configurations
+                {
+                    SellerId = 12345,
+                    TenantId = new Guid("41261775-DB6D-4B44-9A36-7EB8565C7D22"),
+                    ClientId = new Guid("3F0BCAEF-6334-48CF-837F-81CB0F1F2C45")
+                },
+                CancellationToken.None);
+
+            var config = await new ConfigurationManager<Configurations>(ConfigurationsSourceGenerationContext.Default.Configurations, "settings.json", null)
+                .LoadAsync(false, CancellationToken.None);
+
+            config.SellerId.Should().Be(12345);
+            config.TenantId.Should().Be(new Guid("41261775-DB6D-4B44-9A36-7EB8565C7D22"));
+            config.ClientId.Should().Be(new Guid("3F0BCAEF-6334-48CF-837F-81CB0F1F2C45"));
+        }
+
+        [TestMethod]
+        public async Task ConfigurationManager_LoadShouldThrowIfSettingsAreInvalidAndShouldNotClearThem()
+        {
+            var configurationManager = CreateConfigurationManager();
+
+            Directory.CreateDirectory(_settingsDirectory);
+            await File.WriteAllTextAsync(configurationManager.ConfigPath, "not a json", TestContext.CancellationToken);
+
+            await Assert.ThrowsExactlyAsync<JsonException>(() => configurationManager.LoadAsync(false, CancellationToken.None));
+
+            (await File.ReadAllTextAsync(configurationManager.ConfigPath, TestContext.CancellationToken)).Should().Be("not a json");
+        }
+
+        [TestMethod]
+        public async Task ConfigurationManager_LoadShouldClearInvalidSettingsIfRequested()
+        {
+            var configurationManager = CreateConfigurationManager();
+
+            Directory.CreateDirectory(_settingsDirectory);
+            await File.WriteAllTextAsync(configurationManager.ConfigPath, "not a json", TestContext.CancellationToken);
+
+            var config = await configurationManager.LoadAsync(true, CancellationToken.None);
+
+            config.SellerId.Should().BeNull();
+            (await File.ReadAllTextAsync(configurationManager.ConfigPath, TestContext.CancellationToken)).Should().NotBe("not a json");
+        }
+    }
+}

MSStore.CLI/MicrosoftStoreCLI.cs

@@ -118,7 +118,7 @@ internal static async Task<bool> InitAsync(IAnsiConsole ansiConsole, IConfigurat
 
             if (config.SellerId == null)
             {
-                logger.LogCritical("SellerId is not set.");
+                logger.LogCritical("SellerId is not set. Settings file path: '{SettingsPath}'. Please, run the 'reconfigure' command.", configurationManager.ConfigPath);
                 return false;
             }
 

MSStore.CLI/Services/ConfigurationManager.cs

@@ -19,7 +19,73 @@ namespace MSStore.CLI.Services
     internal class ConfigurationManager<T>(JsonTypeInfo<T> jsonTypeInfo, string fileName, ILogger<ConfigurationManager<T>>? logger) : IConfigurationManager<T>
         where T : new()
     {
-        private static readonly string SettingsDirectory = Path.Combine(GetSystemLocalApplicationDataPath(), "Microsoft", "MSStore.CLI");
+        /// <summary>
+        /// Environment variable that overrides the directory where the CLI stores its settings files.
+        /// Must be set to an absolute path. Useful when the user's local application data folder cannot be
+        /// resolved, or is not stable between invocations (containers without a passwd entry, ephemeral
+        /// <c>HOME</c> directories, etc).
+        /// </summary>
+        internal static readonly string SettingsDirectoryEnvironmentVariable = "MSSTORE_SETTINGS_DIRECTORY";
+
+        /// <summary>
+        /// Resolves the directory where the settings files live. The returned path is always rooted, so that
+        /// it can never be interpreted relative to the current working directory, which would make the settings
+        /// files resolve to different locations depending on where the CLI happens to be invoked from.
+        /// </summary>
+        /// <param name="logger">Logger used to report an unusable override.</param>
+        /// <returns>The rooted settings directory path.</returns>
+        private static string GetSettingsDirectory(ILogger? logger)
+        {
+            var settingsDirectoryOverride = Environment.GetEnvironmentVariable(SettingsDirectoryEnvironmentVariable);
+            if (!string.IsNullOrWhiteSpace(settingsDirectoryOverride))
+            {
+                // A relative override would put the settings files at a different place for each working
+                // directory the CLI is invoked from, which is exactly what this resolution avoids, so it is
+                // ignored rather than honored.
+                if (Path.IsPathRooted(settingsDirectoryOverride))
+                {
+                    return Path.GetFullPath(settingsDirectoryOverride);
+                }
+
+                logger?.LogWarning(
+                    "Ignoring the {EnvironmentVariable} environment variable: '{SettingsDirectory}' is not an absolute path.",
+                    SettingsDirectoryEnvironmentVariable,
+                    settingsDirectoryOverride);
+            }
+
+            var localApplicationDataPath = GetSystemLocalApplicationDataPath();
+
+            if (string.IsNullOrEmpty(localApplicationDataPath) || !Path.IsPathRooted(localApplicationDataPath))
+            {
+                // The system could not tell us where the local application data folder is (for instance, on Unix,
+                // when neither XDG_DATA_HOME, nor HOME, nor the passwd entry are available). Falling back to a
+                // relative path would make the settings file depend on the current working directory, so a
+                // rooted, invocation-independent location is used instead.
+                localApplicationDataPath = Path.Combine(Path.GetTempPath(), GetTemporarySettingsFolderName());
+            }
+
+            return Path.GetFullPath(Path.Combine(localApplicationDataPath, "Microsoft", "MSStore.CLI"));
+        }
+
+        /// <summary>
+        /// Builds the name of the folder used, inside the temporary folder, when the local application data
+        /// folder cannot be resolved. The user name is appended, when it is usable as a folder name, so that
+        /// different users on the same machine do not share the same settings folder.
+        /// </summary>
+        /// <returns>The temporary settings folder name.</returns>
+        private static string GetTemporarySettingsFolderName()
+        {
+            const string FolderName = ".msstore-cli";
+
+            var userName = Environment.UserName;
+
+            if (string.IsNullOrWhiteSpace(userName) || userName.AsSpan().IndexOfAny(Path.GetInvalidFileNameChars()) >= 0)
+            {
+                return FolderName;
+            }
+
+            return $"{FolderName}-{userName}";
+        }
 
         private static string GetSystemLocalApplicationDataPath()
         {
@@ -43,7 +109,7 @@ private static string GetSystemLocalApplicationDataPath()
             return Environment.GetFolderPath(Environment.SpecialFolder.LocalApplicationData);
         }
 
-        private readonly string _settingsPath = Path.Combine(SettingsDirectory, fileName);
+        private readonly string _settingsPath = Path.Combine(GetSettingsDirectory(logger), fileName);
         private readonly JsonTypeInfo<T> _jsonTypeInfo = jsonTypeInfo ?? throw new ArgumentNullException(nameof(jsonTypeInfo));
         private readonly ILogger? _logger = logger;
 
@@ -53,10 +119,13 @@ public async Task<T> LoadAsync(bool clearInvalidConfig, CancellationToken ct)
         {
             try
             {
-                EnsureDirectoryExists();
+                // No directory is created here on purpose: loading the configuration must not require write
+                // access, so that a missing settings file can always fall back to the default settings.
                 if (!File.Exists(_settingsPath))
                 {
-                    return await ClearAsync(ct);
+                    _logger?.LogInformation("Settings file not found at '{SettingsPath}'. Using the default settings.", _settingsPath);
+
+                    return new T();
                 }
 
                 using var file = File.Open(_settingsPath, FileMode.Open);
@@ -88,6 +157,7 @@ public async Task<T> ClearAsync(CancellationToken ct)
 
         public async Task SaveAsync(T config, CancellationToken ct)
         {
+            EnsureDirectoryExists();
             using var file = File.Open(_settingsPath, FileMode.OpenOrCreate);
             file.SetLength(0);
             file.Position = 0;
@@ -96,14 +166,16 @@ public async Task SaveAsync(T config, CancellationToken ct)
 
         private void EnsureDirectoryExists()
         {
-            if (Directory.Exists(SettingsDirectory))
+            var settingsDirectory = Path.GetDirectoryName(_settingsPath)!;
+
+            if (Directory.Exists(settingsDirectory))
             {
                 return;
             }
 
-            _logger?.LogInformation("Creating settings directory: {SettingsDirectory}", SettingsDirectory);
+            _logger?.LogInformation("Creating settings directory: {SettingsDirectory}", settingsDirectory);
 
-            _ = Directory.CreateDirectory(SettingsDirectory);
+            _ = Directory.CreateDirectory(settingsDirectory);
         }
     }
 }

README.md

@@ -8,6 +8,12 @@ The Microsoft Store Developer Command Line Interface is a cross-platform (Window
 ## Helpful links
 * [Documentation](https://aka.ms/msstoredevcli/docs) - Microsoft's official documentation on regards to available commands, installation steps, how to properly setup CI/CD environments, and general guidance.
 
+## Settings location
+
+The CLI stores its (non-secret) configuration in a `settings.json` file, inside the `Microsoft/MSStore.CLI` folder of the user's local application data folder (`%LOCALAPPDATA%` on Windows, `~/Library/Application Support` on MacOS, and `$XDG_DATA_HOME`/`~/.local/share` on Linux). Secrets are never stored in this file, they always go to the operating system's credential store.
+
+If that folder cannot be resolved, or is not stable between invocations (containers without a `passwd` entry, or CI setups that use an ephemeral `HOME`, for example), set the `MSSTORE_SETTINGS_DIRECTORY` environment variable to an absolute path, and the CLI will read and write its settings files there. Relative paths are ignored, as they would make the settings location depend on the directory the CLI is invoked from.
+
 ## Contributing
 
 This project welcomes contributions and suggestions.  Most contributions require you to agree to a