From 28191437b9a61a7dfb534ac1e5aad941bef1d5be Mon Sep 17 00:00:00 2001 From: Stephan Renatus Date: Wed, 6 Jul 2022 17:20:10 +0200 Subject: [PATCH] server: check old policy path for bundle ownership (#4847) Before, we'd only check if the NEW policy path was owned by a bundle. Now, we'll also check if the to-be-updated policy is owned by a bundle. If so, return an error. Fixes #4846 Signed-off-by: Stephan Renatus (cherry picked from commit b2bf19f6b535cd7917f3bd56a86771da6236b029) --- server/server.go | 15 ++++++++++----- server/server_test.go | 7 +++++++ 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/server/server.go b/server/server.go index bf74190de6..39cecf3189 100644 --- a/server/server.go +++ b/server/server.go @@ -1982,7 +1982,7 @@ func (s *Server) v1PoliciesPut(w http.ResponseWriter, r *http.Request) { ctx := r.Context() vars := mux.Vars(r) - path, err := url.PathUnescape(vars["path"]) + id, err := url.PathUnescape(vars["path"]) if err != nil { writer.ErrorString(w, http.StatusBadRequest, types.CodeInvalidParameter, err) return @@ -2010,7 +2010,12 @@ func (s *Server) v1PoliciesPut(w http.ResponseWriter, r *http.Request) { return } - if bs, err := s.store.GetPolicy(ctx, txn, path); err != nil { + if err := s.checkPolicyIDScope(ctx, txn, id); err != nil && !storage.IsNotFound(err) { + s.abortAuto(ctx, txn, w, err) + return + } + + if bs, err := s.store.GetPolicy(ctx, txn, id); err != nil { if !storage.IsNotFound(err) { s.abortAuto(ctx, txn, w, err) return @@ -2026,7 +2031,7 @@ func (s *Server) v1PoliciesPut(w http.ResponseWriter, r *http.Request) { } m.Timer(metrics.RegoModuleParse).Start() - parsedMod, err := ast.ParseModule(path, string(buf)) + parsedMod, err := ast.ParseModule(id, string(buf)) m.Timer(metrics.RegoModuleParse).Stop() if err != nil { @@ -2057,7 +2062,7 @@ func (s *Server) v1PoliciesPut(w http.ResponseWriter, r *http.Request) { return } - modules[path] = parsedMod + modules[id] = parsedMod c := ast.NewCompiler(). SetErrorLimit(s.errLimit). @@ -2075,7 +2080,7 @@ func (s *Server) v1PoliciesPut(w http.ResponseWriter, r *http.Request) { m.Timer(metrics.RegoModuleCompile).Stop() - if err := s.store.UpsertPolicy(ctx, txn, path, buf); err != nil { + if err := s.store.UpsertPolicy(ctx, txn, id, buf); err != nil { s.abortAuto(ctx, txn, w, err) return } diff --git a/server/server_test.go b/server/server_test.go index 859fb92a8e..0dd449f033 100644 --- a/server/server_test.go +++ b/server/server_test.go @@ -1843,6 +1843,13 @@ func TestBundleScope(t *testing.T) { code: http.StatusBadRequest, resp: `{"code": "invalid_parameter", "message": "path a/b is owned by bundle \"test-bundle\""}`, }, + { + method: "PUT", + path: "/policies/someid", + body: `package other.path`, + code: http.StatusBadRequest, + resp: `{"code": "invalid_parameter", "message": "path x/y/z is owned by bundle \"test-bundle\""}`, + }, { method: "DELETE", path: "/policies/someid",