feat(auth): implement MCP auth tool-level scopes validation

[duwenxin99]

No description provided.

[gemini-code-assist]

Code Review

This pull request implements tool-level scope authorization for the Model Context Protocol (MCP), enabling granular access control for database tools. The changes include updating the Tool interface, adding scopesRequired configuration to all supported database tools, and integrating scope verification logic into the MCP request handler. Feedback focuses on resolving critical field shadowing in Serverless Spark configurations and eliminating redundant global scope registries in favor of the new interface methods. Additionally, improvements were suggested for more robust scope string parsing and ensuring proper header value escaping using the %q verb.

I am having trouble creating individual review comments. Click here to see my feedback.

internal/tools/serverlessspark/serverlesssparkcreatepysparkbatch/serverlesssparkcreatepysparkbatch.go (46-49)

The ScopesRequired field is already present in the embedded createbatch.Config struct. Adding it again here shadows the base field. When the YAML is decoded, the outer field will be populated while the inner one (used by the embedded tool logic) will remain empty, effectively breaking scope enforcement for this tool.

	createbatch.Config
}

internal/tools/serverlessspark/serverlesssparkcreatesparkbatch/serverlesssparkcreatesparkbatch.go (46-49)

The ScopesRequired field is already present in the embedded createbatch.Config struct. Adding it again here shadows the base field, which will cause the tool to ignore configured scopes.

	createbatch.Config
}

internal/server/config.go (319-332)

This manual parsing of scopesRequired and its storage in a global map is redundant. The field is already automatically unmarshaled into the tool's Config struct by the factory (as seen in the changes to individual tool files). Since the Tool interface now includes a GetScopesRequired() method, it is better to rely on the tool instance itself to provide its required scopes.

To maintain consistency with authRequired, you can simply ensure the field is initialized if missing.

	// Ensure scopesRequired is initialized if not present
	if _, ok := r["scopesRequired"]; !ok {
		r["scopesRequired"] = []string{}
	}

internal/server/mcp.go (585)

The error_description value should be properly escaped to ensure the header remains valid if the error message contains double quotes. Using the %q verb in fmt.Sprintf will automatically handle quoting and escaping of the string.

					w.Header().Set("WWW-Authenticate", fmt.Sprintf("Bearer error=\"insufficient_scope\", scope=\"%s\", resource_metadata=\"%s\", error_description=%q", strings.Join(mcpErr.ScopesRequired, " "), s.toolboxUrl+"/.well-known/oauth-protected-resource", mcpErr.Message))

internal/server/mcp/v20251125/method.go (213)

Instead of using the global tools.GetToolScopes map, use the GetScopesRequired() method from the Tool interface. This makes the tool implementation self-contained and avoids the need for a global registry of scopes.

	toolScopes := tool.GetScopesRequired()

internal/server/mcp/v20251125/method.go (226)

Using strings.Fields is more robust than strings.Split with a single space, as it correctly handles multiple spaces and returns an empty slice for empty strings, avoiding potential issues with empty string elements in the resulting slice.

		tokenScopes := strings.Fields(scopeClaim)

internal/tools/tools.go (38-48)

The toolScopes map and its associated getter/setter functions are redundant now that the Tool interface includes the GetScopesRequired() method and the scopes are being unmarshaled directly into the tool configurations.