Merge pull request #4223 from github/mbg/diagnostics/failed-sarif

Include diagnostics in failed SARIF if they were created before a database was initialised
This commit is contained in:
Michael B. Gale
2026-10-22 13:38:57 +00:00
committed by GitHub
20 changed files with 1458 additions and 621 deletions

143
.github/workflows/__diagnostics-without-db.yml generated vendored Normal file
View File

@@ -0,0 +1,143 @@
# Warning: This file is generated automatically, and should not be modified.
# Instead, please modify the template in the pr-checks directory and run:
# pr-checks/sync.sh
# to regenerate this file.
name: PR Check - Diagnostic export without database
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
GO111MODULE: auto
on:
push:
branches:
- main
- releases/v*
pull_request: {}
merge_group:
types:
- checks_requested
schedule:
- cron: '0 5 * * *'
workflow_dispatch:
inputs: {}
workflow_call:
inputs: {}
defaults:
run:
shell: bash
concurrency:
cancel-in-progress: ${{ github.event_name == 'pull_request' || false }}
group: diagnostics-without-db-${{github.ref}}
jobs:
diagnostics-without-db:
strategy:
fail-fast: false
matrix:
include:
- os: ubuntu-latest
version: linked
name: Diagnostic export without database
if: github.triggering_actor != 'dependabot[bot]'
permissions:
contents: read
security-events: read
timeout-minutes: 45
runs-on: ${{ matrix.os }}
steps:
# We deliberately need to make this step fail before the database is initialised, but after
# some diagnostic has been produced. The simplest way to do that right now is to:
#
# - Produce a diagnostic before the db is initialised by forcing the use of a nightly release
# with the `CODEQL_ACTION_FORCE_NIGHTLY` feature flag.
# - Cause `init` to abort because we are attempting to analyse Swift on a non-macOS runner.
#
- name: Check out repository
uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
- name: Install Node.js
uses: actions/setup-node@949feb2413d6458794dcd2491c4babbbce0c15c1 # v7.1.0
with:
node-version: 20.x
cache: npm
- name: Install newer npm
run: npm install -g npm@11.19.1
- name: Install dependencies
run: npm ci
- name: Prepare test
id: prepare-test
uses: ./.github/actions/prepare-test
with:
version: ${{ matrix.version }}
use-all-platform-bundle: 'false'
setup-kotlin: 'true'
- name: Initialise failing analysis
id: init
uses: ./../action/init
continue-on-error: true
env:
CODEQL_ACTION_FORCE_NIGHTLY: true
with:
languages: swift
tools: ${{ steps.prepare-test.outputs.tools-url }}
- name: Analyze
uses: ./analyze
# In a real workflow, this step wouldn't run. Since we used `continue-on-error`
# above, we manually disable it with an `if` condition. Having this step is required
# for the init post-action because it checks the workflow definition for e.g. the
# `category` input. Without this step, that part of the init post-action would fail.
if: false
with:
category: '/test-codeql-version:${{ matrix.version }}'
- name: Check diagnostics appear on disk
env:
DIAGNOSTICS_DIR: '${{ runner.temp }}/codeql-action-temp-diagnostics/'
run: |
cd ../action
npx tsx ./pr-checks/diagnostics-without-db.ts
env:
CODEQL_ACTION_SKIP_GENERATE_FAILED_SARIF: false
CODEQL_ACTION_SKIP_SARIF_UPLOAD: true
CODEQL_ACTION_TESTING_ENVIRONMENT: codeql-action-pr-checks
CODEQL_ACTION_TEST_MODE: true
diagnostics-without-db-validation:
if: github.triggering_actor != 'dependabot[bot]'
needs:
- diagnostics-without-db
permissions:
contents: read
security-events: read
timeout-minutes: 5
runs-on: ubuntu-latest
steps:
- name: Check out repository
uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
- name: Install Node.js
uses: actions/setup-node@949feb2413d6458794dcd2491c4babbbce0c15c1 # v7.1.0
with:
node-version: 20.x
cache: npm
- name: Install newer npm
run: npm install -g npm@11.19.1
- name: Install dependencies
run: npm ci
- name: Download workflow artifact
uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1
with:
pattern: test-mode-sarif-*
path: sarif
- name: List artifact contents
run: ls -laR ./sarif
- name: Check diagnostics appear in SARIF
env:
SARIF_PATH: "./sarif/codeql-failed-run.sarif"
DIAGNOSTIC_ID: "codeql-action/forced-nightly-cli"
DIAGNOSTIC_NAME: "A nightly release of CodeQL was used"
EXPECTED_LOCATIONS: "0"
EXPECTED_MESSAGE: |
GitHub configured this analysis to use a nightly release of CodeQL to allow you to preview changes from an upcoming release.
run: npx tsx ./pr-checks/check-diagnostics.ts
env:
CODEQL_ACTION_TEST_MODE: true

View File

@@ -71,7 +71,9 @@ jobs:
run: exit 1
- uses: ./analyze
# In a real workflow, this step wouldn't run. Since we used `continue-on-error`
# above, we manually disable it with an `if` condition.
# above, we manually disable it with an `if` condition. Having this step is required
# for the init post-action because it checks the workflow definition for e.g. the
# `category` input. Without this step, that part of the init post-action would fail.
if: false
with:
category: '/test-codeql-version:${{ matrix.version }}'

969
lib/entry-points.js generated

File diff suppressed because it is too large Load Diff

View File

@@ -4,16 +4,32 @@ import * as core from "@actions/core";
import * as sarif from "../src/sarif";
function checkStatusPageNotification(n) {
const expectedMessage = "Plaintext message";
if (n.message.text !== expectedMessage) {
core.setFailed(
`Expected the status page diagnostic to have the message '${expectedMessage}', but found '${n.message.text}'.`,
);
const DIAGNOSTIC_ID =
process.env["DIAGNOSTIC_ID"] ?? "lang/diagnostics/example";
const DIAGNOSTIC_NAME = process.env["DIAGNOSTIC_NAME"] ?? "Diagnostic name";
const EXPECTED_MESSAGE = process.env["EXPECTED_MESSAGE"] ?? "Plaintext message";
const EXPECTED_LOCATIONS = Number.parseInt(
process.env["EXPECTED_LOCATIONS"] ?? "1",
);
function checkStatusPageNotification(n: sarif.Notification) {
const expectedMessage = EXPECTED_MESSAGE.trim();
if (!n.message.text?.includes(expectedMessage)) {
if (n.message.text !== undefined) {
core.setFailed(
`Expected the status page diagnostic to contain the message '${expectedMessage}', but found '${n.message.text.replaceAll("\n", "\\n")}'.`,
);
} else {
core.setFailed(
`Expected the status page diagnostic to contain the message '${expectedMessage}', but found nothing.`,
);
}
}
if (n.locations.length !== 1) {
const locationCount = n.locations?.length ?? 0;
if (locationCount !== EXPECTED_LOCATIONS) {
core.setFailed(
`Expected the status page diagnostic to have exactly 1 location, but found ${n.locations.length}.`,
`Expected the status page diagnostic to have exactly ${EXPECTED_LOCATIONS} location(s), but found ${locationCount}.`,
);
}
}
@@ -40,8 +56,7 @@ if (toolExecutionNotifications === undefined) {
const statusPageNotifications = toolExecutionNotifications.filter(
(n) =>
n.descriptor?.id === "lang/diagnostics/example" &&
n.properties?.visibility?.statusPage,
n.descriptor?.id === DIAGNOSTIC_ID && n.properties?.visibility?.statusPage,
);
if (statusPageNotifications.length !== 1) {
core.setFailed(
@@ -61,9 +76,9 @@ if (notifications === undefined) {
const diagnosticNotification = notifications.filter(
(n) =>
n.id === "lang/diagnostics/example" &&
n.name === "lang/diagnostics/example" &&
n.fullDescription?.text === "Diagnostic name",
n.id === DIAGNOSTIC_ID &&
n.name === DIAGNOSTIC_ID &&
n.fullDescription?.text === DIAGNOSTIC_NAME,
);
if (diagnosticNotification.length !== 1) {
core.setFailed(

View File

@@ -0,0 +1,72 @@
name: "Diagnostic export without database"
description: "Tests that temporary diagnostics are correctly included in the failed SARIF."
versions:
# Overridden by the FF in the `init` step below.
- linked
installNode: true
env:
# Allow the failed SARIF file to be generated.
CODEQL_ACTION_SKIP_GENERATE_FAILED_SARIF: false
# But don't actually upload it.
CODEQL_ACTION_SKIP_SARIF_UPLOAD: true
# Mark telemetry for this workflow so it can be treated separately.
CODEQL_ACTION_TESTING_ENVIRONMENT: codeql-action-pr-checks
steps:
# We deliberately need to make this step fail before the database is initialised, but after
# some diagnostic has been produced. The simplest way to do that right now is to:
#
# - Produce a diagnostic before the db is initialised by forcing the use of a nightly release
# with the `CODEQL_ACTION_FORCE_NIGHTLY` feature flag.
# - Cause `init` to abort because we are attempting to analyse Swift on a non-macOS runner.
#
- name: Initialise failing analysis
id: init
uses: ./../action/init
continue-on-error: true
env:
CODEQL_ACTION_FORCE_NIGHTLY: true
with:
languages: swift
tools: ${{ steps.prepare-test.outputs.tools-url }}
- name: Analyze
uses: ./analyze
# In a real workflow, this step wouldn't run. Since we used `continue-on-error`
# above, we manually disable it with an `if` condition. Having this step is required
# for the init post-action because it checks the workflow definition for e.g. the
# `category` input. Without this step, that part of the init post-action would fail.
if: false
with:
category: "/test-codeql-version:${{ matrix.version }}"
- name: Check diagnostics appear on disk
env:
DIAGNOSTICS_DIR: "${{ runner.temp }}/codeql-action-temp-diagnostics/"
run: |
cd ../action
npx tsx ./pr-checks/diagnostics-without-db.ts
validationJobs:
diagnostics-without-db-validation:
checkout: true
installNode: true
steps:
- name: Download workflow artifact
uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1
with:
pattern: test-mode-sarif-*
path: sarif
- name: List artifact contents
run: ls -laR ./sarif
- name: Check diagnostics appear in SARIF
env:
SARIF_PATH: "./sarif/codeql-failed-run.sarif"
DIAGNOSTIC_ID: "codeql-action/forced-nightly-cli"
DIAGNOSTIC_NAME: "A nightly release of CodeQL was used"
EXPECTED_LOCATIONS: "0"
EXPECTED_MESSAGE: |
GitHub configured this analysis to use a nightly release of CodeQL to allow you to preview changes from an upcoming release.
run: npx tsx ./pr-checks/check-diagnostics.ts

View File

@@ -35,7 +35,9 @@ steps:
run: exit 1
- uses: ./analyze
# In a real workflow, this step wouldn't run. Since we used `continue-on-error`
# above, we manually disable it with an `if` condition.
# above, we manually disable it with an `if` condition. Having this step is required
# for the init post-action because it checks the workflow definition for e.g. the
# `category` input. Without this step, that part of the init post-action would fail.
if: false
with:
category: "/test-codeql-version:${{ matrix.version }}"

View File

@@ -0,0 +1,48 @@
import * as fs from "fs";
import * as path from "path";
import * as core from "@actions/core";
import type * as diagnostics from "../src/diagnostics";
const DIAGNOSTIC_ID = "codeql-action/forced-nightly-cli";
const diagnosticDirectory = process.env["DIAGNOSTICS_DIR"];
if (diagnosticDirectory === undefined) {
throw new Error("DIAGNOSTICS_DIR is undefined.");
}
const paths = fs
.readdirSync(diagnosticDirectory, {
recursive: true,
encoding: "utf-8",
})
.filter((p) => p.endsWith(".json"));
core.info(`Found ${paths.length} paths in '${diagnosticDirectory}'.`);
core.info(`Searching for a diagnostic with id '${DIAGNOSTIC_ID}'...`);
let found = false;
for (const diagnosticPath of paths) {
if (path.extname(diagnosticPath) !== ".json") {
core.info(`Skipping ${diagnosticPath}.`);
continue;
}
core.info(`Found ${diagnosticPath}.`);
const contents = JSON.parse(
fs.readFileSync(path.resolve(diagnosticDirectory, diagnosticPath), "utf-8"),
) as diagnostics.DiagnosticMessage;
if (contents.source.id === DIAGNOSTIC_ID) {
core.info(
`Found '${DIAGNOSTIC_ID}' in '${diagnosticPath}'. Skipping remaining files.`,
);
found = true;
break;
}
}
if (!found) {
throw new Error(`Didn't find '${DIAGNOSTIC_ID}' diagnostic.`);
}

View File

@@ -90,7 +90,7 @@ interface Specification extends JobSpecification {
services?: any;
/** Additional jobs to run after the main PR check job. */
validationJobs?: Record<string, JobSpecification>;
validationJobs?: Record<string, ValidationJobSpecification>;
/** If set, this check is part of a named collection that gets its own caller workflow. */
collection?: string;
@@ -122,6 +122,10 @@ interface JobSpecification {
installYq?: boolean;
}
type ValidationJobSpecification = JobSpecification & {
checkout?: boolean;
};
/** Describes language/framework-specific steps and inputs. */
interface LanguageSetup {
specProperty: keyof JobSpecification;
@@ -462,6 +466,20 @@ function generateJobMatrix(
return matrix;
}
/**
* Gets the `actions/checkout` step.
*/
function getCheckoutStep(): Step {
return {
name: "Check out repository",
uses: pinnedUses(
"actions/checkout",
"3d3c42e5aac5ba805825da76410c181273ba90b1",
"v7.0.1",
),
};
}
/**
* Retrieves setup steps and additional input definitions based on specific languages or frameworks
* that are requested by the `checkSpecification`.
@@ -531,14 +549,7 @@ function generateJob(
// Construct the workflow steps needed for this check.
const steps: Step[] = [
{
name: "Check out repository",
uses: pinnedUses(
"actions/checkout",
"3d3c42e5aac5ba805825da76410c181273ba90b1",
"v7.0.1",
),
},
getCheckoutStep(),
...setupInfo.steps,
{
name: "Prepare test",
@@ -609,13 +620,16 @@ function generateJob(
/** Generates a validation job. */
function generateValidationJob(
specDocument: yaml.Document,
jobSpecification: JobSpecification,
jobSpecification: ValidationJobSpecification,
checkName: string,
name: string,
) {
// Determine which languages or frameworks have to be installed.
const { inputs, steps } = getSetupSteps(jobSpecification);
// Construct a checkout step if requested.
const checkoutStep = jobSpecification.checkout ? [getCheckoutStep()] : [];
// Extract the sequence of steps from the YAML document to persist as much formatting as possible.
const specSteps = specDocument.getIn([
"validationJobs",
@@ -624,7 +638,7 @@ function generateValidationJob(
]) as yaml.YAMLSeq;
// Add the generated steps in front of the ones from the specification.
specSteps.items.unshift(...steps);
specSteps.items.unshift(...checkoutStep, ...steps);
const validationJob: Record<string, any> = {
name: jobSpecification.name,
@@ -635,7 +649,7 @@ function generateValidationJob(
"security-events": "read",
},
"timeout-minutes": 5,
"runs-on": "ubuntu-slim",
"runs-on": "ubuntu-latest",
steps: specSteps,
};

View File

@@ -199,6 +199,7 @@ export interface CodeQL {
sarifFile: string,
automationDetailsId: string | undefined,
config: ActionConfig<"tempDir">,
diagnosticDir: string | undefined,
): Promise<void>;
/** Get the location of an extractor for the specified language. */
resolveExtractor(language: Language): Promise<string>;
@@ -903,15 +904,43 @@ export async function getCodeQLForCmd(
sarifFile: string,
automationDetailsId: string | undefined,
config: ActionConfig<"tempDir">,
diagnosticDir: string | undefined,
): Promise<void> {
// We may be exporting diagnostics for a failed SARIF, in which case the configuration
// file may not have been written to disk and there may not be a database. The CLI
// will produce a SARIF for diagnostics from `diagnosticDir` without it. Therefore,
// we check whether `configPath` exists and only provide it to the CLI if it does.
// Although not needed, the CLI will produce an error if it
// is provided with an argument to `--sarif-codescanning-config` that does not exist.
let configPath: string | undefined =
getGeneratedCodeScanningConfigPath(config);
if (!fs.existsSync(configPath)) {
// Warn if there is no configuration file, even though `config` claims that `init`
// was successful. This shouldn't be happening.
if (config.successfullyInitialised) {
logger.warning(
[
`Expected a configuration file at '${configPath}', which does not exist.`,
"Continuing with diagnostics export anyway, without a configuration file.",
].join(" "),
);
}
configPath = undefined;
}
const args = [
"diagnostics",
"export",
"--format=sarif-latest",
`--output=${sarifFile}`,
`--sarif-codescanning-config=${getGeneratedCodeScanningConfigPath(
config,
)}`,
...(configPath !== undefined
? [`--sarif-codescanning-config=${configPath}`]
: []),
...(diagnosticDir !== undefined
? [`--diagnostic-dir=${diagnosticDir}`]
: []),
...getExtraOptionsFromEnv(["diagnostics", "export"]),
];
if (automationDetailsId !== undefined) {

View File

@@ -465,6 +465,8 @@ export async function initActionState(
overlayModeSetExplicitly: false,
repositoryProperties,
enableFileCoverageInformation,
// We don't set this to `true` until `init` is successful.
successfullyInitialised: false,
};
}

View File

@@ -130,6 +130,9 @@ export interface Config {
* Whether to enable file coverage information.
*/
enableFileCoverageInformation: boolean;
/** Whether the analysis was successfully initialised using the configuration. */
successfullyInitialised: boolean;
}
/** Properties available in `Config`. */

View File

@@ -8,8 +8,9 @@ import {
LoggedMessage,
makeMacro,
RecordingLogger,
setupActionsVars,
} from "../testing-utils";
import { ConfigurationError, prettyPrintPack } from "../util";
import { ConfigurationError, prettyPrintPack, withTmpDir } from "../util";
import * as dbConfig from "./db-config";
@@ -561,69 +562,84 @@ test("mergeDefaultSetupAndUserConfigs - keeps other properties from user-supplie
t.deepEqual(result, configFile);
});
test("mergeDefaultSetupAndUserConfigs - ignores, but warns about, unknown keys from Default Setup", async (t) => {
const logger = new RecordingLogger();
const configFile: dbConfig.UserConfig = {
"query-filters": [{ exclude: { a: "b" } }],
"paths-ignore": ["path"],
};
test.serial(
"mergeDefaultSetupAndUserConfigs - ignores, but warns about, unknown keys from Default Setup",
async (t) => {
const logger = new RecordingLogger();
const configFile: dbConfig.UserConfig = {
"query-filters": [{ exclude: { a: "b" } }],
"paths-ignore": ["path"],
};
const result = dbConfig.mergeDefaultSetupAndUserConfigs(
logger,
{
"default-setup": {
borg: [],
org: {
unknown: "foo",
"model-packs": [],
await withTmpDir(async (tmpDir) => {
setupActionsVars(tmpDir, tmpDir);
const result = dbConfig.mergeDefaultSetupAndUserConfigs(
logger,
{
"default-setup": {
borg: [],
org: {
unknown: "foo",
"model-packs": [],
},
} as unknown as dbConfig.DefaultSetupConfig,
"paths-ignore": ["other-path"],
},
} as unknown as dbConfig.DefaultSetupConfig,
"paths-ignore": ["other-path"],
},
configFile,
);
configFile,
);
t.deepEqual(result, {
...configFile,
"default-setup": { org: { "model-packs": [] } },
});
t.deepEqual(result, {
...configFile,
"default-setup": { org: { "model-packs": [] } },
});
const expectedUnrecognisedKeys = [
".default-setup.org.unknown",
".default-setup.borg",
".paths-ignore",
].join(", ");
checkExpectedLogMessages(t, logger.messages, [
`Unrecognised keys in Default Setup configuration: ${expectedUnrecognisedKeys}`,
]);
});
const expectedUnrecognisedKeys = [
".default-setup.org.unknown",
".default-setup.borg",
".paths-ignore",
].join(", ");
checkExpectedLogMessages(t, logger.messages, [
`Unrecognised keys in Default Setup configuration: ${expectedUnrecognisedKeys}`,
]);
});
},
);
test("mergeDefaultSetupAndUserConfigs - warns about invalid keys from Default Setup", async (t) => {
const logger = new RecordingLogger();
const configFile: dbConfig.UserConfig = {};
test.serial(
"mergeDefaultSetupAndUserConfigs - warns about invalid keys from Default Setup",
async (t) => {
const logger = new RecordingLogger();
const configFile: dbConfig.UserConfig = {};
const result = dbConfig.mergeDefaultSetupAndUserConfigs(
logger,
{
"default-setup": {
org: {
"model-packs": [123],
await withTmpDir(async (tmpDir) => {
setupActionsVars(tmpDir, tmpDir);
const result = dbConfig.mergeDefaultSetupAndUserConfigs(
logger,
{
"default-setup": {
org: {
"model-packs": [123],
},
} as unknown as dbConfig.DefaultSetupConfig,
},
} as unknown as dbConfig.DefaultSetupConfig,
},
configFile,
);
configFile,
);
t.deepEqual(result, {
...configFile,
"default-setup": { org: { "model-packs": [123] } },
});
t.deepEqual(result, {
...configFile,
"default-setup": { org: { "model-packs": [123] } },
});
const expectedInvalidKeys = [".default-setup.org.model-packs[0]"].join(", ");
checkExpectedLogMessages(t, logger.messages, [
`Invalid keys in Default Setup configuration: ${expectedInvalidKeys}`,
]);
});
const expectedInvalidKeys = [".default-setup.org.model-packs[0]"].join(
", ",
);
checkExpectedLogMessages(t, logger.messages, [
`Invalid keys in Default Setup configuration: ${expectedInvalidKeys}`,
]);
});
},
);
/** Parses `contents` as a configuration without validating it. */
function parseUnvalidatedConfig(contents: string): dbConfig.UserConfig {

165
src/diagnostics.test.ts Normal file
View File

@@ -0,0 +1,165 @@
import * as fs from "fs";
import path from "path";
import test, { type ExecutionContext } from "ava";
import * as diagnostics from "./diagnostics";
import { BuiltInLanguage } from "./languages";
import {
checkExpectedLogMessages,
createTestConfig,
initAllState,
RecordingLogger,
setupTests,
} from "./testing-utils";
import { withTmpDir } from "./util";
setupTests(test);
const id = "codeql-action/test-diagnostic";
const name = "Test title";
test("makeDiagnostic - adds expected properties", (t) => {
const diagnostic = diagnostics.makeDiagnostic(id, name, {});
t.truthy(diagnostic.timestamp);
t.is(diagnostic.source.id, id);
t.is(diagnostic.source.name, name);
});
test("getLanguageDiagnosticsPath gets language-specific path", async (t) =>
withTmpDir(async (tmpDir) => {
for (const language of Object.values(BuiltInLanguage)) {
t.is(
diagnostics.getLanguageDiagnosticsPath(tmpDir, language),
path.resolve(
tmpDir,
language,
...diagnostics.CODEQL_ACTION_DIAGNOSTIC_SUBDIR_PARTS,
),
);
}
}));
test("getTempLanguageDiagnosticsPath returns path without language", async (t) =>
withTmpDir(async (tmpDir) => {
t.is(
diagnostics.getTempLanguageDiagnosticsPath(tmpDir),
path.resolve(
tmpDir,
diagnostics.TEMP_DIAGNOSTIC_DIR_NAME,
...diagnostics.CODEQL_ACTION_DIAGNOSTIC_SUBDIR_PARTS,
),
);
}));
test("getTempLanguageDiagnosticsPath returns language-specific path if a language is provided", async (t) =>
withTmpDir(async (tmpDir) => {
t.is(
diagnostics.getTempLanguageDiagnosticsPath(tmpDir, BuiltInLanguage.swift),
path.resolve(
tmpDir,
diagnostics.TEMP_DIAGNOSTIC_DIR_NAME,
BuiltInLanguage.swift,
...diagnostics.CODEQL_ACTION_DIAGNOSTIC_SUBDIR_PARTS,
),
);
}));
test("getDatabaseDiagnosticsPath returns language-specific path", async (t) =>
withTmpDir(async (tmpDir) => {
t.is(
diagnostics.getDatabaseDiagnosticsPath(
createTestConfig({ dbLocation: tmpDir }),
BuiltInLanguage.swift,
),
path.resolve(
tmpDir,
BuiltInLanguage.swift,
...diagnostics.CODEQL_ACTION_DIAGNOSTIC_SUBDIR_PARTS,
),
);
}));
/**
* Diagnostic files that are written to disk don't have deterministic names.
* This function recursively finds all `.json` files at {@link rootPath},
* asserts that there is exactly one, and that it matches {@link expected}.
*
* @param rootPath The path to start searching from.
*/
function assertSingleDiagnosticIn(
t: ExecutionContext<unknown>,
rootPath: string,
expected: diagnostics.DiagnosticMessage,
) {
// Find all `.json` files in `rootPath` and check that there is exactly one.
const filePaths = fs
.readdirSync(rootPath, {
recursive: true,
encoding: "utf-8",
})
.filter((p) => p.endsWith(".json"));
t.is(filePaths.length, 1);
// Check that the written diagnostic matches the one we added.
const contents = fs.readFileSync(
path.resolve(rootPath, filePaths[0]),
"utf-8",
);
const readDiagnostic = JSON.parse(contents) as diagnostics.DiagnosticMessage;
t.deepEqual(readDiagnostic, expected);
}
test("addDiagnostic writes temporary diagnostics and flushDiagnostics moves them to the database", async (t) =>
withTmpDir(async (tmpDir) => {
const logger = new RecordingLogger();
const tempDiagDir = path.resolve(tmpDir, "temp");
const testDbDir = path.resolve(tmpDir, "test", "database");
const diagnostic = diagnostics.makeDiagnostic(id, name, {});
// Write the diagnostic. The path provided for `dbLocation` doesn't exist,
// so the diagnostic will be written to a temporary location.
t.notThrows(() => {
diagnostics.addDiagnostic(
createTestConfig({ tempDir: tempDiagDir, dbLocation: testDbDir }),
BuiltInLanguage.actions,
diagnostic,
logger,
);
});
// Check that `tempDiagDir` was created.
t.true(fs.existsSync(tempDiagDir));
// Check that the diagnostic exists under `tempDiagDir`.
assertSingleDiagnosticIn(t, tempDiagDir, diagnostic);
// Flush the diagnostics. This will cause the diagnostic we wrote previously to
// be moved to a location under `dbLocation`.
t.notThrows(() => {
diagnostics.flushDiagnostics(
initAllState({ logger }),
createTestConfig({
tempDir: tempDiagDir,
dbLocation: testDbDir,
}),
);
});
// Check that `testDbDir` was created.
t.true(fs.existsSync(testDbDir));
// Check that the diagnostic exists under `testDbDir`.
assertSingleDiagnosticIn(t, testDbDir, diagnostic);
// Check that we logged that the database directory doesn't exist yet
// and that we moved one diagnostic.
checkExpectedLogMessages(t, logger.messages, [
`but the database at ${path.join(testDbDir, "actions")} does not exist yet`,
"Moving 1 diagnostic(s) to their respective databases.",
]);
}));

View File

@@ -1,10 +1,24 @@
import { existsSync, mkdirSync, writeFileSync } from "fs";
import * as fs from "fs";
import path from "path";
import type { ActionState } from "./action-common";
import { getTemporaryDirectory } from "./actions-util";
import type { Config } from "./config-utils";
import { Language } from "./languages";
import { getActionsLogger } from "./logging";
import { getCodeQLDatabasePath } from "./util";
import { getActionsLogger, type Logger } from "./logging";
import {
getCodeQLDatabasePath,
getErrorMessage,
isErrnoException,
} from "./util";
/** The name of the directory we use to temporarily store diagnostics. */
export const TEMP_DIAGNOSTIC_DIR_NAME = "codeql-action-temp-diagnostics";
export const CODEQL_ACTION_DIAGNOSTIC_SUBDIR_PARTS = [
"diagnostic",
"codeql-action",
];
/**
* Known tags for diagnostics. There is currently only "internal-error",
@@ -84,22 +98,16 @@ export type DiagnosticMessage = DiagnosticMessageOptions & {
source: DiagnosticSource;
};
/** Represents a diagnostic message that has not yet been written to the database. */
interface UnwrittenDiagnostic {
/** The diagnostic message that has not yet been written. */
diagnostic: DiagnosticMessage;
/** The language the diagnostic is for. */
language: Language;
/** Represents a diagnostic message that has not yet been saved to the database. */
interface TemporaryDiagnostic {
/** The path to which the diagnostic has temporarily been written to. */
path: string;
/** The language the diagnostic is for, if any. */
language?: Language;
}
/** A list of diagnostics which have not yet been written to disk. */
let unwrittenDiagnostics: UnwrittenDiagnostic[] = [];
/**
* A list of diagnostics which have not yet been written to disk,
* and where the language does not matter.
*/
let unwrittenDefaultLanguageDiagnostics: DiagnosticMessage[] = [];
/** A list of diagnostics which have not yet been saved to the database. */
let temporaryDiagnostics: TemporaryDiagnostic[] = [];
/**
* Counter used to generate a unique suffix for each diagnostic filename, so that
@@ -128,6 +136,51 @@ export function makeDiagnostic(
};
}
/**
* Gets a path relative to {@link basePath} where diagnostics for {@link language} should be stored at.
*/
export function getLanguageDiagnosticsPath(
basePath: string,
language: Language,
) {
return path.resolve(
basePath,
language,
...CODEQL_ACTION_DIAGNOSTIC_SUBDIR_PARTS,
);
}
/**
* Gets a path relative to {@link tmpDir} where diagnostics should temporarily be stored
* until they can be moved to the database.
*/
export function getTempDiagnosticsRoot(tmpDir: string): string {
return path.resolve(tmpDir, TEMP_DIAGNOSTIC_DIR_NAME);
}
/**
* Gets a path relative to {@link tmpDir} where diagnostics for {@link language}
* can temporarily be stored at before the database is initialised. If {@link language}
* is `undefined`, then a non-language-specific path is returned.
*/
export function getTempLanguageDiagnosticsPath(
tmpDir: string,
language?: Language,
) {
const root = getTempDiagnosticsRoot(tmpDir);
if (language !== undefined) {
return getLanguageDiagnosticsPath(root, language);
}
return path.resolve(root, ...CODEQL_ACTION_DIAGNOSTIC_SUBDIR_PARTS);
}
/** Gets the path where diagnostics for {@link language} should be stored in the database. */
export function getDatabaseDiagnosticsPath(config: Config, language: Language) {
return getLanguageDiagnosticsPath(config.dbLocation, language);
}
/**
* Adds the given diagnostic to the database. If the database does not yet exist,
* the diagnostic will be written to it once it has been created.
@@ -140,22 +193,23 @@ export function addDiagnostic(
config: Config,
language: Language,
diagnostic: DiagnosticMessage,
logger: Logger = getActionsLogger(),
) {
const logger = getActionsLogger();
const databasePath = language
? getCodeQLDatabasePath(config, language)
: config.dbLocation;
// Check that the database exists before writing to it. If the database does not yet exist,
// store the diagnostic in memory and write it later.
if (existsSync(databasePath)) {
writeDiagnostic(config, language, diagnostic);
// store the diagnostic in a temporary location and move it to the database later.
if (fs.existsSync(databasePath)) {
writeDiagnostic({ logger }, config, language, diagnostic);
} else {
logger.debug(
`Writing a diagnostic for ${language}, but the database at ${databasePath} does not exist yet.`,
);
unwrittenDiagnostics.push({ diagnostic, language });
// Write the diagnostic to a temporary location.
writeTemporaryDiagnosticFile(logger, config.tempDir, language, diagnostic);
}
}
@@ -163,6 +217,7 @@ export function addDiagnostic(
export function addNoLanguageDiagnostic(
config: Config | undefined,
diagnostic: DiagnosticMessage,
logger: Logger = getActionsLogger(),
) {
if (config !== undefined) {
addDiagnostic(
@@ -173,10 +228,74 @@ export function addNoLanguageDiagnostic(
diagnostic,
);
} else {
unwrittenDefaultLanguageDiagnostics.push(diagnostic);
logger.debug(
`Writing a diagnostic not associated with any language, but we have no configuration to tell us where the database is yet.`,
);
// Write the diagnostic to a temporary location.
writeTemporaryDiagnosticFile(
logger,
getTemporaryDirectory(),
undefined,
diagnostic,
);
}
}
/**
* Writes {@link diagnostic} for an optional {@link language} to a subdirectory of {@link tempDir}
* and tracks it as a diagnostic that was written to a temporary location.
*/
function writeTemporaryDiagnosticFile(
logger: Logger,
tempDir: string,
language: Language | undefined,
diagnostic: DiagnosticMessage,
) {
const tempDiagnosticsPath = getTempLanguageDiagnosticsPath(tempDir, language);
try {
// Write the diagnostic to a temporary location.
const diagnosticPath = writeDiagnosticFile(tempDiagnosticsPath, diagnostic);
// Track the temporary file.
temporaryDiagnostics.push({ path: diagnosticPath, language });
} catch (err) {
logger.warning(
`Unable to write temporary diagnostic to '${tempDiagnosticsPath}': ${getErrorMessage(err)}\n\n${JSON.stringify(diagnostic)}`,
);
}
}
/**
* Writes {@link diagnostic} to a file in {@link diagnosticsPath}.
*/
function writeDiagnosticFile(
diagnosticsPath: string,
diagnostic: DiagnosticMessage,
): string {
// Create the directory if it doesn't exist yet.
fs.mkdirSync(diagnosticsPath, { recursive: true });
// Include a monotonically increasing suffix to avoid filename collisions
// between diagnostics produced within the same millisecond.
const uniqueSuffix = (diagnosticCounter++).toString();
// We should only need to remove colons, but to be defensive, only allow a restricted set of
// characters.
const sanitizedTimestamp = diagnostic.timestamp.replace(
/[^a-zA-Z0-9.-]/g,
"",
);
const jsonPath = path.resolve(
diagnosticsPath,
`codeql-action-${sanitizedTimestamp}-${uniqueSuffix}.json`,
);
fs.writeFileSync(jsonPath, JSON.stringify(diagnostic));
return jsonPath;
}
/**
* Writes the given diagnostic to the database.
*
@@ -185,78 +304,94 @@ export function addNoLanguageDiagnostic(
* @param diagnostic The diagnostic message to add to the database.
*/
function writeDiagnostic(
action: ActionState<["Logger"]>,
config: Config,
language: Language | undefined,
language: Language,
diagnostic: DiagnosticMessage,
) {
const logger = getActionsLogger();
const databasePath = language
? getCodeQLDatabasePath(config, language)
: config.dbLocation;
const diagnosticsPath = path.resolve(
databasePath,
"diagnostic",
"codeql-action",
);
const diagnosticsPath = getDatabaseDiagnosticsPath(config, language);
try {
// Create the directory if it doesn't exist yet.
mkdirSync(diagnosticsPath, { recursive: true });
// Include a monotonically increasing suffix to avoid filename collisions
// between diagnostics produced within the same millisecond.
const uniqueSuffix = (diagnosticCounter++).toString();
// We should only need to remove colons, but to be defensive, only allow a restricted set of
// characters.
const sanitizedTimestamp = diagnostic.timestamp.replace(
/[^a-zA-Z0-9.-]/g,
"",
);
const jsonPath = path.resolve(
diagnosticsPath,
`codeql-action-${sanitizedTimestamp}-${uniqueSuffix}.json`,
);
writeFileSync(jsonPath, JSON.stringify(diagnostic));
writeDiagnosticFile(diagnosticsPath, diagnostic);
} catch (err) {
logger.warning(`Unable to write diagnostic message to database: ${err}`);
logger.debug(JSON.stringify(diagnostic));
action.logger.warning(
`Unable to write diagnostic message to database: ${err}`,
);
action.logger.debug(JSON.stringify(diagnostic));
}
}
/** Report if there are unwritten diagnostics and write them to the log. */
export function logUnwrittenDiagnostics() {
const logger = getActionsLogger();
const num = unwrittenDiagnostics.length;
if (num > 0) {
logger.warning(
`${num} diagnostic(s) could not be written to the database and will not appear on the Tool Status Page.`,
/** Report if there are temporary diagnostics and write them to the log. */
export function logTemporaryDiagnostics(action: ActionState<["Logger"]>) {
const num = temporaryDiagnostics.length;
if (num > 0 && action.logger.isDebug()) {
action.logger.startGroup("Diagnostics in temporary location");
action.logger.debug(
`${num} diagnostic(s) were not moved to the database and remain in the temporary directory.`,
);
for (const unwritten of unwrittenDiagnostics) {
logger.debug(JSON.stringify(unwritten.diagnostic));
for (const temporary of temporaryDiagnostics) {
try {
action.logger.debug(fs.readFileSync(temporary.path, "utf-8"));
} catch (err) {
action.logger.debug(
`Failed to read diagnostics from temporary location at '${temporary.path}': ${getErrorMessage(err)}`,
);
}
}
action.logger.endGroup();
}
}
/** Relocates all temporary diagnostics to the respective databases. */
export function flushDiagnostics(
action: ActionState<["Logger"]>,
config: Config,
) {
const diagnosticsCount = temporaryDiagnostics.length;
action.logger.debug(
`Moving ${diagnosticsCount} diagnostic(s) to their respective databases.`,
);
const failedFlushDiagnostics: TemporaryDiagnostic[] = [];
for (const temporary of temporaryDiagnostics) {
// If `temporary.language` is `undefined`, then we didn't have a `config` at the time that
// `addNoLanguageDiagnostic` was called. In that case, we arbitrarily choose the first
// configured language here like we would have done in `addNoLanguageDiagnostic`.
const directory = getDatabaseDiagnosticsPath(
config,
temporary.language ?? config.languages[0],
);
const filename = path.basename(temporary.path);
const destination = path.join(directory, filename);
try {
fs.mkdirSync(directory, { recursive: true });
try {
fs.renameSync(temporary.path, destination);
} catch (renameErr: unknown) {
// Rethrow the error if it isn't an EXDEV one.
if (!isErrnoException(renameErr) || renameErr.code !== "EXDEV") {
throw renameErr;
}
// The error was an EXDEV one: try to copy the file and delete the temporary one.
fs.copyFileSync(temporary.path, destination);
fs.unlinkSync(temporary.path);
}
} catch (err) {
action.logger.warning(
`Failed to relocate '${temporary.path}' to '${destination}': ${getErrorMessage(err)}`,
);
failedFlushDiagnostics.push(temporary);
}
}
}
/** Writes all unwritten diagnostics to disk. */
export function flushDiagnostics(config: Config) {
const logger = getActionsLogger();
const diagnosticsCount =
unwrittenDiagnostics.length + unwrittenDefaultLanguageDiagnostics.length;
logger.debug(`Writing ${diagnosticsCount} diagnostic(s) to database.`);
for (const unwritten of unwrittenDiagnostics) {
writeDiagnostic(config, unwritten.language, unwritten.diagnostic);
}
for (const unwritten of unwrittenDefaultLanguageDiagnostics) {
addNoLanguageDiagnostic(config, unwritten);
}
// Reset the unwritten diagnostics arrays.
unwrittenDiagnostics = [];
unwrittenDefaultLanguageDiagnostics = [];
// Reset the temporary diagnostics arrays.
temporaryDiagnostics = failedFlushDiagnostics;
}
/**

View File

@@ -1,3 +1,6 @@
import * as fs from "fs";
import * as path from "path";
import * as core from "@actions/core";
import test, { ExecutionContext } from "ava";
import * as sinon from "sinon";
@@ -8,6 +11,7 @@ import * as apiClient from "./api-client";
import * as codeql from "./codeql";
import * as configUtils from "./config-utils";
import * as debugArtifacts from "./debug-artifacts";
import { getTempDiagnosticsRoot } from "./diagnostics";
import { EnvVar } from "./environment";
import { Feature } from "./feature-flags";
import * as initActionPostHelper from "./init-action-post-helper";
@@ -35,6 +39,47 @@ const NUM_BYTES_PER_GIB = 1024 * 1024 * 1024;
setupTests(test);
test.serial("generateFailedSarif picks up unwritten diagnostics", async (t) =>
util.withTmpDir(async (tmpDir) => {
const logger = new RecordingLogger();
const tempDiagDir = getTempDiagnosticsRoot(tmpDir);
fs.mkdirSync(tempDiagDir, { recursive: true });
const codeqlObject = await getCodeQLForTesting();
const diagnosticsExport = sinon
.stub(codeqlObject, "diagnosticsExport")
.resolves();
sinon.stub(codeql, "getCodeQL").resolves(codeqlObject);
const sarifFile = path.join(tmpDir, "failed.sarif");
const result = await initActionPostHelper.generateFailedSarif(
logger,
createFeatures([Feature.ExportDiagnosticsEnabled]),
createTestConfig({
tempDir: tmpDir,
// Explicitly undefined to ensure we get the "no database" path.
dbLocation: undefined,
}),
"category",
"",
sarifFile,
);
t.truthy(result);
t.is(diagnosticsExport.callCount, 1);
t.true(
diagnosticsExport.calledOnceWith(
sarifFile,
"category",
sinon.match.any,
tempDiagDir,
),
);
}),
);
test.serial("init-post action with debug mode off", async (t) => {
return await util.withTmpDir(async (tmpDir) => {
setupActionsVars(tmpDir, tmpDir);

View File

@@ -24,6 +24,7 @@ import {
type UploadDebugArtifactsConfig,
} from "./debug-artifacts";
import * as dependencyCaching from "./dependency-caching";
import { getTempDiagnosticsRoot } from "./diagnostics";
import { EnvVar, ReadOnlyEnv } from "./environment";
import { Feature, FeatureEnablement } from "./feature-flags";
import { Logger } from "./logging";
@@ -177,7 +178,16 @@ async function prepareFailedSarif(
}
}
async function generateFailedSarif(
/**
* Generates a SARIF file for a failed run, i.e. an analysis where no SARIF was produced as a result of the analysis.
*
* Not intended to be called directly, other than by `prepareFailedSarif`.
*
* @param category The analysis category to include in the SARIF.
* @param checkoutPath The path where the repository was checked out at.
* @param sarifFile The SARIF file to produce.
*/
export async function generateFailedSarif(
logger: Logger,
features: FeatureEnablement,
config: ActionConfig<"codeQLCmd" | "tempDir">,
@@ -194,11 +204,26 @@ async function generateFailedSarif(
}
// If there is no database or the feature flag is off, we run 'export diagnostics'
if (
databasePath === undefined ||
!(await features.getValue(Feature.ExportDiagnosticsEnabled, codeql))
) {
await codeql.diagnosticsExport(sarifFile, category, config);
const exportDiagnosticsEnabled = await features.getValue(
Feature.ExportDiagnosticsEnabled,
codeql,
);
if (databasePath === undefined || !exportDiagnosticsEnabled) {
// If we don't have a database path, then we may have diagnostics that were written to a
// temporary location. Get the path that they would have been written to and check whether
// it exists. If so, pass it on to the CLI. The CLI will not complain if we give it a
// path that doesn't exist, so we are just being extra defensive here.
// We also don't pass the path to the CLI if `exportDiagnosticsEnabled` is not enabled, to preserve
// its intended behaviour of controlling whether diagnostics are included in the SARIF or not.
let diagnosticDir: string | undefined = getTempDiagnosticsRoot(
config.tempDir,
);
if (!exportDiagnosticsEnabled || !fs.existsSync(diagnosticDir)) {
diagnosticDir = undefined;
}
await codeql.diagnosticsExport(sarifFile, category, config, diagnosticDir);
} else {
// We call 'database export-diagnostics' to find any per-database diagnostics.
await codeql.databaseExportDiagnostics(databasePath, sarifFile, category);

View File

@@ -40,7 +40,7 @@ import {
addDiagnostic,
addNoLanguageDiagnostic,
flushDiagnostics,
logUnwrittenDiagnostics,
logTemporaryDiagnostics,
makeDiagnostic,
makeTelemetryDiagnostic,
} from "./diagnostics";
@@ -484,6 +484,7 @@ async function run(
if (statusReportBase !== undefined) {
await sendStatusReport(statusReportBase);
}
logTemporaryDiagnostics(actionState);
return;
}
@@ -766,14 +767,15 @@ async function run(
);
}
// Write diagnostics to the database that we previously stored in memory because the database
// did not exist until now.
flushDiagnostics(config);
// Move diagnostics to the database that we previously kept in a temporary location
// because the database did not exist until now
flushDiagnostics(actionState, config);
// We save the config here instead of at the end of `initConfig` because we
// may have updated the config returned from `initConfig`, e.g. to revert to
// `OverlayDatabaseMode.None` if we failed to download an overlay-base
// database.
config.successfullyInitialised = true;
await configUtils.saveConfig(config, logger);
core.setOutput("codeql-path", config.codeQLCmd);
@@ -797,7 +799,7 @@ async function run(
);
return;
} finally {
logUnwrittenDiagnostics();
logTemporaryDiagnostics(actionState);
}
await sendCompletedStatusReport(
startedAt,

View File

@@ -1714,15 +1714,19 @@ test.serial(
},
]);
const result = await setupCodeql.getEnabledVersionsWithOverlayBaseDatabases(
overlayMatchEnabledVersions,
["javascript"],
new RecordingLogger(),
);
t.deepEqual(result, [
{ cliVersion: "2.20.1", tagName: "codeql-bundle-v2.20.1" },
{ cliVersion: "2.20.0", tagName: "codeql-bundle-v2.20.0" },
]);
await withTmpDir(async (tmpDir) => {
setupActionsVars(tmpDir, tmpDir);
const result =
await setupCodeql.getEnabledVersionsWithOverlayBaseDatabases(
overlayMatchEnabledVersions,
["javascript"],
new RecordingLogger(),
);
t.deepEqual(result, [
{ cliVersion: "2.20.1", tagName: "codeql-bundle-v2.20.1" },
{ cliVersion: "2.20.0", tagName: "codeql-bundle-v2.20.0" },
]);
});
},
);

View File

@@ -1032,6 +1032,7 @@ export function createTestConfig(overrides: Partial<Config>): Config {
overlayModeSetExplicitly: false,
repositoryProperties: {},
enableFileCoverageInformation: true,
successfullyInitialised: false,
} satisfies Config,
overrides,
);

View File

@@ -900,6 +900,21 @@ export function getErrorMessage(error: unknown): string {
return error instanceof Error ? error.message : String(error);
}
/**
* Approximates whether {@link error} is like a {@link NodeJS.ErrnoException}.
* We only typically care about the `code` property, so that's what we check for here.
*/
export function isErrnoException(
error: unknown,
): error is NodeJS.ErrnoException {
return (
typeof error === "object" &&
error !== null &&
"code" in error &&
typeof error.code === "string"
);
}
export function prettyPrintPack(pack: Pack) {
return `${pack.name}${pack.version ? `@${pack.version}` : ""}${
pack.path ? `:${pack.path}` : ""