mcp: add log and per-artifact issue query tools - #306
Conversation
| content = response.content | ||
| if content[:2] == b"\x1f\x8b": | ||
| try: | ||
| content = gzip.decompress(content) |
There was a problem hiding this comment.
The download and decompression are not memory-bounded
A large log could exhaust the MCP process’s memory.
This should use streaming decompression with a bounded head/tail buffer and a hard size limit.
|
The trust boundary around log_url should be clarified. The code performs a direct GET, including redirects, against a URL obtained from dashboard data. If external submitters can control that field, this creates risk. The implementation should validate schemes, private/link-local addresses, DNS results, and redirect targets or document why all returned URLs are trusted. |
|
The happy paths have pytest coverage, but the security- and boundary-sensitive behavior is not covered. Please add tests for zero/negative/oversized max_bytes, truncated or malformed gzip input, download failures, bounded streaming/decompression, and URL/redirect validation. These tests should accompany the corresponding implementation fixes before approval. |
Add three read-only dashboard tools, exposed both as MCP tools and on KernelCIClient: - get_log resolves a test's log from log_url, or a log entry in output_files when log_url is empty (as it is for many failures), and returns the decompressed text bounded to the tail by default with total_bytes and a truncated flag. The download streams with a bounded head/tail buffer, decompresses gzip incrementally, caps max_bytes at a hard ceiling, stops after a scan limit to bound memory, and validates the URL scheme and resolved address (rejecting private/link-local hosts and unvalidated redirects) since log URLs come from submitter data - get_test_issues / get_build_issues list the issues tracked for a specific test or build, the inverse of the existing get_issue_tests and get_issue_builds get_log is new on KernelCIClient; the issue tools expose existing client methods that were not yet surfaced in the MCP. Signed-off-by: Ben Copeland <ben.copeland@linaro.org>
9001297 to
5700d3b
Compare
Good point, now becomes a KciDevError I also fixed multi-member gzip, which previously dropped everything after the first member. To your first comment r.e. log_url, I added a fetch-time guard that allows only http/https. Which also resolves the host and rejects private/loopback/link-local/reserved addresses (and follows redirects). Also have added tests. Thanks for the feedback @aliceinwire. Sorry would done it quicker (was on holiday but back now) |
2 participants
Add three read-only dashboard tools, exposed both as MCP tools and on KernelCIClient:
get_log is new on KernelCIClient; the issue tools expose existing client methods that were not yet surfaced in the MCP.