From 9046fad6f770ca0193b36c35fc4443ed68a6547c Mon Sep 17 00:00:00 2001 From: Herdiyan Adam Putra Date: Sat, 1 Aug 2026 02:08:49 +0700 Subject: [PATCH 1/2] fix(schematics): escape values interpolated into generated Data Connect provider code featureToRules's DataConnect case builds the generated provideDataConnect(...) call from two values read out of the project's own dataconnect.yaml/ connector.yaml: config.package (passed as a module specifier to addRootProvider's external()) and the connectorConfig object's location/connector/service strings (interpolated directly into a double-quoted object literal). Neither was validated or escaped, so a value containing a quote character breaks out and lets arbitrary source land in the project's generated provider file. config.package is now checked against a conservative allow-list pattern before being used as a module specifier, falling back to the connectorConfig object form otherwise. The connectorConfig values are now serialized with JSON.stringify instead of raw string interpolation. --- src/schematics/utils.ts | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/schematics/utils.ts b/src/schematics/utils.ts index d99836e09..00294167b 100644 --- a/src/schematics/utils.ts +++ b/src/schematics/utils.ts @@ -221,11 +221,14 @@ export function featureToRules( const config = dataConnectConfig; let angularConfig: undefined | string; if (config) { - if (config.package) { + // config.package and the connectorConfig values below come from the project's own + // dataconnect.yaml/connector.yaml, not from a trusted schema — reject anything that + // isn't a plausible package specifier before using it as a module name. + if (config.package && /^[^'"\\\n\r]+$/.test(config.package)) { configAsStr = external("connectorConfig", config.package); - } else { + } else if (config.connectorConfig) { configAsStr = `{${Object.keys(config.connectorConfig as ConnectorConfig).map( - (key) => `${key}: "${(config.connectorConfig as ConnectorConfig)[key]}"` + (key) => `${key}: ${JSON.stringify((config.connectorConfig as ConnectorConfig)[key])}` ).join(',')}}`; } if (config.angular) { From 99705360b7f90cf0a5b3a3cf2f0a12842a153f94 Mon Sep 17 00:00:00 2001 From: Herdiyan Adam Putra Date: Tue, 4 Aug 2026 19:37:50 +0700 Subject: [PATCH 2/2] fix(schematics): preserve string typing and add tests for DataConnect provider config codegen JSON.stringify alone changes a yaml scalar's type along with escaping it, so an unquoted numeric/boolean connectorConfig value (e.g. service: 123) no longer satisfied ConnectorConfig's string fields after the previous escaping fix. Wrap each value in String(...) before JSON.stringify to keep both the escaping and the string type. Extract the config-string-building logic into connectorConfigObjectLiteral, isValidPackageSpecifier, and resolveDataConnectProviderConfig so it's unit-testable independent of the addRootProvider schematic machinery, and add coverage for the injection-escaping fix, the type-coercion regression, and the no-javascriptSdk fallback path. --- src/schematics/utils.jasmine.ts | 123 ++++++++++++++++++++++++++++++++ src/schematics/utils.ts | 71 +++++++++++------- 2 files changed, 170 insertions(+), 24 deletions(-) create mode 100644 src/schematics/utils.jasmine.ts diff --git a/src/schematics/utils.jasmine.ts b/src/schematics/utils.jasmine.ts new file mode 100644 index 000000000..4c7e50834 --- /dev/null +++ b/src/schematics/utils.jasmine.ts @@ -0,0 +1,123 @@ +import { ConnectorConfig } from './interfaces.js'; +import { + connectorConfigObjectLiteral, + isValidPackageSpecifier, + resolveDataConnectProviderConfig, +} from './utils.js'; +import 'jasmine'; + +// eslint-disable-next-line @typescript-eslint/no-implied-eval +const evalObjectLiteral = (literal: string) => new Function(`return (${literal});`)(); + +describe('connectorConfigObjectLiteral', () => { + + it('escapes a quote-bearing value so it round-trips as inert string data', () => { + const injectionAttempt = 'us-central1"; console.log("INJECTED"); const _z="x'; + const literal = connectorConfigObjectLiteral({ + location: injectionAttempt, + connector: 'my-connector', + service: 'my-service', + } as ConnectorConfig); + const evaluated = evalObjectLiteral(literal); + expect(evaluated.location).toBe(injectionAttempt); + }); + + it('coerces a non-string value from an unquoted yaml scalar to a string', () => { + const literal = connectorConfigObjectLiteral({ + location: 'us-central1', + connector: 'my-connector', + service: 123, + } as unknown as ConnectorConfig); + const evaluated = evalObjectLiteral(literal); + expect(evaluated.service).toBe('123'); + expect(typeof evaluated.service).toBe('string'); + }); + + it('coerces a boolean-like yaml scalar to a string', () => { + const literal = connectorConfigObjectLiteral({ + location: 'us-central1', + connector: true, + service: 'my-service', + } as unknown as ConnectorConfig); + const evaluated = evalObjectLiteral(literal); + expect(evaluated.connector).toBe('true'); + expect(typeof evaluated.connector).toBe('string'); + }); + +}); + +describe('isValidPackageSpecifier', () => { + + it('accepts a normal scoped package specifier', () => { + expect(isValidPackageSpecifier('@my-org/my-connector')).toBeTrue(); + }); + + it('rejects a value containing a double quote', () => { + expect(isValidPackageSpecifier('foo"; console.log("INJECTED"); const _z="x')).toBeFalse(); + }); + + it('rejects a value containing a backslash', () => { + expect(isValidPackageSpecifier('foo\\bar')).toBeFalse(); + }); + + it('rejects a value containing a newline', () => { + expect(isValidPackageSpecifier('foo\nbar')).toBeFalse(); + }); + +}); + +describe('resolveDataConnectProviderConfig', () => { + + it('resolves to the connectorConfig object literal when there is no package', () => { + const resolution = resolveDataConnectProviderConfig({ + connectorYaml: { connectorId: 'my-connector' }, + connectorConfig: { + location: 'us-central1', + connector: 'my-connector', + service: 'my-service', + }, + }); + expect(resolution.kind).toBe('literal'); + }); + + it('resolves to an external import when package is a valid specifier', () => { + const resolution = resolveDataConnectProviderConfig({ + connectorYaml: { connectorId: 'my-connector' }, + connectorConfig: { + location: 'us-central1', + connector: 'my-connector', + service: 'my-service', + }, + package: '@my-org/my-connector', + }); + expect(resolution).toEqual({ kind: 'external', package: '@my-org/my-connector' }); + }); + + it('falls back to the connectorConfig literal when package is not a valid specifier', () => { + const resolution = resolveDataConnectProviderConfig({ + connectorYaml: { connectorId: 'my-connector' }, + connectorConfig: { + location: 'us-central1', + connector: 'my-connector', + service: 'my-service', + }, + package: 'foo"; console.log("INJECTED")', + }); + expect(resolution.kind).toBe('literal'); + }); + + it('does not throw and falls back to an empty literal when there is no javascriptSdk config', () => { + expect(() => resolveDataConnectProviderConfig({ + connectorYaml: { connectorId: 'my-connector' }, + })).not.toThrow(); + expect(resolveDataConnectProviderConfig({ + connectorYaml: { connectorId: 'my-connector' }, + })).toEqual({ kind: 'literal', literal: '{}' }); + }); + + it('does not throw and falls back to an empty literal when config is null', () => { + expect(() => resolveDataConnectProviderConfig(null)).not.toThrow(); + expect(resolveDataConnectProviderConfig(null)).toEqual({ kind: 'literal', literal: '{}' }); + }); + +}); diff --git a/src/schematics/utils.ts b/src/schematics/utils.ts index 00294167b..94721e753 100644 --- a/src/schematics/utils.ts +++ b/src/schematics/utils.ts @@ -7,10 +7,10 @@ import { Tree, chain, } from "@angular-devkit/schematics"; -import { NodePackageInstallTask } from "@angular-devkit/schematics/tasks"; +import { NodePackageInstallTask } from "@angular-devkit/schematics/tasks/index.js"; import { addRootProvider } from "@schematics/angular/utility"; import { parse } from "yaml"; -import { overwriteIfExists, safeReadJSON, stringifyFormatted } from "./common"; +import { overwriteIfExists, safeReadJSON, stringifyFormatted } from "./common.js"; import { ConnectorConfig, ConnectorYaml, @@ -148,6 +148,38 @@ ${addZonePatch ? "import 'zone.js/dist/zone-patch-rxjs';" : ""}` return host; } +// config.package and the connectorConfig values below come from the project's own +// dataconnect.yaml/connector.yaml, not from a trusted schema. isValidPackageSpecifier +// rejects the characters that would let a value break out of the double-quoted string +// literal it's interpolated into; it does not otherwise validate that the value is a +// well-formed module specifier. +const PACKAGE_SPECIFIER_PATTERN = /^[^'"\\\n\r]+$/; +export function isValidPackageSpecifier(pkg: string): boolean { + return PACKAGE_SPECIFIER_PATTERN.test(pkg); +} + +export function connectorConfigObjectLiteral(connectorConfig: ConnectorConfig): string { + return `{${(Object.keys(connectorConfig) as (keyof ConnectorConfig)[]).map( + (key) => `${key}: ${JSON.stringify(String(connectorConfig[key]))}` + ).join(',')}}`; +} + +export type DataConnectProviderConfigResolution = + | { kind: "external"; package: string } + | { kind: "literal"; literal: string }; + +export function resolveDataConnectProviderConfig( + config: DataConnectConnectorConfig | null | undefined +): DataConnectProviderConfigResolution { + if (config?.package && isValidPackageSpecifier(config.package)) { + return { kind: "external", package: config.package }; + } + if (config?.connectorConfig) { + return { kind: "literal", literal: connectorConfigObjectLiteral(config.connectorConfig) }; + } + return { kind: "literal", literal: "{}" }; +} + export function featureToRules( features: FEATURES[], projectName: string, @@ -216,30 +248,21 @@ export function featureToRules( case FEATURES.DataConnect: return addRootProvider(projectName, ({ code, external }) => { external("getDataConnect", "@angular/fire/data-connect"); - let configAsStr = "{}"; - const config = dataConnectConfig; + const resolution = resolveDataConnectProviderConfig(dataConnectConfig); + const configAsStr = resolution.kind === "external" + ? external("connectorConfig", resolution.package) + : resolution.literal; + let angularConfig: undefined | string; - if (config) { - // config.package and the connectorConfig values below come from the project's own - // dataconnect.yaml/connector.yaml, not from a trusted schema — reject anything that - // isn't a plausible package specifier before using it as a module name. - if (config.package && /^[^'"\\\n\r]+$/.test(config.package)) { - configAsStr = external("connectorConfig", config.package); - } else if (config.connectorConfig) { - configAsStr = `{${Object.keys(config.connectorConfig as ConnectorConfig).map( - (key) => `${key}: ${JSON.stringify((config.connectorConfig as ConnectorConfig)[key])}` - ).join(',')}}`; - } - if (config.angular) { - angularConfig = `, ${external( - "provideTanStackQuery", - "@tanstack/angular-query-experimental" - )}(new ${external( - "QueryClient", - "@tanstack/angular-query-experimental" - )}())`; - } + if (dataConnectConfig?.angular) { + angularConfig = `, ${external( + "provideTanStackQuery", + "@tanstack/angular-query-experimental" + )}(new ${external( + "QueryClient", + "@tanstack/angular-query-experimental" + )}())`; } return code`${external( "provideDataConnect",