Various app improvements (#4278)

Co-authored-by: KristofferC <[email protected]>
(cherry picked from commit 109eaea)

Author: KristofferC

src/Apps/Apps.jl

@@ -15,8 +15,41 @@ julia_bin_path() = joinpath(first(DEPOT_PATH), "bin")
 
 app_context() = Context(env=EnvCache(joinpath(app_env_folder(), "Project.toml")))
 
+function validate_app_name(name::AbstractString)
+    if isempty(name)
+        error("App name cannot be empty")
+    end
+    if !occursin(r"^[a-zA-Z][a-zA-Z0-9_-]*$", name)
+        error("App name must start with a letter and contain only letters, numbers, underscores, and hyphens")
+    end
+    if occursin(r"\.\.", name) || occursin(r"[/\\]", name)
+        error("App name cannot contain path traversal sequences or path separators")
+    end
+end
+
+function validate_package_name(name::AbstractString)
+    if isempty(name)
+        error("Package name cannot be empty")
+    end
+    if !occursin(r"^[a-zA-Z][a-zA-Z0-9_]*$", name)
+        error("Package name must start with a letter and contain only letters, numbers, and underscores")
+    end
+end
+
+function validate_submodule_name(name::Union{AbstractString,Nothing})
+    if name !== nothing
+        if isempty(name)
+            error("Submodule name cannot be empty")
+        end
+        if !occursin(r"^[a-zA-Z][a-zA-Z0-9_]*$", name)
+            error("Submodule name must start with a letter and contain only letters, numbers, and underscores")
+        end
+    end
+end
+
 
 function rm_shim(name; kwargs...)
+    validate_app_name(name)
     Base.rm(joinpath(julia_bin_path(), name * (Sys.iswindows() ? ".bat" : "")); kwargs...)
 end
 
@@ -38,6 +71,30 @@ function overwrite_file_if_different(file, content)
     end
 end
 
+function check_apps_in_path(apps)
+    for app_name in keys(apps)
+        which_result = Sys.which(app_name)
+        if which_result === nothing
+            @warn """
+            App '$app_name' was installed but is not available in PATH.
+            Consider adding '$(julia_bin_path())' to your PATH environment variable.
+            """ maxlog=1
+            break  # Only show warning once per installation
+        else
+            # Check for collisions
+            expected_path = joinpath(julia_bin_path(), app_name * (Sys.iswindows() ? ".bat" : ""))
+            if which_result != expected_path
+                @warn """
+                App '$app_name' collision detected:
+                Expected: $expected_path
+                Found: $which_result
+                Another application with the same name exists in PATH.
+                """
+            end
+        end
+    end
+end
+
 function get_max_version_register(pkg::PackageSpec, regs)
     max_v = nothing
     tree_hash = nothing
@@ -157,6 +214,7 @@ function add(pkg::PackageSpec)
     precompile(pkg.name)
 
     @info "For package: $(pkg.name) installed apps $(join(keys(project.apps), ","))"
+    check_apps_in_path(project.apps)
 end
 
 function develop(pkg::Vector{PackageSpec})
@@ -192,6 +250,7 @@ function develop(pkg::PackageSpec)
     _resolve(manifest, pkg.name)
     precompile(pkg.name)
     @info "For package: $(pkg.name) installed apps: $(join(keys(project.apps), ","))"
+    check_apps_in_path(project.apps)
 end
 
 
@@ -205,6 +264,7 @@ function update(pkgs_or_apps::Vector)
     end
 end
 
+# XXX: Is updating an app ever different from rm-ing and adding it from scratch?
 function update(pkg::Union{PackageSpec, Nothing}=nothing)
     ctx = app_context()
     manifest = ctx.env.manifest
@@ -385,21 +445,30 @@ const SHIM_VERSION = 1.0
 const SHIM_HEADER = """$SHIM_COMMENT This file is generated by the Julia package manager.
                        $SHIM_COMMENT Shim version: $SHIM_VERSION"""
 
-
 function generate_shims_for_apps(pkgname, apps, env, julia)
     for (_, app) in apps
         generate_shim(pkgname, app, env, julia)
     end
 end
 
 function generate_shim(pkgname, app::AppInfo, env, julia)
+    validate_package_name(pkgname)
+    validate_app_name(app.name)
+    validate_submodule_name(app.submodule)
+
+    module_spec = app.submodule === nothing ? pkgname : "$(pkgname).$(app.submodule)"
+    
     filename = app.name * (Sys.iswindows() ? ".bat" : "")
     julia_bin_filename = joinpath(julia_bin_path(), filename)
-    mkpath(dirname(filename))
+    mkpath(dirname(julia_bin_filename))
     content = if Sys.iswindows()
-        windows_shim(pkgname, app, julia, env)
+        julia_escaped = "\"$(Base.shell_escape_wincmd(julia))\""
+        module_spec_escaped = "\"$(Base.shell_escape_wincmd(module_spec))\""
+        windows_shim(julia_escaped, module_spec_escaped, env)
     else
-        bash_shim(pkgname, app, julia, env)
+        julia_escaped = Base.shell_escape(julia)
+        module_spec_escaped = Base.shell_escape(module_spec)
+        bash_shim(julia_escaped, module_spec_escaped, env)
     end
     overwrite_file_if_different(julia_bin_filename, content)
     if Sys.isunix()
@@ -408,24 +477,22 @@ function generate_shim(pkgname, app::AppInfo, env, julia)
 end
 
 
-function bash_shim(pkgname, app::AppInfo, julia::String, env)
-    module_spec = app.submodule === nothing ? pkgname : "$(pkgname).$(app.submodule)"
+function bash_shim(julia_escaped::String, module_spec_escaped::String, env)
     return """
         #!/usr/bin/env bash
 
         $SHIM_HEADER
 
         export JULIA_LOAD_PATH=$(repr(env))
         export JULIA_DEPOT_PATH=$(repr(join(DEPOT_PATH, ':')))
-        exec $julia \\
+        exec $julia_escaped \\
             --startup-file=no \\
-            -m $(module_spec) \\
+            -m $module_spec_escaped \\
             "\$@"
         """
 end
 
-function windows_shim(pkgname, app::AppInfo, julia::String, env)
-    module_spec = app.submodule === nothing ? pkgname : "$(pkgname).$(app.submodule)"
+function windows_shim(julia_escaped::String, module_spec_escaped::String, env)
     return """
         @echo off
 
@@ -435,9 +502,9 @@ function windows_shim(pkgname, app::AppInfo, julia::String, env)
         set JULIA_LOAD_PATH=$env
         set JULIA_DEPOT_PATH=$(join(DEPOT_PATH, ';'))
 
-        $julia ^
+        $julia_escaped ^
             --startup-file=no ^
-            -m $(module_spec) ^
+            -m $module_spec_escaped ^
             %*
         """
 end

src/Types.jl

@@ -241,7 +241,6 @@ Base.hash(t::Compat, h::UInt) = hash(t.val, h)
 struct AppInfo
     name::String
     julia_command::Union{String, Nothing}
-    julia_version::Union{VersionNumber, Nothing}
     submodule::Union{String, Nothing}
     other::Dict{String,Any}
 end

src/manifest.jl

@@ -89,7 +89,6 @@ function read_apps(apps::Dict)
         submodule = get(app, "submodule", nothing)
         appinfo = AppInfo(appname::String,
                 app["julia_command"]::String,
-                VersionNumber(app["julia_version"]::String),
                 submodule,
                 app)
         appinfos[appinfo.name] = appinfo
@@ -335,8 +334,7 @@ function destructure(manifest::Manifest)::Dict
             new_entry["apps"] = Dict{String,Any}()
             for (appname, appinfo) in entry.apps
                 julia_command = @something appinfo.julia_command joinpath(Sys.BINDIR, "julia" * (Sys.iswindows() ? ".exe" : ""))
-                julia_version = @something appinfo.julia_version VERSION
-                app_dict = Dict{String,Any}("julia_command" => julia_command, "julia_version" => julia_version)
+                app_dict = Dict{String,Any}("julia_command" => julia_command)
                 if appinfo.submodule !== nothing
                     app_dict["submodule"] = appinfo.submodule
                 end

src/project.jl

@@ -83,7 +83,7 @@ function read_project_apps(raw::Dict{String,Any}, project::Project)
             Expected value for app `$name` to be a dictionary.
         """)
         submodule = get(info, "submodule", nothing)
-        appinfos[name] = AppInfo(name, nothing, nothing, submodule, other)
+        appinfos[name] = AppInfo(name, nothing, submodule, other)
     end
     return appinfos
 end