From daac6bd424b579e457a50606d057efd00fd94dfe Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Fri, 14 Aug 2026 20:07:19 +0200 Subject: [PATCH] fix(daemon): name the directory that actually refused, not the one after it posix_directory_parent_secure(current_fd) validates the directory we are ALREADY IN, but the refusal message printed `component` - the child about to be entered. Every reporter has therefore been sent to inspect the wrong directory: #1537 read "ancestor '.cache'" when /Users/ was refusing #1621 read "ancestor 'cbm-daemon-501'" when /private/tmp was refusing Both inspected the named directory, found it clean, and said so. They were right. #1537 has been open for weeks with the reporter repeatedly confirming a correct `.cache` - including on v0.10.4 today - because we kept pointing at it. The message now names the CONTAINING directory and says explicitly not to check the component itself. This fixes no permission logic. It changes weeks of talking past each other into a report someone can act on in a minute, which for this class of bug is the whole game: the refusal is invisible from outside, so the message IS the diagnosis. Found while reviewing the CBM_RUNTIME_DIR work; the original detail was added in the earlier half of #1537 and named the wrong variable from the start. Signed-off-by: Martin Vogel --- src/daemon/ipc.c | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/src/daemon/ipc.c b/src/daemon/ipc.c index 5fdb7efe1..4ef1a238a 100644 --- a/src/daemon/ipc.c +++ b/src/daemon/ipc.c @@ -1465,11 +1465,23 @@ static int private_directory_tree_open(const char *directory_path) { * caller fell back to printing errno — which NOTHING here sets. * A reporter was handed "errno 2" (ENOENT) for a permission * refusal and went looking for a missing file that existed. - * An unset errno is not a diagnosis; name the component. */ - ipc_validation_detail_set("%s: ancestor '%s' is not a usable private-directory " - "parent (must be owned by you, not world-writable, and " - "carry no allow-ACL)", - directory_path, component); + * An unset errno is not a diagnosis. + * + * The first version of that fix then named the WRONG directory. + * posix_directory_parent_secure() validates current_fd — the + * directory we are already in — but the message printed + * `component`, the child about to be entered. So #1537 read + * "ancestor '.cache'" when /Users/ was refusing, and + * #1621 read "cbm-daemon-501" when /private/tmp was. Both + * reporters inspected a directory that was not the one + * refusing, found it clean, and said so — correctly. Naming the + * containing directory is the difference between a report we + * can act on and weeks of talking past each other. */ + ipc_validation_detail_set( + "%s: the directory CONTAINING '%s' is not a usable private-directory parent " + "(it must be owned by you, not world-writable, and carry no allow-ACL). Check " + "that containing directory, not '%s' itself", + directory_path, component, component); } bool created = ok && mkdirat(current_fd, component, 0700) == 0; if (!created && errno != EEXIST) {