mirror of
https://github.com/DeusData/codebase-memory-mcp.git
synced 2026-11-04 10:48:20 +00:00
fix(cli): fold per-user /home entry into the alias loop, add test seam
Address review on #2309: - Fold the /home/<name> handling into the existing #2175 alias loop via a shared activation_alias_map() that builds the mapped path in a stack buffer and returns activation_string_copy(); raw allocator sites in activation_transaction.c drop from 80 to 78 (baseline lowered). - Comment the deliberate difference: the per-user entry may resolve to a directory owned by the current account, the whole-alias case may not. - Add cbm_activation_transaction_set_home_root_for_testing() (test seams only) and a Linux test that fails without the per-user branch. Co-authored-by: Claude <noreply@anthropic.com> Signed-off-by: Glen Ko <gleko@amazon.com>
This commit is contained in:
1 parent
42971f2c02
commit
2fbf3fb977
4 files changed
+159
-60
No files matched your search
@@ -14,7 +14,7 @@ internal/cbm/lsp/py_lsp.c 2
|
||||
internal/cbm/macro_table.c 1
|
||||
internal/cbm/service_patterns.c 2
|
||||
internal/cbm/sqlite_writer.c 74
|
||||
src/cli/activation_transaction.c 80
|
||||
src/cli/activation_transaction.c 78
|
||||
src/cli/agent_clients.c 73
|
||||
src/cli/agent_profiles.c 7
|
||||
src/cli/cli.c 170
|
||||
|
||||
@@ -890,49 +890,44 @@ static bool activation_posix_acl_empty(int descriptor) {
|
||||
}
|
||||
|
||||
#ifdef __linux__
|
||||
/* Managed Linux hosts often keep /home a real directory and point the per-user
|
||||
* entry elsewhere (/home/alice -> /local/home/alice). Trust that entry only
|
||||
* when root owns it inside a /home nobody else can write, and it resolves to a
|
||||
* directory owned by root or the current user. Returns NULL when not
|
||||
* applicable; the O_NOFOLLOW walk then rejects the path as before. */
|
||||
static char *activation_linux_home_entry_path(const char *directory) {
|
||||
static const char home_prefix[] = "/home/";
|
||||
size_t prefix_length = sizeof(home_prefix) - 1U;
|
||||
if (strncmp(directory, home_prefix, prefix_length) != 0) {
|
||||
return NULL;
|
||||
}
|
||||
size_t name_length = strcspn(directory + prefix_length, "/");
|
||||
char entry[sizeof(home_prefix) + 256U];
|
||||
if (name_length == 0 || name_length >= 256U) {
|
||||
return NULL;
|
||||
}
|
||||
memcpy(entry, directory, prefix_length + name_length);
|
||||
entry[prefix_length + name_length] = '\0';
|
||||
/* Root of the per-user home tree and the account trusted to own it. Root and
|
||||
* "/home" in production; the test seam substitutes a fixture tree and the test
|
||||
* account, because the real layout needs root-owned entries to reproduce. */
|
||||
static const char *g_activation_home_root = "/home";
|
||||
static uid_t g_activation_home_owner = 0;
|
||||
|
||||
struct stat home_status;
|
||||
struct stat entry_status;
|
||||
struct stat resolved_status;
|
||||
#ifdef CBM_ENABLE_TEST_SEAMS
|
||||
void cbm_activation_transaction_set_home_root_for_testing(const char *home_root,
|
||||
unsigned long owner_uid) {
|
||||
g_activation_home_root = home_root ? home_root : "/home";
|
||||
g_activation_home_owner = home_root ? (uid_t)owner_uid : 0;
|
||||
}
|
||||
#endif
|
||||
#endif
|
||||
|
||||
#if defined(__APPLE__) || defined(__linux__)
|
||||
/* Map a trusted symlink `link` (already lstat'd) onto its resolved target and
|
||||
* append `rest`. The link and the target must both belong to `owner`; with
|
||||
* accept_own_target the target may instead belong to the current account.
|
||||
* Returns NULL when either check fails or the mapped path does not fit. */
|
||||
static char *activation_alias_map(const char *link, const struct stat *link_status, uid_t owner,
|
||||
bool accept_own_target, const char *rest) {
|
||||
char resolved[4096];
|
||||
if (lstat("/home", &home_status) != 0 || !S_ISDIR(home_status.st_mode) ||
|
||||
home_status.st_uid != 0 || (home_status.st_mode & 0022) != 0 ||
|
||||
lstat(entry, &entry_status) != 0 || !S_ISLNK(entry_status.st_mode) ||
|
||||
entry_status.st_uid != 0 || !realpath(entry, resolved) ||
|
||||
lstat(resolved, &resolved_status) != 0 || !S_ISDIR(resolved_status.st_mode) ||
|
||||
(resolved_status.st_uid != 0 && resolved_status.st_uid != geteuid())) {
|
||||
struct stat resolved_status;
|
||||
if (link_status->st_uid != owner || !realpath(link, resolved) ||
|
||||
lstat(resolved, &resolved_status) != 0 || !S_ISDIR(resolved_status.st_mode)) {
|
||||
return NULL;
|
||||
}
|
||||
const char *rest = directory + prefix_length + name_length;
|
||||
size_t needed = strlen(resolved) + strlen(rest) + 1U;
|
||||
char *mapped = malloc(needed);
|
||||
if (!mapped) {
|
||||
if (resolved_status.st_uid != owner &&
|
||||
!(accept_own_target && resolved_status.st_uid == geteuid())) {
|
||||
return NULL;
|
||||
}
|
||||
int written = snprintf(mapped, needed, "%s%s", resolved, rest);
|
||||
if (written <= 0 || (size_t)written >= needed) {
|
||||
free(mapped);
|
||||
char mapped[4096];
|
||||
int written = snprintf(mapped, sizeof(mapped), "%s%s", resolved, rest);
|
||||
if (written <= 0 || (size_t)written >= sizeof(mapped)) {
|
||||
return NULL;
|
||||
}
|
||||
return mapped;
|
||||
return activation_string_copy(mapped);
|
||||
}
|
||||
#endif
|
||||
|
||||
@@ -943,7 +938,7 @@ static char *activation_posix_walk_path(const char *directory) {
|
||||
* Resolve only these trusted system aliases; arbitrary user symlinks must
|
||||
* still fail the O_NOFOLLOW walk below. */
|
||||
#ifdef __linux__
|
||||
static const char *const aliases[] = {"/tmp", "/var", "/home"};
|
||||
const char *const aliases[] = {"/tmp", "/var", g_activation_home_root};
|
||||
#else
|
||||
static const char *const aliases[] = {"/tmp", "/var"};
|
||||
#endif
|
||||
@@ -954,39 +949,48 @@ static char *activation_posix_walk_path(const char *directory) {
|
||||
(directory[alias_length] != '\0' && directory[alias_length] != '/')) {
|
||||
continue;
|
||||
}
|
||||
uid_t owner = 0;
|
||||
#ifdef __linux__
|
||||
bool is_home = alias == g_activation_home_root;
|
||||
if (is_home) {
|
||||
owner = g_activation_home_owner;
|
||||
}
|
||||
#endif
|
||||
struct stat alias_status;
|
||||
char resolved[4096];
|
||||
if (lstat(alias, &alias_status) != 0) {
|
||||
continue;
|
||||
}
|
||||
if (!S_ISLNK(alias_status.st_mode)) {
|
||||
if (S_ISLNK(alias_status.st_mode)) {
|
||||
return activation_alias_map(alias, &alias_status, owner, false,
|
||||
directory + alias_length);
|
||||
}
|
||||
#ifdef __linux__
|
||||
/* Managed Linux hosts often keep /home a real directory and point the
|
||||
* per-user entry elsewhere (/home/alice -> /local/home/alice, #2306).
|
||||
* Trust that entry only inside a root-owned /home nobody else can
|
||||
* write, and only when root owns the entry itself. Unlike the whole-
|
||||
* alias case above, the target may belong to the current account:
|
||||
* that is the normal shape of a relocated home, and an account can
|
||||
* already write its own home, so it gains nothing it did not have. */
|
||||
const char *name = directory + alias_length;
|
||||
size_t name_length = *name == '/' ? strcspn(name + 1, "/") : 0;
|
||||
if (!is_home || !S_ISDIR(alias_status.st_mode) || alias_status.st_uid != owner ||
|
||||
(alias_status.st_mode & 0022) != 0 || name_length == 0) {
|
||||
continue;
|
||||
}
|
||||
if (alias_status.st_uid != 0 || !realpath(alias, resolved)) {
|
||||
char entry[4096];
|
||||
size_t entry_length = alias_length + 1U + name_length;
|
||||
if (entry_length >= sizeof(entry)) {
|
||||
return NULL;
|
||||
}
|
||||
struct stat resolved_status;
|
||||
if (lstat(resolved, &resolved_status) != 0 || !S_ISDIR(resolved_status.st_mode) ||
|
||||
resolved_status.st_uid != 0) {
|
||||
return NULL;
|
||||
memcpy(entry, directory, entry_length);
|
||||
entry[entry_length] = '\0';
|
||||
struct stat entry_status;
|
||||
if (lstat(entry, &entry_status) == 0 && S_ISLNK(entry_status.st_mode)) {
|
||||
return activation_alias_map(entry, &entry_status, owner, true,
|
||||
directory + entry_length);
|
||||
}
|
||||
size_t needed = strlen(resolved) + strlen(directory + alias_length) + 1U;
|
||||
char *mapped = malloc(needed);
|
||||
if (!mapped) {
|
||||
return NULL;
|
||||
}
|
||||
int written = snprintf(mapped, needed, "%s%s", resolved, directory + alias_length);
|
||||
if (written <= 0 || (size_t)written >= needed) {
|
||||
free(mapped);
|
||||
return NULL;
|
||||
}
|
||||
return mapped;
|
||||
}
|
||||
#endif
|
||||
#ifdef __linux__
|
||||
char *home_entry = activation_linux_home_entry_path(directory);
|
||||
if (home_entry) {
|
||||
return home_entry;
|
||||
}
|
||||
#endif
|
||||
return activation_string_copy(directory);
|
||||
|
||||
@@ -100,6 +100,11 @@ const char *cbm_activation_transaction_refusal_note(void);
|
||||
#ifdef CBM_ENABLE_TEST_SEAMS
|
||||
void cbm_activation_transaction_note_refusal_for_testing(const char *predicate,
|
||||
unsigned long os_error);
|
||||
/* Linux only: stand home_root in for /home and owner_uid in for root when
|
||||
* deciding whether /home or a /home/<name> entry is a trusted alias. NULL
|
||||
* restores the production values. Inert elsewhere. */
|
||||
void cbm_activation_transaction_set_home_root_for_testing(const char *home_root,
|
||||
unsigned long owner_uid);
|
||||
#endif
|
||||
|
||||
#endif /* CBM_ACTIVATION_TRANSACTION_H */
|
||||
@@ -764,6 +764,95 @@ TEST(activation_transaction_rejects_symlink_candidate_target_and_parent) {
|
||||
PASS();
|
||||
}
|
||||
|
||||
/* #2306: managed Linux hosts keep /home a real directory and point the
|
||||
* per-user entry elsewhere (/home/alice -> /local/home/alice). Root owns that
|
||||
* entry, so it is trusted like the #2175 /home alias; the seam stands a
|
||||
* fixture tree in for /home and the test account in for root. Every status is
|
||||
* captured before the seam is reset so a failure cannot leak into the
|
||||
* following tests. */
|
||||
TEST(activation_transaction_follows_trusted_per_user_home_entry) {
|
||||
#if defined(__linux__) && defined(CBM_ENABLE_TEST_SEAMS)
|
||||
char root[ACTIVATION_TEST_PATH_CAP];
|
||||
char home[ACTIVATION_TEST_PATH_CAP];
|
||||
char entry[ACTIVATION_TEST_PATH_CAP];
|
||||
char relocated[ACTIVATION_TEST_PATH_CAP];
|
||||
char relocated_bin[ACTIVATION_TEST_PATH_CAP];
|
||||
char relocated_target[ACTIVATION_TEST_PATH_CAP];
|
||||
char target[ACTIVATION_TEST_PATH_CAP];
|
||||
ASSERT_TRUE(activation_test_fixture(root));
|
||||
ASSERT_TRUE(activation_test_path(home, root, "home"));
|
||||
ASSERT_TRUE(activation_test_path(entry, home, "alice"));
|
||||
ASSERT_TRUE(activation_test_path(relocated, root, "local-alice"));
|
||||
ASSERT_TRUE(activation_test_path(relocated_bin, relocated, "bin"));
|
||||
ASSERT_TRUE(activation_test_path(relocated_target, relocated_bin, "cbm"));
|
||||
ASSERT_TRUE(activation_test_path(target, entry, "bin/cbm"));
|
||||
ASSERT_TRUE(cbm_mkdir_p(home, 0755));
|
||||
ASSERT_EQ(chmod(home, 0755), 0);
|
||||
ASSERT_TRUE(cbm_mkdir_p(relocated_bin, 0700));
|
||||
ASSERT_EQ(symlink(relocated, entry), 0);
|
||||
unsigned long self = (unsigned long)geteuid();
|
||||
|
||||
/* Without the seam the entry is an ordinary user symlink: refused. */
|
||||
cbm_activation_transaction_t *transaction = NULL;
|
||||
cbm_activation_transaction_status_t untrusted_status = cbm_activation_transaction_stage_bytes(
|
||||
target, "candidate", strlen("candidate"), &transaction);
|
||||
bool untrusted_created = transaction != NULL;
|
||||
(void)cbm_activation_transaction_close(&transaction);
|
||||
|
||||
/* The home root and the entry must belong to the trusted owner. */
|
||||
cbm_activation_transaction_set_home_root_for_testing(home, self + 1U);
|
||||
cbm_activation_transaction_status_t foreign_owner_status =
|
||||
cbm_activation_transaction_stage_bytes(target, "candidate", strlen("candidate"),
|
||||
&transaction);
|
||||
bool foreign_owner_created = transaction != NULL;
|
||||
(void)cbm_activation_transaction_close(&transaction);
|
||||
|
||||
/* A home root others can write could have its entry swapped: refused. */
|
||||
cbm_activation_transaction_set_home_root_for_testing(home, self);
|
||||
(void)chmod(home, 0775);
|
||||
cbm_activation_transaction_status_t writable_home_status =
|
||||
cbm_activation_transaction_stage_bytes(target, "candidate", strlen("candidate"),
|
||||
&transaction);
|
||||
bool writable_home_created = transaction != NULL;
|
||||
(void)cbm_activation_transaction_close(&transaction);
|
||||
(void)chmod(home, 0755);
|
||||
|
||||
/* Trusted root, trusted entry: staged and committed into the target. */
|
||||
cbm_activation_transaction_status_t trusted_status = cbm_activation_transaction_stage_bytes(
|
||||
target, "candidate", strlen("candidate"), &transaction);
|
||||
cbm_activation_transaction_status_t commit_status =
|
||||
transaction ? cbm_activation_transaction_commit(transaction, NULL, NULL)
|
||||
: CBM_ACTIVATION_TRANSACTION_INVALID_STATE;
|
||||
cbm_activation_transaction_status_t finalize_status =
|
||||
transaction ? cbm_activation_transaction_finalize(transaction)
|
||||
: CBM_ACTIVATION_TRANSACTION_INVALID_STATE;
|
||||
cbm_activation_transaction_status_t close_status =
|
||||
cbm_activation_transaction_close(&transaction);
|
||||
cbm_activation_transaction_set_home_root_for_testing(NULL, 0);
|
||||
|
||||
char contents[ACTIVATION_TEST_CONTENT_CAP] = "";
|
||||
bool published = activation_test_read(relocated_target, contents);
|
||||
int unlink_status = unlink(entry);
|
||||
int cleanup_status = th_rmtree(root);
|
||||
|
||||
ASSERT_EQ(untrusted_status, CBM_ACTIVATION_TRANSACTION_IO);
|
||||
ASSERT_FALSE(untrusted_created);
|
||||
ASSERT_EQ(foreign_owner_status, CBM_ACTIVATION_TRANSACTION_IO);
|
||||
ASSERT_FALSE(foreign_owner_created);
|
||||
ASSERT_EQ(writable_home_status, CBM_ACTIVATION_TRANSACTION_IO);
|
||||
ASSERT_FALSE(writable_home_created);
|
||||
ASSERT_EQ(trusted_status, CBM_ACTIVATION_TRANSACTION_OK);
|
||||
ASSERT_EQ(commit_status, CBM_ACTIVATION_TRANSACTION_OK);
|
||||
ASSERT_EQ(finalize_status, CBM_ACTIVATION_TRANSACTION_OK);
|
||||
ASSERT_EQ(close_status, CBM_ACTIVATION_TRANSACTION_OK);
|
||||
ASSERT_TRUE(published);
|
||||
ASSERT_STR_EQ(contents, "candidate");
|
||||
ASSERT_EQ(unlink_status, 0);
|
||||
ASSERT_EQ(cleanup_status, 0);
|
||||
#endif
|
||||
PASS();
|
||||
}
|
||||
|
||||
TEST(activation_transaction_fails_closed_if_target_directory_is_replaced) {
|
||||
char root[ACTIVATION_TEST_PATH_CAP];
|
||||
char active[ACTIVATION_TEST_PATH_CAP];
|
||||
@@ -1089,6 +1178,7 @@ SUITE(activation_transaction) {
|
||||
RUN_TEST(activation_transaction_admits_group_writable_ancestor);
|
||||
RUN_TEST(activation_transaction_rejects_windows_callback_allow_directory_ace);
|
||||
RUN_TEST(activation_transaction_rejects_symlink_candidate_target_and_parent);
|
||||
RUN_TEST(activation_transaction_follows_trusted_per_user_home_entry);
|
||||
RUN_TEST(activation_transaction_fails_closed_if_target_directory_is_replaced);
|
||||
RUN_TEST(activation_transaction_does_not_replace_target_created_at_publish_boundary);
|
||||
RUN_TEST(activation_transaction_rejects_macos_mutating_extended_acl);
|
||||
|
||||
Reference in new issue
Block a user