Merge pull request opensearch-project#1024 from AndreKurait/RemovePla…

…intextPasswordPrinting Remove arg printing from command line apps with secret values
gregschohn · Sep 27, 2024 · 0ab273e · 0ab273e
2 parents d0d78b0 + 61dd72f
commit 0ab273e
Show file tree

Hide file tree

Showing 4 changed files with 28 additions and 10 deletions.
diff --git a/CreateSnapshot/src/main/java/org/opensearch/migrations/CreateSnapshot.java b/CreateSnapshot/src/main/java/org/opensearch/migrations/CreateSnapshot.java
@@ -73,6 +73,7 @@ public static class S3RepoInfo {
     }
 
     public static void main(String[] args) throws Exception {
+        // TODO: Add back arg printing after not consuming plaintext password MIGRATIONS-1915
         Args arguments = new Args();
         JCommander jCommander = JCommander.newBuilder().addObject(arguments).build();
         jCommander.parse(args);
@@ -98,7 +99,6 @@ public static void main(String[] args) throws Exception {
             throw new ParameterException("If an s3 repo is being used, s3-region must be set");
         }
 
-        log.info("Running CreateSnapshot with {}", String.join(" ", args));
         var snapshotCreator = new CreateSnapshot(arguments, rootContext.createSnapshotCreateContext());
         snapshotCreator.run();
     }

diff --git a/...ntsFromSnapshotMigration/src/main/java/org/opensearch/migrations/RfsMigrateDocuments.java b/...ntsFromSnapshotMigration/src/main/java/org/opensearch/migrations/RfsMigrateDocuments.java
@@ -164,7 +164,7 @@ public static void validateArgs(Args args) {
     }
 
     public static void main(String[] args) throws Exception {
-        log.info("Got args: " + String.join("; ", args));
+        // TODO: Add back arg printing after not consuming plaintext password MIGRATIONS-1915
         var workerId = ProcessHelpers.getNodeInstanceName();
         log.info("Starting RfsMigrateDocuments with workerId =" + workerId);
 

diff --git a/MetadataMigration/src/main/java/org/opensearch/migrations/MetadataMigration.java b/MetadataMigration/src/main/java/org/opensearch/migrations/MetadataMigration.java
@@ -24,7 +24,7 @@ public class MetadataMigration {
     public static void main(String[] args) throws Exception {
         var metadataArgs = new MetadataArgs();
         var migrateArgs = new MigrateArgs();
-        var evaluateArgs = new EvaluateArgs(); 
+        var evaluateArgs = new EvaluateArgs();
         var jCommander = JCommander.newBuilder()
             .addObject(metadataArgs)
             .addCommand(migrateArgs)
@@ -40,7 +40,7 @@ public static void main(String[] args) throws Exception {
 
         var meta = new MetadataMigration();
 
-        log.atInfo().setMessage("Command line arguments: {}\n").addArgument(String.join(" ", args)).log();
+        // TODO: Add back arg printing after not consuming plaintext password MIGRATIONS-1915
 
         if (metadataArgs.help || jCommander.getParsedCommand() == null) {
             printTopLevelHelp(jCommander);

diff --git a/MetadataMigration/src/test/java/org/opensearch/migrations/MetadataMigrationTest.java b/MetadataMigration/src/test/java/org/opensearch/migrations/MetadataMigrationTest.java
@@ -9,9 +9,29 @@
 import static org.hamcrest.CoreMatchers.containsString;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.hasSize;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertThrows;
 
 public class MetadataMigrationTest {
 
+    @Test
+    void testMain_expectNoPasswordLogged() {
+        List<String[]> testCases = List.of(
+            new String[]{"--source-password", "mySecretPassword", "--target-host", "http://localhost"},
+            new String[]{"--target-password", "mySecretPassword", "--target-host", "http://localhost"}
+        );
+        for (var testCase : testCases) {
+            try (var closeableLogSetup = new CloseableLogSetup(MetadataMigration.class.getName())) {
+
+                assertThrows(com.beust.jcommander.ParameterException.class, () -> MetadataMigration.main(testCase));
+
+                var logEvents = closeableLogSetup.getLogEvents();
+
+                assertFalse(logEvents.stream().anyMatch(s -> s.contains("mySecretPassword")));
+            }
+        }
+    }
+
     @Test
     void testMain_expectTopLevelHelp() throws Exception {
         var testCases = List.of(
@@ -25,9 +45,8 @@ void testMain_expectTopLevelHelp() throws Exception {
 
                 var logEvents = closeableLogSetup.getLogEvents();
 
-                assertThat(logEvents, hasSize(2));
-                assertThat(logEvents.get(0), containsString("Command line arguments"));
-                assertThat(logEvents.get(1), containsString("Usage: [options] [command] [commandOptions]"));
+                assertThat(logEvents, hasSize(1));
+                assertThat(logEvents.get(0), containsString("Usage: [options] [command] [commandOptions]"));
             }
         }
     }
@@ -44,9 +63,8 @@ void testMain_expectCommandHelp() throws Exception {
 
                 var logEvents = closeableLogSetup.getLogEvents();
 
-                assertThat(logEvents, hasSize(2));
-                assertThat(logEvents.get(0), containsString("Command line arguments"));
-                assertThat(logEvents.get(1), containsString("Usage: " + testCase[0] + " [options]"));
+                assertThat(logEvents, hasSize(1));
+                assertThat(logEvents.get(0), containsString("Usage: " + testCase[0] + " [options]"));
             }
         }
     }