From f5eef7da8abb22bf79c94504fa0ea67b893494de Mon Sep 17 00:00:00 2001 From: "zhenshan.cao" Date: Tue, 25 Aug 2026 21:25:22 -0700 Subject: [PATCH] enhance: remove the /expr runtime-introspection endpoint (#52876) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Removes the `/expr` runtime-introspection endpoint, everything that exists only to serve it, and the forked expression library it depended on. ### Why `/expr` evaluated a caller-supplied expression against live in-process objects (`param`, `proxy`, `cache`, and the component servers) and returned the result. It was useful for reading an effective config value or unexported state without a debugger or a rebuild, but the cost is hard to justify for what it does: - It required a fork of `expr-lang/expr` whose purpose is to remove the library's sandbox. The first commit on that fork is "Support the private field and auto change the struct to the point when exec the method". Upstream deliberately refuses unexported-field access; the fork exists to allow it. - That fork sits on a personal account, is 81 commits behind upstream, and was last pushed in May 2025. `go.mod` pinned it through `replace`, making it build-critical. Every other `replace` in that block points at `milvus-io/*`. - Nothing else imports the library. `pkg/util/expr/expr.go` was the only importer, using four symbols: `expr.Compile`, `expr.Env`, `expr.WithContext` and `vm.VM.Run`. The fork's ~550 lines of product code add no API; they change the behaviour of those four calls. Swapping back to upstream would therefore still compile and still pass CI, and break only at runtime — no test in the tree exercises unexported-field access, pointer-method calls, `mapValue`, the `view` builtin, or automatic type conversion. Removing the endpoint removes the only reason the fork was there. ### What is removed - the `pkg/util/expr` package - nine `expr.Init` / `expr.Register` call sites across roles, datacoord, datanode, proxy, meta cache, querycoord, querynode and rootcoord - the `/static/` Expr Executor page (`internal/http/static/index.html`, whose only function was to call `/expr`), `static_view.go`, `StaticPath` and its route registration - the `/expr` handler in `internal/http/server.go`, `ExprPath` in `router.go`, and `CheckExprAuth` / `checkExprRootAuth` / the two auth-mode constants in `internal/http/rbac.go` - `common.security.exprEnabled` and `common.security.exprAuthMode`, and their entries in `configs/milvus.yaml` - `github.com/expr-lang/expr` and its `replace` to `github.com/SimFG/expr`, from both `go.mod` and `pkg/go.mod` ### The Expr privilege v3.0.0 shipped `Expr` as a grantable RBAC privilege for this endpoint. The name cannot simply be dropped: grant and revoke share one validation path (`ddl_callbacks_rbac_privilege.go` dispatches on request version, not on grant-vs-revoke), so an undefined name would leave grants made on v3.0.0 both unusable and unrevocable, clearable only by editing etcd by hand. The second commit rejects the grant explicitly instead, with a message that says why, and leaves every other path untouched. The name stays recognized and inert so existing grants remain listable and revocable, and can be dropped entirely once no supported release still has the endpoint. ### What is deliberately kept `CheckPrivilege`, `RegisterGetUserRoleFunc`, `RegisterPasswordVerifyFunc` and `parseHTTPAuth` in `internal/http/rbac.go`. These are general HTTP RBAC helpers rather than expr-specific, and #52580 builds on that surface. After this change only tests call `CheckPrivilege`; that is intentional, not an oversight. ### Verification Unit tests pass for `internal/http`, `internal/rootcoord` and `pkg/util`, including `TestExprPrivilegeDefinition` and a new `TestIsDeprecatedExprPrivilege` that covers both the API and metastore spellings plus negative cases. Behaviour was also checked against a running instance rather than inferred: ``` route removal /expr 404 "404 page not found" /nonexistent-route-xyz 404 "404 page not found" identical response /healthz, /metrics 200 server is healthy /expr on a build that still registers it 500 from expr.Exec privilege guard grant Expr (v2) rejected: privilege "Expr" is deprecated: the /expr grant Expr (v1) rejected endpoint it granted access to has been removed revoke Expr (v2) accepted revoke Expr (v1) accepted grant/revoke Load accepted (control) a pre-existing v3.0.0 grant wrote grantee-id//PrivilegeExpr directly into etcd, restarted the cluster describe_role shows Global/Expr alongside Global/CreateDatabase revoke Expr succeeds describe_role only CreateDatabase remains etcd the PrivilegeExpr key is gone, no orphan left behind ``` ### Scope note This does not close the unauthenticated `:9091` surface. `/metrics` and `/debug/pprof/` remain unauthenticated; that is #52580's subject. This change removes one route from that port and the supply-chain exposure behind it. --------- Signed-off-by: zhenshan.cao Signed-off-by: Zack Co-authored-by: Zack Co-authored-by: Claude Fable 5 --- cmd/roles/roles.go | 3 - configs/milvus.yaml | 2 - go.mod | 2 - go.sum | 2 - internal/datacoord/server.go | 2 - internal/datanode/data_node.go | 2 - internal/http/rbac.go | 49 --- internal/http/rbac_test.go | 44 --- internal/http/router.go | 6 - internal/http/server.go | 45 --- internal/http/server_test.go | 301 ----------------- internal/http/static/index.html | 314 ------------------ internal/http/static_view.go | 33 -- internal/proxy/meta_cache.go | 2 - internal/proxy/proxy.go | 2 - internal/querycoordv2/server.go | 2 - internal/querynodev2/server.go | 2 - .../rootcoord/ddl_callbacks_rbac_privilege.go | 16 + .../ddl_callbacks_rbac_privilege_test.go | 14 + internal/rootcoord/root_coord.go | 2 - pkg/go.mod | 2 - pkg/go.sum | 2 - pkg/util/constant.go | 9 +- pkg/util/expr/expr.go | 109 ------ pkg/util/expr/expr_test.go | 131 -------- pkg/util/paramtable/component_param.go | 22 -- 26 files changed, 37 insertions(+), 1083 deletions(-) delete mode 100644 internal/http/static/index.html delete mode 100644 internal/http/static_view.go delete mode 100644 pkg/util/expr/expr.go delete mode 100644 pkg/util/expr/expr_test.go diff --git a/cmd/roles/roles.go b/cmd/roles/roles.go index 135d18706a..0e90c25324 100644 --- a/cmd/roles/roles.go +++ b/cmd/roles/roles.go @@ -53,7 +53,6 @@ import ( "github.com/milvus-io/milvus/pkg/v3/tracer" "github.com/milvus-io/milvus/pkg/v3/util/conc" "github.com/milvus-io/milvus/pkg/v3/util/etcd" - "github.com/milvus-io/milvus/pkg/v3/util/expr" "github.com/milvus-io/milvus/pkg/v3/util/gc" "github.com/milvus-io/milvus/pkg/v3/util/logutil" "github.com/milvus-io/milvus/pkg/v3/util/metricsinfo" @@ -467,8 +466,6 @@ func (mr *MilvusRoles) Run() { }) healthz.SetComponentNum(len(enableComponents)) - expr.Init() - expr.Register("param", paramtable.Get()) mr.setupLogger() defer mlog.Cleanup() diff --git a/configs/milvus.yaml b/configs/milvus.yaml index 323dbc7d67..9ae0fb4709 100644 --- a/configs/milvus.yaml +++ b/configs/milvus.yaml @@ -1181,8 +1181,6 @@ common: defaultRootPassword: Milvus rootShouldBindRole: false # Whether the root user should bind a role when the authorization is enabled. enablePublicPrivilege: true # Whether to enable public privilege - exprEnabled: false # Whether to enable the /expr endpoint for debugging. - exprAuthMode: rootOnly # Authentication mode for the /expr endpoint. Valid values: rootOnly, rbac. rootOnly accepts only the root credentials via HTTP Basic Auth. rbac requires common.security.authorizationEnabled=true and grants access to any user holding the Expr privilege. internaltlsEnabled: false tlsMode: 0 session: diff --git a/go.mod b/go.mod index 5cfe10924e..85b2e03500 100644 --- a/go.mod +++ b/go.mod @@ -157,7 +157,6 @@ require ( github.com/ebitengine/purego v0.9.0 // indirect github.com/envoyproxy/go-control-plane/envoy v1.37.0 // indirect github.com/envoyproxy/protoc-gen-validate v1.3.3 // indirect - github.com/expr-lang/expr v1.15.7 // indirect github.com/felixge/httpsnoop v1.0.4 // indirect github.com/fxamacker/cbor/v2 v2.7.0 // indirect github.com/gabriel-vasile/mimetype v1.4.8 // indirect @@ -314,7 +313,6 @@ require ( replace ( github.com/apache/arrow/go/v17 => github.com/milvus-io/arrow/go/v17 v17.0.2 github.com/bketelsen/crypt => github.com/bketelsen/crypt v0.0.4 // Fix security alert for core-os/etcd - github.com/expr-lang/expr => github.com/SimFG/expr v0.0.0-20250513112851-9b981e8400b9 github.com/go-kit/kit => github.com/go-kit/kit v0.1.0 github.com/greatroar/blobloom => github.com/milvus-io/blobloom v0.0.0-20240603110411-471ae49f3b93 github.com/ianlancetaylor/cgosymbolizer => github.com/milvus-io/cgosymbolizer v0.0.0-20260810014817-f3b338cf6824 diff --git a/go.sum b/go.sum index 352b448e19..1ffbf78820 100644 --- a/go.sum +++ b/go.sum @@ -110,8 +110,6 @@ github.com/OneOfOne/xxhash v1.2.2/go.mod h1:HSdplMjZKSmBqAxg5vPj2TmRDmfkzw+cTzAE github.com/RoaringBitmap/roaring/v2 v2.8.0 h1:y1rdtixfXvaITKzkfiKvScI0hlBJHe9sfzJp8cgeM7w= github.com/RoaringBitmap/roaring/v2 v2.8.0/go.mod h1:FiJcsfkGje/nZBZgCu0ZxCPOKD/hVXDS2dXi7/eUFE0= github.com/Shopify/goreferrer v0.0.0-20181106222321-ec9c9a553398/go.mod h1:a1uqRtAwp2Xwc6WNPJEufxJ7fx3npB4UV/JOLmbu5I0= -github.com/SimFG/expr v0.0.0-20250513112851-9b981e8400b9 h1:eXnmJhsHt8m6NU3IJ19UthXJ8JK6e3tmfN07nym3BXs= -github.com/SimFG/expr v0.0.0-20250513112851-9b981e8400b9/go.mod h1:8/vRC7+7HBzESEqt5kKpYXxrxkr31SaO8r40VO/1IT4= github.com/StackExchange/wmi v0.0.0-20190523213315-cbe66965904d h1:G0m3OIz70MZUWq3EgK3CesDbo8upS2Vm9/P3FtgI+Jk= github.com/StackExchange/wmi v0.0.0-20190523213315-cbe66965904d/go.mod h1:3eOhrUMpNV+6aFIbp5/iudMxNCF27Vw2OZgy4xEx0Fg= github.com/actgardner/gogen-avro/v10 v10.1.0/go.mod h1:o+ybmVjEa27AAr35FRqU98DJu1fXES56uXniYFv4yDA= diff --git a/internal/datacoord/server.go b/internal/datacoord/server.go index 09ed0485b3..66c3d20ef8 100644 --- a/internal/datacoord/server.go +++ b/internal/datacoord/server.go @@ -53,7 +53,6 @@ import ( "github.com/milvus-io/milvus/pkg/v3/proto/datapb" "github.com/milvus-io/milvus/pkg/v3/proto/querypb" "github.com/milvus-io/milvus/pkg/v3/util" - "github.com/milvus-io/milvus/pkg/v3/util/expr" "github.com/milvus-io/milvus/pkg/v3/util/lock" "github.com/milvus-io/milvus/pkg/v3/util/logutil" "github.com/milvus-io/milvus/pkg/v3/util/merr" @@ -221,7 +220,6 @@ func CreateServer(ctx context.Context, factory dependency.Factory, opts ...Optio for _, opt := range opts { opt(s) } - expr.Register("datacoord", s) return s } diff --git a/internal/datanode/data_node.go b/internal/datanode/data_node.go index 5153a8c465..5cdbc41b48 100644 --- a/internal/datanode/data_node.go +++ b/internal/datanode/data_node.go @@ -45,7 +45,6 @@ import ( "github.com/milvus-io/milvus/pkg/v3/metrics" "github.com/milvus-io/milvus/pkg/v3/mlog" "github.com/milvus-io/milvus/pkg/v3/util/conc" - "github.com/milvus-io/milvus/pkg/v3/util/expr" "github.com/milvus-io/milvus/pkg/v3/util/lifetime" "github.com/milvus-io/milvus/pkg/v3/util/merr" "github.com/milvus-io/milvus/pkg/v3/util/metricsinfo" @@ -129,7 +128,6 @@ func NewDataNode(ctx context.Context) *DataNode { node.taskManager = index.NewTaskManager(ctx2) node.externalCollectionManager = external.NewExternalCollectionManager(ctx2, 8) node.UpdateStateCode(commonpb.StateCode_Abnormal) - expr.Register("datanode", node) return node } diff --git a/internal/http/rbac.go b/internal/http/rbac.go index b3bf7d15a4..1ac3ef5bdb 100644 --- a/internal/http/rbac.go +++ b/internal/http/rbac.go @@ -20,7 +20,6 @@ import ( "context" "fmt" "net/http" - "strings" "github.com/cockroachdb/errors" @@ -32,11 +31,6 @@ import ( "github.com/milvus-io/milvus/pkg/v3/util/paramtable" ) -const ( - ExprAuthModeRootOnly = "rootOnly" - ExprAuthModeRBAC = "rbac" -) - // getUserRoleFunc is a callback function to get user roles. // This is set by the proxy package to avoid circular dependency. var getUserRoleFunc func(username string) ([]string, error) @@ -80,49 +74,6 @@ func parseHTTPAuth(req *http.Request) (username, password string, ok bool) { return req.BasicAuth() } -// CheckExprAuth authenticates and authorizes access to the /expr endpoint. -func CheckExprAuth(ctx context.Context, req *http.Request) error { - mode := paramtable.Get().CommonCfg.ExprAuthMode.GetValue() - switch strings.ToLower(mode) { - case "", strings.ToLower(ExprAuthModeRootOnly): - return checkExprRootAuth(ctx, req) - case ExprAuthModeRBAC: - if !paramtable.Get().CommonCfg.AuthorizationEnabled.GetAsBool() { - return &ErrPermissionDenied{msg: "authorization must be enabled when common.security.exprAuthMode is rbac"} - } - return CheckPrivilege( - ctx, - req, - commonpb.ObjectType_Global, - util.PrivilegeExpr, - util.AnyWord, - util.DefaultDBName, - ) - default: - return &ErrPermissionDenied{msg: fmt.Sprintf("invalid expr auth mode %q", mode)} - } -} - -func checkExprRootAuth(ctx context.Context, req *http.Request) error { - username, password, ok := parseHTTPAuth(req) - if !ok || username == "" || password == "" { - return &ErrAuthentication{msg: "authentication required. Use HTTP Basic Auth with root credentials"} - } - if username != util.UserRoot { - mlog.Warn(ctx, "non-root user attempted to access /expr", mlog.String("username", username)) - return &ErrPermissionDenied{msg: "only root user can access /expr endpoint"} - } - if passwordVerifyFunc == nil { - return &ErrServiceUnavailable{msg: "password verification not available"} - } - if !passwordVerifyFunc(ctx, username, password) { - mlog.Warn(ctx, "invalid root password for /expr access") - return &ErrAuthentication{msg: "invalid root password"} - } - mlog.Info(ctx, "root user authenticated for /expr access") - return nil -} - // CheckPrivilege checks if the authenticated user has the specified privilege. func CheckPrivilege(ctx context.Context, req *http.Request, objectType commonpb.ObjectType, objectPrivilege string, objectName string, dbName string, diff --git a/internal/http/rbac_test.go b/internal/http/rbac_test.go index 41a43c0175..eb82a7bb88 100644 --- a/internal/http/rbac_test.go +++ b/internal/http/rbac_test.go @@ -107,48 +107,6 @@ func TestEnforceErrorMapsToInternalServerError(t *testing.T) { assert.Equal(t, http.StatusInternalServerError, HTTPStatusFromPrivilegeError(err)) } -func TestCheckExprAuth(t *testing.T) { - paramtable.Init() - originalPasswordVerify := passwordVerifyFunc - originalGetUserRole := getUserRoleFunc - defer func() { - passwordVerifyFunc = originalPasswordVerify - getUserRoleFunc = originalGetUserRole - paramtable.Get().Reset(paramtable.Get().CommonCfg.AuthorizationEnabled.Key) - paramtable.Get().Reset(paramtable.Get().CommonCfg.RootShouldBindRole.Key) - paramtable.Get().Reset(paramtable.Get().CommonCfg.ExprAuthMode.Key) - }() - - t.Run("root_only_rejects_non_root_with_valid_credentials", func(t *testing.T) { - paramtable.Get().Save(paramtable.Get().CommonCfg.ExprAuthMode.Key, ExprAuthModeRootOnly) - RegisterPasswordVerifyFunc(func(ctx context.Context, username, password string) bool { - return username == "admin" && password == "admin123" - }) - req := httptest.NewRequest(http.MethodGet, "/expr", nil) - req.SetBasicAuth("admin", "admin123") - - err := CheckExprAuth(context.Background(), req) - - assert.Error(t, err) - assert.True(t, IsPermissionDeniedError(err)) - assert.Equal(t, http.StatusForbidden, HTTPStatusFromPrivilegeError(err)) - }) - - t.Run("rbac_fails_closed_when_authorization_disabled", func(t *testing.T) { - paramtable.Get().Save(paramtable.Get().CommonCfg.ExprAuthMode.Key, ExprAuthModeRBAC) - paramtable.Get().Save(paramtable.Get().CommonCfg.AuthorizationEnabled.Key, "false") - req := httptest.NewRequest(http.MethodGet, "/expr", nil) - req.SetBasicAuth(util.UserRoot, "Milvus") - - err := CheckExprAuth(context.Background(), req) - - assert.Error(t, err) - assert.True(t, IsPermissionDeniedError(err)) - assert.Contains(t, err.Error(), "authorization must be enabled") - }) -} - -// CheckPrivilegeTestSuite tests the CheckPrivilege function type CheckPrivilegeTestSuite struct { suite.Suite ctx context.Context @@ -174,7 +132,6 @@ func (s *CheckPrivilegeTestSuite) TearDownTest() { // Reset paramtable settings paramtable.Get().Reset(paramtable.Get().CommonCfg.AuthorizationEnabled.Key) paramtable.Get().Reset(paramtable.Get().CommonCfg.RootShouldBindRole.Key) - paramtable.Get().Reset(paramtable.Get().CommonCfg.ExprAuthMode.Key) } func (s *CheckPrivilegeTestSuite) TestAuthorizationDisabledFailsClosed() { @@ -419,7 +376,6 @@ func (s *CheckPrivilegeWithEnforcerTestSuite) TearDownTest() { getUserRoleFunc = s.originalGetUserRole paramtable.Get().Reset(paramtable.Get().CommonCfg.AuthorizationEnabled.Key) paramtable.Get().Reset(paramtable.Get().CommonCfg.RootShouldBindRole.Key) - paramtable.Get().Reset(paramtable.Get().CommonCfg.ExprAuthMode.Key) privilege.CleanPrivilegeCache() } diff --git a/internal/http/router.go b/internal/http/router.go index 7eb06939d4..a348b1396c 100644 --- a/internal/http/router.go +++ b/internal/http/router.go @@ -28,12 +28,6 @@ const LogLevelRouterPath = "/log/level" // EventLogRouterPath is path for eventlog control. const EventLogRouterPath = "/eventlog" -// ExprPath is path for expression. -const ExprPath = "/expr" - -// StaticPath is path for the static view. -const StaticPath = "/static/" - const RootPath = "/" // Prometheus restful api path diff --git a/internal/http/server.go b/internal/http/server.go index 1c56d28d2b..6eb003e825 100644 --- a/internal/http/server.go +++ b/internal/http/server.go @@ -31,7 +31,6 @@ import ( "github.com/milvus-io/milvus/internal/http/healthz" "github.com/milvus-io/milvus/pkg/v3/eventlog" "github.com/milvus-io/milvus/pkg/v3/mlog" - "github.com/milvus-io/milvus/pkg/v3/util/expr" "github.com/milvus-io/milvus/pkg/v3/util/paramtable" ) @@ -100,50 +99,6 @@ func registerDefaults() { Path: EventLogRouterPath, Handler: eventlog.Handler(), }) - Register(&Handler{ - Path: ExprPath, - Handler: http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { - // Check if expr endpoint is enabled - if !paramtable.Get().CommonCfg.ExprEnabled.GetAsBool() { - w.WriteHeader(http.StatusForbidden) - w.Write([]byte(`{"msg": "expr endpoint is disabled. Set common.security.exprEnabled to true to enable it."}`)) - return - } - - code := req.URL.Query().Get("code") - var auth string - - // Only Proxy nodes can access /expr endpoint - if !expr.HasRegistered("proxy") || passwordVerifyFunc == nil { - w.WriteHeader(http.StatusForbidden) - w.Write([]byte(`{"msg": "/expr endpoint is only available on Proxy nodes"}`)) - return - } - - if err := CheckExprAuth(req.Context(), req); err != nil { - writeJSONError(w, HTTPStatusFromPrivilegeError(err), err.Error()) - return - } - // Use bypass since we've already authenticated - auth = expr.AuthBypass - - output, err := expr.Exec(code, auth) - if err != nil { - writeJSONError(w, http.StatusInternalServerError, - fmt.Sprintf("failed to execute expression, %s", err.Error())) - return - } - w.Header().Set("Content-Type", "application/json") - w.WriteHeader(http.StatusOK) - resp := make(map[string]string) - resp["output"] = output - json.NewEncoder(w).Encode(resp) - }), - }) - Register(&Handler{ - Path: StaticPath, - Handler: GetStaticHandler(), - }) if paramtable.Get().HTTPCfg.EnableWebUI.GetAsBool() { RegisterWebUIHandler() diff --git a/internal/http/server_test.go b/internal/http/server_test.go index 6a5f905627..4553d430bf 100644 --- a/internal/http/server_test.go +++ b/internal/http/server_test.go @@ -35,11 +35,7 @@ import ( "github.com/milvus-io/milvus-proto/go-api/v3/commonpb" "github.com/milvus-io/milvus/internal/http/healthz" "github.com/milvus-io/milvus/internal/json" - "github.com/milvus-io/milvus/internal/proxy/privilege" "github.com/milvus-io/milvus/pkg/v3/mlog" - "github.com/milvus-io/milvus/pkg/v3/util" - "github.com/milvus-io/milvus/pkg/v3/util/expr" - "github.com/milvus-io/milvus/pkg/v3/util/funcutil" "github.com/milvus-io/milvus/pkg/v3/util/paramtable" ) @@ -202,303 +198,6 @@ func (suite *HTTPServerTestSuite) TestPprofHandler() { } } -func (suite *HTTPServerTestSuite) TestExprHandler() { - expr.Init() - expr.Register("foo", "hello") - - suite.Run("disabled_by_default", func() { - // By default, exprEnabled is false, should return 403 - paramtable.Get().Save("common.security.exprEnabled", "false") - url := "http://localhost:" + DefaultListenPort + ExprPath + "?code=foo&auth=by-dev" - client := http.Client{} - req, _ := http.NewRequest(http.MethodGet, url, nil) - resp, err := client.Do(req) - suite.Nil(err) - defer resp.Body.Close() - suite.Equal(http.StatusForbidden, resp.StatusCode) - body, _ := io.ReadAll(resp.Body) - suite.True(strings.Contains(string(body), "expr endpoint is disabled")) - }) - - suite.Run("disabled_on_non_proxy_nodes", func() { - // When enabled but not on Proxy node (no proxy registered, no passwordVerifyFunc), - // it should return 403 Forbidden - paramtable.Get().Save("common.security.exprEnabled", "true") - - // Should be forbidden on non-Proxy nodes - url := "http://localhost:" + DefaultListenPort + ExprPath + "?code=foo&auth=by-dev" - client := http.Client{} - req, _ := http.NewRequest(http.MethodGet, url, nil) - resp, err := client.Do(req) - suite.Nil(err) - defer resp.Body.Close() - suite.Equal(http.StatusForbidden, resp.StatusCode) - body, _ := io.ReadAll(resp.Body) - suite.True(strings.Contains(string(body), "only available on Proxy nodes")) - }) - - suite.Run("root_only_requires_root_when_authorization_disabled", func() { - paramtable.Get().Save("common.security.exprEnabled", "true") - paramtable.Get().Save("common.security.exprAuthMode", ExprAuthModeRootOnly) - paramtable.Get().Save("common.security.authorizationEnabled", "false") - expr.Register("proxy", "mock_proxy") - - RegisterPasswordVerifyFunc(func(ctx context.Context, username, password string) bool { - return (username == "root" && password == "Milvus") || - (username == "admin" && password == "admin123") - }) - - url := "http://localhost:" + DefaultListenPort + ExprPath + "?code=foo" - client := http.Client{} - req, _ := http.NewRequest(http.MethodGet, url, nil) - req.SetBasicAuth("admin", "admin123") - resp, err := client.Do(req) - suite.Nil(err) - defer resp.Body.Close() - suite.Equal(http.StatusForbidden, resp.StatusCode) - body, _ := io.ReadAll(resp.Body) - suite.True(strings.Contains(string(body), "only root user can access")) - - req, _ = http.NewRequest(http.MethodGet, url, nil) - req.SetBasicAuth("root", "Milvus") - resp, err = client.Do(req) - suite.Nil(err) - defer resp.Body.Close() - suite.Equal(http.StatusOK, resp.StatusCode) - body, _ = io.ReadAll(resp.Body) - suite.True(strings.Contains(string(body), "hello")) - }) - - suite.Run("rbac_mode_requires_authorization_enabled", func() { - paramtable.Get().Save("common.security.exprEnabled", "true") - paramtable.Get().Save("common.security.exprAuthMode", ExprAuthModeRBAC) - paramtable.Get().Save("common.security.authorizationEnabled", "false") - expr.Register("proxy", "mock_proxy") - - RegisterPasswordVerifyFunc(func(ctx context.Context, username, password string) bool { - return username == "root" && password == "Milvus" - }) - - url := "http://localhost:" + DefaultListenPort + ExprPath + "?code=foo" - client := http.Client{} - req, _ := http.NewRequest(http.MethodGet, url, nil) - req.SetBasicAuth("root", "Milvus") - resp, err := client.Do(req) - suite.Nil(err) - defer resp.Body.Close() - suite.Equal(http.StatusForbidden, resp.StatusCode) - body, _ := io.ReadAll(resp.Body) - suite.True(strings.Contains(string(body), "authorization must be enabled")) - }) - - suite.Run("enabled_on_proxy_with_rbac_root_bypass", func() { - // When authorization is enabled but RootShouldBindRole is false, - // root user should be able to access via bypass - paramtable.Get().Save("common.security.exprEnabled", "true") - paramtable.Get().Save("common.security.exprAuthMode", ExprAuthModeRBAC) - paramtable.Get().Save("common.security.authorizationEnabled", "true") - paramtable.Get().Save("common.security.rootShouldBindRole", "false") - expr.Register("proxy", "mock_proxy") - - // Register mock functions - RegisterPasswordVerifyFunc(func(ctx context.Context, username, password string) bool { - return (username == "root" && password == "Milvus") || - (username == "admin" && password == "admin123") - }) - - // Without auth header - should fail with 401 - url := "http://localhost:" + DefaultListenPort + ExprPath + "?code=foo" - client := http.Client{} - req, _ := http.NewRequest(http.MethodGet, url, nil) - resp, err := client.Do(req) - suite.Nil(err) - defer resp.Body.Close() - suite.Equal(http.StatusUnauthorized, resp.StatusCode) - body, _ := io.ReadAll(resp.Body) - suite.True(strings.Contains(string(body), "authentication required")) - - // With non-root user (without Expr privilege) - should fail with 401 (invalid credentials) - url = "http://localhost:" + DefaultListenPort + ExprPath + "?code=foo" - req, _ = http.NewRequest(http.MethodGet, url, nil) - req.SetBasicAuth("admin", "wrong_password") - resp, err = client.Do(req) - suite.Nil(err) - defer resp.Body.Close() - suite.Equal(http.StatusUnauthorized, resp.StatusCode) - body, _ = io.ReadAll(resp.Body) - suite.True(strings.Contains(string(body), "invalid credentials")) - - // With root user but wrong password - should fail with 401 - url = "http://localhost:" + DefaultListenPort + ExprPath + "?code=foo" - req, _ = http.NewRequest(http.MethodGet, url, nil) - req.SetBasicAuth("root", "wrong_password") - resp, err = client.Do(req) - suite.Nil(err) - defer resp.Body.Close() - suite.Equal(http.StatusUnauthorized, resp.StatusCode) - body, _ = io.ReadAll(resp.Body) - suite.True(strings.Contains(string(body), "invalid credentials")) - - // With correct root credentials - should succeed via root bypass - url = "http://localhost:" + DefaultListenPort + ExprPath + "?code=foo" - req, _ = http.NewRequest(http.MethodGet, url, nil) - req.SetBasicAuth("root", "Milvus") - resp, err = client.Do(req) - suite.Nil(err) - defer resp.Body.Close() - suite.Equal(http.StatusOK, resp.StatusCode) - body, _ = io.ReadAll(resp.Body) - suite.True(strings.Contains(string(body), "hello")) - }) - - suite.Run("invalid_auth_mode_returns_valid_json", func() { - paramtable.Get().Save("common.security.exprEnabled", "true") - paramtable.Get().Save("common.security.exprAuthMode", "rbca") - paramtable.Get().Save("common.security.authorizationEnabled", "true") - paramtable.Get().Save("common.security.rootShouldBindRole", "false") - expr.Register("proxy", "mock_proxy") - - RegisterPasswordVerifyFunc(func(ctx context.Context, username, password string) bool { - return (username == "root" && password == "Milvus") || - (username == "admin" && password == "admin123") - }) - - url := "http://localhost:" + DefaultListenPort + ExprPath + "?code=foo" - client := http.Client{} - req, _ := http.NewRequest(http.MethodGet, url, nil) - req.SetBasicAuth("root", "Milvus") - resp, err := client.Do(req) - suite.Nil(err) - defer resp.Body.Close() - suite.Equal(http.StatusForbidden, resp.StatusCode) - - body, _ := io.ReadAll(resp.Body) - var parsed map[string]string - err = json.Unmarshal(body, &parsed) - suite.NoError(err) - suite.Contains(parsed["msg"], "rbca") - }) - - suite.Run("exec_error_returns_valid_json", func() { - paramtable.Get().Save("common.security.exprEnabled", "true") - paramtable.Get().Save("common.security.exprAuthMode", ExprAuthModeRootOnly) - paramtable.Get().Save("common.security.authorizationEnabled", "false") - paramtable.Get().Save("common.security.rootShouldBindRole", "false") - expr.Register("proxy", "mock_proxy") - - RegisterPasswordVerifyFunc(func(ctx context.Context, username, password string) bool { - return (username == "root" && password == "Milvus") || - (username == "admin" && password == "admin123") - }) - - url := "http://localhost:" + DefaultListenPort + ExprPath + "?code=1%2B" - client := http.Client{} - req, _ := http.NewRequest(http.MethodGet, url, nil) - req.SetBasicAuth("root", "Milvus") - resp, err := client.Do(req) - suite.Nil(err) - defer resp.Body.Close() - suite.Equal(http.StatusInternalServerError, resp.StatusCode) - - body, _ := io.ReadAll(resp.Body) - var parsed map[string]string - err = json.Unmarshal(body, &parsed) - suite.NoError(err) - suite.Contains(parsed["msg"], "failed to execute expression") - suite.Contains(parsed["msg"], "unexpected token EOF") - }) - - suite.Run("enabled_on_proxy_non_root_user_without_privilege", func() { - // Non-root user with valid credentials but without PrivilegeExpr privilege - // should get 403 Forbidden - paramtable.Get().Save("common.security.exprEnabled", "true") - paramtable.Get().Save("common.security.exprAuthMode", ExprAuthModeRBAC) - paramtable.Get().Save("common.security.authorizationEnabled", "true") - paramtable.Get().Save("common.security.rootShouldBindRole", "false") - expr.Register("proxy", "mock_proxy") - privilege.InitPrivilegeGroups() - - // Register mock password verify function - RegisterPasswordVerifyFunc(func(ctx context.Context, username, password string) bool { - return (username == "root" && password == "Milvus") || - (username == "admin" && password == "admin123") - }) - - // Register mock getUserRoleFunc - admin has role1 but no PrivilegeExpr - RegisterGetUserRoleFunc(func(username string) ([]string, error) { - if username == "admin" { - return []string{"role1"}, nil - } - return []string{}, nil - }) - - loadPrivilegePoliciesForTest(suite.T(), []string{ - funcutil.PolicyForPrivilege("role1", commonpb.ObjectType_Collection.String(), "col1", commonpb.ObjectPrivilege_PrivilegeLoad.String(), "default"), - }) - defer privilege.CleanPrivilegeCache() - - // Non-root user with valid credentials but without PrivilegeExpr - should fail with 403 - url := "http://localhost:" + DefaultListenPort + ExprPath + "?code=foo" - client := http.Client{} - req, _ := http.NewRequest(http.MethodGet, url, nil) - req.SetBasicAuth("admin", "admin123") - resp, err := client.Do(req) - suite.Nil(err) - defer resp.Body.Close() - suite.Equal(http.StatusForbidden, resp.StatusCode) - body, _ := io.ReadAll(resp.Body) - suite.True(strings.Contains(string(body), "permission denied")) - }) - - suite.Run("enabled_on_proxy_non_root_user_with_privilege", func() { - // Non-root user with valid credentials and PrivilegeExpr privilege - // should get 200 OK - paramtable.Get().Save("common.security.exprEnabled", "true") - paramtable.Get().Save("common.security.exprAuthMode", ExprAuthModeRBAC) - paramtable.Get().Save("common.security.authorizationEnabled", "true") - paramtable.Get().Save("common.security.rootShouldBindRole", "false") - expr.Register("proxy", "mock_proxy") - privilege.InitPrivilegeGroups() - - // Register mock password verify function - RegisterPasswordVerifyFunc(func(ctx context.Context, username, password string) bool { - return (username == "root" && password == "Milvus") || - (username == "admin" && password == "admin123") - }) - - // Register mock getUserRoleFunc - admin has role1 with PrivilegeExpr - RegisterGetUserRoleFunc(func(username string) ([]string, error) { - if username == "admin" { - return []string{"role1"}, nil - } - return []string{}, nil - }) - - loadPrivilegePoliciesForTest(suite.T(), []string{ - funcutil.PolicyForPrivilege("role1", commonpb.ObjectType_Global.String(), "*", util.PrivilegeExpr, "default"), - }) - defer privilege.CleanPrivilegeCache() - - // Non-root user with valid credentials and PrivilegeExpr - should succeed with 200 - url := "http://localhost:" + DefaultListenPort + ExprPath + "?code=foo" - client := http.Client{} - req, _ := http.NewRequest(http.MethodGet, url, nil) - req.SetBasicAuth("admin", "admin123") - resp, err := client.Do(req) - suite.Nil(err) - defer resp.Body.Close() - suite.Equal(http.StatusOK, resp.StatusCode) - body, _ := io.ReadAll(resp.Body) - suite.True(strings.Contains(string(body), "hello")) - }) - - // Reset config - paramtable.Get().Save("common.security.exprEnabled", "false") - paramtable.Get().Save("common.security.exprAuthMode", ExprAuthModeRootOnly) - paramtable.Get().Save("common.security.authorizationEnabled", "false") - paramtable.Get().Save("common.security.rootShouldBindRole", "false") -} - func TestHTTPServerSuite(t *testing.T) { suite.Run(t, new(HTTPServerTestSuite)) } diff --git a/internal/http/static/index.html b/internal/http/static/index.html deleted file mode 100644 index 49551383c9..0000000000 --- a/internal/http/static/index.html +++ /dev/null @@ -1,314 +0,0 @@ - - - - - - Milvus Expr Executor - - - -
Milvus Expr Executor
-
-
-

Input

- -
-
- -
-
-
- -
- -
-
-

Result

-

-    
-
-
-

Parameter meaning: -The auth parameter is etcd root path, and the code parameter is expr execution expression.
-Injection object: -Currently, the objects injected by expr include: param, proxy, rootcoord, querycoord, datacoord, quernode, datanode. You can use this tool to get the running value of the object.
-Usage example: -1. Get a configuration: param.CommonCfg.GracefulTime.GetValue() -2. Get a property value in the proxy object: proxy.address -3. Determine whether a graph exists in the datanode: datanode.flowgraphManager.HasFlowgraph("aaa")
-Limitations: -1. Variables cannot be modified. -2. Methods with non-basic type parameters cannot be executed. -3. Functions with multiple return values cannot be chained.

-
- - - \ No newline at end of file diff --git a/internal/http/static_view.go b/internal/http/static_view.go deleted file mode 100644 index cb8e941ffe..0000000000 --- a/internal/http/static_view.go +++ /dev/null @@ -1,33 +0,0 @@ -/* - * Licensed to the LF AI & Data foundation under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package http - -import ( - "embed" - "io/fs" - "net/http" -) - -//go:embed static/* -var staticDir embed.FS - -func GetStaticHandler() http.Handler { - innerDir, _ := fs.Sub(staticDir, "static") - return http.StripPrefix("/static/", http.FileServer(http.FS(innerDir))) -} diff --git a/internal/proxy/meta_cache.go b/internal/proxy/meta_cache.go index 7b0c790b25..fa258fe3f0 100644 --- a/internal/proxy/meta_cache.go +++ b/internal/proxy/meta_cache.go @@ -25,7 +25,6 @@ import ( "github.com/milvus-io/milvus/internal/proxy/privilege" "github.com/milvus-io/milvus/internal/types" "github.com/milvus-io/milvus/pkg/v3/mlog" - "github.com/milvus-io/milvus/pkg/v3/util/expr" ) // Cache is the interface for system metadata cache. @@ -67,7 +66,6 @@ func initMetaCache(ctx context.Context, mixCoord types.MixCoordClient) (Cache, e if err != nil { return nil, err } - expr.Register("cache", metaCache) err = privilege.InitPrivilegeCache(ctx, mixCoord) if err != nil { diff --git a/internal/proxy/proxy.go b/internal/proxy/proxy.go index f4bfbab397..766f636649 100644 --- a/internal/proxy/proxy.go +++ b/internal/proxy/proxy.go @@ -40,7 +40,6 @@ import ( "github.com/milvus-io/milvus/pkg/v3/metrics" "github.com/milvus-io/milvus/pkg/v3/mlog" "github.com/milvus-io/milvus/pkg/v3/proto/internalpb" - "github.com/milvus-io/milvus/pkg/v3/util/expr" "github.com/milvus-io/milvus/pkg/v3/util/merr" "github.com/milvus-io/milvus/pkg/v3/util/metricsinfo" "github.com/milvus-io/milvus/pkg/v3/util/paramtable" @@ -139,7 +138,6 @@ func NewProxy(ctx context.Context, factory dependency.Factory) (*Proxy, error) { slowQueries: expirable.NewLRU[Timestamp, *metricsinfo.SlowQuery](20, nil, time.Minute*15), } node.UpdateStateCode(commonpb.StateCode_Abnormal) - expr.Register("proxy", node) hookutil.SetHook(connection.GetManager()) hookutil.InitOnceHook() mlog.Debug(ctx, "create a new Proxy instance", mlog.Any("state", node.stateCode.Load())) diff --git a/internal/querycoordv2/server.go b/internal/querycoordv2/server.go index a7a5031398..f40b2e5422 100644 --- a/internal/querycoordv2/server.go +++ b/internal/querycoordv2/server.go @@ -58,7 +58,6 @@ import ( "github.com/milvus-io/milvus/pkg/v3/metrics" "github.com/milvus-io/milvus/pkg/v3/mlog" "github.com/milvus-io/milvus/pkg/v3/util" - "github.com/milvus-io/milvus/pkg/v3/util/expr" "github.com/milvus-io/milvus/pkg/v3/util/merr" "github.com/milvus-io/milvus/pkg/v3/util/metricsinfo" "github.com/milvus-io/milvus/pkg/v3/util/paramtable" @@ -150,7 +149,6 @@ func NewQueryCoord(ctx context.Context) (*Server, error) { } server.UpdateStateCode(commonpb.StateCode_Abnormal) server.queryNodeCreator = session.DefaultQueryNodeCreator - expr.Register("querycoord", server) return server, nil } diff --git a/internal/querynodev2/server.go b/internal/querynodev2/server.go index f8bde40837..286a5e40bd 100644 --- a/internal/querynodev2/server.go +++ b/internal/querynodev2/server.go @@ -72,7 +72,6 @@ import ( "github.com/milvus-io/milvus/pkg/v3/mq/msgdispatcher" "github.com/milvus-io/milvus/pkg/v3/proto/datapb" "github.com/milvus-io/milvus/pkg/v3/streaming/util/message" - "github.com/milvus-io/milvus/pkg/v3/util/expr" "github.com/milvus-io/milvus/pkg/v3/util/lifetime" "github.com/milvus-io/milvus/pkg/v3/util/lock" "github.com/milvus-io/milvus/pkg/v3/util/merr" @@ -164,7 +163,6 @@ func NewQueryNode(ctx context.Context, factory dependency.Factory) *QueryNode { distDeltaTracker: newDataDistributionDeltaTracker(), } - expr.Register("querynode", node) return node } diff --git a/internal/rootcoord/ddl_callbacks_rbac_privilege.go b/internal/rootcoord/ddl_callbacks_rbac_privilege.go index 1536900f01..afbd64a70f 100644 --- a/internal/rootcoord/ddl_callbacks_rbac_privilege.go +++ b/internal/rootcoord/ddl_callbacks_rbac_privilege.go @@ -39,6 +39,10 @@ func (c *Core) broadcastOperatePrivilege(ctx context.Context, in *milvuspb.Opera return merr.Wrap(err, "failed to operate privilege common check") } privName := in.Entity.Grantor.Privilege.Name + if in.Type == milvuspb.OperatePrivilegeType_Grant && isDeprecatedExprPrivilege(privName) { + return merr.WrapErrParameterInvalidMsg( + "privilege %q is deprecated: the /expr endpoint it granted access to has been removed", privName) + } switch in.Version { case "v2": if err := c.isValidPrivilegeV2(ctx, privName); err != nil { @@ -84,6 +88,18 @@ func (c *Core) broadcastOperatePrivilege(ctx context.Context, in *milvuspb.Opera return err } +// isDeprecatedExprPrivilege reports whether name refers to the Expr privilege, +// in either its API form ("Expr") or its metastore form ("PrivilegeExpr"). +// +// The /expr endpoint that this privilege guarded has been removed, so granting +// it confers nothing. New grants are rejected, but the name stays valid +// everywhere else so that grants created on releases which shipped the endpoint +// can still be listed and revoked -- grant and revoke otherwise share one +// validation path, and rejecting both would strand them. +func isDeprecatedExprPrivilege(name string) bool { + return name == util.PrivilegeExpr || util.PrivilegeNameForMetastore(name) == util.PrivilegeExpr +} + func (c *DDLCallback) alterPrivilegeV2AckCallback(ctx context.Context, result message.BroadcastResultAlterPrivilegeMessageV2) error { return executeOperatePrivilegeTaskSteps(ctx, c.Core, result.Message.Header().Entity, milvuspb.OperatePrivilegeType_Grant) } diff --git a/internal/rootcoord/ddl_callbacks_rbac_privilege_test.go b/internal/rootcoord/ddl_callbacks_rbac_privilege_test.go index 8736dcafbc..1b3c1e7e09 100644 --- a/internal/rootcoord/ddl_callbacks_rbac_privilege_test.go +++ b/internal/rootcoord/ddl_callbacks_rbac_privilege_test.go @@ -24,6 +24,7 @@ import ( "github.com/milvus-io/milvus-proto/go-api/v3/milvuspb" "github.com/milvus-io/milvus/pkg/v3/proto/internalpb" + "github.com/milvus-io/milvus/pkg/v3/util" "github.com/milvus-io/milvus/pkg/v3/util/merr" ) @@ -182,3 +183,16 @@ func TestDDLCallbacksRBACPrivilegeGroup(t *testing.T) { }) require.NoError(t, merr.CheckRPCCall(status, err)) } + +func TestIsDeprecatedExprPrivilege(t *testing.T) { + // Both the API form and the metastore form must be recognized, since the + // grant path is reached with either depending on the request version. + require.True(t, isDeprecatedExprPrivilege("Expr")) + require.True(t, isDeprecatedExprPrivilege(util.PrivilegeExpr)) + + // Live privileges must not be caught by the deprecation guard. + require.False(t, isDeprecatedExprPrivilege("Load")) + require.False(t, isDeprecatedExprPrivilege("CreateCollection")) + require.False(t, isDeprecatedExprPrivilege("")) + require.False(t, isDeprecatedExprPrivilege("ExprSomethingElse")) +} diff --git a/internal/rootcoord/root_coord.go b/internal/rootcoord/root_coord.go index f717ebd694..16486987ee 100644 --- a/internal/rootcoord/root_coord.go +++ b/internal/rootcoord/root_coord.go @@ -66,7 +66,6 @@ import ( "github.com/milvus-io/milvus/pkg/v3/util" "github.com/milvus-io/milvus/pkg/v3/util/commonpbutil" "github.com/milvus-io/milvus/pkg/v3/util/contextutil" - "github.com/milvus-io/milvus/pkg/v3/util/expr" "github.com/milvus-io/milvus/pkg/v3/util/funcutil" "github.com/milvus-io/milvus/pkg/v3/util/merr" "github.com/milvus-io/milvus/pkg/v3/util/metricsinfo" @@ -202,7 +201,6 @@ func NewCore(c context.Context, factory dependency.Factory) (*Core, error) { core.UpdateStateCode(commonpb.StateCode_Abnormal) core.SetProxyCreator(proxyutil.DefaultProxyCreator) - expr.Register("rootcoord", core) return core, nil } diff --git a/pkg/go.mod b/pkg/go.mod index e88cd2d0d4..a71ed33ec4 100644 --- a/pkg/go.mod +++ b/pkg/go.mod @@ -17,7 +17,6 @@ require ( github.com/confluentinc/confluent-kafka-go v1.9.1 github.com/containerd/cgroups/v3 v3.0.3 github.com/dave/jennifer v1.7.1 - github.com/expr-lang/expr v1.15.7 github.com/huaweicloud/huaweicloud-sdk-go-v3 v0.1.174 github.com/jolestar/go-commons-pool/v2 v2.1.2 github.com/json-iterator/go v1.1.13-0.20220915233716-71ac16282d12 @@ -246,7 +245,6 @@ require ( replace ( github.com/bketelsen/crypt => github.com/bketelsen/crypt v0.0.4 // Fix security alert for core-os/etcd - github.com/expr-lang/expr => github.com/SimFG/expr v0.0.0-20250513112851-9b981e8400b9 github.com/go-kit/kit => github.com/go-kit/kit v0.1.0 github.com/ianlancetaylor/cgosymbolizer => github.com/milvus-io/cgosymbolizer v0.0.0-20260810014817-f3b338cf6824 github.com/tecbot/gorocksdb => github.com/milvus-io/gorocksdb v0.0.0-20220624081344-8c5f4212846b // indirect diff --git a/pkg/go.sum b/pkg/go.sum index 45e20aba23..6ec90b6624 100644 --- a/pkg/go.sum +++ b/pkg/go.sum @@ -99,8 +99,6 @@ github.com/Microsoft/go-winio v0.6.2/go.mod h1:yd8OoFMLzJbo9gZq8j5qaps8bJ9aShtEA github.com/RoaringBitmap/roaring/v2 v2.8.0 h1:y1rdtixfXvaITKzkfiKvScI0hlBJHe9sfzJp8cgeM7w= github.com/RoaringBitmap/roaring/v2 v2.8.0/go.mod h1:FiJcsfkGje/nZBZgCu0ZxCPOKD/hVXDS2dXi7/eUFE0= github.com/Shopify/goreferrer v0.0.0-20181106222321-ec9c9a553398/go.mod h1:a1uqRtAwp2Xwc6WNPJEufxJ7fx3npB4UV/JOLmbu5I0= -github.com/SimFG/expr v0.0.0-20250513112851-9b981e8400b9 h1:eXnmJhsHt8m6NU3IJ19UthXJ8JK6e3tmfN07nym3BXs= -github.com/SimFG/expr v0.0.0-20250513112851-9b981e8400b9/go.mod h1:8/vRC7+7HBzESEqt5kKpYXxrxkr31SaO8r40VO/1IT4= github.com/actgardner/gogen-avro/v10 v10.1.0/go.mod h1:o+ybmVjEa27AAr35FRqU98DJu1fXES56uXniYFv4yDA= github.com/actgardner/gogen-avro/v10 v10.2.1/go.mod h1:QUhjeHPchheYmMDni/Nx7VB0RsT/ee8YIgGY/xpEQgQ= github.com/actgardner/gogen-avro/v9 v9.1.0/go.mod h1:nyTj6wPqDJoxM3qdnjcLv+EnMDSDFqE0qDpva2QRmKc= diff --git a/pkg/util/constant.go b/pkg/util/constant.go index 0c81ea05f0..89a1fc5438 100644 --- a/pkg/util/constant.go +++ b/pkg/util/constant.go @@ -74,6 +74,9 @@ const ( PreserveFieldIdsKey = "preserve_field_ids" + // PrivilegeExpr is deprecated: the /expr endpoint it guarded has been removed. + // It is retained so that grants made on v3.0.0 stay revocable. See + // isPrivilegeNameForMetastoreDefined. PrivilegeExpr = "PrivilegeExpr" ) @@ -518,8 +521,10 @@ func isPrivilegeNameForMetastoreDefined(name string) bool { if _, ok := commonpb.ObjectPrivilege_value[name]; ok { return true } - // TODO: drop this special case once PrivilegeExpr is promoted to a proto enum value - // in milvus-io/milvus-proto (commonpb.ObjectPrivilege_PrivilegeExpr). + // PrivilegeExpr is deprecated and never became a proto enum value. It stays + // recognized here so that grants created on v3.0.0, the only release that + // shipped the /expr endpoint, remain listable and revocable. New grants are + // rejected in rootcoord. Drop this once no supported release has the endpoint. return name == PrivilegeExpr } diff --git a/pkg/util/expr/expr.go b/pkg/util/expr/expr.go deleted file mode 100644 index 9b5d06c074..0000000000 --- a/pkg/util/expr/expr.go +++ /dev/null @@ -1,109 +0,0 @@ -/* - * Licensed to the LF AI & Data foundation under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package expr - -import ( - "context" - "fmt" - "unsafe" - - "github.com/expr-lang/expr" - "github.com/expr-lang/expr/vm" - "google.golang.org/protobuf/proto" - - "github.com/milvus-io/milvus/pkg/v3/mlog" - "github.com/milvus-io/milvus/pkg/v3/util/merr" - "github.com/milvus-io/milvus/pkg/v3/util/paramtable" -) - -const ( - // AuthBypass is a special auth value that skips the auth check. - // This should only be used when authentication has already been verified externally. - AuthBypass = "__bypass__" -) - -var ( - v *vm.VM - env map[string]any - authKey string -) - -func Init() { - v = &vm.VM{} - env = map[string]any{ - "ctx": context.TODO(), - "objSize": func(p any) int { - message, ok := p.(proto.Message) - if !ok { - return int(unsafe.Sizeof(p)) - } - return proto.Size(message) - }, - } - authKey = paramtable.Get().EtcdCfg.RootPath.GetValue() -} - -func Register(key string, value any) { - if env != nil { - env[key] = value - } -} - -// HasRegistered checks if a key has been registered in the expr environment. -// This is useful for determining which component is running (e.g., checking if "proxy" is registered). -func HasRegistered(key string) bool { - if env == nil { - return false - } - _, ok := env[key] - return ok -} - -func Exec(code, auth string) (res string, err error) { - defer func() { - if e := recover(); e != nil { - err = merr.WrapErrParameterInvalidMsg("panic: %v", e) - } - }() - if v == nil { - return "", merr.WrapErrParameterInvalidMsg("the expr isn't inited") - } - if code == "" { - return "", merr.WrapErrParameterInvalidMsg("the expr code is empty") - } - if auth == "" { - return "", merr.WrapErrParameterInvalidMsg("the expr auth is empty") - } - // Allow bypass when authentication has been verified externally (e.g., by HTTP handler) - if auth != AuthBypass && authKey != auth { - return "", merr.WrapErrParameterInvalidMsg("the expr auth is invalid") - } - program, err := expr.Compile(code, expr.Env(env), expr.WithContext("ctx")) - if err != nil { - mlog.Warn(context.TODO(), "expr compile failed", mlog.String("code", code), mlog.Err(err)) - return "", err - } - - output, err := v.Run(program, env) - if err != nil { - mlog.Warn(context.TODO(), "expr run failed", mlog.String("code", code), mlog.Err(err)) - return "", err - } - return fmt.Sprintf("%v", output), nil -} diff --git a/pkg/util/expr/expr_test.go b/pkg/util/expr/expr_test.go deleted file mode 100644 index e04eef54d4..0000000000 --- a/pkg/util/expr/expr_test.go +++ /dev/null @@ -1,131 +0,0 @@ -/* - * Licensed to the LF AI & Data foundation under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package expr - -import ( - "context" - "fmt" - "testing" - "unsafe" - - "github.com/stretchr/testify/assert" - "google.golang.org/protobuf/proto" - - "github.com/milvus-io/milvus-proto/go-api/v3/milvuspb" - "github.com/milvus-io/milvus/pkg/v3/util/paramtable" -) - -type FooID = int64 - -func TestExec(t *testing.T) { - paramtable.Init() - t.Run("not init", func(t *testing.T) { - _, err := Exec("1+1", "by-dev") - assert.Error(t, err) - }) - Init() - Register("foo", "hello") - Register("FuncWithContext", func(ctx context.Context, i FooID) int { - return int(100 + i) - }) - mockMessage := &milvuspb.UserEntity{Name: "foo"} - Register("GetMockMessage", func() proto.Message { - return mockMessage - }) - - t.Run("empty code", func(t *testing.T) { - _, err := Exec("", "by-dev") - assert.Error(t, err) - }) - - t.Run("empty auth", func(t *testing.T) { - _, err := Exec("1+1", "") - assert.Error(t, err) - }) - - t.Run("invalid auth", func(t *testing.T) { - _, err := Exec("1+1", "000") - assert.Error(t, err) - }) - - t.Run("invalid code", func(t *testing.T) { - _, err := Exec("1+", "by-dev") - assert.Error(t, err) - }) - - t.Run("valid code", func(t *testing.T) { - out, err := Exec("foo", "by-dev") - assert.NoError(t, err) - assert.Equal(t, "hello", out) - }) - - t.Run("context function", func(t *testing.T) { - out, err := Exec("FuncWithContext(100)", "by-dev") - assert.NoError(t, err) - assert.Equal(t, "200", out) - }) - - innerSize := func(p any) int { - message, ok := p.(proto.Message) - if !ok { - return int(unsafe.Sizeof(p)) - } - return proto.Size(message) - } - - t.Run("size", func(t *testing.T) { - out, err := Exec("objSize(1)", "by-dev") - assert.NoError(t, err) - assert.Equal(t, fmt.Sprintf("%d", innerSize(1)), out) - }) - - t.Run("proto size", func(t *testing.T) { - out, err := Exec("objSize(GetMockMessage())", "by-dev") - assert.NoError(t, err) - assert.Equal(t, fmt.Sprintf("%d", innerSize(mockMessage)), out) - }) - - t.Run("auth bypass", func(t *testing.T) { - // AuthBypass should allow execution without checking the auth key - out, err := Exec("foo", AuthBypass) - assert.NoError(t, err) - assert.Equal(t, "hello", out) - }) -} - -func TestHasRegistered(t *testing.T) { - // Before init, should return false - env = nil - assert.False(t, HasRegistered("foo")) - - // After init - Init() - Register("testKey", "testValue") - - // Registered key should return true - assert.True(t, HasRegistered("testKey")) - - // Non-registered key should return false - assert.False(t, HasRegistered("nonExistentKey")) - - // Check for "proxy" key (used to detect Proxy node) - assert.False(t, HasRegistered("proxy")) - Register("proxy", "mock_proxy") - assert.True(t, HasRegistered("proxy")) -} diff --git a/pkg/util/paramtable/component_param.go b/pkg/util/paramtable/component_param.go index f331d82028..d2e9bb5b81 100644 --- a/pkg/util/paramtable/component_param.go +++ b/pkg/util/paramtable/component_param.go @@ -286,8 +286,6 @@ type commonConfig struct { DefaultRootPassword ParamItem `refreshable:"false"` RootShouldBindRole ParamItem `refreshable:"true"` EnablePublicPrivilege ParamItem `refreshable:"false"` - ExprEnabled ParamItem `refreshable:"false"` - ExprAuthMode ParamItem `refreshable:"false"` ClusterName ParamItem `refreshable:"false"` @@ -1012,26 +1010,6 @@ Large numeric passwords require double quotes to avoid yaml parsing precision is } p.EnablePublicPrivilege.Init(base.mgr) - p.ExprEnabled = ParamItem{ - Key: "common.security.exprEnabled", - Version: "2.6.0", - DefaultValue: "false", - Doc: "Whether to enable the /expr endpoint for debugging.", - Export: true, - } - p.ExprEnabled.Init(base.mgr) - - p.ExprAuthMode = ParamItem{ - Key: "common.security.exprAuthMode", - Version: "2.6.0", - DefaultValue: "rootOnly", - Doc: "Authentication mode for the /expr endpoint. Valid values: rootOnly, rbac. " + - "rootOnly accepts only the root credentials via HTTP Basic Auth. " + - "rbac requires common.security.authorizationEnabled=true and grants access to any user holding the Expr privilege.", - Export: true, - } - p.ExprAuthMode.Init(base.mgr) - p.ClusterName = ParamItem{ Key: "common.cluster.name", Version: "2.0.0",