diff --git a/CHANGELOG.md b/CHANGELOG.md index c522dfc6..f3965a71 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -266,6 +266,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 predicate paths, AWS and GCP resource names, Snowflake account segments, Databricks git refs, and the identifier and version checks of ODCS and ODPS imports. +- **Schema validation let a value with a trailing newline through a `pattern`.** JSON Schema + writes `pattern` in ECMA-262, where `$` matches only at the end of the value, but the + jsonschema library runs it with Python's `re.search`, whose `$` also matches before a final + newline. So `fluid validate` accepted `id: "orders\n"`, which an `id: |` block scalar produces, + under every bundled schema version (0.7.1 to 0.7.7). Contract, fragment and bundle validation + now read `$` as the end of the value. Such a value is refused at its path, with a hint when + the line break is the only problem: `(it ends in a line break, which a YAML | or > block keeps + and |- or >- drops)`. A `sink.catalogAuth` field gets no hint, since its value is never shown. + The schemas themselves are unchanged. + ## [0.26.0] — 2026-11-01 ### Added diff --git a/fluid_build/forge/core/validators.py b/fluid_build/forge/core/validators.py index 83472ccf..2bab29bc 100644 --- a/fluid_build/forge/core/validators.py +++ b/fluid_build/forge/core/validators.py @@ -390,9 +390,14 @@ def validate_json_schema( ) ] schema = schema_manager.get_schema(version) - try: - import jsonschema # type: ignore - except ImportError: # pragma: no cover — jsonschema is a hard dep upstream + from fluid_build.schema_manager import ( + JSONSCHEMA_AVAILABLE, + format_schema_error, + schema_validator, + ) + + # The flag schema_validator needs: a package that is found can still fail to import. + if not JSONSCHEMA_AVAILABLE: return [ ValidationIssue( file=path, @@ -405,9 +410,7 @@ def validate_json_schema( # Honour the schema's declared $schema; Draft 7 silently ignores # newer keywords, which drops the constraints they express. - from fluid_build.schema_manager import format_schema_error - - validator = jsonschema.validators.validator_for(schema)(schema) + validator = schema_validator(schema) errors = sorted(validator.iter_errors(doc), key=lambda e: list(e.absolute_path)) for err in errors: loc = "/".join(str(p) for p in err.absolute_path) or "" diff --git a/fluid_build/schema_manager.py b/fluid_build/schema_manager.py index a757b877..1b992d7e 100644 --- a/fluid_build/schema_manager.py +++ b/fluid_build/schema_manager.py @@ -21,13 +21,14 @@ from __future__ import annotations +import functools import json import logging import re from dataclasses import dataclass, field from datetime import datetime, timedelta from pathlib import Path -from typing import Any, Callable, Dict, FrozenSet, List, Optional, Union +from typing import Any, Callable, Dict, FrozenSet, Iterator, List, Optional, Union import yaml @@ -110,6 +111,7 @@ def _locate_error(error: Any, locate: Callable[..., Optional[str]]) -> Optional[ known = schema.get("properties") or {} patterns = list((schema.get("patternProperties") or {}).keys()) for key in error.instance: + # re.search, as jsonschema's additionalProperties reads these keys. if key not in known and not any(re.search(p, str(key)) for p in patterns): return locate([*path, key]) if error.validator == "required": @@ -132,6 +134,72 @@ def _error_path(error: Any, locate: Optional[Callable[..., Optional[str]]] = Non return f"{where} ({path})" if where else path +def _end_anchored(pattern: str) -> str: + r"""``pattern`` with each ``$`` anchor written ``\Z``, the end of the value. + + JSON Schema writes ``pattern`` in ECMA-262, where ``$`` matches only at the + end of the value. Python's ``$`` also matches before a final ``\n``, so + jsonschema's ``re.search`` let ``"orders\n"`` through ``^[a-z_]+$``. + fastjsonschema and js-regex make the same rewrite with a ``(? re.Pattern[str]: + return re.compile(_end_anchored(pattern)) + + +def _pattern_keyword(validator: Any, pattern: str, instance: Any, schema: Any) -> Iterator[Any]: + """jsonschema's ``pattern`` keyword, with ``$`` read as ECMA-262 reads it.""" + if not validator.is_type(instance, "string") or _compiled_pattern(pattern).search(instance): + return + message = f"{instance!r} does not match {pattern!r}" + if re.search(pattern, instance): # only the final line break fails + message += " (it ends in a line break, which a YAML | or > block keeps and |- or >- drops)" + yield jsonschema.ValidationError(message) + + +@functools.lru_cache(maxsize=None) +def _end_anchored_validator(validator_class: Any) -> Any: + return jsonschema.validators.extend(validator_class, {"pattern": _pattern_keyword}) + + +def schema_validator(schema: Dict[str, Any]) -> Any: + """A jsonschema validator for ``schema``, in the dialect it declares, whose + ``pattern`` ends at the end of the value (:func:`_end_anchored`). Validate + against a FLUID schema through this, never ``validator_for`` directly. + + The keyword is replaced the way check-jsonschema replaces it, with + ``jsonschema.validators.extend``. Two places keep jsonschema's own + ``re.search``: ``patternProperties`` keys, as its ``additionalProperties`` + and :func:`_locate_error` read them, and a subschema that declares its own + ``$schema`` (or a ``$ref`` to the root, which does), for which jsonschema + picks that dialect's own class. No FLUID schema has either; a test fails if + one does. + """ + return _end_anchored_validator(jsonschema.validators.validator_for(schema))(schema) + + def _schema_version_sort_key(version: str) -> tuple[int, int, int]: match = VERSION_PATTERN.match(version) if not match: @@ -736,7 +804,7 @@ def _validate_with_jsonschema( # 2020-12; validating them with Draft 7 worked only because none # has yet used a 2020-12-only keyword. The first one to do so # would have been accepted without error. - validator = jsonschema.validators.validator_for(schema)(schema) + validator = schema_validator(schema) # Validate and collect errors errors = sorted(validator.iter_errors(contract), key=lambda e: e.path) diff --git a/tests/test_catalog_auth_schema.py b/tests/test_catalog_auth_schema.py index 30a9527b..06275bb6 100644 --- a/tests/test_catalog_auth_schema.py +++ b/tests/test_catalog_auth_schema.py @@ -164,6 +164,8 @@ def test_ga_075_refuses_the_key(): ({**_OAUTH2, "credentialEnv": "LK_CRED\n"}, "credentialEnv: does not match"), ({"type": "bearer", "tokenEnv": "UC_TOKEN\n"}, "tokenEnv: does not match"), ({"type": "bearer", "tokenEnv": _PAT}, "tokenEnv: must not match"), + # The token check's "$" no longer matches before the "\n"; the name pattern refuses it. + ({"type": "bearer", "tokenEnv": _PAT + "\n"}, "tokenEnv: does not match"), ( {**_OAUTH2, "credentialEnv": "AKIAIOSFODNN7EXAMPLE"}, # pragma: allowlist secret "credentialEnv: must not match", @@ -193,9 +195,10 @@ def test_076_refuses_a_malformed_catalog_auth(catalog_auth, expected): ({"type": "bearer", "tokenEnv": "T", "credentialEnv": _SECRET}, _SECRET), ({"type": _SECRET}, _SECRET), ({"type": "bearer", "tokenEnv": _PAT}, _PAT), + ({"type": "bearer", "tokenEnv": _PAT + "\n"}, _PAT), ({**_OAUTH2, "oauth2ServerUri": _USERINFO_URI}, "2OR3eRvYfSZ"), ], - ids=["credentialEnv", "tokenEnv", "wrong-type-field", "type", "pat", "userinfo"], + ids=["credentialEnv", "tokenEnv", "wrong-type-field", "type", "pat", "pat-lf", "userinfo"], ) def test_a_schema_error_never_quotes_the_value(catalog_auth, secret): errors = _errors("0.7.6", catalog_auth) @@ -224,6 +227,7 @@ def test_schema_and_parser_share_the_variable_name_rule(): {**_OAUTH2, "credentialEnv": "LK_CRED\n"}, {"type": "bearer", "tokenEnv": "UC_TOKEN\n"}, {"type": "bearer", "tokenEnv": _PAT}, + {"type": "bearer", "tokenEnv": _PAT + "\n"}, {"type": "bearer", "tokenEnv": "S3_CREDENTIAL_V2"}, {"type": "bearer", "tokenEnv": "LAKEKEEPERCATALOGCREDENTIALPROD"}, {**_OAUTH2, "oauth2ServerUri": _USERINFO_URI}, diff --git a/tests/test_schema_pattern_end_anchor.py b/tests/test_schema_pattern_end_anchor.py new file mode 100644 index 00000000..6b12793d --- /dev/null +++ b/tests/test_schema_pattern_end_anchor.py @@ -0,0 +1,267 @@ +# Copyright 2024-2026 Agentics Transformation Ltd +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +"""A schema ``pattern`` ends where the value ends, as ECMA-262 reads ``$``. + +JSON Schema writes ``pattern`` in the ECMA-262 dialect, where ``$`` (no ``m`` +flag) matches only at the end of the input. jsonschema runs it with Python's +``re.search``, and Python's ``$`` also matches before a final ``\\n``. So +``fluid validate`` accepted ``id: "orders\\n"``, which a YAML ``|`` block +scalar produces, while an editor validating against the same schema in +JavaScript refused it. +""" + +from __future__ import annotations + +import json +from pathlib import Path +from re import sub +from typing import Any, Dict, Iterator, List + +import pytest +import yaml + +import fluid_build +from fluid_build.schema_manager import FluidSchemaManager, _end_anchored, schema_validator + +try: # Python 3.11+ + from re import _parser as sre_parse +except ImportError: # Python 3.10 + import sre_parse # type: ignore[no-redef] + +pytestmark = [pytest.mark.unit] + +_VERSIONS = list(FluidSchemaManager.BUNDLED_VERSIONS) +_SCHEMA_DIR = Path(fluid_build.__file__).parent / "schemas" +_LINE_BREAK_HINT = " (it ends in a line break, which a YAML | or > block keeps and |- or >- drops)" + + +def _expose(expose_id: str = "orders") -> Dict[str, Any]: + return { + "exposeId": expose_id, + "kind": "table", + "binding": { + "platform": "local", + "format": "csv", + "location": {"path": "data/orders.csv"}, + }, + "contract": {"schema": [{"name": "id", "type": "integer"}]}, + } + + +def _contract(version: str, product_id: str = "orders") -> Dict[str, Any]: + return { + "fluidVersion": version, + "kind": "DataProduct", + "id": product_id, + "name": "Orders", + "domain": "sales", + "metadata": {"layer": "Bronze", "owner": {"team": "data"}}, + "exposes": [_expose()], + } + + +def _errors(contract: Dict[str, Any]) -> List[str]: + version = contract["fluidVersion"] + result = FluidSchemaManager().validate_contract(contract, version, offline_only=True) + return [] if result.is_valid else list(result.errors) + + +def test_every_bundled_version_is_checked() -> None: + """Guard the guard: an empty version list would pass every case below.""" + assert len(_VERSIONS) >= 7, _VERSIONS + + +@pytest.mark.parametrize("version", _VERSIONS) +def test_an_id_that_ends_in_a_newline_is_refused(version: str) -> None: + assert _errors(_contract(version)) == [] + errors = _errors(_contract(version, "orders\n")) + assert any(e.startswith("id: ") for e in errors), errors + + +def test_fluid_validate_refuses_a_block_scalar_id(tmp_path: Path, capsys: Any) -> None: + """``id: |`` keeps the line break, so the value is ``"orders\\n"``.""" + from fluid_build.cli import main + + body = yaml.safe_dump({k: v for k, v in _contract("0.7.6").items() if k != "id"}) + contract = tmp_path / "contract.fluid.yaml" + contract.write_text(f"id: |\n orders\n{body}", encoding="utf-8") + assert yaml.safe_load(contract.read_text(encoding="utf-8"))["id"] == "orders\n" + + rc = main(["validate", str(contract), "--offline", "--cache-dir", str(tmp_path / "cache")]) + + captured = capsys.readouterr() + text = captured.out + captured.err + assert rc == 1, text + assert "id" in text and "does not match" in text, text + + +def test_fluid_validate_refuses_a_fragment_whose_expose_id_ends_in_a_newline( + tmp_path: Path, capsys: Any +) -> None: + from fluid_build.cli import main + + fragment = tmp_path / "fragments" / "orders.yaml" + fragment.parent.mkdir(parents=True) + fragment.write_text(yaml.safe_dump(_expose("orders\n")), encoding="utf-8") + argv = ["validate", "--offline", "--cache-dir", str(tmp_path / "cache")] + argv += ["--schema-version", "0.7.6", "--fragment", "expose", str(fragment)] + + rc = main(argv) + + captured = capsys.readouterr() + text = captured.out + captured.err + assert rc == 1, text + assert "exposeId" in text and "does not match" in text, text + + +def test_bundle_validation_refuses_an_id_that_ends_in_a_newline() -> None: + """``fluid validate .tgz`` checks the resolved contract through + ``forge.core.validators``, not ``FluidSchemaManager.validate_contract``.""" + from fluid_build.forge.core.validators import validate_json_schema + + def issues(product_id: str) -> List[str]: + found = validate_json_schema( + "contract.resolved.yaml", + _contract("0.7.6", product_id), + schema_manager=FluidSchemaManager(), + ) + return [i.message for i in found] + + assert issues("orders") == [] + messages = issues("orders\n") + assert any(m.startswith("at id: ") for m in messages), messages + + +def test_bundle_validation_without_jsonschema_is_an_error(monkeypatch: Any) -> None: + """The check reads the flag ``schema_validator`` needs: a jsonschema that is found but + fails to import is the library-missing error, not a skipped check.""" + from fluid_build.forge.core.validators import validate_json_schema + + monkeypatch.setattr("fluid_build.schema_manager.JSONSCHEMA_AVAILABLE", False) + issues = validate_json_schema( + "contract.resolved.yaml", _contract("0.7.6"), schema_manager=FluidSchemaManager() + ) + assert [(i.code, i.severity) for i in issues] == [("SCHEMA-LIB-MISSING", "error")] + + +def test_the_error_says_the_value_ends_in_a_line_break() -> None: + errors = _errors(_contract("0.7.6", "orders\n")) + assert len(errors) == 1, errors + assert errors[0].startswith("id: 'orders\\n' does not match ") + assert errors[0].endswith(_LINE_BREAK_HINT) + + +@pytest.mark.parametrize("product_id", ["orders!", "orders!\n"]) +def test_a_value_wrong_for_another_reason_gets_no_line_break_hint(product_id: str) -> None: + errors = _errors(_contract("0.7.6", product_id)) + assert errors and not any("line break" in e for e in errors), errors + + +def test_the_json_schema_test_suite_case() -> None: + """JSON-Schema-Test-Suite, ``tests/draft2020-12/optional/ecmascript-regex.json``: + "ECMA 262 regex $ does not match trailing newline".""" + schema = { + "$schema": "https://json-schema.org/draft/2020-12/schema", + "type": "string", + "pattern": "^abc$", + } + assert schema_validator(schema).is_valid("abc") + assert not schema_validator(schema).is_valid("abc\n") + + +def test_a_not_pattern_reads_the_end_anchor_the_same_way() -> None: + """``not: {pattern: ...}`` now matches fewer values, as in ECMA-262: a deny rule that + has to catch a line break must name one (catalogAuth refuses ``[\\x00-\\x1f\\x7f]``).""" + schema = { + "$schema": "https://json-schema.org/draft/2020-12/schema", + "not": {"pattern": "^x$"}, + } + assert not schema_validator(schema).is_valid("x") + assert schema_validator(schema).is_valid("x\n") + + +# ── the rewrite ───────────────────────────────────────────────────────────── + + +def _tree(pattern: str) -> str: + """Python's own parse: ``[(AT, AT_BEGINNING), (LITERAL, 97), (AT, AT_END)]``.""" + return repr(sre_parse.parse(pattern)) + + +def _only_end_anchors_changed(pattern: str) -> bool: + """Python's parser reads the rewrite as ``pattern`` with each ``$`` (``AT_END``) + made ``\\Z`` (``AT_END_STRING``), and nothing else changed.""" + expected = sub(r"\bAT_END\b", "AT_END_STRING", _tree(pattern)) + return _tree(_end_anchored(pattern)) == expected + + +@pytest.mark.parametrize( + "pattern, rewritten", + [ + (r"^abc$", r"^abc\Z"), + (r"^a$|^b$", r"^a\Z|^b\Z"), + (r"(?:^|_)[A-Za-z0-9]{20,}(?:_|$)", r"(?:^|_)[A-Za-z0-9]{20,}(?:_|\Z)"), + (r"^P(?!$)(\d+D)?$", r"^P(?!\Z)(\d+D)?\Z"), + (r"^\$\{$", r"^\$\{\Z"), # an escaped "$" is a dollar sign + (r"^a\\$", r"^a\\\Z"), # after an escaped backslash it is the anchor + (r"^[$]+$", r"^[$]+\Z"), # in a set it is a dollar sign + (r"^[\]$]+$", r"^[\]$]+\Z"), + (r"^[]$]+$", r"^[]$]+\Z"), # a "]" first in a set does not close it + (r"^[^]$]+$", r"^[^]$]+\Z"), + (r"^[a-]$", r"^[a-]\Z"), + ], +) +def test_the_rewrite_changes_only_the_end_anchors(pattern: str, rewritten: str) -> None: + assert _end_anchored(pattern) == rewritten + assert _only_end_anchors_changed(pattern) + + +def _nodes(node: Any) -> Iterator[Dict[str, Any]]: + """Every object in a schema document, the root first.""" + if isinstance(node, dict): + yield node + for value in node.values(): + yield from _nodes(value) + elif isinstance(node, list): + for value in node: + yield from _nodes(value) + + +def test_every_bundled_schema_pattern_is_rewritten_anchor_for_anchor() -> None: + """The scan reads escapes and sets as Python does, checked against Python's own + parser for every ``pattern`` the bundled schemas hold.""" + patterns = { + node["pattern"] + for path in _SCHEMA_DIR.glob("fluid-*schema-*.json") + for node in _nodes(json.loads(path.read_text(encoding="utf-8"))) + if isinstance(node.get("pattern"), str) + } + assert len(patterns) >= 20, patterns # the glob found the schemas + assert [p for p in patterns if not _only_end_anchors_changed(p)] == [] + + +@pytest.mark.parametrize( + "path", sorted(_SCHEMA_DIR.glob("fluid-schema-*.json")), ids=lambda path: path.name +) +def test_every_pattern_of_the_schema_is_checked_with_the_rewrite(path: Path) -> None: + """jsonschema picks the validator class again for each subschema it enters, by + that subschema's ``$schema``. One that declares it, or a ``$ref`` to the root, + which does, would be checked with jsonschema's own ``pattern`` again. + ``patternProperties`` keys are not rewritten (``schema_validator`` says why), so + a schema that starts to use the keyword has to come back here.""" + below_root = list(_nodes(json.loads(path.read_text(encoding="utf-8"))))[1:] + assert [n for n in below_root if isinstance(n.get("$schema"), str)] == [] + assert [n for n in below_root if n.get("$ref") == "#"] == [] + assert [n for n in below_root if "patternProperties" in n] == []