SequentialAttack reports FAILURE when no child attack reached a verdict, contradicting attack_outcome_from_score's stated contract

Open
#2,658 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Assessment

Difficulty
3/5
Estimated time
1-2 days
Newbie friendliness
68/100
Issue type
Bug
Clarity
Mostly clear
Activity status
Active
Tech stack
python
Domain
ai, security

Research direction

Start in pyrit/executor/attack/compound/sequential_attack.py by tracing _compute_outcome for each completion policy, then read the existing TestOutcomeDerivation cases in tests/unit/executor/attack/compound/test_sequential_attack.py. Compare those expectations with the attack_outcome_from_score contract and the supplied stubbed-child reproduction. Done means the intended contract is resolved and the relevant tests consistently reflect the chosen outcome semantics.

Written by the indexing model from the issue text.

Description

Bug: triage
Describe the bug

attack_outcome_from_score in pyrit/executor/attack/core/attack_strategy.py states the contract in its own docstring:

This is the attack-side contract for undetermined scores, stated once so no attack invents its own. An undetermined score is neither achievement nor refutation, so it never reads as failure; an attack that ends on one ends undetermined.

The leaf attacks honour it. PromptSendingAttack._determine_outcome and MultiPromptSendingAttack._determine_outcome both carry UNDETERMINED through explicitly rather than letting it fall into the failure branch, and CrescendoAttack and RedTeamingAttack route through the helper directly.

SequentialAttack._compute_outcome in pyrit/executor/attack/compound/sequential_attack.py does not. Both aggregation paths end in an unconditional return AttackOutcome.FAILURE, so any sequence that produced no SUCCESS and was not entirely ERROR is reported as a failure, whether or not a single child attack actually refuted the objective.

It is the difference between a target that refused and a harness that could not tell, and downstream those two are counted the same: scenario_run_service.py computes succeeded = sum(result.outcome == AttackOutcome.SUCCESS ...), so everything not SUCCESS lands in one bucket and the scenario run page shows it as a clean negative.

This matters more as scorers become more willing to abstain. A scorer with a calibrated abstain band is built to return undetermined on the responses it is least sure about, and those are exactly the responses a red-team report should not quietly record as safe.

Steps or Code to Reproduce

Appended to tests/unit/executor/attack/compound/test_sequential_attack.py, reusing that file's own helpers:

@pytest.mark.usefixtures("patch_central_database")
async def test_undetermined_children_should_not_be_a_failure(target, seed_group):
    strategies = [
        _make_strategy(outcomes=[AttackOutcome.UNDETERMINED], name="a"),
        _make_strategy(outcomes=[AttackOutcome.UNDETERMINED], name="b"),
    ]
    child_attacks = [SequentialChildAttack(strategy=s, seed_group=seed_group) for s in strategies]
    compound = SequentialAttack(
        objective_target=target,
        child_attacks=child_attacks,
        completion_policy=SequenceCompletionPolicy.EXHAUSTIVE,
    )
    patcher, _ = _patch_run_child_attack(strategies_by_id={id(s): s for s in strategies})
    with patcher:
        result = await compound._perform_async(context=_make_context())
    assert result.outcome is AttackOutcome.UNDETERMINED
Expected Results

AttackOutcome.UNDETERMINED. Neither child attack refuted the objective, so the envelope has no grounds to say one did.

Actual Results

AttackOutcome.FAILURE. Sweeping every policy through _perform_async with stubbed child attacks, on main at a2d8675:

policy child outcomes envelope outcome
first_success UNDETERMINED, UNDETERMINED failure
first_decisive UNDETERMINED, UNDETERMINED failure
strict_all SUCCESS, UNDETERMINED failure
exhaustive UNDETERMINED, UNDETERMINED failure
exhaustive UNDETERMINED, ERROR failure
exhaustive UNDETERMINED, FAILURE failure
last_result UNDETERMINED undetermined

The last two rows are controls. EXHAUSTIVE with a real FAILURE among the children should be a failure and is, so the rule is not broken everywhere. LAST_RESULT is the only policy that preserves an undetermined verdict, and only because it inherits the final child's outcome verbatim rather than aggregating.

Suggested direction

Reach FAILURE only when a child actually refuted the objective, and fall through to UNDETERMINED otherwise. For the any-success policies:

if any(r.outcome is AttackOutcome.SUCCESS for r in results):
    return AttackOutcome.SUCCESS
if all(r.outcome is AttackOutcome.ERROR for r in results):
    return AttackOutcome.ERROR
if any(r.outcome is AttackOutcome.FAILURE for r in results):
    return AttackOutcome.FAILURE
return AttackOutcome.UNDETERMINED

and the same ordering for STRICT_ALL after its existing SUCCESS and ERROR checks. Any sequence containing a real FAILURE still returns FAILURE.

Whether _should_stop_after should also treat UNDETERMINED distinctly under STRICT_ALL is a separate question and I have not proposed anything there. Stopping on a non-SUCCESS is defensible for pipeline semantics; it is the label the envelope then reports that looks wrong.

One thing worth flagging before anyone acts on this

TestOutcomeDerivation already carries two cases asserting the current behaviour: EXHAUSTIVE over [UNDETERMINED, UNDETERMINED] and STRICT_ALL over [SUCCESS, UNDETERMINED], both expecting FAILURE. So this was decided once, and any fix flips two expectations that are there on purpose.

Either compound attacks are deliberately exempt from the contract attack_outcome_from_score says is "stated once so no attack invents its own", in which case the helper's docstring is what needs correcting, or the exemption was not intended. The two cannot both stand, since they contradict each other inside the same package.

My reading is that the helper has it right and the aggregation should follow it, and I have opened a PR on that basis rather than leaving it as a question. If the intent was the other way round then the PR is the wrong fix and I would rather be told than guess.

Versions

PyRIT main at a2d8675, Python 3.11, Linux. I have not run a full SequentialAttack end to end against a live target with a genuinely abstaining scorer; everything above is the unit path with stubbed children.

Developed with AI assistance.

Dominant language
Python
Stars
4.5k
Forks
896
Avg merge
3d 7h
Merged PRs (30d)
155

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from microsoft/PyRIT

All issues in microsoft/PyRIT

Similar issues

More Python issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.