Skip to content

fix(cli): reject partially numeric line and hunk values #535

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
fallintoplace wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(cli): reject partially numeric line and hunk values #535

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/strict-cli-integers.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"hunkdiff": patch
---

Reject malformed and unsafe line and hunk numbers instead of accepting their numeric prefix.
56 changes: 41 additions & 15 deletions src/core/cli.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1044,6 +1044,21 @@ describe("parseCli command help text", () => {
});

describe("parseCli argument validation", () => {
/** Parse one numeric session-navigation flag through the public CLI parser. */
function parseTestNavigationTarget(flag: "--hunk" | "--old-line" | "--new-line", value: string) {
return parseCli([
"bun",
"hunk",
"session",
"navigate",
"session-1",
"--file",
"README.md",
flag,
value,
]);
}

test("rejects an invalid layout mode and rethrows the parser error", async () => {
await expect(parseCli(["bun", "hunk", "diff", "--mode", "bogus"])).rejects.toThrow(
"Invalid layout mode: bogus",
Expand All @@ -1056,21 +1071,32 @@ describe("parseCli argument validation", () => {
);
});

test("rejects a non-positive integer navigation target", async () => {
await expect(
parseCli([
"bun",
"hunk",
"session",
"navigate",
"session-1",
"--file",
"README.md",
"--hunk",
"0",
]),
).rejects.toThrow("Invalid positive integer: 0");
});
test.each(["1", "42", "9007199254740991"])(
"accepts positive integer navigation target %s",
async (value) => {
await expect(parseTestNavigationTarget("--hunk", value)).resolves.toMatchObject({
hunkNumber: Number(value),
});
},
);

test.each(["0", "-1", "1.5", "1e3", "12abc", "9007199254740992"])(
"rejects malformed positive integer navigation target %s",
async (value) => {
await expect(parseTestNavigationTarget("--hunk", value)).rejects.toThrow(
`Invalid positive integer: ${value}`,
);
},
);

test.each(["--hunk", "--old-line", "--new-line"])(
"rejects a partially numeric value for %s",
async (flag) => {
await expect(parseTestNavigationTarget(flag, "12abc")).rejects.toThrow(
"Invalid positive integer: 12abc",
);
},
);

test("rejects ambiguous diff input that is neither a single target nor a file pair", async () => {
await expect(parseCli(["bun", "hunk", "diff", "--staged", "left", "right"])).rejects.toThrow(
Expand Down
8 changes: 6 additions & 2 deletions src/core/cli.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,12 @@ function parseLayoutMode(value: string): LayoutMode {

/** Parse one required positive integer CLI value. */
function parsePositiveInt(value: string) {
const parsed = Number.parseInt(value, 10);
if (!Number.isInteger(parsed) || parsed <= 0) {
if (!/^[1-9]\d*$/.test(value)) {
throw new Error(`Invalid positive integer: ${value}`);
}

const parsed = Number(value);
if (!Number.isSafeInteger(parsed)) {
throw new Error(`Invalid positive integer: ${value}`);
}

Expand Down