-
Notifications
You must be signed in to change notification settings - Fork 75
Add filesystem identifiers to walkfs #1316
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 3 commits
7a57b20
04c0fd7
6fcd734
e6e1635
40fec39
05a7561
98865f4
06a5541
a53f925
3ae2c78
72ce981
24d4a49
a7dcc1a
c22ca26
d83e8b5
78ffbd8
c88fdc5
51a2986
c124db2
242d243
7508485
63db79d
206ecc1
d0d1e18
5f0ea91
4feb0c8
fbc84b0
497fdbf
4219d5e
f400fb9
ae6d271
197e7b9
675da3d
0cbc19d
ced4a6d
8c13863
c0fa1b1
99742b8
2dc8f06
f1d2f8b
5a739d0
c84310f
d89f264
6110be9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
| @@ -1,13 +1,14 @@ | ||||||||
| from __future__ import annotations | ||||||||
|
|
||||||||
| from typing import TYPE_CHECKING | ||||||||
| from uuid import UUID | ||||||||
|
|
||||||||
| from dissect.util.ts import from_unix | ||||||||
|
|
||||||||
| from dissect.target.exceptions import FileNotFoundError, UnsupportedPluginError | ||||||||
| from dissect.target.filesystem import FilesystemEntry, LayerFilesystemEntry | ||||||||
| from dissect.target.helpers.record import TargetRecordDescriptor | ||||||||
| from dissect.target.plugin import Plugin, arg, export | ||||||||
| from dissect.target.plugin import Plugin, arg, export, internal | ||||||||
|
|
||||||||
| if TYPE_CHECKING: | ||||||||
| from collections.abc import Iterator | ||||||||
|
|
@@ -28,6 +29,8 @@ | |||||||
| ("uint32", "uid"), | ||||||||
| ("uint32", "gid"), | ||||||||
| ("string[]", "fstypes"), | ||||||||
| ("string[]", "vuuid"), | ||||||||
| ("string[]", "dserial"), | ||||||||
|
||||||||
| ("string[]", "dserial"), | |
| ("string[]", "disk_identifiers"), |
maybe a bit more clear what the field is for
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the @internal annotation is only used within plugins, So you can leave this out.
loaflover marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
loaflover marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it might be better to create a Filesystem.uuid function. And implement these lookups inside each respective filesystem.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i am going over the changes and don't entirely understand this one, could you elaborate a bit on exactly what you are suggesting? thanks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was talking about adding a function to the Filesystem representation itself.
So in dissect/target/filesystem.py:
class Filesystem:
...
def uuid(self) -> UUID | None:
return NoneAnd then implement it for those specific filesystems, e.g. ntfs etc:
dissect/target/filesystems/extfs.py
class ExtFilesystem(Filesystem):
...
def uuid(self) -> UUID | None:
return self.extfs.uuidand that for all those filesystems above. Then you can replace those if statements with:
fs.uuid()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldn't it be nicer to have it as either a property or a __init__ argument?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm almost done adding it as a property. just need to make sure tests pass.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it will take a bit longer then expected. not only does it seem i have tio rebase (some tests fail unless i rebase even when going before my first commit, i assume something internally has changed?) but also with the uuid moving to a property ill have to totally redo the tests, as mock doesnt use them
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The internal comment before was also for this one ;)
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| @internal | |
| def get_disk_serial(entry: FilesystemEntry) -> str: | |
| def get_disk_serial(entry: FilesystemEntry) -> str | None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And the same suggestion about changing the FilesystemEntry here.
loaflover marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of iterating over these entries 3 times, it would be better to just do it once.
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This change will fail linting. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,12 +2,14 @@ | |
|
|
||
| from pathlib import Path | ||
| from typing import TYPE_CHECKING | ||
| from unittest.mock import Mock, MagicMock | ||
| from uuid import UUID | ||
|
|
||
| import pytest | ||
|
|
||
| from dissect.target.filesystem import VirtualFile, VirtualFilesystem | ||
| from dissect.target.filesystem import FilesystemEntry, VirtualFile, VirtualFilesystem | ||
| from dissect.target.loaders.tar import TarLoader | ||
| from dissect.target.plugins.filesystem.walkfs import WalkFSPlugin | ||
| from dissect.target.plugins.filesystem.walkfs import WalkFSPlugin, get_disk_serial, get_volume_uuid | ||
| from tests._utils import absolute_path | ||
|
|
||
| if TYPE_CHECKING: | ||
|
|
@@ -17,6 +19,7 @@ | |
|
|
||
|
|
||
| def test_walkfs_plugin(target_unix: Target, fs_unix: VirtualFilesystem) -> None: | ||
|
|
||
| fs_unix.map_file_entry("/path/to/some/file", VirtualFile(fs_unix, "file", None)) | ||
| fs_unix.map_file_entry("/path/to/some/other/file.ext", VirtualFile(fs_unix, "file.ext", None)) | ||
| fs_unix.map_file_entry("/root_file", VirtualFile(fs_unix, "root_file", None)) | ||
|
|
@@ -25,7 +28,7 @@ def test_walkfs_plugin(target_unix: Target, fs_unix: VirtualFilesystem) -> None: | |
| fs_unix.map_file_entry("/.test/.more.test.txt", VirtualFile(fs_unix, ".more.test.txt", None)) | ||
|
|
||
| target_unix.add_plugin(WalkFSPlugin) | ||
|
|
||
|
||
| results = list(target_unix.walkfs()) | ||
| assert len(results) == 14 | ||
| assert sorted([r.path for r in results]) == [ | ||
|
|
@@ -57,3 +60,132 @@ def test_benchmark_walkfs(target_bare: Target, benchmark: BenchmarkFixture) -> N | |
| result = benchmark(lambda: next(WalkFSPlugin(target_bare).walkfs())) | ||
|
|
||
| assert result.path == "/" | ||
|
|
||
|
|
||
| @pytest.fixture | ||
| def mock_fs_entry(): | ||
| """Fixture to create a mock FilesystemEntry object.""" | ||
| mock_entry = MagicMock(spec=FilesystemEntry) | ||
| mock_fs = Mock() | ||
| mock_volume = Mock() | ||
| mock_disk = Mock() | ||
|
|
||
| # Set up the mock object structure | ||
| mock_entry.fs = mock_fs | ||
| mock_fs.volume = mock_volume | ||
| mock_volume.disk = mock_disk | ||
|
|
||
| # Clear any attributes from previous tests | ||
| mock_volume.guid = None | ||
| mock_fs.__type__ = 'generic' | ||
| if hasattr(mock_fs, 'ntfs'): del mock_fs.ntfs | ||
| if hasattr(mock_fs, 'extfs'): del mock_fs.extfs | ||
| if hasattr(mock_fs, 'fatfs'): del mock_fs.fatfs | ||
| if hasattr(mock_fs, 'exfat'): del mock_fs.exfat | ||
| if hasattr(mock_disk.vs, 'serial'): del mock_disk.vs.serial | ||
|
||
|
|
||
| return mock_entry | ||
|
|
||
|
|
||
| def test_get_volume_uuid_ntfs(mock_fs_entry): | ||
| """Test get_volume_uuid for an NTFS filesystem.""" | ||
| # Mock NTFS-specific attributes | ||
| mock_fs_entry.fs.__type__ = 'ntfs' | ||
| mock_fs_entry.fs.ntfs = Mock(serial=123456789) | ||
|
|
||
| expected_uuid = UUID(int=123456789) | ||
| assert get_volume_uuid(mock_fs_entry) == expected_uuid | ||
|
|
||
|
|
||
| def test_get_volume_uuid_ext(mock_fs_entry): | ||
| """Test get_volume_uuid for an EXT filesystem (ext2/3/4).""" | ||
| # Mock EXT-specific attributes | ||
| mock_fs_entry.fs.__type__ = 'ext4' | ||
| mock_fs_entry.fs.extfs = Mock(uuid="e0c3d987-a36c-4f9e-9b2f-90e633d7d7a1") | ||
|
|
||
| expected_uuid = "e0c3d987-a36c-4f9e-9b2f-90e633d7d7a1" | ||
| assert get_volume_uuid(mock_fs_entry) == expected_uuid | ||
|
|
||
|
|
||
| def test_get_volume_uuid_fat(mock_fs_entry): | ||
| """Test get_volume_uuid for a FAT filesystem.""" | ||
| # Mock FAT-specific attributes | ||
| mock_fs_entry.fs.__type__ = 'fat' | ||
| mock_fs_entry.fs.fatfs = Mock(volume_id="1a2b3c4d") | ||
|
|
||
| expected_uuid = UUID(int=0x1a2b3c4d) | ||
| assert get_volume_uuid(mock_fs_entry) == expected_uuid | ||
|
|
||
|
|
||
| def test_get_volume_uuid_exfat(mock_fs_entry): | ||
| """Test get_volume_uuid for an ExFAT filesystem.""" | ||
| # Mock ExFAT-specific attributes | ||
| mock_fs_entry.fs.__type__ = 'exfat' | ||
| mock_fs_entry.fs.exfat = Mock(vbr=Mock(volume_serial=987654321)) | ||
|
|
||
| expected_uuid = UUID(int=987654321) | ||
| assert get_volume_uuid(mock_fs_entry) == expected_uuid | ||
|
|
||
|
|
||
| def test_get_volume_uuid_guid(mock_fs_entry): | ||
| """Test get_volume_uuid when `volume.guid` exists (higher priority).""" | ||
| # Mock a GUID that should be returned first | ||
| mock_fs_entry.fs.volume.guid = b'\x00\x01\x02\x03\x04\x05\x06\x07\x08\x09\x0a\x0b\x0c\x0d\x0e\x0f' | ||
|
|
||
| expected_uuid = UUID(bytes_le=mock_fs_entry.fs.volume.guid) | ||
| assert get_volume_uuid(mock_fs_entry) == expected_uuid | ||
|
|
||
|
|
||
| def test_get_volume_uuid_no_match(mock_fs_entry): | ||
| """Test get_volume_uuid when no valid UUID is found.""" | ||
| # The default mock has no specific filesystem type | ||
| mock_fs_entry.fs.__type__ = 'unsupported_fs' | ||
|
|
||
| assert get_volume_uuid(mock_fs_entry) is None | ||
|
|
||
|
|
||
| def test_get_disk_serial(mock_fs_entry): | ||
| """Test get_disk_serial when a serial number is available.""" | ||
| # Mock the `serial` attribute on the `vs` object | ||
| mock_fs_entry.fs.volume.disk.vs = Mock(serial='A1B2C3D4') | ||
|
|
||
| assert get_disk_serial(mock_fs_entry) == 'A1B2C3D4' | ||
|
|
||
|
|
||
| def test_get_disk_serial_no_serial(mock_fs_entry): | ||
| """Test get_disk_serial when the `serial` attribute is missing.""" | ||
| # The default mock aparently does have the `serial` attribute on `vs` | ||
| mock_fs_entry.fs.volume.disk.vs = Mock() | ||
| if hasattr(mock_fs_entry.fs.volume.disk.vs, 'serial'): | ||
| delattr(mock_fs_entry.fs.volume.disk.vs, 'serial') | ||
| assert get_disk_serial(mock_fs_entry) is None | ||
|
|
||
|
|
||
| def test_walkfs_plugin(target_unix: Target, fs_unix: VirtualFilesystem) -> None: | ||
|
||
| fs_unix.map_file_entry("/path/to/some/file", VirtualFile(fs_unix, "file", None)) | ||
| fs_unix.map_file_entry("/path/to/some/other/file.ext", VirtualFile(fs_unix, "file.ext", None)) | ||
| fs_unix.map_file_entry("/root_file", VirtualFile(fs_unix, "root_file", None)) | ||
| fs_unix.map_file_entry("/other_root_file.ext", VirtualFile(fs_unix, "other_root_file.ext", None)) | ||
| fs_unix.map_file_entry("/.test/test.txt", VirtualFile(fs_unix, "test.txt", None)) | ||
| fs_unix.map_file_entry("/.test/.more.test.txt", VirtualFile(fs_unix, ".more.test.txt", None)) | ||
|
|
||
| target_unix.add_plugin(WalkFSPlugin) | ||
|
|
||
| results = list(target_unix.walkfs()) | ||
| assert len(results) == 14 | ||
| assert sorted([r.path for r in results]) == [ | ||
| "/", | ||
| "/.test", | ||
| "/.test/.more.test.txt", | ||
| "/.test/test.txt", | ||
| "/etc", | ||
| "/other_root_file.ext", | ||
| "/path", | ||
| "/path/to", | ||
| "/path/to/some", | ||
| "/path/to/some/file", | ||
| "/path/to/some/other", | ||
| "/path/to/some/other/file.ext", | ||
| "/root_file", | ||
| "/var", | ||
| ] | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe an idea to change it to
identifiersnow, as that is what we use in the code.