diff --git a/src/constants/outputStyles.protoName.test.ts b/src/constants/outputStyles.protoName.test.ts new file mode 100644 index 0000000000..31bb8196f9 --- /dev/null +++ b/src/constants/outputStyles.protoName.test.ts @@ -0,0 +1,48 @@ +import { expect, test } from 'bun:test' + +import { resolveOutputStyle } from './outputStyles.js' + +// `settings.outputStyle` is a free-form `z.string()` with no enum, and the +// style maps are plain object literals, so the name reaches a bare index. The +// resolved value flows into the model-facing system prompt +// (`# Output Style: ${config.name}\n${config.prompt}`) and the output-style +// system reminder, so a non-config must resolve to null rather than be rendered. +const STYLES = { + default: { name: 'Default', prompt: 'be concise' }, + explanatory: { name: 'Explanatory', prompt: 'explain more' }, +} + +const PROTO_NAMES = [ + 'constructor', + '__proto__', + 'toString', + 'valueOf', + 'hasOwnProperty', + 'isPrototypeOf', +] + +test('resolves a real style by name', () => { + expect(resolveOutputStyle(STYLES, 'explanatory')).toEqual({ + name: 'Explanatory', + prompt: 'explain more', + }) +}) + +test('returns null for an unconfigured style name', () => { + expect(resolveOutputStyle(STYLES, 'nonexistent-style')).toBeNull() +}) + +test('returns null for Object.prototype member names', () => { + // Before the fix each of these resolved to an inherited member. `?? null` + // did not neutralize it — the Object constructor is not nullish — so + // `outputStyle: "constructor"` injected "# Output Style: Object\nundefined" + // into the system prompt instead of falling back to the default. + for (const name of PROTO_NAMES) { + expect(resolveOutputStyle(STYLES, name)).toBeNull() + } +}) + +test('returns null for an explicitly null entry', () => { + // The map type allows null values; those are not usable configs either. + expect(resolveOutputStyle({ broken: null }, 'broken')).toBeNull() +}) diff --git a/src/constants/outputStyles.ts b/src/constants/outputStyles.ts index e8f760aebc..69442d551c 100644 --- a/src/constants/outputStyles.ts +++ b/src/constants/outputStyles.ts @@ -207,7 +207,27 @@ export async function getOutputStyleConfig(): Promise const outputStyle = (settings?.outputStyle || DEFAULT_OUTPUT_STYLE_NAME) as string - return allStyles[outputStyle] ?? null + return resolveOutputStyle(allStyles, outputStyle) +} + +/** + * Look up a style by name, returning null for anything that is not a real, + * configured style. + * + * `settings.outputStyle` is a free-form string with no enum, and the style maps + * are plain objects, so a bare index resolves inherited Object.prototype + * members. A trailing `?? null` does not neutralize that — `styles.constructor` + * is the Object constructor, which is not nullish — so the "unknown style falls + * back to the default" contract is skipped and a function is handed on as if it + * were a config. + * + * exported for testing + */ +export function resolveOutputStyle( + styles: Record, + name: string, +): T | null { + return Object.hasOwn(styles, name) ? (styles[name] ?? null) : null } export function hasCustomOutputStyle(): boolean { diff --git a/src/utils/messages.ts b/src/utils/messages.ts index 23bdd82878..a4afc17bb1 100644 --- a/src/utils/messages.ts +++ b/src/utils/messages.ts @@ -24,7 +24,10 @@ import { sanitizeToolNameForAnalytics } from 'src/services/analytics/metadata.js import type { AgentId } from 'src/types/ids.js' import { companionIntroText } from '../buddy/prompt.js' import { NO_CONTENT_MESSAGE } from '../constants/messages.js' -import { OUTPUT_STYLE_CONFIG } from '../constants/outputStyles.js' +import { + OUTPUT_STYLE_CONFIG, + resolveOutputStyle, +} from '../constants/outputStyles.js' import { isAutoMemoryEnabled } from '../memdir/paths.js' import { checkStatsigFeatureGate_CACHED_MAY_BE_STALE, @@ -2751,10 +2754,15 @@ Read the team config to discover your teammates' names. Check the task list peri ]) } case 'output_style': { - const outputStyle = - OUTPUT_STYLE_CONFIG[ - attachment.style as keyof typeof OUTPUT_STYLE_CONFIG - ] + // Own-property lookup: OUTPUT_STYLE_CONFIG is a plain object and + // `attachment.style` carries the free-form settings value, so a bare + // index resolves inherited Object.prototype members. Those are truthy, so + // the guard below would pass and the reminder would announce a style that + // does not exist ("Object output style is active"). + const outputStyle = resolveOutputStyle( + OUTPUT_STYLE_CONFIG, + attachment.style, + ) if (!outputStyle) { return [] }