fix: resubmit 5 quality-debt fixes from closed batch PRs (batch 2)

[johnwaldo]

Summary

Second batch of fixes lost when batch PRs (#3916, #3843) were closed for blast radius (t1422 cap at 5 files). Each fix re-verified against current main.

Changes

File	Issue	Fix
.agents/scripts/settings-helper.sh	#3225	jq injection — ".$section.$key" interpolation replaced with --arg to prevent code injection via crafted key names
.agents/scripts/pre-commit-hook.sh	#3317	grep duplication — 5-stage grep pipeline ran twice (if-check + output); now runs once, stores in variable
.agents/scripts/commands/patterns.md	#3400	missing query — recall --type X --limit N errors without a search query; added required positional query
.agents/plugins/opencode-aidevops/tools.mjs	#3434	error masking — removed 2>/dev/null from memory recall/store commands
tests/test-smoke-help.sh	#3728	dead code — `

Closes #3225 Closes #3317 Closes #3400 Closes #3434 Closes #3728

[gemini-code-assist]

Summary of Changes

Hello, I'm Gemini Code Assist¹! 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 reintroduces five critical quality-debt fixes that were previously lost when earlier batch PRs were closed due to blast radius concerns. Each fix has been re-verified against the current main branch, addressing issues ranging from security vulnerabilities and redundant code execution to improved error handling and corrected test logic.

Highlights

• jq injection prevention: Updated jq commands in settings-helper.sh to use --arg for safer variable interpolation, preventing potential code injection via crafted key names.
• Optimized grep execution: Refactored pre-commit-hook.sh to execute a complex grep pipeline only once, storing its output in a variable to avoid redundant processing.
• Required query for memory recall: Added the necessary positional query argument to recall command examples in patterns.md, addressing errors when recall was invoked without a search query.
• Improved error visibility: Removed 2>/dev/null from memory-helper.sh calls within tools.mjs to ensure that error messages are no longer masked and are visible for debugging.
• Corrected timeout handling: Modified test-smoke-help.sh by removing || true from a timeout command, allowing the script to correctly detect and react to non-zero exit codes from timed-out processes.

Changelog

• .agents/plugins/opencode-aidevops/tools.mjs
  • Removed 2>/dev/null from the recall command to unmask errors.
  • Removed 2>/dev/null from the store command to unmask errors.
• .agents/scripts/commands/patterns.md
  • Added a positional query argument to recall command examples for success patterns.
  • Added a positional query argument to recall command examples for failure patterns.
  • Added a positional query argument to recall command examples for working solutions.
  • Added a positional query argument to recall command examples for failed approaches.
• .agents/scripts/pre-commit-hook.sh
  • Refactored the grep pipeline to execute once and store its output in a variable.
  • Updated conditional logic to check for violations_output before printing errors and incrementing the counter.
• .agents/scripts/settings-helper.sh
  • Updated jq command for listing keys to use --arg s "$section" for secure interpolation.
  • Updated jq command for getting values to use --arg s "$section" --arg k "$key" for secure interpolation.
• tests/test-smoke-help.sh
  • Removed || true from the timeout command for help_output capture to correctly handle exit codes.

Activity

• No human activity (comments, reviews, etc.) has been recorded for this pull request yet.

Using Gemini Code Assist

The 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 /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.

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

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[coderabbitai]

Important

Review skipped

Auto reviews are limited based on label configuration.

🚫 Review skipped — only excluded labels are configured. (1)

• no-review

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 88cc9936-da5f-4277-8177-400fd8bf7bec

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[gemini-code-assist]

Code Review

This pull request aims to bundle five distinct fixes, addressing issues from security and performance to documentation and test correctness. It successfully includes a fix for a jq injection vulnerability in settings-helper.sh, along with other improvements like eliminating a redundant grep pipeline, correcting example commands, and fixing dead code in a test script. However, a critical concern is that the removal of error masking (2>/dev/null) in tools.mjs exposes existing high-severity command injection vulnerabilities, as user-supplied arguments are still directly interpolated into shell commands, which remain unresolved.

[gemini-code-assist]

The args.query and args.limit variables are directly interpolated into a shell command. If these variables are controlled by an untrusted source (e.g., an LLM or a user), an attacker could inject arbitrary shell commands. For example, a query like "; touch /tmp/pwned; # could lead to command execution.

[johnwaldo]

Already addressed in the second commit — added shellEscape() helper (line 11-13) that wraps values in single quotes with proper escaping. All user-controlled values (args.query, args.limit, content, args.confidence) now pass through shellEscape() before shell interpolation (lines 279, 297).

[gemini-code-assist]

The content and args.confidence variables are directly interpolated into a shell command. If these variables are controlled by an untrusted source, an attacker could inject arbitrary shell commands. This poses a significant security risk as it allows for arbitrary code execution on the host system.

[johnwaldo]

Same fix — shellEscape() applied to both content and args.confidence at line 297. Shell injection is prevented by single-quote wrapping with internal quote escaping.

[johnwaldo]

@coderabbitai review

Addressed Gemini's security-high shell injection finding: added shellEscape() helper that wraps values in single quotes with internal quote escaping. Applied to all user-controlled values (args.query, args.limit, content, args.confidence) before interpolation into shell command strings.

[coderabbitai]

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Read the tools.mjs file to verify the shellEscape() implementation
fd "tools.mjs" --exec cat {}

echo "---SEPARATOR---"
# Also check the other files in the PR
fd "settings-helper.sh" .agents/scripts --exec cat {}

echo "---SEPARATOR---"
fd "pre-commit-hook.sh" .agents/scripts --exec cat {}

echo "---SEPARATOR---"
cat .agents/plugins/opencode-aidevops/tools.mjs 2>/dev/null || echo "File not found via direct path"

echo "---SEPARATOR---"
cat tests/test-smoke-help.sh 2>/dev/null | head -210

Length of output: 60371

---

@johnwaldo: The shellEscape() implementation is solid — the '\\'' POSIX technique correctly handles embedded single quotes. Coverage over the memory tool arguments (args.query, args.limit, content, args.confidence) closes the highest-risk vectors. A few remaining concerns worth addressing:

---

✅ shellEscape() — Correct Implementation

function shellEscape(str) {
  return "'" + String(str).replace(/'/g, "'\\''") + "'";
}

Standard and correct. Well-placed as a shared helper.

---

⚠️ Three Remaining Unescaped Interpolation Sites

1. createAidevopsTool — args.command not escaped

// Line in createAidevopsTool:
const cmd = `aidevops ${args.command || args}`;

args.command passes directly into the shell string. A malicious value like status; rm -rf / would be executed. Suggest:

const cmd = `aidevops ${shellEscape(String(args.command || args))}`;

2. createPreEditCheckTool — args.task in double quotes

const taskFlag = args.task
  ? ` --loop-mode --task "${args.task}"`
  : "";

Double-quoting prevents word splitting but not command substitution ($(...) or backticks). A value like $(rm -rf /) still executes. Suggest:

const taskFlag = args.task
  ? ` --loop-mode --task ${shellEscape(args.task)}`
  : "";

3. runHookHelper — action not escaped

const result = execSync(
  `bash "${helperScript}" ${action}`,  // action unquoted/unescaped
  ...
);

action comes from args.action || args || "install" — user-controlled. Suggest:

`bash "${helperScript}" ${shellEscape(String(action))}`

---

🔍 Minor: args.query Undefined Guard

If the LLM omits query, shellEscape(args.query) → shellEscape(undefined) → 'undefined'. Not a security issue, but it would silently pass undefined as a search term to memory-helper.sh. A lightweight guard would make failures explicit:

buildArgs: (args, helper) => {
  if (!args.query) {
    return { cmd: `echo "Error: query is required" >&2; exit 1`, timeout: 1000 };
  }
  return {
    cmd: `bash "${helper}" recall ${shellEscape(args.query)} --limit ${shellEscape(args.limit || "5")}`,
    timeout: 10000,
  };
},

---

Summary

Location	Status
memory_recall — args.query, args.limit	✅ Escaped
memory_store — content, args.confidence	✅ Escaped
createAidevopsTool — args.command	⚠️ Unescaped
createPreEditCheckTool — args.task	⚠️ Double-quoted only
runHookHelper — action	⚠️ Unescaped

The three remaining sites are lower-risk than the memory tools (they don't take arbitrary user data over a network), but they follow the same injection pattern. Would you like to address them in this PR or track as a follow-up?

---

🧠 Learnings used

Learnt from: marcusquinn
Repo: marcusquinn/aidevops PR: 0
File: :0-0
Timestamp: 2026-03-03T16:33:18.313Z
Learning: For the marcusquinn/aidevops repository: The daily quality sweep system (pulse-wrapper.sh) should only mention coderabbitai for full codebase review when there are significant changes (Quality Gate failure, +10 issues in one day, etc.), not on every automated sweep with stable metrics.

---

Learnt from: alex-solovyev
Repo: marcusquinn/aidevops PR: 0
File: :0-0
Timestamp: 2026-03-04T05:50:49.615Z
Learning: In the marcusquinn/aidevops repository: As of 2026-03-04, the chrome-webstore-helper.sh SC2119/SC2120 (load_credentials argument mismatch) and stash-audit-helper.sh SC2181 (indirect exit code checking) ShellCheck issues have been resolved. ShellCheck now reports 0 errors and 0 warnings for these files.

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[marcusquinn]

This PR is from an external contributor (@johnwaldo). Auto-merge is disabled for external PRs — a maintainer must review and approve manually.

---

To approve or decline, comment on this PR:

• approved — removes the review gate and allows merge (CI permitting)
• declined: <reason> — closes this PR (include your reason after the colon)