From 15e3f79dde4c810ef3b368e8ada2249f353a05e1 Mon Sep 17 00:00:00 2001 From: OdevMatheus Date: Mon, 9 Nov 2026 11:41:25 -0300 Subject: [PATCH] docs(contract): codify inv-skill-06 security invariant and mandatory pr security review checklist (closes #192) --- .github/PULL_REQUEST_TEMPLATE.md | 5 +++++ CHANGELOG.md | 1 + CONTRIBUTING.md | 7 +++++-- SKILL_CONTRACT.md | 7 ++++++- 4 files changed, 17 insertions(+), 3 deletions(-) diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index e30c8bd..6e2946d 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -19,5 +19,10 @@ Closes # - [ ] `SKILL.md` description (`40–110` words, `<= 160` tokens) starts with `"Use when..."` and body is `<= 250` lines (`INV-SKILL-01`). - [ ] `SUBAGENT.md` forbids wildcard `["*"]` tools and sets `max-output-tokens <= 400` (`INV-SKILL-04`). - [ ] Verified dry-run transpilation across all 28 targets (`node dist/cli.js add --targets all --dry-run`). +- [ ] **Security & Supply-Chain Gate (`INV-SKILL-06` & `CODEOWNERS`):** + - [ ] **Zero Network Exfiltration:** Bundled scripts do not import outbound network clients (`urllib.request`, `http.client`, `socket`, `requests`, `httpx`, `aiohttp`, `paramiko`) or pipe remote downloads into shells (`curl | sh`). + - [ ] **Zero Dynamic Exec & Unsafe Shell:** Bundled scripts do not call `eval()`, `exec()`, `__import__()`, `pickle`, `marshal`, `ctypes`, `os.system()`, `os.popen()`, or `subprocess(..., shell=True)`. + - [ ] **Zero Trojan-Source & Opaque Payloads:** No hidden bidirectional Unicode (`U+202A..U+202E`, `U+2066..U+2069`), LLM invisible Unicode Tags (`U+E0000..U+E007F`), symbolic links, or compiled binaries/archives (`.exe`, `.dll`, `.so`, `.pyc`, `.zip`, `.tar.gz`). + - [ ] **Mandatory Maintainer Code Review:** Reviewed line-by-line by `@OdevMatheus` via `.github/CODEOWNERS`. - [ ] **Verification Gates:** `npm run lint`, `npm run typecheck`, `npm test`, and `npm run build` pass locally. - [ ] **Sanitization:** Zero local machine paths (`C:\Users\...`, `/Users/...`), OS usernames, or leaked AI API keys (`sk-proj-...`, `sk-ant-...`, `AIzaSy...`). diff --git a/CHANGELOG.md b/CHANGELOG.md index 05427b0..25a88ac 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] ### Added +- **`INV-SKILL-06` Security Invariant & Mandatory PR Security Review Checklist (`#192`):** Codified `INV-SKILL-06` (*Zero-Network, Trojan-Free & Boundary-Safe Security Contract*) in [`SKILL_CONTRACT.md`](./SKILL_CONTRACT.md) and added a mandatory supply-chain security review checklist in [`.github/PULL_REQUEST_TEMPLATE.md`](./.github/PULL_REQUEST_TEMPLATE.md) and [`CONTRIBUTING.md`](./CONTRIBUTING.md). - **Anti-Trojan Unicode, Symlink, Binary & Path Containment Guards (`#191`):** Extended [`src/auditor/security.ts`](./src/auditor/security.ts), [`src/auditor/parser.ts`](./src/auditor/parser.ts), and [`src/sync/adapters.ts`](./src/sync/adapters.ts) (`resolveSafeRepoTargetPath`) to block Trojan-Source (CVE-2021-42574) bidirectional control characters, LLM invisible Unicode Tag smuggling (`U+E0000..U+E007F`), zero-width script identifier spoofing, symbolic links, opaque compiled binary/archive payloads (`.exe`, `.dll`, `.so`, `.dylib`, `.bin`, `.wasm`, `.pyc`, `.zip`, `.tar.gz`), and repository boundary path traversal. - **Catalog Script AST & Lexical Security Scanner (`#190`):** Added [`src/auditor/security.ts`](./src/auditor/security.ts) (`auditSkillSecurity`, `auditCatalogSecurity`) and a Python `ast` security gate in [`.github/workflows/ai-ci.yml`](./.github/workflows/ai-ci.yml) to block outbound network modules (`urllib.request`, `http.client`, `socket`, `requests`, `curl | sh`), dynamic execution/deserialization (`eval`, `exec`, `__import__`, `pickle`, `marshal`, `ctypes`), and unsafe shell execution (`os.system`, `os.popen`, `shell=True`) in catalog Skill scripts with zero false positives on legitimate large skills. - **Global User Home Directory Scope (`--global` / `-g`) (`#176`):** Added `isGlobalFlag()`, `resolveGlobalHomeDir()`, and `resolveTargetRepoPath()` in [`src/cli.ts`](./src/cli.ts), updated [`src/ui/help.ts`](./src/ui/help.ts), and surfaced `[Global: ~]` scope indicators in [`src/sync/adapters.ts`](./src/sync/adapters.ts) so `npx skillops-cli add -g` and `npx skillops-cli remove -g` install and remove Skills and Subagents directly in `os.homedir()` (`~/.claude/`, `~/.cursor/`, `~/.gemini/`, etc.). diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 7696f94..1377c45 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -37,7 +37,7 @@ npm test ## 3. Track A: Contributing a New Skill or Subagent (No Issue Needed) -Every item in [`catalog/`](./catalog) must comply with [`SKILL_CONTRACT.md`](./SKILL_CONTRACT.md) (`INV-SKILL-01` through `INV-SKILL-04`): +Every item in [`catalog/`](./catalog) must comply with [`SKILL_CONTRACT.md`](./SKILL_CONTRACT.md) (`INV-SKILL-01` through `INV-SKILL-06`): ### 3.1 Adding a Hybrid Skill (`SKILL.md`) 1. Create a folder under `catalog///` (e.g., `ai-engineering`, `python-backend`, `arch-quality`): @@ -48,9 +48,10 @@ Every item in [`catalog/`](./catalog) must comply with [`SKILL_CONTRACT.md`](./S │ └── .py └── references/ # Optional deep reference docs loaded on demand ``` -2. **Contract checklist for `SKILL.md`:** +2. **Contract checklist for `SKILL.md` & `scripts/`:** - `description` must start with `"Use when..."` and contain **`40–110` words** (`<= 160` tokens), including explicit `Do not trigger for...` negative triggers. - Markdown body must stay **`<= 250` lines** (`<= 1,800` tokens). + - **Security Invariant (`INV-SKILL-06`):** Scripts must be 100% offline/stdlib-only (`auditCatalogSecurity` in [`src/auditor/security.ts`](./src/auditor/security.ts)). Outbound network clients (`urllib.request`, `http.client`, `socket`, `requests`, `curl | sh`), dynamic execution (`eval`, `exec`, `pickle`, `ctypes`), unsafe shell execution (`os.system`, `shell=True`), Trojan-Source hidden Unicode, symbolic links, and binary/archive files (`.exe`, `.dll`, `.so`, `.pyc`, `.zip`, `.tar.gz`) are automatically blocked by CI. ### 3.2 Adding a Context-Isolated Subagent (`SUBAGENT.md`) 1. Create a folder under `catalog/subagents//`: @@ -108,3 +109,5 @@ All Pull Requests are automatically checked by [`.github/workflows/pr-governance - **No Absolute Machine Paths:** Never commit `C:\Users\...`, `/Users/...`, or `/home/...` in any tracked file. Use relative paths (`./...`). - **No Environment or Corporate Metadata:** Never expose OS usernames, local hostnames, or private domains. - **No Secrets or API Keys:** Never commit `.env` files or AI provider keys (`sk-proj-...`, `sk-ant-...`, `AIzaSy...`, `hf_...`). +4. **Mandatory `CODEOWNERS` Security Review & Automated Gates:** + - Every Pull Request targeting `main`—especially additions or edits under `catalog/`—must pass all automated CI gates (`pr-governance.yml`, `ai-ci.yml` AST/Trojan-Source security scanner, and `codeql.yml` SAST for Python & TypeScript) **and** receive an explicit manual line-by-line review and approval from `@OdevMatheus` via [`.github/CODEOWNERS`](./.github/CODEOWNERS). Stale approvals are automatically dismissed whenever new commits are pushed. diff --git a/SKILL_CONTRACT.md b/SKILL_CONTRACT.md index a3e5ea9..daf73b2 100644 --- a/SKILL_CONTRACT.md +++ b/SKILL_CONTRACT.md @@ -96,7 +96,7 @@ user-invocable: true Every Python tool bundled inside `catalog///scripts/*.py` must adhere to a uniform Agent-Computer Interface (`ScriptOutputSchema` in [`src/models.ts`](./src/models.ts)): -1. **Zero Heavy Third-Party Dependencies:** Use Python 3.12+ standard library (`ast`, `pathlib`, `json`, `argparse`, `re`, `sqlite3`, `tokenize`) so scripts run instantaneously in any environment. +1. **Zero Heavy Third-Party Dependencies & Zero Outbound Network Exfiltration (`INV-SKILL-06`):** Use Python 3.12+ standard library (`ast`, `pathlib`, `json`, `argparse`, `re`, `sqlite3`, `tokenize`, `urllib.parse`, `http.server`) so scripts run deterministically offline in any environment. Outbound network/exfiltration modules (`urllib.request`, `http.client`, `socket`, `requests`, `httpx`, `aiohttp`, `paramiko`, `websocket`), dynamic code execution/deserialization (`eval`, `exec`, `__import__`, `pickle`, `marshal`, `ctypes`), and unsafe shell execution (`os.system`, `os.popen`, `pty`, `subprocess` with `shell=True`) are blocked by [`src/auditor/security.ts`](./src/auditor/security.ts). 2. **Mandatory `--json` Flag:** Every script must support `--json` and emit a deterministic JSON payload to `stdout`: ```json @@ -136,6 +136,11 @@ The **SkillOps** TypeScript NPX engine enforces the following **EARS (Easy Appro - **`INV-SKILL-03` (Event-Driven — Context-Aware Bundle Cap):** **When** `npx skillops route` or `npx skillops sync` compiles a skill bundle for a target repository, the system **shall** select at most `12` collision-free skills (`<= 1,500` total Level 1 tokens) matching the repository's `RepoProfile`. - **`INV-SKILL-04` (Ubiquitous — Context-Isolated Subagent Contract):** Every subagent governed by SkillOps **shall** declare an explicit least-privilege `tools` allowlist (never `["*"]`), keep its system prompt `<= 1,000 tokens`, and cap `max_output_tokens <= 400`. - **`INV-SKILL-05` (Unwanted Behavior — Harmful or Redundant Skill Detection):** **If** a skill's `with_skill` pass rate in `npx skillops eval` fails to improve over `without_skill` by at least `+15%` (or inflates token usage by `> 25%` without improving pass rate), **then** the evaluation engine **shall** mark the skill verdict as `REJECTED_REGRESSION`. +- **`INV-SKILL-06` (Ubiquitous — Zero-Network, Trojan-Free & Boundary-Safe Security Contract):** Every Skill and Subagent in `catalog/` **shall** pass `auditCatalogSecurity` ([`src/auditor/security.ts`](./src/auditor/security.ts)) and mandatory `CODEOWNERS` human review: + 1. **No Outbound Network / Exfiltration Primitives:** Scripts must not import outbound network clients (`urllib.request`, `urllib2`, `urllib3`, `http.client`, `httplib`, `socket`, `socketserver`, `requests`, `httpx`, `aiohttp`, `ftplib`, `smtplib`, `telnetlib`, `xmlrpc.client`, `paramiko`, `websocket`, `websockets`) or pipe remote downloads into shells (`curl | sh`, `wget | bash`). Offline URL parsers (`urllib.parse`) and local preview servers (`http.server`, `webbrowser`) are permitted. + 2. **No Dynamic Execution or Unsafe Deserialization:** Scripts must not invoke `eval()`, `exec()`, `__import__()`, `Function()`, `pickle`, `marshal`, `shelve`, `ctypes`, or `cffi`. + 3. **No Unsafe Shell Invocation:** Scripts must not invoke `os.system()`, `os.popen()`, `pty`, reverse-shell primitives (`/dev/tcp/`, `nc -e`), or `subprocess` with `shell=True` (argument-array `subprocess.run([...])` with `shell=False` is permitted). + 4. **No Trojan-Source Unicode, Symlinks, or Opaque Binaries:** Catalog directories must not contain Trojan-Source (CVE-2021-42574) bidirectional control characters (`U+202A..U+202E`, `U+2066..U+2069`, `U+061C`), LLM invisible Unicode Tag smuggling (`U+E0000..U+E007F`), zero-width script identifier spoofing, symbolic links, or opaque compiled binary/archive files (`.exe`, `.dll`, `.so`, `.dylib`, `.bin`, `.wasm`, `.pyc`, `.zip`, `.tar.gz`). Any planned artifact write or removal must resolve strictly inside the target repository boundary (`resolveSafeRepoTargetPath`). ### Standard CLI Exit Codes (`ExitCode` in `src/models.ts`) - `0` (`SUCCESS`): All checks, routing, sync, or evaluations passed.