Skip to content

Commit af787c1

Browse files
fix: address PR #433 review comments
- Extract IExecutionContextLogger / ExecutionContextLogger service Uses IConfigService.LoadAsync for merged config (static + generated + env-var overrides) so displayed values match what commands actually act against. Removes raw JsonDocument parsing from Program.cs. - Gate execution context panel on --verbose / A365_SHOW_CONTEXT=1 Previously ran az account show unconditionally on every non-help command (700ms-2s on Windows). Now opt-in only. - Drop az-derived context panel; JWT path is authoritative AuthenticationService.LogAuthenticationContext emits user/tenant from JWT claims (what the command actually acts against). Removes conflict between two panels that could disagree. - Consolidate --config arg parsing into Program.ResolveConfigPath Removes duplicate parser in DI setup. New helper handles both '--config value' and '--config=value' forms. Guards empty/whitespace values (fixes misleading 'not found (<cwd>)' diagnostic). - Move LogInformation inside lock in LogAuthenticationContext Removes the awkward changed-outside-lock shape. - Use Console.WriteLine for blank-line spacing in context panel logger.LogInformation empty strings emit garbage under JSON/file log formatters. - Refactor TryExtractUpnFromJwt into TryExtractClaimFromJwt helper Shared by UPN and tenant-id extraction, removing duplication. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 79f9a1e commit af787c1

8 files changed

Lines changed: 393 additions & 30 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ Agents provisioned before this release need `Agent365.Observability.OtelWrite` g
2323
**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.
2424

2525
### Added
26+
- Opt-in startup execution context display via `--verbose`, `-v`, or `A365_SHOW_CONTEXT=1`, showing the invoked command, resolved config path, merged config tenant, effective environment, and agent identity while redacting secret option values.
2627
- `--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).
2728
- 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.
2829
- `logs export [command] [--output <dir>]` — exports a redacted copy of a CLI diagnostic log safe to share with Microsoft support. Redacts JWT tokens, email addresses, OS-path usernames, and tenant-specific GUIDs; replaces identical values with consistent aliases so log correlation is preserved. Preserves diagnostic IDs that aren't sensitive but are useful for debugging — `TraceId`, `CorrelationId`, Microsoft Graph `request-id` and `client-request-id` values, and well-known public Microsoft / Agent 365 resource appIds (such as the Microsoft Graph appId `00000003-0000-0000-c000-000000000000`). Omit `[command]` to export all available logs at once.

‎src/Microsoft.Agents.A365.DevTools.Cli/Helpers/CommandStringHelper.cs‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,18 @@ namespace Microsoft.Agents.A365.DevTools.Cli.Helpers;
88
/// </summary>
99
public static class CommandStringHelper
1010
{
11+
private const string RedactedValue = "<redacted>";
12+
private static readonly HashSet<string> SensitiveValueOptions = new(StringComparer.OrdinalIgnoreCase)
13+
{
14+
"--access-token",
15+
"--api-key",
16+
"--client-secret",
17+
"--idp-client-secret",
18+
"--password",
19+
"--refresh-token",
20+
"--token"
21+
};
22+
1123
/// <summary>
1224
/// Escapes a string for safe use in PowerShell single-quoted strings.
1325
/// In PowerShell single-quoted strings, only single quotes need escaping (doubled).
@@ -41,4 +53,44 @@ public static bool ContainsDangerousCharacters(string input)
4153
var dangerous = new[] { '\'', '"', ';', '`', '$', '&', '|', '<', '>', '\n', '\r', '\t' };
4254
return input.IndexOfAny(dangerous) >= 0;
4355
}
56+
57+
/// <summary>
58+
/// Formats a command line for diagnostics while redacting values for options that carry secrets.
59+
/// </summary>
60+
public static string FormatForDisplay(string executable, IReadOnlyList<string> args)
61+
{
62+
ArgumentException.ThrowIfNullOrWhiteSpace(executable);
63+
ArgumentNullException.ThrowIfNull(args);
64+
65+
var redactedArgs = new List<string>(args.Count);
66+
var redactNext = false;
67+
68+
foreach (var arg in args)
69+
{
70+
if (redactNext)
71+
{
72+
redactedArgs.Add(RedactedValue);
73+
redactNext = false;
74+
continue;
75+
}
76+
77+
var equalsIndex = arg.IndexOf('=');
78+
if (equalsIndex > 0)
79+
{
80+
var optionName = arg[..equalsIndex];
81+
if (SensitiveValueOptions.Contains(optionName))
82+
{
83+
redactedArgs.Add($"{optionName}={RedactedValue}");
84+
continue;
85+
}
86+
}
87+
88+
redactedArgs.Add(arg);
89+
redactNext = SensitiveValueOptions.Contains(arg);
90+
}
91+
92+
return redactedArgs.Count == 0
93+
? executable
94+
: $"{executable} {string.Join(" ", redactedArgs)}";
95+
}
4496
}

‎src/Microsoft.Agents.A365.DevTools.Cli/Program.cs‎

Lines changed: 64 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,9 @@
22
// Licensed under the MIT License.
33

44
using Microsoft.Agents.A365.DevTools.Cli.Commands;
5+
using Microsoft.Agents.A365.DevTools.Cli.Constants;
56
using Microsoft.Agents.A365.DevTools.Cli.Exceptions;
7+
using Microsoft.Agents.A365.DevTools.Cli.Helpers;
68
using Microsoft.Agents.A365.DevTools.Cli.Services;
79
using Microsoft.Agents.A365.DevTools.Cli.Services.Helpers;
810
using Microsoft.Extensions.DependencyInjection;
@@ -33,8 +35,7 @@ static async Task<int> Main(string[] args)
3335
{
3436
try
3537
{
36-
// args are CLI arguments only — no secrets are passed as args (API keys, passwords are read from config/env).
37-
var commandLine = "a365 " + string.Join(" ", args);
38+
var commandLine = CommandStringHelper.FormatForDisplay("a365", args);
3839
var separator =
3940
Environment.NewLine +
4041
"============================================================" + Environment.NewLine +
@@ -249,6 +250,17 @@ await Task.WhenAll(
249250
|| a.StartsWith("--show-secret=", StringComparison.Ordinal));
250251
if (!isHelpOrVersion && !isShowSecret)
251252
{
253+
// Print execution context panel when --verbose or A365_SHOW_CONTEXT=1 is set.
254+
// Gated so fast read-only commands (list/get) are not penalised by the overhead.
255+
var showContext = isVerbose
256+
|| string.Equals(Environment.GetEnvironmentVariable("A365_SHOW_CONTEXT"), "1", StringComparison.Ordinal);
257+
if (showContext)
258+
{
259+
var executionContextLogger = serviceProvider.GetRequiredService<IExecutionContextLogger>();
260+
var configPath = ResolveConfigPath(args);
261+
await executionContextLogger.LogAsync(configPath, args);
262+
}
263+
252264
try
253265
{
254266
await configService.TryResolveClientAppIdAsync(graphApiService);
@@ -333,35 +345,28 @@ private static void ConfigureServices(IServiceCollection services, LogLevel mini
333345
string environment = Environment.GetEnvironmentVariable("A365_ENVIRONMENT") ?? "prod";
334346

335347
var args = Environment.GetCommandLineArgs();
336-
var configIndex = Array.FindIndex(args, arg => arg == "--config" || arg == "-c");
337-
if (configIndex >= 0 && configIndex < args.Length - 1)
348+
var configFilePath = ResolveConfigPath(args);
349+
try
338350
{
339-
try
351+
if (File.Exists(configFilePath))
340352
{
341-
var configFilePath = args[configIndex + 1];
342-
if (!Path.IsPathRooted(configFilePath))
343-
configFilePath = Path.Combine(System.Environment.CurrentDirectory, configFilePath);
344-
345-
if (File.Exists(configFilePath))
353+
var json = File.ReadAllText(configFilePath);
354+
using var doc = System.Text.Json.JsonDocument.Parse(json);
355+
if (doc.RootElement.TryGetProperty("environment", out var envProp))
346356
{
347-
var json = File.ReadAllText(configFilePath);
348-
using var doc = System.Text.Json.JsonDocument.Parse(json);
349-
if (doc.RootElement.TryGetProperty("environment", out var envProp))
357+
var envValue = envProp.GetString();
358+
if (!string.IsNullOrWhiteSpace(envValue))
350359
{
351-
var envValue = envProp.GetString();
352-
if (!string.IsNullOrWhiteSpace(envValue))
353-
{
354-
environment = envValue;
355-
}
360+
environment = envValue;
356361
}
357362
}
358363

359364
logger.LogDebug("Resolved environment from config: {Environment}", environment);
360365
}
361-
catch (Exception ex)
362-
{
363-
logger.LogDebug("Failed to read environment from config: {Error}", ex.Message);
364-
}
366+
}
367+
catch (Exception ex)
368+
{
369+
logger.LogDebug("Failed to read environment from config: {Error}", ex.Message);
365370
}
366371

367372
return new Agent365ToolingService(configService, authService, logger, environment);
@@ -394,7 +399,10 @@ private static void ConfigureServices(IServiceCollection services, LogLevel mini
394399

395400
// Register Azure CLI service
396401
services.AddSingleton<IAzureCliService, AzureCliService>();
397-
402+
403+
// Register execution context logger (used when --verbose or A365_SHOW_CONTEXT=1)
404+
services.AddSingleton<IExecutionContextLogger, ExecutionContextLogger>();
405+
398406
// Register confirmation provider for user prompts
399407
services.AddSingleton<IConfirmationProvider, ConsoleConfirmationProvider>();
400408

@@ -433,5 +441,38 @@ private static string DetectCommandName(string[] args)
433441
.Replace(" ", "-")
434442
.Replace("_", "-");
435443
}
444+
445+
/// <summary>
446+
/// Resolves the absolute path to a365.config.json from CLI arguments.
447+
/// Handles both "--config path" and "--config=path" forms.
448+
/// Empty or whitespace --config values fall back to the default "a365.config.json"
449+
/// in the working directory so diagnostics never report the current directory as a config file.
450+
/// </summary>
451+
internal static string ResolveConfigPath(string[] args)
452+
{
453+
ArgumentNullException.ThrowIfNull(args);
454+
455+
for (var i = 0; i < args.Length; i++)
456+
{
457+
string? raw = null;
458+
459+
if ((args[i] == "--config" || args[i] == "-c") && i < args.Length - 1)
460+
raw = args[i + 1];
461+
else if (args[i].StartsWith("--config=", StringComparison.Ordinal))
462+
raw = args[i]["--config=".Length..];
463+
464+
if (raw is not null)
465+
{
466+
if (string.IsNullOrWhiteSpace(raw))
467+
return Path.Combine(Environment.CurrentDirectory, ConfigConstants.DefaultConfigFileName);
468+
469+
return Path.IsPathRooted(raw)
470+
? raw
471+
: Path.Combine(Environment.CurrentDirectory, raw);
472+
}
473+
}
474+
475+
return Path.Combine(Environment.CurrentDirectory, ConfigConstants.DefaultConfigFileName);
476+
}
436477
}
437478

‎src/Microsoft.Agents.A365.DevTools.Cli/Services/AuthenticationService.cs‎

Lines changed: 61 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,9 @@ public class AuthenticationService : IAuthenticationService
6363
{
6464
private readonly ILogger<AuthenticationService> _logger;
6565
private readonly string _tokenCachePath;
66+
private readonly object _authContextLogLock = new();
67+
private string? _lastLoggedUser;
68+
private string? _lastLoggedTenant;
6669

6770
public AuthenticationService(ILogger<AuthenticationService> logger)
6871
{
@@ -168,13 +171,15 @@ public async Task<string> GetAccessTokenAsync(
168171
}
169172
else
170173
{
174+
LogAuthenticationContext(cachedToken.AccessToken, cachedToken.TenantId, userId, resourceUrl, fromCache: true);
171175
_logger.LogDebug("Using cached authentication token for {ResourceUrl} (tenant: {TenantId})",
172176
resourceUrl, tenantId);
173177
return cachedToken.AccessToken;
174178
}
175179
}
176180
else
177181
{
182+
LogAuthenticationContext(cachedToken.AccessToken, cachedToken.TenantId, userId, resourceUrl, fromCache: true);
178183
_logger.LogDebug("Using cached authentication token for {ResourceUrl} (tenant: {TenantId})",
179184
resourceUrl, tenantId);
180185
return cachedToken.AccessToken;
@@ -183,6 +188,7 @@ public async Task<string> GetAccessTokenAsync(
183188
}
184189
else
185190
{
191+
LogAuthenticationContext(cachedToken.AccessToken, cachedToken.TenantId, userId, resourceUrl, fromCache: true);
186192
_logger.LogDebug("Using cached authentication token for {ResourceUrl}", resourceUrl);
187193
return cachedToken.AccessToken;
188194
}
@@ -212,6 +218,7 @@ public async Task<string> GetAccessTokenAsync(
212218
_logger.LogDebug(
213219
"Authentication returned token for {ReturnedUser} but {RequestedUser} was requested. Not caching.",
214220
returnedUpn, userId);
221+
LogAuthenticationContext(token.AccessToken, token.TenantId, userId, resourceUrl, fromCache: false);
215222
// Return the token as-is — it may still be valid for this call.
216223
// Do not write it to cache under the userId key.
217224
return token.AccessToken;
@@ -220,6 +227,7 @@ public async Task<string> GetAccessTokenAsync(
220227

221228
// Cache the token with the appropriate cache key
222229
await CacheTokenAsync(cacheKey, token);
230+
LogAuthenticationContext(token.AccessToken, token.TenantId, userId, resourceUrl, fromCache: false);
223231

224232
return token.AccessToken;
225233
}
@@ -685,6 +693,21 @@ protected virtual TokenCredential CreateDeviceCodeCredential(string clientId, st
685693
}
686694

687695
private static string? TryExtractUpnFromJwt(string? jwt)
696+
{
697+
foreach (var claim in new[] { "upn", "preferred_username", "unique_name" })
698+
{
699+
var value = TryExtractClaimFromJwt(jwt, claim);
700+
if (!string.IsNullOrWhiteSpace(value))
701+
return value;
702+
}
703+
704+
return null;
705+
}
706+
707+
private static string? TryExtractTenantIdFromJwt(string? jwt)
708+
=> TryExtractClaimFromJwt(jwt, "tid");
709+
710+
private static string? TryExtractClaimFromJwt(string? jwt, string claimName)
688711
{
689712
if (string.IsNullOrWhiteSpace(jwt)) return null;
690713
try
@@ -698,17 +721,48 @@ protected virtual TokenCredential CreateDeviceCodeCredential(string clientId, st
698721
payload = payload.PadRight(payload.Length + (4 - payload.Length % 4) % 4, '=');
699722
var bytes = Convert.FromBase64String(payload);
700723
using var doc = JsonDocument.Parse(bytes);
701-
if (doc.RootElement.TryGetProperty("upn", out var upn) && !string.IsNullOrWhiteSpace(upn.GetString()))
702-
return upn.GetString();
703-
if (doc.RootElement.TryGetProperty("preferred_username", out var pref) && !string.IsNullOrWhiteSpace(pref.GetString()))
704-
return pref.GetString();
705-
if (doc.RootElement.TryGetProperty("unique_name", out var uniqueName) && !string.IsNullOrWhiteSpace(uniqueName.GetString()))
706-
return uniqueName.GetString();
724+
return doc.RootElement.TryGetProperty(claimName, out var claim) && claim.ValueKind == JsonValueKind.String
725+
? claim.GetString()
726+
: null;
707727
}
708-
catch { } // Static helper — no logger access. Caller logs via ResolveLoginHintFromCacheAsync.
728+
catch { } // Static helper — no logger access. Callers log when needed.
709729
return null;
710730
}
711731

732+
private void LogAuthenticationContext(
733+
string accessToken,
734+
string? fallbackTenantId,
735+
string? fallbackUserId,
736+
string resourceUrl,
737+
bool fromCache)
738+
{
739+
var user = TryExtractUpnFromJwt(accessToken) ?? fallbackUserId ?? "(unknown)";
740+
var tenant = TryExtractTenantIdFromJwt(accessToken) ?? fallbackTenantId ?? "(unknown)";
741+
742+
lock (_authContextLogLock)
743+
{
744+
var changed = !string.Equals(_lastLoggedUser, user, StringComparison.OrdinalIgnoreCase)
745+
|| !string.Equals(_lastLoggedTenant, tenant, StringComparison.OrdinalIgnoreCase);
746+
747+
if (changed)
748+
{
749+
_lastLoggedUser = user;
750+
_lastLoggedTenant = tenant;
751+
_logger.LogInformation(
752+
"Authentication context: API calls will use user {User} in tenant {TenantId} ({Source})",
753+
user,
754+
tenant,
755+
fromCache ? "cached token" : "interactive sign-in");
756+
}
757+
}
758+
759+
_logger.LogDebug(
760+
"Resolved access token for {ResourceUrl} using user {User} in tenant {TenantId}",
761+
resourceUrl,
762+
user,
763+
tenant);
764+
}
765+
712766
/// <summary>
713767
/// Clears cached authentication token(s)
714768
/// </summary>

0 commit comments

Comments
 (0)