microsoft / microsoft/PyRIT

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
Bug: triage
Dominant language
Python
Stars
4.5k
Forks
893
Avg merge
3d 50m
Merged PRs (30d)
165

Description

#### 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:

```python
@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:

```python
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.

Contributor guide

No contributing guide indexed for this repository

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.

Assessment

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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.