execute_code approval persistence and guard checks

Fixes on record

Merged to main. A stamp counts the issues a fix is linked to.

• #39466 fix(approval): persist execute_code session/always approvals (#39275)

  merged to main

  files: tests/tools/test_execute_code_approval_cluster.py, tools/approval.py

  Show the diff

  • tests/tools/test_execute_code_approval_cluster.py

    @@ -176,6 +176,53 @@ def test_guard_gateway_user_approves_is_one_shot(gw_session):
         assert A.is_approved(gw_session, "execute_code") is False
     
     
    +def test_guard_gateway_user_approves_session_persists(gw_session):
    +    """'Approve session' stores session-level approval (#39275)."""
    +    _register_resolver(gw_session, "session")
    +    res = A.check_execute_code_guard("import os; print(1)", "local")
    +    assert res["approved"] is True
    +    assert res.get("user_approved") is True
    +    # Session approval should now be stored.
    +    assert A.is_approved(gw_session, "execute_code") is True
    +    # Subsequent calls should auto-approve without prompting.
    +    res2 = A.check_execute_code_guard("import os; print(2)", "local")
    +    assert res2["approved"] is True
    +    # Cleanup
    +    with A._lock:
    +        s = A._session_approved.get(gw_session, set())
    +        s.discard("execute_code")
    +
    +
    +def test_guard_gateway_user_approves_always_persists(gw_session):
    +    """'Always' stores permanent approval (#39275)."""
    +    _register_resolver(gw_session, "always")
    +    res = A.check_execute_code_guard("import os; print(1)", "local")
    +    assert res["approved"] is True
    +    assert res.get("user_approved") is True
    +    # Permanent approval should now be stored.
    +    assert A.is_approved(gw_session, "execute_code") is True
    +    # Cleanup
    +    with A._lock:
    +        A._permanent_approved.discard("execute_code")
    +        s = A._session_approved.get(gw_session, set())
    +        s.discard("execute_code")
    +
    +
    +def test_guard_session_approval_short_circuits_prompt(gw_session):
    +    """Once session-approved, execute_code skips the approval prompt (#39275)."""
    +    # Manually set session approval.
    +    A.approve_session(gw_session, "execute_code")
    +    try:
    +        # Even with a denier registered, the is_approved check short-circuits.
    +        _register_resolver(gw_session, "deny")
    +        res = A.check_execute_code_guard("import os", "local")
    +        assert res["approved"] is True
    +    finally:
    +        with A._lock:
    +            s = A._session_approved.get(gw_session, set())
    +            s.discard("execute_code")
    +
    +
     def test_guard_gateway_user_denies_blocks(gw_session):
         _register_resolver(gw_session, "deny")
         res = A.check_execute_code_guard("import os", "local")

  • tools/approval.py

    @@ -1584,6 +1584,12 @@ def check_execute_code_guard(code: str, env_type: str) -> dict:
         # paths don't pay to copy a potentially-large script into this string.
         command = f"execute_code <<'PY'\n{code}\nPY"
     
    +    # Check session/permanent approval — same gate as check_all_command_guards.
    +    # Without this, "Approve session" / "Always" choices are stored but never
    +    # consulted, so every execute_code call re-prompts the user (#39275).
    +    if is_approved(session_key, pattern_key):
    +        return {"approved": True, "message": None}
    +
         # Smart mode: ask the aux LLM about the whole script. An APPROVE here only
         # suppresses the redundant whole-script prompt; the per-call terminal()
         # guards (restored by context propagation) still run independently.
    @@ -1674,9 +1680,15 @@ def check_execute_code_guard(code: str, env_type: str) -> dict:
                 "user_consent": False,
             }
     
    -    # Approved — one-shot only. Deliberately NO approve_session/approve_permanent:
    -    # each execute_code script is distinct arbitrary code, so approval never
    -    # persists to future scripts.
    +    # Approved — persist based on scope (same logic as check_all_command_guards).
    +    if choice == "session":
    +        approve_session(session_key, pattern_key)
    +    elif choice == "always":
    +        approve_session(session_key, pattern_key)
    +        approve_permanent(pattern_key)
    +        save_permanent_allowlist(_permanent_approved)
    +    # choice == "once": no persistence — approval lasts this single call only.
    +
         return {"approved": True, "message": None,
                 "user_approved": True, "description": description}