Skip to content

Commit 5b9257c

Browse files
committed
fix(grafana): let the health check report ill-health, and disambiguate block outputs
The data source health check could only ever report health. Grafana answers an unhealthy source with HTTP 400 carrying the same {status, message} payload as a healthy one, and the tool framework converts any non-2xx into an opaque tool error — so the diagnostic the caller actually wants was unreachable. The check now goes through an internal route that reads the verdict off either status and reports it as a successful check, while a failure carrying no verdict (missing data source, bad token, plugin with no health endpoint) stays a real error. The plugin's `details` payload is surfaced too. Also on that route, matching the other three: an outbound timeout, redirect auth stripping, a truncated upstream error, and a URL-encoded UID. Block output descriptions: ten keys are emitted by several tools with different meanings and were described for only one producer — `database` meant both a data source name and a health status, `annotations` both an annotation list and an alert rule's summary map. Eleven `json` outputs were opaque although the tools already document their inner fields. All rewritten to name every producer. Smaller alignment fixes: - the same EmbeddedContactPoint.settings field was typed `object` in list and `json` in create - list_contact_points mapped non-nullable uid/name/type through `?? null`; Grafana returns an empty string, which is what create already assumed - create_alert_rule sent `orgID`, which Grafana overwrites from the authenticated context, and `Number()` on a non-numeric value put NaN -> null in the body - the three update routes declared `output` as required though the auth short-circuit omits it, and did not declare the `details` they emit on a validation error
1 parent 4242f05 commit 5b9257c

11 files changed

Lines changed: 442 additions & 51 deletions

File tree

apps/docs/content/docs/en/integrations/grafana.mdx

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -397,7 +397,7 @@ List all alert notification contact points
397397
|`uid` | string | Contact point UID |
398398
|`name` | string | Contact point name |
399399
|`type` | string | Notification type \(email, slack, etc.\) |
400-
|`settings` | object | Type-specific settings |
400+
|`settings` | json | Type-specific settings |
401401
|`disableResolveMessage` | boolean | Whether resolve messages are disabled |
402402
|`provenance` | string | Provisioning source — "api" for API-managed, empty when created with X-Disable-Provenance and therefore still editable in the Grafana UI |
403403

@@ -629,8 +629,9 @@ Test connectivity to a data source by its UID
629629

630630
| Parameter | Type | Description |
631631
| --------- | ---- | ----------- |
632-
| `status` | string | Health status of the data source \(e.g., OK\) |
633-
| `message` | string | Detailed health message from the data source |
632+
| `status` | string | Verdict Grafana returned for the data source, e.g. OK or ERROR. An unhealthy source reports here rather than failing the tool |
633+
| `message` | string | The plugin's diagnostic detail, which carries the reason on a failed check |
634+
| `details` | json | Extra structured detail, when the data source plugin supplies any |
634635

635636
### Grafana List Folders
636637

Lines changed: 131 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,131 @@
1+
/**
2+
* @vitest-environment node
3+
*/
4+
import { createMockRequest, hybridAuthMockFns } from '@sim/testing'
5+
import { beforeEach, describe, expect, it, vi } from 'vitest'
6+
7+
const { mockSecureFetch, mockValidateUrl, MOCK_MAX_JSON_BYTES } = vi.hoisted(() => ({
8+
mockSecureFetch: vi.fn(),
9+
mockValidateUrl: vi.fn(),
10+
MOCK_MAX_JSON_BYTES: 10 * 1024 * 1024,
11+
}))
12+
13+
vi.mock('@/lib/core/security/input-validation.server', () => ({
14+
secureFetchWithPinnedIP: mockSecureFetch,
15+
validateUrlWithDNS: mockValidateUrl,
16+
MAX_JSON_API_RESPONSE_BYTES: MOCK_MAX_JSON_BYTES,
17+
}))
18+
19+
import { POST } from '@/app/api/tools/grafana/check_data_source_health/route'
20+
21+
const baseBody = {
22+
apiKey: 'glsa_token',
23+
baseUrl: 'https://grafana.example.com',
24+
dataSourceUid: 'P1234AB5678',
25+
}
26+
27+
function grafanaResponse(body: unknown, status: number) {
28+
return {
29+
ok: status >= 200 && status < 300,
30+
status,
31+
statusText: '',
32+
headers: new Headers(),
33+
text: async () => (typeof body === 'string' ? body : JSON.stringify(body)),
34+
}
35+
}
36+
37+
function post(body: Record<string, unknown> = baseBody) {
38+
return POST(createMockRequest('POST', body) as never, undefined as never)
39+
}
40+
41+
describe('POST /api/tools/grafana/check_data_source_health', () => {
42+
beforeEach(() => {
43+
vi.clearAllMocks()
44+
hybridAuthMockFns.mockCheckInternalAuth.mockResolvedValue({ success: true, userId: 'user-1' })
45+
mockValidateUrl.mockResolvedValue({ isValid: true, resolvedIP: '203.0.113.10' })
46+
})
47+
48+
it('reports a healthy data source', async () => {
49+
mockSecureFetch.mockResolvedValue(
50+
grafanaResponse({ status: 'OK', message: 'Data source is working' }, 200)
51+
)
52+
53+
const response = await post()
54+
const data = await response.json()
55+
56+
expect(data.success).toBe(true)
57+
expect(data.output).toEqual({ status: 'OK', message: 'Data source is working' })
58+
})
59+
60+
it('reports an UNHEALTHY data source, which Grafana answers with HTTP 400', async () => {
61+
mockSecureFetch.mockResolvedValue(
62+
grafanaResponse({ status: 'ERROR', message: 'dial tcp: connection refused' }, 400)
63+
)
64+
65+
const response = await post()
66+
const data = await response.json()
67+
68+
expect(data.success).toBe(true)
69+
expect(data.output.status).toBe('ERROR')
70+
expect(data.output.message).toBe('dial tcp: connection refused')
71+
})
72+
73+
it('surfaces the plugin details when Grafana supplies them', async () => {
74+
mockSecureFetch.mockResolvedValue(
75+
grafanaResponse(
76+
{ status: 'ERROR', message: 'bad query', details: { verboseMessage: 'x' } },
77+
400
78+
)
79+
)
80+
81+
const response = await post()
82+
const data = await response.json()
83+
84+
expect(data.output.details).toEqual({ verboseMessage: 'x' })
85+
})
86+
87+
it('treats a failure with no health verdict as a real request failure', async () => {
88+
mockSecureFetch.mockResolvedValue(grafanaResponse({ message: 'Data source not found' }, 404))
89+
90+
const response = await post()
91+
const data = await response.json()
92+
93+
expect(data.success).toBe(false)
94+
expect(data.error).toContain('404')
95+
})
96+
97+
it('bounds and protects the outbound call', async () => {
98+
mockSecureFetch.mockResolvedValue(grafanaResponse({ status: 'OK', message: 'ok' }, 200))
99+
100+
await post()
101+
102+
const [url, resolvedIP, options] = mockSecureFetch.mock.calls[0]
103+
expect(resolvedIP).toBe('203.0.113.10')
104+
expect(url).toBe('https://grafana.example.com/api/datasources/uid/P1234AB5678/health')
105+
expect(options.maxResponseBytes).toBe(MOCK_MAX_JSON_BYTES)
106+
expect(options.timeout).toBeGreaterThan(0)
107+
expect(options.stripAuthOnRedirect).toBe(true)
108+
expect(options.headers.Authorization).toBe('Bearer glsa_token')
109+
})
110+
111+
it('encodes the UID so it cannot re-target the request path', async () => {
112+
mockSecureFetch.mockResolvedValue(grafanaResponse({ status: 'OK', message: 'ok' }, 200))
113+
114+
await post({ ...baseBody, dataSourceUid: 'a/../../admin' })
115+
116+
const [url] = mockSecureFetch.mock.calls[0]
117+
expect(url).toBe('https://grafana.example.com/api/datasources/uid/a%2F..%2F..%2Fadmin/health')
118+
})
119+
120+
it('rejects an unauthenticated request before reaching Grafana', async () => {
121+
hybridAuthMockFns.mockCheckInternalAuth.mockResolvedValue({
122+
success: false,
123+
error: 'Authentication required',
124+
})
125+
126+
const response = await post()
127+
128+
expect(response.status).toBe(401)
129+
expect(mockSecureFetch).not.toHaveBeenCalled()
130+
})
131+
})
Lines changed: 139 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,139 @@
1+
import { createLogger } from '@sim/logger'
2+
import { getErrorMessage } from '@sim/utils/errors'
3+
import { truncate } from '@sim/utils/string'
4+
import { type NextRequest, NextResponse } from 'next/server'
5+
import { grafanaCheckDataSourceHealthContract } from '@/lib/api/contracts/tools/grafana'
6+
import { getValidationErrorMessage, parseRequest } from '@/lib/api/server'
7+
import { checkInternalAuth } from '@/lib/auth/hybrid'
8+
import {
9+
MAX_JSON_API_RESPONSE_BYTES,
10+
secureFetchWithPinnedIP,
11+
validateUrlWithDNS,
12+
} from '@/lib/core/security/input-validation.server'
13+
import { generateRequestId } from '@/lib/core/utils/request'
14+
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
15+
16+
export const dynamic = 'force-dynamic'
17+
18+
const logger = createLogger('GrafanaCheckDataSourceHealthAPI')
19+
20+
const OUTBOUND_FETCH_TIMEOUT_MS = 30_000
21+
const MAX_ERROR_MESSAGE_LENGTH = 2000
22+
23+
/**
24+
* Runs a data source health check.
25+
*
26+
* Grafana answers an *unhealthy* data source with HTTP 400 carrying the same
27+
* `{status, message}` payload it uses for a healthy one, so the diagnostic the
28+
* caller actually wants only exists on the failure status. A plain tool would
29+
* have that converted into an opaque tool error, making the check able to report
30+
* health and never ill-health — hence this route, which reads the payload off
31+
* either status and reports it as a successful check.
32+
*/
33+
export const POST = withRouteHandler(async (request: NextRequest) => {
34+
const requestId = generateRequestId()
35+
36+
try {
37+
const authResult = await checkInternalAuth(request, { requireWorkflowId: false })
38+
if (!authResult.success || !authResult.userId) {
39+
logger.warn(`[${requestId}] Unauthorized Grafana health check: ${authResult.error}`)
40+
return NextResponse.json(
41+
{ success: false, error: authResult.error || 'Authentication required' },
42+
{ status: 401 }
43+
)
44+
}
45+
46+
const parsed = await parseRequest(
47+
grafanaCheckDataSourceHealthContract,
48+
request,
49+
{},
50+
{
51+
validationErrorResponse: (error) => {
52+
logger.warn(`[${requestId}] Invalid request data`, { errors: error.issues })
53+
return NextResponse.json(
54+
{
55+
success: false,
56+
error: getValidationErrorMessage(error, 'Invalid request data'),
57+
details: error.issues,
58+
},
59+
{ status: 400 }
60+
)
61+
},
62+
}
63+
)
64+
if (!parsed.success) return parsed.response
65+
const params = parsed.data.body
66+
67+
const baseUrl = params.baseUrl.replace(/\/$/, '')
68+
const healthUrl = `${baseUrl}/api/datasources/uid/${encodeURIComponent(
69+
params.dataSourceUid.trim()
70+
)}/health`
71+
72+
const urlValidation = await validateUrlWithDNS(healthUrl, 'baseUrl')
73+
if (!urlValidation.isValid || !urlValidation.resolvedIP) {
74+
return NextResponse.json({
75+
success: false,
76+
error: `Invalid Grafana baseUrl: ${urlValidation.error}`,
77+
})
78+
}
79+
80+
const headers: Record<string, string> = {
81+
Accept: 'application/json',
82+
Authorization: `Bearer ${params.apiKey}`,
83+
}
84+
if (params.organizationId) {
85+
headers['X-Grafana-Org-Id'] = params.organizationId
86+
}
87+
88+
const response = await secureFetchWithPinnedIP(healthUrl, urlValidation.resolvedIP, {
89+
method: 'GET',
90+
headers,
91+
maxResponseBytes: MAX_JSON_API_RESPONSE_BYTES,
92+
timeout: OUTBOUND_FETCH_TIMEOUT_MS,
93+
stripAuthOnRedirect: true,
94+
})
95+
96+
const raw = await response.text()
97+
let body: unknown = null
98+
if (raw.length > 0) {
99+
try {
100+
body = JSON.parse(raw)
101+
} catch {
102+
body = null
103+
}
104+
}
105+
106+
const payload =
107+
body && typeof body === 'object'
108+
? (body as { status?: unknown; message?: unknown; details?: unknown })
109+
: null
110+
111+
/**
112+
* A `status` in the body means Grafana ran the check and reported a verdict,
113+
* whatever the HTTP status. Anything else — an auth failure, a missing data
114+
* source, a plugin with no health endpoint — is a genuine request failure.
115+
*/
116+
if (payload && typeof payload.status === 'string') {
117+
return NextResponse.json({
118+
success: true,
119+
output: {
120+
status: payload.status,
121+
message: typeof payload.message === 'string' ? payload.message : null,
122+
...(payload.details === undefined ? {} : { details: payload.details }),
123+
},
124+
})
125+
}
126+
127+
logger.warn(`[${requestId}] Grafana health check did not report a status (${response.status})`)
128+
return NextResponse.json({
129+
success: false,
130+
error: `Failed to check data source health: HTTP ${response.status} ${truncate(
131+
raw,
132+
MAX_ERROR_MESSAGE_LENGTH
133+
)}`,
134+
})
135+
} catch (error) {
136+
logger.error(`[${requestId}] Error checking Grafana data source health:`, error)
137+
return NextResponse.json({ success: false, error: getErrorMessage(error) })
138+
}
139+
})

0 commit comments

Comments
 (0)