Hello,
Currently the `svn patch` command changes the permissions on any file
it patches to 600. This occurs because it creates a file under the
system temporary directory for applying the patch, and then copies
this file to the final destination. `apr_file_mktemp` sensibly assigns
mode 600 to files created under the system temporary directory to
avoid them being exposed to the entire system. So the result is that
any file that `svn patch` patches ends up with 600 permissions rather
than whatever it had before.
Here is an example of the current behaviour (before my patch):
[Cathy@localhost subversion]$ ls -la INSTALL
-rw-rw-r--. 1 Cathy Cathy 63057 Nov 20 17:34 INSTALL
[Cathy@localhost subversion]$ ./svn patch patch.diff
U INSTALL
[Cathy@localhost subversion]$ ls -la INSTALL
-rw-------. 1 Cathy Cathy 63070 Nov 20 17:58 INSTALL
Notice that applying the patch has changed the permissions of INSTALL.
After my patch, we instead get the following:
[Cathy@localhost subversion]$ ls -la INSTALL
-rw-rw-r--. 1 Cathy Cathy 63057 Nov 20 17:59 INSTALL
[Cathy@localhost subversion]$ ./svn patch patch.diff
U INSTALL
[Cathy@localhost subversion]$ ls -la INSTALL
-rw-rw-r--. 1 Cathy Cathy 63070 Nov 20 17:59 INSTALL
As expected, patching the file no longer alters its permissions.
I have attached my patch to this email and it includes a log message.
Regards,
Cathy
[[[
svn patch: Don't change permissions of patched files
* subversion/libsvn_client/patch.c
(init_patch_target): Create temporary file for result of patch
under the working directory, rather than under the system
temporary directory. This is already how EOL translation
works (see `svn_subst_copy_and_translate4`) and has the
advantage that we can safely change permissions on the file
without potentially exposing the file to the entire system.
(install_patched_target): Make sure the final patched file has
the same permissions as the original file before patching.
Patch by: Cathy J. Fitzpatrick <[email protected]>
]]]
Index: subversion/libsvn_client/patch.c
===================================================================
--- subversion/libsvn_client/patch.c (revision 1543974)
+++ subversion/libsvn_client/patch.c (working copy)
@@ -1076,23 +1076,33 @@
* ### we should have kept the patch field in patch_target_t to be
* ### able to distinguish between 'what the patch says we should do'
* ### and 'what we can do with the given state of our WC'. */
if (patch->operation == svn_diff_op_added)
target->added = TRUE;
else if (patch->operation == svn_diff_op_deleted)
target->deleted = TRUE;
if (! target->is_symlink)
{
- /* Open a temporary file to write the patched result to. */
+ /* Open a temporary file to write the patched result to.
+ *
+ * Create the temporary file under the working directory (the same
+ * as EOL translation uses) so that it is safe to change its file
+ * permissions without exposing the file to the entire system, which
+ * could happen if the file were under the system temporary
+ * directory, rather than the working directory.
+ */
SVN_ERR(svn_io_open_unique_file3(&target->patched_file,
- &target->patched_path, NULL,
+ &target->patched_path,
+ svn_dirent_dirname(
+ wcroot_abspath,
+ scratch_pool),
remove_tempfiles ?
svn_io_file_del_on_pool_cleanup :
svn_io_file_del_none,
result_pool, scratch_pool));
/* Put the write callback in place. */
content->write = write_file;
content->write_baton = target->patched_file;
}
else
@@ -2564,20 +2574,27 @@
svn_boolean_t repair_eol;
/* Copy the patched file on top of the target file.
* Always expand keywords in the patched file, but repair EOL
* only if svn:eol-style dictates a particular style. */
repair_eol = (target->content->eol_style ==
svn_subst_eol_style_fixed ||
target->content->eol_style ==
svn_subst_eol_style_native);
+ /* Make sure the patched file has the same permissions the
+ * original file. By design, `patched_path` will be under
+ * the working directory, so this does introduce any
+ * security issues with temp files. */
+ SVN_ERR(svn_io_copy_perms(
+ target->local_abspath, target->patched_path, pool));
+
SVN_ERR(svn_subst_copy_and_translate4(
target->patched_path, target->local_abspath,
target->content->eol_str, repair_eol,
target->content->keywords,
TRUE /* expand */, FALSE /* special */,
ctx->cancel_func, ctx->cancel_baton, pool));
}
if (target->added || target->replaced)
{
