feat(skills): add clawsec-clawhub-checker reputation checking skill#41
Conversation
- Adds ClawHub reputation checks to guarded installer - Integrates VirusTotal Code Insight scores - Requires --confirm-reputation for suspicious skills - Enhances advisory guardian hook with reputation warnings - Defense-in-depth layer for skill installation security
- Enhanced guarded installer with reputation checks - VirusTotal Code Insight integration - Reputation scoring (0-100) with multiple signals - New exit code 43 for reputation warnings - Requires --confirm-reputation for suspicious skills - Integration with clawsec-advisory-guardian hook - Standalone skill compatible with dynamic catalog system Note: Removed hardcoded catalog entry to work with new dynamic catalog system (discover_skill_catalog.mjs).
- Remove unused imports (fs, os, path) from check_clawhub_reputation.mjs - Remove unused variable in setup_reputation_hook.mjs - Remove unused os import from update_suite_catalog.mjs - All ESLint checks now pass - TypeScript check passes - Build check passes
f70aa5e to
e6b9e90
Compare
PR Notes for ClawSec ClawHub CheckerImportant Limitation NoticeThis skill currently catches VirusTotal Code Insight flags but cannot access OpenClaw internal check results because:
Example from
|
…d SKILL.md feat: add input validation for skill slug and version in check_clawhub_reputation.mjs fix: enhance argument parsing in enhanced_guarded_install.mjs test: add reputation check tests for input validation and output formatting chore: delete unused update_suite_catalog.mjs script
There was a problem hiding this comment.
Pull request overview
This PR adds a new ClawSec skill that enhances the guarded skill installer with ClawHub reputation checking capabilities. The skill integrates VirusTotal Code Insight scores and heuristic checks (skill age, author reputation, downloads) to provide defense-in-depth security before skill installation.
Changes:
- New reputation checking skill with scoring algorithm (0-100) based on multiple security signals
- Enhanced installer wrapper that performs reputation checks before delegating to original guarded installer
- Setup script to integrate reputation checks into clawsec-advisory-guardian hook
- New exit code 43 for reputation warnings requiring
--confirm-reputationflag - Comprehensive test suite with 11 tests covering input validation and argument parsing
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
skills/clawsec-clawhub-checker/skill.json |
Skill metadata, dependencies, SBOM, and integration specifications |
skills/clawsec-clawhub-checker/scripts/check_clawhub_reputation.mjs |
Core reputation checking logic with input validation and VirusTotal integration |
skills/clawsec-clawhub-checker/scripts/enhanced_guarded_install.mjs |
Enhanced installer wrapper that adds reputation checks before skill installation |
skills/clawsec-clawhub-checker/scripts/setup_reputation_hook.mjs |
Integration script to modify clawsec-suite handler.ts with reputation checks |
skills/clawsec-clawhub-checker/hooks/clawsec-advisory-guardian/lib/reputation.mjs |
Reputation checking module for hook integration |
skills/clawsec-clawhub-checker/test/reputation_check.test.mjs |
Comprehensive test suite for reputation checking and input validation |
skills/clawsec-clawhub-checker/SKILL.md |
Main user-facing documentation with installation and usage instructions |
skills/clawsec-clawhub-checker/README.md |
Developer documentation with architecture and testing guidance |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
skills/clawsec-clawhub-checker/scripts/setup_reputation_hook.mjs
Outdated
Show resolved
Hide resolved
skills/clawsec-clawhub-checker/scripts/setup_reputation_hook.mjs
Outdated
Show resolved
Hide resolved
skills/clawsec-clawhub-checker/scripts/check_clawhub_reputation.mjs
Outdated
Show resolved
Hide resolved
| async function main() { | ||
| try { | ||
| const args = parseArgs(process.argv.slice(2)); | ||
|
|
||
| // Build args for original script (excluding reputation-specific args) | ||
| const originalArgs = []; | ||
| for (let i = 0; i < process.argv.slice(2).length; i++) { | ||
| const token = process.argv.slice(2)[i]; | ||
| if (token === "--confirm-reputation" || token === "--reputation-threshold") { | ||
| i += token === "--reputation-threshold" ? 1 : 0; | ||
| continue; | ||
| } | ||
| originalArgs.push(token); | ||
| } | ||
|
|
||
| args.originalArgs = originalArgs; | ||
|
|
There was a problem hiding this comment.
The code parses arguments twice: once via parseArgs(process.argv.slice(2)) at line 130, and again manually at lines 133-141 to filter reputation-specific arguments. This duplication is error-prone and could lead to inconsistencies if argument parsing logic changes.
Consider refactoring to build originalArgs during the initial parseArgs call, or have parseArgs return both the parsed object and the filtered array for the original script. This would ensure a single source of truth for argument parsing logic.
| async function main() { | |
| try { | |
| const args = parseArgs(process.argv.slice(2)); | |
| // Build args for original script (excluding reputation-specific args) | |
| const originalArgs = []; | |
| for (let i = 0; i < process.argv.slice(2).length; i++) { | |
| const token = process.argv.slice(2)[i]; | |
| if (token === "--confirm-reputation" || token === "--reputation-threshold") { | |
| i += token === "--reputation-threshold" ? 1 : 0; | |
| continue; | |
| } | |
| originalArgs.push(token); | |
| } | |
| args.originalArgs = originalArgs; | |
| function buildOriginalArgs(argv) { | |
| const originalArgs = []; | |
| for (let i = 0; i < argv.length; i++) { | |
| const token = argv[i]; | |
| if (token === "--confirm-reputation" || token === "--reputation-threshold") { | |
| if (token === "--reputation-threshold" && i + 1 < argv.length) { | |
| // Skip the value associated with --reputation-threshold | |
| i += 1; | |
| } | |
| continue; | |
| } | |
| originalArgs.push(token); | |
| } | |
| return originalArgs; | |
| } | |
| async function main() { | |
| try { | |
| const cliArgs = process.argv.slice(2); | |
| const args = parseArgs(cliArgs); | |
| // Build args for original script (excluding reputation-specific args) | |
| args.originalArgs = buildOriginalArgs(cliArgs); |
| { | ||
| "path": "README.md", | ||
| "required": false, | ||
| "description": "Additional documentation and development guide" |
There was a problem hiding this comment.
The test file is not included in the SBOM. For consistency with the repository's testing practices and to ensure tests are distributed with the skill, consider adding the test directory to the SBOM:
{
"path": "test/reputation_check.test.mjs",
"required": false,
"description": "Test suite for reputation checking functionality"
}| "description": "Additional documentation and development guide" | |
| "description": "Additional documentation and development guide" | |
| }, | |
| { | |
| "path": "test/reputation_check.test.mjs", | |
| "required": false, | |
| "description": "Test suite for reputation checking functionality" |
skills/clawsec-clawhub-checker/scripts/enhanced_guarded_install.mjs
Outdated
Show resolved
Hide resolved
skills/clawsec-clawhub-checker/scripts/check_clawhub_reputation.mjs
Outdated
Show resolved
Hide resolved
skills/clawsec-clawhub-checker/scripts/enhanced_guarded_install.mjs
Outdated
Show resolved
Hide resolved
…scripts and usage instructions
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 8 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // CLI interface for direct usage | ||
| if (import.meta.url === `file://${process.argv[1]}`) { | ||
| async function main() { |
There was a problem hiding this comment.
The CLI entrypoint guard import.meta.url === file://${process.argv[1]}`` won’t work when the script is invoked via a relative path (e.g., node scripts/check_clawhub_reputation.mjs ... as shown in docs), because import.meta.url is absolute while argv[1] may be relative. Use a robust comparison (e.g., resolve argv[1] and convert to a file URL) or remove the guard.
|
|
||
| const skillSlug = args[0]; | ||
| const version = args[1] || ""; | ||
| const threshold = args[2] ? parseInt(args[2], 10) : 70; |
There was a problem hiding this comment.
The CLI parsing allows an invalid threshold: parseInt can yield NaN (or an out-of-range number), which then makes result.safe = result.score >= threshold behave incorrectly (NaN makes it always false). Validate threshold (finite integer 0-100) and either default to 70 or exit 1 with a clear error.
| const threshold = args[2] ? parseInt(args[2], 10) : 70; | |
| let threshold; | |
| if (args[2] === undefined) { | |
| threshold = 70; | |
| } else { | |
| const parsedThreshold = parseInt(args[2], 10); | |
| if (!Number.isInteger(parsedThreshold) || parsedThreshold < 0 || parsedThreshold > 100) { | |
| console.error( | |
| `Invalid threshold: "${args[2]}". Threshold must be an integer between 0 and 100.` | |
| ); | |
| process.exit(1); | |
| } | |
| threshold = parsedThreshold; | |
| } |
| if (token === "--version") { | ||
| parsed.version = String(argv[i + 1] ?? "").trim(); | ||
| i += 1; | ||
| continue; |
There was a problem hiding this comment.
--version is accepted without any validation here. Because --confirm-reputation skips the call to checkClawhubReputation entirely, a user can bypass the checker’s version-format validation by passing --confirm-reputation. Consider always validating --version format in parseArgs, and only using --confirm-reputation to bypass the reputation threshold decision (not input validation).
| - `1` - Error | ||
|
|
There was a problem hiding this comment.
This section documents CLAWHUB_ALLOW_SUSPICIOUS and CLAWHUB_VIRUSTOTAL_API_KEY, but there is no implementation in this skill that reads or applies these environment variables. Either implement them or remove/clarify them so users don’t assume the behavior exists.
| - `1` - Error | |
| - `CLAWHUB_ALLOW_SUSPICIOUS` - Reserved for future use; not currently read by this skill. Intended to allow installation of suspicious skills without confirmation. | |
| - `CLAWHUB_VIRUSTOTAL_API_KEY` - Reserved for future use; not currently read by this skill. Intended for using your own VirusTotal API key for deeper scans. |
| ## Reputation Signals Checked | ||
|
|
||
| 1. **VirusTotal Code Insight** - Malicious code patterns | ||
| 2. **Skill Age** - New skills (<7 days) are riskier |
There was a problem hiding this comment.
README.md lists CLAWHUB_ALLOW_SUSPICIOUS as a supported environment variable, but none of the scripts read it. Either implement the behavior or remove it from the docs to avoid misleading configuration guidance.
| 2. **Skill Age** - New skills (<7 days) are riskier |
| handlerContent = handlerContent.slice(0, lineEndIndex + 1) + `${importLine}\n` + handlerContent.slice(lineEndIndex + 1); | ||
| handlerChanged = true; | ||
| } else { | ||
| console.log("✓ Hook handler already imports reputation module"); | ||
| } |
There was a problem hiding this comment.
The insertion logic searches for the exact line const matches = findMatches(feed, installedSkills);, but the current clawsec-suite hook handler uses const allMatches = findMatches(feed, installedSkills); and later declares matches separately. This means findMatchesLine will be -1 and the script will skip integration (only printing the warning). Update the search/insert to match the current handler.ts structure (or use AST-based editing).
| const reputationModuleDst = path.join(hookLibDir, "reputation.mjs"); | ||
|
|
||
| await fs.copyFile(reputationModuleSrc, reputationModuleDst); | ||
| console.log(`✓ Copied reputation module to ${reputationModuleDst}`); |
There was a problem hiding this comment.
setup_reputation_hook.mjs copies reputation.mjs into clawsec-suite, but the copied module executes scripts/check_clawhub_reputation.mjs relative to its own location. Since clawsec-suite does not contain that script, the hook integration will fail at runtime unless you also copy check_clawhub_reputation.mjs into the suite (or change the module to locate the checker skill directory explicitly).
| console.log(`✓ Copied reputation module to ${reputationModuleDst}`); | |
| console.log(`✓ Copied reputation module to ${reputationModuleDst}`); | |
| // Copy the ClawHub reputation checker script so reputation.mjs can resolve it relative to the suite root | |
| const reputationScriptSrc = path.join(checkerDir, "scripts", "check_clawhub_reputation.mjs"); | |
| await fs.mkdir(suiteScriptsDir, { recursive: true }); | |
| const reputationScriptDst = path.join(suiteScriptsDir, "check_clawhub_reputation.mjs"); | |
| await fs.copyFile(reputationScriptSrc, reputationScriptDst); | |
| console.log(`✓ Copied ClawHub reputation checker script to ${reputationScriptDst}`); |
| const reputationCheck = spawnSync( | ||
| "node", | ||
| [ | ||
| `${checkerDir}/scripts/check_clawhub_reputation.mjs`, | ||
| skillSlug, |
There was a problem hiding this comment.
This spawnSync call runs ${checkerDir}/scripts/check_clawhub_reputation.mjs where checkerDir is derived from the current file location. After setup_reputation_hook.mjs copies this module into clawsec-suite/hooks/..., checkerDir will resolve to the suite root (which does not contain that script), so the hook’s reputation check will fail. Make the script location resolution independent of the file’s location, or ensure the setup step copies the checker script into the suite.
…ld; update documentation
Opener Type
Summary
Adds a new skill that enhances the ClawSec guarded skill installer with ClawHub reputation checks and VirusTotal Code Insight integration. Provides a defense-in-depth layer for skill installation security.
Changes Made
clawsec-clawhub-checkerwith reputation scoring (0-100) using multiple signals--confirm-reputationRelated Issues
N/A - New feature contribution
Type of Change
Testing
formatReputationWarningoutput formatting testsRun tests:
node skills/clawsec-clawhub-checker/test/reputation_check.test.mjsChecklist
Limitation Note
This skill catches VirusTotal Code Insight flags but cannot access OpenClaw internal check results (only shown on ClawHub website, not exposed via CLI/API). Heuristic checks (skill age, author reputation, downloads) provide similar risk assessment.