feat(sql): SqlDriverBrowser transient-connection open with env-ref + literal
Opens one transient DbConnection from a form-supplied driver-config blob and hands ownership to SqlBrowseSession, which closes it on disposal. A database is named either by connectionStringRef — resolved in the AdminUI process from Sql__ConnectionStrings__<ref>, with an unresolvable ref naming the exact missing variable — or by a pasted, session-only literal. The literal wins and the ref is then not read; it cannot arrive from a persisted blob (SqlDriverConfigDto has no such property), so its presence proves an operator typed it now. Credential hygiene is the point of the type: no cached config field, nothing persisted, and no connection text in any log line or exception. Live-probed: Microsoft.Data.SqlClient and Microsoft.Data.Sqlite keep connection-string values out of SqlException/SqliteException, but their connection-string PARSER echoes an unrecognised keyword verbatim — and an unquoted ';' inside a password splits, so the tail of the password is reported as a keyword (Password=Sup3r;SecretTail => "Keyword not supported: 'secrettail;connect timeout'."). The parser's message is therefore never surfaced; every other failure additionally passes through a substring redactor that drops the inner exception when it fires. DriverType comes from SqlDriver.DriverTypeName, the driver's interim local constant — DriverTypeNames.Sql is added by the driver-factory task. Claude-Session: https://claude.ai/code/session_01GASWkNEi68FSCtvr6rLoEW
This commit is contained in:
Joseph Doherty
@@ -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;
|
||||
|
||||
/// <summary>
|
||||
/// Opens a <b>transient</b> database connection from form-supplied JSON for the AdminUI schema picker,
|
||||
/// and hands it to a <see cref="SqlBrowseSession"/>. The AdminUI's <c>BrowseSessionRegistry</c> (its idle
|
||||
/// TTL reaper included) owns the returned session unchanged — nothing about the SQL browse needs a
|
||||
/// registry of its own.
|
||||
/// <para><b>Two ways to name the database, and only one of them is persisted.</b>
|
||||
/// <c>connectionStringRef</c> is the deployed form: a name resolved <em>in the AdminUI process</em> from
|
||||
/// the environment as <c>Sql__ConnectionStrings__<ref></c> (design §4.4), so a deployed artifact
|
||||
/// never carries credentials. <c>connectionString</c> is an ad-hoc, <b>session-only</b> 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.</para>
|
||||
/// <para><b>Precedence: a pasted literal wins over a ref</b>, and the ref is then not even read. The
|
||||
/// literal cannot arrive from a persisted blob — <see cref="SqlDriverConfigDto"/> 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 <em>ref</em>, never any connection text) so
|
||||
/// the override is visible rather than silent.</para>
|
||||
/// <para><b>Credential hygiene is the point of this type.</b> 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 <see cref="Sanitize"/> before it leaves this
|
||||
/// class.</para>
|
||||
/// </summary>
|
||||
public sealed class SqlDriverBrowser : IDriverBrowser
|
||||
{
|
||||
/// <summary>
|
||||
/// Prefix of the environment variable a <c>connectionStringRef</c> resolves through — the
|
||||
/// double-underscore form .NET configuration uses for the <c>Sql:ConnectionStrings:<ref></c>
|
||||
/// key path.
|
||||
/// </summary>
|
||||
public const string ConnectionStringEnvironmentPrefix = "Sql__ConnectionStrings__";
|
||||
|
||||
/// <summary>
|
||||
/// 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.
|
||||
/// </summary>
|
||||
private static readonly TimeSpan ConnectBudget = TimeSpan.FromSeconds(30);
|
||||
|
||||
/// <summary>
|
||||
/// Connection-string keys whose <em>values</em> 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 <b>not</b> redacted — "server X was not found"
|
||||
/// is the diagnostic the operator needs, and a hostname is not a credential.
|
||||
/// </summary>
|
||||
private static readonly string[] CredentialKeys =
|
||||
[
|
||||
"password", "pwd", "user id", "uid", "user", "username", "accesstoken", "access token",
|
||||
];
|
||||
|
||||
/// <summary>
|
||||
/// Kept deliberately identical in shape to the driver factory's options (design §5.1): enum-valued
|
||||
/// knobs are authored as their <em>names</em>, so the browser parses a given <c>DriverConfig</c> blob
|
||||
/// exactly as the runtime factory does. <see cref="JsonStringEnumConverter"/> also accepts ordinals,
|
||||
/// so a numeric config still binds.
|
||||
/// </summary>
|
||||
private static readonly JsonSerializerOptions JsonOpts = new()
|
||||
{
|
||||
UnmappedMemberHandling = JsonUnmappedMemberHandling.Skip,
|
||||
PropertyNameCaseInsensitive = true,
|
||||
Converters = { new JsonStringEnumConverter() },
|
||||
};
|
||||
|
||||
private readonly Func<SqlProvider, ISqlDialect?> _dialectSelector;
|
||||
private readonly Func<string, string?> _environmentReader;
|
||||
private readonly ILogger<SqlDriverBrowser> _logger;
|
||||
|
||||
/// <summary>Creates a browser. Every dependency has a production default, so DI may construct it bare.</summary>
|
||||
/// <param name="dialectSelector">
|
||||
/// Maps an authored <see cref="SqlProvider"/> onto the dialect that browses it, or <see langword="null"/>
|
||||
/// for a provider this build does not carry. Defaults to <see cref="DefaultDialectSelector"/>
|
||||
/// (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.
|
||||
/// </param>
|
||||
/// <param name="environmentReader">
|
||||
/// Reads one environment variable. Defaults to <see cref="Environment.GetEnvironmentVariable(string)"/>.
|
||||
/// Injectable because environment variables are process-global, and a test that sets one races every
|
||||
/// other test in the assembly.
|
||||
/// </param>
|
||||
/// <param name="logger">Optional; defaults to <see cref="NullLogger{T}"/>.</param>
|
||||
public SqlDriverBrowser(
|
||||
Func<SqlProvider, ISqlDialect?>? dialectSelector = null,
|
||||
Func<string, string?>? environmentReader = null,
|
||||
ILogger<SqlDriverBrowser>? logger = null)
|
||||
{
|
||||
_dialectSelector = dialectSelector ?? DefaultDialectSelector;
|
||||
_environmentReader = environmentReader ?? Environment.GetEnvironmentVariable;
|
||||
_logger = logger ?? NullLogger<SqlDriverBrowser>.Instance;
|
||||
}
|
||||
|
||||
/// <inheritdoc />
|
||||
/// <remarks>
|
||||
/// Sourced from <see cref="SqlDriver.DriverTypeName"/>, the driver's interim local constant, rather
|
||||
/// than from <c>DriverTypeNames</c> — the shared constant set is added by the driver-factory task,
|
||||
/// because adding a member there reddens <c>DriverTypeNamesGuardTests</c> until a factory is
|
||||
/// registered against it. Repointing that one constant repoints this too.
|
||||
/// </remarks>
|
||||
public string DriverType => SqlDriver.DriverTypeName;
|
||||
|
||||
/// <summary>The environment variable a <c>connectionStringRef</c> resolves through.</summary>
|
||||
/// <param name="connectionStringRef">The authored reference name.</param>
|
||||
/// <returns>The full environment-variable name, e.g. <c>Sql__ConnectionStrings__MesStaging</c>.</returns>
|
||||
public static string EnvironmentVariableFor(string connectionStringRef) =>
|
||||
ConnectionStringEnvironmentPrefix + connectionStringRef;
|
||||
|
||||
/// <summary>
|
||||
/// The providers this build can browse. v1 constructs SQL Server only; every other
|
||||
/// <see cref="SqlProvider"/> member is a reserved name, so it resolves to <see langword="null"/> and
|
||||
/// the caller reports it as unavailable rather than crashing on a null dialect.
|
||||
/// </summary>
|
||||
/// <param name="provider">The authored provider.</param>
|
||||
/// <returns>The dialect, or <see langword="null"/> when this build does not carry the provider.</returns>
|
||||
public static ISqlDialect? DefaultDialectSelector(SqlProvider provider) =>
|
||||
provider == SqlProvider.SqlServer ? new SqlServerDialect() : null;
|
||||
|
||||
/// <inheritdoc />
|
||||
/// <exception cref="InvalidOperationException">
|
||||
/// The configuration is absent / malformed, names a provider this build does not carry, names neither
|
||||
/// a <c>connectionStringRef</c> nor a literal, resolves a ref that is not set in the environment, or
|
||||
/// the connection could not be opened.
|
||||
/// </exception>
|
||||
public async Task<IBrowseSession> 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);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Opens one transient connection and hands ownership to the session. <b>The session closes the
|
||||
/// connection on its own disposal</b>, 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.
|
||||
/// </summary>
|
||||
private static async Task<IBrowseSession> 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);
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Picks the connection string, literal first. A blank literal is treated as absent (an untouched
|
||||
/// form field), never as an empty connection string.
|
||||
/// </summary>
|
||||
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}<ref>', 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;
|
||||
}
|
||||
|
||||
/// <summary>How the connection string was obtained, for the log. Carries no connection text.</summary>
|
||||
private static string DescribeSource(string? literal, string? reference) =>
|
||||
!string.IsNullOrWhiteSpace(literal)
|
||||
? "a pasted literal (session-only, not persisted)"
|
||||
: $"connectionStringRef '{reference}'";
|
||||
|
||||
/// <summary>
|
||||
/// Pulls the ad-hoc <c>connectionString</c> literal out of the raw JSON. It is read here rather than
|
||||
/// off the DTO because <see cref="SqlDriverConfigDto"/> deliberately has no such property — the
|
||||
/// persisted contract must not be able to carry a credential — and the DTO's
|
||||
/// <see cref="JsonUnmappedMemberHandling.Skip"/> would silently drop it.
|
||||
/// </summary>
|
||||
/// <returns>The literal, or <see langword="null"/> when absent, blank, or not a JSON string.</returns>
|
||||
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;
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Parses the raw configuration. The <see cref="JsonException"/> is <b>not</b> 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.
|
||||
/// </summary>
|
||||
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.)");
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Binds the typed configuration. A bind failure (e.g. an unknown <c>provider</c> name) is reported
|
||||
/// with the parser's own message redacted against the literal, which is known by now.
|
||||
/// </summary>
|
||||
private static SqlDriverConfigDto Deserialize(string configJson, string? literal)
|
||||
{
|
||||
try
|
||||
{
|
||||
return JsonSerializer.Deserialize<SqlDriverConfigDto>(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)));
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Turns a provider-side open failure into an exception that cannot carry the connection string.
|
||||
/// <para><b>Live-probed, not assumed.</b> <c>Microsoft.Data.SqlClient</c> and
|
||||
/// <c>Microsoft.Data.Sqlite</c> keep connection-string <em>values</em> out of their
|
||||
/// <c>SqlException</c>/<c>SqliteException</c> text (a failed connect says "the server was not found",
|
||||
/// never what it was handed). Their connection-string <b>parser</b> is the exception: it reports an
|
||||
/// unrecognised keyword by quoting it, and a value carrying an unquoted <c>;</c> — 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. <c>Password=Sup3r;SecretTail</c> yields
|
||||
/// <c>Keyword not supported: 'secrettail;connect timeout'.</c> — the credential, in the message,
|
||||
/// lower-cased and therefore invisible to a naive ordinal search for the value.</para>
|
||||
/// <para>So the parser's message (an <see cref="ArgumentException"/>) is <b>never</b> surfaced. Every
|
||||
/// other failure is additionally passed through the substring redactor as a backstop, and when that
|
||||
/// fires anywhere in the exception <em>chain</em> the inner exception is dropped too — keeping it
|
||||
/// would re-expose through <c>ToString()</c> exactly what the message just removed.</para>
|
||||
/// </summary>
|
||||
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);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// 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).
|
||||
/// </summary>
|
||||
private static List<string> CollectSecrets(string? connectionString)
|
||||
{
|
||||
var secrets = new List<string>();
|
||||
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<string> secrets)
|
||||
{
|
||||
foreach (var secret in secrets)
|
||||
{
|
||||
if (text.Contains(secret, StringComparison.OrdinalIgnoreCase)) return true;
|
||||
}
|
||||
|
||||
return false;
|
||||
}
|
||||
|
||||
private static string Redact(string text, List<string> secrets)
|
||||
{
|
||||
foreach (var secret in secrets)
|
||||
{
|
||||
text = text.Replace(secret, "[redacted]", StringComparison.OrdinalIgnoreCase);
|
||||
}
|
||||
|
||||
return text;
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user