Merge branch 'worktree-agent-abae452487b6dcad8' into fix/flow-debugger-arch-review
This commit is contained in:
@@ -506,6 +506,23 @@ public class InboundScriptExecutor
|
|||||||
_logger.LogDebug("Inbound API request for method {Method} cancelled by client", method.Name);
|
_logger.LogDebug("Inbound API request for method {Method} cancelled by client", method.Name);
|
||||||
return new InboundScriptResult(false, null, "Request cancelled by client");
|
return new InboundScriptResult(false, null, "Request cancelled by client");
|
||||||
}
|
}
|
||||||
|
catch (RouteSiteScopeException ex)
|
||||||
|
{
|
||||||
|
// Arch-review F2: the script routed to a site outside the caller's site
|
||||||
|
// scope. RouteTarget.ResolveSiteAsync refused BEFORE any site traffic, so
|
||||||
|
// the script never saw a routed reply — there is nothing to roll back.
|
||||||
|
// The message is the operator-facing denial, safe to surface verbatim
|
||||||
|
// (it names only the instance and its site), unlike the generic
|
||||||
|
// SCRIPT_ERROR path which deliberately hides internals.
|
||||||
|
//
|
||||||
|
// Ordered before the SiteUnreachableException catch purely for
|
||||||
|
// readability: the two types are siblings under InvalidOperationException,
|
||||||
|
// so neither shadows the other.
|
||||||
|
_logger.LogWarning(
|
||||||
|
"API method {Method} refused a routed call outside the caller's site scope: {Message}",
|
||||||
|
method.Name, ex.Message);
|
||||||
|
return new InboundScriptResult(false, null, ex.Message, "SITE_SCOPE_DENIED");
|
||||||
|
}
|
||||||
catch (SiteUnreachableException)
|
catch (SiteUnreachableException)
|
||||||
{
|
{
|
||||||
// C3: a routed Route.To(...).Call(...) reached a site that could not be
|
// C3: a routed Route.To(...).Call(...) reached a site that could not be
|
||||||
|
|||||||
@@ -36,6 +36,34 @@ public sealed class SiteUnreachableException : InvalidOperationException
|
|||||||
public SiteUnreachableException(string message, Exception innerException) : base(message, innerException) { }
|
public SiteUnreachableException(string message, Exception innerException) : base(message, innerException) { }
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// Arch-review F2: a routed call whose target site is outside the caller's site
|
||||||
|
/// scope. Raised by <c>RouteTarget.ResolveSiteAsync</c> — the single choke point every routed
|
||||||
|
/// operation passes through — so it is thrown <b>before any site traffic</b>: nothing is sent,
|
||||||
|
/// nothing is read, nothing is written.
|
||||||
|
///
|
||||||
|
/// <para>
|
||||||
|
/// The scope exists for the Flow Debugger's method-entry Live run, where an operator picks a
|
||||||
|
/// method rather than a site: a site-scoped Deployer must not reach outside their sites just
|
||||||
|
/// because the method's script routes there. The ordinary HTTP inbound path attaches no scope
|
||||||
|
/// (an API key is not a site-scoped principal) and is unaffected.
|
||||||
|
/// </para>
|
||||||
|
///
|
||||||
|
/// <para>
|
||||||
|
/// Derives from <see cref="InvalidOperationException"/> so existing callers catching the base
|
||||||
|
/// type are unaffected, while <see cref="InboundScriptExecutor"/> catches this more specific
|
||||||
|
/// type to surface the <c>SITE_SCOPE_DENIED</c> error code instead of a generic
|
||||||
|
/// <c>SCRIPT_ERROR</c>. Deliberately NOT the ManagementService's <c>SiteScopeViolationException</c>
|
||||||
|
/// — InboundAPI cannot reference ManagementService; the runner converts the code back.
|
||||||
|
/// </para>
|
||||||
|
/// </summary>
|
||||||
|
public sealed class RouteSiteScopeException : InvalidOperationException
|
||||||
|
{
|
||||||
|
/// <summary>Initializes a new <see cref="RouteSiteScopeException"/> with the given message.</summary>
|
||||||
|
/// <param name="message">The scope-denial message naming the instance and its site.</param>
|
||||||
|
public RouteSiteScopeException(string message) : base(message) { }
|
||||||
|
}
|
||||||
|
|
||||||
/// <summary>
|
/// <summary>
|
||||||
/// Route.To() helper for cross-site calls from inbound API scripts.
|
/// Route.To() helper for cross-site calls from inbound API scripts.
|
||||||
/// Resolves instance to site, routes via <see cref="IInstanceRouter"/>, blocks until
|
/// Resolves instance to site, routes via <see cref="IInstanceRouter"/>, blocks until
|
||||||
@@ -57,6 +85,7 @@ public class RouteHelper
|
|||||||
private readonly INodeIdentityProvider? _nodeIdentity;
|
private readonly INodeIdentityProvider? _nodeIdentity;
|
||||||
private readonly bool _trace;
|
private readonly bool _trace;
|
||||||
private readonly Action<AuditEvent>? _traceSink;
|
private readonly Action<AuditEvent>? _traceSink;
|
||||||
|
private readonly IReadOnlySet<string>? _permittedSiteIdentifiers;
|
||||||
|
|
||||||
/// <summary>
|
/// <summary>
|
||||||
/// Initializes a new <see cref="RouteHelper"/> with no deadline token and no parent execution id.
|
/// Initializes a new <see cref="RouteHelper"/> with no deadline token and no parent execution id.
|
||||||
@@ -73,7 +102,7 @@ public class RouteHelper
|
|||||||
ICentralAuditWriter? auditWriter = null,
|
ICentralAuditWriter? auditWriter = null,
|
||||||
INodeIdentityProvider? nodeIdentity = null)
|
INodeIdentityProvider? nodeIdentity = null)
|
||||||
: this(instanceLocator, instanceRouter, CancellationToken.None, CancellationToken.None, parentExecutionId: null,
|
: this(instanceLocator, instanceRouter, CancellationToken.None, CancellationToken.None, parentExecutionId: null,
|
||||||
auditWriter, nodeIdentity, trace: false, traceSink: null)
|
auditWriter, nodeIdentity, trace: false, traceSink: null, permittedSiteIdentifiers: null)
|
||||||
{
|
{
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -86,8 +115,10 @@ public class RouteHelper
|
|||||||
ICentralAuditWriter? auditWriter,
|
ICentralAuditWriter? auditWriter,
|
||||||
INodeIdentityProvider? nodeIdentity,
|
INodeIdentityProvider? nodeIdentity,
|
||||||
bool trace,
|
bool trace,
|
||||||
Action<AuditEvent>? traceSink)
|
Action<AuditEvent>? traceSink,
|
||||||
|
IReadOnlySet<string>? permittedSiteIdentifiers)
|
||||||
{
|
{
|
||||||
|
_permittedSiteIdentifiers = permittedSiteIdentifiers;
|
||||||
_instanceLocator = instanceLocator;
|
_instanceLocator = instanceLocator;
|
||||||
_instanceRouter = instanceRouter;
|
_instanceRouter = instanceRouter;
|
||||||
_deadlineToken = deadlineToken;
|
_deadlineToken = deadlineToken;
|
||||||
@@ -110,7 +141,7 @@ public class RouteHelper
|
|||||||
/// <returns>A new <see cref="RouteHelper"/> inheriting the given deadline token.</returns>
|
/// <returns>A new <see cref="RouteHelper"/> inheriting the given deadline token.</returns>
|
||||||
public RouteHelper WithDeadline(CancellationToken deadlineToken) =>
|
public RouteHelper WithDeadline(CancellationToken deadlineToken) =>
|
||||||
new(_instanceLocator, _instanceRouter, deadlineToken, _requestAbortedToken, _parentExecutionId,
|
new(_instanceLocator, _instanceRouter, deadlineToken, _requestAbortedToken, _parentExecutionId,
|
||||||
_auditWriter, _nodeIdentity, _trace, _traceSink);
|
_auditWriter, _nodeIdentity, _trace, _traceSink, _permittedSiteIdentifiers);
|
||||||
|
|
||||||
/// <summary>
|
/// <summary>
|
||||||
/// Returns a <see cref="RouteHelper"/> carrying the raw request-abort
|
/// Returns a <see cref="RouteHelper"/> carrying the raw request-abort
|
||||||
@@ -124,7 +155,7 @@ public class RouteHelper
|
|||||||
/// <returns>A new <see cref="RouteHelper"/> carrying the given request-abort token.</returns>
|
/// <returns>A new <see cref="RouteHelper"/> carrying the given request-abort token.</returns>
|
||||||
public RouteHelper WithRequestAborted(CancellationToken requestAbortedToken) =>
|
public RouteHelper WithRequestAborted(CancellationToken requestAbortedToken) =>
|
||||||
new(_instanceLocator, _instanceRouter, _deadlineToken, requestAbortedToken, _parentExecutionId,
|
new(_instanceLocator, _instanceRouter, _deadlineToken, requestAbortedToken, _parentExecutionId,
|
||||||
_auditWriter, _nodeIdentity, _trace, _traceSink);
|
_auditWriter, _nodeIdentity, _trace, _traceSink, _permittedSiteIdentifiers);
|
||||||
|
|
||||||
/// <summary>
|
/// <summary>
|
||||||
/// Audit Log (ParentExecutionId): returns a <see cref="RouteHelper"/> whose
|
/// Audit Log (ParentExecutionId): returns a <see cref="RouteHelper"/> whose
|
||||||
@@ -139,7 +170,7 @@ public class RouteHelper
|
|||||||
/// <returns>A new <see cref="RouteHelper"/> carrying the given parent execution id.</returns>
|
/// <returns>A new <see cref="RouteHelper"/> carrying the given parent execution id.</returns>
|
||||||
public RouteHelper WithParentExecutionId(Guid? parentExecutionId) =>
|
public RouteHelper WithParentExecutionId(Guid? parentExecutionId) =>
|
||||||
new(_instanceLocator, _instanceRouter, _deadlineToken, _requestAbortedToken, parentExecutionId,
|
new(_instanceLocator, _instanceRouter, _deadlineToken, _requestAbortedToken, parentExecutionId,
|
||||||
_auditWriter, _nodeIdentity, _trace, _traceSink);
|
_auditWriter, _nodeIdentity, _trace, _traceSink, _permittedSiteIdentifiers);
|
||||||
|
|
||||||
/// <summary>
|
/// <summary>
|
||||||
/// Flow Debugger (design 2026-09-05 §5.2): returns a <see cref="RouteHelper"/> whose routed
|
/// Flow Debugger (design 2026-09-05 §5.2): returns a <see cref="RouteHelper"/> whose routed
|
||||||
@@ -150,7 +181,7 @@ public class RouteHelper
|
|||||||
/// <returns>A new <see cref="RouteHelper"/> carrying the flag.</returns>
|
/// <returns>A new <see cref="RouteHelper"/> carrying the flag.</returns>
|
||||||
public RouteHelper WithTrace(bool trace) =>
|
public RouteHelper WithTrace(bool trace) =>
|
||||||
new(_instanceLocator, _instanceRouter, _deadlineToken, _requestAbortedToken, _parentExecutionId,
|
new(_instanceLocator, _instanceRouter, _deadlineToken, _requestAbortedToken, _parentExecutionId,
|
||||||
_auditWriter, _nodeIdentity, trace, _traceSink);
|
_auditWriter, _nodeIdentity, trace, _traceSink, _permittedSiteIdentifiers);
|
||||||
|
|
||||||
/// <summary>
|
/// <summary>
|
||||||
/// Flow Debugger (design 2026-09-05 §7.1): returns a <see cref="RouteHelper"/> that also
|
/// Flow Debugger (design 2026-09-05 §7.1): returns a <see cref="RouteHelper"/> that also
|
||||||
@@ -162,7 +193,7 @@ public class RouteHelper
|
|||||||
/// <returns>A new <see cref="RouteHelper"/> carrying the sink.</returns>
|
/// <returns>A new <see cref="RouteHelper"/> carrying the sink.</returns>
|
||||||
public RouteHelper WithTraceSink(Action<AuditEvent>? traceSink) =>
|
public RouteHelper WithTraceSink(Action<AuditEvent>? traceSink) =>
|
||||||
new(_instanceLocator, _instanceRouter, _deadlineToken, _requestAbortedToken, _parentExecutionId,
|
new(_instanceLocator, _instanceRouter, _deadlineToken, _requestAbortedToken, _parentExecutionId,
|
||||||
_auditWriter, _nodeIdentity, _trace, traceSink);
|
_auditWriter, _nodeIdentity, _trace, traceSink, _permittedSiteIdentifiers);
|
||||||
|
|
||||||
/// <summary>
|
/// <summary>
|
||||||
/// Flow Debugger outbound calls (plan 2026-09-06, D5/D7): the trace sink this helper carries,
|
/// Flow Debugger outbound calls (plan 2026-09-06, D5/D7): the trace sink this helper carries,
|
||||||
@@ -172,6 +203,32 @@ public class RouteHelper
|
|||||||
/// </summary>
|
/// </summary>
|
||||||
public Action<AuditEvent>? TraceSink => _traceSink;
|
public Action<AuditEvent>? TraceSink => _traceSink;
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// Arch-review F2: returns a <see cref="RouteHelper"/> that refuses a routed call
|
||||||
|
/// whose target site is not in <paramref name="permittedSiteIdentifiers"/> — site
|
||||||
|
/// <em>identifiers</em> (e.g. <c>site-a</c>), the values <see cref="IInstanceLocator"/> returns,
|
||||||
|
/// NOT the numeric ids an <c>AuthenticatedUser.PermittedSiteIds</c> carries. Matching is
|
||||||
|
/// whatever comparer the supplied set uses, so pass an ordinal-ignore-case set.
|
||||||
|
///
|
||||||
|
/// <para>
|
||||||
|
/// <c>null</c> = unscoped, the default and the state of every ordinary inbound HTTP request.
|
||||||
|
/// An EMPTY set is not the same thing: it permits nothing, which is the correct outcome for
|
||||||
|
/// a site-scoped user whose sites no longer exist.
|
||||||
|
/// </para>
|
||||||
|
///
|
||||||
|
/// <para>
|
||||||
|
/// The Flow Debugger's method-entry Live run sets this from the operator's site-scoped
|
||||||
|
/// Deployer role, because the operator picks a <em>method</em> — the site is only known once
|
||||||
|
/// the script routes, so the check cannot happen up front the way it does for a value-change
|
||||||
|
/// run's named instance.
|
||||||
|
/// </para>
|
||||||
|
/// </summary>
|
||||||
|
/// <param name="permittedSiteIdentifiers">The site identifiers routed calls may reach; null = unscoped.</param>
|
||||||
|
/// <returns>A new <see cref="RouteHelper"/> carrying the scope.</returns>
|
||||||
|
public RouteHelper WithSiteScope(IReadOnlySet<string>? permittedSiteIdentifiers) =>
|
||||||
|
new(_instanceLocator, _instanceRouter, _deadlineToken, _requestAbortedToken, _parentExecutionId,
|
||||||
|
_auditWriter, _nodeIdentity, _trace, _traceSink, permittedSiteIdentifiers);
|
||||||
|
|
||||||
/// <summary>
|
/// <summary>
|
||||||
/// Creates a route target for the specified instance.
|
/// Creates a route target for the specified instance.
|
||||||
/// </summary>
|
/// </summary>
|
||||||
@@ -181,7 +238,7 @@ public class RouteHelper
|
|||||||
{
|
{
|
||||||
return new RouteTarget(
|
return new RouteTarget(
|
||||||
instanceCode, _instanceLocator, _instanceRouter, _deadlineToken, _requestAbortedToken, _parentExecutionId,
|
instanceCode, _instanceLocator, _instanceRouter, _deadlineToken, _requestAbortedToken, _parentExecutionId,
|
||||||
_auditWriter, _nodeIdentity, _trace, _traceSink);
|
_auditWriter, _nodeIdentity, _trace, _traceSink, _permittedSiteIdentifiers);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -200,6 +257,7 @@ public class RouteTarget
|
|||||||
private readonly INodeIdentityProvider? _nodeIdentity;
|
private readonly INodeIdentityProvider? _nodeIdentity;
|
||||||
private readonly bool _trace;
|
private readonly bool _trace;
|
||||||
private readonly Action<AuditEvent>? _traceSink;
|
private readonly Action<AuditEvent>? _traceSink;
|
||||||
|
private readonly IReadOnlySet<string>? _permittedSiteIdentifiers;
|
||||||
|
|
||||||
/// <summary>
|
/// <summary>
|
||||||
/// Initializes a new <see cref="RouteTarget"/> for the specified instance.
|
/// Initializes a new <see cref="RouteTarget"/> for the specified instance.
|
||||||
@@ -210,6 +268,11 @@ public class RouteTarget
|
|||||||
/// <param name="deadlineToken">Cancellation token representing the method-level deadline.</param>
|
/// <param name="deadlineToken">Cancellation token representing the method-level deadline.</param>
|
||||||
/// <param name="requestAbortedToken">Raw client-disconnect token, independent of the method timeout.</param>
|
/// <param name="requestAbortedToken">Raw client-disconnect token, independent of the method timeout.</param>
|
||||||
/// <param name="parentExecutionId">Optional parent execution id for audit correlation on routed calls.</param>
|
/// <param name="parentExecutionId">Optional parent execution id for audit correlation on routed calls.</param>
|
||||||
|
/// <param name="auditWriter">Central-direct-write audit writer for the always-on <c>RouteCall</c> row; null = no row.</param>
|
||||||
|
/// <param name="nodeIdentity">Stamps <c>SourceNode</c> on the <c>RouteCall</c> row; null = NULL.</param>
|
||||||
|
/// <param name="trace">True to trace the routed site run.</param>
|
||||||
|
/// <param name="traceSink">Receives the emitted audit events in order; null = none.</param>
|
||||||
|
/// <param name="permittedSiteIdentifiers">Arch-review F2: site identifiers this call may reach; null = unscoped.</param>
|
||||||
internal RouteTarget(
|
internal RouteTarget(
|
||||||
string instanceCode,
|
string instanceCode,
|
||||||
IInstanceLocator instanceLocator,
|
IInstanceLocator instanceLocator,
|
||||||
@@ -220,8 +283,10 @@ public class RouteTarget
|
|||||||
ICentralAuditWriter? auditWriter = null,
|
ICentralAuditWriter? auditWriter = null,
|
||||||
INodeIdentityProvider? nodeIdentity = null,
|
INodeIdentityProvider? nodeIdentity = null,
|
||||||
bool trace = false,
|
bool trace = false,
|
||||||
Action<AuditEvent>? traceSink = null)
|
Action<AuditEvent>? traceSink = null,
|
||||||
|
IReadOnlySet<string>? permittedSiteIdentifiers = null)
|
||||||
{
|
{
|
||||||
|
_permittedSiteIdentifiers = permittedSiteIdentifiers;
|
||||||
_instanceCode = instanceCode;
|
_instanceCode = instanceCode;
|
||||||
_instanceLocator = instanceLocator;
|
_instanceLocator = instanceLocator;
|
||||||
_instanceRouter = instanceRouter;
|
_instanceRouter = instanceRouter;
|
||||||
@@ -549,6 +614,16 @@ public class RouteTarget
|
|||||||
$"Instance '{_instanceCode}' not found or has no assigned site");
|
$"Instance '{_instanceCode}' not found or has no assigned site");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Arch-review F2: the site scope gate. This method is the single choke point —
|
||||||
|
// Call, GetAttributes, WaitForAttribute and SetAttributes all resolve the site
|
||||||
|
// here first — so refusing at this point guarantees no site traffic of any kind
|
||||||
|
// is emitted for an out-of-scope instance.
|
||||||
|
if (_permittedSiteIdentifiers is not null && !_permittedSiteIdentifiers.Contains(siteId))
|
||||||
|
{
|
||||||
|
throw new RouteSiteScopeException(
|
||||||
|
$"Access denied: instance '{_instanceCode}' is at site '{siteId}', outside your site scope.");
|
||||||
|
}
|
||||||
|
|
||||||
return siteId;
|
return siteId;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -38,6 +38,14 @@ public sealed class FlowDebugRunner
|
|||||||
/// <summary>Error code when the method / instance / site does not exist.</summary>
|
/// <summary>Error code when the method / instance / site does not exist.</summary>
|
||||||
public const string NotFoundCode = "NOT_FOUND";
|
public const string NotFoundCode = "NOT_FOUND";
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// Arch-review F2: <see cref="InboundScriptExecutor"/>'s code for a routed call refused
|
||||||
|
/// because the target site is outside the operator's site scope. InboundAPI cannot
|
||||||
|
/// reference this assembly, so it raises its own <c>RouteSiteScopeException</c> and the
|
||||||
|
/// runner converts the code back to <see cref="SiteScopeViolationException"/>.
|
||||||
|
/// </summary>
|
||||||
|
public const string SiteScopeDeniedCode = "SITE_SCOPE_DENIED";
|
||||||
|
|
||||||
/// <summary>The <c>Extra.source</c> value stamped on rows this runner synthesizes.</summary>
|
/// <summary>The <c>Extra.source</c> value stamped on rows this runner synthesizes.</summary>
|
||||||
public const string SourceTag = "flow-debugger";
|
public const string SourceTag = "flow-debugger";
|
||||||
|
|
||||||
@@ -103,7 +111,12 @@ public sealed class FlowDebugRunner
|
|||||||
var sw = Stopwatch.StartNew();
|
var sw = Stopwatch.StartNew();
|
||||||
var timeout = TimeSpan.FromSeconds(Math.Max(1, method.TimeoutSeconds));
|
var timeout = TimeSpan.FromSeconds(Math.Max(1, method.TimeoutSeconds));
|
||||||
|
|
||||||
var route = sp.GetRequiredService<RouteHelper>().WithTraceSink(events.Enqueue);
|
// Arch-review F2: a method Live run has no single target site to check up front the
|
||||||
|
// way RunValueChangeAsync does — the operator picks a METHOD, and where it routes is
|
||||||
|
// only known once the script runs. So the operator's scope is pushed into the
|
||||||
|
// RouteHelper and enforced at route time, before any site traffic.
|
||||||
|
var scope = await ResolvePermittedSiteIdentifiersAsync(sp, user, ct).ConfigureAwait(false);
|
||||||
|
var route = sp.GetRequiredService<RouteHelper>().WithTraceSink(events.Enqueue).WithSiteScope(scope);
|
||||||
var executor = sp.GetRequiredService<InboundScriptExecutor>();
|
var executor = sp.GetRequiredService<InboundScriptExecutor>();
|
||||||
|
|
||||||
InboundScriptResult result;
|
InboundScriptResult result;
|
||||||
@@ -140,6 +153,16 @@ public sealed class FlowDebugRunner
|
|||||||
extra: new ScriptTraceExtra { Source = SourceTag }.ToJson());
|
extra: new ScriptTraceExtra { Source = SourceTag }.ToJson());
|
||||||
await WriteCentralAsync(sp, inboundRow, ct).ConfigureAwait(false);
|
await WriteCentralAsync(sp, inboundRow, ct).ConfigureAwait(false);
|
||||||
|
|
||||||
|
// Arch-review F2: convert the executor's code into the same authorization failure
|
||||||
|
// RunValueChangeAsync raises, so both Live entry points surface one exception type
|
||||||
|
// (→ ManagementUnauthorized). Deliberately AFTER the inbound row is written: a
|
||||||
|
// denied attempt must still be audited against the operator's name.
|
||||||
|
if (string.Equals(result.ErrorCode, SiteScopeDeniedCode, StringComparison.Ordinal))
|
||||||
|
{
|
||||||
|
throw new SiteScopeViolationException(
|
||||||
|
result.ErrorMessage ?? "Access denied: routed call outside your site scope.");
|
||||||
|
}
|
||||||
|
|
||||||
var ordered = new List<AuditEvent>(events.Count + 1) { inboundRow };
|
var ordered = new List<AuditEvent>(events.Count + 1) { inboundRow };
|
||||||
ordered.AddRange(events.OrderBy(e => e.OccurredAtUtc).ThenBy(e => e.EventId));
|
ordered.AddRange(events.OrderBy(e => e.OccurredAtUtc).ThenBy(e => e.EventId));
|
||||||
|
|
||||||
@@ -301,6 +324,35 @@ public sealed class FlowDebugRunner
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// Arch-review F2: the operator's <c>PermittedSiteIds</c> — NUMERIC site ids as strings —
|
||||||
|
/// resolved to the site IDENTIFIERS <see cref="IInstanceLocator"/> speaks, for
|
||||||
|
/// <c>RouteHelper.WithSiteScope</c>. Returns null (unscoped) for an unscoped user or an
|
||||||
|
/// Administrator, exactly as <see cref="EnforceSiteScope"/> treats them.
|
||||||
|
///
|
||||||
|
/// <para>
|
||||||
|
/// A returned set may be EMPTY — a scoped user whose sites no longer exist routes nowhere.
|
||||||
|
/// That is the intended fail-closed outcome, and is why null and empty are not the same.
|
||||||
|
/// </para>
|
||||||
|
/// </summary>
|
||||||
|
private static async Task<IReadOnlySet<string>?> ResolvePermittedSiteIdentifiersAsync(
|
||||||
|
IServiceProvider sp, AuthenticatedUser user, CancellationToken ct)
|
||||||
|
{
|
||||||
|
if (user.PermittedSiteIds.Length == 0) return null;
|
||||||
|
if (user.Roles.Contains(Roles.Administrator, StringComparer.OrdinalIgnoreCase)) return null;
|
||||||
|
|
||||||
|
var sites = sp.GetRequiredService<ISiteRepository>();
|
||||||
|
var set = new HashSet<string>(StringComparer.OrdinalIgnoreCase);
|
||||||
|
foreach (var raw in user.PermittedSiteIds)
|
||||||
|
{
|
||||||
|
if (!int.TryParse(raw, out var id)) continue;
|
||||||
|
var site = await sites.GetSiteByIdAsync(id, ct).ConfigureAwait(false);
|
||||||
|
if (site is not null) set.Add(site.SiteIdentifier);
|
||||||
|
}
|
||||||
|
|
||||||
|
return set;
|
||||||
|
}
|
||||||
|
|
||||||
private async Task WriteCentralAsync(IServiceProvider sp, AuditEvent evt, CancellationToken ct)
|
private async Task WriteCentralAsync(IServiceProvider sp, AuditEvent evt, CancellationToken ct)
|
||||||
{
|
{
|
||||||
var writer = sp.GetService<ICentralAuditWriter>();
|
var writer = sp.GetService<ICentralAuditWriter>();
|
||||||
|
|||||||
@@ -621,4 +621,86 @@ public class RouteHelperTests
|
|||||||
await Assert.ThrowsAsync<SiteUnreachableException>(
|
await Assert.ThrowsAsync<SiteUnreachableException>(
|
||||||
() => CreateHelper().To("inst").GetAttributes(new[] { "a" }));
|
() => CreateHelper().To("inst").GetAttributes(new[] { "a" }));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// --- Arch-review F2: site scope on routed calls ---
|
||||||
|
//
|
||||||
|
// The gate lives in RouteTarget.ResolveSiteAsync — the single choke point every
|
||||||
|
// routed operation passes through — so it fires BEFORE any site traffic.
|
||||||
|
|
||||||
|
private static IReadOnlySet<string> Scope(params string[] siteIdentifiers) =>
|
||||||
|
new HashSet<string>(siteIdentifiers, StringComparer.OrdinalIgnoreCase);
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public async Task Call_OutsideSiteScope_ThrowsRouteSiteScopeException_BeforeRouting()
|
||||||
|
{
|
||||||
|
SiteResolves("inst-1", "site-b");
|
||||||
|
|
||||||
|
var helper = CreateHelper().WithSiteScope(Scope("site-a"));
|
||||||
|
|
||||||
|
var ex = await Assert.ThrowsAsync<RouteSiteScopeException>(
|
||||||
|
() => helper.To("inst-1").Call("doWork"));
|
||||||
|
Assert.Contains("site-b", ex.Message);
|
||||||
|
Assert.Contains("inst-1", ex.Message);
|
||||||
|
// Nothing was routed: the check runs before the site is contacted.
|
||||||
|
await _router.DidNotReceiveWithAnyArgs().RouteToCallAsync(default!, default!, default);
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public async Task Call_InsideSiteScope_Routes()
|
||||||
|
{
|
||||||
|
SiteResolves("inst-1", "site-b");
|
||||||
|
_router.RouteToCallAsync("site-b", Arg.Any<RouteToCallRequest>(), Arg.Any<CancellationToken>())
|
||||||
|
.Returns(ci => new RouteToCallResponse(
|
||||||
|
((RouteToCallRequest)ci[1]).CorrelationId, true, 42, null, DateTimeOffset.UtcNow));
|
||||||
|
|
||||||
|
var helper = CreateHelper().WithSiteScope(Scope("SITE-B"));
|
||||||
|
|
||||||
|
Assert.Equal(42, await helper.To("inst-1").Call("doWork"));
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public async Task Call_NullScope_IsUnscoped()
|
||||||
|
{
|
||||||
|
SiteResolves("inst-1", "site-b");
|
||||||
|
_router.RouteToCallAsync("site-b", Arg.Any<RouteToCallRequest>(), Arg.Any<CancellationToken>())
|
||||||
|
.Returns(ci => new RouteToCallResponse(
|
||||||
|
((RouteToCallRequest)ci[1]).CorrelationId, true, 42, null, DateTimeOffset.UtcNow));
|
||||||
|
|
||||||
|
var helper = CreateHelper().WithSiteScope(null);
|
||||||
|
|
||||||
|
Assert.Equal(42, await helper.To("inst-1").Call("doWork"));
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public async Task SetAttributes_OutsideSiteScope_ThrowsRouteSiteScopeException_BeforeRouting()
|
||||||
|
{
|
||||||
|
// ResolveSiteAsync is shared by all four routed operations, so the gate is
|
||||||
|
// not Call-specific.
|
||||||
|
SiteResolves("inst-1", "site-b");
|
||||||
|
|
||||||
|
var helper = CreateHelper().WithSiteScope(Scope("site-a"));
|
||||||
|
|
||||||
|
await Assert.ThrowsAsync<RouteSiteScopeException>(
|
||||||
|
() => helper.To("inst-1").SetAttributes(new Dictionary<string, string> { ["x"] = "1" }));
|
||||||
|
await _router.DidNotReceiveWithAnyArgs().RouteToSetAttributesAsync(default!, default!, default);
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public async Task WithSiteScope_SurvivesTheOtherWithCopies()
|
||||||
|
{
|
||||||
|
// The scope must not be dropped by the copies InboundScriptExecutor makes
|
||||||
|
// (WithDeadline / WithRequestAborted / WithParentExecutionId / WithTrace).
|
||||||
|
SiteResolves("inst-1", "site-b");
|
||||||
|
|
||||||
|
var helper = CreateHelper()
|
||||||
|
.WithSiteScope(Scope("site-a"))
|
||||||
|
.WithDeadline(CancellationToken.None)
|
||||||
|
.WithRequestAborted(CancellationToken.None)
|
||||||
|
.WithParentExecutionId(Guid.NewGuid())
|
||||||
|
.WithTrace(true)
|
||||||
|
.WithTraceSink(null);
|
||||||
|
|
||||||
|
await Assert.ThrowsAsync<RouteSiteScopeException>(
|
||||||
|
() => helper.To("inst-1").Call("doWork"));
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -233,6 +233,87 @@ public class FlowDebugRunnerTests
|
|||||||
Assert.Equal(FlowDebugRunner.NotFoundCode, empty.ErrorCode);
|
Assert.Equal(FlowDebugRunner.NotFoundCode, empty.ErrorCode);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// --- Arch-review F2: a method-entry Live run is bound by the operator's site scope ---
|
||||||
|
//
|
||||||
|
// RunValueChange already called EnforceSiteScope on the target instance; a method
|
||||||
|
// Live run has no single target site up front, so the scope is pushed into the
|
||||||
|
// RouteHelper and enforced at route time (RouteTarget.ResolveSiteAsync), before any
|
||||||
|
// site traffic. The denial comes back as SITE_SCOPE_DENIED and the runner converts it.
|
||||||
|
|
||||||
|
/// <summary>Wires a method whose script routes to <paramref name="instanceCode"/>, resident at <paramref name="siteIdentifier"/>.</summary>
|
||||||
|
private ISiteRepository WireRoutedMethod(int methodId, string instanceCode, string siteIdentifier)
|
||||||
|
{
|
||||||
|
var method = new ApiMethod("M", "x") { Id = methodId, TimeoutSeconds = 5 };
|
||||||
|
_apiRepo.GetApiMethodByIdAsync(methodId, Arg.Any<CancellationToken>()).Returns(method);
|
||||||
|
_locator.GetSiteIdForInstanceAsync(instanceCode, Arg.Any<CancellationToken>()).Returns(siteIdentifier);
|
||||||
|
_router.RouteToCallAsync(siteIdentifier, Arg.Any<RouteToCallRequest>(), Arg.Any<CancellationToken>())
|
||||||
|
.Returns(ci => new RouteToCallResponse(
|
||||||
|
((RouteToCallRequest)ci[1]).CorrelationId, true, 7, null, DateTimeOffset.UtcNow));
|
||||||
|
_executor.RegisterHandler("M", ctx => ctx.Route.To(instanceCode).Call("work"));
|
||||||
|
|
||||||
|
// The user's PermittedSiteIds are NUMERIC ids; the locator speaks site IDENTIFIERS,
|
||||||
|
// so the runner resolves one to the other through the repository.
|
||||||
|
var sites = Substitute.For<ISiteRepository>();
|
||||||
|
sites.GetSiteByIdAsync(1, Arg.Any<CancellationToken>()).Returns(new Site("Site A", "site-a") { Id = 1 });
|
||||||
|
_services.AddSingleton(sites);
|
||||||
|
return sites;
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public async Task RunMethod_ScopedDeployer_RouteToOtherSite_IsUnauthorized_AndInboundRowIsWritten()
|
||||||
|
{
|
||||||
|
WireRoutedMethod(11, "inst", "site-b");
|
||||||
|
var scoped = new AuthenticatedUser("ops", "Ops", new[] { "Deployer" }, new[] { "1" });
|
||||||
|
|
||||||
|
await Assert.ThrowsAsync<SiteScopeViolationException>(() => Runner().RunMethodAsync(
|
||||||
|
_services.BuildServiceProvider(), new FlowDebugRunMethodCommand(11, null), scoped, CancellationToken.None));
|
||||||
|
|
||||||
|
// The site was never contacted...
|
||||||
|
await _router.DidNotReceiveWithAnyArgs().RouteToCallAsync(default!, default!, default);
|
||||||
|
// ...but the denied attempt IS recorded against the operator's name.
|
||||||
|
Assert.Contains(_central, e => e.AsRow().Kind == AuditKind.InboundRequest && e.Actor == "ops");
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public async Task RunMethod_ScopedDeployer_RouteWithinScope_Runs()
|
||||||
|
{
|
||||||
|
// The same scoped user, routing to a site their numeric id DOES resolve to.
|
||||||
|
WireRoutedMethod(12, "inst", "site-a");
|
||||||
|
var scoped = new AuthenticatedUser("ops", "Ops", new[] { "Deployer" }, new[] { "1" });
|
||||||
|
|
||||||
|
var result = await Runner().RunMethodAsync(
|
||||||
|
_services.BuildServiceProvider(), new FlowDebugRunMethodCommand(12, null), scoped, CancellationToken.None);
|
||||||
|
|
||||||
|
Assert.True(result.Completed, result.Error);
|
||||||
|
await _router.ReceivedWithAnyArgs(1).RouteToCallAsync(default!, default!, default);
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public async Task RunMethod_Administrator_IsNeverScoped()
|
||||||
|
{
|
||||||
|
WireRoutedMethod(13, "inst", "site-b");
|
||||||
|
var admin = new AuthenticatedUser("boss", "Boss", new[] { "Deployer", "Administrator" }, new[] { "1" });
|
||||||
|
|
||||||
|
var result = await Runner().RunMethodAsync(
|
||||||
|
_services.BuildServiceProvider(), new FlowDebugRunMethodCommand(13, null), admin, CancellationToken.None);
|
||||||
|
|
||||||
|
Assert.True(result.Completed, result.Error);
|
||||||
|
await _router.ReceivedWithAnyArgs(1).RouteToCallAsync(default!, default!, default);
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public async Task RunMethod_UnscopedUser_IsNeverScoped()
|
||||||
|
{
|
||||||
|
WireRoutedMethod(14, "inst", "site-b");
|
||||||
|
|
||||||
|
// Deployer with an EMPTY PermittedSiteIds = global scope, as EnforceSiteScope treats it.
|
||||||
|
var result = await Runner().RunMethodAsync(
|
||||||
|
_services.BuildServiceProvider(), new FlowDebugRunMethodCommand(14, null), Deployer, CancellationToken.None);
|
||||||
|
|
||||||
|
Assert.True(result.Completed, result.Error);
|
||||||
|
await _router.ReceivedWithAnyArgs(1).RouteToCallAsync(default!, default!, default);
|
||||||
|
}
|
||||||
|
|
||||||
[Fact]
|
[Fact]
|
||||||
public void RoleGate_TheTwoRunCommands_AreDeployerOnly()
|
public void RoleGate_TheTwoRunCommands_AreDeployerOnly()
|
||||||
{
|
{
|
||||||
|
|||||||
Reference in New Issue
Block a user