Skip to content

security: complete input validation across all tool handlers#8

Merged
notque merged 1 commit into
mainfrom
security/complete-input-validation
May 7, 2026
Merged

security: complete input validation across all tool handlers#8
notque merged 1 commit into
mainfrom
security/complete-input-validation

Conversation

@notque
Copy link
Copy Markdown
Owner

@notque notque commented May 7, 2026

Summary

  • Add UUID validation to all tool handlers that accept ID parameters used in URL paths (11 handlers across 9 services)
  • Replace unsafe manual string concatenation in Hermes with url.Values encoding
  • Add allowlist validation for Hermes list_attributes to prevent path traversal
  • Remove duplicate uuidPattern from Castellum, consolidate on shared.ValidateUUID
  • Add path segment validation for Ironic node_id (accepts both UUIDs and names)
  • Add test coverage for multi-param and path traversal encoding

Context

PR #7 applied input validation to archer, keppel, and limes. This PR completes the job across all remaining services that embed user-provided values in URL paths or query strings without proper validation.

Handlers Fixed

Service Handler Parameter Validation
Keystone deleteAppCredentialHandler id UUID
Cinder getVolumeHandler volume_id UUID
Glance getImageHandler image_id UUID
Designate getZoneHandler zone_id UUID
Designate listRecordsetsHandler zone_id UUID
Octavia getLoadbalancerHandler loadbalancer_id UUID
Manila getShareHandler share_id UUID
Barbican getSecretHandler secret_id UUID
Ironic getNodeHandler node_id PathSegment
Nova getServerHandler server_id UUID
Nova serverActionHandler server_id UUID
Hermes getEventHandler event_id UUID
Hermes listAttributesHandler attribute Allowlist
Hermes listEventsHandler all query params url.Values
Castellum all handlers project_id shared.ValidateUUID (deduplicated)

Test plan

  • go test ./... passes
  • make run-golangci-lint — 0 issues
  • go build ./... compiles cleanly
  • New test cases for multi-param SafeQueryParams

Apply UUID validation and safe query encoding consistently across all
services, not just the three initially hardened (archer, keppel, limes).

Fixes:
- keystone: validate UUID before delete app credential (path segment)
- cinder: validate volume_id before get (path segment)
- glance: validate image_id before get (path segment)
- designate: validate zone_id in get_zone and list_recordsets
- octavia: validate loadbalancer_id before get (path segment)
- manila: validate share_id before get (path segment)
- barbican: validate secret_id before get (path segment)
- ironic: validate node_id via path segment (supports UUID or name)
- nova: validate server_id in both get and action handlers
- hermes: replace unsafe string concatenation with url.Values encoding,
  add UUID validation to get_event, add allowlist for list_attributes
- castellum: remove duplicate uuidPattern, use shared.ValidateUUID

Also adds test coverage for multi-param SafeQueryParams and path
traversal encoding.
@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 7, 2026

Merging this branch changes the coverage (9 decrease, 1 increase)

Impacted Packages Coverage Δ 🤖
github.com/notque/openstack-mcp-server/internal/tools/barbican 10.53% (-0.58%) 👎
github.com/notque/openstack-mcp-server/internal/tools/castellum 8.96% (ø)
github.com/notque/openstack-mcp-server/internal/tools/cinder 10.26% (-0.55%) 👎
github.com/notque/openstack-mcp-server/internal/tools/designate 9.68% (-0.67%) 👎
github.com/notque/openstack-mcp-server/internal/tools/glance 8.89% (-0.41%) 👎
github.com/notque/openstack-mcp-server/internal/tools/hermes 8.57% (+0.24%) 👍
github.com/notque/openstack-mcp-server/internal/tools/ironic 9.52% (-0.48%) 👎
github.com/notque/openstack-mcp-server/internal/tools/keystone 9.32% (-0.16%) 👎
github.com/notque/openstack-mcp-server/internal/tools/manila 9.09% (-0.43%) 👎
github.com/notque/openstack-mcp-server/internal/tools/nova 10.00% (-0.47%) 👎
github.com/notque/openstack-mcp-server/internal/tools/octavia 7.21% (-0.13%) 👎
github.com/notque/openstack-mcp-server/internal/tools/shared 68.52% (ø)

Coverage by file

Changed files (no unit tests)

Changed File Coverage Δ Total Covered Missed 🤖
github.com/notque/openstack-mcp-server/internal/tools/barbican/barbican.go 10.53% (-0.58%) 38 (+2) 4 34 (+2) 👎
github.com/notque/openstack-mcp-server/internal/tools/castellum/castellum.go 8.96% (ø) 67 6 61
github.com/notque/openstack-mcp-server/internal/tools/cinder/cinder.go 10.26% (-0.55%) 39 (+2) 4 35 (+2) 👎
github.com/notque/openstack-mcp-server/internal/tools/designate/designate.go 9.68% (-0.67%) 62 (+4) 6 56 (+4) 👎
github.com/notque/openstack-mcp-server/internal/tools/glance/glance.go 8.89% (-0.41%) 45 (+2) 4 41 (+2) 👎
github.com/notque/openstack-mcp-server/internal/tools/hermes/hermes.go 8.57% (+0.24%) 70 (-2) 6 64 (-2) 👍
github.com/notque/openstack-mcp-server/internal/tools/ironic/ironic.go 9.52% (-0.48%) 42 (+2) 4 38 (+2) 👎
github.com/notque/openstack-mcp-server/internal/tools/keystone/keystone.go 9.32% (-0.16%) 118 (+2) 11 107 (+2) 👎
github.com/notque/openstack-mcp-server/internal/tools/manila/manila.go 9.09% (-0.43%) 44 (+2) 4 40 (+2) 👎
github.com/notque/openstack-mcp-server/internal/tools/nova/nova.go 10.00% (-0.47%) 90 (+4) 9 81 (+4) 👎
github.com/notque/openstack-mcp-server/internal/tools/octavia/octavia.go 7.21% (-0.13%) 111 (+2) 8 103 (+2) 👎

Please note that the "Total", "Covered", and "Missed" counts above refer to code statements instead of lines of code. The value in brackets refers to the test coverage of that file in the old version of the code.

Changed unit test files

  • github.com/notque/openstack-mcp-server/internal/tools/shared/helpers_test.go

@notque notque merged commit 33cedc8 into main May 7, 2026
4 checks passed
@notque notque deleted the security/complete-input-validation branch May 7, 2026 13:12
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