morluto/rea

Fork 0

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

[PR #5175] [CLOSED] fix(frida): count lone high surrogates in JSON byte budgets #6801

New issue

Closed

opened 2026-11-01 21:43:07 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-11-01 21:43:07 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/5175
Author: @prabhuramkarki
Created: 10/19/2026
Status: ❌ Closed

Base: main ← Head: fix/frida-surrogate-byte-budget

📝 Commits (2)

e1b30c2 fix(frida): count lone high surrogates in JSON byte budgets
fceee5f chore(frida): sync upstream cancellation and decoding fixes

📊 Changes

3 files changed (+108 additions, -2 deletions)

View changed files

📝 src/application/frida/FridaInstrumentationManager.test.ts (+65 -0)
📝 src/frida/FridaInstrumentationManager.ts (+2 -1)
📝 tests/boundary/mcp/fridaLifecycleMcp.test.ts (+41 -1)

📄 Description

Summary

Fixes #5097. Frida message text containing an unpaired UTF-16 high surrogate now respects its existing serialized JSON byte limit and reports truncation when text is omitted.

Problem and expected behavior

charCodeAt returns NaN beyond the end of a string. The JSON byte counter checked whether the following code unit was outside the low-surrogate range, but both comparisons were false for a trailing high surrogate. It therefore charged three bytes for a character that JSON serializes as six bytes.

This also affected repeated high surrogates because the bounded-string owner measures each code point separately. On unchanged main 9af0b6113, the two reported descriptions retained 32,771 and 65,532 JSON-content bytes against the 32,768-byte cap, without truncation flags.

Change and scope

Recognize the missing following code unit in the existing shared counter. The bounded-string owner continues to select the retained prefix and set its existing field/script flags. No downstream repair, new parser, dependencies, or configurable limits.

Synced with upstream main 048d70755 after its Frida cancellation/decoding changes created an import conflict in the manager tests. Both sets of imports and regression coverage are retained; the surrogate byte-counter fix is unchanged.

Contract and boundary impact

Semantic owner: jsonStringBytes in FridaInstrumentationManager, before message retention.
Contribution scope: ordinary correctness fix within existing Frida boundaries; new-capability maintainer decision not applicable.
CLI/MCP contracts: unchanged schemas and limits. Oversized malformed text now has the correct retained prefix and truncation flags.
Provider/platform compatibility: no native protocol changes; valid surrogate pairs and exactly fitting ASCII remain unchanged.
Evidence/provenance: omitted text remains explicit through field flags, script results, retained session status, and MCP Evidence.
Target selection, execution authority, cleanup, generated metadata, and package/install behavior: unchanged.

Evidence and regression coverage

Executed baseline: the five new manager cases produced two failures and three passing controls. The entire manager file had 2 failed / 38 passed. Actual JSON serialization independently exposed the 32,771- and 65,532-byte overruns.
Executed baseline MCP: both new error-message cases failed because messages_truncated was false; 26 existing cases passed.
Executed candidate: 110 checks passed across six files. Manager controls cover description, stack, file name, retained session status, lone high/low surrogates, valid pairs, exact-limit ASCII, retained prefixes, and truncation flags. SDK MCP controls verify both load Evidence and subsequent session-status Evidence.
Independent source review found no blockers; its separate byte-count oracle agreed with JSON serialization on its tested inputs.

Validation performed

Node 24.18.0 / npm 11.16.0, invoked through npm exec:

npm run test:focused -- src/application/frida/FridaInstrumentationManager.test.ts src/application/frida/FridaInstrumentationService.test.ts src/contracts/fridaToolContracts.test.ts tests/boundary/mcp/fridaLifecycleMcp.test.ts tests/boundary/mcp/fridaMcp.test.ts tests/boundary/cli/fridaCli.test.ts — 110 passed; runtime build completed.
npm run check — passed: compilation, typecheck, lint/module boundaries, repository formatting, knip, package metadata, and architecture guards.
git diff --check — passed.

Compatibility, safety, and release

No breaking changes or new target side effects. Public CLI/MCP adapters were tested with simulated Frida bindings; no native Frida session or real target instrumentation was executed. The full repository suite and other operating systems were not tested. Serialized field sizes were checked directly; token costs, CPU/latency, and peak process-family memory were not profiled, and no performance improvement is claimed.

GitHub validation status

As of 2026-10-19, upstream CI is action_required and has not executed. This contributor account cannot approve fork workflow runs (HTTP 403: repository admin rights required). The separate PR readiness status is pending because GitHub returns no upstream PR association for this fork head. Local validation above is not a claim that upstream CI passed.

Author readiness

Human owner: @prabhuramkarki

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.

Keep this AI-assisted PR in draft until the human owner completes these declarations.

_{🔄 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/5175 **Author:** [@prabhuramkarki](https://github.com/prabhuramkarki) **Created:** 10/19/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/frida-surrogate-byte-budget` --- ### 📝 Commits (2) - [`e1b30c2`](https://github.com/morluto/rea/commit/e1b30c27cadc461376526237b06af7c245131042) fix(frida): count lone high surrogates in JSON byte budgets - [`fceee5f`](https://github.com/morluto/rea/commit/fceee5f49e61dfbd79cb3b1eac369ddd47d844ad) chore(frida): sync upstream cancellation and decoding fixes ### 📊 Changes **3 files changed** (+108 additions, -2 deletions) <details> <summary>View changed files</summary> 📝 `src/application/frida/FridaInstrumentationManager.test.ts` (+65 -0) 📝 `src/frida/FridaInstrumentationManager.ts` (+2 -1) 📝 `tests/boundary/mcp/fridaLifecycleMcp.test.ts` (+41 -1) </details> ### 📄 Description ## Summary Fixes #5097. Frida message text containing an unpaired UTF-16 high surrogate now respects its existing serialized JSON byte limit and reports truncation when text is omitted. ## Problem and expected behavior `charCodeAt` returns `NaN` beyond the end of a string. The JSON byte counter checked whether the following code unit was outside the low-surrogate range, but both comparisons were false for a trailing high surrogate. It therefore charged three bytes for a character that JSON serializes as six bytes. This also affected repeated high surrogates because the bounded-string owner measures each code point separately. On unchanged main `9af0b6113`, the two reported descriptions retained 32,771 and 65,532 JSON-content bytes against the 32,768-byte cap, without truncation flags. ## Change and scope Recognize the missing following code unit in the existing shared counter. The bounded-string owner continues to select the retained prefix and set its existing field/script flags. No downstream repair, new parser, dependencies, or configurable limits. Synced with upstream main `048d70755` after its Frida cancellation/decoding changes created an import conflict in the manager tests. Both sets of imports and regression coverage are retained; the surrogate byte-counter fix is unchanged. ## Contract and boundary impact - Semantic owner: `jsonStringBytes` in `FridaInstrumentationManager`, before message retention. - Contribution scope: ordinary correctness fix within existing Frida boundaries; new-capability maintainer decision not applicable. - CLI/MCP contracts: unchanged schemas and limits. Oversized malformed text now has the correct retained prefix and truncation flags. - Provider/platform compatibility: no native protocol changes; valid surrogate pairs and exactly fitting ASCII remain unchanged. - Evidence/provenance: omitted text remains explicit through field flags, script results, retained session status, and MCP Evidence. - Target selection, execution authority, cleanup, generated metadata, and package/install behavior: unchanged. ## Evidence and regression coverage - **Executed baseline:** the five new manager cases produced two failures and three passing controls. The entire manager file had 2 failed / 38 passed. Actual JSON serialization independently exposed the 32,771- and 65,532-byte overruns. - **Executed baseline MCP:** both new error-message cases failed because `messages_truncated` was false; 26 existing cases passed. - **Executed candidate:** 110 checks passed across six files. Manager controls cover description, stack, file name, retained session status, lone high/low surrogates, valid pairs, exact-limit ASCII, retained prefixes, and truncation flags. SDK MCP controls verify both load Evidence and subsequent session-status Evidence. - Independent source review found no blockers; its separate byte-count oracle agreed with JSON serialization on its tested inputs. ## Validation performed Node 24.18.0 / npm 11.16.0, invoked through `npm exec`: - `npm run test:focused -- src/application/frida/FridaInstrumentationManager.test.ts src/application/frida/FridaInstrumentationService.test.ts src/contracts/fridaToolContracts.test.ts tests/boundary/mcp/fridaLifecycleMcp.test.ts tests/boundary/mcp/fridaMcp.test.ts tests/boundary/cli/fridaCli.test.ts` — 110 passed; runtime build completed. - `npm run check` — passed: compilation, typecheck, lint/module boundaries, repository formatting, knip, package metadata, and architecture guards. - `git diff --check` — passed. ## Compatibility, safety, and release No breaking changes or new target side effects. Public CLI/MCP adapters were tested with simulated Frida bindings; no native Frida session or real target instrumentation was executed. The full repository suite and other operating systems were not tested. Serialized field sizes were checked directly; token costs, CPU/latency, and peak process-family memory were not profiled, and no performance improvement is claimed. ## GitHub validation status As of 2026-10-19, upstream CI is `action_required` and has not executed. This contributor account cannot approve fork workflow runs (HTTP 403: repository admin rights required). The separate PR readiness status is pending because GitHub returns no upstream PR association for this fork head. Local validation above is not a claim that upstream CI passed. ## Author readiness Human owner: @prabhuramkarki - [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. Keep this AI-assisted PR in draft until the human owner completes these declarations. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-11-01 21:43:08 +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#6801

No description provided.

Rows
Columns