Skip to content

Commit

Permalink
Stop allowing running MRVA with query outside of workspace (#3302)
Browse files Browse the repository at this point in the history
  • Loading branch information
charisk authored Feb 5, 2024
1 parent 7017180 commit bc4d742
Show file tree
Hide file tree
Showing 5 changed files with 36 additions and 23 deletions.
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");
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);
});
});
14 changes: 7 additions & 7 deletions extensions/ql-vscode/test/unit-tests/variant-analysis/ql.test.ts
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

0 comments on commit bc4d742

Please sign in to comment.