Skip to content

feat(amazonq): add consent prompt for workspace-scoped MCP servers#2708

Merged
laileni-aws merged 4 commits intoaws:mainfrom
laileni-aws:pr-2702
Apr 28, 2026
Merged

feat(amazonq): add consent prompt for workspace-scoped MCP servers#2708
laileni-aws merged 4 commits intoaws:mainfrom
laileni-aws:pr-2702

Conversation

@laileni-aws
Copy link
Copy Markdown
Contributor

@laileni-aws laileni-aws commented Apr 28, 2026

Problem

Workspace-scoped MCP server configurations (.amazonq/mcp.json or .amazonq/agents/default.json inside a project folder) are started without user consent. Since these config files live in the workspace directory, they could be attacker-controlled — a cloned repo could contain a malicious MCP config that executes arbitrary commands on the user's machine when opened in the IDE.

Additionally, the original implementation in #2702 had several issues:

  1. Prompt text didn't communicate persistence semantics — users couldn't tell whether "Allow" was permanent or session-scoped, or whether the approval applied to just this workspace or globally.
  2. isWorkspaceScoped check missed getGlobalPersonaConfigPath — configs from ~/.aws/amazonq/personas/default.json were incorrectly treated as workspace-scoped, triggering unnecessary consent prompts for trusted global configs.
  3. sessionDeniedConsent was not cleared in close() — stale denial entries persisted through reinitializeMcpServers() calls, inconsistent with how all other state is managed.
  4. sessionDeniedConsent used unnecessary ! non-null assertion — the field was initialized in the constructor, making the definite assignment assertion redundant.
  5. Stale approvals accumulated in mcp-approvals.json — when a server's config changed (new fingerprint), the old approval entry was kept alongside the new one instead of being replaced, causing the store to grow unbounded.
  6. fingerprintWorkspace was not OS-agnostic — path hashing didn't normalize separators, so the same workspace could produce different hashes on Windows vs macOS/Linux.

Solution

Consent gate (from #2702)

  • Added a consent prompt in initOneServerInternal that fires before connecting to any workspace-scoped MCP server
  • Global configs (~/.aws/amazonq/mcp.json, agents/default.json, personas/default.json) are trusted implicitly and bypass the prompt
  • User choices are persisted to ~/.aws/amazonq/mcp-approvals.json, keyed on (serverName, configFingerprint, workspaceHash)
  • Config fingerprinting uses SHA-256 on (command, args, env, url) — any mutation invalidates prior approvals and re-triggers the prompt
  • Session-scoped denial cache prevents re-prompting within the same session after "Deny"

Fixes on top of #2702

  1. Improved prompt text — added: "Your choice will be remembered for this workspace. If you allow, you won't be asked again unless the server configuration changes."
  2. Added getGlobalPersonaConfigPath to the trusted set — the isWorkspaceScoped check now compares against all three global paths (mcp, agent, persona)
  3. Clear sessionDeniedConsent in close() — added this.sessionDeniedConsent.clear() alongside the other state clears
  4. Replaced ! assertion with inline initializationprivate sessionDeniedConsent = new Set<string>()
  5. Stale approval evictionrecordApproval now filters by (serverName, workspaceHash) instead of (serverName, fingerprint, workspaceHash), so config changes replace the old entry instead of accumulating
  6. OS-agnostic path hashingfingerprintWorkspace now normalizes paths with path.resolve() and replaces backslashes with forward slashes before hashing

Testing

  • 17 unit tests for mcpConsentStore (fingerprinting, approval CRUD, deduplication, stale eviction, edge cases)
  • 8 unit tests for the consent gate in mcpManager (global path bypass for mcp/agent/persona, workspace prompts, denial caching, fingerprint invalidation, approval recording)

Testing done

  • Unit tests: all 25 tests pass
  • Manual testing: verified consent prompt appears for workspace-scoped configs, "Allow" persists across reloads, "Deny" is session-scoped, config mutation re-triggers prompt, global configs bypass prompt

License

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@laileni-aws laileni-aws marked this pull request as ready for review April 28, 2026 22:30
@laileni-aws laileni-aws requested a review from a team as a code owner April 28, 2026 22:30
@codecov-commenter
Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 97.22222% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 60.53%. Comparing base (ae7d3fc) to head (35d8c05).

Files with missing lines Patch % Lines
...ge-server/agenticChat/tools/mcp/mcpConsentStore.ts 97.36% 3 Missing ⚠️
...anguage-server/agenticChat/tools/mcp/mcpManager.ts 96.96% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2708      +/-   ##
==========================================
+ Coverage   60.43%   60.53%   +0.10%     
==========================================
  Files         278      279       +1     
  Lines       66065    66245     +180     
  Branches     4221     4248      +27     
==========================================
+ Hits        39927    40103     +176     
- Misses      26054    26057       +3     
- Partials       84       85       +1     
Flag Coverage Δ
unittests 60.53% <97.22%> (+0.10%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@laileni-aws laileni-aws merged commit 7b8595a into aws:main Apr 28, 2026
7 checks passed
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.

5 participants