audit-rules

name	audit-rules
description	Architectural compliance and code quality rules for Node.js + TypeScript projects (Vue 3 frontend, Express + Socket.IO backend). Provides systematic review criteria for security, code organization, error handling, and project-specific conventions. Use when auditing code quality.

Architectural Audit Rules

This skill provides systematic review criteria for auditing TypeScript codebases against architectural best practices and the harness-visualizer project's conventions (see AGENTS.md).

Use this skill when reviewing changes to:

• Express handlers and Socket.IO event handlers in backend/src/
• Vue 3 components, Pinia stores, and composables in frontend/src/
• Shared zod schemas, types, and event maps in shared/src/

Audit Categories

1. Security Patterns (Project-Critical)

These map directly to the non-negotiable security rules in AGENTS.md. Any violation here is a release blocker.

Bind to 127.0.0.1 only

Rule: The HTTP/Socket.IO server must bind to 127.0.0.1, never 0.0.0.0 or ::.

// BAD: exposes the dev tool to the network
httpServer.listen(port, '0.0.0.0');
httpServer.listen(port); // default may bind to all interfaces

// GOOD
httpServer.listen(port, '127.0.0.1');

Severity: CRITICAL

File IO must go through safeOpen

Rule: All filesystem reads/writes go through backend/src/api/safe-open.ts — a TOCTOU-safe primitive that does realpath → lstat → open(O_NOFOLLOW) → fstat → (ino, dev) compare.

// BAD: direct fs access bypasses the safety primitive
await fs.readFile(userProvidedPath, 'utf8');
await fs.writeFile(somePath, content);

// GOOD: use safeOpen
const handle = await safeOpen(userProvidedPath, watchRoots);
const content = await handle.readFile('utf8');

Severity: CRITICAL

Path-traversal & symlink rejection

Rule: Every read/write goes through path.resolve → fs.realpath → watch-root allowlist check. Symlinks resolving outside any watch root are rejected.

Checklist:

• User-provided paths resolved via path.resolve
• fs.realpath called before allowlist check
• Allowlist check uses Vite-style isParentDirectory (no string startsWith)
• Symlink follow happens once via realpath; subsequent swaps rejected

Severity: CRITICAL

Extension allowlist & size cap

Rule: Editable files limited to .md, .markdown, .json, .yml, .yaml, .txt. Reads/writes capped at 1 MB. Anything else returns 403.

// GOOD
const ALLOWED_EXTENSIONS = new Set(['.md', '.markdown', '.json', '.yml', '.yaml', '.txt']);
const MAX_FILE_BYTES = 1024 * 1024;

if (!ALLOWED_EXTENSIONS.has(path.extname(filepath))) {
  return res.status(403).json({ error: 'extension-not-allowed' });
}

Severity: CRITICAL (extension), HIGH (size cap)

Schema validation on every boundary

Rule: Every REST request body and Socket.IO payload is zod-validated on both ends.

// BAD: trusting client input
app.post('/api/patterns', (req, res) => {
  store.update(req.body); // unchecked
});

// GOOD: zod parse at the boundary
import { PatternMapSchema } from '@harness-visualizer/shared';

app.post('/api/patterns', (req, res) => {
  const parsed = PatternMapSchema.safeParse(req.body);
  if (!parsed.success) return res.status(400).json({ error: parsed.error.flatten() });
  store.update(parsed.data);
});

Severity: HIGH

No shell-outs to user-controlled paths

Rule: Filesystem APIs only. No child_process.exec(userPath), no spawn with concatenated user input.

Severity: CRITICAL

2. Code Organization

Express handlers stay thin

Rule: Handlers parse + validate + delegate. Business logic lives in scanner/, scoring/, watcher/, etc.

// BAD: handler with logic
app.get('/api/browse', async (req, res) => {
  const entries = await fs.readdir(req.query.path);
  const filtered = entries.filter((e) => !e.startsWith('.'));
  // ...20 more lines
  res.json(filtered);
});

// GOOD: thin handler delegating to a module
app.get('/api/browse', async (req, res) => {
  const parsed = BrowseRequestSchema.safeParse(req.query);
  if (!parsed.success) return res.status(400).json({ error: parsed.error });
  const result = await browseDirectory(parsed.data, watchRoots);
  res.json(result);
});

Severity: MEDIUM

Vue components use <script setup lang="ts">

Rule: All Vue components use Composition API with <script setup lang="ts">. No Options API. No JS-only <script> tags.

Severity: MEDIUM

State management: Pinia setup stores only

Rule: Cross-component state lives in Pinia setup-stores (defineStore('name', () => { ... })). No global event buses.

// GOOD
export const useHarnessStore = defineStore('harness', () => {
  const entries = ref<HarnessEntry[]>([]);
  function applyUpdate(payload: HarnessUpdatePayload) {
    entries.value = payload.entries;
  }
  return { entries, applyUpdate };
});

Severity: MEDIUM

ESM and .js extensions in TS imports

Rule: ESM throughout. TS imports of local files MUST include explicit .js extensions (required by module: NodeNext).

// BAD
import { scanner } from './scanner';
import { __dirname } from 'somewhere';

// GOOD
import { scanner } from './scanner.js';
import { fileURLToPath } from 'node:url';
const __dirname = path.dirname(fileURLToPath(import.meta.url));

Severity: HIGH (build will break without .js)

No barrel files

Rule: No index.ts re-export barrels unless they remove genuine duplication. shared/src/index.ts is a deliberate exception.

Severity: LOW

3. Error Handling

Specific exception handling

Rule: Don't catch generic Error unless you handle it specifically and re-throw or log with context.

// BAD: swallowing
try {
  await scanner.scan(root);
} catch (e) {
  // silent
}

// BAD: generic
try {
  await scanner.scan(root);
} catch (e) {
  res.status(500).json({ error: 'something went wrong' });
}

// GOOD: distinguish recoverable from unexpected
try {
  await scanner.scan(root);
} catch (e) {
  if (e instanceof InvalidPatternError) {
    return res.status(400).json({ error: e.message, code: 'invalid-pattern' });
  }
  logger.error({ err: e, root }, 'scanner failed');
  return res.status(500).json({ error: 'scan-failed' });
}

Severity: MEDIUM

Diagnostic codes are stable contracts

Rule: New diagnostic codes must be added to DIAGNOSTIC_CODES in @harness-visualizer/shared and documented in AGENTS.md. Don't emit ad-hoc string codes — the Phase 7 UI maps codes to icons.

Severity: HIGH

4. Backend Patterns

Debounce file events at ~250 ms

Rule: Chokidar events feed into a hand-rolled debounce before emitting harness:update. No raw event emission.

Severity: MEDIUM

chokidar v5 unwatch footgun

Rule: unwatch(parent) adds a recursive ignore matcher that silently filters events from parent/sub even if sub was added separately.

• Always dedup parent/child overlap server-side BEFORE diffing roots
• Call unwatch(removed) BEFORE add(added) when reconfiguring watch roots
• Don't watch the same directory twice via different roots

Severity: HIGH

STANDARD_IGNORES must include **/.harness-visualizer/**

Rule: App-managed config files (patterns.json, future .harness-visualizer/cache/**) are written by the app and would otherwise trigger redundant chokidar events. Triggering re-scans is the responsibility of the handler that wrote the config.

Severity: HIGH

Real filesystem in tests — do not mock fs

Rule: Backend tests use real tmp dirs via withTmpRoot. The scanner code paths are small enough that integration coverage beats mock fidelity.

// GOOD
await withTmpRoot(async (root) => {
  await fs.writeFile(path.join(root, 'AGENTS.md'), '# test');
  const entries = await scanner.scan(root, patterns);
  expect(entries).toHaveLength(1);
});

Severity: HIGH (mocking fs is forbidden)

5. Frontend Patterns

Scanner re-emit vs ref watchers

Rule: Use watch(() => x.value?.id, cb) for harness-store-derived computeds — never watch(x, cb) against a ref whose value is replaced wholesale.

// BAD: cb fires every harness:update because the ref itself is replaced
const selected = computed(() => store.entries.find((e) => e.id === selectedId.value));
watch(selected, (v) => doExpensiveWork(v));

// GOOD: cb fires only when the meaningful key changes
watch(
  () => selected.value?.id,
  (id) => doExpensiveWork(selected.value),
);

Severity: HIGH (performance + infinite-loop risk)

Typed Socket.IO event maps

Rule: Frontend uses Socket<ServerToClientEvents, ClientToServerEvents> (note flipped order vs server). Both sides import the event-map interfaces from @harness-visualizer/shared.

Severity: HIGH

Tailwind v4 utilities, not scoped CSS

Rule: Prefer Tailwind utility classes. Reach for <style scoped> only when a utility class would be unreadable. CSS-first config (@import "tailwindcss" + @theme + @custom-variant dark).

Severity: LOW

6. Shared package conventions

zod schemas drive types

Rule: Runtime schemas live in shared/src/schemas.ts. Types in shared/src/types.ts derive via z.infer<typeof X>. Never define a type in both places.

// shared/src/schemas.ts
export const HarnessEntrySchema = z.object({
  id: z.string(),
  layer: LayerSchema,
  // ...
});

// shared/src/types.ts
export type HarnessEntry = z.infer<typeof HarnessEntrySchema>;

Severity: HIGH (drift between schema and type breaks validation)

7. Testing Patterns

Test independence

Rule: Tests should not depend on each other or on external state. Vitest runs tests in parallel by default.

• Tests that require a specific order: BAD
• Tests that share global state: BAD
• Tests that depend on files outside tmp dirs: BAD

Severity: MEDIUM

Specific assertions

Rule: Tests should assert exact values, not just truthiness.

// BAD: weak
expect(result).toBeDefined();
expect(response.status).toBe(200);

// GOOD: specific
expect(result.entries).toHaveLength(3);
expect(result.entries[0]?.layer).toBe('skill');
expect(response.body).toMatchObject({ data: { count: 3 } });

Severity: MEDIUM

Failing-on-purpose security tests

Rule: Every security branch (rejected traversal, rejected symlink, rejected oversize, rejected extension) MUST have a unit test that asserts the rejection.

Severity: HIGH

Audit Severity Levels

Level	Meaning	Action Required
CRITICAL	Security vulnerability or data integrity risk	Block release until fixed
HIGH	Significant issue that could cause problems	Fix before release
MEDIUM	Best practice violation	Should fix, can defer
LOW	Style/consistency issue	Nice to fix

Audit Report Format

# Architectural Audit Report

## Summary
- **Files Reviewed**: 15
- **Critical Issues**: 0
- **High Issues**: 2
- **Medium Issues**: 5
- **Low Issues**: 3

## Critical Issues
(None found)

## High Issues

### AUDIT-001: File IO bypasses safeOpen
- **File**: `backend/src/api/patterns.ts:42`
- **Rule**: File IO must go through safeOpen
- **Description**: Pattern handler reads user-provided path with `fs.readFile` directly
- **Recommendation**: Replace with `await safeOpen(path, watchRoots)` and use the returned handle

### AUDIT-002: Missing zod validation on Socket.IO payload
- **File**: `backend/src/server.ts:88`
- **Rule**: Schema validation on every boundary
- **Description**: `configure-watch` handler trusts payload without validation
- **Recommendation**: Parse with `ConfigureWatchPayloadSchema.safeParse(payload)`

## Medium Issues
...

## Recommendations
1. Add a CI check that greps for `fs.readFile|fs.writeFile` outside `safe-open.ts`
2. Add zod parsing to every Socket.IO handler in `backend/src/server.ts`

Quick Reference Checklists

Express handler review

• Body / query / params validated with zod schema from @harness-visualizer/shared
• Thin: delegates to scanner/scoring/watcher module
• Returns JSON via res.json(...), no raw res.write
• Errors handled with specific status codes (400, 403, 404, 500)
• If touching files: uses safeOpen, not bare fs.*

Socket.IO handler review

• Payload validated with zod schema
• Unknown event names emit unknown-event diagnostic
• Reconfiguration paths debounced
• Updates emitted via typed event map

Vue component review

• <script setup lang="ts"> only
• Cross-component state lives in Pinia
• No watch(refToWholeObject, cb) — use watch(() => ref.value?.id, cb)
• Tailwind v4 utilities preferred over scoped CSS
• Tests in __tests__/ or sibling .test.ts

Shared schema review

• Schema in schemas.ts, type derived in types.ts via z.infer
• Re-exported through shared/src/index.ts
• If a new diagnostic code: added to DIAGNOSTIC_CODES and to AGENTS.md diagnostic table

Security-sensitive endpoint review

• Path resolved via path.resolve + fs.realpath
• Path inside watch-root allowlist (Vite-style isParentDirectory)
• Extension in allowlist for editable files
• 1 MB size cap enforced
• Failing-on-purpose test exists for each rejection branch