Skip to content

Add F5 Distributed Cloud (XC) toolset for WAF and load balancer investigation - #2351

Open
arikalon1 wants to merge 1 commit into
masterfrom
claude/holmes-f5-xc-integration-17r46n
Open

Add F5 Distributed Cloud (XC) toolset for WAF and load balancer investigation#2351
arikalon1 wants to merge 1 commit into
masterfrom
claude/holmes-f5-xc-integration-17r46n

Conversation

@arikalon1

@arikalon1 arikalon1 commented Aug 3, 2026

Copy link
Copy Markdown
Collaborator

Summary

Adds a new built-in toolset for F5 Distributed Cloud (XC) to enable investigation of WAF security events, bot defense, API security, HTTP request logs, and load balancer configuration. This allows users to query which applications are under attack, why requests are being blocked, and whether origin servers are healthy.

Key Changes

  • New F5 XC Toolset (holmes/plugins/toolsets/f5xc/f5xc.py):

    • F5XCConfig: Configuration class with API URL, token, SSL verification, timeout, and optional extra headers
    • F5XCToolset: Main toolset class with health check via API token validation
    • Seven tools for investigating F5 XC resources:
      • f5xc_list_namespaces: List all namespaces in the tenant
      • f5xc_list_http_load_balancers: List HTTP load balancers in a namespace (with optional full spec)
      • f5xc_get_http_load_balancer: Get full configuration of a single load balancer
      • f5xc_list_origin_pools: List origin pools (backend server groups)
      • f5xc_query_security_events: Query WAF, bot defense, API security, and service policy events with LogQL-like filtering
      • f5xc_aggregate_security_events: Aggregate (count) security events by field for overview questions
      • f5xc_query_request_logs: Query HTTP request logs with filtering by response code, latency, etc.
  • API Integration Details:

    • Thin wrapper around F5 XC REST APIs using requests library
    • Automatic RFC3339 timestamp conversion for relative time ranges (e.g., -3600 for last hour)
    • Handles F5 XC's JSON-encoded event/log items (parses embedded JSON strings)
    • Comprehensive error handling with detailed error messages for LLM self-correction
    • Support for extra HTTP headers via Jinja2 templates (for custom auth scenarios)
    • Per-page API limit of 500 items enforced with configurable defaults
  • Query Capabilities:

    • LogQL-like label filter syntax for security events and request logs (e.g., {vh_name="ves-io-http-loadbalancer-my-lb", sec_event_type=~"waf_sec_event|bot_defense_sec_event"})
    • Time range filtering with sensible defaults (24 hours for security events, 1 hour for request logs)
    • Aggregation by field (e.g., top attacking IPs, attack types, affected load balancers)
    • Truncation detection with notes when results exceed returned count
  • Documentation:

    • User guide (docs/data-sources/builtin-toolsets/f5-distributed-cloud.md) with setup instructions for Holmes CLI, Helm, and Robusta
    • API token creation walkthrough
    • Common use cases and advanced HTTP connector alternative
    • LLM instructions (holmes/plugins/toolsets/f5xc/instructions.jinja2) for investigation workflow
  • Testing (tests/plugins/toolsets/test_f5xc.py):

    • Configuration validation tests
    • Health check tests (success, auth failure, access denied)
    • Tool invocation tests for all seven tools
    • Error handling and edge case coverage (no data, truncation, API errors)
    • Embedded JSON parsing verification
  • Integration:

    • Registered in holmes/plugins/toolsets/__init__.py
    • Added to documentation index and README
    • Icon from Simple Icons (F5 brand color)

Implementation Notes

  • Follows the thin API wrapper pattern established in the codebase (similar to ServiceNow toolset)
  • Uses JsonFilterMixin for optional client-side filtering on list endpoints
  • Implements BaseF5XCTool abstract class for shared error handling and limit resolution
  • Health check validates API token and RBAC permissions (recommends read-only monitor role)
  • All tools return detailed error

https://claude.ai/code/session_01GPVxj8kCYpKkSv7Gc4Dyo8

Summary by CodeRabbit

  • New Features
    • Added F5 Distributed Cloud as a built-in data source.
    • Supports WAF security events, bot defense, request logs, load balancer diagnostics, namespaces, and origin pools.
    • Added configurable authentication, timeouts, SSL verification, result limits, filtering, aggregation, and multi-instance support.
  • Documentation
    • Added setup instructions, API-token guidance, configuration options, troubleshooting tips, common queries, and investigation workflows.
    • Added F5 Distributed Cloud to the built-in toolset navigation and security integrations overview.

Adds a read-only integration with F5 Distributed Cloud for investigating
WAF security events, bot defense, HTTP request logs, and load balancer
configuration.

- New f5xc toolset (holmes/plugins/toolsets/f5xc/) with 7 tools:
  list namespaces, list/get HTTP load balancers, list origin pools,
  query security events (per-namespace or tenant-wide), aggregate
  security events by field, and query request logs
- API token auth (Authorization: APIToken), health check via
  GET /api/web/namespaces, JsonFilterMixin on large-payload tools,
  RFC3339/relative time params, limits capped at the API's 500 max
- Log/event entries returned by the F5 XC API as JSON-encoded strings
  are decoded before being returned to the LLM
- Unit tests with mocked responses for config validation, health check
  and every tool (success, error, and no-data paths)
- Docs page including an HTTP connector alternative config for API
  endpoints not covered by the built-in tools; listed in README, docs
  index/nav and why-holmesgpt

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GPVxj8kCYpKkSv7Gc4Dyo8
Signed-off-by: Claude <noreply@anthropic.com>

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude Code Review

This repository is configured for manual code reviews. Comment @claude review for a one-time review, or @claude review always to subscribe this PR to a review on every future push.

Tip: disable this comment in your organization's Code Review settings.

@coderabbitai

coderabbitai Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

The PR adds F5 Distributed Cloud as a built-in, multi-instance toolset. It implements authenticated resource discovery, security-event aggregation, request-log queries, structured errors, tests, setup instructions, navigation, and integration documentation.

Changes

F5 Distributed Cloud integration

Layer / File(s) Summary
Configuration, registration, and shared API handling
holmes/plugins/toolsets/f5xc/f5xc.py, holmes/plugins/toolsets/f5xc/instructions.jinja2, holmes/plugins/toolsets/__init__.py, tests/plugins/toolsets/test_f5xc.py
Adds F5XC configuration, authentication, health checks, shared HTTP handling, error responses, instructions, tool registration, and validation tests.
Namespace and resource discovery
holmes/plugins/toolsets/f5xc/f5xc.py, tests/plugins/toolsets/test_f5xc.py
Adds namespace, HTTP load-balancer, load-balancer detail, and origin-pool tools with filtering and no-data handling.
Security and request-log queries
holmes/plugins/toolsets/f5xc/f5xc.py, tests/plugins/toolsets/test_f5xc.py
Adds security-event queries, aggregations, and request-log queries with filters, time ranges, limits, embedded JSON decoding, truncation notes, and error handling.
Integration documentation and navigation
docs/data-sources/builtin-toolsets/f5-distributed-cloud.md, docs/data-sources/builtin-toolsets/index.md, docs/data-sources/builtin-toolsets/.nav.yml, README.md, docs/why-holmesgpt.md
Documents setup, configuration, troubleshooting queries, HTTP connector usage, capabilities, and F5 Distributed Cloud integration locations.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant HolmesGPT
  participant F5XCToolset
  participant QuerySecurityEvents
  participant F5XCAPI
  HolmesGPT->>F5XCToolset: initialize F5 Distributed Cloud tools
  F5XCToolset->>F5XCAPI: perform authenticated health check
  HolmesGPT->>QuerySecurityEvents: submit filters and time range
  QuerySecurityEvents->>F5XCAPI: query security events
  F5XCAPI-->>QuerySecurityEvents: return event data
  QuerySecurityEvents-->>HolmesGPT: return decoded results or no-data response
Loading

Possibly related PRs

Suggested reviewers: aantn

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 7.94% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: adding an F5 Distributed Cloud toolset for WAF and load balancer investigation.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@netlify

netlify Bot commented Aug 3, 2026

Copy link
Copy Markdown

Deploy Preview for holmes-docs ready!

Name Link
🔨 Latest commit ee513aa
🔍 Latest deploy log https://app.netlify.com/projects/holmes-docs/deploys/6a70febd7839850007837ef8
😎 Deploy Preview https://deploy-preview-2351--holmes-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@github-actions

github-actions Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Docker images ready for 19a0c75d1 (built in 6m 21s)

⚠️ Warning: does not support ARM (ARM images are built on release only - not on every PR)

Use these tags to pull the images for testing.

📋 Copy commands

⚠️ Temporary images are deleted after 30 days. Copy to a permanent registry before using them:

gcloud auth configure-docker us-central1-docker.pkg.dev
docker pull us-central1-docker.pkg.dev/robusta-development/temporary-builds/holmes:19a0c75d1
docker tag us-central1-docker.pkg.dev/robusta-development/temporary-builds/holmes:19a0c75d1 me-west1-docker.pkg.dev/robusta-development/development/holmes-dev:19a0c75d1
docker push me-west1-docker.pkg.dev/robusta-development/development/holmes-dev:19a0c75d1
docker pull us-central1-docker.pkg.dev/robusta-development/temporary-builds/holmes-operator:19a0c75d1
docker tag us-central1-docker.pkg.dev/robusta-development/temporary-builds/holmes-operator:19a0c75d1 me-west1-docker.pkg.dev/robusta-development/development/holmes-operator-dev:19a0c75d1
docker push me-west1-docker.pkg.dev/robusta-development/development/holmes-operator-dev:19a0c75d1

Patch Helm values in one line (choose the chart you use):

HolmesGPT chart:

helm upgrade --install holmesgpt ./helm/holmes \
  --set registry=me-west1-docker.pkg.dev/robusta-development/development \
  --set image=holmes-dev:19a0c75d1 \
  --set operator.registry=me-west1-docker.pkg.dev/robusta-development/development \
  --set operator.image=holmes-operator-dev:19a0c75d1

Robusta wrapper chart:

helm upgrade --install robusta robusta/robusta \
  --reuse-values \
  --set holmes.registry=me-west1-docker.pkg.dev/robusta-development/development \
  --set holmes.image=holmes-dev:19a0c75d1 \
  --set holmes.operator.registry=me-west1-docker.pkg.dev/robusta-development/development \
  --set holmes.operator.image=holmes-operator-dev:19a0c75d1

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🧹 Nitpick comments (2)
tests/plugins/toolsets/test_f5xc.py (1)

161-238: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add coverage for extra_headers rendering.

The tests exercise the Authorization header only. No test covers extra_headers, so the Jinja2 template path in _make_api_request stays untested. Add a case that configures extra_headers and asserts the rendered header value on rsps.calls[0].request.headers.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/plugins/toolsets/test_f5xc.py` around lines 161 - 238, Add a test
covering extra_headers rendering in the relevant F5XC tool invocation, configure
an extra header in the toolset or request setup, and assert its rendered value
via rsps.calls[0].request.headers. Keep the existing Authorization coverage and
use the established test helper/configuration path.
holmes/plugins/toolsets/f5xc/f5xc.py (1)

229-239: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Consider adding retries for transient failures.

_make_api_request performs a single attempt. A transient 429, 502, or read timeout surfaces immediately as a tool error. The query tools are read-only and idempotent, so a bounded retry with backoff is safe here. Use tenacity as required by the repository guidelines.

♻️ Sketch of a tenacity-based retry
+from tenacity import (
+    retry,
+    retry_if_exception,
+    stop_after_attempt,
+    wait_exponential,
+)
+
+
+def _is_retryable(exc: BaseException) -> bool:
+    if isinstance(exc, (requests.exceptions.Timeout, requests.exceptions.ConnectionError)):
+        return True
+    return (
+        isinstance(exc, requests.exceptions.HTTPError)
+        and exc.response is not None
+        and exc.response.status_code in (429, 502, 503, 504)
+    )
+
+
+    `@retry`(
+        retry=retry_if_exception(_is_retryable),
+        stop=stop_after_attempt(3),
+        wait=wait_exponential(multiplier=1, max=8),
+        reraise=True,
+    )
     def _make_api_request(

Only the health check should stay single-attempt, to keep startup fast.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@holmes/plugins/toolsets/f5xc/f5xc.py` around lines 229 - 239, Update
_make_api_request to use tenacity for bounded retries with backoff around
transient HTTP 429/502 responses and read timeouts, while preserving the
existing request and JSON response behavior. Keep the health-check path
single-attempt and do not apply the retry policy to it.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/data-sources/builtin-toolsets/f5-distributed-cloud.md`:
- Around line 199-209: Remove the entire “Capabilities” section from the F5
Distributed Cloud documentation, including its heading and tool table. Leave the
surrounding documentation unchanged.
- Around line 134-150: Update the four Common Use Cases examples by converting
each prompt into its own separate fenced bash block containing a holmes ask
command with the prompt quoted as its argument. Preserve the prompt text exactly
and ensure every fence is labeled bash so markdown linting passes.

In `@holmes/plugins/toolsets/f5xc/f5xc.py`:
- Around line 334-336: Prevent path traversal through LLM-supplied endpoint
values by adding a shared _encode_segment helper that percent-encodes values
with no safe characters, then use it at every interpolation site:
holmes/plugins/toolsets/f5xc/f5xc.py lines 334-336 for namespace, 398-401 for
namespace and name, 452-454 for namespace, 543-550 for namespace, 679-686 for
namespace, and 804-806 for namespace. Keep endpoint structure unchanged while
ensuring each dynamic path segment is encoded before URL construction.
- Around line 249-251: The limit parsing in BaseF5XCTool._resolve_limit must
guard int() with TypeError and ValueError handling, fall back to default_limit,
and clamp the result between 1 and MAX_QUERY_LIMIT. In
holmes/plugins/toolsets/f5xc/f5xc.py:249-251, implement this directly in
_resolve_limit; in holmes/plugins/toolsets/f5xc/f5xc.py:687-688, convert field
with str() before upper(), and resolve topk through a shared BaseF5XCTool helper
using the same guarded coercion and positive lower bound.

---

Nitpick comments:
In `@holmes/plugins/toolsets/f5xc/f5xc.py`:
- Around line 229-239: Update _make_api_request to use tenacity for bounded
retries with backoff around transient HTTP 429/502 responses and read timeouts,
while preserving the existing request and JSON response behavior. Keep the
health-check path single-attempt and do not apply the retry policy to it.

In `@tests/plugins/toolsets/test_f5xc.py`:
- Around line 161-238: Add a test covering extra_headers rendering in the
relevant F5XC tool invocation, configure an extra header in the toolset or
request setup, and assert its rendered value via rsps.calls[0].request.headers.
Keep the existing Authorization coverage and use the established test
helper/configuration path.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: eba33997-b521-46b3-86b7-626a4b7b1e58

📥 Commits

Reviewing files that changed from the base of the PR and between e63f341 and ee513aa.

⛔ Files ignored due to path filters (1)
  • images/integration_logos/f5-icon.svg is excluded by !**/*.svg
📒 Files selected for processing (9)
  • README.md
  • docs/data-sources/builtin-toolsets/.nav.yml
  • docs/data-sources/builtin-toolsets/f5-distributed-cloud.md
  • docs/data-sources/builtin-toolsets/index.md
  • docs/why-holmesgpt.md
  • holmes/plugins/toolsets/__init__.py
  • holmes/plugins/toolsets/f5xc/f5xc.py
  • holmes/plugins/toolsets/f5xc/instructions.jinja2
  • tests/plugins/toolsets/test_f5xc.py

Comment on lines +134 to +150
## Common Use Cases

```
Which of my applications had WAF security events in the last 24 hours?
```

```
Why are requests to app.example.com getting blocked?
```

```
Show me the top attacking IPs across all namespaces today
```

```
Are there 5xx errors on the checkout load balancer in the last hour?
```

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Make the Common Use Cases examples runnable.

Convert each prompt to a holmes ask command and label each fence as bash. The current blocks at Lines [136-138], [140-142], [144-146], and [148-150] are bare prompts without a language.

Proposed fix
-```
-Which of my applications had WAF security events in the last 24 hours?
-```
+```bash
+holmes ask "Which of my applications had WAF security events in the last 24 hours?"
+```

-```
-Why are requests to app.example.com getting blocked?
-```
+```bash
+holmes ask "Why are requests to app.example.com getting blocked?"
+```

-```
-Show me the top attacking IPs across all namespaces today
-```
+```bash
+holmes ask "Show me the top attacking IPs across all namespaces today"
+```

-```
-Are there 5xx errors on the checkout load balancer in the last hour?
-```
+```bash
+holmes ask "Are there 5xx errors on the checkout load balancer in the last hour?"
+```

Based on learnings and static analysis, each individual holmes ask command must use its own separate fenced bash block, and markdownlint-cli2 reports missing fence languages at Lines [136], [140], [144], and [148].

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
## Common Use Cases
```
Which of my applications had WAF security events in the last 24 hours?
```
```
Why are requests to app.example.com getting blocked?
```
```
Show me the top attacking IPs across all namespaces today
```
```
Are there 5xx errors on the checkout load balancer in the last hour?
```
## Common Use Cases
🧰 Tools
🪛 markdownlint-cli2 (0.23.1)

[warning] 136-136: Fenced code blocks should have a language specified

(MD040, fenced-code-language)


[warning] 140-140: Fenced code blocks should have a language specified

(MD040, fenced-code-language)


[warning] 144-144: Fenced code blocks should have a language specified

(MD040, fenced-code-language)


[warning] 148-148: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/data-sources/builtin-toolsets/f5-distributed-cloud.md` around lines 134
- 150, Update the four Common Use Cases examples by converting each prompt into
its own separate fenced bash block containing a holmes ask command with the
prompt quoted as its argument. Preserve the prompt text exactly and ensure every
fence is labeled bash so markdown linting passes.

Sources: Learnings, Linters/SAST tools

Comment on lines +199 to +209
## Capabilities

| Tool Name | Description |
|-----------|-------------|
| f5xc_list_namespaces | List all namespaces in the tenant |
| f5xc_list_http_load_balancers | List HTTP load balancers in a namespace, optionally with full specs (domains, routes, WAF policy) |
| f5xc_get_http_load_balancer | Get the full configuration of a single HTTP load balancer |
| f5xc_list_origin_pools | List origin pools (backend server groups), optionally with origin servers and health checks |
| f5xc_query_security_events | Query WAF, bot defense, API security and service policy events, per namespace or tenant-wide |
| f5xc_aggregate_security_events | Count security events by field (top attack types, attacking IPs, targeted apps) |
| f5xc_query_request_logs | Query HTTP request (access) logs with response codes, paths, and timing breakdowns |

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Remove the Capabilities section.

Delete the heading and table at Lines [199-209]. Built-in toolset documentation must skip Capabilities sections because feature lists become stale.

Proposed fix
-## Capabilities
-
-| Tool Name | Description |
-|-----------|-------------|
-| f5xc_list_namespaces | List all namespaces in the tenant |
-| f5xc_list_http_load_balancers | List HTTP load balancers in a namespace, optionally with full specs (domains, routes, WAF policy) |
-| f5xc_get_http_load_balancer | Get the full configuration of a single HTTP load balancer |
-| f5xc_list_origin_pools | List origin pools (backend server groups), optionally with origin servers and health checks |
-| f5xc_query_security_events | Query WAF, bot defense, API security and service policy events, per namespace or tenant-wide |
-| f5xc_aggregate_security_events | Count security events by field (top attack types, attacking IPs, targeted apps) |
-| f5xc_query_request_logs | Query HTTP request (access) logs with response codes, paths, and timing breakdowns |

As per coding guidelines, docs/data-sources/builtin-toolsets/**/*.md documentation must skip “Capabilities” sections.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
## Capabilities
| Tool Name | Description |
|-----------|-------------|
| f5xc_list_namespaces | List all namespaces in the tenant |
| f5xc_list_http_load_balancers | List HTTP load balancers in a namespace, optionally with full specs (domains, routes, WAF policy) |
| f5xc_get_http_load_balancer | Get the full configuration of a single HTTP load balancer |
| f5xc_list_origin_pools | List origin pools (backend server groups), optionally with origin servers and health checks |
| f5xc_query_security_events | Query WAF, bot defense, API security and service policy events, per namespace or tenant-wide |
| f5xc_aggregate_security_events | Count security events by field (top attack types, attacking IPs, targeted apps) |
| f5xc_query_request_logs | Query HTTP request (access) logs with response codes, paths, and timing breakdowns |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/data-sources/builtin-toolsets/f5-distributed-cloud.md` around lines 199
- 209, Remove the entire “Capabilities” section from the F5 Distributed Cloud
documentation, including its heading and tool table. Leave the surrounding
documentation unchanged.

Source: Coding guidelines

Comment on lines +249 to +251
def _resolve_limit(self, params: dict) -> int:
limit = params.get("limit") or self._toolset.f5xc_config.default_limit
return min(int(limit), MAX_QUERY_LIMIT)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Validate LLM-supplied numeric parameters inside the error-handling path. The tools coerce limit and topk with bare int() and accept negative values. The coercion runs before the try block, so a bad value raises an unstructured exception instead of a self-correcting tool error.

  • holmes/plugins/toolsets/f5xc/f5xc.py#L249-L251: wrap int() in try/except (TypeError, ValueError), fall back to default_limit, and clamp with max(1, min(limit, MAX_QUERY_LIMIT)).
  • holmes/plugins/toolsets/f5xc/f5xc.py#L687-L688: coerce field with str() before .upper(), and resolve topk through a shared helper on BaseF5XCTool that applies the same guarded coercion and a positive lower bound.
📍 Affects 1 file
  • holmes/plugins/toolsets/f5xc/f5xc.py#L249-L251 (this comment)
  • holmes/plugins/toolsets/f5xc/f5xc.py#L687-L688
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@holmes/plugins/toolsets/f5xc/f5xc.py` around lines 249 - 251, The limit
parsing in BaseF5XCTool._resolve_limit must guard int() with TypeError and
ValueError handling, fall back to default_limit, and clamp the result between 1
and MAX_QUERY_LIMIT. In holmes/plugins/toolsets/f5xc/f5xc.py:249-251, implement
this directly in _resolve_limit; in
holmes/plugins/toolsets/f5xc/f5xc.py:687-688, convert field with str() before
upper(), and resolve topk through a shared BaseF5XCTool helper using the same
guarded coercion and positive lower bound.

Comment on lines +334 to +336
def _invoke(self, params: dict, context: ToolInvokeContext) -> StructuredToolResult:
namespace = params["namespace"]
endpoint = f"/api/config/namespaces/{namespace}/http_loadbalancers"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Percent-encode LLM-supplied values before path interpolation. Every tool builds its endpoint with an f-string over namespace and name. build_url then calls urljoin, which normalizes .. and / segments. A crafted value therefore redirects the request to a different F5 XC API path. Add one helper, for example _encode_segment(value: str) -> str returning quote(str(value), safe=""), and apply it at every interpolation site.

  • holmes/plugins/toolsets/f5xc/f5xc.py#L334-L336: encode namespace in the http_loadbalancers endpoint.
  • holmes/plugins/toolsets/f5xc/f5xc.py#L398-L401: encode both namespace and name in the single load balancer endpoint.
  • holmes/plugins/toolsets/f5xc/f5xc.py#L452-L454: encode namespace in the origin_pools endpoint.
  • holmes/plugins/toolsets/f5xc/f5xc.py#L543-L550: encode namespace in the app_security/events endpoint.
  • holmes/plugins/toolsets/f5xc/f5xc.py#L679-L686: encode namespace in the aggregation endpoint.
  • holmes/plugins/toolsets/f5xc/f5xc.py#L804-L806: encode namespace in the access_logs endpoint.
📍 Affects 1 file
  • holmes/plugins/toolsets/f5xc/f5xc.py#L334-L336 (this comment)
  • holmes/plugins/toolsets/f5xc/f5xc.py#L398-L401
  • holmes/plugins/toolsets/f5xc/f5xc.py#L452-L454
  • holmes/plugins/toolsets/f5xc/f5xc.py#L543-L550
  • holmes/plugins/toolsets/f5xc/f5xc.py#L679-L686
  • holmes/plugins/toolsets/f5xc/f5xc.py#L804-L806
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@holmes/plugins/toolsets/f5xc/f5xc.py` around lines 334 - 336, Prevent path
traversal through LLM-supplied endpoint values by adding a shared
_encode_segment helper that percent-encodes values with no safe characters, then
use it at every interpolation site: holmes/plugins/toolsets/f5xc/f5xc.py lines
334-336 for namespace, 398-401 for namespace and name, 452-454 for namespace,
543-550 for namespace, 679-686 for namespace, and 804-806 for namespace. Keep
endpoint structure unchanged while ensuring each dynamic path segment is encoded
before URL construction.

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.

2 participants