[PR #3230] [CLOSED] fix: remediate unhandled I/O, validation, and lifecycle defects #5659

New issue

Closed

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

bsakaguchi commented

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

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/3230
Author: @qorvr
Created: 10/14/2026
Status: ❌ Closed

Base: main ← Head: fix/subq-audit-remediations

📝 Commits (8)

e68b73c fix(main): safely guard realpathSync during direct entrypoint check
8dffa2d test(main): test undefined entrypoint safely and guard parameter fallback
d23f893 fix(electron): attach error listener to spawned node process in readiness fixture
51bcaa6 fix: remediate unhandled I/O, validation, and lifecycle defects
dd23ba9 fix(identity,capture,evaluation): guard process.argv fallbacks, safe request reads, and fixture hashes
8ad8119 fix(capture,windows): preserve packet capture diagnostics and retain failed native process handles
aa47fdd Merge branch 'upstream/main' into fix/subq-audit-remediations
068fac6 fix(domain): pipe evidence schema validation before semantic ID authentication

📊 Changes

18 files changed (+233 additions, -46 deletions)

View changed files

📝 src/application/ClientRegistrationIdentity.ts (+10 -2)
📝 src/browser/history/NodeCaptureProcess.ts (+24 -4)
📝 src/captures/parsePacketOutput.ts (+9 -1)
📝 src/domain/evidence.ts (+13 -13)
📝 src/domain/propertyListXmlText.ts (+4 -2)
📝 src/evaluation/AgentEvaluationFixtures.ts (+14 -2)
📝 src/evm/EvmoleInterfaceWorker.ts (+22 -8)
📝 src/main/transport.ts (+8 -2)
📝 src/process/MacCodeSignature.ts (+10 -3)
📝 src/serverIdentity.ts (+4 -1)
📝 src/sqlite/SqliteDatabaseWorker.ts (+17 -3)
📝 src/windows/WindowsOwnedProcess.test.ts (+46 -2)
📝 src/windows/WindowsOwnedProcess.ts (+2 -2)
📝 tests/boundary/captures/captureProvider.test.ts (+4 -0)
📝 tests/boundary/captures/packetOutput.test.ts (+6 -0)
📝 tests/boundary/filesystem/appleXmlDeclarations.test.ts (+12 -0)
➕ tests/boundary/process/macCodeSignature.test.ts (+26 -0)
📝 tests/conformance/readiness/electron/main.js (+2 -1)

📄 Description

Summary

This pull request provides comprehensive defect remediations across core I/O, validation, and process lifecycle boundaries identified during deep code audits:

src/captures/parsePacketOutput.ts:
- Safely wraps JSON.parse in try/catch to convert raw SyntaxError failures into typed AnalysisOutputError when TShark returns invalid or truncated output.
src/domain/propertyListXmlText.ts:
- Adds input validation ensuring bytes is an instance of Uint8Array.
- Adds buffer length guard before checking UTF-16 BOM prefixes to prevent unexpected out-of-bounds indexing on short or empty buffers.
- Adds unit test coverage in tests/boundary/filesystem/appleXmlDeclarations.test.ts.
src/evm/EvmoleInterfaceWorker.ts:
- Shields worker request reading and parsing within explicit try/catch blocks with informative error diagnostics.
- Refactors nested package.json read and JSON parsing into distinct stages.
src/sqlite/SqliteDatabaseWorker.ts:
- Wraps request file read and JSON parsing inside a try/catch boundary to catch missing or malformed request payloads gracefully.
src/browser/history/NodeCaptureProcess.ts:
- Adds presence guards for process.argv arguments and wraps request reading/JSON parsing inside explicit error handling boundaries.
src/main/transport.ts:
- Ensures optionalProviders (including browserScenarioCapture) are closed via Promise.allSettled when dependencies.serve fails during startup, preventing dangling provider handles.
src/process/MacCodeSignature.ts:
- Adds strict format validation (/^[\w.-]+$/) on identifier and teamIdentifier before embedding into Apple codesign requirement expressions to prevent requirement language injection.
- Adds unit tests in tests/boundary/process/macCodeSignature.test.ts.
src/application/ClientRegistrationIdentity.ts:
- Refactors isOwnedClientRegistrationCommand to use rest parameters, eliminating the default parameter fallback trap that triggers host runner process.argv[1] evaluations when undefined is provided.
src/serverIdentity.ts:
- Safely guards process.argv[1] existence before resolving paths to prevent unexpected resolution fallbacks.
src/evaluation/AgentEvaluationFixtures.ts:
- Wraps fixture package file reads inside a try/catch boundary with explicit failure diagnostics.
src/windows/WindowsOwnedProcess.ts:
- Hardens #release() with try/finally so stream cleanup, #closed = true, and settlement resolution always execute even if the native process_close call throws.

Evaluated and error found using SubQ, fixed by Qorvr. Request submitted by Automation.

_{🔄 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/3230 **Author:** [@qorvr](https://github.com/qorvr) **Created:** 10/14/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/subq-audit-remediations` --- ### 📝 Commits (8) - [`e68b73c`](https://github.com/morluto/rea/commit/e68b73c85cee2c89eca080d6d0e8b9806423b79c) fix(main): safely guard realpathSync during direct entrypoint check - [`8dffa2d`](https://github.com/morluto/rea/commit/8dffa2d740390020cdbe3d71dcc816ca2acc1323) test(main): test undefined entrypoint safely and guard parameter fallback - [`d23f893`](https://github.com/morluto/rea/commit/d23f893eaff0858741bfd851fc7a96b952bb275d) fix(electron): attach error listener to spawned node process in readiness fixture - [`51bcaa6`](https://github.com/morluto/rea/commit/51bcaa69c3f0a18561dfdeffd9c41ddb4cc65fd5) fix: remediate unhandled I/O, validation, and lifecycle defects - [`dd23ba9`](https://github.com/morluto/rea/commit/dd23ba9c9501adec6f7fa452ee20f4eeec4c5a86) fix(identity,capture,evaluation): guard process.argv fallbacks, safe request reads, and fixture hashes - [`8ad8119`](https://github.com/morluto/rea/commit/8ad8119d920c2df146eb45fd7020ee25501bce91) fix(capture,windows): preserve packet capture diagnostics and retain failed native process handles - [`aa47fdd`](https://github.com/morluto/rea/commit/aa47fddb352be1ecdb15f9f2fd10653cf11ceb9e) Merge branch 'upstream/main' into fix/subq-audit-remediations - [`068fac6`](https://github.com/morluto/rea/commit/068fac6e8d629625ed003217edc95fd1662d50c4) fix(domain): pipe evidence schema validation before semantic ID authentication ### 📊 Changes **18 files changed** (+233 additions, -46 deletions) <details> <summary>View changed files</summary> 📝 `src/application/ClientRegistrationIdentity.ts` (+10 -2) 📝 `src/browser/history/NodeCaptureProcess.ts` (+24 -4) 📝 `src/captures/parsePacketOutput.ts` (+9 -1) 📝 `src/domain/evidence.ts` (+13 -13) 📝 `src/domain/propertyListXmlText.ts` (+4 -2) 📝 `src/evaluation/AgentEvaluationFixtures.ts` (+14 -2) 📝 `src/evm/EvmoleInterfaceWorker.ts` (+22 -8) 📝 `src/main/transport.ts` (+8 -2) 📝 `src/process/MacCodeSignature.ts` (+10 -3) 📝 `src/serverIdentity.ts` (+4 -1) 📝 `src/sqlite/SqliteDatabaseWorker.ts` (+17 -3) 📝 `src/windows/WindowsOwnedProcess.test.ts` (+46 -2) 📝 `src/windows/WindowsOwnedProcess.ts` (+2 -2) 📝 `tests/boundary/captures/captureProvider.test.ts` (+4 -0) 📝 `tests/boundary/captures/packetOutput.test.ts` (+6 -0) 📝 `tests/boundary/filesystem/appleXmlDeclarations.test.ts` (+12 -0) ➕ `tests/boundary/process/macCodeSignature.test.ts` (+26 -0) 📝 `tests/conformance/readiness/electron/main.js` (+2 -1) </details> ### 📄 Description ## Summary This pull request provides comprehensive defect remediations across core I/O, validation, and process lifecycle boundaries identified during deep code audits: 1. **`src/captures/parsePacketOutput.ts`**: - Safely wraps `JSON.parse` in `try/catch` to convert raw `SyntaxError` failures into typed `AnalysisOutputError` when TShark returns invalid or truncated output. 2. **`src/domain/propertyListXmlText.ts`**: - Adds input validation ensuring `bytes` is an instance of `Uint8Array`. - Adds buffer length guard before checking UTF-16 BOM prefixes to prevent unexpected out-of-bounds indexing on short or empty buffers. - Adds unit test coverage in `tests/boundary/filesystem/appleXmlDeclarations.test.ts`. 3. **`src/evm/EvmoleInterfaceWorker.ts`**: - Shields worker request reading and parsing within explicit `try/catch` blocks with informative error diagnostics. - Refactors nested `package.json` read and JSON parsing into distinct stages. 4. **`src/sqlite/SqliteDatabaseWorker.ts`**: - Wraps request file read and JSON parsing inside a `try/catch` boundary to catch missing or malformed request payloads gracefully. 5. **`src/browser/history/NodeCaptureProcess.ts`**: - Adds presence guards for `process.argv` arguments and wraps request reading/JSON parsing inside explicit error handling boundaries. 6. **`src/main/transport.ts`**: - Ensures `optionalProviders` (including `browserScenarioCapture`) are closed via `Promise.allSettled` when `dependencies.serve` fails during startup, preventing dangling provider handles. 7. **`src/process/MacCodeSignature.ts`**: - Adds strict format validation (`/^[\w.-]+$/`) on `identifier` and `teamIdentifier` before embedding into Apple codesign requirement expressions to prevent requirement language injection. - Adds unit tests in `tests/boundary/process/macCodeSignature.test.ts`. 8. **`src/application/ClientRegistrationIdentity.ts`**: - Refactors `isOwnedClientRegistrationCommand` to use rest parameters, eliminating the default parameter fallback trap that triggers host runner `process.argv[1]` evaluations when `undefined` is provided. 9. **`src/serverIdentity.ts`**: - Safely guards `process.argv[1]` existence before resolving paths to prevent unexpected resolution fallbacks. 10. **`src/evaluation/AgentEvaluationFixtures.ts`**: - Wraps fixture package file reads inside a `try/catch` boundary with explicit failure diagnostics. 11. **`src/windows/WindowsOwnedProcess.ts`**: - Hardens `#release()` with `try/finally` so stream cleanup, `#closed = true`, and settlement resolution always execute even if the native `process_close` call throws. --- Evaluated and error found using SubQ, fixed by Qorvr. Request submitted by Automation. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-31 01:08:20 +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#5659

No description provided.

Rows
Columns