Harden checkpoint store against path traversal and resource exhaustion#18
Merged
antonpk1 merged 1 commit intoFeb 12, 2026
Merged
Conversation
The checkpoint ID parameter flows from client input to filesystem paths without validation. A crafted ID like "../../etc/cron.d/evil" would write files outside the checkpoint directory via path.join. This adds three layers of defense: - Checkpoint ID validation: reject anything that isn't alphanumeric, hyphens, or underscores (blocks path traversal characters) - Resolved path verification: confirm the final path stays within the checkpoint directory even after symlink resolution - Input size limits: cap elements/data/json inputs at 5 MB to prevent memory and disk exhaustion - Checkpoint pruning: FileCheckpointStore now evicts oldest entries beyond 100 to prevent unbounded disk growth, matching the TTL that RedisCheckpointStore already has Signed-off-by: debu-sinha <debusinha2009@gmail.com>
|
@debu-sinha is attempting to deploy a commit to the Excalidraw Team on Vercel. A member of the Team first needs to authorize it. |
antonpk1
approved these changes
Feb 12, 2026
antonpk1
left a comment
antonpk1
left a comment
Collaborator
There was a problem hiding this comment.
super helpful, thanks!
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this fixes
The checkpoint ID flows from client input directly into filesystem paths via
path.join(this.dir, id + ".json")without sanitization. A crafted checkpoint ID containing path traversal sequences (e.g.,../../etc/cron.d/evil) could write arbitrary JSON files outside the intended checkpoint directory.While
save_checkpointandread_checkpointare scoped to widget visibility (["app"]), the input still reachesFileCheckpointStoreunsanitized. In HTTP/Streamable transport mode, this surface is reachable without additional authentication.Additionally, there are no bounds on input size or checkpoint count, which could lead to disk/memory exhaustion.
I wrote about these patterns more broadly in The MCP Ecosystem Has a Security Problem -- this PR addresses the specific instances found here.
Changes
Path traversal prevention (
checkpoint-store.ts):validateCheckpointId()that rejects any ID not matching[a-zA-Z0-9_-]with a 64-char length cappath.resolveprefix check) as defense-in-depthResource exhaustion limits:
elements,json, anddatatool inputs inserver.tsFileCheckpointStorenow prunes oldest checkpoints beyond 100 entries (matching the TTL thatRedisCheckpointStorealready has)MemoryCheckpointStoreevicts oldest entry when exceeding limitNo breaking changes -- server-generated checkpoint IDs are already hex UUIDs (
crypto.randomUUID().replace(/-/g, "").slice(0, 18)) which pass the new validation. The size limits are generous enough for normal usage.Testing
npx tsc --noEmitpasses with no type errors../../etc/passwdand../../../tmp/evilare rejected