morluto/rea

Fork 0

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

[GH-ISSUE #7409] [Bug] inspect-adb-package treats an ID with attached letters as a valid declaration #4416

New issue

Open

opened 2026-10-31 00:49:37 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 00:49:37 +00:00

Owner

Originally created by @coygeek on GitHub (Oct 27, 2026).
Original GitHub issue: https://github.com/morluto/rea/issues/7409

Before submitting

I searched the open and closed issues for a related report.
I removed credentials, tokens, private paths, and sensitive target data.

Area

Process capture or runtime observation

Observed behavior

With a local adb stand-in returning userId=10123abc, the production inspect-adb-package CLI returned user_id: 10123, no numeric issues, and complete coverage. The malformed declaration also prevented a later valid appId=10456 from being selected. When the valid declaration came first, its value survived, but the later malformed declaration was still absent from the diagnostics and coverage remained complete.

These are observed CLI results from synthetic malformed output. No real Android device or emulator was used, and no real-device occurrence of the attached-letter shape has been established.

Reproduction

Use an empty working directory beside the built baseline checkout, with that checkout available as ../rea. Put Node 24 on PATH. Save the following self-contained probe as probe.py; it creates a local executable and package fixtures, redirects HOME, and accepts only the adb calls used by this operation.

import json, os, pathlib, subprocess
root=pathlib.Path.cwd()
(root/'home').mkdir(exist_ok=True)
adb=root/'fake-adb'
adb.write_text("""#!/bin/sh
if [ "$1" = version ]; then
  printf 'Android Debug Bridge version 1.0.41\nVersion 34.0.5-fixture\n'
  exit 0
fi
if [ "$1" = -s ] && [ "$2" = fixture-device ] && [ "$3" = shell ] && [ "$4" = dumpsys ] && [ "$5" = package ] && [ "$6" = com.example.app ]; then
  cat "$ADB_FIXTURE"
  exit 0
fi
exit 91
""")
adb.chmod(0o755)
cases=[('malformed-1','userId=10123abc'),('malformed-2','userId=10123abc'),('valid','userId=10123'),('nonnumeric','userId=abc'),('malformed-appid','appId=10123abc'),('malformed-before-valid','userId=10123abc\n    appId=10456'),('valid-before-malformed','userId=10456\n    appId=10123abc'),('unsafe','userId=9007199254740992'),('zero','userId=0')]
for name,declaration in cases:
    fixture=root/'package.txt'
    fixture.write_text('Packages:\n  Package [com.example.app] (abcdef):\n    '+declaration+'\n    pkg=Package{abcdef com.example.app}\n    versionCode=42 minSdk=24 targetSdk=34\n    versionName=1.0\n    requested permissions:\n      android.permission.INTERNET\n')
    env={'HOME':str(root/'home'),'PATH':os.environ['PATH'],'REA_ADB_PATH':str(adb),'ADB_FIXTURE':str(fixture)}
    run=subprocess.run(['node','../rea/scripts/rea.mjs','inspect-adb-package','fixture-device','com.example.app','--json','--full-output'],env=env,text=True,capture_output=True)
    (root/(name+'.json')).write_text(run.stdout)
    assert run.returncode==0,(name,run.returncode,run.stdout,run.stderr)
    assert not run.stderr,(name,run.stderr)
    evidence=json.loads(run.stdout)
    result=evidence.get('data',evidence)['normalized_result']
    print(json.dumps({'case':name,**{key:result[key] for key in ['user_id','numeric_issues','coverage']}},separators=(',',':')))

Run the probe from that empty working directory:

python3 probe.py > results.jsonl
cat results.jsonl

Compare the malformed cases with the controls. All nine calls exited 0 with empty stderr. The single malformed userId case failed in both attempts; the malformed appId and each declaration-order case failed once in one attempt. The four controls each ran once. The exact relevant printed lines were:

{"case":"malformed-1","user_id":10123,"numeric_issues":{"sample":[],"total":0},"coverage":"complete"}
{"case":"malformed-2","user_id":10123,"numeric_issues":{"sample":[],"total":0},"coverage":"complete"}
{"case":"valid","user_id":10123,"numeric_issues":{"sample":[],"total":0},"coverage":"complete"}
{"case":"nonnumeric","user_id":null,"numeric_issues":{"sample":[{"command":"dumpsys package","field":"user_id","reason":"invalid_format","raw_line":"    userId=abc"}],"total":1},"coverage":"partial"}
{"case":"malformed-appid","user_id":10123,"numeric_issues":{"sample":[],"total":0},"coverage":"complete"}
{"case":"malformed-before-valid","user_id":10123,"numeric_issues":{"sample":[],"total":0},"coverage":"complete"}
{"case":"valid-before-malformed","user_id":10456,"numeric_issues":{"sample":[],"total":0},"coverage":"complete"}
{"case":"unsafe","user_id":null,"numeric_issues":{"sample":[{"command":"dumpsys package","field":"user_id","reason":"unsafe_integer","raw_line":"    userId=9007199254740992"}],"total":1},"coverage":"partial"}
{"case":"zero","user_id":0,"numeric_issues":{"sample":[],"total":0},"coverage":"complete"}

Expected behavior

Under "Interpret the result", docs/adb-device-analysis.md states: "Malformed declarations before or after a valid value remain in numeric_issues" and "Any issue makes package coverage partial". The same bullet selects the first valid nonnegative safe integer declaration, including zero. An identifier token containing attached alphabetic characters should be retained as malformed rather than replaced by its digit prefix. If no valid declaration exists, user_id should remain null; if a later valid declaration exists, that value should be selected. Either ordering must retain the malformed declaration and report partial coverage.

Whitespace-separated suffixes are a separate supported case in the existing acceptance tests. This report does not propose rejecting those suffixes or changing first-valid selection for well-formed declarations.

Impact and scope

The executed malformed fixtures produced an incorrect package ID or concealed a malformed declaration while claiming complete coverage. An invalid declaration before a valid one changed which ID the caller received. The observed scope is five failing CLI calls against local synthetic output; well-formed, nonnumeric, unsafe-integer, and zero controls behaved consistently with the documented contract. Real-device prevalence, downstream effects, other platforms, and MCP delivery were not measured.

REA version

6.8.0 (source commit a905a71e31)

Runtime and platform

Node 24.22.1; macOS 27.0.1 arm64.

Target and artifact context

Synthetic single-package dumpsys package text for com.example.app, with a loaded parsed package, version metadata, and one requested permission. The local adb stand-in reports 34.0.5-fixture; it contacts no device.

Interface

CLI

Earliest failure stage

Capture or observation

Relevant output or evidence

Observed evidence is the nine CLI responses printed above and the complete JSON saved by the probe. Baseline identity was confirmed with git rev-parse HEAD. npm view rea-agents version returned 6.8.0, matching the baseline package version; an independently installed npm runtime was not executed.

The documented expected behavior is at docs/adb-device-analysis.md:139. CLI dispatch is at src/cli/adbCommands.ts:133, numeric admission at src/adb/AdbShellOutput.ts:424, and provider projection/coverage at src/adb/AdbProvider.ts:1575. Existing accepted whitespace-suffix examples are at tests/acceptance/applications/adbNumericDeclarations.test.ts:347.

Source-only producer check, not a real-device reproduction: release-tagged Android 8 and 12 print a package integer with println after userId=; the inspected Android 14 and 16 sources do so after appId=. These producers do not explain an attached alphabetic suffix. The report concerns REA's explicit malformed-input guarantee; occurrence on an OEM implementation is unknown.

Typed outcome or recovery expected

For userId=10123abc, retain an invalid_format numeric issue containing the original line. Return null when it is the only ID declaration, select 10456 when a valid declaration supplies that value, and mark coverage partial in both cases. Keep the issue when a valid declaration appeared earlier too.

Analysis or suspected cause

Source-derived: src/adb/AdbShellOutput.ts:425 matches a digit prefix and passes only those digits to the safe-integer check. That loses the attached text before the validity decision. An admitted prefix then participates in first-valid selection, while the empty diagnostic collection lets src/adb/AdbProvider.ts:1592 report complete coverage. Existing acceptance tests deliberately allow suffixes separated by whitespace, so requiring the entire trimmed line to consist of digits would change an existing accepted behavior. MCP shares the operation according to source; it was not executed.

Additional context

The open and closed issue/PR search found no exact duplicate. #5426 concerns recognition of the appId spelling, and #6584 concerns Unicode permission counting; neither asks for malformed-ID token diagnostics. The specifically checked #6440 concerns APK signer comparison and is unrelated. The baseline already includes the documented numeric-diagnostic machinery; its handling of attached suffixes still violates that contract.

Originally created by @coygeek on GitHub (Oct 27, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/7409 ### Before submitting - [x] I searched the open and closed issues for a related report. - [x] I removed credentials, tokens, private paths, and sensitive target data. ### Area Process capture or runtime observation ### Observed behavior With a local adb stand-in returning `userId=10123abc`, the production `inspect-adb-package` CLI returned `user_id: 10123`, no numeric issues, and complete coverage. The malformed declaration also prevented a later valid `appId=10456` from being selected. When the valid declaration came first, its value survived, but the later malformed declaration was still absent from the diagnostics and coverage remained complete. These are observed CLI results from synthetic malformed output. No real Android device or emulator was used, and no real-device occurrence of the attached-letter shape has been established. ### Reproduction 1. Use an empty working directory beside the built baseline checkout, with that checkout available as `../rea`. Put Node 24 on PATH. Save the following self-contained probe as `probe.py`; it creates a local executable and package fixtures, redirects HOME, and accepts only the adb calls used by this operation. ```python import json, os, pathlib, subprocess root=pathlib.Path.cwd() (root/'home').mkdir(exist_ok=True) adb=root/'fake-adb' adb.write_text("""#!/bin/sh if [ "$1" = version ]; then printf 'Android Debug Bridge version 1.0.41\nVersion 34.0.5-fixture\n' exit 0 fi if [ "$1" = -s ] && [ "$2" = fixture-device ] && [ "$3" = shell ] && [ "$4" = dumpsys ] && [ "$5" = package ] && [ "$6" = com.example.app ]; then cat "$ADB_FIXTURE" exit 0 fi exit 91 """) adb.chmod(0o755) cases=[('malformed-1','userId=10123abc'),('malformed-2','userId=10123abc'),('valid','userId=10123'),('nonnumeric','userId=abc'),('malformed-appid','appId=10123abc'),('malformed-before-valid','userId=10123abc\n appId=10456'),('valid-before-malformed','userId=10456\n appId=10123abc'),('unsafe','userId=9007199254740992'),('zero','userId=0')] for name,declaration in cases: fixture=root/'package.txt' fixture.write_text('Packages:\n Package [com.example.app] (abcdef):\n '+declaration+'\n pkg=Package{abcdef com.example.app}\n versionCode=42 minSdk=24 targetSdk=34\n versionName=1.0\n requested permissions:\n android.permission.INTERNET\n') env={'HOME':str(root/'home'),'PATH':os.environ['PATH'],'REA_ADB_PATH':str(adb),'ADB_FIXTURE':str(fixture)} run=subprocess.run(['node','../rea/scripts/rea.mjs','inspect-adb-package','fixture-device','com.example.app','--json','--full-output'],env=env,text=True,capture_output=True) (root/(name+'.json')).write_text(run.stdout) assert run.returncode==0,(name,run.returncode,run.stdout,run.stderr) assert not run.stderr,(name,run.stderr) evidence=json.loads(run.stdout) result=evidence.get('data',evidence)['normalized_result'] print(json.dumps({'case':name,**{key:result[key] for key in ['user_id','numeric_issues','coverage']}},separators=(',',':'))) ``` 2. Run the probe from that empty working directory: ```sh python3 probe.py > results.jsonl cat results.jsonl ``` 3. Compare the malformed cases with the controls. All nine calls exited 0 with empty stderr. The single malformed `userId` case failed in both attempts; the malformed `appId` and each declaration-order case failed once in one attempt. The four controls each ran once. The exact relevant printed lines were: ```json {"case":"malformed-1","user_id":10123,"numeric_issues":{"sample":[],"total":0},"coverage":"complete"} {"case":"malformed-2","user_id":10123,"numeric_issues":{"sample":[],"total":0},"coverage":"complete"} {"case":"valid","user_id":10123,"numeric_issues":{"sample":[],"total":0},"coverage":"complete"} {"case":"nonnumeric","user_id":null,"numeric_issues":{"sample":[{"command":"dumpsys package","field":"user_id","reason":"invalid_format","raw_line":" userId=abc"}],"total":1},"coverage":"partial"} {"case":"malformed-appid","user_id":10123,"numeric_issues":{"sample":[],"total":0},"coverage":"complete"} {"case":"malformed-before-valid","user_id":10123,"numeric_issues":{"sample":[],"total":0},"coverage":"complete"} {"case":"valid-before-malformed","user_id":10456,"numeric_issues":{"sample":[],"total":0},"coverage":"complete"} {"case":"unsafe","user_id":null,"numeric_issues":{"sample":[{"command":"dumpsys package","field":"user_id","reason":"unsafe_integer","raw_line":" userId=9007199254740992"}],"total":1},"coverage":"partial"} {"case":"zero","user_id":0,"numeric_issues":{"sample":[],"total":0},"coverage":"complete"} ``` ### Expected behavior Under "Interpret the result", `docs/adb-device-analysis.md` states: "Malformed declarations before or after a valid value remain in `numeric_issues`" and "Any issue makes package coverage partial". The same bullet selects the first valid nonnegative safe integer declaration, including zero. An identifier token containing attached alphabetic characters should be retained as malformed rather than replaced by its digit prefix. If no valid declaration exists, `user_id` should remain null; if a later valid declaration exists, that value should be selected. Either ordering must retain the malformed declaration and report partial coverage. Whitespace-separated suffixes are a separate supported case in the existing acceptance tests. This report does not propose rejecting those suffixes or changing first-valid selection for well-formed declarations. ### Impact and scope The executed malformed fixtures produced an incorrect package ID or concealed a malformed declaration while claiming complete coverage. An invalid declaration before a valid one changed which ID the caller received. The observed scope is five failing CLI calls against local synthetic output; well-formed, nonnumeric, unsafe-integer, and zero controls behaved consistently with the documented contract. Real-device prevalence, downstream effects, other platforms, and MCP delivery were not measured. ### REA version 6.8.0 (source commit a905a71e31f71457cb21fd056b234f872126974e) ### Runtime and platform Node 24.22.1; macOS 27.0.1 arm64. ### Target and artifact context Synthetic single-package `dumpsys package` text for `com.example.app`, with a loaded parsed package, version metadata, and one requested permission. The local adb stand-in reports 34.0.5-fixture; it contacts no device. ### Interface CLI ### Earliest failure stage Capture or observation ### Relevant output or evidence Observed evidence is the nine CLI responses printed above and the complete JSON saved by the probe. Baseline identity was confirmed with `git rev-parse HEAD`. `npm view rea-agents version` returned 6.8.0, matching the baseline package version; an independently installed npm runtime was not executed. The documented expected behavior is at `docs/adb-device-analysis.md:139`. CLI dispatch is at `src/cli/adbCommands.ts:133`, numeric admission at `src/adb/AdbShellOutput.ts:424`, and provider projection/coverage at `src/adb/AdbProvider.ts:1575`. Existing accepted whitespace-suffix examples are at `tests/acceptance/applications/adbNumericDeclarations.test.ts:347`. Source-only producer check, not a real-device reproduction: release-tagged Android [8](https://github.com/aosp-mirror/platform_frameworks_base/blob/android-8.0.0_r1/services/core/java/com/android/server/pm/Settings.java#L4627) and [12](https://github.com/aosp-mirror/platform_frameworks_base/blob/android-12.0.0_r1/services/core/java/com/android/server/pm/Settings.java#L4449) print a package integer with `println` after `userId=`; the inspected Android [14](https://github.com/aosp-mirror/platform_frameworks_base/blob/android-14.0.0_r1/services/core/java/com/android/server/pm/Settings.java#L4703) and [16](https://github.com/aosp-mirror/platform_frameworks_base/blob/android-16.0.0_r1/services/core/java/com/android/server/pm/Settings.java#L5139) sources do so after `appId=`. These producers do not explain an attached alphabetic suffix. The report concerns REA's explicit malformed-input guarantee; occurrence on an OEM implementation is unknown. ### Typed outcome or recovery expected For `userId=10123abc`, retain an `invalid_format` numeric issue containing the original line. Return null when it is the only ID declaration, select 10456 when a valid declaration supplies that value, and mark coverage partial in both cases. Keep the issue when a valid declaration appeared earlier too. ### Analysis or suspected cause Source-derived: `src/adb/AdbShellOutput.ts:425` matches a digit prefix and passes only those digits to the safe-integer check. That loses the attached text before the validity decision. An admitted prefix then participates in first-valid selection, while the empty diagnostic collection lets `src/adb/AdbProvider.ts:1592` report complete coverage. Existing acceptance tests deliberately allow suffixes separated by whitespace, so requiring the entire trimmed line to consist of digits would change an existing accepted behavior. MCP shares the operation according to source; it was not executed. ### Additional context The open and closed issue/PR search found no exact duplicate. [#5426](https://github.com/morluto/rea/issues/5426) concerns recognition of the `appId` spelling, and [#6584](https://github.com/morluto/rea/issues/6584) concerns Unicode permission counting; neither asks for malformed-ID token diagnostics. The specifically checked [#6440](https://github.com/morluto/rea/issues/6440) concerns APK signer comparison and is unrelated. The baseline already includes the documented numeric-diagnostic machinery; its handling of attached suffixes still violates that contract.

bsakaguchi referenced this issue

2026-10-31 01:12:51 +00:00

[PR #4416] [MERGED] fix(dotnet): reject unfinished CIL prefix chains #6227

bsakaguchi referenced this issue

2026-10-31 01:13:19 +00:00

[PR #4528] [CLOSED] fix(dotnet): reject unfinished CIL prefix chain at end of method body #6288

bsakaguchi referenced this issue

2026-11-01 21:10:53 +00:00

[GH-ISSUE #3582] [Tracker] Ready to close or merge: issues and pull requests (October 2026) #1330