endojs / endo

Endo is a distributed secure JavaScript sandbox, based on SES
Apache License 2.0
829 stars 72 forks source link

M.call pattern lets extra arguments through #1757

Closed mhofman closed 10 months ago

mhofman commented 1 year ago

Describe the bug

M.call() does not check arguments not declared in the interface and lets them through.

Steps to reproduce

const ExtraArgsI = M.interface('ExtraArgs', {
  method: M.call(M.string(), M.any()).returns(),
});

test.failing('extra args', t => {
  const makeExtraArgs = defineExoClass('ExtraArgs', ExtraArgsI, () => {}, {
    method(channelId, obj, foo) {
      // If doesn't throw should be undefined
      t.is(foo, undefined);
    },
  });

  const extraArgs = makeExtraArgs();

  // Maybe throw?
  t.throws(() => extraArgs.method('foo', {}, 42));
});

Expected behavior

I believe the intent was for undeclared arguments to cause a rejection of the call. At the very least the argument should not be passed through and the implementation should only see undefined. This latter behavior may make some upgrade path easier in the case of optional new arguments being added (the question remains of what the caller would expect the behavior to be if it provides the optional argument, but the receiving version cannot process it).

kriskowal commented 10 months ago

@erights @mhofman I’m closing this since my latest integration with Agoric SDK provided ample evidence that this has been addressed. Extra arguments now cause an exception.