Make the server authoritative for what endpoints watch and launch
Fleet health showed dncMain.exe red on endpoints that never ran eDNC. Three
layers each re-added the monitor, so fixing any one of them in isolation left
the others to put it back.
The installer baked a watch list into appsettings.json on every package, and
appsettings.json is never rewritten on the box, so an entry outlived whatever
policy justified it. The server auto-attached shipped monitors by PC type name.
The agent then UNIONED the server's resolved list with its local one, so an
entry could never be removed from the server side at all.
Now: the installer seeds nothing, auto-attach by PC type is gone, and
MonitorConfig.Merge replaces the local list with the server's. Null and empty
stay distinct - null means the fetch failed so keep what we have, empty means
watch nothing. Collapsing those blanks monitoring fleet-wide on one failed HTTP
call, or makes detaching the last monitor silently do nothing forever.
Ingest filters reported services against the resolved monitors as well. The
agent config sits on the endpoint, so it is not a trustworthy account of what
that endpoint should be reporting.
Launch paths move the same way, which exposed an older bug. MTConnect installs
to ProgramData and starts through run.bat, but LaunchPathPolicy approved only
the Program Files roots, so the launchpath on its shipped monitor row - seeded
in 0006, present from the start - was rejected every time as outside every
approved root. That was invisible because a local launchpaths entry wins in
ResolveLaunchPath and never reaches validation; remote start worked only
through the bypass. The same precedence meant editing an eDNC, UDC or MTConnect
path in the WebUI could never reach an existing endpoint.
So ProgramData is approved and the local launch map ships empty. ProgramData is
world-writable and the agent launches as SYSTEM, which is why it was excluded
originally; it is deliberately too broad for now and carries a REVISIT comment.
The remaining gates still hold - admin-configured monitor row, exists,
launchable extension, re-checked after symlink resolution. Narrowing it to
ProgramData\udc needs the fleet's non-Program-Files install locations
confirmed first.
Also seeds the Office ProPlus LTSC monitor templates, x64 and x86 as separate
rows since they differ only by path, and adds an agent test project. It links
the platform-pure sources rather than referencing the net8.0-windows assembly
so the semantics run on the Linux CI runners. Writing those tests caught a
merge bug where only the dedupe key was trimmed and the stored name was not.
Note for the wj merge: plugins/usb/frontend/views/usbSettings.spec.js fails on
this branch and is untouched here. Commit da91a29 added four kiosk settings the
save assertion does not expect. The spec looks like the stale side, but that is
the usb author's call - flagging rather than guessing.
This commit is contained in:
benjamin.baker
312
agent/ShopDbEndpointAgent.Core.Tests/LaunchPathPolicyTests.cs
Normal file
312
agent/ShopDbEndpointAgent.Core.Tests/LaunchPathPolicyTests.cs
Normal file
@@ -0,0 +1,312 @@
|
||||
using ShopDbEndpointAgent.Core;
|
||||
using Xunit;
|
||||
|
||||
namespace ShopDbEndpointAgent.Core.Tests;
|
||||
|
||||
// Pins LaunchPathPolicy, the safeguard that lets the agent honor a launch path
|
||||
// chosen in the WebUI without turning "write access to a file" into "remote code
|
||||
// execution as LocalSystem".
|
||||
//
|
||||
// These tests build a real directory tree under the temp dir and point the
|
||||
// policy at it via the approved-roots override, rather than asserting against
|
||||
// hardcoded C:\Program Files strings. That keeps them honest about the checks
|
||||
// (which touch the disk: File.Exists, link resolution) while staying runnable on
|
||||
// the Linux CI runners. Cases that genuinely require Windows semantics, notably
|
||||
// NTFS alternate data streams and junctions, are called out where they appear.
|
||||
public sealed class LaunchPathPolicyTests : IDisposable
|
||||
{
|
||||
private readonly string root;
|
||||
|
||||
public LaunchPathPolicyTests()
|
||||
{
|
||||
root = Path.Combine(Path.GetTempPath(),
|
||||
"shopdb-launchpolicy-" + Guid.NewGuid().ToString("n"));
|
||||
Directory.CreateDirectory(root);
|
||||
}
|
||||
|
||||
public void Dispose()
|
||||
{
|
||||
try { Directory.Delete(root, recursive: true); }
|
||||
catch (IOException) { }
|
||||
catch (UnauthorizedAccessException) { }
|
||||
}
|
||||
|
||||
private LaunchPathPolicy PolicyForRoot()
|
||||
{
|
||||
return new LaunchPathPolicy(new[] { root });
|
||||
}
|
||||
|
||||
private string CreateFile(string relativePath)
|
||||
{
|
||||
var full = Path.Combine(root, relativePath);
|
||||
Directory.CreateDirectory(Path.GetDirectoryName(full)!);
|
||||
File.WriteAllText(full, "");
|
||||
return full;
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void An_approved_executable_under_an_approved_root_is_accepted()
|
||||
{
|
||||
var target = CreateFile(Path.Combine("UDC", "udc.exe"));
|
||||
|
||||
var accepted = PolicyForRoot().TryValidate(target, out var fullPath, out var reason);
|
||||
|
||||
Assert.True(accepted, reason);
|
||||
Assert.Equal(target, fullPath);
|
||||
}
|
||||
|
||||
[Theory]
|
||||
[InlineData("udc.exe")]
|
||||
[InlineData("run.bat")]
|
||||
[InlineData("run.cmd")]
|
||||
public void Every_launchable_extension_is_accepted(string fileName)
|
||||
{
|
||||
// .bat and .cmd are deliberate: some shop-floor apps ship a launcher
|
||||
// script rather than an exe.
|
||||
var target = CreateFile(fileName);
|
||||
|
||||
var accepted = PolicyForRoot().TryValidate(target, out _, out var reason);
|
||||
|
||||
Assert.True(accepted, reason);
|
||||
}
|
||||
|
||||
[Theory]
|
||||
[InlineData("payload.dll")]
|
||||
[InlineData("payload.ps1")]
|
||||
[InlineData("readme.txt")]
|
||||
[InlineData("payload.vbs")]
|
||||
public void A_non_launchable_extension_is_rejected(string fileName)
|
||||
{
|
||||
var target = CreateFile(fileName);
|
||||
|
||||
var accepted = PolicyForRoot().TryValidate(target, out _, out var reason);
|
||||
|
||||
Assert.False(accepted);
|
||||
Assert.Contains("not launchable", reason);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void A_path_outside_every_approved_root_is_rejected()
|
||||
{
|
||||
var outside = Path.Combine(Path.GetTempPath(),
|
||||
"shopdb-outside-" + Guid.NewGuid().ToString("n"));
|
||||
Directory.CreateDirectory(outside);
|
||||
try
|
||||
{
|
||||
var target = Path.Combine(outside, "evil.exe");
|
||||
File.WriteAllText(target, "");
|
||||
|
||||
var accepted = PolicyForRoot().TryValidate(target, out _, out var reason);
|
||||
|
||||
Assert.False(accepted);
|
||||
Assert.Contains("outside every approved launch root", reason);
|
||||
}
|
||||
finally
|
||||
{
|
||||
Directory.Delete(outside, recursive: true);
|
||||
}
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void A_sibling_root_with_a_shared_prefix_is_rejected()
|
||||
{
|
||||
// Guards the root comparison: "<root>" must not also match
|
||||
// "<root>Evil", which a naive StartsWith without a separator allows.
|
||||
var sibling = root + "Evil";
|
||||
Directory.CreateDirectory(sibling);
|
||||
try
|
||||
{
|
||||
var target = Path.Combine(sibling, "evil.exe");
|
||||
File.WriteAllText(target, "");
|
||||
|
||||
var accepted = PolicyForRoot().TryValidate(target, out _, out var reason);
|
||||
|
||||
Assert.False(accepted);
|
||||
Assert.Contains("outside every approved launch root", reason);
|
||||
}
|
||||
finally
|
||||
{
|
||||
Directory.Delete(sibling, recursive: true);
|
||||
}
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void A_traversal_escaping_an_approved_root_is_rejected()
|
||||
{
|
||||
// GetFullPath normalizes the "..", so the check sees where the path
|
||||
// actually lands rather than the approved-looking prefix it started
|
||||
// with.
|
||||
var target = Path.Combine(root, "..", "escaped.exe");
|
||||
|
||||
var accepted = PolicyForRoot().TryValidate(target, out _, out var reason);
|
||||
|
||||
Assert.False(accepted);
|
||||
Assert.Contains("outside every approved launch root", reason);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void A_path_that_does_not_exist_is_rejected()
|
||||
{
|
||||
var target = Path.Combine(root, "missing.exe");
|
||||
|
||||
var accepted = PolicyForRoot().TryValidate(target, out _, out var reason);
|
||||
|
||||
Assert.False(accepted);
|
||||
Assert.Contains("does not exist", reason);
|
||||
}
|
||||
|
||||
[Theory]
|
||||
[InlineData(null)]
|
||||
[InlineData("")]
|
||||
[InlineData(" ")]
|
||||
public void An_empty_candidate_is_rejected(string? candidate)
|
||||
{
|
||||
// This is the common case, not an edge case: most monitors carry no
|
||||
// launch path at all, and the caller must fail closed rather than
|
||||
// treating "no path" as "any path".
|
||||
var accepted = PolicyForRoot().TryValidate(candidate, out _, out var reason);
|
||||
|
||||
Assert.False(accepted);
|
||||
Assert.Equal("empty path", reason);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void A_rejected_path_never_reports_a_usable_full_path()
|
||||
{
|
||||
// The caller keys off the bool but passes fullPath onward, so a
|
||||
// rejection must not leave anything launchable in it.
|
||||
var accepted = PolicyForRoot().TryValidate(
|
||||
Path.Combine(root, "missing.exe"), out var fullPath, out _);
|
||||
|
||||
Assert.False(accepted);
|
||||
Assert.Equal("", fullPath);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void An_alternate_data_stream_is_rejected()
|
||||
{
|
||||
// Windows-only semantics. On net8.0 GetFullPath no longer throws on an
|
||||
// interior colon, so "readme.txt:payload.exe" would otherwise report an
|
||||
// .exe extension, pass the root check, and exist as a stream.
|
||||
if (!OperatingSystem.IsWindows())
|
||||
return;
|
||||
|
||||
CreateFile("readme.txt");
|
||||
var target = Path.Combine(root, "readme.txt:payload.exe");
|
||||
|
||||
var accepted = PolicyForRoot().TryValidate(target, out _, out var reason);
|
||||
|
||||
Assert.False(accepted);
|
||||
Assert.Contains("alternate data stream", reason);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void A_symlink_pointing_outside_an_approved_root_is_rejected()
|
||||
{
|
||||
// The lexical checks constrain the NAME; this pins that the policy also
|
||||
// resolves what the name actually refers to. Creating a link needs no
|
||||
// privilege beyond write access to the parent, and vendor install trees
|
||||
// under Program Files are not reliably locked down.
|
||||
var outside = Path.Combine(Path.GetTempPath(),
|
||||
"shopdb-linktarget-" + Guid.NewGuid().ToString("n"));
|
||||
Directory.CreateDirectory(outside);
|
||||
try
|
||||
{
|
||||
var realTarget = Path.Combine(outside, "payload.exe");
|
||||
File.WriteAllText(realTarget, "");
|
||||
|
||||
var link = Path.Combine(root, "innocent.exe");
|
||||
try
|
||||
{
|
||||
File.CreateSymbolicLink(link, realTarget);
|
||||
}
|
||||
catch (UnauthorizedAccessException)
|
||||
{
|
||||
return;
|
||||
}
|
||||
catch (IOException)
|
||||
{
|
||||
return;
|
||||
}
|
||||
|
||||
var accepted = PolicyForRoot().TryValidate(link, out _, out var reason);
|
||||
|
||||
Assert.False(accepted);
|
||||
Assert.Contains("resolves outside every approved launch root", reason);
|
||||
}
|
||||
finally
|
||||
{
|
||||
Directory.Delete(outside, recursive: true);
|
||||
}
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void An_empty_roots_override_falls_back_to_the_shipped_defaults()
|
||||
{
|
||||
// A site that configures no roots must not end up with an empty
|
||||
// allowlist, which would fail every launch closed.
|
||||
var policy = new LaunchPathPolicy(Array.Empty<string>());
|
||||
var target = CreateFile("udc.exe");
|
||||
|
||||
var accepted = policy.TryValidate(target, out _, out var reason);
|
||||
|
||||
Assert.False(accepted);
|
||||
Assert.Contains("outside every approved launch root", reason);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void A_whitespace_only_root_override_is_ignored()
|
||||
{
|
||||
var policy = new LaunchPathPolicy(new[] { " ", "" });
|
||||
var target = CreateFile("udc.exe");
|
||||
|
||||
var accepted = policy.TryValidate(target, out _, out _);
|
||||
|
||||
Assert.False(accepted);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void ProgramData_is_an_approved_root_so_MTConnect_can_start()
|
||||
{
|
||||
// MTConnect installs to ProgramData\udc\mtconnect_udc\agent and starts
|
||||
// through run.bat. With only the Program Files roots approved, its
|
||||
// server-provisioned launch path was rejected outright, which is why
|
||||
// remote start only ever worked through a local launchpaths pin that
|
||||
// skipped validation. Deliberately broad for now - see BuildDefaultRoots.
|
||||
var programData = Environment.GetFolderPath(
|
||||
Environment.SpecialFolder.CommonApplicationData);
|
||||
Assert.False(string.IsNullOrWhiteSpace(programData));
|
||||
|
||||
Assert.Contains(
|
||||
LaunchPathPolicy.DefaultApprovedRoots,
|
||||
root => root.StartsWith(programData, StringComparison.OrdinalIgnoreCase));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void The_Program_Files_roots_are_still_approved()
|
||||
{
|
||||
// Widening to ProgramData must not have replaced the original roots:
|
||||
// eDNC and UDC both install under Program Files.
|
||||
var programFiles = Environment.GetFolderPath(
|
||||
Environment.SpecialFolder.ProgramFiles);
|
||||
Assert.False(string.IsNullOrWhiteSpace(programFiles));
|
||||
|
||||
Assert.Contains(
|
||||
LaunchPathPolicy.DefaultApprovedRoots,
|
||||
root => root.StartsWith(programFiles, StringComparison.OrdinalIgnoreCase));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void A_launcher_script_under_an_approved_root_is_accepted()
|
||||
{
|
||||
// The run.bat pattern: no directly launchable .exe, so the approved
|
||||
// extension set has to keep covering .bat.
|
||||
var policy = PolicyForRoot();
|
||||
var target = CreateFile("run.bat");
|
||||
|
||||
var accepted = policy.TryValidate(target, out var validated, out var reason);
|
||||
|
||||
Assert.True(accepted, reason);
|
||||
Assert.EndsWith("run.bat", validated, StringComparison.OrdinalIgnoreCase);
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,71 @@
|
||||
using ShopDbEndpointAgent.Core;
|
||||
using Xunit;
|
||||
|
||||
namespace ShopDbEndpointAgent.Core.Tests;
|
||||
|
||||
// Pins the local launch map, which decides where process_start looks for an
|
||||
// executable before it consults the server.
|
||||
//
|
||||
// The rule these tests protect: launch paths are SERVER authoritative. A local
|
||||
// entry wins over the server-provisioned path and bypasses LaunchPathPolicy
|
||||
// validation entirely, so anything shipped here is both unvalidated and immune
|
||||
// to WebUI edits. That combination is what left stale pins on endpoints, so the
|
||||
// compiled defaults are empty on purpose and a test guards that.
|
||||
public class LaunchPathResolutionTests
|
||||
{
|
||||
[Fact]
|
||||
public void No_launch_paths_are_compiled_into_the_agent()
|
||||
{
|
||||
// A compiled pin cannot be changed from the WebUI and skips validation.
|
||||
// eDNC, UDC and MTConnect all carry their launchpath on the monitor row.
|
||||
Assert.Empty(AgentConfig.DefaultLaunchPaths);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void A_config_without_launch_paths_resolves_to_an_empty_map()
|
||||
{
|
||||
// Empty here is what makes ResolveLaunchPath fall through to the
|
||||
// server-provisioned path on the monitor row.
|
||||
var config = new AgentConfig();
|
||||
|
||||
Assert.Empty(config.ResolveLaunchPaths());
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void A_local_override_still_wins_when_a_site_sets_one()
|
||||
{
|
||||
// The escape hatch stays available for an endpoint that cannot use the
|
||||
// server value; it just is not shipped by default any more.
|
||||
var config = new AgentConfig();
|
||||
config.LaunchPaths["UDC.exe"] = @"D:\Custom\UDC.exe";
|
||||
|
||||
var resolved = config.ResolveLaunchPaths();
|
||||
|
||||
Assert.Equal(@"D:\Custom\UDC.exe", resolved["UDC"]);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void A_local_override_key_is_normalized_to_a_bare_image_name()
|
||||
{
|
||||
// Keys are stored bare and the map is case-insensitive; CommandExecutor
|
||||
// normalizes the lookup side the same way, so '.exe' or not both hit.
|
||||
var config = new AgentConfig();
|
||||
config.LaunchPaths["DncMain.exe"] = @"D:\Dnc\DncMain.exe";
|
||||
|
||||
var resolved = config.ResolveLaunchPaths();
|
||||
|
||||
Assert.True(resolved.ContainsKey("DncMain"));
|
||||
Assert.True(resolved.ContainsKey("dncmain"));
|
||||
Assert.False(resolved.ContainsKey("DncMain.exe"));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void Blank_launch_path_entries_are_ignored()
|
||||
{
|
||||
var config = new AgentConfig();
|
||||
config.LaunchPaths["UDC.exe"] = " ";
|
||||
config.LaunchPaths[" "] = @"D:\Stray.exe";
|
||||
|
||||
Assert.Empty(config.ResolveLaunchPaths());
|
||||
}
|
||||
}
|
||||
203
agent/ShopDbEndpointAgent.Core.Tests/MonitorConfigMergeTests.cs
Normal file
203
agent/ShopDbEndpointAgent.Core.Tests/MonitorConfigMergeTests.cs
Normal file
@@ -0,0 +1,203 @@
|
||||
using ShopDbEndpointAgent.Core;
|
||||
using Xunit;
|
||||
|
||||
namespace ShopDbEndpointAgent.Core.Tests;
|
||||
|
||||
// Pins MonitorConfig.Merge, which decides what an endpoint watches.
|
||||
//
|
||||
// The rule these tests exist to protect: the SERVER is authoritative when it
|
||||
// answers, and a non-answer is not an answer. Concretely, null means "the fetch
|
||||
// failed, keep what we have" while an empty-but-present list means "watch
|
||||
// nothing". Collapsing those two cases is the bug this suite is here to catch,
|
||||
// and it is a bug in both directions. Treat null as empty and one failed HTTP
|
||||
// call blanks monitoring on every endpoint at once. Treat empty as null and
|
||||
// detaching the last monitor in the WebUI silently does nothing forever.
|
||||
public class MonitorConfigMergeTests
|
||||
{
|
||||
private static MonitorConfig Local(params string[] processNames)
|
||||
{
|
||||
return new MonitorConfig
|
||||
{
|
||||
Processes = processNames
|
||||
.Select(name => new MonitorTarget { Name = name })
|
||||
.ToArray(),
|
||||
};
|
||||
}
|
||||
|
||||
private static string[] Names(MonitorTarget[] targets)
|
||||
{
|
||||
return targets.Select(target => target.Name).OrderBy(name => name).ToArray();
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void A_server_list_replaces_the_local_one_rather_than_unioning()
|
||||
{
|
||||
// The regression that motivated the change: dncMain.exe was baked into
|
||||
// appsettings.json by the installer and kept being reported on every
|
||||
// endpoint even though no policy attached it. Under the old union it
|
||||
// survived forever because the server had no way to retract it.
|
||||
var local = Local("dncMain.exe", "UDC.exe");
|
||||
var server = new ResolvedMonitors
|
||||
{
|
||||
Processes = new[] { new MonitorTarget { Name = "notepad.exe" } },
|
||||
};
|
||||
|
||||
var merged = MonitorConfig.Merge(local, server);
|
||||
|
||||
Assert.Equal(new[] { "notepad.exe" }, Names(merged.Processes));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void A_null_server_response_keeps_the_current_watch_list()
|
||||
{
|
||||
// FetchResolvedMonitorsAsync returns null on every failure path. A
|
||||
// transient blip must not blank monitoring fleet-wide.
|
||||
var local = Local("dncMain.exe");
|
||||
|
||||
var merged = MonitorConfig.Merge(local, null);
|
||||
|
||||
Assert.Equal(new[] { "dncMain.exe" }, Names(merged.Processes));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void An_empty_server_response_clears_the_watch_list()
|
||||
{
|
||||
// The counterpart to the test above, and the reason null and empty must
|
||||
// stay distinguishable: detaching the last monitor has to take effect.
|
||||
var local = Local("dncMain.exe");
|
||||
var server = new ResolvedMonitors();
|
||||
|
||||
var merged = MonitorConfig.Merge(local, server);
|
||||
|
||||
Assert.Empty(merged.Processes);
|
||||
Assert.Empty(merged.Services);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void Services_are_replaced_independently_of_processes()
|
||||
{
|
||||
var local = new MonitorConfig
|
||||
{
|
||||
Processes = new[] { new MonitorTarget { Name = "dncMain.exe" } },
|
||||
Services = new[] { new MonitorTarget { Name = "MTConnectAgent" } },
|
||||
};
|
||||
var server = new ResolvedMonitors
|
||||
{
|
||||
Processes = new[] { new MonitorTarget { Name = "udc.exe" } },
|
||||
};
|
||||
|
||||
var merged = MonitorConfig.Merge(local, server);
|
||||
|
||||
Assert.Equal(new[] { "udc.exe" }, Names(merged.Processes));
|
||||
Assert.Empty(merged.Services);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void A_null_local_config_with_a_null_server_response_is_not_a_crash()
|
||||
{
|
||||
// Defensive: the worker holds a singleton AgentConfig whose Monitors
|
||||
// property is assignable, so null is reachable.
|
||||
var merged = MonitorConfig.Merge(null, null);
|
||||
|
||||
Assert.Empty(merged.Processes);
|
||||
Assert.Empty(merged.Services);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void Server_entries_are_deduplicated_case_insensitively_by_name()
|
||||
{
|
||||
// Names are matched by string equality downstream, in the collector and
|
||||
// in the launch-path lookup, so a duplicate would be reported twice.
|
||||
var server = new ResolvedMonitors
|
||||
{
|
||||
Processes = new[]
|
||||
{
|
||||
new MonitorTarget { Name = "UDC.exe", App = "first" },
|
||||
new MonitorTarget { Name = "udc.exe", App = "second" },
|
||||
},
|
||||
};
|
||||
|
||||
var merged = MonitorConfig.Merge(Local(), server);
|
||||
|
||||
var single = Assert.Single(merged.Processes);
|
||||
Assert.Equal("second", single.App);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void Blank_server_entries_are_dropped()
|
||||
{
|
||||
var server = new ResolvedMonitors
|
||||
{
|
||||
Processes = new[]
|
||||
{
|
||||
new MonitorTarget { Name = " " },
|
||||
new MonitorTarget { Name = "" },
|
||||
new MonitorTarget { Name = "udc.exe" },
|
||||
},
|
||||
};
|
||||
|
||||
var merged = MonitorConfig.Merge(Local(), server);
|
||||
|
||||
Assert.Equal(new[] { "udc.exe" }, Names(merged.Processes));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void Server_names_are_trimmed_so_downstream_matching_works()
|
||||
{
|
||||
// The collector compares the reported name against enumerated process
|
||||
// images by exact string equality; an untrimmed name never matches and
|
||||
// the monitor silently reports stopped forever.
|
||||
var server = new ResolvedMonitors
|
||||
{
|
||||
Processes = new[] { new MonitorTarget { Name = " udc.exe " } },
|
||||
};
|
||||
|
||||
var merged = MonitorConfig.Merge(Local(), server);
|
||||
|
||||
Assert.Equal("udc.exe", Assert.Single(merged.Processes).Name);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void A_server_provisioned_launch_path_survives_the_replace()
|
||||
{
|
||||
// CommandExecutor.ServerLaunchPathFor reads the merged watch list to
|
||||
// find launch paths, so dropping LaunchPath here would break
|
||||
// process_start for policy-provisioned apps.
|
||||
var server = new ResolvedMonitors
|
||||
{
|
||||
Processes = new[]
|
||||
{
|
||||
new MonitorTarget
|
||||
{
|
||||
Name = "udc.exe",
|
||||
App = "UDC",
|
||||
LaunchPath = @"C:\Program Files\UDC\udc.exe",
|
||||
},
|
||||
},
|
||||
};
|
||||
|
||||
var merged = MonitorConfig.Merge(Local(), server);
|
||||
|
||||
var target = Assert.Single(merged.Processes);
|
||||
Assert.Equal(@"C:\Program Files\UDC\udc.exe", target.LaunchPath);
|
||||
Assert.Equal("UDC", target.App);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void A_local_only_monitor_does_not_survive_a_successful_fetch()
|
||||
{
|
||||
// The security half of the change. A tampered appsettings.json could
|
||||
// otherwise add a never-running entry and park the asset at a permanent
|
||||
// condition, training operators to ignore a genuinely amber endpoint.
|
||||
var local = Local("fabricated-never-running.exe");
|
||||
var server = new ResolvedMonitors
|
||||
{
|
||||
Processes = new[] { new MonitorTarget { Name = "udc.exe" } },
|
||||
};
|
||||
|
||||
var merged = MonitorConfig.Merge(local, server);
|
||||
|
||||
Assert.DoesNotContain(merged.Processes,
|
||||
target => target.Name == "fabricated-never-running.exe");
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,60 @@
|
||||
<Project Sdk="Microsoft.NET.Sdk">
|
||||
|
||||
<!-- Targets plain net8.0, NOT net8.0-windows, and deliberately does not
|
||||
ProjectReference ShopDbEndpointAgent.Core.
|
||||
|
||||
Core targets net8.0-windows because it needs WMI, the registry and the
|
||||
service control manager. Every CI runner in this repo is Linux
|
||||
(arc-runner-set), and a net8.0 project cannot even reference a
|
||||
net8.0-windows assembly. A test project that mirrored Core's target
|
||||
would therefore never run in CI, and a suite that does not run is not a
|
||||
gate - it is a file nobody reads.
|
||||
|
||||
So the platform-PURE sources are linked in and compiled again here as
|
||||
ordinary .NET 8 code. The three files below have exactly one using
|
||||
between them (System.Text.Json.Serialization); they are data shapes and
|
||||
string logic with no Windows surface, so they behave identically on
|
||||
Linux. This keeps the suite in the existing gates at no infrastructure
|
||||
cost.
|
||||
|
||||
The tradeoff, stated plainly: these tests exercise a recompilation of
|
||||
the source, not the shipped Windows assembly. That is sound only while
|
||||
the linked files stay platform-pure. Adding a Windows-only using to one
|
||||
of them breaks this build loudly, which is fine. Adding an #if WINDOWS
|
||||
branch would NOT break it and would quietly make the tested path differ
|
||||
from the shipped one - so do not do that to a linked file. The exit from
|
||||
this arrangement is multi-targeting Core as net8.0-windows;net8.0, at
|
||||
which point these tests reference the real assembly and carry over
|
||||
unchanged.
|
||||
|
||||
AgentConfigProvider (Microsoft.Win32) and CommandExecutor
|
||||
(System.ServiceProcess, System.Security.*) are NOT linked here: they
|
||||
cannot compile on Linux, and testing them honestly needs a seam so the
|
||||
registry and SCM can be faked. That refactor is tracked separately. -->
|
||||
|
||||
<PropertyGroup>
|
||||
<TargetFramework>net8.0</TargetFramework>
|
||||
<ImplicitUsings>enable</ImplicitUsings>
|
||||
<Nullable>enable</Nullable>
|
||||
<RootNamespace>ShopDbEndpointAgent.Core.Tests</RootNamespace>
|
||||
<AssemblyName>ShopDbEndpointAgent.Core.Tests</AssemblyName>
|
||||
<InvariantGlobalization>true</InvariantGlobalization>
|
||||
<IsPackable>false</IsPackable>
|
||||
</PropertyGroup>
|
||||
|
||||
<ItemGroup>
|
||||
<Compile Include="..\ShopDbEndpointAgent.Core\AgentConfig.cs"
|
||||
Link="Linked\AgentConfig.cs" />
|
||||
<Compile Include="..\ShopDbEndpointAgent.Core\Contracts.cs"
|
||||
Link="Linked\Contracts.cs" />
|
||||
<Compile Include="..\ShopDbEndpointAgent.Core\LaunchPathPolicy.cs"
|
||||
Link="Linked\LaunchPathPolicy.cs" />
|
||||
</ItemGroup>
|
||||
|
||||
<ItemGroup>
|
||||
<PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.11.1" />
|
||||
<PackageReference Include="xunit" Version="2.9.2" />
|
||||
<PackageReference Include="xunit.runner.visualstudio" Version="2.8.2" />
|
||||
</ItemGroup>
|
||||
|
||||
</Project>
|
||||
@@ -81,22 +81,27 @@ public sealed class AgentConfig
|
||||
[JsonPropertyName("approvedlaunchroots")]
|
||||
public List<string> ApprovedLaunchRoots { get; set; } = new();
|
||||
|
||||
// Shipped defaults for the org's common shop-floor apps. A site's
|
||||
// appsettings.json LaunchPaths entries win over these on a per-image basis.
|
||||
// MTConnect is intentionally absent here: it is a Windows service controlled
|
||||
// by the fixed mtconnect_* handler (install.bat + service control), not the
|
||||
// generic process_start path. eDNC and UDC launch their runtime executables
|
||||
// directly.
|
||||
// Intentionally EMPTY: launch paths are server authoritative.
|
||||
//
|
||||
// Launch paths now live on the monitor rows the WebUI owns, the same place
|
||||
// the watch list comes from, so editing a path in the WebUI reaches every
|
||||
// endpoint with no rebuild and no redeploy. A compiled default here (or an
|
||||
// appsettings.json launchpaths entry) still WINS over the server path and
|
||||
// skips LaunchPathPolicy validation entirely, which is exactly how a stale
|
||||
// pinned path would outlive the configuration that justified it.
|
||||
//
|
||||
// This stays empty on purpose. Populate it only as a deliberate local
|
||||
// override for one endpoint that cannot use the server value, and expect
|
||||
// that endpoint to stop tracking WebUI changes for that image.
|
||||
public static readonly IReadOnlyDictionary<string, string> DefaultLaunchPaths =
|
||||
new Dictionary<string, string>(StringComparer.OrdinalIgnoreCase)
|
||||
{
|
||||
["DncMain.exe"] = @"C:\Program Files (x86)\Dnc\bin\DncMain.exe",
|
||||
["UDC.exe"] = @"C:\Program Files\UDC\UDC.exe",
|
||||
};
|
||||
new Dictionary<string, string>(StringComparer.OrdinalIgnoreCase);
|
||||
|
||||
// The effective launch map: shipped defaults overlaid with appsettings.json
|
||||
// entries (site wins). Keys are normalized to bare image names so a lookup
|
||||
// tolerates 'DncMain' or 'DncMain.exe'.
|
||||
// The effective launch map: shipped defaults (now empty) overlaid with
|
||||
// appsettings.json entries. Normally empty, in which case process_start
|
||||
// resolves through the server-provisioned path on the monitor row. Keys are
|
||||
// normalized to bare image names so a lookup tolerates 'DncMain' or
|
||||
// 'DncMain.exe'. Anything here BYPASSES LaunchPathPolicy - see
|
||||
// DefaultLaunchPaths.
|
||||
public IReadOnlyDictionary<string, string> ResolveLaunchPaths()
|
||||
{
|
||||
var resolved = new Dictionary<string, string>(StringComparer.OrdinalIgnoreCase);
|
||||
@@ -212,38 +217,58 @@ public sealed class MonitorConfig
|
||||
[JsonPropertyName("services")] public MonitorTarget[] Services { get; set; }
|
||||
= Array.Empty<MonitorTarget>();
|
||||
|
||||
// Union the local (appsettings.json) watch list with the server-resolved one,
|
||||
// keyed case-insensitively by wire name so a target watched both locally and
|
||||
// by policy is not enumerated twice. Server entries win on the label (App) so
|
||||
// a policy display name overrides a stale local one, but a local-only target
|
||||
// is preserved - the server list augments, it does not replace. Nulls on
|
||||
// either side degrade to the other, which is what keeps monitoring alive when
|
||||
// a fetch fails.
|
||||
// The SERVER decides what this endpoint watches. A server list replaces the
|
||||
// local (appsettings.json) one outright rather than merging into it.
|
||||
//
|
||||
// This used to union the two, which made the effective watch list "whatever
|
||||
// the server says PLUS whatever the box says", and the box half could not be
|
||||
// retracted from the server at all. Two consequences, both bad. Operationally,
|
||||
// monitors the installer had baked into appsettings.json kept being reported
|
||||
// long after policy stopped attaching them, so detaching a monitor in the
|
||||
// WebUI silently did nothing on already-deployed endpoints. Security-wise, it
|
||||
// made a local file an input to server-side health: anyone able to edit it
|
||||
// could add a never-running entry and park the asset at a permanent
|
||||
// condition, training operators to ignore an endpoint that is amber for a
|
||||
// fabricated reason.
|
||||
//
|
||||
// The caller MUST only call this when the fetch succeeded. A failed fetch
|
||||
// returns null from FetchResolvedMonitorsAsync and the caller keeps the
|
||||
// current list - that is what stops a transient network blip from blanking
|
||||
// monitoring fleet-wide. A null server argument here is therefore treated as
|
||||
// "no answer, keep local" rather than "watch nothing"; an EMPTY but present
|
||||
// server list is a real answer and does clear the watch list, which is what
|
||||
// makes detaching the last monitor take effect.
|
||||
public static MonitorConfig Merge(MonitorConfig? local, ResolvedMonitors? server)
|
||||
{
|
||||
if (server is null)
|
||||
return local ?? new MonitorConfig();
|
||||
|
||||
return new MonitorConfig
|
||||
{
|
||||
Processes = MergeTargets(local?.Processes, server?.Processes),
|
||||
Services = MergeTargets(local?.Services, server?.Services),
|
||||
Processes = Authoritative(server.Processes),
|
||||
Services = Authoritative(server.Services),
|
||||
};
|
||||
}
|
||||
|
||||
private static MonitorTarget[] MergeTargets(
|
||||
MonitorTarget[]? local, MonitorTarget[]? server)
|
||||
// Copies the server list, dropping unusable entries. Names are trimmed on
|
||||
// the TARGET, not just on the dictionary key: downstream matching reads
|
||||
// target.Name (the collector compares it against enumerated process images,
|
||||
// and CommandExecutor.ServerLaunchPathFor matches it against the requested
|
||||
// image), so a padded name that was only trimmed for de-duplication would
|
||||
// never match anything and the monitor would report stopped forever.
|
||||
private static MonitorTarget[] Authoritative(MonitorTarget[]? server)
|
||||
{
|
||||
if (server is null || server.Length == 0)
|
||||
return Array.Empty<MonitorTarget>();
|
||||
|
||||
var byName = new Dictionary<string, MonitorTarget>(
|
||||
StringComparer.OrdinalIgnoreCase);
|
||||
foreach (var target in local ?? Array.Empty<MonitorTarget>())
|
||||
foreach (var target in server)
|
||||
{
|
||||
if (string.IsNullOrWhiteSpace(target?.Name))
|
||||
continue;
|
||||
byName[target.Name.Trim()] = target;
|
||||
}
|
||||
foreach (var target in server ?? Array.Empty<MonitorTarget>())
|
||||
{
|
||||
if (string.IsNullOrWhiteSpace(target?.Name))
|
||||
continue;
|
||||
byName[target.Name.Trim()] = target;
|
||||
target.Name = target.Name.Trim();
|
||||
byName[target.Name] = target;
|
||||
}
|
||||
return byName.Values.ToArray();
|
||||
}
|
||||
@@ -258,12 +283,13 @@ public sealed class MonitorTarget
|
||||
// Human-facing application label, optional. Falls back to Name when empty.
|
||||
[JsonPropertyName("app")] public string App { get; set; } = "";
|
||||
|
||||
// Optional server-provisioned launch path for a process monitor. Only set on
|
||||
// process targets that a site leader configured a path for. The agent NEVER
|
||||
// launches this blindly: LaunchPathPolicy validates it against an approved-
|
||||
// root allowlist first, and a local appsettings.json launchpaths entry for
|
||||
// the same image still wins. Null/empty means "no server path" - the agent
|
||||
// falls back to its local launch map, and process_start fails closed when
|
||||
// neither yields a validated path.
|
||||
// Optional server-provisioned launch path for a process monitor. Set on
|
||||
// process targets a site leader configured a path for, and now the PRIMARY
|
||||
// source for process_start since the local launch map ships empty. The
|
||||
// agent never launches this blindly: LaunchPathPolicy validates it against
|
||||
// the approved-root allowlist, checks the extension, confirms it exists and
|
||||
// re-checks it after resolving symlinks/junctions. A local appsettings.json
|
||||
// launchpaths entry for the same image still wins if one is present.
|
||||
// Null/empty means "no server path" and process_start fails closed.
|
||||
[JsonPropertyName("launchpath")] public string? LaunchPath { get; set; }
|
||||
}
|
||||
|
||||
@@ -237,8 +237,10 @@ public sealed class CommandResult
|
||||
// The server-resolved watch list for this endpoint (processes and services the
|
||||
// site's policy attaches to the box's PC type), fetched each telemetry cycle so
|
||||
// a policy-attached monitor is enumerated without editing appsettings.json on
|
||||
// the box. Same {name, app} shape as the local MonitorConfig, so the merge is a
|
||||
// direct list union. Empty lists mean "watch nothing extra beyond local".
|
||||
// the box. Same {name, app} shape as the local MonitorConfig, and it REPLACES
|
||||
// that local list rather than unioning with it - the server is authoritative on
|
||||
// what is watched. Empty lists mean "watch nothing", which is how detaching the
|
||||
// last monitor takes effect; "no answer" is a null response, not an empty one.
|
||||
public sealed class ResolvedMonitors
|
||||
{
|
||||
[JsonPropertyName("processes")] public MonitorTarget[] Processes { get; set; }
|
||||
|
||||
@@ -23,8 +23,10 @@ public sealed class LaunchPathPolicy
|
||||
private static readonly string[] ApprovedExtensions =
|
||||
{ ".exe", ".bat", ".cmd" };
|
||||
|
||||
// Shipped org defaults: the standard 64- and 32-bit install roots. Resolved
|
||||
// from the environment so a non-C: system drive still matches.
|
||||
// Shipped org defaults: the standard 64- and 32-bit install roots plus
|
||||
// ProgramData. See BuildDefaultRoots for why ProgramData is included and
|
||||
// why it should be narrowed later. Resolved from the environment so a
|
||||
// non-C: system drive still matches.
|
||||
public static IReadOnlyList<string> DefaultApprovedRoots { get; } =
|
||||
BuildDefaultRoots();
|
||||
|
||||
@@ -222,12 +224,37 @@ public sealed class LaunchPathPolicy
|
||||
return full.TrimEnd(Path.DirectorySeparatorChar);
|
||||
}
|
||||
|
||||
// REVISIT: ProgramData is deliberately broad here and should be narrowed.
|
||||
//
|
||||
// Shop-floor apps do not all install to Program Files. MTConnect lives at
|
||||
// ProgramData\udc\mtconnect_udc\agent and starts through run.bat, so with
|
||||
// only the Program Files roots approved, every server-provisioned launch
|
||||
// path for it was rejected as "outside every approved launch root". That
|
||||
// was previously masked because a local appsettings.json launchpaths entry
|
||||
// bypasses validation entirely; now that launch paths are server
|
||||
// authoritative, the root list is the only gate and it has to cover the
|
||||
// real install locations.
|
||||
//
|
||||
// WARNING: ProgramData is world-writable by default on Windows. A
|
||||
// non-administrator who can write under an approved ProgramData subtree can
|
||||
// plant a .bat/.cmd/.exe there, and the agent runs launch paths as SYSTEM.
|
||||
// The other gates still apply (the path must come from a monitor row an
|
||||
// admin configured, must exist, must be a launchable extension, and is
|
||||
// re-checked after symlink/junction resolution), so this is not an open
|
||||
// door - but it is a wider blast radius than Program Files, whose ACLs
|
||||
// require elevation to write.
|
||||
//
|
||||
// Narrow this to the specific subtrees in use (ProgramData\udc rather than
|
||||
// all of ProgramData) once the full set of non-Program-Files install
|
||||
// locations across the fleet is confirmed. A site can already do that today
|
||||
// via AgentConfig.ApprovedLaunchRoots, which REPLACES this list.
|
||||
private static IReadOnlyList<string> BuildDefaultRoots()
|
||||
{
|
||||
var candidates = new[]
|
||||
{
|
||||
Environment.GetFolderPath(Environment.SpecialFolder.ProgramFiles),
|
||||
Environment.GetFolderPath(Environment.SpecialFolder.ProgramFilesX86),
|
||||
Environment.GetFolderPath(Environment.SpecialFolder.CommonApplicationData),
|
||||
};
|
||||
return candidates
|
||||
.Where(path => !string.IsNullOrWhiteSpace(path))
|
||||
|
||||
@@ -231,9 +231,11 @@ public sealed class TelemetryWorker : BackgroundService
|
||||
{
|
||||
// Refresh the watch list from server policy BEFORE collecting so a
|
||||
// policy-attached monitor (added in the WebUI, never in appsettings.json)
|
||||
// is enumerated and reported this cycle. Server entries merge over the
|
||||
// local list; a failed fetch returns null and Merge keeps the local list,
|
||||
// so monitoring never blanks on a transient blip.
|
||||
// is enumerated and reported this cycle. The server list REPLACES the
|
||||
// local one, so detaching a monitor in the WebUI actually stops it being
|
||||
// watched even on endpoints whose installer baked it into
|
||||
// appsettings.json. A failed fetch returns null and Merge keeps the
|
||||
// current list, so monitoring never blanks on a transient blip.
|
||||
var resolved = await client.FetchResolvedMonitorsAsync(
|
||||
Environment.MachineName, token);
|
||||
if (resolved is not null)
|
||||
|
||||
Reference in New Issue
Block a user