Skip to content

fix(a2a): stop returning exception text to remote peers - #7446

Open
sushant-me wants to merge 3 commits into
google:mainfrom
sushant-me:fix-a2a-error-leak
Open

sushant-me wants to merge 3 commits into
google:mainfrom
sushant-me:fix-a2a-error-leak

Conversation

@sushant-me

Copy link
Copy Markdown
Contributor

Fixes #7445.

The problem

A2aAgentExecutor.execute serialized the throwable's message into the peer-visible failed-task response:

except Exception as e:
    logger.error('Error handling A2A request: %s', e, exc_info=True)
    ...
    parts=[_compat.make_text_part(str(e))],   # <- str(e) goes to the remote caller

The exception is already logged in full on the line above. But a remote peer that can reach the endpoint also received the same text, which routinely names absolute filesystem paths, module locations, configuration-key names and upstream error text. The caller does not need any of it to handle a failed task, and all of it is useful reconnaissance — a confidentiality problem across the A2A trust boundary.

The fix

Port the behaviour of adk-java a3df463 (same wording, same id shape):

error_id = _new_error_id()
logger.error('Error handling A2A request [error_id=%s]: %s', error_id, e, exc_info=True)
...
parts=[_compat.make_text_part(failure_text(e, error_id))],

The peer now receives only:

Agent execution failed. (error_id: <12 hex chars>)

The operator gets the full stack trace in the log under that id. ADK_DEBUG_ERRORS=1 puts the exception text back into the response, off by default and intended for local debugging only — enabling it on a network-reachable deployment restores the disclosure this change exists to prevent.

The id is 12 hex characters, matching newErrorId() in adk-java, which in turn documents itself as matching new_error_id() in adk-python.

Tests

tests/unittests/a2a/executor/test_a2a_agent_executor.py:

  • test_failure_event_does_not_leak_exception_text — a throwable whose message looks like real host detail (/home/victim/.config/adk/credentials.json) must not appear in the peer-visible part, and the text must match Agent execution failed\. \(error_id: [0-9a-f]{12}\).
  • test_failure_event_leaks_when_debug_errors_enabled — ADK_DEBUG_ERRORS=1 restores the text, so the escape hatch is covered too.

The helper reads the part text on both A2A v1 (flat Part.text) and 0.3.x (union via .root), so the test is not tied to one SDK shape.

pytest tests/unittests/a2a/executor/test_a2a_agent_executor.py
32 passed (30 pre-existing + 2 new)

Scope

Intentionally limited to the Python A2A executor's failure path. It does not claim code execution, credential theft or token disclosure — this is a defense-in-depth fix for information exposure across the A2A boundary.

One thing I did not do: A2aAgentExecutorConfig has no existing debug toggle, so I read the env var directly rather than widen that config's surface. Happy to move it into the config if you would prefer it there.

The A2A executor put str(exception) into the peer-visible failed-task response.
A remote caller that can reach the endpoint therefore receives server-side detail:
absolute filesystem paths, module locations, configuration-key names and upstream
error text - none of which the peer needs, all of which is useful reconnaissance.

The throwable is already logged in full. It is now logged under a short opaque
correlation id, and the peer gets only:

    Agent execution failed. (error_id: <12 hex chars>)

Ports the behaviour of adk-java a3df463. ADK_DEBUG_ERRORS=1 restores the old
peer-visible text, for local debugging only and off by default.

Verified: ./pytest tests/unittests/a2a/executor/test_a2a_agent_executor.py
          32 passed (30 pre-existing + 2 new)

Fixes google#7445
Follow-up to google#7446, addressing review feedback from @sanketpatil06.

The legacy executor stopped handing the throwable to the peer, but the new
integration path (a2a_agent_executor_impl.py, reached with force_new_version or
the new-integration extension) still built the failed-task message from str(e),
so the same disclosure survived on that route.

- a2a_agent_executor_impl.py: build the peer-visible text with failure_text()
  and log the throwable against its correlation id instead. The peer receives
  'Agent execution failed. (error_id: ...)'; ADK_DEBUG_ERRORS=1 still restores
  the old behaviour for local debugging.
- a2a_agent_executor.py: move the helper block below the imports, where it
  belongs (it previously split the import list in two).
- tests: the impl suite asserted the disclosure in
  test_execute_with_exception_handling; that assertion now pins the opaque id
  instead, and two tests mirror the legacy pair (no leak by default, leak when
  ADK_DEBUG_ERRORS is set).

89 passed in tests/unittests/a2a/executor/, 32 in the legacy file.
@sushant-me

Copy link
Copy Markdown
Contributor Author

@sanketpatil06 — done in 483ef1c, all three points.

The new integration path. a2a_agent_executor_impl.py now builds the peer-visible text with the same failure_text() helper and logs the throwable against its correlation id instead:

except Exception as e:
  error_id = _new_error_id()
  logger.error(
      'Error handling A2A request (error_id: %s): %s', error_id, e, exc_info=True
  )
  ...
  parts=[_compat.make_text_part(failure_text(e, error_id))],

So the peer gets Agent execution failed. (error_id: …) on both routes, and ADK_DEBUG_ERRORS=1 still restores the old behaviour for local debugging.

The test. test_a2a_agent_executor_impl.py already had test_execute_with_exception_handling asserting "Test error" in <part text> — that assertion was pinning the disclosure, so it now pins the opaque id instead. I added the two mirrors of the legacy pair: test_failure_event_does_not_leak_exception_text (a secret carrying /home/victim/.config/adk/credentials.json, asserting re.fullmatch on the id pattern and that neither the secret nor the path appears) and test_failure_event_leaks_when_debug_errors_enabled.

The helper block. Moved below the imports in a2a_agent_executor.py — it had split the import list in two, with execute_after_event_interceptors and execute_before_agent_interceptors arriving after the new failure_text definition.

89 passed in tests/unittests/a2a/executor/, 32 passed in the legacy file.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

A2A executor returns raw exception details to remote peers

1 participant