Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
47 commits
Select commit Hold shift + click to select a range
7d14bb1
Add database-level access control
dvkashapov Jul 4, 2025
97a27c5
Fix selector flag assignment and apply clang-format
dvkashapov Jul 7, 2025
b2fd61a
Extend serverCommand with get_dbid_args, unify ACL checks
dvkashapov Jul 14, 2025
b4adb23
Run generate commands
dvkashapov Oct 30, 2025
c0709a1
Merge remote-tracking branch 'upstream/unstable' into unstable
dvkashapov Oct 30, 2025
88d96ba
Fix tests and apply clang
dvkashapov Oct 30, 2025
98f4550
Delete CROSS_DB and NOT_IMPLEMENTED
dvkashapov Oct 30, 2025
e6c10dd
Delete CROSS_DB from commands.def
dvkashapov Oct 30, 2025
74ccbec
Add db+= and db-= syntax
dvkashapov Oct 31, 2025
8bc8d13
Fix comment style
dvkashapov Oct 31, 2025
a4397dc
clang-format fix
dvkashapov Oct 31, 2025
fa90b04
Refactor and add more tests
dvkashapov Nov 5, 2025
a3af4f6
Merge remote-tracking branch 'upstream/unstable' into unstable
dvkashapov Nov 6, 2025
15068cf
Fix reason in ACL LOG and add test
dvkashapov Nov 16, 2025
ec93e49
Use cmd->fullname when no argpos
dvkashapov Nov 16, 2025
711b4f3
Add module api for db-level check and test
dvkashapov Nov 16, 2025
fc5bc3a
apply pr suggestions
dvkashapov Nov 17, 2025
10d7499
apply clang-format
dvkashapov Nov 17, 2025
7f0e020
add ALL_DBS flag to migration/slot commands
dvkashapov Nov 17, 2025
b6ab15b
db= syntax, acl getuser support
dvkashapov Nov 26, 2025
0562230
Return ACL_DENIED_DB only for R/W commands in forbidden db
dvkashapov Nov 26, 2025
04568af
Use sds
dvkashapov Nov 26, 2025
19186e7
Merge remote-tracking branch 'upstream/unstable' into unstable
dvkashapov Nov 27, 2025
d35a3a7
Merge remote-tracking branch 'upstream/unstable' into unstable
dvkashapov Nov 27, 2025
f725b5a
Merge remote-tracking branch 'upstream/unstable' into unstable
dvkashapov Dec 4, 2025
a16ffb6
Add new errmsg for invalid dbid
dvkashapov Dec 4, 2025
7825f1d
Delete prev module api for databases
dvkashapov Dec 4, 2025
1b88e62
Add new API
dvkashapov Dec 4, 2025
a17927a
Reorder transaction_db_id in select and more MULTI/EXEC tests
dvkashapov Dec 4, 2025
3f437c8
Apply review suggestions
dvkashapov Dec 12, 2025
efa1125
Comment for VM_ACLCheckCommandPermissions and delete reset dbid
dvkashapov Dec 13, 2025
c3afa10
Revert reset in multi, move errno ERANGE
dvkashapov Dec 15, 2025
d7c9bd2
make ACLSelectorCanAccessDb static inline
dvkashapov Dec 16, 2025
42b8334
Restrict all commands with keys/keyspace, add alldbs to FLUSHSLOT
dvkashapov Dec 17, 2025
16dc2dc
Eval/func tests, clean up some lines
dvkashapov Dec 17, 2025
5dbce55
New api with denial_reason ptr
dvkashapov Dec 17, 2025
5d815aa
Merge remote-tracking branch 'upstream/unstable' into unstable
dvkashapov Dec 17, 2025
0ba174a
Small fixes
dvkashapov Dec 17, 2025
ab93b29
Change wording in JSON files
dvkashapov Dec 18, 2025
8ce4a75
Apply review suggestions
dvkashapov Dec 20, 2025
41703e4
Merge remote-tracking branch 'upstream/unstable' into unstable
dvkashapov Dec 20, 2025
1411b8a
Update lua and module api to use new VM_ACL check
dvkashapov Dec 20, 2025
19ae1b7
Merge remote-tracking branch 'upstream/unstable' into unstable
dvkashapov Dec 21, 2025
755678a
Apply review suggestions
dvkashapov Dec 21, 2025
da84977
Del test for COUNTKEYSINSLOT
dvkashapov Dec 21, 2025
5666547
Delete get_dbid_args for MIGRATE
dvkashapov Dec 22, 2025
e7907a5
Del dbid >= server.dbnum
dvkashapov Dec 23, 2025
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
242 changes: 225 additions & 17 deletions src/acl.c

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions src/cli_commands.c
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@

/* Definitions to configure commands.c to generate the above structs. */
#define MAKE_CMD(name, summary, complexity, since, doc_flags, replaced, deprecated, group, group_enum, history, \
num_history, tips, num_tips, function, arity, flags, acl, key_specs, key_specs_num, get_keys, \
numargs) \
num_history, tips, num_tips, function, arity, flags, acl, get_dbid_args, key_specs, \
key_specs_num, get_keys, numargs) \
name, summary, group, since, numargs
#define MAKE_ARG(name, type, key_spec_index, token, summary, since, flags, numsubargs, deprecated_since) \
name, type, token, since, flags, numsubargs
Expand Down
6 changes: 3 additions & 3 deletions src/commands.c
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@
#include "server.h"

#define MAKE_CMD(name, summary, complexity, since, doc_flags, replaced, deprecated, group, group_enum, history, \
num_history, tips, num_tips, function, arity, flags, acl, key_specs, key_specs_num, get_keys, \
numargs) \
num_history, tips, num_tips, function, arity, flags, acl, get_dbid_args, key_specs, \
key_specs_num, get_keys, numargs) \
name, summary, complexity, since, doc_flags, replaced, deprecated, group_enum, history, num_history, tips, \
num_tips, function, arity, flags, acl, key_specs, key_specs_num, get_keys, numargs
num_tips, function, arity, flags, acl, get_dbid_args, key_specs, key_specs_num, get_keys, numargs
#define MAKE_ARG(name, type, key_spec_index, token, summary, since, flags, numsubargs, deprecated_since) \
name, type, key_spec_index, token, summary, since, flags, deprecated_since, numsubargs
#define COMMAND_STRUCT serverCommand
Expand Down
848 changes: 425 additions & 423 deletions src/commands.def

Large diffs are not rendered by default.

4 changes: 4 additions & 0 deletions src/commands/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ following keys. To be safe, assume all of them are optional.
the command. (Don't use it for anything else.)
* `"command_flags"`: An array of flags represented as strings. Command flags:
* `"ADMIN"`
* `"ALL_DBS"`
* `"ALLOW_BUSY"`
* `"ASKING"`
* `"BLOCKING"`
Expand Down Expand Up @@ -93,6 +94,9 @@ following keys. To be safe, assume all of them are optional.
* `"STREAM"`
* `"STRING"`
* `"TRANSACTION"`
* `"get_dbid_args"`: The name of the C function in Valkey's source code
Comment thread
madolson marked this conversation as resolved.
implementing retrieval of database ID arguments from commands that accept
database ID as an argument.
* `"command_tips"`: Optional. A list of one or more of these strings:
* `"NONDETERMINISTIC_OUTPUT"`
* `"NONDETERMINISTIC_OUTPUT_ORDER"`
Expand Down
11 changes: 11 additions & 0 deletions src/commands/acl-getuser.json
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@
[
"7.0.0",
"Added selectors and changed the format of key and channel patterns from a list to their rule representation."
],
[
"9.1.0",
"Added database permission rules."
]
],
"command_flags": [
Expand Down Expand Up @@ -61,6 +65,10 @@
"description": "Root selector's channels.",
"type": "string"
},
"databases": {
"description": "Root selector's databases.",
"type": "string"
},
"selectors": {
"type": "array",
"items": {
Expand All @@ -75,6 +83,9 @@
},
"channels": {
"type": "string"
},
"databases": {
"type": "string"
}
}
}
Expand Down
4 changes: 4 additions & 0 deletions src/commands/acl-setuser.json
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@
[
"7.0.0",
"Added selectors and key based permissions."
],
[
"9.1.0",
"Added database permission rules."
]
],
"command_flags": [
Expand Down
3 changes: 2 additions & 1 deletion src/commands/cluster-cancelslotmigrations.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@
"command_flags": [
"NO_ASYNC_LOADING",
"ADMIN",
"STALE"
"STALE",
"ALL_DBS"
],
"reply_schema": {
"const": "OK"
Expand Down
3 changes: 2 additions & 1 deletion src/commands/cluster-migrateslots.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@
"command_flags": [
"NO_ASYNC_LOADING",
"ADMIN",
"STALE"
"STALE",
"ALL_DBS"
],
"arguments": [
{
Expand Down
1 change: 1 addition & 0 deletions src/commands/copy.json
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
"SLOW",
"WRITE"
],
"get_dbid_args": "copyDbIdArgs",
"key_specs": [
{
"flags": [
Expand Down
3 changes: 2 additions & 1 deletion src/commands/flushall.json
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@
]
],
"command_flags": [
"WRITE"
"WRITE",
"ALL_DBS"
],
"acl_categories": [
"DANGEROUS",
Expand Down
1 change: 1 addition & 0 deletions src/commands/move.json
Comment thread
dvkashapov marked this conversation as resolved.
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
"KEYSPACE",
"WRITE"
],
"get_dbid_args": "moveDbIdArgs",
"key_specs": [
{
"flags": [
Expand Down
1 change: 1 addition & 0 deletions src/commands/select.json
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
"CONNECTION",
"FAST"
],
"get_dbid_args": "selectDbIdArgs",
Comment thread
dvkashapov marked this conversation as resolved.
"reply_schema": {
"const": "OK"
},
Expand Down
1 change: 1 addition & 0 deletions src/commands/swapdb.json
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
"KEYSPACE",
"WRITE"
],
"get_dbid_args": "swapdbDbIdArgs",
"arguments": [
{
"name": "index1",
Expand Down
65 changes: 63 additions & 2 deletions src/db.c
Original file line number Diff line number Diff line change
Expand Up @@ -899,9 +899,14 @@ void selectCommand(client *c) {

if (selectDb(c, id) == C_ERR) {
addReplyError(c, "DB index is out of range");
} else {
addReply(c, shared.ok);
return;
}

if (c->flag.multi) {
serverAssert(c->mstate != NULL);
c->mstate->transaction_db_id = id;
}
addReply(c, shared.ok);
}

void randomkeyCommand(client *c) {
Expand Down Expand Up @@ -2954,3 +2959,59 @@ int bitfieldGetKeys(struct serverCommand *cmd, robj **argv, int argc, getKeysRes
}
return 1;
}

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

long long dbid;
if (getLongLongFromObject(argv[1], &dbid) != C_OK) return NULL;
if (dbid < 0 || dbid >= server.dbnum) return NULL;

int *result = zmalloc(sizeof(int));
result[0] = (int)dbid;
*count = 1;
return result;
}

int *swapdbDbIdArgs(robj **argv, int argc, int *count) {
if (argc < 3) return NULL;

long long db1, db2;
if (getLongLongFromObject(argv[1], &db1) != C_OK ||
getLongLongFromObject(argv[2], &db2) != C_OK) return NULL;
if (db1 < 0 || db1 >= server.dbnum || db2 < 0 || db2 >= server.dbnum) return NULL;

int *result = zmalloc(2 * sizeof(int));
result[0] = (int)db1;
result[1] = (int)db2;
*count = 2;
return result;
}

int *moveDbIdArgs(robj **argv, int argc, int *count) {
if (argc < 3) return NULL;

long long dbid;
if (getLongLongFromObject(argv[2], &dbid) != C_OK) return NULL;
if (dbid < 0 || dbid >= server.dbnum) return NULL;

int *result = zmalloc(sizeof(int));
result[0] = (int)dbid;
*count = 1;
return result;
}

int *copyDbIdArgs(robj **argv, int argc, int *count) {
if (argc < 5) return NULL;

if (strcasecmp(argv[3]->ptr, "db") != 0) return NULL;

long long dbid;
if (getLongLongFromObject(argv[4], &dbid) != C_OK) return NULL;
if (dbid < 0 || dbid >= server.dbnum) return NULL;

int *result = zmalloc(sizeof(int));
result[0] = (int)dbid;
*count = 1;
return result;
}
15 changes: 15 additions & 0 deletions src/intset.c
Original file line number Diff line number Diff line change
Expand Up @@ -335,3 +335,18 @@ int intsetValidateIntegrity(const unsigned char *p, size_t size, int deep) {

return 1;
}

/* Free an intset */
void intsetFree(intset *is) {
if (is) zfree(is);
}

/* Deep copy an intset */
intset *intsetDup(intset *is) {
if (!is) return intsetNew();

size_t size = intsetBlobLen(is);
intset *copy = zmalloc(size);
memcpy(copy, is, size);
return copy;
}
2 changes: 2 additions & 0 deletions src/intset.h
Original file line number Diff line number Diff line change
Expand Up @@ -49,5 +49,7 @@ uint8_t intsetGet(intset *is, uint32_t pos, int64_t *value);
uint32_t intsetLen(const intset *is);
size_t intsetBlobLen(intset *is);
int intsetValidateIntegrity(const unsigned char *is, size_t size, int deep);
void intsetFree(intset *is);
intset *intsetDup(intset *is);

#endif // __INTSET_H
80 changes: 76 additions & 4 deletions src/module.c
Comment thread
dvkashapov marked this conversation as resolved.
Original file line number Diff line number Diff line change
Expand Up @@ -1261,6 +1261,7 @@ int64_t commandFlagsFromString(char *s) {
else if (!strcasecmp(t,"no-cluster")) flags |= CMD_MODULE_NO_CLUSTER;
else if (!strcasecmp(t,"no-mandatory-keys")) flags |= CMD_NO_MANDATORY_KEYS;
else if (!strcasecmp(t,"allow-busy")) flags |= CMD_ALLOW_BUSY;
else if (!strcasecmp(t,"all-dbs")) flags |= CMD_ALL_DBS;
else break;
/* clang-format on */
}
Expand Down Expand Up @@ -1352,6 +1353,8 @@ ValkeyModuleCommand *moduleCreateCommandProxy(struct ValkeyModule *module,
* * **"allow-busy"**: Permit the command while the server is blocked either by
* a script or by a slow module command, see
* VM_Yield.
* * **"all-dbs"**: The command accesses all databases and to execute this
* command user has to have `alldbs`
* * **"getchannels-api"**: The command implements the interface to return
* the arguments that are channels.
*
Expand Down Expand Up @@ -3975,9 +3978,12 @@ int VM_PublishMessageShard(ValkeyModuleCtx *ctx, ValkeyModuleString *channel, Va
return pubsubPublishMessageAndPropagateToCluster(channel, message, 1);
}

/* Return the currently selected DB. */
/* Return the currently selected DB.
* When inside a MULTI/EXEC transaction, returns transaction_db_id which tracks
* the DB selected within the transaction (may differ from client->db->id).
* Otherwise, returns the client's actual DB. */
int VM_GetSelectedDb(ValkeyModuleCtx *ctx) {
return ctx->client->db->id;
return (ctx->client->flag.multi) ? ctx->client->mstate->transaction_db_id : ctx->client->db->id;
}


Expand Down Expand Up @@ -6612,7 +6618,8 @@ ValkeyModuleCallReply *VM_Call(ValkeyModuleCtx *ctx, const char *cmdname, const
int acl_errpos;
int acl_retval;

acl_retval = ACLCheckAllUserCommandPerm(user, c->cmd, c->argv, c->argc, &acl_errpos);
int dbid = (c->flag.multi) ? c->mstate->transaction_db_id : c->db->id;
acl_retval = ACLCheckAllUserCommandPerm(user, c->cmd, c->argv, c->argc, dbid, &acl_errpos);
if (acl_retval != ACL_OK) {
int context = scriptIsRunning() ? ACL_LOG_CTX_SCRIPT : ACL_LOG_CTX_MODULE;
sds object = (acl_retval == ACL_DENIED_CMD) ? sdsdup(c->cmd->fullname) : sdsdup(c->argv[acl_errpos]->ptr);
Expand Down Expand Up @@ -10129,6 +10136,14 @@ ValkeyModuleUser *VM_GetModuleUserFromUserName(ValkeyModuleString *name) {
*
* * ENOENT: Specified command does not exist.
* * EACCES: Command cannot be executed, according to ACL rules
*
* NOTE: Since 9.1, the underlying ACL check will NOT validate the user's access to the database.
* For users WITHOUT the `alldbs` flag: VALKEYMODULE_ERR will be returned for any
* READ or WRITE command, even if the user has permission to access the current database.
*
* For comprehensive ACL validation that handles all types of permissions for users, it is
* recommended to use VM_ACLCheckPermissions() instead, which accepts a dbid parameter
* and properly validates access.
*/
int VM_ACLCheckCommandPermissions(ValkeyModuleUser *user, ValkeyModuleString **argv, int argc) {
int keyidxptr;
Expand All @@ -10140,7 +10155,7 @@ int VM_ACLCheckCommandPermissions(ValkeyModuleUser *user, ValkeyModuleString **a
return VALKEYMODULE_ERR;
}

if (ACLCheckAllUserCommandPerm(user->user, cmd, argv, argc, &keyidxptr) != ACL_OK) {
if (ACLCheckAllUserCommandPerm(user->user, cmd, argv, argc, -1, &keyidxptr) != ACL_OK) {
errno = EACCES;
return VALKEYMODULE_ERR;
}
Expand Down Expand Up @@ -10211,6 +10226,61 @@ int VM_ACLCheckChannelPermissions(ValkeyModuleUser *user, ValkeyModuleString *ch
return VALKEYMODULE_OK;
}

/* Check if the command with its arguments can be executed by the user, according to the
* ACLs associated with it. This function performs a comprehensive ACL check including:
* - Command permissions
* - Key permissions
* - Channel permissions
* - Database permissions
*
* On success VALKEYMODULE_OK is returned, otherwise VALKEYMODULE_ERR is returned and
* errno is set to one of the following values:
* * EINVAL: Invalid arguments (e.g., negative dbid or dbid >= server.dbnum, command does not exist)
* * EACCES: Permission denied (for any ACL violation)
*
* The optional denial_reason parameter can be used to get more specific information about
* why the permission was denied. If provided (not NULL), it will be set to one of:
* * VALKEYMODULE_ACL_LOG_CMD: User does not have permission to execute the command
* * VALKEYMODULE_ACL_LOG_KEY: User does not have permission to access a key
* * VALKEYMODULE_ACL_LOG_CHANNEL: User does not have permission to access a channel
* * VALKEYMODULE_ACL_LOG_DB: User does not have permission to access the database
*/
int VM_ACLCheckPermissions(ValkeyModuleUser *user,
ValkeyModuleString **argv,
int argc,
int dbid,
ValkeyModuleACLLogEntryReason *denial_reason) {
int keyidxptr;
struct serverCommand *cmd;

if (dbid < 0 || dbid >= server.dbnum) {
errno = EINVAL;
return VALKEYMODULE_ERR;
}

if ((cmd = lookupCommand(argv, argc)) == NULL) {
errno = EINVAL;
return VALKEYMODULE_ERR;
}

int acl_retval = ACLCheckAllUserCommandPerm(user->user, cmd, argv, argc, dbid, &keyidxptr);
if (acl_retval != ACL_OK) {
errno = EACCES;
if (denial_reason) {
switch (acl_retval) {
case ACL_DENIED_CMD: *denial_reason = VALKEYMODULE_ACL_LOG_CMD; break;
case ACL_DENIED_KEY: *denial_reason = VALKEYMODULE_ACL_LOG_KEY; break;
case ACL_DENIED_CHANNEL: *denial_reason = VALKEYMODULE_ACL_LOG_CHANNEL; break;
case ACL_DENIED_DB: *denial_reason = VALKEYMODULE_ACL_LOG_DB; break;
default: *denial_reason = VALKEYMODULE_ACL_LOG_CMD; break;
}
}
return VALKEYMODULE_ERR;
}

return VALKEYMODULE_OK;
}

/* Helper function to map a ValkeyModuleACLLogEntryReason to ACL Log entry reason. */
int moduleGetACLLogEntryReason(ValkeyModuleACLLogEntryReason reason) {
int acl_reason = 0;
Expand All @@ -10219,6 +10289,7 @@ int moduleGetACLLogEntryReason(ValkeyModuleACLLogEntryReason reason) {
case VALKEYMODULE_ACL_LOG_KEY: acl_reason = ACL_DENIED_KEY; break;
case VALKEYMODULE_ACL_LOG_CHANNEL: acl_reason = ACL_DENIED_CHANNEL; break;
case VALKEYMODULE_ACL_LOG_CMD: acl_reason = ACL_DENIED_CMD; break;
case VALKEYMODULE_ACL_LOG_DB: acl_reason = ACL_DENIED_DB; break;
default: break;
}
return acl_reason;
Expand Down Expand Up @@ -14620,6 +14691,7 @@ void moduleRegisterCoreAPI(void) {
REGISTER_API(ACLCheckCommandPermissions);
REGISTER_API(ACLCheckKeyPermissions);
REGISTER_API(ACLCheckChannelPermissions);
REGISTER_API(ACLCheckPermissions);
REGISTER_API(ACLAddLogEntry);
REGISTER_API(ACLAddLogEntryByUserName);
REGISTER_API(FreeModuleUser);
Expand Down
Loading
Loading