Skip to content

Commit 76f47bf

Browse files
committed
fix: address audit findings in agents, skills, and commands
- security-reviewer: remove fabricated os.tmpdir() prohibition (CLAUDE.md recommends it), add fetch() prohibition from CLAUDE.md - code-reviewer: add missing rules (undefined over null, __proto__: null, error handling, backward compat, spawn, loop annotations) - ci-cascade: add missing Layer 4 (local wrappers) before external propagation - quality-scan: fix "4 scan types" → "all scan types", fix "Task tool" → "Agent tool" - quality-loop: remove stale architectural issue from wrong repo (socket-btm) - Delete stale scratch scripts from .claude/ (migration scripts, update-workflow-shas)
1 parent bb9f582 commit 76f47bf

File tree

5 files changed

+24
-46
lines changed

5 files changed

+24
-46
lines changed

.claude/agents/code-reviewer.md

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,21 @@
11
You are a code reviewer for a Node.js/TypeScript monorepo (socket-registry).
22

3-
Apply these rules from CLAUDE.md exactly:
3+
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.
44

5-
**Code Style - File Organization**: kebab-case filenames, @fileoverview headers, node: prefix imports, import sorting order (node → external → @socketsecurity → local → types).
5+
**Code Style - File Organization**: kebab-case filenames, @fileoverview headers, node: prefix imports, import sorting order (node → external → @socketsecurity → local → types), fs import pattern.
66

7-
**Code Style - Patterns**: UPPER_SNAKE_CASE constants, undefined over null, __proto__: null first in literals, { 0: key, 1: val } for entries loops, !array.length not === 0, += 1 not ++, template literals not concatenation, no semicolons, no any types.
7+
**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.
88

9-
**Code Style - Functions**: Alphabetical order (private first, exported second), shell: WIN32 not shell: true, never process.chdir().
9+
**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.
1010

1111
**Code Style - Comments**: Default NO comments. Only when WHY is non-obvious. End with periods. Single-line only. JSDoc: description + @throws only.
1212

1313
**Code Style - Sorting**: All lists, exports, properties, destructuring alphabetical. Type properties: required first, optional second.
1414

15+
**Error Handling**: catch (e) not catch (error), double-quoted error messages, { cause: e } chaining.
16+
17+
**Backward Compatibility**: FORBIDDEN — actively remove compat shims, don't maintain them.
18+
1519
**Test Style**: Functional tests over source scanning. Never read source files and assert on contents. Verify behavior with real function calls.
1620

1721
For each file reviewed, report:

.claude/agents/security-reviewer.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ You are a security reviewer for Socket Security Node.js repositories.
22

33
Apply these rules from CLAUDE.md exactly:
44

5-
**Safe File Operations**: Use safeDelete()/safeDeleteSync() from @socketsecurity/lib/fs. NEVER fs.rm(), fs.rmSync(), or rm -rf. NEVER use os.tmpdir() — use tempDir from @socketsecurity/lib/env/temp-dir.
5+
**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.
66

77
**Absolute Rules**: NEVER use npx, pnpm dlx, or yarn dlx. Use pnpm exec or pnpm run with pinned devDeps.
88

@@ -13,7 +13,7 @@ Apply these rules from CLAUDE.md exactly:
1313
1. **Secrets**: Hardcoded API keys, passwords, tokens, private keys in code or config
1414
2. **Injection**: Command injection via shell: true or string interpolation in spawn/exec. Path traversal in file operations.
1515
3. **Dependencies**: npx/dlx usage. Unpinned versions (^ or ~). Missing minimumReleaseAge bypass justification.
16-
4. **File operations**: fs.rm without safeDelete. os.tmpdir usage. process.chdir usage.
16+
4. **File operations**: fs.rm without safeDelete. process.chdir usage. fetch() usage (must use lib's httpRequest).
1717
5. **GitHub Actions**: Unpinned action versions (must use full SHA). Secrets outside env blocks. Template injection from untrusted inputs.
1818
6. **Error handling**: Sensitive data in error messages. Stack traces exposed to users.
1919

.claude/commands/quality-loop.md

Lines changed: 0 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -16,31 +16,3 @@ Run the quality-scan skill and fix all issues found. Repeat until zero issues re
1616
- Do not skip architectural fixes
1717
- Run tests after fixes to verify nothing broke
1818
- Track iteration count and report progress
19-
20-
## Outstanding Architectural Issue (Requires Design Review)
21-
22-
### Checkpoint Cache Invalidation (High Severity)
23-
24-
**Issue**: Source package changes don't invalidate checkpoints properly.
25-
26-
**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.
27-
28-
**Scenario**:
29-
1. Developer modifies `packages/binject/src/socketsecurity/binject/file.c`
30-
2. Runs `pnpm --filter node-smol-builder clean && pnpm build`
31-
3. `prepareExternalSources()` syncs binject → additions/source-patched/src/socketsecurity/binject/
32-
4. Cache key computed from additions/ files matches old checkpoint
33-
5. Build restores stale checkpoint, skips recompilation
34-
6. **Result**: Binary contains old binject code
35-
36-
**Impact**: Silent build incorrectness when modifying source packages
37-
38-
**Proposed Solutions** (require architectural review):
39-
- Option 1: Include source package mtimes in cache key metadata
40-
- Option 2: Make `prepareExternalSources()` idempotent, always re-sync
41-
42-
**Files Affected**:
43-
- packages/node-smol-builder/scripts/common/shared/build.mjs (collectBuildSourceFiles)
44-
- packages/node-smol-builder/scripts/common/shared/checkpoints.mjs (cache key generation)
45-
46-
**Status**: Documented for architectural review and future implementation

.claude/skills/ci-cascade/SKILL.md

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -39,20 +39,22 @@ Starting from the layer above the change, create a PR for each layer:
3939
7. **Push and create PR**
4040
8. **Wait for merge** before proceeding to next layer
4141

42-
### Phase 3: Propagate to external repos
42+
### Phase 3: Update Layer 4 (local wrappers)
4343

44-
The **propagation SHA** is the Layer 3 merge SHA (where ci.yml and provenance.yml were updated). All external repos pin to this same SHA.
44+
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.
4545

46-
External repos (update all):
47-
- socket-btm, socket-cli, socket-sdk-js, socket-packageurl-js
48-
- socket-sbom-generator, socket-lib, ultrathink
46+
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.
47+
48+
### Phase 4: Propagate to external repos
49+
50+
All external repos pin to the **propagation SHA** (Layer 3 merge SHA).
4951

5052
For each repo:
5153
1. Update all `SocketDev/socket-registry/.github/` refs to the propagation SHA
52-
2. Push directly to main where allowed
53-
3. Create PRs where branch protection requires it
54+
2. Push directly to main where allowed (socket-btm, socket-sbom-generator, ultrathink)
55+
3. Create PRs where branch protection requires it (socket-cli, socket-lib, socket-sdk-js, socket-packageurl-js)
5456

55-
### Phase 4: Verify
57+
### Phase 5: Verify
5658

5759
For each updated repo, confirm no old SHAs remain:
5860
```bash

.claude/skills/quality-scan/SKILL.md

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -312,7 +312,7 @@ Use AskUserQuestion tool:
312312
- Header: "Scan Types"
313313
- multiSelect: true
314314
- Options:
315-
- "All scans (recommended)" → Run all 4 scan types
315+
- "All scans (recommended)" → Run all scan types
316316
- "Critical only" → Run critical scan only
317317
- "Critical + Logic" → Run critical and logic scans
318318
- "Custom selection" → Ask user to specify which scans
@@ -336,7 +336,7 @@ If user requests non-existent scan type, report error and suggest valid types.
336336
### Phase 6: Execute Scans
337337

338338
<action>
339-
For each enabled scan type, spawn a specialized agent using Task tool:
339+
For each enabled scan type, spawn a specialized agent using Agent tool:
340340
</action>
341341

342342
```typescript
@@ -365,7 +365,7 @@ Scan systematically and report all findings. If no issues found, state that expl
365365
**For each scan:**
366366
1. Load agent prompt template from `reference.md`
367367
2. Customize for repository context (determine monorepo vs single package structure)
368-
3. Spawn agent with Task tool using "general-purpose" subagent_type
368+
3. Spawn agent with Agent tool using "general-purpose" subagent_type
369369
4. Capture findings from agent response
370370
5. Parse and categorize results
371371

@@ -617,7 +617,7 @@ This skill is self-contained. No external commands needed.
617617

618618
This skill provides systematic code quality analysis by:
619619
- Spawning specialized agents for targeted analysis
620-
- Using Task tool to run agents autonomously
620+
- Using Agent tool to run agents autonomously
621621
- Embedding agent prompts in reference.md following best practices
622622
- Generating prioritized, actionable reports
623623
- Supporting partial scans (user can select specific scan types)

0 commit comments

Comments
 (0)