This is an automated email from the ASF dual-hosted git repository.
hansva pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/hop.git
The following commit(s) were added to refs/heads/main by this push:
new a48c6febc7 feat(action): support hidden arguments in Shell action to
prevent credential leaks in execution logs (#4935) (#8094)
a48c6febc7 is described below
commit a48c6febc7dedc97dc08cc697a350cd11efd02c7
Author: zanarelli <[email protected]>
AuthorDate: Thu Aug 27 06:29:52 2026 -0700
feat(action): support hidden arguments in Shell action to prevent
credential leaks in execution logs (#4935) (#8094)
* feat(action): support hidden arguments to prevent credential leaks in
Shell action logs (#4935)
* refactor(action): remove loadXml in favor of ILegacyXml and add backward
compatibility tests
* test(action): add hidden-argument integration test and Arguments tab
(#4935)
Run a Shell script with a hidden argument and use Simple Eval to assert
the command log is masked and the secret is not written.
Move argument widgets to a dedicated tab, keep them available when Insert
script is enabled, rename execute-per-row to result row, and document the
Hidden column.
---------
Co-authored-by: mattcasters <[email protected]>
---
.../modules/ROOT/pages/workflow/actions/shell.adoc | 13 +-
integration-tests/actions/0023-log-variables.sh | 23 ++
.../actions/main-0023-shell-hidden-arguments.hwf | 282 +++++++++++++++++++++
.../hop/workflow/actions/shell/ActionShell.java | 170 +++++++++----
.../workflow/actions/shell/ActionShellDialog.java | 113 +++++----
.../shell/messages/messages_en_US.properties | 7 +-
.../shell/WorkflowActionShellLoadSaveTest.java | 132 ++++++++++
7 files changed, 645 insertions(+), 95 deletions(-)
diff --git
a/docs/hop-user-manual/modules/ROOT/pages/workflow/actions/shell.adoc
b/docs/hop-user-manual/modules/ROOT/pages/workflow/actions/shell.adoc
index 481304e63f..c301c2fc76 100644
--- a/docs/hop-user-manual/modules/ROOT/pages/workflow/actions/shell.adoc
+++ b/docs/hop-user-manual/modules/ROOT/pages/workflow/actions/shell.adoc
@@ -74,12 +74,21 @@ When the field is left empty or the working directory is
invalid {openvar}user.d
|Include time in filename?|Adds the system time to the filename. (_235959)
|Loglevel|Specifies the logging level for the execution of the shell.
See also the logging window in .14 Logging
+|===
+
+=== Arguments tab
+
+[options="header", width="90%", cols="1,3"]
+|===
+|Option|Description
|Copy previous results to arguments?|The results from a previous pipeline can
be sent to the shell script using the "Copy rows to result" transform. (as
arguments)
-|Execute once for every input row|This implements looping.
+|Execute for every result row|This implements looping.
If the previous workflow action returns a set of result rows, you can have
this shell script executed once for every row found.
One row is passed to this script at every execution in combination with the
copy previous result to arguments.
The values of the corresponding result row can then be found on command line
argument $1, $2, ... (%1, %2, %3, ... on Windows)
-|Arguments table|Specify the strings to use as arguments for the shell script.
+|Arguments table a|Specify the strings to use as arguments for the shell
script.
+
+The **Hidden (Y/N)** column controls logging only: when set to `Y`, the
argument is replaced by `***` in the Shell action log. The script still
receives the real value.
|===
=== Script tab
diff --git a/integration-tests/actions/0023-log-variables.sh
b/integration-tests/actions/0023-log-variables.sh
new file mode 100755
index 0000000000..07260cf9f2
--- /dev/null
+++ b/integration-tests/actions/0023-log-variables.sh
@@ -0,0 +1,23 @@
+#!/bin/bash
+
+#
+# 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.
+#
+#
+
+echo "VISIBLE_USER=$1"
+echo "VISIBLE_FLAG=$3"
+echo "PROJECT_HOME=${PROJECT_HOME}"
diff --git a/integration-tests/actions/main-0023-shell-hidden-arguments.hwf
b/integration-tests/actions/main-0023-shell-hidden-arguments.hwf
new file mode 100644
index 0000000000..dedf27b735
--- /dev/null
+++ b/integration-tests/actions/main-0023-shell-hidden-arguments.hwf
@@ -0,0 +1,282 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+
+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.
+
+-->
+<workflow>
+ <name>main-0023-shell-hidden-arguments</name>
+ <name_sync_with_filename>Y</name_sync_with_filename>
+ <description>Issue #4935: hidden Shell arguments are masked as *** in the
action log and must not appear as plaintext.</description>
+ <extended_description/>
+ <workflow_version/>
+ <created_user>-</created_user>
+ <created_date>2026/08/27 12:00:00.000</created_date>
+ <modified_user>-</modified_user>
+ <modified_date>2026/08/27 12:00:00.000</modified_date>
+ <parameters>
+ </parameters>
+ <actions>
+ <action>
+ <name>Start</name>
+ <description/>
+ <type>SPECIAL</type>
+ <attributes/>
+ <DayOfMonth>1</DayOfMonth>
+ <doNotWaitOnFirstExecution>N</doNotWaitOnFirstExecution>
+ <hour>12</hour>
+ <intervalMinutes>60</intervalMinutes>
+ <intervalSeconds>0</intervalSeconds>
+ <minutes>0</minutes>
+ <repeat>N</repeat>
+ <schedulerType>0</schedulerType>
+ <weekDay>1</weekDay>
+ <parallel>N</parallel>
+ <xloc>64</xloc>
+ <yloc>96</yloc>
+ <attributes_hac/>
+ </action>
+ <action>
+ <name>Log variables with hidden password</name>
+ <description>Run a script that logs visible arguments. The password
argument is marked hidden so Hop must mask it in the command log.</description>
+ <type>SHELL</type>
+ <attributes/>
+ <filename>${PROJECT_HOME}/0023-log-variables.sh</filename>
+ <work_directory>${PROJECT_HOME}</work_directory>
+ <arg_from_previous>N</arg_from_previous>
+ <exec_per_row>N</exec_per_row>
+ <set_logfile>N</set_logfile>
+ <logfile/>
+ <set_append_logfile>N</set_append_logfile>
+ <logext/>
+ <add_date>N</add_date>
+ <add_time>N</add_time>
+ <insertScript>N</insertScript>
+ <script/>
+ <arguments>
+ <argument>
+ <value>VisibleUser4935</value>
+ <hidden>N</hidden>
+ </argument>
+ <argument>
+ <value>HiddenSecret4935</value>
+ <hidden>Y</hidden>
+ </argument>
+ <argument>
+ <value>--from-it-4935</value>
+ <hidden>N</hidden>
+ </argument>
+ </arguments>
+ <loglevel>Basic</loglevel>
+ <parallel>N</parallel>
+ <xloc>288</xloc>
+ <yloc>96</yloc>
+ <attributes_hac/>
+ </action>
+ <action>
+ <name>Command log masks hidden argument</name>
+ <description>The Executing command line must show the visible args with
the hidden argument replaced by ***.</description>
+ <type>SIMPLE_EVAL</type>
+ <attributes/>
+ <valuetype>logtext</valuetype>
+ <fieldtype>string</fieldtype>
+ <fieldname/>
+ <variablename/>
+ <mask/>
+ <comparevalue>VisibleUser4935 *** --from-it-4935</comparevalue>
+ <minvalue/>
+ <maxvalue/>
+ <successcondition>contains</successcondition>
+ <successwhenvarset>N</successwhenvarset>
+ <successbooleancondition>true</successbooleancondition>
+ <successnumbercondition>equal</successnumbercondition>
+ <xloc>544</xloc>
+ <yloc>96</yloc>
+ <parallel>N</parallel>
+ <attributes_hac/>
+ </action>
+ <action>
+ <name>Secret is not in the log</name>
+ <description>The hidden argument value must not appear anywhere in the
accumulated action log.</description>
+ <type>SIMPLE_EVAL</type>
+ <attributes/>
+ <valuetype>logtext</valuetype>
+ <fieldtype>string</fieldtype>
+ <fieldname/>
+ <variablename/>
+ <mask/>
+ <comparevalue>HiddenSecret4935</comparevalue>
+ <minvalue/>
+ <maxvalue/>
+ <successcondition>notcontains</successcondition>
+ <successwhenvarset>N</successwhenvarset>
+ <successbooleancondition>true</successbooleancondition>
+ <successnumbercondition>equal</successnumbercondition>
+ <xloc>800</xloc>
+ <yloc>96</yloc>
+ <parallel>N</parallel>
+ <attributes_hac/>
+ </action>
+ <action>
+ <name>Script logged visible variables</name>
+ <description>The script stdout (which Hop logs) must still contain the
non-hidden variables.</description>
+ <type>SIMPLE_EVAL</type>
+ <attributes/>
+ <valuetype>logtext</valuetype>
+ <fieldtype>string</fieldtype>
+ <fieldname/>
+ <variablename/>
+ <mask/>
+ <comparevalue>VISIBLE_USER=VisibleUser4935</comparevalue>
+ <minvalue/>
+ <maxvalue/>
+ <successcondition>contains</successcondition>
+ <successwhenvarset>N</successwhenvarset>
+ <successbooleancondition>true</successbooleancondition>
+ <successnumbercondition>equal</successnumbercondition>
+ <xloc>1056</xloc>
+ <yloc>96</yloc>
+ <parallel>N</parallel>
+ <attributes_hac/>
+ </action>
+ <action>
+ <name>Success</name>
+ <description/>
+ <type>SUCCESS</type>
+ <attributes/>
+ <parallel>N</parallel>
+ <xloc>1312</xloc>
+ <yloc>96</yloc>
+ <attributes_hac/>
+ </action>
+ <action>
+ <name>Abort shell failed</name>
+ <description/>
+ <type>ABORT</type>
+ <attributes/>
+ <always_log_rows>N</always_log_rows>
+ <message>Shell action failed before the hidden-argument log checks could
run</message>
+ <parallel>N</parallel>
+ <xloc>288</xloc>
+ <yloc>240</yloc>
+ <attributes_hac/>
+ </action>
+ <action>
+ <name>Abort command not masked</name>
+ <description/>
+ <type>ABORT</type>
+ <attributes/>
+ <always_log_rows>N</always_log_rows>
+ <message>Shell command log did not contain the masked argument sequence
VisibleUser4935 *** --from-it-4935</message>
+ <parallel>N</parallel>
+ <xloc>544</xloc>
+ <yloc>240</yloc>
+ <attributes_hac/>
+ </action>
+ <action>
+ <name>Abort secret leaked</name>
+ <description/>
+ <type>ABORT</type>
+ <attributes/>
+ <always_log_rows>N</always_log_rows>
+ <message>Hidden Shell argument HiddenSecret4935 appeared in the action
log</message>
+ <parallel>N</parallel>
+ <xloc>800</xloc>
+ <yloc>240</yloc>
+ <attributes_hac/>
+ </action>
+ <action>
+ <name>Abort visible variables missing</name>
+ <description/>
+ <type>ABORT</type>
+ <attributes/>
+ <always_log_rows>N</always_log_rows>
+ <message>Shell script did not log VISIBLE_USER=VisibleUser4935</message>
+ <parallel>N</parallel>
+ <xloc>1056</xloc>
+ <yloc>240</yloc>
+ <attributes_hac/>
+ </action>
+ </actions>
+ <hops>
+ <hop>
+ <from>Start</from>
+ <to>Log variables with hidden password</to>
+ <enabled>Y</enabled>
+ <evaluation>Y</evaluation>
+ <unconditional>Y</unconditional>
+ </hop>
+ <hop>
+ <from>Log variables with hidden password</from>
+ <to>Command log masks hidden argument</to>
+ <enabled>Y</enabled>
+ <evaluation>Y</evaluation>
+ <unconditional>N</unconditional>
+ </hop>
+ <hop>
+ <from>Log variables with hidden password</from>
+ <to>Abort shell failed</to>
+ <enabled>Y</enabled>
+ <evaluation>N</evaluation>
+ <unconditional>N</unconditional>
+ </hop>
+ <hop>
+ <from>Command log masks hidden argument</from>
+ <to>Secret is not in the log</to>
+ <enabled>Y</enabled>
+ <evaluation>Y</evaluation>
+ <unconditional>N</unconditional>
+ </hop>
+ <hop>
+ <from>Command log masks hidden argument</from>
+ <to>Abort command not masked</to>
+ <enabled>Y</enabled>
+ <evaluation>N</evaluation>
+ <unconditional>N</unconditional>
+ </hop>
+ <hop>
+ <from>Secret is not in the log</from>
+ <to>Script logged visible variables</to>
+ <enabled>Y</enabled>
+ <evaluation>Y</evaluation>
+ <unconditional>N</unconditional>
+ </hop>
+ <hop>
+ <from>Secret is not in the log</from>
+ <to>Abort secret leaked</to>
+ <enabled>Y</enabled>
+ <evaluation>N</evaluation>
+ <unconditional>N</unconditional>
+ </hop>
+ <hop>
+ <from>Script logged visible variables</from>
+ <to>Success</to>
+ <enabled>Y</enabled>
+ <evaluation>Y</evaluation>
+ <unconditional>N</unconditional>
+ </hop>
+ <hop>
+ <from>Script logged visible variables</from>
+ <to>Abort visible variables missing</to>
+ <enabled>Y</enabled>
+ <evaluation>N</evaluation>
+ <unconditional>N</unconditional>
+ </hop>
+ </hops>
+ <notepads>
+ </notepads>
+ <attributes/>
+</workflow>
diff --git
a/plugins/actions/shell/src/main/java/org/apache/hop/workflow/actions/shell/ActionShell.java
b/plugins/actions/shell/src/main/java/org/apache/hop/workflow/actions/shell/ActionShell.java
index 14725046a3..99a82f0aa7 100644
---
a/plugins/actions/shell/src/main/java/org/apache/hop/workflow/actions/shell/ActionShell.java
+++
b/plugins/actions/shell/src/main/java/org/apache/hop/workflow/actions/shell/ActionShell.java
@@ -24,7 +24,6 @@ import java.io.OutputStream;
import java.text.SimpleDateFormat;
import java.util.ArrayList;
import java.util.Calendar;
-import java.util.Iterator;
import java.util.List;
import java.util.Map;
import lombok.Getter;
@@ -38,7 +37,6 @@ import org.apache.hop.core.ResultFile;
import org.apache.hop.core.RowMetaAndData;
import org.apache.hop.core.annotations.Action;
import org.apache.hop.core.exception.HopException;
-import org.apache.hop.core.exception.HopXmlException;
import org.apache.hop.core.file.IHasFilename;
import org.apache.hop.core.logging.FileLoggingEventListener;
import org.apache.hop.core.logging.HopLogStore;
@@ -51,6 +49,7 @@ import org.apache.hop.core.xml.XmlHandler;
import org.apache.hop.i18n.BaseMessages;
import org.apache.hop.metadata.api.HopMetadataProperty;
import org.apache.hop.metadata.api.IHopMetadataProvider;
+import org.apache.hop.metadata.serializer.xml.ILegacyXml;
import org.apache.hop.resource.ResourceEntry;
import org.apache.hop.resource.ResourceEntry.ResourceType;
import org.apache.hop.resource.ResourceReference;
@@ -74,7 +73,50 @@ import org.w3c.dom.Node;
@SuppressWarnings("java:S1104")
@Getter
@Setter
-public class ActionShell extends ActionBase {
+public class ActionShell extends ActionBase implements ILegacyXml {
+ public static class ShellArgument implements ILegacyXml {
+ @HopMetadataProperty(key = "value")
+ private String value;
+
+ @HopMetadataProperty(key = "hidden")
+ private boolean hidden;
+
+ public ShellArgument() {
+ this("", false);
+ }
+
+ public ShellArgument(String value, boolean hidden) {
+ this.value = value;
+ this.hidden = hidden;
+ }
+
+ public String getValue() {
+ return value;
+ }
+
+ public void setValue(String value) {
+ this.value = value;
+ }
+
+ public boolean isHidden() {
+ return hidden;
+ }
+
+ public void setHidden(boolean hidden) {
+ this.hidden = hidden;
+ }
+
+ @Override
+ public void convertLegacyXml(Node node) {
+ if (XmlHandler.getSubNode(node, "value") == null) {
+ String text = XmlHandler.getNodeValue(node);
+ if (text != null) {
+ this.value = text;
+ }
+ }
+ }
+ }
+
private static final Class<?> PKG = ActionShell.class;
public static final String CONST_JAVA_IO_TMPDIR = "java.io.tmpdir";
public static final String CONST_WINDOWS_95 = "Windows 95";
@@ -87,7 +129,7 @@ public class ActionShell extends ActionBase {
private String workDirectory;
@HopMetadataProperty(groupKey = "arguments", key = "argument")
- public List<String> arguments;
+ public List<ShellArgument> arguments;
@HopMetadataProperty(key = "arg_from_previous")
public boolean argFromPrevious;
@@ -131,34 +173,19 @@ public class ActionShell extends ActionBase {
clear();
}
- /**
- * @deprecated keep for backwards compatibility
- * @param entrynode the top-level XML node
- * @param metadataProvider The metadataProvider to optionally load from.
- * @param variables
- * @throws HopXmlException
- */
@Override
- @Deprecated(since = "2.13")
- public void loadXml(Node entrynode, IHopMetadataProvider metadataProvider,
IVariables variables)
- throws HopXmlException {
- try {
- super.loadXml(entrynode, metadataProvider, variables);
-
- // How many arguments?
- int argnr = 0;
- while (XmlHandler.getTagValue(entrynode, "argument" + argnr) != null) {
- argnr++;
- }
-
- // Read them all...
- // THIS IS A VERY BAD WAY OF READING/SAVING AS IT MAKES
- // THE XML "DUBIOUS". DON'T REUSE IT.
- for (int a = 0; a < argnr; a++) {
- arguments.add(XmlHandler.getTagValue(entrynode, "argument" + a));
+ public void convertLegacyXml(Node node) throws HopException {
+ if (node == null) {
+ return;
+ }
+ // Backward compatibility with legacy argument0, argument1... tags
+ int argnr = 0;
+ while (XmlHandler.getTagValue(node, "argument" + argnr) != null) {
+ if (arguments == null) {
+ arguments = new ArrayList<>();
}
- } catch (HopException e) {
- throw new HopXmlException("Unable to load action of type 'shell' from
XML node", e);
+ arguments.add(new ShellArgument(XmlHandler.getTagValue(node, "argument"
+ argnr), false));
+ argnr++;
}
}
@@ -243,15 +270,20 @@ public class ActionShell extends ActionBase {
// "Translate" the arguments for later
String[] substArgs = null;
+ boolean[] hiddenArgs = null;
if (arguments != null) {
substArgs = new String[arguments.size()];
+ hiddenArgs = new boolean[arguments.size()];
for (int idx = 0; idx < arguments.size(); idx++) {
- substArgs[idx] = resolve(arguments.get(idx));
+ ShellArgument arg = arguments.get(idx);
+ substArgs[idx] = resolve(arg.getValue());
+ hiddenArgs[idx] = arg.isHidden();
}
}
int iteration = 0;
String[] args = substArgs;
+ boolean[] hArgs = hiddenArgs;
RowMetaAndData resultRow = null;
boolean first = true;
List<RowMetaAndData> rows = result.getRows();
@@ -281,6 +313,7 @@ public class ActionShell extends ActionBase {
if (resultRow != null) {
args = new String[resultRow.size()];
+ hArgs = new boolean[resultRow.size()];
for (int i = 0; i < resultRow.size(); i++) {
args[i] = resultRow.getString(i, null);
}
@@ -295,8 +328,10 @@ public class ActionShell extends ActionBase {
if (argFromPrevious) {
// Only put the first Row on the arguments
args = null;
+ hArgs = null;
if (resultRow != null) {
args = new String[resultRow.size()];
+ hArgs = new boolean[resultRow.size()];
for (int i = 0; i < resultRow.size(); i++) {
args[i] = resultRow.getString(i, null);
}
@@ -309,7 +344,7 @@ public class ActionShell extends ActionBase {
}
}
- executeShell(result, cmdRows, args);
+ executeShell(result, cmdRows, args, hArgs);
iteration++;
}
@@ -330,7 +365,8 @@ public class ActionShell extends ActionBase {
return result;
}
- private void executeShell(Result result, List<RowMetaAndData> cmdRows,
String[] args) {
+ private void executeShell(
+ Result result, List<RowMetaAndData> cmdRows, String[] args, boolean[]
hiddenArgs) {
FileObject fileObject = null;
String realScript = null;
FileObject tempFile = null;
@@ -442,20 +478,9 @@ public class ActionShell extends ActionBase {
}
}
- StringBuilder command = new StringBuilder();
-
- Iterator<String> it = cmds.iterator();
- boolean first = true;
- while (it.hasNext()) {
- if (!first) {
- command.append(' ');
- } else {
- first = false;
- }
- command.append(it.next());
- }
+ String logCommand = buildLogCommand(cmds, fileObject, args, hiddenArgs);
if (isBasic()) {
- logBasic(BaseMessages.getString(PKG, "ActionShell.ExecCommand",
command.toString()));
+ logBasic(BaseMessages.getString(PKG, "ActionShell.ExecCommand",
logCommand));
}
// Build the environment variable list...
@@ -491,7 +516,7 @@ public class ActionShell extends ActionBase {
proc.waitFor();
if (isDetailed()) {
- logDetailed(BaseMessages.getString(PKG, "ActionShell.CommandFinished",
command.toString()));
+ logDetailed(BaseMessages.getString(PKG, "ActionShell.CommandFinished",
logCommand));
}
// What's the exit status?
@@ -683,4 +708,55 @@ public class ActionShell extends ActionBase {
}
return () -> filename;
}
+
+ @VisibleForTesting
+ protected String buildLogCommand(
+ List<String> cmds, FileObject fileObject, String[] args, boolean[]
hiddenArgs) {
+ if (args == null || hiddenArgs == null || !hasHiddenArgs(hiddenArgs)) {
+ return String.join(" ", cmds);
+ }
+ StringBuilder sb = new StringBuilder();
+ boolean isWindows =
+ Const.getSystemOs().equals(CONST_WINDOWS_95)
+ || Const.getSystemOs().startsWith(CONST_WINDOWS);
+ if (isWindows) {
+ if (Const.getSystemOs().equals(CONST_WINDOWS_95)) {
+ sb.append("command.com /C \"");
+ } else {
+ sb.append("cmd.exe /C \"");
+ }
+
sb.append(Const.optionallyQuoteStringByOS(HopVfs.getFilename(fileObject)));
+ for (int i = 0; i < args.length; i++) {
+ sb.append(' ');
+ if (i < hiddenArgs.length && hiddenArgs[i]) {
+ sb.append("***");
+ } else {
+ sb.append(Const.optionallyQuoteStringByOS(args[i]));
+ }
+ }
+ sb.append('"');
+ } else {
+ sb.append(HopVfs.getFilename(fileObject));
+ for (int i = 0; i < args.length; i++) {
+ sb.append(' ');
+ if (i < hiddenArgs.length && hiddenArgs[i]) {
+ sb.append("***");
+ } else {
+ sb.append(args[i]);
+ }
+ }
+ }
+ return sb.toString();
+ }
+
+ private boolean hasHiddenArgs(boolean[] hiddenArgs) {
+ if (hiddenArgs != null) {
+ for (boolean b : hiddenArgs) {
+ if (b) {
+ return true;
+ }
+ }
+ }
+ return false;
+ }
}
diff --git
a/plugins/actions/shell/src/main/java/org/apache/hop/workflow/actions/shell/ActionShellDialog.java
b/plugins/actions/shell/src/main/java/org/apache/hop/workflow/actions/shell/ActionShellDialog.java
index ba16246c4d..85c0b4f403 100644
---
a/plugins/actions/shell/src/main/java/org/apache/hop/workflow/actions/shell/ActionShellDialog.java
+++
b/plugins/actions/shell/src/main/java/org/apache/hop/workflow/actions/shell/ActionShellDialog.java
@@ -424,15 +424,46 @@ public class ActionShellDialog extends ActionDialog {
// / END OF LOGGING GROUP
// ///////////////////////////////////////////////////////////
- wlPrevious = new Label(wGeneralComp, SWT.RIGHT);
+ FormData fdGeneralComp = new FormData();
+ fdGeneralComp.left = new FormAttachment(0, 0);
+ fdGeneralComp.top = new FormAttachment(0, 0);
+ fdGeneralComp.right = new FormAttachment(100, 0);
+ fdGeneralComp.bottom = new FormAttachment(100, 0);
+ wGeneralComp.setLayoutData(fdGeneralComp);
+
+ wGeneralComp.layout();
+ wGeneralTab.setControl(wGeneralComp);
+ PropsUi.setLook(wGeneralComp);
+
+ // ///////////////////////////////////////////////////////////
+ // / END OF GENERAL TAB
+ // ///////////////////////////////////////////////////////////
+
+ // ////////////////////////////////////
+ // START OF ARGUMENTS TAB ///
+ // ///////////////////////////////////
+
+ CTabItem wArgumentsTab = new CTabItem(wTabFolder, SWT.NONE);
+ wArgumentsTab.setFont(GuiResource.getInstance().getFontDefault());
+ wArgumentsTab.setText(BaseMessages.getString(PKG,
"ActionShell.Tab.Arguments.Label"));
+
+ Composite wArgumentsComp = new Composite(wTabFolder, SWT.NONE);
+ PropsUi.setLook(wArgumentsComp);
+
+ FormLayout argumentsLayout = new FormLayout();
+ argumentsLayout.marginWidth = 3;
+ argumentsLayout.marginHeight = 3;
+ wArgumentsComp.setLayout(argumentsLayout);
+
+ wlPrevious = new Label(wArgumentsComp, SWT.RIGHT);
wlPrevious.setText(BaseMessages.getString(PKG,
"ActionShell.Previous.Label"));
PropsUi.setLook(wlPrevious);
FormData fdlPrevious = new FormData();
fdlPrevious.left = new FormAttachment(0, 0);
- fdlPrevious.top = new FormAttachment(wLogging, margin * 3);
+ fdlPrevious.top = new FormAttachment(0, margin);
fdlPrevious.right = new FormAttachment(middle, -margin);
wlPrevious.setLayoutData(fdlPrevious);
- wPrevious = new Button(wGeneralComp, SWT.CHECK);
+ wPrevious = new Button(wArgumentsComp, SWT.CHECK);
PropsUi.setLook(wPrevious);
wPrevious.setSelection(action.argFromPrevious);
wPrevious.setToolTipText(BaseMessages.getString(PKG,
"ActionShell.Previous.Tooltip"));
@@ -452,7 +483,7 @@ public class ActionShellDialog extends ActionDialog {
}
});
- wlEveryRow = new Label(wGeneralComp, SWT.RIGHT);
+ wlEveryRow = new Label(wArgumentsComp, SWT.RIGHT);
wlEveryRow.setText(BaseMessages.getString(PKG,
"ActionShell.ExecForEveryInputRow.Label"));
PropsUi.setLook(wlEveryRow);
FormData fdlEveryRow = new FormData();
@@ -460,7 +491,7 @@ public class ActionShellDialog extends ActionDialog {
fdlEveryRow.top = new FormAttachment(wPrevious, margin);
fdlEveryRow.right = new FormAttachment(middle, -margin);
wlEveryRow.setLayoutData(fdlEveryRow);
- wEveryRow = new Button(wGeneralComp, SWT.CHECK);
+ wEveryRow = new Button(wArgumentsComp, SWT.CHECK);
PropsUi.setLook(wEveryRow);
wEveryRow.setSelection(action.execPerRow);
wEveryRow.setToolTipText(
@@ -479,7 +510,7 @@ public class ActionShellDialog extends ActionDialog {
}
});
- wlFields = new Label(wGeneralComp, SWT.NONE);
+ wlFields = new Label(wArgumentsComp, SWT.NONE);
wlFields.setText(BaseMessages.getString(PKG, "ActionShell.Fields.Label"));
PropsUi.setLook(wlFields);
FormData fdlFields = new FormData();
@@ -487,7 +518,7 @@ public class ActionShellDialog extends ActionDialog {
fdlFields.top = new FormAttachment(wlEveryRow, margin);
wlFields.setLayoutData(fdlFields);
- final int nrFieldsCols = 1;
+ final int nrFieldsCols = 2;
int nrRows =
action.arguments == null ? 1 : (action.arguments.isEmpty() ? 1 :
action.arguments.size());
@@ -499,10 +530,18 @@ public class ActionShellDialog extends ActionDialog {
false);
fieldColumns[0].setUsingVariables(true);
+ fieldColumns[1] =
+ new ColumnInfo(
+ BaseMessages.getString(PKG, "ActionShell.Fields.Hidden.Label"),
+ ColumnInfo.COLUMN_TYPE_CCOMBO,
+ new String[] {"Y", "N"},
+ false);
+ fieldColumns[1].setToolTip(BaseMessages.getString(PKG,
"ActionShell.Fields.Hidden.Tooltip"));
+
wFields =
new TableView(
variables,
- wGeneralComp,
+ wArgumentsComp,
SWT.BORDER | SWT.FULL_SELECTION | SWT.MULTI,
fieldColumns,
nrRows,
@@ -519,19 +558,18 @@ public class ActionShellDialog extends ActionDialog {
wlFields.setEnabled(!action.argFromPrevious);
wFields.setEnabled(!action.argFromPrevious);
- FormData fdGeneralComp = new FormData();
- fdGeneralComp.left = new FormAttachment(0, 0);
- fdGeneralComp.top = new FormAttachment(0, 0);
- fdGeneralComp.right = new FormAttachment(100, 0);
- fdGeneralComp.bottom = new FormAttachment(500, -margin);
- wGeneralComp.setLayoutData(fdGeneralComp);
+ FormData fdArgumentsComp = new FormData();
+ fdArgumentsComp.left = new FormAttachment(0, 0);
+ fdArgumentsComp.top = new FormAttachment(0, 0);
+ fdArgumentsComp.right = new FormAttachment(100, 0);
+ fdArgumentsComp.bottom = new FormAttachment(100, 0);
+ wArgumentsComp.setLayoutData(fdArgumentsComp);
- wGeneralComp.layout();
- wGeneralTab.setControl(wGeneralComp);
- PropsUi.setLook(wGeneralComp);
+ wArgumentsComp.layout();
+ wArgumentsTab.setControl(wArgumentsComp);
// ///////////////////////////////////////////////////////////
- // / END OF GENERAL TAB
+ // / END OF ARGUMENTS TAB
// ///////////////////////////////////////////////////////////
// ////////////////////////////////////
@@ -596,29 +634,11 @@ public class ActionShellDialog extends ActionDialog {
}
private void activeInsertScript() {
- wFilename.setEnabled(!wInsertScript.getSelection());
- wlFilename.setEnabled(!wInsertScript.getSelection());
- wbFilename.setEnabled(!wInsertScript.getSelection());
- wScript.setEnabled(wInsertScript.getSelection());
- // We can not use arguments !!!
- if (wInsertScript.getSelection()) {
- wFields.clearAll(false);
- wFields.setEnabled(false);
- wlFields.setEnabled(false);
- wPrevious.setSelection(false);
- wPrevious.setEnabled(false);
- wlPrevious.setEnabled(false);
- wEveryRow.setSelection(false);
- wEveryRow.setEnabled(false);
- wlEveryRow.setEnabled(false);
- } else {
- wFields.setEnabled(true);
- wlFields.setEnabled(true);
- wPrevious.setEnabled(true);
- wlPrevious.setEnabled(true);
- wEveryRow.setEnabled(true);
- wlEveryRow.setEnabled(true);
- }
+ boolean insert = wInsertScript.getSelection();
+ wFilename.setEnabled(!insert);
+ wlFilename.setEnabled(!insert);
+ wbFilename.setEnabled(!insert);
+ wScript.setEnabled(insert);
}
public void enableFields() {
@@ -655,8 +675,12 @@ public class ActionShellDialog extends ActionDialog {
if (action.arguments != null) {
for (int i = 0; i < action.arguments.size(); i++) {
TableItem ti = wFields.table.getItem(i);
- if (action.arguments.get(i) != null) {
- ti.setText(1, action.arguments.get(i));
+ ActionShell.ShellArgument arg = action.arguments.get(i);
+ if (arg != null) {
+ if (arg.getValue() != null) {
+ ti.setText(1, arg.getValue());
+ }
+ ti.setText(2, arg.isHidden() ? "Y" : "N");
}
}
wFields.setRowNums();
@@ -708,8 +732,9 @@ public class ActionShellDialog extends ActionDialog {
for (int i = 0; i < wFields.nrNonEmpty(); i++) {
String arg = wFields.getNonEmpty(i).getText(1);
+ boolean hidden = "Y".equalsIgnoreCase(wFields.getNonEmpty(i).getText(2));
if (!Utils.isEmpty(arg)) {
- action.arguments.add(arg);
+ action.arguments.add(new ActionShell.ShellArgument(arg, hidden));
}
}
diff --git
a/plugins/actions/shell/src/main/resources/org/apache/hop/workflow/actions/shell/messages/messages_en_US.properties
b/plugins/actions/shell/src/main/resources/org/apache/hop/workflow/actions/shell/messages/messages_en_US.properties
index b8704ab1fe..3f904ff854 100644
---
a/plugins/actions/shell/src/main/resources/org/apache/hop/workflow/actions/shell/messages/messages_en_US.properties
+++
b/plugins/actions/shell/src/main/resources/org/apache/hop/workflow/actions/shell/messages/messages_en_US.properties
@@ -25,10 +25,12 @@ ActionShell.Error.UnableopenAppenderFile=Unable to open
file appender for file [
ActionShell.ErrorRunningShell=Error running shell [{0}] \: {1}
ActionShell.Exception.LogFilenameMissing=Log filename is missing\!
ActionShell.ExecCommand=Executing command \: {0}
-ActionShell.ExecForEveryInputRow.Label=Execute for every input row
-ActionShell.ExecForEveryInputRow.Tooltip=Check this to execute this pipeline
mulitple times\: once for every input row.
+ActionShell.ExecForEveryInputRow.Label=Execute for every result row
+ActionShell.ExecForEveryInputRow.Tooltip=Check this to execute this action
once for every result row from the previous action.
ActionShell.ExitStatus=Exit status of shell [{0}] was {1}
ActionShell.Fields.Argument.Label=Argument
+ActionShell.Fields.Hidden.Label=Hidden (Y/N)
+ActionShell.Fields.Hidden.Tooltip=When Y, this argument is masked as *** in
the Shell action log. The script still receives the real value.
ActionShell.Fields.Label=Fields\:
ActionShell.Fileformat.All=All files
ActionShell.Fileformat.Scripts=Shell scripts
@@ -52,6 +54,7 @@ ActionShell.Previous.Tooltip=Check this to pass the results
of the previous entr
ActionShell.RunningOn=Running on platform \: {0}
ActionShell.Shellinterupted=Shell [{0}] was interupted \: {1}
ActionShell.Specify.Logfile.Label=Specify logfile
+ActionShell.Tab.Arguments.Label=Arguments
ActionShell.Tab.General.Label=General
ActionShell.Tab.Script.Label=Script
ActionShell.Title=Shell
diff --git
a/plugins/actions/shell/src/test/java/org/apache/hop/workflow/actions/shell/WorkflowActionShellLoadSaveTest.java
b/plugins/actions/shell/src/test/java/org/apache/hop/workflow/actions/shell/WorkflowActionShellLoadSaveTest.java
index f91171b14d..935df7e222 100644
---
a/plugins/actions/shell/src/test/java/org/apache/hop/workflow/actions/shell/WorkflowActionShellLoadSaveTest.java
+++
b/plugins/actions/shell/src/test/java/org/apache/hop/workflow/actions/shell/WorkflowActionShellLoadSaveTest.java
@@ -18,10 +18,22 @@
package org.apache.hop.workflow.actions.shell;
import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import java.util.List;
+import org.apache.commons.vfs2.FileObject;
import org.apache.hop.core.Const;
+import org.apache.hop.core.vfs.HopVfs;
+import org.apache.hop.core.xml.XmlHandler;
+import org.apache.hop.metadata.serializer.memory.MemoryMetadataProvider;
+import org.apache.hop.metadata.serializer.xml.XmlMetadataUtil;
+import org.apache.hop.workflow.action.ActionMeta;
import org.apache.hop.workflow.action.ActionSerializationTestUtil;
import org.junit.jupiter.api.Test;
+import org.w3c.dom.Document;
+import org.w3c.dom.Node;
class WorkflowActionShellLoadSaveTest {
@Test
@@ -32,6 +44,8 @@ class WorkflowActionShellLoadSaveTest {
assertEquals("${PROJECT_HOME}/0002-shell-test.sh", meta.getFilename());
assertEquals(Const.VAR_PROJECT_HOME, meta.getWorkDirectory());
assertEquals(1, meta.getArguments().size());
+ assertEquals("argument", meta.getArguments().get(0).getValue());
+ assertFalse(meta.getArguments().get(0).isHidden());
}
@Test
@@ -43,5 +57,123 @@ class WorkflowActionShellLoadSaveTest {
assertEquals(clone.getFilename(), meta.getFilename());
assertEquals(clone.getWorkDirectory(), meta.getWorkDirectory());
assertEquals(clone.getArguments().size(), meta.getArguments().size());
+ assertEquals(clone.getArguments().get(0).getValue(),
meta.getArguments().get(0).getValue());
+ assertEquals(clone.getArguments().get(0).isHidden(),
meta.getArguments().get(0).isHidden());
+ }
+
+ @Test
+ void testHiddenArguments() {
+ ActionShell meta = new ActionShell();
+ meta.setArguments(
+ List.of(
+ new ActionShell.ShellArgument("regularArg", false),
+ new ActionShell.ShellArgument("secretPassword", true)));
+
+ assertEquals(2, meta.getArguments().size());
+ assertEquals("regularArg", meta.getArguments().get(0).getValue());
+ assertFalse(meta.getArguments().get(0).isHidden());
+ assertEquals("secretPassword", meta.getArguments().get(1).getValue());
+ assertTrue(meta.getArguments().get(1).isHidden());
+ }
+
+ @Test
+ void testLegacyNumberedArgumentsBackwardCompatibility() throws Exception {
+ String xml =
+ "<action>\n"
+ + " <name>shell-legacy</name>\n"
+ + " <type>SHELL</type>\n"
+ + " <filename>/tmp/test.sh</filename>\n"
+ + " <argument0>firstArg</argument0>\n"
+ + " <argument1>secondArg</argument1>\n"
+ + "</action>";
+
+ Document doc = XmlHandler.loadXmlString(xml);
+ Node node = XmlHandler.getSubNode(doc, ActionMeta.XML_TAG);
+ ActionShell meta =
+ XmlMetadataUtil.deSerializeFromXml(node, ActionShell.class, new
MemoryMetadataProvider());
+
+ assertNotNull(meta.getArguments());
+ assertEquals(2, meta.getArguments().size());
+ assertEquals("firstArg", meta.getArguments().get(0).getValue());
+ assertFalse(meta.getArguments().get(0).isHidden());
+ assertEquals("secondArg", meta.getArguments().get(1).getValue());
+ assertFalse(meta.getArguments().get(1).isHidden());
+ }
+
+ @Test
+ void testLegacyGroupArgumentsBackwardCompatibility() throws Exception {
+ String xml =
+ "<action>\n"
+ + " <name>shell-legacy-group</name>\n"
+ + " <type>SHELL</type>\n"
+ + " <filename>/tmp/test.sh</filename>\n"
+ + " <arguments>\n"
+ + " <argument>param1</argument>\n"
+ + " <argument>param2</argument>\n"
+ + " </arguments>\n"
+ + "</action>";
+
+ Document doc = XmlHandler.loadXmlString(xml);
+ Node node = XmlHandler.getSubNode(doc, ActionMeta.XML_TAG);
+ ActionShell meta =
+ XmlMetadataUtil.deSerializeFromXml(node, ActionShell.class, new
MemoryMetadataProvider());
+
+ assertNotNull(meta.getArguments());
+ assertEquals(2, meta.getArguments().size());
+ assertEquals("param1", meta.getArguments().get(0).getValue());
+ assertFalse(meta.getArguments().get(0).isHidden());
+ assertEquals("param2", meta.getArguments().get(1).getValue());
+ assertFalse(meta.getArguments().get(1).isHidden());
+ }
+
+ @Test
+ void testNewFormatSerializationRoundTrip() throws Exception {
+ String xml =
+ "<action>\n"
+ + " <name>shell-new-format</name>\n"
+ + " <type>SHELL</type>\n"
+ + " <filename>/tmp/test.sh</filename>\n"
+ + " <arguments>\n"
+ + " <argument>\n"
+ + " <value>publicUser</value>\n"
+ + " <hidden>N</hidden>\n"
+ + " </argument>\n"
+ + " <argument>\n"
+ + " <value>secretPassword</value>\n"
+ + " <hidden>Y</hidden>\n"
+ + " </argument>\n"
+ + " </arguments>\n"
+ + "</action>";
+
+ Document doc = XmlHandler.loadXmlString(xml);
+ Node node = XmlHandler.getSubNode(doc, ActionMeta.XML_TAG);
+ MemoryMetadataProvider provider = new MemoryMetadataProvider();
+ ActionShell meta = XmlMetadataUtil.deSerializeFromXml(node,
ActionShell.class, provider);
+
+ assertNotNull(meta.getArguments());
+ assertEquals(2, meta.getArguments().size());
+ assertEquals("publicUser", meta.getArguments().get(0).getValue());
+ assertFalse(meta.getArguments().get(0).isHidden());
+ assertEquals("secretPassword", meta.getArguments().get(1).getValue());
+ assertTrue(meta.getArguments().get(1).isHidden());
+
+ String serializedXml = ActionSerializationTestUtil.getXml(meta);
+ ActionSerializationTestUtil.testXmlStringSerialization(
+ ActionShell.class, ActionMeta.XML_TAG, provider, serializedXml, meta);
+ }
+
+ @Test
+ void testBuildLogCommandMasksHiddenArgs() throws Exception {
+ ActionShell action = new ActionShell();
+ FileObject fileObject = HopVfs.getFileObject("/tmp/script.sh");
+ String[] args = new String[] {"user", "mySecret123", "--flag"};
+ boolean[] hidden = new boolean[] {false, true, false};
+ List<String> cmds = List.of("/tmp/script.sh", "user", "mySecret123",
"--flag");
+
+ String logCommand = action.buildLogCommand(cmds, fileObject, args, hidden);
+ assertFalse(logCommand.contains("mySecret123"));
+ assertTrue(logCommand.contains("***"));
+ assertTrue(logCommand.contains("user"));
+ assertTrue(logCommand.contains("--flag"));
}
}
