Add validation to ensure that settings.configs values are dictionaries, in order to prevent misuse

CHANGELOG.md

@@ -2,6 +2,9 @@
 
 ## Next Version
 
+### Fixed
+- Added validation to ensure that all values in `settings.configs` are mappings. Previously, passing non-mapping values did not raise an error, making it difficult to detect misconfigurations. Now, `SpecParsingError.invalidConfigsMappingFormat` is thrown if misused. #1547 @Ryu0118
+
 ## 2.43.0
 
 ### Added

Sources/ProjectSpec/AggregateTarget.swift

@@ -60,7 +60,7 @@ extension AggregateTarget: NamedJSONDictionaryConvertible {
     public init(name: String, jsonDictionary: JSONDictionary) throws {
         self.name = jsonDictionary.json(atKeyPath: "name") ?? name
         targets = jsonDictionary.json(atKeyPath: "targets") ?? []
-        settings = jsonDictionary.json(atKeyPath: "settings") ?? .empty
+        settings = try BuildSettingsParser(jsonDictionary: jsonDictionary).parse()
         configFiles = jsonDictionary.json(atKeyPath: "configFiles") ?? [:]
         buildScripts = jsonDictionary.json(atKeyPath: "buildScripts") ?? []
         buildToolPlugins = jsonDictionary.json(atKeyPath: "buildToolPlugins") ?? []

Sources/ProjectSpec/BuildSettingsExtractor.swift

@@ -0,0 +1,37 @@
+import Foundation
+import JSONUtilities
+
+/// A helper for extracting and validating the `Settings` object from a JSON dictionary.
+struct BuildSettingsParser {
+    let jsonDictionary: JSONDictionary
+
+    /// Attempts to extract and parse the `Settings` from the dictionary.
+    ///
+    /// - Returns: A valid `Settings` object
+    func parse() throws -> Settings {
+        do {
+            return try jsonDictionary.json(atKeyPath: "settings")
+        } catch let specParsingError as SpecParsingError {
+            // Re-throw `SpecParsingError` to prevent the misuse of settings.configs.
+            throw specParsingError
+        } catch {
+            // Ignore all errors except `SpecParsingError`
+            return .empty
+        }
+    }
+
+    /// Attempts to extract and parse setting groups from the dictionary with fallback defaults.
+    ///
+    /// - Returns: Parsed setting groups or default groups if parsing fails
+    func parseSettingGroups() throws -> [String: Settings] {
+        do {
+            return try jsonDictionary.json(atKeyPath: "settingGroups", invalidItemBehaviour: .fail)
+        } catch let specParsingError as SpecParsingError {
+            // Re-throw `SpecParsingError` to prevent the misuse of settingGroups.
+            throw specParsingError
+        } catch {
+            // Ignore all errors except `SpecParsingError`
+            return jsonDictionary.json(atKeyPath: "settingPresets") ?? [:]
+        }
+    }
+}

Sources/ProjectSpec/Project.swift

@@ -171,11 +171,13 @@ extension Project {
         self.basePath = basePath
 
         let jsonDictionary = Project.resolveProject(jsonDictionary: jsonDictionary)
+        let buildSettingsParser = BuildSettingsParser(jsonDictionary: jsonDictionary)
 
         name = try jsonDictionary.json(atKeyPath: "name")
-        settings = jsonDictionary.json(atKeyPath: "settings") ?? .empty
-        settingGroups = jsonDictionary.json(atKeyPath: "settingGroups")
-            ?? jsonDictionary.json(atKeyPath: "settingPresets") ?? [:]
+
+        settings = try buildSettingsParser.parse()
+        settingGroups = try buildSettingsParser.parseSettingGroups()
+
         let configs: [String: String] = jsonDictionary.json(atKeyPath: "configs") ?? [:]
         self.configs = configs.isEmpty ? Config.defaultConfigs :
             configs.map { Config(name: $0, type: ConfigType(rawValue: $1)) }.sorted { $0.name < $1.name }

Sources/ProjectSpec/Settings.swift

@@ -28,14 +28,35 @@ public struct Settings: Equatable, JSONObjectConvertible, CustomStringConvertibl
             groups = jsonDictionary.json(atKeyPath: "groups") ?? jsonDictionary.json(atKeyPath: "presets") ?? []
             let buildSettingsDictionary: JSONDictionary = jsonDictionary.json(atKeyPath: "base") ?? [:]
             buildSettings = buildSettingsDictionary
-            configSettings = jsonDictionary.json(atKeyPath: "configs") ?? [:]
+
+            self.configSettings = try Self.extractValidConfigs(from: jsonDictionary)
         } else {
             buildSettings = jsonDictionary
             configSettings = [:]
             groups = []
         }
     }
 
+    /// Extracts and validates the `configs` mapping from the given JSON dictionary.
+    /// - Parameter jsonDictionary: The JSON dictionary to extract `configs` from.
+    /// - Returns: A dictionary mapping configuration names to `Settings` objects.
+    private static func extractValidConfigs(from jsonDictionary: JSONDictionary) throws -> [String: Settings] {
+        guard let configSettings = jsonDictionary["configs"] as? JSONDictionary else {
+            return [:]
+        }
+
+        let invalidConfigKeys = Set(
+            configSettings.filter { !($0.value is JSONDictionary) }
+                .map(\.key)
+        )
+
+        guard invalidConfigKeys.isEmpty else {
+            throw SpecParsingError.invalidConfigsMappingFormat(keys: invalidConfigKeys)
+        }
+
+        return try jsonDictionary.json(atKeyPath: "configs")
+    }
+
     public static func == (lhs: Settings, rhs: Settings) -> Bool {
         NSDictionary(dictionary: lhs.buildSettings).isEqual(to: rhs.buildSettings) &&
             lhs.configSettings == rhs.configSettings &&

Sources/ProjectSpec/SpecParsingError.swift

@@ -15,6 +15,7 @@ public enum SpecParsingError: Error, CustomStringConvertible {
     case unknownBreakpointActionType(String)
     case unknownBreakpointActionConveyanceType(String)
     case unknownBreakpointActionSoundName(String)
+    case invalidConfigsMappingFormat(keys: Set<String>)
 
     public var description: String {
         switch self {
@@ -46,6 +47,8 @@ public enum SpecParsingError: Error, CustomStringConvertible {
             return "Unknown Breakpoint Action conveyance type: \(type)"
         case let .unknownBreakpointActionSoundName(name):
             return "Unknown Breakpoint Action sound name: \(name)"
+        case let .invalidConfigsMappingFormat(keys):
+            return "Invalid format: The value for \"\(keys.sorted().joined(separator: ", "))\" in `configs` must be mapping format"
         }
     }
 }

Sources/ProjectSpec/Target.swift

@@ -316,7 +316,7 @@ extension Target: NamedJSONDictionaryConvertible {
             deploymentTarget = nil
         }
 
-        settings = jsonDictionary.json(atKeyPath: "settings") ?? .empty
+        settings = try BuildSettingsParser(jsonDictionary: jsonDictionary).parse()
         configFiles = jsonDictionary.json(atKeyPath: "configFiles") ?? [:]
         if let source: String = jsonDictionary.json(atKeyPath: "sources") {
             sources = [TargetSource(path: source)]

Tests/Fixtures/invalid_configs/invalid_configs_value_non_mapping_aggregate_targets.yml

@@ -0,0 +1,24 @@
+name: InvalidConfigsValueNonMappingAggregateTargets
+
+# This fixture tests validation of `settings.configs` under an aggregate target.
+# Here, `invalid_key0` and `invalid_key1` are scalar values (not mappings),
+# so parsing should throw SpecParsingError.invalidConfigsMappingFormat.
+targets:
+  valid_target1:
+    type: application
+    platform: iOS
+  valid_target2:
+    type: application
+    platform: iOS
+
+aggregateTargets:
+  invalid_target:
+    targets:
+      - valid_target1
+      - valid_target2
+    settings:
+      configs:
+        invalid_key0: value0
+        debug:
+          valid_key: value1
+        invalid_key1: value2

Tests/Fixtures/invalid_configs/invalid_configs_value_non_mapping_setting_groups.yml

@@ -0,0 +1,19 @@
+name: InvalidConfigsValueNonMappingSettingGroups
+
+# This fixture tests validation of `settings.configs` under an aggregate target.
+# Here, `invalid_key0` and `invalid_key1` are scalar values (not mappings),
+# so parsing should throw SpecParsingError.invalidConfigsMappingFormat.
+settingGroups:
+  invalid_preset:
+    configs:
+      invalid_key0: value0
+      debug:
+        valid_key: value1
+      invalid_key1: value2
+targets:
+  invalid_target:
+    type: application
+    platform: iOS
+    settings:
+      groups:
+        - invalid_preset

Tests/Fixtures/invalid_configs/invalid_configs_value_non_mapping_settings.yml

@@ -0,0 +1,11 @@
+name: InvalidConfigsValueNonMappingSettings
+
+# This fixture tests validation of `settings.configs` at the top level.
+# Here, `invalid_key0` and `invalid_key1` are scalar values (not mappings),
+# so parsing should throw SpecParsingError.invalidConfigsMappingFormat.
+settings:
+  configs:
+    invalid_key0: value0
+    debug:
+      valid_key: value1
+    invalid_key1: value2

Tests/Fixtures/invalid_configs/invalid_configs_value_non_mapping_targets.yml

@@ -0,0 +1,15 @@
+name: InvalidConfigsValueNonMappingTargets
+
+# This fixture tests nested validation of `settings.configs` under a target.
+# Here, `invalid_key0` and `invalid_key1` are scalar values (not mappings),
+# so parsing should throw SpecParsingError.invalidConfigsMappingFormat.
+targets:
+  invalid_target:
+    type: application
+    platform: iOS
+    settings:
+      configs:
+        invalid_key0: value0
+        debug:
+          valid_key: value1
+        invalid_key1: value2

Tests/ProjectSpecTests/InvalidConfigsFormatTests.swift

@@ -0,0 +1,43 @@
+import ProjectSpec
+import Testing
+import TestSupport
+import PathKit
+
+struct invalidConfigsMappingFormatTests {
+    struct InvalidConfigsTestArguments {
+        var fixturePath: Path
+        var expectedError: SpecParsingError
+    }
+
+    private static var testArguments: [InvalidConfigsTestArguments] {
+        let invalidConfigsFixturePath: Path = fixturePath + "invalid_configs"
+        return [
+            InvalidConfigsTestArguments(
+                fixturePath: invalidConfigsFixturePath + "invalid_configs_value_non_mapping_settings.yml",
+                expectedError: SpecParsingError.invalidConfigsMappingFormat(keys: ["invalid_key0", "invalid_key1"])
+            ),
+            InvalidConfigsTestArguments(
+                fixturePath: invalidConfigsFixturePath + "invalid_configs_value_non_mapping_targets.yml",
+                expectedError: SpecParsingError.invalidConfigsMappingFormat(keys: ["invalid_key0", "invalid_key1"])
+            ),
+            InvalidConfigsTestArguments(
+                fixturePath: invalidConfigsFixturePath + "invalid_configs_value_non_mapping_aggregate_targets.yml",
+                expectedError: SpecParsingError.invalidConfigsMappingFormat(keys: ["invalid_key0", "invalid_key1"])
+            ),
+            InvalidConfigsTestArguments(
+                fixturePath: invalidConfigsFixturePath + "invalid_configs_value_non_mapping_setting_groups.yml",
+                expectedError: SpecParsingError.invalidConfigsMappingFormat(keys: ["invalid_key0", "invalid_key1"])
+            )
+        ]
+    }
+
+    @Test("throws invalidConfigsMappingFormat for non-mapping configs entries", arguments: testArguments)
+    func testInvalidConfigsMappingFormat(_ arguments: InvalidConfigsTestArguments) throws {
+        #expect {
+            try Project(path: arguments.fixturePath)
+        } throws: { actualError in
+            (actualError as any CustomStringConvertible).description
+            == arguments.expectedError.description
+        }
+    }
+}