Give the controller the contract it can fill (hq ADR 0325, issue 567)

The controller cannot know the operator's account's uid, names an access by its
declared id (which holds a slash), and should not parse words. A tool runner's
account may now come by name alone and is read here (refused when absent or root's,
its uid said in the access's listing); the take act matches the declared id exactly;
an access wait says its reason (keep or disk) and, for a filesystem that refuses
ACLs, whether the take act can enable them.
This commit is contained in:
2026-10-12 19:51:09 +02:00
parent ef26dba275
commit aad33dfe5c
12 changed files with 354 additions and 21 deletions
+24 -5
View File
@@ -47,11 +47,14 @@ func TestTheTakeActRunsOnlyOnAWarrant(t *testing.T) {
Nonce: nonce}
}
for why, body := range map[string][]byte{
"no warrant": signedAct(t, key, ask(aNonce+"01"), AccessActAsk{Act: ActTake, Access: "radarr.movies"}),
"another act": signedAct(t, key, ask(aNonce+"02"), AccessActAsk{Act: "share", Access: "radarr.movies", Warrant: "w"}),
"no access": signedAct(t, key, ask(aNonce+"03"), AccessActAsk{Act: ActTake, Warrant: "w"}),
"a hand-over's": signedAsk(t, key, ask(aNonce+"04")),
"a path as access": signedAct(t, key, ask(aNonce+"05"), AccessActAsk{Act: ActTake, Access: "/srv", Warrant: "w"}),
"no warrant": signedAct(t, key, ask(aNonce+"01"), AccessActAsk{Act: ActTake, Access: "radarr.movies"}),
"another act": signedAct(t, key, ask(aNonce+"02"), AccessActAsk{Act: "share", Access: "radarr.movies", Warrant: "w"}),
"no access": signedAct(t, key, ask(aNonce+"03"), AccessActAsk{Act: ActTake, Warrant: "w"}),
"a hand-over's": signedAsk(t, key, ask(aNonce+"04")),
"a slash in the warrant": signedAct(t, key, ask(aNonce+"05"), AccessActAsk{Act: ActTake, Access: "radarr.movies",
Warrant: "w/1"}),
"two lines as access": signedAct(t, key, ask(aNonce+"07"), AccessActAsk{Act: ActTake, Access: "radarr.\nmovies",
Warrant: "w"}),
} {
if a := AnswerAccessAct(check, handle, body); a.Refused == "" || !strings.HasSuffix(a.Refused, "nothing was changed") {
t.Errorf("%s: not refused, or refused without saying nothing was changed: %+v", why, a)
@@ -66,3 +69,19 @@ func TestTheTakeActRunsOnlyOnAWarrant(t *testing.T) {
t.Fatalf("a warranted ask did not reach the act whole: %+v %+v", a, given)
}
}
// The controller names an access by its declared resource id exactly, which may hold a `/`: it reaches the act,
// which matches it against what was declared.
func TestATakeActNamesItsAccessByItsDeclaredId(t *testing.T) {
m, key := aMember(t)
var given []AccessAct
handle := func(a AccessAct) (string, error) { given = append(given, a); return "started", nil }
check := HandOverCheck{Signer: m.Signer, Node: m.Node, Nonces: &Nonces{}}
ask := HandOverAsk{Node: m.Node, Path: "/services/media/movies", By: "the operator",
Expires: time.Now().Add(HandOverGood), Nonce: aNonce + "08"}
id := "radarr.access-services/media/movies"
a := AnswerAccessAct(check, handle, signedAct(t, key, ask, AccessActAsk{Act: ActTake, Access: id, Warrant: "w-1"}))
if a.Refused != "" || len(given) != 1 || given[0].Access != id {
t.Fatalf("an access id with a slash did not reach the act as declared: %+v %+v", a, given)
}
}
+10 -5
View File
@@ -355,7 +355,8 @@ const (
)
// AccessActAsk is an act's own fields, beside a hand-over's (node, path, by, expires, nonce): which act, the
// access by its resource id, and the warrant the operator approved.
// access by its declared resource id exactly as the declaration names it (`/` allowed), and the warrant the
// operator approved.
type AccessActAsk struct {
Act string `json:"act"`
Access string `json:"access"`
@@ -400,10 +401,14 @@ func AnswerAccessAct(check HandOverCheck, handle AccessActFunc, body []byte) Han
case act.Act != ActTake:
return refusedAct(fmt.Sprintf("the ask names the act %q, and the one act asked is %q", act.Act, ActTake))
}
for _, v := range []string{act.Access, act.Warrant} {
if len(v) > MaxActField || strings.ContainsAny(v, "\r\n\x00/") {
return refusedAct("the ask's access or warrant is not one short name")
}
// The access is the declared resource's id exactly as the declaration names it — which may hold a `/`
// (radarr.access-services/media/movies) — and the engine matches it against what it was declared, refusing
// anything else; the warrant is one short name.
if len(act.Access) > MaxActField || strings.ContainsAny(act.Access, "\r\n\x00") {
return refusedAct("the ask's access is not one short line")
}
if len(act.Warrant) > MaxActField || strings.ContainsAny(act.Warrant, "\r\n\x00/") {
return refusedAct("the ask's warrant is not one short name")
}
said, err := handle(AccessAct{Act: act.Act, Access: act.Access, Path: ask.Path, Warrant: act.Warrant, By: ask.By})
if err != nil {
+15
View File
@@ -453,8 +453,23 @@ type AccessWait struct {
// Placed names, with no path, the filesystem the folder is on now, on a wait for the operator's word: a warrant
// given for another filesystem (the folder moved) does not answer it, and the controller asks again.
Placed string `json:"placed,omitempty"`
// Reason is what the wait is for, for the controller to read without parsing words: WaitKeep (the operator's
// word to keep it) or WaitDisk (its disk mounted).
Reason string `json:"reason"`
// ACL is said on a keep wait whose folder's filesystem refuses access entries now: ACLNeedsEnabling when the
// take act will let it (a ZFS dataset's acltype, which the ask then says it sets), ACLUnsupported when no act
// can (the operator mounts it with ACLs). Absent when it allows them.
ACL string `json:"acl,omitempty"`
}
// The reasons of an access wait, and what its filesystem says of ACLs.
const (
WaitKeep = "keep"
WaitDisk = "disk"
ACLNeedsEnabling = "needs-enabling"
ACLUnsupported = "unsupported"
)
// KeptEntry is one user of a kept folder as its listing says it.
type KeptEntry struct {
Identity string `json:"identity"`