Skip to content

Conversation

gautambaghel
Copy link
Member

PCI review checklist

  • I have documented a clear reason for, and description of, the change I am making.

  • If applicable, I've documented a plan to revert these changes if they require more than reverting the pull request.

  • If applicable, I've documented the impact of any changes to security controls.

    Examples of changes to security controls include using new access control methods, adding or removing logging pipelines, etc.

@gautambaghel gautambaghel requested a review from a team as a code owner July 31, 2025 09:39
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR implements authentication and dynamic tool registry functionality while updating the mcp-go library. The changes introduce context-based client management, session-aware tool availability, and middleware for handling Terraform configuration through HTTP headers or environment variables.

Key changes:

  • Added dynamic tool registry that registers TFE-specific tools only when valid TFE credentials are available
  • Implemented middleware to extract Terraform configuration from HTTP headers/query parameters/environment variables
  • Refactored HTTP client creation from static dependency injection to context-based retrieval

Reviewed Changes

Copilot reviewed 21 out of 22 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
pkg/client/client.go New client management system with session-based TFE client creation and context retrieval
pkg/client/middleware.go New middleware for extracting Terraform configuration from requests and adding to context
pkg/tools/dynamic_tool.go Dynamic tool registry that conditionally registers TFE tools based on session state
pkg/tools/*.go Refactored all tool handlers to use context-based client retrieval instead of injected HTTP client
cmd/terraform-mcp-server/main.go Updated server initialization to include session hooks and remove direct HTTP client injection
go.mod Updated mcp-go library version and added new dependencies

Copy link
Collaborator

@dduzgun-security dduzgun-security left a comment

Choose a reason for hiding this comment

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

Great work here, I would really recommend looking at the elicitation feature to allow interactive user inputs for what we pass as environment variables. Also, left some suggestions.

@gautambaghel
Copy link
Member Author

Great work here, I would really recommend looking at the elicitation feature to allow interactive user inputs for what we pass as environment variables. Also, left some suggestions.

@dduzgun-security Thanks for the review deniz, elicitation is not yet supported in the mcp-go library yet, def consideration for the future, depends on mark3labs/mcp-go#495

gautambaghel and others added 18 commits July 31, 2025 13:57
Co-authored-by: Deniz Onur Duzgun <[email protected]>
Co-authored-by: Deniz Onur Duzgun <[email protected]>
Co-authored-by: Deniz Onur Duzgun <[email protected]>
Co-authored-by: Deniz Onur Duzgun <[email protected]>
Co-authored-by: Deniz Onur Duzgun <[email protected]>
Co-authored-by: Deniz Onur Duzgun <[email protected]>
Co-authored-by: Deniz Onur Duzgun <[email protected]>
Co-authored-by: Deniz Onur Duzgun <[email protected]>
Co-authored-by: Deniz Onur Duzgun <[email protected]>
Co-authored-by: Deniz Onur Duzgun <[email protected]>
@dduzgun-security dduzgun-security self-requested a review August 1, 2025 14:28
Copy link
Collaborator

@dduzgun-security dduzgun-security left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for looking into the suggestions 🎉

@gautambaghel gautambaghel merged commit e5a6ee0 into main Aug 14, 2025
50 checks passed
@gautambaghel gautambaghel deleted the feature/auth-tfe branch August 14, 2025 20:59
Copy link

Reminder to the contributor that merged this PR: if your changes have added important functionality or fixed a relevant bug, open a follow-up PR to update CHANGELOG.md with a note on your changes.

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.

3 participants