diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Browser/SqlDriverBrowser.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Browser/SqlDriverBrowser.cs new file mode 100644 index 00000000..bd43f089 --- /dev/null +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Browser/SqlDriverBrowser.cs @@ -0,0 +1,401 @@ +using System.Data.Common; +using System.Text.Json; +using System.Text.Json.Serialization; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Logging.Abstractions; +using ZB.MOM.WW.OtOpcUa.Commons.Browsing; +using ZB.MOM.WW.OtOpcUa.Driver.Sql.Contracts; + +namespace ZB.MOM.WW.OtOpcUa.Driver.Sql.Browser; + +/// +/// Opens a transient database connection from form-supplied JSON for the AdminUI schema picker, +/// and hands it to a . The AdminUI's BrowseSessionRegistry (its idle +/// TTL reaper included) owns the returned session unchanged — nothing about the SQL browse needs a +/// registry of its own. +/// Two ways to name the database, and only one of them is persisted. +/// connectionStringRef is the deployed form: a name resolved in the AdminUI process from +/// the environment as Sql__ConnectionStrings__<ref> (design §4.4), so a deployed artifact +/// never carries credentials. connectionString is an ad-hoc, session-only literal an +/// operator may paste to browse a database that has no ref yet; it is used for this one connection and +/// then forgotten — there is deliberately no cached-config field on this type, and nothing writes it +/// anywhere. +/// Precedence: a pasted literal wins over a ref, and the ref is then not even read. The +/// literal cannot arrive from a persisted blob — has no such property, so +/// the driver factory would drop it — which makes its presence proof that an operator typed it just now. +/// A config carrying both logs a warning (naming the shadowed ref, never any connection text) so +/// the override is visible rather than silent. +/// Credential hygiene is the point of this type. A connection string is a secret. It is +/// passed to the provider and to nothing else: it never reaches a log line, an exception message, a field, +/// or anything persisted. Because ADO.NET providers are free to echo connection-string content in their +/// own errors, every failure on the open path goes through before it leaves this +/// class. +/// +public sealed class SqlDriverBrowser : IDriverBrowser +{ + /// + /// Prefix of the environment variable a connectionStringRef resolves through — the + /// double-underscore form .NET configuration uses for the Sql:ConnectionStrings:<ref> + /// key path. + /// + public const string ConnectionStringEnvironmentPrefix = "Sql__ConnectionStrings__"; + + /// + /// Hard cap on the connect phase, layered on the caller's token. The AdminUI already bounds each + /// browse call at 20 s; this only stops a pathological provider-side connect from outliving the + /// picker entirely. + /// + private static readonly TimeSpan ConnectBudget = TimeSpan.FromSeconds(30); + + /// + /// Connection-string keys whose values are redacted out of any provider error text. The + /// whole connection string is redacted unconditionally; these are the parts that could survive as a + /// fragment. Server / database names are deliberately not redacted — "server X was not found" + /// is the diagnostic the operator needs, and a hostname is not a credential. + /// + private static readonly string[] CredentialKeys = + [ + "password", "pwd", "user id", "uid", "user", "username", "accesstoken", "access token", + ]; + + /// + /// Kept deliberately identical in shape to the driver factory's options (design §5.1): enum-valued + /// knobs are authored as their names, so the browser parses a given DriverConfig blob + /// exactly as the runtime factory does. also accepts ordinals, + /// so a numeric config still binds. + /// + private static readonly JsonSerializerOptions JsonOpts = new() + { + UnmappedMemberHandling = JsonUnmappedMemberHandling.Skip, + PropertyNameCaseInsensitive = true, + Converters = { new JsonStringEnumConverter() }, + }; + + private readonly Func _dialectSelector; + private readonly Func _environmentReader; + private readonly ILogger _logger; + + /// Creates a browser. Every dependency has a production default, so DI may construct it bare. + /// + /// Maps an authored onto the dialect that browses it, or + /// for a provider this build does not carry. Defaults to + /// (SqlServer only, in v1). A constructor parameter rather than a test hatch: it is also how a P2 + /// provider gets added without touching this class. + /// + /// + /// Reads one environment variable. Defaults to . + /// Injectable because environment variables are process-global, and a test that sets one races every + /// other test in the assembly. + /// + /// Optional; defaults to . + public SqlDriverBrowser( + Func? dialectSelector = null, + Func? environmentReader = null, + ILogger? logger = null) + { + _dialectSelector = dialectSelector ?? DefaultDialectSelector; + _environmentReader = environmentReader ?? Environment.GetEnvironmentVariable; + _logger = logger ?? NullLogger.Instance; + } + + /// + /// + /// Sourced from , the driver's interim local constant, rather + /// than from DriverTypeNames — the shared constant set is added by the driver-factory task, + /// because adding a member there reddens DriverTypeNamesGuardTests until a factory is + /// registered against it. Repointing that one constant repoints this too. + /// + public string DriverType => SqlDriver.DriverTypeName; + + /// The environment variable a connectionStringRef resolves through. + /// The authored reference name. + /// The full environment-variable name, e.g. Sql__ConnectionStrings__MesStaging. + public static string EnvironmentVariableFor(string connectionStringRef) => + ConnectionStringEnvironmentPrefix + connectionStringRef; + + /// + /// The providers this build can browse. v1 constructs SQL Server only; every other + /// member is a reserved name, so it resolves to and + /// the caller reports it as unavailable rather than crashing on a null dialect. + /// + /// The authored provider. + /// The dialect, or when this build does not carry the provider. + public static ISqlDialect? DefaultDialectSelector(SqlProvider provider) => + provider == SqlProvider.SqlServer ? new SqlServerDialect() : null; + + /// + /// + /// The configuration is absent / malformed, names a provider this build does not carry, names neither + /// a connectionStringRef nor a literal, resolves a ref that is not set in the environment, or + /// the connection could not be opened. + /// + public async Task OpenAsync(string configJson, CancellationToken cancellationToken) + { + if (string.IsNullOrWhiteSpace(configJson)) + { + throw new InvalidOperationException( + "The Sql browser requires a driver configuration; none was supplied."); + } + + var literal = ReadPastedConnectionString(configJson); + var config = Deserialize(configJson, literal); + + // Provider first: "this build cannot browse Postgres" is true regardless of how the database is + // named, and answering it before touching the environment keeps the two failures from masking. + var dialect = _dialectSelector(config.Provider) + ?? throw new InvalidOperationException( + $"Sql provider '{config.Provider}' is not available in this build; v1 browses " + + $"'{SqlProvider.SqlServer}' only."); + + var reference = config.ConnectionStringRef; + var connectionString = ResolveConnectionString(literal, reference); + + _logger.LogInformation( + "AdminUI Sql browse session opening (provider {Provider}, connection from {Source})", + config.Provider, + DescribeSource(literal, reference)); + + return await OpenSessionAsync(dialect, connectionString, cancellationToken).ConfigureAwait(false); + } + + /// + /// Opens one transient connection and hands ownership to the session. The session closes the + /// connection on its own disposal, so the only disposal this method owns is the failure path — + /// disposing on success would double-close and leave the picker with a dead session. + /// + private static async Task OpenSessionAsync( + ISqlDialect dialect, string connectionString, CancellationToken cancellationToken) + { + var connection = dialect.Factory.CreateConnection() + ?? throw new InvalidOperationException( + $"The ADO.NET provider for '{dialect.Provider}' returned no connection object."); + + try + { + connection.ConnectionString = connectionString; + + using var connectCts = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); + connectCts.CancelAfter(ConnectBudget); + await connection.OpenAsync(connectCts.Token).ConfigureAwait(false); + + return new SqlBrowseSession(connection, dialect); + } + catch (Exception ex) + { + try { await connection.DisposeAsync().ConfigureAwait(false); } + catch { /* best-effort: the original failure is the useful one. */ } + + if (ex is OperationCanceledException) throw; + throw Sanitize(ex, connectionString); + } + } + + /// + /// Picks the connection string, literal first. A blank literal is treated as absent (an untouched + /// form field), never as an empty connection string. + /// + private string ResolveConnectionString(string? literal, string? reference) + { + if (!string.IsNullOrWhiteSpace(literal)) + { + if (!string.IsNullOrWhiteSpace(reference)) + { + _logger.LogWarning( + "Sql browse: a pasted connection string was supplied alongside connectionStringRef " + + "'{Ref}'. The pasted value wins for this session only and is not persisted.", + reference); + } + + return literal; + } + + if (string.IsNullOrWhiteSpace(reference)) + { + throw new InvalidOperationException( + "The Sql browser needs a database to browse: set 'connectionStringRef' to a name resolved " + + $"from the environment as '{ConnectionStringEnvironmentPrefix}', or paste a " + + "connection string into 'connectionString' for an ad-hoc browse."); + } + + var variable = EnvironmentVariableFor(reference); + var resolved = _environmentReader(variable); + if (string.IsNullOrWhiteSpace(resolved)) + { + // Name the exact variable: "the ref did not resolve" is unactionable, and the operator's next + // move is to set this one name on the AdminUI host. + throw new InvalidOperationException( + $"connectionStringRef '{reference}' does not resolve: environment variable " + + $"'{variable}' is not set on the AdminUI host. Set it there (the AdminUI resolves browse " + + "connection strings in its own process), or paste a connection string for an ad-hoc " + + "browse."); + } + + return resolved; + } + + /// How the connection string was obtained, for the log. Carries no connection text. + private static string DescribeSource(string? literal, string? reference) => + !string.IsNullOrWhiteSpace(literal) + ? "a pasted literal (session-only, not persisted)" + : $"connectionStringRef '{reference}'"; + + /// + /// Pulls the ad-hoc connectionString literal out of the raw JSON. It is read here rather than + /// off the DTO because deliberately has no such property — the + /// persisted contract must not be able to carry a credential — and the DTO's + /// would silently drop it. + /// + /// The literal, or when absent, blank, or not a JSON string. + private static string? ReadPastedConnectionString(string configJson) + { + using var document = ParseDocument(configJson); + if (document.RootElement.ValueKind != JsonValueKind.Object) + { + throw new InvalidOperationException( + "The Sql browser's driver configuration must be a JSON object."); + } + + foreach (var property in document.RootElement.EnumerateObject()) + { + if (!string.Equals(property.Name, "connectionString", StringComparison.OrdinalIgnoreCase)) + continue; + + if (property.Value.ValueKind != JsonValueKind.String) return null; + var value = property.Value.GetString(); + return string.IsNullOrWhiteSpace(value) ? null : value; + } + + return null; + } + + /// + /// Parses the raw configuration. The is not attached or echoed: + /// at this point nothing has been extracted, so there is no known secret to redact against, and the + /// malformed text may itself be a pasted connection string. + /// + private static JsonDocument ParseDocument(string configJson) + { + try + { + return JsonDocument.Parse(configJson); + } + catch (JsonException) + { + throw new InvalidOperationException( + "The Sql browser's driver configuration is not valid JSON. (The parser's message is " + + "withheld because the malformed text may carry a connection string.)"); + } + } + + /// + /// Binds the typed configuration. A bind failure (e.g. an unknown provider name) is reported + /// with the parser's own message redacted against the literal, which is known by now. + /// + private static SqlDriverConfigDto Deserialize(string configJson, string? literal) + { + try + { + return JsonSerializer.Deserialize(configJson, JsonOpts) + ?? throw new InvalidOperationException( + "The Sql browser's driver configuration deserialized to null."); + } + catch (JsonException ex) + { + throw new InvalidOperationException( + "The Sql browser's driver configuration could not be bound: " + + Redact(ex.Message, CollectSecrets(literal))); + } + } + + /// + /// Turns a provider-side open failure into an exception that cannot carry the connection string. + /// Live-probed, not assumed. Microsoft.Data.SqlClient and + /// Microsoft.Data.Sqlite keep connection-string values out of their + /// SqlException/SqliteException text (a failed connect says "the server was not found", + /// never what it was handed). Their connection-string parser is the exception: it reports an + /// unrecognised keyword by quoting it, and a value carrying an unquoted ; — legal inside a + /// password, which ADO.NET expects you to quote — splits, so the tail of the password is parsed as a + /// keyword and echoed verbatim. Password=Sup3r;SecretTail yields + /// Keyword not supported: 'secrettail;connect timeout'. — the credential, in the message, + /// lower-cased and therefore invisible to a naive ordinal search for the value. + /// So the parser's message (an ) is never surfaced. Every + /// other failure is additionally passed through the substring redactor as a backstop, and when that + /// fires anywhere in the exception chain the inner exception is dropped too — keeping it + /// would re-expose through ToString() exactly what the message just removed. + /// + private static InvalidOperationException Sanitize(Exception ex, string connectionString) + { + if (ex is ArgumentException) + { + return new InvalidOperationException( + "The Sql browser could not open a connection: the connection string is malformed, or " + + "carries a keyword this provider does not support. (The provider's own message is " + + "withheld because it quotes connection-string text verbatim; check for an unquoted ';' " + + "or '=' inside a value such as the password.)"); + } + + var secrets = CollectSecrets(connectionString); + var leaked = ContainsAny(ex.ToString(), secrets); + var reason = Redact(ex.Message, secrets); + + var message = leaked + ? "The Sql browser could not open a connection: " + reason + + " (the provider's error echoed the connection string, so it was redacted and the inner " + + "exception withheld)" + : "The Sql browser could not open a connection: " + reason; + + return new InvalidOperationException(message, leaked ? null : ex); + } + + /// + /// The substrings that must never leave this class: the whole connection string, and the values of + /// its credential-bearing keys (which could surface on their own). + /// + private static List CollectSecrets(string? connectionString) + { + var secrets = new List(); + if (string.IsNullOrWhiteSpace(connectionString)) return secrets; + + secrets.Add(connectionString); + + try + { + var builder = new DbConnectionStringBuilder { ConnectionString = connectionString }; + foreach (var key in CredentialKeys) + { + if (!builder.TryGetValue(key, out var value)) continue; + var text = value?.ToString(); + if (!string.IsNullOrWhiteSpace(text)) secrets.Add(text); + } + } + catch (ArgumentException) + { + // A malformed connection string has no parseable parts; the whole-string entry still stands. + } + + // Longest first, so redacting a short credential value cannot chop a longer secret into + // unredactable halves. + secrets.Sort(static (left, right) => right.Length.CompareTo(left.Length)); + return secrets; + } + + private static bool ContainsAny(string text, List secrets) + { + foreach (var secret in secrets) + { + if (text.Contains(secret, StringComparison.OrdinalIgnoreCase)) return true; + } + + return false; + } + + private static string Redact(string text, List secrets) + { + foreach (var secret in secrets) + { + text = text.Replace(secret, "[redacted]", StringComparison.OrdinalIgnoreCase); + } + + return text; + } +} diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Browser.Tests/SqlDriverBrowserTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Browser.Tests/SqlDriverBrowserTests.cs new file mode 100644 index 00000000..c984c7bd --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Browser.Tests/SqlDriverBrowserTests.cs @@ -0,0 +1,384 @@ +using System.Text.Json; +using Microsoft.Extensions.Logging; +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Commons.Browsing; +using ZB.MOM.WW.OtOpcUa.Driver.Sql.Contracts; +using ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests; + +namespace ZB.MOM.WW.OtOpcUa.Driver.Sql.Browser.Tests; + +/// +/// Covers 's open side: which of the two ways to name a database wins, what +/// an unresolvable reference tells the operator, which providers this build carries — and, above all, +/// that a connection string never escapes into a log line or an exception. +/// The successful-open cases run through the linked against the real +/// seeded , injected via the browser's dialectSelector +/// constructor parameter. That parameter is the production extension point for a P2 provider, not a test +/// hatch — the same conclusion the driver author reached for SqlDriver's factory +/// parameter. +/// +public sealed class SqlDriverBrowserTests +{ + /// + /// The credential every hygiene assertion hunts for. Distinctive enough that a substring match + /// anywhere in an exception or a log line is proof of a leak, not a coincidence. + /// + private const string Password = "Sup3rSecret-OtOpcUa-Probe"; + + /// + /// A host that cannot resolve, so the connect fails in DNS rather than on a timer. .invalid is + /// reserved by RFC 2606 and can never be registered. + /// + private const string DeadHost = "otopcua-no-such-host.invalid"; + + private static Func SqliteSelector => static _ => new SqliteDialect(); + + // ---- identity ---- + + [Fact] + public void DriverType_isTheSqlDriversOwnInterimConstant() + { + // Deliberately asserted against SqlDriver.DriverTypeName rather than a literal: DriverTypeNames.Sql + // does not exist yet (adding it reddens DriverTypeNamesGuardTests until a factory is registered), so + // the driver's local constant is the single definition both sides must agree on. + new SqlDriverBrowser().DriverType.ShouldBe(SqlDriver.DriverTypeName); + } + + [Fact] + public void DefaultDialectSelector_carriesSqlServerOnly() + { + SqlDriverBrowser.DefaultDialectSelector(SqlProvider.SqlServer).ShouldBeOfType(); + + foreach (var provider in Enum.GetValues().Where(p => p != SqlProvider.SqlServer)) + SqlDriverBrowser.DefaultDialectSelector(provider).ShouldBeNull(); + } + + // ---- connection-string reference resolution ---- + + /// The plan's headline case: an unresolvable ref must name the exact variable to set. + [Fact] + public async Task Open_unresolvableRef_failsNamingExactEnvVar() + { + var browser = new SqlDriverBrowser(); + var ex = await Should.ThrowAsync(() => + browser.OpenAsync("""{"provider":"SqlServer","connectionStringRef":"MesStaging"}""", default)); + ex.Message.ShouldContain("Sql__ConnectionStrings__MesStaging"); + } + + /// + /// The default (production) path: no injected reader, a real process environment variable. Uses a + /// run-unique name and restores it in a finally, so a failure cannot leave it set for the rest + /// of the assembly — environment variables are process-global and every other test shares them. + /// + [Fact] + public async Task Open_refResolvedFromTheRealEnvironment_opensSession() + { + await using var db = await SqliteBrowseFixture.CreateAsync(); + var reference = "OtOpcUaBrowseTest" + Guid.NewGuid().ToString("N"); + var variable = SqlDriverBrowser.EnvironmentVariableFor(reference); + + Environment.SetEnvironmentVariable(variable, db.ConnectionString); + try + { + var browser = new SqlDriverBrowser(SqliteSelector); + + await using var session = await browser.OpenAsync( + ConfigJson(connectionStringRef: reference), CancellationToken.None); + + var roots = await session.RootAsync(CancellationToken.None); + roots.ShouldHaveSingleItem().DisplayName.ShouldBe(SqliteDialect.MainSchema); + } + finally + { + Environment.SetEnvironmentVariable(variable, null); + } + } + + [Fact] + public async Task Open_neitherRefNorLiteral_failsNamingBothWays() + { + var browser = new SqlDriverBrowser(SqliteSelector); + + var ex = await Should.ThrowAsync(() => + browser.OpenAsync(ConfigJson(), CancellationToken.None)); + + ex.Message.ShouldContain("connectionStringRef"); + ex.Message.ShouldContain("connectionString"); + } + + [Fact] + public async Task Open_blankRef_failsRatherThanResolvingAnEmptyVariableName() + { + var browser = new SqlDriverBrowser(SqliteSelector, static _ => "should never be read"); + + var ex = await Should.ThrowAsync(() => + browser.OpenAsync(ConfigJson(connectionStringRef: " "), CancellationToken.None)); + + ex.Message.ShouldContain("connectionStringRef"); + } + + // ---- pasted literal, and its precedence ---- + + /// + /// A pasted literal opens a working session, and the session owns the connection — it is still + /// live after OpenAsync returns (the browser did not close it) and dead after the session is + /// disposed (nothing else will). + /// + [Fact] + public async Task Open_pastedLiteral_opensWorkingSessionThatOwnsTheConnection() + { + await using var db = await SqliteBrowseFixture.CreateAsync(); + var browser = new SqlDriverBrowser(SqliteSelector); + + var session = await browser.OpenAsync( + ConfigJson(connectionString: db.ConnectionString), CancellationToken.None); + + var children = await session.ExpandAsync(SqliteBrowseFixture.SchemaNodeId, CancellationToken.None); + children.ShouldContain(n => n.DisplayName == SqliteBrowseFixture.KeyValueTable); + + await session.DisposeAsync(); + + await Should.ThrowAsync(() => + session.ExpandAsync(SqliteBrowseFixture.SchemaNodeId, CancellationToken.None)); + } + + /// + /// Precedence: the pasted literal wins, and the reference is not even read. Proven by a reader + /// that records every call — an assertion on the resulting session alone would still pass if the ref + /// were consulted first and merely lost a tie-break. + /// + [Fact] + public async Task Open_refAndLiteralTogether_literalWinsAndRefIsNeverRead() + { + await using var db = await SqliteBrowseFixture.CreateAsync(); + var reads = new List(); + var logger = new CapturingLogger(); + var browser = new SqlDriverBrowser( + SqliteSelector, + name => + { + reads.Add(name); + return "Data Source=/otopcua-no-such-directory/never.db"; + }, + logger); + + await using var session = await browser.OpenAsync( + ConfigJson(connectionStringRef: "MesStaging", connectionString: db.ConnectionString), + CancellationToken.None); + + reads.ShouldBeEmpty(); + var roots = await session.RootAsync(CancellationToken.None); + roots.ShouldHaveSingleItem().DisplayName.ShouldBe(SqliteDialect.MainSchema); + + // The override is announced, naming the shadowed ref — never any connection text. + logger.Entries.ShouldContain(e => e.Contains("MesStaging", StringComparison.Ordinal)); + logger.Entries.ShouldNotContain(e => e.Contains(db.ConnectionString, StringComparison.OrdinalIgnoreCase)); + } + + // ---- provider availability ---- + + [Fact] + public async Task Open_providerThisBuildDoesNotCarry_saysNotAvailableInThisBuild() + { + var browser = new SqlDriverBrowser(); + + var ex = await Should.ThrowAsync(() => + browser.OpenAsync( + ConfigJson("Postgres", connectionStringRef: "MesStaging"), CancellationToken.None)); + + ex.Message.ShouldContain("Postgres"); + ex.Message.ShouldContain("not available in this build"); + } + + /// + /// Provider availability is answered before the environment is touched: "this build cannot browse + /// Postgres" is true whatever the ref resolves to, and the two failures must not mask each other. + /// + [Fact] + public async Task Open_unavailableProviderAndUnresolvableRef_reportsTheProvider() + { + var reads = new List(); + var browser = new SqlDriverBrowser(environmentReader: name => { reads.Add(name); return null; }); + + var ex = await Should.ThrowAsync(() => + browser.OpenAsync( + ConfigJson("Oracle", connectionStringRef: "MesStaging"), CancellationToken.None)); + + ex.Message.ShouldContain("not available in this build"); + reads.ShouldBeEmpty(); + } + + // ---- malformed configuration ---- + + [Theory] + [InlineData("")] + [InlineData(" ")] + public async Task Open_emptyConfig_failsAskingForOne(string configJson) + { + var browser = new SqlDriverBrowser(SqliteSelector); + + var ex = await Should.ThrowAsync(() => + browser.OpenAsync(configJson, CancellationToken.None)); + + ex.Message.ShouldContain("configuration"); + } + + /// + /// Malformed JSON must fail as malformed JSON — and must not echo the offending text back, because + /// the text a JSON parser chokes on may itself be a half-pasted connection string. + /// + [Fact] + public async Task Open_malformedJson_failsWithoutEchoingTheConfig() + { + var browser = new SqlDriverBrowser(SqliteSelector); + var configJson = $$"""{"provider":"SqlServer","connectionString":"Password={{Password}}" """; + + var ex = await Should.ThrowAsync(() => + browser.OpenAsync(configJson, CancellationToken.None)); + + ex.Message.ShouldContain("not valid JSON"); + ex.ToString().ShouldNotContain(Password, Case.Insensitive); + } + + [Fact] + public async Task Open_jsonThatIsNotAnObject_fails() + { + var browser = new SqlDriverBrowser(SqliteSelector); + + var ex = await Should.ThrowAsync(() => + browser.OpenAsync("[1,2,3]", CancellationToken.None)); + + ex.Message.ShouldContain("JSON object"); + } + + /// An unknown provider name is a bind failure, and must not echo the config either. + [Fact] + public async Task Open_unknownProviderName_failsWithoutEchoingTheConfig() + { + var browser = new SqlDriverBrowser(SqliteSelector); + + var ex = await Should.ThrowAsync(() => + browser.OpenAsync( + ConfigJson("NotARealProvider", connectionString: $"Server=x;Password={Password}"), + CancellationToken.None)); + + ex.Message.ShouldContain("could not be bound"); + ex.ToString().ShouldNotContain(Password, Case.Insensitive); + } + + // ---- credential hygiene ---- + + /// + /// The natural case the task names: a well-formed connection string carrying a password, pointed at a + /// host that does not exist. Nothing about the string may reach the exception. + /// + [Fact] + public async Task Open_unreachableHost_neverLeaksThePastedConnectionString() + { + var connectionString = + $"Server={DeadHost};Database=Mes;User ID=sa;Password={Password};Connect Timeout=1;" + + "TrustServerCertificate=true"; + var browser = new SqlDriverBrowser(); + + var ex = await Should.ThrowAsync(() => + browser.OpenAsync(ConfigJson(connectionString: connectionString), CancellationToken.None)); + + ex.ToString().ShouldNotContain(Password, Case.Insensitive); + ex.ToString().ShouldNotContain(connectionString, Case.Insensitive); + // Still says something useful about *why*. + ex.Message.ShouldContain("could not open a connection"); + } + + /// + /// The leak that is real, and is the reason the parser's message is suppressed rather than trusted. + /// ADO.NET expects a value containing ; to be quoted. An unquoted one splits, and the + /// tail of the password is then parsed as a keyword — which + /// Microsoft.Data.SqlClient echoes verbatim (lower-cased) in + /// Keyword not supported: '…'. The first half of this test is the control: it proves the raw + /// provider really does leak, so the second half asserting the browser does not is not vacuous. + /// + [Fact] + public async Task Open_semicolonBearingPassword_neverLeaksTheProvidersEchoedKeyword() + { + const string tail = "OtOpcUaTailSecret"; + var connectionString = $"Server={DeadHost};Password=Sup3r;{tail};Connect Timeout=1"; + + // Control: the bare provider leaks the tail of the password into its own exception. + var raw = Should.Throw(() => + { + using var connection = new SqlServerDialect().Factory.CreateConnection()!; + connection.ConnectionString = connectionString; + }); + raw.ToString().ShouldContain(tail, Case.Insensitive); + + // The browser does not. + var browser = new SqlDriverBrowser(); + var ex = await Should.ThrowAsync(() => + browser.OpenAsync(ConfigJson(connectionString: connectionString), CancellationToken.None)); + + ex.ToString().ShouldNotContain(tail, Case.Insensitive); + ex.Message.ShouldContain("malformed"); + } + + /// + /// Nothing the browser logs — on the success path or the failure path — carries connection text. The + /// ref name is fair game; the string never is. + /// + [Fact] + public async Task Open_neverLogsTheConnectionString() + { + await using var db = await SqliteBrowseFixture.CreateAsync(); + var logger = new CapturingLogger(); + var browser = new SqlDriverBrowser(SqliteSelector, static _ => null, logger); + + await using (await browser.OpenAsync( + ConfigJson(connectionString: db.ConnectionString), CancellationToken.None)) + { + // Opened purely for its log output. + } + + var dead = $"Data Source=/otopcua-no-such-directory/never.db;Password={Password}"; + await Should.ThrowAsync(() => + browser.OpenAsync(ConfigJson(connectionString: dead), CancellationToken.None)); + + logger.Entries.ShouldNotBeEmpty(); + foreach (var entry in logger.Entries) + { + entry.ShouldNotContain(db.ConnectionString, Case.Insensitive); + entry.ShouldNotContain(Password, Case.Insensitive); + entry.ShouldNotContain("Data Source", Case.Insensitive); + } + } + + // ---- helpers ---- + + /// + /// Builds a driver-config blob. Serialized rather than interpolated so a connection string containing + /// JSON metacharacters (backslashes in a Data Source path, quotes in a password) escapes + /// correctly instead of producing malformed JSON the test would then misattribute. + /// + private static string ConfigJson( + string provider = "SqlServer", string? connectionStringRef = null, string? connectionString = null) + { + var map = new Dictionary { ["provider"] = provider }; + if (connectionStringRef is not null) map["connectionStringRef"] = connectionStringRef; + if (connectionString is not null) map["connectionString"] = connectionString; + return JsonSerializer.Serialize(map); + } + + /// Records every formatted log message so the hygiene assertions can read them back. + private sealed class CapturingLogger : ILogger + { + public List Entries { get; } = []; + + IDisposable? ILogger.BeginScope(TState state) => null; + + public bool IsEnabled(LogLevel logLevel) => true; + + public void Log( + LogLevel logLevel, EventId eventId, TState state, Exception? exception, + Func formatter) => + Entries.Add(formatter(state, exception)); + } +}