Skip to content

feat: add MCP configuration review action #2878

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
dungdong-aws wants to merge 1 commit into
base: feature/mcp-security-enchance
Choose a base branch
from
+72 −36
Open

feat: add MCP configuration review action #2878

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 19 additions & 1 deletion server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/mcp/mcpManager.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1943,6 +1943,7 @@ describe('consent gate for workspace-scoped MCP servers (P417451767)', () => {
const workspaceMcp = '/tmp/ws-a/.amazonq/mcp.json'

let showMessageStub: sinon.SinonStub
let showDocumentStub: sinon.SinonStub
let hasApprovalStub: sinon.SinonStub
let recordApprovalStub: sinon.SinonStub
let setStateSpy: sinon.SinonSpy
Expand All @@ -1953,13 +1954,14 @@ describe('consent gate for workspace-scoped MCP servers (P417451767)', () => {
recordApprovalStub = sinon.stub(consentStore, 'recordApproval').resolves()

showMessageStub = sinon.stub()
showDocumentStub = sinon.stub().resolves({ success: true })
const featuresWithPrompt = {
...features,
workspace: {
...fakeWorkspace,
fs: { ...fakeWorkspace.fs, getUserHomeDir: () => fakeHome },
},
lsp: { window: { showMessageRequest: showMessageStub } },
lsp: { window: { showMessageRequest: showMessageStub, showDocument: showDocumentStub } },
}
sinon.stub(mcpUtils, 'loadAgentConfig').resolves({
servers: new Map(),
Expand Down Expand Up @@ -2028,6 +2030,22 @@ describe('consent gate for workspace-scoped MCP servers (P417451767)', () => {
expect(showMessageStub.calledOnce).to.be.true
})

it('opens the workspace configuration and re-prompts before allowing', async () => {
const mgr = await buildMgr()
showMessageStub.onFirstCall().resolves({ title: 'View full configuration' })
showMessageStub.onSecondCall().resolves({ title: 'Deny' })
const cfg: MCPServerConfig = { command: 'sh', args: ['-c', 'x'], __configPath__: workspaceMcp }

try {
await (mgr as any).initOneServerInternal('svc', cfg)
} catch {}

expect(showMessageStub.calledTwice).to.be.true
expect(showDocumentStub.calledOnce).to.be.true
expect(showDocumentStub.firstCall.args[0].uri).to.equal('file:///tmp/ws-a/.amazonq/mcp.json')
expect(showDocumentStub.firstCall.args[0].takeFocus).to.be.true
expect(recordApprovalStub.called).to.be.false
})
it('denial sets DISABLED state and caches the decision', async () => {
const mgr = await buildMgr()
showMessageStub.resolves({ title: 'Deny' })
Expand Down
88 changes: 53 additions & 35 deletions server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/mcp/mcpManager.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -460,42 +460,60 @@ export class McpManager {
const headerLine = headerNames.length > 0 ? `Headers: ${headerNames.join(', ').slice(0, 200)}\n` : ''
const allowBtn = { title: 'Allow for this server' }
const denyBtn = { title: 'Deny' }
let choice: { title: string } | null | undefined
try {
choice = await this.features.lsp.window.showMessageRequest({
type: MessageType.Warning,
message:
`Amazon Q — Untrusted MCP Server\n\n` +
`A workspace configuration file wants to start an MCP server.\n` +
`Server: ${serverName}\n` +
`Command: ${cmdLine}\n` +
envLine +
headerLine +
`Source: ${configPath}\n\n` +
`Running this server executes the above command on your machine, ` +
`with the environment variables listed above. ` +
`Review them in the configuration file if you are unsure — variables such as ` +
`NODE_OPTIONS can cause additional code to run. ` +
`Only allow if you trust the authors of this workspace.\n\n` +
`Your choice will be remembered for this workspace. ` +
`If you allow, you won't be asked again unless the server configuration changes.`,
actions: [allowBtn, denyBtn],
})
} catch (e: any) {
this.features.logging.warn(`MCP: consent prompt failed for '${serverName}': ${e?.message}`)
this.setState(serverName, McpServerStatus.FAILED, 0, 'consent prompt failed')
return
}
if (choice?.title !== allowBtn.title) {
this.features.logging.info(
`MCP: user declined consent for workspace-scoped server '${serverName}' (response: ${choice?.title ?? 'dismissed'})`
)
this.sessionDeniedConsent.add(denyKey)
this.setState(serverName, McpServerStatus.DISABLED, 0, 'consent not granted')
return
const reviewBtn = { title: 'View full configuration' }

while (true) {
let choice: { title: string } | null | undefined
try {
choice = await this.features.lsp.window.showMessageRequest({
type: MessageType.Warning,
message:
`Amazon Q — Untrusted MCP Server\n\n` +
`A workspace configuration file wants to start an MCP server.\n` +
`Server: ${serverName}\n` +
`Command: ${cmdLine}\n` +
envLine +
headerLine +
`Source: ${configPath}\n\n` +
`Running this server executes the above command on your machine, ` +
`with the environment variables listed above. ` +
`Use View full configuration to inspect the source before allowing. ` +
`Variables such as NODE_OPTIONS can cause additional code to run. ` +
`Only allow if you trust the authors of this workspace.\n\n` +
`Your choice will be remembered for this workspace. ` +
`If you allow, you won't be asked again unless the server configuration changes.`,
actions: [reviewBtn, allowBtn, denyBtn],
})
} catch (e: any) {
this.features.logging.warn(`MCP: consent prompt failed for '${serverName}': ${e?.message}`)
this.setState(serverName, McpServerStatus.FAILED, 0, 'consent prompt failed')
return
}
if (choice?.title === reviewBtn.title) {
try {
await this.features.lsp.window.showDocument({
uri: URI.file(configPath).toString(),
takeFocus: true,
})
} catch (e: any) {
this.features.logging.warn(
`MCP: failed to open configuration for '${serverName}': ${e?.message}`
)
}
continue
}
if (choice?.title !== allowBtn.title) {
this.features.logging.info(
`MCP: user declined consent for workspace-scoped server '${serverName}' (response: ${choice?.title ?? 'dismissed'})`
)
this.sessionDeniedConsent.add(denyKey)
this.setState(serverName, McpServerStatus.DISABLED, 0, 'consent not granted')
return
}
await recordApproval(this.features.workspace, this.features.logging, serverName, cfg, configPath)
this.features.logging.info(`MCP: recorded consent for workspace-scoped server '${serverName}'`)
break
}
await recordApproval(this.features.workspace, this.features.logging, serverName, cfg, configPath)
this.features.logging.info(`MCP: recorded consent for workspace-scoped server '${serverName}'`)
}
}

Expand Down
Loading