diff --git a/docs/plans/2026-07-15-sql-poll-driver-design.md b/docs/plans/2026-07-15-sql-poll-driver-design.md index f125f91f..0aa81f58 100644 --- a/docs/plans/2026-07-15-sql-poll-driver-design.md +++ b/docs/plans/2026-07-15-sql-poll-driver-design.md @@ -588,6 +588,40 @@ SQL Server client works cross-platform, so `Sql` runs on macOS dev too (unlike G `dialect.QuoteIdentifier` (which escapes/rejects embedded quote characters). A table/column string in a `TagConfig` is validated against the live catalog (or an allow-list) before it's ever quoted into text; an unknown identifier rejects the tag (→ `BadNodeIdUnknown`) rather than executing. + + **Implemented (Gitea #496).** `SqlCatalogGate` + `SqlCatalogLoader`, run from + `SqlDriver.InitializeAsync` after the liveness check and before the driver reports Healthy. Shape, + and the decisions worth knowing before touching it: + + - **The catalog's spelling is substituted, not merely checked.** An accepted identifier is rewritten + to the string the catalog returned, so the text quoted into a poll query is one this driver read + back out of `ListSchemas`/`ListTables`/`ListColumns` — not operator input. Matching is + exact-ordinal first, then a *unique* case-insensitive hit (SQL Server's default collation is CI, so + case variants have always worked and rejecting them would break valid configs); an ambiguous CI + match under a case-sensitive collation is refused rather than guessed, because picking one would + publish another column's data under the operator's node. + - **Charset check first, catalog lookup second.** Each identifier goes through `QuoteIdentifier` for + its rejection rules *before* being looked up, so a name carrying a control or Unicode format + character is rejected **without its value being echoed** into a log line (Trojan-Source). A name + that passes is safe to render, which is why catalog-miss messages *do* name it — an operator + hunting a typo has to see what they wrote. + - **Rejection is per-tag and keeps the node.** The rejected tag is dropped from the *polled* table + but stays in the *authored* table, so it still materializes as a node and reads + `BadNodeIdUnknown` (§8.1's specified outcome) instead of vanishing from the address space. Every + drop is logged at Warning with the tag, the field and the reason. + - **A catalog that cannot be read faults Initialize; it does not reject every tag.** An unreadable + catalog is the *absence* of evidence about the tags, not evidence against them — rejecting all of + them would serve a confidently-empty address space and send the operator hunting typos that do not + exist. Zero visible schemas is treated the same way, because that is what a missing GRANT looks + like. Faulting lands `DriverInstanceActor` in Reconnecting with its retry timer running. + - **Bounded load:** one schema-list query, one default-schema scalar (`ISqlDialect.DefaultSchemaSql`, + added for this — an unqualified `TagValues` must resolve in whatever schema the *server* says, not + a guessed `dbo`), then one `ListTables` per distinct authored schema and one `ListColumns` per + distinct authored relation. It never enumerates the whole catalog, and the load runs under the same + wall-clock bound as the liveness check (R2-01 / STAB-14). + - **Accepted v1 limitation:** a 3-part `db.schema.table` (or a linked-server name) addresses a + catalog this connection cannot enumerate, so it cannot be allow-listed and is rejected with a + message pointing at the fix — expose the data through a view in the connected database. - **Treat any code path that builds SQL by string-concatenating a tag field as a defect** — enforce with a review checklist item + a unit test that feeds a malicious `keyValue`/`table` (`'; DROP TABLE …`) and asserts it either binds harmlessly (value) or is rejected (identifier), diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql/ISqlDialect.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql/ISqlDialect.cs index 9b15d22e..13f26329 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql/ISqlDialect.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql/ISqlDialect.cs @@ -14,11 +14,13 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Sql; /// parameterized in SQL, so the few that must appear as text are emitted through /// . Any code path that builds SQL by concatenating an authored tag field /// without going through is a defect (design §8.1). -/// Not yet implemented: design §8.1 also specifies that an authored table/column is -/// validated against the live catalog before it is ever quoted into text, so that an unknown identifier -/// rejects the tag. No such gate exists yet — an identifier reaches -/// straight from the authored TagConfig blob, so quoting is currently the only defence, -/// not a backstop behind an upstream filter. Scrutinise it accordingly. +/// Quoting is the backstop, not the only defence. Design §8.1's catalog gate is +/// implemented: resolves every authored table/column against the live +/// catalog at Initialize and replaces it with the catalog's own spelling, so an identifier +/// reaching on the poll path is a string this driver read back out of +/// / / — not +/// operator input. An identifier that resolves to nothing rejects its tag (which then publishes +/// BadNodeIdUnknown) rather than being quoted into a query against a nonexistent object. /// Public because Driver.Sql.Browser consumes it — the catalog SQL is the browse /// engine, so it is shared rather than duplicated. Implementations own their provider package; /// no provider-specific type appears in this signature ( is the abstract @@ -89,6 +91,20 @@ public interface ISqlDialect /// Catalog query listing schemas — the browser's RootAsync level. Takes no parameters. string ListSchemasSql { get; } + /// + /// Scalar query returning the schema an unqualified object name resolves to for the connecting + /// principal — T-SQL's SELECT SCHEMA_NAME(). Takes no parameters. + /// + /// + /// Needed by : a tag authored as TagValues rather than + /// dbo.TagValues has to be looked up in some schema, and guessing dbo would be a + /// silent lie on any estate that maps its service accounts to their own default schema. Asking the + /// server is the only answer that matches how the poll query itself will resolve the name. + /// It is a query rather than a name because the answer is per-connection, not per-dialect + /// — the same dialect resolves differently for two different logins. + /// + string DefaultSchemaSql { get; } + /// Catalog query listing tables + views in one schema — the browser's schema-expand level. Bind @schema. string ListTablesSql { get; } diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql/SqlCatalog.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql/SqlCatalog.cs new file mode 100644 index 00000000..16b8def1 --- /dev/null +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql/SqlCatalog.cs @@ -0,0 +1,137 @@ +namespace ZB.MOM.WW.OtOpcUa.Driver.Sql; + +/// +/// An immutable snapshot of the parts of a database's catalog the authored tags actually name — the +/// allow-list design §8.1 requires identifiers to come from. +/// This type holds catalog strings only. Every name in it was read back out of +/// / / +/// ; nothing an operator typed is ever stored here. That is what +/// lets hand the planner catalog spellings rather than authored ones, so the +/// identifier text in an emitted query is a string the database gave us. +/// Deliberately partial. Only the schemas/tables the authored tags name are loaded — a +/// driver polling three tables must not enumerate a warehouse's ten thousand. A name absent from this +/// snapshot therefore means "not found when we looked", which is exactly the question the gate +/// asks. +/// +public sealed class SqlCatalog +{ + private readonly IReadOnlyList _schemas; + + /// Canonical schema → canonical table names in it. + private readonly IReadOnlyDictionary> _tablesBySchema; + + /// Canonical schema.table → that relation's canonical column names. + private readonly IReadOnlyDictionary> _columnsByTable; + + /// Constructs a catalog snapshot. + /// The schema an unqualified object name resolves to for this connection. + /// Every schema the connection can see. + /// Canonical schema → its tables/views. + /// Canonical schema.table → its columns. + internal SqlCatalog( + string defaultSchema, + IReadOnlyList schemas, + IReadOnlyDictionary> tablesBySchema, + IReadOnlyDictionary> columnsByTable) + { + DefaultSchema = defaultSchema; + _schemas = schemas; + _tablesBySchema = tablesBySchema; + _columnsByTable = columnsByTable; + } + + /// The schema an unqualified authored object name is resolved in. + public string DefaultSchema { get; } + + /// Resolves an authored schema name to the catalog's own spelling. + /// The authored schema, or null/blank for the default schema. + /// The catalog's spelling, when resolved. + /// when the schema exists (or the default schema is used). + public bool TryResolveSchema(string? authored, out string canonical) + { + if (string.IsNullOrWhiteSpace(authored)) + { + canonical = DefaultSchema; + return true; + } + + return TryMatch(_schemas, authored, out canonical); + } + + /// Resolves an authored table/view name within a canonical schema. + /// The schema, already resolved through . + /// The authored table or view name. + /// The catalog's spelling, when resolved. + /// when the relation exists in that schema. + public bool TryResolveTable(string canonicalSchema, string authored, out string canonical) + { + canonical = string.Empty; + return _tablesBySchema.TryGetValue(canonicalSchema, out var tables) + && TryMatch(tables, authored, out canonical); + } + + /// Resolves an authored column name within a canonical relation. + /// The resolved schema. + /// The resolved table or view. + /// The authored column name. + /// The catalog's spelling, when resolved. + /// when the column exists on that relation. + public bool TryResolveColumn( + string canonicalSchema, string canonicalTable, string authored, out string canonical) + { + canonical = string.Empty; + return _columnsByTable.TryGetValue(QualifiedKey(canonicalSchema, canonicalTable), out var columns) + && TryMatch(columns, authored, out canonical); + } + + /// The dictionary key for a canonical relation. + /// The canonical schema. + /// The canonical table. + /// The composite key. + internal static string QualifiedKey(string schema, string table) => schema + "." + table; + + /// + /// Matches an authored name against catalog spellings: an exact (ordinal) hit wins, otherwise a + /// unique case-insensitive hit is accepted and its catalog spelling returned. + /// + /// + /// Why case-insensitive at all. SQL Server's default collation is case-insensitive, so + /// num_value and NUM_VALUE genuinely name the same column and both work today. Matching + /// ordinally would reject configurations that have always been valid — a gate that breaks working + /// deployments is not defence in depth. + /// Why exact-first, and why ambiguity fails. On a case-sensitive collation a + /// relation may legitimately carry both Value and value. Preferring the exact match keeps + /// such a config resolving to what the operator wrote, and refusing to choose between two + /// case-insensitive candidates is the only safe answer — silently picking one would publish a different + /// column's data under the operator's node, which is worse than rejecting the tag. + /// + /// The catalog spellings to match against. + /// The authored name. + /// The catalog's spelling, when matched. + /// on an unambiguous match. + internal static bool TryMatch(IReadOnlyList candidates, string authored, out string canonical) + { + foreach (var candidate in candidates) + { + if (!string.Equals(candidate, authored, StringComparison.Ordinal)) continue; + canonical = candidate; + return true; + } + + canonical = string.Empty; + var matches = 0; + foreach (var candidate in candidates) + { + if (!string.Equals(candidate, authored, StringComparison.OrdinalIgnoreCase)) continue; + if (++matches > 1) + { + canonical = string.Empty; + return false; + } + + canonical = candidate; + } + + return matches == 1; + } +} diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql/SqlCatalogGate.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql/SqlCatalogGate.cs new file mode 100644 index 00000000..3485da74 --- /dev/null +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql/SqlCatalogGate.cs @@ -0,0 +1,241 @@ +using ZB.MOM.WW.OtOpcUa.Driver.Sql.Contracts; + +namespace ZB.MOM.WW.OtOpcUa.Driver.Sql; + +/// +/// One tag the gate refused, and why. Carried rather than logged in place so the caller decides the log +/// shape and the caller's tests can assert on the reason. +/// +/// The rejected tag's identity — its RawPath, which is trusted structure, not blob content. +/// The field that failed (e.g. ValueColumn). +/// An operator-actionable explanation, safe to log. +public sealed record SqlCatalogRejection(string RawPath, string Field, string Reason); + +/// The outcome of applying a to a set of authored tags. +/// +/// The surviving definitions, rewritten to catalog spellings. Safe to hand to +/// . +/// +/// Every tag the gate refused, in input order. +public sealed record SqlCatalogGateResult( + IReadOnlyList Accepted, + IReadOnlyList Rejected); + +/// +/// Design §8.1's identifier gate: validates every authored table/column against the live catalog +/// before it can be quoted into SQL text, and replaces it with the catalog's own spelling. +/// +/// +/// What this closes. Until this existed, was the +/// sole defence: an identifier went from the authored TagConfig blob straight into a command text, +/// bracket-quoted. The residual risk was bounded — a hostile name is quoted into one nonexistent object, +/// the query fails after the connection opens, and the tag Bad-codes — but "bounded" is not "filtered", +/// and the design promised a filter. With the gate in place the identifier text in an emitted query is a +/// string this driver read back out of the catalog, so quoting is a backstop behind an allow-list rather +/// than the whole story. +/// Charset first, catalog second. Each identifier is passed through +/// before it is looked up, purely for its rejection rules +/// (control characters, Unicode format characters, over-length). That ordering is deliberate: a name that +/// fails the charset check is rejected without its value being echoed, so nothing carrying a bidi +/// override or a NUL can reach a log line through this type's rejection messages. A name that passes has +/// been proven safe to render, which is why the catalog-miss messages can afford to name it — and they +/// must, or an operator has no way to find the typo. +/// Rejection is per-tag, never driver-wide. A tag naming a dropped column is dropped from the +/// driver's table, so its RawPath resolves to nothing and it publishes +/// — design §8.1's specified outcome — while every other tag +/// on that database keeps polling. This mirrors the "one malformed blob must not take the whole driver +/// down" rule the tag-table build already follows. +/// +public static class SqlCatalogGate +{ + /// + /// The most name parts this gate can validate. A three-part db.schema.table (or a + /// four-part linked-server name) addresses a catalog this connection's INFORMATION_SCHEMA + /// cannot see, so it cannot be allow-listed. + /// + private const int MaxNameParts = 2; + + /// + /// Applies to , returning the accepted tags + /// with catalog-spelled identifiers and the rejected ones with reasons. + /// + /// The authored definitions, as parsed from their TagConfig blobs. + /// The catalog snapshot to validate against. + /// Supplies the identifier charset rules via . + /// The accepted and rejected sets. + /// A required argument is null. + public static SqlCatalogGateResult Apply( + IEnumerable definitions, SqlCatalog catalog, ISqlDialect dialect) + { + ArgumentNullException.ThrowIfNull(definitions); + ArgumentNullException.ThrowIfNull(catalog); + ArgumentNullException.ThrowIfNull(dialect); + + var accepted = new List(); + var rejected = new List(); + + foreach (var definition in definitions) + { + ArgumentNullException.ThrowIfNull(definition); + if (TryCanonicalize(definition, catalog, dialect, out var canonical, out var rejection)) + accepted.Add(canonical!); + else + rejected.Add(rejection!); + } + + return new SqlCatalogGateResult(accepted, rejected); + } + + /// Resolves one definition's identifiers, or explains the first failure. + private static bool TryCanonicalize( + SqlTagDefinition definition, + SqlCatalog catalog, + ISqlDialect dialect, + out SqlTagDefinition? canonical, + out SqlCatalogRejection? rejection) + { + canonical = null; + rejection = null; + + if (!TryResolveRelation(definition, catalog, dialect, out var schema, out var table, out rejection)) + return false; + + // Every identifier-bearing field, paired with its name for the rejection message. A field the tag's + // model does not use is null and simply skipped — the planner enforces which are required. + var resolved = new Dictionary(StringComparer.Ordinal); + foreach (var (field, authored) in new (string Field, string? Authored)[] + { + (nameof(SqlTagDefinition.KeyColumn), definition.KeyColumn), + (nameof(SqlTagDefinition.ValueColumn), definition.ValueColumn), + (nameof(SqlTagDefinition.TimestampColumn), definition.TimestampColumn), + (nameof(SqlTagDefinition.ColumnName), definition.ColumnName), + (nameof(SqlTagDefinition.RowSelectorColumn), definition.RowSelectorColumn), + (nameof(SqlTagDefinition.RowSelectorTopByTimestamp), definition.RowSelectorTopByTimestamp), + }) + { + if (string.IsNullOrWhiteSpace(authored)) + { + resolved[field] = authored; + continue; + } + + if (!IsRenderableIdentifier(authored, dialect)) + { + rejection = new SqlCatalogRejection(definition.Name, field, + $"the authored {field} is not a usable identifier (it is over-long, or contains control " + + "or Unicode format characters); the value is withheld from this message because it " + + "cannot be safely rendered"); + return false; + } + + if (!catalog.TryResolveColumn(schema, table, authored, out var column)) + { + rejection = new SqlCatalogRejection(definition.Name, field, + $"column '{authored}' does not exist on '{schema}.{table}' (or matches more than one " + + "column under a case-sensitive collation)"); + return false; + } + + resolved[field] = column; + } + + canonical = definition with + { + Table = SqlCatalog.QualifiedKey(schema, table), + KeyColumn = resolved[nameof(SqlTagDefinition.KeyColumn)], + ValueColumn = resolved[nameof(SqlTagDefinition.ValueColumn)], + TimestampColumn = resolved[nameof(SqlTagDefinition.TimestampColumn)], + ColumnName = resolved[nameof(SqlTagDefinition.ColumnName)], + RowSelectorColumn = resolved[nameof(SqlTagDefinition.RowSelectorColumn)], + RowSelectorTopByTimestamp = resolved[nameof(SqlTagDefinition.RowSelectorTopByTimestamp)], + }; + return true; + } + + /// Splits and resolves the authored table to a canonical schema + relation. + private static bool TryResolveRelation( + SqlTagDefinition definition, + SqlCatalog catalog, + ISqlDialect dialect, + out string schema, + out string table, + out SqlCatalogRejection? rejection) + { + schema = string.Empty; + table = string.Empty; + rejection = null; + const string Field = nameof(SqlTagDefinition.Table); + + if (string.IsNullOrWhiteSpace(definition.Table)) + { + rejection = new SqlCatalogRejection(definition.Name, Field, + "the tag has no 'table'; author the table or view to read"); + return false; + } + + var parts = definition.Table.Split('.'); + if (parts.Length > MaxNameParts) + { + rejection = new SqlCatalogRejection(definition.Name, Field, + $"'{definition.Table}' is a {parts.Length}-part name. Only 'table' and 'schema.table' can be " + + "validated against this connection's catalog, so a cross-database or linked-server name " + + "cannot be allow-listed; expose the data through a view in this database instead"); + return false; + } + + var authoredSchema = parts.Length == MaxNameParts ? parts[0] : null; + var authoredTable = parts[^1]; + + foreach (var part in parts) + { + if (IsRenderableIdentifier(part, dialect)) continue; + rejection = new SqlCatalogRejection(definition.Name, Field, + "the authored table name has a part that is not a usable identifier (empty, over-long, or " + + "containing control or Unicode format characters); the value is withheld from this message " + + "because it cannot be safely rendered"); + return false; + } + + if (!catalog.TryResolveSchema(authoredSchema, out schema)) + { + rejection = new SqlCatalogRejection(definition.Name, Field, + $"schema '{authoredSchema}' does not exist (or matches more than one schema under a " + + "case-sensitive collation)"); + return false; + } + + if (!catalog.TryResolveTable(schema, authoredTable, out table)) + { + rejection = new SqlCatalogRejection(definition.Name, Field, + $"table or view '{authoredTable}' does not exist in schema '{schema}'" + + (authoredSchema is null + ? $" (the connection's default schema — qualify the name as 'schema.{authoredTable}' if it lives elsewhere)" + : string.Empty)); + return false; + } + + return true; + } + + /// + /// True when the dialect's quoting rules accept — i.e. it is safe both to + /// embed in SQL and to render into a log line or an AdminUI label. + /// + /// + /// Uses as the charset authority rather than duplicating its + /// rules, so the two can never disagree about what a legal identifier is. The quoted result is + /// discarded: only whether it threw is interesting here. + /// + private static bool IsRenderableIdentifier(string identifier, ISqlDialect dialect) + { + try + { + dialect.QuoteIdentifier(identifier); + return true; + } + catch (ArgumentException) + { + return false; + } + } +} diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql/SqlCatalogLoader.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql/SqlCatalogLoader.cs new file mode 100644 index 00000000..f0802333 --- /dev/null +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql/SqlCatalogLoader.cs @@ -0,0 +1,189 @@ +using System.Data; +using System.Data.Common; + +namespace ZB.MOM.WW.OtOpcUa.Driver.Sql; + +/// +/// Loads the slice the authored tags need, over one connection, using only the +/// dialect's catalog SQL (design §8.1). +/// +/// +/// Bounded by what is authored, not by what exists. One query for the schema list, one for +/// the default schema, then one per distinct authored schema and +/// one per distinct authored relation. A driver polling three +/// tables issues a handful of round-trips at Initialize and none thereafter; it never enumerates a +/// warehouse. +/// Every parameter is bound. Authored names reach the catalog queries as +/// @schema/@table parameters — the same discipline the schema browser follows — so loading +/// the allow-list cannot itself be an injection vector. No identifier is quoted into text on this path. +/// Failures throw; they do not degrade to an empty catalog. An empty catalog would reject +/// every tag, which is indistinguishable at the OPC UA surface from a database whose objects were all +/// dropped. A caller that cannot read the catalog has not learned that the tags are invalid — it has +/// learned nothing — and must fail its Initialize so the driver retries rather than serving a +/// confidently-empty address space. +/// +internal static class SqlCatalogLoader +{ + /// Column alias projects. + private const string SchemaColumn = "TABLE_SCHEMA"; + + /// Column alias projects. + private const string TableNameColumn = "TABLE_NAME"; + + /// Column alias projects. + private const string ColumnNameColumn = "COLUMN_NAME"; + + /// + /// Reads the catalog slice covering . + /// + /// An already-open connection. Not disposed here; the caller owns it. + /// Supplies the catalog SQL. + /// The distinct authored table strings, as written in the blobs. + /// Per-command server-side backstop. + /// Cancellation token for the operation. + /// The loaded catalog. + /// The connection can see no schemas at all. + internal static async Task LoadAsync( + DbConnection connection, + ISqlDialect dialect, + IReadOnlyCollection authoredTables, + TimeSpan commandTimeout, + CancellationToken cancellationToken) + { + var timeoutSeconds = Math.Max(1, (int)Math.Ceiling(commandTimeout.TotalSeconds)); + + var schemas = await QueryColumnAsync( + connection, dialect.ListSchemasSql, SchemaColumn, + static _ => { }, timeoutSeconds, cancellationToken).ConfigureAwait(false); + + // Zero schemas is a permissions or visibility symptom, never a real database. Rejecting every tag on + // that basis would report an authoring fault for what is actually a grant problem, and would send the + // operator to the wrong system — the same misdiagnosis the driver's health classification avoids. + if (schemas.Count == 0) + throw new InvalidOperationException( + "the catalog returned no schemas at all; the connecting principal most likely cannot read " + + "the catalog views, which is a grant problem rather than a tag-authoring one"); + + var defaultSchema = await ReadDefaultSchemaAsync( + connection, dialect, timeoutSeconds, cancellationToken).ConfigureAwait(false); + + var tablesBySchema = new Dictionary>(StringComparer.Ordinal); + var columnsByTable = new Dictionary>(StringComparer.Ordinal); + + // Resolving here goes through SqlCatalog.TryMatch — the SAME matcher the gate will use — so the + // loader and the gate can never disagree about which relation an authored name resolved to. Loading + // one table and validating against another would be a silent wrong-column defect. + foreach (var authored in authoredTables) + { + if (string.IsNullOrWhiteSpace(authored)) continue; + + var parts = authored.Split('.'); + if (parts.Length > 2) continue; // the gate rejects these by name; nothing to load. + + var authoredTable = parts[^1]; + string schema; + if (parts.Length == 2) + { + if (!SqlCatalog.TryMatch(schemas, parts[0], out schema)) continue; + } + else + { + schema = defaultSchema; + } + + if (!tablesBySchema.TryGetValue(schema, out var tables)) + { + tables = await QueryColumnAsync( + connection, dialect.ListTablesSql, TableNameColumn, + command => Bind(command, "@schema", schema), + timeoutSeconds, cancellationToken).ConfigureAwait(false); + tablesBySchema[schema] = tables; + } + + if (!SqlCatalog.TryMatch(tables, authoredTable, out var table)) continue; + + var key = SqlCatalog.QualifiedKey(schema, table); + if (columnsByTable.ContainsKey(key)) continue; + + columnsByTable[key] = await QueryColumnAsync( + connection, dialect.ListColumnsSql, ColumnNameColumn, + command => + { + Bind(command, "@schema", schema); + Bind(command, "@table", table); + }, + timeoutSeconds, cancellationToken).ConfigureAwait(false); + } + + return new SqlCatalog(defaultSchema, schemas, tablesBySchema, columnsByTable); + } + + /// + /// Reads the connection's default schema, falling back to the sole visible schema when the dialect's + /// query answers null/blank. + /// + /// + /// A null answer is possible (a login with no default schema mapped). Falling back to the single + /// visible schema keeps the common one-schema database working; with several visible schemas there is + /// no defensible guess, so unqualified names simply will not resolve and the gate says so by name. + /// + private static async Task ReadDefaultSchemaAsync( + DbConnection connection, ISqlDialect dialect, int timeoutSeconds, CancellationToken cancellationToken) + { + var command = connection.CreateCommand(); + await using (command.ConfigureAwait(false)) + { + command.CommandText = dialect.DefaultSchemaSql; + command.CommandTimeout = timeoutSeconds; + var value = await command.ExecuteScalarAsync(cancellationToken).ConfigureAwait(false); + var schema = value is null or DBNull ? null : value.ToString(); + return string.IsNullOrWhiteSpace(schema) ? string.Empty : schema; + } + } + + /// Runs one catalog query and projects a single string column from every row. + private static async Task> QueryColumnAsync( + DbConnection connection, + string sql, + string columnName, + Action bind, + int timeoutSeconds, + CancellationToken cancellationToken) + { + var results = new List(); + + var command = connection.CreateCommand(); + await using (command.ConfigureAwait(false)) + { + command.CommandText = sql; + command.CommandTimeout = timeoutSeconds; + bind(command); + + var reader = await command + .ExecuteReaderAsync(CommandBehavior.SingleResult, cancellationToken).ConfigureAwait(false); + await using (reader.ConfigureAwait(false)) + { + var ordinal = -1; + while (await reader.ReadAsync(cancellationToken).ConfigureAwait(false)) + { + if (ordinal < 0) ordinal = reader.GetOrdinal(columnName); + if (reader.IsDBNull(ordinal)) continue; + var value = reader.GetValue(ordinal)?.ToString(); + if (!string.IsNullOrWhiteSpace(value)) results.Add(value); + } + } + } + + return results; + } + + /// Binds one catalog-query parameter — the only way an authored name reaches the database here. + private static void Bind(DbCommand command, string name, string value) + { + var parameter = command.CreateParameter(); + parameter.ParameterName = name; + parameter.DbType = DbType.String; + parameter.Value = value; + command.Parameters.Add(parameter); + } +} diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql/SqlDriver.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql/SqlDriver.cs index 9d2396bf..e99c3355 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql/SqlDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql/SqlDriver.cs @@ -74,7 +74,23 @@ public sealed class SqlDriver private IReadOnlyDictionary _tagsByRawPath = new Dictionary(StringComparer.Ordinal); - /// Resolves a read/subscribe RawPath to its authored definition — a single table hit, or a miss. + /// + /// The subset of that survived the design §8.1 catalog gate, with every + /// identifier rewritten to the catalog's own spelling. This — not the authored table — is what a + /// read or a poll resolves against, so an identifier that never appeared in the catalog can never + /// reach a query. + /// Why the two tables are separate. A tag the gate rejected must still exist as a + /// node: §8.1's specified outcome is that it publishes + /// , and a status code can only be published by a node + /// that is there. Dropping rejected tags from the authored table too would delete the node instead, + /// turning a diagnosable Bad quality into a silently missing address-space entry — a strictly worse + /// failure for the operator trying to find their typo. + /// Swapped atomically, for the same reason the authored table is. + /// + private IReadOnlyDictionary _polledByRawPath = + new Dictionary(StringComparer.Ordinal); + + /// Resolves a read/subscribe RawPath to its catalog-validated definition, or a miss. private readonly EquipmentTagRefResolver _resolver; /// @@ -182,8 +198,18 @@ public sealed class SqlDriver /// The current authored-tag snapshot. Read through a barrier — swaps it. private IReadOnlyDictionary Tags => Volatile.Read(ref _tagsByRawPath); - /// The resolver's lookup: RawPath → authored definition, or null on a miss. - private SqlTagDefinition? Lookup(string rawPath) => Tags.GetValueOrDefault(rawPath); + /// + /// The catalog-validated snapshot. Read through a barrier — swaps it. + /// + private IReadOnlyDictionary PolledTags => Volatile.Read(ref _polledByRawPath); + + /// + /// The resolver's lookup: RawPath → catalog-validated definition, or null on a miss. + /// Deliberately reads rather than : a tag the gate + /// rejected must miss here so the reader publishes + /// , exactly as design §8.1 specifies. + /// + private SqlTagDefinition? Lookup(string rawPath) => PolledTags.GetValueOrDefault(rawPath); // ---- IDriver lifecycle ---- @@ -196,11 +222,16 @@ public sealed class SqlDriver /// I/O, and on failure this method records and rethrows — /// DriverInstanceActor reads a throw as InitializeFailed and lands in Reconnecting with its /// retry timer running, which is exactly the recovery a database that is merely down needs. + /// The liveness check is then followed by the design §8.1 catalog gate + /// (), which is the second and last piece of I/O. It runs after + /// liveness because it needs a working connection, and before the driver reports Healthy because the + /// tag table it publishes must be the validated one — a poll must never see an unvalidated + /// identifier. /// /// The driver configuration JSON (unused; see remarks). /// Cancellation token for the operation. /// A task that represents the asynchronous operation. - /// The database could not be reached. + /// The database could not be reached, or its catalog could not be read. public async Task InitializeAsync(string driverConfigJson, CancellationToken cancellationToken) { WriteHealth(new DriverHealth(DriverState.Initializing, null, null)); @@ -234,6 +265,36 @@ public sealed class SqlDriver throw new InvalidOperationException(message, ex); } + // Separate try from liveness so the operator surface names the stage that actually failed: "could + // not reach the database" and "reached it but could not read its catalog" send an operator to + // different places, and the second is usually a GRANT rather than an outage. + try + { + await ApplyCatalogGateAsync(cancellationToken).ConfigureAwait(false); + } + catch (OperationCanceledException) when (cancellationToken.IsCancellationRequested) + { + WriteHealth(new DriverHealth(DriverState.Unknown, null, null)); + throw; + } + catch (Exception ex) + { + // Same discipline as above: exception TYPE only on the operator surface, full exception to the + // log sink. A catalog read that fails is NOT evidence the tags are wrong — it is the absence of + // evidence — so this faults the driver (and retries) rather than rejecting every tag and serving + // a confidently-empty address space. + var message = + $"Sql driver '{_driverInstanceId}' reached {Endpoint} but could not read its catalog to " + + $"validate authored tables/columns: {ex.GetType().Name}. Check that the connecting principal " + + "can read the catalog views."; + _logger.LogError(ex, + "Sql driver {DriverInstanceId} catalog validation against {Endpoint} failed.", + _driverInstanceId, Endpoint); + WriteHealth(new DriverHealth(DriverState.Faulted, null, message)); + TransitionHostTo(HostState.Stopped); + throw new InvalidOperationException(message, ex); + } + WriteHealth(new DriverHealth(DriverState.Healthy, DateTime.UtcNow, null)); TransitionHostTo(HostState.Running); _logger.LogInformation( @@ -594,6 +655,94 @@ public sealed class SqlDriver } Volatile.Write(ref _tagsByRawPath, table); + + // Nothing is pollable until the catalog gate has validated it. Clearing here (rather than leaving a + // previous generation's validated table in place) means a Reinitialize whose gate then fails cannot + // keep polling the OLD identifiers against a database whose schema may be exactly what changed. + Volatile.Write(ref _polledByRawPath, new Dictionary(StringComparer.Ordinal)); + } + + // ---- catalog gate (design §8.1) ---- + + /// + /// Validates every authored identifier against the live catalog and republishes the tag table with + /// catalog spellings, dropping the tags that do not resolve (design §8.1). + /// + /// + /// Why this must run before the driver reports Healthy. The table this swaps in is what + /// every poll resolves against. Running it later — or in the background — would leave a window in + /// which an unvalidated identifier could be quoted into a query, which is the entire hole the gate + /// exists to close. + /// A dropped tag is not a driver fault. It disappears from the table, so its RawPath + /// resolves to nothing and it publishes — one operator + /// typo must not stop the other tags on that database, the same rule + /// follows for a malformed blob. Each drop is logged at Warning with the + /// tag, the field and the reason, because a node that silently goes Bad with nothing in the log is + /// unsupportable. + /// No tags ⇒ no I/O. A driver with nothing authored has nothing to validate, and issuing + /// catalog queries to prove that would be a round-trip that can only fail. + /// Bounded by the same wall-clock discipline as the liveness check (R2-01 / STAB-14): the work + /// runs on the thread pool so a provider implementing the async path synchronously blocks a pool + /// thread rather than wedging Initialize forever with no retry. + /// + /// The caller's token. + /// A task that completes when the tag table has been validated and republished. + private async Task ApplyCatalogGateAsync(CancellationToken cancellationToken) + { + var authored = Tags; + if (authored.Count == 0) return; + + var tables = authored.Values + .Select(definition => definition.Table) + .Where(table => !string.IsNullOrWhiteSpace(table)) + .Distinct(StringComparer.OrdinalIgnoreCase) + .ToArray(); + + var catalog = await RunBoundedAsync( + token => LoadCatalogAsync(tables, token), + _options.OperationTimeout, + "the catalog queries did not return", + cancellationToken).ConfigureAwait(false); + + var result = SqlCatalogGate.Apply(authored.Values, catalog, _dialect); + + foreach (var rejection in result.Rejected) + { + _logger.LogWarning( + "Sql tag '{Tag}' rejected by the catalog gate: {Field} — {Reason}. It will publish " + + "BadNodeIdUnknown until the tag or the database is corrected. Driver={DriverInstanceId}", + rejection.RawPath, rejection.Field, rejection.Reason, _driverInstanceId); + } + + var validated = new Dictionary(result.Accepted.Count, StringComparer.Ordinal); + foreach (var definition in result.Accepted) validated[definition.Name] = definition; + + // Only the POLLED table is replaced. The authored table is left intact so every authored tag keeps + // its node and a rejected one publishes BadNodeIdUnknown rather than vanishing (see _polledByRawPath). + Volatile.Write(ref _polledByRawPath, validated); + + _logger.LogInformation( + "Sql driver {DriverInstanceId} catalog gate accepted {Accepted} of {Total} authored tag(s) " + + "across {Tables} table(s) on {Endpoint}.", + _driverInstanceId, result.Accepted.Count, authored.Count, tables.Length, Endpoint); + } + + /// Opens one connection and reads the catalog slice the authored tables need. + private async Task LoadCatalogAsync( + IReadOnlyCollection authoredTables, CancellationToken cancellationToken) + { + var connection = _factory.CreateConnection() + ?? throw new InvalidOperationException( + $"the {_dialect.Provider} provider factory returned no connection."); + + await using (connection.ConfigureAwait(false)) + { + connection.ConnectionString = _connectionString; + await connection.OpenAsync(cancellationToken).ConfigureAwait(false); + return await SqlCatalogLoader + .LoadAsync(connection, _dialect, authoredTables, _options.CommandTimeout, cancellationToken) + .ConfigureAwait(false); + } } // ---- liveness ---- @@ -611,18 +760,50 @@ public sealed class SqlDriver /// /// The caller's token. /// A task that completes when the database has answered. - private async Task VerifyLivenessAsync(CancellationToken cancellationToken) + private Task VerifyLivenessAsync(CancellationToken cancellationToken) => + RunBoundedAsync( + async token => { await PingAsync(token).ConfigureAwait(false); return true; }, + _options.CommandTimeout, + "the liveness statement did not return", + cancellationToken); + + /// + /// Runs under a hard wall-clock , on the thread pool, + /// converting a breach into a worded from . + /// + /// + /// The R2-01 / STAB-14 shape, shared by the two pieces of I/O Initialize performs. It exists + /// because a token is not a deadline: some ADO.NET providers implement the async path synchronously, + /// and a wedged socket can hang inside the provider's own cancellation handshake. Without an + /// independent wall clock, DriverInstanceActor's init task would never complete and the driver + /// would sit in Connecting forever with no retry — the exact frozen-peer wedge the S7 driver shipped. + /// Both the linked CTS deadline and an outer wait are used, because + /// each covers a case the other does not: the CTS handles a provider that does honour + /// cancellation, and the outer wait handles one that does not. An abandoned attempt keeps ownership of + /// its own connection and disposes it; observes its eventual fault. + /// + /// The work's result type. + /// The operation to bound. Receives the deadline-linked token. + /// The wall-clock allowance. + /// Sentence fragment naming the operation, e.g. "the catalog queries did not return". + /// The caller's token. + /// The work's result. + /// The budget elapsed first. + private static async Task RunBoundedAsync( + Func> work, + TimeSpan budget, + string what, + CancellationToken cancellationToken) { - var budget = _options.CommandTimeout; var deadline = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); - Task work; + Task running; try { deadline.CancelAfter(budget); - work = Task.Run( + running = Task.Run( async () => { - try { await PingAsync(deadline.Token).ConfigureAwait(false); } + try { return await work(deadline.Token).ConfigureAwait(false); } finally { deadline.Dispose(); } }, CancellationToken.None); @@ -636,20 +817,18 @@ public sealed class SqlDriver try { - await work.WaitAsync(budget, cancellationToken).ConfigureAwait(false); + return await running.WaitAsync(budget, cancellationToken).ConfigureAwait(false); } catch (TimeoutException) { - Detach(work); - throw new TimeoutException( - $"the liveness statement did not return within {(int)budget.TotalMilliseconds} ms."); + Detach(running); + throw new TimeoutException($"{what} within {(int)budget.TotalMilliseconds} ms."); } catch (OperationCanceledException) when (!cancellationToken.IsCancellationRequested) { // Our deadline fired and the provider DID honour the linked token. - Detach(work); - throw new TimeoutException( - $"the liveness statement did not return within {(int)budget.TotalMilliseconds} ms."); + Detach(running); + throw new TimeoutException($"{what} within {(int)budget.TotalMilliseconds} ms."); } } @@ -752,6 +931,7 @@ public sealed class SqlDriver { await _poll.DisposeAsync().ConfigureAwait(false); Volatile.Write(ref _tagsByRawPath, new Dictionary(StringComparer.Ordinal)); + Volatile.Write(ref _polledByRawPath, new Dictionary(StringComparer.Ordinal)); _resolver.Clear(); // no-op in v3 (the resolver reads the live table by closure); kept for symmetry } } diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql/SqlServerDialect.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql/SqlServerDialect.cs index e666856f..ca38e8bb 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql/SqlServerDialect.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql/SqlServerDialect.cs @@ -44,6 +44,12 @@ public sealed class SqlServerDialect : ISqlDialect public string ListSchemasSql => "SELECT DISTINCT TABLE_SCHEMA FROM INFORMATION_SCHEMA.TABLES ORDER BY TABLE_SCHEMA"; + /// + /// SCHEMA_NAME() with no argument returns the calling principal's default schema — the one an + /// unqualified object name resolves in, which is precisely the question the catalog gate asks. + /// + public string DefaultSchemaSql => "SELECT SCHEMA_NAME()"; + /// public string ListTablesSql => "SELECT TABLE_NAME, TABLE_TYPE FROM INFORMATION_SCHEMA.TABLES " + @@ -70,9 +76,11 @@ public sealed class SqlServerDialect : ISqlDialect /// control-character rule. /// The rejection messages deliberately do not echo the offending value — it is untrusted /// input and this exception's text reaches the driver log. - /// This is currently the only defence, not a backstop. Design §8.1 specifies that an - /// identifier is validated against the live catalog before reaching here; that gate is not implemented - /// yet, so an authored TagConfig table/column arrives unfiltered. + /// This is a backstop, not the only defence. Design §8.1's catalog gate + /// () resolves an authored table/column against the live catalog at + /// Initialize and substitutes the catalog's own spelling, so on the poll path the value arriving here + /// is a string read back out of the catalog. The rules below still run — the gate uses them as its own + /// charset filter, and they must hold for any future caller that has no catalog to check against. /// /// The bare, unquoted identifier part. /// The bracket-quoted identifier. diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.IntegrationTests/SqlServerReadTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.IntegrationTests/SqlServerReadTests.cs index c3fe2e99..ebf2d212 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.IntegrationTests/SqlServerReadTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.IntegrationTests/SqlServerReadTests.cs @@ -475,6 +475,99 @@ public sealed class SqlServerReadTests // ---- helpers ---- + // ---- design §8.1 catalog gate (Gitea #496), against a real INFORMATION_SCHEMA ---- + + /// + /// The gate's live proof: an authored column that does not exist is refused by the allow-list at + /// Initialize, so it never reaches a query — and its neighbour on the same table keeps reading. + /// + /// + /// Only a real SQL Server exercises the real SELECT SCHEMA_NAME() + INFORMATION_SCHEMA + /// path the loader depends on; the SQLite-backed suites prove the logic but not that T-SQL's catalog + /// answers the shape the loader expects. + /// + [Fact] + public async Task CatalogGate_realSqlServer_rejectsAnUnknownColumn_andSparesItsNeighbour() + { + SkipUnlessLive(); + + var bogus = Tag("Sql/Line1/Bogus", new JsonObject + { + ["driver"] = "Sql", + ["model"] = "KeyValue", + ["table"] = SqlPollServerFixture.Qualified(SqlPollServerFixture.KeyValueTable), + ["keyColumn"] = SqlPollServerFixture.KeyColumn, + ["keyValue"] = SqlPollServerFixture.PresentKey, + ["valueColumn"] = "no_such_column", + ["timestampColumn"] = SqlPollServerFixture.TimestampColumn, + }); + + await using var driver = await StartAsync( + KeyValue("Sql/Line1/Speed", SqlPollServerFixture.PresentKey), bogus); + + // An authoring typo is not a database fault: the driver stays Healthy. + driver.GetHealth().State.ShouldBe(DriverState.Healthy); + + var snapshots = await driver.ReadAsync( + ["Sql/Line1/Speed", "Sql/Line1/Bogus"], TestContext.Current.CancellationToken); + + SqlStatusCodes.IsGood(snapshots[0].StatusCode).ShouldBeTrue(); + snapshots[1].StatusCode.ShouldBe(SqlStatusCodes.BadNodeIdUnknown); + } + + /// + /// T-SQL object names are case-insensitive under the default collation, so a case-variant tag has + /// always been valid and must keep working — the gate substitutes the catalog's spelling rather than + /// rejecting the operator's. + /// + [Fact] + public async Task CatalogGate_realSqlServer_acceptsACaseVariantIdentifierAndStillReads() + { + SkipUnlessLive(); + + var upper = Tag("Sql/Line1/Speed", new JsonObject + { + ["driver"] = "Sql", + ["model"] = "KeyValue", + ["table"] = SqlPollServerFixture.Qualified(SqlPollServerFixture.KeyValueTable).ToUpperInvariant(), + ["keyColumn"] = SqlPollServerFixture.KeyColumn.ToUpperInvariant(), + ["keyValue"] = SqlPollServerFixture.PresentKey, + ["valueColumn"] = SqlPollServerFixture.ValueColumn.ToUpperInvariant(), + ["timestampColumn"] = SqlPollServerFixture.TimestampColumn.ToUpperInvariant(), + }); + + await using var driver = await StartAsync(upper); + + var snapshot = (await driver.ReadAsync(["Sql/Line1/Speed"], TestContext.Current.CancellationToken)) + .ShouldHaveSingleItem(); + SqlStatusCodes.IsGood(snapshot.StatusCode).ShouldBeTrue(); + } + + /// + /// A cross-database name cannot be validated from this connection's catalog, so it is refused rather + /// than quoted into a query — the documented v1 limitation, asserted so it stays deliberate. + /// + [Fact] + public async Task CatalogGate_realSqlServer_rejectsAThreePartName() + { + SkipUnlessLive(); + + var threePart = Tag("Sql/Line1/Remote", new JsonObject + { + ["driver"] = "Sql", + ["model"] = "KeyValue", + ["table"] = $"otherdb.{SqlPollServerFixture.Qualified(SqlPollServerFixture.KeyValueTable)}", + ["keyColumn"] = SqlPollServerFixture.KeyColumn, + ["keyValue"] = SqlPollServerFixture.PresentKey, + ["valueColumn"] = SqlPollServerFixture.ValueColumn, + }); + + await using var driver = await StartAsync(threePart); + + (await driver.ReadAsync(["Sql/Line1/Remote"], TestContext.Current.CancellationToken)) + .ShouldHaveSingleItem().StatusCode.ShouldBe(SqlStatusCodes.BadNodeIdUnknown); + } + /// Skips the calling test when the live-server gate is not configured or not reachable. private void SkipUnlessLive() { diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqlCatalogGateDriverTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqlCatalogGateDriverTests.cs new file mode 100644 index 00000000..9236f3e3 --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqlCatalogGateDriverTests.cs @@ -0,0 +1,354 @@ +using System.Globalization; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Logging.Abstractions; +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; +using ZB.MOM.WW.OtOpcUa.Driver.Sql.Contracts; + +namespace ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests; + +/// +/// The design §8.1 catalog gate end-to-end through , against the real SQLite +/// catalog the poll fixture creates — real ListSchemas/ListTables/ListColumns +/// round-trips, not a hand-built . +/// The two facts that matter most here are the ones a pure unit test cannot show: that a +/// rejected tag still has a node and reads BadNodeIdUnknown (rather than silently +/// vanishing from the address space), and that a catalog the driver cannot read faults Initialize instead +/// of rejecting every tag. +/// +public sealed class SqlCatalogGateDriverTests +{ + private const string DriverInstanceId = "sql-gate"; + private const string ConfigJson = """{"provider":"SqlServer"}"""; + + [Fact] + public async Task A_tag_naming_a_real_table_and_columns_polls_normally() + { + using var fixture = new SqlitePollFixture(); + await using var driver = NewDriver(fixture, KvEntry("Speed", SqlitePollFixture.PresentKey)); + + await driver.InitializeAsync(ConfigJson, CancellationToken.None); + + driver.GetHealth().State.ShouldBe(DriverState.Healthy); + var snapshot = (await driver.ReadAsync(["Speed"], CancellationToken.None)).ShouldHaveSingleItem(); + SqlStatusCodes.IsGood(snapshot.StatusCode).ShouldBeTrue(); + snapshot.Value.ShouldBe(SqlitePollFixture.PresentValue); + } + + /// + /// §8.1's specified outcome, in full: the tag is refused by the allow-list, so it never reaches a + /// query — but its node still exists and reads BadNodeIdUnknown. Dropping the node instead would + /// turn a diagnosable Bad quality into a missing address-space entry. + /// + [Fact] + public async Task A_tag_naming_an_unknown_column_keeps_its_node_and_reads_BadNodeIdUnknown() + { + using var fixture = new SqlitePollFixture(); + await using var driver = NewDriver( + fixture, + KvEntry("Speed", SqlitePollFixture.PresentKey), + KvEntry("Bogus", SqlitePollFixture.PresentKey, valueColumn: "no_such_column")); + + await driver.InitializeAsync(ConfigJson, CancellationToken.None); + + // The driver is healthy: an authoring typo is not a database fault. + driver.GetHealth().State.ShouldBe(DriverState.Healthy); + + // The node is still materialized... + var capture = new CapturingBuilder(); + await ((ITagDiscovery)driver).DiscoverAsync(capture, CancellationToken.None); + capture.Variables.Select(v => v.Info.FullName).ShouldBe(["Speed", "Bogus"], ignoreOrder: true); + + // ...and it is the rejected tag — and only it — that reads BadNodeIdUnknown. + var snapshots = await driver.ReadAsync(["Speed", "Bogus"], CancellationToken.None); + SqlStatusCodes.IsGood(snapshots[0].StatusCode).ShouldBeTrue(); + snapshots[1].StatusCode.ShouldBe(SqlStatusCodes.BadNodeIdUnknown); + } + + [Fact] + public async Task A_tag_naming_an_unknown_table_reads_BadNodeIdUnknown() + { + using var fixture = new SqlitePollFixture(); + await using var driver = NewDriver( + fixture, KvEntry("Bogus", SqlitePollFixture.PresentKey, table: "NoSuchTable")); + + await driver.InitializeAsync(ConfigJson, CancellationToken.None); + + (await driver.ReadAsync(["Bogus"], CancellationToken.None)) + .ShouldHaveSingleItem().StatusCode.ShouldBe(SqlStatusCodes.BadNodeIdUnknown); + } + + /// + /// The injection shape #496 exists to close. Before the gate, this name was bracket-quoted into a real + /// query that failed only once the connection was open; now it never reaches a query at all. + /// + [Fact] + public async Task A_hostile_table_name_never_reaches_a_query() + { + using var fixture = new SqlitePollFixture(); + var logger = new CapturingLogger(); + await using var driver = NewDriver( + fixture, logger, + KvEntry("Evil", SqlitePollFixture.PresentKey, table: "TagValues\"; DROP TABLE TagValues--")); + + await driver.InitializeAsync(ConfigJson, CancellationToken.None); + + (await driver.ReadAsync(["Evil"], CancellationToken.None)) + .ShouldHaveSingleItem().StatusCode.ShouldBe(SqlStatusCodes.BadNodeIdUnknown); + + // The fixture's table is untouched — proven by a second tag still reading through it. + await using var honest = NewDriver(fixture, KvEntry("Speed", SqlitePollFixture.PresentKey)); + await honest.InitializeAsync(ConfigJson, CancellationToken.None); + var snapshot = (await honest.ReadAsync(["Speed"], CancellationToken.None)).ShouldHaveSingleItem(); + SqlStatusCodes.IsGood(snapshot.StatusCode).ShouldBeTrue(); + + logger.Entries.ShouldContain(e => + e.Level == LogLevel.Warning && e.Message.Contains("rejected by the catalog gate", StringComparison.Ordinal)); + } + + /// + /// A node that goes Bad with nothing in the log is unsupportable, so every drop names the tag, the + /// field and the reason. + /// + [Fact] + public async Task Every_rejection_is_logged_with_the_tag_the_field_and_the_reason() + { + using var fixture = new SqlitePollFixture(); + var logger = new CapturingLogger(); + await using var driver = NewDriver( + fixture, logger, KvEntry("Bogus", SqlitePollFixture.PresentKey, valueColumn: "num_valeu")); + + await driver.InitializeAsync(ConfigJson, CancellationToken.None); + + var warning = logger.Entries + .Where(e => e.Level == LogLevel.Warning) + .Select(e => e.Message) + .ShouldHaveSingleItem(); + warning.ShouldContain("Bogus"); + warning.ShouldContain(nameof(SqlTagDefinition.ValueColumn)); + warning.ShouldContain("num_valeu"); + } + + /// + /// Case-insensitive authoring has always worked on SQL Server's default collation, so the gate must + /// accept it — and it substitutes the catalog's spelling, which is what makes the emitted SQL carry + /// catalog strings rather than operator input. + /// + [Fact] + public async Task A_case_variant_identifier_is_accepted_and_polls() + { + using var fixture = new SqlitePollFixture(); + await using var driver = NewDriver( + fixture, + KvEntry( + "Speed", SqlitePollFixture.PresentKey, + table: SqlitePollFixture.KeyValueTable.ToUpperInvariant(), + valueColumn: SqlitePollFixture.ValueColumn.ToUpperInvariant())); + + await driver.InitializeAsync(ConfigJson, CancellationToken.None); + + var snapshot = (await driver.ReadAsync(["Speed"], CancellationToken.None)).ShouldHaveSingleItem(); + SqlStatusCodes.IsGood(snapshot.StatusCode).ShouldBeTrue(); + snapshot.Value.ShouldBe(SqlitePollFixture.PresentValue); + } + + /// + /// A driver with nothing authored has nothing to validate; issuing catalog queries to prove that would + /// be a round-trip that can only fail. + /// + [Fact] + public async Task A_driver_with_no_authored_tags_initializes_without_touching_the_catalog() + { + using var fixture = new SqlitePollFixture(); + await using var driver = NewDriver(fixture); + + await driver.InitializeAsync(ConfigJson, CancellationToken.None); + + driver.GetHealth().State.ShouldBe(DriverState.Healthy); + } + + /// + /// The fail-closed rule. A catalog that cannot be read is the ABSENCE of evidence about the + /// tags, not evidence against them. Rejecting every tag would serve a confidently-empty address space + /// and send the operator hunting typos that do not exist; faulting Initialize instead lands + /// DriverInstanceActor in Reconnecting with its retry timer running. + /// + [Fact] + public async Task A_catalog_that_cannot_be_read_faults_Initialize_rather_than_rejecting_every_tag() + { + using var fixture = new SqlitePollFixture(); + await using var driver = new SqlDriver( + new SqlDriverOptions + { + RawTags = [KvEntry("Speed", SqlitePollFixture.PresentKey)], + OperationTimeout = TimeSpan.FromSeconds(15), + CommandTimeout = TimeSpan.FromSeconds(10), + }, + DriverInstanceId, + new CatalogSqlOverride("SELECT this is not valid sql"), + fixture.ConnectionString, + factory: fixture.Factory, + logger: NullLogger.Instance); + + var thrown = await Should.ThrowAsync( + async () => await driver.InitializeAsync(ConfigJson, CancellationToken.None)); + + // The operator surface names the stage that actually failed — "reached it but could not read the + // catalog" and "could not reach it" send an operator to different systems. + thrown.Message.ShouldContain("catalog"); + driver.GetHealth().State.ShouldBe(DriverState.Faulted); + } + + /// + /// Zero visible schemas is a grant problem, not an empty database, so it must fault rather than reject + /// every tag for an authoring fault the operator does not have. + /// + [Fact] + public async Task A_catalog_reporting_no_schemas_at_all_faults_Initialize() + { + using var fixture = new SqlitePollFixture(); + await using var driver = new SqlDriver( + new SqlDriverOptions + { + RawTags = [KvEntry("Speed", SqlitePollFixture.PresentKey)], + OperationTimeout = TimeSpan.FromSeconds(15), + CommandTimeout = TimeSpan.FromSeconds(10), + }, + DriverInstanceId, + new CatalogSqlOverride("SELECT 'x' AS TABLE_SCHEMA WHERE 1 = 0"), + fixture.ConnectionString, + factory: fixture.Factory, + logger: NullLogger.Instance); + + await Should.ThrowAsync( + async () => await driver.InitializeAsync(ConfigJson, CancellationToken.None)); + + driver.GetHealth().State.ShouldBe(DriverState.Faulted); + } + + // ---- helpers ---- + + /// + /// Delegates every member to except , so the + /// catalog-load failure modes can be driven against an otherwise-real dialect. + /// + /// + /// A decorator rather than a subclass: is sealed, and even if it were not, + /// new-hiding a property would leave interface dispatch calling the base — the substitution + /// would silently not happen and both tests below would pass for the wrong reason. + /// + private sealed class CatalogSqlOverride(string listSchemasSql) : ISqlDialect + { + private static readonly SqliteDialect Inner = new(); + + public SqlProvider Provider => Inner.Provider; + + public System.Data.Common.DbProviderFactory Factory => Inner.Factory; + + public string LivenessSql => Inner.LivenessSql; + + public string SingleRowLimitPrefix => Inner.SingleRowLimitPrefix; + + public string SingleRowLimitSuffix => Inner.SingleRowLimitSuffix; + + public string ListSchemasSql { get; } = listSchemasSql; + + public string DefaultSchemaSql => Inner.DefaultSchemaSql; + + public string ListTablesSql => Inner.ListTablesSql; + + public string ListColumnsSql => Inner.ListColumnsSql; + + public string QuoteIdentifier(string ident) => Inner.QuoteIdentifier(ident); + + public DriverDataType MapColumnType(string sqlDataType) => Inner.MapColumnType(sqlDataType); + } + + private static SqlDriver NewDriver(SqlitePollFixture fixture, params RawTagEntry[] rawTags) + => NewDriver(fixture, new CapturingLogger(), rawTags); + + private static SqlDriver NewDriver( + SqlitePollFixture fixture, CapturingLogger logger, params RawTagEntry[] rawTags) + => new( + new SqlDriverOptions + { + RawTags = rawTags, + OperationTimeout = TimeSpan.FromSeconds(15), + CommandTimeout = TimeSpan.FromSeconds(10), + }, + DriverInstanceId, + new SqliteDialect(), + fixture.ConnectionString, + factory: fixture.Factory, + logger: logger); + + /// One authored raw tag, with the table and value column overridable so the gate can be exercised. + private static RawTagEntry KvEntry( + string rawPath, + string keyValue, + string? table = null, + string? valueColumn = null) + => new(rawPath, string.Create(CultureInfo.InvariantCulture, $$""" + { + "driver": "Sql", + "model": "KeyValue", + "table": "{{(table ?? SqlitePollFixture.KeyValueTable).Replace("\"", "\\\"", StringComparison.Ordinal)}}", + "keyColumn": "{{SqlitePollFixture.KeyColumn}}", + "keyValue": "{{keyValue}}", + "valueColumn": "{{valueColumn ?? SqlitePollFixture.ValueColumn}}", + "timestampColumn": "{{SqlitePollFixture.TimestampColumn}}" + } + """), WriteIdempotent: false); + + /// Records everything the driver streams into the address space. + private sealed class CapturingBuilder : IAddressSpaceBuilder + { + /// The variables registered, in order. + public List<(string BrowseName, DriverAttributeInfo Info)> Variables { get; } = []; + + public IAddressSpaceBuilder Folder(string browseName, string displayName) => this; + + public IVariableHandle Variable(string browseName, string displayName, DriverAttributeInfo attributeInfo) + { + Variables.Add((browseName, attributeInfo)); + return new Handle(attributeInfo.FullName); + } + + public void AddProperty(string browseName, DriverDataType dataType, object? value) { } + + private sealed class Handle(string fullReference) : IVariableHandle + { + public string FullReference => fullReference; + + public IAlarmConditionSink MarkAsAlarmCondition(AlarmConditionInfo info) => new Sink(); + + private sealed class Sink : IAlarmConditionSink + { + public void OnTransition(AlarmEventArgs args) { } + } + } + } + + /// Records every log record, level + rendered message. + private sealed class CapturingLogger : ILogger + { + public List<(LogLevel Level, string Message)> Entries { get; } = []; + + public IDisposable BeginScope(TState state) where TState : notnull => NullScope.Instance; + + public bool IsEnabled(LogLevel logLevel) => true; + + public void Log( + LogLevel logLevel, EventId eventId, TState state, Exception? exception, + Func formatter) + => Entries.Add((logLevel, formatter(state, exception))); + + private sealed class NullScope : IDisposable + { + public static NullScope Instance { get; } = new(); + + public void Dispose() { } + } + } +} diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqlCatalogGateTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqlCatalogGateTests.cs new file mode 100644 index 00000000..51d9acbf --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqlCatalogGateTests.cs @@ -0,0 +1,316 @@ +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Driver.Sql.Contracts; + +namespace ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests; + +/// +/// The design §8.1 catalog gate (Gitea #496), tested as a pure function over a hand-built +/// . The end-to-end behaviour against a real catalog — and the proof that a +/// rejected tag keeps its node and publishes BadNodeIdUnknown — lives in +/// . +/// +public sealed class SqlCatalogGateTests +{ + private static readonly SqliteDialect Dialect = new(); + + /// A catalog with one schema, two relations, and deliberately mixed-case column spellings. + private static SqlCatalog Catalog(string defaultSchema = "dbo") => new( + defaultSchema, + ["dbo", "mes"], + new Dictionary>(StringComparer.Ordinal) + { + ["dbo"] = ["TagValues", "LatestStatus"], + ["mes"] = ["Orders"], + }, + new Dictionary>(StringComparer.Ordinal) + { + ["dbo.TagValues"] = ["tag_name", "num_value", "sample_ts"], + ["dbo.LatestStatus"] = ["station_id", "oven_temp", "pressure", "sample_ts"], + ["mes.Orders"] = ["order_id", "qty"], + }); + + private static SqlTagDefinition KeyValueTag( + string table = "dbo.TagValues", + string keyColumn = "tag_name", + string valueColumn = "num_value", + string? timestampColumn = "sample_ts") => + new("plant/sql/Speed", SqlTagModel.KeyValue, table, + KeyColumn: keyColumn, KeyValue: "Line1.Speed", + ValueColumn: valueColumn, TimestampColumn: timestampColumn); + + private static SqlCatalogGateResult Apply(params SqlTagDefinition[] tags) => + SqlCatalogGate.Apply(tags, Catalog(), Dialect); + + [Fact] + public void A_fully_resolvable_tag_is_accepted() + { + var result = Apply(KeyValueTag()); + + result.Rejected.ShouldBeEmpty(); + result.Accepted.Count.ShouldBe(1); + } + + /// + /// The heart of §8.1: what reaches the planner must be a string the catalog gave us, not the string an + /// operator typed. Authoring every identifier in the wrong case proves the substitution actually + /// happens rather than the input merely being waved through. + /// + [Fact] + public void Accepted_identifiers_are_rewritten_to_the_catalogs_own_spelling() + { + var authored = KeyValueTag( + table: "DBO.TAGVALUES", keyColumn: "TAG_NAME", valueColumn: "Num_Value", timestampColumn: "SAMPLE_TS"); + + var accepted = Apply(authored).Accepted.ShouldHaveSingleItem(); + + accepted.Table.ShouldBe("dbo.TagValues"); + accepted.KeyColumn.ShouldBe("tag_name"); + accepted.ValueColumn.ShouldBe("num_value"); + accepted.TimestampColumn.ShouldBe("sample_ts"); + // Identity and bound VALUES are untouched — the gate rewrites identifiers only. + accepted.Name.ShouldBe(authored.Name); + accepted.KeyValue.ShouldBe(authored.KeyValue); + } + + [Fact] + public void An_unqualified_table_resolves_in_the_default_schema() + { + var accepted = Apply(KeyValueTag(table: "TagValues")).Accepted.ShouldHaveSingleItem(); + + accepted.Table.ShouldBe("dbo.TagValues"); + } + + /// + /// Guessing dbo would be a silent lie on an estate that maps service accounts to their own + /// default schema, so the gate must resolve an unqualified name in whatever schema the server reports. + /// + [Fact] + public void An_unqualified_table_follows_a_non_dbo_default_schema() + { + var catalog = Catalog(defaultSchema: "mes"); + + var result = SqlCatalogGate.Apply([KeyValueTag(table: "Orders", keyColumn: "order_id", valueColumn: "qty", timestampColumn: null)], catalog, Dialect); + + result.Accepted.ShouldHaveSingleItem().Table.ShouldBe("mes.Orders"); + } + + [Fact] + public void An_unknown_table_rejects_the_tag() + { + var rejection = Apply(KeyValueTag(table: "dbo.NoSuchTable")).Rejected.ShouldHaveSingleItem(); + + rejection.RawPath.ShouldBe("plant/sql/Speed"); + rejection.Field.ShouldBe(nameof(SqlTagDefinition.Table)); + rejection.Reason.ShouldContain("NoSuchTable"); + } + + [Fact] + public void An_unknown_schema_rejects_the_tag() + { + var rejection = Apply(KeyValueTag(table: "nope.TagValues")).Rejected.ShouldHaveSingleItem(); + + rejection.Field.ShouldBe(nameof(SqlTagDefinition.Table)); + rejection.Reason.ShouldContain("nope"); + } + + [Fact] + public void An_unknown_column_rejects_the_tag_and_names_the_field() + { + var rejection = Apply(KeyValueTag(valueColumn: "no_such_column")).Rejected.ShouldHaveSingleItem(); + + rejection.Field.ShouldBe(nameof(SqlTagDefinition.ValueColumn)); + rejection.Reason.ShouldContain("no_such_column"); + rejection.Reason.ShouldContain("dbo.TagValues"); + } + + /// + /// A table in the right catalog but the wrong schema must not resolve — otherwise the gate would + /// allow-list a column set from a relation the query will never read. + /// + [Fact] + public void A_table_from_another_schema_does_not_resolve_unqualified() + { + var rejection = Apply(KeyValueTag(table: "Orders", keyColumn: "order_id", valueColumn: "qty", timestampColumn: null)) + .Rejected.ShouldHaveSingleItem(); + + rejection.Field.ShouldBe(nameof(SqlTagDefinition.Table)); + } + + /// + /// A cross-database or linked-server name addresses a catalog this connection cannot enumerate, so it + /// cannot be allow-listed. Rejecting is the fail-closed answer, and the message says what to do instead. + /// + [Fact] + public void A_three_part_name_is_rejected_with_an_actionable_message() + { + var rejection = Apply(KeyValueTag(table: "otherdb.dbo.TagValues")).Rejected.ShouldHaveSingleItem(); + + rejection.Field.ShouldBe(nameof(SqlTagDefinition.Table)); + rejection.Reason.ShouldContain("3-part"); + rejection.Reason.ShouldContain("view"); + } + + /// + /// The injection shape the whole gate exists for: a hostile identifier must be refused by the + /// allow-list, not merely quoted into a query against a nonexistent object. + /// + [Theory] + [InlineData("TagValues]; DROP TABLE TagValues--")] + [InlineData("'; DROP TABLE TagValues--")] + [InlineData("TagValues WHERE 1=1 OR 1=1")] + public void A_hostile_table_name_is_rejected_by_the_allow_list(string table) + { + Apply(KeyValueTag(table: table)).Rejected.ShouldHaveSingleItem() + .Field.ShouldBe(nameof(SqlTagDefinition.Table)); + } + + [Theory] + [InlineData("num_value]; DROP TABLE TagValues--")] + [InlineData("(SELECT password FROM users)")] + public void A_hostile_column_name_is_rejected_by_the_allow_list(string column) + { + Apply(KeyValueTag(valueColumn: column)).Rejected.ShouldHaveSingleItem() + .Field.ShouldBe(nameof(SqlTagDefinition.ValueColumn)); + } + + /// + /// A name carrying a control or Unicode format character cannot be safely rendered into a log line + /// (the Trojan-Source class, CVE-2021-42574), so the gate rejects it without echoing it — the + /// charset check runs before the catalog lookup precisely so no such string can reach a message. + /// + /// + /// Driven against , not the test-only : the + /// charset rules belong to the dialect (the gate delegates to + /// rather than duplicating them), and SQLite's rules + /// deliberately stop at control characters. Asserting SQL Server's rules means using SQL Server's + /// dialect — the first draft of this test asserted them against SQLite's and failed for that reason. + /// + [Theory] + [InlineData("num\u0000value")] // Cc — embedded NUL + [InlineData("num\u0009value")] // Cc — tab; truncates or corrupts a logged statement + [InlineData("num\u202Evalue")] // Cf — right-to-left override + [InlineData("num\u200Bvalue")] // Cf — zero-width space + public void An_unrenderable_identifier_is_rejected_without_being_echoed(string column) + { + var result = SqlCatalogGate.Apply( + [KeyValueTag(valueColumn: column)], Catalog(), new SqlServerDialect()); + + var rejection = result.Rejected.ShouldHaveSingleItem(); + rejection.Field.ShouldBe(nameof(SqlTagDefinition.ValueColumn)); + rejection.Reason.ShouldContain("withheld"); + rejection.Reason.ShouldNotContain(column); + } + + /// + /// An over-long name cannot name a real object and is likewise withheld, rather than pasting an + /// unbounded slab of operator input into a log line. + /// + [Fact] + public void An_over_long_identifier_is_rejected_without_being_echoed() + { + var column = new string('x', SqlServerDialect.MaxIdentifierLength + 1); + + var result = SqlCatalogGate.Apply( + [KeyValueTag(valueColumn: column)], Catalog(), new SqlServerDialect()); + + result.Rejected.ShouldHaveSingleItem().Reason.ShouldContain("withheld"); + } + + /// + /// The complement, and the reason the charset check is not simply "withhold everything": a name that + /// IS safely renderable gets echoed, because an operator hunting a typo has to see what they wrote. + /// + [Fact] + public void A_renderable_but_unknown_identifier_IS_echoed_so_the_typo_is_findable() + { + var rejection = Apply(KeyValueTag(valueColumn: "num_valeu")).Rejected.ShouldHaveSingleItem(); + + rejection.Reason.ShouldContain("num_valeu"); + rejection.Reason.ShouldNotContain("withheld"); + } + + /// + /// On a case-sensitive collation a relation may legitimately carry both Value and value. + /// Picking one would publish a different column's data under the operator's node, so the only safe + /// answer is to refuse — but an EXACT match must still win, or a valid config would break. + /// + [Fact] + public void An_ambiguous_case_insensitive_column_match_is_rejected_but_an_exact_match_still_wins() + { + var catalog = new SqlCatalog( + "dbo", + ["dbo"], + new Dictionary>(StringComparer.Ordinal) { ["dbo"] = ["T"] }, + new Dictionary>(StringComparer.Ordinal) + { + ["dbo.T"] = ["k", "Value", "value"], + }); + + var ambiguous = new SqlTagDefinition( + "p/Ambiguous", SqlTagModel.KeyValue, "dbo.T", + KeyColumn: "k", KeyValue: "x", ValueColumn: "VALUE"); + SqlCatalogGate.Apply([ambiguous], catalog, Dialect).Rejected.ShouldHaveSingleItem() + .Field.ShouldBe(nameof(SqlTagDefinition.ValueColumn)); + + var exact = ambiguous with { Name = "p/Exact", ValueColumn = "value" }; + SqlCatalogGate.Apply([exact], catalog, Dialect).Accepted.ShouldHaveSingleItem() + .ValueColumn.ShouldBe("value"); + } + + /// + /// One operator typo must not stop the other tags on the same database — the same rule the tag-table + /// build already follows for a malformed blob. + /// + [Fact] + public void A_rejected_tag_does_not_take_its_healthy_neighbours_with_it() + { + var good = KeyValueTag() with { Name = "plant/sql/Good" }; + var bad = KeyValueTag(valueColumn: "typo") with { Name = "plant/sql/Bad" }; + + var result = Apply(good, bad); + + result.Accepted.ShouldHaveSingleItem().Name.ShouldBe("plant/sql/Good"); + result.Rejected.ShouldHaveSingleItem().RawPath.ShouldBe("plant/sql/Bad"); + } + + /// Every identifier-bearing field is checked, not just the two the key-value model happens to use. + [Fact] + public void The_wide_row_models_identifier_fields_are_validated_too() + { + var selectorTypo = new SqlTagDefinition( + "p/Oven", SqlTagModel.WideRow, "dbo.LatestStatus", + ColumnName: "oven_temp", RowSelectorColumn: "no_such_selector", RowSelectorValue: "7"); + Apply(selectorTypo).Rejected.ShouldHaveSingleItem() + .Field.ShouldBe(nameof(SqlTagDefinition.RowSelectorColumn)); + + var orderTypo = new SqlTagDefinition( + "p/Newest", SqlTagModel.WideRow, "dbo.LatestStatus", + ColumnName: "oven_temp", RowSelectorTopByTimestamp: "no_such_ts"); + Apply(orderTypo).Rejected.ShouldHaveSingleItem() + .Field.ShouldBe(nameof(SqlTagDefinition.RowSelectorTopByTimestamp)); + + var columnTypo = new SqlTagDefinition( + "p/Bad", SqlTagModel.WideRow, "dbo.LatestStatus", + ColumnName: "no_such_column", RowSelectorColumn: "station_id", RowSelectorValue: "7"); + Apply(columnTypo).Rejected.ShouldHaveSingleItem() + .Field.ShouldBe(nameof(SqlTagDefinition.ColumnName)); + + var ok = new SqlTagDefinition( + "p/Ok", SqlTagModel.WideRow, "dbo.LatestStatus", + ColumnName: "OVEN_TEMP", RowSelectorColumn: "STATION_ID", RowSelectorValue: "7"); + var accepted = Apply(ok).Accepted.ShouldHaveSingleItem(); + accepted.ColumnName.ShouldBe("oven_temp"); + accepted.RowSelectorColumn.ShouldBe("station_id"); + accepted.RowSelectorValue.ShouldBe("7"); // a bound value, never canonicalized + } + + /// An absent optional identifier is not a rejection — only a present, unresolvable one is. + [Fact] + public void An_absent_optional_timestamp_column_is_left_alone() + { + var accepted = Apply(KeyValueTag(timestampColumn: null)).Accepted.ShouldHaveSingleItem(); + + accepted.TimestampColumn.ShouldBeNull(); + } +} diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqlInjectionRegressionTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqlInjectionRegressionTests.cs index 784b3dd7..a49be22d 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqlInjectionRegressionTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqlInjectionRegressionTests.cs @@ -12,16 +12,19 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests; /// identifier — which cannot be parameterized in SQL — is dialect-quoted so a hostile one becomes /// an inert reference to a nonexistent object rather than executable text. If these pass, a malicious tag /// cannot alter the database; the seed table is intact after every hostile poll. -/// Scope note — what this suite does NOT assume. Design §8.1 also specifies a catalog gate: -/// validate every authored table/column against INFORMATION_SCHEMA before quoting it, so an unknown -/// identifier rejects the tag. No such gate exists in the driver yet (see the "Not yet -/// implemented" note on ), and no task in this workstream builds one. So a hostile -/// identifier here is not rejected as BadNodeIdUnknown — it is bracket-/double-quoted into a -/// single nonexistent identifier, the query then fails after the connection opened, and the tag Bad-codes -/// as a query failure (). This suite proves the payload -/// is inert — it does not execute and the table survives — which is the guarantee the code -/// actually makes. The catalog gate is a separate follow-up; a test that pretended it existed would be -/// asserting fiction. +/// Scope note — this suite deliberately tests the layer BELOW the catalog gate. Design +/// §8.1's catalog gate now exists (Gitea #496): validates every authored +/// table/column against the live catalog at Initialize, so in the assembled driver a hostile identifier +/// is rejected up front and its tag reads — proven by +/// SqlCatalogGateDriverTests. +/// The tests here construct a directly, with hand-built +/// definitions that never pass through the gate, and that is the point: defence in depth is only worth +/// the name if each layer holds on its own. These assertions pin the quoting backstop — that +/// even with the allow-list bypassed entirely, a hostile identifier becomes an inert reference to a +/// nonexistent object, the query fails after the connection opened, the tag Bad-codes as a query failure +/// (), and the seed table survives. Rewriting them to +/// expect BadNodeIdUnknown would delete the only coverage the backstop has and leave the gate as a +/// single point of failure. /// public sealed class SqlInjectionRegressionTests { @@ -68,9 +71,11 @@ public sealed class SqlInjectionRegressionTests using var fixture = new SqlitePollFixture(); var seedRows = await RowCountAsync(fixture, SqlitePollFixture.KeyValueTable); - // A table name carrying a statement terminator + DROP. There is NO catalog gate, so this is not - // rejected up front — it is quoted into one nonexistent identifier. The query fails (no such table), - // the connection having opened, so the tag Bad-codes as a query failure. The DROP never runs. + // A table name carrying a statement terminator + DROP. The reader is driven directly, so the + // catalog gate never sees it and the quoting backstop is on its own: the name is quoted into one + // nonexistent identifier, the query fails (no such table) with the connection already open, so the + // tag Bad-codes as a query failure. The DROP never runs. In the assembled driver the gate rejects + // this first and the tag reads BadNodeIdUnknown instead — see SqlCatalogGateDriverTests. var reader = NewReader(fixture, new SqlTagDefinition( Name: "Evil", @@ -83,7 +88,7 @@ public sealed class SqlInjectionRegressionTests var snapshots = await reader.ReadAsync(["Evil"], CancellationToken.None); - // Bad-coded — but as a QUERY failure, not BadNodeIdUnknown (there is no identifier-catalog gate). + // Bad-coded as a QUERY failure, not BadNodeIdUnknown — this reader was built without the gate. SqlStatusCodes.IsBad(snapshots[0].StatusCode).ShouldBeTrue(); snapshots[0].StatusCode.ShouldBe(SqlStatusCodes.BadCommunicationError); // The load-bearing assertion: the payload did NOT execute. The table and its rows are untouched. diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqliteDialect.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqliteDialect.cs index 8a7a7527..a3ec1ca6 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqliteDialect.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqliteDialect.cs @@ -64,6 +64,12 @@ public sealed class SqliteDialect : ISqlDialect /// public string ListSchemasSql => $"SELECT '{MainSchema}' AS TABLE_SCHEMA"; + /// + /// SQLite has no per-principal default schema, so an unqualified name always resolves in + /// — the same single schema reports. + /// + public string DefaultSchemaSql => $"SELECT '{MainSchema}'"; + /// /// Tables + views from sqlite_schema (the modern name for sqlite_master), with the /// internal sqlite_* objects filtered out and the type folded onto the