Figure out how to do read-only Programmable property access

[nwinter]

In the programming.Programmable Component, the player's code runs within a context that is a copy of part of the World, limited to the properties specified in programmableProperties. So for example, if I'm coding chooseAction() for my Artillery, I might have;

programmableProperties = [
  'pos', 'setTarget', 'setAction', 'getNearestEnemy', 'say', 'attackRange'
];

All this code should work:

var enemy = this.getNearestEnemy();
if(this.pos.distance(enemy.pos) < this.attackRange) {
  this.setTarget(enemy);
  this.setAction('attack');
  this.say('Die, ' + enemy.id + '!');
}

So we need the getNearestEnemy, setTarget, setAction, and say methods to actually interact with the World to get the enemy and set the target and action and such. But this code should not work:

this.world.abort();
// Good--doesn't work, because world isn't in context

this.getNearestEnemy().world.abort();
// Oops, that was the real world, wasn't it?

this.getNearestEnemy().health = -9001;
// Yup, you just killed it!

this.pos.z = 100;
// Good--doesn't work because this is just a copy of this.pos

this.getNearestEnemy().getNearestEnemy().pos.z = 100;
// Aaaaa--<splat>

I was trying to get around this with Object.defineProperty and marking things readonly and such, but I ran into some bugs where it wouldn't work. More testing is needed. Also, more thinking, since I'm a bit confused myself as to how best to do this. I originally thought that making a copy-only context, running the code in that, and them pulling the necessary changes out with a whitelist approach would do it, but then I ran into these problems and made the functions refer to the real functions, and now we have neither full security nor full versatility.

[DavidBruant]

In current browsers, a start would be to isolate the code doing something like:

with(newContext){
  eval(playerCode)
}

(yeah, I know, 3 lines of JS and I use both with and eval :-). This part obviously can't be in strict mode, but can and should be isolated in its own file)

In newContext, you put all the global things you want the code to have access to and not more (wrap your APIs into read-only versions if necessary).
In newContext, you need to set all free variables targeted by the player's code. You can find a superset of this set the same way Caja does.

[nwinter]

Unless I misunderstand, I think we can handle that sort of restricting access to the function context already. Here's an example of how the Programmable Component is currently doing it:

http://pastebin.com/zQjmg67z

Relevant part:

    # ...
    inner = aether.createFunction()
    outer = (args...) ->
      userContext = @createUserContext()
      try
        result = inner.apply userContext, args
      catch error
        @handleProgrammingError error, methodName
        @replaceMethodCode methodName, null  # no-op when this method is called
        result = null
      @updateFromUserContext userContext  # Apply any viable changes they actually made in the context to the real objects
      return result

  # ...

  createUserContext: ->
    ctx = {}
    thang = @
    props = _.union @programmableProperties, @extraProgrammableProperties
    for prop in props
      value = @[prop]
      v2 = value
      if _.isFunction value
        do (prop, v2, thang) ->
          value = =>
            result = v2.apply thang, arguments  # Functions use original thangs
            # But then the context also needs to get updated with any new state that happened as a result of the function call!
            # (this.setTarget(obj) gives thang.target == obj, but not ctx.target == obj)
            for anotherProp in props
              newValue = thang[anotherProp]
              unless _.isFunction newValue
                ctx[anotherProp] = newValue
            result
      # Object.defineProperty doesn't work in web worker in my test, no idea why; works in Chrome console
      #Object.defineProperty ctx, prop, {value: value, writable: false, enumerable: true}
      # It does seem to work in IE 9 and 10 but not 11 and then make it so the property isn't updatable.
      # Or something. Man, that was a terrible bug. An endless ocean of blood was spilt here.
      ctx[prop] = value  # so doing this instbead
    for prop in @userCreatedProperties
      ctx[prop] = @[prop]
    ctx

  updateFromUserContext: (ctx) ->
    for own prop, value of ctx
      if (prop in @programmableProperties) or (prop in @extraProgrammableProperties) then continue
      unless prop in @userCreatedProperties
        if @[prop]? then continue
        @userCreatedProperties.push prop
      if prop in @userCreatedProperties
        @[prop] = value

[nwinter]

Working on a way to do this over in Aether:

https://github.com/codecombat/aether/blob/master/src/protectAPI.coffee
https://github.com/codecombat/aether/blob/master/src/transforms.coffee#L183-L190
https://github.com/codecombat/aether/blob/master/test/protect_api_spec.coffee

It's pretty much the slowest, most brutish thing I could think of that might work: every time user code gets access to an object (through an argument to the method being defined, through the "this" value, or through the return value of some method call), Aether replaces it with a deep clone that has access only to properties listed in the object's "apiProperties" array, but with any methods within bound to the original objects (so that things like setters still work to update the game). Then when user code sends out an object (through the return value or a method call), Aether restores original value from the clone.

Still a long way to go on this, but maybe it will work.

[nwinter]

Despite the horrifying complexity of my approach, I think I've pretty much got it. Let the hacking attempts commence. Anyone who can still modify the supposedly read-only properties of other Thangs from within CodeCombat spells wins my grudging admiration!