Skip to content

Commit 01532a5

Browse files
mldangelo-oaibatmnnncursoragent
authored
fix: keep scan output private through completion and exports (#118)
* fix: keep scan output dirs private across shared parents Reject group/world-writable non-sticky output parents at prepare time, and re-check scan-directory ownership/mode whenever the workbench or contract loader resolves the path, so a mid-scan rename under a shared parent cannot substitute forged completion artifacts. Co-authored-by: Cursor <cursoragent@cursor.com> * style: format inherited scan-output regression coverage * test: keep completed scan event fixtures private * fix: verify trusted output ancestry throughout contract loading * fix: enforce trusted scan ancestry in TypeScript and Python --------- Co-authored-by: batmanknows <122247678+batmnnn@users.noreply.github.com> Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent 2b2d87c commit 01532a5

7 files changed

Lines changed: 273 additions & 2 deletions

File tree

sdk/typescript/_bundled_plugin/scripts/workbench_db.py

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3476,6 +3476,38 @@ def require_canonical_scan_directory(scan_dir: Path) -> Path:
34763476
scan_dir
34773477
):
34783478
raise SystemExit("Scan directory must be an existing canonical non-symlink directory.")
3479+
# Re-check privacy on every resolution so a mid-scan rename/replace under a
3480+
# shared parent cannot substitute another user's forged artifact tree.
3481+
if os.name != "nt":
3482+
if stat.S_IMODE(metadata.st_mode) & 0o077:
3483+
raise SystemExit(
3484+
"Scan directory must not be accessible to other users (chmod 700)."
3485+
)
3486+
geteuid = getattr(os, "geteuid", None)
3487+
effective_uid = geteuid() if geteuid is not None else None
3488+
if effective_uid is not None and metadata.st_uid != effective_uid:
3489+
raise SystemExit("Scan directory must be owned by the current user.")
3490+
for parent in scan_dir.parents:
3491+
try:
3492+
parent_metadata = parent.lstat()
3493+
except OSError as exc:
3494+
raise SystemExit("Scan output parent could not be inspected.") from exc
3495+
if not stat.S_ISDIR(parent_metadata.st_mode) or stat.S_ISLNK(
3496+
parent_metadata.st_mode
3497+
):
3498+
raise SystemExit("Scan output parent must be a non-symlink directory.")
3499+
if effective_uid is not None and parent_metadata.st_uid not in {
3500+
0,
3501+
effective_uid,
3502+
}:
3503+
raise SystemExit("Scan output parent must have a trusted owner.")
3504+
if (
3505+
stat.S_IMODE(parent_metadata.st_mode) & 0o022
3506+
and not parent_metadata.st_mode & stat.S_ISVTX
3507+
):
3508+
raise SystemExit(
3509+
"Scan output parent must not be group- or world-writable without the sticky bit."
3510+
)
34793511
return scan_dir
34803512

34813513

sdk/typescript/src/contract.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,10 @@ import type {
1717
FindingsDocument,
1818
ScanManifest,
1919
} from "./models.js";
20+
import {
21+
requirePrivateOutputDirectory,
22+
requireSecureOutputAncestry,
23+
} from "./runtime.js";
2024
import type { NormalizedTarget, ScanMode } from "./targets.js";
2125

2226
const DOCUMENTS = {
@@ -581,9 +585,21 @@ async function requireScanRoot(
581585
) {
582586
throw new Error("not a directory");
583587
}
588+
try {
589+
requirePrivateOutputDirectory(returned, canonical);
590+
await requireSecureOutputAncestry(canonical);
591+
} catch (error) {
592+
throw new ContractValidationError(
593+
error instanceof Error
594+
? error.message
595+
: "Scan directory must remain private to the current user.",
596+
{ cause: error },
597+
);
598+
}
584599
return { path: canonical, metadata: returned };
585600
} catch (error) {
586601
throwIfAborted(signal);
602+
if (error instanceof ContractValidationError) throw error;
587603
throw new ContractValidationError(
588604
"Scan directory must be an existing non-symlink directory.",
589605
{ cause: error },
@@ -606,6 +622,8 @@ async function verifyScanRoot(
606622
) {
607623
throw new Error("scan directory changed while reading");
608624
}
625+
requirePrivateOutputDirectory(current, root.path);
626+
await requireSecureOutputAncestry(root.path);
609627
} catch (error) {
610628
throwIfAborted(signal);
611629
throw new ContractValidationError(

sdk/typescript/src/runtime.ts

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -567,6 +567,7 @@ export async function validateOutputDir(
567567
);
568568
}
569569
requirePrivateOutputDirectory(metadata, path);
570+
await requireSecureOutputAncestry(path);
570571
const canonical = await realpath(path);
571572
requireModelSafeOutputDir(canonical);
572573
return canonical;
@@ -581,6 +582,7 @@ export async function validateOutputDir(
581582
relative(parent, path),
582583
);
583584
requireModelSafeOutputDir(canonical);
585+
await requireSecureOutputAncestry(canonical);
584586
return canonical;
585587
}
586588
break;
@@ -704,6 +706,7 @@ export async function validatePreparedOutputDir(
704706
);
705707
}
706708
requirePrivateOutputDirectory(metadata, path);
709+
await requireSecureOutputAncestry(canonical);
707710
return canonical;
708711
}
709712

@@ -725,6 +728,78 @@ export function requirePrivateOutputDirectory(
725728
}
726729
}
727730

731+
/** Reject shared parents whose owner can rename or replace private scan output. */
732+
export async function requireSecureOutputAncestry(
733+
path: string,
734+
effectiveUid = process.geteuid?.(),
735+
): Promise<void> {
736+
if (process.platform === "win32") return;
737+
let current = dirname(resolve(path));
738+
while (true) {
739+
try {
740+
current = await realpath(current);
741+
break;
742+
} catch (error) {
743+
if (nodeErrorCode(error) !== "ENOENT") {
744+
throw new OutputDirectoryError(
745+
`Unable to inspect scan output parent directory: ${current}`,
746+
{ cause: error },
747+
);
748+
}
749+
const parent = dirname(current);
750+
if (parent === current) {
751+
throw new OutputDirectoryError(
752+
`Unable to inspect scan output parent directory: ${current}`,
753+
{ cause: error },
754+
);
755+
}
756+
current = parent;
757+
}
758+
}
759+
while (true) {
760+
let metadata: Stats;
761+
try {
762+
metadata = await lstat(current);
763+
} catch (error) {
764+
throw new OutputDirectoryError(
765+
`Unable to inspect scan output parent directory: ${current}`,
766+
{ cause: error },
767+
);
768+
}
769+
if (!metadata.isDirectory() || metadata.isSymbolicLink()) {
770+
throw new OutputDirectoryError(
771+
`Scan output parent must be a non-symlink directory: ${current}`,
772+
);
773+
}
774+
requireTrustedOutputAncestor(metadata, current, effectiveUid);
775+
const parent = dirname(current);
776+
if (parent === current) return;
777+
current = parent;
778+
}
779+
}
780+
781+
export function requireTrustedOutputAncestor(
782+
metadata: Pick<Stats, "mode" | "uid">,
783+
path: string,
784+
effectiveUid = process.geteuid?.(),
785+
): void {
786+
if (
787+
effectiveUid !== undefined &&
788+
metadata.uid !== 0 &&
789+
metadata.uid !== effectiveUid
790+
) {
791+
throw new OutputDirectoryError(
792+
`Scan output parent must have a trusted owner: ${path}`,
793+
);
794+
}
795+
if ((metadata.mode & 0o022) === 0) return;
796+
if ((metadata.mode & 0o1000) === 0) {
797+
throw new OutputDirectoryError(
798+
`Scan output parent must not be group- or world-writable without the sticky bit: ${path}`,
799+
);
800+
}
801+
}
802+
728803
async function removeEmptyDirectories(
729804
path: string,
730805
root: string,

sdk/typescript/tests-ts/contract.test.ts

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { createHash } from "node:crypto";
22
import {
3+
chmod,
34
cp,
45
mkdir,
56
mkdtemp,
@@ -32,6 +33,7 @@ async function copyExample(): Promise<string> {
3233
temporaryDirectories.push(root);
3334
const scanDir = join(root, "scan");
3435
await cp(EXAMPLE, scanDir, { recursive: true });
36+
if (process.platform !== "win32") await chmod(scanDir, 0o700);
3537
return scanDir;
3638
}
3739

@@ -124,6 +126,34 @@ describe("canonical scan contract", () => {
124126
).rejects.toMatchObject({ name: "AbortError" });
125127
});
126128

129+
test.skipIf(process.platform === "win32")(
130+
"rejects a scan directory that is no longer private to the current user",
131+
async () => {
132+
const scanDir = await copyExample();
133+
await chmod(scanDir, 0o755);
134+
await expect(
135+
loadContract(scanDir, { pluginRoot: PLUGIN_ROOT }),
136+
).rejects.toThrow("must not be accessible to other users");
137+
},
138+
);
139+
140+
test.skipIf(process.platform === "win32")(
141+
"rejects a private scan beneath an insecure shared ancestor",
142+
async () => {
143+
const scanDir = await copyExample();
144+
const parent = dirname(scanDir);
145+
await chmod(parent, 0o777);
146+
147+
try {
148+
await expect(
149+
loadContract(scanDir, { pluginRoot: PLUGIN_ROOT }),
150+
).rejects.toThrow("sticky bit");
151+
} finally {
152+
await chmod(parent, 0o700);
153+
}
154+
},
155+
);
156+
127157
test.skipIf(process.platform === "win32")(
128158
"accepts a scan directory beneath a symlinked parent",
129159
async () => {
@@ -134,7 +164,9 @@ describe("canonical scan contract", () => {
134164
const parent = join(root, "actual-parent");
135165
const linkedParent = join(root, "linked-parent");
136166
await mkdir(parent);
137-
await cp(EXAMPLE, join(parent, "scan"), { recursive: true });
167+
const scanDir = join(parent, "scan");
168+
await cp(EXAMPLE, scanDir, { recursive: true });
169+
if (process.platform !== "win32") await chmod(scanDir, 0o700);
138170
await symlink(parent, linkedParent);
139171

140172
await expect(

sdk/typescript/tests-ts/runtime.test.ts

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,8 @@ import {
5757
preparePersistentScanRoot,
5858
requirePrivateCredentialHome,
5959
requirePrivateOutputDirectory,
60+
requireSecureOutputAncestry,
61+
requireTrustedOutputAncestor,
6062
runWorkbench,
6163
setCodexSecurityCredentialLogout,
6264
} from "../src/runtime.js";
@@ -1664,6 +1666,80 @@ describe("runtime directories and plugin Python boundary", () => {
16641666
).not.toThrow();
16651667
});
16661668

1669+
testPosix(
1670+
"rejects scan output under a non-sticky shared parent directory",
1671+
async () => {
1672+
const root = await temporaryDirectory();
1673+
const shared = join(root, "shared");
1674+
await mkdir(shared, { mode: 0o777 });
1675+
await chmod(shared, 0o777);
1676+
const output = join(shared, "results");
1677+
1678+
await expect(prepareOutputDir(output, "repo")).rejects.toThrow(
1679+
"sticky bit",
1680+
);
1681+
await expect(requireSecureOutputAncestry(output)).rejects.toThrow(
1682+
"sticky bit",
1683+
);
1684+
},
1685+
);
1686+
1687+
testPosix(
1688+
"accepts scan output under a sticky shared parent directory",
1689+
async () => {
1690+
const root = await temporaryDirectory();
1691+
const shared = join(root, "shared");
1692+
await mkdir(shared, { mode: 0o1777 });
1693+
await chmod(shared, 0o1777);
1694+
// Some filesystems (notably user dirs on macOS APFS) ignore sticky on
1695+
// chmod; fall back to the process temp root when it is already sticky.
1696+
let stickyParent = shared;
1697+
if (((await lstat(shared)).mode & 0o1000) === 0) {
1698+
stickyParent = await realpath(tmpdir());
1699+
if (((await lstat(stickyParent)).mode & 0o1000) === 0) {
1700+
return;
1701+
}
1702+
}
1703+
const output = join(
1704+
stickyParent,
1705+
`codex-security-sticky-${process.pid}-${Date.now()}`,
1706+
);
1707+
temporaryDirectories.push(output);
1708+
1709+
await expect(
1710+
requireSecureOutputAncestry(output),
1711+
).resolves.toBeUndefined();
1712+
expect(await prepareOutputDir(output, "repo")).toBe(output);
1713+
},
1714+
);
1715+
1716+
testPosix("rejects sticky shared parents controlled by another user", () => {
1717+
expect(() =>
1718+
requireTrustedOutputAncestor(
1719+
{ mode: 0o41777, uid: 1001 },
1720+
"/shared",
1721+
1000,
1722+
),
1723+
).toThrow("trusted owner");
1724+
expect(() =>
1725+
requireTrustedOutputAncestor(
1726+
{ mode: 0o40755, uid: 1001 },
1727+
"/other-user",
1728+
1000,
1729+
),
1730+
).toThrow("trusted owner");
1731+
expect(() =>
1732+
requireTrustedOutputAncestor(
1733+
{ mode: 0o41777, uid: 1000 },
1734+
"/shared",
1735+
1000,
1736+
),
1737+
).not.toThrow();
1738+
expect(() =>
1739+
requireTrustedOutputAncestor({ mode: 0o41777, uid: 0 }, "/tmp", 1000),
1740+
).not.toThrow();
1741+
});
1742+
16671743
test("archives a non-empty private output directory", async () => {
16681744
const root = await temporaryDirectory();
16691745
const output = join(root, "scan");

sdk/typescript/tests-ts/support/api-events.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { cp, mkdtemp, realpath, rm, writeFile } from "node:fs/promises";
1+
import { chmod, cp, mkdtemp, realpath, rm, writeFile } from "node:fs/promises";
22
import { tmpdir } from "node:os";
33
import { join } from "node:path";
44
import type { ThreadEvent } from "@openai/codex-sdk";
@@ -31,6 +31,7 @@ export function createApiTestFixtures() {
3131
await cp(join(PLUGIN_ROOT, "examples", "completed-scan"), scanDir, {
3232
recursive: true,
3333
});
34+
await chmod(scanDir, 0o700);
3435
await writeFile(join(scanDir, "report.md"), "# Scan report\n");
3536
return scanDir;
3637
},

0 commit comments

Comments
 (0)