Skip to content

Commit f41c1fc

Browse files
authored
fix: enforce root containment for renderFile/parseFile lookups (#870)
Made-with: Cursor
1 parent db43485 commit f41c1fc

5 files changed

Lines changed: 31 additions & 10 deletions

File tree

src/fs/loader.spec.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,5 +30,11 @@ describe('fs/loader', function () {
3030
const result = toValueSync(loader.lookup('./foo/bar', LookupType.Partials, true, '/root/current'))
3131
expect(result).toBe(resolve('/root/foo/bar'))
3232
})
33+
it('should enforce containment for LookupType.Root', function () {
34+
const mockFs = { ...fs, existsSync: () => true, exists: async () => true }
35+
const loader = new Loader({ relativeReference: false, fs: mockFs, extname: '', root: ['/safe'] } as any)
36+
expect(() => toValueSync(loader.lookup('/etc/hosts', LookupType.Root, true)))
37+
.toThrow(/ENOENT/)
38+
})
3339
})
3440
})

src/fs/loader.ts

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -43,15 +43,12 @@ export class Loader {
4343

4444
public * lookup (file: string, type: LookupType, sync?: boolean, currentFile?: string): Generator<unknown, string, string> {
4545
const dirs = this.options[type]
46-
const enforceRoot = type !== LookupType.Root
4746
for (const filepath of this.candidates(file, dirs, currentFile)) {
48-
if (enforceRoot) {
49-
let allowed = false
50-
for (const dir of dirs) {
51-
if (yield this.contains(!!sync, dir, filepath)) { allowed = true; break }
52-
}
53-
if (!allowed) continue
47+
let allowed = false
48+
for (const dir of dirs) {
49+
if (yield this.contains(!!sync, dir, filepath)) { allowed = true; break }
5450
}
51+
if (!allowed) continue
5552
if (yield this.exists(!!sync, filepath)) return filepath
5653
}
5754
throw this.lookupError(file, dirs)

test/e2e/render-file.spec.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ describe('#renderFile()', function () {
3838
return expect(html).toContain('"name": "liquidjs"')
3939
})
4040
it('should render file with context', async function () {
41+
engine = new Liquid({ root: views, extname: '.html' })
4142
const html = await engine.renderFile(resolve(views, 'name.html'), { name: 'harttle' })
4243
return expect(html).toBe('My name is harttle.')
4344
})

test/e2e/render-to-node-stream.spec.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,8 @@ import { drainStream } from '../stub/stream'
44
describe('.renderToNodeStream()', function () {
55
it('should render to stream in Node.js', done => {
66
const cjs = require('../../dist/liquid.node')
7-
const engine = new cjs.Liquid()
8-
const tpl = engine.parseFileSync(resolve(__dirname, '../stub/root/foo.html'))
7+
const engine = new cjs.Liquid({ root: resolve(__dirname, '../stub/root/') })
8+
const tpl = engine.parseFileSync('foo.html')
99
const stream = engine.renderToNodeStream(tpl)
1010
let html = ''
1111
stream.on('data', (data: string) => { html += data })

test/integration/liquid/liquid.spec.ts

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,13 +109,30 @@ describe('Liquid', function () {
109109
})
110110
})
111111
describe('#renderFile', function () {
112+
afterEach(restore)
112113
it('should throw with lookup list when file not exist', function () {
113114
const engine = new Liquid({
114115
root: ['/boo', '/root/'],
115116
extname: '.html'
116117
})
117118
return expect(engine.renderFile('/not/exist.html')).rejects.toThrow(/Failed to lookup "\/not\/exist.html" in "\/boo,\/root\/"/)
118119
})
120+
it('should reject absolute paths outside root', async function () {
121+
mock({
122+
'/safe/foo.html': 'safe',
123+
'/etc/secret': 'SECRET'
124+
})
125+
const engine = new Liquid({ root: ['/safe'] })
126+
await expect(engine.renderFile('/etc/secret')).rejects.toThrow(/Failed to lookup/)
127+
})
128+
it('should reject absolute paths outside root (sync)', function () {
129+
mock({
130+
'/safe/foo.html': 'safe',
131+
'/etc/secret': 'SECRET'
132+
})
133+
const engine = new Liquid({ root: ['/safe'] })
134+
expect(() => engine.renderFileSync('/etc/secret')).toThrow(/Failed to lookup/)
135+
})
119136
})
120137
describe('#parseFile', function () {
121138
it('should throw with lookup list when file not exist', function () {
@@ -127,7 +144,7 @@ describe('Liquid', function () {
127144
})
128145
it('should fallback to require.resolve in Node.js', async function () {
129146
const engine = new Liquid({
130-
root: ['/root/'],
147+
root: [process.cwd()],
131148
extname: '.html'
132149
})
133150
const tpls = await engine.parseFileSync('jest')

0 commit comments

Comments
 (0)