Skip to content

Commit 98b0cef

Browse files
committed
fix: reject escaping template symlinks
1 parent 0729657 commit 98b0cef

2 files changed

Lines changed: 85 additions & 9 deletions

File tree

src/template-resolver.js

Lines changed: 41 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -24,25 +24,39 @@ function invalidTemplateNameError(label, name) {
2424
);
2525
}
2626

27+
function isPathContained(root, target) {
28+
const relativePath = path.relative(root, target);
29+
return (
30+
relativePath !== '..' &&
31+
!relativePath.startsWith(`..${path.sep}`) &&
32+
!path.isAbsolute(relativePath)
33+
);
34+
}
35+
2736
function resolveNamedTemplatePath(directory, name, label) {
2837
if (typeof name !== 'string' || !TEMPLATE_NAME_PATTERN.test(name)) {
2938
throw invalidTemplateNameError(label, name);
3039
}
3140

3241
const templateRoot = path.resolve(directory);
3342
const templatePath = path.resolve(templateRoot, `${name}.json`);
34-
const relativePath = path.relative(templateRoot, templatePath);
35-
if (
36-
relativePath === '..' ||
37-
relativePath.startsWith(`..${path.sep}`) ||
38-
path.isAbsolute(relativePath)
39-
) {
43+
if (!isPathContained(templateRoot, templatePath)) {
4044
throw invalidTemplateNameError(label, name);
4145
}
4246

4347
return templatePath;
4448
}
4549

50+
function canonicalizeNamedTemplatePath(directory, templatePath, name, label) {
51+
const canonicalRoot = fs.realpathSync(directory);
52+
const canonicalTemplatePath = fs.realpathSync(templatePath);
53+
if (!isPathContained(canonicalRoot, canonicalTemplatePath)) {
54+
throw invalidTemplateNameError(label, name);
55+
}
56+
57+
return canonicalTemplatePath;
58+
}
59+
4660
function isIdentifierChar(char) {
4761
if (!char) return false;
4862
const code = char.charCodeAt(0);
@@ -137,11 +151,17 @@ class TemplateResolver {
137151
throw new Error(`Config not found: ${config} (looked in ${configPath})`);
138152
}
139153

154+
const canonicalConfigPath = canonicalizeNamedTemplatePath(
155+
this.templatesDir,
156+
configPath,
157+
config,
158+
'config'
159+
);
140160
return {
141161
kind: 'static',
142162
name: config,
143163
params: null,
144-
loadedConfig: JSON.parse(fs.readFileSync(configPath, 'utf8')),
164+
loadedConfig: JSON.parse(fs.readFileSync(canonicalConfigPath, 'utf8')),
145165
};
146166
}
147167

@@ -179,7 +199,13 @@ class TemplateResolver {
179199
throw new Error(`Base template not found: ${baseName} (looked in ${templatePath})`);
180200
}
181201

182-
const templateJson = fs.readFileSync(templatePath, 'utf8');
202+
const canonicalTemplatePath = canonicalizeNamedTemplatePath(
203+
this.baseTemplatesDir,
204+
templatePath,
205+
baseName,
206+
'base template'
207+
);
208+
const templateJson = fs.readFileSync(canonicalTemplatePath, 'utf8');
183209
const template = JSON.parse(templateJson);
184210

185211
return this.resolveTemplate(template, params);
@@ -482,7 +508,13 @@ class TemplateResolver {
482508
if (!fs.existsSync(templatePath)) {
483509
return null;
484510
}
485-
const template = JSON.parse(fs.readFileSync(templatePath, 'utf8'));
511+
const canonicalTemplatePath = canonicalizeNamedTemplatePath(
512+
this.baseTemplatesDir,
513+
templatePath,
514+
baseName,
515+
'base template'
516+
);
517+
const template = JSON.parse(fs.readFileSync(canonicalTemplatePath, 'utf8'));
486518
return {
487519
name: template.name,
488520
description: template.description,

tests/template-resolver.test.js

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -217,6 +217,50 @@ describe('TemplateResolver config references', function () {
217217
assert.strictEqual(parameterizedReference.loadedConfig.agents[0].id, 'parameterized-agent');
218218
});
219219

220+
it('should load symlinks whose canonical targets remain inside each template root', function () {
221+
fs.symlinkSync(
222+
path.join(templatesDir, 'safe-config_1.json'),
223+
path.join(templatesDir, 'static-alias.json')
224+
);
225+
fs.symlinkSync(
226+
path.join(templatesDir, 'base-templates', 'safe-base_template.json'),
227+
path.join(templatesDir, 'base-templates', 'base-alias.json')
228+
);
229+
230+
const staticReference = resolver.resolveConfigReference('static-alias');
231+
assert.strictEqual(staticReference.loadedConfig.agents[0].id, 'static-agent');
232+
233+
const parameterizedReference = resolver.resolveConfigReference({
234+
base: 'base-alias',
235+
params: {},
236+
});
237+
assert.strictEqual(parameterizedReference.loadedConfig.agents[0].id, 'parameterized-agent');
238+
});
239+
240+
it('should reject safe-named symlinks escaping each template root', function () {
241+
fs.symlinkSync(
242+
path.join(tempDir, 'outside-static.json'),
243+
path.join(templatesDir, 'escaped-static.json')
244+
);
245+
fs.symlinkSync(
246+
path.join(templatesDir, 'outside-base.json'),
247+
path.join(templatesDir, 'base-templates', 'escaped-base.json')
248+
);
249+
250+
assert.throws(
251+
() => resolver.resolveConfigReference('escaped-static'),
252+
/Invalid config name/
253+
);
254+
assert.throws(
255+
() => resolver.resolveConfigReference({ base: 'escaped-base', params: {} }),
256+
/Invalid base template name/
257+
);
258+
assert.throws(
259+
() => resolver.getTemplateInfo('escaped-base'),
260+
/Invalid base template name/
261+
);
262+
});
263+
220264
it('should preserve not-found and config-shape behavior for safe names', function () {
221265
assert.throws(
222266
() => resolver.resolveConfigReference('does-not-exist'),

0 commit comments

Comments
 (0)