morluto/rea

Fork 0

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

[PR #6350] fix(contracts): advertise the Frida remote address character set #7296

New issue

Open

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

bsakaguchi commented

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

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/6350
Author: @saimmehmood
Created: 10/22/2026
Status: 🔄 Open

Base: main ← Head: fix/advertise-frida-remote-address-charset

📝 Commits (3)

2ca4b78 fix(contracts): advertise the Frida remote address character set
da794bd fix(contracts): spell the advertised solidus as a hex escape
d8987ef Merge origin/main into fix/advertise-frida-remote-address-charset

📊 Changes

3 files changed (+100 additions, -5 deletions)

View changed files

📝 src/contracts/fridaToolContracts.test.ts (+62 -0)
📝 src/contracts/fridaToolContracts.ts (+17 -4)
📝 src/domain/fridaRemoteAddress.ts (+21 -1)

📄 Description

Summary

The advertised input schema for list_frida_devices and list_frida_processes
accepted remote.address values that canonical validation rejects. A
schema-driven caller could build a request that passes the published JSON Schema
and is then refused by the handler.

Relates to #5493 (MCP input schemas omit host-absolute local path constraints),
which is the same root cause in a different cohort: a Zod refine predicate
does not project into JSON Schema.

Problem and expected behavior

Smallest trigger — call list_frida_devices with a single space as the address:

{ "remote": { "address": " ", "keepalive_interval": 30 } }

The advertised schema accepted it; inputSchema.safeParse rejects it with
Expected a Frida host address, optionally followed by a port.

Violated invariant: the advertised schema must express the constraints the
canonical validator already enforces, so SDK conversion and runtime validation
agree. The address property advertised only type, minLength and maxLength,
because the character and URL rules lived entirely in
.refine(isValidFridaRemoteAddress).

On the base revision the projected property carries no pattern at all:

{ "type": "string", "minLength": 1, "maxLength": 2048 }

Change and scope

Hoist the existing inline character class out of the predicate into
FRIDA_REMOTE_ADDRESS_CHARSET and advertise it as a schema pattern next to the
unchanged predicate.

Canonical acceptance is unchanged. The predicate still owns the URL-level rules a
portable pattern cannot express: nonempty hostname, port range, absent
credentials, absent path/query/fragment, and the trailing-colon rejection.

Intentionally not included:

No attempt to encode the full address grammar as a regex. bytePattern.ts
records the opposite preference for grammar-level rules ("exact grammar belongs
to the object refinement, not a repeating advertised regex"), so only the flat
character class is advertised.
No lookbehind for the trailing-colon rule. The advertised pattern must compile
in strict schema consumers, and a client can reject an entire tool request when
one pattern fails to compile.
The cdp_endpoint / inspector_endpoint cohort (browserObservation.ts) has
the same shape but validates through .transform(), and that file is already
being changed by open PRs; it is left alone.

Contract and boundary impact

Semantic owner and earliest changed stage: src/contracts/fridaRemoteAddress.ts
owns the character set; src/contracts/fridaToolContracts.ts advertises it at
request-schema advertisement and validation.
Contribution scope fit and linked maintainer decision: not applicable; a fix
inside an existing contract boundary, no new provider or capability.
CLI and MCP/tool-catalog contract: the advertised input schema for
list_frida_devices and list_frida_processes gains a pattern keyword. The
canonical accepted set does not change, so no caller sending a valid address is
affected. npm run metadata:check passes; docs/public/product-catalog.json is
gitignored and regenerated by CI.
Provider, bridge, target-format, or platform compatibility: none. No Frida
Core interaction changes; the character set is platform-independent.
Evidence, artifact, provenance, or reconstruction contract: unchanged.
Target selection, authority, upstream behavior, and cleanup: unchanged. Remote
credential handling is untouched and still covered by the existing MCP boundary
test.
Generated metadata, package, or installation impact: advertised schemas feed
the generated catalog and tool identity digest; both are generated artifacts,
not committed here.

Expected effects:

Documentation or usability only; no target interaction.

Evidence and regression coverage

Tests added or updated: a new Frida remote address advertisement block in
src/contracts/fridaToolContracts.test.ts asserts the projected property
carries a pattern, that the pattern accepts 127.0.0.1:27042,
frida-target.example and [::1]:27042, and that it rejects the same
character-level values the predicate rejects. It is a separate describe so
the existing block stays within max-lines-per-function.
Base reproduction: executed. Reverting only the two source files while
keeping the new test fails with the bug itself — the advertised property has no
pattern:
```
× advertises the character set so a projected schema agrees with canonical validation
  AssertionError: expected 'undefined' to be 'string'
```
Differential audit (ad-hoc script, not committed): compiled every advertised
input schema for all 220 tool contracts with the installed Ajv2020 and
compared acceptance against canonical safeParse. Before this change,
remote.address on both Frida tools accepted whitespace probes the canonical
validator rejects. After it, those cases agree.
Equivalence of the refactor: the previous predicate, reimplemented inline from
the base revision, and the new predicate return identical results across 37
cases (valid hosts/ports, IPv6 brackets, 2048/2049-length inputs, credentials,
paths, queries, fragments, bad ports, whitespace, NUL, DEL and C0 controls).
Advertised-vs-canonical safety: across those cases the advertised schema is
never stricter than canonical (0 occurrences), so no previously valid
request becomes schema-invalid. 16 cases now agree on rejection where the
advertised schema previously accepted them.

User-visible MCP output: the advertised property becomes

{ "type": "string", "minLength": 1, "maxLength": 2048,
  "pattern": "^[^?@#/\\s\\x00-\\x1f\\x7f]+$" }

Remaining proof gaps:
- Six semantic cases stay advertised-permissive because a portable pattern
  cannot express them: 127.0.0.1:, host:0, host:65536, host:99999,
  host:abc and bare ::1. The predicate still rejects all of them; only the
  published schema is less precise.
- No real Frida device or remote daemon was contacted; coverage is contract,
  projection and MCP-boundary level.
- oxlint reports no-control-regex on the hoisted class. That warning is
  pre-existing — it is reported at fridaRemoteAddress.ts:4 on the base
  revision for the same characters — and is only relocated by this change.

For evidence-bearing changes:

Observed, derived, and inferred claims remain distinguishable.
Artifact identity, source provenance, and failed attempts remain preserved.
Unsupported, incomplete, unavailable, or uncertain outcomes remain visible.

Validation performed

npm run test:local -- src/contracts/fridaToolContracts.test.ts — 6/6
passed.
npm run test:local -- src/contracts/fridaToolContracts.test.ts src/contracts/compactInputSchema.test.ts src/contracts/advertisedInputExamples.test.ts src/contracts/toolInputReferences.test.ts — 4 files, 65/65 passed.
npx vitest run --project mcp-boundary tests/boundary/mcp/fridaMcp.test.ts tests/boundary/mcp/absoluteLocalPathSchema.test.ts — 2 files, 7/7 passed,
including advertises Frida MCP schemas and keeps remote credentials out of results and advertises full host-absolute path constraints after SDK schema conversion.
Base-revision regression check (sources reverted, test kept) — failed as
intended, 1 test failing.
npm run check:fast (turbo typecheck + lint) — 2 successful, 2 total.
npm run metadata:check — passed, no drift reported.
npm run format:check (oxfmt) — no issues in any changed file.
npx oxlint on the three changed files — no errors; one pre-existing
no-control-regex warning, as noted above.
Pre-commit hooks (lint-staged + oxlint --fix) — pass. An earlier attempt
failed max-lines-per-function on the test file; the new test was moved into
its own describe rather than raising the limit.

Compatibility, safety, and release

Breaking changes or migration steps: none. The advertised schema becomes more
precise, never more restrictive than canonical validation, so a caller sending a
valid address is unaffected. A caller sending an invalid address now fails at
schema validation instead of handler validation, with the same message.
Real Hopper/Ghidra, browser, or OS coverage: not applicable; no provider
execution path changes.
Package or release metadata impact: generated catalog/tool-identity digests
change because the advertised schema changed. Both are generated, gitignored
artifacts; metadata:check passes.
Security, privacy, process, or containment review: the advertised pattern
excludes @, so it also makes the no-credentials rule visible to callers
earlier. No new data is disclosed; remote credential redaction is unchanged.

Author readiness

Human owner: @saimmehmood

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.

Before merge

This PR addresses a concrete problem or an agreed enhancement.
Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff.
The PR has one focused outcome and the title follows type(scope): outcome.
Related issue is linked, or the reason for not linking one is stated above.
Tests cover changed observable behavior and meaningful failure paths.
Owning docs, contracts, and generated metadata are updated where needed.
User-visible CLI/MCP changes include representative output.
I checked the final diff for secrets, unrelated cleanup, and unsupported claims.

_{🔄 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/6350 **Author:** [@saimmehmood](https://github.com/saimmehmood) **Created:** 10/22/2026 **Status:** 🔄 Open **Base:** `main` ← **Head:** `fix/advertise-frida-remote-address-charset` --- ### 📝 Commits (3) - [`2ca4b78`](https://github.com/morluto/rea/commit/2ca4b789c58e29b3d7b660e2f17d076bfb4f2f43) fix(contracts): advertise the Frida remote address character set - [`da794bd`](https://github.com/morluto/rea/commit/da794bd11ad88067d4fab90cd26f38b5f2b4685d) fix(contracts): spell the advertised solidus as a hex escape - [`d8987ef`](https://github.com/morluto/rea/commit/d8987ef81edcacaa9caf45a94492f56b40a71c7a) Merge origin/main into fix/advertise-frida-remote-address-charset ### 📊 Changes **3 files changed** (+100 additions, -5 deletions) <details> <summary>View changed files</summary> 📝 `src/contracts/fridaToolContracts.test.ts` (+62 -0) 📝 `src/contracts/fridaToolContracts.ts` (+17 -4) 📝 `src/domain/fridaRemoteAddress.ts` (+21 -1) </details> ### 📄 Description ## Summary The advertised input schema for `list_frida_devices` and `list_frida_processes` accepted `remote.address` values that canonical validation rejects. A schema-driven caller could build a request that passes the published JSON Schema and is then refused by the handler. Relates to #5493 (MCP input schemas omit host-absolute local path constraints), which is the same root cause in a different cohort: a Zod `refine` predicate does not project into JSON Schema. ## Problem and expected behavior Smallest trigger — call `list_frida_devices` with a single space as the address: ```json { "remote": { "address": " ", "keepalive_interval": 30 } } ``` The advertised schema accepted it; `inputSchema.safeParse` rejects it with `Expected a Frida host address, optionally followed by a port`. Violated invariant: the advertised schema must express the constraints the canonical validator already enforces, so SDK conversion and runtime validation agree. The address property advertised only `type`, `minLength` and `maxLength`, because the character and URL rules lived entirely in `.refine(isValidFridaRemoteAddress)`. On the base revision the projected property carries no `pattern` at all: ```json { "type": "string", "minLength": 1, "maxLength": 2048 } ``` ## Change and scope Hoist the existing inline character class out of the predicate into `FRIDA_REMOTE_ADDRESS_CHARSET` and advertise it as a schema `pattern` next to the unchanged predicate. Canonical acceptance is unchanged. The predicate still owns the URL-level rules a portable pattern cannot express: nonempty hostname, port range, absent credentials, absent path/query/fragment, and the trailing-colon rejection. Intentionally not included: - No attempt to encode the full address grammar as a regex. `bytePattern.ts` records the opposite preference for grammar-level rules ("exact grammar belongs to the object refinement, not a repeating advertised regex"), so only the flat character class is advertised. - No lookbehind for the trailing-colon rule. The advertised pattern must compile in strict schema consumers, and a client can reject an entire tool request when one pattern fails to compile. - The `cdp_endpoint` / `inspector_endpoint` cohort (`browserObservation.ts`) has the same shape but validates through `.transform()`, and that file is already being changed by open PRs; it is left alone. ## Contract and boundary impact - Semantic owner and earliest changed stage: `src/contracts/fridaRemoteAddress.ts` owns the character set; `src/contracts/fridaToolContracts.ts` advertises it at request-schema advertisement and validation. - Contribution scope fit and linked maintainer decision: not applicable; a fix inside an existing contract boundary, no new provider or capability. - CLI and MCP/tool-catalog contract: the advertised input schema for `list_frida_devices` and `list_frida_processes` gains a `pattern` keyword. The canonical accepted set does not change, so no caller sending a valid address is affected. `npm run metadata:check` passes; `docs/public/product-catalog.json` is gitignored and regenerated by CI. - Provider, bridge, target-format, or platform compatibility: none. No Frida Core interaction changes; the character set is platform-independent. - Evidence, artifact, provenance, or reconstruction contract: unchanged. - Target selection, authority, upstream behavior, and cleanup: unchanged. Remote credential handling is untouched and still covered by the existing MCP boundary test. - Generated metadata, package, or installation impact: advertised schemas feed the generated catalog and tool identity digest; both are generated artifacts, not committed here. Expected effects: - [x] Documentation or usability only; no target interaction. ## Evidence and regression coverage - Tests added or updated: a new `Frida remote address advertisement` block in `src/contracts/fridaToolContracts.test.ts` asserts the projected property carries a `pattern`, that the pattern accepts `127.0.0.1:27042`, `frida-target.example` and `[::1]:27042`, and that it rejects the same character-level values the predicate rejects. It is a separate `describe` so the existing block stays within `max-lines-per-function`. - Base reproduction: **executed**. Reverting only the two source files while keeping the new test fails with the bug itself — the advertised property has no pattern: ``` × advertises the character set so a projected schema agrees with canonical validation AssertionError: expected 'undefined' to be 'string' ``` - Differential audit (ad-hoc script, not committed): compiled every advertised input schema for all 220 tool contracts with the installed `Ajv2020` and compared acceptance against canonical `safeParse`. Before this change, `remote.address` on both Frida tools accepted whitespace probes the canonical validator rejects. After it, those cases agree. - Equivalence of the refactor: the previous predicate, reimplemented inline from the base revision, and the new predicate return **identical results across 37 cases** (valid hosts/ports, IPv6 brackets, 2048/2049-length inputs, credentials, paths, queries, fragments, bad ports, whitespace, NUL, DEL and C0 controls). - Advertised-vs-canonical safety: across those cases the advertised schema is **never stricter than canonical** (0 occurrences), so no previously valid request becomes schema-invalid. 16 cases now agree on rejection where the advertised schema previously accepted them. - User-visible MCP output: the advertised property becomes ```json { "type": "string", "minLength": 1, "maxLength": 2048, "pattern": "^[^?@#/\\s\\x00-\\x1f\\x7f]+$" } ``` - Remaining proof gaps: - Six semantic cases stay advertised-permissive because a portable pattern cannot express them: `127.0.0.1:`, `host:0`, `host:65536`, `host:99999`, `host:abc` and bare `::1`. The predicate still rejects all of them; only the published schema is less precise. - No real Frida device or remote daemon was contacted; coverage is contract, projection and MCP-boundary level. - `oxlint` reports `no-control-regex` on the hoisted class. That warning is **pre-existing** — it is reported at `fridaRemoteAddress.ts:4` on the base revision for the same characters — and is only relocated by this change. For evidence-bearing changes: - [x] Observed, derived, and inferred claims remain distinguishable. - [x] Artifact identity, source provenance, and failed attempts remain preserved. - [x] Unsupported, incomplete, unavailable, or uncertain outcomes remain visible. ## Validation performed - `npm run test:local -- src/contracts/fridaToolContracts.test.ts` — **6/6 passed**. - `npm run test:local -- src/contracts/fridaToolContracts.test.ts src/contracts/compactInputSchema.test.ts src/contracts/advertisedInputExamples.test.ts src/contracts/toolInputReferences.test.ts` — **4 files, 65/65 passed**. - `npx vitest run --project mcp-boundary tests/boundary/mcp/fridaMcp.test.ts tests/boundary/mcp/absoluteLocalPathSchema.test.ts` — **2 files, 7/7 passed**, including `advertises Frida MCP schemas and keeps remote credentials out of results` and `advertises full host-absolute path constraints after SDK schema conversion`. - Base-revision regression check (sources reverted, test kept) — **failed as intended**, 1 test failing. - `npm run check:fast` (turbo `typecheck` + `lint`) — **2 successful, 2 total**. - `npm run metadata:check` — passed, no drift reported. - `npm run format:check` (`oxfmt`) — **no issues in any changed file**. - `npx oxlint` on the three changed files — no errors; one pre-existing `no-control-regex` warning, as noted above. - Pre-commit hooks (`lint-staged` + `oxlint --fix`) — pass. An earlier attempt failed `max-lines-per-function` on the test file; the new test was moved into its own `describe` rather than raising the limit. ## Compatibility, safety, and release - Breaking changes or migration steps: none. The advertised schema becomes more precise, never more restrictive than canonical validation, so a caller sending a valid address is unaffected. A caller sending an invalid address now fails at schema validation instead of handler validation, with the same message. - Real Hopper/Ghidra, browser, or OS coverage: not applicable; no provider execution path changes. - Package or release metadata impact: generated catalog/tool-identity digests change because the advertised schema changed. Both are generated, gitignored artifacts; `metadata:check` passes. - Security, privacy, process, or containment review: the advertised pattern excludes `@`, so it also makes the no-credentials rule visible to callers earlier. No new data is disclosed; remote credential redaction is unchanged. ## Author readiness Human owner: @saimmehmood  - [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. ## Before merge - [x] This PR addresses a concrete problem or an agreed enhancement. - [x] Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff. - [x] The PR has one focused outcome and the title follows `type(scope): outcome`. - [x] Related issue is linked, or the reason for not linking one is stated above. - [x] Tests cover changed observable behavior and meaningful failure paths. - [x] Owning docs, contracts, and generated metadata are updated where needed. - [x] User-visible CLI/MCP changes include representative output. - [x] I checked the final diff for secrets, unrelated cleanup, and unsupported claims. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi added the

pull-request

label

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

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

No description provided.

Rows
Columns