Cap inherit_from recursion in DeploySpec::FileSystem

app/models/shipit/deploy_spec/file_system.rb

@@ -151,17 +151,26 @@ def app_name
       end
 
       SHIPIT_CONFIG_INHERIT_FROM_KEY = "inherit_from"
-      def build_config(path, config_obj)
+      MAX_INHERIT_FROM_DEPTH = 10
+
+      def build_config(path, config_obj, depth = 0)
         return config_obj if config_obj.blank? || !config_obj.key?(SHIPIT_CONFIG_INHERIT_FROM_KEY)
 
+        # An inherit_from cycle (a.yml <-> b.yml) would otherwise recurse until
+        # SystemStackError. Surface a clear configuration error instead.
+        if depth >= MAX_INHERIT_FROM_DEPTH
+          raise Error, "inherit_from chain exceeds #{MAX_INHERIT_FROM_DEPTH} levels " \
+                       "(cycle?) while resolving #{path}"
+        end
+
         inherits_from_path = path.dirname.join(config_obj.delete(SHIPIT_CONFIG_INHERIT_FROM_KEY))
         if inherits_from_path.exist?
           inherits_config_obj = read_config(inherits_from_path)
           config_obj = inherits_config_obj.deep_merge(config_obj)
           path = inherits_from_path
         end
 
-        build_config(path, config_obj)
+        build_config(path, config_obj, depth + 1)
       end
 
       def read_config(path)

app/models/shipit/deploy_spec/git_object_file_system.rb

@@ -79,7 +79,7 @@ def file(path, root: false)
       # file. The old checkout-based path is uncapped (a cycle loops forever)
       # and follows escaping paths onto the worker filesystem; both are hard
       # fallbacks here.
-      def build_config(path, config_obj)
+      def build_config(path, config_obj, depth = 0)
         if config_obj.present? && config_obj.key?(SHIPIT_CONFIG_INHERIT_FROM_KEY)
           @inherit_reads += 1
           raise FallbackRequired.new(:inherit_depth, @inherit_chain.join(' -> ')) if @inherit_reads > MAX_INHERIT_READS

test/models/deploy_spec_test.rb

@@ -11,6 +11,31 @@ class DeploySpecTest < ActiveSupport::TestCase
       @spec.stubs(:load_config).returns({})
     end
 
+    test 'an inherit_from cycle raises a clear error instead of recursing forever' do
+      Dir.mktmpdir do |dir|
+        File.write(File.join(dir, 'shipit.production.yml'), "inherit_from: a.yml\n")
+        File.write(File.join(dir, 'a.yml'), "inherit_from: b.yml\n")
+        File.write(File.join(dir, 'b.yml'), "inherit_from: a.yml\n")
+
+        spec = DeploySpec::FileSystem.new(dir, @stack)
+        error = assert_raises(DeploySpec::Error) { spec.cacheable }
+        assert_match(/inherit_from chain exceeds/, error.message)
+      end
+    end
+
+    test 'an inherit_from chain under the depth cap resolves normally' do
+      Dir.mktmpdir do |dir|
+        File.write(File.join(dir, 'shipit.production.yml'), "inherit_from: a.yml\ndeploy:\n  override:\n    - echo child\n")
+        File.write(File.join(dir, 'a.yml'), "inherit_from: b.yml\n")
+        File.write(File.join(dir, 'b.yml'), "machine:\n  environment:\n    FOO: bar\n")
+
+        spec = DeploySpec::FileSystem.new(dir, @stack)
+        config = spec.cacheable.config
+        assert_equal ['echo child'], config.dig('deploy', 'override')
+        assert_equal 'bar', config.dig('machine', 'environment', 'FOO')
+      end
+    end
+
     test '#supports_fetch_deployed_revision? returns false by default' do
       refute @spec.supports_fetch_deployed_revision?
     end