From 79c736f2ae00ef782c640402ccb9ec71215c4121 Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Fri, 31 Jul 2026 10:25:12 -0400 Subject: [PATCH 1/4] Validate cross-skill references in `lint-skill-entry` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three reference kinds could name something no reader can reach, and none were checked: - Lane ids (`B7`, `C4`) are addresses into `evidence-catalog.md`, not names. A `description` cannot link out to the catalog at all, so a lane id there is unresolvable by construction — that is an error. In the body it is a warning unless the line links the catalog. - `[[snake_case]]` wiki links come from a private authoring vault and render as literal brackets here. Matching on the underscore keeps JS array literals (`[[signer1.address, …]]`) from tripping the rule. - Names in `## Related` must resolve to a skill. A warning rather than an error: the gate runs on the PR's own branch, where a sibling skill shipping in a concurrent PR does not exist yet. `collectSkills` takes an array of roots; passing a bare string iterates its characters and yields zero skills, which would have made every check above pass vacuously. --- .github/scripts/lint-skill-entry.mjs | 81 +++++++++++++++++++++++++++- 1 file changed, 79 insertions(+), 2 deletions(-) diff --git a/.github/scripts/lint-skill-entry.mjs b/.github/scripts/lint-skill-entry.mjs index 8de11ba..c9564d4 100644 --- a/.github/scripts/lint-skill-entry.mjs +++ b/.github/scripts/lint-skill-entry.mjs @@ -9,7 +9,7 @@ // Run against the repo: node .github/scripts/lint-skill-entry.mjs // Run against another tree: SKILLS_LINT_ROOT=/path node .github/scripts/lint-skill-entry.mjs -import { readFileSync, readdirSync, statSync } from 'node:fs'; +import { existsSync, readFileSync, readdirSync, statSync } from 'node:fs'; import path from 'node:path'; import { fileURLToPath } from 'node:url'; @@ -40,8 +40,10 @@ export function lintSkill(skill) { const dirName = skill.id.slice(skill.domain.length + 1); let raw; + let source = ''; try { - raw = parseFrontmatter(readFileSync(path.join(skill.path, 'skill.md'), 'utf8')); + source = readFileSync(path.join(skill.path, 'skill.md'), 'utf8'); + raw = parseFrontmatter(source); } catch (error) { return { errors: [`could not read skill.md: ${error.message}`], warnings }; } @@ -121,9 +123,84 @@ export function lintSkill(skill) { } } + crossReferenceChecks(skill, raw, source, errors, warnings); + return { errors, warnings }; } +// Lane IDs (`B7`, `C4`) are addresses into evidence-catalog.md, not names. They carry no +// meaning to a reader who has not opened the catalog, and a `description` cannot link out +// to it — frontmatter is plain text. So: never in a description, and in the body only on a +// line that also links the catalog. The catalog's own skill is exempt: it defines them. +const LANE_ID = /(? skill.name)); + return nameCache; +} + // Restrict to skills touched by the given file paths (the CI gate passes the // PR's changed files, so pre-existing drift in untouched skills never blocks an // unrelated change). With no paths, every skill is linted (a full audit). From 43462549df2cff8d38417d4dfb5fb25d047c9ab7 Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Tue, 4 Aug 2026 06:08:07 -0400 Subject: [PATCH 2/4] Add `check-public-refs`: every repository named here must resolve anonymously MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This repo is public, so naming a repository discloses that it exists, who owns it and roughly what it holds. A prohibition discloses exactly as much as a recommendation — "do not re-host to , it is private" publishes the name either way — so the rule is about the mention, not the sentiment attached to it. A skill shipped with a personal repo named in a warning, on that reasoning. The check is a request rather than a list. An owner allowlist was tried first and was wrong on its first run: it flagged `nock/nock` and `phishfort/phishfort-lists`, because "is this owner well known" is not the property that matters. The property is whether a reader who is not you can open the link, and an unauthenticated HEAD answers it exactly. Deliberately unauthenticated — a token would see private repos and pass them. `MetaMask` and `Consensys` are exempt: org repos are unreachable publicly but readable by colleagues, and naming them is a deliberate call rather than a leak. Two-arm verified: fires on `MajorLift/Reprise` in the tree that shipped it, silent on the tree with it removed. The first version scanned only skill.md and would have passed the violation that motivated it, which was in a references/ file. --- .github/scripts/check-public-refs.mjs | 85 +++++++++++++++++++++++++++ 1 file changed, 85 insertions(+) create mode 100644 .github/scripts/check-public-refs.mjs diff --git a/.github/scripts/check-public-refs.mjs b/.github/scripts/check-public-refs.mjs new file mode 100644 index 0000000..7ef2e58 --- /dev/null +++ b/.github/scripts/check-public-refs.mjs @@ -0,0 +1,85 @@ +#!/usr/bin/env node +// +// Every repository named in this repo must be fetchable by an anonymous reader. +// +// This repo is public. Naming a repository here discloses that it exists, who owns it and +// roughly what is in it — and a prohibition discloses exactly as much as a recommendation: +// "do not re-host to acme/secret-notes, it is private" publishes the name either way. So the +// rule is about the mention, not the sentiment attached to it. +// +// The check is a request, not a list. An owner allowlist looked cheaper and was wrong on its +// first run: it cleared nothing useful and flagged `nock/nock` and `phishfort/phishfort-lists`, +// because "is this owner well known" is not the property that matters. The property is whether +// a reader who is not you can open the link — which an unauthenticated request answers exactly. +// 404 means private or absent; both are unresolvable for a public reader, and both are defects. +// +// Deliberately unauthenticated: a token would see private repos and pass them, which is the +// failure this exists to prevent. +// +// 0 every referenced repository resolves anonymously +// 1 one or more do not +// 2 could not run (offline) — reported, not silently passed +import { readdirSync, readFileSync, statSync } from 'node:fs'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; + +const ROOT = process.env.SKILLS_LINT_ROOT + ? path.resolve(process.env.SKILLS_LINT_ROOT) + : path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..', '..'); + +// `orgs/`, `sponsors/` and friends are github.com paths that are not repositories. +const NOT_A_REPO = new Set(['orgs', 'sponsors', 'users', 'settings', 'apps', 'topics', 'features', 'pricing']); +// Org-internal repos are private to the public but readable by colleagues, and naming them is a +// deliberate call: they are load-bearing context for the audience this repo is written for. The +// rule being enforced is about *personal* repos, which are unreachable by colleagues too. +const INTERNAL_OWNERS = new Set(['MetaMask', 'Consensys']); +// Template placeholders in contributor docs are meant to be substituted, not resolved. +const PLACEHOLDER = /^(YOUR|MY|<|\$\{)/u; +const REPO_REF = /https?:\/\/(?:www\.)?github\.com\/([A-Za-z0-9][\w.-]*)\/([A-Za-z0-9][\w.-]*)/gu; +const TEXT = /\.(md|sh|py|mjs|js|ya?ml|json|tsx?)$/u; + +function walk(dir, out = []) { + for (const e of readdirSync(dir, { withFileTypes: true })) { + if (e.name === '.git' || e.name === 'node_modules') continue; + const full = path.join(dir, e.name); + if (e.isDirectory()) walk(full, out); + else if (TEXT.test(e.name)) out.push(full); + } + return out; +} + +const refs = new Map(); // "owner/repo" -> Set of relative paths +for (const file of walk(ROOT)) { + let text; + try { text = readFileSync(file, 'utf8'); } catch { continue; } + for (const [, owner, repo] of text.matchAll(REPO_REF)) { + if (NOT_A_REPO.has(owner) || INTERNAL_OWNERS.has(owner) || PLACEHOLDER.test(owner)) continue; + const key = `${owner}/${repo.replace(/\.git$/u, '')}`; + if (!refs.has(key)) refs.set(key, new Set()); + refs.get(key).add(path.relative(ROOT, file)); + } +} + +if (refs.size === 0) { console.log('check-public-refs: no repository references found'); process.exit(0); } + +let bad = 0, unknown = 0; +for (const [key, files] of [...refs].sort()) { + let status; + try { + const res = await fetch(`https://github.com/${key}`, { method: 'HEAD', redirect: 'follow' }); + status = res.status; + } catch { + console.error(` ???? ${key} — request failed; cannot conclude`); + unknown += 1; + continue; + } + if (status === 200) continue; + bad += 1; + console.error(` FAIL ${key} — HTTP ${status} anonymously; a public reader cannot open this`); + for (const f of files) console.error(` ${f}`); +} + +console.log(`\ncheck-public-refs: ${refs.size} repository reference(s) checked`); +if (unknown > 0 && bad === 0) { console.error(`${unknown} could not be checked — offline?`); process.exit(2); } +if (bad > 0) { console.error(`${bad} unresolvable. Cite an org-owned location, or state the rule without the example.`); process.exit(1); } +console.log('every referenced repository resolves anonymously'); From a26881ed473bfc747dbab0a9cb8dbcc82baee659 Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Tue, 4 Aug 2026 08:00:07 -0400 Subject: [PATCH 3/4] Run `check-public-refs` in CI, over the whole tree MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Scoped to the whole repository rather than the PR's changed files: whether this repo names something a reader cannot open is a property of what it publishes, not of what a given PR touched, and the reference that motivated this had sat unnoticed through several PRs that edited the same file. `GH_TOKEN` and `GITHUB_TOKEN` are blanked for the step. The check must run as a stranger — an authenticated request resolves private repositories and passes them, which is the exact failure it exists to catch. --- .github/workflows/lint-skill-entry.yml | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/.github/workflows/lint-skill-entry.yml b/.github/workflows/lint-skill-entry.yml index b774aa7..86b637d 100644 --- a/.github/workflows/lint-skill-entry.yml +++ b/.github/workflows/lint-skill-entry.yml @@ -6,7 +6,11 @@ on: - 'domains/**' - 'tools/**' - '.github/scripts/lint-skill-entry.mjs' + - '.github/scripts/check-public-refs.mjs' - '.github/workflows/lint-skill-entry.yml' + - 'test/**' + - 'CONTRIBUTING.md' + - 'README.md' permissions: contents: read @@ -64,3 +68,13 @@ jobs: run: | mapfile -d '' -t files < changed-skill-files.bin node .github/scripts/lint-skill-entry.mjs "${files[@]}" + + # Runs on the WHOLE tree, not the changed files: a private-repo reference is a + # property of what this repository publishes, and a PR that touches nothing can + # still be the moment someone notices one. Unauthenticated by construction — a + # token would see private repos and pass them, which is the failure it prevents. + - name: Every referenced repository resolves anonymously + env: + GH_TOKEN: '' + GITHUB_TOKEN: '' + run: node .github/scripts/check-public-refs.mjs From a7e1c4b8bf039dac6ff5f67d55af414ba649767b Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Tue, 4 Aug 2026 08:38:02 -0400 Subject: [PATCH 4/4] Add `skill-audit`: what loaded, and what published without its gate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit "Did the right skill load" is a question about a probabilistic event — a description matched by a model — so the only honest answer comes from the transcript rather than from the description. This reads one and reports what entered context, by route. Three routes leave three different traces (Skill tool call, slash command, and the loader's base-directory announcement on a description match). Counting one of them reads as silence: run against the session that produced this file, the Skill-tool count alone suggests the reasoning skills were used, and all three together show that not one of them loaded at any point. The second report is the deterministic one. A publish is UNGATED if no gate ran before it at all, and UNCHAINED if a gate ran earlier but not as the same command. The distinction is the finding: a gate that merely ran earlier proves nothing, because the verdict can be read after the write — which is exactly how a blocked artifact reached a public pull request in the session this was written from. Only `gate && publish` makes the shell enforce the dependency. The first version reported that session as clean on the generous rule. It is 195 unchained publishes on the strict one. --- tools/skill-audit.mjs | 106 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 106 insertions(+) create mode 100644 tools/skill-audit.mjs diff --git a/tools/skill-audit.mjs b/tools/skill-audit.mjs new file mode 100644 index 0000000..8c52fd3 --- /dev/null +++ b/tools/skill-audit.mjs @@ -0,0 +1,106 @@ +#!/usr/bin/env node +// +// What actually loaded, and what published without its gate. +// +// Skill loading is not deterministic. A description is matched by a model, so "did the right +// skill load" is a question about a probabilistic event, and the only honest way to answer it +// is to look at what happened rather than at what the description says should happen. +// +// Two reports: +// loaded every skill that entered context, by route. Three routes exist and they leave +// different traces, which is why counting only one of them reads as silence. +// ungated every outward-facing publish with no gate run before it in the same session. +// This one is deterministic and is the reason the script exists: whether a gate +// ran before a write is a fact about the transcript, not a judgement. +// +// Usage: node tools/skill-audit.mjs [--json] +import { createReadStream } from 'node:fs'; +import { createInterface } from 'node:readline'; + +const [file, ...flags] = process.argv.slice(2); +if (!file) { + console.error('usage: skill-audit.mjs [--json]'); + process.exit(2); +} + +const ROUTES = [ + // Skill tool call — the explicit path. + [/"skill"\s*:\s*"([a-z0-9-]+)"/g, 'skill-tool'], + // Slash command injected into the turn. + [/\/?([a-z0-9-]+)<\/command-name>/g, 'slash-command'], + // Description match / directory scope: the loader announces where it read the file from. + [/Base directory for this skill:[^\n"]*?skills\/([a-z0-9-]+)/g, 'auto-load'], +]; + +// An outward-facing write. Deliberately broader than the porcelain: `gh api` with a body +// field is the path that bypasses `gh pr comment`, and it is the one that got used. +const PUBLISH = /gh\s+(?:pr|issue)\s+(?:comment|edit|create)\b|gh\s+api\b[^"']*(?:-F|-f|--field|--raw-field)\s+body=/; +const GATE = /attest-gate\.sh|pr-evidence-gate\.py/; + +const loaded = new Map(); +const events = []; +let line = 0; + +const rl = createInterface({ input: createReadStream(file), crlfDelay: Infinity }); +for await (const raw of rl) { + line += 1; + for (const [re, route] of ROUTES) { + re.lastIndex = 0; + let m; + while ((m = re.exec(raw)) !== null) { + const key = `${m[1]} (${route})`; + loaded.set(key, (loaded.get(key) ?? 0) + 1); + } + } + const isGate = GATE.test(raw); + const isPublish = PUBLISH.test(raw); + if (isGate) events.push({ line, kind: 'gate' }); + // Chained means the gate and the write are one command, so the shell enforces the + // ordering. A gate that merely ran EARLIER proves nothing: the verdict can be read after + // the write, or not read at all, which is how a blocked artifact reached a public PR in + // the session this script was written from. + if (isPublish) events.push({ line, kind: 'publish', chained: isGate }); +} + +// A publish is gated if a gate invocation appears earlier in the transcript. This is +// deliberately generous — same session, any distance — because the failure it looks for is +// "no gate at all", and a stricter window would produce arguments about proximity rather +// than findings. +let lastGate = -1; +const ungated = []; +const unchained = []; +for (const e of events) { + if (e.kind === 'gate') { lastGate = e.line; continue; } + if (lastGate < 0) ungated.push(e.line); + if (!e.chained) unchained.push(e.line); +} + +const report = { + transcript: file, + loaded: Object.fromEntries([...loaded].sort((a, b) => b[1] - a[1])), + publishes: events.filter((e) => e.kind === 'publish').length, + gateRuns: events.filter((e) => e.kind === 'gate').length, + ungatedPublishLines: ungated, + unchainedPublishLines: unchained, +}; + +if (flags.includes('--json')) { + console.log(JSON.stringify(report, null, 2)); +} else { + console.log(`skill-audit: ${file}\n`); + console.log('loaded:'); + for (const [k, v] of Object.entries(report.loaded)) console.log(` ${String(v).padStart(4)} ${k}`); + if (!Object.keys(report.loaded).length) console.log(' (none)'); + console.log(`\npublishes: ${report.publishes} gate runs: ${report.gateRuns}`); + console.log(`unchained publishes: ${unchained.length} of ${report.publishes}`); + if (ungated.length) { + console.log(`\nUNGATED (${ungated.length}) — no gate ran at all before these:`); + for (const l of ungated.slice(0, 10)) console.log(` line ${l}`); + } + if (unchained.length) { + console.log(`\nUNCHAINED (${unchained.length}) — a gate ran earlier, but not as the same`); + console.log('command, so nothing forced the write to depend on its verdict:'); + for (const l of unchained.slice(0, 10)) console.log(` line ${l}`); + } +} +process.exit(ungated.length || unchained.length ? 1 : 0);