Permission display binary path and line break. #11657
Conversation
| } else { | ||
| const auto LOOKUP = binaryNameForPid(pid); | ||
| description = std::format(std::runtime_format(permissionToHumanString(type)), lookup, binaryPath); | ||
| description = std::format(std::runtime_format(permissionToHumanString(type)), std::format("{}<br/> ({})", lookup, binaryPath)); |
There was a problem hiding this comment.
Just out of curiosity, how are non-printing characters displayed here?
Since the binaryPath shown is supposed to allow the user to actually check the file that is going to be to be executed, the displayed name should reflect precisely the actual path, displaying non-printing characters in some way
There was a problem hiding this comment.
Good point. Would using {:?} debug formatting work?
| std::string binaryName = binaryPath.contains("/") ? binaryPath.substr(binaryPath.find_last_of('/') + 1) : binaryPath; | ||
| description = std::format(std::runtime_format(permissionToHumanString(type)), std::format("{}</b> ({})", binaryName, binaryPath)); | ||
| std::string binaryName = std::filesystem::path(binaryPath).filename().string(); | ||
| description = std::format(std::runtime_format(permissionToHumanString(type)), std::format("{:?}<br/>({:?})", binaryName, binaryPath)); |
There was a problem hiding this comment.
I dont think this is a perfectly sound idea.
An application myApp
/usr/bin/myApp is trying to capture your screen
that looks... cursed?
There was a problem hiding this comment.
I mean for such security-related stuff it's a valuable info to know for the user. We don't want to let arbitrary apps fool the user by setting "OBS" as their window title, right?
Perhaps better phrasing in the dialog window could help make it look a bit nicer
There was a problem hiding this comment.
It's actually not window title, just the filename of the full binary path so it does look cursed, but idk good UI design :/
There was a problem hiding this comment.
It's actually not window title, just the filename of the full binary path so it does look cursed, but idk good UI design :/
I didn't see that, anyways I feel like having the full path somewhere should be better. Either a full path or a PID or something else that uniquely identifies an app would work.
It might a bit of a stretch, but imagine with the current setup some malicious app creates an executable somewhere (and thus chooses the executable file name) and runs it, and this executable makes the request, then forwarding the display capture to anything it wants.
This might be a stupid argument because when it reaches the point where an app can create and run an arbitrary executable you might be screwed already for anything security-related, idk.
There was a problem hiding this comment.
what's wrong with the current design though? It shows both the path and the name...
Actually display binary path for permission popup.
Also fixed a typo what looked like should be a line-break.
(Ready for merge)