Merge pull request #3414 from github/koesie10/verify-valid-sarif

Add check for id property when running variant analysis

extensions/ql-vscode/src/common/query-metadata.ts

@@ -0,0 +1,17 @@
+const SARIF_RESULTS_QUERY_KINDS = [
+  "problem",
+  "alert",
+  "path-problem",
+  "path-alert",
+];
+
+/**
+ * Returns whether this query kind supports producing SARIF results.
+ */
+export function isSarifResultsQueryKind(kind: string | undefined): boolean {
+  if (!kind) {
+    return false;
+  }
+
+  return SARIF_RESULTS_QUERY_KINDS.includes(kind);
+}

extensions/ql-vscode/src/variant-analysis/code-scanning-pack.ts

@@ -4,6 +4,7 @@ import type { QueryLanguage } from "../common/query-language";
 import type { CodeQLCliServer } from "../codeql-cli/cli";
 import type { QlPackDetails } from "./ql-pack-details";
 import { getQlPackFilePath } from "../common/ql";
+import { isSarifResultsQueryKind } from "../common/query-metadata";
 
 export async function resolveCodeScanningQueryPack(
   logger: BaseLogger,
@@ -64,10 +65,7 @@ async function filterToOnlyProblemQueries(
   const problemQueries: string[] = [];
   for (const query of queries) {
     const queryMetadata = await cliServer.resolveMetadata(query);
-    if (
-      queryMetadata.kind === "problem" ||
-      queryMetadata.kind === "path-problem"
-    ) {
+    if (isSarifResultsQueryKind(queryMetadata.kind)) {
       problemQueries.push(query);
     } else {
       void logger.log(`Skipping non-problem query ${query}`);

extensions/ql-vscode/src/variant-analysis/variant-analysis-manager.ts

@@ -96,6 +96,7 @@ import { tryGetQueryMetadata } from "../codeql-cli/query-metadata";
 import { getOnDiskWorkspaceFolders } from "../common/vscode/workspace-folders";
 import { findVariantAnalysisQlPackRoot } from "./ql";
 import { resolveCodeScanningQueryPack } from "./code-scanning-pack";
+import { isSarifResultsQueryKind } from "../common/query-metadata";
 
 const maxRetryCount = 3;
 
@@ -310,21 +311,6 @@ export class VariantAnalysisManager
       message: "Getting credentials",
     });
 
-    const {
-      actionBranch,
-      base64Pack,
-      repoSelection,
-      controllerRepo,
-      queryStartTime,
-    } = await prepareRemoteQueryRun(
-      this.cliServer,
-      this.app.credentials,
-      qlPackDetails,
-      progress,
-      token,
-      this.dbManager,
-    );
-
     // For now we get the metadata for the first query in the pack.
     // and use that in the submission and query history. In the future
     // we'll need to consider how to handle having multiple queries.
@@ -343,6 +329,32 @@ export class VariantAnalysisManager
       );
     }
 
+    // It's not possible to interpret a BQRS file to SARIF without an id property.
+    if (
+      queryMetadata?.kind &&
+      isSarifResultsQueryKind(queryMetadata.kind) &&
+      !queryMetadata.id
+    ) {
+      throw new UserCancellationException(
+        `${firstQueryFile} does not have the required @id property for a ${queryMetadata.kind} query.`,
+      );
+    }
+
+    const {
+      actionBranch,
+      base64Pack,
+      repoSelection,
+      controllerRepo,
+      queryStartTime,
+    } = await prepareRemoteQueryRun(
+      this.cliServer,
+      this.app.credentials,
+      qlPackDetails,
+      progress,
+      token,
+      this.dbManager,
+    );
+
     const queryText = await readFile(firstQueryFile, "utf8");
 
     const queries: VariantAnalysisQueries | undefined =

extensions/ql-vscode/src/view/variant-analysis/RepositoriesSearchSortRow.tsx

@@ -11,6 +11,7 @@ import { RepositoriesSort } from "./RepositoriesSort";
 import { RepositoriesFilter } from "./RepositoriesFilter";
 import { RepositoriesResultFormat } from "./RepositoriesResultFormat";
 import type { ResultFormat } from "../../variant-analysis/shared/variant-analysis-result-format";
+import { isSarifResultsQueryKind } from "../../common/query-metadata";
 
 type Props = {
   filterSortValue: RepositoriesFilterSortState;
@@ -47,10 +48,7 @@ const RepositoriesResultFormatColumn = styled(RepositoriesResultFormat)`
 function showResultFormatColumn(
   variantAnalysisQueryKind: string | undefined,
 ): boolean {
-  return (
-    variantAnalysisQueryKind === "problem" ||
-    variantAnalysisQueryKind === "path-problem"
-  );
+  return isSarifResultsQueryKind(variantAnalysisQueryKind);
 }
 
 export const RepositoriesSearchSortRow = ({