Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 13 additions & 1 deletion .github/workflows/rubygems.yml
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,14 @@ jobs:
- ruby: { name: truffleruby, value: truffleruby-24.2.1 }
os: { name: Ubuntu, value: ubuntu-24.04 }

- ruby: { name: no symlinks, value: 4.0.0 }
os: { name: Windows, value: windows-2025 }
symlink: off

steps:
- name: disable development mode on Windows
run: powershell -c "Set-ItemProperty -Path HKLM:\SOFTWARE\Microsoft\Windows\CurrentVersion\AppModelUnlock -Name AllowDevelopmentWithoutDevLicense -Value 0"
if: matrix.symlink == 'off'
- uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
with:
persist-credentials: false
Expand All @@ -52,7 +59,7 @@ jobs:
run: bin/rake setup
- name: Run Test
run: bin/rake test
if: matrix.ruby.name != 'truffleruby' && matrix.ruby.name != 'jruby'
if: matrix.ruby.name != 'truffleruby' && matrix.ruby.name != 'jruby' && matrix.symlink != 'off'
- name: Run Test isolatedly
run: bin/rake test:isolated
if: matrix.ruby.name == '3.4' && matrix.os.name != 'Windows'
Expand All @@ -62,6 +69,11 @@ jobs:
- name: Run Test (Truffleruby)
run: TRUFFLERUBYOPT="--experimental-options --testing-rubygems" bin/rake test
if: matrix.ruby.name == 'truffleruby'
- name: Run Test with non-Admin user
run: |
gem inst win32-process --no-doc --conservative
ruby bin/windows_run_as_user ruby -S rake test
if: matrix.symlink == 'off'

timeout-minutes: 60

Expand Down
36 changes: 36 additions & 0 deletions bin/windows_run_as_user
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
require "win32/process"
require "rbconfig"

testuser = "testuser"
testpassword = "Password123+"

# Remove a previous test user if present
# See https://learn.microsoft.com/en-us/windows-server/administration/windows-commands/net-user
system("net user #{testuser} /del 2>NUL")
# Create a new non-admin user
system("net user #{testuser} \"#{testpassword}\" /add")

pinfo = nil
IO.pipe do |stdout_read, stdout_write|
cmd = ARGV.join(" ")
env = {
"TMP" => "#{Dir.pwd}/tmp",
"TEMP" => "#{Dir.pwd}/tmp"
}
pinfo = Process.create command_line: cmd,
with_logon: testuser,
password: testpassword,
cwd: Dir.pwd,
environment: ENV.to_h.merge(env).map{|k,v| "#{k}=#{v}" },
startup_info: { stdout: stdout_write, stderr: stdout_write }

stdout_write.close
stdout_read.each_line do |line|
puts(line)
end
end

# Wait for process to terminate
sleep 1 while !(ecode=Process.get_exitcode(pinfo.process_id))

exit ecode
17 changes: 16 additions & 1 deletion lib/rubygems/package.rb
Original file line number Diff line number Diff line change
Expand Up @@ -466,7 +466,7 @@ def extract_tar_gz(io, destination_dir, pattern = "*") # :nodoc:

symlinks.each do |name, target, destination, real_destination|
if File.exist?(real_destination)
File.symlink(target, destination)
create_symlink(target, destination)
else
alert_warning "#{@spec.full_name} ships with a dangling symlink named #{name} pointing to missing #{target} file. Ignoring"
end
Expand Down Expand Up @@ -721,6 +721,21 @@ def limit_read(io, name, limit)
raise Gem::Package::FormatError, "#{name} is too big (over #{limit} bytes)" if bytes.size > limit
bytes
end

if Gem.win_platform?
# Create a symlink and fallback to copy the file or directory on Windows,
# where symlink creation needs special privileges in form of the Developer Mode.
def create_symlink(old_name, new_name)
File.symlink(old_name, new_name)
rescue Errno::EACCES
from = File.expand_path(old_name, File.dirname(new_name))
FileUtils.cp_r(from, new_name)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we be worried about gems that include a symlink where the target is a system path (either maliciously or by mistake) and we'd end up coping recursively its whole content?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

end
else
def create_symlink(old_name, new_name)
File.symlink(old_name, new_name)
end
end
end

require_relative "package/digest_io"
Expand Down
18 changes: 18 additions & 0 deletions test/rubygems/helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1238,6 +1238,24 @@ def nmake_found?
system("nmake /? 1>NUL 2>&1")
end

@@symlink_supported = nil

# This is needed for Windows environment without symlink support enabled (the default
# for non admin) to be able to skip test for features using symlinks.
def symlink_supported?
if @@symlink_supported.nil?
begin
File.symlink(File.join(@tempdir, "a"), File.join(@tempdir, "b"))
rescue NotImplementedError, SystemCallError
@@symlink_supported = false
else
File.unlink(File.join(@tempdir, "b"))
@@symlink_supported = true
end
end
@@symlink_supported
end

# In case we're building docs in a background process, this method waits for
# that process to exit (or if it's already been reaped, or never happened,
# swallows the Errno::ECHILD error).
Expand Down
17 changes: 0 additions & 17 deletions test/rubygems/installer_test_case.rb
Original file line number Diff line number Diff line change
Expand Up @@ -237,21 +237,4 @@ def test_ensure_writable_dir_creates_missing_parent_directories
assert_directory_exists non_existent_parent, "Parent directory should exist now"
assert_directory_exists target_dir, "Target directory should exist now"
end

@@symlink_supported = nil

# This is needed for Windows environment without symlink support enabled (the default
# for non admin) to be able to skip test for features using symlinks.
def symlink_supported?
if @@symlink_supported.nil?
begin
File.symlink("", "")
rescue Errno::ENOENT, Errno::EEXIST
@@symlink_supported = true
rescue NotImplementedError, SystemCallError
@@symlink_supported = false
end
end
@@symlink_supported
end
end
8 changes: 6 additions & 2 deletions test/rubygems/test_gem_installer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -759,8 +759,12 @@ def test_generate_bin_with_dangling_symlink

errors = @ui.error.split("\n")
assert_equal "WARNING: ascii_binder-0.1.10.1 ships with a dangling symlink named bin/ascii_binder pointing to missing bin/asciibinder file. Ignoring", errors.shift
assert_empty errors

if symlink_supported?
assert_empty errors
else
assert_match(/Unable to use symlinks, installing wrapper/i,
errors.to_s)
end
assert_empty @ui.output
end

Expand Down
69 changes: 38 additions & 31 deletions test/rubygems/test_gem_package.rb
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,9 @@ def test_add_files
end

def test_add_files_symlink
unless symlink_supported?
omit("symlink - developer mode must be enabled on Windows")
end
spec = Gem::Specification.new
spec.files = %w[lib/code.rb lib/code_sym.rb lib/code_sym2.rb]

Expand All @@ -185,16 +188,8 @@ def test_add_files_symlink
end

# NOTE: 'code.rb' is correct, because it's relative to lib/code_sym.rb
begin
File.symlink("code.rb", "lib/code_sym.rb")
File.symlink("../lib/code.rb", "lib/code_sym2.rb")
rescue Errno::EACCES => e
if Gem.win_platform?
pend "symlink - must be admin with no UAC on Windows"
else
raise e
end
end
File.symlink("code.rb", "lib/code_sym.rb")
File.symlink("../lib/code.rb", "lib/code_sym2.rb")

package = Gem::Package.new "bogus.gem"
package.spec = spec
Expand Down Expand Up @@ -583,25 +578,45 @@ def test_extract_tar_gz_symlink_relative_path
tar.add_symlink "lib/foo.rb", "../relative.rb", 0o644
end

begin
package.extract_tar_gz tgz_io, @destination
rescue Errno::EACCES => e
if Gem.win_platform?
pend "symlink - must be admin with no UAC on Windows"
else
raise e
end
end
package.extract_tar_gz tgz_io, @destination

extracted = File.join @destination, "lib/foo.rb"
assert_path_exist extracted
assert_equal "../relative.rb",
File.readlink(extracted)
if symlink_supported?
assert_equal "../relative.rb",
File.readlink(extracted)
end
assert_equal "hi",
File.read(extracted),
"should read file content either by following symlink or on Windows by reading copy"
end

def test_extract_tar_gz_symlink_directory
package = Gem::Package.new @gem
package.verify

tgz_io = util_tar_gz do |tar|
tar.add_symlink "link", "lib/orig", 0o644
tar.mkdir "lib", 0o755
tar.mkdir "lib/orig", 0o755
tar.add_file "lib/orig/file.rb", 0o644 do |io|
io.write "ok"
end
end

package.extract_tar_gz tgz_io, @destination
extracted = File.join @destination, "link/file.rb"
assert_path_exist extracted
if symlink_supported?
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small nitpick, maybe it would be easier without this condition and just check that reading the file works and the content is "ok", regardless if it's a symlink or not

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is a valid addition: If symlinks are supported then a symlink should actually be created. So I wouldn't remove it.

assert_equal "lib/orig",
File.readlink(File.dirname(extracted))
end
assert_equal "ok",
File.read(extracted)
end

def test_extract_symlink_into_symlink_dir
omit "Symlinks not supported or not enabled" unless symlink_supported?
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this test be skipped? Since we now support extracting symlinks even is symlink is not supported

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test fails if symlink_supported?==false , since it checks extracting into a symlinked directory and that symlink creation fails.

package = Gem::Package.new @gem
tgz_io = util_tar_gz do |tar|
tar.mkdir "lib", 0o755
Expand Down Expand Up @@ -665,14 +680,10 @@ def test_extract_symlink_parent
destination_subdir = File.join @destination, "subdir"
FileUtils.mkdir_p destination_subdir

expected_exceptions = Gem.win_platform? ? [Gem::Package::SymlinkError, Errno::EACCES] : [Gem::Package::SymlinkError]

e = assert_raise(*expected_exceptions) do
e = assert_raise(Gem::Package::SymlinkError) do
package.extract_tar_gz tgz_io, destination_subdir
end

pend "symlink - must be admin with no UAC on Windows" if Errno::EACCES === e

assert_equal("installing symlink 'lib/link' pointing to parent path #{@destination} of " \
"#{destination_subdir} is not allowed", e.message)

Expand Down Expand Up @@ -700,14 +711,10 @@ def test_extract_symlink_parent_doesnt_delete_user_dir
tar.add_symlink "link/dir", ".", 16_877
end

expected_exceptions = Gem.win_platform? ? [Gem::Package::SymlinkError, Errno::EACCES] : [Gem::Package::SymlinkError]

e = assert_raise(*expected_exceptions) do
e = assert_raise(Gem::Package::SymlinkError) do
package.extract_tar_gz tgz_io, destination_subdir
end

pend "symlink - must be admin with no UAC on Windows" if Errno::EACCES === e

assert_equal("installing symlink 'link' pointing to parent path #{destination_user_dir} of " \
"#{destination_subdir} is not allowed", e.message)

Expand Down