From daf330a159f2d8db9cca6f6a2c376cabbf65122f Mon Sep 17 00:00:00 2001 From: Injoong Yoon <27440940+injoongy@users.noreply.github.com> Date: Thu, 12 Feb 2026 16:48:32 -0800 Subject: [PATCH] fix: stop Tigris misconfig in rails scanner Stop inferring object storage from Dockerfile sqlite3 package presence, which caused false positives in GitHub launch flows. Refine Active Storage migration detection to skip object storage when production.rb explicitly uses local/test storage, and make storage.yml S3 detection robust across YAML map decode shapes while still ignoring commented entries. --- scanner/rails.go | 35 +++- .../rails_object_storage_detection_test.go | 166 ++++++++++++++++++ 2 files changed, 194 insertions(+), 7 deletions(-) create mode 100644 scanner/rails_object_storage_detection_test.go diff --git a/scanner/rails.go b/scanner/rails.go index c3c19e0030..8030ac840c 100644 --- a/scanner/rails.go +++ b/scanner/rails.go @@ -136,10 +136,6 @@ func configureRails(sourceDir string, config *ScannerConfig) (*SourceInfo, error // sqlite (detected from existing Dockerfile) s.DatabaseDesired = DatabaseKindSqlite s.SkipDatabase = true - // Only request object storage if not skipping extensions (for litestream replication) - if os.Getenv("SKIP_EXTENSIONS") == "" { - s.ObjectStorageDesired = true - } } else if checksPass(sourceDir, dirContains("config/database.yml", "adapter.*sqlite")) { // sqlite (detected from database.yml) s.DatabaseDesired = DatabaseKindSqlite @@ -198,7 +194,11 @@ func configureRails(sourceDir string, config *ScannerConfig) (*SourceInfo, error if checksPass(sourceDir, dirContains("Gemfile", "aws-sdk-s3")) || checksPass(sourceDir, dirContains("Gemfile.lock", "aws-sdk-s3")) { s.ObjectStorageDesired = true } else if checksPass(sourceDir+"/db/migrate", dirContains("*.rb", ":active_storage_attachments")) { - s.ObjectStorageDesired = true + // Rails apps may keep old Active Storage migrations around even when + // runtime storage is explicitly local. Avoid a false positive in that case. + if !railsProductionActiveStorageIsLocal(sourceDir) { + s.ObjectStorageDesired = true + } } else if checksPass(sourceDir+"/config", fileExists("storage.yml")) { cfgMap := map[string]any{} buf, err := os.ReadFile(path.Join(sourceDir, "config", "storage.yml")) @@ -209,8 +209,13 @@ func configureRails(sourceDir string, config *ScannerConfig) (*SourceInfo, error if err == nil { for _, v := range cfgMap { - submap, ok := v.(map[interface{}]interface{}) - if ok { + switch submap := v.(type) { + case map[interface{}]interface{}: + service, ok := submap["service"].(string) + if ok && service == "S3" { + s.ObjectStorageDesired = true + } + case map[string]interface{}: service, ok := submap["service"].(string) if ok && service == "S3" { s.ObjectStorageDesired = true @@ -334,6 +339,22 @@ Once ready: run 'fly deploy' to deploy your Rails app. return s, nil } +func railsProductionActiveStorageIsLocal(sourceDir string) bool { + buf, err := os.ReadFile(path.Join(sourceDir, "config", "environments", "production.rb")) + if err != nil { + return false + } + + prodEnv := string(buf) + re := regexp.MustCompile(`(?m)config\.active_storage\.service\s*=\s*:(local|test)\b`) + if re.MatchString(prodEnv) { + return true + } + + re = regexp.MustCompile(`(?m)config\.active_storage\.service\s*=\s*["'](local|test)["']\b`) + return re.MatchString(prodEnv) +} + func RailsCallback(appName string, srcInfo *SourceInfo, plan *plan.LaunchPlan, flags []string) error { // Overall strategy: Install and use the dockerfile-rails gem to generate a Dockerfile. // diff --git a/scanner/rails_object_storage_detection_test.go b/scanner/rails_object_storage_detection_test.go new file mode 100644 index 0000000000..b6ed166449 --- /dev/null +++ b/scanner/rails_object_storage_detection_test.go @@ -0,0 +1,166 @@ +package scanner + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestRailsObjectStorageDetection(t *testing.T) { + t.Run("migration plus local production storage does not request object storage", func(t *testing.T) { + dir := t.TempDir() + writeRailsScannerFixture(t, dir, "config.active_storage.service = :local\n") + writeActiveStorageMigration(t, dir) + + originalDir, err := os.Getwd() + require.NoError(t, err) + defer os.Chdir(originalDir) + require.NoError(t, os.Chdir(dir)) + + si, err := configureRails(dir, &ScannerConfig{}) + require.NoError(t, err) + require.NotNil(t, si) + assert.False(t, si.ObjectStorageDesired) + }) + + t.Run("migration plus non-local production storage requests object storage", func(t *testing.T) { + dir := t.TempDir() + writeRailsScannerFixture(t, dir, "config.active_storage.service = :amazon\n") + writeActiveStorageMigration(t, dir) + + originalDir, err := os.Getwd() + require.NoError(t, err) + defer os.Chdir(originalDir) + require.NoError(t, os.Chdir(dir)) + + si, err := configureRails(dir, &ScannerConfig{}) + require.NoError(t, err) + require.NotNil(t, si) + assert.True(t, si.ObjectStorageDesired) + }) + + t.Run("sqlite package in Dockerfile alone does not request object storage", func(t *testing.T) { + dir := t.TempDir() + writeRailsScannerFixture(t, dir, "config.active_storage.service = :local\n") + + require.NoError(t, os.WriteFile( + filepath.Join(dir, "Dockerfile"), + []byte("FROM ruby:3.3-slim\nRUN apt-get update -qq && apt-get install --no-install-recommends -y sqlite3\nEXPOSE 80\n"), + 0644, + )) + + originalDir, err := os.Getwd() + require.NoError(t, err) + defer os.Chdir(originalDir) + require.NoError(t, os.Chdir(dir)) + + si, err := configureRails(dir, &ScannerConfig{}) + require.NoError(t, err) + require.NotNil(t, si) + assert.False(t, si.ObjectStorageDesired) + }) + + t.Run("commented S3 entries in storage.yml do not request object storage", func(t *testing.T) { + dir := t.TempDir() + writeRailsScannerFixture(t, dir, "config.active_storage.service = :local\n") + + require.NoError(t, os.WriteFile( + filepath.Join(dir, "config", "storage.yml"), + []byte("local:\n service: Disk\n\n# amazon:\n# service: S3\n# bucket: foo\n"), + 0644, + )) + + originalDir, err := os.Getwd() + require.NoError(t, err) + defer os.Chdir(originalDir) + require.NoError(t, os.Chdir(dir)) + + si, err := configureRails(dir, &ScannerConfig{}) + require.NoError(t, err) + require.NotNil(t, si) + assert.False(t, si.ObjectStorageDesired) + }) + + t.Run("active S3 service in storage.yml requests object storage", func(t *testing.T) { + dir := t.TempDir() + writeRailsScannerFixture(t, dir, "config.active_storage.service = :local\n") + + require.NoError(t, os.WriteFile( + filepath.Join(dir, "config", "storage.yml"), + []byte("amazon:\n service: S3\n bucket: foo\n"), + 0644, + )) + + originalDir, err := os.Getwd() + require.NoError(t, err) + defer os.Chdir(originalDir) + require.NoError(t, os.Chdir(dir)) + + si, err := configureRails(dir, &ScannerConfig{}) + require.NoError(t, err) + require.NotNil(t, si) + assert.True(t, si.ObjectStorageDesired) + }) +} + +func writeRailsScannerFixture(t *testing.T, dir string, prodStorageLine string) { + t.Helper() + + require.NoError(t, os.MkdirAll(filepath.Join(dir, "config", "environments"), 0755)) + require.NoError(t, os.MkdirAll(filepath.Join(dir, "db", "migrate"), 0755)) + + require.NoError(t, os.WriteFile( + filepath.Join(dir, "Gemfile"), + []byte("source 'https://rubygems.org'\ngem 'rails', '~> 8.0'\n"), + 0644, + )) + require.NoError(t, os.WriteFile( + filepath.Join(dir, "Gemfile.lock"), + []byte("GEM\n specs:\n rails (8.0.0)\n"), + 0644, + )) + require.NoError(t, os.WriteFile( + filepath.Join(dir, "config.ru"), + []byte("require_relative 'config/environment'\nrun Rails.application\n"), + 0644, + )) + + require.NoError(t, os.WriteFile( + filepath.Join(dir, "Dockerfile"), + []byte("FROM ruby:3.3-slim\nEXPOSE 80\n"), + 0644, + )) + + require.NoError(t, os.WriteFile( + filepath.Join(dir, "config", "storage.yml"), + []byte("local:\n service: Disk\n"), + 0644, + )) + + for _, env := range []string{"development", "test"} { + require.NoError(t, os.WriteFile( + filepath.Join(dir, "config", "environments", env+".rb"), + []byte("Rails.application.configure do\n config.active_storage.service = :local\nend\n"), + 0644, + )) + } + + require.NoError(t, os.WriteFile( + filepath.Join(dir, "config", "environments", "production.rb"), + []byte("Rails.application.configure do\n "+prodStorageLine+"end\n"), + 0644, + )) +} + +func writeActiveStorageMigration(t *testing.T, dir string) { + t.Helper() + + require.NoError(t, os.WriteFile( + filepath.Join(dir, "db", "migrate", "20250101000000_create_active_storage_tables.rb"), + []byte("class CreateActiveStorageTables < ActiveRecord::Migration[7.1]\n def change\n create_table :active_storage_attachments do |t|\n t.string :name\n end\n end\nend\n"), + 0644, + )) +}