fix(17): WR-06 apply the admin save rules in menu:import-winter

The importer copied URLs, targets and cross-menu parents as exported, so
a javascript: URL or a _parent target broke the first admin save of the
row, and an entry under a parent in another menu vanished from the
public tree. Kept entries now get a trimmed or cleared URL, a cleared
target and a top-level place in their own menu, each with a report
line.
This commit is contained in:
2026-10-12 21:39:57 +02:00
parent 64215f2bf3
commit 9c0f833878
3 changed files with 73 additions and 2 deletions

View File

@@ -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

View File

@@ -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 {

View File

@@ -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)
}
}