morluto/rea

Fork 0

mirror of https://github.com/morluto/rea.git synced 2026-11-05 06:45:22 +00:00

[GH-ISSUE #5604] [Bug] IDA reader in #5591 loses descriptor ownership after close failure #3398

New issue

Closed

opened 2026-10-31 00:39:55 +00:00 by bsakaguchi · 1 comment

bsakaguchi commented

2026-10-31 00:39:55 +00:00

Owner

Originally created by @morluto on GitHub (Oct 20, 2026).
Original GitHub issue: https://github.com/morluto/rea/issues/5604

Before submitting

I searched open and closed issues for a related report.
I removed credentials, private paths, and sensitive target data.

Area

Provider or capability discovery

Observed behavior

PR-head finding, not a current-main reproduction. In #5591 at d80bda9bec08349637463f2143006b0f61a41ca3, the new IDA registration reader owns a raw descriptor. If its final closeSync fails, readIdaConfiguration returns an ordinary configuration/read error while the descriptor can remain open with no retained cleanup owner.

Two independent controlled probes reproduced this on that exact source. Each injected EIO before closing only the descriptor opened by the reader. After the parser returned, fstatSync still succeeded on it. Each probe restored the real functions and closed that exact descriptor. This was fault injection, not a naturally occurring OS close failure.

Reproduction

On a checkout of the pinned PR head with dependencies installed, save this as probe.mjs at its root and run node --import tsx/esm probe.mjs:

import fs from 'node:fs';
import { syncBuiltinESMExports } from 'node:module';
import { mkdtempSync, writeFileSync, rmSync } from 'node:fs';
import { tmpdir } from 'node:os';
import { join } from 'node:path';
import { readIdaConfiguration } from './src/ida/IdaConfiguration.ts';

const dir = mkdtempSync(join(tmpdir(), 'rea-ida-close-'));
const path = join(dir, 'registration.json');
writeFileSync(path, JSON.stringify({ command: 'fixture' }));
const open = fs.openSync, close = fs.closeSync;
let fd, injected = false;
try {
  fs.openSync = (...args) => (fd = open(...args));
  fs.closeSync = candidate => {
    if (candidate === fd && !injected) {
      injected = true;
      throw Object.assign(new Error('controlled close refusal'), { code: 'EIO' });
    }
    return close(candidate);
  };
  syncBuiltinESMExports();
  const result = readIdaConfiguration(path);
  console.log({ ok: result.ok, injected, stillOpen: fs.fstatSync(fd).isFile() });
} finally {
  fs.openSync = open; fs.closeSync = close; syncBuiltinESMExports();
  if (fd !== undefined) close(fd);
  rmSync(dir, { recursive: true });
}

Observed result: { ok: false, injected: true, stillOpen: true }. The fuller captured probe also reports Cannot read IDA MCP registration ...; provide a readable JSON configuration.

Expected behavior

Retain the exact descriptor owner when release is unconfirmed, distinguish cleanup failure from read/configuration failure, and allow the responsible lifecycle owner to retry. This follows the repository's ownership rule and its existing OwnedFileHandle/owned regular-file boundaries. Successfully read bytes and any primary failure should remain distinguishable from cleanup outcome.

Impact and scope

One parser-owned descriptor remains open in each injected failure. Natural failure frequency and production impact: NOT_ENOUGH_INFO; no natural OS fault or long-running leak test was performed. No IDA process or target was executed.

REA version

Proposed source in #5591 at d80bda9bec08349637463f2143006b0f61a41ca3. Main at 0ba4d6f43ad610dacc861a1548be1a5c4712cd61 still used readFileSync; this explicit-descriptor reproduction is not claimed for main.

Runtime and platform

Node.js 24.18.0; Darwin 25.3.0, arm64, non-root user. Linux/Windows behavior: NOT_ENOUGH_INFO.

Target and artifact context

Temporary regular JSON registration containing { "command": "fixture" }; no engine or analysis target.

Interface

Other — shared registration parser used by provider admission and diagnostics.

Earliest failure stage

Cleanup, cancellation, or lifecycle

Relevant output or evidence

The new reader's final close throws before returning its read result. The outer catch maps it to a configuration error. The probes established that the owned descriptor remained open after that result; cleanup was then performed by the probe.

Typed outcome or recovery expected

Expose incomplete cleanup with its exact retry owner rather than an ordinary unreadable-registration result.

Additional context

Prepared with AI assistance. Related #5561 is the FIFO hang that #5591 addresses; this report concerns the new descriptor's failed-close path. Closed #4910 fixed the same ownership class in registration/maintenance application helpers, and #4658/#4676 cover other descriptor readers. No open IDA-parser cleanup report was found. This issue records a review finding before integration; it does not establish a released regression or maintainer readiness decision.

Originally created by @morluto on GitHub (Oct 20, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/5604 ### Before submitting - [x] I searched open and closed issues for a related report. - [x] I removed credentials, private paths, and sensitive target data. ### Area Provider or capability discovery ### Observed behavior **PR-head finding, not a current-main reproduction.** In #5591 at `d80bda9bec08349637463f2143006b0f61a41ca3`, the new IDA registration reader owns a raw descriptor. If its final `closeSync` fails, `readIdaConfiguration` returns an ordinary configuration/read error while the descriptor can remain open with no retained cleanup owner. Two independent controlled probes reproduced this on that exact source. Each injected `EIO` before closing only the descriptor opened by the reader. After the parser returned, `fstatSync` still succeeded on it. Each probe restored the real functions and closed that exact descriptor. This was fault injection, not a naturally occurring OS close failure. ### Reproduction On a checkout of the pinned PR head with dependencies installed, save this as `probe.mjs` at its root and run `node --import tsx/esm probe.mjs`: ```js import fs from 'node:fs'; import { syncBuiltinESMExports } from 'node:module'; import { mkdtempSync, writeFileSync, rmSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { readIdaConfiguration } from './src/ida/IdaConfiguration.ts'; const dir = mkdtempSync(join(tmpdir(), 'rea-ida-close-')); const path = join(dir, 'registration.json'); writeFileSync(path, JSON.stringify({ command: 'fixture' })); const open = fs.openSync, close = fs.closeSync; let fd, injected = false; try { fs.openSync = (...args) => (fd = open(...args)); fs.closeSync = candidate => { if (candidate === fd && !injected) { injected = true; throw Object.assign(new Error('controlled close refusal'), { code: 'EIO' }); } return close(candidate); }; syncBuiltinESMExports(); const result = readIdaConfiguration(path); console.log({ ok: result.ok, injected, stillOpen: fs.fstatSync(fd).isFile() }); } finally { fs.openSync = open; fs.closeSync = close; syncBuiltinESMExports(); if (fd !== undefined) close(fd); rmSync(dir, { recursive: true }); } ``` Observed result: `{ ok: false, injected: true, stillOpen: true }`. The fuller captured probe also reports `Cannot read IDA MCP registration ...; provide a readable JSON configuration.` ### Expected behavior Retain the exact descriptor owner when release is unconfirmed, distinguish cleanup failure from read/configuration failure, and allow the responsible lifecycle owner to retry. This follows the repository's ownership rule and its existing `OwnedFileHandle`/owned regular-file boundaries. Successfully read bytes and any primary failure should remain distinguishable from cleanup outcome. ### Impact and scope One parser-owned descriptor remains open in each injected failure. Natural failure frequency and production impact: `NOT_ENOUGH_INFO`; no natural OS fault or long-running leak test was performed. No IDA process or target was executed. ### REA version Proposed source in #5591 at `d80bda9bec08349637463f2143006b0f61a41ca3`. Main at `0ba4d6f43ad610dacc861a1548be1a5c4712cd61` still used `readFileSync`; this explicit-descriptor reproduction is not claimed for main. ### Runtime and platform Node.js 24.18.0; Darwin 25.3.0, arm64, non-root user. Linux/Windows behavior: `NOT_ENOUGH_INFO`. ### Target and artifact context Temporary regular JSON registration containing `{ "command": "fixture" }`; no engine or analysis target. ### Interface Other — shared registration parser used by provider admission and diagnostics. ### Earliest failure stage Cleanup, cancellation, or lifecycle ### Relevant output or evidence The [new reader's final close](https://github.com/morluto/rea/blob/d80bda9bec08349637463f2143006b0f61a41ca3/src/ida/IdaConfiguration.ts#L128-L133) throws before returning its read result. The [outer catch](https://github.com/morluto/rea/blob/d80bda9bec08349637463f2143006b0f61a41ca3/src/ida/IdaConfiguration.ts#L78-L84) maps it to a configuration error. The probes established that the owned descriptor remained open after that result; cleanup was then performed by the probe. ### Typed outcome or recovery expected Expose incomplete cleanup with its exact retry owner rather than an ordinary unreadable-registration result. ### Additional context Prepared with AI assistance. Related #5561 is the FIFO hang that #5591 addresses; this report concerns the new descriptor's failed-close path. Closed #4910 fixed the same ownership class in registration/maintenance application helpers, and #4658/#4676 cover other descriptor readers. No open IDA-parser cleanup report was found. This issue records a review finding before integration; it does not establish a released regression or maintainer readiness decision.

bsakaguchi

2026-10-31 00:39:55 +00:00

closed this issue
added the
area: providers

bug

triage
labels

bsakaguchi commented

2026-10-31 00:39:56 +00:00

Author

Owner

@matbrgz commented on GitHub (Oct 20, 2026):

Cross-referencing with canonical issue #5559 and sibling #5561 regarding IDA provider registration descriptor lifecycle and cleanup handling. Cataloged under Cluster BF in consolidation tracker #4330.

@matbrgz commented on GitHub (Oct 20, 2026): Cross-referencing with canonical issue #5559 and sibling #5561 regarding IDA provider registration descriptor lifecycle and cleanup handling. Cataloged under Cluster BF in consolidation tracker #4330.

bsakaguchi referenced this issue

2026-10-31 01:09:08 +00:00

[PR #3454] [CLOSED] fix(rust): isolate ELF section reads from Go build-info admission #5747

bsakaguchi referenced this issue

2026-11-01 21:42:54 +00:00

[PR #5069] feat(rust): read cargo-auditable dependencies from WebAssembly modules #6768