dlandon / unassigned.devices

Unassigned Devices plugin for unRAID
Other
81 stars 39 forks source link

apfs-fuse password option fails due to errant space character #102

Closed npondel closed 1 month ago

npondel commented 2 months ago

Somehow, a space is being added to the mount command for APFS encrypted volumes, which breaks the command syntax. If I remove that space and run the command manually in the terminal, the partition mounts.

image

https://github.com/dlandon/unassigned.devices/blob/79e127e8f592c0c986d0053e73ac2720905bd661/source/Unassigned.devices/include/lib.php#L1808

What I can't immediately tell is if it's the $vol or the $recovery that's adding the space (or PHP nonsense?). Happy to help debug this, I just don't really know PHP.

npondel commented 2 months ago

Actually, I think it has to be a bug in the $recovery variable construction. It adds the comma following vol=0. For the life of me, I can't see how a space is possibly getting in there, though.

https://github.com/dlandon/unassigned.devices/blob/79e127e8f592c0c986d0053e73ac2720905bd661/source/Unassigned.devices/include/lib.php#L1805

npondel commented 2 months ago

I think I see it now...

https://github.com/dlandon/unassigned.devices/blob/79e127e8f592c0c986d0053e73ac2720905bd661/source/Unassigned.devices/include/lib.php#L1861

Line 1861 sets the $recovery to replace the password with *, for safe logging, however this appears to happen before the apfs mount command actually executes on line 1881.

https://github.com/dlandon/unassigned.devices/blob/79e127e8f592c0c986d0053e73ac2720905bd661/source/Unassigned.devices/include/lib.php#L1881

So 1) we need to remove that space in 1861 and 2) we need to move that masking command after 1881?

I will make a pull request! Let me know if you think I'm right 😄

npondel commented 1 month ago

Confirmed fixed in 2024.06.30!