diff --git a/agent/ShopDbEndpointAgent.Core/AgentConfig.cs b/agent/ShopDbEndpointAgent.Core/AgentConfig.cs index 991d857e..92320022 100644 --- a/agent/ShopDbEndpointAgent.Core/AgentConfig.cs +++ b/agent/ShopDbEndpointAgent.Core/AgentConfig.cs @@ -89,38 +89,44 @@ public sealed class AgentConfig return bare; } - // Per-name allowlist of pre-staged scripts for the generic run_script - // action. Exactly like LaunchPaths, the wire never carries a script body or - // path - only a script NAME the agent already holds here. A name absent - // from this map fails closed. There are no shipped defaults: a script is a - // site-specific artifact, so an empty map means run_script does nothing - // until a site stages scripts and lists them in appsettings.json. Keyed by - // script name (case-insensitive, .ps1 optional). - [JsonPropertyName("scriptpaths")] - public Dictionary ScriptPaths { get; set; } = new(); + // REMOVED DELIBERATELY - DO NOT REINTRODUCE: 'scriptpaths' / ScriptPaths. + // + // This used to be a per-name map of PRE-STAGED scripts on the endpoint + // (typically under C:\ProgramData\ShopDB\EndpointAgent\scripts), which + // run_script would look up by name and execute. It was superseded by + // server-hosted delivery and left behind as dead config: nothing read the + // map, so it was an execution path that appeared supported but was not. + // + // It is gone rather than merely unused because reviving it is a security + // regression, and dead code invites revival. That model had NO integrity + // check - no digest, no provenance - so whoever could write into the staging + // directory chose what the agent ran as SYSTEM. ProgramData is world- + // writable by default unless the tree is explicitly tightened, which made + // the gate "is this name in appsettings.json", not "is this content + // trusted". + // + // The supported path is run_script with a server-issued delivery descriptor: + // the body is fetched from ShopDB over the collector credential and verified + // against a server-pinned SHA-256 before it executes, so the content is + // authenticated and the action is attributable in the command log. If you + // need a script to run on an endpoint, publish it server-side and send it + // through that path. If you need to trigger something already installed on + // the box, use scheduledtask_run, which is far narrower. - // The effective script map: appsettings.json entries only (no defaults). - // Keys are normalized to bare names so a lookup tolerates 'reset' or - // 'reset.ps1'. - public IReadOnlyDictionary ResolveScriptPaths() - { - var resolved = new Dictionary(StringComparer.OrdinalIgnoreCase); - foreach (var pair in ScriptPaths) - { - if (string.IsNullOrWhiteSpace(pair.Key) || string.IsNullOrWhiteSpace(pair.Value)) - continue; - resolved[NormalizeScript(pair.Key)] = pair.Value; - } - return resolved; - } - - private static string NormalizeScript(string name) - { - var bare = (name ?? "").Trim(); - if (bare.EndsWith(".ps1", StringComparison.OrdinalIgnoreCase)) - bare = bare[..^4]; - return bare; - } + // Approved scheduled task paths for the scheduledtask_run action. The wire + // carries a free-text task path so the WebUI can expose it as a single + // field; ScheduledTaskPolicy validates it against this list and fails closed + // when it does not match. An entry matches that exact task path, or acts as + // a folder prefix for anything beneath it. + // + // Empty means "use the shipped Corporate Tools defaults" + // (ScheduledTaskPolicy.DefaultApprovedTaskPaths). Setting it REPLACES those + // defaults, so a site can narrow the set as well as widen it without an + // agent rebuild. Keep it tight: every entry here is something an operator + // holding endpoints.deploy can start, in whatever context the task itself + // registered. + [JsonPropertyName("approvedtaskpaths")] + public List ApprovedTaskPaths { get; set; } = new(); public bool IsUsable() => !string.IsNullOrWhiteSpace(BaseUrl) diff --git a/agent/ShopDbEndpointAgent.Core/CommandExecutor.cs b/agent/ShopDbEndpointAgent.Core/CommandExecutor.cs index 6bd3315b..99277899 100644 --- a/agent/ShopDbEndpointAgent.Core/CommandExecutor.cs +++ b/agent/ShopDbEndpointAgent.Core/CommandExecutor.cs @@ -64,6 +64,10 @@ public sealed class CommandExecutor // before they are honored. Local launchPaths always win over a server path. private readonly LaunchPathPolicy launchPathPolicy; + // Validates the free-text task path carried by scheduledtask_run before the + // agent will start anything. Fails closed: an unapproved path never runs. + private readonly ScheduledTaskPolicy scheduledTaskPolicy; + // Held for update_app: the agent joins InstallPath to BaseUrl and downloads // the pinned installer with its own collector credential (ApiKey), so the // installer body never rides the command wire. @@ -77,6 +81,7 @@ public sealed class CommandExecutor this.config = config ?? new AgentConfig(); this.launchPaths = this.config.ResolveLaunchPaths(); this.launchPathPolicy = new LaunchPathPolicy(this.config.ApprovedLaunchRoots); + this.scheduledTaskPolicy = new ScheduledTaskPolicy(this.config.ApprovedTaskPaths); } public async Task ExecuteAsync( @@ -116,6 +121,16 @@ public sealed class CommandExecutor return RequireTarget(action, target) ?? await ProcessStart(target, token); + // Start an already-registered scheduled task by path. Deliberately + // narrow: the agent does not create, modify, or define tasks, it + // only starts one that already exists AND clears the approved-task + // policy. Prefer this over run_script whenever the work is already + // packaged as a task - it is a far smaller grant than shipping a + // script body, and the command log names exactly what ran. + case "scheduledtask_run": + return RequireTarget(action, target) + ?? await ScheduledTaskRun(target, token); + // Software delivery. run_script fetches a ShopDB-hosted script // pinned by SHA-256 and runs it in the script's configured context // (system or the interactive user); update_app fetches a ShopDB @@ -650,6 +665,139 @@ public sealed class CommandExecutor return found; } + // ---- Scheduled tasks ---- + + // Upper bound on how long a task start may block the command worker. This + // waits only for schtasks.exe to hand the request to the Task Scheduler + // service, NOT for the task's own work to finish - schtasks /run returns as + // soon as the task is launched. So success here means "the task was + // started", not "the task succeeded", and the timeout only needs to cover + // the handoff. + private static readonly TimeSpan ScheduledTaskStartTimeout = TimeSpan.FromSeconds(30); + + // Start an already-registered scheduled task by path, after the policy + // approves it. Uses schtasks.exe with ArgumentList (no shell, no string + // concatenation) so the task path is passed as a single argv entry and + // cannot break out into additional arguments. + // + // The task runs in whatever security context its own registered definition + // specifies - SYSTEM, an interactive user, whatever principal was set when + // the task was created. The agent does not and cannot change that, which is + // exactly why this action is narrower than run_script: the endpoint owns the + // definition of what happens, the server only asks for it to happen now. + private async Task ScheduledTaskRun(string target, CancellationToken token) + { + if (!scheduledTaskPolicy.TryValidate(target, out var taskPath, out var reason)) + { + logger.LogWarning( + "Rejected scheduledtask_run for {Target}: {Reason}", target, reason); + return new CommandResult + { + Succeeded = false, + Message = $"Scheduled task '{target}' rejected: {reason}", + }; + } + + try + { + var startInfo = new ProcessStartInfo + { + FileName = "schtasks.exe", + UseShellExecute = false, + CreateNoWindow = true, + RedirectStandardOutput = true, + RedirectStandardError = true, + }; + startInfo.ArgumentList.Add("/run"); + startInfo.ArgumentList.Add("/tn"); + startInfo.ArgumentList.Add(taskPath); + + using var process = Process.Start(startInfo); + if (process is null) + { + return new CommandResult + { + Succeeded = false, + Message = $"Failed to start schtasks.exe for task {taskPath}", + }; + } + + var stdout = ""; + var stderr = ""; + + using var timeout = CancellationTokenSource.CreateLinkedTokenSource(token); + timeout.CancelAfter(ScheduledTaskStartTimeout); + try + { + await process.WaitForExitAsync(timeout.Token); + // Read only after exit. Reading to end first would block until + // the streams close - which IS process exit - so the timeout + // above could never fire. Safe here because schtasks emits a + // line or two, far below the pipe buffer that would deadlock a + // chattier child. + stdout = await process.StandardOutput.ReadToEndAsync(token); + stderr = await process.StandardError.ReadToEndAsync(token); + } + catch (OperationCanceledException) when (!token.IsCancellationRequested) + { + TryKill(process); + return new CommandResult + { + Succeeded = false, + Message = $"Timed out starting scheduled task {taskPath}", + }; + } + + var exit = process.ExitCode; + var ok = exit == 0; + // schtasks writes the useful diagnosis (task not found, access + // denied, disabled) to its output, so surface it rather than making + // the operator guess from a bare exit code. + var detail = (stderr.Trim().Length > 0 ? stderr.Trim() : stdout.Trim()); + if (ok) + { + logger.LogInformation("Started scheduled task {TaskPath}", taskPath); + } + else + { + logger.LogWarning( + "schtasks /run for {TaskPath} exited {Exit}: {Detail}", + taskPath, exit, detail); + } + + return new CommandResult + { + Succeeded = ok, + Message = ok + ? $"Started scheduled task {taskPath}" + : $"Scheduled task {taskPath} failed to start (exit {exit})" + + (detail.Length > 0 ? $": {detail}" : ""), + }; + } + catch (Exception error) + { + logger.LogWarning(error, "scheduledtask_run for {TaskPath} failed", taskPath); + return new CommandResult + { + Succeeded = false, + Message = $"Scheduled task {taskPath} failed: {error.Message}", + }; + } + } + + private void TryKill(Process process) + { + try + { + if (!process.HasExited) + process.Kill(entireProcessTree: true); + } + catch (Exception error) + { + logger.LogDebug(error, "Could not kill timed-out schtasks.exe"); + } + } + // ---- Software delivery: run_script and update_app ---- // Run a ShopDB-hosted script pinned by the server's delivery descriptor. The @@ -659,6 +807,26 @@ public sealed class CommandExecutor // 'system' through the agent's own service account, or 'user' in the active // console session so the script sees the interactive user's resources (for // example mapped network drives). Anything missing or mismatched fails closed. + // + // SECURITY CONTRACT - read before changing this method. This is arbitrary + // code execution on the endpoint, by design, and the agent is a SYSTEM + // service: a script whose descriptor does not pin the user context runs with + // full machine privilege. Three properties make that acceptable, and all + // three are load-bearing: + // + // 1. The body is never carried on the command wire. The agent fetches it + // itself from ShopDB using its own collector credential, so queueing a + // command cannot smuggle in code. + // 2. The body is verified against the server-pinned SHA-256 before it + // runs. A tampered or swapped body fails closed. + // 3. The action is gated on endpoints.deploy server-side and is meant to + // sit behind an explicit UI confirmation, the same as system_restart. + // + // Do not add a path that runs a script the agent found locally rather than + // fetched and verified - that was the old pre-staged 'scriptpaths' model and + // it was removed on purpose (see the tombstone in AgentConfig). If the goal + // is to trigger something already installed on the endpoint, use + // scheduledtask_run instead: it is a much smaller grant. private async Task RunScript( string name, ScriptDeliveryClaim? script, CancellationToken token) { diff --git a/agent/ShopDbEndpointAgent.Core/Contracts.cs b/agent/ShopDbEndpointAgent.Core/Contracts.cs index 171a63de..911ad397 100644 --- a/agent/ShopDbEndpointAgent.Core/Contracts.cs +++ b/agent/ShopDbEndpointAgent.Core/Contracts.cs @@ -75,6 +75,14 @@ public sealed class EndpointReport // the probe did not produce a result this cycle - absent means "not // observed", which is deliberately distinct from an empty list. [JsonPropertyName("usercontext")] public object? UserContext { get; set; } + + // Pending Dell Command | Update results, produced by the shopdb-dellupdates + // agent utility rather than collected in C#: the data comes from dcu-cli's + // own scan output, so the utility that parses it belongs next to the task + // that writes it. Null means no scan has ever run (or the utility is not + // installed), which is deliberately distinct from a scan that found nothing + // pending - the first is "unknown", the second is "known clean". + [JsonPropertyName("dellupdates")] public object? DellUpdates { get; set; } } public sealed class EndpointEventRecord diff --git a/agent/ShopDbEndpointAgent.Core/ScheduledTaskPolicy.cs b/agent/ShopDbEndpointAgent.Core/ScheduledTaskPolicy.cs new file mode 100644 index 00000000..0a328267 --- /dev/null +++ b/agent/ShopDbEndpointAgent.Core/ScheduledTaskPolicy.cs @@ -0,0 +1,159 @@ +namespace ShopDbEndpointAgent.Core; + +// Guards which registered scheduled tasks the scheduledtask_run action may +// start. The wire carries a free-text task path (so the WebUI can expose it as +// one field, and a site can widen the set without an agent rebuild); this +// policy is the safeguard that makes that free text safe to honor. +// +// WHY THIS EXISTS AT ALL. The agent runs as SYSTEM. A scheduled task is an +// INDIRECTION: the task path names a definition, and that definition chooses +// what actually runs. So "start the task named X" is only as safe as the set of +// X the agent will accept - an unconstrained task name would be a general- +// purpose "run anything already registered on this box, as whatever context it +// asked for" primitive. That is a materially bigger grant than any other +// allowlisted action, and it would be invisible in the audit trail (the command +// log would read "scheduledtask_run \Foo" no matter what \Foo does). +// +// WHAT THIS DOES AND DOES NOT BUY. A prefix allowlist stops a compromised or +// misconfigured SERVER from starting arbitrary tasks on the fleet - that is the +// threat it actually addresses, and it is the realistic one. It does NOT stop an +// attacker who can already register or rewrite task definitions on the endpoint: +// such an attacker can define a task under an approved prefix and have the agent +// start it. That is accepted, because anyone who can write task definitions on a +// box already has the privilege the task would grant them. Local task definition +// ACLs, not this policy, are the control for that. +// +// The approved-prefix list ships with the Corporate Tools set and is overridable +// per site via AgentConfig.ApprovedTaskPaths, matching the LaunchPathPolicy +// pattern so the future Site Agent Configuration tool can narrow or widen it +// without a rebuild. +public sealed class ScheduledTaskPolicy +{ + // Shipped defaults: the Corporate Tools task set. Prefix entries end with a + // backslash and match a whole folder; entries without one match that exact + // task path only. Kept deliberately tight - widen per site, not here. + // + // The ShopDB-* entries are tasks ShopDB itself owns. They are separate task + // definitions from any corporate-initiated equivalent ON PURPOSE: a + // ShopDB-initiated Dell Command scan must write its results somewhere the + // agent's collector can attribute to ShopDB, rather than into whatever + // location a corporate-managed task already writes. Keeping the definitions + // distinct is what makes "who asked for this scan" answerable at all - see + // the note in commands.py about the DCU output path convention. + public static IReadOnlyList DefaultApprovedTaskPaths { get; } = new[] + { + @"\ReportIP", + @"\SFLD-ApplyDSCConfig", + @"\SFLD - Consume Credentials", + @"\ShopDB-DellCommandScan", + @"\ShopDB-DellCommandUpdate", + }; + + private readonly IReadOnlyList approvedPaths; + + public ScheduledTaskPolicy(IEnumerable? approvedPathsOverride = null) + { + var paths = (approvedPathsOverride ?? Enumerable.Empty()) + .Where(path => !string.IsNullOrWhiteSpace(path)) + .Select(NormalizeTaskPath) + .ToList(); + approvedPaths = paths.Count > 0 ? paths : DefaultApprovedTaskPaths; + } + + // Validates a candidate task path. On success taskPath is the normalized + // path to hand to Task Scheduler; on failure reason explains the rejection + // so the command result tells the operator WHY rather than just "denied". + public bool TryValidate(string? candidate, out string taskPath, out string reason) + { + taskPath = ""; + reason = ""; + + if (string.IsNullOrWhiteSpace(candidate)) + { + reason = "empty task path"; + return false; + } + + var normalized = NormalizeTaskPath(candidate); + + // Traversal in a task path is meaningless to Task Scheduler but very + // meaningful to a prefix check: "\Approved\..\Evil" starts with an + // approved prefix as a STRING while naming something else entirely. + // Reject rather than try to canonicalize it away. + if (normalized.Contains("..")) + { + reason = "task path contains a traversal segment"; + return false; + } + + // Control characters and quotes have no place in a task path and are the + // shape of an attempt to break out of the argument. The invocation uses + // ArgumentList (no shell, no string concatenation) so this is defense in + // depth rather than the primary control, but a task path that needs + // these is not a task path we should be starting. + foreach (var ch in normalized) + { + if (char.IsControl(ch) || ch == '"' || ch == '&' || ch == '|') + { + reason = "task path contains an illegal character"; + return false; + } + } + + if (!IsApproved(normalized)) + { + reason = "task path is not in the approved task list"; + return false; + } + + taskPath = normalized; + return true; + } + + // Task Scheduler paths are rooted and backslash-separated. Accept the + // operator writing "ReportIP", "\ReportIP", or "/ReportIP" and settle on the + // rooted form, so an allowlist entry does not hinge on how it was typed. + private static string NormalizeTaskPath(string path) + { + var trimmed = (path ?? "").Trim().Replace('/', '\\'); + if (trimmed.Length == 0) + return ""; + if (!trimmed.StartsWith('\\')) + trimmed = "\\" + trimmed; + // Collapse a trailing separator so "\Folder\" and "\Folder" compare + // equal as exact entries; prefix semantics are handled in IsApproved. + while (trimmed.Length > 1 && trimmed.EndsWith('\\')) + trimmed = trimmed[..^1]; + return trimmed; + } + + // An entry ending in a separator is a FOLDER prefix and matches anything + // beneath it; any other entry must match the whole task path exactly. The + // exact-match case is why "\Approved" does not silently authorize + // "\ApprovedEvil". + private bool IsApproved(string candidate) + { + foreach (var entry in approvedPaths) + { + if (entry.Length == 0) + continue; + + var isFolderPrefix = entry.EndsWith('\\'); + if (isFolderPrefix) + { + if (candidate.StartsWith(entry, StringComparison.OrdinalIgnoreCase)) + return true; + continue; + } + + if (string.Equals(candidate, entry, StringComparison.OrdinalIgnoreCase)) + return true; + + // Allow the folder form without its trailing separator to still act + // as a folder: "\SFLD" approves "\SFLD\Thing" but never "\SFLDEvil". + if (candidate.StartsWith(entry + "\\", StringComparison.OrdinalIgnoreCase)) + return true; + } + return false; + } +} diff --git a/agent/ShopDbEndpointAgent.Core/ShopDbEndpointAgent.Core.csproj b/agent/ShopDbEndpointAgent.Core/ShopDbEndpointAgent.Core.csproj index 013080b0..ee711845 100644 --- a/agent/ShopDbEndpointAgent.Core/ShopDbEndpointAgent.Core.csproj +++ b/agent/ShopDbEndpointAgent.Core/ShopDbEndpointAgent.Core.csproj @@ -17,6 +17,10 @@ + + diff --git a/agent/ShopDbEndpointAgent.Core/TelemetryWorker.cs b/agent/ShopDbEndpointAgent.Core/TelemetryWorker.cs index 2efa847d..97b94bb4 100644 --- a/agent/ShopDbEndpointAgent.Core/TelemetryWorker.cs +++ b/agent/ShopDbEndpointAgent.Core/TelemetryWorker.cs @@ -15,6 +15,11 @@ namespace ShopDbEndpointAgent.Core; // cadence tick rather than hammering the server. public sealed class TelemetryWorker : BackgroundService { + // The one agent utility whose output is forwarded to the server today. The + // name is the .ps1 filename without its extension, which is how + // PowerShellUtilityRunner keys its results. + public const string DellUpdatesUtilityName = "shopdb-dellupdates"; + private readonly ICollector collector; private readonly CollectorClient client; private readonly IAgentUtilityRunner? utilities; @@ -22,11 +27,31 @@ public sealed class TelemetryWorker : BackgroundService private readonly string agentVersion; private readonly ILogger logger; private readonly CollectSignal? collectSignal; + private readonly UserContextSpool? userContextSpool; + private readonly UserContextRefreshChannel? userContextRefresh; private readonly Random jitter = new(); + // How long to wait for the tray to answer a refresh request. + // + // Bounded, and bounded SHORT. Telemetry is this agent's liveness signal: a + // report that arrives with slightly stale mapped drives is a working + // endpoint, while a report that never arrives because we blocked on an + // absent tray is an endpoint that looks dead. The probe itself is a single + // short PowerShell run, so a tray that is alive answers well inside this; + // anything longer means nobody is home. + private static readonly TimeSpan UserContextRefreshTimeout = TimeSpan.FromSeconds(12); + + // Poll interval while waiting for the stamp to advance. There is no + // completion signal coming back the other way - the tray's answer IS the + // rewritten spool file - so this is a poll by design, kept coarse enough to + // be free and fine enough not to add visible latency. + private static readonly TimeSpan UserContextPollInterval = TimeSpan.FromMilliseconds(400); + public TelemetryWorker(ICollector collector, CollectorClient client, AgentConfig config, string agentVersion, ILogger logger, - IAgentUtilityRunner? utilities = null, CollectSignal? collectSignal = null) + IAgentUtilityRunner? utilities = null, CollectSignal? collectSignal = null, + UserContextSpool? userContextSpool = null, + UserContextRefreshChannel? userContextRefresh = null) { this.collector = collector; this.client = client; @@ -35,6 +60,77 @@ public sealed class TelemetryWorker : BackgroundService this.logger = logger; this.utilities = utilities; this.collectSignal = collectSignal; + this.userContextSpool = userContextSpool; + this.userContextRefresh = userContextRefresh; + } + + // Request a tray-side re-probe and wait, briefly, for the spool to be + // rewritten. Never throws and never fails the run. + // + // The service does NOT collect session data itself and must not start doing + // so: mapped drives belong to the operator's logon token, and a LocalSystem + // probe reads a different, near-empty drive table. See the tombstone in + // Program.cs. This method is the entire service-side involvement - ask, then + // wait for the answer to appear on disk. + private async Task RefreshUserContextAsync(CancellationToken token) + { + if (userContextSpool is null || userContextRefresh is null + || !OperatingSystem.IsWindows()) + { + return; + } + + var resultPath = userContextSpool.TryGetResultPath(out _); + if (resultPath is null) + return; + + // Captured BEFORE the request so "advanced" is measured against what we + // would otherwise have reported. A missing spool reads as null, and any + // stamp at all then counts as progress - that is the first-probe case. + var before = userContextSpool.TryReadCollectedAt(resultPath); + + if (!userContextRefresh.RequestRefresh()) + { + // Could not signal at all. Note that the inverse does NOT mean a tray + // is listening: the channel opens-or-creates and does not hold the + // handle, so signalling into an empty machine succeeds by creating + // the event and finding nobody waiting on it. Detecting an absent + // tray is the timeout's job below, not this return value's. + logger.LogDebug( + "Could not request a user-context refresh; reporting the existing spool."); + return; + } + + var deadline = DateTime.UtcNow + UserContextRefreshTimeout; + while (DateTime.UtcNow < deadline) + { + try + { + await Task.Delay(UserContextPollInterval, token); + } + catch (OperationCanceledException) + { + return; + } + + var after = userContextSpool.TryReadCollectedAt(resultPath); + if (after is not null && (before is null || after > before)) + { + logger.LogDebug( + "User-context spool refreshed by the tray (collectedat={Stamp}).", + after); + return; + } + } + + // Timed out. Proceed anyway - deliberately. Session data is one card on + // one page; telemetry carries heartbeat, inventory, events and command + // results, and holding all of that hostage to a tray that did not answer + // in twelve seconds trades a small inaccuracy for a total outage. + logger.LogDebug( + "The tray did not refresh the user-context spool within {Timeout}; " + + "reporting with the existing session data.", + UserContextRefreshTimeout); } protected override async Task ExecuteAsync(CancellationToken stoppingToken) @@ -111,6 +207,15 @@ public sealed class TelemetryWorker : BackgroundService config.Monitors.Processes.Length, config.Monitors.Services.Length); } + // Ask the tray to re-probe session data and WAIT for it, before + // collecting. Ordering is the whole point: the collector reads the spool + // file, so a refresh that lands after Collect() is a refresh the server + // does not see until the NEXT cycle. That was the bug - "Send Status" + // and "Refresh Status" both re-sent a spool nobody had rewritten, so a + // drive the operator had just unmapped kept reporting OK for up to half + // an hour and no amount of clicking changed it. + await RefreshUserContextAsync(token); + var report = collector.Collect(agentVersion, adopt); if (utilities is not null) @@ -118,6 +223,17 @@ public sealed class TelemetryWorker : BackgroundService var extra = utilities.RunAll(token); if (extra.Count > 0) logger.LogDebug("Loaded {Count} agent utility result(s)", extra.Count); + + // NARROW ON PURPOSE. The runner executes every .ps1 in the utilities + // directory, but only this one known key is attached to the report. + // Forwarding the whole dictionary would mean any script dropped into + // that directory starts uploading its output to the server on every + // cadence - see the signing caveat in PowerShellUtilityRunner's + // header. Widening this to the full map is the intended end state of + // the IAgentUtilityRunner seam, but it needs script signing or an + // allowlist first. Until then, add a key here per utility. + if (extra.TryGetValue(DellUpdatesUtilityName, out var dellUpdates)) + report.DellUpdates = dellUpdates; } logger.LogInformation( diff --git a/agent/ShopDbEndpointAgent.Core/UserContextRefreshChannel.cs b/agent/ShopDbEndpointAgent.Core/UserContextRefreshChannel.cs new file mode 100644 index 00000000..af07004d --- /dev/null +++ b/agent/ShopDbEndpointAgent.Core/UserContextRefreshChannel.cs @@ -0,0 +1,179 @@ +using System.Runtime.Versioning; +using System.Security.AccessControl; +using System.Security.Principal; +using System.Threading; + +using Microsoft.Extensions.Logging; + +namespace ShopDbEndpointAgent.Core; + +// The service-to-tray request channel for an on-demand user-context refresh. +// +// WHY THIS EXISTS AT ALL. Mapped drives are a property of the operator's logon +// token, not of the machine. The agent is a session-0 LocalSystem service, so it +// CANNOT enumerate them - not by impersonation, not by CreateProcessAsUser, not +// by starting the probe task itself. That approach was built once and reverted; +// see the tombstone in Program.cs where UserContextProbe is deliberately kept +// out of the service's DI container. The only process on the box that can see +// the right drive table is one already running inside the operator's session: +// the tray. +// +// So the service does not probe. It ASKS. This channel is the ask, and the +// spool file remains the answer. +// +// WHY A NAMED EVENT rather than a pipe or a socket. The request carries no data +// - it is a single bit, "please re-probe now" - and the reply already has a +// transport (the spool file, whose collectedat stamp the service watches +// advance). A kernel event is the smallest thing that crosses the session-0 +// boundary, needs no serialization, no port, no firewall rule, and no cleanup if +// either side dies. A file-watcher trigger would also work but leaves litter on +// disk when the service is killed mid-request. +// +// THE Global\ PREFIX IS LOAD-BEARING. Without it the name is scoped to the +// caller's session, so the service (session 0) and the tray (session 1+) would +// create two unrelated events with the same name and never see each other. That +// failure is silent: both sides "work", nothing is ever refreshed. +// +// THE DACL IS ALSO LOAD-BEARING. The service creates the event as LocalSystem, +// which by default produces an object a non-elevated user cannot even open. The +// tray runs unelevated as ShopFloor by design (an ELEVATED tray is refused by +// the probe, because it would read the wrong drive table). So the creator grants +// Users explicit Synchronize + Modify rights, or the tray gets access-denied on +// open and the refresh silently never happens. +public sealed class UserContextRefreshChannel +{ + // Session-global so session 0 and the interactive session share one object. + public const string EventName = @"Global\ShopDbEndpointAgent.UserContextRefresh"; + + private readonly ILogger logger; + + // Takes the base ILogger rather than ILogger: both the service (generic + // host) and the tray (its own file-backed logger named "Tray") construct + // this, and the tray has no DI container to resolve a typed logger from. + public UserContextRefreshChannel(ILogger logger) + { + this.logger = logger; + } + + // Signal the tray that a fresh probe is wanted. + // + // Returns false when the event could not be opened or created, which is the + // normal state on a machine with nobody signed in and no tray running. The + // caller treats that as "no refresh available" and reports what it already + // has - a stale mapped-drive list is worth strictly more than a missed + // heartbeat, because telemetry IS the liveness signal. + // + // AutoReset, so exactly one waiting tray is released per request and the + // event does not latch on for every subsequent listener iteration. + [SupportedOSPlatform("windows")] + public bool RequestRefresh() + { + try + { + using var handle = OpenOrCreate(out var created); + if (handle is null) + return false; + + handle.Set(); + logger.LogDebug( + "Requested a user-context refresh on {Event} (created={Created})", + EventName, created); + return true; + } + catch (Exception error) + { + // Never throws to the caller. This sits directly on the telemetry + // path, and a refresh is an enhancement to the report, not a + // precondition for sending it. + logger.LogDebug(error, + "Could not request a user-context refresh on {Event}", EventName); + return false; + } + } + + // Tray side: block until the service asks for a refresh, the timeout expires, + // or the token is cancelled. Returns true only for an actual request. + [SupportedOSPlatform("windows")] + public bool WaitForRequest(TimeSpan timeout, CancellationToken token) + { + try + { + using var handle = OpenOrCreate(out _); + if (handle is null) + return false; + + var index = WaitHandle.WaitAny( + new[] { handle, token.WaitHandle }, timeout); + + // Index 1 is cancellation, WaitTimeout is the idle path; only index 0 + // is a genuine request. + return index == 0; + } + catch (Exception error) + { + logger.LogDebug(error, + "Could not wait on the user-context refresh channel {Event}", EventName); + return false; + } + } + + // Open the shared event, creating it if this process is first. + // + // Either side may start first - the service can boot before anyone signs in, + // and the tray can start before the service finishes initializing - so both + // sides use the same create-or-open call rather than designating an owner. + // Whoever wins applies the DACL; the loser opens the existing object. + [SupportedOSPlatform("windows")] + private EventWaitHandle? OpenOrCreate(out bool created) + { + created = false; + try + { + var security = new EventWaitHandleSecurity(); + + // BUILTIN\Users by SID, not by name: on a non-English Windows the + // group is not called "Users", and this agent ships to whatever + // locale the site runs. Same reasoning as the task registration. + var users = new SecurityIdentifier(WellKnownSidType.BuiltinUsersSid, null); + security.AddAccessRule(new EventWaitHandleAccessRule( + users, + EventWaitHandleRights.Synchronize | EventWaitHandleRights.Modify, + AccessControlType.Allow)); + + // FullControl for SYSTEM and Administrators so the service retains + // ownership semantics regardless of which side created the object. + var system = new SecurityIdentifier(WellKnownSidType.LocalSystemSid, null); + security.AddAccessRule(new EventWaitHandleAccessRule( + system, EventWaitHandleRights.FullControl, AccessControlType.Allow)); + + var admins = new SecurityIdentifier( + WellKnownSidType.BuiltinAdministratorsSid, null); + security.AddAccessRule(new EventWaitHandleAccessRule( + admins, EventWaitHandleRights.FullControl, AccessControlType.Allow)); + + return EventWaitHandleAcl.Create( + initialState: false, + mode: EventResetMode.AutoReset, + name: EventName, + createdNew: out created, + eventSecurity: security); + } + catch (UnauthorizedAccessException) + { + // The object exists with a DACL that does not admit this caller. + // Retry read-side-only rights: the tray only ever needs to wait, and + // an event created by an older agent build may not grant Modify. + try + { + return EventWaitHandle.OpenExisting(EventName); + } + catch (Exception error) + { + logger.LogDebug(error, + "The user-context refresh event {Event} exists but is not accessible", + EventName); + return null; + } + } + } +} diff --git a/agent/ShopDbEndpointAgent.Core/UserContextSpool.cs b/agent/ShopDbEndpointAgent.Core/UserContextSpool.cs index b036298c..8bb5e8a3 100644 --- a/agent/ShopDbEndpointAgent.Core/UserContextSpool.cs +++ b/agent/ShopDbEndpointAgent.Core/UserContextSpool.cs @@ -221,6 +221,72 @@ public sealed class UserContextSpool public Dictionary? ConsumeResult(string resultPath) => TryReadResult(resultPath); + // The collectedat stamp on disk, or null when there is no readable stamp. + // + // This is how the service PROVES a refresh actually happened rather than + // assuming it. Signalling the tray is asynchronous and the probe takes a + // second or two, so a caller that simply slept would either send the old + // spool anyway or pad the delay and hope. Watching this value advance past + // the pre-request stamp is the only honest way to know the file in front of + // us is the answer to OUR request and not the previous one. + // + // Deliberately NOT routed through TryReadResult: this must report the stamp + // of whatever is on disk, including a spool that is expired, elevated, or + // from the wrong account. Those are all rejected later for reporting, but + // for ordering we only care whether the file was rewritten - conflating "not + // usable" with "not fresh" would make the service wait out the full timeout + // on every cycle for a machine whose spool is being rejected for some other + // reason. + // + // Also deliberately NOT the file's LastWriteTime, for the reason IsExpired + // gives: a spool can be copied or restored with a roaming profile and carry + // a misleading filesystem timestamp. The stamp inside the file is written by + // the probe itself. + public DateTimeOffset? TryReadCollectedAt(string resultPath) + { + try + { + if (!File.Exists(resultPath)) + return null; + + var info = new FileInfo(resultPath); + if (info.Length == 0 || info.Length > MaxResultBytes) + return null; + + var json = File.ReadAllText(resultPath); + if (string.IsNullOrWhiteSpace(json)) + return null; + + var parsed = JsonSerializer.Deserialize>(json); + if (parsed is null + || !parsed.TryGetValue("collectedat", out var raw) + || raw is null) + { + return null; + } + + if (DateTimeOffset.TryParse( + raw.ToString(), + System.Globalization.CultureInfo.InvariantCulture, + System.Globalization.DateTimeStyles.RoundtripKind, + out var collectedat)) + { + return collectedat; + } + + return null; + } + catch (Exception exception) + { + // A torn read while the probe is mid-write is the expected failure + // here and is not worth a warning: the caller polls, so the next + // attempt a moment later sees the completed file. + logger.LogDebug(exception, + "Could not read the user-context collectedat stamp at {Path}", resultPath); + return null; + } + } + // Reject a spool that some other account produced. // // Pinning the path guarantees the file lands where the reader looks, but it diff --git a/agent/ShopDbEndpointAgent.PolicyProbe/Program.cs b/agent/ShopDbEndpointAgent.PolicyProbe/Program.cs index bee73158..ddaaee67 100644 --- a/agent/ShopDbEndpointAgent.PolicyProbe/Program.cs +++ b/agent/ShopDbEndpointAgent.PolicyProbe/Program.cs @@ -92,6 +92,57 @@ else var shortRoot = Path.Combine(@"C:\PROGRA~1", "LppFixture", "good.exe"); Check("8.3 short name resolving into the approved root", true, shortRoot, policy); +// --- ScheduledTaskPolicy ---------------------------------------------------- +// Pure string logic, unlike LaunchPathPolicy: a task path names an entry in the +// Task Scheduler namespace, not a file, so there is no disk state to plant. The +// cases that matter are the ones where a candidate LOOKS like an approved entry +// as a string but is not the same task. + +void CheckTask(string label, bool expectAccept, string candidate, ScheduledTaskPolicy policy) +{ + var ok = policy.TryValidate(candidate, out var full, out var reason); + var pass = ok == expectAccept; + if (!pass) failures++; + var verdict = pass ? "PASS" : "FAIL"; + var got = ok ? $"accepted -> {full}" : $"rejected ({reason})"; + Console.WriteLine($"{verdict} {label}"); + Console.WriteLine($" want={(expectAccept ? "accept" : "reject")} got={got}"); +} + +Console.WriteLine(); +Console.WriteLine("--- ScheduledTaskPolicy ---"); + +var taskPolicy = new ScheduledTaskPolicy(new[] { @"\ReportIP", @"\SFLD\" }); + +CheckTask("exact approved task", true, @"\ReportIP", taskPolicy); +// Task Scheduler is case-insensitive, so the policy must be too, or the same +// task typed differently would be refused. +CheckTask("approved task, different case", true, @"\reportip", taskPolicy); +// The operator should not have to know the rooted form. +CheckTask("approved task without leading separator", true, "ReportIP", taskPolicy); +CheckTask("approved task with forward slashes", true, "/ReportIP", taskPolicy); +// The prefix-suffix confusion this policy exists to prevent: "\ReportIP" must +// NOT authorize "\ReportIPEvil". +CheckTask("sibling task sharing an approved prefix", false, @"\ReportIPEvil", taskPolicy); +CheckTask("unapproved task", false, @"\Evil", taskPolicy); +CheckTask("empty task path", false, "", taskPolicy); +// Folder entries authorize everything beneath them, but not a sibling folder +// whose name merely starts the same way. +CheckTask("task inside an approved folder", true, @"\SFLD\ApplyDSCConfig", taskPolicy); +CheckTask("folder itself", true, @"\SFLD", taskPolicy); +CheckTask("sibling folder sharing the prefix", false, @"\SFLDEvil\Thing", taskPolicy); +// Traversal would let an approved prefix string-match something else entirely. +CheckTask("traversal out of an approved folder", false, @"\SFLD\..\Evil", taskPolicy); +// Defense in depth: the invocation uses ArgumentList, but a task path with +// these characters is not one we should be starting. +CheckTask("argument-injection characters", false, "\\ReportIP\" & calc", taskPolicy); +CheckTask("control character", false, "\\Report\u0000IP", taskPolicy); +// An empty override must fall back to the shipped defaults, never to "allow +// everything". +var defaultPolicy = new ScheduledTaskPolicy(Array.Empty()); +CheckTask("shipped default is honored when override is empty", true, @"\ReportIP", defaultPolicy); +CheckTask("empty override does not mean allow-all", false, @"\Evil", defaultPolicy); + Console.WriteLine(); Console.WriteLine(failures == 0 ? "ALL EXPECTATIONS MET" : $"{failures} EXPECTATION(S) NOT MET"); return failures == 0 ? 0 : 1; diff --git a/agent/ShopDbEndpointAgent.PolicyProbe/ShopDbEndpointAgent.PolicyProbe.csproj b/agent/ShopDbEndpointAgent.PolicyProbe/ShopDbEndpointAgent.PolicyProbe.csproj index 9112673f..1736af61 100644 --- a/agent/ShopDbEndpointAgent.PolicyProbe/ShopDbEndpointAgent.PolicyProbe.csproj +++ b/agent/ShopDbEndpointAgent.PolicyProbe/ShopDbEndpointAgent.PolicyProbe.csproj @@ -16,5 +16,9 @@ + + diff --git a/agent/ShopDbEndpointAgent.Tray/ShopDbEndpointAgent.Tray.csproj b/agent/ShopDbEndpointAgent.Tray/ShopDbEndpointAgent.Tray.csproj index a1e05a25..90a0532c 100644 --- a/agent/ShopDbEndpointAgent.Tray/ShopDbEndpointAgent.Tray.csproj +++ b/agent/ShopDbEndpointAgent.Tray/ShopDbEndpointAgent.Tray.csproj @@ -54,7 +54,24 @@ only way to enter the operator's existing session unelevated. Probe exit code 2 - the agent refusing to collect while elevated - is reported by name instead of as a bare code. Requires agent 1.10.1. --> - 1.3.0 + + 1.4.0 true win-x64 true diff --git a/agent/ShopDbEndpointAgent.Tray/TrayContext.cs b/agent/ShopDbEndpointAgent.Tray/TrayContext.cs index f7b05997..41039414 100644 --- a/agent/ShopDbEndpointAgent.Tray/TrayContext.cs +++ b/agent/ShopDbEndpointAgent.Tray/TrayContext.cs @@ -59,6 +59,29 @@ internal sealed class TrayContext : ApplicationContext private bool userContextStarted; private bool collectingUserContext; + // Lets the service ask this tray to re-probe on demand, so a "Refresh + // Status" click in the WebUI produces genuinely current session data instead + // of whatever the last 30-minute tick happened to capture. The service + // cannot collect this itself at any price - mapped drives live on the + // operator's logon token - so the only way for an on-demand refresh to be + // accurate is for the request to end up here, in the operator's session. + private readonly UserContextRefreshChannel? refreshChannel; + private readonly CancellationTokenSource refreshCancellation = new(); + + // How long each wait blocks before looping. The wait returns false on + // timeout and the loop simply waits again, so this is not a deadline for + // anything - it just bounds how long the handle stays open, which lets a + // channel that was somehow broken get re-opened on the next pass instead of + // leaving the tray deaf until it restarts. + private static readonly TimeSpan RefreshWaitTimeout = TimeSpan.FromMinutes(5); + + // The UI thread's context, captured in the constructor. The listener runs on + // a background thread, but every probe attempt is marshalled back here so + // that collectingUserContext is only ever read or written from one thread. + // Without this the timer tick and a service request could both pass the + // guard and launch two probes at the same spool file. + private readonly SynchronizationContext? uiContext; + // The tray logs into the operator's own profile // (C:\Users\ShopFloor\ShopDB\Logs\-trayagent.log), NOT into the // service's ProgramData directory. The operator cannot write there, and the @@ -122,6 +145,88 @@ internal sealed class TrayContext : ApplicationContext }; userContextTimer.Tick += OnUserContextTimer; userContextTimer.Start(); + + uiContext = SynchronizationContext.Current; + + // Best-effort. A tray that cannot open the channel still probes on its + // timer, so the failure mode is "refresh is not immediate", not "session + // data stops working" - which is why this is caught and logged rather + // than allowed to take the tray down at startup. + try + { + refreshChannel = new UserContextRefreshChannel( + logProvider.CreateLogger(nameof(UserContextRefreshChannel))); + StartRefreshListener(); + } + catch (Exception exception) + { + logger.LogWarning(exception, + "Could not open the user-context refresh channel; on-demand " + + "refresh requests from the service will be ignored and session " + + "data will only update on the {Interval} timer.", + UserContextInterval); + } + } + + // Waits for service refresh requests for the life of the tray. + // + // A dedicated background thread rather than a Task: this blocks on a kernel + // wait handle essentially forever, which is exactly the workload that should + // not sit on a thread-pool thread. IsBackground keeps it from holding the + // process open at exit. + private void StartRefreshListener() + { + var thread = new Thread(() => + { + var token = refreshCancellation.Token; + while (!token.IsCancellationRequested) + { + try + { + if (!refreshChannel!.WaitForRequest(RefreshWaitTimeout, token)) + continue; + + logger.LogInformation( + "The service requested a user-context refresh."); + + // Marshalled, not awaited: the service is not waiting on a + // reply through this channel, it is watching the spool file's + // collectedat stamp. Our only job is to get the probe started + // on the UI thread and go back to waiting. + var post = uiContext; + if (post is not null) + post.Post(_ => _ = CollectUserContextAsync(), null); + else + _ = CollectUserContextAsync(); + } + catch (OperationCanceledException) + { + return; + } + catch (Exception exception) + { + // Never let a bad wait kill the listener - if this thread + // exits, on-demand refresh is silently dead for the rest of + // the session with no other symptom. + logger.LogWarning(exception, + "The user-context refresh listener failed; retrying."); + try + { + Thread.Sleep(TimeSpan.FromSeconds(5)); + } + catch (Exception) + { + return; + } + } + } + }) + { + IsBackground = true, + Name = "ShopDB user-context refresh listener", + }; + + thread.Start(); } // Fires once after the startup delay, then re-arms at the steady-state @@ -136,8 +241,27 @@ internal sealed class TrayContext : ApplicationContext userContextTimer.Interval = (int)UserContextInterval.TotalMilliseconds; } + await CollectUserContextAsync(); + } + + // Runs the probe once and returns when the spool has been rewritten. + // + // Shared by all three callers - the cadence timer, an on-demand service + // request, and "Send Status now" - so there is exactly one place that knows + // how to invoke the probe and how to interpret its exit codes. Returns true + // only when a spool file is actually on disk afterwards, which is what lets + // Send Status decide whether it is worth waiting on. + private async Task CollectUserContextAsync() + { if (collectingUserContext) - return; + { + // Another probe is already in flight against the same spool path. + // Its result is at most a second or two old and satisfies whatever + // this caller wanted, so coalescing is correct here, not a shortcut. + logger.LogDebug( + "A user-context probe is already running; skipping this request."); + return false; + } var agentExe = ResolveAgentExe(); if (agentExe is null) @@ -148,7 +272,7 @@ internal sealed class TrayContext : ApplicationContext "Skipping the user-context probe: {Agent} was not found beside the tray " + "({Beside}) or at {Installed}.", AgentExeName, AppContext.BaseDirectory, DefaultInstallDir); - return; + return false; } collectingUserContext = true; @@ -168,6 +292,7 @@ internal sealed class TrayContext : ApplicationContext logger.LogInformation( "The user-context probe finished; spool present at {Spool}.", resultPath); + return true; } else if (exitCode == 0) { @@ -195,12 +320,15 @@ internal sealed class TrayContext : ApplicationContext "The user-context probe exited with code {ExitCode}; expected a " + "spool at {Spool}.", exitCode, resultPath); } + + return false; } catch (Exception exception) { // No longer silent. The agent logs its own detail, but if the process // could not even be started the agent never ran and never logged. logger.LogWarning(exception, "Could not run the user-context probe."); + return false; } finally { @@ -330,6 +458,18 @@ internal sealed class TrayContext : ApplicationContext try { + // Probe BEFORE reporting, and await it. The operator clicking this + // has just changed something - remapped a drive, signed in - and + // expects the result to show up. --once reads the spool file, so a + // probe started after it, or not awaited, would publish the previous + // probe's answer and the operator would click again and see the same + // stale card. + // + // Not fatal if it fails: a report with older session data still + // carries the heartbeat, inventory and events, which is most of what + // this button is for. + await CollectUserContextAsync(); + int exitCode = await RunAgentAsync(agentExe, "--once"); if (exitCode == 0) ShowBalloon("Send Status", "Status sent to ShopDB.", @@ -508,6 +648,13 @@ internal sealed class TrayContext : ApplicationContext { userContextTimer.Stop(); userContextTimer.Dispose(); + + // Cancel and let the listener unwind on its own. The channel opens + // and closes its handle inside each wait rather than holding one, so + // there is nothing here to dispose. + refreshCancellation.Cancel(); + refreshCancellation.Dispose(); + trayIcon.Dispose(); brandIcon.Dispose(); } diff --git a/agent/ShopDbEndpointAgent/Program.cs b/agent/ShopDbEndpointAgent/Program.cs index c3472cb0..ffa770bd 100644 --- a/agent/ShopDbEndpointAgent/Program.cs +++ b/agent/ShopDbEndpointAgent/Program.cs @@ -304,6 +304,13 @@ static void RegisterCoreServices(IServiceCollection services, AgentConfig config services.AddSingleton(sp => new UserContextSpool( sp.GetRequiredService>())); + // The ask-the-tray side of the same split. Registering this is NOT a + // loophole around the paragraph above: the service still cannot collect + // session data, it can only request that the process which can does so, and + // then read the file that appears. The probe stays out of this container. + services.AddSingleton(sp => new UserContextRefreshChannel( + sp.GetRequiredService>())); + services.AddSingleton(sp => new EndpointCollector( sp.GetRequiredService>(), config, @@ -321,7 +328,9 @@ static void RegisterCoreServices(IServiceCollection services, AgentConfig config agentVersion, sp.GetRequiredService>(), sp.GetService(), - sp.GetRequiredService())); + sp.GetRequiredService(), + sp.GetRequiredService(), + sp.GetRequiredService())); // Command channel: outbound long-poll for allowlisted WebUI commands, run // through the fail-closed executor. Hosted alongside telemetry but on its diff --git a/agent/ShopDbEndpointAgent/ShopDbEndpointAgent.csproj b/agent/ShopDbEndpointAgent/ShopDbEndpointAgent.csproj index 62a24fea..ec456c3b 100644 --- a/agent/ShopDbEndpointAgent/ShopDbEndpointAgent.csproj +++ b/agent/ShopDbEndpointAgent/ShopDbEndpointAgent.csproj @@ -131,7 +131,26 @@ (AgentConfig.LaunchPaths, shipped defaults overridable per site). 1.1.0: generic, data-driven service_*/process_* actions carry their target over the wire, so new policies/apps need no agent rebuild. --> - 1.10.1 + + 1.11.0 true win-x64 true diff --git a/agent/utilities/shopdb-dellupdates.ps1 b/agent/utilities/shopdb-dellupdates.ps1 new file mode 100644 index 00000000..98451c6b --- /dev/null +++ b/agent/utilities/shopdb-dellupdates.ps1 @@ -0,0 +1,119 @@ +<# +.SYNOPSIS + ShopDB agent utility: reports pending Dell Command | Update items. + +.DESCRIPTION + Discovered and executed by PowerShellUtilityRunner on the telemetry cadence. + Emits a single JSON object on stdout, which the agent attaches to the report + as `dellupdates`. Never throws and never writes anything but JSON: a failure + here must not break a telemetry run, so every error path still emits a valid + object with a `status` explaining why the list is empty. + + THIS SCRIPT DOES NOT SCAN. It only reads the log a previous scan wrote. The + scan itself is fired by the \ShopDB-DellCommandScan scheduled task, which is + what pins dcu-cli.exe's -outputLog to the path below. Keeping the two apart + is deliberate: a telemetry collector that shelled out to dcu-cli would block + the cadence for minutes and hammer Dell's servers every 15 minutes. + + OUTPUT CONTRACT (keep in sync with plugins/endpoints/plugin.py): + status unknown | ok | error - "unknown" means no scan has ever run, + which is NOT the same as "ok" with an empty list (known clean). + scannedat ISO-8601 UTC timestamp of the scan log's last write, or null. + count total pending updates found. + updates up to $MaxUpdates entries of { name, severity, version }. + truncated true when count exceeded $MaxUpdates. + + Dell's own long description text is deliberately NOT carried: it is + sentence-length marketing prose per update, which would add roughly an order + of magnitude to every report for no operational value. Name plus severity is + what an operator decides on. +#> + +$ErrorActionPreference = 'Stop' + +# Cap the list rather than the payload. A machine that is years behind can have +# dozens of pending updates, and an unbounded list would bloat every report on +# the cadence. Mirrors the MaxReportedConnections precedent in CommandExecutor: +# a truncated answer is more useful than a rejected payload. +$MaxUpdates = 25 + +# Must match the -outputLog path set in the \ShopDB-DellCommandScan task +# definition. ShopDB-initiated scans write HERE rather than to Dell's default +# location, which is what keeps them distinguishable from the corporate-managed +# DCU tasks that also run on these machines. +$ScanLog = Join-Path $env:ProgramData 'ShopDB\EndpointAgent\dellcommand\scan.xml' + +function Write-Result { + param($Status, $ScannedAt, $Count, $Updates, $Truncated) + # Compress so the payload carries no pretty-printing whitespace. Depth 3 + # covers the nested updates array. + [pscustomobject]@{ + status = $Status + scannedat = $ScannedAt + count = $Count + updates = @($Updates) + truncated = [bool]$Truncated + } | ConvertTo-Json -Depth 3 -Compress +} + +try { + if (-not (Test-Path -LiteralPath $ScanLog)) { + # No scan has run yet. This is the normal state for a freshly adopted + # endpoint, not an error, but it must stay distinct from "scanned and + # found nothing" or the UI would claim a machine is up to date when + # nobody has ever checked. + Write-Result -Status 'unknown' -ScannedAt $null -Count 0 -Updates @() -Truncated $false + return + } + + $logFile = Get-Item -LiteralPath $ScanLog + $scannedAt = $logFile.LastWriteTimeUtc.ToString('o') + + [xml]$report = Get-Content -LiteralPath $ScanLog -Raw + + # dcu-cli's scan report shape has varied across DCU versions, so select the + # update nodes by name anywhere in the tree rather than by a fixed path. + $nodes = @($report.SelectNodes('//update')) + if ($nodes.Count -eq 0) { + $nodes = @($report.SelectNodes('//Update')) + } + + $updates = foreach ($node in $nodes) { + # Attribute casing also varies by version; take whichever is present and + # fall back rather than emitting a null-filled row. + $name = $node.name + if (-not $name) { $name = $node.Name } + if (-not $name) { $name = $node.display } + if (-not $name) { continue } + + $severity = $node.urgency + if (-not $severity) { $severity = $node.Urgency } + if (-not $severity) { $severity = $node.criticality } + + $version = $node.version + if (-not $version) { $version = $node.Version } + + [pscustomobject]@{ + name = [string]$name + severity = if ($severity) { [string]$severity } else { 'unknown' } + version = if ($version) { [string]$version } else { $null } + } + } + + $updates = @($updates) + $count = $updates.Count + $truncated = $count -gt $MaxUpdates + if ($truncated) { + $updates = $updates[0..($MaxUpdates - 1)] + } + + Write-Result -Status 'ok' -ScannedAt $scannedAt -Count $count ` + -Updates $updates -Truncated $truncated +} +catch { + # Emit a valid object rather than failing: the runner would log a non-JSON + # utility and drop it, which looks identical to "never ran" on the server. + # An explicit error status tells an operator the scan log exists but could + # not be read, which is a different problem from no scan at all. + Write-Result -Status 'error' -ScannedAt $null -Count 0 -Updates @() -Truncated $false +} diff --git a/plugins/computers/frontend/views/PCDetail.vue b/plugins/computers/frontend/views/PCDetail.vue index e8c5bce1..da3a12a1 100644 --- a/plugins/computers/frontend/views/PCDetail.vue +++ b/plugins/computers/frontend/views/PCDetail.vue @@ -423,6 +423,42 @@
+ +
+
+

Corporate Tools

+

+ Starts a scheduled task that is already registered on this computer. + Each task runs in the security context its own definition specifies. +

+ +

+ These actions take effect immediately on the live computer and + cannot be undone from here. A Dell update run can reboot the machine + and may interrupt the person using it. +

+
+
+
+
{{ tool.label }}
+

{{ tool.warning }}

+
{{ tool.description }}
+
+ +
+
+
+
@@ -575,6 +611,13 @@ const hasCloudIdentity = computed(() => portalTemplatesLoaded.value && Boolean( // populatedGroups comes from PluginAssetPanels, which knows which panels // actually returned data; Software is additionally true when core has an // installed-applications list of its own. +// Gates the Corporate Tools tab. endpoints.deploy currently bundles +// scheduledtask_run with run_script/update_app - see the note beside the +// permission declaration in plugins/endpoints/plugin.py if that gets split. +// Declared ahead of detailtabs because the watch below evaluates that computed +// during setup, which reads canDeploy before this line would otherwise run. +const canDeploy = computed(() => authStore.hasPermission('endpoints.deploy')) + const detailtabs = computed(() => { const filled = assetPanels.value?.populatedGroups ?? new Set() const has = (key) => filled.has ? filled.has(key) : false @@ -595,6 +638,13 @@ const detailtabs = computed(() => { tabs.push({ key: 'software', label: 'Software', count: installedApps.value.length || null }) } if (has('activity')) tabs.push({ key: 'activity', label: 'Activity' }) + // Corporate Tools is permission-gated rather than data-gated: it is a set of + // fixed actions, not a rendering of collected telemetry, so there is no + // populatedGroup to key off. Hidden without endpoints.deploy because every + // button here is an elevated command; the server re-checks regardless. + if (canDeploy.value && liveness.value) { + tabs.push({ key: 'corptools', label: 'Corporate Tools' }) + } return tabs }) @@ -805,6 +855,93 @@ async function confirmRestart() { } } +// Fixed set, deliberately not user-editable from this page: these are the task +// paths the agent ships as its approved list (ScheduledTaskPolicy). Adding one +// here without adding it to the agent's allowlist gets a clean rejection from +// the endpoint rather than an execution, which is the intended failure mode. +const corporateTools = [ + { + task: '\\ReportIP', + label: 'Report IP', + description: 'Re-runs the corporate IP reporting agent on this computer.', + }, + { + task: '\\SFLD-ApplyDSCConfig', + label: 'Apply DSC Configuration', + description: 'Re-applies the shop floor DSC baseline. Runs as SYSTEM and ' + + 'may change system configuration.', + }, + { + task: '\\SFLD - Consume Credentials', + label: 'Consume Credentials', + description: 'Runs the credential consumption task in the logged-on ' + + 'user\'s session. Requires a user to be signed in.', + }, + // Dell Command | Update, split into two entries because they are two + // utilities with very different consequences: the scan only reports, the + // update installs and may reboot. Keeping them as separate buttons means the + // destructive one is never a field edit away from the safe one. + { + task: '\\ShopDB-DellCommandScan', + label: 'Dell Command: Scan', + description: 'Scans for available Dell driver and firmware updates. ' + + 'Reports only - installs nothing and does not reboot.', + }, + { + task: '\\ShopDB-DellCommandUpdate', + label: 'Dell Command: Apply Updates', + // Persistent breakglass warning. Tanium is the primary update utility for + // this fleet; this tool exists only for when Tanium has failed. The text is + // rendered in the card at all times (not behind a hover or the confirm + // dialog) so the operator cannot reach Run without having seen it. + warning: 'These updates are meant to be executed by the primary utility - ' + + 'Tanium. This tool should only be used in the event of a failed ' + + 'update or a problem with Tanium. This is a breakglass update ' + + 'utility to account for critical failures in the primary ' + + 'management solution.', + description: 'Installs available Dell driver and firmware updates. Runs as ' + + 'SYSTEM and MAY REBOOT this computer when it finishes.', + destructive: true, + }, +] + +const firingTask = ref('') + +// Elevated: queues a scheduledtask_run. The confirm gate mirrors +// confirmRestart - these start real work on a live machine and some of them +// change system state - and the server re-checks endpoints.deploy regardless. +// +// Tools marked destructive get a TYPED confirmation rather than an OK button, +// for the same reason Restart has its own gate: applying firmware updates can +// reboot a machine that may be mid-cycle on the shop floor, and a misclick +// should not be able to do that. +async function confirmRunTask(tool) { + const label = computer.value?.assetnumber || computer.value?.name || 'this PC' + if (tool.destructive) { + const typed = prompt(`${tool.label} on ${label}\n\n` + + `${tool.description}\n\n` + + 'Type RUN to confirm.') + if (typed !== 'RUN') return + } else if (!confirm(`Run "${tool.label}" on ${label}?\n\n` + + `${tool.description}\n\n` + + 'The task starts immediately on the endpoint.')) { + return + } + firingTask.value = tool.task + try { + await endpointsApi.enqueueCommand( + computer.value.assetid, 'scheduledtask_run', tool.task) + // "Started", not "finished": the agent reports success as soon as Task + // Scheduler accepts the task, so this never means the work is done. Any + // result the task produces comes back through telemetry, not this command. + toast.success(`${tool.label} started on ${label}`) + } catch (error) { + toast.error(apiError(error, `Failed to queue ${tool.label}`)) + } finally { + firingTask.value = '' + } +} + // Retire, not delete: the server deactivates the asset, so the PC // leaves every list and report while its history and relationships stay put. // Reversible by an admin setting isactive back to true. @@ -995,6 +1132,60 @@ async function confirmRetire() { } .triage-chip.ok .triage-dot { background: var(--success); } .triage-chip.warning .triage-dot { background: var(--warning); } + +/* Corporate Tools: a plain action list, not a data table - each row is a + trigger with the consequence stated next to it, so the operator reads what + it does before the confirm gate rather than after. */ +.corptools-intro { + margin: 0 0 1rem; + color: var(--text-secondary); + font-size: 0.875rem; +} +/* Warning weight without alert-banner styling: it is permanent, so a loud + dismissible-looking banner would be tuned out within a day. The left rule and + warning tint keep it legible as a standing caution. */ +.corptools-warning { + margin: 0 0 1rem; + padding: 0.5rem 0.75rem; + border-left: 3px solid var(--warning, #d97706); + background: var(--bg-secondary); + color: var(--text-primary); + font-size: 0.875rem; +} +.corptools-list { + display: flex; + flex-direction: column; + gap: 0.5rem; +} +.corptools-row { + display: flex; + align-items: flex-start; + justify-content: space-between; + gap: 1rem; + padding: 0.75rem; + border: 1px solid var(--border); + border-radius: 4px; +} +.corptools-name { + font-weight: 600; +} +.corptools-desc { + color: var(--text-secondary); + font-size: 0.8125rem; + margin-top: 0.125rem; +} +/* Row-level breakglass notice. Carries the same left-rule treatment as the + card-level warning so it reads as a caution and not as body copy, but sits + inside the row it applies to. */ +.corptools-rowwarning { + margin: 0.25rem 0 0.375rem; + padding: 0.375rem 0.625rem; + border-left: 3px solid var(--danger, #dc2626); + background: var(--bg-secondary); + color: var(--text-primary); + font-size: 0.8125rem; + font-weight: 500; +} .triage-chip.critical .triage-dot { background: var(--danger); } .triage-chip.ok { color: var(--success-dark); } .triage-chip.warning { color: var(--warning-dark); } diff --git a/plugins/endpoints/api/routes.py b/plugins/endpoints/api/routes.py index 2986fd9d..d2e49458 100644 --- a/plugins/endpoints/api/routes.py +++ b/plugins/endpoints/api/routes.py @@ -494,6 +494,60 @@ def _drift_for_state(state): return parsed if isinstance(parsed, list) else [] +def _dell_updates_summary(state): + """Parse the cached Dell Command | Update summary off an EndpointState. + + dellupdatesjson is written on check-in from the shopdb-dellupdates agent + utility, which reads the log the \\ShopDB-DellCommandScan task produces. + Null means the endpoint has never reported a scan. Tolerant of a malformed + blob so a bad cache never breaks the panel. + """ + raw = getattr(state, 'dellupdatesjson', None) if state else None + if not raw: + return None + try: + parsed = json.loads(raw) + except (TypeError, ValueError): + return None + return parsed if isinstance(parsed, dict) else None + + +def _dell_updates_value(state): + """Render the informational 'Pending Dell updates' status value. + + Informational only - this row never claims an endpoint needs attention, + because a pending optional driver is not a support problem. Three states are + kept distinct on purpose, and all three are VISIBLE: + + never scanned -> 'Never scanned'. Deliberately not 'No': those are + different facts, and claiming 'No' would tell an operator + the machine is current when nobody has ever checked. It + is also not hidden - a missing row reads as a broken + panel, so the unknown state says so out loud. + scanned clean -> 'No'. + pending -> 'Yes (n)', with the count because one optional update and + fourteen urgent ones warrant different urgency. + """ + summary = _dell_updates_summary(state) + if not summary: + return 'Never scanned' + status = summary.get('status') + if status == 'error': + # The scan log exists but could not be read. A different problem from + # never having scanned, and worth saying so rather than implying the + # endpoint was never checked. + return 'Unknown (scan unreadable)' + if status != 'ok': + return 'Never scanned' + try: + count = int(summary.get('count') or 0) + except (TypeError, ValueError): + return 'Unknown (scan unreadable)' + if count <= 0: + return 'No' + return 'Yes ({})'.format(count) + + @endpoints_bp.route('/asset//status', methods=['GET']) @require_permission('endpoints.view') def panel_status(asset_id: int): @@ -530,6 +584,11 @@ def panel_status(asset_id: int): ] fields = [f for f in fields if f['value'] not in (None, '')] + # Informational only, and appended after the null-filter above because it + # always renders - including an explicit "Never scanned", which a hidden row + # would leave looking like a broken panel. + fields.append(field('Pending Dell updates', _dell_updates_value(state))) + # Warn-only reconciler summary: monitors site policy expects to be running # that the last check-in did not report as running. Surfaced as a status # field so an operator sees the drift on the panel; nothing is auto-started. diff --git a/plugins/endpoints/api/siteagent.py b/plugins/endpoints/api/siteagent.py index 1e2bf0f5..56554537 100644 --- a/plugins/endpoints/api/siteagent.py +++ b/plugins/endpoints/api/siteagent.py @@ -136,6 +136,12 @@ _BASE_APPSETTINGS = { 'UDC.exe': 'C:\\Program Files\\UDC\\UDC.exe', 'agent.exe': 'C:\\ProgramData\\udc\\mtconnect_udc\\agent\\run.bat', }, + # Approved scheduled task paths for scheduledtask_run. Shipped empty, which + # means the agent falls back to ScheduledTaskPolicy.DefaultApprovedTaskPaths + # (the Corporate Tools set). Listing entries here REPLACES those defaults, so + # a site can narrow or widen the set without an agent rebuild. Anything not + # matched fails closed. + 'approvedtaskpaths': [], } _README = """ShopDB endpoint agent (site build) @@ -224,6 +230,11 @@ param( # reliable way to collect them; see the registration block below. [string] $UserContextTaskName = "ShopDB user-context probe", [int] $UserContextTaskMinutes = 30, + + # The account the probe task runs as. Must match the account whose profile + # the agent reads the spool from (FileLoggerProvider.ShopFloorUserName), or + # the probe writes session data the service never looks at. + [string] $UserContextTaskUser = "ShopFloor", [switch] $NoUserContextTask ) @@ -385,14 +396,23 @@ if (-not $NoTray) {{ }} }} -# 2b. Register the user-context probe as a scheduled task owned by the Users -# GROUP. This is what makes mapped drives, user listeners and idle time +# 2b. Register the user-context probe as a scheduled task owned by the SHOPFLOOR +# USER. This is what makes mapped drives, user listeners and idle time # collectable: it is the only mechanism that starts a process in the operator's # own session with the operator's own token. # # RunLevel Limited is load-bearing, not a hardening nicety - the unelevated -# token is precisely what sees the operator's mapped drives. GroupId rather -# than UserId so the task runs as WHOEVER signs in, each in their own session. +# token is precisely what sees the operator's mapped drives. +# +# UserId, NOT GroupId. A group-principal task is registered against the group +# rather than against any one account, and a member of that group cannot invoke +# it on demand: Start-ScheduledTask from the ShopFloor session returns access +# denied, so the task only ever ran on its own trigger. That is exactly the bug +# where "Refresh Status" appeared to do nothing. Registering to the specific +# account makes the task startable by that account, which is what both the tray +# and any manual retry depend on. The group form is kept only as a fallback for +# a host where the account cannot be resolved - it still collects on a timer, +# it just cannot be kicked on demand. $taskState = "skipped" if (-not $NoUserContextTask) {{ Write-Host " registering user-context probe task ..." @@ -410,15 +430,26 @@ if (-not $NoUserContextTask) {{ }} $taskSettings = New-ScheduledTaskSettingsSet -AllowStartIfOnBatteries -DontStopIfGoingOnBatteries -StartWhenAvailable -ExecutionTimeLimit (New-TimeSpan -Minutes 5) - # BUILTIN\\Users by SID first, then by name: the SID is locale - # independent (on non-English Windows the group is not called "Users"), - # but a hardened host can still reject it. - foreach ($groupId in @("S-1-5-32-545", "BUILTIN\\Users")) {{ + # Try the machine-qualified account first, then the bare name. Interactive + # LogonType so the task joins the existing interactive session instead of + # creating a separate one with its own (empty) drive table. + $taskCandidates = @( + @{{ Kind = "user"; Id = "$env:COMPUTERNAME\\$UserContextTaskUser" }}, + @{{ Kind = "user"; Id = $UserContextTaskUser }}, + @{{ Kind = "group"; Id = "S-1-5-32-545" }}, + @{{ Kind = "group"; Id = "BUILTIN\\Users" }} + ) + foreach ($candidate in $taskCandidates) {{ try {{ - $taskPrincipal = New-ScheduledTaskPrincipal -GroupId $groupId -RunLevel Limited -ErrorAction Stop + if ($candidate.Kind -eq "user") {{ + $taskPrincipal = New-ScheduledTaskPrincipal -UserId $candidate.Id -LogonType Interactive -RunLevel Limited -ErrorAction Stop + }} + else {{ + $taskPrincipal = New-ScheduledTaskPrincipal -GroupId $candidate.Id -RunLevel Limited -ErrorAction Stop + }} Register-ScheduledTask -TaskName $UserContextTaskName -Action $taskAction -Trigger $taskTriggers -Principal $taskPrincipal -Settings $taskSettings -Force -ErrorAction Stop | Out-Null $taskRegistered = $true - $taskDetail = $groupId + $taskDetail = "$($candidate.Kind):$($candidate.Id)" break }} catch {{ $taskDetail = $_.Exception.Message }} @@ -440,12 +471,17 @@ if (-not $NoUserContextTask) {{ $taskState = "UNVERIFIED:elevated" Write-Warning " user-context task runs ELEVATED; mapped drives would be collected from the wrong token." }} - elseif (-not $verified.Principal.GroupId) {{ - $taskState = "UNVERIFIED:no group principal" - Write-Warning " user-context task has no group principal; it would run as one specific account." + elseif ($verified.Principal.GroupId) {{ + # Registered, will collect on its triggers, but cannot be started on + # demand by the operator - so "Refresh Status" will fall back to the + # timer. Surfaced rather than reported as ok, because the symptom + # (stale session data that never responds to a refresh) is otherwise + # indistinguishable from a broken probe. + $taskState = "DEGRADED:group principal $($verified.Principal.GroupId)" + Write-Warning " user-context task fell back to a GROUP principal; on-demand refresh will not work, only the $UserContextTaskMinutes-minute interval." }} else {{ - $taskState = "ok:$($verified.Principal.GroupId)/$($verified.Principal.RunLevel)" + $taskState = "ok:$($verified.Principal.UserId)/$($verified.Principal.RunLevel)" Write-Host " user-context task registered and verified: $taskState" }} diff --git a/plugins/endpoints/commands.py b/plugins/endpoints/commands.py index 74fd0d9b..c1cfa78f 100644 --- a/plugins/endpoints/commands.py +++ b/plugins/endpoints/commands.py @@ -83,12 +83,30 @@ COMMAND_ALLOWLIST = { 'Read-only diagnostic; the standard report carries only ' 'listening ports because established connections churn.', }, + 'scheduledtask_run': { + 'label': 'Run scheduled task', + 'description': 'Start an already-registered Windows scheduled task on ' + 'the endpoint by its task path (for example ' + '\\ReportIP). The agent does not create, modify, or ' + 'define tasks - it only starts one that already exists, ' + 'and only when the path clears the agent-side approved ' + 'task allowlist. The task runs in whatever context its ' + 'own registered definition specifies, not the agent\'s.', + 'targetkind': 'scheduledtask', + }, 'run_script': { 'label': 'Run script', 'description': 'Run a ShopDB-hosted script on the endpoint. The agent ' 'fetches the script body by digest using its collector ' 'credential and runs it in the script\'s configured ' - 'context (system or the logged-on user).', + 'context (system or the logged-on user). CAUTION: a ' + 'script sent this way runs as SYSTEM unless its ' + 'descriptor pins the user context, so this is arbitrary ' + 'code execution on the endpoint by design. Grant ' + 'endpoints.deploy only to the most trusted operators, ' + 'and gate the UI behind an explicit confirm the way ' + 'system_restart is gated (see confirmRestart in ' + 'PCDetail.vue).', 'targetkind': 'script', }, 'update_app': { @@ -102,6 +120,35 @@ COMMAND_ALLOWLIST = { } +# Dell Command | Update: TWO utilities, deliberately two separate actions. +# A scan reports what is available and changes nothing; an update installs and +# can reboot. Collapsing them into one parameterized action would make the +# destructive one reachable by editing a field, so they stay distinct and the +# UI can gate them differently. +# +# Both are driven through scheduledtask_run against ShopDB-owned task +# definitions (\ShopDB-DellCommandScan and \ShopDB-DellCommandUpdate, already in +# ScheduledTaskPolicy's shipped defaults) rather than a bespoke verb. The task +# definition - not the agent - pins the dcu-cli.exe path, its arguments, and its +# output path, which is why no new agent code is needed to run them. +# +# OUTPUT PATH CONVENTION. A ShopDB-initiated scan MUST write its report under +# %ProgramData%\ShopDB\EndpointAgent\ (the same SYSTEM-owned root as the agent +# logs), NOT to Dell's default location. Corporate-managed DCU tasks write to +# the default; keeping ShopDB's output separate is the only thing that makes +# "did ShopDB ask for this scan, or did corporate" answerable when the results +# are collected. The path lives in the task definition and is generated +# server-side, so it can change without touching the agent. +# +# STILL TO DO - the results collector. Firing DCU is the easy half. schtasks +# /run returns as soon as the task STARTS, so a successful scheduledtask_run +# means "scan started", never "scan finished" - the command result cannot carry +# findings. Reporting available updates back into ShopDB needs a collector that +# reads the report from the path above and folds it into the telemetry payload. +# That is a separate change from this one; do not infer from a green command +# result that the data path exists. + + # Reserved seam - one-click mapped-drive remediation. NOT implemented; this is a # placement note so the next person adds it here rather than inventing a new # transport. When it lands it will re-establish a Disconnected mapping surfaced @@ -126,8 +173,23 @@ COMMAND_ALLOWLIST = { # Actions that demand a permission stricter than the base endpoints.command. # The enqueue route requires the caller to ALSO hold the mapped permission, so a # destructive action (a remote reboot) is a heavier grant than a routine toggle. +# NOTE FOR THE ROLES/PERMISSIONS WORK: scheduledtask_run is deliberately mapped +# onto the EXISTING endpoints.deploy grant rather than introducing a new one, so +# the wire is pre-built and needs no migration. Be aware this WIDENS what +# endpoints.deploy means: it previously covered run_script + update_app, and now +# also covers starting approved scheduled tasks. If that bundle is too coarse, +# the clean split is a separate endpoints.corptools permission mapped here - no +# other code needs to change, because the enqueue route reads this table. +# +# Suggested operator-facing wording for the combined grant: +# "Caution: Operator can run scheduled tasks, update applications, and affect +# system-level changes to the endpoint." +# That wording is accurate for scheduledtask_run and update_app. It UNDERSTATES +# run_script, which is arbitrary SYSTEM code execution - if run_script stays in +# this bundle, say so plainly in the role description. ELEVATED_ACTION_PERMISSIONS = { 'system_restart': 'endpoints.restart', + 'scheduledtask_run': 'endpoints.deploy', 'run_script': 'endpoints.deploy', 'update_app': 'endpoints.deploy', } @@ -242,6 +304,24 @@ def monitor_id_from_action(action): return None +# Actions gated by ROLE ONLY, never by per-endpoint Configuration Policy. +# +# These are the Corporate Tools actions. They start already-registered, +# agent-allowlisted tasks and are not a per-machine configuration concern, so a +# policy that never mentions them must not remove them. Policy exists to shape +# what a given PC TYPE exposes (which monitors, which service primitives); it is +# not the authority on whether an admin may run a corporate tool. +# +# Without this exemption the intersection at the end of ALLOWED_ACTIONS_FOR +# silently drops these actions on any endpoint whose effective policy is +# non-empty, producing "Action not permitted by policy for this endpoint" even +# for a caller holding endpoints.deploy. The real gate for these stays +# ELEVATED_ACTION_PERMISSIONS (role), enforced in the enqueue route. +POLICY_EXEMPT_ACTIONS = frozenset({ + 'scheduledtask_run', +}) + + def ALLOWED_ACTIONS_FOR(pctype=None, assetid=None): """The action names an endpoint may run. @@ -250,8 +330,9 @@ def ALLOWED_ACTIONS_FOR(pctype=None, assetid=None): result is narrowed to the actions the endpoint's effective policy permits - this is the allowlist-gating half of the policy design. With the feature off (or no assetid), it returns the full base allowlist, so nothing changes until - a site opts in. Policy resolution is imported lazily to keep this module free - of a hard dependency on the computers plugin. + a site opts in. Actions in POLICY_EXEMPT_ACTIONS are never narrowed away; + they are gated by role alone. Policy resolution is imported lazily to keep + this module free of a hard dependency on the computers plugin. """ base = set(COMMAND_ALLOWLIST.keys()) if assetid is None: @@ -274,6 +355,9 @@ def ALLOWED_ACTIONS_FOR(pctype=None, assetid=None): # "not permitted by policy". Expand each monitor: to its kind's control # actions before intersecting. allowed = allowed | _monitor_control_actions(allowed) + # Role-gated actions are added back unconditionally: policy narrows the + # per-endpoint surface, but it does not own the corporate tools. + allowed = allowed | POLICY_EXEMPT_ACTIONS # Never widen past the code-level allowlist: policy can only gate within it. return base & allowed diff --git a/plugins/endpoints/migrations/versions/0016_endpoint_dellupdates.py b/plugins/endpoints/migrations/versions/0016_endpoint_dellupdates.py new file mode 100644 index 00000000..358b4759 --- /dev/null +++ b/plugins/endpoints/migrations/versions/0016_endpoint_dellupdates.py @@ -0,0 +1,51 @@ +"""Add dellupdatesjson to endpointstates. + +Stores the pending Dell Command | Update summary reported by the +shopdb-dellupdates agent utility. MEDIUMTEXT for consistency with the other +snapshot columns on this table. + +Nullable with no backfill. Null means the endpoint has never reported a Dell +scan result, which is deliberately distinct from a stored payload whose status +is 'ok' with an empty list (scanned, nothing pending). The status card renders +those two states differently, so defaulting this to an empty summary would +assert every pre-existing endpoint had been scanned and found clean. + +Idempotent: the column-add checks the live schema first, so a re-run and a +db.create_all() site both stay clean on strict MySQL 8. +""" +from alembic import op + +import sqlalchemy as sa + + +# revision identifiers, used by Alembic. +revision = 'endpoints0016dellupdates' +down_revision = 'endpoints0015systemtelemetry' +branch_labels = None +depends_on = None + +TABLE_NAME = 'endpointstates' + +COLUMN_NAME = 'dellupdatesjson' + + +def _has_column(table_name, column_name): + from sqlalchemy import inspect + + inspector = inspect(op.get_bind()) + if not inspector.has_table(table_name): + return False + return any(col['name'] == column_name + for col in inspector.get_columns(table_name)) + + +def upgrade(): + if not _has_column(TABLE_NAME, COLUMN_NAME): + op.add_column( + TABLE_NAME, + sa.Column(COLUMN_NAME, sa.Text(length=16777215), nullable=True)) + + +def downgrade(): + if _has_column(TABLE_NAME, COLUMN_NAME): + op.drop_column(TABLE_NAME, COLUMN_NAME) diff --git a/plugins/endpoints/models/endpoint.py b/plugins/endpoints/models/endpoint.py index 5e173028..6a0a27a7 100644 --- a/plugins/endpoints/models/endpoint.py +++ b/plugins/endpoints/models/endpoint.py @@ -129,6 +129,16 @@ class EndpointState(db.Model): # already accumulating when someone is ready to interpret it. defenderjson = db.Column(db.Text(length=16777215), nullable=True) + # Pending Dell Command | Update items, produced by the shopdb-dellupdates + # agent utility from the log the \ShopDB-DellCommandScan task writes. Only + # the count drives the status card; the short list is stored so the panel + # can show what is actually pending without a second round trip. + # + # A null column and a stored payload with status 'unknown' both mean "never + # scanned", which stays distinct from a scan that found nothing pending. The + # status card relies on that distinction - see _dell_updates_field. + dellupdatesjson = db.Column(db.Text(length=16777215), nullable=True) + # Warn-only reconciler output, refreshed on every agent check-in in the same # commit as servicesjson. Each entry names a policy/shipped-attached monitor # the endpoint is expected to be running but the latest snapshot does not diff --git a/plugins/endpoints/plugin.py b/plugins/endpoints/plugin.py index 2cf669bd..f3cdb870 100644 --- a/plugins/endpoints/plugin.py +++ b/plugins/endpoints/plugin.py @@ -54,7 +54,7 @@ KNOWN_PAYLOAD_KEYS = frozenset({ 'scheduledtasks', 'applications', 'events', 'disks', 'bios', 'memory', 'processor', 'osinfo', 'domain', 'pendingreboot', 'usercontext', 'diskhealth', - 'rebootreasons', 'bootinfo', 'defender', + 'rebootreasons', 'bootinfo', 'defender', 'dellupdates', }) @@ -182,12 +182,32 @@ class EndpointsPlugin(BasePlugin): # remote reboot is a strictly heavier grant than a routine toggle. ('endpoints.restart', 'Restart endpoint computers (destructive)', 'endpoints'), - # Elevated grant for software delivery (run_script, update_app). + # Elevated grant for software delivery (run_script, update_app) and + # for starting approved scheduled tasks (scheduledtask_run). # Required IN ADDITION to endpoints.command by the enqueue route, so # running a staged script or pushing an installer is a strictly # heavier grant than a routine service/process toggle. + # + # NOTE FOR THE ROLES WORK - this wire is pre-built for you. + # scheduledtask_run was deliberately mapped onto this EXISTING grant + # rather than minting a new permission, so nothing needs migrating. + # Be aware that WIDENS what endpoints.deploy means: it used to cover + # run_script + update_app, and now also covers starting any task on + # the agent's approved-task allowlist. Splitting it out is a one-line + # change (add endpoints.corptools here and remap it in + # commands.ELEVATED_ACTION_PERMISSIONS) - the enqueue route reads + # that table, so no other code changes. + # + # If you do split it, note the three are NOT equal risk: + # scheduledtask_run only starts something already registered on the + # endpoint and is allowlisted agent-side; update_app runs a pinned + # installer; run_script is arbitrary code that runs as SYSTEM unless + # its descriptor pins the user context. run_script is the reason this + # grant's description has to stay blunt. ('endpoints.deploy', - 'Run staged scripts and push application installers to endpoints', 'endpoints'), + 'Run staged scripts, start approved scheduled tasks, and push ' + 'application installers to endpoints (scripts run as SYSTEM; ' + 'grant only to fully trusted operators)', 'endpoints'), # Packaging the agent is a heavier grant than deploying to one, and # it is not the same job either: the generated ZIP carries a live # collector key stamped into its install script, so whoever holds @@ -1113,6 +1133,13 @@ class EndpointsPlugin(BasePlugin): if 'defender' in payload: state.defenderjson = _as_json(payload.get('defender')) + # Presence-guarded like the probe output below: the agent omits the key + # entirely when the collector utility is not installed, and overwriting + # to null there would discard a still-valid scan result from before the + # utility was removed. + if 'dellupdates' in payload: + state.dellupdatesjson = _as_json(payload.get('dellupdates')) + # Session-scoped probe output. The agent repeats its last result between # probe runs and omits the key entirely when nobody is signed in, so the # presence guard preserves the last known mapped drives across an diff --git a/tests/test_plugin_migrations.py b/tests/test_plugin_migrations.py index b85ce5aa..971b85fc 100644 --- a/tests/test_plugin_migrations.py +++ b/tests/test_plugin_migrations.py @@ -106,7 +106,7 @@ EXPECTED_HEAD_REVISION['warranty'] = 'warranty0002proof' # 0013 adds the inventory columns. # 0014 adds endpointstates.diskhealthjson, and 0015 adds the system telemetry # columns (boot info, reboot reasons, Defender health, NIC link state). -EXPECTED_HEAD_REVISION['endpoints'] = 'endpoints0015systemtelemetry' +EXPECTED_HEAD_REVISION['endpoints'] = 'endpoints0016dellupdates' # Plugins built after the cutover: their 0001 baseline really creates tables the # core chain never owned. diff --git a/tests/test_plugins/test_endpoints_dellupdates.py b/tests/test_plugins/test_endpoints_dellupdates.py new file mode 100644 index 00000000..1e048ee2 --- /dev/null +++ b/tests/test_plugins/test_endpoints_dellupdates.py @@ -0,0 +1,168 @@ +"""Pending Dell Command | Update reporting: ingest and status-card exposure. + +The agent's shopdb-dellupdates utility reads the log the \\ShopDB-DellCommandScan +scheduled task writes and reports a short summary on the normal telemetry +check-in. These tests cover the ingest write path and the three-state rendering +on the status panel, which is the part most likely to be broken by a well-meaning +simplification: "never scanned" must NOT render as "No". +""" + +import json + +from shopdb.core.models import Asset, AssetType + + +STATUS_URL = '/api/endpoints/asset/{}/status' + + +def _computer(db, hostname): + """Create a computer Asset + Computer row; return the asset.""" + from plugins.computers.models import Computer, ComputerType + + atype = AssetType.query.filter_by(assettype='computer').first() + if not atype: + atype = AssetType(assettype='computer') + db.session.add(atype) + db.session.flush() + + ctype = ComputerType.query.filter_by(computertype='CMM PC').first() + if not ctype: + ctype = ComputerType(computertype='CMM PC') + db.session.add(ctype) + db.session.flush() + + asset = Asset(assetnumber=hostname, assettypeid=atype.assettypeid, + isactive=True) + db.session.add(asset) + db.session.flush() + + db.session.add(Computer(assetid=asset.assetid, hostname=hostname, + computertypeid=ctype.computertypeid)) + db.session.flush() + return asset + + +def _checkin(db, hostname, payload): + """Run a check-in for hostname with the extra payload keys merged in.""" + from plugins.endpoints.plugin import EndpointsPlugin + + body = {'hostname': hostname} + body.update(payload) + EndpointsPlugin().apply_collector_payload(body) + db.session.commit() + + +def _status_fields(client, asset, auth_headers): + response = client.get(STATUS_URL.format(asset.assetid), headers=auth_headers) + assert response.status_code == 200 + return {f['label']: f['value'] + for f in response.get_json()['data']['fields']} + + +def test_checkin_persists_the_dell_summary(client, db, app, auth_headers): + """A reported dellupdates payload is stored on the endpoint state.""" + from plugins.endpoints.models.endpoint import EndpointState + + _computer(db, 'DELL01') + _checkin(db, 'DELL01', {'dellupdates': { + 'status': 'ok', 'count': 1, 'truncated': False, + 'updates': [{'name': 'Dell BIOS', 'severity': 'Urgent', + 'version': '1.2.3'}], + }}) + + state = EndpointState.query.filter_by( + assetid=Asset.query.filter_by(assetnumber='DELL01').first().assetid + ).first() + stored = json.loads(state.dellupdatesjson) + assert stored['count'] == 1 + assert stored['updates'][0]['name'] == 'Dell BIOS' + + +def test_pending_updates_show_yes_with_a_count(client, db, app, auth_headers): + """A scan with pending items renders Yes plus how many.""" + asset = _computer(db, 'DELL02') + _checkin(db, 'DELL02', {'dellupdates': { + 'status': 'ok', 'count': 3, 'truncated': False, 'updates': [], + }}) + + fields = _status_fields(client, asset, auth_headers) + assert fields['Pending Dell updates'] == 'Yes (3)' + + +def test_a_clean_scan_shows_no(client, db, app, auth_headers): + """A scan that found nothing renders No, not a hidden row.""" + asset = _computer(db, 'DELL03') + _checkin(db, 'DELL03', {'dellupdates': { + 'status': 'ok', 'count': 0, 'truncated': False, 'updates': [], + }}) + + fields = _status_fields(client, asset, auth_headers) + assert fields['Pending Dell updates'] == 'No' + + +def test_a_never_scanned_endpoint_says_so(client, db, app, auth_headers): + """Never scanned renders as its own visible state, not as No and not hidden. + + Two failure modes are being guarded at once: showing 'No' would claim the + machine is current when nobody ever checked, and hiding the row entirely + reads to an operator as a broken panel. + """ + asset = _computer(db, 'DELL04') + _checkin(db, 'DELL04', {}) + + fields = _status_fields(client, asset, auth_headers) + assert fields['Pending Dell updates'] == 'Never scanned' + + +def test_an_unknown_status_says_never_scanned(client, db, app, auth_headers): + """The utility's own 'never scanned' answer renders as never scanned.""" + asset = _computer(db, 'DELL05') + _checkin(db, 'DELL05', {'dellupdates': { + 'status': 'unknown', 'count': 0, 'truncated': False, 'updates': [], + }}) + + fields = _status_fields(client, asset, auth_headers) + assert fields['Pending Dell updates'] == 'Never scanned' + + +def test_a_failed_scan_read_is_distinct_from_never_scanned(client, db, app, + auth_headers): + """An unreadable scan log is its own state, not silence and not clean.""" + asset = _computer(db, 'DELL06') + _checkin(db, 'DELL06', {'dellupdates': { + 'status': 'error', 'count': 0, 'truncated': False, 'updates': [], + }}) + + fields = _status_fields(client, asset, auth_headers) + assert fields['Pending Dell updates'] == 'Unknown (scan unreadable)' + + +def test_a_malformed_blob_does_not_break_the_panel(client, db, app, auth_headers): + """A corrupt cached payload degrades to the unknown state, not a 500.""" + from plugins.endpoints.models.endpoint import EndpointState + + asset = _computer(db, 'DELL07') + _checkin(db, 'DELL07', {}) + + state = EndpointState.query.filter_by(assetid=asset.assetid).first() + state.dellupdatesjson = 'not json at all' + db.session.commit() + + fields = _status_fields(client, asset, auth_headers) + assert fields['Pending Dell updates'] == 'Never scanned' + + +def test_an_omitted_key_preserves_the_last_result(client, db, app, auth_headers): + """A check-in without the key does not erase a previously reported scan. + + The agent omits dellupdates entirely when the collector utility is absent, + so overwriting to null would discard a still-valid result. + """ + asset = _computer(db, 'DELL08') + _checkin(db, 'DELL08', {'dellupdates': { + 'status': 'ok', 'count': 2, 'truncated': False, 'updates': [], + }}) + _checkin(db, 'DELL08', {}) + + fields = _status_fields(client, asset, auth_headers) + assert fields['Pending Dell updates'] == 'Yes (2)' diff --git a/tests/test_plugins/test_endpoints_siteagent_build.py b/tests/test_plugins/test_endpoints_siteagent_build.py new file mode 100644 index 00000000..e69de29b