morluto/rea

Fork 0

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

[PR #3507] [CLOSED] fix(frida): preserve script-file identity with strict UTF-8 admission #5789

New issue

Closed

opened 2026-10-31 01:09:27 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 01:09:27 +00:00

Owner

📋 Pull Request Information

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

Base: main ← Head: fix/frida-script-file-utf8

📝 Commits (4)

ad1c181 fix(frida): preserve script-file identity with strict UTF-8 admission
71ef4de Merge remote-tracking branch 'origin/main' into fix/frida-script-file-utf8
35717b4 fix(ci): cover GitHub action sources in lint cache inputs
33fa5f6 fix(ci): normalize Windows paths in duplication reports

📊 Changes

6 files changed (+205 additions, -6 deletions)

View changed files

📝 docs/frida-instrumentation.md (+2 -0)
📝 scripts/ci/static-check-report.mjs (+7 -2)
📝 src/frida/FridaInstrumentationManager.ts (+19 -4)
➕ tests/acceptance/fridaScriptEncodingCli.test.ts (+108 -0)
➕ tests/fixtures/fridaScriptSource.mjs (+67 -0)
📝 turbo.json (+2 -0)

📄 Description

Summary

Fixes #3396. Frida script files with invalid UTF-8 now return invalid_request / invalid_format before native script creation instead of producing successful Evidence for replacement-decoded bytes.

Problem and approach

Previously, reading files with readFile(path, "utf8") replaced invalid bytes before SHA-256 calculation. Different selected files could therefore report the same digest as a valid file containing U+FFFD.

Read file sources as bytes, preserve cancellation/error ordering, validate UTF-8 before decoding, and hash the original bytes. Inline source behavior remains unchanged. File permissions/read failures keep their existing typed handling. Document the supported UTF-8 file contract, including BOM identity. This ordinary input/provenance correction adds no provider capability or target authority.

Executed evidence

Before the fix: four invalid-byte CLI/MCP regressions fail because they are wrongly accepted; eight valid controls pass.
After the fix: all 12 public regressions pass for ff, fe, actual U+FFFD, ASCII, Unicode and BOM inputs across compiled CLI and stdio MCP. Invalid inputs never reach native script creation; valid inputs retain both the selected-file digest and the digest of the delivered source.
The public regression substitutes only the native Frida module. Adapters, file acquisition, manager/service logic, contracts and Evidence run normally; those tests do not attach to a real process.
Six focused files / 91 tests pass, including existing Frida manager/service, CLI/MCP and lifecycle/cancellation/cleanup coverage.
npm run verify:frida — real Frida verification passes on this Windows host using its own short-lived Node target; marker observed and cleanup_error: null.
Additional real Frida file-source checks through the compiled application service: Unicode and BOM files produce the target marker and exact original-file SHA-256 with no cleanup error; invalid ff input returns invalid_format. Only self-created Node targets were used.

Validation

npm run check and npm run check:ci — pass.
npm run docs:check — pass.
npm run verify:test-discovery — all 1,275 test files discovered exactly once.

Local review traced the source acquisition boundary, both public consumers, cancellation priority, typed errors and script-registration ordering. Host/toolchain: Windows, pinned Node 24.18.0, locked dependencies. Remote devices, Linux/macOS and the full repository suite were not exercised. The added UTF-8 validation pass was not separately benchmarked.

CI integration follow-up

Synced this branch with main eab789aed. Both lint task cache inputs now include checker-consumed code under .github, correcting the CI inventory failure caused by the publishing-action files.

Commit 33fa5f675 addresses the valid review finding that the previous test-side path conversion could hide incorrect saved report paths. The test again asserts the raw report names. The report writer now normalizes both absolute paths to the same Windows namespace before computing relative paths; clone locations and threshold handling are preserved. Restoring the strict assertion reproduced absolute //?/C:/... names before the writer fix, and the unchanged assertion passes after it.

Executed for this correction: 2 files / 20 tests (public Frida encoding regressions and static-check reports) pass; npm run check:ci and normal pre-push checks pass. New-head remote CI is pending. Earlier validation above applies to its stated revisions; native runtime checks were not repeated for this CI/reporting correction.

Author readiness

Human owner: @chqcjq

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.

The human owner's readiness declarations are recorded above.

_{🔄 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/3507 **Author:** [@chqcjq](https://github.com/chqcjq) **Created:** 10/14/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/frida-script-file-utf8` --- ### 📝 Commits (4) - [`ad1c181`](https://github.com/morluto/rea/commit/ad1c1818d0fd0353cc2083016fe9aaddb8689ee6) fix(frida): preserve script-file identity with strict UTF-8 admission - [`71ef4de`](https://github.com/morluto/rea/commit/71ef4debdaf077d94ac08b1e7a542a0916efa228) Merge remote-tracking branch 'origin/main' into fix/frida-script-file-utf8 - [`35717b4`](https://github.com/morluto/rea/commit/35717b4a9e453e0e08286d4d41808277fb186ae7) fix(ci): cover GitHub action sources in lint cache inputs - [`33fa5f6`](https://github.com/morluto/rea/commit/33fa5f675dd3fadb9dd659be62da029bbd04d240) fix(ci): normalize Windows paths in duplication reports ### 📊 Changes **6 files changed** (+205 additions, -6 deletions) <details> <summary>View changed files</summary> 📝 `docs/frida-instrumentation.md` (+2 -0) 📝 `scripts/ci/static-check-report.mjs` (+7 -2) 📝 `src/frida/FridaInstrumentationManager.ts` (+19 -4) ➕ `tests/acceptance/fridaScriptEncodingCli.test.ts` (+108 -0) ➕ `tests/fixtures/fridaScriptSource.mjs` (+67 -0) 📝 `turbo.json` (+2 -0) </details> ### 📄 Description ## Summary Fixes #3396. Frida script files with invalid UTF-8 now return `invalid_request` / `invalid_format` before native script creation instead of producing successful Evidence for replacement-decoded bytes. ## Problem and approach Previously, reading files with `readFile(path, "utf8")` replaced invalid bytes before SHA-256 calculation. Different selected files could therefore report the same digest as a valid file containing U+FFFD. Read file sources as bytes, preserve cancellation/error ordering, validate UTF-8 before decoding, and hash the original bytes. Inline source behavior remains unchanged. File permissions/read failures keep their existing typed handling. Document the supported UTF-8 file contract, including BOM identity. This ordinary input/provenance correction adds no provider capability or target authority. ## Executed evidence - Before the fix: four invalid-byte CLI/MCP regressions fail because they are wrongly accepted; eight valid controls pass. - After the fix: all 12 public regressions pass for `ff`, `fe`, actual U+FFFD, ASCII, Unicode and BOM inputs across compiled CLI and stdio MCP. Invalid inputs never reach native script creation; valid inputs retain both the selected-file digest and the digest of the delivered source. - The public regression substitutes only the native Frida module. Adapters, file acquisition, manager/service logic, contracts and Evidence run normally; those tests do not attach to a real process. - Six focused files / **91 tests pass**, including existing Frida manager/service, CLI/MCP and lifecycle/cancellation/cleanup coverage. - `npm run verify:frida` — real Frida verification passes on this Windows host using its own short-lived Node target; marker observed and `cleanup_error: null`. - Additional real Frida file-source checks through the compiled application service: Unicode and BOM files produce the target marker and exact original-file SHA-256 with no cleanup error; invalid `ff` input returns `invalid_format`. Only self-created Node targets were used. ## Validation - `npm run check` and `npm run check:ci` — pass. - `npm run docs:check` — pass. - `npm run verify:test-discovery` — all 1,275 test files discovered exactly once. Local review traced the source acquisition boundary, both public consumers, cancellation priority, typed errors and script-registration ordering. Host/toolchain: Windows, pinned Node 24.18.0, locked dependencies. Remote devices, Linux/macOS and the full repository suite were not exercised. The added UTF-8 validation pass was not separately benchmarked. ## CI integration follow-up Synced this branch with main `eab789aed`. Both lint task cache inputs now include checker-consumed code under `.github`, correcting the CI inventory failure caused by the publishing-action files. Commit `33fa5f675` addresses the valid review finding that the previous test-side path conversion could hide incorrect saved report paths. The test again asserts the raw report names. The report writer now normalizes both absolute paths to the same Windows namespace before computing relative paths; clone locations and threshold handling are preserved. Restoring the strict assertion reproduced absolute `//?/C:/...` names before the writer fix, and the unchanged assertion passes after it. Executed for this correction: 2 files / 20 tests (public Frida encoding regressions and static-check reports) pass; `npm run check:ci` and normal pre-push checks pass. New-head remote CI is pending. Earlier validation above applies to its stated revisions; native runtime checks were not repeated for this CI/reporting correction. ## Author readiness Human owner: @chqcjq - [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. The human owner's readiness declarations are recorded above. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-31 01:09:27 +00:00

closed this issue
added the
pull-request
label

bsakaguchi referenced this issue

2026-11-01 21:45:04 +00:00

[PR #5789] [MERGED] fix(model): preserve unsupported GGUF version diagnostics #7063

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

No description provided.

Rows
Columns