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 <ca...@cathyjf.com> ]]] 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) {