From 81957307b34a968c0279b1288e718ce8da81408d Mon Sep 17 00:00:00 2001 From: Jesse Turner Date: Thu, 18 Jun 2026 08:33:07 -0700 Subject: [PATCH 1/2] fix(policy): correct INSULTS content-filter enum and validate --form-filters MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CONTENT_FILTER_FILTERS listed 'INSULT' (singular); the service enum is 'INSULTS' (plural). The TUI offered the bad value and the non-interactive CLI accepted any string for --form-filters, so both only failed at `agentcore deploy` with an opaque CFN ValidationException. Fix the enum (also fixes the TUI, which derives its options from it) and add client-side validation of --form-filters against the per-category enums already defined in types.ts, turning a deploy-time failure into an immediate, clear error. Closes #1571 Constraint: Validation scoped to form-filter enums the TUI/CLI already offers, per maintainer guidance — not blanket validation of every flag Rejected: New per-category filter constants in PolicyPrimitive | duplicates the source of truth already in types.ts Confidence: high Scope-risk: narrow --- src/cli/primitives/PolicyPrimitive.ts | 20 +++++++++- .../policy/__tests__/synthesize-cedar.test.ts | 6 +++ .../screens/policy/__tests__/types.test.ts | 37 +++++++++++++++++++ src/cli/tui/screens/policy/types.ts | 21 ++++++++++- 4 files changed, 81 insertions(+), 3 deletions(-) create mode 100644 src/cli/tui/screens/policy/__tests__/types.test.ts diff --git a/src/cli/primitives/PolicyPrimitive.ts b/src/cli/primitives/PolicyPrimitive.ts index 950416ef8..c3be8a233 100644 --- a/src/cli/primitives/PolicyPrimitive.ts +++ b/src/cli/primitives/PolicyPrimitive.ts @@ -9,7 +9,14 @@ import type { RemovalPreview, SchemaChange } from '../operations/remove/types'; import { runCliCommand, withCommandRunTelemetry } from '../telemetry/cli-command-run.js'; import { PolicyValidationMode, standardize } from '../telemetry/schemas/common-shapes.js'; import { requireTTY } from '../tui/guards/tty'; -import { type PolicyEffect, authorizationPhaseForEffect, defaultDataPathForEffect } from '../tui/screens/policy/types'; +import { + FILTERS_BY_CATEGORY, + type GuardrailCategoryType, + type PolicyEffect, + authorizationPhaseForEffect, + defaultDataPathForEffect, + invalidFiltersForCategory, +} from '../tui/screens/policy/types'; import { BasePrimitive } from './BasePrimitive'; import { SOURCE_CODE_NOTE } from './constants'; import type { AddResult, AddScreenComponent, RemovableResource } from './types'; @@ -408,6 +415,15 @@ export class PolicyPrimitive extends BasePrimitive s.trim()); + const category = cliOptions.formCategory as GuardrailCategoryType; + const invalidFilters = invalidFiltersForCategory(category, filters); + if (invalidFilters.length > 0) { + throw new ValidationError( + `Invalid filter(s) for category '${category}': ${invalidFilters.join(', ')}. ` + + `Allowed: ${FILTERS_BY_CATEGORY[category].join(', ')}` + ); + } + let resolvedGatewayArn: string | undefined; let resolvedTargetName: string | undefined = cliOptions.target; if (cliOptions.gateway) { @@ -446,7 +462,7 @@ export class PolicyPrimitive extends BasePrimitive { expect(result).toContain('[context.input.prompt]'); }); + it('generates a policy with the INSULTS content filter (canonical plural name)', () => { + const form: GuardrailFormConfig = { ...baseForm, filters: ['INSULTS'] }; + const result = synthesizeCedar(form); + expect(result).toContain('["INSULTS"]'); + }); + it('generates permit policy with single filter using lessThanOrEqual', () => { const form: GuardrailFormConfig = { ...baseForm, effect: 'permit' }; const result = synthesizeCedar(form); diff --git a/src/cli/tui/screens/policy/__tests__/types.test.ts b/src/cli/tui/screens/policy/__tests__/types.test.ts new file mode 100644 index 000000000..725ca1db9 --- /dev/null +++ b/src/cli/tui/screens/policy/__tests__/types.test.ts @@ -0,0 +1,37 @@ +import { CONTENT_FILTER_FILTERS, FILTERS_BY_CATEGORY, invalidFiltersForCategory } from '../types.js'; +import { describe, expect, it } from 'vitest'; + +describe('CONTENT_FILTER_FILTERS', () => { + it('uses the canonical plural INSULTS, not the singular INSULT', () => { + expect(CONTENT_FILTER_FILTERS).toContain('INSULTS'); + expect(CONTENT_FILTER_FILTERS).not.toContain('INSULT'); + }); +}); + +describe('invalidFiltersForCategory', () => { + it('returns an empty array when every filter is valid for the category', () => { + expect(invalidFiltersForCategory('contentFilter', ['VIOLENCE', 'INSULTS'])).toEqual([]); + expect(invalidFiltersForCategory('promptAttack', ['JAILBREAK'])).toEqual([]); + expect(invalidFiltersForCategory('sensitiveInformation', ['EMAIL', 'PHONE'])).toEqual([]); + }); + + it('rejects an unknown filter value', () => { + expect(invalidFiltersForCategory('contentFilter', ['VIOLENCE', 'NOTAREAL'])).toEqual(['NOTAREAL']); + }); + + it('rejects the legacy singular INSULT (regression guard for #1571)', () => { + expect(invalidFiltersForCategory('contentFilter', ['INSULT'])).toEqual(['INSULT']); + expect(invalidFiltersForCategory('contentFilter', ['INSULTS'])).toEqual([]); + }); + + it('rejects a filter from a different category', () => { + // JAILBREAK is valid for promptAttack but not for contentFilter + expect(invalidFiltersForCategory('contentFilter', ['JAILBREAK'])).toEqual(['JAILBREAK']); + }); + + it('exposes the allowed filters per category', () => { + expect(FILTERS_BY_CATEGORY.contentFilter).toBe(CONTENT_FILTER_FILTERS); + expect(FILTERS_BY_CATEGORY.promptAttack.length).toBeGreaterThan(0); + expect(FILTERS_BY_CATEGORY.sensitiveInformation.length).toBeGreaterThan(0); + }); +}); diff --git a/src/cli/tui/screens/policy/types.ts b/src/cli/tui/screens/policy/types.ts index b1bceaf5b..7b94045c7 100644 --- a/src/cli/tui/screens/policy/types.ts +++ b/src/cli/tui/screens/policy/types.ts @@ -12,7 +12,7 @@ export type PolicySourceMethod = 'file' | 'inline' | 'generate' | 'form'; export type GuardrailCategoryType = 'contentFilter' | 'promptAttack' | 'sensitiveInformation'; -export const CONTENT_FILTER_FILTERS = ['VIOLENCE', 'HATE', 'SEXUAL', 'MISCONDUCT', 'INSULT'] as const; +export const CONTENT_FILTER_FILTERS = ['VIOLENCE', 'HATE', 'SEXUAL', 'MISCONDUCT', 'INSULTS'] as const; export type ContentFilterCategory = (typeof CONTENT_FILTER_FILTERS)[number]; export const PROMPT_ATTACK_FILTERS = ['JAILBREAK', 'PROMPT_INJECTION', 'PROMPT_LEAKAGE'] as const; @@ -81,6 +81,25 @@ export const GUARDRAIL_CATEGORY_OPTIONS: { }, ]; +/** + * Valid filters per guardrail category. Single source of truth for both the TUI options + * and client-side validation of the non-interactive `--form-filters` flag. + */ +export const FILTERS_BY_CATEGORY: Record = { + contentFilter: CONTENT_FILTER_FILTERS, + promptAttack: PROMPT_ATTACK_FILTERS, + sensitiveInformation: SENSITIVE_INFO_FILTERS, +}; + +/** + * Returns the filter values that are not valid for the given category. + * An empty array means every supplied filter is valid. + */ +export function invalidFiltersForCategory(category: GuardrailCategoryType, filters: string[]): string[] { + const allowed = FILTERS_BY_CATEGORY[category]; + return filters.filter(f => !allowed.includes(f)); +} + export type PolicyEffect = 'permit' | 'forbid' | 'suppressOutput'; /** From a4dd5bac053318138414247146988eedf54f8535 Mon Sep 17 00:00:00 2001 From: Jesse Turner Date: Thu, 18 Jun 2026 08:40:53 -0700 Subject: [PATCH 2/2] test(policy): trim redundant filter-validation tests Drop three tests that padded coverage without exercising distinct behavior: the constant-literal INSULTS assertion (already covered by the regression guard through the helper), the cross-category case (same code path as the unknown-filter test), and the FILTERS_BY_CATEGORY wiring assertion (structure, not behavior). Keep happy-path, unknown-filter, and the #1571 regression guard. Confidence: high Scope-risk: narrow --- .../screens/policy/__tests__/types.test.ts | 20 +------------------ 1 file changed, 1 insertion(+), 19 deletions(-) diff --git a/src/cli/tui/screens/policy/__tests__/types.test.ts b/src/cli/tui/screens/policy/__tests__/types.test.ts index 725ca1db9..28cd47ca1 100644 --- a/src/cli/tui/screens/policy/__tests__/types.test.ts +++ b/src/cli/tui/screens/policy/__tests__/types.test.ts @@ -1,13 +1,6 @@ -import { CONTENT_FILTER_FILTERS, FILTERS_BY_CATEGORY, invalidFiltersForCategory } from '../types.js'; +import { invalidFiltersForCategory } from '../types.js'; import { describe, expect, it } from 'vitest'; -describe('CONTENT_FILTER_FILTERS', () => { - it('uses the canonical plural INSULTS, not the singular INSULT', () => { - expect(CONTENT_FILTER_FILTERS).toContain('INSULTS'); - expect(CONTENT_FILTER_FILTERS).not.toContain('INSULT'); - }); -}); - describe('invalidFiltersForCategory', () => { it('returns an empty array when every filter is valid for the category', () => { expect(invalidFiltersForCategory('contentFilter', ['VIOLENCE', 'INSULTS'])).toEqual([]); @@ -23,15 +16,4 @@ describe('invalidFiltersForCategory', () => { expect(invalidFiltersForCategory('contentFilter', ['INSULT'])).toEqual(['INSULT']); expect(invalidFiltersForCategory('contentFilter', ['INSULTS'])).toEqual([]); }); - - it('rejects a filter from a different category', () => { - // JAILBREAK is valid for promptAttack but not for contentFilter - expect(invalidFiltersForCategory('contentFilter', ['JAILBREAK'])).toEqual(['JAILBREAK']); - }); - - it('exposes the allowed filters per category', () => { - expect(FILTERS_BY_CATEGORY.contentFilter).toBe(CONTENT_FILTER_FILTERS); - expect(FILTERS_BY_CATEGORY.promptAttack.length).toBeGreaterThan(0); - expect(FILTERS_BY_CATEGORY.sensitiveInformation.length).toBeGreaterThan(0); - }); });