Skip to content

Fix SecurityValidator yaml import crash via lazy dynamic load (#156)#157

Merged
virtualian merged 1 commit intomainfrom
fix/156-securityvalidator-yaml-package
Apr 26, 2026
Merged

Fix SecurityValidator yaml import crash via lazy dynamic load (#156)#157
virtualian merged 1 commit intomainfrom
fix/156-securityvalidator-yaml-package

Conversation

@virtualian
Copy link
Copy Markdown
Owner

Summary

  • Replace the static import 'yaml' in SecurityValidator.hook.ts with a guarded await import('yaml'), so the missing yaml package no longer crashes the hook at module load.
  • Hook now silently fails open (consistent with existing missing-patterns and parse-error paths) instead of emitting Cannot find package 'yaml' from … on every PreToolUse:Read.
  • Runtime ~/.pai/hooks/ and shipped Releases/v4.0.3+/.claude/hooks/ stay byte-identical.

Test plan

  • diff runtime vs shipped → byte-identical
  • echo JSON | bun run …SecurityValidator.hook.ts for {Read, Bash, Write} tool inputs → all return {"continue":true} exit 0 with zero stderr
  • Failing-dynamic-import latency measured at ~10ms total per invocation (mostly Bun cold-start), well inside the 200ms stdin window
  • Audit of all 17 PAI hooks for third-party npm imports → only SecurityValidator.hook.ts uses any (yaml); no other hooks affected
  • import type erasure verified empirically — runtime test confirms no resolution attempt for the type-only import

Out of scope (deeper finding — follow-up issue)

SecurityValidator has been silently a no-op since v4.x. Even with this fix, parseYaml remains null because Bun resolves dynamic imports relative to the script file, not cwd, and no node_modules exists at any ancestor of ~/.pai/hooks/. Additionally, patterns.example.yaml stopped shipping after v3.0 and is absent from Releases/v4.0.3+/. Restoring real pattern matching needs both pieces. Filing follow-up issue.

Closes #156.

Static `import { parse as parseYaml } from 'yaml'` crashed the hook at
module load on every PreToolUse:Read because no node_modules resolves
`yaml` from `~/.pai/hooks/`. Claude Code reported the error as
non-blocking, but the warning fired on every Read, polluting session
output.

Replace with a lazy dynamic import wrapped in try/catch:
- Type-only import via `import type { parse as YamlParse } from 'yaml'`
  (erased at parse time; cannot reintroduce the resolution warning)
- `ensureYamlLoaded()` awaits `import('yaml')` once at the top of `main()`
  and silently sets `parseYaml = null` on failure
- `loadPatterns()` short-circuits to a fail-open empty config when
  `parseYaml` is null, symmetric with the existing no-file and
  parse-error fail-open paths

The same edit is applied to the runtime `~/.pai/hooks/SecurityValidator.hook.ts`
so the warning stops in the current session; the runtime and shipped
copies remain byte-identical.

Verified empirically: hook returns `{"continue":true}` exit 0 with zero
stderr for Read/Bash/Write inputs (vs pre-fix `Cannot find package 'yaml'`
warning). Failing dynamic-import cost ~10ms total per invocation, well
inside the 200ms stdin window.

Audit confirms only `SecurityValidator.hook.ts` uses any third-party npm
package across all 17 PAI hooks — no other hooks affected.

Note: this fix silences the warning but does NOT restore SecurityValidator
to functional state. The validator has been silently a no-op since the
v3.0 → v4.x refactor stopped shipping `patterns.example.yaml`. Restoring
real pattern matching requires a separate PR that (a) makes `yaml`
resolvable from `~/.pai/hooks/` and (b) restores the patterns file under
`Releases/v4.0.3+/.claude/PAISECURITYSYSTEM/`. Tracked in follow-up issue.

Closes #156.
@virtualian virtualian merged commit 41621df into main Apr 26, 2026
1 check passed
@virtualian virtualian deleted the fix/156-securityvalidator-yaml-package branch April 26, 2026 21:24
virtualian added a commit that referenced this pull request Apr 26, 2026
…159)

Two independent regressions had silently fail-opened SecurityValidator since v4.0.0+:

1. yaml package not resolvable from ~/.pai/hooks/ — Bun resolves dynamic
   await import('yaml') relative to the script file. PR #156 silenced the
   import-time crash with lazy-load + try/catch, but parseYaml permanently
   resolved to null, so loadPatterns() returned the empty fail-open config.

2. patterns.example.yaml was no longer shipped under Releases/v4.0.3+/.
   Last canonical copy lived under Releases/v3.0/.claude/skills/PAI/.
   Even with yaml resolvable, getPatternsPath() returned null and
   loadPatterns() fell through to the no-patterns fail-open branch.

This commit is the runtime/Releases-backport scope (option 1 of three).
Restores both shipping artefacts and adds a behaviour-test harness so the
regression cannot return silently. Installer wiring (a new step that copies
package.json to the runtime root and runs bun install there, plus
materializing the PAI/PAISECURITYSYSTEM/ subtree) is deferred to a follow-up
issue.

Files:
- Releases/v4.0.3+/.claude/PAI/PAISECURITYSYSTEM/patterns.example.yaml
  (restored verbatim from Releases/v3.0/, version 1.0; path mirrors
  codePath('PAI', 'PAISECURITYSYSTEM', ...) under the v4 two-root layout)
- Releases/v4.0.3+/.claude/package.json (declares yaml ^2.0.0)
- Tools/verify-security-validator.sh (8 checks: 4 prerequisites, 4 hook
  scenarios — block, zero-access, allow, confirm)

Verified locally: harness reports PASS=8 FAIL=0; filesystem-destruction
patterns block with exit 2, Read of zero-access SSH key paths blocks with
exit 2, safe commands continue, force-push patterns prompt for
confirmation. Fail-open paths from #156 still degrade gracefully when
yaml or patterns.example.yaml is missing.

SecurityValidator.hook.ts itself is unchanged.

Refs #156 #157
virtualian added a commit that referenced this pull request Apr 27, 2026
…) (#161)

Closes the regression chain #156#157#158#159#160. New
`migratePaiRuntime` helper copies `~/.claude/{package.json,bun.lock}`
and `~/.claude/PAI/PAISECURITYSYSTEM/` into `~/.pai/`, then runs
`bun install` if `node_modules/yaml/` is absent or the manifest was
just refreshed. Adds `tryExecAt` (structured cwd, no shell) to
`exec.ts`. Tracks `Releases/v4.0.3+/.claude/bun.lock` for reproducible
installs (pins yaml@2.8.3).

Soft-fails per sub-routine — failures surface via
`Tools/verify-security-validator.sh` rather than aborting the install.
After this lands, a fresh-machine install passes the verify script
PASS=8 FAIL=0 with no manual setup.

- New: `Releases/v4.0.3+/.claude/PAI-Install/engine/pai-runtime-migration.ts`
- New: `Releases/v4.0.3+/.claude/bun.lock`
- Edit: `actions.ts` — invoke from `runRepository` after
  `migratePerPackCommands`, both fresh-install and upgrade paths
- Edit: `exec.ts` — add `tryExecAt` for shell-free subprocess calls
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SecurityValidator hook fires PreToolUse:Read error: missing 'yaml' package in ~/.pai/

1 participant