morluto/rea

Fork 0

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

[PR #4405] [MERGED] fix(objc): validate direct protocol enumeration input #6216

New issue

Closed

opened 2026-10-31 01:12:46 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 01:12:46 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/4405
Author: @lyly-bonbon
Created: 10/16/2026
Status: ✅ Merged
Merged: 10/21/2026
Merged by: @morluto

Base: main ← Head: fix/objc-protocol-input-validation

📝 Commits (1)

e13b86d fix(objc): validate direct protocol enumeration input

📊 Changes

2 files changed (+69 additions, -2 deletions)

View changed files

➕ src/application/EnhancedTools.inputValidation.test.ts (+63 -0)
📝 src/application/EnhancedTools.ts (+6 -2)

📄 Description

Summary

Fixes #4383.

Direct EnhancedTools.execute("get_objc_protocols", { unexpected: "benign" }) calls discarded the supplied input and invoked the provider successfully. They now return AnalysisInputError before provider invocation. Valid {} calls still return empty or populated protocol inventories.

Change and scope

Parse the caller's input with the existing canonical get_objc_protocols schema, matching adjacent branches. Reuse the existing typed error path and pass parsed input to executeValidated.

Semantic owner: the direct application input boundary in EnhancedTools.execute.
Scope: a routine validation fix within existing behavior; no maintainer scope decision is needed.
CLI/MCP contracts and tool catalog: unchanged. MCP already validates separately and continues to use executeValidated.
Provider, bridge, target formats, Evidence, provenance, and cleanup: unchanged. Invalid direct calls no longer reach the provider; valid calls retain the existing read-only inventory operation.
Generated metadata, packaging, installation, and new execution/network authority: none.

Evidence and regression coverage

On base 2c7583599b9ac1c847bd77b1940930fd8d4594c7, the new direct-call regression suite had 5 failures and 2 passes: every invalid input invoked list_names. After the fix, all 7 pass. Rejections cover an extra property, null, undefined, an array, and a string, asserting both zero provider calls and the typed operation-specific error. Positive cases preserve empty and populated inventories.

Executed on macOS arm64 with Node 24.18.0, npm 11.16.0, and locked dependencies:

npm run test:local -- src/application/EnhancedTools.inputValidation.test.ts src/application/EnhancedToolSelectorOrigin.test.ts — 16 passed.
npm run test:focused -- tests/boundary/mcp/enhancedTools.inputValidation.test.ts tests/boundary/mcp/enhancedTools.04-uses-complete-inventories-inline.test.ts — 17 passed.
npm run check — passed, including typecheck, lint, formatting, knip, metadata, module boundaries, and architecture guards; lint reported warnings but no errors.
npm run verify:test-discovery — all 1,415 test files discovered exactly once.
git diff --check — passed.

Compatibility and remaining proof gaps

This restores the existing strict empty-object contract: direct callers passing undefined or other invalid inputs must pass {}. No valid input or output shape changes. Direct application and MCP boundary tests use an injected provider; no real Hopper/Ghidra or other native engine was exercised. No performance claim or full-suite result is asserted.

Author readiness

Human owner: @lyly-bonbon

I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work.
I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps.
I declare this PR ready for initial maintainer review.

_{🔄 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/4405 **Author:** [@lyly-bonbon](https://github.com/lyly-bonbon) **Created:** 10/16/2026 **Status:** ✅ Merged **Merged:** 10/21/2026 **Merged by:** [@morluto](https://github.com/morluto) **Base:** `main` ← **Head:** `fix/objc-protocol-input-validation` --- ### 📝 Commits (1) - [`e13b86d`](https://github.com/morluto/rea/commit/e13b86ddddd17dfd6a5d0a4492e14282720fb4b3) fix(objc): validate direct protocol enumeration input ### 📊 Changes **2 files changed** (+69 additions, -2 deletions) <details> <summary>View changed files</summary> ➕ `src/application/EnhancedTools.inputValidation.test.ts` (+63 -0) 📝 `src/application/EnhancedTools.ts` (+6 -2) </details> ### 📄 Description ## Summary Fixes #4383. Direct `EnhancedTools.execute("get_objc_protocols", { unexpected: "benign" })` calls discarded the supplied input and invoked the provider successfully. They now return `AnalysisInputError` before provider invocation. Valid `{}` calls still return empty or populated protocol inventories. ## Change and scope Parse the caller's input with the existing canonical `get_objc_protocols` schema, matching adjacent branches. Reuse the existing typed error path and pass parsed input to `executeValidated`. - Semantic owner: the direct application input boundary in `EnhancedTools.execute`. - Scope: a routine validation fix within existing behavior; no maintainer scope decision is needed. - CLI/MCP contracts and tool catalog: unchanged. MCP already validates separately and continues to use `executeValidated`. - Provider, bridge, target formats, Evidence, provenance, and cleanup: unchanged. Invalid direct calls no longer reach the provider; valid calls retain the existing read-only inventory operation. - Generated metadata, packaging, installation, and new execution/network authority: none. ## Evidence and regression coverage On base `2c7583599b9ac1c847bd77b1940930fd8d4594c7`, the new direct-call regression suite had **5 failures and 2 passes**: every invalid input invoked `list_names`. After the fix, all 7 pass. Rejections cover an extra property, `null`, `undefined`, an array, and a string, asserting both zero provider calls and the typed operation-specific error. Positive cases preserve empty and populated inventories. Executed on macOS arm64 with Node 24.18.0, npm 11.16.0, and locked dependencies: - `npm run test:local -- src/application/EnhancedTools.inputValidation.test.ts src/application/EnhancedToolSelectorOrigin.test.ts` — 16 passed. - `npm run test:focused -- tests/boundary/mcp/enhancedTools.inputValidation.test.ts tests/boundary/mcp/enhancedTools.04-uses-complete-inventories-inline.test.ts` — 17 passed. - `npm run check` — passed, including typecheck, lint, formatting, knip, metadata, module boundaries, and architecture guards; lint reported warnings but no errors. - `npm run verify:test-discovery` — all 1,415 test files discovered exactly once. - `git diff --check` — passed. ## Compatibility and remaining proof gaps This restores the existing strict empty-object contract: direct callers passing `undefined` or other invalid inputs must pass `{}`. No valid input or output shape changes. Direct application and MCP boundary tests use an injected provider; no real Hopper/Ghidra or other native engine was exercised. No performance claim or full-suite result is asserted. ## Author readiness Human owner: @lyly-bonbon - [x] I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work. - [x] I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps. - [x] I declare this PR ready for initial maintainer review. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-31 01:12:46 +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#6216

No description provided.

Rows
Columns