You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
test: cover cached EVALSHA_RO after EVAL_RO ACL revocation #3579
Following #3563 and the merged fix in #3567, I'd like to add one focused regression test for the direct, cached read-only script path.
The existing integration tests exercise a missing script falling back to EVAL_RO and the read-only MULTI path. I couldn't find a test for the following sequence:
With a Redis ACL user allowed EVAL_RO and EVALSHA_RO but denied EVAL and EVALSHA, flush the script cache.
Call a read-only defined script once. The initial NOSCRIPT should fall back to EVAL_RO and populate the cache.
Revoke only EVAL_RO from that user; verify a direct EVAL_RO call now fails with NOPERM.
Call the defined script again; it should still succeed through cached EVALSHA_RO.
This would catch an unintended fallback or command-selection regression that the initial-load and transaction cases don't directly exercise. The proposed change is test-only, in packages/client/lib/client/index.spec.ts, without changing command behavior or public APIs.
I've prepared a small candidate diff against master at 4ce935514016e9f40561740343d5b9d831044631. The test has not yet been run against a native Redis instance or the project's full suite, so I'm not reporting a runtime pass or an existing defect.
Would this regression fit the project's test coverage? If so, I'll run it in the existing harness and submit a focused PR for review.
The existing loads a missing script with EVAL_RO test already covers both halves. Its user is denied EVALSHA, so the test only passes if the client tries EVALSHA_RO first and then falls back to EVAL_RO. The client keeps no "loaded" state and falls back only on NOSCRIPT. A second call with the script already cached therefore takes the same EVALSHA_RO path. An extra integration test for that case would add ACL setup to the suite without catching anything new.
Closing as not planned. If you find a case where a read-only script uses the wrong command, please open an issue with a reproduction.
Activity
nkaradzhov commentedon Nov 4, 2026
Thanks, but we'll pass on this one.
The existing
loads a missing script with EVAL_ROtest already covers both halves. Its user is deniedEVALSHA, so the test only passes if the client triesEVALSHA_ROfirst and then falls back toEVAL_RO. The client keeps no "loaded" state and falls back only onNOSCRIPT. A second call with the script already cached therefore takes the sameEVALSHA_ROpath. An extra integration test for that case would add ACL setup to the suite without catching anything new.Closing as not planned. If you find a case where a read-only script uses the wrong command, please open an issue with a reproduction.