[PR #3788] [MERGED] fix(javascript): reject FIFO recovery inputs without blocking #5900

New issue

Closed

opened 2026-10-31 01:10:17 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 01:10:17 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/3788
Author: @kaoru0822-kitauji
Created: 10/15/2026
Status: ✅ Merged
Merged: 10/15/2026
Merged by: @kaoru0822-kitauji

Base: main ← Head: fix/3332-recovery-fifo-input

📝 Commits (2)

54d4db6 fix(javascript): reject FIFO recovery inputs without blocking
5fa89ff fix(javascript): keep a failed admission close owned on cancellation

📊 Changes

3 files changed (+109 additions, -5 deletions)

View changed files

➕ src/javascript/recovery/RecoveryFiles.test.ts (+55 -0)
📝 src/javascript/recovery/RecoveryFiles.ts (+23 -4)
📝 tests/boundary/javascript-recovery/wakaruIntegration.test.ts (+31 -1)

📄 Description

Fixes #3332

Root cause

scanRecoveryFile (src/javascript/recovery/RecoveryFiles.ts), which reads every selected and additional recovery input, called open(path, O_RDONLY | O_NOFOLLOW) and only checked isFile() afterwards. On a FIFO with no writer, that open blocks inside a libuv threadpool thread. Cancellation and the recovery deadline are checked only after open returns, so the read never settles. Even process.exit() cannot finish, because teardown waits for the blocked thread.

The repository already owns this check: src/filesystem/RegularFile.ts#openRegularFile opens with O_NONBLOCK | O_NOCTTY (plus O_NOFOLLOW when symlinks are rejected) and verifies a regular file on the descriptor before any read. The pcap, ZIP, Mach-O, APK, Flutter and inventory readers already use it. The recovery reader was the remaining duplicate open path.

Change

scanRecoveryFile now admits its input through openRegularFile(path, { symlinks: "reject", signal }):

A FIFO, directory or other non-regular file fails admission immediately. It maps to the existing Expected a regular file no larger than … bytes output error, which the input snapshot already reports as invalid_request.
Symlink rejection is unchanged (O_NOFOLLOW, ELOOP).
A cancellation observed during admission still surfaces as AnalysisCancelledError.
A descriptor whose close failed is still surfaced through RegularFileAdmissionFailure, which retains its owner.

The local isFile() check is superseded and removed; the size bound stays.

Validation

Production reader (dist/javascript/recovery/RecoveryFiles.js#readRecoveryFile), using the issue's probe (abort after 100 ms), macOS 26 arm64, Node 26.5:

input	main	this PR
regular file	resolved, 2 ms	resolved, 2 ms
FIFO, no writer	never settled; still blocked after >60 s, ignored `process.exit`, needed SIGTERM	rejected `AnalysisOutputError` in 1 ms
directory	rejected (regular-file error)	rejected (same error)
symlink	rejected `ELOOP`	rejected `ELOOP`

Output size is unchanged: the same typed error envelope.

Tests:

New src/javascript/recovery/RecoveryFiles.test.ts, non-Windows: a regular-file control plus a writerless FIFO, bounded by the shared readWithoutFifoWriter fixture. It fails on main; I checked by temporarily restoring the blocking open. The writer guard times out after 1 s with "Recovery input read waited for a FIFO writer".
New workflow test in tests/boundary/javascript-recovery/wakaruIntegration.test.ts ("Wakaru input admission", Linux x64): service.recover on a FIFO returns invalid_request without waiting, never launches Wakaru, and creates no output.
Recovery unit tests pass (3 files, 17 tests), as does npm run check:fast.

Not verified locally: the public recover-javascript-sources CLI path and the new Linux x64 boundary test. Wakaru recovery is Linux x64 only; on macOS the CLI refuses with capability_unavailable before reading input, and the boundary suite is skipped. CI's Linux x64 shard runs that test.

🤖 Generated with Claude Code

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/morluto/rea/pull/3788 **Author:** [@kaoru0822-kitauji](https://github.com/kaoru0822-kitauji) **Created:** 10/15/2026 **Status:** ✅ Merged **Merged:** 10/15/2026 **Merged by:** [@kaoru0822-kitauji](https://github.com/kaoru0822-kitauji) **Base:** `main` ← **Head:** `fix/3332-recovery-fifo-input` --- ### 📝 Commits (2) - [`54d4db6`](https://github.com/morluto/rea/commit/54d4db6492a4a0fa357e6be0d60ae1706856c5df) fix(javascript): reject FIFO recovery inputs without blocking - [`5fa89ff`](https://github.com/morluto/rea/commit/5fa89ff93a44afcbb04f4b96538fe19031b2d9b1) fix(javascript): keep a failed admission close owned on cancellation ### 📊 Changes **3 files changed** (+109 additions, -5 deletions) <details> <summary>View changed files</summary> ➕ `src/javascript/recovery/RecoveryFiles.test.ts` (+55 -0) 📝 `src/javascript/recovery/RecoveryFiles.ts` (+23 -4) 📝 `tests/boundary/javascript-recovery/wakaruIntegration.test.ts` (+31 -1) </details> ### 📄 Description Fixes #3332 ## Root cause `scanRecoveryFile` (`src/javascript/recovery/RecoveryFiles.ts`), which reads every selected and additional recovery input, called `open(path, O_RDONLY | O_NOFOLLOW)` and only checked `isFile()` afterwards. On a FIFO with no writer, that `open` blocks inside a libuv threadpool thread. Cancellation and the recovery deadline are checked only after `open` returns, so the read never settles. Even `process.exit()` cannot finish, because teardown waits for the blocked thread. The repository already owns this check: `src/filesystem/RegularFile.ts#openRegularFile` opens with `O_NONBLOCK | O_NOCTTY` (plus `O_NOFOLLOW` when symlinks are rejected) and verifies a regular file on the descriptor before any read. The pcap, ZIP, Mach-O, APK, Flutter and inventory readers already use it. The recovery reader was the remaining duplicate open path. ## Change `scanRecoveryFile` now admits its input through `openRegularFile(path, { symlinks: "reject", signal })`: - A FIFO, directory or other non-regular file fails admission immediately. It maps to the existing `Expected a regular file no larger than … bytes` output error, which the input snapshot already reports as `invalid_request`. - Symlink rejection is unchanged (`O_NOFOLLOW`, `ELOOP`). - A cancellation observed during admission still surfaces as `AnalysisCancelledError`. - A descriptor whose close failed is still surfaced through `RegularFileAdmissionFailure`, which retains its owner. The local `isFile()` check is superseded and removed; the size bound stays. ## Validation Production reader (`dist/javascript/recovery/RecoveryFiles.js#readRecoveryFile`), using the issue's probe (abort after 100 ms), macOS 26 arm64, Node 26.5: | input | main | this PR | |---|---|---| | regular file | resolved, 2 ms | resolved, 2 ms | | FIFO, no writer | **never settled**; still blocked after >60 s, ignored `process.exit`, needed SIGTERM | rejected `AnalysisOutputError` in **1 ms** | | directory | rejected (regular-file error) | rejected (same error) | | symlink | rejected `ELOOP` | rejected `ELOOP` | Output size is unchanged: the same typed error envelope. **Tests:** - New `src/javascript/recovery/RecoveryFiles.test.ts`, non-Windows: a regular-file control plus a writerless FIFO, bounded by the shared `readWithoutFifoWriter` fixture. It **fails on `main`**; I checked by temporarily restoring the blocking open. The writer guard times out after 1 s with "Recovery input read waited for a FIFO writer". - New workflow test in `tests/boundary/javascript-recovery/wakaruIntegration.test.ts` ("Wakaru input admission", Linux x64): `service.recover` on a FIFO returns `invalid_request` without waiting, never launches Wakaru, and creates no output. - Recovery unit tests pass (3 files, 17 tests), as does `npm run check:fast`. **Not verified locally:** the public `recover-javascript-sources` CLI path and the new Linux x64 boundary test. Wakaru recovery is Linux x64 only; on macOS the CLI refuses with `capability_unavailable` before reading input, and the boundary suite is skipped. CI's Linux x64 shard runs that test. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-31 01:10:17 +00:00