Add validation to ensure that settings.configs values are dictionaries, in order to prevent misuse by Ryu0118 · Pull Request #1547 · yonaskolb/XcodeGen

Ryu0118 · 2025-05-19T10:27:28Z

Motivation

In our team, we previously encountered cases where settings.configs was misused by passing non-dictionary values.
However, because no error was raised, the issue went unnoticed.

Proposed Solution

This PR adds validation to ensure that all values in settings.configs are dictionaries.
For example, the following invalid configuration will now throw a SpecParsingError.invalidConfigsFormat during parsing:

settings:
  configs:
    SOME_FLAG: true

If any non-dictionary values are found, a SpecParsingError.invalidConfigsFormat is thrown during parsing.

By introducing this validation, such misconfigurations can now be detected early.

…prevent misuse

Sources/ProjectSpec/Settings.swift

Tests/Fixtures/invalid_configs_value_non_dict.yml

Tests/ProjectSpecTests/InvalidConfigsFormatTests.swift

Sources/ProjectSpec/Project.swift

Sources/ProjectSpec/SpecParsingError.swift

Sources/ProjectSpec/Settings.swift

giginet · 2025-05-20T02:06:11Z

Could you also update CHANGELOG.md?

Tests/ProjectSpecTests/InvalidConfigsFormatTests.swift

Sources/ProjectSpec/Settings.swift

Tests/ProjectSpecTests/InvalidConfigsFormatTests.swift

Ryu0118

Currently, the following YAML is correctly detected as invalid:

settings:
  configs:
    invalid_key: value

However, the following YAML is not detected as invalid:

targets:
  invalid_target:
    settings:
      configs:
        invalid_key: value

targets:
  valid_target1:
    type: application
    platform: iOS
  valid_target2:
    type: application
    platform: iOS

aggregateTargets:
  invalid_target:
    targets:
      - valid_target1
      - valid_target2
    settings:
      configs:
        invalid_key: value

This needs to be fixed.

…dictionary to Settings

Sources/ProjectSpec/ValidSettingsExtractor.swift

Tests/ProjectSpecTests/InvalidConfigsFormatTests.swift

freddi-kit · 2025-06-03T08:36:15Z

Tests/ProjectSpecTests/InvalidConfigsFormatTests.swift

+import TestSupport
+import PathKit
+
+struct invalidConfigsMappingFormatTests {


Suggested change

struct invalidConfigsMappingFormatTests {

@Suite

struct invalidConfigsMappingFormatTests {

Do we need it? Or do you remove it for other reason?

even if we don't apply the @Suite, swift-testing recognizes this as test suites.

The use of the @suite attribute is optional; types are recognized as test suites even if they do not have the @suite attribute applied to them.

ref: https://developer.apple.com/documentation/testing/suite(_:_:)

yonaskolb

Thank you @Ryu0118

Tests/ProjectSpecTests/InvalidConfigsFormatTests.swift

yonaskolb · 2025-06-06T14:20:00Z

And thank you for reviewing @freddi-kit and @giginet 🙏

* Custom error handler * Check for error reason and expected type * Improvements * Update CHANGELOG.md * Update master (#1) * Update CI equipments and drop Xcode 15 support (#1548) * Add validation to ensure that settings.configs values are dictionaries, in order to prevent misuse (#1547) * Add validation to ensure settings.configs values are dictionaries to prevent misuse * Add tests for invalid settings.configs value formats * Replaced with filter and split into a function * Rename invalidConfigsFormat to invalidConfigsMappingFormat * Add comments to explain invalid fixture * Rename test fixture * Update CHANGELOG.md * Correct grammer * Use KeyPath instead of closure * Rename validateMappingStyleInConfig to extractValidConfigs * Add a document comment for extractValidConfigs(from:) * Use old testing api and remove EquatableErrorBox * Rename test case to use "mapping" instead of "dictionary" * Add ValidSettingsExtractor to encapsulate the logic for converting a dictionary to Settings * Add settings validation for both Target and AggregateTarget * Add tests for invalid settings.configs in Target and AggregateTarget * Add document comments for ValidSettingsExtractor * Rename ValidSettingsExtractor to BuildSettingsExtractor * Add settings validation for settingGroups * Add tests for settingGroups * Rename extract to parse * Refactor * Update Tests/ProjectSpecTests/InvalidConfigsFormatTests.swift --------- Co-authored-by: Yonas Kolb <[email protected]> * Synced folders (#1541) * update xcodeproj to 8.27.7 * add syncedFolder source type * drop xcode 15 support * Rely on fileReference instead of adding new synchronizedRootGroup (#1557) * fix: don't include untracked children in cache --------- Co-authored-by: Kirill Yakimovich <[email protected]> * Use USER env var instead of LOGNAME (#1559) During user switch with su/sudo in system LOGNAME may not be initialised, but USER env var is always exist. * Address Sanitizer options in run/test schemes (#1550) * Expose address sanitizer flags in run and test BuildActions in Schemes * Update testJSONEncodable to test the new fields * Also test the asan setting values for run scheme * Update changelog --------- Co-authored-by: Yonas Kolb <[email protected]> * Update to 2.44.0 # Conflicts: # CHANGELOG.md --------- Co-authored-by: Kohki Miki <[email protected]> Co-authored-by: Ryu <[email protected]> Co-authored-by: Yonas Kolb <[email protected]> Co-authored-by: Kirill Yakimovich <[email protected]> Co-authored-by: Kanstantsin Shautsou <[email protected]> Co-authored-by: Himanshu Kumar <[email protected]> * Revert * Refactoring started * func json(atKeyPath keyPath: String) -> String? * All cases --------- Co-authored-by: Kohki Miki <[email protected]> Co-authored-by: Ryu <[email protected]> Co-authored-by: Yonas Kolb <[email protected]> Co-authored-by: Kirill Yakimovich <[email protected]> Co-authored-by: Kanstantsin Shautsou <[email protected]> Co-authored-by: Himanshu Kumar <[email protected]>

Ryu0118 added 2 commits May 19, 2025 19:00

Add validation to ensure settings.configs values are dictionaries to …

0cdd80d

…prevent misuse

Add tests for invalid settings.configs value formats

58cdf9b

giginet reviewed May 20, 2025

View reviewed changes

freddi-kit reviewed May 20, 2025

View reviewed changes

Sources/ProjectSpec/Project.swift Outdated Show resolved Hide resolved

Sources/ProjectSpec/Project.swift Outdated Show resolved Hide resolved

Sources/ProjectSpec/Project.swift Outdated Show resolved Hide resolved

Sources/ProjectSpec/SpecParsingError.swift Outdated Show resolved Hide resolved

freddi-kit reviewed May 20, 2025

View reviewed changes

Sources/ProjectSpec/SpecParsingError.swift Outdated Show resolved Hide resolved

freddi-kit reviewed May 20, 2025

View reviewed changes

Sources/ProjectSpec/Settings.swift Outdated Show resolved Hide resolved

giginet mentioned this pull request May 20, 2025

Update CI equipments and drop Xcode 15 support #1548

Merged

Ryu0118 added 6 commits May 20, 2025 11:47

Merge branch 'master' into validate_invalid_configs

6af2709

Replaced with filter and split into a function

5c164b2

Rename invalidConfigsFormat to invalidConfigsMappingFormat

d90866f

Add comments to explain invalid fixture

eac4831

Rename test fixture

8d90c4a

Update CHANGELOG.md

340b8e6

Ryu0118 force-pushed the validate_invalid_configs branch from c79e246 to 340b8e6 Compare May 20, 2025 04:21

Correct grammer

1b32082

Ryu0118 requested review from freddi-kit and giginet May 20, 2025 04:26

giginet reviewed May 20, 2025

View reviewed changes

Ryu0118 added 4 commits May 20, 2025 14:16

Use KeyPath instead of closure

70bf933

Rename validateMappingStyleInConfig to extractValidConfigs

d9ca225

Add a document comment for extractValidConfigs(from:)

53968a5

Use old testing api and remove EquatableErrorBox

c56b072

Ryu0118 commented May 20, 2025

View reviewed changes

Tests/ProjectSpecTests/InvalidConfigsFormatTests.swift Outdated Show resolved Hide resolved

Rename test case to use "mapping" instead of "dictionary"

d65fc9e

giginet approved these changes May 20, 2025

View reviewed changes

freddi-kit approved these changes May 20, 2025

View reviewed changes

freddi-kit requested a review from yonaskolb May 20, 2025 06:05

Ryu0118 commented May 20, 2025

View reviewed changes

Add ValidSettingsExtractor to encapsulate the logic for converting a …

cb2f605

…dictionary to Settings

Ryu0118 added 2 commits May 20, 2025 17:57

Add settings validation for both Target and AggregateTarget

54081ce

Add tests for invalid settings.configs in Target and AggregateTarget

24040e7

Ryu0118 requested review from freddi-kit and giginet May 20, 2025 09:20

Add document comments for ValidSettingsExtractor

9dc8405

giginet reviewed May 26, 2025

View reviewed changes

Sources/ProjectSpec/ValidSettingsExtractor.swift Outdated Show resolved Hide resolved

giginet reviewed May 26, 2025

View reviewed changes

Tests/ProjectSpecTests/InvalidConfigsFormatTests.swift Outdated Show resolved Hide resolved

Tests/ProjectSpecTests/InvalidConfigsFormatTests.swift Outdated Show resolved Hide resolved

giginet self-requested a review May 26, 2025 02:12

Ryu0118 added 5 commits May 26, 2025 11:24

Rename ValidSettingsExtractor to BuildSettingsExtractor

ca9ed3e

Add settings validation for settingGroups

6b9cc59

Add tests for settingGroups

6f7592d

Rename extract to parse

79d78ca

Refactor

03bcec1

giginet approved these changes May 26, 2025

View reviewed changes

freddi-kit approved these changes Jun 3, 2025

View reviewed changes

yonaskolb approved these changes Jun 6, 2025

View reviewed changes

Tests/ProjectSpecTests/InvalidConfigsFormatTests.swift Outdated Show resolved Hide resolved

Update Tests/ProjectSpecTests/InvalidConfigsFormatTests.swift

a792bcc

yonaskolb merged commit 53cb43c into yonaskolb:master Jun 6, 2025
3 checks passed

NicolasBuquet mentioned this pull request Jan 21, 2026

[Build] update target files for xcodegen 2.44 tchapgouv/tchap-ios#1249

Closed

	struct invalidConfigsMappingFormatTests {
	@Suite
	struct invalidConfigsMappingFormatTests {

Comments

Conversation

Ryu0118 commented May 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

Proposed Solution

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

giginet commented May 20, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Ryu0118 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

freddi-kit Jun 3, 2025

Choose a reason for hiding this comment

Uh oh!

Ryu0118 Jun 6, 2025

Choose a reason for hiding this comment

Uh oh!

yonaskolb left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

yonaskolb commented Jun 6, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Ryu0118 commented May 19, 2025 •

edited

Loading

Ryu0118 left a comment •

edited

Loading