Skip to content

fix(adb): mark unknown permission coverage partial - #4663

Merged
morluto merged 1 commit into
morluto:mainfrom
gongfpp:fix/unknown-package-permission-count
Oct 17, 2026
Merged

morluto merged 1 commit into
morluto:mainfrom
gongfpp:fix/unknown-package-permission-count

Conversation

@gongfpp

@gongfpp gongfpp commented Oct 17, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Follow-up to #4665 for Issue #4654. The merged change handles explicit pkg=null; this PR covers unestablished package state and keeps permission coverage visibly partial instead of reporting a complete zero.

Expected behavior

  • pkg=null with no requested-permissions section: requested_permissions_count: null, coverage: "partial".
  • No recognized pkg value and no requested-permissions section: also unknown (null / partial).
  • A confirmed loaded pkg=Package{...} with no requested-permissions section: known empty (0 / complete).
  • When the requested-permissions section is present, count its listed entries.

Change and scope

  • Track whether the permission section was present and whether dumpsys established a loaded package.
  • Mark package analysis partial whenever the requested-permissions count is unknown.
  • Keep the existing nullable output contract; no schema, CLI/MCP contract, generated metadata, or device-operation changes.
  • ADB package analysis only. The PR is a follow-up to the explicit pkg=null fix merged in Fix permissions count issue for pkg=null in ADB analysis and add test cases #4665, not a second implementation of that same fix.

Evidence and regression coverage

  • Fake-ADB/provider tests cover explicit pkg=null, missing pkg, a confirmed loaded package with no permission section, and a populated permission list.
  • Parser/provider focused tests: npm run test:local -- src/adb/AdbProvider.test.ts src/adb/AdbShellOutput.test.ts — 90 passed.
  • npm run check:fast — passed.
  • npm run build:cached — passed; emitted two existing INEFFECTIVE_DYNAMIC_IMPORT warnings.
  • No physical Android device was used.
  • Fresh current-head CI run 39017514115 passed all selected required checks: source tests, static checks, build package, all five test shards, Portable tests, CI required, and native package lanes. PR title check run 39017519480 passed; optional lanes were skipped by change selection.
  • Baseline main CI run 39016861665 passed for base a04053347034fb2a5061a927eebd0b5a6ef772c3.

Contract and boundary impact

  • Semantic owner: dumpsys package parser, then ADB package provider projection.
  • Unsupported or incomplete permission data remains explicit as null with partial coverage.
  • No target process interaction, mutation, or new dependency.

Author readiness

Human owner: @gongfpp

  • 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

  • Required CI is green for the current PR head.
  • I checked the final diff for secrets, unrelated changes, and unsupported claims.
  • Maintainer review and merge decision are complete.

@gongfpp
gongfpp force-pushed the fix/unknown-package-permission-count branch from 894ae21 to 03809a8 Compare October 17, 2026 03:55
@gongfpp
gongfpp marked this pull request as ready for review October 17, 2026 05:03
@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ⚠️ Failed 2026-10-17T05:03:59.089265Z 03809a8 Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Repo admins can enable using credits for code reviews in their settings.

@greptile-apps

greptile-apps Bot commented Oct 17, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerView in GreptileConfidence Score: 5/5

[Medium impact] The reviewed changes appear safe to merge, subject to the required checks passing.

Summary

Preserves unknown permission counts from dumpsys package instead of reporting an unsupported zero.

  • inspect_adb_package distinguishes an empty permission list from an unknown count.

No new actionable issues or security concerns were identified in the changed code. Tests were inspected, not run. Checking whether this PR was superseded was blocked by missing GitHub authentication.

Reviews (4) · Last reviewed commit: "fix(adb): mark unknown package coverage ..." · Reviewed by Greptile

Comment thread src/adb/AdbProvider.test.ts Resolved
@gongfpp
gongfpp force-pushed the fix/unknown-package-permission-count branch 4 times, most recently from 6996490 to 106e559 Compare October 17, 2026 07:37
@gongfpp
gongfpp force-pushed the fix/unknown-package-permission-count branch from 106e559 to 0c06f1c Compare October 17, 2026 08:28
@gongfpp gongfpp changed the title fix(adb): preserve unknown package permission counts fix(adb): mark unknown permission coverage partial Oct 17, 2026
@morluto
morluto merged commit 9534e42 into morluto:main Oct 17, 2026
34 of 35 checks passed
@morluto

morluto commented Oct 20, 2026

Copy link
Copy Markdown
Owner

Thanks @gongfpp for clarifying when Android permission coverage is actually established. Marking unknown package state or an unestablished requested-permissions count as partial keeps missing evidence from looking like a complete zero.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants