fix/entity name validation v2#30211
Conversation
…ty name validation regex
…names Fixes open-metadata#23268 Updated regex patterns in JSON schema definitions to block reserved FQN separator characters (::, >, ") and newlines in entity names. Files changed: - openmetadata-spec/.../type/basic.json: entityName, testCaseEntityName - openmetadata-spec/.../entity/data/table.json: columnName
Updated pattern to use \x00-\x1f range to block all control characters including \r, \n, \t, \0 in addition to reserved FQN characters. This also restores the \r restriction that was present in the original dot metacharacter but lost in the character class rewrite.
…hema fields and UI - Added < and | to blocked characters (consistent with entityLink pattern) - Updated searchIndexFieldName pattern in searchIndex.json - Added pattern validation to tableConstraint columns and partitionColumnDetails in table.json - Updated UI ENTITY_NAME_REGEX to match backend validation - Added unit tests for new blocked characters in regex.constants.test.ts Addresses review feedback on open-metadata#27521
Addresses inconsistency flagged in code review - columnName definition was missing < and | from blocked characters unlike other patterns.
…e in ENTITY_NAME_REGEX
…erved characters Added Java integration tests to verify that entity names containing reserved FQN characters (::, >, <, ", |) and control characters are rejected by the backend validation. Files changed: - TableResourceIT.java: added column name validation tests - TopicResourceIT.java: added schema field name validation tests - PipelineResourceIT.java: added task name validation tests - SearchIndexResourceIT.java: added search index field name validation tests - TestSuiteBootstrap.java: updated bootstrap for test suite - pipeline.json: updated pattern for task names - schema.json: updated pattern for schema field names
…less formatting - Added minLength: 1 to pipeline task name to prevent empty strings - Applied mvn spotless:apply formatting fixes
…ttern - TEST_CASE_NAME_REGEX still used the old lookahead-only pattern and did not block `|` or ASCII control characters, unlike ENTITY_NAME_REGEX and the tightened testCaseEntityName pattern in basic.json. This allowed names like `my|testcase` to pass client-side validation on the data quality test case form, then fail with a 400 from the backend. - Reuses ENTITY_NAME_REGEX directly to prevent future drift between the two constants.
…tors for entity names
…te static baseline
…ya6400/OpenMetadata into fix/entity-name-validation-v2
❌ PR checklist incompleteThis PR cannot be merged until the following are addressed on its linked issue:
The fields live on the linked issue in the Shipping project (open the issue → right sidebar → Projects). After you set them, re-run this check (or push a commit) — issue/project changes do not re-trigger it automatically. Maintainers can bypass this check by adding the |
|
Hi there 👋 Thanks for your contribution! The OpenMetadata team will review the PR shortly! Once it has been labeled as Let us know if you need any help! |
![gitar-bot[bot]](https://avatars.githubusercontent.com/in/827041?s=60&v=4){kind=small}
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Two related issues surfaced after adding client-side name validation for open-metadata#23268 (blocking reserved FQN characters in entity/column names): 1. scripts/datamodel_generation.py: remove the field_validator injection for EntityName, TestCaseEntityName, Column2, ColumnName, and FieldName. These validators rejected reserved characters (::, ><"|, control chars) at Pydantic construction time, which conflicts with transform_entity_names() in custom_basemodel_validation.py — an existing mechanism that deliberately allows these characters through the Python model layer so it can encode them (e.g. "::" -> "__reserved__colon__") on Create/Store models and decode them back on Fetch models. Constructing the raw-named object is a prerequisite for that encode step, so rejecting at construction broke it entirely (test_custom_basemodel_validation.py). 2. openmetadata-spec/.../entity/data/table.json: revert the inline "pattern" added to tableConstraint.columns items and partitionColumnDetails.columnName. Adding a pattern there forced datamodel-code-generator to wrap those fields in a dedicated RootModel class instead of plain str, changing TableConstraint.columns from List[str] to List[Column2]. That broke every call site treating constraint columns as plain strings, e.g. normalize_table_constraints()'s `c.lower()` in database_service.py, causing failures across test_common_db_source, test_bigquery, test_burstiq_metadata, test_filter_invalid_constraints, test_cockroach, test_table_constraints, and test_custom_pydantic. In both cases the actual enforcement of open-metadata#23268 is unaffected: the JSON Schema pattern on the columnName/entityName/testCaseEntityName definitions themselves (validated server-side) and the UI's ENTITY_NAME_REGEX are untouched. A constraint's columns list only references names that were already validated when the column itself was created, so re-validating (and re-typing) it a second time was redundant and is what caused the regressions. Verified: full ingestion unit-test suite passes (124 passed).
|
Hi there 👋 Thanks for your contribution! The OpenMetadata team will review the PR shortly! Once it has been labeled as Let us know if you need any help! |
Code Review ✅ Approved 2 resolved / 2 findingsTightens entity and column name validation across the platform to block reserved characters and control codes, resolving issues with FQN construction. This update includes automated codegen fixes for Pydantic models, UI constant synchronization, and improved E2E test coverage. ✅ 2 resolved✅ Quality: Codegen validator injection silently no-ops if output changes
✅ Edge Case: Client-side validation dropped for task/field/constraint column names
OptionsDisplay: compact → Showing less information. Comment with these commands to change the behavior for this request:
Was this helpful? React with 👍 / 👎 | Gitar |
IceS2
added
the
safe to test
Jest test CoverageUI tests summary
|
|
|
🔴 Playwright Results — 4 pipeline/setup failure(s)✅ 0 passed · ❌ 0 failed · 🟡 0 flaky · ⏭️ 0 skipped Pipeline and setup failures
How to debug locally# Download playwright-test-results-<shard> artifact and unzip
npx playwright show-trace path/to/trace.zip # view trace |
|
Hi @chirag-madlani could you check again, most playwright tests are passing now. could we merge it? Thanks @IceS2 for adding the |
|
Hey @jaya6400, This is looking great but there is still one thing I had flagged I would like you to check out:
Replace_separators in custom_basemodel_validation.py only handles ::, >, ", \n, \r, \t: That means that they would reach the server raw and fail. I think the simplest way is dropping them from the pattern since they should not cause any issues, what do you think? |
…cter such as > rather than old < or |
|
Thanks @IceS2 for your feedback (#30211 (comment)), now
Could u review and run the workflows again? |
Code Review ✅ Approved 2 resolved / 2 findingsTightens entity and column name validation to block reserved FQN characters, ensuring consistency across backend schemas, frontend regex, and generated models. All identified validation regressions and test coverage gaps have been successfully addressed. ✅ 2 resolved✅ Quality: Codegen validator injection silently no-ops if output changes
✅ Edge Case: Client-side validation dropped for task/field/constraint column names
OptionsDisplay: compact → Showing less information. Comment with these commands to change the behavior for this request:
Was this helpful? React with 👍 / 👎 | Gitar |
2 participants
Describe your changes:
Fixes #23268
I updated the regex patterns in JSON schema definitions to block reserved
FQN separator characters (
::,>,") and newlines (\n) from beingused in entity and column names.
The backend was accepting entity names with special characters like double
quotes and newlines, which caused issues in FQN construction since these
are reserved separator characters in OpenMetadata's FQN system.
Changes made:
openmetadata-spec/src/main/resources/json/schema/type/basic.json— updated pattern forentityNameandtestCaseEntityNameopenmetadata-spec/src/main/resources/json/schema/entity/data/table.json— updated pattern forcolumnNamePattern changed from
^((?!::).)*$to^((?!::)[^>\"\\x00-\\x1f])*$Unit Test: #27521 (comment)
Type of change:
Checklist:
Fixes #23268: <short explanation>Summary by Gitar
entityName,testCaseEntityName,columnName, andfieldNameto block reserved characters><"|and ASCII control characters.patternconstraints topipelinetask names andsearchIndexfields.datamodel_generation.pyto remove unsupported regex patterns in generated Python models and inject custom Pydanticfield_validatorfunctions to enforce name validation.EntityCsvto properly handle validation error reporting by skipping email prefix checks when name patterns fail.playwright.config.ts) to improve test stability.TEST_CASE_NAME_REGEXin the UI with backend patterns to ensure consistent validation across the platform.This will update automatically on new commits.
Greptile Summary
This PR tightens name validation across entity schemas by expanding the existing
::negative-lookahead pattern to a character-class approach that also blocks>,", and all ASCII control characters. The fix touches JSON schemas, the Python model-generation post-processing script, the frontend regex constants, and Java CSV import logic.basic.json,table.json,searchIndex.json,pipeline.json,schema.json): Pattern updated from^((?!::).)*$to^((?!::)[^>\"\\x00-\\x1f])*$;pipeline.jsonalso gains a newminLength: 1guard on task names.datamodel_generation.py): Pattern removal now targets all generated.pyfiles viaglobinstead of a fixed list, and correctly removes both the old and new pattern variants; the previously-plannedfield_validatorinjection was dropped to avoid conflicts withtransform_entity_namesencoding logic.ENTITY_NAME_REGEXandTEST_CASE_NAME_REGEXare unified and synced with the backend pattern;EntityCsv.shouldValidateUserNameWithEmailPrefixuses prefix matching so it works correctly regardless of the exact pattern string in the violation message.Confidence Score: 5/5
Safe to merge — changes only tighten input validation at the schema layer with no effect on existing data, and the Python model layer correctly remains permissive for internal encoding purposes.
All changes are additive restrictions on new input only. The Java validation message matching in EntityCsv.java is correctly updated to use prefix matching rather than the old literal string. The Python model-generation script removes the unsupported pattern from all generated files, and the explanation for why no client-side validator is injected is clearly documented. Integration tests cover the key new rejection cases.
scripts/datamodel_generation.py — the six-entry patterns_to_remove list has some redundancy worth a comment, but no functional issue.
Important Files Changed
entityNameandtestCaseEntityNamepatterns to block>,", and ASCII control chars (\x00-\x1f) in addition to::.minLength: 1and the new pattern constraint to the Tasknamefield, which previously had no validation at all.shouldValidateUserNameWithEmailPrefix()usingstartsWithprefix matching;violationListis always initialized as an emptyArrayListso no NPE risk.glob; removed the now-unnecessaryfield_validatorinjection; includes 6 pattern variants to handle different code-generator escape representations. Thereimport is correctly removed.ENTITY_NAME_REGEXto character-class approach and aliasedTEST_CASE_NAME_REGEXto it; no/gflag so sharing a reference is safe; frontend now in sync with backend patterns.Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[Client Request - entity name input] --> B{Frontend ENTITY_NAME_REGEX} B -->|fails| C[UI validation error - block submission] B -->|passes| D[HTTP POST to backend] D --> E{Java JSON Schema validation - Jakarta Validator} E -->|fails - name must match pattern| F[400 Bad Request] E -->|passes| G[Entity created or updated] H[CSV Import] --> I{ValidatorUtil.validate entity} I -->|violation starts with name must match| J[skip email-prefix check] I -->|no name violation| K[run email-prefix check] J --> L[importFailure] K --> M{email prefix valid?} M -->|no| L M -->|yes| N[createOrUpdate entity] O[Python SDK generation] --> P[Generate models from JSON Schema] P --> Q[Strip unsupported pattern= strings from all generated .py files] Q --> R[Server-side Java enforces validation - Python layer is permissive]%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%% flowchart TD A[Client Request - entity name input] --> B{Frontend ENTITY_NAME_REGEX} B -->|fails| C[UI validation error - block submission] B -->|passes| D[HTTP POST to backend] D --> E{Java JSON Schema validation - Jakarta Validator} E -->|fails - name must match pattern| F[400 Bad Request] E -->|passes| G[Entity created or updated] H[CSV Import] --> I{ValidatorUtil.validate entity} I -->|violation starts with name must match| J[skip email-prefix check] I -->|no name violation| K[run email-prefix check] J --> L[importFailure] K --> M{email prefix valid?} M -->|no| L M -->|yes| N[createOrUpdate entity] O[Python SDK generation] --> P[Generate models from JSON Schema] P --> Q[Strip unsupported pattern= strings from all generated .py files] Q --> R[Server-side Java enforces validation - Python layer is permissive]Reviews (5): Last reviewed commit: "Merge branch 'main' into fix/entity-name..." | Re-trigger Greptile