lance: deregister and re-register need UpdateTable (#12552)

* lance: require UpdateTable to deregister a table

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* lance: require UpdateTable to register over an existing table

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* lance: test that GetTable alone cannot deregister or re-register a table

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* lance: a register that finds a table the caller cannot read needs UpdateTable to bring it back

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* lance: decide declare write access by whether the table existed, not by what the caller can read

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* lance: test register and declare by a caller that cannot read the table

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* lance: register clears the marker on the create branch only after checking it

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* lance: keep a register whose table was created when the marker check fails

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>

weed/s3api/lance/handlers_table.go

@@ -135,7 +135,7 @@ func (s *Server) handleDeclareTable(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	_, lookupErr := s.loadLanceTable(r, bucket, ns, name)
+	_, lookupErr := s.loadLanceTableInternal(r, bucket, ns, name)
 	location, err = s.createTable(r, bucket, ns, name, location)
 	if err != nil {
 		writeStorageError(w, r, err)
@@ -143,7 +143,8 @@ func (s *Server) handleDeclareTable(w http.ResponseWriter, r *http.Request) {
 	}
 	// A declare that creates the table may write it: a Lance commit is a direct
 	// S3 write that never revisits the catalog, so the create authorization is
-	// the write evidence. One that only finds the table answers like describe.
+	// the write evidence. One that only finds the table answers like describe,
+	// including a table the caller cannot read, which CreateTable still returns.
 	var writable bool
 	if isNotFound(lookupErr) {
 		writable = s.mayWriteNamespace(r, bucket, ns)
@@ -382,6 +383,7 @@ func (s *Server) handleRegisterTable(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
+	restore := true
 	existing, err := s.loadLanceTable(r, bucket, ns, name)
 	switch {
 	case err != nil && !isNotFound(err):
@@ -403,6 +405,10 @@ func (s *Server) handleRegisterTable(w http.ResponseWriter, r *http.Request) {
 			writeStorageError(w, r, err)
 			return
 		}
+		if err := s.authorizeUpdateTable(r, bucket, ns, name); err != nil {
+			writeStorageError(w, r, err)
+			return
+		}
 		if existing.location != location {
 			// Repointing the name at another dataset is an update. Dropping and
 			// recreating the entry would take the old dataset's files with it,
@@ -417,11 +423,26 @@ func (s *Server) handleRegisterTable(w http.ResponseWriter, r *http.Request) {
 			writeStorageError(w, r, err)
 			return
 		}
+		// CreateTable also answers for a table the caller cannot read, so
+		// bringing that one back still needs UpdateTable.
+		deregistered, _, stateErr := s.datasetState(r, location)
+		if stateErr != nil {
+			glog.V(1).Infof("lance: could not check %s for %s: %v", deregisteredMarker, location, stateErr)
+		}
+		if deregistered {
+			if err := s.authorizeUpdateTable(r, bucket, ns, name); err != nil {
+				writeStorageError(w, r, err)
+				return
+			}
+		}
+		restore = deregistered
 	}
 
 	// Registering a deregistered dataset brings it back.
-	if err := s.removeMarker(r, location, deregisteredMarker); err != nil {
-		glog.V(2).Infof("lance: could not clear %s for %s: %v", deregisteredMarker, location, err)
+	if restore {
+		if err := s.removeMarker(r, location, deregisteredMarker); err != nil {
+			glog.V(2).Infof("lance: could not clear %s for %s: %v", deregisteredMarker, location, err)
+		}
 	}
 
 	writeJSON(w, http.StatusOK, RegisterTableResponse{
@@ -462,6 +483,10 @@ func (s *Server) handleDeregisterTable(w http.ResponseWriter, r *http.Request) {
 		writeStorageError(w, r, err)
 		return
 	}
+	if err := s.authorizeUpdateTable(r, bucket, ns, name); err != nil {
+		writeStorageError(w, r, err)
+		return
+	}
 
 	// Deregistering is a state, not a deletion. Dropping the catalog entry would
 	// take the dataset with it, because the entry is the dataset directory and

weed/s3api/lance/handlers_test.go

@@ -451,14 +451,18 @@ func TestDataPlaneIsUnsupported(t *testing.T) {
 }
 
 // createOnlyAuthenticator signs in an identity that may create and read tables
-// but not update them.
-type createOnlyAuthenticator struct{}
+// but not update them; noRead also takes away reading.
+type createOnlyAuthenticator struct{ noRead bool }
 
-func (createOnlyAuthenticator) AuthenticateRequest(*http.Request) (string, interface{}, s3err.ErrorCode) {
+func (a createOnlyAuthenticator) AuthenticateRequest(*http.Request) (string, interface{}, s3err.ErrorCode) {
+	actions := []string{"s3tables:CreateTable", "s3tables:GetTable"}
+	if a.noRead {
+		actions = actions[:1]
+	}
 	return "creator", &struct {
 		Name    string
 		Actions []string
-	}{Name: "creator", Actions: []string{"s3tables:CreateTable", "s3tables:GetTable"}}, s3err.ErrNone
+	}{Name: "creator", Actions: actions}, s3err.ErrNone
 }
 
 func (createOnlyAuthenticator) DefaultAllow() bool { return false }
@@ -495,6 +499,47 @@ func TestDeclareVendsWriteOnlyForATableItCreates(t *testing.T) {
 	}
 }
 
+// Deregistering hides a table and registering over one brings it back, so
+// both change its catalog state and need UpdateTable, not just GetTable.
+func TestDeregisterAndReregisterNeedUpdateTable(t *testing.T) {
+	h := newTestHarness(t)
+	h.createBucket(t, "lnc", s3tables.FormatLance)
+	h.mustDo(t, http.MethodPost, "/v1/namespace/lnc$ns/create", `{}`, http.StatusOK)
+	h.mustDo(t, http.MethodPost, "/v1/table/lnc$ns$theirs/declare", `{}`, http.StatusOK)
+
+	server := NewServer(s3tables.NewManagerClient(h.filer.Client), createOnlyAuthenticator{})
+	router := mux.NewRouter().SkipClean(true)
+	server.RegisterRoutes(router)
+	reader := &testHarness{router: router, filer: h.filer, admin: h.admin, server: server}
+
+	reader.mustDo(t, http.MethodPost, "/v1/table/lnc$ns$theirs/describe", `{}`, http.StatusOK)
+	reader.mustDo(t, http.MethodPost, "/v1/table/lnc$ns$theirs/deregister", `{}`, http.StatusForbidden)
+	if h.filer.Get("/buckets/lnc/ns/theirs", deregisteredMarker) != nil {
+		t.Fatal("a refused deregister left its marker")
+	}
+
+	h.mustDo(t, http.MethodPost, "/v1/table/lnc$ns$theirs/deregister", `{}`, http.StatusOK)
+	reader.mustDo(t, http.MethodPost, "/v1/table/lnc$ns$theirs/register", `{"location":"s3://lnc/ns/theirs"}`, http.StatusForbidden)
+	if h.filer.Get("/buckets/lnc/ns/theirs", deregisteredMarker) == nil {
+		t.Fatal("a refused register brought the table back")
+	}
+
+	// Without GetTable the table looks absent, but CreateTable still returns it.
+	blindServer := NewServer(s3tables.NewManagerClient(h.filer.Client), createOnlyAuthenticator{noRead: true})
+	blindServer.SetCredentialVendor(recordingVendor{})
+	blindRouter := mux.NewRouter().SkipClean(true)
+	blindServer.RegisterRoutes(blindRouter)
+	blind := &testHarness{router: blindRouter, filer: h.filer, admin: h.admin, server: blindServer}
+
+	blind.mustDo(t, http.MethodPost, "/v1/table/lnc$ns$theirs/register", `{"location":"s3://lnc/ns/theirs"}`, http.StatusNotFound)
+	if got := blind.vendedKey(t, "/v1/table/lnc$ns$theirs/declare", ""); got == "read-write" {
+		t.Error("a declare that found an unreadable table vended write credentials")
+	}
+	if h.filer.Get("/buckets/lnc/ns/theirs", deregisteredMarker) == nil {
+		t.Fatal("a caller without GetTable brought the table back")
+	}
+}
+
 // An in-process describe for a trusted caller resolves the table without the
 // caller's catalog grants, and still refuses a deregistered one.
 func TestDescribeTableInternalIgnoresCatalogGrants(t *testing.T) {

weed/s3api/lance/storage.go

@@ -224,6 +224,20 @@ func (s *Server) mayUpdateTable(r *http.Request, bucket string, ns []string, nam
 	return err == nil
 }
 
+// authorizeUpdateTable checks the grant a change to the table's catalog state
+// needs; a cluster process goes through the trusted manager.
+func (s *Server) authorizeUpdateTable(r *http.Request, bucket string, ns []string, name string) error {
+	req := &s3tables.UpdateTableRequest{
+		TableBucketARN: bucketARN(bucket),
+		Namespace:      ns,
+		Name:           name,
+	}
+	identityName := s3_constants.GetIdentityNameFromContext(r)
+	return s.filerClient.WithFilerClient(false, func(client filer_pb.SeaweedFilerClient) error {
+		return s.manager(r).AuthorizeUpdateTable(r.Context(), s3tables.NewManagerClient(client), req, identityName)
+	})
+}
+
 // mayWriteNamespace keeps the vector namespace of a vector bucket out of reach
 // of s3tables grants: its index data is written only by trusted callers, and
 // the vector bucket policy gates everyone else through S3 Vectors.