Record which credential DefaultAzureCredential selected, on an event-id allowlist (#3218 follow-up)

[erikdarlingdata]

EntraDefaultCredential signs in as whichever Azure identity the machine already has, in an order the driver owns and this app cannot narrow. #3218 disclosed that in dialog text and in the README, and recommended instrumenting it as a follow-up. This is the follow-up: the credential type DefaultAzureCredential actually selected, in the log, and nothing else.

Observability only. No change to the connection string or the credential chain, and the only change to control flow is a try/finally around each open whose finally writes a log line.

The upstream event, re-verified at the tag rather than at main

Read at tag Azure.Identity_1.18.0 — the version the repository resolves, transitively, through Microsoft.Data.SqlClient.Extensions.Azure 7.0.2 (#3219). All four line citations I was handed check out exactly, which is worth saying because a main-vs-tag mismatch cost this queue three corrections in one day:

claim	verified at Azure.Identity_1.18.0
Credentials/DefaultAzureCredential.cs:178 raises DefaultAzureCredentialCredentialSelected(credential.GetType().FullName) on the success path	exact
AzureIdentityEventSource.cs:18 — source name Azure-Identity	exact
AzureIdentityEventSource.cs:34 — event id 13	exact
AzureIdentityEventSource.cs:307 — Level = EventLevel.Informational, one string credentialType	exact

An allowlist, because the level cannot narrow this and keywords do not exist here

The brief that reached me said event 13 shares Informational with 18 other events. The count is 19 others, and the framing understates the problem in a way that matters: EventListener.EnableEvents(source, level) admits every event at or above the requested severity, so Informational delivers 28 of that source's 29 events — 20 Informational, 4 Warning, 2 Error, 1 Critical, 1 LogAlways. Only the single Verbose event is excluded. So the level admits MsalLogError, MsalLogCritical, MsalLogAlways and ProcessRunnerError as well as the Informational siblings that carry tenant ids, account details, scopes, parent request ids and a formatted Exception.

And no event in that file declares Keywords — 0 of 29 — so EnableEvents has no dimension to exclude them on.

Every count above was taken twice, off two things that fail differently: parsing the [Event] attributes in the source at tag Azure.Identity_1.18.0, and reflecting over the shipped Azure.Identity.dll in the harness's own closure. They agree exactly — 29 events, 20 Informational, 28 delivered at Informational, 0 with Keywords — and the second instrument also confirms the assembly is 1.18.0+05d48fdf, so the line numbers and the histogram are about the same build. The callback filter is the only barrier, which is why it is an allowlist on EventId == 13 and not a denylist: a future Azure.Identity adding an event is precisely where a denylist fails and an allowlist survives.

Two barriers, not one. The allowlist guarantees the event; a shape check guarantees the value taken from it. Payload[0] is forwarded only if it looks like a namespace-qualified CLR type name — which rejects every sensitive sibling payload on a character a type name cannot contain: a tenant id has -, an account UPN has @, a scope has : and /, a message has spaces. That is the barrier that survives a payload reorder, which the id allowlist alone does not. Never eventData.ToString(), never the payload collection, never Message formatted with its arguments.

The sharpest evidence that the id is doing the work: ManagedIdentityCredentialSelected (event 26, Informational) has a first payload slot that is also called credentialType and also holds a real credential type name. It passes the level, the source name and the shape check. Only the id keeps it out, and a test raises it to prove that.

Both lifetime decisions

The listener's window is one connection open, not the process. While the source is enabled, AzureIdentityEventSource.IsEnabled(Informational, …) answers true inside Azure.Identity and it performs the formatting work its seven if (IsEnabled(…)) guards otherwise skip. A monitoring tool that left it on would pay that on every token operation for the life of the process, forever, to learn a fact that can only be reported once. Begin is called immediately before the open; the listener is disposed immediately after it, because an undisposed EventListener keeps receiving events. The cost of the narrow window is that an event raised outside it is missed — which is why both of Lite's connection-open sites carry one rather than only the dialog a user is looking at.

Only the last reported name outlives the attempt. The listener is gone and the captured value is read once. One static string exists so an unchanged repeat is Debug rather than Information: the selection is a process fact, and a monitoring tool restating it once per server per sweep would be noise. It is claimed with Interlocked.Exchange rather than read and then written — one raised event is delivered to every attached listener and CheckAllConnectionsAsync checks servers concurrently, so two concurrent first connections capture the same name and a read-then-write pair lets both decide they are first. A changed name still reports at Information — the driver clears its credential cache (ActiveDirectoryAuthenticationProvider.cs:136-138), so a different source genuinely can start winning, and that is a different identity connecting.

Why it reports at most once, stated rather than discovered. Two process-wide caches sit above the event. DefaultAzureCredential raises event 13 only on the branch with no cached credential (DefaultAzureCredential.cs:168-179); once _credentialLock holds a value, later requests take the HasValue branch and raise nothing. And SqlClient caches the DefaultAzureCredential instance in a static map keyed by authority, scope, audience and client id (ActiveDirectoryAuthenticationProvider.cs:29, :258-262). So the event fires at most once per process per key, and a second connection to the same tenant reports nothing at all. That absence is a Debug line saying why, not silence.

Two sites out of about twenty, and that is sound for a reason rather than by hope. Lite has roughly twenty new SqlConnection sites that reach a monitored server. Only two are instrumented, which only works because the connectivity check gets there first — and it does, by construction: CollectionBackgroundService.ExecuteAsync runs ServerManager.CheckAllConnectionsAsync before RemoteCollectorService.RunDueCollectorsAsync on every cycle including the first, and that loop is the only thing that drives collection. The interactive first-touch paths are the dialog's Test button and MainWindow's explicit retry, and both go through an instrumented open. A reorder of that loop would break observability with nothing to notice — the app connects normally, the listener attaches normally, and the event has already been raised and discarded, permanently — so EntraCredentialSelectionOrderingTests pins it.

What that still does not cover, said rather than implied: the bulk-add dialog, the excluded-databases dialog, a server tab's ad-hoc reads and the plan fetcher all open connections too. Each needs an already-configured server, so in practice the sweep has run first; a user who drives one inside the background service's five-second startup delay could acquire the first token there. The consequence is the Debug "not observed" line rather than a wrong one, which is the direction to fail in, and it is why that line names the caching as the expected cause instead of asserting it.

Reported on the failure path as well, and that is where it matters most. Azure.Identity raises event 13 when it acquires a token — before SQL Server has accepted or rejected the identity that token names. So "DefaultAzureCredential picked the wrong ambient identity", which is the exact complaint this feature exists to answer, arrives as a login failure with the selection already captured. Reporting only after a successful open dropped precisely that case; both sites now report from a finally. It is also the only chance to see it: the driver caches the credential the moment a token exists, so a retry raises nothing at all. Report takes nothing but the listener — it has no success argument to get wrong — and it does no I/O, so a finally cannot mask the connection's own exception. There is deliberately no blanket catch in it: one would make the feature silently dead at the moment it broke.

Which paths log, and which deliberately do not

"Logs on success and on failure" is true and is not the claim. The claim is it logs whenever a selection occurred, on either path, and it never records a selection that did not occur.

Path	What is written
Selection observed, open succeeded	Information, naming the credential type — or Debug "(unchanged)" if it repeats a name already reported this process
Selection observed, open failed	identical. This is the primary case: the token is acquired, and the event raised, before SQL Server accepts or rejects the identity it names
No selection observed — a repeat connection on the driver's cached credential, a chain that found nothing so raised no event, or a pre-token failure (DNS, network, timeout)	one Debug line naming the caching as the expected cause, carrying no credential name. Debug is below the default minimum, so on a default install this path writes nothing at all
Event 13 raised but Payload[0] failed the type-name shape check	one Debug line saying the payload was not a type name, not carrying the value
Any mode other than EntraDefaultCredential	nothing, ever. Begin returns null, Report no-ops, and the event source is never enabled
Lite's other four SqlConnection sites (bulk-add dialog, excluded-databases dialog, server-tab ad-hoc reads, plan fetcher)	not instrumented. See the ordering section above for why that is sound and what it costs

Three pins hold the "never records a selection that did not occur" half, each falsified: the not-observed line written at Information instead of Debug (Failed: 6), that line rewritten to claim a selection and name it unknown (Failed: 3), and the refused-payload line rewritten to name the refused value (which cannot leak, for a structural reason recorded in the file). The Debug explanation is deliberately not silence — an operator who lowers the level and goes looking should find "the event fires once per credential instance and the driver caches the instance" rather than an absence they have to explain.

No server name on the line. The selection is scoped to the process and the driver's cache key, not to a server: the chain resolves from environment variables, an az login session and machine identity, none of which are per-server. Naming the server that happened to observe it would assert a per-server fact that is not true.

The gate keys off the builder, not the mode string. SqlAuthenticationMethod.ActiveDirectoryDefault is the keyword the driver acts on, so the gate cannot disagree with the connection string — and it follows a credential profile into this mode for free if one is ever offered there (today profiles offer SQL, service principal and managed identity only). The one degree of indirection that buys is closed by a test that runs the real ServerConnection.ApplyAuthentication for every mode reflected off AuthenticationTypes and asserts exactly one attaches a listener.

How the pin is falsified in both directions

Every pin raises the real event on the real event source: Azure.Identity's AzureIdentityEventSource is internal, so it is reached by reflection and its actual methods are invoked. The id, the level, the payload shape and the source name are the ones that ship.

• Positive — event 13 raised, the type name must arrive. Raised with the source already created, so OnEventSourceCreated is reached from the EventListener base constructor: the path on which a derived field assigned in a constructor body is still null and the listener silently never enables. Both values that callback needs are const for exactly that reason.
• Negative, on the same listener — three siblings raised first (ManagedIdentityCredentialSelected, whose payload passes every check but the id; TenantIdDiscoveredAndUsed, which carries tenant ids; UserAssignedManagedIdentityNotSupported, at Warning), none forwarded — then event 13, which must be. A listener that simply received nothing fails the last assertion, so the first three are evidence of filtering rather than of silence.
• An independent instrument for the level claim — a recording listener enabling the same source at the same level records every id delivered, so "the sibling was not forwarded" can be told apart from "the level never delivered it". Without it the callback filter could be deleted with nothing noticing.
• Enables Azure-Identity and nothing else — a bystander EventSource must be un-enabled while Azure-Identity demonstrably is. A dropped name check would turn this into a listener that enables every source in the process, and every other pin here would stay green.
• The upstream contract — source name, method signature, [Event] id and level read off the shipped assembly; and that no event declares Keywords, with the event count as the control. This is the pin that makes Azure.Identity is unpinned and transitive, so #3218's broker-free guarantee rests on a dependency-graph fact nothing tracks #3219's unpinned transitive dependency loud instead of silent.
• Payload slot 0 is credentialType, by NAME, on a real written event — read off EventWrittenEventArgs.PayloadNames rather than the reflected signature, because the name the runtime attaches to that position is what the positional read actually gets. The runtime does report payload names for this event (measured). A parameter added or reordered ahead of credentialType is declined at runtime by the shape check; this is what makes it loud rather than merely survivable.
• Both call sites, in order, and reporting on both paths — asserted on StripCommentsAndStrings' output, because both sites carry a comment naming EntraCredentialSelectionLog and a raw substring search would be satisfied by the comment with every call deleted (Record and classify Entra MFA broker failures instead of showing one line and logging nothing (#3196) #3201's defect). Three discriminating assertions: Begin before the open (a Report placed first captures nothing forever while reading as fully instrumented); Report after it; and a try/finally in the window between them, because Begin-before-open plus Report-after-open is satisfied by a Report on the success tail. Asserted on the window rather than the method body on purpose — RunConnectionTestAsync already had an unrelated finally for re-enabling its buttons, which would have satisfied a body-wide check.
• Through the real AppLogger, at the default level — the first observation must reach the log; the unchanged repeat must not. Nothing else would catch a Report that wrote every line at Debug: the messages would still be correct and the whole feature would be invisible on every shipped install.

Mutation table

Baseline before every mutation, in a net10.0 xunit.v3 harness compiling the real shipped .cs under AssemblyName=Lite.Tests (so InternalsVisibleTo applies) with versions from Directory.Packages.props:

Total: 57, Errors: 0, Failed: 0, Skipped: 0, Not Run: 0 for the table below, and Total: 58, Errors: 0, Failed: 0, Skipped: 0, Not Run: 0 after the payload-name pin was added, for the three rows that follow it.

Each mutation was applied by unique-anchor byte replacement, confirmed present by git diff --numstat and by content occurrence count, rebuilt, run, restored with git checkout --, and confirmed gone by content plus an empty git status.

Mutation	Result
allowlist on EventId removed	Failed: 1
allowlist replaced by a denylist of today's known-sensitive ids	Failed: 1
payload shape check removed	Failed: 1
shape check widened by ONE character, so a hyphen passes	Failed: 1
source-name check dropped from OnEventSourceCreated	Failed: 1
source name moved off const into a constructor-body field, read from the callback	Failed: 6
Report writes every line at Debug	Failed: 1
Report never persists the reported name	Failed: 2
Begin returns a listener for every authentication method	Failed: 11
Decide reports an unchanged repeat at Information	Failed: 3
Decide trusts the listener's bool instead of re-reading the rule	Failed: 3
the listener retains a refused payload anyway	Failed: 1
ServerManager reports BEFORE the open	Failed: 1
AddServerDialog's whole scope deleted, its explaining comment kept	Failed: 1
instrument: Raise() made a no-op, so no real event is written	Failed: 5
instrument: the reflection target aimed at a type that does not exist	Failed: 9
instrument: log reader filters on a source never written	Failed: 2
instrument: call-site pin reads RAW source, dialog's calls deleted, comment kept	Failed: 1
instrument: sibling pin loses its positive control AND the listener never enables	Failed: 5
instrument: enabled-level pin's floor control >= 3 → >= 0 AND the listener never enables	Failed: 6
instrument: keywords pin loses its count control AND enumerates nothing	Failed: 0
payload-name pin expects a different name (is the assertion live?)	Failed: 1
payload-name pin's captured VALUE assertion expects the wrong credential	Failed: 1
instrument: the capture listener discards payload names, count assertion deleted	Failed: 1
ServerManager reverted to reporting on the success tail (the defect review found)	Failed: 1
AddServerDialog reverted to reporting on the success tail	Failed: 1
Report gains a success flag and gates on it	Failed: 1
instrument: the finally assertion reads the whole method body instead of the Begin→Report window, and the dialog reverts	Failed: 0
dedupe claimed by read-then-write instead of Interlocked.Exchange	Failed: 0
the collection loop genuinely collects before it checks connections	Failed: 1
the same inversion plus a comment naming the check, strip intact	Failed: 1
ServerManager stops attaching the listener at all	Failed: 2
instrument: ordering pin drops its strip, inverted loop carries a comment naming the check	Failed: 0

Baseline: Total: 59 for the finally rows, Total: 61 for the ordering rows, Errors: 0, Failed: 0, Skipped: 0, Not Run: 0 in both cases.

Re-run in full against the head, after the finally move

Moving a call changes which paths the pins cover, so the whole table was regenerated against the current source and re-run rather than assumed to survive — 27 mutations, baseline Total: 61, Errors: 0, Failed: 0, Skipped: 0, Not Run: 0. 23 red. The four green are the same four disclosed above (Interlocked.Exchange, and the three instrument demonstrations); nothing went vacuous from the move, and in particular the success-tail revert, the whole-scope deletion and the success-flag mutation are all red at both sites.

Three more on the no-selection path, baseline Total: 62:

Mutation	Result
the not-observed line written at Information instead of Debug	Failed: 6
the not-observed arm claims a selection and names it unknown	Failed: 3
instrument: the new default-level pin loses its positive control	Failed: 0

The ordering rows are the clean three-way discrimination, and getting there took a correction worth recording: my first attempt at inverting the loop moved the collector call to just after the connection check and I read the resulting green as a vacuous pin. It was a vacuous mutation. The real inversion fails (Failed: 1); the same inversion with the strip removed and one comment added naming CheckAllConnectionsAsync passes (Failed: 0); and the same inversion with the strip and the comment fails again (Failed: 1) — so it is the strip, not the absence of the comment, doing the work. No comment in that file names either identifier today, so the strip is invisible until someone reordering the loop writes a one-line ordering note, which is exactly the case the pin exists for.

Three more rows are green and each is disclosed rather than explained away. The body-wide finally check is the demonstration that the window matters: RunConnectionTestAsync's own button-re-enabling finally satisfies it while the selection is dropped on every failed open. And the Interlocked.Exchange claim is not pinned by anything — the race is two concurrent first connections capturing one delivered event, which is not reproducible on demand, so the argument is the exchange's atomicity and the sequential form of the same path is what the suite covers. Said here rather than left to look tested.

The keywords row is green on purpose and is the demonstration, not an escape: with the event-count control removed and the enumeration aimed at a type carrying no events, "no event declares Keywords" passes while asserting nothing. That control is why the real pin is not decoration.

Two pins went green that should have been red, and mutation is what found them

Every rejection case for the shape check was also missing a dot, so the dot requirement alone rejected it and the character allowlist was never under test — widening it by a hyphen left all 44 tests green. Fixed by ten rows that are each a well-formed dotted name differing from an acceptable one by exactly one illegal character, so each character in the allowlist is load-bearing.

Decide re-reads the shape rule rather than trusting the listener's verdict, and nothing exercised that re-read — keying the refusal off the bool alone was green. Fixed by handing Decide a value that is not a type name with rejectedPayload: false, which is the shape a listener that stopped applying the rule would produce.

A third mutation — interpolating the refused value into Decide's message — also went green, and that one is not a gap: the refused value never leaves OnEventWritten (the listener stores a bool, not the string), so Decide structurally cannot name it. The test's DoesNotContain there is a consequence of the design rather than an independent check of it, and now says so in the file.

Mutation also corrected a claim I had written about a test. I had called the payload-name pin's names.Count > 0 assertion a control against a vacuous index check; discarding the names and deleting the assertion still failed, because names[0] on an empty list throws. It is not a control — it names the cause instead of leaving an index-out-of-range to diagnose — and the file now says that rather than the flattering version.

Counts in comments

Every count in the new file names the version it was measured at, because a count in prose is a partial list with a numeral welded on. One was simply wrong on the first pass: the cost note said Azure.Identity has seven if (IsEnabled(…)) guards, and at 1.18.0 it has 21, plus 10 when IsEnabled(…) switch arms. Corrected, and the event histogram now records that it was counted two ways.

Review

Third round. claude[bot] found that EntraCredentialSelectionTests reads and mutates AppLogger's process-wide statics — it drains the buffer and asserts on what came out, and one pin moves the minimum — without joining the app-logger-statics xUnit collection that LiteLogLevelGateTests created for exactly that. Verified: that class's own remarks state the invariant this PR falsified, in so many words — "this class is the only reader of the sink anywhere in the suite" — and name the trigger, "the condition that would make it bite is any of those five reading the log, and then they join app-logger-statics".

Both mechanisms are real and neither is reproducible on demand: DrainBufferedLines is destructive, so a concurrent sweep dequeues the line this class is about to look for and tag-filtering on the log source cannot recover it; and those sweeps set the minimum as far as None, so the Information line this class expects to be admitted is never enqueued at all. Two green CI runs do not disprove a flake. Fixed by joining the collection — which means the name now serialises two classes rather than nothing — and by correcting the invariant in LiteLogLevelGateTests' own remarks, rather than leaving a false claim for the next person to reason from. And the listener needed the same collection, for a second reason. The EventListener/EventSource pair is process-wide too: enabling a source is a global effect, and a raised event is delivered to every listener attached to it anywhere in the process. EntraCredentialSelectionModeGateTests calls Begin, so it constructs real listeners on the same source that EntraCredentialSelectionTests raises real events on — so it joins, not for the AppLogger reason (it touches none) but because a class can belong to only one collection, and a separate name for the event-source hazard would have failed to serialise it against the class that matters. It changes no assertion there today, because those pins only ask whether Begin returned a listener and never read what one captured; the condition that would make it bite is reading SelectedCredentialType/RejectedPayload or calling Report. That widening is recorded in the collection's home file as well, so the name is not the only thing describing what it covers.

Measured rather than assumed: these are the only EventListener subclasses and the only Azure-Identity consumers anywhere in the repository, so no third party can observe the events raised here today.

EntraCredentialSelectionOrderingTests stays out, and is the case that shows the rule is about reach rather than subject matter: it is entirely about this feature and reads source files only — no AppLogger, no listener, no event source, nothing process-wide to share.

First round. claude[bot] found a real defect and it is fixed above: Report reached only the success path, which drops the selection in the one case the feature exists for. Both call sites now report from a finally, and the call-site pin was extended to require it — the previous pin passed with the defect restored, which is the part worth recording.

Its second finding — s_lastReported written with only volatile while CheckAllConnectionsAsync runs concurrently — is also fixed, though the analysis was slightly off in a way that makes it worse than "cosmetic": one raised event is delivered to every attached listener, so two concurrent first connections do not merely race on the write, they both legitimately capture the same name. Interlocked.Exchange now claims it.

The one CI failure, and what it says about local verification

build went red on c849c9f13 at Darling.Tests.DocCommentHygieneTests.NoMemberCarriesTwoStackedSummaryBlocks — pointing at Lite.Tests/EntraCredentialSelectionTests.cs:792. Run Lite tests in the same job passed; the failure was Darling's whole-tree doc guard reading Lite source.

The cause is worth naming because it is a mechanism, not a typo: two separate later insertions anchored on private sealed class RecordingListener : EventListener — the class declaration — rather than on the line above its doc comment. Each insertion therefore pushed that comment further up and left the class undocumented behind a stack of summaries. XML docs take the last summary, so tooling read correctly and only a human reading the file was misled. The guard's own message warns against deleting the first block for exactly this reason, so it was moved back onto the member it documents, not removed.

It also exposed a gap in my local verification, now closed: I had been running my own suite and not the whole-tree guards that read my files from the other app's suite. Fixed by compiling the real DocCommentHygieneTests.cs plus CSharpSourceWalker.cs into a throwaway net10.0 console shim — the guard walks up from the test binary looking for PerformanceMonitor.sln, so the shim has to live inside the tree (Darling/tools/pg-harnesses/, which is gitignored). 76 of that class's 77 tests pass; the 77th, TheDerivedProjectListCoversEveryProjectInTheTree, fails naming the shim's own .csproj, because it enumerates every .csproj on disk. The shim was deleted afterwards for that reason — leaving it would fail an unrelated real guard on the next local run.

Verified red-first: restoring the displaced state fails with the same two line numbers CI reported (792, 801); the fix passes.

What only CI can confirm, and what nobody here can

Lite.Tests targets net10.0-windows and cannot run on macOS. EntraCredentialSelectionModeGateTests is a separate file because it needs ServerConnection, whose closure reaches Windows-only credential storage — so the mode → ApplyAuthentication → gate link ran in build only, not locally. Everything else ran locally against the real Azure.Identity 1.18.0 assembly.

Execution in CI confirmed by test-count delta, since MTP prints no Passed lines: Lite.Tests reports 3568 on dev at 91c0a919a (run 34371264224) and 3631 here, Failed: 0, Not Run: 0. The delta of 63 is exactly 19 [Fact] + 30 [InlineData] rows + 10 [MemberData] rows in EntraCredentialSelectionTests, 2 in EntraCredentialSelectionOrderingTests and 2 in EntraCredentialSelectionModeGateTests.

And a note on which check to read: Darling Linux build (11s) and Darling PostgreSQL tests (17s) both take build.yml's path fast path, because the diff touches no Darling/** and no shared project. Their green says nothing about this change. build is the check that ran my content, at ~8.5 minutes.

And the part no test reaches: that a live Entra tenant issues a token to a real az login session through this path, and that the type name which then lands in the log is the one a user would recognise. Still unverified against a live tenant — the limit #3196, #3214 and #3218 all had.

CHANGELOG

Not edited, per lane convention. Entry text:

• Lite now records which Azure credential DefaultAzureCredential selected for an Existing Sign-In (az login) connection ([Add a broker-free Entra sign-in path: ActiveDirectoryDefault returns before the WAM broker is built (#3214) #3218] follow-up, and see [Azure.Identity is unpinned and transitive, so #3218's broker-free guarantee rests on a dependency-graph fact nothing tracks #3219]) - the mode signs in as whichever identity the machine already has, in an order the driver owns and the app cannot narrow, which the connection dialog and the README warned about and nothing recorded. A narrowly-scoped EventListener enables Azure.Identity's Azure-Identity event source for exactly the duration of one connection open, forwards event 13 and nothing else, and writes the selected credential's type name - AzureCliCredential, EnvironmentCredential, ManagedIdentityCredential and so on - through AppLogger. The filter is an allowlist on the event id rather than a level or a keyword because neither can narrow this: event 13 is itself Informational, so that is the lowest level which delivers it, EnableEvents admits everything at or above the level requested (28 of that source's 29 events), and not one event in it declares Keywords - so the callback filter is the only barrier between the app's log and the siblings carrying tenant ids, account details and raw exceptions. Payload[0] is the only value read and only if it has the shape of a CLR type name, which is what survives a future payload reorder. Expect one line per run of the app: Azure.Identity reports the selection once per credential instance and the driver caches that instance in a process-wide static, so later connections have nothing new to report and say so at Debug rather than silently. Observability only - the connection path, the connection string and the credential chain are unchanged, and no other authentication mode enables the event source or pays anything for this

[claude]

Reviewed this PR (Lite-only, C#, no T-SQL changes — the CONTRIBUTING.md T-SQL style rules don't apply here). Summary:

Correctness — found and flagged inline (both call sites): EntraCredentialSelectionLog.Report(...) is only invoked after await connection.OpenAsync() returns successfully, in both ServerManager.CheckConnectionAsync and AddServerDialog.RunConnectionTestAsync. If OpenAsync() throws — which is exactly what happens when DefaultAzureCredential selects a source that SQL Server then rejects (wrong tenant/identity, missing permission) — control jumps to the exception handlers and the captured selection is silently dropped, never logged. That's the single scenario this feature (#3218 follow-up) exists to make legible: "it connected as the wrong ambient identity." Suggested fix on both inline comments: move Report into a finally (or wrap in try/finally) so it runs on both the success and failure paths.

Lite/Darling parity: No drift. Darling never builds an Azure-token connection at all (ActiveDirectoryDefault/EntraDefaultCredential is rejected at the credential step per the README note this PR adds to, and confirmed — Darling.Tests/ViewerServerManagementTests.cs only exercises the rejection path), so there's no counterpart code for Darling to keep in sync with here.

Security: The event-id allowlist + payload shape-check design (reject anything without the shape of a dotted CLR type name) is sound and well-tested — it structurally excludes the sensitive sibling Azure-Identity events (tenant ids, UPNs, scopes, exception text) rather than relying on the informational level or keywords, neither of which can narrow this event source. No secrets, connection strings, or tokens touch the log path; only the credential type name (e.g. AzureCliCredential) is ever forwarded.

Minor/non-blocking: EntraCredentialSelectionLog.s_lastReported is a static field written from Report() with only volatile, no lock. CheckAllConnectionsAsync runs connection checks concurrently, so if multiple EntraDefaultCredential servers are checked at once on first run, it's theoretically possible for two Report() calls to race and both log at Information instead of the second deduping to Debug. Cosmetic (a possible duplicate log line), not worth blocking on.

Test coverage for the listener/allowlist/shape-check/gate logic is thorough.

[claude]

Review

Reviewed the diff (Lite/Helpers/EntraCredentialSelection.cs, the ServerManager.cs / AddServerDialog.xaml.cs call sites, both test files, and the README addition) against CONTRIBUTING.md. This PR has no T-SQL, so the T-SQL style rules don't apply; the C# in scope follows the repo's conventions (copyright header present, _camelCase/s_ private field naming, multi-type-per-file already has precedent in Lite/Helpers/QueryLogger.cs and MethodProfiler.cs).

Correctness

• The event-id allowlist + IsCredentialTypeName shape check are sound: the character set (ASCII letter/digit, ., _, +, requiring an interior dot) rejects every sibling payload shape the tests exercise (tenant id, UPN, scope, message), and generic type names.
• The try/finally at both call sites correctly reports on the failure path too, which is the case the feature exists for (token acquired → event raised → SQL Server then rejects the identity). Verified ServerManager.CheckConnectionAsync's outer catch still handles the OpenAsync exception normally; the inner finally doesn't swallow it.
• EntraCredentialSelectionLog.Report's Interlocked.Exchange-based dedup correctly avoids a double Information log when two connections race on first-observing the same credential (traced through both branches of the compensating Decide re-check).
• Begin keys off builder.Authentication == ActiveDirectoryDefault rather than the app's own mode string — confirmed ServerConnection.cs:561 sets that property directly for EntraDefaultCredential, so the gate can't drift from what the driver acts on.

Lite/Darling parity

• No concern: README.md already states Darling doesn't offer EntraDefaultCredential (its connect path builds Windows-integrated/SQL-login only), so there's no Darling counterpart for this listener to have.
• EntraCredentialSelectionTests.cs reading Lite/Services/ServerManager.cs / Lite/Windows/AddServerDialog.xaml.cs as text is a same-app read (Lite.Tests → Lite/), not a cross-app one, so CrossAppGuardCiGateTests's filter-reachability requirement doesn't apply to it.

Security

• Only Payload[0] is ever read, and only after the shape check; never .ToString()/.Message/the raw payload collection. No connection string, secret, or token is logged — matches the "observability only" framing.

No blocking issues found. Nothing to flag inline.

[claude]

Reviewed. This is a C#/Lite-only change (no T-SQL, so the collector-query style rules don't apply here).

Correctness — the two instrumented call sites (ServerManager.CheckConnectionAsync, AddServerDialog.RunConnectionTestAsync) correctly build the EntraCredentialSelectionListener from the same SqlConnectionStringBuilder instance used to open the connection, attach it before OpenAsync, and report from a finally so a failed login (the case this feature exists for) still gets logged. Traced the Report/Decide/Interlocked.Exchange dedup path by hand for the concurrent-first-connection race described in the comments (two servers racing to be the first ActiveDirectoryDefault open in the process) — it holds: exactly one caller ends up logging at Information regardless of exchange order, everyone else recomputes against the just-written value and downgrades to Debug. No bug found there.

One thing worth being aware of (not a bug, and it looks deliberate): EventListener delivery is process-wide, not scoped to the connection that attached the listener, so if two different ActiveDirectoryDefault connections to genuinely different tenants/authorities race for the first token acquisition in the process, either listener could observe the other's event. The log message is explicitly documented as process-scoped rather than per-server ("No server name on the line"), so this doesn't produce a wrong attribution claim — just flagging that I traced through this exact edge case and it's consistent with the stated design, not an oversight.

Security — the allowlist-on-event-id + shape-check-on-payload design is sound: only Payload[0] from event id 13 on the Azure-Identity source is ever read, and only when it matches a CLR type-name shape (rejects tenant ids, UPNs, scopes, exception text on the first illegal character). No tenant/account/scope data can reach the log through this path.

Lite/Darling parity — checked whether this needed a Darling counterpart. It doesn't: Darling's ServerStoreCredentialTests.MapAuth_AzureModes_AreUnsupported confirms EntraDefaultCredential is explicitly rejected as unsupported in Darling (DarlingServerConnector/MonitoredServerConnection only build Windows-integrated or SQL-login connections), and the README update in this PR correctly notes that. No drift.

Style — EntraCredentialSelection.cs holds three related types (listener, static log helper, result record struct) in one file, which is technically against the CONTRIBUTING.md "one class per file" C# guideline, but matches the existing convention in Lite/Helpers/ (e.g. MethodProfiler.cs, QueryLogger.cs each pair a static class with a small companion type), so not flagging it.

Nothing blocking. Nice work on the mutation-testing coverage in the test files.

[claude]

Review

This is a well-scoped, observability-only change: an EventListener on Azure.Identity's internal Azure-Identity source, filtered by an id allowlist (event 13) plus a payload shape check, wired into the two connection-open sites that can be first to acquire a token (ServerManager.CheckConnectionAsync, AddServerDialog.RunConnectionTestAsync). Verified against the working tree:

• The Microsoft.Data.SqlClient.Extensions.Azure version claim (7.0.2, transitively unpinned Azure.Identity) matches Directory.Packages.props:19.
• The ordering claim the tests pin (CheckAllConnectionsAsync before RunDueCollectorsAsync) matches CollectionBackgroundService.cs:171/181.
• CheckAllConnectionsAsync does run connections concurrently via Task.WhenAll (ServerManager.cs:594-600), which is what makes the Interlocked.Exchange dedup race in EntraCredentialSelectionLog.Report load-bearing rather than defensive — traced it through and the logic is correct (first writer keeps its Information line, later writers of the same value on Exchange collision correctly downgrade to Debug via a re-Decide).
• IsCredentialTypeName's character allowlist correctly excludes every character present in the sensitive sibling payloads it's meant to keep out (hyphens in tenant ids, @ in UPNs, :// in scopes, whitespace in messages/MSAL log lines, backtick+brackets in generic type names) — this is a solid second barrier behind the event-id allowlist.
• No Lite/Darling parity concern: Darling doesn't offer EntraDefaultCredential at all (unchanged README line just below the edit — "Darling does not offer this mode... rejects every Azure mode at the credential step"), so this is correctly Lite-only.
• No T-SQL in this PR, so the collector style conventions don't apply here.

One finding posted inline: EntraCredentialSelectionTests.cs reads/mutates AppLogger's process-wide static buffer and minimum level but doesn't join the app-logger-statics xUnit collection that LiteLogLevelGateTests established specifically for this hazard — and that class's own doc comment states the invariant this PR now violates ("this class is the only reader of the sink anywhere in the suite"). This is a likely source of intermittent CI flakes rather than a functional bug.

I wasn't able to run dotnet build/dotnet test in this sandbox (command execution was blocked), so this review is static/read-only — worth confirming the new test suite actually passes in CI.

🤖 Generated with Claude Code

[claude]

Reviewed. This is a tightly-scoped, purely additive observability change (one EventListener on Azure-Identity event 13, a shape-checked allowlist, and a try/finally at the two connection-open sites that get there first). No changes to the connection string, credential chain, or control flow beyond logging.

Checked and found no issues:

• Correctness: The Interlocked.Exchange-based dedup in EntraCredentialSelectionLog.Report correctly handles the concurrent-first-connection race (two servers racing to report the same newly-observed credential name resolve to one Information line + one Debug "unchanged" line, verified by tracing both interleavings). The try/finally placement at both call sites (ServerManager.CheckConnectionAsync, AddServerDialog.RunConnectionTestAsync) correctly reports on both success and failure paths, and is nested inside the existing outer try/catch without altering existing error handling.
• Security: The event-id allowlist (13 only) plus the independent IsCredentialTypeName shape check on the payload is a solid double barrier against the 19 other Informational-or-above sibling events on Azure-Identity that carry tenant ids, account details, scopes, and formatted exceptions — since none of those events declare Keywords, the callback-level allowlist is correctly identified as the only available barrier (EnableEvents alone can't discriminate). Only Payload[0], and only when it passes the dotted-type-name shape check, ever reaches AppLogger.
• Lite/Darling parity: No parity drift. This only touches Lite. Confirmed EntraDefaultCredential is explicitly unsupported on the Darling side (Darling.Tests/ViewerServerManagementTests.cs — ServerStoreCredential.MapAuth returns null/unsupported for it), consistent with the README's existing statement that Darling's connect path never acquires Azure tokens. Nothing here needed a Darling counterpart.
• Coverage gap, but disclosed: Several other SqlConnection.OpenAsync sites in Lite (bulk-add dialog, excluded-databases dialog, ad-hoc reads, plan fetcher) aren't instrumented, so a user hitting one of those first (e.g., during the background service's startup delay) would get a Debug "not observed" line instead of the real selection. This is explicitly called out in the EntraCredentialSelectionLog doc comment as a known, accepted tradeoff (fails toward a missing-but-harmless line, not a wrong one), so I'm not flagging it as a defect — just confirming the documented limitation is accurate.

No blocking findings.

[claude]

Review

Scope: Lite-only observability addition — logs which DefaultAzureCredential credential type won, via an allowlisted EventListener on Azure.Identity's Azure-Identity event source (id 13 only), reported from ServerManager.CheckConnectionAsync and AddServerDialog.RunConnectionTestAsync.

Checked in detail:

• Event-source gate correctness: OnEventSourceCreated/CredentialSelectedEventId are const, avoiding the classic EventListener derived-field-not-yet-assigned trap; EnableEvents(source, Informational) with no keyword filter is correctly reasoned (no events in that source declare keywords, so level is the only usable coarse filter and the id check is the real barrier).
• Payload allowlist (IsCredentialTypeName): correctly rejects every sensitive sibling shape (tenant ids, UPNs, scopes, exception text, generic type names) via the dotted-ASCII-only rule; boundary length check verified.
• Dedup/race logic in EntraCredentialSelectionLog.Report: the Interlocked.Exchange + re-Decide pattern correctly collapses concurrent first-observations of the same credential type into one Information line and the rest Debug, including under CheckAllConnectionsAsync's concurrent per-server checks. Walked through several interleavings; didn't find a case producing a duplicate Information line or a lost first observation.
• Call sites: both ServerManager.cs and AddServerDialog.xaml.cs attach the listener before OpenAsync and report in a finally, so a failed login (the case this feature exists for) still gets the selection logged. Gate (Begin) keys off builder.Authentication == ActiveDirectoryDefault, which matches exactly what ServerConnection.ApplyAuthentication sets for EntraDefaultCredential — confirmed by reading that mapping.
• Lite/Darling parity: intentionally Lite-only; README explicitly states Darling's connect path never acquires Azure tokens and rejects Entra modes at the credential step, so there's no counterpart to update. No drift.
• Security: no tenant id, UPN, scope, or exception content can reach the log — enforced by both the event-id allowlist and the independent payload-shape check, each covered by dedicated negative tests. No secrets, file, or process I/O introduced.
• Style: matches CONTRIBUTING.md's C# conventions (naming, static field s_ prefix consistent with the rest of Lite/Services, one type per concern). N/A T-SQL sections since no SQL changed.

No correctness, security, or parity issues found. The test suite (EntraCredentialSelectionTests, EntraCredentialSelectionModeGateTests, EntraCredentialSelectionOrderingTests) is unusually thorough — it independently verifies the upstream event contract by reflection, raises the real sibling events to prove filtering rather than absence, and pins call-site ordering against brace-balanced source rather than a plain substring match. I don't have anything to add beyond what's already pinned.

One minor, non-blocking observation: s_lastReported is a single process-wide slot, so if two different servers legitimately resolve to different DefaultAzureCredential instances (different tenant/scope/client-id) that happen to select the same-named credential type, only the first is logged at Information — a later distinct-but-same-named selection reads as "unchanged." That appears to be the intended tradeoff per the Report doc comment (avoiding per-server noise), not a bug, but worth being aware of if this ever needs to be diagnostic per-server rather than "did the process-wide winner change."

[claude]

Reviewed. This is a tightly-scoped, well-tested observability addition — no correctness issues found.

Parity check: Confirmed this is intentionally Lite-only. Darling's MonitoredServerConnection.BuildConnectionString only ever builds SQL-auth or Windows-integrated connection strings (no SqlAuthenticationMethod reference anywhere in Darling/** outside tests), and ServerStoreCredential.MapAuth/IsSupported explicitly reject EntraDefaultCredential (and every other Azure mode) at the viewer boundary — pinned by ViewerServerManagementTests.MapAuth_AzureModes_AreUnsupported et al. README.md's existing "Darling does not offer this mode" line (unchanged context, not part of this diff) matches. No counterpart change needed.

Correctness:

• Both call sites (ServerManager.CheckConnectionAsync, AddServerDialog.RunConnectionTestAsync) attach the listener before OpenAsync and report from a finally, so a failed open (the case this feature exists for — wrong ambient identity) still captures the selection before the driver's token cache makes it unobservable again.
• EntraCredentialSelectionLog.Begin keys off builder.Authentication == ActiveDirectoryDefault, not the app's mode string, so it can't disagree with what the driver actually does with the connection string.
• Walked through the Interlocked.Exchange race-correction in Report() for concurrent first-observations from CheckAllConnectionsAsync's parallel sweep (same name, different names, 3-way) — in every case exactly one caller logs Information and the rest fall back to the "unchanged" Debug line. No double-logging or lost updates found.
• The allowlist (event id 13) + shape check (IsCredentialTypeName) is a real two-barrier design, not just asserted in prose — EntraCredentialSelectionTests raises the actual sensitive sibling events (tenant id, UPN-shaped, warning-level) through the real Azure.Identity event source via reflection and confirms none are forwarded, while event 13 still is on the same listener instance.

Security: No secrets, tenant ids, or account details reach the log — verified by the test suite's negative controls, and the shape check independently rejects every sensitive sibling payload shape on a character a CLR type name can't contain.

Nothing blocking. Only note is what the PR body already discloses: unverified against a live Entra tenant (needs a Windows host + real tenant), which is a pre-existing limitation of this whole auth mode, not something introduced here.