Security: Command injection risk in global teardown process-kill commands

[tomaioo]

Summary

Security: Command injection risk in global teardown process-kill commands

Problem

Severity: High | File: e2e/global-teardown.ts:L16

The teardown script builds shell commands with BACKEND_PORT from environment variables and passes them to execSync (pgrep -f 'e2e:${BACKEND_PORT}' / pkill -f 'e2e:${BACKEND_PORT}'). If BACKEND_PORT is attacker-controlled (e.g., in CI or local env), shell metacharacters or quote-breaking payloads could inject arbitrary commands.

Solution

Avoid shell interpolation. Use spawn/execFile with argument arrays and strict validation of BACKEND_PORT (e.g., /^\d{2,5}$/). Example: validate port, then call pgrep/pkill with args ['-f', e2e:${port}] without invoking a shell.

Changes

• e2e/global-teardown.ts (modified)

[linux-foundation-easycla]

• ❌ - login: @tomaioo / name: tomaioo . The commit (c22cab0) is not authorized under a signed CLA. Please click here to be authorized. For further assistance with EasyCLA, please submit a support request ticket.

[norman-abramovitz]

Thanks for the patch. The code change itself is clean — execFileSync with argv avoids shell interpretation entirely, and the /^\d{2,5}$/ guard adds a nice defense-in-depth check. It's a straightforward hygiene improvement and I'm fine merging it on those grounds.

That said, I'm not going to characterize this as a security issue. BACKEND_PORT is read from the environment of a developer-invoked test-teardown script — anyone able to set it already has local code execution and doesn't need an injection vector. No trust boundary is crossed, no untrusted input reaches this path, no production code runs this file. This is a linter pattern-match, not a vulnerability — please label future contributions as hygiene rather than severity-rated security issues.

Dismissing — posting as comment instead.

[norman-abramovitz]

Thanks for the patch. The code change itself is clean — execFileSync with argv avoids shell interpretation entirely, and the /^\d{2,5}$/ guard adds a nice defense-in-depth check. It's a straightforward hygiene improvement and I'm fine merging it on those grounds.

That said, I'm not going to characterize this as a security issue. BACKEND_PORT is read from the environment of a developer-invoked test-teardown script — anyone able to set it already has local code execution and doesn't need an injection vector. No trust boundary is crossed, no untrusted input reaches this path, no production code runs this file. This is a linter pattern-match, not a vulnerability — please label future contributions as hygiene rather than severity-rated security issues.

[norman-abramovitz]

Thanks for catching this and for the patch. Unfortunately we can't merge it because the EasyCLA check hasn't been signed, and it has been open since April. The same fix (credited to you) has landed via #5555, so I'm closing this one. If you sign the CLA in the future we'd be glad to take contributions directly.