Skip to content

refactor(benchmark): use shell: false and quote args in benchmark-compare workflow #68136

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
herdiyana256 wants to merge 1 commit into
base: main
Choose a base branch
from
+10 −4
Open

refactor(benchmark): use shell: false and quote args in benchmark-compare workflow #68136

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .github/workflows/benchmark-compare.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,8 @@ jobs:
COMMENT_BODY: ${{ github.event.comment.body }}
run: pnpm benchmarks prepare-for-github-action "$COMMENT_BODY"

- run: pnpm benchmarks run-compare ${{steps.info.outputs.compareSha}} ${{steps.info.outputs.benchmarkTarget}}
- run: pnpm benchmarks run-compare ${{steps.info.outputs.compareSha}} "${{steps.info.outputs.benchmarkTarget}}"

id: benchmark
name: Running benchmark

Expand Down
11 changes: 8 additions & 3 deletions scripts/benchmarks/utils.mts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,20 @@ const scriptDir = path.dirname(url.fileURLToPath(import.meta.url));
export const projectDir: string = path.join(scriptDir, '../..');

/**
* Executes the given command, forwarding stdin, stdout and stderr while
* still capturing stdout in order to return it.
* Executes the given command with the provided arguments. Arguments are passed
* as a discrete array to the child process, bypassing shell interpretation.
* This ensures that special shell characters within arguments are treated as
* literal values and cannot be used to inject additional commands.
*/
export function exec(cmd: string, args: string[] = []): Promise<string> {
return new Promise((resolve, reject) => {
Log.info('Running command:', cmd, args.join(' '));

const proc = childProcess.spawn(cmd, args, {
shell: true,
// Do not use a shell to spawn the process. This ensures that arguments
// are passed directly to the executable without shell interpretation,
// preventing injection via shell metacharacters.
shell: false,
cwd: projectDir,
// Only capture `stdout`. Forward the rest to the parent TTY.
stdio: ['inherit', 'pipe', 'inherit'],
Expand Down
Loading