address review comments

.github/workflows/manual-package.yml

@@ -11,10 +11,6 @@ on:
           - deb
           - rpm
           - both
-      artifact_run_id:
-        description: "Run ID to download binary artifact from (leave empty for latest on this branch)"
-        required: false
-        type: string
       version:
         description: "Version override (leave empty to auto-detect)"
         required: false

.github/workflows/on-trigger.yml

@@ -79,7 +79,7 @@ jobs:
     strategy:
       fail-fast: ${{ github.event_name == 'merge_group' }}
       matrix:
-        os: [linux]
+        os: [linux, macos, windows]
     with:
       # Enable ccache only for events targeting the XRPLF repository, since
       # other accounts will not have access to our remote cache storage.

.github/workflows/reusable-package.yml

@@ -45,25 +45,32 @@ jobs:
         uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
 
       - name: Download pre-built binary
-        uses: actions/download-artifact@v4
+        uses: actions/download-artifact@fa0a91b85d4f404e444306234e96a603a47e3d4d # v4.1.8
         with:
           name: ${{ inputs.artifact_name }}
           path: ${{ env.BUILD_DIR }}
 
       - name: Make binary executable
-        run: chmod +x ${{ env.BUILD_DIR }}/xrpld
+        run: chmod +x "${BUILD_DIR}/xrpld"
 
       - name: Generate RPM spec from template
         if: ${{ inputs.pkg_type == 'rpm' }}
+        env:
+          PKG_VERSION: ${{ inputs.version }}
+          PKG_RELEASE: ${{ inputs.pkg_release }}
         run: |
-          mkdir -p ${{ env.BUILD_DIR }}/package/rpm
-          sed -e "s/@xrpld_version@/${{ inputs.version }}/" \
-              -e "s/@pkg_release@/${{ inputs.pkg_release }}/" \
-            package/rpm/xrpld.spec.in > ${{ env.BUILD_DIR }}/package/rpm/xrpld.spec
+          mkdir -p "${BUILD_DIR}/package/rpm"
+          sed -e "s/@xrpld_version@/${PKG_VERSION}/" \
+              -e "s/@pkg_release@/${PKG_RELEASE}/" \
+            package/rpm/xrpld.spec.in > "${BUILD_DIR}/package/rpm/xrpld.spec"
 
       - name: Build package
+        env:
+          PKG_TYPE: ${{ inputs.pkg_type }}
+          PKG_VERSION: ${{ inputs.version }}
+          PKG_RELEASE: ${{ inputs.pkg_release }}
         run: |
-          ./package/build_pkg.sh ${{ inputs.pkg_type }} . ${{ env.BUILD_DIR }} "${{ inputs.version }}" "${{ inputs.pkg_release }}"
+          ./package/build_pkg.sh "$PKG_TYPE" . "$BUILD_DIR" "$PKG_VERSION" "$PKG_RELEASE"
 
       - name: Upload package artifact
         uses: actions/upload-artifact@bbbca2ddaa5d8feaa63e36b76fdaad77386f024f # v7.0.0

package/build_pkg.sh

@@ -39,6 +39,8 @@ build_rpm() {
     cp "${SHARED}/xrpld.service"  "${topdir}/SOURCES/xrpld.service"
     cp "${SHARED}/xrpld.sysusers" "${topdir}/SOURCES/xrpld.sysusers"
     cp "${SHARED}/xrpld.tmpfiles" "${topdir}/SOURCES/xrpld.tmpfiles"
+    cp "${SRC_DIR}/LICENSE.md"    "${topdir}/SOURCES/LICENSE.md"
+    cp "${SRC_DIR}/README.md"     "${topdir}/SOURCES/README.md"
 
     set -x
     rpmbuild -bb \
@@ -77,8 +79,7 @@ EOF
     chmod +x "${staging}/debian/rules"
 
     set -x
-    cd "${staging}"
-    dpkg-buildpackage -b --no-sign -d
+    ( cd "${staging}" && dpkg-buildpackage -b --no-sign -d )
 }
 
 case "${PKG_TYPE}" in

package/deb/debian/rules

@@ -21,8 +21,6 @@ override_dh_auto_install:
 
 override_dh_installsystemd:
 	dh_installsystemd
-#	see if this still works
-#	dh_installsystemd --no-start
 
 override_dh_installsysusers:
 	dh_installsysusers

package/rpm/xrpld.spec.in

@@ -23,6 +23,8 @@ Source5:  xrpld.tmpfiles
 Source6:  xrpld.logrotate
 Source7:  update-xrpld.sh
 Source8:  update-xrpld-cron
+Source9:  LICENSE.md
+Source10: README.md
 
 BuildArch:     x86_64
 BuildRequires: systemd-rpm-macros
@@ -86,6 +88,8 @@ install -Dm0644 %{SOURCE8} %{buildroot}%{_opt_prefix}/bin/update-xrpld-cron
 %systemd_postun_with_restart xrpld.service
 
 %files
+%license %{SOURCE9}
+%doc %{SOURCE10}
 %dir %{_opt_prefix}
 %dir %{_opt_prefix}/bin
 %{_opt_prefix}/bin/xrpld

package/shared/update-xrpld-cron

@@ -3,7 +3,7 @@
 
 # bash required for use of RANDOM below.
 SHELL=/bin/bash
-PATH=/sbin;/bin;/usr/sbin;/usr/bin
+PATH=/sbin:/bin:/usr/sbin:/usr/bin
 
 # invoke check/update script with random delay up to 59 mins
 0 * * * * root sleep $((RANDOM*3540/32768)) && /opt/xrpld/bin/update-xrpld.sh

package/shared/update-xrpld.sh

@@ -13,12 +13,12 @@ UPDATELOG=/var/log/xrpld/update.log
 
 function cleanup {
   # If this directory isn't removed, future updates will fail.
-  rmdir $LOCKDIR
+  rmdir "$LOCKDIR"
 }
 
 # Use mkdir to check if process is already running. mkdir is atomic, as against file create.
-if ! mkdir $LOCKDIR 2>/dev/null; then
-  echo $(date -u) "lockdir exists - won't proceed." >> $UPDATELOG
+if ! mkdir "$LOCKDIR" 2>/dev/null; then
+  echo "$(date -u) lockdir exists - won't proceed." >> "$UPDATELOG"
   exit 1
 fi
 trap cleanup EXIT
@@ -35,16 +35,24 @@ if [[ "$ID" == "ubuntu" || "$ID" == "debian" ]] ; then
   test "$XRPLD" == "xrpld" && can_update=true
 
   function apply_update {
-    apt-get install xrpld -qq
+    DEBIAN_FRONTEND=noninteractive apt-get install -y -qq xrpld
   }
 elif [[ "$ID" == "fedora" || "$ID" == "centos" || "$ID" == "rhel" || "$ID" == "scientific" ]] ; then
   RIPPLE_REPO=${RIPPLE_REPO-stable}
-  yum --disablerepo=* --enablerepo=ripple-$RIPPLE_REPO clean expire-cache
+  yum --disablerepo=* --enablerepo="ripple-${RIPPLE_REPO}" clean expire-cache
 
-  yum check-update -q --enablerepo=ripple-$RIPPLE_REPO xrpld || can_update=true
+  # yum check-update exits 100 when updates are available, 0 for none, 1 for errors.
+  yum check-update -q --enablerepo="ripple-${RIPPLE_REPO}" xrpld
+  rc=$?
+  if [ $rc -eq 100 ]; then
+    can_update=true
+  elif [ $rc -ne 0 ]; then
+    echo "yum check-update failed with exit code $rc"
+    exit 1
+  fi
 
   function apply_update {
-    yum update -y --enablerepo=ripple-$RIPPLE_REPO xrpld
+    yum update -y --enablerepo="ripple-${RIPPLE_REPO}" xrpld
   }
 else
   echo "unrecognized distro!"
@@ -53,12 +61,12 @@ fi
 
 # Do the actual update and restart the service after reloading systemctl daemon.
 if [ "$can_update" = true ] ; then
-  exec 3>&1 1>>${UPDATELOG} 2>&1
+  exec 3>&1 1>>"${UPDATELOG}" 2>&1
   set -e
   apply_update
   systemctl daemon-reload
   systemctl restart xrpld.service
-  echo $(date -u) "xrpld daemon updated."
+  echo "$(date -u) xrpld daemon updated."
 else
-  echo $(date -u) "no updates available" >> $UPDATELOG
+  echo "$(date -u) no updates available" >> "$UPDATELOG"
 fi

package/shared/xrpld.service

@@ -7,6 +7,9 @@ Wants=network-online.target
 Type=simple
 ExecStart=/opt/xrpld/bin/xrpld --net --silent --conf /etc/opt/xrpld/xrpld.cfg
 Restart=on-failure
+NoNewPrivileges=true
+ProtectSystem=full
+PrivateTmp=true
 User=xrpld
 Group=xrpld
 LimitNOFILE=65536

package/test/smoketest.sh

@@ -4,6 +4,7 @@
 # Usage: smoketest.sh local
 #   Expects packages in build/{dpkg,rpm}/packages/ or build/debbuild/ / build/rpmbuild/RPMS/
 
+set -o pipefail
 set -x
 trap 'test $? -ne 0 && touch /tmp/test_failed' EXIT
 
@@ -32,21 +33,21 @@ fi
 if [ "${pkgtype}" = "dpkg" ] ; then
     apt-get -y update
     # Find .deb files — check both possible output locations
-    debs=$(find build/debbuild/ build/dpkg/packages/ -name '*.deb' ! -name '*dbgsym*' 2>/dev/null | head -5)
-    if [ -z "$debs" ]; then
+    mapfile -t debs < <(find build/debbuild/ build/dpkg/packages/ -name '*.deb' ! -name '*dbgsym*' 2>/dev/null)
+    if [ ${#debs[@]} -eq 0 ]; then
         echo "No .deb files found"
         exit 1
     fi
-    dpkg --no-debsig -i $debs || apt-get -y install -f
+    dpkg --no-debsig -i "${debs[@]}" || apt-get -y install -f
 elif [ "${pkgtype}" = "rpm" ] ; then
     # Find .rpm files — check both possible output locations
-    rpms=$(find build/rpmbuild/RPMS/ build/rpm/packages/ -name '*.rpm' \
-        ! -name '*debug*' ! -name '*devel*' ! -name '*.src.rpm' 2>/dev/null | head -5)
-    if [ -z "$rpms" ]; then
+    mapfile -t rpms < <(find build/rpmbuild/RPMS/ build/rpm/packages/ -name '*.rpm' \
+        ! -name '*debug*' ! -name '*devel*' ! -name '*.src.rpm' 2>/dev/null)
+    if [ ${#rpms[@]} -eq 0 ]; then
         echo "No .rpm files found"
         exit 1
     fi
-    rpm -i $rpms
+    rpm -i "${rpms[@]}"
 fi
 
 # Verify installed version
@@ -61,12 +62,22 @@ else
     unittest_jobs=16
 fi
 
-cd /tmp
-/opt/xrpld/bin/xrpld --unittest --unittest-jobs ${unittest_jobs} > /tmp/unittest_results || true
-cd -
+(
+    cd /tmp
+    /opt/xrpld/bin/xrpld --unittest --unittest-jobs "${unittest_jobs}" > /tmp/unittest_results || true
+)
+
+if [ ! -s /tmp/unittest_results ]; then
+    echo "Unit test results file is empty — xrpld may have crashed"
+    exit 1
+fi
 
 num_failures=$(tail /tmp/unittest_results -n1 | grep -oP '\d+(?= failures)')
-if [ "${num_failures:-0}" -ne 0 ]; then
+if [ -z "$num_failures" ]; then
+    echo "Could not parse unit test results — expected summary line not found"
+    exit 1
+fi
+if [ "${num_failures}" -ne 0 ]; then
     echo "$num_failures unit test(s) failed:"
     grep 'failed:' /tmp/unittest_results
     exit 1