diff --git a/code-reviews/InboundAPI/findings.md b/code-reviews/InboundAPI/findings.md index d8edbd6..3ef2912 100644 --- a/code-reviews/InboundAPI/findings.md +++ b/code-reviews/InboundAPI/findings.md @@ -8,7 +8,7 @@ | Last reviewed | 2026-05-17 | | Reviewer | claude-agent | | Commit reviewed | `39d737e` | -| Open findings | 4 | +| Open findings | 0 | ## Summary @@ -606,7 +606,7 @@ from "key not approved"), but that doc edit is outside this module's editable sc |--|--| | Severity | Medium | | Category | Design-document adherence | -| Status | Open | +| Status | Resolved | | Location | `src/ScadaLink.InboundAPI/InboundScriptExecutor.cs:201-205`, `src/ScadaLink.Commons/Entities/InboundApi/ApiMethod.cs:10` | **Description** @@ -639,7 +639,24 @@ value serialized as-is. Code and design doc must be reconciled. **Resolution** -_Unresolved._ +Resolved 2026-05-17 (commit ``): root cause confirmed — `ApiMethod` carries +`ReturnDefinition` but the executor did a blind `JsonSerializer.Serialize(result)`, +so a script returning the wrong shape silently emitted a malformed 200. Took +option (a): added `ReturnValueValidator`, the response-side mirror of +`ParameterValidator`. It parses `ReturnDefinition` (a JSON array of `{name,type}` +field definitions, same extended-type set as parameters), validates the serialized +script result against it — declared fields must be present with a compatible JSON +type, primitives type-checked, `Object`/`List` shape-checked — and a `null`/non-object +result is rejected when a structure is declared. `InboundScriptExecutor.ExecuteAsync` +now runs the validator after serialization and, on mismatch, logs and returns a +script failure (`"Method return value did not match its return definition"`, → 500) +instead of a malformed 200. A method with no `ReturnDefinition` stays unconstrained +(backward compatible). Doc-owner follow-up (outside this module's editable scope): +the `Component-InboundAPI.md` "Response Format" section may note that return shaping +is validation-only (no coercion). Regression tests: `ReturnValueValidatorTests` +(12 cases) plus executor-level `ReturnValue_MatchingReturnDefinition_Succeeds`, +`ReturnValue_NotMatchingReturnDefinition_ReturnsFailureNotMalformed200`, and +`ReturnValue_NoReturnDefinition_IsUnconstrained`. ### InboundAPI-015 — `ForbiddenApiChecker` is purely textual and is bypassable via reflection reachable without a forbidden namespace token @@ -647,7 +664,7 @@ _Unresolved._ |--|--| | Severity | Medium | | Category | Security | -| Status | Open | +| Status | Resolved | | Location | `src/ScadaLink.InboundAPI/ForbiddenApiChecker.cs:63-119`, `src/ScadaLink.InboundAPI/InboundScriptExecutor.cs:109-126` | **Description** @@ -691,7 +708,29 @@ that the current check is best-effort and does not stop a determined script. **Resolution** -_Unresolved._ +Resolved 2026-05-17 (commit ``): the specific reflection-via-permitted-member +vector was confirmed and the textual checker materially hardened against it (full +sandboxing remains a separate, larger design effort — see below). `ForbiddenApiWalker` +now, in addition to the namespace deny-list, rejects a curated set of reflection-gateway +**member names** (`GetType`, `GetTypeInfo`, `Assembly`, `Module`, `CreateInstance`, +`InvokeMember`, `GetMethod(s)`, `GetConstructor(s)`, `GetField(s)`, `GetProperty(ies)`, +`GetMember(s)`, `GetRuntimeMethod(s)`, `MethodHandle`, `TypeHandle`) regardless of the +receiver expression — so `typeof(string).Assembly.GetType("System.IO.File")` is now +caught because `.Assembly` and `.GetType` appear as accessed member names. It also +rejects a bare `Activator` identifier and the `dynamic` keyword (which widens +late-bound access the static walker cannot see through). `Invoke` is deliberately +**not** flagged so legitimate `Action`/`Func` delegate invocation still compiles — +the reflection `MethodInfo.Invoke` path is cut off by rejecting the `GetMethod` that +produces the `MethodInfo`. **Documented limitation:** this is hardened defence-in-depth, +not a true sandbox — a determined author may still find a vector the syntax walker +cannot see (e.g. via `Microsoft.CSharp.RuntimeBinder` internals or generics tricks). +Genuine containment needs a runtime boundary (restricted `AssemblyLoadContext` / +curated reference set that does not expose reflection-to-arbitrary-type / out-of-process +sandbox); that is tracked as a future design change and noted in the `ForbiddenApiChecker` +XML summary. Regression tests: new `ForbiddenApiCheckerTests` suite (19 cases) covering +the `Assembly`/`GetType`/`Type.GetType`/`Activator.CreateInstance`/`InvokeMember`/ +`GetMethod`/`GetTypeInfo`/`dynamic` bypass vectors plus permitted-script and +namespace-deny-list regression guards. ### InboundAPI-016 — Routed `Route.To().Call()` invocations are not bound by the method timeout @@ -699,7 +738,7 @@ _Unresolved._ |--|--| | Severity | Medium | | Category | Design-document adherence | -| Status | Open | +| Status | Resolved | | Location | `src/ScadaLink.InboundAPI/RouteHelper.cs:59-152`, `src/ScadaLink.InboundAPI/InboundScriptExecutor.cs:177`, `:199` | **Description** @@ -738,7 +777,24 @@ and abandon the in-flight request when it fires. **Resolution** -_Unresolved._ +Resolved 2026-05-17 (commit ``): root cause confirmed — every `RouteTarget` +method took `CancellationToken cancellationToken = default`, so a natural script +`Route.To("inst").Call("doWork", p)` routed with `CancellationToken.None` and was not +bound by the method timeout at all. `RouteHelper` now carries the executing method's +deadline token: `InboundScriptExecutor.ExecuteAsync` calls the new +`RouteHelper.WithDeadline(cts.Token)` when it builds the script context, so the route +helper handed to the script is bound to the method-level timeout CTS. Each +`RouteTarget` method resolves an *effective* token — the explicitly-supplied token if +the caller passed one (tighter bound preserved), otherwise the method deadline — and +forwards it into both `IInstanceLocator` site resolution and the routed call. The +deadline token therefore flows through to `CommunicationService.RouteTo*Async`, so +an in-flight routed call observes cancellation when the method timeout fires instead +of running orphaned. Regression tests (in the new `RouteHelperTests`): +`Call_WithNoExplicitToken_InheritsMethodDeadlineToken`, +`Call_WhenMethodDeadlineCancelled_RoutedCallObservesCancellation`, +`Call_ExplicitToken_OverridesDeadlineToken`, +`GetAttributes_WithNoExplicitToken_InheritsMethodDeadlineToken`, +`SetAttributes_WithNoExplicitToken_InheritsMethodDeadlineToken`. ### InboundAPI-017 — `RouteHelper` / `RouteTarget` has no test coverage @@ -746,7 +802,7 @@ _Unresolved._ |--|--| | Severity | Low | | Category | Testing coverage | -| Status | Open | +| Status | Resolved | | Location | `src/ScadaLink.InboundAPI/RouteHelper.cs:1-165`, `tests/ScadaLink.InboundAPI.Tests/` | **Description** @@ -776,4 +832,15 @@ wiring is added. **Resolution** -_Unresolved._ +Resolved 2026-05-17 (commit ``): confirmed — `ScadaLink.InboundAPI.Tests` had +no file exercising `RouteHelper`/`RouteTarget`. To make the surface testable without a +live actor system, an `IInstanceRouter` seam was introduced in the module (the routing +transport `RouteHelper` depends on); the production `CommunicationServiceInstanceRouter` +delegates to `CommunicationService` and is registered by `AddInboundAPI`. `RouteHelper` +now depends on `IInstanceLocator` + `IInstanceRouter` (both substitutable). Added the +`RouteHelperTests` suite (15 cases) covering: the happy path of `Call`/`GetAttribute(s)`/ +`SetAttribute(s)`, correlation-ID generation, the unresolved-instance +`InvalidOperationException` path, the `!Success` → `InvalidOperationException` mapping +for each routed method, `GetAttribute` delegating to the batch `GetAttributes` and +returning `null` for an absent key, `SetAttribute` delegating to `SetAttributes`, and +the InboundAPI-016 deadline-token inheritance behaviour. All 15 pass. diff --git a/src/ScadaLink.InboundAPI/CommunicationServiceInstanceRouter.cs b/src/ScadaLink.InboundAPI/CommunicationServiceInstanceRouter.cs new file mode 100644 index 0000000..5cfd634 --- /dev/null +++ b/src/ScadaLink.InboundAPI/CommunicationServiceInstanceRouter.cs @@ -0,0 +1,31 @@ +using ScadaLink.Commons.Messages.InboundApi; +using ScadaLink.Communication; + +namespace ScadaLink.InboundAPI; + +/// +/// Default implementation. Delegates every routed +/// call to , which dispatches to the target +/// site cluster via the central communication actor. +/// +public sealed class CommunicationServiceInstanceRouter : IInstanceRouter +{ + private readonly CommunicationService _communicationService; + + public CommunicationServiceInstanceRouter(CommunicationService communicationService) + { + _communicationService = communicationService; + } + + public Task RouteToCallAsync( + string siteId, RouteToCallRequest request, CancellationToken cancellationToken) => + _communicationService.RouteToCallAsync(siteId, request, cancellationToken); + + public Task RouteToGetAttributesAsync( + string siteId, RouteToGetAttributesRequest request, CancellationToken cancellationToken) => + _communicationService.RouteToGetAttributesAsync(siteId, request, cancellationToken); + + public Task RouteToSetAttributesAsync( + string siteId, RouteToSetAttributesRequest request, CancellationToken cancellationToken) => + _communicationService.RouteToSetAttributesAsync(siteId, request, cancellationToken); +} diff --git a/src/ScadaLink.InboundAPI/ForbiddenApiChecker.cs b/src/ScadaLink.InboundAPI/ForbiddenApiChecker.cs index bda3219..191ec19 100644 --- a/src/ScadaLink.InboundAPI/ForbiddenApiChecker.cs +++ b/src/ScadaLink.InboundAPI/ForbiddenApiChecker.cs @@ -15,6 +15,21 @@ namespace ScadaLink.InboundAPI; /// sandbox — so a script can fully-qualify any referenced type. This static check /// walks the script syntax tree and rejects any reference to a forbidden namespace, /// whether reached through a using directive or a fully-qualified name. +/// +/// +/// InboundAPI-015: a purely namespace-textual deny-list is bypassable because +/// reflection is reachable through members of permitted types that never +/// spell a forbidden namespace, e.g. +/// typeof(string).Assembly.GetType("System.IO.File"). The walker therefore +/// also rejects a curated set of reflection-gateway member names (GetType, +/// Assembly, GetMethod, InvokeMember, CreateInstance, …) +/// and the dynamic keyword. This is hardening of a best-effort static check, +/// not a true sandbox — a determined script author may still find +/// a vector the syntax walker cannot see (see the security notes in +/// code-reviews/InboundAPI/findings.md, InboundAPI-015). The check is +/// defence-in-depth; genuine containment needs a runtime boundary (restricted +/// AssemblyLoadContext / curated reference set / out-of-process sandbox). +/// /// public static class ForbiddenApiChecker { @@ -42,6 +57,40 @@ public static class ForbiddenApiChecker "System.Threading.Tasks", }; + /// + /// InboundAPI-015: member names that are reflection gateways. Reaching any of + /// these — even off a permitted type such as typeof(string) or a plain + /// string — lets a script escape the namespace deny-list (obtain an + /// arbitrary Type, load an assembly, late-bind a method). They are + /// rejected regardless of the receiver expression. Invoke is deliberately + /// excluded because Action/Func delegate invocation is legitimate; + /// the reflection MethodInfo.Invoke path is already cut off by rejecting + /// the GetMethod/GetConstructor that produces the MethodInfo. + /// + private static readonly HashSet ForbiddenMemberNames = new(StringComparer.Ordinal) + { + "GetType", // object.GetType() / Type.GetType(string) — yields a System.Type + "GetTypeInfo", // -> TypeInfo (reflection) + "Assembly", // Type.Assembly — yields a System.Reflection.Assembly + "Module", // Type.Module / MethodBase.Module + "CreateInstance", // Activator.CreateInstance / Assembly.CreateInstance + "InvokeMember", // Type.InvokeMember — late-bound dispatch + "GetMethod", + "GetMethods", + "GetConstructor", + "GetConstructors", + "GetField", + "GetFields", + "GetProperty", + "GetProperties", + "GetMember", + "GetMembers", + "GetRuntimeMethod", + "GetRuntimeMethods", + "MethodHandle", // RuntimeMethodHandle escape + "TypeHandle", + }; + /// /// Analyses the script source and returns the list of trust-model violations. /// An empty list means the script is acceptable. @@ -115,7 +164,42 @@ public static class ForbiddenApiChecker return; } + // InboundAPI-015: reject reflection-gateway members regardless of the + // receiver. typeof(string).Assembly.GetType("System.IO.File") never + // spells a forbidden namespace, but '.Assembly' and '.GetType' do + // appear here as the accessed member name. + var memberName = node.Name.Identifier.ValueText; + if (ForbiddenMemberNames.Contains(memberName)) + { + _violations.Add($"forbidden reflection member access '.{memberName}'"); + // Still descend: the receiver may contain a further violation. + } + base.VisitMemberAccessExpression(node); } + + public override void VisitIdentifierName(IdentifierNameSyntax node) + { + // InboundAPI-015: 'dynamic' widens late-bound member access that the + // static walker cannot see through — reject its use outright. The + // 'dynamic' contextual keyword surfaces as an identifier name. + if (node.Identifier.ValueText == "dynamic") + { + _violations.Add("forbidden use of the 'dynamic' keyword"); + return; + } + + // InboundAPI-015: a bare reference to the reflection entry-point types + // (e.g. 'Activator', 'Type') as an identifier. 'Activator' has no + // non-reflection use; flag it. ('Type' as an identifier is too broad + // to flag here — the gateway members above already cut off its use.) + if (node.Identifier.ValueText == "Activator") + { + _violations.Add("forbidden reflection type reference 'Activator'"); + return; + } + + base.VisitIdentifierName(node); + } } } diff --git a/src/ScadaLink.InboundAPI/IInstanceRouter.cs b/src/ScadaLink.InboundAPI/IInstanceRouter.cs new file mode 100644 index 0000000..21afdf3 --- /dev/null +++ b/src/ScadaLink.InboundAPI/IInstanceRouter.cs @@ -0,0 +1,22 @@ +using ScadaLink.Commons.Messages.InboundApi; + +namespace ScadaLink.InboundAPI; + +/// +/// Seam over the cross-site routing transport used by . +/// The production implementation () +/// delegates to ScadaLink.Communication.CommunicationService; the interface +/// exists so / can be unit tested +/// without a live actor system (InboundAPI-017). +/// +public interface IInstanceRouter +{ + Task RouteToCallAsync( + string siteId, RouteToCallRequest request, CancellationToken cancellationToken); + + Task RouteToGetAttributesAsync( + string siteId, RouteToGetAttributesRequest request, CancellationToken cancellationToken); + + Task RouteToSetAttributesAsync( + string siteId, RouteToSetAttributesRequest request, CancellationToken cancellationToken); +} diff --git a/src/ScadaLink.InboundAPI/InboundScriptExecutor.cs b/src/ScadaLink.InboundAPI/InboundScriptExecutor.cs index 43ba800..ad969f2 100644 --- a/src/ScadaLink.InboundAPI/InboundScriptExecutor.cs +++ b/src/ScadaLink.InboundAPI/InboundScriptExecutor.cs @@ -174,7 +174,10 @@ public class InboundScriptExecutor try { - var context = new InboundScriptContext(parameters, route, cts.Token); + // InboundAPI-016: bind the route helper to the method deadline so a + // routed Route.To(...).Call(...) inherits the method-level timeout + // without the script having to thread the context token by hand. + var context = new InboundScriptContext(parameters, route.WithDeadline(cts.Token), cts.Token); if (!_scriptHandlers.TryGetValue(method.Name, out var handler)) { @@ -202,6 +205,19 @@ public class InboundScriptExecutor ? JsonSerializer.Serialize(result) : null; + // InboundAPI-014: validate the script's return value against the + // method's declared ReturnDefinition. A method whose script returns a + // shape inconsistent with its definition must not silently emit a + // malformed 200 — surface it as a script failure (500) and log. + var returnValidation = ReturnValueValidator.Validate(resultJson, method.ReturnDefinition); + if (!returnValidation.IsValid) + { + _logger.LogWarning( + "API method {Method} return value rejected: {Error}", + method.Name, returnValidation.ErrorMessage); + return new InboundScriptResult(false, null, "Method return value did not match its return definition"); + } + return new InboundScriptResult(true, resultJson, null); } catch (OperationCanceledException) diff --git a/src/ScadaLink.InboundAPI/ReturnValueValidator.cs b/src/ScadaLink.InboundAPI/ReturnValueValidator.cs new file mode 100644 index 0000000..24292c8 --- /dev/null +++ b/src/ScadaLink.InboundAPI/ReturnValueValidator.cs @@ -0,0 +1,144 @@ +using System.Text.Json; + +namespace ScadaLink.InboundAPI; + +/// +/// InboundAPI-014: validates a method script's return value against the method's +/// declared ReturnDefinition. Component-InboundAPI.md ("Return Value +/// Definition" / "Response Format") states the success body has "fields matching +/// the return value definition"; this is the response-side mirror of +/// . +/// +/// +/// The return definition is a JSON array of +/// (the same {name,type} shape as a parameter definition). A method whose +/// ReturnDefinition is null/empty is unconstrained — its return value is +/// serialized as-is (backward compatible). Primitive fields (Boolean / Integer / +/// Float / String) are type-checked; the extended Object/List types +/// are shape-checked only (object vs. array), consistent with how +/// treats inbound extended types. +/// +/// +public static class ReturnValueValidator +{ + /// + /// Validates the serialized script result JSON against the method's return + /// definition. Returns when no + /// definition is configured or the result conforms to it. + /// + public static ReturnValidationResult Validate(string? resultJson, string? returnDefinition) + { + if (string.IsNullOrWhiteSpace(returnDefinition)) + { + // No declared return shape — the script's return value is unconstrained. + return ReturnValidationResult.Valid(); + } + + List fields; + try + { + fields = JsonSerializer.Deserialize>( + returnDefinition, + new JsonSerializerOptions { PropertyNameCaseInsensitive = true }) + ?? []; + } + catch (JsonException) + { + return ReturnValidationResult.Invalid( + "Invalid return definition in method configuration"); + } + + if (fields.Count == 0) + { + return ReturnValidationResult.Valid(); + } + + if (string.IsNullOrWhiteSpace(resultJson)) + { + return ReturnValidationResult.Invalid( + "Method declares a return structure but the script returned no value"); + } + + JsonElement root; + try + { + using var doc = JsonDocument.Parse(resultJson); + root = doc.RootElement.Clone(); + } + catch (JsonException) + { + return ReturnValidationResult.Invalid("Script return value is not valid JSON"); + } + + if (root.ValueKind != JsonValueKind.Object) + { + return ReturnValidationResult.Invalid( + "Method declares a return structure but the script did not return an object"); + } + + var errors = new List(); + foreach (var field in fields) + { + if (!root.TryGetProperty(field.Name, out var value)) + { + errors.Add($"missing return field '{field.Name}'"); + continue; + } + + var typeError = CheckFieldType(value, field.Type, field.Name); + if (typeError != null) + errors.Add(typeError); + } + + return errors.Count > 0 + ? ReturnValidationResult.Invalid( + $"Return value does not match the declared return definition: {string.Join("; ", errors)}") + : ReturnValidationResult.Valid(); + } + + private static string? CheckFieldType(JsonElement value, string declaredType, string fieldName) + { + // A null value satisfies any field type — the script may legitimately omit + // optional data; only a missing field (handled by the caller) is an error. + if (value.ValueKind == JsonValueKind.Null) + return null; + + var ok = declaredType.ToLowerInvariant() switch + { + "boolean" => value.ValueKind is JsonValueKind.True or JsonValueKind.False, + "integer" => value.ValueKind == JsonValueKind.Number && value.TryGetInt64(out _), + "float" => value.ValueKind == JsonValueKind.Number, + "string" => value.ValueKind == JsonValueKind.String, + "object" => value.ValueKind == JsonValueKind.Object, + "list" => value.ValueKind == JsonValueKind.Array, + _ => true, // unknown declared type — do not block the response + }; + + return ok ? null : $"return field '{fieldName}' must be {declaredType}"; + } +} + +/// +/// InboundAPI-014: one field of a method's declared return structure — the +/// deserialized form of an entry in ApiMethod.ReturnDefinition. Defined in +/// this module (not Commons) because the inbound API is currently its only consumer. +/// +public sealed class ReturnFieldDefinition +{ + public string Name { get; set; } = string.Empty; + public string Type { get; set; } = "String"; +} + +/// +/// Result of validating a script return value against a method's return definition. +/// +public sealed class ReturnValidationResult +{ + public bool IsValid { get; private init; } + public string ErrorMessage { get; private init; } = string.Empty; + + public static ReturnValidationResult Valid() => new() { IsValid = true }; + + public static ReturnValidationResult Invalid(string message) => + new() { IsValid = false, ErrorMessage = message }; +} diff --git a/src/ScadaLink.InboundAPI/RouteHelper.cs b/src/ScadaLink.InboundAPI/RouteHelper.cs index 207b83a..dc83d63 100644 --- a/src/ScadaLink.InboundAPI/RouteHelper.cs +++ b/src/ScadaLink.InboundAPI/RouteHelper.cs @@ -1,34 +1,58 @@ using ScadaLink.Commons.Interfaces.Services; using ScadaLink.Commons.Messages.InboundApi; using ScadaLink.Commons.Types; -using ScadaLink.Communication; namespace ScadaLink.InboundAPI; /// /// WP-4: Route.To() helper for cross-site calls from inbound API scripts. -/// Resolves instance to site, routes via CommunicationService, blocks until response or timeout. -/// Site unreachable returns error (no store-and-forward). +/// Resolves instance to site, routes via , blocks until +/// response or timeout. Site unreachable returns error (no store-and-forward). +/// +/// InboundAPI-016: the helper carries the executing method's +/// (the method-level timeout). Routed calls inherit that deadline by default, so a +/// natural script — Route.To("inst").Call("doWork", p) — is timeout-bounded +/// without the script having to thread a token explicitly. /// public class RouteHelper { private readonly IInstanceLocator _instanceLocator; - private readonly CommunicationService _communicationService; + private readonly IInstanceRouter _instanceRouter; + private readonly CancellationToken _deadlineToken; public RouteHelper( IInstanceLocator instanceLocator, - CommunicationService communicationService) + IInstanceRouter instanceRouter) + : this(instanceLocator, instanceRouter, CancellationToken.None) + { + } + + private RouteHelper( + IInstanceLocator instanceLocator, + IInstanceRouter instanceRouter, + CancellationToken deadlineToken) { _instanceLocator = instanceLocator; - _communicationService = communicationService; + _instanceRouter = instanceRouter; + _deadlineToken = deadlineToken; } + /// + /// InboundAPI-016: returns a whose routed calls inherit + /// (the executing method's timeout) by default. + /// calls this when it builds the script + /// context so the method timeout actually covers routed calls, as the design doc + /// requires. + /// + public RouteHelper WithDeadline(CancellationToken deadlineToken) => + new(_instanceLocator, _instanceRouter, deadlineToken); + /// /// Creates a route target for the specified instance. /// public RouteTarget To(string instanceCode) { - return new RouteTarget(instanceCode, _instanceLocator, _communicationService); + return new RouteTarget(instanceCode, _instanceLocator, _instanceRouter, _deadlineToken); } } @@ -39,36 +63,43 @@ public class RouteTarget { private readonly string _instanceCode; private readonly IInstanceLocator _instanceLocator; - private readonly CommunicationService _communicationService; + private readonly IInstanceRouter _instanceRouter; + private readonly CancellationToken _deadlineToken; internal RouteTarget( string instanceCode, IInstanceLocator instanceLocator, - CommunicationService communicationService) + IInstanceRouter instanceRouter, + CancellationToken deadlineToken) { _instanceCode = instanceCode; _instanceLocator = instanceLocator; - _communicationService = communicationService; + _instanceRouter = instanceRouter; + _deadlineToken = deadlineToken; } /// /// Calls a script on the remote instance. Synchronous from API caller's /// perspective. may be a dictionary or an /// anonymous object (new { name = "Bob" }) — see . + /// + /// InboundAPI-016: when is not supplied the + /// routed call inherits the executing method's timeout, so the call is bounded by + /// the method-level deadline with no token argument. /// public async Task Call( string scriptName, object? parameters = null, CancellationToken cancellationToken = default) { - var siteId = await ResolveSiteAsync(cancellationToken); + var token = Effective(cancellationToken); + var siteId = await ResolveSiteAsync(token); var correlationId = Guid.NewGuid().ToString(); var request = new RouteToCallRequest( correlationId, _instanceCode, scriptName, ScriptArgs.Normalize(parameters), DateTimeOffset.UtcNow); - var response = await _communicationService.RouteToCallAsync( - siteId, request, cancellationToken); + var response = await _instanceRouter.RouteToCallAsync(siteId, request, token); if (!response.Success) { @@ -97,14 +128,14 @@ public class RouteTarget IEnumerable attributeNames, CancellationToken cancellationToken = default) { - var siteId = await ResolveSiteAsync(cancellationToken); + var token = Effective(cancellationToken); + var siteId = await ResolveSiteAsync(token); var correlationId = Guid.NewGuid().ToString(); var request = new RouteToGetAttributesRequest( correlationId, _instanceCode, attributeNames.ToList(), DateTimeOffset.UtcNow); - var response = await _communicationService.RouteToGetAttributesAsync( - siteId, request, cancellationToken); + var response = await _instanceRouter.RouteToGetAttributesAsync(siteId, request, token); if (!response.Success) { @@ -135,14 +166,14 @@ public class RouteTarget IReadOnlyDictionary attributeValues, CancellationToken cancellationToken = default) { - var siteId = await ResolveSiteAsync(cancellationToken); + var token = Effective(cancellationToken); + var siteId = await ResolveSiteAsync(token); var correlationId = Guid.NewGuid().ToString(); var request = new RouteToSetAttributesRequest( correlationId, _instanceCode, attributeValues, DateTimeOffset.UtcNow); - var response = await _communicationService.RouteToSetAttributesAsync( - siteId, request, cancellationToken); + var response = await _instanceRouter.RouteToSetAttributesAsync(siteId, request, token); if (!response.Success) { @@ -151,6 +182,13 @@ public class RouteTarget } } + /// + /// InboundAPI-016: a routed call with no explicit token inherits the executing + /// method's deadline. An explicitly supplied token (for a tighter bound) wins. + /// + private CancellationToken Effective(CancellationToken explicitToken) => + explicitToken.CanBeCanceled ? explicitToken : _deadlineToken; + private async Task ResolveSiteAsync(CancellationToken cancellationToken) { var siteId = await _instanceLocator.GetSiteIdForInstanceAsync(_instanceCode, cancellationToken); diff --git a/src/ScadaLink.InboundAPI/ServiceCollectionExtensions.cs b/src/ScadaLink.InboundAPI/ServiceCollectionExtensions.cs index d67d454..b98c014 100644 --- a/src/ScadaLink.InboundAPI/ServiceCollectionExtensions.cs +++ b/src/ScadaLink.InboundAPI/ServiceCollectionExtensions.cs @@ -10,6 +10,10 @@ public static class ServiceCollectionExtensions services.AddSingleton(); services.AddScoped(); + // InboundAPI-017: routed calls go through the IInstanceRouter seam; the + // production implementation delegates to CommunicationService. + services.AddScoped(); + // InboundAPI-006 / InboundAPI-008: endpoint filter enforcing the request // body size cap and active-node gating for POST /api/{methodName}. services.AddSingleton(); diff --git a/tests/ScadaLink.InboundAPI.Tests/ForbiddenApiCheckerTests.cs b/tests/ScadaLink.InboundAPI.Tests/ForbiddenApiCheckerTests.cs new file mode 100644 index 0000000..fe70fda --- /dev/null +++ b/tests/ScadaLink.InboundAPI.Tests/ForbiddenApiCheckerTests.cs @@ -0,0 +1,104 @@ +namespace ScadaLink.InboundAPI.Tests; + +/// +/// InboundAPI-005 / InboundAPI-015: tests for the script-trust-model checker. +/// +/// InboundAPI-015 hardens the textual walker against reflection reached through +/// permitted-type members that never spell a forbidden namespace, e.g. +/// typeof(string).Assembly.GetType("System.IO.File"). +/// +public class ForbiddenApiCheckerTests +{ + private static bool IsRejected(string script) => + ForbiddenApiChecker.FindViolations(script).Count > 0; + + // --- Baseline: legitimate scripts must still pass --- + + [Theory] + [InlineData("return 1 + 1;")] + [InlineData("var list = new List { 1, 2, 3 }; return list.Sum();")] + [InlineData("return Parameters.Get(\"x\") * 2;")] + [InlineData("await Task.Delay(1); return null;")] + [InlineData("var r = await Route.To(\"inst\").Call(\"s\"); return r;")] + [InlineData("Action a = () => {}; a.Invoke(); return null;")] + public void PermittedScript_NotRejected(string script) + { + Assert.False(IsRejected(script), script); + } + + // --- Baseline: forbidden namespaces (textual) must still be rejected --- + + [Theory] + [InlineData("System.IO.File.Delete(\"/tmp/x\"); return null;")] + [InlineData("System.Diagnostics.Process.Start(\"/bin/sh\"); return null;")] + [InlineData("using System.Reflection; return null;")] + [InlineData("var s = new System.Net.Sockets.Socket(default, default, default); return null;")] + public void ForbiddenNamespace_Rejected(string script) + { + Assert.True(IsRejected(script), script); + } + + // --- InboundAPI-015: reflection reachable without a forbidden namespace token --- + + [Fact] + public void Reflection_AssemblyPropertyAccess_Rejected() + { + // typeof(string).Assembly — .Assembly is a reflection gateway off a permitted type. + Assert.True(IsRejected("var a = typeof(string).Assembly; return null;")); + } + + [Fact] + public void Reflection_AssemblyGetType_Rejected() + { + // The classic bypass: obtain System.IO.File as a Type via a string literal. + Assert.True(IsRejected( + "var t = typeof(string).Assembly.GetType(\"System.IO.File\"); return null;")); + } + + [Fact] + public void Reflection_ObjectGetType_Rejected() + { + // x.GetType() returns a System.Type — a reflection gateway. + Assert.True(IsRejected("var t = \"\".GetType(); return null;")); + } + + [Fact] + public void Reflection_TypeGetTypeStatic_Rejected() + { + Assert.True(IsRejected("var t = Type.GetType(\"System.IO.File\"); return null;")); + } + + [Fact] + public void Reflection_ActivatorCreateInstance_Rejected() + { + Assert.True(IsRejected( + "var o = Activator.CreateInstance(Type.GetType(\"System.IO.File\")); return null;")); + } + + [Fact] + public void Reflection_InvokeMember_Rejected() + { + Assert.True(IsRejected( + "typeof(object).InvokeMember(\"x\", default, null, null, null); return null;")); + } + + [Fact] + public void Reflection_GetMethodInvoke_Rejected() + { + Assert.True(IsRejected( + "var m = typeof(object).GetMethod(\"ToString\"); return null;")); + } + + [Fact] + public void Reflection_GetTypeInfo_Rejected() + { + Assert.True(IsRejected("var ti = \"\".GetType().GetTypeInfo(); return null;")); + } + + [Fact] + public void DynamicKeyword_Rejected() + { + // dynamic widens late-bound member access the static walker cannot see through. + Assert.True(IsRejected("dynamic d = Parameters; return null;")); + } +} diff --git a/tests/ScadaLink.InboundAPI.Tests/InboundScriptExecutorTests.cs b/tests/ScadaLink.InboundAPI.Tests/InboundScriptExecutorTests.cs index 3dbf375..5fbc6ee 100644 --- a/tests/ScadaLink.InboundAPI.Tests/InboundScriptExecutorTests.cs +++ b/tests/ScadaLink.InboundAPI.Tests/InboundScriptExecutorTests.cs @@ -3,7 +3,6 @@ using Microsoft.Extensions.Logging.Abstractions; using NSubstitute; using ScadaLink.Commons.Entities.InboundApi; using ScadaLink.Commons.Interfaces.Services; -using ScadaLink.Communication; namespace ScadaLink.InboundAPI.Tests; @@ -20,10 +19,8 @@ public class InboundScriptExecutorTests { _executor = new InboundScriptExecutor(NullLogger.Instance, Substitute.For()); var locator = Substitute.For(); - var commService = Substitute.For( - Microsoft.Extensions.Options.Options.Create(new CommunicationOptions()), - NullLogger.Instance); - _route = new RouteHelper(locator, commService); + var router = Substitute.For(); + _route = new RouteHelper(locator, router); } [Fact] @@ -364,6 +361,72 @@ public class InboundScriptExecutorTests Assert.True(_executor.CompileAndRegister(good)); } + // --- InboundAPI-014: the script return value is validated against ReturnDefinition --- + + [Fact] + public async Task ReturnValue_MatchingReturnDefinition_Succeeds() + { + var method = new ApiMethod("shaped", "return x;") + { + Id = 1, + TimeoutSeconds = 10, + ReturnDefinition = """[{"name":"siteName","type":"String"},{"name":"total","type":"Integer"}]""", + }; + _executor.RegisterHandler("shaped", async ctx => + { + await Task.CompletedTask; + return new { siteName = "Site Alpha", total = 14250 }; + }); + + var result = await _executor.ExecuteAsync( + method, new Dictionary(), _route, TimeSpan.FromSeconds(10)); + + Assert.True(result.Success, result.ErrorMessage); + } + + [Fact] + public async Task ReturnValue_NotMatchingReturnDefinition_ReturnsFailureNotMalformed200() + { + // The script returns a structure inconsistent with the declared return + // definition (missing 'total'). It must surface as a failure, not a 200. + var method = new ApiMethod("misshaped", "return x;") + { + Id = 1, + TimeoutSeconds = 10, + ReturnDefinition = """[{"name":"siteName","type":"String"},{"name":"total","type":"Integer"}]""", + }; + _executor.RegisterHandler("misshaped", async ctx => + { + await Task.CompletedTask; + return new { siteName = "Site Alpha" }; // 'total' missing + }); + + var result = await _executor.ExecuteAsync( + method, new Dictionary(), _route, TimeSpan.FromSeconds(10)); + + Assert.False(result.Success); + Assert.Null(result.ResultJson); + } + + [Fact] + public async Task ReturnValue_NoReturnDefinition_IsUnconstrained() + { + // A method with no ReturnDefinition keeps the prior behaviour — the return + // value is serialized as-is. + var method = new ApiMethod("free", "return x;") { Id = 1, TimeoutSeconds = 10 }; + _executor.RegisterHandler("free", async ctx => + { + await Task.CompletedTask; + return new { whatever = 1 }; + }); + + var result = await _executor.ExecuteAsync( + method, new Dictionary(), _route, TimeSpan.FromSeconds(10)); + + Assert.True(result.Success, result.ErrorMessage); + Assert.Contains("whatever", result.ResultJson!); + } + private sealed class CompileLogCounter { public int CompilationFailures; diff --git a/tests/ScadaLink.InboundAPI.Tests/ReturnValueValidatorTests.cs b/tests/ScadaLink.InboundAPI.Tests/ReturnValueValidatorTests.cs new file mode 100644 index 0000000..5e08830 --- /dev/null +++ b/tests/ScadaLink.InboundAPI.Tests/ReturnValueValidatorTests.cs @@ -0,0 +1,118 @@ +namespace ScadaLink.InboundAPI.Tests; + +/// +/// InboundAPI-014: tests for return-value validation against a method's +/// ReturnDefinition. Previously the script's return value was serialized +/// verbatim with no checking against the declared return structure. +/// +public class ReturnValueValidatorTests +{ + // --- No definition → no validation (backward compatible) --- + + [Theory] + [InlineData(null)] + [InlineData("")] + [InlineData(" ")] + public void NoReturnDefinition_AnythingIsValid(string? returnDefinition) + { + var result = ReturnValueValidator.Validate("{\"anything\":1}", returnDefinition); + Assert.True(result.IsValid); + } + + [Fact] + public void NoReturnDefinition_NullResult_IsValid() + { + var result = ReturnValueValidator.Validate(null, null); + Assert.True(result.IsValid); + } + + // --- Happy path: result matches the declared field shape --- + + [Fact] + public void ResultMatchingDefinition_IsValid() + { + const string def = """[{"name":"siteName","type":"String"},{"name":"totalUnits","type":"Integer"}]"""; + const string json = """{"siteName":"Site Alpha","totalUnits":14250}"""; + + var result = ReturnValueValidator.Validate(json, def); + + Assert.True(result.IsValid); + } + + [Fact] + public void ResultWithListField_ShapeChecked_IsValid() + { + const string def = """[{"name":"lines","type":"List"}]"""; + const string json = """{"lines":[{"lineName":"Line-1","units":8200}]}"""; + + var result = ReturnValueValidator.Validate(json, def); + + Assert.True(result.IsValid); + } + + // --- Mismatches must be reported --- + + [Fact] + public void ResultMissingDeclaredField_IsInvalid() + { + const string def = """[{"name":"siteName","type":"String"},{"name":"totalUnits","type":"Integer"}]"""; + const string json = """{"siteName":"Site Alpha"}"""; + + var result = ReturnValueValidator.Validate(json, def); + + Assert.False(result.IsValid); + Assert.Contains("totalUnits", result.ErrorMessage); + } + + [Fact] + public void ResultFieldWrongType_IsInvalid() + { + const string def = """[{"name":"totalUnits","type":"Integer"}]"""; + const string json = """{"totalUnits":"not-a-number"}"""; + + var result = ReturnValueValidator.Validate(json, def); + + Assert.False(result.IsValid); + Assert.Contains("totalUnits", result.ErrorMessage); + } + + [Fact] + public void NullResultWhenStructureRequired_IsInvalid() + { + const string def = """[{"name":"siteName","type":"String"}]"""; + + var result = ReturnValueValidator.Validate(null, def); + + Assert.False(result.IsValid); + } + + [Fact] + public void NonObjectResultWhenStructureRequired_IsInvalid() + { + const string def = """[{"name":"siteName","type":"String"}]"""; + + var result = ReturnValueValidator.Validate("42", def); + + Assert.False(result.IsValid); + } + + [Fact] + public void ListFieldGivenNonArray_IsInvalid() + { + const string def = """[{"name":"lines","type":"List"}]"""; + const string json = """{"lines":"not-a-list"}"""; + + var result = ReturnValueValidator.Validate(json, def); + + Assert.False(result.IsValid); + Assert.Contains("lines", result.ErrorMessage); + } + + [Fact] + public void MalformedReturnDefinition_IsInvalid() + { + var result = ReturnValueValidator.Validate("{\"x\":1}", "%%% not json %%%"); + + Assert.False(result.IsValid); + } +} diff --git a/tests/ScadaLink.InboundAPI.Tests/RouteHelperTests.cs b/tests/ScadaLink.InboundAPI.Tests/RouteHelperTests.cs new file mode 100644 index 0000000..3feaaaf --- /dev/null +++ b/tests/ScadaLink.InboundAPI.Tests/RouteHelperTests.cs @@ -0,0 +1,268 @@ +using NSubstitute; +using ScadaLink.Commons.Interfaces.Services; +using ScadaLink.Commons.Messages.InboundApi; + +namespace ScadaLink.InboundAPI.Tests; + +/// +/// WP-4: Tests for / — the +/// cross-site Route.To() routing surface inbound API scripts use. +/// +/// InboundAPI-017: this surface previously had zero coverage. +/// InboundAPI-016: routed calls must inherit the executing method's deadline token. +/// +public class RouteHelperTests +{ + private readonly IInstanceLocator _locator = Substitute.For(); + private readonly IInstanceRouter _router = Substitute.For(); + + private RouteHelper CreateHelper() => new(_locator, _router); + + private void SiteResolves(string instanceCode, string siteId) => + _locator.GetSiteIdForInstanceAsync(instanceCode, Arg.Any()) + .Returns(siteId); + + // --- Call --- + + [Fact] + public async Task Call_HappyPath_ResolvesSiteAndReturnsValue() + { + SiteResolves("inst-1", "SiteA"); + _router.RouteToCallAsync("SiteA", Arg.Any(), Arg.Any()) + .Returns(ci => new RouteToCallResponse( + ((RouteToCallRequest)ci[1]).CorrelationId, true, 99, null, DateTimeOffset.UtcNow)); + + var result = await CreateHelper().To("inst-1").Call("doWork", new { x = 1 }); + + Assert.Equal(99, result); + } + + [Fact] + public async Task Call_GeneratesCorrelationId() + { + SiteResolves("inst-1", "SiteA"); + RouteToCallRequest? captured = null; + _router.RouteToCallAsync("SiteA", Arg.Do(r => captured = r), Arg.Any()) + .Returns(ci => new RouteToCallResponse( + ((RouteToCallRequest)ci[1]).CorrelationId, true, null, null, DateTimeOffset.UtcNow)); + + await CreateHelper().To("inst-1").Call("doWork"); + + Assert.NotNull(captured); + Assert.True(Guid.TryParse(captured!.CorrelationId, out _)); + } + + [Fact] + public async Task Call_RemoteFailure_ThrowsInvalidOperationException() + { + SiteResolves("inst-1", "SiteA"); + _router.RouteToCallAsync("SiteA", Arg.Any(), Arg.Any()) + .Returns(ci => new RouteToCallResponse( + ((RouteToCallRequest)ci[1]).CorrelationId, false, null, "site exploded", DateTimeOffset.UtcNow)); + + var ex = await Assert.ThrowsAsync( + () => CreateHelper().To("inst-1").Call("doWork")); + Assert.Equal("site exploded", ex.Message); + } + + [Fact] + public async Task Call_UnresolvedInstance_ThrowsInvalidOperationException() + { + // Locator returns null → instance not found / no assigned site. + _locator.GetSiteIdForInstanceAsync("ghost", Arg.Any()) + .Returns((string?)null); + + var ex = await Assert.ThrowsAsync( + () => CreateHelper().To("ghost").Call("doWork")); + Assert.Contains("ghost", ex.Message); + } + + // --- GetAttribute(s) --- + + [Fact] + public async Task GetAttributes_HappyPath_ReturnsValues() + { + SiteResolves("inst-1", "SiteA"); + _router.RouteToGetAttributesAsync("SiteA", Arg.Any(), Arg.Any()) + .Returns(ci => new RouteToGetAttributesResponse( + ((RouteToGetAttributesRequest)ci[1]).CorrelationId, + new Dictionary { ["a"] = 1, ["b"] = 2 }, + true, null, DateTimeOffset.UtcNow)); + + var result = await CreateHelper().To("inst-1").GetAttributes(new[] { "a", "b" }); + + Assert.Equal(1, result["a"]); + Assert.Equal(2, result["b"]); + } + + [Fact] + public async Task GetAttribute_DelegatesToBatch_AndReturnsSingleValue() + { + SiteResolves("inst-1", "SiteA"); + _router.RouteToGetAttributesAsync("SiteA", Arg.Any(), Arg.Any()) + .Returns(ci => new RouteToGetAttributesResponse( + ((RouteToGetAttributesRequest)ci[1]).CorrelationId, + new Dictionary { ["temp"] = 21.5 }, + true, null, DateTimeOffset.UtcNow)); + + var value = await CreateHelper().To("inst-1").GetAttribute("temp"); + + Assert.Equal(21.5, value); + } + + [Fact] + public async Task GetAttribute_AbsentKey_ReturnsNull() + { + SiteResolves("inst-1", "SiteA"); + _router.RouteToGetAttributesAsync("SiteA", Arg.Any(), Arg.Any()) + .Returns(ci => new RouteToGetAttributesResponse( + ((RouteToGetAttributesRequest)ci[1]).CorrelationId, + new Dictionary(), // batch returns nothing for the key + true, null, DateTimeOffset.UtcNow)); + + var value = await CreateHelper().To("inst-1").GetAttribute("missing"); + + Assert.Null(value); + } + + [Fact] + public async Task GetAttributes_RemoteFailure_ThrowsInvalidOperationException() + { + SiteResolves("inst-1", "SiteA"); + _router.RouteToGetAttributesAsync("SiteA", Arg.Any(), Arg.Any()) + .Returns(ci => new RouteToGetAttributesResponse( + ((RouteToGetAttributesRequest)ci[1]).CorrelationId, + new Dictionary(), false, "read failed", DateTimeOffset.UtcNow)); + + var ex = await Assert.ThrowsAsync( + () => CreateHelper().To("inst-1").GetAttributes(new[] { "a" })); + Assert.Equal("read failed", ex.Message); + } + + // --- SetAttribute(s) --- + + [Fact] + public async Task SetAttribute_DelegatesToBatch_WithSingleEntry() + { + SiteResolves("inst-1", "SiteA"); + RouteToSetAttributesRequest? captured = null; + _router.RouteToSetAttributesAsync("SiteA", Arg.Do(r => captured = r), Arg.Any()) + .Returns(ci => new RouteToSetAttributesResponse( + ((RouteToSetAttributesRequest)ci[1]).CorrelationId, true, null, DateTimeOffset.UtcNow)); + + await CreateHelper().To("inst-1").SetAttribute("setpoint", "42"); + + Assert.NotNull(captured); + Assert.Equal("42", captured!.AttributeValues["setpoint"]); + Assert.Single(captured.AttributeValues); + } + + [Fact] + public async Task SetAttributes_RemoteFailure_ThrowsInvalidOperationException() + { + SiteResolves("inst-1", "SiteA"); + _router.RouteToSetAttributesAsync("SiteA", Arg.Any(), Arg.Any()) + .Returns(ci => new RouteToSetAttributesResponse( + ((RouteToSetAttributesRequest)ci[1]).CorrelationId, false, "write rejected", DateTimeOffset.UtcNow)); + + var ex = await Assert.ThrowsAsync( + () => CreateHelper().To("inst-1").SetAttributes( + new Dictionary { ["x"] = "1" })); + Assert.Equal("write rejected", ex.Message); + } + + // --- InboundAPI-016: routed calls inherit the method deadline token --- + + [Fact] + public async Task Call_WithNoExplicitToken_InheritsMethodDeadlineToken() + { + // A natural script — Route.To("x").Call("s", p) — passes no token. The routed + // call must still be bounded by the executing method's timeout: the helper + // bound to a deadline token must forward THAT token to the router. + SiteResolves("inst-1", "SiteA"); + using var deadline = new CancellationTokenSource(); + CancellationToken seen = default; + _router.RouteToCallAsync("SiteA", Arg.Any(), Arg.Do(t => seen = t)) + .Returns(ci => new RouteToCallResponse( + ((RouteToCallRequest)ci[1]).CorrelationId, true, null, null, DateTimeOffset.UtcNow)); + + var bound = CreateHelper().WithDeadline(deadline.Token); + await bound.To("inst-1").Call("doWork"); + + Assert.Equal(deadline.Token, seen); + } + + [Fact] + public async Task Call_WhenMethodDeadlineCancelled_RoutedCallObservesCancellation() + { + // When the method timeout fires, an in-flight routed call must see the + // cancellation rather than running orphaned past the deadline. + SiteResolves("inst-1", "SiteA"); + using var deadline = new CancellationTokenSource(); + _router.RouteToCallAsync("SiteA", Arg.Any(), Arg.Any()) + .Returns(async ci => + { + var token = (CancellationToken)ci[2]; + await Task.Delay(TimeSpan.FromSeconds(30), token); + return new RouteToCallResponse( + ((RouteToCallRequest)ci[1]).CorrelationId, true, null, null, DateTimeOffset.UtcNow); + }); + + var bound = CreateHelper().WithDeadline(deadline.Token); + deadline.CancelAfter(TimeSpan.FromMilliseconds(100)); + + await Assert.ThrowsAnyAsync( + () => bound.To("inst-1").Call("doWork")); + } + + [Fact] + public async Task Call_ExplicitToken_OverridesDeadlineToken() + { + // A script that DOES pass a (tighter) token must have that token honoured. + SiteResolves("inst-1", "SiteA"); + using var deadline = new CancellationTokenSource(); + using var explicitCts = new CancellationTokenSource(); + CancellationToken seen = default; + _router.RouteToCallAsync("SiteA", Arg.Any(), Arg.Do(t => seen = t)) + .Returns(ci => new RouteToCallResponse( + ((RouteToCallRequest)ci[1]).CorrelationId, true, null, null, DateTimeOffset.UtcNow)); + + var bound = CreateHelper().WithDeadline(deadline.Token); + await bound.To("inst-1").Call("doWork", null, explicitCts.Token); + + Assert.Equal(explicitCts.Token, seen); + } + + [Fact] + public async Task GetAttributes_WithNoExplicitToken_InheritsMethodDeadlineToken() + { + SiteResolves("inst-1", "SiteA"); + using var deadline = new CancellationTokenSource(); + CancellationToken seen = default; + _router.RouteToGetAttributesAsync("SiteA", Arg.Any(), Arg.Do(t => seen = t)) + .Returns(ci => new RouteToGetAttributesResponse( + ((RouteToGetAttributesRequest)ci[1]).CorrelationId, + new Dictionary(), true, null, DateTimeOffset.UtcNow)); + + var bound = CreateHelper().WithDeadline(deadline.Token); + await bound.To("inst-1").GetAttributes(new[] { "a" }); + + Assert.Equal(deadline.Token, seen); + } + + [Fact] + public async Task SetAttributes_WithNoExplicitToken_InheritsMethodDeadlineToken() + { + SiteResolves("inst-1", "SiteA"); + using var deadline = new CancellationTokenSource(); + CancellationToken seen = default; + _router.RouteToSetAttributesAsync("SiteA", Arg.Any(), Arg.Do(t => seen = t)) + .Returns(ci => new RouteToSetAttributesResponse( + ((RouteToSetAttributesRequest)ci[1]).CorrelationId, true, null, DateTimeOffset.UtcNow)); + + var bound = CreateHelper().WithDeadline(deadline.Token); + await bound.To("inst-1").SetAttributes(new Dictionary { ["x"] = "1" }); + + Assert.Equal(deadline.Token, seen); + } +}