Skip to content

Commit 5e0f912

Browse files
committed
✨ Add intent support and enhance package listing with security fixes
Add shared intent functionality for starting activities - Create common.py with reusable device connection helper - Extract start_intent to intents.py module for use across tools - Add START_INTENT action to android-app tool with extras support - Refactor android-ui tool to use shared intent helper Enhance list_packages with richer output options - Add include_app_name parameter for human-friendly labels - Add include_apk_path toggle for output customization - Add max_packages limit with pagination notes Improve shell command security validation - Refactor assess_command_risk to properly parse chained commands - Add rm to disallowed commands list - Allow uiautomator dump with path restrictions - Add unit tests for security validation edge cases
1 parent d1e9b4c commit 5e0f912

File tree

8 files changed

+287
-76
lines changed

8 files changed

+287
-76
lines changed

droidmind/security.py

Lines changed: 78 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,9 @@ class RiskLevel(Enum):
2727
CRITICAL = auto()
2828

2929

30+
_COMMAND_SEPARATORS: set[str] = {";", "&&", "||", "|", "&"}
31+
32+
3033
# Set of allowed shell commands based on toybox commands
3134
# This is a comprehensive list of generally safe commands
3235
ALLOWED_SHELL_COMMANDS: set[str] = {
@@ -125,6 +128,7 @@ class RiskLevel(Enum):
125128
"screenrecord",
126129
"wm",
127130
"ime",
131+
"uiautomator",
128132
# Text processing
129133
"awk",
130134
"sed",
@@ -170,6 +174,7 @@ class RiskLevel(Enum):
170174
# Commands that are explicitly disallowed due to their destructive potential
171175
DISALLOWED_SHELL_COMMANDS: set[str] = {
172176
# System modification
177+
"rm",
173178
"mkfs",
174179
"mke2fs",
175180
"mkswap",
@@ -269,55 +274,63 @@ def assess_command_risk(command: str) -> RiskLevel:
269274
Returns:
270275
RiskLevel enum indicating the risk level
271276
"""
272-
# Split the command into individual piped commands
273-
piped_commands = [cmd.strip() for cmd in command.split("|")]
274-
275277
highest_risk = RiskLevel.SAFE
276278

277-
for cmd in piped_commands:
278-
# Split the command to get the base command
279-
try:
280-
parts = shlex.split(cmd)
281-
except ValueError:
282-
return RiskLevel.HIGH
283-
284-
if not parts:
279+
try:
280+
tokens = shlex.split(command)
281+
except ValueError:
282+
return RiskLevel.HIGH
283+
284+
# Split token stream into segments separated by shell operators.
285+
segments: list[list[str]] = []
286+
current: list[str] = []
287+
operators: list[str] = []
288+
for token in tokens:
289+
if token in _COMMAND_SEPARATORS:
290+
if current:
291+
segments.append(current)
292+
current = []
293+
operators.append(token)
285294
continue
295+
current.append(token)
296+
if current:
297+
segments.append(current)
286298

287-
base_cmd = parts[0]
299+
for segment in segments:
300+
if not segment:
301+
continue
302+
base_cmd = segment[0]
303+
segment_str = " ".join(segment)
288304

289-
# Check if command is explicitly disallowed
290305
if base_cmd in DISALLOWED_SHELL_COMMANDS:
291306
return RiskLevel.CRITICAL
292-
293-
# Check if command is not in allowed list
294307
if base_cmd not in ALLOWED_SHELL_COMMANDS:
295308
return RiskLevel.HIGH
296309

297-
# Check for suspicious patterns
310+
# uiautomator is safe-ish, but writes UI dumps to disk; treat as MEDIUM.
311+
if base_cmd == "uiautomator" and highest_risk.value < RiskLevel.MEDIUM.value:
312+
highest_risk = RiskLevel.MEDIUM
313+
298314
for pattern in SUSPICIOUS_PATTERNS:
299-
if re.search(pattern, cmd):
315+
if re.search(pattern, segment_str):
300316
return RiskLevel.HIGH
301317

302-
# Check for protected paths
303318
for path in PROTECTED_PATHS:
304-
if f" {path}" in cmd or f"={path}" in cmd or cmd.startswith(path):
305-
# Commands that just read from protected paths are medium risk
319+
if f" {path}" in segment_str or f"={path}" in segment_str or segment_str.startswith(path):
306320
if base_cmd in {"ls", "cat", "head", "tail", "grep", "find"}:
307321
if highest_risk.value < RiskLevel.MEDIUM.value:
308322
highest_risk = RiskLevel.MEDIUM
309323
else:
310-
# Other operations on protected paths are high risk
311324
return RiskLevel.HIGH
312325

313-
# Check for specific risky operations
314-
if ">" in cmd or ">>" in cmd:
326+
if ">" in segment or ">>" in segment:
315327
if highest_risk.value < RiskLevel.MEDIUM.value:
316-
highest_risk = RiskLevel.MEDIUM # File redirection
328+
highest_risk = RiskLevel.MEDIUM
317329

318-
if ";" in cmd or "&&" in cmd:
319-
if highest_risk.value < RiskLevel.MEDIUM.value:
320-
highest_risk = RiskLevel.MEDIUM # Command chaining
330+
# Command chaining increases risk (but should still be allowed if all segments are safe).
331+
if any(op in {";", "&&", "||"} for op in operators):
332+
if highest_risk.value < RiskLevel.MEDIUM.value:
333+
highest_risk = RiskLevel.MEDIUM
321334

322335
return highest_risk
323336

@@ -335,26 +348,53 @@ def validate_shell_command(command: str) -> bool:
335348
Raises:
336349
ValueError: If command is explicitly disallowed or contains suspicious patterns
337350
"""
338-
# Split the command to get the base command
339351
try:
340-
parts = shlex.split(command)
352+
tokens = shlex.split(command)
341353
except ValueError as e:
342354
raise ValueError(f"Invalid command syntax: {e}") from e
343355

344-
if not parts:
356+
if not tokens:
345357
return True
346358

347-
base_cmd = parts[0]
359+
# Split token stream into segments separated by shell operators.
360+
segments: list[list[str]] = []
361+
current: list[str] = []
362+
for token in tokens:
363+
if token in _COMMAND_SEPARATORS:
364+
if current:
365+
segments.append(current)
366+
current = []
367+
continue
368+
current.append(token)
369+
if current:
370+
segments.append(current)
348371

349-
# Check if command is explicitly disallowed
350-
if base_cmd in DISALLOWED_SHELL_COMMANDS:
351-
raise ValueError(f"Command '{base_cmd}' is explicitly disallowed for security reasons")
372+
# Validate each segment independently.
373+
for segment in segments:
374+
if not segment:
375+
continue
376+
base_cmd = segment[0]
352377

353-
# Check if command is not in allowed list
354-
if base_cmd not in ALLOWED_SHELL_COMMANDS:
355-
raise ValueError(f"Command '{base_cmd}' is not in the allowed commands list")
378+
if base_cmd in DISALLOWED_SHELL_COMMANDS:
379+
raise ValueError(f"Command '{base_cmd}' is explicitly disallowed for security reasons")
356380

357-
# Check for suspicious patterns
381+
if base_cmd not in ALLOWED_SHELL_COMMANDS:
382+
raise ValueError(f"Command '{base_cmd}' is not in the allowed commands list")
383+
384+
# Extra restrictions for uiautomator: only allow safe hierarchy dumps.
385+
if base_cmd == "uiautomator":
386+
if len(segment) < 2 or segment[1] != "dump":
387+
raise ValueError("Only 'uiautomator dump' is allowed")
388+
if len(segment) > 3:
389+
raise ValueError("Unsupported uiautomator arguments")
390+
if len(segment) == 3:
391+
output_path = segment[2]
392+
if ".." in output_path:
393+
raise ValueError("Invalid uiautomator output path")
394+
if not output_path.startswith(("/sdcard/", "/data/local/tmp/")):
395+
raise ValueError("uiautomator output must be under /sdcard/ or /data/local/tmp/")
396+
397+
# Check for suspicious patterns across the whole command string.
358398
for pattern in SUSPICIOUS_PATTERNS:
359399
if re.search(pattern, command):
360400
raise ValueError(f"Command contains suspicious pattern: {pattern}")

droidmind/tools/app_management.py

Lines changed: 92 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
from droidmind.context import mcp
1616
from droidmind.devices import get_device_manager
1717
from droidmind.log import logger
18+
from droidmind.tools.intents import start_intent
1819

1920

2021
class AppAction(str, Enum):
@@ -23,6 +24,7 @@ class AppAction(str, Enum):
2324
INSTALL_APP = "install_app"
2425
UNINSTALL_APP = "uninstall_app"
2526
START_APP = "start_app"
27+
START_INTENT = "start_intent"
2628
STOP_APP = "stop_app"
2729
CLEAR_APP_DATA = "clear_app_data"
2830
LIST_PACKAGES = "list_packages"
@@ -443,13 +445,23 @@ async def _clear_app_data_impl(serial: str, package: str, ctx: Context) -> str:
443445
return f"Error clearing app data: {e!s}"
444446

445447

446-
async def _list_packages_impl(serial: str, ctx: Context, include_system_apps: bool = False) -> str:
448+
async def _list_packages_impl(
449+
serial: str,
450+
ctx: Context,
451+
include_system_apps: bool = False,
452+
include_app_name: bool = False,
453+
include_apk_path: bool = True,
454+
max_packages: int | None = 200,
455+
) -> str:
447456
"""
448457
List installed packages on the device.
449458
450459
Args:
451460
serial: Device serial number
452461
include_system_apps: Whether to include system apps in the list
462+
include_app_name: Whether to attempt to include a human-friendly app name
463+
include_apk_path: Whether to include the APK path
464+
max_packages: Maximum number of packages to return
453465
454466
Returns:
455467
Formatted list of installed packages
@@ -467,16 +479,52 @@ async def _list_packages_impl(serial: str, ctx: Context, include_system_apps: bo
467479
if not app_list:
468480
return "No packages found on the device."
469481

470-
result = "# Installed Packages\n\n"
471-
result += "| Package Name | APK Path |\n"
472-
result += "|-------------|----------|\n"
473-
474-
for app in app_list:
482+
result_lines: list[str] = ["# Installed Packages", ""]
483+
484+
truncated_note: str | None = None
485+
effective_list = app_list
486+
if max_packages is not None and 0 < max_packages < len(app_list):
487+
effective_list = app_list[:max_packages]
488+
truncated_note = f"_Showing first {max_packages} of {len(app_list)} packages._"
489+
490+
# If the caller wants app names, we need per-package queries; cap to keep it responsive.
491+
if include_app_name and max_packages is not None and max_packages > 50:
492+
effective_list = effective_list[:50]
493+
truncated_note = "_Showing first 50 packages (app names require per-package queries)._"
494+
495+
if truncated_note:
496+
result_lines.append(truncated_note)
497+
result_lines.append("")
498+
499+
columns: list[str] = []
500+
if include_app_name:
501+
columns.append("App Name")
502+
columns.append("Package Name")
503+
if include_apk_path:
504+
columns.append("APK Path")
505+
506+
result_lines.append("| " + " | ".join(columns) + " |")
507+
result_lines.append("|" + "|".join(["-" * (len(col) + 2) for col in columns]) + "|")
508+
509+
async def get_app_name(package: str) -> str:
510+
# Best-effort extraction from dumpsys; varies by Android version/vendor.
511+
cmd = f'dumpsys package {package} | grep "application-label" | head -n 1'
512+
line = await device.run_shell(cmd)
513+
match = re.search(r"application-label(?:-[^:]+)?:\\s*'?([^'\\r\\n]+)'?", line)
514+
return match.group(1).strip() if match else "Unknown"
515+
516+
for app in effective_list:
475517
package_name = app.get("package", "Unknown")
476518
apk_path = app.get("path", "Unknown")
477-
result += f"| `{package_name}` | `{apk_path}` |\n"
478-
479-
return result
519+
row: list[str] = []
520+
if include_app_name:
521+
row.append(await get_app_name(package_name))
522+
row.append(f"`{package_name}`")
523+
if include_apk_path:
524+
row.append(f"`{apk_path}`")
525+
result_lines.append("| " + " | ".join(row) + " |")
526+
527+
return "\n".join(result_lines)
480528
except Exception as e:
481529
logger.exception("Error listing packages: %s", e)
482530
return f"Error listing packages: {e!s}"
@@ -701,6 +749,7 @@ async def _get_app_info_impl(serial: str, package: str, ctx: Context) -> str:
701749

702750
@mcp.tool(name="android-app")
703751
async def app_operations(
752+
# pylint: disable=too-many-arguments
704753
serial: str,
705754
action: AppAction,
706755
ctx: Context,
@@ -710,7 +759,11 @@ async def app_operations(
710759
grant_permissions: bool = True, # For install_app
711760
keep_data: bool = False, # For uninstall_app
712761
activity: str = "", # For start_app
762+
extras: dict[str, str] | None = None, # For start_intent
713763
include_system_apps: bool = False, # For list_packages
764+
include_app_name: bool = False, # For list_packages
765+
include_apk_path: bool = True, # For list_packages
766+
max_packages: int | None = 200, # For list_packages
714767
) -> str:
715768
"""
716769
Perform various application management operations on an Android device.
@@ -728,7 +781,11 @@ async def app_operations(
728781
grant_permissions (Optional[bool]): Whether to grant all requested permissions. Used by `install_app`.
729782
keep_data (Optional[bool]): Whether to keep app data and cache directories. Used by `uninstall_app`.
730783
activity (Optional[str]): Optional activity name to start. Used by `start_app`.
784+
extras (Optional[dict[str, str]]): Optional intent extras. Used by `start_intent`.
731785
include_system_apps (Optional[bool]): Whether to include system apps. Used by `list_packages`.
786+
include_app_name (Optional[bool]): Whether to include app labels (best-effort). Used by `list_packages`.
787+
include_apk_path (Optional[bool]): Whether to include APK paths. Used by `list_packages`.
788+
max_packages (Optional[int]): Max packages to return. Used by `list_packages`.
732789
733790
Returns:
734791
A string message indicating the result or status of the operation.
@@ -745,12 +802,15 @@ async def app_operations(
745802
3. `action="start_app"`
746803
- Requires: `package`
747804
- Optional: `activity`
805+
3b. `action="start_intent"`
806+
- Requires: `package`, `activity`
807+
- Optional: `extras`
748808
4. `action="stop_app"`
749809
- Requires: `package`
750810
5. `action="clear_app_data"`
751811
- Requires: `package`
752812
6. `action="list_packages"`
753-
- Optional: `include_system_apps`
813+
- Optional: `include_system_apps`, `include_app_name`, `include_apk_path`, `max_packages`
754814
7. `action="get_app_manifest"`
755815
- Requires: `package`
756816
8. `action="get_app_permissions"`
@@ -768,6 +828,7 @@ async def app_operations(
768828
in [
769829
AppAction.UNINSTALL_APP,
770830
AppAction.START_APP,
831+
AppAction.START_INTENT,
771832
AppAction.STOP_APP,
772833
AppAction.CLEAR_APP_DATA,
773834
AppAction.GET_APP_MANIFEST,
@@ -782,6 +843,9 @@ async def app_operations(
782843
if action == AppAction.INSTALL_APP and apk_path is None:
783844
return "❌ Error: 'apk_path' is required for action 'install_app'."
784845

846+
if action == AppAction.START_INTENT and not activity:
847+
return "❌ Error: 'activity' is required for action 'start_intent'."
848+
785849
# Dispatch to implementations
786850
if action == AppAction.INSTALL_APP:
787851
# We already checked apk_path is not None
@@ -790,12 +854,29 @@ async def app_operations(
790854
return await _uninstall_app_impl(serial, package, ctx, keep_data) # type: ignore
791855
if action == AppAction.START_APP:
792856
return await _start_app_impl(serial, package, ctx, activity) # type: ignore
857+
if action == AppAction.START_INTENT:
858+
assert package is not None
859+
return await start_intent(
860+
serial=serial,
861+
package=package,
862+
activity=activity,
863+
ctx=ctx,
864+
extras=extras,
865+
device_manager=get_device_manager(),
866+
) # type: ignore[arg-type]
793867
if action == AppAction.STOP_APP:
794868
return await _stop_app_impl(serial, package, ctx) # type: ignore
795869
if action == AppAction.CLEAR_APP_DATA:
796870
return await _clear_app_data_impl(serial, package, ctx) # type: ignore
797871
if action == AppAction.LIST_PACKAGES:
798-
return await _list_packages_impl(serial, ctx, include_system_apps)
872+
return await _list_packages_impl(
873+
serial,
874+
ctx,
875+
include_system_apps=include_system_apps,
876+
include_app_name=include_app_name,
877+
include_apk_path=include_apk_path,
878+
max_packages=max_packages,
879+
)
799880
if action == AppAction.GET_APP_MANIFEST:
800881
return await _get_app_manifest_impl(serial, package, ctx) # type: ignore
801882
if action == AppAction.GET_APP_PERMISSIONS:

0 commit comments

Comments
 (0)