From 5471b66b8f7c63e3e860c06714d84b75006d1e08 Mon Sep 17 00:00:00 2001 From: Jakub Zych Date: Tue, 27 Oct 2026 15:22:21 +0100 Subject: [PATCH] fix(32): CR-02 gate field type uploads on extension visibility The plugin field-type upload route resolved a form extension's field without checking the extension's Permissions and Visible, so an administrator who cannot see the extension could still run its Upload. The route now answers 404 for them, like the extension's widget actions and partials. --- docs/backend/admin-controllers.md | 2 +- modules/cabana/field_type_upload.go | 14 +++- .../cabana/form_extensions_postgres_test.go | 76 +++++++++++++++++++ 3 files changed, 90 insertions(+), 2 deletions(-) diff --git a/docs/backend/admin-controllers.md b/docs/backend/admin-controllers.md index 7d3e999c..f4b3e4a7 100644 --- a/docs/backend/admin-controllers.md +++ b/docs/backend/admin-controllers.md @@ -616,7 +616,7 @@ An extension adds value fields (`text`, `textarea`, `number`, `checkbox`, `switc ### Permissions, widgets and partials -`Permissions` (backend permission codes some plugin registers) and `Visible` (asked per request) decide who sees the extension. An administrator who fails either gets none of its fields in the form schema or the record; a save drops their posted keys for it and does not call its `Save`, so stored values stay as they are. Its widget actions and partials answer 404 for them. +`Permissions` (backend permission codes some plugin registers) and `Visible` (asked per request) decide who sees the extension. An administrator who fails either gets none of its fields in the form schema or the record; a save drops their posted keys for it and does not call its `Save`, so stored values stay as they are. Its widget actions, partials and field type uploads answer 404 for them. A `widget` field's tag uses the contributing plugin's prefix (`acme-extras-...`), its `action` names one of the extension's `Actions` (a name the owner already registers stops the boot), and its `fill` keys name value fields of the same extension. Its element script comes from one of the plugin's field types (loaded on every admin page) or from the plugin's own `pact.AdminClientAssets`, which cabana adds to the target form's assets. A `partial` field renders `_.htm` from `Dir` in the contributing plugin's `AdminFS` through `PartialData`, with the same view model rules as `pact.AdminPartialData`; a partial name the owner already uses stops the boot. diff --git a/modules/cabana/field_type_upload.go b/modules/cabana/field_type_upload.go index 606a540b..afc32983 100644 --- a/modules/cabana/field_type_upload.go +++ b/modules/cabana/field_type_upload.go @@ -8,6 +8,7 @@ import ( "net/http" "strings" + "git.golem15.com/golem15/summercms/modules/bouncer" "git.golem15.com/golem15/summercms/modules/lagoon" "git.golem15.com/golem15/summercms/modules/pact" "gorm.io/gorm" @@ -28,7 +29,8 @@ import ( // the record transaction fails after Upload or the element stops waiting; // Upload stores under a name that is safe to leave behind. It answers // 201 {data: {url}}; a pact.FormFieldError is a 422 on the field, any other -// Upload error the opaque 500. +// Upload error the opaque 500. A form extension's field answers 404 to an +// administrator who does not see the extension. func (s *service) fieldUpload(w http.ResponseWriter, r *http.Request) { s.protect(w, r, func(cc *CompiledController) { id, err := pathID(r) @@ -49,6 +51,16 @@ func (s *service) fieldUpload(w http.ResponseWriter, r *http.Request) { writeNotFound(w, r) return } + // D-91: a form extension's field exists only for an administrator + // who sees the extension (its Permissions and Visible), as on its + // widget actions and partials. + if ext := cc.extensionOf(field.Name); ext != nil { + principal, _ := bouncer.User(r.Context()) + if !extensionVisible(r.Context(), principal, ext) { + writeNotFound(w, r) + return + } + } db, err := s.db() if err != nil { WriteError(w, http.StatusInternalServerError, "error", msgServerError) diff --git a/modules/cabana/form_extensions_postgres_test.go b/modules/cabana/form_extensions_postgres_test.go index 0285f8e9..de461df2 100644 --- a/modules/cabana/form_extensions_postgres_test.go +++ b/modules/cabana/form_extensions_postgres_test.go @@ -6,6 +6,7 @@ import ( "encoding/json" "errors" "io/fs" + "mime/multipart" "net/http" "net/http/httptest" "strconv" @@ -598,3 +599,78 @@ func TestFormExtensionRequired(t *testing.T) { t.Fatalf("editor update: status=%d body=%s", rec.Code, rec.Body.String()) } } + +// TestFormExtensionUpload (CR-02): the field upload route of an extension's +// plugin field type answers 404 to an administrator who does not see the +// extension, and its Upload never runs for them. +func TestFormExtensionUpload(t *testing.T) { + var uploads int + fileType := pact.FormFieldType{ + Name: "extrafile", Element: "acme-extra-file", Scripts: []string{"assets/js/extra.js"}, + Decode: func(_ context.Context, in pact.FormFieldValue) (pact.FormFieldDecoded, error) { + return pact.FormFieldDecoded{Value: in.Value}, nil + }, + Upload: func(_ context.Context, in pact.FormFieldUpload) (pact.FormFieldUploadResult, error) { + uploads++ + return pact.FormFieldUploadResult{URL: "/media/" + in.Filename}, nil + }, + } + extra := feContributor{ + id: "acme.extra", + fsys: fstest.MapFS{ + "extensions/posts.yaml": &fstest.MapFile{Data: []byte("fields:\n img:\n type: extrafile\n after: a\n")}, + "assets/js/extra.js": &fstest.MapFile{Data: []byte("// extra\n")}, + }, + types: []pact.FormFieldType{fileType}, + extensions: []pact.AdminActionExtension{{ + Controller: "acme.blog.posts", + Form: &pact.AdminFormExtension{Fields: "extensions/posts.yaml", Permissions: []string{"acme.extra.manage"}, Save: feNoopSave}, + }}, + } + env := newFEEnv(t, feOwner{fields: feOwnerFields}, extra) + seed := fePost{A: "A0"} + if err := env.db.Create(&seed).Error; err != nil { + t.Fatal(err) + } + upload := func(principal *bouncer.Principal, id string) *httptest.ResponseRecorder { + t.Helper() + var buf bytes.Buffer + mw := multipart.NewWriter(&buf) + part, err := mw.CreateFormFile("file", "a.png") + if err != nil { + t.Fatal(err) + } + if _, err := part.Write([]byte("png")); err != nil { + t.Fatal(err) + } + if err := mw.Close(); err != nil { + t.Fatal(err) + } + req := httptest.NewRequest(http.MethodPost, "/", &buf) + req.Header.Set("Content-Type", mw.FormDataContentType()) + req.Header.Set("X-Requested-With", "XMLHttpRequest") + for name, value := range map[string]string{"vendor": "acme", "plugin": "blog", "controller": "posts", "id": id, "field": "img"} { + req.SetPathValue(name, value) + } + req = req.WithContext(bouncer.WithUser(req.Context(), principal)) + rec := httptest.NewRecorder() + requireAjax(env.svc.fieldUpload)(rec, req) + return rec + } + id := strconv.FormatUint(uint64(seed.ID), 10) + for _, target := range []string{"0", id} { + if rec := upload(superUser(), target); rec.Code != http.StatusCreated || !strings.Contains(rec.Body.String(), "/media/a.png") { + t.Fatalf("visible upload to %s: status=%d body=%s", target, rec.Code, rec.Body.String()) + } + } + editor := &bouncer.Principal{ID: 2, Backend: true, PermissionGrants: map[string]bool{"acme.blog.access": true}} + before := uploads + for _, target := range []string{"0", id} { + if rec := upload(editor, target); rec.Code != http.StatusNotFound { + t.Fatalf("invisible upload to %s: status=%d body=%s", target, rec.Code, rec.Body.String()) + } + } + if uploads != before { + t.Fatalf("Upload ran for an administrator who does not see the extension") + } +}