FIX: stop seeded converters from reseeding the global RNG - #2397
FIX: stop seeded converters from reseeding the global RNG#2397Vishnu Rajeev (VishnuR23) wants to merge 1 commit into
Conversation
ZalgoConverter, ProportionSelectionStrategy and WordProportionSelectionStrategy called random.seed() on the process-wide RNG. Passing seed= to any one of them reset global random state on every conversion, so every other component drawing from the `random` module (~17 modules, including CharSwapConverter, RandomCapitalLettersConverter, InsertPunctuationConverter and seed sampling) silently stopped varying. Each of the three now owns a random.Random instance instead. Seeded output is byte-identical to before; only the global side effect is removed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Vishnu Rajeev (@VishnuR23) please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.
Contributor License AgreementContribution License AgreementThis Contribution License Agreement (“Agreement”) is agreed to by the party signing below (“You”),
|
| # converter yields the same output on every call. | ||
| if self._seed is not None: | ||
| random.seed(self._seed) | ||
| self._rng.seed(self._seed) |
There was a problem hiding this comment.
This no longer guarantees the converter's documented reproducibility when Zalgo is composed with an unseeded random word selector. WordLevelConverter.convert_async() performs word selection after this reset, but WordProportionSelectionStrategy now owns an independent RNG, so repeated calls to ZalgoConverter(seed=42, word_selection_strategy=WordProportionSelectionStrategy(proportion=0.5)) can select different words and produce different outputs. Please either use one operation-local RNG across selection and mark generation, or explicitly define seeds as component-scoped and update the public contract accordingly. Add a regression test for this composed case.
|
|
||
| async def test_zalgo_seed_does_not_disturb_global_rng(): | ||
| """A seeded converter must not reseed the process-wide RNG.""" | ||
| random.seed(0) |
There was a problem hiding this comment.
This regression test mutates the process-wide RNG and leaves it seeded at 0, which can make later tests order-dependent. The same pattern appears in both new selection-strategy tests. No setup seed is needed here: capture the existing random.getstate(), exercise the component, and compare against that state. Alternatively, restore the original state in a finally block.
Description
Three components seed Python's process-wide RNG when given a
seed:ZalgoConverter.validate_input—random.seed(self._seed)ProportionSelectionStrategy.select_range(anchor="random") —random.seed(self._seed)WordProportionSelectionStrategy.select_words—random.seed(self._seed)Each then draws from the
randommodule itself. Becausevalidate_input/select_*run on every conversion, a single seeded instance resets global random state repeatedly, and roughly 17 modules underpyrit/draw from that same global RNG —CharSwapConverter,RandomCapitalLettersConverter,InsertPunctuationConverter,EmojiConverter,LeetspeakConverter,UnicodeConfusableConverter,SeedDatasetsampling, and others.The result: seeding one converter for reproducibility silently de-randomizes unrelated converters in the same process. For a red-teaming framework this quietly costs attack diversity — a campaign keeps re-testing the same variations while appearing randomized.
Reproduction on
main— an unrelated converter, alongside a seededZalgoConverter:Fix
Each of the three now owns a
random.Randominstance and reseeds that rather than the global module.random.Random(seed)yields the same sequence asrandom.seed(seed)plus the module-level functions, so seeded output is byte-identical to before — I verified this by capturing outputs for seeds 1/42/123 on both sides of the change and diffing them. Only the global side effect is removed.grep -rn "random\.seed(" pyrit/is now empty.Note this does change one edge case: an unseeded instance no longer inherits a user's global
random.seed(...). That path is what the per-componentseedargument is for, and relying on it is what caused the bug.Tests and Documentation
Three regression tests assert
random.getstate()is unchanged across a seeded call — precise and non-flaky, no reliance on sampling luck:test_zalgo_seed_does_not_disturb_global_rngTestProportionSelectionStrategy::test_select_range_seed_does_not_disturb_global_rngTestWordProportionSelectionStrategy::test_select_words_seed_does_not_disturb_global_rngAll three fail on
mainand pass with the fix (confirmed by reverting only the source changes and re-running:3 failed, 103 passed). Also addedtest_zalgo_seed_is_repeatable_on_same_instanceandtest_zalgo_unseeded_converters_stay_independentto pin both directions of the contract.One existing test needed updating:
test_char_swap_converter_proportion_unchanged_with_iterationspatchedrandom.sampleto control word selection, which worked only because the strategy called the global module. It now patches the strategy's own RNG; the assertion it exists for (selection happens once, not per iteration) is unchanged.Verification:
pytest -n 4 --dist=loadfile tests/unit-> 15130 passed, 121 skippedpytest tests/unit/converter-> 1125 passed, 34 skippedpre-commit run --files <changed>-> all hooks pass, includingruff format,ruff check, andtyNo documentation changes — internal RNG ownership only, no public API or notebook surface affected.