purescript-contrib / purescript-pathy

A type-safe abstraction for platform-independent file system paths.
Apache License 2.0
33 stars 17 forks source link

`sandbox` should not coerce to `Rel` #26

Closed natefaubion closed 6 years ago

natefaubion commented 7 years ago

Right now, sandbox is merely flip relativeTo, which turns any path into Rel. Shouldn't it return whatever the original path was, merely flipping Unsandboxed to Sandboxed?

jdegoes commented 7 years ago

It should probably turn to Abs, too, shouldn't it?

natefaubion commented 7 years ago

It should probably turn to Abs, too, shouldn't it?

I don't know. Since you can have a sandboxed relative path, then I think it should just flip it from Unsandboxed to Sandboxed. I see sandbox as a kind of validation. I don't think it should change anything about the representation of the path.

jdegoes commented 7 years ago

@natefaubion OK, I'd buy that, it's just if it returns Rel, a user could append it to another path, and then end up outside of the directory they sandboxed it to (which is strange, if possibly not a real problem).

natefaubion commented 7 years ago

Yeah, I find that a little confusing, but then I don't understand why we have Sandboxed Rel paths at all.

jdegoes commented 7 years ago

Well it's hard to eliminate combinations of type parameters when using phantom types...

natefaubion commented 7 years ago

If sandboxed and relative is not a real thing, then I'm fine with it turning the path into Abs.

jdegoes commented 7 years ago

👍

garyb commented 6 years ago

We're doing some updates on this just now, and the question of what to do about this issue has come up again.

One of the changes so far has been to use explicit kinds for the parameters:

foreign import kind FileOrDir
foreign import data File :: FileOrDir
foreign import data Dir :: FileOrDir

foreign import kind RelOrAbs
foreign import data Rel :: RelOrAbs
foreign import data Abs :: RelOrAbs

foreign import kind SandboxedOrNot
foreign import data Sandboxed :: SandboxedOrNot
foreign import data Unsandboxed :: SandboxedOrNot

With this now, we can make a slight tweak to make the Rel+Sandboxed combination impossible, by parameterising Abs instead:

foreign import data Abs :: SandboxedOrNot -> RelOrAbs

This means we can drop the s parameter everywhere aside from functions that deal with Abs explicitly... and with functional dependencies we can do some calculation to ensure things work out. For instance, appendPath should unsandbox any abs path it seems, since the rel part being appended to it could "go up":

appendPath :: forall a a' b. AppendOutcome a a' => Path a Dir -> Path Rel b -> Path a' b

class AppendOutcome (i :: RelOrAbs) (o :: RelOrAbs) | i -> o
instance relRelAppendOutcome :: AppendOutcome Rel Rel
instance absRelAppendOutcome :: AppendOutcome (Abs s) (Abs Unsandboxed)

So now if you have a myAbsDir </> file "foo" it will even infer the type as Path (Abs Unsandboxed) File. Although if you try and coerce it the error will be about a missing instance rather than a direct type error, but it's still pretty neat.

@jdegoes The only question I have now, is... assuming the goal of sandboxing is to ensure path values in an app do not reach "above" some set boundary, it seems like the peel and parentDir functions should also return unsandboxed results. Is that right, or am I misrepresenting the goal of sandboxing?

garyb commented 6 years ago

We could possibly take this a bit further and track cases where we can determine a relative path only goes down too, as appending down-only-rel paths to abs-sandboxed could give us an abs-sandboxed result too then I guess.

That could be peano-style, so then we could even support some going-up without losing that state entirely, but it's probably a bit heavy-weight, and I imagine the more common case is dealing with paths at runtime where we'd only be able to make the vague "it's not going up" guarantee anyway, not the actual depth.

garyb commented 6 years ago

Given what I said above, it seems there is actually a valid "Rel+Sandboxed" meaning: it's relative paths that are known to only descend further.