Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 <appId>` 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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,18 @@ namespace Microsoft.Agents.A365.DevTools.Cli.Helpers;
/// </summary>
public static class CommandStringHelper
{
private const string RedactedValue = "<redacted>";
private static readonly HashSet<string> SensitiveValueOptions = new(StringComparer.OrdinalIgnoreCase)
{
"--access-token",
"--api-key",
"--client-secret",
"--idp-client-secret",
"--password",
"--refresh-token",
"--token"
};

/// <summary>
/// Escapes a string for safe use in PowerShell single-quoted strings.
/// In PowerShell single-quoted strings, only single quotes need escaping (doubled).
Expand Down Expand Up @@ -41,4 +53,44 @@ public static bool ContainsDangerousCharacters(string input)
var dangerous = new[] { '\'', '"', ';', '`', '$', '&', '|', '<', '>', '\n', '\r', '\t' };
return input.IndexOfAny(dangerous) >= 0;
}

/// <summary>
/// Formats a command line for diagnostics while redacting values for options that carry secrets.
/// </summary>
public static string FormatForDisplay(string executable, IReadOnlyList<string> args)
{
ArgumentException.ThrowIfNullOrWhiteSpace(executable);
ArgumentNullException.ThrowIfNull(args);

var redactedArgs = new List<string>(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)}";
}
}
32 changes: 12 additions & 20 deletions src/Microsoft.Agents.A365.DevTools.Cli/Program.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -34,8 +35,7 @@ static async Task<int> 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 +
Expand Down Expand Up @@ -332,30 +332,22 @@ private static void ConfigureServices(IServiceCollection services, LogLevel mini
var authService = provider.GetRequiredService<AuthenticationService>();
var logger = provider.GetRequiredService<ILogger<Agent365ToolingService>>();

// 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;
}
}

Expand Down Expand Up @@ -397,7 +389,7 @@ private static void ConfigureServices(IServiceCollection services, LogLevel mini

// Register Azure CLI service
services.AddSingleton<IAzureCliService, AzureCliService>();

// Register confirmation provider for user prompts
services.AddSingleton<IConfirmationProvider, ConsoleConfirmationProvider>();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,12 @@ public class AuthenticationService : IAuthenticationService
/// </summary>
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<AuthenticationService> logger)
{
_logger = logger;
Expand Down Expand Up @@ -169,6 +175,7 @@ public async Task<string> GetAccessTokenAsync(
}
}

LogAuthenticationContext(token.AccessToken, token.TenantId, userId, resourceUrl);
return token.AccessToken;
}

Expand Down Expand Up @@ -616,6 +623,51 @@ private Task ClearMsalCacheAsync()
return Task.CompletedTask;
}

private void LogAuthenticationContext(
Comment thread
sellakumaran marked this conversation as resolved.
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);
}

/// <summary>
/// Records the current authentication user/tenant and returns whether it changed
/// since the last logged context. Thread-safe.
/// </summary>
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;
}
}

/// <summary>
/// Best-effort deletion of the legacy plaintext <c>auth-token.json</c> cache. The current CLI
/// never writes this file; this exists solely to clean up artifacts from older versions.
Expand Down Expand Up @@ -651,4 +703,4 @@ private class TokenInfo
public DateTime ExpiresOn { get; set; }
public string? TenantId { get; set; }
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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 <redacted>",
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=<redacted>",
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");
}
}