use spawn instead of exec to avoid injection vulnerability (#416)
https://github.com/openai/codex/pull/160 introduced a call to `exec()` that takes a format string as an argument, but it is not clear that the expansions within the format string are escaped safely. As written, it is possible a carefully crafted command (e.g., if `cwd` were `"; && rm -rf` or something...) could run arbitrary code. Moving to `spawn()` makes this a bit better, as now at least `spawn()` itself won't run an arbitrary process, though I suppose `osascript` itself still could if the value passed to `-e` were abused. I'm not clear on the escaping rules for AppleScript to ensure that `safePreview` and `cwd` are injected safely. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/openai/codex/pull/416). * #423 * #420 * #419 * __->__ #416
This commit is contained in:
@@ -32,7 +32,7 @@ import HelpOverlay from "../help-overlay.js";
|
||||
import HistoryOverlay from "../history-overlay.js";
|
||||
import ModelOverlay from "../model-overlay.js";
|
||||
import { Box, Text } from "ink";
|
||||
import { exec } from "node:child_process";
|
||||
import { spawn } from "node:child_process";
|
||||
import OpenAI from "openai";
|
||||
import React, { useEffect, useMemo, useRef, useState } from "react";
|
||||
import { inspect } from "util";
|
||||
@@ -374,9 +374,10 @@ export default function TerminalChat({
|
||||
const safePreview = preview.replace(/"/g, '\\"');
|
||||
const title = "Codex CLI";
|
||||
const cwd = PWD;
|
||||
exec(
|
||||
`osascript -e 'display notification "${safePreview}" with title "${title}" subtitle "${cwd}" sound name "Ping"'`,
|
||||
);
|
||||
spawn("osascript", [
|
||||
"-e",
|
||||
`display notification "${safePreview}" with title "${title}" subtitle "${cwd}" sound name "Ping"`,
|
||||
]);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user