fix(security): require /etc/shells for shell env fallback
This commit is contained in:
@@ -28,6 +28,7 @@ describe("shell env fallback", () => {
|
||||
}
|
||||
|
||||
function runShellEnvFallbackForShell(shell: string) {
|
||||
resetShellPathCacheForTests();
|
||||
const env: NodeJS.ProcessEnv = { SHELL: shell };
|
||||
const exec = vi.fn(() => Buffer.from("OPENAI_API_KEY=from-shell\0"));
|
||||
const res = loadShellEnvFallback({
|
||||
@@ -170,18 +171,44 @@ describe("shell env fallback", () => {
|
||||
expect(exec).toHaveBeenCalledWith("/bin/sh", ["-l", "-c", "env -0"], expect.any(Object));
|
||||
});
|
||||
|
||||
it("uses trusted absolute SHELL path when executable on posix-style paths", () => {
|
||||
const accessSyncSpy = vi.spyOn(fs, "accessSync").mockImplementation(() => undefined);
|
||||
it("falls back to /bin/sh when SHELL is absolute but not registered in /etc/shells", () => {
|
||||
const readFileSyncSpy = vi
|
||||
.spyOn(fs, "readFileSync")
|
||||
.mockImplementation((filePath, encoding) => {
|
||||
if (filePath === "/etc/shells" && encoding === "utf8") {
|
||||
return "/bin/sh\n/bin/bash\n/bin/zsh\n";
|
||||
}
|
||||
throw new Error(`Unexpected readFileSync(${String(filePath)}) in test`);
|
||||
});
|
||||
try {
|
||||
const trustedShell = "/usr/bin/zsh-trusted";
|
||||
const { res, exec } = runShellEnvFallbackForShell(trustedShell);
|
||||
const expectedShell = process.platform === "win32" ? "/bin/sh" : trustedShell;
|
||||
const { res, exec } = runShellEnvFallbackForShell("/opt/homebrew/bin/evil-shell");
|
||||
|
||||
expect(res.ok).toBe(true);
|
||||
expect(exec).toHaveBeenCalledTimes(1);
|
||||
expect(exec).toHaveBeenCalledWith(expectedShell, ["-l", "-c", "env -0"], expect.any(Object));
|
||||
expect(exec).toHaveBeenCalledWith("/bin/sh", ["-l", "-c", "env -0"], expect.any(Object));
|
||||
} finally {
|
||||
accessSyncSpy.mockRestore();
|
||||
readFileSyncSpy.mockRestore();
|
||||
}
|
||||
});
|
||||
|
||||
it("uses SHELL when it is explicitly registered in /etc/shells", () => {
|
||||
const readFileSyncSpy = vi
|
||||
.spyOn(fs, "readFileSync")
|
||||
.mockImplementation((filePath, encoding) => {
|
||||
if (filePath === "/etc/shells" && encoding === "utf8") {
|
||||
return "/bin/sh\n/usr/bin/zsh-trusted\n";
|
||||
}
|
||||
throw new Error(`Unexpected readFileSync(${String(filePath)}) in test`);
|
||||
});
|
||||
try {
|
||||
const trustedShell = "/usr/bin/zsh-trusted";
|
||||
const { res, exec } = runShellEnvFallbackForShell(trustedShell);
|
||||
|
||||
expect(res.ok).toBe(true);
|
||||
expect(exec).toHaveBeenCalledTimes(1);
|
||||
expect(exec).toHaveBeenCalledWith(trustedShell, ["-l", "-c", "env -0"], expect.any(Object));
|
||||
} finally {
|
||||
readFileSyncSpy.mockRestore();
|
||||
}
|
||||
});
|
||||
|
||||
|
||||
@@ -8,13 +8,6 @@ import { sanitizeHostExecEnv } from "./host-env-security.js";
|
||||
const DEFAULT_TIMEOUT_MS = 15_000;
|
||||
const DEFAULT_MAX_BUFFER_BYTES = 2 * 1024 * 1024;
|
||||
const DEFAULT_SHELL = "/bin/sh";
|
||||
const TRUSTED_SHELL_PREFIXES = [
|
||||
"/bin/",
|
||||
"/usr/bin/",
|
||||
"/usr/local/bin/",
|
||||
"/opt/homebrew/bin/",
|
||||
"/run/current-system/sw/bin/",
|
||||
];
|
||||
let lastAppliedKeys: string[] = [];
|
||||
let cachedShellPath: string | null | undefined;
|
||||
let cachedEtcShells: Set<string> | null | undefined;
|
||||
@@ -70,21 +63,7 @@ function isTrustedShellPath(shell: string): boolean {
|
||||
|
||||
// Primary trust anchor: shell registered in /etc/shells.
|
||||
const registeredShells = readEtcShells();
|
||||
if (registeredShells?.has(shell)) {
|
||||
return true;
|
||||
}
|
||||
|
||||
// Fallback for environments where /etc/shells is incomplete/unavailable.
|
||||
if (!TRUSTED_SHELL_PREFIXES.some((prefix) => shell.startsWith(prefix))) {
|
||||
return false;
|
||||
}
|
||||
|
||||
try {
|
||||
fs.accessSync(shell, fs.constants.X_OK);
|
||||
return true;
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
return registeredShells?.has(shell) === true;
|
||||
}
|
||||
|
||||
function resolveShell(env: NodeJS.ProcessEnv): string {
|
||||
|
||||
Reference in New Issue
Block a user