Skip to content
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions src/acl.c
Original file line number Diff line number Diff line change
Expand Up @@ -1726,6 +1726,11 @@ static int ACLSelectorCheckKey(aclSelector *selector, const char *key, int keyle
/* The selector can access any key */
if (selector->flags & SELECTOR_FLAG_ALLKEYS) return ACL_OK;

/* NOT_KEY entries are routing-only tokens (e.g. CLUSTERSCAN cursor,
* pub/sub channels); they are not real user keys and should bypass
* key-pattern ACL checks, consistent with how keyspecs skip them. */
if (keyspec_flags & CMD_KEY_NOT_KEY) return ACL_OK;
Comment thread
enjoy-binbin marked this conversation as resolved.

listIter li;
listNode *ln;
listRewind(selector->patterns, &li);
Expand Down
2 changes: 1 addition & 1 deletion src/commands.def
Original file line number Diff line number Diff line change
Expand Up @@ -11860,7 +11860,7 @@ struct COMMAND_STRUCT serverCommandTable[] = {
/* cluster */
{MAKE_CMD("asking","Signals that a cluster client is following an -ASK redirect.","O(1)","3.0.0",CMD_DOC_NONE,NULL,NULL,"cluster",COMMAND_GROUP_CLUSTER,ASKING_History,0,ASKING_Tips,0,askingCommand,1,CMD_FAST,ACL_CATEGORY_CONNECTION|ACL_CATEGORY_FAST,NULL,ASKING_Keyspecs,0,NULL,0)},
{MAKE_CMD("cluster","A container for Cluster commands.","Depends on subcommand.","3.0.0",CMD_DOC_NONE,NULL,NULL,"cluster",COMMAND_GROUP_CLUSTER,CLUSTER_History,0,CLUSTER_Tips,0,NULL,-2,0,ACL_CATEGORY_SLOW,NULL,CLUSTER_Keyspecs,0,NULL,0),.subcommands=CLUSTER_Subcommands},
{MAKE_CMD("clusterscan","Iterates over the keys in the cluster.","O(N) where N is the number of elements returned.","9.1.0",CMD_DOC_NONE,NULL,NULL,"cluster",COMMAND_GROUP_CLUSTER,CLUSTERSCAN_History,0,CLUSTERSCAN_Tips,3,clusterscanCommand,-2,CMD_READONLY|CMD_TOUCHES_ARBITRARY_KEYS,ACL_CATEGORY_KEYSPACE|ACL_CATEGORY_READ|ACL_CATEGORY_SLOW,NULL,CLUSTERSCAN_Keyspecs,1,NULL,5),.args=CLUSTERSCAN_Args},
{MAKE_CMD("clusterscan","Iterates over the keys in the cluster.","O(N) where N is the number of elements returned.","9.1.0",CMD_DOC_NONE,NULL,NULL,"cluster",COMMAND_GROUP_CLUSTER,CLUSTERSCAN_History,0,CLUSTERSCAN_Tips,3,clusterscanCommand,-2,CMD_READONLY|CMD_TOUCHES_ARBITRARY_KEYS,ACL_CATEGORY_KEYSPACE|ACL_CATEGORY_READ|ACL_CATEGORY_SLOW,NULL,CLUSTERSCAN_Keyspecs,1,clusterscanGetKeys,5),.args=CLUSTERSCAN_Args},
{MAKE_CMD("readonly","Enables read-only queries for a connection to a Valkey replica node.","O(1)","3.0.0",CMD_DOC_NONE,NULL,NULL,"cluster",COMMAND_GROUP_CLUSTER,READONLY_History,0,READONLY_Tips,0,readonlyCommand,1,CMD_FAST|CMD_LOADING|CMD_STALE,ACL_CATEGORY_CONNECTION|ACL_CATEGORY_FAST,NULL,READONLY_Keyspecs,0,NULL,0)},
{MAKE_CMD("readwrite","Enables read-write queries for a connection to a Valkey replica node.","O(1)","3.0.0",CMD_DOC_NONE,NULL,NULL,"cluster",COMMAND_GROUP_CLUSTER,READWRITE_History,0,READWRITE_Tips,0,readwriteCommand,1,CMD_FAST|CMD_LOADING|CMD_STALE,ACL_CATEGORY_CONNECTION|ACL_CATEGORY_FAST,NULL,READWRITE_Keyspecs,0,NULL,0)},
/* connection */
Expand Down
1 change: 1 addition & 0 deletions src/commands/clusterscan.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
"since": "9.1.0",
"arity": -2,
"function": "clusterscanCommand",
"get_keys_function": "clusterscanGetKeys",
"command_flags": [
"READONLY",
"TOUCHES_ARBITRARY_KEYS"
Expand Down
23 changes: 23 additions & 0 deletions src/db.c
Original file line number Diff line number Diff line change
Expand Up @@ -3029,6 +3029,29 @@ int bitfieldGetKeys(struct serverCommand *cmd, robj **argv, int argc, getKeysRes
return 1;
}

/* CLUSTERSCAN getkeys function: when the cursor is "0", no slot routing is
* needed (any node can handle the initial scan call). For non-"0" cursors the
* format is "<fingerprint>-<hashtag>-<local_cursor>" and the cursor itself
* contains a hashtag that routes to the correct slot, so we return it as a
* "key" for the cluster routing layer to compute the correct target node. */
int clusterscanGetKeys(struct serverCommand *cmd, robj **argv, int argc, getKeysResult *result) {
Comment thread
enjoy-binbin marked this conversation as resolved.
UNUSED(cmd);
UNUSED(argc);

/* Initial cursor: no routing needed, handle on any node. */
if (strcmp(objectGetVal(argv[1]), "0") == 0) {
result->numkeys = 0;
return 0;
}

/* Non-"0" cursor contains {hashtag} for routing. */
keyReference *keys = getKeysPrepareResult(result, 1);
keys[0].pos = 1;
keys[0].flags = CMD_KEY_NOT_KEY;
result->numkeys = 1;
return 1;
}

int *selectDbIdArgs(robj **argv, int argc, int *count) {
if (argc < 2) return NULL;

Expand Down
1 change: 1 addition & 0 deletions src/server.h
Original file line number Diff line number Diff line change
Expand Up @@ -3826,6 +3826,7 @@ int zmpopGetKeys(struct serverCommand *cmd, robj **argv, int argc, getKeysResult
int bzmpopGetKeys(struct serverCommand *cmd, robj **argv, int argc, getKeysResult *result);
int setGetKeys(struct serverCommand *cmd, robj **argv, int argc, getKeysResult *result);
int bitfieldGetKeys(struct serverCommand *cmd, robj **argv, int argc, getKeysResult *result);
int clusterscanGetKeys(struct serverCommand *cmd, robj **argv, int argc, getKeysResult *result);

unsigned short crc16(const char *buf, int len);

Expand Down
63 changes: 63 additions & 0 deletions tests/unit/cluster/clusterscan.tcl
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,35 @@ start_cluster 1 0 {tags {external:skip cluster}} {

assert_equal $cursor "0"
}

test "CLUSTERSCAN cursor is NOT_KEY and does not break ACL key checks" {
# The CLUSTERSCAN cursor (e.g. "0" or "0-{06S}-0") is a routing token,
# not a real user key. A user with restricted key permissions (+clusterscan
# but only ~foo:*) must still be able to use CLUSTERSCAN without getting
# "NOPERM No permissions to access a key" on the cursor itself.

# Create a user with limited key access but allowed to run clusterscan
R 0 ACL SETUSER scan_acl_leak on >pass resetkeys ~foo:* resetchannels -@all +clusterscan
set rd [valkey_deferring_client 0]
$rd AUTH scan_acl_leak pass
$rd read

# Test 1: Initial cursor "0" should work (no key access needed)
$rd clusterscan 0
# Should not error
$rd read

# Test 2: Non-"0" cursor (contains {hashtag}) should also work.
# The cursor is a routing token marked NOT_KEY, not a real key,
# so ACL should not validate it against key patterns.
$rd clusterscan 0-{06S}-0
$rd clusterscan 0-{6ZJ}-0
# Should not error
$rd read
$rd read

$rd close
}
}

# CLUSTERSCAN Tests - 3-node cluster tests
Expand Down Expand Up @@ -758,3 +787,37 @@ start_cluster 1 1 {tags {external:skip cluster} overrides {hash-seed "fingerprin
assert_equal $n [llength $all_keys]
}
}

start_cluster 3 3 {tags {external:skip cluster}} {
test "CLUSTERSCAN 0 can be executed on any node without MOVED" {
# Cursor "0" is the initial scan start point and does not access any
# slot data, so it must be handled locally by any node regardless of
# slot ownership or role (primary/replica).
for {set id 0} {$id < [llength $::servers]} {incr id} {
# Plain clusterscan 0
# slot 0 hashtag is {06S}
assert_equal [R $id clusterscan 0] {0-{06S}-0 {}}

# clusterscan 0 with match
# "valkey" slot is 7659, hashtag is {39t}
assert_equal [R $id clusterscan 0 match valkey] {0-{39t}-0 {}}

# clusterscan 0 with slot
# slot 16383 hashtag is {6ZJ}
assert_equal [R $id clusterscan 0 slot 16383] {0-{6ZJ}-0 {}}

# clusterscan 0 with match and slot
# "valkey" slot is 7659, hashtag is {39t}
assert_equal [R $id clusterscan 0 match valkey slot 7659] {0-{39t}-0 {}}

# clusterscan 0 with skipscan
# "valkey" slot is 7659, hashtag is {39t}
assert_equal [R $id clusterscan 0 match valkey slot 0] {0 {}}
assert_equal [R $id clusterscan 0 match valkey slot 5460] {0 {}}
assert_equal [R $id clusterscan 0 match valkey slot 5461] {0 {}}
assert_equal [R $id clusterscan 0 match valkey slot 10922] {0 {}}
assert_equal [R $id clusterscan 0 match valkey slot 10923] {0 {}}
assert_equal [R $id clusterscan 0 match valkey slot 16383] {0 {}}
}
}
}
Loading