[PR #6010] [MERGED] fix(adb): report an unparsed push checksum as unverified #7119

Closed
opened 2026-11-01 21:45:30 +00:00 by bsakaguchi · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/6010
Author: @kaoru0822-kitauji
Created: 10/21/2026
Status: ✅ Merged
Merged: 10/21/2026
Merged by: @kaoru0822-kitauji

Base: main ← Head: fix/5924-adb-push-digest-source


📝 Commits (1)

  • 6b4cc55 fix(adb): derive push digest source from the parsed device digest

📊 Changes

4 files changed (+136 additions, -40 deletions)

View changed files

📝 docs/adb-device-analysis.md (+3 -1)
📝 src/adb/AdbProvider.test.ts (+69 -25)
📝 src/adb/AdbProvider.ts (+56 -14)
📝 src/adb/AdbShellOutput.ts (+8 -0)

📄 Description

What broke: after a successful push_adb_file, a sha256sum that exited 0 with no recognizable digest produced device_sha256: null with device_digest_source: "device_sha256sum" and dropped the "not device-side verified" limitation.
Fix: the push receipt takes one tagged device digest (digest + source together), derived from a single sha256sum parser, so unparsed output reports "unavailable", the unverified limitation, and the raw command output.
Risk: low; ADB push receipt only, output schema unchanged. GNU-escaped (\-prefixed) checksum lines now verify instead of being treated as unrecognized.

Summary

Fixes #5924. push_adb_file now reports device_digest_source: "device_sha256sum" only when a device digest was actually parsed. A zero-exit sha256sum with empty or unrecognizable output is reported like a missing command: device_sha256: null, device_digest_source: "unavailable", and a limitation stating the transfer is not device-side verified. The unrecognized output is kept in raw_result.device_sha256sum.

Problem and expected behavior

pushFile built its receipt from two independent arguments: the regex match (or null) and a hard-coded "device_sha256sum" source on every zero-exit path. Any successful exit therefore claimed sha256sum as the digest source, even with no digest parsed, and skipped the unverified-transfer limitation that the nonzero-exit branch adds. Expected: a successful exit alone does not establish that a checksum was obtained or compared (issue #5924).

Change and scope

  • src/adb/AdbProvider.ts: the receipt takes a tagged AdbDeviceDigest ({device_sha256: string, source: "device_sha256sum"} or {device_sha256: null, source: "unavailable", reason}). Source, digest and the unverified limitation can no longer disagree. The unparsed branch keeps the command's stdout/stderr as rawResult, following the existing start_adb_activity pattern.
  • src/adb/AdbShellOutput.ts: parseSha256sumOutput owns the sha256sum FILE line format alongside the other shell-output parsers. It requires the <digest> <space|*> separator and accepts GNU's leading backslash, which marks an escaped echoed name; the digest itself is never escaped. This replaces the inline /^([a-f0-9]{64})\s/ regex.
  • docs/adb-device-analysis.md: documents the unavailable outcome.
  • The advertised output schema (device_sha256 nullable + device_digest_source enum) is unchanged. I kept it that way rather than making it a discriminated union, which would change the published JSON Schema.
  • Related ADB paths checked: pull, bugreport, screencap and package pull use host-side digests (digest_status). Install has no digest. Push is the only device-side digest reader.
  • Not included: the capture budget for long device paths (#3369, open PR #3458 touches the neighbouring lines; whichever lands second needs a small rebase).

Ordinary fix within existing boundaries; scope decision not applicable.

Contract and boundary impact

  • Semantic owner and earliest changed stage: ADB provider push verification (AdbShellOutput parser → pushFile receipt).
  • Contribution scope fit and linked maintainer decision: not applicable (bug fix).
  • CLI and MCP/tool-catalog contract: schema unchanged; field values for unparseable checksum output change as described.
  • Provider, bridge, target-format, or platform compatibility: ADB provider only.
  • Evidence, artifact, provenance, or reconstruction contract: unparsed checksum output now adds a raw_result with device_sha256sum.stdout/stderr and the unverified limitation.
  • Target selection, authority, upstream behavior, and cleanup details (if applicable): same adb commands, arguments and limits.
  • Generated metadata, package, or installation impact: none.

Expected effects (these belong to the existing push_adb_file operation; this PR adds none):

  • 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.

Evidence and regression coverage

  • Tests added or updated: src/adb/AdbProvider.test.ts "derives the push digest source from …" is a table through the production AdbProvider with a simulated adb executable. Cases: valid line, GNU-escaped line, empty stdout, malformed line (63 hex chars), nonzero exit (127). It asserts digest/source, the unverified limitation, and rawResult. It replaces the single-case "pushes a file and verifies the device-side digest" test.
  • Base reproduction: on 6d356b843 the new table failed for escaped, empty and malformed (3 failed, 2 passed). The issue's repro test through AdbDeviceAnalysisService on this branch: valid, empty and missing pass. escaped now reports a verified digest (device_sha256sum) instead of the issue's expected unavailable, by design (see Change).
  • User-visible output (from the issue repro, empty case): {"device_sha256":null,"device_digest_source":"unavailable","limitations":[…,"The device's sha256sum output contained no recognizable digest, so the transfer is not device-side verified"]}.
  • Remaining proof gaps: no real Android device or emulator was contacted. Toybox sha256sum output was not observed directly; the parser accepts its documented <digest> <name> form.

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/adb/*.test.ts src/application/adb/*.test.ts src/contracts/toolEffects.test.ts: 8 files, 201 tests passed
  • Issue #5924 repro test (temporary, removed): valid/empty/missing passed; escaped intentionally differs as described
  • npm run check:fast: passed
  • npm run format:check: passed
  • npm run docs:check: passed

Compatibility, safety, and release

  • Breaking changes or migration steps: none. Callers that already treated a null device_sha256 as unverified are unaffected.
  • Real Hopper/Ghidra, browser, or OS coverage: macOS with a simulated adb; no device.
  • Package or release metadata impact: none.
  • Security, privacy, process, or containment review: raw_result holds at most the existing 1 KiB capture of sha256sum output for the caller-selected device path.

Author readiness

Human owner: @kaoru0822-kitauji

  • 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.

🤖 Generated with Claude Code


🔄 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/6010 **Author:** [@kaoru0822-kitauji](https://github.com/kaoru0822-kitauji) **Created:** 10/21/2026 **Status:** ✅ Merged **Merged:** 10/21/2026 **Merged by:** [@kaoru0822-kitauji](https://github.com/kaoru0822-kitauji) **Base:** `main` ← **Head:** `fix/5924-adb-push-digest-source` --- ### 📝 Commits (1) - [`6b4cc55`](https://github.com/morluto/rea/commit/6b4cc5594880c5cf18b0203b4041acf8204fd871) fix(adb): derive push digest source from the parsed device digest ### 📊 Changes **4 files changed** (+136 additions, -40 deletions) <details> <summary>View changed files</summary> 📝 `docs/adb-device-analysis.md` (+3 -1) 📝 `src/adb/AdbProvider.test.ts` (+69 -25) 📝 `src/adb/AdbProvider.ts` (+56 -14) 📝 `src/adb/AdbShellOutput.ts` (+8 -0) </details> ### 📄 Description **What broke:** after a successful `push_adb_file`, a `sha256sum` that exited 0 with no recognizable digest produced `device_sha256: null` with `device_digest_source: "device_sha256sum"` and dropped the "not device-side verified" limitation. **Fix:** the push receipt takes one tagged device digest (digest + source together), derived from a single `sha256sum` parser, so unparsed output reports `"unavailable"`, the unverified limitation, and the raw command output. **Risk:** low; ADB push receipt only, output schema unchanged. GNU-escaped (`\`-prefixed) checksum lines now verify instead of being treated as unrecognized. ## Summary Fixes #5924. `push_adb_file` now reports `device_digest_source: "device_sha256sum"` only when a device digest was actually parsed. A zero-exit `sha256sum` with empty or unrecognizable output is reported like a missing command: `device_sha256: null`, `device_digest_source: "unavailable"`, and a limitation stating the transfer is not device-side verified. The unrecognized output is kept in `raw_result.device_sha256sum`. ## Problem and expected behavior `pushFile` built its receipt from two independent arguments: the regex match (or `null`) and a hard-coded `"device_sha256sum"` source on every zero-exit path. Any successful exit therefore claimed `sha256sum` as the digest source, even with no digest parsed, and skipped the unverified-transfer limitation that the nonzero-exit branch adds. Expected: a successful exit alone does not establish that a checksum was obtained or compared (issue #5924). ## Change and scope - `src/adb/AdbProvider.ts`: the receipt takes a tagged `AdbDeviceDigest` (`{device_sha256: string, source: "device_sha256sum"}` or `{device_sha256: null, source: "unavailable", reason}`). Source, digest and the unverified limitation can no longer disagree. The unparsed branch keeps the command's stdout/stderr as `rawResult`, following the existing `start_adb_activity` pattern. - `src/adb/AdbShellOutput.ts`: `parseSha256sumOutput` owns the `sha256sum FILE` line format alongside the other shell-output parsers. It requires the `<digest> <space|*>` separator and accepts GNU's leading backslash, which marks an escaped echoed name; the digest itself is never escaped. This replaces the inline `/^([a-f0-9]{64})\s/` regex. - `docs/adb-device-analysis.md`: documents the unavailable outcome. - The advertised output schema (`device_sha256` nullable + `device_digest_source` enum) is unchanged. I kept it that way rather than making it a discriminated union, which would change the published JSON Schema. - Related ADB paths checked: pull, bugreport, screencap and package pull use host-side digests (`digest_status`). Install has no digest. Push is the only device-side digest reader. - Not included: the capture budget for long device paths (#3369, open PR #3458 touches the neighbouring lines; whichever lands second needs a small rebase). Ordinary fix within existing boundaries; scope decision not applicable. ## Contract and boundary impact - Semantic owner and earliest changed stage: ADB provider push verification (`AdbShellOutput` parser → `pushFile` receipt). - Contribution scope fit and linked maintainer decision: not applicable (bug fix). - CLI and MCP/tool-catalog contract: schema unchanged; field values for unparseable checksum output change as described. - Provider, bridge, target-format, or platform compatibility: ADB provider only. - Evidence, artifact, provenance, or reconstruction contract: unparsed checksum output now adds a `raw_result` with `device_sha256sum.stdout/stderr` and the unverified limitation. - Target selection, authority, upstream behavior, and cleanup details (if applicable): same adb commands, arguments and limits. - Generated metadata, package, or installation impact: none. Expected effects (these belong to the existing `push_adb_file` operation; this PR adds none): - [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. ## Evidence and regression coverage - Tests added or updated: `src/adb/AdbProvider.test.ts` "derives the push digest source from …" is a table through the production `AdbProvider` with a simulated adb executable. Cases: valid line, GNU-escaped line, empty stdout, malformed line (63 hex chars), nonzero exit (127). It asserts digest/source, the unverified limitation, and `rawResult`. It replaces the single-case "pushes a file and verifies the device-side digest" test. - Base reproduction: on `6d356b843` the new table failed for escaped, empty and malformed (3 failed, 2 passed). The issue's repro test through `AdbDeviceAnalysisService` on this branch: valid, empty and missing pass. `escaped` now reports a verified digest (`device_sha256sum`) instead of the issue's expected `unavailable`, by design (see Change). - User-visible output (from the issue repro, empty case): `{"device_sha256":null,"device_digest_source":"unavailable","limitations":[…,"The device's sha256sum output contained no recognizable digest, so the transfer is not device-side verified"]}`. - Remaining proof gaps: no real Android device or emulator was contacted. Toybox `sha256sum` output was not observed directly; the parser accepts its documented `<digest> <name>` form. 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/adb/*.test.ts src/application/adb/*.test.ts src/contracts/toolEffects.test.ts`: 8 files, 201 tests passed - Issue #5924 repro test (temporary, removed): valid/empty/missing passed; escaped intentionally differs as described - `npm run check:fast`: passed - `npm run format:check`: passed - `npm run docs:check`: passed ## Compatibility, safety, and release - Breaking changes or migration steps: none. Callers that already treated a null `device_sha256` as unverified are unaffected. - Real Hopper/Ghidra, browser, or OS coverage: macOS with a simulated adb; no device. - Package or release metadata impact: none. - Security, privacy, process, or containment review: `raw_result` holds at most the existing 1 KiB capture of `sha256sum` output for the caller-selected device path. ## Author readiness Human owner: @kaoru0822-kitauji - [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. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
bsakaguchi 2026-11-01 21:45:30 +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#7119
No description provided.