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:
https://github.com/Nautilus-Cyberneering/git-queue/blob/d47bd36bdad81487862ae73f3464a89c1a4fe9bd/src/git-repo-dir.ts#L9
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.
Tracking issue for:
The
Gitrepoclass contains a two calls toexecSync: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:
https://github.com/Nautilus-Cyberneering/git-queue/blob/d47bd36bdad81487862ae73f3464a89c1a4fe9bd/src/git-repo-dir.ts#L9
Anyway, we could refactor it in order to avoid getting alerts from
Code scanning.They suggest:
instead of:
For the Librarian we build a wrapper to execute console commands.
I think it's a good idea because:
dirclass validation is accidentally removed the console command will fail with a malicious string.