fix(inbound): authorize+secure Database helper, async/deadline-bound DB, wait-timeout-bound WaitForAttribute
Resolves InboundAPI-026/027/028/029 (+ newly-surfaced -030). - 026: authorize the scoped Database helper in the design doc; SQL-injection protection is parameter binding (values never concatenated); allow writes via ExecuteAsync; drop the false 'read-only' claim. Named connections only. - 027: async ADO.NET end-to-end (no .GetAwaiter().GetResult()); honour the method deadline token on ExecuteScalarAsync/ExecuteReaderAsync/ExecuteNonQueryAsync + a CommandTimeout backstop derived from the method timeout. - 028: negative-path tests (null-gateway, deadline cancellation, parameterization) + e2e Database + WaitForAttribute cases through the real endpoint. - 029: WaitForAttribute is bounded by its WAIT timeout (per-wait CTS + client-abort + explicit token), NOT the method deadline (spec §6) — a long wait may outlive the method timeout; WithRequestAborted threads the raw client-abort token separately. - 030: Central UI compile-surface mirrors (InboundScriptHost / SandboxInboundScriptHost) gained the Database member (drifted since the runtime helper was added) so the authorized async API type-checks at the design-time gate.
This commit is contained in:
@@ -16,8 +16,12 @@ using ZB.MOM.WW.Auth.ApiKeys.Sqlite;
|
||||
using ZB.MOM.WW.ScadaBridge.Commons.Entities.InboundApi;
|
||||
using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Repositories;
|
||||
using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Services;
|
||||
using ZB.MOM.WW.ScadaBridge.Commons.Messages.InboundApi;
|
||||
using ZB.MOM.WW.ScadaBridge.Commons.Observability;
|
||||
using ZB.MOM.WW.ScadaBridge.Commons.Types;
|
||||
using ZB.MOM.WW.ScadaBridge.InboundAPI.Middleware;
|
||||
using Microsoft.Data.Sqlite;
|
||||
using System.Data.Common;
|
||||
using System.Diagnostics.Metrics;
|
||||
using System.Net;
|
||||
using System.Net.Http.Headers;
|
||||
@@ -548,6 +552,102 @@ public sealed class EndpointExtensionsTests : IDisposable
|
||||
public void Dispose() => _listener.Dispose();
|
||||
}
|
||||
|
||||
// --- InboundAPI-028: drive the new script-facing capabilities (Database +
|
||||
// WaitForAttribute) end-to-end through the real POST /api/{methodName} flow,
|
||||
// so a wiring regression in executor→context→helper / RouteHelper is caught. ---
|
||||
|
||||
[Fact]
|
||||
public async Task Script_UsingDatabase_RunsEndToEndThroughEndpoint()
|
||||
{
|
||||
// A method whose script reads through Database.QueryAsync — proving the executor
|
||||
// builds the Database helper, resolves IDatabaseGateway from the per-execution
|
||||
// DI scope, and the parameterized read reaches the body. (InboundAPI-028)
|
||||
var method = SeedMethod(1, "movein",
|
||||
"return await Database.QuerySingleAsync<string>(\"BTDB\", \"SELECT Code FROM Machine WHERE SAPID=@s\", new { s = (string)Parameters[\"sap\"] });",
|
||||
"""[{"name":"sap","type":"String","required":true}]""");
|
||||
|
||||
using var host = await BuildHostAsync(method, additionalServices: services =>
|
||||
{
|
||||
services.AddSingleton<IDatabaseGateway>(SeededSqliteGateway());
|
||||
});
|
||||
var token = await SeedKeyAsync(host, keyId: "key1", displayName: "movein-caller",
|
||||
scopes: new[] { "movein" });
|
||||
var client = host.GetTestClient();
|
||||
|
||||
var response = await client.SendAsync(BuildPost("movein", """{"sap":"131453"}""", token));
|
||||
var body = await response.Content.ReadAsStringAsync();
|
||||
|
||||
Assert.Equal(HttpStatusCode.OK, response.StatusCode);
|
||||
Assert.Contains("Z28061A", body);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task Script_UsingWaitForAttribute_RunsEndToEndThroughEndpoint()
|
||||
{
|
||||
// A method whose script awaits Route.To(...).WaitForAttribute(...) — proving the
|
||||
// executor binds the RouteHelper and the wait routes through IInstanceRouter and
|
||||
// returns its result through the endpoint. (InboundAPI-028)
|
||||
var method = SeedMethod(2, "waitmethod",
|
||||
"return await Route.To(\"inst-1\").WaitForAttribute(\"Flag\", true, System.TimeSpan.FromSeconds(1));");
|
||||
|
||||
var locator = Substitute.For<IInstanceLocator>();
|
||||
locator.GetSiteIdForInstanceAsync("inst-1", Arg.Any<CancellationToken>()).Returns("SiteA");
|
||||
var router = Substitute.For<IInstanceRouter>();
|
||||
router.RouteToWaitForAttributeAsync("SiteA", Arg.Any<RouteToWaitForAttributeRequest>(), Arg.Any<CancellationToken>())
|
||||
.Returns(ci => new RouteToWaitForAttributeResponse(
|
||||
((RouteToWaitForAttributeRequest)ci[1]).CorrelationId,
|
||||
Matched: true, Value: true, Quality: "Good", TimedOut: false,
|
||||
Success: true, ErrorMessage: null, DateTimeOffset.UtcNow));
|
||||
|
||||
using var host = await BuildHostAsync(method, additionalServices: services =>
|
||||
{
|
||||
services.RemoveAll<IInstanceLocator>();
|
||||
services.AddSingleton(locator);
|
||||
services.RemoveAll<IInstanceRouter>();
|
||||
services.AddSingleton(router);
|
||||
});
|
||||
var token = await SeedKeyAsync(host, keyId: "key1", displayName: "wait-caller",
|
||||
scopes: new[] { "waitmethod" });
|
||||
var client = host.GetTestClient();
|
||||
|
||||
var response = await client.SendAsync(BuildPost("waitmethod", "{}", token));
|
||||
var body = await response.Content.ReadAsStringAsync();
|
||||
|
||||
Assert.Equal(HttpStatusCode.OK, response.StatusCode);
|
||||
Assert.Contains("true", body);
|
||||
}
|
||||
|
||||
// SQLite-backed IDatabaseGateway fake. Owns its keep-alive connection so the shared
|
||||
// in-memory db lives for the gateway's (= host's) lifetime — NOT a GC-eligible local,
|
||||
// which could be collected mid-suite (under GC pressure + SqliteConnection.ClearAllPools
|
||||
// in Dispose) and drop the db before the request opens its own connection. A unique db
|
||||
// name per instance also keeps tests from contaminating each other. Seeded with
|
||||
// Machine(Code,SAPID)=('Z28061A','131453').
|
||||
private sealed class SqliteGateway : IDatabaseGateway, IDisposable
|
||||
{
|
||||
private readonly string _cs;
|
||||
private readonly SqliteConnection _keepAlive;
|
||||
public SqliteGateway(string cs)
|
||||
{
|
||||
_cs = cs;
|
||||
_keepAlive = new SqliteConnection(cs);
|
||||
_keepAlive.Open();
|
||||
using var cmd = _keepAlive.CreateCommand();
|
||||
cmd.CommandText = "CREATE TABLE IF NOT EXISTS Machine(Code TEXT, SAPID TEXT); DELETE FROM Machine; INSERT INTO Machine VALUES('Z28061A','131453');";
|
||||
cmd.ExecuteNonQuery();
|
||||
}
|
||||
public async Task<DbConnection> GetConnectionAsync(string name, CancellationToken ct = default)
|
||||
{ var c = new SqliteConnection(_cs); await c.OpenAsync(ct); return c; }
|
||||
public Task<ExternalCallResult> CachedWriteAsync(string c, string s,
|
||||
IReadOnlyDictionary<string, object?>? p = null, string? o = null, CancellationToken ct = default,
|
||||
TrackedOperationId? t = null, Guid? e = null, string? src = null, Guid? pe = null)
|
||||
=> throw new NotImplementedException();
|
||||
public void Dispose() => _keepAlive.Dispose();
|
||||
}
|
||||
|
||||
private static SqliteGateway SeededSqliteGateway() =>
|
||||
new($"DataSource=file:movein-endpoint-{Guid.NewGuid():N}?mode=memory&cache=shared");
|
||||
|
||||
private static HttpRequestMessage BuildPost(string methodName, string body, string bearerToken)
|
||||
{
|
||||
var request = new HttpRequestMessage(HttpMethod.Post, "/api/" + methodName)
|
||||
|
||||
@@ -12,46 +12,51 @@ public class InboundDatabaseHelperTests
|
||||
private sealed class SqliteGateway : IDatabaseGateway
|
||||
{
|
||||
private readonly string _cs;
|
||||
public SqliteGateway(string cs) => _cs = cs;
|
||||
// InboundAPI-027 test seam: when false the open ignores the passed token, so a
|
||||
// pre-cancelled token surfaces at Execute/Read (not at open) — letting a test
|
||||
// prove the EXECUTE path honours the deadline token, not just the open.
|
||||
private readonly bool _honorTokenOnOpen;
|
||||
public SqliteGateway(string cs, bool honorTokenOnOpen = true)
|
||||
{ _cs = cs; _honorTokenOnOpen = honorTokenOnOpen; }
|
||||
public async Task<DbConnection> GetConnectionAsync(string name, CancellationToken ct = default)
|
||||
{ var c = new SqliteConnection(_cs); await c.OpenAsync(ct); return c; }
|
||||
{ var c = new SqliteConnection(_cs); await c.OpenAsync(_honorTokenOnOpen ? ct : CancellationToken.None); return c; }
|
||||
public Task<ExternalCallResult> CachedWriteAsync(string c, string s,
|
||||
IReadOnlyDictionary<string, object?>? p = null, string? o = null, CancellationToken ct = default,
|
||||
TrackedOperationId? t = null, Guid? e = null, string? src = null, Guid? pe = null)
|
||||
=> throw new NotImplementedException();
|
||||
}
|
||||
|
||||
private static SqliteGateway SeededGateway()
|
||||
private static SqliteGateway SeededGateway(bool honorTokenOnOpen = true)
|
||||
{
|
||||
var keep = new SqliteConnection("DataSource=file:movein?mode=memory&cache=shared");
|
||||
keep.Open(); // keep-alive: shared in-memory db lives until process exit
|
||||
using var cmd = keep.CreateCommand();
|
||||
cmd.CommandText = "CREATE TABLE IF NOT EXISTS Machine(Code TEXT, SAPID TEXT); DELETE FROM Machine; INSERT INTO Machine VALUES('Z28061A','131453');";
|
||||
cmd.ExecuteNonQuery();
|
||||
return new SqliteGateway("DataSource=file:movein?mode=memory&cache=shared");
|
||||
return new SqliteGateway("DataSource=file:movein?mode=memory&cache=shared", honorTokenOnOpen);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void QuerySingle_returns_first_column_with_bound_parameter()
|
||||
public async Task QuerySingleAsync_returns_first_column_with_bound_parameter()
|
||||
{
|
||||
var helper = new InboundDatabaseHelper(SeededGateway(), CancellationToken.None);
|
||||
var code = helper.QuerySingle<string>("BTDB", "SELECT Code FROM Machine WHERE SAPID=@s", new { s = "131453" });
|
||||
var code = await helper.QuerySingleAsync<string>("BTDB", "SELECT Code FROM Machine WHERE SAPID=@s", new { s = "131453" });
|
||||
Assert.Equal("Z28061A", code);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void QuerySingle_returns_default_when_no_rows()
|
||||
public async Task QuerySingleAsync_returns_default_when_no_rows()
|
||||
{
|
||||
var helper = new InboundDatabaseHelper(SeededGateway(), CancellationToken.None);
|
||||
var code = helper.QuerySingle<string>("BTDB", "SELECT Code FROM Machine WHERE SAPID=@s", new { s = "999999" });
|
||||
var code = await helper.QuerySingleAsync<string>("BTDB", "SELECT Code FROM Machine WHERE SAPID=@s", new { s = "999999" });
|
||||
Assert.Null(code);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void Query_returns_row_with_case_insensitive_keys()
|
||||
public async Task QueryAsync_returns_row_with_case_insensitive_keys()
|
||||
{
|
||||
var helper = new InboundDatabaseHelper(SeededGateway(), CancellationToken.None);
|
||||
var rows = helper.Query("BTDB", "SELECT Code, SAPID FROM Machine WHERE SAPID=@s", new { s = "131453" });
|
||||
var rows = await helper.QueryAsync("BTDB", "SELECT Code, SAPID FROM Machine WHERE SAPID=@s", new { s = "131453" });
|
||||
Assert.Single(rows);
|
||||
var row = rows[0];
|
||||
Assert.Equal("Z28061A", row["code"]);
|
||||
@@ -59,10 +64,82 @@ public class InboundDatabaseHelperTests
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void Query_returns_empty_list_when_no_rows_match()
|
||||
public async Task QueryAsync_returns_empty_list_when_no_rows_match()
|
||||
{
|
||||
var helper = new InboundDatabaseHelper(SeededGateway(), CancellationToken.None);
|
||||
var rows = helper.Query("BTDB", "SELECT Code, SAPID FROM Machine WHERE SAPID=@s", new { s = "999999" });
|
||||
var rows = await helper.QueryAsync("BTDB", "SELECT Code, SAPID FROM Machine WHERE SAPID=@s", new { s = "999999" });
|
||||
Assert.Empty(rows);
|
||||
}
|
||||
|
||||
// --- InboundAPI-026: writes are permitted (design decision) ---
|
||||
|
||||
[Fact]
|
||||
public async Task ExecuteAsync_performs_write_and_returns_rows_affected()
|
||||
{
|
||||
var helper = new InboundDatabaseHelper(SeededGateway(), CancellationToken.None);
|
||||
var affected = await helper.ExecuteAsync(
|
||||
"BTDB", "UPDATE Machine SET Code=@c WHERE SAPID=@s", new { c = "UPDATED", s = "131453" });
|
||||
Assert.Equal(1, affected);
|
||||
|
||||
// The write actually landed (read it back through the same helper).
|
||||
var code = await helper.QuerySingleAsync<string>("BTDB", "SELECT Code FROM Machine WHERE SAPID=@s", new { s = "131453" });
|
||||
Assert.Equal("UPDATED", code);
|
||||
}
|
||||
|
||||
// --- InboundAPI-026: SQL-injection protection — values are bound, not concatenated ---
|
||||
|
||||
[Fact]
|
||||
public async Task ParameterValues_are_bound_not_concatenated()
|
||||
{
|
||||
// A classic injection payload as the parameter VALUE. If the helper concatenated
|
||||
// it, "WHERE SAPID='131453' OR '1'='1'" would match the seeded row and return
|
||||
// "Z28061A". Because the value is bound as a literal parameter, it matches no
|
||||
// SAPID and the query returns null — proving the value never altered the SQL.
|
||||
var helper = new InboundDatabaseHelper(SeededGateway(), CancellationToken.None);
|
||||
var code = await helper.QuerySingleAsync<string>(
|
||||
"BTDB", "SELECT Code FROM Machine WHERE SAPID=@s", new { s = "131453' OR '1'='1" });
|
||||
Assert.Null(code);
|
||||
}
|
||||
|
||||
// --- InboundAPI-028: negative paths ---
|
||||
|
||||
[Fact]
|
||||
public async Task QuerySingleAsync_null_gateway_throws()
|
||||
{
|
||||
var helper = new InboundDatabaseHelper(gateway: null, CancellationToken.None);
|
||||
await Assert.ThrowsAsync<InvalidOperationException>(
|
||||
() => helper.QuerySingleAsync<string>("BTDB", "SELECT 1"));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task QueryAsync_null_gateway_throws()
|
||||
{
|
||||
var helper = new InboundDatabaseHelper(gateway: null, CancellationToken.None);
|
||||
await Assert.ThrowsAsync<InvalidOperationException>(
|
||||
() => helper.QueryAsync("BTDB", "SELECT 1"));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task ExecuteAsync_null_gateway_throws()
|
||||
{
|
||||
var helper = new InboundDatabaseHelper(gateway: null, CancellationToken.None);
|
||||
await Assert.ThrowsAsync<InvalidOperationException>(
|
||||
() => helper.ExecuteAsync("BTDB", "DELETE FROM Machine"));
|
||||
}
|
||||
|
||||
// --- InboundAPI-027: the method-deadline token bounds the query on the EXECUTE path ---
|
||||
|
||||
[Fact]
|
||||
public async Task QuerySingleAsync_honours_cancellation_token_on_execute()
|
||||
{
|
||||
// The gateway opens with None (so the open succeeds), but the helper forwards the
|
||||
// pre-cancelled deadline token to ExecuteScalarAsync — which must observe it. This
|
||||
// proves the fix threads the token past the connection open, where the old
|
||||
// synchronous ExecuteScalar() ignored cancellation entirely.
|
||||
using var cts = new CancellationTokenSource();
|
||||
await cts.CancelAsync();
|
||||
var helper = new InboundDatabaseHelper(SeededGateway(honorTokenOnOpen: false), cts.Token);
|
||||
await Assert.ThrowsAnyAsync<OperationCanceledException>(
|
||||
() => helper.QuerySingleAsync<string>("BTDB", "SELECT Code FROM Machine"));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -549,7 +549,7 @@ public class InboundScriptExecutorTests
|
||||
[Fact]
|
||||
public async Task Script_UsingDatabase_QueriesViaGateway()
|
||||
{
|
||||
// A script that calls Database.QuerySingle runs against an executor whose
|
||||
// A script that calls Database.QuerySingleAsync runs against an executor whose
|
||||
// ServiceProvider registers an IDatabaseGateway backed by in-memory SQLite.
|
||||
var services = new ServiceCollection();
|
||||
services.AddSingleton<IDatabaseGateway>(SeededSqliteGateway());
|
||||
@@ -560,7 +560,7 @@ public class InboundScriptExecutorTests
|
||||
|
||||
var method = new ApiMethod(
|
||||
"movein",
|
||||
"return new { v = Database.QuerySingle<string>(\"BTDB\", \"SELECT Code FROM Machine WHERE SAPID=@s\", new { s = (string)Parameters[\"sap\"] }) };")
|
||||
"return new { v = await Database.QuerySingleAsync<string>(\"BTDB\", \"SELECT Code FROM Machine WHERE SAPID=@s\", new { s = (string)Parameters[\"sap\"] }) };")
|
||||
{
|
||||
Id = 1,
|
||||
TimeoutSeconds = 10,
|
||||
|
||||
@@ -214,10 +214,15 @@ public class RouteHelperTests
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task WaitForAttribute_WithNoExplicitToken_InheritsMethodDeadlineToken()
|
||||
public async Task WaitForAttribute_IsNotBoundByMethodDeadline_WaitTimeoutGoverns()
|
||||
{
|
||||
// InboundAPI-029 (spec §6): unlike Call/GetAttributes/SetAttributes, the wait is
|
||||
// bounded by its OWN timeout, not the method deadline. Even with the method
|
||||
// deadline ALREADY cancelled, the wait still proceeds and returns the site's
|
||||
// wait-timeout result — the deadline does not cut it short.
|
||||
SiteResolves("inst-1", "SiteA");
|
||||
using var deadline = new CancellationTokenSource();
|
||||
await deadline.CancelAsync(); // method deadline already elapsed
|
||||
CancellationToken seen = default;
|
||||
_router.RouteToWaitForAttributeAsync("SiteA", Arg.Any<RouteToWaitForAttributeRequest>(), Arg.Do<CancellationToken>(t => seen = t))
|
||||
.Returns(ci => new RouteToWaitForAttributeResponse(
|
||||
@@ -226,9 +231,52 @@ public class RouteHelperTests
|
||||
Success: true, ErrorMessage: null, DateTimeOffset.UtcNow));
|
||||
|
||||
var bound = CreateHelper().WithDeadline(deadline.Token);
|
||||
var matched = await bound.To("inst-1").WaitForAttribute("Flag", true, TimeSpan.FromSeconds(30));
|
||||
|
||||
Assert.False(matched); // the site's wait-timeout result, not a cancellation
|
||||
Assert.False(seen.IsCancellationRequested); // the wait token ignores the cancelled method deadline
|
||||
Assert.NotEqual(deadline.Token, seen); // it is a per-wait token, not the method deadline
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task WaitForAttribute_ExplicitToken_IsHonoured()
|
||||
{
|
||||
// An explicit caller token still applies (a tighter bound the script chose).
|
||||
SiteResolves("inst-1", "SiteA");
|
||||
using var explicitCts = new CancellationTokenSource();
|
||||
await explicitCts.CancelAsync();
|
||||
CancellationToken seen = default;
|
||||
_router.RouteToWaitForAttributeAsync("SiteA", Arg.Any<RouteToWaitForAttributeRequest>(), Arg.Do<CancellationToken>(t => seen = t))
|
||||
.Returns(ci => new RouteToWaitForAttributeResponse(
|
||||
((RouteToWaitForAttributeRequest)ci[1]).CorrelationId,
|
||||
Matched: false, Value: null, Quality: null, TimedOut: true,
|
||||
Success: true, ErrorMessage: null, DateTimeOffset.UtcNow));
|
||||
|
||||
await CreateHelper().To("inst-1")
|
||||
.WaitForAttribute("Flag", true, TimeSpan.FromSeconds(30), explicitCts.Token);
|
||||
|
||||
Assert.True(seen.IsCancellationRequested); // the explicit caller token cancels the wait
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task WaitForAttribute_ClientDisconnect_CancelsTheWait()
|
||||
{
|
||||
// A client disconnect (the raw request-abort token, threaded via WithRequestAborted)
|
||||
// still cancels the wait even though the method deadline does not bound it.
|
||||
SiteResolves("inst-1", "SiteA");
|
||||
using var clientAbort = new CancellationTokenSource();
|
||||
await clientAbort.CancelAsync();
|
||||
CancellationToken seen = default;
|
||||
_router.RouteToWaitForAttributeAsync("SiteA", Arg.Any<RouteToWaitForAttributeRequest>(), Arg.Do<CancellationToken>(t => seen = t))
|
||||
.Returns(ci => new RouteToWaitForAttributeResponse(
|
||||
((RouteToWaitForAttributeRequest)ci[1]).CorrelationId,
|
||||
Matched: false, Value: null, Quality: null, TimedOut: true,
|
||||
Success: true, ErrorMessage: null, DateTimeOffset.UtcNow));
|
||||
|
||||
var bound = CreateHelper().WithRequestAborted(clientAbort.Token);
|
||||
await bound.To("inst-1").WaitForAttribute("Flag", true, TimeSpan.FromSeconds(30));
|
||||
|
||||
Assert.Equal(deadline.Token, seen);
|
||||
Assert.True(seen.IsCancellationRequested); // the client-abort token cancels the wait
|
||||
}
|
||||
|
||||
[Fact]
|
||||
|
||||
Reference in New Issue
Block a user