feat: extend setup.sh to deploy plugins during aidevops update (t136.3)#762
Conversation
Add deploy_plugins() to setup.sh that runs after core agent deployment. Reads ~/.config/aidevops/plugins.json, installs missing enabled plugins, cleans up disabled plugin directories, and preserves plugin namespaces during clean mode. Plugin directories are excluded from rsync/tar to prevent core deployment from overwriting them. Update plugins.md to document automatic deployment behavior. Fixes: t136.3 ref:GH#730
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
WalkthroughAdds automatic plugin deployment support to the aidevops framework. Updates documentation about deployment behavior and extends setup.sh to preserve plugin namespaces, deploy enabled plugins, and clean disabled ones during agent deployment. Changes
Sequence Diagram(s)sequenceDiagram
participant Deploy as Agent Deployment
participant Plugins as plugins.json
participant PM as Plugin Manager<br/>(deploy_plugins)
participant FS as File System
Deploy->>Plugins: Read plugin namespaces
activate Plugins
Plugins-->>Deploy: Return namespace list
deactivate Plugins
Deploy->>FS: Copy agent files (exclude namespaces)
activate FS
Note over FS: rsync/tar preserves<br/>plugin directories
FS-->>Deploy: Copy complete
deactivate FS
Deploy->>PM: Call deploy_plugins(target, config)
activate PM
PM->>Plugins: Parse enabled plugins
PM->>FS: Clone enabled plugins
Note over FS: Install to target<br/>namespace dirs
PM->>FS: Clean disabled plugins
Note over FS: Remove disabled<br/>plugin directories
PM->>FS: Set permissions & log
PM-->>Deploy: Deployment complete
deactivate PM
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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: Mon Feb 9 05:26:06 UTC 2026 Generated by AI DevOps Framework Code Review Monitoring |
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.agents/aidevops/plugins.md (1)
7-7:⚠️ Potential issue | 🟡 MinorInconsistent config file path:
.aidevops.jsonvs~/.config/aidevops/plugins.jsonLine 7 states plugins are configured in
.aidevops.json, but thesetup.shimplementation reads from$HOME/.config/aidevops/plugins.json(Line 2545 of setup.sh), and Line 138 of this same file also references~/.config/aidevops/plugins.json. Users looking at the schema example may create the wrong file.
🤖 Fix all issues with AI agents
In `@setup.sh`:
- Around line 2756-2782: The loop uses the unvalidated namespace variable pns to
build clone_dir and pass to git clone, allowing path traversal; before
constructing clone_dir or calling git clone, validate and sanitize pns (and/or
normalize it) to reject absolute paths or any segment containing .. or /, and
restrict to a safe pattern (e.g., alphanumeric, hyphen, underscore) or map
invalid values to a safe fallback and log a warning; update the loop around
pns/clone_dir (the variables pns, clone_dir and the git clone block) to perform
this check and skip/mark the plugin as failed if the namespace is unsafe.
- Around line 2740-2747: The loop uses rm -rf "$target_dir/$disabled_ns"
unguarded, which can remove $target_dir when disabled_ns is empty or malicious;
change the code to (1) require a non-empty namespace via a parameter guard
(e.g., check that disabled_ns is non-empty instead of relying only on [[ -z ...
]] and avoid unquoted empty expansions), (2) validate disabled_ns against a safe
whitelist regex (e.g., ^[A-Za-z0-9_.-]+$) to block path traversal like ../, (3)
build an explicit path var (e.g., path="$target_dir/$disabled_ns"), verify that
path indeed is under $target_dir (or that disabled_ns passed the regex), and
only then run rm -rf "$path"; if validation fails, skip and log a warning
referencing disabled_ns, plugins_file, target_dir.
- Around line 2558-2564: The collected plugin_namespaces array is populated
directly from jq output (plugins_file) and later used in rsync exclude and
preserve lists, risking shell/path-injection; validate each namespace when
reading into plugin_namespaces by accepting only names matching the regex
^[a-z0-9][a-z0-9-]*$ (or implement a small helper function like
validate_namespace and call it from the collection loop) and skip/log invalid
entries so only safe namespace tokens enter plugin_namespaces before any rsync
or clean-mode usage.
🧹 Nitpick comments (1)
setup.sh (1)
2770-2772: Consider suppressing git clone stderr more selectively
git clone ... 2>/dev/nullswallows all stderr, including auth prompts and useful diagnostics. When a clone fails, the user sees only the generic "network or auth issue" warning with no actionable detail.♻️ Capture and display stderr on failure
- if git clone --branch "$pbranch" --depth 1 "$prepo" "$clone_dir" 2>/dev/null; then + local clone_err + if clone_err=$(git clone --branch "$pbranch" --depth 1 "$prepo" "$clone_dir" 2>&1); thenThen on failure:
else - print_warning " Failed to install plugin '$pname' (network or auth issue)" + print_warning " Failed to install plugin '$pname': ${clone_err##*$'\n'}" failed=$((failed + 1))
| # Collect plugin namespace directories to preserve during deployment | ||
| local -a plugin_namespaces=() | ||
| if [[ -f "$plugins_file" ]] && command -v jq &>/dev/null; then | ||
| while IFS= read -r ns; do | ||
| [[ -n "$ns" ]] && plugin_namespaces+=("$ns") | ||
| done < <(jq -r '.plugins[].namespace // empty' "$plugins_file" 2>/dev/null) | ||
| fi |
There was a problem hiding this comment.
Namespace collection also lacks validation — propagates to rsync exclusions and preserve lists
Unvalidated namespaces from plugins.json flow into rsync --exclude patterns (Line 2621) and the clean-mode preserve list (Lines 2578-2580). A namespace containing shell metacharacters or path separators could cause unexpected behavior. Apply the same ^[a-z0-9][a-z0-9-]*$ validation here at the collection point for defense-in-depth, or better yet, extract a shared validation helper.
🛡️ Proposed fix: validate at collection point
if [[ -f "$plugins_file" ]] && command -v jq &>/dev/null; then
while IFS= read -r ns; do
- [[ -n "$ns" ]] && plugin_namespaces+=("$ns")
+ [[ -z "$ns" ]] && continue
+ if [[ "$ns" =~ ^[a-z0-9][a-z0-9-]*$ ]]; then
+ plugin_namespaces+=("$ns")
+ else
+ print_warning "Ignoring invalid plugin namespace: $ns"
+ fi
done < <(jq -r '.plugins[].namespace // empty' "$plugins_file" 2>/dev/null)
fi🤖 Prompt for AI Agents
In `@setup.sh` around lines 2558 - 2564, The collected plugin_namespaces array is
populated directly from jq output (plugins_file) and later used in rsync exclude
and preserve lists, risking shell/path-injection; validate each namespace when
reading into plugin_namespaces by accepting only names matching the regex
^[a-z0-9][a-z0-9-]*$ (or implement a small helper function like
validate_namespace and call it from the collection loop) and skip/log invalid
entries so only safe namespace tokens enter plugin_namespaces before any rsync
or clean-mode usage.
| local disabled_ns | ||
| while IFS= read -r disabled_ns; do | ||
| [[ -z "$disabled_ns" ]] && continue | ||
| if [[ -d "$target_dir/$disabled_ns" ]]; then | ||
| rm -rf "$target_dir/$disabled_ns" | ||
| print_info " Removed disabled plugin directory: $disabled_ns" | ||
| fi | ||
| done < <(jq -r '.plugins[] | select(.enabled == false) | .namespace // empty' "$plugins_file" 2>/dev/null) |
There was a problem hiding this comment.
rm -rf on potentially empty variable can wipe $target_dir
If disabled_ns is empty (e.g., a malformed entry in plugins.json), rm -rf "$target_dir/$disabled_ns" expands to rm -rf "$target_dir/", deleting the entire agents directory. The static analysis tool flagged this as SC2115.
Additionally, disabled_ns is sourced from user-editable JSON with no path validation — a value like ../../.config would escape the target directory.
🛡️ Proposed fix: validate namespace and use :? guard
while IFS= read -r disabled_ns; do
[[ -z "$disabled_ns" ]] && continue
+ # Validate namespace: only allow lowercase alphanumeric and hyphens (no path traversal)
+ if [[ ! "$disabled_ns" =~ ^[a-z0-9][a-z0-9-]*$ ]]; then
+ print_warning " Skipping invalid plugin namespace: $disabled_ns"
+ continue
+ fi
- if [[ -d "$target_dir/$disabled_ns" ]]; then
- rm -rf "$target_dir/$disabled_ns"
+ if [[ -d "$target_dir/${disabled_ns:?}" ]]; then
+ rm -rf "${target_dir:?}/${disabled_ns:?}"
print_info " Removed disabled plugin directory: $disabled_ns"
fi
done < <(jq -r '.plugins[] | select(.enabled == false) | .namespace // empty' "$plugins_file" 2>/dev/null)📝 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.
| local disabled_ns | |
| while IFS= read -r disabled_ns; do | |
| [[ -z "$disabled_ns" ]] && continue | |
| if [[ -d "$target_dir/$disabled_ns" ]]; then | |
| rm -rf "$target_dir/$disabled_ns" | |
| print_info " Removed disabled plugin directory: $disabled_ns" | |
| fi | |
| done < <(jq -r '.plugins[] | select(.enabled == false) | .namespace // empty' "$plugins_file" 2>/dev/null) | |
| local disabled_ns | |
| while IFS= read -r disabled_ns; do | |
| [[ -z "$disabled_ns" ]] && continue | |
| # Validate namespace: only allow lowercase alphanumeric and hyphens (no path traversal) | |
| if [[ ! "$disabled_ns" =~ ^[a-z0-9][a-z0-9-]*$ ]]; then | |
| print_warning " Skipping invalid plugin namespace: $disabled_ns" | |
| continue | |
| fi | |
| if [[ -d "$target_dir/${disabled_ns:?}" ]]; then | |
| rm -rf "${target_dir:?}/${disabled_ns:?}" | |
| print_info " Removed disabled plugin directory: $disabled_ns" | |
| fi | |
| done < <(jq -r '.plugins[] | select(.enabled == false) | .namespace // empty' "$plugins_file" 2>/dev/null) |
🧰 Tools
🪛 GitHub Check: CodeFactor
[warning] 2744-2744: setup.sh#L2744
Use "${var:?}" to ensure this never expands to / . (SC2115)
🤖 Prompt for AI Agents
In `@setup.sh` around lines 2740 - 2747, The loop uses rm -rf
"$target_dir/$disabled_ns" unguarded, which can remove $target_dir when
disabled_ns is empty or malicious; change the code to (1) require a non-empty
namespace via a parameter guard (e.g., check that disabled_ns is non-empty
instead of relying only on [[ -z ... ]] and avoid unquoted empty expansions),
(2) validate disabled_ns against a safe whitelist regex (e.g.,
^[A-Za-z0-9_.-]+$) to block path traversal like ../, (3) build an explicit path
var (e.g., path="$target_dir/$disabled_ns"), verify that path indeed is under
$target_dir (or that disabled_ns passed the regex), and only then run rm -rf
"$path"; if validation fails, skip and log a warning referencing disabled_ns,
plugins_file, target_dir.
| while IFS=$'\t' read -r pname prepo pns pbranch; do | ||
| [[ -z "$pname" ]] && continue | ||
| pbranch="${pbranch:-main}" | ||
| local clone_dir="$target_dir/$pns" | ||
|
|
||
| if [[ -d "$clone_dir" ]]; then | ||
| # Plugin directory exists — skip re-clone during setup | ||
| # Users can force update via: aidevops plugin update [name] | ||
| skipped=$((skipped + 1)) | ||
| continue | ||
| fi | ||
|
|
||
| # Clone plugin repo | ||
| print_info " Installing plugin '$pname' ($prepo)..." | ||
| if git clone --branch "$pbranch" --depth 1 "$prepo" "$clone_dir" 2>/dev/null; then | ||
| # Remove .git directory (tracked via plugins.json, not nested git) | ||
| rm -rf "$clone_dir/.git" | ||
| # Set permissions on any scripts | ||
| if [[ -d "$clone_dir/scripts" ]]; then | ||
| chmod +x "$clone_dir/scripts/"*.sh 2>/dev/null || true | ||
| fi | ||
| deployed=$((deployed + 1)) | ||
| else | ||
| print_warning " Failed to install plugin '$pname' (network or auth issue)" | ||
| failed=$((failed + 1)) | ||
| fi | ||
| done < <(jq -r '.plugins[] | select(.enabled != false) | [.name, .repo, .namespace, (.branch // "main")] | @tsv' "$plugins_file" 2>/dev/null) |
There was a problem hiding this comment.
Same path traversal risk applies to enabled plugin clone paths
The pns (namespace) value from plugins.json is used directly in clone_dir="$target_dir/$pns" (Line 2759) and passed to git clone (Line 2770). A malicious or misconfigured namespace like ../../../tmp/evil would clone outside the agents directory. The same validation should be applied here.
🛡️ Proposed fix: validate namespace before clone
while IFS=$'\t' read -r pname prepo pns pbranch; do
[[ -z "$pname" ]] && continue
pbranch="${pbranch:-main}"
+ # Validate namespace: only allow lowercase alphanumeric and hyphens
+ if [[ ! "$pns" =~ ^[a-z0-9][a-z0-9-]*$ ]]; then
+ print_warning " Skipping plugin '$pname': invalid namespace '$pns'"
+ failed=$((failed + 1))
+ continue
+ fi
local clone_dir="$target_dir/$pns"📝 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.
| while IFS=$'\t' read -r pname prepo pns pbranch; do | |
| [[ -z "$pname" ]] && continue | |
| pbranch="${pbranch:-main}" | |
| local clone_dir="$target_dir/$pns" | |
| if [[ -d "$clone_dir" ]]; then | |
| # Plugin directory exists — skip re-clone during setup | |
| # Users can force update via: aidevops plugin update [name] | |
| skipped=$((skipped + 1)) | |
| continue | |
| fi | |
| # Clone plugin repo | |
| print_info " Installing plugin '$pname' ($prepo)..." | |
| if git clone --branch "$pbranch" --depth 1 "$prepo" "$clone_dir" 2>/dev/null; then | |
| # Remove .git directory (tracked via plugins.json, not nested git) | |
| rm -rf "$clone_dir/.git" | |
| # Set permissions on any scripts | |
| if [[ -d "$clone_dir/scripts" ]]; then | |
| chmod +x "$clone_dir/scripts/"*.sh 2>/dev/null || true | |
| fi | |
| deployed=$((deployed + 1)) | |
| else | |
| print_warning " Failed to install plugin '$pname' (network or auth issue)" | |
| failed=$((failed + 1)) | |
| fi | |
| done < <(jq -r '.plugins[] | select(.enabled != false) | [.name, .repo, .namespace, (.branch // "main")] | @tsv' "$plugins_file" 2>/dev/null) | |
| while IFS=$'\t' read -r pname prepo pns pbranch; do | |
| [[ -z "$pname" ]] && continue | |
| pbranch="${pbranch:-main}" | |
| # Validate namespace: only allow lowercase alphanumeric and hyphens | |
| if [[ ! "$pns" =~ ^[a-z0-9][a-z0-9-]*$ ]]; then | |
| print_warning " Skipping plugin '$pname': invalid namespace '$pns'" | |
| failed=$((failed + 1)) | |
| continue | |
| fi | |
| local clone_dir="$target_dir/$pns" | |
| if [[ -d "$clone_dir" ]]; then | |
| # Plugin directory exists — skip re-clone during setup | |
| # Users can force update via: aidevops plugin update [name] | |
| skipped=$((skipped + 1)) | |
| continue | |
| fi | |
| # Clone plugin repo | |
| print_info " Installing plugin '$pname' ($prepo)..." | |
| if git clone --branch "$pbranch" --depth 1 "$prepo" "$clone_dir" 2>/dev/null; then | |
| # Remove .git directory (tracked via plugins.json, not nested git) | |
| rm -rf "$clone_dir/.git" | |
| # Set permissions on any scripts | |
| if [[ -d "$clone_dir/scripts" ]]; then | |
| chmod +x "$clone_dir/scripts/"*.sh 2>/dev/null || true | |
| fi | |
| deployed=$((deployed + 1)) | |
| else | |
| print_warning " Failed to install plugin '$pname' (network or auth issue)" | |
| failed=$((failed + 1)) | |
| fi | |
| done < <(jq -r '.plugins[] | select(.enabled != false) | [.name, .repo, .namespace, (.branch // "main")] | `@tsv`' "$plugins_file" 2>/dev/null) |
🤖 Prompt for AI Agents
In `@setup.sh` around lines 2756 - 2782, The loop uses the unvalidated namespace
variable pns to build clone_dir and pass to git clone, allowing path traversal;
before constructing clone_dir or calling git clone, validate and sanitize pns
(and/or normalize it) to reject absolute paths or any segment containing .. or
/, and restrict to a safe pattern (e.g., alphanumeric, hyphen, underscore) or
map invalid values to a safe fallback and log a warning; update the loop around
pns/clone_dir (the variables pns, clone_dir and the git clone block) to perform
this check and skip/mark the plugin as failed if the namespace is unsafe.
1 participant
Summary
deploy_plugins()function tosetup.shthat deploys enabled plugins from~/.config/aidevops/plugins.jsonafter core agent deploymentplugins.mdto document automatic deployment behaviorDetails
Part of the Plugin System plan (t136). Follows t136.2 (plugin CLI commands, PR #759).
How it works
deploy_aidevops_agents()readsplugins.jsonto collect plugin namespacesdeploy_plugins()is called:aidevops plugin updateto refresh)Edge cases handled
plugins.jsonfile: silently skipsjqavailable: warns and skipsTesting
bash -n setup.sh-- syntax OKFixes: t136.3 ref:GH#730
Summary by CodeRabbit
New Features
Documentation