fix: normalize legacy configuration safely

.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.

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`.
 

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<Record<string, unknown>> = {}
-    if (isRecord(config)) {
-      settings = config
-    } else if (config !== undefined && config !== null) {
+    try {
+      let settings: Readonly<Record<string, unknown>> = {}
+      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,15 +361,40 @@ function normalizeTags(value: unknown, reporter: Reporter): ReadonlyArray<string
   if (typeof value === 'string') {
     return [value]
   }
-  if (Array.isArray(value) && value.every((item): item is string => 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)
   }
   return undefined
 }
 
+function getArrayLength(value: ReadonlyArray<unknown>): number | undefined {
+  const length: unknown = value.length
+  return typeof length === 'number' && Number.isSafeInteger(length) && length >= 0
+    ? length
+    : undefined
+}
+
+function cloneConfigurationRecord(
+  value: Readonly<Record<string, unknown>> | undefined,
+): Record<string, unknown> | 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

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()