William Valentin
153 lines
4.5 KiB
Markdown
153 lines
4.5 KiB
Markdown
# Skill/Plugin Safety Scanner — Implementation Checklist
|
|
|
|
> **Archived (2026-02-18):** Historical implementation checklist. Canonical status is tracked in `docs/plans/state.json`; unchecked boxes here are not active backlog unless explicitly re-opened.
|
|
|
|
|
|
**Date:** 2026-02-15
|
|
|
|
**Parent roadmap:** `docs/plans/2026-02-15-openclaw-gap-roadmap.md`
|
|
|
|
**Goal:** Close the gap item "Skill/plugin code safety scanner" by adding static analysis gates for skills so unsafe skill packages are rejected (or marked unavailable) before they can be injected into prompts or used in routing.
|
|
|
|
## Scope
|
|
|
|
### In scope
|
|
|
|
- Add a static scanner for skill directories.
|
|
- Run the scanner during skill load (covers daemon startup, watcher reloads, and CLI skill operations).
|
|
- Optionally run the scanner during install/upgrade (pre-copy) to avoid persisting unsafe content.
|
|
- Emit audit events (pass/fail + reason) without leaking sensitive content.
|
|
|
|
### Out of scope (for this milestone)
|
|
|
|
- Full SAST for arbitrary languages.
|
|
- Code signing / provenance chains.
|
|
- Remote registries.
|
|
|
|
## Baseline
|
|
|
|
Current skill load/install entry points:
|
|
|
|
- Load/validate: `src/skills/loader.ts` (`loadSkill()`)
|
|
- Install/upgrade: `src/skills/installer.ts`
|
|
- CLI workflows: `src/cli/skills.ts`
|
|
- Prompt injection: `src/skills/registry.ts` -> `src/daemon/services.ts` (`# Available Skills` section)
|
|
|
|
## Scanner Policy (MVP)
|
|
|
|
### File system safety
|
|
|
|
- Deny any symlinks inside a skill directory.
|
|
- Deny files above a size threshold (default: 1MB) unless allowlisted.
|
|
- Deny binary blobs (heuristic: NUL bytes or high non-text ratio) for `SKILL.md` and `manifest.json`.
|
|
|
|
### Manifest safety
|
|
|
|
- `manifest.json` must parse as JSON if present.
|
|
- If a skill is intended for routing (i.e. referenced by an intent target), require `manifest.json.permissions`.
|
|
- This aligns with the deny-by-default runtime enforcement: skills without permissions should not be routable.
|
|
|
|
### Prompt content safety (lightweight)
|
|
|
|
- Scan `SKILL.md` for obvious prompt-injection patterns:
|
|
- "ignore previous" / "system prompt" / "exfiltrate" / "send secrets" etc.
|
|
- Treat these as warnings or failures (recommend: failure for now).
|
|
|
|
## Implementation Strategy
|
|
|
|
### Scanner API
|
|
|
|
Create a small scanner module:
|
|
|
|
- `src/skills/scanner.ts`
|
|
- `scanSkillDirectory(dir): { ok: boolean; issues: SkillScanIssue[] }`
|
|
- `SkillScanIssue = { severity: 'error' | 'warn'; code: string; message: string; path?: string }`
|
|
|
|
Config knobs (optional in MVP):
|
|
|
|
- `skills.scan.enabled` (default true)
|
|
- `skills.scan.max_file_size_bytes` (default 1_000_000)
|
|
- `skills.scan.fail_on_warnings` (default false)
|
|
|
|
### Enforce during load
|
|
|
|
- In `src/skills/loader.ts` inside `loadSkill()`:
|
|
- run scanner before returning a `Skill`
|
|
- on failure:
|
|
- either return `null` (hard fail), OR
|
|
- return Skill with `available=false` and add reasons
|
|
|
|
Recommendation: mark skill unavailable (not null) so the user can see it in `flynn skills list` with reasons.
|
|
|
|
### Enforce during install (optional but recommended)
|
|
|
|
- In `src/skills/installer.ts`:
|
|
- scan `sourceDir` before copying
|
|
- fail install if scanner errors
|
|
|
|
### Audit events
|
|
|
|
- Add audit event types for skill scans:
|
|
- `skills.scan.pass`
|
|
- `skills.scan.fail`
|
|
- Ensure messages do not include raw secret content; include issue codes and counts.
|
|
|
|
## PR Breakdown
|
|
|
|
### PR 1 — Scanner module + loader integration
|
|
|
|
Checklist:
|
|
|
|
- [ ] Add `src/skills/scanner.ts` with MVP rules.
|
|
- [ ] Integrate into `src/skills/loader.ts`.
|
|
- [ ] Update `src/skills/loader.test.ts` with fixtures:
|
|
- symlink skill rejected
|
|
- oversized file rejected
|
|
- injection marker in SKILL.md rejected
|
|
|
|
Acceptance:
|
|
|
|
- `flynn doctor` / skill load does not crash on bad skills.
|
|
- Bad skills become unavailable with clear reasons.
|
|
|
|
Tests:
|
|
|
|
- `pnpm test:run src/skills/loader.test.ts`
|
|
|
|
---
|
|
|
|
### PR 2 — Installer integration
|
|
|
|
Checklist:
|
|
|
|
- [ ] Add scan preflight to `src/skills/installer.ts install()`.
|
|
- [ ] Add tests for install failure on scan errors.
|
|
|
|
Tests:
|
|
|
|
- `pnpm test:run src/skills/installer.test.ts` (add if missing)
|
|
|
|
---
|
|
|
|
### PR 3 — Audit events
|
|
|
|
Checklist:
|
|
|
|
- [ ] Extend `src/audit/types.ts` to include skill scan events.
|
|
- [ ] Extend `src/audit/logger.ts` helpers.
|
|
- [ ] Emit events from loader/install paths.
|
|
- [ ] Add targeted tests validating event shape (if audit logger has test coverage).
|
|
|
|
Acceptance:
|
|
|
|
- Audit logs show scan pass/fail with counts and stable issue codes.
|
|
|
|
## Final Checks
|
|
|
|
- [ ] `pnpm typecheck`
|
|
- [ ] `pnpm test:run`
|
|
- [ ] Update `docs/plans/state.json` entry to `completed` once implemented (include test status).
|
|
|
|
|
|
|