github/codeql-action
1
0
Fork 0
mirror of https://github.com/github/codeql-action.git synced 2026-11-10 14:56:01 +00:00
Code Issues Packages Projects Releases Wiki Activity

Remove bespoke post-comment argument validation

Browse Source 
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
Mario Campos
2026-10-28 15:39:43 -05:00
parent ae9f97bade
commit 90d1bc983c
2 changed files with 19 additions and 134 deletions
Download Patch File Download Diff File Expand all files Collapse all files

60
pr-checks/post-comment.test.ts
View File

@@ -69,13 +69,6 @@ describe("parseOptions", async () => {
assert.equal(options.actionCondition, false);
});
await it("rejects a missing --body", () => {
assert.throws(
() => parseOptions(withoutFlag("--body")),
/Missing required flag --body/,
);
});
await it("rejects a missing --marker", () => {
assert.throws(
() => parseOptions(withoutFlag("--marker")),
@@ -97,53 +90,16 @@ describe("parseOptions", async () => {
);
});
await it("rejects an invalid --action-condition value", () => {
assert.throws(
() => parseOptions(withFlag("--action-condition", "yes")),
/Invalid value 'yes' for --action-condition/,
);
await it("rejects unknown flags using parseArgs", () => {
assert.throws(() => parseOptions([...validFlags, "--unknown", "value"]), {
code: "ERR_PARSE_ARGS_UNKNOWN_OPTION",
});
});
await it("rejects an invalid --action-if-true value", () => {
assert.throws(
() => parseOptions(withFlag("--action-if-true", "bogus")),
/Invalid value 'bogus' for --action-if-true/,
);
});
await it("rejects an invalid --action-if-false value", () => {
assert.throws(
() => parseOptions(withFlag("--action-if-false", "bogus")),
/Invalid value 'bogus' for --action-if-false/,
);
});
await it("rejects a missing --issue-id", () => {
assert.throws(
() => parseOptions(withoutFlag("--issue-id")),
/Missing required flag --issue-id/,
);
});
await it("rejects a non-numeric --issue-id", () => {
assert.throws(
() => parseOptions(withFlag("--issue-id", "not-a-number")),
/Invalid value 'not-a-number' for --issue-id/,
);
});
await it("rejects a missing --repository", () => {
assert.throws(
() => parseOptions(withoutFlag("--repository")),
/Missing required flag --repository/,
);
});
await it("rejects a --repository that isn't in the form 'owner/repo'", () => {
assert.throws(
() => parseOptions(withFlag("--repository", "not-a-slug")),
/Invalid value 'not-a-slug' for --repository/,
);
await it("rejects flags without a value using parseArgs", () => {
assert.throws(() => parseOptions([...validFlags, "--body"]), {
code: "ERR_PARSE_ARGS_INVALID_OPTION_VALUE",
});
});
});

93
pr-checks/post-comment.ts
View File

@@ -15,6 +15,7 @@ The `.github/actions/post-comment` action requires callers to supply this value
`marker` input.
*/
import assert from "node:assert/strict";
import { parseArgs } from "node:util";
import { type ApiClient, getApiClient } from "./api-client";
@@ -42,72 +43,7 @@ export interface Options {
repository: { owner: string; repo: string };
}
/**
* Validates and parses a required CLI flag's raw string `value`, delegating the actual
* value-specific validation/conversion to `parse`.
*/
function parseFlag<T>(
flag: string,
value: string | undefined,
parse: (value: string) => T,
): T {
if (value === undefined) {
throw new Error(`Missing required flag --${flag}.`);
}
try {
return parse(value);
} catch (err) {
const reason = err instanceof Error ? err.message : String(err);
throw new Error(`Invalid value '${value}' for --${flag}. ${reason}`);
}
}
/** Parses and validates `value` as an `Action`. */
function parseAction(flag: string, value: string | undefined): Action {
return parseFlag(flag, value, (raw) => {
if (!(ACTIONS as readonly string[]).includes(raw)) {
throw new Error(`Must be one of: ${ACTIONS.join(", ")}.`);
}
return raw as Action;
});
}
/** Parses and validates `value` as a boolean. */
function parseBoolean(flag: string, value: string | undefined): boolean {
return parseFlag(flag, value, (raw) => {
if (raw !== "true" && raw !== "false") {
throw new Error("Must be 'true' or 'false'.");
}
return raw === "true";
});
}
/** Parses and validates `value` as an issue/pull request number. */
function parseIssueId(value: string | undefined): number {
return parseFlag("issue-id", value, (raw) => {
const parsed = Number(raw);
if (!Number.isInteger(parsed) || parsed <= 0) {
throw new Error("Must be a positive integer.");
}
return parsed;
});
}
/** Parses and validates `value` as an `owner/repo` repository reference. */
function parseRepository(value: string | undefined): {
owner: string;
repo: string;
} {
return parseFlag("repository", value, (raw) => {
const [owner, repo, ...rest] = raw.split("/");
if (!owner || !repo || rest.length > 0) {
throw new Error("Must be in the form 'owner/repo'.");
}
return { owner, repo };
});
}
/** Parses the command-line arguments into `Options`. */
/** Parses CLI arguments supplied by the composite action into `Options`. */
export function parseOptions(argv?: string[]): Options {
const { values } = parseArgs({
args: argv,
@@ -123,24 +59,17 @@ export function parseOptions(argv?: string[]): Options {
strict: true,
});
if (values.body === undefined) {
throw new Error("Missing required flag --body.");
}
if (values.marker === undefined || values.marker.trim() === "") {
throw new Error("Missing required flag --marker.");
}
assert(values.marker?.trim(), "Missing required flag --marker.");
const [owner, repo] = values.repository!.split("/");
return {
body: values.body,
marker: values.marker,
actionCondition: parseBoolean(
"action-condition",
values["action-condition"],
),
actionIfTrue: parseAction("action-if-true", values["action-if-true"]),
actionIfFalse: parseAction("action-if-false", values["action-if-false"]),
issueId: parseIssueId(values["issue-id"]),
repository: parseRepository(values.repository),
body: values.body!,
marker: values.marker!,
actionCondition: values["action-condition"] === "true",
actionIfTrue: values["action-if-true"] as Action,
actionIfFalse: values["action-if-false"] as Action,
issueId: Number(values["issue-id"]),
repository: { owner, repo },
};
}