From 088af11b29bdf480c60dda103f35c17cee90cc2f Mon Sep 17 00:00:00 2001 From: usercao Date: Sat, 31 Oct 2026 01:07:44 +0800 Subject: [PATCH] fix: normalize legacy configuration safely --- .changeset/steady-legacy-configuration.md | 7 ++ docs/architecture.md | 3 +- src/configuration/plugin-configuration.ts | 101 +++++++++++++++----- test/unit/plugin-configuration.test.ts | 110 ++++++++++++++++++++++ 4 files changed, 194 insertions(+), 27 deletions(-) create mode 100644 .changeset/steady-legacy-configuration.md diff --git a/.changeset/steady-legacy-configuration.md b/.changeset/steady-legacy-configuration.md new file mode 100644 index 0000000..da62bf1 --- /dev/null +++ b/.changeset/steady-legacy-configuration.md @@ -0,0 +1,7 @@ +--- +'@styled/typescript-styled-plugin': patch +--- + +Keep normalized tag arrays and top-level Emmet options independent of caller mutations, and preserve unknown lint keys as own properties. + +Reject invalid array lengths and fall back to defaults when direct JavaScript API callers supply configuration values that throw while being read. diff --git a/docs/architecture.md b/docs/architecture.md index 1e777f5..9b98171 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -143,13 +143,14 @@ Spec for how the plugin turns a tsserver request inside a tagged template into a ## Configuration -- Source: `compilerOptions.plugins[]` entry at startup, then `configurePlugin` requests. `PluginConfigurationManager` (`src/configuration/plugin-configuration.ts`) normalizes any input shape and never throws, including when its logger throws. +- Source: `compilerOptions.plugins[]` entry at startup, then `configurePlugin` requests. The public `PluginConfigurationManager` (`src/configuration/plugin-configuration.ts`) also accepts JavaScript values from direct API callers. If reading or normalizing a configuration throws, the manager resets the whole configuration to defaults, logs the problem, and notifies its listeners once. Logger failures are ignored; listener callbacks run after normalization and outside its recovery guard. - Normalization per field: - `tags`: a list of strings, or one string as a one-item list. Anything else is rejected. - `validate`: a boolean, or the string `"true"` or `"false"`. Anything else is rejected. - `lint`, `emmet`: an object; anything else is rejected. Inside `lint`, a level setting (every key of `StyledPluginLintConfiguration` except `validProperties`) takes `"ignore"`, `"warning"`, or `"error"`, plus the aliases `"off"` (as `"ignore"`) and `"warn"` (as `"warning"`), each in any letter case (`"Error"` as `"error"`); another value is rejected and the key dropped, so a key with a plugin default (`defaultConfiguration.lint`, such as `emptyRules`) keeps that default, and any other key gets the CSS language service's own default for that rule. The log message names the plugin default where one applies. `validProperties` takes a list, keeping its string entries and rejecting the rest, or one string as a one-item list. Unknown `lint` keys and every `emmet` key pass through unchanged. - A rejected value falls back to its default and logs one message to the tsserver log naming the setting, the accepted form, and the value received. An accepted alias or letter-case variant logs nothing, and neither does an absent field or a `null` or `undefined` top-level configuration. A field set to `null` is rejected. - A top-level key that is not one of the fields above is ignored and logs one message naming it and those fields, plus a "Did you mean" suggestion: the setting nearest by edit distance with letter case ignored, when that distance is at most 40% of the key's length (at least 1), so `tag` suggests `tags` and `validation` suggests `validate`. The keys tsserver adds to the configuration it passes (`name` and `global`, docs/tsserver-host.md) log nothing. +- Accepted tag arrays are copied before listeners run. Lint and Emmet records are copied one level deep, with nested values shared; `lint.validProperties` is normalized into a new array. Unknown lint keys, including `__proto__`, remain own data properties without changing the record's prototype. An array with a reported length that is not a nonnegative safe integer is rejected before normalization iterates its entries; describing a rejected value for the log can still read its properties. - Each update replaces the whole configuration (fields absent from an update return to defaults), so a `configurePlugin` request replaces the `tsconfig.json` settings rather than merging into them, as in 1.0.1. - Defaults (tags, lint levels) live in `PluginConfigurationManager.defaultConfiguration`; user-facing documentation in `docs/usage.md`. diff --git a/src/configuration/plugin-configuration.ts b/src/configuration/plugin-configuration.ts index 470a3e1..38d10cf 100644 --- a/src/configuration/plugin-configuration.ts +++ b/src/configuration/plugin-configuration.ts @@ -95,32 +95,41 @@ export class PluginConfigurationManager { public updateFromPluginConfig(config: unknown, logger?: ConfigurationLogger) { const reporter = createReporter(logger) const defaults = PluginConfigurationManager.defaultConfiguration - let settings: Readonly> = {} - if (isRecord(config)) { - settings = config - } else if (config !== undefined && config !== null) { + try { + let settings: Readonly> = {} + if (isRecord(config)) { + settings = config + } else if (config !== undefined && config !== null) { + reporter.report( + `Ignored the plugin configuration: it accepts an object of plugin settings; received ${describeValue(config)}. The defaults apply.`, + ) + } + + this.configuration = { + emmet: + cloneConfigurationRecord( + normalizeRecord(settings.emmet, 'emmet', 'an object of Emmet options', reporter), + ) ?? defaults.emmet, + lint: + cloneConfigurationRecord({ + ...defaults.lint, + ...normalizeLint( + normalizeRecord(settings.lint, 'lint', 'an object of lint settings', reporter), + defaults.lint, + reporter, + ), + }) ?? defaults.lint, + tags: normalizeTags(settings.tags, reporter) ?? defaults.tags, + validate: normalizeBoolean(settings.validate, 'validate', reporter) ?? defaults.validate, + } + reportUnknownSettings(settings, reporter) + } catch { + this.configuration = defaults reporter.report( - `Ignored the plugin configuration: it accepts an object of plugin settings; received ${describeValue(config)}. The defaults apply.`, + 'Ignored the plugin configuration because it could not be read safely. The defaults apply.', ) } - this.configuration = { - emmet: - normalizeRecord(settings.emmet, 'emmet', 'an object of Emmet options', reporter) ?? - defaults.emmet, - lint: { - ...defaults.lint, - ...normalizeLint( - normalizeRecord(settings.lint, 'lint', 'an object of lint settings', reporter), - defaults.lint, - reporter, - ), - }, - tags: normalizeTags(settings.tags, reporter) ?? defaults.tags, - validate: normalizeBoolean(settings.validate, 'validate', reporter) ?? defaults.validate, - } - reportUnknownSettings(settings, reporter) - for (const listener of this.updateListeners) { listener() } @@ -274,7 +283,12 @@ function normalizeLint( continue } if (!hasOwn(LINT_LEVEL_SETTINGS, name)) { - normalized[name] = value + Object.defineProperty(normalized, name, { + configurable: true, + enumerable: true, + value, + writable: true, + }) continue } const level = typeof value === 'string' ? toLintLevel(value) : undefined @@ -318,7 +332,17 @@ function normalizeValidProperties( } const names: string[] = [] const rejected: unknown[] = [] - for (const item of value) { + const itemCount = getArrayLength(value) + if (itemCount === undefined) { + reporter.reportRejected( + `lint.${VALID_PROPERTIES_SETTING}`, + 'a list of property names, such as ["margin-vertical"]', + value, + ) + return undefined + } + for (let index = 0; index < itemCount; index++) { + const item: unknown = value[index] if (typeof item === 'string') { names.push(item) } else { @@ -337,8 +361,20 @@ function normalizeTags(value: unknown, reporter: Reporter): ReadonlyArray typeof item === 'string')) { - return value + if (Array.isArray(value)) { + const tagCount = getArrayLength(value) + if (tagCount !== undefined) { + const tags: string[] = [] + for (let index = 0; index < tagCount; index++) { + const item: unknown = value[index] + if (typeof item !== 'string') { + reporter.reportRejected('tags', 'a list of tag names, such as ["styled", "css"]', value) + return undefined + } + tags.push(item) + } + return tags + } } if (value !== undefined) { reporter.reportRejected('tags', 'a list of tag names, such as ["styled", "css"]', value) @@ -346,6 +382,19 @@ function normalizeTags(value: unknown, reporter: Reporter): ReadonlyArray): number | undefined { + const length: unknown = value.length + return typeof length === 'number' && Number.isSafeInteger(length) && length >= 0 + ? length + : undefined +} + +function cloneConfigurationRecord( + value: Readonly> | undefined, +): Record | undefined { + return value === undefined ? undefined : Object.fromEntries(Object.entries(value)) +} + function normalizeBoolean(value: unknown, name: string, reporter: Reporter): boolean | undefined { if (typeof value === 'boolean') { return value diff --git a/test/unit/plugin-configuration.test.ts b/test/unit/plugin-configuration.test.ts index f29fc26..fbf7c55 100644 --- a/test/unit/plugin-configuration.test.ts +++ b/test/unit/plugin-configuration.test.ts @@ -199,6 +199,116 @@ describe('PluginConfigurationManager', () => { ]) }) + it('should copy tag arrays before notifying listeners', () => { + const manager = new PluginConfigurationManager() + const tags = ['styled'] + manager.onUpdatedConfig(() => tags.push('later')) + + manager.updateFromPluginConfig({ tags }) + + assert.deepEqual(manager.config.tags, ['styled']) + assert.deepEqual(tags, ['styled', 'later']) + }) + + it('should make a one-level copy of lint and Emmet configuration', () => { + const manager = new PluginConfigurationManager() + const preferences = { compact: true } + const validProperties = ['brand-tone'] + const emmet = { preferences, showExpandedAbbreviation: 'never' } + const lint = { unknownProperties: 'ignore', validProperties } + + manager.updateFromPluginConfig({ emmet, lint }) + emmet.showExpandedAbbreviation = 'always' + lint.unknownProperties = 'error' + validProperties.push('later') + + assert.deepEqual(manager.config.emmet, { preferences, showExpandedAbbreviation: 'never' }) + assert.strictEqual(manager.config.emmet.preferences, preferences) + assert.deepEqual(manager.config.lint, { + emptyRules: 'ignore', + unknownProperties: 'ignore', + validProperties: ['brand-tone'], + }) + }) + + it('should preserve __proto__ as an own lint key without changing the prototype', () => { + const manager = new PluginConfigurationManager() + const logger = createLogger() + const lint = JSON.parse('{"__proto__":{"unknownProperties":"ignore"},"zeroUnits":"error"}') + + manager.updateFromPluginConfig({ lint }, logger) + + assert.deepEqual(manager.config.lint, { + ['__proto__']: { unknownProperties: 'ignore' }, + emptyRules: 'ignore', + zeroUnits: 'error', + }) + assert.strictEqual(Object.getPrototypeOf(manager.config.lint), Object.prototype) + assert.deepEqual(logger.messages, []) + }) + + it.each(['tags', 'lint.validProperties'])( + 'should reject an array with an invalid length for %s', + (setting) => { + const manager = new PluginConfigurationManager() + const logger = createLogger() + const values = new Proxy(['styled'], { + get(target, key, receiver) { + if (key === Symbol.iterator) { + return target[Symbol.iterator].bind(target) + } + return key === 'length' ? '1' : Reflect.get(target, key, receiver) + }, + }) + + manager.updateFromPluginConfig( + setting === 'tags' + ? { tags: values, validate: false } + : { lint: { validProperties: values }, validate: false }, + logger, + ) + + assert.isTrue(manager.config.tags === new PluginConfigurationManager().config.tags) + assert.deepEqual(manager.config.lint, { emptyRules: 'ignore' }) + assert.isFalse(manager.config.validate) + assert.deepEqual(logger.messages, [ + `Ignored the plugin setting ${setting}: it accepts ${setting === 'tags' ? 'a list of tag names, such as ["styled", "css"]' : 'a list of property names, such as ["margin-vertical"]'}; received ["styled"]. The default applies.`, + ]) + }, + ) + + it.each(['getter', 'revoked proxy'])( + 'should reset to defaults and notify once when configuration contains a throwing %s', + (kind) => { + const manager = new PluginConfigurationManager() + const logger = createLogger() + manager.updateFromPluginConfig({ tags: ['previous'], validate: false }) + let notifications = 0 + manager.onUpdatedConfig(() => notifications++) + const revoked = Proxy.revocable({}, {}) + revoked.revoke() + const config = + kind === 'getter' + ? { + get tags() { + throw new Error('tags could not be read') + }, + } + : revoked.proxy + + assert.doesNotThrow(() => manager.updateFromPluginConfig(config, logger)) + + assert.deepEqual(manager.config, new PluginConfigurationManager().config) + assert.strictEqual(notifications, 1) + assert.deepEqual(logger.messages, [ + 'Ignored the plugin configuration because it could not be read safely. The defaults apply.', + ]) + manager.updateFromPluginConfig({ tags: ['next'] }) + assert.deepEqual(manager.config.tags, ['next']) + assert.strictEqual(notifications, 2) + }, + ) + it('should log a top-level configuration that is not an object', () => { const manager = new PluginConfigurationManager() const logger = createLogger()