Support censoring of command arguments

3 files changed, 46 insertions, 14 deletions

diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/process/CommandLine.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/process/CommandLine.java
index 4dbf1f5f973..2153a15e76b 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/process/CommandLine.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/process/CommandLine.java
@@ -11,9 +11,11 @@ import java.time.Duration;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Objects;
 import java.util.Optional;
+import java.util.Set;
 import java.util.TreeMap;
 import java.util.function.Predicate;
 import java.util.logging.Logger;
@@ -49,6 +51,7 @@ public class CommandLine {
     public static final Duration DEFAULT_SIGKILL_GRACE_PERIOD = Duration.ofMinutes(30);
 
     private final List<String> arguments = new ArrayList<>();
+    private final Set<Integer> censoredArgumentIndices = new HashSet<>();
     private final TreeMap<String, String> environment = new TreeMap<>();
     private final TaskContext taskContext;
     private final ProcessFactory processFactory;
@@ -102,6 +105,12 @@ public class CommandLine {
         return this;
     }
 
+    /** Censor (prevent logging of) the last argument added to this */
+    public CommandLine censorArgument() {
+        censoredArgumentIndices.add(arguments.size() - 1);
+        return this;
+    }
+
     /**
      * Execute a shell-like program in a child process:
      *  - the program is recorded and logged as modifying the system, but see executeSilently().
@@ -175,24 +184,35 @@ public class CommandLine {
     /** Returns a shell-like representation of the command. */
     @Override
     public String toString() {
+        return toString(true);
+    }
+
+    String toString(boolean censor) {
         var command = new StringBuilder();
 
         if (!environment.isEmpty()) {
             // Pretend environment is propagated through the env program for display purposes
             command.append(environment.entrySet().stream()
-                    .map(entry -> {
-                        if (entry.getValue() == null) {
-                            return "-u " + maybeEscapeArgument(entry.getKey());
-                        } else {
-                            return maybeEscapeArgument(entry.getKey() + "=" + entry.getValue());
-                        }
-                    })
-                    .collect(Collectors.joining(" ", "env ", " ")));
+                                      .map(entry -> {
+                                          if (entry.getValue() == null) {
+                                              return "-u " + maybeEscapeArgument(entry.getKey());
+                                          } else {
+                                              return maybeEscapeArgument(entry.getKey() + "=" + entry.getValue());
+                                          }
+                                      })
+                                      .collect(Collectors.joining(" ", "env ", " ")));
         }
 
-        command.append(arguments.stream()
-                .map(CommandLine::maybeEscapeArgument)
-                .collect(Collectors.joining(" ")));
+        for (int i = 0; i < arguments.size(); i++) {
+            if (censor && censoredArgumentIndices.contains(i)) {
+                command.append("<censored>");
+            } else {
+                command.append(maybeEscapeArgument(arguments.get(i)));
+            }
+            if (i < arguments.size() - 1) {
+                command.append(" ");
+            }
+        }
 
         // Note: Both of these cannot be confused with an argument since they would
         // require escaping.
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/process/TestProcessFactory.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/process/TestProcessFactory.java
index 063dc7f1324..29f1ffef6f2 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/process/TestProcessFactory.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/process/TestProcessFactory.java
@@ -77,7 +77,7 @@ public class TestProcessFactory implements ProcessFactory {
 
     @Override
     public ChildProcess2 spawn(CommandLine commandLine) {
-        String commandLineString = commandLine.toString();
+        String commandLineString = commandLine.toString(false);
         if (spawnCommandLines.size() + 1 > expectedSpawnCalls.size()) {
             throw new IllegalStateException("Too many invocations: " + commandLineString);
         }
@@ -90,7 +90,7 @@ public class TestProcessFactory implements ProcessFactory {
                                               String expectedCommandLineString,
                                               ChildProcess2 toReturn,
                                               int commandSequenceNumber) {
-        String actualCommandLineString = commandLine.toString();
+        String actualCommandLineString = commandLine.toString(false);
         if (!Objects.equals(actualCommandLineString, expectedCommandLineString)) {
             muteVerifyAllCommandsExecuted = true;
             throw new IllegalArgumentException("Expected command #" + commandSequenceNumber + " to be: \n" +
diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/process/CommandLineTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/process/CommandLineTest.java
index 872a730c342..78079cc9fd5 100644
--- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/process/CommandLineTest.java
+++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/process/CommandLineTest.java
@@ -175,4 +175,16 @@ public class CommandLineTest {
         terminal.verifyAllCommandsExecuted();
     }
 
-}
\ No newline at end of file
+    @Test
+    public void testToString() {
+        commandLine.add("bash", "-c", "echo", "$MY_SECRET");
+        assertEquals("bash -c echo \"$MY_SECRET\" 2>&1", commandLine.toString());
+        commandLine.censorArgument();
+        assertEquals("bash -c echo <censored> 2>&1", commandLine.toString());
+
+        terminal.expectCommand("bash -c echo \"$MY_SECRET\" 2>&1");
+        commandLine.execute();
+        terminal.verifyAllCommandsExecuted();
+    }
+
+}