t1027: Refactor opencode-aidevops/index.mjs — reduce complexity, extract tools module#1349
t1027: Refactor opencode-aidevops/index.mjs — reduce complexity, extract tools module#1349marcusquinn merged 2 commits intomainfrom
Conversation
… detectOhMyOpenCode, registerMcpServers, validateReturnStatements (t1027) - Extract all 6 tool definitions from index.mjs into tools.mjs module - Deduplicate memory_recall/memory_store via createMemoryTool() factory (16 lines each -> shared) - Split detectOhMyOpenCode(37) into detectOmocInOpenCodeConfig, detectOmocConfigFiles, parseOmocConfig, finalizeOmocState - Split registerMcpServers(44) into shouldSkipMcp, buildMcpConfigEntry, registerSingleMcp, applyMcpToolPermissions, disableStaleOmocPatterns - Split validateReturnStatements(39) into matchFunctionDef, braceDepthDelta, hasReturnStatement, recordMissingReturn - Add shared parseJsonc utility for JSONC comment stripping - Remove unused relative import from path
|
Caution Review failedThe pull request is closed. WalkthroughRefactored aidevops plugin to extract tool definitions into a new tools.mjs module, reorganizing OMOC detection and MCP registration logic into modular helpers, and enhancing quality gate tracking for shell/markdown/security checks while maintaining the existing AidevopsPlugin API. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🔍 Code Quality Report�[0;35m[MONITOR]�[0m Code Review Monitoring Report �[0;34m[INFO]�[0m Latest Quality Status: �[0;34m[INFO]�[0m Recent monitoring activity: 📈 Current Quality Metrics
Generated on: Fri Feb 13 11:37:48 UTC 2026 Generated by AI DevOps Framework Code Review Monitoring |
…1027) - Extract isWriteOrEditTool, isBashTool helper predicates - Extract logQualityGateResult for shared quality gate logging - Extract recordGitPattern and trackBashOperation from toolExecuteAfter - toolExecuteAfter: 23 -> 8 cyclomatic complexity - toolExecuteBefore: 19 -> 10 cyclomatic complexity - All functions now under 20 (max 16, eslint verified)
🔍 Code Quality Report�[0;35m[MONITOR]�[0m Code Review Monitoring Report �[0;34m[INFO]�[0m Latest Quality Status: �[0;34m[INFO]�[0m Recent monitoring activity: 📈 Current Quality Metrics
Generated on: Fri Feb 13 11:40:52 UTC 2026 Generated by AI DevOps Framework Code Review Monitoring |
Summary of ChangesHello @marcusquinn, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request undertakes a significant refactoring of the Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request is an excellent refactoring effort that significantly reduces complexity and improves the maintainability of the opencode-aidevops plugin by extracting helper functions and a dedicated tools.mjs module. The use of dependency injection for the tools is a great pattern.
However, the review identified several critical command injection vulnerabilities in the newly created tools.mjs file where user-provided arguments (from an LLM) are used to construct shell commands. These must be addressed. I've also included a couple of minor suggestions to improve code clarity.
| const taskFlag = args.task | ||
| ? ` --loop-mode --task "${args.task}"` | ||
| : ""; | ||
| try { | ||
| const result = execSync(`bash "${script}"${taskFlag}`, { | ||
| encoding: "utf-8", | ||
| timeout: 10000, | ||
| stdio: ["pipe", "pipe", "pipe"], | ||
| }); | ||
| return `Pre-edit check PASSED (exit 0):\n${result.trim()}`; | ||
| } catch (err) { | ||
| const code = err.status || 1; | ||
| const cmdOutput = (err.stdout || "") + (err.stderr || ""); | ||
| return `Pre-edit check exit ${code}: ${PRE_EDIT_GUIDANCE[code] || "Unknown"}\n${cmdOutput.trim()}`; | ||
| } | ||
| }, |
There was a problem hiding this comment.
Constructing a shell command by concatenating strings with user input is vulnerable to command injection. An attacker could craft args.task to break out of the quotes and execute arbitrary commands. To fix this, use the array-based version of execSync to pass arguments, which avoids shell interpretation.
try {
const execArgs = [script];
if (args.task) {
execArgs.push("--loop-mode", "--task", args.task);
}
const result = execSync("bash", execArgs, {
encoding: "utf-8",
timeout: 10000,
stdio: ["pipe", "pipe", "pipe"],
});
return `Pre-edit check PASSED (exit 0):\n${result.trim()}`;
} catch (err) {
const code = err.status || 1;
const cmdOutput = (err.stdout || "") + (err.stderr || "");
return `Pre-edit check exit ${code}: ${PRE_EDIT_GUIDANCE[code] || "Unknown"}\n${cmdOutput.trim()}`;
}| try { | ||
| const result = execSync( | ||
| `bash "${helperScript}" ${action}`, | ||
| { | ||
| encoding: "utf-8", | ||
| timeout: 15000, | ||
| stdio: ["pipe", "pipe", "pipe"], | ||
| }, | ||
| ); | ||
| return result.trim(); | ||
| } catch (err) { | ||
| const cmdOutput = (err.stdout || "") + (err.stderr || ""); | ||
| return `Hook ${action} failed:\n${cmdOutput.trim()}`; | ||
| } |
There was a problem hiding this comment.
The action argument is interpolated into the shell command without quoting or validation, creating a command injection vulnerability. The fix is to validate the action against an allowlist and use the array-based execSync signature to prevent shell interpretation.
const ALLOWED_ACTIONS = ["install", "uninstall", "status", "test"];
if (!ALLOWED_ACTIONS.includes(action)) {
return `Invalid action: "${action}". Must be one of: ${ALLOWED_ACTIONS.join(", ")}`;
}
try {
const result = execSync("bash", [helperScript, action], {
encoding: "utf-8",
timeout: 15000,
stdio: ["pipe", "pipe", "pipe"],
});
return result.trim();
} catch (err) {
const cmdOutput = (err.stdout || "") + (err.stderr || "");
return `Hook ${action} failed:\n${cmdOutput.trim()}`;
}| * @param {function} pipelines.scanForSecrets | ||
| * @returns {Record<string, object>} | ||
| */ | ||
| export function createTools(scriptsDir, run, pipelines) { |
There was a problem hiding this comment.
Multiple tools created in this function are vulnerable to command injection because they use the run function with a command string constructed with unsanitized user input. This is a critical security risk.
Vulnerable tools and arguments:
aidevops:args.commandorargsaidevops_memory_recall:args.queryaidevops_memory_store:args.content
An attacker (or a compromised LLM) could provide a malicious string like status; rm -rf / or foo $(rm -rf /) to execute arbitrary commands.
Recommendation:
The run function should be refactored to accept arguments safely, without shell interpretation. The ideal way is to use an array-based API for executing commands, similar to Node.js's execFile or spawn. The tool definitions should then be updated to pass arguments as an array. For example: run('bash', [helper, 'recall', args.query, ...]).
| function isWriteOrEditTool(tool) { | ||
| return tool === "Write" || tool === "Edit" || tool === "write" || tool === "edit"; | ||
| } |
There was a problem hiding this comment.
This check can be simplified by converting the input to a consistent case first, which makes it more robust and easier to read.
| function isWriteOrEditTool(tool) { | |
| return tool === "Write" || tool === "Edit" || tool === "write" || tool === "edit"; | |
| } | |
| function isWriteOrEditTool(tool) { | |
| const lowerTool = tool.toLowerCase(); | |
| return lowerTool === "write" || lowerTool === "edit"; | |
| } |
| function isBashTool(tool) { | ||
| return tool === "Bash" || tool === "bash"; | ||
| } |
There was a problem hiding this comment.
This check can be simplified by converting the input to lowercase first.
| function isBashTool(tool) { | |
| return tool === "Bash" || tool === "bash"; | |
| } | |
| function isBashTool(tool) { | |
| return tool.toLowerCase() === "bash"; | |
| } |
1 participant
Summary
Refactor
.agents/plugins/opencode-aidevops/index.mjsto reduce complexity and improve maintainability.Changes
tools.mjsmodule (~180 lines moved out)createMemoryTool()factory pattern (memory_recall and memory_store shared 16 identical lines each)registerMcpServerscomplexity: 44 → 6 (extractedshouldSkipMcp,buildMcpConfigEntry,registerSingleMcp,applyMcpToolPermissions,disableStaleOmocPatterns)validateReturnStatementscomplexity: 39 → 13 (extractedmatchFunctionDef,braceDepthDelta,hasReturnStatement,recordMissingReturn)detectOhMyOpenCodecomplexity: 37 → 6 (extracteddetectOmocInOpenCodeConfig,detectOmocConfigFiles,parseOmocConfig,finalizeOmocState,parseJsonc)createToolscomplexity: 32 → 3 (moved to tools.mjs with dependency injection)toolExecuteAftercomplexity: 23 → 8 (extractedisBashTool,recordGitPattern,trackBashOperation)toolExecuteBeforecomplexity: 19 → 10 (extractedisWriteOrEditTool,logQualityGateResult)relativeimport, consolidateosimportsVerification
node --checkpasses on both filesRef #1347
Summary by CodeRabbit
New Features
Refactor