Skip to content

Commit d859d17

Browse files
authored
Merge pull request #1946 from github/mbg/unconditioal-go-warning
Make Go path warning unconditional
2 parents 7e7f0cc + 325a0b0 commit d859d17

File tree

9 files changed

+102
-78
lines changed

9 files changed

+102
-78
lines changed

lib/analyze-action.js

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

lib/analyze-action.js.map

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

lib/environment.js

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

lib/environment.js.map

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

lib/init-action.js

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

lib/init-action.js.map

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/analyze-action.ts

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -233,21 +233,23 @@ async function run() {
233233
logger,
234234
);
235235

236-
// Check that `which go` still points at the wrapper script we installed in the `init` Action,
237-
// if the corresponding environment variable is set. This is to ensure that there isn't a step
238-
// in the workflow after the `init` step which installs a different version of Go and takes
239-
// precedence in the PATH, thus potentially circumventing our workaround that allows tracing to work.
240-
const goWrapperPath = process.env[EnvVar.GO_BINARY_LOCATION];
236+
// Check that `which go` still points at the same path it did when the `init` Action ran to ensure that no steps
237+
// in-between performed any setup. We encourage users to perform all setup tasks before initializing CodeQL so that
238+
// the setup tasks do not interfere with our analysis.
239+
// Furthermore, if we installed a wrapper script in the `init` Action, we need to ensure that there isn't a step
240+
// in the workflow after the `init` step which installs a different version of Go and takes precedence in the PATH,
241+
// thus potentially circumventing our workaround that allows tracing to work.
242+
const goInitPath = process.env[EnvVar.GO_BINARY_LOCATION];
241243

242244
if (
243245
process.env[EnvVar.DID_AUTOBUILD_GOLANG] !== "true" &&
244-
goWrapperPath !== undefined
246+
goInitPath !== undefined
245247
) {
246248
const goBinaryPath = await safeWhich("go");
247249

248-
if (goWrapperPath !== goBinaryPath) {
250+
if (goInitPath !== goBinaryPath) {
249251
core.warning(
250-
`Expected \`which go\` to return ${goWrapperPath}, but got ${goBinaryPath}: please ensure that the correct version of Go is installed before the \`codeql-action/init\` Action is used.`,
252+
`Expected \`which go\` to return ${goInitPath}, but got ${goBinaryPath}: please ensure that the correct version of Go is installed before the \`codeql-action/init\` Action is used.`,
251253
);
252254

253255
addDiagnostic(

src/environment.ts

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -64,9 +64,8 @@ export enum EnvVar {
6464
WORKFLOW_STARTED_AT = "CODEQL_WORKFLOW_STARTED_AT",
6565

6666
/**
67-
* The path where we initially discovered the Go binary in the system path
68-
* before replacing it with a wrapper script. We check this later to ensure
69-
* that it hasn't been tampered with by a late e.g. `setup-go` step.
67+
* The path where we initially discovered the Go binary in the system path.
68+
* We check this later to ensure that it hasn't been tampered with by a late e.g. `setup-go` step.
7069
*/
7170
GO_BINARY_LOCATION = "CODEQL_ACTION_GO_BINARY",
7271
}

src/init-action.ts

Lines changed: 45 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -336,49 +336,61 @@ async function run() {
336336
);
337337
}
338338

339-
// Go 1.21 and above ships with statically linked binaries on Linux. CodeQL cannot currently trace custom builds
340-
// where the entry point is a statically linked binary. Until that is fixed, we work around the problem by
341-
// replacing the `go` binary with a shell script that invokes the actual `go` binary. Since the shell is typically
342-
// dynamically linked, this provides a suitable entry point for the CodeQL tracer.
343339
if (
344340
config.languages.includes(Language.go) &&
345-
process.platform === "linux" &&
346-
!isSupportedToolsFeature(
347-
versionInfo,
348-
ToolsFeature.IndirectTracingSupportsStaticBinaries,
349-
)
341+
process.platform === "linux"
350342
) {
351343
try {
352344
const goBinaryPath = await safeWhich("go");
353345
const fileOutput = await getFileType(goBinaryPath);
354346

355-
if (fileOutput.includes("statically linked")) {
356-
logger.debug(`Applying static binary workaround for Go`);
357-
358-
// Create a directory that we can add to the system PATH.
359-
const tempBinPath = path.resolve(
360-
getTemporaryDirectory(),
361-
"codeql-action-go-tracing",
362-
"bin",
363-
);
364-
fs.mkdirSync(tempBinPath, { recursive: true });
365-
core.addPath(tempBinPath);
366-
367-
// Write the wrapper script to the directory we just added to the PATH.
368-
const goWrapperPath = path.resolve(tempBinPath, "go");
369-
fs.writeFileSync(
370-
goWrapperPath,
371-
`#!/bin/bash\n\nexec ${goBinaryPath} "$@"`,
372-
);
373-
fs.chmodSync(goWrapperPath, "755");
374-
375-
// Store the original location of our wrapper script somewhere where we can
376-
// later retrieve it from and cross-check that it hasn't been changed.
377-
core.exportVariable(EnvVar.GO_BINARY_LOCATION, goWrapperPath);
347+
// Go 1.21 and above ships with statically linked binaries on Linux. CodeQL cannot currently trace custom builds
348+
// where the entry point is a statically linked binary. Until that is fixed, we work around the problem by
349+
// replacing the `go` binary with a shell script that invokes the actual `go` binary. Since the shell is
350+
// typically dynamically linked, this provides a suitable entry point for the CodeQL tracer.
351+
if (
352+
fileOutput.includes("statically linked") &&
353+
!isSupportedToolsFeature(
354+
versionInfo,
355+
ToolsFeature.IndirectTracingSupportsStaticBinaries,
356+
)
357+
) {
358+
try {
359+
logger.debug(`Applying static binary workaround for Go`);
360+
361+
// Create a directory that we can add to the system PATH.
362+
const tempBinPath = path.resolve(
363+
getTemporaryDirectory(),
364+
"codeql-action-go-tracing",
365+
"bin",
366+
);
367+
fs.mkdirSync(tempBinPath, { recursive: true });
368+
core.addPath(tempBinPath);
369+
370+
// Write the wrapper script to the directory we just added to the PATH.
371+
const goWrapperPath = path.resolve(tempBinPath, "go");
372+
fs.writeFileSync(
373+
goWrapperPath,
374+
`#!/bin/bash\n\nexec ${goBinaryPath} "$@"`,
375+
);
376+
fs.chmodSync(goWrapperPath, "755");
377+
378+
// Store the original location of our wrapper script somewhere where we can
379+
// later retrieve it from and cross-check that it hasn't been changed.
380+
core.exportVariable(EnvVar.GO_BINARY_LOCATION, goWrapperPath);
381+
} catch (e) {
382+
logger.warning(
383+
`Analyzing Go on Linux, but failed to install wrapper script. Tracing custom builds may fail: ${e}`,
384+
);
385+
}
386+
} else {
387+
// Store the location of the original Go binary, so we can check that no setup tasks were performed after the
388+
// `init` Action ran.
389+
core.exportVariable(EnvVar.GO_BINARY_LOCATION, goBinaryPath);
378390
}
379391
} catch (e) {
380392
logger.warning(
381-
`Analyzing Go on Linux, but failed to install wrapper script. Tracing custom builds may fail: ${e}`,
393+
`Failed to determine the location of the Go binary: ${e}`,
382394
);
383395
}
384396
}

0 commit comments

Comments
 (0)