Repository navigation
security: schema validation let a value with a trailing newline through a pattern - #799
Merged
Merged
Conversation
…gh a pattern
JSON Schema writes `pattern` in ECMA-262, where `$` matches only at the end
of the value. jsonschema runs it with Python's re.search, whose `$` also
matches before a final newline, so `fluid validate` accepted
`id: "orders\n"` (what an `id: |` block scalar produces) under every
bundled schema, 0.7.1 to 0.7.7. An editor validating the same schema in
JavaScript refuses it.
schema_manager.schema_validator(schema) extends the declared dialect's
validator with a `pattern` keyword that rewrites each `$` anchor to `\Z`.
Contract and fragment validation (_validate_with_jsonschema) and bundle
validation (forge.core.validators.validate_json_schema) use it. The
schemas are unchanged, so cached and fetched ones are covered too, and
forge_docs, the fluid spec and Command Center's models need no change.
When the line break is the only problem, the error says so.
patternProperties keys keep jsonschema's re.search, as its own
additionalProperties and _locate_error read them. No FLUID schema uses
the keyword, and a test fails if one starts to. The catalogAuth token
check, `not: {pattern: ...(?:_|$)}`, now reads `$` as ECMA-262 does
too; the name pattern beside it already refuses any value that ends in
a newline, pinned in test_catalog_auth_schema.py.
Borrowed: the keyword override from check-jsonschema
(jsonschema.validators.extend), and the `$` to `\Z` rewrite from
fastjsonschema 2.21.2 and js-regex 1.0.1. Their (?<!\\)\$ finder turns
`[$]` into `[\Z]` (bad escape) and misses `\\$`, so this scan skips
escapes and sets. Conformance case from JSON-Schema-Test-Suite
draft2020-12/optional/ecmascript-regex.json.
…imported Review follow-ups for the pattern end-anchor change: - validate_json_schema checked importlib.util.find_spec, which finds a jsonschema that then fails to import. schema_validator then raised NameError, reported as a SCHEMA999 warning when not strict, where the base returned the SCHEMA-LIB-MISSING error. It now reads schema_manager.JSONSCHEMA_AVAILABLE, the flag schema_validator needs. - The schema_validator docstring says FLUID-schema validation goes through it, and names the second place jsonschema's own re.search remains: a subschema that declares its own $schema, or a $ref to the root. - CHANGELOG: a sink.catalogAuth field gets no line-break hint, since its value is never shown. - Tests: the library-missing error, and a not-pattern reading $ as ECMA-262 does.
…ailing-newline # Conflicts: # CHANGELOG.md
github-actions
Bot
added
tests
📄 Documentation ReminderThis PR appears to be missing a documentation reference. Our docs live in a separate repo. Please update the PR description with one of:
See the Contributing Guide for details. |
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
The code-quality check flagged re and fluid_build.schema_manager as imported with both import and from-import. re now comes in as `from re import sub`, and the jsonschema-missing test patches the flag by its dotted name.
This was referenced Nov 4, 2026
fas89
added a commit
that referenced
this pull request
Nov 4, 2026
… through a pattern (#811) * security: the ODCS and plugin-extension checks let a trailing newline through a pattern The FLUID fix (schema_validator, #799) left six JSON Schema checks on jsonschema's own pattern keyword, Python's re.search, whose $ also matches before a final newline: - forge/core/artifact_validators.py (fluid validate-artifacts: ODCS, Bitol ODPS) - providers/odcs/validation.py::collect_errors (fluid odcs validate) - providers/odps_standard/validation.py::validate - providers/opds/validator.py (pinned Draft202012Validator) - forge_datamodel/emit/osi_sidecar.py (the copilot's Ossie check) - extension_schemas.py::assemble_proposed_extensions (pinned Draft7Validator) So ODCS refused schema[].id "t 1" but took "t1\n", which an id: | block produces, and the copilot kept an LLM-proposed customScaffold block with use: "ci:basic\n", ref: "main\n" or id: "ci\n". All six now go through schema_validator, and say so with the same line-break hint. schema_validator takes a default dialect for a schema that declares none, passed to jsonschema's validator_for(default=), the way OpenStack Rally uses it. An extension schema is read in the dialect it declares, draft-07 when it declares none (the documented plugin contract), so a 2020-12 schema's unevaluatedProperties: false is enforced instead of ignored. OPDS keeps 2020-12 as its default. patternProperties keys keep re.search. jsonschema matches them in patternProperties, additionalProperties and unevaluatedProperties, and check-jsonschema's override of patternProperties alone lets a key that one keyword matches and another does not through additionalProperties: false unchecked. Only OPDS has key patterns (^[a-z]{2}$, three places). The tripwires now cover every shipped schema, pin that set, and also fail on a $recursiveRef or $dynamicRef, which re-pick the stock class as a nested $schema does. Follows #799. * security: review follow-ups for the vendored-schema end-anchor change - fluid odcs validate cuts each schema message to 120 characters, so for an id like customer_orders_v1 the hint ended "... block keeps and |", which reads as advice to use |. _brief cuts the message ahead of the hint instead; the hint is schema_manager.LINE_BREAK_HINT now, so the command can find it. - schema_validator raises ImportError when jsonschema could not be imported. validate_osi_document, the one unguarded caller, raised NameError: jsonschema instead. validate_opds reads the same JSONSCHEMA_AVAILABLE flag as the other checks; the find_spec probe that found an unimportable jsonschema is gone. - A test walks fluid_build/ and fails on any validator_for(), Draft*Validator() or jsonschema.validate() without cls= outside schema_manager, so a check added later cannot read $ as Python does again. It finds the six sites on the base and seven on the old main. - Tests for the flag branches of collect_errors and the artifact validators, and for OPDS reading an undeclared schema as 2020-12. - CHANGELOG: the extension check shows no message, so it carries no hint. - odps_standard.validate validates against the schema for the document's apiVersion, not only v1.0.0. * fix: a bracket in a value or an unusable plugin schema no longer stops the run Defect-scan follow-ups for the vendored-schema end-anchor change: - fluid odcs validate printed each schema error as Rich markup, so a failing value with "[/]" in it ended the command with "Unexpected error: closing tag '[/]' ... has nothing to close" (exit 2, also on main). The line is printed with markup=False, like the vowl line in the same function. - assemble_proposed_extensions raised when a plugin schema's $schema was not a string: validator_for cannot look it up, where Draft7Validator ignored it. A schema that cannot be applied now drops its block with a redacted warning, as a block that fails its schema is dropped, and the run goes on. This also covers {"type": 5}, which raised on main. - The guard test also catches `from jsonschema import validate`, `import jsonschema as js` with js.validate(...), jsonschema.validators.validate/extend/create, and a cls= that does not come from schema_validator. ODPS passes cls=type(schema_validator(...)) inline so the guard can see it. It parses only files that mention jsonschema. * docs(changelog): keep the two pattern end-anchor entries together The rebase onto main put two new fluid diff --baseline entries between the FLUID-schema entry (#799) and this branch's, which begins "The fix above covered the FLUID schemas only".
fas89
added a commit
that referenced
this pull request
Nov 4, 2026
…atch (#816) Python's $ also matches just before a final "\n", so re.match(r"^[a-z]+$", "orders\n") succeeds. #788 and #797 fixed the known validators with \Z or fullmatch, but nothing kept the class from coming back: no ruff, pylint, bandit, dlint or Semgrep CE rule covers it (astral-sh/ruff#15017 proposes one). tests/security/test_no_dollar_anchored_validators.py walks the AST of fluid_build/ and follows the pattern of every .match / .search call back to where it is written: a literal or constant, a dict or tuple, an import, a helper's return value, a property, a re.Pattern parameter, or a pattern compiled at run time. It reads each pattern with re's own parser, so alternatives, groups, escapes and VERBOSE count the way re reads them. Calls whose $ is deliberate are listed in _ACCEPTED with the reason. An entry that no longer matches a call fails as well, so the list only shrinks. On main before #797 it fails on 44 calls, including the shapes the sweep missed: a pattern held in a dict, schema patterns compiled at run time, and a pattern the caller passes in. It also caught the $ + .match sites merged since (#793, #801, #799). Each was read and is listed with its reason.
2 tasks done
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What and why
JSON Schema writes
patternin ECMA-262, where$matches only at the end of the value. jsonschema runs it with Python'sre.search, and Python's$also matches before a final\n. Sofluid validateacceptedid: "orders\n"under every bundled schema, 0.7.1 to 0.7.7. That is the value anid: |block scalar produces. An editor that validates against the same schema in JavaScript refuses it.schema_manager.schema_validator(schema)extends the validator of the dialect the schema declares, and replaces itspatternkeyword with one that rewrites each$anchor to\Z._validate_with_jsonschema) and bundle validation (forge.core.validators.validate_json_schema) both use it.gen_contract_reference.py --check), the fluid spec and Command Center's models need no change.Fix choice: validator, not schema
The bundled schemas already say the right thing: under ECMA-262,
^…$refuses"orders\n". JSON-Schema-Test-Suite pins this indraft2020-12/optional/ecmascript-regex.json, "ECMA 262 regex $ does not match trailing newline". The part that breaks the spec is the Python validator.Rewriting the schemas to end in
(?![\s\S])was rejected. It would change the published pattern text in all 7 versions, including stable ones, plus the generated authoring schema. It would also mean re-vendoring and doc drift across three repos, and every new pattern would have to remember the idiom.What stays as it is
patternPropertieskeys keep jsonschema'sre.search, which is how its ownadditionalPropertiesreads them._locate_error(schema_manager.py:113) mirrors that, so it still names the key the validator reported. RewritingpatternPropertiesalone, as check-jsonschema does, would let anadditionalProperties: falsekey slip past both keywords. No FLUID schema usespatternProperties. A test fails if one starts to, or if a subschema declares its own$schema, or if a$refpoints at the root (jsonschema re-picks the validator class there).notpattern gets looser: the catalogAuth token check(?:_|$)oncredentialEnv/tokenEnv. It now also reads$as ECMA-262 does. The name pattern on the same fields already refuses any value that ends in a newline, andtest_catalog_auth_schema.pypins it: refused, and the value is not quoted.Borrow-before-build receipts
jsonschema.validators.extend(cls, {"pattern": …, "patternProperties": …}), backed by regress. We borrow the keyword-override shape.draft04.py,DOLLAR_FINDER) and js-regex 1.0.1 (_impl.py:84-86, archived 2020, MPL-2.0) do the$→\Zrewrite with a(?<!\\)\$finder. Probed: it turns^[$]+$into^[\Z]+\Z(re.error: bad escape \Z) and leaves^a\\$unrewritten. This scan skips escapes and[...]sets instead.$→\zfor the same ECMA semantics.column.typepattern starts with Python-only(?i)in all 7 versions, and no ECMA engine compiles that. regress is also a native dependency, against the lightweight-CLI rule.Validation
id: |block,--fragment expose, bundle validation). The test-suite auditor confirmed this independently on base01497207.fluid validateon theid: |contract: exit 1 on the branch, exit 0 with ✅ on main.fluid validate --fragment exposeonexposeId: >: exit 1, error located atfragments/orders.yaml:1 (exposeId)with the hint..tgzbundled on main, thenfluid validate orders.tgz: 1 error on the branch, 0 issues on main.--format jsonoutput carries the hint, and the shape is unchanged.id: |-) validates; a freshfluid init --template customer-360still validates.schema_validator.3704ab3f,-n 6): 31,934 passed, 14 failed, 2 errors. Rerun on clean base and on the branch, the failing tests fail identically on both. They are the startup and timing budgets, the DuckDB temp-root tests, the dlt version in the local venv, and thetest_ci_*import errors.test_coding_agent_livecalls the realclaudeCLI and was not rerun.--help).lint-imports(6 kept, 0 broken), product guardrail (0 failing),detect-secrets-hook1.5.0 on the changed files, andscripts/mypy_strict.pyall pass.--quality-delta: no gating row on changed code beyond the known noise._pattern_keywordand the test functions are called dynamically, by jsonschema and pytest.validate_json_schemagrew from 72 to 75 lines. It was already over the 60-line bar.Reviews
patterncheck, it was called 0 times on the bundled schemas.84d9c36a.84d9c36a.not-pattern test is added in84d9c36a.evolvefallback for a nested$schemais latent and guarded by a test.find_spec. On a broken install it turned theSCHEMA-LIB-MISSINGerror into a SCHEMA999 warning. Fixed in84d9c36a, and pinned by a test that fails on3704ab3f.(?#…)or(?x)comments containing[,(?m)), which is not ECMA-262 and is in no bundled schema; and a claimed regression for a quantified$(a$*fails to compile both before and after).Follow-ups (not in this PR)
forge/core/artifact_validators.py:338,providers/odcs/validation.py,forge_datamodel/emit/osi_sidecar.py,extension_schemas.py) still use jsonschema's ownpattern. Reproduced: an ODCSschema[].idof"t1\n"passes. The fix is to route these validators throughschema_validator, as a separate PR. security: refuse a trailing newline in whole-string validators #797 covered other code: it changed the Python.matchchecks that apply these schemas' patterns..also matches\r(so^(user|group|serviceAccount|domain):.+$acceptsuser:alice\r), and\d/\wmatch Unicode characters.column.type's pattern starts with Python-only(?i)in all 7 schema versions. That is not valid ECMA-262, so an editor validating against the schema in JavaScript cannot compile it.