generated from actions/typescript-action
-
Notifications
You must be signed in to change notification settings - Fork 3
Labels
Description
Tracking issue for:
The Gitrepo
class contains a two calls to execSync
:
execSync(
git -C ${this.getDirPath()} status, {stdio: 'ignore'})
execSync(
git -C ${this.dir.getDirPath()} log -n 0, {stdio: 'ignore'})
https://github.com/Nautilus-Cyberneering/git-queue/blob/main/src/git-repo.ts
Since we use a class for the dir we are safe against shell injection as long as the class validates the dir:
Anyway, we could refactor it in order to avoid getting alerts from Code scanning
.
They suggest:
var cp = require("child_process"),
path = require("path");
function cleanupTemp() {
let cmd = "rm",
args = ["-rf", path.join(__dirname, "temp")];
cp.execFileSync(cmd, args); // GOOD
}
instead of:
var cp = require("child_process"),
path = require("path");
function cleanupTemp() {
let cmd = "rm -rf " + path.join(__dirname, "temp");
cp.execSync(cmd); // BAD
}
For the Librarian we build a wrapper to execute console commands.
I think it's a good idea because:
- It forces the user to pass the arguments for the console command with other function parameters that could be escaped.
- We can check easily check that all console commands are executed via the wrapper.
- If the validation of the object fails or stops working we are still safe against shell injection because we escape the arguments. For example, if the
dir
class validation is accidentally removed the console command will fail with a malicious string.