Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ability to save definitions typed at the REPL to a .sw file #7

Open
Tracked by #1571
byorgey opened this issue Sep 20, 2021 · 12 comments
Open
Tracked by #1571

Ability to save definitions typed at the REPL to a .sw file #7

byorgey opened this issue Sep 20, 2021 · 12 comments
Labels
C-Moderate Effort Should take a moderate amount of time to address. S-Moderate The fix or feature would substantially improve user experience. Z-Feature A new feature to be added to the game. Z-User Experience This issue seeks to make the game more enjoyable to play.

Comments

@byorgey
Copy link
Member

byorgey commented Sep 20, 2021

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,

> def x = 3 end
> x + 2
5
> def m2 = move; move end
> :save myDefs.sw

might create a file myDefs.sw which contains the code defining x and m2. 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 the base robot, and pretty-print the definitions it contains (possibly adding some type annotations from the robotCtx). It may be necessary to add something to keep track of the order of the definitions (something the robotEnv does not care about) so that we can pretty-print them to a file in the same order the user entered them.

@byorgey byorgey added Z-User Experience This issue seeks to make the game more enjoyable to play. Z-Feature A new feature to be added to the game. C-Low Hanging Fruit Ideal issue for new contributors. S-Critical This is an issue that seriously affects playability or user experience. labels Sep 20, 2021
@byorgey byorgey added C-Moderate Effort Should take a moderate amount of time to address. and removed C-Low Hanging Fruit Ideal issue for new contributors. labels Sep 28, 2021
This was referenced Oct 3, 2021
@xsebek xsebek self-assigned this Oct 3, 2021
@byorgey
Copy link
Member Author

byorgey commented Feb 24, 2022

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 .

@TristanCacqueray
Copy link
Collaborator

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.

@byorgey
Copy link
Member Author

byorgey commented Feb 25, 2022

Hah, yes, good point.

@byorgey
Copy link
Member Author

byorgey commented May 7, 2022

Other related issues illustrating problems with the run command: #208 , #328 . And a proposal for making run work better: #495

@xsebek
Copy link
Member

xsebek commented Jul 29, 2022

@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. 😕

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. 🙂

@TristanCacqueray
Copy link
Collaborator

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.

@xsebek
Copy link
Member

xsebek commented Jul 30, 2022

@TristanCacqueray I think robots can only use let though. So far I predefined everything in base so I could reuse it for multiple robots.

@byorgey
Copy link
Member Author

byorgey commented Jul 30, 2022

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.

@byorgey byorgey changed the title Saving/loading definitions Saving definitions Oct 31, 2022
@byorgey byorgey added S-Moderate The fix or feature would substantially improve user experience. C-Low Hanging Fruit Ideal issue for new contributors. and removed S-Critical This is an issue that seriously affects playability or user experience. C-Moderate Effort Should take a moderate amount of time to address. labels Oct 31, 2022
@byorgey byorgey added S-Nice to have The bug fix or feature would be nice but doesn't currently have much negative impact. C-Moderate Effort Should take a moderate amount of time to address. and removed S-Moderate The fix or feature would substantially improve user experience. C-Low Hanging Fruit Ideal issue for new contributors. labels Oct 31, 2022
@byorgey
Copy link
Member Author

byorgey commented Oct 31, 2022

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.

@xsebek
Copy link
Member

xsebek commented Nov 1, 2022

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.

@byorgey
Copy link
Member Author

byorgey commented Nov 1, 2022

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 or inl ()) 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.

@byorgey byorgey changed the title Saving definitions Ability to save definitions typed at the REPL to a .sw file May 31, 2023
@byorgey byorgey added S-Moderate The fix or feature would substantially improve user experience. and removed S-Nice to have The bug fix or feature would be nice but doesn't currently have much negative impact. labels May 30, 2024
@byorgey
Copy link
Member Author

byorgey commented May 30, 2024

Just to summarize the current state of this issue:

  • Saving definitions should be triggered by something in the UI, like a key shortcut or perhaps some kind of menu.
  • Our pretty-printing is relatively decent now, so that should be fine.
  • The main thing we will need to change is to store the original 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-Moderate Effort Should take a moderate amount of time to address. S-Moderate The fix or feature would substantially improve user experience. Z-Feature A new feature to be added to the game. Z-User Experience This issue seeks to make the game more enjoyable to play.
Projects
No open projects
Status: Deferred
Development

No branches or pull requests

3 participants