From a690109975a8f5ff1e08a2bdba100088dab9e53e Mon Sep 17 00:00:00 2001 From: opencode Date: Wed, 16 Sep 2026 22:41:12 +0200 Subject: [PATCH] feat(parity): resolve --chown TO names on receiver via map rules --- src/shared/identity.c | 89 +++++++++++++++++++++++++++++++---------- tests/test_client_cli.c | 38 ++++++++++++++++-- tests/test_config.c | 12 +++++- 3 files changed, 112 insertions(+), 27 deletions(-) diff --git a/src/shared/identity.c b/src/shared/identity.c index c806789..1f717d5 100644 --- a/src/shared/identity.c +++ b/src/shared/identity.c @@ -589,6 +589,67 @@ static int identity_split_chown(const char* value, char** puser, char** pgroup) return 0; } +/* --chown is rsync's shorthand for "--usermap=*:USER --groupmap=*:GROUP", so a + * name TO value must be resolved on the RECEIVER, not on the sender. Append the + * equivalent map rule (FROM matches every id). The numeric/'*' forms are stored + * numerically exactly as rsync's id_parse/user_to_uid would. Returns 0 on + * success, -1 on a malformed numeric token or allocation failure. */ +static int identity_append_chown_rule(Config* config, bool is_group, const char* token) { + IdentityMap rule; + memset(&rule, 0, sizeof(rule)); + rule.from = IDENTITY_MATCH_ANY; + rule.from_hi = IDENTITY_MATCH_ANY; + if (strcmp(token, "*") == 0) { + rule.to = IDENTITY_CURRENT; + } else if (identity_all_digits(token[0] == '@' ? token + 1 : token)) { + if (identity_resolve_token(token, is_group, &rule.to) != 0) { + log_message(LOG_LEVEL_ERROR, "--chown numeric id is out of range: %s", token); + return -1; + } + } else { + rule.to = 0; + rule.to_name = str_dup(token); + if (!rule.to_name) + return -1; + } + if (identity_append_rule(is_group ? &config->groupmap : &config->usermap, + is_group ? &config->groupmap_count : &config->usermap_count, + &rule) != 0) { + free(rule.to_name); + log_message(LOG_LEVEL_ERROR, "--chown has too many rules (max %d)", MAX_IDENTITY_MAP); + return -1; + } + return 0; +} + +/* Resolve/record one --chown side. The source-side numeric value is kept in + * chown_uid/chown_gid purely as a fallback (the appended map rule resolves the + * name on the receiver and wins); a name that does not exist on the sender is + * accepted and left to receiver-side resolution, matching rsync. */ +static int identity_parse_chown_side(Config* config, bool is_group, const char* token) { + if (identity_append_chown_rule(config, is_group, token) != 0) + return -1; + bool numeric = identity_all_digits(token[0] == '@' ? token + 1 : token); + int32_t resolved; + if (identity_resolve_token(token, is_group, &resolved) == 0) { + if (is_group) { + config->chown_gid = resolved; + config->chown_gid_set = true; + } else { + config->chown_uid = resolved; + config->chown_uid_set = true; + } + return 0; + } + if (numeric) { + log_message(LOG_LEVEL_ERROR, "--chown could not resolve numeric id '%s'", token); + return -1; + } + /* Unknown sender-side name: rsync accepts it and resolves it (or warns) on + * the receiver; do the same instead of failing the whole run. */ + return 0; +} + int identity_parse_chown(Config* config, const char* value) { if (!config || !value || *value == '\0') { log_message(LOG_LEVEL_ERROR, "--chown requires a value (USER:GROUP, USER, or :GROUP)"); @@ -627,32 +688,18 @@ int identity_parse_chown(Config* config, const char* value) { if (*user == '\0') { log_message(LOG_LEVEL_ERROR, "--chown requires a user or group (got '%s')", value); ret = -1; - } else if (identity_resolve_token(user, false, &config->chown_uid) != 0) { - log_message(LOG_LEVEL_ERROR, - "--chown could not resolve user '%s' (use a name that exists " - "on the source, '*', or @N)", - value); + } else if (identity_parse_chown_side(config, false, user) != 0) { ret = -1; - } else { - config->chown_uid_set = true; } } else { /* --chown=USER:GROUP, --chown=:GROUP, --chown=USER: */ - if (*user != '\0') { - if (identity_resolve_token(user, false, &config->chown_uid) != 0) { - log_message(LOG_LEVEL_ERROR, "--chown could not resolve user '%s'", value); - ret = -1; - goto done; - } - config->chown_uid_set = true; + if (*user != '\0' && identity_parse_chown_side(config, false, user) != 0) { + ret = -1; + goto done; } - if (*group != '\0') { - if (identity_resolve_token(group, true, &config->chown_gid) != 0) { - log_message(LOG_LEVEL_ERROR, "--chown could not resolve group '%s'", value); - ret = -1; - goto done; - } - config->chown_gid_set = true; + if (*group != '\0' && identity_parse_chown_side(config, true, group) != 0) { + ret = -1; + goto done; } if (!*user && !*group) { log_message(LOG_LEVEL_ERROR, "--chown must set a user, a group, or both (got '%s')", value); diff --git a/tests/test_client_cli.c b/tests/test_client_cli.c index 66c6f3d..b3a7c43 100644 --- a/tests/test_client_cli.c +++ b/tests/test_client_cli.c @@ -1044,10 +1044,11 @@ static void test_parse_args_basis_dirs() { config_delete(cfg); } -/* Absolute, escaping, or degenerate basis-dir values must be rejected up - front: they would resolve outside the destination root on the receiver. */ +/* Escaping or degenerate basis-dir values must be rejected up front (they would + resolve outside the destination root on the receiver); an absolute path is + accepted (rsync parity) and canonicalized with its leading '/' preserved. */ static void test_parse_args_basis_invalid_paths() { - static const char* const invalid[] = {"/abs", "..", "a/../b", "."}; + static const char* const invalid[] = {"..", "a/../b", ".", "/", ""}; for (size_t i = 0; i < sizeof(invalid) / sizeof(invalid[0]); i++) { Config* cfg = config_create(); char* argv[] = {"fastsync", "--link-dest", (char*)invalid[i], "/src", "/dst"}; @@ -1056,6 +1057,15 @@ static void test_parse_args_basis_invalid_paths() { EXPECT_EQ_INT(parse_args(cfg, 5, argv, positional_args, &positional_count), -1); config_delete(cfg); } + + Config* cfg = config_create(); + char* argv[] = {"fastsync", "--link-dest=/abs/dir", "/src", "/dst"}; + int positional_args[2]; + int positional_count = 0; + EXPECT_EQ_INT(parse_args(cfg, 4, argv, positional_args, &positional_count), 0); + EXPECT_EQ_INT(cfg->basis_count, 1); + EXPECT_EQ_STR(cfg->basis_dirs[0].path, "/abs/dir"); + config_delete(cfg); } /* Basis dirs require the per-file incremental handshake, which -s disables. */ @@ -3146,6 +3156,27 @@ static void test_parse_args_chown() { EXPECT_TRUE(cfg->chown_gid_set); EXPECT_EQ_INT(cfg->chown_gid, IDENTITY_CURRENT); config_delete(cfg); + + /* A --chown NAME is converted to the equivalent receiver-resolved map rule + * (rsync implements --chown as --usermap=*:USER --groupmap=*:GROUP), so the + * name is carried on the wire as to_name instead of being resolved on the + * sender. A name that does not exist on the sender is accepted and left for + * the receiver to resolve (or warn about), matching rsync. */ + cfg = config_create(); + positional_count = 0; + char* argv5[] = {"fastsync", "--chown=no_such_user_zzz:no_such_group_zzz", "/src", "/dst"}; + EXPECT_EQ_INT(parse_args(cfg, 4, argv5, positional_args, &positional_count), 0); + EXPECT_EQ_INT(cfg->usermap_count, 1); + EXPECT_NOT_NULL(cfg->usermap[0].to_name); + if (cfg->usermap[0].to_name) + EXPECT_EQ_STR(cfg->usermap[0].to_name, "no_such_user_zzz"); + EXPECT_FALSE(cfg->chown_uid_set); + EXPECT_EQ_INT(cfg->groupmap_count, 1); + EXPECT_NOT_NULL(cfg->groupmap[0].to_name); + if (cfg->groupmap[0].to_name) + EXPECT_EQ_STR(cfg->groupmap[0].to_name, "no_such_group_zzz"); + EXPECT_FALSE(cfg->chown_gid_set); + config_delete(cfg); } /* --copy-as=USER[:GROUP] (P7 Wave E): resolve the user/group against the local @@ -3220,7 +3251,6 @@ static void test_parse_args_rejects_malformed_identity() { {"--groupmap", "@1"}, {"--groupmap", "no_such_group_qqq:x"}, {"--chown", "a:b:c"}, - {"--chown", "no_such_user_zzz:"}, {"--copy-as", ""}, {"--copy-as", ":"}, {"--copy-as", "a:b:c"}, diff --git a/tests/test_config.c b/tests/test_config.c index e8ed56b..a5fe1c7 100644 --- a/tests/test_config.c +++ b/tests/test_config.c @@ -1192,7 +1192,10 @@ static void test_config_basis_wire_rejects_escaping() { c->basis_dirs = calloc(1, sizeof(BasisDest)); c->basis_dirs[0].type = BASIS_DEST_LINK; c->basis_dirs[0].path = str_dup("/abs"); - EXPECT_FALSE(roundtrip_config_ok(c)); + /* An absolute basis dir is accepted (rsync parity); it is only usable when it + lies within the receiver's authorized root, which file_open_secure_parent + enforces at lookup time. */ + EXPECT_TRUE(roundtrip_config_ok(c)); config_delete(c); /* A well-formed list still round-trips even with a manually built struct. */ @@ -1225,8 +1228,13 @@ static void test_config_basis_normalization() { /* Degenerate values that normalize away to nothing stay rejected. */ EXPECT_EQ_INT(config_basis_append(c, BASIS_DEST_LINK, "."), -1); EXPECT_EQ_INT(config_basis_append(c, BASIS_DEST_LINK, ".."), -1); - EXPECT_EQ_INT(config_basis_append(c, BASIS_DEST_LINK, "/abs"), -1); + /* An absolute path is canonicalized (leading '/' preserved) and accepted. */ + EXPECT_EQ_INT(config_basis_append(c, BASIS_DEST_LINK, "/abs"), 0); + EXPECT_EQ_STR(c->basis_dirs[c->basis_count - 1].path, "/abs"); + EXPECT_EQ_INT(config_basis_append(c, BASIS_DEST_LINK, "/a//b/"), 0); + EXPECT_EQ_STR(c->basis_dirs[c->basis_count - 1].path, "/a/b"); EXPECT_EQ_INT(config_basis_append(c, BASIS_DEST_LINK, "a/../b"), -1); + EXPECT_EQ_INT(config_basis_append(c, BASIS_DEST_LINK, "/"), -1); EXPECT_EQ_INT(config_basis_append(c, BASIS_DEST_LINK, ""), -1); config_delete(c); }