Skip to content
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
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
35 changes: 35 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Copy link

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
This is a comment left during a code review.
Path: README.md
Line: 79:79

Comment:
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).

How can I resolve this? If you propose a fix, please make it concise.

- **Structured JSON output** for programmatic consumption
- **Interactive review mode** with one-by-one fix workflow
- **Auto-fix commands** for findings that support automated remediation
Expand Down Expand Up @@ -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)
Expand All @@ -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

Expand All @@ -205,6 +210,36 @@ bash clawpinch.sh --fix

---

## Performance
Copy link

Choose a reason for hiding this comment

The 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 AI
This 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

```
Expand Down
93 changes: 91 additions & 2 deletions clawpinch.sh
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ SHOW_FIX=0
QUIET=0
NO_INTERACTIVE=0
REMEDIATE=0
PARALLEL_SCANNERS=1
CONFIG_DIR=""

# ─── Usage ───────────────────────────────────────────────────────────────────
Expand All @@ -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
Expand All @@ -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)
Expand Down Expand Up @@ -110,6 +113,66 @@ 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
Copy link

Choose a reason for hiding this comment

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

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.

Suggested change
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.


# Track background job PIDs
declare -a pids=()

# 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
if command -v python3 &>/dev/null; then
python3 "$scanner" > "$temp_file" 2>/dev/null || true
elif command -v python &>/dev/null; then
python "$scanner" > "$temp_file" 2>/dev/null || true
fi
Comment on lines +146 to +150

Choose a reason for hiding this comment

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

high

The fallback to python if python3 is not found is a potential bug. The Python scanner scripts in this repository require Python 3 features (e.g., f-strings, type hints) and will fail if run with Python 2. On many systems, python still points to Python 2. To prevent silent failures or runtime errors, it's safer to exclusively use python3 and not run the scanner if it's unavailable. This behavior would be consistent with how other failures are handled in the parallel runner.

Suggested change
if command -v python3 &>/dev/null; then
python3 "$scanner" > "$temp_file" 2>/dev/null || true
elif command -v python &>/dev/null; then
python "$scanner" > "$temp_file" 2>/dev/null || true
fi
if command -v python3 &>/dev/null; then
python3 "$scanner" > "$temp_file" 2>/dev/null || true
fi

fi
) &

pids+=("$!")
done

# Wait for all background jobs to complete
for pid in "${pids[@]}"; do
wait "$pid" 2>/dev/null || true
done

# Merge all JSON outputs
ALL_FINDINGS="[]"
for temp_file in "$temp_dir"/*.json; do
if [[ -f "$temp_file" ]]; then
output="$(cat "$temp_file")"
if [[ -n "$output" ]]; then
if echo "$output" | jq -e 'type == "array"' >/dev/null 2>&1; then
ALL_FINDINGS="$(echo "$ALL_FINDINGS" "$output" | jq -s '.[0] + .[1]')"
fi
fi
fi
done

Choose a reason for hiding this comment

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

high

The current implementation for merging JSON files from parallel scanners is inefficient. It calls jq inside a loop, once for each scanner, which creates unnecessary process overhead. This can be significantly optimized by using a single jq command to process all result files at once. This approach is more performant, idiomatic, and better aligns with the performance goals of this pull request.

Suggested change
ALL_FINDINGS="[]"
for temp_file in "$temp_dir"/*.json; do
if [[ -f "$temp_file" ]]; then
output="$(cat "$temp_file")"
if [[ -n "$output" ]]; then
if echo "$output" | jq -e 'type == "array"' >/dev/null 2>&1; then
ALL_FINDINGS="$(echo "$ALL_FINDINGS" "$output" | jq -s '.[0] + .[1]')"
fi
fi
fi
done
# Merge all JSON outputs in a single, efficient jq command.
local json_files=("$temp_dir"/*.json)
if [[ -e "${json_files[0]}" ]]; then
ALL_FINDINGS=$(jq -s 'add' "${json_files[@]}")
else
ALL_FINDINGS="[]"
fi


# Temp directory cleaned up by EXIT trap
}

# ─── Discover scanner scripts ───────────────────────────────────────────────

scanners=()
Expand Down Expand Up @@ -143,7 +206,32 @@ _SPINNER_PID=""
# Record scan start time
_scan_start="${EPOCHSECONDS:-$(date +%s)}"

for scanner in "${scanners[@]}"; do
# ─── Execute scanners (parallel or sequential) ──────────────────────────────

if [[ "$PARALLEL_SCANNERS" -eq 1 ]]; then
# Parallel execution
if [[ "$JSON_OUTPUT" -eq 0 ]] && [[ "$QUIET" -eq 0 ]]; then
start_spinner "Running ${scanner_count} scanners in parallel..."
fi

# Record parallel execution start time
_parallel_start="${EPOCHSECONDS:-$(date +%s)}"

run_scanners_parallel

# Calculate parallel execution elapsed time
_parallel_end="${EPOCHSECONDS:-$(date +%s)}"
_parallel_elapsed=$(( _parallel_end - _parallel_start ))

# Count findings from merged results
_parallel_count="$(echo "$ALL_FINDINGS" | jq 'length')"

if [[ "$JSON_OUTPUT" -eq 0 ]] && [[ "$QUIET" -eq 0 ]]; then
stop_spinner "Parallel scan" "$_parallel_count" "$_parallel_elapsed"
fi
else
# Sequential execution
for scanner in "${scanners[@]}"; do
scanner_idx=$((scanner_idx + 1))
scanner_name="$(basename "$scanner")"
scanner_base="${scanner_name%.*}"
Expand Down Expand Up @@ -200,7 +288,8 @@ for scanner in "${scanners[@]}"; do
if [[ "$JSON_OUTPUT" -eq 0 ]] && [[ "$QUIET" -eq 0 ]]; then
stop_spinner "$local_name" "$local_count" "$_scanner_elapsed"
fi
done
done
fi

# Calculate total scan time
_scan_end="${EPOCHSECONDS:-$(date +%s)}"
Expand Down