From: Tomi Ollila <tomi.ollila@iki.fi>
To: Daniel Kahn Gillmor <dkg@fifthhorseman.net>,
Notmuch Mail <notmuch@notmuchmail.org>
Subject: Re: [PATCH v3] cli/insert: add --world-readable flag
Date: Thu, 15 Feb 2018 08:08:18 +0200 [thread overview]
Message-ID: <m2fu62rdil.fsf@guru.guru-group.fi> (raw)
In-Reply-To: <20180209041058.4037-1-dkg@fifthhorseman.net>
On Thu, Feb 08 2018, Daniel Kahn Gillmor wrote:
> In some cases (e.g. when building a publicly-visible e-mail archive)
> it doesn't make any sense to restrict visibility of the message to the
> current user account.
>
> This adds a --world-readable boolean option for "notmuch insert", so
> that those who want to archive their mail publicly can feed their
> archiver with:
>
> notmuch insert --world-readable
>
> Other local delivery agents (postfix's local, and dovecot's lda) all
> default to delivery in mode 0600 rather than relying on the user's
> umask, so this fix doesn't change the default.
>
> Also, this does not override the user's umask. if the umask is
> already set tight, it will not become looser as the result of passing
> --world-readable.
Code looks good to me and tests ... also, +1
testing is too hard atm...
Tomi
>
> Signed-off-by: Daniel Kahn Gillmor <dkg@fifthhorseman.net>
> ---
> doc/man1/notmuch-insert.rst | 6 ++++++
> notmuch-insert.c | 25 ++++++++++++++-----------
> test/T070-insert.sh | 45 +++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 65 insertions(+), 11 deletions(-)
>
> diff --git a/doc/man1/notmuch-insert.rst b/doc/man1/notmuch-insert.rst
> index 47884515..86e2f567 100644
> --- a/doc/man1/notmuch-insert.rst
> +++ b/doc/man1/notmuch-insert.rst
> @@ -51,6 +51,12 @@ Supported options for **insert** include
> ``--no-hooks``
> Prevent hooks from being run.
>
> +``--world-readable``
> + When writing mail to the mailbox, allow it to be read by users
> + other than the current user. Note that this does not override
> + umask. By default, delivered mail is only readable by the current
> + user.
> +
> ``--decrypt=(true|nostash|auto|false)``
> If ``true`` and the message is encrypted, try to decrypt the
> message while indexing, stashing any session keys discovered. If
> diff --git a/notmuch-insert.c b/notmuch-insert.c
> index 48490b51..d229c9dc 100644
> --- a/notmuch-insert.c
> +++ b/notmuch-insert.c
> @@ -159,10 +159,10 @@ mkdir_recursive (const void *ctx, const char *path, int mode)
> * otherwise. Partial results are not cleaned up on errors.
> */
> static bool
> -maildir_create_folder (const void *ctx, const char *maildir)
> +maildir_create_folder (const void *ctx, const char *maildir, bool world_readable)
> {
> const char *subdirs[] = { "cur", "new", "tmp" };
> - const int mode = 0700;
> + const int mode = (world_readable ? 0755 : 0700);
> char *subdir;
> unsigned int i;
>
> @@ -211,10 +211,11 @@ tempfilename (const void *ctx)
> * is not touched).
> */
> static int
> -maildir_mktemp (const void *ctx, const char *maildir, char **path_out)
> +maildir_mktemp (const void *ctx, const char *maildir, bool world_readable, char **path_out)
> {
> char *filename, *path;
> int fd;
> + const int mode = (world_readable ? 0644 : 0600);
>
> do {
> filename = tempfilename (ctx);
> @@ -227,7 +228,7 @@ maildir_mktemp (const void *ctx, const char *maildir, char **path_out)
> return -1;
> }
>
> - fd = open (path, O_WRONLY | O_CREAT | O_TRUNC | O_EXCL, 0600);
> + fd = open (path, O_WRONLY | O_CREAT | O_TRUNC | O_EXCL, mode);
> } while (fd == -1 && errno == EEXIST);
>
> if (fd == -1) {
> @@ -289,12 +290,12 @@ copy_fd (int fdout, int fdin)
> * the file, or NULL on errors.
> */
> static char *
> -maildir_write_tmp (const void *ctx, int fdin, const char *maildir)
> +maildir_write_tmp (const void *ctx, int fdin, const char *maildir, bool world_readable)
> {
> char *path;
> int fdout;
>
> - fdout = maildir_mktemp (ctx, maildir, &path);
> + fdout = maildir_mktemp (ctx, maildir, world_readable, &path);
> if (fdout < 0)
> return NULL;
>
> @@ -323,11 +324,11 @@ FAIL:
> * errors.
> */
> static char *
> -maildir_write_new (const void *ctx, int fdin, const char *maildir)
> +maildir_write_new (const void *ctx, int fdin, const char *maildir, bool world_readable)
> {
> char *cleanpath, *tmppath, *newpath, *newdir;
>
> - tmppath = maildir_write_tmp (ctx, fdin, maildir);
> + tmppath = maildir_write_tmp (ctx, fdin, maildir, world_readable);
> if (! tmppath)
> return NULL;
> cleanpath = tmppath;
> @@ -457,6 +458,7 @@ notmuch_insert_command (notmuch_config_t *config, int argc, char *argv[])
> bool create_folder = false;
> bool keep = false;
> bool hooks = true;
> + bool world_readable = false;
> bool synchronize_flags;
> char *maildir;
> char *newpath;
> @@ -467,7 +469,8 @@ notmuch_insert_command (notmuch_config_t *config, int argc, char *argv[])
> { .opt_string = &folder, .name = "folder", .allow_empty = true },
> { .opt_bool = &create_folder, .name = "create-folder" },
> { .opt_bool = &keep, .name = "keep" },
> - { .opt_bool = &hooks, .name = "hooks" },
> + { .opt_bool = &hooks, .name = "hooks" },
> + { .opt_bool = &world_readable, .name = "world-readable" },
> { .opt_inherit = notmuch_shared_indexing_options },
> { .opt_inherit = notmuch_shared_options },
> { }
> @@ -523,7 +526,7 @@ notmuch_insert_command (notmuch_config_t *config, int argc, char *argv[])
> }
>
> strip_trailing (maildir, '/');
> - if (create_folder && ! maildir_create_folder (config, maildir))
> + if (create_folder && ! maildir_create_folder (config, maildir, world_readable))
> return EXIT_FAILURE;
>
> /* Set up our handler for SIGINT. We do not set SA_RESTART so that copying
> @@ -535,7 +538,7 @@ notmuch_insert_command (notmuch_config_t *config, int argc, char *argv[])
> sigaction (SIGINT, &action, NULL);
>
> /* Write the message to the Maildir new directory. */
> - newpath = maildir_write_new (config, STDIN_FILENO, maildir);
> + newpath = maildir_write_new (config, STDIN_FILENO, maildir, world_readable);
> if (! newpath) {
> return EXIT_FAILURE;
> }
> diff --git a/test/T070-insert.sh b/test/T070-insert.sh
> index 40519bb2..05be473a 100755
> --- a/test/T070-insert.sh
> +++ b/test/T070-insert.sh
> @@ -4,6 +4,10 @@ test_description='"notmuch insert"'
>
> test_require_external_prereq gdb
>
> +# subtests about file permissions assume that we're working with umask
> +# 022 by default.
> +umask 022
> +
> # Create directories and database before inserting.
> mkdir -p "$MAIL_DIR"/{cur,new,tmp}
> mkdir -p "$MAIL_DIR"/Drafts/{cur,new,tmp}
> @@ -37,6 +41,9 @@ notmuch insert < "$gen_msg_filename"
> cur_msg_filename=$(notmuch search --output=files "subject:insert-subject")
> test_expect_equal_file "$cur_msg_filename" "$gen_msg_filename"
>
> +test_begin_subtest "Permissions on inserted message should be 0600"
> +test_expect_equal "600" "$(stat -c %a "$cur_msg_filename")"
> +
> test_begin_subtest "Insert message adds default tags"
> output=$(notmuch show --format=json "subject:insert-subject")
> expected='[[[{
> @@ -73,6 +80,27 @@ notmuch insert +custom < "$gen_msg_filename"
> output=$(notmuch search --output=messages tag:custom)
> test_expect_equal "$output" "id:$gen_msg_id"
>
> +test_begin_subtest "Insert tagged world-readable message"
> +gen_insert_msg
> +notmuch insert --world-readable +world-readable-test < "$gen_msg_filename"
> +cur_msg_filename=$(notmuch search --output=files "tag:world-readable-test")
> +test_expect_equal_file "$cur_msg_filename" "$gen_msg_filename"
> +
> +test_begin_subtest "Permissions on inserted world-readable message should be 0644"
> +test_expect_equal "644" "$(stat -c %a "$cur_msg_filename")"
> +
> +test_begin_subtest "Insert tagged world-readable message with group-only umask"
> +oldumask=$(umask)
> +umask 027
> +gen_insert_msg
> +notmuch insert --world-readable +world-readable-umask-test < "$gen_msg_filename"
> +cur_msg_filename=$(notmuch search --output=files "tag:world-readable-umask-test")
> +umask "$oldumask"
> +test_expect_equal_file "$cur_msg_filename" "$gen_msg_filename"
> +
> +test_begin_subtest "Permissions on inserted world-readable message with funny umask should be 0640"
> +test_expect_equal "640" "$(stat -c %a "$cur_msg_filename")"
> +
> test_begin_subtest "Insert message, add/remove tags"
> gen_insert_msg
> notmuch insert +custom -unread < "$gen_msg_filename"
> @@ -170,6 +198,23 @@ output=$(notmuch search --output=files path:F/G/H/I/J/new tag:folder)
> basename=$(basename "$output")
> test_expect_equal_file "$gen_msg_filename" "${MAIL_DIR}/F/G/H/I/J/new/${basename}"
>
> +test_begin_subtest "Created subfolder should have permissions 0700"
> +test_expect_equal "700" "$(stat -c %a "${MAIL_DIR}/F/G/H/I/J")"
> +test_begin_subtest "Created subfolder new/ should also have permissions 0700"
> +test_expect_equal "700" "$(stat -c %a "${MAIL_DIR}/F/G/H/I/J/new")"
> +
> +test_begin_subtest "Insert message, create world-readable subfolder"
> +gen_insert_msg
> +notmuch insert --folder=F/G/H/I/J/K --create-folder --world-readable +folder-world-readable < "$gen_msg_filename"
> +output=$(notmuch search --output=files path:F/G/H/I/J/K/new tag:folder-world-readable)
> +basename=$(basename "$output")
> +test_expect_equal_file "$gen_msg_filename" "${MAIL_DIR}/F/G/H/I/J/K/new/${basename}"
> +
> +test_begin_subtest "Created world-readable subfolder should have permissions 0755"
> +test_expect_equal "755" "$(stat -c %a "${MAIL_DIR}/F/G/H/I/J/K")"
> +test_begin_subtest "Created world-readable subfolder new/ should also have permissions 0755"
> +test_expect_equal "755" "$(stat -c %a "${MAIL_DIR}/F/G/H/I/J/K/new")"
> +
> test_begin_subtest "Insert message, create existing subfolder"
> gen_insert_msg
> notmuch insert --folder=F/G/H/I/J --create-folder +folder < "$gen_msg_filename"
> --
> 2.15.1
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> https://notmuchmail.org/mailman/listinfo/notmuch
next prev parent reply other threads:[~2018-02-15 6:08 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-02-05 4:37 [PATCH] cli/insert: new message file can be world-readable (rely on umask) Daniel Kahn Gillmor
2018-02-05 11:59 ` Peter Wang
2018-02-06 19:43 ` [PATCH v2] " Daniel Kahn Gillmor
2018-02-09 1:40 ` Daniel Kahn Gillmor
2018-02-09 1:52 ` Brian Sniffen
2018-02-09 2:33 ` Daniel Kahn Gillmor
2018-02-09 2:00 ` Daniel Kahn Gillmor
2018-02-09 4:10 ` [PATCH v3] cli/insert: add --world-readable flag Daniel Kahn Gillmor
2018-02-15 6:08 ` Tomi Ollila [this message]
2018-03-24 23:17 ` David Bremner
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://notmuchmail.org/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=m2fu62rdil.fsf@guru.guru-group.fi \
--to=tomi.ollila@iki.fi \
--cc=dkg@fifthhorseman.net \
--cc=notmuch@notmuchmail.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://yhetil.org/notmuch.git/
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).