From a9740207cc08db6a0c21417fa117dc94b1b09504 Mon Sep 17 00:00:00 2001 From: Chili Palmer Date: Fri, 29 May 2026 22:21:47 +0200 Subject: [PATCH] fix: D1-6 enforce LiquidFilterSubset, reject unsupported filters at publish --- SPECGAPS.md | 2 +- lib/bds/mcp/tools.ex | 2 +- lib/bds/rendering/liquid_parser.ex | 52 ++++++++++++++++++++++++++++++ lib/bds/templates.ex | 2 +- specs/template.allium | 7 +++- test/bds/templates_test.exs | 49 ++++++++++++++++++++++++++++ 6 files changed, 110 insertions(+), 4 deletions(-) diff --git a/SPECGAPS.md b/SPECGAPS.md index 8b2bca5..9f2dcdf 100644 --- a/SPECGAPS.md +++ b/SPECGAPS.md @@ -119,7 +119,7 @@ All reconciled to follow code. Specs must be self-consistent and match code. | D1-3 | ~~BundledDefaultTemplatesExistOutsideProjectData~~ | template.allium:65 | **Resolved:** added 4 tests in `template_lookup_priority_test.exs` — with no Template rows for the project, `load_template_source/3` resolves bundled single-post/post-list/not-found defaults (and still resolves when the project has no `templates/` directory at all) | | 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 | Write test: unsupported filter raises error | +| 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-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 | diff --git a/lib/bds/mcp/tools.ex b/lib/bds/mcp/tools.ex index d641756..3cd315a 100644 --- a/lib/bds/mcp/tools.ex +++ b/lib/bds/mcp/tools.ex @@ -71,7 +71,7 @@ defmodule BDS.MCP.Tools do @spec validate_template(String.t()) :: {:ok, %{valid: boolean(), errors: [String.t()]}} def validate_template(source) when is_binary(source) do - case Liquex.parse(source, BDS.Rendering.LiquidParser) do + case BDS.Rendering.LiquidParser.validate(source) do {:ok, _ast} -> {:ok, %{valid: true, errors: []}} diff --git a/lib/bds/rendering/liquid_parser.ex b/lib/bds/rendering/liquid_parser.ex index e4aeef0..e7a0a98 100644 --- a/lib/bds/rendering/liquid_parser.ex +++ b/lib/bds/rendering/liquid_parser.ex @@ -17,10 +17,62 @@ defmodule BDS.Rendering.LiquidParser do Pass this module as the second argument to `Liquex.parse/2` (and `Liquex.parse!/2`) so validation and rendering share the same surface. + + `validate/1` additionally enforces the `LiquidFilterSubset` invariant: only + the four standard filters (`escape`, `url_encode`, `default`, `append`) and + 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. """ import NimbleParsec + # LiquidFilterSubset invariant (specs/template.allium). + @allowed_filters ~w(escape url_encode default append i18n markdown slugify) + + @doc "The filter names permitted by the `LiquidFilterSubset` invariant." + @spec allowed_filters() :: [String.t()] + def allowed_filters, do: @allowed_filters + + @doc """ + Parses `source` with the restricted tag grammar and enforces the + `LiquidFilterSubset` invariant. + + Returns `{:ok, ast}` on success, or `{:error, reason, line}` on a parse error + or an unsupported filter (mirroring the `Liquex.parse/2` error shape so + callers can treat both failures uniformly). + """ + @spec validate(binary()) :: {:ok, term()} | {:error, term(), non_neg_integer()} + 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} + end + + {:error, _reason, _line} = error -> + error + end + end + + @spec unsupported_filters(term()) :: [String.t()] + defp unsupported_filters(ast) do + ast + |> collect_filters() + |> Enum.uniq() + |> Enum.reject(&(&1 in @allowed_filters)) + end + + @spec collect_filters(term()) :: [String.t()] + defp collect_filters({:filter, [name | rest]}) when is_binary(name) do + [name | collect_filters(rest)] + end + + defp collect_filters(term) when is_tuple(term), do: collect_filters(Tuple.to_list(term)) + defp collect_filters(term) when is_list(term), do: Enum.flat_map(term, &collect_filters/1) + defp collect_filters(_term), do: [] + @tags [ Liquex.Tag.AssignTag, Liquex.Tag.ForTag, diff --git a/lib/bds/templates.ex b/lib/bds/templates.ex index bc5f241..9a67b01 100644 --- a/lib/bds/templates.ex +++ b/lib/bds/templates.ex @@ -350,7 +350,7 @@ defmodule BDS.Templates do end defp validate_liquid(source) do - case Liquex.parse(source, BDS.Rendering.LiquidParser) do + case BDS.Rendering.LiquidParser.validate(source) do {:ok, _ast} -> :ok {:error, reason, line} -> {:error, "#{reason} at line #{line}"} end diff --git a/specs/template.allium b/specs/template.allium index f223616..a19c1d1 100644 --- a/specs/template.allium +++ b/specs/template.allium @@ -199,11 +199,16 @@ invariant LiquidFilterSubset { -- | default: fallback_value -- | append: suffix_string -- - -- Custom filters (2): + -- Custom filters (3): -- | i18n: language — translates a key string for given language -- | markdown: post_id, post_data_json_by_id, canonical_post_path_by_slug, -- canonical_media_path_by_source_path, language, language_prefix -- — renders Markdown to HTML with link rewriting (6 arguments) + -- | slugify — slugifies a string (used for tag/category URLs) + -- + -- Enforced at publish/validation time (LiquidParser.validate); any other + -- filter is rejected even though Liquex would otherwise apply it as a + -- built-in standard filter. -- -- NOT used: date, strip_html, truncate, split, join, size (as filter), -- upcase, downcase, replace, remove, sort, map, where, first, last, diff --git a/test/bds/templates_test.exs b/test/bds/templates_test.exs index 3826320..adcbd78 100644 --- a/test/bds/templates_test.exs +++ b/test/bds/templates_test.exs @@ -340,6 +340,55 @@ defmodule BDS.TemplatesTest do end end + # LiquidFilterSubset invariant (template.allium): only escape, url_encode, + # default, append (standard) and i18n, markdown, slugify (custom) are allowed. + # Any other filter must be rejected at publish time, even though Liquex would + # otherwise apply it as a built-in standard filter. + for filter <- ["upcase", "downcase", "date", "truncate", "split", "reverse"] do + @filter_name filter + test "publish_template rejects unsupported Liquid filter #{filter}", %{ + project: project + } do + filter = @filter_name + + assert {:ok, template} = + BDS.Templates.create_template(%{ + project_id: project.id, + title: "Filter #{filter}", + kind: :post, + content: "{{ title | #{filter} }}" + }) + + assert {:error, {:invalid_liquid, reason}} = + BDS.Templates.publish_template(template.id) + + assert reason =~ "unsupported filter: #{filter}" + + reloaded = Repo.get!(BDS.Templates.Template, template.id) + assert reloaded.status == :draft + end + end + + for filter <- ["escape", "url_encode", "slugify"] do + @filter_name filter + test "publish_template allows supported Liquid filter #{filter}", %{ + project: project + } do + filter = @filter_name + + assert {:ok, template} = + BDS.Templates.create_template(%{ + project_id: project.id, + title: "Filter #{filter}", + kind: :post, + content: "{{ title | #{filter} }}" + }) + + 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