From patchwork Tue Jul 30 18:43:49 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Junio C Hamano X-Patchwork-Id: 13747768 Received: from pb-smtp21.pobox.com (pb-smtp21.pobox.com [173.228.157.53]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 815571AA3DE for ; Tue, 30 Jul 2024 18:44:07 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=173.228.157.53 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1722365048; cv=none; b=vB4KU2DZqF2PBX5H3Crnuhr6VWnWvBjNOqXfp5oou8qzZ5AiwgDe7hTWYfVkEq74hgbw8ejwFCjYon3tZ902eE4COC5P7mEE7DzYJe9wl6ZG2HIuHKNqt7LcVZqhLxZ8/deJ4plMqLcsESAe8uZtWsqr1ovCsVlAP037SbnXjdk= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1722365048; c=relaxed/simple; bh=peNggeRphG34L/UmwcpDr6En10ce6XQJ2ph26t0OO+0=; h=From:To:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=ieoOCE6e3fADsJBkzqhk0Mam/baRN2HNA3GNsTLcrUvhjQ2TigCz0F7nDC7+pPesRepu3C6TCEXKvyLVLEeTun7VNF0pNv3CBKd28GBtiB4obyWyU1Ks+LmV87gin8xyA7i/HHYa92dxco/6yWHp7CTCkN0uBCUhigehQwHoV0s= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=pobox.com; spf=pass smtp.mailfrom=pobox.com; dkim=pass (1024-bit key) header.d=pobox.com header.i=@pobox.com header.b=Zad2DxQj; arc=none smtp.client-ip=173.228.157.53 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=pobox.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=pobox.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=pobox.com header.i=@pobox.com header.b="Zad2DxQj" Received: from pb-smtp21.pobox.com (unknown [127.0.0.1]) by pb-smtp21.pobox.com (Postfix) with ESMTP id E5ED11ACC2; Tue, 30 Jul 2024 14:44:01 -0400 (EDT) (envelope-from gitster@pobox.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed; d=pobox.com; h=from:to :subject:date:message-id:in-reply-to:references:mime-version :content-transfer-encoding; s=sasl; bh=peNggeRphG34L/UmwcpDr6En1 0ce6XQJ2ph26t0OO+0=; b=Zad2DxQj8n/qP8xcOQR8/fJsZNpddGLHtGBhvCrwv 2PUYH0sv80iJTVEmstcxZtA9U7ts0PSHUideKntw4nBRWE9I/sGh9Gh3KKRpudwH 4DwhgYnxHW1NzaPwkNpnf4B4FWEAjfXvC0lUucf3MD0laPAuUPyPob4Brn7K1dED Xs= Received: from pb-smtp21.sea.icgroup.com (unknown [127.0.0.1]) by pb-smtp21.pobox.com (Postfix) with ESMTP id DFCBA1ACC1; Tue, 30 Jul 2024 14:44:01 -0400 (EDT) (envelope-from gitster@pobox.com) Received: from pobox.com (unknown [34.125.139.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by pb-smtp21.pobox.com (Postfix) with ESMTPSA id 7DF2E1ACBF; Tue, 30 Jul 2024 14:43:58 -0400 (EDT) (envelope-from gitster@pobox.com) From: Junio C Hamano To: git@vger.kernel.org Subject: [PATCH v4 1/4] safe.directory: preliminary clean-up Date: Tue, 30 Jul 2024 11:43:49 -0700 Message-ID: <20240730184352.2503276-2-gitster@pobox.com> X-Mailer: git-send-email 2.46.0-77-g633c50689c In-Reply-To: <20240730184352.2503276-1-gitster@pobox.com> References: <20240730011004.4030246-1-gitster@pobox.com> <20240723021900.388020-1-gitster@pobox.com> <20240720220915.2933266-1-gitster@pobox.com> <20240730184352.2503276-1-gitster@pobox.com> Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Pobox-Relay-ID: AE74B454-4EA3-11EF-B346-9625FCCAB05B-77302942!pb-smtp21.pobox.com The paths given in the safe.directory configuration variable are allowed to contain "~user" (which interpolates to user's home directory) and "%(prefix)" (which interpolates to the installation location in RUNTIME_PREFIX-enabled builds, and a call to the git_config_pathname() function is tasked to obtain a copy of the path with these constructs interpolated. The function, when it succeeds, always yields an allocated string in the location given as the out-parameter; even when there is nothing to interpolate in the original, a literal copy is made. The code path that contains this caller somehow made two contradicting and incorrect assumptions of the behaviour when there is no need for interpolation, and was written with extra defensiveness against two phantom risks that do not exist. One wrong assumption was that the function might yield NULL when there is no interpolation. This led to the use of an extra "check" variable, conditionally holding either the interpolated or the original string. The assumption was with us since 8959555c (setup_git_directory(): add an owner check for the top-level directory, 2022-03-02) originally introduced the safe.directory feature. Another wrong assumption was that the function might yield the same pointer as the input when there is no interpolation. This led to a conditional free'ing of the interpolated copy, that the conditional never skipped, as we always received an allocated string. Simplify the code by removing the extra defensiveness. Signed-off-by: Junio C Hamano --- setup.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/setup.c b/setup.c index d458edcc02..3177d010d1 100644 --- a/setup.c +++ b/setup.c @@ -1235,17 +1235,15 @@ static int safe_directory_cb(const char *key, const char *value, char *allowed = NULL; if (!git_config_pathname(&allowed, key, value)) { - const char *check = allowed ? allowed : value; - if (ends_with(check, "/*")) { - size_t len = strlen(check); - if (!fspathncmp(check, data->path, len - 1)) + if (ends_with(allowed, "/*")) { + size_t len = strlen(allowed); + if (!fspathncmp(allowed, data->path, len - 1)) data->is_safe = 1; - } else if (!fspathcmp(data->path, check)) { + } else if (!fspathcmp(data->path, allowed)) { data->is_safe = 1; } - } - if (allowed != value) free(allowed); + } } return 0; From patchwork Tue Jul 30 18:43:50 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Junio C Hamano X-Patchwork-Id: 13747767 Received: from pb-smtp20.pobox.com (pb-smtp20.pobox.com [173.228.157.52]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 649661AA3D0 for ; Tue, 30 Jul 2024 18:44:06 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=173.228.157.52 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1722365047; cv=none; b=pMg756FjOE+L8Hu2tzYBof46lXHrGd73U8P2tNx8ZWM2EyJKwO6sf0ib1evDqwbMUOJaN3KO2Zatl4HpFGX+ZjaFPrAbFAA/8EL5JiTLv6xMJW4HzH5XNFbkahY8VXmjpiuufNWF2LCGr4LF1RkonB/7RXWHY7vNoIJ5CeJyAAA= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1722365047; c=relaxed/simple; bh=1p4bJDibchEEzBB0BwvdUQSyRdi/ytWgmzGHtmyfLys=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=Sy9E5IoBaYXIlrh9H7axw+oI/tVlCB69XQ14olQQS5K8kDV960Bt1haS9PVXcpyiESDrbI6O+/RaXxrh9iflR+pqWfaZM8hSQMO/7PDrKOkU6uphDIPbvnuWddvUD9YauluvLgf/F41H+cIcqhDtl7DvPHU7Jz5RJJE7nyMwdWA= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=pobox.com; spf=pass smtp.mailfrom=pobox.com; dkim=pass (1024-bit key) header.d=pobox.com header.i=@pobox.com header.b=ccEAeYKm; arc=none smtp.client-ip=173.228.157.52 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=pobox.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=pobox.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=pobox.com header.i=@pobox.com header.b="ccEAeYKm" Received: from pb-smtp20.pobox.com (unknown [127.0.0.1]) by pb-smtp20.pobox.com (Postfix) with ESMTP id B705D39B9D; Tue, 30 Jul 2024 14:44:05 -0400 (EDT) (envelope-from gitster@pobox.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed; d=pobox.com; h=from:to:cc :subject:date:message-id:in-reply-to:references:mime-version :content-transfer-encoding; s=sasl; bh=1p4bJDibchEEzBB0BwvdUQSyR di/ytWgmzGHtmyfLys=; b=ccEAeYKmJ4bZThP1b+NbX1JeEXV2GJOj+HpaJ34qw 34dOePcnHaL5Yod+R2Ad4KndlGEkzc2IFeoJ7iL1nG4f9SxsWssrcHE34d0Wdcw5 WQ8x5w4S8GkWGuKS/0PNLwlt/li4U6JPaQxFEp2ONi8sH0GZEangKokqjTdxbIzo 7E= Received: from pb-smtp20.sea.icgroup.com (unknown [127.0.0.1]) by pb-smtp20.pobox.com (Postfix) with ESMTP id AF6B839B9C; Tue, 30 Jul 2024 14:44:05 -0400 (EDT) (envelope-from gitster@pobox.com) Received: from pobox.com (unknown [34.125.139.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by pb-smtp20.pobox.com (Postfix) with ESMTPSA id 46F7739B9B; Tue, 30 Jul 2024 14:44:02 -0400 (EDT) (envelope-from gitster@pobox.com) From: Junio C Hamano To: git@vger.kernel.org Cc: Phillip Wood Subject: [PATCH v4 2/4] safe.directory: normalize the checked path Date: Tue, 30 Jul 2024 11:43:50 -0700 Message-ID: <20240730184352.2503276-3-gitster@pobox.com> X-Mailer: git-send-email 2.46.0-77-g633c50689c In-Reply-To: <20240730184352.2503276-1-gitster@pobox.com> References: <20240730011004.4030246-1-gitster@pobox.com> <20240723021900.388020-1-gitster@pobox.com> <20240720220915.2933266-1-gitster@pobox.com> <20240730184352.2503276-1-gitster@pobox.com> Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Pobox-Relay-ID: B0B4B638-4EA3-11EF-B596-92D9AF168FA5-77302942!pb-smtp20.pobox.com The pathname of a repository comes from getcwd() and it could be a path aliased via symbolic links, e.g., the real directory may be /home/u/repository but a symbolic link /home/u/repo may point at it, and the clone request may come as "git clone file:///home/u/repo/". A request to check if /home/u/repo is safe would be rejected if the safe.directory configuration allows /home/u/repository/ but not its alias /home/u/repo/. Normalize the path being checked before comparing with safe.directory value(s). Suggested-by: Phillip Wood Signed-off-by: Junio C Hamano --- setup.c | 16 ++++++++--- t/t0033-safe-directory.sh | 57 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 69 insertions(+), 4 deletions(-) diff --git a/setup.c b/setup.c index 3177d010d1..54cce7219b 100644 --- a/setup.c +++ b/setup.c @@ -1215,7 +1215,7 @@ static int canonicalize_ceiling_entry(struct string_list_item *item, } struct safe_directory_data { - const char *path; + char *path; int is_safe; }; @@ -1261,9 +1261,7 @@ static int ensure_valid_ownership(const char *gitfile, const char *worktree, const char *gitdir, struct strbuf *report) { - struct safe_directory_data data = { - .path = worktree ? worktree : gitdir - }; + struct safe_directory_data data = { 0 }; if (!git_env_bool("GIT_TEST_ASSUME_DIFFERENT_OWNER", 0) && (!gitfile || is_path_owned_by_current_user(gitfile, report)) && @@ -1271,6 +1269,15 @@ static int ensure_valid_ownership(const char *gitfile, (!gitdir || is_path_owned_by_current_user(gitdir, report))) return 1; + /* + * normalize the data.path for comparison with normalized paths + * that come from the configuration file. The path is unsafe + * if it cannot be normalized. + */ + data.path = real_pathdup(worktree ? worktree : gitdir, 0); + if (!data.path) + return 0; + /* * data.path is the "path" that identifies the repository and it is * constant regardless of what failed above. data.is_safe should be @@ -1278,6 +1285,7 @@ static int ensure_valid_ownership(const char *gitfile, */ git_protected_config(safe_directory_cb, &data); + free(data.path); return data.is_safe; } diff --git a/t/t0033-safe-directory.sh b/t/t0033-safe-directory.sh index 5fe61f1291..07ac0f9a01 100755 --- a/t/t0033-safe-directory.sh +++ b/t/t0033-safe-directory.sh @@ -119,4 +119,61 @@ test_expect_success 'local clone of unowned repo accepted in safe directory' ' test_path_is_dir target ' +test_expect_success SYMLINKS 'checked paths are normalized' ' + test_when_finished "rm -rf repository; rm -f repo" && + ( + sane_unset GIT_TEST_ASSUME_DIFFERENT_OWNER && + git config --global --unset-all safe.directory + ) && + git init repository && + ln -s repository repo && + ( + cd repository && + sane_unset GIT_TEST_ASSUME_DIFFERENT_OWNER && + test_commit sample + ) && + + ( + sane_unset GIT_TEST_ASSUME_DIFFERENT_OWNER && + git config --global safe.directory "$(pwd)/repository" + ) && + git -C repository for-each-ref && + git -C repository/ for-each-ref && + git -C repo for-each-ref && + git -C repo/ for-each-ref && + test_must_fail git -C repository/.git for-each-ref && + test_must_fail git -C repository/.git/ for-each-ref && + test_must_fail git -C repo/.git for-each-ref && + test_must_fail git -C repo/.git/ for-each-ref +' + +test_expect_success SYMLINKS 'checked leading paths are normalized' ' + test_when_finished "rm -rf repository; rm -f repo" && + ( + sane_unset GIT_TEST_ASSUME_DIFFERENT_OWNER && + git config --global --unset-all safe.directory + ) && + mkdir -p repository && + git init repository/s && + ln -s repository repo && + ( + cd repository/s && + sane_unset GIT_TEST_ASSUME_DIFFERENT_OWNER && + test_commit sample + ) && + + ( + sane_unset GIT_TEST_ASSUME_DIFFERENT_OWNER && + git config --global safe.directory "$(pwd)/repository/*" + ) && + git -C repository/s for-each-ref && + git -C repository/s/ for-each-ref && + git -C repo/s for-each-ref && + git -C repo/s/ for-each-ref && + git -C repository/s/.git for-each-ref && + git -C repository/s/.git/ for-each-ref && + git -C repo/s/.git for-each-ref && + git -C repo/s/.git/ for-each-ref +' + test_done From patchwork Tue Jul 30 18:43:51 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Junio C Hamano X-Patchwork-Id: 13747770 Received: from pb-smtp2.pobox.com (pb-smtp2.pobox.com [64.147.108.71]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id CA4D01AA3D1 for ; Tue, 30 Jul 2024 18:44:12 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=64.147.108.71 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1722365054; cv=none; b=Z7t5DBUopdKHODbmJq9BMcXNzwNtaH9a+/UHTdNOCBDzG/t8P49x+0hEBUiKN/KZqFGct93ddkCAIfgETYM1FKSGutHwNmsO/4gjTPrPQMoQUBrAM6ph8yJDezRamxo7ZkL5WdH0QiR0oOsEZM/5wpGso87UIkZzTkjAJ/S+WRQ= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1722365054; c=relaxed/simple; bh=KOrMfVRQAvXvWAbImEE1U1ZCuCAWv5JWQI+y9MJ1WaQ=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=rLTBaXa8eZn9cXubNnNhuKFIXBhRwmbMHxpZABIOSDWd5T9ecnGeyUt4Ne0+NbDT4p1QwOOTWaQUE+toURMHhTcMe+l1afZkLVDPTccZALifr7o7IkqzBfR75gnBsmPK143XsHrhGptD+DhKCb63RX+TygwnOmXCM7OWOIjYG8E= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=pobox.com; spf=pass smtp.mailfrom=pobox.com; dkim=pass (1024-bit key) header.d=pobox.com header.i=@pobox.com header.b=fe0VRJOp; arc=none smtp.client-ip=64.147.108.71 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=pobox.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=pobox.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=pobox.com header.i=@pobox.com header.b="fe0VRJOp" Received: from pb-smtp2.pobox.com (unknown [127.0.0.1]) by pb-smtp2.pobox.com (Postfix) with ESMTP id 767BE2FFBB; Tue, 30 Jul 2024 14:44:06 -0400 (EDT) (envelope-from gitster@pobox.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed; d=pobox.com; h=from:to:cc :subject:date:message-id:in-reply-to:references:mime-version :content-transfer-encoding; s=sasl; bh=KOrMfVRQAvXvWAbImEE1U1ZCu CAWv5JWQI+y9MJ1WaQ=; b=fe0VRJOp0fEDJxdS6P0qpwXGmcORimGMzhrEzLuKx V0ZvDOc8In3f3fuw/XbCbRKFaNdb4gZs8refkhTpO38zu/4SiTRv21Fq/s+ubouF PAwggmWZqOIJT1jqce1HqZrSwqr7lkZrbw3XL4vbaFs1z5dqP5Zk9LImWeAHLjk/ SE= Received: from pb-smtp2.nyi.icgroup.com (unknown [127.0.0.1]) by pb-smtp2.pobox.com (Postfix) with ESMTP id 6C1952FFBA; Tue, 30 Jul 2024 14:44:06 -0400 (EDT) (envelope-from gitster@pobox.com) Received: from pobox.com (unknown [34.125.139.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by pb-smtp2.pobox.com (Postfix) with ESMTPSA id B1AC52FFB9; Tue, 30 Jul 2024 14:44:05 -0400 (EDT) (envelope-from gitster@pobox.com) From: Junio C Hamano To: git@vger.kernel.org Cc: Phillip Wood , Jeff King Subject: [PATCH v4 3/4] safe.directory: normalize the configured path Date: Tue, 30 Jul 2024 11:43:51 -0700 Message-ID: <20240730184352.2503276-4-gitster@pobox.com> X-Mailer: git-send-email 2.46.0-77-g633c50689c In-Reply-To: <20240730184352.2503276-1-gitster@pobox.com> References: <20240730011004.4030246-1-gitster@pobox.com> <20240723021900.388020-1-gitster@pobox.com> <20240720220915.2933266-1-gitster@pobox.com> <20240730184352.2503276-1-gitster@pobox.com> Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Pobox-Relay-ID: B2C1BF20-4EA3-11EF-A24E-BAC1940A682E-77302942!pb-smtp2.pobox.com The pathname of a repository comes from getcwd() and it could be a path aliased via symbolic links, e.g., the real directory may be /home/u/repository but a symbolic link /home/u/repo may point at it, and the clone request may come as "git clone file:///home/u/repo/" A request to check if /home/u/repository is safe would be rejected if the safe.directory configuration allows /home/u/repo/ but not its alias /home/u/repository/. Normalize the paths configured for the safe.directory configuration variable before comparing them with the path being checked. Two and a half things to note, compared to the previous step to normalize the actual path of the suspected repository, are: - A configured safe.directory may be coming from .gitignore in the home directory that may be shared across machines. The path meant to match with an entry may not necessarily exist on all of such machines, so not being able to convert them to real path on this machine is *not* a condition that is worthy of warning. Hence, we ignore a path that cannot be converted to a real path. - A configured safe.directory is essentially a random string that user throws at us, written completely unrelated to the directory the current process happens to be in. Hence it makes little sense to give a non-absolute path. Hence we ignore any non-absolute paths, except for ".". - The safe.directory set to "." was once advertised on the list as a valid workaround for the regression caused by the overly tight safe.directory check introduced in 2.45.1; we treat it to mean "if we are at the top level of a repository, it is OK". (cf. <834862fd-b579-438a-b9b3-5246bf27ce8a@gmail.com>). Suggested-by: Phillip Wood Helped-by: Jeff King Signed-off-by: Junio C Hamano --- setup.c | 38 +++++++++++++++++++++++--- t/t0033-safe-directory.sh | 57 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 91 insertions(+), 4 deletions(-) diff --git a/setup.c b/setup.c index 54cce7219b..5f81d9fac0 100644 --- a/setup.c +++ b/setup.c @@ -1235,13 +1235,43 @@ static int safe_directory_cb(const char *key, const char *value, char *allowed = NULL; if (!git_config_pathname(&allowed, key, value)) { - if (ends_with(allowed, "/*")) { - size_t len = strlen(allowed); - if (!fspathncmp(allowed, data->path, len - 1)) + char *normalized = NULL; + + /* + * Setting safe.directory to a non-absolute path + * makes little sense---it won't be relative to + * the configuration file the item is defined in. + * Except for ".", which means "if we are at the top + * level of a repository, then it is OK", which is + * slightly tighter than "*" that allows discovery. + */ + if (!is_absolute_path(allowed) && strcmp(allowed, ".")) { + warning(_("safe.directory '%s' not absolute"), + allowed); + goto next; + } + + /* + * A .gitconfig in $HOME may be shared across + * different machines and safe.directory entries + * may or may not exist as paths on all of these + * machines. In other words, it is not a warning + * worthy event when there is no such path on this + * machine---the entry may be useful elsewhere. + */ + normalized = real_pathdup(allowed, 0); + if (!normalized) + goto next; + + if (ends_with(normalized, "/*")) { + size_t len = strlen(normalized); + if (!fspathncmp(normalized, data->path, len - 1)) data->is_safe = 1; - } else if (!fspathcmp(data->path, allowed)) { + } else if (!fspathcmp(data->path, normalized)) { data->is_safe = 1; } + next: + free(normalized); free(allowed); } } diff --git a/t/t0033-safe-directory.sh b/t/t0033-safe-directory.sh index 07ac0f9a01..ea74657255 100755 --- a/t/t0033-safe-directory.sh +++ b/t/t0033-safe-directory.sh @@ -176,4 +176,61 @@ test_expect_success SYMLINKS 'checked leading paths are normalized' ' git -C repo/s/.git/ for-each-ref ' +test_expect_success SYMLINKS 'configured paths are normalized' ' + test_when_finished "rm -rf repository; rm -f repo" && + ( + sane_unset GIT_TEST_ASSUME_DIFFERENT_OWNER && + git config --global --unset-all safe.directory + ) && + git init repository && + ln -s repository repo && + ( + cd repository && + sane_unset GIT_TEST_ASSUME_DIFFERENT_OWNER && + test_commit sample + ) && + + ( + sane_unset GIT_TEST_ASSUME_DIFFERENT_OWNER && + git config --global safe.directory "$(pwd)/repo" + ) && + git -C repository for-each-ref && + git -C repository/ for-each-ref && + git -C repo for-each-ref && + git -C repo/ for-each-ref && + test_must_fail git -C repository/.git for-each-ref && + test_must_fail git -C repository/.git/ for-each-ref && + test_must_fail git -C repo/.git for-each-ref && + test_must_fail git -C repo/.git/ for-each-ref +' + +test_expect_success SYMLINKS 'configured leading paths are normalized' ' + test_when_finished "rm -rf repository; rm -f repo" && + ( + sane_unset GIT_TEST_ASSUME_DIFFERENT_OWNER && + git config --global --unset-all safe.directory + ) && + mkdir -p repository && + git init repository/s && + ln -s repository repo && + ( + cd repository/s && + sane_unset GIT_TEST_ASSUME_DIFFERENT_OWNER && + test_commit sample + ) && + + ( + sane_unset GIT_TEST_ASSUME_DIFFERENT_OWNER && + git config --global safe.directory "$(pwd)/repo/*" + ) && + git -C repository/s for-each-ref && + git -C repository/s/ for-each-ref && + git -C repository/s/.git for-each-ref && + git -C repository/s/.git/ for-each-ref && + git -C repo/s for-each-ref && + git -C repo/s/ for-each-ref && + git -C repo/s/.git for-each-ref && + git -C repo/s/.git/ for-each-ref +' + test_done From patchwork Tue Jul 30 18:43:52 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Junio C Hamano X-Patchwork-Id: 13747769 Received: from pb-smtp1.pobox.com (pb-smtp1.pobox.com [64.147.108.70]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 6946A1AA3F6 for ; Tue, 30 Jul 2024 18:44:09 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=64.147.108.70 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1722365050; cv=none; b=LByTi5JYOeX2pJuAS2tZ3Xw9sf5CxysdIH4bjlQyJDesiNzqOMwZNU8xm3WpLeOTFCOWtnR0E1xTZ9VwlYGdb3u3Wm4AYgwlUazBwPTAagMXaI2q9skd8T7n1lAh+Z1JFHMqQj0dch4ddyAA5YeZtLKlfIMVrgdkqVGTFM1JjrY= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1722365050; c=relaxed/simple; bh=F3oKBO04l6XBcfTHkpxxV+9vd8FrFUFXNIwcJiswUI8=; h=From:To:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=cUyT20wxa7BGcp91dXYmW7pG69BHrU3xlAYwCsI9q5BSYC/YaEjL+I5JpX7V4RLrhTUPOckUq3xGDvmbFaRpmVp4Jlhu8wscFBhDe9rA4KKLUZUhN1jwXWpOtlqBtnVjhDnCStCvfqNqx8gp9G+pQlvg+/qSIqPbtqMGB72Gv6I= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=pobox.com; spf=pass smtp.mailfrom=pobox.com; dkim=pass (1024-bit key) header.d=pobox.com header.i=@pobox.com header.b=LgLWHgHa; arc=none smtp.client-ip=64.147.108.70 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=pobox.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=pobox.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=pobox.com header.i=@pobox.com header.b="LgLWHgHa" Received: from pb-smtp1.pobox.com (unknown [127.0.0.1]) by pb-smtp1.pobox.com (Postfix) with ESMTP id 2A08734F2A; Tue, 30 Jul 2024 14:44:08 -0400 (EDT) (envelope-from gitster@pobox.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed; d=pobox.com; h=from:to :subject:date:message-id:in-reply-to:references:mime-version :content-transfer-encoding; s=sasl; bh=F3oKBO04l6XBcfTHkpxxV+9vd 8FrFUFXNIwcJiswUI8=; b=LgLWHgHaee8EfXejIgtz8JQPJoKzt8N9b98jeSmnI Ypd2cZqc3NJ4em0LLvy4/NMA5s+VWt2xmbrl6Vzj4CJj8HjjicA3kyBwXZbMlpLi 2UKI3C21uI7dkY5B707ZXRy5zrIYlm8P4GAeO7cwWvAqUtzNOT8cX0WxM0gBHGkd 2k= Received: from pb-smtp1.nyi.icgroup.com (unknown [127.0.0.1]) by pb-smtp1.pobox.com (Postfix) with ESMTP id 1D09234F29; Tue, 30 Jul 2024 14:44:08 -0400 (EDT) (envelope-from gitster@pobox.com) Received: from pobox.com (unknown [34.125.139.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by pb-smtp1.pobox.com (Postfix) with ESMTPSA id 6D03E34F26; Tue, 30 Jul 2024 14:44:07 -0400 (EDT) (envelope-from gitster@pobox.com) From: Junio C Hamano To: git@vger.kernel.org Subject: [PATCH v4 4/4] safe.directory: setting safe.directory="." allows the "current" directory Date: Tue, 30 Jul 2024 11:43:52 -0700 Message-ID: <20240730184352.2503276-5-gitster@pobox.com> X-Mailer: git-send-email 2.46.0-77-g633c50689c In-Reply-To: <20240730184352.2503276-1-gitster@pobox.com> References: <20240730011004.4030246-1-gitster@pobox.com> <20240723021900.388020-1-gitster@pobox.com> <20240720220915.2933266-1-gitster@pobox.com> <20240730184352.2503276-1-gitster@pobox.com> Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Pobox-Relay-ID: B3C7B9D8-4EA3-11EF-A703-34EEED2EC81B-77302942!pb-smtp1.pobox.com When "git daemon" enters a repository, it chdir's to the requested repository and then uses "." (the curent directory) to consult the "is this repository considered safe?" when it is not owned by the same owner as the process. Make sure this access will be allowed by setting safe.directory to ".", as that was once advertised on the list as a valid workaround to the overly tight safe.directory settings introduced by 2.45.1 (cf. <834862fd-b579-438a-b9b3-5246bf27ce8a@gmail.com>). Also add simlar test to show what happens in the same setting if the safe.directory is set to "*" instead of "."; in short, "." is a bit tighter (as it is custom designed for git-daemon situation) than "anything goes" settings given by "*". Signed-off-by: Junio C Hamano --- t/t0033-safe-directory.sh | 64 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 64 insertions(+) diff --git a/t/t0033-safe-directory.sh b/t/t0033-safe-directory.sh index ea74657255..e97a84764f 100755 --- a/t/t0033-safe-directory.sh +++ b/t/t0033-safe-directory.sh @@ -233,4 +233,68 @@ test_expect_success SYMLINKS 'configured leading paths are normalized' ' git -C repo/s/.git/ for-each-ref ' +test_expect_success 'safe.directory set to a dot' ' + test_when_finished "rm -rf repository" && + ( + sane_unset GIT_TEST_ASSUME_DIFFERENT_OWNER && + git config --global --unset-all safe.directory + ) && + mkdir -p repository/subdir && + git init repository && + ( + cd repository && + sane_unset GIT_TEST_ASSUME_DIFFERENT_OWNER && + test_commit sample + ) && + + ( + sane_unset GIT_TEST_ASSUME_DIFFERENT_OWNER && + git config --global safe.directory "." + ) && + git -C repository for-each-ref && + git -C repository/ for-each-ref && + git -C repository/.git for-each-ref && + git -C repository/.git/ for-each-ref && + + # What is allowed is repository/subdir but the repository + # path is repository. + test_must_fail git -C repository/subdir for-each-ref && + + # Likewise, repository .git/refs is allowed with "." but + # repository/.git that is accessed is not allowed. + test_must_fail git -C repository/.git/refs for-each-ref +' + +test_expect_success 'safe.directory set to asterisk' ' + test_when_finished "rm -rf repository" && + ( + sane_unset GIT_TEST_ASSUME_DIFFERENT_OWNER && + git config --global --unset-all safe.directory + ) && + mkdir -p repository/subdir && + git init repository && + ( + cd repository && + sane_unset GIT_TEST_ASSUME_DIFFERENT_OWNER && + test_commit sample + ) && + + ( + sane_unset GIT_TEST_ASSUME_DIFFERENT_OWNER && + git config --global safe.directory "*" + ) && + # these are trivial + git -C repository for-each-ref && + git -C repository/ for-each-ref && + git -C repository/.git for-each-ref && + git -C repository/.git/ for-each-ref && + + # With "*", everything is allowed, and the repository is + # discovered, which is different behaviour from "." above. + git -C repository/subdir for-each-ref && + + # Likewise. + git -C repository/.git/refs for-each-ref +' + test_done