Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions .claude/agents/security-reviewer.md
Original file line number Diff line number Diff line change
@@ -1,18 +1,24 @@
---
name: security-reviewer
description: Reviews findings from AgentShield + zizmor against the project's CLAUDE.md security rules and grades the result A-F. Spawned by the security-scan skill after the static scans run.
tools: Read, Grep, Glob, Bash(git:*), Bash(rg:*), Bash(grep:*), Bash(find:*), Bash(ls:*), Bash(pnpm exec agentshield:*), Bash(zizmor:*), Bash(command -v:*), Bash(cat:*), Bash(head:*), Bash(tail:*)
---

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.
**Absolute Rules**: NEVER use npx, pnpm dlx, or yarn dlx. Use pnpm exec or pnpm run with pinned devDeps. # zizmor: documentation-prohibition

**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.
3. **Dependencies**: npx/dlx usage. Unpinned versions (^ or ~). Missing soak-window bypass justification (pnpm-workspace.yaml `minimumReleaseAgeExclude`). # zizmor: documentation-checklist
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.
Expand Down
39 changes: 39 additions & 0 deletions .claude/skills/_shared/path-guard-rule.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
<!--
Shared snippet — the canonical "1 path, 1 reference" rule text.
Synced byte-identical across the Socket fleet via socket-repo-template's
sync-scaffolding.mts (SHARED_SKILL_FILES).

This file is the source of truth for the rule's wording. Three artifacts
embed (or paraphrase) it:

1. CLAUDE.md — every Socket repo's instructions to Claude.
2. .claude/hooks/path-guard/README.md — what the hook blocks.
3. .claude/skills/path-guard/SKILL.md — what the skill enforces.

If the wording changes here, re-run `node scripts/sync-scaffolding.mts
--all --fix` from socket-repo-template to propagate.
-->

## 1 path, 1 reference

**A path is *constructed* exactly once. Everywhere else *references* the constructed value.**

Referencing a single computed path many times is fine — that's the whole point of computing it once. What's banned is *re-constructing* the same path in multiple places, because that's where drift is born. Three concrete shapes:

1. **Within a package** — every script, test, and lib file that needs a build path imports it from the package's `scripts/paths.mts` (or `lib/paths.mts`). No `path.join('build', mode, ...)` outside that module.

2. **Across packages** — when package B consumes package A's output, B imports A's `paths.mts` via the workspace `exports` field. Never `path.join(PKG, '..', '<sibling>', 'build', ...)`. The R28 yoga/ink bug — ink hand-building yoga's wasm path and missing the `wasm/` segment — is the canonical failure mode this rule prevents.

3. **Workflows, Dockerfiles, shell scripts** — they can't `import` TS, so they construct the string once and reference it everywhere downstream. Workflows: a "Compute paths" step exposes `steps.paths.outputs.final_dir`; later steps read `${{ steps.paths.outputs.final_dir }}`. Dockerfiles/shell: assign once to a variable, reference by name thereafter. Each canonical construction carries a comment naming the source-of-truth `paths.mts` so the YAML can't drift from TS without a flagged change. **Re-building** the same path in a second step is the violation, not referring to the constructed value many times.

Comments that re-state a full path are forbidden. The import statement IS the comment. Docs and READMEs may describe the structure ("output goes under the Final dir") but should not encode a complete `build/<mode>/<platform-arch>/out/Final/binary` string — encoded paths get parsed by tools and silently rot.

Code execution takes priority over docs: violations in `.mts`/`.cts`, Makefiles, Dockerfiles, workflow YAML, and shell scripts are blocking. README and doc-comment violations are advisory unless they contain a fully-qualified path with no parametric placeholders.

### Three-level enforcement

- **Hook** — `.claude/hooks/path-guard/` blocks `Edit`/`Write` calls that would introduce a violation in a `.mts`/`.cts` file. Refusal at edit time stops new duplication from landing.
- **Gate** — `scripts/check-paths.mts` runs in `pnpm check`. Fails the build on any violation that isn't allowlisted.
- **Skill** — `/path-guard` audits the repo and fixes findings; `/path-guard check` reports only; `/path-guard install` drops the gate + hook + rule into a fresh repo.

The mantra is intentionally short so it sticks: **1 path, 1 reference**. When in doubt, find the canonical owner and import from it.
250 changes: 250 additions & 0 deletions .claude/skills/path-guard/SKILL.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,250 @@
---
name: path-guard
description: Audit and fix path duplication in this Socket repo. Apply the strict "1 path, 1 reference" rule — every build/test/runtime/config path is constructed exactly once; everywhere else references the constructed value. Default mode finds and fixes; `check` mode reports only; `install` mode drops the gate + hook + rule into a fresh repo.
user-invocable: true
allowed-tools: Task, Read, Edit, Write, Grep, Glob, AskUserQuestion, Bash(pnpm run check:*), Bash(node scripts/check-paths:*), Bash(rg:*), Bash(grep:*), Bash(find:*), Bash(git:*)
---

# path-guard

**Mantra: 1 path, 1 reference.** A path is constructed exactly once; everywhere else references the constructed value. Re-constructing the same path twice is the violation, not referencing the constructed value many times.

## Modes

- `/path-guard` — full audit-and-fix conversion of the current repo (default).
- `/path-guard check` — read-only audit, report violations, no fixes.
- `/path-guard fix <id>` — fix a single finding from a prior `check` run, by index.
- `/path-guard install` — drop the gate + hook + rule + allowlist into a fresh repo (for new Socket repos).

## Three-level enforcement

The strategy lives in three artifacts that ship together:

1. **CLAUDE.md rule** — the mantra and detection rules in plain language. Every Socket repo's CLAUDE.md carries `## 1 path, 1 reference`. Synced from `.claude/skills/_shared/path-guard-rule.md`.
2. **Hook** — `.claude/hooks/path-guard/index.mts` runs `PreToolUse` on `Edit`/`Write` of `.mts`/`.cts` files. Blocks new violations at edit time. Mandatory across the fleet.
3. **Gate** — `scripts/check-paths.mts` runs in `pnpm check` (and CI). Whole-repo scan. Fails the build on any unsanctioned violation.

The hook and gate share their stage / build-root / mode / sibling-package vocabulary via `.claude/hooks/path-guard/segments.mts` — a single canonical source. Adding a new stage segment or fleet package means editing one file; the two consumers can never drift on what counts as a build-output path.

This skill is the *audit-and-fix workflow* that makes a repo conform initially and validates conformance over time.

## Detection rules

The gate enforces six rules. The hook enforces a subset (A and B) since it sees only one diff at a time.

| Rule | What it catches | Where checked |
|---|---|---|
| **A** | Multi-stage `path.join(...)` constructed inline. Two or more "stage" segments (Final, Release, Stripped, Compressed, Optimized, Synced, wasm, downloaded), or one stage + build-root + mode. | `.mts`/`.cts` files outside a `paths.mts`. Hook + gate. |
| **B** | Cross-package traversal: `path.join(*, '..', '<sibling-package>', 'build', ...)` reaching into a sibling's output instead of importing via `exports`. | `.mts`/`.cts` files. Hook + gate. |
| **C** | Workflow YAML constructs the same path string in 2+ steps outside a "Compute paths" step. | `.github/workflows/*.yml`. Gate. |
| **D** | Comment encodes a fully-qualified multi-stage path string (e.g. `# build/dev/darwin-arm64/out/Final/binary`). | `.github/workflows/*.yml`. Gate. |
| **F** | Same path shape constructed in 2+ different files. | All scanned files. Gate. |
| **G** | Hand-built multi-stage path constructed 2+ times in the same Makefile/Dockerfile/shell stage. | `Makefile`, `*.mk`, `*.Dockerfile`, `Dockerfile.*`, `*.sh`. Gate. |

Comments may describe path *structure* with placeholders (`<mode>/<arch>` or `${BUILD_MODE}/${PLATFORM_ARCH}`) but should not encode a complete literal path string. Code execution takes priority over docs: violations in `.mts`, Makefiles, Dockerfiles, workflow YAML, shell scripts are blocking.

## Mode: audit-and-fix (default)

When invoked as `/path-guard` with no arg:

1. **Setup** — spawn a worktree off `main` per `CLAUDE.md` parallel-sessions rule:
```bash
git worktree add -b paths-audit ../<repo>-paths-audit main
cd ../<repo>-paths-audit
```

2. **Audit** — run the gate to enumerate findings:
```bash
pnpm run check:paths --json > /tmp/paths-findings.json
pnpm run check:paths --explain # human-readable
```

3. **Fix loop** — for each finding, apply the matching pattern below. After each fix, re-run the gate. Stop iterating when `pnpm run check:paths` exits 0.

4. **Verify** — run the full check suite + zizmor on any modified workflow:
```bash
pnpm check
for w in .github/workflows/*.yml; do zizmor "$w"; done
```

5. **Commit and push** — group fixes by logical category (workflows, code, Dockerfiles). Push directly to `main` for repos that allow direct push, or open a PR for repos that require it (socket-cli, socket-sdk-js, socket-registry per their CLAUDE.md / memory entries).

## Fix patterns

### Rule A — Multi-stage path constructed inline (in `.mts`/`.cts`)

**Bad**:
```ts
const finalBinary = path.join(PACKAGE_ROOT, 'build', BUILD_MODE, PLATFORM_ARCH, 'out', 'Final', 'binary')
```

**Fix**: move the construction into the package's `scripts/paths.mts` (or `lib/paths.mts`), or use a build-infra helper:
```ts
// In packages/foo/scripts/paths.mts:
export function getBuildPaths(mode, platformArch) {
// ... constructs once ...
return { outputFinalBinary: path.join(PACKAGE_ROOT, 'build', mode, platformArch, 'out', 'Final', binaryName) }
}

// In the consumer:
import { getBuildPaths } from './paths.mts'
const { outputFinalBinary } = getBuildPaths(mode, platformArch)
```

For binsuite tools (binpress/binflate/binject) the canonical helper is `getFinalBinaryPath(packageRoot, mode, platformArch, binaryName)` from `build-infra/lib/paths`. For download caches use `getDownloadedDir(packageRoot)`.

### Rule B — Cross-package traversal

**Bad**:
```ts
const liefDir = path.join(PACKAGE_ROOT, '..', 'lief-builder', 'build', mode, platformArch, 'out', 'Final', 'lief')
```

**Fix**: declare the workspace dep, expose `paths.mts` via the producer's `exports`, import the helper:

1. In producer's `package.json`:
```json
"exports": {
"./scripts/paths": "./scripts/paths.mts"
}
```
2. In consumer's `package.json` `dependencies`:
```json
"lief-builder": "workspace:*"
```
3. In consumer:
```ts
import { getBuildPaths as getLiefBuildPaths } from 'lief-builder/scripts/paths'
const { outputFinalDir } = getLiefBuildPaths(mode, platformArch)
```

### Rule C — Workflow path repetition

**Bad** (3 steps each rebuilding the same path):
```yaml
- name: Step A
run: cd packages/foo/build/${BUILD_MODE}/${PLATFORM_ARCH}/out/Final && do-thing-1
- name: Step B
run: cd packages/foo/build/${BUILD_MODE}/${PLATFORM_ARCH}/out/Final && do-thing-2
- name: Step C
run: cd packages/foo/build/${BUILD_MODE}/${PLATFORM_ARCH}/out/Final && do-thing-3
```

**Fix**: add a "Compute <pkg> paths" step early in the job that constructs the path once, expose via `$GITHUB_OUTPUT`, reference downstream:

```yaml
- name: Compute foo paths
id: paths
env:
BUILD_MODE: ${{ steps.build-mode.outputs.mode }}
PLATFORM_ARCH: ${{ steps.platform-arch.outputs.platform_arch }}
run: |
PACKAGE_DIR="packages/foo"
PLATFORM_BUILD_DIR="${PACKAGE_DIR}/build/${BUILD_MODE}/${PLATFORM_ARCH}"
FINAL_DIR="${PLATFORM_BUILD_DIR}/out/Final"
{
echo "package_dir=${PACKAGE_DIR}"
echo "platform_build_dir=${PLATFORM_BUILD_DIR}"
echo "final_dir=${FINAL_DIR}"
} >> "$GITHUB_OUTPUT"

- name: Step A
env:
FINAL_DIR: ${{ steps.paths.outputs.final_dir }}
run: cd "$FINAL_DIR" && do-thing-1
# ... etc
```

For paths used inside `working-directory: packages/foo` steps, expose a `_rel` companion (e.g. `final_dir_rel=build/${BUILD_MODE}/${PLATFORM_ARCH}/out/Final`) and reference that.

### Rule D — Comment-encoded paths

**Bad**:
```yaml
# Path: packages/foo/build/dev/darwin-arm64/out/Final/binary
COPY --from=builder /build/.../out/Final/binary /out/Final/binary
```

**Fix**: cite the canonical `paths.mts` instead of duplicating the string:
```yaml
# Layout owned by packages/foo/scripts/paths.mts:getBuildPaths().
COPY --from=builder /build/packages/foo/build/${BUILD_MODE}/${PLATFORM_ARCH}/out/Final/binary /out/Final/binary
```

The comment may describe structure (`<mode>/<arch>`) but should not be a parsable literal path.

### Rule G — Dockerfile/Makefile/shell duplicate construction

**Bad** (Dockerfile reconstructs the path 3 times in the same stage):
```dockerfile
RUN mkdir -p build/${BUILD_MODE}/${PLATFORM_ARCH}/out/Final && \
cp src build/${BUILD_MODE}/${PLATFORM_ARCH}/out/Final/output && \
ls build/${BUILD_MODE}/${PLATFORM_ARCH}/out/Final/
```

**Fix**: declare an `ENV` once, reference everywhere:
```dockerfile
# Layout owned by packages/foo/scripts/paths.mts.
ENV FINAL_DIR=build/${BUILD_MODE}/${PLATFORM_ARCH}/out/Final
RUN mkdir -p "$FINAL_DIR" && cp src "$FINAL_DIR/output" && ls "$FINAL_DIR/"
```

Each Dockerfile `FROM` stage is its own scope — ENV from the build stage doesn't reach a subsequent `FROM scratch AS export` stage. The gate accounts for this.

## Mode: check (read-only)

When invoked as `/path-guard check`:

```bash
pnpm run check:paths --explain
```

Print the gate's findings without making any edits. Exit 0 if clean, 1 if findings present. Useful for CI / pre-merge inspection.

## Allowlisting a finding

When a genuine exemption is needed (rare — most "false positives" should be reported as gate bugs), add an entry to `.github/paths-allowlist.yml`. Two ways to pin the entry to a specific site:

- **`line:`** — exact line number. Strict; a single-line edit above shifts the entry off-target and the finding re-surfaces.
- **`snippet_hash:`** — 12-char SHA-256 prefix of the offending snippet (whitespace-normalized). Drift-resistant: survives reformatting, but any content-changing edit invalidates it. Get the hash:
```bash
pnpm run check:paths --show-hashes
```

Both may be set — either matching is sufficient. Prefer `snippet_hash` over raw `line:` when the exemption is expected to outlive routine reformatting; prefer `line:` when you specifically *want* the entry to fall off after any nearby edit.

## Mode: install (new repo)

When invoked as `/path-guard install` on a Socket repo that doesn't yet have the gate:

1. Copy the gate file from this skill's reference dir:
```bash
cp .claude/skills/path-guard/reference/check-paths.mts.tmpl scripts/check-paths.mts
```
2. Copy the empty allowlist:
```bash
cp .claude/skills/path-guard/reference/paths-allowlist.yml.tmpl .github/paths-allowlist.yml
```
3. Add `"check:paths": "node scripts/check-paths.mts"` to `package.json`.
4. Wire `runPathHygieneCheck()` into `scripts/check.mts` (after the existing checks).
5. Append the rule snippet from `.claude/skills/_shared/path-guard-rule.md` to the repo's `CLAUDE.md` if a `1 path, 1 reference` section is missing.
6. Add the hook entry to `.claude/settings.json` `PreToolUse` matcher `Edit|Write`:
```json
{ "type": "command", "command": "node .claude/hooks/path-guard/index.mts" }
```
7. Run the gate against the repo. Triage findings as you would in audit-and-fix mode.

## Tie-in with quality-scan

The `/quality-scan` skill should call `pnpm run check:paths --json` as one of its sub-scans and surface findings as part of its A-F graded report. Failures roll into the overall quality grade. The full audit-and-fix workflow lives here; quality-scan just *detects* during periodic scans.

## Reference patterns

When converting a repo to the strategy, the patterns I keep reusing:

- **TS-first packages**: each package owns a `scripts/paths.mts` with `PACKAGE_ROOT`, `BUILD_ROOT`, `getBuildPaths(mode, platformArch)` returning at minimum `outputFinalDir` and `outputFinalBinary`/`outputFinalFile`.
- **Cross-package consumers**: `package.json` `exports` whitelists `./scripts/paths`. Consumer adds `"<producer>: workspace:*"` and imports.
- **Workflows**: each job has a "Compute <pkg> paths" step (`id: paths`) early in the job. Step outputs include `package_dir`, `platform_build_dir`, `final_dir`, named files. `_rel` companions when `working-directory:` is used.
- **Docker stages**: each `FROM` stage declares `ENV PLATFORM_BUILD_DIR=...` and `ENV FINAL_DIR=...` once. Subsequent RUN steps reference the variables.

The first repo (socket-btm) is the worked example. Read its `scripts/paths.mts` files and `.github/workflows/*.yml` for canonical patterns when applying the strategy elsewhere.
Loading