morluto/rea

Fork 0

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

[GH-ISSUE #6030] [Bug] Hopper file-offset close failures hide completed reads and lose descriptor ownership #3678

New issue

Closed

opened 2026-10-31 00:42:58 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 00:42:58 +00:00

Owner

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

Before submitting

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

Area

Native analysis or decompilation

Observed behavior

A focused production-provider test reproduced two consequences of a rejected source-descriptor close during address_to_file_offset. After a complete read, injected close EIO became BinaryTargetError, and injected close EPERM became AnalysisAccessDeniedError, both with cleanupIncomplete=false. The exact descriptor remained open after the provider client's close() returned success. With normal closes, the same input returned file offset 8.

A nonregular-source control returned AnalysisArtifactChangedError; adding the close failure replaced that earlier source verdict with BinaryTargetError.

The test used real file handles and the production mapper, with a simulated successful bridge response and controlled close failures. No real Hopper installation, CLI command or MCP transport was exercised.

Reproduction

Save the following as src/hopper/fileOffsetCloseReproduction.test.ts in the baseline checkout with locked dependencies installed. Set REA_HOPPER_TEST_ROOT to a disposable writable directory and run:

node node_modules/vitest/vitest.mjs run src/hopper/fileOffsetCloseReproduction.test.ts --maxWorkers 1

The assertions characterize the current defect; passing confirms the incorrect classification and retained open descriptors. Remove the temporary test after running. This focused case passed with the output below.

import { createHash } from 'node:crypto';
import { open, writeFile, mkdir, mkdtemp, rm, type FileHandle } from 'node:fs/promises';
import { join } from 'node:path';
import { expect, it, vi } from 'vitest';
import { HopperProvider } from './HopperProvider.js';
import { HopperClient } from './HopperClient.js';
import { parseConfig } from '../config/parseConfig.js';
import { createAnalysisProfile } from '../domain/analysisProfile.js';
import { silentLogger } from '../logger.js';
import { ok } from '../domain/result.js';

const state = vi.hoisted(() => ({path:'', fail:false, code:'EIO', handles:[] as Array<{handle:FileHandle, close:()=>Promise<void>}>, ended:0}));
vi.mock('node:fs/promises', async (importOriginal) => {
  const actual = await importOriginal<typeof import('node:fs/promises')>();
  return {...actual, open: async (...args: Parameters<typeof actual.open>) => {
    const handle = await actual.open(...args);
    if (String(args[0]) === state.path) {
      const close = handle.close.bind(handle);
      state.handles.push({handle,close});
      const stream = handle.createReadStream.bind(handle);
      handle.createReadStream = (options) => stream(options).on('end',()=>{state.ended++;});
      handle.close = async () => {
        if(state.fail) throw Object.assign(new Error('controlled descriptor close failure'),{code:state.code});
        await close();
      };
    }
    return handle;
  }};
});
const root = await mkdtemp(join(process.env.REA_HOPPER_TEST_ROOT!, 'runtime-'));
const observation = {address:'0x1000',provider_file_offset:8,provider_source_path:null,provider_image_header_hex:null};
it('provider offset read succeeds, but failed close is projected as source failure and not retried by close', async () => {
  await mkdir(root,{recursive:true});
  const bytes = Buffer.alloc(64);
  bytes.writeUInt32LE(0xfeedfacf,0);bytes.writeUInt32LE(0x0100000c,4);
  state.path = join(root,'sample');
  await writeFile(state.path,bytes);
  const config = parseConfig({HOPPER_LAUNCHER_PATH:process.execPath});
  if(!config.ok) throw config.error;
  const provider = new HopperProvider(config.value,silentLogger,{});
  const client = provider.createClient({path:state.path,sha256:createHash('sha256').update(bytes).digest('hex'),kind:'executable',format:'mach-o',architecture:'arm64',availableArchitectures:['arm64']},createAnalysisProfile({id:'hopper',name:'Hopper',version:'fixture'},{macho_container:'thin'}));
  const call = vi.spyOn(HopperClient.prototype,'callTool').mockResolvedValue(ok(observation));
  try {
    const control = await client.execute('address_to_file_offset',{address:'0x1000'});
    expect(control.ok).toBe(true);
    if(control.ok) expect(JSON.stringify(control.value)).toContain('"file_offset":8');
    for(const code of ['EIO','EPERM']) {
      state.code=code;state.fail=true;
      const result=await client.execute('address_to_file_offset',{address:'0x1000'});
      expect(result.ok).toBe(false);
      if(result.ok) throw new Error('Expected failure');
      expect(result.error._tag).toBe(code==='EIO'?'BinaryTargetError':'AnalysisAccessDeniedError');
      expect(result.error.cleanupIncomplete).toBe(false);
      const owned=state.handles.at(-1)!;
      expect(owned.handle.fd).toBeGreaterThanOrEqual(0);
      expect((await client.close()).ok).toBe(true);
      expect(owned.handle.fd).toBeGreaterThanOrEqual(0);
      console.log(JSON.stringify({code,error:result.error._tag,readEnded:state.ended,cleanupIncomplete:result.error.cleanupIncomplete,descriptorOpenAfterClientClose:true}));
    }
    state.fail=false;
    const restored=await client.execute('address_to_file_offset',{address:'0x1000'});
    expect(restored.ok).toBe(true);
    expect(state.ended).toBe(4);
    console.log('control and restored close: verified file_offset=8');
    await rm(state.path);await mkdir(state.path);
    const nonregular = await client.execute('address_to_file_offset',{address:'0x1000'});
    expect(nonregular).toMatchObject({ok:false,error:{_tag:'AnalysisArtifactChangedError'}});
    state.fail=true;state.code='EIO';
    const masked = await client.execute('address_to_file_offset',{address:'0x1000'});
    expect(masked).toMatchObject({ok:false,error:{_tag:'BinaryTargetError',cleanupIncomplete:false}});
    console.log('nonregular-source control: AnalysisArtifactChangedError; close failure replaces it with BinaryTargetError');
  } finally {
    call.mockRestore();state.fail=false;
    for(const entry of state.handles) {await entry.close();expect(entry.handle.fd).toBe(-1);}
    await client.close();await rm(root,{recursive:true,force:true});
  }
});

Expected behavior

Keep the completed read/hash snapshot or earlier nonregular-source failure while reporting close failure separately as typed cleanup. Retain the exact descriptor owner for cleanup retry; a successful owner retry should clear only that resource's incomplete status. Do not describe failed close as failed source access. Do not return successful coordinates until the normal source-identity checks have run.

The lifecycle rules require preserving completed observations and primary failures and retaining descriptors after rejected close. The file-coordinate contract distinguishes changed/nonregular sources from access/filesystem failures.

Impact and scope

P2: a bounded diagnostic and cleanup-ownership defect during executable file-offset verification. Under controlled close failures, correct readable bytes led to misleading source/access errors, and provider cleanup did not reclaim the affected descriptors. Nonregular-source failure precedence was also lost. The test reclaimed all descriptors separately.

The normal-close control and restoration returned offset 8. Natural close-failure frequency and a public recovery workaround for an already-lost descriptor are NOT_ENOUGH_INFO. No data loss, target execution or security-boundary crossing was observed. Database targets return capability-unavailable before this source read.

REA version

rea-agents 6.7.0; source revision 5241597594.

Runtime and platform

Node.js 24.22.0; macOS 27.0.1 arm64; locked Vitest 5.0.3. Hopper responses were simulated; no Hopper engine version was exercised.

Target and artifact context

Inert 64-byte thin Mach-O fixture with a matching SHA-256 and an explicit thin-container analysis profile. The nonregular-source case replaced the fixture with a directory after the normal-close controls. Neither input was executed.

Interface

Other

Earliest failure stage

Cleanup, cancellation, or lifecycle

Relevant output or evidence

Observed focused-test output:

{"code":"EIO","error":"BinaryTargetError","readEnded":2,"cleanupIncomplete":false,"descriptorOpenAfterClientClose":true}
{"code":"EPERM","error":"AnalysisAccessDeniedError","readEnded":3,"cleanupIncomplete":false,"descriptorOpenAfterClientClose":true}
control and restored close: verified file_offset=8
nonregular-source control: AnalysisArtifactChangedError; close failure replaces it with BinaryTargetError

readEnded counts completed source streams, including the initial successful control. Finalizers restored the mocked boundary, closed every captured real handle and checked fd === -1.

The snapshot helper awaits raw close in finally. The outer catch classifies that rejection as source verification/access failure. The provider operation calls this helper after receiving a successful native mapping. Public CLI/MCP reachability is source-derived; executed coverage ends at this provider boundary.

Typed outcome or recovery expected

A typed cleanup failure preserving the available native mapping and completed read facts, or the original artifact_changed failure for a nonregular source, with retryable ownership of the exact failed-close descriptor. Session close must retry or honestly report remaining cleanup instead of succeeding while losing that owner.

Analysis or suspected cause

The helper opens a raw FileHandle without an owning resource scope. A rejection in finally replaces its pending Result; the outer catch cannot distinguish failure to read from failure to close after reading. Its returned error has no cleanup owner, and provider client close therefore has nothing to retry.

The target digest comparison and final coordinate mapping run after the snapshot helper returns. This report does not claim the failing calls had already produced verified coordinates; it establishes completed reads, lost primary failure precedence, wrong failure classification and lost cleanup ownership.

Additional context

Current upstream source was checked at 6d356b8434; the affected helper and provider mapping were unchanged.

Related #5688 concerns HopperMachOImage preflight, and #4266/#4598 concern StableFileHash. This mapper uses its own snapshotFromPath implementation, so fixes confined to those producers do not repair it. #5841 concerns publisher retry sequencing; #5832, #5919, #5920, #5922 and #5986 concern other admission, installation or regex diagnostics.

Originally created by @coygeek on GitHub (Oct 21, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/6030 ### Before submitting - [x] I searched the open and closed issues for a related report. - [x] I removed credentials, tokens, private paths, and sensitive target data. ### Area Native analysis or decompilation ### Observed behavior A focused production-provider test reproduced two consequences of a rejected source-descriptor close during `address_to_file_offset`. After a complete read, injected close `EIO` became `BinaryTargetError`, and injected close `EPERM` became `AnalysisAccessDeniedError`, both with `cleanupIncomplete=false`. The exact descriptor remained open after the provider client's `close()` returned success. With normal closes, the same input returned file offset 8. A nonregular-source control returned `AnalysisArtifactChangedError`; adding the close failure replaced that earlier source verdict with `BinaryTargetError`. The test used real file handles and the production mapper, with a simulated successful bridge response and controlled close failures. No real Hopper installation, CLI command or MCP transport was exercised. ### Reproduction Save the following as `src/hopper/fileOffsetCloseReproduction.test.ts` in the baseline checkout with locked dependencies installed. Set `REA_HOPPER_TEST_ROOT` to a disposable writable directory and run: ```sh node node_modules/vitest/vitest.mjs run src/hopper/fileOffsetCloseReproduction.test.ts --maxWorkers 1 ``` The assertions characterize the current defect; passing confirms the incorrect classification and retained open descriptors. Remove the temporary test after running. This focused case passed with the output below. ```ts import { createHash } from 'node:crypto'; import { open, writeFile, mkdir, mkdtemp, rm, type FileHandle } from 'node:fs/promises'; import { join } from 'node:path'; import { expect, it, vi } from 'vitest'; import { HopperProvider } from './HopperProvider.js'; import { HopperClient } from './HopperClient.js'; import { parseConfig } from '../config/parseConfig.js'; import { createAnalysisProfile } from '../domain/analysisProfile.js'; import { silentLogger } from '../logger.js'; import { ok } from '../domain/result.js'; const state = vi.hoisted(() => ({path:'', fail:false, code:'EIO', handles:[] as Array<{handle:FileHandle, close:()=>Promise<void>}>, ended:0})); vi.mock('node:fs/promises', async (importOriginal) => { const actual = await importOriginal<typeof import('node:fs/promises')>(); return {...actual, open: async (...args: Parameters<typeof actual.open>) => { const handle = await actual.open(...args); if (String(args[0]) === state.path) { const close = handle.close.bind(handle); state.handles.push({handle,close}); const stream = handle.createReadStream.bind(handle); handle.createReadStream = (options) => stream(options).on('end',()=>{state.ended++;}); handle.close = async () => { if(state.fail) throw Object.assign(new Error('controlled descriptor close failure'),{code:state.code}); await close(); }; } return handle; }}; }); const root = await mkdtemp(join(process.env.REA_HOPPER_TEST_ROOT!, 'runtime-')); const observation = {address:'0x1000',provider_file_offset:8,provider_source_path:null,provider_image_header_hex:null}; it('provider offset read succeeds, but failed close is projected as source failure and not retried by close', async () => { await mkdir(root,{recursive:true}); const bytes = Buffer.alloc(64); bytes.writeUInt32LE(0xfeedfacf,0);bytes.writeUInt32LE(0x0100000c,4); state.path = join(root,'sample'); await writeFile(state.path,bytes); const config = parseConfig({HOPPER_LAUNCHER_PATH:process.execPath}); if(!config.ok) throw config.error; const provider = new HopperProvider(config.value,silentLogger,{}); const client = provider.createClient({path:state.path,sha256:createHash('sha256').update(bytes).digest('hex'),kind:'executable',format:'mach-o',architecture:'arm64',availableArchitectures:['arm64']},createAnalysisProfile({id:'hopper',name:'Hopper',version:'fixture'},{macho_container:'thin'})); const call = vi.spyOn(HopperClient.prototype,'callTool').mockResolvedValue(ok(observation)); try { const control = await client.execute('address_to_file_offset',{address:'0x1000'}); expect(control.ok).toBe(true); if(control.ok) expect(JSON.stringify(control.value)).toContain('"file_offset":8'); for(const code of ['EIO','EPERM']) { state.code=code;state.fail=true; const result=await client.execute('address_to_file_offset',{address:'0x1000'}); expect(result.ok).toBe(false); if(result.ok) throw new Error('Expected failure'); expect(result.error._tag).toBe(code==='EIO'?'BinaryTargetError':'AnalysisAccessDeniedError'); expect(result.error.cleanupIncomplete).toBe(false); const owned=state.handles.at(-1)!; expect(owned.handle.fd).toBeGreaterThanOrEqual(0); expect((await client.close()).ok).toBe(true); expect(owned.handle.fd).toBeGreaterThanOrEqual(0); console.log(JSON.stringify({code,error:result.error._tag,readEnded:state.ended,cleanupIncomplete:result.error.cleanupIncomplete,descriptorOpenAfterClientClose:true})); } state.fail=false; const restored=await client.execute('address_to_file_offset',{address:'0x1000'}); expect(restored.ok).toBe(true); expect(state.ended).toBe(4); console.log('control and restored close: verified file_offset=8'); await rm(state.path);await mkdir(state.path); const nonregular = await client.execute('address_to_file_offset',{address:'0x1000'}); expect(nonregular).toMatchObject({ok:false,error:{_tag:'AnalysisArtifactChangedError'}}); state.fail=true;state.code='EIO'; const masked = await client.execute('address_to_file_offset',{address:'0x1000'}); expect(masked).toMatchObject({ok:false,error:{_tag:'BinaryTargetError',cleanupIncomplete:false}}); console.log('nonregular-source control: AnalysisArtifactChangedError; close failure replaces it with BinaryTargetError'); } finally { call.mockRestore();state.fail=false; for(const entry of state.handles) {await entry.close();expect(entry.handle.fd).toBe(-1);} await client.close();await rm(root,{recursive:true,force:true}); } }); ``` ### Expected behavior Keep the completed read/hash snapshot or earlier nonregular-source failure while reporting close failure separately as typed cleanup. Retain the exact descriptor owner for cleanup retry; a successful owner retry should clear only that resource's incomplete status. Do not describe failed close as failed source access. Do not return successful coordinates until the normal source-identity checks have run. The [lifecycle rules](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/docs/tool-design.md#L123-L139) require preserving completed observations and primary failures and retaining descriptors after rejected close. The [file-coordinate contract](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/docs/native-investigation.md#L335-L342) distinguishes changed/nonregular sources from access/filesystem failures. ### Impact and scope P2: a bounded diagnostic and cleanup-ownership defect during executable file-offset verification. Under controlled close failures, correct readable bytes led to misleading source/access errors, and provider cleanup did not reclaim the affected descriptors. Nonregular-source failure precedence was also lost. The test reclaimed all descriptors separately. The normal-close control and restoration returned offset 8. Natural close-failure frequency and a public recovery workaround for an already-lost descriptor are NOT_ENOUGH_INFO. No data loss, target execution or security-boundary crossing was observed. Database targets return capability-unavailable before this source read. ### REA version rea-agents 6.7.0; source revision 52415975942a4b9c53a817915b313ccc404f280e. ### Runtime and platform Node.js 24.22.0; macOS 27.0.1 arm64; locked Vitest 5.0.3. Hopper responses were simulated; no Hopper engine version was exercised. ### Target and artifact context Inert 64-byte thin Mach-O fixture with a matching SHA-256 and an explicit thin-container analysis profile. The nonregular-source case replaced the fixture with a directory after the normal-close controls. Neither input was executed. ### Interface Other ### Earliest failure stage Cleanup, cancellation, or lifecycle ### Relevant output or evidence Observed focused-test output: ```text {"code":"EIO","error":"BinaryTargetError","readEnded":2,"cleanupIncomplete":false,"descriptorOpenAfterClientClose":true} {"code":"EPERM","error":"AnalysisAccessDeniedError","readEnded":3,"cleanupIncomplete":false,"descriptorOpenAfterClientClose":true} control and restored close: verified file_offset=8 nonregular-source control: AnalysisArtifactChangedError; close failure replaces it with BinaryTargetError ``` `readEnded` counts completed source streams, including the initial successful control. Finalizers restored the mocked boundary, closed every captured real handle and checked `fd === -1`. The [snapshot helper](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/hopper/HopperFileOffset.ts#L145-L155) awaits raw close in `finally`. The [outer catch](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/hopper/HopperFileOffset.ts#L116-L141) classifies that rejection as source verification/access failure. The [provider operation](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/hopper/HopperProvider.ts#L350-L361) calls this helper after receiving a successful native mapping. Public CLI/MCP reachability is source-derived; executed coverage ends at this provider boundary. ### Typed outcome or recovery expected A typed cleanup failure preserving the available native mapping and completed read facts, or the original `artifact_changed` failure for a nonregular source, with retryable ownership of the exact failed-close descriptor. Session close must retry or honestly report remaining cleanup instead of succeeding while losing that owner. ### Analysis or suspected cause The helper opens a raw `FileHandle` without an owning resource scope. A rejection in `finally` replaces its pending Result; the outer catch cannot distinguish failure to read from failure to close after reading. Its returned error has no cleanup owner, and provider client close therefore has nothing to retry. The target digest comparison and final coordinate mapping run after the snapshot helper returns. This report does not claim the failing calls had already produced verified coordinates; it establishes completed reads, lost primary failure precedence, wrong failure classification and lost cleanup ownership. ### Additional context Current upstream source was checked at 6d356b84344659085f2b912857b5e27fe0160c9b; the affected helper and provider mapping were unchanged. Related #5688 concerns `HopperMachOImage` preflight, and #4266/#4598 concern `StableFileHash`. This mapper uses its own `snapshotFromPath` implementation, so fixes confined to those producers do not repair it. #5841 concerns publisher retry sequencing; #5832, #5919, #5920, #5922 and #5986 concern other admission, installation or regex diagnostics.

bsakaguchi closed this issue

2026-10-31 00:42:59 +00:00

bsakaguchi referenced this issue

2026-10-31 00:44:18 +00:00

[GH-ISSUE #6312] [Bug] Invalid CArchive compression-flag diagnostics omit the TOC entry index #3813

bsakaguchi referenced this issue

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

[GH-ISSUE #6441] [Bug] extract-artifact drops PyInstaller derivation notices and misstates decoded PYZ output #3854

bsakaguchi referenced this issue

2026-10-31 00:47:07 +00:00

[GH-ISSUE #6789] [Bug] PyInstaller extraction prepends a second pyc header to legacy modules #4142

bsakaguchi referenced this issue