fix(guardrail): do not block prose whose words fuse into a blocklist entry - #177
fix(guardrail): do not block prose whose words fuse into a blocklist entry#177rwagwani wants to merge 4 commits into
Conversation
Blocklist.censor_prompt decided whether anything had been censored by
searching its own output for the censor character, which was produced by
misc.Color.red("*"). termcolor strips ANSI escapes when stdout is not a
TTY, so in any run whose output was piped to a file the sentinel degraded
to a bare "*" and the check silently became "does the text contain an
asterisk?".
Text containing Markdown emphasis therefore blocked itself. The Cosmos3
Edge reasoner emits Markdown, so its own "**bold**" headings were reported
as blocked content with no blocklist match involved -- and the safety
verdict differed between an interactive terminal and a log file.
Measured on a 492-item Edge reasoner QA sweep: 7 outputs contained "**"
and all 7 were blocked; no output containing "**" escaped blocking. Four
further items were blocked on genuine blocklist hits. Of 16 items
describing a humanoid robot, the 15 written in plain prose passed and the
one that emitted "**humanoid robot**" was blocked.
Detect censorship with a sentinel that cannot occur in ordinary text and
keep the colourised character for display only, so the reported "Censored
Prompt" is unchanged. Adds regression tests covering Markdown emphasis,
bare asterisks, and a real blocklist hit alongside Markdown; these build
the matcher directly so they need no checkpoint download.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
547f048 to
ed3eccb
Compare
lfengad
left a comment
There was a problem hiding this comment.
Thanks for splitting these out — the deskin false positive is real and the read of any_next_words_form_swear_word is spot on.
My one structural concern: _has_legitimate_match re-derives matching independently of the library, so anywhere it's narrower than the library, a genuine hit gets silently let through. I found two such spots while testing — left them inline. Both are cases where stock better_profanity blocks and this branch doesn't, so they're bugs rather than part of the trade you documented.
On the trade you did document: I actually think it's reasonable for this list. It's mostly trademarks, and nobody evades a trademark filter by writing n ike — they just pick another word. So fused matching wasn't buying much here to begin with, which is also why I'd rather not pay for it with a heuristic.
Which brings me to your own closing suggestion — pruning or lengthening deskin. I think that's the better fix and the two findings below are arguments for it: any version of this heuristic re-derives the library's matching and will keep drifting from it. Could we ask the list owners first? If they'll take the data change this PR can just be withdrawn.
| # apart from a match that only worked because the spaces were deleted. | ||
| self._blocklist_phrases = { | ||
| " ".join(w.lower().split()) for w in self.blocklist_words if " " in w | ||
| } |
There was a problem hiding this comment.
This is a plain string set, but the library stores entries as VaryingString with CHARS_MAPPING (o->0, s->5, ...). So a leet-spelled phrase match fails this check, and then can't match the fused branch either (that entry contains a space), and we let it through.
Measured with entry boston dynamics:
| text | stock | this branch |
|---|---|---|
a boston dynamics robot |
blocked | blocked |
a b0ston dynamics robot |
blocked | not blocked |
a boston dynamic5 robot |
blocked | not blocked |
Since the list is mostly trademarks, multi-word entries are exactly the ones that matter. Storing them the way the library does makes in dispatch to VaryingString.__eq__, same as the library's own full_word in censor_words:
self._blocklist_phrases = [
VaryingString(" ".join(w.lower().split()), char_map=self.profanity.CHARS_MAPPING)
for w in self.blocklist_words if " " in w
]I checked this restores both rows without re-blocking a desk in the background.
There was a problem hiding this comment.
Fixed — stored as VaryingString with CHARS_MAPPING, exactly as you wrote it. Reproduced both rows first:
'a b0ston dynamics robot' stock=blocked branch=NOT blocked
'a boston dynamic5 robot' stock=blocked branch=NOT blocked
Both block now, and a desk in the background still passes. Covered by test_leet_spelling_of_a_multi_word_entry_still_blocks.
| # Phrase matches are bounded by the longest blocklist entry, but a | ||
| # separator-stripped join fuses several tokens into ONE entry word, so it | ||
| # needs its own bound. Three matches the library's own lookahead. | ||
| max_window = max(self._max_blocklist_words, _MAX_JOIN_TOKENS) |
There was a problem hiding this comment.
This bound is smaller than the library's actual reach. any_next_words_form_swear_word fuses up to MAX_NUMBER_COMBINATIONS + 1 tokens, and that value is monotonic — load_censor_words only ever raises it — so the module-level profanity singleton already carries ~5 over from the default wordlist before the custom list loads.
So a 4-token split escapes: entry supercalifragil, text su per cali fragil — stock blocks, this branch doesn't.
You half-caught this in the description ("passed against the production list only because that list happens to contain a 6-word phrase"). It's still coupled to that accident: one entry with a hyphen or apostrophe raises the library's reach without raising _max_blocklist_words. Taking it from the source of truth pins it:
max_window = max(self._max_blocklist_words, self.profanity.MAX_NUMBER_COMBINATIONS + 1)(_MAX_JOIN_TOKENS on L37 then becomes unnecessary, and its "mirrors better_profanity's own lookahead" comment isn't accurate today.)
There was a problem hiding this comment.
Fixed — max_window = max(self._max_blocklist_words, self.profanity.MAX_NUMBER_COMBINATIONS + 1), and _MAX_JOIN_TOKENS is gone along with its inaccurate comment.
Worth recording that reproducing this corrected my own repro first: with every fragment marked a dictionary word, su per cali fragil now is rejected by the prose exemption rather than by the window, so the case only isolates the window when at least one part is not a dictionary word. test_join_window_follows_the_library_reach is written that way.
| bl.whitelist_words = [] | ||
| bl.guardrail_exempt_fused_prose = True | ||
| bl._dictionary_cache = dict(dictionary or {}) | ||
| bl._max_blocklist_words = max((len(w.split()) for w in words), default=1) |
There was a problem hiding this comment.
Worth flagging that both bugs above live in lines no test executes — the helper goes through __new__ and re-implements __init__'s bookkeeping here, so _max_blocklist_words and _blocklist_phrases are only ever exercised in their test copy.
Pulling those three assignments into a small _configure_join_bookkeeping() called from both __init__ and this helper would keep the tests pointed at the real thing. Otherwise fixing the two findings won't actually be covered.
There was a problem hiding this comment.
Fixed — the three assignments moved into _configure_join_bookkeeping(), called from both __init__ and the test helper, so the tests now exercise production's derivation.
You were right that it mattered rather than being tidiness: all three findings live in lines no test executed, and the helper's copy would have kept passing after the fixes landed.
| self, | ||
| guardrail_partial_match_min_chars: int = 6, | ||
| guardrail_partial_match_letter_count: float = 0.4, | ||
| guardrail_exempt_fused_prose: bool = True, |
There was a problem hiding this comment.
Small thing: this isn't reachable from anywhere in practice — presets.py:17 builds Blocklist() with no arguments, and that's the only real construction site. So "set it False to restore the stricter behaviour" currently means editing code. If it's meant as a genuine escape hatch it needs a config path; if not, probably worth saying so in the docstring.
Separately: flipping a guardrail's default to the laxer setting for every caller feels like a call for the guardrail owners rather than something to carry in on a false-positive fix. Might be worth looping them in regardless of which fix we land.
There was a problem hiding this comment.
Fixed the half that is mechanical: the flag now falls back to COSMOS_GUARDRAIL_EXEMPT_FUSED_PROSE when not passed explicitly, so strict matching is selectable at deploy time without editing source.
COSMOS_GUARDRAIL_EXEMPT_FUSED_PROSE=0
Your second point stands and I have not tried to settle it here — whether the relaxed setting should be the default for every caller is the guardrail owners' call. It is now called out in the PR body rather than left implicit.
|
One more thing, separate from the two bugs — I want to state the net effect on strictness plainly, because I don't think it should ride in implicitly on a false-positive fix. Even with both findings above fixed, this PR still makes the blocklist less strict by default for every caller. Three layers, only the first of which is in the description:
None of this is an argument that the false positive should stay — it's real and worth fixing. It's an argument about where. Removing If we do land the code version instead, could we loop in whoever owns the guardrail list and config so the default change is signed off rather than inherited? Happy either way, I just don't want it to land unremarked. |
|
|
||
| whitelist = {w.lower() for w in self.whitelist_words} | ||
| raw_tokens = input_prompt.split() | ||
| tokens = [t.strip(string.punctuation).lower() for t in raw_tokens] |
There was a problem hiding this comment.
Sorry, one more — I went back and enumerated the gap systematically (disabling the WordNet exemption so that anything left is structural rather than the trade you intended), and this line turns up a third case.
string.punctuation and better_profanity's ALLOWED_CHARACTERS overlap on " $ ' * @ — and those are exactly the leet characters the library substitutes (a->@, o->@, s->$, and * for a/i/o/u/v/e). So a token spelled with one of them gets stripped to "" here, and then not all(window) breaks out of the window loop and the whole join branch is abandoned:
entry 'sike':
'wear s ike shoes' -> stock: blocked, this branch: blocked
'wear $ ike shoes' -> stock: blocked, this branch: NOT blocked
This one is independent of the window-size finding — it still reproduces with a production-sized window of 6 — so it'd survive fixing L209.
Stripping only the characters the library doesn't consider part of a word keeps them visible:
_STRIP_CHARS = "".join(set(string.punctuation) - ALLOWED_CHARACTERS)
...
tokens = [t.strip(_STRIP_CHARS).lower() for t in raw_tokens]I checked that this re-blocks wear $ ike shoes without re-blocking a desk in the background.
While I was in there: a to yota car with to on the whitelist also slips through, but that one reproduces on main too — uncensor_whitelist indexes censored_words[i] positionally while the library collapses to yota into a single replacement token, so the whitelist pass wipes the marker out before your code is ever reached. Pre-existing, not yours; I'll file it separately. (It can also IndexError when a whitelisted word sits after a collapsed match, which is worth a look on its own.)
There was a problem hiding this comment.
Fixed. Confirmed the overlap is exactly what you said:
set(string.punctuation) & ALLOWED_CHARACTERS -> " $ ' * @
Tokens are now stripped with _STRIP_CHARS = "".join(sorted(set(string.punctuation) - ALLOWED_CHARACTERS)). wear $ ike shoes blocks again, wear s ike shoes still blocks, a desk in the background still passes. Covered by test_leet_characters_are_not_stripped_from_tokens.
On the to yota case you flagged as pre-existing — it is, and it is worse than "slips through". With the production lists (whitelist is the single word flat, custom has 95 multi-word entries) the index shift also corrupts the message and can crash:
'a Snow White poster on a flat wall' -> 'a **** poster on a flat flat' # 'wall' overwritten
'Snow White is flat' -> IndexError: list assignment index out of range
'a Snow White poster is flat' -> IndexError
Reproduced on main, no code changes. Happy for you to file it, or I can — say which.
…ring to_ascii preserves \x00 (its range is [^\x00-\x7F]), so a NUL arriving in the input reached the sentinel check unmodified and reported the prompt as censored with no blocklist match -- the same in-band-marker failure the sentinel replaced. Remove it rather than substitute a space, so the sentinel cannot be used to split a blocked word into two innocuous tokens: "n\x00ike" fuses back to a blocked word instead of becoming "n ike". Verified over the 492-item Edge reasoner QA corpus against the production Cosmos-Guardrail1 word lists: identical verdicts to the previous commit on every item. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…entry
better_profanity concatenates adjacent words with their separators removed
(utils.any_next_words_form_swear_word: full_word = cur_word + single_word)
so that a blocked word written with a space inserted mid-word is still
caught -- "n ike" for the entry "nike". The side effect is that ordinary
prose collides with short blocklist entries: "a desk in the background"
fuses into "deskin", which is on the list, and the prompt is blocked as
gore.
Note this is not governed by guardrail_partial_match_min_chars -- that
parameter applies to check_partial_match against the exact-match list,
which is a different code path.
Re-check a flagged prompt with word boundaries respected before blocking.
A match counts as legitimate when a single whitespace-delimited token
matches on its own (so joins across punctuation inside a token, such as
"desk-in", still block), when a run of tokens matches a blocklist entry
that genuinely contains spaces ("Boston Dynamics"), or when a run matches
only once the spaces are deleted AND at least one part is not ordinary
English -- the shape of a real evasion. Single letters do not count as
ordinary English, so "n ike" and "to yota" still block.
Matching is delegated to better_profanity throughout, so leet
substitutions and punctuation behave exactly as before. The re-check runs
only after something has already matched, so unflagged text costs nothing;
on a flagged 106-token output it adds ~43 ms.
_has_legitimate_match documents what the check does and does not buy: the
escape class this introduces (a banned word split into pieces that are all
multi-letter dictionary words), the escape classes that already existed
(with the entry "nike", stock code blocks "n ike" and "ni ke" but not
"nik e" or "n i k e", because the library looks ahead only two words), that
WordNet is the trust boundary and over-includes single letters, and that
the blocklist is a coarse pre-filter running alongside model based
guardrails.
Adds guardrail_exempt_fused_prose (default True). Set False to restore the
stricter previous behaviour: ordinary prose is blocked again, but no fused
match is ever let through.
Measured over the 492-item Edge reasoner QA corpus: blocks drop from 11 to
the 3 genuine trademark hits (INTEL, Nike, Huracan) with no new blocks;
strict mode adds the fused case back. Adds six regression tests covering
the fused-prose false positive, split evasions, single-letter splits,
multi-word phrases, punctuation joins, and the opt-out; they inject the
dictionary rather than reading WordNet, so they need no checkpoint and no
nltk corpus.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… behind the library Review of NVIDIA#177 found three cases where _has_legitimate_match re-derived the library's matching more narrowly than the library itself, so a prompt stock better_profanity blocks was let through. * Multi-word blocklist entries were held as plain strings, while the library stores them as VaryingString with CHARS_MAPPING. "b0ston dynamics" and "boston dynamic5" escaped. Store them the library's way so comparison dispatches to VaryingString.__eq__. * The join window used a local constant of 3. MAX_NUMBER_COMBINATIONS is monotonic across load_censor_words calls, so the matcher's real reach is wider and a 4-token split escaped. Take the bound from the matcher. * Tokens were stripped with string.punctuation, which overlaps the library's ALLOWED_CHARACTERS on " $ ' * @ -- the characters it substitutes for letters. A token spelled with one emptied and aborted the window scan, so "wear $ ike shoes" escaped. Strip only what the library ignores. Also: * Extract _configure_join_bookkeeping() and call it from both __init__ and the test helper, so tests exercise production's derivation rather than a copy of it. All three fixes above live in lines no test previously executed. * Resolve guardrail_exempt_fused_prose from COSMOS_GUARDRAIL_EXEMPT_FUSED_PROSE when not passed explicitly, so strict matching is selectable without editing code -- presets.py constructs Blocklist() with no arguments. * Correct the docstring's claim about the library's reach. Stock blocks "nik e" and "n i k e" too once a following token closes the window; the earlier claim was measured on fragments with no trailing word. Verified against the 492-item Edge reasoner QA corpus with the production Cosmos-Guardrail1 lists: 3 blocked, all genuine trademark hits, with 4_3 ("sitting at a desk in the background") the only item the exemption removes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ed3eccb to
4953c1b
Compare
|
Thanks for the depth here — all three findings reproduced exactly as you described, and all three are fixed. Pushed as
Plus the bookkeeping extraction, so those lines are now under test, and an environment path for the strict setting. Tests are 18 passed. Over the 492-item QA corpus with the production lists: 3 blocked, all genuine trademark hits, with A correction to this PR's own description, which cuts against it. While reproducing your finding about So stock is stricter than I documented, and this PR gives up more than it claimed. The four forms do all still block with the PR against the production WordNet — but only because the corpus has no synset for On the strictness comment. I have not tried to resolve it in code, because I do not think it is a code question. The mechanical half is done — strict mode is now reachable via My own view has not changed from the closing paragraph: pruning or lengthening |
|
Withdrawing this in favour of the data fix, as offered in the description and as you've recommended twice. Before I close it, here is the measurement that decided it — because it says the trade is considerably worse than either of us assessed, and the reasoning is worth keeping on the record. The exposure is not confined to the trademark entriesWe both reasoned from "it's mostly trademarks, and nobody evades a trademark filter by writing Using entries that are safe to quote here, the shape is: The concerning part is what else is in those 110. A substantial share are the content-safety entries — the explicit and graphic-violence terms the filter exists for — not the IP list. I have not quoted them for obvious reasons; the script that regenerates the list is available internally if you want to see it in full. The trademark entries are the collateral here, not the substance. A zero-width space makes it invisible
This reproduces on the content-safety entries identically. Nothing visible distinguishes the evading prompt from the plain one, which is what makes it worse than the documented "type a space" case. Introduced here — stock blocks all of them. Narrowing the heuristic does not reach itI prototyped capping the exemption at 2-token joins, which was the most promising way to keep the code fix. It buys back What this PR was buyingOne false positive, The data fix, and a wider askRemoving Worth raising with the list owners as a rule rather than one deletion, though: 114 of 288 single-word entries split into two dictionary words, so the same collision shape recurs with the next short entry added. A hygiene check at list-edit time ("does this entry fuse out of two common English words?") prevents the class. I'm happy to hand over the script that produces the list. AlsoTwo things from this review that outlive the PR:
Thanks for the depth on this one — three real bugs plus the structural objection, and the objection turned out to be the important half. |
|
Closing per the above — withdrawn in favour of the data fix. The branch stays on the fork if the measurement or the three fixes are wanted later. |
|
Thanks for this — and for going and measuring it instead of taking the easy exit. Worth saying plainly: the premise you overturned was mine. I'm the one who wrote that the trade looked reasonable "for this list as it stands today," on the reasoning that it's mostly trademarks and nobody evades a trademark filter by writing So my structural objection was pointed the right way but argued weakly — I said the threat model didn't hold, when in fact the exposure was about an order of magnitude larger than I assumed. Your numbers, not mine. On the two leftovers:
Sorry for the volume of review on something that ends up withdrawn. It was worth doing. |
|
Closing the loop on this thread, since the follow-up work is now open elsewhere. First, thank you for the review here and on #176. Pushing back on the exemption rather than tuning it was the right call: measured against the shipped list, the version proposed here made 108 of the 285 single-word entries evadable by inserting one space, and capping the exemption at two-token joins reached none of them. The review also turned up the invisible-character case, which is what ruled the approach out for good. Where each of the asks landed:
Reviews on #183 and #186 would be very welcome whenever you have time. |
fix(guardrail): do not block prose whose words fuse into a blocklist entry
What is wrong
better_profanityconcatenates adjacent words with their separators removed, so that ablocked word written with a space inserted mid-word is still caught —
"n ike"for the entrynike:The side effect is that ordinary prose collides with short blocklist entries.
"a desk in the background"fuses intodeskin, which is on the blocklist alongsidedeskinnedanddeskinning, so a sentence describing office furniture is blocked as gore.This is not governed by
guardrail_partial_match_min_chars; that parameter applies tocheck_partial_matchagainst the exact-match list, which is a different code path.Observed in an Edge reasoner QA sweep: item
4_3, whose output describes "a robot standingin a library, carrying a stack of books, with a desk in the background".
The fix
Before blocking, re-check the flagged prompt with word boundaries respected. A match counts
as legitimate when:
token, such as
"desk-in", blocking;"Boston Dynamics";ordinary English — the shape of a deliberate evasion.
A space-deleted join whose every part is an ordinary word is rejected as coincidence.
Single letters do not count as ordinary English even though WordNet knows them, because a
lone letter beside another word is what an evasion looks like, not normal writing. That keeps
"n ike"blocking.Matching is delegated to
better_profanitythroughout, so leet substitutions and punctuationbehave exactly as before. WordNet is consulted only to classify a word as ordinary English;
if the corpus is unavailable the answer is "not ordinary", which keeps the filter at least as
strict as it is today.
Behaviour
a desk in the backgrounda desk in front of a bookshelfn ike shoes(evasion)to yota cars(evasion)desk-in the corner(punctuation join)a Boston Dynamics robot(multi-word entry)the Nike logo(single-word entry)deskin(the entry itself)Swept over all 492 outputs of the Edge reasoner QA corpus: blocks drop from 11 to the 3
genuine trademark hits (
INTEL,Nike,Huracan), with no new blocks. The other fiveof the original eleven are cleared by #176.
Cost
The re-check runs only after something has already matched, so text that was never flagged
costs nothing. On a flagged 106-token output the added work is ~43 ms, against ~340 ms
for the surrounding
is_safe()call, which is dominated by the pre-existing nltk tokeniseand lemmatise pass.
Tests
Ten regression tests added, covering the fused-prose false positive, split evasions,
single-letter splits, multi-word phrases, punctuation joins, leet spellings of multi-word
entries, the join window's bound, leet characters inside tokens, and the strict-mode opt-out.
They inject the dictionary instead of reading WordNet, so they are hermetic — no checkpoint
download, no nltk corpus — and they exercise the heuristic directly rather than through
whatever the corpus happens to contain.
The helper builds its state by calling
_configure_join_bookkeeping(), the same method__init__uses, so the join bookkeeping under test is production's derivation rather than acopy of it.
One of these tests caught a real bug during development: the join window was initially bounded
by the longest blocklist entry's word count, which is 1 for a single-word list, so pairs were
never examined. It passed against the production list only because that list happens to
contain a 6-word phrase. Join detection now carries its own bound.
Escape class, stated plainly
This narrows evasion detection, and the trade is documented in
_has_legitimate_matchratherthan left implicit:
Introduced here. A banned word split into pieces that are all multi-letter dictionary
words is no longer treated as an evasion —
"assassin"written as"ass ass in"would pass.CORRECTED. An earlier revision of this section claimed fused matching "never reached far
to begin with" — that with the entry
nike, stock blocked"n ike"and"ni ke"but not"nik e"or"n i k e". That was measured on fragments with no trailing token, and it iswrong. The library's window needs a following word to close; once there is one, stock blocks
all four forms:
n ike shoeni ke shoenik e shoen i k e shoeSo stock is stricter than this PR originally described, and the exemption below gives up more
than was first stated. Measured against the production WordNet corpus, all four rows still
block with this PR — but only because WordNet has no synset for
ke,nik,kalone as afused part, and so on. That is the trust boundary in the next bullet doing the work, and it is
the honest reason those rows survive, not a property of the heuristic itself.
WordNet is the trust boundary. "Ordinary English" means "WordNet has a synset", which
over-includes: it counts single letters such as
nandfas words. That is why_is_prose_wordrequires two characters — without it,"n ike"would escape.Layering. The blocklist is a coarse pre-filter running alongside model-based guardrails
(
llamaGuard3,qwen3guard, video content safety). It is not the last line of defence,which is what makes this trade reasonable rather than reckless.
Opt-out
guardrail_exempt_fused_proseswitches the behaviour. Set itFalseto restore the stricterprevious behaviour: ordinary prose is blocked again, but no fused match is ever let through.
Verified both ways over the QA corpus — the default blocks the 3 genuine trademark hits, strict
mode adds
4_3back.Because
presets.py:17constructsBlocklist()with no arguments, the flag also readsCOSMOS_GUARDRAIL_EXEMPT_FUSED_PROSEwhen it is not passed explicitly, so strict matching isselectable at deploy time rather than only by editing source:
That closes the "no off switch in practice" half of the strictness question. The other half —
whether the relaxed setting should be the default for every caller at all — is a decision for
the guardrail owners, not something this PR should settle on its own. Flagging it explicitly
rather than letting it ride in on a false-positive fix.
If the maintainers would rather not carry a heuristic here at all, there is a cleaner option
worth considering instead of this PR: fix the data.
deskinis a rare gore stem that isalso the fusion of two of the commonest English words, sitting on a list that is otherwise
trademarks. Pruning or lengthening entries like it removes the false positive with no loss of
evasion resistance, which is strictly better than any code-side heuristic. I am happy to
withdraw this in favour of that if the list owners will take it.
🤖 Generated with Claude Code