Re: [Monotone-devel] [Patch] mtn automate lua

[Stephen Leake]

Thomas Keller <address@hidden> writes:

> It took me some time (and I've learned quite a lot about lua and its C
> interface), but especially with the help of the lua community I was able
> to build a general "automate lua" command which lets you call lua hooks
> over automate just as monotone would call them!

This sounds very useful, although I can't think of an immediate use
in my work.

> So I'd like to include this in mainline (tests and documentation will be
> added, of course) if other people find this useful as well and are ok
> with the implementation.

The code looks simple and straight-forward (except see one comment
below). Although I can appreciate that it took a while to get it
working!

> I'm still a bit undecided when it comes to security constraints, like
> should a couple of hooks be disabled (i.e. should I maintain a list of
> "supported" hooks) or should I just leave this out into the wild as is?

As I understand the current security model, there are two layers. If
there is a network connection, that connection has its own
authentication/validation method, which we can assume works. Then
there is a key required for write access to the database.

> However, this might be a completly different cup of tea if we open / use
> the automation interface for networking - of course we do not want to
> allow anybody even with write access to a database being able to
> retrieve any kind of the local password. 

That's a good point. In terms of the two layers above, this adds a
third layer, requiring protection of "local data". Not really another
layer, just covers different data.

> Maybe we then need to introduce some kind of marking for each
> automate command, like "secure_for_local_use" vs.
> "secure_for_foreign_use" and check if automate stdio is connected to
> a socket? 

I think that makes sense. Although I'm not sure it's easy to tell what
automate stdio is connected to. We could probably set a global state
variable in the top level main procedure, where it is clear what stdio
is connected to.


> +function hook_wrapper(func_name, ...)
> +    -- evaluate each single string argument to resolve types
> +    -- like nil's, table's and others - the select('#', ...) syntax is
> +    -- borrowed from http://lua-users.org/wiki/StoringNilsInTables to
> +    -- let this code properly work for nil arguments as well
> +    local args = {n=select('#',...), ...}
> +    for i=2,args.n do
> +        args[i] = assert(loadstring("return " .. args[i]))()

It might be good to specify a more informative message in `assert' here.
I think this is where syntax and "not defined" problems in the
arguments will be detected, and a generic "assertion failed" will not
be much help. Printing out the actual argument that caused the failure
might be enough.

In the tests, include some that have erroneous arguments, to show what
kind of error messages you get.

-- 
-- Stephe