From d606d9b26bda93de06df346ba93383b7e362b0df Mon Sep 17 00:00:00 2001 From: Chili Palmer Date: Fri, 29 May 2026 22:25:06 +0200 Subject: [PATCH] test: D1-7 enforce LiquidOperatorSubset, reject unsupported comparison operators at publish --- SPECGAPS.md | 2 +- lib/bds/rendering/liquid_parser.ex | 40 +++++++++++++++++++--- specs/template.allium | 4 +++ test/bds/templates_test.exs | 53 ++++++++++++++++++++++++++++++ 4 files changed, 94 insertions(+), 5 deletions(-) diff --git a/SPECGAPS.md b/SPECGAPS.md index 9f2dcdf..bcf52cf 100644 --- a/SPECGAPS.md +++ b/SPECGAPS.md @@ -120,7 +120,7 @@ All reconciled to follow code. Specs must be self-consistent and match code. | D1-4 | ~~UserTemplateDirectoryOverridesBundledDefaults~~ | template.allium:75 | **Resolved:** added 2 tests in `template_lookup_priority_test.exs` — a published project Template row with the bundled default slug (`single-post`) wins over the bundled default both when resolving `:post` with no explicit slug and when the slug is requested explicitly | | D1-5 | ~~LiquidTagSubset (5 tags only)~~ | template.allium:179 | **Resolved:** added `BDS.Rendering.LiquidParser`, a restricted Liquex parser recognizing only the subset (if/for/assign/render + `{{ }}` output); any other tag (`unless`, `case`, `capture`, `tablerow`, `cycle`, `increment`, …) leaves unmatched input and fails `eos/0`. Wired into `validate_liquid` (publish gate), `template_selection.render_template`, `filters.render_macro_source`, and MCP `validate_template` so validation and rendering share the same surface; 6 parametrized tests added asserting unsupported tags are rejected at publish | | D1-6 | ~~LiquidFilterSubset (4 standard + 2 custom)~~ | template.allium:191 | **Resolved:** added `LiquidParser.validate/1`, which parses with the restricted tag grammar then walks the AST to reject any filter outside the allowed set — 4 standard (`escape`, `url_encode`, `default`, `append`) + 3 custom (`i18n`, `markdown`, `slugify`). Wired into `validate_liquid` (publish gate) and MCP `validate_template` so unsupported filters are rejected even though Liquex would otherwise apply them as built-in standard filters. Spec corrected to 3 custom filters (bundled templates use `slugify`); 9 tests added (6 unsupported filters rejected, 3 supported filters accepted). | -| D1-7 | LiquidOperatorSubset | template.allium:210 | Write test: unsupported operator raises error | +| D1-7 | ~~LiquidOperatorSubset~~ | template.allium:210 | **Resolved:** `LiquidParser.validate/1` now walks the parsed AST for `{:op, _}` nodes and rejects any comparison operator outside the allowed `==`/`>` subset (`!=`, `<`, `>=`, `<=`, `contains`), sharing the publish gate and MCP `validate_template` surface with the tag/filter checks; spec `LiquidOperatorSubset` annotated with enforcement note; 10 tests added (5 unsupported operators rejected at publish, 5 supported `==`/`>`/`and`/`or`/bare-truthy expressions accepted). | | D1-8 | MacroTimeout guarantee | script.allium:94-95 | Write test: macro times out within budget | | D1-9 | ExecuteTransform rule (pipeline, ordering, toast budget) | script.allium:229-263 | Write test: transform pipeline executes in order, toast budget enforced | | D1-10 | TransformPipelineContinuation | script.allium:247-249 | Write test: error in transform doesn't halt pipeline | diff --git a/lib/bds/rendering/liquid_parser.ex b/lib/bds/rendering/liquid_parser.ex index e7a0a98..a3aeec1 100644 --- a/lib/bds/rendering/liquid_parser.ex +++ b/lib/bds/rendering/liquid_parser.ex @@ -23,6 +23,12 @@ defmodule BDS.Rendering.LiquidParser do three custom filters (`i18n`, `markdown`, `slugify`) are permitted. Any other filter (`upcase`, `date`, `truncate`, `split`, `join`, …) is rejected even though Liquex would otherwise apply it as a built-in standard filter. + + `validate/1` also enforces the `LiquidOperatorSubset` invariant: only the + `==` and `>` comparison operators are permitted (alongside the `and`/`or` + logical operators and bare-variable truthiness). Any other comparison + operator (`!=`, `<`, `>=`, `<=`, `contains`) is rejected even though Liquex + would otherwise evaluate it. """ import NimbleParsec @@ -30,6 +36,9 @@ defmodule BDS.Rendering.LiquidParser do # LiquidFilterSubset invariant (specs/template.allium). @allowed_filters ~w(escape url_encode default append i18n markdown slugify) + # LiquidOperatorSubset invariant (specs/template.allium). + @allowed_operators [:==, :>] + @doc "The filter names permitted by the `LiquidFilterSubset` invariant." @spec allowed_filters() :: [String.t()] def allowed_filters, do: @allowed_filters @@ -46,9 +55,12 @@ defmodule BDS.Rendering.LiquidParser do def validate(source) when is_binary(source) do case Liquex.parse(source, __MODULE__) do {:ok, ast} -> - case unsupported_filters(ast) do - [] -> {:ok, ast} - [name | _] -> {:error, "unsupported filter: #{name}", 0} + with [] <- unsupported_filters(ast), + [] <- unsupported_operators(ast) do + {:ok, ast} + else + [{:filter, name} | _] -> {:error, "unsupported filter: #{name}", 0} + [{:operator, op} | _] -> {:error, "unsupported operator: #{op}", 0} end {:error, _reason, _line} = error -> @@ -56,12 +68,22 @@ defmodule BDS.Rendering.LiquidParser do end end - @spec unsupported_filters(term()) :: [String.t()] + @spec unsupported_filters(term()) :: [{:filter, String.t()}] defp unsupported_filters(ast) do ast |> collect_filters() |> Enum.uniq() |> Enum.reject(&(&1 in @allowed_filters)) + |> Enum.map(&{:filter, &1}) + end + + @spec unsupported_operators(term()) :: [{:operator, String.t()}] + defp unsupported_operators(ast) do + ast + |> collect_operators() + |> Enum.uniq() + |> Enum.reject(&(&1 in @allowed_operators)) + |> Enum.map(&{:operator, operator_to_string(&1)}) end @spec collect_filters(term()) :: [String.t()] @@ -73,6 +95,16 @@ defmodule BDS.Rendering.LiquidParser do defp collect_filters(term) when is_list(term), do: Enum.flat_map(term, &collect_filters/1) defp collect_filters(_term), do: [] + @spec collect_operators(term()) :: [atom()] + defp collect_operators({:op, op}) when is_atom(op), do: [op] + defp collect_operators(term) when is_tuple(term), do: collect_operators(Tuple.to_list(term)) + defp collect_operators(term) when is_list(term), do: Enum.flat_map(term, &collect_operators/1) + defp collect_operators(_term), do: [] + + @spec operator_to_string(atom()) :: String.t() + defp operator_to_string(:contains), do: "contains" + defp operator_to_string(op), do: Atom.to_string(op) + @tags [ Liquex.Tag.AssignTag, Liquex.Tag.ForTag, diff --git a/specs/template.allium b/specs/template.allium index a19c1d1..898acb5 100644 --- a/specs/template.allium +++ b/specs/template.allium @@ -223,6 +223,10 @@ invariant LiquidOperatorSubset { -- Special values: blank (nil/empty comparison) -- Property access: dot notation (object.property), .size on arrays, -- bracket notation for map lookups (map[key]) + -- + -- Enforced at publish/validation time (LiquidParser.validate); any other + -- comparison operator (!=, <, >=, <=, contains) is rejected even though + -- Liquex would otherwise evaluate it. } invariant LiquidRenderContext { diff --git a/test/bds/templates_test.exs b/test/bds/templates_test.exs index adcbd78..0e97dde 100644 --- a/test/bds/templates_test.exs +++ b/test/bds/templates_test.exs @@ -389,6 +389,59 @@ defmodule BDS.TemplatesTest do end end + # LiquidOperatorSubset invariant (template.allium): only == and > comparison + # operators (plus the and/or logical operators and bare-variable truthiness) + # are allowed. Any other comparison operator must be rejected at publish time, + # even though Liquex would otherwise evaluate it. + for op <- ["!=", "<", ">=", "<=", "contains"] do + @op_name op + test "publish_template rejects unsupported Liquid operator #{op}", %{ + project: project + } do + op = @op_name + + assert {:ok, template} = + BDS.Templates.create_template(%{ + project_id: project.id, + title: "Operator #{op}", + kind: :post, + content: "{% if title #{op} other %}yes{% endif %}" + }) + + assert {:error, {:invalid_liquid, reason}} = + BDS.Templates.publish_template(template.id) + + assert reason =~ "unsupported operator: #{op}" + + reloaded = Repo.get!(BDS.Templates.Template, template.id) + assert reloaded.status == :draft + end + end + + for content <- [ + "{% if title == other %}yes{% endif %}", + "{% if total > 0 %}yes{% endif %}", + "{% if published %}yes{% endif %}", + "{% if a == b and c > d %}yes{% endif %}", + "{% if a == b or c == d %}yes{% endif %}" + ] do + @op_content content + test "publish_template allows supported operators in #{content}", %{project: project} do + content = @op_content + + assert {:ok, template} = + BDS.Templates.create_template(%{ + project_id: project.id, + title: "Allowed #{content}", + kind: :post, + content: content + }) + + assert {:ok, published} = BDS.Templates.publish_template(template.id) + assert published.status == :published + end + end + test "rebuild_templates_from_files recreates published templates from disk", %{ project: project, temp_dir: temp_dir