diff --git a/CHANGELOG.md b/CHANGELOG.md index 889ab858..c82ca592 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,8 @@ Agents provisioned before this release need `Agent365.Observability.OtelWrite` g **Option B — CLI** (`a365 setup admin`) has been removed in this release. Use Option A above, or copy the PowerShell instructions printed in the `a365 setup all` summary output. ### Added +- Log separator written at the start of each CLI invocation now redacts values for secret-bearing options (e.g. `--idp-client-secret`) so they are not written to the log file in plain text. +- Authentication context (tenant and user) is now logged at the `Information` level whenever the resolved sign-in identity changes, giving operators a clear audit trail in the log file of who the CLI is acting as, without exposing credentials. - `a365 develop-mcp evaluate` command for evaluating MCP server tool schema quality — runs deterministic and semantic checks (via GitHub Copilot or Claude Code CLIs), computes maturity scoring, and generates an interactive HTML report - `--skip-sp-provisioning` option on `setup all` — skips the interactive in-line provisioning of missing resource service principals (issue #429). Default: setup prompts per-resource and runs `az ad sp create --id ` using the operator's `az login`. With this flag, missing SPs are excluded from the consent URL and listed in the Action Required block with the `az` command and a per-SP consent URL. Implicitly enabled when stdin is redirected (CI / pipe scenarios). - Action Required block in the `setup all` summary now lists missing service principals — each entry shows the resource, pending scopes, the `az ad sp create` command, and the per-SP consent URL needed to complete provisioning. diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Helpers/CommandStringHelper.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Helpers/CommandStringHelper.cs index 1289337f..02e0e5d6 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Helpers/CommandStringHelper.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Helpers/CommandStringHelper.cs @@ -8,6 +8,18 @@ namespace Microsoft.Agents.A365.DevTools.Cli.Helpers; /// public static class CommandStringHelper { + private const string RedactedValue = ""; + private static readonly HashSet SensitiveValueOptions = new(StringComparer.OrdinalIgnoreCase) + { + "--access-token", + "--api-key", + "--client-secret", + "--idp-client-secret", + "--password", + "--refresh-token", + "--token" + }; + /// /// Escapes a string for safe use in PowerShell single-quoted strings. /// In PowerShell single-quoted strings, only single quotes need escaping (doubled). @@ -41,4 +53,44 @@ public static bool ContainsDangerousCharacters(string input) var dangerous = new[] { '\'', '"', ';', '`', '$', '&', '|', '<', '>', '\n', '\r', '\t' }; return input.IndexOfAny(dangerous) >= 0; } + + /// + /// Formats a command line for diagnostics while redacting values for options that carry secrets. + /// + public static string FormatForDisplay(string executable, IReadOnlyList args) + { + ArgumentException.ThrowIfNullOrWhiteSpace(executable); + ArgumentNullException.ThrowIfNull(args); + + var redactedArgs = new List(args.Count); + var redactNext = false; + + foreach (var arg in args) + { + if (redactNext) + { + redactedArgs.Add(RedactedValue); + redactNext = false; + continue; + } + + var equalsIndex = arg.IndexOf('='); + if (equalsIndex > 0) + { + var optionName = arg[..equalsIndex]; + if (SensitiveValueOptions.Contains(optionName)) + { + redactedArgs.Add($"{optionName}={RedactedValue}"); + continue; + } + } + + redactedArgs.Add(arg); + redactNext = SensitiveValueOptions.Contains(arg); + } + + return redactedArgs.Count == 0 + ? executable + : $"{executable} {string.Join(" ", redactedArgs)}"; + } } diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Program.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Program.cs index 1cd397d7..6661d462 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Program.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Program.cs @@ -3,6 +3,7 @@ using Microsoft.Agents.A365.DevTools.Cli.Commands; using Microsoft.Agents.A365.DevTools.Cli.Exceptions; +using Microsoft.Agents.A365.DevTools.Cli.Helpers; using Microsoft.Agents.A365.DevTools.Cli.Services; using Microsoft.Agents.A365.DevTools.Cli.Services.Evaluate; using Microsoft.Agents.A365.DevTools.Cli.Services.Helpers; @@ -34,8 +35,7 @@ static async Task Main(string[] args) { try { - // args are CLI arguments only — no secrets are passed as args (API keys, passwords are read from config/env). - var commandLine = "a365 " + string.Join(" ", args); + var commandLine = CommandStringHelper.FormatForDisplay("a365", args); var separator = Environment.NewLine + "============================================================" + Environment.NewLine + @@ -332,30 +332,22 @@ private static void ConfigureServices(IServiceCollection services, LogLevel mini var authService = provider.GetRequiredService(); var logger = provider.GetRequiredService>(); - // Default to "prod". Override with A365_ENVIRONMENT env var or --config file. + // Default to "prod". Override with A365_ENVIRONMENT env var or a365.config.json. string environment = Environment.GetEnvironmentVariable("A365_ENVIRONMENT") ?? "prod"; - var args = Environment.GetCommandLineArgs(); - var configIndex = Array.FindIndex(args, arg => arg == "--config" || arg == "-c"); - if (configIndex >= 0 && configIndex < args.Length - 1) + var configFilePath = ConfigService.GetConfigFilePath(); + if (configFilePath != null) { try { - var configFilePath = args[configIndex + 1]; - if (!Path.IsPathRooted(configFilePath)) - configFilePath = Path.Combine(System.Environment.CurrentDirectory, configFilePath); - - if (File.Exists(configFilePath)) + var json = File.ReadAllText(configFilePath); + using var doc = System.Text.Json.JsonDocument.Parse(json); + if (doc.RootElement.TryGetProperty("environment", out var envProp)) { - var json = File.ReadAllText(configFilePath); - using var doc = System.Text.Json.JsonDocument.Parse(json); - if (doc.RootElement.TryGetProperty("environment", out var envProp)) + var envValue = envProp.GetString(); + if (!string.IsNullOrWhiteSpace(envValue)) { - var envValue = envProp.GetString(); - if (!string.IsNullOrWhiteSpace(envValue)) - { - environment = envValue; - } + environment = envValue; } } @@ -397,7 +389,7 @@ private static void ConfigureServices(IServiceCollection services, LogLevel mini // Register Azure CLI service services.AddSingleton(); - + // Register confirmation provider for user prompts services.AddSingleton(); diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/AuthenticationService.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/AuthenticationService.cs index a1f1f495..878d50ec 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/AuthenticationService.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/AuthenticationService.cs @@ -74,6 +74,12 @@ public class AuthenticationService : IAuthenticationService /// private const string LegacyTokenCacheFileName = "auth-token.json"; + // Deduplicates the "Authentication context" audit line so it only logs when the + // resolved user or tenant changes between token acquisitions. + private readonly object _authContextLogLock = new(); + private string? _lastLoggedUser; + private string? _lastLoggedTenant; + public AuthenticationService(ILogger logger) { _logger = logger; @@ -169,6 +175,7 @@ public async Task GetAccessTokenAsync( } } + LogAuthenticationContext(token.AccessToken, token.TenantId, userId, resourceUrl); return token.AccessToken; } @@ -616,6 +623,51 @@ private Task ClearMsalCacheAsync() return Task.CompletedTask; } + private void LogAuthenticationContext( + string accessToken, + string? fallbackTenantId, + string? fallbackUserId, + string resourceUrl) + { + var user = TryExtractUpnFromJwt(accessToken) ?? fallbackUserId ?? "(unknown)"; + var tenant = JwtHelper.TryDecodeClaim(accessToken, "tid") ?? fallbackTenantId ?? "(unknown)"; + + if (TryClaimContextChange(user, tenant)) + { + _logger.LogInformation( + "Authentication context: API calls will use user {User} in tenant {TenantId}", + user, + tenant); + } + + _logger.LogDebug( + "Resolved access token for {ResourceUrl} using user {User} in tenant {TenantId}", + resourceUrl, + user, + tenant); + } + + /// + /// Records the current authentication user/tenant and returns whether it changed + /// since the last logged context. Thread-safe. + /// + private bool TryClaimContextChange(string user, string tenant) + { + lock (_authContextLogLock) + { + var changed = !string.Equals(_lastLoggedUser, user, StringComparison.OrdinalIgnoreCase) + || !string.Equals(_lastLoggedTenant, tenant, StringComparison.OrdinalIgnoreCase); + + if (changed) + { + _lastLoggedUser = user; + _lastLoggedTenant = tenant; + } + + return changed; + } + } + /// /// Best-effort deletion of the legacy plaintext auth-token.json cache. The current CLI /// never writes this file; this exists solely to clean up artifacts from older versions. @@ -651,4 +703,4 @@ private class TokenInfo public DateTime ExpiresOn { get; set; } public string? TenantId { get; set; } } -} \ No newline at end of file +} diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Helpers/CommandStringHelperTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Helpers/CommandStringHelperTests.cs index 59b87a48..e755e77d 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Helpers/CommandStringHelperTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Helpers/CommandStringHelperTests.cs @@ -127,4 +127,50 @@ public void EscapePowerShellString_WorksInRealWorldScenario() // The command string is now safe - the injected code will be treated as literal text // because single quotes within PowerShell single-quoted strings are escaped as '' } + + [Fact] + public void FormatForDisplay_IdpClientSecretSeparateValue_RedactsSecret() + { + var result = CommandStringHelper.FormatForDisplay( + "a365", + ["develop-mcp", "register-external-mcp-server", "--idp-client-secret", "super-secret"]); + + result.Should().Contain("--idp-client-secret ", + because: "diagnostic command echoes must not expose a client secret passed as a separate CLI argument"); + result.Should().NotContain("super-secret", + because: "the original secret value must not survive in diagnostic output"); + } + + [Fact] + public void FormatForDisplay_IdpClientSecretEqualsValue_RedactsSecret() + { + var result = CommandStringHelper.FormatForDisplay( + "a365", + ["develop-mcp", "register-external-mcp-server", "--idp-client-secret=super-secret"]); + + result.Should().Contain("--idp-client-secret=", + because: "diagnostic command echoes must redact inline secret option values"); + result.Should().NotContain("super-secret", + because: "the original secret value must not survive in diagnostic output"); + } + + [Fact] + public void FormatForDisplay_SecretLifetimeMonths_IsNotRedacted() + { + var result = CommandStringHelper.FormatForDisplay( + "a365", + ["develop-mcp", "register-external-mcp-server", "--secret-lifetime-months", "3"]); + + result.Should().Contain("--secret-lifetime-months 3", + because: "secret lifetime is a numeric policy value, not a secret-bearing option"); + } + + [Fact] + public void FormatForDisplay_NoArguments_ReturnsExecutableOnly() + { + var result = CommandStringHelper.FormatForDisplay("a365", []); + + result.Should().Be("a365", + because: "the diagnostic command line should not include a trailing space when no arguments were supplied"); + } }