Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion src/compile-string.ts
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ export function compileBody(this: Eta, buff: Array<AstObject>): string {
// we know string exists
returnStr += "__eta.res+='" + str + "';\n";
} else {
const type = currentBlock.t; // "r", "e", or "i"
const type = currentBlock.t; // "r", "e", "i" or custom tag name
let content = currentBlock.val || "";

if (config.debug) returnStr += "__eta.line=" + currentBlock.lineNo + "\n";
Expand Down Expand Up @@ -123,6 +123,10 @@ export function compileBody(this: Eta, buff: Array<AstObject>): string {
} else if (type === "e") {
// execute
returnStr += content + "\n";

} else if (type in config.customTags) {

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

customTag is assigned but never used. This line can be removed.


Generated by Claude Code

const customTag = config.customTags[type];

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Security: Code injection via template content

The content variable is interpolated directly into a single-quoted string literal without any escaping:

returnStr += `__eta.res += this.config.customTags['${type}']('${content}', ${config.varName});\n`;

If a template contains something like <%* foo'; console.log('pwned'); ' %>, the single quote in content will break out of the string and allow arbitrary code execution in the compiled template function.

content needs to be properly escaped (at minimum, single quotes and backslashes), or passed through a mechanism that doesn't involve string interpolation into generated code. Consider passing content as a variable reference rather than inlining it as a string literal, similar to how the built-in tag types handle their content.


Generated by Claude Code

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also worth noting: there's no validation that type (the custom tag key) doesn't contain characters that would break out of the bracket notation string (e.g., a key containing '). While users control the keys via customTags config, it's still worth sanitizing or validating tag prefix characters.


Generated by Claude Code

returnStr += `__eta.res += this.config.customTags['${type}']('${content}', ${config.varName});\n`;
}
}
}
Expand Down
4 changes: 4 additions & 0 deletions src/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@ export interface EtaConfig {
/** Holds cache of resolved filepaths. Set to `false` to disable. */
cacheFilepaths: boolean;

/** Object specifying custom tags. Keys are tag prefixes, values are functions which take tag content and return a string. */

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Making customTags a required property on EtaConfig is a breaking change for anyone implementing or extending the interface directly. Consider making it optional (customTags?:) to maintain backwards compatibility.


Generated by Claude Code

customTags: Record<string, (content: string, data: unknown) => string>;

/** Whether to pretty-format error messages (introduces runtime penalties) */
debug: boolean;

Expand Down Expand Up @@ -89,6 +92,7 @@ const defaultConfig: EtaConfig = {
autoTrim: [false, "nl"],
cache: false,
cacheFilepaths: true,
customTags: {},
debug: false,
escapeFunction: XMLEscape,
// default filter function (not used unless enables) just stringifies the input
Expand Down
21 changes: 10 additions & 11 deletions src/parse.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,8 @@ import { ParseErr } from "./err.ts";
import type { Eta } from "./internal.ts";
import { trimWS } from "./utils.ts";

export type TagType = "r" | "e" | "i" | "";

export interface TemplateObject {

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removing the TagType union and widening t to string is a type safety regression that affects all consumers of TemplateObject (including the plugin API's processAST).

Consider keeping the named type and extending it instead:

export type TagType = "r" | "e" | "i" | "" | (string & {});

This preserves autocomplete for the built-in types while still allowing arbitrary custom tag strings.


Generated by Claude Code

t: TagType;
t: string;
val: string;
lineNo?: number;
}
Expand Down Expand Up @@ -40,6 +38,8 @@ export function parse(this: Eta, str: string): Array<AstObject> {
let lastIndex = 0;
const parseOptions = config.parse;

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's no validation that custom tag prefixes don't collide with the built-in prefixes ("", "=", "~") or with the trim indicators ("-", "_"). A collision would cause silent, confusing bugs. Consider adding a check here that warns or throws if a custom prefix conflicts with built-in ones.


Generated by Claude Code


const customTagPrefixes = Object.keys(config.customTags)

if (config.plugins) {
for (let i = 0; i < config.plugins.length; i++) {
const plugin = config.plugins[i];
Expand Down Expand Up @@ -90,6 +90,7 @@ export function parse(this: Eta, str: string): Array<AstObject> {
parseOptions.exec,
parseOptions.interpolate,
parseOptions.raw,
...customTagPrefixes,
].reduce((accumulator, prefix) => {
if (accumulator && prefix) {
return accumulator + "|" + escapeRegExp(prefix);
Expand Down Expand Up @@ -138,14 +139,12 @@ export function parse(this: Eta, str: string): Array<AstObject> {

trimLeftOfNextStr = closeTag[2];

const currentType: TagType =
prefix === parseOptions.exec
? "e"
: prefix === parseOptions.raw
? "r"
: prefix === parseOptions.interpolate
? "i"
: "";
let currentType = "";
if(prefix === config.parse.exec) currentType = "e";
else if(prefix === config.parse.interpolate) currentType = "i";
else if(prefix === config.parse.raw) currentType = "r";
// custom tags
else if(customTagPrefixes.includes(prefix)) currentType = prefix;

Comment on lines +142 to 148

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: missing spaces after if/else if — the rest of the codebase uses if ( with a space. Also missing semicolon on line 39.

Suggested change
let currentType = "";
if(prefix === config.parse.exec) currentType = "e";
else if(prefix === config.parse.interpolate) currentType = "i";
else if(prefix === config.parse.raw) currentType = "r";
// custom tags
else if(customTagPrefixes.includes(prefix)) currentType = prefix;
let currentType = "";
if (prefix === config.parse.exec) currentType = "e";
else if (prefix === config.parse.interpolate) currentType = "i";
else if (prefix === config.parse.raw) currentType = "r";
// custom tags
else if (customTagPrefixes.includes(prefix)) currentType = prefix;

Generated by Claude Code

currentObj = { t: currentType, val: content };
break;
Expand Down
Loading