-
Notifications
You must be signed in to change notification settings - Fork 2
Parallelize independent scanner execution for 2-3x speedup #3
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.
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
Changes from 11 commits
2eb0f6b
7ada8d5
0475aa1
84559d8
6232003
c22a795
78f071a
268411c
26eec65
bbacfaf
25a100b
600f012
ad08cbb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -76,6 +76,7 @@ bash clawpinch.sh | |
| ## Features | ||
|
|
||
| - **63 checks** across 8 scanner categories | ||
| - **Parallel scanner execution** -- 2-3x faster scans by running all scanners concurrently (use `--sequential` for debugging) | ||
| - **Structured JSON output** for programmatic consumption | ||
| - **Interactive review mode** with one-by-one fix workflow | ||
| - **Auto-fix commands** for findings that support automated remediation | ||
|
|
@@ -179,6 +180,7 @@ In the interactive review mode, press `[a]` on any finding to copy a structured | |
|
|
||
| ```bash | ||
| # Standard interactive scan (review findings, auto-fix, export reports) | ||
| # Runs all scanners in parallel by default for 2-3x speedup | ||
| bash clawpinch.sh | ||
|
|
||
| # Deep scan (supply-chain hash verification, skill decompilation) | ||
|
|
@@ -196,6 +198,9 @@ bash clawpinch.sh --no-interactive | |
| # AI-powered remediation -- scan then pipe findings to Claude for automated fixing | ||
| bash clawpinch.sh --remediate | ||
|
|
||
| # Sequential mode -- run scanners one-by-one (for debugging) | ||
| bash clawpinch.sh --sequential | ||
|
|
||
| # Point at a custom config directory | ||
| bash clawpinch.sh --config-dir /path/to/openclaw/config | ||
|
|
||
|
|
@@ -205,6 +210,36 @@ bash clawpinch.sh --fix | |
|
|
||
| --- | ||
|
|
||
| ## Performance | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Claimed "1.5-3x faster" speedup doesn't match "2-3x faster" from title and line 79. Use consistent speedup claims throughout documentation. Prompt To Fix With AIThis is a comment left during a code review.
Path: README.md
Line: 213:213
Comment:
Claimed "1.5-3x faster" speedup doesn't match "2-3x faster" from title and line 79.
Use consistent speedup claims throughout documentation.
How can I resolve this? If you propose a fix, please make it concise. |
||
|
|
||
| **ClawPinch runs all 8 scanner categories in parallel by default**, achieving **2-3x faster scan times** compared to sequential execution. | ||
|
|
||
| ### Speedup Breakdown | ||
|
|
||
| - **Sequential mode**: 15-40 seconds (one scanner at a time) | ||
| - **Parallel mode** (default): 10-25 seconds (all scanners concurrently) | ||
| - **Speedup**: 1.5-3x faster (system-dependent) | ||
|
|
||
| **Note**: Actual speedup varies by system (CPU cores, I/O speed, scanner workload). Most systems see 1.5-2x improvement, with optimal systems reaching 3x. | ||
|
|
||
| Scanners are independent (configuration, secrets, network, skills, permissions, cron, CVE, supply chain) and have no dependencies between them, making parallel execution safe and efficient. | ||
|
|
||
| ### When to Use Sequential Mode | ||
|
|
||
| Use `--sequential` for debugging when: | ||
| - You need to isolate which scanner is causing an issue | ||
| - You're developing a new scanner and want predictable output ordering | ||
| - You're on a resource-constrained system | ||
|
|
||
| ```bash | ||
| # Run scanners one-by-one for debugging | ||
| bash clawpinch.sh --sequential | ||
| ``` | ||
|
|
||
| **Default behavior**: All scans run in parallel unless `--sequential` is specified. | ||
|
|
||
| --- | ||
|
|
||
| ## Example Output | ||
|
|
||
| ``` | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -23,6 +23,7 @@ SHOW_FIX=0 | |||||||||||||||||||
| QUIET=0 | ||||||||||||||||||||
| NO_INTERACTIVE=0 | ||||||||||||||||||||
| REMEDIATE=0 | ||||||||||||||||||||
| PARALLEL_SCANNERS=1 | ||||||||||||||||||||
| CONFIG_DIR="" | ||||||||||||||||||||
|
|
||||||||||||||||||||
| # ─── Usage ─────────────────────────────────────────────────────────────────── | ||||||||||||||||||||
|
|
@@ -36,6 +37,7 @@ Options: | |||||||||||||||||||
| --json Output findings as JSON array only | ||||||||||||||||||||
| --fix Show auto-fix commands in report | ||||||||||||||||||||
| --quiet Print summary line only | ||||||||||||||||||||
| --sequential Run scanners sequentially (default is parallel) | ||||||||||||||||||||
| --no-interactive Disable interactive post-scan menu | ||||||||||||||||||||
| --remediate Run scan then pipe findings to Claude for AI remediation | ||||||||||||||||||||
| --config-dir PATH Explicit path to openclaw config directory | ||||||||||||||||||||
|
|
@@ -56,6 +58,7 @@ while [[ $# -gt 0 ]]; do | |||||||||||||||||||
| --json) JSON_OUTPUT=1; shift ;; | ||||||||||||||||||||
| --fix) SHOW_FIX=1; shift ;; | ||||||||||||||||||||
| --quiet) QUIET=1; shift ;; | ||||||||||||||||||||
| --sequential) PARALLEL_SCANNERS=0; shift ;; | ||||||||||||||||||||
| --no-interactive) NO_INTERACTIVE=1; shift ;; | ||||||||||||||||||||
| --remediate) REMEDIATE=1; NO_INTERACTIVE=1; shift ;; | ||||||||||||||||||||
| --config-dir) | ||||||||||||||||||||
|
|
@@ -110,6 +113,60 @@ if [[ "$JSON_OUTPUT" -eq 0 ]] && [[ "$QUIET" -eq 0 ]]; then | |||||||||||||||||||
| printf '\n' | ||||||||||||||||||||
| fi | ||||||||||||||||||||
|
|
||||||||||||||||||||
| # ─── Parallel scanner execution function ──────────────────────────────────── | ||||||||||||||||||||
|
|
||||||||||||||||||||
| run_scanners_parallel() { | ||||||||||||||||||||
| local temp_dir="" | ||||||||||||||||||||
| temp_dir="$(mktemp -d "${TMPDIR:-/tmp}/clawpinch.XXXXXX")" | ||||||||||||||||||||
|
|
||||||||||||||||||||
| # Use EXIT trap for robust cleanup — covers INT, TERM, and set -e failures | ||||||||||||||||||||
| trap 'rm -rf "$temp_dir"' EXIT | ||||||||||||||||||||
|
||||||||||||||||||||
| trap 'rm -rf "$temp_dir"' EXIT | |
| trap 'rm -rf "$temp_dir"' RETURN |
Use RETURN trap instead - cleans up when function returns, not when script exits.
Prompt To Fix With AI
This is a comment left during a code review.
Path: clawpinch.sh
Line: 123:123
Comment:
EXIT trap conflicts with main script's `set -e` and potential other traps.
When `run_scanners_parallel` returns to main script, the EXIT trap remains active and fires at script end. If main script has its own cleanup or if user hits Ctrl+C, this trap will interfere.
```suggestion
trap 'rm -rf "$temp_dir"' RETURN
```
Use RETURN trap instead - cleans up when function returns, not when script exits.
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Check that subshell inherits CLAWPINCH_* environment variables needed by scanners.
Scanners expect CLAWPINCH_DEEP, CLAWPINCH_CONFIG_DIR, etc. (exported at lines 82-90). Verify these are properly inherited by subshells.
Prompt To Fix With AI
This is a comment left during a code review.
Path: clawpinch.sh
Line: 134:149
Comment:
Check that subshell inherits `CLAWPINCH_*` environment variables needed by scanners.
Scanners expect `CLAWPINCH_DEEP`, `CLAWPINCH_CONFIG_DIR`, etc. (exported at lines 82-90). Verify these are properly inherited by subshells.
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The parallel implementation for Python scanners silently skips them if python3 is not found. This is inconsistent with the sequential mode, which logs a warning. This could lead to confusion, as a user might not realize some checks were omitted.
I suggest adding a check for python3 once before launching the scanners and issuing a single warning if it's missing but Python scanners are present. This provides better feedback to the user without cluttering the output with multiple warnings from parallel processes.
# Check for python3 once if any Python scanners are present
local has_py_scanners=0
for scanner in "${scanners[@]}"; do
if [[ "$scanner" == *.py ]]; then
has_py_scanners=1
break
fi
done
if [[ "$has_py_scanners" -eq 1 ]] && ! command -v python3 &>/dev/null; then
log_warn "python3 not found, skipping Python-based scanners."
fi
# Launch all scanners in parallel
for scanner in "${scanners[@]}"; do
local scanner_name="$(basename "$scanner")"
local temp_file="$temp_dir/${scanner_name}.json"
# Run scanner in background, redirecting output to temp file
(
# Initialize with empty array in case scanner fails to run
echo '[]' > "$temp_file"
# Run scanner - exit code doesn't matter, we just need valid JSON output
# (Scanners exit with code 1 when they find critical findings, but still output valid JSON)
# Use command -v instead of has_cmd — bash functions aren't inherited by subshells
if [[ "$scanner" == *.sh ]]; then
bash "$scanner" > "$temp_file" 2>/dev/null || true
elif [[ "$scanner" == *.py ]]; then
# Python 3 only — scanners use f-strings and type hints that fail under Python 2
if command -v python3 &>/dev/null; then
python3 "$scanner" > "$temp_file" 2>/dev/null || true
fi
fi
) &
pids+=("$!")
doneThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the newly added run_scanners_parallel function, the merging of scanner results is performed using a single jq command. If jq is not installed, or if any single scanner produces invalid JSON output (which could be triggered by unexpected data in the environment being scanned), the entire jq command will fail. Because the command's stderr is redirected to /dev/null and it is followed by a fallback to an empty array (|| ALL_FINDINGS="[]"), this failure is completely silent. The orchestrator will proceed to report zero findings, creating a false sense of security.
In contrast, the sequential execution mode (lines 269-276) validates each scanner's output individually and provides warnings if a scanner fails to produce valid JSON. The parallel mode should adopt a similar robust approach to ensure that a single failure does not suppress all other security alerts.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Glob expansion not quoted - will fail if no .json files exist.
If all scanners fail or produce no output, "$temp_dir"/*.json expands to literal string *.json, causing jq to fail silently.
| local json_files=("$temp_dir"/*.json) | |
| if [[ -e "${json_files[0]}" ]]; then | |
| ALL_FINDINGS="$(jq -s 'add' "${json_files[@]}" 2>/dev/null)" || ALL_FINDINGS="[]" | |
| local json_files=("$temp_dir"/*.json) | |
| if [[ -e "${json_files[0]}" ]]; then | |
| ALL_FINDINGS="$(jq -s 'add' "${json_files[@]}" 2>/dev/null || echo '[]')" | |
| else | |
| ALL_FINDINGS="[]" | |
| fi |
Use command substitution fallback for jq failure.
Prompt To Fix With AI
This is a comment left during a code review.
Path: clawpinch.sh
Line: 160:162
Comment:
Glob expansion not quoted - will fail if no `.json` files exist.
If all scanners fail or produce no output, `"$temp_dir"/*.json` expands to literal string `*.json`, causing `jq` to fail silently.
```suggestion
local json_files=("$temp_dir"/*.json)
if [[ -e "${json_files[0]}" ]]; then
ALL_FINDINGS="$(jq -s 'add' "${json_files[@]}" 2>/dev/null || echo '[]')"
else
ALL_FINDINGS="[]"
fi
```
Use command substitution fallback for `jq` failure.
How can I resolve this? If you propose a fix, please make it concise.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still claims "2-3x faster" here but Performance section (line 215) says "1.5-3x faster".
Use "1.5-3x faster" consistently (matches actual benchmark data showing most systems get 1.5-2x).
Prompt To Fix With AI