fix(pen-core): reject invalid enum values in 5 more element builders
Sweep follow-up to 113bd55a — same defensive pattern (reject unknown enum strings at the entry boundary) applied to every other builder that indexed a Record<EnumLiteral, T> with a value sourced from raw JSON args. Builders + enums covered: - buildTag — TagTone (default | accent | success | warning | error) - buildCallout — CalloutTone (info | success | warning | danger | note) - buildActivityLog — tone (info | success | warning | danger | neutral) - buildInviteRow — InviteStatus (pending | expired | accepted) - buildMemberRow — trailing.tone for status_dot (online | busy | away | offline). role_badge / menu variants skip the check (no tone field) Same failure mode each one fixed: when a model invents an out-of-enum string (gpt-5.4 did this with `level: "caption"` in ab-v4), the lookup `TONES[bad]` / `STATUS_TONE[bad]` returned undefined, the next property access crashed mid-batch with a cryptic `undefined is not an object`, and the surrounding dispatch loop dropped every remaining tag (until df33e937 + 07639f6d landed the per-shape continuation + partial-doc scoring earlier today). With validation in place, a bad enum becomes a clean per-shape error message + the rest of the batch still applies. 13 new edge-case tests cover throw on bad input + valid path on every enum value + omitted-default for each builder. 3785 vitest pass, format clean, tsc silent. Builders not touched: heading.ts (already done in 113bd55a). Builders that don't fit this pattern (no enum→Record lookup of a user-controlled string): everything else surveyed via grep on `Record<.*Tone|Status|Level|Mode|Kind`.
This commit is contained in:
parent
b4b931211c
commit
bafbdfaeef
|
|
@ -1,16 +1,21 @@
|
|||
import { describe, it, expect } from 'vitest';
|
||||
import {
|
||||
buildActivityLog,
|
||||
buildBadge,
|
||||
buildBodyText,
|
||||
buildCalendarGrid,
|
||||
buildCallout,
|
||||
buildCardRow,
|
||||
buildChartBars,
|
||||
buildEmptyState,
|
||||
buildHeading,
|
||||
buildInviteRow,
|
||||
buildKbd,
|
||||
buildMemberRow,
|
||||
buildPrice,
|
||||
buildRatingStars,
|
||||
buildStepper,
|
||||
buildTag,
|
||||
buildTimeline,
|
||||
} from '../element-builders/index.js';
|
||||
|
||||
|
|
@ -172,3 +177,97 @@ describe('buildHeading — invalid level rejection', () => {
|
|||
expect(() => buildHeading({ content: 'no level' })).not.toThrow();
|
||||
});
|
||||
});
|
||||
|
||||
// Same defensive pattern as buildHeading — the MCP tool defs declare
|
||||
// tone / status enums, but ab-corpus + production both call these
|
||||
// builders with raw JSON args, bypassing schema validation. A model
|
||||
// inventing an out-of-enum string used to crash the dispatch with
|
||||
// `undefined is not an object`. Each builder now rejects at the entry
|
||||
// boundary so the surrounding batch keeps going through the rest.
|
||||
describe('buildTag — invalid tone rejection', () => {
|
||||
it('throws for an unknown tone', () => {
|
||||
expect(() => buildTag({ label: 'Active', tone: 'critical' as never })).toThrow(
|
||||
/invalid tone "critical"/,
|
||||
);
|
||||
});
|
||||
it('accepts every valid tone + omitted', () => {
|
||||
for (const tone of ['default', 'accent', 'success', 'warning', 'error'] as const) {
|
||||
expect(() => buildTag({ label: 'x', tone })).not.toThrow();
|
||||
}
|
||||
expect(() => buildTag({ label: 'x' })).not.toThrow();
|
||||
});
|
||||
});
|
||||
|
||||
describe('buildCallout — invalid tone rejection', () => {
|
||||
it('throws for an unknown tone', () => {
|
||||
expect(() => buildCallout({ body: 'x', tone: 'fancy' as never })).toThrow(
|
||||
/invalid tone "fancy"/,
|
||||
);
|
||||
});
|
||||
it('accepts every valid tone + omitted', () => {
|
||||
for (const tone of ['info', 'success', 'warning', 'danger', 'note'] as const) {
|
||||
expect(() => buildCallout({ body: 'x', tone })).not.toThrow();
|
||||
}
|
||||
expect(() => buildCallout({ body: 'x' })).not.toThrow();
|
||||
});
|
||||
});
|
||||
|
||||
describe('buildActivityLog — invalid tone rejection', () => {
|
||||
it('throws for an unknown tone', () => {
|
||||
expect(() =>
|
||||
buildActivityLog({
|
||||
actor: 'Sarah',
|
||||
action: 'merged PR',
|
||||
timestamp: '2h ago',
|
||||
tone: 'critical' as never,
|
||||
}),
|
||||
).toThrow(/invalid tone "critical"/);
|
||||
});
|
||||
it('accepts every valid tone + omitted', () => {
|
||||
const base = { actor: 'a', action: 'b', timestamp: '1m' };
|
||||
for (const tone of ['info', 'success', 'warning', 'danger', 'neutral'] as const) {
|
||||
expect(() => buildActivityLog({ ...base, tone })).not.toThrow();
|
||||
}
|
||||
expect(() => buildActivityLog(base)).not.toThrow();
|
||||
});
|
||||
});
|
||||
|
||||
describe('buildInviteRow — invalid status rejection', () => {
|
||||
it('throws for an unknown status', () => {
|
||||
expect(() => buildInviteRow({ email: 'a@b.com', status: 'revoked' as never })).toThrow(
|
||||
/invalid status "revoked"/,
|
||||
);
|
||||
});
|
||||
it('accepts every valid status + omitted', () => {
|
||||
for (const status of ['pending', 'expired', 'accepted'] as const) {
|
||||
expect(() => buildInviteRow({ email: 'x@y.com', status })).not.toThrow();
|
||||
}
|
||||
expect(() => buildInviteRow({ email: 'x@y.com' })).not.toThrow();
|
||||
});
|
||||
});
|
||||
|
||||
describe('buildMemberRow — invalid trailing.tone rejection (status_dot variant)', () => {
|
||||
it('throws when trailing.kind=status_dot has an unknown tone', () => {
|
||||
expect(() =>
|
||||
buildMemberRow({
|
||||
name: 'Sarah',
|
||||
trailing: { kind: 'status_dot', tone: 'unknown' as never },
|
||||
}),
|
||||
).toThrow(/invalid trailing\.tone "unknown"/);
|
||||
});
|
||||
it('accepts every valid status tone', () => {
|
||||
for (const tone of ['online', 'busy', 'away', 'offline'] as const) {
|
||||
expect(() =>
|
||||
buildMemberRow({ name: 'x', trailing: { kind: 'status_dot', tone } }),
|
||||
).not.toThrow();
|
||||
}
|
||||
// omitted tone defaults to 'online'
|
||||
expect(() => buildMemberRow({ name: 'x', trailing: { kind: 'status_dot' } })).not.toThrow();
|
||||
});
|
||||
it('does not validate when trailing is role_badge or menu (no tone field)', () => {
|
||||
expect(() =>
|
||||
buildMemberRow({ name: 'x', trailing: { kind: 'role_badge', value: 'Owner' } }),
|
||||
).not.toThrow();
|
||||
expect(() => buildMemberRow({ name: 'x', trailing: { kind: 'menu' } })).not.toThrow();
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -13,6 +13,14 @@ export interface ActivityLogParams {
|
|||
tone?: 'info' | 'success' | 'warning' | 'danger' | 'neutral';
|
||||
}
|
||||
|
||||
const VALID_ACTIVITY_LOG_TONES = new Set<string>([
|
||||
'info',
|
||||
'success',
|
||||
'warning',
|
||||
'danger',
|
||||
'neutral',
|
||||
]);
|
||||
|
||||
const ACTOR_FG = '#0F172A';
|
||||
const ACTION_FG = '#475569';
|
||||
const TS_FG = '#94A3B8';
|
||||
|
|
@ -46,7 +54,13 @@ const TONE_FG: Record<NonNullable<ActivityLogParams['tone']>, string> = {
|
|||
* "审计日志条目", "活动记录".
|
||||
*/
|
||||
export function buildActivityLog(params: ActivityLogParams): ElementTree {
|
||||
const tone = params.tone ?? 'info';
|
||||
const requestedTone = (params.tone ?? 'info') as string;
|
||||
if (!VALID_ACTIVITY_LOG_TONES.has(requestedTone)) {
|
||||
throw new Error(
|
||||
`add_activity_log_v0: invalid tone "${requestedTone}"; expected one of: info, success, warning, danger, neutral`,
|
||||
);
|
||||
}
|
||||
const tone = requestedTone as NonNullable<ActivityLogParams['tone']>;
|
||||
const rowChildren: ElementTree[] = [];
|
||||
|
||||
if (params.icon) {
|
||||
|
|
|
|||
|
|
@ -2,6 +2,8 @@ import type { ElementTree } from './helpers.js';
|
|||
|
||||
export type CalloutTone = 'info' | 'success' | 'warning' | 'danger' | 'note';
|
||||
|
||||
const VALID_CALLOUT_TONES = new Set<string>(['info', 'success', 'warning', 'danger', 'note']);
|
||||
|
||||
export interface CalloutParams {
|
||||
/** Body text. Required. */
|
||||
body: string;
|
||||
|
|
@ -33,7 +35,13 @@ const TONES: Record<CalloutTone, ToneSpec> = {
|
|||
* onboarding notes, "did you know" panels.
|
||||
*/
|
||||
export function buildCallout(params: CalloutParams): ElementTree {
|
||||
const tone = TONES[params.tone ?? 'note'];
|
||||
const requestedTone = (params.tone ?? 'note') as string;
|
||||
if (!VALID_CALLOUT_TONES.has(requestedTone)) {
|
||||
throw new Error(
|
||||
`add_callout_v0: invalid tone "${requestedTone}"; expected one of: info, success, warning, danger, note`,
|
||||
);
|
||||
}
|
||||
const tone = TONES[requestedTone as CalloutTone];
|
||||
const stackChildren: ElementTree[] = [];
|
||||
if (params.title) {
|
||||
stackChildren.push({
|
||||
|
|
|
|||
|
|
@ -2,6 +2,8 @@ import type { ElementTree } from './helpers.js';
|
|||
|
||||
export type InviteStatus = 'pending' | 'expired' | 'accepted';
|
||||
|
||||
const VALID_INVITE_STATUSES = new Set<string>(['pending', 'expired', 'accepted']);
|
||||
|
||||
export interface InviteRowParams {
|
||||
/** Invitee email (e.g. "sarah@acme.com"). */
|
||||
email: string;
|
||||
|
|
@ -33,7 +35,13 @@ const ACTION_FG = '#2563EB';
|
|||
* invite row", "邀请列表行", "待接受邀请".
|
||||
*/
|
||||
export function buildInviteRow(params: InviteRowParams): ElementTree {
|
||||
const status = params.status ?? 'pending';
|
||||
const requestedStatus = (params.status ?? 'pending') as string;
|
||||
if (!VALID_INVITE_STATUSES.has(requestedStatus)) {
|
||||
throw new Error(
|
||||
`add_invite_row_v0: invalid status "${requestedStatus}"; expected one of: pending, expired, accepted`,
|
||||
);
|
||||
}
|
||||
const status = requestedStatus as InviteStatus;
|
||||
const tone = STATUS_TONE[status];
|
||||
const actionLabel = params.action_label ?? 'Resend';
|
||||
const initial = (params.email.charAt(0) ?? '?').toUpperCase();
|
||||
|
|
|
|||
|
|
@ -26,6 +26,8 @@ const MENU_FG = '#94A3B8';
|
|||
|
||||
type StatusTone = 'online' | 'busy' | 'away' | 'offline';
|
||||
|
||||
const VALID_STATUS_TONES = new Set<string>(['online', 'busy', 'away', 'offline']);
|
||||
|
||||
const STATUS_TONE_FILL: Record<StatusTone, string> = {
|
||||
online: '#10B981',
|
||||
busy: '#EF4444',
|
||||
|
|
@ -69,7 +71,13 @@ function buildTrailing(t: MemberRowTrailing): ElementTree {
|
|||
fill: [{ type: 'solid', color: MENU_FG }],
|
||||
};
|
||||
}
|
||||
const tone = t.tone ?? 'online';
|
||||
const requestedTone = (t.tone ?? 'online') as string;
|
||||
if (!VALID_STATUS_TONES.has(requestedTone)) {
|
||||
throw new Error(
|
||||
`add_member_row_v0: invalid trailing.tone "${requestedTone}"; expected one of: online, busy, away, offline`,
|
||||
);
|
||||
}
|
||||
const tone = requestedTone as StatusTone;
|
||||
return {
|
||||
type: 'frame',
|
||||
name: 'Status Dot',
|
||||
|
|
|
|||
|
|
@ -2,6 +2,8 @@ import type { ElementTree } from './helpers.js';
|
|||
|
||||
export type TagTone = 'default' | 'accent' | 'success' | 'warning' | 'error';
|
||||
|
||||
const VALID_TAG_TONES = new Set<string>(['default', 'accent', 'success', 'warning', 'error']);
|
||||
|
||||
export interface TagParams {
|
||||
label: string;
|
||||
/** Render the trailing × close icon. Default true — that's what makes
|
||||
|
|
@ -36,7 +38,13 @@ const TONES: Record<TagTone, ToneSpec> = {
|
|||
* clicking the ×: "Status: Active ×", "Plan: Pro ×", "Tag: design ×".
|
||||
*/
|
||||
export function buildTag(params: TagParams): ElementTree {
|
||||
const tone = TONES[params.tone ?? 'default'];
|
||||
const requestedTone = (params.tone ?? 'default') as string;
|
||||
if (!VALID_TAG_TONES.has(requestedTone)) {
|
||||
throw new Error(
|
||||
`add_tag_v0: invalid tone "${requestedTone}"; expected one of: default, accent, success, warning, error`,
|
||||
);
|
||||
}
|
||||
const tone = TONES[requestedTone as TagTone];
|
||||
const removable = params.removable ?? true;
|
||||
const children: ElementTree[] = [
|
||||
{
|
||||
|
|
|
|||
Loading…
Reference in a new issue