Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions extensions/ql-vscode/src/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -961,3 +961,9 @@ export const AUTOFIX_MODEL = new Setting("model", AUTOFIX_SETTING);
export function getAutofixModel(): string | undefined {
return AUTOFIX_MODEL.getValue<string>() || undefined;
}

export const AUTOFIX_CAPI_DEV_KEY = new Setting("capiDevKey", AUTOFIX_SETTING);

export function getAutofixCapiDevKey(): string | undefined {
return AUTOFIX_CAPI_DEV_KEY.getValue<string>() || undefined;
}
89 changes: 81 additions & 8 deletions extensions/ql-vscode/src/variant-analysis/view-autofixes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,11 @@ import type { VariantAnalysisResultsManager } from "./variant-analysis-results-m
import {
getAutofixPath,
getAutofixModel,
getAutofixCapiDevKey,
downloadTimeout,
AUTOFIX_PATH,
AUTOFIX_MODEL,
AUTOFIX_CAPI_DEV_KEY,
} from "../config";
import { asError, getErrorMessage } from "../common/helpers-pure";
import { createTimeoutSignal } from "../common/fetch-stream";
Expand Down Expand Up @@ -155,6 +157,39 @@ async function findLocalAutofix(): Promise<string> {
return localAutofixPath;
}

/**
* Finds and resolves the Copilot API dev key from the `codeQL.autofix.capiDevKey` setting.
* The key can be specified as an environment variable reference (e.g., `env:MY_ENV_VAR`)
* or a 1Password secret reference (e.g., `op://vault/item/field`). By default, it uses
* the environment variable `CAPI_DEV_KEY`.
*
* @returns The resolved Copilot API dev key.
* @throws Error if the Copilot API dev key is not found or invalid.
*/
async function findCapiDevKey(): Promise<string> {
let capiDevKey = getAutofixCapiDevKey() || "env:CAPI_DEV_KEY";

if (!capiDevKey.startsWith("env:") && !capiDevKey.startsWith("op://")) {
// Don't allow literal keys in config.json for security reasons
throw new Error(
`Invalid CAPI dev key format. Use 'env:<ENV_VAR_NAME>' or 'op://<1PASSWORD_SECRET_REFERENCE>'.`,
);
}
if (capiDevKey.startsWith("env:")) {
const envVarName = capiDevKey.substring("env:".length);
capiDevKey = process.env[envVarName] || "";
}
if (capiDevKey.startsWith("op://")) {
Comment on lines +181 to +182
Copy link

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The logic allows a value that starts with env: but resolves to an empty string to fall through to the 1Password check. If the environment variable doesn't exist (line 180), capiDevKey becomes an empty string, but then line 182 checks if it starts with op://, which will be false. The empty string should trigger the error at line 185-188, but the control flow is confusing. Consider using else if at line 182 to make the mutually exclusive nature of these options clearer and prevent potential confusion.

Suggested change
}
if (capiDevKey.startsWith("op://")) {
} else if (capiDevKey.startsWith("op://")) {

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wanted to allow the option of the environment variable containing an op:// reference.

capiDevKey = await opRead(capiDevKey);
}
if (!capiDevKey) {
throw new Error(
`Copilot API dev key not found. Make sure ${AUTOFIX_CAPI_DEV_KEY.qualifiedName} is set correctly.`,
Copy link

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This error message doesn't provide enough context about why the key wasn't found. Since the key could be missing due to an undefined environment variable or failed 1Password read, consider including which method was attempted (env var name or op:// reference) to help users debug the issue.

Suggested change
`Copilot API dev key not found. Make sure ${AUTOFIX_CAPI_DEV_KEY.qualifiedName} is set correctly.`,
`Copilot API dev key not found. Attempted to retrieve using '${getAutofixCapiDevKey() || "env:CAPI_DEV_KEY"}'. Make sure ${AUTOFIX_CAPI_DEV_KEY.qualifiedName} is set correctly and that the referenced environment variable or 1Password secret exists and is accessible.`,

Copilot uses AI. Check for mistakes.
);
}
return capiDevKey;
}

/**
* Overrides the query help from a given variant analysis
* at a location within the `localAutofixPath` directory .
Expand Down Expand Up @@ -214,7 +249,9 @@ async function overrideQueryHelp(
// Note: the path to this directory may change in the future.
const queryHelpOverrideDirectory = join(
localAutofixPath,
"prompt-templates",
"pkg",
"autofix",
"prompt",
"qhelps",
`${queryIdWithDash}.md`,
);
Expand Down Expand Up @@ -607,9 +644,9 @@ async function runAutofixForRepository(
} = await getRepoStoragePaths(autofixOutputStoragePath, nwo);

// Get autofix binary.
// Switch to Go binary in the future and have user pass full path
// In the future, have user pass full path
// in an environment variable instead of hardcoding part here.
const cocofixBin = join(process.cwd(), localAutofixPath, "bin", "cocofix.js");
const autofixBin = join(process.cwd(), localAutofixPath, "bin", "autofix");

// Limit number of fixes generated.
const limitFixesBoolean: boolean = resultCount > MAX_NUM_FIXES;
Expand Down Expand Up @@ -642,7 +679,7 @@ async function runAutofixForRepository(
transcriptFiles.push(tempTranscriptFilePath);

await runAutofixOnResults(
cocofixBin,
autofixBin,
sarifFile,
srcRootPath,
tempOutputTextFilePath,
Expand All @@ -661,7 +698,7 @@ async function runAutofixForRepository(
} else {
// Run autofix once for all alerts.
await runAutofixOnResults(
cocofixBin,
autofixBin,
sarifFile,
srcRootPath,
outputTextFilePath,
Expand Down Expand Up @@ -707,7 +744,7 @@ async function getRepoStoragePaths(
* Runs autofix on the results in the given SARIF file.
*/
async function runAutofixOnResults(
cocofixBin: string,
autofixBin: string,
sarifFile: string,
srcRootPath: string,
outputTextFilePath: string,
Expand Down Expand Up @@ -751,12 +788,12 @@ async function runAutofixOnResults(
}

await execAutofix(
cocofixBin,
autofixBin,
autofixArgs,
{
cwd: workDir,
env: {
CAPI_DEV_KEY: process.env.CAPI_DEV_KEY,
CAPI_DEV_KEY: await findCapiDevKey(),
PATH: process.env.PATH,
},
},
Expand Down Expand Up @@ -826,6 +863,42 @@ function execAutofix(
});
}

/** Execute the 1Password CLI command `op read <secretReference>`, if the `op` command exists on the PATH. */
async function opRead(secretReference: string): Promise<string> {
return new Promise((resolve, reject) => {
const opProcess = spawn("op", ["read", secretReference], {
stdio: ["ignore", "pipe", "pipe"],
});
Comment on lines +867 to +871
Copy link

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When the op command is not found on the PATH, the error from line 884-885 will be a system error like "ENOENT" which may not be clear to users. Consider catching this specific case and providing a more helpful error message indicating that the 1Password CLI needs to be installed and available on PATH.

Copilot uses AI. Check for mistakes.

let stdoutBuffer = "";
let stderrBuffer = "";

opProcess.stdout?.on("data", (data) => {
stdoutBuffer += data.toString();
});

opProcess.stderr?.on("data", (data) => {
stderrBuffer += data.toString();
});

opProcess.on("error", (error) => {
reject(error);
});

opProcess.on("exit", (code) => {
if (code === 0) {
resolve(stdoutBuffer.trim());
} else {
reject(
new Error(
`1Password CLI exited with code ${code}. Stderr: ${stderrBuffer.trim()}`,
),
);
}
});
});
}

/**
* Creates a new file path by appending the given suffix.
* @param filePath The original file path.
Expand Down