Disallow parent traversal in Defaults List config group paths

[thatbagu]

Motivation

It has been 2 years since original issue #2878 related to this PR have been opened by me, a lot has changed in my life since then, and now it is time to put this issue and following issues #3202 to the conclusion.

Have you read the Contributing Guidelines on pull requests?

Yes

Test Plan

Tests have been added to cover and reproduce these new error messages appearing.

[thatbagu]

I haven't put the check directly here, because I thought doing it downstream would be better for future readability, since all other checks would be near as well

[omry]

Thanks for working on this.

I want this to reject .. in config group paths regardless of where the defaults entry comes from: config files, nested defaults, and command-line defaults-list appends.

One reason I originally decided not to support relative parent traversal here is command-line override clarity. Defaults List config group paths have a stable identity: they are relative to the containing config, or absolute when prefixed with /. If parent traversal is allowed, it becomes harder to reason about what the canonical override key should be, especially from the command line. Seeing the full absolute group path in the Defaults List makes the behavior much clearer.

So the intended rule is simpler:

• group/subgroup: option remains supported.
• /group/subgroup: option remains supported.
• Any .. path segment in a config group path should fail with a clear error.
• This should apply no matter whether the entry came from a config file or from a CLI append such as +group=option.

I think the current PR catches config-file entries but misses external appends, because overrides.append_group_defaults are added after the new validation point. Could you move/add the validation so appended defaults are covered too, and add a focused test for a CLI append containing ..?

[thatbagu]

Thank you for the explanation. I agree that stable, predictable group paths make override semantics clearer. I've simplified the validation to a single rule (reject any .. segment regardless of context) and added a test covering the CLI append path.

[omry]

Thanks, this is moving in the right direction.

I think there is still one important gap: the validation currently checks only the config group path (d.group), but .. can also appear on the selected option side.

For example, these are still accepted by the parser and flow into Defaults List processing:

+missing_group=../file1
+missing_group=[file1,../file1]
group1=../file1

The same gap exists in config files:

defaults:
  - group1: ../file1

and:

defaults:
  - group1:
      - file1
      - ../file1

The intended rule is that .. is not allowed as a path segment anywhere in Defaults List config group selections, regardless of whether it appears in the group path or in the selected option path, and regardless of whether it came from a config file or a command-line override.

Suggested additional tests:

• CLI append with value traversal: +missing_group=../file1
• CLI append with list value traversal: +missing_group=[file1,../file1]
• CLI override of an existing defaults entry: group1=../file1
• config-file default: group1: ../file1
• config-file options list containing ../file1

The distinction matters for the CLI tests:

• +missing_group=... should use a group that is not already in the defaults list, so it exercises the external append path.
• group1=... should use a config that already contains group1 in the defaults list, so it exercises the override path instead of failing earlier with “No match in the defaults list.”

[omry]

Thanks for working on this and for addressing the earlier feedback. I have taken over the PR, completed the remaining validation and test coverage, and rebased it onto current main. Your original commits and authorship are preserved. Thanks again for getting this started.