Delegate package file validation to hex_core

Author: ericmj

lib/hex/tar.ex

@@ -14,7 +14,11 @@ defmodule Hex.Tar do
         filename -> String.to_charlist(filename)
       end)
 
-    case :mix_hex_tarball.create(metadata, files) do
+    config =
+      :mix_hex_core.default_config()
+      |> Map.put(:tarball_files_root, File.cwd!() |> String.to_charlist())
+
+    case :mix_hex_tarball.create(metadata, files, config) do
       {:ok, %{tarball: tarball} = result} ->
         if output != :memory, do: File.write!(output, tarball)
         result

lib/mix/tasks/hex.build.ex

@@ -55,7 +55,7 @@ defmodule Mix.Tasks.Hex.Build do
     exclude_deps = build.exclude_deps
 
     Hex.Shell.info("Building #{meta.name} #{meta.version}")
-    print_info(meta, organization, exclude_deps, package[:files], build.file_errors)
+    print_info(meta, organization, exclude_deps, package[:files])
 
     if opts[:unpack] do
       output = Keyword.get(opts, :output, "#{meta.name}-#{meta.version}")
@@ -100,21 +100,20 @@ defmodule Mix.Tasks.Hex.Build do
     check_misspellings!(package)
     {organization, package} = Map.pop(package, :organization)
     {deps, exclude_deps} = dependencies()
-    {meta, file_errors} = meta_for(config, package, deps)
+    meta = meta_for(config, package, deps)
 
     %{
       config: config,
       package: package,
       deps: deps,
       exclude_deps: exclude_deps,
-      file_errors: file_errors,
       meta: meta,
       organization: organization
     }
   end
 
   @doc false
-  def print_info(meta, organization, exclude_deps, package_files, file_errors \\ []) do
+  def print_info(meta, organization, exclude_deps, package_files) do
     if meta[:requirements] != [] do
       Hex.Shell.info("  Dependencies:")
 
@@ -138,7 +137,6 @@ defmodule Mix.Tasks.Hex.Build do
       Enum.concat([
         check_missing_fields(meta, organization),
         check_description_length(meta),
-        file_errors,
         check_missing_files(package_files || []),
         check_reserved_files(package_files || []),
         check_excluded_deps(exclude_deps)
@@ -180,14 +178,12 @@ defmodule Mix.Tasks.Hex.Build do
   end
 
   defp meta_for(config, package, deps) do
-    {package, file_errors} =
-      config
-      |> Keyword.take(@root_fields)
-      |> Map.new()
-      |> Map.merge(package)
-      |> package_with_file_errors(config)
-
-    {Map.put(package, :requirements, deps), file_errors}
+    config
+    |> Keyword.take(@root_fields)
+    |> Map.new()
+    |> Map.merge(package)
+    |> package(config)
+    |> Map.put(:requirements, deps)
   end
 
   defp dependencies() do
@@ -254,36 +250,22 @@ defmodule Mix.Tasks.Hex.Build do
 
   @doc false
   def package(package, config) do
-    {package, _file_errors} = package_with_file_errors(package, config)
-    package
-  end
-
-  defp package_with_file_errors(package, config) do
     files = package[:files] || Hex.Package.default_files()
     exclude_patterns = (package[:exclude_patterns] || []) ++ [~r/\W\.DS_Store$/]
-    root = File.cwd!()
-
-    {files, path_errors} =
-      files
-      |> expand_paths(root)
 
     files =
       files
+      |> expand_paths(File.cwd!())
       |> Enum.reject(fn path ->
         Enum.any?(exclude_patterns, &(path =~ &1))
       end)
 
-    file_errors = path_errors ++ check_symlinks(files, root)
-
-    package =
-      package
-      |> Map.put(:files, files)
-      |> maybe_put(:description, package[:description], &String.trim/1)
-      |> maybe_put(:name, package[:name] || config[:app], &to_string(&1))
-      |> maybe_put(:build_tools, !package[:build_tools] && guess_build_tools(files), & &1)
-      |> Map.take(@meta_fields)
-
-    {package, file_errors}
+    package
+    |> Map.put(:files, files)
+    |> maybe_put(:description, package[:description], &String.trim/1)
+    |> maybe_put(:name, package[:name] || config[:app], &to_string(&1))
+    |> maybe_put(:build_tools, !package[:build_tools] && guess_build_tools(files), & &1)
+    |> Map.take(@meta_fields)
   end
 
   defp maybe_put(map, key, value, transform) do
@@ -335,56 +317,13 @@ defmodule Mix.Tasks.Hex.Build do
   defp expand_paths(paths, dir) do
     expand_dir = Path.expand(dir)
 
-    paths =
-      paths
-      |> Enum.flat_map(fn pattern ->
-        dir
-        |> Path.join(pattern)
-        |> Path.wildcard()
-        |> Enum.flat_map(&dir_files/1)
-        |> Enum.map(&{pattern, Path.expand(&1)})
-      end)
-      |> Enum.uniq_by(fn {_pattern, path} -> path end)
-
-    path_errors =
-      paths
-      |> Enum.reject(fn {_pattern, path} -> in_path?(path, expand_dir) end)
-      |> Enum.map(fn {pattern, _path} -> "Path escapes package root: #{pattern}" end)
-      |> Enum.uniq()
-
-    files =
-      paths
-      |> Enum.filter(fn {_pattern, path} -> in_path?(path, expand_dir) end)
-      |> Enum.map(fn {_pattern, path} -> Path.relative_to(path, expand_dir) end)
-
-    {files, path_errors}
-  end
-
-  defp check_symlinks(files, root) do
-    Enum.flat_map(files, fn file ->
-      path = Path.join(root, file)
-
-      case File.lstat(path) do
-        {:ok, %File.Stat{type: :symlink}} ->
-          link_target = File.read_link!(path)
-          resolved_target = Path.expand(link_target, Path.dirname(path))
-
-          if in_path?(resolved_target, root) do
-            []
-          else
-            ["Symlink points outside package root: #{file} -> #{link_target}"]
-          end
-
-        _ ->
-          []
-      end
-    end)
-  end
-
-  defp in_path?(path, root) do
-    path = Path.expand(path)
-    root = Path.expand(root)
-    path == root or String.starts_with?(path, root <> "/")
+    paths
+    |> Enum.map(&Path.join(dir, &1))
+    |> Enum.flat_map(&Path.wildcard/1)
+    |> Enum.flat_map(&dir_files/1)
+    |> Enum.map(&Path.expand/1)
+    |> Enum.uniq()
+    |> Enum.map(&Path.relative_to(&1, expand_dir))
   end
 
   defp dir_files(path) do

lib/mix/tasks/hex.publish.ex

@@ -213,7 +213,7 @@ defmodule Mix.Tasks.Hex.Publish do
     package = build.package
 
     Hex.Shell.info("Building #{meta.name} #{meta.version}")
-    Build.print_info(meta, organization, exclude_deps, package[:files], build.file_errors)
+    Build.print_info(meta, organization, exclude_deps, package[:files])
 
     print_link_to_coc()
     print_public_private(organization)

src/mix_hex_core.erl

@@ -68,6 +68,9 @@
 %% * `tarball_max_uncompressed_size' - Maximum size of uncompressed package tarball, defaults to
 %%   `134_217_728' (128 MiB). Set to `infinity' to not enforce the limit.
 %%
+%% * `tarball_files_root' - Root directory source files must resolve inside when creating tarballs.
+%%   Set to `undefined' to skip source root validation (default: `undefined').
+%%
 %% * `docs_tarball_max_size' - Maximum size of docs tarball, defaults to
 %%   `16_777_216' (16 MiB). Set to `infinity' to not enforce the limit.
 %%
@@ -110,6 +113,7 @@
     repo_verify => boolean(),
     repo_verify_origin => boolean(),
     send_100_continue => boolean(),
+    tarball_files_root => file:filename() | undefined,
     tarball_max_size => pos_integer() | infinity,
     tarball_max_uncompressed_size => pos_integer() | infinity,
     docs_tarball_max_size => pos_integer() | infinity,
@@ -136,6 +140,7 @@ default_config() ->
         repo_verify => true,
         repo_verify_origin => true,
         send_100_continue => true,
+        tarball_files_root => undefined,
         tarball_max_size => 16 * 1024 * 1024,
         tarball_max_uncompressed_size => 128 * 1024 * 1024,
         docs_tarball_max_size => 16 * 1024 * 1024,

src/mix_hex_tarball.erl

@@ -68,14 +68,15 @@ create(Metadata, Files, Config) ->
         tarball_max_size := TarballMaxSize,
         tarball_max_uncompressed_size := TarballMaxUncompressedSize
     } = Config,
+    FilesRoot = maps:get(tarball_files_root, Config, undefined),
 
     MetadataBinary = encode_metadata(Metadata),
 
     case valid_size(MetadataBinary, ?MAX_METADATA_SIZE) of
         false ->
             {error, {tarball, {file_too_big, "metadata.config"}}};
         true ->
-            case validate_create_files(Files) of
+            case validate_create_files(Files, FilesRoot) of
                 ok ->
                     ContentsTarball = create_memory_tarball(Files),
                     ContentsTarballCompressed = gzip(ContentsTarball),
@@ -139,8 +140,9 @@ create_docs(Files, Config) ->
         docs_tarball_max_size := TarballMaxSize,
         docs_tarball_max_uncompressed_size := TarballMaxUncompressedSize
     } = Config,
+    FilesRoot = maps:get(tarball_files_root, Config, undefined),
 
-    case validate_create_files(Files) of
+    case validate_create_files(Files, FilesRoot) of
         ok ->
             UncompressedTarball = create_memory_tarball(Files),
 
@@ -648,23 +650,23 @@ guess_build_tools(Metadata) ->
 %%====================================================================
 
 %% @private
-validate_create_files(Files) when is_list(Files) ->
-    validate_create_files(Files, ok).
+validate_create_files(Files, FilesRoot) when is_list(Files) ->
+    validate_create_files(Files, FilesRoot, ok).
 
-validate_create_files(_Files, {error, _} = Error) ->
+validate_create_files(_Files, _FilesRoot, {error, _} = Error) ->
     Error;
-validate_create_files([], ok) ->
+validate_create_files([], _FilesRoot, ok) ->
     ok;
-validate_create_files([File | Rest], ok) ->
-    validate_create_files(Rest, validate_create_file(File)).
+validate_create_files([File | Rest], FilesRoot, ok) ->
+    validate_create_files(Rest, FilesRoot, validate_create_file(File, FilesRoot)).
 
-validate_create_file({Filename, Contents}) when is_list(Filename), is_binary(Contents) ->
+validate_create_file({Filename, Contents}, _FilesRoot) when is_list(Filename), is_binary(Contents) ->
     validate_archive_path(Filename);
-validate_create_file(Filename) when is_list(Filename) ->
-    validate_create_file({Filename, Filename});
-validate_create_file({Filename, AbsFilename}) when is_list(Filename), is_list(AbsFilename) ->
+validate_create_file(Filename, FilesRoot) when is_list(Filename) ->
+    validate_create_file({Filename, Filename}, FilesRoot);
+validate_create_file({Filename, AbsFilename}, FilesRoot) when is_list(Filename), is_list(AbsFilename) ->
     case validate_archive_path(Filename) of
-        ok -> validate_source_file(Filename, AbsFilename);
+        ok -> validate_source_file(Filename, AbsFilename, FilesRoot);
         {error, _} = Error -> Error
     end.
 
@@ -674,13 +676,13 @@ validate_archive_path(Filename) ->
         true -> ok
     end.
 
-validate_source_file(ArchiveName, SourcePath) ->
+validate_source_file(ArchiveName, SourcePath, FilesRoot) ->
     case file:read_link_info(SourcePath, []) of
         {ok, #file_info{type = Type}} when Type =:= regular; Type =:= directory ->
-            ok;
+            validate_source_root(ArchiveName, SourcePath, FilesRoot);
         {ok, #file_info{type = symlink}} ->
             {ok, LinkTarget} = file:read_link(SourcePath),
-            ResolvedTarget = filename:join(filename:dirname(ArchiveName), LinkTarget),
+            ResolvedTarget = archive_join(archive_dirname(ArchiveName), LinkTarget),
             case safe_relative_archive_path(ResolvedTarget) of
                 false -> {error, {tarball, {unsafe_symlink, ArchiveName, LinkTarget}}};
                 true -> ok
@@ -691,10 +693,18 @@ validate_source_file(ArchiveName, SourcePath) ->
             ok
     end.
 
+validate_source_root(_ArchiveName, _SourcePath, undefined) ->
+    ok;
+validate_source_root(ArchiveName, SourcePath, FilesRoot) ->
+    case filelib:safe_relative_path(SourcePath, filename:absname(FilesRoot)) of
+        unsafe -> {error, {tarball, {unsafe_path, ArchiveName}}};
+        _ -> ok
+    end.
+
 safe_relative_archive_path(Path) ->
-    case filename:pathtype(Path) of
-        relative -> safe_relative_archive_path(filename:split(Path), []);
-        _ -> false
+    case archive_path_absolute(Path) orelse archive_path_drive(Path) of
+        true -> false;
+        false -> safe_relative_archive_path(archive_path_split(Path), [])
     end.
 
 safe_relative_archive_path([], _Acc) ->
@@ -708,6 +718,47 @@ safe_relative_archive_path([".." | Rest], [_ | Acc]) ->
 safe_relative_archive_path([_Part | Rest], Acc) ->
     safe_relative_archive_path(Rest, [ok | Acc]).
 
+archive_path_absolute([$/ | _Rest]) ->
+    true;
+archive_path_absolute([$\\ | _Rest]) ->
+    true;
+archive_path_absolute(_Path) ->
+    false.
+
+archive_path_drive([Drive, $: | _Rest]) when
+    Drive >= $a, Drive =< $z;
+    Drive >= $A, Drive =< $Z
+->
+    true;
+archive_path_drive(_Path) ->
+    false.
+
+archive_path_split(Path) ->
+    string:tokens(Path, "/\\").
+
+archive_dirname(Path) ->
+    case archive_path_split(Path) of
+        [] -> ".";
+        [_Name] -> ".";
+        Parts -> string:join(lists:droplast(Parts), "/")
+    end.
+
+archive_join(_Dir, Path) when Path =:= [] ->
+    Path;
+archive_join(_Dir, Path = [$/ | _Rest]) ->
+    Path;
+archive_join(_Dir, Path = [$\\ | _Rest]) ->
+    Path;
+archive_join(_Dir, Path = [Drive, $: | _Rest]) when
+    Drive >= $a, Drive =< $z;
+    Drive >= $A, Drive =< $Z
+->
+    Path;
+archive_join(".", Path) ->
+    Path;
+archive_join(Dir, Path) ->
+    Dir ++ "/" ++ Path.
+
 %% @private
 unpack_tarball(Source, memory, MaxSize) ->
     case mix_hex_erl_tar:extract(Source, [memory, compressed, {max_size, MaxSize}]) of