From 99d36e6e2fbea941f57362102b23e6cdebe0f1c7 Mon Sep 17 00:00:00 2001 From: Chili Palmer Date: Fri, 29 May 2026 09:16:07 +0200 Subject: [PATCH] fix: A1-8 add Liquid/Lua validation gates before template and script publish --- SPECGAPS.md | 2 +- lib/bds/scripts.ex | 52 ++++++++++++++++--------- lib/bds/templates.ex | 52 ++++++++++++++++--------- test/bds/csm013_bang_rendering_test.exs | 27 +------------ test/bds/scripts_test.exs | 29 ++++++++++++++ test/bds/templates_test.exs | 29 ++++++++++++++ 6 files changed, 127 insertions(+), 64 deletions(-) diff --git a/SPECGAPS.md b/SPECGAPS.md index 759ff2f..e2620ac 100644 --- 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 ``, 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 | diff --git a/lib/bds/scripts.ex b/lib/bds/scripts.ex index f22c992..7a57d7c 100644 --- 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 diff --git a/lib/bds/templates.ex b/lib/bds/templates.ex index d04d4cc..1254ee6 100644 --- 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 diff --git a/test/bds/csm013_bang_rendering_test.exs b/test/bds/csm013_bang_rendering_test.exs index c0e131b..4637889 100644 --- 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 diff --git a/test/bds/scripts_test.exs b/test/bds/scripts_test.exs index bfa967b..68f2962 100644 --- 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 diff --git a/test/bds/templates_test.exs b/test/bds/templates_test.exs index 18418ec..b9ac764 100644 --- 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