mattcasters commented on code in PR #8316:
URL: https://github.com/apache/hop/pull/8316#discussion_r3999910933
##########
plugins/actions/pgpfiles/src/main/java/org/apache/hop/workflow/actions/pgpencryptfiles/GPG.java:
##########
@@ -300,23 +343,20 @@ public void encryptFile(
public void encryptFile(String filename, String userID, String
cryptedFilename, boolean asciiMode)
throws HopException {
try {
- execGnuPG(
- CONST_BATCH_YES
- + (asciiMode ? " -a" : "")
- + " -r "
- + "\""
- + Const.NVL(userID, "")
- + "\" "
- + "--output "
- + "\""
- + cryptedFilename
- + "\" "
- + "--encrypt "
- + "\""
- + filename
- + "\"",
- null,
- true);
+ List<String> args = new ArrayList<>(BATCH_YES);
+ if (asciiMode) {
+ args.add("-a");
+ }
+ if (!Utils.isEmpty(userID)) {
Review Comment:
**[suggestion]** `encryptFile` used to always pass `-r` with
`Const.NVL(userID, "")`, so an empty user ID became `gpg -r ""` and failed even
when `gpg.conf` set `default-recipient` / `encrypt-to`. Omitting `-r` when the
user ID is empty is cleaner, but it is a behavior change: the same empty user
ID now encrypts to the configured default recipient (verified with GnuPG
2.4.4). A workflow that used to error can start succeeding and seal files to an
unexpected key.
**Suggestion:** Keep omitting an empty `-r` for sign, where it was already
omitted and `-r` is the wrong flag anyway. For encrypt / sign-and-encrypt,
either fail closed when the user ID is empty, or document that an empty user ID
now honors GnuPG's default recipient. Add a unit assertion for the encrypt
path, not only `signFile`.
##########
plugins/actions/pgpfiles/src/main/java/org/apache/hop/workflow/actions/pgpencryptfiles/GPG.java:
##########
@@ -300,23 +343,20 @@ public void encryptFile(
public void encryptFile(String filename, String userID, String
cryptedFilename, boolean asciiMode)
throws HopException {
try {
- execGnuPG(
- CONST_BATCH_YES
- + (asciiMode ? " -a" : "")
- + " -r "
- + "\""
- + Const.NVL(userID, "")
- + "\" "
- + "--output "
- + "\""
- + cryptedFilename
- + "\" "
- + "--encrypt "
- + "\""
- + filename
- + "\"",
- null,
- true);
+ List<String> args = new ArrayList<>(BATCH_YES);
+ if (asciiMode) {
+ args.add("-a");
+ }
+ if (!Utils.isEmpty(userID)) {
+ args.add("-r");
+ args.add(userID);
+ }
+ args.add("--output");
+ args.add(cryptedFilename);
+ args.add("--encrypt");
+ args.add(filename);
Review Comment:
**[suggestion]** Filenames and key IDs are now separate argv elements, so a
shell can no longer rewrite them, but GnuPG still option-parses any argument
that starts with `-`. A scanned relative name such as `-o`, `--output`, or
`--status-fd` can still be taken as a switch. Hop actions usually pass
`HopVfs.getFilename()` absolute paths (`/drop/-o` is safe), so this is not the
#8311 bug, but the public `String` overloads and any relative destination still
have no `--` terminator.
**Suggestion:** Insert `--` immediately before every positional path
(source, destination, verify target, temp file). That is the usual way to stop
option injection without changing legitimate absolute paths.
##########
plugins/actions/pgpfiles/src/main/java/org/apache/hop/workflow/actions/pgpencryptfiles/GPG.java:
##########
@@ -149,31 +154,36 @@ public String getGpgExeFile() {
}
/**
- * Runs GnuPG external program
+ * Runs GnuPG external program.
+ *
+ * <p>The arguments are handed to the process as a list, never as a single
command line: they are
+ * passed to GnuPG verbatim and are never interpreted by a shell. Filenames
reach this method from
+ * directory scans, so any character a filesystem accepts has to survive
unchanged.
*
- * @param commandArgs command line arguments
- * @param inputStr key ID of the key in GnuPG's key database
- * @param fileMode
+ * @param args command line arguments
+ * @param inputStr data written to the standard input of the process, or null
+ * @param fileMode true when GnuPG reads the data to process from a file
rather than stdin
* @return result
* @throws HopException
Review Comment:
**[suggestion]** The new `@param fileMode` text says it is true when GnuPG
reads the data from a file rather than stdin. That does not match the callers:
`sign`, `decrypt`, and `signAndEncrypt` all pass `fileMode == false` (so
`--batch --armor` is prepended) while the payload sits in a temp file and stdin
carries the passphrase. Only `encrypt(String, keyID)` actually puts the payload
on stdin. A later change that trusted this javadoc could put passphrase and
message on the same fd.
**Suggestion:** Describe `fileMode` as "when false, prepend `--batch
--armor`" (the real branch), or split the two concerns so passphrase-on-stdin
and data-on-stdin cannot be confused.
##########
plugins/actions/pgpfiles/src/main/java/org/apache/hop/workflow/actions/pgpencryptfiles/GPG.java:
##########
@@ -42,7 +43,11 @@ public class GPG {
private ILogChannel log;
- private final String gnuPGCommand = "--batch --armor ";
+ /** Options prepended when GnuPG reads the data to process from stdin
instead of a file. */
+ private static final List<String> GNU_PG_COMMAND = List.of("--batch",
"--armor");
+
+ /** Options prepended to the file based operations. */
+ private static final List<String> BATCH_YES = List.of("--batch", "--yes");
Review Comment:
**[nit]** `BATCH_YES` replaces the old command-string fragment, but the
public `CONST_BATCH_YES = "--batch --yes"` field is now unreferenced. Spotless
will not drop it, and a later edit could start concatenating it back into a
shell string.
**Suggestion:** Remove `CONST_BATCH_YES`.
##########
plugins/actions/pgpfiles/src/test/java/org/apache/hop/workflow/actions/pgpencryptfiles/GpgArgumentPassingTest.java:
##########
@@ -0,0 +1,325 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hop.workflow.actions.pgpencryptfiles;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import java.io.IOException;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.attribute.PosixFilePermissions;
+import java.util.Comparator;
+import java.util.List;
+import org.apache.hop.core.logging.HopLogStore;
+import org.apache.hop.core.logging.ILogChannel;
+import org.apache.hop.core.logging.LogChannel;
+import org.apache.hop.core.variables.IVariables;
+import org.apache.hop.core.variables.Variables;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.condition.EnabledOnOs;
+import org.junit.jupiter.api.condition.OS;
+
+/**
+ * Asserts what {@link GPG} actually hands to the GnuPG process, by standing a
recorder script in
+ * for the binary and reading back the argument vector it was given.
+ *
+ * <p>Observing the arguments rather than whether an operation succeeded is
what makes these tests
+ * meaningful in both directions: they describe a property ("the filename
arrives as one literal
+ * argument", "the passphrase is never on the command line") that can be
checked against any
+ * implementation, and they cover the methods whose real GnuPG operation
cannot easily be made to
+ * succeed in a unit test. Run them against the implementation that built a
shell command string and
+ * they fail; see https://github.com/apache/hop/issues/8311.
+ *
+ * <p>POSIX only: the recorder is a shell script. Argument passing on Windows
is not covered here.
+ */
+@EnabledOnOs({OS.LINUX, OS.MAC})
+class GpgArgumentPassingTest {
+
+ /**
+ * Names that a shell would rewrite. Every one is a legal POSIX filename,
and every one is what an
+ * attacker who can write into a scanned folder would choose.
+ */
+ private static final List<String> HOSTILE_NAMES =
+ List.of(
+ "report$(echo pwned).csv",
+ "report`echo pwned`.csv",
+ "report$HOME.csv",
+ "report;echo pwned;.csv",
+ "report\";echo pwned;\".csv",
+ "report'; echo pwned; '.csv",
+ "report file with spaces.csv",
+ "report&&echo pwned.csv",
+ "report|echo pwned.csv",
+ "report*.csv");
+
+ private static final String PASSPHRASE = "s3cr3t-do-not-leak";
+
+ /** Created by the payloads below if a shell ever evaluates them, and by
nothing else. */
+ private static final String EXPLOIT_MARKER = "hop-pgp-exploit-marker";
+
+ private Path sandbox;
+ private Path recorder;
+ private Path record;
+ private ILogChannel log;
+ private IVariables variables;
+
+ @BeforeAll
+ static void initLogging() {
+ HopLogStore.init();
+ }
+
+ @BeforeEach
+ void createRecorder() throws Exception {
+ sandbox = Files.createTempDirectory("hop-gpg-argv");
+ record = sandbox.resolve("argv.txt");
+ recorder = sandbox.resolve("gpg-recorder.sh");
+
+ // Writes one argument per line and succeeds, so the caller carries on as
if GnuPG had run.
+ Files.writeString(
+ recorder,
+ "#!/bin/sh\n"
+ + "{ for a in \"$@\"; do printf '%s\\n' \"$a\"; done; } > '"
+ + record
+ + "'\n"
+ + "exit 0\n",
+ StandardCharsets.UTF_8);
+ Files.setPosixFilePermissions(recorder,
PosixFilePermissions.fromString("rwx------"));
+
+ log = new LogChannel("GpgArgumentPassingTest");
+ variables = new Variables();
+ }
+
+ @AfterEach
+ void removeSandbox() throws Exception {
+ // If a payload ever did run, do not leave its marker behind for the next
test to trip over.
+
Files.deleteIfExists(Path.of(System.getProperty("user.dir")).resolve(EXPLOIT_MARKER));
+ deleteRecursively(sandbox);
+ }
+
+ @Test
+ void signFilePassesFilenamesLiterally() throws Exception {
+ for (String name : HOSTILE_NAMES) {
+ gpg().signFile(name, "", "signed-" + name, true);
+ assertPassedLiterally(name, "signFile source");
+ assertPassedLiterally("signed-" + name, "signFile destination");
+ }
+ }
+
+ @Test
+ void encryptFilePassesFilenamesLiterally() throws Exception {
+ for (String name : HOSTILE_NAMES) {
+ gpg().encryptFile(name, "[email protected]", "encrypted-" + name, false);
+ assertPassedLiterally(name, "encryptFile source");
+ assertPassedLiterally("encrypted-" + name, "encryptFile destination");
+ }
+ }
+
+ @Test
+ void signAndEncryptFilePassesFilenamesLiterally() throws Exception {
+ for (String name : HOSTILE_NAMES) {
+ gpg().signAndEncryptFile(name, "[email protected]", "sealed-" + name,
true);
+ assertPassedLiterally(name, "signAndEncryptFile source");
+ assertPassedLiterally("sealed-" + name, "signAndEncryptFile
destination");
+ }
+ }
+
+ @Test
+ void decryptFilePassesFilenamesLiterally() throws Exception {
+ for (String name : HOSTILE_NAMES) {
+ gpg().decryptFile(name, "", "opened-" + name);
+ assertPassedLiterally(name, "decryptFile source");
+ assertPassedLiterally("opened-" + name, "decryptFile destination");
+ }
+ }
+
+ @Test
+ void verifySignaturePassesFilenamesLiterally() throws Exception {
+ for (String name : HOSTILE_NAMES) {
+ gpg().verifySignature(name);
+ assertPassedLiterally(name, "verifySignature filename");
+ }
+ }
+
+ @Test
+ void verifyDetachedSignaturePassesFilenamesLiterally() throws Exception {
+ for (String name : HOSTILE_NAMES) {
+ gpg().verifyDetachedSignature(name, "original-" + name);
+ assertPassedLiterally(name, "verifyDetachedSignature signature");
+ assertPassedLiterally("original-" + name, "verifyDetachedSignature
original");
+ }
+ }
+
+ /** Used by the PGP encrypt stream transform in plugins/transforms/pgp. */
+ @Test
+ void encryptStringPassesTheKeyIdLiterally() throws Exception {
+ for (String keyId : HOSTILE_NAMES) {
+ gpg().encrypt("some data", keyId);
+ assertPassedLiterally(keyId, "encrypt key id");
+ }
+ }
+
+ /** Used by the PGP decrypt stream transform in plugins/transforms/pgp. */
+ @Test
+ void decryptStringKeepsThePassphraseOffTheCommandLine() throws Exception {
+ gpg().decrypt("some data", PASSPHRASE);
+ assertPassphraseAbsent("decrypt");
+ }
+
+ @Test
+ void signStringKeepsThePassphraseOffTheCommandLine() throws Exception {
+ gpg().sign("some data", PASSPHRASE);
+ assertPassphraseAbsent("sign");
+ }
+
+ @Test
+ void signAndEncryptStringKeepsThePassphraseOffTheCommandLine() throws
Exception {
+ gpg().signAndEncrypt("some data", "[email protected]", PASSPHRASE);
+ assertPassphraseAbsent("signAndEncrypt");
+ }
+
+ /**
+ * The passphrase must travel over stdin. On the command line it is readable
by every other user
+ * on the machine for as long as GnuPG runs.
+ */
+ @Test
+ void decryptFileKeepsThePassphraseOffTheCommandLine() throws Exception {
+ gpg().decryptFile("sealed.asc", PASSPHRASE, "opened.txt");
Review Comment:
**[suggestion]** `decryptFileKeepsThePassphraseOffTheCommandLine` (and the
string-method cousins) prove the passphrase is absent from argv and that
`--passphrase-fd` is present for `decryptFile`. The recorder never captures
stdin and never asserts `--pinentry-mode loopback`. A regression that passed
`null` as `inputStr` while still adding `--passphrase-fd 0` would hang or fail
only in the integration suite; a regression that dropped loopback would pass
these unit tests and then fail on GnuPG 2.1+ with "Inappropriate ioctl for
device".
**Suggestion:** Have the recorder also dump stdin to a file and assert it
equals the passphrase (plus EOF). Assert `--pinentry-mode` / `loopback` on
every passphrase path, not only `--passphrase-fd` on `decryptFile`.
##########
plugins/actions/pgpfiles/src/test/java/org/apache/hop/workflow/actions/pgpencryptfiles/GpgArgumentPassingTest.java:
##########
@@ -0,0 +1,325 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hop.workflow.actions.pgpencryptfiles;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import java.io.IOException;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.attribute.PosixFilePermissions;
+import java.util.Comparator;
+import java.util.List;
+import org.apache.hop.core.logging.HopLogStore;
+import org.apache.hop.core.logging.ILogChannel;
+import org.apache.hop.core.logging.LogChannel;
+import org.apache.hop.core.variables.IVariables;
+import org.apache.hop.core.variables.Variables;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.condition.EnabledOnOs;
+import org.junit.jupiter.api.condition.OS;
+
+/**
+ * Asserts what {@link GPG} actually hands to the GnuPG process, by standing a
recorder script in
+ * for the binary and reading back the argument vector it was given.
+ *
+ * <p>Observing the arguments rather than whether an operation succeeded is
what makes these tests
+ * meaningful in both directions: they describe a property ("the filename
arrives as one literal
+ * argument", "the passphrase is never on the command line") that can be
checked against any
+ * implementation, and they cover the methods whose real GnuPG operation
cannot easily be made to
+ * succeed in a unit test. Run them against the implementation that built a
shell command string and
+ * they fail; see https://github.com/apache/hop/issues/8311.
+ *
+ * <p>POSIX only: the recorder is a shell script. Argument passing on Windows
is not covered here.
+ */
+@EnabledOnOs({OS.LINUX, OS.MAC})
+class GpgArgumentPassingTest {
+
+ /**
+ * Names that a shell would rewrite. Every one is a legal POSIX filename,
and every one is what an
+ * attacker who can write into a scanned folder would choose.
+ */
+ private static final List<String> HOSTILE_NAMES =
+ List.of(
+ "report$(echo pwned).csv",
+ "report`echo pwned`.csv",
+ "report$HOME.csv",
+ "report;echo pwned;.csv",
+ "report\";echo pwned;\".csv",
+ "report'; echo pwned; '.csv",
+ "report file with spaces.csv",
+ "report&&echo pwned.csv",
+ "report|echo pwned.csv",
+ "report*.csv");
+
+ private static final String PASSPHRASE = "s3cr3t-do-not-leak";
+
+ /** Created by the payloads below if a shell ever evaluates them, and by
nothing else. */
+ private static final String EXPLOIT_MARKER = "hop-pgp-exploit-marker";
+
+ private Path sandbox;
+ private Path recorder;
+ private Path record;
+ private ILogChannel log;
+ private IVariables variables;
+
+ @BeforeAll
+ static void initLogging() {
+ HopLogStore.init();
+ }
+
+ @BeforeEach
+ void createRecorder() throws Exception {
+ sandbox = Files.createTempDirectory("hop-gpg-argv");
+ record = sandbox.resolve("argv.txt");
+ recorder = sandbox.resolve("gpg-recorder.sh");
+
+ // Writes one argument per line and succeeds, so the caller carries on as
if GnuPG had run.
+ Files.writeString(
+ recorder,
+ "#!/bin/sh\n"
+ + "{ for a in \"$@\"; do printf '%s\\n' \"$a\"; done; } > '"
+ + record
+ + "'\n"
+ + "exit 0\n",
+ StandardCharsets.UTF_8);
+ Files.setPosixFilePermissions(recorder,
PosixFilePermissions.fromString("rwx------"));
+
+ log = new LogChannel("GpgArgumentPassingTest");
+ variables = new Variables();
+ }
+
+ @AfterEach
+ void removeSandbox() throws Exception {
+ // If a payload ever did run, do not leave its marker behind for the next
test to trip over.
+
Files.deleteIfExists(Path.of(System.getProperty("user.dir")).resolve(EXPLOIT_MARKER));
+ deleteRecursively(sandbox);
+ }
+
+ @Test
+ void signFilePassesFilenamesLiterally() throws Exception {
+ for (String name : HOSTILE_NAMES) {
+ gpg().signFile(name, "", "signed-" + name, true);
+ assertPassedLiterally(name, "signFile source");
+ assertPassedLiterally("signed-" + name, "signFile destination");
+ }
+ }
+
+ @Test
+ void encryptFilePassesFilenamesLiterally() throws Exception {
+ for (String name : HOSTILE_NAMES) {
+ gpg().encryptFile(name, "[email protected]", "encrypted-" + name, false);
+ assertPassedLiterally(name, "encryptFile source");
+ assertPassedLiterally("encrypted-" + name, "encryptFile destination");
+ }
+ }
+
+ @Test
+ void signAndEncryptFilePassesFilenamesLiterally() throws Exception {
+ for (String name : HOSTILE_NAMES) {
+ gpg().signAndEncryptFile(name, "[email protected]", "sealed-" + name,
true);
+ assertPassedLiterally(name, "signAndEncryptFile source");
+ assertPassedLiterally("sealed-" + name, "signAndEncryptFile
destination");
+ }
+ }
+
+ @Test
+ void decryptFilePassesFilenamesLiterally() throws Exception {
+ for (String name : HOSTILE_NAMES) {
+ gpg().decryptFile(name, "", "opened-" + name);
+ assertPassedLiterally(name, "decryptFile source");
+ assertPassedLiterally("opened-" + name, "decryptFile destination");
+ }
+ }
+
+ @Test
+ void verifySignaturePassesFilenamesLiterally() throws Exception {
+ for (String name : HOSTILE_NAMES) {
+ gpg().verifySignature(name);
+ assertPassedLiterally(name, "verifySignature filename");
+ }
+ }
+
+ @Test
+ void verifyDetachedSignaturePassesFilenamesLiterally() throws Exception {
+ for (String name : HOSTILE_NAMES) {
+ gpg().verifyDetachedSignature(name, "original-" + name);
+ assertPassedLiterally(name, "verifyDetachedSignature signature");
+ assertPassedLiterally("original-" + name, "verifyDetachedSignature
original");
+ }
+ }
+
+ /** Used by the PGP encrypt stream transform in plugins/transforms/pgp. */
+ @Test
+ void encryptStringPassesTheKeyIdLiterally() throws Exception {
+ for (String keyId : HOSTILE_NAMES) {
+ gpg().encrypt("some data", keyId);
+ assertPassedLiterally(keyId, "encrypt key id");
+ }
+ }
+
+ /** Used by the PGP decrypt stream transform in plugins/transforms/pgp. */
+ @Test
+ void decryptStringKeepsThePassphraseOffTheCommandLine() throws Exception {
+ gpg().decrypt("some data", PASSPHRASE);
+ assertPassphraseAbsent("decrypt");
+ }
+
+ @Test
+ void signStringKeepsThePassphraseOffTheCommandLine() throws Exception {
+ gpg().sign("some data", PASSPHRASE);
+ assertPassphraseAbsent("sign");
+ }
+
+ @Test
+ void signAndEncryptStringKeepsThePassphraseOffTheCommandLine() throws
Exception {
+ gpg().signAndEncrypt("some data", "[email protected]", PASSPHRASE);
+ assertPassphraseAbsent("signAndEncrypt");
+ }
+
+ /**
+ * The passphrase must travel over stdin. On the command line it is readable
by every other user
+ * on the machine for as long as GnuPG runs.
+ */
+ @Test
+ void decryptFileKeepsThePassphraseOffTheCommandLine() throws Exception {
+ gpg().decryptFile("sealed.asc", PASSPHRASE, "opened.txt");
+ assertPassphraseAbsent("decryptFile");
+ assertTrue(
+ recordedArguments().contains("--passphrase-fd"),
+ "decryptFile must ask GnuPG to read the passphrase from a file
descriptor");
+ }
+
+ @Test
+ void anEmptyUserIdOmitsTheRecipientFlag() throws Exception {
+ gpg().signFile("plain.txt", "", "plain.txt.asc", true);
+ assertFalse(
+ recordedArguments().contains("-r"),
+ "an empty user id must not be passed to GnuPG as an empty recipient");
+
+ gpg().signFile("plain.txt", "[email protected]", "plain.txt.asc", true);
+ List<String> args = recordedArguments();
+ assertTrue(args.contains("-r"), "a user id must be passed as a recipient");
+ assertEquals(
+ "[email protected]",
+ args.get(args.indexOf("-r") + 1),
+ "the recipient must follow -r as its own argument");
+ }
+
+ /**
Review Comment:
**[suggestion]** Several new comments narrate the old defect and the test
strategy ("The one that matters: proves the defect was executable, not merely
untidy.", the class-level history of the shell command string, the long
`execGnuPG` / `addPassPhraseFromStdin` design write-ups). They restate what the
code and issue #8311 already say rather than a non-obvious constraint.
**Suggestion:** Keep the loopback / process-table WHY in
`addPassPhraseFromStdin`. Drop the history-of-the-bug commentary from tests and
the "safe to log because secrets are on stdin" aside; a one-line pointer to
#8311 is enough.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
