Add filesystem identifiers to walkfs #1316

loaflover · 2025-09-07T21:45:54Z

title basically. i did what i said i did. i also added the appropriate tests because apparently that is how you write code (BORING)

in my test files i never saw disk serial populated but since its a field i assume it is populated sometimes. hope i didnt spend a few hours on nothing.

i also made it so adding volume uuid's for new fs's very easy to do. theres like, a WHOLE function that does it, and you just need to add a new ELIF clause for your filesystem. your welcome, everyone who is gonna use this (probably only me 😢 )

… removed serial for the test that needs it gone. finally finished :3

loaflover · 2025-09-10T21:05:01Z

@Schamper is there an update on this pr? no rush just wanna know whats happening 😄

Schamper · 2025-09-12T19:55:02Z

@Schamper is there an update on this pr? no rush just wanna know whats happening 😄

I'm currently on leave for a little bit and this is too big a PR to review during toilet breaks, so I'll get to it when I'm back!

loaflover · 2025-09-12T20:15:58Z

@Schamper is there an update on this pr? no rush just wanna know whats happening 😄

I'm currently on leave for a little bit and this is too big a PR to review during toilet breaks, so I'll get to it when I'm back!

alright, looking forward to it!

enjoy your leave (:

loaflover · 2025-09-28T21:17:55Z

hey sorry to bother but is there an update? @Schamper

Miauwkeru · 2025-10-16T13:34:07Z

dissect/target/plugins/filesystem/walkfs.py

                self.target.log.debug("", exc_info=e)
                continue

+@internal


the @internal annotation is only used within plugins, So you can leave this out.

Miauwkeru · 2025-10-16T13:36:24Z

tests/plugins/filesystem/test_walkfs.py


    target_unix.add_plugin(WalkFSPlugin)
-
+    


The linter will complain about this. So you can just run tox run -e fix to fix the fixable linting issues. Use tox run -e lint to see any other changes

Miauwkeru · 2025-10-16T13:50:29Z

dissect/target/plugins/filesystem/walkfs.py

        fs_types = [sub_entry.fs.__type__ for sub_entry in entry.entries]
+        volume_uuids = [
+            get_volume_uuid(sub_entry)
+            for sub_entry in entry.entries
+        ]
+
+        disk_serials = [
+            get_disk_serial(sub_entry)
+            for sub_entry in entry.entries
+        ]


Instead of iterating over these entries 3 times, it would be better to just do it once.

Miauwkeru · 2025-10-16T14:29:12Z

tests/plugins/filesystem/test_walkfs.py

+    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


The reason these objects have these attributes, is because it is a Mock object. These Mock objects are designed to have every attribute. Instead of deleting them, setting them to None would suffice

Miauwkeru · 2025-10-16T14:45:54Z

dissect/target/plugins/filesystem/walkfs.py

+    elif entry.fs.__type__ == 'ntfs':
+        return UUID(int=entry.fs.ntfs.serial)
+    elif entry.fs.__type__ in ['ext2', 'ext3', 'ext4']:
+        return entry.fs.extfs.uuid
+    elif entry.fs.__type__ == 'fat':
+        return UUID(int=int(entry.fs.fatfs.volume_id, 16))
+    elif entry.fs.__type__ == 'exfat':
+        return UUID(int=entry.fs.exfat.vbr.volume_serial)


I think it might be better to create a Filesystem.uuid function. And implement these lookups inside each respective filesystem.

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.

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 None

And 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.uuid

and that for all those filesystems above. Then you can replace those if statements with:
fs.uuid()

Wouldn't it be nicer to have it as either a property or a __init__ argument?

I'm almost done adding it as a property. just need to make sure tests pass.

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

dissect/target/plugins/filesystem/walkfs.py

Miauwkeru · 2025-10-16T14:53:56Z

dissect/target/plugins/filesystem/walkfs.py

+@internal
+def get_disk_serial(entry: FilesystemEntry) -> str:


Suggested change

@internal

def get_disk_serial(entry: FilesystemEntry) -> str:

def get_disk_serial(entry: FilesystemEntry) -> str | None:

And the same suggestion about changing the FilesystemEntry here.

dissect/target/plugins/filesystem/walkfs.py

Miauwkeru · 2025-10-16T15:05:02Z

tests/plugins/filesystem/test_walkfs.py

+    assert get_disk_serial(mock_fs_entry) is None
+
+
+def test_walkfs_plugin(target_unix: Target, fs_unix: VirtualFilesystem) -> None:


This test is already defined at the top of this file, you can remove this one

Co-authored-by: Miauwkeru <[email protected]>

…ssect.target into feature/uuid-walkfs

… removed serial for the test that needs it gone. finally finished :3

Co-authored-by: Miauwkeru <[email protected]>

…ssect.target into feature/uuid-walkfs

loaflover · 2025-10-20T19:09:51Z

i believe i covered all bases and then-some

by the way, this pr has evolved to a point where it makes my older pr, where i added UUID to mft parser, obsolete. id like to amend the code, i assume you'd prefer a new pr for it, but if you'd like it here just say. (should be a few lines at most)

Miauwkeru

by the way, this pr has evolved to a point where it makes my older pr, where i added UUID to mft parser, obsolete. id like to amend the code, i assume you'd prefer a new pr for it, but if you'd like it here just say. (should be a few lines at most)

Because it would be a different feature, unrelated to the changes in this code I would prefer it to be a different PR.

dissect/target/filesystem.py

Miauwkeru · 2025-10-21T08:30:43Z

dissect/target/plugins/filesystem/walkfs.py

                self.target.log.warning("File not found: %s", entry)
                self.target.log.debug("", exc_info=e)
            except Exception as e:
+                print(e)


Suggested change

print(e)

Miauwkeru · 2025-10-21T09:50:15Z

dissect/target/plugins/filesystem/walkfs.py

        ("uint32", "uid"),
        ("uint32", "gid"),
        ("string[]", "fstypes"),
+        ("string[]", "vuuid"),


Suggested change

("string[]", "vuuid"),

("string[]", "identifiers"),

Maybe an idea to change it to identifiers now, as that is what we use in the code.

Miauwkeru · 2025-10-21T10:24:11Z

dissect/target/plugins/filesystem/walkfs.py

+def get_disk_serial(filesystem: Filesystem) -> str | None:
+    """
+    Returns the disk serial number if it exists; otherwise, returns None.
+
+    Args:
+        entry: :class:`Filesystem` instance
+
+    Returns:
+        Disk serial number as str, or None if not available.
+    """
+    entry_volume = getattr(filesystem, "volume", None)
+    volume_vs = getattr(entry_volume, "vs", None)
+    return getattr(volume_vs, "serial", None)


I am wondering whether the disk serial is useful in this state. Mostly because serial is only defined for an MBR partition. I am discussing it internally whether to expose more serials / identifiers to the volumesystem.
As I feel it might just create a lot of visual noise right now with dserials being defined as ["None"]

We went back over it, and yes, getting the disk serial or disk identifier at this point is a bit useless as its only populated in an MBR volume system and is not populated otherwise. So it is better to remove this function all together.

If we want to add this functionality we need to make more changes to volume and volumesystem to change it

Miauwkeru · 2025-10-21T10:25:17Z

dissect/target/plugins/filesystem/walkfs.py

        ("uint32", "gid"),
        ("string[]", "fstypes"),
+        ("string[]", "vuuid"),
+        ("string[]", "dserial"),


Suggested change

("string[]", "dserial"),

("string[]", "disk_identifiers"),

maybe a bit more clear what the field is for

Miauwkeru · 2025-10-21T10:44:44Z

tests/plugins/filesystem/test_walkfs.py

+def test_get_disk_serial(mock_fs_entry: MagicMock) -> None:
+    """Test get_disk_serial when a serial number is available."""
+    # Mock the `serial` attribute on the `vs` object
+    mock_fs_entry.fs.volume.vs = Mock(serial="A1B2C3D4")
+
+    assert get_disk_serial(mock_fs_entry.fs) == "A1B2C3D4"


Suggested change

def test_get_disk_serial(mock_fs_entry: MagicMock) -> None:

"""Test get_disk_serial when a serial number is available."""

# Mock the `serial` attribute on the `vs` object

mock_fs_entry.fs.volume.vs = Mock(serial="A1B2C3D4")

assert get_disk_serial(mock_fs_entry.fs) == "A1B2C3D4"

def test_get_disk_serial() -> None:

"""Test get_disk_serial when a serial number is available."""

vfs = VirtualFilesystem()

# Mock the `serial` attribute on the `vs` object

vfs.volume = Mock(vs=Mock(serial="A1B2C3D4"))

assert get_disk_serial(vfs) == "A1B2C3D4"

This feels a bit cleaner compared to the custom fixture

Miauwkeru · 2025-10-21T10:45:16Z

tests/plugins/filesystem/test_walkfs.py

+def test_get_disk_serial_no_serial(mock_fs_entry: MagicMock) -> None:
+    """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.vs, "serial"):
+        delattr(mock_fs_entry.fs.volume.vs, "serial")
+    assert get_disk_serial(mock_fs_entry.fs) is None


Suggested change

def test_get_disk_serial_no_serial(mock_fs_entry: MagicMock) -> None:

"""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.vs, "serial"):

delattr(mock_fs_entry.fs.volume.vs, "serial")

assert get_disk_serial(mock_fs_entry.fs) is None

def test_get_disk_serial_no_serial() -> None:

"""Test get_disk_serial when the `serial` attribute is missing."""

# The default mock aparently does have the `serial` attribute on `vs`

vfs = VirtualFilesystem()

# An eempty object without any attributes

vfs.volume = Mock(vs=object())

assert get_disk_serial(vfs) is None

This feels a bit cleaner compared to the custom fixture

Miauwkeru · 2025-10-21T10:46:38Z

tests/plugins/filesystem/test_walkfs.py

+
+
+@pytest.fixture
+def mock_fs_entry() -> MagicMock:
+    """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"
+    for attr in ("ntfs", "extfs", "fatfs", "exfat"):
+        if hasattr(mock_fs, attr):
+            setattr(mock_fs, attr, None)
+    if hasattr(mock_disk.vs, "serial"):
+        mock_disk.vs = None
+
+    return mock_entry


Suggested change

@pytest.fixture

def mock_fs_entry() -> MagicMock:

"""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"

for attr in ("ntfs", "extfs", "fatfs", "exfat"):

if hasattr(mock_fs, attr):

setattr(mock_fs, attr, None)

if hasattr(mock_disk.vs, "serial"):

mock_disk.vs = None

return mock_entry

With the other changes in this file, this can be removed

Miauwkeru · 2025-10-21T10:46:49Z

tests/plugins/filesystem/test_uuid.py

+def test_ext_identifier_no_guid() -> None:
+    """EXT.identifier fallback using extfs.identifier when volume.guid is None."""
+    fs = ExtFilesystem(fh=absolute_path("_data/filesystems/symlink_disk.ext4").open("rb"))
+    fs.volume = Mock(guid=None)
+    fs.extfs = Mock(uuid="e0c3d987-a36c-4f9e-9b2f-90e633d7d7a1")
+
+    expected_uuid = "e0c3d987-a36c-4f9e-9b2f-90e633d7d7a1"
+    assert fs.identifier == expected_uuid


can be moved to tests/filesystems/test_extfs.py

Miauwkeru · 2025-10-21T10:48:26Z

tests/plugins/filesystem/test_uuid.py

+def test_filesystem_identifier_from_volume_guid() -> None:
+    """Filesystem.identifier returns a string when volume.guid is set."""
+    guid = "test" * 4
+
+    fs = MockFilesystem()
+    fs.volume = Mock(guid=guid)
+
+    assert fs.identifier == guid
+
+
+def test_filesystem_identifier_string_when_no_guid() -> None:
+    """Filesystem.identifier returns the volume name when volume.guid is None."""
+
+    fs = MockFilesystem()
+    fs.volume = Mock(guid=None)
+    fs.volume.name = "TestVolume"
+
+    assert fs.identifier == "TestVolume"
+
+
+def test_filesystem_identifier_string_when_no_guid_or_name() -> None:
+    """Filesystem.identifier returns the fs type name when volume.guid and volume name are None."""
+
+    fs = MockFilesystem()
+    fs.volume = Mock(guid=None)
+    fs.volume.name = None
+
+    assert fs.identifier == "filesystem_test"


Can be moved to tests/test_filesystem.py

loaflover · 2026-01-24T12:34:44Z

hey guys, im back after a long hiatus. had to merge because rebase was taking forever and was getting messy. also had to discard all added/changed tests due to (it seems) a test overhaul at least for this test file. will obv re-add them.

Co-authored-by: Miauwkeru <[email protected]>

…ssect.target into feature/uuid-walkfs

loaflover · 2026-01-24T13:42:59Z

i believe every issue has been delt with (:

Miauwkeru · 2026-01-30T09:55:26Z

dissect/target/plugins/filesystem/walkfs.py

+def get_disk_serial(filesystem: Filesystem) -> str | None:
+    """
+    Returns the disk serial number if it exists; otherwise, returns None.
+
+    Args:
+        entry: :class:`Filesystem` instance
+
+    Returns:
+        Disk serial number as str, or None if not available.
+    """
+    entry_volume = getattr(filesystem, "volume", None)
+    volume_vs = getattr(entry_volume, "vs", None)
+    return getattr(volume_vs, "serial", None)


We went back over it, and yes, getting the disk serial or disk identifier at this point is a bit useless as its only populated in an MBR volume system and is not populated otherwise. So it is better to remove this function all together.

If we want to add this functionality we need to make more changes to volume and volumesystem to change it

dissect/target/plugins/filesystem/walkfs.py

Miauwkeru · 2026-01-30T10:05:31Z

tests/plugins/filesystem/test_walkfs.py


 import pytest

 from dissect.target.filesystem import VirtualFile, VirtualFilesystem


with the proposed changes, the changes in this file can be removed

Co-authored-by: Miauwkeru <[email protected]>

…ssect.target into feature/uuid-walkfs

loaflover · 2026-01-30T13:48:59Z

ill make sure everything is changed

dissect/target/plugins/filesystem/walkfs.py

Co-authored-by: Miauwkeru <[email protected]>

Schamper · 2026-01-31T17:07:03Z

tests/plugins/filesystem/test_walkfs.py

This change will fail linting.

Schamper · 2026-01-31T17:07:30Z

dissect/target/filesystem.py

+                return guid
+            if name := getattr(self.volume, "name", None):
+                return name
+        log.error("no identifier found for filesystem of type %s", self.__type__)


Suggested change

log.error("no identifier found for filesystem of type %s", self.__type__)

Schamper · 2026-01-31T17:09:28Z

dissect/target/filesystem.py

+
+    @cached_property
+    def identifier(self) -> str:
+        """Returns the identifier of the Filesystem."""


Suggested change

"""Returns the identifier of the Filesystem."""

"""Returns the identifier of the filesystem.

Usually this is a serial or UUID, but it can also be a volume GUID or name.

"""

Schamper · 2026-01-31T17:10:42Z

dissect/target/filesystem.py

+        return None
+
+    @cached_property
+    def serial(self) -> int | str | None:


Suggested change

def serial(self) -> int | str | None:

def serial(self) -> int | str | None:

"""Return the serial of the filesystem if it has one."""

Schamper · 2026-01-31T17:10:51Z

dissect/target/filesystem.py

        return self.get(path).hash(algos)

+    @cached_property
+    def uuid(self) -> UUID | None:


Suggested change

def uuid(self) -> UUID | None:

def uuid(self) -> UUID | None:

"""Return the UUID of the filesystem if it has one."""

Schamper · 2026-01-31T17:14:12Z

dissect/target/filesystem.py

+            if name := getattr(self.volume, "name", None):
+                return name
+        log.error("no identifier found for filesystem of type %s", self.__type__)
+        return f"filesystem_{self.__type__}"


Why None? Or repr(self) as a fallback?

loaflover added 3 commits September 7, 2025 23:18

added disk serial and volume uuid

7a57b20

adding tests

04c0fd7

fixed default test failing. added ceck to see if volume doesnt exist.…

6fcd734

… removed serial for the test that needs it gone. finally finished :3

Miauwkeru requested changes Oct 16, 2025

View reviewed changes

loaflover and others added 22 commits October 17, 2025 13:37

fixed some of the requested chages

e6e1635

Apply suggestions from code review

40fec39

Co-authored-by: Miauwkeru <[email protected]>

Merge branch 'feature/uuid-walkfs' of https://github.com/loaflover/di…

05a7561

…ssect.target into feature/uuid-walkfs

more fixes

98865f4

more changes

06a5541

linting stuff

a53f925

added stuf

3ae2c78

added disk serial and volume uuid

72ce981

adding tests

24d4a49

fixed default test failing. added ceck to see if volume doesnt exist.…

a7dcc1a

… removed serial for the test that needs it gone. finally finished :3

fixed some of the requested chages

c22ca26

Apply suggestions from code review

d83e8b5

Co-authored-by: Miauwkeru <[email protected]>

more fixes

78ffbd8

more changes

c88fdc5

linting stuff

51a2986

added stuf

c124db2

Merge branch 'feature/uuid-walkfs' of https://github.com/loaflover/di…

242d243

…ssect.target into feature/uuid-walkfs

fixed all tests??

7508485

looks better idk

63db79d

oops...

206ecc1

added new test for new code

d0d1e18

linting i think

5f0ea91

loaflover added 6 commits October 20, 2025 21:16

applied some of that changes

fbc84b0

Merge branch 'feature/uuid-walkfs' of https://github.com/loaflover/di…

497fdbf

…ssect.target into feature/uuid-walkfs

fixed filesystems (:

4219d5e

oops

f400fb9

fixed issue with test

ae6d271

fixed test titles

197e7b9

loaflover requested a review from Miauwkeru October 20, 2025 19:09

Miauwkeru requested changes Oct 21, 2025

View reviewed changes

Miauwkeru changed the title ~~added disk serial and volume uuid to walkfs. made volume uuid change based on the volume fs.~~ Add filesystem identifiers to walkfs Oct 21, 2025

loaflover added 2 commits January 24, 2026 14:31

Merge remote-tracking branch 'upstream/main' into feature/uuid-walkfs

675da3d

ruff + hey yall im back

0cbc19d

loaflover and others added 6 commits January 24, 2026 15:14

fixes

ced4a6d

Apply suggestions from code review

8c13863

Co-authored-by: Miauwkeru <[email protected]>

Merge branch 'feature/uuid-walkfs' of https://github.com/loaflover/di…

c0fa1b1

…ssect.target into feature/uuid-walkfs

fixed some thingies

99742b8

fixed walkfs test (have not applied suggestions yet!)

2dc8f06

ruff

f1d2f8b

loaflover requested a review from Miauwkeru January 24, 2026 13:42

Miauwkeru reviewed Jan 30, 2026

View reviewed changes

loaflover and others added 3 commits January 30, 2026 15:42

Apply suggestions from code review

5a739d0

Co-authored-by: Miauwkeru <[email protected]>

fixed other requested change

c84310f

Merge branch 'feature/uuid-walkfs' of https://github.com/loaflover/di…

d89f264

…ssect.target into feature/uuid-walkfs

Miauwkeru reviewed Jan 30, 2026

View reviewed changes

dissect/target/plugins/filesystem/walkfs.py Outdated Show resolved Hide resolved

Update dissect/target/plugins/filesystem/walkfs.py

6110be9

Co-authored-by: Miauwkeru <[email protected]>

loaflover requested a review from Miauwkeru January 31, 2026 11:37

Schamper requested changes Jan 31, 2026

View reviewed changes

		@internal
		def get_disk_serial(entry: FilesystemEntry) -> str:

		assert get_disk_serial(mock_fs_entry) is None


		def test_walkfs_plugin(target_unix: Target, fs_unix: VirtualFilesystem) -> None:


		import pytest

		from dissect.target.filesystem import VirtualFile, VirtualFilesystem

-        """Returns the identifier of the Filesystem."""
+        """Returns the identifier of the filesystem.
+        Usually this is a serial or UUID, but it can also be a volume GUID or name.
+        """

	def serial(self) -> int \| str \| None:
	def serial(self) -> int \| str \| None:
	"""Return the serial of the filesystem if it has one."""

	def uuid(self) -> UUID \| None:
	def uuid(self) -> UUID \| None:
	"""Return the UUID of the filesystem if it has one."""

Add filesystem identifiers to walkfs #1316

Are you sure you want to change the base?

Add filesystem identifiers to walkfs #1316

Uh oh!

Conversation

loaflover commented Sep 7, 2025

Uh oh!

loaflover commented Sep 10, 2025

Uh oh!

Schamper commented Sep 12, 2025

Uh oh!

loaflover commented Sep 12, 2025

Uh oh!

loaflover commented Sep 28, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

loaflover commented Oct 20, 2025

Uh oh!

Miauwkeru left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

loaflover commented Jan 24, 2026

Uh oh!

loaflover commented Jan 24, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!