fix: sort and sort_natural filters bypass ownPropertyOnly (#869)

Use _getFromScope for property access in sort/sort_natural filters to respect the ownPropertyOnly security option, preventing prototype chain traversal that could leak sensitive inherited properties.

Also extract shared sortBy helper, add orderedCompare with nil handling consistent with caseInsensitiveCompare and Ruby Liquid.

Made-with: Cursor

Author: harttle

src/context/scope.ts

@@ -1,6 +1,6 @@
 import { Drop } from '../drop/drop'
 
-interface ScopeObject extends Record<string, any> {
+interface ScopeObject extends Record<string | number | symbol, any> {
   toLiquid?: () => any;
 }
 

src/filters/array.ts

@@ -1,4 +1,4 @@
-import { toArray, argumentsToValue, toValue, stringify, caseInsensitiveCompare, isArray, isNil, last as arrayLast, isArrayLike, toEnumerable } from '../util'
+import { toArray, argumentsToValue, toValue, stringify, caseInsensitiveCompare, orderedCompare, isArray, isNil, last as arrayLast, isArrayLike, toEnumerable } from '../util'
 import { arrayIncludes, equals, evalToken, isTruthy } from '../render'
 import { Value, FilterImpl } from '../template'
 import { Tokenizer } from '../parser'
@@ -20,8 +20,8 @@ export const reverse = argumentsToValue(function (this: FilterImpl, v: any[]) {
   return [...array].reverse()
 })
 
-export function * sort<T> (this: FilterImpl, arr: T[], property?: string): IterableIterator<unknown> {
-  const values: [T, string | number][] = []
+function * sortBy<T> (this: FilterImpl, arr: T[], property: string | undefined, comparator: (a: unknown, b: unknown) => number): IterableIterator<unknown> {
+  const values: [T, unknown][] = []
   const array = toArray(arr)
   this.context.memoryLimit.use(array.length)
   for (const item of array) {
@@ -30,21 +30,15 @@ export function * sort<T> (this: FilterImpl, arr: T[], property?: string): Itera
       property ? yield this.context._getFromScope(item, stringify(property).split('.'), false) : item
     ])
   }
-  return values.sort((lhs, rhs) => {
-    const lvalue = lhs[1]
-    const rvalue = rhs[1]
-    return lvalue < rvalue ? -1 : (lvalue > rvalue ? 1 : 0)
-  }).map(tuple => tuple[0])
+  return values.sort((lhs, rhs) => comparator(lhs[1], rhs[1])).map(tuple => tuple[0])
 }
 
-export function sort_natural<T> (this: FilterImpl, input: T[], property?: string) {
-  const propertyString = stringify(property)
-  const compare = property === undefined
-    ? caseInsensitiveCompare
-    : (lhs: T, rhs: T) => caseInsensitiveCompare(lhs[propertyString], rhs[propertyString])
-  const array = toArray(input)
-  this.context.memoryLimit.use(array.length)
-  return [...array].sort(compare)
+export function * sort<T> (this: FilterImpl, arr: T[], property?: string): IterableIterator<unknown> {
+  return yield * sortBy.call(this, arr, property, orderedCompare)
+}
+
+export function * sort_natural<T> (this: FilterImpl, arr: T[], property?: string): IterableIterator<unknown> {
+  return yield * sortBy.call(this, arr, property, caseInsensitiveCompare)
 }
 
 export const size = (v: string | any[]) => (v && v.length) || 0

src/util/underscore.ts

@@ -170,11 +170,20 @@ export function ellipsis (str: string, N: number): string {
   return str.length > N ? str.slice(0, N - 3) + '...' : str
 }
 
+export function orderedCompare (a: any, b: any) {
+  if (isNil(a) && isNil(b)) return 0
+  if (isNil(a)) return 1
+  if (isNil(b)) return -1
+  if (a < b) return -1
+  if (a > b) return 1
+  return 0
+}
+
 // compare string in case-insensitive way, undefined values to the tail
 export function caseInsensitiveCompare (a: any, b: any) {
-  if (a == null && b == null) return 0
-  if (a == null) return 1
-  if (b == null) return -1
+  if (isNil(a) && isNil(b)) return 0
+  if (isNil(a)) return 1
+  if (isNil(b)) return -1
   a = toLowerCase.call(a)
   b = toLowerCase.call(b)
   if (a < b) return -1

test/integration/filters/array.spec.ts

@@ -315,6 +315,26 @@ describe('filters/array', function () {
     it('should return empty array for nil value', () => {
       return test('{{notDefined | sort | size}}', {}, '0')
     })
+    it('should respect ownPropertyOnly', async () => {
+      const engine = new Liquid({ ownPropertyOnly: true })
+      const a = Object.create({ secret: 'ccc' })
+      a.name = 'a'
+      const b = Object.create({ secret: 'aaa' })
+      b.name = 'b'
+      const html = await engine.parseAndRender(
+        '{{ arr | sort: "secret" | map: "name" | join: "," }}',
+        { arr: [a, b] }
+      )
+      expect(html).toBe('a,b')
+    })
+    it('should handle nil property values', async () => {
+      const arr = [{ age: 'cc' }, { name: 'x' }, { age: 'aa' }, { age: 'bb' }]
+      await test('{% assign sorted = arr | sort: "age" %}{% for item in sorted %}[{{ item.age }}]{% endfor %}', { arr }, '[aa][bb][cc][]')
+    })
+    it('should handle mixed-type items', async () => {
+      const arr = ['40', null, 30, undefined, true, false, 0, 'str', 50]
+      await test('{% assign sorted = arr | sort %}{% for item in sorted %}[{{ item }}]{% endfor %}', { arr }, '[false][0][true][30][40][str][50][][]')
+    })
   })
   describe('sort_natural', function () {
     it('should sort alphabetically', () => {
@@ -360,6 +380,23 @@ describe('filters/array', function () {
       { students: undefined },
       '0'
     ))
+    it('should respect ownPropertyOnly', async () => {
+      const engine = new Liquid({ ownPropertyOnly: true })
+      const target = Object.create({ secret: 'bbb' })
+      const html = await engine.parseAndRender(
+        '{{ arr | sort_natural: "secret" | map: "secret" | join: "," }}',
+        { arr: [{ secret: 'ccc' }, target, { secret: 'aaa' }] }
+      )
+      expect(html).toBe('aaa,ccc,')
+    })
+    it('should handle nil property values', async () => {
+      const arr = [{ age: '40' }, { name: 'x' }, { age: 30 }, { age: 50 }]
+      await test('{% assign sorted = arr | sort_natural: "age" %}{% for item in sorted %}[{{ item.age }}]{% endfor %}', { arr }, '[30][40][50][]')
+    })
+    it('should handle mixed-type items', async () => {
+      const arr = ['40', null, 30, undefined, true, false, 0, 'str', 50]
+      await test('{% assign sorted = arr | sort_natural %}{% for item in sorted %}[{{ item }}]{% endfor %}', { arr }, '[0][30][40][50][false][str][true][][]')
+    })
   })
   describe('uniq', function () {
     it('should uniq string list', function () {