Skip to content

Commit 2a3e756

Browse files
mbgmbg
authored
Merge pull request #4148 from github/mbg/improve-checkout-path
Validate `checkout_path` input in `analyze` action
2 parents f460dc7 + 4f74769 commit 2a3e756

24 files changed

Lines changed: 1166 additions & 704 deletions

‎lib/entry-points.js‎

Lines changed: 634 additions & 569 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎src/actions-util.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ export function getActionVersion(): string {
9393
*
9494
* This will be "dynamic" for default setup workflow runs.
9595
*/
96-
export function getWorkflowEventName(env: Env = getEnv()) {
96+
export function getWorkflowEventName(env: ReadOnlyEnv = getEnv()) {
9797
return env.getRequired(ActionsEnvVars.GITHUB_EVENT_NAME);
9898
}
9999

@@ -120,7 +120,7 @@ function getRelativeScriptPath(env: ReadOnlyEnv): string {
120120
}
121121

122122
/** Returns the contents of `GITHUB_EVENT_PATH` as a JSON object. */
123-
export function getWorkflowEvent(env: Env = getEnv()): any {
123+
export function getWorkflowEvent(env: ReadOnlyEnv = getEnv()): any {
124124
const eventJsonFile = env.getRequired(ActionsEnvVars.GITHUB_EVENT_PATH);
125125
try {
126126
return JSON.parse(fs.readFileSync(eventJsonFile, "utf-8"));

‎src/analyze-action.test.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ test.serial(
4545
requiredInputStub.withArgs("token").returns("fake-token");
4646
requiredInputStub.withArgs("upload-database").returns("false");
4747
requiredInputStub.withArgs("output").returns("out");
48+
requiredInputStub.withArgs("checkout_path").returns("");
4849
const optionalInputStub = sinon.stub(actionsUtil, "getOptionalInput");
4950
optionalInputStub.withArgs("expect-error").returns("false");
5051
sinon.stub(api, "getGitHubVersion").resolves(gitHubVersion);
@@ -104,6 +105,7 @@ test.serial(
104105
requiredInputStub.withArgs("token").returns("fake-token");
105106
requiredInputStub.withArgs("upload-database").returns("false");
106107
requiredInputStub.withArgs("output").returns("out");
108+
requiredInputStub.withArgs("checkout_path").returns("");
107109
const optionalInputStub = sinon.stub(actionsUtil, "getOptionalInput");
108110
optionalInputStub.withArgs("expect-error").returns("false");
109111
sinon.stub(api, "getGitHubVersion").resolves(gitHubVersion);

‎src/analyze-action.ts‎

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,18 @@ import { performance } from "perf_hooks";
44

55
import * as core from "@actions/core";
66

7-
import { Action, ActionState, runInActions } from "./action-common";
7+
import {
8+
Action,
9+
ActionState,
10+
extendActionState,
11+
runInActions,
12+
} from "./action-common";
813
import * as actionsUtil from "./actions-util";
914
import * as analyses from "./analyses";
1015
import {
1116
CodeQLAnalysisError,
1217
dbIsFinalized,
18+
determineCheckoutPath,
1319
QueriesStatusReport,
1420
runFinalize,
1521
runQueries,
@@ -212,14 +218,11 @@ async function runAutobuildIfLegacyGoWorkflow(config: Config, logger: Logger) {
212218
await runAutobuild(config, BuiltInLanguage.go, logger);
213219
}
214220

215-
async function run({
216-
startedAt,
217-
env,
218-
logger,
219-
actions,
220-
}: ActionState<["Base", "Env", "Logger", "Actions"]>) {
221+
async function run(action: ActionState<["Base", "Logger", "Env", "Actions"]>) {
221222
// To capture errors appropriately, keep as much code within the try-catch as
222223
// possible, and only use safe functions outside.
224+
const startedAt = action.startedAt;
225+
const logger = action.logger;
223226

224227
let uploadResults:
225228
| Partial<Record<analyses.AnalysisKind, UploadResult>>
@@ -277,7 +280,7 @@ async function run({
277280

278281
const apiDetails = getApiDetails();
279282
const outputDir = actionsUtil.getRequiredInput("output");
280-
env.export(EnvVar.SARIF_RESULTS_OUTPUT_DIR, outputDir);
283+
action.env.export(EnvVar.SARIF_RESULTS_OUTPUT_DIR, outputDir);
281284
const threads = util.getThreadsFlag(
282285
actionsUtil.getOptionalInput("threads") || process.env["CODEQL_THREADS"],
283286
logger,
@@ -301,7 +304,7 @@ async function run({
301304
logger,
302305
);
303306

304-
const checkoutPath = actions.getRequiredInput("checkout_path");
307+
const checkoutPath = await determineCheckoutPath(action, config);
305308

306309
// Setup diff informed analysis if needed (based on whether init created the file)
307310
const diffRangePackDir = await setupDiffInformedQueryRun(
@@ -393,11 +396,13 @@ async function run({
393396
checkoutPath,
394397
);
395398

399+
const actionWithFeatures = extendActionState(action, { features });
400+
396401
// Possibly upload the database bundles for remote queries.
397402
// Note: Take care with the ordering of this call since databases may be cleaned up
398403
// at the `overlay` or `clear` level.
399404
databaseUploadResults = await cleanupAndUploadDatabases(
400-
{ logger, features },
405+
actionWithFeatures,
401406
repositoryNwo,
402407
codeql,
403408
config,
@@ -446,7 +451,7 @@ async function run({
446451
`expect-error input was set to true but no error was thrown.`,
447452
);
448453
}
449-
env.export(EnvVar.ANALYZE_DID_COMPLETE_SUCCESSFULLY, "true");
454+
action.env.export(EnvVar.ANALYZE_DID_COMPLETE_SUCCESSFULLY, "true");
450455
} catch (unwrappedError) {
451456
const error = util.wrapError(unwrappedError);
452457
if (

‎src/analyze.test.ts‎

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,23 +10,111 @@ import {
1010
resolveQuerySuiteAlias,
1111
addSarifExtension,
1212
diffRangeExtensionPackContents,
13+
determineCheckoutPath,
1314
} from "./analyze";
1415
import { createStubCodeQL } from "./codeql";
1516
import { defaultSuites } from "./config/db-config";
1617
import { Feature } from "./feature-flags";
18+
import * as gitUtils from "./git-utils";
1719
import { BuiltInLanguage } from "./languages";
1820
import { getRunnerLogger } from "./logging";
1921
import {
2022
setupTests,
2123
setupActionsVars,
2224
createFeatures,
2325
createTestConfig,
26+
callee,
2427
} from "./testing-utils";
2528
import * as uploadLib from "./upload-lib";
2629
import * as util from "./util";
2730

2831
setupTests(test);
2932

33+
test.serial(
34+
"determineCheckoutPath - logs when checkout_path is not in a work tree",
35+
async (t) => {
36+
const expectedPath = path.resolve("/checkout/path");
37+
const target = callee(determineCheckoutPath)
38+
.withActions((actions) => {
39+
sinon
40+
.stub(actions, "getRequiredInput")
41+
.withArgs("checkout_path")
42+
.returns(expectedPath);
43+
})
44+
.withArgs(createTestConfig({}));
45+
sinon.stub(gitUtils, "getGitRoot").resolves(undefined);
46+
47+
await target
48+
.logs(t, "is not in the work tree of a git repository")
49+
.passes(t.is, expectedPath);
50+
},
51+
);
52+
53+
test.serial(
54+
"determineCheckoutPath - logs when checkout_path is not a repo root",
55+
async (t) => {
56+
const expectedPath = path.resolve("/checkout/path");
57+
const target = callee(determineCheckoutPath)
58+
.withActions((actions) => {
59+
sinon
60+
.stub(actions, "getRequiredInput")
61+
.withArgs("checkout_path")
62+
.returns(expectedPath);
63+
})
64+
.withArgs(createTestConfig({}));
65+
sinon.stub(gitUtils, "getGitRoot").resolves("/checkout");
66+
67+
await target
68+
.logs(t, "is not the root of the repository")
69+
.passes(t.is, expectedPath);
70+
},
71+
);
72+
73+
test.serial(
74+
"determineCheckoutPath - logs when checkout_path is not the same as repo root in config",
75+
async (t) => {
76+
const expectedPath = path.resolve("/checkout/path");
77+
const target = callee(determineCheckoutPath)
78+
.withActions((actions) => {
79+
sinon
80+
.stub(actions, "getRequiredInput")
81+
.withArgs("checkout_path")
82+
.returns(expectedPath);
83+
})
84+
.withArgs(createTestConfig({ repositoryRoot: "/some/other/path" }));
85+
sinon.stub(gitUtils, "getGitRoot").resolves(expectedPath);
86+
87+
await target
88+
.logs(t, "does not match that found by the 'codeql-action/init' step")
89+
.passes(t.is, expectedPath);
90+
},
91+
);
92+
93+
test.serial(
94+
"determineCheckoutPath - doesn't log any of the messages when all is as expected",
95+
async (t) => {
96+
const expectedPath = path.resolve("/checkout/path");
97+
const target = callee(determineCheckoutPath)
98+
.withActions((actions) => {
99+
sinon
100+
.stub(actions, "getRequiredInput")
101+
.withArgs("checkout_path")
102+
.returns(expectedPath);
103+
})
104+
.withArgs(createTestConfig({ repositoryRoot: expectedPath }));
105+
sinon.stub(gitUtils, "getGitRoot").resolves(expectedPath);
106+
107+
await target
108+
.notLogs(
109+
t,
110+
"is not in the work tree of a git repository",
111+
"is not the root of the repository",
112+
"does not match that found by the 'codeql-action/init' step",
113+
)
114+
.passes(t.is, expectedPath);
115+
},
116+
);
117+
30118
/**
31119
* Checks the status report produced by the analyze Action.
32120
*

‎src/analyze.ts‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { performance } from "perf_hooks";
55
import * as io from "@actions/io";
66
import * as yaml from "js-yaml";
77

8+
import type { ActionState } from "./action-common";
89
import { getTemporaryDirectory } from "./actions-util";
910
import * as analyses from "./analyses";
1011
import { setupCppAutobuild } from "./autobuild";
@@ -23,6 +24,7 @@ import {
2324
} from "./diff-informed-analysis-utils";
2425
import { EnvVar } from "./environment";
2526
import { FeatureEnablement, Feature } from "./feature-flags";
27+
import { getGitRoot } from "./git-utils";
2628
import { BuiltInLanguage, Language } from "./languages";
2729
import { Logger, withGroupAsync } from "./logging";
2830
import { OverlayDatabaseMode } from "./overlay/overlay-database-mode";
@@ -87,6 +89,58 @@ export interface QueriesStatusReport
8789
event_reports?: EventReport[];
8890
}
8991

92+
/**
93+
* Determines the path at which the repository being analysed is checked out at.
94+
* Returns the value of the required `checkout_path` input and validates that it
95+
* refers to the root of a repository.
96+
*
97+
* @param action The action state.
98+
* @param config The CodeQL Action configuration state.
99+
*/
100+
export async function determineCheckoutPath(
101+
action: ActionState<["Logger", "Actions"]>,
102+
config: configUtils.Config,
103+
) {
104+
const checkoutPathInput = action.actions.getRequiredInput("checkout_path");
105+
106+
// Try to obtain the root path of the repository and validate that it matches the input.
107+
const absCheckoutPathInput = path.resolve(checkoutPathInput);
108+
const repositoryRoot = await getGitRoot(absCheckoutPathInput);
109+
110+
if (repositoryRoot === undefined) {
111+
action.logger.warning(
112+
[
113+
`The directory at '${absCheckoutPathInput}' is not in the work tree of a git repository.`,
114+
"If the repository being analyzed is checked out elsewhere,",
115+
"you must explicitly set the 'checkout_path' input for the 'codeql-action/analyze' step to",
116+
"the checkout path.",
117+
].join(" "),
118+
);
119+
} else if (repositoryRoot !== absCheckoutPathInput) {
120+
action.logger.warning(
121+
[
122+
`The directory at '${absCheckoutPathInput}' is not the root of the repository ('${repositoryRoot}').`,
123+
"Set the 'checkout_path' input for the 'codeql-action/analyze' step to the root path of the checkout.",
124+
].join(" "),
125+
);
126+
} else if (
127+
config.repositoryRoot !== undefined &&
128+
repositoryRoot !== config.repositoryRoot
129+
) {
130+
// The repository root that was persisted by the `init` step doesn't match the one we have found here.
131+
action.logger.warning(
132+
[
133+
`The repository path at '${repositoryRoot}' does not match that found by the 'codeql-action/init' step: '${config.repositoryRoot}'.`,
134+
"Ensure that the 'checkout_path' input for the 'codeql-action/analyze' step is set to the path of the same repository that",
135+
"the 'codeql-action/init' step determined. This is either the GitHub Actions workspace or the repository root corresponding to",
136+
"the 'source-root' input if that was provided.",
137+
].join(" "),
138+
);
139+
}
140+
141+
return absCheckoutPathInput;
142+
}
143+
90144
async function setupPythonExtractor(logger: Logger) {
91145
const codeqlPython = process.env["CODEQL_PYTHON"];
92146
if (codeqlPython === undefined || codeqlPython.length === 0) {

‎src/codeql.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1225,7 +1225,7 @@ export async function getTrapCachingExtractorConfigArgsForLang(
12251225
): Promise<string[]> {
12261226
const cacheDir = config.trapCaches[language];
12271227
if (cacheDir === undefined) return [];
1228-
const write = await isAnalyzingDefaultBranch();
1228+
const write = await isAnalyzingDefaultBranch(getEnv(), config.repositoryRoot);
12291229
return [
12301230
`-O=${language}.trap.cache.dir=${cacheDir}`,
12311231
`-O=${language}.trap.cache.bound=${TRAP_CACHE_SIZE_MB}`,

‎src/config-utils.test.ts‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -172,6 +172,7 @@ test.serial("load empty config", async (t) => {
172172
createTestInitConfigInputs({
173173
languagesInput: languages,
174174
repository: { owner: "github", repo: "example" },
175+
sourceRoot: tempDir,
175176
tempDir,
176177
codeql,
177178
logger,
@@ -186,6 +187,7 @@ test.serial("load empty config", async (t) => {
186187
logger,
187188
}),
188189
{},
190+
undefined,
189191
);
190192

191193
t.deepEqual(config, expectedConfig);
@@ -216,6 +218,7 @@ test.serial("load code quality config", async (t) => {
216218
analysisKinds: [AnalysisKind.CodeQuality],
217219
languagesInput: languages,
218220
repository: { owner: "github", repo: "example" },
221+
sourceRoot: tempDir,
219222
tempDir,
220223
codeql,
221224
logger,
@@ -296,6 +299,7 @@ test.serial(
296299
analysisKinds: [AnalysisKind.CodeQuality],
297300
languagesInput: languages,
298301
repository: { owner: "github", repo: "example" },
302+
sourceRoot: tempDir,
299303
tempDir,
300304
codeql,
301305
repositoryProperties,
@@ -518,6 +522,7 @@ test.serial("load non-empty input", async (t) => {
518522
// And the config we expect it to parse to
519523
const expectedConfig = createTestConfig({
520524
languages: [BuiltInLanguage.javascript],
525+
repositoryRoot: undefined,
521526
buildMode: BuildMode.None,
522527
originalUserInput: userConfig,
523528
computedConfig: userConfig,
@@ -538,6 +543,7 @@ test.serial("load non-empty input", async (t) => {
538543
state,
539544
createTestInitConfigInputs({
540545
languagesInput,
546+
sourceRoot: tempDir,
541547
buildModeInput: "none",
542548
configFile: configFilePath,
543549
debugArtifactName: "my-artifact",
@@ -1105,11 +1111,6 @@ const checkOverlayEnablementMacro = makeMacro({
11051111
return lang === BuiltInLanguage.java;
11061112
});
11071113

1108-
// Mock git root detection
1109-
if (setup.gitRoot !== undefined) {
1110-
sinon.stub(gitUtils, "getGitRoot").resolves(setup.gitRoot);
1111-
}
1112-
11131114
// Mock submodule detection
11141115
sinon.stub(gitUtils, "hasSubmodules").returns(setup.hasSubmodules);
11151116

@@ -1122,6 +1123,7 @@ const checkOverlayEnablementMacro = makeMacro({
11221123
codeql,
11231124
features,
11241125
setup.languages,
1126+
setup.gitRoot, // repositoryRoot
11251127
tempDir, // sourceRoot
11261128
setup.buildMode,
11271129
undefined,

0 commit comments

Comments
 (0)