Fix CI

Author: trinitytakei

Diff for: .github/workflows/ci.yml

@@ -44,6 +44,11 @@ jobs:
       - name: Install packages
         run: sudo apt-get update && sudo apt-get install --no-install-recommends -y google-chrome-stable curl libjemalloc2 libsqlite3-0 libsqlite3-0
 
+      - name: Create required directories
+        run: |
+          sudo mkdir -p /var/lib/rails-new-io/{home,config,cache,data,gems/bin,gems/specifications,gems/gems,gems/extensions,workspaces}
+          sudo chmod -R 777 /var/lib/rails-new-io
+
       - name: Checkout code
         uses: actions/checkout@v4
 

Diff for: app/services/command_execution_service.rb

@@ -1,9 +1,11 @@
 require "open3"
 require "timeout"
 require "fileutils"
+require "pathname"
 
 class CommandExecutionService
   RAILS_GEN_ROOT = "/var/lib/rails-new-io/rails-env".freeze
+  WORKSPACES_ROOT = "/var/lib/rails-new-io/workspaces".freeze
   RUBY_VERSION = "3.4.1".freeze
   RAILS_VERSION = "8.0.1".freeze
   BUNDLER_VERSION = "2.6.3".freeze
@@ -54,6 +56,15 @@ class CommandExecutionService
   VALID_OPTIONS = /\A--?[a-z][\w-]*\z/  # Must start with letter after dash(es)
   MAX_TIMEOUT = 300 # 5 minutes
 
+  ALLOWED_PATHS = [
+    RAILS_GEN_ROOT,
+    WORKSPACES_ROOT,
+    "/var/lib/rails-new-io/home",
+    "/var/lib/rails-new-io/config",
+    "/var/lib/rails-new-io/cache",
+    "/var/lib/rails-new-io/data"
+  ].map { |path| Pathname.new(path).freeze }.freeze
+
   class InvalidCommandError < StandardError
     attr_reader :metadata
 
@@ -73,7 +84,9 @@ def initialize(generated_app, logger, command = nil)
 
   def execute
     validate_command!
+    validate_work_directory! if @work_dir  # Check existing work_dir if set
     setup_work_directory
+    validate_work_directory!  # Validate again after setup
 
     Timeout.timeout(MAX_TIMEOUT) do
       run_isolated_process
@@ -117,25 +130,41 @@ def validate_command!
     @logger.debug("Command validation successful", { command: @command })
   end
 
+  def validate_work_directory!
+    raise InvalidCommandError, "Work directory not set up" unless @work_dir
+    work_dir_path = Pathname.new(@work_dir)
+
+    # Allow test directories (those under /tmp or /var/folders) in test environment
+    return if Rails.env.test? && (work_dir_path.to_s.start_with?("/tmp/") || work_dir_path.to_s.start_with?("/var/folders/"))
+
+    # Ensure the work directory is under an allowed path
+    unless ALLOWED_PATHS.any? { |allowed| work_dir_path.to_s.start_with?(allowed.to_s) }
+      @logger.error("Invalid work directory path", { work_dir: @work_dir })
+      raise InvalidCommandError, "Invalid work directory path"
+    end
+
+    raise InvalidCommandError, "Work directory does not exist" unless Dir.exist?(@work_dir)
+  end
+
   def setup_work_directory
+    base_dir = Pathname.new(WORKSPACES_ROOT)
+
     @work_dir = if @command.start_with?("rails new")
-      base_dir = "/var/lib/rails-new-io/workspaces"
       FileUtils.mkdir_p(base_dir)
-      workspace_dir_name = "workspace-#{Time.current.to_i}-#{SecureRandom.hex(4)}"
 
-      File.join(base_dir, workspace_dir_name).tap do |dir|
-        FileUtils.mkdir_p(dir)
-        @generated_app.update(workspace_path: dir)
-        @logger.info("Created workspace directory", { workspace_path: @work_dir })
-      end
-    else
-      File.join(@generated_app.workspace_path, @generated_app.name).tap do |dir|
-        @logger.info("Using existing workspace directory", { workspace_path: dir })
-      end
-    end
+      timestamp = Time.current.to_i.to_s
+      random_hex = SecureRandom.hex(4)
+      workspace_dir_name = [ "workspace", timestamp, random_hex ].join("-")
 
-    if !Dir.exist?(@work_dir)
-      raise InvalidCommandError, "Work directory #{@work_dir} does not exist!"
+      dir = base_dir.join(workspace_dir_name)
+      FileUtils.mkdir_p(dir)
+      @generated_app.update(workspace_path: dir.to_s)
+      @logger.info("Created workspace directory", { workspace_path: dir.to_s })
+      dir.to_s
+    else
+      dir = Pathname.new(@generated_app.workspace_path).join(@generated_app.name)
+      @logger.info("Using existing workspace directory", { workspace_path: dir.to_s })
+      dir.to_s
     end
   end
 
@@ -161,16 +190,19 @@ def run_isolated_process
 
     rails_cmd = "#{RAILS_GEN_ROOT}/gems/bin/rails"
 
-    command = if @command.start_with?("rails new")
-      args = @command.split[2..-1].join(" ")
-      "#{rails_cmd} new #{args}"
+    command_args = if @command.start_with?("rails new")
+      [ "new", *@command.split[2..-1] ]
     else
       # For other rails commands (like app:template), don't include 'rails' in the args
-      "#{rails_cmd} #{@command.split[1..-1].join(' ')}"
+      @command.split[1..-1]
     end
 
+    # Validate work directory one final time before execution
+    validate_work_directory!
+    options = { unsetenv_others: true, chdir: @work_dir }
+
     Bundler.with_unbundled_env do
-      execute_command(env, command, buffer, error_buffer)
+      execute_command(env, [ rails_cmd, *command_args ], buffer, error_buffer, options)
     end
 
     @work_dir
@@ -201,8 +233,8 @@ def env_for_command
     base_env
   end
 
-  def execute_command(env, command, buffer, error_buffer)
-    Open3.popen3(env, command, chdir: @work_dir, unsetenv_others: true) do |stdin, stdout, stderr, wait_thr|
+  def execute_command(env, command_with_args, buffer, error_buffer, options)
+    Open3.popen3(env, *command_with_args, options) do |stdin, stdout, stderr, wait_thr|
       @pid = wait_thr&.pid
 
       stdout_thread = Thread.new do
@@ -230,7 +262,7 @@ def execute_command(env, command, buffer, error_buffer)
           status: exit_status,
           output: output,
           error_buffer: error_buffer.join("<br>"),
-          command: command,
+          command: command_with_args.join(" "),
           directory: @work_dir,
           env: env
         })

Diff for: test/services/command_execution_service_test.rb

@@ -299,7 +299,7 @@ def teardown
   private
 
   def mock_popen3(stdout, stderr, success: true, pid: 12345)
-    lambda do |env, command, **options, &block|
+    lambda do |env, *command_with_args, options, &block|
       mock_stdin = StringIO.new
       mock_stdout = StringIO.new(stdout)
       mock_stderr = StringIO.new(stderr)
@@ -414,8 +414,36 @@ def mock_popen3(stdout, stderr, success: true, pid: 12345)
           @service.execute
         end
 
-        assert_equal "Work directory /var/lib/rails-new-io/workspaces/workspace-1739965840-1a21373e does not exist!", error.message
+        assert_equal "Work directory does not exist", error.message
       end
     end
   end
+
+  test "raises error when work directory is outside allowed paths" do
+    # Set up a directory outside of allowed paths
+    malicious_path = "/etc/some/path"
+    @generated_app.update!(workspace_path: malicious_path)
+
+    # Override the test environment check to ensure the validation runs
+    Rails.env.stubs(:test?).returns(false)
+
+    # Use a template command since it uses the existing workspace path
+    service = CommandExecutionService.new(@generated_app, @logger, "rails app:template LOCATION=lib/templates/template.rb")
+
+    # Set the work_dir directly to trigger validation
+    service.instance_variable_set(:@work_dir, malicious_path)
+
+    assert_difference -> { AppGeneration::LogEntry.count }, 1 do
+      error = assert_raises(CommandExecutionService::InvalidCommandError) do
+        service.send(:validate_work_directory!)
+      end
+
+      assert_equal "Invalid work directory path", error.message
+    end
+
+    log_entries = @generated_app.log_entries.order(created_at: :asc).last(1)
+    assert log_entries[0].error?
+    assert_equal "Invalid work directory path", log_entries[0].message
+    assert_equal({ "work_dir" => malicious_path }, log_entries[0].metadata)
+  end
 end