diff --git a/README.md b/README.md index 4fee762..73ee9a0 100644 --- a/README.md +++ b/README.md @@ -180,6 +180,7 @@ cors: - Reads one JSON object per line from each file. Ids, `menu_id`, `parent_id`, the nest columns and `sort_order` are kept; both id sequences are moved past the copied ids with `setval`. - Types are mapped by `console.DefaultTypeMap` (`link` to `url`, `blog_categories` to `all-journal-categories`); repeatable `--map old=new` adds or overrides a mapping, and `--map old=` drops the type. - Dropped with their subtree, one report line each: `page` entries (Winter.Pages is not ported), entries whose type is neither built-in, registered nor mapped, and registry entries whose reference is not among the type's current references. Every remapped entry is reported too. +- Kept entries are made to pass the admin save rules, one report line each: a URL `classes.AllowedURL` refuses is trimmed when only surrounding whitespace is wrong and cleared otherwise; a target other than `_self` or `_blank` is cleared; an entry whose parent is in another menu is copied as a top-level entry of its own menu (with its own children), so it is not left out of the public tree. - Tables that already hold rows are refused unless `--force`, which replaces them in the same transaction. `--dry-run` prints the report and writes nothing. An empty file writes nothing and reports zero. ## Winter adaptations diff --git a/console/import_winter.go b/console/import_winter.go index 7e4b1d0..f2245ba 100644 --- a/console/import_winter.go +++ b/console/import_winter.go @@ -9,6 +9,7 @@ import ( "strings" "time" + "git.golem15.com/golem15/sm-menu-plugin/classes" "git.golem15.com/golem15/sm-menu-plugin/models" "git.golem15.com/golem15/summercms/modules/backpack" "git.golem15.com/golem15/summercms/modules/bonfire" @@ -59,7 +60,12 @@ type WinterReport struct { // all-journal-categories, --map old=new on top); a page entry, an entry of a // type that is neither built-in nor registered, and a registry entry whose // reference is not among the type's current references are dropped with -// their subtree, and every dropped or remapped entry is reported. Tables +// their subtree, and every dropped or remapped entry is reported. The rules +// an admin save enforces are applied to the copies with a report line each: +// a URL that classes.AllowedURL refuses is trimmed when only surrounding +// whitespace is wrong and cleared otherwise, a target other than _self or +// _blank is cleared, and an entry whose parent is in another menu is copied +// as a top-level entry of its own menu. Tables // that already hold rows are refused without --force, which replaces them in // the same transaction. Both id sequences are moved past the copied ids. func ImportWinter(app *backpack.App) bonfire.Command { @@ -233,7 +239,8 @@ func mergedMaps(maps map[string]string) map[string]string { // export, when its parent is missing or dropped (with the whole subtree), // for a page type, for a type mapped to nothing, for a type that is neither // built-in nor registered, and for a registry type that takes references -// when its reference is not among them. +// when its reference is not among them. A kept entry's URL, target and +// parent are made to pass the admin save rules (see ImportWinter). func mapEntries(ctx context.Context, reg *signpost.Registry, entries []models.MenuEntry, menuIDs map[uint]bool, maps map[string]string, report *WinterReport) ([]models.MenuEntry, error) { byID := make(map[uint]*models.MenuEntry, len(entries)) for i := range entries { @@ -319,11 +326,43 @@ func mapEntries(ctx context.Context, reg *signpost.Registry, entries []models.Me report.Dropped++ continue } + report.Lines = append(report.Lines, adminRules(e, byID, oldType[e.ID])...) kept = append(kept, *e) } return kept, nil } +// adminRules makes a kept entry pass the rules an admin save enforces and +// returns a report line per change: a URL classes.AllowedURL refuses is +// trimmed when that is enough and cleared otherwise, a target +// classes.AllowedTarget refuses is cleared, and a parent in another menu is +// dropped so the entry is a top-level entry of its own menu (the public tree +// would leave it out, the reorder page would show it at the top). +func adminRules(e *models.MenuEntry, byID map[uint]*models.MenuEntry, typ string) []string { + var lines []string + label := describe(e, typ) + if url := models.Text(e.URL); !classes.AllowedURL(url) { + if trimmed := strings.TrimSpace(url); trimmed != url && classes.AllowedURL(trimmed) { + e.URL = &trimmed + lines = append(lines, fmt.Sprintf("%s: url '%s' trimmed to '%s'", label, url, trimmed)) + } else { + e.URL = nil + lines = append(lines, fmt.Sprintf("%s: url %q cleared (only http, https, mailto, tel, / and # links are served)", label, url)) + } + } + if target := models.Text(e.Target); !classes.AllowedTarget(target) { + e.Target = nil + lines = append(lines, fmt.Sprintf("%s: target '%s' cleared (only _self and _blank are allowed)", label, target)) + } + if e.ParentID != nil { + if parent, ok := byID[*e.ParentID]; ok && parent.MenuID != e.MenuID { + lines = append(lines, fmt.Sprintf("%s: copied as a top-level entry (its parent %d is in menu %d)", label, parent.ID, parent.MenuID)) + e.ParentID = nil + } + } + return lines +} + // describe names an entry in a report line by id, title, menu, exported // type and reference. func describe(e *models.MenuEntry, typ string) string { diff --git a/menu_api_test.go b/menu_api_test.go index acc488e..e4e6657 100644 --- a/menu_api_test.go +++ b/menu_api_test.go @@ -291,4 +291,35 @@ func TestMenuImport(t *testing.T) { if err := e.gdb.Model(&models.MenuEntry{}).Count(&count).Error; err != nil || count != 2 { t.Fatalf("refused imports changed the entries: %d %v", count, err) } + + // WR-06: the copies pass the admin save rules, with a report line each. + twoMenus := writeFile(t, dir, "two-menus.jsonl", `{"id": 1, "name": "Main", "code": "main", "is_active": 1}`+"\n"+`{"id": 2, "name": "Footer", "code": "footer", "is_active": 1}`+"\n") + ruled := writeFile(t, dir, "ruled.jsonl", + `{"id": 1, "menu_id": 1, "parent_id": null, "sort_order": 1, "type": "link", "title": "Evil", "url": "javascript:alert(1)", "target": "_parent", "is_published": 1}`+"\n"+ + `{"id": 2, "menu_id": 1, "parent_id": null, "sort_order": 2, "type": "link", "title": "Spaced", "url": " /spaced ", "target": "_blank", "is_published": 1}`+"\n"+ + `{"id": 3, "menu_id": 2, "parent_id": 2, "sort_order": 1, "type": "link", "title": "Elsewhere", "url": "/e", "is_published": 1}`+"\n"+ + `{"id": 4, "menu_id": 2, "parent_id": 3, "sort_order": 2, "type": "link", "title": "Under elsewhere", "url": "/u", "is_published": 1}`+"\n") + out, buf = testOutput() + report, err = console.ImportWinterFiles(ctx, e.gdb, reg, console.WinterOptions{MenusFile: twoMenus, EntriesFile: ruled, Force: true}, out) + if err != nil || report.Entries != 4 || report.Dropped != 0 { + t.Fatalf("ruled import: %+v %v\n%s", report, err, buf.String()) + } + for _, want := range []string{ + `entry 1 'Evil' (menu 1, type 'link'): url "javascript:alert(1)" cleared (only http, https, mailto, tel, / and # links are served)`, + "entry 1 'Evil' (menu 1, type 'link'): target '_parent' cleared (only _self and _blank are allowed)", + "entry 2 'Spaced' (menu 1, type 'link'): url ' /spaced ' trimmed to '/spaced'", + "entry 3 'Elsewhere' (menu 2, type 'link'): copied as a top-level entry (its parent 2 is in menu 1)", + } { + if !strings.Contains(buf.String(), want) { + t.Errorf("report lacks %q:\n%s", want, buf.String()) + } + } + kept = nil + if err := e.gdb.Order("id").Find(&kept).Error; err != nil || len(kept) != 4 { + t.Fatalf("ruled entries = %+v %v", kept, err) + } + if kept[0].URL != nil || kept[0].Target != nil || models.Text(kept[1].URL) != "/spaced" || models.Text(kept[1].Target) != "_blank" || + kept[2].ParentID != nil || kept[2].MenuID != 2 || kept[3].ParentID == nil || *kept[3].ParentID != 3 { + t.Fatalf("ruled entries = %+v", kept) + } }