Merge 012ad7e69e into sapling-pr-archive-bolinfest

This commit is contained in:
Michael Bolin
2025-04-22 12:56:14 -07:00
committed by GitHub
5 changed files with 64 additions and 13 deletions

View File

@@ -71,13 +71,14 @@ export type ApprovalPolicy =
*/
export function canAutoApprove(
command: ReadonlyArray<string>,
workdir: string | undefined,
policy: ApprovalPolicy,
writableRoots: ReadonlyArray<string>,
env: NodeJS.ProcessEnv = process.env,
): SafetyAssessment {
if (command[0] === "apply_patch") {
return command.length === 2 && typeof command[1] === "string"
? canAutoApproveApplyPatch(command[1], writableRoots, policy)
? canAutoApproveApplyPatch(command[1], workdir, writableRoots, policy)
: {
type: "reject",
reason: "Invalid apply_patch command",
@@ -103,7 +104,12 @@ export function canAutoApprove(
) {
const applyPatchArg = tryParseApplyPatch(command[2]);
if (applyPatchArg != null) {
return canAutoApproveApplyPatch(applyPatchArg, writableRoots, policy);
return canAutoApproveApplyPatch(
applyPatchArg,
workdir,
writableRoots,
policy,
);
}
let bashCmd;
@@ -162,6 +168,7 @@ export function canAutoApprove(
function canAutoApproveApplyPatch(
applyPatchArg: string,
workdir: string | undefined,
writableRoots: ReadonlyArray<string>,
policy: ApprovalPolicy,
): SafetyAssessment {
@@ -179,7 +186,13 @@ function canAutoApproveApplyPatch(
break;
}
if (isWritePatchConstrainedToWritablePaths(applyPatchArg, writableRoots)) {
if (
isWritePatchConstrainedToWritablePaths(
applyPatchArg,
workdir,
writableRoots,
)
) {
return {
type: "auto-approve",
reason: "apply_patch command is constrained to writable paths",
@@ -208,6 +221,7 @@ function canAutoApproveApplyPatch(
*/
function isWritePatchConstrainedToWritablePaths(
applyPatchArg: string,
workdir: string | undefined,
writableRoots: ReadonlyArray<string>,
): boolean {
// `identify_files_needed()` returns a list of files that will be modified or
@@ -222,10 +236,12 @@ function isWritePatchConstrainedToWritablePaths(
return (
allPathsConstrainedTowritablePaths(
identify_files_needed(applyPatchArg),
workdir,
writableRoots,
) &&
allPathsConstrainedTowritablePaths(
identify_files_added(applyPatchArg),
workdir,
writableRoots,
)
);
@@ -233,19 +249,29 @@ function isWritePatchConstrainedToWritablePaths(
function allPathsConstrainedTowritablePaths(
candidatePaths: ReadonlyArray<string>,
workdir: string | undefined,
writableRoots: ReadonlyArray<string>,
): boolean {
return candidatePaths.every((candidatePath) =>
isPathConstrainedTowritablePaths(candidatePath, writableRoots),
isPathConstrainedTowritablePaths(candidatePath, workdir, writableRoots),
);
}
/** If candidatePath is relative, it will be resolved against cwd. */
function isPathConstrainedTowritablePaths(
candidatePath: string,
workdir: string | undefined,
writableRoots: ReadonlyArray<string>,
): boolean {
const candidateAbsolutePath = path.resolve(candidatePath);
let candidateAbsolutePath: string;
if (path.isAbsolute(candidatePath)) {
candidateAbsolutePath = candidatePath;
} else if (workdir != null) {
candidateAbsolutePath = path.resolve(workdir, candidatePath);
} else {
candidateAbsolutePath = path.resolve(candidatePath);
}
return writableRoots.some((writablePath) =>
pathContains(writablePath, candidateAbsolutePath),
);

View File

@@ -254,15 +254,27 @@ const imagePaths = cli.flags.image;
const provider = cli.flags.provider ?? config.provider ?? "openai";
const apiKey = getApiKey(provider);
if (!apiKey) {
// Set of providers that don't require API keys
const NO_API_KEY_REQUIRED = new Set(["ollama"]);
// Skip API key validation for providers that don't require an API key
if (!apiKey && !NO_API_KEY_REQUIRED.has(provider.toLowerCase())) {
// eslint-disable-next-line no-console
console.error(
`\n${chalk.red(`Missing ${provider} API key.`)}\n\n` +
`Set the environment variable ${chalk.bold("OPENAI_API_KEY")} ` +
`Set the environment variable ${chalk.bold(
`${provider.toUpperCase()}_API_KEY`,
)} ` +
`and re-run this command.\n` +
`You can create a key here: ${chalk.bold(
chalk.underline("https://platform.openai.com/account/api-keys"),
)}\n`,
`${
provider.toLowerCase() === "openai"
? `You can create a key here: ${chalk.bold(
chalk.underline("https://platform.openai.com/account/api-keys"),
)}\n`
: `You can create a ${chalk.bold(
`${provider.toUpperCase()}_API_KEY`,
)} ` + `in the ${chalk.bold(`${provider}`)} dashboard.\n`
}`,
);
process.exit(1);
}

View File

@@ -81,7 +81,7 @@ export async function handleExecCommand(
) => Promise<CommandConfirmation>,
abortSignal?: AbortSignal,
): Promise<HandleExecCommandResult> {
const { cmd: command } = args;
const { cmd: command, workdir } = args;
const key = deriveCommandKey(command);
@@ -103,7 +103,7 @@ export async function handleExecCommand(
// working directory so that edits are constrained to the project root. If
// the caller wishes to broaden or restrict the set it can be made
// configurable in the future.
const safety = canAutoApprove(command, policy, [process.cwd()]);
const safety = canAutoApprove(command, workdir, policy, [process.cwd()]);
let runInSandbox: boolean;
switch (safety.type) {

View File

@@ -47,6 +47,13 @@ export function getBaseUrl(provider: string = "openai"): string | undefined {
return OPENAI_BASE_URL;
}
// Check for a PROVIDER-specific override: e.g. OLLAMA_BASE_URL
const envKey = `${provider.toUpperCase()}_BASE_URL`;
if (process.env[envKey]) {
return process.env[envKey];
}
// Use the default URL from providers if available
const providerInfo = providers[provider.toLowerCase()];
if (providerInfo) {
return providerInfo.baseURL;

View File

@@ -11,7 +11,13 @@ describe("canAutoApprove()", () => {
const writeablePaths: Array<string> = [];
const check = (command: ReadonlyArray<string>): SafetyAssessment =>
canAutoApprove(command, "suggest", writeablePaths, env);
canAutoApprove(
command,
/* workdir */ undefined,
"suggest",
writeablePaths,
env,
);
test("simple safe commands", () => {
expect(check(["ls"])).toEqual({