fix(security): import trust gate covers instance alarm-override trigger expressions — 5th call site is surface-complete (plan R2-05 T6)
This commit is contained in:
@@ -946,8 +946,11 @@ public sealed class BundleImporter : IBundleImporter
|
||||
/// <summary>
|
||||
/// Enumerates every executable C# surface a bundle carries that the
|
||||
/// trust gate must vet: non-Skip template scripts + their Expression-trigger
|
||||
/// bodies, template alarm Expression-trigger bodies, shared scripts, and
|
||||
/// ApiMethod scripts. Expression triggers compile and execute at the site
|
||||
/// bodies, template alarm Expression-trigger bodies, shared scripts,
|
||||
/// ApiMethod scripts, AND instance alarm-override trigger expressions
|
||||
/// (<c>InstanceAlarmOverrideDto.TriggerConfigurationOverride</c>). Expression
|
||||
/// triggers — template alarms AND the instance overrides that replace them in
|
||||
/// the flattened config — compile and execute at the site
|
||||
/// (`TriggerExpressionGlobals`), so they are a genuine trust surface, not just
|
||||
/// script bodies. A null <paramref name="resolutionMap"/> (preview time, before
|
||||
/// resolutions are chosen) gates everything. Yields <c>(kind, entityName,
|
||||
@@ -998,6 +1001,25 @@ public sealed class BundleImporter : IBundleImporter
|
||||
yield return ("ApiMethod", m.Name, m.Name, m.Script);
|
||||
}
|
||||
}
|
||||
foreach (var i in content.Instances)
|
||||
{
|
||||
if (IsSkipResolution(resolutionMap, "Instance", i.UniqueName)) continue;
|
||||
foreach (var o in i.AlarmOverrides)
|
||||
{
|
||||
// The DTO carries no trigger type (the type lives on the template
|
||||
// alarm), so gate ANY override config carrying an {"expression":...}
|
||||
// string body: if the overridden alarm is not Expression-triggered
|
||||
// the body never executes, but vetting it anyway is fail-safe — the
|
||||
// structured (HiLo/threshold) configs have no "expression" key, so
|
||||
// there is no false-positive channel.
|
||||
var expr = ExtractExpressionBody(o.TriggerConfigurationOverride);
|
||||
if (!string.IsNullOrEmpty(expr))
|
||||
{
|
||||
yield return ("Instance", i.UniqueName,
|
||||
$"{o.AlarmCanonicalName} (alarm trigger override expression)", expr);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
@@ -1009,8 +1031,24 @@ public sealed class BundleImporter : IBundleImporter
|
||||
private static string? ExtractTriggerExpression(string? triggerType, string? triggerConfigJson)
|
||||
{
|
||||
if (triggerType is null
|
||||
|| !triggerType.Equals("Expression", StringComparison.OrdinalIgnoreCase)
|
||||
|| string.IsNullOrWhiteSpace(triggerConfigJson))
|
||||
|| !triggerType.Equals("Expression", StringComparison.OrdinalIgnoreCase))
|
||||
{
|
||||
return null;
|
||||
}
|
||||
return ExtractExpressionBody(triggerConfigJson);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Extracts the C# boolean expression from an <c>{"expression":"…"}</c> trigger
|
||||
/// configuration, WITHOUT a trigger-type guard — used where the caller has no
|
||||
/// trigger type to check (an instance alarm override; the type lives on the
|
||||
/// template alarm). A config carrying no <c>"expression"</c> string key (the
|
||||
/// structured HiLo/threshold shapes) or malformed JSON yields null — nothing to
|
||||
/// gate.
|
||||
/// </summary>
|
||||
private static string? ExtractExpressionBody(string? triggerConfigJson)
|
||||
{
|
||||
if (string.IsNullOrWhiteSpace(triggerConfigJson))
|
||||
{
|
||||
return null;
|
||||
}
|
||||
|
||||
+179
@@ -2,6 +2,8 @@ using Microsoft.EntityFrameworkCore;
|
||||
using Microsoft.EntityFrameworkCore.Diagnostics;
|
||||
using Microsoft.Extensions.Configuration;
|
||||
using Microsoft.Extensions.DependencyInjection;
|
||||
using ZB.MOM.WW.ScadaBridge.Commons.Entities.Instances;
|
||||
using ZB.MOM.WW.ScadaBridge.Commons.Entities.Sites;
|
||||
using ZB.MOM.WW.ScadaBridge.Commons.Entities.Templates;
|
||||
using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Repositories;
|
||||
using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Services;
|
||||
@@ -122,6 +124,78 @@ public sealed class SemanticValidatorImportTests : IDisposable
|
||||
return sessionId;
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Seeds a template + site + instance whose single alarm override carries the
|
||||
/// given <paramref name="triggerConfigJson"/> as its
|
||||
/// <c>TriggerConfigurationOverride</c>, so an export→load carries the instance
|
||||
/// alarm-override trigger expression through the trust gate.
|
||||
/// </summary>
|
||||
private async Task SeedInstanceAlarmOverrideAsync(string triggerConfigJson)
|
||||
{
|
||||
await using var scope = _provider.CreateAsyncScope();
|
||||
var ctx = scope.ServiceProvider.GetRequiredService<ScadaBridgeDbContext>();
|
||||
|
||||
var template = new Template("Pump") { Description = "pump tpl" };
|
||||
ctx.Templates.Add(template);
|
||||
|
||||
var site = new Site("Plant 1", "plant-1")
|
||||
{
|
||||
NodeAAddress = "akka://site@10.0.0.1:2552",
|
||||
NodeBAddress = "akka://site@10.0.0.2:2552",
|
||||
GrpcNodeAAddress = "10.0.0.1:8083",
|
||||
GrpcNodeBAddress = "10.0.0.2:8083",
|
||||
};
|
||||
ctx.Sites.Add(site);
|
||||
await ctx.SaveChangesAsync();
|
||||
|
||||
var instance = new Instance("Pump-01")
|
||||
{
|
||||
TemplateId = template.Id,
|
||||
SiteId = site.Id,
|
||||
State = InstanceState.Enabled,
|
||||
};
|
||||
instance.AlarmOverrides.Add(new InstanceAlarmOverride("HiAlarm")
|
||||
{
|
||||
TriggerConfigurationOverride = triggerConfigJson,
|
||||
});
|
||||
ctx.Instances.Add(instance);
|
||||
await ctx.SaveChangesAsync();
|
||||
}
|
||||
|
||||
/// <summary>Exports every seeded site (+ template) closure into a bundle and loads it.</summary>
|
||||
private async Task<Guid> ExportSitesAndLoadAsync()
|
||||
{
|
||||
byte[] bundleBytes;
|
||||
await using (var scope = _provider.CreateAsyncScope())
|
||||
{
|
||||
var exporter = scope.ServiceProvider.GetRequiredService<IBundleExporter>();
|
||||
var ctx = scope.ServiceProvider.GetRequiredService<ScadaBridgeDbContext>();
|
||||
var siteIds = await ctx.Sites.Select(s => s.Id).ToListAsync();
|
||||
var selection = new ExportSelection(
|
||||
TemplateIds: await ctx.Templates.Select(t => t.Id).ToListAsync(),
|
||||
SharedScriptIds: Array.Empty<int>(),
|
||||
ExternalSystemIds: Array.Empty<int>(),
|
||||
DatabaseConnectionIds: Array.Empty<int>(),
|
||||
NotificationListIds: Array.Empty<int>(),
|
||||
SmtpConfigurationIds: Array.Empty<int>(),
|
||||
ApiMethodIds: Array.Empty<int>(),
|
||||
IncludeDependencies: true,
|
||||
SiteIds: siteIds);
|
||||
var stream = await exporter.ExportAsync(selection, user: "alice", sourceEnvironment: "dev",
|
||||
passphrase: null, cancellationToken: CancellationToken.None);
|
||||
using var ms = new MemoryStream();
|
||||
await stream.CopyToAsync(ms);
|
||||
bundleBytes = ms.ToArray();
|
||||
}
|
||||
|
||||
await using (var scope = _provider.CreateAsyncScope())
|
||||
{
|
||||
var importer = scope.ServiceProvider.GetRequiredService<IBundleImporter>();
|
||||
using var input = new MemoryStream(bundleBytes, writable: false);
|
||||
return (await importer.LoadAsync(input, passphrase: null)).SessionId;
|
||||
}
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task TemplateScript_with_unresolved_reference_warns_but_does_not_block_import()
|
||||
{
|
||||
@@ -406,6 +480,111 @@ public sealed class SemanticValidatorImportTests : IDisposable
|
||||
Assert.Contains(ex.Errors, err => err.Contains("Process", StringComparison.Ordinal));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task Apply_InstanceAlarmOverrideExpressionWithForbiddenApi_HardBlocks()
|
||||
{
|
||||
// N5 — an instance alarm override's TriggerConfigurationOverride replaces the
|
||||
// template alarm's trigger config in the flattened config and compiles/executes
|
||||
// at the site, so a forbidden API in it must be caught at import review — the
|
||||
// same hard-block severity as every other trust-gate surface.
|
||||
await SeedInstanceAlarmOverrideAsync(
|
||||
"{\"expression\":\"System.Diagnostics.Process.GetProcesses().Length > 0\"}");
|
||||
|
||||
var sessionId = await ExportSitesAndLoadAsync();
|
||||
|
||||
SemanticValidationException ex = default!;
|
||||
await using (var scope = _provider.CreateAsyncScope())
|
||||
{
|
||||
var importer = scope.ServiceProvider.GetRequiredService<IBundleImporter>();
|
||||
ex = await Assert.ThrowsAsync<SemanticValidationException>(() =>
|
||||
importer.ApplyAsync(sessionId,
|
||||
new List<ImportResolution>
|
||||
{
|
||||
new("Template", "Pump", ResolutionAction.Add, null),
|
||||
new("Site", "plant-1", ResolutionAction.Add, null),
|
||||
new("Instance", "Pump-01", ResolutionAction.Add, null),
|
||||
},
|
||||
user: "bob"));
|
||||
}
|
||||
|
||||
Assert.NotEmpty(ex.Errors);
|
||||
Assert.Contains(ex.Errors, e =>
|
||||
e.Contains("Process", StringComparison.Ordinal)
|
||||
&& e.Contains("trigger override", StringComparison.Ordinal));
|
||||
|
||||
// Rollback contract: nothing from THIS import was written (the seed rows
|
||||
// pre-exist, but no new import ran — the throw is pre-write at Pass 0).
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task Preview_InstanceAlarmOverrideExpressionWithForbiddenApi_SurfacesBlocker()
|
||||
{
|
||||
await SeedInstanceAlarmOverrideAsync(
|
||||
"{\"expression\":\"System.Diagnostics.Process.GetProcesses().Length > 0\"}");
|
||||
|
||||
var sessionId = await ExportSitesAndLoadAsync();
|
||||
|
||||
ImportPreview preview;
|
||||
await using (var scope = _provider.CreateAsyncScope())
|
||||
{
|
||||
var importer = scope.ServiceProvider.GetRequiredService<IBundleImporter>();
|
||||
preview = await importer.PreviewAsync(sessionId);
|
||||
}
|
||||
|
||||
Assert.Contains(preview.Items, b =>
|
||||
b.Kind == ConflictKind.Blocker && b.EntityType == "Instance"
|
||||
&& b.BlockerReason!.Contains("trust violation", StringComparison.Ordinal));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task Apply_InstanceAlarmOverrideExpression_Clean_Imports()
|
||||
{
|
||||
// A clean expression against the trigger surface imports without hard-block;
|
||||
// the override row is persisted verbatim (bundle fidelity).
|
||||
await SeedInstanceAlarmOverrideAsync(
|
||||
"{\"expression\":\"Attributes[\\\"Temp\\\"] != null\"}");
|
||||
|
||||
var sessionId = await ExportSitesAndLoadAsync();
|
||||
|
||||
// Wipe so the apply exercises a create; re-create the template the instance binds to.
|
||||
await using (var scope = _provider.CreateAsyncScope())
|
||||
{
|
||||
var ctx = scope.ServiceProvider.GetRequiredService<ScadaBridgeDbContext>();
|
||||
ctx.InstanceAlarmOverrides.RemoveRange(ctx.InstanceAlarmOverrides);
|
||||
ctx.Instances.RemoveRange(ctx.Instances);
|
||||
ctx.Sites.RemoveRange(ctx.Sites);
|
||||
ctx.Templates.RemoveRange(ctx.Templates);
|
||||
await ctx.SaveChangesAsync();
|
||||
ctx.Templates.Add(new Template("Pump") { Description = "pump tpl" });
|
||||
await ctx.SaveChangesAsync();
|
||||
}
|
||||
|
||||
var nameMap = new BundleNameMap(
|
||||
Sites: new[] { new SiteMapping("plant-1", MappingAction.CreateNew, null) },
|
||||
Connections: Array.Empty<ConnectionMapping>());
|
||||
|
||||
await using (var scope = _provider.CreateAsyncScope())
|
||||
{
|
||||
var importer = scope.ServiceProvider.GetRequiredService<IBundleImporter>();
|
||||
await importer.ApplyAsync(sessionId,
|
||||
new List<ImportResolution>
|
||||
{
|
||||
new("Template", "Pump", ResolutionAction.Skip, null),
|
||||
new("Site", "plant-1", ResolutionAction.Add, null),
|
||||
new("Instance", "Pump-01", ResolutionAction.Add, null),
|
||||
},
|
||||
user: "bob", ct: CancellationToken.None, nameMap: nameMap);
|
||||
}
|
||||
|
||||
await using (var verify = _provider.CreateAsyncScope())
|
||||
{
|
||||
var ctx = verify.ServiceProvider.GetRequiredService<ScadaBridgeDbContext>();
|
||||
var ovr = await ctx.InstanceAlarmOverrides.SingleAsync();
|
||||
Assert.Equal("HiAlarm", ovr.AlarmCanonicalName);
|
||||
Assert.Contains("Attributes", ovr.TriggerConfigurationOverride!);
|
||||
}
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task Apply_ApiMethodReference_NotSuppressedByTemplateLocalFunction()
|
||||
{
|
||||
|
||||
Reference in New Issue
Block a user