From 8bf3011626e0817ef8fd6ba90e1d5f9debf8cb4c Mon Sep 17 00:00:00 2001 From: Tomas Prokop Date: Wed, 15 Jul 2026 16:31:49 +0200 Subject: [PATCH] Structural auth guardrails so agents never trigger sign-in silently MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes the recurring failure mode where an AI coding agent driving txc CLI/MCP gets confused about Profile/Connection/Credential, invents new profiles/connections speculatively, or ends up stuck on an unattended device-code prompt when a token expires. Grounded in real usage from the alm-lab checkpoint scripts, which show the golden pattern: sign-in is always a deliberate human action (--device-code passed explicitly, never relying on auto-fallback), and existing profiles/connections are always listed before creating new ones. No agent-harness environment-variable detection is used anywhere — the fixes are purely structural and apply uniformly to any caller. Tier 1 (structural, safety-critical): - AuthLoginCliCommand: remove the silent browser-unavailable -> device-code auto-fallback. Now fails fast with a clear message instead of ever starting an unattended device-code flow. - DataverseAccessTokenService.AcquirePublicClientSilentAsync: remove the automatic interactive/device-code re-authentication attempted on MsalUiRequiredException during token refresh (the exact "token expires mid-session" case reported) — now always throws immediately with a manual-sign-in message. - HeadlessAuthRequiredException.BuildMessage: lead with the manual-human-sign-in rule before headless remedy details. - ConfigurationResolver / ProfileValidateCliCommand: extend "no profile" error messages with a list-before-create nudge (config profile list / config connection list). - Bake the Credential/Connection/Profile model, list-before-create, and manual-sign-in rules directly into --help text for AuthCliCommand, ProfileCliCommand, ConnectionCliCommand, ProfileCreateCliCommand. - CliSubprocessRunner: extract BuildStartInfo so the MCP-forced TXC_NON_INTERACTIVE=1 contract has direct unit test coverage (CliSubprocessRunnerTests) instead of relying only on indirect integration coverage. Tier 2 (optional documentation reinforcement): - docs/profiles-and-authentication.md: add a "Sign-in is manual" callout for agent-adjacent workflows. - MCP troubleshooting-patterns skill: add auth-failure anti-patterns (don't self-sign-in, don't create profiles speculatively). - CopilotInstructionsManager: add a short Authentication section to the generated copilot-instructions.md. Tests: 701/701 passing (698 pre-existing + 3 new CliSubprocessRunner regression tests). Full solution builds with 0 errors. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- docs/profiles-and-authentication.md | 21 +++++ .../Headless/HeadlessAuthRequiredException.cs | 10 ++- .../Resolution/ConfigurationResolver.cs | 7 +- .../Auth/AuthCliCommand.cs | 2 +- .../Auth/AuthLoginCliCommand.cs | 38 ++++++--- .../Connection/ConnectionCliCommand.cs | 2 +- .../Profile/ProfileCliCommand.cs | 2 +- .../Profile/ProfileCreateCliCommand.cs | 2 +- .../Profile/ProfileValidateCliCommand.cs | 5 +- src/TALXIS.CLI.MCP/CliSubprocessRunner.cs | 17 +++- .../CopilotInstructionsManager.cs | 12 +++ .../Internal/troubleshooting-patterns.md | 10 +++ .../Runtime/DataverseAccessTokenService.cs | 81 +++---------------- .../Commands/Auth/AuthLoginCommandTests.cs | 14 ++-- .../MCP/CliSubprocessRunnerTests.cs | 51 ++++++++++++ 15 files changed, 174 insertions(+), 100 deletions(-) create mode 100644 tests/TALXIS.CLI.Tests/MCP/CliSubprocessRunnerTests.cs diff --git a/docs/profiles-and-authentication.md b/docs/profiles-and-authentication.md index 7fbc01c4..f7b9e152 100644 --- a/docs/profiles-and-authentication.md +++ b/docs/profiles-and-authentication.md @@ -71,6 +71,27 @@ Unpin a repository-level profile with: txc config profile unpin ``` +## Sign-in is manual — including when a coding agent is driving `txc` + +`txc` never starts an interactive browser sign-in or a device-code flow on +anyone's behalf. `txc config auth login` and `txc config profile create --url` +only run an interactive flow when a human deliberately invokes them, in their +own terminal, and only when a browser is actually reachable — there is no +silent fallback to device-code if it isn't. If a token has expired or no +credential exists yet, every affected command fails fast with an error asking +for a **manual** `txc config auth login` (or `--device-code` explicitly, if +that's genuinely what's needed) instead of starting a flow no one is watching. + +If you are scripting or delegating work to a coding agent: + +- Sign in yourself, once, ahead of time — in a CI runner or agent sandbox, use + the [headless workflow](#headless-and-ci-workflow) below instead of + interactive sign-in. +- Before creating a new profile or connection, run `txc config profile list` + and `txc config connection list` first — reuse an existing one that already + targets the environment you need instead of letting an agent create one + speculatively. + ## Headless and CI workflow For CI runners and other non-interactive environments, isolate config and load the secret from an environment variable: diff --git a/src/TALXIS.CLI.Core/Headless/HeadlessAuthRequiredException.cs b/src/TALXIS.CLI.Core/Headless/HeadlessAuthRequiredException.cs index 1c48e949..66651c21 100644 --- a/src/TALXIS.CLI.Core/Headless/HeadlessAuthRequiredException.cs +++ b/src/TALXIS.CLI.Core/Headless/HeadlessAuthRequiredException.cs @@ -44,16 +44,18 @@ private static string BuildMessage(CredentialKind kind, string reason) .OrderBy(s => s, StringComparer.Ordinal)); return - $"Credential kind '{ToKebab(kind)}' requires an interactive TTY, " + - $"but this process is running in headless mode ({reason}). " + + $"Interactive sign-in requires a human to run it deliberately in their own terminal — " + + $"it is never performed automatically. Credential kind '{ToKebab(kind)}' requires an interactive " + + $"TTY, but this process is running in headless mode ({reason}). " + $"Permitted headless kinds: {permitted}. " + - "To run non-interactively, register a headless-capable credential with either " + + "To run non-interactively, ask a human to register a headless-capable credential with either " + "`txc config auth add-service-principal --alias --tenant " + "--client-id --secret-from-env ` or " + "`txc config auth add-federated --alias --tenant --client-id `, and bind it to the profile, " + "or supply the credential via environment variables " + "(AZURE_CLIENT_ID / AZURE_CLIENT_SECRET / AZURE_TENANT_ID for SPN, " + - "AZURE_FEDERATED_TOKEN_FILE for workload-identity federation)."; + "AZURE_FEDERATED_TOKEN_FILE for workload-identity federation). " + + "Otherwise, ask the human to run `txc config auth login` themselves in an interactive terminal, then retry."; } private static string ToKebab(CredentialKind kind) diff --git a/src/TALXIS.CLI.Core/Resolution/ConfigurationResolver.cs b/src/TALXIS.CLI.Core/Resolution/ConfigurationResolver.cs index 18afb37a..c16f8ac7 100644 --- a/src/TALXIS.CLI.Core/Resolution/ConfigurationResolver.cs +++ b/src/TALXIS.CLI.Core/Resolution/ConfigurationResolver.cs @@ -50,8 +50,11 @@ public async Task ResolveAsync(string? profileName, Canc if (name is null) { throw new ConfigurationResolutionException( - "No txc profile could be resolved. Pass --profile , set TXC_PROFILE, " - + "pin a workspace default with 'txc config profile pin', or select a global default with 'txc config profile select'.", + "No txc profile could be resolved. Run 'txc config profile list' and 'txc config connection list' " + + "first to check whether a suitable profile or connection already exists before creating a new " + + "one — confirm the target environment with the user if it's unclear. To resolve: pass --profile , " + + "set TXC_PROFILE, pin a workspace default with 'txc config profile pin', or select a global default " + + "with 'txc config profile select'.", ConfigurationResolutionFailureReason.NoProfile); } diff --git a/src/TALXIS.CLI.Features.Config/Auth/AuthCliCommand.cs b/src/TALXIS.CLI.Features.Config/Auth/AuthCliCommand.cs index 51b88075..aa0ef6a7 100644 --- a/src/TALXIS.CLI.Features.Config/Auth/AuthCliCommand.cs +++ b/src/TALXIS.CLI.Features.Config/Auth/AuthCliCommand.cs @@ -11,7 +11,7 @@ namespace TALXIS.CLI.Features.Config.Auth; /// [CliCommand( Name = "auth", - Description = "Manage Entra / Dataverse credentials stored in the OS vault.", + Description = "Manage Entra / Dataverse credentials stored in the OS vault — the \"who\". Interactive sign-in ('login') and device-code sign-in are manual actions performed deliberately by a human in their own terminal; they are never triggered automatically on a caller's behalf.", Children = new[] { typeof(AuthLoginCliCommand), diff --git a/src/TALXIS.CLI.Features.Config/Auth/AuthLoginCliCommand.cs b/src/TALXIS.CLI.Features.Config/Auth/AuthLoginCliCommand.cs index 78a81db7..b6f92b7d 100644 --- a/src/TALXIS.CLI.Features.Config/Auth/AuthLoginCliCommand.cs +++ b/src/TALXIS.CLI.Features.Config/Auth/AuthLoginCliCommand.cs @@ -20,15 +20,19 @@ namespace TALXIS.CLI.Features.Config.Auth; /// /// Fails fast with exit 1 in headless contexts — interactive browser is /// never a permitted headless kind. See . -/// When a local browser is not available (Codespaces, SSH, no DISPLAY), -/// the command automatically falls back to device code flow unless -/// --device-code was already specified. +/// Sign-in is always a deliberate, manual action taken by a human in their +/// own terminal — this command never guesses on the caller's behalf. When a +/// local browser is not available (Codespaces, SSH, no DISPLAY) it does +/// not silently switch to device code; it fails fast and tells the +/// human to re-run with --device-code themselves. This mirrors how +/// real scripted workflows already use this command (they always pass +/// --device-code explicitly rather than relying on auto-detection). /// [CliIdempotent] [McpIgnore] [CliCommand( Name = "login", - Description = "Interactive sign-in. Uses browser login by default; falls back to device code in browser-isolated environments (Codespaces, SSH)." + Description = "Interactive sign-in, run manually by a human in their own terminal — never by an automated caller. Uses browser login by default; pass --device-code yourself when no local browser can reach localhost (Codespaces, SSH, containers)." )] public class AuthLoginCliCommand : TxcLeafCommand { @@ -43,7 +47,7 @@ public class AuthLoginCliCommand : TxcLeafCommand [CliOption(Name = "--cloud", Description = "Sovereign cloud. Default: public.", Required = false)] public CloudInstance? Cloud { get; set; } - [CliOption(Name = "--device-code", Description = "Use device code flow instead of browser login. Required when no local browser can reach localhost (Codespaces, SSH, containers).", Required = false)] + [CliOption(Name = "--device-code", Description = "Use device code flow instead of browser login. Pass this yourself — deliberately, as the human running this command — when no local browser can reach localhost (Codespaces, SSH, containers). Never inferred automatically.", Required = false)] public bool DeviceCode { get; set; } protected override async Task ExecuteAsync() @@ -53,15 +57,27 @@ protected override async Task ExecuteAsync() var browserProbe = TxcServices.Get(); var cloud = Cloud ?? CloudInstance.Public; - // Determine whether to use device code flow: explicit flag or - // or automatic detection of browser-isolated environments. - var useDeviceCode = DeviceCode || !browserProbe.IsBrowserAvailable; + // Sign-in is always a deliberate, manual choice made by a human. + // Never silently switch flows on the caller's behalf: if no local + // browser is reachable and --device-code wasn't explicitly passed, + // fail fast instead of starting an unattended device-code prompt + // that nobody will complete (see HeadlessAuthRequiredException for + // the equivalent fully-headless case). + if (!DeviceCode && !browserProbe.IsBrowserAvailable) + { + Logger.LogError( + "No local browser is available ({Reason}). Interactive sign-in requires a human to run it " + + "deliberately in their own terminal. Re-run this command yourself with '--device-code' to sign in " + + "using the device code flow. This is never chosen automatically.", + browserProbe.UnavailableReason ?? "unknown reason"); + return ExitError; + } - if (useDeviceCode) + if (DeviceCode) { var deviceCodeLogin = TxcServices.Get(); - Logger.LogInformation("Starting device code sign-in (browser unavailable: {Reason})...", - browserProbe.UnavailableReason ?? "--device-code flag"); + Logger.LogInformation("Starting device code sign-in (requested via --device-code; browser reachable: {BrowserAvailable})...", + browserProbe.IsBrowserAvailable); var result = await DeviceCodeCredentialBootstrapper.AcquireAndPersistAsync( deviceCodeLogin, store, headless, Tenant, cloud, Alias, CancellationToken.None).ConfigureAwait(false); diff --git a/src/TALXIS.CLI.Features.Config/Connection/ConnectionCliCommand.cs b/src/TALXIS.CLI.Features.Config/Connection/ConnectionCliCommand.cs index 8a65e0cb..56a29854 100644 --- a/src/TALXIS.CLI.Features.Config/Connection/ConnectionCliCommand.cs +++ b/src/TALXIS.CLI.Features.Config/Connection/ConnectionCliCommand.cs @@ -12,7 +12,7 @@ namespace TALXIS.CLI.Features.Config.Connection; /// [CliCommand( Name = "connection", - Description = "Manage service endpoint metadata (Dataverse environments, etc.).", + Description = "Manage service endpoint metadata (Dataverse environments, etc.) — the \"where\", not the \"who\". Run 'connection list' before 'connection create' to check for an existing one targeting the same environment.", Children = new[] { typeof(ConnectionCreateCliCommand), diff --git a/src/TALXIS.CLI.Features.Config/Profile/ProfileCliCommand.cs b/src/TALXIS.CLI.Features.Config/Profile/ProfileCliCommand.cs index dd386ec7..19cb3a72 100644 --- a/src/TALXIS.CLI.Features.Config/Profile/ProfileCliCommand.cs +++ b/src/TALXIS.CLI.Features.Config/Profile/ProfileCliCommand.cs @@ -10,7 +10,7 @@ namespace TALXIS.CLI.Features.Config.Profile; /// [CliCommand( Name = "profile", - Description = "Manage profiles (bind one auth to one connection).", + Description = "Manage profiles (bind one auth to one connection). Profile = context (\"which environment, as whom\"); Connection = where; Credential = who. Run 'profile list' before 'profile create' — reuse an existing profile instead of guessing a new one.", Children = new[] { typeof(ProfileCreateCliCommand), diff --git a/src/TALXIS.CLI.Features.Config/Profile/ProfileCreateCliCommand.cs b/src/TALXIS.CLI.Features.Config/Profile/ProfileCreateCliCommand.cs index b5debc8e..42b83e16 100644 --- a/src/TALXIS.CLI.Features.Config/Profile/ProfileCreateCliCommand.cs +++ b/src/TALXIS.CLI.Features.Config/Profile/ProfileCreateCliCommand.cs @@ -35,7 +35,7 @@ namespace TALXIS.CLI.Features.Config.Profile; [CliIdempotent] [CliCommand( Name = "create", - Description = "Create a profile. Quickstart: --url . Advanced: --auth --connection ." + Description = "Create a profile. Check 'txc config profile list' and 'txc config connection list' first — reuse an existing profile/connection when one already targets the environment you need instead of creating a new one. Quickstart: --url . Advanced: --auth --connection ." )] public class ProfileCreateCliCommand : TxcLeafCommand { diff --git a/src/TALXIS.CLI.Features.Config/Profile/ProfileValidateCliCommand.cs b/src/TALXIS.CLI.Features.Config/Profile/ProfileValidateCliCommand.cs index cf603ccb..350b0d87 100644 --- a/src/TALXIS.CLI.Features.Config/Profile/ProfileValidateCliCommand.cs +++ b/src/TALXIS.CLI.Features.Config/Profile/ProfileValidateCliCommand.cs @@ -49,7 +49,10 @@ protected override async Task ExecuteAsync() if (context.Profile is null) { - Logger.LogError("Resolved configuration is ephemeral. 'txc config profile validate' requires a stored profile."); + Logger.LogError( + "Resolved configuration is ephemeral. 'txc config profile validate' requires a stored profile. " + + "Run 'txc config profile list' and 'txc config connection list' to check for an existing profile " + + "before creating a new one — confirm the target environment with the user if it's unclear."); return ExitValidationError; } diff --git a/src/TALXIS.CLI.MCP/CliSubprocessRunner.cs b/src/TALXIS.CLI.MCP/CliSubprocessRunner.cs index 3354fd42..0dd995fc 100644 --- a/src/TALXIS.CLI.MCP/CliSubprocessRunner.cs +++ b/src/TALXIS.CLI.MCP/CliSubprocessRunner.cs @@ -97,6 +97,20 @@ private static async Task ReadLinesAsync( } private static Process StartProcess(IReadOnlyList cliArgs, string? workingDirectory = null) + { + ProcessStartInfo startInfo = BuildStartInfo(cliArgs, workingDirectory); + return Process.Start(startInfo) + ?? throw new InvalidOperationException("Failed to start the txc CLI subprocess."); + } + + /// + /// Builds the for a txc CLI subprocess without + /// starting it. Extracted from so the + /// forced-headless contract (TXC_NON_INTERACTIVE=1) can be covered by a + /// unit test that doesn't spawn a real process — see + /// CliSubprocessRunnerTests. + /// + internal static ProcessStartInfo BuildStartInfo(IReadOnlyList cliArgs, string? workingDirectory = null) { (string fileName, string? assemblyPath) = ResolveCliHost(); ProcessStartInfo startInfo = new() @@ -159,8 +173,7 @@ private static Process StartProcess(IReadOnlyList cliArgs, string? worki startInfo.ArgumentList.Add(cliArg); } - return Process.Start(startInfo) - ?? throw new InvalidOperationException("Failed to start the txc CLI subprocess."); + return startInfo; } private static (string FileName, string? AssemblyPath) ResolveCliHost() diff --git a/src/TALXIS.CLI.MCP/CopilotInstructionsManager.cs b/src/TALXIS.CLI.MCP/CopilotInstructionsManager.cs index d1f81776..40b6a37c 100644 --- a/src/TALXIS.CLI.MCP/CopilotInstructionsManager.cs +++ b/src/TALXIS.CLI.MCP/CopilotInstructionsManager.cs @@ -19,6 +19,18 @@ public class CopilotInstructionsManager # Instructions for performing tasks over the repository Use the TALXIS CLI MCP Server (txc-mcp) for all development tasks. Call `guide_workspace`, `guide_environment`, `guide_deployment`, `guide_data`, `guide_config`, or `get_skill_details` to discover available tools and guidance. +## Authentication + +- Never attempt to sign in yourself. `txc`/txc-mcp structurally refuses to start + an interactive browser or device-code flow on your behalf — there is no + headless fallback. If a tool call fails with an auth-required/expired-token + error, stop and ask the user to run `txc config auth login` manually in + their own terminal, then retry. +- Never create a new profile or connection speculatively. Run + `config_profile_list` / `config_connection_list` (or `txc config profile + list` / `txc config connection list`) first and reuse an existing one that + already targets the environment you need. + ## Project Structure and Naming Conventions **Note**: These are recommended naming conventions. Users may choose different naming styles based on their preferences or organizational standards. diff --git a/src/TALXIS.CLI.MCP/Skills/Internal/troubleshooting-patterns.md b/src/TALXIS.CLI.MCP/Skills/Internal/troubleshooting-patterns.md index bd3195dd..c6d0363f 100644 --- a/src/TALXIS.CLI.MCP/Skills/Internal/troubleshooting-patterns.md +++ b/src/TALXIS.CLI.MCP/Skills/Internal/troubleshooting-patterns.md @@ -36,6 +36,14 @@ config_profile_validate → result? ├─ Invalid → config_profile_get → config_connection_get → fix credentials └─ Valid but failing → check security roles in Power Platform admin center (outside txc) ``` +→ If the failure is an expired/missing credential (AUTH_REQUIRED-style error), do +NOT attempt to sign in yourself and do NOT create a new profile/connection to work +around it. Sign-in is always a manual, human action — `txc` structurally refuses to +start an interactive or device-code flow on your behalf (there is no headless +fallback). Run `config_profile_list` / `config_connection_list` to confirm nothing +existing already covers the target, then STOP and ask the user to run +`txc config auth login` themselves in their own terminal, and retry once they confirm. + ## Diagnostic Priority Order When unsure where to start: @@ -53,3 +61,5 @@ When unsure where to start: - ❌ Querying the `asyncoperation` table via `environment_data_query_sql` to check import status → call `environment_deployment_get --async-operation-id ` instead — raw SQL gives unstructured statuscodes, the proper tool returns parsed findings and human-readable errors - ❌ Jumping to layer inspection before checking basic auth/connectivity - ❌ Using environment schema tools to "fix" what should be fixed locally and redeployed +- ❌ Creating a new profile or connection speculatively on an auth/"no profile" error → list first (`config_profile_list`, `config_connection_list`) and reuse an existing one +- ❌ Trying to trigger or work around sign-in yourself (interactive browser, device-code, or otherwise) → stop and ask the user to run `txc config auth login` manually diff --git a/src/TALXIS.CLI.Platform.Dataverse.Runtime/Runtime/DataverseAccessTokenService.cs b/src/TALXIS.CLI.Platform.Dataverse.Runtime/Runtime/DataverseAccessTokenService.cs index 6272ce3e..ab3a5d11 100644 --- a/src/TALXIS.CLI.Platform.Dataverse.Runtime/Runtime/DataverseAccessTokenService.cs +++ b/src/TALXIS.CLI.Platform.Dataverse.Runtime/Runtime/DataverseAccessTokenService.cs @@ -29,8 +29,6 @@ public sealed class DataverseAccessTokenService : IDataverseAccessTokenService private readonly MsalTokenCacheBinder _cacheBinder; private readonly ICredentialVault _vault; private readonly IEnvironmentReader _env; - private readonly IHeadlessDetector _headless; - private readonly IBrowserAvailabilityProbe _browserProbe; private readonly ILogger _logger; private readonly InMemoryTokenCache _tokenCache = new(); @@ -39,16 +37,12 @@ public DataverseAccessTokenService( MsalTokenCacheBinder cacheBinder, ICredentialVault vault, IEnvironmentReader env, - IHeadlessDetector headless, - IBrowserAvailabilityProbe browserProbe, ILogger? logger = null) { _clientFactory = clientFactory ?? throw new ArgumentNullException(nameof(clientFactory)); _cacheBinder = cacheBinder ?? throw new ArgumentNullException(nameof(cacheBinder)); _vault = vault ?? throw new ArgumentNullException(nameof(vault)); _env = env ?? throw new ArgumentNullException(nameof(env)); - _headless = headless ?? throw new ArgumentNullException(nameof(headless)); - _browserProbe = browserProbe ?? throw new ArgumentNullException(nameof(browserProbe)); _logger = logger ?? NullLogger.Instance; } @@ -143,7 +137,8 @@ private async Task AcquirePublicClientSilentAsync( { throw new InvalidOperationException( $"No cached sign-in found for credential '{credential.Id}'. " + - "Run 'txc config auth login' and retry."); + "Interactive sign-in requires a human to run it deliberately in their own terminal — " + + "ask the user to run 'txc config auth login' themselves, then retry."); } try @@ -157,71 +152,17 @@ private async Task AcquirePublicClientSilentAsync( } catch (MsalUiRequiredException ex) { - // In interactive sessions, attempt automatic re-authentication. - // The strategy depends on whether a local browser is reachable: - // - Browser available: AcquireTokenInteractive with Dataverse scope - // - Browser isolated (Codespaces/SSH): device code flow - if (!_headless.IsHeadless) - { - _logger.LogWarning( - "Token for credential '{CredentialId}' expired or requires consent — attempting re-authentication.", - credential.Id); - - try - { - if (_browserProbe.IsBrowserAvailable) - { - // Browser is reachable — use interactive flow with the - // actual Dataverse scope that needs consent. - var interactiveResult = await app - .AcquireTokenInteractive(new[] { scope }) - .WithAccount(account) - .WithUseEmbeddedWebView(false) - .ExecuteAsync(ct) - .ConfigureAwait(false); - _tokenCache.Set(cacheKey, interactiveResult); - return interactiveResult.AccessToken; - } - else - { - // Browser-isolated environment (Codespaces, SSH, etc.) - // — use device code flow for re-authentication. - _logger.LogWarning( - "No local browser available ({Reason}). Using device code for re-authentication.", - _browserProbe.UnavailableReason); - - // Note: AcquireTokenWithDeviceCode does not support WithLoginHint - // (no account pre-selection in device code flow). We log the expected - // account so the user can verify they are signing in as the right identity. - _logger.LogWarning( - "Please sign in as {ExpectedUpn} when completing device code authentication.", - account.Username); - - var deviceCodeResult = await app - .AcquireTokenWithDeviceCode(new[] { scope }, dcr => - { - _logger.LogWarning("{DeviceCodeMessage}", dcr.Message); - return Task.CompletedTask; - }) - .ExecuteAsync(ct) - .ConfigureAwait(false); - _tokenCache.Set(cacheKey, deviceCodeResult); - return deviceCodeResult.AccessToken; - } - } - catch (OperationCanceledException) - { - throw; // Propagate Ctrl+C / browser close cancellation. - } - catch (Exception reAuthEx) - { - _logger.LogDebug(reAuthEx, "Automatic re-authentication failed; falling through to original error."); - } - } - + // Sign-in is always a deliberate, manual action taken by a human in + // their own terminal — a command that merely needs a Dataverse token + // must never launch an interactive browser or device-code flow on the + // caller's behalf, even when a TTY looks available. Silently doing so + // here previously stranded unattended callers (e.g. AI agents) on an + // unattended device-code prompt nobody would complete. Always fail + // fast and hand the human the exact remedy command instead. throw new InvalidOperationException( $"Cached token for '{credential.Id}' expired or is missing consent. " + - "Run 'txc config auth login' and retry.", ex); + "Interactive sign-in requires a human to run it deliberately in their own terminal — " + + "ask the user to run 'txc config auth login' themselves, then retry.", ex); } } diff --git a/tests/TALXIS.CLI.Tests/Config/Commands/Auth/AuthLoginCommandTests.cs b/tests/TALXIS.CLI.Tests/Config/Commands/Auth/AuthLoginCommandTests.cs index d45a81f5..c2e400f8 100644 --- a/tests/TALXIS.CLI.Tests/Config/Commands/Auth/AuthLoginCommandTests.cs +++ b/tests/TALXIS.CLI.Tests/Config/Commands/Auth/AuthLoginCommandTests.cs @@ -234,22 +234,24 @@ public async Task Login_DeviceCode_PersistsCredential_WhenFlagExplicit() } [Fact] - public async Task Login_DeviceCode_TriggeredAutomatically_WhenBrowserUnavailable() + public async Task Login_FailsFast_WhenBrowserUnavailable_AndDeviceCodeNotExplicit() { + // Sign-in must always be a deliberate, manual choice made by a human — the + // command must never silently switch to device code on the caller's behalf, + // since nobody may be present to complete an unattended device-code prompt. using var host = new CommandTestHost( browserAvailable: false, loginResult: new InteractiveLoginResult("tomas@contoso.com", "t-guid")); var store = (ICredentialStore)host.Provider.GetService(typeof(ICredentialStore))!; - // No --device-code flag; auto-detection via FakeBrowserProbe should trigger device code + // No --device-code flag; the command must fail fast rather than auto-detect. var exit = await new AuthLoginCliCommand().RunAsync(); - Assert.Equal(0, exit); - Assert.Equal(1, host.Login.Calls); + Assert.Equal(1, exit); + Assert.Equal(0, host.Login.Calls); var creds = await store.ListAsync(default); - var cred = Assert.Single(creds); - Assert.Equal(CredentialKind.DeviceCode, cred.Kind); + Assert.Empty(creds); } [Fact] diff --git a/tests/TALXIS.CLI.Tests/MCP/CliSubprocessRunnerTests.cs b/tests/TALXIS.CLI.Tests/MCP/CliSubprocessRunnerTests.cs new file mode 100644 index 00000000..7cc19ee2 --- /dev/null +++ b/tests/TALXIS.CLI.Tests/MCP/CliSubprocessRunnerTests.cs @@ -0,0 +1,51 @@ +using TALXIS.CLI.MCP; +using Xunit; + +namespace TALXIS.CLI.Tests.MCP; + +/// +/// Regression coverage for the forced-headless contract every MCP-spawned txc +/// subprocess must honor: interactive/device-code sign-in flows must never be +/// reachable when txc is invoked through MCP, because stdout is reserved for +/// JSON-RPC frames and no human is watching the terminal. This is enforced by +/// unconditionally setting TXC_NON_INTERACTIVE=1 on every subprocess +/// , which +/// then uses to block -guarded +/// interactive credential kinds. See src/TALXIS.CLI.MCP/README.md#auth-contract. +/// +public class CliSubprocessRunnerTests +{ + [Fact] + public void BuildStartInfo_AlwaysForcesNonInteractiveEnvVar() + { + var startInfo = CliSubprocessRunner.BuildStartInfo(new[] { "config", "profile", "list" }); + + Assert.True(startInfo.Environment.TryGetValue("TXC_NON_INTERACTIVE", out var value)); + Assert.Equal("1", value); + } + + [Fact] + public void BuildStartInfo_ForcesNonInteractiveEnvVar_RegardlessOfCliArgs() + { + // Even for commands that don't touch auth at all, the flag must still be + // present — this is a blanket subprocess-level guarantee, not something + // decided per-command. + var startInfo = CliSubprocessRunner.BuildStartInfo(new[] { "config", "auth", "login", "--device-code" }); + + Assert.True(startInfo.Environment.TryGetValue("TXC_NON_INTERACTIVE", out var value)); + Assert.Equal("1", value); + } + + [Fact] + public void BuildStartInfo_RedirectsStandardStreams() + { + // Redirected stdin/stdout is one of the two conditions HeadlessDetector + // checks; redirected output alone (with TXC_NON_INTERACTIVE also set) + // makes the headless determination doubly robust for the MCP path. + var startInfo = CliSubprocessRunner.BuildStartInfo(new[] { "config", "profile", "list" }); + + Assert.True(startInfo.RedirectStandardOutput); + Assert.True(startInfo.RedirectStandardError); + Assert.False(startInfo.UseShellExecute); + } +}