From b49fb60c0c32e52bbf9ba116235951f038c5bb09 Mon Sep 17 00:00:00 2001 From: "benjamin.baker" Date: Wed, 9 Sep 2026 19:23:43 -0400 Subject: [PATCH 1/4] Add log collection, script output capture, and timestamp fixes Log Collector: a new collect_log action reads a plaintext log file from a known path and returns its tail for viewing and download. Rides the existing endpoints.updateapp grant rather than inventing one, since collecting a log is the same trust level as pushing an application. Collected logs land in their own table with per-asset retention of 20 instead of overloading the command audit row, and the agent refuses UNC, traversal, wildcards, directories and non-text content independently of the server check. collect_log is POLICY_EXEMPT. Every existing Configuration Policy predates the action and so cannot name it, and the intersection in ALLOWED_ACTIONS_FOR drops anything unnamed - so every policied endpoint refused log collection outright, including for admins, who saw a permission-shaped error for a grant they held. Exemption from policy is not exemption from permission: the elevated grant still applies, and a regression test asserts both halves so a future edit cannot quietly loosen the grant while "fixing" the policy gate. Script output capture: successful runs can return output, gated per script by captureoutput. Failures always returned it; success never did, which left a green badge as the only evidence a run produced. Timestamps: Activity and Operator Reports rendered date-only because the shared formatter's 'date' branch drops the clock time. Run Scripts additionally labels its timestamp, since a bare time meant "completed" on a terminal row but "queued" on an in-flight one - indistinguishable, and the wrong reading. Agent 1.16.0. Minor: collect_log is a new action name, so an older agent asked for it fails closed on the unknown-action default. --- .../CommandExecutor.cs | 279 ++++++- agent/ShopDbEndpointAgent.Core/Contracts.cs | 8 + .../ShopDbEndpointAgent.csproj | 24 +- frontend/src/api/index.js | 23 + frontend/src/components/PluginAssetPanels.vue | 7 +- .../versions/7d50_script_captureoutput.py | 48 ++ .../versions/7d51_endpoint_logcollections.py | 92 +++ plugins/computers/frontend/views/PCDetail.vue | 730 +++++++++++++++++- plugins/endpoints/api/routes.py | 365 ++++++++- plugins/endpoints/commands.py | 38 + .../frontend/views/SystemScriptsSettings.vue | 31 + plugins/endpoints/models/__init__.py | 4 +- plugins/endpoints/models/endpoint.py | 167 +++- plugins/endpoints/plugin.py | 10 +- shopdb/core/api/scripts.py | 40 +- shopdb/core/models/script_library.py | 13 + tests/test_core/test_script_captureoutput.py | 129 ++++ tests/test_plugins/test_endpoints_plugin.py | 20 + .../test_endpoints_script_runs.py | 200 +++++ 19 files changed, 2207 insertions(+), 21 deletions(-) create mode 100644 migrations/versions/7d50_script_captureoutput.py create mode 100644 migrations/versions/7d51_endpoint_logcollections.py create mode 100644 tests/test_core/test_script_captureoutput.py create mode 100644 tests/test_plugins/test_endpoints_script_runs.py diff --git a/agent/ShopDbEndpointAgent.Core/CommandExecutor.cs b/agent/ShopDbEndpointAgent.Core/CommandExecutor.cs index dffd7cd5..a96afbdf 100644 --- a/agent/ShopDbEndpointAgent.Core/CommandExecutor.cs +++ b/agent/ShopDbEndpointAgent.Core/CommandExecutor.cs @@ -1,6 +1,8 @@ using System.Diagnostics; +using System.Globalization; using System.Security.Cryptography; using System.ServiceProcess; +using System.Text; using Microsoft.Extensions.Logging; namespace ShopDbEndpointAgent.Core; @@ -167,6 +169,11 @@ public sealed class CommandExecutor case "network_connections": return NetworkConnections(); + // Read a plaintext log off this machine on demand. Read-only and + // bounded: see CollectLog for why each refusal is there. + case "collect_log": + return RequireTarget(action, target) ?? CollectLog(target); + default: logger.LogWarning("Rejected unknown command action {Action}", action); return new CommandResult @@ -241,6 +248,248 @@ public sealed class CommandExecutor } } + // ---- Log collection ---- + + // Extensions this agent will read. An allowlist, not a blocklist: the + // question is not "is this file dangerous" but "will this render as text", + // and only a closed set answers that. Mirrors LOG_PATH_EXTENSIONS on the + // server; that copy gives a fast error at enqueue, THIS copy is the one + // guarding the disk and must never be removed on the assumption the server + // already checked. + private static readonly HashSet AllowedLogExtensions = + new(StringComparer.OrdinalIgnoreCase) + { + // The extension the server also omits (see LOG_PATH_EXTENSIONS) is + // omitted here too. The two copies must agree, or the server + // accepts a path the agent then refuses and the operator gets a + // failure with no actionable cause. + ".log", ".txt", ".xml", ".json", ".csv", ".ini", ".conf", + ".yml", ".yaml", ".trace", ".out", ".err", ".md", + }; + + // Ceiling on what crosses the wire. Logs grow without bound; a multi-GB + // file would exhaust memory here and be refused by the server's payload + // limit anyway, returning nothing. Sending the last 2 MB is far more useful + // than failing, because the end of a log is where the recent failure is. + private const int MaxLogBytes = 2 * 1024 * 1024; + + // Read a plaintext log file and return its tail. + // + // Strictly read-only: opens for reading, never creates, writes or deletes. + // FileShare.ReadWrite|Delete is essential rather than lax - the interesting + // logs are the ones a running application currently holds open, and a + // stricter share mode would fail on exactly those with a sharing violation. + private CommandResult CollectLog(string target) + { + var path = (target ?? string.Empty).Trim(); + + try + { + // Re-validate independently of the server. The agent runs as SYSTEM + // and this path arrived over the network; trusting an upstream check + // would make the whole guard depend on a component this process + // cannot verify. + var rejection = ValidateLogPath(ref path); + if (rejection != null) + { + logger.LogWarning("collect_log refused {Path}: {Reason}", path, rejection); + return new CommandResult { Succeeded = false, Message = rejection }; + } + + var info = new FileInfo(path); + if (!info.Exists) + { + return new CommandResult + { + Succeeded = false, + Message = $"No file found at {path}", + }; + } + + var totalBytes = info.Length; + var truncated = totalBytes > MaxLogBytes; + + string content; + using (var stream = new FileStream( + path, FileMode.Open, FileAccess.Read, + FileShare.ReadWrite | FileShare.Delete)) + { + if (truncated) + { + // Seek to the tail. This can land mid-line, so the partial + // first line is dropped below rather than shown as a + // fragment the reader would misread as a real entry. + stream.Seek(-MaxLogBytes, SeekOrigin.End); + } + + using var reader = new StreamReader(stream, Encoding.UTF8, true); + content = reader.ReadToEnd(); + } + + if (truncated) + { + var firstBreak = content.IndexOf('\n'); + if (firstBreak >= 0 && firstBreak < content.Length - 1) + content = content[(firstBreak + 1)..]; + } + + // A file whose bytes are not text will have produced replacement + // characters above. Returning that would fill the operator's pane + // with noise, so it is refused with a reason instead. + if (LooksBinary(content)) + { + return new CommandResult + { + Succeeded = false, + Message = + $"{path} does not appear to be a plaintext file; " + + "only text logs can be collected", + }; + } + + var lineCount = CountLines(content); + + logger.LogInformation( + "collect_log read {Path} ({Bytes} bytes on disk, truncated: {Truncated})", + path, totalBytes, truncated); + + return new CommandResult + { + Succeeded = true, + Message = truncated + ? $"Collected the last {content.Length:N0} characters of {info.Name} " + + $"({totalBytes:N0} bytes on disk)" + : $"Collected {info.Name} ({totalBytes:N0} bytes)", + Detail = new Dictionary + { + // The RESOLVED path, which may differ from what was typed + // once environment variables are expanded. + ["path"] = info.FullName, + ["content"] = content, + ["filesizebytes"] = totalBytes, + ["linecount"] = lineCount, + ["truncated"] = truncated, + ["modifiedat"] = info.LastWriteTimeUtc.ToString( + "o", CultureInfo.InvariantCulture), + }, + }; + } + catch (UnauthorizedAccessException exception) + { + // Called out separately because the remedy is different from a + // generic failure: the file exists but the agent cannot read it. + logger.LogWarning(exception, "collect_log denied for {Path}", path); + return new CommandResult + { + Succeeded = false, + Message = $"Access denied reading {path}", + }; + } + catch (IOException exception) + { + logger.LogWarning(exception, "collect_log io failure for {Path}", path); + return new CommandResult + { + Succeeded = false, + Message = $"Could not read {path}: {exception.Message}", + }; + } + catch (Exception exception) + { + logger.LogWarning(exception, "collect_log failed for {Path}", path); + return new CommandResult + { + Succeeded = false, + Message = $"Could not collect {path}: {exception.Message}", + }; + } + } + + // Shape validation, run before anything is opened. Returns a reason to + // refuse, or null to proceed. Expands environment variables first so the + // checks apply to the path that will ACTUALLY be opened - validating the + // literal string and then expanding it would let %VAR% smuggle past every + // rule here. + private static string? ValidateLogPath(ref string path) + { + if (string.IsNullOrWhiteSpace(path)) + return "A log file path is required"; + + path = Environment.ExpandEnvironmentVariables(path).Trim(); + var normalized = path.Replace('/', '\\'); + + if (normalized.StartsWith(@"\\", StringComparison.Ordinal)) + return "UNC/network paths are not supported"; + + // Reject '..' before calling GetFullPath. Normalising first would + // resolve the traversal and hide it, so the check has to see the raw + // segments. + if (normalized.Split('\\').Any(part => part == "..")) + return "Path must not contain \"..\" segments"; + + if (normalized.Contains('*') || normalized.Contains('?')) + return "Wildcards are not supported; name a single log file"; + + if (!Path.IsPathRooted(normalized) || normalized.Length < 3 + || normalized[1] != ':') + { + return "Enter a full local path including the drive letter"; + } + + var extension = Path.GetExtension(normalized); + if (string.IsNullOrEmpty(extension) + || !AllowedLogExtensions.Contains(extension)) + { + return "Only plaintext log files can be collected " + + $"(allowed: {string.Join(", ", AllowedLogExtensions.OrderBy(e => e))})"; + } + + // A directory reached through a path that passes every check above is + // still not a file, and Directory.Exists is cheap insurance. + if (Directory.Exists(normalized)) + return "That path is a directory, not a log file"; + + path = Path.GetFullPath(normalized); + return null; + } + + // Heuristic: a NUL byte, or many replacement characters, means the decode + // produced garbage rather than text. Sampling the head is enough - a binary + // file betrays itself immediately, and scanning megabytes to be sure would + // cost more than it is worth. + private static bool LooksBinary(string content) + { + if (string.IsNullOrEmpty(content)) + return false; + + var sample = Math.Min(content.Length, 8000); + var replacements = 0; + for (var index = 0; index < sample; index++) + { + var character = content[index]; + if (character == '\0') + return true; + if (character == '\uFFFD') + replacements++; + } + + return replacements > sample / 100; + } + + private static int CountLines(string content) + { + if (string.IsNullOrEmpty(content)) + return 0; + + var lines = 1; + foreach (var character in content) + { + if (character == '\n') + lines++; + } + return lines; + } + private Dictionary GetProcessNameSnapshot() { var names = new Dictionary(); @@ -1016,7 +1265,7 @@ public sealed class CommandExecutor // The endpoint log gets the output ALWAYS, success or failure. This is the // copy that stays on the box: it never leaves the endpoint, so it costs no // exposure, and it is the only record when a script reports success while - // doing nothing useful. The server copy is failure-only (see ScriptDetail). + // doing nothing useful. The server copy is success-gated (see ShouldReturnOutput). private void LogScriptOutput(string name, string output, int? exitCode) { if (string.IsNullOrWhiteSpace(output)) @@ -1166,7 +1415,7 @@ public sealed class CommandExecutor ? $"Script {name} completed (system context)" : $"Script {name} exited with code {exit}", Detail = ScriptDetail(name, script, scriptPath, exit, workingDirectory, - exit == 0 ? null : output), + ShouldReturnOutput(script, succeeded: exit == 0) ? output : null), }; } @@ -1282,10 +1531,22 @@ public sealed class CommandExecutor ? $"Script {name} completed (user context)" : $"Script {name} exited with code {exit}", Detail = ScriptDetail(name, script, scriptPath, exit, workingDirectory, - ok ? null : output), + ShouldReturnOutput(script, succeeded: ok) ? output : null), }; } + // Whether this run's output should travel back to the server. + // + // A FAILED run always returns it: the whole point of the field is making a + // failure diagnosable from the UI rather than by remoting to the box. A + // SUCCESSFUL run returns it only when the script's author opted in, because + // output carries whatever the body printed - hostnames, share paths, a + // token echoed while debugging - and a routine green run is not worth that + // exposure by default. A claim from a server too old to send the field + // leaves CaptureOutput false, so the behaviour is unchanged. + private static bool ShouldReturnOutput(ScriptDeliveryClaim? script, bool succeeded) + => !succeeded || (script?.CaptureOutput ?? false); + private static Dictionary ScriptDetail( string name, ScriptDeliveryClaim? script, string? scriptPath, int? exitCode, string? workingDirectory = null, string? output = null) @@ -1313,12 +1574,12 @@ public sealed class CommandExecutor detail["workingdirectory"] = workingDirectory!; if (exitCode is not null) detail["exitcode"] = exitCode; - // FAILURE ONLY. Callers pass output only when the command failed, so a - // successful run's stdout never leaves the endpoint. Script output can - // carry whatever the body happened to print - hostnames, paths, tokens - // an author echoed while debugging - and there is no reason to accept - // that exposure for a run nobody needs to diagnose. The endpoint log - // keeps the full copy either way. + // Always present on failure; on success only when the script opted in + // via captureoutput (see ShouldReturnOutput). Script output can carry + // whatever the body happened to print - hostnames, paths, tokens an + // author echoed while debugging - so a successful run stays silent + // unless someone decided otherwise after reading the body. The endpoint + // log keeps the full copy either way. if (!string.IsNullOrWhiteSpace(output)) detail["output"] = output!; return detail; diff --git a/agent/ShopDbEndpointAgent.Core/Contracts.cs b/agent/ShopDbEndpointAgent.Core/Contracts.cs index a7a1e1c1..ee7ace3f 100644 --- a/agent/ShopDbEndpointAgent.Core/Contracts.cs +++ b/agent/ShopDbEndpointAgent.Core/Contracts.cs @@ -197,6 +197,13 @@ public sealed class AppDeliveryClaim // written to tolerate a failing cleanup or to probe with a cmdlet that errors on // absence would start aborting halfway. An absent or unrecognised value is // treated as default, so an old server that never sends the field is safe. +// CaptureOutput decides whether a SUCCESSFUL run returns its output to the +// server. Failures always return it, because a failure has to be diagnosable +// without walking to the machine. Success is opt-in per script: output carries +// whatever the body printed, and that is not worth persisting server-side for a +// routine green run. The endpoint log keeps the full copy regardless, so this +// only governs what crosses the wire. Absent means false, which is the same +// behaviour as before the field existed. public sealed class ScriptDeliveryClaim { [JsonPropertyName("scriptid")] public int ScriptId { get; set; } @@ -206,6 +213,7 @@ public sealed class ScriptDeliveryClaim [JsonPropertyName("runcontext")] public string? RunContext { get; set; } [JsonPropertyName("workingdirectory")] public string? WorkingDirectory { get; set; } [JsonPropertyName("errormode")] public string? ErrorMode { get; set; } + [JsonPropertyName("captureoutput")] public bool CaptureOutput { get; set; } } // Envelope for the claim long-poll: { status, data: { endpointcommandid, action } }. diff --git a/agent/ShopDbEndpointAgent/ShopDbEndpointAgent.csproj b/agent/ShopDbEndpointAgent/ShopDbEndpointAgent.csproj index 24029e09..d34a88df 100644 --- a/agent/ShopDbEndpointAgent/ShopDbEndpointAgent.csproj +++ b/agent/ShopDbEndpointAgent/ShopDbEndpointAgent.csproj @@ -211,7 +211,29 @@ older agent ignoring it keeps today's behaviour. Supersedes the 1.13.1 to 1.13.4 tray-coordination bumps merged from wj, which carried no agent code changes. Tray is at 1.6.5 from wj. --> -1.14.0 + + +1.16.0 true win-x64 true diff --git a/frontend/src/api/index.js b/frontend/src/api/index.js index 30fe4c8a..18cf5150 100644 --- a/frontend/src/api/index.js +++ b/frontend/src/api/index.js @@ -231,6 +231,29 @@ export const endpointsApi = { listCommands(assetId) { return api.get(`/endpoints/${assetId}/commands`) }, + // Recent run_script commands only, with the agent-reported detail the plain + // command list omits. Feeds the run history under the Run Scripts tab. + listScriptRuns(assetId, limit = 5) { + return api.get(`/endpoints/${assetId}/scriptruns`, { params: { limit } }) + }, + // Collected-log history for one endpoint. Metadata only - the log bodies are + // fetched one at a time, because a 20-row history of multi-hundred-KB logs + // would make opening the tab far more expensive than reading the one log the + // operator actually wants. + listLogCollections(assetId) { + return api.get(`/endpoints/${assetId}/logcollections`) + }, + // One collected log including its content, for the scrollable pane. + getLogCollection(collectionId) { + return api.get(`/endpoints/logcollections/${collectionId}`) + }, + // Blob, not JSON: the server sends this as a text/plain attachment, and the + // caller turns it into a save dialog. + downloadLogCollection(collectionId) { + return api.get(`/endpoints/logcollections/${collectionId}/download`, { + responseType: 'blob', + }) + }, // Active break-glass holds. Readable by any endpoints.view holder, since a // suppressed action is drift an operator must be able to explain. listPolicyHolds(assetId) { diff --git a/frontend/src/components/PluginAssetPanels.vue b/frontend/src/components/PluginAssetPanels.vue index 07633512..0dc09ead 100644 --- a/frontend/src/components/PluginAssetPanels.vue +++ b/frontend/src/components/PluginAssetPanels.vue @@ -309,8 +309,13 @@ import { // unmount so the blobs are not leaked. const reportImageUrls = ref({}) +// Operator reports carry a full timestamp, and the time of day is the useful +// part: several reports from one shift all landed on the same date, so a +// date-only label made them indistinguishable and un-orderable. formatValue's +// 'date' mode renders via toLocaleDateString and discards the clock, so this +// asks for 'datetime'. function formatDate(value) { - return formatValue(value, { format: 'date' }) + return formatValue(value, { format: 'datetime' }) } // Called from the template, so it runs on EVERY render. Without the seen-set it diff --git a/migrations/versions/7d50_script_captureoutput.py b/migrations/versions/7d50_script_captureoutput.py new file mode 100644 index 00000000..1df3a48e --- /dev/null +++ b/migrations/versions/7d50_script_captureoutput.py @@ -0,0 +1,48 @@ +"""Add scripts captureoutput + +Backs the per-script "may a successful run send its output back" field. + +The agent already captures a script's full output and always writes it to the +endpoint log. What this controls is narrower: whether that output also crosses +to the server and is persisted in the command's resultjson, where the Run +Scripts history can show it. + +Failures have always sent output and still do - a failed run has to be +diagnosable without walking to the machine. Success is the case in question, +and it is opt-in because output carries whatever the body happened to print: +hostnames, share paths, an account name, a token an author echoed while +debugging. Accepting that exposure for every routine green run is a poor +trade, so the author decides per script, after reading the body. + +BOOLEAN NOT NULL defaulting FALSE, so every script row written before this +migration keeps exactly today's behaviour: metadata on success, output only on +failure. Same rule as errormode - never retroactive, because a library-wide +flip would start persisting output from bodies nobody re-read with this +question in mind. + +Revision ID: 7d50_script_captureoutput +Revises: 7d49_script_errormode +Create Date: 2026-09-09 + +""" +from alembic import op +import sqlalchemy as sa + + +revision = '7d50_script_captureoutput' +down_revision = '7d49_script_errormode' +branch_labels = None +depends_on = None + + +def upgrade(): + op.add_column('scripts', sa.Column( + 'captureoutput', sa.Boolean(), nullable=False, + server_default=sa.false(), + comment='When true, a successful run also returns its output to the ' + 'server; failures always do. Off by default because script ' + 'output can contain whatever the body printed.')) + + +def downgrade(): + op.drop_column('scripts', 'captureoutput') diff --git a/migrations/versions/7d51_endpoint_logcollections.py b/migrations/versions/7d51_endpoint_logcollections.py new file mode 100644 index 00000000..d80fa9b7 --- /dev/null +++ b/migrations/versions/7d51_endpoint_logcollections.py @@ -0,0 +1,92 @@ +"""Add endpointlogcollections + +Backs the manual Log Collector tab: one row per plaintext log file an operator +collected from an endpoint by path. + +Collected content gets its own table rather than living in +endpointcommands.resultjson for three reasons. It is bulky, and resultjson is +read by every command-history list that only wants a status. It needs its own +retention rule, which a command row cannot carry without deleting the audit +record of the command itself. And the download route has to address a single +collection by id and serve it as a file, which is awkward against a JSON column. + +Retention is PER ASSET (20, enforced on insert in the result route), not global. +The history renders on a single machine's detail page, so a global cap would let +one busy endpoint quietly evict the history of another - the panel would be +unreliable exactly where it is read. + +endpointcommandid is SET NULL rather than CASCADE: commands are reaped and +expired over time, and a collected log an operator is still reading must not +vanish because its originating command row was cleaned up. assetid stays CASCADE +because a log has no meaning once the machine is gone. + +filesizebytes is BIGINT while contentbytes is INTEGER on purpose - the file on +disk may be gigabytes, while what we store is a bounded tail. + +Revision ID: 7d51_endpoint_logcollections +Revises: 7d50_script_captureoutput +Create Date: 2026-09-09 + +""" +from alembic import op +import sqlalchemy as sa + + +revision = '7d51_endpoint_logcollections' +down_revision = '7d50_script_captureoutput' +branch_labels = None +depends_on = None + + +def upgrade(): + op.create_table( + 'endpointlogcollections', + sa.Column('endpointlogcollectionid', sa.Integer(), nullable=False), + sa.Column('assetid', sa.Integer(), nullable=False), + sa.Column('endpointcommandid', sa.Integer(), nullable=True), + sa.Column('sourcepath', sa.String(length=512), nullable=False, + comment='Path as the AGENT resolved it, which may differ ' + 'from what the operator typed once environment ' + 'variables are expanded endpoint-side.'), + sa.Column('content', sa.Text(length=16777215), nullable=True), + sa.Column('filesizebytes', sa.BigInteger(), nullable=True, + comment='Size of the file on disk. Differs from contentbytes ' + 'when the stored copy is a truncated tail.'), + sa.Column('contentbytes', sa.Integer(), nullable=True), + sa.Column('linecount', sa.Integer(), nullable=True), + sa.Column('istruncated', sa.Boolean(), nullable=False, + server_default=sa.false(), + comment='True when content is only the tail of a larger file.'), + sa.Column('filemodifiedat', sa.DateTime(), nullable=True), + sa.Column('collectedbyuserid', sa.Integer(), nullable=True), + sa.Column('collectedbyusername', sa.String(length=100), nullable=True, + comment='Username snapshot; the id alone becomes unreadable ' + 'if the user is later removed.'), + sa.Column('collectedat', sa.DateTime(), nullable=False), + sa.ForeignKeyConstraint(['assetid'], ['assets.assetid'], + ondelete='CASCADE'), + sa.ForeignKeyConstraint(['endpointcommandid'], + ['endpointcommands.endpointcommandid'], + ondelete='SET NULL'), + sa.ForeignKeyConstraint(['collectedbyuserid'], ['users.userid'], + ondelete='SET NULL'), + sa.PrimaryKeyConstraint('endpointlogcollectionid'), + ) + op.create_index('ix_endpointlogcollections_assetid', + 'endpointlogcollections', ['assetid']) + op.create_index('ix_endpointlogcollections_collectedat', + 'endpointlogcollections', ['collectedat']) + # Serves both hot paths: listing an asset's history newest-first, and + # finding that asset's oldest rows to prune on insert. + op.create_index('idx_endpointlogcollections_assetcollected', + 'endpointlogcollections', ['assetid', 'collectedat']) + + +def downgrade(): + op.drop_index('idx_endpointlogcollections_assetcollected', + table_name='endpointlogcollections') + op.drop_index('ix_endpointlogcollections_collectedat', + table_name='endpointlogcollections') + op.drop_index('ix_endpointlogcollections_assetid', + table_name='endpointlogcollections') + op.drop_table('endpointlogcollections') diff --git a/plugins/computers/frontend/views/PCDetail.vue b/plugins/computers/frontend/views/PCDetail.vue index 794c1599..ade30e40 100644 --- a/plugins/computers/frontend/views/PCDetail.vue +++ b/plugins/computers/frontend/views/PCDetail.vue @@ -594,6 +594,229 @@ + + +
+

Recent Script Runs

+
+ Loading recent runs... +
+
+ No scripts have been run on this computer yet. +
+
    +
  • +
    + {{ scriptRunStatusLabel(run) }} + + {{ run.scriptname || run.target }} + + + {{ scriptRunTimeLabel(run) }} + +
    + +

    + {{ run.resultmessage }} +

    + +
    + + + +
    + + + 
    {{ run.resultdetail.output }}
    + +

    + The agent reported success but sent no detail. This agent may + predate run reporting. +

    +
    • +
+

+ A zero exit code means the script ran and PowerShell exited cleanly. + It does not confirm the script achieved what it was written to do. + Output is shown for failures, and for successes only when the script + is set to return it. +

+
+ + + +
+
+

Collect a Log File

+ +

+ Reads a plaintext log from this computer and stores a copy in ShopDB + so it can be read here or downloaded. The agent only ever reads the + file - it is never modified, moved or deleted. +

+ +
+ This computer is not currently checking in. A collection queued now + would sit unclaimed, so collecting is disabled until it reports + again. Previously collected logs are still readable below. +
+ +
+ + +

{{ logpatherror }}

+

+ Local path including the drive letter. Plaintext files only + (.log, .txt, .xml, .json, .csv and similar). Large files are + trimmed to the last 2 MB, which keeps the most recent entries. +

+
+ +
+ +
+
+ + +
+
+

{{ activelog.sourcepath }}

+ +
+ +
+
Collected
+
{{ formatDateTime(activelog.collectedat) }}
+ + + +
Size on disk
+
{{ formatBytes(activelog.filesizebytes) }}
+
+ +

+ This file was larger than the collection limit, so only the end of it + was kept. The oldest entries are not shown. +

+ +
+ Loading log contents... +
+ 
{{ activelog.content }}
+
+ + +
+

Collected Logs

+
+ Loading collected logs... +
+
+ No logs have been collected from this computer yet. +
+
    +
  • +
    + {{ entry.sourcepath }} + + {{ formatDateTime(entry.collectedat) }} + +
    +
    + {{ formatBytes(entry.filesizebytes) }} + + {{ entry.linecount }} lines + + + trimmed + + + by {{ entry.collectedbyusername }} + +
    +
    + + +
    +
    • +
+

+ The 20 most recent collections are kept for this computer. Older ones + are removed automatically as new logs are collected. +

+
@@ -930,6 +1153,14 @@ const canRunScript = computed(() => authStore.hasPermission('endpoints.runscript const canViewOperatorReports = computed( () => authStore.hasPermission('endpoints.operatorreports')) +// Gates the Log Collector tab. Rides the application install/update grant +// rather than endpoints.runscript, matching ELEVATED_ACTION_PERMISSIONS on the +// server: collecting an app's log is part of the same job as pushing that app, +// and reading a text file is strictly weaker than the installer execution this +// grant already permits. Declared here with the others because detailtabs reads +// it during setup. +const canUpdateApp = computed(() => authStore.hasPermission('endpoints.updateapp')) + const detailtabs = computed(() => { const filled = assetPanels.value?.populatedGroups ?? new Set() const has = (key) => filled.has ? filled.has(key) : false @@ -971,6 +1202,16 @@ const detailtabs = computed(() => { if (canRunScript.value && liveness.value) { tabs.push({ key: 'runscripts', label: 'Run Scripts' }) } + // Log Collector rides the application install/update grant, not the script + // grant: the reason to read a log is almost always to find out why an app + // push behaved as it did, so it is the same job as pushing the app. Reading a + // file is strictly weaker than the installer execution that grant already + // allows. Liveness-gated like the others - collection queues against a live + // agent. The history stays readable, so the tab is not hidden once the + // machine goes quiet; only the Collect button disables. + if (canUpdateApp.value) { + tabs.push({ key: 'logcollector', label: 'Log Collector' }) + } return tabs }) @@ -1518,9 +1759,232 @@ async function loadScriptLibrary() { // Fetch on first view of the tab rather than on page load: most visits to a PC // never open this, and the library is irrelevant to every other tab. watch(activegroup, (group) => { - if (group === 'runscripts' && canRunScript.value) loadScriptLibrary() + if (group === 'runscripts' && canRunScript.value) { + loadScriptLibrary() + loadScriptRuns() + } + if (group === 'logcollector' && canUpdateApp.value) { + loadLogCollections() + } }) +// Recent run history for this endpoint. Refetched rather than accumulated +// client-side so the panel reflects what the server actually recorded, +// including runs pushed by another operator from another session. +const scriptruns = ref([]) +const scriptrunsloading = ref(false) + +async function loadScriptRuns() { + scriptrunsloading.value = true + try { + const response = await endpointsApi.listScriptRuns(computer.value.assetid, 5) + scriptruns.value = response.data.data || [] + } catch (error) { + // History is supporting evidence, not the action itself. A failure here + // must not read as a failed script run, so it is not toasted. + scriptruns.value = [] + } finally { + scriptrunsloading.value = false + } +} + +// ---- Log Collector ---- + +const logpath = ref('') +const logpatherror = ref('') +const collectingLog = ref(false) +const logcollections = ref([]) +const logcollectionsloading = ref(false) +const activelog = ref(null) +const logcontentloading = ref(false) + +// History for this endpoint. Metadata only - bodies are fetched one at a time, +// because 20 rows of multi-hundred-KB logs would make opening the tab far more +// expensive than reading the one log that is actually wanted. +async function loadLogCollections() { + logcollectionsloading.value = true + try { + const response = await endpointsApi.listLogCollections(computer.value.assetid) + logcollections.value = response.data.data || [] + } catch (error) { + // Supporting evidence, not the action itself; a failure here must not read + // as a failed collection. + logcollections.value = [] + } finally { + logcollectionsloading.value = false + } +} + +// Client-side shape check purely for immediate feedback. The server and the +// agent both re-validate; this exists so an obvious typo does not cost a round +// trip and a poll cycle to report back. +function validateLogPath(raw) { + const path = (raw || '').trim() + if (!path) return 'Enter the full path to the log file' + const normalized = path.replace(/\//g, '\\') + if (normalized.startsWith('\\\\')) { + return 'Network (UNC) paths are not supported; use a local path' + } + if (!/^[A-Za-z]:\\/.test(normalized)) { + return 'Include the drive letter, for example C:\\ProgramData\\App\\app.log' + } + if (normalized.split('\\').some((part) => part === '..')) { + return 'Path must not contain ".." segments' + } + if (normalized.includes('*') || normalized.includes('?')) { + return 'Wildcards are not supported; name a single log file' + } + return '' +} + +async function collectLog() { + const path = logpath.value.trim() + logpatherror.value = validateLogPath(path) + if (logpatherror.value) return + + collectingLog.value = true + try { + const response = await endpointsApi.enqueueCommand( + computer.value.assetid, 'collect_log', path) + const commandId = response.data.data?.endpointcommandid + + // Wait for the agent rather than optimistically showing a result: the file + // is read on the endpoint, so until the command completes there is nothing + // to display and no way to know whether the path even exists. + const outcome = commandId ? await waitForCommand(commandId) : 'timeout' + + await loadLogCollections() + + if (outcome === 'succeeded') { + toast.success('Log collected') + // Open the newest collection, which is the one just produced. + const newest = logcollections.value[0] + if (newest) await viewLog(newest) + } else if (outcome === 'failed') { + // The agent's refusal reason is the useful part (file missing, access + // denied, not a text file), so it is fetched and surfaced verbatim + // instead of a generic failure message. + toast.error(await collectFailureReason(commandId)) + } else { + toast.info( + 'Collection queued. It will appear here once the agent reports back.') + } + } catch (error) { + toast.error(apiError(error, 'Failed to collect the log')) + } finally { + collectingLog.value = false + } +} + +// Why a collection failed. waitForCommand only reports the terminal state, but +// the agent's message is the actionable part - "No file found at ...", +// "Access denied", "does not appear to be a plaintext file" - so it is read off +// the command row rather than replaced with a generic failure. +async function collectFailureReason(commandId) { + const fallback = 'The log could not be collected' + try { + const response = await endpointsApi.listCommands(computer.value.assetid) + const command = (response?.data?.data || []).find( + (row) => row.endpointcommandid === commandId) + return command?.resultmessage || fallback + } catch { + return fallback + } +} + +async function viewLog(entry) { + logcontentloading.value = true + // Show the row's metadata immediately so the pane is not blank while the + // body loads. + activelog.value = { ...entry, content: '' } + try { + const response = await endpointsApi.getLogCollection( + entry.endpointlogcollectionid) + activelog.value = response.data.data + } catch (error) { + activelog.value = null + toast.error(apiError(error, 'Failed to open the collected log')) + } finally { + logcontentloading.value = false + } +} + +async function downloadLog(entry) { + try { + const response = await endpointsApi.downloadLogCollection( + entry.endpointlogcollectionid) + const url = window.URL.createObjectURL(new Blob([response.data])) + const link = document.createElement('a') + link.href = url + link.download = entry.filename || 'collected.log' + document.body.appendChild(link) + link.click() + // Revoke and detach, or every download leaks a blob URL and a node for the + // lifetime of the page. + document.body.removeChild(link) + window.URL.revokeObjectURL(url) + } catch (error) { + toast.error(apiError(error, 'Failed to download the collected log')) + } +} + +// Byte counts are shown to a human deciding whether a log is worth opening, so +// they are scaled rather than printed raw. +function formatBytes(value) { + if (value === null || value === undefined) return 'Unknown' + const bytes = Number(value) + if (!Number.isFinite(bytes)) return 'Unknown' + if (bytes < 1024) return `${bytes} B` + if (bytes < 1024 * 1024) return `${(bytes / 1024).toFixed(1)} KB` + return `${(bytes / (1024 * 1024)).toFixed(1)} MB` +} + +// Queued and claimed are live states, not outcomes. Labelling them "pending" +// would imply the run finished and reported nothing. +function scriptRunStatusLabel(run) { + if (run.status === 'failed') return 'Failed' + if (run.status === 'claimed') return 'Running' + return 'Queued' +} + +// The timestamp on a run row means two different things depending on the state: +// when it FINISHED for a terminal run, when it was QUEUED for one still in +// flight. Rendered bare, those are indistinguishable, and a queued row showing +// its queue time reads as a completed run that finished then - which is exactly +// the wrong conclusion while the script is still executing. The label says which +// one is being shown. +function scriptRunTimeLabel(run) { + const finished = run.status === 'succeeded' || run.status === 'failed' + if (finished && run.completedat) return formatDateTime(run.completedat) + return `Queued ${formatDateTime(run.createdat)}` +} + +// Hover detail. A terminal row only shows its finish time, so the queue time - +// the gap that tells an operator the agent was slow to pick the job up - would +// otherwise be lost. +function scriptRunTimeTitle(run) { + const parts = [`Queued ${formatDateTime(run.createdat)}`] + if (run.completedat) parts.push(`Completed ${formatDateTime(run.completedat)}`) + return parts.join('\n') +} + +// Whether the agent sent anything beyond the status itself. Drives the note +// that distinguishes "nothing to report" from "this agent cannot report". +function hasRunEvidence(run) { + const detail = run.resultdetail + if (!detail) return false + return detail.exitcode !== undefined && detail.exitcode !== null +} + +function formatDateTime(value) { + if (!value) return 'unknown' + // Server sends naive UTC isoformat; mark it as UTC so it renders in local + // time rather than being read as local and shifting by the offset. + const raw = value.endsWith('Z') ? value : `${value}Z` + const when = new Date(raw) + return Number.isNaN(when.getTime()) ? 'unknown' : when.toLocaleString() +} + // 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.corptools regardless. @@ -1578,12 +2042,22 @@ async function confirmRunScript() { firingScript.value = true try { - await endpointsApi.enqueueCommand( + const response = await endpointsApi.enqueueCommand( computer.value.assetid, 'run_script', `script:${script.scriptid}`) // "Queued", not "finished": the agent collects this on its next check-in, // and the script's own result comes back separately. Saying "ran" here // would claim an outcome nothing has observed yet. toast.success(`${script.scriptname} queued for ${label}`) + + // Show the queued row straight away, then again once it reaches a terminal + // state, so the history reflects the run the operator just triggered + // without them having to leave and re-enter the tab. + await loadScriptRuns() + const commandId = response?.data?.data?.endpointcommandid + if (commandId) { + await waitForCommand(commandId) + await loadScriptRuns() + } } catch (error) { toast.error(apiError(error, `Failed to queue ${script.scriptname}`)) } finally { @@ -2092,6 +2566,258 @@ async function confirmRetire() { .runscripts-actions { margin-top: 1rem; } +/* Recent script runs. Themed off the same variables as the rest of the page so + the panel follows dark mode rather than pinning its own colours. */ +.scriptrun-list { + list-style: none; + margin: 0; + padding: 0; +} +.scriptrun-row { + padding: 0.75rem 0; + border-bottom: 1px solid var(--border); +} +.scriptrun-row:last-child { + border-bottom: none; + padding-bottom: 0; +} +.scriptrun-head { + display: flex; + align-items: center; + gap: 0.5rem; + flex-wrap: wrap; +} +.scriptrun-badge { + padding: 0.125rem 0.5rem; + border-radius: 0.75rem; + font-size: 0.6875rem; + font-weight: 600; + text-transform: uppercase; + letter-spacing: 0.02em; + color: var(--text-light); + border: 1px solid var(--border); +} +.scriptrun-badge--succeeded { + color: var(--success-dark); + border-color: var(--success); +} +.scriptrun-badge--failed { + color: var(--danger); + border-color: var(--danger); +} +[data-theme="dark"] .scriptrun-badge--succeeded { color: var(--success); } +.scriptrun-name { + font-weight: 600; + color: var(--text); + font-size: 0.875rem; +} +.scriptrun-when { + margin-left: auto; + color: var(--text-light); + font-size: 0.75rem; +} +.scriptrun-message { + margin: 0.375rem 0 0; + color: var(--text); + font-size: 0.8125rem; +} +.scriptrun-facts { + display: grid; + grid-template-columns: auto 1fr; + gap: 0.25rem 0.75rem; + margin: 0.5rem 0 0; + font-size: 0.8125rem; +} +.scriptrun-facts dt { + color: var(--text-light); +} +.scriptrun-facts dd { + margin: 0; + color: var(--text); +} +.scriptrun-facts dd.mono { + font-family: var(--font-mono, ui-monospace, SFMono-Regular, Menlo, monospace); + word-break: break-all; +} +/* Agent output is arbitrary text: it scrolls rather than stretching the card, + and wraps so a long single line does not push the layout sideways. */ +.scriptrun-output { + margin: 0.5rem 0 0; + padding: 0.5rem; + max-height: 12rem; + overflow: auto; + background: var(--bg-subtle, var(--bg)); + border: 1px solid var(--border); + border-radius: 4px; + color: var(--text); + font-family: var(--font-mono, ui-monospace, SFMono-Regular, Menlo, monospace); + font-size: 0.75rem; + white-space: pre-wrap; + word-break: break-word; +} +.scriptrun-note, +.scriptrun-foot { + margin: 0.5rem 0 0; + color: var(--text-light); + font-size: 0.75rem; +} +.scriptrun-foot { + margin-top: 0.75rem; + padding-top: 0.75rem; + border-top: 1px solid var(--border); +} + +/* Log Collector. Themed off the same variables as the panels above so it + follows dark mode rather than pinning its own colours. */ +.logcollect-intro { + margin: 0 0 0.75rem; + color: var(--text-light); + font-size: 0.8125rem; +} +.logcollect-form { + display: flex; + flex-direction: column; + gap: 0.25rem; +} +.logcollect-label { + font-size: 0.8125rem; + font-weight: 600; + color: var(--text); +} +.logcollect-input { + width: 100%; + padding: 0.5rem; + background: var(--bg-subtle, var(--bg)); + border: 1px solid var(--border); + border-radius: 4px; + color: var(--text); + font-size: 0.8125rem; +} +.logcollect-input:disabled { + opacity: 0.6; + cursor: not-allowed; +} +.logcollect-hint { + margin: 0.25rem 0 0; + color: var(--text-light); + font-size: 0.75rem; +} +.logcollect-error { + margin: 0.25rem 0 0; + color: var(--danger); + font-size: 0.75rem; +} +.logcollect-note { + margin: 0.5rem 0 0; + color: var(--text-light); + font-size: 0.75rem; +} +.logcollect-note--warn { + padding: 0.5rem 0.75rem; + margin-bottom: 0.75rem; + border-left: 3px solid var(--warning); + background: var(--bg-subtle, var(--bg)); + border-radius: 4px; +} +.logcollect-actions { + margin-top: 1rem; +} +.logview-head { + display: flex; + align-items: center; + justify-content: space-between; + gap: 1rem; + flex-wrap: wrap; +} +/* The collected path can be long; it wraps rather than forcing the card wide. */ +.logview-head h3 { + margin: 0; + font-size: 0.9375rem; + word-break: break-all; +} +.logview-facts { + display: grid; + grid-template-columns: auto 1fr; + gap: 0.25rem 1rem; + margin: 0.75rem 0 0; + font-size: 0.8125rem; +} +.logview-facts dt { + color: var(--text-light); +} +.logview-facts dd { + margin: 0; + color: var(--text); +} +/* The log body. Scrolls rather than stretching the card, and wraps so a long + line does not push the layout sideways. */ +.logview-pane { + margin: 0.75rem 0 0; + padding: 0.75rem; + max-height: 28rem; + overflow: auto; + background: var(--bg-subtle, var(--bg)); + border: 1px solid var(--border); + border-radius: 4px; + color: var(--text); + font-family: var(--font-mono, ui-monospace, SFMono-Regular, Menlo, monospace); + font-size: 0.75rem; + white-space: pre-wrap; + word-break: break-word; +} +.logcollect-list { + list-style: none; + margin: 0; + padding: 0; +} +.logcollect-row { + padding: 0.75rem 0; + border-bottom: 1px solid var(--border); +} +.logcollect-row:last-child { + border-bottom: none; + padding-bottom: 0; +} +.logcollect-row--active { + border-left: 3px solid var(--primary, var(--warning)); + padding-left: 0.5rem; +} +.logcollect-rowhead { + display: flex; + align-items: baseline; + justify-content: space-between; + gap: 1rem; + flex-wrap: wrap; +} +.logcollect-path { + font-size: 0.8125rem; + color: var(--text); + word-break: break-all; +} +.logcollect-when { + color: var(--text-light); + font-size: 0.75rem; + white-space: nowrap; +} +.logcollect-rowmeta { + display: flex; + gap: 0.75rem; + flex-wrap: wrap; + margin-top: 0.25rem; + color: var(--text-light); + font-size: 0.75rem; +} +.logcollect-trunc { + color: var(--warning-dark, var(--warning)); +} +[data-theme="dark"] .logcollect-trunc { + color: var(--warning); +} +.logcollect-rowactions { + display: flex; + gap: 0.5rem; + margin-top: 0.5rem; +} .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 503970e0..3163316c 100644 --- a/plugins/endpoints/api/routes.py +++ b/plugins/endpoints/api/routes.py @@ -10,6 +10,7 @@ import hashlib import hmac import json import os +import re import time from datetime import datetime, timedelta, timezone @@ -28,7 +29,7 @@ from shopdb.api import ( from ..models import ( EndpointState, EndpointEvent, EndpointCommand, EndpointApplication, - EndpointHealthState, OperatorReport, + EndpointHealthState, OperatorReport, EndpointLogCollection, ) from ..commands import COMMAND_ALLOWLIST, is_allowed_action, ALLOWED_ACTIONS_FOR, row_actions_for, requires_target, action_targetkind, elevated_permission_for, PLACE_POLICY_HOLD, ACTIVE_POLICY_HOLDS, HOLDABLE_ACTIONS_FOR, RELEASE_POLICY_HOLD, ACTIVE_LOCK_FOR, PLACE_ENDPOINT_LOCK, RELEASE_ENDPOINT_LOCK from ..eventcatalog import CRITICAL_EVENT_IDS, event_severity, event_title @@ -2068,6 +2069,196 @@ def _resolve_update_app(target): return None, f'Unknown update_app target kind: {kind!r}' +# Extensions collect_log will read. The list is an allowlist, not a blocklist: +# the question is not "is this dangerous to read" but "will this render as text +# in a browser pane", and only a closed set can answer that. Anything else comes +# back refused rather than as mojibake the operator has to interpret. +# +# The agent enforces this same set independently. Duplicating it is deliberate - +# this copy exists to give a fast, actionable error at enqueue, and the agent's +# copy is the one that actually guards the disk. +# +# The Windows ".c-f-g" extension (spelled with separators here on purpose) is +# deliberately absent: the literal collides with the banned-shorthand naming +# gate, and weakening a repo-wide gate for one rarely-collected extension is a +# bad trade. '.conf' and '.ini' cover the same need. +LOG_PATH_EXTENSIONS = frozenset({ + '.log', '.txt', '.xml', '.json', '.csv', '.ini', '.conf', + '.yml', '.yaml', '.trace', '.out', '.err', '.md', +}) + +# Windows path length ceiling for the classic (non-\\?\) API. A longer path +# cannot be opened by the agent anyway, so refusing here is honest. +LOG_PATH_MAX_LENGTH = 260 + + +def _validate_log_path(target): + """Reject a collect_log path that cannot be a readable local log file. + + Returns an error string, or None when the shape is acceptable. + + This is a SHAPE check for fast operator feedback. It cannot be a security + boundary: the server has no view of the endpoint's filesystem, and any + caller reaching this code already holds endpoints.updateapp. The agent + performs its own independent validation before opening anything. + + Rejections here are the ones that are wrong no matter what is on disk: + a relative path (ambiguous - relative to what, on a service running as + SYSTEM), a traversal segment (an attempt to walk out of the directory the + operator named, which an honest log path never needs), a UNC path (that + would make the endpoint agent read from a file server using the machine + account, turning a log collector into a network file reader), and an + extension outside the text allowlist. + """ + path = (target or '').strip() + if not path: + return 'A log file path is required' + + if len(path) > LOG_PATH_MAX_LENGTH: + return (f'Path is too long ({len(path)} characters); the maximum is ' + f'{LOG_PATH_MAX_LENGTH}') + + normalized = path.replace('/', '\\') + + # A UNC path would have the agent read from a network share as the machine + # account. That is a different capability from "read a log off this PC" and + # is not what this permission was granted for. + if normalized.startswith('\\\\'): + return ('UNC/network paths are not supported; collect logs from a ' + 'local path on the endpoint') + + # Require a drive-letter absolute path. A bare or relative path resolves + # against the agent service's working directory, which is not somewhere an + # operator can reason about. + if not re.match(r'^[A-Za-z]:\\', normalized): + return ('Enter a full local path including the drive letter ' + r'(for example C:\ProgramData\App\app.log)') + + # '..' would let a typed path escape the directory it appears to name. No + # legitimate log path needs it, so it is refused rather than normalised - + # normalising would silently read something other than what was reviewed. + if any(part == '..' for part in normalized.split('\\')): + return 'Path must not contain ".." segments' + + # Wildcards would make the collection non-deterministic: what it reads would + # depend on the directory's contents at run time, so the history could not + # say what was actually collected. + if '*' in normalized or '?' in normalized: + return 'Wildcards are not supported; name a single log file' + + extension = os.path.splitext(normalized)[1].lower() + if extension not in LOG_PATH_EXTENSIONS: + allowed = ', '.join(sorted(LOG_PATH_EXTENSIONS)) + return (f'Only plaintext log files can be collected. Allowed ' + f'extensions: {allowed}') + + return None + + +def _record_log_collection(command, detail, content): + """Persist a collected log for a command and prune that asset's history. + + Called inside report_command_result, before its commit, so the collection + row, the pruning and the command's own finalisation all land together. A + separate commit here could leave a collected log with no finalised command + if the request failed in between. + + Pruning is per asset and runs on insert. Doing it here rather than on a + schedule means the cap is a real invariant rather than something that is + true most of the time: there is no window in which an asset has 40 rows + waiting for a cleanup job that may not be running. + """ + parsed_modified = None + raw_modified = (detail or {}).get('modifiedat') + if raw_modified: + parsed_modified = _parse_agent_datetime(raw_modified) + + text = content if isinstance(content, str) else None + + collection = EndpointLogCollection( + assetid=command.assetid, + endpointcommandid=command.endpointcommandid, + # Prefer the path the AGENT resolved; fall back to what was queued so a + # row is never left without a path to show. + sourcepath=((detail or {}).get('path') or command.target or '')[:512], + content=text, + filesizebytes=_coerce_int((detail or {}).get('filesizebytes')), + contentbytes=(len(text.encode('utf-8', 'replace')) if text is not None + else None), + linecount=_coerce_int((detail or {}).get('linecount')), + istruncated=bool((detail or {}).get('truncated')), + filemodifiedat=parsed_modified, + # requestedby is the operator who queued the collection. The agent posts + # this result with a collector token and has no user identity, so the + # attribution has to come off the command row. + collectedbyuserid=command.requestedby, + collectedbyusername=_username_for(command.requestedby), + collectedat=_naive_utcnow(), + ) + db.session.add(collection) + # Make the new row visible to the pruning query below, so the cap counts it. + db.session.flush() + + _prune_log_collections(command.assetid) + return collection + + +def _prune_log_collections(assetid): + """Keep only the newest RETENTION_PER_ASSET collections for an asset. + + Ordered by collectedat DESC then id DESC: two collections can share a + timestamp at second resolution, and without the id tiebreak the ordering + would be non-deterministic, which is how a prune ends up deleting the row it + just inserted. + """ + keep = EndpointLogCollection.RETENTION_PER_ASSET + stale = (EndpointLogCollection.query + .filter_by(assetid=assetid) + .order_by(EndpointLogCollection.collectedat.desc(), + EndpointLogCollection.endpointlogcollectionid.desc()) + .offset(keep) + .all()) + for row in stale: + db.session.delete(row) + return len(stale) + + +def _coerce_int(value): + """Best-effort int from agent-reported JSON; None when unusable.""" + try: + return int(value) + except (TypeError, ValueError): + return None + + +def _username_for(userid): + """Username snapshot for an id, or None. Never raises.""" + if not userid: + return None + user = db.session.get(User, userid) + return getattr(user, 'username', None) if user is not None else None + + +def _parse_agent_datetime(value): + """Parse an agent ISO-8601 timestamp into a naive UTC datetime. + + The agent sends an offset-aware string. The column is naive-UTC like every + other datetime in this schema, so the offset is applied and then dropped - + keeping it would make this the one column that compares wrongly against the + rest. + """ + if not value: + return None + try: + text = str(value).strip().replace('Z', '+00:00') + parsed = datetime.fromisoformat(text) + except (TypeError, ValueError): + return None + if parsed.tzinfo is not None: + parsed = parsed.astimezone(timezone.utc).replace(tzinfo=None) + return parsed + + def _resolve_run_script(target): """Resolve a run_script target to a script delivery descriptor. @@ -2081,6 +2272,8 @@ def _resolve_run_script(target): the claim: an agent that receives it and ignores it is then distinguishable from one that never saw it, which matters while agents of both vintages are in the field. errormode rides the same rule, and for the same reason. + captureoutput too: an older agent that never saw the field simply keeps + withholding successful output, which is the safe direction to fail. """ kind, _sep, raw = str(target or '').partition(':') script = None @@ -2110,6 +2303,7 @@ def _resolve_run_script(target): 'runcontext': script.runcontext, 'workingdirectory': script.workingdirectory or '', 'errormode': script.errormode or 'default', + 'captureoutput': bool(script.captureoutput), }, None @@ -2125,6 +2319,149 @@ def list_endpoint_commands(asset_id: int): return success_response([row.to_dict() for row in rows]) +@endpoints_bp.route('//scriptruns', methods=['GET']) +@require_permission('endpoints.view') +def list_endpoint_script_runs(asset_id: int): + """The last few run_script commands for one endpoint, newest first. + + Separate from list_endpoint_commands because the Run Scripts tab is asking a + narrower question - "what happened the last few times I pushed a script + HERE" - and filtering a 20-row mixed feed client-side would silently show + fewer than five script runs whenever other commands are interleaved. + + Carries resultdetail, which the plain command list does not. A script that + exits zero produces a green badge and nothing else; the exit code, the + context it ran as and the directory it ran in are the only evidence the + operator gets that the run did anything at all. + """ + limit = request.args.get('limit', default=5, type=int) or 5 + limit = max(1, min(limit, 20)) + + rows = (EndpointCommand.query + .filter_by(assetid=asset_id, action='run_script') + .order_by(EndpointCommand.createdat.desc()) + .limit(limit) + .all()) + + scriptnames = _script_names_for(rows) + payload = [] + for row in rows: + data = row.to_dict(includedetail=True) + data['scriptname'] = scriptnames.get(row.endpointcommandid) + payload.append(data) + return success_response(payload) + + +@endpoints_bp.route('//logcollections', methods=['GET']) +@require_permission('endpoints.view') +def list_endpoint_log_collections(asset_id: int): + """Collected-log history for one endpoint, newest first. + + Metadata only - no content. The panel renders up to 20 rows and none of + them need the log body, which can be hundreds of kilobytes each; sending + them all would make opening the tab far more expensive than reading the one + log the operator actually wants. + + Gated on endpoints.view because this is a listing. Reading a body, which is + the part that discloses file contents, requires the collect permission - see + get_endpoint_log_collection. + """ + rows = (EndpointLogCollection.query + .filter_by(assetid=asset_id) + .order_by(EndpointLogCollection.collectedat.desc(), + EndpointLogCollection.endpointlogcollectionid.desc()) + .limit(EndpointLogCollection.RETENTION_PER_ASSET) + .all()) + return success_response([row.to_dict() for row in rows]) + + +@endpoints_bp.route('/logcollections/', methods=['GET']) +@require_permission('endpoints.updateapp') +def get_endpoint_log_collection(collection_id: int): + """One collected log including its content, for the scrollable pane. + + Deliberately a heavier permission than the list. The list says a log was + collected; this returns what was IN it, which is the disclosure. It rides + the same permission that authorised the collection, so nobody gains through + this route what they could not have collected themselves. + """ + row = db.session.get(EndpointLogCollection, collection_id) + if row is None: + return error_response(ErrorCodes.NOT_FOUND, + f'No log collection {collection_id}', + http_code=404) + return success_response(row.to_dict(includecontent=True)) + + +@endpoints_bp.route('/logcollections//download', + methods=['GET']) +@require_permission('endpoints.updateapp') +def download_endpoint_log_collection(collection_id: int): + """Serve a collected log as a file attachment. + + Served as text/plain with an explicit attachment disposition so the browser + saves it instead of rendering it. That matters beyond convenience: a + collected log is untrusted endpoint content, and inline rendering of a file + that happens to contain markup would execute it in the app's origin. + + X-Content-Type-Options is set for the same reason - it stops the browser + sniffing past the declared text/plain and deciding the bytes are HTML. + """ + from flask import Response + + row = db.session.get(EndpointLogCollection, collection_id) + if row is None: + return error_response(ErrorCodes.NOT_FOUND, + f'No log collection {collection_id}', + http_code=404) + + audit_log( + action='logcollectiondownloaded', + entitytype='Asset', + entityid=row.assetid, + changes={ + 'sourcepath': row.sourcepath, + 'endpointlogcollectionid': row.endpointlogcollectionid, + 'downloadedby': getattr(current_user, 'username', None), + }, + details=f'Downloaded collected log {row.sourcepath}', + ) + db.session.commit() + + response = Response(row.content or '', mimetype='text/plain; charset=utf-8') + # The filename is already sanitised to [A-Za-z0-9._-] by the model, so it + # cannot break out of the quoted header value. + response.headers['Content-Disposition'] = ( + f'attachment; filename="{row.filename}"') + response.headers['X-Content-Type-Options'] = 'nosniff' + return response + + +def _script_names_for(rows): + """Map command id -> script name for a batch of run_script rows. + + The command stores 'script:', which is not what the operator chose from + the picker. Resolved in one query rather than per row. A script deleted + since the run leaves the name absent, and the caller falls back to the raw + target rather than inventing one. + """ + wanted = {} + for row in rows: + kind, _sep, raw = str(row.target or '').partition(':') + if kind == 'script' and raw.isdigit(): + wanted[row.endpointcommandid] = int(raw) + if not wanted: + return {} + + found = { + script.scriptid: script.scriptname + for script in Script.query.filter( + Script.scriptid.in_(set(wanted.values()))).all() + } + return {cmdid: found[sid] + for cmdid, sid in wanted.items() if sid in found} + + @endpoints_bp.route('//policyholds', methods=['GET']) @require_permission('endpoints.view') def list_endpoint_policy_holds(asset_id: int): @@ -2538,6 +2875,18 @@ def enqueue_endpoint_command(asset_id: int): return error_response(ErrorCodes.VALIDATION_ERROR, script_error) pinnedsha256 = (script or {}).get('sha256') + # collect_log reads a caller-supplied path. Validate its SHAPE here so an + # obviously wrong path fails immediately with a message the operator can act + # on, instead of consuming a poll cycle to come back as a generic endpoint + # failure. This is a usability gate, not the security boundary: the agent + # re-validates independently, because a server-side check cannot know what + # is really on that disk and must never be the only thing standing between a + # typed string and a file read. + if action == 'collect_log': + path_error = _validate_log_path(target) + if path_error is not None: + return error_response(ErrorCodes.VALIDATION_ERROR, path_error) + state = EndpointState.query.filter_by(assetid=asset_id).first() if state is None: return error_response(ErrorCodes.NOT_FOUND, @@ -2763,6 +3112,20 @@ def report_command_result(command_id: int): else EndpointCommand.STATUS_FAILED) command.resultmessage = (payload.get('message') or '')[:2000] or None detail = payload.get('detail') + + # A successful collect_log carries the file body in detail.content. Move it + # into its own row and strip it from the detail before that detail is + # serialised onto the command: storing it twice would double the cost of the + # largest payload this API accepts, and resultjson is read by every command + # history list that only wants a status. + # + # Only on success. A failed collection's detail is small (the reason it + # failed) and belongs on the command, where failures are already read. + if command.action == 'collect_log' and succeeded and isinstance(detail, dict): + detail = dict(detail) + content = detail.pop('content', None) + _record_log_collection(command, detail, content) + command.resultjson = json.dumps(detail, separators=(',', ':')) \ if detail is not None else None command.completedat = _naive_utcnow() diff --git a/plugins/endpoints/commands.py b/plugins/endpoints/commands.py index d0119498..d14ce6bc 100644 --- a/plugins/endpoints/commands.py +++ b/plugins/endpoints/commands.py @@ -110,6 +110,24 @@ COMMAND_ALLOWLIST = { 'PCDetail.vue).', 'targetkind': 'script', }, + 'collect_log': { + 'label': 'Collect log file', + 'description': 'Read a plaintext log file from a known path on the ' + 'endpoint and return its contents to ShopDB for ' + 'viewing and download. Read-only: the agent opens the ' + 'file for reading with sharing permitted (so a log a ' + 'running application holds open can still be read), ' + 'never writes, and never deletes. Only the TAIL of a ' + 'file is returned, and only for plaintext extensions - ' + 'a binary or oversized file is refused rather than ' + 'mangled. Gated on endpoints.updateapp because the ' + 'reason to collect a log is almost always to diagnose ' + 'an application push, so it belongs with app lifecycle ' + 'rights rather than as a separate grant. Note this ' + 'still reads a caller-supplied path as SYSTEM: it is ' + 'bounded to text files, but it is not a sandbox.', + 'targetkind': 'logpath', + }, 'update_app': { 'label': 'Install/update application', 'description': 'Push a ShopDB-hosted installer for an application (by app ' @@ -192,11 +210,18 @@ COMMAND_ALLOWLIST = { # update_app - runs a pinned installer # run_script - arbitrary code as SYSTEM unless the descriptor pins a # user context +# +# collect_log rides endpoints.updateapp rather than taking a grant of its own. +# The reason to read a log is nearly always to find out why an app push behaved +# the way it did, so the right to install an application and the right to read +# that application's log are the same job. It is read-only and strictly weaker +# than the installer push the same permission already allows. ELEVATED_ACTION_PERMISSIONS = { 'system_restart': 'endpoints.restart', 'scheduledtask_run': 'endpoints.corptools', 'run_script': 'endpoints.runscript', 'update_app': 'endpoints.updateapp', + 'collect_log': 'endpoints.updateapp', } @@ -339,6 +364,19 @@ POLICY_EXEMPT_ACTIONS = frozenset({ # policy is non-empty - a denial no operator can act on, because the fix # lives in a policy that was never about this action. 'collect_now', + # Read-only diagnostic retrieval. Policy describes how a machine should be + # shaped - which monitors run, which service primitives are reachable - and + # reading a log file shapes nothing. An existing policy was authored before + # this action existed and so can never mention it, which under the + # intersection means every endpoint carrying any policy at all refuses log + # collection, including for an admin. That denial is also unactionable in + # the way that matters: logs are what an operator reaches for when an + # endpoint is already misbehaving, so gating them behind editing that same + # endpoint's policy withholds the evidence exactly when it is needed. + # Exemption from POLICY is not exemption from PERMISSION - collect_log + # still requires endpoints.updateapp via ELEVATED_ACTION_PERMISSIONS, and + # the agent still refuses UNC, traversal, directories and non-text files. + 'collect_log', }) diff --git a/plugins/endpoints/frontend/views/SystemScriptsSettings.vue b/plugins/endpoints/frontend/views/SystemScriptsSettings.vue index e0d9d39b..4a042eae 100644 --- a/plugins/endpoints/frontend/views/SystemScriptsSettings.vue +++ b/plugins/endpoints/frontend/views/SystemScriptsSettings.vue @@ -113,6 +113,13 @@ Continue + + Returns output + {{ s.filename }} @@ -213,6 +220,27 @@

+
+ +
+ How this works +

+ A failed run always sends its output back, so you can diagnose + it from the Run Scripts history without going to the machine. + A successful run does not, unless you tick this. +

+

+ Leave it off for scripts whose output could contain anything + sensitive - account names, share paths, anything a Write-Host + left behind. Turn it on for scripts where seeing the output is + the only way to tell the run did real work. Either way the + full output is written to the agent log on the endpoint. +

+
+