Skip to content

Commit 758ab96

Browse files
svidgensvidgen
authored
fix(schema-generator): sanitize database identifiers in TypeScript output path (#3510)
## 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
1 parent 6693c8f commit 758ab96

3 files changed

Lines changed: 178 additions & 79 deletions

File tree

‎packages/amplify-graphql-schema-generator/src/__tests__/__snapshots__/generate-ts-data-schema.test.ts.snap‎

Lines changed: 74 additions & 74 deletions
Original file line numberDiff line numberDiff line change
@@ -7,14 +7,14 @@ import { a } from \\"@aws-amplify/data-schema\\";
77
88
export const schema = a.schema({
99
\\"User\\": a.model({
10-
id: a.string().required(),
11-
name: a.string()
10+
\\"id\\": a.string().required(),
11+
\\"name\\": a.string()
1212
}).identifier([
1313
\\"id\\"
1414
]),
1515
\\"Profile\\": a.model({
16-
id: a.string().required(),
17-
details: a.string()
16+
\\"id\\": a.string().required(),
17+
\\"details\\": a.string()
1818
}).identifier([
1919
\\"id\\"
2020
])
@@ -29,22 +29,22 @@ import { a } from \\"@aws-amplify/data-schema\\";
2929
3030
export const schema = a.schema({
3131
\\"Table\\": a.model({
32-
id: a.string().required(),
33-
field1: a.string(),
34-
field2: a.string(),
35-
field3: a.integer(),
36-
field4: a.float(),
37-
field5: a.boolean(),
38-
field6: a.id(),
39-
field7: a.date(),
40-
field8: a.time(),
41-
field9: a.datetime(),
42-
field10: a.timestamp(),
43-
field11: a.json(),
44-
field12: a.email(),
45-
field13: a.phone(),
46-
field14: a.url(),
47-
field15: a.ipAddress()
32+
\\"id\\": a.string().required(),
33+
\\"field1\\": a.string(),
34+
\\"field2\\": a.string(),
35+
\\"field3\\": a.integer(),
36+
\\"field4\\": a.float(),
37+
\\"field5\\": a.boolean(),
38+
\\"field6\\": a.id(),
39+
\\"field7\\": a.date(),
40+
\\"field8\\": a.time(),
41+
\\"field9\\": a.datetime(),
42+
\\"field10\\": a.timestamp(),
43+
\\"field11\\": a.json(),
44+
\\"field12\\": a.email(),
45+
\\"field13\\": a.phone(),
46+
\\"field14\\": a.url(),
47+
\\"field15\\": a.ipAddress()
4848
}).identifier([
4949
\\"id\\"
5050
])
@@ -67,8 +67,8 @@ export const schema = configure({
6767
}
6868
}).schema({
6969
\\"User\\": a.model({
70-
id: a.string().required(),
71-
name: a.string()
70+
\\"id\\": a.string().required(),
71+
\\"name\\": a.string()
7272
}).identifier([
7373
\\"id\\"
7474
])
@@ -83,13 +83,13 @@ import { a } from \\"@aws-amplify/data-schema\\";
8383
8484
export const schema = a.schema({
8585
\\"User\\": a.model({
86-
id: a.string().required(),
87-
name: a.string(),
88-
status: a.ref(\\"UserStatus\\")
86+
\\"id\\": a.string().required(),
87+
\\"name\\": a.string(),
88+
\\"status\\": a.ref(\\"UserStatus\\")
8989
}).identifier([
9090
\\"id\\"
9191
]),
92-
UserStatus: a.enum([
92+
\\"UserStatus\\": a.enum([
9393
\\"ACTIVE\\",
9494
\\"INACTIVE\\"
9595
])
@@ -104,24 +104,24 @@ import { a } from \\"@aws-amplify/data-schema\\";
104104
105105
export const schema = a.schema({
106106
\\"User\\": a.model({
107-
id: a.string().required(),
108-
name: a.string(),
109-
status: a.ref(\\"UserStatus\\").required()
107+
\\"id\\": a.string().required(),
108+
\\"name\\": a.string(),
109+
\\"status\\": a.ref(\\"UserStatus\\").required()
110110
}).identifier([
111111
\\"id\\"
112112
]),
113113
\\"Test\\": a.model({
114-
id: a.string().required(),
115-
name: a.string(),
116-
age: a.ref(\\"TestAge\\").required()
114+
\\"id\\": a.string().required(),
115+
\\"name\\": a.string(),
116+
\\"age\\": a.ref(\\"TestAge\\").required()
117117
}).identifier([
118118
\\"id\\"
119119
]),
120-
UserStatus: a.enum([
120+
\\"UserStatus\\": a.enum([
121121
\\"ACTIVE\\",
122122
\\"INACTIVE\\"
123123
]),
124-
TestAge: a.enum([
124+
\\"TestAge\\": a.enum([
125125
\\"Above18\\",
126126
\\"Below18\\"
127127
])
@@ -136,18 +136,18 @@ import { a } from \\"@aws-amplify/data-schema\\";
136136
137137
export const schema = a.schema({
138138
\\"User\\": a.model({
139-
id: a.string().required(),
140-
name: a.string(),
141-
statsu1: a.ref(\\"UserStatus1\\"),
142-
status: a.ref(\\"UserStatus\\").required()
139+
\\"id\\": a.string().required(),
140+
\\"name\\": a.string(),
141+
\\"statsu1\\": a.ref(\\"UserStatus1\\"),
142+
\\"status\\": a.ref(\\"UserStatus\\").required()
143143
}).identifier([
144144
\\"id\\"
145145
]),
146-
UserStatus1: a.enum([
146+
\\"UserStatus1\\": a.enum([
147147
\\"ACTIVE\\",
148148
\\"INACTIVE\\"
149149
]),
150-
UserStatus: a.enum([
150+
\\"UserStatus\\": a.enum([
151151
\\"ACTIVE\\",
152152
\\"INACTIVE\\"
153153
])
@@ -162,13 +162,13 @@ import { a } from \\"@aws-amplify/data-schema\\";
162162
163163
export const schema = a.schema({
164164
\\"User\\": a.model({
165-
id: a.string().required(),
166-
name: a.string(),
167-
status: a.ref(\\"UserStatus\\").required()
165+
\\"id\\": a.string().required(),
166+
\\"name\\": a.string(),
167+
\\"status\\": a.ref(\\"UserStatus\\").required()
168168
}).identifier([
169169
\\"id\\"
170170
]),
171-
UserStatus: a.enum([
171+
\\"UserStatus\\": a.enum([
172172
\\"ACTIVE\\",
173173
\\"INACTIVE\\"
174174
])
@@ -183,20 +183,20 @@ import { a } from \\"@aws-amplify/data-schema\\";
183183
184184
export const schema = a.schema({
185185
\\"User\\": a.model({
186-
id: a.string().required(),
187-
name: a.string(),
188-
status: a.ref(\\"UserStatus\\").required()
186+
\\"id\\": a.string().required(),
187+
\\"name\\": a.string(),
188+
\\"status\\": a.ref(\\"UserStatus\\").required()
189189
}).identifier([
190190
\\"id\\"
191191
]),
192192
\\"Test\\": a.model({
193-
id: a.string().required(),
194-
name: a.string(),
195-
status: a.ref(\\"UserStatus\\").required()
193+
\\"id\\": a.string().required(),
194+
\\"name\\": a.string(),
195+
\\"status\\": a.ref(\\"UserStatus\\").required()
196196
}).identifier([
197197
\\"id\\"
198198
]),
199-
UserStatus: a.enum([
199+
\\"UserStatus\\": a.enum([
200200
\\"ACTIVE\\",
201201
\\"INACTIVE\\"
202202
])
@@ -231,8 +231,8 @@ export const schema = configure({
231231
}
232232
}).schema({
233233
\\"User\\": a.model({
234-
id: a.string().required(),
235-
name: a.string()
234+
\\"id\\": a.string().required(),
235+
\\"name\\": a.string()
236236
}).identifier([
237237
\\"id\\"
238238
])
@@ -277,14 +277,14 @@ export const schema = configure({
277277
}
278278
}).schema({
279279
\\"User\\": a.model({
280-
id: a.string().required(),
281-
name: a.string()
280+
\\"id\\": a.string().required(),
281+
\\"name\\": a.string()
282282
}).identifier([
283283
\\"id\\"
284284
]),
285285
\\"Profile\\": a.model({
286-
id: a.string().required(),
287-
details: a.string()
286+
\\"id\\": a.string().required(),
287+
\\"details\\": a.string()
288288
}).identifier([
289289
\\"id\\"
290290
])
@@ -307,14 +307,14 @@ export const schema = configure({
307307
}
308308
}).schema({
309309
\\"User\\": a.model({
310-
id: a.string().required(),
311-
name: a.string()
310+
\\"id\\": a.string().required(),
311+
\\"name\\": a.string()
312312
}).identifier([
313313
\\"id\\"
314314
]),
315315
\\"Profile\\": a.model({
316-
id: a.string().required(),
317-
details: a.string()
316+
\\"id\\": a.string().required(),
317+
\\"details\\": a.string()
318318
}).identifier([
319319
\\"id\\"
320320
])
@@ -338,14 +338,14 @@ export const schema = configure({
338338
}
339339
}).schema({
340340
\\"User\\": a.model({
341-
id: a.string().required(),
342-
name: a.string()
341+
\\"id\\": a.string().required(),
342+
\\"name\\": a.string()
343343
}).identifier([
344344
\\"id\\"
345345
]),
346346
\\"Profile\\": a.model({
347-
id: a.string().required(),
348-
details: a.string()
347+
\\"id\\": a.string().required(),
348+
\\"details\\": a.string()
349349
}).identifier([
350350
\\"id\\"
351351
])
@@ -368,9 +368,9 @@ export const schema = configure({
368368
}
369369
}).schema({
370370
\\"CoffeeQueue\\": a.model({
371-
id: a.integer().default(),
372-
name: a.string(),
373-
orderNumber: a.integer().default()
371+
\\"id\\": a.integer().default(),
372+
\\"name\\": a.string(),
373+
\\"orderNumber\\": a.integer().default()
374374
}).identifier([
375375
\\"id\\"
376376
])
@@ -415,8 +415,8 @@ export const schema = configure({
415415
}
416416
}).schema({
417417
\\"User\\": a.model({
418-
id: a.string().required(),
419-
name: a.string()
418+
\\"id\\": a.string().required(),
419+
\\"name\\": a.string()
420420
}).identifier([
421421
\\"id\\"
422422
])
@@ -462,8 +462,8 @@ export const schema = configure({
462462
}
463463
}).schema({
464464
\\"User\\": a.model({
465-
id: a.string().required(),
466-
name: a.string()
465+
\\"id\\": a.string().required(),
466+
\\"name\\": a.string()
467467
}).identifier([
468468
\\"id\\"
469469
])
@@ -509,8 +509,8 @@ export const schema = configure({
509509
}
510510
}).schema({
511511
\\"Person\\": a.model({
512-
id: a.string().required(),
513-
name: a.string()
512+
\\"id\\": a.string().required(),
513+
\\"name\\": a.string()
514514
}).identifier([
515515
\\"id\\"
516516
])

‎packages/amplify-graphql-schema-generator/src/__tests__/generate-ts-data-schema.test.ts‎

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -521,3 +521,78 @@ describe('Type name conversions', () => {
521521
);
522522
});
523523
});
524+
525+
describe('Identifier sanitization', () => {
526+
it('should sanitize column names with shell metacharacters', () => {
527+
const dbschema = new Schema(new Engine('MySQL'));
528+
const model = new Model('User');
529+
model.addField(new Field('id', { kind: 'NonNull', type: { kind: 'Scalar', name: 'String' } }));
530+
model.addField(new Field("require('child_process').execSync('echo pwned')", { kind: 'Scalar', name: 'String' }));
531+
model.setPrimaryKey(['id']);
532+
dbschema.addModel(model);
533+
534+
const graphqlSchema = generateTypescriptDataSchema(dbschema);
535+
// The malicious column name with special characters must not appear verbatim in the output
536+
expect(graphqlSchema).not.toContain("require('child_process')");
537+
expect(graphqlSchema).not.toContain("execSync('echo pwned')");
538+
// It should be sanitized: special chars replaced with underscores, safely quoted as a string literal
539+
expect(graphqlSchema).toContain('"require_child_process_execSync_echo_pwned_"');
540+
});
541+
542+
it('should sanitize column names with square brackets and special characters', () => {
543+
const dbschema = new Schema(new Engine('MySQL'));
544+
const model = new Model('User');
545+
model.addField(new Field('id', { kind: 'NonNull', type: { kind: 'Scalar', name: 'String' } }));
546+
model.addField(new Field('[malicious](injection)', { kind: 'Scalar', name: 'String' }));
547+
model.setPrimaryKey(['id']);
548+
dbschema.addModel(model);
549+
550+
const graphqlSchema = generateTypescriptDataSchema(dbschema);
551+
expect(graphqlSchema).not.toContain('[malicious]');
552+
expect(graphqlSchema).toContain('"malicious_injection_"');
553+
});
554+
555+
it('should sanitize enum type names with special characters', () => {
556+
const dbschema = new Schema(new Engine('Postgres'));
557+
const model = new Model('User');
558+
model.addField(new Field('id', { kind: 'NonNull', type: { kind: 'Scalar', name: 'String' } }));
559+
model.addField(new Field('status', { kind: 'Enum', name: "evil';DROP TABLE", values: ['A', 'B'] }));
560+
model.setPrimaryKey(['id']);
561+
dbschema.addModel(model);
562+
563+
const graphqlSchema = generateTypescriptDataSchema(dbschema);
564+
// The original unsanitized name should not appear
565+
expect(graphqlSchema).not.toContain("evil';DROP TABLE");
566+
// Sanitized and PascalCased enum name should be used as property key and ref target
567+
expect(graphqlSchema).toContain('"Evil_DROP_TABLE"');
568+
// The a.ref() should reference the sanitized name
569+
expect(graphqlSchema).toContain('a.ref("Evil_DROP_TABLE")');
570+
});
571+
572+
it('should produce a fallback name for column names with no alphabetic characters', () => {
573+
const dbschema = new Schema(new Engine('MySQL'));
574+
const model = new Model('User');
575+
model.addField(new Field('id', { kind: 'NonNull', type: { kind: 'Scalar', name: 'String' } }));
576+
model.addField(new Field('!@#$%^&*()', { kind: 'Scalar', name: 'String' }));
577+
model.setPrimaryKey(['id']);
578+
dbschema.addModel(model);
579+
580+
const graphqlSchema = generateTypescriptDataSchema(dbschema);
581+
// Should use fallback "field" for fields with no alpha chars
582+
expect(graphqlSchema).toContain('"field"');
583+
});
584+
585+
it('should use string literals for property keys (not raw identifiers)', () => {
586+
const dbschema = new Schema(new Engine('MySQL'));
587+
const model = new Model('User');
588+
model.addField(new Field('id', { kind: 'NonNull', type: { kind: 'Scalar', name: 'String' } }));
589+
model.addField(new Field('normal_name', { kind: 'Scalar', name: 'String' }));
590+
model.setPrimaryKey(['id']);
591+
dbschema.addModel(model);
592+
593+
const graphqlSchema = generateTypescriptDataSchema(dbschema);
594+
// Property keys should be string-quoted
595+
expect(graphqlSchema).toContain('"normal_name"');
596+
expect(graphqlSchema).toContain('"id"');
597+
});
598+
});

0 commit comments

Comments
 (0)