fix(16.2): WR-01 check the declared save before a field upload

The field upload route ran only protect, so the create form of a
controller without the create toolbar button could still write files to
the public bucket. It now refuses with 403 where the save route would:
create for {id} 0, update otherwise (operationDeclared).
This commit is contained in:
2026-10-10 16:42:28 +02:00
parent 9899ac61dd
commit 3158bb5b57
5 changed files with 50 additions and 4 deletions

View File

@@ -52,7 +52,7 @@ All paths are relative to `<prefix>/api/v1`. A controller ID `vendor.plugin.cont
| POST and DELETE `/account/avatar` | Upload the signed-in administrator's avatar (one multipart `file_data` part: a jpg, jpeg, png, gif or webp image of at most 2 MiB; anything else is a 422 on `avatar`, a body over the cap a 413) replacing the previous one, or remove it (also 200 when there is none). Both answer the `cabana.AdminProfile`. Needs any backend session. |
| GET `/navigation`, GET `/settings` | Navigation and settings entries the administrator may open; a settings entry's `controller` names the admin controller a link entry opens and is empty for a singleton form. |
| GET `/field-types` | The plugin field types (`pact.HasFormFieldTypes`) sorted by name, each with its `element`, `translatable` flag and the versioned URLs of its `scripts` and `styles` (`cabana.FieldTypesResponse`); an empty list when no plugin registers one. The SPA loads them after sign-in. Needs any backend session. |
| POST `/{vendor}/{plugin}/{controller}/{id}/fields/{field}/upload` | Store one multipart part named `file` through the `Upload` of the field's plugin field type and answer 201 `{url}` (`cabana.FieldUploadResponse`). `{id}` 0 is the create form and passes no record; any other id loads the record through the form scope (404 outside it). 404 for a field without a plugin type with `Upload` or whose context does not allow the operation, 413 over `http.body_limits.upload_bytes`, 422 on the field for a malformed body or a `pact.FormFieldError`. |
| POST `/{vendor}/{plugin}/{controller}/{id}/fields/{field}/upload` | Store one multipart part named `file` through the `Upload` of the field's plugin field type and answer 201 `{url}` (`cabana.FieldUploadResponse`). `{id}` 0 is the create form and passes no record; any other id loads the record through the form scope (404 outside it). 403 when the controller does not declare the save the upload belongs to (create for `{id}` 0, which needs the list's `create` toolbar button, update otherwise), as the save routes answer. 404 for a field without a plugin type with `Upload` or whose context does not allow the operation, 413 over `http.body_limits.upload_bytes`, 422 on the field for a malformed body or a `pact.FormFieldError`. |
| GET `/settings/{code}/schema`, GET and PUT `/settings/{code}` | Settings form schema, values and update. |
| POST `/markdown/preview` | Render `{markdown}` through `cabana.RenderMarkdown` for the form preview and answer `{html}`; a refused output is a 422 `validation_failed` on `markdown`. Needs any backend session. |
| GET `/{vendor}/{plugin}/{controller}/schema/list`, `.../schema/form`, `.../schema/relation/{name}` | Localized list, form and relation schemas. |

View File

@@ -401,9 +401,11 @@ func ftNotesMLType() pact.FormFieldType {
const ftBodyField = " body:\n type: notesml\n label: Body\n"
// ftMLPlugin has both types, a translatable body field and typed columns.
// ftMLPlugin has both types, a translatable body field, typed columns and
// the create toolbar button (the create form may upload).
func ftMLPlugin() ftPlugin {
fsys := ftFS(ftNotesField, ftBodyField)
fsys["controllers/notes/config_list.yaml"] = &fstest.MapFile{Data: []byte(ftListConfig + "toolbar:\n buttons: [create]\n")}
fsys["models/note/columns.yaml"] = &fstest.MapFile{Data: []byte("columns:\n title:\n label: Title\n notes:\n label: Notes\n type: notes\n body:\n label: Body\n type: notesml\n")}
return ftPlugin{id: "acme.demo", fsys: fsys, types: []pact.FormFieldType{ftNotesType(), ftNotesMLType()}}
}

View File

@@ -1225,9 +1225,16 @@ func TestFieldUpload(t *testing.T) {
t.Fatal(err)
}
// upService compiles the notes controller with the notes field given.
// The notes list declares the create toolbar button, so the create form
// may upload (WR-01).
upFS := func(notesField string) fstest.MapFS {
fsys := ftFS(notesField, ftBodyField)
fsys["controllers/notes/config_list.yaml"] = &fstest.MapFile{Data: []byte(ftListConfig + "toolbar:\n buttons: [create]\n")}
return fsys
}
upService := func(t *testing.T, notesField string, types ...pact.FormFieldType) *service {
t.Helper()
reg, err := ftCompile(ftPlugin{id: "acme.demo", fsys: ftFS(notesField, ftBodyField), types: types})
reg, err := ftCompile(ftPlugin{id: "acme.demo", fsys: upFS(notesField), types: types})
if err != nil {
t.Fatalf("compile: %v", err)
}
@@ -1256,6 +1263,33 @@ func TestFieldUpload(t *testing.T) {
}
})
// WR-01: the upload route refuses where the save route would.
t.Run("403 for the create form of a controller that declares no create", func(t *testing.T) {
calls := 0
counting := ftNotesType()
upload := counting.Upload
counting.Upload = func(ctx context.Context, in pact.FormFieldUpload) (pact.FormFieldUploadResult, error) {
calls++
return upload(ctx, in)
}
reg, err := ftCompile(ftPlugin{id: "acme.demo", fsys: ftFS(ftNotesField, ftBodyField), types: []pact.FormFieldType{counting, ftNotesMLType()}})
if err != nil {
t.Fatalf("compile: %v", err)
}
cc, ok := reg.Get("acme.demo.notes")
if !ok || cc.operationDeclared("create") {
t.Fatalf("fixture: the notes controller must not declare create")
}
noCreate := &service{app: app, reg: reg, uploadBytes: 4096}
rec := upCall(t, noCreate, "0", "notes", file, admin)
if rec.Code != http.StatusForbidden || calls != 0 {
t.Fatalf("status=%d calls=%d body=%s", rec.Code, calls, rec.Body.String())
}
if rec := upCall(t, svc, "0", "notes", file, admin); rec.Code != http.StatusCreated {
t.Fatalf("with the create button: status=%d body=%s", rec.Code, rec.Body.String())
}
})
t.Run("404 for a core field, a type without Upload, an unknown field and a context refusal", func(t *testing.T) {
for _, field := range []string{"title", "body", "missing"} {
if rec := upCall(t, svc, "0", field, file, admin); rec.Code != http.StatusNotFound {

View File

@@ -16,6 +16,8 @@ import (
// an element of a plugin field type uploads, stored by the type's Upload.
// cabana owns the route: the backend guard and the X-Requested-With check
// (mounted with requireAjax), the controller's permissions (protect), the
// operation the form would save (create for {id} 0, update otherwise), which
// the controller must declare as the save routes require (403 otherwise), the
// field and its context, the record scope and the body cap. {id} 0 is the
// create form and passes no record; any other id loads the record through
// the controller's FormExtendQuery scope inside a transaction, and a record
@@ -29,6 +31,14 @@ func (s *service) fieldUpload(w http.ResponseWriter, r *http.Request) {
writeCRUDError(w, err)
return
}
// The upload belongs to a save: refuse it where the save route would.
op := "update"
if id == 0 {
op = "create"
}
if !s.operationDeclared(w, r, cc, op) {
return
}
field, ok := uploadField(cc, r.PathValue("field"), id)
if !ok {
writeNotFound(w, r)