Skip to content

MOD-14479: Solve Infinite Loop When FT.CURSOR With Specific ACL Permission Is Used#8888

Merged
kei-nan merged 2 commits into8.2from
8.2_jk_fix_command_key_offsets
Apr 5, 2026
Merged

MOD-14479: Solve Infinite Loop When FT.CURSOR With Specific ACL Permission Is Used#8888
kei-nan merged 2 commits into8.2from
8.2_jk_fix_command_key_offsets

Conversation

@kei-nan
Copy link
Copy Markdown
Collaborator

@kei-nan kei-nan commented Mar 31, 2026 •
edited by cursor Bot
Loading

Release note suggestion: "Fixed an infinite loop that could occur when FT.CURSOR was used with ACL users configured with specific key permissions."

In certain scenarios the command key step would be used by redis and could lead to an infinite loop when parsing the command.
Avoid using negative key step values when registering search commands.

Related PR #7971

Main objects this PR modified

  1. Command Registration

Mark if applicable

  • This PR introduces API changes
  • This PR introduces serialization changes

Release Notes

  • This PR requires release notes
  • This PR does not require release notes

If a release note is required (bug fix / new feature / enhancement), describe the user impact of this PR in the title.

Note

Medium Risk
Changes Redis module command registration key metadata and shifts Enterprise routing policy to pack/ramp-enterprise.yml, which could affect command routing/validation if misconfigured. Scope is limited to RediSearch command metadata and a small set of commands.

Overview
Fixes a Redis Enterprise hang/infinite loop that could occur when running FT.CURSOR under certain ACL key-permission configurations.

The module no longer registers RediSearch commands with negative keystep/key-spec values (which could be interpreted by Redis during ACL/key parsing); instead, Enterprise-specific routing/key metadata for FT.SEARCH/FT.AGGREGATE/FT.CURSOR is defined in pack/ramp-enterprise.yml via overide_command.

Reviewed by Cursor Bugbot for commit 9a56e2d. Bugbot is set up for automated code reviews on this repo. Configure here.

@kei-nan kei-nan requested review from Itzikvaknin and raz-mon March 31, 2026 10:06
@kei-nan kei-nan self-assigned this Mar 31, 2026
@jit-ci
Copy link
Copy Markdown

jit-ci Bot commented Mar 31, 2026 •
edited
Loading

🛡️ Jit Security Scan Results

CRITICAL HIGH MEDIUM

✅ No security findings were detected in this PR

Security scan by Jit

cursor[bot]
cursor Bot reviewed Mar 31, 2026
View reviewed changes
Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

Comment thread pack/ramp-enterprise.yml
@codecov
Copy link
Copy Markdown

codecov Bot commented Mar 31, 2026 •
edited
Loading

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.97%. Comparing base (a20af23) to head (9a56e2d).
⚠️ Report is 2 commits behind head on 8.2.

Additional details and impacted files 
@@            Coverage Diff             @@
##              8.2    #8888      +/-   ##
==========================================
- Coverage   88.99%   88.97%   -0.03%     
==========================================
  Files         260      260              
  Lines       42059    42055       -4     
  Branches     3851     3851              
==========================================
- Hits        37432    37418      -14     
- Misses       4578     4588      +10     
  Partials       49       49
Flag Coverage Δ
flow 82.15% <100.00%> (+<0.01%) ⬆️
unit 46.82% <0.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@raz-mon
Copy link
Copy Markdown
Collaborator

raz-mon commented Mar 31, 2026 •
edited
Loading

@kei-nan Can you describe the infinite loop scenario? E.g., via a reproduction script.
Also:

  • How does this fix it?
  • Why does this target 8.2 and not master, is it not relevant for v >= 8.4?

@kei-nan
Copy link
Copy Markdown
Collaborator Author

kei-nan commented Apr 5, 2026

Hey @raz-mon,
In order to reproduce this we need to cause the module to think it is running in enterprise.
In GDB that can be done by breaking in the DetectClusterType function and simulating that the env is an enterprise version.
Then in CLI you can do something like:

redis-cli -p 6399 ACL SETUSER mod14479_user resetkeys resetchannels resetpass on \>123 ~allowed:* +@all
timeout 5 redis-cli -p 6399 --user mod14479_user -a 123 FT.CURSOR READ mod14479_idx 555 COUNT 1
echo $?   # 124\

raz-mon
raz-mon previously approved these changes Apr 5, 2026
View reviewed changes
@kei-nan kei-nan requested a review from oshadmi April 5, 2026 14:09
@kei-nan
Copy link
Copy Markdown
Collaborator Author

kei-nan commented Apr 5, 2026

Relates to:
#7971

oshadmi
oshadmi reviewed Apr 5, 2026
View reviewed changes
Comment thread pack/ramp-enterprise.yml Outdated
@github-actions github-actions Bot added the size:S label Apr 5, 2026
@kei-nan kei-nan requested review from oshadmi and raz-mon April 5, 2026 14:46
@kei-nan kei-nan enabled auto-merge April 5, 2026 14:50
@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented Apr 5, 2026

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@kei-nan kei-nan added this pull request to the merge queue Apr 5, 2026
Merged via the queue into 8.2 with commit 4be7ec0 Apr 5, 2026
35 checks passed
@kei-nan kei-nan deleted the 8.2_jk_fix_command_key_offsets branch April 5, 2026 16:47
@redisearch-backport-pull-request
Copy link
Copy Markdown
Contributor

redisearch-backport-pull-request Bot commented Apr 5, 2026

Backport failed for 2.8, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin 2.8
git worktree add -d .worktree/backport-8888-to-2.8 origin/2.8
cd .worktree/backport-8888-to-2.8
git switch --create backport-8888-to-2.8
git cherry-pick -x 4be7ec06dcb58391745ab844f255f675e6b7f993

@redisearch-backport-pull-request
Copy link
Copy Markdown
Contributor

redisearch-backport-pull-request Bot commented Apr 5, 2026

Backport failed for 2.6, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin 2.6
git worktree add -d .worktree/backport-8888-to-2.6 origin/2.6
cd .worktree/backport-8888-to-2.6
git switch --create backport-8888-to-2.6
git cherry-pick -x 4be7ec06dcb58391745ab844f255f675e6b7f993

@redisearch-backport-pull-request
Copy link
Copy Markdown
Contributor

redisearch-backport-pull-request Bot commented Apr 5, 2026

Backport failed for 2.10, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin 2.10
git worktree add -d .worktree/backport-8888-to-2.10 origin/2.10
cd .worktree/backport-8888-to-2.10
git switch --create backport-8888-to-2.10
git cherry-pick -x 4be7ec06dcb58391745ab844f255f675e6b7f993

This was referenced Apr 6, 2026
Merged
Merged
Merged
@YaacovHazan YaacovHazan mentioned this pull request May 5, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cursor cursor[bot] cursor[bot] left review comments

@oshadmi oshadmi oshadmi approved these changes

@Itzikvaknin Itzikvaknin Awaiting requested review from Itzikvaknin

@raz-mon raz-mon Awaiting requested review from raz-mon

Assignees

@kei-nan kei-nan

Labels

backport 2.6 backport 2.8 backport 2.10 size:S

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@kei-nan @raz-mon @oshadmi