diff --git a/doc/code/registry/0_registry.md b/doc/code/registry/0_registry.md index 24e3f0f355..35c49335f4 100644 --- a/doc/code/registry/0_registry.md +++ b/doc/code/registry/0_registry.md @@ -131,7 +131,12 @@ reconstruction: Use `source.params` for overrides; do not also supply `params`. The backend keeps the source's other constructor inputs, including authentication, on the -server. It does not change the source. Missing, changed, or ambiguous sources +server. It does not change the source. A copy also keeps the settings that +choose where requests go and how they sign in, such as `endpoint`, `auth_mode`, +`api_key` and `headers` for OpenAI targets, and the region, key and resource ID +for the Azure Speech converters; overriding one of these is an error. Such +parameters report `copy_locked: true`, and components declare them in +`COPY_LOCKED_PARAMETER_NAMES`. Missing, changed, or ambiguous sources produce an error. If a source was renamed, a unique matching hash can resolve it. An optional `effective_hash` checks the reconstructed object's identity. Registered descriptors report `reconstructable` and, for targets, diff --git a/frontend/src/components/Registry/CreateConverterDialog.test.tsx b/frontend/src/components/Registry/CreateConverterDialog.test.tsx index 0204fc4107..3e1581c623 100644 --- a/frontend/src/components/Registry/CreateConverterDialog.test.tsx +++ b/frontend/src/components/Registry/CreateConverterDialog.test.tsx @@ -179,6 +179,82 @@ describe('CreateConverterDialog', () => { expect(onTemporary.mock.calls[0][1].hash).toBe('private-hash') }) + it('keeps a copy on its source connection settings', async () => { + const user = userEvent.setup() + const onTemporary = jest.fn() + mockConverterParameters([ + { name: 'azure_speech_region', type_name: 'str', required: false, default: null, copy_locked: true }, + { name: 'synthesis_voice_name', type_name: 'str', required: false, default: 'en-US-AvaNeural' }, + ], 'AzureSpeechTextToAudioConverter') + const converter = { + converter_id: 'speech', + identifier: { + class_name: 'AzureSpeechTextToAudioConverter', class_module: 'pyrit.converter', + pyrit_version: 'test', hash: 'source-hash', + }, + } + mockedConvertersApi.buildConverter.mockResolvedValue({ identifier: { ...converter.identifier, hash: 'copy-hash' } }) + renderDialog({ editing: { converter }, onTemporary }) + + const voice = await screen.findByRole('textbox', { name: /synthesis_voice_name/i }) + expect(screen.queryByRole('textbox', { name: /azure_speech_region/i })).not.toBeInTheDocument() + expect(screen.getByText(/connection settings come from the source converter/i)).toBeInTheDocument() + await user.clear(voice) + await user.type(voice, 'en-US-JennyNeural') + await user.click(screen.getByRole('button', { name: 'Apply Settings' })) + + await waitFor(() => expect(onTemporary).toHaveBeenCalledTimes(1)) + expect(mockedConvertersApi.buildConverter).toHaveBeenCalledWith('AzureSpeechTextToAudioConverter', { + source_name: 'speech', source_hash: 'source-hash', params: { synthesis_voice_name: 'en-US-JennyNeural' }, + }) + }) + + it('drops a connection setting carried over from earlier temporary settings', async () => { + const user = userEvent.setup() + const onTemporary = jest.fn() + mockConverterParameters([ + { name: 'azure_speech_region', type_name: 'str', required: false, default: null, copy_locked: true }, + { name: 'synthesis_voice_name', type_name: 'str', required: false, default: 'en-US-AvaNeural' }, + { name: 'output_format', type_name: 'str', required: false, default: 'wav' }, + ], 'AzureSpeechTextToAudioConverter') + const converter = { + converter_id: 'speech', + identifier: { + class_name: 'AzureSpeechTextToAudioConverter', class_module: 'pyrit.converter', + pyrit_version: 'test', hash: 'source-hash', + }, + } + const spec = { + source_name: 'speech', source_hash: 'source-hash', + params: { azure_speech_region: 'westus2', output_format: 'mp3' }, + } + mockedConvertersApi.buildConverter.mockResolvedValue({ identifier: { ...converter.identifier, hash: 'copy-hash' } }) + renderDialog({ editing: { converter, spec }, onTemporary }) + + const voice = await screen.findByRole('textbox', { name: /synthesis_voice_name/i }) + await user.clear(voice) + await user.type(voice, 'en-US-JennyNeural') + await user.click(screen.getByRole('button', { name: 'Apply Settings' })) + + await waitFor(() => expect(onTemporary).toHaveBeenCalledTimes(1)) + expect(mockedConvertersApi.buildConverter).toHaveBeenCalledWith('AzureSpeechTextToAudioConverter', { + source_name: 'speech', + source_hash: 'source-hash', + params: { output_format: 'mp3', synthesis_voice_name: 'en-US-JennyNeural' }, + }) + }) + + it('offers connection settings when creating a converter', async () => { + mockConverterParameters([ + { name: 'azure_speech_region', type_name: 'str', required: false, default: null, copy_locked: true }, + ], 'AzureSpeechTextToAudioConverter') + renderDialog() + await selectConverterType('AzureSpeechTextToAudioConverter') + + expect(await screen.findByRole('textbox', { name: /azure_speech_region/i })).toBeInTheDocument() + expect(screen.queryByText(/connection settings come from the source converter/i)).not.toBeInTheDocument() + }) + it('locks scalar inputs while Apply Settings is building so a newer edit cannot be lost', async () => { const user = userEvent.setup() const onTemporary = jest.fn() diff --git a/frontend/src/components/Registry/CreateConverterDialog.tsx b/frontend/src/components/Registry/CreateConverterDialog.tsx index 99d3bdc247..deb19588cb 100644 --- a/frontend/src/components/Registry/CreateConverterDialog.tsx +++ b/frontend/src/components/Registry/CreateConverterDialog.tsx @@ -463,6 +463,18 @@ export default function CreateConverterDialog({ () => converterTypes.find((item) => item.converter_type === selectedType), [converterTypes, selectedType], ) + // Temporary settings can't move a copy to a different endpoint or credential. + const lockedParameterNames = useMemo( + () => new Set((editing ? selectedConverterType?.parameters ?? [] : []) + .filter((parameter) => parameter.copy_locked) + .map((parameter) => parameter.name)), + [selectedConverterType, editing], + ) + const editableParameters = useMemo( + () => (selectedConverterType?.parameters ?? []).filter((parameter) => !lockedParameterNames.has(parameter.name)), + [selectedConverterType, lockedParameterNames], + ) + const keepsSourceConnection = lockedParameterNames.size > 0 const groupedConverterTypes = useMemo(() => { const groups = new Map() for (const converterType of converterTypes) { @@ -594,7 +606,7 @@ export default function CreateConverterDialog({ } const submit = async () => { - const missingParameters = (selectedConverterType?.parameters ?? []).some( + const missingParameters = editableParameters.some( (parameter) => parameter.required && !parameter.default && !acceptsEmptySearchReplaceValue(selectedType, parameter.name) @@ -605,7 +617,7 @@ export default function CreateConverterDialog({ return } - const parameters = (selectedConverterType?.parameters ?? []).filter( + const parameters = editableParameters.filter( (parameter) => !editing || changedParameters.has(parameter.name), ) const params: Record = Object.fromEntries( @@ -640,8 +652,9 @@ export default function CreateConverterDialog({ source_name: editing.spec?.source_name ?? editing.converter.converter_id, source_hash: editing.spec?.source_hash ?? editing.converter.identifier.hash, params: { + // Settings applied before copies kept their source's connection would now be refused. ...Object.fromEntries(Object.entries(editing.spec?.params ?? {}) - .filter(([name]: [string, unknown]) => !changedParameters.has(name))), + .filter(([name]: [string, unknown]) => !changedParameters.has(name) && !lockedParameterNames.has(name))), ...params, }, } @@ -805,8 +818,11 @@ export default function CreateConverterDialog({ /> } {editing && Changes apply only to this stage. The registered converter stays unchanged.} + {keepsSourceConnection && ( + Connection settings come from the source converter and can't be changed here. + )}
- {selectedConverterType?.parameters.map((parameter) => ( + {editableParameters.map((parameter) => (
{parameter.variants || parameter.multiline ? ( "CreateTargetRequest": - if self.source is not None and (self.register or self.params or "auth_mode" in self.model_fields_set): - raise ValueError("source requires register=false and overrides, including auth_mode, in source.params") + if self.source is None: + return self + if self.register: + raise ValueError("source requires register=false") + if self.params: + raise ValueError("a copy takes its overrides in source.params") + if "auth_mode" in self.model_fields_set: + raise ValueError("a copy uses its source's auth mode, so auth_mode can't be set") return self diff --git a/pyrit/backend/services/converter_service.py b/pyrit/backend/services/converter_service.py index 81194954d7..b2b8835517 100644 --- a/pyrit/backend/services/converter_service.py +++ b/pyrit/backend/services/converter_service.py @@ -100,7 +100,7 @@ def _build_instance_from_object(self, *, converter_id: str, converter_obj: Any) from pyrit.registry.resolution import project_parameter_value for parameter in metadata.parameters if metadata else []: - if not parameter.is_external_input or parameter.sensitive: + if not parameter.is_external_input or parameter.sensitive or parameter.copy_locked: continue try: value = project_parameter_value( diff --git a/pyrit/converter/azure_speech_audio_to_text_converter.py b/pyrit/converter/azure_speech_audio_to_text_converter.py index 641f428cd7..a96074dbe0 100644 --- a/pyrit/converter/azure_speech_audio_to_text_converter.py +++ b/pyrit/converter/azure_speech_audio_to_text_converter.py @@ -5,7 +5,7 @@ import logging import time from collections.abc import Awaitable, Callable -from typing import TYPE_CHECKING, Any +from typing import TYPE_CHECKING, Any, ClassVar if TYPE_CHECKING: import azure.cognitiveservices.speech as speechsdk @@ -44,6 +44,10 @@ class AzureSpeechAudioToTextConverter(Converter): #: The resource ID for accessing the service when using Entra ID auth. AZURE_SPEECH_RESOURCE_ID_ENVIRONMENT_VARIABLE: str = "AZURE_SPEECH_RESOURCE_ID" + COPY_LOCKED_PARAMETER_NAMES: ClassVar[frozenset[str]] = frozenset( + {"azure_speech_region", "azure_speech_key", "azure_speech_resource_id"} + ) + def __init__( self, *, diff --git a/pyrit/converter/azure_speech_text_to_audio_converter.py b/pyrit/converter/azure_speech_text_to_audio_converter.py index 77ccc7ab70..1bd6312fdb 100644 --- a/pyrit/converter/azure_speech_text_to_audio_converter.py +++ b/pyrit/converter/azure_speech_text_to_audio_converter.py @@ -3,7 +3,7 @@ import logging from collections.abc import Awaitable, Callable -from typing import TYPE_CHECKING, Literal +from typing import TYPE_CHECKING, ClassVar, Literal if TYPE_CHECKING: import azure.cognitiveservices.speech as speechsdk # noqa: F401 @@ -42,6 +42,10 @@ class AzureSpeechTextToAudioConverter(Converter): #: The resource ID for accessing the service when using Entra ID auth. AZURE_SPEECH_RESOURCE_ID_ENVIRONMENT_VARIABLE: str = "AZURE_SPEECH_RESOURCE_ID" + COPY_LOCKED_PARAMETER_NAMES: ClassVar[frozenset[str]] = frozenset( + {"azure_speech_region", "azure_speech_key", "azure_speech_resource_id"} + ) + #: Supported audio formats for output. AzureSpeechAudioFormat = Literal["wav", "mp3"] diff --git a/pyrit/converter/converter.py b/pyrit/converter/converter.py index d9cf1654a6..df48a86d6d 100644 --- a/pyrit/converter/converter.py +++ b/pyrit/converter/converter.py @@ -64,6 +64,10 @@ class Converter(Identifiable): #: Constructor parameters whose controls must preserve line breaks. MULTILINE_PARAMETER_NAMES: ClassVar[frozenset[str]] = frozenset() + #: Constructor parameters that choose where requests go or how they authenticate. A private + #: copy built from API input keeps its source's values for these. + COPY_LOCKED_PARAMETER_NAMES: ClassVar[frozenset[str]] = frozenset() + #: Capability requirements placed on the converter's target (if any). #: Subclasses that use a target should override this and pass the target to #: ``super().__init__(converter_target=...)`` so the base class can validate it. diff --git a/pyrit/models/parameter.py b/pyrit/models/parameter.py index f20b268982..dc45b7f452 100644 --- a/pyrit/models/parameter.py +++ b/pyrit/models/parameter.py @@ -96,8 +96,8 @@ class Parameter(BaseModel): value's live Python type and its allowed set (a ``Literal[...]`` or ``Enum`` *is* the allowed set) and drives ``coerce_value`` / ``validate``; it is **not** serialized. Serialization instead projects the type into the display fields - ``type_name``, ``choices``, ``is_list``, ``sensitive``, ``multiline``, and - ``identity_conflicting`` (plus ``required`` from the ``REQUIRED_VALUE`` + ``type_name``, ``choices``, ``is_list``, ``sensitive``, ``multiline``, + ``identity_conflicting``, and ``copy_locked`` (plus ``required`` from the ``REQUIRED_VALUE`` sentinel), so a consumer can rebuild a usable contract from the registry without the live type travelling on the wire. @@ -150,6 +150,13 @@ class Parameter(BaseModel): default=False, description="Whether this parameter must be omitted when identity-based authentication is selected.", ) + copy_locked: bool = Field( + default=False, + description=( + "Whether a private copy keeps its source's value, because the parameter chooses where requests go " + "or how they authenticate." + ), + ) destination: ParameterDestination = Field( default=ParameterDestination.CONSTRUCTOR, exclude=True, diff --git a/pyrit/prompt_target/common/prompt_target.py b/pyrit/prompt_target/common/prompt_target.py index a020642981..edec6ee636 100644 --- a/pyrit/prompt_target/common/prompt_target.py +++ b/pyrit/prompt_target/common/prompt_target.py @@ -109,6 +109,11 @@ class PromptTarget(Identifiable): # unaffected. server_credential_parameter: ClassVar[str | None] = None + # Constructor parameters that choose where requests go or how they authenticate. A private + # copy built from API input keeps its source's values for these, so a credential stays + # with the endpoint it was configured for. + COPY_LOCKED_PARAMETER_NAMES: ClassVar[frozenset[str]] = frozenset() + def __init_subclass__(cls, **kwargs: object) -> None: """ Validate that subclasses follow the keyword-only ``__init__`` contract. diff --git a/pyrit/prompt_target/openai/openai_target.py b/pyrit/prompt_target/openai/openai_target.py index f2034fa7a9..0bb58ef0fd 100644 --- a/pyrit/prompt_target/openai/openai_target.py +++ b/pyrit/prompt_target/openai/openai_target.py @@ -64,6 +64,10 @@ class OpenAITarget(PromptTarget): # such as Ollama or vLLM may need no key at all. supported_auth_modes: ClassVar[tuple[AuthMode, ...]] = ("api_key", "identity", "none") + COPY_LOCKED_PARAMETER_NAMES: ClassVar[frozenset[str]] = frozenset( + {"endpoint", "auth_mode", "api_key", "headers", "httpx_client_kwargs"} + ) + model_name_environment_variable: str endpoint_environment_variable: str api_key_environment_variable: str diff --git a/pyrit/registry/registry.py b/pyrit/registry/registry.py index 5c76ae6253..97599b45f0 100644 --- a/pyrit/registry/registry.py +++ b/pyrit/registry/registry.py @@ -982,7 +982,8 @@ def recreate_instance( InstanceT: The new component, owned by the caller. Raises: - ValueError: If overrides are unknown or the source class is unavailable. + ValueError: If overrides are unknown, external overrides set a connection setting the + source's class keeps for copies, or the source class is unavailable. """ inputs = self.get_reconstruction_parameters(source) raw_params = dict(params) @@ -1011,6 +1012,14 @@ def recreate_instance( seam_base_type=self._seam_base_type(), allowed_path_params=allowed_path_params, ) + # A copy keeps where its source sends requests and how it signs in, so a credential + # can't be pointed at a different endpoint. Any value is refused, even the source's own. + locked = sorted(name for name in params if by_name[name].copy_locked) + if locked: + raise ValueError( + f"A copy keeps its source's connection settings, so it can't set {', '.join(locked)}. " + "Create a new one to use different settings." + ) from pyrit.models import SeedPrompt for name, value in raw_params.items(): diff --git a/pyrit/registry/resolution.py b/pyrit/registry/resolution.py index ff4448cd3d..531bc53a83 100644 --- a/pyrit/registry/resolution.py +++ b/pyrit/registry/resolution.py @@ -265,6 +265,7 @@ def _parameters_from_signature( sensitive_parameter_names: frozenset[str], multiline_parameter_names: frozenset[str], identity_conflicting_parameter_names: frozenset[str], + copy_locked_parameter_names: frozenset[str], ) -> list[Parameter]: """ Build parameters declared by one constructor signature. @@ -280,6 +281,8 @@ def _parameters_from_signature( names whose values require multiline controls. identity_conflicting_parameter_names (frozenset[str]): Identifier-declared names that must be omitted for identity-based authentication. + copy_locked_parameter_names (frozenset[str]): Component-declared names whose + values a private copy keeps from its source. Returns: list[Parameter]: Parameters declared by the constructor. @@ -315,6 +318,7 @@ def _parameters_from_signature( sensitive=name in sensitive_parameter_names, multiline=name in multiline_parameter_names, identity_conflicting=name in identity_conflicting_parameter_names, + copy_locked=name in copy_locked_parameter_names, ) if parameter.is_seed_prompt: parameter = parameter.model_copy(update={"multiline": True}) @@ -342,6 +346,10 @@ def derive_parameters( of constructor parameters whose controls must preserve line breaks. These names supplement the identifier's shared multiline names for that component only. + A component can also declare ``COPY_LOCKED_PARAMETER_NAMES``: the constructor + parameters that choose where its requests go or how they authenticate. A private + copy built from external input keeps the source's values for these. + A non-required reference parameter declared by ``seam_base_type`` is a construction seam, not a build input of every subclass: the base accepts it so subclasses can forward a value (e.g. for validation) through one construction @@ -377,6 +385,7 @@ def derive_parameters( identity_conflicting_parameter_names = ( identifier_type.get_identity_conflicting_parameter_names() if identifier_type is not None else frozenset[str]() ) + copy_locked_parameter_names = getattr(cls, "COPY_LOCKED_PARAMETER_NAMES", frozenset[str]()) parameters: list[Parameter] = [] seen: set[str] = set() for owner, signature in _constructor_sources(cls): @@ -387,6 +396,7 @@ def derive_parameters( sensitive_parameter_names=sensitive_parameter_names, multiline_parameter_names=multiline_parameter_names, identity_conflicting_parameter_names=identity_conflicting_parameter_names, + copy_locked_parameter_names=copy_locked_parameter_names, ): if parameter.name in seen: continue diff --git a/tests/unit/backend/test_converter_service.py b/tests/unit/backend/test_converter_service.py index 1cde8c8195..2db3fe795e 100644 --- a/tests/unit/backend/test_converter_service.py +++ b/tests/unit/backend/test_converter_service.py @@ -32,6 +32,7 @@ get_converter_service, ) from pyrit.converter import ( + AzureSpeechTextToAudioConverter, Base64Converter, BinaryConverter, CaesarConverter, @@ -420,6 +421,40 @@ async def test_role_play_editor_receives_full_system_prompt_async(upload_service assert parameter.default == prompt +@pytest.mark.usefixtures("patch_central_database") +async def test_speech_copies_keep_their_source_connection_async(upload_service: ConverterService) -> None: + source = AzureSpeechTextToAudioConverter(azure_speech_region="eastus", azure_speech_key="source-key") + upload_service._registry.instances.register(source, name="speech") + spec = SourceInstanceSpec( + source_name="speech", source_hash=source.get_identifier().hash, params={"azure_speech_region": "westus2"} + ) + + instance = await upload_service.get_converter_async(converter_id="speech") + assert instance is not None + assert {parameter.name for parameter in instance.parameters} >= {"synthesis_voice_name"} + assert not {parameter.name for parameter in instance.parameters} & source.COPY_LOCKED_PARAMETER_NAMES + with pytest.raises(ValueError, match="can't set azure_speech_region"): + await upload_service.create_converter_async( + request=CreateConverterRequest(type="AzureSpeechTextToAudioConverter", register=False, source=spec) + ) + with pytest.raises(ValueError, match="can't set azure_speech_region"): + await upload_service.create_converter_async( + request=CreateConverterRequest( + type="AzureSpeechTextToAudioConverter", register=True, name="moved_speech", source=spec + ) + ) + assert upload_service._registry.instances.get("moved_speech") is None + # The preview runs the converter, so keep a regression from reaching the Speech service. + with ( + patch.object(AzureSpeechTextToAudioConverter, "convert_async", new_callable=AsyncMock) as convert, + pytest.raises(ValueError, match="can't set azure_speech_region"), + ): + await upload_service.preview_conversion_async( + request=ConverterPreviewRequest(original_value="hello", converter_ids=["speech"], converter_specs=[spec]) + ) + convert.assert_not_called() + + @pytest.mark.usefixtures("patch_central_database") @pytest.mark.parametrize("custom", [False, True]) async def test_translation_editor_receives_the_effective_prompt_without_changing_identity_async( diff --git a/tests/unit/backend/test_technique_edit_service.py b/tests/unit/backend/test_technique_edit_service.py index 8a9bf335cd..8d0ad08333 100644 --- a/tests/unit/backend/test_technique_edit_service.py +++ b/tests/unit/backend/test_technique_edit_service.py @@ -22,7 +22,13 @@ from pyrit.backend.services.technique_service import TechniqueService from pyrit.common.apply_defaults import GlobalDefaultValues from pyrit.common.path import EXECUTOR_SEED_PROMPT_PATH, EXECUTOR_SIMULATED_TARGET_PATH -from pyrit.converter import Base64Converter, CaesarConverter, ImageOverlayConverter, ROT13Converter +from pyrit.converter import ( + AzureSpeechTextToAudioConverter, + Base64Converter, + CaesarConverter, + ImageOverlayConverter, + ROT13Converter, +) from pyrit.executor.attack import ( AttackConverterConfig, AttackExecutor, @@ -165,6 +171,16 @@ def test_temporary_converter_effective_hash_is_checked(edit_catalog: AttackTechn TechniqueEditService.validate(_specification(changes=TechniqueChanges(request_converters=[spec]))) +def test_temporary_converter_keeps_its_source_connection(edit_catalog: AttackTechniqueRegistry) -> None: + converter = AzureSpeechTextToAudioConverter(azure_speech_region="eastus", azure_speech_key="source-key") + ConverterRegistry.get_registry_singleton().instances.register(converter, name="speech") + spec = SourceInstanceSpec( + source_name="speech", source_hash=converter.get_identifier().hash, params={"azure_speech_key": "other-key"} + ) + with pytest.raises(ValueError, match="can't set azure_speech_key"): + TechniqueEditService.validate(_specification(changes=TechniqueChanges(request_converters=[spec]))) + + def test_temporary_file_converter_preserves_its_file_and_rejects_replacement( edit_catalog: AttackTechniqueRegistry, ) -> None: diff --git a/tests/unit/backend/test_temporary_components.py b/tests/unit/backend/test_temporary_components.py index de94717020..8b046b8d2d 100644 --- a/tests/unit/backend/test_temporary_components.py +++ b/tests/unit/backend/test_temporary_components.py @@ -276,22 +276,28 @@ async def test_identity_source_reuses_resolved_token_provider_async(self) -> Non await source.cleanup_target_async() @pytest.mark.parametrize("auth_mode", ["none", "identity", "api_key"]) - def test_source_takes_its_auth_mode_only_from_source_params(self, auth_mode: str) -> None: + def test_a_copy_uses_its_sources_auth_mode(self, auth_mode: str) -> None: source = SourceInstanceSpec(source_name="source", source_hash="hash", params={}) - with pytest.raises(ValidationError, match="including auth_mode"): + with pytest.raises(ValidationError, match="a copy uses its source's auth mode"): CreateTargetRequest(type="OpenAIChatTarget", source=source, register=False, auth_mode=auth_mode) @pytest.mark.parametrize( - "overrides", + ("fields", "message"), [ - {"temperature": 0.8}, - {"temperature": 0.8, "api_key": None}, - {"temperature": 0.8, "headers": None}, - {"temperature": 0.8, "headers": ""}, + ({}, "source requires register=false"), + ({"register": False, "params": {"temperature": 0.8}}, "a copy takes its overrides in source.params"), ], ) - async def test_no_key_source_stays_no_key_async(self, overrides: dict[str, Any]) -> None: + def test_a_copy_is_unregistered_and_takes_overrides_in_its_source( + self, fields: dict[str, object], message: str + ) -> None: + source = SourceInstanceSpec(source_name="source", source_hash="hash", params={}) + + with pytest.raises(ValidationError, match=message): + CreateTargetRequest(type="OpenAIChatTarget", source=source, **fields) + + async def test_no_key_source_stays_no_key_async(self) -> None: source = OpenAIChatTarget(endpoint="http://localhost:11434/v1", model_name="llama3", auth_mode="none") registry = TargetRegistry.get_registry_singleton() registry.instances.register(source, name="source") @@ -301,7 +307,9 @@ async def test_no_key_source_stays_no_key_async(self, overrides: dict[str, Any]) {"OPENAI_CHAT_KEY": "server-key", "OPENAI_ADDITIONAL_REQUEST_HEADERS": '{"X-Server-Secret": "s"}'}, ): private = await TargetService().build_from_source_async( - SourceInstanceSpec(source_name="source", source_hash=source.get_identifier().hash, params=overrides) + SourceInstanceSpec( + source_name="source", source_hash=source.get_identifier().hash, params={"temperature": 0.8} + ) ) try: assert private._api_key == "not-needed" @@ -312,6 +320,22 @@ async def test_no_key_source_stays_no_key_async(self, overrides: dict[str, Any]) finally: await source.cleanup_target_async() + @pytest.mark.parametrize("overrides", [{"api_key": None}, {"headers": None}, {"headers": ""}]) + async def test_no_key_source_copy_cannot_clear_its_key_or_headers_async(self, overrides: dict[str, Any]) -> None: + source = OpenAIChatTarget(endpoint="http://localhost:11434/v1", model_name="llama3", auth_mode="none") + TargetRegistry.get_registry_singleton().instances.register(source, name="source") + try: + with pytest.raises(ValueError, match=f"can't set {next(iter(overrides))}"): + await TargetService().build_from_source_async( + SourceInstanceSpec( + source_name="source", + source_hash=source.get_identifier().hash, + params={"temperature": 0.8, **overrides}, + ) + ) + finally: + await source.cleanup_target_async() + async def test_two_temperature_variants_are_isolated_async(self) -> None: source = source_target() TargetRegistry.get_registry_singleton().instances.register(source, name="source") @@ -764,6 +788,27 @@ async def test_source_overrides_keep_external_input_validation_async(self) -> No finally: await source.cleanup_target_async() + @pytest.mark.parametrize( + "params", + [ + {"endpoint": "https://other.test/v1"}, + {"endpoint": "https://example.test/v1"}, + {"api_key": "other-key"}, + {"auth_mode": "identity"}, + {"headers": '{"X-Extra": "1"}'}, + ], + ) + async def test_source_copy_keeps_connection_settings_async(self, params: dict[str, str]) -> None: + source = source_target() + TargetRegistry.get_registry_singleton().instances.register(source, name="source") + try: + with pytest.raises(ValueError, match=f"can't set {next(iter(params))}"): + await TargetService().build_from_source_async( + SourceInstanceSpec(source_name="source", source_hash=source.get_identifier().hash, params=params) + ) + finally: + await source.cleanup_target_async() + @pytest.mark.parametrize("operation_by_id", [False, True]) async def test_repeated_send_owns_each_temperature_target_async( self, sqlite_instance: SQLiteMemory, operation_by_id: bool diff --git a/tests/unit/models/test_parameter.py b/tests/unit/models/test_parameter.py index 50ba2ddb20..2b640e941c 100644 --- a/tests/unit/models/test_parameter.py +++ b/tests/unit/models/test_parameter.py @@ -206,6 +206,7 @@ def test_scalar_with_default(self) -> None: "sensitive": False, "multiline": False, "identity_conflicting": False, + "copy_locked": False, } def test_excludes_live_only_fields(self) -> None: diff --git a/tests/unit/registry/test_converter_registry.py b/tests/unit/registry/test_converter_registry.py index 99eac3aef0..2d9860af22 100644 --- a/tests/unit/registry/test_converter_registry.py +++ b/tests/unit/registry/test_converter_registry.py @@ -258,6 +258,44 @@ def test_recreate_instance_rejects_path_overrides_unless_allowed(self, registry: assert variant.get_identifier().params["base_image"] == "other.png" assert source.get_identifier().params["base_image"] == "input.png" + @pytest.mark.parametrize("type_name", ["AzureSpeechTextToAudioConverter", "AzureSpeechAudioToTextConverter"]) + @pytest.mark.parametrize( + "params", + [ + {"azure_speech_region": "westus2"}, + {"azure_speech_region": "eastus"}, + {"azure_speech_key": "other-key"}, + {"azure_speech_resource_id": "other-resource"}, + ], + ) + def test_recreate_instance_keeps_speech_connection_settings( + self, registry: ConverterRegistry, type_name: str, params: dict[str, str] + ): + source = registry.create_named_instance( + name="speech", + type_name=type_name, + params={"azure_speech_region": "eastus", "azure_speech_key": "source-key"}, + ) + + with pytest.raises(ValueError, match=f"can't set {next(iter(params))}"): + registry.recreate_instance(source=source, params=params, external_input=True) + + def test_recreate_instance_still_changes_other_speech_settings(self, registry: ConverterRegistry): + source = registry.create_named_instance( + name="speech", + type_name="AzureSpeechTextToAudioConverter", + params={"azure_speech_region": "eastus", "azure_speech_key": "source-key"}, + ) + + copy = registry.recreate_instance( + source=source, params={"synthesis_voice_name": "en-US-JennyNeural"}, external_input=True + ) + trusted = registry.recreate_instance(source=source, params={"azure_speech_region": "westus2"}) + + assert copy.get_identifier().params["synthesis_voice_name"] == "en-US-JennyNeural" + assert copy._azure_speech_region == "eastus" + assert trusted._azure_speech_region == "westus2" + @pytest.mark.parametrize("name", ["preview", "types"]) def test_create_named_instance_rejects_reserved_name(self, registry: ConverterRegistry, name: str): with pytest.raises(ValueError, match="reserved"): diff --git a/tests/unit/registry/test_copy_locked_parameters.py b/tests/unit/registry/test_copy_locked_parameters.py new file mode 100644 index 0000000000..c8b5f6c9aa --- /dev/null +++ b/tests/unit/registry/test_copy_locked_parameters.py @@ -0,0 +1,43 @@ +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT license. + +""" +Tests that every class whose instances can be copied through the API keeps its credentials for the copies. +""" + +from collections.abc import Iterator + +import pytest + +from pyrit.registry import ConverterRegistry, TargetRegistry + +_REGISTRIES = (TargetRegistry, ConverterRegistry) + + +@pytest.fixture(autouse=True) +def fresh_registries() -> Iterator[None]: + for registry in _REGISTRIES: + registry.reset_registry_singleton() + yield + for registry in _REGISTRIES: + registry.reset_registry_singleton() + + +@pytest.mark.parametrize("registry_class", _REGISTRIES) +def test_every_copyable_class_locks_its_credential_parameters( + registry_class: type[TargetRegistry] | type[ConverterRegistry], +) -> None: + registry = registry_class.get_registry_singleton() + copyable: list[str] = [] + unlocked: dict[str, list[str]] = {} + for metadata in registry.get_all_registered_class_metadata(): + cls = registry.get_class(metadata.class_name) + if not callable(getattr(cls, "get_reconstruction_parameters", None)): + continue + copyable.append(metadata.class_name) + sensitive = {parameter.name for parameter in metadata.parameters if parameter.sensitive} + if missing := sensitive - cls.COPY_LOCKED_PARAMETER_NAMES: + unlocked[metadata.class_name] = sorted(missing) + + assert copyable + assert unlocked == {} diff --git a/tests/unit/registry/test_resolution.py b/tests/unit/registry/test_resolution.py index 1d60fe1d9b..0fecd88433 100644 --- a/tests/unit/registry/test_resolution.py +++ b/tests/unit/registry/test_resolution.py @@ -728,6 +728,15 @@ def __init__(self, *, http_request: str = "", note: str = "") -> None: assert params["note"].multiline assert TargetIdentifier.get_multiline_parameter_names() == frozenset({"http_request"}) + def test_component_copy_locked_names_mark_only_declared_parameters(self) -> None: + class _Locked(_Plain): + COPY_LOCKED_PARAMETER_NAMES = frozenset({"note"}) + + params = {p.name: p for p in derive_parameters(cls=_Locked)} + assert params["note"].copy_locked + assert not params["mode"].copy_locked + assert not any(p.copy_locked for p in derive_parameters(cls=_Plain)) + def test_order_follows_signature(self) -> None: names = [p.name for p in derive_parameters(cls=_Plain)] assert names == ["count", "ratio", "mode", "note"]