From f8c6b8c63ac8a7a216699d7f89466dec6568b156 Mon Sep 17 00:00:00 2001 From: Lewis Date: Wed, 10 Jun 2026 14:02:49 +0300 Subject: [PATCH] appview: stop pds-record acl fallback if knot cap probe fails Lewis: May this revision serve well! --- appview/knotcompat/version.go | 51 ++++++++++++++++------- appview/knotcompat/version_test.go | 67 ++++++++++++++++++++++-------- appview/knots/knots.go | 16 ++++++- appview/repo/repo.go | 14 ++++++- 4 files changed, 111 insertions(+), 37 deletions(-) diff --git a/appview/knotcompat/version.go b/appview/knotcompat/version.go index 55533808..f05629a3 100644 --- a/appview/knotcompat/version.go +++ b/appview/knotcompat/version.go @@ -82,6 +82,19 @@ func (c *versionProbeCache) supports(now time.Time, host string, minMajor, minMi var probeCache = &versionProbeCache{entries: map[string]versionProbeEntry{}} +type CapStatus int + +const ( + CapUnknown CapStatus = iota + CapAbsent + CapPresent +) + +type negEntry struct { + status CapStatus + until time.Time +} + type NativeLatch interface { IsNative(host string) bool MarkNative(host string) @@ -117,29 +130,30 @@ func (g *nativeGate) currentLatch() NativeLatch { return nil } -func (g *nativeGate) isNative(host string, probe func() bool) bool { +func (g *nativeGate) status(host string, probe func() CapStatus) CapStatus { if _, ok := g.memo.Load(host); ok { - return true + return CapPresent } - if until, ok := g.negMemo.Load(host); ok { - if g.clock().Before(until.(time.Time)) { - return false + if v, ok := g.negMemo.Load(host); ok { + e := v.(negEntry) + if g.clock().Before(e.until) { + return e.status } g.negMemo.Delete(host) } if l := g.currentLatch(); l != nil && l.IsNative(host) { g.memo.Store(host, struct{}{}) - return true + return CapPresent } - if !probe() { - g.negMemo.Store(host, g.clock().Add(versionProbeFresh)) - return false + if s := probe(); s != CapPresent { + g.negMemo.Store(host, negEntry{status: s, until: g.clock().Add(versionProbeFresh)}) + return s } g.memo.Store(host, struct{}{}) if l := g.currentLatch(); l != nil { l.MarkNative(host) } - return true + return CapPresent } var nativeProbeGate = &nativeGate{} @@ -152,13 +166,17 @@ func KnotSupports114(ctx context.Context, host string, dev bool) bool { return knotSupportsVersion(ctx, host, dev, 1, 14, true) } -func KnotHasCapability(ctx context.Context, host string, dev bool, capability consts.Capability) bool { - return nativeProbeGate.isNative(host, func() bool { +func KnotCapability(ctx context.Context, host string, dev bool, capability consts.Capability) CapStatus { + return nativeProbeGate.status(host, func() CapStatus { return knotDeclares(ctx, host, dev, capability) }) } -func knotDeclares(ctx context.Context, host string, dev bool, capability consts.Capability) bool { +func KnotHasCapability(ctx context.Context, host string, dev bool, capability consts.Capability) bool { + return KnotCapability(ctx, host, dev, capability) == CapPresent +} + +func knotDeclares(ctx context.Context, host string, dev bool, capability consts.Capability) CapStatus { scheme := "https" if dev { scheme = "http" @@ -173,9 +191,12 @@ func knotDeclares(ctx context.Context, host string, dev bool, capability consts. resp, err := tangled.KnotVersion(ctx, client) if err != nil || resp == nil { - return false + return CapUnknown + } + if slices.Contains(resp.Capabilities, string(capability)) { + return CapPresent } - return slices.Contains(resp.Capabilities, string(capability)) + return CapAbsent } func knotSupportsVersion(ctx context.Context, host string, dev bool, minMajor, minMinor int, failOpen bool) bool { diff --git a/appview/knotcompat/version_test.go b/appview/knotcompat/version_test.go index 73f21bc9..98db29a1 100644 --- a/appview/knotcompat/version_test.go +++ b/appview/knotcompat/version_test.go @@ -44,22 +44,26 @@ func (f *fakeLatch) markCount() int { return len(f.marks) } -func probeReturning(v bool, calls *int) func() bool { - return func() bool { +func probeReturning(s CapStatus, calls *int) func() CapStatus { + return func() CapStatus { *calls++ - return v + return s } } +func isNativeForTest(g *nativeGate, host string, probe func() CapStatus) bool { + return g.status(host, probe) == CapPresent +} + func TestNativeGateMemoSkipsSecondProbe(t *testing.T) { g := &nativeGate{} calls := 0 - probe := probeReturning(true, &calls) + probe := probeReturning(CapPresent, &calls) - if !g.isNative("clam.nel.pet", probe) { + if !isNativeForTest(g, "clam.nel.pet", probe) { t.Fatal("first probe true: want native") } - if !g.isNative("clam.nel.pet", probe) { + if !isNativeForTest(g, "clam.nel.pet", probe) { t.Fatal("memoized: want native") } if calls != 1 { @@ -74,7 +78,7 @@ func TestNativeGateLatchHitSkipsProbe(t *testing.T) { g.use(fl) calls := 0 - if !g.isNative("whelk.nel.pet", probeReturning(false, &calls)) { + if !isNativeForTest(g, "whelk.nel.pet", probeReturning(CapAbsent, &calls)) { t.Fatal("latched native: want native even though the probe would fail") } if calls != 0 { @@ -91,15 +95,15 @@ func TestNativeGateProbeMarksLatchOnce(t *testing.T) { g.use(fl) calls := 0 - probe := probeReturning(true, &calls) - if !g.isNative("limpet.nel.pet", probe) { + probe := probeReturning(CapPresent, &calls) + if !isNativeForTest(g, "limpet.nel.pet", probe) { t.Fatal("probe true: want native") } if fl.markCount() != 1 { t.Fatalf("marks = %d, want 1; a first successful probe must latch the host", fl.markCount()) } - g.isNative("limpet.nel.pet", probe) + isNativeForTest(g, "limpet.nel.pet", probe) if fl.markCount() != 1 { t.Fatalf("marks = %d, want 1; the memo must prevent a second mark", fl.markCount()) } @@ -111,7 +115,7 @@ func TestNativeGateProbeFalseDoesNotMark(t *testing.T) { g.use(fl) calls := 0 - if g.isNative("clam.nel.pet", probeReturning(false, &calls)) { + if isNativeForTest(g, "clam.nel.pet", probeReturning(CapAbsent, &calls)) { t.Fatal("probe false on a fresh host: want not native") } if fl.markCount() != 0 { @@ -125,14 +129,14 @@ func TestNativeGateDurableAcrossMemoReset(t *testing.T) { warm := &nativeGate{} warm.use(fl) calls := 0 - if !warm.isNative("whelk.nel.pet", probeReturning(true, &calls)) { + if !isNativeForTest(warm, "whelk.nel.pet", probeReturning(CapPresent, &calls)) { t.Fatal("warm gate probe true: want native") } restarted := &nativeGate{} restarted.use(fl) cold := 0 - if !restarted.isNative("whelk.nel.pet", probeReturning(false, &cold)) { + if !isNativeForTest(restarted, "whelk.nel.pet", probeReturning(CapAbsent, &cold)) { t.Fatal("after restart the durable latch must resolve native without a probe") } if cold != 0 { @@ -147,10 +151,10 @@ func TestNativeGateNegativeMemoThrottlesLatchReads(t *testing.T) { g.use(fl) calls := 0 - probe := probeReturning(false, &calls) + probe := probeReturning(CapAbsent, &calls) for range 5 { - if g.isNative("clam.nel.pet", probe) { + if isNativeForTest(g, "clam.nel.pet", probe) { t.Fatal("a probe-false host must not be native") } } @@ -162,7 +166,7 @@ func TestNativeGateNegativeMemoThrottlesLatchReads(t *testing.T) { } now = now.Add(versionProbeFresh + time.Second) - if g.isNative("clam.nel.pet", probe) { + if isNativeForTest(g, "clam.nel.pet", probe) { t.Fatal("still not native after the window") } if fl.readCount() != 2 { @@ -180,7 +184,7 @@ func TestNativeGateNegativeMemoNeverShadowsLatchedNative(t *testing.T) { g.use(fl) calls := 0 - if g.isNative("whelk.nel.pet", probeReturning(false, &calls)) { + if isNativeForTest(g, "whelk.nel.pet", probeReturning(CapAbsent, &calls)) { t.Fatal("probe false on a fresh host: want not native") } @@ -189,11 +193,38 @@ func TestNativeGateNegativeMemoNeverShadowsLatchedNative(t *testing.T) { fl.mu.Unlock() now = now.Add(versionProbeFresh + time.Second) - if !g.isNative("whelk.nel.pet", probeReturning(false, &calls)) { + if !isNativeForTest(g, "whelk.nel.pet", probeReturning(CapAbsent, &calls)) { t.Fatal("once the negative memo expires a latched host must resolve native again") } } +func TestNativeGateUnknownDistinctFromAbsent(t *testing.T) { + now := time.Now() + g := &nativeGate{now: func() time.Time { return now }} + fl := newFakeLatch() + g.use(fl) + + calls := 0 + probe := probeReturning(CapUnknown, &calls) + + for range 3 { + if got := g.status("clam.nel.pet", probe); got != CapUnknown { + t.Fatalf("status = %v, want CapUnknown for a failed probe", got) + } + } + if calls != 1 { + t.Fatalf("probe calls = %d, want 1; a memoized unknown must throttle re-probes within the window", calls) + } + if fl.markCount() != 0 { + t.Fatalf("marks = %d, want 0; an unknown probe must never latch", fl.markCount()) + } + + now = now.Add(versionProbeFresh + time.Second) + if got := g.status("clam.nel.pet", probeReturning(CapAbsent, &calls)); got != CapAbsent { + t.Fatalf("status = %v, want CapAbsent once a fresh probe reaches the knot", got) + } +} + func TestAtLeast(t *testing.T) { cases := []struct { in string diff --git a/appview/knots/knots.go b/appview/knots/knots.go index 01e6111c..7b920b93 100644 --- a/appview/knots/knots.go +++ b/appview/knots/knots.go @@ -553,7 +553,13 @@ func (k *Knots) addMember(w http.ResponseWriter, r *http.Request) { return } - if knotcompat.KnotHasCapability(r.Context(), domain, k.Config.Core.Dev, consts.CapKnotACL) { + capStatus := knotcompat.KnotCapability(r.Context(), domain, k.Config.Core.Dev, consts.CapKnotACL) + if capStatus == knotcompat.CapUnknown { + l.Error("knot capability probe failed") + k.Pages.Notice(w, noticeId, "Could not reach the knot to add the member. Try again later.") + return + } + if capStatus == knotcompat.CapPresent { client, err := k.OAuth.ServiceClient( r, oauth.WithService(domain), @@ -660,7 +666,13 @@ func (k *Knots) removeMember(w http.ResponseWriter, r *http.Request) { return } - if knotcompat.KnotHasCapability(r.Context(), domain, k.Config.Core.Dev, consts.CapKnotACL) { + capStatus := knotcompat.KnotCapability(r.Context(), domain, k.Config.Core.Dev, consts.CapKnotACL) + if capStatus == knotcompat.CapUnknown { + l.Error("knot capability probe failed") + k.Pages.Notice(w, noticeId, "Could not reach the knot to remove the member. Try again later.") + return + } + if capStatus == knotcompat.CapPresent { client, err := k.OAuth.ServiceClient( r, oauth.WithService(domain), diff --git a/appview/repo/repo.go b/appview/repo/repo.go index 3e7975a2..771170ea 100644 --- a/appview/repo/repo.go +++ b/appview/repo/repo.go @@ -759,7 +759,12 @@ func (rp *Repo) AddCollaborator(w http.ResponseWriter, r *http.Request) { l = l.With("collaborator", collaboratorIdent.Handle) l = l.With("knot", f.Knot) - if knotcompat.KnotHasCapability(r.Context(), f.Knot, rp.config.Core.Dev, consts.CapKnotACL) { + capStatus := knotcompat.KnotCapability(r.Context(), f.Knot, rp.config.Core.Dev, consts.CapKnotACL) + if capStatus == knotcompat.CapUnknown { + fail("Could not reach the knot to add the collaborator. Try again later.", nil) + return + } + if capStatus == knotcompat.CapPresent { if f.RepoDid == "" { fail("This repository is missing its DID and cannot manage collaborators.", nil) return @@ -927,7 +932,12 @@ func (rp *Repo) RemoveCollaborator(w http.ResponseWriter, r *http.Request) { return } - if knotcompat.KnotHasCapability(r.Context(), f.Knot, rp.config.Core.Dev, consts.CapKnotACL) { + capStatus := knotcompat.KnotCapability(r.Context(), f.Knot, rp.config.Core.Dev, consts.CapKnotACL) + if capStatus == knotcompat.CapUnknown { + fail("Could not reach the knot to remove the collaborator. Try again later.", nil) + return + } + if capStatus == knotcompat.CapPresent { if f.RepoDid == "" { fail("This repository is missing its DID and cannot manage collaborators.", nil) return -- 2.51.2