Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 23 commits into
base: master
Choose a base branch
from

Conversation

Ryu0118
Copy link

@Ryu0118 Ryu0118 commented May 19, 2025

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.

@giginet
Copy link
Collaborator

giginet commented May 20, 2025

Could you also update CHANGELOG.md?

@Ryu0118 Ryu0118 force-pushed the validate_invalid_configs branch from c79e246 to 340b8e6 Compare May 20, 2025 04:21
@Ryu0118 Ryu0118 requested review from giginet and freddi-kit May 20, 2025 04:26
@freddi-kit freddi-kit requested a review from yonaskolb May 20, 2025 06:05
Copy link
Author

@Ryu0118 Ryu0118 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@Ryu0118 Ryu0118 requested review from giginet and freddi-kit May 20, 2025 09:20
@giginet giginet self-requested a review May 26, 2025 02:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants