fix(websocket): validate last_id query param format#40626
Conversation
getLastId returned the raw last_id query param, which was then passed to
incrementId and used as a Redis stream range start. Malformed values such as
last_id=abc-xyz produced ids like abc-NaN. Redis handles these gracefully and a
client can only read its own channel's stream, so impact is limited, but no
positive validation enforced the expected format.
Validate last_id against the Redis stream ID format (/^\d{1,15}-\d{1,10}$/) and
return null for anything that doesn't match, so malformed input is ignored
rather than processed. Export getLastId and add unit tests for valid and
malformed values plus a wsConnection test confirming a malformed last_id does
not trigger a stream range fetch.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Code Review Agent Run #a8a300Actionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
✅ Deploy Preview for superset-docs-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
There was a problem hiding this comment.
Pull request overview
Adds positive validation for the last_id query parameter in the Superset websocket server. Previously, getLastId returned the raw value without validation, allowing malformed inputs to be passed downstream to Redis stream operations. The change enforces the Redis stream ID format <millisecondsTime>-<sequence> via regex and returns null for malformed values.
Changes:
- Add
REDIS_STREAM_ID_REGEXand validatelast_idingetLastId, returningnull(with a warning log) for malformed values. - Export
getLastIdso it can be unit-tested. - Add unit tests for
getLastIdand awsConnectiontest asserting malformedlast_iddoes not trigger a stream range fetch.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| superset-websocket/src/index.ts | Add regex validation for last_id, export getLastId, log and ignore malformed values. |
| superset-websocket/spec/index.test.ts | Add tests for getLastId and for ignoring malformed last_id in wsConnection. |
sha174n
added
the
merge-if-green
github-project-automation
Bot
moved this from Needs Review
to Approved and/or Merged
in Superset Review Help Wanted
SUMMARY
In
superset-websocket,getLastIdreturned the rawlast_idquery parameter, which is then passed toincrementIdand used as the start of a Redis stream range read during the client reconnection flow. Malformed values likelast_id=abc-xyzproduced ids such asabc-NaN. Redis handles these gracefully and a client can only read its own channel's stream, so the impact is limited — but no positive validation enforced the expected format.This adds validation:
last_idmust match the Redis stream ID format/^\d{1,15}-\d{1,10}$/(<millisecondsTime>-<sequence>); anything else is ignored (returnsnull), so malformed input is no longer processed.TESTING INSTRUCTIONS
New tests:
getLastId: returnsnullwhen absent, returns well-formed IDs, and returnsnullfor a range of malformed inputs (abc-xyz, missing parts, injection-like suffixes, extra segments).wsConnection: a malformedlast_iddoes not trigger a stream range fetch.ADDITIONAL INFORMATION
🤖 Generated with Claude Code