fix(gitlab): harden API redirects and CSRF compatibility

src/usr/local/emhttp/plugins/ci-runner-farm/include/exec.php

@@ -19,15 +19,17 @@
   ? $var : @parse_ini_file('/var/local/emhttp/var.ini');
 $csrf = is_array($csrfState) && is_string($csrfState['csrf_token'] ?? null)
   ? $csrfState['csrf_token'] : '';
+$platformCsrfTokenPresent = array_key_exists('csrf_token', get_defined_vars());
+$platformCsrfTokenMatches = !$platformCsrfTokenPresent
+  || (is_string($csrf_token) && hash_equals($csrf, $csrf_token));
 $platformCsrfValidated = function_exists('csrf_terminate')
-  && isset($csrf_token) && is_string($csrf_token)
   && $csrf !== ''
-  // Unraid 7.3's auto_prepend_file validates either transport, then unsets both
-  // before this endpoint runs. Requiring that consumed state prevents a stray
-  // same-named variable from bypassing the standalone fallback below.
+  // Unraid's auto_prepend_file validates either transport, then consumes both
+  // keys before this endpoint runs. Releases through 7.2 do not retain a local
+  // $csrf_token; 7.3 does. If the platform supplies one, it must still match.
   && !array_key_exists('csrf_token', $_POST)
   && !array_key_exists('HTTP_X_CSRF_TOKEN', $_SERVER)
-  && hash_equals($csrf, $csrf_token);
+  && $platformCsrfTokenMatches;
 if (!$platformCsrfValidated) {
   // Standalone/CLI fallback for tests and any host that does not use Unraid's
   // local_prepend.php. On Unraid, a missing or incorrect token has already been

src/usr/local/emhttp/plugins/ci-runner-farm/include/providers/gitlab.sh

@@ -302,26 +302,74 @@ gitlab_api_token_ready() {
     && printf '%s' "$GITLAB_API_TOKEN" | grep -qE '^[A-Za-z0-9_.@:+/=~-]+$'
 }
 
-gitlab_api() {
-  local method="$1" path="$2" ca=()
+# Deliberately no --location. curl strips only Authorization on a cross-host
+# redirect, so a followed 3xx would resend PRIVATE-TOKEN to whatever host the
+# instance names. These are plain /api/v4 requests against the configured base
+# URL and have no legitimate reason to leave it. `-q` must be curl's first
+# argument so an operator's ~/.curlrc cannot turn redirects back on.
+gitlab_api_request() {
+  local method="$1" path="$2" body="$3" headers="$4" status ca=()
   gitlab_validate_url >/dev/null 2>&1 || return 1
   gitlab_api_token_ready || return 1
   [ -f "$GITLAB_CA_FILE" ] && ca=( --cacert "$GITLAB_CA_FILE" )
-  printf 'header = "PRIVATE-TOKEN: %s"\n' "$GITLAB_API_TOKEN" \
-    | curl -fsSL -g -m 12 -X "$method" --config - \
-      -H 'Accept: application/json' ${ca[@]+"${ca[@]}"} \
-      "$(gitlab_url)/api/v4${path}" 2>/dev/null
+  : > "$body" && : > "$headers" || return 1
+  status="$(
+    printf 'header = "PRIVATE-TOKEN: %s"\n' "$GITLAB_API_TOKEN" \
+      | curl -q -fsS -g -m 12 -X "$method" --config - \
+        -H 'Accept: application/json' ${ca[@]+"${ca[@]}"} \
+        -D "$headers" -o "$body" -w '%{http_code}' \
+        "$(gitlab_url)/api/v4${path}" 2>/dev/null
+  )" || return 1
+  case "$status" in 2[0-9][0-9]) return 0 ;; *) return 1 ;; esac
+}
+
+gitlab_api() {
+  local method="$1" path="$2" tmpdir body headers rc=1
+  tmpdir="$(mktemp -d "$RUNDIR/gitlab-api.XXXXXX" 2>/dev/null)" || return 1
+  body="$tmpdir/body"; headers="$tmpdir/headers"
+  if gitlab_api_request "$method" "$path" "$body" "$headers"; then
+    cat "$body"; rc=$?
+  fi
+  rm -rf -- "$tmpdir"
+  return "$rc"
 }
 
 gitlab_api_capture() {
-  local path="$1" body="$2" headers="$3" ca=()
-  gitlab_validate_url >/dev/null 2>&1 || return 1
-  gitlab_api_token_ready || return 1
-  [ -f "$GITLAB_CA_FILE" ] && ca=( --cacert "$GITLAB_CA_FILE" )
-  printf 'header = "PRIVATE-TOKEN: %s"\n' "$GITLAB_API_TOKEN" \
-    | curl -fsSL -g -m 12 --config - -H 'Accept: application/json' \
-      ${ca[@]+"${ca[@]}"} -D "$headers" -o "$body" \
-      "$(gitlab_url)/api/v4${path}" 2>/dev/null
+  gitlab_api_request GET "$1" "$2" "$3"
+}
+
+# Split the operator-entered monitored-project list without pathname expansion:
+# an entry such as group/* is a literal telemetry path, never a filesystem glob,
+# and `for p in $GITLAB_PROJECTS` would expand it against the process CWD. Only
+# well-formed namespace/project paths are emitted; anything else is advisory
+# input that cannot address a real project, so it is skipped rather than
+# blocking the fleet on a telemetry-only field.
+gitlab_project_path_valid() {
+  local path="$1" segment
+  local -a segments=()
+  case "$path" in ''|/*|*/|*'//'*) return 1 ;; esac
+  IFS='/' read -r -a segments <<< "$path"
+  [ "${#segments[@]}" -ge 2 ] || return 1
+  for segment in "${segments[@]}"; do
+    case "$segment" in ''|.|..|-*) return 1 ;; esac
+    printf '%s' "$segment" \
+      | grep -qE '^[A-Za-z0-9_.][A-Za-z0-9._-]*$' || return 1
+  done
+}
+
+gitlab_projects_list() {
+  local -a items=()
+  local item normalized
+  # `read -a` consumes only one physical line. Normalize newlines to another
+  # default-IFS character first so the complete setting retains the historical
+  # space/tab/newline word-list behavior without ever enabling pathname globbing.
+  normalized="${GITLAB_PROJECTS//$'\n'/ }"
+  read -r -a items <<< "$normalized"
+  [ "${#items[@]}" -gt 0 ] || return 0
+  for item in "${items[@]}"; do
+    gitlab_project_path_valid "$item" || continue
+    printf '%s\n' "$item"
+  done
 }
 
 gitlab_public_repo_problem() {
@@ -333,13 +381,13 @@ gitlab_public_repo_problem() {
   [ "$DIND" != "true" ] && [ "$SHARE_DOCKER_SOCK" = "true" ] \
     && msg="GitLab host-socket mode gives every accepted job root-equivalent control of this Unraid host. Restrict the runner in GitLab to trusted, protected projects and refs; isolated DinD is the safer default."
   if gitlab_api_token_ready && [ -n "$GITLAB_PROJECTS" ]; then
-    for project in $GITLAB_PROJECTS; do
+    while IFS= read -r project; do
       [ -n "$project" ] || continue
       encoded="$(urlencode "$project")"
       body="$(gitlab_api GET "/projects/$encoded" 2>/dev/null)"
       vis="$(printf '%s' "$body" | grep -o '"visibility"[[:space:]]*:[[:space:]]*"[^"]*"' | head -1 | sed 's/.*"\([^"]*\)"$/\1/')"
       [ "$vis" = public ] && pub="$pub $project"
-    done
+    done <<< "$(gitlab_projects_list)"
     [ -n "$pub" ] && msg="PUBLIC GitLab project(s) monitored while jobs can control a Docker daemon:${pub}. Untrusted merge-request code can control that slot's job, helper, and service containers; the privileged DinD sidecar is not a host security boundary. Use protected/trusted projects and runner policies. Host-socket mode directly exposes the Unraid Docker daemon. Monitored projects are advisory and do not define the runner's scope."
   fi
   security_cache_put "$msg"
@@ -1489,7 +1537,7 @@ gitlab_queued_refresh() {
   local total=0 got=0 failed=0 project encoded tmpd body headers n
   tmpd="$(mktemp -d 2>/dev/null)"
   [ -n "$tmpd" ] || { echo "gitlab $(date +%s) -1" > "$RUNDIR/queued.cache"; return 0; }
-  for project in $GITLAB_PROJECTS; do
+  while IFS= read -r project; do
     [ -n "$project" ] || continue
     encoded="$(urlencode "$project")"; body="$tmpd/body"; headers="$tmpd/headers"
     : > "$body"; : > "$headers"
@@ -1501,7 +1549,7 @@ gitlab_queued_refresh() {
     else
       failed=1
     fi
-  done
+  done <<< "$(gitlab_projects_list)"
   rm -rf "$tmpd"
   [ "$got" = 1 ] && [ "$failed" = 0 ] || total=-1
   echo "gitlab $(date +%s) $total" > "$RUNDIR/queued.cache"
@@ -1513,7 +1561,9 @@ gitlab_stats_refresh() {
     echo "gitlab $(date +%s) 0 0 0 0 -1" > "$RUNDIR/stats.cache"; return 0
   fi
   local ok=0 fail=0 cancel=0 other=0 total got=0 failed=0 project encoded body status
-  for project in $GITLAB_PROJECTS; do
+  # Read the project list on fd 3: the per-project status scan below is itself a
+  # `while read` and would otherwise share this loop's stdin.
+  while IFS= read -r project <&3; do
     [ -n "$project" ] || continue
     encoded="$(urlencode "$project")"
     if body="$(gitlab_api GET "/projects/$encoded/jobs?per_page=50&order_by=id&sort=desc")"; then
@@ -1536,7 +1586,7 @@ gitlab_stats_refresh() {
     else
       failed=1
     fi
-  done
+  done 3<<< "$(gitlab_projects_list)"
   if [ "$got" = 1 ] && [ "$failed" = 0 ]; then total=$((ok+fail+cancel+other)); else total=-1; fi
   echo "gitlab $(date +%s) $ok $fail $cancel $other $total" > "$RUNDIR/stats.cache"
 }

tests/exec-csrf.sh

@@ -1,8 +1,9 @@
 #!/usr/bin/env bash
-# Exercise both CSRF paths in exec.php. Unraid 7.3 validates and consumes the
-# request token in auto_prepend_file before the endpoint runs; CLI/tests need the
-# endpoint's standalone fallback instead. Every action input below is invalid so
-# this test can never write the real /boot credential path.
+# Exercise every CSRF path in exec.php. Unraid validates and consumes the request
+# token in auto_prepend_file before the endpoint runs; releases through 7.2 do
+# not retain a local token variable, while 7.3 does. CLI/tests need the endpoint's
+# standalone fallback instead. Every action below is invalid, so this test can
+# never write the real /boot credential path.
 set -euo pipefail
 cd "$(dirname "$0")/.."
 
@@ -78,10 +79,18 @@ if ($csrfCase === 'platform') {
   $csrf_token = 'known-test-token';
   // This is the state Unraid 7.3 leaves after successful prevalidation.
   $_POST = ['action'=>'set-gitlab-runner-token', 'token'=>$token];
+} elseif ($csrfCase === 'platform-legacy') {
+  function csrf_terminate($reason) {}
+  // This is the state Unraid 6.12 through 7.2 leave after prevalidation.
+  $_POST = ['action'=>'set-gitlab-runner-token', 'token'=>$token];
 } elseif ($csrfCase === 'platform-spoof') {
   function csrf_terminate($reason) {}
   $csrf_token = 'wrong-test-token';
   $_POST = ['action'=>'set-gitlab-runner-token', 'token'=>$token];
+} elseif ($csrfCase === 'platform-nonscalar') {
+  function csrf_terminate($reason) {}
+  $csrf_token = ['known-test-token'];
+  $_POST = ['action'=>'set-gitlab-runner-token', 'token'=>$token];
 } elseif ($csrfCase === 'standalone') {
   $_POST = [
     'csrf_token'=>'known-test-token',
@@ -123,7 +132,7 @@ assert_action_error() {
 
 check_token_case() {
   local token_case="$1" expected_code="$2" expected_message="$3" csrf_case actual
-  for csrf_case in platform standalone; do
+  for csrf_case in platform platform-legacy standalone; do
     actual="$(run_case "$csrf_case" "$token_case")" \
       || fail "$csrf_case/$token_case endpoint invocation failed"
     assert_action_error "$csrf_case/$token_case" "$actual" "$expected_code" "$expected_message"
@@ -157,6 +166,8 @@ check_token_case non-scalar runner_token_prefix "$prefix_message"
 csrf_error='{"ok":false,"error":"csrf"}'
 [ "$(run_case platform-spoof prefix)" = "$csrf_error" ] \
   || fail "spoofed platform CSRF variable bypassed validation"
+[ "$(run_case platform-nonscalar prefix)" = "$csrf_error" ] \
+  || fail "non-scalar platform CSRF variable bypassed validation"
 [ "$(run_case standalone-bad prefix)" = "$csrf_error" ] \
   || fail "standalone invalid CSRF token bypassed validation"
 

tests/gitlab-policy.sh

@@ -149,4 +149,29 @@ do
   [ "$(crf_confgen)" != "$baseline" ] || fail "confgen ignores $key"
 done
 
-echo "gitlab-policy: OK — executor allowlists, pull policy, shm size, validation, and confgen are wired"
+# The monitored-project list is advisory operator text, not a shell word list.
+# An entry such as group/* must stay a literal telemetry path: iterating it
+# unquoted would expand it against the process CWD and silently query whatever
+# directory names happened to match. Malformed entries cannot address a real
+# project, so they are dropped rather than blocking a fleet on telemetry input.
+projects_probe="$tmp/projects-probe"
+mkdir -p "$projects_probe/group/decoy-match"
+GITLAB_PROJECTS='group/proj group/* ../escape group/../escape bare /absolute group/trailing/ group//double -group/project group/-project group/sub/proj _group/_project .group/.project'
+( cd "$projects_probe" && gitlab_projects_list ) > "$tmp/projects.out" \
+  || fail "monitored-project list could not be enumerated"
+expected="group/proj
+group/sub/proj
+_group/_project
+.group/.project"
+[ "$(cat "$tmp/projects.out")" = "$expected" ] \
+  || fail "monitored-project list did not preserve valid paths and drop glob/relative/bare entries safely"
+GITLAB_PROJECTS=$'group/one\ngroup/two\tgroup/three\n_group/.project'
+[ "$(gitlab_projects_list)" = "group/one
+group/two
+group/three
+_group/.project" ] \
+  || fail "newline/tab-delimited monitored projects were truncated or parsed incorrectly"
+GITLAB_PROJECTS=''
+[ -z "$(gitlab_projects_list)" ] || fail "an empty monitored-project list emitted entries"
+
+echo "gitlab-policy: OK — executor allowlists, pull policy, shm size, monitored projects, validation, and confgen are wired"

tests/provider-mocks.sh

@@ -724,7 +724,7 @@ REGISTRY_TOKEN=''
 export MOCK_CURL_ARGS="$tmp/curl.argv"
 : > "$MOCK_CURL_ARGS"
 curl() {
-  local headers='' output='' url='' arg input
+  local headers='' output='' url='' arg input code="${MOCK_CURL_STATUS:-200}"
   printf '%s\n' "$*" >> "$MOCK_CURL_ARGS"
   input="$(cat)"
   case "$input" in 'header = "PRIVATE-TOKEN: '*'"') ;; *) return 22 ;; esac
@@ -736,27 +736,77 @@ curl() {
       http*) url="$arg" ;;
     esac
   done
-  if [ -n "$headers" ]; then
-    printf 'HTTP/2 200\r\nx-total: 3\r\n\r\n' > "$headers"
-    printf '[{"id":101,"status":"pending","pipeline":{"id":9},"runner":{"id":4}}]' > "$output"
+  [ -n "$headers" ] && printf 'HTTP/2 %s\r\n' "$code" > "$headers"
+  if [ "$code" != 200 ]; then
+    printf 'location: https://redirect.invalid/token-catcher\r\n\r\n' >> "$headers"
+    printf '%s' '{"visibility":"public","id":999,"status":"success"}' > "$output"
+    printf '%s' "$code"
+    return 0
+  fi
+  if printf '%s' "$url" | grep -q 'scope%5B%5D=pending'; then
+    if printf '%s' "$url" | grep -q 'group%2Fsub%2Fproject'; then
+      printf 'x-total: 2\r\n\r\n' >> "$headers"
+    else
+      printf 'x-total: 3\r\n\r\n' >> "$headers"
+    fi
+    printf '%s' '[{"id":101,"status":"pending","pipeline":{"id":9},"runner":{"id":4}}]' > "$output"
   elif printf '%s' "$url" | grep -q '/jobs?per_page=50'; then
-    printf '%s' '[{"id":1,"status":"success","pipeline":{"id":2,"status":"failed"},"runner":{"id":3,"status":"online"}},{"id":4,"status":"failed","pipeline":{"id":5,"status":"success"},"runner":{"id":6,"status":"offline"}}]'
+    if printf '%s' "$url" | grep -q 'group%2Fsub%2Fproject'; then
+      printf '%s' '[{"id":7,"status":"canceled","pipeline":{"id":8,"status":"success"},"runner":{"id":9,"status":"online"}},{"id":10,"status":"skipped","pipeline":{"id":11,"status":"failed"},"runner":{"id":12,"status":"offline"}}]' > "$output"
+    else
+      printf '%s' '[{"id":1,"status":"success","pipeline":{"id":2,"status":"failed"},"runner":{"id":3,"status":"online"}},{"id":4,"status":"failed","pipeline":{"id":5,"status":"success"},"runner":{"id":6,"status":"offline"}}]' > "$output"
+    fi
   else
-    printf '%s' '{"visibility":"private"}'
+    if printf '%s' "$url" | grep -q 'group%2Fsub%2Fproject'; then
+      printf '%s' '{"visibility":"public"}' > "$output"
+    else
+      printf '%s' '{"visibility":"private"}' > "$output"
+    fi
   fi
+  printf '%s' "$code"
 }
 
 GITLAB_API_TOKEN='custom@prefix-token_1234567890'
-GITLAB_PROJECTS='group/project'
+GITLAB_PROJECTS='group/project group/sub/project'
 gitlab_queued_refresh
 read -r qprovider _ qcount < "$CRF_RUNDIR/queued.cache"
-[ "$qprovider" = gitlab ] && [ "$qcount" = 3 ] || fail "GitLab queue pagination mapping is wrong"
+[ "$qprovider" = gitlab ] && [ "$qcount" = 5 ] || fail "GitLab multi-project queue pagination mapping is wrong"
 gitlab_stats_refresh
 read -r sprovider _ sok sfail scancel sother stotal < "$CRF_RUNDIR/stats.cache"
 [ "$sprovider" = gitlab ] && [ "$sok" = 1 ] && [ "$sfail" = 1 ] \
-  && [ "$scancel" = 0 ] && [ "$sother" = 0 ] && [ "$stotal" = 2 ] \
-  || fail "GitLab stats counted nested status fields"
+  && [ "$scancel" = 1 ] && [ "$sother" = 1 ] && [ "$stotal" = 4 ] \
+  || fail "GitLab multi-project stats aggregation counted nested status fields"
+rm -f "$SECURITY_CACHE"
+public_warning="$(gitlab_public_repo_problem)"
+printf '%s' "$public_warning" | grep -qF 'group/sub/project' \
+  || fail "GitLab public-project warning did not inspect every monitored project"
 if grep -qF "$GITLAB_API_TOKEN" "$MOCK_CURL_ARGS"; then fail "API token leaked into curl argv"; fi
+while IFS= read -r curl_args; do
+  read -r -a curl_argv <<< "$curl_args"
+  [ "${curl_argv[0]:-}" = -q ] || fail "GitLab API curl did not disable curlrc first"
+  for curl_arg in "${curl_argv[@]}"; do
+    case "$curl_arg" in
+      --location|--location-trusted|--follow) fail "GitLab API curl can follow a redirect" ;;
+      --*) ;;
+      -*) case "${curl_arg#-}" in *L*) fail "GitLab API curl can follow a redirect" ;; esac ;;
+    esac
+  done
+done < "$MOCK_CURL_ARGS"
+
+# curl's fail mode treats 3xx as success. Every redirect status must therefore
+# be rejected explicitly so a redirect body cannot become dashboard data and
+# the custom PRIVATE-TOKEN can never reach its Location target.
+for redirect_code in 300 301 302 303 307 308 399; do
+  MOCK_CURL_STATUS="$redirect_code"
+  gitlab_queued_refresh
+  read -r _ _ qcount < "$CRF_RUNDIR/queued.cache"
+  [ "$qcount" = -1 ] || fail "GitLab $redirect_code queue redirect was treated as API data"
+  gitlab_stats_refresh
+  read -r _ _ _ _ _ _ stotal < "$CRF_RUNDIR/stats.cache"
+  [ "$stotal" = -1 ] || fail "GitLab $redirect_code stats redirect was treated as API data"
+done
+unset MOCK_CURL_STATUS
+
 GITLAB_API_TOKEN=''
 gitlab_queued_refresh
 read -r _ _ qcount < "$CRF_RUNDIR/queued.cache"