fix(detectors): guard against non-string output.text in AttackRogueString

[lavkeshdwivedi]

Summary

When a REST generator returns a structured JSON response, output.text can be a dict rather than a str, causing a crash in promptinject.AttackRogueString.detect():

AttributeError: 'dict' object has no attribute 'lower'
  File "garak/detectors/promptinject.py", line 39, in detect
      trigger, output_text = trigger.lower(), output_text.lower()

Root cause

REST generators that return structured JSON populate output.text with the parsed dict. The subsequent .lower() call in the detector then crashes.

Fix

Coerce non-string responses to str in generators/rest.py before wrapping them in Message objects. This is the correct layer since the type contract should be enforced at the generator boundary:

return [Message(str(r) if r is not None and not isinstance(r, str) else r) for r in response]

This ensures Message.text is always str | None as the type annotation promises, without changing detector logic.

Test plan

• Run promptinject.HijackKillHumans against a REST endpoint that returns a JSON dict; verify no AttributeError is raised
• Existing string outputs still scored correctly

Fixes #1888.

[jmartin-tech]

The signature for detect accepts Attempt objects and output here should be a Message object on that Attempt which should have a text attribute that is defined being str or None.

Defensive guards are used to validate typing, however I am not sure this is the appropriate solution here. The issue reported used the rest generator which may be the more appropriate location to coerce the value to str.

[lavkeshdwivedi]

Thank you for the direction — you're right. Moved the fix to RestGenerator._get_response() where Message objects are constructed. The coercion now happens at line 406: Message(str(r) if r is not None and not isinstance(r, str) else r), which upholds the Message.text: str | None contract before any detector sees the value. Reverted the isinstance guard from the detector entirely.

[lavkeshdwivedi]

The remaining Windows CI failure (build_windows (3.10/3.12)) is on tests/generators/test_rest_mtls_integration.py::test_mtls_verify_ssl_false, which:

• Pre-exists in main: the test was added in commit af5564f before this PR was opened (git show origin/main:tests/generators/test_rest_mtls_integration.py confirms it)
• Is unrelated to our change: our diff only touches garak/generators/rest.py line 406 (JSON response coercion) and garak/detectors/promptinject.py. Neither code path is exercised by the mTLS SSL-verification test.
• Is a network flake on Windows: the failure is RemoteDisconnected('Remote end closed connection without response') at the HTTP level, not an assertion error. Linux and macOS builds pass cleanly (4582 tests each).

Willing to add @pytest.mark.skip(reason="flaky on Windows CI") to that test if the maintainers prefer, but since it lives outside our change set I wanted to flag it here first.

[jmartin-tech]

Tests have been re-triggered, the failure was env related and does not warrant change being to mark as flaky as this is one of the first times it has failed due to a networking issue outside the control of the test itself.

[lavkeshdwivedi]

Thanks for the review @jmartin-tech. The current diff already implements the approach you suggested, coercing non-string responses to str in generators/rest.py before creating Message objects, rather than adding a guard in the detector:

return [Message(str(r) if r is not None and not isinstance(r, str) else r) for r in response]

The promptinject.py change in this PR is just a blank-line removal (no logic change). The PR body has been updated to reflect this approach. Let me know if you'd prefer a different shape.

[lavkeshdwivedi]

All CI now green including the previously-flaky build_windows job (re-triggered run 28871026420 — all 3 Windows matrix variants pass). DCO is clean. The current diff implements exactly what you recommended: coercing non-string responses to str in generators/rest.py before wrapping in Message objects. Let me know if you need any further adjustments.

[lavkeshdwivedi]

Hi @jmartin-tech, I've addressed your feedback. Could you re-review when you get a chance?

[lavkeshdwivedi]

Added two regression tests in tests/generators/test_rest.py (latest commit):

• test_json_rest_non_string_response_coerced_to_str — parametrized over int/float/bool, mocks a JSON response where response_json_field extracts a non-string scalar and asserts the output is [Message(str(value))]
• test_json_rest_non_string_list_coerced_to_str — mocks a JSONPath multi-value response ($.scores[*]) returning [1, 2, 3] and asserts each is wrapped as [Message("1"), Message("2"), Message("3")]

Both tests call _call_model directly through requests_mock so they exercise the production code path in _get_response without mocking the coercion logic itself.

[lavkeshdwivedi]

All CI is green across all platforms (Linux, macOS, Windows, arm64) and Python versions (3.10–3.13), including DCO. The two regression tests requested are in place. Could you re-review when you get a chance?