Skip to content

fix(schema-generator): sanitize database identifiers in TypeScript output path#3510

Merged
svidgen merged 1 commit into
mainfrom
fix/sanitize-schema-generator-identifiers
Jul 9, 2026
Merged

fix(schema-generator): sanitize database identifiers in TypeScript output path#3510
svidgen merged 1 commit into
mainfrom
fix/sanitize-schema-generator-identifiers

Conversation

@svidgen

@svidgen svidgen commented Jul 8, 2026

Copy link
Copy Markdown
Member

Summary

Sanitizes database column names and enum type names in the TypeScript data schema output path, matching the existing cleanMappedName behavior in the GraphQL-SDL output path.

Problem

The TypeScript schema generator passed raw database column names and enum type names directly to ts.factory.createIdentifier(), which emits them as bare JavaScript identifiers without any sanitization. Names containing special characters (parentheses, brackets, quotes, etc.) could produce invalid or unsafe TypeScript output.

Changes

  • Add sanitizeIdentifier() helper that strips leading non-alphabetic characters and replaces non-alphanumeric characters with underscores (matching cleanMappedName from the GraphQL-SDL path)
  • Use createStringLiteral instead of createIdentifier for field property names in createProperty()
  • Use createStringLiteral instead of createIdentifier for enum type property names in createEnums()
  • Sanitize enum type names in a.ref() calls for consistency between the enum definition and its references
  • Fix enum deduplication logic to work with StringLiteral property names (uses .text instead of .escapedText)

Defense in Depth

This is a defense-in-depth measure. The TS output path should apply the same sanitization as the GraphQL-SDL path to ensure consistent, safe output regardless of database column naming.

Testing

  • Added 5 new unit tests verifying:
    • Column names with shell metacharacters are sanitized
    • Column names with brackets/special chars are sanitized
    • Enum type names with special characters are sanitized
    • Fallback names are generated for all-special-character column names
    • Property keys are emitted as string literals
  • Updated 16 existing snapshots to reflect the new quoted property key format
  • All 100 tests in the package pass

…cript output path

Add sanitizeIdentifier() to strip/escape non-alphanumeric characters from
database column names and enum type names before emitting them as TypeScript
property keys. This matches the existing cleanMappedName behavior in the
GraphQL-SDL output path.

Changes:
- Replace createIdentifier(field.name) with createStringLiteral(sanitizeIdentifier(field.name))
  in createProperty() for model field properties
- Replace createIdentifier(toPascalCase([type.name])) with
  createStringLiteral(toPascalCase([sanitizeIdentifier(type.name)])) in createEnums()
- Sanitize enum type name in a.ref() calls for consistency
- Fix enum deduplication to work with StringLiteral property names
- Add unit tests verifying sanitization of dangerous column/enum names
- Update existing snapshots to reflect quoted property key format
@svidgen svidgen requested a review from a team as a code owner July 8, 2026 20:10
@svidgen

svidgen commented Jul 9, 2026

Copy link
Copy Markdown
Member Author

E2E & PR Checks Results ✅

Both batches completed successfully:

E2E batch (amplify-category-api-e2e-workflow:fec71fde-851d-46f5-ad25-8c7e92db76f5): 195/196 passed

  • Only persistent failure: cleanup_e2e_resources — this is a post-test infrastructure cleanup job, not an actual test. All 195 real test builds passed.
  • CodeBuild batch

PR checks batch (amplify-category-api-pr-workflow:fff7e3bd-b94c-4947-9ce9-18e35d6b16bd): 11/11 passed

Ready for merge.

Comment on lines +31 to +34
if (!name?.match(/[a-zA-Z]/)) {
const suffix = name?.replace(/[^0-9]+/g, '') ?? '';
return isField ? `field${suffix}` : `Model${suffix}`;
}

@Simone319 Simone319 Jul 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even though unlikely, if multiple fields contain no letter, and no number, they will result in the same name field. Shall we add a randomized suffix?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that equates to a bad state. Randomization doesn't give an anchor point to code against.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair point — randomization is the wrong call; a non-deterministic name would churn on every regen and give consumers nothing stable to code against. Withdrawing that suggestion.

That said, the underlying collision is still real: two all-special-character columns (e.g. () and @#$) both collapse to field, producing duplicate property keys. Since a column with zero alphanumeric characters is arguably a bad-state input anyway, a good follow-up would be to error with a clear message (pointing the user at an explicit rename/@mapsTo) rather than silently coercing. Fine to leave that for a separate change — flagging it so we don't lose the thread.

@svidgen svidgen merged commit 758ab96 into main Jul 9, 2026
7 of 8 checks passed
@svidgen svidgen deleted the fix/sanitize-schema-generator-identifiers branch July 9, 2026 20:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants