diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index b3af9b3..31af56d 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -24,6 +24,8 @@ jobs: - uses: actions/checkout@v4 - name: Validate plugin metadata run: python tests/test-plugin-metadata.py + - name: Scan the shipped skill surface + run: python tests/test-skill-safety.py - name: Test hooks on Linux if: runner.os == 'Linux' run: bash tests/test-hooks.sh diff --git a/.gitignore b/.gitignore index 03bdc94..c5a1e06 100644 --- a/.gitignore +++ b/.gitignore @@ -7,5 +7,6 @@ # Local working notes (kept out of the public repo) /CLAUDE.md +/AGENTS.md /discussion/ /experiments/ diff --git a/README.md b/README.md index 84556e8..48ba13a 100644 --- a/README.md +++ b/README.md @@ -85,7 +85,14 @@ second installs the plugin. Open a new session after installation. To test a local checkout, replace the repository name in the first command with a local path. -### Skills only, without hooks (Codex) +### Skills only, without hooks (Codex and other hosts) + +The eight skills are plain `SKILL.md` files that conform to the +[agentskills.io](https://agentskills.io) spec — conformant `name`, a +description within the 1024-character limit, and a `license` on every file so a +copied skill still carries its terms. A CI test enforces those limits, because +Claude Code is lenient about all three and a violation would fail silently +wherever else the directory is copied. Codex supports `SKILL.md` directly: @@ -130,7 +137,13 @@ and execution. - **SessionStart discipline:** injects a compact, host-neutral primer that requires matched skills to be formally invoked, preserves instruction precedence, and tells the agent to answer in the user's language while - matching the repository language for files and comments. + matching the repository language for files and comments. The hook emits the + envelope each host documents — `hookSpecificOutput.additionalContext` for + Claude Code and Codex, `additional_context` for Cursor, top-level + `additionalContext` for Copilot — and appends only the host's own invocation + note. Tests cover all four envelope shapes; Claude Code and Codex are the two + used daily, so treat Cursor and Copilot as conformant-but-unproven until you + have run them yourself. - **Passive local usage counting:** appends session activations to `~/.coding-discipline/usage.jsonl`. Claude Code also exposes per-skill invocations. Nothing is sent over the network. @@ -183,6 +196,30 @@ Report false triggers and missed triggers with the repository's routing feedback issue template. Regression cases grow from real failures rather than a synthetic release suite. +## Security + +A skill is executable context: whatever it says enters an agent that already +holds your credentials and your file system. Public skill catalogues scanned in +2026 turned out to carry prompt injection and credential exfiltration in a +large minority of entries, so this plugin treats "our skills are clean" as a +claim that has to be checked rather than promised. + +`tests/test-skill-safety.py` scans every shipped file for the known attack +classes — network egress, credential and private-key paths, piping into a +shell, decoded or indirect execution, and invisible or bidirectional Unicode +that hides instructions from a human reading the diff. Each rule carries a +sample it must still match, so a pattern that quietly stops working fails the +build instead of reporting success forever. CI runs it on Ubuntu and Windows. + +The plugin makes no network calls and sends no telemetry. Its only write +outside your repository is the local usage log at +`~/.coding-discipline/usage.jsonl`, and the test fails if that sink ever moves +without this claim being re-checked. + +```bash +python tests/test-skill-safety.py +``` + ## Dependencies - **bash** — included on macOS and Linux. On Windows, install @@ -194,6 +231,7 @@ synthetic release suite. ```bash python tests/test-plugin-metadata.py +python tests/test-skill-safety.py bash tests/test-hooks.sh ``` diff --git a/README.zh-CN.md b/README.zh-CN.md index 595a87f..d45dc12 100644 --- a/README.zh-CN.md +++ b/README.zh-CN.md @@ -73,7 +73,13 @@ codex plugin marketplace add . 第一条命令把本仓库添加为自托管插件源，第二条安装插件。安装后新开一个 session。本地测试时，把第一条中的仓库名换成本地路径。 -### 只安装 skills，不安装 hooks（Codex） +### 只安装 skills，不安装 hooks（Codex 及其他宿主） + +8 个 skill 都是符合 [agentskills.io](https://agentskills.io) 规范的普通 +`SKILL.md`：合规的 `name`、不超过 1024 字符的 description，以及每个文件 +自带的 `license`——这样单独拷走的 skill 仍然带着授权条款。CI 会强制这三 +条限制，因为 Claude Code 对它们都很宽松，一旦违反，在这里照常工作，拷到 +别的宿主却会静默失败。 Codex 原生支持 `SKILL.md`： @@ -116,7 +122,12 @@ Copy-Item -Recurse "plugins\coding-discipline\skills\*" "$HOME\.agents\skills\" - **SessionStart 纪律总纲**：注入一段精简、平台中立的纪律，要求正式调用 命中的 skill，维护指令优先级，并要求 agent 用用户的语言回答、用仓库 - 已有语言编写文件和注释。 + 已有语言编写文件和注释。Hook 会按各宿主自己的文档输出对应信封—— + Claude Code 和 Codex 用 `hookSpecificOutput.additionalContext`，Cursor 用 + `additional_context`，Copilot 用顶层 `additionalContext`——并只追加该宿主 + 自己的调用说明。四种信封形状都有测试覆盖；日常在用的是 Claude Code 和 + Codex，所以在你自己跑通之前，请把 Cursor 和 Copilot 当作"符合规范但未 + 实测"。 - **被动本地用量计数**：把 session 激活记录追加到 `~/.coding-discipline/usage.jsonl`。Claude Code 还可以记录每个 skill 的调用。数据不会上传。 @@ -163,6 +174,27 @@ Skill 是提示词层面的纪律，不是确定性规则引擎。它能降低 误触发和漏触发请通过仓库的 routing feedback issue 模板反馈。回归案例只 从真实失败中生长，不维护人工场景发版套件。 +## 安全 + +Skill 本身就是可执行的上下文：它写什么，都会进入一个已经握着你的凭据和 +文件系统的 agent。2026 年对公开 skill 目录的扫描发现，其中相当一部分带有 +提示词注入和凭据外传，所以本插件把"我们的 skill 是干净的"当成一个需要被 +检验的声明，而不是一句承诺。 + +`tests/test-skill-safety.py` 会扫描所有随插件分发的文件，检查已知的攻击 +类型——网络外传、凭据与私钥路径、管道进 shell、解码或间接执行，以及会对 +人眼隐藏指令的不可见/双向控制字符。每条规则都带一个必须命中的样本，所以 +一条悄悄失效的规则会让构建失败，而不是永远报告通过。CI 在 Ubuntu 和 +Windows 上都会执行。 + +本插件不发起任何网络请求，也不上报任何遥测。它在你的仓库之外唯一的写入 +是本地用量日志 `~/.coding-discipline/usage.jsonl`；如果这个位置发生变化， +测试会失败，提醒重新核对上面这条声明。 + +```bash +python tests/test-skill-safety.py +``` + ## 依赖 - **bash**：macOS 和 Linux 自带；Windows 安装 @@ -174,6 +206,7 @@ Skill 是提示词层面的纪律，不是确定性规则引擎。它能降低 ```bash python tests/test-plugin-metadata.py +python tests/test-skill-safety.py bash tests/test-hooks.sh ``` diff --git a/plugins/coding-discipline/.claude-plugin/plugin.json b/plugins/coding-discipline/.claude-plugin/plugin.json index 9085e89..1ca09b2 100644 --- a/plugins/coding-discipline/.claude-plugin/plugin.json +++ b/plugins/coding-discipline/.claude-plugin/plugin.json @@ -1,7 +1,7 @@ { "name": "coding-discipline", "description": "Workflow-agnostic guardrails for AI coding agents: spec-sync preserves confirmed intent across sessions; brainstorming, TDD, root-cause debugging, prioritized review, evidence-before-done, disciplined git, and context hygiene trigger only when their risk conditions match. Includes a SessionStart discipline primer, optional guide-doc seeding, and passive local usage counting.", - "version": "0.8.1", + "version": "0.9.0", "author": { "name": "chipfighter" }, diff --git a/plugins/coding-discipline/.codex-plugin/plugin.json b/plugins/coding-discipline/.codex-plugin/plugin.json index cb17333..5b80e8e 100644 --- a/plugins/coding-discipline/.codex-plugin/plugin.json +++ b/plugins/coding-discipline/.codex-plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "coding-discipline", - "version": "0.8.1", + "version": "0.9.0", "description": "Workflow-agnostic guardrails for AI coding agents: spec sync preserves confirmed intent across sessions, while risk-triggered skills enforce design alignment, TDD, root-cause debugging, prioritized review, evidence-before-done, disciplined git, and context hygiene.", "author": { "name": "chipfighter" diff --git a/plugins/coding-discipline/skills/brainstorming/SKILL.md b/plugins/coding-discipline/skills/brainstorming/SKILL.md index 448e1ef..e22f239 100644 --- a/plugins/coding-discipline/skills/brainstorming/SKILL.md +++ b/plugins/coding-discipline/skills/brainstorming/SKILL.md @@ -1,6 +1,7 @@ --- name: brainstorming description: Use before coding when requirements have multiple reasonable interpretations, approaches require tradeoffs, or mistakes would be costly (permissions/payments/data migration/public APIs/data structures/hard-to-revert changes). Clarify requirements and design one question at a time, then get approval before implementation. Do not trigger for a focused small change with a clear goal and approach, or for mechanical edits (copy/constants/config values). +license: MIT --- **Hard rule: write no code until the design is approved.** Once this skill triggers, follow it through—the design may be only a few sentences, but it must be explained and approved first. Deciding midway that it is "actually simple" is not a reason to exit: "simple" often means unstated assumptions. diff --git a/plugins/coding-discipline/skills/code-review/SKILL.md b/plugins/coding-discipline/skills/code-review/SKILL.md index 5bc07aa..24b9899 100644 --- a/plugins/coding-discipline/skills/code-review/SKILL.md +++ b/plugins/coding-discipline/skills/code-review/SKILL.md @@ -1,6 +1,7 @@ --- name: code-review description: Use when changes affect interactions across multiple modules, involve high-risk areas (authorization/authentication, payments/funds, data deletion/migration, public APIs/cross-service interfaces/security boundaries), or the user requests a review. Review in this order—correctness → requirements → security → simplicity → style. Verify feedback before acting; do not comply blindly or agree performatively. Unless the user asks, documentation-only changes, explicit config values, and mechanical renames do not trigger merely because of a PR or merge. +license: MIT --- ## Performing a review (review it yourself; delegate when worthwhile and supported) diff --git a/plugins/coding-discipline/skills/context-hygiene/SKILL.md b/plugins/coding-discipline/skills/context-hygiene/SKILL.md index 19e8e12..eb9f42c 100644 --- a/plugins/coding-discipline/skills/context-hygiene/SKILL.md +++ b/plugins/coding-discipline/skills/context-hygiene/SKILL.md @@ -1,6 +1,7 @@ --- name: context-hygiene description: Use when starting a project, reading project documentation or history, or resolving conflicts between documents. For current state, read only the latest documents designated by the project; do not proactively read archives; do not duplicate the spec; use long-term memory only for environment, people, and preferences not recorded in the repository. +license: MIT --- Old documents can trace history, but they are not current facts. Models easily follow the most extensively documented old account instead of finding the latest, correct one. If the project guide document (Codex's `AGENTS.md` / Claude Code's `CLAUDE.md`) designates a current-state document and archive location, follow it. diff --git a/plugins/coding-discipline/skills/git-flow/SKILL.md b/plugins/coding-discipline/skills/git-flow/SKILL.md index 48797ab..66a8b34 100644 --- a/plugins/coding-discipline/skills/git-flow/SKILL.md +++ b/plugins/coding-discipline/skills/git-flow/SKILL.md @@ -1,6 +1,7 @@ --- name: git-flow description: Use when creating a branch, using worktrees for parallel work, committing, or wrapping up. Covers branch naming, conventional commits, short-lived branches, worktree isolation, and final merge/cleanup. +license: MIT --- > Branch naming rules, protected branches, whether to merge directly or through a PR / MR, whether merging triggers CI, and **how to choose versions / who creates tags** are project-specific rules governed by this repository's `AGENTS.md` / `CLAUDE.md`. This skill provides only general discipline. diff --git a/plugins/coding-discipline/skills/spec-sync/SKILL.md b/plugins/coding-discipline/skills/spec-sync/SKILL.md index 46164ea..3021c3c 100644 --- a/plugins/coding-discipline/skills/spec-sync/SKILL.md +++ b/plugins/coding-discipline/skills/spec-sync/SKILL.md @@ -1,6 +1,7 @@ --- name: spec-sync description: Use in cross-session work when user-confirmed outcomes, non-goals, hard constraints, or acceptance criteria have not been written back to the current source of truth (spec), or reality conflicts with it enough that following it would be wrong; also use during wrap-up or handoff when a source of truth should exist but does not. Edit only the affected original wording; do not keep separate old and new versions. Do not trigger for work that can be completed within one session, local implementation details, verification against existing acceptance criteria, locating the source of truth, or ordinary documentation edits. +license: MIT --- Hard rule: **A change confirmed in conversation must not remain only in the conversation.** If user-confirmed intent that affects future collaboration or acceptance is not written back to the source of truth in this session, the next session cannot even see that the user changed their mind—intent that was never persisted cannot be recovered. diff --git a/plugins/coding-discipline/skills/systematic-debugging/SKILL.md b/plugins/coding-discipline/skills/systematic-debugging/SKILL.md index fdc6934..3cf279b 100644 --- a/plugins/coding-discipline/skills/systematic-debugging/SKILL.md +++ b/plugins/coding-discipline/skills/systematic-debugging/SKILL.md @@ -1,6 +1,7 @@ --- name: systematic-debugging description: Use before proposing a permanent fix for bugs, test failures, or unexpected behavior with an unknown root cause—mandatory when symptoms are far from the cause, a prior fix failed, or the issue crosses components. Reproduce, trace backward to the root cause, fix once at the source, and add a regression test. Do not trigger for obvious errors whose message identifies the cause (syntax/typo/missing import); fix those directly. +license: MIT --- Hard rule: **do not package a guess as a permanent fix before locating the root cause.** If the user explicitly needs emergency containment, you may apply a reversible temporary mitigation first, but you must state its risks and that the root cause is not fixed, then continue investigating. diff --git a/plugins/coding-discipline/skills/tdd/SKILL.md b/plugins/coding-discipline/skills/tdd/SKILL.md index c70aa23..29b2f7f 100644 --- a/plugins/coding-discipline/skills/tdd/SKILL.md +++ b/plugins/coding-discipline/skills/tdd/SKILL.md @@ -1,6 +1,7 @@ --- name: tdd description: Use before implementation when behavior can be verified by automated tests and regression coverage has clear value. For bugs with an unknown root cause, use systematic-debugging first, then return for red-green. Follow red-green-refactor—write a failing test first, add only enough code to pass it, then refactor. Do not trigger for documentation, configuration, copy-only, or styling changes with no testable behavior. +license: MIT --- Hard rule: **write no implementation code without a test that failed first.** diff --git a/plugins/coding-discipline/skills/verify-before-done/SKILL.md b/plugins/coding-discipline/skills/verify-before-done/SKILL.md index 74f4b58..c3767b9 100644 --- a/plugins/coding-discipline/skills/verify-before-done/SKILL.md +++ b/plugins/coding-discipline/skills/verify-before-done/SKILL.md @@ -1,6 +1,7 @@ --- name: verify-before-done description: Use before claiming "done", "fixed", "tested", or "it runs", and before committing or opening a PR or MR—no task is exempt. Accept only evidence gathered after the last relevant change and just sufficient to prove the claim; do not over-verify small changes. +license: MIT --- Hard rule: **Only evidence gathered after the last relevant change to the code, files, or environment, and directly sufficient to prove the claim, counts.** diff --git a/tests/test-hooks.sh b/tests/test-hooks.sh index 8473ef9..ea146b6 100755 --- a/tests/test-hooks.sh +++ b/tests/test-hooks.sh @@ -7,9 +7,11 @@ HOOK="${PLUGIN_ROOT}/hooks/session-start-skills" TMP_ROOT="$(mktemp -d "${TMPDIR:-/tmp}/coding-discipline-tests.XXXXXX")" # A candidate interpreter must actually run. On Windows, the Microsoft Store -# python3.exe stub passes command -v but exits 49 when invoked. +# python3.exe stub passes command -v but exits 49 when invoked; `py` is the +# launcher a real Windows install provides, so it has to be a candidate too or +# this suite is unrunnable on the Git Bash setup the README tells people to use. PYTHON= -for cand in python3 python; do +for cand in python3 python py; do if "$cand" -c '' >/dev/null 2>&1; then PYTHON="$cand"; break; fi done if [ -z "$PYTHON" ]; then @@ -123,4 +125,53 @@ make_repo "$repo_opt_out" [ ! -e "${repo_opt_out}/AGENTS.md" ] || fail 'guide opt-out was ignored' [ ! -e "${TMP_ROOT}/disabled.jsonl" ] || fail 'usage opt-out was ignored' +# Cursor and Copilot read the primer from different envelope fields than Claude +# Code and Codex. The hook already handles all four; these cases keep a refactor +# from silently dropping the two that no maintainer runs day to day. +assert_no_guide() { + local repo="$1" host="$2" + [ ! -e "${repo}/AGENTS.md" ] || fail "${host} seeded AGENTS.md" + [ ! -e "${repo}/CLAUDE.md" ] || fail "${host} seeded CLAUDE.md" +} + +repo_cursor="${TMP_ROOT}/repo-cursor" +make_repo "$repo_cursor" +output="$({ + cd "$repo_cursor" + env -u CODEX_HOME -u PLUGIN_ROOT -u COPILOT_CLI \ + CURSOR_PLUGIN_ROOT="$PLUGIN_ROOT" \ + CD_USAGE_LOG="${TMP_ROOT}/usage-cursor.jsonl" \ + "$HOOK" cursor +})" +printf '%s\n' "$output" | "$PYTHON" -c 'import json,sys; p=json.load(sys.stdin); assert set(p) == {"additional_context"}, p; assert p["additional_context"].strip()' \ + || fail 'Cursor did not receive additional_context' +grep -q '"platform":"cursor"' "${TMP_ROOT}/usage-cursor.jsonl" \ + || fail 'Cursor usage record has the wrong platform' +assert_no_guide "$repo_cursor" Cursor + +repo_copilot="${TMP_ROOT}/repo-copilot" +make_repo "$repo_copilot" +output="$({ + cd "$repo_copilot" + env -u CODEX_HOME -u PLUGIN_ROOT -u CURSOR_PLUGIN_ROOT \ + COPILOT_CLI=1 \ + CD_USAGE_LOG="${TMP_ROOT}/usage-copilot.jsonl" \ + "$HOOK" copilot +})" +printf '%s\n' "$output" | "$PYTHON" -c 'import json,sys; p=json.load(sys.stdin); assert set(p) == {"additionalContext"}, p; assert p["additionalContext"].strip()' \ + || fail 'Copilot did not receive additionalContext' +grep -q '"platform":"copilot"' "${TMP_ROOT}/usage-copilot.jsonl" \ + || fail 'Copilot usage record has the wrong platform' +assert_no_guide "$repo_copilot" Copilot + +# Hosts outside the two first-class ones must get the neutral invocation note +# rather than Claude Code's Skill-tool wording. +case "$output" in + *'Load skills through the skill mechanism of this host'*) ;; + *) fail 'a non-primary host did not receive the neutral invocation note' ;; +esac +case "$output" in + *'Skill tool'*) fail 'a non-primary host received Claude Code wording' ;; +esac + printf 'hook behavior tests passed\n' diff --git a/tests/test-plugin-metadata.py b/tests/test-plugin-metadata.py index 47db38c..8434ba1 100644 --- a/tests/test-plugin-metadata.py +++ b/tests/test-plugin-metadata.py @@ -11,6 +11,12 @@ PLUGIN = ROOT / "plugins" / "coding-discipline" SEMVER = re.compile(r"^\d+\.\d+\.\d+(?:-[0-9A-Za-z.-]+)?(?:\+[0-9A-Za-z.-]+)?$") HAN = re.compile(r"[\u3400-\u4dbf\u4e00-\u9fff]") +# agentskills.io constrains the two fields every host reads. Claude Code is +# lenient about both, so a violation keeps working here and fails silently +# wherever else the skills directory is copied. +SKILL_NAME = re.compile(r"[a-z0-9]+(?:-[a-z0-9]+)*") +SKILL_NAME_MAX = 64 +SKILL_DESCRIPTION_MAX = 1024 # Re-frozen after the 2026-07 dedup trim (4944 at v0.8.0). Any new primer or # description text must be offset elsewhere (zero-sum). FIXED_CONTEXT_BUDGET = 4580 @@ -86,6 +92,15 @@ def load_json(path: Path) -> dict: description = re.search(r"(?m)^description:\s*(.+)\s*$", frontmatter) assert name and name.group(1) == path.parent.name, f"{path} has the wrong name" assert description, f"{path} has no description" + assert SKILL_NAME.fullmatch(name.group(1)), f"{path} name is not portable" + assert len(name.group(1)) <= SKILL_NAME_MAX, f"{path} name is too long" + assert len(description.group(1)) <= SKILL_DESCRIPTION_MAX, ( + f"{path} description exceeds the {SKILL_DESCRIPTION_MAX}-char spec limit" + ) + # The skills directory is documented as copyable on its own, so each file + # has to carry its own licence rather than rely on the repository root. + licence = re.search(r"(?m)^license:\s*(.+?)\s*$", frontmatter) + assert licence and licence.group(1) == "MIT", f"{path} has no license: MIT" description_chars += len(description.group(1)) primer_text = (PLUGIN / "hooks" / "skill-discipline.md").read_text(encoding="utf-8") diff --git a/tests/test-skill-safety.py b/tests/test-skill-safety.py new file mode 100644 index 0000000..09a6e45 --- /dev/null +++ b/tests/test-skill-safety.py @@ -0,0 +1,108 @@ +#!/usr/bin/env python3 +"""Prove the shipped plugin surface is free of the known SKILL.md attack classes. + +Published skills are executable context: a host loads them into an agent that +already holds the user's credentials and file system. Industry scans of public +skill catalogues in 2026 found prompt injection and credential exfiltration in +a large minority of them, so "our skills are clean" has to be a checked claim +rather than a promise in the README. + +Scope is everything under plugins/coding-discipline/ that ships to users. The +tests/ directory is excluded on purpose: it names these patterns in order to +search for them. +""" +from __future__ import annotations + +import re +import subprocess +from pathlib import Path + + +ROOT = Path(__file__).resolve().parents[1] +SHIPPED = "plugins/coding-discipline/" + +# Each rule is (label, pattern, sample that must match). Patterns target the +# *action*, not the vocabulary: a GitHub URL in a manifest is metadata, while +# `curl https://...` inside a hook is egress. Keeping that distinction is what +# stops this file from degrading into noise everyone learns to ignore. +# +# The sample is not decoration. A scanner whose pattern silently stops matching +# is worse than no scanner, because it reports success forever. Every rule has +# to prove it still fires before it is trusted to prove anything else. +RULES: list[tuple[str, re.Pattern[str], str]] = [ + ( + "network egress from a shipped file", + re.compile( + r"(? {match.group(0)!r}") + +assert not findings, "shipped plugin surface failed the safety scan:\n" + "\n".join(findings) + +# The plugin's only persistence is the local usage log. If that sink ever moves, +# the "nothing is sent over the network" claim in README.md needs re-checking +# rather than inheriting the trust this test grants it today. +usage_lib = (ROOT / SHIPPED / "hooks" / "usage-lib.sh").read_text(encoding="utf-8") +assert 'CD_USAGE_LOG="${CD_USAGE_LOG:-$HOME/.coding-discipline/usage.jsonl}"' in usage_lib, ( + "the local usage sink moved; re-confirm the no-network privacy claim in README.md" +) + +print(f"skill safety tests passed ({scanned} shipped files scanned)")