From fe3fbf32d34ad9bf18ba0449fd71c63baddb1670 Mon Sep 17 00:00:00 2001 From: Bertrand Martin Date: Mon, 10 Aug 2026 21:33:29 +0200 Subject: [PATCH 1/2] Add shell environment variables: environment(name, value) and --env NAME=VALUE (#137) The WSMan Create request now carries an optional rsp:Environment block, placed per the MS-WSMV Shell_Type schema sequence (Environment, then WorkingDirectory, then the stream declarations). Like the working directory, the environment is shell-scoped: it is honored by the client's first command and replayed with its original values when a reaped shell is silently recreated. - Fluent API: CommandRequest.environment(name, value), repeatable, insertion order preserved, values XML-escaped. - SPI: environment-aware default overloads on WindowsRemoteExecutor that delegate to the historical entry points when no variable is set and refuse (never silently drop) otherwise. - CLI: --env NAME=VALUE, repeatable, split on the first '=', valid for the command and shell subcommands (winrs -env parity). - Docs: commands.md option table, cli.md options table, --help line. Co-Authored-By: Claude Fable 5 --- .../org/metricshub/winrm/CommandRequest.java | 60 ++++++++++++-- .../winrm/WindowsRemoteExecutor.java | 79 +++++++++++++++++++ .../metricshub/winrm/cli/CliArguments.java | 29 +++++++ .../org/metricshub/winrm/cli/WinRmCli.java | 28 +++++-- .../org/metricshub/winrm/light/Envelopes.java | 26 +++++- .../winrm/light/LightWinRMService.java | 32 +++++++- .../metricshub/winrm/light/WsmanClient.java | 51 ++++++++---- src/site/markdown/cli.md | 1 + src/site/markdown/commands.md | 1 + .../metricshub/winrm/StreamingApiTest.java | 52 ++++++++++++ .../winrm/WinRMClientBuilderTest.java | 2 + .../org/metricshub/winrm/WinRMClientTest.java | 58 +++++++++++++- .../winrm/cli/CliArgumentsTest.java | 54 +++++++++++++ .../metricshub/winrm/cli/WinRmCliTest.java | 40 ++++++++++ 14 files changed, 478 insertions(+), 35 deletions(-) diff --git a/src/main/java/org/metricshub/winrm/CommandRequest.java b/src/main/java/org/metricshub/winrm/CommandRequest.java index cf09591..1ad95cf 100644 --- a/src/main/java/org/metricshub/winrm/CommandRequest.java +++ b/src/main/java/org/metricshub/winrm/CommandRequest.java @@ -28,7 +28,9 @@ import java.nio.file.Path; import java.time.Duration; import java.util.ArrayList; +import java.util.LinkedHashMap; import java.util.List; +import java.util.Map; import java.util.concurrent.ExecutionException; import java.util.concurrent.TimeoutException; import java.util.function.Consumer; @@ -51,6 +53,7 @@ public final class CommandRequest { private final WinRMClient client; private final String commandLine; private String workingDirectory; + private final Map environment = new LinkedHashMap<>(); private Duration timeout; private Charset charset; private Charset stdinCharset; @@ -93,6 +96,31 @@ public CommandRequest workingDirectory(final String workingDirectory) { return this; } + /** + * Set an environment variable in the remote shell — the equivalent of {@code winrs -env}. May + * be called several times; insertion order is preserved, and setting the same name again + * replaces its value. Like {@link #workingDirectory(String)}, the environment is shell-scoped: + * the remote command shell is created on the first command a client executes and is reused + * afterward, so this setting takes effect only when this is the client's first command. + * + * 
{@code
+	 * CommandResult result = client.command("build.cmd")
+	 * 	.environment("BUILD_NUMBER", "42")
+	 * 	.environment("CONFIG", "release")
+	 * 	.execute();
+	 * }
+ * + * @param name the variable name + * @param value the variable value + * @return this request + */ + public CommandRequest environment(final String name, final String value) { + Utils.checkNonBlank(name, "name"); + Utils.checkNonNull(value, "value"); + environment.put(name, value); + return this; + } + /** * Set the timeout of this command. For {@link #execute()} it is a wall-clock deadline covering * file uploads and the command itself; for {@link #start()} it is an @@ -332,7 +360,13 @@ public CommandResult execute() { if (stdoutConsumer == null && stderrConsumer == null && stdinSource == null) { final WindowsRemoteCommandResult result = client .executor() - .executeCommand(prepared.command, prepared.workingDirectory, prepared.charset, remaining); + .executeCommand( + prepared.command, + prepared.workingDirectory, + prepared.environment, + prepared.charset, + remaining + ); return new CommandResult( result.getStdout(), @@ -396,7 +430,7 @@ public RemoteProcess start() { // round trip (inactivity), not the overall exchange the preparation steps count against. final CommandCursor cursor = client .executor() - .startCommand(prepared.command, prepared.workingDirectory, timeoutMillis, !pipeStdin); + .startCommand(prepared.command, prepared.workingDirectory, prepared.environment, timeoutMillis, !pipeStdin); if (stdinSource != null) { try { feedStdin(cursor, prepared.stdinCharset); @@ -429,17 +463,25 @@ public RemoteProcess start() { } } - /** The command, working directory and charset actually sent, after the preparation steps. */ + /** The command, working directory, environment and charset actually sent, after the preparation steps. */ private static final class Prepared { final String command; final String workingDirectory; + final Map environment; final Charset charset; final Charset stdinCharset; - Prepared(final String command, final String workingDirectory, final Charset charset, final Charset stdinCharset) { + Prepared( + final String command, + final String workingDirectory, + final Map environment, + final Charset charset, + final Charset stdinCharset + ) { this.command = command; this.workingDirectory = workingDirectory; + this.environment = environment; this.charset = charset; this.stdinCharset = stdinCharset; } @@ -454,11 +496,12 @@ private Prepared prepare(final long timeoutMillis, final long start) throws IOException, TimeoutException, WindowsRemoteException { String actualCommand = commandLine; String actualWorkingDirectory = workingDirectory; + Map actualEnvironment = environment; if (!uploads.isEmpty()) { // Copy the files through the command shell and rewrite the command to reference the - // remote copies; the transfer commands create the shell, so the working directory no - // longer applies (the shell already exists when the real command runs). + // remote copies; the transfer commands create the shell, so the working directory and + // environment no longer apply (the shell already exists when the real command runs). final List localFiles = uploads.stream().map(Path::toString).collect(Collectors.toList()); final String updatedCommand = ShellFileCopy.copyLocalFilesToRemote( client.executor(), @@ -468,11 +511,12 @@ private Prepared prepare(final long timeoutMillis, final long start) ); actualCommand = String.format("CMD.EXE /C (%s)", updatedCommand); actualWorkingDirectory = null; + actualEnvironment = null; } final Charset actualCharset = charset != null ? charset : WindowsRemoteExecutor.SHELL_OUTPUT_CHARSET; final Charset actualStdinCharset = stdinCharset != null ? stdinCharset : actualCharset; - return new Prepared(actualCommand, actualWorkingDirectory, actualCharset, actualStdinCharset); + return new Prepared(actualCommand, actualWorkingDirectory, actualEnvironment, actualCharset, actualStdinCharset); } /** @@ -489,7 +533,7 @@ private CommandResult drainWithCallbacks(final Prepared prepared, final long tim final StringBuilder stderr = new StringBuilder(); try ( CommandCursor cursor = client.executor() - .startCommand(prepared.command, prepared.workingDirectory, timeoutMillis, !pipeStdin)) { + .startCommand(prepared.command, prepared.workingDirectory, prepared.environment, timeoutMillis, !pipeStdin)) { if (stdinSource != null) { feedStdin(cursor, prepared.stdinCharset); } diff --git a/src/main/java/org/metricshub/winrm/WindowsRemoteExecutor.java b/src/main/java/org/metricshub/winrm/WindowsRemoteExecutor.java index 0748a27..7eee4b3 100644 --- a/src/main/java/org/metricshub/winrm/WindowsRemoteExecutor.java +++ b/src/main/java/org/metricshub/winrm/WindowsRemoteExecutor.java @@ -194,6 +194,47 @@ default CommandCursor startCommand( throw new UnsupportedOperationException(getClass().getName() + " does not support pipe-mode standard input."); } + /** + *

+ * Variant of {@link #startCommand(String, String, long, boolean)} that also sets environment + * variables in the remote shell. Like the working directory, the environment is shell-scoped: + * it is honored only when the shell is created, i.e. by the first command this executor runs. + *

+ *

+ * The default implementation delegates to {@link #startCommand(String, String, long, boolean)} + * when no variable is requested — an executor unaware of this variant keeps working for + * ordinary commands — and throws {@link UnsupportedOperationException} otherwise: only + * executors that can put the variables on the wire (such as the built-in lightweight backend) + * implement it, and silently dropping them would run the command in the wrong environment. + *

+ * + * @param command The command to execute + * @param workingDirectory Path of the directory for the spawned process on the remote system (can be null) + * @param environment Environment variables of the remote shell, in insertion order (can be null + * or empty for none) + * @param timeout timeout in milliseconds of each WSMan round trip — the inactivity timeout of + * the stream, not an overall deadline (throws an IllegalArgumentException if negative + * or zero) + * @param consoleModeStdin the value of the {@code WINRS_CONSOLEMODE_STDIN} option: {@code true} + * for console semantics (the historical default), {@code false} for pipe semantics + * @return a cursor over the command output, owning the executor's connection until the command + * completes or the cursor is closed — always close it (try-with-resources) + * @throws TimeoutException when the server does not answer the command startup in time + * @throws WindowsRemoteException For any problem encountered + */ + default CommandCursor startCommand( + final String command, + final String workingDirectory, + final Map environment, + final long timeout, + final boolean consoleModeStdin + ) throws TimeoutException, WindowsRemoteException { + if (environment == null || environment.isEmpty()) { + return startCommand(command, workingDirectory, timeout, consoleModeStdin); + } + throw new UnsupportedOperationException(getClass().getName() + " does not support shell environment variables."); + } + /** * Execute the command on the remote * @@ -213,6 +254,44 @@ WindowsRemoteCommandResult executeCommand( final long timeout ) throws WindowsRemoteException, TimeoutException; + /** + *

+ * Variant of {@link #executeCommand(String, String, Charset, long)} that also sets environment + * variables in the remote shell. Like the working directory, the environment is shell-scoped: + * it is honored only when the shell is created, i.e. by the first command this executor runs. + *

+ *

+ * The default implementation delegates to {@link #executeCommand(String, String, Charset, long)} + * when no variable is requested — an executor unaware of this variant keeps working for + * ordinary commands — and throws {@link UnsupportedOperationException} otherwise: only + * executors that can put the variables on the wire (such as the built-in lightweight backend) + * implement it, and silently dropping them would run the command in the wrong environment. + *

+ * + * @param command The command to execute + * @param workingDirectory Path of the directory for the spawned process on the remote system (can be null) + * @param environment Environment variables of the remote shell, in insertion order (can be null + * or empty for none) + * @param charset The charset decoding the command output; {@code null} uses + * {@link #SHELL_OUTPUT_CHARSET}, which is what the remote shell actually emits + * @param timeout Timeout in milliseconds + * @return The command result + * @throws WindowsRemoteException For any problem encountered + * @throws TimeoutException To notify userName of timeout. + */ + default WindowsRemoteCommandResult executeCommand( + final String command, + final String workingDirectory, + final Map environment, + final Charset charset, + final long timeout + ) throws WindowsRemoteException, TimeoutException { + if (environment == null || environment.isEmpty()) { + return executeCommand(command, workingDirectory, charset, timeout); + } + throw new UnsupportedOperationException(getClass().getName() + " does not support shell environment variables."); + } + /** * Get the hostname. * diff --git a/src/main/java/org/metricshub/winrm/cli/CliArguments.java b/src/main/java/org/metricshub/winrm/cli/CliArguments.java index 0258907..e120579 100644 --- a/src/main/java/org/metricshub/winrm/cli/CliArguments.java +++ b/src/main/java/org/metricshub/winrm/cli/CliArguments.java @@ -31,8 +31,10 @@ import java.nio.file.Path; import java.util.ArrayList; import java.util.Arrays; +import java.util.LinkedHashMap; import java.util.List; import java.util.Locale; +import java.util.Map; import org.metricshub.winrm.WinRMHttpProtocolEnum; import org.metricshub.winrm.service.WinRMEndpoint; import org.metricshub.winrm.service.client.auth.AuthenticationEnum; @@ -70,6 +72,7 @@ enum Operation { private final boolean kerberosRealmInferred; private final boolean forwardStdin; private final String directory; + private final Map environment; private final String input; private CliArguments(final Builder builder) { @@ -87,6 +90,7 @@ private CliArguments(final Builder builder) { kerberosRealmInferred = builder.kerberosRealmInferred; forwardStdin = builder.forwardStdin; directory = builder.directory; + environment = builder.environment; input = builder.input; } @@ -160,6 +164,9 @@ private static int parseOption(final Builder builder, final String[] arguments, case "-d": builder.directory = optionValue(arguments, index, option); return nextIndex(argument, index); + case "--env": + parseEnvironmentVariable(builder, optionValue(arguments, index, option), option); + return nextIndex(argument, index); case "--ntlm": builder.ntlm = true; return index + 1; @@ -187,6 +194,20 @@ private static int parseOption(final Builder builder, final String[] arguments, } } + /** + * Record one {@code --env NAME=VALUE} occurrence, winrs-style: the value is split on its FIRST + * {@code =} (so the variable's value may itself contain {@code =}), the name must be non-blank, + * insertion order is preserved, and repeating a name replaces its value. + */ + private static void parseEnvironmentVariable(final Builder builder, final String value, final String option) + throws CliUsageException { + final int separator = value.indexOf('='); + if (separator < 0 || value.substring(0, separator).trim().isEmpty()) { + throw new CliUsageException(option + " requires NAME=VALUE"); + } + builder.environment.put(value.substring(0, separator), value.substring(separator + 1)); + } + private static void parseOperation(final Builder builder, final String name, final List values) throws CliUsageException { if ("shell".equals(name)) { @@ -256,6 +277,9 @@ private static void validate(final Builder builder) throws CliUsageException { if (builder.directory != null && builder.operation == Operation.WQL) { throw new CliUsageException("--directory requires the command or shell subcommand"); } + if (!builder.environment.isEmpty() && builder.operation == Operation.WQL) { + throw new CliUsageException("--env requires the command or shell subcommand"); + } if (builder.operation == Operation.SHELL && builder.timeout < MIN_SHELL_TIMEOUT) { throw new CliUsageException("shell requires --timeout of at least " + MIN_SHELL_TIMEOUT + " milliseconds"); } @@ -496,6 +520,10 @@ String directory() { return directory; } + Map environment() { + return environment; + } + String input() { return input; } @@ -524,6 +552,7 @@ private static final class Builder { private boolean kerberosRealmInferred; private boolean forwardStdin; private String directory; + private final Map environment = new LinkedHashMap<>(); private Integer port; private long timeout = DEFAULT_TIMEOUT; private String input; diff --git a/src/main/java/org/metricshub/winrm/cli/WinRmCli.java b/src/main/java/org/metricshub/winrm/cli/WinRmCli.java index ff90a8c..fc652a0 100644 --- a/src/main/java/org/metricshub/winrm/cli/WinRmCli.java +++ b/src/main/java/org/metricshub/winrm/cli/WinRmCli.java @@ -256,6 +256,7 @@ private static int execute( final int exitCode = remote.executeCommand( arguments.input(), arguments.directory(), + arguments.environment(), arguments.timeout(), arguments.forwardStdin() || !localInput.terminal ? localInput.stream : null, chunk -> { @@ -300,6 +301,7 @@ private static int interactiveShell( final int exitCode = remote.shell( arguments.timeout(), arguments.directory(), + arguments.environment(), localInput.stream, standardOutput, standardError, @@ -498,6 +500,7 @@ private static String help() { " -P, --port Target port (default: HTTP 5985, HTTPS 5986)\n" + " -t, --timeout Operation timeout in milliseconds (default: 60000)\n" + " -d, --directory Working directory of the remote command or shell\n" + + " --env Environment variable of the remote command or shell (repeatable)\n" + " -i, --stdin Forward the local standard input to the remote command\n" + " --https Use HTTPS\n" + " --https-permissive Trust any HTTPS certificate and hostname (insecure)\n" + @@ -543,12 +546,14 @@ interface RemoteOperations extends AutoCloseable { /** * Run the command, forwarding each decoded output chunk to the matching consumer as it * arrives, and return the remote exit code. A non-null {@code workingDirectory} is the - * directory the command starts in. A non-null {@code stdin} is consumed to its end and - * forwarded as the command's standard input. + * directory the command starts in; a non-empty {@code environment} is set in the remote + * shell. A non-null {@code stdin} is consumed to its end and forwarded as the command's + * standard input. */ int executeCommand( String command, String workingDirectory, + Map environment, long timeout, InputStream stdin, Consumer stdoutConsumer, @@ -556,13 +561,14 @@ int executeCommand( ) throws Exception; /** - * Start {@code cmd.exe} on the remote host — in the given working directory when non-null — - * and bridge it to the given local streams until it exits; return its exit code. See - * {@link InteractiveShell}. + * Start {@code cmd.exe} on the remote host — in the given working directory when non-null, + * with the given environment variables when non-empty — and bridge it to the given local + * streams until it exits; return its exit code. See {@link InteractiveShell}. */ int shell( long timeout, String workingDirectory, + Map environment, InputStream localInput, PrintStream out, PrintStream err, @@ -618,6 +624,7 @@ public void streamWql(final String query, final long timeout, final Consumer environment, final long timeout, final InputStream stdin, final Consumer stdoutConsumer, @@ -629,10 +636,13 @@ public int executeCommand( .onStdout(stdoutConsumer) .onStderr(stderrConsumer); // A CLI invocation is one client running one command, so the API's "first command - // only" pinning of the working directory always applies here. + // only" pinning of the working directory and environment always applies here. if (workingDirectory != null) { request.workingDirectory(workingDirectory); } + if (environment != null) { + environment.forEach(request::environment); + } if (stdin != null) { request.stdin(stdin); } @@ -643,6 +653,7 @@ public int executeCommand( public int shell( final long timeout, final String workingDirectory, + final Map environment, final InputStream localInput, final PrintStream out, final PrintStream err, @@ -669,10 +680,13 @@ public int shell( .charset(encoding.charset()) .stdin(); // The session client's first (and only) command is the shell itself, so the - // working directory always takes effect. + // working directory and environment always take effect. if (workingDirectory != null) { shellRequest.workingDirectory(workingDirectory); } + if (environment != null) { + environment.forEach(shellRequest::environment); + } try (RemoteProcess process = shellRequest.start()) { return InteractiveShell.run( process, diff --git a/src/main/java/org/metricshub/winrm/light/Envelopes.java b/src/main/java/org/metricshub/winrm/light/Envelopes.java index 3e0a734..35c0115 100644 --- a/src/main/java/org/metricshub/winrm/light/Envelopes.java +++ b/src/main/java/org/metricshub/winrm/light/Envelopes.java @@ -24,6 +24,7 @@ import java.text.DecimalFormat; import java.text.DecimalFormatSymbols; import java.util.Locale; +import java.util.Map; import java.util.UUID; /** @@ -144,12 +145,15 @@ static String release(final String url, final String namespace, final String con /** * Create a command shell. * + * @param environment environment variables of the shell, in insertion order; {@code null} or + * empty omits the {@code rsp:Environment} block * @param codePage the console code page of the shell ({@code WINRS_CODEPAGE}); 0 uses * {@link #CODEPAGE_UTF8}, the default that makes every command's output UTF-8 */ static String createShell( final String url, final String workingDirectory, + final Map environment, final long timeoutMs, final int codePage ) { @@ -161,15 +165,35 @@ static String createShell( final String workingDir = (workingDirectory == null || workingDirectory.trim().isEmpty()) ? "" : "" + escape(workingDirectory) + ""; + // The MS-WSMV Shell_Type schema is a sequence: Environment, then WorkingDirectory, then the + // stream declarations — the order of the protocol's own Create example. return envelopeOpen(true) + header(url, SHELL_RESOURCE_URI, ACTION_CREATE, timeoutMs, null, optionSet) + "" + + environmentBlock(environment) + + workingDir + "stdin" + "stdout stderr" + - workingDir + ""; } + /** The {@code rsp:Environment} block of a Create request, or an empty string for no variables. */ + private static String environmentBlock(final Map environment) { + if (environment == null || environment.isEmpty()) { + return ""; + } + final StringBuilder block = new StringBuilder(""); + for (final Map.Entry variable : environment.entrySet()) { + block + .append("") + .append(escape(variable.getValue())) + .append(""); + } + return block.append("").toString(); + } + /** * Start a command in an existing shell. * diff --git a/src/main/java/org/metricshub/winrm/light/LightWinRMService.java b/src/main/java/org/metricshub/winrm/light/LightWinRMService.java index 1d1fead..329d873 100644 --- a/src/main/java/org/metricshub/winrm/light/LightWinRMService.java +++ b/src/main/java/org/metricshub/winrm/light/LightWinRMService.java @@ -327,6 +327,17 @@ public CommandCursor startCommand( final String workingDirectory, final long timeout, final boolean consoleModeStdin + ) throws TimeoutException, WindowsRemoteException { + return startCommand(command, workingDirectory, null, timeout, consoleModeStdin); + } + + @Override + public CommandCursor startCommand( + final String command, + final String workingDirectory, + final Map environment, + final long timeout, + final boolean consoleModeStdin ) throws TimeoutException, WindowsRemoteException { checkNotClosed(); Utils.checkNonNull(command, "command"); @@ -335,7 +346,7 @@ public CommandCursor startCommand( // Shell creation and command startup happen here, on the caller's thread, so failures // surface immediately rather than on the first output chunk. final WsmanClient.RemoteCommand remoteCommand = callStreaming( - () -> client.startCommand(command, workingDirectory, timeout, true, consoleModeStdin) + () -> client.startCommand(command, workingDirectory, environment, timeout, true, consoleModeStdin) ); return new CommandCursor() { @Override @@ -429,6 +440,17 @@ public WindowsRemoteCommandResult executeCommand( final String workingDirectory, final Charset charset, final long timeout + ) throws WindowsRemoteException, TimeoutException { + return executeCommand(command, workingDirectory, null, charset, timeout); + } + + @Override + public WindowsRemoteCommandResult executeCommand( + final String command, + final String workingDirectory, + final Map environment, + final Charset charset, + final long timeout ) throws WindowsRemoteException, TimeoutException { checkNotClosed(); Utils.checkNonNull(command, "command"); @@ -439,7 +461,13 @@ public WindowsRemoteCommandResult executeCommand( return executeWithTimeout( () -> { final long start = Utils.getCurrentTimeMillis(); - final WsmanClient.CommandOutput output = client.executeCommand(command, workingDirectory, charset, timeout); + final WsmanClient.CommandOutput output = client.executeCommand( + command, + workingDirectory, + environment, + charset, + timeout + ); final float executionTime = (Utils.getCurrentTimeMillis() - start) / 1000.0f; return new WindowsRemoteCommandResult(output.stdout, output.stderr, executionTime, output.exitCode); }, diff --git a/src/main/java/org/metricshub/winrm/light/WsmanClient.java b/src/main/java/org/metricshub/winrm/light/WsmanClient.java index 0fd96ca..bd85c4f 100644 --- a/src/main/java/org/metricshub/winrm/light/WsmanClient.java +++ b/src/main/java/org/metricshub/winrm/light/WsmanClient.java @@ -87,12 +87,13 @@ final class WsmanClient implements AutoCloseable { private String pendingAuthorization; private String shellId; - // The shell's working directory is pinned by the FIRST command on this connection and reused - // whenever the shell must be (re)created — e.g. after the server reaped it — so a recreation - // stays invisible to the caller instead of silently moving later commands to the default - // directory. Guarded by connectionPermit, like shellId. + // The shell's working directory and environment variables are pinned by the FIRST command on + // this connection and reused whenever the shell must be (re)created — e.g. after the server + // reaped it — so a recreation stays invisible to the caller instead of silently moving later + // commands to the default directory or environment. Guarded by connectionPermit, like shellId. private String shellWorkingDirectory; - private boolean shellWorkingDirectoryPinned; + private Map shellEnvironment; + private boolean shellSettingsPinned; // A single NTLM connection is a serial channel: one socket, stateful RC4 ciphers with sequence // numbers, and a single shellId. Concurrent callers (e.g. one executor shared across @@ -425,6 +426,7 @@ static final class CommandOutput { * * @param commandLine the command line to run * @param workingDirectory working directory of the shell (only honored when the shell is created) + * @param environment environment variables of the shell (only honored when the shell is created) * @param charset the charset decoding the output streams; {@code null} uses * {@link WindowsRemoteExecutor#SHELL_OUTPUT_CHARSET} * @param operationTimeoutMs this operation's timeout, driving the WSMan OperationTimeout header @@ -433,13 +435,22 @@ static final class CommandOutput { CommandOutput executeCommand( final String commandLine, final String workingDirectory, + final Map environment, final Charset charset, final long operationTimeoutMs ) throws Exception { final Charset cs = charset != null ? charset : WindowsRemoteExecutor.SHELL_OUTPUT_CHARSET; final ByteArrayOutputStream stdout = new ByteArrayOutputStream(); final ByteArrayOutputStream stderr = new ByteArrayOutputStream(); - try (RemoteCommand command = startCommand(commandLine, workingDirectory, operationTimeoutMs, false, true)) { + try ( + RemoteCommand command = startCommand( + commandLine, + workingDirectory, + environment, + operationTimeoutMs, + false, + true + )) { RemoteCommand.Chunk chunk; while ((chunk = command.nextChunk()) != null) { stdout.write(chunk.stdout, 0, chunk.stdout.length); @@ -461,6 +472,7 @@ CommandOutput executeCommand( * * @param commandLine the command line to run * @param workingDirectory working directory of the shell (only honored when the shell is created) + * @param environment environment variables of the shell (only honored when the shell is created) * @param operationTimeoutMs each WSMan round trip's timeout, driving the OperationTimeout * header and the socket read timeout — for a streaming consumer this is the inactivity * timeout: the longest silence tolerated between two responses @@ -474,6 +486,7 @@ CommandOutput executeCommand( RemoteCommand startCommand( final String commandLine, final String workingDirectory, + final Map environment, final long operationTimeoutMs, final boolean failOnQuietTimeout, final boolean consoleModeStdin @@ -484,12 +497,17 @@ RemoteCommand startCommand( boolean opened = false; try { configureTimeouts(operationTimeoutMs, failOnQuietTimeout); - if (!shellWorkingDirectoryPinned) { + if (!shellSettingsPinned) { shellWorkingDirectory = workingDirectory; - shellWorkingDirectoryPinned = true; + // A defensive copy: a recreation must replay exactly what the first command set, not + // whatever the caller's map contains by then. + shellEnvironment = environment == null || environment.isEmpty() + ? null + : new LinkedHashMap<>(environment); + shellSettingsPinned = true; } if (shellId == null) { - createShell(shellWorkingDirectory, operationTimeoutMs, failOnQuietTimeout); + createShell(shellWorkingDirectory, shellEnvironment, operationTimeoutMs, failOnQuietTimeout); } // The caller's timeout may have fired while the Create response was being awaited (socket // reads do not observe interrupts): never START the command after the reported timeout. @@ -503,9 +521,10 @@ RemoteCommand startCommand( } // The server reaped the cached shell between commands (e.g. its IdleTimeout expired on a // long-lived client). The Command was rejected before it could run, so it is safe to - // recreate the shell — with its ORIGINAL working directory — and retry once. + // recreate the shell — with its ORIGINAL working directory and environment — and retry + // once. shellId = null; - createShell(shellWorkingDirectory, operationTimeoutMs, failOnQuietTimeout); + createShell(shellWorkingDirectory, shellEnvironment, operationTimeoutMs, failOnQuietTimeout); checkNotCancelled(); commandId = sendCommand(commandLine, operationTimeoutMs, failOnQuietTimeout, consoleModeStdin); } @@ -877,10 +896,14 @@ public void close() throws Exception { } } - private void createShell(final String workingDirectory, final long timeoutMs, final boolean failOnQuietTimeout) - throws Exception { + private void createShell( + final String workingDirectory, + final Map environment, + final long timeoutMs, + final boolean failOnQuietTimeout + ) throws Exception { final Document doc = exchange( - Envelopes.createShell(url, workingDirectory, timeoutMs, consoleCodePage), + Envelopes.createShell(url, workingDirectory, environment, timeoutMs, consoleCodePage), "Create shell", timeoutMs, failOnQuietTimeout diff --git a/src/site/markdown/cli.md b/src/site/markdown/cli.md index 9f5d136..e886f86 100644 --- a/src/site/markdown/cli.md +++ b/src/site/markdown/cli.md @@ -42,6 +42,7 @@ takes no argument. | `-P, --port ` | Target port. Default: 5985 for HTTP, 5986 for HTTPS. | | `-t, --timeout ` | Operation timeout in milliseconds. Default: 60000. See [Timeout semantics](#Timeout_semantics). | | `-d, --directory ` | Working directory the remote command or interactive shell starts in, like `winrs -d` (only with `command` and `shell`). Default: the remote user's profile directory. | +| `--env ` | Environment variable set in the remote shell, like `winrs -env` (only with `command` and `shell`). Repeatable — one occurrence per variable; the value is split on the first `=`, so it may itself contain `=`. | | `-i, --stdin` | Forward the local standard input to the remote command (only with `command`); see below. | | `--https` | Connect over HTTPS. | | `--https-permissive` | Trust any HTTPS certificate and hostname. Intentionally insecure: testing and isolated hosts only. Requires `--https`. | diff --git a/src/site/markdown/commands.md b/src/site/markdown/commands.md index 8139bc5..63536e9 100644 --- a/src/site/markdown/commands.md +++ b/src/site/markdown/commands.md @@ -46,6 +46,7 @@ Everything between `command(...)` and `execute()` is optional: | `timeout(Duration)` | the client's timeout | Wall-clock deadline covering file uploads and the command itself with `execute()`; inactivity timeout with `start()`. | | `charset(Charset)` | `UTF-8` | The charset used to decode the command output (see below). | | `workingDirectory(String)` | remote default | Working directory of the remote process. The remote shell is created by the client's **first** command and reused afterward, so this only takes effect on that first command. | +| `environment(String, String)` | none | Environment variable set in the remote shell, like `winrs -env` — call it once per variable, insertion order is preserved. Shell-scoped like `workingDirectory`: only takes effect on the client's **first** command. | | `upload(Path...)` | none | Local files to copy to the host before running (see below). | | `stdin(String)` / `stdin(Path)` / `stdin(InputStream)` | none | Standard input fed to the command — the remote equivalent of a `< file` redirection (see below). | | `stdin()` | console semantics | Declare interactive input through `RemoteProcess.stdin()` (with `start()`): pipe semantics without pre-supplied content (see below). | diff --git a/src/test/java/org/metricshub/winrm/StreamingApiTest.java b/src/test/java/org/metricshub/winrm/StreamingApiTest.java index a9fbb89..014deca 100644 --- a/src/test/java/org/metricshub/winrm/StreamingApiTest.java +++ b/src/test/java/org/metricshub/winrm/StreamingApiTest.java @@ -42,6 +42,7 @@ import java.util.ArrayList; import java.util.Iterator; import java.util.List; +import java.util.Map; import java.util.stream.Stream; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; @@ -793,6 +794,57 @@ public CommandCursor startCommand(final String command, final String workingDire assertThrows(UnsupportedOperationException.class, () -> legacy.startCommand("dir", null, 1000, false)); } + @Test + void executorsUnawareOfEnvironmentVariablesKeepWorkingWhenNoneAreSet() throws Exception { + // A pre-existing executor overrides only the historical entry points: the environment-aware + // defaults must delegate to them when no variable is requested, and must refuse — never + // silently drop the variables — otherwise. + final CommandCursor canned = new CommandCursor() { + @Override + public Chunk next() { + return null; + } + + @Override + public int exitCode() { + return 0; + } + + @Override + public void close() {} + }; + final WindowsRemoteCommandResult cannedResult = new WindowsRemoteCommandResult("out", "", 0.1f, 0); + final WindowsRemoteExecutor legacy = new ScriptedWindowsRemoteExecutor() { + @Override + public CommandCursor startCommand(final String command, final String workingDirectory, final long timeout) { + return canned; + } + + @Override + public WindowsRemoteCommandResult executeCommand( + final String command, + final String workingDirectory, + final java.nio.charset.Charset charset, + final long timeout + ) { + return cannedResult; + } + }; + + assertEquals(canned, legacy.startCommand("dir", null, null, 1000, true)); + assertEquals(canned, legacy.startCommand("dir", null, Map.of(), 1000, true)); + assertEquals(cannedResult, legacy.executeCommand("dir", null, null, StandardCharsets.UTF_8, 1000)); + assertEquals(cannedResult, legacy.executeCommand("dir", null, Map.of(), StandardCharsets.UTF_8, 1000)); + assertThrows( + UnsupportedOperationException.class, + () -> legacy.startCommand("dir", null, Map.of("A", "1"), 1000, true) + ); + assertThrows( + UnsupportedOperationException.class, + () -> legacy.executeCommand("dir", null, Map.of("A", "1"), StandardCharsets.UTF_8, 1000) + ); + } + private static byte[] concat(final byte[] a, final byte[] b) { final byte[] result = new byte[a.length + b.length]; System.arraycopy(a, 0, result, 0, a.length); diff --git a/src/test/java/org/metricshub/winrm/WinRMClientBuilderTest.java b/src/test/java/org/metricshub/winrm/WinRMClientBuilderTest.java index 45882f2..2b0dd66 100644 --- a/src/test/java/org/metricshub/winrm/WinRMClientBuilderTest.java +++ b/src/test/java/org/metricshub/winrm/WinRMClientBuilderTest.java @@ -134,6 +134,8 @@ void commandRequestValidatesItsOptions() { assertThrows(IllegalArgumentException.class, () -> client.command(" ")); final CommandRequest request = client.command("ipconfig"); assertThrows(IllegalArgumentException.class, () -> request.workingDirectory(" ")); + assertThrows(IllegalArgumentException.class, () -> request.environment(" ", "value")); + assertThrows(IllegalArgumentException.class, () -> request.environment("NAME", null)); assertThrows(IllegalArgumentException.class, () -> request.timeout(Duration.ZERO)); assertThrows(IllegalArgumentException.class, () -> request.charset(null)); assertThrows(IllegalArgumentException.class, () -> request.upload((java.nio.file.Path) null)); diff --git a/src/test/java/org/metricshub/winrm/WinRMClientTest.java b/src/test/java/org/metricshub/winrm/WinRMClientTest.java index 9353732..dfa2617 100644 --- a/src/test/java/org/metricshub/winrm/WinRMClientTest.java +++ b/src/test/java/org/metricshub/winrm/WinRMClientTest.java @@ -239,6 +239,47 @@ void commandReturnsTypedResult() throws Exception { assertTrue(requests.get(1).contains("mycommand.exe"), requests.get(1)); } + @Test + void environmentVariablesAreSentInTheCreateShellRequest() throws Exception { + server + .enqueue(200, envelope(resourceCreated("SHELL-1"))) + .enqueue(200, envelope(commandResponse("CMD-1"))) + .enqueue( + 200, + envelope(receiveResponse(stream("stdout", "CMD-1", "42".getBytes(StandardCharsets.UTF_8)), done("CMD-1", 0))) + ) + .enqueue(200, envelope(signalResponse())); + + try (WinRMClient client = builder(PASSWORD).build()) { + final CommandResult result = client + .command("echo %BUILD_NUMBER%") + .environment("BUILD_NUMBER", "42") + .environment("CONFIG", "a" + + "42" + + "a<b&"c"" + + "" + ), + create + ); + // The MS-WSMV Shell_Type schema sequence: Environment, then WorkingDirectory, then the + // stream declarations. + final int environment = create.indexOf(""); + final int workingDirectory = create.indexOf(""); + final int inputStreams = create.indexOf(""); + assertTrue(environment < workingDirectory && workingDirectory < inputStreams, create); + } + @Test void commandDecodesOutputAsUtf8WithoutProbingTheRemoteCodeSet() throws Exception { server @@ -522,7 +563,13 @@ void expiredCachedShellIsRecreatedAndTheCommandRetried() throws Exception { try (WinRMClient client = builder(PASSWORD).build()) { assertEquals( "first", - client.command("first.exe").workingDirectory("C:\\Work").charset(StandardCharsets.UTF_8).execute().stdout() + client + .command("first.exe") + .workingDirectory("C:\\Work") + .environment("BUILD_NUMBER", "42") + .charset(StandardCharsets.UTF_8) + .execute() + .stdout() ); assertEquals("second", client.command("second.exe").charset(StandardCharsets.UTF_8).execute().stdout()); } @@ -540,9 +587,14 @@ void expiredCachedShellIsRecreatedAndTheCommandRetried() throws Exception { .collect(java.util.stream.Collectors.toList()); assertEquals(2, creates.size()); assertEquals(2, requests.stream().filter(r -> r.contains(">second.exe<")).count()); - // The recreated shell keeps the working directory pinned by the FIRST command, even though - // the retried command did not set one. + // The recreated shell keeps the working directory AND the environment pinned by the FIRST + // command, even though the retried command did not set them. assertTrue(creates.get(1).contains("C:\\Work"), creates.get(1)); + assertTrue( + creates.get(1) + .contains("42"), + creates.get(1) + ); } @Test diff --git a/src/test/java/org/metricshub/winrm/cli/CliArgumentsTest.java b/src/test/java/org/metricshub/winrm/cli/CliArgumentsTest.java index 68a620a..b66cd72 100644 --- a/src/test/java/org/metricshub/winrm/cli/CliArgumentsTest.java +++ b/src/test/java/org/metricshub/winrm/cli/CliArgumentsTest.java @@ -158,6 +158,52 @@ void parsesTheRemoteWorkingDirectory() throws Exception { } } + @Test + void parsesRepeatableEnvironmentVariables() throws Exception { + try ( + CliArguments parsed = CliArguments.parse( + new String[] + { + "-h", + "host", + "-u", + "user", + "-p", + "secret", + "--env", + "BUILD_NUMBER=42", + "--env=CONFIG=release", + "--env", + "OPTIONS=a=b", + "command", + "build.cmd" + } + )) { + // Insertion order preserved; the value is split on the FIRST '=' only. + assertEquals(List.of("BUILD_NUMBER", "CONFIG", "OPTIONS"), List.copyOf(parsed.environment().keySet())); + assertEquals("42", parsed.environment().get("BUILD_NUMBER")); + assertEquals("release", parsed.environment().get("CONFIG")); + assertEquals("a=b", parsed.environment().get("OPTIONS")); + } + // A repeated name replaces the value; an empty value is allowed (winrs-style). + try ( + CliArguments parsed = CliArguments.parse( + new String[] + { "-h", "host", "-u", "user", "-p", "secret", "--env", "A=1", "--env", "A=2", "--env", "B=", "shell" } + )) { + assertEquals("2", parsed.environment().get("A")); + assertEquals("", parsed.environment().get("B")); + } + // Without the option, the environment is empty. + try ( + CliArguments parsed = CliArguments.parse( + new String[] + { "-h", "host", "-u", "user", "-p", "secret", "command", "whoami" } + )) { + assertTrue(parsed.environment().isEmpty()); + } + } + @Test void acceptsEveryCommandAlias() throws Exception { for (final String alias : List.of("command", "cmd", "exec", "run")) { @@ -247,6 +293,14 @@ void rejectsInvalidArguments() { "--directory requires the command or shell subcommand", concat(base, "-d", "C:\\build", "wql", "SELECT Name FROM Win32_Service") }, + { "--env requires a value", concat(base, "--env") }, + { "--env requires NAME=VALUE", concat(base, "--env", "NOEQUALS", "command", "whoami") }, + { "--env requires NAME=VALUE", concat(base, "--env", "=value", "command", "whoami") }, + { "--env requires NAME=VALUE", concat(base, "--env", " =value", "command", "whoami") }, + { + "--env requires the command or shell subcommand", + concat(base, "--env", "A=b", "wql", "SELECT Name FROM Win32_Service") + }, { "-P must be between 1 and 65535", concat(base, "-P", "65536", "command", "whoami") }, { "-t must be greater than zero", concat(base, "-t", "0", "command", "whoami") }, { "missing subcommand (wql, command, or shell)", base }, diff --git a/src/test/java/org/metricshub/winrm/cli/WinRmCliTest.java b/src/test/java/org/metricshub/winrm/cli/WinRmCliTest.java index 8745d8b..d7ef394 100644 --- a/src/test/java/org/metricshub/winrm/cli/WinRmCliTest.java +++ b/src/test/java/org/metricshub/winrm/cli/WinRmCliTest.java @@ -56,6 +56,7 @@ void helpAndVersionDoNotConnect() throws Exception { assertTrue(help.stdout.contains("[options] shell")); assertTrue(help.stdout.contains("-P, --port")); assertTrue(help.stdout.contains("-d, --directory")); + assertTrue(help.stdout.contains("--env ")); assertTrue(help.stdout.contains("--kerberos-kdc")); assertTrue(help.stdout.contains("--kerberos-realm")); // The details (streaming behavior, password files, exit codes) live in the online manual. @@ -128,6 +129,27 @@ void passesTheDirectoryOptionThroughAsTheWorkingDirectory() throws Exception { org.junit.jupiter.api.Assertions.assertNull(defaulted.workingDirectory); } + @Test + void passesTheEnvOptionsThroughAsTheShellEnvironment() throws Exception { + final FakeRemote remote = new FakeRemote(); + final Invocation invocation = invoke( + concat(REQUIRED, "--env", "BUILD_NUMBER=42", "--env", "CONFIG=release", "exec", "build.cmd"), + args -> remote + ); + assertEquals(0, invocation.exitCode); + assertEquals(Map.of("BUILD_NUMBER", "42", "CONFIG", "release"), remote.environment); + + // The shell subcommand gets the environment too. + final FakeRemote shellRemote = new FakeRemote(); + assertEquals(0, invoke(concat(REQUIRED, "--env=CONFIG=release", "shell"), args -> shellRemote).exitCode); + assertEquals(Map.of("CONFIG", "release"), shellRemote.environment); + + // Without the option, an empty environment is passed: the remote default applies. + final FakeRemote defaulted = new FakeRemote(); + assertEquals(0, invoke(concat(REQUIRED, "exec", "build.cmd"), args -> defaulted).exitCode); + assertTrue(defaulted.environment.isEmpty()); + } + @Test void sendsTheWorkingDirectoryInTheCreateShellRequest() throws Exception { // Full stack against the in-process WSMan server, through the CLI's real connect factory: @@ -152,6 +174,8 @@ void sendsTheWorkingDirectoryInTheCreateShellRequest() throws Exception { "30000", "-d", "C:\\build", + "--env", + "BUILD_NUMBER=42", "exec", "build.cmd" }, @@ -162,6 +186,11 @@ void sendsTheWorkingDirectoryInTheCreateShellRequest() throws Exception { assertEquals("ok", invocation.stdout); final String create = server.decryptedRequests().get(0); assertTrue(create.contains("C:\\build"), create); + // --env reaches the wire too, as the rsp:Environment block of the same Create request. + assertTrue( + create.contains("42"), + create + ); } } @@ -232,6 +261,8 @@ public int read() { "10000", "-d", "C:\\build", + "--env", + "CONFIG=release", "shell" }, WinRmCli::connect, @@ -244,6 +275,10 @@ public int read() { final String create = requests.get(1); assertTrue(create.contains("1252"), create); assertTrue(create.contains("C:\\build"), create); + assertTrue( + create.contains("release"), + create + ); final String command = requests.get(2); assertTrue(command.contains("cmd.exe /Q"), command); assertTrue(command.contains("FALSE"), command); @@ -665,6 +700,7 @@ private static final class FakeRemote implements WinRmCli.RemoteOperations { private Exception failure; private String command; private String workingDirectory; + private Map environment; private java.io.InputStream forwardedStdin; private boolean shellStarted; private boolean closed; @@ -680,6 +716,7 @@ public void streamWql(final String query, final long timeout, final Consumer environment, final long timeout, final java.io.InputStream stdin, final Consumer stdoutConsumer, @@ -687,6 +724,7 @@ public int executeCommand( ) throws Exception { this.command = command; this.workingDirectory = workingDirectory; + this.environment = environment; this.forwardedStdin = stdin; failIfConfigured(); stdoutChunks.forEach(stdoutConsumer); @@ -698,6 +736,7 @@ public int executeCommand( public int shell( final long timeout, final String workingDirectory, + final Map environment, final java.io.InputStream localInput, final java.io.PrintStream out, final java.io.PrintStream err, @@ -705,6 +744,7 @@ public int shell( ) throws Exception { shellStarted = true; this.workingDirectory = workingDirectory; + this.environment = environment; failIfConfigured(); stdoutChunks.forEach(chunk -> { out.print(chunk); From 798d200d4fb9525f23e2413fb9764c646f956fec Mon Sep 17 00:00:00 2001 From: Bertrand Martin Date: Mon, 10 Aug 2026 23:43:28 +0200 Subject: [PATCH 2/2] Carry the shell environment into the transfer commands that create the shell (#137) When .environment(...) is combined with .upload(...), the ShellFileCopy transfer commands run first and are what actually creates (and pins the settings of) the remote shell; the environment was discarded after preparation and the real command silently ran without it. ShellFileCopy.copyLocalFilesToRemote now takes the shell environment and threads it into every command leg: whichever leg creates the shell pins it, and the caller's command inherits it. The historical entry point delegates with no environment, so the legacy WinRMCommandExecutor and custom executors are unaffected. Tests: transfer legs carry the environment (ShellFileCopyTest), and an end-to-end wire test asserts the Create request of an upload-created shell carries the rsp:Environment block (WinRMClientTest). Addresses the Codex review on PR #156. Co-Authored-By: Claude Fable 5 --- .../org/metricshub/winrm/CommandRequest.java | 8 +- .../org/metricshub/winrm/ShellFileCopy.java | 94 +++++++++++++++++-- .../metricshub/winrm/ShellFileCopyTest.java | 82 +++++++++++++++- .../org/metricshub/winrm/WinRMClientTest.java | 65 +++++++++++++ 4 files changed, 236 insertions(+), 13 deletions(-) diff --git a/src/main/java/org/metricshub/winrm/CommandRequest.java b/src/main/java/org/metricshub/winrm/CommandRequest.java index 1ad95cf..fb3a820 100644 --- a/src/main/java/org/metricshub/winrm/CommandRequest.java +++ b/src/main/java/org/metricshub/winrm/CommandRequest.java @@ -500,17 +500,21 @@ private Prepared prepare(final long timeoutMillis, final long start) if (!uploads.isEmpty()) { // Copy the files through the command shell and rewrite the command to reference the - // remote copies; the transfer commands create the shell, so the working directory and - // environment no longer apply (the shell already exists when the real command runs). + // remote copies. The transfer commands are what actually creates the shell, so the + // shell-scoped environment must ride them — the real command then inherits it. The + // working directory is not carried over: with uploads it has never applied, and the + // transfer commands were built for the default directory. final List localFiles = uploads.stream().map(Path::toString).collect(Collectors.toList()); final String updatedCommand = ShellFileCopy.copyLocalFilesToRemote( client.executor(), commandLine, localFiles, + environment, TimeoutHelper.getRemainingTime(timeoutMillis, start, "No time left to copy the local files") ); actualCommand = String.format("CMD.EXE /C (%s)", updatedCommand); actualWorkingDirectory = null; + // Already applied when the transfer commands created the shell. actualEnvironment = null; } diff --git a/src/main/java/org/metricshub/winrm/ShellFileCopy.java b/src/main/java/org/metricshub/winrm/ShellFileCopy.java index a3f477c..388ce89 100644 --- a/src/main/java/org/metricshub/winrm/ShellFileCopy.java +++ b/src/main/java/org/metricshub/winrm/ShellFileCopy.java @@ -30,6 +30,7 @@ import java.util.Base64; import java.util.List; import java.util.Locale; +import java.util.Map; import java.util.Optional; import java.util.concurrent.TimeoutException; import java.util.regex.Pattern; @@ -141,6 +142,34 @@ public static String copyLocalFilesToRemote( final String command, final List localFiles, final long timeout + ) throws IOException, TimeoutException, WindowsRemoteException { + return copyLocalFilesToRemote(windowsRemoteExecutor, command, localFiles, null, timeout); + } + + /** + * Variant of {@link #copyLocalFilesToRemote(WindowsRemoteExecutor, String, List, long)} that + * also sets environment variables in the remote shell. The transfer commands are the first + * commands the executor runs, so they are what actually creates (and pins the settings of) + * the shell the caller's command will then run in: shell-scoped settings must ride them, or + * they would be silently lost. + * + * @param windowsRemoteExecutor Executor connected to the remote host (mandatory) + * @param command The command referencing the local files (mandatory) + * @param localFiles The list of local files to copy (may be null or empty: no-op) + * @param environment Environment variables of the remote shell, in insertion order (can be + * null or empty for none) + * @param timeout Timeout in milliseconds (throws an IllegalArgumentException if negative or zero) + * @return The command updated with the remote paths of the copied files + * @throws IOException If a local file cannot be read + * @throws TimeoutException To notify userName of timeout + * @throws WindowsRemoteException For any problem encountered on the remote host + */ + public static String copyLocalFilesToRemote( + final WindowsRemoteExecutor windowsRemoteExecutor, + final String command, + final List localFiles, + final Map environment, + final long timeout ) throws IOException, TimeoutException, WindowsRemoteException { Utils.checkNonNull(windowsRemoteExecutor, "windowsRemoteExecutor"); Utils.checkNonNull(command, "command"); @@ -171,6 +200,7 @@ public static String copyLocalFilesToRemote( String .format("forfiles /P \"%s\" /D -%d /C \"cmd /c del /f /q @path\" 2>NUL & ", remoteDirectory, CLEANUP_AGE_DAYS) + WindowsTempShare.buildCreateRemoteDirectoryCommand(remoteDirectory), + environment, "create the remote temporary directory", timeout, start @@ -178,7 +208,14 @@ public static String copyLocalFilesToRemote( String updatedCommand = command; for (final String localFile : localFiles) { - final String remoteFile = copyFile(windowsRemoteExecutor, Paths.get(localFile), remoteDirectory, timeout, start); + final String remoteFile = copyFile( + windowsRemoteExecutor, + Paths.get(localFile), + remoteDirectory, + environment, + timeout, + start + ); updatedCommand = WindowsRemoteProcessUtils.caseInsensitiveReplace(updatedCommand, localFile, remoteFile); } @@ -193,6 +230,7 @@ public static String copyLocalFilesToRemote( * @param windowsRemoteExecutor Executor connected to the remote host * @param localPath The local file to copy * @param remoteDirectory The existing remote directory receiving the file + * @param environment Environment variables of the remote shell (can be null or empty for none) * @param timeout Timeout in milliseconds * @param start Operation start time in milliseconds * @return the path of the file on the remote host @@ -204,6 +242,7 @@ static String copyFile( final WindowsRemoteExecutor windowsRemoteExecutor, final Path localPath, final String remoteDirectory, + final Map environment, final long timeout, final long start ) throws IOException, TimeoutException, WindowsRemoteException { @@ -227,7 +266,7 @@ static String copyFile( final String remoteFile = remoteDirectory + "\\" + contentAddressedName(fileName, content, maxRemoteNameLength(remoteDirectory)); - transferContent(windowsRemoteExecutor, localPath, content, remoteFile, timeout, start); + transferContent(windowsRemoteExecutor, localPath, content, remoteFile, environment, timeout, start); return remoteFile; } @@ -285,12 +324,13 @@ public static void copyLocalFileToRemoteFile( runChecked( windowsRemoteExecutor, WindowsTempShare.buildCreateRemoteDirectoryCommand(remoteDirectory), + null, "create the remote directory", timeout, start ); - transferContent(windowsRemoteExecutor, localPath, content, remoteFile, timeout, start); + transferContent(windowsRemoteExecutor, localPath, content, remoteFile, null, timeout, start); } /** @@ -322,6 +362,7 @@ static void checkEmbeddableRemotePath(final String path) { * @param localPath The local file, for the failure messages * @param content The file content * @param remoteFile The destination path on the remote host + * @param environment Environment variables of the remote shell (can be null or empty for none) * @param timeout Timeout in milliseconds * @param start Operation start time in milliseconds * @throws TimeoutException To notify userName of timeout @@ -332,13 +373,20 @@ private static void transferContent( final Path localPath, final byte[] content, final String remoteFile, + final Map environment, final long timeout, final long start ) throws TimeoutException, WindowsRemoteException { // Skip the transfer if the remote host already has an identical copy. A destination that // exists with a DIFFERENT digest (e.g. a cached copy corrupted or modified in place) is // remembered: it must be repaired by replacement, not trusted. - final Optional existing = remoteDigest(windowsRemoteExecutor, remoteFile, timeout, start); + final Optional existing = remoteDigest( + windowsRemoteExecutor, + remoteFile, + environment, + timeout, + start + ); if (existing.isPresent() && existing.get().matches(content)) { return; } @@ -354,6 +402,7 @@ private static void transferContent( : String.format("IF NOT EXIST \"%s\" TYPE NUL >\"%s\"", remoteFile, remoteFile)) + " & " + digestProbe(remoteFile), + environment, "create an empty file", timeout, start @@ -375,7 +424,7 @@ private static void transferContent( // so a concurrent operation can never invalidate a copy another operation verified. final String stagingFile = String.format("%s.%s.part", remoteFile, uniqueSuffix()); try { - upload(windowsRemoteExecutor, content, stagingFile, localPath, timeout, start); + upload(windowsRemoteExecutor, content, stagingFile, localPath, environment, timeout, start); publish( windowsRemoteExecutor, @@ -384,11 +433,12 @@ private static void transferContent( mismatchedDestination, content, localPath, + environment, timeout, start ); } catch (final TimeoutException | WindowsRemoteException | RuntimeException e) { - bestEffortDelete(windowsRemoteExecutor, timeout, start, stagingFile); + bestEffortDelete(windowsRemoteExecutor, environment, timeout, start, stagingFile); throw e; } @@ -426,6 +476,7 @@ private static WindowsRemoteException integrityCheckFailure( * must be replaced * @param content The expected file content * @param localPath The local file, for the failure message + * @param environment Environment variables of the remote shell (can be null or empty for none) * @param timeout Timeout in milliseconds * @param start Operation start time in milliseconds * @throws TimeoutException To notify userName of timeout @@ -439,6 +490,7 @@ private static void publish( final boolean replaceMismatched, final byte[] content, final Path localPath, + final Map environment, final long timeout, final long start ) throws TimeoutException, WindowsRemoteException { @@ -453,6 +505,7 @@ private static void publish( )) + " & " + digestProbe(remoteFile), + environment, "publish the transferred file", timeout, start @@ -489,6 +542,7 @@ private static String uniqueSuffix() { * @param content The file content * @param remoteFile The target path on the remote host * @param localPath The local file, for the failure message + * @param environment Environment variables of the remote shell (can be null or empty for none) * @param timeout Timeout in milliseconds * @param start Operation start time in milliseconds * @throws TimeoutException To notify userName of timeout @@ -499,6 +553,7 @@ private static void upload( final byte[] content, final String remoteFile, final Path localPath, + final Map environment, final long timeout, final long start ) throws TimeoutException, WindowsRemoteException { @@ -510,7 +565,7 @@ private static void upload( Base64.getEncoder().encodeToString(content), base64File )) { - runChecked(windowsRemoteExecutor, uploadCommand, "upload the file content", timeout, start); + runChecked(windowsRemoteExecutor, uploadCommand, environment, "upload the file content", timeout, start); } final WindowsRemoteCommandResult decoded = run( @@ -518,6 +573,7 @@ private static void upload( String.format("certutil -f -decode \"%s\" \"%s\" && DEL /F /Q \"%s\"", base64File, remoteFile, base64File) + " & " + digestProbe(remoteFile), + environment, "decode the transferred file", timeout, start @@ -538,7 +594,7 @@ private static void upload( throw integrityCheckFailure(localPath, remoteFile, windowsRemoteExecutor); } } catch (final TimeoutException | WindowsRemoteException | RuntimeException e) { - bestEffortDelete(windowsRemoteExecutor, timeout, start, base64File, remoteFile); + bestEffortDelete(windowsRemoteExecutor, environment, timeout, start, base64File, remoteFile); throw e; } @@ -584,6 +640,7 @@ static List buildUploadCommands(final String base64, final String base64 * * @param windowsRemoteExecutor Executor connected to the remote host * @param remoteFile The remote file to hash + * @param environment Environment variables of the remote shell (can be null or empty for none) * @param timeout Timeout in milliseconds * @param start Operation start time in milliseconds * @return the digest of the remote file, or an empty Optional if it couldn't be computed @@ -594,12 +651,14 @@ static List buildUploadCommands(final String base64, final String base64 private static Optional remoteDigest( final WindowsRemoteExecutor windowsRemoteExecutor, final String remoteFile, + final Map environment, final long timeout, final long start ) throws TimeoutException, WindowsRemoteException { final WindowsRemoteCommandResult result = run( windowsRemoteExecutor, digestProbe(remoteFile), + environment, "hash the remote file", timeout, start @@ -674,6 +733,7 @@ static Optional parseCertutilDigest(final String output, final String al * * @param windowsRemoteExecutor Executor connected to the remote host * @param command The command to execute + * @param environment Environment variables of the remote shell (can be null or empty for none) * @param description What the command does, for the timeout and failure messages * @param timeout Timeout in milliseconds * @param start Operation start time in milliseconds @@ -683,11 +743,19 @@ static Optional parseCertutilDigest(final String output, final String al private static void runChecked( final WindowsRemoteExecutor windowsRemoteExecutor, final String command, + final Map environment, final String description, final long timeout, final long start ) throws TimeoutException, WindowsRemoteException { - final WindowsRemoteCommandResult result = run(windowsRemoteExecutor, command, description, timeout, start); + final WindowsRemoteCommandResult result = run( + windowsRemoteExecutor, + command, + environment, + description, + timeout, + start + ); if (result.getStatusCode() != 0) { throw new WindowsRemoteException( @@ -705,15 +773,19 @@ private static void runChecked( private static WindowsRemoteCommandResult run( final WindowsRemoteExecutor windowsRemoteExecutor, final String command, + final Map environment, final String description, final long timeout, final long start ) throws TimeoutException, WindowsRemoteException { for (int attempt = 0;; attempt++) { try { + // The environment rides every leg: whichever command happens to create the shell + // pins it (the others are no-ops), and the caller's command then inherits it. return windowsRemoteExecutor.executeCommand( command, null, + environment, null, TimeoutHelper.getRemainingTime(timeout, start, "No time left to " + description) ); @@ -766,12 +838,14 @@ static boolean isRetryableQuotaRejection(final Exception exception) { * where the original exception must not be masked. * * @param windowsRemoteExecutor Executor connected to the remote host + * @param environment Environment variables of the remote shell (can be null or empty for none) * @param timeout Timeout in milliseconds * @param start Operation start time in milliseconds * @param remoteFiles The remote files to delete */ private static void bestEffortDelete( final WindowsRemoteExecutor windowsRemoteExecutor, + final Map environment, final long timeout, final long start, final String... remoteFiles @@ -782,7 +856,7 @@ private static void bestEffortDelete( } try { - run(windowsRemoteExecutor, "DEL /F /Q" + files, "clean up", timeout, start); + run(windowsRemoteExecutor, "DEL /F /Q" + files, environment, "clean up", timeout, start); } catch (final Exception ignored) { // Cleanup is best-effort: the exception that triggered it matters more } diff --git a/src/test/java/org/metricshub/winrm/ShellFileCopyTest.java b/src/test/java/org/metricshub/winrm/ShellFileCopyTest.java index b743f62..59f3a9b 100644 --- a/src/test/java/org/metricshub/winrm/ShellFileCopyTest.java +++ b/src/test/java/org/metricshub/winrm/ShellFileCopyTest.java @@ -338,6 +338,86 @@ void transfersEmptyFile() throws Exception { assertFalse(executor.getExecutedCommands().stream().anyMatch(command -> command.contains(" echo "))); } + @Test + void transferCommandsCarryTheShellEnvironment() throws Exception { + // The transfer commands are what actually creates (and pins the settings of) the remote + // shell: the caller's shell-scoped environment must ride every leg, or the command the + // files were uploaded for would silently run without it. + final byte[] content = "env".getBytes(UTF_8); + final Path localFile = tempDir.resolve("env.bat"); + Files.write(localFile, content); + + // Cheap skip path: the remote copy already carries the identical digest + final ScriptedWindowsRemoteExecutor delegate = executorWithTempDirectory() + .expectCommand("certutil -hashfile", hashOutput("SHA256", sha256Hex(content))); + + final Map environment = Map.of("BUILD_NUMBER", "42"); + final List> received = new java.util.ArrayList<>(); + final WindowsRemoteExecutor recording = new WindowsRemoteExecutor() { + @Override + public List> executeWql(final String wqlQuery, final long timeout) { + return delegate.executeWql(wqlQuery, timeout); + } + + @Override + public WindowsRemoteCommandResult executeCommand( + final String command, + final String workingDirectory, + final java.nio.charset.Charset charset, + final long timeout + ) { + throw new AssertionError("The transfer must use the environment-aware entry point: " + command); + } + + @Override + public WindowsRemoteCommandResult executeCommand( + final String command, + final String workingDirectory, + final Map commandEnvironment, + final java.nio.charset.Charset charset, + final long timeout + ) { + received.add(commandEnvironment); + return delegate.executeCommand(command, workingDirectory, charset, timeout); + } + + @Override + public String getHostname() { + return delegate.getHostname(); + } + + @Override + public String getUsername() { + return delegate.getUsername(); + } + + @Override + public char[] getPassword() { + return delegate.getPassword(); + } + + @Override + public void close() { + delegate.close(); + } + }; + + final String updatedCommand = ShellFileCopy.copyLocalFilesToRemote( + recording, + localFile.toString(), + List.of(localFile.toString()), + environment, + TIMEOUT + ); + + assertEquals( + expectedRemoteDirectory() + "\\" + ShellFileCopy.contentAddressedName("env.bat", content), + updatedCommand + ); + // Two legs (cleanup + MKDIR, then the digest probe), each carrying the caller's environment. + assertEquals(List.of(environment, environment), received); + } + @Test void returnsCommandUnchangedWithoutFiles() throws Exception { // No handler registered: any remote interaction would fail the test @@ -356,7 +436,7 @@ void rejectsPathWithoutFileName() { // A root path has no file name component (Path.getFileName() is null) assertThrows( IllegalArgumentException.class, - () -> ShellFileCopy.copyFile(executor, Path.of("C:\\"), "C:\\Windows\\Temp", TIMEOUT, 0L) + () -> ShellFileCopy.copyFile(executor, Path.of("C:\\"), "C:\\Windows\\Temp", null, TIMEOUT, 0L) ); assertTrue(executor.getExecutedCommands().isEmpty()); } diff --git a/src/test/java/org/metricshub/winrm/WinRMClientTest.java b/src/test/java/org/metricshub/winrm/WinRMClientTest.java index dfa2617..b65038d 100644 --- a/src/test/java/org/metricshub/winrm/WinRMClientTest.java +++ b/src/test/java/org/metricshub/winrm/WinRMClientTest.java @@ -43,6 +43,7 @@ import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; import org.metricshub.winrm.exceptions.WinRMAuthenticationException; import org.metricshub.winrm.exceptions.WinRMFaultException; import org.metricshub.winrm.exceptions.WinRMTimeoutException; @@ -280,6 +281,70 @@ void environmentVariablesAreSentInTheCreateShellRequest() throws Exception { assertTrue(environment < workingDirectory && workingDirectory < inputStreams, create); } + @Test + void uploadsCarryTheEnvironmentIntoTheShellTheyCreate(@TempDir final java.nio.file.Path tempDir) throws Exception { + // .environment(...) combined with .upload(...): the transfer commands run FIRST and are + // what actually creates the shell, so they must carry the environment — the real command + // then inherits it. Without that, the variables would be silently dropped. + final byte[] content = "collect".getBytes(StandardCharsets.UTF_8); + final java.nio.file.Path localFile = tempDir.resolve("collect.bat"); + java.nio.file.Files.write(localFile, content); + final StringBuilder digest = new StringBuilder(); + for (final byte b : java.security.MessageDigest.getInstance("SHA-256").digest(content)) { + digest.append(String.format("%02x", b)); + } + final String certutil = "SHA256 hash of file x:\r\n" + + digest + + "\r\nCertUtil: -hashfile command completed successfully.\r\n"; + + server + // ShellFileCopy locates the Windows directory with a WQL query (no shell involved)... + .enqueue(200, envelope(enumerationDone(instance("Win32_OperatingSystem", "WindowsDirectory", "C:\\Windows")))) + // ...then its first command leg (cleanup + MKDIR) creates the shell... + .enqueue(200, envelope(resourceCreated("SHELL-1"))) + .enqueue(200, envelope(commandResponse("CMD-1"))) + .enqueue(200, envelope(receiveResponse("", done("CMD-1", 0)))) + .enqueue(200, envelope(signalResponse())) + // ...the digest probe reports an identical remote copy (transfer skipped)... + .enqueue(200, envelope(commandResponse("CMD-2"))) + .enqueue( + 200, + envelope( + receiveResponse(stream("stdout", "CMD-2", certutil.getBytes(StandardCharsets.UTF_8)), done("CMD-2", 0)) + ) + ) + .enqueue(200, envelope(signalResponse())) + // ...and the real command runs in the SAME shell. + .enqueue(200, envelope(commandResponse("CMD-3"))) + .enqueue( + 200, + envelope(receiveResponse(stream("stdout", "CMD-3", "done".getBytes(StandardCharsets.UTF_8)), done("CMD-3", 0))) + ) + .enqueue(200, envelope(signalResponse())); + + try (WinRMClient client = builder(PASSWORD).build()) { + final CommandResult result = client + .command(localFile.toString()) + .upload(localFile) + .environment("BUILD_NUMBER", "42") + .execute(); + + assertEquals("done", result.stdout()); + } + + final List requests = server.decryptedRequests(); + final List creates = requests + .stream() + .filter(r -> r.contains("")) + .collect(java.util.stream.Collectors.toList()); + assertEquals(1, creates.size(), () -> String.join("\n---\n", requests)); + assertTrue( + creates.get(0) + .contains("42"), + creates.get(0) + ); + } + @Test void commandDecodesOutputAsUtf8WithoutProbingTheRemoteCodeSet() throws Exception { server