Show CLI execution context at startup#433
Conversation
ae1b6b2 to
5b353d1
Compare
- 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>
- 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>
5b353d1 to
af787c1
Compare
- 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>
af787c1 to
8dfd2c4
Compare
|
@sellakumaran please review once & suggest accordingly. |
| The body still says "Azure CLI user, tenant, subscription" but those are no longer printed. Update the description so reviewers and future readers see what the merged feature actually does. -- |
|
Thanks for the rework @beyondszine — you've addressed every v1 blocking concern, added tests, fixed the lock pattern in AuthenticationService, and the CommandStringHelper for secret redaction is a nice utility. This is great. Two architectural concerns left before I can sign off, both about scope.
The panel prints "Config: " / "Config: not found at " on every --verbose run (when config file is not there in local folder, initial state), and its tenant and agent-identity rows read from a365.config.json. We've been deliberately reducing the user-facing dependency on that file — not requiring it, not advertising it, and not treating it as the source-of-truth for who-am-I. This PR walks that direction back into the operator UX. The IConfigService.LoadAsync fix from v1 solved the correctness issue, but the panel still has to reach into the config file to populate its rows, so the underlying tension stays. The good operator-trust signal is already in this PR — your JWT-derived line in AuthenticationService.LogAuthenticationContext ("Authentication context: API calls will use user X in tenant Y (cached token | interactive sign-in)"). That sources from the actual MSAL token, needs no file, and is the right feature for telling operators which identity their command is about to act under. Also note that the current setup command (and I verified with clean up as well) prints the following info already:
If we drop the panel, the PR collapses to:
That is the lean version of the same feature. Could you cut the Execution Context panel in this PR? Specifically:
Keep everything else. The PR then ships the JWT-based auth context line — the operator-trust signal the description was selling — without re-exposing a365.config.json or growing the surface to eight files for one log line. Happy to LGTM the scoped-down version. The work you've already done is not wasted — the helper, its tests, and the auth-context plumbing all stay. |
sellakumaran
left a comment
There was a problem hiding this comment.
See my last comment in PR
- 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>
8dfd2c4 to
0ec1410
Compare
- Program.cs: drop the dead --config/-c ResolveConfigPath parser (those flags were removed in microsoft#385) and resolve environment via ConfigService.GetConfigFilePath() in the tooling-service factory. - AuthenticationService.cs: factor the auth-context lock/dedup into a TryClaimContextChange(user, tenant) helper that returns the changed bool. - CHANGELOG: drop "subscription" (only user and tenant are logged), name a real flag (--idp-client-secret), and scope the redaction claim to the log separator.
Resolved conflicts in CHANGELOG.md and AuthenticationService.cs (Program.cs auto-merged): - AuthenticationService.cs: main's microsoft#430 rework removed the CLI's disk token cache, so the auth-context audit line is re-hooked to a single call site before the final return in GetAccessTokenAsync. Dropped the now-unusable "(cached token | interactive sign-in)" source suffix — the CLI can no longer tell whether MSAL served the token silently. Reused main's TryExtractUpnFromJwt and JwtHelper.TryDecodeClaim(..., "tid") instead of the branch's duplicate JWT decode helpers. Kept the dedup fields and TryClaimContextChange helper. - CHANGELOG.md: kept both this branch's entries and main's develop-mcp evaluate entry under [Unreleased] Added. Build: 0 warnings, 0 errors. Tests: 1902 passed, 12 skipped, 0 failed.

Summary
Two hardening changes to the CLI diagnostic log — no new commands or panels.
1. Secret redaction in the log separator
Each run writes a separator block to the log so operators can find where it begins.
That block included the raw command line, exposing values passed to secret options.
CommandStringHelper.FormatForDisplay()now replaces the value of any known secretoption (
--client-secret,--idp-client-secret,--password,--token,--access-token,--api-key,--refresh-token) with<redacted>, handling both--opt valueand--opt=valueforms.2. Authentication context log line
AuthenticationServicelogs the resolved user (UPN) and tenant atInformationlevel when the auth context changes — extracted from the MSAL token's claims (
upn/
preferred_username/unique_name,tid), so it reflects who the command actsas with no
a365.config.jsondependency and no credentials exposed. Deduplicated:fires only when user or tenant changes within a run.
Files changed
Helpers/CommandStringHelper.csFormatForDisplay()with secret-option redactionServices/AuthenticationService.csTryClaimContextChangededup; reuses sharedJwtHelper/TryExtractUpnFromJwtfor claimsProgram.csFormatForDisplay(); environment read resolves config viaConfigService.GetConfigFilePath()Tests/.../CommandStringHelperTests.csCHANGELOG.md[Unreleased]sectionScoped out during review
An earlier
az account show–based startup panel was removed — it re-reada365.config.json, ran a subprocess per command, and could disagree with the MSALtoken. The JWT line above delivers the same operator-trust signal without those
costs. The
(cached token | interactive sign-in)suffix was also dropped — tokenpersistence is delegated to MSAL's cache, so the CLI can't reliably tell the two
apart.
Testing
dotnet build— 0 warnings, 0 errorsdotnet test— 1902 passed, 12 skipped, 0 failed (full suite, after mergingmain)