Skip to content

fix: Replace Bun.stdin with process.stdin in 10 v3.0 hooks (Windows/MSYS)#713

Closed
chrisglick wants to merge 3 commits intodanielmiessler:mainfrom
chrisglick:fix/bun-stdin-windows-compat
Closed

fix: Replace Bun.stdin with process.stdin in 10 v3.0 hooks (Windows/MSYS)#713
chrisglick wants to merge 3 commits intodanielmiessler:mainfrom
chrisglick:fix/bun-stdin-windows-compat

Conversation

@chrisglick
Copy link
Copy Markdown
Contributor

Summary

  • 10 hooks still used Bun.stdin.text() or Bun.stdin.stream().getReader(), which fail silently on Windows/MSYS (empty return or indefinite hang)
  • All 10 now use process.stdin with event listeners and a timeout fallback — the same cross-platform pattern already used by the other 11 hooks
  • Zero references to Bun.stdin remain in any v3.0 hook file

Files Changed

Hook Old Pattern Timeout
AlgorithmTracker stream().getReader() 200ms
IntegrityCheck stream().getReader() 500ms
QuestionAnswered Bun.stdin.text() 1s
SecurityValidator Promise.race + Bun.stdin.text() 100ms
SessionAutoName stream().getReader() 2s
SessionSummary Promise.race + Bun.stdin.text() 3s
StartupGreeting Bun.stdin.text() 1s
StopOrchestrator stream().getReader() 500ms
VoiceGate stream().getReader() 200ms
WorkCompletionLearning Promise.race + Bun.stdin.text() 3s

The Pattern

const input = await new Promise<string>((resolve) => {
  let data = '';
  const timer = setTimeout(() => resolve(data), timeout);
  process.stdin.setEncoding('utf8');
  process.stdin.on('data', chunk => { data += chunk; });
  process.stdin.on('end', () => { clearTimeout(timer); resolve(data); });
  process.stdin.on('error', () => { clearTimeout(timer); resolve(''); });
});

Test plan

  • Verify grep -r "Bun\.stdin" Releases/v3.0/.claude/hooks/ returns zero matches
  • Run hooks on Windows/MSYS and confirm stdin is read correctly
  • Run hooks on macOS/Linux and confirm no regression

Fixes #385

🤖 Generated with Claude Code

@kaimagnus
Copy link
Copy Markdown
Collaborator

Thanks for the thorough Windows compatibility work @chrisglick! We're reviewing the Windows PR series (#704, #706, #713) as a batch since they're interconnected. We don't have a Windows test environment so we'll coordinate with other Windows users to verify before merging. Great contribution! 🙏

nsalvacao added a commit to Experiments-Nuno-Salvacao/Personal_AI_Infrastructure that referenced this pull request Feb 17, 2026
…in in hooks for Windows/MSYS)

# Conflicts:
#	Releases/v3.0/.claude/hooks/SecurityValidator.hook.ts
@fayerman-source
Copy link
Copy Markdown
Contributor

Tested on Windows 11, Git Bash (MINGW64). Here's what I got:

grep -r "Bun\.stdin" . — clean, nothing comes back

Hook responses:

  • SecurityValidator → {"continue":true} right away
  • VoiceGate → {"continue":true} right away
  • StartupGreeting → errored on missing settings.json (no PAI installed in my test env), but didn't hang
  • SessionAutoName → exited clean

None of them hung. Looks good to me.

chrisglick and others added 3 commits February 18, 2026 09:42
…s/MSYS compatibility

Bun.stdin.text() returns empty and Bun.stdin.stream().getReader() can hang
indefinitely on Windows/MSYS. All 10 remaining hooks now use process.stdin
with event listeners and a timeout fallback, matching the pattern already
used by 11 other hooks.

Fixes danielmiessler#385

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Addresses code review feedback from nsalvacao's Gemini audit:
- Extract duplicated process.stdin pattern into reusable readStdinWithTimeout()
- All 10 hooks now import from lib/stdin.ts instead of inline patterns
- Fix require('os').homedir() → import { homedir } from 'os' for consistency
- stdin.destroy() on timeout prevents event loop hanging (DoS mitigation)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Addresses Gemini Code Assist security findings:
- Add sanitizeSessionId() to lib/paths.ts — strips path traversal chars
- Apply sanitization in SessionSummary, WorkCompletionLearning, StartupGreeting
- Replace inline BASE_DIR with getPaiDir() from lib/paths.ts (DRY)
- Remove unused homedir imports after centralizing

Mitigates: path traversal via crafted session_id in stdin input.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@chrisglick chrisglick force-pushed the fix/bun-stdin-windows-compat branch from c029678 to d02c07e Compare February 18, 2026 17:53
@chrisglick
Copy link
Copy Markdown
Contributor Author

Update: Rebased, DRY refactor, and security improvements

Thanks @kaimagnus for coordinating the batch review, and @fayerman-source for the Windows 11 testing — great to have confirmation it works clean on Git Bash/MINGW64.

What changed in this push

I've rebased onto latest main (resolving the merge conflict in SecurityValidator.hook.ts) and added two follow-up commits addressing code review feedback:

1. DRY refactor (refactor: Extract readStdinWithTimeout to shared lib/stdin.ts)

  • The duplicated process.stdin reading pattern (flagged across 9+ hooks) is now a single reusable function in hooks/lib/stdin.ts
  • All 10 hooks import from the shared utility instead of inline patterns
  • Fixed inconsistent require('os').homedir() → proper ES module import { homedir } from 'os'
  • Added process.stdin.destroy() on timeout to prevent event loop hanging (DoS mitigation)

2. Security hardening (security: Add session_id sanitization and centralize BASE_DIR)

  • Added sanitizeSessionId() to lib/paths.ts — strips path traversal characters (../../) from session IDs before they're used in file path construction
  • Applied to SessionSummary, WorkCompletionLearning, and StartupGreeting hooks
  • Replaced inline BASE_DIR resolution with centralized getPaiDir() from lib/paths.ts (DRY)

On the failing CI check

The claude-review check failure is a workflow configuration issue — needs id-token: write permission in the GitHub Actions workflow YAML. Not related to this PR's code changes.

Credit

The DRY and security findings came from @nsalvacao's fork (Experiments-Nuno-Salvacao#5) which ran Gemini Code Assist reviews against these changes. The path traversal findings in particular were valuable — session_id from stdin was being used unsanitized in join() calls that feed into unlinkSync, readFileSync, and writeFileSync. Now sanitized at the entry point.

Test plan (updated)

  • grep -r "Bun\.stdin" Releases/v3.0/.claude/hooks/ → zero matches
  • All 10 hooks import from shared lib/stdin.ts
  • stripEnvVarPrefix and other upstream security additions preserved after rebase
  • sanitizeSessionId applied to hooks that use session_id in file paths
  • Run hooks on Windows/MSYS — confirm no regression
  • Run hooks on macOS/Linux — confirm no regression

p4gs added a commit to p4gs/Personal_AI_Infrastructure that referenced this pull request Feb 20, 2026
…ening

Replace Bun.stdin (broken on Windows) with shared readStdinWithTimeout()
utility across 10 hooks. Add sanitizeSessionId() to prevent path traversal.

Changes:
- Create hooks/lib/stdin.ts with cross-platform stdin reading
- Migrate 10 hooks from Bun.stdin.text()/stream() to readStdinWithTimeout()
- Add sanitizeSessionId() to hooks/lib/paths.ts for path traversal prevention
- Update PRD with Phase 2 verification evidence (7/30 ISC passing)

Hooks migrated: AlgorithmTracker, IntegrityCheck, SecurityValidator,
StopOrchestrator, VoiceGate, QuestionAnswered, SessionAutoName,
SessionSummary, StartupGreeting, WorkCompletionLearning

Inspired by chrisglick's PR danielmiessler#713 analysis (adapted for native Windows).

Generated with [Claude Code](https://claude.ai/code)
via [Happy](https://happy.engineering)

Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Happy <yesreply@happy.engineering>
@danielmiessler
Copy link
Copy Markdown
Owner

Thanks for this contribution! PAI v4.0 significantly restructured the architecture — new directory layout, Algorithm v3.6.0 (up from v1.x), hooks/lib/paths.ts for path resolution, and many of the underlying systems this PR targets have been rewritten.

This PR targets the v3.0 architecture and can't be cleanly applied to v4.0.x. Closing as superseded.

The latest release is v4.0.2. We're actively working on platform compatibility and other bigger items. If there's still a gap not covered by the new releases, we'd welcome a fresh PR against the current codebase. Thanks again!

@nsalvacao
Copy link
Copy Markdown

Update: Rebased, DRY refactor, and security improvements

Thanks @kaimagnus for coordinating the batch review, and @fayerman-source for the Windows 11 testing — great to have confirmation it works clean on Git Bash/MINGW64.

What changed in this push

I've rebased onto latest main (resolving the merge conflict in SecurityValidator.hook.ts) and added two follow-up commits addressing code review feedback:

1. DRY refactor (refactor: Extract readStdinWithTimeout to shared lib/stdin.ts)

  • The duplicated process.stdin reading pattern (flagged across 9+ hooks) is now a single reusable function in hooks/lib/stdin.ts
  • All 10 hooks import from the shared utility instead of inline patterns
  • Fixed inconsistent require('os').homedir() → proper ES module import { homedir } from 'os'
  • Added process.stdin.destroy() on timeout to prevent event loop hanging (DoS mitigation)

2. Security hardening (security: Add session_id sanitization and centralize BASE_DIR)

  • Added sanitizeSessionId() to lib/paths.ts — strips path traversal characters (../../) from session IDs before they're used in file path construction
  • Applied to SessionSummary, WorkCompletionLearning, and StartupGreeting hooks
  • Replaced inline BASE_DIR resolution with centralized getPaiDir() from lib/paths.ts (DRY)

On the failing CI check

The claude-review check failure is a workflow configuration issue — needs id-token: write permission in the GitHub Actions workflow YAML. Not related to this PR's code changes.

Credit

The DRY and security findings came from @nsalvacao's fork (Experiments-Nuno-Salvacao#5) which ran Gemini Code Assist reviews against these changes. The path traversal findings in particular were valuable — session_id from stdin was being used unsanitized in join() calls that feed into unlinkSync, readFileSync, and writeFileSync. Now sanitized at the entry point.

Test plan (updated)

  • grep -r "Bun\.stdin" Releases/v3.0/.claude/hooks/ → zero matches
  • All 10 hooks import from shared lib/stdin.ts
  • stripEnvVarPrefix and other upstream security additions preserved after rebase
  • sanitizeSessionId applied to hooks that use session_id in file paths
  • Run hooks on Windows/MSYS — confirm no regression
  • Run hooks on macOS/Linux — confirm no regression

Thanks for the credit! 👍 :)

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.

Windows/MSYS: Bun.stdin.text() doesn't work - hooks fail silently

5 participants