Sanitize dsID before interpolating into file paths#126
Merged
Conversation
Saved-query filenames were guarded against path separators, but dsID was interpolated straight into filepath.Join for the cache, history, and queries paths. dsID is app-generated today, but a malformed or imported config with a dsID containing '../' could escape ~/.snowy. Add validateDsID (charset allowlist ^[A-Za-z0-9_-]+$) and call it at the three path-builder choke points: metadataCachePath, historyFile, and queriesDir. Valid IDs (timestamps, 'default', UUIDs) are unchanged. Closes #117
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
1 participant
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.
Closes #117.
What changed
Saved-query filenames were already guarded against path separators, but
dsIDwas interpolated straight intofilepath.Join(...)for the cache, history, and queries paths.dsIDis app-generated (Date.now()timestamps) today, but a malformed or imported config with adsIDcontaining../could escape~/.snowy.Added a shared
validateDsID(charset allowlist^[A-Za-z0-9_-]+$) and call it at the three path-builder choke points every operation funnels through:metadataCachePath— cache load/savehistoryFile— history record/readqueriesDir— all saved-query ops (save/list/load/delete/rename)The allowlist admits every real ID (timestamps, the literal
default, UUID-style with hyphens) and rejects..,/,\,., whitespace, null bytes, and empty.Acceptance criteria
dsIDis validated before use in any filesystem path (cache, history, queries)dsIDcontaining path traversal or separators is rejected, with a testTesting
dsid_test.go: table-drivenvalidateDsID(valid + malicious inputs) and a boundary test assertingSaveMetadataCache/RecordHistory/SaveQueryall reject../escapedand that no file is created outside~/.snowy.go test .— full suite green (the pre-existing filename-guard test still passes).