Add support for targetrc file to execute commands when a target shell is spawned #859

twiggler · 2024-09-19T11:23:40Z

Support execution of a ~/.targetrc file when a target shell is opened, and ~/.targetrc.registry in case of the registry shell.

We have decided on separate run command files because the registry and target shell are not compatible.

Additionally:

The run commands files to be executed can be configured per-target by setting the TARGETRCFILE (and TARGETRCFILE_REGISTRY) variables inside the target configuration file.

Closes #592

codecov · 2024-09-19T11:27:02Z

Codecov Report

Attention: Patch coverage is 87.50000% with 3 lines in your changes missing coverage. Please review.

Project coverage is 76.34%. Comparing base (3ac5b79) to head (5b96b70).
Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
dissect/target/tools/shell.py	87.50%	3 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #859   +/-   ##
=======================================
  Coverage   76.33%   76.34%           
=======================================
  Files         313      313           
  Lines       26859    26883   +24     
=======================================
+ Hits        20503    20524   +21     
- Misses       6356     6359    +3

Flag	Coverage Δ
unittests	`76.34% <87.50%> (+<0.01%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Schamper · 2024-09-20T10:38:28Z

dissect/target/tools/shell.py

+        runcommands_file = pathlib.Path(
+            getattr(target._config, "TARGETRCFILE", self.DEFAULT_RUNCOMMANDSFILE)
+        ).expanduser()
+        self.load_runcommands(runcommands_file)


Why not in TargetCmd?

The registry shell inherits from TargetCmd. Further, the "ordinary" shell and the registry shell are not fully compatible. Therefore, I don't think it makes sense to execute the same rc file in both shells.

We could support the registry shell with a different rc file. (I asked the team this on Thursday, and they indicated to restrict the rc file to the "ordinary" shell.)

@JSCU-CNI would you mind giving your thoughts on this?

We agree it may be better to have different rc files for different shells.
Perhaps the logic of loading the targetrc file and executing commands can be stored in a superclass and inheriting classes can override where the commands are loaded from.

For example, the _load_targetrc function and calling said function from __init__ can be moved to the ExtendedCmd class, which has a DEFAULT_RUNCOMMANDSFILE property of None. Then, inheriting classes could override that property. This also leaves a good boilerplate for usages of ExtendedCmd that are not inheriting from TargetCmd.

dissect/target/tools/shell.py

pyrco · 2024-10-03T13:13:12Z

dissect/target/tools/shell.py

+    def _get_targetrc_path(self) -> pathlib.Path:
+        """Get the path to the run commands file."""
+
+        return pathlib.Path(self.DEFAULT_RUNCOMMANDSFILE).expanduser()


Does expanduser() work for ~ in paths on windows?

Yes it does.

(Verified on virtual machine, and see https://docs.python.org/3/library/os.path.html#os.path.expanduser)

pyrco · 2024-10-03T13:21:59Z

dissect/target/tools/shell.py

@@ -95,6 +95,7 @@ class ExtendedCmd(cmd.Cmd):
    """

    CMD_PREFIX = "cmd_"
+    DEFAULT_RUNCOMMANDSFILE = "~/.targetrc"


Can you move this to TargetCmd and have this value be None?

Schamper · 2024-10-07T09:10:23Z

dissect/target/tools/shell.py

@@ -96,6 +96,7 @@ class ExtendedCmd(cmd.Cmd):

    CMD_PREFIX = "cmd_"
    _runtime_aliases = {}
+    DEFAULT_RUNCOMMANDSFILE = None


Suggested change

DEFAULT_RUNCOMMANDSFILE = None

DEFAULT_RUNCOMMANDS_FILE = None

Schamper · 2024-10-07T09:11:13Z

dissect/target/tools/shell.py

+        """Get the path to the run commands file. Can return None if DEFAULT_RUNCOMMANDSFILE is not set."""
+        return pathlib.Path(self.DEFAULT_RUNCOMMANDSFILE).expanduser() if self.DEFAULT_RUNCOMMANDSFILE else None


Suggested change

"""Get the path to the run commands file. Can return None if DEFAULT_RUNCOMMANDSFILE is not set."""

return pathlib.Path(self.DEFAULT_RUNCOMMANDSFILE).expanduser() if self.DEFAULT_RUNCOMMANDSFILE else None

"""Get the path to the run commands file. Can return ``None`` if ``DEFAULT_RUNCOMMANDS_FILE`` is not set."""

return pathlib.Path(self.DEFAULT_RUNCOMMANDS_FILE).expanduser() if self.DEFAULT_RUNCOMMANDS_FILE else None

Schamper · 2024-10-07T09:11:44Z

dissect/target/tools/shell.py

+        targetrc_path = self._get_targetrc_path()
+        if targetrc_path is not None:


Suggested change

targetrc_path = self._get_targetrc_path()

if targetrc_path is not None:

if targetrc_path := self._get_targetrc_path() is not None:

Schamper · 2024-10-07T09:12:14Z

dissect/target/tools/shell.py

+    DEFAULT_RUNCOMMANDSFILE = "~/.targetrc"
+    CONFIG_KEY_RUNCOMMANDSFILE = "TARGETRCFILE"


Suggested change

DEFAULT_RUNCOMMANDSFILE = "~/.targetrc"

CONFIG_KEY_RUNCOMMANDSFILE = "TARGETRCFILE"

DEFAULT_RUNCOMMANDS_FILE = "~/.targetrc"

CONFIG_KEY_RUNCOMMANDS_FILE = "TARGETRCFILE"

Schamper · 2024-10-07T09:12:28Z

dissect/target/tools/shell.py

+        """Get the path to the run commands file."""
+
+        return pathlib.Path(
+            getattr(self.target._config, self.CONFIG_KEY_RUNCOMMANDSFILE, self.DEFAULT_RUNCOMMANDSFILE)


Suggested change

getattr(self.target._config, self.CONFIG_KEY_RUNCOMMANDSFILE, self.DEFAULT_RUNCOMMANDSFILE)

getattr(self.target._config, self.CONFIG_KEY_RUNCOMMANDS_FILE, self.DEFAULT_RUNCOMMANDS_FILE)

Schamper · 2024-10-07T09:12:46Z

dissect/target/tools/shell.py

+    DEFAULT_RUNCOMMANDSFILE = "~/.targetrc.registry"
+    CONFIG_KEY_RUNCOMMANDSFILE = "TARGETRCFILE_REGISTRY"


Suggested change

DEFAULT_RUNCOMMANDSFILE = "~/.targetrc.registry"

CONFIG_KEY_RUNCOMMANDSFILE = "TARGETRCFILE_REGISTRY"

DEFAULT_RUNCOMMANDS_FILE = "~/.targetrc.registry"

CONFIG_KEY_RUNCOMMANDS_FILE = "TARGETRCFILE_REGISTRY"

This avoids unnecessary state in the TargetCLI class

This prevents polution of the TargetCli interface.

Schamper · 2024-10-07T10:27:53Z

dissect/target/tools/shell.py

+        except Exception as e:
+            log.debug("Error processing .targetrc file: %s", e)
+
+    def _get_targetrc_path(self) -> Optional[pathlib.Path]:


Suggested change

def _get_targetrc_path(self) -> Optional[pathlib.Path]:

def _get_targetrc_path(self) -> pathlib.Path | None:

Schamper · 2024-10-07T10:28:02Z

dissect/target/tools/shell.py

@@ -19,7 +19,7 @@
 import sys
 from contextlib import contextmanager
 from datetime import datetime, timedelta, timezone
-from typing import Any, BinaryIO, Callable, Iterator, TextIO
+from typing import Any, BinaryIO, Callable, Iterator, Optional, TextIO


Suggested change

from typing import Any, BinaryIO, Callable, Iterator, Optional, TextIO

from typing import Any, BinaryIO, Callable, Iterator, TextIO

Schamper

Meant to be a request changes.

Schamper reviewed

twiggler requested a review from Schamper September 19, 2024 11:31

Schamper requested changes Sep 20, 2024

View reviewed changes

twiggler requested a review from Schamper September 23, 2024 07:52

twiggler force-pushed the 529-targetrc branch 3 times, most recently from 0fac854 to 72ee60f Compare September 30, 2024 14:41

twiggler requested review from JSCU-CNI and pyrco and removed request for JSCU-CNI September 30, 2024 14:47

pyrco previously requested changes Oct 3, 2024

View reviewed changes

twiggler force-pushed the 529-targetrc branch from 72ee60f to 9e82179 Compare October 7, 2024 07:00

twiggler requested a review from pyrco October 7, 2024 07:04

Schamper requested changes Oct 7, 2024

View reviewed changes

twiggler force-pushed the 529-targetrc branch from 914f7bc to e0490d2 Compare October 7, 2024 09:48

twiggler and others added 7 commits October 7, 2024 11:49

Add loading and executtion of rc file on cli init

a434c19

Add test to check if .targetrc file is processed

a06173a

Refactor to make run commands file path a local variable.

386eacb

This avoids unnecessary state in the TargetCLI class

Make method for loading rc file private

607a1a4

This prevents polution of the TargetCli interface.

Support loading of runcommand files in registry shell

d0e3cf2

Enhance abstraction by moving targetrc constant to subclass

754b57e

Rename constants

afc3150

twiggler force-pushed the 529-targetrc branch from e0490d2 to afc3150 Compare October 7, 2024 09:50

twiggler requested a review from Schamper October 7, 2024 10:00

Schamper approved these changes Oct 7, 2024

View reviewed changes

Schamper requested changes Oct 7, 2024

View reviewed changes

Use union syntax instead of optional

5b96b70

twiggler requested a review from Schamper October 7, 2024 11:04

Schamper approved these changes Oct 7, 2024

View reviewed changes

twiggler merged commit b96029a into main Oct 7, 2024
18 checks passed

twiggler deleted the 529-targetrc branch October 7, 2024 11:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for targetrc file to execute commands when a target shell is spawned #859

Add support for targetrc file to execute commands when a target shell is spawned #859

twiggler commented Sep 19, 2024 •

edited

Loading

codecov bot commented Sep 19, 2024 •

edited

Loading

Schamper Sep 20, 2024

twiggler Sep 23, 2024

Schamper Sep 23, 2024

JSCU-CNI Sep 26, 2024

pyrco Oct 3, 2024

twiggler Oct 3, 2024

pyrco Oct 3, 2024

Schamper Oct 7, 2024

Schamper Oct 7, 2024

Schamper Oct 7, 2024

Schamper Oct 7, 2024

Schamper Oct 7, 2024

Schamper Oct 7, 2024

Schamper Oct 7, 2024

Schamper Oct 7, 2024

Schamper left a comment

	DEFAULT_RUNCOMMANDSFILE = None
	DEFAULT_RUNCOMMANDS_FILE = None

		"""Get the path to the run commands file. Can return None if DEFAULT_RUNCOMMANDSFILE is not set."""
		return pathlib.Path(self.DEFAULT_RUNCOMMANDSFILE).expanduser() if self.DEFAULT_RUNCOMMANDSFILE else None

		targetrc_path = self._get_targetrc_path()
		if targetrc_path is not None:

	targetrc_path = self._get_targetrc_path()
	if targetrc_path is not None:
	if targetrc_path := self._get_targetrc_path() is not None:

		DEFAULT_RUNCOMMANDSFILE = "~/.targetrc"
		CONFIG_KEY_RUNCOMMANDSFILE = "TARGETRCFILE"

	getattr(self.target._config, self.CONFIG_KEY_RUNCOMMANDSFILE, self.DEFAULT_RUNCOMMANDSFILE)
	getattr(self.target._config, self.CONFIG_KEY_RUNCOMMANDS_FILE, self.DEFAULT_RUNCOMMANDS_FILE)

		DEFAULT_RUNCOMMANDSFILE = "~/.targetrc.registry"
		CONFIG_KEY_RUNCOMMANDSFILE = "TARGETRCFILE_REGISTRY"

	def _get_targetrc_path(self) -> Optional[pathlib.Path]:
	def _get_targetrc_path(self) -> pathlib.Path \| None:

	from typing import Any, BinaryIO, Callable, Iterator, Optional, TextIO
	from typing import Any, BinaryIO, Callable, Iterator, TextIO

Add support for targetrc file to execute commands when a target shell is spawned #859

Add support for targetrc file to execute commands when a target shell is spawned #859

Conversation

twiggler commented Sep 19, 2024 • edited Loading

codecov bot commented Sep 19, 2024 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Schamper left a comment

Choose a reason for hiding this comment

twiggler commented Sep 19, 2024 •

edited

Loading

codecov bot commented Sep 19, 2024 •

edited

Loading