Skip to content

🛡️ Sentinel: [CRITICAL] Fix command injection vulnerabilities #493

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
Theory903 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

🛡️ Sentinel: [CRITICAL] Fix command injection vulnerabilities #493

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
5 changes: 5 additions & 0 deletions .jules/sentinel.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,8 @@
**Vulnerability:** Hardcoded API key ("ippoc-secret-key") used as default in `src/cortex/cortex/server.py`.
**Learning:** Default configurations for development often make their way into production or expose systems during testing if not explicitly overridden. The system relied on a specific hardcoded string for default auth, which is a Critical vulnerability (CWE-798).
**Prevention:** Never provide a hardcoded default for secrets. If a secret is missing, either generate a secure random one at runtime (fail-safe) or refuse to start (fail-secure).

## 2025-02-12 - Prevent Command Injection via execFile
**Vulnerability:** Found multiple usages of `child_process.exec` (via `promisify(exec)`) constructed using template literals or string concatenation, presenting a command injection vulnerability.
**Learning:** Using `exec` invokes a shell, making it susceptible to shell metacharacters if user input is interpolated into the command string. `execFile` avoids the shell and passes arguments directly to the executable.
**Prevention:** Always use `execFile` or `spawn` with an array of arguments instead of `exec` when invoking external processes, to ensure arguments are not evaluated by a shell.
10 changes: 7 additions & 3 deletions src/cortex/cortex/openclaw-cortex/openclaw-cortex/src/agents/toolsmith.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -262,15 +262,19 @@ export class ToolSmith {
}

try {
const { exec } = require("child_process");
const { execFile } = require("child_process");
const util = require("util");
const execAsync = util.promisify(exec);
const execAsync = util.promisify(execFile);

const resourceFlag = resources.length > 0 ? `--resources ${resources.join(",")}` : "";
const cmd = `python3 "${scriptPath}" ${name} --path "${pathStr}" ${resourceFlag}`;

console.log(`[ToolSmith] Executing: ${cmd}`);
const { stdout, stderr } = await execAsync(cmd);
const args = [scriptPath, name, "--path", pathStr];
Comment on lines 269 to +273

@coderabbitai coderabbitai Bot Jun 23, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win

Remove shell-style interpolated cmd logging now that execFile is in use.

execFile fixed shell injection, but building/logging an interpolated command string from name, pathStr, and resources still leaves a log-injection/leak surface and keeps the old unsafe pattern alive.

Suggested patch 
-        const resourceFlag = resources.length > 0 ? `--resources ${resources.join(",")}` : "";
-        const cmd = `python3 "${scriptPath}" ${name} --path "${pathStr}" ${resourceFlag}`;
-        
-        console.log(`[ToolSmith] Executing: ${cmd}`);
         const args = [scriptPath, name, "--path", pathStr];
         if (resources.length > 0) {
             args.push("--resources", resources.join(","));
         }
+        console.log("[ToolSmith] Executing python3 with args:", args);
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/cortex/cortex/openclaw-cortex/openclaw-cortex/src/agents/toolsmith.ts`
around lines 269 - 273, Remove the interpolated command string logging that uses
the `cmd` variable, which is built by combining name, pathStr, and resourceFlag
using template literal syntax. Since execFile is now in use (which doesn't
invoke a shell and takes separate arguments), the shell-style cmd variable is no
longer needed and creates an unnecessary log-injection/leak surface. Delete the
line that builds the cmd variable and the console.log statement that logs it,
keeping only the args array which properly separates the command arguments for
execFile.

if (resources.length > 0) {
args.push("--resources", resources.join(","));
}
const { stdout, stderr } = await execAsync("python3", args);

console.log(stdout);
if (stderr) console.warn(stderr);
Expand Down
10 changes: 7 additions & 3 deletions src/ippoc/cortex/cortex/openclaw-cortex/src/agents/toolsmith.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -262,15 +262,19 @@ export class ToolSmith {
}

try {
const { exec } = require("child_process");
const { execFile } = require("child_process");
const util = require("util");
const execAsync = util.promisify(exec);
const execAsync = util.promisify(execFile);

const resourceFlag = resources.length > 0 ? `--resources ${resources.join(",")}` : "";
const cmd = `python3 "${scriptPath}" ${name} --path "${pathStr}" ${resourceFlag}`;

console.log(`[ToolSmith] Executing: ${cmd}`);
const { stdout, stderr } = await execAsync(cmd);
const args = [scriptPath, name, "--path", pathStr];
Comment on lines 269 to +273

@coderabbitai coderabbitai Bot Jun 23, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win

Apply the same cleanup here: avoid interpolated command-string logging.

This copy still constructs/logs cmd from user-influenced values; prefer structured argument logging and remove the interpolated shell-like string.

Suggested patch 
-        const resourceFlag = resources.length > 0 ? `--resources ${resources.join(",")}` : "";
-        const cmd = `python3 "${scriptPath}" ${name} --path "${pathStr}" ${resourceFlag}`;
-        
-        console.log(`[ToolSmith] Executing: ${cmd}`);
         const args = [scriptPath, name, "--path", pathStr];
         if (resources.length > 0) {
             args.push("--resources", resources.join(","));
         }
+        console.log("[ToolSmith] Executing python3 with args:", args);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const resourceFlag = resources.length > 0 ? `--resources ${resources.join(",")}` : "";
const cmd = `python3 "${scriptPath}" ${name} --path "${pathStr}" ${resourceFlag}`;
console.log(`[ToolSmith] Executing: ${cmd}`);
const { stdout, stderr } = await execAsync(cmd);
const args = [scriptPath, name, "--path", pathStr];
const args = [scriptPath, name, "--path", pathStr];
if (resources.length > 0) {
args.push("--resources", resources.join(","));
}
console.log("[ToolSmith] Executing python3 with args:", args);
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/ippoc/cortex/cortex/openclaw-cortex/src/agents/toolsmith.ts` around lines
269 - 273, The code constructs an interpolated command string in the `cmd`
variable from user-influenced values and logs it with console.log, which should
be avoided for security and clarity reasons. Remove the `cmd` variable that
constructs the interpolated command string and refactor the console.log
statement to use structured argument logging instead. Log the individual
arguments (scriptPath, name, pathStr, and resource flags) in a structured format
rather than logging the interpolated shell-like command string. Ensure that the
resourceFlag is also properly included in the args array when present.

if (resources.length > 0) {
args.push("--resources", resources.join(","));
}
const { stdout, stderr } = await execAsync("python3", args);

console.log(stdout);
if (stderr) console.warn(stderr);
Expand Down
Loading