(fix) update tests and minor bugfixes

CONTRIBUTING.md

@@ -21,7 +21,25 @@ Whether you want to contribute to the TypeScript agent or the Python command lin
 
 ### Python Command Line
 
-Any Python 3 environment should do, but we recommend you use the latest version of Python. To satisfy all of the dependencies that you may need, install those defined in the [`requirements-dev.txt`](https://github.com/sensepost/objection/blob/master/requirements-dev.txt) file in the project's root. This would make all of the code dependencies available, as well as some useful debugging helpers.
+Any Python 3 environment should do, but we recommend you use the latest version of Python. To satisfy all of the dependencies that you may need, install the development dependency group defined in `pyproject.toml`:
+
+```zsh
+uv sync --group dev
+```
+
+This makes the code dependencies available, along with pytest and other useful development helpers.
+
+To run the test suite, you can then use:
+
+```zsh
+uv run pytest
+```
+
+or:
+
+```zsh
+make test
+```
 
 ### TypeScript Agent
 

Makefile

@@ -11,6 +11,9 @@ frida-agent:
 sdist:
 	uv build
 
+test:
+	uv run pytest
+
 testupload:
 	uv publish --index testpypi
 

objection/commands/filemanager.py

@@ -8,7 +8,7 @@
 from ..state.connection import state_connection
 from ..state.device import device_state, Ios, Android
 from ..state.filemanager import file_manager_state
-from ..utils.helpers import sizeof_fmt
+from ..utils.helpers import sizeof_fmt, is_unix_absolute_path
 
 # variable used to cache entries from the ls-like
 # commands used in the below helpers. only used
@@ -74,7 +74,7 @@ def cd(args: list) -> None:
 
     # if we got an absolute path, check if the path
     # actually exists, and then cd to it if we can
-    if os.path.isabs(path):
+    if is_unix_absolute_path(path):
 
         # assume the path does not exist by default
         does_exist = False
@@ -258,7 +258,7 @@ def ls(args: list) -> None:
         path = pwd()
     else:
         path = args[0]
-        if not os.path.isabs(path):
+        if not is_unix_absolute_path(path):
             path = device_state.platform.path_separator.join([pwd(), path])
 
     # based on the runtime, execute the correct ls method.
@@ -414,9 +414,12 @@ def download(args: list) -> None:
     # if we didnt get a specification of where to dump the file,
     # assume the same name should be used locally.
     source = args[0]
-    destination = args[1] if len(args) > 1 else os.path.basename(source)
-
     should_download_folder = _should_download_folder(args)
+    # If the user specified a destination, use it. Otherwise, use the basename of the source.
+    if len(args) > (1 + should_download_folder):
+        destination = args[1]
+    else:
+        destination = os.path.basename(source)
 
     if device_state.platform == Ios:
         _download_ios(source, destination, should_download_folder)
@@ -436,7 +439,7 @@ def _download_ios(path: str, destination: str, should_download_folder: bool, pat
 
     # if the path we got is not absolute, join it with the
     # current working directory
-    if not os.path.isabs(path):
+    if not is_unix_absolute_path(path):
         path = device_state.platform.path_separator.join([pwd(), path])
 
     api = state_connection.get_api()
@@ -500,7 +503,7 @@ def _download_android(path: str, destination: str, should_download_folder: bool,
 
     # if the path we got is not absolute, join it with the
     # current working directory
-    if not os.path.isabs(path):
+    if not is_unix_absolute_path(path):
         path = device_state.platform.path_separator.join([pwd(), path])
 
     api = state_connection.get_api()
@@ -587,7 +590,7 @@ def _upload_ios(path: str, destination: str) -> None:
         :return:
     """
 
-    if not os.path.isabs(destination):
+    if not is_unix_absolute_path(destination):
         destination = device_state.platform.path_separator.join([pwd(), destination])
 
     api = state_connection.get_api()
@@ -622,7 +625,7 @@ def _upload_android(path: str, destination: str) -> None:
         :return:
     """
 
-    if not os.path.isabs(destination):
+    if not is_unix_absolute_path(destination):
         destination = device_state.platform.path_separator.join([pwd(), destination])
 
     api = state_connection.get_api()
@@ -662,7 +665,7 @@ def rm(args: list) -> None:
 
     target = args[0]
 
-    if not os.path.isabs(target):
+    if not is_unix_absolute_path(target):
         target = device_state.platform.path_separator.join([pwd(), target])
 
     if not click.confirm('Really delete {0} ?'.format(target)):

objection/commands/ios/plist.py

@@ -1,10 +1,9 @@
-import os
-
 import click
 
 from objection.commands import filemanager
 from objection.state.connection import state_connection
 from objection.state.device import device_state
+from objection.utils.helpers import is_unix_absolute_path
 
 
 def cat(args: list = None) -> None:
@@ -22,7 +21,7 @@ def cat(args: list = None) -> None:
 
     plist = args[0]
 
-    if not os.path.isabs(plist):
+    if not is_unix_absolute_path(plist):
         pwd = filemanager.pwd()
         plist = device_state.platform.path_separator.join([pwd, plist])
 

objection/commands/sqlite.py

@@ -7,6 +7,7 @@
 from litecli.main import LiteCli
 
 from ..commands.filemanager import download, upload, pwd, path_exists
+from ..utils.helpers import is_unix_absolute_path
 
 
 def modify_config(rc):
@@ -73,7 +74,7 @@ def connect(args: list) -> None:
 
     # update the full remote path for future syncs
     full_remote_file = db_location \
-        if os.path.isabs(db_location) else os.path.join(pwd(), db_location)
+        if is_unix_absolute_path(db_location) else os.path.join(pwd(), db_location)
 
     click.secho('Caching local copy of database file...', fg='green')
     download([db_location, local_path])

objection/console/cli.py

@@ -285,7 +285,7 @@ def version() -> None:
 @click.option('--script-source', '-l', default=None, help=(
         'A script file to use with the the "path" config type. '
         'Remember that use the name of this file in your "path". It will be next to the config.'), show_default=False)
-@click.option('--bundle-id', '-b', default=None, help='The bundleid to set when codesigning the IPA')
+@click.option('--bundle-id', '-B', default=None, help='The bundleid to set when codesigning the IPA')
 def patchipa(source: str, gadget_version: str, codesign_signature: str, provision_file: str, binary_name: str,
              skip_cleanup: bool, pause: bool, unzip_unicode: bool, gadget_config: str, script_source: str,
              bundle_id: str) -> None:

objection/console/completer.py

@@ -40,12 +40,15 @@ def find_completions(self, document: Document) -> dict:
         # how a shell invocation would have been done.
         # we will also cleanup flags that come in the form
         #  of --flag so that multiples can be suggested.
-        tokens = [token for token in get_tokens(document.text) if not token.startswith('--')]
+        all_tokens = get_tokens(document.text)
+        tokens = [token for token in all_tokens if not token.startswith('--')]
 
         # extract the flags in the received tokens. This list
         # will be used to remove suggested flags from those
         # already present in the command.
-        flags = [flag for flag in get_tokens(document.text) if flag.startswith('--')]
+        flags = [flag for flag in all_tokens if flag.startswith('--')]
+        has_trailing_space = document.text.endswith(' ')
+        active_word = document.get_word_before_cursor()
 
         # start with the current suggestions dictionary being
         # all commands
@@ -56,7 +59,7 @@ def find_completions(self, document: Document) -> dict:
         #   command sub_command sub_sub_command
         # so, lets use that and search the the COMMAND dictionary for
         # the last dictionary with a correct suggestion
-        for token in tokens:
+        for i, token in enumerate(tokens):
 
             candidate = token.lower()
 
@@ -66,6 +69,26 @@ def find_completions(self, document: Document) -> dict:
                 if 'commands' in current_suggestions[candidate]:
                     current_suggestions = current_suggestions[candidate]['commands']
 
+                # Some commands can have both dynamic and flag completions.
+                # In that case, complete the first positional argument from
+                # dynamic suggestions, then switch to flag suggestions.
+                elif 'dynamic' in current_suggestions[candidate] and 'flags' in current_suggestions[candidate]:
+                    positional_args = tokens[i + 1:]
+
+                    completed_positional_args = len(positional_args)
+                    if (not has_trailing_space
+                            and active_word
+                            and not active_word.startswith('--')
+                            and completed_positional_args > 0):
+                        completed_positional_args -= 1
+
+                    if completed_positional_args == 0:
+                        current_suggestions = current_suggestions[candidate]['dynamic']()
+                    else:
+                        current_suggestions = {
+                            flag: '' for flag in current_suggestions[candidate]['flags'] if flag not in flags
+                        }
+
                 # dynamic commands change based on the current status of the
                 # environment, so, call the method defined
                 elif 'dynamic' in current_suggestions[candidate]:

objection/state/jobs.py

@@ -38,7 +38,8 @@ def end(self):
 
             click.secho("[job manager] Killing job {0}. Name: {1}. Type: {2}"
                         .format(self.uuid, self.name, self.job_type), dim=True)
-            self.handle.unload()
+            if self.handle is not None:
+                self.handle.unload()
         elif self.job_type == "hook":
             api = state_connection.get_api()
             api.jobs_kill(self.uuid)

objection/utils/helpers.py

@@ -1,3 +1,4 @@
+import os
 import re
 import shlex
 
@@ -94,6 +95,21 @@ def clean_argument_flags(args: list) -> list:
     return [x for x in args if not x.startswith('--')]
 
 
+def is_unix_absolute_path(path: str) -> bool:
+    """
+        Determines whether a path should be treated as absolute on
+        remote Unix-like targets.
+
+        On Windows hosts, os.path.isabs('/foo') may not behave as expected
+        for remote device paths that are always POSIX style.
+
+        :param path:
+        :return:
+    """
+
+    return path.startswith('/') or os.path.isabs(path)
+
+
 def to_snake_case(w: str) -> str:
     """
         https://stackoverflow.com/a/1176023

pyproject.toml

@@ -38,6 +38,12 @@ Homepage = "https://github.com/sensepost/objection"
 Repository = "https://github.com/sensepost/objection"
 "Bug Tracker" = "https://github.com/sensepost/objection/issues"
 
+[dependency-groups]
+dev = [
+    "pytest>=7.0.0",
+    "pytest-cov>=4.0.0",
+]
+
 [project.scripts]
 objection = "objection.console.cli:cli"
 
@@ -55,3 +61,10 @@ objection = [
     "utils/assets/*.xml",
     "agent.js",
 ]
+
+[tool.pytest.ini_options]
+testpaths = ["tests"]
+python_files = "test_*.py"
+python_classes = "Test*"
+python_functions = "test_*"
+addopts = "-v --strict-markers"

tests/commands/android/test_keystore.py

@@ -2,7 +2,7 @@
 from unittest import mock
 
 from objection.commands.android.keystore import entries, clear
-from ...helpers import capture
+from ...helpers import capture, normalize_table_whitespace
 
 
 class TestKeystore(unittest.TestCase):
@@ -13,11 +13,11 @@ def test_entries_handles_empty_data(self, mock_api):
         with capture(entries, []) as o:
             output = o
 
-        expected_output = """Alias    Key    Certificate
--------  -----  -------------
+        expected_output = """Alias  Key  Certificate
+-----  ---  -----------
 """
 
-        self.assertEqual(output, expected_output)
+        self.assertEqual(normalize_table_whitespace(output), normalize_table_whitespace(expected_output))
 
     @mock.patch('objection.state.connection.state_connection.get_api')
     def test_entries_handles(self, mock_api):
@@ -30,12 +30,12 @@ def test_entries_handles(self, mock_api):
         with capture(entries, []) as o:
             output = o
 
-        expected_output = """Alias    Key    Certificate
--------  -----  -------------
-test     True   True
+        expected_output = """Alias  Key   Certificate
+-----  ----  -----------
+test   True  True
 """
 
-        self.assertEqual(output, expected_output)
+        self.assertEqual(normalize_table_whitespace(output), normalize_table_whitespace(expected_output))
 
     @mock.patch('objection.state.connection.state_connection.get_api')
     @mock.patch('objection.commands.android.keystore.click.confirm')

tests/commands/ios/test_bundles.py

@@ -3,7 +3,7 @@
 
 from objection.commands.ios.bundles import show_frameworks, _should_include_apple_bundles, _should_print_full_path, \
     _is_apple_bundle, show_bundles
-from ...helpers import capture
+from ...helpers import capture, normalize_table_whitespace
 
 
 class TestBundles(unittest.TestCase):
@@ -76,7 +76,7 @@ def test_show_frameworks_prints_without_apple_bundles(self, mock_api):
 MapKit        za.apple.MapKit                1  /MapKit
 """
 
-        self.assertEqual(output, expected)
+        self.assertEqual(normalize_table_whitespace(output), normalize_table_whitespace(expected))
 
     @mock.patch('objection.state.connection.state_connection.get_api')
     def test_show_frameworks_prints_with_apple_bundles(self, mock_api):
@@ -93,7 +93,7 @@ def test_show_frameworks_prints_with_apple_bundles(self, mock_api):
 MapKit                    za.apple.MapKit                             1  /MapKit
 """
 
-        self.assertEqual(output, expected)
+        self.assertEqual(normalize_table_whitespace(output), normalize_table_whitespace(expected))
 
     @mock.patch('objection.state.connection.state_connection.get_api')
     def test_show_frameworks_prints_with_apple_bundles_and_full_paths(self, mock_api):
@@ -110,7 +110,7 @@ def test_show_frameworks_prints_with_apple_bundles_and_full_paths(self, mock_api
 MapKit                    za.apple.MapKit                             1  /MapKit
 """
 
-        self.assertEqual(output, expected)
+        self.assertEqual(normalize_table_whitespace(output), normalize_table_whitespace(expected))
 
     @mock.patch('objection.state.connection.state_connection.get_api')
     def test_show_bundles_prints_bundles(self, mock_api):
@@ -119,15 +119,15 @@ def test_show_bundles_prints_bundles(self, mock_api):
         with capture(show_bundles, []) as o:
             output = o
 
-        expected = """Executable                Bundle                                Version  Path
-------------------------  ----------------------------------  ---------  -------------------------------------------
-AppleIDSSOAuthentication  com.apple.AppleIDSSOAuthentication          1  /AppleIDSSOAuthentication
-LinguisticData            com.apple.LinguisticData                    1  ...nguisticDataLinguisticDataLinguisticData
-hockeyapp                 net.hockeyapp.sdk.ios                       1  /hockeyapp
-MapKit                    za.apple.MapKit                             1  /MapKit
+        expected = """Executable                Bundle                              Version  Path
+------------------------  ----------------------------------  -------  ------------------------------------------------------------------------
+AppleIDSSOAuthentication  com.apple.AppleIDSSOAuthentication        1  /AppleIDSSOAuthentication
+LinguisticData            com.apple.LinguisticData                  1  /LinguisticData/LinguisticDataLinguisticDataLinguisticDataLinguisticData
+hockeyapp                 net.hockeyapp.sdk.ios                     1  /hockeyapp
+MapKit                    za.apple.MapKit                           1  /MapKit
 """
 
-        self.assertEqual(output, expected)
+        self.assertEqual(normalize_table_whitespace(output), normalize_table_whitespace(expected))
 
     @mock.patch('objection.state.connection.state_connection.get_api')
     def test_show_bundles_prints_bundles(self, mock_api):
@@ -144,4 +144,4 @@ def test_show_bundles_prints_bundles(self, mock_api):
 MapKit                    za.apple.MapKit                             1  /MapKit
 """
 
-        self.assertEqual(output, expected)
+        self.assertEqual(normalize_table_whitespace(output), normalize_table_whitespace(expected))