fix: A1-8 add Liquid/Lua validation gates before template and script publish

2026-05-29 09:16:07 +02:00
parent d7e30b94cb
commit 99d36e6e2f
6 changed files with 127 additions and 64 deletions
--- a/SPECGAPS.md
+++ b/SPECGAPS.md
@@ -17,7 +17,7 @@ Gap categories: **SC** = spec correct, fix code | **CS** = code correct, update
 | A1-5 | ~~Auto-save after 3000ms idle~~ | editor_post.allium:183-188 | PostEditor schedules auto-save via parent timer on dirty change | **Resolved:** 3000ms idle auto-save timer in Bridges, tab-switch save in ShellLive, cancel on manual save, 3 tests added |
 | A1-6 | ~~On-demand rendering in preview server~~ | preview.allium:53-93 | `Preview.Router` matches post/archive/home/language routes and renders on-demand via `Rendering` | **Resolved:** `Preview.Router` implements on-demand template rendering for post, archive, home, date, tag, category, page, and language-prefixed routes; static file fallback retained for non-HTML assets (pagefind, feeds); 6 tests added |
 | A1-7 | ~~Template lookup must use all 4 levels (post→tag→category→default)~~ | template_context.allium:267-277 | `resolve_post_template_slug/3` implements tag→category cascade; all callers (preview, generation) updated | **Resolved:** `resolve_post_template_slug/3` in template_selection.ex, callers in preview.ex, router.ex, outputs.ex updated, 8 tests added |
-| A1-8 | `ValidateLiquid`/`ValidateScript` before publish | template.allium:110, script.allium:165 | No validation gate before publish | Fix code: add validation step before publish |
+| A1-8 | ~~`ValidateLiquid`/`ValidateScript` before publish~~ | template.allium:110, script.allium:165 | `publish_template` validates Liquid via `Liquex.parse`, `publish_script` validates Lua via `BDS.Scripting.validate` | **Resolved:** validation gates added to `publish_template/1` and `publish_script/1`, invalid content returns `{:error, {:invalid_liquid|:invalid_script, reason}}`, 4 tests added |
 | A1-9 | 17 preset colors + custom hex in tag picker | editor_tags.allium | Native `<input type="color">`, no preset palette | Fix code: implement preset color palette popover |
 | A1-10 | Template file written on create | engine_side_effects.allium:151-153 | Draft templates have `file_path=""` | Fix code: write template file on create |
 | A1-11 | Graceful shutdown with inflight request tracking | preview.allium:47-48 | Kills acceptor process, no inflight tracking | Fix code: track inflight requests, drain before shutdown |
--- a/lib/bds/scripts.ex
+++ b/lib/bds/scripts.ex
@@ -57,28 +57,35 @@ defmodule BDS.Scripts do
        {:error, :not_found}

      script ->
-        file_path = script_file_path(script.slug)
-        full_path = full_file_path(script.project_id, file_path)
-        updated_at = Persistence.now_ms()
        content = script.content || ""

-        :ok =
-          Persistence.atomic_write(
-            full_path,
-            serialize_script_file(
-              %{script | status: :published, file_path: file_path, updated_at: updated_at},
-              content
-            )
-          )
+        case validate_script(content) do
+          :ok ->
+            file_path = script_file_path(script.slug)
+            full_path = full_file_path(script.project_id, file_path)
+            updated_at = Persistence.now_ms()

-        script
-        |> Script.changeset(%{
-          status: :published,
-          file_path: file_path,
-          content: nil,
-          updated_at: updated_at
-        })
-        |> Repo.update()
+            :ok =
+              Persistence.atomic_write(
+                full_path,
+                serialize_script_file(
+                  %{script | status: :published, file_path: file_path, updated_at: updated_at},
+                  content
+                )
+              )
+
+            script
+            |> Script.changeset(%{
+              status: :published,
+              file_path: file_path,
+              content: nil,
+              updated_at: updated_at
+            })
+            |> Repo.update()
+
+          {:error, reason} ->
+            {:error, {:invalid_script, reason}}
+        end
    end
  end

@@ -254,6 +261,13 @@ defmodule BDS.Scripts do
    not Repo.exists?(scoped_query)
  end

+  defp validate_script(source) do
+    case BDS.Scripting.validate(source) do
+      :ok -> :ok
+      {:error, reason} -> {:error, reason}
+    end
+  end
+
  defp script_file_path(slug), do: Path.join(["scripts", "#{slug}.lua"])

  defp full_file_path(project_id, relative_path) do
--- a/lib/bds/templates.ex
+++ b/lib/bds/templates.ex
@@ -62,28 +62,35 @@ defmodule BDS.Templates do
        {:error, :not_found}

      template ->
-        file_path = template_file_path(template.slug)
-        full_path = full_file_path(template.project_id, file_path)
-        updated_at = Persistence.now_ms()
        content = template.content || ""

-        :ok =
-          Persistence.atomic_write(
-            full_path,
-            serialize_template_file(
-              %{template | status: :published, file_path: file_path, updated_at: updated_at},
-              content
-            )
-          )
+        case validate_liquid(content) do
+          :ok ->
+            file_path = template_file_path(template.slug)
+            full_path = full_file_path(template.project_id, file_path)
+            updated_at = Persistence.now_ms()

-        template
-        |> Template.changeset(%{
-          status: :published,
-          file_path: file_path,
-          content: nil,
-          updated_at: updated_at
-        })
-        |> Repo.update()
+            :ok =
+              Persistence.atomic_write(
+                full_path,
+                serialize_template_file(
+                  %{template | status: :published, file_path: file_path, updated_at: updated_at},
+                  content
+                )
+              )
+
+            template
+            |> Template.changeset(%{
+              status: :published,
+              file_path: file_path,
+              content: nil,
+              updated_at: updated_at
+            })
+            |> Repo.update()
+
+          {:error, reason} ->
+            {:error, {:invalid_liquid, reason}}
+        end
    end
  end

@@ -327,6 +334,13 @@ defmodule BDS.Templates do
    not Repo.exists?(scoped_query)
  end

+  defp validate_liquid(source) do
+    case Liquex.parse(source) do
+      {:ok, _ast} -> :ok
+      {:error, reason, line} -> {:error, "#{reason} at line #{line}"}
+    end
+  end
+
  defp template_file_path(slug), do: Path.join(["templates", "#{slug}.liquid"])

  defp full_file_path(project_id, relative_path) do
--- a/test/bds/csm013_bang_rendering_test.exs
+++ b/test/bds/csm013_bang_rendering_test.exs
@@ -31,7 +31,7 @@ defmodule BDS.CSM013BangRenderingTest do
      assert {:error, _reason} = result
    end

-    test "render_post_page returns {:error, _} for broken template source", %{project: project} do
+    test "publish_template rejects broken template source", %{project: project} do
      {:ok, template} =
        BDS.Templates.create_template(%{
          project_id: project.id,
@@ -40,30 +40,7 @@ defmodule BDS.CSM013BangRenderingTest do
          content: "{% if true %}unclosed if"
        })

-      {:ok, published_template} = BDS.Templates.publish_template(template.id)
-
-      {:ok, post} =
-        BDS.Posts.create_post(%{
-          project_id: project.id,
-          title: "Test Post",
-          content: "Body",
-          language: "en",
-          template_slug: published_template.slug
-        })
-
-      {:ok, published_post} = BDS.Posts.publish_post(post.id)
-
-      result =
-        Rendering.render_post_page(project.id, published_template.slug, %{
-          id: published_post.id,
-          title: published_post.title,
-          content: published_post.content || "",
-          slug: published_post.slug,
-          language: "en",
-          template_slug: published_post.template_slug
-        })
-
-      assert {:error, _reason} = result
+      assert {:error, {:invalid_liquid, _reason}} = BDS.Templates.publish_template(template.id)
    end
  end

--- a/test/bds/scripts_test.exs
+++ b/test/bds/scripts_test.exs
@@ -126,6 +126,35 @@ defmodule BDS.ScriptsTest do
    refute File.exists?(Path.join(temp_dir, published.file_path))
  end

+  test "publish_script rejects invalid Lua syntax", %{project: project} do
+    assert {:ok, script} =
+             BDS.Scripts.create_script(%{
+               project_id: project.id,
+               title: "Bad Script",
+               kind: :utility,
+               content: "function main( missing end"
+             })
+
+    assert {:error, {:invalid_script, _reason}} = BDS.Scripts.publish_script(script.id)
+
+    reloaded = Repo.get!(Script, script.id)
+    assert reloaded.status == :draft
+    assert reloaded.content == "function main( missing end"
+  end
+
+  test "publish_script allows valid Lua syntax", %{project: project} do
+    assert {:ok, script} =
+             BDS.Scripts.create_script(%{
+               project_id: project.id,
+               title: "Good Script",
+               kind: :utility,
+               content: "function main() return 42 end"
+             })
+
+    assert {:ok, published} = BDS.Scripts.publish_script(script.id)
+    assert published.status == :published
+  end
+
  test "rebuild_scripts_from_files recreates published scripts from disk", %{
    project: project,
    temp_dir: temp_dir
--- a/test/bds/templates_test.exs
+++ b/test/bds/templates_test.exs
@@ -264,6 +264,35 @@ defmodule BDS.TemplatesTest do
    assert reloaded_tag.post_template_slug == "feature-view"
  end

+  test "publish_template rejects invalid Liquid syntax", %{project: project} do
+    assert {:ok, template} =
+             BDS.Templates.create_template(%{
+               project_id: project.id,
+               title: "Bad Template",
+               kind: :post,
+               content: "{% for item in items %}unclosed"
+             })
+
+    assert {:error, {:invalid_liquid, _reason}} = BDS.Templates.publish_template(template.id)
+
+    reloaded = Repo.get!(BDS.Templates.Template, template.id)
+    assert reloaded.status == :draft
+    assert reloaded.content == "{% for item in items %}unclosed"
+  end
+
+  test "publish_template allows valid Liquid syntax", %{project: project} do
+    assert {:ok, template} =
+             BDS.Templates.create_template(%{
+               project_id: project.id,
+               title: "Good Template",
+               kind: :post,
+               content: "{% for item in items %}{{ item }}{% endfor %}"
+             })
+
+    assert {:ok, published} = BDS.Templates.publish_template(template.id)
+    assert published.status == :published
+  end
+
  test "rebuild_templates_from_files recreates published templates from disk", %{
    project: project,
    temp_dir: temp_dir