Bug 333577 - QSaveFile: kate changes file owner
Summary: QSaveFile: kate changes file owner
Status: RESOLVED FIXED
Alias: None
Product: kate
Classification: Applications
Component: general (show other bugs)
Version: unspecified
Platform: unspecified Linux
: HI major
Target Milestone: ---
Assignee: KWrite Developers
URL:
Keywords:
: 322715 333214 339510 362201 376376 (view as bug list)
Depends on:
Blocks:
 
Reported: 2014-04-18 11:52 UTC by Hannes
Modified: 2018-08-18 09:14 UTC (History)
14 users (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Hannes 2014-04-18 11:52:58 UTC
This concerns kde version 4.10.5 .

I normal user I can open files of owner root, edit them and then save them.
These files are saved with my user and group id.
So the group and user id changes.
I think this is a major security risk.
I can do the same thing with kwrite.
I suspect that this might be a bug in a library and not in Kate itself.
Comment 1 Dominik Haumann 2014-04-29 11:44:31 UTC
@David: A possible solution would be to add K/QSaveFile::setDirectWriteMode(mode), where mode is
- WriteOnMissingPermissions
- WriteOnPermissionChanges
or similar. This ways, one could tell K/QSaveFile to directly write to avoid wrong file permissions.
Comment 2 Dominik Haumann 2014-09-29 16:21:11 UTC
*** Bug 339510 has been marked as a duplicate of this bug. ***
Comment 3 Dominik Haumann 2014-09-29 16:23:18 UTC
*** Bug 322715 has been marked as a duplicate of this bug. ***
Comment 4 Greg Rundlett 2014-09-29 16:51:04 UTC
My bug report https://bugs.kde.org/show_bug.cgi?id=339510 was marked as duplicate.  For people waiting for a fix, the (bad) workaround that I described is to launch Kate (or Kwrite) with sudo.
Comment 5 Dominik Haumann 2014-10-29 19:46:27 UTC
*** Bug 333214 has been marked as a duplicate of this bug. ***
Comment 6 Michal Humpula 2015-02-18 18:26:50 UTC
Interestingly enough, the Vim editor reads the original file permissions (mode, owner, group, even acl and times) and tries to preserve them to a newly created file. But in case it detects it will not be able to do that, it writes straight to old file. Part of the writing code has this commment:

/*
* Don't rename the file when:
 * - it's a hard link
 * - it's a symbolic link
 * - we don't have write permission in the directory
 * - we can't set the owner/group of the new file
 */

Considering that linux doesn't possess transactional file api, this seems to me the only way to do it. @Dominic, agreed?
Comment 7 Dominik Haumann 2015-02-22 12:11:21 UTC
Yes, I agree. It can be added directly to Kate, or even to QSaveFile.
Comment 8 Christian Quast 2016-04-14 00:24:50 UTC
are there any news on this? 

For me  it causes wrong file permissions (+x) for (source) files in a subversion repository if the files are mounted using sshfs (see https://bugs.kde.org/show_bug.cgi?id=333214 which is marked as duplicate).
Comment 9 coyote4til7 2016-04-27 16:59:19 UTC
Confirmed in Kate Version 3.13.3.

It doesn't seem to be related to permissions on the file itself or the file's containing folder. It also affects only certain files. When it does happen, every save makes the edited file invisible to the webserver. AKA.. editing projects I'm working on, breaks the projects.

This bug is two years old this month and no indication anyone is working on this bug. Has Kate development stopped?
Comment 10 pk 2016-06-30 09:12:50 UTC
I just upgraded from ubuntu 12.04 to 16.04 ....

suid bit  of files ( chmod a+sx ) before editing are reset after a write with kwrite...
it was ok on previous version...

kwrite old (ok) version :
kwrite -v ---->
Qt : 4.8.1
Plate-forme de développement de KDE : 4.8.5 (4.8.5)
KWrite : 4.8.5 (4.8.5)

new version with bug:
kwrite -v ---->
kwrite 15.12.3
Comment 11 Friedrich W. H. Kossebau 2016-10-01 21:06:46 UTC
Still confirmed with KF5 5.26. Not only ownership is reset, but extended attributes are also lost.

This behaviour is very annoying in many situations for me, so I am finally ready to try to scratch my itch here.
@Dominik: Did I correctly identify the code that needs changing to be Kate::TextBuffer::save() in KTextEditor?
IMHO the fix should be in QSaveFile, as writing to a file should not suddenly have a side-effect of changed ownership or other properties, unless requested (modulo modification time of course :) ).
But I do not have a Qt development setup and also would like a workaround in KTextEditor until some released version of Qt with the fix has made it to my work machines.
Has there been some further discussion on the topic? Any recommendations what I should try to do? I could not find any Qt API to change file properties besides permissions.
Comment 12 Dominik Haumann 2016-10-02 17:12:25 UTC
Hi Friedrich,

yes, a possible way is to fix it in Kate::TextBuffer.

Having talked to dfaure about it, we already came to the conclusion that it should be fixed in Qt: Since QSaveFile already has a function called directWriteFallback(), it is the correct way to extend the checks to fallback t a direct write in case of the mentioned corner cases.

While a workaround in Kate::TextBuffer would of course improve the situation, the proper fix is definitely in QSaveFile.

See also:
- https://bugs.kde.org/show_bug.cgi?id=354405
- https://bugs.kde.org/show_bug.cgi?id=358457
Comment 13 Friedrich W. H. Kossebau 2016-10-04 16:07:27 UTC
For a start I opened a bug now upstream for QSaveFile:
https://bugreports.qt.io/browse/QTBUG-56366, "QSaveFile results in loss of extended file attributes and change of file ownership"

For some intermediate work-around in Kate::TextBuffer I still have to learn more about stuff and system APIs that would be used, hope to look into it the next days more.
Controlling file ownership though might be a unsolvable one.

As there are some users who might value to no lose ownership data or hard links over support for transactional saving, perhaps there could be some flag somewhere in the KTextEditor API which allows to control this and which could also be forwarded into the Kate module config GUI?
At least I have never experienced any problems with incomplete writes, but this bug needs me to do lots of fixups whenever I use KWrite, Kate & KDevelop with system settings or in special development setups. So I would instantly toggle such a setting :)
Comment 14 Dominik Haumann 2017-02-18 11:55:53 UTC
*** Bug 376376 has been marked as a duplicate of this bug. ***
Comment 15 Dominik Haumann 2017-02-26 16:43:07 UTC
*** Bug 362201 has been marked as a duplicate of this bug. ***
Comment 16 Dominik Haumann 2017-02-26 16:44:31 UTC
*** Bug 362733 has been marked as a duplicate of this bug. ***
Comment 17 Christoph Cullmann 2018-08-18 09:14:02 UTC
Remove QSaveFile in favor of plain old file saving

Summary: Rationale: for many use cases that e.g. have acls, complex other extended attributes, static links e.g. the rename() doesnt do the trick it should other ways would be start to add workarounds to all cases, which is hard, e.g. if that is something shared via SMB...

Test Plan: make && make test

Reviewers: dhaumann, dfaure

Reviewed By: dhaumann, dfaure

Subscribers: dfaure, kwrite-devel, kde-frameworks-devel

Tags: #kate, #frameworks

Differential Revision: https://phabricator.kde.org/D14890