fix: pass artifact paths to rsync via --files-from (#1825)

Author: Paul-Goulpie

src/job.ts

@@ -1424,16 +1424,18 @@ If you know what you're doing and would like to suppress this warning, use one o
         let time, endTime;
         let cpCmd = "shopt -s globstar nullglob dotglob\n";
         cpCmd += `mkdir -p ${artifactsPath}/${safeJobName}\n`;
-        cpCmd += "rsync --exclude '.gitlab-ci-local/**' -Ra ";
+        cpCmd += "_gcl_files_tmp=\\$(mktemp)\n";
+        for (const artifactPath of this.artifacts?.paths ?? []) {
+            const expandedPath = Utils.expandText(artifactPath, expanded).replace(`${expanded.CI_PROJECT_DIR}/`, "");
+            cpCmd += `for _gcl_f in ./${expandedPath}; do printf '%s\\n' "\\$_gcl_f"; done >> \\$_gcl_files_tmp\n`;
+        }
+        cpCmd += "rsync --exclude '.gitlab-ci-local/**' -rRa ";
         for (const artifactExcludePath of this.artifacts?.exclude ?? []) {
             const expandedPath = Utils.expandText(artifactExcludePath, expanded).replace(`${expanded.CI_PROJECT_DIR}/`, "");
             cpCmd += `--exclude '${expandedPath}' `;
         }
-        for (const artifactPath of this.artifacts?.paths ?? []) {
-            const expandedPath = Utils.expandText(artifactPath, expanded).replace(`${expanded.CI_PROJECT_DIR}/`, "");
-            cpCmd += `./${expandedPath} `;
-        }
-        cpCmd += `${artifactsPath}/${safeJobName}/. || true\n`;
+        cpCmd += `--files-from=\\$_gcl_files_tmp . ${artifactsPath}/${safeJobName}/. || true\n`;
+        cpCmd += "rm -f \\$_gcl_files_tmp\n";
         const reportDotenv = Utils.expandText(this.artifacts.reports?.dotenv ?? null, expanded);
         const reportDotenvs: string[] | null = (typeof reportDotenv === "string") ? // normalize to string[] for easier handling
             [reportDotenv] :

tests/test-cases/artifacts-many-paths/.gitignore

@@ -0,0 +1,3 @@
+/artifact_*
+/output_dir/
+/.gitlab-ci-local-many-paths/

tests/test-cases/artifacts-many-paths/.gitlab-ci.yml

@@ -0,0 +1,30 @@
+---
+# Creates 9000 files whose names are 255 characters long.
+# With the old approach (paths passed as rsync arguments), the total argument
+# size (~2.3 MB) exceeded ARG_MAX (~2 MB) and rsync failed silently.
+# With the new approach (--files-from + bash for loop), there is no limit on
+# the number of files.
+produce-artifacts:
+  stage: build
+  script:
+    - |
+      python3 -c "
+      import os
+      padding = 'x' * 240
+      for i in range(9000):
+          name = f'artifact_{i:05d}_{padding}'
+          fd = os.open(name, os.O_CREAT | os.O_WRONLY, 0o644)
+          os.close(fd)
+      "
+    - mkdir -p output_dir && echo "hello" > output_dir/result.txt
+  artifacts:
+    paths:
+      - artifact_*
+      - output_dir/
+
+consume-artifacts:
+  stage: test
+  script:
+    - count=$(find . -maxdepth 1 -name 'artifact_*' | wc -l)
+    - if [ "$count" -ne 9000 ]; then echo "FAIL expected 9000 artifacts, got $count"; exit 1; fi
+    - if [ ! -f output_dir/result.txt ]; then echo "FAIL output_dir/result.txt not found"; exit 1; fi

tests/test-cases/artifacts-many-paths/integration.test.ts

@@ -0,0 +1,25 @@
+import {WriteStreamsMock} from "../../../src/write-streams.js";
+import {handler} from "../../../src/handler.js";
+import {initSpawnSpy} from "../../mocks/utils.mock.js";
+import {WhenStatics} from "../../mocks/when-statics.js";
+
+beforeAll(() => {
+    initSpawnSpy(WhenStatics.all);
+});
+
+// Validates that --files-from works correctly when the number of artifact files
+// would exceed ARG_MAX if their paths were passed directly as rsync arguments
+// (9000 files × ~257 bytes ≈ 2.3 MB > ARG_MAX ~2 MB).
+test.concurrent("artifacts-many-paths --shell-isolation 9000 artifacts via --files-from", async () => {
+    const writeStreams = new WriteStreamsMock();
+    await handler({
+        cwd: "tests/test-cases/artifacts-many-paths",
+        shellIsolation: true,
+        stateDir: ".gitlab-ci-local-many-paths",
+    }, writeStreams);
+
+    // Ensures artifacts were actually exported (not silently swallowed by || true).
+    expect(writeStreams.stdoutLines.join("\n")).toMatch(/produce-artifacts.*exported artifacts/);
+    // Ensures consume-artifacts found all 9000 expected files.
+    expect(writeStreams.stderrLines.join("\n")).not.toMatch(/FAIL/);
+}, 120_000);