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'); 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). 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 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);