Skip to content

Commit

Permalink
Optimize allowlist handling (#3804)
Browse files Browse the repository at this point in the history
* Optimize allowlist handling

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

* Use start_with?
  • Loading branch information
robinkar authored Sep 20, 2024
1 parent 63b3ba2 commit a40d82a
Showing 1 changed file with 9 additions and 10 deletions.
19 changes: 9 additions & 10 deletions apps/dashboard/app/models/allowlist_policy.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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

0 comments on commit a40d82a

Please sign in to comment.