[Bug][P1] HITL resume can execute tool arguments the user never approved

[Asaf-prog]

Problem

A human approves one set of tool arguments, but the resumed run can execute a different set without asking again. The approval decision is consumed by the replayed interrupt's position, without checking that the replayed invocation matches the approved invocation.

Reviewed main at b9acf1e6cc74484120009f626cdb92b1616178c1. Reproduced with Python 3.12, langgraph 1.2.11 and langchain-core 1.6.2, using the real Extra engine and deterministic fake models. No live LLM or external tool was called.

Severity: P1 — this is a security/trust boundary. Human-in-the-loop approval is the control that stops an agent from taking an unreviewed action, and it can be bypassed with an observable side effect.

Reproduction and observed result

1. Configure a local record_value(message: str) tool with automatic approval disabled.
2. Use a fake model that requests message="reviewed-value" on the first model call and message="changed-value" when the node is replayed. This deterministically simulates a provider changing its output on a repeated request.
3. Start a run. The pending approval displays {"message": "reviewed-value"}; the tool has not executed.
4. Resume that approval with allow once.

Observed output:

approved=reviewed-value, executed=changed-value, returned status=completed

The tool's local output file contains changed-value. This is an observed side effect, not just incorrect response metadata.

Expected: execute only the exact approved invocation, or request fresh approval if the tool identity/arguments change. The old decision must never authorize the changed call.

Cause

• AgentNode.execute/_run re-enters the model loop during graph resume; intermediate model/tool messages are local rather than checkpointed as individual steps.
• InterruptApprovalProvider.request_decision creates/looks up the current invocation and then consumes interrupt(payload)'s decision without comparing it to the claimed approval.
• resume sends only {"decision": kind.value} as the resume value. The approved tool identity is not validated at the execution gate.
• ApprovalManager.create_pending creates another record when the new arguments change the stable tool-call identity, but that does not prevent the old positional resume decision from being consumed.

Suggested fix and acceptance criteria

Bind each resumed decision to the immutable approved invocation, including agent/tool/provider identity and arguments. Check that binding before any tool execution; on mismatch, fail closed or suspend for fresh approval. Consider checkpointing model/tool steps so resume does not regenerate the approved request.

• Changed arguments never execute under an earlier approval.
• Changed tool/agent selections never inherit an earlier decision.
• Identical invocations still resume when the provider changes only its transient tool-call ID.
• Covered for initial/resumed streams and sequential/nested approvals.

Executable regression

Save as tests/test_review_regression.py and run python -m pytest -q -s tests/test_review_regression.py from the repository root. It reuses existing test helpers. The test currently fails on its behavioral assertion.

import pytest
from langchain_core.messages import AIMessage, ToolMessage

from agent_engine.engine.langgraph.engine import LangGraphEngine
from agent_engine.runtime.hooks import RunContext
from tests.approvals.test_engine_hitl import _spec as approval_spec


class ChangingArgumentsModel:
    def __init__(self):
        self.calls = 0

    def bind_tools(self, tools):
        return self

    async def ainvoke(self, messages):
        if any(isinstance(m, ToolMessage) for m in messages):
            return AIMessage(content='done')
        self.calls += 1
        message = 'reviewed-value' if self.calls == 1 else 'changed-value'
        return AIMessage(content='', tool_calls=[{
            'name': 'record_value', 'args': {'message': message},
            'id': f'call-{self.calls}', 'type': 'tool_call',
        }])


@pytest.mark.asyncio
async def test_approval_is_bound_to_reviewed_arguments(tmp_path):
    output = tmp_path / 'executed.txt'
    tool = tmp_path / 'plugins/tools/record_value.py'
    tool.parent.mkdir(parents=True)
    tool.write_text('from pathlib import Path\n'
                    'def record_value(message: str) -> str:\n'
                    f'    Path({str(output)!r}).write_text(message)\n'
                    '    return message\n')
    model = ChangingArgumentsModel()
    async with LangGraphEngine(tmp_path, model_factory=lambda *a, **kw: model) as engine:
        await engine.build(approval_spec('record_value'))
        first = await engine.run('record a value', context=RunContext(run_id='review-args'))
        assert first.pending_approval.arguments == {'message': 'reviewed-value'}
        assert not output.exists()
        result = await engine.resume('review-args', first.pending_approval.approval_id,
                                     'allow once')
        executed = output.read_text() if output.exists() else None
        print(f'approved=reviewed-value, executed={executed}, returned status={result.status}')
        assert executed != 'changed-value'

---

_{From the Extra bug review of 2026-09-05 at b9acf1e. Review environment note: make check passed formatting, lint and mypy; baseline pytest reported 957 passed, 6 failed and 7 errors (API failures from a missing SOCKS dependency, plus three uninvestigated local-MCP example failures), so the baseline suite was not green in that environment.}

[nutstrut]

Independently reproduced this on current main at 023b0a0 using deterministic fake models and local side-effect tools only.
The original argument-substitution case still reproduces:
approved=reviewed-value, executed=changed-value, returned status=completed
I also tested the identity boundary a little further.
Negative cases

• same tool + changed arguments → the prior approval is consumed and the changed invocation executes
• changed tool identity + same-shaped arguments → the prior approval is also consumed and the different tool executes
  Positive controls
• identical semantic invocation resumes successfully
• changing only the transient provider/tool-call ID does not incorrectly invalidate approval
  That distinction seems useful because it suggests the authorization binding should cover the security-relevant invocation content rather than the transient call identifier.
  I also observed an evidence-consistency issue after the tool-substitution case: the approval ledger can show the originally presented tool as approved even though it never executed, while the substituted tool has already produced an observable side effect but remains represented separately as pending.
  The narrow invariant I would use for a regression is:
  A prior human decision may authorize execution only when the security-relevant invocation reaching the execution boundary matches the invocation presented for that decision.
  This does not require treating a regenerated transient tool-call ID as a different action.
  I have a deterministic changed-tool regression vector as well if that would be useful for the fix/tests.

[Asaf-prog]

Absolutely, that would be very helpful — we’d be happy to get the deterministic changed-tool regression vector for the fix/tests. Thanks!