From bd63a20342cef9d625c308cd9e826ccff3357ef3 Mon Sep 17 00:00:00 2001 From: "songtianyi.theo" Date: Fri, 3 Jul 2026 15:01:32 +0800 Subject: [PATCH] feat: allow unrestricted svglide experiment assets --- internal/svglide/author.go | 11 ++----- internal/svglide/author_test.go | 55 +++++++++++++++++++++---------- internal/svglide/quality.go | 45 +++++++++++++------------ internal/svglide/quality_test.go | 35 +++++++++++++------- internal/svglide/validate.go | 23 ------------- internal/svglide/validate_test.go | 50 ++++++++++++++-------------- 6 files changed, 112 insertions(+), 107 deletions(-) diff --git a/internal/svglide/author.go b/internal/svglide/author.go index d1bece312..e4d59e4e9 100644 --- a/internal/svglide/author.go +++ b/internal/svglide/author.go @@ -273,19 +273,12 @@ func selectAuthorRenderableImageAssets(safeRoot string, content authorSlideConte return nil } -func authorImageAssetUsable(safeRoot string, asset authorAsset) bool { +func authorImageAssetUsable(_ string, asset authorAsset) bool { if strings.TrimSpace(asset.Type) != "image" { return false } path := strings.TrimSpace(asset.Path) - path, err := validatePreparedImageAssetPath(path) - if err != nil { - return false - } - if _, err := readRunRegularArtifact(safeRoot, path); err != nil { - return false - } - return true + return path != "" } func validateAuthorDeckContent(deck authorDeck, contentByID map[string]authorSlideContent) error { diff --git a/internal/svglide/author_test.go b/internal/svglide/author_test.go index c8a7c433b..adab0a47a 100644 --- a/internal/svglide/author_test.go +++ b/internal/svglide/author_test.go @@ -200,23 +200,44 @@ func TestAuthorSlidesDoesNotRenderImageForMismatchedVisualID(t *testing.T) { } } -func TestAuthorSlidesSkipsUnsafeOrUnsupportedReadyImageAssets(t *testing.T) { +func TestAuthorSlidesRendersExperimentRemoteImageAsset(t *testing.T) { + initStatusTestRun(t) + + mustWriteTestFile(t, "demo/brief/design_brief.json", `{"narrative_spine":"A to B","depth":"medium","tone":"clear"}`) + mustWriteTestFile(t, "demo/brief/visual_system.json", `{"color_system":{"background":"#FFFFFF","ink":"#111827","muted":"#6B7280","accent":"#2563EB"},"typography":{"title":32,"body":16},"layout_language":"analyst deck"}`) + mustWriteTestFile(t, "demo/outline/deck.json", `{"title":"Demo Deck","slides":[{"id":"s1","title":"Hero slide","summary":"Hero summary","role":"cover","key_message":"Hero key message","path":"slides/01.svg"}]}`) + mustWriteTestFile(t, "demo/research/sources.json", `{"sources":[{"id":"web1","path":"https://example.com/demo","title":"Demo source","excerpt":"Demo excerpt","usage":"support","retrieval":"full_page"}]}`) + mustWriteTestFile(t, "demo/content/slide_content.json", `{"slides":[{"id":"s1","content":"First body line","source_refs":["web1"],"visuals":[{"id":"hero","type":"image","instruction":"Use the remote hero image"}]}]}`) + mustWriteTestFile(t, "demo/assets/assets_plan.json", `{"mode":"experiment_unrestricted_assets","assets":[{"id":"hero","slide_id":"s1","type":"image","path":"https://example.com/hero.png","usage":"Hero image","status":"ready"}]}`) + + run := readStatusTestRunFile(t) + run.CurrentStage = StageSVGAuthor + writeStatusTestRunFile(t, run) + + if _, err := AuthorSlides("demo"); err != nil { + t.Fatal(err) + } + + raw, err := os.ReadFile(filepath.Join("demo", "slides", "01.svg")) + if err != nil { + t.Fatal(err) + } + svg := string(raw) + for _, want := range []string{ + ` asset", - }) - } if excluded { return issues } @@ -289,22 +282,6 @@ func elementHasNonPositiveDimension(start xml.StartElement) bool { return false } -func imageHrefIsUnsafe(start xml.StartElement) bool { - for _, attr := range start.Attr { - if !isAllowedImageHrefAttr(attr) { - continue - } - if _, err := validatePreparedImageAssetPath(attr.Value); err != nil { - return true - } - } - return false -} - -func isAllowedImageHrefAttr(attr xml.Attr) bool { - return attr.Name.Local == "href" && (attr.Name.Space == "" || attr.Name.Space == xlinkNamespace) -} - func hasSlideAttr(start xml.StartElement, local string, value string) bool { for _, attr := range start.Attr { if attr.Name.Space == slideNamespace && attr.Name.Local == local && strings.TrimSpace(attr.Value) == value { diff --git a/internal/svglide/validate_test.go b/internal/svglide/validate_test.go index ba3359948..e9d7e23a4 100644 --- a/internal/svglide/validate_test.go +++ b/internal/svglide/validate_test.go @@ -539,7 +539,7 @@ func TestValidateRunRejectsNegativeElementDimensions(t *testing.T) { } } -func TestValidateRunRejectsRemoteImageHref(t *testing.T) { +func TestValidateRunAllowsExperimentImageHref(t *testing.T) { initValidateTestRun(t) writeMinimalDeck(t, "demo", "slides/01.svg") writeValidateTestFile(t, filepath.Join("demo", "slides", "01.svg"), ` @@ -550,15 +550,15 @@ func TestValidateRunRejectsRemoteImageHref(t *testing.T) { if err != nil { t.Fatal(err) } - if report.OK { - t.Fatalf("OK = true, want false") + if !report.OK { + t.Fatalf("OK = false, want true: %+v", report.Issues) } - if !validationIssuesContainCode(report.Issues, "svglide.remote_asset") { - t.Fatalf("issues = %+v, want remote asset issue", report.Issues) + if validationIssuesContainCode(report.Issues, "svglide.remote_asset") { + t.Fatalf("issues = %+v, did not expect remote asset issue", report.Issues) } } -func TestValidateRunRejectsRemoteImageHrefCaseInsensitive(t *testing.T) { +func TestValidateRunAllowsExperimentImageHrefCaseInsensitive(t *testing.T) { initValidateTestRun(t) writeMinimalDeck(t, "demo", "slides/01.svg") writeValidateTestFile(t, filepath.Join("demo", "slides", "01.svg"), ` @@ -569,11 +569,11 @@ func TestValidateRunRejectsRemoteImageHrefCaseInsensitive(t *testing.T) { if err != nil { t.Fatal(err) } - if report.OK { - t.Fatalf("OK = true, want false") + if !report.OK { + t.Fatalf("OK = false, want true: %+v", report.Issues) } - if !validationIssuesContainCode(report.Issues, "svglide.remote_asset") { - t.Fatalf("issues = %+v, want remote asset issue", report.Issues) + if validationIssuesContainCode(report.Issues, "svglide.remote_asset") { + t.Fatalf("issues = %+v, did not expect remote asset issue", report.Issues) } } @@ -653,7 +653,7 @@ func TestValidateRunIgnoresGeometryAndImageRoleInsideExcludedContent(t *testing. } } -func TestValidateRunRejectsRemoteImageHrefWithXLink(t *testing.T) { +func TestValidateRunAllowsExperimentImageHrefWithXLink(t *testing.T) { initValidateTestRun(t) writeMinimalDeck(t, "demo", "slides/01.svg") writeValidateTestFile(t, filepath.Join("demo", "slides", "01.svg"), ` @@ -664,15 +664,15 @@ func TestValidateRunRejectsRemoteImageHrefWithXLink(t *testing.T) { if err != nil { t.Fatal(err) } - if report.OK { - t.Fatalf("OK = true, want false") + if !report.OK { + t.Fatalf("OK = false, want true: %+v", report.Issues) } - if !validationIssuesContainCode(report.Issues, "svglide.remote_asset") { - t.Fatalf("issues = %+v, want remote asset issue", report.Issues) + if validationIssuesContainCode(report.Issues, "svglide.remote_asset") { + t.Fatalf("issues = %+v, did not expect remote asset issue", report.Issues) } } -func TestValidateRunRejectsUnsafeImageHrefVariants(t *testing.T) { +func TestValidateRunAllowsExperimentImageHrefVariants(t *testing.T) { tests := []struct { name string href string @@ -697,17 +697,17 @@ func TestValidateRunRejectsUnsafeImageHrefVariants(t *testing.T) { if err != nil { t.Fatal(err) } - if report.OK { - t.Fatalf("OK = true, want false") + if !report.OK { + t.Fatalf("OK = false, want true for %s: %+v", tt.href, report.Issues) } - if !validationIssuesContainCode(report.Issues, "svglide.remote_asset") { - t.Fatalf("issues = %+v, want remote asset issue", report.Issues) + if validationIssuesContainCode(report.Issues, "svglide.remote_asset") { + t.Fatalf("issues = %+v, did not expect remote asset issue", report.Issues) } }) } } -func TestValidateRunRejectsUnsafeImageHrefInsideExcludedContent(t *testing.T) { +func TestValidateRunAllowsExperimentImageHrefInsideExcludedContent(t *testing.T) { initValidateTestRun(t) writeMinimalDeck(t, "demo", "slides/01.svg") writeValidateTestFile(t, filepath.Join("demo", "slides", "01.svg"), ` @@ -719,11 +719,11 @@ func TestValidateRunRejectsUnsafeImageHrefInsideExcludedContent(t *testing.T) { if err != nil { t.Fatal(err) } - if report.OK { - t.Fatalf("OK = true, want false") + if !report.OK { + t.Fatalf("OK = false, want true: %+v", report.Issues) } - if !validationIssuesContainCode(report.Issues, "svglide.remote_asset") { - t.Fatalf("issues = %+v, want remote asset issue", report.Issues) + if validationIssuesContainCode(report.Issues, "svglide.remote_asset") { + t.Fatalf("issues = %+v, did not expect remote asset issue", report.Issues) } if validationIssuesContainCode(report.Issues, "svglide.geometry") { t.Fatalf("issues = %+v, want no geometry issue inside excluded content", report.Issues)