Skip to content

Commit d219942

Browse files
author
Gyan Ranjan A
committed
fix: separate global/component input namespaces, extract shared isStructured helper
Addresses review feedback: - Separate global and component CLI inputs into _global/_components namespaces in argv.input to prevent key collisions (#2) - Extract isStructuredInputsFile() and getGlobalFileInputs() to Utils to deduplicate detection logic in parser.ts and parser-includes.ts (#3) - Add test for namespace collision scenario
1 parent def5b04 commit d219942

5 files changed

Lines changed: 35 additions & 43 deletions

File tree

src/argv.ts

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -223,9 +223,10 @@ export class Argv {
223223
return variables;
224224
}
225225

226-
get input (): {[key: string]: any} {
226+
get input (): {_global: {[key: string]: any}; _components: {[key: string]: {[key: string]: any}}} {
227227
const val = this.map.get("input");
228-
const inputs: {[key: string]: any} = {};
228+
const _global: {[key: string]: any} = {};
229+
const _components: {[key: string]: {[key: string]: any}} = {};
229230
const pairs = typeof val == "string" ? val.split(" ") : val;
230231
const dangerousKeys = new Set(["__proto__", "constructor", "prototype"]);
231232
(pairs ?? []).forEach((inputPair: string) => {
@@ -249,16 +250,14 @@ export class Argv {
249250
}
250251

251252
if (component) {
252-
// Component-specific input: store under component namespace
253-
if (!inputs[component]) inputs[component] = {};
254-
inputs[component][key] = parsedValue;
253+
if (!_components[component]) _components[component] = {};
254+
_components[component][key] = parsedValue;
255255
} else {
256-
// Global input
257-
inputs[key] = parsedValue;
256+
_global[key] = parsedValue;
258257
}
259258
}
260259
});
261-
return inputs;
260+
return {_global, _components};
262261
}
263262

264263
get unsetVariables (): string[] {

src/parser-includes.ts

Lines changed: 4 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -57,24 +57,11 @@ export class ParserIncludes {
5757
static async init (gitlabData: any, opts: ParserIncludesInitOptions): Promise<any[]> {
5858
const {argv, inputs: inputsConfig} = opts;
5959
const fileInputs = inputsConfig._file ?? {};
60-
const cliInputs = inputsConfig._cli ?? {};
61-
62-
// Extract global CLI inputs (non-component-specific)
63-
const cliGlobalInputs: {[key: string]: any} = {};
64-
const cliComponentInputs: {[key: string]: any} = {};
65-
for (const [key, value] of Object.entries(cliInputs)) {
66-
if (typeof value === "object" && value !== null && !Array.isArray(value)) {
67-
// This is a component-specific input (e.g., {deploy: {replicas: 5}})
68-
cliComponentInputs[key] = value;
69-
} else {
70-
// This is a global input
71-
cliGlobalInputs[key] = value;
72-
}
73-
}
60+
const cliGlobalInputs = inputsConfig._cliGlobal ?? {};
61+
const cliComponentInputs = inputsConfig._cliComponents ?? {};
7462

75-
// If file has _global key, use structured format; otherwise treat entire file as global
76-
const isStructured = fileInputs._global !== undefined || Object.keys(fileInputs).some(k => !k.startsWith("_") && typeof fileInputs[k] === "object" && fileInputs[k] !== null && !Array.isArray(fileInputs[k]));
77-
const globalInputs = isStructured ? {...(fileInputs._global ?? {}), ...cliGlobalInputs} : {...fileInputs, ...cliGlobalInputs};
63+
const isStructured = Utils.isStructuredInputsFile(fileInputs);
64+
const globalInputs = {...Utils.getGlobalFileInputs(fileInputs), ...cliGlobalInputs};
7865
this.count++;
7966
assert(
8067
this.count <= opts.maximumIncludes + 1, // 1st init call is not counted

src/parser.ts

Lines changed: 3 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -108,17 +108,8 @@ export class Parser {
108108
const inputs = await this.loadInputs(cwd, argv);
109109

110110
// Build root-level inputs from global CLI + global file inputs
111-
const cliInputs = inputs._cli ?? {};
112111
const fileInputs = inputs._file ?? {};
113-
const globalCliInputs: {[key: string]: any} = {};
114-
for (const [k, v] of Object.entries(cliInputs)) {
115-
if (typeof v !== "object" || v === null || Array.isArray(v)) {
116-
globalCliInputs[k] = v;
117-
}
118-
}
119-
const isStructuredFile = fileInputs._global !== undefined || Object.keys(fileInputs).some(k => !k.startsWith("_") && typeof fileInputs[k] === "object" && fileInputs[k] !== null && !Array.isArray(fileInputs[k]));
120-
const fileGlobalInputs = isStructuredFile ? (fileInputs._global ?? {}) : fileInputs;
121-
const rootInputs = {...fileGlobalInputs, ...globalCliInputs};
112+
const rootInputs = {...Utils.getGlobalFileInputs(fileInputs), ...inputs._cliGlobal};
122113

123114
let yamlDataList: any[] = [{stages: [".pre", "build", "test", "deploy", ".post"]}];
124115
const gitlabCiData = await Parser.loadYaml(`${cwd}/${file}`, {inputs: rootInputs}, this.expandVariables, writeStreams);
@@ -258,8 +249,8 @@ export class Parser {
258249
}
259250
}
260251

261-
// Return both file inputs and CLI inputs separately for component-specific merging
262-
return {_file: fileInputs, _cli: argv.input};
252+
const cliInput = argv.input;
253+
return {_file: fileInputs, _cliGlobal: cliInput._global, _cliComponents: cliInput._components};
263254
}
264255

265256
static async loadYaml (filePath: string, ctx: any = {}, expandVariables: boolean = true, writeStreams?: WriteStreams): Promise<any> {

src/utils.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -550,4 +550,12 @@ export class Utils {
550550
// Return the first alias in the set
551551
return aliases.values().next().value!;
552552
}
553+
554+
static isStructuredInputsFile (fileInputs: {[key: string]: any}): boolean {
555+
return fileInputs._global !== undefined || Object.keys(fileInputs).some(k => !k.startsWith("_") && typeof fileInputs[k] === "object" && fileInputs[k] !== null && !Array.isArray(fileInputs[k]));
556+
}
557+
558+
static getGlobalFileInputs (fileInputs: {[key: string]: any}): {[key: string]: any} {
559+
return Utils.isStructuredInputsFile(fileInputs) ? (fileInputs._global ?? {}) : {...fileInputs};
560+
}
553561
}

tests/argv-input.test.ts

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,31 +11,38 @@ beforeEach(() => {
1111
test("input parses component names with slashes", async () => {
1212
const argv = await Argv.build({input: ["templates/deploy:replicas=5"]}, writeStreams);
1313
const result = argv.input;
14-
expect(result["templates/deploy"]).toEqual({replicas: 5});
14+
expect(result._components["templates/deploy"]).toEqual({replicas: 5});
1515
});
1616

1717
test("input keeps global and component keys separate", async () => {
1818
const argv = await Argv.build({input: ["deploy:replicas=5", "environment=prod"]}, writeStreams);
1919
const result = argv.input;
20-
expect(result["deploy"]).toEqual({replicas: 5});
21-
expect(result["environment"]).toEqual("prod");
20+
expect(result._components["deploy"]).toEqual({replicas: 5});
21+
expect(result._global["environment"]).toEqual("prod");
2222
});
2323

2424
test("input ignores __proto__ component to prevent prototype pollution", async () => {
2525
const argv = await Argv.build({input: ["__proto__:polluted=true"]}, writeStreams);
2626
const result = argv.input;
27-
expect(Object.hasOwn(result, "__proto__")).toBe(false);
27+
expect(Object.hasOwn(result._components, "__proto__")).toBe(false);
2828
expect(({} as any).polluted).toBeUndefined();
2929
});
3030

3131
test("input ignores __proto__ key to prevent prototype pollution", async () => {
3232
const argv = await Argv.build({input: ["__proto__=true"]}, writeStreams);
3333
const result = argv.input;
34-
expect(Object.keys(result)).not.toContain("__proto__");
34+
expect(Object.keys(result._global)).not.toContain("__proto__");
3535
});
3636

3737
test("input ignores constructor key to prevent prototype pollution", async () => {
3838
const argv = await Argv.build({input: ["constructor:toString=bad"]}, writeStreams);
3939
const result = argv.input;
40-
expect(Object.hasOwn(result, "constructor")).toBe(false);
40+
expect(Object.hasOwn(result._components, "constructor")).toBe(false);
41+
});
42+
43+
test("input namespace collision: same key as global and component name", async () => {
44+
const argv = await Argv.build({input: ["deploy=prod", "deploy:replicas=5"]}, writeStreams);
45+
const result = argv.input;
46+
expect(result._global["deploy"]).toEqual("prod");
47+
expect(result._components["deploy"]).toEqual({replicas: 5});
4148
});

0 commit comments

Comments
 (0)