Skip to content

fix: stop handing exception text to remote A2A peers and trigger callers - #1388

Open
copybara-service[bot] wants to merge 1 commit into
mainfrom
test_957201675
Open

fix: stop handing exception text to remote A2A peers and trigger callers#1388
copybara-service[bot] wants to merge 1 commit into
mainfrom
test_957201675

Conversation

@copybara-service

Copy link
Copy Markdown

fix: stop handing exception text to remote A2A peers and trigger callers

An exception message is written for whoever reads the log, not for whoever sent
the request. Both remote-facing surfaces were putting it in the response
anyway: the A2A executors built the failed TaskStatus message out of
str(exception), the /trigger/* handlers interpolated it into the
HTTPException detail, and the Java executor used Throwable#getMessage.
FileNotFoundError, ImportError, pydantic.ValidationError and SDK client errors
routinely carry absolute filesystem paths, the layout of the installed
packages, environment variable names, configuration values and echoed request
payloads. A caller who can reach either endpoint could therefore map the host
by provoking failures -- the reconnaissance a path-traversal or enumeration
attempt wants, and nothing a legitimate caller can act on.

The exception now stays server-side. A new utils/_error_reporting helper logs
the full traceback next to a short opaque id and returns a fixed summary plus
that id, so an operator handed the id from a report lands on the real
exception. Every remote-facing handler goes through it: both A2A executors, the
Pub/Sub and Eventarc handlers, and the base64 decode path.

The retry path needed one more change. TransientError was formatting the
provider's error text into its own message, so redacting at the boundary alone
would have let it out regardless; the cause is chained instead of interpolated.
Java gets the same treatment in AgentExecutor#failedMessage, which now also
owns the logging so the id in the response is the id in the log.

ADK_DEBUG_ERRORS=1 puts the exception text back in for local debugging. It is
off by default in both languages, and the summary is a constant at every call
site, so nothing interpolated can ride out past the redaction.

Each surface is covered by a test asserting that the leaked detail is gone and
the correlation id is present, and by one asserting the debug flag restores it.
The redaction tests fail against the previous implementation.

An exception message is written for whoever reads the log, not for whoever sent
the request. Both remote-facing surfaces were putting it in the response
anyway: the A2A executors built the failed TaskStatus message out of
`str(exception)`, the /trigger/* handlers interpolated it into the
HTTPException detail, and the Java executor used `Throwable#getMessage`.
FileNotFoundError, ImportError, pydantic.ValidationError and SDK client errors
routinely carry absolute filesystem paths, the layout of the installed
packages, environment variable names, configuration values and echoed request
payloads. A caller who can reach either endpoint could therefore map the host
by provoking failures -- the reconnaissance a path-traversal or enumeration
attempt wants, and nothing a legitimate caller can act on.

The exception now stays server-side. A new `utils/_error_reporting` helper logs
the full traceback next to a short opaque id and returns a fixed summary plus
that id, so an operator handed the id from a report lands on the real
exception. Every remote-facing handler goes through it: both A2A executors, the
Pub/Sub and Eventarc handlers, and the base64 decode path.

The retry path needed one more change. `TransientError` was formatting the
provider's error text into its own message, so redacting at the boundary alone
would have let it out regardless; the cause is chained instead of interpolated.
Java gets the same treatment in `AgentExecutor#failedMessage`, which now also
owns the logging so the id in the response is the id in the log.

`ADK_DEBUG_ERRORS=1` puts the exception text back in for local debugging. It is
off by default in both languages, and the summary is a constant at every call
site, so nothing interpolated can ride out past the redaction.

Each surface is covered by a test asserting that the leaked detail is gone and
the correlation id is present, and by one asserting the debug flag restores it.
The redaction tests fail against the previous implementation.

PiperOrigin-RevId: 957201675
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.

1 participant