Skip to content

Commit 913b329

Browse files
committed
feat(rules): add FreeMarker (.ftl) review support
closes alibaba#371; adds .ftl to allowlist, a freemarker.md rule checklist (SSTI/XSS/null-handling), glob mapping, tests, and docs. Note the maintainer invited a FreeMarker rule spec contribution.
1 parent 0ec3769 commit 913b329

8 files changed

Lines changed: 53 additions & 0 deletions

File tree

internal/config/allowlist/allowed_ext_test.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,12 @@ func TestIsAllowedExt(t *testing.T) {
2222
{".ETS", true},
2323
{".json5", true},
2424
{".JSON5", true},
25+
{".ftl", true},
26+
{".FTL", true},
27+
{".ftlh", true},
28+
{".FTLH", true},
29+
{".ftlx", true},
30+
{".FTLX", true},
2531
{".txt", false},
2632
{".md", false},
2733
{".png", false},

internal/config/allowlist/supported_file_types.json

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,9 @@
4343
".less",
4444
".html",
4545
".htm",
46+
".ftl",
47+
".ftlh",
48+
".ftlx",
4649
".astro",
4750
".vue",
4851
".svelte",
Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
#### Obvious Typos or Spelling Errors
2+
- Spelling errors in macro names, assigned variable names, or user-facing text at their declaration sites; do not report at reference sites
3+
- Typos in `<#assign>`/`<#macro>`/`<#function>` names that will silently resolve to a missing variable at runtime
4+
5+
#### Output Escaping and XSS
6+
- Interpolations (`${...}`) that reach HTML without escaping: flag when the template lacks `<#ftl output_format="HTML">` (auto-escaping, FreeMarker 2.3.24+) and the value is not passed through `?html`/`?url`/`?js_string` appropriate to its sink (HTML body, attribute, URL, JS, CSS)
7+
- Explicit `?no_esc` or `<#noautoesc>` on values that carry user-controlled data — treat as a high-risk escape hatch; flag unless the source is clearly trusted or already sanitized
8+
- Escaping with the wrong context builtin (e.g. `?html` for a value placed inside a URL or inline `<script>`)
9+
- Do not report missing `?html` when auto-escaping is active for the file's output format and no override disables it
10+
11+
#### Template Injection / RCE (SSTI)
12+
- User-controlled data concatenated into template source, or templates whose name/body derives from request input (`<#include>`, `<#import>`, `.get_optional_template(userValue)`) — enables server-side template injection
13+
- Use of the `?new()` builtin to instantiate `TemplateModel` classes, especially `freemarker.template.utility.Execute` or `ObjectConstructor` — arbitrary code execution; flag unless the class is a vetted internal type
14+
- `?api` / `?eval` on untrusted input, or exposing raw `Class`/`ClassLoader`/`ProcessBuilder`-reachable objects into the data model
15+
- Templates authored from untrusted input without a restricted `TemplateClassResolver` (e.g. `SAFER_RESOLVER`) — call it out as a hardening gap
16+
17+
#### Null and Missing-Value Handling
18+
- Interpolations or directive arguments on possibly-absent values without `!` (default) or `??` (existence) — missing values raise `InvalidReferenceException` at render time
19+
- Overuse of a bare `!` that masks genuinely-required data with a silent empty string; prefer an explicit default (`value!"fallback"`) or an `<#if value??>` guard where absence is meaningful
20+
- `!` precedence mistakes in expressions (`a.b.c!` guards only the last step); confirm the intended nullable segment
21+
22+
#### Logic-in-Template Smells
23+
- Business logic, data-access, or non-trivial computation embedded in templates that belongs in the controller/model layer
24+
- Deeply nested `<#if>`/`<#list>` or duplicated conditional blocks that indicate the view is doing the model's job
25+
- `<#assign>` used to build state that should have been prepared before rendering
26+
27+
#### Macro and Include Hygiene
28+
- `<#include>` where `<#import>` (namespaced) is intended, causing global-namespace pollution or accidental variable shadowing
29+
- Macros/functions defined but never called, or duplicated across templates instead of shared via a common library template
30+
- Relative template names passed to `<#include>`/`.get_optional_template` without `?absolute_template_name` when resolution context is ambiguous
31+
- Missing-template failures not handled (`.get_optional_template(...).exists`) where the include is optional
32+
33+
#### Internationalization and Locale-Sensitive Formatting
34+
- Numbers, dates, times, and currency emitted with locale-default formatting where a fixed machine format is required (e.g. `?string`/`?c` for numbers in URLs, JSON, or IDs) — `?c` (computer format) prevents locale-dependent thousands separators corrupting non-display output
35+
- Hard-coded user-facing strings that should come from a localized message/resource bundle
36+
- Date/number output relying on an implicit locale/timezone without confirming the render environment sets them intentionally

internal/config/rules/system_rules.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
".github/**/*.{yaml,yml}": "github_config.md",
1313
"**/*.{yaml,yml}": "yaml.md",
1414
"**/*.java": "java.md",
15+
"**/*.{ftl,ftlh,ftlx}": "freemarker.md",
1516
"**/*.ets": "arkts.md",
1617
"**/*.astro": "astro.md",
1718
"**/*.{ts,js,tsx,jsx}": "ts_js_tsx_jsx.md",

internal/config/rules/system_rules_test.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,10 @@ func TestResolve_DefaultRules(t *testing.T) {
5959
}{
6060
{"src/main/java/com/example/foo.java", "Logic Error Detection"},
6161
{"foo.java", "Logic Error Detection"},
62+
{"src/main/resources/templates/email.ftl", "Template Injection"},
63+
{"foo.ftl", "Template Injection"},
64+
{"foo.ftlh", "Template Injection"},
65+
{"foo.ftlx", "Template Injection"},
6266
{"src/main/resources/mapper/usermapper.xml", "SQL Logic Error Detection"},
6367
{"src/main/resources/dao/userdao.xml", "SQL Logic Error Detection"},
6468
{"pom.xml", "snapshot"},

pages/src/content/docs/en/review-rules.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,7 @@ The embedded `system_rules.json` ships with these patterns (in order):
156156
| `.github/**/*.{yaml,yml}` | `github_config.md` — other `.github` config YAML. |
157157
| `**/*.{yaml,yml}` | `yaml.md` |
158158
| `**/*.java` | `java.md` |
159+
| `**/*.{ftl,ftlh,ftlx}` | `freemarker.md` — FreeMarker templates (SSTI / XSS / null handling). |
159160
| `**/*.ets` | `arkts.md` — ArkTS / HarmonyOS. |
160161
| `**/*.{ts,js,tsx,jsx}` | `ts_js_tsx_jsx.md` |
161162
| `**/*.{kt}` | `kotlin.md` |

pages/src/content/docs/ja/review-rules.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,7 @@ OCR は [`bmatcuk/doublestar/v4`](https://pkg.go.dev/github.com/bmatcuk/doublest
119119
| `.github/**/*.{yaml,yml}` | `github_config.md`: その他の `.github` 設定 YAML。 |
120120
| `**/*.{yaml,yml}` | `yaml.md` |
121121
| `**/*.java` | `java.md` |
122+
| `**/*.{ftl,ftlh,ftlx}` | `freemarker.md`: FreeMarker テンプレート(SSTI / XSS / null 処理)。 |
122123
| `**/*.ets` | `arkts.md`: ArkTS / HarmonyOS。 |
123124
| `**/*.{ts,js,tsx,jsx}` | `ts_js_tsx_jsx.md` |
124125
| `**/*.{kt}` | `kotlin.md` |

pages/src/content/docs/zh/review-rules.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,7 @@ OCR 用 [`bmatcuk/doublestar/v4`](https://pkg.go.dev/github.com/bmatcuk/doublest
140140
| `.github/**/*.{yaml,yml}` | `github_config.md`——其他 `.github` 配置 YAML。 |
141141
| `**/*.{yaml,yml}` | `yaml.md` |
142142
| `**/*.java` | `java.md` |
143+
| `**/*.{ftl,ftlh,ftlx}` | `freemarker.md`——FreeMarker 模板(SSTI / XSS / null 处理)。 |
143144
| `**/*.ets` | `arkts.md`——ArkTS / HarmonyOS。 |
144145
| `**/*.{ts,js,tsx,jsx}` | `ts_js_tsx_jsx.md` |
145146
| `**/*.{kt}` | `kotlin.md` |

0 commit comments

Comments
 (0)