build: gate htssafe pointer-dest regressions with -Werror=attribute-warning

The strcpybuff/strcatbuff/strncatbuff family in htssafe.h silently degrades
to an unchecked strcpy/strcat/strncat when the destination is a bare char*
(capacity unknown); the pointer-destination stubs carry a 'warning' function
attribute to flag every such call. The migration that converted those 241
sites is complete, so any new char* destination is a regression.

Promote that attribute to a hard error in our own build via
-Werror=attribute-warning (gcc) / -Werror=user-defined-warnings (clang),
probed with AX_CHECK_COMPILE_FLAG so each compiler picks up only the spelling
it accepts. htssafe.h is unchanged, so downstream consumers of the installed
header still see a plain warning rather than a build break.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Xavier Roche <roche@httrack.com>

.github/workflows/ci.yml

@@ -320,20 +320,37 @@ jobs:
   lint:
     name: lint (shellcheck, shfmt)
     runs-on: ubuntu-24.04
+    env:
+      SHFMT_VERSION: v3.8.0
     steps:
       - uses: actions/checkout@v6
 
+      # shfmt is a pinned release binary, so it never changes: cache it keyed on
+      # the version. Same rationale as the git-clang-format driver below -- avoid
+      # re-downloading an unchanging file from github.com on every run.
+      - name: Cache shfmt binary
+        uses: actions/cache@v4
+        with:
+          path: ~/.cache/shfmt/shfmt
+          key: shfmt-${{ env.SHFMT_VERSION }}-${{ runner.arch }}
+
       - name: Install linters
-        env:
-          SHFMT_VERSION: v3.8.0
         run: |
           set -euo pipefail
           sudo apt-get update
           sudo apt-get install -y --no-install-recommends shellcheck
-          # shfmt is not packaged in apt; fetch a pinned release binary.
-          curl -fsSL -o /tmp/shfmt \
-            "https://github.com/mvdan/sh/releases/download/${SHFMT_VERSION}/shfmt_${SHFMT_VERSION}_linux_$(dpkg --print-architecture)"
-          sudo install -m 0755 /tmp/shfmt /usr/local/bin/shfmt
+          # shfmt is not packaged in apt; fetch a pinned release binary (cold
+          # cache only), retrying through transient errors.
+          shfmt="$HOME/.cache/shfmt/shfmt"
+          if [ ! -s "$shfmt" ]; then
+            echo "shfmt cache MISS: fetching ${SHFMT_VERSION} from github.com"
+            mkdir -p "$(dirname "$shfmt")"
+            curl --retry 5 --retry-all-errors -fsSL -o "$shfmt" \
+              "https://github.com/mvdan/sh/releases/download/${SHFMT_VERSION}/shfmt_${SHFMT_VERSION}_linux_$(dpkg --print-architecture)"
+          else
+            echo "shfmt cache HIT: using cached ${SHFMT_VERSION}"
+          fi
+          sudo install -m 0755 "$shfmt" /usr/local/bin/shfmt
 
       # Lint the scripts we maintain; the legacy scripts are a separate cleanup.
       - name: shellcheck
@@ -349,11 +366,24 @@ jobs:
     name: format (clang-format-19, changed lines)
     if: github.event_name == 'pull_request'
     runs-on: ubuntu-24.04
+    env:
+      # Single-source the tag so the cache key and the fetch URL can never drift.
+      LLVM_TAG: llvmorg-19.1.7
     steps:
       - uses: actions/checkout@v6
         with:
           fetch-depth: 0
 
+      # The git-clang-format driver is pinned to an immutable release tag, so the
+      # fetched file never changes: cache it keyed on the tag. raw.githubusercontent.com
+      # 429-rate-limits the shared runner egress IPs, and re-downloading an unchanging
+      # file every run was the only thing that could (and did) hit that limit.
+      - name: Cache git-clang-format driver
+        uses: actions/cache@v4
+        with:
+          path: ~/.cache/git-clang-format/git-clang-format
+          key: git-clang-format-${{ env.LLVM_TAG }}
+
       - name: Install clang-format 19 (pinned, from apt.llvm.org)
         run: |
           set -euo pipefail
@@ -364,11 +394,17 @@ jobs:
             | sudo tee /etc/apt/sources.list.d/llvm-19.list >/dev/null
           sudo apt-get update
           sudo apt-get install -y --no-install-recommends clang-format-19
-          # git-clang-format driver, pinned to an immutable release tag (not a
-          # moving branch) since we curl and then execute it.
-          sudo curl -fsSL -o /usr/local/bin/git-clang-format \
-            https://raw.githubusercontent.com/llvm/llvm-project/llvmorg-19.1.7/clang/tools/clang-format/git-clang-format
-          sudo chmod 0755 /usr/local/bin/git-clang-format
+          # Cold cache only: fetch the driver, retrying through transient 429s.
+          driver="$HOME/.cache/git-clang-format/git-clang-format"
+          if [ ! -s "$driver" ]; then
+            echo "git-clang-format cache MISS: fetching ${LLVM_TAG} from raw.githubusercontent.com"
+            mkdir -p "$(dirname "$driver")"
+            curl --retry 5 --retry-all-errors -fsSL -o "$driver" \
+              "https://raw.githubusercontent.com/llvm/llvm-project/${LLVM_TAG}/clang/tools/clang-format/git-clang-format"
+          else
+            echo "git-clang-format cache HIT: using cached ${LLVM_TAG}"
+          fi
+          sudo install -m 0755 "$driver" /usr/local/bin/git-clang-format
           clang-format-19 --version
 
       - name: Check formatting of changed lines

configure

@@ -15541,6 +15541,91 @@ else case e in #(
 esac
 fi
 
+# Make htssafe.h's pointer-dest 'warning' attribute a hard error in our build
+# (migration is at zero; a new char* dest is a regression). gcc/clang each take
+# only their own spelling; downstream keeps the plain warning, not a build break.
+{ printf "%s\n" "$as_me:${as_lineno-$LINENO}: checking whether C compiler accepts -Werror=attribute-warning" >&5
+printf %s "checking whether C compiler accepts -Werror=attribute-warning... " >&6; }
+if test ${ax_cv_check_cflags___Werror_attribute_warning+y}
+then :
+  printf %s "(cached) " >&6
+else case e in #(
+  e)
+  ax_check_save_flags=$CFLAGS
+  CFLAGS="$CFLAGS  -Werror=attribute-warning"
+  cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h.  */
+
+int
+main (void)
+{
+
+  ;
+  return 0;
+}
+_ACEOF
+if ac_fn_c_try_compile "$LINENO"
+then :
+  ax_cv_check_cflags___Werror_attribute_warning=yes
+else case e in #(
+  e) ax_cv_check_cflags___Werror_attribute_warning=no ;;
+esac
+fi
+rm -f core conftest.err conftest.$ac_objext conftest.beam conftest.$ac_ext
+  CFLAGS=$ax_check_save_flags ;;
+esac
+fi
+{ printf "%s\n" "$as_me:${as_lineno-$LINENO}: result: $ax_cv_check_cflags___Werror_attribute_warning" >&5
+printf "%s\n" "$ax_cv_check_cflags___Werror_attribute_warning" >&6; }
+if test "x$ax_cv_check_cflags___Werror_attribute_warning" = xyes
+then :
+  DEFAULT_CFLAGS="$DEFAULT_CFLAGS -Werror=attribute-warning"
+else case e in #(
+  e) : ;;
+esac
+fi
+
+{ printf "%s\n" "$as_me:${as_lineno-$LINENO}: checking whether C compiler accepts -Werror=user-defined-warnings" >&5
+printf %s "checking whether C compiler accepts -Werror=user-defined-warnings... " >&6; }
+if test ${ax_cv_check_cflags___Werror_user_defined_warnings+y}
+then :
+  printf %s "(cached) " >&6
+else case e in #(
+  e)
+  ax_check_save_flags=$CFLAGS
+  CFLAGS="$CFLAGS  -Werror=user-defined-warnings"
+  cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h.  */
+
+int
+main (void)
+{
+
+  ;
+  return 0;
+}
+_ACEOF
+if ac_fn_c_try_compile "$LINENO"
+then :
+  ax_cv_check_cflags___Werror_user_defined_warnings=yes
+else case e in #(
+  e) ax_cv_check_cflags___Werror_user_defined_warnings=no ;;
+esac
+fi
+rm -f core conftest.err conftest.$ac_objext conftest.beam conftest.$ac_ext
+  CFLAGS=$ax_check_save_flags ;;
+esac
+fi
+{ printf "%s\n" "$as_me:${as_lineno-$LINENO}: result: $ax_cv_check_cflags___Werror_user_defined_warnings" >&5
+printf "%s\n" "$ax_cv_check_cflags___Werror_user_defined_warnings" >&6; }
+if test "x$ax_cv_check_cflags___Werror_user_defined_warnings" = xyes
+then :
+  DEFAULT_CFLAGS="$DEFAULT_CFLAGS -Werror=user-defined-warnings"
+else case e in #(
+  e) : ;;
+esac
+fi
+
 { printf "%s\n" "$as_me:${as_lineno-$LINENO}: checking whether C compiler accepts -fstrict-aliasing -Wstrict-aliasing" >&5
 printf %s "checking whether C compiler accepts -fstrict-aliasing -Wstrict-aliasing... " >&6; }
 if test ${ax_cv_check_cflags___fstrict_aliasing__Wstrict_aliasing+y}

configure.ac

@@ -84,6 +84,11 @@ AX_CHECK_COMPILE_FLAG([-Wformat-nonliteral], [DEFAULT_CFLAGS="$DEFAULT_CFLAGS -W
 AX_CHECK_COMPILE_FLAG([-Wmissing-parameter-type], [DEFAULT_CFLAGS="$DEFAULT_CFLAGS -Wmissing-parameter-type"])
 AX_CHECK_COMPILE_FLAG([-Wold-style-definition], [DEFAULT_CFLAGS="$DEFAULT_CFLAGS -Wold-style-definition"])
 AX_CHECK_COMPILE_FLAG([-Wignored-qualifiers], [DEFAULT_CFLAGS="$DEFAULT_CFLAGS -Wignored-qualifiers"])
+# Make htssafe.h's pointer-dest 'warning' attribute a hard error in our build
+# (migration is at zero; a new char* dest is a regression). gcc/clang each take
+# only their own spelling; downstream keeps the plain warning, not a build break.
+AX_CHECK_COMPILE_FLAG([-Werror=attribute-warning], [DEFAULT_CFLAGS="$DEFAULT_CFLAGS -Werror=attribute-warning"])
+AX_CHECK_COMPILE_FLAG([-Werror=user-defined-warnings], [DEFAULT_CFLAGS="$DEFAULT_CFLAGS -Werror=user-defined-warnings"])
 AX_CHECK_COMPILE_FLAG([-fstrict-aliasing -Wstrict-aliasing], [DEFAULT_CFLAGS="$DEFAULT_CFLAGS -fstrict-aliasing -Wstrict-aliasing"])
 AX_CHECK_COMPILE_FLAG([-fstack-protector], [DEFAULT_CFLAGS="$DEFAULT_CFLAGS -fstack-protector"])
 AX_CHECK_COMPILE_FLAG([-fstack-clash-protection], [DEFAULT_CFLAGS="$DEFAULT_CFLAGS -fstack-clash-protection"])

src/htsparse.c

@@ -617,13 +617,15 @@ int htsparse(htsmoduleStruct * str, htsmoduleStructExtended * stre) {
                                              "index.html")) == 0) {
                         detect_title = 1;       // ok détecté pour cette page!
                         makeindex_links++;      // un de plus
-                        strcpybuff(makeindex_firstlink, tempo);
+                        strlcpybuff(makeindex_firstlink, tempo,
+                                    HTS_URLMAXSIZE * 2);
                         //
 
                         /* Hack */
                         if (opt->mimehtml) {
-                          strcpybuff(makeindex_firstlink,
-                                     "cid:primary/primary");
+                          strlcpybuff(makeindex_firstlink,
+                                      "cid:primary/primary",
+                                      HTS_URLMAXSIZE * 2);
                         }
 
                         if ((b == a) || (a == NULL) || (b == NULL)) {   // pas de titre
@@ -2319,12 +2321,12 @@ int htsparse(htsmoduleStruct * str, htsmoduleStructExtended * stre) {
                       switch (p_type) {
                       case 2:{
                           //if (*lien!='/') strcatbuff(base,"/");
-                          strcpybuff(base, lien);
+                          strlcpybuff(base, lien, HTS_URLMAXSIZE * 2);
                         }
                         break;  // base
                       case -2:{
                           //if (*lien!='/') strcatbuff(codebase,"/");
-                          strcpybuff(codebase, lien);
+                          strlcpybuff(codebase, lien, HTS_URLMAXSIZE * 2);
                         }
                         break;  // base
                       }
@@ -4397,7 +4399,7 @@ int hts_mirror_wait_for_next_file(htsmoduleStruct * str,
       memcpy(r, &(back[b].r), sizeof(htsblk));
       r->location = stre->loc_; // ne PAS copier location!! adresse, pas de buffer
       if (back[b].r.location)
-        strcpybuff(r->location, back[b].r.location);
+        strlcpybuff(r->location, back[b].r.location, HTS_URLMAXSIZE * 2);
       back[b].r.adr = NULL;     // ne pas faire de desalloc ensuite
 
       // libérer emplacement backing

src/htsserver.c

@@ -129,7 +129,8 @@ HTS_UNUSED static int linputsoc_t(T_SOC soc, char *s, int max, int timeout);
 HTS_UNUSED static int linput(FILE * fp, char *s, int max);
 
 /* Language files */
-HTS_UNUSED static int htslang_load(char *limit_to, const char *apppath);
+HTS_UNUSED static int htslang_load(char *limit_to, size_t limit_size,
+                                   const char *apppath);
 HTS_UNUSED static void conv_printf(const char *from, char *to);
 HTS_UNUSED static void LANG_DELETE(void);
 HTS_UNUSED static void LANG_INIT(const char *path);
@@ -325,7 +326,7 @@ int smallserver(T_SOC soc, char *url, char *method, char *data, char *path) {
 
   /* Load strings */
   htslang_init();
-  if (!htslang_load(NULL, path)) {
+  if (!htslang_load(NULL, 0, path)) {
     fprintf(stderr, "unable to find lang.def and/or lang/ strings in %s\n",
             path);
     return 0;
@@ -511,7 +512,7 @@ int smallserver(T_SOC soc, char *url, char *method, char *data, char *path) {
         char *s = buffer;
         char *e, *f;
 
-        strcatbuff(buffer, "&");
+        strlcatbuff(buffer, "&", buffer_size);
         while(s && (e = strchr(s, '=')) && (f = strchr(s, '&'))) {
           const char *ua;
           String sua = STRING_EMPTY;
@@ -935,7 +936,7 @@ int smallserver(T_SOC soc, char *url, char *method, char *data, char *path) {
                     int listDefault = 0;
 
                     name[0] = '\0';
-                    strncatbuff(name, str, n);
+                    strlncatbuff(name, str, sizeof(name_), n);
 
                     if (strncmp(name, "/*", 2) == 0) {
                       /* comments */
@@ -1490,7 +1491,7 @@ int smallserver_setkeyarr(const char *key, int id, const char *key2, const char
   return coucal_write(NewLangList, tmp, (intptr_t) strdup(value));
 }
 
-static int htslang_load(char *limit_to, const char *path) {
+static int htslang_load(char *limit_to, size_t limit_size, const char *path) {
   const char *hashname;
   char catbuff[CATBUFF_SIZE];
 
@@ -1545,7 +1546,7 @@ static int htslang_load(char *limit_to, const char *path) {
             char *const buff = (char *) malloc(len + 1);
 
             if (buff) {
-              strcpybuff(buff, intkey);
+              strlcpybuff(buff, intkey, len + 1);
               coucal_add(NewLangStrKeys, key, (intptr_t) buff);
             }
           }
@@ -1568,9 +1569,9 @@ static int htslang_load(char *limit_to, const char *path) {
   /* Get only language name */
   if (limit_to) {
     if (hashname)
-      strcpybuff(limit_to, hashname);
+      strlcpybuff(limit_to, hashname, limit_size);
     else
-      strcpybuff(limit_to, "???");
+      strlcpybuff(limit_to, "???", limit_size);
     return 0;
   }
 
@@ -1750,7 +1751,7 @@ static void LANG_INIT(const char *path) {
 static int LANG_T(const char *path, int l) {
   if (l >= 0) {
     QLANG_T(l);
-    htslang_load(NULL, path);
+    htslang_load(NULL, 0, path);
   }
   return QLANG_T(-1);           // 0=default (english)
 }
@@ -1764,7 +1765,7 @@ static int LANG_SEARCH(const char *path, const char *iso) {
   do {
     QLANG_T(i);
     strcpybuff(lang_str, "LANGUAGE_ISO");
-    htslang_load(lang_str, path);
+    htslang_load(lang_str, sizeof(lang_str), path);
     if (strfield(iso, lang_str)) {
       found = i;
     }
@@ -1782,11 +1783,11 @@ static int LANG_LIST(const char *path, char *buffer, size_t buffer_size) {
   buffer[0] = '\0';
   do {
     QLANG_T(i);
-    strlcpybuff(lang_str, "LANGUAGE_NAME", buffer_size);
-    htslang_load(lang_str, path);
+    strlcpybuff(lang_str, "LANGUAGE_NAME", sizeof(lang_str));
+    htslang_load(lang_str, sizeof(lang_str), path);
     if (strlen(lang_str) > 0) {
       if (buffer[0])
-        strcatbuff(buffer, "\n");
+        strlcatbuff(buffer, "\n", buffer_size);
       strlcatbuff(buffer, lang_str, buffer_size);
     }
     i++;