From cd705563d082914d1424021147d226675493bf0a Mon Sep 17 00:00:00 2001 From: "benjamin.baker" Date: Sun, 6 Sep 2026 05:22:05 -0400 Subject: [PATCH] Add Corporate Tools tab, Dell Command actions, and user-context refresh Corporate Tools tab on PC Details, gated by endpoints.deploy and liveness. Starts already-registered scheduled tasks; the agent's ScheduledTaskPolicy allowlist remains the authority on what may actually run. Dell Command | Update split into two actions (Scan, Apply Updates) because they have very different consequences. Apply Updates carries a persistent breakglass warning in the card naming Tanium as the primary update utility, rendered between the row title and description so it is visible at all times rather than only in the confirm dialog. Destructive actions keep the typed RUN confirmation. Fix scheduledtask_run being stripped by Configuration Policy: the intersection in ALLOWED_ACTIONS_FOR dropped it on any endpoint whose effective policy was non-empty, so every Corporate Tools click failed with "not permitted by policy" regardless of the caller's role. Corporate tools are role-gated only, via POLICY_EXEMPT_ACTIONS; policy keeps authority over monitors and service/process primitives. Fix blank PC Details page: watch(detailtabs) evaluates its source during setup and read canDeploy before its const declaration, throwing a TDZ ReferenceError that aborted setup with no API request made. Mapped-drive refresh moves to the tray. SYSTEM cannot probe user-scoped drives, so the service requests a re-probe over a named event and waits for the spool collectedat stamp to advance before collecting. The probe task is registered as a ShopFloor user principal; the Users-group principal remains only as a degraded fallback since a group member cannot start it. --- agent/ShopDbEndpointAgent.Core/AgentConfig.cs | 68 ++++--- .../CommandExecutor.cs | 168 +++++++++++++++ agent/ShopDbEndpointAgent.Core/Contracts.cs | 8 + .../ScheduledTaskPolicy.cs | 159 +++++++++++++++ .../ShopDbEndpointAgent.Core.csproj | 4 + .../TelemetryWorker.cs | 118 ++++++++++- .../UserContextRefreshChannel.cs | 179 ++++++++++++++++ .../UserContextSpool.cs | 66 ++++++ .../Program.cs | 51 +++++ .../ShopDbEndpointAgent.PolicyProbe.csproj | 4 + .../ShopDbEndpointAgent.Tray.csproj | 19 +- agent/ShopDbEndpointAgent.Tray/TrayContext.cs | 151 +++++++++++++- agent/ShopDbEndpointAgent/Program.cs | 11 +- .../ShopDbEndpointAgent.csproj | 21 +- agent/utilities/shopdb-dellupdates.ps1 | 119 +++++++++++ plugins/computers/frontend/views/PCDetail.vue | 191 ++++++++++++++++++ plugins/endpoints/api/routes.py | 59 ++++++ plugins/endpoints/api/siteagent.py | 64 ++++-- plugins/endpoints/commands.py | 90 ++++++++- .../versions/0016_endpoint_dellupdates.py | 51 +++++ plugins/endpoints/models/endpoint.py | 10 + plugins/endpoints/plugin.py | 33 ++- tests/test_plugin_migrations.py | 2 +- .../test_endpoints_dellupdates.py | 168 +++++++++++++++ .../test_endpoints_siteagent_build.py | 0 25 files changed, 1756 insertions(+), 58 deletions(-) create mode 100644 agent/ShopDbEndpointAgent.Core/ScheduledTaskPolicy.cs create mode 100644 agent/ShopDbEndpointAgent.Core/UserContextRefreshChannel.cs create mode 100644 agent/utilities/shopdb-dellupdates.ps1 create mode 100644 plugins/endpoints/migrations/versions/0016_endpoint_dellupdates.py create mode 100644 tests/test_plugins/test_endpoints_dellupdates.py create mode 100644 tests/test_plugins/test_endpoints_siteagent_build.py 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