Skip to content

Commit 1206342

Browse files
gyanranjanGyan Ranjan Afirecow
authored
fix: prevent ENAMETOOLONG crash for long parallel:matrix job names (#1865)
Co-authored-by: Gyan Ranjan A <gyan.a.ranjan@ericsson.com> Co-authored-by: Mads Jon Nielsen <madsjon@gmail.com>
1 parent 5e5c4ac commit 1206342

5 files changed

Lines changed: 107 additions & 1 deletion

File tree

src/utils.ts

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import checksum from "checksum";
88
import base64url from "base64url";
99
import execa, {ExecaError} from "execa";
1010
import assert from "node:assert";
11+
import {createHash} from "node:crypto";
1112
import {CICDVariable} from "./variables-from-files.js";
1213
import {GitData} from "./git-data.js";
1314
import {globbySync} from "globby";
@@ -49,10 +50,22 @@ export class Utils {
4950
return url.replace(/^https:\/\//g, "").replace(/^http:\/\//g, "");
5051
}
5152

53+
// gcl-${safeJobName}-${jobId}-build → wrapper is 17 chars (jobId max 6 digits)
54+
static readonly MAX_FILENAME_LENGTH = 255 - 17; // NAME_MAX (bytes) - wrapper
55+
5256
static safeDockerString (jobName: string) {
53-
return jobName.replace(/[^\w-]+/g, (match) => {
57+
// INVARIANT: \w without /u is ASCII-only ([A-Za-z0-9_]), so `encoded` is pure ASCII
58+
// and .length === byte length. NAME_MAX is a byte limit — adding /u would break this.
59+
// We hash `jobName` (not `encoded`) because base64url encoding isn't injective.
60+
const encoded = jobName.replace(/[^\w-]+/g, (match) => {
5461
return base64url.encode(match);
5562
});
63+
if (encoded.length <= Utils.MAX_FILENAME_LENGTH) {
64+
return encoded;
65+
}
66+
const hash = createHash("sha256").update(jobName).digest("hex").substring(0, 16);
67+
const prefix = encoded.substring(0, Utils.MAX_FILENAME_LENGTH - 1 - hash.length);
68+
return `${prefix}-${hash}`;
5669
}
5770

5871
static safeBashString (s: string) {
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
.gitlab-ci-local-*
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
---
2+
build-job:
3+
stage: build
4+
script:
5+
- echo "APP='${APP}' $CI_NODE_INDEX/$CI_NODE_TOTAL"
6+
parallel:
7+
matrix:
8+
- APP:
9+
- "my-app-controller,My app controller to be used as reference for development teams,python311,\
10+
controller,common,controller/setup.py,controller/setup_c.py,controller/setup_n.py,controller/tests/**/*,\
11+
controller/coverage/*,controller/build/**/*,controller/coverage/coverage-unit.xml,75,true"
12+
- short
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
import {WriteStreamsMock} from "../../../src/write-streams.js";
2+
import {handler} from "../../../src/handler.js";
3+
4+
test.concurrent("parallel-matrix-long-name - completes without ENAMETOOLONG", async () => {
5+
const writeStreams = new WriteStreamsMock();
6+
await handler({
7+
cwd: "tests/test-cases/parallel-matrix-long-name",
8+
shellIsolation: true,
9+
stateDir: ".gitlab-ci-local-parallel-matrix-long-name",
10+
}, writeStreams);
11+
12+
// Both matrix entries must complete — the long one would crash with ENAMETOOLONG before the fix
13+
const passing = writeStreams.stdoutLines.filter(l => l.includes(" PASS "));
14+
expect(passing.length).toBe(2);
15+
expect(writeStreams.stdoutLines.some(l => l.includes("build-job: [short]"))).toBe(true);
16+
expect(writeStreams.stdoutLines.some(l => l.includes("build-job: [my-app-controller,"))).toBe(true);
17+
});

tests/utils.test.ts

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -266,3 +266,66 @@ describe("getServiceAlias", () => {
266266
expect(Utils.getServiceAlias({...base, name: "library/nginx", alias: "my-nginx"})).toBe("my-nginx");
267267
});
268268
});
269+
270+
describe("safeDockerString", () => {
271+
it("should return encoded name unchanged when within limit", () => {
272+
const result = Utils.safeDockerString("short-job-name");
273+
expect(result).toBe("short-job-name");
274+
});
275+
276+
it("should encode non-alphanumeric characters", () => {
277+
const result = Utils.safeDockerString("job/name");
278+
expect(result).toBe("jobLwname"); // '/' → 'Lw'
279+
});
280+
281+
it("should truncate and hash when encoded name exceeds MAX_FILENAME_LENGTH", () => {
282+
const longName = "my-group/common/python-unit-test: [my-app-controller,My app controller to be used as reference for development teams,python311,controller,common,controller/setup.py,controller/setup_c.py,controller/setup_n.py,controller/tests/**/*,controller/coverage/*,controller/build/**/*,controller/coverage/coverage-unit.xml,75,true]";
283+
const result = Utils.safeDockerString(longName);
284+
expect(result.length).toBeLessThanOrEqual(Utils.MAX_FILENAME_LENGTH);
285+
});
286+
287+
it("should produce deterministic output for the same input", () => {
288+
const longName = "a".repeat(50) + "/" + "b".repeat(200);
289+
const result1 = Utils.safeDockerString(longName);
290+
const result2 = Utils.safeDockerString(longName);
291+
expect(result1).toBe(result2);
292+
});
293+
294+
it("should produce different output for different long inputs", () => {
295+
const name1 = "job: [" + "a".repeat(300) + "]";
296+
const name2 = "job: [" + "b".repeat(300) + "]";
297+
const result1 = Utils.safeDockerString(name1);
298+
const result2 = Utils.safeDockerString(name2);
299+
expect(result1).not.toBe(result2);
300+
});
301+
302+
it("should handle extremely long job names (1000+ chars)", () => {
303+
const extremeName = "group/subgroup/job: [" + "x".repeat(2000) + "]";
304+
const result = Utils.safeDockerString(extremeName);
305+
expect(result.length).toBeLessThanOrEqual(Utils.MAX_FILENAME_LENGTH);
306+
expect(result.length).toBeGreaterThan(16); // has prefix + hash
307+
});
308+
309+
it("should keep volume name within NAME_MAX=255 (worst-case suffix)", () => {
310+
const longName = "my-group/common/python-unit-test: [" + "a/b/c,".repeat(100) + "]";
311+
const safeJobName = Utils.safeDockerString(longName);
312+
const worstCaseVolume = `gcl-${safeJobName}-999999-build`;
313+
expect(worstCaseVolume.length).toBeLessThanOrEqual(255);
314+
});
315+
316+
it("should not hash names that are exactly at the limit", () => {
317+
// Create a name whose encoded form is exactly MAX_FILENAME_LENGTH
318+
const name = "a".repeat(Utils.MAX_FILENAME_LENGTH);
319+
const result = Utils.safeDockerString(name);
320+
expect(result).toBe(name); // all alphanumeric, no encoding, no hash
321+
});
322+
323+
it("should hash names whose encoded form is one char over the limit", () => {
324+
// 'a' stays as 'a', '/' encodes to 'Lw' (2 chars)
325+
// Build a string that encodes to exactly MAX_FILENAME_LENGTH + 1
326+
const name = "a".repeat(Utils.MAX_FILENAME_LENGTH - 1) + "/"; // '/' -> 'Lw' = +2, total = MAX+1
327+
const result = Utils.safeDockerString(name);
328+
expect(result.length).toBeLessThanOrEqual(Utils.MAX_FILENAME_LENGTH);
329+
expect(result).toContain("-"); // has hash separator
330+
});
331+
});

0 commit comments

Comments
 (0)