[PR #3458] [CLOSED] fix(adb): verify pushes to long device paths #5750

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

📋 Pull Request Information

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

Base: main ← Head: fix/adb-push-digest-budget


📝 Commits (7)

  • d2dc61d fix(adb): verify pushes to long device paths
  • 10d44df Merge remote-tracking branch 'origin/main' into fix/adb-push-digest-budget
  • b79b8bc Merge remote-tracking branch 'origin/main' into fix/adb-push-digest-budget
  • 0bb369c Merge remote-tracking branch 'origin/main' into fix/adb-push-digest-budget
  • 2f69bb7 Keep adbDeviceAnalysis.ts as on main
  • 6f3c1ca Merge remote-tracking branch 'origin/main' into work/3458
  • de75d49 test(adb): keep only the long-path digest test

📊 Changes

2 files changed (+36 additions, -1 deletions)

View changed files

📝 src/adb/AdbProvider.test.ts (+30 -0)
📝 src/adb/AdbProvider.ts (+6 -1)

📄 Description

Summary

Fixes #3369. push-adb-file now verifies a push to a long device path. Previously, a correct sha256sum answer that exceeded the capture budget was reported as device_digest_source: "unavailable". When an answer genuinely cannot be read within its budget, the operation now returns the documented resource_constraint failure instead of an unverified success.

Problem and expected behavior

sha256sum echoes the device path after the digest, but the capture budget was a fixed 1 KiB. For a 1,142-character path, the correct checksum line exceeded it. runAdb returned AnalysisResourceConstraintError, and the push path treated every digest failure as "the device did not answer sha256sum". Callers got the same outcome as a device without sha256sum, and the reason was lost. The issue expects a correct digest to verify the transfer (docs/adb-device-analysis.md), and a capture-budget failure to remain a resource_constraint, distinguishable from an absent command.

Change and scope

In src/adb/AdbProvider.ts (pushFile):

  • The sha256sum capture budget is 1024 + 2 × byteLength(device_path). The factor of two leaves room for a sha256sum that escapes special characters in the echoed path.
  • A digest probe that fails with AnalysisResourceConstraintError is returned as that error instead of becoming "unavailable". Other failures (no sha256sum, protocol errors) keep the existing "unavailable" result and limitation.

Not included: changing what other digest failures (for example cancellation) report.

Ordinary fix within existing boundaries; scope decision not applicable. The push itself and its mutation authority are unchanged.

Contract and boundary impact

  • Semantic owner and earliest changed stage: ADB provider push verification.
  • Contribution scope fit and linked maintainer decision: not applicable (bug fix).
  • CLI and MCP/tool-catalog contract: unchanged schema; an oversized digest answer now yields the existing resource_constraint error code.
  • Provider, bridge, target-format, or platform compatibility: ADB provider only.
  • Evidence, artifact, provenance, or reconstruction contract: long-path pushes report device_sha256 and device_digest_source: "device_sha256sum".
  • Target selection, authority, upstream behavior, and cleanup details (if applicable): same adb commands and arguments; only the capture budget changes.
  • Generated metadata, package, or installation impact: none.

Expected effects (check known effects; use documentation-only on its own).
Unchecked items do not establish absence; leave uncertainties visible.

  • Read existing files or captured data.
  • Launch or attach to a process.
  • Inject instrumentation or run scripts/code.
  • Change process memory, registers, files, or configuration.
  • Contact network services.
  • Recover protected code or work around protections.
  • Documentation or usability only; no target interaction.
  • Not sure yet; investigation or maintainer guidance needed.

(The checked effects belong to the existing push_adb_file operation, which launches adb and writes the caller's file to the device; this PR doesn't add any.)

Evidence and regression coverage

  • Tests added or updated (src/adb/AdbProvider.test.ts):
    • "verifies the device-side digest for a long device path" — a stub adb answers sha256sum for a 1,132-character path built from 220-character directories (each component within the usual 255-byte limit, per review); expects device_sha256 to equal the local digest with source device_sha256sum.
    • "reports an oversized sha256sum answer as a resource constraint" — a stub answers with 8 KiB; expects a resource_constraint refusal.
  • Base reproduction or other evidence: both tests fail with the source change reverted (the first returns unavailable/null, the second succeeds). A CLI run on base with a stub adb (REA_ADB_PATH) and a 1,142-character path returned device_digest_source: "unavailable", device_sha256: null.
  • User-visible CLI/MCP output (if applicable): after the change, the same CLI run returns device_digest_source: "device_sha256sum" with the correct digest for both a 25-character and a 1,142-character path.
  • Remaining proof gaps: no live Android device or emulator was used; the producer is a local stub, as in the issue's reproduction.

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

  • New tests with the fix reverted — both fail
  • npx vitest run src/adb tests/boundary/mcp/adbMcp.test.ts — 134 passed
  • npm run check — passed
  • node scripts/rea.mjs push-adb-file probe-device <local> <device-path> --format json with a stub adb, before/after — unavailable → device_sha256sum for the 1,142-character path (macOS arm64, Node 24.13)

Compatibility, safety, and release

  • Breaking changes or migration steps: none. A capture that overflows even the larger budget now fails with resource_constraint after the push has completed; previously it returned success with an unverified digest.
  • Real Hopper/Ghidra, browser, or OS coverage: no real device.
  • Package or release metadata impact: none.
  • Security, privacy, process, or containment review: the capture budget grows with the caller-supplied path (bounded by its length); no new commands.

Author readiness

Human owner: @HawkForge

  • 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/3458 **Author:** [@HawkForge](https://github.com/HawkForge) **Created:** 10/14/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/adb-push-digest-budget` --- ### 📝 Commits (7) - [`d2dc61d`](https://github.com/morluto/rea/commit/d2dc61ddce9d1f53dbecd8e31228a36441ce53bc) fix(adb): verify pushes to long device paths - [`10d44df`](https://github.com/morluto/rea/commit/10d44dfe2e716cadfb48aff38b446de001e77db1) Merge remote-tracking branch 'origin/main' into fix/adb-push-digest-budget - [`b79b8bc`](https://github.com/morluto/rea/commit/b79b8bca7044b581ab8af3f0438024000dc3aef8) Merge remote-tracking branch 'origin/main' into fix/adb-push-digest-budget - [`0bb369c`](https://github.com/morluto/rea/commit/0bb369c8dc8dc41f552458373e395c2cd501ebd9) Merge remote-tracking branch 'origin/main' into fix/adb-push-digest-budget - [`2f69bb7`](https://github.com/morluto/rea/commit/2f69bb798976a5cf6e7f826d1e061da5a37d5607) Keep adbDeviceAnalysis.ts as on main - [`6f3c1ca`](https://github.com/morluto/rea/commit/6f3c1ca191c84b49dc9f8d11da3246dc3483efd5) Merge remote-tracking branch 'origin/main' into work/3458 - [`de75d49`](https://github.com/morluto/rea/commit/de75d490db0caae81aca8a68f715f224df0f0ee5) test(adb): keep only the long-path digest test ### 📊 Changes **2 files changed** (+36 additions, -1 deletions) <details> <summary>View changed files</summary> 📝 `src/adb/AdbProvider.test.ts` (+30 -0) 📝 `src/adb/AdbProvider.ts` (+6 -1) </details> ### 📄 Description ## Summary Fixes #3369. `push-adb-file` now verifies a push to a long device path. Previously, a correct `sha256sum` answer that exceeded the capture budget was reported as `device_digest_source: "unavailable"`. When an answer genuinely cannot be read within its budget, the operation now returns the documented `resource_constraint` failure instead of an unverified success. ## Problem and expected behavior `sha256sum` echoes the device path after the digest, but the capture budget was a fixed 1 KiB. For a 1,142-character path, the correct checksum line exceeded it. `runAdb` returned `AnalysisResourceConstraintError`, and the push path treated every digest failure as "the device did not answer sha256sum". Callers got the same outcome as a device without `sha256sum`, and the reason was lost. The issue expects a correct digest to verify the transfer (docs/adb-device-analysis.md), and a capture-budget failure to remain a `resource_constraint`, distinguishable from an absent command. ## Change and scope In `src/adb/AdbProvider.ts` (`pushFile`): - The `sha256sum` capture budget is `1024 + 2 × byteLength(device_path)`. The factor of two leaves room for a `sha256sum` that escapes special characters in the echoed path. - A digest probe that fails with `AnalysisResourceConstraintError` is returned as that error instead of becoming `"unavailable"`. Other failures (no `sha256sum`, protocol errors) keep the existing `"unavailable"` result and limitation. Not included: changing what other digest failures (for example cancellation) report. Ordinary fix within existing boundaries; scope decision not applicable. The push itself and its mutation authority are unchanged. ## Contract and boundary impact - Semantic owner and earliest changed stage: ADB provider push verification. - Contribution scope fit and linked maintainer decision: not applicable (bug fix). - CLI and MCP/tool-catalog contract: unchanged schema; an oversized digest answer now yields the existing `resource_constraint` error code. - Provider, bridge, target-format, or platform compatibility: ADB provider only. - Evidence, artifact, provenance, or reconstruction contract: long-path pushes report `device_sha256` and `device_digest_source: "device_sha256sum"`. - Target selection, authority, upstream behavior, and cleanup details (if applicable): same adb commands and arguments; only the capture budget changes. - Generated metadata, package, or installation impact: none. Expected effects (check known effects; use documentation-only on its own). Unchecked items do not establish absence; leave uncertainties visible. - [x] Read existing files or captured data. - [x] Launch or attach to a process. - [ ] Inject instrumentation or run scripts/code. - [x] Change process memory, registers, files, or configuration. - [ ] Contact network services. - [ ] Recover protected code or work around protections. - [ ] Documentation or usability only; no target interaction. - [ ] Not sure yet; investigation or maintainer guidance needed. (The checked effects belong to the existing `push_adb_file` operation, which launches adb and writes the caller's file to the device; this PR doesn't add any.) ## Evidence and regression coverage - Tests added or updated (`src/adb/AdbProvider.test.ts`): - "verifies the device-side digest for a long device path" — a stub adb answers `sha256sum` for a 1,132-character path built from 220-character directories (each component within the usual 255-byte limit, per review); expects `device_sha256` to equal the local digest with source `device_sha256sum`. - "reports an oversized sha256sum answer as a resource constraint" — a stub answers with 8 KiB; expects a `resource_constraint` refusal. - Base reproduction or other evidence: both tests fail with the source change reverted (the first returns `unavailable`/`null`, the second succeeds). A CLI run on base with a stub adb (`REA_ADB_PATH`) and a 1,142-character path returned `device_digest_source: "unavailable"`, `device_sha256: null`. - User-visible CLI/MCP output (if applicable): after the change, the same CLI run returns `device_digest_source: "device_sha256sum"` with the correct digest for both a 25-character and a 1,142-character path. - Remaining proof gaps: no live Android device or emulator was used; the producer is a local stub, as in the issue's reproduction. 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 - New tests with the fix reverted — both fail - `npx vitest run src/adb tests/boundary/mcp/adbMcp.test.ts` — 134 passed - `npm run check` — passed - `node scripts/rea.mjs push-adb-file probe-device <local> <device-path> --format json` with a stub adb, before/after — `unavailable` → `device_sha256sum` for the 1,142-character path (macOS arm64, Node 24.13) ## Compatibility, safety, and release - Breaking changes or migration steps: none. A capture that overflows even the larger budget now fails with `resource_constraint` after the push has completed; previously it returned success with an unverified digest. - Real Hopper/Ghidra, browser, or OS coverage: no real device. - Package or release metadata impact: none. - Security, privacy, process, or containment review: the capture budget grows with the caller-supplied path (bounded by its length); no new commands. ## Author readiness Human owner: @HawkForge - [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 2026-10-31 01:09:09 +00:00
Sign in to join this conversation.
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#5750
No description provided.