Skip to content

Conversation

@skepppy
Copy link
Contributor

@skepppy skepppy commented Jan 26, 2026

This PR adds the commands md5sum, sha1sum and sha256sum to target-shell.

Closes #1516.

@skepppy skepppy force-pushed the add/hash-command-target-shell branch from 0493abb to c27b643 Compare January 26, 2026 21:55
@codecov
Copy link

codecov bot commented Jan 26, 2026

Codecov Report

❌ Patch coverage is 85.71429% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.78%. Comparing base (f787f5a) to head (fc2b41a).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
dissect/target/tools/shell.py 85.71% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1517      +/-   ##
==========================================
+ Coverage   80.75%   80.78%   +0.02%     
==========================================
  Files         396      396              
  Lines       34750    34771      +21     
==========================================
+ Hits        28063    28089      +26     
+ Misses       6687     6682       -5     
Flag Coverage Δ
unittests 80.78% <85.71%> (+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.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@codspeed-hq
Copy link

codspeed-hq bot commented Jan 26, 2026

Merging this PR will not alter performance

✅ 11 untouched benchmarks


Comparing skepppy:add/hash-command-target-shell (fc2b41a) with main (f787f5a)

Open in CodSpeed

Copy link
Member

@Schamper Schamper left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The alias and docstring comments also for the other ones.

return False

@arg("path")
@alias("md5sum")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to add alias, they're already named the same.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh, I'll remove them.

@arg("path")
@alias("md5sum")
def cmd_md5sum(self, args: argparse.Namespace, stdout: TextIO) -> bool:
"""Print the MD5 hash of the file"""
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"""Print the MD5 hash of the file"""
"""print the MD5 hash of the file"""

Comment on lines 1172 to 1180
path = self.check_file(args.path)
if not path:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
path = self.check_file(args.path)
if not path:
if not (path := self.check_file(args.path)):


def test_target_cli_hashsums(tmp_path: Path, target_unix: Target, capsys: pytest.CaptureFixture) -> None:
target_unix.fs.map_file_fh("/test-file", BytesIO(b"Hello world!"))
out_file = tmp_path / "checksum.txt"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not read stdout?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was piping my commands to md5sum thinking it would pipe the output to the cmd_md5sum() call, but it would invoke Linux' md5sum, resulting in capsys.readstdouterr() having no data in its stdout, which I thought was because it didn't work on my machine. That's why I piped it to a file, but I changed it now.

Copy link
Member

@yunzheng yunzheng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of calculating the default md5, sha1 and sha256 hashes, specify the algo via hash().

if not path:
return False

_, _, sha256 = path.get().hash()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
_, _, sha256 = path.get().hash()
sha256, *_ = path.get().hash(["sha256"])

if not path:
return False

_, sha1, _ = path.get().hash()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
_, sha1, _ = path.get().hash()
sha1, *_ = path.get().hash(["sha1"])

if not path:
return False

md5, _, _ = path.get().hash()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
md5, _, _ = path.get().hash()
md5, *_ = path.get().hash(["md5"])

@skepppy skepppy force-pushed the add/hash-command-target-shell branch from c27b643 to 8c94292 Compare January 27, 2026 13:04
@Schamper Schamper changed the title Add/hash command target shell Add individual hash command to target-shell Jan 27, 2026
@skepppy skepppy force-pushed the add/hash-command-target-shell branch 3 times, most recently from 7059b97 to 93022e2 Compare January 27, 2026 22:20
Copy link
Member

@Schamper Schamper left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same for the rest.

The docstrings in this file as specifically formatted like this to look the most "CLI-like" when used as help messages. Perhaps there should be a comment explaining that in this file.

Comment on lines 1170 to 1178
"""print the MD5 checksum of a file provided by a path or from stdin.

Args:
args (argparse.Namespace): the arguments passed to md5sum.
stdout (TextIO): the file handle to write the md5sum output to.

Returns:
bool: always False, since True indicates the CLI should exit.
"""
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"""print the MD5 checksum of a file provided by a path or from stdin.
Args:
args (argparse.Namespace): the arguments passed to md5sum.
stdout (TextIO): the file handle to write the md5sum output to.
Returns:
bool: always False, since True indicates the CLI should exit.
"""
"""print the MD5 checksum of a file at the given path"""

@skepppy skepppy force-pushed the add/hash-command-target-shell branch from 93022e2 to c581405 Compare January 30, 2026 13:41
@Schamper Schamper merged commit 2294418 into fox-it:main Jan 30, 2026
22 of 24 checks passed
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.

Add md5sum, sha1sum and sha256sum to target-shell

3 participants