Arc updates #324
Arc updates #324
Conversation
There was a problem hiding this comment.
Please find StepSecurity AI-CodeWise code comments inline or below.
src/cleanup.ts
Please refer to 2 inline comments.
src/setup.ts
Please refer to 3 inline comments.
dist/post/index.js
Please refer to 4 inline comments.
dist/pre/index.js
Please refer to 2 inline comments.
Feedback
We appreciate your feedback in helping us improve the service! To provide feedback, please use emojis on this comment. If you find a comment helpful, give it a 👍. If they aren't useful, kindly express that with a 👎. If you have questions or detailed feedback, please create n GitHub issue in StepSecurity/AI-CodeWise.
| ); | ||
| console.log(cacheResult); | ||
| } catch (exception) { | ||
| console.log(exception); |
There was a problem hiding this comment.
[High]Avoid logging sensitive information
Sensitive information should not be printed to the console as it could be visible to other users and may pose a security risk. Remove the console log statement or mask sensitive information before printing to the console.
| @@ -18,6 +18,18 @@ import { arcCleanUp, isArcRunner, removeStepPolicyFiles } from "./arc-runner"; | |||
| return; | |||
| } | |||
|
|
|||
| if (isValidEvent()) { | |||
There was a problem hiding this comment.
[Medium]Consider including this refactored function into your pull request
Removed the try-catch block and directly passed the parameters to cache.saveCache() function.
if (isValidEvent()) {
const cacheResult = await cache.saveCache([
path.join(__dirname, "cache.txt")
], cacheKey);
console.log(cacheResult);
}
| } | ||
| } | ||
|
|
||
| if (!confg.disable_telemetry || confg.egress_policy === "audit") { |
There was a problem hiding this comment.
[High]Add input validation while setting the disable_telemetry value
The value of disable_telemetry is not being validated. Ensure that the value of disable_telemetry is a boolean value
| } catch (exception) { | ||
| // some exception has occurred. | ||
| core.info(`Unable to fetch cacheURL`); | ||
| if (confg.egress_policy === "block") { |
There was a problem hiding this comment.
[High]Add check to validate the egress_policy
The value of egress_policy is being used without validation. Ensure that the value of egress_policy is set to a proper value
| ); | ||
| const url = new URL(cacheEntry.archiveLocation); | ||
| core.info(`Adding cacheHost: ${url.hostname}:443 to allowed-endpoints`); | ||
| confg.allowed_endpoints += ` ${url.hostname}:443`; |
There was a problem hiding this comment.
[Low]Avoid multiple blank space while string concatenation
There are multiple blank spaces while concatenating strings. Remove extra blank spaces while concatenating strings
| } | ||
| } | ||
| function applyPolicy(count) { | ||
| cp.execSync(`echo "step_policy_apply_${count}" > "${getRunnerTempDir()}/step_policy_apply_${count}"`); | ||
| const fileName = `step_policy_apply_${count}`; |
There was a problem hiding this comment.
[Medium]Use const instead of let when declaring variables that aren't reassigned
The variable 'fileName' in function 'applyPolicy' is declared with 'let', but it's never reassigned. Replace 'let' with 'const'.
| if (runner_user_agent.indexOf("actions-runner-controller/") > -1) | ||
| out = true; | ||
| return out; | ||
| const runnerUserAgent = process.env["GITHUB_ACTIONS_RUNNER_EXTRA_USER_AGENT"]; |
There was a problem hiding this comment.
[Medium]Check if a value is null or undefined before calling methods on it
The value of 'runnerUserAgent' in function 'isArcRunner' can be undefined, which will cause an error if calling methods on it. Check if 'runnerUserAgent' is null or undefined before calling methods on it.
| if (allowed_endpoints.length > 0) { | ||
| for (let endp of allowed_endpoints) { | ||
| cp.execSync(`echo "${endp}" > "${getRunnerTempDir()}/step_policy_endpoint_\`echo "${endp}" | base64\`"`); | ||
| const allowedEndpoints = endpoints.split(" "); // endpoints are space separated |
There was a problem hiding this comment.
[Medium]Check if a value is null or undefined before calling methods on it
The value of 'allowedEndpoints' in function 'sendAllowedEndpoints' can be null or undefined, which will cause an error if calling methods on it. Check if 'allowedEndpoints' is null or undefined before calling methods on it.
| } | ||
| } | ||
| function applyPolicy(count) { | ||
| external_child_process_.execSync(`echo "step_policy_apply_${count}" > "${getRunnerTempDir()}/step_policy_apply_${count}"`); | ||
| const fileName = `step_policy_apply_${count}`; |
There was a problem hiding this comment.
[Low]Use a more descriptive var name for cached file name
On line 37, the variable fileName can be more descriptive. Use a more descriptive variable name for fileName.
| out = true; | ||
| return out; | ||
| const runnerUserAgent = process.env["GITHUB_ACTIONS_RUNNER_EXTRA_USER_AGENT"]; | ||
| if (!runnerUserAgent) { |
There was a problem hiding this comment.
[Low]Use more explicit comparison with null
On line 9, a more explicit comparison with null can be used instead of double negation with typeof. Use 'runnerUserAgent !== null' instead of '!runnerUserAgent'
No description provided.