Harden utils

.github/workflows/test.yaml

@@ -14,7 +14,6 @@ jobs:
       fail-fast: true
 
     runs-on: ubuntu-latest
-    if: github.event.pull_request.draft == false
 
     steps:
       - name: Allow unprivileged user namespaces (for Ubuntu 24.04)
@@ -26,7 +25,7 @@ jobs:
           sudo apt-get install expect mergerfs attr pandoc
 
       - name: Checkout
-        uses: actions/checkout@v4
+        uses: actions/checkout@v7
         with:
           ref: ${{ github.event.pull_request.head.sha }}
 
@@ -42,7 +41,7 @@ jobs:
           make dist
 
       - name: Upload dist tarball
-        uses: actions/upload-artifact@v4
+        uses: actions/upload-artifact@v7
         with:
           name: try-dist.tgz
           path: try-*.tgz
@@ -65,12 +64,13 @@ jobs:
           sudo apt-get install expect mergerfs attr pandoc
 
       - name: Download dist tarball
-        uses: actions/download-artifact@v4
+        uses: actions/download-artifact@v8
 
       - name: Unpack tarball; configure and build utilities
         run: |
-          tar xzf try-dist.tgz/try-*.tgz --strip-components=1
-          rm -r try-dist.tgz
+          ls -lR
+          tar xzf try-*.tgz --strip-components=1
+          rm try-*.tgz
           ./configure --disable-utils
           make all
           sudo make install
@@ -107,7 +107,6 @@ jobs:
       fail-fast: false
 
     runs-on: ubuntu-latest
-    if: github.event.pull_request.draft == false
 
     steps:
       - name: Allow unprivileged user namespaces (for Ubuntu 24.04)
@@ -119,7 +118,7 @@ jobs:
           sudo apt-get install expect mergerfs attr pandoc
 
       - name: Checkout
-        uses: actions/checkout@v4
+        uses: actions/checkout@v7
         with:
           ref: ${{ github.event.pull_request.head.sha }}
 
@@ -162,7 +161,7 @@ jobs:
 
     steps:
       - name: Checkout
-        uses: actions/checkout@v4
+        uses: actions/checkout@v7
         with:
           ref: ${{ github.event.pull_request.head.sha }}
 
@@ -172,7 +171,7 @@ jobs:
   trycase-check:
     runs-on: ubuntu-latest
     steps:
-      - uses: actions/checkout@v4
+      - uses: actions/checkout@v7
         with:
           ref: ${{ github.event.pull_request.head.sha }}
 
@@ -181,11 +180,10 @@ jobs:
 
   version-check:
     runs-on: ubuntu-latest
-    if: github.event.pull_request.draft == false
 
     steps:
       - name: Checkout
-        uses: actions/checkout@v4
+        uses: actions/checkout@v7
         with:
           ref: ${{ github.event.pull_request.head.sha }}
 
@@ -196,7 +194,7 @@ jobs:
     runs-on: ubuntu-latest
     steps:
       - name: Checkout
-        uses: actions/checkout@v4
+        uses: actions/checkout@v7
         with:
           ref: ${{ github.event.pull_request.head.sha }}
 
@@ -212,7 +210,7 @@ jobs:
     runs-on: ubuntu-latest
     steps:
       - name: Checkout
-        uses: actions/checkout@v4
+        uses: actions/checkout@v7
         with:
           ref: ${{ github.event.pull_request.head.sha }}
 

Makefile.in

@@ -9,21 +9,21 @@ CC=@CC@
 endif
 CFLAGS=@CFLAGS@
 CPPFLAGS=@CPPFLAGS@
+LDFLAGS=@LDFLAGS@
 INSTALL=@INSTALL@
 
 .PHONY: all install test dist lint clean
 
 DISTDIR=@PACKAGE_TARNAME@-@PACKAGE_VERSION@
 DISTTGZ=$(DISTDIR).tgz
 
-TARGETS=utils/make-socket $(if $(findstring yes,@enable_utils@),utils/try-summary utils/try-commit) man/try.1.gz
+TARGETS=utils/try-make-socket $(if $(findstring yes,@enable_utils@),utils/try-summary utils/try-commit) man/try.1.gz
 
 all: $(TARGETS)
 
 install: $(TARGETS)
 	$(INSTALL) -d $(bindir)
 	$(INSTALL) -m 755 try $(bindir)
-	$(INSTALL) -m 755 utils/make-socket $(bindir)
 	$(INSTALL) -m 755 utils/try-parse-trace $(bindir)
 ifeq (@enable_utils@, yes)
 	$(INSTALL) -m 755 utils/try-summary $(bindir)
@@ -55,15 +55,15 @@ man/try.1: docs/try.1.md
 	pandoc --standalone --from markdown-smart --to man $< -o $@
 
 utils/try-summary: utils/excludes.o utils/try-summary.o
-	$(CC) $(CPPFLAGS) $(CFLAGS) -o $@ -g $^
+	$(CC) $(CPPFLAGS) $(CFLAGS) $(LDFLAGS) -o $@ -g $^
 
 utils/try-commit: utils/excludes.o utils/try-commit.o
-	$(CC) $(CPPFLAGS) $(CFLAGS) -o $@ -g $^
+	$(CC) $(CPPFLAGS) $(CFLAGS) $(LDFLAGS) -o $@ -g $^
 
-utils/make-socket: utils/make-socket.o
-	$(CC) $(CPPFLAGS) $(CFLAGS) -o $@ -g $^
+utils/try-make-socket: utils/try-make-socket.o
+	$(CC) $(CPPFLAGS) $(CFLAGS) $(LDFLAGS) -o $@ -g $^
 
-test: try $(if $(findstring yes,@enable_utils@),utils/try-summary utils/try-commit) utils/make-socket
+test: try $(if $(findstring yes,@enable_utils@),utils/try-summary utils/try-commit) utils/try-make-socket
 	scripts/run_tests.sh
 
 lint:

configure.ac

@@ -11,23 +11,30 @@ AC_ARG_ENABLE([utils],
               [enable_utils=${enableval}], [enable_utils=yes])
 
 
-AC_REQUIRE_AUX_FILE([utils/make-socket.c])
+AC_REQUIRE_AUX_FILE([utils/try-make-socket.c])
+
+# unconditionally require CC and its flags (for try-make-socket)
+
+# -O2 is required for -D_FORTIFY_SOURCE to do anything; -U_FORTIFY_SOURCE
+# first avoids a redefinition warning on distros whose GCC predefines it.
+AUTO_CFLAGS="-O2 -Wall -Wextra -Werror -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -fstack-protector-strong -fPIE"
+AUTO_CPPFLAGS=""
+AUTO_LDFLAGS="-pie -Wl,-z,relro,-z,now"
+
+CFLAGS=${CFLAGS-"$AUTO_CFLAGS"}
+CPPFLAGS=${CPPFLAGS-"$AUTO_CPPFLAGS"}
+LDFLAGS=${LDFLAGS-"$AUTO_LDFLAGS"}
+AC_SUBST(CFLAGS)
+AC_SUBST(CPPFLAGS)
+AC_SUBST(LDFLAGS)
 
 # build tools
 AC_PROG_CC
 
-# CFLAGS AND CPPFLAGGS
-AUTO_CFLAGS=""
-AUTO_CPPFLAGS=""
 
 if test "$enable_utils" = "yes"
 then
-    AC_REQUIRE_AUX_FILE([utils/try-commit.c])
-
-	CFLAGS=${CFLAGS-"$AUTO_CFLAGS"}
-	CPPFLAGS=${CPPFLAGS-"$AUTO_CPPFLAGS"}
-	AC_SUBST(CFLAGS)
-	AC_SUBST(CPPFLAGS)
+	AC_REQUIRE_AUX_FILE([utils/try-commit.c])
 
 	# needed C types
 	AC_TYPE_PID_T

package.nix

@@ -18,6 +18,10 @@ stdenv.mkDerivation {
 
   src = ./.;
 
+  # "out" is the real product; "test" holds try-make-socket, a helper the
+  # test suite needs but that should never be installed onto a user's PATH.
+  outputs = [ "out" "test" ];
+
   # skip TRY_REQUIRE_PROG as it detects executable dependencies by running it
   postPatch = ''
     sed -i '/^AC_DEFUN(\[TRY_REQUIRE_PROG\]/,/^])$/c\AC_DEFUN([TRY_REQUIRE_PROG], [])' configure.ac
@@ -35,7 +39,6 @@ stdenv.mkDerivation {
     install -Dt $out/bin try
     install -Dt $out/bin utils/try-commit
     install -Dt $out/bin utils/try-summary
-    install -Dt $out/bin utils/make-socket
     wrapProgram $out/bin/try --prefix PATH : ${
       lib.makeBinPath [
         coreutils
@@ -46,6 +49,9 @@ stdenv.mkDerivation {
     }
     installManPage man/try.1.gz
     installShellCompletion --bash --name try.bash completions/try.bash
+
+    install -Dt $test/bin utils/try-make-socket
+
     runHook postInstall
   '';
 
@@ -68,6 +74,7 @@ stdenv.mkDerivation {
     ];
     license = with lib.licenses; [ mit ];
     platforms = lib.platforms.linux;
+    outputsToInstall = [ "out" ];
   };
 }
 

shell.nix

@@ -1,5 +1,8 @@
 { pkgs ? import <nixpkgs> {}}:
 
+let
+  tryPkg = pkgs.callPackage ./package.nix {};
+in
 pkgs.mkShell {
   buildInputs = with pkgs; [
     expect
@@ -10,7 +13,8 @@ pkgs.mkShell {
     shellcheck
     autoconf
     pandoc
-    (pkgs.callPackage ./package.nix {})
+    tryPkg
+    tryPkg.test
   ];
 }
 

test/all-commit-cases.sh

@@ -15,6 +15,10 @@ cleanup() {
 
 trap 'cleanup' EXIT
 
+# expose test-only utils, like `try-make-socket`
+PATH="$TRY_TOP/utils:$PATH"
+export PATH
+
 try_workspace="$(mktemp -d)"
 cd "$try_workspace" || exit 99
 
@@ -272,11 +276,16 @@ rm newpipe
 
 : $((COUNT += 1))
 
+if ! type try-make-socket >/dev/null 2>&1
+then
+    echo "could not find try-make-socket in PATH=$PATH"
+    fail
+fi
 ! [ -e newsock ] || fail
 "$TRY" -y "touch newsock; echo hello> newsock"
 [ -f newsock ] || fail
 [ "$(cat newsock)" = "hello" ] || fail
-"$TRY" -y "rm newsock; make-socket newsock"
+"$TRY" -y "rm newsock; try-make-socket newsock"
 [ -S newsock ] || fail
 rm newsock
 
@@ -287,7 +296,7 @@ rm newsock
 ! [ -e newsock ] || fail
 "$TRY" -y "mkdir newsock"
 [ -d newsock ] || fail
-"$TRY" -y "rm -r newsock; make-socket newsock"
+"$TRY" -y "rm -r newsock; try-make-socket newsock"
 [ -S newsock ] || fail
 rm newsock
 
@@ -298,7 +307,7 @@ rm newsock
 ! [ -e newsock ] || fail
 ln -s "$TRY" newsock
 [ -L newsock ] || fail
-"$TRY" -y "rm newsock; make-socket newsock"
+"$TRY" -y "rm newsock; try-make-socket newsock"
 ! [ -e newlink ] || fail
 [ -S newsock ] || fail
 rm newsock
@@ -308,6 +317,6 @@ rm newsock
 : $((COUNT += 1))
 
 ! [ -e newsock ]
-"$TRY" -y "make-socket newsock"
+"$TRY" -y "try-make-socket newsock"
 [ -S newsock ] || fail
 rm newsock

utils/.gitignore

@@ -1,4 +1,4 @@
 try-summary
 try-commit
 *.o
-make-socket
+try-make-socket

utils/excludes.c

@@ -6,13 +6,13 @@
 
 static regex_t *excludes = NULL;
 static size_t excludes_len = 0;
-static int num_excludes = 0;
+static size_t num_excludes = 0;
 static regex_t *includes = NULL;
 static size_t includes_len = 0;
-static int num_includes = 0;
+static size_t num_includes = 0;
 
 int should_exclude(char *filename) {
-  for (int i = 0; i < num_excludes; i += 1) {
+  for (size_t i = 0; i < num_excludes; i += 1) {
     if (regexec(&excludes[i], filename, 0, NULL, 0) == 0) {
       return 1;
     }
@@ -26,7 +26,7 @@ int should_include(char *filename) {
     return 1;
   }
 
-  for (int i = 0; i < num_includes; i += 1) {
+  for (size_t i = 0; i < num_includes; i += 1) {
     if (regexec(&includes[i], filename, 0, NULL, 0) == 0) {
       return 1;
     }
@@ -114,7 +114,7 @@ void load_includes(char *progname, char *include_filename) {
 }
 
 void free_excludes() {
-  for (int i = 0; i < num_excludes; i += 1) {
+  for (size_t i = 0; i < num_excludes; i += 1) {
     regfree(&excludes[i]);
   }
   free(excludes);
@@ -124,7 +124,7 @@ void free_excludes() {
 }
 
 void free_includes() {
-  for (int i = 0; i < num_includes; i += 1) {
+  for (size_t i = 0; i < num_includes; i += 1) {
     regfree(&includes[i]);
   }
   free(includes);

utils/try-commit.c

@@ -7,6 +7,7 @@
 #include <linux/limits.h>
 #include <stdio.h>
 #include <stdlib.h>
+#include <stdnoreturn.h>
 #include <string.h>
 #include <sys/types.h>
 #include <sys/stat.h>
@@ -31,7 +32,9 @@ void run(char *argv[], char *file) {
 
     if (pid == 0) {
       execvp(argv[0], argv);
-      return; // unreachable
+      // reachable only when we can't exec `argv[0]`
+      fprintf(stderr, "try-commit: couldn't run '%s': %s\n", argv[0], strerror(errno));
+      _exit(127);
     }
 
     int status = -1;
@@ -77,7 +80,7 @@ void remove_local(char *local_file, int local_exists, struct stat *local_stat) {
   }
 }
 
-void usage(int status) {
+noreturn void usage(int status) {
   fprintf(stderr, "Usage: try-commit [-c] [-E EXCLUDE_FILE] [-I INCLUDE_FILE] SANDBOX_DIR\n");
   fprintf(stderr, "\t-c\tcopy files instead of moving them\n");
   exit(status);
@@ -237,12 +240,12 @@ int main(int argc, char *argv[]) {
       }
 
       char *tgt = malloc(sizeof(char) * tgt_len);
-      int nbytes = readlink(ent->fts_path, tgt, tgt_len);
+      ssize_t nbytes = readlink(ent->fts_path, tgt, tgt_len);
       if (nbytes == -1) {
         commit_error(ent->fts_path, "ln -s");
       }
 
-      while (nbytes == tgt_len) {
+      while ((size_t) nbytes == tgt_len) {
         tgt_len *= 2;
         tgt = realloc(tgt, sizeof(char) * tgt_len);
         nbytes = readlink(ent->fts_path, tgt, tgt_len);