Clay permissions

[Fang-]

As discussed internally, permissions for the filesystem.

Permissions are set for all revisions of a node. If a node doesn't have permissions set, those of the parent node are used. If no permissions are set all the way up to root, we default to fully private.

The clay interface sees some additions to support basic permissions management.

++task:

• {$cred our/ship nom/@ta cew/crew} for setting permission groups.
• {$crew our/ship} for getting permission groups.
• {$perm our/ship des/desk pax/path rit/rite} for setting permissions for a node.
• A %p ++care for reading permissions set for a node on a local desk.

++gift:

• {$cruz cez/(map @ta crew)}, the response to %crew requests.
• {$rule red/dict wit/dict}, the response to %p file requests.

And a notable change that was required for making permission checks actually functional:
The interface for %warp file request now includes a ship which has to be set to the requesting ship.
When doing a %warp for local data, you just set it to our, as you do for many other tasks. When doing a %west for foreign data, the target ship automatically turns it into a %warp with the original sender specified.
Read requests from foreign ships come in as %wests. They now get re-sent to the local clay as %werps, which get dealt with as %warps that require permission checks. No checks are done for write requests, because those only originate locally right now.

Eventually, the above will be more elegantly solved from within arvo itself, passing vanes a pair of recipients and responsible ships for each incoming event. Not to mention making a global our available once the new boot sequence lands. Changing this to make use of that shouldn't be difficult.

Aside, having to take original requester into account when deduping requests likely completely obliterates any use that still saw. We're fine to keep it in, but at this point it might be costing more performance than it'll ever gain us.

Need to do more thorough testing and implement a state adapter, but it boots (off an updated pill at least) and seems to work as advertised so far, so I'd say that's good enough for review/discussion.

[Fang-]

Changing the %warp interface was giving problems, new-style %warp events were being sent before clay itself got properly updates to handle them.
Settled on adding %werp for incoming external read requests and leaving %warp unchanged. This makes upgrading easier, doesn't break compatibility, and keeps the internal file request interface clean.

[belisarius222]

Looks structurally sound, and I think it'll work. Some stylistic comments and a few questions before I'll sign off on it.

[belisarius222]

I don't see this in the state anywhere.

[Fang-]

Lingering trace of experimentation. Will delete.

[belisarius222]

Do we not want to default everything to private, i.e. a whitelist with no elements?

[Fang-]

These are the defaults for local copies of foreign desks. Those are not accessible by outsiders over the network. In a previous iteration of the PR setting these to be readable was required.
That said, the current version always says "no permission checks" for all internal requests, so there's no need to set permissions here explicitly anymore.

[belisarius222]

This looks like an unused argument.

[Fang-]

It should be used, since this is how %many requests get their data. Going to add a permission check here.

[belisarius222]

This seems likely to be a duplicate of the usual %y / %z logic. If so it might be good to make a little helper function.

[Fang-]

It's not an exact match. %y wants just the path, %z wants lobes too, and they both want only the path tails, where the permission check just wants the full paths.
The logic it still fairly transferable though, the data structures here are easy enough to work with. Having one function for "get child nodes with tail paths" seems to give a net gain to simplicity.

[Fang-]

Actually, I'm having trouble finding a helper function here that isn't in some way terrible to use for two out of three cases. Problem is that if we do a dir listing for /one with the following data stored, then we still want to see ./two show up, but we can't provide a lobe alongside that, unlike for ./three

/one/two/x
/one/two/y
/one/three

[belisarius222]

Ok, no need to force it if it doesn't feel right.

[belisarius222]

Could (~(get by cez) p.w) ever fail? I feel like that should be a ~(got by ...)

[Fang-]

It totally can. Either I added a group which I haven't defined yet, or I deleted a group and there's still references to it lingering.

[belisarius222]

q.hic could really use an alias. Comparing p.q.hic and p.q.q.hic a little later in this function is cruel and unusual punishment.

[Fang-]

You're right, even just aliasing q.hic to req makes a world of difference.
I also made a commit that adds actual faces to the ++task. Not sure if that's boyscouting a bit too aggressively.

[belisarius222]

What's weird about it?

[Fang-]

Nothing but the fact that I left a debug statement in it. Cleaning up.

[belisarius222]

There's no way I can reasonably review u.q.r.q.hic. I realize this was already the style in this part of the code, but it's not fit for human consumption.

[Fang-]

This particular comment can probably just get removed, but I agree that I should probably add a few =* here.

[belisarius222]

I think this would be cleaner as a |^ since it has so many internal functions.

[Fang-]

Fair, on it!

[belisarius222]

Can you explain why this change was necessary?

[Fang-]

Since all local desks are fully private by default, we need to make our %kids desk public explicitly if we want people to be able to sync with it.

[belisarius222]

I think a comment here would be helpful.

[belisarius222]

Looks like this could be a ++skim instead of a ++murn.

[belisarius222]

Looks good to me. How's the testing coming?

[Fang-]

As you might deduce from the commits, testing pointed to a few issues that needed solving. Everything now works as expected.
Both upgrading from livenet, booting using old pill and booting using new pill all work fine and don't break syncs.

For the record, and for the future, here are the beginnings of a clay testing lib. Should work nearly identically for other vanes. Also see this bigger one for jael.
Didn't do much actual testing with this because the testing framework wasn't playing well with it and I just wanted to get this PR finished.

Included is a pair of generators for setting the readability of desks (or "directories" therein). |public and |private %desk /optional/path.

Unless there's anything other than the %kids desk that needs to remain public (web.plan maybe?), I'll call this fine to be merged. @belisarius222, want to take a final look at this?

[belisarius222]

Re-reviewed. Looks good to me.

[Fang-]

I lied about the booting btw (thanks, last-minute additions), kiln does complain about not finding one of the new clay types if you don't boot with a new-clay pill. So we'll want to make sure to push an up-to-date latest.pill when this goes onto the network.

[belisarius222]

Does that mean this needs to be breached in?

[Fang-]

It doesn't need to be breached in, but I just double-checked, and the kiln changes did break the updating. Particularly, it says it can't find ++rite, a new clay type.

We could do like a partial update that doesn't include the offending kiln changes, and then push out the second half separately.
I think the slightly better solution is to do what we did when we saw similar behavior with ++riot (also used by kiln), and just include the relevant structures in kiln directly. We can then (after this has been pushed to live) remove the structure definitions from kiln for cleanliness. That won't have to be pushed out immediately. Sound good?

(I guess ultimately this is an issue of kiln not getting recompiled alongside a zuse change...)