KIO Polkit review
Issue 1
Architecture wise I don't like the FDReciever class: We want to send an FD back from the helper to the client slave.
- We start with polkit which can do any DBus call which natively pass file descriptors
- We use an abstraction on top (Kauth) which doesn't expose this
- So we create a new layer(!) on top where we create a side channel socket to pass file descriptors to solve a problem that we didn't have in the first place?
Not only is it a bit silly, but it means replacing XDG_RUNTIME_DIR/filehelper/* immediately after a job is started means a random client will get the FD's. That's a security problem.
Either add FD support to kauth (following up on https://phabricator.kde.org/D24245) or drop kauth and just duplicate/export KAuth's Polkit1Backend::isCallerAuthorized and use the lower level. I would strongly suggest the latter it gives more flexibility.
Issue 2
There's a bit of code I find suspect in file_unix ::copy
auto attemptWithAuthentication = [=]() {
auto err = execWithElevatedPrivilege(COPY, {srcUrl, destUrl}, errno);
if (err) {
if (!err.wasCanceled()) {
error(KIO::ERR_UNKNOWN, QString());
}
} else {
finished();
}
};
If the user cancel's an auth prompt we just fall through silently. Given a move across filesystems is a copy followed by a delete presumably we just randomly delete user's files?
Other
There's a lot of unresolved issues left on !143 (merged) It's easy to lose track of them now that the review is merged.