Skip to content
Open
Show file tree
Hide file tree
Changes from all 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
34 changes: 31 additions & 3 deletions src/db.c
Original file line number Diff line number Diff line change
Expand Up @@ -2503,9 +2503,14 @@ int getKeysFromCommandWithSpecs(struct serverCommand *cmd,

/* This function returns a sanity check if the command may have keys. */
int doesCommandHaveKeys(struct serverCommand *cmd) {
return cmd->getkeys_proc || /* has getkeys_proc (non modules) */
(cmd->flags & CMD_MODULE_GETKEYS) || /* module with GETKEYS */
(getAllKeySpecsFlags(cmd, 1) & CMD_KEY_NOT_KEY); /* has at least one key-spec not marked as NOT_KEY */
/* At least one key-spec not marked as NOT_KEY means the command has real keys. */
if (getAllKeySpecsFlags(cmd, 1) & CMD_KEY_NOT_KEY) return 1;
/* If the command has getkeys_proc but all its key-specs are NOT_KEY
* (e.g. CLUSTERSCAN), the proc is only used for cluster slot routing,
* not for real key arguments. */
if (cmd->getkeys_proc && cmd->key_specs_num > 0) return 0;
return cmd->getkeys_proc || /* has getkeys_proc (non modules) */
(cmd->flags & CMD_MODULE_GETKEYS); /* module with GETKEYS */
}

/* A simplified channel spec table that contains all of the commands
Expand Down Expand Up @@ -3029,6 +3034,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 {}}
}
}
}
16 changes: 16 additions & 0 deletions tests/unit/introspection-2.tcl
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,22 @@ start_server {tags {"introspection"}} {
assert_equal {{key1{t} {OW update}} {key2{t} {OW update}}} [r command getkeysandflags msetex 2 key1{t} val1 key2{t} val2 nx]
}

test {COMMAND GETKEYS* NOT_KEY commands should report no key arguments} {
# Shard pub/sub - shard channel is NOT_KEY
assert_error {ERR The command has no key arguments} {r command getkeys ssubscribe mychannel}
assert_error {ERR The command has no key arguments} {r command getkeys sunsubscribe mychannel}
assert_error {ERR The command has no key arguments} {r command getkeys spublish mychannel mymessage}
assert_error {ERR The command has no key arguments} {r command getkeysandflags ssubscribe mychannel}
assert_error {ERR The command has no key arguments} {r command getkeysandflags sunsubscribe mychannel}
assert_error {ERR The command has no key arguments} {r command getkeysandflags spublish mychannel mymessage}

# CLUSTERSCAN - cursor is a routing token, not a key
assert_error {ERR The command has no key arguments} {r command getkeys clusterscan 0}
assert_error {ERR The command has no key arguments} {r command getkeys clusterscan 0-{06S}-0}
assert_error {ERR The command has no key arguments} {r command getkeysandflags clusterscan 0}
assert_error {ERR The command has no key arguments} {r command getkeysandflags clusterscan 0-{06S}-0}
}

test "COMMAND LIST syntax error" {
assert_error "ERR syntax error*" {r command list bad_arg}
assert_error "ERR syntax error*" {r command list filterby bad_arg}
Expand Down
Loading