Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stop allowing running MRVA with query outside of workspace #3302

Merged
merged 5 commits into from
Feb 5, 2024
Merged
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
2 changes: 2 additions & 0 deletions extensions/ql-vscode/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

## [UNRELEASED]

- Stop allowing running MRVA with a query outside of the workspace. [#3302](https://github.com/github/vscode-codeql/pull/3302)

## 1.12.1 - 31 January 2024

- Enable collection of telemetry for the `codeQL.addingDatabases.addDatabaseSourceToWorkspace` setting. [#3238](https://github.com/github/vscode-codeql/pull/3238)
Expand Down
7 changes: 6 additions & 1 deletion extensions/ql-vscode/src/common/files.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { pathExists, stat, readdir, opendir } from "fs-extra";
import { pathExists, stat, readdir, opendir, lstatSync } from "fs-extra";
import { dirname, isAbsolute, join, relative, resolve } from "path";
import { tmpdir as osTmpdir } from "os";

Expand Down Expand Up @@ -149,6 +149,11 @@ export function findCommonParentDir(...paths: string[]): string {

paths = paths.map((path) => normalizePath(path));

// If there's only one path and it's a file, return its dirname
if (paths.length === 1) {
return lstatSync(paths[0]).isFile() ? dirname(paths[0]) : paths[0];
}

let commonDir = paths[0];
while (!paths.every((path) => containsPath(commonDir, path))) {
if (isTopLevelPath(commonDir)) {
Expand Down
9 changes: 2 additions & 7 deletions extensions/ql-vscode/src/variant-analysis/ql.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import { dirname } from "path";
import { containsPath, findCommonParentDir } from "../common/files";
import { findPackRoot } from "../common/ql";

Expand All @@ -9,8 +8,8 @@ import { findPackRoot } from "../common/ql";
* - If all query files are in the same QL pack, it returns the root directory of that pack.
* - If the query files are in different QL packs, it throws an error.
* - If some query files are in a QL pack and some aren't, it throws an error.
* - If none of the query files are in a QL pack, it returns the common parent directory of the query files. However,
* if there are more than one query files and they're not in the same workspace folder, it throws an error.
* - If none of the query files are in a QL pack, it returns the common parent directory of the query
* files. However, if the common parent directory is not in a workspace folder, it throws an error.
*
* @param queryFiles - An array of file paths for the query files.
* @param workspaceFolders - An array of workspace folder paths.
Expand All @@ -31,10 +30,6 @@ export async function findVariantAnalysisQlPackRoot(
packRoots.push(packRoot);
}

if (queryFiles.length === 1) {
return packRoots[0] ?? dirname(queryFiles[0]);
}

const uniquePackRoots = Array.from(new Set(packRoots));

if (uniquePackRoots.length > 1) {
Expand Down
27 changes: 19 additions & 8 deletions extensions/ql-vscode/test/unit-tests/common/files.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -541,14 +541,6 @@ describe("findCommonParentDir", () => {
expect(commonDir).toEqualPath(rootDir);
});

it("should handle a single path", async () => {
const paths = [join("/foo", "bar", "baz")];

const commonDir = findCommonParentDir(...paths);

expect(commonDir).toEqualPath(join("/foo", "bar", "baz"));
});

it("should return the same path if all paths are identical", () => {
const paths = [
join("/foo", "bar", "baz"),
Expand Down Expand Up @@ -580,4 +572,23 @@ describe("findCommonParentDir", () => {

expect(commonDir).toEqualPath(rootDir);
});

it("should return the parent dir of a single file", async () => {
const dataDir = join(__dirname, "../../data");
charisk marked this conversation as resolved.
Show resolved Hide resolved
const paths = [dataDir];

const commonDir = findCommonParentDir(...paths);

expect(commonDir).toEqualPath(dataDir);
});

it("should return the dir if a single dir is provided", async () => {
const dataDir = join(__dirname, "../../data");
const filePath = join(dataDir, "query.ql");
const paths = [filePath];

const commonDir = findCommonParentDir(...paths);

expect(commonDir).toEqualPath(dataDir);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -45,15 +45,15 @@ describe("findVariantAnalysisQlPackRoot", () => {
expect(packRoot).toEqualPath(getFullPath("workspace1"));
});

it("should return the pack root of a single query not in a pack or workspace", async () => {
const queryFiles = [getFullPath("dir1/query1.ql")];
it("should fail if single query not in a pack or workspace", async () => {
const queryFiles = [getFullPath("workspace1/query1.ql")];
const workspaceFolders = [getFullPath("workspace2")];

const packRoot = await findVariantAnalysisQlPackRoot(
queryFiles,
workspaceFolders,
await expect(
findVariantAnalysisQlPackRoot(queryFiles, workspaceFolders),
).rejects.toThrow(
"All queries must be within the workspace and within the same workspace root",
);

expect(packRoot).toEqualPath(getFullPath("dir1"));
});

it("should throw an error if some queries are in a pack and some are not", async () => {
Expand Down
Loading