FEAT: Generalize Colloquial Wordswap Attack Converter #1348
FEAT: Generalize Colloquial Wordswap Attack Converter #1348taherakolawala wants to merge 4 commits intoAzure:mainfrom
Conversation
|
@microsoft-github-policy-service agree |
romanlutz
left a comment
There was a problem hiding this comment.
Very nice! I am personally not convinced Singaporean should be the default BUT that's what we put in the issue description and it means current users won't be experiencing a breaking change. We may update that in the future, of course. Thanks a ton!
| def __init__( | ||
| self, deterministic: bool = False, custom_substitutions: Optional[Dict[str, List[str]]] = None | ||
| ) -> None: | ||
| def __init__(self, deterministic: bool = False, wordswap_path: Optional[str] = None) -> None: |
There was a problem hiding this comment.
Actually this is a breaking change. My gut feeling is that it is better to keep the existing signature but having the files is also not a bad idea (even though the substitutions aren't very many and probably not that meaningful.)
There was a problem hiding this comment.
Got it, I made some changes to the code so that the custom_substitutions parameter is back. The function signature is now the same as and it I think it should not be a breaking change for pre-existing code.
There was a problem hiding this comment.
Pull request overview
Generalizes ColloquialWordswapConverter to load colloquial word-substitution mappings from YAML files (with several new regional examples) while retaining support for direct in-code substitution dictionaries.
Changes:
- Add
wordswap_pathoption to load substitutions from YAML underpyrit/datasets/prompt_converters/colloquial_wordswaps/(defaulting tosingaporean.yaml). - Add new colloquial wordswap YAML datasets (Singaporean + multiple regional examples).
- Update and expand unit tests to cover YAML-based wordswaps and constructor argument conflicts.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
pyrit/prompt_converter/colloquial_wordswap_converter.py |
Loads substitutions from YAML via wordswap_path, adds conflict validation, and keeps custom-dict support. |
tests/unit/converter/test_colloquial_wordswap_converter.py |
Updates tests for YAML-based swaps and adds new scenarios/constructor validation tests. |
pyrit/datasets/prompt_converters/colloquial_wordswaps/singaporean.yaml |
Moves prior default substitutions into a dataset file. |
pyrit/datasets/prompt_converters/colloquial_wordswaps/filipino.yaml |
Adds Filipino example substitutions. |
pyrit/datasets/prompt_converters/colloquial_wordswaps/indian.yaml |
Adds Indian example substitutions. |
pyrit/datasets/prompt_converters/colloquial_wordswaps/southern_american.yaml |
Adds Southern American example substitutions. |
pyrit/datasets/prompt_converters/colloquial_wordswaps/multicultural_london.yaml |
Adds Multicultural London example substitutions. |
| # if neither custom_sub nor wordswap_path is given then singaporean substituions are used | ||
| file_path = ( | ||
| wordswap_directory / wordswap_path | ||
| if wordswap_path is not None | ||
| else wordswap_directory / "singaporean.yaml" | ||
| ) |
There was a problem hiding this comment.
wordswap_path is appended with / directly, so an absolute path (or a path with ..) can escape colloquial_wordswaps and load an arbitrary YAML file. Constrain this to files within the intended directory (e.g., resolve the candidate path and verify it is relative to wordswap_directory, and reject absolute paths / path traversal).
| # ensure that wordswap YAML is in the correct format. | ||
| if not isinstance(data, dict): | ||
| raise ValueError("Wordswap YAML must be a dict[str, list[str]] mapping words to substitutions") | ||
|
|
||
| self._colloquial_substitutions = data |
There was a problem hiding this comment.
Only the top-level YAML type is validated. If any substitution value is not a non-empty list[str], convert_async will crash (IndexError/random.choice) or produce non-string outputs. Validate that keys are strings (and normalize to lowercase if needed) and that each value is a non-empty list of strings before storing it.
| deterministic: bool = False, | ||
| *, |
There was a problem hiding this comment.
Constructor signature makes deterministic positional while the class now has multiple parameters. This is inconsistent with other converters in this repo that enforce keyword-only args for multi-parameter __init__ methods (e.g., LeetspeakConverter). Consider making deterministic keyword-only as well by placing it after *.
| deterministic: bool = False, | |
| *, | |
| *, | |
| deterministic: bool = False, |
| class ColloquialWordswapConverter(PromptConverter): | ||
| """ | ||
| Converts text into colloquial Singaporean context. | ||
| """ | ||
|
|
||
| SUPPORTED_INPUT_TYPES = ("text",) | ||
| SUPPORTED_OUTPUT_TYPES = ("text",) | ||
|
|
||
| def __init__( | ||
| self, deterministic: bool = False, custom_substitutions: Optional[Dict[str, List[str]]] = None | ||
| self, | ||
| deterministic: bool = False, | ||
| *, | ||
| custom_substitutions: Optional[Dict[str, List[str]]] = None, | ||
| wordswap_path: Optional[str] = None, | ||
| ) -> None: | ||
| """ | ||
| Initialize the converter with optional deterministic mode and custom substitutions. | ||
| Args: | ||
| deterministic (bool): If True, use the first substitution for each wordswap. | ||
| If False, randomly choose a substitution for each wordswap. Defaults to False. | ||
| custom_substitutions (Optional[Dict[str, List[str]]], Optional): A dictionary of custom substitutions to | ||
| override the defaults. Defaults to None. | ||
| custom_substitutions (Optional[Dict[str, List[str]]], Optional): A dictionary of | ||
| custom substitutions to override the defaults. Defaults to none. | ||
| wordswap_path (Optional[str]): Name of a YAML file located in the | ||
| PyRIT datasets prompt_converters/colloquial_wordswaps directory. | ||
There was a problem hiding this comment.
Docstring has minor issues: "Defaults to none" should be "Defaults to None", and the class description still claims it "Converts text into colloquial Singaporean context" even though the converter is now generalized. Please update these strings to reflect the new behavior and fix casing/typos.
| if custom_substitutions is not None: | ||
| self._colloquial_substitutions = custom_substitutions | ||
| else: | ||
| # if neither custom_sub nor wordswap_path is given then singaporean substituions are used |
There was a problem hiding this comment.
Typo in comment: "singaporean substituions" -> "singaporean substitutions".
| # if neither custom_sub nor wordswap_path is given then singaporean substituions are used | |
| # if neither custom_sub nor wordswap_path is given then singaporean substitutions are used |
| if not file_path.exists(): | ||
| raise FileNotFoundError(f"Colloquial wordswap file not found: {file_path}") | ||
| with file_path.open("r", encoding="utf-8") as f: | ||
| data = yaml.safe_load(f) | ||
| # ensure that wordswap YAML is in the correct format. | ||
| if not isinstance(data, dict): | ||
| raise ValueError("Wordswap YAML must be a dict[str, list[str]] mapping words to substitutions") |
There was a problem hiding this comment.
New error paths were introduced (missing YAML file, invalid YAML, invalid mapping format) but there are no unit tests covering these failure modes. Add tests that assert the specific exception types/messages for these branches to prevent regressions.
| if not file_path.exists(): | ||
| raise FileNotFoundError(f"Colloquial wordswap file not found: {file_path}") | ||
| with file_path.open("r", encoding="utf-8") as f: | ||
| data = yaml.safe_load(f) |
There was a problem hiding this comment.
The docstring says invalid YAML raises ValueError, but yaml.safe_load can raise yaml.YAMLError which will currently escape. Catch yaml.YAMLError and re-raise a ValueError (ideally including the file path) to match the documented behavior.
| data = yaml.safe_load(f) | |
| try: | |
| data = yaml.safe_load(f) | |
| except yaml.YAMLError as exc: | |
| raise ValueError(f"Invalid YAML format in wordswap file: {file_path}") from exc |
| mother: ["mumsy", "moms", "mummy", "ma"] | ||
| grandfather: ["grandad", "gramps"] | ||
| grandmother: ["nan", "nanna", "gran"] | ||
| girl: ["gyal", "ting", "shorty", " peng ting"] |
There was a problem hiding this comment.
The newly-added YAML value includes a leading space (" peng ting"), which will introduce unexpected spacing in outputs. Remove the leading whitespace so substitutions are clean.
| girl: ["gyal", "ting", "shorty", " peng ting"] | |
| girl: ["gyal", "ting", "shorty", "peng ting"] |
| self._colloquial_substitutions = data | ||
|
|
||
| self._deterministic = deterministic | ||
|
|
||
| def _build_identifier(self) -> ConverterIdentifier: |
There was a problem hiding this comment.
Converter behavior now depends on the selected wordswap source (custom dict vs specific YAML file), but the identifier logic doesn't currently incorporate that source. Please update the converter identifier so two instances with different wordswap_path/substitutions don't end up with the same identifier (e.g., include wordswap_path or a stable hash of the substitutions content).
|
|
||
| import yaml | ||
|
|
||
| from pyrit.common.path import DATASETS_PATH |
There was a problem hiding this comment.
This builds the prompt-converter dataset path manually via DATASETS_PATH / "prompt_converters" .... Most other converters use the dedicated CONVERTER_SEED_PROMPT_PATH constant for this (e.g., pyrit/prompt_converter/atbash_converter.py:7 and pyrit/prompt_converter/tone_converter.py:9). Consider switching to CONVERTER_SEED_PROMPT_PATH here for consistency and to avoid repeating directory segments.
Description
In accordance with the Issue #418 the converter in
colloquial_wordswap_converter.pyhas been generalized to use different versions of colloquial word swaps.A new directory has been created in
pyrit/datasets/prompt_converterscalledcolloquial_wordswaps. This directory contains the original default Singaporean word substitutions as wells as a few different regional colloquial word swap YAML examples. TheColloquialWordSwapConverterclass now accepts a new parameter calledwordswap_pathduring initialization. It defaults tosingaporean.yamlhowever the argument can be filled with any YAML file located in thecolloquial_wordswapsdirectory mentioned before.In the same vein, if you want to add a new set of word substitutions, all you need to do is create a new YAML file in the same format as any of the others and add it to the
pyrit/datasets/prompt_converters/colloquial_wordswapsdirectory. Here is an example of how a YAML file must be formatted:The following is an example initialization of the converter with a non-default wordswapper YAML:
People of whom this PR may be of interest to: @eugeniavkim @romanlutz
Closes #418
Tests and Documentation
test_colloquial_wordswap_converter.pyfor the generalizable wordswap converter.test_colloquial_wordswap_converter.pyfor checking multiple word custom word swapper conversions.uv run pytest tests/unit/converter/test_colloquial_wordswap_converter.py