fix: restore .env("GCL") and strip GCL_VARIABLE_* before yargs parse

Keep yargs .env("GCL") + .strictOptions() for standard options so that
validation, coercion, precedence, and unknown-option rejection all work
as before. Strip GCL_VARIABLE_* entries from process.env before yargs
sees them (avoiding strictOptions conflict), then inject them into
argv.variable in the handler.

Author: firecow

src/argv.ts

@@ -22,58 +22,29 @@ async function gitRootPath () {
     return stdout;
 }
 
-export function injectGclVariableEnvVars (argv: {variable?: string[]; [key: string]: any}, env: Record<string, string | undefined>): void {
+export function stripGclVariableEnvVars (env: Record<string, string | undefined>): Record<string, string> {
     const prefix = "GCL_VARIABLE_";
-    for (const [envKey, envValue] of Object.entries(env)) {
-        if (!envKey.startsWith(prefix) || envValue == null) continue;
+    const stripped: Record<string, string> = {};
+    for (const key of Object.keys(env)) {
+        if (!key.startsWith(prefix) || env[key] == null) continue;
+        if (key.length <= prefix.length) continue;
+        stripped[key] = env[key]!;
+        delete env[key];
+    }
+    return stripped;
+}
+
+export function injectGclVariableEnvVars (argv: {variable?: string[]; [key: string]: any}, gclVars: Record<string, string>): void {
+    const prefix = "GCL_VARIABLE_";
+    for (const [envKey, envValue] of Object.entries(gclVars)) {
         const varName = envKey.slice(prefix.length);
-        if (varName.length === 0) continue;
         if (argv.variable == null) {
             argv.variable = [];
         }
         argv.variable.unshift(`${varName}=${envValue}`);
     }
 }
 
-interface YargsOptionsMeta {
-    "array": string[];
-    "boolean": string[];
-    "number": string[];
-    "default": Record<string, any>;
-    "key": Record<string, boolean>;
-}
-
-export function injectGclEnvVars (
-    argv: Record<string, any>,
-    yargsOptions: YargsOptionsMeta,
-    env: Record<string, string | undefined>,
-    defaulted: Record<string, boolean> = {},
-): void {
-    const arrays = new Set(yargsOptions.array.map(String));
-    const booleans = new Set(yargsOptions.boolean.map(String));
-    const numbers = new Set(yargsOptions.number.map(String));
-
-    for (const key of Object.keys(yargsOptions.key)) {
-        if (key.includes("-") || key === "_" || key === "$0") continue;
-
-        const envKey = `GCL_${key.replace(/[A-Z]/g, c => `_${c}`).toUpperCase()}`;
-        const envValue = env[envKey];
-        if (envValue == null) continue;
-
-        if (arrays.has(key)) {
-            const cliValues = Array.isArray(argv[key]) ? argv[key] : [];
-            argv[key] = [...envValue.split(";"), ...cliValues];
-            continue;
-        }
-
-        if (argv[key] !== undefined && !defaulted[key]) continue;
-
-        if (booleans.has(key)) argv[key] = envValue === "true" || envValue === "1";
-        else if (numbers.has(key)) argv[key] = Number(envValue);
-        else argv[key] = envValue;
-    }
-}
-
 export class Argv {
     static readonly default = {
         "variablesFile": ".gitlab-ci-local-variables.yml",

src/index.ts

@@ -6,7 +6,7 @@ import * as state from "./state.js";
 import {WriteStreamsProcess, WriteStreamsMock} from "./write-streams.js";
 import {handler} from "./handler.js";
 import {Executor} from "./executor.js";
-import {Argv, injectGclVariableEnvVars, injectGclEnvVars} from "./argv.js";
+import {Argv, stripGclVariableEnvVars, injectGclVariableEnvVars} from "./argv.js";
 import {AssertionError} from "assert";
 import {Job, cleanupJobResources} from "./job.js";
 import {GitlabRunnerPresetValues} from "./gitlab-preset.js";
@@ -33,6 +33,7 @@ process.on("SIGUSR2", async () => {
 });
 
 (() => {
+    const gclVariableEnvVars = stripGclVariableEnvVars(process.env);
     const yparser = yargs(process.argv.slice(2));
     yparser.parserConfiguration({"greedy-arrays": false})
         .showHelpOnFail(false)
@@ -41,9 +42,7 @@ process.on("SIGUSR2", async () => {
         .command({
             handler: async (argv) => {
                 try {
-                    const defaulted: Record<string, boolean> = (yparser as any).parsed?.defaulted ?? {};
-                    injectGclVariableEnvVars(argv, process.env);
-                    injectGclEnvVars(argv, (yparser as any).getOptions(), process.env, defaulted);
+                    injectGclVariableEnvVars(argv, gclVariableEnvVars);
                     await handler(argv, new WriteStreamsProcess(), jobs);
                     const failedJobs = Executor.getFailed(jobs);
                     process.exit(failedJobs.length > 0 ? 1 : 0);
@@ -77,6 +76,7 @@ process.on("SIGUSR2", async () => {
         })
         .usage("Find more information at https://github.com/firecow/gitlab-ci-local.\nNote: To negate an option use '--no-(option)'.")
         .strictOptions()
+        .env("GCL")
         .option("manual", {
             type: "array",
             description: "One or more manual jobs to run during a pipeline",
@@ -366,9 +366,7 @@ process.on("SIGUSR2", async () => {
                 if (current.startsWith("-")) {
                     completionFilter();
                 } else {
-                    const completionDefaulted: Record<string, boolean> = (yparser as any).parsed?.defaulted ?? {};
-                    injectGclVariableEnvVars(yargsArgv, process.env);
-                    injectGclEnvVars(yargsArgv, (yparser as any).getOptions(), process.env, completionDefaulted);
+                    injectGclVariableEnvVars(yargsArgv, gclVariableEnvVars);
                     Argv.build({...yargsArgv, autoCompleting: true})
                         .then(argv => state.getPipelineIid(argv.cwd, argv.stateDir).then(pipelineIid => ({argv, pipelineIid})))
                         .then(({argv, pipelineIid}) => Parser.create(argv, new WriteStreamsMock(), pipelineIid, []))

tests/test-cases/gcl-variable-env/integration.test.ts

@@ -1,17 +1,56 @@
-import {injectGclVariableEnvVars, injectGclEnvVars} from "../../../src/argv.js";
+import {stripGclVariableEnvVars, injectGclVariableEnvVars} from "../../../src/argv.js";
 import {execFile} from "child_process";
 import {promisify} from "util";
 
 const execFileAsync = promisify(execFile);
 
-describe("injectGclVariableEnvVars unit tests", () => {
-    test("injects single GCL_VARIABLE_ entry", () => {
+describe("stripGclVariableEnvVars", () => {
+    test("strips GCL_VARIABLE_* entries and returns them", () => {
+        const env: Record<string, string | undefined> = {
+            "HOME": "/home/user",
+            "GCL_CWD": "/tmp",
+            "GCL_VARIABLE_MY_VAR": "hello",
+            "GCL_VARIABLE_OTHER": "world",
+        };
+        const stripped = stripGclVariableEnvVars(env);
+        expect(stripped).toEqual({
+            "GCL_VARIABLE_MY_VAR": "hello",
+            "GCL_VARIABLE_OTHER": "world",
+        });
+        expect(env["GCL_VARIABLE_MY_VAR"]).toBeUndefined();
+        expect(env["GCL_VARIABLE_OTHER"]).toBeUndefined();
+        expect(env["HOME"]).toBe("/home/user");
+        expect(env["GCL_CWD"]).toBe("/tmp");
+    });
+
+    test("skips GCL_VARIABLE_ with empty name", () => {
+        const env: Record<string, string | undefined> = {"GCL_VARIABLE_": "empty"};
+        const stripped = stripGclVariableEnvVars(env);
+        expect(stripped).toEqual({});
+        expect(env["GCL_VARIABLE_"]).toBe("empty");
+    });
+
+    test("skips null/undefined values", () => {
+        const env: Record<string, string | undefined> = {"GCL_VARIABLE_FOO": undefined};
+        const stripped = stripGclVariableEnvVars(env);
+        expect(stripped).toEqual({});
+    });
+
+    test("returns empty object when no matches", () => {
+        const env: Record<string, string | undefined> = {"HOME": "/home/user", "GCL_CWD": "/tmp"};
+        const stripped = stripGclVariableEnvVars(env);
+        expect(stripped).toEqual({});
+    });
+});
+
+describe("injectGclVariableEnvVars", () => {
+    test("injects single entry", () => {
         const argv: {variable?: string[]} = {};
         injectGclVariableEnvVars(argv, {"GCL_VARIABLE_MY_VAR": "hello"});
         expect(argv.variable).toEqual(["MY_VAR=hello"]);
     });
 
-    test("injects multiple GCL_VARIABLE_ entries", () => {
+    test("injects multiple entries", () => {
         const argv: {variable?: string[]} = {};
         injectGclVariableEnvVars(argv, {
             "GCL_VARIABLE_VAR1": "one",
@@ -34,28 +73,6 @@ describe("injectGclVariableEnvVars unit tests", () => {
         expect(argv.variable).toEqual(["SAME=from_env", "SAME=from_cli"]);
     });
 
-    test("skips non GCL_VARIABLE_ env vars", () => {
-        const argv: {variable?: string[]} = {};
-        injectGclVariableEnvVars(argv, {
-            "GCL_CWD": "/tmp",
-            "HOME": "/home/user",
-            "GCL_VARIABLE_REAL": "yes",
-        });
-        expect(argv.variable).toEqual(["REAL=yes"]);
-    });
-
-    test("skips GCL_VARIABLE_ with empty name", () => {
-        const argv: {variable?: string[]} = {};
-        injectGclVariableEnvVars(argv, {"GCL_VARIABLE_": "empty_name"});
-        expect(argv.variable).toBeUndefined();
-    });
-
-    test("skips null/undefined env values", () => {
-        const argv: {variable?: string[]} = {};
-        injectGclVariableEnvVars(argv, {"GCL_VARIABLE_FOO": undefined});
-        expect(argv.variable).toBeUndefined();
-    });
-
     test("handles empty value", () => {
         const argv: {variable?: string[]} = {};
         injectGclVariableEnvVars(argv, {"GCL_VARIABLE_FOO": ""});
@@ -75,89 +92,6 @@ describe("injectGclVariableEnvVars unit tests", () => {
     });
 });
 
-describe("injectGclEnvVars unit tests", () => {
-    const baseOptions = {
-        array: ["volume"],
-        boolean: ["quiet"],
-        number: ["concurrency"],
-        default: {quiet: false, concurrency: 0, cwd: "."},
-        key: {quiet: true, concurrency: true, cwd: true, volume: true, _: true, $0: true, "some-kebab": true},
-    };
-
-    test("injects string env var", () => {
-        const argv: Record<string, any> = {cwd: ".", quiet: false};
-        injectGclEnvVars(argv, baseOptions, {"GCL_CWD": "/tmp/test"}, {cwd: true, quiet: true});
-        expect(argv.cwd).toBe("/tmp/test");
-    });
-
-    test("injects boolean env var", () => {
-        const argv: Record<string, any> = {quiet: false};
-        injectGclEnvVars(argv, baseOptions, {"GCL_QUIET": "true"}, {quiet: true});
-        expect(argv.quiet).toBe(true);
-    });
-
-    test("injects boolean env var from '1'", () => {
-        const argv: Record<string, any> = {quiet: false};
-        injectGclEnvVars(argv, baseOptions, {"GCL_QUIET": "1"}, {quiet: true});
-        expect(argv.quiet).toBe(true);
-    });
-
-    test("injects number env var", () => {
-        const argv: Record<string, any> = {concurrency: 0};
-        injectGclEnvVars(argv, baseOptions, {"GCL_CONCURRENCY": "4"}, {concurrency: true});
-        expect(argv.concurrency).toBe(4);
-    });
-
-    test("splits array env var on semicolons", () => {
-        const argv: Record<string, any> = {volume: []};
-        injectGclEnvVars(argv, baseOptions, {"GCL_VOLUME": "/a:/b;/c:/d"}, {volume: true});
-        expect(argv.volume).toEqual(["/a:/b", "/c:/d"]);
-    });
-
-    test("merges array env var with CLI values", () => {
-        const argv: Record<string, any> = {volume: ["/cli:/path"]};
-        injectGclEnvVars(argv, baseOptions, {"GCL_VOLUME": "/env:/path"}, {});
-        expect(argv.volume).toEqual(["/env:/path", "/cli:/path"]);
-    });
-
-    test("CLI explicit value takes precedence over env", () => {
-        const argv: Record<string, any> = {concurrency: 8};
-        injectGclEnvVars(argv, baseOptions, {"GCL_CONCURRENCY": "4"}, {});
-        expect(argv.concurrency).toBe(8);
-    });
-
-    test("CLI explicit value takes precedence even when matching default", () => {
-        const argv: Record<string, any> = {concurrency: 0};
-        injectGclEnvVars(argv, baseOptions, {"GCL_CONCURRENCY": "4"}, {});
-        expect(argv.concurrency).toBe(0);
-    });
-
-    test("env overrides when value was defaulted", () => {
-        const argv: Record<string, any> = {concurrency: 0};
-        injectGclEnvVars(argv, baseOptions, {"GCL_CONCURRENCY": "4"}, {concurrency: true});
-        expect(argv.concurrency).toBe(4);
-    });
-
-    test("skips keys with hyphens", () => {
-        const argv: Record<string, any> = {};
-        injectGclEnvVars(argv, baseOptions, {"GCL_SOME_KEBAB": "val"}, {});
-        expect(argv["some-kebab"]).toBeUndefined();
-    });
-
-    test("skips _ and $0 keys", () => {
-        const argv: Record<string, any> = {_: [], $0: "bin"};
-        injectGclEnvVars(argv, baseOptions, {"GCL__": "x", "GCL_$0": "y"}, {});
-        expect(argv._).toEqual([]);
-        expect(argv.$0).toBe("bin");
-    });
-
-    test("skips undefined env values", () => {
-        const argv: Record<string, any> = {quiet: false};
-        injectGclEnvVars(argv, baseOptions, {"GCL_QUIET": undefined}, {quiet: true});
-        expect(argv.quiet).toBe(false);
-    });
-});
-
 test("GCL_VARIABLE_* env vars are injected into job output via CLI", async () => {
     const {stdout} = await execFileAsync("bun", ["src/index.ts", "test-job", "--cwd", "tests/test-cases/gcl-variable-env"], {
         env: {