fix(memory-storage): prevent storage names from escaping the storage directory#3715
Merged
Conversation
…directory Storage names passed to the KeyValueStore, Dataset, and RequestQueue open APIs were used directly as filesystem path components, so a name containing `..` or an absolute path could resolve outside the intended storages directory. Add a shared `resolveStorageDirectory` helper that confines the resolved directory under its base storage directory, and route all name-based path construction (create and rename) through it.
github-actions
Bot
added
t-tooling
…irectory Key-value store record keys are used as on-disk file names, so a key containing `..` or an absolute path could escape the store directory when written via the storage client's `setRecord` (the core `KeyValueStore.setValue` already validates keys, but the lower-level client did not). Route the record key/file path construction through the same confinement check used for storage names, and rename the helper to `resolveWithinDirectory` since it now guards both storage directories and record files.
barjin
approved these changes
Jun 4, 2026
vladfrangu
approved these changes
Jun 4, 2026
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
4 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.
Storage names and key-value-store record keys are used directly as on-disk path components in
@crawlee/memory-storage. A value containing..or an absolute path could therefore resolve outside the intended storage directory and create/write files elsewhere:KeyValueStore/Dataset/RequestQueuegetOrCreate(name)andupdate({ name })resolvedpath.resolve(baseDir, name), so e.g.KeyValueStore.open('../escaped')escaped thekey_value_stores/datasets/request_queuesdirectory.setRecord({ key })built the file path asresolve(storeDir, key). The coreKeyValueStore.setValuealready validates keys against a restricted charset, but the lower-level memory-storage client did not, so a key like../escapedescaped the store directory.Adds a shared
resolveWithinDirectory(baseDirectory, segment)helper that resolves the candidate path and asserts it stays within the base directory, throwing otherwise. All name- and key-based path construction (resource-client constructors, rename inupdate, thefindOrCache*lookup helpers, and the key-value-store filesystem entry) now routes through it.Legitimate names/keys are unaffected — including nested segments that stay within the directory; only values that actually escape are rejected. Dataset entity IDs (sequential index) and request IDs (hashed) are internally generated and were already safe.