Skip to content

mcp-data-platform-v1.57.6

Choose a tag to compare

@github-actions github-actions released this 06 May 00:06
· 148 commits to main since this release
f788602

Highlights

  • OAuth token-endpoint observabilityPOST /oauth/token now emits structured slog events at INFO (success) and WARN (failure) so operators can diagnose silent re-auth loops, refresh-grant rejections, and parse-form failures that previously left no observable trace. Strict redaction: secret, token, and authorization-code values are NEVER logged; only their presence as booleans.
  • Admin Tools page now correctly attributes connections for fan-out toolkits. Tools proxied through the gateway are no longer all bucketed under "platform" — each tool is now labeled with its actual upstream connection.
  • Repo-wide code-quality cleanup: 13 gosec findings fixed (3 G710 open-redirect annotations, 10 G124 test-cookie attributes), 3 single-occurrence lints (gocognit, govet, modernize), and a ~38% reduction in goconst findings (269 → 166 with the cap removed). CI lint stays green via only-new-issues; this release introduces zero new findings.
  • MCP SDK upgraded to github.com/modelcontextprotocol/go-sdk v1.6.0.

Bug fixes

fix(oauth): structured token-endpoint observability + repo-wide lint cleanup (#358)

pkg/oauth/server.go handleTokenEndpoint was previously silent on both success and failure, leaving operators no way to diagnose silent re-auth loops or distinguish between client-mismatch / invalid-refresh / parse-form rejections.

New log emissions:

  • INFO on successful issuance: grant_type, client_id, credential_source (form vs basic), has_refresh_token (response-side boolean confirming refresh tokens are being issued), expires_in, scope, duration_ms.
  • WARN on rejected grant: grant_type, client_id, credential_source, presence-booleans for has_client_secret / has_refresh_token / has_code / has_code_verifier, duration_ms, plus the error reason from the grant handler.
  • WARN on r.ParseForm() failure (malformed body / oversize).
  • credential_source differentiates between form-supplied client credentials and HTTP Basic auth.

Redaction contract — verified by tests:

  • Secret values, refresh-token values, authorization-code values, and code_verifier values are NEVER logged. Only their presence as booleans.
  • The success test decodes the response body and asserts the actually-issued AccessToken and RefreshToken values are absent — proving the contract holds against the real outbound tokens, not just the test's input strings.
  • pkg/oauth/server_test.go adds 4 tests covering success / form-rejection / basic-auth-rejection / parse-form-failure.
  • handleTokenEndpoint coverage: 100.0%.

Operator impact: when a Claude.ai client (or any MCP client) appears to be silently re-authenticating in a loop, operators can now correlate the WARN log's error reason with platform observability to distinguish "stored refresh token is invalid (user must re-auth)" from "client_id mismatch (config drift)" from "invalid client credentials" — these were indistinguishable before.

fix(admin): list-tools attributes per-tool connection for fan-out toolkits (#355)

GET /api/v1/admin/tools previously labeled every tool's owning connection with tk.Connection() — the toolkit's instance-level default. This works for 1:1 toolkits (trino/datahub/s3, where one toolkit instance maps to one connection) but breaks for the gateway toolkit, which is 1:many: a single gateway instance proxies multiple upstream MCP servers, each tool namespaced as <connection_name>__<remote_tool>.

Result before fix: every gateway-proxied tool inherited the gateway's instance-level Connection() (typically empty in operator configs), and the admin Tools page grouped them all under "platform" via the t.connection || "platform" fallback in ToolsList.tsx.

Fix: listTools now type-asserts each toolkit to registry.ConnectionResolver and calls ConnectionForTool(name) per tool when the interface is implemented, falling back to tk.Connection() for unrouted tools. The ConnectionResolver interface already existed in pkg/registry/toolkit.go (used by the audit middleware to attribute proxied calls); the listing handler just wasn't using it.

Patch coverage: 100% (16/16 changed lines). Test: TestListTools/uses_ConnectionResolver_per_tool_when_toolkit_fans_out_across_upstreams.

Internal / quality (folded into #358)

Security scanner findings addressed

  • G710 (open redirect — 3 sites): annotated with // #nosec G710 plus a per-site justification at each call site:
    • pkg/oauth/server.go — destination is the registered redirect_uri, validated at /authorize.
    • pkg/admin/gateway_oauth_handler.go — destination is constrained by safeReturnURL (covered by TestSafeReturnURL) to a constant fallback or a same-origin relative path.
    • cmd/dev-mcp-mock/main.go — OAuth /authorize spec mandates redirect to client-supplied redirect_uri; this is a dev fixture.
  • G124 (insecure cookie attributes — 10 sites in tests): every test cookie now sets HttpOnly: true, Secure: true, SameSite: http.SameSiteStrictMode. Touches pkg/browsersession/, pkg/admin/, pkg/portal/.

Linter findings addressed

  • gocognit (cognitive complexity > 15): pkg/admin/system.go listTools was at 18. Extracted collectToolkitTools and resolveToolConnection helpers; doc comment on collectToolkitTools explains why fan-out toolkits need per-tool connection resolution.
  • govet inline: reflect.Ptrreflect.Pointer in pkg/toolkits/trino/connection_required.go.
  • modernize slicesbackward: pkg/platform/lifecycle.go Stop switched to slices.Backward.
  • goconst (~38% reduction — 269 → 166 with the cap removed): introduced file- or package-local constants for repeated string literals across many packages. Constants explicitly separate orthogonal concepts that share a string value today (e.g. paramGrantType for the OAuth wire-protocol form-field name vs logKeyGrantType for the structured-log key vs grantTypeRefreshToken for the wire value), so a future rename of one cannot silently change the meaning of another.

Generated artifacts

  • Regenerated internal/apidocs/{docs.go,swagger.json,swagger.yaml} to pick up the gateway.OAuthStatus.refresh_token_revoked field (model field added in a prior commit without doc regeneration).

Dependency updates

  • deps: bump github.com/modelcontextprotocol/go-sdk from 1.5.0 to 1.6.0 (#357)
  • ci: bump github/codeql-action from 4.35.2 to 4.35.3 (#356)

Upgrade notes

  • No config changes required. The new OAuth log emissions write through slog.Default(); no new YAML keys, env vars, or migrations.
  • Log volume: every successful and rejected token issuance now produces one structured event. Plan log retention accordingly. Each event is one line with bounded fields (no token values, no claim payloads).
  • No API/wire-format changes. GET /api/v1/admin/tools response shape is unchanged — the connection field is now populated correctly for fan-out toolkits (gateway), where it was previously empty/falling back to "platform".
  • Backwards compatible with v1.57.x deployments. No database migrations.

Verification

  • All CI checks green on the merged PR: Lint (only-new-issues: true), Test, Build, Security Scan, CodeQL, Analyze, Frontend Build, codecov/patch, codecov/project.
  • go test -race ./... passes across the full repo.
  • make security clean (gosec + govulncheck — no vulnerabilities).
  • gofmt -s -l . clean.
  • Patch coverage on changed lines ≥ 80% (codecov gate).
  • Release artifacts signed via Sigstore; SBOMs published per platform.

Full changelog: v1.57.5...v1.57.6