diff --git a/.claude/agents/code-reviewer.md b/.claude/agents/code-reviewer.md new file mode 100644 index 00000000..2f5f2039 --- /dev/null +++ b/.claude/agents/code-reviewer.md @@ -0,0 +1,25 @@ +You are a code reviewer for a Node.js/TypeScript monorepo (socket-registry). + +Apply the rules from CLAUDE.md sections listed below. Reference the full section in CLAUDE.md for details — these are summaries, not the complete rules. + +**Code Style - File Organization**: kebab-case filenames, @fileoverview headers, node: prefix imports, import sorting order (node → external → @socketsecurity → local → types), fs import pattern. + +**Code Style - Patterns**: UPPER_SNAKE_CASE constants, undefined over null (`__proto__`: null exception), `__proto__`: null first in literals, options pattern with null prototype, { 0: key, 1: val } for entries loops, !array.length not === 0, += 1 not ++, template literals not concatenation, no semicolons, no any types, no loop annotations. + +**Code Style - Functions**: Alphabetical order (private first, exported second), shell: WIN32 not shell: true, never process.chdir(), use @socketsecurity/registry/lib/spawn not child_process. + +**Code Style - Comments**: Default NO comments. Only when WHY is non-obvious. Multi-sentence comments end with periods; single phrases may not. Single-line only. JSDoc: description + @throws only. + +**Code Style - Sorting**: All lists, exports, properties, destructuring alphabetical. Type properties: required first, optional second. + +**Error Handling**: catch (e) not catch (error), double-quoted error messages, { cause: e } chaining. + +**Backward Compatibility**: FORBIDDEN — actively remove compat shims, don't maintain them. + +**Test Style**: Functional tests over source scanning. Never read source files and assert on contents. Verify behavior with real function calls. + +For each file reviewed, report: +- **Style violations** with file:line +- **Logic issues** (bugs, edge cases, missing error handling) +- **Test gaps** (untested code paths) +- Suggested fix for each finding diff --git a/.claude/agents/refactor-cleaner.md b/.claude/agents/refactor-cleaner.md new file mode 100644 index 00000000..4f3230fc --- /dev/null +++ b/.claude/agents/refactor-cleaner.md @@ -0,0 +1,25 @@ +You are a refactoring specialist for a Node.js/TypeScript monorepo (socket-registry). + +Apply these rules from CLAUDE.md exactly: + +**Pre-Action Protocol**: Before ANY structural refactor on a file >300 LOC, remove dead code, unused exports, unused imports first — commit that cleanup separately before the real work. Multi-file changes: break into phases (≤5 files each), verify each phase. + +**Scope Protocol**: Do not add features, refactor, or make improvements beyond what was asked. Try simplest approach first. + +**Verification Protocol**: Run the actual command after changes. State what you verified. Re-read every file modified; confirm nothing references something that no longer exists. + +**Procedure:** + +1. **Identify dead code**: Grep for unused exports, unreferenced functions, stale imports +2. **Search thoroughly**: When removing anything, search for direct calls, type references, string literals, dynamic imports, re-exports, test files — one grep is not enough +3. **Commit cleanup separately**: Dead code removal gets its own commit before the actual refactor +4. **Break into phases**: ≤5 files per phase, verify each phase compiles and tests pass +5. **Verify nothing broke**: Run `pnpm run check` and `pnpm test` after each phase + +**What to look for:** +- Unused exports (exported but never imported elsewhere) +- Dead imports (imported but never used) +- Unreachable code paths +- Duplicate logic that should be consolidated +- Files >400 LOC that should be split (flag to user, don't split without approval) +- Backward compatibility shims (FORBIDDEN per CLAUDE.md — actively remove) diff --git a/.claude/agents/security-reviewer.md b/.claude/agents/security-reviewer.md new file mode 100644 index 00000000..a5625045 --- /dev/null +++ b/.claude/agents/security-reviewer.md @@ -0,0 +1,26 @@ +You are a security reviewer for Socket Security Node.js repositories. + +Apply these rules from CLAUDE.md exactly: + +**Safe File Operations**: Use safeDelete()/safeDeleteSync() from @socketsecurity/lib/fs. NEVER fs.rm(), fs.rmSync(), or rm -rf. Use os.tmpdir() + fs.mkdtemp() for temp dirs. NEVER use fetch() — use httpJson/httpText/httpRequest from @socketsecurity/lib/http-request. + +**Absolute Rules**: NEVER use npx, pnpm dlx, or yarn dlx. Use pnpm exec or pnpm run with pinned devDeps. + +**Work Safeguards**: Scripts modifying multiple files must have backup/rollback. Git operations that rewrite history require explicit confirmation. + +**Review checklist:** + +1. **Secrets**: Hardcoded API keys, passwords, tokens, private keys in code or config +2. **Injection**: Command injection via shell: true or string interpolation in spawn/exec. Path traversal in file operations. +3. **Dependencies**: npx/dlx usage. Unpinned versions (^ or ~). Missing minimumReleaseAge bypass justification. +4. **File operations**: fs.rm without safeDelete. process.chdir usage. fetch() usage (must use lib's httpRequest). +5. **GitHub Actions**: Unpinned action versions (must use full SHA). Secrets outside env blocks. Template injection from untrusted inputs. +6. **Error handling**: Sensitive data in error messages. Stack traces exposed to users. + +For each finding, report: +- **Severity**: CRITICAL / HIGH / MEDIUM / LOW +- **Location**: file:line +- **Issue**: what's wrong +- **Fix**: how to fix it + +Run `pnpm audit` for dependency vulnerabilities. Run `pnpm run security` for config/workflow scanning. diff --git a/.claude/commands/quality-loop.md b/.claude/commands/quality-loop.md index a818f350..ddd59375 100644 --- a/.claude/commands/quality-loop.md +++ b/.claude/commands/quality-loop.md @@ -1,46 +1,21 @@ -Run the quality-scan skill and fix all issues found. Repeat until zero issues remain or 5 iterations complete. +Run the `/quality-scan` skill and fix all issues found. Repeat until zero issues remain or 5 iterations complete. + +**Interactive only** — this command makes code changes and commits. Do not use as an automated pipeline gate. ## Process -1. Run quality-scan skill -2. If issues found: fix ALL of them -3. Run quality-scan again -4. Repeat until: +1. Run `/quality-scan` skill (all scan types) +2. If issues found: spawn the `refactor-cleaner` agent (see `agents/refactor-cleaner.md`) to fix them, grouped by category +3. Run verify-build (see `_shared/verify-build.md`) after fixes +4. Run `/quality-scan` again +5. Repeat until: - Zero issues found (success), OR - 5 iterations completed (stop) -5. Commit all fixes with message: "fix: resolve quality scan issues (iteration N)" +6. Commit all fixes: `fix: resolve quality scan issues (iteration N)` ## Rules -- Fix every issue, not just "easy" ones -- Do not skip architectural fixes +- Fix every issue, not just easy ones +- Spawn refactor-cleaner with CLAUDE.md's pre-action protocol: dead code first, then structural changes, ≤5 files per phase - Run tests after fixes to verify nothing broke - Track iteration count and report progress - -## Outstanding Architectural Issue (Requires Design Review) - -### Checkpoint Cache Invalidation (High Severity) - -**Issue**: Source package changes don't invalidate checkpoints properly. - -**Root Cause**: Cache keys are computed AFTER `prepareExternalSources()` syncs source packages to additions/. Since cache keys hash files in additions/ (which are now synced), they match the checkpoint even though source packages changed. - -**Scenario**: -1. Developer modifies `packages/binject/src/socketsecurity/binject/file.c` -2. Runs `pnpm --filter node-smol-builder clean && pnpm build` -3. `prepareExternalSources()` syncs binject → additions/source-patched/src/socketsecurity/binject/ -4. Cache key computed from additions/ files matches old checkpoint -5. Build restores stale checkpoint, skips recompilation -6. **Result**: Binary contains old binject code - -**Impact**: Silent build incorrectness when modifying source packages - -**Proposed Solutions** (require architectural review): -- Option 1: Include source package mtimes in cache key metadata -- Option 2: Make `prepareExternalSources()` idempotent, always re-sync - -**Files Affected**: -- packages/node-smol-builder/scripts/common/shared/build.mjs (collectBuildSourceFiles) -- packages/node-smol-builder/scripts/common/shared/checkpoints.mjs (cache key generation) - -**Status**: Documented for architectural review and future implementation diff --git a/.claude/commands/release-changelog.md b/.claude/commands/release-changelog.md new file mode 100644 index 00000000..c119573b --- /dev/null +++ b/.claude/commands/release-changelog.md @@ -0,0 +1,3 @@ +Run the `/release` skill starting from the changelog phase. + +If you want the full release pipeline (quality gate → security gate → changelog → version bump), run `/release` instead. diff --git a/.claude/commands/security-scan.md b/.claude/commands/security-scan.md new file mode 100644 index 00000000..6c629680 --- /dev/null +++ b/.claude/commands/security-scan.md @@ -0,0 +1,3 @@ +Run the `/security-scan` skill. This chains AgentShield (Claude config audit) → zizmor (GitHub Actions security) → security-reviewer agent (grading). + +For a quick manual run without the full pipeline: `pnpm run security` diff --git a/.claude/ops/queue.yaml b/.claude/ops/queue.yaml new file mode 100644 index 00000000..df32751e --- /dev/null +++ b/.claude/ops/queue.yaml @@ -0,0 +1,12 @@ +schema_version: 1 + +phase_order: + quality-scan: [env-check, dep-update, tools-install, cleanup, structural, scan-scope, scans, aggregate, report, complete] + security-scan: [env-check, agentshield, zizmor, grade-report] + updating: [env-check, npm-update, validate, report] + ci-cascade: [identify, layer-prs, layer-4-update, propagate, verify] + release: [quality-gate, security-gate, changelog, version-bump] + +# Completed runs are appended here by skills. Prune periodically — +# keep the last 10 entries and delete older ones to avoid unbounded growth. +runs: [] diff --git a/.claude/skills/_shared/env-check.md b/.claude/skills/_shared/env-check.md new file mode 100644 index 00000000..ee2c203d --- /dev/null +++ b/.claude/skills/_shared/env-check.md @@ -0,0 +1,27 @@ +# Environment Check + +Shared prerequisite validation for all pipelines. Run at the start of every skill. + +## Steps + +1. Run `git status` to check working directory state +2. Detect CI mode: check for `GITHUB_ACTIONS` or `CI` environment variables +3. Verify `node_modules/` exists (run `pnpm install` if missing) +4. Verify on a valid branch (`git branch --show-current`) + +## Behavior + +- **Clean working directory**: proceed normally +- **Dirty working directory**: warn and continue (most skills are read-only or create their own commits) +- **CI mode**: set `CI_MODE=true` — skills should skip interactive prompts and local-only validation +- **Missing node_modules**: run `pnpm install` before proceeding + +## Queue Tracking + +Write a run entry to `.claude/ops/queue.yaml` with: +- `id`: `{pipeline}-{YYYY-MM-DD}-{NNN}` +- `pipeline`: the invoking skill name +- `status`: `in-progress` +- `started`: current UTC timestamp +- `current_phase`: `env-check` +- `completed_phases`: `[]` diff --git a/.claude/skills/_shared/report-format.md b/.claude/skills/_shared/report-format.md new file mode 100644 index 00000000..3e8cdce7 --- /dev/null +++ b/.claude/skills/_shared/report-format.md @@ -0,0 +1,47 @@ +# Report Format + +Shared output format for all scan and review pipelines. + +## Finding Format + +Each finding: +``` +- **[SEVERITY]** file:line — description + Fix: how to fix it +``` + +Severity levels: CRITICAL, HIGH, MEDIUM, LOW + +## Grade Calculation + +Based on finding severity distribution: +- **A** (90-100): 0 critical, 0 high +- **B** (80-89): 0 critical, 1-3 high +- **C** (70-79): 0 critical, 4+ high OR 1 critical +- **D** (60-69): 2-3 critical +- **F** (< 60): 4+ critical + +## Pipeline HANDOFF + +When a skill completes as part of a larger pipeline (e.g., quality-scan within release), +output a structured handoff block: + +``` +=== HANDOFF: {skill-name} === +Status: {pass|fail} +Grade: {A-F} +Findings: {critical: N, high: N, medium: N, low: N} +Summary: {one-line description} +=== END HANDOFF === +``` + +The parent pipeline reads this to decide whether to proceed (gate check) or abort. + +## Queue Completion + +When the final phase completes, update `.claude/ops/queue.yaml`: +- `status`: `done` (or `failed`) +- `completed`: current UTC timestamp +- `current_phase`: `~` (null) +- `completed_phases`: full list +- `findings_count`: `{critical: N, high: N, medium: N, low: N}` diff --git a/.claude/skills/_shared/security-tools.md b/.claude/skills/_shared/security-tools.md new file mode 100644 index 00000000..9a1f0271 --- /dev/null +++ b/.claude/skills/_shared/security-tools.md @@ -0,0 +1,41 @@ +# Security Tools + +Shared tool detection for security scanning pipelines. + +## AgentShield + +Installed as a pinned devDependency (`ecc-agentshield` in pnpm-workspace.yaml catalog). +Run via: `pnpm exec agentshield scan` +No install step needed — available after `pnpm install`. + +## Zizmor + +Not an npm package. Installed via `pnpm run setup` which downloads the pinned version +from GitHub releases with SHA256 checksum verification (see `external-tools.json`). + +The binary is cached at `.cache/external-tools/zizmor/{version}-{platform}/zizmor`. + +Detection order: +1. `command -v zizmor` (if already on PATH, e.g. via brew) +2. `.cache/external-tools/zizmor/*/zizmor` (from `pnpm run setup`) + +Run via the full path if not on PATH: +```bash +ZIZMOR="$(find .cache/external-tools/zizmor -name zizmor -type f 2>/dev/null | head -1)" +if [ -z "$ZIZMOR" ]; then ZIZMOR="$(command -v zizmor 2>/dev/null)"; fi +if [ -n "$ZIZMOR" ]; then "$ZIZMOR" .github/; else echo "zizmor not installed — run pnpm run setup"; fi +``` + +If not available: +- Warn: "zizmor not installed — run `pnpm run setup` to install" +- Skip the zizmor phase (don't fail the pipeline) + +## Socket CLI + +Optional. Used for dependency scanning in the updating and security-scan pipelines. + +Detection: `command -v socket` + +If not available: +- Skip socket-scan phases gracefully +- Note in report: "Socket CLI not available — dependency scan skipped" diff --git a/.claude/skills/_shared/verify-build.md b/.claude/skills/_shared/verify-build.md new file mode 100644 index 00000000..5dc82c03 --- /dev/null +++ b/.claude/skills/_shared/verify-build.md @@ -0,0 +1,22 @@ +# Verify Build + +Shared build/test/lint validation. Referenced by skills that modify code or dependencies. + +## Steps + +Run in order, stop on first failure: + +1. `pnpm run fix --all` — auto-fix lint and formatting issues +2. `pnpm run check --all` — lint + typecheck + validation (read-only, fails on violations) +3. `pnpm test` — full test suite + +## CI Mode + +When `CI_MODE=true` (detected by env-check), skip this validation entirely. +CI runs these checks in its own matrix (Node 20/22/24 × ubuntu/windows). + +## On Failure + +- Report which step failed with the error output +- Do NOT proceed to the next pipeline phase +- Mark the pipeline run as `status: failed` in `.claude/ops/queue.yaml` diff --git a/.claude/skills/ci-cascade/SKILL.md b/.claude/skills/ci-cascade/SKILL.md new file mode 100644 index 00000000..b2cecb25 --- /dev/null +++ b/.claude/skills/ci-cascade/SKILL.md @@ -0,0 +1,87 @@ +--- +name: ci-cascade +description: Execute the GitHub Actions SHA pin cascade when a socket-registry action changes. Creates PRs in dependency order, waits for merges, and propagates the final SHA to all consuming repos. +--- + +# CI SHA Pin Cascade + +Implements the cascade procedure defined in CLAUDE.md § "GitHub Actions SHA Pin Cascade (CRITICAL)". + +## When to Use + +- After modifying any GitHub Action in `.github/actions/` +- After modifying a reusable workflow in `.github/workflows/ci.yml` or `provenance.yml` +- When a dependency (Node.js version, pnpm version, sfw-free) changes in an action + +## Procedure + +Follow `_shared/env-check.md` to initialize a queue run entry for `ci-cascade`. + +Follow the layer order exactly. Each layer gets its own PR. Never combine layers. + +### Phase 1: Identify what changed + +Update queue: advance `current_phase` in `.claude/ops/queue.yaml` + +Determine which layer was modified: +- Layer 1 (leaf actions): checkout, install, debug, setup-git-signing, cleanup-git-signing, run-script, artifacts, cache-npm-packages +- Layer 2a (setup): references debug +- Layer 2b (setup-and-install): references checkout, setup, install +- Layer 3 (reusable workflows): ci.yml, provenance.yml +- Layer 4 (local wrappers): _local-not-for-reuse-* + +### Phase 2: Create PRs in order + +Update queue: advance `current_phase` in `.claude/ops/queue.yaml` + +Starting from the layer above the change, create a PR for each layer: + +1. **Get current SHA**: `git fetch origin main && git rev-parse origin/main` +2. **Create branch**: `git checkout -b chore/ci-cascade-layer-N` +3. **Update refs**: Replace old SHA with new SHA in all files at this layer +4. **Verify**: `grep -rn "SocketDev/socket-registry" .github/ | grep "@" | grep -v ""` +5. **Don't clobber**: Never replace third-party SHAs (actions/checkout, actions/upload-artifact, etc.) +6. **Commit**: `chore(ci): bump socket-registry action refs to main ()` +7. **Push and create PR** +8. **Wait for merge** before proceeding to next layer + +### Phase 3: Update Layer 4 (local wrappers) + +Update queue: advance `current_phase` in `.claude/ops/queue.yaml` + +After Layer 3 merges, get the **propagation SHA** (Layer 3 merge SHA). Update the `_local-not-for-reuse-*` workflows to pin to this SHA. Create a PR, merge it. + +The Layer 4 merge SHA is NOT used for external pinning — external repos pin to the Layer 3 SHA because that's where the reusable workflows (ci.yml, provenance.yml) were updated. + +### Phase 4: Propagate to external repos + +Update queue: advance `current_phase` in `.claude/ops/queue.yaml` + +All external repos pin to the **propagation SHA** (Layer 3 merge SHA). + +For each repo: +1. Update all `SocketDev/socket-registry/.github/` refs to the propagation SHA +2. Push directly to main where allowed (socket-btm, socket-sbom-generator, ultrathink) +3. Create PRs where branch protection requires it (socket-cli, socket-lib, socket-sdk-js, socket-packageurl-js) + +### Phase 5: Verify + +Update queue: advance `current_phase` in `.claude/ops/queue.yaml` + +For each updated repo, confirm no old SHAs remain: +```bash +grep -rn "SocketDev/socket-registry" .github/ | grep "@" | grep -v "" +``` + +### Completion + +Output a HANDOFF block per `_shared/report-format.md`: + +``` +=== HANDOFF: ci-cascade === +Status: {pass|fail} +Summary: {one-line: "Propagated SHA XXXXX to N repos"} +=== END HANDOFF === +``` + +Update queue: `status: done`, write completion timestamp. diff --git a/.claude/skills/quality-scan/SKILL.md b/.claude/skills/quality-scan/SKILL.md index 7b895a54..6381af40 100644 --- a/.claude/skills/quality-scan/SKILL.md +++ b/.claude/skills/quality-scan/SKILL.md @@ -36,7 +36,7 @@ All agent prompts are embedded in `reference.md` with structured , **CRITICAL Requirements:** -- Read-only analysis (no code changes during scan) +- Analysis phase (dependency updates and cleanup happen in early phases, code scanning is read-only) - Must complete all enabled scans before reporting - Findings must be prioritized by severity (Critical → High → Medium → Low) - Must generate actionable tasks with file:line references @@ -64,30 +64,16 @@ Execute the following phases sequentially to perform comprehensive quality analy ### Phase 1: Validate Environment - -Verify the environment before starting scans: - +Update queue: advance `current_phase` in `.claude/ops/queue.yaml` -```bash -git status -``` - - -**Expected State:** -- Working directory should be clean (warn if dirty but continue) -- On a valid branch -- Node modules installed - -**If working directory dirty:** -- Warn user: "Working directory has uncommitted changes - continuing with scan" -- Continue with scans (quality scanning is read-only) - - +Follow `_shared/env-check.md` to validate the environment and initialize a queue run entry for `quality-scan`. --- ### Phase 2: Update Dependencies +Update queue: advance `current_phase` in `.claude/ops/queue.yaml` + Update dependencies in the current repository only: @@ -114,61 +100,16 @@ pnpm run update ### Phase 2b: Install External Tools (zizmor) - -Install zizmor for GitHub Actions security scanning using version that meets repository's minimumReleaseAge policy. - +Update queue: advance `current_phase` in `.claude/ops/queue.yaml` - -Determine the appropriate zizmor version dynamically: - -1. **Read minimumReleaseAge from `.pnpmrc`**: - ```bash - grep 'minimumReleaseAge' .pnpmrc | cut -d'=' -f2 - ``` - This returns minutes (e.g., `10080` = 7 days). Default to 10080 if not found. - -2. **Query zizmor releases** (using curl or gh): - ```bash - # Option A: curl (universally available) - curl -s "https://api.github.com/repos/zizmorcore/zizmor/releases" | \ - jq '[.[] | select(.prerelease == false) | {tag: .tag_name, date: .published_at}] | .[0:10]' - - # Option B: gh (if available) - gh api repos/zizmorcore/zizmor/releases --jq \ - '[.[] | select(.prerelease == false) | {tag: .tag_name, date: .published_at}] | .[0:10]' - ``` - -3. **Calculate age and select version**: - - Convert minimumReleaseAge from minutes to days: `minutes / 1440` - - Find latest stable release older than that threshold - - Example: If minimumReleaseAge=10080 (7 days) and today is March 24, select releases from March 17 or earlier - -4. **Install selected version** (choose based on available tools): - ```bash - # macOS with Homebrew (latest only, version pinning limited) - brew install zizmor - - # Python environments (version pinning supported) - pipx install zizmor==VERSION - uv tool install zizmor==VERSION - uvx zizmor@VERSION --help - ``` - - **Recommended priority**: pipx/uvx > brew - - - -Using minimumReleaseAge prevents supply chain attacks from compromised new releases. The 7-day window allows community detection of malicious packages before adoption. - - - -If no release meets the age requirement, warn the user and skip zizmor scan. Never install a release younger than minimumReleaseAge. - +See `_shared/security-tools.md` for zizmor detection. Use `pnpm run setup` to install pinned tools. --- ### Phase 3: Repository Cleanup +Update queue: advance `current_phase` in `.claude/ops/queue.yaml` + Clean up junk files and organize the repository before scanning: @@ -226,16 +167,13 @@ find . -type f -name '*.log' \ **For each file found:** 1. Show the file path to user 2. Explain why it's considered junk -3. Ask user for confirmation before deleting (use AskUserQuestion) -4. Delete confirmed files: `git rm` if tracked, `rm` if untracked -5. Report files removed +3. In interactive mode: ask user for confirmation before deleting +4. In CI mode or when called as a pipeline gate: auto-delete without prompting +5. Delete confirmed files: `git rm` if tracked, `rm` if untracked +6. Report files removed **If no junk files found:** - Report: "✓ Repository is clean - no junk files found" - -**Important:** -- Always get user confirmation before deleting -- Show file contents if user is unsure - Track deleted files for reporting @@ -244,16 +182,18 @@ find . -type f -name '*.log' \ ### Phase 4: Structural Validation +Update queue: advance `current_phase` in `.claude/ops/queue.yaml` + Run automated consistency checker to validate architectural patterns: **Validation Tasks:** -Run the consistency checker to validate monorepo structure: +Run the check script to validate monorepo structure: ```bash -node scripts/check-consistency.mjs +pnpm run check --all ``` **The consistency checker validates:** @@ -293,6 +233,8 @@ node scripts/check-consistency.mjs ### Phase 5: Determine Scan Scope +Update queue: advance `current_phase` in `.claude/ops/queue.yaml` + Ask user which scans to run: @@ -306,27 +248,25 @@ Ask user which scans to run: 6. **security** - GitHub Actions security (template injection, cache poisoning, etc.) 7. **documentation** - Documentation accuracy (README errors, outdated docs) -**User Interaction:** -Use AskUserQuestion tool: -- Question: "Which quality scans would you like to run?" -- Header: "Scan Types" -- multiSelect: true -- Options: - - "All scans (recommended)" → Run all 4 scan types - - "Critical only" → Run critical scan only - - "Critical + Logic" → Run critical and logic scans - - "Custom selection" → Ask user to specify which scans +**Scan Selection:** +- In CI mode or when called as a pipeline gate (e.g. by `/release`): run all scans automatically, no prompting +- In interactive mode: ask the user which scans to run: + - "All scans (recommended)" — run all scan types + - "Critical only" — run critical scan only + - "Critical + Logic" — run critical and logic scans + - "Custom selection" — ask user to specify -**Default:** If user doesn't specify, run all scans. +**Default:** If not specified, run all scans. Validate selected scan types exist in reference.md: -- critical-scan → reference.md line ~5 -- logic-scan → reference.md line ~100 -- cache-scan → reference.md line ~200 -- workflow-scan → reference.md line ~300 -- security-scan → reference.md line ~400 -- documentation-scan → reference.md line ~810 +- critical-scan → reference.md § "Critical Scan Agent" +- logic-scan → reference.md § "Logic Scan Agent" +- cache-scan → reference.md § "Cache Scan Agent" +- workflow-scan → reference.md § "Workflow Scan Agent" +- security-scan → reference.md § "Security Scan Agent" +- workflow-optimization-scan → reference.md § "Workflow Optimization Scan Agent" +- documentation-scan → reference.md § "Documentation Scan Agent" If user requests non-existent scan type, report error and suggest valid types. @@ -335,16 +275,16 @@ If user requests non-existent scan type, report error and suggest valid types. ### Phase 6: Execute Scans +Update queue: advance `current_phase` in `.claude/ops/queue.yaml` + -For each enabled scan type, spawn a specialized agent using Task tool: +For each enabled scan type, spawn a specialized agent using Agent tool: -```typescript -// Example: Critical scan -Task({ - subagent_type: "general-purpose", - description: "Critical bugs scan", - prompt: `${CRITICAL_SCAN_PROMPT_FROM_REFERENCE_MD} +``` +Example: Critical scan via Agent tool + +Prompt the agent with the CRITICAL_SCAN_PROMPT from reference.md, adding: [IF monorepo] Focus on packages/ directories and root-level scripts/. [IF single package] Focus on src/, lib/, and scripts/ directories. @@ -358,14 +298,13 @@ Report findings in this format: - Fix: Suggested fix - Impact: What happens if triggered -Scan systematically and report all findings. If no issues found, state that explicitly.` -}) +Scan systematically and report all findings. If no issues found, state that explicitly. ``` **For each scan:** 1. Load agent prompt template from `reference.md` 2. Customize for repository context (determine monorepo vs single package structure) -3. Spawn agent with Task tool using "general-purpose" subagent_type +3. Spawn agent with Agent tool using "general-purpose" subagent_type 4. Capture findings from agent response 5. Parse and categorize results @@ -375,14 +314,18 @@ Scan systematically and report all findings. If no issues found, state that expl - cache - workflow (lowest priority) +**Agent Rules:** +- For critical, logic, and cache scans: the agent should apply the rules from `agents/code-reviewer.md` (code style, patterns, error handling) in addition to the scan-type-specific prompt from reference.md. +- For the security scan type: the agent should apply the rules from `agents/security-reviewer.md` (safe file ops, secret detection, dependency rules). + **Agent Prompt Sources:** -- Critical scan: reference.md starting at line ~12 -- Logic scan: reference.md starting at line ~100 -- Cache scan: reference.md starting at line ~200 -- Workflow scan: reference.md starting at line ~300 -- Security scan: reference.md starting at line ~400 -- Workflow-optimization scan: reference.md starting at line ~860 -- Documentation scan: reference.md starting at line ~1040 +- Critical scan: reference.md § "Critical Scan Agent" +- Logic scan: reference.md § "Logic Scan Agent" +- Cache scan: reference.md § "Cache Scan Agent" +- Workflow scan: reference.md § "Workflow Scan Agent" +- Security scan: reference.md § "Security Scan Agent" +- Workflow-optimization scan: reference.md § "Workflow Optimization Scan Agent" +- Documentation scan: reference.md § "Documentation Scan Agent" For each scan completion: @@ -396,6 +339,8 @@ For each scan completion: ### Phase 7: Aggregate Findings +Update queue: advance `current_phase` in `.claude/ops/queue.yaml` + Collect all findings from agents and aggregate: @@ -435,6 +380,8 @@ interface Finding { ### Phase 8: Generate Report +Update queue: advance `current_phase` in `.claude/ops/queue.yaml` + Create structured quality report with all findings: @@ -517,8 +464,9 @@ Create structured quality report with all findings: ``` **Output Report:** -1. Display report to console (user sees it) -2. Offer to save to file (optional): `reports/quality-scan-YYYY-MM-DD.md` +1. Display report to console +2. In interactive mode: offer to save to file (`reports/quality-scan-YYYY-MM-DD.md`) +3. In CI mode or pipeline gate mode: skip save prompt **Report Quality Checks:** @@ -533,9 +481,18 @@ Create structured quality report with all findings: ### Phase 9: Complete +Update queue: advance `current_phase` in `.claude/ops/queue.yaml` + -```xml -QUALITY_SCAN_COMPLETE +Output a HANDOFF block per `_shared/report-format.md`: + +``` +=== HANDOFF: quality-scan === +Status: {pass|fail} +Grade: {A-F} +Findings: {critical: N, high: N, medium: N, low: N} +Summary: {one-line description} +=== END HANDOFF === ``` @@ -587,7 +544,7 @@ All findings include file:line references and suggested fixes. ## Success Criteria -- ✅ `QUALITY_SCAN_COMPLETE` output +- ✅ HANDOFF block output per `_shared/report-format.md` - ✅ All enabled scans completed without errors - ✅ Findings prioritized by severity (Critical → Low) - ✅ All findings include file:line references @@ -611,13 +568,13 @@ All agent prompts follow Claude best practices with , , < ## Commands -This skill is self-contained. No external commands needed. +Requires: `git`, `pnpm`, `node`, and optionally `zizmor` (see `_shared/security-tools.md`). ## Context This skill provides systematic code quality analysis by: - Spawning specialized agents for targeted analysis -- Using Task tool to run agents autonomously +- Using Agent tool to run agents autonomously - Embedding agent prompts in reference.md following best practices - Generating prioritized, actionable reports - Supporting partial scans (user can select specific scan types) diff --git a/.claude/skills/quality-scan/reference.md b/.claude/skills/quality-scan/reference.md index 8f91a0a2..d80de687 100644 --- a/.claude/skills/quality-scan/reference.md +++ b/.claude/skills/quality-scan/reference.md @@ -17,13 +17,13 @@ Examples: **BAD - Ignoring return values and reconstructing paths:** ```javascript -await downloadSocketBtmRelease({ asset, downloadDir, tool: 'lief' }) -const downloadedPath = path.join(downloadDir, 'lief', 'assets', asset) // ❌ Assumes path structure +await downloadAsset({ asset, downloadDir, tool: 'package' }) +const downloadedPath = path.join(downloadDir, 'package', 'assets', asset) // ❌ Assumes path structure ``` **GOOD - Use the return value:** ```javascript -const downloadedPath = await downloadSocketBtmRelease({ asset, downloadDir }) // ✅ Simple, trust the function +const downloadedPath = await downloadAsset({ asset, downloadDir }) // ✅ Simple, trust the function ``` **Principle**: If a function returns what you need, use it. Don't reconstruct or assume. @@ -57,7 +57,6 @@ Common characteristics to look for: Scan all code files for these critical bug patterns: - [IF monorepo] TypeScript/JavaScript: packages/npm/*/scripts/**/*.{mjs,mts}, registry/lib/**/*.js, scripts/**/*.{mjs,mts} - [IF single package] TypeScript/JavaScript: src/**/*.{ts,mts,mjs,js}, lib/**/*.{ts,mts,mjs,js} -- [IF C/C++ code exists] C/C++: src/**/*.{c,cc,cpp,h} - Focus on: @@ -101,92 +100,6 @@ Scan all code files for these critical bug patterns: - Buffer operations without size checks - -**CRITICAL for Node.js internal code (IF repository has Node.js internals):** -- Direct Promise method calls (Promise.all, Promise.allSettled, Promise.race, Promise.any) instead of primordials -- Missing primordials import in internal/ modules -- Using Promise.all where Promise.allSettled would be safer for error collection -- [SOCKET-BTM SPECIFIC] Check additions/source-patched/lib/internal/socketsecurity/smol/*.js for SafePromiseAllSettled usage -- [SOCKET-BTM SPECIFIC] Check additions/source-patched/deps/fast-webstreams/*.js for SafePromiseAllReturnVoid usage -- [SOCKET-BTM SPECIFIC] Verify sync scripts (vendor-fast-webstreams/sync.mjs) inject primordials when syncing from upstream - - - -**[SOCKET-BTM SPECIFIC] CRITICAL for binary tooling (binject, binpress, stubs-builder):** -Cross-platform binary processing that uses compile-time platform detection: -- `#ifdef __APPLE__` / `#ifdef _WIN32` selecting sizes, offsets, or behaviors for binary operations -- Host platform detection instead of target binary format detection -- Example buggy pattern (causes "Cannot inject into uncompressed stub" errors): - ```c - #ifdef __APPLE__ - size_t search_size = SEARCH_SIZE_MACHO; // Uses macOS size even for Linux ELF binaries! - #else - size_t search_size = SEARCH_SIZE_ELF; - #endif - ``` -- Fix: Use runtime binary format detection based on the executable being processed: - ```c - binject_format_t format = binject_detect_format(executable); - size_t search_size = get_search_size_for_format(format); - ``` -- Impact: Cross-platform CI (e.g., macOS building Linux binaries) silently produces broken binaries - - - -**[SOCKET-BTM SPECIFIC] CRITICAL for Windows binary tooling:** -Windows PE binaries missing required .rsrc sections for LIEF injection: -- LIEF cannot create resource sections from scratch - must exist in the binary -- Symptoms: "Binary has no resources section" error during SEA injection -- Check Windows Makefiles (Makefile.windows) for: - 1. Missing .rc resource file compilation with windres - 2. Missing RESOURCE_OBJ in the final link step -- Required for any PE binary that will receive NODE_SEA_BLOB or other injected resources - - - -**[SOCKET-BTM SPECIFIC] CRITICAL for test reliability and CI stability:** -Tests compressing binsuite binaries (binpress, binflate, binject) as test input cause: -- Flaky/slow tests: These binaries vary between builds, causing inconsistent test behavior -- CI timeouts: Large binary compression takes excessive time -- Parallel test interference: Tests using static paths collide in parallel runs - -**Bad patterns to flag:** -```typescript -// BAD - compressing binsuite tools themselves -const originalBinary = BINPRESS // or BINFLATE, BINJECT -await execCommand(BINPRESS, [BINFLATE, '-o', output]) - -// BAD - static testDir paths (parallel collision risk) -const testDir = path.join(PACKAGE_DIR, 'test-tmp') -``` - -**Correct patterns:** -```typescript -// GOOD - use Node.js binary (consistent across builds) -const NODE_BINARY = process.execPath -let testDir: string -let testBinary: string - -beforeAll(async () => { - // Unique testDir isolates parallel test runs - const uniqueId = `${Date.now()}-${Math.random().toString(36).slice(2, 8)}` - testDir = path.join(os.tmpdir(), `package-test-${uniqueId}`) - await safeMkdir(testDir) - // Copy Node.js as consistent test input - testBinary = path.join(testDir, 'test-node') - await fs.copyFile(NODE_BINARY, testBinary) -}) -``` - -**Where to check:** -- packages/npm/*/test/*.test.mts -- test/**/*.test.js -- test/**/*.test.mts -- Any test that uses external package managers or performs heavy I/O operations - -**Impact:** Tests performing intensive npm operations can cause CI timeouts - - For each bug found, think through: 1. Can this actually crash in production? 2. What input would trigger it? @@ -213,21 +126,12 @@ Trigger: When operation fails without error handling Fix: `await asyncOperation().catch(err => { logger.error(err); throw new Error(\`Operation failed: \${err.message}\`) })` Impact: Uncaught exception crashes process -Example (C/C++): -File: src/path/to/file.c:234 -Issue: Potential null pointer dereference after malloc -Severity: Critical -Pattern: `uint8_t* buffer = malloc(size); memcpy(buffer, data, size);` -Trigger: When malloc fails due to insufficient memory -Fix: `uint8_t* buffer = malloc(size); if (!buffer) return ERROR_MEMORY; memcpy(buffer, data, size);` -Impact: Segmentation fault crashes process - Only report actual bugs, not style issues or minor improvements - Verify bugs are not already handled by surrounding code - Prioritize bugs affecting reliability and correctness -- For C/C++: Focus on memory safety, null checks, buffer overflows - For TypeScript: Focus on promise handling, type guards, external input validation - Skip false positives (TypeScript type guards are sufficient in many cases) - [IF monorepo] Scan across all packages systematically @@ -298,73 +202,6 @@ Algorithm implementation issues: - Deduplication: Missing deduplication of duplicate items - -**[SOCKET-BTM SPECIFIC] Patch application logic errors:** -- Unified diff parsing: Line offset calculation errors, context matching failures -- Hunk application: Off-by-one in line number calculations -- Patch validation: Missing validation of patch format (malformed hunks) -- Backup/restore: Not properly handling patch failures mid-application -- Independent patches: Assumptions about patch ordering or dependencies - - - -**[SOCKET-BTM SPECIFIC] Binary format handling errors:** -- Format detection: Misidentifying ELF/Mach-O/PE headers -- Section/segment: Off-by-one in offset calculations, size validation missing -- Endianness: Not handling big-endian vs little-endian correctly -- Alignment: Missing alignment requirements for injected data -- Cross-platform: Windows vs Unix path separators, line endings - - - -**[SOCKET-BTM SPECIFIC] Cross-platform binary processing bugs (CRITICAL - causes silent failures):** -- Using `#ifdef __APPLE__` / `#ifdef _WIN32` to select binary format-specific behavior -- Code uses host platform instead of target binary format for size/offset calculations -- Magic marker search using compile-time constants instead of runtime binary format detection -- Example bug pattern: - ```c - // BAD - uses compile-time platform, breaks cross-platform builds - #ifdef __APPLE__ - size_t search_size = 64 * 1024; // Mach-O size - #elif defined(_WIN32) - size_t search_size = 128 * 1024; // PE size - #else - size_t search_size = 1408 * 1024; // ELF size - #endif - - // GOOD - runtime detection based on binary being processed - binject_format_t format = binject_detect_format(executable); - size_t search_size = get_search_size_for_format(format); - ``` -- Symptoms: "Cannot inject into uncompressed stub binary" when building Linux binaries on macOS -- Critical for: binject, binpress, stubs-builder, any code manipulating binaries cross-platform -- Real-world impact: macOS CI processing Linux ELF binaries uses wrong search size, fails to find markers - - - -**[SOCKET-BTM SPECIFIC] Windows PE binaries missing required resource sections:** -- PE stub binaries without .rsrc section cannot receive LIEF-injected resources -- LIEF cannot create resource sections from scratch - binary must have existing resources -- Symptoms: "Binary has no resources section" error during Windows SEA injection -- Check Windows Makefiles for: - 1. Missing .rc resource file (stub.rc or similar) - 2. Missing windres compilation step: `$(WINDRES) -i $(RESOURCE_RC) -o $@` - 3. Missing resource object in link step: `$(CC) ... $(RESOURCE_OBJ) ...` -- Required for binaries that need NODE_SEA_BLOB or other injected resources -- Example fix for Makefile.windows: - ```makefile - RESOURCE_RC = src/socketsecurity/stubs-builder/stub.rc - RESOURCE_OBJ = $(OUT_DIR)/stub.res.o - WINDRES ?= windres - - $(RESOURCE_OBJ): $(RESOURCE_RC) | $(OUT_DIR) - $(WINDRES) -i $(RESOURCE_RC) -o $@ - - $(OUT_DIR)/$(TARGET): $(SOURCE) $(RESOURCE_OBJ) ... - $(CC) ... $(SOURCE) $(RESOURCE_OBJ) ... - ``` - - Before reporting, think through: 1. Does this logic error produce incorrect output? 2. What specific input would trigger it? @@ -391,14 +228,6 @@ Pattern: `for (let i = 0; i < items.length - 1; i++)` Fix: `for (let i = 0; i < items.length; i++)` Impact: Last item is silently omitted, causing incorrect processing -Example (C code): -File: src/path/to/file.c:234 -Issue: Incorrect size calculation with alignment -Severity: High -Edge Case: When data requires alignment -Pattern: `new_size = existing_size + data_size;` -Fix: `new_size = ALIGN_UP(existing_size + data_size, alignment);` -Impact: Misaligned data causes segfault @@ -406,7 +235,6 @@ Impact: Misaligned data causes segfault - Focus on errors affecting correctness and data integrity - Verify logic errors aren't false alarms due to type narrowing - Consider real-world edge cases: malformed input, unusual formats, cross-platform paths -- [IF C/C++ exists] Pay special attention to pointer arithmetic and buffer calculations Analyze systematically and report all logic errors found. If no errors are found, state that explicitly. @@ -425,7 +253,7 @@ Analyze systematically and report all logic errors found. If no errors are found Your task is to analyze caching implementation for correctness, staleness bugs, and performance issues. Focus on cache corruption, invalidation failures, and race conditions. -**[SOCKET-BTM SPECIFIC - Adapt for your repository's caching strategy]** +[CONDITIONAL: Adapt for your repository's caching strategy] This project uses caching for npm package overrides and registry data: - **Storage**: Package override caching in packages/npm/* @@ -458,7 +286,7 @@ Checkpoint key generation correctness: - Patch ordering: Does key depend on patch application order? - Platform isolation: Are Windows/macOS/Linux checkpoints properly separated? - Arch isolation: Are ARM64/x64 checkpoints kept separate? -- Additions: Are build-infra/binject changes invalidating checkpoints? +- Dependencies: Are dependency changes invalidating caches? - Environment: Are env vars (NODE_OPTIONS, etc.) affecting builds included? **NOTE**: Platform-agnostic operations (npm package parsing) may share cache keys across platforms, @@ -489,7 +317,7 @@ Scenarios producing stale/incorrect checkpoints: - Platform mismatch: Restoring darwin checkpoint on linux - Arch mismatch: Restoring arm64 checkpoint for x64 build - Version mismatch: Node.js version changed but checkpoint reused -- Additions changed: build-infra/binject updated but checkpoint not invalidated +- Dependencies changed: Updated dependencies but checkpoint not invalidated - Environment drift: Build flags changed but cache key unchanged @@ -544,28 +372,29 @@ Analyze the checkpoint implementation thoroughly across all checkpoint stages an ### Workflow Scan Agent -**Mission**: Detect problems in build scripts, CI configuration, git hooks, and developer workflows across the socket-btm monorepo. +**Mission**: Detect problems in build scripts, CI configuration, git hooks, and developer workflows across the socket-registry monorepo. -**Scan Targets**: All `scripts/`, `package.json`, `.git-hooks/*`, `.github/workflows/*` across packages +**Scan Targets**: All `scripts/`, `package.json`, `.git-hooks/*`, `.husky/*`, `.github/workflows/*` across packages **Prompt Template:** ``` -Your task is to identify issues in socket-btm's development workflows, build scripts, and CI configuration that could cause build failures, test flakiness, or poor developer experience. +Your task is to identify issues in socket-registry's development workflows, build scripts, and CI configuration that could cause build failures, test flakiness, or poor developer experience. -socket-btm is a pnpm monorepo with: +socket-registry is a pnpm monorepo with: +- **npm packages**: packages/npm/* (override packages published to npm) - **Build scripts**: scripts/**/*.{mjs,mts} (ESM, cross-platform Node.js) - **Package manager**: pnpm workspaces with scripts in each package.json -- **Git hooks**: .git-hooks/* for pre-commit, pre-push validation -- **CI**: GitHub Actions (.github/workflows/) -- **Platforms**: Must work on Windows, macOS, Linux (ARM64, x64) +- **Git hooks**: .git-hooks/* and .husky/* for pre-commit, pre-push validation +- **CI**: GitHub Actions (.github/workflows/) - reusable workflows consumed by other repos +- **Platforms**: Must work on Windows, macOS, Linux - **CLAUDE.md**: Defines conventions (no process.exit(), no backward compat, etc.) -- **Critical**: Build scripts compile C/C++ code and apply patches - must handle errors gracefully +- **Critical**: Build scripts generate npm override packages - must handle errors gracefully -Packages: -- node-smol-builder: Main build orchestration -- binject: C/C++ binary injection library -- bin-infra, build-infra: Utilities used by node-smol-builder +Key directories: +- packages/npm/ - Override npm packages +- registry/lib/ - Registry library code +- scripts/ - Build and maintenance scripts @@ -594,7 +423,7 @@ Error handling in scripts: Import style conventions (Socket Security standards): - Use `@socketsecurity/lib/logger` instead of custom log functions or cherry-picked console methods -- Use `@socketsecurity/lib/spawn` instead of `node:child_process` (except in `additions/` directory) +- Use `@socketsecurity/lib/spawn` instead of `node:child_process` - For Node.js built-in modules: **Cherry-pick fs, default import path/os/url/crypto** - For `fs`: cherry-pick sync methods, use promises namespace for async - For `child_process`: **avoid direct usage** - prefer `@socketsecurity/lib/spawn` @@ -609,7 +438,7 @@ Import style conventions (Socket Security standards): Examples of what to flag: - Custom log functions: `function log(msg) { console.log(msg) }` → use `@socketsecurity/lib/logger` -- Direct child_process usage (except in additions/): +- Direct child_process usage: - `import { execSync } from 'node:child_process'` → use `import { spawn } from '@socketsecurity/lib/spawn'` - `execSync('cmd arg1')` → use `await spawn('cmd', ['arg1'])` - Default imports for fs: @@ -650,12 +479,11 @@ Git hooks configuration: CI pipeline issues: - Build order: Are steps in correct sequence (install → build → test)? - Cross-platform: Are Windows/macOS/Linux builds all tested? -- C/C++ compilation: Are compiler toolchains (clang, gcc, MSVC) properly configured? -- Build artifacts: Are Node.js binaries uploaded for each platform? -- Checkpoint caching: Are build checkpoints cached across CI runs? +- Build artifacts: Are npm packages built and published correctly? +- Caching: Are node_modules and build outputs cached across CI runs? - Failure notifications: Are build failures clearly visible? -- Node.js versions: Are upstream Node.js version updates tested? -- Patch validation: Are patch files validated before application? +- Node.js versions: Are multiple Node.js versions tested? +- Reusable workflows: Are workflow inputs and outputs documented? @@ -665,56 +493,24 @@ Documentation and setup: -Build script architecture and helper methods (CRITICAL for consistent builds): +Build script architecture (CRITICAL for consistent package builds): **Package Build Entry Points:** -- Packages MUST use `scripts/build.mjs` as the build entry point, never direct Makefile invocation -- build.mjs handles: dependency downloads, environment setup, then Make invocation -- Direct `make -f Makefile.` bypasses critical setup (curl/LIEF downloads) - -**Required build.mjs patterns:** -```javascript -// CORRECT - uses buildBinSuitePackage from bin-infra -import { buildBinSuitePackage } from 'bin-infra/lib/builder' - -buildBinSuitePackage({ - packageName: 'tool-name', - packageDir: packageRoot, - beforeBuild: async () => { - // Download dependencies BEFORE Make runs - await ensureCurl() // stubs-builder - await ensureLief() // binject, binpress - }, - smokeTest: async (binaryPath) => { ... } -}) -``` - -**Dependency download helpers:** -- `ensureCurl()` - Downloads curl+mbedTLS from releases (stubs-builder) -- `ensureLief()` - Downloads LIEF library from releases (binject, binpress) -- `downloadSocketBtmRelease()` - Generic helper from `@socketsecurity/lib/releases/socket-btm` +- Use `pnpm run build` as the build entry point +- Build scripts in `scripts/` handle package generation and validation **Common mistakes to flag:** -1. Makefile invoked directly without pnpm wrapper: - - Bug: `make -f Makefile.macos` in documentation or scripts +1. Missing pnpm wrapper: + - Bug: Running scripts directly instead of through pnpm - Fix: Use `pnpm run build` or `pnpm --filter build` -2. Missing beforeBuild hook: - - Bug: build.mjs doesn't download dependencies before Make - - Fix: Add beforeBuild with appropriate ensure* calls - -3. Wrong dependency helper: - - Bug: Manually downloading curl/LIEF with curl/wget - - Fix: Use downloadSocketBtmRelease() or package-specific helpers - -4. Not using buildBinSuitePackage: - - Bug: Custom build script without standard patterns - - Fix: Use bin-infra/lib/builder for consistent behavior +2. Missing error handling in build scripts: + - Bug: Build script doesn't validate outputs + - Fix: Add validation after each build step **Check these files:** -- scripts/build.mjs - Build orchestration for overrides +- scripts/ - Build and maintenance scripts - packages/npm/*/package.json - Override package definitions -- README.md files - Should document `pnpm run build`, not direct make For each issue, consider: @@ -924,28 +720,7 @@ Zizmor is a GitHub Actions workflow security scanner that detects: This repository uses GitHub Actions for CI/CD with workflows in `.github/workflows/`. **Installation:** -Zizmor is not available via npm. Install zizmor v1.22.0 using one of these methods: - -**GitHub Releases (Recommended):** -```bash -# Download from https://github.com/zizmorcore/zizmor/releases/tag/v1.22.0 -# macOS ARM64: -curl -L https://github.com/zizmorcore/zizmor/releases/download/v1.22.0/zizmor-aarch64-apple-darwin -o /usr/local/bin/zizmor -chmod +x /usr/local/bin/zizmor - -# macOS x64: -curl -L https://github.com/zizmorcore/zizmor/releases/download/v1.22.0/zizmor-x86_64-apple-darwin -o /usr/local/bin/zizmor -chmod +x /usr/local/bin/zizmor - -# Linux x64: -curl -L https://github.com/zizmorcore/zizmor/releases/download/v1.22.0/zizmor-x86_64-unknown-linux-musl -o /usr/local/bin/zizmor -chmod +x /usr/local/bin/zizmor -``` - -**Alternative Methods:** -- Homebrew: `brew install zizmor@1.22.0` -- Cargo: `cargo install zizmor --version 1.22.0` -- See https://docs.zizmor.sh/installation/ for all options +Zizmor is not an npm package. See `_shared/security-tools.md` for detection and `external-tools.json` for the pinned version. Install via `pnpm run setup`. @@ -1055,163 +830,79 @@ Group findings by severity (Error → High → Medium → Low → Info) ## Workflow Optimization Scan Agent -**Mission**: Verify GitHub Actions workflows optimize CI time by checking `build-required` conditions on expensive installation/setup steps when checkpoint caching is used. +**Mission**: Verify GitHub Actions workflows optimize CI time by using proper caching, conditional steps, and avoiding redundant work. -**Scan Targets**: All `.github/workflows/*.yml` files that use checkpoint caching +**Scan Targets**: All `.github/workflows/*.yml` files **Prompt Template:** ``` -Your task is to verify GitHub Actions workflows properly skip expensive tool installation steps when builds are restored from cache by checking for `build-required` conditions. +Your task is to verify GitHub Actions workflows properly optimize CI time by using caching, conditional steps, and avoiding redundant installations. **Why Workflow Optimization Matters:** -CI workflows waste significant time installing build tools (compilers, CMake, toolchains) even when builds are restored from cache. For socket-btm with 9 workflows building Node.js binaries across multiple platforms, these optimizations save: -- **Windows**: 1-3 minutes per run (MinGW/LLVM/CMake installation) -- **macOS**: 30-60 seconds per run (brew installs, Xcode setup) -- **Linux**: 30-60 seconds per run (apt-get installs, toolchain setup) - -**socket-btm Checkpoint Caching System:** -socket-btm uses a sophisticated checkpoint caching system to speed up builds: -- `restore-checkpoint` action: Restores build artifacts from cache (yoga-layout, models, lief, onnxruntime, node-smol) -- `setup-checkpoints` action: Manages checkpoints for binsuite tools (binpress, binflate, binject) -- `build-required` output: Indicates if actual build is needed (false when cache restored) - -**Workflows Using Checkpoints:** -1. binsuite.yml - Uses `setup-checkpoints` (binpress, binflate, binject jobs) -2. node-smol.yml - Uses `restore-checkpoint` -3. lief.yml - Uses `restore-checkpoint` -4. onnxruntime.yml - Uses `restore-checkpoint` -5. yoga-layout.yml - Uses `restore-checkpoint` (Docker-only, no native steps) -6. models.yml - Uses `restore-checkpoint` (Docker-only, no native steps) -7. curl.yml - No checkpoint caching -8. stubs.yml - No checkpoint caching for native builds - -**Expected Pattern:** -Installation steps should check `build-required` before running: -```yaml -- name: Install CMake (Windows) - if: steps.setup-checkpoints.outputs.build-required == 'true' && matrix.os == 'windows' - run: choco install cmake -y -``` - -Or the full cache validation pattern: -```yaml -- name: Setup build toolchain (macOS) - if: | - matrix.os == 'macos' && - ((steps.lief-checkpoint-cache.outputs.cache-hit != 'true' || steps.validate-cache.outputs.valid == 'false') || - steps.restore-checkpoint.outputs.build-required == 'true') - run: brew install cmake ccache -``` +CI workflows waste significant time when they don't leverage caching or skip unnecessary steps. + +**socket-registry CI Structure:** +socket-registry provides reusable GitHub Actions workflows consumed by other Socket repos: +- Reusable workflows in `.github/workflows/` with `workflow_call` triggers +- Package build and test workflows +- Publishing workflows for npm packages + +**Expected Patterns:** +- Cache node_modules and pnpm store across runs +- Skip build steps when outputs are cached +- Use `if` conditions to skip unnecessary platform-specific steps -Systematically verify all workflows that use checkpoint caching properly optimize installation steps: +Systematically verify all workflows optimize CI time: -**Step 1: Identify workflows with checkpoint caching** +**Step 1: Identify workflows with caching** ```bash -grep -l "restore-checkpoint\|setup-checkpoints" .github/workflows/*.yml +ls .github/workflows/*.yml ``` **Step 2: For each workflow, check:** -1. Which checkpoint action is used (`restore-checkpoint` or `setup-checkpoints`) -2. Identify ALL installation/setup steps: - - Steps with names: "Install", "Setup", "Select Xcode" - - Steps running: `choco install`, `apt-get install`, `brew install`, `xcode-select` - - Steps downloading tools: llvm-mingw downloads, toolchain downloads - -**Step 3: Verify each installation step has correct condition:** -- For `setup-checkpoints`: `steps.setup-checkpoints.outputs.build-required == 'true'` -- For `restore-checkpoint`: `steps.restore-checkpoint.outputs.build-required == 'true'` -- OR full cache check: `(cache-hit != 'true' || valid == 'false') || build-required == 'true'` - -**Step 4: Exceptions (steps that should NOT check build-required):** -- pnpm/Node.js setup (needed to run build scripts) -- QEMU/Docker setup for testing (not build dependencies) -- Depot CLI setup (needed for Docker builds) -- Steps in workflows without checkpoint caching - - -Installation step runs unconditionally even when cache is restored: -```yaml -# BAD - no build-required check -- name: Install CMake (Windows) - if: matrix.os == 'windows' - run: choco install cmake -y - -# GOOD - checks build-required -- name: Install CMake (Windows) - if: steps.setup-checkpoints.outputs.build-required == 'true' && matrix.os == 'windows' - run: choco install cmake -y -``` +1. Is pnpm store cached? (`actions/cache` or `pnpm/action-setup` with cache) +2. Are build outputs cached when possible? +3. Are installation steps conditional on cache misses? -Look for steps that: -- Install compilers (gcc, clang, MinGW, llvm-mingw) -- Install build tools (cmake, ninja, make, ccache) -- Setup toolchains (musl-tools, cross-compilers) -- Select Xcode versions -- Download toolchain archives - +**Step 3: Verify optimization patterns:** - -Step references wrong checkpoint action output: + +Workflows that install dependencies without caching: ```yaml -# BAD - binsuite.yml uses setup-checkpoints, not restore-checkpoint -- name: Install tools - if: steps.restore-checkpoint.outputs.build-required == 'true' - -# GOOD - correct reference -- name: Install tools - if: steps.setup-checkpoints.outputs.build-required == 'true' +# BAD - no caching +- run: pnpm install + +# GOOD - with pnpm store cache +- uses: pnpm/action-setup@v4 +- uses: actions/setup-node@v4 + with: + cache: 'pnpm' +- run: pnpm install ``` - -socket-btm conventions: -- binsuite.yml (binpress/binflate/binject): Use `steps.setup-checkpoints.outputs.build-required` -- node-smol.yml, lief.yml, onnxruntime.yml: Use `steps.restore-checkpoint.outputs.build-required` - -Step checks some conditions but misses build-required: -```yaml -# BAD - checks platform but not build-required -- name: Setup toolchain (Windows ARM64) - if: matrix.os == 'windows' && matrix.cross_compile - run: | - curl -o llvm-mingw.zip https://... - 7z x llvm-mingw.zip - -# GOOD - includes build-required check -- name: Setup toolchain (Windows ARM64) - if: | - matrix.os == 'windows' && matrix.cross_compile && - ((steps.cache.outputs.cache-hit != 'true' || steps.validate.outputs.valid == 'false') || - steps.restore-checkpoint.outputs.build-required == 'true') -``` + +Steps that run unconditionally but could be skipped: +- Installation steps that re-run even when cached +- Build steps that re-run even when outputs exist +- Test setup that duplicates other workflow steps -**socket-btm-Specific Workflow Patterns:** - -1. **binsuite.yml** (binpress, binflate, binject jobs): - - Uses: `setup-checkpoints` action - - Steps to check: Compiler setup, dependency installation, toolchain setup, Xcode selection, CMake installation - -2. **node-smol.yml**: - - Uses: `restore-checkpoint` action - - Steps to check: musl toolchain, Xcode selection, Windows tools, compression dependencies - -3. **lief.yml**: - - Uses: `restore-checkpoint` action with full cache validation - - Steps to check: macOS toolchain (brew install cmake ccache), Windows toolchains - -4. **onnxruntime.yml**: - - Uses: `restore-checkpoint` action - - Steps to check: Build tools installation (ninja-build) + +Reusable workflow problems: +- Missing required inputs documentation +- Outputs not properly propagated to callers +- Secrets not passed through correctly + For each issue found: 1. Identify the workflow file and line number -2. Show the current condition -3. Explain why build-required check is missing or incorrect -4. Provide the corrected condition +2. Show the current configuration +3. Explain the optimization opportunity +4. Provide the corrected configuration 5. Estimate time savings from the fix @@ -1219,33 +910,31 @@ For each issue found: For each finding, report: File: .github/workflows/workflow-name.yml:line -Issue: Installation step missing build-required check +Issue: [One-line description] Severity: Medium -Impact: Wastes N seconds/minutes installing tools when cache is restored -Pattern: [current condition] -Fix: [corrected condition with build-required check] -Savings: Estimated ~N seconds per cached CI run +Impact: Wastes N seconds/minutes per CI run +Pattern: [current configuration] +Fix: [corrected configuration] +Savings: Estimated ~N seconds per CI run Example: -File: .github/workflows/lief.yml:310 -Issue: macOS toolchain setup missing build-required check +File: .github/workflows/ci.yml:45 +Issue: pnpm store not cached across runs Severity: Medium -Impact: Wastes 30-60 seconds installing cmake/ccache when LIEF is restored from cache -Pattern: `if: matrix.os == 'macos'` -Fix: `if: matrix.os == 'macos' && ((steps.lief-checkpoint-cache.outputs.cache-hit != 'true' || steps.validate-cache.outputs.valid == 'false') || steps.restore-checkpoint.outputs.build-required == 'true')` -Savings: ~45 seconds per cached macOS CI run +Impact: Wastes 30-60 seconds reinstalling dependencies on each run +Pattern: Missing `cache: 'pnpm'` in setup-node step +Fix: Add `cache: 'pnpm'` to actions/setup-node configuration +Savings: ~45 seconds per CI run -- Only report installation/setup steps that install tools needed for building -- Don't report steps needed for running build scripts (pnpm, Node.js) -- Verify the checkpoint action type before suggesting fix -- Calculate realistic time savings based on actual tool installation times +- Only report steps where caching or conditions would provide real savings +- Don't report steps that genuinely need to run every time - If all workflows are optimized, state that explicitly - Group findings by workflow file -Systematically analyze all workflows with checkpoints and report all missing optimizations. If workflows are fully optimized, state: "✓ All workflows properly optimize installation steps with build-required checks." +Systematically analyze all workflows and report all missing optimizations. If workflows are fully optimized, state: "All workflows properly optimize CI time." ``` --- @@ -1350,10 +1039,8 @@ Look for: Look for: - Build output paths that don't match actual outputs -- File sizes that are significantly outdated -- Checkpoint names that don't match actual implementation -- Binary names that are incorrect -- Missing intermediate build stages +- Package names that are incorrect +- Missing build steps in documentation @@ -1363,14 +1050,10 @@ Look for: - Tool version requirements that are incorrect - Patch counts that don't match actual patches -**CRITICAL: For third-party library versions (LIEF, Node.js, ONNX Runtime, etc.):** +**CRITICAL: For dependency versions:** - DO NOT blindly "correct" documented versions without verification -- For socket-btm specifically, versions must align with what Node.js upstream uses -- LIEF version: Documented version (v0.17.0) is correct - aligned with Node.js needs -- Node.js version: Check .node-version file (source of truth) -- ONNX Runtime, Yoga: Verify against package configurations +- Check package.json files as the source of truth for dependency versions - If unsure about a version, SKIP reporting it as incorrect - ask user to verify -- NEVER change version numbers based on git describe output from dependencies - When in doubt, assume documentation is correct unless you can definitively verify otherwise @@ -1386,8 +1069,8 @@ Look for: **CRITICAL: Evaluate documentation from a junior developer perspective** Check for junior-developer unfriendly patterns: -- Missing "Why" explanations (e.g., "Use binject to inject SEA" without explaining what SEA is) -- Assumed knowledge not documented (Node.js SEA, LIEF, VFS concepts) +- Missing "Why" explanations (e.g., "Use overrides to replace packages" without explaining what overrides are) +- Assumed knowledge not documented (npm overrides, pnpm workspaces) - No examples for common workflows (first-time setup, typical usage) - Missing troubleshooting sections - No explanation of error messages @@ -1405,11 +1088,10 @@ Check for junior-developer unfriendly patterns: 5. **Error message handling** - Should help debug, not confuse **Areas requiring extra scrutiny:** -- Binary manipulation concepts (SEA, VFS, section injection) -- Build system complexity (checkpoints, caching, cross-compilation) -- Patch management (upstream sync, patch regeneration) -- C/C++ integration points (LIEF, native code) -- Cross-platform differences (Linux musl/glibc, macOS universal binaries, Windows) +- npm override concepts (how overrides work, why they exist) +- Package registry architecture (packages/npm/ structure, registry/lib/) +- Build scripts and automation (scripts/ directory) +- CI/CD workflows (reusable workflows, publishing) For each junior-dev issue: - Identify the knowledge gap or assumption @@ -1418,9 +1100,9 @@ For each junior-dev issue: - Provide example of clear explanation Example findings: -- "README assumes knowledge of Node.js SEA without explaining it" -- "No explanation of what 'upstream sync' means or why it matters" -- "Technical term 'checkpoint caching' used without definition" +- "README assumes knowledge of npm overrides without explaining them" +- "No explanation of what 'override packages' means or why they exist" +- "Technical term 'pnpm workspaces' used without definition" - "Build errors not documented in troubleshooting section" diff --git a/.claude/skills/release/SKILL.md b/.claude/skills/release/SKILL.md new file mode 100644 index 00000000..119b2042 --- /dev/null +++ b/.claude/skills/release/SKILL.md @@ -0,0 +1,88 @@ +--- +name: release +description: Release pipeline orchestrator. Runs quality and security gates, generates changelog, bumps version, and offers to publish. +--- + +# Release + +Orchestrates a release by chaining quality-scan and security-scan as gates, then generating a changelog and version bump. + +## When to Use + +- Preparing a new release +- When `/release-changelog` command is invoked (delegates here) + +## Process + +### Phase 1: Quality Gate + +Run the `/quality-scan` skill with all scan types enabled. + +**Gate condition**: zero CRITICAL findings. If any CRITICAL findings exist, abort the release and report them. + +Read the HANDOFF block from quality-scan to check: +``` +Findings: {critical: N, ...} +``` + +If `critical > 0`: stop, report findings, do not proceed. + +Update queue: `current_phase: quality-gate` + +--- + +### Phase 2: Security Gate + +Run the `/security-scan` skill. + +**Gate condition**: grade B or above. If grade is C, D, or F, abort the release and report findings. + +Read the HANDOFF block from security-scan to check: +``` +Grade: {A-F} +``` + +If grade is C or worse: stop, report findings, do not proceed. + +Update queue: `current_phase: security-gate` + +--- + +### Phase 3: Changelog + +Generate changelog entry following CLAUDE.md § "Changelog Management": + +1. Read current `CHANGELOG.md` to get the last released version (if it doesn't exist, create it with the Keep a Changelog header) +2. If no tags exist, use all commits on main; otherwise run `git log --oneline ..HEAD` +3. Categorize: Added / Changed / Fixed / Removed +4. Skip chore/ci/deps commits unless they affect user-facing behavior +5. Write new entry at top of CHANGELOG.md +6. Present for review before committing + +Update queue: `current_phase: changelog` + +--- + +### Phase 4: Version Bump + +Determine version bump from commit types: +- `feat` commits → minor bump +- `fix` commits only → patch bump +- Breaking changes (noted in commits) → major bump + +Update `version` in `package.json`. + +Present the version change for approval. Commit changelog + version bump together: +``` +chore(release): X.Y.Z +``` + +Update queue: `status: done` + +--- + +### Post-Release (Manual) + +After the release commit is merged: +1. Tag: `git tag vX.Y.Z && git push origin vX.Y.Z` +2. The provenance workflow handles npm publishing diff --git a/.claude/skills/security-scan/SKILL.md b/.claude/skills/security-scan/SKILL.md new file mode 100644 index 00000000..0ba403fe --- /dev/null +++ b/.claude/skills/security-scan/SKILL.md @@ -0,0 +1,82 @@ +--- +name: security-scan +description: Run a multi-tool security scan — AgentShield for Claude config, zizmor for GitHub Actions, and optionally Socket CLI for dependency scanning. Produces an A-F graded security report. +--- + +# Security Scan + +Multi-tool security scanning pipeline for the repository. + +## When to Use + +- After modifying `.claude/` config, settings, hooks, or agent definitions +- After modifying GitHub Actions workflows +- Before releases (called as a gate by the release pipeline) +- Periodic security hygiene checks + +## Prerequisites + +See `_shared/security-tools.md` for tool detection and installation. + +## Process + +### Phase 1: Environment Check + +Follow `_shared/env-check.md`. Initialize a queue run entry for `security-scan`. + +--- + +### Phase 2: AgentShield Scan + +Scan Claude Code configuration for security issues: + +```bash +pnpm exec agentshield scan +``` + +Checks `.claude/` for: +- Hardcoded secrets in CLAUDE.md and settings +- Overly permissive tool allow lists (e.g. `Bash(*)`) +- Prompt injection patterns in agent definitions +- Command injection risks in hooks +- Risky MCP server configurations + +Capture the grade and findings count. + +Update queue: `current_phase: agentshield` → `completed_phases: [env-check, agentshield]` + +--- + +### Phase 3: Zizmor Scan + +Scan GitHub Actions workflows for security issues. + +See `_shared/security-tools.md` for zizmor detection. If not installed, skip with a warning. + +```bash +zizmor .github/ +``` + +Checks for: +- Unpinned actions (must use full SHA, not tags) +- Secrets used outside `env:` blocks +- Injection risks from untrusted inputs (template injection) +- Overly permissive permissions + +Capture findings. Update queue phase. + +--- + +### Phase 4: Grade + Report + +Spawn the `security-reviewer` agent (see `agents/security-reviewer.md`) with the combined output from AgentShield and zizmor. + +The agent: +1. Applies CLAUDE.md security rules to evaluate the findings +2. Calculates an A-F grade per `_shared/report-format.md` +3. Generates a prioritized report (CRITICAL first) +4. Suggests fixes for HIGH and CRITICAL findings + +Output a HANDOFF block per `_shared/report-format.md` for pipeline chaining. + +Update queue: `status: done`, write `findings_count` and final grade. diff --git a/.claude/skills/updating/SKILL.md b/.claude/skills/updating/SKILL.md index f97d5548..adc8b446 100644 --- a/.claude/skills/updating/SKILL.md +++ b/.claude/skills/updating/SKILL.md @@ -43,33 +43,16 @@ This skill updates npm packages for security patches, bug fixes, and new feature ### Phase 1: Validate Environment - -Check working directory is clean and detect CI mode: - +Update queue: advance `current_phase` in `.claude/ops/queue.yaml` -```bash -# Detect CI mode -if [ "$CI" = "true" ] || [ -n "$GITHUB_ACTIONS" ]; then - CI_MODE=true - echo "Running in CI mode - will skip build validation" -else - CI_MODE=false - echo "Running in interactive mode - will validate builds" -fi - -# Check working directory is clean -git status --porcelain -``` - - -- Working directory must be clean -- CI_MODE detected for subsequent phases - +Follow `_shared/env-check.md` to validate the environment and initialize a queue run entry for `updating`. --- ### Phase 2: Update npm Packages +Update queue: advance `current_phase` in `.claude/ops/queue.yaml` + Run pnpm run update to update npm dependencies: @@ -79,8 +62,8 @@ Run pnpm run update to update npm dependencies: pnpm run update # Check if there are changes -if [ -n "$(git status --porcelain pnpm-lock.yaml package.json)" ]; then - git add pnpm-lock.yaml package.json +if [ -n "$(git status --porcelain)" ]; then + git add pnpm-lock.yaml pnpm-workspace.yaml package.json packages/npm/*/package.json git commit -m "chore: update npm dependencies Updated npm packages via pnpm run update." @@ -94,26 +77,16 @@ fi ### Phase 3: Final Validation - -Run build and test suite (skip in CI mode): - +Update queue: advance `current_phase` in `.claude/ops/queue.yaml` -```bash -if [ "$CI_MODE" = "true" ]; then - echo "CI mode: Skipping final validation (CI will run builds/tests separately)" - echo "Commits created - ready for push by CI workflow" -else - echo "Interactive mode: Running full validation..." - pnpm run fix --all - pnpm run check --all - pnpm test -fi -``` +Follow `_shared/verify-build.md` for build validation. --- ### Phase 4: Report Summary +Update queue: advance `current_phase` in `.claude/ops/queue.yaml` + Generate update report: @@ -145,6 +118,20 @@ Generate update report: 3. Review PR when CI passes ``` +### Completion + +Output a HANDOFF block per `_shared/report-format.md`: + +``` +=== HANDOFF: updating === +Status: {pass|fail} +Findings: {packages_updated: N} +Summary: {one-line description of what was updated} +=== END HANDOFF === +``` + +Update queue: `status: done`, write completion timestamp. + ## Success Criteria diff --git a/.git-hooks/commit-msg b/.git-hooks/commit-msg index b85d1f50..1210859a 100755 --- a/.git-hooks/commit-msg +++ b/.git-hooks/commit-msg @@ -47,7 +47,7 @@ if [ -f "$COMMIT_MSG_FILE" ]; then # Read the commit message line by line and filter out AI attribution. while IFS= read -r line || [ -n "$line" ]; do # Check if this line contains AI attribution patterns. - if echo "$line" | grep -qiE "(Generated with|Co-Authored-By: Claude|Co-Authored-By: AI|🤖 Generated|AI generated|Claude Code|@anthropic|Assistant:|Generated by Claude|Machine generated)"; then + if echo "$line" | grep -qiE "(Generated with|Co-Authored-By: Claude|Co-Authored-By: AI|🤖 Generated|AI generated|Claude Code|@anthropic\.com|Assistant:|Generated by Claude|Machine generated)" && ! echo "$line" | grep -qE "@anthropic-ai/"; then REMOVED_LINES=$((REMOVED_LINES + 1)) else # Line doesn't contain AI attribution, keep it. diff --git a/.git-hooks/pre-commit b/.git-hooks/pre-commit index 28c0da69..7ae6f541 100755 --- a/.git-hooks/pre-commit +++ b/.git-hooks/pre-commit @@ -112,6 +112,24 @@ for file in $STAGED_FILES; do fi done +# Check for npx/dlx usage (use pnpm exec or pnpm run instead). +printf "Checking for npx/dlx usage...\n" +for file in $STAGED_FILES; do + if [ -f "$file" ]; then + # Skip node_modules, lockfiles, and this hook itself. + if echo "$file" | grep -qE 'node_modules/|pnpm-lock\.yaml|\.git-hooks/'; then + continue + fi + + if grep -nE '\bnpx\b|\bpnpm dlx\b|\byarn dlx\b' "$file" 2>/dev/null | grep -v '# zizmor:' | grep -q .; then + printf "${RED}✗ ERROR: npx/dlx usage found in: $file${NC}\n" + grep -nE '\bnpx\b|\bpnpm dlx\b|\byarn dlx\b' "$file" | grep -v '# zizmor:' | head -3 + printf "Use 'pnpm exec ' or 'pnpm run