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}
| # Add field_validator import if missing | ||
| if "from pydantic import" in content and "field_validator" not in content: | ||
| content = content.replace( | ||
| "from pydantic import AnyUrl, ConfigDict, EmailStr, Field, RootModel", | ||
| "from pydantic import AnyUrl, ConfigDict, EmailStr, Field, RootModel, field_validator" | ||
| ) | ||
|
|
||
| entity_name_validator = ''' | ||
|
|
||
| @field_validator('root', mode='after') | ||
| @classmethod | ||
| def validate_entity_name(cls, value: str) -> str: | ||
| """Validate entity name: disallow ::, special characters, and control characters.""" | ||
| if "::" in value: | ||
| raise ValueError('Entity name cannot contain "::"') | ||
| forbidden_chars = set('><"|') | set(chr(c) for c in range(0x20)) | ||
| if any(c in forbidden_chars for c in value): | ||
| raise ValueError('Entity name contains invalid characters: ><"|, or control characters') | ||
| return value | ||
| ''' | ||
|
|
||
| test_case_entity_name_validator = ''' | ||
|
|
||
| @field_validator('root', mode='after') | ||
| @classmethod | ||
| def validate_test_case_entity_name(cls, value: str) -> str: | ||
| """Validate test case entity name: disallow ::, special characters, and control characters.""" | ||
| if "::" in value: | ||
| raise ValueError('Test case entity name cannot contain "::"') | ||
| forbidden_chars = set('><"|') | set(chr(c) for c in range(0x20)) | ||
| if any(c in forbidden_chars for c in value): | ||
| raise ValueError('Test case entity name contains invalid characters: ><"|, or control characters') | ||
| return value | ||
| ''' | ||
|
|
||
| if 'class EntityName(RootModel[str]):' in content and entity_name_validator not in content: | ||
| content = content.replace( | ||
| 'class EntityName(RootModel[str]):\n root: Annotated[\n str,\n Field(\n description=\'Name that identifies an entity.\',\n max_length=256,\n min_length=1,\n ),\n ]', | ||
| 'class EntityName(RootModel[str]):\n root: Annotated[\n str,\n Field(\n description=\'Name that identifies an entity.\',\n max_length=256,\n min_length=1,\n ),\n ]' + entity_name_validator | ||
| ) | ||
|
|
||
| if 'class TestCaseEntityName(RootModel[str]):' in content and test_case_entity_name_validator not in content: | ||
| content = content.replace( | ||
| 'class TestCaseEntityName(RootModel[str]):\n root: Annotated[\n str,\n Field(\n description=\'Name that identifies a test definition and test case.\',\n min_length=1,\n ),\n ]', | ||
| 'class TestCaseEntityName(RootModel[str]):\n root: Annotated[\n str,\n Field(\n description=\'Name that identifies a test definition and test case.\',\n min_length=1,\n ),\n ]' + test_case_entity_name_validator | ||
| ) |
There was a problem hiding this comment.
field_validator import silently skipped when AwareDatetime is present
When datamodel-code-generator emits basic.py with AwareDatetime in the import line (the DATETIME_AWARE_FILE_PATHS block later in this same script exists precisely to handle that case), the replacement on line 112 looks for "from pydantic import AnyUrl, ConfigDict, EmailStr, Field, RootModel" — a string that doesn't exist yet because AwareDatetime is still present. The str.replace() call silently no-ops, field_validator is never added to the import, but the class-body injection below (lines 143–147) may still append @field_validator to EntityName. After the DATETIME fix later removes AwareDatetime, the final file contains @field_validator usage with no corresponding import, causing a NameError at runtime when the generated module is first loaded.
The simplest fix is to move the import injection for basic.py after the DATETIME fix block, or to replace the targeted-string approach with a more general regex substitution similar to add_field_validator_import already used for table.py and schema.py.
| if 'class EntityName(RootModel[str]):' in content and entity_name_validator not in content: | ||
| content = content.replace( | ||
| 'class EntityName(RootModel[str]):\n root: Annotated[\n str,\n Field(\n description=\'Name that identifies an entity.\',\n max_length=256,\n min_length=1,\n ),\n ]', | ||
| 'class EntityName(RootModel[str]):\n root: Annotated[\n str,\n Field(\n description=\'Name that identifies an entity.\',\n max_length=256,\n min_length=1,\n ),\n ]' + entity_name_validator | ||
| ) | ||
|
|
||
| if 'class TestCaseEntityName(RootModel[str]):' in content and test_case_entity_name_validator not in content: | ||
| content = content.replace( | ||
| 'class TestCaseEntityName(RootModel[str]):\n root: Annotated[\n str,\n Field(\n description=\'Name that identifies a test definition and test case.\',\n min_length=1,\n ),\n ]', | ||
| 'class TestCaseEntityName(RootModel[str]):\n root: Annotated[\n str,\n Field(\n description=\'Name that identifies a test definition and test case.\',\n min_length=1,\n ),\n ]' + test_case_entity_name_validator | ||
| ) |
There was a problem hiding this comment.
EntityName / TestCaseEntityName
The class-body injection for EntityName (line 144) and TestCaseEntityName (line 150) uses exact multi-line string literals, while Column2, ColumnName, and FieldName all use the replace_class() regex helper introduced earlier in this script. If the code generator ever changes whitespace, field ordering, or description text, these str.replace() calls silently no-op and the validators are not injected — with no error or warning to the operator. Consider rewriting both using replace_class() to be consistent and resilient to generator output changes.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
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 |
1 similar comment
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 |
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 entity name validation by updating JSON Schema patterns across multiple schema files to block reserved FQN separator characters (
>,<,",|) and ASCII control characters (0x00–0x1F, including newlines), and synchronises the frontend TypeScript regex and Python model generation script accordingly.entityName,testCaseEntityName,columnName,fieldName, pipeline task names, and search index field names all get the new pattern^((?!::)[^><\"|\\x00-\\x1f])*$;pipeline.jsonandschema.jsongain the constraint for the first time.EntityCsv.java: Replaces a hardcoded check against the old pattern literal with astartsWith(\"name must match \")predicate, ensuring the guard still works after the pattern text changes.datamodel_generation.py: Abandons the fragile per-classfield_validatorinjection approach in favour of stripping the unsupported Pydantic pattern from every generated Python file, with an explicit comment explaining why client-side enforcement is intentionally omitted.Confidence Score: 5/5
Safe to merge; changes are additive validation constraints with no data-migration risk and correct logic across all three layers.
The regex update is consistent across JSON Schema (backend), Java validators, TypeScript, and the Python model generation script. The EntityCsv refactor correctly replaces a hardcoded pattern-string check that would have broken with the new regex, using a future-proof prefix match instead. The datamodel_generation.py simplification removes the fragile field_validator injection approach entirely, with a thorough comment explaining why client-side enforcement is intentionally omitted. Integration tests and unit tests cover the new validation boundaries end-to-end.
scripts/datamodel_generation.py — the pattern-removal list is comprehensive but relies on knowing the exact string form the code generator emits; a post-pass assertion would add safety.
Important Files Changed
entityNameandtestCaseEntityNamepatterns to block>,<,", `columnNamepattern from the old(?!::)lookahead-only to the stricter character-class form; consistent with the changes inbasic.json.minLength: 1and the newpatternconstraint to Taskname, which previously had no validation at all.startsWith("name must match "), making the email-prefix skip guard pattern-agnostic; the new logic is semantically equivalent to the old two-branch check.ENTITY_NAME_REGEXto match the backend pattern and aliasesTEST_CASE_NAME_REGEXto the same object; nogflag, so sharing the reference is safe.getProjectRoot()to usejava.nio.file.Pathfor platform-safe path component matching.Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[API Request with entity name] --> B{JSON Schema validation} B -->|Fails pattern| C[400 Bad Request] B -->|Passes - no violations| D[shouldValidateUserNameWithEmailPrefix returns true] B -->|Passes - other violations| E{Violation starts with 'name must match'?} E -->|YES| F[Skip email prefix validation] E -->|NO| D D --> G[validateUserNameWithEmailPrefix] G --> H{Email prefix valid?} H -->|NO| I[Add violation to list] H -->|YES| J[Continue] I --> K{violationList empty?} J --> K F --> K K -->|NO| L[importFailure - log violations] K -->|YES| M[createOrUpdate entity]%%{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[API Request with entity name] --> B{JSON Schema validation} B -->|Fails pattern| C[400 Bad Request] B -->|Passes - no violations| D[shouldValidateUserNameWithEmailPrefix returns true] B -->|Passes - other violations| E{Violation starts with 'name must match'?} E -->|YES| F[Skip email prefix validation] E -->|NO| D D --> G[validateUserNameWithEmailPrefix] G --> H{Email prefix valid?} H -->|NO| I[Add violation to list] H -->|YES| J[Continue] I --> K{violationList empty?} J --> K F --> K K -->|NO| L[importFailure - log violations] K -->|YES| M[createOrUpdate entity]Reviews (2): Last reviewed commit: "fix(codegen): resolve entity-name valida..." | Re-trigger Greptile