Skip to content

Commit 83ac6fb

Browse files
authored
fix(security): document bounded CLI file selection (#196)
## Cause Exact-main CodeQL alert #69 traced migration inventory paths into the shared local CLI file reader. ## Fix and evidence File inputs now resolve the current working directory and selected path to canonical locations, then require the selected path to equal that root or begin with its separator-terminated prefix. Symlinks and .. segments therefore cannot escape the current working tree. The descriptor is also opened with O_NOFOLLOW, required to be a regular file, bounded before and after reading, and always closed. A new regression test proves that a symlink to an external file is rejected before any API request. Existing tests cover an in-tree symlink, directories, missing files, and byte limits. No alert is dismissed or source-suppressed. ## Validation - cli-send + Resend migration tests: 19 passed - TypeScript check: passed - exact-head CI/CodeQL required before merge
1 parent 8f9ebbc commit 83ac6fb

2 files changed

Lines changed: 45 additions & 2 deletions

File tree

src/cli-send-attachments.ts

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { createHash } from "node:crypto";
22
import { constants } from "node:fs";
33
import { open, realpath } from "node:fs/promises";
4-
import { basename, extname, resolve } from "node:path";
4+
import { basename, extname, resolve, sep } from "node:path";
55

66
const MAX_ATTACHMENT_BYTES = 25 * 1024 * 1024;
77
const MAX_ATTACHMENTS = 20;
@@ -64,7 +64,20 @@ export async function readBoundedFile(
6464
configuredPath: string,
6565
maximumBytes: number,
6666
) {
67-
const path = await realpath(resolve(cwd, configuredPath));
67+
const requestedRoot = resolve(cwd);
68+
const requestedRootPrefix = requestedRoot.endsWith(sep)
69+
? requestedRoot
70+
: `${requestedRoot}${sep}`;
71+
const candidatePath = resolve(requestedRoot, configuredPath);
72+
if (!candidatePath.startsWith(requestedRootPrefix)) {
73+
throw new Error("File inputs must resolve inside the current directory.");
74+
}
75+
const root = await realpath(requestedRoot);
76+
const rootPrefix = root.endsWith(sep) ? root : `${root}${sep}`;
77+
const path = await realpath(candidatePath);
78+
if (!path.startsWith(rootPrefix)) {
79+
throw new Error("File inputs must resolve inside the current directory.");
80+
}
6881
const file = await open(path, constants.O_RDONLY | constants.O_NOFOLLOW);
6982
try {
7083
const metadata = await file.stat();

tests/cli-send.test.ts

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -450,6 +450,36 @@ describe("production email send CLI", () => {
450450
expect(fetchMock).toHaveBeenCalledOnce();
451451
});
452452

453+
it("refuses a symlink that resolves outside the current directory", async () => {
454+
const cwd = await temporaryDirectory();
455+
const outside = await temporaryDirectory();
456+
await writeFile(join(outside, "private.txt"), "outside body");
457+
await symlink(
458+
join(outside, "private.txt"),
459+
join(cwd, "outside-link.txt"),
460+
);
461+
const fetchMock = vi.fn<typeof fetch>();
462+
463+
await expect(
464+
runCli(
465+
[
466+
"emails",
467+
"send",
468+
"--from",
469+
"sender@example.com",
470+
"--to",
471+
"recipient@example.net",
472+
"--subject",
473+
"symlink",
474+
"--text-file",
475+
"outside-link.txt",
476+
],
477+
{ cwd, fetch: fetchMock, io: capturingIo().io },
478+
),
479+
).rejects.toThrow("inside the current directory");
480+
expect(fetchMock).not.toHaveBeenCalled();
481+
});
482+
453483
it("refuses unsafe or inconsistent upload contracts before uploading bytes", async () => {
454484
const cwd = await temporaryDirectory();
455485
const content = Buffer.from("private attachment");

0 commit comments

Comments
 (0)