morluto/rea

Fork 0

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

[GH-ISSUE #4672] [Bug] Ghidra installation properties bypass bounded regular-file admission #2432

New issue

Closed

opened 2026-10-17 21:04:35 +00:00 by bsakaguchi · 3 comments

bsakaguchi commented

2026-10-17 21:04:35 +00:00

Owner

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

Before submitting

Searched open and closed Ghidra FIFO reports; #4508 concerns a different TCP endpoint file boundary.
Removed credentials and sensitive target data.

Area

Provider or capability discovery

Observed behavior

Source-only finding: systemGhidraInstallationHost.readText reads the selected Ghidra/application.properties with readFileSync(path), without nonblocking descriptor admission, regular-file validation, or a byte bound. installationCoordinates calls it before launching the owned Java prerequisite.

Reproduction

NOT_ENOUGH_INFO: no pipe hang or large-file memory experiment executed. Inspected the producing filesystem boundary on e567033a71 and the pending #4561 migration; the synchronous metadata read remains in both.

Expected behavior

Admit and read installation metadata through the existing owned, bounded file boundary. A special file, changed descriptor identity, read failure or failed close should produce an attributed outcome and retain exact cleanup ownership. A child-process deadline cannot bound an earlier synchronous filesystem read.

Impact and scope

Source-derived possibility of blocked provider discovery/doctor cancellation when the selected properties path is a pipe without a writer, and unbounded retained bytes for an oversized file. Actual affected users, frequency and runtime severity: NOT_ENOUGH_INFO; not measured. No target execution involved.

REA version

6.5.0 source, e567033a71; persists in pending #4561 integration.

Runtime and platform

Source review on Node24.18.0/Darwin arm64. POSIX pipe behavior is an inferred failure mode; no Windows/Linux runtime claim.

Target and artifact context

Caller-selected Ghidra installation metadata; no binary target.

Interface

CLI/MCP provider discovery, source-derived

Earliest failure stage

Installation metadata read before Java probe

Relevant output or evidence

src/ghidra/GhidraInstallation.ts, systemGhidraInstallationHost.readText: readFileSync(path).toString("latin1"); installationCoordinates reads applicationPropertiesPath through that port. The neighboring executable-header reader already uses O_NONBLOCK, but the properties producer does not.

Typed outcome or recovery expected

Explicit unavailable/malformed/special-file or file-operation outcome with producer facts and exact cleanup retry where needed; preserve the Java Properties Latin-1 semantics.

Analysis or suspected cause

Source-derived: a synchronous unbounded path read sits outside the Java subprocess owner. Proposed checks: private FIFO and oversize metadata fixtures, cancellation, replacement/identity races, denied close and exact retry through doctor/discovery. These checks have not run for this finding.

Additional context

Related #4508 and #4227. #4561 fixes the distinct synchronous Java subprocess boundary; the installation file-port migration should be handled separately and remains audit work.

Originally created by @morluto on GitHub (Oct 17, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/4672 ### Before submitting - [x] Searched open and closed Ghidra FIFO reports; #4508 concerns a different TCP endpoint file boundary. - [x] Removed credentials and sensitive target data. ### Area Provider or capability discovery ### Observed behavior Source-only finding: systemGhidraInstallationHost.readText reads the selected Ghidra/application.properties with readFileSync(path), without nonblocking descriptor admission, regular-file validation, or a byte bound. installationCoordinates calls it before launching the owned Java prerequisite. ### Reproduction NOT_ENOUGH_INFO: no pipe hang or large-file memory experiment executed. Inspected the producing filesystem boundary on e567033a7137d5eebf49147ed325cad141cff5c2 and the pending #4561 migration; the synchronous metadata read remains in both. ### Expected behavior Admit and read installation metadata through the existing owned, bounded file boundary. A special file, changed descriptor identity, read failure or failed close should produce an attributed outcome and retain exact cleanup ownership. A child-process deadline cannot bound an earlier synchronous filesystem read. ### Impact and scope Source-derived possibility of blocked provider discovery/doctor cancellation when the selected properties path is a pipe without a writer, and unbounded retained bytes for an oversized file. Actual affected users, frequency and runtime severity: NOT_ENOUGH_INFO; not measured. No target execution involved. ### REA version 6.5.0 source, e567033a7137d5eebf49147ed325cad141cff5c2; persists in pending #4561 integration. ### Runtime and platform Source review on Node24.18.0/Darwin arm64. POSIX pipe behavior is an inferred failure mode; no Windows/Linux runtime claim. ### Target and artifact context Caller-selected Ghidra installation metadata; no binary target. ### Interface CLI/MCP provider discovery, source-derived ### Earliest failure stage Installation metadata read before Java probe ### Relevant output or evidence src/ghidra/GhidraInstallation.ts, systemGhidraInstallationHost.readText: `readFileSync(path).toString("latin1")`; installationCoordinates reads applicationPropertiesPath through that port. The neighboring executable-header reader already uses O_NONBLOCK, but the properties producer does not. ### Typed outcome or recovery expected Explicit unavailable/malformed/special-file or file-operation outcome with producer facts and exact cleanup retry where needed; preserve the Java Properties Latin-1 semantics. ### Analysis or suspected cause Source-derived: a synchronous unbounded path read sits outside the Java subprocess owner. Proposed checks: private FIFO and oversize metadata fixtures, cancellation, replacement/identity races, denied close and exact retry through doctor/discovery. These checks have not run for this finding. ### Additional context Related #4508 and #4227. #4561 fixes the distinct synchronous Java subprocess boundary; the installation file-port migration should be handled separately and remains audit work.

bsakaguchi added the

triage

bug

labels

2026-10-17 21:04:35 +00:00

bsakaguchi commented

2026-10-17 21:04:36 +00:00

Author

Owner

@morluto commented on GitHub (Oct 17, 2026):

Published on main in 5112fe07ae.

Replaced the synchronous unbounded properties read with owned regular-file admission and a 1 MiB metadata payload budget (a resource policy, not a Java Properties format limit). Reads preserve Latin-1 and selected symlinks, verify descriptor/name identity and extent, and retain exact failed-close ownership. Missing-at-open is distinct from disappearance or descriptor read failure; physical error codes/syscalls and completed metadata observations remain explicit. Cleanup retry closes the retained descriptor even after its pathname has been replaced, without reopening the replacement.

Extracted the stable bounded byte reader for profile artifacts and consolidated Ghidra file failure projection; removed the old read loop and helper export. Permission, identity-change and resource refusals use typed outcomes attributed to the consuming operation. Profile capacity refusals now carry observed limits instead of a generic provider error.

Runtime reproduction against cc2e18a26: a private FIFO without a writer blocks both direct reading and public doctor; each required the external two-second watchdog and verified cleanup at about 2.2s. Candidate direct reading and public doctor returned independently in about 0.40s/0.57s. The direct timer fired only on the candidate. All four fixture process groups were verified empty and the private fixture directory removed. No Java or Ghidra process launched in these FIFO controls.

Executed 384 focused tests across 14 files, 42 seed/extension source controls, and 32 final metadata/profile/CLI controls after structured filesystem facts were strengthened. check:fast, docs:check (five conformance controls), normal commit/pre-push hooks passed. Coverage includes actual FIFO and permission refusal, Latin-1/symlinks, oversized sparse files rejected before any payload read, growth/shrink/replacement/disappearance, pre-aborted opaque reasons, read failures, cleanup-time cancellation and exact retry. Existing regular-file, PSP, CLI and MCP regressions passed.

Completed-task comparison with cc2e18a26: same real private files and successful provider facts; public doctor uses a controlled selected idle Node Java-shaped executable, not a JVM. Two alternating warm filesystem repeats per build plus separate first processes:

Task	Elapsed baseline/candidate	CPU baseline/candidate	Sampled peak family RSS	Raw reply tokens
Metadata read	0.061/0.060s	0.051/0.053s	62.0/61.9MB	30/30
Profile read	0.060/0.060s	0.042/0.040s	60.3/60.5MB	30/30
Public doctor	1.211/1.230s	1.239/1.247s	523.5/524.3MB	3116/3116

No performance improvement claimed. CPU is wait4 root plus reaped children; RSS is sampled PPID-family memory and may miss transient peaks. Tokens are cl100k_base on complete raw replies. True cold caches, whole-agent, MCP/catalog/retry, cancellation-path and real-engine costs remain unmeasured. Measured source hashes match published files.

Scope: Node 24.18.0/Darwin arm64 controlled files/processes and the specified public CLI/MCP workflows. No new real JVM/Ghidra or Windows/Linux lifecycle verification. Other installation observations, including the separate executable-header port, remain outside this properties-reader repair and remain audit work. The full codebase audit stays active; heavy Qodo/profiler workloads stayed stopped.

@morluto commented on GitHub (Oct 17, 2026): Published on main in 5112fe07ae44cc99cb47641d50279efb5595ea71. Replaced the synchronous unbounded properties read with owned regular-file admission and a 1 MiB metadata payload budget (a resource policy, not a Java Properties format limit). Reads preserve Latin-1 and selected symlinks, verify descriptor/name identity and extent, and retain exact failed-close ownership. Missing-at-open is distinct from disappearance or descriptor read failure; physical error codes/syscalls and completed metadata observations remain explicit. Cleanup retry closes the retained descriptor even after its pathname has been replaced, without reopening the replacement. Extracted the stable bounded byte reader for profile artifacts and consolidated Ghidra file failure projection; removed the old read loop and helper export. Permission, identity-change and resource refusals use typed outcomes attributed to the consuming operation. Profile capacity refusals now carry observed limits instead of a generic provider error. Runtime reproduction against cc2e18a26: a private FIFO without a writer blocks both direct reading and public doctor; each required the external two-second watchdog and verified cleanup at about 2.2s. Candidate direct reading and public doctor returned independently in about 0.40s/0.57s. The direct timer fired only on the candidate. All four fixture process groups were verified empty and the private fixture directory removed. No Java or Ghidra process launched in these FIFO controls. Executed 384 focused tests across 14 files, 42 seed/extension source controls, and 32 final metadata/profile/CLI controls after structured filesystem facts were strengthened. check:fast, docs:check (five conformance controls), normal commit/pre-push hooks passed. Coverage includes actual FIFO and permission refusal, Latin-1/symlinks, oversized sparse files rejected before any payload read, growth/shrink/replacement/disappearance, pre-aborted opaque reasons, read failures, cleanup-time cancellation and exact retry. Existing regular-file, PSP, CLI and MCP regressions passed. Completed-task comparison with cc2e18a26: same real private files and successful provider facts; public doctor uses a controlled selected idle Node Java-shaped executable, not a JVM. Two alternating warm filesystem repeats per build plus separate first processes: | Task | Elapsed baseline/candidate | CPU baseline/candidate | Sampled peak family RSS | Raw reply tokens | | --- | --- | --- | --- | --- | | Metadata read | 0.061/0.060s | 0.051/0.053s | 62.0/61.9MB | 30/30 | | Profile read | 0.060/0.060s | 0.042/0.040s | 60.3/60.5MB | 30/30 | | Public doctor | 1.211/1.230s | 1.239/1.247s | 523.5/524.3MB | 3116/3116 | No performance improvement claimed. CPU is wait4 root plus reaped children; RSS is sampled PPID-family memory and may miss transient peaks. Tokens are cl100k_base on complete raw replies. True cold caches, whole-agent, MCP/catalog/retry, cancellation-path and real-engine costs remain unmeasured. Measured source hashes match published files. Scope: Node 24.18.0/Darwin arm64 controlled files/processes and the specified public CLI/MCP workflows. No new real JVM/Ghidra or Windows/Linux lifecycle verification. Other installation observations, including the separate executable-header port, remain outside this properties-reader repair and remain audit work. The full codebase audit stays active; heavy Qodo/profiler workloads stayed stopped.

bsakaguchi closed this issue

2026-10-31 00:33:50 +00:00

bsakaguchi commented

2026-10-31 00:33:52 +00:00

Author

Owner

@XinwangTao commented on GitHub (Oct 18, 2026):

Agree with this finding: installation metadata is the first file the process touches, so it should go through the same owned, bounded admission as every other target read. readFileSync without descriptor validation and a byte bound is exactly the class of boundary that the #4561 migration exists to close, and the 1 MiB metadata payload budget in the fix commit is a sensible resource policy (it is a bound on admission, not a format limitation). Also good that the follow-up keeps the sync read path covered - installation happens before async plumbing is available.

@XinwangTao commented on GitHub (Oct 18, 2026): Agree with this finding: installation metadata is the first file the process touches, so it should go through the same owned, bounded admission as every other target read. readFileSync without descriptor validation and a byte bound is exactly the class of boundary that the #4561 migration exists to close, and the 1 MiB metadata payload budget in the fix commit is a sensible resource policy (it is a bound on admission, not a format limitation). Also good that the follow-up keeps the sync read path covered - installation happens before async plumbing is available.

bsakaguchi commented

2026-10-31 00:33:52 +00:00

Author

Owner

@morluto commented on GitHub (Oct 18, 2026):

Closing as fixed on main after reconciling this report with the landed owner/consumer changes.

5112fe07a: fix(ghidra): admit installation metadata through owned bounded reads. Reuse the stable bounded reader for profile artifacts and consolidate Ghidra file failure projection. Distinguish missing-at-open from disappearance or read failure, and attribute typed prerequisite failures at the consuming operation.
cc2e18a26: fix(ghidra): own Java prerequisite probes through final cleanup. Preserve completed physical failures across late cancellation, exact cleanup dependencies and partial observations. Admit target/profile inputs before probing and return typed factory failures instead of unavailable client wrappers.

Evidence scope: this closure uses the landed source, regression coverage and validation already recorded in the linked commits/thread. No new local tests or native-provider reproduction were run during this backlog pass; previously documented platform and real-provider gaps still apply.

@morluto commented on GitHub (Oct 18, 2026):  Closing as fixed on `main` after reconciling this report with the landed owner/consumer changes. - [5112fe07a](https://github.com/morluto/rea/commit/5112fe07ae44cc99cb47641d50279efb5595ea71): fix(ghidra): admit installation metadata through owned bounded reads. Reuse the stable bounded reader for profile artifacts and consolidate Ghidra file failure projection. Distinguish missing-at-open from disappearance or read failure, and attribute typed prerequisite failures at the consuming operation. - [cc2e18a26](https://github.com/morluto/rea/commit/cc2e18a265198492dff1587a343bfb802d8c591b): fix(ghidra): own Java prerequisite probes through final cleanup. Preserve completed physical failures across late cancellation, exact cleanup dependencies and partial observations. Admit target/profile inputs before probing and return typed factory failures instead of unavailable client wrappers. Evidence scope: this closure uses the landed source, regression coverage and validation already recorded in the linked commits/thread. No new local tests or native-provider reproduction were run during this backlog pass; previously documented platform and real-provider gaps still apply.

bsakaguchi referenced this issue

2026-10-31 01:04:57 +00:00

[PR #2432] [MERGED] feat(website): add a persistent light and dark theme switch #5225

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

morluto/rea#2432

No description provided.

Rows
Columns