diff --git a/agent/ShopDbEndpointAgent.Core/CommandExecutor.cs b/agent/ShopDbEndpointAgent.Core/CommandExecutor.cs index 99277899..dffd7cd5 100644 --- a/agent/ShopDbEndpointAgent.Core/CommandExecutor.cs +++ b/agent/ShopDbEndpointAgent.Core/CommandExecutor.cs @@ -901,6 +901,7 @@ public sealed class CommandExecutor var runInUser = string.Equals( script.RunContext, "user", StringComparison.OrdinalIgnoreCase); + ApplyErrorMode(script, tempPath, name); return runInUser ? RunScriptInUserSession(name, script, tempPath) : await RunScriptAsSystem(name, script, tempPath, token); @@ -933,18 +934,203 @@ public sealed class CommandExecutor } } + // Best-effort. A missing transcript directory must not stop the script from + // running: the capture is diagnostics, and losing diagnostics is a smaller + // failure than refusing to do the work that was asked for. + private void TryEnsureTranscriptDirectory(string transcriptPath, string name) + { + try + { + var directory = Path.GetDirectoryName(transcriptPath); + if (!string.IsNullOrWhiteSpace(directory)) + Directory.CreateDirectory(directory!); + } + catch (Exception error) + { + logger.LogWarning( + error, + "run_script {Name}: could not prepare the output transcript folder; " + + "the script will still run but its output will not be captured", name); + } + } + + // Read the transcript back and delete it. Deleted whether or not the read + // worked: it sits in the operator's own profile where they could read it, + // and a script's output is not theirs to keep lying around after the agent + // has reported it. + private string ReadAndClearTranscript(string transcriptPath, string name) + { + var output = ""; + try + { + if (File.Exists(transcriptPath)) + output = CombineScriptOutput(File.ReadAllText(transcriptPath), null); + } + catch (Exception error) + { + logger.LogWarning( + error, "run_script {Name}: could not read the output transcript", name); + } + try + { + if (File.Exists(transcriptPath)) + File.Delete(transcriptPath); + } + catch (Exception error) + { + logger.LogDebug( + error, "run_script {Name}: transcript cleanup failed", name); + } + return output; + } + + // Cap on captured script output. Two different limits are at work: the + // endpoint log should stay readable, and the server payload rides an HTTP + // POST into a MEDIUMTEXT column. A runaway loop printing megabytes must not + // turn a failed script into a failed REPORT - losing the verdict entirely is + // worse than losing the tail of the evidence. Head is kept rather than tail + // because the FIRST error is the one that explains the failure; under strict + // mode nothing after it ran anyway. + private const int MaxCapturedOutputChars = 8000; + + // Merge the two streams into one operator-readable blob. stderr is labelled + // rather than concatenated blind, because "which stream said this" is the + // difference between a warning a script prints and the error that killed it. + private static string CombineScriptOutput(string? stdout, string? stderr) + { + var parts = new List(2); + if (!string.IsNullOrWhiteSpace(stdout)) + parts.Add(stdout!.TrimEnd()); + if (!string.IsNullOrWhiteSpace(stderr)) + parts.Add("[stderr] " + stderr!.TrimEnd()); + var combined = string.Join(Environment.NewLine, parts); + if (combined.Length <= MaxCapturedOutputChars) + return combined; + // Say so explicitly. Silently truncated evidence reads as a script that + // stopped talking, which is a different diagnosis than one that was cut. + return combined.Substring(0, MaxCapturedOutputChars) + + Environment.NewLine + + $"[truncated after {MaxCapturedOutputChars} characters]"; + } + + // 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). + private void LogScriptOutput(string name, string output, int? exitCode) + { + if (string.IsNullOrWhiteSpace(output)) + { + logger.LogInformation( + "run_script {Name} produced no output (exit {Exit})", + name, exitCode?.ToString() ?? "unknown"); + return; + } + logger.LogInformation( + "run_script {Name} output (exit {Exit}):{NewLine}{Output}", + name, exitCode?.ToString() ?? "unknown", Environment.NewLine, output); + } + + // Prepended to a strict-mode body. Written on ONE line so every line number + // PowerShell reports still matches the body the author sees in the WebUI + // editor: a multi-line prologue would shift them and make the error text + // point at the wrong statement during exactly the debugging session that + // needed it. + private const string StrictModePrologue = + "$ErrorActionPreference = 'Stop'; $ProgressPreference = 'SilentlyContinue'"; + + // Turn a script's non-terminating errors fatal when the library says strict. + // + // MUST run AFTER the SHA-256 check. The digest pins the bytes the SERVER + // holds; prepending first would rewrite the file and fail its own integrity + // check. Verify what was delivered, then modify how it is interpreted. + // + // Best-effort by design: if the rewrite fails the script still runs under + // PowerShell's default rules. That is the behaviour it had before this + // existed, so a failed rewrite degrades to "as before" rather than blocking + // work, and the warning says the verdict is the weaker one. + private void ApplyErrorMode(ScriptDeliveryClaim script, string scriptPath, string name) + { + if (!string.Equals(script.ErrorMode, "strict", StringComparison.OrdinalIgnoreCase)) + return; + + try + { + var body = File.ReadAllText(scriptPath); + File.WriteAllText(scriptPath, StrictModePrologue + Environment.NewLine + body); + logger.LogInformation( + "run_script {Name}: strict error mode applied; a non-terminating " + + "error will now stop the script and fail the command", name); + } + catch (Exception error) + { + logger.LogWarning( + error, + "run_script {Name}: could not apply strict error mode; running under " + + "PowerShell defaults, so a non-terminating error will NOT fail the command", + name); + } + } + + // Where the script actually runs. Empty means the agent staging directory, + // which is right for a self-contained script and wrong for one that shells + // out to a corporate tool already installed on the endpoint (Sysinternals + // and similar, delivered by Intune) and invokes it by relative path. The + // server carries that folder per script. + // + // A NAMED DIRECTORY THAT IS ABSENT FAILS THE COMMAND. Falling back to + // staging would run SYSTEM PowerShell somewhere other than where the author + // said; the script would then fail further in with an error naming the + // missing tool rather than the missing folder, or half-succeed, which is + // worse. Fail closed and name the folder. + private static bool TryResolveWorkingDirectory( + ScriptDeliveryClaim? script, string scriptPath, + out string workingDirectory, out string error) + { + error = ""; + workingDirectory = Path.GetDirectoryName(scriptPath) ?? ""; + + var requested = script?.WorkingDirectory; + if (string.IsNullOrWhiteSpace(requested)) + return true; + + if (!Directory.Exists(requested)) + { + error = $"working directory not found on this endpoint: {requested}"; + return false; + } + + workingDirectory = requested!; + return true; + } + // System context: run through the agent's own service account and wait for // the exit code, the same as before. Fine for machine-scoped scripts that do // not need the interactive user's session resources. private async Task RunScriptAsSystem( string name, ScriptDeliveryClaim script, string scriptPath, CancellationToken token) { + if (!TryResolveWorkingDirectory( + script, scriptPath, out var workingDirectory, out var directoryError)) + { + logger.LogWarning( + "run_script for {Name} not started: {Error}", name, directoryError); + return new CommandResult + { + Succeeded = false, + Message = $"Script {name} not run: {directoryError}", + Detail = ScriptDetail(name, script, scriptPath, exitCode: null), + }; + } + var startInfo = new ProcessStartInfo { FileName = "powershell.exe", UseShellExecute = false, CreateNoWindow = true, - WorkingDirectory = Path.GetDirectoryName(scriptPath) ?? "", + WorkingDirectory = workingDirectory, + RedirectStandardOutput = true, + RedirectStandardError = true, }; startInfo.ArgumentList.Add("-NoProfile"); startInfo.ArgumentList.Add("-NonInteractive"); @@ -960,19 +1146,27 @@ public sealed class CommandExecutor { Succeeded = false, Message = $"Failed to start script {name}", - Detail = ScriptDetail(name, script, scriptPath, exitCode: null), + Detail = ScriptDetail(name, script, scriptPath, exitCode: null, workingDirectory), }; } await process.WaitForExitAsync(token); + // Read only AFTER exit, for the reason spelled out on the schtasks path: + // ReadToEnd blocks until the streams close, which IS process exit, so + // reading first would make the caller's cancellation unable to fire. + var stdout = await process.StandardOutput.ReadToEndAsync(token); + var stderr = await process.StandardError.ReadToEndAsync(token); var exit = process.ExitCode; + var output = CombineScriptOutput(stdout, stderr); + LogScriptOutput(name, output, exit); return new CommandResult { Succeeded = exit == 0, Message = exit == 0 ? $"Script {name} completed (system context)" : $"Script {name} exited with code {exit}", - Detail = ScriptDetail(name, script, scriptPath, exit), + Detail = ScriptDetail(name, script, scriptPath, exit, workingDirectory, + exit == 0 ? null : output), }; } @@ -988,13 +1182,57 @@ public sealed class CommandExecutor // hung script pinned the caller forever. private static readonly TimeSpan UserScriptWaitTimeout = TimeSpan.FromMinutes(5); + // Where a user-session script's output is captured to. + // + // The SYSTEM path can simply redirect the child's stdout/stderr handles. + // This path cannot: it goes through CreateProcessAsUser to cross a session + // boundary, and inheriting pipe handles across that boundary is exactly the + // part of the launcher that is delicate (session enumeration, token + // duplication). So PowerShell redirects to a file itself and the agent reads + // it back after exit. + // + // The file lands in the OPERATOR's profile, not the agent's ProgramData + // staging folder: staging is SYSTEM-owned and the signed-in operator cannot + // write there, which is the same constraint that put the tray's own log in + // the profile. Safe to rely on here because a user-context script only runs + // when someone is interactively signed in, and a signed-in account has a + // materialised profile by definition. + private static string UserTranscriptPath(int scriptId) + => Path.Combine( + FileLoggerProvider.UserProfileLogDirectory(), + $"shopdb-script-{scriptId}-output.txt"); + private CommandResult RunScriptInUserSession( string name, ScriptDeliveryClaim script, string scriptPath) { var quotedPath = "\"" + scriptPath + "\""; + // Redirect inside the user session rather than from the service. The + // child is launched through CreateProcessAsUser, so the agent holds no + // handles to its streams; -Command wraps the same -File invocation and + // sends both streams to a file the operator can write, which the agent + // reads back after exit. 2>&1 merges stderr into the same transcript + // because ordering between the two matters more than telling them apart + // when reconstructing what a script did. + var transcriptPath = UserTranscriptPath(script.ScriptId); + var quotedTranscript = "\"" + transcriptPath + "\""; + TryEnsureTranscriptDirectory(transcriptPath, name); var arguments = - "-NoProfile -NonInteractive -ExecutionPolicy Bypass -File " + quotedPath; - var workingDirectory = Path.GetDirectoryName(scriptPath) ?? ""; + "-NoProfile -NonInteractive -ExecutionPolicy Bypass -Command " + + "\"& { & powershell.exe -NoProfile -NonInteractive -ExecutionPolicy " + + "Bypass -File " + quotedPath.Replace("\"", "\\\"") + + " *> " + quotedTranscript.Replace("\"", "\\\"") + "; exit $LASTEXITCODE }\""; + if (!TryResolveWorkingDirectory( + script, scriptPath, out var workingDirectory, out var directoryError)) + { + logger.LogWarning( + "run_script for {Name} not started: {Error}", name, directoryError); + return new CommandResult + { + Succeeded = false, + Message = $"Script {name} not run: {directoryError}", + Detail = ScriptDetail(name, script, scriptPath, exitCode: null), + }; + } var outcome = InteractiveProcessLauncher.LaunchInActiveSession( "powershell.exe", arguments, workingDirectory, waitForExit: true, @@ -1008,7 +1246,8 @@ public sealed class CommandExecutor { Succeeded = false, Message = $"Script {name} could not run in the user session: {outcome.Error}", - Detail = ScriptDetail(name, script, scriptPath, exitCode: null), + Detail = ScriptDetail(name, script, scriptPath, exitCode: null, workingDirectory, + ReadAndClearTranscript(transcriptPath, name)), }; } @@ -1018,19 +1257,22 @@ public sealed class CommandExecutor // as unresolved rather than success, because we never saw an exit // code and cannot claim it worked. logger.LogWarning( - "run_script for {Name} still running after {Timeout}; stopped waiting", - name, UserScriptWaitTimeout); + "run_script for {Name} still running after {Timeout}; stopped waiting. " + + "Partial output remains at {Transcript} while it runs", + name, UserScriptWaitTimeout, transcriptPath); return new CommandResult { Succeeded = false, Message = $"Script {name} did not finish within " + $"{UserScriptWaitTimeout.TotalMinutes:0} minutes and is still running", - Detail = ScriptDetail(name, script, scriptPath, exitCode: null), + Detail = ScriptDetail(name, script, scriptPath, exitCode: null, workingDirectory), }; } var exit = outcome.ExitCode; var ok = exit is null || exit == 0; + var output = ReadAndClearTranscript(transcriptPath, name); + LogScriptOutput(name, output, exit); return new CommandResult { Succeeded = ok, @@ -1039,12 +1281,14 @@ public sealed class CommandExecutor : ok ? $"Script {name} completed (user context)" : $"Script {name} exited with code {exit}", - Detail = ScriptDetail(name, script, scriptPath, exit), + Detail = ScriptDetail(name, script, scriptPath, exit, workingDirectory, + ok ? null : output), }; } private static Dictionary ScriptDetail( - string name, ScriptDeliveryClaim? script, string? scriptPath, int? exitCode) + string name, ScriptDeliveryClaim? script, string? scriptPath, int? exitCode, + string? workingDirectory = null, string? output = null) { var detail = new Dictionary { @@ -1056,11 +1300,27 @@ public sealed class CommandExecutor detail["scriptid"] = script.ScriptId; if (!string.IsNullOrWhiteSpace(script.RunContext)) detail["runcontext"] = script.RunContext!; + if (!string.IsNullOrWhiteSpace(script.ErrorMode)) + detail["errormode"] = script.ErrorMode!; } if (scriptPath is not null) detail["path"] = scriptPath; + // The directory the script REALLY ran in, not the one that was asked + // for. An agent too old to know the field reports nothing here, which is + // how an operator tells "ignored" apart from "honoured" while agents of + // both vintages are in the field. + if (!string.IsNullOrWhiteSpace(workingDirectory)) + 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. + if (!string.IsNullOrWhiteSpace(output)) + detail["output"] = output!; return detail; } diff --git a/agent/ShopDbEndpointAgent.Core/Contracts.cs b/agent/ShopDbEndpointAgent.Core/Contracts.cs index 911ad397..a7a1e1c1 100644 --- a/agent/ShopDbEndpointAgent.Core/Contracts.cs +++ b/agent/ShopDbEndpointAgent.Core/Contracts.cs @@ -182,6 +182,21 @@ public sealed class AppDeliveryClaim // downloads with its collector credential; Sha256 pins the exact bytes; // RunContext is 'system' (the agent's own service account) or 'user' (the // interactive logged-on session), decided per script in the ShopDB library. +// WorkingDirectory is the optional folder to run the script IN. Empty means the +// agent's staging directory, which is what every script did before the field +// existed. It is there so a script can sit beside a corporate tool already +// installed on the endpoint and invoke it by relative path; it confers no +// rights, since the body is already arbitrary PowerShell that could change its +// own directory. +// ErrorMode is 'default' or 'strict'. Default is what PowerShell does unaided: +// a non-terminating error is written and the script continues, so powershell.exe +// still exits 0 and the command reports SUCCEEDED even when the body did none of +// its work. Strict makes the agent prepend $ErrorActionPreference = 'Stop' so the +// first error terminates and the exit code means what an operator reads it to +// mean. Per script, never global: strict changes execution semantics, and a body +// 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. public sealed class ScriptDeliveryClaim { [JsonPropertyName("scriptid")] public int ScriptId { get; set; } @@ -189,6 +204,8 @@ public sealed class ScriptDeliveryClaim [JsonPropertyName("installpath")] public string? InstallPath { get; set; } [JsonPropertyName("sha256")] public string? Sha256 { get; set; } [JsonPropertyName("runcontext")] public string? RunContext { get; set; } + [JsonPropertyName("workingdirectory")] public string? WorkingDirectory { get; set; } + [JsonPropertyName("errormode")] public string? ErrorMode { 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 e3ca12f1..f8678bf3 100644 --- a/agent/ShopDbEndpointAgent/ShopDbEndpointAgent.csproj +++ b/agent/ShopDbEndpointAgent/ShopDbEndpointAgent.csproj @@ -131,6 +131,21 @@ (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.12.0 + + 1.14.0 true win-x64 true diff --git a/migrations/versions/7d48_script_workingdirectory.py b/migrations/versions/7d48_script_workingdirectory.py new file mode 100644 index 00000000..4aef61e7 --- /dev/null +++ b/migrations/versions/7d48_script_workingdirectory.py @@ -0,0 +1,41 @@ +"""Add scripts workingdirectory + +Backs the optional "run in this folder" field on a library script. The agent +otherwise runs a delivered script from its own staging directory, which is +correct for a self-contained script and wrong for one that shells out to a +corporate tool already installed on the endpoint (Sysinternals and similar, +delivered by Intune) and referenced by relative path. + +NULLABLE with no default, so every script row that existed before this +migration stays valid and keeps today's behaviour exactly: empty means the +staging directory. The path is validated in shopdb/core/api/scripts.py where a +rejection can explain itself, not in the schema where it would only produce an +IntegrityError. + +This column grants no capability. A script body is already arbitrary PowerShell +run as SYSTEM and can change its own directory; the boundary is the +endpoints.runscript grant and the digest-pinned library. + +Revision ID: 7d48_script_workingdir +Revises: 7d47_endpoints_operatorreports +Create Date: 2026-09-09 + +""" +from alembic import op +import sqlalchemy as sa + + +revision = '7d48_script_workingdir' +down_revision = '7d47_endpoints_operatorreports' +branch_labels = None +depends_on = None + + +def upgrade(): + op.add_column('scripts', sa.Column( + 'workingdirectory', sa.String(length=255), nullable=True, + comment='Folder to run the script in; empty means the agent staging dir')) + + +def downgrade(): + op.drop_column('scripts', 'workingdirectory') diff --git a/migrations/versions/7d49_script_errormode.py b/migrations/versions/7d49_script_errormode.py new file mode 100644 index 00000000..a8da063b --- /dev/null +++ b/migrations/versions/7d49_script_errormode.py @@ -0,0 +1,46 @@ +"""Add scripts errormode + +Backs the per-script "how should a non-terminating error be treated" field. + +PowerShell invoked with -File exits 0 whenever it reaches the end of the body, +even if every meaningful line inside wrote an error. The agent derives the +command verdict from that exit code, so a script could report SUCCEEDED while +doing none of its work - which is exactly how an account-provisioning script +came back green with the account only partly built. + +NOT NULL with server_default 'default', so every script row that existed before +this migration keeps today's permissive behaviour exactly. Strict is opt-in per +script and never retroactive: turning it on globally would change the meaning of +bodies that were authored under the permissive rule and deliberately tolerate a +failing Remove-Item, probe with Get-LocalUser, or loop past a bad item. Those +would begin aborting partway through, which is worse than a false green because +it leaves the endpoint half-configured. + +The value is validated in shopdb/core/api/scripts.py where a rejection can +explain itself, not in the schema where it would only produce an IntegrityError. + +Revision ID: 7d49_script_errormode +Revises: 7d48_script_workingdir +Create Date: 2026-09-09 + +""" +from alembic import op +import sqlalchemy as sa + + +revision = '7d49_script_errormode' +down_revision = '7d48_script_workingdir' +branch_labels = None +depends_on = None + + +def upgrade(): + op.add_column('scripts', sa.Column( + 'errormode', sa.String(length=16), nullable=False, + server_default='default', + comment="'default' keeps PowerShell's permissive behaviour; " + "'strict' stops on the first error so the exit code is honest")) + + +def downgrade(): + op.drop_column('scripts', 'errormode') diff --git a/plugins/computers/frontend/views/PCDetail.vue b/plugins/computers/frontend/views/PCDetail.vue index 6a8deb72..7fe76e7c 100644 --- a/plugins/computers/frontend/views/PCDetail.vue +++ b/plugins/computers/frontend/views/PCDetail.vue @@ -512,6 +512,90 @@ + +
+
+

Run Scripts

+

+ Sends a script from the ShopDB script library to this computer. The + agent fetches the body by digest and runs it in the context the + script itself specifies. +

+ +

+ A script here runs as SYSTEM unless it is configured to run as the + logged-on user. It takes effect immediately on the live computer and + cannot be undone from here. +

+ +
Loading scripts...
+
+ No runnable scripts. A script needs an uploaded body before it can be + sent; add one under Settings then return here. +
+ +
+
+ +
+ How does this work? +
+

Delivery

+

+ The body is stored here and hashed. At push time the agent downloads it + with its own credential and checks the digest before running anything, + so the bytes that run are the bytes that were reviewed. The body never + travels on the command channel and is deleted from the endpoint after + the run. +

+ +

Runs in

+

+ System is the agent's own service account: no desktop, + no mapped drives, no signed-in person's credentials. + User runs in the interactive session instead, and only + works when somebody is actually signed in. +

+ +

Run in folder

+

+ The working directory for the run. Blank means the agent's staging + folder, which is right for a self-contained script. Set it when the + script calls a tool installed separately on the endpoint by relative + path. If the folder is missing the run fails outright rather than + running somewhere unintended. +

+ +

When a line fails

+

+ This is the one worth understanding. PowerShell treats most errors as + non-terminating: it writes the error, carries on to the next + line, and still exits zero. Because the command's verdict comes from + that exit code, Continue means a script can report + success having accomplished nothing. +

+

+ Stop makes the first error end the run, so the verdict + means what you would assume it means. It is not the default because it + changes how a script behaves: a body that tolerates a failing cleanup, + probes with a cmdlet that errors when something is absent, or loops + past bad items will start aborting partway through, leaving the + endpoint half-configured. Read the script first, then switch it on. +

+ +

Where the output goes

+

+ Script output is always written to the agent log on the endpoint. It is + sent back to ShopDB only when the run fails, so routine output stays on + the machine that produced it. Long output is truncated and marked as + such. +

+

+ In Stop mode the agent adds one line to the top of the script, so an + error reported on line N refers to line N-1 of the body shown here. +

+
+
+
Loading...