mattcasters commented on code in PR #8790: URL: https://github.com/apache/hop/pull/8790#discussion_r4210069542
########## plugins/actions/shell/src/main/samples/actions/shell.hwf: ########## @@ -0,0 +1,155 @@ +<?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>shell</name> + <name_sync_with_filename>Y</name_sync_with_filename> + <description/> + <extended_description/> + <created_user>-</created_user> + <modified_user>-</modified_user> + <created_date>2026/10/07 10:00:00.000</created_date> + <modified_date>2026/10/07 10:00:00.000</modified_date> + <created_hop_version/> + <modified_hop_version/> + <workflow_version/> + <parameters/> + <actions> + <action> + <repeat>N</repeat> + <schedulerType>0</schedulerType> + <intervalSeconds>0</intervalSeconds> + <intervalMinutes>60</intervalMinutes> + <DayOfMonth>1</DayOfMonth> + <weekDay>1</weekDay> + <minutes>0</minutes> + <hour>12</hour> + <doNotWaitOnFirstExecution>N</doNotWaitOnFirstExecution> + <name>Start</name> + <description/> + <type>SPECIAL</type> + <attributes/> + <xloc>80</xloc> + <yloc>256</yloc> + <parallel>N</parallel> + <attributes_hac/> + </action> + <action> + <filename/> + <work_directory>${PROJECT_HOME}</work_directory> + <arguments/> + <arg_from_previous>N</arg_from_previous> + <set_logfile>N</set_logfile> + <logfile/> + <logext/> + <add_date>N</add_date> + <add_time>N</add_time> + <loglevel>Basic</loglevel> + <exec_per_row>N</exec_per_row> + <set_append_logfile>N</set_append_logfile> + <insertScript>Y</insertScript> + <script>echo Hello from the Shell action</script> Review Comment: **[bug]** Hop does not run an inline script through a shell. On Linux and macOS `ActionShell` writes this text to a temp file, marks it executable, and `ProcessBuilder` execs that file. Without a shebang the kernel returns `ENOEXEC` (`Exec format error`), the action fails, and the note that `echo` works on every operating system is wrong. A `#!/bin/sh` first line is not enough on its own either: Windows runs the same text as a `.bat` file, and `cmd.exe` treats `#!/bin/sh` as a command. The failure hop only writes a log line. `Write to log` succeeds, so the workflow result stays successful and `read-samples-build-hop-run` will not notice that the script never ran. **Suggestion:** Branch with the JavaScript action (`is_windows` is already in scope). On Windows keep `echo Hello from the Shell action`. On Linux and macOS use a script whose first line is `#!/bin/sh`. Point the failure hop at an Abort action so a script that does not run fails the sample. ########## plugins/actions/pgpfiles/src/main/samples/actions/requires-setup/pgp-encrypt-decrypt-verify.hwf: ########## @@ -0,0 +1,320 @@ +<?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>pgp-encrypt-decrypt-verify</name> + <name_sync_with_filename>Y</name_sync_with_filename> + <description/> + <extended_description/> + <created_user>-</created_user> + <modified_user>-</modified_user> + <created_date>2026/10/07 10:00:00.000</created_date> + <modified_date>2026/10/07 10:00:00.000</modified_date> + <created_hop_version/> + <modified_hop_version/> + <workflow_version/> + <parameters/> + <actions> + <action> + <repeat>N</repeat> + <schedulerType>0</schedulerType> + <intervalSeconds>0</intervalSeconds> + <intervalMinutes>60</intervalMinutes> + <DayOfMonth>1</DayOfMonth> + <weekDay>1</weekDay> + <minutes>0</minutes> + <hour>12</hour> + <doNotWaitOnFirstExecution>N</doNotWaitOnFirstExecution> + <name>Start</name> + <description/> + <type>SPECIAL</type> + <attributes/> + <xloc>80</xloc> + <yloc>400</yloc> + <parallel>N</parallel> + <attributes_hac/> + </action> + <action> + <filename>${java.io.tmpdir}/hop-samples/pgp/report.csv</filename> + <createParentFolder>Y</createParentFolder> + <appendFile>N</appendFile> + <content>id,amount +1,100</content> + <encoding>UTF-8</encoding> + <name>Write file</name> + <description/> + <type>WRITE_TO_FILE</type> + <attributes/> + <xloc>272</xloc> + <yloc>400</yloc> + <parallel>N</parallel> + <attributes_hac/> + </action> + <action> + <arg_from_previous>N</arg_from_previous> + <include_subfolders>N</include_subfolders> + <add_result_filesname>N</add_result_filesname> + <destination_is_a_file>Y</destination_is_a_file> + <create_destination_folder>N</create_destination_folder> + <fields> + <field> + <action_type>encrypt</action_type> + <source_filefolder>${java.io.tmpdir}/hop-samples/pgp/report.csv</source_filefolder> + <userid>${PGP_USER_ID}</userid> + <destination_filefolder>${java.io.tmpdir}/hop-samples/pgp/report.csv.gpg</destination_filefolder> + <wildcard/> + </field> + </fields> + <nr_errors_less_than>10</nr_errors_less_than> + <success_condition>success_if_no_errors</success_condition> + <add_date>N</add_date> + <add_time>N</add_time> + <SpecifyFormat>N</SpecifyFormat> + <date_time_format/> + <AddDateBeforeExtension>N</AddDateBeforeExtension> + <DoNotKeepFolderStructure>N</DoNotKeepFolderStructure> + <iffileexists>overwrite_file</iffileexists> + <destinationFolder/> + <ifmovedfileexists>do_nothing</ifmovedfileexists> + <moved_date_time_format/> + <AddMovedDateBeforeExtension>N</AddMovedDateBeforeExtension> + <add_moved_date>N</add_moved_date> + <add_moved_time>N</add_moved_time> + <SpecifyMoveFormat>N</SpecifyMoveFormat> + <create_move_to_folder>N</create_move_to_folder> + <gpglocation>${PGP_GPG_LOCATION}</gpglocation> + <asciiMode>Y</asciiMode> + <name>Encrypt file</name> + <description/> + <type>PGP_ENCRYPT_FILES</type> + <attributes/> + <xloc>464</xloc> + <yloc>400</yloc> + <parallel>N</parallel> + <attributes_hac/> + </action> + <action> + <arg_from_previous>N</arg_from_previous> + <include_subfolders>N</include_subfolders> + <add_result_filesname>N</add_result_filesname> + <destination_is_a_file>Y</destination_is_a_file> + <create_destination_folder>N</create_destination_folder> + <fields> + <field> + <source_filefolder>${java.io.tmpdir}/hop-samples/pgp/report.csv.gpg</source_filefolder> + <passphrase>${PGP_PASSPHRASE}</passphrase> + <destination_filefolder>${java.io.tmpdir}/hop-samples/pgp/report-decrypted.csv</destination_filefolder> + <wildcard/> + </field> + </fields> + <nr_errors_less_than>10</nr_errors_less_than> + <success_condition>success_if_no_errors</success_condition> + <add_date>N</add_date> + <add_time>N</add_time> + <SpecifyFormat>N</SpecifyFormat> + <date_time_format/> + <AddDateBeforeExtension>N</AddDateBeforeExtension> + <DoNotKeepFolderStructure>N</DoNotKeepFolderStructure> + <iffileexists>overwrite_file</iffileexists> + <destinationFolder/> + <ifmovedfileexists>do_nothing</ifmovedfileexists> + <moved_date_time_format/> + <AddMovedDateBeforeExtension>N</AddMovedDateBeforeExtension> + <add_moved_date>N</add_moved_date> + <add_moved_time>N</add_moved_time> + <SpecifyMoveFormat>N</SpecifyMoveFormat> + <create_move_to_folder>N</create_move_to_folder> + <gpglocation>${PGP_GPG_LOCATION}</gpglocation> + <name>Decrypt file</name> + <description/> + <type>PGP_DECRYPT_FILES</type> + <attributes/> + <xloc>656</xloc> + <yloc>400</yloc> + <parallel>N</parallel> + <attributes_hac/> + </action> + <action> + <arg_from_previous>N</arg_from_previous> + <include_subfolders>N</include_subfolders> + <add_result_filesname>N</add_result_filesname> + <destination_is_a_file>Y</destination_is_a_file> + <create_destination_folder>N</create_destination_folder> + <fields> + <field> + <action_type>sign</action_type> + <source_filefolder>${java.io.tmpdir}/hop-samples/pgp/report.csv</source_filefolder> + <userid>${PGP_USER_ID}</userid> Review Comment: **[bug]** This row is the sign step, but `userid` is only the encryption recipient. `ActionPGPEncryptFiles` signs with `local_user` (`gpg -u`) and explicitly ignores `userid` for `action_type` `sign`, logging `UserIdIgnoredWhenSigning`. The note tells you to set `PGP_USER_ID` as the key, yet this step leaves `local_user` empty, so GnuPG signs with its default secret key instead of that id. Encrypt above is fine; sign and the following verify are not using the key the sample documents. **Suggestion:** On the sign field, set `local_user` to `${PGP_USER_ID}` and drop `userid` (or leave it empty). Keep `userid` only on the encrypt row. -- 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]
