Add target-shell history

[JSCU-CNI]

This PR adds support for target-shell history files to dissect as described in #650.

Small changes were made to the config helper utility to also scan the current working directory for .targetcfg.py files. The storage location of the shell history by default is $HOME/.dissect_history and can be changed with the HISTFILE variable in .targetcfg.py.

[Miauwkeru]

Thanks for your contribution, we did want to have the history option inside target shell. We'll look at it later.

Could you also attach this PR to an existing issue? That way we are better able to keep track of it.

[JSCU-CNI]

We mentioned #650 in the opening comment of this PR. Is that not sufficient? We will probably incorporate this into the ExtendedCmd class as discussed in #664 (comment).

[Schamper]

Very nice! Do you think you could implement the feature as described in #650 with regards to templating too? I like these additions but it would be awesome if you could also add that part.

For example, to keep with the naming scheme:

• HISTDIR - default to None, if set, takes precedence over HISTFILE
• HISTDIRFMT (?) - default to {uid}-{target}-history perhaps. Only used if HISTDIR is set

Or maybe a better name for HISTDIRFMT. It should only allow for a hardcoded set of template variables. I'd say only uid and target for now (with {target} being t.name, the slugified target name).

[Miauwkeru]

We mentioned #650 in the opening comment of this PR. Is that not sufficient? We will probably incorporate this into the ExtendedCmd class as discussed in #664 (comment).

the association for us is in the development section of the pr. That specifically links a pr to an issue.

[JSCU-CNI]

the association for us is in the development section of the pr. That specifically links a pr to an issue.

I do not think we have access to modify that attribute of a PR in GitHub.

[JSCU-CNI]

Very nice! Do you think you could implement the feature as described in #650 with regards to templating too? I like these additions but it would be awesome if you could also add that part.

Added in b67cc39

[Schamper]

Is that not sufficient?

It was:

[codecov]

Codecov Report

Attention: Patch coverage is 89.74359% with 4 lines in your changes missing coverage. Please review.

Project coverage is 75.45%. Comparing base (827da4e) to head (cca8f46).

Files	Patch %	Lines
dissect/target/tools/shell.py	83.33%	4 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #786      +/-   ##
==========================================
+ Coverage   75.42%   75.45%   +0.02%     
==========================================
  Files         296      296              
  Lines       25600    25629      +29     
==========================================
+ Hits        19310    19339      +29     
  Misses       6290     6290              

Flag	Coverage Δ
unittests	75.45% <89.74%> (+0.02%)	⬆️

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]

The PyPy tests on Linux are struggling real hard, not sure if it's a CI problem or related to this PR.

[Schamper]

I think it's the PR 😉 maybe one of the shell changes, or the addition of looking for a config file in os.cwd()? Maybe an infinite loop in there in the CI environment.

[JSCU-CNI]

Can you see what test is hanging? What is the WORKDIR of the pypy CI environment? Maybe it tries to walk a large folder or something.

[Schamper]

Should be tests/tools/test_shell.py::test_target_cli_unicode_argparse.

[JSCU-CNI]

The tests seem to be fine, I cannot reproduce this with pytest.

(pypy-venv) user@hostname:/tmp/dissect.target$ pypy --version
Python 3.9.18 (7.3.15+dfsg-1build3, Apr 01 2024, 03:12:48)
[PyPy 7.3.15 with GCC 13.2.0]

(pypy-venv) user@hostname:/tmp/dissect.target$ pytest tests -vv
...
================================================================================== 1422 passed, 4 skipped, 11 warnings in 40.09s ==================================================================================

Running tox -e pypy39 however seems to hang when using pypy3.9. Removing os.getcwd() from config.load does not fix it. Downgrading tox or virtualenv also does not seem to help.

[Schamper]

Running tox -e pypy39 however seems to hang when using pypy3.9. Removing os.getcwd() from config.load does not fix it. Downgrading tox or virtualenv also does not seem to help.

Unfortunately I can't reproduce this on Apple Silicon.

[JSCU-CNI]

What's interesting here is it seems that the windows pypy tests are unaffected, which might suggest this has to do with readline (which is unavailable on windows)?

I've run tox on pypy (linux, x86_64) with a profiler attached, but that seems to fix the issue 😅. It seems to be stuck on collecting 56 items for a while but runs fine after waiting for a little while.

(pypy-venv) $ cat tox.ini
...
commands =
    pypy -m cProfile -o tox.pstats -m pytest --basetemp="{envtmpdir}" {posargs:--color=yes --cov=dissect --cov-report=term-missing -v tests}
...

(pypy-venv) $ pypy -m cProfile -o profile.pstats -m tox run -e pypy3.9 --exit-and-dump-after 600
...
platform linux -- Python 3.9.18[pypy-7.3.15-final], pytest-8.3.2, pluggy-1.5.0 -- /tmp/dissect.target/.tox/pypy3.9/bin/python
cachedir: .tox/pypy3.9/.pytest_cache
rootdir: /tmp/dissect.target
configfile: pyproject.toml
plugins: cov-5.0.0
collected 1426 items
...
1422 passed, 4 skipped, 11 warnings in 514.49s (0:08:34)

---

Edit: I've tried running tox run -e pypy3.9 without modifications this morning and was not able to reproduce the hanging of tox anymore. @Schamper could you re-run the CI/CD, perhaps it was caused by some dependency?

[JSCU-CNI]

Seems to be stuck at 98% now. What is the RAM/CPU size of the runners for the pypy3.9 and pypy3.10 tests? Are they perhaps throttled?

[Schamper]

I somehow managed to get a reproducable case on my own system. I think the test got stuck in an infinite loop because of some sort of unhandled error (OSError on my system) when trying to read the history file, which would cause run_cli to loop infinitely. The proposed changes moves the error handling directly to the reading and writing of the history files, which should fix the issue.

[JSCU-CNI]

Nice find! I wonder why those stacktraces were not showing up during testing. Perhaps because of the new self.debug behaviour of TargetCmd?

[Schamper]

Nice find! I wonder why those stacktraces were not showing up during testing. Perhaps because of the new self.debug behaviour of TargetCmd?

test_target_cli_unicode_argparse uses capsys so they were captured by pytest 😅.