Fill out more scheme builtins #4355
Merged
Conversation
This file contains 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
- `Value.validateSandboxed` checks an interchange value for sandboxing before it is decoded. It either returns a list of dependencies the runtime doesn't know about yet, or a list of disallowed builtins, both as lists of termlinks - `sandboxLinks` returns the list of sandboxed builtins transitively referenced by the code associated to a given termlink
- Creates a map storing associations of termlinks with sensitive ops
- Procedures for dynamically expanding the map, and declaring
associations statically
- Structural termlink equality and hashing has been added to the struct,
because otherwise looking up associated links was failing.
- Declarations for all the sensitive builtins have been included
statically in the sandbox implementation file
* Generated code will use a direct declaration of all builtins
transitively used, since we can look that up at code generation time
- Both sandbox validation functions have been provided, but the one for
arbitrary values is incomplete.
- Sandboxing has been added to the jit test suite.
Lists vs. chunked lists, byte strings vs. typelinks, etc.
- Value.validateSandboxed wasn't being aliased there - The sandbox check in the interpreter wasn't handling builtin dependencies correctly
pchiusano
approved these changes
Oct 24, 2023
| pure (denv, ustk, bstk, k) | ||
| exec !env !denv !_activeThreads !ustk !bstk !k _ (BPrim2 SDBX i j) | ||
| | sandboxed env = | ||
| die "attempted to use sandboxed operation: sandboxLinks" |
There was a problem hiding this comment.
Should the error message say "sandboxLinks" here or something else?
There was a problem hiding this comment.
The error message is correct, but I'd put it in the wrong case there. It was meant for SDBL.
| (sandbox-builtin "IO.array") | ||
| (sandbox-builtin "IO.arrayOf") | ||
| (sandbox-builtin "IO.bytearray") | ||
| (sandbox-builtin "IO.bytearrayOf") |
There was a problem hiding this comment.
I went ahead and double checked these to make sure they're consistent with what's in Builtin.hs.
Contains a serialized, mutually recursive function that won't be recognized as unsafe unless the sandboxing information is expanded correctly.
It was failing to check the case where we already have all dependencies, which was causing a crash when generating the module name, since we use the termlinks to influence the name.
Was wrapping in value too many times, which makes things safe.
CACH wasn't actually adding to the sandbox, and the sandbox expansion algorithm wasn't correctly handling mutually recursive functions.
Add some cases that actually fail if statically generated sandboxing information isn't produced.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This PR includes various builtin additions, mainly for scheme, the most significant of which are sandboxing related.
Value.dependenciesandCode.dependencieswill be included, as they are auto-generated from unison, and will appear when the share version is incrementedCode.isMissinghas been implemented in schemesandboxLinkslooks up the list of sensitive builtins associated with a termlink. This is used when statically generating code, to just look up the sandboxing information in the current runtime and add it to the file.Value.validateSandboxedis a revised version of the sandboxing check. It allows checking the sandboxing status of aValuebefore it is loaded into an arbitrary unison value. This is a bit easier to check on the scheme side. It also has a different API, which can report all the builtins that caused the value to fail the sandbox check, rather than just a boolean.Some tests are included, but I'm going to add a bit more, as I think there might be at least one corner case I missed on the scheme end.