commit: Add --mode-ro-executables option

I think we should encourage removing the writable bits from
executables.  This has happened to me:
https://thomask.sdf.org/blog/2019/11/09/take-care-editing-bash-scripts.html

And not having the writable bit may help prevent hardlink
corruption with OSTree in some cases.

We can't do this by default, but add a convenient CLI flag
for it.

diff --git a/src/ostree/ot-builtin-commit.c b/src/ostree/ot-builtin-commit.c
index 88a5d4f4bf9e8079719ce8cd79dfb78231220661..253a58f721a86d94f6c6efe37e0cd34bfd9d9d25 100644 (file)
--- a/src/ostree/ot-builtin-commit.c
+++ b/src/ostree/ot-builtin-commit.c
@@ -56,6 +56,7 @@ static gboolean opt_no_xattrs;
 static char *opt_selinux_policy;
 static gboolean opt_selinux_policy_from_base;
 static gboolean opt_canonical_permissions;
+static gboolean opt_ro_executables;
 static gboolean opt_consume;
 static gboolean opt_devino_canonical;
 static char *opt_base;
@@ -111,6 +112,7 @@ static GOptionEntry options[] = {
   { "owner-uid", 0, 0, G_OPTION_ARG_INT, &opt_owner_uid, "Set file ownership user id", "UID" },
   { "owner-gid", 0, 0, G_OPTION_ARG_INT, &opt_owner_gid, "Set file ownership group id", "GID" },
   { "canonical-permissions", 0, 0, G_OPTION_ARG_NONE, &opt_canonical_permissions, "Canonicalize permissions in the same way bare-user does for hardlinked files", NULL },
+  { "mode-ro-executables", 0, 0, G_OPTION_ARG_NONE, &opt_ro_executables, "Ensure executable files are not writable", NULL },
   { "no-xattrs", 0, 0, G_OPTION_ARG_NONE, &opt_no_xattrs, "Do not import extended attributes", NULL },
   { "selinux-policy", 0, 0, G_OPTION_ARG_FILENAME, &opt_selinux_policy, "Set SELinux labels based on policy in root filesystem PATH (may be /)", "PATH" },
   { "selinux-policy-from-base", 'P', 0, G_OPTION_ARG_NONE, &opt_selinux_policy_from_base, "Set SELinux labels based on first --tree argument", NULL },
@@ -194,13 +196,19 @@ commit_filter (OstreeRepo         *self,
     g_file_info_set_attribute_uint32 (file_info, "unix::uid", opt_owner_uid);
   if (opt_owner_gid >= 0)
     g_file_info_set_attribute_uint32 (file_info, "unix::gid", opt_owner_gid);
+  guint mode = g_file_info_get_attribute_uint32 (file_info, "unix::mode");
+
+  if (S_ISREG (mode) && opt_ro_executables && (mode & (S_IXUSR | S_IXGRP | S_IXOTH)))
+    {
+      mode &= ~(S_IWUSR | S_IWGRP | S_IWOTH);
+      g_file_info_set_attribute_uint32 (file_info, "unix::mode", mode);
+    }
 
   if (mode_adds && g_hash_table_lookup_extended (mode_adds, path, NULL, &value))
     {
-      guint current_mode = g_file_info_get_attribute_uint32 (file_info, "unix::mode");
       guint mode_add = GPOINTER_TO_UINT (value);
       g_file_info_set_attribute_uint32 (file_info, "unix::mode",
-                                        current_mode | mode_add);
+                                        mode | mode_add);
       g_hash_table_remove (mode_adds, path);
     }
   else if (mode_overrides && g_hash_table_lookup_extended (mode_overrides, path, NULL, &value))
@@ -572,6 +580,7 @@ ostree_builtin_commit (int argc, char **argv, OstreeCommandInvocation *invocatio
       || opt_statoverride_file != NULL
       || opt_skiplist_file != NULL
       || opt_no_xattrs
+      || opt_ro_executables
       || opt_selinux_policy
       || opt_selinux_policy_from_base)
     {

diff --git a/tests/basic-test.sh b/tests/basic-test.sh
index ba88cc73170ef0ea2037603ef00381726463188d..97cd05e23166bd44c8840997afd3a496ba1151a8 100644 (file)
--- a/tests/basic-test.sh
+++ b/tests/basic-test.sh
@@ -21,7 +21,7 @@
 
 set -euo pipefail
 
-echo "1..$((88 + ${extra_basic_tests:-0}))"
+echo "1..$((89 + ${extra_basic_tests:-0}))"
 
 CHECKOUT_U_ARG=""
 CHECKOUT_H_ARGS="-H"
@@ -541,6 +541,24 @@ fi
 assert_file_has_mode checkout-test2-override/a/readable-only 600
 echo "ok commit statoverride"
 
+cd ${test_tmpdir}
+rm test2-checkout -rf
+$OSTREE checkout test2 test2-checkout
+cd test2-checkout
+install -m 0755 /dev/null user-wx
+install -m 0575 /dev/null group-wx
+install -m 0775 /dev/null both-wx
+install -m 0555 /dev/null ugox
+install -m 0644 /dev/null user-writable
+cd ..
+$OSTREE commit ${COMMIT_ARGS} -b test2-w-xor-x --mode-ro-executables --tree=dir=test2-checkout
+$OSTREE ls test2-w-xor-x > ls.txt
+for x in /{user,group,both}-wx; do
+    assert_file_has_content ls.txt '^-00555 .*'$x
+done
+assert_file_has_content ls.txt '^-00644 .*/user-writable'
+echo "ok commit --mode-ro-executables"
+
 cd ${test_tmpdir}
 cat > test-skiplist.txt <<EOF
 /a/nested/3