use StrictCollectionPath for uploading collection snapshot

[generall]

• Prevent security risk of uploading snapshots into collection with malformed name

[mac-chaffee]

Looks good: https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=92d0e7af8fbf49b5b39326c6c7bbdafa

Although to me, it feels like a half-measure, since a slight change to the allowed collection names would bring the vulnerability back (are we 100% certain we'd never approve a future PR that says "allow / in collection names"?). Like trying to solve SQL injection with more sanitization rather than just using prepared statements.

This change is fine for now, but a more thorough fix might be adding validation like this right before doing any filesystem operations:

let snapshot_path = Path::new(&self.storage_config.snapshots_path);
let absolute_snapshot_path = snapshot_path.canonicalize()?;
let absolute_upload_path = upload_path.canonicalize()?;

assert!(absolute_upload_path.starts_with(absolute_snapshot_path))

(this is probably buggy code, just used for illustration purposes)

[timvisee]

Like trying to solve SQL injection with more sanitization rather than just using prepared statements.

There is no such thing as prepared statements in here.

We may be able to solve this with using cap-std internally, but it proved extremely hard to implement in a way that makes sense. It also is not compatible with our S3 interface for snapshots.

[mac-chaffee]

There is no such thing as prepared statements in here.

Just an example of solving the problem at the lowest layer (right before execution) rather than the highest layer (sanitizing inputs).

We are very lucky that we can say "Qdrant should never be operating on paths that are outside of snapshots_path or storage_path". But none of our functions that perform filesystem operations enforce this assumption. In the cloud, we enforce this assumption with readOnlyRootFilesystem.

Adding something like assert!(absolute_upload_path.starts_with(absolute_snapshot_path)) to a few known places seems simpler than chasing down every location where user-provided input is used to construct file paths, for all current and future code :)