morluto/rea

Fork 0

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

[PR #6273] [MERGED] fix(ghidra): reject JDK_JAVA_OPTIONS argument-file platform overrides #7247

New issue

Closed

opened 2026-11-01 21:46:28 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-11-01 21:46:28 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/6273
Author: @mvanhorn
Created: 10/22/2026
Status: ✅ Merged
Merged: 10/22/2026
Merged by: @gensokyo-dev

Base: main ← Head: fix/ghidra-jdk-argfile-overrides

📝 Commits (4)

2051330 fix(ghidra): reject JDK_JAVA_OPTIONS argument-file platform overrides
a972cfd chore: integrate JDK option admission fix with current main
67590c2 fix(ghidra): own and freeze admitted JDK argument files
6228973 fix(ghidra): preserve native argument-file lexer state

📊 Changes

6 files changed (+706 additions, -60 deletions)

View changed files

📝 docs/installation.md (+11 -1)
📝 src/ghidra/GhidraJavaLaunch.ts (+41 -41)
➕ src/ghidra/GhidraJavaOptions.ts (+278 -0)
📝 src/ghidra/GhidraLauncher.ts (+17 -14)
➕ src/ghidra/GhidraRuntimeFile.ts (+16 -0)
📝 tests/boundary/providers/ghidra/ghidraJavaLaunch.test.ts (+343 -4)

📄 Description

Summary

The Ghidra headless launcher already refused literal LaunchSupport -Dos.arch / -Dos.name values in helper option variables, but a JDK_JAVA_OPTIONS=@file token whose file contained the same override reached process creation.

JDK_JAVA_OPTIONS argument files are expanded and inspected with the existing platform-override check. Unreadable files and nested @ references are refused before the analysis JVM starts. Ordinary launches without a platform override are unchanged.

JAVA_TOOL_OPTIONS and _JAVA_OPTIONS stay literal-token checks: the JVM does not expand @file in those variables, so expanding them would inspect options the launched JVM would not apply.

Closes #6212

Changes

Expand bounded JDK_JAVA_OPTIONS @file tokens (64 KiB, no nested @ files, unreadable path is a launch failure).
Apply the same os.arch / os.name admission policy used for literal helper options.
Document the argument-file check in docs/installation.md.

Testing

npm run test:focused -- tests/boundary/providers/ghidra/ghidraJavaLaunch.test.ts: 34 passed (argument file with -Dos.arch rejected before spawn; literal overrides still rejected; argument file without an override admitted; unreadable and nested @ files rejected).
On main, the argument-file -Dos.arch case launched (ok: true) instead of rejecting.
npm run check:fast and npm run docs:check passed.

Ghidra and a real JDK are not required: the tests use the existing LaunchSupport recording helper and a spawn callback that records process-creation attempts.

Demo

Current-main follow-up

Head 62289735f preserves the original contribution and fixes the three actionable review findings at their producing boundary:

Parse argument files once with launcher quote/escape/comment/continuation rules, rather than stripping comments then applying environment quoting. Preserve partial quotes, quoted fragments across comments, EOF continuation and disabled expansion.
Reuse regular-file admission and descriptor ownership: refuse FIFOs, carry caller cancellation through final close, and retain/retry the exact failed-close owner without losing an earlier parse failure.
Freeze validated bytes in the already owned runtime root and use those references in later helpers and POSIX/Windows analysis launch. Repeated reads share one per-launch snapshot, but each expansion counts against the cumulative bound. No cross-launch live cache. Existing private runtime writing now has one shared implementation.

Executed on Linux, Node 24.18.0/npm 11.16.0:

Original-head grammar counterexamples: 2 failed / 34 passed. A later quoted-fragment counterexample also failed before its lexer correction.
Final focused launch/probe/environment/launcher checks: 97 passed, 3 explicit skips. Includes oversize/invalid encoding/FIFO admission, repeated-expansion budget, snapshot mutation, failed-close retry, and exact late cancellation.
npm run check, npm run docs:check, npm run verify:test-discovery, pre-push check:fast: passed.
Two independent local Codex CLI reviews found no definite remaining actionable issue; the final review independently checked all seven retained snapshots against original oracle bytes.
Real installed OpenJDK 17.0.20.1: 12 grammar/property scenarios, 5 effective platform overrides refused and 7 normal snapshot outcomes matched the native launcher despite subsequent source-file mutation. Final run: 1.24 s wall, 1.37 s CPU, 75.6 MiB sampled process-family RSS (500 ms sampling estimate, not a kernel peak). Earlier equivalent 10-case first/repeat runs were 1.09/1.13 s; different case counts are not a speedup comparison.

Limits: no real Ghidra engine, supported-JDK25 runtime or native Windows qualification was available here. --disable-@files coverage uses the OpenJDK25 source contract plus recording integration, not an installed25 claim. No agent-token reduction or analysis speedup is claimed; public tool schemas are unchanged. The inherited 64 KiB admission budget now covers cumulative expansion and prepared environment growth, before unbounded allocation.

_{🔄 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/6273 **Author:** [@mvanhorn](https://github.com/mvanhorn) **Created:** 10/22/2026 **Status:** ✅ Merged **Merged:** 10/22/2026 **Merged by:** [@gensokyo-dev](https://github.com/gensokyo-dev) **Base:** `main` ← **Head:** `fix/ghidra-jdk-argfile-overrides` --- ### 📝 Commits (4) - [`2051330`](https://github.com/morluto/rea/commit/2051330564fc085c58014d0039848e6222e6dd9d) fix(ghidra): reject JDK_JAVA_OPTIONS argument-file platform overrides - [`a972cfd`](https://github.com/morluto/rea/commit/a972cfd7401171a3814c6ca003e66a1637747678) chore: integrate JDK option admission fix with current main - [`67590c2`](https://github.com/morluto/rea/commit/67590c2c54398f3a2d2ce82f6111b2d122d15a8e) fix(ghidra): own and freeze admitted JDK argument files - [`6228973`](https://github.com/morluto/rea/commit/62289735f1a253bb8c5fc73f98c8e98767a6ca20) fix(ghidra): preserve native argument-file lexer state ### 📊 Changes **6 files changed** (+706 additions, -60 deletions) <details> <summary>View changed files</summary> 📝 `docs/installation.md` (+11 -1) 📝 `src/ghidra/GhidraJavaLaunch.ts` (+41 -41) ➕ `src/ghidra/GhidraJavaOptions.ts` (+278 -0) 📝 `src/ghidra/GhidraLauncher.ts` (+17 -14) ➕ `src/ghidra/GhidraRuntimeFile.ts` (+16 -0) 📝 `tests/boundary/providers/ghidra/ghidraJavaLaunch.test.ts` (+343 -4) </details> ### 📄 Description ## Summary The Ghidra headless launcher already refused literal LaunchSupport `-Dos.arch` / `-Dos.name` values in helper option variables, but a `JDK_JAVA_OPTIONS=@file` token whose file contained the same override reached process creation. `JDK_JAVA_OPTIONS` argument files are expanded and inspected with the existing platform-override check. Unreadable files and nested `@` references are refused before the analysis JVM starts. Ordinary launches without a platform override are unchanged. `JAVA_TOOL_OPTIONS` and `_JAVA_OPTIONS` stay literal-token checks: the JVM does not expand `@file` in those variables, so expanding them would inspect options the launched JVM would not apply. Closes #6212 ## Changes - Expand bounded `JDK_JAVA_OPTIONS` `@file` tokens (64 KiB, no nested `@` files, unreadable path is a launch failure). - Apply the same `os.arch` / `os.name` admission policy used for literal helper options. - Document the argument-file check in `docs/installation.md`. ## Testing - `npm run test:focused -- tests/boundary/providers/ghidra/ghidraJavaLaunch.test.ts`: 34 passed (argument file with `-Dos.arch` rejected before spawn; literal overrides still rejected; argument file without an override admitted; unreadable and nested `@` files rejected). - On `main`, the argument-file `-Dos.arch` case launched (`ok: true`) instead of rejecting. - `npm run check:fast` and `npm run docs:check` passed. Ghidra and a real JDK are not required: the tests use the existing LaunchSupport recording helper and a spawn callback that records process-creation attempts. ## Demo ![demo](https://raw.githubusercontent.com/mvanhorn/pr-demo-assets/main/rea/ghidra-jdk-argfile-overrides/demo.gif) <video src="https://raw.githubusercontent.com/mvanhorn/pr-demo-assets/main/rea/ghidra-jdk-argfile-overrides/demo.mp4" controls width="720"></video> ## Current-main follow-up Head `62289735f` preserves the original contribution and fixes the three actionable review findings at their producing boundary: - Parse argument files once with launcher quote/escape/comment/continuation rules, rather than stripping comments then applying environment quoting. Preserve partial quotes, quoted fragments across comments, EOF continuation and disabled expansion. - Reuse regular-file admission and descriptor ownership: refuse FIFOs, carry caller cancellation through final close, and retain/retry the exact failed-close owner without losing an earlier parse failure. - Freeze validated bytes in the already owned runtime root and use those references in later helpers and POSIX/Windows analysis launch. Repeated reads share one per-launch snapshot, but each expansion counts against the cumulative bound. No cross-launch live cache. Existing private runtime writing now has one shared implementation. Executed on Linux, Node 24.18.0/npm 11.16.0: - Original-head grammar counterexamples: 2 failed / 34 passed. A later quoted-fragment counterexample also failed before its lexer correction. - Final focused launch/probe/environment/launcher checks: **97 passed, 3 explicit skips**. Includes oversize/invalid encoding/FIFO admission, repeated-expansion budget, snapshot mutation, failed-close retry, and exact late cancellation. - `npm run check`, `npm run docs:check`, `npm run verify:test-discovery`, pre-push `check:fast`: passed. - Two independent local Codex CLI reviews found no definite remaining actionable issue; the final review independently checked all seven retained snapshots against original oracle bytes. - Real installed OpenJDK 17.0.20.1: **12 grammar/property scenarios**, 5 effective platform overrides refused and 7 normal snapshot outcomes matched the native launcher despite subsequent source-file mutation. Final run: 1.24 s wall, 1.37 s CPU, 75.6 MiB sampled process-family RSS (500 ms sampling estimate, not a kernel peak). Earlier equivalent 10-case first/repeat runs were 1.09/1.13 s; different case counts are not a speedup comparison. Limits: no real Ghidra engine, supported-JDK25 runtime or native Windows qualification was available here. `--disable-@files` coverage uses the OpenJDK25 source contract plus recording integration, not an installed25 claim. No agent-token reduction or analysis speedup is claimed; public tool schemas are unchanged. The inherited 64 KiB admission budget now covers cumulative expansion and prepared environment growth, before unbounded allocation. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-11-01 21:46:28 +00:00

closed this issue
added the
pull-request
label

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#7247

No description provided.

Rows
Columns