diff --git a/CHANGELOG.md b/CHANGELOG.md index 056ae9d..5fe7bfe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,19 @@ All notable changes to cc-settings are documented here. > **Versioning** — cc-settings uses a single version number matching the installer (`src/setup.ts` `VERSION` constant, written to `~/.claude/.cc-settings-version` sentinel). Historical entries below 10.0 predate this unification; the jump from v8.x to v10.x in April 2026 realigned the product version with the installer version that was already ahead. +## [Unreleased] + +### Changed + +- Codex bridge hardening: `--force` escape bypasses a sticky rate-limited/no-access verdict (which Codex emits even on auth mismatch); a fresh `available` verdict skips the per-call `codex login status` probe for 60s; timeouts now report partial output + a split/raise hint instead of an opaque exit code; `exec` appends `git status`/`diff --stat` so the changed files are always surfaced; `sanitizeOutput` strips ANSI and redacts secrets (`sk-`/`Bearer`/`Authorization`/`*_API_KEY|TOKEN|SECRET=`) on all returned output. +- Hook tamper-defense: the three divergent "managed `~/.claude/src` hook command" classifiers (settings-merge, light-profile, audit-hooks) are unified into `src/lib/hook-command.ts`; the trusted regex is tightened to `(scripts|hooks)` only (drops `lib/`); hook-block traversal goes through a `HooksBlock`-schema-driven `iterCommandHooks`. `readFingerprint`/`readSrcManifest` now validate through zod, strip control chars from `installedAt`, and reject manifest keys with `..`/absolute paths. +- Installer: `buildInstallPlan` is the single source of truth for install/prune footprint; `main()` split into `runMigrateOnly`/`runFullInstall`; the in-installer skill-prune loop removed. `managed-skills.ts` split into `ACTIVE_SKILLS` (completed — 7 missing current skills added: `codex`, `freeze`, `plan-ceo-review`, `proof-of-work`, `retro`, `review-batch`, `strategist`) + `TOMBSTONE_SKILLS`; `lint:skills` now asserts `ACTIVE_SKILLS` matches `skills/` on disk. +- Settings merger is now pure: MCP-server preservation moved out of `settings-merge.ts` into `mcp.ts` `resolveMcpServers` (behavior unchanged). + +### Internal + +- `claude-audit.ts` split into `analyzeCommands` (model) + `renderAudit` (render); shared frontmatter-lint core extracted from `lint-skills`/`lint-knowledge`; `writeState` is now atomic (tmp+rename); `tool-cadence` `CounterState` rehydration goes through `normalizeCounterState`. + ## [11.27.1] — 2026-06-19 ### Fix — installer merger now deep-merges object config defaults diff --git a/CLAUDE-FULL.md b/CLAUDE-FULL.md index b2f798b..d56332a 100644 --- a/CLAUDE-FULL.md +++ b/CLAUDE-FULL.md @@ -137,4 +137,4 @@ Full threat model + remediation: see `SECURITY.md`. ### Skill library soft cap — 40 -Anthropic's Skills guide flags 20–50 skills as the point where the Skill selector starts struggling to read every description per turn. We sit at 34 cc-settings skills (Tier P1 cleanup May 2026: retired `audit`, `lenis`; merged `create-handoff`+`resume-handoff` → `handoff`, `discovery`+`prd` → `plan-feature`, `ask`+`premortem`+`compare-approaches` → `oracle`, `tdd` folded into `test`, `cc-sync`+`cc-update` → `cc`; folded `long-task` into `orchestrate`; demoted `write-a-skill` to `bun run new-skill` CLI; `nuclear-review` ported from Cursor team-kit May 2026; `share-learning` revived May 2026; `proof-of-work` + `review-batch` added May 2026 from the Orchestration Tax; `freeze` edit-scope lock ported from gstack June 2026). **Adding a new skill past 40 requires removing one** — re-evaluate `skills/` for consolidation candidates first. Validate the library with `bun run lint:skills`, which enforces the spec (kebab-case folders, frontmatter contract, no angle brackets, …) and surfaces the cap as a warning when crossed. Drift hides easily; let the linter catch it. +Anthropic's Skills guide flags 20–50 skills as the point where the Skill selector starts struggling to read every description per turn. We sit at 35 cc-settings skills (Tier P1 cleanup May 2026: retired `audit`, `lenis`; merged `create-handoff`+`resume-handoff` → `handoff`, `discovery`+`prd` → `plan-feature`, `ask`+`premortem`+`compare-approaches` → `oracle`, `tdd` folded into `test`, `cc-sync`+`cc-update` → `cc`; folded `long-task` into `orchestrate`; demoted `write-a-skill` to `bun run new-skill` CLI; `nuclear-review` ported from Cursor team-kit May 2026; `share-learning` revived May 2026; `proof-of-work` + `review-batch` added May 2026 from the Orchestration Tax; `freeze` edit-scope lock ported from gstack June 2026). **Adding a new skill past 40 requires removing one** — re-evaluate `skills/` for consolidation candidates first. Validate the library with `bun run lint:skills`, which enforces the spec (kebab-case folders, frontmatter contract, no angle brackets, …) and surfaces the cap as a warning when crossed. Drift hides easily; let the linter catch it. diff --git a/CLAUDE.md b/CLAUDE.md index 44e8e45..de0d1e5 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -17,7 +17,7 @@ for standards. ## About This Repo -**TypeScript on Bun** (migrated from bash in April 2026; see git log for history). Runtime: `bun >=1.1.30`. Skills: 34 cc-settings skills (Tier P1 cleanup May 2026: retired 3, merged 7 pairs into 4, demoted 1 to CLI; `nuclear-review` ported from Cursor team-kit May 2026; share-learning revived May 2026; `freeze` edit-scope lock ported from gstack June 2026). +**TypeScript on Bun** (migrated from bash in April 2026; see git log for history). Runtime: `bun >=1.1.30`. Skills: 35 cc-settings skills (Tier P1 cleanup May 2026: retired 3, merged 7 pairs into 4, demoted 1 to CLI; `nuclear-review` ported from Cursor team-kit May 2026; share-learning revived May 2026; `freeze` edit-scope lock ported from gstack June 2026). Deps: `zod`, `@inquirer/confirm`, `yaml`. Dev: `@biomejs/biome`, `typescript`, `@types/bun`. diff --git a/MANUAL.md b/MANUAL.md index 5c278f0..ffad6ad 100644 --- a/MANUAL.md +++ b/MANUAL.md @@ -98,7 +98,7 @@ You'll be asked to pick a starter — **satus** (Next.js, content sites) or **no | *"review my changes"* | `/review` — checks against TypeScript / React / a11y / performance rules | | *"audit my recent activity"* | `bun run claude-audit` — analyzes Bash command logs | -**4. Don't memorize skills.** Just talk normally. The `Skill` tool auto-matches the right one. To see all 34 cc-settings skills, scroll to [All Skills](#all-skills) below. (Native Claude Code skills like `/loop`, `/schedule`, `/code-review`, `/review`, `/init`, `/security-review`, and any plugins like `sanity:*` or `vercel:*` load in addition — your session typically sees 60–80 skills total.) +**4. Don't memorize skills.** Just talk normally. The `Skill` tool auto-matches the right one. To see all 35 cc-settings skills, scroll to [All Skills](#all-skills) below. (Native Claude Code skills like `/loop`, `/schedule`, `/code-review`, `/review`, `/init`, `/security-review`, and any plugins like `sanity:*` or `vercel:*` load in addition — your session typically sees 60–80 skills total.) **5. When something is unclear**, ask Claude directly: *"what skill handles X?"* or *"what just changed in cc-settings?"*. The setup is self-describing. diff --git a/src/hooks/tool-cadence.ts b/src/hooks/tool-cadence.ts index 11c8a6a..10b5ab4 100644 --- a/src/hooks/tool-cadence.ts +++ b/src/hooks/tool-cadence.ts @@ -66,12 +66,14 @@ const FILE_EDIT_TOOLS: Record = { interface CounterState { count: number; lastTool: string; + /** Debounce timestamp; genuinely absent until the first fire. */ firedAt?: number; - files?: string[]; - nudged?: boolean; - countAtNudge?: number; - filesAtNudge?: number; - escalated?: boolean; + // The rest are always present once normalizeCounterState() has run. + files: string[]; + nudged: boolean; + countAtNudge: number; + filesAtNudge: number; + escalated: boolean; } type Payload = { @@ -100,24 +102,41 @@ async function currentHead(cwd: string): Promise { return out || undefined; } +/** Validate and default every field of CounterState from an unknown raw value. + * Replaces the previous inline re-hydration + `as number | undefined` cast. */ +function normalizeCounterState(raw: unknown): CounterState { + if (raw === null || typeof raw !== "object") { + return { + count: 0, + lastTool: "", + files: [], + nudged: false, + countAtNudge: 0, + filesAtNudge: 0, + escalated: false, + }; + } + const r = raw as Record; + return { + count: typeof r.count === "number" ? r.count : 0, + lastTool: typeof r.lastTool === "string" ? r.lastTool : "", + firedAt: typeof r.firedAt === "number" ? r.firedAt : undefined, + files: Array.isArray(r.files) ? (r.files as string[]) : [], + nudged: typeof r.nudged === "boolean" ? r.nudged : false, + countAtNudge: typeof r.countAtNudge === "number" ? r.countAtNudge : 0, + filesAtNudge: typeof r.filesAtNudge === "number" ? r.filesAtNudge : 0, + escalated: typeof r.escalated === "boolean" ? r.escalated : false, + }; +} + // --- Branch 1: consecutive non-Agent call counter -------------------------- async function parallelmaxBranch( toolName: string, toolInput: Payload["tool_input"], ): Promise { - // Defensive defaults for old-shape state files. - const raw = await readState(COUNTER_STATE, { count: 0, lastTool: "" }); - const state = { - count: raw.count ?? 0, - lastTool: raw.lastTool ?? "", - firedAt: raw.firedAt as number | undefined, - files: raw.files ?? ([] as string[]), - nudged: raw.nudged ?? false, - countAtNudge: raw.countAtNudge ?? 0, - filesAtNudge: raw.filesAtNudge ?? 0, - escalated: raw.escalated ?? false, - }; + const raw = await readState(COUNTER_STATE, null); + const state = normalizeCounterState(raw); if (toolName === "Agent") { // Reset the whole streak on delegation; preserve firedAt for debounce. diff --git a/src/lib/audit-hooks.ts b/src/lib/audit-hooks.ts index 5f5c104..8945b60 100644 --- a/src/lib/audit-hooks.ts +++ b/src/lib/audit-hooks.ts @@ -6,7 +6,7 @@ // // Classification rules: // trusted — matches cc-settings' shipped command pattern -// (`bun "$HOME/.claude/src/{scripts,hooks,lib}/.ts"`) +// (`bun "$HOME/.claude/src/{scripts,hooks}/.ts"`) // AND the referenced file's content hash matches the install // manifest written by setup.ts. Trust is content-based, not // path-based: a path-shaped match alone proves nothing, since @@ -34,8 +34,8 @@ import { existsSync } from "node:fs"; import { readFile } from "node:fs/promises"; import { homedir } from "node:os"; import { join } from "node:path"; -import type { Hook, HookGroup } from "../schemas/hooks.ts"; import { HooksBlock } from "../schemas/hooks.ts"; +import { parseHookCommand } from "./hook-command.ts"; import { hashFileOrNull, readSrcManifest } from "./hooks-fingerprint.ts"; export type HookSeverity = "trusted" | "unknown" | "stale" | "suspicious"; @@ -50,16 +50,30 @@ export interface HookFinding { reasons: string[]; } +/** Schema-validation meta-finding — distinguishes schema failures from real hook findings. */ +export interface SchemaFinding { + event: "schema"; + groupIndex: -1; + hookIndex: -1; + type: "schema-validation"; + command: string; + severity: "unknown"; + reasons: string[]; +} + +/** Discriminated union of audit findings. */ +export type AuditFinding = HookFinding | SchemaFinding; + export interface AuditResult { settingsPath: string; exists: boolean; totalHooks: number; - findings: HookFinding[]; + findings: AuditFinding[]; } /** Content-integrity view consumed by the classifier: rel path under - * ~/.claude/src → does the on-disk content hash match the install manifest? - * `null`/absent means no manifest exists — classification degrades to + * ~/.claude/src - does the on-disk content hash match the install manifest? + * `null`/absent means no manifest exists - classification degrades to * "unknown" for shipped-pattern commands rather than trusting path shape. * `fileExists` answers whether a given rel path is currently present on disk * (independent of the manifest), used to distinguish stale entries (file gone) @@ -71,7 +85,7 @@ export interface SrcIntegrity { /** Build the SrcIntegrity view: every manifested file, hashed on disk and * compared. Returns null when no manifest exists (pre-manifest install) or - * on any read error — fail-soft, never throws. */ + * on any read error - fail-soft, never throws. */ export async function loadSrcIntegrity(claudeDir?: string): Promise { try { const dir = claudeDir ?? join(homedir(), ".claude"); @@ -87,18 +101,10 @@ export async function loadSrcIntegrity(claudeDir?: string): Promise/.ts"` — accept quoted and unquoted -// `$HOME` and `${HOME}` forms. Allow an optional trailing arg list (some -// hooks invoke a script with positional args, e.g. `swarm-log.ts start`), -// but ONLY simple word-like tokens separated by spaces/tabs: shell -// metacharacters (`;`, `|`, `&`, `$(`, backticks, redirects) are rejected, -// and so are newlines — `\s` would treat `\n` as an arg separator while the -// shell treats it as a command separator, letting a payload on the next line -// (`bun .../x.ts\n/path/to/evilbin`) ride the shipped-pattern match. -// Capture group 1 is the path relative to ~/.claude/src — the -// install-manifest key. -const TRUSTED_BUN_CC = - /^bun[ \t]+"?\$\{?HOME\}?\/\.claude\/src\/((?:scripts|hooks|lib)\/[a-zA-Z0-9_-]+\.ts)"?(?:[ \t]+[A-Za-z0-9_.:=/-]+)*[ \t]*$/; +// NOTE: The managed pattern lives in hook-command.ts (MANAGED_HOOK_CMD). +// It is (scripts|hooks) ONLY - lib/ files are support modules, not hooks, +// and were dropped from the trusted surface in the nuclear-review refactor. +// [ \t] (not \s) arg-separation is preserved from the original, see hook-command.ts. /** Classify a single shipped-pattern command against the install manifest. */ function classifyShippedPath( @@ -140,22 +146,30 @@ function classifyShippedPath( }; } +// Compound command separator regex. Splits on ; && || and newline variants +// including U+2028 LINE SEPARATOR and U+2029 PARAGRAPH SEPARATOR. +// Built via new RegExp so \uNNNN escapes stay as explicit codepoints in the +// source text rather than invisible literal characters (encoding-safe). +const COMPOUND_SEP = /\s*(?:;|&&|\|\||\r?\n|\r|\u2028|\u2029)\s*/; + // Compound commands that chain multiple trusted bun scripts with `;` or `&&`. -// Each sub-command must match TRUSTED_BUN_CC *and* pass content verification; +// Each sub-command must match MANAGED_HOOK_CMD *and* pass content verification; // the compound's severity is the worst of its parts. function classifyCompound( cmd: string, integrity: SrcIntegrity | null | undefined, ): { severity: HookSeverity; reasons: string[] } | null { - // Newlines (and the U+2028/U+2029 line separators) are command separators - // too — split on them like `;`/`&&` so a newline-chained pair still gets - // per-part manifest verification. - const parts = cmd.split(/\s*(?:;|&&|\|\||\r?\n|\r|\u2028|\u2029)\s*/).filter((p) => p.length > 0); + const parts = cmd.split(COMPOUND_SEP).filter((p) => p.length > 0); if (parts.length < 2) return null; - const matches = parts.map((p) => p.match(TRUSTED_BUN_CC)); - if (!matches.every((m) => m?.[1])) return null; - const results = matches.map((m) => classifyShippedPath(m?.[1] ?? "", integrity)); + // Parse each part via the shared managed-command parser (drops lib/). + // SECURITY: trust requires EVERY part to be managed (incl. `||` branches — a + // failure-path command still runs). Never relax this `every` to `some`, or a + // `managed || evil` compound would classify as trusted. + const parsed = parts.map((p) => parseHookCommand(p)); + if (!parsed.every((p) => p.managed && p.relPath)) return null; + + const results = parsed.map((p) => classifyShippedPath(p.relPath ?? "", integrity)); const suspicious = results.filter((r) => r.severity === "suspicious"); if (suspicious.length > 0) { return { severity: "suspicious", reasons: suspicious.flatMap((r) => r.reasons) }; @@ -212,12 +226,12 @@ const SUSPICIOUS_PATTERNS: Array<{ rx: RegExp; reason: string }> = [ // Quick "is this obviously a one-liner blob of opaque code?" check. function looksOpaque(cmd: string): boolean { if (cmd.length < 250) return false; - // No spaces in long runs → likely obfuscated/encoded. reduce (not spread) - // because `cmd` is arbitrary settings.json text — a pathological token count + // No spaces in long runs -> likely obfuscated/encoded. reduce (not spread) + // because `cmd` is arbitrary settings.json text - a pathological token count // would blow the call stack with Math.max(...tokens). const longestRun = cmd.split(/\s/).reduce((m, s) => Math.max(m, s.length), 0); if (longestRun > 200) return true; - // High density of base64 alphabet → encoded payload + // High density of base64 alphabet -> encoded payload const b64Chars = (cmd.match(/[A-Za-z0-9+/=]/g) ?? []).length; return b64Chars / cmd.length > 0.85; } @@ -235,19 +249,19 @@ export function classifyHookCommand( cmd: string, integrity?: SrcIntegrity | null, ): { severity: HookSeverity; reasons: string[] } { - // Malware signatures are checked FIRST and unconditionally — a hit always + // Malware signatures are checked FIRST and unconditionally - a hit always // wins, even when the command would otherwise match the shipped pattern and - // verify against the manifest. Defense in depth: TRUSTED_BUN_CC already + // verify against the manifest. Defense in depth: MANAGED_HOOK_CMD already // rejects shell metacharacters in trailing args, but pattern-match trust // must never suppress an explicit malware signal. const sus = matchSuspicious(cmd); if (sus.length > 0) return { severity: "suspicious", reasons: sus }; // Shipped-pattern commands next. Trust is CONTENT-based (install manifest), - // not path-based. - const m = cmd.match(TRUSTED_BUN_CC); - if (m?.[1]) { - return classifyShippedPath(m[1], integrity); + // not path-based. lib/ files are NOT in the managed pattern (tightened). + const parsed = parseHookCommand(cmd); + if (parsed.managed && parsed.relPath) { + return classifyShippedPath(parsed.relPath, integrity); } const compound = classifyCompound(cmd, integrity); if (compound) return compound; @@ -260,7 +274,7 @@ export function classifyHookCommand( // Settings.json shape we care about: top-level `hooks` is `{ [event]: HookGroup[] }` // where each group has `hooks: Hook[]`. We tolerate unknown shapes silently -// (return empty findings) — a malformed settings.json is a different problem. +// (return empty findings) - a malformed settings.json is a different problem. export function auditHooks(settings: unknown, integrity?: SrcIntegrity | null): HookFinding[] { if (!settings || typeof settings !== "object") return []; @@ -272,13 +286,16 @@ export function auditHooks(settings: unknown, integrity?: SrcIntegrity | null): if (!Array.isArray(groups)) continue; groups.forEach((group: unknown, gi: number) => { if (!group || typeof group !== "object") return; - const entries = (group as HookGroup).hooks; - if (!Array.isArray(entries)) return; - entries.forEach((entry: Hook, hi: number) => { + const hooksBlock = (group as Record).hooks; + if (!Array.isArray(hooksBlock)) return; + hooksBlock.forEach((entry: unknown, hi: number) => { + if (!entry || typeof entry !== "object") return; + const e = entry as Record; // Only `command`-type hooks have a string command. Prompt/agent/http/ // mcp_tool hooks don't run arbitrary shell, so out of scope here. - if (entry?.type !== "command") return; - const cmd = entry.command.trim(); + if (e.type !== "command") return; + if (typeof e.command !== "string") return; + const cmd = e.command.trim(); if (!cmd) return; const { severity, reasons } = classifyHookCommand(cmd, integrity); findings.push({ @@ -309,18 +326,18 @@ export async function auditSettingsFile(path?: string, claudeDir?: string): Prom const text = await readFile(settingsPath, "utf8"); parsed = JSON.parse(text); } catch { - // Malformed JSON — return empty audit (this is not the file we're trying + // Malformed JSON - return empty audit (this is not the file we're trying // to defend against; a broken file is its own problem). return { settingsPath, exists: true, totalHooks: 0, findings: [] }; } // Validate the hooks block against the schema before walking it. A failure - // doesn't stop the audit — we surface it as an `unknown` finding and then - // fall through to auditHooks (which degrades gracefully on malformed input). - // Using `unknown` rather than `suspicious` because a schema mismatch alone - // doesn't prove malice — it might be forward-compat drift from a newer + // doesn't stop the audit - we surface it as a SchemaFinding (unknown severity) + // and then fall through to auditHooks (which degrades gracefully on malformed + // input). Using `unknown` rather than `suspicious` because a schema mismatch + // alone doesn't prove malice - it might be forward-compat drift from a newer // Claude Code version. - const extraFindings: HookFinding[] = []; + const extraFindings: SchemaFinding[] = []; if (parsed !== null && typeof parsed === "object") { const hooksRaw = (parsed as Record).hooks; if (hooksRaw !== undefined) { @@ -342,15 +359,15 @@ export async function auditSettingsFile(path?: string, claudeDir?: string): Prom } } - // Content verification against the install manifest — shipped-pattern + // Content verification against the install manifest - shipped-pattern // commands are only "trusted" when the file they point at hashes to what // setup.ts installed. const integrity = await loadSrcIntegrity(claudeDir); // totalHooks counts audited command hooks (the "hook command(s) total" the - // CLI prints) — NOT findings, which also include the schema pseudo-finding. + // CLI prints) - NOT findings, which also include the schema pseudo-finding. const hookFindings = auditHooks(parsed, integrity); - const findings = [...extraFindings, ...hookFindings]; + const findings: AuditFinding[] = [...extraFindings, ...hookFindings]; return { settingsPath, exists: true, totalHooks: hookFindings.length, findings }; } @@ -376,7 +393,7 @@ export function formatAuditReport(result: AuditResult): string { lines.push(` ${result.totalHooks} hook command(s) total.`); lines.push(""); - const grouped: Record = { + const grouped: Record = { suspicious: [], stale: [], unknown: [], diff --git a/src/lib/codex.ts b/src/lib/codex.ts index 74ec025..41b82ca 100644 --- a/src/lib/codex.ts +++ b/src/lib/codex.ts @@ -27,6 +27,9 @@ const NO_ACCESS_TTL_MS = 24 * 60 * 60 * 1000; // entitlement: re-check daily const RATE_LIMIT_TTL_MS = 5 * 60 * 60 * 1000; // Codex meters per ~5-hour window const DEFAULT_EXEC_TIMEOUT_MS = 10 * 60 * 1000; const MAX_EXEC_TIMEOUT_MS = 15 * 60 * 1000; // hard cap so a hung call can't run unbounded +// Skip the up-to-5s `codex login status` spawn when a recent "available" verdict +// is still fresh. 60s is safe: the badge poll and exec preflight share this cache. +const AVAILABLE_TTL_MS = 60_000; // ESC[…m built from charCode(27) rather than a literal control char, so the // regex doesn't trip biome's noControlCharactersInRegex rule. @@ -84,6 +87,11 @@ export interface CodexRunOptions { sandbox?: CodexSandbox; cwd?: string; timeoutMs?: number; + /** When true, bypass a sticky rate-limited or no-access verdict and re-probe + * with a real call. Useful when the quota message was a false positive (auth + * mismatch). Does NOT bypass not-installed or unauthenticated — those can't + * succeed regardless. */ + force?: boolean; } export interface CodexRunResult { @@ -157,9 +165,17 @@ export function reconcile(live: LiveAvailability, cached: CodexVerdict): CodexVe } /** Run the cheap check, reconcile with the cache, persist, and return the verdict. - * Shared by the SessionStart hook (for the badge) and the preflight gate. */ + * Shared by the SessionStart hook (for the badge) and the preflight gate. + * When the cached verdict is "available" and still within AVAILABLE_TTL_MS, the + * up-to-5s `codex login status` spawn is skipped entirely to avoid latency. */ export async function refreshCodexVerdict(): Promise { - const [live, cached] = await Promise.all([checkCodexAvailability(), readCodexVerdict()]); + const cached = await readCodexVerdict(); + // Short-circuit: a recent "available" verdict doesn't need a fresh login-status + // spawn. Negative verdicts always go through the full reconcile path. + if (cached.state === "available" && ageMs(cached.checkedAt) < AVAILABLE_TTL_MS) { + return cached; + } + const live = await checkCodexAvailability(); const v = reconcile(live, cached); await writeCodexVerdict(v); return v; @@ -167,6 +183,20 @@ export async function refreshCodexVerdict(): Promise { // ── L2: classify a real call's failure ───────────────────────────────────────── +/** + * Strip ANSI escape codes and redact credentials from any string. Operates on + * the full text (no line capping, no length limit). Used by both `firstLine` + * (for cached detail snippets) and `sanitizeOutput` (for full output surfaces). + */ +export function sanitizeOutput(s: string): string { + return s + .replace(ANSI_RE, "") + .replace(/sk-[A-Za-z0-9_-]{16,}/g, "sk-[redacted]") + .replace(/Bearer[\s:]+\S+/gi, "Bearer [redacted]") + .replace(/(Authorization:\s*)\S+/gi, "$1[redacted]") + .replace(/\b([A-Z0-9_]*(?:API_?KEY|TOKEN|SECRET))=\S+/gi, "$1=[redacted]"); +} + /** First non-empty line of a subprocess stderr, sanitized before it's cached to * ~/.claude/tmp or echoed to the terminal: strip ANSI escapes, redact * token-shaped substrings, and cap length. Defense-in-depth against a future or @@ -177,12 +207,7 @@ function firstLine(s: string): string { .split("\n") .find((l) => l.trim().length > 0) ?.trim() ?? ""; - return line - .replace(ANSI_RE, "") - .replace(/sk-[A-Za-z0-9_-]{16,}/g, "sk-[redacted]") - .replace(/Bearer\s+\S+/gi, "Bearer [redacted]") - .replace(/(Authorization:\s*)\S+/gi, "$1[redacted]") - .slice(0, 200); + return sanitizeOutput(line).slice(0, 200); } /** Map a failed `codex exec` to a verdict state. Codex's "Quota exceeded / you've @@ -213,8 +238,12 @@ function blocked(state: CodexState, message: string): CodexRunResult { /** Is it worth attempting a Codex call right now? Returns a blocking result with * guidance when not, or null when clear to proceed (including "unknown" — there - * the call itself is the probe). */ -export async function codexPreflight(): Promise { + * the call itself is the probe). + * + * When `force` is true, sticky rate-limited and no-access verdicts are bypassed + * so the real call can re-probe. not-installed and unauthenticated are never + * bypassed — a call cannot succeed in those states regardless. */ +export async function codexPreflight(force = false): Promise { const v = await refreshCodexVerdict(); switch (v.state) { case "available": @@ -230,14 +259,16 @@ export async function codexPreflight(): Promise { "Codex is installed but not logged in. Run `codex login` (use your ChatGPT/Codex subscription) to enable the bridge. Continuing Claude-only.", ); case "no-access": + if (force) return null; // re-probe: the sticky may be a false positive return blocked( v.state, - `This account showed no Codex access on its plan (checked ${v.checkedAt}). If that's wrong, run \`codex logout && codex login\` — the entitlement error is sometimes a stale/cross-workspace credential. Continuing Claude-only.`, + `This account showed no Codex access on its plan (checked ${v.checkedAt}). If that's wrong, run \`codex logout && codex login\` — the entitlement error is sometimes a stale/cross-workspace credential. Or pass --force to re-probe. Continuing Claude-only.`, ); case "rate-limited": + if (force) return null; // re-probe: quota message is also emitted on auth mismatch return blocked( v.state, - `Codex hit its usage limit for this ~5-hour window (since ${v.checkedAt}). Failing over to Claude-only; retry after the window resets.`, + `Codex hit its usage limit for this ~5-hour window (since ${v.checkedAt}). Failing over to Claude-only; retry after the window resets, or pass --force to re-probe.`, ); default: return null; @@ -247,9 +278,16 @@ export async function codexPreflight(): Promise { /** Run `codex exec` with the gate in front. On success the plain final message is * returned (no --json, so stdout is the answer, not an event stream). On failure * the error is classified and cached so repeated entitlement failures stop - * retrying while one-off errors don't poison the cache. */ + * retrying while one-off errors don't poison the cache. + * + * Timeout: if the process is still running at the effective timeout limit, the + * partial stdout is surfaced with a human-readable detail message. Timeouts are + * NOT cached as sticky failures — the bridge stays "available" for the next call. + * + * Output sanitization: both success and failure output paths run sanitizeOutput + * before returning, so callers never need to strip credentials themselves. */ export async function runCodexExec(opts: CodexRunOptions): Promise { - const gate = await codexPreflight(); + const gate = await codexPreflight(opts.force ?? false); if (gate) return gate; const sandbox = opts.sandbox ?? "workspace-write"; @@ -262,6 +300,7 @@ export async function runCodexExec(opts: CodexRunOptions): Promise= timeout) { + await writeCodexVerdict({ state: "available", checkedAt: now, sticky: false }); + const minutes = Math.round(timeout / 60_000); + return { + ok: false, + output: sanitizeOutput(stdout.trim()), + state: "unknown", + detail: `Codex timed out after ${minutes}m — split the task into smaller pieces or raise the timeout.`, + }; } const cls = classifyCodexError(exit, stderr); @@ -288,5 +342,5 @@ export async function runCodexExec(opts: CodexRunOptions): Promise/.ts"` — accept quoted and unquoted +// `$HOME` and `${HOME}` forms. Allow an optional trailing arg list (some +// hooks invoke a script with positional args, e.g. `handoff.ts create`), +// but ONLY simple word-like tokens separated by [ \t] (spaces/tabs): shell +// metacharacters (`;`, `|`, `&`, `$(`, backticks, redirects) are rejected, +// and so are newlines — `\s` would treat `\n` as an arg separator while the +// shell treats it as a command separator, letting a payload on the next line +// (`bun .../x.ts\n/path/to/evilbin`) ride the shipped-pattern match. +// NOTE: [ \t] (not \s) arg-separation is a deliberate security property — +// do NOT widen to \s; that would allow newline-chained payloads to match. +// Capture group 1 is the path relative to ~/.claude/src — the install-manifest key. +export const MANAGED_HOOK_CMD: RegExp = + /^bun[ \t]+"?\$\{?HOME\}?\/\.claude\/src\/((?:scripts|hooks)\/[a-zA-Z0-9_-]+\.ts)"?(?:[ \t]+[A-Za-z0-9_.:=/-]+)*[ \t]*$/; + +export interface ParsedHookCommand { + raw: string; + /** true iff a bun invocation of ~/.claude/src/{scripts,hooks}/.ts */ + managed: boolean; + dir: "scripts" | "hooks" | null; + /** e.g. "hooks/safety-net.ts", or null when not managed */ + relPath: string | null; +} + +/** Parse a hook command string against the managed pattern. */ +export function parseHookCommand(command: string): ParsedHookCommand { + const m = command.match(MANAGED_HOOK_CMD); + if (!m?.[1]) { + return { raw: command, managed: false, dir: null, relPath: null }; + } + const relPath = m[1]; + const dir = relPath.startsWith("scripts/") ? "scripts" : "hooks"; + return { raw: command, managed: true, dir, relPath }; +} + +/** Shorthand predicate — equivalent to parseHookCommand(command).managed */ +export function isManagedHookCommand(command: string): boolean { + return MANAGED_HOOK_CMD.test(command); +} + +// --------------------------------------------------------------------------- +// Schema-driven hook traversal — replaces three hand-rolled walks +// --------------------------------------------------------------------------- + +/** + * Tolerantly iterate every command-hook in a settings object or a hooks block. + * + * Uses the HooksBlock zod schema: safeParse on success → iterate the typed + * result. On schema failure falls back to a defensive walk (fail-open — this + * runs in tamper-detection code and must never throw). + * + * The input may be a full settings object (with a `.hooks` sub-key) or a + * bare hooks block. Both shapes are handled. + */ +export function* iterCommandHooks( + settingsOrHooks: unknown, +): Generator<{ event: string; command: string }, void, unknown> { + if (!settingsOrHooks || typeof settingsOrHooks !== "object") return; + + // Unwrap the hooks sub-key if the caller passed a full settings object. + const hooksCandidate = + "hooks" in (settingsOrHooks as Record) + ? (settingsOrHooks as Record).hooks + : settingsOrHooks; + + // --- Schema-driven path --- + const parsed = HooksBlock.safeParse(hooksCandidate); + if (parsed.success) { + for (const [event, groups] of Object.entries(parsed.data)) { + if (!Array.isArray(groups)) continue; + for (const group of groups) { + for (const hook of group.hooks) { + if (hook.type === "command") { + yield { event, command: hook.command }; + } + } + } + } + return; + } + + // --- Defensive fallback (fail-open) --- + // Schema parse failed — walk the raw object defensively so we still surface + // hooks from settings that use a newer (forward-compat) shape. + if (!hooksCandidate || typeof hooksCandidate !== "object") return; + for (const [event, groups] of Object.entries(hooksCandidate as Record)) { + if (!Array.isArray(groups)) continue; + for (const group of groups) { + if (!group || typeof group !== "object") continue; + const hooks = (group as Record).hooks; + if (!Array.isArray(hooks)) continue; + for (const hook of hooks) { + if (!hook || typeof hook !== "object") continue; + const h = hook as Record; + if (h.type === "command" && typeof h.command === "string") { + yield { event, command: h.command }; + } + } + } + } +} diff --git a/src/lib/hook-runtime.ts b/src/lib/hook-runtime.ts index 8a720b0..2f3db08 100644 --- a/src/lib/hook-runtime.ts +++ b/src/lib/hook-runtime.ts @@ -2,7 +2,7 @@ // duplicate: stdin-JSON parsing with env fallback, ~/.claude/tmp/.json // state IO, top-level fail-open wrapper. Extracted in v11.1.1 — see CHANGELOG. -import { mkdir, readFile, writeFile } from "node:fs/promises"; +import { mkdir, readFile, rename, writeFile } from "node:fs/promises"; import { homedir } from "node:os"; import { join } from "node:path"; @@ -40,10 +40,14 @@ export async function readState(name: string, fallback: T): Promise { } } -/** Atomic-ish write to ~/.claude/tmp/.json. Creates the dir if missing. */ +/** Atomic write to ~/.claude/tmp/.json. Creates the dir if missing. + * Uses a tmp-file + rename so a crash never leaves a half-written target. */ export async function writeState(name: string, data: unknown): Promise { await mkdir(TMP_DIR, { recursive: true }); - await writeFile(join(TMP_DIR, name), JSON.stringify(data)); + const target = join(TMP_DIR, name); + const tmp = join(TMP_DIR, `.${process.pid}-${Date.now()}.tmp`); + await writeFile(tmp, JSON.stringify(data)); + await rename(tmp, target); } /** Parse the TOOL_INPUT env JSON blob (the full tool input Claude Code passes diff --git a/src/lib/hooks-fingerprint.ts b/src/lib/hooks-fingerprint.ts index 3542c90..a23188c 100644 --- a/src/lib/hooks-fingerprint.ts +++ b/src/lib/hooks-fingerprint.ts @@ -14,8 +14,29 @@ import { readdir, readFile } from "node:fs/promises"; import { homedir } from "node:os"; import { join } from "node:path"; import { CryptoHasher } from "bun"; +import { z } from "zod"; +import { iterCommandHooks } from "./hook-command.ts"; import { atomicWriteJson } from "./json-io.ts"; +// `installedAt` is echoed verbatim into the terminal warning banner, and the +// fingerprint file is exactly what the Shai-Hulud threat model lets an attacker +// rewrite — so a crafted value could inject ANSI escapes to disguise the alarm. +// Strip control characters (incl. ESC) on read. +const stripControl = (s: string): string => + Array.from(s) + .filter((c) => { + const n = c.charCodeAt(0); + return n > 0x1f && n !== 0x7f && (n < 0x80 || n > 0x9f); // drop C0/C1 controls + }) + .join(""); + +// Zod schema for the fingerprint record written to disk. +const FingerprintRecordSchema = z.object({ + hash: z.string().min(1), + installedAt: z.string().default("").transform(stripControl), + hooksCount: z.number().int().nonnegative().default(0), +}); + export const FINGERPRINT_FILENAME = ".cc-settings-hooks-fingerprint"; // Stable JSON serialization: sort object keys recursively, no whitespace. @@ -52,13 +73,13 @@ export async function readFingerprint(claudeDir?: string): Promise; - if (typeof parsed.hash !== "string") return null; - return { - hash: parsed.hash, - installedAt: parsed.installedAt ?? "", - hooksCount: typeof parsed.hooksCount === "number" ? parsed.hooksCount : 0, - }; + const raw = JSON.parse(text); + // Validate through the zod schema. Mirrors status.ts:35 — field values are + // echoed into the session-start warning banner, so they must be validated + // strings, not trusted as-cast. On failure return null (treat as missing). + const result = FingerprintRecordSchema.safeParse(raw); + if (!result.success) return null; + return result.data; } catch { return null; } @@ -71,20 +92,9 @@ export async function writeFingerprint( const dir = claudeDir ?? join(homedir(), ".claude"); const path = join(dir, FINGERPRINT_FILENAME); - const hooks = - settings && typeof settings === "object" - ? ((settings as Record).hooks ?? {}) - : {}; - let hooksCount = 0; - if (hooks && typeof hooks === "object") { - for (const groups of Object.values(hooks as Record)) { - if (!Array.isArray(groups)) continue; - for (const group of groups) { - const entries = (group as { hooks?: unknown[] })?.hooks; - if (Array.isArray(entries)) hooksCount += entries.length; - } - } - } + // Count command hooks via the shared iterCommandHooks walk (fail-open: + // never throws on malformed input). This replaces the hand-rolled walk. + const hooksCount = [...iterCommandHooks(settings)].length; const record: FingerprintRecord = { hash: hashHooks(settings), @@ -218,7 +228,16 @@ export async function readSrcManifest(claudeDir?: string): Promise = {}; + for (const [rel, hash] of Object.entries(parsed.files)) { + if (typeof hash !== "string") return null; + if (rel.startsWith("/") || rel.split(/[/\\]/).includes("..")) return null; + files[rel] = hash; + } + return { files, installedAt: stripControl(parsed.installedAt ?? "") }; } catch { return null; } diff --git a/src/lib/light-profile.ts b/src/lib/light-profile.ts index 674b4aa..0863c38 100644 --- a/src/lib/light-profile.ts +++ b/src/lib/light-profile.ts @@ -12,6 +12,7 @@ // - src/setup.ts — filters file copies, applies transform before staging // - tests/light-profile.test.ts — parity guard + transform units +import { isManagedHookCommand } from "./hook-command.ts"; import { asRecord, subtractByKey } from "./merge-keyed.ts"; export type Profile = "full" | "light"; @@ -166,13 +167,12 @@ export function stripManagedSettings( // a) Its JSON string matches any group in the full baseline (keyed // subtraction below), OR // b) ALL hook commands in the group reference a cc-settings script path. - const CC_SCRIPT_PATHS = ["/.claude/src/hooks/", "/.claude/src/scripts/"]; - const isCcScriptCommand = (cmd: unknown): boolean => - typeof cmd === "string" && CC_SCRIPT_PATHS.some((p) => cmd.includes(p)); + // Uses isManagedHookCommand (hook-command.ts) as the single source of truth: + // (scripts|hooks) only, lib/ excluded — tightened in nuclear-review. const isAllCcScriptGroup = (group: unknown): boolean => { const g = group as { hooks?: Array<{ command?: unknown }> }; if (Array.isArray(g.hooks) && g.hooks.length > 0) { - return g.hooks.every((h) => isCcScriptCommand(h.command)); + return g.hooks.every((h) => typeof h.command === "string" && isManagedHookCommand(h.command)); } return false; }; diff --git a/src/lib/lint-frontmatter.ts b/src/lib/lint-frontmatter.ts new file mode 100644 index 0000000..f17ed66 --- /dev/null +++ b/src/lib/lint-frontmatter.ts @@ -0,0 +1,67 @@ +// Shared scaffolding for frontmatter-based linters. +// +// Handles the mechanical per-file boilerplate that both lint-skills and +// lint-knowledge duplicate: +// 1. Extract the `---`-delimited YAML block (report "frontmatter-missing"). +// 2. Parse with strict YAML (report "yaml-parse" errors per error object). +// 3. Delegate to the caller's domain-specific logic via `onParsed`. +// +// The public types exposed here are intentionally minimal — callers map +// BaseFinding into their own domain-specific finding types (LintFinding, +// KnowledgeFinding) which carry a domain-specific item-name field. + +import { extractFrontmatterBlock, parseFrontmatterStrict } from "./frontmatter.ts"; + +/** Severity shared across all linting domains. */ +export type LintSeverity = "error" | "warning"; + +/** Minimal finding shape: severity + rule + message, without any item field. */ +export interface BaseFinding { + severity: LintSeverity; + rule: string; + message: string; +} + +/** + * Run the shared frontmatter scaffolding on a single file's text content. + * + * Returns an array of `BaseFinding`s for the scaffolding rules + * (`frontmatter-missing`, `yaml-parse`). When those pass, calls `onParsed` + * with the parsed YAML object so the caller can apply domain-specific rules + * and append their own findings. + * + * @param text Full file text (UTF-8). + * @param onParsed Called with the successfully parsed YAML object. May be + * sync or async. Return `[]` for no additional findings. + */ +export async function lintFrontmatterCore( + text: string, + onParsed: (data: unknown) => BaseFinding[] | Promise, +): Promise { + const findings: BaseFinding[] = []; + + const block = extractFrontmatterBlock(text); + if (!block) { + findings.push({ + severity: "error", + rule: "frontmatter-missing", + message: "no `---`-delimited YAML frontmatter at top of file", + }); + return findings; + } + + const { data: parsed, errors: yamlErrors } = parseFrontmatterStrict(text); + if (yamlErrors.length > 0) { + for (const e of yamlErrors) { + findings.push({ + severity: "error", + rule: "yaml-parse", + message: `${e.code ?? "YAML"} at line ${e.line ?? "?"}, col ${e.col ?? "?"}: ${e.message}`, + }); + } + return findings; + } + + findings.push(...(await onParsed(parsed))); + return findings; +} diff --git a/src/lib/lint-knowledge.ts b/src/lib/lint-knowledge.ts index a9c2d99..788eaab 100644 --- a/src/lib/lint-knowledge.ts +++ b/src/lib/lint-knowledge.ts @@ -9,7 +9,7 @@ import { existsSync } from "node:fs"; import { readdir, readFile } from "node:fs/promises"; import { basename, extname, join } from "node:path"; import { KnowledgeFrontmatter } from "../schemas/knowledge.ts"; -import { extractFrontmatterBlock, parseFrontmatterStrict } from "./frontmatter.ts"; +import { lintFrontmatterCore } from "./lint-frontmatter.ts"; export type KnowledgeSeverity = "error" | "warning"; @@ -43,76 +43,65 @@ async function lintOne( const stem = basename(filename, extname(filename)); const notePath = join(dir, filename); + // Helper: attach the note filename to a base finding. + const push = (severity: KnowledgeSeverity, rule: string, message: string) => { + findings.push({ note: filename, severity, rule, message }); + }; + const text = await readFile(notePath, "utf8"); - const block = extractFrontmatterBlock(text); - - if (!block) { - findings.push({ - note: filename, - severity: "error", - rule: "frontmatter-missing", - message: "no `---`-delimited YAML frontmatter at top of file", - }); - return findings; - } - const { data: parsed, errors: yamlErrors } = parseFrontmatterStrict(text); - if (yamlErrors.length > 0) { - for (const e of yamlErrors) { - findings.push({ - note: filename, - severity: "error", - rule: "yaml-parse", - message: `${e.code ?? "YAML"} at line ${e.line ?? "?"}, col ${e.col ?? "?"}: ${e.message}`, - }); + // Shared scaffolding: frontmatter-missing + yaml-parse errors. + const baseFindings = await lintFrontmatterCore(text, (parsed) => { + const domainFindings: Array<{ severity: KnowledgeSeverity; rule: string; message: string }> = + []; + + const result = KnowledgeFrontmatter.safeParse(parsed); + if (!result.success) { + for (const issue of result.error.issues) { + domainFindings.push({ + severity: "error", + rule: "schema", + message: `${issue.path.join(".") || "(root)"}: ${issue.message}`, + }); + } + return domainFindings; } - return findings; - } - const result = KnowledgeFrontmatter.safeParse(parsed); - if (!result.success) { - for (const issue of result.error.issues) { - findings.push({ - note: filename, + const fm = result.data; + + // name must equal the filename stem. + if (fm.name !== stem) { + domainFindings.push({ severity: "error", - rule: "schema", - message: `${issue.path.join(".") || "(root)"}: ${issue.message}`, + rule: "name-filename-mismatch", + message: `frontmatter name "${fm.name}" does not match filename stem "${stem}"`, }); } - return findings; - } - const fm = result.data; + // supersedes must reference a name that exists in the directory. + if (fm.supersedes !== undefined && !allNames.has(fm.supersedes)) { + domainFindings.push({ + severity: "warning", + rule: "supersedes-unknown", + message: `supersedes "${fm.supersedes}" does not match any note name in this directory`, + }); + } - // name must equal the filename stem. - if (fm.name !== stem) { - findings.push({ - note: filename, - severity: "error", - rule: "name-filename-mismatch", - message: `frontmatter name "${fm.name}" does not match filename stem "${stem}"`, - }); - } + // Body must be non-empty (meaningful content after frontmatter). + const body = bodyAfterFrontmatter(text).trim(); + if (!body) { + domainFindings.push({ + severity: "error", + rule: "empty-body", + message: "note body is empty — add what/why/how-to-apply content", + }); + } - // supersedes must reference a name that exists in the directory. - if (fm.supersedes !== undefined && !allNames.has(fm.supersedes)) { - findings.push({ - note: filename, - severity: "warning", - rule: "supersedes-unknown", - message: `supersedes "${fm.supersedes}" does not match any note name in this directory`, - }); - } + return domainFindings; + }); - // Body must be non-empty (meaningful content after frontmatter). - const body = bodyAfterFrontmatter(text).trim(); - if (!body) { - findings.push({ - note: filename, - severity: "error", - rule: "empty-body", - message: "note body is empty — add what/why/how-to-apply content", - }); + for (const f of baseFindings) { + push(f.severity, f.rule, f.message); } return findings; diff --git a/src/lib/lint-skills.ts b/src/lib/lint-skills.ts index fd50b1a..409910f 100644 --- a/src/lib/lint-skills.ts +++ b/src/lib/lint-skills.ts @@ -10,7 +10,8 @@ import { existsSync } from "node:fs"; import { readdir, readFile } from "node:fs/promises"; import { join } from "node:path"; import { SkillFrontmatter } from "../schemas/skill.ts"; -import { extractFrontmatterBlock, parseFrontmatterStrict } from "./frontmatter.ts"; +import { lintFrontmatterCore } from "./lint-frontmatter.ts"; +import { ACTIVE_SKILLS } from "./managed-skills.ts"; export type SkillSeverity = "error" | "warning"; @@ -48,146 +49,124 @@ async function lintOne(skillsDir: string, name: string): Promise const dir = join(skillsDir, name); const skillPath = join(dir, "SKILL.md"); + // Helper: attach the skill name to a base finding. + const push = (severity: SkillSeverity, rule: string, message: string) => { + findings.push({ skill: name, severity, rule, message }); + }; + if (!KEBAB_CASE.test(name)) { - findings.push({ - skill: name, - severity: "error", - rule: "folder-kebab-case", - message: `folder "${name}" is not kebab-case (allowed: a-z, 0-9, -)`, - }); + push("error", "folder-kebab-case", `folder "${name}" is not kebab-case (allowed: a-z, 0-9, -)`); } for (const prefix of RESERVED_PREFIXES) { if (name === prefix || name.startsWith(`${prefix}-`)) { - findings.push({ - skill: name, - severity: "error", - rule: "reserved-name", - message: `name "${name}" uses reserved prefix "${prefix}" — Claude.ai rejects these`, - }); + push( + "error", + "reserved-name", + `name "${name}" uses reserved prefix "${prefix}" — Claude.ai rejects these`, + ); } } // The guide is explicit: no README.md inside a skill folder. All docs go in // SKILL.md or references/. if (existsSync(join(dir, "README.md"))) { - findings.push({ - skill: name, - severity: "error", - rule: "no-readme-inside", - message: "README.md found inside skill folder — move docs to SKILL.md or references/", - }); + push( + "error", + "no-readme-inside", + "README.md found inside skill folder — move docs to SKILL.md or references/", + ); } if (!existsSync(skillPath)) { - findings.push({ - skill: name, - severity: "error", - rule: "skill-md-missing", - message: "SKILL.md not found (exact case required)", - }); + push("error", "skill-md-missing", "SKILL.md not found (exact case required)"); return findings; } const text = await readFile(skillPath, "utf8"); - const block = extractFrontmatterBlock(text); - - if (!block) { - findings.push({ - skill: name, - severity: "error", - rule: "frontmatter-missing", - message: "no `---`-delimited YAML frontmatter at top of file", - }); - return findings; - } // Raw-text scan: catches angle brackets in any field, including passthrough // ones like argument-hint where the schema doesn't validate the value. + // Must run before frontmatter parsing so we scan the raw block. + const block = text.match(/^---\r?\n([\s\S]*?)\r?\n---/)?.[1] ?? ""; if (/[<>]/.test(block)) { - findings.push({ - skill: name, - severity: "error", - rule: "no-angle-brackets", - message: - "frontmatter contains `<` or `>` — security restriction, frontmatter is injected into the system prompt", - }); - } + push( + "error", + "no-angle-brackets", + "frontmatter contains `<` or `>` — security restriction, frontmatter is injected into the system prompt", + ); + } + + // Shared scaffolding: frontmatter-missing + yaml-parse errors. + const baseFindings = await lintFrontmatterCore(text, (parsed) => { + const domainFindings: Array<{ severity: SkillSeverity; rule: string; message: string }> = []; + + const result = SkillFrontmatter.safeParse(parsed); + if (!result.success) { + for (const issue of result.error.issues) { + domainFindings.push({ + severity: "error", + rule: "schema", + message: `${issue.path.join(".") || "(root)"}: ${issue.message}`, + }); + } + return domainFindings; + } - const { data: parsed, errors: yamlErrors } = parseFrontmatterStrict(text); - if (yamlErrors.length > 0) { - for (const e of yamlErrors) { - findings.push({ - skill: name, + const fm = result.data; + + if (fm.name !== name) { + domainFindings.push({ severity: "error", - rule: "yaml-parse", - message: `${e.code ?? "YAML"} at line ${e.line ?? "?"}, col ${e.col ?? "?"}: ${e.message}`, + rule: "name-folder-mismatch", + message: `frontmatter name "${fm.name}" does not match folder "${name}"`, }); } - return findings; - } - const result = SkillFrontmatter.safeParse(parsed); - if (!result.success) { - for (const issue of result.error.issues) { - findings.push({ - skill: name, + const desc = fm.description; + + // Guide: under 1024 chars. Hard limit (Claude.ai upload rejects past this). + if (desc.length > 1024) { + domainFindings.push({ severity: "error", - rule: "schema", - message: `${issue.path.join(".") || "(root)"}: ${issue.message}`, + rule: "description-too-long", + message: `description is ${desc.length} chars (max 1024)`, }); } - return findings; - } - const fm = result.data; + // Guide examples of "too vague" descriptions hover around 25-40 chars + // ("Helps with projects."). 50 is a soft floor — flag as warning, don't block. + if (desc.length < 50) { + domainFindings.push({ + severity: "warning", + rule: "description-too-short", + message: `description is only ${desc.length} chars — likely too vague to trigger reliably`, + }); + } - if (fm.name !== name) { - findings.push({ - skill: name, - severity: "error", - rule: "name-folder-mismatch", - message: `frontmatter name "${fm.name}" does not match folder "${name}"`, - }); - } + if (!TRIGGER_PATTERN.test(desc)) { + domainFindings.push({ + severity: "warning", + rule: "description-no-trigger-language", + message: + "description has no trigger language (`Triggers`, `Use when`, `Use for`, …) — the model can't tell when to load it", + }); + } - const desc = fm.description; + return domainFindings; + }); - // Guide: under 1024 chars. Hard limit (Claude.ai upload rejects past this). - if (desc.length > 1024) { - findings.push({ - skill: name, - severity: "error", - rule: "description-too-long", - message: `description is ${desc.length} chars (max 1024)`, - }); - } - - // Guide examples of "too vague" descriptions hover around 25-40 chars - // ("Helps with projects."). 50 is a soft floor — flag as warning, don't block. - if (desc.length < 50) { - findings.push({ - skill: name, - severity: "warning", - rule: "description-too-short", - message: `description is only ${desc.length} chars — likely too vague to trigger reliably`, - }); - } - - if (!TRIGGER_PATTERN.test(desc)) { - findings.push({ - skill: name, - severity: "warning", - rule: "description-no-trigger-language", - message: - "description has no trigger language (`Triggers`, `Use when`, `Use for`, …) — the model can't tell when to load it", - }); + for (const f of baseFindings) { + findings.push({ skill: name, severity: f.severity, rule: f.rule, message: f.message }); } return findings; } -export async function lintSkillsDir(skillsDir: string): Promise { +export async function lintSkillsDir( + skillsDir: string, + opts: { checkManaged?: boolean } = {}, +): Promise { if (!existsSync(skillsDir)) { return { findings: [], skillCount: 0 }; } @@ -214,6 +193,38 @@ export async function lintSkillsDir(skillsDir: string): Promise { }); } + // ACTIVE_SKILLS must match skills/ on disk exactly. A skill present on disk but + // absent from ACTIVE_SKILLS won't be pruned on a full→light switch (cleanOldConfig + // iterates ACTIVE_SKILLS); an ACTIVE_SKILLS entry with no directory is stale. + // Repo-level invariant — only meaningful against the canonical skills/ dir, so + // it is opt-in (the CLI enables it for the default repo run, not custom dirs). + if (opts.checkManaged) { + const onDisk = new Set(skillNames); + const active = new Set(ACTIVE_SKILLS); + for (const name of skillNames) { + if (!active.has(name)) { + findings.push({ + skill: name, + severity: "error", + rule: "managed-skills-missing", + message: + "present in skills/ but missing from ACTIVE_SKILLS (src/lib/managed-skills.ts) — the installer won't prune it on a full→light switch", + }); + } + } + for (const name of ACTIVE_SKILLS) { + if (!onDisk.has(name)) { + findings.push({ + skill: name, + severity: "error", + rule: "managed-skills-stale", + message: + "listed in ACTIVE_SKILLS but skills// does not exist — remove it or move it to TOMBSTONE_SKILLS", + }); + } + } + } + return { findings, skillCount: skillNames.length }; } diff --git a/src/lib/managed-skills.ts b/src/lib/managed-skills.ts index 5c1fb26..ccfe585 100644 --- a/src/lib/managed-skills.ts +++ b/src/lib/managed-skills.ts @@ -15,14 +15,25 @@ // Imported by: // - src/setup.ts — to know what to wipe + reinstall // - src/lib/status.ts — to compute present/missing for `cc status` +// - src/lib/lint-skills.ts — to assert ACTIVE_SKILLS matches skills/ on disk // // Previously duplicated across both files; the duplication caused real drift // risk (every new skill required edits in two places). -export const MANAGED_SKILLS = [ +// +// INVARIANT: ACTIVE_SKILLS must list exactly the directories under skills/. +// lint-skills enforces this — a skill added to skills/ without an ACTIVE_SKILLS +// entry fails `bun run lint:skills`. (This replaces the old installer-side prune +// loop that re-read skills/ directly to cover the gap; the gap is now a lint +// error instead of silent install drift.) + +/** Currently shipped skills — present in skills//SKILL.md, installed to + * ~/.claude/skills//. Keep in sync with the skills/ directory. */ +export const ACTIVE_SKILLS = [ "autoresearch", "build", "cc", "checkpoint", + "codex", "component", "consolidate", "context-doc", @@ -30,24 +41,36 @@ export const MANAGED_SKILLS = [ "dr-init", "explore", "fix", + "freeze", "handoff", "hook", "lighthouse", "nuclear-review", "oracle", "orchestrate", + "plan-ceo-review", "plan-feature", "project", + "proof-of-work", "qa", "refactor", + "retro", "review", + "review-batch", "share-learning", "ship", + "strategist", "test", "tldr", "verify", "zero-tech-debt", - // Kept for upgrade cleanup only — these skills were removed and must be wiped on re-install. +]; + +/** Upgrade-cleanup tombstones — skills removed in prior releases. Kept so a + * re-install on an older user prunes the obsolete directories. Do not remove; + * they are load-bearing for upgrades. When reviving one, move it to + * ACTIVE_SKILLS and recreate its skills// directory. */ +export const TOMBSTONE_SKILLS = [ "ask", "audit", "cc-sync", @@ -75,3 +98,7 @@ export const MANAGED_SKILLS = [ "write-a-skill", "zoom-out", ]; + +/** Everything the installer may wipe + reinstall: active skills plus tombstones + * to prune. Consumers that only care about the wipe set use this. */ +export const MANAGED_SKILLS = [...ACTIVE_SKILLS, ...TOMBSTONE_SKILLS]; diff --git a/src/lib/mcp.ts b/src/lib/mcp.ts index 4313b2f..4492f33 100644 --- a/src/lib/mcp.ts +++ b/src/lib/mcp.ts @@ -14,11 +14,18 @@ // Responsibilities of this file: // 1. User-only server detection (findUserOnlyServers) // 2. User-server preservation prompt (promptPreserveUserServers) -// 3. ~/.claude.json installation (installMcpToClaudeJson) and removal of +// 3. MCP-preservation resolution (resolveMcpServers) — computes the final +// merged mcpServers object (team base + any user-only extras the user +// chose to keep). Extracted from the old settings-merge.ts orchestrator so +// settings-merge.ts stays free of MCP knowledge. +// 4. settings.json merge entry point (mergeSettingsWithMcpPreservation) — the +// thin wrapper that calls resolveMcpServers then delegates to the pure +// mergeSettings function in settings-merge.ts. +// 5. ~/.claude.json installation (installMcpToClaudeJson) and removal of // cc-settings-managed servers on light installs (removeManagedMcpServers) // -// Generic JSON/atomic-file I/O moved to src/lib/json-io.ts; the settings.json -// merge strategies + orchestrator moved to src/lib/settings-merge.ts. +// Generic JSON/atomic-file I/O moved to src/lib/json-io.ts; the pure settings.json +// merge strategies + orchestrator live in src/lib/settings-merge.ts. import { homedir } from "node:os"; import { join } from "node:path"; @@ -29,6 +36,7 @@ import { debug, error, info, success, warn } from "./colors.ts"; import { atomicWriteJson, readJsonOrNull } from "./json-io.ts"; import { asRecord, subtractByKey } from "./merge-keyed.ts"; import { promptYn } from "./prompts.ts"; +import { type MergeOptions, mergeSettings } from "./settings-merge.ts"; type McpServer = z.infer[string]; export type McpServers = Record; @@ -174,3 +182,82 @@ export async function removeManagedMcpServers( } await atomicWriteJson(claudeJsonPath, updated); } + +// --- MCP-preserving settings.json merge ---------------------------------- + +/** + * Compute the final merged mcpServers value from a user's existing servers and + * the team's baseline servers. This encapsulates the MCP-preservation semantics + * that used to live inline in the settings merger: + * + * - Team servers form the base. + * - Servers present in the user's settings but absent from the team config + * are "user-only" extras. The user is prompted to keep or drop them + * (or CC_WIPE_CUSTOM_MCP=1 drops them silently). + * - Kept user-only servers are overlaid onto the team base. Team wins on any + * key that exists in both (shared server names use the team definition). + * + * @param userServers McpServers extracted from the user's existing settings.json + * @param teamServers McpServers from the composed team config (already validated) + * @returns The resolved McpServers to write into the output file + */ +export async function resolveMcpServers( + userServers: McpServers, + teamServers: McpServers, +): Promise { + const userOnly = findUserOnlyServers(userServers, teamServers); + let preserved: McpServers = {}; + if (userOnly.length > 0) { + ({ preserved } = await promptPreserveUserServers(userOnly, userServers)); + } else { + debug("No user-only MCP servers"); + } + // Team is the base; user-only preserved extras are overlaid. + return { ...teamServers, ...preserved }; +} + +/** + * Merge user's existing settings.json with the in-memory team settings object, + * preserving user-only MCP servers via an interactive (or CC_WIPE_CUSTOM_MCP=1) + * preservation prompt. + * + * This is the thin orchestration wrapper that: + * 1. Reads + parses the user's mcpServers from the existing settings.json. + * 2. Calls resolveMcpServers to compute the final merged mcpServers. + * 3. Delegates the full settings merge to the pure `mergeSettings` function in + * settings-merge.ts, passing the resolved mcpServers so the pure merger + * doesn't need MCP knowledge. + * + * The observable output is byte-identical to the old combined function — same + * servers preserved, same precedence, same _status handling. + */ +export async function mergeSettingsWithMcpPreservation( + existingPath: string, + teamSettings: Record, + outputPath: string, + opts: MergeOptions = {}, +): Promise { + // Peek at the user's existing file to extract current mcpServers so we can + // run the preservation prompt before the per-key merge loop. + // readJsonOrNull throws on unparseable JSON (JsonParseError) — honored here + // so bad JSON always aborts rather than silently wiping user MCP config. + const userRaw = (await readJsonOrNull(existingPath)) as Record | null; + + if (!userRaw) { + // No existing file — delegate directly; the pure merger writes team as-is. + await mergeSettings(existingPath, teamSettings, outputPath, opts); + return; + } + + // asRecord: a corrupt string-valued mcpServers degrades to {} instead of + // leaking a string into the server merge. + const userServers = asRecord(userRaw.mcpServers) as McpServers; + const teamServers = asRecord(teamSettings.mcpServers) as McpServers; + + const resolvedMcp = await resolveMcpServers(userServers, teamServers); + + // Delegate to the pure merger, supplying the already-resolved mcpServers. + // The pure merger skips the mcpServers key in its per-key strategy loop and + // uses the value we computed here instead. + await mergeSettings(existingPath, teamSettings, outputPath, opts, resolvedMcp); +} diff --git a/src/lib/settings-merge.ts b/src/lib/settings-merge.ts index c8d22a9..a45f9b5 100644 --- a/src/lib/settings-merge.ts +++ b/src/lib/settings-merge.ts @@ -1,25 +1,26 @@ // Settings.json merger — strategy-based merge of team + user settings.json. // // Extracted from src/lib/mcp.ts (was responsibility #4 of 5 in that file). -// The orchestrator `mergeSettingsWithMcpPreservation` and its 5 strategies are -// now individually exported so they can be unit-tested in isolation. +// The orchestrator `mergeSettings` and its 5 strategies are now individually +// exported so they can be unit-tested in isolation. +// +// MCP-preservation semantics live in src/lib/mcp.ts as `resolveMcpServers`. +// The thin wrapper `mergeSettingsWithMcpPreservation` (which computes +// resolveMcpServers then calls mergeSettings) lives there too — keeping this +// file free of MCP knowledge. // // For the full merge algorithm and invariant documentation see the -// JSDoc on `mergeSettingsWithMcpPreservation` below. +// JSDoc on `mergeSettings` below. -import type { z } from "zod"; -import type { McpServers as McpServersSchema } from "../schemas/mcp.ts"; import { Settings } from "../schemas/settings.ts"; import { debug, info, success } from "./colors.ts"; +import { isManagedHookCommand } from "./hook-command.ts"; import { atomicWriteJson, readJsonOrNull } from "./json-io.ts"; -import { findUserOnlyServers, promptPreserveUserServers } from "./mcp.ts"; import { asRecord, canonicalKey, subtractByKey, unionByKey, uniqueByKey } from "./merge-keyed.ts"; import { promptYn } from "./prompts.ts"; type StringArray = string[] | undefined; type UnknownRecord = Record; -type McpServer = z.infer[string]; -type McpServers = Record; /** * Merge policy options. Interactive mode only asks where auto-merge has a real @@ -259,10 +260,11 @@ function commandsOf(group: unknown): string[] { .filter((c): c is string => typeof c === "string"); } -// cc-settings-managed hook implementations all live under ~/.claude/src/. -// User-authored custom hooks point elsewhere (their own scripts). +// cc-settings-managed hook implementations live under ~/.claude/src/{scripts,hooks}/. +// Delegates to the single source of truth in hook-command.ts (scripts|hooks only, +// lib/ excluded — tightened in nuclear-review). function isManagedCommand(command: string): boolean { - return /[/\\]\.claude[/\\]src[/\\]/.test(command); + return isManagedHookCommand(command); } // A group is deprecated only if every hook inside it is deprecated. Mixed @@ -454,17 +456,21 @@ export const STRATEGIES: Record = { hooks: hooksStrategy, env: envStrategy, statusLine: statusLineStrategy, - // mcpServers is handled separately — it needs the user-server preservation - // prompt run BEFORE the per-key loop (the prompt is shared across the whole - // settings.json merge, not just the mcpServers field). + // mcpServers is handled by the caller (resolveMcpServers in mcp.ts) before + // this function is invoked — it is excluded from the per-key strategy loop. }; -// --- Orchestrator -------------------------------------------------------- +// --- Pure orchestrator --------------------------------------------------- /** - * Merge user's existing settings.json with the in-memory team settings object - * (the composed config/ fragments — already schema-validated by composeSettings, - * so there is no team-side file read or re-validation here). + * Pure settings merge: reads existingPath, merges with teamSettings using the + * per-key strategy table, writes the result to outputPath. + * + * `resolvedMcpServers` is the already-computed mcpServers value (team base + + * any preserved user-only extras). The caller is responsible for running + * `resolveMcpServers` (from src/lib/mcp.ts) before calling this function and + * passing the result here. When undefined, mcpServers is merged via the + * userWinsScalarStrategy fallback like any other unknown key. * * Non-interactive policy (default): * - User-declared keys win for top-level scalars (model, theme, …). @@ -479,26 +485,30 @@ export const STRATEGIES: Record = { * - `env` shallow-merges with user values winning. * - `statusLine` user wins, except when the user's command targets a * removed cc-settings script (then reset to team). - * - `mcpServers` uses the interactive preservation prompt. + * - `mcpServers` is injected from resolvedMcpServers (caller responsibility). * * Interactive policy (`opts.interactive`): * - Scalar conflicts prompt "keep your value / take team's". * - Team-added permission rules (allow/ask) and hook groups prompt "adopt / skip". * - `deny` rules and user-only entries stay automatic (guardrails / additive). */ -export async function mergeSettingsWithMcpPreservation( +export async function mergeSettings( existingPath: string, teamSettings: Record, outputPath: string, opts: MergeOptions = {}, + resolvedMcpServers?: Record, ): Promise { - const teamRaw: UnknownRecord = teamSettings; - const userRaw = (await readJsonOrNull(existingPath)) as UnknownRecord | null; - // No existing file → write team as-is (atomic). + // No existing file → write team as-is (atomic), injecting resolvedMcpServers + // if provided (callers may have already resolved servers even for a fresh install). if (!userRaw) { - await atomicWriteJson(outputPath, teamRaw); + const out = + resolvedMcpServers !== undefined + ? { ...teamSettings, mcpServers: resolvedMcpServers } + : teamSettings; + await atomicWriteJson(outputPath, out); return; } @@ -506,7 +516,7 @@ export async function mergeSettingsWithMcpPreservation( // message and proceed with the raw object — forward-compat safety: a new // Claude Code settings key not yet in the schema must not block the merger. // teamSettings needs no validation here: composeSettings already - // schema-checked the composed fragments and throws on failure. + // schema-checked the composed config/ fragments and throws on failure. const userValidation = Settings.safeParse(userRaw); if (!userValidation.success) { const issues = userValidation.error.issues @@ -515,20 +525,6 @@ export async function mergeSettingsWithMcpPreservation( debug(`User settings.json failed schema validation (proceeding with raw): ${issues}`); } - // mcpServers needs the preservation prompt to run BEFORE the per-key loop — - // the prompt is shared across the whole merge, not scoped to one strategy. - // asRecord: a corrupt string-valued mcpServers degrades to {} instead of - // leaking a string into the server merge. - const userServers = asRecord(userRaw.mcpServers) as McpServers; - const teamServers = asRecord(teamRaw.mcpServers) as McpServers; - const userOnly = findUserOnlyServers(userServers, teamServers); - let preserved: McpServers = {}; - if (userOnly.length > 0) { - ({ preserved } = await promptPreserveUserServers(userOnly, userServers)); - } else { - debug("No user-only MCP servers"); - } - const ctx: StrategyContext = { opts, accounting: { @@ -550,15 +546,18 @@ export async function mergeSettingsWithMcpPreservation( // Walk every key in (team ∪ user). Strategy table picks the merge logic; // unknown keys fall through to user-wins-scalar. const merged: UnknownRecord = {}; - const allKeys = new Set([...Object.keys(teamRaw), ...Object.keys(userRaw)]); + const allKeys = new Set([...Object.keys(teamSettings), ...Object.keys(userRaw)]); - // mcpServers gets a fixed result based on the prompt outcome above. - merged.mcpServers = { ...teamServers, ...preserved }; - allKeys.delete("mcpServers"); + // mcpServers: injected from the pre-resolved value when provided; otherwise + // falls through to the userWinsScalarStrategy like any unknown key. + if (resolvedMcpServers !== undefined) { + merged.mcpServers = resolvedMcpServers; + allKeys.delete("mcpServers"); + } for (const key of allKeys) { const strategy = STRATEGIES[key] ?? userWinsScalarStrategy; - const result = await strategy(key, teamRaw[key], userRaw[key], ctx); + const result = await strategy(key, teamSettings[key], userRaw[key], ctx); if (result.keep) merged[key] = result.value; } diff --git a/src/scripts/claude-audit.ts b/src/scripts/claude-audit.ts index b243208..a0d5a80 100644 --- a/src/scripts/claude-audit.ts +++ b/src/scripts/claude-audit.ts @@ -103,50 +103,117 @@ function sortDesc(m: Map): Array<[string, number]> { return [...m.entries()].sort((a, b) => b[1] - a[1]); } -// --- Analyze -------------------------------------------------------------- +// --- AuditModel ----------------------------------------------------------- + +/** A classified command line. */ +type ClassifiedLine = LogLine & { klass: string }; + +/** Per-group aggregation for command categories. */ +interface CommandGroup { + name: string; + count: number; + /** Top details within this group (up to 6). */ + details: Array<{ name: string; count: number }>; + detailMax: number; +} + +/** A security flag. */ +interface SecurityFlag { + time: string; + desc: string; + cmd: string; +} + +/** A session (15-min idle threshold). */ +interface AuditSession { + start: string; + end: string; + count: number; + domProject: string; + durationMs: number; +} + +/** A context-heavy operation hit. */ +interface CtxHit { + time: string; + cmd: string; + reason: string; +} -function analyze(label: string, files: string[]): void { +/** + * Fully structured audit data — no formatting, no I/O. + * Produced by `analyzeCommands`, consumed by `renderAudit`. + */ +export interface AuditModel { + label: string; + totalCommands: number; + /** null when there are no commands (empty data case). */ + data: { + /** Command groups sorted descending by count. */ + groups: CommandGroup[]; + globalMax: number; + /** Project distribution sorted descending by count. */ + projects: Array<{ name: string; count: number }>; + projectMax: number; + securityFlags: SecurityFlag[]; + /** Exact commands repeated 3+ times, sorted descending, top 5. */ + repeated: Array<{ cmd: string; count: number }>; + /** Top 3 sessions by command count. */ + topSessions: AuditSession[]; + totalSessions: number; + ctxHits: CtxHit[]; + } | null; +} + +// --- analyzeCommands (pure data stage) ------------------------------------ + +function timeToMs(t: string): number { + const parts = t.split(":").map(Number); + return ((parts[0] ?? 0) * 3600 + (parts[1] ?? 0) * 60 + (parts[2] ?? 0)) * 1000; +} + +/** + * Parse and classify log lines, aggregate all sections into an `AuditModel`. + * Pure function — no console output, no side effects. + */ +export function analyzeCommands(label: string, files: string[]): AuditModel { const lines = parseLines(files); if (lines.length === 0) { - console.log(` ${label}: no data`); - return; + return { label, totalCommands: 0, data: null }; } - console.log(` ${label} (${lines.length} commands)`); - const classified = lines.map((l) => ({ ...l, klass: classifyBashCommand(l.cmd) })); - const groups = countBy(classified, (v) => v.klass.split(":")[0] ?? "other"); - const sortedGroups = sortDesc(groups); + const classified: ClassifiedLine[] = lines.map((l) => ({ + ...l, + klass: classifyBashCommand(l.cmd), + })); + + // --- Command groups --- + const groupCounts = countBy(classified, (v) => v.klass.split(":")[0] ?? "other"); + const sortedGroups = sortDesc(groupCounts); const globalMax = sortedGroups[0]?.[1] ?? 0; - for (const [gname, gcount] of sortedGroups) { - console.log(""); - console.log(` ${padRight(gname, 24)} ${padBar(gcount, globalMax, 12)} ${gcount}`); - const details = countBy( + const groups: CommandGroup[] = sortedGroups.map(([gname, gcount]) => { + const detailMap = countBy( classified.filter((v) => v.klass.startsWith(`${gname}:`)), (v) => v.klass.slice(gname.length + 1), ); - const sorted = sortDesc(details); - const dmax = sorted[0]?.[1] ?? 0; - for (const [dname, dcount] of sorted.slice(0, 6)) { - console.log(` ${padRight(dname, 22)} ${padBar(dcount, dmax, 8)} ${dcount}`); - } - } + const sortedDetails = sortDesc(detailMap); + const dmax = sortedDetails[0]?.[1] ?? 0; + const details = sortedDetails.slice(0, 6).map(([dname, dcount]) => ({ + name: dname, + count: dcount, + })); + return { name: gname, count: gcount, details, detailMax: dmax }; + }); - // Project distribution - const projects = countBy(lines, (v) => v.project); - if (projects.size > 0) { - console.log(""); - console.log(" projects"); - const sorted = sortDesc(projects); - const pmax = sorted[0]?.[1] ?? 0; - for (const [name, count] of sorted) { - console.log(` ${padRight(name, 22)} ${padBar(count, pmax, 8)} ${count}`); - } - } + // --- Project distribution --- + const projectMap = countBy(lines, (v) => v.project); + const sortedProjects = sortDesc(projectMap); + const projectMax = sortedProjects[0]?.[1] ?? 0; + const projects = sortedProjects.map(([name, count]) => ({ name, count })); - // Security scan - type Flag = { time: string; desc: string; cmd: string }; - const flags: Flag[] = []; + // --- Security scan --- + const securityFlags: SecurityFlag[] = []; for (const l of lines) { const c = l.cmd; let desc = ""; @@ -159,109 +226,178 @@ function analyze(label: string, files: string[]): void { else if (/\bscp\b|rsync.*@/i.test(c)) desc = "remote file transfer"; else if (/git\s+push\s+--force|git\s+push\s+-f/i.test(c)) desc = "force push"; else if (/git\s+reset\s+--hard/i.test(c)) desc = "hard reset"; - if (desc) flags.push({ time: l.time, desc, cmd: c }); + if (desc) securityFlags.push({ time: l.time, desc, cmd: c }); } - console.log(""); - if (flags.length > 0) { - console.log(` ⚠ security (${flags.length} flags)`); - for (const f of flags.slice(0, 10)) { - console.log(` [${f.time}] ${f.desc}`); - console.log(` ${f.cmd.slice(0, 70)}`); + + // --- Repeated commands --- + const repeatMap = countBy(lines, (v) => v.cmd); + const repeated = sortDesc(repeatMap) + .filter(([, count]) => count >= 3) + .slice(0, 5) + .map(([cmd, count]) => ({ cmd, count })); + + // --- Sessions --- + const SESSION_GAP_MS = 15 * 60 * 1000; + + type RawSession = { + start: string; + end: string; + count: number; + projects: Map; + }; + + const rawSessions: RawSession[] = []; + let cur: RawSession | null = null; + let prevMs = -1; + + for (const l of lines) { + const ms = timeToMs(l.time); + if (cur === null || ms - prevMs > SESSION_GAP_MS) { + cur = { start: l.time, end: l.time, count: 0, projects: new Map() }; + rawSessions.push(cur); } - } else { - console.log(" ✓ no security concerns"); + cur.end = l.time; + cur.count++; + cur.projects.set(l.project, (cur.projects.get(l.project) ?? 0) + 1); + prevMs = ms; } - // Repeated exact commands (3+) - const repeats = countBy(lines, (v) => v.cmd); - const repeated = sortDesc(repeats).filter(([, count]) => count >= 3); - if (repeated.length > 0) { - console.log(""); - console.log(" ↻ repeated (optimization candidates)"); - for (const [cmd, count] of repeated.slice(0, 5)) { - console.log(` ${count}x ${cmd.slice(0, 55)}`); + const totalSessions = rawSessions.length; + const topSessions: AuditSession[] = [...rawSessions] + .sort((a, b) => b.count - a.count) + .slice(0, 3) + .map((s) => ({ + start: s.start, + end: s.end, + count: s.count, + domProject: [...s.projects.entries()].sort((a, b) => b[1] - a[1])[0]?.[0] ?? "—", + durationMs: timeToMs(s.end) - timeToMs(s.start), + })); + + // --- Context-heavy ops --- + const ctxHits: CtxHit[] = []; + for (const l of lines) { + const c = l.cmd.trim(); + let reason = ""; + if (/^(cat|head|tail|less)\b/.test(c)) reason = "file reader (prefer Read tool)"; + else if (/^find\s+[./~]/.test(c)) reason = "broad find scan"; + else if (/^grep\s.*-r\b(?!.*\s\S+\s*$)/.test(c) || /^grep\s+-r\b/.test(c)) + reason = "recursive grep without explicit scope"; + else if (/^ls\s.*-[a-zA-Z]*R/.test(c) || /^ls\s+-R\b/.test(c)) reason = "recursive ls"; + else if (/^tree\b(?!.*--depth\b)(?!.*-L\b)/.test(c)) reason = "tree without depth limit"; + if (reason) ctxHits.push({ time: l.time, cmd: c, reason }); + } + + return { + label, + totalCommands: lines.length, + data: { + groups, + globalMax, + projects, + projectMax, + securityFlags, + repeated, + topSessions, + totalSessions, + ctxHits, + }, + }; +} + +// --- renderAudit (pure rendering stage) ----------------------------------- + +function fmtDuration(ms: number): string { + const m = Math.floor(ms / 60000); + if (m < 60) return `${m}m`; + return `${Math.floor(m / 60)}h ${m % 60}m`; +} + +/** + * Render an `AuditModel` to a human-readable string. + * Mirrors the exact output of the original `analyze()` function. + */ +export function renderAudit(model: AuditModel): string { + const lines: string[] = []; + + if (model.data === null) { + lines.push(` ${model.label}: no data`); + return lines.join("\n"); + } + + const { data } = model; + lines.push(` ${model.label} (${model.totalCommands} commands)`); + + // Command groups + for (const group of data.groups) { + lines.push(""); + lines.push( + ` ${padRight(group.name, 24)} ${padBar(group.count, data.globalMax, 12)} ${group.count}`, + ); + for (const d of group.details) { + lines.push(` ${padRight(d.name, 22)} ${padBar(d.count, group.detailMax, 8)} ${d.count}`); } } - // Sessions — split commands into 15-min idle sessions, show top 3 by volume. - { - type Session = { start: string; end: string; count: number; projects: Map }; - const SESSION_GAP_MS = 15 * 60 * 1000; - - const timeToMs = (t: string): number => { - const parts = t.split(":").map(Number); - return ((parts[0] ?? 0) * 3600 + (parts[1] ?? 0) * 60 + (parts[2] ?? 0)) * 1000; - }; - - const fmtDuration = (ms: number): string => { - const m = Math.floor(ms / 60000); - if (m < 60) return `${m}m`; - return `${Math.floor(m / 60)}h ${m % 60}m`; - }; - - const sessions: Session[] = []; - let cur: Session | null = null; - let prevMs = -1; - - for (const l of lines) { - const ms = timeToMs(l.time); - if (cur === null || ms - prevMs > SESSION_GAP_MS) { - cur = { start: l.time, end: l.time, count: 0, projects: new Map() }; - sessions.push(cur); - } - cur.end = l.time; - cur.count++; - cur.projects.set(l.project, (cur.projects.get(l.project) ?? 0) + 1); - prevMs = ms; + // Project distribution + if (data.projects.length > 0) { + lines.push(""); + lines.push(" projects"); + for (const p of data.projects) { + lines.push(` ${padRight(p.name, 22)} ${padBar(p.count, data.projectMax, 8)} ${p.count}`); } + } - if (sessions.length > 0) { - console.log(""); - console.log(` ⏱ sessions (${sessions.length} total, 15-min idle threshold)`); - const top = [...sessions].sort((a, b) => b.count - a.count).slice(0, 3); - const smax = top[0]?.count ?? 0; - for (const s of top) { - const domProject = [...s.projects.entries()].sort((a, b) => b[1] - a[1])[0]?.[0] ?? "—"; - const durationMs = timeToMs(s.end) - timeToMs(s.start); - const dur = durationMs >= 0 ? fmtDuration(durationMs) : "< 1m"; - const bar = padBar(s.count, smax, 8); - console.log(` ${s.start} ${bar} ${s.count} cmds ${dur} ${padRight(domProject, 20)}`); - } + // Security scan + lines.push(""); + if (data.securityFlags.length > 0) { + lines.push(` ⚠ security (${data.securityFlags.length} flags)`); + for (const f of data.securityFlags.slice(0, 10)) { + lines.push(` [${f.time}] ${f.desc}`); + lines.push(` ${f.cmd.slice(0, 70)}`); } + } else { + lines.push(" ✓ no security concerns"); } - // Context-heavy operations — proxies for excessive context window burn. - { - type CtxHit = { time: string; cmd: string; reason: string }; - const hits: CtxHit[] = []; - - for (const l of lines) { - const c = l.cmd.trim(); - let reason = ""; - if (/^(cat|head|tail|less)\b/.test(c)) reason = "file reader (prefer Read tool)"; - else if (/^find\s+[./~]/.test(c)) reason = "broad find scan"; - else if (/^grep\s.*-r\b(?!.*\s\S+\s*$)/.test(c) || /^grep\s+-r\b/.test(c)) - reason = "recursive grep without explicit scope"; - else if (/^ls\s.*-[a-zA-Z]*R/.test(c) || /^ls\s+-R\b/.test(c)) reason = "recursive ls"; - else if (/^tree\b(?!.*--depth\b)(?!.*-L\b)/.test(c)) reason = "tree without depth limit"; - if (reason) hits.push({ time: l.time, cmd: c, reason }); + // Repeated commands + if (data.repeated.length > 0) { + lines.push(""); + lines.push(" ↻ repeated (optimization candidates)"); + for (const r of data.repeated) { + lines.push(` ${r.count}x ${r.cmd.slice(0, 55)}`); } + } + + // Sessions + if (data.topSessions.length > 0) { + lines.push(""); + lines.push(` ⏱ sessions (${data.totalSessions} total, 15-min idle threshold)`); + const smax = data.topSessions[0]?.count ?? 0; + for (const s of data.topSessions) { + const dur = s.durationMs >= 0 ? fmtDuration(s.durationMs) : "< 1m"; + const bar = padBar(s.count, smax, 8); + lines.push(` ${s.start} ${bar} ${s.count} cmds ${dur} ${padRight(s.domProject, 20)}`); + } + } - const ctxMax = hits.length; - if (hits.length > 0) { - console.log(""); - console.log(` 📄 context-heavy ops (${hits.length})`); - const bar = padBar(hits.length, ctxMax > 0 ? ctxMax : 1, 8); - console.log(` ${bar} ${hits.length} total`); - for (const h of hits.slice(0, 5)) { - console.log(` [${h.time}] ${h.reason}`); - console.log(` ${h.cmd.slice(0, 65)}`); - } - console.log( - ` → ${hits.length} context-heavy op${hits.length === 1 ? "" : "s"}; prefer Read/Grep tools or scope these to specific paths`, - ); + // Context-heavy ops + if (data.ctxHits.length > 0) { + const ctxMax = data.ctxHits.length; + lines.push(""); + lines.push(` 📄 context-heavy ops (${data.ctxHits.length})`); + const bar = padBar(data.ctxHits.length, ctxMax > 0 ? ctxMax : 1, 8); + lines.push(` ${bar} ${data.ctxHits.length} total`); + for (const h of data.ctxHits.slice(0, 5)) { + lines.push(` [${h.time}] ${h.reason}`); + lines.push(` ${h.cmd.slice(0, 65)}`); } + lines.push( + ` → ${data.ctxHits.length} context-heavy op${data.ctxHits.length === 1 ? "" : "s"}; prefer Read/Grep tools or scope these to specific paths`, + ); } + + return lines.join("\n"); } // --- Main ----------------------------------------------------------------- @@ -286,7 +422,7 @@ function main(): void { console.log(`🔍 Claude Audit · ${header}`); console.log(""); - analyze("Today", [join(LOG_DIR, `bash-${today}.log`)]); + console.log(renderAudit(analyzeCommands("Today", [join(LOG_DIR, `bash-${today}.log`)]))); // Week: Monday through today. const dow = weekdayIdx(now); @@ -301,7 +437,7 @@ function main(): void { } if (weekFiles.length > 1) { console.log(""); - analyze("This week", weekFiles); + console.log(renderAudit(analyzeCommands("This week", weekFiles))); } console.log(""); } diff --git a/src/scripts/codex-run.ts b/src/scripts/codex-run.ts index 85345f8..fb580bf 100644 --- a/src/scripts/codex-run.ts +++ b/src/scripts/codex-run.ts @@ -1,26 +1,42 @@ #!/usr/bin/env bun + // CLI for the /codex skill — delegates tasks, reviews diffs, or asks questions // using the OpenAI Codex CLI as a second model in the Claude x Codex pairing. // // Usage: -// codex-run.ts exec "" — delegate mechanical/bulk work to Codex -// codex-run.ts review — independent review of the current uncommitted diff -// codex-run.ts ask "" — read-only second opinion from Codex +// codex-run.ts exec [--force] "" — delegate mechanical/bulk work to Codex +// codex-run.ts review [--force] — independent review of the current uncommitted diff +// codex-run.ts ask [--force] "" — read-only second opinion from Codex +// +// --force: bypass a sticky rate-limited or no-access verdict and re-probe with a +// real call. Useful when the quota message was a false positive (e.g. auth mismatch). +// Does NOT bypass not-installed or unauthenticated — those can't succeed regardless. import { runCodexExec } from "../lib/codex.ts"; +import { runGit } from "../lib/git.ts"; function usage(): void { console.error( [ - "Usage: codex-run.ts [args]", + "Usage: codex-run.ts [--force] [args]", + "", + " exec [--force] Delegate mechanical/bulk work to Codex (workspace-write sandbox)", + " review [--force] Review the current uncommitted diff for bugs, security issues, and quality", + " ask [--force] Read-only second opinion from Codex", "", - " exec Delegate mechanical/bulk work to Codex (workspace-write sandbox)", - " review Review the current uncommitted diff for bugs, security issues, and quality", - " ask Read-only second opinion from Codex", + " --force Bypass a sticky rate-limited/no-access verdict and re-probe.", + " Useful when the quota error is a false positive (e.g. auth mismatch).", ].join("\n"), ); } +/** Strip --force from an argv array and return {force, rest}. */ +function parseForce(args: string[]): { force: boolean; rest: string[] } { + const force = args.includes("--force"); + const rest = args.filter((a) => a !== "--force"); + return { force, rest }; +} + const [, , subcommand, ...rest] = process.argv; if (!subcommand) { @@ -30,15 +46,28 @@ if (!subcommand) { switch (subcommand) { case "exec": { - const task = rest.join(" ").trim(); + const { force, rest: execArgs } = parseForce(rest); + const task = execArgs.join(" ").trim(); if (!task) { console.error("Error: exec requires a task argument.\n"); usage(); process.exit(2); } - const result = await runCodexExec({ prompt: task, sandbox: "workspace-write" }); + const result = await runCodexExec({ prompt: task, sandbox: "workspace-write", force }); if (result.ok) { console.log(result.output); + // Surface the changed-file summary so callers always see what exec wrote. + try { + const status = await runGit(["status", "--porcelain"]); + const stat = await runGit(["diff", "--stat"]); + if (status || stat) { + console.log("\n── git summary ──────────────────────────────"); + if (status) console.log(status); + if (stat) console.log(stat); + } + } catch { + // Not a git repo or git unavailable — skip the summary gracefully. + } process.exit(0); } else { console.error(result.detail ?? `Codex exec failed (state: ${result.state})`); @@ -48,6 +77,7 @@ switch (subcommand) { } case "review": { + const { force } = parseForce(rest); const reviewPrompt = [ "You are performing an independent code review of the current uncommitted diff in this repository.", "", @@ -62,7 +92,7 @@ switch (subcommand) { "", "Be concise and precise. Focus on real problems, not style preferences.", ].join("\n"); - const result = await runCodexExec({ prompt: reviewPrompt, sandbox: "read-only" }); + const result = await runCodexExec({ prompt: reviewPrompt, sandbox: "read-only", force }); if (result.ok) { console.log(result.output); process.exit(0); @@ -74,13 +104,14 @@ switch (subcommand) { } case "ask": { - const question = rest.join(" ").trim(); + const { force, rest: askArgs } = parseForce(rest); + const question = askArgs.join(" ").trim(); if (!question) { console.error("Error: ask requires a question argument.\n"); usage(); process.exit(2); } - const result = await runCodexExec({ prompt: question, sandbox: "read-only" }); + const result = await runCodexExec({ prompt: question, sandbox: "read-only", force }); if (result.ok) { console.log(result.output); process.exit(0); diff --git a/src/scripts/lint-skills.ts b/src/scripts/lint-skills.ts index 9342aa5..e6e8db6 100644 --- a/src/scripts/lint-skills.ts +++ b/src/scripts/lint-skills.ts @@ -11,7 +11,9 @@ import { formatFindings, hasErrors, lintSkillsDir } from "../lib/lint-skills.ts" async function main(): Promise { const arg = process.argv[2]; const skillsDir = arg ? resolve(arg) : join(import.meta.dir, "..", "..", "skills"); - const result = await lintSkillsDir(skillsDir); + // Enable the ACTIVE_SKILLS↔disk invariant only for the canonical repo run + // (no custom dir arg); a custom dir isn't necessarily the managed skills/. + const result = await lintSkillsDir(skillsDir, { checkManaged: !arg }); console.log(formatFindings(result)); return hasErrors(result) ? 1 : 0; } diff --git a/src/setup.ts b/src/setup.ts index 08ed2d0..42d56e3 100644 --- a/src/setup.ts +++ b/src/setup.ts @@ -52,11 +52,11 @@ import { CLAUDE_JSON_PATH, installMcpToClaudeJson, type McpServers, + mergeSettingsWithMcpPreservation, removeManagedMcpServers, } from "./lib/mcp.ts"; import { ensurePythonPackage, ensureSystemPackage, getInstallHint } from "./lib/packages.ts"; import { getTimestamp, hasCommand, isWindows } from "./lib/platform.ts"; -import { mergeSettingsWithMcpPreservation } from "./lib/settings-merge.ts"; import { formatPrereqWarnings, reportMissingPrereqs } from "./lib/skill-prereqs.ts"; import { gatherStatus } from "./lib/status.ts"; import type { StatusData } from "./lib/status-types.ts"; @@ -175,6 +175,112 @@ async function cmdRollback(target: string | true): Promise { return code; } +// --- Install plan -------------------------------------------------------- + +/** + * Describes one planned install action — either a file/dir copy or a prune. + * Produced once by buildInstallPlan; consumed by installConfigFiles (action), + * cmdDryRun (display), and showSummary (display). + */ +export interface InstallStep { + /** Path relative to both sourceDir (for "copy") and CLAUDE_DIR (for "prune"). */ + rel: string; + /** "copy" = copy from source → target; "prune" = remove from target. */ + action: "copy" | "prune"; + /** Whether the source (for "copy") or target (for "prune") exists on disk. */ + exists: boolean; + /** Human-readable annotation shown in dry-run / summary tables. */ + label?: string; +} + +/** + * Produce the complete planned footprint for an install: every file/dir that + * will be copied or pruned, derived from PROFILE_MANIFEST and LIGHT_SKILLS. + * All four traversal sites (installConfigFiles, removeLightIncompatibleFiles, + * showSummary, cmdDryRun) derive from this single list instead of re-walking + * the manifest independently. + * + * Does NOT touch disk — pure computation from the manifest + existsSync checks. + */ +export function buildInstallPlan(sourceDir: string, profile: "full" | "light"): InstallStep[] { + const steps: InstallStep[] = []; + + if (profile === "light") { + // Copy the LIGHT_SKILLS subset of skills/. + for (const skill of LIGHT_SKILLS) { + const rel = `skills/${skill}`; + steps.push({ + rel, + action: "copy", + exists: existsSync(join(sourceDir, rel)), + label: `→ ~/.claude/${rel}/`, + }); + } + + // Prune skills from a prior full install that are not in the light set. + // Scoped to MANAGED_SKILLS so user-authored skills are never touched. + const lightSkillSet = new Set(LIGHT_SKILLS); + for (const skill of MANAGED_SKILLS) { + if (!lightSkillSet.has(skill)) { + steps.push({ + rel: `skills/${skill}`, + action: "prune", + exists: existsSync(join(CLAUDE_DIR, `skills/${skill}`)), + }); + } + } + + // Prune full-only rootFiles and dirs (CLAUDE.md, AGENTS.md, agents/, …). + const { full, light } = PROFILE_MANIFEST; + const lightFiles = new Set(light.rootFiles.map(([, dest]) => dest)); + const lightDirs = new Set([...light.dirs, ...light.retainedDirs]); + for (const [, dest] of full.rootFiles) { + if (!lightFiles.has(dest)) { + steps.push({ + rel: dest, + action: "prune", + exists: existsSync(join(CLAUDE_DIR, dest)), + }); + } + } + for (const d of full.dirs) { + if (!lightDirs.has(d)) { + steps.push({ + rel: d, + action: "prune", + exists: existsSync(join(CLAUDE_DIR, d)), + }); + } + } + } else { + // Full profile: every rootFile + dir from the manifest. + const ROOT_FILE_LABELS: Record = { + "CLAUDE.md": "(Claude-Code config)", + "AGENTS.md": "(portable standards)", + }; + const manifest = PROFILE_MANIFEST.full; + for (const [src, dest] of manifest.rootFiles) { + const label = ROOT_FILE_LABELS[dest] ? `${dest} ${ROOT_FILE_LABELS[dest]}` : dest; + steps.push({ + rel: src, + action: "copy", + exists: existsSync(join(sourceDir, src)), + label, + }); + } + for (const d of manifest.dirs) { + steps.push({ + rel: d, + action: "copy", + exists: existsSync(join(sourceDir, d)), + label: `${d}/`, + }); + } + } + + return steps; +} + // --- Install phases ------------------------------------------------------ async function createBackup(): Promise { @@ -322,28 +428,27 @@ async function copyDirContents(srcDir: string, dstDir: string): Promise { return copyDirContentsFiltered(srcDir, dstDir, () => true); } +/** + * Execute the copy/prune steps from buildInstallPlan. cleanOldConfig must + * already have run so MANAGED_SKILLS dirs are wiped before copy. + */ async function installConfigFiles(source: string, profile: Profile): Promise { - if (profile === "light") { - // Light = raw Claude Code: the manifest has no rootFiles/dirs for light. - // Install ONLY the LIGHT_SKILLS subset (share-learning) — the skill - // filter stays as code; only the file/dir lists live in the manifest. + const plan = buildInstallPlan(source, profile); - // skills: only LIGHT_SKILLS (share-learning) + if (profile === "light") { + // Copy LIGHT_SKILLS subset. const lightSkillSet = new Set(LIGHT_SKILLS); await copyDirContentsFiltered(join(source, "skills"), join(CLAUDE_DIR, "skills"), (name) => lightSkillSet.has(name), ); - - // Prune cc-settings skills left over from a prior full install that are - // not in the light set. Scope removal to skill folders that exist in the - // source repo — never touch user-authored skills. - const sourceSkillDirs = (await readdir(join(source, "skills"), { withFileTypes: true })) - .filter((e) => e.isDirectory()) - .map((e) => e.name); + // Prune skill dirs from the plan that cleanOldConfig may have left behind + // (MANAGED_SKILLS sweep in cleanOldConfig covers the known list; skill-prune + // steps from the plan cover any that slipped through). + const pruneSteps = plan.filter((s) => s.action === "prune" && s.rel.startsWith("skills/")); await Promise.all( - sourceSkillDirs - .filter((name) => !lightSkillSet.has(name)) - .map((name) => rm(join(CLAUDE_DIR, "skills", name), { recursive: true, force: true })), + pruneSteps.map((s) => + rm(join(CLAUDE_DIR, s.rel), { recursive: true, force: true }).catch(() => {}), + ), ); } else { // Full profile: every rootFile + dir from the manifest. Destinations are @@ -360,21 +465,19 @@ async function installConfigFiles(source: string, profile: Profile): Promise { - const { full, light } = PROFILE_MANIFEST; - const lightFiles = new Set(light.rootFiles.map(([, dest]) => dest)); - const lightDirs = new Set([...light.dirs, ...light.retainedDirs]); + // buildInstallPlan for light already includes prune steps for full-only + // rootFiles and dirs. Re-use the plan here so there's one source of truth. + // We use an empty sourceDir because prune steps don't read the source. + const plan = buildInstallPlan("", "light"); + const pruneNonSkill = plan.filter((s) => s.action === "prune" && !s.rel.startsWith("skills/")); const removeIfExists = (p: string) => rm(p, { recursive: true, force: true }).catch(() => {}); await Promise.all([ - ...full.rootFiles - .filter(([, dest]) => !lightFiles.has(dest)) - .map(([, dest]) => removeIfExists(join(CLAUDE_DIR, dest))), - ...full.dirs.filter((d) => !lightDirs.has(d)).map((d) => removeIfExists(join(CLAUDE_DIR, d))), + ...pruneNonSkill.map((s) => removeIfExists(join(CLAUDE_DIR, s.rel))), removeIfExists(join(CLAUDE_DIR, "contexts")), // legacy; contexts/ retired ]); } @@ -754,6 +857,55 @@ async function cmdStatus(sourceDir: string): Promise { // --- Main ---------------------------------------------------------------- +/** + * Run the migrate-only path: backup + settings merger + version sentinel. + * Skips file copy, dependency install, and skill/agent refresh. + */ +async function runMigrateOnly(args: Args): Promise { + info("Migrate-only: backup + merger + sentinel; skipping file copy"); + await createBackup(); + await createDirectories(); // idempotent — ensures ~/.claude/ shape exists for merger +} + +/** + * Run the full install path: deps → backup → dirs → clean → light-incompatible + * removal → file copy → TS source copy → src manifest. + * + * PHASE ORDER IS CORRECTNESS-CRITICAL: + * clean before copy; fingerprint after settings write; manifest write for + * tamper defense. Do not reorder. + */ +async function runFullInstall(args: Args): Promise { + info("Installing dependencies..."); + await installDependencies(args.profile); + printPreflightReport(checkCliTools()); + + info("Creating backup..."); + await createBackup(); + + info("Installing configuration..."); + await createDirectories(); + await cleanOldConfig(); + // For light: remove dirs that full installs but light must not have + // (CLAUDE.md, AGENTS.md, agents/, rules/, profiles/, docs/). + // Must run AFTER cleanOldConfig so any leftover full-install content is gone. + if (args.profile === "light") { + await removeLightIncompatibleFiles(); + } + // Disjoint destination trees (config dirs vs ~/.claude/src), so install both + // in parallel. Both must follow the clean above. + await Promise.all([ + installConfigFiles(args.sourceDir, args.profile), + installTsSources(args.sourceDir), + ]); + // Content manifest of the just-installed ~/.claude/src tree — the + // supply-chain layer that catches dropped/patched script content + // (verify-hooks.ts re-checks it at SessionStart; audit-hooks.ts gates + // "trusted" on it). Refreshed ONLY here, never by the auditor. Best-effort: + // a failed write just downgrades audit classifications to "unknown". + await writeSrcManifest(join(CLAUDE_DIR, "src"), CLAUDE_DIR).catch(() => {}); +} + async function main(): Promise { const args = parseArgs(process.argv.slice(2)); if (args.help) { @@ -788,39 +940,11 @@ async function main(): Promise { const fmWarning = formatFrontmatterIssues(fmIssues); if (fmWarning) warn(fmWarning); + // Dispatch to migrate-only or full install path. if (args.migrateOnly) { - info("Migrate-only: backup + merger + sentinel; skipping file copy"); - await createBackup(); - await createDirectories(); // idempotent — ensures ~/.claude/ shape exists for merger + await runMigrateOnly(args); } else { - info("Installing dependencies..."); - await installDependencies(args.profile); - printPreflightReport(checkCliTools()); - - info("Creating backup..."); - await createBackup(); - - info("Installing configuration..."); - await createDirectories(); - await cleanOldConfig(); - // For light: remove dirs that full installs but light must not have - // (CLAUDE.md, AGENTS.md, agents/, rules/, profiles/, docs/). - // Must run AFTER cleanOldConfig so any leftover full-install content is gone. - if (args.profile === "light") { - await removeLightIncompatibleFiles(); - } - // Disjoint destination trees (config dirs vs ~/.claude/src), so install both - // in parallel. Both must follow the clean above. - await Promise.all([ - installConfigFiles(args.sourceDir, args.profile), - installTsSources(args.sourceDir), - ]); - // Content manifest of the just-installed ~/.claude/src tree — the - // supply-chain layer that catches dropped/patched script content - // (verify-hooks.ts re-checks it at SessionStart; audit-hooks.ts gates - // "trusted" on it). Refreshed ONLY here, never by the auditor. Best-effort: - // a failed write just downgrades audit classifications to "unknown". - await writeSrcManifest(join(CLAUDE_DIR, "src"), CLAUDE_DIR).catch(() => {}); + await runFullInstall(args); } try { diff --git a/tests/audit-hooks.test.ts b/tests/audit-hooks.test.ts index f14eb36..199cb66 100644 --- a/tests/audit-hooks.test.ts +++ b/tests/audit-hooks.test.ts @@ -305,6 +305,29 @@ describe("classifyHookCommand — suspicious malware patterns", () => { }); }); +describe("classifyHookCommand — lib/ files are NOT auto-trusted", () => { + // lib/ files are support modules, not hooks — tightened in nuclear-review. + // A bun invocation of ~/.claude/src/lib/.ts must NOT match the managed + // pattern: it falls to manifest-absent → unknown, or manifest-present → check + // manifest, not auto-trusted by path shape alone. + test("bun invocation of a lib/ file is not auto-trusted (unknown without manifest)", () => { + const r = classifyHookCommand('bun "$HOME/.claude/src/lib/colors.ts"'); + // lib/ is excluded from MANAGED_HOOK_CMD — falls to unknown, not trusted + expect(r.severity).toBe("unknown"); + expect(r.reasons.join(" ")).toMatch(/does not match cc-settings shipped pattern/); + }); + + test("lib/ file with a matching manifest entry is still not trusted via path alone", () => { + // Even if lib/colors.ts is in the install manifest with a valid hash, the + // auditor must not promote it to trusted — only scripts/ and hooks/ qualify. + const r = classifyHookCommand( + 'bun "$HOME/.claude/src/lib/colors.ts"', + integrityOf({ "lib/colors.ts": true }), + ); + expect(r.severity).toBe("unknown"); + }); +}); + describe("classifyHookCommand — unknown (user-added, not malware)", () => { test("plain echo command", () => { const r = classifyHookCommand("echo 'hello'"); diff --git a/tests/codex.test.ts b/tests/codex.test.ts index 008b7eb..eea36d4 100644 --- a/tests/codex.test.ts +++ b/tests/codex.test.ts @@ -2,7 +2,12 @@ // No filesystem I/O, no subprocess spawning — pure logic only. import { describe, expect, test } from "bun:test"; -import { type CodexVerdict, classifyCodexError, reconcile } from "../src/lib/codex.ts"; +import { + type CodexVerdict, + classifyCodexError, + reconcile, + sanitizeOutput, +} from "../src/lib/codex.ts"; // --------------------------------------------------------------------------- // classifyCodexError — state classification @@ -322,6 +327,86 @@ describe("reconcile — live 'available' + stale sticky negatives expire", () => }); }); +// --------------------------------------------------------------------------- +// sanitizeOutput — full-text redaction (no line/length limits) +// --------------------------------------------------------------------------- + +describe("sanitizeOutput — multi-line credential redaction", () => { + test("strips ANSI escapes from full text", () => { + const ESC = String.fromCharCode(27); + const input = `line1\n${ESC}[31mred line${ESC}[0m\nline3`; + const result = sanitizeOutput(input); + expect(result).not.toContain(ESC); + expect(result).toContain("red line"); + expect(result).toContain("line1"); + expect(result).toContain("line3"); + }); + + test("redacts sk- tokens across multiple lines", () => { + const token = "sk-ABCDEFGHIJKLMNOPabcdefghijklmnop"; + const input = `line1\nerror: ${token}\nline3`; + const result = sanitizeOutput(input); + expect(result).not.toContain(token); + expect(result).toContain("sk-[redacted]"); + expect(result).toContain("line1"); + expect(result).toContain("line3"); + }); + + test("does NOT cap length (operates on full text)", () => { + const long = "x".repeat(500); + const result = sanitizeOutput(long); + expect(result.length).toBe(500); + }); + + test("redacts Bearer tokens on any line", () => { + const input = `ok\nAuthorization header Bearer eyJhbGciOiJSUzI1NiJ9.pay.sig\ndone`; + const result = sanitizeOutput(input); + expect(result).not.toContain("eyJhbGciOiJSUzI1NiJ9"); + expect(result).toContain("Bearer [redacted]"); + }); + + test("redacts Authorization: header value", () => { + const input = `Authorization: secret_token_here\nother line`; + const result = sanitizeOutput(input); + expect(result).not.toContain("secret_token_here"); + expect(result).toContain("Authorization: [redacted]"); + }); + + test("OPENAI_API_KEY= is redacted — env-var pattern covers *_API_KEY names", () => { + // Business rule: env-var assignments exposing API keys must never reach + // the verdict file or the statusline. The pattern targets *_API_KEY, + // *_TOKEN, and *_SECRET names — common CI/CD credential conventions. + const result = sanitizeOutput("OPENAI_API_KEY=sk-abcd1234efgh5678ijkl"); + expect(result).toContain("OPENAI_API_KEY=[redacted]"); + expect(result).not.toContain("sk-abcd1234efgh5678ijkl"); + }); + + test("MY_TOKEN= is redacted — env-var pattern covers *_TOKEN names", () => { + const result = sanitizeOutput("MY_TOKEN=supersecretvalue"); + expect(result).toBe("MY_TOKEN=[redacted]"); + }); + + test("GITHUB_SECRET= is redacted — env-var pattern covers *_SECRET names", () => { + const result = sanitizeOutput("GITHUB_SECRET=abc123"); + expect(result).toBe("GITHUB_SECRET=[redacted]"); + }); + + test("Bearer: (colon, no space) is redacted — colon is treated as separator", () => { + // The regex uses [\s:]+ to capture both 'Bearer ' and 'Bearer:' + // so colon-separated variants are also caught. + const result = sanitizeOutput("Bearer:tok_nospaces_here"); + expect(result).toContain("Bearer [redacted]"); + expect(result).not.toContain("tok_nospaces_here"); + }); + + test("ordinary key=value pairs like count=5 or level=high are NOT mangled", () => { + // The env-var pattern only targets uppercase names ending in _API_KEY, + // _TOKEN, or _SECRET — lowercase or generic names must pass through. + const plain = "count=5 level=high status=ok"; + expect(sanitizeOutput(plain)).toBe(plain); + }); +}); + describe("reconcile — live 'available' + cached available", () => { test("live available + cached available (sticky:false) → available", () => { const cached: CodexVerdict = { diff --git a/tests/golden-migrations.test.ts b/tests/golden-migrations.test.ts index e81d4a6..10e53bc 100644 --- a/tests/golden-migrations.test.ts +++ b/tests/golden-migrations.test.ts @@ -21,7 +21,7 @@ import { readdirSync, statSync } from "node:fs"; import { mkdtemp, readFile, rm, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join, resolve } from "node:path"; -import { mergeSettingsWithMcpPreservation } from "../src/lib/settings-merge.ts"; +import { mergeSettingsWithMcpPreservation } from "../src/lib/mcp.ts"; const FIXTURES = resolve(import.meta.dir, "fixtures", "migrations"); diff --git a/tests/hooks-fingerprint.test.ts b/tests/hooks-fingerprint.test.ts index 744ebb3..40b98ad 100644 --- a/tests/hooks-fingerprint.test.ts +++ b/tests/hooks-fingerprint.test.ts @@ -344,6 +344,96 @@ describe("src manifest — write + verify", () => { } }); + test("readSrcManifest rejects a key containing a '..' path segment", async () => { + // Business rule: manifest keys are later join()'d under ~/.claude/src — + // a '..' segment must not let a tampered manifest point outside the tree. + // The whole manifest is rejected (fail-open to null / 'missing') rather + // than silently skipping the bad entry. + const dir = await mkdtemp(join(tmpdir(), "cc-srcm-")); + try { + await writeFile( + join(dir, SRC_MANIFEST_FILENAME), + JSON.stringify({ files: { "../escape.ts": "abc" }, installedAt: "x" }), + ); + expect(await readSrcManifest(dir)).toBeNull(); + } finally { + await rm(dir, { recursive: true, force: true }); + } + }); + + test("readSrcManifest rejects an absolute key", async () => { + // An absolute path like '/etc/passwd' would let a read bypass the src tree. + const dir = await mkdtemp(join(tmpdir(), "cc-srcm-")); + try { + await writeFile( + join(dir, SRC_MANIFEST_FILENAME), + JSON.stringify({ files: { "/etc/passwd": "abc" }, installedAt: "x" }), + ); + expect(await readSrcManifest(dir)).toBeNull(); + } finally { + await rm(dir, { recursive: true, force: true }); + } + }); + + test("readSrcManifest rejects a non-string hash value", async () => { + // Hash values are compared to the output of CryptoHasher (a string); + // a non-string value indicates a tampered manifest and must be rejected. + const dir = await mkdtemp(join(tmpdir(), "cc-srcm-")); + try { + await writeFile( + join(dir, SRC_MANIFEST_FILENAME), + JSON.stringify({ files: { "hooks/x.ts": 123 }, installedAt: "x" }), + ); + expect(await readSrcManifest(dir)).toBeNull(); + } finally { + await rm(dir, { recursive: true, force: true }); + } + }); + + test("readSrcManifest returns a populated record for a clean manifest", async () => { + // A well-formed manifest with a valid relative key and string hash must + // be accepted and returned with files intact. + const dir = await mkdtemp(join(tmpdir(), "cc-srcm-")); + try { + await writeFile( + join(dir, SRC_MANIFEST_FILENAME), + JSON.stringify({ + files: { "hooks/safety-net.ts": "deadbeef" }, + installedAt: "2026-06-19T00:00:00.000Z", + }), + ); + const result = await readSrcManifest(dir); + expect(result).not.toBeNull(); + expect(result?.files["hooks/safety-net.ts"]).toBe("deadbeef"); + } finally { + await rm(dir, { recursive: true, force: true }); + } + }); + + test("readSrcManifest strips control chars from installedAt (ANSI injection defense)", async () => { + // installedAt is echoed verbatim into the terminal warning banner; + // a crafted value containing ESC sequences could disguise the alarm. + // The stripControl transform must remove all chars with codePoint <= 0x1f. + const dir = await mkdtemp(join(tmpdir(), "cc-srcm-")); + try { + const poisoned = String.fromCharCode(27) + "[1mX"; + await writeFile( + join(dir, SRC_MANIFEST_FILENAME), + JSON.stringify({ + files: { "hooks/safety-net.ts": "deadbeef" }, + installedAt: poisoned, + }), + ); + const result = await readSrcManifest(dir); + expect(result).not.toBeNull(); + // No char in installedAt should have codePoint <= 0x1f (C0 controls incl. ESC) + const codePoints = Array.from(result!.installedAt).map((c) => c.charCodeAt(0)); + expect(codePoints.every((n) => n > 0x1f)).toBe(true); + } finally { + await rm(dir, { recursive: true, force: true }); + } + }); + test("node_modules and non-.ts files are excluded from the manifest", async () => { const dir = await mkdtemp(join(tmpdir(), "cc-srcm-")); try { diff --git a/tests/mcp.test.ts b/tests/mcp.test.ts index 0580738..8fbc401 100644 --- a/tests/mcp.test.ts +++ b/tests/mcp.test.ts @@ -6,8 +6,11 @@ import { mkdtemp, readFile, rm, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { JsonParseError } from "../src/lib/json-io.ts"; -import { findUserOnlyServers, installMcpToClaudeJson } from "../src/lib/mcp.ts"; -import { mergeSettingsWithMcpPreservation } from "../src/lib/settings-merge.ts"; +import { + findUserOnlyServers, + installMcpToClaudeJson, + mergeSettingsWithMcpPreservation, +} from "../src/lib/mcp.ts"; async function withTmp(fn: (dir: string) => Promise): Promise { const dir = await mkdtemp(join(tmpdir(), "cc-mcp-schema-")); diff --git a/tests/settings-merge.test.ts b/tests/settings-merge.test.ts index e93e572..1d230ff 100644 --- a/tests/settings-merge.test.ts +++ b/tests/settings-merge.test.ts @@ -6,12 +6,12 @@ import { describe, expect, test } from "bun:test"; import { mkdtemp, rm, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join } from "node:path"; +import { mergeSettingsWithMcpPreservation } from "../src/lib/mcp.ts"; import type { MergeAccounting, MergeOptions, StrategyContext } from "../src/lib/settings-merge.ts"; import { DEPRECATED_COMMAND_PATTERNS, envStrategy, hooksStrategy, - mergeSettingsWithMcpPreservation, permissionsStrategy, pruneDeprecatedHooks, statusLineStrategy, diff --git a/tests/tool-cadence.test.ts b/tests/tool-cadence.test.ts index 54d28e1..c9f337e 100644 --- a/tests/tool-cadence.test.ts +++ b/tests/tool-cadence.test.ts @@ -208,6 +208,23 @@ describe("tool-cadence — parallelmax branch (e2e)", () => { } }); + // ── Test 5a: Completely missing state file handled without crashing ──────── + test("missing state file (null) handled without crashing", async () => { + const home = await mkdtemp(join(tmpdir(), "cc-pm-")); + try { + // No state file written — readState returns null, normalizeCounterState defaults all fields. + const r = await runHook(readPayload(), home, "12"); + expect(r.exit).toBe(0); + const state = await readCounterState(home); + expect(state?.count).toBe(1); + expect(state?.nudged).toBe(false); + expect(state?.escalated).toBe(false); + expect(Array.isArray(state?.files)).toBe(true); + } finally { + await rm(home, { recursive: true, force: true }); + } + }); + // ── Test 5: Old-shape state file handled without crashing ───────────────── test("old-shape state file (no new fields) handled without crashing", async () => { const home = await mkdtemp(join(tmpdir(), "cc-pm-"));