From c8e0961740dd61560e2a35b8d20ce85b92e03774 Mon Sep 17 00:00:00 2001 From: Jakub Zych Date: Tue, 27 Oct 2026 15:35:02 +0100 Subject: [PATCH] fix: social images are validated on the API, MCP, batch and import paths (WR-16) Only the admin seoimage field refused values that are not a path or an http(s) URL; SetURLSEO, SaveRule and SaveSettings stored javascript:, data: or protocol-relative values, which /resolve then served as og:image. All three now refuse them with image.url_invalid. --- classes/owner.go | 3 +++ classes/rules.go | 8 ++++++-- classes/settings_write.go | 3 +++ mcp_test.go | 15 +++++++++++++++ 4 files changed, 27 insertions(+), 2 deletions(-) diff --git a/classes/owner.go b/classes/owner.go index 7e1593c..6dbb318 100644 --- a/classes/owner.go +++ b/classes/owner.go @@ -145,6 +145,9 @@ func (s *Service) SetURLSEO(ctx context.Context, tx *gorm.DB, in URLSEOWrite) (U case column == "canonical" && v != "" && !ValidURLRef(v): problems[column] = append(problems[column], models.Problem{Key: "golem15.seo::lang.api.canonical_invalid"}) continue + case column == "og_image" && !ValidImageURL(v): + problems[column] = append(problems[column], models.Problem{Key: errImageURL}) + continue } values[column] = v } diff --git a/classes/rules.go b/classes/rules.go index 2b02343..c0ecd8e 100644 --- a/classes/rules.go +++ b/classes/rules.go @@ -28,8 +28,8 @@ func LoadRule(db *gorm.DB, id uint) (models.Rule, error) { // SaveRule validates and stores r inside tx and records it in history: the // address is trimmed and the match normalised, robots values are brought -// to the C5 grammar and a canonical must be a path or an http(s) URL. Field -// problems are refused with a *ProblemsError. +// to the C5 grammar, and a canonical and the social image must be a path +// or an http(s) URL. Field problems are refused with a *ProblemsError. func (s *Service) SaveRule(ctx context.Context, tx *gorm.DB, r *models.Rule) error { if r == nil { return errors.New("golem15.seo: no rule") @@ -47,6 +47,10 @@ func (s *Service) SaveRule(ctx context.Context, tx *gorm.DB, r *models.Rule) err if v := strings.TrimSpace(r.Canonical); v != "" && !ValidURLRef(v) { problems["canonical"] = append(problems["canonical"], models.Problem{Key: "golem15.seo::lang.api.canonical_invalid"}) } + r.OGImage = strings.TrimSpace(r.OGImage) + if !ValidImageURL(r.OGImage) { + problems["og_image"] = append(problems["og_image"], models.Problem{Key: errImageURL}) + } if len(problems) > 0 { return &ProblemsError{Problems: problems} } diff --git a/classes/settings_write.go b/classes/settings_write.go index 887f117..5780d32 100644 --- a/classes/settings_write.go +++ b/classes/settings_write.go @@ -202,6 +202,9 @@ func (s *Service) SaveSettings(ctx context.Context, tx *gorm.DB, in SettingsWrit oneOf("url_host", row.URLHost, models.HostKeep, models.HostWWW, models.HostBare) oneOf("url_slash", row.URLSlash, models.SlashKeep, models.SlashAdd, models.SlashDrop) oneOf("csv_delimiter", row.CSVDelimiter, "", ",", ";") + if _, sent := in.Values["default_og_image"]; sent && !ValidImageURL(row.DefaultOGImage) { + problems["default_og_image"] = append(problems["default_og_image"], models.Problem{Key: errImageURL}) + } for field, list := range row.Problems() { problems[field] = append(problems[field], list...) } diff --git a/mcp_test.go b/mcp_test.go index 3134982..e4bc9af 100644 --- a/mcp_test.go +++ b/mcp_test.go @@ -242,6 +242,21 @@ func TestManagementAPI(t *testing.T) { t.Fatalf("bad url = %d, want 422", rec.Code) } + // A social image is a path or an http(s) URL on every write path, as + // in the admin field (WR-16). + for _, bad := range []string{"javascript:alert(1)", "data:image/png;base64,AAAA", "//evil.example/x.png", "a b.png"} { + if rec := env.mcp(t, http.MethodPut, "/_seo/api/v1/seo", agent, map[string]any{"url": "/pages/a", "values": map[string]any{"og_image": bad}}); rec.Code != http.StatusUnprocessableEntity || !strings.Contains(rec.Body.String(), `"og_image"`) { + t.Fatalf("set_url_seo og_image %q = %d %s, want 422 on og_image", bad, rec.Code, rec.Body.String()) + } + if rec := env.mcp(t, http.MethodPost, "/_seo/api/v1/rules", agent, map[string]any{"url": "/img-rule", "og_image": bad}); rec.Code != http.StatusUnprocessableEntity || !strings.Contains(rec.Body.String(), `"og_image"`) { + t.Fatalf("create_rule og_image %q = %d %s, want 422 on og_image", bad, rec.Code, rec.Body.String()) + } + if rec := env.mcp(t, http.MethodPut, "/_seo/api/v1/settings", agent, map[string]any{"default_og_image": bad}); rec.Code != http.StatusUnprocessableEntity || !strings.Contains(rec.Body.String(), `"default_og_image"`) { + t.Fatalf("update_settings default_og_image %q = %d %s, want 422", bad, rec.Code, rec.Body.String()) + } + } + env.mcpExpect(t, http.StatusOK, http.MethodPut, "/_seo/api/v1/seo", agent, map[string]any{"url": "/pages/a", "values": map[string]any{"og_image": "https://cdn.example/a.png"}}) + // URL rules. rule := env.mcpExpect(t, http.StatusCreated, http.MethodPost, "/_seo/api/v1/rules", agent, map[string]any{"url": "/blog?tag=*", "match": "pattern", "robots": "noindex"}) data, _ := rule["data"].(map[string]any)