Skip to content

Commit 3cd6dc9

Browse files
authored
FIX: Accept only supported external inputs when building components (#2959)
1 parent fa829c3 commit 3cd6dc9

24 files changed

Lines changed: 1153 additions & 82 deletions

‎doc/code/framework.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -403,6 +403,7 @@ See [message normalizers](./targets/11_message_normalizer) for capability behavi
403403

404404
- If you are creating a component with user input (e.g. via config, REST, or automatically) it should always use the registry
405405
- If you are storing an instance of a component, it should always use the registry
406+
- The registry accepts only explicitly supported external inputs, permits opaque Python objects only for in-process callers, and leaves component validation to constructors.
406407

407408
## [Setup](./setup/0_setup)
408409

‎doc/gui/0_gui.md‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -588,9 +588,11 @@ at `GET /api/runtime`.
588588
## Registry API Migration Notes
589589

590590
Use `/api/converters/types` and `/api/targets/types` for registry build metadata.
591-
These endpoints return all constructor parameters from the registry, including
592-
lists, unions, and component references. The temporary `/catalog` routes retain
593-
their scalar-only filtering for the current UI.
591+
These endpoints return the constructor parameters external callers can set, each
592+
described in the form callers send it: a flat collection as a list, a union as its
593+
first alternative callers can send (`font_size: int | tuple[int, int]` as `int`),
594+
and a component reference as a name. They leave out types external callers can't
595+
create. Registry metadata keeps every parameter with its full annotation.
594596
Create requests should supply an explicit registry `name`. Converter creation
595597
returns the complete `ConverterInstance`; read its type from
596598
`identifier.class_name`, not the old top-level `converter_type` field. Treat
@@ -607,10 +609,8 @@ allowlisted image, audio, and video extensions inline. Other files, including PD
607609
SVG, HTML, text, and executables, download as `application/octet-stream` attachments.
608610

609611
**Temporary compatibility, scheduled for removal with the chat migration:**
610-
the `/api/converters/catalog` and `/api/targets/catalog` routes project the same
611-
registry metadata for the current UI. Create requests without a name receive a
612-
generated `compat_...` name. New clients should not depend on these routes or
613-
unnamed creation.
612+
target create requests without a name receive a generated `compat_...` name.
613+
New clients should supply an explicit name.
614614

615615
## Connection Health
616616

‎frontend/src/components/Config/CreateTargetDialog.test.tsx‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1140,6 +1140,66 @@ describe("CreateTargetDialog", () => {
11401140
});
11411141
});
11421142

1143+
it("should submit the catalog video duration as a number", async () => {
1144+
const user = userEvent.setup();
1145+
const videoParameters: Parameter[] = [
1146+
{
1147+
name: "resolution_dimensions",
1148+
type_name: "str",
1149+
is_list: false,
1150+
choices: ["720x1280", "1280x720", "1024x1792", "1792x1024"],
1151+
default: "1280x720",
1152+
required: false,
1153+
},
1154+
{ name: "n_seconds", type_name: "int", is_list: false, choices: null, default: "4", required: false },
1155+
{ name: "model_name", type_name: "str", is_list: false, choices: null, default: null, required: true },
1156+
{ name: "endpoint", type_name: "str", is_list: false, choices: null, default: null, required: true },
1157+
{ name: "api_key", type_name: "str", is_list: false, choices: null, default: null, required: false },
1158+
{ name: "underlying_model", type_name: "str", is_list: false, choices: null, default: null, required: false },
1159+
];
1160+
mockedTargetsApi.listTargetTypes.mockResolvedValue({
1161+
items: TARGET_TYPES.items.map((item) => (
1162+
item.target_type === "OpenAIVideoTarget" ? { ...item, parameters: videoParameters } : item
1163+
)),
1164+
});
1165+
mockedTargetsApi.createTarget.mockResolvedValue(makeTarget({
1166+
target_registry_name: "openai_video_custom",
1167+
target_type: "OpenAIVideoTarget",
1168+
}));
1169+
1170+
render(
1171+
<TestWrapper>
1172+
<CreateTargetDialog {...defaultProps} />
1173+
</TestWrapper>
1174+
);
1175+
1176+
await selectTargetType("OpenAIVideoTarget");
1177+
await user.click(screen.getByText("Advanced settings"));
1178+
1179+
fireEvent.change(screen.getByPlaceholderText("https://your-resource.openai.azure.com/"), {
1180+
target: { value: "https://api.openai.com" },
1181+
});
1182+
fireEvent.change(screen.getByPlaceholderText("e.g. gpt-4o, my-deployment"), {
1183+
target: { value: "sora-2" },
1184+
});
1185+
fireEvent.change(screen.getByLabelText("N Seconds"), {
1186+
target: { value: "8" },
1187+
});
1188+
1189+
await user.click(screen.getByText("Create Target"));
1190+
1191+
await waitFor(() => {
1192+
expect(mockedTargetsApi.createTarget).toHaveBeenCalledWith({
1193+
type: "OpenAIVideoTarget",
1194+
params: {
1195+
endpoint: "https://api.openai.com",
1196+
model_name: "sora-2",
1197+
n_seconds: 8,
1198+
},
1199+
});
1200+
});
1201+
});
1202+
11431203
it("should render and submit metadata choices and booleans", async () => {
11441204
const user = userEvent.setup();
11451205
mockedTargetsApi.createTarget.mockResolvedValue(makeTarget({

‎frontend/src/components/Registry/CreateConverterDialog.test.tsx‎

Lines changed: 76 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -282,7 +282,7 @@ describe('CreateConverterDialog', () => {
282282
expect(mockedConvertersApi.createConverter).toHaveBeenCalledWith({
283283
name: 'caesar-custom',
284284
type: 'CaesarConverter',
285-
params: { caesar_offset: '5' },
285+
params: { caesar_offset: 5 },
286286
})
287287
expect(onCreated).toHaveBeenCalledWith('caesar-custom')
288288
})
@@ -332,11 +332,85 @@ describe('CreateConverterDialog', () => {
332332
expect(mockedConvertersApi.createConverter).toHaveBeenCalledWith({
333333
name: 'SearchReplaceConverter',
334334
type: 'SearchReplaceConverter',
335-
params: { pattern: 'hello', replace: 'world', regex_flags: '0' },
335+
params: { pattern: 'hello', replace: 'world', regex_flags: 0 },
336336
})
337337
},
338338
)
339339

340+
it('should submit the catalog font size as a number', async () => {
341+
const user = userEvent.setup()
342+
mockConverterParameters([
343+
{ name: 'img_to_add', type_name: 'Path', is_list: false, choices: null, default: null, required: true },
344+
{ name: 'font_name', type_name: 'Path', is_list: false, choices: null, default: null, required: false },
345+
{ name: 'font_size', type_name: 'int', is_list: false, choices: null, default: '15', required: false },
346+
{ name: 'rotation', type_name: 'float', is_list: false, choices: null, default: '0.0', required: false },
347+
{ name: 'center_text', type_name: 'bool', is_list: false, choices: null, default: 'False', required: false },
348+
], 'AddImageTextConverter')
349+
renderDialog()
350+
await selectConverterType('AddImageTextConverter')
351+
352+
await user.type(screen.getByRole('textbox', { name: 'img_to_add *' }), 'data:image/png;base64,iVBORw0K')
353+
const fontSize = screen.getByRole('textbox', { name: 'font_size' })
354+
expect(fontSize).toHaveValue('15')
355+
await user.clear(fontSize)
356+
await user.type(fontSize, 'large')
357+
await user.click(screen.getByRole('button', { name: 'Add Converter' }))
358+
expect(screen.getByText('font_size must be a number.')).toBeInTheDocument()
359+
expect(mockedConvertersApi.createConverter).not.toHaveBeenCalled()
360+
361+
await user.clear(fontSize)
362+
await user.type(fontSize, '24')
363+
await user.click(screen.getByRole('button', { name: 'Add Converter' }))
364+
expect(mockedConvertersApi.createConverter).toHaveBeenCalledWith({
365+
name: 'AddImageTextConverter',
366+
type: 'AddImageTextConverter',
367+
params: { img_to_add: 'data:image/png;base64,iVBORw0K', font_size: 24, rotation: 0 },
368+
})
369+
})
370+
371+
it('should submit the catalog SATA word lists as arrays', async () => {
372+
const user = userEvent.setup()
373+
mockConverterParameters([
374+
{ name: 'mask_token', type_name: 'str', is_list: false, choices: null, default: '[MASK]', required: false },
375+
{ ...wordSelectionParameter, name: 'selection_strategy' },
376+
{ name: 'num_masks', type_name: 'int', is_list: false, choices: null, default: null, required: false },
377+
{ name: 'stopwords', type_name: 'list[str]', is_list: true, choices: null, default: null, required: false },
378+
{
379+
name: 'candidate_words', type_name: 'list[str]', is_list: true, choices: null, default: null, required: false,
380+
},
381+
], 'SATAMaskingConverter')
382+
renderDialog()
383+
await selectConverterType('SATAMaskingConverter')
384+
385+
await user.type(screen.getByRole('textbox', { name: 'stopwords' }), 'the, a')
386+
await user.type(screen.getByRole('textbox', { name: 'candidate_words' }), 'bomb')
387+
await user.click(screen.getByRole('button', { name: 'Add Converter' }))
388+
expect(mockedConvertersApi.createConverter).toHaveBeenCalledWith({
389+
name: 'SATAMaskingConverter',
390+
type: 'SATAMaskingConverter',
391+
params: { mask_token: '[MASK]', stopwords: ['the', 'a'], candidate_words: ['bomb'] },
392+
})
393+
})
394+
395+
it('should send text values exactly as entered', async () => {
396+
const user = userEvent.setup()
397+
mockConverterParameters([
398+
{ name: 'pattern', type_name: 'str', required: true },
399+
{ name: 'replace', type_name: 'str', required: true },
400+
], 'SearchReplaceConverter')
401+
renderDialog()
402+
await selectConverterType('SearchReplaceConverter')
403+
404+
await user.type(screen.getByRole('textbox', { name: 'pattern *' }), 'cat')
405+
await user.type(screen.getByRole('textbox', { name: 'replace *' }), ' dog ')
406+
await user.click(screen.getByRole('button', { name: 'Add Converter' }))
407+
expect(mockedConvertersApi.createConverter).toHaveBeenCalledWith({
408+
name: 'SearchReplaceConverter',
409+
type: 'SearchReplaceConverter',
410+
params: { pattern: 'cat', replace: ' dog ' },
411+
})
412+
})
413+
340414
it('selects a registered target for a target reference parameter', async () => {
341415
mockedConvertersApi.listConverterTypes.mockResolvedValue({
342416
items: [

‎frontend/src/components/Registry/CreateConverterDialog.tsx‎

Lines changed: 23 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,9 @@ import ParameterField from '@/components/Parameters/ParameterField'
2828
import {
2929
buildParametersFromForm,
3030
getInitialFormValues,
31+
getParameterControlKind,
3132
isStructuredParameterFormValue,
33+
type ParameterControlKind,
3234
type ParameterFormValue,
3335
} from '@/components/Parameters/parameterForm'
3436

@@ -117,6 +119,15 @@ function canConfigureConverterType(converterType: ConverterTypeEntry): boolean {
117119
)
118120
}
119121

122+
// Text, JSON, and multiselect values go to the API exactly as entered: whitespace can be the
123+
// value (a replacement, for one), and this dialog shows those parameters as text or a single
124+
// select. The other controls are converted to the JSON type their parameter describes.
125+
const TYPED_CONTROL_KINDS = new Set<ParameterControlKind>(['structured', 'boolean', 'select', 'list', 'number'])
126+
127+
function sendsTypedValue(parameter: Parameter): boolean {
128+
return TYPED_CONTROL_KINDS.has(getParameterControlKind(parameter))
129+
}
130+
120131
function parameterDefaultValue(parameter: Parameter): string {
121132
if (Array.isArray(parameter.default)) {
122133
return parameter.default.join(', ')
@@ -408,21 +419,23 @@ export default function CreateConverterDialog({
408419
}
409420

410421
const parameters = selectedConverterType?.parameters ?? []
411-
const params = Object.fromEntries(
412-
Object.entries(parameterValues).filter(([, value]) => !isStructuredParameterFormValue(value)),
413-
)
414-
const structured = buildParametersFromForm(
415-
parameters.filter((parameter) => parameter.variants),
416-
parameterValues,
422+
const params: Record<string, unknown> = Object.fromEntries(
423+
parameters
424+
.filter((parameter) => !sendsTypedValue(parameter))
425+
.flatMap((parameter) => {
426+
const value = parameterValues[parameter.name]
427+
return typeof value === 'string' ? [[parameter.name, value]] : []
428+
}),
417429
)
418-
if (!structured.ok) {
430+
const typed = buildParametersFromForm(parameters.filter(sendsTypedValue), parameterValues)
431+
if (!typed.ok) {
419432
// Not tagged as a submission failure: nothing was disabled, so the keyboard
420433
// is still on the primary action and has nothing to be restored from.
421-
setError({ message: structured.error, fromSubmit: false })
434+
setError({ message: typed.error, fromSubmit: false })
422435
return
423436
}
424-
if (structured.parameters) {
425-
Object.assign(params, structured.parameters)
437+
if (typed.parameters) {
438+
Object.assign(params, typed.parameters)
426439
}
427440

428441
const epoch = openEpochRef.current

‎pyrit/backend/services/converter_service.py‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -112,9 +112,11 @@ async def list_converter_types_async(self) -> ConverterTypeResponse:
112112
"""
113113
List all available converter types from the converter class registry.
114114
115-
Returns every constructible converter. Deciding which entries to surface
116-
to a user is a presentation concern owned by the caller (e.g. the
117-
frontend), not this service.
115+
Returns every converter that external callers can build, with only the
116+
parameters they may supply, each described in the form callers send it;
117+
converters that need a Python object for a required parameter are left out.
118+
Deciding which entries to surface to a user is a presentation concern owned
119+
by the caller (e.g. the frontend), not this service.
118120
119121
Returns:
120122
ConverterTypeResponse containing all available converter classes.
@@ -124,11 +126,14 @@ async def list_converter_types_async(self) -> ConverterTypeResponse:
124126
converter_type=metadata.class_name,
125127
supported_input_types=list(metadata.supported_input_types),
126128
supported_output_types=list(metadata.supported_output_types),
127-
parameters=list(metadata.parameters),
129+
parameters=[
130+
parameter.for_external_catalog() for parameter in metadata.parameters if parameter.is_external_input
131+
],
128132
is_llm_based=metadata.is_llm_based,
129133
description=metadata.class_description or None,
130134
)
131135
for metadata in self._registry.get_all_registered_class_metadata()
136+
if all(parameter.is_external_input for parameter in metadata.parameters if parameter.required)
132137
]
133138

134139
return ConverterTypeResponse(items=items)
@@ -197,7 +202,7 @@ async def create_converter_async(self, *, request: CreateConverterRequest) -> Co
197202
try:
198203
# Uploads may have yielded to another request that took the name.
199204
self._registry.instances.validate_name_available(request.name)
200-
converter_obj = self._registry.create_instance(request.type, **params)
205+
converter_obj = self._registry.create_instance_from_external_input(request.type, params=params)
201206
converter = self._build_instance_from_object(converter_id=request.name, converter_obj=converter_obj)
202207
self._registry.instances.register(
203208
converter_obj,

‎pyrit/backend/services/scenario_run_service.py‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@
7676
)
7777
from pyrit.prompt_target import PromptTarget
7878
from pyrit.registry import InitializerRegistry, ScenarioRegistry
79-
from pyrit.registry.resolution import resolve_declared_params
79+
from pyrit.registry.resolution import reject_non_external_params, resolve_declared_params
8080
from pyrit.scenario import Scenario
8181
from pyrit.scenario.core import override_default_adversarial_target
8282

@@ -626,6 +626,12 @@ async def _prepare_run_async(self, *, request: RunScenarioRequest) -> _PreparedR
626626
ValueError: If scenario, target, initializer, or technique cannot be found.
627627
"""
628628
scenario_class = self._configuration_resolver.resolve_scenario_class(scenario_name=request.scenario_name)
629+
if request.scenario_params:
630+
reject_non_external_params(
631+
params=request.scenario_params,
632+
declared=scenario_class.supported_parameters(),
633+
owner=request.scenario_name,
634+
)
629635
await self._run_initializers_async(request=request)
630636
objective_target = self._configuration_resolver.resolve_target(target_name=request.target_name)
631637
adversarial_target = self._configuration_resolver.resolve_adversarial_target(

‎pyrit/backend/services/scenario_service.py‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
ScenarioRunSizeEstimateRequest,
2222
)
2323
from pyrit.registry import ScenarioMetadata, ScenarioRegistry
24+
from pyrit.registry.resolution import reject_non_external_params
2425
from pyrit.scenario.core import Scenario, override_default_adversarial_target
2526
from pyrit.scenario.core.dataset_configuration import read_only_dataset_resolution
2627

@@ -68,7 +69,11 @@ def _metadata_to_registered_scenario(
6869
all_techniques=list(metadata.all_techniques),
6970
technique_summaries=list(metadata.technique_summaries),
7071
default_datasets=list(metadata.default_datasets),
71-
supported_parameters=list(metadata.supported_parameters),
72+
supported_parameters=[
73+
parameter.for_external_catalog()
74+
for parameter in metadata.supported_parameters
75+
if parameter.is_external_input
76+
],
7277
baseline_policy=metadata.baseline_policy,
7378
include_baseline_by_default=metadata.include_baseline_by_default,
7479
uses_default_adversarial_target=metadata.uses_default_adversarial_target,
@@ -193,6 +198,12 @@ async def estimate_scenario_run_size_async(
193198
scenario_class = self._registry.get_class(scenario_name)
194199
except KeyError:
195200
return None
201+
if request.scenario_params:
202+
reject_non_external_params(
203+
params=request.scenario_params,
204+
declared=scenario_class.supported_parameters(),
205+
owner=scenario_name,
206+
)
196207

197208
estimate_key = self._build_configured_estimate_key(
198209
scenario_name=scenario_name, scenario_class=scenario_class, request=request

‎pyrit/backend/services/scorer_service.py‎

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,21 +36,30 @@ def _build_instance(self, *, name: str, scorer: Any) -> ScorerInstance:
3636

3737
async def list_scorer_types_async(self) -> ScorerTypeResponse:
3838
"""
39-
List registered scorer class metadata without constructing scorers.
39+
List the scorer types external callers can build, without constructing scorers.
40+
41+
Each entry lists only the parameters external callers may supply, each
42+
described in the form callers send it; types that need a Python object for a
43+
required parameter are left out.
4044
4145
Returns:
42-
ScorerTypeResponse: All registered scorer type metadata.
46+
ScorerTypeResponse: Scorer type metadata for external callers.
4347
"""
4448

4549
def list_types() -> ScorerTypeResponse:
4650
items = [
4751
ScorerTypeEntry(
4852
scorer_type=metadata.class_name,
49-
parameters=list(metadata.parameters),
53+
parameters=[
54+
parameter.for_external_catalog()
55+
for parameter in metadata.parameters
56+
if parameter.is_external_input
57+
],
5058
is_llm_based=metadata.is_llm_based,
5159
description=metadata.class_description or None,
5260
)
5361
for metadata in self._registry.get_all_registered_class_metadata()
62+
if all(parameter.is_external_input for parameter in metadata.parameters if parameter.required)
5463
]
5564
return ScorerTypeResponse(items=items)
5665

@@ -110,6 +119,7 @@ def create() -> ScorerInstance:
110119
name=request.name,
111120
type_name=request.type,
112121
params=request.params,
122+
external_input=True,
113123
)
114124
return self._build_instance(name=request.name, scorer=scorer)
115125

0 commit comments

Comments
 (0)