Fix GEORADIUS STORE ACL bypass via duplicate options

[tjade273]

Problem

getKeysFromCommandWithSpecs() (used by the ACL path and COMMAND GETKEYS) prefers a command's JSON key specs over its get_keys_function when the specs carry no VARIABLE_FLAGS. For GEORADIUS/GEORADIUSBYMEMBER the STORE/STOREDIST specs are keyword searches, and getKeysUsingKeySpecs() stops at the first matching keyword. The geo.c parser, however, is last-one-wins. So for GEORADIUS ... STORE scratch:ok STORE protected:dst, ACL only checks scratch:ok, while the command writes (or, when the source is missing, deletes) protected:dst, bypassing key-level write ACLs.

A user with +georadius, read on the source pattern, and write on only one benign destination pattern can delete or overwrite a key outside their permitted patterns.

Fix

Flag the STORE/STOREDIST key specs in georadius.json / georadiusbymember.json with VARIABLE_FLAGS so getKeysFromCommandWithSpecs() falls through to georadiusGetKeys, which resolves the effective (last) destination. This mirrors how MIGRATE routes through its getkeys_proc. commands.def regenerated.

georadiusGetKeys previously returned flags = 0 for every key, since it was only used by cluster routing where read/write direction is irrelevant. Now that ACL uses it, set CMD_KEY_RO | CMD_KEY_ACCESS on the source and CMD_KEY_OW | CMD_KEY_UPDATE on the store key so %R~ / %W~ direction is enforced.

Testing

Added regression tests:

• tests/unit/acl-v2.tcl: a restricted user's STORE scratch:ok STORE protected:dst is denied with NOPERM (both the overwrite and the missing-source delete variant), checked via ACL DRYRUN and end-to-end; a legitimate single destination still works.
• tests/unit/geo.tcl: COMMAND GETKEYS ... STORE a STORE b returns the last destination b for both commands.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 79eb49cd-93bc-495f-b627-0b59af784280

📥 Commits

Reviewing files that changed from the base of the PR and between f3bdf50 and 65a26ef.

📒 Files selected for processing (6)

• src/commands.def
• src/commands/georadius.json
• src/commands/georadiusbymember.json
• src/db.c
• tests/unit/acl-v2.tcl
• tests/unit/geo.tcl

---

📝 Walkthrough

Walkthrough

Updates GEORADIUS and GEORADIUSBYMEMBER commands to correctly handle repeated STORE/STOREDIST options by using the final destination key for ACL enforcement. Command schemas are marked with VARIABLE_FLAGS, the key extraction helper is updated to report accurate per-key flags, and comprehensive unit and ACL regression tests validate the behavior.

Changes

GEO Command Variable Flags

Layer / File(s)	Summary
Command metadata schema updates src/commands.def, src/commands/georadius.json, src/commands/georadiusbymember.json	GEORADIUS and GEORADIUSBYMEMBER key_specs entries are updated to include VARIABLE_FLAGS alongside UPDATE flag, signaling that per-key flags are computed dynamically.
georadiusGetKeys implementation and documentation src/db.c	georadiusGetKeys helper documentation clarified to explain that the effective destination key is determined by the last STORE/STOREDIST occurrence. Implementation changed to set CMD_KEY_RO | CMD_KEY_ACCESS on source key and CMD_KEY_OW | CMD_KEY_UPDATE on the final destination key instead of using zero flags.
Key extraction unit tests tests/unit/geo.tcl	New tests verify COMMAND GETKEYS returns only source when no STORE is present, and returns the effective (last) destination key when STORE/STOREDIST options are combined in various ways for both GEORADIUS and GEORADIUSBYMEMBER.
ACL regression tests tests/unit/acl-v2.tcl	Two new regression tests: first validates ACL checks apply to the final destination key (not decoy earlier keys), second ensures duplicate STORE cannot bypass write ACLs and protected keys remain unchanged when access is denied.

🎯 2 (Simple) | ⏱️ ~12 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately and specifically describes the main fix: addressing an ACL bypass vulnerability in GEORADIUS STORE by handling duplicate options.
Description check	✅ Passed	The description is directly related to the changeset, detailing the problem (ACL bypass via duplicate STORE options), the fix (flagging specs with VARIABLE_FLAGS), and the testing approach.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}