Skip to content

Commit 2609b97

Browse files
vincentkocvincentkoc
committed
fix(scripts): reject short flag values in bench parsers
1 parent 03bc600 commit 2609b97

6 files changed

Lines changed: 78 additions & 35 deletions

‎scripts/bench-sqlite-state.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,7 @@ function parseFlagValue(flag: string, argv: string[]): string | undefined {
123123
return undefined;
124124
}
125125
const value = argv[index + 1];
126-
if (!value || value.startsWith("--")) {
126+
if (!value || value.startsWith("-")) {
127127
throw new CliUsageError(`${flag} requires a value`);
128128
}
129129
return value;
@@ -141,7 +141,7 @@ function validateArgs(argv: string[]): void {
141141
}
142142
if (VALUE_FLAGS.has(arg)) {
143143
const value = argv[index + 1];
144-
if (!value || value.startsWith("--")) {
144+
if (!value || value.startsWith("-")) {
145145
throw new CliUsageError(`${arg} requires a value`);
146146
}
147147
index += 1;

‎scripts/embedded-run-abort-leak.ts‎

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ type Options = {
3636

3737
function readValue(raw: string | undefined, flag: string): string {
3838
const value = raw?.trim() ?? "";
39-
if (!value || value.startsWith("--")) {
39+
if (!value || value.startsWith("-")) {
4040
fail(`${flag} requires a value`);
4141
}
4242
return value;
@@ -69,21 +69,23 @@ function parseArgs(argv: string[]): Options {
6969
opts.snapDir = readValue(next, arg);
7070
i += 1;
7171
break;
72-
case "--mode":
72+
case "--mode": {
73+
const mode = readValue(next, arg);
7374
if (
74-
next === "production" ||
75-
next === "closure-extracted" ||
76-
next === "closure-inline" ||
77-
next === "synthetic-leak"
75+
mode === "production" ||
76+
mode === "closure-extracted" ||
77+
mode === "closure-inline" ||
78+
mode === "synthetic-leak"
7879
) {
79-
opts.mode = next;
80+
opts.mode = mode;
8081
} else {
8182
fail(
8283
`--mode must be one of: production, closure-extracted, closure-inline, synthetic-leak`,
8384
);
8485
}
8586
i += 1;
8687
break;
88+
}
8789
case "--max-rss-growth-mb":
8890
opts.maxRssGrowthMb = parseNonNegativeInt(next, arg);
8991
i += 1;
@@ -139,6 +141,9 @@ function parseStrictInt(
139141
label: "positive" | "non-negative",
140142
): number {
141143
const text = (raw ?? "").trim();
144+
if (!text || text.startsWith("-")) {
145+
fail(`${flag} requires a value`);
146+
}
142147
if (!/^\d+$/u.test(text)) {
143148
fail(`${flag} must be a ${label} integer`);
144149
}

‎scripts/perf/issue-78851-model-resolution.ts‎

Lines changed: 32 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -85,24 +85,24 @@ type Report = {
8585
cpuProfilePath?: string;
8686
};
8787

88-
function parseFlagValue(flag: string): string | undefined {
89-
const index = process.argv.indexOf(flag);
88+
function parseFlagValue(flag: string, args = process.argv.slice(2)): string | undefined {
89+
const index = args.indexOf(flag);
9090
if (index === -1) {
9191
return undefined;
9292
}
93-
const value = process.argv[index + 1];
94-
if (!value || value.startsWith("--")) {
93+
const value = args[index + 1];
94+
if (!value || value.startsWith("-")) {
9595
throw new CliArgumentError(`${flag} requires a value`);
9696
}
9797
return value;
9898
}
9999

100-
function hasFlag(flag: string): boolean {
101-
return process.argv.includes(flag);
100+
function hasFlag(flag: string, args = process.argv.slice(2)): boolean {
101+
return args.includes(flag);
102102
}
103103

104-
function parsePositiveInt(flag: string, fallback: number): number {
105-
const raw = parseFlagValue(flag);
104+
function parsePositiveInt(flag: string, fallback: number, args = process.argv.slice(2)): number {
105+
const raw = parseFlagValue(flag, args);
106106
if (!raw) {
107107
return fallback;
108108
}
@@ -116,8 +116,8 @@ function parsePositiveInt(flag: string, fallback: number): number {
116116
return value;
117117
}
118118

119-
function parseNonNegativeInt(flag: string, fallback: number): number {
120-
const raw = parseFlagValue(flag);
119+
function parseNonNegativeInt(flag: string, fallback: number, args = process.argv.slice(2)): number {
120+
const raw = parseFlagValue(flag, args);
121121
if (!raw) {
122122
return fallback;
123123
}
@@ -138,28 +138,32 @@ function validateCliArgs(args = process.argv.slice(2)): void {
138138
continue;
139139
}
140140
if (VALUE_FLAGS.has(arg)) {
141+
const value = args[index + 1];
142+
if (!value || value.startsWith("-")) {
143+
throw new CliArgumentError(`${arg} requires a value`);
144+
}
141145
index += 1;
142146
continue;
143147
}
144148
throw new CliArgumentError(`Unknown argument: ${arg}`);
145149
}
146150
}
147151

148-
function parseOptions(): Options {
149-
validateCliArgs();
152+
function parseOptions(args = process.argv.slice(2)): Options {
153+
validateCliArgs(args);
150154
return {
151-
agentCount: parsePositiveInt("--agents", 8),
152-
cpuProfDir: parseFlagValue("--cpu-prof-dir"),
153-
cpuProfOutput: parseFlagValue("--cpu-prof-output"),
154-
json: hasFlag("--json"),
155-
keepTemp: hasFlag("--keep-temp"),
156-
lookupsPerRun: parsePositiveInt("--lookups", 32),
157-
modelsPerProvider: parsePositiveInt("--models-per-provider", 16),
158-
output: parseFlagValue("--output"),
159-
providers: parsePositiveInt("--providers", 48),
160-
runs: parsePositiveInt("--runs", 8),
161-
runtimeHooks: hasFlag("--runtime-hooks"),
162-
warmup: parseNonNegativeInt("--warmup", 1),
155+
agentCount: parsePositiveInt("--agents", 8, args),
156+
cpuProfDir: parseFlagValue("--cpu-prof-dir", args),
157+
cpuProfOutput: parseFlagValue("--cpu-prof-output", args),
158+
json: hasFlag("--json", args),
159+
keepTemp: hasFlag("--keep-temp", args),
160+
lookupsPerRun: parsePositiveInt("--lookups", 32, args),
161+
modelsPerProvider: parsePositiveInt("--models-per-provider", 16, args),
162+
output: parseFlagValue("--output", args),
163+
providers: parsePositiveInt("--providers", 48, args),
164+
runs: parsePositiveInt("--runs", 8, args),
165+
runtimeHooks: hasFlag("--runtime-hooks", args),
166+
warmup: parseNonNegativeInt("--warmup", 1, args),
163167
};
164168
}
165169

@@ -468,11 +472,13 @@ function printHuman(report: Report, cpuProfilePath?: string): void {
468472
}
469473

470474
async function main(): Promise<void> {
471-
if (hasFlag("--help") || hasFlag("-h")) {
475+
const args = process.argv.slice(2);
476+
validateCliArgs(args);
477+
if (hasFlag("--help", args) || hasFlag("-h", args)) {
472478
printUsage();
473479
return;
474480
}
475-
const options = parseOptions();
481+
const options = parseOptions(args);
476482
const tempRoot = await mkdtemp(path.join(tmpdir(), "openclaw-issue-78851-"));
477483
const workspaceDir = path.join(tempRoot, "workspace");
478484
await mkdir(workspaceDir, { recursive: true });

‎test/scripts/bench-sqlite-state.test.ts‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,14 @@ describe("scripts/bench-sqlite-state", () => {
3030
expect(result.stderr.trim()).toBe("error: --output requires a value");
3131
});
3232

33+
it("rejects short flag output values before seeding benchmark databases", () => {
34+
const result = runBench(["--output", "-h"]);
35+
36+
expect(result.status).toBe(2);
37+
expect(result.stdout).toBe("");
38+
expect(result.stderr.trim()).toBe("error: --output requires a value");
39+
});
40+
3341
it("rejects invalid profiles without printing a stack trace", () => {
3442
const result = runBench(["--profile", "huge"]);
3543

‎test/scripts/embedded-run-abort-leak.test.ts‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,4 +60,20 @@ describe("scripts/embedded-run-abort-leak", () => {
6060
expect(result.stdout).toBe("");
6161
expect(result.stderr).toContain("error: --snap-dir requires a value");
6262
});
63+
64+
it("rejects short flag values before writing heap snapshots", () => {
65+
const snapDirResult = runHarness(["--snap-dir", "-h", "--quiet", "--iters", "1"]);
66+
const itersResult = runHarness(["--iters", "-h", "--quiet"]);
67+
const modeResult = runHarness(["--mode", "-h", "--quiet"]);
68+
69+
expect(snapDirResult.status).toBe(2);
70+
expect(snapDirResult.stdout).toBe("");
71+
expect(snapDirResult.stderr).toContain("error: --snap-dir requires a value");
72+
expect(itersResult.status).toBe(2);
73+
expect(itersResult.stdout).toBe("");
74+
expect(itersResult.stderr).toContain("error: --iters requires a value");
75+
expect(modeResult.status).toBe(2);
76+
expect(modeResult.stdout).toBe("");
77+
expect(modeResult.stderr).toContain("error: --mode requires a value");
78+
});
6379
});

‎test/scripts/issue-78851-model-resolution.test.ts‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,4 +40,12 @@ describe("issue 78851 model resolution profiler CLI", () => {
4040
expect(result.stdout).toBe("");
4141
expect(result.stderr.trim()).toBe("--providers must be a positive integer");
4242
});
43+
44+
it("rejects short flag values before starting the profiler", () => {
45+
const result = runProfiler("--providers", "-h");
46+
47+
expect(result.status).toBe(1);
48+
expect(result.stdout).toBe("");
49+
expect(result.stderr.trim()).toBe("--providers requires a value");
50+
});
4351
});

0 commit comments

Comments
 (0)