[FIX] Sqlite3 Should create db for missing parent directories as well

[maniSHarma7575]

Motivation / Background

This Pull Request has been created to fix #51623

ActiveRecord::ConnectionAdapters::SQLite3Adapter#initialize will check if a sqlite3 database file path exists, and if not it will attempt to create the directory using Dir.mkdir. However, Dir.mkdir cannot create all parent directories like FileUtils.mkdir_p. Thus an No such file or directory @ dir_s_mkdir - does/not/exist/yet (Errno::ENOENT) will be raised if one of the parent-parent directories of the sqlite3 database file do not exist yet.

Detail

This Pull Request changes:
ActiveRecord::ConnectionAdapters::SQLite3Adapter#initialize

Replaced the Dir.mkdir(dirname) with FileUtils.mkdir_p(dirname) to create the directory for the parent directories in the path if they don't exists.

Additional information

Checklist

Before submitting the PR make sure the following are checked:

• This Pull Request is related to one change. Unrelated changes should be opened in separate PRs.
• Commit message has a detailed description of what changed and why. If this PR fixes a related issue include it in the commit message. Ex: [Fix #51623 ]
• Tests are added or updated if you fix a bug or add a feature.
• CHANGELOG files are updated for the changed libraries if there is a behavior change or additional feature. Minor bug fixes and documentation changes should not be included.

[maniSHarma7575]

@fatkodima this is ready for review? Please see if we can merge this.

[byroot]

This test shouldn't be deleted but updated so it still simulate a failure. Of course it's trickier now that mkdir_p is used.

One way could be to create a temporary directory and change its permissions. e.g.

dir = Dir.mktmpdir
File.chmod(0x000, dir)
db_path  = File.join(dir, "db/cant-be-created.db")

[maniSHarma7575]

@byroot, observe the alterations made:

Before:

begin
  Dir.mkdir(dirname)
rescue Errno::ENOENT => error
  if error.message.include?("No such file or directory")
    raise ActiveRecord::NoDatabaseError.new(connection_pool: @pool)
  else
    raise
  end
end

Now:

  FileUtils.mkdir_p(dirname)

The removal of the rescue block is notable. Why?

1. The likelihood of encountering an Errno::ENOENT exception diminishes since the file or directory will now be created.
2. In scenarios where permission constraints prevent directory or file creation, triggering a failure, an Errno::EACCES exception will be raised instead.

Incorporating a failure test, I've implemented a test here activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb:16

def test_database_should_not_get_created_for_parent_directories_with_missing_permission
  dir = Dir.mktmpdir
  File.chmod(0x000, dir)
  db_path = File.join(dir, "db/cant-be-created.db")
  assert_raise Errno::EACCES do
    connection = SQLite3Adapter.new(adapter: "sqlite3", database: db_path)
    connection.drop_table "ex", if_exists: true
  end
end

This test is green on my local machine. And it's strange that in buildkite/rails CI it's failing. Could you please check here what I am doing wrong?

bin/test test/cases/adapters/sqlite3/sqlite3_adapter_test.rb
Using sqlite3
Run options: --seed 52964

# Running:

..................................................................................

Finished in 0.070632s, 1160.9469 runs/s, 2718.3146 assertions/s.
82 runs, 192 assertions, 0 failures, 0 errors, 0 skips

[byroot]

Not sure, try File.chmod(0x000, dir) (not FileUtils).

Also the bubbled exception shouldn't be an errno, we should handle all SyscallError and wrap them in ActiveRecord::NoDatabaseError.

[maniSHarma7575]

Thanks, @byroot! I also attempted to use File.chmod(0x000, dir).
https://github.com/rails/rails/pull/51628/files#diff-91ca39a63c4ff81c0580c12472a0a709c1d9c428a3ace4970ff3076d90ba6b13

But that doesn't seems to work.
Interestingly, the same test runs fine on my local machine.

I debugged and discovered something unexpected in CI.

https://buildkite.com/rails/rails/builds/106656#018f1f16-c702-468a-9f0b-d14d5eeba392

That parent directory permission mode is: /tmp/d20240427-17-857wvl: 40000

And this parent directory is still able to make the child directorydb with /tmp/d20240427-17-857wvl/db: 40755

[maniSHarma7575]

@byroot , I've identified the issue above and why it's exhibiting this unusual behavior.

To replicate the behavior, I conducted independent experiments with the FileUtils.mkdir_p method. Consequently, I crafted a script to test the method:

require 'test/unit'
require 'fileutils'
require 'tmpdir'
require 'debug'

class TestFileUtils < Test::Unit::TestCase
  def test_write_independent_test_to_check_fileutils
    Dir.mktmpdir do |dir|
      # Create a directory with restricted permissions
      File.chmod(0o000, dir)

      # Attempt to create a directory within the restricted directory
      db_path = File.join(dir, "check-fileutils/db/-cinco-dog.sqlite3")

      assert_raise Errno::EACCES do
        FileUtils.mkdir_p(db_path)
      end
    end
  end
end

When I executed the above script on my local machine with Ruby installed (ruby 3.2.2 (2023-03-30 revision e51014f9c0) [arm64-darwin22]), it ran without issues:

(base)  manishsharma@Manishs-MacBook-Air  ~/Development/personal/opensource/experiment  ruby reproduce.rb
Loaded suite reproduce
Started
.
Finished in 0.000621 seconds.
------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
1 tests, 1 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications
100% passed
------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
1610.31 tests/s, 1610.31 assertions/s

Since Builtkite Rails CI runs on Docker, I decided to run the same script within a Docker container. For this purpose, I created the following Dockerfile:

FROM ruby:latest
COPY reproduce.rb .
ENTRYPOINT ["ruby", "reproduce.rb"]

The reproduce.rb script remains unchanged.

Here's the output from running and building the image:

(base)  manishsharma@Manishs-MacBook-Air  ~/Development/personal/opensource/experiment  docker run fileutils-reproduce
Loaded suite reproduce
Started
F
===============================================================================
Failure: test_write_independent_test_to_check_fileutils(TestFileUtils): <Errno::EACCES> exception was expected but none was thrown.
reproduce.rb:15:in `block in test_write_independent_test_to_check_fileutils'
     12:       # Attempt to create a directory within the restricted directory
     13:       db_path = File.join(dir, "check-fileutils/db/-cinco-dog.sqlite3")
     14:
  => 15:       assert_raise Errno::EACCES  do
     16:         FileUtils.mkdir_p(db_path)
     17:       end
     18:     end
/usr/local/lib/ruby/3.3.0/tmpdir.rb:99:in `mktmpdir'
reproduce.rb:8:in `test_write_independent_test_to_check_fileutils'
===============================================================================
Finished in 0.003941125 seconds.
-------------------------------------------------------------------------------
1 tests, 1 assertions, 1 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications
0% passed
-------------------------------------------------------------------------------
253.73 tests/s, 253.73 assertions/s

The question arises: Why does the Docker environment not raise an exception?

Upon inspecting the Docker container, I discovered that chmod sets the correct permissions for the directory:

# pwd
/tmp
# ls -la
total 16
drwxrwxrwt 1 root root 4096 Apr 27 15:07 .
drwxr-xr-x 1 root root 4096 Apr 27 15:03 ..
d--------- 2 root root 4096 Apr 27 15:03 d20240427-1-ihvptr
d--------- 3 root root 4096 Apr 27 15:09 d20240427-24-sa4xz6

Creating another directory inside d20240427-24-sa4xz6 occurs without errors because both the owner and group have root privileges:

# cd d20240427-24-sa4xz6
# mkdir -p create-without-issue
# ls -la
total 16
d--------- 4 root root 4096 Apr 27 15:27 .
drwxrwxrwt 1 root root 4096 Apr 27 15:07 ..
drwxr-xr-x 2 root root 4096 Apr 27 15:27 create-without-issue
drwxr-xr-x 2 root root 4096 Apr 27 15:09 temp

In summary, the root user can create files in a directory marked as Read Only.

Confirmed the user in Builkite Rails CI pipeline as well it's a root user:

https://buildkite.com/rails/rails/builds/106668#018f2402-28f6-4a89-a27d-04b675b70ed6

[manishElitmus]

@byroot , did you have a chance to look at it?

[maniSHarma7575]

Tried solution:

1. Make directories immutable (using chattr or chflags)

with chattr command it gives an error:
https://buildkite.com/rails/rails/builds/106846#018f48ea-55a2-4af2-9f8d-acf4c38f1a57

chattr: Operation not permitted while setting flags on <directory-name>

Outcome: Docker Deny access to some filesystem operations, like creating new device nodes, changing the owner of files, or altering attributes (including the immutable flag).

It's mentioned here: https://docs.docker.com/engine/security/#linux-kernel-capabilities

Solution to solve this is: This is related to capabilities thing: chattr requires CAPLINUX IMMUTABLE which is disabled in docker by default. Just add -- cap-add
LINUX_ IMMUTABLE to docker container start options to enable it.

That's itself another issue on. I don't know whether to proceed with this or not as we have to do modification in CI.

[byroot]

Sorry, got side tracked, looking at it now.

[byroot]

Alright, I think that the issue is CI runs as root, so it's hard to simulate any problem into creating a directory or file.

So while I don't like not covering this sort of things with a test, I don't quite see what we could do, and anyway it could likely fail in dozens of different ways, so coverage here will never be that good.

I'll remove the test, sorry for sending you on a goose chase.

[maniSHarma7575]

Thanks! @byroot. Goose chase helped me go dig deep into docker and linux.