Skip to content

Comments

feat: add Snyk agent-scan for dynamic skill scanning based on toxicskills#310

Open
lirantal wants to merge 6 commits intoopenclaw:mainfrom
lirantal:feat/agent-scan-skills-pre-install
Open

feat: add Snyk agent-scan for dynamic skill scanning based on toxicskills#310
lirantal wants to merge 6 commits intoopenclaw:mainfrom
lirantal:feat/agent-scan-skills-pre-install

Conversation

@lirantal
Copy link

@lirantal lirantal commented Feb 14, 2026

The Prompt Request here is essentially: use Snyk's mcp-scan Python package via uvx mcp-scan --skills <path-to-skill-directory> --json to dynamically evaluate whether a skill is malicious or adds risk, based on the JSON output show a yes/no prompt to the user asking them to confirm if they wish to proceed with installing the skill

In this PR I am following up on Jamieson O'Reilly's request to create a PR that adds a dynamic skill scanning capability that goes beyond static database checks/heuristics. Initially I would like to add this to the clawhub CLI, hence a PR to this repository, and later evaluate this security scanning powering OpenClaw directly.

Summary

Note: this change assumes uvx (the python command-line tool) is available in the user's environment. We're evaluating at Snyk exposing an API for this instead, so may consider this PR as WIP until next week and I'll post an update.

Testing

How to test this PR:

bun run test -- packages/clawdhub/src/cli/commands/skills.test.ts

Demo

agent-scan-with-openclaw-cli-integration-video.mov

Greptile Overview

Greptile Summary

This PR integrates Snyk's mcp-scan tool to add dynamic security analysis of skills during installation, complementing the existing VirusTotal static checks. The implementation adds a checkSkillSecurity function that executes uvx mcp-scan on downloaded skill files and filters violations against an ignore list.

Key Changes:

  • Added checkSkillSecurity function using execSync to run Snyk's agent-scan tool
  • Integrated scan into cmdInstall after zip extraction but before lockfile writes
  • Added user prompts when violations are detected in interactive mode
  • Cleans up downloaded files if scan fails or user rejects installation
  • Added comprehensive test coverage for various scan scenarios

Critical Issues Found:

  • Command injection vulnerability (line 76): The targetPath is interpolated directly into a shell command without sanitization, allowing potential code execution if malicious slugs are provided
  • Inconsistent --force flag behavior (lines 173-192): The error message claims --force bypasses Snyk violations in non-interactive mode, but the flag is not actually checked

Considerations:

  • The implementation assumes uvx is available in the user's environment (noted as WIP in PR description)
  • Test coverage is thorough and properly mocks the external execSync dependency

Confidence Score: 1/5

  • This PR introduces a critical command injection vulnerability that could allow arbitrary code execution
  • The command injection vulnerability on line 76 is a severe security issue that must be fixed before merging. While the feature itself is valuable for security scanning, the unsafe execSync usage creates a critical attack vector. The --force flag logic inconsistency is also a bug that needs correction. These issues significantly outweigh the security benefits this PR aims to provide.
  • packages/clawdhub/src/cli/commands/skills.ts requires immediate attention for the command injection vulnerability

Last reviewed commit: d59c6e9

(3/5) Reply to the agent's comments like "Can you suggest a fix for this @greptileai?" or ask follow-up questions!

@vercel
Copy link
Contributor

vercel bot commented Feb 14, 2026

@lirantal is attempting to deploy a commit to the Amantus Machina Team on Vercel.

A member of the Team first needs to authorize it.

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

2 files reviewed, 4 comments

Edit Code Review Agent Settings | Greptile

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant