Skip to content

Commit f619c8f

Browse files
committed
config(fix): Validate supplied serialized values
why: Editor schema and runtime validation need one acceptance boundary. what: Validate overridden legacy values and both URL aliases. Normalize integral config numbers before strict backend construction. Reject non-JSON metadata before recursive copying and comparisons. Require structured filter combinations and convert native syntax during migration. Preserve import provenance repair before validating rewritten metadata.
1 parent 372b902 commit f619c8f

7 files changed

Lines changed: 424 additions & 56 deletions

File tree

‎CHANGES‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,8 @@ _Notes on upcoming releases will be added here_
4545
`git`, `hg`, and `svn` blocks match their libvcs options classes. Loading
4646
rejects unknown fields, invalid values, and blocks for a different VCS before
4747
sync starts. Errors identify the file, workspace, repository, and field.
48+
Overridden legacy values are validated too. Integer fields accept integral
49+
numbers such as `2.0`; metadata accepts JSON-compatible values.
4850

4951
```yaml
5052
~/code/:

‎MIGRATION‎

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -62,8 +62,12 @@ After:
6262
`shallow: true` becomes `git.depth: 1`. `git_options`, `hg_options`, and
6363
`svn_options` become `git`, `hg`, and `svn`. `pin_reason` and
6464
`allow_overwrite` move from `options` to the entry level alongside `pin`.
65-
Existing canonical values take precedence. Unknown legacy `options` keys
66-
produce an error before the file is written.
65+
Existing canonical values take precedence. Every supplied setting is
66+
validated before precedence applies, including overridden values. Unknown
67+
legacy `options` keys produce an error before the file is written. Remove
68+
invalid values instead of relying on another field to hide them. Both `url`
69+
and `repo` must be valid strings when present; `url` still takes precedence.
70+
Metadata must contain JSON-compatible values with string keys.
6771

6872
`add` and `discover` write the canonical layout. `import` and `fmt` migrate
6973
entries they rewrite; entries pinned for that command stay unchanged. Use
@@ -75,9 +79,12 @@ belongs in `metadata`, not in a libvcs constructor argument. Sync constructs
7579
the matching immutable libvcs options object from `git`, `hg`, or `svn`.
7680

7781
To omit historical file contents without truncating commit history, add
78-
`git.filter: blob:none`. The value can also be a native filter string, a
79-
kind-tagged mapping, or a nonempty list of filters. Filter and depth changes
80-
only affect a new clone; they do not reconfigure an existing checkout.
82+
`git.filter: blob:none`. The value can also be an atomic native filter
83+
string, a kind-tagged mapping, or a nonempty list of filters. Express combined
84+
filters as `{kind: combine, filters: [blob:none, "tree:3"]}` or nested lists;
85+
`vcspull migrate --write` converts native `combine:` strings to that form.
86+
Filter and depth changes only affect a new clone; they do not reconfigure an
87+
existing checkout.
8188

8289
## vcspull 1.62.0 (2026-05-31)
8390

‎docs/configuration/index.md‎

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,9 @@ See {ref}`cli-worktree` for full command documentation.
138138
Repository entries use one backend block matching their URL: `git`, `hg`,
139139
or `svn`. Unknown keys, wrong-backend blocks, and malformed values fail when
140140
loading, with the file, workspace, repository, and field in the error.
141+
Supplied legacy values must be valid even when a canonical field overrides
142+
them. Numeric integer fields accept `2.0` as `2`; booleans and fractional
143+
values fail. Metadata contains JSON-compatible values with string keys.
141144

142145
| Block | Options |
143146
| --- | --- |
@@ -171,7 +174,20 @@ needed by the current working tree.
171174
depth: 3
172175
```
173176

174-
Filters accept native strings, kind-tagged mappings, or nonempty lists.
177+
Filters accept atomic native strings, kind-tagged mappings, or nonempty lists.
178+
Use a mapping or nested list for a combined filter, without percent encoding:
179+
180+
```yaml
181+
git:
182+
filter:
183+
kind: combine
184+
filters:
185+
- blob:none
186+
- tree:3
187+
```
188+
189+
Native `combine:` strings are rejected in configuration; `vcspull migrate`
190+
converts them to this structured form. Combinations may nest up to 32 levels.
175191
`auto` requires a Git version that supports it and cannot be combined with
176192
other filters or forwarded to submodule initialization. libvcs validates
177193
filter syntax before running Git. The remote must enable partial-clone

‎src/vcspull/cli/import_cmd/_common.py‎

Lines changed: 15 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1061,6 +1061,15 @@ def _run_import(
10611061
if isinstance(existing_raw, dict)
10621062
else {}
10631063
)
1064+
updated["repo"] = incoming_url
1065+
updated.pop("url", None)
1066+
if import_source:
1067+
# None is not safe: setdefault returns existing None
1068+
# instead of replacing it, causing TypeError.
1069+
if not isinstance(updated.get("metadata"), dict):
1070+
updated["metadata"] = {}
1071+
metadata = updated.setdefault("metadata", {})
1072+
metadata["imported_from"] = import_source
10641073
try:
10651074
_, updated = migrate_repo_entry(updated)
10661075
except (TypeError, ValueError) as error:
@@ -1072,15 +1081,6 @@ def _run_import(
10721081
error,
10731082
)
10741083
return 1
1075-
updated["repo"] = incoming_url
1076-
updated.pop("url", None)
1077-
if import_source:
1078-
# None is not safe: setdefault returns existing None
1079-
# instead of replacing it, causing TypeError.
1080-
if not isinstance(updated.get("metadata"), dict):
1081-
updated["metadata"] = {}
1082-
metadata = updated.setdefault("metadata", {})
1083-
metadata["imported_from"] = import_source
10841084
raw_config[repo_workspace_label][repo.name] = updated
10851085
else:
10861086
log.info("[DRY-RUN] Would update URL: %s", repo.name)
@@ -1097,6 +1097,12 @@ def _run_import(
10971097
)
10981098
if needs_tag:
10991099
if not dry_run:
1100+
live = copy.deepcopy(live)
1101+
if not isinstance(existing_meta, dict):
1102+
live["metadata"] = {}
1103+
live.setdefault("metadata", {})["imported_from"] = (
1104+
import_source
1105+
)
11001106
try:
11011107
_, live = migrate_repo_entry(live)
11021108
except (TypeError, ValueError) as error:
@@ -1109,11 +1115,6 @@ def _run_import(
11091115
)
11101116
return 1
11111117
raw_config[repo_workspace_label][repo.name] = live
1112-
if not isinstance(existing_meta, dict):
1113-
live["metadata"] = {}
1114-
live.setdefault("metadata", {})["imported_from"] = (
1115-
import_source
1116-
)
11171118
provenance_tagged_count += 1
11181119
elif isinstance(live, str):
11191120
if not dry_run:

‎src/vcspull/config.py‎

Lines changed: 75 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,13 @@
1414
import typing as t
1515
from collections.abc import Callable
1616

17+
from libvcs.cmd.git_filter import Combine, parse_filter
1718
from libvcs.sync.git import GitRemote
1819

1920
from vcspull.validator import (
2021
is_valid_config,
22+
validate_legacy_options,
23+
validate_metadata,
2124
validate_repo_entry,
2225
validate_working_copy,
2326
)
@@ -29,7 +32,7 @@
2932
config_format_from_path,
3033
)
3134
from .types import ConfigDict, RawConfigDict, WorktreeConfigDict
32-
from .util import get_config_dir, update_dict
35+
from .util import get_config_dir
3336

3437
log = logging.getLogger(__name__)
3538

@@ -269,22 +272,22 @@ def extract_repos(
269272
if isinstance(repo_data, str):
270273
conf["url"] = repo_data
271274
else:
272-
conf = update_dict(conf, repo_data)
273-
274-
if "repo" in conf:
275-
if "url" not in conf:
276-
conf["url"] = conf.pop("repo")
277-
else:
278-
conf.pop("repo", None)
275+
conf = copy.deepcopy(repo_data)
279276

280277
location = f"{directory!r} -> {repo!r}"
281278
try:
282-
_, conf = migrate_repo_entry(conf)
279+
_, conf = migrate_repo_entry(conf, normalize_filters=False)
283280
except (TypeError, ValueError) as error:
284281
msg = f"{location}.{error}"
285282
raise exc.VCSPullException(msg) from error
286283
validate_repo_entry(conf, location=location)
287284

285+
if "repo" in conf:
286+
if "url" not in conf:
287+
conf["url"] = conf.pop("repo")
288+
else:
289+
conf.pop("repo", None)
290+
288291
if "name" not in conf:
289292
conf["name"] = repo
290293

@@ -969,9 +972,8 @@ def resolve_clone_depth(
969972
970973
1. ``explicit_depth`` (from ``--depth N``) → ``(False, explicit_depth)``.
971974
2. ``explicit_shallow`` (from ``--shallow``) → ``(True, None)``.
972-
3. Auto-detected depth (hybrid): a depth-1 checkout records ``shallow:
973-
true`` (the common case), depth > 1 records ``depth: N``, and a full
974-
checkout records neither.
975+
3. Auto-detected depth: shallow checkouts record ``git.depth: N``;
976+
full checkouts omit the depth setting.
975977
976978
Parameters
977979
----------
@@ -1079,7 +1081,47 @@ def build_repo_entry(
10791081
LEGACY_REPO_OPTION_KEYS = ("rev", "shallow", "depth")
10801082

10811083

1082-
def migrate_repo_entry(entry: t.Any) -> tuple[bool, t.Any]:
1084+
def _migrate_filter_config(value: t.Any, *, depth: int = 0) -> t.Any:
1085+
"""Express native combinations without nested percent encoding.
1086+
1087+
>>> _migrate_filter_config("combine:blob:none+tree:1")
1088+
{'kind': 'combine', 'filters': ['blob:none', 'tree:1']}
1089+
"""
1090+
if depth > 33:
1091+
msg = "filter nesting exceeds 32 levels"
1092+
raise ValueError(msg)
1093+
if isinstance(value, str) and value.startswith("combine:"):
1094+
value = parse_filter(value)
1095+
if isinstance(value, Combine):
1096+
return {
1097+
"kind": "combine",
1098+
"filters": [
1099+
_migrate_filter_config(child, depth=depth + 1)
1100+
if isinstance(child, Combine)
1101+
else child.to_spec()
1102+
for child in value.filters
1103+
],
1104+
}
1105+
if isinstance(value, list):
1106+
return [_migrate_filter_config(child, depth=depth + 1) for child in value]
1107+
if (
1108+
isinstance(value, dict)
1109+
and value.get("kind") == "combine"
1110+
and isinstance(value.get("filters"), list)
1111+
):
1112+
return {
1113+
**value,
1114+
"filters": [
1115+
_migrate_filter_config(child, depth=depth + 1)
1116+
for child in value["filters"]
1117+
],
1118+
}
1119+
return value
1120+
1121+
1122+
def migrate_repo_entry(
1123+
entry: t.Any, *, normalize_filters: bool = True
1124+
) -> tuple[bool, t.Any]:
10831125
"""Separate checkout targets, backend settings, and entry policy.
10841126
10851127
Canonical values win over legacy values. Within the legacy layout,
@@ -1090,6 +1132,9 @@ def migrate_repo_entry(entry: t.Any) -> tuple[bool, t.Any]:
10901132
----------
10911133
entry : Any
10921134
A raw repository entry (string shorthand or mapping).
1135+
normalize_filters : bool
1136+
Rewrite native combined filters for serialization. Loading disables
1137+
this conversion so unsupported syntax fails at its source.
10931138
10941139
Returns
10951140
-------
@@ -1121,17 +1166,31 @@ def migrate_repo_entry(entry: t.Any) -> tuple[bool, t.Any]:
11211166
if not isinstance(entry, dict):
11221167
return False, entry
11231168

1169+
if "metadata" in entry:
1170+
try:
1171+
validate_metadata(entry["metadata"])
1172+
except exc.VCSPullException as error:
1173+
raise ValueError(str(error)) from error
1174+
11241175
legacy_keys = (
11251176
*LEGACY_REPO_OPTION_KEYS,
11261177
"options",
11271178
"git_options",
11281179
"hg_options",
11291180
"svn_options",
11301181
)
1131-
if not any(key in entry for key in legacy_keys):
1132-
return False, entry
1133-
11341182
new_entry = copy.deepcopy(entry)
1183+
if normalize_filters:
1184+
for key in ("git", "git_options"):
1185+
options = new_entry.get(key)
1186+
if isinstance(options, dict) and "filter" in options:
1187+
options["filter"] = _migrate_filter_config(options["filter"])
1188+
if not any(key in new_entry for key in legacy_keys):
1189+
return new_entry != entry, new_entry
1190+
try:
1191+
validate_legacy_options(new_entry)
1192+
except exc.VCSPullException as error:
1193+
raise ValueError(str(error)) from error
11351194
options = new_entry.pop("options", {})
11361195
if not isinstance(options, dict):
11371196
msg = "options: expected a mapping"

0 commit comments

Comments
 (0)