Skip to content

Commit f1588cd

Browse files
committed
Add StartProxyError for status-report-safe errors, and use for proxy download
1 parent 4dcc8a9 commit f1588cd

File tree

4 files changed

+144
-22
lines changed

4 files changed

+144
-22
lines changed

lib/start-proxy-action.js

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

src/start-proxy-action.ts

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import { getActionsLogger, Logger } from "./logging";
1212
import {
1313
Credential,
1414
credentialToStr,
15+
downloadProxy,
1516
getCredentials,
1617
getDownloadUrl,
1718
parseLanguage,
@@ -238,14 +239,7 @@ async function getProxyBinaryPath(logger: Logger): Promise<string> {
238239
apiDetails,
239240
proxyInfo.url,
240241
);
241-
const temp = await toolcache.downloadTool(
242-
proxyInfo.url,
243-
undefined,
244-
authorization,
245-
{
246-
accept: "application/octet-stream",
247-
},
248-
);
242+
const temp = await downloadProxy(logger, proxyInfo.url, authorization);
249243
const extracted = await toolcache.extractTar(temp);
250244
proxyBin = await toolcache.cacheDir(
251245
extracted,

src/start-proxy.test.ts

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import * as toolcache from "@actions/tool-cache";
12
import test from "ava";
23
import sinon from "sinon";
34

@@ -318,3 +319,47 @@ test("credentialToStr - hides passwords/tokens", (t) => {
318319
.includes(secret),
319320
);
320321
});
322+
323+
test("getSafeErrorMessage - returns actual message for `StartProxyError`", (t) => {
324+
const error = new startProxyExports.StartProxyError(
325+
startProxyExports.StartProxyErrorType.DownloadFailed,
326+
);
327+
t.is(startProxyExports.getSafeErrorMessage(error), error.message);
328+
});
329+
330+
test("getSafeErrorMessage - does not return message for arbitrary errors", (t) => {
331+
const error = new Error(startProxyExports.StartProxyErrorType.DownloadFailed);
332+
333+
const message = startProxyExports.getSafeErrorMessage(error);
334+
335+
t.not(message, error.message);
336+
t.assert(message.startsWith("Error from start-proxy Action omitted"));
337+
t.assert(message.includes(typeof error));
338+
});
339+
340+
test("downloadProxy - returns file path on success", async (t) => {
341+
const loggedMessages = [];
342+
const logger = getRecordingLogger(loggedMessages);
343+
const testPath = "/some/path";
344+
sinon.stub(toolcache, "downloadTool").resolves(testPath);
345+
346+
const result = await startProxyExports.downloadProxy(
347+
logger,
348+
"url",
349+
undefined,
350+
);
351+
t.is(result, testPath);
352+
});
353+
354+
test("downloadProxy - wraps errors on failure", async (t) => {
355+
const loggedMessages = [];
356+
const logger = getRecordingLogger(loggedMessages);
357+
sinon.stub(toolcache, "downloadTool").throws();
358+
359+
await t.throwsAsync(
360+
startProxyExports.downloadProxy(logger, "url", undefined),
361+
{
362+
instanceOf: startProxyExports.StartProxyError,
363+
},
364+
);
365+
});

src/start-proxy.ts

Lines changed: 66 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import * as core from "@actions/core";
2+
import * as toolcache from "@actions/tool-cache";
23

34
import { getApiClient } from "./api-client";
45
import * as artifactScanner from "./artifact-scanner";
@@ -16,6 +17,26 @@ import {
1617
import * as util from "./util";
1718
import { ConfigurationError, getErrorMessage, isDefined } from "./util";
1819

20+
/**
21+
* Enumerates specific error types, along with suitable error messages, for errors
22+
* that we want to track in status reports.
23+
*/
24+
export enum StartProxyErrorType {
25+
DownloadFailed = "Failed to download proxy archive.",
26+
}
27+
28+
/**
29+
* We want to avoid accidentally leaking secrets that may be contained in exception
30+
* messages in the `start-proxy` action. Consequently, we don't report the messages
31+
* of arbitrary exceptions. This type of error ensures that the message is one from
32+
* `StartProxyErrorType` and therefore safe to include in a status report.
33+
*/
34+
export class StartProxyError extends Error {
35+
constructor(errorType: StartProxyErrorType) {
36+
super(errorType);
37+
}
38+
}
39+
1940
interface StartProxyStatus extends StatusReportBase {
2041
// A comma-separated list of registry types which are configured for CodeQL.
2142
// This only includes registry types we support, not all that are configured.
@@ -53,6 +74,23 @@ export async function sendSuccessStatusReport(
5374
}
5475
}
5576

77+
/**
78+
* Returns an error message for `error` that can safely be reported in a status report,
79+
* i.e. that does not contain sensitive information.
80+
*
81+
* @param error The error for which to get an error message.
82+
*/
83+
export function getSafeErrorMessage(error: Error): string {
84+
// If the error is a `StartProxyError`, the constructor ensures that the
85+
// message comes from `StartProxyErrorType`.
86+
if (error instanceof StartProxyError) {
87+
return error.message;
88+
}
89+
90+
// Otherwise, omit the actual error message.
91+
return `Error from start-proxy Action omitted (${typeof error}).`;
92+
}
93+
5694
/**
5795
* Sends a status report for the `start-proxy` action indicating a failure.
5896
*
@@ -70,6 +108,8 @@ export async function sendFailedStatusReport(
70108
const error = util.wrapError(unwrappedError);
71109
core.setFailed(`start-proxy action failed: ${error.message}`);
72110

111+
const statusReportMessage = getSafeErrorMessage(error);
112+
73113
// We skip sending the error message and stack trace here to avoid the possibility
74114
// of leaking any sensitive information into the telemetry.
75115
const errorStatusReportBase = await createStatusReportBase(
@@ -81,7 +121,7 @@ export async function sendFailedStatusReport(
81121
},
82122
await util.checkDiskUsage(logger),
83123
logger,
84-
"Error from start-proxy Action omitted",
124+
statusReportMessage,
85125
);
86126
if (errorStatusReportBase !== undefined) {
87127
await sendStatusReport(errorStatusReportBase);
@@ -369,3 +409,28 @@ export function credentialToStr(c: Credential): string {
369409
c.username
370410
}; Password: ${c.password !== undefined}; Token: ${c.token !== undefined}`;
371411
}
412+
413+
/**
414+
* Attempts to download a file from `url` into the toolcache.
415+
*
416+
* @param logger THe logger to use.
417+
* @param url The URL to download the proxy binary from.
418+
* @param authorization The authorization information to use.
419+
* @returns If successful, the path to the downloaded file.
420+
*/
421+
export async function downloadProxy(
422+
logger: Logger,
423+
url: string,
424+
authorization: string | undefined,
425+
) {
426+
try {
427+
return toolcache.downloadTool(url, undefined, authorization, {
428+
accept: "application/octet-stream",
429+
});
430+
} catch (error) {
431+
logger.error(
432+
`Failed to download proxy archive from ${url}: ${getErrorMessage(error)}`,
433+
);
434+
throw new StartProxyError(StartProxyErrorType.DownloadFailed);
435+
}
436+
}

0 commit comments

Comments
 (0)