From 46aa7787e64608eab67c838f4a310c1f6df7d1a2 Mon Sep 17 00:00:00 2001
From: Robin Karlsson <61623634+robinkar@users.noreply.github.com>
Date: Fri, 20 Sep 2024 20:41:15 +0300
Subject: [PATCH] Optimize allowlist handling (#3804)

* Optimize allowlist handling

- Return early if allowlist is disabled.
- Handle child path evaluation as strings.

* Use start_with?
---
 apps/dashboard/app/models/allowlist_policy.rb | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/apps/dashboard/app/models/allowlist_policy.rb b/apps/dashboard/app/models/allowlist_policy.rb
index d8470ef5e0..eb6f668c18 100644
--- a/apps/dashboard/app/models/allowlist_policy.rb
+++ b/apps/dashboard/app/models/allowlist_policy.rb
@@ -17,10 +17,11 @@ def initialize(allowlist)
   # @raises ArgumentError if any allowlist path or permitted? argument
   #         has the form ~user/some/path where user doesn't exist
   def permitted?(path)
+    return true if allowlist.blank?
     real_path = real_expanded_path(path.to_s)
     key = path_to_key(real_path)
     Rails.cache.fetch(key) do
-      allowlist.blank? || allowlist.any? { |parent| child?(Pathname.new(parent), real_path) }
+      allowlist.any? { |parent| child?(Pathname.new(parent), real_path) }
     end
   end
 
@@ -71,19 +72,17 @@ def real_expanded_path_not_exist(path)
     Pathname.new(path).expand_path
   end
 
-  # Determine if child is a subpath of parent
-  #
-  # If the relative path from the child to the supposed parent includes '..'
-  # then child is not a subpath of parent
+  # Determine if child is a subpath of parent.
+  # This does not access the filesystem, so symlinks should be evaluated before this.
   #
   # @param parent [Pathname]
   # @param child [Pathname]
   # @return Boolean
   def child?(parent, child)
-    !child.expand_path.relative_path_from(
-      parent.expand_path
-    ).each_filename.to_a.include?(
-      '..'
-    )
+    # Expand both paths to evaluate any potential ".." components to be able to compare them as strings.
+    p = parent.expand_path.to_s
+    c = child.expand_path.to_s
+    # Child path if it is same as parent path, or has additional components after "/".
+    c.start_with?(p) && (c.size == p.size || c[p.size] == "/")
   end
 end