Open byorgey opened 3 years ago
I actually don't really like the idea of special REPL commands like :load
and :save
--- so far we've managed to steer clear of those, so that the only thing you can type at the REPL is a swarm expression.
Hmm, so why not just add a savedefs
function, with type string -> cmd ()
? i.e. you can type savedefs("mydefs.sw")
to save. It might be a bit annoying to type that every time, but (1) I think the idea is to move players towards editing their code in an editor rather than at the command line, and (2) this will be better once we add #85 .
It seems like the savedefs
function could be used by a robot to escape the game by altering the host file :) Perhaps the output should be restricted to a secure temporary location, e.g. ~/.swarm/savedefs/$date.sw
, and the path could be returned by the function, e.g. savedefs: cmd string
instead.
Hah, yes, good point.
Other related issues illustrating problems with the run
command: #208 , #328 . And a proposal for making run
work better: #495
@TristanCacqueray I agree that it should not be possible for robots to escape the game with saving definitions, but that savedefs
function does not seem very user-friendly to me. :confused:
Why not restrict the functionality to the base robot and have a UI list of current definition saves with an extra item to create a new file? Something like this:
┌────────────┬──────────────────────────────────────────────────┐
│ │ │
│ │ │
│ │ │
│ │ │
│ │ ┌──────────SAVE DEFINITIONS───────┐ │
│ │ │ │ │
│ │ │►CREATE NEW DEFINITIONS SAVE │ │
│ │ │ │ │
│ │ ├─ANOTHER FILE │ │
│ │ │ │ │
│ │ ├─MY PRELUDE │ │
│ │ │ │ │
├────────────┤ ├─RELUDE │ │
│ │ │ │ │
│ │ ├─LIST │ │
│ │ │ │ │
│ │ └─────────────────────────────────┘ │
│ │ │
│ │ │
│ ├──────────────────────────────────────────────────┤
│ │ │
│ │> def h = "Hello World!" end; h │
│ │h : string │
│ │> │
└────────────┴──────────────────────────────────────────────────┘
We could tab complete import "re
which would make finding the definition file name easy enough.
If we wanted to be fancy, we could also have dialog to select which definitions in scope (and their transitive dependencies) to export. :slightly_smiling_face:
I meant it might be interesting to support loading arbitrary robot, and in that situation, it would be nice to ensure a definition can't escape the game.
Returning a random file location may not be very user friendly, perhaps the filename could be static, e.g. scratch.sw
. Though I think you are right @xsebek , and this function should be restricted, ideally to the repl.
@TristanCacqueray I think robots can only use let
though. So far I predefined everything in base so I could reuse it for multiple robots.
Eventually I would like to remove the restrictions on def
so that it can be used anywhere. But right now, yes, the base is the only robot that can execute def
.
It seems like the
savedefs
function could be used by a robot to escape the game by altering the host file
Reading this again, I think I no longer understand exactly what the threat is here. A robot might use the savedefs
function to overwrite the swarm executable? Or overwrite one of the .yaml
files describing a scenario? I can't think of any way that wouldn't just lead to syntax errors, and honestly I don't think I particularly want to be concerned with this kind of security. Players can already alter/overwrite files on their own computer anyway, so who cares if they can do it via a robot?
Loading definitions is discussed a lot elsewhere (e.g. #495) so I am changing this issue to focus just on saving definitions. I'm downgrading this to S-Nice to have
(because users can just edit all their functions in an external editor from the get go, it's not really that important if they can save definitions they make at the REPL). I don't know how hard it will be to implement savedefs
. In theory, all we have to do is pretty-print all the current definitions to a file. But (1) the pretty-printer might need to be improved, and (2) the definitions might be stored in a way that makes it hard/impossible to reproduce valid surface Swarm code.
I tend to agree with @TristanCacqueray - if I give you a Swarm code that does something cool, you will probably not expect a game to write to any location on the host system the current user has access to. 🤯
If we were to add savedefs
, I think it would warrant a warning popup. Better to be a bit annoying than to cause a security breach.
Also, I think it would be bad design to let you easily bypass fun challenges like wireless communication by simply writing to a file and then reading from it in another robot. You can do that outside the game with the Web API if you want, but building such a thing in the game as cmd ()
just seems unnecessary. 🤷
@byorgey if we limit the functionality to base (either in UI or :save
in REPL) are there any cases that could not be printed in valid swarm code? I can only think about cases that you might not have the capability to execute (like robotNumbered
or inl ()
) but there should not be unrepresentable stuff in top-level definitions.
Ohhh, I finally understand the specific threat model we're talking about. I honestly didn't understand it before. To make sure I really understand it, and for the benefit of anyone else reading this later, the scenario goes something like this: Eve crafts a malicious .sw
file and sends it to Alice. It looks like it does something cool so Alice loads it into Swarm. Unfortunately it also contains an instruction like savedefs("/home/alice/important-file.txt")
which creates or overwrites some file on Alice's system with Alice's permissions.
I agree we should not allow this. How about just adding a UI command (Ctrl+S?) that pops up a dialog prompting you for a filename in which to save?
are there any cases that could not be printed in valid swarm code? I can only think about cases that you might not have the capability to execute (like
robotNumbered
orinl ()
) but there should not be unrepresentable stuff in top-level definitions.
I don't think so, but we will probably have to save the original AST for function definitions along with their values, just so we can pretty-print them later. Once we evaluate a definition to a value it may end up with unrepresentable stuff that would be hard to reverse-engineer back into valid top-level swarm code.
Just to summarize the current state of this issue:
Term
for each definition (instead of just storing its Value
), so we have something to pretty-print.This would still definitely be nice --- when attempting a new challenge scenario, I often start by playing around at the REPL, and then later create a solution in a .sw
file. Being able to spit out all the definitions I wrote at the REPL into a .sw
file would be a fantastic starting point for crafting a nice solution. In fact I'm upgrading this to S-Moderate
.
I think we ought to get rid of the
run
command and just make some kind of REPL commands for saving and loading the current set of definitions. For example,might create a file
myDefs.sw
which contains the code definingx
andm2
. Similarly:load myDefs.sw
might load the definitions (replacing any currently in scope --- perhaps with some kind of warning and opportunity to cancel).This seems rather critical to actual gameplay since people will develop various useful definitions and want to edit them, reuse them in later sessions, and so on.
I don't think this should be too hard once #8 (and any other pretty printing bugs) is addressed. To save, just get the
robotEnv
from thebase
robot, and pretty-print the definitions it contains (possibly adding some type annotations from therobotCtx
). It may be necessary to add something to keep track of the order of the definitions (something therobotEnv
does not care about) so that we can pretty-print them to a file in the same order the user entered them.