search

codefrau / SqueakJS

A Squeak Smalltalk VM in Javascript
https://squeak.js.org
MIT License
364 stars 75 forks source link

SecurityPlugin: Add primitiveGetUntrustedUserDirectory #131

Closed LinqLover closed 4 months ago

LinqLover commented 2 years ago

This change allows it, for instance, to enable the preference #startInUntrustedDirectory in Squeak.

Please explicitly review whether I have rebuilt the bundles correctly. To me, it looks as if the change in the SoundGenerationPlugin has been forgotten to be built via e3673571294da2cf40adf5265ae0106f087cca65.

LinqLover commented 2 years ago

With https://github.com/squeak-smalltalk/squeak-filesystem/pull/4, also Squeak-FileSystem works in SqueakJS. 🎉

(BTW, it was very nice to get this project running directly from gitpod.io :D)

LinqLover commented 5 months ago

Hi @codefrau, could this be merged? :)

codefrau commented 5 months ago

Hi – yes, something like this could be merged, and I'm sorry for not replying sooner.

I do have a few issues with this PR:

  1. pull requests should not modify the dist/ dir – I will update the bundles when I make a new release. Apps that need unreleased features should use squeak.js directly (which will import everything else in turn) or build their own bundle.
  2. SecurityManager default untrustedUserDirectory already answers '/SqueakJS' – why do we need this?
  3. If we decide to add more SecurityPlugin functions I'd add at the very least both untrustedUserDirectory and trustedUserDirectory prims, to answer distinct directories. trustedUserDirectory must never be a subdirectory of (or the same as) untrustedUserDirectory.
  4. Preferably we'd actually implement SecurityPlugin properly, enforcing file/socket/image write checks.
LinqLover commented 4 months ago

@codefrau Sorry again for the long delay!

  • SecurityManager default untrustedUserDirectory already answers '/SqueakJS' – why do we need this?

This comes from the fallback path in SecurityManager default untrustedUserDirectory, which falls back to FileDirectory default pathName. FileSystem, however, invokes the primitive manually. But I don't know whether it has to. I would also be open to patching FileSystem to use the SecurityManager class instead. This might be simpler than providing a proper implementation of the SecurityPlugin. What do you prefer? Right now I don't have the capacities (nor a personal motivation ^^) to implement a more sophisticated interface in SqueakJS's SecurityPlugin. I just thought it was a low hanging fruit to complete this PR ...

codefrau commented 4 months ago

I merged without your dist changes. It doesn't hurt to have another fake prim, after all.