feat: allow unrestricted svglide experiment assets

This commit is contained in:
songtianyi.theo
2026-07-03 15:01:32 +08:00
parent d82d4e3333
commit bd63a20342
6 changed files with 112 additions and 107 deletions

View File

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

View File

@@ -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{
`<image slide:role="image"`,
`href="https://example.com/hero.png"`,
} {
if !strings.Contains(svg, want) {
t.Fatalf("experiment remote image missing %q:\n%s", want, svg)
}
}
}
func TestAuthorSlidesSkipsUnsupportedReadyImageAssets(t *testing.T) {
tests := []struct {
name string
asset string
}{
{
name: "remote",
asset: `{"assets":[{"id":"hero","slide_id":"s1","type":"image","path":"https://example.com/hero.png","usage":"Hero image","status":"ready"}]}`,
},
{
name: "escape",
asset: `{"assets":[{"id":"hero","slide_id":"s1","type":"image","path":"../hero.png","usage":"Hero image","status":"ready"}]}`,
},
{
name: "wrong directory",
asset: `{"assets":[{"id":"hero","slide_id":"s1","type":"image","path":"assets/other/hero.png","usage":"Hero image","status":"ready"}]}`,
},
{
name: "diagram",
asset: `{"assets":[{"id":"hero","slide_id":"s1","type":"diagram","path":"assets/images/hero.png","usage":"Hero diagram","status":"ready"}]}`,
@@ -251,13 +272,13 @@ func TestAuthorSlidesSkipsUnsafeOrUnsupportedReadyImageAssets(t *testing.T) {
t.Fatal(err)
}
if strings.Contains(string(raw), `<image slide:role="image"`) {
t.Fatalf("unsafe or unsupported asset should not render image:\n%s", string(raw))
t.Fatalf("unsupported asset should not render image:\n%s", string(raw))
}
})
}
}
func TestAuthorSlidesDoesNotRenderExistingAbsoluteImageAsset(t *testing.T) {
func TestAuthorSlidesRendersExistingAbsoluteImageAssetInExperiment(t *testing.T) {
initAuthorDemoRun(t,
`{"color_system":{"background":"#FFFFFF","ink":"#111827","muted":"#6B7280","accent":"#2563EB"},"typography":{"title":32,"body":16},"layout_language":"analyst deck"}`,
`{"title":"Demo Deck","slides":[{"id":"s1","title":"First claim","summary":"First summary","role":"cover","key_message":"First key message","path":"slides/01.svg"}]}`,
@@ -277,8 +298,8 @@ func TestAuthorSlidesDoesNotRenderExistingAbsoluteImageAsset(t *testing.T) {
if err != nil {
t.Fatal(err)
}
if strings.Contains(string(raw), outside) || strings.Contains(string(raw), `<image slide:role="image"`) {
t.Fatalf("absolute asset should not render image:\n%s", string(raw))
if !strings.Contains(string(raw), outside) || !strings.Contains(string(raw), `<image slide:role="image"`) {
t.Fatalf("absolute asset should render image in experiment mode:\n%s", string(raw))
}
}

View File

@@ -129,15 +129,17 @@ func CheckQuality(root string) (QualityReport, error) {
}
assetsBySlideAndID := make(map[string]qualityAsset, len(assets.Assets))
deferredBySlideAndID := make(map[string]qualityAsset, len(assets.Assets))
for _, asset := range assets.Assets {
if strings.TrimSpace(asset.Status) != "ready" {
continue
}
if err := validateQualityAssetPath(safeRoot, asset); err != nil {
report.Issues = append(report.Issues, qualityIssue("assets/assets_plan.json", "svglide.quality.asset_path", err.Error()))
continue
}
status := strings.TrimSpace(asset.Status)
key := strings.TrimSpace(asset.SlideID) + "/" + strings.TrimSpace(asset.ID)
if status == "deferred" {
deferredBySlideAndID[key] = asset
continue
}
if status != "ready" {
continue
}
assetsBySlideAndID[key] = asset
}
@@ -198,6 +200,12 @@ func CheckQuality(root string) (QualityReport, error) {
hasVisual = true
key := id + "/" + strings.TrimSpace(visual.ID)
asset, ok := assetsBySlideAndID[key]
if !ok && visualTypeIsDeferredOnly(visualType) {
if deferredAsset, deferred := deferredBySlideAndID[key]; deferred {
asset = deferredAsset
ok = true
}
}
if !ok {
report.Issues = append(report.Issues, qualityIssue(
"assets/assets_plan.json",
@@ -230,6 +238,15 @@ func CheckQuality(root string) (QualityReport, error) {
return report, nil
}
func visualTypeIsDeferredOnly(value string) bool {
switch strings.TrimSpace(value) {
case "chart", "table", "crop":
return true
default:
return false
}
}
func readQualitySources(safeRoot string) (qualitySourcesFile, error) {
raw, err := readRunRegularArtifact(safeRoot, "research/sources.json")
if err != nil {
@@ -266,20 +283,6 @@ func readQualityAssets(safeRoot string) (qualityAssetsFile, error) {
return file, nil
}
func validateQualityAssetPath(safeRoot string, asset qualityAsset) error {
if strings.TrimSpace(asset.Status) != "ready" {
return nil
}
path, err := validatePreparedImageAssetPath(asset.Path)
if err != nil {
return fmt.Errorf("asset %q: %w", strings.TrimSpace(asset.ID), err)
}
if _, err := readRunRegularArtifact(safeRoot, path); err != nil {
return err
}
return nil
}
func qualityIssue(path, code, message string) QualityIssue {
return QualityIssue{
Path: path,

View File

@@ -119,7 +119,24 @@ func TestCheckQualityPassesAnyGenReadyRun(t *testing.T) {
}
}
func TestCheckQualityRejectsAbsoluteReadyAssetPath(t *testing.T) {
func TestCheckQualityAllowsExperimentAssetsAndDeferredUnsupportedVisuals(t *testing.T) {
t.Chdir(t.TempDir())
initStatusTestRun(t)
mustWriteTestFile(t, "demo/research/sources.json", `{"sources":[{"id":"web1","path":"https://example.com/report","title":"Report","excerpt":"Full page excerpt","usage":"evidence","retrieval":"full_page"}]}`)
mustWriteTestFile(t, "demo/outline/deck.json", `{"main_title":"Demo Deck","style_instruction":{"aesthetic_direction":"Editorial report","color_palette":{},"typography":{}},"slides":[{"id":"s1","title":"Chart claim","summary":"Needs chart later","role":"content","key_message":"Chart is deferred","path":"slides/01.svg"}]}`)
mustWriteTestFile(t, "demo/content/slide_content.json", `{"slides":[{"id":"s1","content":"Chart-backed point","source_refs":["web1"],"visuals":[{"id":"hero","type":"image","instruction":"Use a remote hero image"},{"id":"chart1","type":"chart","instruction":"Use a real chart when chart generation is enabled"}]}]}`)
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"},{"id":"chart1","slide_id":"s1","type":"chart","path":"","usage":"Deferred chart generation","status":"deferred"}]}`)
report, err := CheckQuality("demo")
if err != nil {
t.Fatal(err)
}
if report.Status != "passed" {
t.Fatalf("Status = %q, want passed: %+v", report.Status, report.Issues)
}
}
func TestCheckQualityAllowsAbsoluteReadyAssetPathInExperiment(t *testing.T) {
initStatusTestRun(t)
mustWriteTestFile(t, "demo/outline/deck.json", `{"title":"Demo Deck","slides":[{"id":"s1","title":"First claim","summary":"First summary","role":"cover","key_message":"First key message","path":"slides/01.svg"}]}`)
mustWriteTestFile(t, "demo/research/sources.json", `{"sources":[{"id":"web1","path":"https://example.com/page","title":"Web Source","excerpt":"Input","usage":"Support","retrieval":"full_page"}]}`)
@@ -134,11 +151,8 @@ func TestCheckQualityRejectsAbsoluteReadyAssetPath(t *testing.T) {
if err != nil {
t.Fatal(err)
}
if report.Status != "failed" {
t.Fatalf("status = %q, want failed", report.Status)
}
if !qualityIssueCodesContain(report.Issues, "svglide.quality.asset_path") {
t.Fatalf("issues = %+v, want svglide.quality.asset_path", report.Issues)
if report.Status != "passed" {
t.Fatalf("status = %q, want passed; issues = %+v", report.Status, report.Issues)
}
}
@@ -214,7 +228,7 @@ func TestCheckQualityCountsSlidesWithVisualsPerPage(t *testing.T) {
}
}
func TestCheckQualityRejectsSymlinkReadyAssetPath(t *testing.T) {
func TestCheckQualityAllowsSymlinkReadyAssetPathInExperiment(t *testing.T) {
initStatusTestRun(t)
mustWriteTestFile(t, "demo/outline/deck.json", `{"title":"Demo Deck","slides":[{"id":"s1","title":"First claim","summary":"First summary","role":"cover","key_message":"First key message","path":"slides/01.svg"}]}`)
mustWriteTestFile(t, "demo/research/sources.json", `{"sources":[{"id":"web1","path":"https://example.com/page","title":"Web Source","excerpt":"Input","usage":"Support","retrieval":"full_page"}]}`)
@@ -234,11 +248,8 @@ func TestCheckQualityRejectsSymlinkReadyAssetPath(t *testing.T) {
if err != nil {
t.Fatal(err)
}
if report.Status != "failed" {
t.Fatalf("status = %q, want failed", report.Status)
}
if !qualityIssueCodesContain(report.Issues, "svglide.quality.asset_path") {
t.Fatalf("issues = %+v, want svglide.quality.asset_path", report.Issues)
if report.Status != "passed" {
t.Fatalf("status = %q, want passed; issues = %+v", report.Status, report.Issues)
}
}

View File

@@ -246,13 +246,6 @@ func lintSVGElementProtocol(path string, start xml.StartElement, excluded bool)
}
var issues []ValidationIssue
if start.Name.Local == "image" && imageHrefIsUnsafe(start) {
issues = append(issues, ValidationIssue{
Path: path,
Code: "svglide.remote_asset",
Message: "image href must be a local prepared assets/images/<file> 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 {

View File

@@ -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"), `<svg xmlns="http://www.w3.org/2000/svg" xmlns:slide="https://slides.bytedance.com/ns" viewBox="0 0 960 540" slide:role="slide">
@@ -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"), `<svg xmlns="http://www.w3.org/2000/svg" xmlns:slide="https://slides.bytedance.com/ns" viewBox="0 0 960 540" slide:role="slide">
@@ -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"), `<svg xmlns="http://www.w3.org/2000/svg" xmlns:slide="https://slides.bytedance.com/ns" xmlns:xlink="http://www.w3.org/1999/xlink" viewBox="0 0 960 540" slide:role="slide">
@@ -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"), `<svg xmlns="http://www.w3.org/2000/svg" xmlns:slide="https://slides.bytedance.com/ns" viewBox="0 0 960 540" slide:role="slide">
@@ -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)