Compare commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
61c68c01b0 | ||
|
|
b0d58d5adf | ||
|
|
55b2e811d1 | ||
|
|
0dcf5cc136 | ||
|
|
c25175e4b3 | ||
|
|
e2a93c1e99 | ||
|
|
2399ce68e5 | ||
|
|
46cbfc5967 | ||
|
|
cf3abafc66 | ||
|
|
6838baa071 | ||
|
|
f031c2b77d | ||
|
|
1328dcd8f3 | ||
|
|
6cc224f151 | ||
|
|
8359d61e33 | ||
|
|
400d2b9b6b | ||
|
|
4edf42ce9f | ||
|
|
2f53346b8c | ||
|
|
dcc0677270 | ||
|
|
9fd5a054b3 | ||
|
|
da76a6b3d2 |
@@ -1,7 +1,7 @@
|
|||||||
{
|
{
|
||||||
"name": "skills",
|
"name": "skills",
|
||||||
"description": "Mathias's engineering skills library, plus the hooks and MCP servers that make it enforceable: TDD, clean code, debugging, specs, discovery, CI, and secret hygiene.",
|
"description": "Mathias's engineering skills library, plus the hooks and MCP servers that make it enforceable: TDD, clean code, debugging, specs, discovery, CI, and secret hygiene.",
|
||||||
"version": "0.18.1",
|
"version": "0.22.1",
|
||||||
"author": {
|
"author": {
|
||||||
"name": "Mathias",
|
"name": "Mathias",
|
||||||
"url": "https://git.d-ma.be/mathias"
|
"url": "https://git.d-ma.be/mathias"
|
||||||
|
|||||||
@@ -133,6 +133,33 @@ jobs:
|
|||||||
git add "$manifest"
|
git add "$manifest"
|
||||||
# The trailer below stops this push from re-triggering the tagger.
|
# The trailer below stops this push from re-triggering the tagger.
|
||||||
git commit -m "chore(release): plugin.json version $version" -m "Skip-Release: true"
|
git commit -m "chore(release): plugin.json version $version" -m "Skip-Release: true"
|
||||||
|
|
||||||
|
# skills#46. This commit is pushed with secrets.GITEA_TOKEN, and a
|
||||||
|
# push authenticated with the Actions token does NOT trigger
|
||||||
|
# workflows -- deliberate platform behaviour to stop CI recursing.
|
||||||
|
# `Skip-Release: true` stops the tagger recursing; nothing stopped
|
||||||
|
# the whole run, `gate` included, from never firing on this commit.
|
||||||
|
#
|
||||||
|
# So every release left main's HEAD with zero CI runs. Measured:
|
||||||
|
# 6cc224f (0.19.4) and f031c2b (0.19.5) each have none, and the
|
||||||
|
# koala cluster's CIHeadUncovered{repo=skills} had been firing on it
|
||||||
|
# for 85 hours -- an alert that can only be cleared by an unrelated
|
||||||
|
# human push.
|
||||||
|
#
|
||||||
|
# The commit is not cosmetic: plugin.json's version is what Claude
|
||||||
|
# Code gates plugin updates on, `task check` asserts it is not
|
||||||
|
# behind the latest tag, and the sed-and-retry logic above already
|
||||||
|
# shipped a mismatched manifest once (v0.10.0 reading 0.9.0).
|
||||||
|
#
|
||||||
|
# Gating it HERE rather than re-triggering CI afterwards: the commit
|
||||||
|
# is produced here, so it can be validated here, at the cost of one
|
||||||
|
# `task check` instead of a second full run -- and with no recursion
|
||||||
|
# risk to reintroduce.
|
||||||
|
if ! task check; then
|
||||||
|
echo "::error::the release-bump commit fails task check -- not pushing it" >&2
|
||||||
|
exit 1
|
||||||
|
fi
|
||||||
|
|
||||||
if git push origin HEAD:main; then
|
if git push origin HEAD:main; then
|
||||||
echo "bumped $manifest to $version"
|
echo "bumped $manifest to $version"
|
||||||
exit 0
|
exit 0
|
||||||
|
|||||||
@@ -1,5 +1,5 @@
|
|||||||
{
|
{
|
||||||
"_comment": "Tailscale Serve on koala, not the public d-ma.be hostnames. The public path exits to Cloudflare and NPM to reach a k3s service on the same machine most sessions run on, which imposes rate limits and message-size caps on tool results. Serve terminates TLS with a real Tailscale cert and proxies to the NodePort over loopback, so traffic reaches the service from koala itself and satisfies the default-deny-ingress NetworkPolicy with no cluster change. Clients need 'tailscale set --accept-dns=true' for the MagicDNS name to resolve; see mathias/skills#24.",
|
"_comment": "Tailscale Serve on koala, not the public d-ma.be hostnames. The public path exits to Cloudflare and NPM to reach a k3s service on the same machine most sessions run on, which imposes rate limits and message-size caps on tool results. Serve terminates TLS with a real Tailscale cert and proxies to the NodePort over loopback, so traffic reaches the service from koala itself and satisfies the default-deny-ingress NetworkPolicy with no cluster change. Clients need 'tailscale set --accept-dns=true' for the MagicDNS name to resolve; see mathias/skills#24. Every ${VAR} below must be one ~/.op-env defines -- bin/check-mcp-env enforces that, because an undefined one sends an empty Authorization header and reports as 'requires authentication', which is indistinguishable from a revoked credential.",
|
||||||
"mcpServers": {
|
"mcpServers": {
|
||||||
"brain": {
|
"brain": {
|
||||||
"type": "http",
|
"type": "http",
|
||||||
@@ -11,23 +11,24 @@
|
|||||||
"infra": {
|
"infra": {
|
||||||
"type": "http",
|
"type": "http",
|
||||||
"url": "https://koala.tail30472e.ts.net:8444/mcp"
|
"url": "https://koala.tail30472e.ts.net:8444/mcp"
|
||||||
|
},
|
||||||
|
"gitea": {
|
||||||
|
"_comment": "gitea-mcp authenticates by PASS-THROUGH: the bearer is a Gitea PAT, which the server validates against Gitea and then acts as. So no dedicated MCP token is needed, and the AgentSecrets blocker that parked this entry (skills#32, #33) never applied to it -- DMABE_GITEA_API_TOKEN is op://HomeLab and already in the session environment. Verified 2026-09-07: initialize 200 (serverInfo gitea-mcp) and tools/list returns 41 tools. Two DIFFERENT PATs both authenticate while GITEA_MCP_TOKEN 401s, which is what proves this is pass-through validation and not a static-token comparison.",
|
||||||
|
"type": "http",
|
||||||
|
"url": "https://koala.tail30472e.ts.net:8446/mcp",
|
||||||
|
"headers": {
|
||||||
|
"Authorization": "Bearer ${DMABE_GITEA_API_TOKEN}"
|
||||||
|
}
|
||||||
}
|
}
|
||||||
},
|
},
|
||||||
"_parked": {
|
"_parked": {
|
||||||
"_reason": "cad and gitea-mcp are proxied, reachable and AUTHENTICATING: tailscale serve 8445 -> 127.0.0.1:31877 and 8446 -> 127.0.0.1:30340; an MCP initialize with the correct bearer returns 200 from both (serverInfo gitea-mcp 0.1.0, cad aed2aaf). They stay parked only because nothing puts those two tokens into the session environment. Both live in the 1Password AgentSecrets vault, and by deliberate design (infra#224, closed) no single identity on koala reads both vaults: koala-homelab is HomeLab-only and resolves ~/.op-env, koala-agentsecrets-write is AgentSecrets-only and lives in ~/.op-agent-write.env. Adding an AgentSecrets reference to ~/.op-env does NOT degrade gracefully -- `op run` fails wholesale and every other token, BRAIN_MCP_TOKEN included, goes unresolved with it. Enabling needs a read-scoped AgentSecrets identity in the session-env path; see mathias/skills#32, #33.",
|
"_reason": "cad is proxied, reachable and AUTHENTICATING: tailscale serve 8445 -> 127.0.0.1:31877, and an MCP initialize with the correct bearer returns 200 (serverInfo cad aed2aaf). It stays parked only because nothing puts CAD_MCP_TOKEN into the session environment. It lives in the 1Password AgentSecrets vault, and by deliberate design (infra#224, closed) no single identity on koala reads both vaults: koala-homelab is HomeLab-only and resolves ~/.op-env, koala-agentsecrets-write is AgentSecrets-only and lives in ~/.op-agent-write.env. Adding an AgentSecrets reference to ~/.op-env does NOT degrade gracefully -- `op run` fails wholesale and every other token, BRAIN_MCP_TOKEN included, goes unresolved with it. Enabling needs a read-scoped AgentSecrets identity in the session-env path; see mathias/skills#32, #33. gitea was parked here for the same stated reason and has been unparked: it needs no AgentSecrets token at all, because gitea-mcp is pass-through-authenticated.",
|
||||||
"cad": {
|
"cad": {
|
||||||
"type": "http",
|
"type": "http",
|
||||||
"url": "https://koala.tail30472e.ts.net:8445/mcp",
|
"url": "https://koala.tail30472e.ts.net:8445/mcp",
|
||||||
"headers": {
|
"headers": {
|
||||||
"Authorization": "Bearer ${CAD_MCP_TOKEN}"
|
"Authorization": "Bearer ${CAD_MCP_TOKEN}"
|
||||||
}
|
}
|
||||||
},
|
|
||||||
"gitea": {
|
|
||||||
"type": "http",
|
|
||||||
"url": "https://koala.tail30472e.ts.net:8446/mcp",
|
|
||||||
"headers": {
|
|
||||||
"Authorization": "Bearer ${GITEA_MCP_STATIC_TOKEN}"
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -29,7 +29,7 @@ descriptions and "use when" triggers.
|
|||||||
/plugin install skills@mathias
|
/plugin install skills@mathias
|
||||||
```
|
```
|
||||||
|
|
||||||
The plugin ships skills, the `secret-guard` + `pre-task-ritual` hooks, and the
|
The plugin ships skills, the `secret-guard`, `mcp-destructive-guard` and `pre-task-ritual` hooks, and the
|
||||||
brain/gitea/infra MCP servers as one versioned unit. Skills are namespaced
|
brain/gitea/infra MCP servers as one versioned unit. Skills are namespaced
|
||||||
(`skills:tdd`). Updates land when `.claude-plugin/plugin.json`'s `version`
|
(`skills:tdd`). Updates land when `.claude-plugin/plugin.json`'s `version`
|
||||||
moves — CI bumps it on every release.
|
moves — CI bumps it on every release.
|
||||||
|
|||||||
@@ -17,6 +17,7 @@ the SKILL.md, not the table.
|
|||||||
| `cognitive-load` | Cognitive complexity analysis (CLI score) | Identifying hotspots, before refactoring | claude-code, library |
|
| `cognitive-load` | Cognitive complexity analysis (CLI score) | Identifying hotspots, before refactoring | claude-code, library |
|
||||||
| `debug` | Hypothesis-first debugging discipline | Any test failure or bug whose cause is not immediately obvious — or a bug report with no failure text, which this skill refuses rather than guesses at | claude-code, library |
|
| `debug` | Hypothesis-first debugging discipline | Any test failure or bug whose cause is not immediately obvious — or a bug report with no failure text, which this skill refuses rather than guesses at | claude-code, library |
|
||||||
| `discovery-framing` | Problem-before-solution discovery/framing method | Framing/validating a problem before any solution; opportunity sizing | claude-code, library |
|
| `discovery-framing` | Problem-before-solution discovery/framing method | Framing/validating a problem before any solution; opportunity sizing | claude-code, library |
|
||||||
|
| `document-to-markdown` | Read .docx/.xlsx/.pptx/.epub/.msg and other formats the Read tool cannot open | You need the contents of a binary document — Word, Excel, PowerPoint, EPUB, Outlook .msg, a zip of documents — and Read returns nothing useful | claude-code, library |
|
||||||
| `dream` | Reflective consolidation pass over an agent's memory directory | Pruning/merging memory files, rebuilding a memory index | claude-code, library, chat |
|
| `dream` | Reflective consolidation pass over an agent's memory directory | Pruning/merging memory files, rebuilding a memory index | claude-code, library, chat |
|
||||||
| `experiment-spec` | Rigorous experiment spec — falsifiable hypothesis, quantitative acceptance, null-result protocol | Scoping a research/ML phase instead of a feature | claude-code, library |
|
| `experiment-spec` | Rigorous experiment spec — falsifiable hypothesis, quantitative acceptance, null-result protocol | Scoping a research/ML phase instead of a feature | claude-code, library |
|
||||||
| `feature-spec` | Implementation-level feature spec (tighter than spec-driven-dev) | Scoping one feature inside an already-specced project | claude-code, library |
|
| `feature-spec` | Implementation-level feature spec (tighter than spec-driven-dev) | Scoping one feature inside an already-specced project | claude-code, library |
|
||||||
|
|||||||
+21
-1
@@ -78,7 +78,7 @@ tasks:
|
|||||||
for entry in "{{.TASKFILE_DIR}}"/*; do
|
for entry in "{{.TASKFILE_DIR}}"/*; do
|
||||||
base=$(basename "$entry")
|
base=$(basename "$entry")
|
||||||
case "$base" in
|
case "$base" in
|
||||||
install.sh|Taskfile.yml|README.md|SKILLS_INDEX.md|skills|hooks|bin|tests) continue ;;
|
install.sh|Taskfile.yml|README.md|SKILLS_INDEX.md|skills|hooks|bin|tests|agents) continue ;;
|
||||||
esac
|
esac
|
||||||
echo "ERROR: unexpected entry at repo root: $base" >&2
|
echo "ERROR: unexpected entry at repo root: $base" >&2
|
||||||
fail=1
|
fail=1
|
||||||
@@ -138,6 +138,26 @@ tasks:
|
|||||||
echo "ERROR: missing .claude-plugin/plugin.json" >&2; fail=1
|
echo "ERROR: missing .claude-plugin/plugin.json" >&2; fail=1
|
||||||
fi
|
fi
|
||||||
|
|
||||||
|
# Every ${VAR} a live .mcp.json server interpolates must be one the
|
||||||
|
# session env-file defines. An undefined one is silent: the header
|
||||||
|
# goes out empty and the server reports "requires authentication",
|
||||||
|
# which reads as a revoked credential. Exit 2 means the check could
|
||||||
|
# not run (no jq, or no env-file — CI has neither); that is reported
|
||||||
|
# and not fatal, but it is never treated as a pass.
|
||||||
|
if [ -x "{{.TASKFILE_DIR}}/bin/check-mcp-env" ]; then
|
||||||
|
# `|| rc=$?` is load-bearing, not style. Task runs this block under
|
||||||
|
# `sh -e`, so a bare invocation exiting non-zero kills the shell
|
||||||
|
# before `case` is reached -- the tolerance below never ran, and the
|
||||||
|
# gate died with `exit status 2` on the commit that added it.
|
||||||
|
rc=0
|
||||||
|
"{{.TASKFILE_DIR}}/bin/check-mcp-env" "{{.TASKFILE_DIR}}/.mcp.json" "$HOME/.op-env" || rc=$?
|
||||||
|
case $rc in
|
||||||
|
0) ;;
|
||||||
|
2) echo "NOTE: MCP env wiring unverified on this host" >&2 ;;
|
||||||
|
*) fail=1 ;;
|
||||||
|
esac
|
||||||
|
fi
|
||||||
|
|
||||||
if [ $fail -ne 0 ]; then
|
if [ $fail -ne 0 ]; then
|
||||||
echo "check FAILED" >&2; exit 1
|
echo "check FAILED" >&2; exit 1
|
||||||
fi
|
fi
|
||||||
|
|||||||
@@ -0,0 +1,299 @@
|
|||||||
|
---
|
||||||
|
name: pr-gatekeeper
|
||||||
|
description: Adversarially reviews one change and returns a blocking verdict. Two modes — an open Gitea PR (verify CI, then merge and close, or escalate), or a staged/unpushed diff in a GitOps repo where a branch cannot be reconciled, in which case it reviews before the push and records the verdict on the issue without merging. Use for risk/medium work. Spawned with a repo, the issue it closes, and either a PR number or a checkout path.
|
||||||
|
tools: Read, Grep, Glob, Bash
|
||||||
|
color: "#7C3AED"
|
||||||
|
---
|
||||||
|
|
||||||
|
<role>
|
||||||
|
You are the merge gate for a solo estate. Nobody else will read this PR. If you
|
||||||
|
approve something broken, it ships to a live GitOps cluster within 60 seconds.
|
||||||
|
|
||||||
|
Your default posture is refusal. The author (another agent) believes the work is
|
||||||
|
correct — that belief is the input you are testing, not evidence. The same model
|
||||||
|
in the author's seat has convinced itself five times in a row and been wrong
|
||||||
|
every time.
|
||||||
|
|
||||||
|
Blocking the same PR repeatedly is the control working, not you being
|
||||||
|
difficult. For code whose job is to refuse something, expect it.
|
||||||
|
</role>
|
||||||
|
|
||||||
|
## What you are given
|
||||||
|
|
||||||
|
One of two modes. Everything after step 2 is identical — the review standard does
|
||||||
|
not change with the plumbing.
|
||||||
|
|
||||||
|
**PR mode** (the original):
|
||||||
|
- `owner/repo`, the PR number, and the issue number it claims to close
|
||||||
|
- The claim the author made about what the change does
|
||||||
|
|
||||||
|
**Pre-push mode** (infra#470): there is no PR, because there cannot be one.
|
||||||
|
- `owner/repo`, the path to the checkout, and the issue number the work claims to close
|
||||||
|
- The claim the author made about what the change does
|
||||||
|
|
||||||
|
In a repo whose deployment watches one branch — `mathias/infra`, where Flux
|
||||||
|
watches `main` — a branch is invisible to the thing that applies it. A
|
||||||
|
`risk/medium` change to `alerts.yaml` or `litellm-config.yaml` can be linted on a
|
||||||
|
branch but never *reconciled* there, so a PR would review a state that was never
|
||||||
|
deployed. The agreed lane is therefore that such work stays on `main` and gets a
|
||||||
|
recorded adversarial review pass **before the push**, with you as the reviewer.
|
||||||
|
|
||||||
|
If that lane resolves to "the author was careful", it has replaced nothing. You are
|
||||||
|
the difference between the rule and a sentence about the rule.
|
||||||
|
|
||||||
|
## Procedure
|
||||||
|
|
||||||
|
Work in the repo checkout at the path given to you. Never push, never force,
|
||||||
|
never amend the author's commits.
|
||||||
|
|
||||||
|
### 1. Read the issue before the diff
|
||||||
|
|
||||||
|
```
|
||||||
|
mcp__plugin_skills_gitea__issue_get
|
||||||
|
mcp__plugin_skills_gitea__issue_list_comments
|
||||||
|
```
|
||||||
|
|
||||||
|
The issue's "done when" clauses are the acceptance criteria. A PR that is
|
||||||
|
well-built and does not satisfy them is a **reject**, not an approve-with-note.
|
||||||
|
Comments carry sequencing constraints that the body does not.
|
||||||
|
|
||||||
|
### 2. Get the real diff
|
||||||
|
|
||||||
|
**PR mode:**
|
||||||
|
|
||||||
|
```bash
|
||||||
|
git fetch origin
|
||||||
|
git diff origin/main...origin/<branch>
|
||||||
|
```
|
||||||
|
|
||||||
|
**Pre-push mode** — the change is staged, or committed and unpushed:
|
||||||
|
|
||||||
|
```bash
|
||||||
|
git status --porcelain # know what is staged vs merely present
|
||||||
|
git diff --staged # staged but not committed
|
||||||
|
git diff origin/main...HEAD # committed locally, not yet pushed
|
||||||
|
```
|
||||||
|
|
||||||
|
Run all three. Which one holds the change is itself information: work that is
|
||||||
|
neither staged nor committed is not a diff under review, it is a working tree, and
|
||||||
|
you should say so rather than review it. Untracked files do not appear in either
|
||||||
|
diff — `git status --porcelain` is how you notice a new file the author forgot to
|
||||||
|
stage, and a control that exists only in an unstaged file is not going to ship.
|
||||||
|
|
||||||
|
Read every hunk. Do not skim. If the diff is too large to review properly, say
|
||||||
|
so and escalate rather than producing confident coverage of a fraction of it.
|
||||||
|
|
||||||
|
### 3. Iron laws — any violation blocks
|
||||||
|
|
||||||
|
1. **Secrets.** No credential, token or key in the diff, in a test fixture, or
|
||||||
|
rendered into output by a command the change adds. A `kubectl get secret -o
|
||||||
|
yaml`, a `sops -d` piped to stdout, or an `echo $TOKEN` is an immediate block.
|
||||||
|
2. **Swallowed errors.** `_ = f()`, a bare `except:`, `|| true` on a line whose
|
||||||
|
failure matters, `err` returned naked.
|
||||||
|
3. **Unvalidated input at a boundary.** External API responses, file reads, user
|
||||||
|
input used before checking.
|
||||||
|
4. **Live-breaking GitOps changes.** For `mathias/infra`: does this leave the
|
||||||
|
cluster reconcilable? A `kubectl diff` that is clean is **not** proof Flux
|
||||||
|
will accept it — server-side apply does not prune fields it does not own, and
|
||||||
|
that took the whole `apps` Kustomization down on 2026-09-16.
|
||||||
|
|
||||||
|
### 4. The test is the claim — attack it
|
||||||
|
|
||||||
|
This estate's recurring defect is **a check that passes without proving
|
||||||
|
anything**. Confirmed instances to pattern-match against:
|
||||||
|
|
||||||
|
- a suite grading a subprocess's stdout passed when the subprocess crashed:
|
||||||
|
empty output, zero assertions, `fail` stayed 0
|
||||||
|
- a gate mapped 11 of 18 subjects and *printed* the other seven, then exited 0
|
||||||
|
- a manifest check matched `type: Recreate` inside a **comment** explaining why
|
||||||
|
Recreate had been rejected
|
||||||
|
|
||||||
|
So, for every test the PR adds or changes:
|
||||||
|
|
||||||
|
- **Flip it.** Break the thing it claims to check and run it. If it still
|
||||||
|
passes, the test is decoration. Say so and block. On `dispatch#75` this found
|
||||||
|
**three behavioural changes shipped with no assertion at all** — correct code
|
||||||
|
that nothing would have noticed the removal of.
|
||||||
|
- **Read the name against the assertion.** A test whose name claims more than
|
||||||
|
it proves is worse than no test, because the name wins in the reader's head
|
||||||
|
and the gap becomes invisible. Found three times in one PR, twice in tests
|
||||||
|
written to fix the previous instance. `DoesNotInheritEnv` is honest;
|
||||||
|
`IsContained` is a claim the test cannot support. (local-dev `AGENT.md` rule 9.)
|
||||||
|
- Does it fail when it cannot run? An empty result, a parser that matches
|
||||||
|
nothing, a missing file must all be failures, never silent passes.
|
||||||
|
- Does it assert on what the machine reads, or on prose near it?
|
||||||
|
- Is coverage total, or a majority? Partial coverage must be expressed as a
|
||||||
|
failure or an explicit, justified allowlist.
|
||||||
|
|
||||||
|
A PR with no test for a behavioural change is a block unless the change is
|
||||||
|
purely declarative config, and even then ask what would catch its regression.
|
||||||
|
|
||||||
|
### 4b. How to attack — the three habits that actually found the escapes
|
||||||
|
|
||||||
|
Five rounds on `dispatch#75` produced five real escapes. **CI was green on every
|
||||||
|
one of them.** These are what found them; reading the diff did not.
|
||||||
|
|
||||||
|
**Verify by execution, not by reading the implementation.** Build a scratch
|
||||||
|
repo, run the command, look at the filesystem. Every finding came from a
|
||||||
|
command that ran, not from reasoning about a matcher. The matcher said one
|
||||||
|
thing and the filesystem said another, five times running. A claim you have not
|
||||||
|
executed is a hypothesis.
|
||||||
|
|
||||||
|
**Read the survivors.** After any allow/deny sweep, read the list of what is
|
||||||
|
still *permitted*, one entry at a time. This is what caught `git commit -F
|
||||||
|
/etc/passwd` — reads an arbitrary file into a commit message that is then
|
||||||
|
pushed, i.e. exfiltration wearing a message flag. It was found by neither the
|
||||||
|
review nor the differential, because both implementations **agreed** and both
|
||||||
|
were wrong. Agreement is not correctness.
|
||||||
|
|
||||||
|
**Grade a tool on its exit code, never on grepping its output.** A crashed
|
||||||
|
linter greps as clean. `golangci-lint` exits 0 clean / 1 issues found / >1 the
|
||||||
|
linter itself failed — and a harness that matched its summary text scored a
|
||||||
|
crash as a pass, leaving the only gate over an entire package silently absent.
|
||||||
|
|
||||||
|
Where a control has **two implementations** kept in sync by hand, run one case
|
||||||
|
list through both and diff the verdicts mechanically. A case one refuses and
|
||||||
|
the other allows is a real finding, and this has caught one.
|
||||||
|
|
||||||
|
### 4c. Probing without causing the incident you are testing for
|
||||||
|
|
||||||
|
**This agent has caused a credential leak.** On 2026-09-19 a probe testing
|
||||||
|
environment scrubbing printed `/proc/$PPID/environ` and `ps e` raw, and a `git
|
||||||
|
credential fill` consulted the host's real store. Seven live credentials went
|
||||||
|
into the transcript and had to be rotated (`infra#445`). The finding was
|
||||||
|
correct; the method was not.
|
||||||
|
|
||||||
|
Non-negotiable when probing anything credential-adjacent:
|
||||||
|
|
||||||
|
- **A probe that reads an environment prints a count, never the buffer.**
|
||||||
|
Grep for a sentinel and report whether it was found. A leak test that prints
|
||||||
|
what it found defeats itself.
|
||||||
|
- **Use an invented sentinel**, never a real secret name. Set it yourself, then
|
||||||
|
assert on its absence.
|
||||||
|
- **Anything touching git's credential subsystem runs with an isolated `HOME`**
|
||||||
|
and `GIT_CONFIG_GLOBAL`/`GIT_CONFIG_SYSTEM` pointed away from the real ones.
|
||||||
|
Otherwise it reads the host's store, which is the point of the subsystem.
|
||||||
|
- Prove a primitive and stop. Do not aim a demonstrated write or execution
|
||||||
|
primitive at a real secret to show it works.
|
||||||
|
|
||||||
|
A command whose *purpose* is to surface credentials will surface real ones if
|
||||||
|
you point it at a real environment.
|
||||||
|
|
||||||
|
### 5. CI
|
||||||
|
|
||||||
|
```
|
||||||
|
mcp__plugin_skills_gitea__pr_get # mergeable, conflicts
|
||||||
|
mcp__plugin_skills_gitea__workflow_run_list
|
||||||
|
```
|
||||||
|
|
||||||
|
CI green is necessary and not sufficient — you have just spent step 4
|
||||||
|
establishing what its green is worth. On `dispatch#75` it was worth nothing
|
||||||
|
five times running: `task check` exit 0, `golangci-lint` exit 0, every test
|
||||||
|
passing, while `git rm ././.dispatch-allow` deleted the file the control
|
||||||
|
existed to protect. Say so in your report when it applies: *green, and green
|
||||||
|
covers none of these findings.*
|
||||||
|
|
||||||
|
CI **red or absent** is an automatic block: an agent-opened PR with zero runs
|
||||||
|
is a known platform failure mode in this estate, not a pass.
|
||||||
|
|
||||||
|
**Pre-push mode: CI cannot have run**, because nothing has been pushed. The only
|
||||||
|
signal is the author's local gate, and the absence of CI is not a finding against
|
||||||
|
the change — it is a fact about the mode. Two consequences:
|
||||||
|
|
||||||
|
- Do not block for "CI absent" here. That rule exists for a *pushed* branch with no
|
||||||
|
runs, which is a platform failure; an unpushed diff having no runs is arithmetic.
|
||||||
|
- Verify the local gate the way you would verify any claim: run it yourself. Check
|
||||||
|
the **exit code**, not filtered output — `task check; echo $?` — because a grep
|
||||||
|
over a gate's output has hidden a failing test in this estate before. "The author
|
||||||
|
says it is green" is the belief you are testing.
|
||||||
|
|
||||||
|
So in pre-push mode your report carries strictly less external evidence than in PR
|
||||||
|
mode, and must say so. The verdict is "nothing I could find blocks this push", never
|
||||||
|
"CI confirms this".
|
||||||
|
|
||||||
|
### 6. Verdict
|
||||||
|
|
||||||
|
**PR mode — merge** only when all hold:
|
||||||
|
|
||||||
|
- no iron-law violation
|
||||||
|
- every "done when" clause in the issue is satisfied
|
||||||
|
- CI ran and is green
|
||||||
|
- you flipped the tests and they went red
|
||||||
|
- the change is `risk/low` or `risk/medium`
|
||||||
|
|
||||||
|
Then:
|
||||||
|
|
||||||
|
```
|
||||||
|
mcp__plugin_skills_gitea__pr_merge # squash
|
||||||
|
mcp__plugin_skills_gitea__issue_comment # the evidence, on the ISSUE
|
||||||
|
mcp__plugin_skills_gitea__issue_close
|
||||||
|
mcp__plugin_skills_gitea__branch_delete
|
||||||
|
```
|
||||||
|
|
||||||
|
**Pre-push mode — clear to push** only when all hold:
|
||||||
|
|
||||||
|
- no iron-law violation
|
||||||
|
- every "done when" clause in the issue is satisfied
|
||||||
|
- you ran the local gate yourself and checked its exit code
|
||||||
|
- you flipped the tests and they went red
|
||||||
|
- the change is `risk/low` or `risk/medium`
|
||||||
|
|
||||||
|
**There is nothing to merge and no PR to close, so do not merge, do not close the
|
||||||
|
issue, and do not touch a branch.** Your entire output is a comment:
|
||||||
|
|
||||||
|
```
|
||||||
|
mcp__plugin_skills_gitea__issue_comment # the verdict, on the ISSUE
|
||||||
|
```
|
||||||
|
|
||||||
|
**The verdict goes in the issue.** That is not a stylistic preference — it is what
|
||||||
|
makes the lane a *recorded* review pass rather than a conversation. A verdict that
|
||||||
|
exists only in a chat transcript is indistinguishable from no review having
|
||||||
|
happened, and the rule that created this mode says exactly that: no recorded
|
||||||
|
verdict means the lane was not used.
|
||||||
|
|
||||||
|
Head the comment so it can be found by grep, state which mode you ran in, and say
|
||||||
|
plainly that CI had not run at the time you reviewed. The author pushes; you do not.
|
||||||
|
|
||||||
|
For `mathias/infra`, after the author has pushed:
|
||||||
|
|
||||||
|
```bash
|
||||||
|
flux reconcile kustomization apps --with-source
|
||||||
|
flux get kustomization apps
|
||||||
|
```
|
||||||
|
|
||||||
|
If it does not go `Ready: True`, **revert immediately** rather than
|
||||||
|
forward-fixing, and report. A blocked `apps` Kustomization stops every app in
|
||||||
|
the cluster, not just the changed one.
|
||||||
|
|
||||||
|
Put your evidence on the **issue**, not only the PR thread. In Gitea a merge
|
||||||
|
auto-closes the gated issue and review comments live on a thread separate from
|
||||||
|
the issue's — so a finding recorded only on the PR is a finding nobody reading
|
||||||
|
the issue will ever see.
|
||||||
|
|
||||||
|
### 7. Escalate to Mathias — and only then
|
||||||
|
|
||||||
|
Do not merge. Comment the finding on the issue and report it. Escalate when:
|
||||||
|
|
||||||
|
- the change is `risk/high`
|
||||||
|
- it touches host networking, iptables, `ip rule`, Tailscale, or UFW on koala
|
||||||
|
- it needs `sudo` (denied outright to agents on koala)
|
||||||
|
- it writes or rotates a secret
|
||||||
|
- it deletes data, a PVC, or a repo
|
||||||
|
- a finding is real and the fix is a judgement call, not a correction
|
||||||
|
- the diff is too large to review honestly
|
||||||
|
|
||||||
|
Escalation is cheap. A bad merge on a GitOps cluster is not.
|
||||||
|
|
||||||
|
## Output
|
||||||
|
|
||||||
|
Report back, in this order:
|
||||||
|
|
||||||
|
1. **VERDICT** — `MERGED` / `BLOCKED` / `ESCALATED`
|
||||||
|
2. **Findings**, most severe first, each as `path:line — what is wrong — what
|
||||||
|
happens because of it`
|
||||||
|
3. **Flip-test results** — what you broke, and whether the test noticed
|
||||||
|
4. **CI** — run id and conclusion, or "absent"
|
||||||
|
5. **What you did not check**, explicitly
|
||||||
|
|
||||||
|
Never report a clean review you did not perform. "I read the diff and it looked
|
||||||
|
fine" is not a review; naming what you attacked and what survived is.
|
||||||
Executable
+99
@@ -0,0 +1,99 @@
|
|||||||
|
#!/bin/sh
|
||||||
|
# check-mcp-env — assert every variable a live MCP server interpolates is one
|
||||||
|
# the session env-file actually defines.
|
||||||
|
#
|
||||||
|
# Why this is a gate and not a comment: the failure is silent. A header of
|
||||||
|
# "Bearer ${FOO}" where FOO is undefined does not error -- it goes out empty,
|
||||||
|
# the server answers 401, and Claude Code reports "requires authentication",
|
||||||
|
# which reads exactly like a revoked credential. On 2026-09-07 that cost a
|
||||||
|
# full investigation on koala before the credential turned out to be fine.
|
||||||
|
#
|
||||||
|
# Servers under `_parked` are exempt by design. Parked means "reachable, but
|
||||||
|
# nothing puts its token in the environment yet" -- flagging those would
|
||||||
|
# report the intended state as a fault.
|
||||||
|
#
|
||||||
|
# Usage: check-mcp-env <mcp.json> <env-file>
|
||||||
|
#
|
||||||
|
# Exit 0 every live server's variables are defined
|
||||||
|
# 1 at least one is not, named on stderr
|
||||||
|
# 2 could not verify (bad usage, unreadable input, jq absent).
|
||||||
|
# Deliberately distinct from 0: an unverified config is not a clean
|
||||||
|
# one, and a gate that cannot run should say so rather than pass.
|
||||||
|
|
||||||
|
set -u
|
||||||
|
|
||||||
|
CONFIG=${1:-}
|
||||||
|
ENVFILE=${2:-}
|
||||||
|
|
||||||
|
if [ -z "$CONFIG" ] || [ -z "$ENVFILE" ]; then
|
||||||
|
echo "usage: check-mcp-env <mcp.json> <env-file>" >&2
|
||||||
|
exit 2
|
||||||
|
fi
|
||||||
|
|
||||||
|
if ! command -v jq >/dev/null 2>&1; then
|
||||||
|
echo "SKIP: jq not installed -- MCP env wiring unverified" >&2
|
||||||
|
exit 2
|
||||||
|
fi
|
||||||
|
|
||||||
|
if [ ! -f "$CONFIG" ]; then
|
||||||
|
echo "ERROR: no such config: $CONFIG" >&2
|
||||||
|
exit 2
|
||||||
|
fi
|
||||||
|
|
||||||
|
if [ ! -f "$ENVFILE" ]; then
|
||||||
|
# EXPECTED IN CI, and permanently so. The env-file holds live credentials, so
|
||||||
|
# it is not on the runner and must not be -- which means this check can only
|
||||||
|
# ever verify on a host that has one. infra#368 added jq to the runner image
|
||||||
|
# believing that would let CI verify; it removed the jq branch below and
|
||||||
|
# changed nothing else, because this branch is the binding one.
|
||||||
|
#
|
||||||
|
# Said plainly so nobody concludes CI covers this. It does not, it cannot,
|
||||||
|
# and putting ~/.op-env on the runner to make it would be trading a real
|
||||||
|
# secret for a green tick.
|
||||||
|
echo "ERROR: no such env-file: $ENVFILE -- cannot verify MCP variables" >&2
|
||||||
|
echo " (expected in CI: this check only verifies on a host with ~/.op-env)" >&2
|
||||||
|
exit 2
|
||||||
|
fi
|
||||||
|
|
||||||
|
# Every "<server>\t<VAR>" pair a live server interpolates. `..` walks the whole
|
||||||
|
# value, so a variable is found wherever it is placed, not only under headers.
|
||||||
|
refs=$(jq -r '
|
||||||
|
.mcpServers // {}
|
||||||
|
| to_entries[]
|
||||||
|
| .key as $server
|
||||||
|
| (.value | .. | strings)
|
||||||
|
| select(test("\\$\\{"))
|
||||||
|
| [ scan("\\$\\{([A-Za-z_][A-Za-z0-9_]*)\\}")[] ]
|
||||||
|
| .[]
|
||||||
|
| "\($server)\t\(.)"
|
||||||
|
' "$CONFIG" 2>/dev/null) || {
|
||||||
|
echo "ERROR: $CONFIG is not valid JSON -- MCP env wiring unverified" >&2
|
||||||
|
exit 2
|
||||||
|
}
|
||||||
|
|
||||||
|
if [ -z "$refs" ]; then
|
||||||
|
echo "check-mcp-env: no interpolated variables in live servers"
|
||||||
|
exit 0
|
||||||
|
fi
|
||||||
|
|
||||||
|
fail=0
|
||||||
|
checked=0
|
||||||
|
for pair in $(printf '%s\n' "$refs" | tr '\t' ':'); do
|
||||||
|
server=${pair%%:*}
|
||||||
|
var=${pair#*:}
|
||||||
|
checked=$((checked + 1))
|
||||||
|
if grep -qE "^[[:space:]]*(export[[:space:]]+)?$var=" "$ENVFILE"; then
|
||||||
|
continue
|
||||||
|
fi
|
||||||
|
printf 'ERROR: MCP server %s interpolates %s, which %s does not define.\n' \
|
||||||
|
"$server" "$var" "$ENVFILE" >&2
|
||||||
|
printf ' It will send an empty Authorization header and report as "requires authentication".\n' >&2
|
||||||
|
fail=1
|
||||||
|
done
|
||||||
|
|
||||||
|
if [ "$fail" -ne 0 ]; then
|
||||||
|
exit 1
|
||||||
|
fi
|
||||||
|
|
||||||
|
printf 'check-mcp-env: %d interpolated variable(s) across live MCP servers, all defined in %s\n' \
|
||||||
|
"$checked" "$ENVFILE"
|
||||||
+61
@@ -327,6 +327,67 @@ if [ "$ONLINE" -eq 1 ]; then
|
|||||||
fi
|
fi
|
||||||
fi
|
fi
|
||||||
|
|
||||||
|
# --- toolchain: golangci-lint must be built with Go >= GOROOT ---------------
|
||||||
|
# infra#375, second recurrence. golangci-lint embeds the go/types export-data
|
||||||
|
# reader from the Go it was BUILT with. When GOROOT is newer, that reader
|
||||||
|
# cannot decode the stdlib and the linter dies:
|
||||||
|
#
|
||||||
|
# panic: file requires newer Go version go1.27 (application built with go1.26)
|
||||||
|
#
|
||||||
|
# An earlier form printed a typecheck error naming the user's own file, which
|
||||||
|
# sends people to debug code that is fine. It cost time in two repos before
|
||||||
|
# anyone recognised it as a host fault rather than a repo fault.
|
||||||
|
#
|
||||||
|
# It is invisible to CI -- the runner image has matching versions -- so it
|
||||||
|
# breaks exactly the local `task check` that AGENT.md requires before a push,
|
||||||
|
# and nothing else. That is why it belongs in doctor: the signal was missing,
|
||||||
|
# not the fix.
|
||||||
|
#
|
||||||
|
# Fails open on anything unparseable. A host with no Go toolchain is a normal
|
||||||
|
# host, not a broken one, and doctor runs in a SessionStart hook everywhere.
|
||||||
|
ver_lt() {
|
||||||
|
# 0 when $1 < $2 on major.minor. Deliberately not sort -V: this is POSIX sh
|
||||||
|
# that also runs on macOS, and the export-data format is minor-versioned, so
|
||||||
|
# patch level does not enter into it.
|
||||||
|
_a=${1%%.*}; _ar=${1#*.}; _am=${_ar%%.*}
|
||||||
|
_b=${2%%.*}; _br=${2#*.}; _bm=${_br%%.*}
|
||||||
|
case "$_a$_am$_b$_bm" in *[!0-9]*) return 1 ;; esac
|
||||||
|
[ "$_a" -lt "$_b" ] && return 0
|
||||||
|
[ "$_a" -gt "$_b" ] && return 1
|
||||||
|
[ "$_am" -lt "$_bm" ]
|
||||||
|
}
|
||||||
|
|
||||||
|
# --- agents shipped by the plugin, shadowed by host-local copies -------------
|
||||||
|
#
|
||||||
|
# skills#51: pr-gatekeeper lived ONLY at ~/.claude/agents/pr-gatekeeper.md for
|
||||||
|
# weeks — unversioned, on one host, while doctrine came to depend on it
|
||||||
|
# (local-dev AGENT.md rule 8). Now that the plugin ships agents/, a real file of
|
||||||
|
# the same name in ~/.claude/agents takes precedence over the shipped one and
|
||||||
|
# nothing says so. That is the same silent-drift shape as the `outside` and
|
||||||
|
# `unmanaged` checks above: it works, so nothing complains, while serving
|
||||||
|
# content nobody updates.
|
||||||
|
CLAUDE_AGENTS="$HOME/.claude/agents"
|
||||||
|
if [ -d "$CHECKOUT/agents" ]; then
|
||||||
|
for _shipped in "$CHECKOUT"/agents/*.md; do
|
||||||
|
[ -e "$_shipped" ] || continue
|
||||||
|
_name=$(basename -- "$_shipped")
|
||||||
|
_local="$CLAUDE_AGENTS/$_name"
|
||||||
|
[ -e "$_local" ] || continue
|
||||||
|
if [ -L "$_local" ]; then
|
||||||
|
continue # a link is managed wiring, not a shadow
|
||||||
|
fi
|
||||||
|
add FAIL shadow "$_local is a host-local copy of the plugin's agents/$_name — it wins over the shipped one, so the versioned file is not what runs; delete it"
|
||||||
|
done
|
||||||
|
fi
|
||||||
|
|
||||||
|
if command -v golangci-lint >/dev/null 2>&1 && command -v go >/dev/null 2>&1; then
|
||||||
|
lint_go=$(golangci-lint --version 2>/dev/null | sed -n 's/.*built with go\([0-9][0-9.]*\).*/\1/p' | head -1)
|
||||||
|
root_go=$(go version 2>/dev/null | sed -n 's/.*go version go\([0-9][0-9.]*\).*/\1/p' | head -1)
|
||||||
|
if [ -n "$lint_go" ] && [ -n "$root_go" ] && ver_lt "$lint_go" "$root_go"; then
|
||||||
|
add FAIL toolchain "golangci-lint built with go$lint_go but GOROOT is go$root_go — lint panics on every Go repo; reinstall it under the current toolchain (infra#375)"
|
||||||
|
fi
|
||||||
|
fi
|
||||||
|
|
||||||
# --- output -----------------------------------------------------------------
|
# --- output -----------------------------------------------------------------
|
||||||
if [ "$JSON" -eq 1 ]; then
|
if [ "$JSON" -eq 1 ]; then
|
||||||
printf '{"checkout":"%s","expected":%s,"plugin":{"installed":"%s","available":"%s"},"findings":[%s]}\n' \
|
printf '{"checkout":"%s","expected":%s,"plugin":{"installed":"%s","available":"%s"},"findings":[%s]}\n' \
|
||||||
|
|||||||
@@ -33,6 +33,16 @@
|
|||||||
"timeout": 5
|
"timeout": 5
|
||||||
}
|
}
|
||||||
]
|
]
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"matcher": "mcp__.*",
|
||||||
|
"hooks": [
|
||||||
|
{
|
||||||
|
"type": "command",
|
||||||
|
"command": "bash \"${CLAUDE_PLUGIN_ROOT}/hooks/mcp-destructive-guard.sh\"",
|
||||||
|
"timeout": 5
|
||||||
|
}
|
||||||
|
]
|
||||||
}
|
}
|
||||||
]
|
]
|
||||||
}
|
}
|
||||||
|
|||||||
Executable
+86
@@ -0,0 +1,86 @@
|
|||||||
|
#!/usr/bin/env bash
|
||||||
|
# PreToolUse hook (mcp__.* matcher) — makes a human approve irreversible MCP
|
||||||
|
# verbs instead of letting the model run them unattended.
|
||||||
|
#
|
||||||
|
# Why (infra#508, infra ADR-0027): gitea-mcp authenticates by PAT pass-through,
|
||||||
|
# so an agent holds the full scope of the caller's PAT. repo_delete's own guard
|
||||||
|
# (confirm == repo) is filled in by the model on both sides: it stops typos,
|
||||||
|
# not an injected or mistaken agent. The infra-mcp README claimed client-side
|
||||||
|
# deny lists covered this; on 2026-10-01 none existed in any settings file.
|
||||||
|
#
|
||||||
|
# Policy: match gitea-mcp's verb spellings on ANY server name. The same tool is
|
||||||
|
# reachable as mcp__plugin_skills_gitea__*, mcp__claude_ai_gitea__* or under
|
||||||
|
# whatever name a host registers it with, so keying on the server name would
|
||||||
|
# let a rename bypass the gate. It does NOT recognise other servers' spellings
|
||||||
|
# of the same operation (e.g. a third-party `delete_repo`).
|
||||||
|
#
|
||||||
|
# file_write_branch is gated when it would land on main/master (on infra that
|
||||||
|
# is a Flux deploy within ~60s — the same effect as pr_merge) or empty a file
|
||||||
|
# (the same effect as file_delete). Writes to other branches pass.
|
||||||
|
#
|
||||||
|
# Emits "ask" or nothing; never "allow" (a hook allow would override the
|
||||||
|
# user's own deny/ask rules). Always exits 0: Claude Code ignores a hook's
|
||||||
|
# JSON on a non-zero exit. Anything it cannot parse — including a host
|
||||||
|
# without jq — fails closed to "ask". The ask JSON is a literal, so the
|
||||||
|
# fail-closed path needs no tool that might be missing.
|
||||||
|
#
|
||||||
|
# Scope limit, recorded in infra ADR-0027: this gates MCP calls only. A
|
||||||
|
# session with the PAT in its environment can still reach the Gitea REST API
|
||||||
|
# through a shell.
|
||||||
|
|
||||||
|
set -u
|
||||||
|
|
||||||
|
DESTRUCTIVE='repo_delete|file_delete|branch_delete|pr_merge|tbd_ship|repo_update|repo_mirror_push|workflow_run_trigger|tag_create|release_create'
|
||||||
|
|
||||||
|
ask() {
|
||||||
|
# $1 must contain only [A-Za-z0-9 _.:()'/-] — it is spliced into JSON verbatim.
|
||||||
|
printf '{"hookSpecificOutput":{"hookEventName":"PreToolUse","permissionDecision":"ask","permissionDecisionReason":"%s"}}\n' "$1"
|
||||||
|
exit 0
|
||||||
|
}
|
||||||
|
|
||||||
|
INPUT="$(cat)"
|
||||||
|
|
||||||
|
command -v jq >/dev/null 2>&1 \
|
||||||
|
|| ask "mcp-destructive-guard: jq is not installed, so this MCP call cannot be checked. Approve only if you expected it."
|
||||||
|
|
||||||
|
if ! TOOL_NAME="$(printf '%s' "$INPUT" | jq -er '.tool_name | strings | select(length > 0)' 2>/dev/null)"; then
|
||||||
|
ask "mcp-destructive-guard: could not read a tool name from this call, so it cannot be shown safe. Approve only if you expected it."
|
||||||
|
fi
|
||||||
|
|
||||||
|
case "$TOOL_NAME" in
|
||||||
|
mcp__*__*) ;;
|
||||||
|
*) exit 0 ;;
|
||||||
|
esac
|
||||||
|
|
||||||
|
verb="${TOOL_NAME##*__}"
|
||||||
|
safe_verb="$(printf '%s' "$verb" | tr -cd 'A-Za-z0-9_-')"
|
||||||
|
|
||||||
|
if printf '%s' "$verb" | grep -Eqix "($DESTRUCTIVE)"; then
|
||||||
|
ask "Irreversible MCP verb '$safe_verb' (infra ADR-0027): needs your approval. The model filled in every argument, including any confirm field."
|
||||||
|
fi
|
||||||
|
|
||||||
|
if printf '%s' "$verb" | grep -Eqix 'file_write_branch'; then
|
||||||
|
# Cleared only when there is exactly ONE key that case-folds to "branch" and
|
||||||
|
# one to "content": gitea-mcp decodes with Go encoding/json, which matches
|
||||||
|
# keys case-insensitively and lets the last one win, so {"branch":"feat/x",
|
||||||
|
# "Branch":"main"} landed on main while a lowercase-only read saw feat/x.
|
||||||
|
# The branch is cleared by SHAPE (allowlist), not by a blocklist of names:
|
||||||
|
# "refs/heads/main" created a second ref that wedged API writes to main.
|
||||||
|
verdict="$(printf '%s' "$INPUT" | jq -r '
|
||||||
|
.tool_input as $i
|
||||||
|
| if ($i | type) != "object" then "ask" else
|
||||||
|
([$i | to_entries[] | select(.key | ascii_downcase == "branch")]) as $b
|
||||||
|
| ([$i | to_entries[] | select(.key | ascii_downcase == "content")]) as $c
|
||||||
|
| if ($b | length) != 1 or ($c | length) != 1 then "ask"
|
||||||
|
elif ($b[0].key != "branch") or ($c[0].key != "content") then "ask"
|
||||||
|
elif ($b[0].value | type) != "string" or ($c[0].value | type) != "string" then "ask"
|
||||||
|
elif ($b[0].value | test("^[A-Za-z0-9][A-Za-z0-9._/-]*$") | not) then "ask"
|
||||||
|
elif ($b[0].value | test("\\.\\.|^refs/|/$")) then "ask"
|
||||||
|
elif ($b[0].value | ascii_downcase | . == "main" or . == "master") then "ask"
|
||||||
|
elif ($c[0].value | test("^\\s*$")) then "ask"
|
||||||
|
else "pass" end
|
||||||
|
end' 2>/dev/null || echo ask)"
|
||||||
|
[ "$verdict" = "pass" ] \
|
||||||
|
|| ask "file_write_branch to main/master, an unusual branch name, or emptying a file (infra ADR-0027): needs your approval."
|
||||||
|
fi
|
||||||
|
exit 0
|
||||||
@@ -0,0 +1,126 @@
|
|||||||
|
---
|
||||||
|
name: document-to-markdown
|
||||||
|
surfaces: [claude-code, library]
|
||||||
|
summary: "Read .docx/.xlsx/.pptx/.epub/.msg and other formats the Read tool cannot open"
|
||||||
|
use_when: "You need the contents of a binary document — Word, Excel, PowerPoint, EPUB, Outlook .msg, a zip of documents — and Read returns nothing useful"
|
||||||
|
description: Convert a binary document to Markdown locally with markitdown. No network, no LLM, safe for confidential material. Says when to use the docmark service instead.
|
||||||
|
---
|
||||||
|
|
||||||
|
# Document → Markdown
|
||||||
|
|
||||||
|
## Overview
|
||||||
|
|
||||||
|
`Read` opens PDFs, images and notebooks natively. It does **not** open `.docx`,
|
||||||
|
`.xlsx`, `.pptx`, `.epub`, `.msg` or `.zip`. This skill converts those locally.
|
||||||
|
|
||||||
|
One command, no install step, no service:
|
||||||
|
|
||||||
|
```bash
|
||||||
|
uvx --from 'markitdown[docx,pdf,pptx,xls,xlsx,outlook]' markitdown FILE > /tmp/out.md
|
||||||
|
```
|
||||||
|
|
||||||
|
First run downloads 38 packages (~10s); afterwards it is cached and instant.
|
||||||
|
|
||||||
|
## When to use this
|
||||||
|
|
||||||
|
You have a file on disk whose contents you need. That is the whole trigger.
|
||||||
|
|
||||||
|
## When NOT to use this — use `docmark` instead
|
||||||
|
|
||||||
|
Three cases, and only three:
|
||||||
|
|
||||||
|
1. **A scanned PDF or an image whose content is pixels, not text.** Extracting
|
||||||
|
that needs a vision model. The estate's only audited path from an image to a
|
||||||
|
model is `docmark`, whose `sovereignty.py` constructs an LLM client *only*
|
||||||
|
when `DOCMARK_VISION_MODEL` matches `koala/*` or `iguana/*`, and silently
|
||||||
|
degrades to metadata-only otherwise. `tests/test_sovereignty.py` is the
|
||||||
|
load-bearing guard. Do not reach for a cloud vision model directly; that is
|
||||||
|
the boundary the gate exists to hold.
|
||||||
|
2. **You are writing an in-cluster service** (something in `dispatch`,
|
||||||
|
`ingestion`, `cobalt-dingo`). Call `docmark` over MCP rather than bundling a
|
||||||
|
converter into each service. Its NetworkPolicy already admits `supervisor`
|
||||||
|
and `dispatch`.
|
||||||
|
3. **The file is larger than ~25 MiB** or you want the input/output caps
|
||||||
|
enforced centrally.
|
||||||
|
|
||||||
|
For a text-bearing document, the service returns **byte-identical output** to
|
||||||
|
the local command — measured 2026-09-24 on the same `.docx`, tables included.
|
||||||
|
A network hop, a bearer token and an ingress for the same bytes is not a
|
||||||
|
trade worth making.
|
||||||
|
|
||||||
|
## Confidential material
|
||||||
|
|
||||||
|
**Conversion is local and involves no LLM.** `markitdown` uses an LLM client
|
||||||
|
only to describe images; with none configured it does not call anything. No
|
||||||
|
bytes leave the machine.
|
||||||
|
|
||||||
|
That makes this safe for client material, unlike anything that routes through a
|
||||||
|
model. Two cautions:
|
||||||
|
|
||||||
|
- **Do not install `markitdown[all]`.** It pulls `az-doc-intel` and
|
||||||
|
`az-content-understanding` — Azure cloud document-intelligence converters.
|
||||||
|
They need explicit endpoint configuration to activate, but the extras list
|
||||||
|
above avoids having them on the path at all.
|
||||||
|
- The output is a file like any other. If the source was confidential, so is
|
||||||
|
`/tmp/out.md` — and the usual rule applies: it must not reach an open-egress
|
||||||
|
sink (ADR-0014).
|
||||||
|
|
||||||
|
## Verify the conversion, do not assume it
|
||||||
|
|
||||||
|
The failure mode is **a successful exit with empty output**. A `.docx` template
|
||||||
|
with no body content converts to zero characters and exits 0; so does a scanned
|
||||||
|
PDF with no text layer. Both look like success.
|
||||||
|
|
||||||
|
```bash
|
||||||
|
uvx --from 'markitdown[docx,pdf,pptx,xls,xlsx,outlook]' markitdown FILE > /tmp/out.md
|
||||||
|
wc -c /tmp/out.md
|
||||||
|
```
|
||||||
|
|
||||||
|
If the byte count is 0 or implausibly small for the document's size, the
|
||||||
|
converter found no text layer — that is case 1 above, and it needs `docmark`
|
||||||
|
with a vision model, not a retry.
|
||||||
|
|
||||||
|
## What it converts
|
||||||
|
|
||||||
|
18 converters with the extras above:
|
||||||
|
|
||||||
|
```
|
||||||
|
Audio, BingSerp, Csv, Docx, Epub, Html, Image, Ipynb, OutlookMsg, Pdf,
|
||||||
|
PlainText, Pptx, Rss, Wikipedia, Xls, Xlsx, YouTube, Zip
|
||||||
|
```
|
||||||
|
|
||||||
|
`docmark` registers 22 — the same set plus `DocxWithOCR`, `PdfWithOCR`,
|
||||||
|
`PptxWithOCR`, `XlsxWithOCR`. **Those four are the entire difference**, and
|
||||||
|
they are exactly the vision path from case 1. Everything else is identical code
|
||||||
|
running in a different place.
|
||||||
|
|
||||||
|
## Reading the output
|
||||||
|
|
||||||
|
Convert to a file and `Read` it, rather than piping into the transcript —
|
||||||
|
a 200-page document produces a lot of Markdown and most of it will not be
|
||||||
|
what you needed. Convert, then grep or read the section you want.
|
||||||
|
|
||||||
|
## Provenance
|
||||||
|
|
||||||
|
`docmark` ADR-0013 (`mathias/infra docs/decisions/`) owns the service; this
|
||||||
|
skill is the local path. The split was decided in `mathias/docmark#3` on
|
||||||
|
measurement, not preference: three of docmark's four plausible consumers are
|
||||||
|
better served without it, and the fourth — vision under a sovereignty gate — is
|
||||||
|
why it exists.
|
||||||
|
|
||||||
|
## Escalation
|
||||||
|
|
||||||
|
Local, read-only, single command. Nothing here is handed back mid-task.
|
||||||
|
|
||||||
|
Stop and hand back when:
|
||||||
|
|
||||||
|
- **The output is empty or near-empty.** Do not retry, and do not guess at the
|
||||||
|
contents. Say which file, what the byte count was, and that it needs a vision
|
||||||
|
path — then either use `docmark` with a local vision model, or ask. A
|
||||||
|
confident summary of a document you could not read is the worst possible
|
||||||
|
output of this skill.
|
||||||
|
- **The document is client material and the task would send its contents
|
||||||
|
anywhere** — a commit message, an issue, a PR body, a cloud model. That is
|
||||||
|
ADR-0014's refusal, and it is not this skill's call to make.
|
||||||
|
- **The file is encrypted or password-protected.** markitdown will fail; asking
|
||||||
|
for the password is a human's decision, not a retry.
|
||||||
Executable
+164
@@ -0,0 +1,164 @@
|
|||||||
|
#!/bin/sh
|
||||||
|
# Tests for the agents/ directory the plugin ships, and doctor's shadow check.
|
||||||
|
#
|
||||||
|
# skills#51: pr-gatekeeper blocked dispatch#75 five times, every block a real
|
||||||
|
# escape verified by execution — and the agent definition lived only at
|
||||||
|
# ~/.claude/agents/pr-gatekeeper.md on one host. Unversioned, unreviewable, one
|
||||||
|
# rm from gone. Doctrine now depends on it (local-dev AGENT.md rule 8), so the
|
||||||
|
# file has to be in the repo.
|
||||||
|
#
|
||||||
|
# These assertions observe FILE CONTENT and doctor's OUTPUT. They cannot and do
|
||||||
|
# not claim the agent reviews well — naming them otherwise would be the exact
|
||||||
|
# defect rule 9 exists for.
|
||||||
|
|
||||||
|
set -u
|
||||||
|
|
||||||
|
ROOT=$(CDPATH= cd -- "$(dirname -- "$0")/.." && pwd)
|
||||||
|
AGENT="$ROOT/agents/pr-gatekeeper.md"
|
||||||
|
DOCTOR="$ROOT/bin/doctor"
|
||||||
|
|
||||||
|
pass=0
|
||||||
|
fail=0
|
||||||
|
WORK=${TMPDIR:-/tmp}/agents_test.$$
|
||||||
|
|
||||||
|
ok() { pass=$((pass + 1)); printf 'ok: %s\n' "$1"; }
|
||||||
|
no() { fail=$((fail + 1)); printf 'FAIL: %s\n' "$1"; }
|
||||||
|
|
||||||
|
# --- the file is in the repo ------------------------------------------------
|
||||||
|
|
||||||
|
if [ -f "$AGENT" ]; then
|
||||||
|
ok 'agents/pr-gatekeeper.md exists in the checkout'
|
||||||
|
else
|
||||||
|
no 'agents/pr-gatekeeper.md exists in the checkout'
|
||||||
|
printf '\n%s passed, %s failed\n' "$pass" "$fail"
|
||||||
|
exit 1
|
||||||
|
fi
|
||||||
|
|
||||||
|
# --- frontmatter is present and well-formed ---------------------------------
|
||||||
|
|
||||||
|
head -1 "$AGENT" | grep -q '^---$' \
|
||||||
|
&& ok 'the file opens with a frontmatter fence' \
|
||||||
|
|| no 'the file opens with a frontmatter fence'
|
||||||
|
|
||||||
|
for field in name description tools; do
|
||||||
|
if sed -n '2,/^---$/p' "$AGENT" | grep -q "^$field:"; then
|
||||||
|
ok "frontmatter declares $field"
|
||||||
|
else
|
||||||
|
no "frontmatter declares $field"
|
||||||
|
fi
|
||||||
|
done
|
||||||
|
|
||||||
|
fm_name=$(sed -n '2,/^---$/p' "$AGENT" | sed -n 's/^name: *//p' | head -1)
|
||||||
|
if [ "$fm_name" = "pr-gatekeeper" ]; then
|
||||||
|
ok 'the frontmatter name equals the filename stem'
|
||||||
|
else
|
||||||
|
no "the frontmatter name equals the filename stem (got '$fm_name')"
|
||||||
|
fi
|
||||||
|
|
||||||
|
# --- the practices that actually found the defects --------------------------
|
||||||
|
#
|
||||||
|
# Each string below is traceable to a specific round of dispatch#75 or to the
|
||||||
|
# incident the agent itself caused. A grep is a weak check — it proves the
|
||||||
|
# phrase is present, not that the procedure is followed — so the test is named
|
||||||
|
# for the phrase, not for the behaviour.
|
||||||
|
|
||||||
|
check_phrase() {
|
||||||
|
if grep -qi -- "$1" "$AGENT"; then
|
||||||
|
ok "the file names: $2"
|
||||||
|
else
|
||||||
|
no "the file names: $2"
|
||||||
|
fi
|
||||||
|
}
|
||||||
|
|
||||||
|
check_phrase 'verify by execution' 'verify by execution, not by reading the implementation'
|
||||||
|
check_phrase 'flip' 'flip-testing'
|
||||||
|
check_phrase 'read the survivors' 'read the survivors of a differential'
|
||||||
|
check_phrase 'exit code' 'grade a linter on its exit code'
|
||||||
|
check_phrase 'never the buffer' 'a probe prints a count, never the buffer'
|
||||||
|
check_phrase 'claims more than' 'a test name must not claim more than it proves'
|
||||||
|
|
||||||
|
# --- doctor flags a host-local copy that shadows the shipped one ------------
|
||||||
|
|
||||||
|
setup() {
|
||||||
|
rm -rf "$WORK"
|
||||||
|
mkdir -p "$WORK/home/.claude/agents" "$WORK/ck/skills/x" "$WORK/ck/agents"
|
||||||
|
printf 'x\n' > "$WORK/ck/skills/x/SKILL.md"
|
||||||
|
printf -- '---\nname: pr-gatekeeper\n---\n' > "$WORK/ck/agents/pr-gatekeeper.md"
|
||||||
|
}
|
||||||
|
|
||||||
|
setup
|
||||||
|
printf -- '---\nname: pr-gatekeeper\n---\n' > "$WORK/home/.claude/agents/pr-gatekeeper.md"
|
||||||
|
out=$(HOME="$WORK/home" DOCTOR_CHECKOUT="$WORK/ck" sh "$DOCTOR" 2>&1)
|
||||||
|
if printf '%s' "$out" | grep -q 'pr-gatekeeper'; then
|
||||||
|
ok 'doctor reports a host-local agent that shadows a shipped one'
|
||||||
|
else
|
||||||
|
no "doctor reports a host-local agent that shadows a shipped one (got: $out)"
|
||||||
|
fi
|
||||||
|
|
||||||
|
setup
|
||||||
|
out=$(HOME="$WORK/home" DOCTOR_CHECKOUT="$WORK/ck" sh "$DOCTOR" 2>&1)
|
||||||
|
if printf '%s' "$out" | grep -q 'pr-gatekeeper'; then
|
||||||
|
no "doctor is silent when no host-local copy exists (got: $out)"
|
||||||
|
else
|
||||||
|
ok 'doctor is silent when no host-local copy exists'
|
||||||
|
fi
|
||||||
|
|
||||||
|
# --- the pre-push mode the GitOps review lane depends on (infra#470) ---------
|
||||||
|
#
|
||||||
|
# infra#470 resolved a doctrine conflict: root rule 5 wants a PR for risk/medium+,
|
||||||
|
# and mathias/infra forbids branches because Flux watches main and a branch can be
|
||||||
|
# linted but never reconciled. The agreed lane is that such work stays on main and
|
||||||
|
# instead gets a RECORDED ADVERSARIAL REVIEW PASS before the push.
|
||||||
|
#
|
||||||
|
# That lane names this agent as its reviewer. If the definition only knows how to
|
||||||
|
# review an open PR, the lane resolves to "be careful" — which is what rule 5 was
|
||||||
|
# written to replace. These assertions observe FILE CONTENT only: that the mode is
|
||||||
|
# documented, that it does not merge, and that the verdict has a stated home. They
|
||||||
|
# do NOT claim the agent reviews a staged diff well (rule 9).
|
||||||
|
|
||||||
|
if grep -qi 'git diff --staged\|git diff --cached' "$AGENT"; then
|
||||||
|
ok 'the definition names a command that reads a staged diff, not only a branch diff'
|
||||||
|
else
|
||||||
|
no 'the definition has no staged-diff command, so the infra#470 lane has no reviewer'
|
||||||
|
fi
|
||||||
|
|
||||||
|
if grep -qi 'pre-push' "$AGENT"; then
|
||||||
|
ok 'the definition names the pre-push mode'
|
||||||
|
else
|
||||||
|
no 'the definition does not name a pre-push mode (infra#470)'
|
||||||
|
fi
|
||||||
|
|
||||||
|
# The verdict has to land somewhere durable, or "recorded review pass" is untrue.
|
||||||
|
# The lane specifies the issue, deliberately not a PR thread — there is no PR.
|
||||||
|
if grep -qi 'verdict in the issue\|verdict goes in the issue' "$AGENT"; then
|
||||||
|
ok 'the definition states the verdict goes in the issue'
|
||||||
|
else
|
||||||
|
no 'the definition does not say where a pre-push verdict is recorded'
|
||||||
|
fi
|
||||||
|
|
||||||
|
# In pre-push mode there is nothing to merge and no PR to close. An agent that
|
||||||
|
# reaches for pr_merge there would be acting on a PR that does not exist.
|
||||||
|
#
|
||||||
|
# SCOPED to the pre-push section on purpose. An unscoped grep for "do not merge"
|
||||||
|
# passes on line 202 of the current file, which is the ESCALATION step talking
|
||||||
|
# about a PR — a pass for entirely the wrong reason, and the kind of vacuous
|
||||||
|
# assertion rule 9 is about. Caught by checking what the green was matching.
|
||||||
|
PREPUSH=$(awk '/[Pp]re-push/{f=1} f&&/^## [^#]/&&!/[Pp]re-push/{f=0} f' "$AGENT")
|
||||||
|
if printf '%s' "$PREPUSH" | grep -qi 'nothing to merge\|do not merge\|no PR to merge'; then
|
||||||
|
ok 'the pre-push section itself rules out merging'
|
||||||
|
else
|
||||||
|
no 'the pre-push section does not rule out merging (an unscoped grep would pass here wrongly)'
|
||||||
|
fi
|
||||||
|
|
||||||
|
# CI cannot have run on an unpushed diff, so the CI step's meaning changes. A
|
||||||
|
# definition that still treats "CI green" as available would grade on a signal
|
||||||
|
# that does not exist yet.
|
||||||
|
if grep -qi 'CI has not run\|no CI run yet\|CI cannot have run' "$AGENT"; then
|
||||||
|
ok 'the definition acknowledges CI has not run on an unpushed diff'
|
||||||
|
else
|
||||||
|
no 'the definition does not address CI being absent pre-push'
|
||||||
|
fi
|
||||||
|
|
||||||
|
rm -rf "$WORK"
|
||||||
|
printf '\n%s passed, %s failed\n' "$pass" "$fail"
|
||||||
|
[ "$fail" -eq 0 ]
|
||||||
@@ -0,0 +1,70 @@
|
|||||||
|
#!/bin/sh
|
||||||
|
# `task check` must survive a check that reports "could not verify".
|
||||||
|
#
|
||||||
|
# The regression these exist for: bin/check-mcp-env exits 2 to mean "I could
|
||||||
|
# not run — no jq, or no env-file", which the gate deliberately treats as a
|
||||||
|
# reported NOTE rather than a failure. The Taskfile said exactly that:
|
||||||
|
#
|
||||||
|
# "$DIR/check-mcp-env" "$DIR/.mcp.json" "$HOME/.op-env"
|
||||||
|
# case $? in 0) ;; 2) echo "NOTE: ..." ;; *) fail=1 ;; esac
|
||||||
|
#
|
||||||
|
# and it did not work. Task runs each cmds block under `sh -e`, so a bare
|
||||||
|
# command exiting non-zero aborts the shell *before* `case $?` is ever
|
||||||
|
# reached. The whole gate died with `exit status 2`. Locally the check found
|
||||||
|
# jq and exited 0, so it went green; CI has neither jq nor ~/.op-env, so main
|
||||||
|
# went red on the very commit that added it (skills run #57, 2026-09-07).
|
||||||
|
#
|
||||||
|
# A tolerated exit code that is only tolerated on the host where it never
|
||||||
|
# fires is not tolerated at all. This pins the tolerance itself.
|
||||||
|
#
|
||||||
|
# No mocking: pointing HOME at an empty directory removes ~/.op-env, which is
|
||||||
|
# exactly the condition CI runs under, so check-mcp-env genuinely exits 2.
|
||||||
|
|
||||||
|
set -u
|
||||||
|
|
||||||
|
ROOT=$(CDPATH= cd -- "$(dirname -- "$0")/.." && pwd)
|
||||||
|
|
||||||
|
pass=0
|
||||||
|
fail=0
|
||||||
|
|
||||||
|
ok() { pass=$((pass + 1)); printf ' ok %s\n' "$1"; }
|
||||||
|
no() { fail=$((fail + 1)); printf ' FAIL %s\n' "$1"; }
|
||||||
|
|
||||||
|
if ! command -v task >/dev/null 2>&1; then
|
||||||
|
printf ' SKIP: task not installed -- cannot exercise the gate\n'
|
||||||
|
exit 0
|
||||||
|
fi
|
||||||
|
|
||||||
|
TMP=$(mktemp -d)
|
||||||
|
trap 'rm -rf "$TMP"' EXIT
|
||||||
|
|
||||||
|
# Sanity: the precondition must actually hold, or the test proves nothing.
|
||||||
|
# An empty HOME means no ~/.op-env, so check-mcp-env cannot verify anything.
|
||||||
|
"$ROOT/bin/check-mcp-env" "$ROOT/.mcp.json" "$TMP/.op-env" >/dev/null 2>&1
|
||||||
|
rc=$?
|
||||||
|
if [ "$rc" -eq 2 ]; then
|
||||||
|
ok 'precondition: check-mcp-env exits 2 when the env-file is absent'
|
||||||
|
else
|
||||||
|
no "precondition: expected check-mcp-env to exit 2, got $rc"
|
||||||
|
fi
|
||||||
|
|
||||||
|
# The assertion. `task check` must still pass, and must say why it could not
|
||||||
|
# verify -- silence would mean the check was skipped rather than tolerated.
|
||||||
|
out=$(HOME="$TMP" task --dir "$ROOT" check 2>&1)
|
||||||
|
rc=$?
|
||||||
|
|
||||||
|
if [ "$rc" -eq 0 ]; then
|
||||||
|
ok 'task check passes when MCP env wiring cannot be verified'
|
||||||
|
else
|
||||||
|
no "task check exited $rc when check-mcp-env reported unverifiable"
|
||||||
|
printf '%s\n' "$out" | sed 's/^/ | /' | tail -8
|
||||||
|
fi
|
||||||
|
|
||||||
|
if printf '%s' "$out" | grep -q 'unverified'; then
|
||||||
|
ok 'the unverifiable check is reported, not silently skipped'
|
||||||
|
else
|
||||||
|
no 'no NOTE reported -- an unverifiable check must never look like a pass'
|
||||||
|
fi
|
||||||
|
|
||||||
|
printf ' %d passed, %d failed\n' "$pass" "$fail"
|
||||||
|
[ "$fail" -eq 0 ]
|
||||||
@@ -53,5 +53,31 @@ printf '%s' "$tag_job" | grep -qE '^ +needs: *gate' \
|
|||||||
&& ok 'ungated release.yml removed' \
|
&& ok 'ungated release.yml removed' \
|
||||||
|| no 'ungated release.yml removed'
|
|| no 'ungated release.yml removed'
|
||||||
|
|
||||||
|
# skills#46. The manifest-bump commit is created and pushed by the `tag` job
|
||||||
|
# using secrets.GITEA_TOKEN, and a push authenticated with the Actions token
|
||||||
|
# does not trigger workflows -- deliberate platform behaviour to stop CI
|
||||||
|
# recursing. So the ONE commit in this repo that no CI run has ever seen is the
|
||||||
|
# one the release job writes, every release:
|
||||||
|
#
|
||||||
|
# 6cc224f chore(release): plugin.json version 0.19.4 -> 0 runs
|
||||||
|
# f031c2b chore(release): plugin.json version 0.19.5 -> 0 runs
|
||||||
|
#
|
||||||
|
# The koala cluster's CIHeadUncovered{repo=skills} alert had been firing on it
|
||||||
|
# for 85 hours when this was found, and it cannot clear on its own -- only an
|
||||||
|
# unrelated human push moves main's HEAD to a tested commit.
|
||||||
|
#
|
||||||
|
# The commit is not nothing: plugin.json's version is what Claude Code gates
|
||||||
|
# plugin updates on, `task check` asserts it is not behind the latest tag, and
|
||||||
|
# the sed-and-retry logic that writes it already shipped a mismatched manifest
|
||||||
|
# once (v0.10.0 with the manifest reading 0.9.0).
|
||||||
|
#
|
||||||
|
# Anchored past comments: the tag job's prose already SAYS "`task check`
|
||||||
|
# asserts the manifest is not behind the latest tag", and a bare grep matches
|
||||||
|
# that and passes. A grep for a concept matches the rationale, and the more
|
||||||
|
# carefully a thing is documented the more likely that is.
|
||||||
|
printf '%s' "$tag_job" | grep -qE '^[^#]*task check' \
|
||||||
|
&& ok 'the release-bump commit is gated before it is pushed' \
|
||||||
|
|| no 'the release-bump commit is pushed with no gate -- nothing ever tests it (#46)'
|
||||||
|
|
||||||
printf '\nci_gate: %d passed, %d failed\n' "$pass" "$fail"
|
printf '\nci_gate: %d passed, %d failed\n' "$pass" "$fail"
|
||||||
[ "$fail" -eq 0 ]
|
[ "$fail" -eq 0 ]
|
||||||
|
|||||||
@@ -24,6 +24,18 @@ if [ ! -f "$DOCTOR" ]; then
|
|||||||
exit 1
|
exit 1
|
||||||
fi
|
fi
|
||||||
|
|
||||||
|
# The toolchain check (golangci-lint build-Go vs GOROOT) reads the real host
|
||||||
|
# unless we say otherwise, which made every unrelated fixture fail on a machine
|
||||||
|
# whose golangci-lint is genuinely stale. Give the whole suite a matching fake
|
||||||
|
# pair so only the cases that OPT IN exercise that check.
|
||||||
|
STUB_BIN="$WORK/_stub_bin"
|
||||||
|
mkdir -p "$STUB_BIN"
|
||||||
|
printf '#!/bin/sh\nprintf "golangci-lint has version 2.0.0 built with go1.27.0 from abc\\n"\n' > "$STUB_BIN/golangci-lint"
|
||||||
|
printf '#!/bin/sh\ncase "$1" in version) printf "go version go1.27.0 linux/amd64\\n";; *) printf "\\n";; esac\n' > "$STUB_BIN/go"
|
||||||
|
chmod +x "$STUB_BIN/golangci-lint" "$STUB_BIN/go"
|
||||||
|
PATH="$STUB_BIN:$PATH"
|
||||||
|
export PATH
|
||||||
|
|
||||||
cleanup() { rm -rf "$WORK"; }
|
cleanup() { rm -rf "$WORK"; }
|
||||||
trap cleanup EXIT INT TERM
|
trap cleanup EXIT INT TERM
|
||||||
|
|
||||||
@@ -291,5 +303,75 @@ else
|
|||||||
fail=$((fail + 1)); printf ' FAIL --online reports an unreachable remote as its own fault\n rc=%s got: %s\n' "$_rc" "$_out"
|
fail=$((fail + 1)); printf ' FAIL --online reports an unreachable remote as its own fault\n rc=%s got: %s\n' "$_rc" "$_out"
|
||||||
fi
|
fi
|
||||||
|
|
||||||
|
# --- toolchain: golangci-lint must be built with Go >= GOROOT ---------------
|
||||||
|
# Measured on koala 2026-09-08 (infra#375, second recurrence): golangci-lint
|
||||||
|
# 2.11.4 built with go1.26.1 against a go1.27.0 GOROOT PANICS on any Go repo --
|
||||||
|
#
|
||||||
|
# panic: file requires newer Go version go1.27 (application built with go1.26)
|
||||||
|
#
|
||||||
|
# and the earlier form of the same fault printed a typecheck error naming YOUR
|
||||||
|
# file, so the natural response is to debug code that is fine. It cost time in
|
||||||
|
# two repos. CI never sees it (the runner image matches), so it breaks only the
|
||||||
|
# local `task check` that AGENT.md requires before every push.
|
||||||
|
#
|
||||||
|
# The fixtures inject fake `go` and `golangci-lint` on PATH; nothing here reads
|
||||||
|
# the real host toolchain, so the suite still passes on a machine without Go.
|
||||||
|
mk_toolchain() { # $1=fixture $2=lint-build-go $3=goroot-go ("" = absent)
|
||||||
|
_b="$1/bin"; mkdir -p "$_b"
|
||||||
|
if [ -n "$2" ]; then
|
||||||
|
printf '#!/bin/sh\nprintf "golangci-lint has version 2.11.4 built with %s from abc on 2026-01-01\\n"\n' "$2" > "$_b/golangci-lint"
|
||||||
|
chmod +x "$_b/golangci-lint"
|
||||||
|
fi
|
||||||
|
if [ -n "$3" ]; then
|
||||||
|
printf '#!/bin/sh\ncase "$1" in version) printf "go version %s linux/amd64\\n";; *) printf "\\n";; esac\n' "$3" > "$_b/go"
|
||||||
|
chmod +x "$_b/go"
|
||||||
|
fi
|
||||||
|
printf '%s' "$_b"
|
||||||
|
}
|
||||||
|
|
||||||
|
f=$(new_fixture golangci_old); wire_clean "$f"
|
||||||
|
b=$(mk_toolchain "$f" go1.26.1 go1.27.0)
|
||||||
|
_out=$(PATH="$b:$PATH" HOME="$f/home" DOCTOR_CHECKOUT="$f/ck" "$DOCTOR" 2>&1); _rc=$?
|
||||||
|
if [ "$_rc" -ne 0 ] && printf '%s' "$_out" | grep -qi 'golangci'; then
|
||||||
|
pass=$((pass + 1)); printf ' ok golangci-lint older than GOROOT is reported\n'
|
||||||
|
else
|
||||||
|
fail=$((fail + 1)); printf ' FAIL golangci-lint older than GOROOT is reported\n rc=%s got: %s\n' "$_rc" "$_out"
|
||||||
|
fi
|
||||||
|
|
||||||
|
# Matching versions are the normal case and must stay silent.
|
||||||
|
f=$(new_fixture golangci_ok); wire_clean "$f"
|
||||||
|
b=$(mk_toolchain "$f" go1.27.0 go1.27.0)
|
||||||
|
_out=$(PATH="$b:$PATH" HOME="$f/home" DOCTOR_CHECKOUT="$f/ck" "$DOCTOR" 2>&1); _rc=$?
|
||||||
|
if [ "$_rc" -eq 0 ]; then
|
||||||
|
pass=$((pass + 1)); printf ' ok golangci-lint matching GOROOT is silent\n'
|
||||||
|
else
|
||||||
|
fail=$((fail + 1)); printf ' FAIL golangci-lint matching GOROOT is silent\n rc=%s got: %s\n' "$_rc" "$_out"
|
||||||
|
fi
|
||||||
|
|
||||||
|
# A NEWER lint than GOROOT is fine -- the constraint is one-directional.
|
||||||
|
f=$(new_fixture golangci_newer); wire_clean "$f"
|
||||||
|
b=$(mk_toolchain "$f" go1.28.0 go1.27.0)
|
||||||
|
_out=$(PATH="$b:$PATH" HOME="$f/home" DOCTOR_CHECKOUT="$f/ck" "$DOCTOR" 2>&1); _rc=$?
|
||||||
|
if [ "$_rc" -eq 0 ]; then
|
||||||
|
pass=$((pass + 1)); printf ' ok golangci-lint newer than GOROOT is not a fault\n'
|
||||||
|
else
|
||||||
|
fail=$((fail + 1)); printf ' FAIL golangci-lint newer than GOROOT is not a fault\n rc=%s got: %s\n' "$_rc" "$_out"
|
||||||
|
fi
|
||||||
|
|
||||||
|
# Fail OPEN on anything unparseable. doctor runs in a SessionStart hook on every
|
||||||
|
# machine; a toolchain it cannot read is not a fault it may assert. (A host with
|
||||||
|
# no Go at all takes the same path -- command -v simply misses.)
|
||||||
|
f=$(new_fixture golangci_garbled); wire_clean "$f"
|
||||||
|
_b="$f/bin"; mkdir -p "$_b"
|
||||||
|
printf '#!/bin/sh\nprintf "golangci-lint dev build\\n"\n' > "$_b/golangci-lint"
|
||||||
|
printf '#!/bin/sh\nprintf "go version devel\\n"\n' > "$_b/go"
|
||||||
|
chmod +x "$_b/golangci-lint" "$_b/go"
|
||||||
|
_out=$(PATH="$_b:$PATH" HOME="$f/home" DOCTOR_CHECKOUT="$f/ck" "$DOCTOR" 2>&1); _rc=$?
|
||||||
|
if [ "$_rc" -eq 0 ]; then
|
||||||
|
pass=$((pass + 1)); printf ' ok an unreadable toolchain version is not asserted as a fault\n'
|
||||||
|
else
|
||||||
|
fail=$((fail + 1)); printf ' FAIL an unreadable toolchain version is not asserted as a fault\n rc=%s got: %s\n' "$_rc" "$_out"
|
||||||
|
fi
|
||||||
|
|
||||||
printf '\ndoctor: %d passed, %d failed\n' "$pass" "$fail"
|
printf '\ndoctor: %d passed, %d failed\n' "$pass" "$fail"
|
||||||
[ "$fail" -eq 0 ]
|
[ "$fail" -eq 0 ]
|
||||||
|
|||||||
Executable
+118
@@ -0,0 +1,118 @@
|
|||||||
|
#!/bin/sh
|
||||||
|
# Tests for bin/check-mcp-env.
|
||||||
|
#
|
||||||
|
# The fault this gate exists for is silent, which is why it needs a gate at
|
||||||
|
# all. An `.mcp.json` server whose Authorization header interpolates a
|
||||||
|
# variable nothing in the session environment defines does not error: the
|
||||||
|
# header goes out empty, the server answers 401, and Claude Code reports
|
||||||
|
# "requires authentication". That is indistinguishable from a revoked or
|
||||||
|
# expired credential, and on 2026-09-07 it cost a full investigation on koala
|
||||||
|
# to establish that the credential was fine and the variable was simply never
|
||||||
|
# set.
|
||||||
|
#
|
||||||
|
# So the property is: every ${VAR} a LIVE server interpolates must be a
|
||||||
|
# variable the session env-file actually defines.
|
||||||
|
#
|
||||||
|
# The `_parked` block is deliberately exempt. Parked servers reference
|
||||||
|
# variables nothing provides on purpose -- that is what parked means -- and a
|
||||||
|
# gate that flagged them would be reporting the intended state as a fault.
|
||||||
|
#
|
||||||
|
# Fixtures drive the checker through argv, so no test touches the real host.
|
||||||
|
|
||||||
|
set -u
|
||||||
|
|
||||||
|
ROOT=$(CDPATH= cd -- "$(dirname -- "$0")/.." && pwd)
|
||||||
|
CHECK="$ROOT/bin/check-mcp-env"
|
||||||
|
|
||||||
|
pass=0
|
||||||
|
fail=0
|
||||||
|
WORK=${TMPDIR:-/tmp}/mcp_config_test.$$
|
||||||
|
|
||||||
|
if [ ! -f "$CHECK" ]; then
|
||||||
|
printf 'FAIL: %s does not exist\n' "$CHECK"
|
||||||
|
exit 1
|
||||||
|
fi
|
||||||
|
|
||||||
|
cleanup() { rm -rf "$WORK"; }
|
||||||
|
trap cleanup EXIT INT TERM
|
||||||
|
mkdir -p "$WORK"
|
||||||
|
|
||||||
|
if ! command -v jq >/dev/null 2>&1; then
|
||||||
|
printf 'SKIP: jq not installed -- bin/check-mcp-env cannot be exercised\n'
|
||||||
|
exit 0
|
||||||
|
fi
|
||||||
|
|
||||||
|
# ok <name> / bad <name> <detail>
|
||||||
|
ok() { pass=$((pass+1)); printf ' ok %s\n' "$1"; }
|
||||||
|
bad() { fail=$((fail+1)); printf ' FAIL %s %s\n' "$1" "${2:-}"; }
|
||||||
|
|
||||||
|
# run <mcp-json> <env-file> -> sets $rc and $out
|
||||||
|
run() {
|
||||||
|
out=$("$CHECK" "$1" "$2" 2>&1); rc=$?
|
||||||
|
}
|
||||||
|
|
||||||
|
ENVFILE="$WORK/op-env"
|
||||||
|
cat > "$ENVFILE" <<'ENVEOF'
|
||||||
|
BRAIN_MCP_TOKEN=op://HomeLab/brain/token
|
||||||
|
DMABE_GITEA_API_TOKEN=op://HomeLab/gitea/token
|
||||||
|
ENVEOF
|
||||||
|
|
||||||
|
# --- a live server whose variable IS defined ---------------------------------
|
||||||
|
cat > "$WORK/good.json" <<'JSONEOF'
|
||||||
|
{"mcpServers":{"brain":{"type":"http","url":"https://x/mcp",
|
||||||
|
"headers":{"Authorization":"Bearer ${BRAIN_MCP_TOKEN}"}}}}
|
||||||
|
JSONEOF
|
||||||
|
run "$WORK/good.json" "$ENVFILE"
|
||||||
|
[ "$rc" -eq 0 ] && ok "defined variable passes" || bad "defined variable passes" "rc=$rc $out"
|
||||||
|
|
||||||
|
# --- a live server whose variable is NOT defined ------------------------------
|
||||||
|
# The exact shape that broke: the server looks correct and fails at runtime.
|
||||||
|
cat > "$WORK/missing.json" <<'JSONEOF'
|
||||||
|
{"mcpServers":{"gitea":{"type":"http","url":"https://x/mcp",
|
||||||
|
"headers":{"Authorization":"Bearer ${GITEA_MCP_STATIC_TOKEN}"}}}}
|
||||||
|
JSONEOF
|
||||||
|
run "$WORK/missing.json" "$ENVFILE"
|
||||||
|
if [ "$rc" -ne 0 ]; then
|
||||||
|
case "$out" in
|
||||||
|
*gitea*GITEA_MCP_STATIC_TOKEN*) ok "undefined variable is flagged, naming server and variable" ;;
|
||||||
|
*) bad "undefined variable is flagged, naming server and variable" "report did not name both: $out" ;;
|
||||||
|
esac
|
||||||
|
else
|
||||||
|
bad "undefined variable is flagged, naming server and variable" "rc=0"
|
||||||
|
fi
|
||||||
|
|
||||||
|
# --- a server with no headers at all (the `infra` shape) ----------------------
|
||||||
|
# Unauthenticated by design. Nothing to resolve, so nothing to flag.
|
||||||
|
cat > "$WORK/noauth.json" <<'JSONEOF'
|
||||||
|
{"mcpServers":{"infra":{"type":"http","url":"https://x/mcp"}}}
|
||||||
|
JSONEOF
|
||||||
|
run "$WORK/noauth.json" "$ENVFILE"
|
||||||
|
[ "$rc" -eq 0 ] && ok "header-less server is not flagged" || bad "header-less server is not flagged" "rc=$rc $out"
|
||||||
|
|
||||||
|
# --- parked servers are exempt ------------------------------------------------
|
||||||
|
cat > "$WORK/parked.json" <<'JSONEOF'
|
||||||
|
{"mcpServers":{"brain":{"type":"http","url":"https://x/mcp",
|
||||||
|
"headers":{"Authorization":"Bearer ${BRAIN_MCP_TOKEN}"}}},
|
||||||
|
"_parked":{"_reason":"no identity provides these yet",
|
||||||
|
"cad":{"type":"http","url":"https://x/mcp",
|
||||||
|
"headers":{"Authorization":"Bearer ${CAD_MCP_TOKEN}"}}}}
|
||||||
|
JSONEOF
|
||||||
|
run "$WORK/parked.json" "$ENVFILE"
|
||||||
|
if [ "$rc" -eq 0 ]; then
|
||||||
|
ok "parked server is exempt"
|
||||||
|
else
|
||||||
|
bad "parked server is exempt" "rc=$rc $out"
|
||||||
|
fi
|
||||||
|
|
||||||
|
# --- malformed JSON must not pass silently ------------------------------------
|
||||||
|
# A config the checker cannot read is an unverified config, not a clean one.
|
||||||
|
printf '{"mcpServers":{' > "$WORK/broken.json"
|
||||||
|
run "$WORK/broken.json" "$ENVFILE"
|
||||||
|
[ "$rc" -ne 0 ] && ok "unparseable config does not report success" || bad "unparseable config does not report success" "rc=0"
|
||||||
|
|
||||||
|
# --- a missing env-file is unverifiable, not clean ----------------------------
|
||||||
|
run "$WORK/good.json" "$WORK/no-such-env"
|
||||||
|
[ "$rc" -ne 0 ] && ok "missing env-file does not report success" || bad "missing env-file does not report success" "rc=0"
|
||||||
|
|
||||||
|
printf '\n%s: %d passed, %d failed\n' "$(basename "$0")" "$pass" "$fail"
|
||||||
|
[ "$fail" -eq 0 ]
|
||||||
Executable
+118
@@ -0,0 +1,118 @@
|
|||||||
|
#!/bin/sh
|
||||||
|
# Tests for hooks/mcp-destructive-guard.sh — the PreToolUse gate that makes a
|
||||||
|
# human approve irreversible MCP verbs (infra#508, infra ADR-0027).
|
||||||
|
#
|
||||||
|
# Contract: reads PreToolUse hook JSON on stdin, ALWAYS exits 0, and answers
|
||||||
|
# in hookSpecificOutput.permissionDecision. "ask" = a human must approve;
|
||||||
|
# empty stdout ("NONE") = no opinion, normal permission rules apply. The guard
|
||||||
|
# never emits "allow": an allow from a hook would bypass the user's own rules.
|
||||||
|
#
|
||||||
|
# Cases marked (gatekeeper) are escapes found by the skills#52 review: each
|
||||||
|
# passed the first version of this suite.
|
||||||
|
|
||||||
|
set -u
|
||||||
|
|
||||||
|
ROOT=$(CDPATH= cd -- "$(dirname -- "$0")/.." && pwd)
|
||||||
|
GUARD="$ROOT/hooks/mcp-destructive-guard.sh"
|
||||||
|
|
||||||
|
pass=0
|
||||||
|
fail=0
|
||||||
|
|
||||||
|
# run_guard <json> [PATH] — invoked exactly as hooks.json does: `bash <path>`.
|
||||||
|
# Prints "<decision> rc=<exit>", decision NONE for an empty answer.
|
||||||
|
run_guard() {
|
||||||
|
if [ $# -ge 2 ]; then
|
||||||
|
_out=$(printf '%s' "$1" | PATH="$2" "$BASH_BIN" "$GUARD"); _rc=$?
|
||||||
|
else
|
||||||
|
_out=$(printf '%s' "$1" | bash "$GUARD"); _rc=$?
|
||||||
|
fi
|
||||||
|
if [ -z "$_out" ]; then _d=NONE
|
||||||
|
else _d=$(printf '%s' "$_out" | jq -r '.hookSpecificOutput.permissionDecision // "NONE"' 2>/dev/null || echo BROKEN)
|
||||||
|
fi
|
||||||
|
printf '%s rc=%s' "$_d" "$_rc"
|
||||||
|
}
|
||||||
|
BASH_BIN=$(command -v bash)
|
||||||
|
|
||||||
|
check() {
|
||||||
|
_want=$1 _json=$2 _label=$3
|
||||||
|
_got=$(run_guard "$_json")
|
||||||
|
if [ "$_got" = "$_want rc=0" ]; then
|
||||||
|
pass=$((pass + 1)); printf ' ok %s\n' "$_label"
|
||||||
|
else
|
||||||
|
fail=$((fail + 1)); printf ' FAIL %s\n want=%s rc=0 got=%s\n in=%s\n' "$_label" "$_want" "$_got" "$_json"
|
||||||
|
fi
|
||||||
|
}
|
||||||
|
tool() { printf '{"tool_name":"%s","tool_input":{"owner":"mathias","repo":"x"}}' "$1"; }
|
||||||
|
|
||||||
|
echo "mcp-destructive-guard tests"
|
||||||
|
|
||||||
|
for v in repo_delete file_delete branch_delete pr_merge tbd_ship repo_update repo_mirror_push \
|
||||||
|
workflow_run_trigger tag_create release_create; do
|
||||||
|
check ask "$(tool "mcp__plugin_skills_gitea__$v")" "plugin gitea $v asks"
|
||||||
|
done
|
||||||
|
|
||||||
|
# The verb is matched on any server name: connector, bare, or renamed.
|
||||||
|
check ask "$(tool mcp__claude_ai_gitea__repo_delete)" "connector namespace asks"
|
||||||
|
check ask "$(tool mcp__gitea__repo_delete)" "bare server name asks"
|
||||||
|
check ask "$(tool mcp__git__repo_delete)" "renamed server still asks"
|
||||||
|
check ask "$(tool mcp__plugin_skills_gitea__REPO_DELETE)" "uppercase verb asks"
|
||||||
|
# The verb is the LAST segment (gatekeeper: parsing the second segment stayed green).
|
||||||
|
check ask "$(tool mcp__a__b__repo_delete)" "verb taken from last segment"
|
||||||
|
check NONE "$(tool mcp__repo_delete__issue_get)" "server named like a verb does not trigger"
|
||||||
|
|
||||||
|
# file_write_branch: content landing on main, or emptying a file, is the same
|
||||||
|
# effect as the gated pr_merge / file_delete (gatekeeper).
|
||||||
|
fwb() { printf '{"tool_name":"mcp__plugin_skills_gitea__file_write_branch","tool_input":{"branch":"%s","content":"%s"}}' "$1" "$2"; }
|
||||||
|
check ask "$(fwb main x)" "file_write_branch to main asks"
|
||||||
|
check ask "$(fwb master x)" "file_write_branch to master asks"
|
||||||
|
check ask "$(fwb feat/y '')" "file_write_branch emptying a file asks"
|
||||||
|
check ask '{"tool_name":"mcp__plugin_skills_gitea__file_write_branch","tool_input":{"content":"x"}}' "file_write_branch without branch asks"
|
||||||
|
check NONE "$(fwb feat/y x)" "file_write_branch to a feature branch passes"
|
||||||
|
|
||||||
|
# Go's encoding/json matches keys case-insensitively, last one wins: gitea-mcp
|
||||||
|
# wrote to main from {"branch":"feat/y8","Branch":"main"} while the hook read
|
||||||
|
# only `branch` (gatekeeper round 2, verified against a real Gitea). So the
|
||||||
|
# guard clears a write only when exactly one key case-folds to branch/content.
|
||||||
|
FWB=mcp__plugin_skills_gitea__file_write_branch
|
||||||
|
check ask "{\"tool_name\":\"$FWB\",\"tool_input\":{\"branch\":\"feat/y8\",\"Branch\":\"main\",\"content\":\"x\"}}" "case-variant Branch key asks"
|
||||||
|
check ask "{\"tool_name\":\"$FWB\",\"tool_input\":{\"branch\":\"feat/y\",\"content\":\"x\",\"Content\":\"\"}}" "case-variant Content key asks"
|
||||||
|
check ask "{\"tool_name\":\"$FWB\",\"tool_input\":{\"BRANCH\":\"feat/y\",\"content\":\"x\"}}" "only an uppercase BRANCH key asks"
|
||||||
|
check ask "$(fwb Main x)" "file_write_branch to Main asks"
|
||||||
|
check ask "{\"tool_name\":\"$FWB\",\"tool_input\":{\"branch\":\"feat/y\"}}" "missing content key asks"
|
||||||
|
check ask "$(fwb feat/y ' ')" "whitespace-only content asks"
|
||||||
|
check ask "{\"tool_name\":\"$FWB\",\"tool_input\":\"branch=feat/y\"}" "non-object tool_input asks"
|
||||||
|
# Branch names are cleared by shape, not by blocklist: refs/heads/main created a
|
||||||
|
# second ref that wedged every API write to main (gatekeeper side finding).
|
||||||
|
check ask "$(fwb refs/heads/main x)" "refs/ prefixed branch asks"
|
||||||
|
check ask "$(fwb ' main' x)" "branch with whitespace asks"
|
||||||
|
check ask "$(fwb '../main' x)" "branch with .. asks"
|
||||||
|
check NONE "$(fwb release/v1.2_x x)" "ordinary branch name passes"
|
||||||
|
|
||||||
|
# Reversible / read verbs pass through.
|
||||||
|
for v in issue_close issue_create file_read repo_list pr_create; do
|
||||||
|
check NONE "$(tool "mcp__plugin_skills_gitea__$v")" "$v passes through"
|
||||||
|
done
|
||||||
|
check NONE "$(tool mcp__plugin_skills_gitea__repo_delete_dry_run_list)" "suffix near-miss passes"
|
||||||
|
check NONE "$(tool Bash)" "non-MCP tool passes"
|
||||||
|
check NONE "$(tool repo_delete)" "non-MCP tool named like a verb passes"
|
||||||
|
|
||||||
|
# Malformed input fails CLOSED (gatekeeper: non-string tool_name got NONE).
|
||||||
|
check ask 'not json' "non-JSON asks"
|
||||||
|
check ask '{}' "missing tool_name asks"
|
||||||
|
check ask '{"tool_name":null}' "null tool_name asks"
|
||||||
|
check ask '{"tool_name":""}' "empty tool_name asks"
|
||||||
|
check ask '{"tool_name":5}' "number tool_name asks"
|
||||||
|
check ask '{"tool_name":["mcp__x__repo_delete"]}' "array tool_name asks"
|
||||||
|
check ask '{"tool_name":{"a":1}}' "object tool_name asks"
|
||||||
|
|
||||||
|
# Without jq on PATH the guard must still answer ask, not stay silent
|
||||||
|
# (gatekeeper: the fail-closed path itself needed jq).
|
||||||
|
nojq=$(mktemp -d); trap 'rm -rf "$nojq"' EXIT
|
||||||
|
for b in bash cat grep tr sed; do ln -s "$(command -v $b)" "$nojq/$b"; done
|
||||||
|
_got=$(run_guard "$(tool mcp__plugin_skills_gitea__repo_delete)" "$nojq")
|
||||||
|
if [ "$_got" = "ask rc=0" ]; then pass=$((pass+1)); echo " ok no jq on PATH still asks"; else fail=$((fail+1)); echo " FAIL no jq on PATH: got=$_got"; fi
|
||||||
|
_got=$(run_guard "$(tool mcp__plugin_skills_gitea__issue_get)" "$nojq")
|
||||||
|
if [ "$_got" = "ask rc=0" ]; then pass=$((pass+1)); echo " ok no jq on PATH asks even for a read verb (cannot parse = cannot clear)"; else fail=$((fail+1)); echo " FAIL no jq read verb: got=$_got"; fi
|
||||||
|
|
||||||
|
printf '\n%d passed, %d failed\n' "$pass" "$fail"
|
||||||
|
[ "$fail" -eq 0 ]
|
||||||
@@ -0,0 +1,103 @@
|
|||||||
|
#!/bin/sh
|
||||||
|
# Tests for hooks/pre-task-ritual.sh — the UserPromptSubmit hook that surfaces
|
||||||
|
# rule 0 on every prompt (#44).
|
||||||
|
#
|
||||||
|
# WHY THIS EXISTS, AND WHAT IT DELIBERATELY DOES NOT CLAIM
|
||||||
|
# This hook is three lines and echoes a string. It has no bash-isms, so it
|
||||||
|
# cannot fail the way secret-guard appeared to; that risk was overstated when
|
||||||
|
# this test was requested. What it CAN do is stop working silently, and its
|
||||||
|
# whole job is to be visible -- if the text stops appearing, rule 0 stops being
|
||||||
|
# surfaced on every prompt and nothing announces that.
|
||||||
|
#
|
||||||
|
# THE ASSERTION THAT MATTERS IS REGISTRATION.
|
||||||
|
# A hook file that exists but is not in hooks.json does absolutely nothing, and
|
||||||
|
# looks identical on disk to one that works. So this derives the command from
|
||||||
|
# hooks.json rather than assuming one: the registration is the source of truth
|
||||||
|
# for how the hook is invoked, and a test that invokes it its own way is
|
||||||
|
# testing something the harness never runs.
|
||||||
|
#
|
||||||
|
# That distinction is not hypothetical. secret_guard_test.sh piped its hook into
|
||||||
|
# `sh` while hooks.json runs it with `bash`; on Debian /bin/sh is dash, which
|
||||||
|
# has no `set -o pipefail`, and the suite failed 20/20 in CI against a hook that
|
||||||
|
# was fine in production. The test was wrong, not the hook. Deriving the
|
||||||
|
# invocation removes the opportunity for that class of mistake entirely.
|
||||||
|
|
||||||
|
set -u
|
||||||
|
|
||||||
|
ROOT=$(CDPATH= cd -- "$(dirname -- "$0")/.." && pwd)
|
||||||
|
HOOK="$ROOT/hooks/pre-task-ritual.sh"
|
||||||
|
REG="$ROOT/hooks/hooks.json"
|
||||||
|
|
||||||
|
pass=0
|
||||||
|
fail=0
|
||||||
|
ok() { pass=$((pass + 1)); printf ' ok %s\n' "$1"; }
|
||||||
|
no() { fail=$((fail + 1)); printf ' FAIL %s\n' "$1"; }
|
||||||
|
|
||||||
|
if ! command -v jq >/dev/null 2>&1; then
|
||||||
|
printf ' SKIP: jq not installed -- cannot read the hook registration\n'
|
||||||
|
exit 0
|
||||||
|
fi
|
||||||
|
|
||||||
|
[ -f "$HOOK" ] || { printf ' FAIL %s does not exist\n' "$HOOK"; exit 1; }
|
||||||
|
ok 'hook file exists'
|
||||||
|
|
||||||
|
[ -f "$REG" ] || { printf ' FAIL %s does not exist\n' "$REG"; exit 1; }
|
||||||
|
|
||||||
|
# 1. Registered, and on the right event. UserPromptSubmit is what makes it fire
|
||||||
|
# on every prompt; registered under any other event it would still "exist"
|
||||||
|
# and still do nothing useful.
|
||||||
|
CMD=$(jq -r '
|
||||||
|
.hooks.UserPromptSubmit[]?.hooks[]?
|
||||||
|
| select(.command | test("pre-task-ritual"))
|
||||||
|
| .command
|
||||||
|
' "$REG" 2>/dev/null | head -1)
|
||||||
|
|
||||||
|
if [ -n "$CMD" ]; then
|
||||||
|
ok 'registered on UserPromptSubmit in hooks.json'
|
||||||
|
else
|
||||||
|
no 'not registered on UserPromptSubmit -- the file would exist and never run'
|
||||||
|
printf ' %d passed, %d failed\n' "$pass" "$fail"
|
||||||
|
exit 1
|
||||||
|
fi
|
||||||
|
|
||||||
|
# 2. Run it EXACTLY as registered, with the harness variable substituted the way
|
||||||
|
# the harness substitutes it.
|
||||||
|
OUT=$(CLAUDE_PLUGIN_ROOT="$ROOT" sh -c "$CMD" 2>&1)
|
||||||
|
rc=$?
|
||||||
|
|
||||||
|
[ "$rc" -eq 0 ] && ok 'exits 0 as registered' \
|
||||||
|
|| no "exited $rc -- a non-zero UserPromptSubmit hook can block the prompt"
|
||||||
|
|
||||||
|
# 3. The content is the point. A hook that runs and says nothing is the same as
|
||||||
|
# no hook, and that is the failure this pins.
|
||||||
|
if [ -n "$OUT" ]; then
|
||||||
|
ok 'produces output'
|
||||||
|
else
|
||||||
|
no 'produced no output -- rule 0 would silently stop being surfaced'
|
||||||
|
fi
|
||||||
|
|
||||||
|
case "$OUT" in
|
||||||
|
*"Rule 0"*) ok 'names rule 0' ;;
|
||||||
|
*) no "does not name rule 0: '$OUT'" ;;
|
||||||
|
esac
|
||||||
|
|
||||||
|
# Each of the four steps, individually. Naming rule 0 while having quietly lost
|
||||||
|
# a step is the plausible drift, not losing the whole line.
|
||||||
|
for step in brain_query skill 'failing test' 'success criterion'; do
|
||||||
|
case "$OUT" in
|
||||||
|
*"$step"*) ok "states the '$step' step" ;;
|
||||||
|
*) no "lost the '$step' step from the ritual" ;;
|
||||||
|
esac
|
||||||
|
done
|
||||||
|
|
||||||
|
# 4. The interpreter named in the registration must actually exist, or the hook
|
||||||
|
# fails on that host with a message nobody reads.
|
||||||
|
INTERP=${CMD%% *}
|
||||||
|
if command -v "$INTERP" >/dev/null 2>&1; then
|
||||||
|
ok "registered interpreter '$INTERP' is available"
|
||||||
|
else
|
||||||
|
no "registered interpreter '$INTERP' not found on this host"
|
||||||
|
fi
|
||||||
|
|
||||||
|
printf ' %d passed, %d failed\n' "$pass" "$fail"
|
||||||
|
[ "$fail" -eq 0 ]
|
||||||
@@ -24,7 +24,7 @@ decision_for() {
|
|||||||
_tool=${2:-Bash}
|
_tool=${2:-Bash}
|
||||||
jq -n --arg t "$_tool" --arg c "$_cmd" \
|
jq -n --arg t "$_tool" --arg c "$_cmd" \
|
||||||
'{tool_name: $t, tool_input: {command: $c}}' \
|
'{tool_name: $t, tool_input: {command: $c}}' \
|
||||||
| sh "$GUARD" \
|
| "$GUARD" \
|
||||||
| jq -r '.hookSpecificOutput.permissionDecision // "MISSING"'
|
| jq -r '.hookSpecificOutput.permissionDecision // "MISSING"'
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -42,6 +42,20 @@ expect() {
|
|||||||
fi
|
fi
|
||||||
}
|
}
|
||||||
|
|
||||||
|
# Invoked as a PROGRAM above, never as `sh "$GUARD"`. Claude Code executes the
|
||||||
|
# hook directly, so its `#!/usr/bin/env bash` shebang decides the interpreter;
|
||||||
|
# piping it into `sh` overrides that and tests something production never runs.
|
||||||
|
#
|
||||||
|
# It is not academic. On Debian /bin/sh is dash, which has no `set -o
|
||||||
|
# pipefail`, so the guard died on line 23 before reading a byte of input and
|
||||||
|
# every deny case returned empty -- 20 of 20 assertions failed in CI while
|
||||||
|
# passing on a host where /bin/sh happens to be bash. A security gate that
|
||||||
|
# tests green and cannot start is the worst of both.
|
||||||
|
if [ ! -x "$GUARD" ]; then
|
||||||
|
printf 'FAIL: %s is not executable -- it must run via its own shebang\n' "$GUARD"
|
||||||
|
exit 1
|
||||||
|
fi
|
||||||
|
|
||||||
if [ ! -f "$GUARD" ]; then
|
if [ ! -f "$GUARD" ]; then
|
||||||
printf 'FAIL: %s does not exist\n' "$GUARD"
|
printf 'FAIL: %s does not exist\n' "$GUARD"
|
||||||
exit 1
|
exit 1
|
||||||
|
|||||||
Reference in New Issue
Block a user