From 046c80d24115f832d57e0c718d9936984a28b16e Mon Sep 17 00:00:00 2001 From: sharpninja Date: Sat, 21 Mar 2026 01:44:53 -0500 Subject: [PATCH 1/3] Complete MCP-TODO-006 remediation hardening - harden auth, token, repo, and desktop trust boundaries - add TODO projection status/repair and config write durability - strengthen runtime overflow, pairing, and disposal handling Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- appsettings.yaml | 8 +- docs/CLIENT-INTEGRATION.md | 5 + docs/MCP-SERVER.md | 1 + docs/Project/TODO.yaml | 54 +- docs/Project/Technical-Requirements.md | 12 +- docs/USER-GUIDE.md | 6 +- src/McpServer.Client/DesktopClient.cs | 19 + src/McpServer.Client/McpClientBase.cs | 14 + src/McpServer.Client/McpServerClient.cs | 11 +- .../McpServerClientOptions.cs | 7 + src/McpServer.Client/Models/TodoModels.cs | 68 ++ src/McpServer.Client/TodoClient.cs | 12 + .../CopilotInteractiveSession.cs | 15 +- .../SampleHostPreviewFactory.cs | 7 + src/McpServer.McpAgent/McpAgentOptions.cs | 6 + .../ServiceCollectionExtensions.cs | 1 + .../Notifications/ChannelChangeEventBus.cs | 36 +- .../Notifications/IChangeEventBus.cs | 2 +- .../Options/DesktopLaunchOptions.cs | 37 + .../Services/ApiKeyIssuanceGuard.cs | 54 ++ .../Services/FileGitHubWorkspaceTokenStore.cs | 108 ++- .../Services/GitHubCliService.cs | 312 ++++++-- .../HostedMcpAgentExecutionStrategy.cs | 24 +- .../Services/ITodoService.cs | 27 + .../Services/InteractionLogSubmissionQueue.cs | 6 +- .../Services/PairingLoginAttemptGuard.cs | 208 ++++++ .../Services/PathGlobMatcher.cs | 134 ++++ .../Services/RepoFileService.cs | 132 +++- .../Services/SqliteTodoService.cs | 203 +++++- .../Services/TodoService.cs | 12 + .../Services/ToolBucketService.cs | 2 +- .../Services/WorkspaceTokenService.cs | 7 +- src/McpServer.Storage/McpDbContext.cs | 32 +- .../Controllers/DesktopController.cs | 62 ++ .../Controllers/GitHubController.cs | 670 ++++++++++-------- .../Controllers/TodoController.cs | 32 + .../Controllers/WorkspaceController.cs | 43 +- .../McpStdio/McpServerMcpTools.cs | 46 ++ .../InteractionLoggingMiddleware.cs | 10 +- .../Middleware/WorkspaceAuthMiddleware.cs | 41 +- .../WorkspaceResolutionMiddleware.cs | 22 +- src/McpServer.Support.Mcp/Program.cs | 91 ++- .../Services/AppSettingsFileService.cs | 159 ++++- .../Services/DesktopLaunchService.cs | 60 +- .../Services/WindowsServiceDeploymentGuard.cs | 6 + src/McpServer.Support.Mcp/Web/PairingHtml.cs | 12 +- .../Web/PairingHtmlRenderer.cs | 14 +- src/McpServer.Support.Mcp/appsettings.yaml | 8 +- .../DesktopClientTests.cs | 27 + .../McpServer.Client.Tests/TodoClientTests.cs | 63 ++ .../McpHostedAgentAdapterTests.cs | 17 +- .../Controllers/ApiKeyEndpointTests.cs | 33 + .../Controllers/DesktopControllerTests.cs | 56 +- .../Controllers/Http500ErrorContractTests.cs | 8 + .../MarkerRegenerationIntegrationTests.cs | 18 +- .../McpTransportMultiTenantTests.cs | 30 + .../MultiTenantIntegrationTests.cs | 11 + .../Controllers/PairingEndpointTests.cs | 37 + .../Controllers/TodoControllerTests.cs | 131 +++- .../Controllers/GitHubControllerTests.cs | 89 +++ .../Controllers/TodoControllerTests.cs | 84 +++ .../InteractionLoggingMiddlewareTests.cs | 29 + .../WorkspaceAuthMiddlewareTests.cs | 36 +- .../WorkspaceResolutionMiddlewareTests.cs | 39 +- .../Services/AppSettingsFileServiceTests.cs | 137 +++- .../Services/ChannelChangeEventBusTests.cs | 72 +- .../Services/DesktopLaunchServiceTests.cs | 137 ++++ .../FileGitHubWorkspaceTokenStoreTests.cs | 83 +++ .../Services/GitHubCliServiceTests.cs | 378 +++++----- .../InteractionLogSubmissionChannelTests.cs | 36 + .../Services/PairingLoginAttemptGuardTests.cs | 81 +++ .../Services/RepoFileServiceTests.cs | 126 ++++ .../Services/SqliteTodoServiceTests.cs | 108 +++ .../Services/ToolBucketServiceTests.cs | 24 + .../Services/ToolRegistryScopeTests.cs | 40 ++ .../WindowsServiceDeploymentGuardTests.cs | 14 + .../Storage/McpDbContextMultiTenantTests.cs | 89 ++- .../TestSupport/TestLogger.cs | 40 ++ .../Web/PairingHtmlRendererTests.cs | 4 +- .../TodoEndpointFixture.cs | 64 +- 80 files changed, 4266 insertions(+), 733 deletions(-) create mode 100644 src/McpServer.Services/Options/DesktopLaunchOptions.cs create mode 100644 src/McpServer.Services/Services/ApiKeyIssuanceGuard.cs create mode 100644 src/McpServer.Services/Services/PairingLoginAttemptGuard.cs create mode 100644 src/McpServer.Services/Services/PathGlobMatcher.cs create mode 100644 tests/McpServer.Support.Mcp.IntegrationTests/Controllers/ApiKeyEndpointTests.cs create mode 100644 tests/McpServer.Support.Mcp.Tests/Controllers/GitHubControllerTests.cs create mode 100644 tests/McpServer.Support.Mcp.Tests/Services/DesktopLaunchServiceTests.cs create mode 100644 tests/McpServer.Support.Mcp.Tests/Services/PairingLoginAttemptGuardTests.cs create mode 100644 tests/McpServer.Support.Mcp.Tests/TestSupport/TestLogger.cs diff --git a/appsettings.yaml b/appsettings.yaml index f7221131..afc6f89b 100644 --- a/appsettings.yaml +++ b/appsettings.yaml @@ -37,6 +37,10 @@ Mcp: TodoStorage: Provider: sqlite SqliteDataSource: mcp.db + DesktopLaunch: + Enabled: false + AccessToken: '' + AllowedExecutables: [] Workspaces: - WorkspacePath: E:\github\McpServer Name: McpServer @@ -92,8 +96,8 @@ Mcp: Url: http://localhost:8000 IngestUrl: http://localhost:8000/api/v1/ingest StreamName: mcp - Username: admin - Password: admin + Username: '' + Password: '' FallbackLogPath: logs/mcp-.log Auth: Authority: http://localhost:7080/realms/mcpserver diff --git a/docs/CLIENT-INTEGRATION.md b/docs/CLIENT-INTEGRATION.md index a1fd5c50..d33cf73f 100644 --- a/docs/CLIENT-INTEGRATION.md +++ b/docs/CLIENT-INTEGRATION.md @@ -106,6 +106,7 @@ var client = McpServerClientFactory.Create(new McpServerClientOptions { BaseUrl = new Uri("http://localhost:7147"), ApiKey = "token-from-marker", + DesktopLaunchToken = "desktop-launch-token-from-secure-config", WorkspacePath = @"E:\github\MyProject", }); // All requests include both X-Api-Key and X-Workspace-Path headers @@ -119,6 +120,10 @@ var launch = await client.Desktop.LaunchAsync(new DesktopLaunchRequest }); ``` +Remote desktop launch also requires the server-side `Mcp:DesktopLaunch:Enabled` feature gate, +the `Mcp:DesktopLaunch:AllowedExecutables` allowlist, and the privileged +`X-Desktop-Launch-Token` header supplied by `McpServerClientOptions.DesktopLaunchToken`. + Switch workspace at runtime: ```csharp diff --git a/docs/MCP-SERVER.md b/docs/MCP-SERVER.md index 5e88adc5..2c2b517f 100644 --- a/docs/MCP-SERVER.md +++ b/docs/MCP-SERVER.md @@ -80,6 +80,7 @@ Environment overrides: - `PORT` - highest-priority runtime port override - `MCP_INSTANCE` - instance selection when `--instance` is not passed +- Do not keep Parseable, OAuth, or other runtime secrets in shared repo config. Inject them through environment variables, secure host configuration, or machine-local overrides. ### Example `Mcp:Instances` diff --git a/docs/Project/TODO.yaml b/docs/Project/TODO.yaml index b1d61042..8fab4e1a 100644 --- a/docs/Project/TODO.yaml +++ b/docs/Project/TODO.yaml @@ -1642,7 +1642,7 @@ infrastructure: - Make SQLite the authoritative current-state store for TODO items while keeping docs/Project/TODO.yaml synchronized as a deterministic projection of database state. - Persist append-only audit history for TODO state changes and expose that history through HTTP, MCP STDIO, and the typed client. - Preserve special TODO document sections such as code-review-remediation, completed, and notes without leaving YAML as the runtime source of truth. - done-summary: 'Implemented authoritative SQLite TODO storage with deterministic TODO.yaml projection, append-only audit history, REST/MCP/client audit access, sqlite-default configuration, updated FR/TR/TEST traceability, full local validation, and a green GitHub Actions run 250 on develop.' + done-summary: Implemented authoritative SQLite TODO storage with deterministic TODO.yaml projection, append-only audit history, REST/MCP/client audit access, sqlite-default configuration, updated FR/TR/TEST traceability, full local validation, and a green GitHub Actions run 250 on develop. remaining: '' technical-details: - Keep TODO persistence in the existing raw SQLite TODO store rather than moving this feature into McpDbContext for this change. @@ -1675,6 +1675,58 @@ infrastructure: done: true - task: 'Phase 7: run build and test validation, then finalize the TODO item, session log, commit, push, and CI remediation' done: true + - id: MCP-TODO-006 + title: Remediate high-signal workspace code review findings + estimate: L + note: 'Status 2026-03-21: phases 0 through 8 are now complete locally. Phase 5 removed silent interaction-log and change-event drops in favor of explicit rejection plus warning telemetry, Phase 6 added SQLite-authoritative TODO projection status/repair surfaces with focused service/client/integration coverage, Phase 7 hardened the pairing flow with brute-force throttling plus retry-after signaling, removed the blocking tunnel shutdown hook, replaced hosted-agent sync-over-async disposal bridges with best-effort synchronous cleanup, and extended managed Windows deployment validation to non-service launches when the deployment manifest is present, and Phase 8 moved configuration writes onto shared serialized temp-file-plus-atomic-replace semantics for YAML patching and global prompt updates with focused concurrency/interruption coverage. Phase 9 validation matrix is green locally: `dotnet build src\McpServer.Support.Mcp -c Debug`, `tests\McpServer.Support.Mcp.Tests` filtered remediation slice (62 passed), `tests\McpServer.Client.Tests` filtered `TodoClientTests` slice (16 passed), and `tests\McpServer.Support.Mcp.IntegrationTests` filtered remediation slice (40 passed). Immediate next step: decide how to handle the existing mixed dirty worktree before any commit, push, redeploy, or CI follow-through.' + done: false + description: + - Execute a coordinated remediation program that hardens the workspace against security flaws, data-loss conditions, startup auth bypasses, and runtime/operational hazards uncovered by repository-wide review. + - Sequence the fixes so security and fail-closed behavior land before convenience or cleanup work, reducing the time the deployed service spends in a partially hardened state. + - Deliver each remediation with targeted regression coverage, explicit failure semantics, and configuration/documentation updates so the fixes survive redeploys and future refactors. + - Treat auth boundaries, workspace isolation, repo file access, and privileged local execution as one trust-boundary workstream so fixes do not close one bypass while leaving an equivalent neighboring path open. + remaining: 'Phase 0 through Phase 4 are complete. Immediate next step: begin Phase 5 by replacing silent drop behavior in interaction-log submission and change-event delivery with an explicit durability or backpressure model, then validate the chosen overflow semantics with focused regression coverage.' + technical-details: + - 'Finding A — GitHub CLI argument injection: GitHubCliService.CloseIssueAsync appends the close reason without the same escaping and quoting discipline used for other gh arguments. Fix the specific bug first, then audit the rest of the GitHub CLI command builder paths for the same bug class so the remediation is class-wide instead of one-line-only.' + - 'Finding B — Secret and token storage hardening: remove insecure default credentials from shipped configuration, require secure injection for runtime secrets, restrict GitHub token-store file permissions, and add cross-process-safe write semantics so concurrent service instances cannot silently overwrite or expose credential material.' + - 'Finding C — Startup auth fail-closed behavior: ensure workspace/token initialization completes before protected routes serve traffic, and return explicit failure responses when workspace or token state is unavailable instead of allowing requests through a bootstrap gap.' + - 'Finding D — Silent audit/change loss: InteractionLogSubmissionQueue and ChannelChangeEventBus currently rely on lossy bounded-channel behavior. Decide the supported overflow model (backpressure, explicit rejection, dead-letter persistence, or durable local buffering), then implement that model with telemetry and regression tests instead of silent drops.' + - 'Finding E — TODO persistence semantics: clarify the contract for DB-authoritative success plus YAML-projection failure, add operator-visible repair/consistency verification, and ensure callers/tests can distinguish committed current-state changes from full mutation failure.' + - 'Finding F — Runtime hardening: remove async-over-sync disposal patterns that can deadlock under load or shutdown pressure, strengthen pairing password protection and brute-force resistance, and review whether deployment-integrity validation should cover non-Windows-service launch modes when tamper detection is expected.' + - 'Finding G — Token issuance and default-token scope: /api-key currently issues a usable primary-workspace token through too-broad a trust boundary. Tighten issuance scope, reduce the default token to truly minimal privileges, and add rate-limiting/audit expectations so token issuance is observable and intentionally bounded.' + - 'Finding H — Workspace isolation and tenant scoping: bearer-auth routes that continue with an empty workspace context must fail closed instead of inheriting broad visibility. Remove any empty-workspace means all rows behavior for externally reachable data paths and require explicit workspace resolution where tenant boundaries matter.' + - 'Finding I — Repo file boundary enforcement: replace naive prefix-style allowlist matching with real glob evaluation and canonical-path validation, then add symlink or junction-aware checks so repo read and list surfaces cannot escape the intended boundary via path tricks or overbroad matches.' + - 'Finding J — Privileged local execution: desktop launch and related agent-launch capabilities need a stronger authorization tier, explicit feature gating, and executable allowlists so possession of a broad workspace key does not automatically imply local process-launch authority.' + - 'Finding K — Config patch durability: appsettings patch flows must use serialized temp-file plus atomic replace semantics so concurrent or interrupted writes do not leave invalid YAML or partial config state on disk.' + - 'Scope guard: keep this TODO focused on the reviewed findings above. Do not expand it into unrelated refactors, style cleanup, or speculative architecture work unless one of the remediations proves that such a change is directly required.' + - 'Acceptance criteria: shipped config must no longer embed insecure defaults; /api-key issuance and default-token scope must be intentionally restricted; protected routes and tenant-sensitive data paths must fail closed when workspace/auth context is unresolved; repo access must use real boundary enforcement; privileged local execution must require explicit admin authorization; event/audit delivery semantics must be observable rather than silently lossy; TODO projection failure states must be explicit and recoverable; config patch writes must be atomic; and each remediated finding must have reproducible regression coverage or an explicit operational verification path.' + implementation-tasks: + - task: 'Phase 0: convert the code-review findings into an implementation contract by confirming exact affected code paths, deciding any intentionally deferred sub-findings, defining acceptance criteria per finding, and reserving FR/TR/TEST documentation updates if the fixes introduce new externally visible guarantees' + done: true + - task: 'Phase 1: fix the GitHub CLI command-injection class of issues by escaping and quoting close-reason handling, auditing the remaining gh command builders for equivalent unescaped inputs, and adding adversarial regression tests around GitHubCliService and GitHubController' + done: true + - task: 'Phase 2: harden token issuance, token scope, and secret persistence by restricting /api-key trust boundaries, reducing default-token privileges, removing insecure shipped defaults, requiring secure runtime secret injection, restricting GitHub token-store file permissions, and making token-store writes safe across concurrent processes or service instances' + done: true + - task: 'Phase 3: restore strict workspace isolation and fail-closed auth behavior by ensuring tokens are generated before protected routes serve traffic, rejecting unconfigured workspace or auth states explicitly, requiring resolved workspace context where tenant boundaries apply, and removing externally reachable empty-workspace all-rows behavior' + done: true + - task: 'Phase 4: harden repo and local-execution trust boundaries by replacing naive repo allowlist matching with real glob plus canonical-path enforcement, adding junction or symlink-safe validation, and gating desktop or agent-launch capabilities behind explicit admin controls and executable allowlists' + done: true + - task: 'Phase 5: replace silent drop behavior in interaction-log submission and change-event delivery with supported durability/backpressure semantics, add explicit overflow telemetry, and add tests that prove overflow behavior is visible and non-silent' + done: false + - task: 'Phase 6: harden SQLite-authoritative TODO projection semantics by making DB-commit plus YAML-projection failure states unambiguous to callers, adding operator-visible repair or consistency verification, and covering failure/recovery paths in service and integration tests' + done: false + - task: 'Phase 7: remove runtime hardening gaps by eliminating async-over-sync disposal deadlock risks, upgrading pairing password protection and brute-force resistance, and deciding whether deployment-integrity validation must also apply outside the Windows-service hosting path' + done: false + - task: 'Phase 8: make configuration patch writes atomic and serialized with temp-file plus replace semantics, then add concurrency and interruption coverage so config updates cannot leave partially written YAML on disk' + done: false + - task: 'Phase 9: run the remediation validation matrix, update MCP-TODO-006 and session logs with phase-by-phase status, and only then commit, push, redeploy if needed, and monitor CI until the full remediation set is green' + done: false + - id: MCP-TODO-007 + title: Make workspace configuration database-first and endpoint-managed + estimate: 3-5d + done: false + description: + - Make workspace configuration authoritative in the database and manage workspace CRUD through the endpoints. Updating workspaces should persist the source-of-truth state in the database and only write the corresponding projection to appsettings.yaml. On startup, import workspace definitions from appsettings.yaml only when the workspace database is empty, and do not let file contents overwrite existing database state once records exist. medium-priority: - id: do-not-speak-tables title: '[NEED PLAN] Do Not Speak Tables' diff --git a/docs/Project/Technical-Requirements.md b/docs/Project/Technical-Requirements.md index 301c9ce0..f5e50dc7 100644 --- a/docs/Project/Technical-Requirements.md +++ b/docs/Project/Technical-Requirements.md @@ -517,7 +517,7 @@ Presence signaling SHALL be excluded from one-shot sessions. ## TR-MCP-EVT-001 -**In-Process Change Event Bus** — `ChannelChangeEventBus` SHALL be registered as a singleton `IChangeEventBus` and provide fan-out publish/subscribe semantics to independent subscribers using bounded channels (capacity 1000, `DropOldest` overflow mode). +**In-Process Change Event Bus** — `ChannelChangeEventBus` SHALL be registered as a singleton `IChangeEventBus` and provide fan-out publish/subscribe semantics to independent subscribers using bounded channels (capacity 1000) with non-blocking publish behavior. When a subscriber buffer is full, delivery to that subscriber SHALL be rejected and logged at warning level instead of silently discarding already queued events. **Covered by:** `ChannelChangeEventBus`, `IChangeEventBus`, `Program.cs` @@ -634,7 +634,7 @@ Presence signaling SHALL be excluded from one-shot sessions. ## TR-MCP-AGENT-007 -**Built-In MCP Session Log, TODO, Repository, Desktop-Launch, and PowerShell Workflow for Hosted Agents** — The hosted agent library SHALL implement built-in workflow operations for session bootstrap, turn creation/update, TODO retrieval/update, TODO plan/status/implementation flows, repository read/list/write operations, local desktop process launch using the existing MCP Server contracts, and persistent in-process PowerShell sessions hosted directly inside the current .NET agent process. The workflow SHALL preserve canonical ID conventions for session IDs, request IDs, and TODO IDs, SHALL keep repository access scoped to repo-relative paths, SHALL expose desktop launch through the authenticated workspace context, SHALL keep PowerShell session state local to the hosted agent instance, SHALL expose the same local PowerShell session manager to host applications through `IMcpHostedAgent.PowerShellSessions`, and SHALL prefer reuse of existing client abstractions where server contracts already exist instead of duplicating transport logic. +**Built-In MCP Session Log, TODO, Repository, Desktop-Launch, and PowerShell Workflow for Hosted Agents** — The hosted agent library SHALL implement built-in workflow operations for session bootstrap, turn creation/update, TODO retrieval/update, TODO plan/status/implementation flows, repository read/list/write operations, local desktop process launch using the existing MCP Server contracts, and persistent in-process PowerShell sessions hosted directly inside the current .NET agent process. The workflow SHALL preserve canonical ID conventions for session IDs, request IDs, and TODO IDs, SHALL keep repository access scoped to repo-relative paths, SHALL expose desktop launch through the authenticated workspace context only when the server-side desktop-launch feature gate, executable allowlist, and privileged desktop-launch token requirements are satisfied, SHALL keep PowerShell session state local to the hosted agent instance, SHALL expose the same local PowerShell session manager to host applications through `IMcpHostedAgent.PowerShellSessions`, and SHALL prefer reuse of existing client abstractions where server contracts already exist instead of duplicating transport logic. **Status:** ✅ Complete @@ -650,7 +650,7 @@ Presence signaling SHALL be excluded from one-shot sessions. **Administrative Configuration Snapshot and YAML Patch API** — `ConfigurationController` SHALL expose `GET /mcpserver/configuration` returning the current flattened `IConfiguration` view as `section:key` pairs, and `PATCH /mcpserver/configuration` accepting a flattened dictionary that patches only the submitted keys into `appsettings.yaml`. -Persistence SHALL be delegated to a dedicated helper service that resolves the correct `appsettings` file path, loads and writes YAML safely, and reloads `IConfigurationRoot` after updates. The endpoints SHALL use standard JWT Bearer admin authorization and remain closed when OIDC is disabled. +Persistence SHALL be delegated to a dedicated helper service that resolves the correct loaded `appsettings` file path, serializes concurrent mutations across the full read-modify-write cycle, writes YAML or JSON via temp-file-plus-atomic-replace semantics, and reloads `IConfigurationRoot` after successful updates. `WorkspaceController` global-prompt updates SHALL reuse the same helper so shared configuration writes obey the same durability and reload guarantees. The endpoints SHALL use standard JWT Bearer admin authorization and remain closed when OIDC is disabled. **Status:** ✅ Complete @@ -668,10 +668,10 @@ Projection failures after a committed authoritative mutation SHALL surface an ex ## TR-MCP-TODO-006 -**Append-Only TODO Audit History and Retrieval Contract** — TODO create, update, delete, and bootstrap-import operations SHALL append reconstructable audit snapshots with monotonic per-item versions. The server SHALL expose `GET /mcpserver/todo/{id}/audit` together with typed client parity and MCP STDIO tool parity so callers can retrieve ordered tracked states for a TODO item even when the current row has been deleted but audit history still exists. +**Append-Only TODO Audit History, Projection Failure Classification, and Repair Contract** — TODO create, update, delete, and bootstrap-import operations SHALL append reconstructable audit snapshots with monotonic per-item versions. The server SHALL expose `GET /mcpserver/todo/{id}/audit` together with typed client parity and MCP STDIO tool parity so callers can retrieve ordered tracked states for a TODO item even when the current row has been deleted but audit history still exists. -Mutation results SHALL include a machine-readable failure classification so callers can distinguish validation, not-found, projection-failure, conflict, and external-sync error shapes when TODO operations fail or only partially succeed. +Mutation results SHALL include a machine-readable failure classification so callers can distinguish validation, not-found, projection-failure, conflict, and external-sync error shapes when TODO operations fail or only partially succeed. For SQLite-backed TODO storage, a projection failure SHALL preserve committed authoritative SQLite state, record operator-visible projection failure metadata, and leave `TODO.yaml` repairable without replaying the mutation. The server SHALL expose `GET /mcpserver/todo/projection/status` and `POST /mcpserver/todo/projection/repair` together with typed client parity and MCP STDIO tool parity so operators can verify whether `TODO.yaml` matches authoritative SQLite state and rebuild it on demand. **Status:** ✅ Complete -**Covered by:** `ITodoService`, `ITodoStore`, `SqliteTodoService`, `TodoController`, `McpServerMcpTools`, `TodoClient`, `TodoModels`, `TodoCreationService`, `TodoUpdateService` +**Covered by:** `ITodoService`, `ITodoStore`, `SqliteTodoService`, `TodoYamlFileSerializer`, `TodoController`, `McpServerMcpTools`, `TodoClient`, `TodoModels`, `TodoCreationService`, `TodoUpdateService` diff --git a/docs/USER-GUIDE.md b/docs/USER-GUIDE.md index 181e3b8a..0e1aed80 100644 --- a/docs/USER-GUIDE.md +++ b/docs/USER-GUIDE.md @@ -213,7 +213,9 @@ Response example: ### Desktop controller (`/mcpserver/desktop/*`) -- `POST /mcpserver/desktop/launch` +- `POST /mcpserver/desktop/launch` — requires normal workspace authentication plus the + privileged `X-Desktop-Launch-Token` header, and the target executable must match + `Mcp:DesktopLaunch:AllowedExecutables` while `Mcp:DesktopLaunch:Enabled` is `true`. ### Diagnostic controller (`/mcpserver/diagnostic/*`) @@ -396,7 +398,7 @@ POST /mcpserver/context/pack POST /mcpserver/context/rebuild-index POST /mcpserver/context/search GET /mcpserver/context/sources -POST /mcpserver/desktop/launch +POST /mcpserver/desktop/launch # also requires X-Desktop-Launch-Token when enabled GET /mcpserver/events GET /mcpserver/gh/actions/runs GET /mcpserver/gh/actions/runs/{runId} diff --git a/src/McpServer.Client/DesktopClient.cs b/src/McpServer.Client/DesktopClient.cs index a86f88c7..cfa4da79 100644 --- a/src/McpServer.Client/DesktopClient.cs +++ b/src/McpServer.Client/DesktopClient.cs @@ -13,17 +13,27 @@ namespace McpServer.Client; /// public sealed class DesktopClient : McpClientBase { + private const string DesktopLaunchTokenHeaderName = "X-Desktop-Launch-Token"; + /// public DesktopClient(HttpClient http, McpServerClientOptions options) : base(http, options) { + DesktopLaunchToken = options.DesktopLaunchToken ?? string.Empty; } internal DesktopClient(HttpClient http, McpServerClientOptions options, WorkspacePathHolder holder) : base(http, options, holder) { + DesktopLaunchToken = options.DesktopLaunchToken ?? string.Empty; } + /// + /// Optional privileged token sent only to the desktop-launch endpoint so remote callers can + /// satisfy the server's desktop-launch authorization tier in addition to workspace auth. + /// + public string DesktopLaunchToken { get; set; } = string.Empty; + /// /// Launches a local desktop process through the authenticated MCP Server workspace. /// @@ -38,4 +48,13 @@ public async Task LaunchAsync( ArgumentNullException.ThrowIfNull(request); return await PostAsync("mcpserver/desktop/launch", request, cancellationToken); } + + /// + protected override void AppendCustomHeaders(HttpRequestMessage request) + { + base.AppendCustomHeaders(request); + + if (!string.IsNullOrWhiteSpace(DesktopLaunchToken)) + request.Headers.TryAddWithoutValidation(DesktopLaunchTokenHeaderName, DesktopLaunchToken); + } } diff --git a/src/McpServer.Client/McpClientBase.cs b/src/McpServer.Client/McpClientBase.cs index 78dd1176..953f1e09 100644 --- a/src/McpServer.Client/McpClientBase.cs +++ b/src/McpServer.Client/McpClientBase.cs @@ -286,6 +286,8 @@ protected async Task SendRawAsync( if (!string.IsNullOrWhiteSpace(WorkspacePath)) request.Headers.TryAddWithoutValidation("X-Workspace-Path", WorkspacePath); + AppendCustomHeaders(request); + if (!string.IsNullOrWhiteSpace(acceptMediaType)) request.Headers.Accept.Add(new MediaTypeWithQualityHeaderValue(acceptMediaType)); @@ -344,6 +346,16 @@ protected async Task SendForStatusAsync(HttpMethod method, strin return (bytes, mediaType); } + /// + /// Allows derived clients to append endpoint-specific headers after the shared + /// authentication and workspace headers have been applied. + /// + /// The outbound request receiving any derived-client headers. + protected virtual void AppendCustomHeaders(HttpRequestMessage request) + { + ArgumentNullException.ThrowIfNull(request); + } + /// /// Pre-flight auth check called before every outbound request. Throws with a /// descriptive message when no valid credential is available. @@ -418,6 +430,7 @@ private async Task SendAsync( if (!string.IsNullOrWhiteSpace(WorkspacePath)) request.Headers.TryAddWithoutValidation("X-Workspace-Path", WorkspacePath); + AppendCustomHeaders(request); request.Headers.Accept.Add(new MediaTypeWithQualityHeaderValue("application/json")); _logger?.LogInformation("[McpClient] {Method} {Uri} | Auth={AuthMode} | WorkspacePath={WorkspacePath}", @@ -469,6 +482,7 @@ protected async IAsyncEnumerable StreamSseAsync( request.Headers.TryAddWithoutValidation("X-Api-Key", ApiKey); if (!string.IsNullOrWhiteSpace(WorkspacePath)) request.Headers.TryAddWithoutValidation("X-Workspace-Path", WorkspacePath); + AppendCustomHeaders(request); request.Headers.Accept.Add(new MediaTypeWithQualityHeaderValue("text/event-stream")); using var response = await _http.SendAsync( diff --git a/src/McpServer.Client/McpServerClient.cs b/src/McpServer.Client/McpServerClient.cs index 6939ed89..084af042 100644 --- a/src/McpServer.Client/McpServerClient.cs +++ b/src/McpServer.Client/McpServerClient.cs @@ -16,9 +16,9 @@ namespace McpServer.Client; /// is thrown at request time if the key is empty. /// /// Default key: The default key returned by -/// grants read-only access to all endpoints except TODO routes (/mcpserver/todo*) -/// which are read-write. Consumers with access to the AGENTS-README-FIRST.yaml marker -/// file should use the full-access token from that file instead. +/// grants read-only access only. Consumers with access to the +/// AGENTS-README-FIRST.yaml marker file should use the full-access token from that file +/// for write operations or other privileged flows instead. /// /// Port targeting: Set to retarget all sub-clients /// to a different workspace host at runtime (e.g. after calling the workspace Start @@ -204,9 +204,8 @@ public int Port /// recommended startup call for consumers that do not have access /// to the AGENTS-README-FIRST.yaml marker file. /// - /// The default key grants read-only access to all endpoints except - /// TODO routes (/mcpserver/todo*) which are read-write. For full unrestricted - /// access, use the workspace token from the marker file instead. + /// The default key grants read-only access only. For full unrestricted + /// access, use the workspace token from the marker file or JWT Bearer authentication. /// /// This method is a no-op if is already non-empty /// (i.e. it was seeded via or set diff --git a/src/McpServer.Client/McpServerClientOptions.cs b/src/McpServer.Client/McpServerClientOptions.cs index 776d2513..a2f44521 100644 --- a/src/McpServer.Client/McpServerClientOptions.cs +++ b/src/McpServer.Client/McpServerClientOptions.cs @@ -59,6 +59,13 @@ public sealed class McpServerClientOptions /// public string? WorkspacePath { get; set; } + /// + /// Optional privileged token sent only to the desktop-launch endpoint as + /// X-Desktop-Launch-Token. Use this for callers that are explicitly trusted to invoke + /// remote desktop launch in addition to normal workspace authentication. + /// + public string? DesktopLaunchToken { get; set; } + /// /// HTTP request timeout applied to the internally-created /// when using . diff --git a/src/McpServer.Client/Models/TodoModels.cs b/src/McpServer.Client/Models/TodoModels.cs index 81cdfadd..f367d32b 100644 --- a/src/McpServer.Client/Models/TodoModels.cs +++ b/src/McpServer.Client/Models/TodoModels.cs @@ -294,6 +294,74 @@ public sealed class TodoAuditQueryResult public int TotalCount { get; set; } } +/// Status of SQLite-authoritative TODO.yaml projection health and consistency. +public sealed class TodoProjectionStatusResult +{ + /// Authoritative storage provider name. + [JsonPropertyName("authoritativeStore")] + public string AuthoritativeStore { get; set; } = string.Empty; + + /// Absolute path to the authoritative SQLite data source. + [JsonPropertyName("authoritativeDataSource")] + public string AuthoritativeDataSource { get; set; } = string.Empty; + + /// Absolute path to the projected TODO.yaml file. + [JsonPropertyName("projectionTargetPath")] + public string ProjectionTargetPath { get; set; } = string.Empty; + + /// Whether the projection target currently exists as a file. + [JsonPropertyName("projectionTargetExists")] + public bool ProjectionTargetExists { get; set; } + + /// Whether the projected TODO.yaml content matches authoritative SQLite state. + [JsonPropertyName("projectionConsistent")] + public bool ProjectionConsistent { get; set; } + + /// Whether operator repair is currently required. + [JsonPropertyName("repairRequired")] + public bool RepairRequired { get; set; } + + /// UTC timestamp when the consistency check was performed. + [JsonPropertyName("verifiedAtUtc")] + public string VerifiedAtUtc { get; set; } = string.Empty; + + /// UTC timestamp of the last YAML import into SQLite, when known. + [JsonPropertyName("lastImportedFromYamlUtc")] + public string? LastImportedFromYamlUtc { get; set; } + + /// UTC timestamp of the last successful projection from SQLite to YAML, when known. + [JsonPropertyName("lastProjectedToYamlUtc")] + public string? LastProjectedToYamlUtc { get; set; } + + /// UTC timestamp of the last recorded projection failure, when known. + [JsonPropertyName("lastProjectionFailureUtc")] + public string? LastProjectionFailureUtc { get; set; } + + /// Last recorded projection failure message, when known. + [JsonPropertyName("lastProjectionFailure")] + public string? LastProjectionFailure { get; set; } + + /// Human-readable projection status summary. + [JsonPropertyName("message")] + public string? Message { get; set; } +} + +/// Result of an operator-requested TODO.yaml projection repair attempt. +public sealed class TodoProjectionRepairResult +{ + /// Whether the repair attempt succeeded. + [JsonPropertyName("success")] + public bool Success { get; set; } + + /// Error message on repair failure. + [JsonPropertyName("error")] + public string? Error { get; set; } + + /// Status after the repair attempt completed. + [JsonPropertyName("status")] + public TodoProjectionStatusResult Status { get; set; } = new(); +} + /// Append-only audit entry for a TODO item. public sealed class TodoAuditEntry { diff --git a/src/McpServer.Client/TodoClient.cs b/src/McpServer.Client/TodoClient.cs index b59f5a1f..d76d8a66 100644 --- a/src/McpServer.Client/TodoClient.cs +++ b/src/McpServer.Client/TodoClient.cs @@ -52,6 +52,18 @@ public async Task GetAuditAsync( return await GetAsync($"mcpserver/todo/{Encode(id)}/audit{suffix}", cancellationToken); } + /// Get projection status for SQLite-authoritative TODO storage. + public async Task GetProjectionStatusAsync(CancellationToken cancellationToken = default) + { + return await GetAsync("mcpserver/todo/projection/status", cancellationToken); + } + + /// Repair TODO.yaml projection from SQLite-authoritative TODO storage. + public async Task RepairProjectionAsync(CancellationToken cancellationToken = default) + { + return await PostAsync("mcpserver/todo/projection/repair", null, cancellationToken); + } + /// Create a new TODO item. public async Task CreateAsync(TodoCreateRequest request, CancellationToken cancellationToken = default) { diff --git a/src/McpServer.Common.Copilot/CopilotInteractiveSession.cs b/src/McpServer.Common.Copilot/CopilotInteractiveSession.cs index 23bb5558..098edf36 100644 --- a/src/McpServer.Common.Copilot/CopilotInteractiveSession.cs +++ b/src/McpServer.Common.Copilot/CopilotInteractiveSession.cs @@ -9,7 +9,7 @@ namespace McpServer.Common.Copilot; /// Subsequent prompts are written to stdin; responses are read from stdout /// until the "Esc to cancel" sentinel indicates Copilot is ready for input. /// -public sealed class CopilotInteractiveSession : IAsyncDisposable +public sealed class CopilotInteractiveSession : IAsyncDisposable, IDisposable { private const string Sentinel = "Esc to cancel"; private const int OutputTailLineLimit = 40; @@ -272,6 +272,19 @@ public async ValueTask DisposeAsync() _gate.Dispose(); } + /// + public void Dispose() + { + if (_disposed) + return; + + _disposed = true; + MarkShutdownRequested(); + TryKillProcess(); + _process.Dispose(); + _gate.Dispose(); + } + // ── Internals ────────────────────────────────────────────────────── private async Task ReadUntilSentinelAsync(CancellationToken ct) diff --git a/src/McpServer.McpAgent.SampleHost/SampleHostPreviewFactory.cs b/src/McpServer.McpAgent.SampleHost/SampleHostPreviewFactory.cs index 48fb8f38..627879a5 100644 --- a/src/McpServer.McpAgent.SampleHost/SampleHostPreviewFactory.cs +++ b/src/McpServer.McpAgent.SampleHost/SampleHostPreviewFactory.cs @@ -31,6 +31,7 @@ public static ServiceProvider BuildServiceProvider(IConfiguration configuration) options.BaseUrl = settings.BaseUrl; options.ApiKey = settings.ApiKey; options.BearerToken = settings.BearerToken; + options.DesktopLaunchToken = settings.DesktopLaunchToken; options.RequireAuthentication = settings.RequireAuthentication; options.WorkspacePath = settings.WorkspacePath; options.AgentId = settings.AgentId; @@ -819,6 +820,7 @@ internal sealed record SampleHostSettings( Uri BaseUrl, string? ApiKey, string? BearerToken, + string? DesktopLaunchToken, bool RequireAuthentication, string WorkspacePath, string AgentId, @@ -835,6 +837,7 @@ internal sealed record SampleHostSettings( { private const string ApiKeyEnvironmentVariable = "MCP_SERVER_API_KEY"; private const string BearerTokenEnvironmentVariable = "MCP_SERVER_BEARER_TOKEN"; + private const string DesktopLaunchTokenEnvironmentVariable = "MCP_SERVER_DESKTOP_LAUNCH_TOKEN"; private const string BaseUrlEnvironmentVariable = "MCP_SERVER_BASE_URL"; private const string WorkspacePathEnvironmentVariable = "MCP_SERVER_WORKSPACE_PATH"; private const string AgentIdEnvironmentVariable = "MCP_AGENT_ID"; @@ -862,6 +865,9 @@ public static SampleHostSettings Resolve(IConfiguration configuration) var bearerToken = FirstNonEmpty( ReadEnvironmentVariable(BearerTokenEnvironmentVariable), configuration["McpServer:AgentFramework:BearerToken"]); + var desktopLaunchToken = FirstNonEmpty( + ReadEnvironmentVariable(DesktopLaunchTokenEnvironmentVariable), + configuration["McpServer:AgentFramework:DesktopLaunchToken"]); var requireAuthentication = ResolveRequireAuthentication(configuration, apiKey, bearerToken); var workspacePath = ResolveWorkspacePath(configuration, marker); var modelId = FirstNonEmpty( @@ -894,6 +900,7 @@ public static SampleHostSettings Resolve(IConfiguration configuration) baseUrl, apiKey, bearerToken, + desktopLaunchToken, requireAuthentication, workspacePath, FirstNonEmpty( diff --git a/src/McpServer.McpAgent/McpAgentOptions.cs b/src/McpServer.McpAgent/McpAgentOptions.cs index 9f45db88..9895ca4b 100644 --- a/src/McpServer.McpAgent/McpAgentOptions.cs +++ b/src/McpServer.McpAgent/McpAgentOptions.cs @@ -39,6 +39,11 @@ public sealed class McpAgentOptions /// public string? WorkspacePath { get; set; } + /// + /// Optional privileged token used only for remote desktop-launch requests. + /// + public string? DesktopLaunchToken { get; set; } + /// /// Stable identifier projected into for host-side agent construction. /// @@ -70,6 +75,7 @@ internal McpServerClientOptions ToClientOptions(ILoggerFactory? loggerFactory = ApiKey = ApiKey, BearerToken = BearerToken, BaseUrl = BaseUrl, + DesktopLaunchToken = DesktopLaunchToken, LoggerFactory = loggerFactory, Timeout = Timeout, WorkspacePath = WorkspacePath, diff --git a/src/McpServer.McpAgent/ServiceCollectionExtensions.cs b/src/McpServer.McpAgent/ServiceCollectionExtensions.cs index 41d7995a..4af62390 100644 --- a/src/McpServer.McpAgent/ServiceCollectionExtensions.cs +++ b/src/McpServer.McpAgent/ServiceCollectionExtensions.cs @@ -38,6 +38,7 @@ public static IServiceCollection AddMcpServerMcpAgent(this IServiceCollection se clientOptions.ApiKey = options.ApiKey; clientOptions.BearerToken = options.BearerToken; clientOptions.BaseUrl = options.BaseUrl; + clientOptions.DesktopLaunchToken = options.DesktopLaunchToken; clientOptions.Timeout = options.Timeout; clientOptions.WorkspacePath = options.WorkspacePath; }); diff --git a/src/McpServer.Services/Notifications/ChannelChangeEventBus.cs b/src/McpServer.Services/Notifications/ChannelChangeEventBus.cs index 66abd423..dedc2c78 100644 --- a/src/McpServer.Services/Notifications/ChannelChangeEventBus.cs +++ b/src/McpServer.Services/Notifications/ChannelChangeEventBus.cs @@ -1,26 +1,52 @@ using System.Collections.Concurrent; using System.Runtime.CompilerServices; using System.Threading.Channels; +using Microsoft.Extensions.Logging; namespace McpServer.Support.Mcp.Notifications; /// -/// Fan-out event bus backed by . Each subscriber -/// gets its own bounded channel; publish writes to all subscriber channels. +/// TR-MCP-EVT-001: Fan-out event bus backed by . Each subscriber gets +/// its own bounded channel; publish remains non-blocking by rejecting writes to full subscriber +/// buffers so overflow is logged instead of silently discarding older events. /// public sealed class ChannelChangeEventBus : IChangeEventBus { private const int MaxBufferSize = 1000; private readonly ConcurrentDictionary> _subscribers = new(); + private readonly ILogger _logger; + private long _rejectedSubscriberWrites; + + /// TR-MCP-EVT-001: Constructor. + /// Logger instance. + public ChannelChangeEventBus(ILogger logger) + { + _logger = logger; + } /// public ValueTask PublishAsync(ChangeEvent changeEvent, CancellationToken ct = default) { + var rejectedSubscribers = 0; foreach (var kvp in _subscribers) { - // Non-blocking write; drops if subscriber is full (BoundedChannelFullMode.DropOldest). - kvp.Value.Writer.TryWrite(changeEvent); + if (!kvp.Value.Writer.TryWrite(changeEvent)) + { + rejectedSubscribers++; + } + } + + if (rejectedSubscribers > 0) + { + var totalRejectedWrites = Interlocked.Add(ref _rejectedSubscriberWrites, rejectedSubscribers); + _logger.LogWarning( + "Change event delivery rejected {RejectedSubscriberCount} subscriber writes for {Category}/{Action} entity {EntityId} because subscriber buffers are full. TotalRejectedSubscriberWrites={TotalRejectedSubscriberWrites}", + rejectedSubscribers, + changeEvent.Category, + changeEvent.Action, + changeEvent.EntityId ?? "(none)", + totalRejectedWrites); } return ValueTask.CompletedTask; @@ -34,7 +60,7 @@ public async IAsyncEnumerable SubscribeAsync( var channel = Channel.CreateBounded( new BoundedChannelOptions(MaxBufferSize) { - FullMode = BoundedChannelFullMode.DropOldest, + FullMode = BoundedChannelFullMode.Wait, SingleWriter = false, SingleReader = true, }); diff --git a/src/McpServer.Services/Notifications/IChangeEventBus.cs b/src/McpServer.Services/Notifications/IChangeEventBus.cs index bb8b2ac3..a22e9dd1 100644 --- a/src/McpServer.Services/Notifications/IChangeEventBus.cs +++ b/src/McpServer.Services/Notifications/IChangeEventBus.cs @@ -1,6 +1,6 @@ namespace McpServer.Support.Mcp.Notifications; -/// In-process pub/sub for domain change events. +/// TR-MCP-EVT-001: In-process pub/sub contract for domain change events. public interface IChangeEventBus { /// Publish a change event to all subscribers. diff --git a/src/McpServer.Services/Options/DesktopLaunchOptions.cs b/src/McpServer.Services/Options/DesktopLaunchOptions.cs new file mode 100644 index 00000000..518bfe1f --- /dev/null +++ b/src/McpServer.Services/Options/DesktopLaunchOptions.cs @@ -0,0 +1,37 @@ +using System.Collections.Generic; + +namespace McpServer.Support.Mcp.Options; + +/// +/// FR-MCP-047/TR-MCP-DESKTOP-001: Configuration that gates privileged desktop-launch access, +/// remote authorization, and executable allowlisting. +/// +public sealed class DesktopLaunchOptions +{ + /// + /// FR-MCP-047/TR-MCP-DESKTOP-001: Configuration section containing desktop-launch controls. + /// + public const string SectionName = "Mcp:DesktopLaunch"; + + /// + /// FR-MCP-047/TR-MCP-DESKTOP-001: HTTP header carrying the privileged desktop-launch token. + /// + public const string AccessTokenHeaderName = "X-Desktop-Launch-Token"; + + /// + /// FR-MCP-047/TR-MCP-DESKTOP-001: Gets or sets a value indicating whether desktop launch is enabled. + /// + public bool Enabled { get; set; } + + /// + /// FR-MCP-047/TR-MCP-DESKTOP-001: Gets or sets the privileged HTTP token required for + /// remote desktop-launch requests. + /// + public string? AccessToken { get; set; } + + /// + /// FR-MCP-047/TR-MCP-DESKTOP-001: Gets or sets the executable allowlist patterns that + /// remote and local desktop-launch requests must match. + /// + public List AllowedExecutables { get; set; } = []; +} diff --git a/src/McpServer.Services/Services/ApiKeyIssuanceGuard.cs b/src/McpServer.Services/Services/ApiKeyIssuanceGuard.cs new file mode 100644 index 00000000..19a9c7c1 --- /dev/null +++ b/src/McpServer.Services/Services/ApiKeyIssuanceGuard.cs @@ -0,0 +1,54 @@ +using System.Collections.Concurrent; +using System.Net; + +namespace McpServer.Support.Mcp.Services; + +internal sealed class ApiKeyIssuanceGuard +{ + private const int PermitLimit = 30; + private static readonly TimeSpan Window = TimeSpan.FromMinutes(1); + private readonly ConcurrentDictionary _counters = new(StringComparer.Ordinal); + + public bool TryAcquire(IPAddress? remoteIp, out TimeSpan retryAfter) + { + var now = DateTimeOffset.UtcNow; + var key = remoteIp?.ToString() ?? "loopback"; + var counter = _counters.GetOrAdd(key, _ => new FixedWindowCounter(now)); + + lock (counter.Gate) + { + if ((now - counter.WindowStart) >= Window) + { + counter.WindowStart = now; + counter.Count = 0; + } + + if (counter.Count >= PermitLimit) + { + retryAfter = Window - (now - counter.WindowStart); + if (retryAfter < TimeSpan.Zero) + retryAfter = TimeSpan.Zero; + + return false; + } + + counter.Count++; + retryAfter = TimeSpan.Zero; + return true; + } + } + + private sealed class FixedWindowCounter + { + public FixedWindowCounter(DateTimeOffset windowStart) + { + WindowStart = windowStart; + } + + public object Gate { get; } = new(); + + public DateTimeOffset WindowStart { get; set; } + + public int Count { get; set; } + } +} diff --git a/src/McpServer.Services/Services/FileGitHubWorkspaceTokenStore.cs b/src/McpServer.Services/Services/FileGitHubWorkspaceTokenStore.cs index c6051fac..cae500ed 100644 --- a/src/McpServer.Services/Services/FileGitHubWorkspaceTokenStore.cs +++ b/src/McpServer.Services/Services/FileGitHubWorkspaceTokenStore.cs @@ -1,4 +1,7 @@ using System.Security.Cryptography; +using System.Runtime.Versioning; +using System.Security.AccessControl; +using System.Security.Principal; using System.Text.Json; using McpServer.Support.Mcp.Options; using Microsoft.AspNetCore.DataProtection; @@ -12,6 +15,7 @@ namespace McpServer.Support.Mcp.Services; /// public sealed class FileGitHubWorkspaceTokenStore : IGitHubWorkspaceTokenStore, IDisposable { + private const int StoreLockRetryDelayMilliseconds = 50; private static readonly JsonSerializerOptions s_jsonOptions = new() { PropertyNamingPolicy = JsonNamingPolicy.CamelCase, @@ -43,6 +47,7 @@ public FileGitHubWorkspaceTokenStore( await _gate.WaitAsync(ct).ConfigureAwait(false); try { + using var storeLock = await AcquireStoreLockAsync(ct).ConfigureAwait(false); var doc = await ReadUnlockedAsync(ct).ConfigureAwait(false); var match = doc.Entries.FirstOrDefault(e => string.Equals(e.WorkspacePath, normalized, StringComparison.OrdinalIgnoreCase)); if (match is null) @@ -78,6 +83,7 @@ public async Task UpsertAsync(string workspacePath, string accessToken, DateTime await _gate.WaitAsync(ct).ConfigureAwait(false); try { + using var storeLock = await AcquireStoreLockAsync(ct).ConfigureAwait(false); var doc = await ReadUnlockedAsync(ct).ConfigureAwait(false); var existing = doc.Entries.FirstOrDefault(e => string.Equals(e.WorkspacePath, normalized, StringComparison.OrdinalIgnoreCase)); if (existing is null) @@ -113,6 +119,7 @@ public async Task DeleteAsync(string workspacePath, CancellationToken ct = await _gate.WaitAsync(ct).ConfigureAwait(false); try { + using var storeLock = await AcquireStoreLockAsync(ct).ConfigureAwait(false); var doc = await ReadUnlockedAsync(ct).ConfigureAwait(false); var removed = doc.Entries.RemoveAll(e => string.Equals(e.WorkspacePath, normalized, StringComparison.OrdinalIgnoreCase)) > 0; if (removed) @@ -139,6 +146,7 @@ private async Task ReadUnlockedAsync(CancellationToken try { + EnsureRestrictedFilePermissions(path); var json = await File.ReadAllTextAsync(path, ct).ConfigureAwait(false); var doc = JsonSerializer.Deserialize(json, s_jsonOptions); return doc ?? new GitHubTokenStoreDocument(); @@ -159,8 +167,21 @@ private async Task WriteUnlockedAsync(GitHubTokenStoreDocument doc, Cancellation var json = JsonSerializer.Serialize(doc, s_jsonOptions); var tmp = path + "." + Guid.NewGuid().ToString("N") + ".tmp"; - await File.WriteAllTextAsync(tmp, json, ct).ConfigureAwait(false); - File.Move(tmp, path, true); + try + { + await File.WriteAllTextAsync(tmp, json, ct).ConfigureAwait(false); + EnsureRestrictedFilePermissions(tmp); + if (File.Exists(path)) + File.Replace(tmp, path, null, ignoreMetadataErrors: true); + else + File.Move(tmp, path); + EnsureRestrictedFilePermissions(path); + } + finally + { + if (File.Exists(tmp)) + File.Delete(tmp); + } } private string ResolveStorePath() @@ -174,6 +195,89 @@ private string ResolveStorePath() : Path.GetFullPath(Path.Combine(AppContext.BaseDirectory, configured)); } + private async Task AcquireStoreLockAsync(CancellationToken ct) + { + var lockPath = ResolveStorePath() + ".lock"; + var dir = Path.GetDirectoryName(lockPath); + if (!string.IsNullOrWhiteSpace(dir)) + Directory.CreateDirectory(dir); + + while (true) + { + ct.ThrowIfCancellationRequested(); + + try + { + var stream = new FileStream(lockPath, FileMode.OpenOrCreate, FileAccess.ReadWrite, FileShare.None); + try + { + EnsureRestrictedFilePermissions(lockPath); + return stream; + } + catch + { + stream.Dispose(); + throw; + } + } + catch (IOException) + { + await Task.Delay(StoreLockRetryDelayMilliseconds, ct).ConfigureAwait(false); + } + } + } + + private static void EnsureRestrictedFilePermissions(string path) + { + if (OperatingSystem.IsWindows()) + { + EnsureRestrictedWindowsFilePermissions(path); + return; + } + + if (OperatingSystem.IsLinux() || OperatingSystem.IsMacOS()) + { + File.SetUnixFileMode(path, UnixFileMode.UserRead | UnixFileMode.UserWrite); + return; + } + + throw new PlatformNotSupportedException("GitHub token-store permission hardening requires Windows ACL or Unix file-mode support."); + } + + [SupportedOSPlatform("windows")] + private static void EnsureRestrictedWindowsFilePermissions(string path) + { + using var identity = WindowsIdentity.GetCurrent(); + var currentUser = identity.User + ?? throw new InvalidOperationException("The current Windows identity did not expose a user SID for token-store ACL hardening."); + + var fileInfo = new FileInfo(path); + var security = fileInfo.GetAccessControl(); + security.SetAccessRuleProtection(isProtected: true, preserveInheritance: false); + + foreach (var existingRule in security + .GetAccessRules(includeExplicit: true, includeInherited: true, targetType: typeof(SecurityIdentifier)) + .OfType() + .ToArray()) + { + security.RemoveAccessRuleAll(existingRule); + } + + AddAllowRule(security, currentUser); + AddAllowRule(security, new SecurityIdentifier(WellKnownSidType.BuiltinAdministratorsSid, null)); + AddAllowRule(security, new SecurityIdentifier(WellKnownSidType.LocalSystemSid, null)); + fileInfo.SetAccessControl(security); + } + + [SupportedOSPlatform("windows")] + private static void AddAllowRule(FileSecurity security, SecurityIdentifier identity) + { + security.AddAccessRule(new FileSystemAccessRule( + identity, + FileSystemRights.FullControl, + AccessControlType.Allow)); + } + private static string NormalizeWorkspacePath(string workspacePath) { if (string.IsNullOrWhiteSpace(workspacePath)) diff --git a/src/McpServer.Services/Services/GitHubCliService.cs b/src/McpServer.Services/Services/GitHubCliService.cs index 1e4457b9..26db1d30 100644 --- a/src/McpServer.Services/Services/GitHubCliService.cs +++ b/src/McpServer.Services/Services/GitHubCliService.cs @@ -1,3 +1,5 @@ +using System.Globalization; +using System.Text; using System.Text.Json; using McpServer.Support.Mcp.Models; using McpServer.Support.Mcp.Options; @@ -13,6 +15,17 @@ namespace McpServer.Support.Mcp.Services; public sealed class GitHubCliService : IGitHubCliService { private const string GhExe = "gh"; + private static readonly HashSet AllowedIssueStates = new(StringComparer.OrdinalIgnoreCase) + { + "open", + "closed", + "all" + }; + private static readonly HashSet AllowedCloseReasons = new(StringComparer.OrdinalIgnoreCase) + { + "completed", + "not_planned" + }; private readonly IProcessRunner _processRunner; private readonly ILogger _logger; private readonly IGitHubWorkspaceTokenStore? _tokenStore; @@ -40,12 +53,26 @@ public GitHubCliService( /// public async Task ListIssuesAsync(string? state, int limit, CancellationToken cancellationToken = default) { - var args = $"issue list --limit {Math.Clamp(limit, 1, 100)} --json number,title,url,state"; - if (!string.IsNullOrWhiteSpace(state)) + if (!TryNormalizeIssueState(state, out var normalizedState, out var errorMessage)) + return new GitHubIssueListResult(false, errorMessage, Array.Empty()); + + var args = new List + { + "issue", + "list", + "--limit", + Math.Clamp(limit, 1, 100).ToString(CultureInfo.InvariantCulture), + "--json", + "number,title,url,state" + }; + + if (normalizedState is not null) { - args += " --state " + state.Trim(); + args.Add("--state"); + args.Add(normalizedState); } - var result = await RunGhAsync(args, cancellationToken).ConfigureAwait(false); + + var result = await RunGhAsync(BuildArguments(args), cancellationToken).ConfigureAwait(false); if (result.ExitCode != 0) { return new GitHubIssueListResult(false, result.Stderr ?? "gh failed", Array.Empty()); @@ -57,12 +84,26 @@ public async Task ListIssuesAsync(string? state, int limi /// public async Task ListPullsAsync(string? state, int limit, CancellationToken cancellationToken = default) { - var args = $"pr list --limit {Math.Clamp(limit, 1, 100)} --json number,title,url,state"; - if (!string.IsNullOrWhiteSpace(state)) + if (!TryNormalizeIssueState(state, out var normalizedState, out var errorMessage)) + return new GitHubPullListResult(false, errorMessage, Array.Empty()); + + var args = new List { - args += " --state " + state.Trim(); + "pr", + "list", + "--limit", + Math.Clamp(limit, 1, 100).ToString(CultureInfo.InvariantCulture), + "--json", + "number,title,url,state" + }; + + if (normalizedState is not null) + { + args.Add("--state"); + args.Add(normalizedState); } - var result = await RunGhAsync(args, cancellationToken).ConfigureAwait(false); + + var result = await RunGhAsync(BuildArguments(args), cancellationToken).ConfigureAwait(false); if (result.ExitCode != 0) { return new GitHubPullListResult(false, result.Stderr ?? "gh failed", Array.Empty()); @@ -75,12 +116,14 @@ public async Task ListPullsAsync(string? state, int limit, public async Task CreateIssueAsync(string title, string? body, CancellationToken cancellationToken = default) { ArgumentNullException.ThrowIfNull(title); - var args = $"issue create --title \"{EscapeArg(title)}\""; + var args = new List { "issue", "create", "--title", title }; if (!string.IsNullOrWhiteSpace(body)) { - args += " --body \"" + EscapeArg(body) + "\""; + args.Add("--body"); + args.Add(body); } - var result = await RunGhAsync(args, cancellationToken).ConfigureAwait(false); + + var result = await RunGhAsync(BuildArguments(args), cancellationToken).ConfigureAwait(false); if (result.ExitCode != 0) { return new GitHubCreateIssueResult(false, null, null, result.Stderr ?? "gh failed"); @@ -95,7 +138,8 @@ public async Task CommentOnIssueAsync(string issueId, strin { ArgumentNullException.ThrowIfNull(issueId); ArgumentNullException.ThrowIfNull(body); - var args = $"issue comment {issueId.Trim()} --body \"{EscapeArg(body)}\""; + var normalizedIssueId = NormalizeTargetIdentifier(issueId, nameof(issueId)); + var args = BuildArguments("issue", "comment", "--body", body, "--", normalizedIssueId); var result = await RunGhAsync(args, cancellationToken).ConfigureAwait(false); return new GitHubCommentResult(result.ExitCode == 0, result.ExitCode != 0 ? result.Stderr : null); } @@ -105,7 +149,8 @@ public async Task CommentOnPullAsync(string prId, string bo { ArgumentNullException.ThrowIfNull(prId); ArgumentNullException.ThrowIfNull(body); - var args = $"pr comment {prId.Trim()} --body \"{EscapeArg(body)}\""; + var normalizedPullId = NormalizeTargetIdentifier(prId, nameof(prId)); + var args = BuildArguments("pr", "comment", "--body", body, "--", normalizedPullId); var result = await RunGhAsync(args, cancellationToken).ConfigureAwait(false); return new GitHubCommentResult(result.ExitCode == 0, result.ExitCode != 0 ? result.Stderr : null); } @@ -113,7 +158,12 @@ public async Task CommentOnPullAsync(string prId, string bo /// public async Task GetIssueAsync(int issueNumber, CancellationToken ct = default) { - var args = $"issue view {issueNumber} --json number,title,body,state,url,labels,assignees,milestone,createdAt,updatedAt,closedAt,author,comments"; + var args = BuildArguments( + "issue", + "view", + issueNumber.ToString(CultureInfo.InvariantCulture), + "--json", + "number,title,body,state,url,labels,assignees,milestone,createdAt,updatedAt,closedAt,author,comments"); var result = await RunGhAsync(args, ct).ConfigureAwait(false); if (result.ExitCode != 0) return new GitHubIssueDetailResult(false, null, result.Stderr ?? "gh failed"); @@ -127,19 +177,62 @@ public async Task GetIssueAsync(int issueNumber, Cancel public async Task UpdateIssueAsync(int issueNumber, GitHubIssueUpdateRequest request, CancellationToken ct = default) { ArgumentNullException.ThrowIfNull(request); - var args = $"issue edit {issueNumber}"; - if (request.Title is not null) args += $" --title \"{EscapeArg(request.Title)}\""; - if (request.Body is not null) args += $" --body \"{EscapeArg(request.Body)}\""; + var args = new List { "issue", "edit", issueNumber.ToString(CultureInfo.InvariantCulture) }; + if (request.Title is not null) + { + args.Add("--title"); + args.Add(request.Title); + } + + if (request.Body is not null) + { + args.Add("--body"); + args.Add(request.Body); + } + if (request.AddLabels is { Count: > 0 }) - foreach (var label in request.AddLabels) args += $" --add-label \"{EscapeArg(label)}\""; + { + foreach (var label in request.AddLabels) + { + args.Add("--add-label"); + args.Add(label); + } + } + if (request.RemoveLabels is { Count: > 0 }) - foreach (var label in request.RemoveLabels) args += $" --remove-label \"{EscapeArg(label)}\""; + { + foreach (var label in request.RemoveLabels) + { + args.Add("--remove-label"); + args.Add(label); + } + } + if (request.AddAssignees is { Count: > 0 }) - foreach (var assignee in request.AddAssignees) args += $" --add-assignee \"{EscapeArg(assignee)}\""; + { + foreach (var assignee in request.AddAssignees) + { + args.Add("--add-assignee"); + args.Add(assignee); + } + } + if (request.RemoveAssignees is { Count: > 0 }) - foreach (var assignee in request.RemoveAssignees) args += $" --remove-assignee \"{EscapeArg(assignee)}\""; - if (request.Milestone is not null) args += $" --milestone \"{EscapeArg(request.Milestone)}\""; - var result = await RunGhAsync(args, ct).ConfigureAwait(false); + { + foreach (var assignee in request.RemoveAssignees) + { + args.Add("--remove-assignee"); + args.Add(assignee); + } + } + + if (request.Milestone is not null) + { + args.Add("--milestone"); + args.Add(request.Milestone); + } + + var result = await RunGhAsync(BuildArguments(args), ct).ConfigureAwait(false); if (result.ExitCode != 0) return new GitHubMutationResult(false, null, result.Stderr ?? "gh failed"); return new GitHubMutationResult(true, result.Stdout?.Trim(), null); @@ -148,10 +241,17 @@ public async Task UpdateIssueAsync(int issueNumber, GitHub /// public async Task CloseIssueAsync(int issueNumber, string? reason = null, CancellationToken ct = default) { - var args = $"issue close {issueNumber}"; - if (!string.IsNullOrWhiteSpace(reason)) - args += $" --reason {reason.Trim()}"; - var result = await RunGhAsync(args, ct).ConfigureAwait(false); + if (!TryNormalizeCloseReason(reason, out var normalizedReason, out var errorMessage)) + return new GitHubMutationResult(false, null, errorMessage); + + var args = new List { "issue", "close", issueNumber.ToString(CultureInfo.InvariantCulture) }; + if (normalizedReason is not null) + { + args.Add("--reason"); + args.Add(normalizedReason); + } + + var result = await RunGhAsync(BuildArguments(args), ct).ConfigureAwait(false); if (result.ExitCode != 0) return new GitHubMutationResult(false, null, result.Stderr ?? "gh failed"); return new GitHubMutationResult(true, result.Stdout?.Trim(), null); @@ -160,7 +260,7 @@ public async Task CloseIssueAsync(int issueNumber, string? /// public async Task ReopenIssueAsync(int issueNumber, CancellationToken ct = default) { - var args = $"issue reopen {issueNumber}"; + var args = BuildArguments("issue", "reopen", issueNumber.ToString(CultureInfo.InvariantCulture)); var result = await RunGhAsync(args, ct).ConfigureAwait(false); if (result.ExitCode != 0) return new GitHubMutationResult(false, null, result.Stderr ?? "gh failed"); @@ -170,7 +270,7 @@ public async Task ReopenIssueAsync(int issueNumber, Cancel /// public async Task ListIssueLabelsAsync(CancellationToken ct = default) { - var args = "label list --json name,color,description --limit 100"; + var args = BuildArguments("label", "list", "--json", "name,color,description", "--limit", "100"); var result = await RunGhAsync(args, ct).ConfigureAwait(false); if (result.ExitCode != 0) return new GitHubLabelsResult(false, null, result.Stderr ?? "gh failed"); @@ -182,13 +282,21 @@ public async Task ListIssueLabelsAsync(CancellationToken ct public async Task ListWorkflowRunsAsync(GitHubWorkflowRunQuery query, CancellationToken ct = default) { query ??= new GitHubWorkflowRunQuery(); - var args = $"run list --limit {Math.Clamp(query.Limit, 1, 100)} --json databaseId,workflowName,displayTitle,headBranch,status,conclusion,event,url,createdAt,updatedAt"; - if (!string.IsNullOrWhiteSpace(query.Branch)) args += $" --branch \"{EscapeArg(query.Branch)}\""; - if (!string.IsNullOrWhiteSpace(query.Status)) args += $" --status \"{EscapeArg(query.Status)}\""; - if (!string.IsNullOrWhiteSpace(query.Event)) args += $" --event \"{EscapeArg(query.Event)}\""; - if (!string.IsNullOrWhiteSpace(query.Workflow)) args += $" --workflow \"{EscapeArg(query.Workflow)}\""; - - var result = await RunGhAsync(args, ct).ConfigureAwait(false); + var args = new List + { + "run", + "list", + "--limit", + Math.Clamp(query.Limit, 1, 100).ToString(CultureInfo.InvariantCulture), + "--json", + "databaseId,workflowName,displayTitle,headBranch,status,conclusion,event,url,createdAt,updatedAt" + }; + AddOption(args, "--branch", query.Branch); + AddOption(args, "--status", query.Status); + AddOption(args, "--event", query.Event); + AddOption(args, "--workflow", query.Workflow); + + var result = await RunGhAsync(BuildArguments(args), ct).ConfigureAwait(false); if (result.ExitCode != 0) return new GitHubWorkflowRunListResult(false, Array.Empty(), result.Stderr ?? "gh failed"); @@ -198,7 +306,12 @@ public async Task ListWorkflowRunsAsync(GitHubWorkf /// public async Task GetWorkflowRunAsync(long runId, CancellationToken ct = default) { - var args = $"run view {runId} --json databaseId,workflowName,displayTitle,headBranch,headSha,status,conclusion,event,url,attempt,createdAt,updatedAt,jobs"; + var args = BuildArguments( + "run", + "view", + runId.ToString(CultureInfo.InvariantCulture), + "--json", + "databaseId,workflowName,displayTitle,headBranch,headSha,status,conclusion,event,url,attempt,createdAt,updatedAt,jobs"); var result = await RunGhAsync(args, ct).ConfigureAwait(false); if (result.ExitCode != 0) return new GitHubWorkflowRunDetailResult(false, null, result.Stderr ?? "gh failed"); @@ -212,7 +325,7 @@ public async Task GetWorkflowRunAsync(long runId, /// public async Task RerunWorkflowRunAsync(long runId, CancellationToken ct = default) { - var args = $"run rerun {runId}"; + var args = BuildArguments("run", "rerun", runId.ToString(CultureInfo.InvariantCulture)); var result = await RunGhAsync(args, ct).ConfigureAwait(false); if (result.ExitCode != 0) return new GitHubMutationResult(false, null, result.Stderr ?? "gh failed"); @@ -222,7 +335,7 @@ public async Task RerunWorkflowRunAsync(long runId, Cancel /// public async Task CancelWorkflowRunAsync(long runId, CancellationToken ct = default) { - var args = $"run cancel {runId}"; + var args = BuildArguments("run", "cancel", runId.ToString(CultureInfo.InvariantCulture)); var result = await RunGhAsync(args, ct).ConfigureAwait(false); if (result.ExitCode != 0) return new GitHubMutationResult(false, null, result.Stderr ?? "gh failed"); @@ -287,9 +400,126 @@ private IReadOnlyList ParsePullList(string? json) return null; } - private static string EscapeArg(string s) + private static bool TryNormalizeIssueState(string? state, out string? normalizedState, out string? errorMessage) + { + if (string.IsNullOrWhiteSpace(state)) + { + normalizedState = null; + errorMessage = null; + return true; + } + + normalizedState = state.Trim().ToLowerInvariant(); + if (AllowedIssueStates.Contains(normalizedState)) + { + errorMessage = null; + return true; + } + + normalizedState = null; + errorMessage = "Invalid state. Allowed values: open, closed, all."; + return false; + } + + private static bool TryNormalizeCloseReason(string? reason, out string? normalizedReason, out string? errorMessage) + { + if (string.IsNullOrWhiteSpace(reason)) + { + normalizedReason = null; + errorMessage = null; + return true; + } + + normalizedReason = reason.Trim().ToLowerInvariant(); + if (AllowedCloseReasons.Contains(normalizedReason)) + { + errorMessage = null; + return true; + } + + normalizedReason = null; + errorMessage = "Invalid close reason. Allowed values: completed, not_planned."; + return false; + } + + private static string NormalizeTargetIdentifier(string value, string paramName) + { + ArgumentException.ThrowIfNullOrWhiteSpace(value, paramName); + return value.Trim(); + } + + private static void AddOption(List args, string option, string? value) { - return s.Replace("\\", "\\\\", StringComparison.Ordinal).Replace("\"", "\\\"", StringComparison.Ordinal); + if (string.IsNullOrWhiteSpace(value)) + return; + + args.Add(option); + args.Add(value); + } + + private static string BuildArguments(IEnumerable arguments) + { + return string.Join(" ", arguments.Select(QuoteArgument)); + } + + private static string BuildArguments(params string[] arguments) + { + return BuildArguments((IEnumerable)arguments); + } + + private static string QuoteArgument(string argument) + { + ArgumentNullException.ThrowIfNull(argument); + + if (argument.Length == 0) + return "\"\""; + + var requiresQuoting = false; + for (var i = 0; i < argument.Length; i++) + { + if (char.IsWhiteSpace(argument[i]) || argument[i] == '"') + { + requiresQuoting = true; + break; + } + } + + if (!requiresQuoting) + return argument; + + var builder = new StringBuilder(argument.Length + 2); + builder.Append('"'); + var backslashCount = 0; + foreach (var character in argument) + { + if (character == '\\') + { + backslashCount++; + continue; + } + + if (character == '"') + { + builder.Append('\\', backslashCount * 2 + 1); + builder.Append('"'); + backslashCount = 0; + continue; + } + + if (backslashCount > 0) + { + builder.Append('\\', backslashCount); + backslashCount = 0; + } + + builder.Append(character); + } + + if (backslashCount > 0) + builder.Append('\\', backslashCount * 2); + + builder.Append('"'); + return builder.ToString(); } private async Task RunGhAsync(string args, CancellationToken ct) diff --git a/src/McpServer.Services/Services/HostedMcpAgentExecutionStrategy.cs b/src/McpServer.Services/Services/HostedMcpAgentExecutionStrategy.cs index b0b2905d..50ad2bbf 100644 --- a/src/McpServer.Services/Services/HostedMcpAgentExecutionStrategy.cs +++ b/src/McpServer.Services/Services/HostedMcpAgentExecutionStrategy.cs @@ -7,6 +7,7 @@ using McpServer.McpAgent.Todo; using McpServer.Client; using McpServer.Common.Copilot; +using McpServer.Support.Mcp.Options; using Microsoft.Agents.AI; using Microsoft.Extensions.AI; using Microsoft.Extensions.Options; @@ -16,6 +17,7 @@ namespace McpServer.Support.Mcp.Services; internal sealed class HostedMcpAgentExecutionStrategy( ICopilotClient copilotClient, WorkspaceTokenService workspaceTokenService, + IOptions desktopLaunchOptions, ServerRuntimeInfo serverRuntimeInfo, IServiceProvider serviceProvider, ILogger logger) @@ -34,9 +36,10 @@ public ValueTask CreateSessionAsync( ? request.Options.WorkingDirectory ?? Environment.CurrentDirectory : request.WorkspacePath; var apiKey = workspaceTokenService.GetToken(workspacePath) ?? workspaceTokenService.GenerateToken(workspacePath); + var desktopLaunchToken = desktopLaunchOptions.Value.AccessToken; var baseUrl = new Uri($"http://127.0.0.1:{serverRuntimeInfo.ListenPort}"); var hostedTimeout = ResolveHostedTimeout(request.Options.Timeout); - var hostedOptions = CreateHostedAgentOptions(request, workspacePath, baseUrl, apiKey, hostedTimeout); + var hostedOptions = CreateHostedAgentOptions(request, workspacePath, baseUrl, apiKey, desktopLaunchToken, hostedTimeout); var httpClient = new HttpClient { Timeout = hostedTimeout, @@ -47,6 +50,7 @@ public ValueTask CreateSessionAsync( { ApiKey = apiKey, BaseUrl = baseUrl, + DesktopLaunchToken = desktopLaunchToken, Timeout = httpClient.Timeout, WorkspacePath = workspacePath, }); @@ -82,6 +86,7 @@ private static McpAgentOptions CreateHostedAgentOptions( string workspacePath, Uri baseUrl, string apiKey, + string? desktopLaunchToken, TimeSpan timeout) { var agentName = BuildHostedAgentName(request.AgentName); @@ -91,6 +96,7 @@ private static McpAgentOptions CreateHostedAgentOptions( AgentName = agentName, ApiKey = apiKey, BaseUrl = baseUrl, + DesktopLaunchToken = desktopLaunchToken, Description = $"Hosted MCP Agent execution strategy for {agentName}.", RequireAuthentication = true, SourceType = McpHostedAgentDefaults.DefaultSourceType, @@ -352,13 +358,23 @@ public async ValueTask DisposeAsync() return; _disposed = true; - if (_session is not null) - await _session.DisposeAsync().ConfigureAwait(false); + var session = Interlocked.Exchange(ref _session, null); + if (session is not null) + await session.DisposeAsync().ConfigureAwait(false); _gate.Dispose(); } - public void Dispose() => DisposeAsync().AsTask().GetAwaiter().GetResult(); + public void Dispose() + { + if (_disposed) + return; + + _disposed = true; + var session = Interlocked.Exchange(ref _session, null); + session?.Dispose(); + _gate.Dispose(); + } private async Task SendPromptAsync(string prompt, CancellationToken cancellationToken) { diff --git a/src/McpServer.Services/Services/ITodoService.cs b/src/McpServer.Services/Services/ITodoService.cs index 9b7c0e29..5632c054 100644 --- a/src/McpServer.Services/Services/ITodoService.cs +++ b/src/McpServer.Services/Services/ITodoService.cs @@ -25,6 +25,12 @@ public interface ITodoService /// Get append-only audit history for a TODO item. Task GetAuditAsync(string id, int limit = 50, int offset = 0, CancellationToken cancellationToken = default); + + /// TR-MCP-TODO-006: Get projection status for SQLite-authoritative TODO storage. + Task GetProjectionStatusAsync(CancellationToken cancellationToken = default); + + /// TR-MCP-TODO-006: Repair TODO.yaml projection from SQLite-authoritative TODO storage. + Task RepairProjectionAsync(CancellationToken cancellationToken = default); } /// TR-PLANNED-013: Query parameters for searching TODO items. @@ -247,6 +253,27 @@ public sealed record TodoMutationResult( /// TR-MCP-TODO-005: Result of querying TODO audit history. public sealed record TodoAuditQueryResult(IReadOnlyList Entries, int TotalCount); +/// TR-MCP-TODO-006: Status of SQLite-authoritative TODO.yaml projection health and consistency. +public sealed record TodoProjectionStatusResult( + string AuthoritativeStore, + string AuthoritativeDataSource, + string ProjectionTargetPath, + bool ProjectionTargetExists, + bool ProjectionConsistent, + bool RepairRequired, + string VerifiedAtUtc, + string? LastImportedFromYamlUtc = null, + string? LastProjectedToYamlUtc = null, + string? LastProjectionFailureUtc = null, + string? LastProjectionFailure = null, + string? Message = null); + +/// TR-MCP-TODO-006: Result of an operator-requested TODO.yaml projection repair attempt. +public sealed record TodoProjectionRepairResult( + bool Success, + string? Error, + TodoProjectionStatusResult Status); + /// TR-MCP-TODO-005: Append-only audit entry for a TODO item. public sealed record TodoAuditEntry { diff --git a/src/McpServer.Services/Services/InteractionLogSubmissionQueue.cs b/src/McpServer.Services/Services/InteractionLogSubmissionQueue.cs index f97dca23..807d740b 100644 --- a/src/McpServer.Services/Services/InteractionLogSubmissionQueue.cs +++ b/src/McpServer.Services/Services/InteractionLogSubmissionQueue.cs @@ -7,7 +7,8 @@ namespace McpServer.Support.Mcp.Services; /// -/// TR-PLANNED-013: Channel-based buffer for interaction log entries. Non-blocking enqueue; async dequeue for background submission. +/// TR-PLANNED-013: Channel-based buffer for interaction log entries. Non-blocking enqueue explicitly rejects +/// new entries when the buffer is full, and async dequeue supports background submission. /// public sealed class InteractionLogSubmissionChannel : IInteractionLogSubmissionChannel { @@ -25,7 +26,8 @@ public InteractionLogSubmissionChannel(IOptions op var capacity = options?.Value?.QueueCapacity ?? 1000; _channel = Channel.CreateBounded(new BoundedChannelOptions(capacity) { - FullMode = BoundedChannelFullMode.DropOldest + FullMode = BoundedChannelFullMode.Wait, + SingleReader = true, }); } diff --git a/src/McpServer.Services/Services/PairingLoginAttemptGuard.cs b/src/McpServer.Services/Services/PairingLoginAttemptGuard.cs new file mode 100644 index 00000000..1fb1df38 --- /dev/null +++ b/src/McpServer.Services/Services/PairingLoginAttemptGuard.cs @@ -0,0 +1,208 @@ +using System.Collections.Concurrent; +using System.Net; + +namespace McpServer.Support.Mcp.Services; + +/// +/// FR-MCP-014: Tracks failed pairing sign-in attempts and enforces temporary lockouts. +/// +internal sealed class PairingLoginAttemptGuard +{ + private const int FailedAttemptThreshold = 5; + private const int FailedAttemptIpLimit = 20; + private static readonly TimeSpan FailedAttemptWindow = TimeSpan.FromMinutes(15); + private static readonly TimeSpan IpWindow = TimeSpan.FromMinutes(5); + private static readonly TimeSpan BaseLockout = TimeSpan.FromMinutes(1); + private static readonly TimeSpan MaxLockout = TimeSpan.FromMinutes(15); + + private readonly ConcurrentDictionary _ipFailures = new(StringComparer.Ordinal); + private readonly ConcurrentDictionary _principalFailures = new(StringComparer.OrdinalIgnoreCase); + private readonly Func _utcNow; + + /// + /// Initializes a new instance of the class. + /// + public PairingLoginAttemptGuard() + : this(static () => DateTimeOffset.UtcNow) + { + } + + internal PairingLoginAttemptGuard(Func utcNow) + { + _utcNow = utcNow; + } + + /// + /// Returns true when a pairing sign-in attempt is currently permitted. + /// + public bool TryAcquire(string? username, IPAddress? remoteIp, out TimeSpan retryAfter) + { + var now = _utcNow(); + PurgeExpired(now); + + retryAfter = TimeSpan.Zero; + var ipKey = BuildIpKey(remoteIp); + if (_ipFailures.TryGetValue(ipKey, out var ipCounter)) + { + lock (ipCounter.Gate) + { + if ((now - ipCounter.WindowStart) >= IpWindow) + { + ipCounter.WindowStart = now; + ipCounter.Count = 0; + } + else if (ipCounter.Count >= FailedAttemptIpLimit) + { + retryAfter = IpWindow - (now - ipCounter.WindowStart); + if (retryAfter < TimeSpan.Zero) + retryAfter = TimeSpan.Zero; + + return false; + } + } + } + + var principalKey = BuildPrincipalKey(username, remoteIp); + if (_principalFailures.TryGetValue(principalKey, out var principalState)) + { + lock (principalState.Gate) + { + if ((now - principalState.LastFailureUtc) >= FailedAttemptWindow) + { + principalState.FailureCount = 0; + principalState.LockedUntilUtc = null; + } + + if (principalState.LockedUntilUtc is { } lockedUntilUtc && lockedUntilUtc > now) + { + var principalRetryAfter = lockedUntilUtc - now; + if (principalRetryAfter > retryAfter) + retryAfter = principalRetryAfter; + + return false; + } + } + } + + return true; + } + + /// + /// Records a failed pairing sign-in attempt for the supplied user and remote IP. + /// + public void RecordFailure(string? username, IPAddress? remoteIp) + { + var now = _utcNow(); + PurgeExpired(now); + + var ipKey = BuildIpKey(remoteIp); + var ipCounter = _ipFailures.GetOrAdd(ipKey, _ => new FixedWindowCounter(now)); + lock (ipCounter.Gate) + { + if ((now - ipCounter.WindowStart) >= IpWindow) + { + ipCounter.WindowStart = now; + ipCounter.Count = 0; + } + + ipCounter.Count++; + } + + var principalKey = BuildPrincipalKey(username, remoteIp); + var principalState = _principalFailures.GetOrAdd(principalKey, _ => new PrincipalFailureState(now)); + lock (principalState.Gate) + { + if ((now - principalState.LastFailureUtc) >= FailedAttemptWindow) + { + principalState.FailureCount = 0; + principalState.LockedUntilUtc = null; + } + + principalState.FailureCount++; + principalState.LastFailureUtc = now; + + if (principalState.FailureCount >= FailedAttemptThreshold) + { + var multiplier = 1 << Math.Min(principalState.FailureCount - FailedAttemptThreshold, 4); + var lockoutTicks = Math.Min(BaseLockout.Ticks * multiplier, MaxLockout.Ticks); + principalState.LockedUntilUtc = now.AddTicks(lockoutTicks); + } + } + } + + /// + /// Clears accumulated pairing sign-in failures for the supplied user and remote IP. + /// + public void RecordSuccess(string? username, IPAddress? remoteIp) + { + _principalFailures.TryRemove(BuildPrincipalKey(username, remoteIp), out _); + } + + private void PurgeExpired(DateTimeOffset now) + { + foreach (var entry in _ipFailures) + { + var remove = false; + lock (entry.Value.Gate) + { + remove = (now - entry.Value.WindowStart) >= IpWindow; + } + + if (remove) + _ipFailures.TryRemove(entry.Key, out _); + } + + foreach (var entry in _principalFailures) + { + var remove = false; + lock (entry.Value.Gate) + { + var lastRelevantUtc = entry.Value.LockedUntilUtc is { } lockedUntilUtc && lockedUntilUtc > entry.Value.LastFailureUtc + ? lockedUntilUtc + : entry.Value.LastFailureUtc; + remove = (now - lastRelevantUtc) >= FailedAttemptWindow; + } + + if (remove) + _principalFailures.TryRemove(entry.Key, out _); + } + } + + private static string BuildIpKey(IPAddress? remoteIp) => remoteIp?.ToString() ?? "loopback"; + + private static string BuildPrincipalKey(string? username, IPAddress? remoteIp) + => $"{BuildIpKey(remoteIp)}|{NormalizeUsername(username)}"; + + private static string NormalizeUsername(string? username) + => string.IsNullOrWhiteSpace(username) ? "(empty)" : username.Trim(); + + private sealed class FixedWindowCounter + { + public FixedWindowCounter(DateTimeOffset windowStart) + { + WindowStart = windowStart; + } + + public object Gate { get; } = new(); + + public DateTimeOffset WindowStart { get; set; } + + public int Count { get; set; } + } + + private sealed class PrincipalFailureState + { + public PrincipalFailureState(DateTimeOffset lastFailureUtc) + { + LastFailureUtc = lastFailureUtc; + } + + public object Gate { get; } = new(); + + public int FailureCount { get; set; } + + public DateTimeOffset LastFailureUtc { get; set; } + + public DateTimeOffset? LockedUntilUtc { get; set; } + } +} diff --git a/src/McpServer.Services/Services/PathGlobMatcher.cs b/src/McpServer.Services/Services/PathGlobMatcher.cs new file mode 100644 index 00000000..7cdd5732 --- /dev/null +++ b/src/McpServer.Services/Services/PathGlobMatcher.cs @@ -0,0 +1,134 @@ +using System; +using System.Collections.Generic; +using System.IO.Enumeration; + +namespace McpServer.Support.Mcp.Services; + +/// +/// TR-PLANNED-013/TR-MCP-DESKTOP-001: Shared segment-wise glob matcher used by repo and +/// desktop-launch path allowlists. +/// +internal static class PathGlobMatcher +{ + private static readonly char[] s_trimSlashChars = ['/', '\\']; + + /// + /// TR-PLANNED-013/TR-MCP-DESKTOP-001: Returns whether the candidate path matches any configured glob pattern. + /// + /// Candidate path to evaluate. + /// Glob patterns using *, ?, and ** semantics. + /// when any pattern matches the candidate path; otherwise . + public static bool MatchesAny(string candidatePath, IReadOnlyList patterns) + { + ArgumentNullException.ThrowIfNull(patterns); + var candidateSegments = SplitPathSegments(NormalizePath(candidatePath)); + foreach (var pattern in patterns) + { + var patternSegments = SplitPathSegments(NormalizePath(pattern)); + if (GlobMatches(candidateSegments, 0, patternSegments, 0)) + return true; + } + + return false; + } + + /// + /// TR-PLANNED-013/TR-MCP-DESKTOP-001: Returns whether the directory path is an ancestor + /// of any configured glob pattern. + /// + /// Directory path to evaluate. + /// Glob patterns using *, ?, and ** semantics. + /// when the directory can lead to an allowed path; otherwise . + public static bool MayMatchDirectoryPrefix(string directoryPath, IReadOnlyList patterns) + { + ArgumentNullException.ThrowIfNull(patterns); + var directorySegments = SplitPathSegments(NormalizePath(directoryPath)); + foreach (var pattern in patterns) + { + var patternSegments = SplitPathSegments(NormalizePath(pattern)); + if (PatternMayMatchDirectoryPrefix(directorySegments, 0, patternSegments, 0)) + return true; + } + + return false; + } + + private static bool GlobMatches(string[] candidateSegments, int candidateIndex, string[] patternSegments, int patternIndex) + { + patternIndex = CollapseDoubleStar(patternSegments, patternIndex); + if (patternIndex == patternSegments.Length) + return candidateIndex == candidateSegments.Length; + + if (patternSegments[patternIndex] == "**") + { + if (patternIndex == patternSegments.Length - 1) + return true; + + for (var nextCandidateIndex = candidateIndex; nextCandidateIndex <= candidateSegments.Length; nextCandidateIndex++) + { + if (GlobMatches(candidateSegments, nextCandidateIndex, patternSegments, patternIndex + 1)) + return true; + } + + return false; + } + + if (candidateIndex == candidateSegments.Length) + return false; + + if (!FileSystemName.MatchesSimpleExpression(patternSegments[patternIndex], candidateSegments[candidateIndex], ignoreCase: true)) + return false; + + return GlobMatches(candidateSegments, candidateIndex + 1, patternSegments, patternIndex + 1); + } + + private static bool PatternMayMatchDirectoryPrefix(string[] directorySegments, int directoryIndex, string[] patternSegments, int patternIndex) + { + patternIndex = CollapseDoubleStar(patternSegments, patternIndex); + if (directoryIndex == directorySegments.Length) + return true; + + if (patternIndex == patternSegments.Length) + return false; + + if (patternSegments[patternIndex] == "**") + { + return PatternMayMatchDirectoryPrefix(directorySegments, directoryIndex, patternSegments, patternIndex + 1) + || PatternMayMatchDirectoryPrefix(directorySegments, directoryIndex + 1, patternSegments, patternIndex); + } + + if (!FileSystemName.MatchesSimpleExpression(patternSegments[patternIndex], directorySegments[directoryIndex], ignoreCase: true)) + return false; + + return PatternMayMatchDirectoryPrefix(directorySegments, directoryIndex + 1, patternSegments, patternIndex + 1); + } + + private static int CollapseDoubleStar(string[] patternSegments, int patternIndex) + { + while (patternIndex + 1 < patternSegments.Length + && patternSegments[patternIndex] == "**" + && patternSegments[patternIndex + 1] == "**") + { + patternIndex++; + } + + return patternIndex; + } + + private static string NormalizePath(string? path) + { + if (string.IsNullOrWhiteSpace(path)) + return "."; + + var normalized = path.Replace('\\', '/').TrimStart(s_trimSlashChars); + return string.IsNullOrEmpty(normalized) ? "." : normalized; + } + + private static string[] SplitPathSegments(string path) + { + if (string.IsNullOrWhiteSpace(path) || string.Equals(path, ".", StringComparison.Ordinal)) + return []; + + return path.Split('/', StringSplitOptions.RemoveEmptyEntries | StringSplitOptions.TrimEntries); + } +} diff --git a/src/McpServer.Services/Services/RepoFileService.cs b/src/McpServer.Services/Services/RepoFileService.cs index 6032709b..d693a68d 100644 --- a/src/McpServer.Services/Services/RepoFileService.cs +++ b/src/McpServer.Services/Services/RepoFileService.cs @@ -53,7 +53,7 @@ public Task ListAsync(string? relativePath, CancellationToken ca var dir = NormalizeRelative(relativePath ?? "."); if (!TryResolveFullPath(dir, out var fullPath) || !Directory.Exists(fullPath)) return Task.FromResult(new RepoListResult(dir, Array.Empty())); - if (!IsAllowed(dir)) return Task.FromResult(new RepoListResult(dir, Array.Empty())); + if (!CanListPath(dir)) return Task.FromResult(new RepoListResult(dir, Array.Empty())); var entries = new List(); foreach (var entry in Directory.EnumerateFileSystemEntries(fullPath)) @@ -62,7 +62,16 @@ public Task ListAsync(string? relativePath, CancellationToken ca if (string.IsNullOrEmpty(name) || name.StartsWith('.')) continue; var isDir = Directory.Exists(entry); var childRelative = string.IsNullOrEmpty(dir) || dir == "." ? name : dir + "/" + name; - if (!IsAllowed(childRelative)) continue; + if (isDir) + { + if (!CanListPath(childRelative)) + continue; + } + else if (!IsAllowed(childRelative)) + { + continue; + } + entries.Add(new RepoListEntry(name, isDir)); } return Task.FromResult(new RepoListResult(dir, entries.OrderBy(e => e.Name, StringComparer.OrdinalIgnoreCase).ToList())); @@ -115,16 +124,28 @@ private static string NormalizeRelative(string? relative) private bool TryResolveFullPath(string relativePath, out string fullPath) { fullPath = null!; - if (IsPathTraversal(relativePath)) return false; - var repoRoot = Path.GetFullPath(_workspaceContext.WorkspacePath ?? _options.RepoRoot); - fullPath = Path.GetFullPath(Path.Combine(repoRoot, relativePath)); - return fullPath.StartsWith(repoRoot, StringComparison.OrdinalIgnoreCase); + if (IsPathTraversal(relativePath)) + return false; + + var repoRoot = GetRepoRoot(); + var candidate = Path.GetFullPath(Path.Combine(repoRoot, relativePath)); + if (!IsPathWithinRoot(repoRoot, candidate)) + return false; + + if (ContainsEscapingReparsePoint(repoRoot, candidate, relativePath)) + return false; + + fullPath = candidate; + return true; } private static bool IsPathTraversal(string relativePath) { - return relativePath.Contains("..", StringComparison.Ordinal) || - Path.IsPathRooted(relativePath); + if (Path.IsPathRooted(relativePath)) + return true; + + return SplitPathSegments(relativePath) + .Any(segment => string.Equals(segment, "..", StringComparison.Ordinal)); } private bool IsAllowed(string relativePath) @@ -134,28 +155,107 @@ private bool IsAllowed(string relativePath) return MatchesAllowlist(relativePath, allowlist); } + private bool CanListPath(string relativePath) + { + var allowlist = _options.RepoAllowlist; + if (allowlist == null || allowlist.Count == 0) return true; + return CanListPath(relativePath, allowlist); + } + private static bool MatchesAllowlist(string relativePath, IReadOnlyList patterns) + => PathGlobMatcher.MatchesAny(relativePath, patterns); + + private static bool CanListPath(string relativePath, IReadOnlyList patterns) + => PathGlobMatcher.MayMatchDirectoryPrefix(relativePath, patterns); + + private string GetRepoRoot() + { + return Path.GetFullPath(_workspaceContext.WorkspacePath ?? _options.RepoRoot) + .TrimEnd(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar); + } + + private bool ContainsEscapingReparsePoint(string repoRoot, string candidatePath, string relativePath) { - foreach (var p in patterns) + var current = repoRoot; + foreach (var segment in SplitPathSegments(relativePath)) { - if (p.Contains("**", StringComparison.Ordinal)) + current = Path.Combine(current, segment); + if (!File.Exists(current) && !Directory.Exists(current)) + break; + + try { - var prefix = p.Replace("**", string.Empty, StringComparison.Ordinal).TrimEnd(s_trimSlashChars); - if (relativePath.StartsWith(prefix, StringComparison.OrdinalIgnoreCase)) return true; + var attributes = File.GetAttributes(current); + if (!attributes.HasFlag(FileAttributes.ReparsePoint)) + continue; + + var resolved = ResolveReparsePointTarget(current); + if (resolved is null || !IsPathWithinRoot(repoRoot, resolved.FullName)) + { + _logger.LogWarning( + "Rejected repo path {RelativePath} because reparse point {ReparsePath} resolves outside repo root {RepoRoot}.", + relativePath, + current, + repoRoot); + return true; + } } - else if (p.StartsWith("*.", StringComparison.Ordinal)) + catch (IOException ex) { - var suffix = p[1..]; - if (relativePath.EndsWith(suffix, StringComparison.OrdinalIgnoreCase)) return true; + _logger.LogWarning( + ex, + "Rejected repo path {RelativePath} because reparse-point validation failed at {ReparsePath}.", + relativePath, + current); + return true; } - else if (relativePath.StartsWith(p.TrimStart('/'), StringComparison.OrdinalIgnoreCase)) + catch (UnauthorizedAccessException ex) { + _logger.LogWarning( + ex, + "Rejected repo path {RelativePath} because reparse-point validation could not access {ReparsePath}.", + relativePath, + current); return true; } } + return false; } + private static FileSystemInfo? ResolveReparsePointTarget(string path) + { + if (Directory.Exists(path)) + return new DirectoryInfo(path).ResolveLinkTarget(returnFinalTarget: true); + + if (File.Exists(path)) + return new FileInfo(path).ResolveLinkTarget(returnFinalTarget: true); + + return null; + } + + private static bool IsPathWithinRoot(string rootPath, string candidatePath) + { + var relative = Path.GetRelativePath(rootPath, candidatePath); + if (string.IsNullOrWhiteSpace(relative) || string.Equals(relative, ".", StringComparison.Ordinal)) + return true; + + if (Path.IsPathRooted(relative)) + return false; + + return !string.Equals(relative, "..", StringComparison.Ordinal) + && !relative.StartsWith(".." + Path.DirectorySeparatorChar, StringComparison.Ordinal) + && !relative.StartsWith(".." + Path.AltDirectorySeparatorChar, StringComparison.Ordinal); + } + + private static string[] SplitPathSegments(string path) + { + if (string.IsNullOrWhiteSpace(path) || string.Equals(path, ".", StringComparison.Ordinal)) + return []; + + return path.Split('/', StringSplitOptions.RemoveEmptyEntries | StringSplitOptions.TrimEntries); + } + private async Task PublishChangeSafeAsync(string action, string entityId, CancellationToken cancellationToken) { if (_eventBus is null) diff --git a/src/McpServer.Services/Services/SqliteTodoService.cs b/src/McpServer.Services/Services/SqliteTodoService.cs index df4cd9fa..bbc74fdb 100644 --- a/src/McpServer.Services/Services/SqliteTodoService.cs +++ b/src/McpServer.Services/Services/SqliteTodoService.cs @@ -161,6 +161,49 @@ ORDER BY version ASC return new TodoAuditQueryResult(entries, totalCount); } + /// + public async Task GetProjectionStatusAsync(CancellationToken cancellationToken = default) + { + await EnsureInitializedAsync(cancellationToken).ConfigureAwait(false); + await _writeLock.WaitAsync(cancellationToken).ConfigureAwait(false); + try + { + return await GetProjectionStatusCoreAsync(cancellationToken).ConfigureAwait(false); + } + finally + { + _writeLock.Release(); + } + } + + /// + public async Task RepairProjectionAsync(CancellationToken cancellationToken = default) + { + await EnsureInitializedAsync(cancellationToken).ConfigureAwait(false); + await _writeLock.WaitAsync(cancellationToken).ConfigureAwait(false); + try + { + try + { + await ProjectDatabaseToYamlAsync(cancellationToken).ConfigureAwait(false); + _logger.LogInformation("Repaired TODO.yaml projection from authoritative SQLite storage at {TodoFilePath}.", _todoFilePath); + var status = await GetProjectionStatusCoreAsync(cancellationToken).ConfigureAwait(false); + return new TodoProjectionRepairResult(true, null, status); + } + catch (Exception ex) when (ex is IOException or UnauthorizedAccessException or JsonException or YamlException or InvalidOperationException or SqliteException) + { + _logger.LogError(ex, "Operator-requested TODO projection repair failed for {TodoFilePath}.", _todoFilePath); + await TryRecordProjectionFailureAsync(ex).ConfigureAwait(false); + var status = await GetProjectionStatusCoreAsync(cancellationToken).ConfigureAwait(false); + return new TodoProjectionRepairResult(false, $"Failed to repair TODO projection at '{_todoFilePath}': {ex.Message}", status); + } + } + finally + { + _writeLock.Release(); + } + } + public async Task CreateAsync(TodoCreateRequest request, CancellationToken cancellationToken = default) { ArgumentNullException.ThrowIfNull(request); @@ -439,12 +482,14 @@ singleton_id INTEGER PRIMARY KEY CHECK (singleton_id = 1), completed_json TEXT NULL, code_review_reference TEXT NULL, last_imported_from_yaml_utc TEXT NULL, - last_projected_to_yaml_utc TEXT NULL + last_projected_to_yaml_utc TEXT NULL, + last_projection_failure_utc TEXT NULL, + last_projection_failure_message TEXT NULL ); """; await command.ExecuteNonQueryAsync().ConfigureAwait(false); - var columns = await GetTodoItemColumnsAsync(connection).ConfigureAwait(false); + var columns = await GetTableColumnsAsync(connection, "todo_items").ConfigureAwait(false); if (!columns.Contains("item_kind")) await ExecuteNonQueryAsync(connection, "ALTER TABLE todo_items ADD COLUMN item_kind TEXT NOT NULL DEFAULT 'standard';").ConfigureAwait(false); if (!columns.Contains("section_order")) @@ -453,6 +498,12 @@ last_projected_to_yaml_utc TEXT NULL await ExecuteNonQueryAsync(connection, "ALTER TABLE todo_items ADD COLUMN item_order INTEGER NOT NULL DEFAULT 0;").ConfigureAwait(false); if (!columns.Contains("phase_label")) await ExecuteNonQueryAsync(connection, "ALTER TABLE todo_items ADD COLUMN phase_label TEXT NULL;").ConfigureAwait(false); + + var metadataColumns = await GetTableColumnsAsync(connection, "todo_document_metadata").ConfigureAwait(false); + if (!metadataColumns.Contains("last_projection_failure_utc")) + await ExecuteNonQueryAsync(connection, "ALTER TABLE todo_document_metadata ADD COLUMN last_projection_failure_utc TEXT NULL;").ConfigureAwait(false); + if (!metadataColumns.Contains("last_projection_failure_message")) + await ExecuteNonQueryAsync(connection, "ALTER TABLE todo_document_metadata ADD COLUMN last_projection_failure_message TEXT NULL;").ConfigureAwait(false); } private async Task EnsureMetadataRowAsync(SqliteConnection connection) @@ -469,6 +520,8 @@ private async Task ImportFromYamlAsync(SqliteConnection connection, TodoFile fil file.Completed is null ? null : JsonSerializer.Serialize(file.Completed, _json), file.CodeReviewRemediation?.Reference, importedAtUtc.ToString("O"), + null, + null, null); await UpdateDocumentMetadataAsync(connection, documentMetadata, cancellationToken).ConfigureAwait(false); @@ -649,6 +702,7 @@ private async Task FinalizeMutationAsync(string action, stri } catch (Exception ex) when (ex is IOException or UnauthorizedAccessException or JsonException or YamlException or InvalidOperationException or SqliteException) { + await TryRecordProjectionFailureAsync(ex).ConfigureAwait(false); _logger.LogError(ex, "TODO mutation for {Id} committed in SQLite but projection to {TodoFilePath} failed.", id, _todoFilePath); var message = $"TODO '{id}' was committed to authoritative SQLite storage, but projection to '{_todoFilePath}' failed: {ex.Message}"; return new TodoMutationResult(false, message, item, TodoMutationFailureKind.ProjectionFailed); @@ -658,13 +712,41 @@ private async Task FinalizeMutationAsync(string action, stri } private async Task ProjectDatabaseToYamlAsync(CancellationToken cancellationToken) + { + var file = await BuildProjectedTodoFileAsync(cancellationToken).ConfigureAwait(false); + await TodoYamlFileSerializer.WriteAtomicallyAsync(_todoFilePath, file, cancellationToken).ConfigureAwait(false); + + try + { + using var connection = CreateConnection(); + await connection.OpenAsync(CancellationToken.None).ConfigureAwait(false); + var metadata = await GetDocumentMetadataAsync(connection, CancellationToken.None).ConfigureAwait(false); + var updatedMetadata = metadata with + { + LastProjectedToYamlUtc = DateTime.UtcNow.ToString("O"), + LastProjectionFailureUtc = null, + LastProjectionFailureMessage = null, + }; + await UpdateDocumentMetadataAsync(connection, updatedMetadata, CancellationToken.None).ConfigureAwait(false); + } + catch (Exception ex) when (ex is InvalidOperationException or SqliteException) + { + _logger.LogWarning(ex, "TODO.yaml projection succeeded for {TodoFilePath}, but projection metadata could not be updated.", _todoFilePath); + } + } + + private async Task BuildProjectedTodoFileAsync(CancellationToken cancellationToken) { var items = await GetAllStoredAsync(cancellationToken).ConfigureAwait(false); using var connection = CreateConnection(); await connection.OpenAsync(cancellationToken).ConfigureAwait(false); var metadata = await GetDocumentMetadataAsync(connection, cancellationToken).ConfigureAwait(false); + return BuildProjectedTodoFile(items, metadata); + } + private TodoFile BuildProjectedTodoFile(IReadOnlyList items, TodoDocumentMetadata metadata) + { var file = new TodoFile(); foreach (var sectionGroup in items .Where(static item => item.ItemKind == StandardItemKind) @@ -710,11 +792,97 @@ private async Task ProjectDatabaseToYamlAsync(CancellationToken cancellationToke file.Completed = DeserializeJson>(metadata.CompletedJson); file.Notes = DeserializeJson>(metadata.NotesJson); + return file; + } - await TodoYamlFileSerializer.WriteAtomicallyAsync(_todoFilePath, file, cancellationToken).ConfigureAwait(false); + private async Task GetProjectionStatusCoreAsync(CancellationToken cancellationToken) + { + using var connection = CreateConnection(); + await connection.OpenAsync(cancellationToken).ConfigureAwait(false); + var metadata = await GetDocumentMetadataAsync(connection, cancellationToken).ConfigureAwait(false); + var projectedFile = await BuildProjectedTodoFileAsync(cancellationToken).ConfigureAwait(false); - var updatedMetadata = metadata with { LastProjectedToYamlUtc = DateTime.UtcNow.ToString("O") }; - await UpdateDocumentMetadataAsync(connection, updatedMetadata, cancellationToken).ConfigureAwait(false); + var projectionTargetExists = File.Exists(_todoFilePath); + var projectionConsistent = false; + string? consistencyMessage = null; + + if (!projectionTargetExists) + { + consistencyMessage = Directory.Exists(_todoFilePath) + ? $"Projected TODO target '{_todoFilePath}' is a directory instead of a file." + : $"Projected TODO file '{_todoFilePath}' does not exist."; + } + else + { + try + { + var actualFile = await TodoYamlFileSerializer.ReadIfExistsAsync(_todoFilePath, cancellationToken).ConfigureAwait(false); + if (actualFile is null) + { + consistencyMessage = $"Projected TODO file '{_todoFilePath}' could not be loaded for consistency verification."; + } + else + { + projectionConsistent = string.Equals( + NormalizeYaml(TodoYamlFileSerializer.Serialize(actualFile)), + NormalizeYaml(TodoYamlFileSerializer.Serialize(projectedFile)), + StringComparison.Ordinal); + + if (!projectionConsistent) + consistencyMessage = $"Projected TODO file '{_todoFilePath}' does not match authoritative SQLite state."; + } + } + catch (Exception ex) when (ex is IOException or UnauthorizedAccessException or YamlException) + { + consistencyMessage = $"Projected TODO file '{_todoFilePath}' could not be read for consistency verification: {ex.Message}"; + } + } + + var repairRequired = !projectionTargetExists || !projectionConsistent; + var historicalFailureMessage = string.IsNullOrWhiteSpace(metadata.LastProjectionFailureMessage) + ? null + : $"Last recorded projection failure at {metadata.LastProjectionFailureUtc ?? "an unknown time"}: {metadata.LastProjectionFailureMessage}"; + + var message = consistencyMessage + ?? (repairRequired + ? historicalFailureMessage ?? "TODO.yaml requires repair to match authoritative SQLite state." + : historicalFailureMessage is null + ? "TODO.yaml matches authoritative SQLite state." + : $"TODO.yaml matches authoritative SQLite state. {historicalFailureMessage}"); + + return new TodoProjectionStatusResult( + AuthoritativeStore: "sqlite", + AuthoritativeDataSource: _dataSource, + ProjectionTargetPath: _todoFilePath, + ProjectionTargetExists: projectionTargetExists, + ProjectionConsistent: projectionConsistent, + RepairRequired: repairRequired, + VerifiedAtUtc: DateTime.UtcNow.ToString("O"), + LastImportedFromYamlUtc: metadata.LastImportedFromYamlUtc, + LastProjectedToYamlUtc: metadata.LastProjectedToYamlUtc, + LastProjectionFailureUtc: metadata.LastProjectionFailureUtc, + LastProjectionFailure: metadata.LastProjectionFailureMessage, + Message: message); + } + + private async Task TryRecordProjectionFailureAsync(Exception ex) + { + try + { + using var connection = CreateConnection(); + await connection.OpenAsync(CancellationToken.None).ConfigureAwait(false); + var metadata = await GetDocumentMetadataAsync(connection, CancellationToken.None).ConfigureAwait(false); + var updatedMetadata = metadata with + { + LastProjectionFailureUtc = DateTime.UtcNow.ToString("O"), + LastProjectionFailureMessage = ex.Message, + }; + await UpdateDocumentMetadataAsync(connection, updatedMetadata, CancellationToken.None).ConfigureAwait(false); + } + catch (Exception recordEx) when (recordEx is InvalidOperationException or SqliteException) + { + _logger.LogWarning(recordEx, "Failed to persist TODO projection failure metadata for {TodoFilePath}.", _todoFilePath); + } } private static List? BuildPriorityItems(IGrouping group, string priority) @@ -936,17 +1104,19 @@ INSERT INTO todo_item_history ( private async Task GetDocumentMetadataAsync(SqliteConnection connection, CancellationToken cancellationToken) { using var command = connection.CreateCommand(); - command.CommandText = "SELECT notes_json, completed_json, code_review_reference, last_imported_from_yaml_utc, last_projected_to_yaml_utc FROM todo_document_metadata WHERE singleton_id = 1 LIMIT 1;"; + command.CommandText = "SELECT notes_json, completed_json, code_review_reference, last_imported_from_yaml_utc, last_projected_to_yaml_utc, last_projection_failure_utc, last_projection_failure_message FROM todo_document_metadata WHERE singleton_id = 1 LIMIT 1;"; using var reader = await command.ExecuteReaderAsync(cancellationToken).ConfigureAwait(false); if (!await reader.ReadAsync(cancellationToken).ConfigureAwait(false)) - return new TodoDocumentMetadata(null, null, null, null, null); + return new TodoDocumentMetadata(null, null, null, null, null, null, null); return new TodoDocumentMetadata( GetNullableString(reader, "notes_json"), GetNullableString(reader, "completed_json"), GetNullableString(reader, "code_review_reference"), GetNullableString(reader, "last_imported_from_yaml_utc"), - GetNullableString(reader, "last_projected_to_yaml_utc")); + GetNullableString(reader, "last_projected_to_yaml_utc"), + GetNullableString(reader, "last_projection_failure_utc"), + GetNullableString(reader, "last_projection_failure_message")); } private async Task UpdateDocumentMetadataAsync(SqliteConnection connection, TodoDocumentMetadata metadata, CancellationToken cancellationToken) @@ -958,7 +1128,9 @@ UPDATE todo_document_metadata completed_json = $completedJson, code_review_reference = $codeReviewReference, last_imported_from_yaml_utc = $lastImported, - last_projected_to_yaml_utc = $lastProjected + last_projected_to_yaml_utc = $lastProjected, + last_projection_failure_utc = $lastProjectionFailureUtc, + last_projection_failure_message = $lastProjectionFailureMessage WHERE singleton_id = 1; """; command.Parameters.AddWithValue("$notesJson", (object?)metadata.NotesJson ?? DBNull.Value); @@ -966,6 +1138,8 @@ UPDATE todo_document_metadata command.Parameters.AddWithValue("$codeReviewReference", (object?)metadata.CodeReviewReference ?? DBNull.Value); command.Parameters.AddWithValue("$lastImported", (object?)metadata.LastImportedFromYamlUtc ?? DBNull.Value); command.Parameters.AddWithValue("$lastProjected", (object?)metadata.LastProjectedToYamlUtc ?? DBNull.Value); + command.Parameters.AddWithValue("$lastProjectionFailureUtc", (object?)metadata.LastProjectionFailureUtc ?? DBNull.Value); + command.Parameters.AddWithValue("$lastProjectionFailureMessage", (object?)metadata.LastProjectionFailureMessage ?? DBNull.Value); await command.ExecuteNonQueryAsync(cancellationToken).ConfigureAwait(false); } @@ -1006,10 +1180,10 @@ private async Task GetCurrentItemCountAsync(SqliteConnection connection) return Convert.ToInt32((long)(await command.ExecuteScalarAsync().ConfigureAwait(false) ?? 0L)); } - private async Task> GetTodoItemColumnsAsync(SqliteConnection connection) + private async Task> GetTableColumnsAsync(SqliteConnection connection, string tableName) { using var command = connection.CreateCommand(); - command.CommandText = "PRAGMA table_info(todo_items);"; + command.CommandText = $"PRAGMA table_info({tableName});"; using var reader = await command.ExecuteReaderAsync().ConfigureAwait(false); var columns = new HashSet(StringComparer.OrdinalIgnoreCase); while (await reader.ReadAsync().ConfigureAwait(false)) @@ -1035,6 +1209,9 @@ private async Task ExecuteNonQueryAsync(SqliteConnection connection, string sql) ? default : JsonSerializer.Deserialize(value, _json); + private static string NormalizeYaml(string yaml) + => yaml.Replace("\r\n", "\n", StringComparison.Ordinal).Trim(); + private TodoFlatItem? DeserializeFlatItem(string? value) => DeserializeJson(value); private List? DeserializeList(string? value) => DeserializeJson>(value); @@ -1179,5 +1356,7 @@ private sealed record TodoDocumentMetadata( string? CompletedJson, string? CodeReviewReference, string? LastImportedFromYamlUtc, - string? LastProjectedToYamlUtc); + string? LastProjectedToYamlUtc, + string? LastProjectionFailureUtc, + string? LastProjectionFailureMessage); } diff --git a/src/McpServer.Services/Services/TodoService.cs b/src/McpServer.Services/Services/TodoService.cs index f68d8614..5706f040 100644 --- a/src/McpServer.Services/Services/TodoService.cs +++ b/src/McpServer.Services/Services/TodoService.cs @@ -75,6 +75,18 @@ public Task GetAuditAsync(string id, int limit = 50, int o throw new NotSupportedException("TODO audit history requires sqlite TODO storage."); } + /// + public Task GetProjectionStatusAsync(CancellationToken cancellationToken = default) + { + throw new NotSupportedException("TODO projection status requires sqlite TODO storage."); + } + + /// + public Task RepairProjectionAsync(CancellationToken cancellationToken = default) + { + throw new NotSupportedException("TODO projection repair requires sqlite TODO storage."); + } + /// public async Task CreateAsync(TodoCreateRequest request, CancellationToken cancellationToken = default) { diff --git a/src/McpServer.Services/Services/ToolBucketService.cs b/src/McpServer.Services/Services/ToolBucketService.cs index a05583ad..faa810c0 100644 --- a/src/McpServer.Services/Services/ToolBucketService.cs +++ b/src/McpServer.Services/Services/ToolBucketService.cs @@ -301,7 +301,7 @@ private IQueryable GetVisibleBucketsQuery() var workspaceId = _db.CurrentWorkspaceId; var query = _db.ToolBuckets.IgnoreQueryFilters(); if (string.IsNullOrWhiteSpace(workspaceId)) - return query; + return query.Where(b => b.WorkspaceId == string.Empty); // Default buckets are seeded without a workspace and must remain visible to every workspace. return query.Where(b => b.WorkspaceId == string.Empty || b.WorkspaceId == workspaceId); diff --git a/src/McpServer.Services/Services/WorkspaceTokenService.cs b/src/McpServer.Services/Services/WorkspaceTokenService.cs index d93a753b..ba209cbb 100644 --- a/src/McpServer.Services/Services/WorkspaceTokenService.cs +++ b/src/McpServer.Services/Services/WorkspaceTokenService.cs @@ -11,9 +11,8 @@ namespace McpServer.Support.Mcp.Services; /// AGENTS-README-FIRST.yaml marker file. Grant unrestricted access to all /// /mcpserver/* endpoints. /// Default (anonymous) tokens — returned by the -/// unprotected GET /api-key endpoint. Grant read-only access to all -/// endpoints except TODO routes (/mcpserver/todo*), which are -/// read-write. +/// unprotected GET /api-key endpoint. Grant read-only access only. +/// All write operations require a full workspace token or JWT Bearer auth. /// /// Tokens are held in memory only (never persisted) and rotate on every service restart. /// @@ -68,7 +67,7 @@ public bool ValidateToken(string workspacePath, string? candidate) /// /// Generates a new default (anonymous) token for the given workspace and stores it. /// If a default token already exists for the workspace it is replaced. - /// Default tokens grant read-only access to all endpoints except TODO routes which are read-write. + /// Default tokens grant read-only access only. /// /// The generated base64url token. public string GenerateDefaultToken(string workspacePath) diff --git a/src/McpServer.Storage/McpDbContext.cs b/src/McpServer.Storage/McpDbContext.cs index 4bff4845..f335befc 100644 --- a/src/McpServer.Storage/McpDbContext.cs +++ b/src/McpServer.Storage/McpDbContext.cs @@ -210,22 +210,22 @@ protected override void OnModelCreating(ModelBuilder modelBuilder) e.HasIndex(x => x.EventType); }); - modelBuilder.Entity().HasQueryFilter(e => _workspaceId == "" || e.WorkspaceId == _workspaceId); - modelBuilder.Entity().HasQueryFilter(e => _workspaceId == "" || e.WorkspaceId == _workspaceId); - modelBuilder.Entity().HasQueryFilter(e => _workspaceId == "" || e.WorkspaceId == _workspaceId); - modelBuilder.Entity().HasQueryFilter(e => _workspaceId == "" || e.WorkspaceId == _workspaceId); - modelBuilder.Entity().HasQueryFilter(e => _workspaceId == "" || e.WorkspaceId == _workspaceId); - modelBuilder.Entity().HasQueryFilter(e => _workspaceId == "" || e.WorkspaceId == _workspaceId); - modelBuilder.Entity().HasQueryFilter(e => _workspaceId == "" || e.WorkspaceId == _workspaceId); - modelBuilder.Entity().HasQueryFilter(e => _workspaceId == "" || e.WorkspaceId == _workspaceId); - modelBuilder.Entity().HasQueryFilter(e => _workspaceId == "" || e.WorkspaceId == _workspaceId); - modelBuilder.Entity().HasQueryFilter(e => _workspaceId == "" || e.WorkspaceId == _workspaceId); - modelBuilder.Entity().HasQueryFilter(e => _workspaceId == "" || e.WorkspaceId == string.Empty || e.WorkspaceId == _workspaceId); - modelBuilder.Entity().HasQueryFilter(e => _workspaceId == "" || e.WorkspaceId == string.Empty || e.WorkspaceId == _workspaceId); - modelBuilder.Entity().HasQueryFilter(e => _workspaceId == "" || e.WorkspaceId == _workspaceId); - modelBuilder.Entity().HasQueryFilter(e => _workspaceId == "" || e.WorkspaceId == _workspaceId); - modelBuilder.Entity().HasQueryFilter(e => _workspaceId == "" || e.WorkspaceId == _workspaceId); - modelBuilder.Entity().HasQueryFilter(e => _workspaceId == "" || e.WorkspaceId == _workspaceId); + modelBuilder.Entity().HasQueryFilter(e => !string.IsNullOrEmpty(_workspaceId) && e.WorkspaceId == _workspaceId); + modelBuilder.Entity().HasQueryFilter(e => !string.IsNullOrEmpty(_workspaceId) && e.WorkspaceId == _workspaceId); + modelBuilder.Entity().HasQueryFilter(e => !string.IsNullOrEmpty(_workspaceId) && e.WorkspaceId == _workspaceId); + modelBuilder.Entity().HasQueryFilter(e => !string.IsNullOrEmpty(_workspaceId) && e.WorkspaceId == _workspaceId); + modelBuilder.Entity().HasQueryFilter(e => !string.IsNullOrEmpty(_workspaceId) && e.WorkspaceId == _workspaceId); + modelBuilder.Entity().HasQueryFilter(e => !string.IsNullOrEmpty(_workspaceId) && e.WorkspaceId == _workspaceId); + modelBuilder.Entity().HasQueryFilter(e => !string.IsNullOrEmpty(_workspaceId) && e.WorkspaceId == _workspaceId); + modelBuilder.Entity().HasQueryFilter(e => !string.IsNullOrEmpty(_workspaceId) && e.WorkspaceId == _workspaceId); + modelBuilder.Entity().HasQueryFilter(e => !string.IsNullOrEmpty(_workspaceId) && e.WorkspaceId == _workspaceId); + modelBuilder.Entity().HasQueryFilter(e => !string.IsNullOrEmpty(_workspaceId) && e.WorkspaceId == _workspaceId); + modelBuilder.Entity().HasQueryFilter(e => e.WorkspaceId == string.Empty || (!string.IsNullOrEmpty(_workspaceId) && e.WorkspaceId == _workspaceId)); + modelBuilder.Entity().HasQueryFilter(e => e.WorkspaceId == string.Empty || (!string.IsNullOrEmpty(_workspaceId) && e.WorkspaceId == _workspaceId)); + modelBuilder.Entity().HasQueryFilter(e => e.WorkspaceId == string.Empty || (!string.IsNullOrEmpty(_workspaceId) && e.WorkspaceId == _workspaceId)); + modelBuilder.Entity().HasQueryFilter(e => e.WorkspaceId == string.Empty || (!string.IsNullOrEmpty(_workspaceId) && e.WorkspaceId == _workspaceId)); + modelBuilder.Entity().HasQueryFilter(e => !string.IsNullOrEmpty(_workspaceId) && e.WorkspaceId == _workspaceId); + modelBuilder.Entity().HasQueryFilter(e => !string.IsNullOrEmpty(_workspaceId) && e.WorkspaceId == _workspaceId); modelBuilder.Entity().HasIndex(e => e.WorkspaceId); modelBuilder.Entity().HasIndex(e => e.WorkspaceId); diff --git a/src/McpServer.Support.Mcp/Controllers/DesktopController.cs b/src/McpServer.Support.Mcp/Controllers/DesktopController.cs index 696ff9d1..cf74ea6a 100644 --- a/src/McpServer.Support.Mcp/Controllers/DesktopController.cs +++ b/src/McpServer.Support.Mcp/Controllers/DesktopController.cs @@ -1,6 +1,12 @@ +using System.Security.Cryptography; +using System.Text; using McpServer.Support.Mcp.Models; +using McpServer.Support.Mcp.Options; using McpServer.Support.Mcp.Services; +using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Options; namespace McpServer.Support.Mcp.Controllers; @@ -13,6 +19,8 @@ namespace McpServer.Support.Mcp.Controllers; public sealed class DesktopController : ControllerBase { private readonly DesktopLaunchService _desktopLaunchService; + private readonly ILogger _logger; + private readonly IOptions _desktopLaunchOptions; private readonly WorkspaceContext _workspaceContext; /// @@ -20,12 +28,18 @@ public sealed class DesktopController : ControllerBase /// desktop-launch service and resolved workspace context. /// /// Service that invokes the launcher executable. + /// Privileged desktop-launch configuration. + /// Logger used for denied desktop-launch requests. /// Resolved workspace context for the current request. public DesktopController( DesktopLaunchService desktopLaunchService, + IOptions desktopLaunchOptions, + ILogger logger, WorkspaceContext workspaceContext) { _desktopLaunchService = desktopLaunchService ?? throw new ArgumentNullException(nameof(desktopLaunchService)); + _desktopLaunchOptions = desktopLaunchOptions ?? throw new ArgumentNullException(nameof(desktopLaunchOptions)); + _logger = logger ?? throw new ArgumentNullException(nameof(logger)); _workspaceContext = workspaceContext ?? throw new ArgumentNullException(nameof(workspaceContext)); } @@ -45,10 +59,58 @@ public async Task> LaunchAsync( return BadRequest(new { error = "Request body is required." }); if (string.IsNullOrWhiteSpace(_workspaceContext.WorkspacePath)) return NotFound(new { error = "Workspace could not be resolved." }); + if (!HasAuthorizedDesktopLaunchToken()) + { + return StatusCode( + StatusCodes.Status403Forbidden, + new { error = $"Desktop launch requires the configured {DesktopLaunchOptions.AccessTokenHeaderName} header." }); + } var result = await _desktopLaunchService .LaunchAsync(_workspaceContext.WorkspacePath, request, cancellationToken) .ConfigureAwait(false); return Ok(result); } + + private bool HasAuthorizedDesktopLaunchToken() + { + var configuredToken = _desktopLaunchOptions.Value.AccessToken; + if (string.IsNullOrWhiteSpace(configuredToken)) + { + _logger.LogWarning( + "Rejected desktop launch for workspace {WorkspacePath} because no desktop-launch access token is configured.", + _workspaceContext.WorkspacePath); + return false; + } + + if (!Request.Headers.TryGetValue(DesktopLaunchOptions.AccessTokenHeaderName, out var providedValues)) + { + _logger.LogWarning( + "Rejected desktop launch for workspace {WorkspacePath} because the desktop-launch access token header was missing.", + _workspaceContext.WorkspacePath); + return false; + } + + var providedToken = providedValues.ToString(); + if (string.IsNullOrWhiteSpace(providedToken)) + { + _logger.LogWarning( + "Rejected desktop launch for workspace {WorkspacePath} because the desktop-launch access token header was empty.", + _workspaceContext.WorkspacePath); + return false; + } + + var configuredBytes = Encoding.UTF8.GetBytes(configuredToken); + var providedBytes = Encoding.UTF8.GetBytes(providedToken); + var matches = configuredBytes.Length == providedBytes.Length + && CryptographicOperations.FixedTimeEquals(configuredBytes, providedBytes); + if (!matches) + { + _logger.LogWarning( + "Rejected desktop launch for workspace {WorkspacePath} because the desktop-launch access token was invalid.", + _workspaceContext.WorkspacePath); + } + + return matches; + } } diff --git a/src/McpServer.Support.Mcp/Controllers/GitHubController.cs b/src/McpServer.Support.Mcp/Controllers/GitHubController.cs index 8bdffa45..366d3512 100644 --- a/src/McpServer.Support.Mcp/Controllers/GitHubController.cs +++ b/src/McpServer.Support.Mcp/Controllers/GitHubController.cs @@ -1,46 +1,57 @@ -using McpServer.Support.Mcp.Models; -using McpServer.Support.Mcp.Notifications; -using McpServer.Support.Mcp.Options; -using McpServer.Support.Mcp.Services; -using Microsoft.AspNetCore.WebUtilities; -using Microsoft.AspNetCore.Mvc; -using Microsoft.Extensions.DependencyInjection; -using Microsoft.Extensions.Logging; -using Microsoft.Extensions.Options; - -namespace McpServer.Support.Mcp.Controllers; - -/// -/// TR-PLANNED-013, TR-GH-013-006: GitHub metadata via gh CLI (issues and PRs). +using McpServer.Support.Mcp.Models; +using McpServer.Support.Mcp.Notifications; +using McpServer.Support.Mcp.Options; +using McpServer.Support.Mcp.Services; +using Microsoft.AspNetCore.WebUtilities; +using Microsoft.AspNetCore.Mvc; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Options; + +namespace McpServer.Support.Mcp.Controllers; + +/// +/// TR-PLANNED-013, TR-GH-013-006: GitHub metadata via gh CLI (issues and PRs). /// FR-SUPPORT-010, FR-SUPPORT-013: List, create, comment, update, close, reopen, sync endpoints. /// [ApiController] [Route("mcpserver/gh")] public sealed class GitHubController : ControllerBase { + private static readonly HashSet AllowedIssueStates = new(StringComparer.OrdinalIgnoreCase) + { + "open", + "closed", + "all" + }; + private static readonly HashSet AllowedCloseReasons = new(StringComparer.OrdinalIgnoreCase) + { + "completed", + "not_planned" + }; private readonly IGitHubCliService _gh; - private readonly IIssueTodoSyncService? _syncService; - private readonly IChangeEventBus? _eventBus; - private readonly ILogger _logger; - private readonly IGitHubWorkspaceTokenStore _tokenStore; - private readonly IOptionsMonitor _gitHubOptions; - - /// TR-PLANNED-013: Constructor. - public GitHubController( - IGitHubCliService gh, - IGitHubWorkspaceTokenStore tokenStore, - IOptionsMonitor gitHubOptions, - IIssueTodoSyncService? syncService = null, - IChangeEventBus? eventBus = null, - ILogger? logger = null) - { - _gh = gh; - _tokenStore = tokenStore; - _gitHubOptions = gitHubOptions; - _syncService = syncService; - _eventBus = eventBus; - _logger = logger ?? Microsoft.Extensions.Logging.Abstractions.NullLogger.Instance; - } + private readonly IIssueTodoSyncService? _syncService; + private readonly IChangeEventBus? _eventBus; + private readonly ILogger _logger; + private readonly IGitHubWorkspaceTokenStore _tokenStore; + private readonly IOptionsMonitor _gitHubOptions; + + /// TR-PLANNED-013: Constructor. + public GitHubController( + IGitHubCliService gh, + IGitHubWorkspaceTokenStore tokenStore, + IOptionsMonitor gitHubOptions, + IIssueTodoSyncService? syncService = null, + IChangeEventBus? eventBus = null, + ILogger? logger = null) + { + _gh = gh; + _tokenStore = tokenStore; + _gitHubOptions = gitHubOptions; + _syncService = syncService; + _eventBus = eventBus; + _logger = logger ?? Microsoft.Extensions.Logging.Abstractions.NullLogger.Instance; + } /// TR-PLANNED-013: List issues (gh.issues.list). /// Optional filter: open, closed, all. @@ -48,9 +59,13 @@ public GitHubController( /// Cancellation token. [HttpGet("issues")] [ProducesResponseType(typeof(object), StatusCodes.Status200OK)] + [ProducesResponseType(StatusCodes.Status400BadRequest)] public async Task> ListIssuesAsync([FromQuery] string? state, [FromQuery] int limit = 30, CancellationToken cancellationToken = default) { - var result = await _gh.ListIssuesAsync(state, limit, cancellationToken).ConfigureAwait(false); + if (!TryNormalizeIssueState(state, out var normalizedState, out var errorMessage)) + return BadRequest(new { error = errorMessage }); + + var result = await _gh.ListIssuesAsync(normalizedState, limit, cancellationToken).ConfigureAwait(false); if (!result.Success) return Ok(new { issues = Array.Empty(), error = result.Error }); return Ok(new { issues = result.Issues.Select(i => new { i.Number, i.Title, i.Url, i.State }).ToList() }); @@ -116,7 +131,10 @@ public async Task> UpdateIssueAsync([FromRoute] int number, [ProducesResponseType(StatusCodes.Status400BadRequest)] public async Task> CloseIssueAsync([FromRoute] int number, [FromQuery] string? reason = null, CancellationToken cancellationToken = default) { - var result = await _gh.CloseIssueAsync(number, reason, cancellationToken).ConfigureAwait(false); + if (!TryNormalizeCloseReason(reason, out var normalizedReason, out var errorMessage)) + return BadRequest(new { error = errorMessage }); + + var result = await _gh.CloseIssueAsync(number, normalizedReason, cancellationToken).ConfigureAwait(false); if (!result.Success) return BadRequest(new { error = result.ErrorMessage ?? "failed to close issue" }); await PublishGitHubChangeSafeAsync(ChangeEventActions.Updated, number.ToString(), cancellationToken).ConfigureAwait(false); @@ -158,199 +176,203 @@ public async Task> CommentOnIssueAsync([FromRoute] string i /// TR-GH-013-006: List available repository labels. /// Cancellation token. - [HttpGet("labels")] - [ProducesResponseType(typeof(object), StatusCodes.Status200OK)] - public async Task> ListLabelsAsync(CancellationToken cancellationToken = default) - { - var result = await _gh.ListIssueLabelsAsync(cancellationToken).ConfigureAwait(false); + [HttpGet("labels")] + [ProducesResponseType(typeof(object), StatusCodes.Status200OK)] + public async Task> ListLabelsAsync(CancellationToken cancellationToken = default) + { + var result = await _gh.ListIssueLabelsAsync(cancellationToken).ConfigureAwait(false); if (!result.Success) return Ok(new { labels = Array.Empty(), error = result.ErrorMessage }); - return Ok(new { labels = result.Labels }); - } - - /// TR-MCP-GH-002: Get GitHub auth status for the resolved workspace. - [HttpGet("auth/status")] - [ProducesResponseType(typeof(GitHubAuthStatusResponse), StatusCodes.Status200OK)] - [ProducesResponseType(StatusCodes.Status400BadRequest)] - public async Task> GetAuthStatusAsync(CancellationToken cancellationToken = default) - { - var workspacePath = ResolveWorkspacePath(); - if (workspacePath is null) - return BadRequest(new { error = "Workspace context is required for GitHub auth status." }); - - var storedToken = await _tokenStore.GetAsync(workspacePath, cancellationToken).ConfigureAwait(false); - var options = _gitHubOptions.CurrentValue; - var oauthConfigured = IsOAuthConfigured(options.OAuth); - var mode = storedToken is not null - ? "stored_token" - : options.AllowCliFallback ? "cli_fallback" : "none"; - - return Ok(new GitHubAuthStatusResponse - { - WorkspacePath = workspacePath, - AuthMode = mode, - HasStoredToken = storedToken is not null, - TokenUpdatedAtUtc = storedToken?.UpdatedAtUtc, - TokenExpiresAtUtc = storedToken?.ExpiresAtUtc, - CliFallbackAllowed = options.AllowCliFallback, - OAuthConfigured = oauthConfigured, - }); - } - - /// TR-MCP-GH-002: Set or replace the GitHub token for the resolved workspace. - [HttpPut("auth/token")] - [ProducesResponseType(typeof(object), StatusCodes.Status200OK)] - [ProducesResponseType(StatusCodes.Status400BadRequest)] - public async Task> SetAuthTokenAsync([FromBody] GitHubAuthTokenUpsertRequest? request, CancellationToken cancellationToken = default) - { - if (request is null || string.IsNullOrWhiteSpace(request.AccessToken)) - return BadRequest(new { error = "accessToken is required." }); - - var workspacePath = ResolveWorkspacePath(); - if (workspacePath is null) - return BadRequest(new { error = "Workspace context is required for GitHub auth updates." }); - - await _tokenStore.UpsertAsync(workspacePath, request.AccessToken, request.ExpiresAtUtc, cancellationToken).ConfigureAwait(false); - await PublishGitHubChangeSafeAsync(ChangeEventActions.Updated, "auth-token", cancellationToken).ConfigureAwait(false); - return Ok(new { success = true }); - } - - /// TR-MCP-GH-002: Remove the stored GitHub token for the resolved workspace. - [HttpDelete("auth/token")] - [ProducesResponseType(typeof(object), StatusCodes.Status200OK)] - [ProducesResponseType(StatusCodes.Status400BadRequest)] - public async Task> DeleteAuthTokenAsync(CancellationToken cancellationToken = default) - { - var workspacePath = ResolveWorkspacePath(); - if (workspacePath is null) - return BadRequest(new { error = "Workspace context is required for GitHub auth updates." }); - - var removed = await _tokenStore.DeleteAsync(workspacePath, cancellationToken).ConfigureAwait(false); - if (removed) - await PublishGitHubChangeSafeAsync(ChangeEventActions.Updated, "auth-token", cancellationToken).ConfigureAwait(false); - return Ok(new { success = true, removed }); - } - - /// TR-MCP-GH-001: Returns configured GitHub OAuth app settings for client bootstrap. - [HttpGet("oauth/config")] - [ProducesResponseType(typeof(GitHubOAuthConfigResponse), StatusCodes.Status200OK)] - public ActionResult GetOAuthConfig() - { - var oauth = _gitHubOptions.CurrentValue.OAuth; - return Ok(new GitHubOAuthConfigResponse - { - ClientId = oauth.ClientId, - RedirectUri = oauth.RedirectUri, - Scopes = oauth.Scopes, - AuthorizeEndpoint = oauth.AuthorizeEndpoint, - IsConfigured = IsOAuthConfigured(oauth), - }); - } - - /// TR-MCP-GH-001: Builds a GitHub authorize URL from configured OAuth app values. - [HttpGet("oauth/authorize-url")] - [ProducesResponseType(typeof(object), StatusCodes.Status200OK)] - [ProducesResponseType(StatusCodes.Status400BadRequest)] - public ActionResult GetAuthorizeUrl([FromQuery] string? state = null) - { - var oauth = _gitHubOptions.CurrentValue.OAuth; - if (!IsOAuthConfigured(oauth)) - return BadRequest(new { error = "GitHub OAuth is not fully configured. Set Mcp:GitHub:OAuth:ClientId and RedirectUri." }); - - var query = new Dictionary - { - ["client_id"] = oauth.ClientId, - ["redirect_uri"] = oauth.RedirectUri, - ["scope"] = oauth.Scopes, - ["state"] = state, - }; - var authorizeUrl = QueryHelpers.AddQueryString(oauth.AuthorizeEndpoint, query); - return Ok(new { authorizeUrl }); - } - - /// TR-PLANNED-013: List PRs (gh.prs.list). - /// Optional filter: open, closed, all. - /// Max PRs to return (1–100). - /// Cancellation token. + return Ok(new { labels = result.Labels }); + } + + /// TR-MCP-GH-002: Get GitHub auth status for the resolved workspace. + [HttpGet("auth/status")] + [ProducesResponseType(typeof(GitHubAuthStatusResponse), StatusCodes.Status200OK)] + [ProducesResponseType(StatusCodes.Status400BadRequest)] + public async Task> GetAuthStatusAsync(CancellationToken cancellationToken = default) + { + var workspacePath = ResolveWorkspacePath(); + if (workspacePath is null) + return BadRequest(new { error = "Workspace context is required for GitHub auth status." }); + + var storedToken = await _tokenStore.GetAsync(workspacePath, cancellationToken).ConfigureAwait(false); + var options = _gitHubOptions.CurrentValue; + var oauthConfigured = IsOAuthConfigured(options.OAuth); + var mode = storedToken is not null + ? "stored_token" + : options.AllowCliFallback ? "cli_fallback" : "none"; + + return Ok(new GitHubAuthStatusResponse + { + WorkspacePath = workspacePath, + AuthMode = mode, + HasStoredToken = storedToken is not null, + TokenUpdatedAtUtc = storedToken?.UpdatedAtUtc, + TokenExpiresAtUtc = storedToken?.ExpiresAtUtc, + CliFallbackAllowed = options.AllowCliFallback, + OAuthConfigured = oauthConfigured, + }); + } + + /// TR-MCP-GH-002: Set or replace the GitHub token for the resolved workspace. + [HttpPut("auth/token")] + [ProducesResponseType(typeof(object), StatusCodes.Status200OK)] + [ProducesResponseType(StatusCodes.Status400BadRequest)] + public async Task> SetAuthTokenAsync([FromBody] GitHubAuthTokenUpsertRequest? request, CancellationToken cancellationToken = default) + { + if (request is null || string.IsNullOrWhiteSpace(request.AccessToken)) + return BadRequest(new { error = "accessToken is required." }); + + var workspacePath = ResolveWorkspacePath(); + if (workspacePath is null) + return BadRequest(new { error = "Workspace context is required for GitHub auth updates." }); + + await _tokenStore.UpsertAsync(workspacePath, request.AccessToken, request.ExpiresAtUtc, cancellationToken).ConfigureAwait(false); + await PublishGitHubChangeSafeAsync(ChangeEventActions.Updated, "auth-token", cancellationToken).ConfigureAwait(false); + return Ok(new { success = true }); + } + + /// TR-MCP-GH-002: Remove the stored GitHub token for the resolved workspace. + [HttpDelete("auth/token")] + [ProducesResponseType(typeof(object), StatusCodes.Status200OK)] + [ProducesResponseType(StatusCodes.Status400BadRequest)] + public async Task> DeleteAuthTokenAsync(CancellationToken cancellationToken = default) + { + var workspacePath = ResolveWorkspacePath(); + if (workspacePath is null) + return BadRequest(new { error = "Workspace context is required for GitHub auth updates." }); + + var removed = await _tokenStore.DeleteAsync(workspacePath, cancellationToken).ConfigureAwait(false); + if (removed) + await PublishGitHubChangeSafeAsync(ChangeEventActions.Updated, "auth-token", cancellationToken).ConfigureAwait(false); + return Ok(new { success = true, removed }); + } + + /// TR-MCP-GH-001: Returns configured GitHub OAuth app settings for client bootstrap. + [HttpGet("oauth/config")] + [ProducesResponseType(typeof(GitHubOAuthConfigResponse), StatusCodes.Status200OK)] + public ActionResult GetOAuthConfig() + { + var oauth = _gitHubOptions.CurrentValue.OAuth; + return Ok(new GitHubOAuthConfigResponse + { + ClientId = oauth.ClientId, + RedirectUri = oauth.RedirectUri, + Scopes = oauth.Scopes, + AuthorizeEndpoint = oauth.AuthorizeEndpoint, + IsConfigured = IsOAuthConfigured(oauth), + }); + } + + /// TR-MCP-GH-001: Builds a GitHub authorize URL from configured OAuth app values. + [HttpGet("oauth/authorize-url")] + [ProducesResponseType(typeof(object), StatusCodes.Status200OK)] + [ProducesResponseType(StatusCodes.Status400BadRequest)] + public ActionResult GetAuthorizeUrl([FromQuery] string? state = null) + { + var oauth = _gitHubOptions.CurrentValue.OAuth; + if (!IsOAuthConfigured(oauth)) + return BadRequest(new { error = "GitHub OAuth is not fully configured. Set Mcp:GitHub:OAuth:ClientId and RedirectUri." }); + + var query = new Dictionary + { + ["client_id"] = oauth.ClientId, + ["redirect_uri"] = oauth.RedirectUri, + ["scope"] = oauth.Scopes, + ["state"] = state, + }; + var authorizeUrl = QueryHelpers.AddQueryString(oauth.AuthorizeEndpoint, query); + return Ok(new { authorizeUrl }); + } + + /// TR-PLANNED-013: List PRs (gh.prs.list). + /// Optional filter: open, closed, all. + /// Max PRs to return (1–100). + /// Cancellation token. [HttpGet("pulls")] [ProducesResponseType(typeof(object), StatusCodes.Status200OK)] + [ProducesResponseType(StatusCodes.Status400BadRequest)] public async Task> ListPullsAsync([FromQuery] string? state, [FromQuery] int limit = 30, CancellationToken cancellationToken = default) { - var result = await _gh.ListPullsAsync(state, limit, cancellationToken).ConfigureAwait(false); - if (!result.Success) - return Ok(new { pulls = Array.Empty(), error = result.Error }); - return Ok(new { pulls = result.Pulls.Select(p => new { p.Number, p.Title, p.Url, p.State }).ToList() }); - } - - /// TR-MCP-GH-004: List workflow runs. - [HttpGet("actions/runs")] - [ProducesResponseType(typeof(object), StatusCodes.Status200OK)] - public async Task> ListWorkflowRunsAsync( - [FromQuery] string? branch = null, - [FromQuery] string? status = null, - [FromQuery(Name = "event")] string? eventName = null, - [FromQuery] string? workflow = null, - [FromQuery] int limit = 30, - CancellationToken cancellationToken = default) - { - var query = new GitHubWorkflowRunQuery - { - Branch = branch, - Status = status, - Event = eventName, - Workflow = workflow, - Limit = limit, - }; - - var result = await _gh.ListWorkflowRunsAsync(query, cancellationToken).ConfigureAwait(false); - if (!result.Success) - return Ok(new { runs = Array.Empty(), error = result.ErrorMessage ?? "failed to list workflow runs" }); - return Ok(new { runs = result.Runs, error = (string?)null }); - } - - /// TR-MCP-GH-004: Get workflow run details. - [HttpGet("actions/runs/{runId:long}")] - [ProducesResponseType(typeof(GitHubWorkflowRunDetail), StatusCodes.Status200OK)] - [ProducesResponseType(StatusCodes.Status404NotFound)] - [ProducesResponseType(StatusCodes.Status400BadRequest)] - public async Task> GetWorkflowRunAsync([FromRoute] long runId, CancellationToken cancellationToken = default) - { - var result = await _gh.GetWorkflowRunAsync(runId, cancellationToken).ConfigureAwait(false); - if (!result.Success || result.Run is null) - { - if (result.ErrorMessage?.Contains("not found", StringComparison.OrdinalIgnoreCase) == true) - return NotFound(new { error = result.ErrorMessage }); - return BadRequest(new { error = result.ErrorMessage ?? "failed to fetch workflow run" }); - } - - return Ok(result.Run); - } - - /// TR-MCP-GH-004: Re-run a workflow run. - [HttpPost("actions/runs/{runId:long}/rerun")] - [ProducesResponseType(typeof(object), StatusCodes.Status200OK)] - [ProducesResponseType(StatusCodes.Status400BadRequest)] - public async Task> RerunWorkflowRunAsync([FromRoute] long runId, CancellationToken cancellationToken = default) - { - var result = await _gh.RerunWorkflowRunAsync(runId, cancellationToken).ConfigureAwait(false); - if (!result.Success) - return BadRequest(new { error = result.ErrorMessage ?? "failed to rerun workflow" }); - await PublishGitHubChangeSafeAsync(ChangeEventActions.Updated, $"workflow-run-{runId}", cancellationToken).ConfigureAwait(false); - return Ok(new { success = true }); - } - - /// TR-MCP-GH-004: Cancel an in-progress workflow run. - [HttpPost("actions/runs/{runId:long}/cancel")] - [ProducesResponseType(typeof(object), StatusCodes.Status200OK)] - [ProducesResponseType(StatusCodes.Status400BadRequest)] - public async Task> CancelWorkflowRunAsync([FromRoute] long runId, CancellationToken cancellationToken = default) - { - var result = await _gh.CancelWorkflowRunAsync(runId, cancellationToken).ConfigureAwait(false); - if (!result.Success) - return BadRequest(new { error = result.ErrorMessage ?? "failed to cancel workflow" }); - await PublishGitHubChangeSafeAsync(ChangeEventActions.Updated, $"workflow-run-{runId}", cancellationToken).ConfigureAwait(false); - return Ok(new { success = true }); - } + if (!TryNormalizeIssueState(state, out var normalizedState, out var errorMessage)) + return BadRequest(new { error = errorMessage }); + + var result = await _gh.ListPullsAsync(normalizedState, limit, cancellationToken).ConfigureAwait(false); + if (!result.Success) + return Ok(new { pulls = Array.Empty(), error = result.Error }); + return Ok(new { pulls = result.Pulls.Select(p => new { p.Number, p.Title, p.Url, p.State }).ToList() }); + } + + /// TR-MCP-GH-004: List workflow runs. + [HttpGet("actions/runs")] + [ProducesResponseType(typeof(object), StatusCodes.Status200OK)] + public async Task> ListWorkflowRunsAsync( + [FromQuery] string? branch = null, + [FromQuery] string? status = null, + [FromQuery(Name = "event")] string? eventName = null, + [FromQuery] string? workflow = null, + [FromQuery] int limit = 30, + CancellationToken cancellationToken = default) + { + var query = new GitHubWorkflowRunQuery + { + Branch = branch, + Status = status, + Event = eventName, + Workflow = workflow, + Limit = limit, + }; + + var result = await _gh.ListWorkflowRunsAsync(query, cancellationToken).ConfigureAwait(false); + if (!result.Success) + return Ok(new { runs = Array.Empty(), error = result.ErrorMessage ?? "failed to list workflow runs" }); + return Ok(new { runs = result.Runs, error = (string?)null }); + } + + /// TR-MCP-GH-004: Get workflow run details. + [HttpGet("actions/runs/{runId:long}")] + [ProducesResponseType(typeof(GitHubWorkflowRunDetail), StatusCodes.Status200OK)] + [ProducesResponseType(StatusCodes.Status404NotFound)] + [ProducesResponseType(StatusCodes.Status400BadRequest)] + public async Task> GetWorkflowRunAsync([FromRoute] long runId, CancellationToken cancellationToken = default) + { + var result = await _gh.GetWorkflowRunAsync(runId, cancellationToken).ConfigureAwait(false); + if (!result.Success || result.Run is null) + { + if (result.ErrorMessage?.Contains("not found", StringComparison.OrdinalIgnoreCase) == true) + return NotFound(new { error = result.ErrorMessage }); + return BadRequest(new { error = result.ErrorMessage ?? "failed to fetch workflow run" }); + } + + return Ok(result.Run); + } + + /// TR-MCP-GH-004: Re-run a workflow run. + [HttpPost("actions/runs/{runId:long}/rerun")] + [ProducesResponseType(typeof(object), StatusCodes.Status200OK)] + [ProducesResponseType(StatusCodes.Status400BadRequest)] + public async Task> RerunWorkflowRunAsync([FromRoute] long runId, CancellationToken cancellationToken = default) + { + var result = await _gh.RerunWorkflowRunAsync(runId, cancellationToken).ConfigureAwait(false); + if (!result.Success) + return BadRequest(new { error = result.ErrorMessage ?? "failed to rerun workflow" }); + await PublishGitHubChangeSafeAsync(ChangeEventActions.Updated, $"workflow-run-{runId}", cancellationToken).ConfigureAwait(false); + return Ok(new { success = true }); + } + + /// TR-MCP-GH-004: Cancel an in-progress workflow run. + [HttpPost("actions/runs/{runId:long}/cancel")] + [ProducesResponseType(typeof(object), StatusCodes.Status200OK)] + [ProducesResponseType(StatusCodes.Status400BadRequest)] + public async Task> CancelWorkflowRunAsync([FromRoute] long runId, CancellationToken cancellationToken = default) + { + var result = await _gh.CancelWorkflowRunAsync(runId, cancellationToken).ConfigureAwait(false); + if (!result.Success) + return BadRequest(new { error = result.ErrorMessage ?? "failed to cancel workflow" }); + await PublishGitHubChangeSafeAsync(ChangeEventActions.Updated, $"workflow-run-{runId}", cancellationToken).ConfigureAwait(false); + return Ok(new { success = true }); + } /// TR-PLANNED-013: Comment on PR (gh.prs.comment). /// PR number. @@ -381,7 +403,11 @@ public async Task> SyncFromGitHubAsync([FromQuery] string? { if (_syncService is null) return BadRequest(new { error = "Issue sync service not configured" }); - var result = await _syncService.SyncAllIssuesToTodosAsync(state, limit, cancellationToken).ConfigureAwait(false); + + if (!TryNormalizeIssueState(state, out var normalizedState, out var errorMessage)) + return BadRequest(new { error = errorMessage }); + + var result = await _syncService.SyncAllIssuesToTodosAsync(normalizedState, limit, cancellationToken).ConfigureAwait(false); await PublishGitHubChangeSafeAsync(ChangeEventActions.Updated, "sync-from-github", cancellationToken).ConfigureAwait(false); return Ok(result); } @@ -453,22 +479,64 @@ await _eventBus.PublishAsync( } catch (Exception ex) { - _logger.LogWarning(ex, "Failed publishing GitHub change event for {EntityId}", entityId); - } - } - - private string? ResolveWorkspacePath() - { - return HttpContext.RequestServices.GetService()?.WorkspacePath; - } - - private static bool IsOAuthConfigured(GitHubOAuthOptions oauth) - { - return !string.IsNullOrWhiteSpace(oauth.ClientId) - && !string.IsNullOrWhiteSpace(oauth.RedirectUri) - && !string.IsNullOrWhiteSpace(oauth.AuthorizeEndpoint); - } -} + _logger.LogWarning(ex, "Failed publishing GitHub change event for {EntityId}", entityId); + } + } + + private string? ResolveWorkspacePath() + { + return HttpContext.RequestServices.GetService()?.WorkspacePath; + } + + private static bool IsOAuthConfigured(GitHubOAuthOptions oauth) + { + return !string.IsNullOrWhiteSpace(oauth.ClientId) + && !string.IsNullOrWhiteSpace(oauth.RedirectUri) + && !string.IsNullOrWhiteSpace(oauth.AuthorizeEndpoint); + } + + private static bool TryNormalizeIssueState(string? state, out string? normalizedState, out string? errorMessage) + { + if (string.IsNullOrWhiteSpace(state)) + { + normalizedState = null; + errorMessage = null; + return true; + } + + normalizedState = state.Trim().ToLowerInvariant(); + if (AllowedIssueStates.Contains(normalizedState)) + { + errorMessage = null; + return true; + } + + normalizedState = null; + errorMessage = "Invalid state. Allowed values: open, closed, all."; + return false; + } + + private static bool TryNormalizeCloseReason(string? reason, out string? normalizedReason, out string? errorMessage) + { + if (string.IsNullOrWhiteSpace(reason)) + { + normalizedReason = null; + errorMessage = null; + return true; + } + + normalizedReason = reason.Trim().ToLowerInvariant(); + if (AllowedCloseReasons.Contains(normalizedReason)) + { + errorMessage = null; + return true; + } + + normalizedReason = null; + errorMessage = "Invalid close reason. Allowed values: completed, not_planned."; + return false; + } +} /// Request to create GitHub issue. TR-PLANNED-013. public sealed class GitHubIssueRequest @@ -481,62 +549,62 @@ public sealed class GitHubIssueRequest } /// Request to add comment. TR-PLANNED-013. -public sealed class GitHubCommentRequest -{ - /// Comment body. - public string? Body { get; set; } -} - -/// TR-MCP-GH-002: Request body for setting a workspace GitHub token. -public sealed class GitHubAuthTokenUpsertRequest -{ - /// OAuth access token or personal access token. - public string? AccessToken { get; set; } - - /// Optional token expiration timestamp in UTC. - public DateTimeOffset? ExpiresAtUtc { get; set; } -} - -/// TR-MCP-GH-002: Workspace GitHub auth status response payload. -public sealed class GitHubAuthStatusResponse -{ - /// Resolved workspace path. - public string WorkspacePath { get; set; } = string.Empty; - - /// Current auth mode (stored_token, cli_fallback, or none). - public string AuthMode { get; set; } = "none"; - - /// Whether a workspace token is stored. - public bool HasStoredToken { get; set; } - - /// When the stored token was last updated. - public DateTimeOffset? TokenUpdatedAtUtc { get; set; } - - /// When the stored token expires, if known. - public DateTimeOffset? TokenExpiresAtUtc { get; set; } - - /// Whether ambient CLI auth fallback is allowed. - public bool CliFallbackAllowed { get; set; } - - /// Whether OAuth app bootstrap settings are configured. - public bool OAuthConfigured { get; set; } -} - -/// TR-MCP-GH-001: OAuth app bootstrap configuration payload. -public sealed class GitHubOAuthConfigResponse -{ - /// GitHub OAuth app client identifier. - public string ClientId { get; set; } = string.Empty; - - /// OAuth redirect URI configured for the app. - public string RedirectUri { get; set; } = string.Empty; - - /// Space-separated OAuth scopes. - public string Scopes { get; set; } = string.Empty; - - /// GitHub authorize endpoint URI. - public string AuthorizeEndpoint { get; set; } = string.Empty; - - /// Whether OAuth values are complete enough to build an authorize URL. - public bool IsConfigured { get; set; } -} +public sealed class GitHubCommentRequest +{ + /// Comment body. + public string? Body { get; set; } +} + +/// TR-MCP-GH-002: Request body for setting a workspace GitHub token. +public sealed class GitHubAuthTokenUpsertRequest +{ + /// OAuth access token or personal access token. + public string? AccessToken { get; set; } + + /// Optional token expiration timestamp in UTC. + public DateTimeOffset? ExpiresAtUtc { get; set; } +} + +/// TR-MCP-GH-002: Workspace GitHub auth status response payload. +public sealed class GitHubAuthStatusResponse +{ + /// Resolved workspace path. + public string WorkspacePath { get; set; } = string.Empty; + + /// Current auth mode (stored_token, cli_fallback, or none). + public string AuthMode { get; set; } = "none"; + + /// Whether a workspace token is stored. + public bool HasStoredToken { get; set; } + + /// When the stored token was last updated. + public DateTimeOffset? TokenUpdatedAtUtc { get; set; } + + /// When the stored token expires, if known. + public DateTimeOffset? TokenExpiresAtUtc { get; set; } + + /// Whether ambient CLI auth fallback is allowed. + public bool CliFallbackAllowed { get; set; } + + /// Whether OAuth app bootstrap settings are configured. + public bool OAuthConfigured { get; set; } +} + +/// TR-MCP-GH-001: OAuth app bootstrap configuration payload. +public sealed class GitHubOAuthConfigResponse +{ + /// GitHub OAuth app client identifier. + public string ClientId { get; set; } = string.Empty; + + /// OAuth redirect URI configured for the app. + public string RedirectUri { get; set; } = string.Empty; + + /// Space-separated OAuth scopes. + public string Scopes { get; set; } = string.Empty; + + /// GitHub authorize endpoint URI. + public string AuthorizeEndpoint { get; set; } = string.Empty; + + /// Whether OAuth values are complete enough to build an authorize URL. + public bool IsConfigured { get; set; } +} diff --git a/src/McpServer.Support.Mcp/Controllers/TodoController.cs b/src/McpServer.Support.Mcp/Controllers/TodoController.cs index b080f0d5..7a22336d 100644 --- a/src/McpServer.Support.Mcp/Controllers/TodoController.cs +++ b/src/McpServer.Support.Mcp/Controllers/TodoController.cs @@ -97,6 +97,38 @@ public async Task> GetAuditAsync( } } + /// TR-MCP-TODO-006: Get SQLite-authoritative TODO projection status and repair guidance. + [HttpGet("projection/status")] + public async Task> GetProjectionStatusAsync(CancellationToken cancellationToken) + { + try + { + var result = await _todoService.GetProjectionStatusAsync(cancellationToken).ConfigureAwait(false); + return Ok(result); + } + catch (NotSupportedException ex) + { + return StatusCode(StatusCodes.Status501NotImplemented, new { error = ex.Message }); + } + } + + /// TR-MCP-TODO-006: Repair TODO.yaml projection from SQLite-authoritative TODO storage. + [HttpPost("projection/repair")] + public async Task> RepairProjectionAsync(CancellationToken cancellationToken) + { + try + { + var result = await _todoService.RepairProjectionAsync(cancellationToken).ConfigureAwait(false); + return result.Success + ? Ok(result) + : StatusCode(StatusCodes.Status500InternalServerError, result); + } + catch (NotSupportedException ex) + { + return StatusCode(StatusCodes.Status501NotImplemented, new { error = ex.Message }); + } + } + /// TR-PLANNED-013: Create a new TODO item. [HttpPost] public async Task> CreateAsync( diff --git a/src/McpServer.Support.Mcp/Controllers/WorkspaceController.cs b/src/McpServer.Support.Mcp/Controllers/WorkspaceController.cs index a3f2ccae..9c2ecc8a 100644 --- a/src/McpServer.Support.Mcp/Controllers/WorkspaceController.cs +++ b/src/McpServer.Support.Mcp/Controllers/WorkspaceController.cs @@ -1,5 +1,3 @@ -using System.Text.Json; -using System.Text.Json.Nodes; using McpServer.Support.Mcp.Notifications; using McpServer.Support.Mcp.Options; using McpServer.Support.Mcp.Services; @@ -305,39 +303,7 @@ public async Task> UpdateGlobalPromptAsync( var newTemplate = string.IsNullOrWhiteSpace(request.Template) ? null : request.Template.Trim(); - var appsettingsPath = _appSettingsFileService.ResolvePreferredAppsettingsPath(); - var reloadedByYamlService = false; - if (appsettingsPath.EndsWith(".yaml", StringComparison.OrdinalIgnoreCase)) - { - var data = await _appSettingsFileService.LoadYamlAsync(appsettingsPath, ct).ConfigureAwait(false); - if (!data.TryGetValue("Mcp", out var mcpObj) || mcpObj is not IDictionary mcpDict) - { - data["Mcp"] = mcpDict = new Dictionary(); - } - - if (newTemplate is null) - mcpDict.Remove("MarkerPromptTemplate"); - else - mcpDict["MarkerPromptTemplate"] = newTemplate; - - await _appSettingsFileService.SaveYamlAsync(data, appsettingsPath, ct).ConfigureAwait(false); - reloadedByYamlService = true; - } - else - { - var jsonText = await System.IO.File.ReadAllTextAsync(appsettingsPath, ct).ConfigureAwait(false); - var doc = JsonNode.Parse(jsonText, new JsonNodeOptions { PropertyNameCaseInsensitive = true })!; - var mcp = doc["Mcp"] as JsonObject ?? new JsonObject(); - if (newTemplate is null) - mcp.Remove("MarkerPromptTemplate"); - else - mcp["MarkerPromptTemplate"] = newTemplate; - doc["Mcp"] = mcp; - await System.IO.File.WriteAllTextAsync(appsettingsPath, doc.ToJsonString(s_jsonOptions), ct).ConfigureAwait(false); - } - - if (!reloadedByYamlService && _configuration is IConfigurationRoot root) - root.Reload(); + await _appSettingsFileService.UpdateGlobalPromptTemplateAsync(newTemplate, ct).ConfigureAwait(false); // Regenerate all marker files so running workspaces pick up the new global prompt. // Pass the new template explicitly to avoid IOptionsMonitor staleness after reload. @@ -382,13 +348,6 @@ private static bool IsPrimaryInstance(WorkspaceDto _) // All workspaces share a single port; the primary workspace is always served by this process. return true; } - - private static readonly JsonSerializerOptions s_jsonOptions = new() - { - WriteIndented = true, - DefaultIgnoreCondition = System.Text.Json.Serialization.JsonIgnoreCondition.WhenWritingNull, - }; - // Base64URL encode a workspace path for use as a URL key. private static string EncodeKey(string path) { diff --git a/src/McpServer.Support.Mcp/McpStdio/McpServerMcpTools.cs b/src/McpServer.Support.Mcp/McpStdio/McpServerMcpTools.cs index 7dc538f0..568b55ab 100644 --- a/src/McpServer.Support.Mcp/McpStdio/McpServerMcpTools.cs +++ b/src/McpServer.Support.Mcp/McpStdio/McpServerMcpTools.cs @@ -488,6 +488,52 @@ public async Task TodoAudit( } } + /// TR-MCP-TODO-006: Get SQLite-authoritative TODO projection status. + [McpServerTool(Name = "todo_projection_status"), Description("Get projection status for SQLite-backed TODO storage.")] + public async Task TodoProjectionStatus( + [Description("Workspace path (required)")] string workspacePath, + CancellationToken cancellationToken = default) + { + ApplyWorkspaceOverride(workspacePath); + try + { + var result = await _workspaceAccessor.GetTodoService().GetProjectionStatusAsync(cancellationToken).ConfigureAwait(false); + return JsonSerializer.Serialize(result); + } + catch (NotSupportedException ex) + { + return JsonSerializer.Serialize(new { error = ex.Message }); + } + catch (Exception ex) + { + _logger.LogError("{ExceptionDetail}", ex.ToString()); + return JsonSerializer.Serialize(new { error = ex.Message }); + } + } + + /// TR-MCP-TODO-006: Repair TODO.yaml projection from SQLite-authoritative TODO storage. + [McpServerTool(Name = "todo_projection_repair"), Description("Repair TODO.yaml projection from authoritative SQLite-backed TODO storage.")] + public async Task TodoProjectionRepair( + [Description("Workspace path (required)")] string workspacePath, + CancellationToken cancellationToken = default) + { + ApplyWorkspaceOverride(workspacePath); + try + { + var result = await _workspaceAccessor.GetTodoService().RepairProjectionAsync(cancellationToken).ConfigureAwait(false); + return JsonSerializer.Serialize(result); + } + catch (NotSupportedException ex) + { + return JsonSerializer.Serialize(new { error = ex.Message }); + } + catch (Exception ex) + { + _logger.LogError("{ExceptionDetail}", ex.ToString()); + return JsonSerializer.Serialize(new { error = ex.Message }); + } + } + /// TR-PLANNED-013: Create a new TODO item. [McpServerTool(Name = "todo_create"), Description("Create a new TODO item. Requires id, title, section, priority.")] public async Task TodoCreate( diff --git a/src/McpServer.Support.Mcp/Middleware/InteractionLoggingMiddleware.cs b/src/McpServer.Support.Mcp/Middleware/InteractionLoggingMiddleware.cs index 35ebec11..392aa5ee 100644 --- a/src/McpServer.Support.Mcp/Middleware/InteractionLoggingMiddleware.cs +++ b/src/McpServer.Support.Mcp/Middleware/InteractionLoggingMiddleware.cs @@ -124,8 +124,14 @@ public async Task InvokeAsync(HttpContext context) entry.RequestBody ?? "(none)", entry.ResponseBody ?? "(none)"); - if (!string.IsNullOrWhiteSpace(_options.LoggingServiceUrl) && _channel != null) - _channel.TryEnqueue(entry); + if (!string.IsNullOrWhiteSpace(_options.LoggingServiceUrl) && _channel != null && !_channel.TryEnqueue(entry)) + { + _logger.LogWarning( + "Interaction log forwarding rejected request {RequestId} for {Method} {Path} because the submission queue is full.", + entry.RequestId, + entry.Method, + entry.Path); + } } } diff --git a/src/McpServer.Support.Mcp/Middleware/WorkspaceAuthMiddleware.cs b/src/McpServer.Support.Mcp/Middleware/WorkspaceAuthMiddleware.cs index 9b10620b..130d0e64 100644 --- a/src/McpServer.Support.Mcp/Middleware/WorkspaceAuthMiddleware.cs +++ b/src/McpServer.Support.Mcp/Middleware/WorkspaceAuthMiddleware.cs @@ -16,7 +16,7 @@ namespace McpServer.Support.Mcp.Middleware; /// no fallthrough to API-key auth occurs. /// API key — for agents that cannot perform OIDC. /// Full-access keys (from marker files) grant unrestricted access. Default keys -/// (from GET /api-key) grant read-only access except for TODO routes. +/// (from GET /api-key) grant read-only access only. /// /// Non-/mcpserver/ routes (health, swagger, MCP transport, /api-key) pass through unprotected. /// @@ -124,17 +124,37 @@ await context.Response.WriteAsync( // ── API key path (agents only) ──────────────────────────────────────── var workspacePath = workspaceContext.WorkspacePath ?? configuration["Mcp:RepoRoot"] ?? string.Empty; - // If no workspace is configured or no token generated yet (startup race), allow through. + // Fail closed when workspace resolution or token initialization is unavailable. if (string.IsNullOrWhiteSpace(workspacePath)) { - await _next(context).ConfigureAwait(false); + _logger.LogWarning("[WS-Auth] {Method} {Path} | Workspace unresolved for API-key auth → 503", + method, path); + context.Response.StatusCode = StatusCodes.Status503ServiceUnavailable; + context.Response.ContentType = "application/json"; + var unresolvedWorkspaceBody = new + { + error = "Workspace authentication is unavailable because no workspace could be resolved for this request. Send X-Workspace-Path or retry after startup completes." + }; + await context.Response.WriteAsync( + JsonSerializer.Serialize(unresolvedWorkspaceBody, s_json), + context.RequestAborted).ConfigureAwait(false); return; } var expected = tokenService.GetToken(workspacePath); if (expected is null) { - await _next(context).ConfigureAwait(false); + _logger.LogWarning("[WS-Auth] {Method} {Path} | Full workspace token missing for {WorkspacePath} → 503", + method, path, workspacePath); + context.Response.StatusCode = StatusCodes.Status503ServiceUnavailable; + context.Response.ContentType = "application/json"; + var missingTokenBody = new + { + error = "Workspace authentication is temporarily unavailable because the workspace API token has not been initialized. Retry after startup completes." + }; + await context.Response.WriteAsync( + JsonSerializer.Serialize(missingTokenBody, s_json), + context.RequestAborted).ConfigureAwait(false); return; } @@ -148,28 +168,25 @@ await context.Response.WriteAsync( return; } - // Default (anonymous) token — read-only except for TODO routes. + // Default (anonymous) token — read-only only. if (tokenService.ValidateDefaultToken(workspacePath, provided)) { context.Items[IsDefaultKeyItem] = true; - - var isTodoRoute = path.StartsWithSegments("/mcpserver/todo", StringComparison.OrdinalIgnoreCase); var isReadOnly = s_readOnlyMethods.Contains(context.Request.Method); - if (isTodoRoute || isReadOnly) + if (isReadOnly) { await _next(context).ConfigureAwait(false); return; } - // Write operation on a non-todo route with only a default key — reject. + // Write operation with only a default key — reject. context.Response.StatusCode = StatusCodes.Status403Forbidden; context.Response.ContentType = "application/json"; var forbiddenBody = new { - error = "Default API key grants read-only access to non-todo endpoints. " + - "Use the full workspace API key from the AGENTS-README-FIRST.yaml marker file for write operations, " + - "or authenticate with a valid JWT Bearer token." + error = "Default API key grants read-only access only. " + + "Use the full workspace API key from the AGENTS-README-FIRST.yaml marker file or a valid JWT Bearer token for write operations." }; await context.Response.WriteAsync( JsonSerializer.Serialize(forbiddenBody, s_json), diff --git a/src/McpServer.Support.Mcp/Middleware/WorkspaceResolutionMiddleware.cs b/src/McpServer.Support.Mcp/Middleware/WorkspaceResolutionMiddleware.cs index bc6d0eb2..d6a330fa 100644 --- a/src/McpServer.Support.Mcp/Middleware/WorkspaceResolutionMiddleware.cs +++ b/src/McpServer.Support.Mcp/Middleware/WorkspaceResolutionMiddleware.cs @@ -9,8 +9,9 @@ namespace McpServer.Support.Mcp.Middleware; /// X-Workspace-Path header — explicit workspace path (highest priority). /// X-Api-Key reverse lookup via . /// -/// If neither tier resolves a workspace, workspace-independent routes pass through with an -/// empty ; workspace-required routes receive a 404. +/// If neither tier resolves a workspace, API-key and unauthenticated callers may continue on +/// explicitly workspace-independent routes, but Bearer-authenticated callers must still supply +/// X-Workspace-Path for tenant-scoped routes. Workspace-required routes receive a 404. /// Populates the scoped for downstream services. /// Non-/mcpserver/ and non-/mcp-transport routes skip resolution. /// @@ -42,6 +43,12 @@ public sealed class WorkspaceResolutionMiddleware "/mcp-transport", }; + private static readonly HashSet BearerWorkspaceIndependentPrefixes = new(StringComparer.OrdinalIgnoreCase) + { + "/mcpserver/workspace", + "/mcpserver/tools", + }; + private readonly RequestDelegate _next; private readonly ILogger _logger; @@ -135,7 +142,7 @@ await context.Response.WriteAsync( } // No workspace resolved — check whether this route requires one. - if (IsWorkspaceIndependent(path)) + if (IsWorkspaceIndependent(path, hasBearerToken)) { _logger.LogDebug("[WS-Resolve] {Method} {Path} | SKIP: workspace-independent route, proceeding without workspace", method, path); @@ -149,13 +156,16 @@ await context.Response.WriteAsync( context.Response.StatusCode = StatusCodes.Status404NotFound; context.Response.ContentType = "application/json"; await context.Response.WriteAsync( - """{"error":"Workspace required. Send X-Workspace-Path header."}""", + hasBearerToken + ? """{"error":"Workspace required. Bearer-authenticated requests must send X-Workspace-Path for tenant-scoped routes."}""" + : """{"error":"Workspace required. Send X-Workspace-Path header."}""", context.RequestAborted).ConfigureAwait(false); } - private static bool IsWorkspaceIndependent(PathString path) + private static bool IsWorkspaceIndependent(PathString path, bool hasBearerToken) { - foreach (var prefix in WorkspaceIndependentPrefixes) + var prefixes = hasBearerToken ? BearerWorkspaceIndependentPrefixes : WorkspaceIndependentPrefixes; + foreach (var prefix in prefixes) { if (path.StartsWithSegments(prefix, StringComparison.OrdinalIgnoreCase)) return true; diff --git a/src/McpServer.Support.Mcp/Program.cs b/src/McpServer.Support.Mcp/Program.cs index fb464acc..41ee6886 100644 --- a/src/McpServer.Support.Mcp/Program.cs +++ b/src/McpServer.Support.Mcp/Program.cs @@ -1,6 +1,7 @@ // TR-PLANNED-013 / FR-SUPPORT-010: MCP Context Unification - local MCP server for Cursor and Copilot. using System.Globalization; +using System.Net; using System.Security.Claims; using System.Security.Cryptography; using System.Runtime.Versioning; @@ -388,12 +389,15 @@ static string ResolvePath(string repoRootPath, string path) => builder.Services.AddScoped(); builder.Services.AddScoped(); builder.Services.AddSingleton(); +builder.Services.AddSingleton(); builder.Services.AddScoped(); builder.Services.AddSingleton(new ServerRuntimeInfo(serverStartupUtc, listenPort)); builder.Services.AddSingleton(); +builder.Services.Configure(builder.Configuration.GetSection(DesktopLaunchOptions.SectionName)); builder.Services.Configure(builder.Configuration.GetSection(PairingOptions.SectionName)); builder.Services.Configure(builder.Configuration.GetSection(OidcAuthOptions.SectionName)); builder.Services.Configure(builder.Configuration.GetSection(ToolRegistryOptions.SectionName)); +builder.Services.AddSingleton(); builder.Services.AddSingleton(); var oidcAuthBootstrap = builder.Configuration.GetSection(OidcAuthOptions.SectionName).Get() @@ -576,9 +580,6 @@ static string ResolvePath(string repoRootPath, string path) => } } -app.Lifetime.ApplicationStopping.Register(() => - app.Services.GetRequiredService().StopAllAsync().GetAwaiter().GetResult()); - app.UseGlobalExceptionHandler(); app.UseMiddleware(); @@ -607,8 +608,19 @@ static string ResolvePath(string repoRootPath, string path) => restrictToCurrentRepoRoot: false)) .ExcludeFromDescription(); -app.MapGet("/api-key", (WorkspaceTokenService tokenService) => +app.MapGet("/api-key", (HttpContext context, WorkspaceTokenService tokenService, ApiKeyIssuanceGuard apiKeyIssuanceGuard) => { + if (!IsLoopbackRequest(context)) + { + app.Logger.LogWarning( + "Rejected non-loopback /api-key request: RemoteIp={RemoteIp}", + context.Connection.RemoteIpAddress?.ToString() ?? "(none)"); + return Results.NotFound(); + } + + context.Response.Headers.CacheControl = "no-store, no-cache"; + context.Response.Headers.Pragma = "no-cache"; + var workspacePath = ResolvePrimaryApiKeyWorkspacePath(app.Configuration, app.Environment, instanceName) ?? string.Empty; if (string.IsNullOrWhiteSpace(workspacePath)) return Results.Problem("No workspace configured.", statusCode: 503); @@ -616,12 +628,28 @@ static string ResolvePath(string repoRootPath, string path) => var defaultToken = tokenService.GetDefaultToken(workspacePath); if (defaultToken is null) { - defaultToken = tokenService.GenerateDefaultToken(workspacePath); - app.Logger.LogWarning( - "Default API token was missing during /api-key request and was generated on demand: Workspace={WorkspacePath}", + app.Logger.LogError( + "Default API token unavailable during /api-key request: Workspace={WorkspacePath}", workspacePath); + return Results.Problem("Default API token unavailable.", statusCode: 503); + } + + if (!apiKeyIssuanceGuard.TryAcquire(context.Connection.RemoteIpAddress, out var retryAfter)) + { + var retryAfterSeconds = Math.Max(1, (int)Math.Ceiling(retryAfter.TotalSeconds)); + context.Response.Headers.RetryAfter = retryAfterSeconds.ToString(CultureInfo.InvariantCulture); + app.Logger.LogWarning( + "Default API token issuance throttled: RemoteIp={RemoteIp}; Workspace={WorkspacePath}; RetryAfterSeconds={RetryAfterSeconds}", + context.Connection.RemoteIpAddress?.ToString() ?? "loopback", + workspacePath, + retryAfterSeconds); + return Results.Problem("Default API token issuance is temporarily rate-limited.", statusCode: 429); } + app.Logger.LogInformation( + "Default API token issued: RemoteIp={RemoteIp}; Workspace={WorkspacePath}", + context.Connection.RemoteIpAddress?.ToString() ?? "loopback", + workspacePath); return Results.Ok(new { apiKey = defaultToken }); }).ExcludeFromDescription(); @@ -636,22 +664,53 @@ static string ResolvePath(string repoRootPath, string path) => return Results.Content(await pairingRenderer.RenderLoginPageAsync().ConfigureAwait(false), "text/html"); }).ExcludeFromDescription(); -app.MapPost("/pair", async (HttpContext context, IOptions opts, PairingSessionService sessions, PairingHtmlRenderer pairingRenderer) => +app.MapPost("/pair", async (HttpContext context, IOptions opts, PairingSessionService sessions, PairingLoginAttemptGuard attemptGuard, PairingHtmlRenderer pairingRenderer) => { var o = opts.Value; if (o.PairingUsers.Count == 0 || string.IsNullOrEmpty(o.ApiKey)) return Results.Content(await pairingRenderer.RenderNotConfiguredPageAsync().ConfigureAwait(false), "text/html"); var form = await context.Request.ReadFormAsync().ConfigureAwait(false); - var username = form["username"].ToString(); + var username = form["username"].ToString().Trim(); var password = form["password"].ToString(); + var remoteIp = context.Connection.RemoteIpAddress; + + if (!attemptGuard.TryAcquire(username, remoteIp, out var retryAfter)) + { + var retryAfterSeconds = Math.Max(1, (int)Math.Ceiling(retryAfter.TotalSeconds)); + context.Response.Headers.RetryAfter = retryAfterSeconds.ToString(CultureInfo.InvariantCulture); + app.Logger.LogWarning( + "Pairing sign-in blocked after repeated failures: RemoteIp={RemoteIp}; Username={Username}; RetryAfterSeconds={RetryAfterSeconds}", + remoteIp?.ToString() ?? "loopback", + username, + retryAfterSeconds); + return Results.Content( + await pairingRenderer.RenderLoginPageAsync("Too many failed sign-in attempts. Please wait and try again.").ConfigureAwait(false), + "text/html", + Encoding.UTF8, + StatusCodes.Status429TooManyRequests); + } var user = o.PairingUsers.Find(u => string.Equals(u.Username, username, StringComparison.OrdinalIgnoreCase)); if (user is null || !VerifyPairingPassword(password, user.PasswordHash)) - return Results.Content(await pairingRenderer.RenderLoginPageAsync(error: true).ConfigureAwait(false), "text/html"); + { + attemptGuard.RecordFailure(username, remoteIp); + app.Logger.LogWarning( + "Pairing sign-in failed: RemoteIp={RemoteIp}; Username={Username}", + remoteIp?.ToString() ?? "loopback", + username); + return Results.Content( + await pairingRenderer.RenderLoginPageAsync("Invalid username or password.").ConfigureAwait(false), + "text/html"); + } + attemptGuard.RecordSuccess(username, remoteIp); + app.Logger.LogInformation( + "Pairing sign-in succeeded: RemoteIp={RemoteIp}; Username={Username}", + remoteIp?.ToString() ?? "loopback", + username); var token = sessions.CreateToken(); context.Response.Cookies.Append("mcp_pair", token, new CookieOptions { @@ -691,6 +750,12 @@ static bool VerifyPairingPassword(string plaintext, string expectedHash) return CryptographicOperations.FixedTimeEquals(computed, expected); } +static bool IsLoopbackRequest(HttpContext context) +{ + var remoteIp = context.Connection.RemoteIpAddress; + return remoteIp is null || IPAddress.IsLoopback(remoteIp); +} + static void DisableEnvironmentSpecificJsonConfigForWindowsService(WebApplicationBuilder builder) { if (!OperatingSystem.IsWindows() || !WindowsServiceHelpers.IsWindowsService()) @@ -718,7 +783,11 @@ static void DisableEnvironmentSpecificJsonConfigForWindowsService(WebApplication static void EnsureApprovedWindowsServiceDeployment() { - if (!OperatingSystem.IsWindows() || !WindowsServiceHelpers.IsWindowsService()) + if (!OperatingSystem.IsWindows()) + return; + + if (!WindowsServiceHelpers.IsWindowsService() && + !WindowsServiceDeploymentGuard.HasDeploymentManifest(AppContext.BaseDirectory)) return; WindowsServiceDeploymentGuard.EnsureApprovedDeployment(AppContext.BaseDirectory, WriteWindowsServiceDeploymentFailure); } diff --git a/src/McpServer.Support.Mcp/Services/AppSettingsFileService.cs b/src/McpServer.Support.Mcp/Services/AppSettingsFileService.cs index cf087838..2e492665 100644 --- a/src/McpServer.Support.Mcp/Services/AppSettingsFileService.cs +++ b/src/McpServer.Support.Mcp/Services/AppSettingsFileService.cs @@ -1,3 +1,5 @@ +using System.Text.Json; +using System.Text.Json.Nodes; using Microsoft.AspNetCore.Hosting; using Microsoft.Extensions.Configuration; using YamlDotNet.Serialization; @@ -21,16 +23,39 @@ public sealed class AppSettingsFileService .ConfigureDefaultValuesHandling(DefaultValuesHandling.OmitNull) .Build(); + private static readonly JsonNodeOptions s_jsonNodeOptions = new() + { + PropertyNameCaseInsensitive = true, + }; + + private static readonly JsonSerializerOptions s_jsonOptions = new() + { + WriteIndented = true, + DefaultIgnoreCondition = System.Text.Json.Serialization.JsonIgnoreCondition.WhenWritingNull, + }; + private readonly IConfiguration _configuration; private readonly IWebHostEnvironment _environment; + private readonly SemaphoreSlim _writeLock = new(1, 1); + private readonly Func _writeTextAsync; /// Initializes a new instance of the class. /// The active configuration root. /// The current host environment. public AppSettingsFileService(IConfiguration configuration, IWebHostEnvironment environment) + : this(configuration, environment, static (path, content, ct) => File.WriteAllTextAsync(path, content, ct)) + { + } + + internal AppSettingsFileService( + IConfiguration configuration, + IWebHostEnvironment environment, + Func writeTextAsync) { _configuration = configuration; _environment = environment; + ArgumentNullException.ThrowIfNull(writeTextAsync); + _writeTextAsync = writeTextAsync; } /// @@ -115,6 +140,56 @@ public string ResolveYamlAppsettingsPath() public async Task> LoadYamlAsync(string? path = null, CancellationToken ct = default) { var resolvedPath = path ?? ResolveYamlAppsettingsPath(); + return await LoadYamlCoreAsync(resolvedPath, ct).ConfigureAwait(false); + } + + /// + /// Updates the global marker prompt template in the active appsettings file using atomic write semantics. + /// + /// The new marker prompt template identifier, or null to remove it. + /// The cancellation token. + public async Task UpdateGlobalPromptTemplateAsync(string? template, CancellationToken ct = default) + { + await _writeLock.WaitAsync(ct).ConfigureAwait(false); + try + { + var resolvedPath = ResolvePreferredAppsettingsPath(); + if (resolvedPath.EndsWith(".yaml", StringComparison.OrdinalIgnoreCase)) + { + var data = await LoadYamlCoreAsync(resolvedPath, ct).ConfigureAwait(false); + if (!data.TryGetValue("Mcp", out var mcpObj) || mcpObj is not IDictionary mcpDict) + data["Mcp"] = mcpDict = new Dictionary(); + + if (template is null) + mcpDict.Remove("MarkerPromptTemplate"); + else + mcpDict["MarkerPromptTemplate"] = template; + + await SaveYamlCoreAsync(data, resolvedPath, ct).ConfigureAwait(false); + } + else + { + var document = await LoadJsonCoreAsync(resolvedPath, ct).ConfigureAwait(false); + var mcp = document["Mcp"] as JsonObject ?? new JsonObject(); + if (template is null) + mcp.Remove("MarkerPromptTemplate"); + else + mcp["MarkerPromptTemplate"] = template; + + document["Mcp"] = mcp; + await SaveJsonCoreAsync(document, resolvedPath, ct).ConfigureAwait(false); + } + + ReloadConfiguration(); + } + finally + { + _writeLock.Release(); + } + } + + private async Task> LoadYamlCoreAsync(string resolvedPath, CancellationToken ct) + { if (!File.Exists(resolvedPath)) return []; @@ -136,13 +211,16 @@ public async Task SaveYamlAsync(Dictionary data, string? path = ArgumentNullException.ThrowIfNull(data); var resolvedPath = path ?? ResolveYamlAppsettingsPath(); - var directory = Path.GetDirectoryName(resolvedPath); - if (!string.IsNullOrWhiteSpace(directory)) - Directory.CreateDirectory(directory); - - var yamlText = s_serializer.Serialize(data); - await File.WriteAllTextAsync(resolvedPath, yamlText, ct).ConfigureAwait(false); - ReloadConfiguration(); + await _writeLock.WaitAsync(ct).ConfigureAwait(false); + try + { + await SaveYamlCoreAsync(data, resolvedPath, ct).ConfigureAwait(false); + ReloadConfiguration(); + } + finally + { + _writeLock.Release(); + } } /// @@ -157,14 +235,23 @@ public async Task> PatchYamlConfigurationAsy { ArgumentNullException.ThrowIfNull(updates); - var resolvedPath = ResolveYamlAppsettingsPath(); - var data = await LoadYamlAsync(resolvedPath, ct).ConfigureAwait(false); + await _writeLock.WaitAsync(ct).ConfigureAwait(false); + try + { + var resolvedPath = ResolveYamlAppsettingsPath(); + var data = await LoadYamlCoreAsync(resolvedPath, ct).ConfigureAwait(false); - foreach (var update in updates) - ApplyPatch(data, update.Key, update.Value); + foreach (var update in updates) + ApplyPatch(data, update.Key, update.Value); - await SaveYamlAsync(data, resolvedPath, ct).ConfigureAwait(false); - return GetConfigurationValues(); + await SaveYamlCoreAsync(data, resolvedPath, ct).ConfigureAwait(false); + ReloadConfiguration(); + return GetConfigurationValues(); + } + finally + { + _writeLock.Release(); + } } private void ReloadConfiguration() @@ -173,6 +260,52 @@ private void ReloadConfiguration() root.Reload(); } + private async Task LoadJsonCoreAsync(string resolvedPath, CancellationToken ct) + { + if (!File.Exists(resolvedPath)) + return new JsonObject(); + + var jsonText = await File.ReadAllTextAsync(resolvedPath, ct).ConfigureAwait(false); + if (string.IsNullOrWhiteSpace(jsonText)) + return new JsonObject(); + + return JsonNode.Parse(jsonText, s_jsonNodeOptions) as JsonObject ?? new JsonObject(); + } + + private async Task SaveYamlCoreAsync(Dictionary data, string resolvedPath, CancellationToken ct) + { + var yamlText = s_serializer.Serialize(data); + await WriteTextAtomicallyAsync(resolvedPath, yamlText, ct).ConfigureAwait(false); + } + + private async Task SaveJsonCoreAsync(JsonObject data, string resolvedPath, CancellationToken ct) + { + var jsonText = data.ToJsonString(s_jsonOptions); + await WriteTextAtomicallyAsync(resolvedPath, jsonText, ct).ConfigureAwait(false); + } + + private async Task WriteTextAtomicallyAsync(string resolvedPath, string content, CancellationToken ct) + { + var directory = Path.GetDirectoryName(resolvedPath); + if (!string.IsNullOrWhiteSpace(directory)) + Directory.CreateDirectory(directory); + + var tempPath = Path.Combine( + directory ?? AppContext.BaseDirectory, + $".{Path.GetFileName(resolvedPath)}.{Guid.NewGuid():N}.tmp"); + + try + { + await _writeTextAsync(tempPath, content, ct).ConfigureAwait(false); + File.Move(tempPath, resolvedPath, overwrite: true); + } + finally + { + if (File.Exists(tempPath)) + File.Delete(tempPath); + } + } + private string? ResolveLoadedAppsettingsPath(string fileName) { if (_configuration is not IConfigurationRoot root) diff --git a/src/McpServer.Support.Mcp/Services/DesktopLaunchService.cs b/src/McpServer.Support.Mcp/Services/DesktopLaunchService.cs index 7bce54c6..47fe5105 100644 --- a/src/McpServer.Support.Mcp/Services/DesktopLaunchService.cs +++ b/src/McpServer.Support.Mcp/Services/DesktopLaunchService.cs @@ -1,8 +1,10 @@ using System.Text.Json; using System.Text.Json.Serialization; using McpServer.Support.Mcp.Models; +using McpServer.Support.Mcp.Options; using Microsoft.Extensions.Configuration; using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Options; namespace McpServer.Support.Mcp.Services; @@ -18,6 +20,7 @@ public sealed class DesktopLaunchService }; private readonly IConfiguration _configuration; + private readonly DesktopLaunchOptions _desktopLaunchOptions; private readonly ILogger _logger; private readonly IProcessRunner _processRunner; @@ -26,14 +29,17 @@ public sealed class DesktopLaunchService /// configured launcher location, structured process runner, and logger. /// /// Application configuration used to resolve launcher paths. + /// Privileged desktop-launch feature-gate and allowlist configuration. /// Process runner used to invoke McpServer.Launcher.exe. /// Logger for diagnostic output. public DesktopLaunchService( IConfiguration configuration, + IOptions desktopLaunchOptions, IProcessRunner processRunner, ILogger logger) { _configuration = configuration ?? throw new ArgumentNullException(nameof(configuration)); + _desktopLaunchOptions = desktopLaunchOptions?.Value ?? throw new ArgumentNullException(nameof(desktopLaunchOptions)); _processRunner = processRunner ?? throw new ArgumentNullException(nameof(processRunner)); _logger = logger ?? throw new ArgumentNullException(nameof(logger)); } @@ -57,6 +63,35 @@ public async Task LaunchAsync( if (string.IsNullOrWhiteSpace(workspacePath)) return CreateFailureResult("workspacePath is required."); + if (!_desktopLaunchOptions.Enabled) + { + _logger.LogWarning( + "Rejected desktop launch for workspace {WorkspacePath} because desktop launch is disabled.", + workspacePath); + return CreateFailureResult("Desktop launch is disabled. Enable Mcp:DesktopLaunch:Enabled to allow local process launch."); + } + + if (_desktopLaunchOptions.AllowedExecutables.Count == 0) + { + _logger.LogWarning( + "Rejected desktop launch for workspace {WorkspacePath} because no desktop executables are allowlisted.", + workspacePath); + return CreateFailureResult("No desktop executables are allowlisted. Configure Mcp:DesktopLaunch:AllowedExecutables."); + } + + var normalizedExecutablePath = NormalizeExecutablePath(request.ExecutablePath); + if (normalizedExecutablePath is null) + return CreateFailureResult("executablePath must be a non-empty absolute path."); + + if (!PathGlobMatcher.MatchesAny(normalizedExecutablePath, _desktopLaunchOptions.AllowedExecutables)) + { + _logger.LogWarning( + "Rejected desktop launch for workspace {WorkspacePath} because executable {ExecutablePath} does not match the configured allowlist.", + workspacePath, + normalizedExecutablePath); + return CreateFailureResult("Executable path is not in the configured desktop allowlist."); + } + var launcherPath = ResolveLauncherPath(workspacePath); if (launcherPath is null) return CreateFailureResult("McpServer.Launcher.exe not found. Check Mcp:LauncherPath configuration."); @@ -65,7 +100,7 @@ public async Task LaunchAsync( { var payload = new DesktopLaunchRequest { - ExecutablePath = request.ExecutablePath, + ExecutablePath = normalizedExecutablePath, Arguments = request.Arguments, WorkingDirectory = request.WorkingDirectory, EnvironmentVariables = request.EnvironmentVariables, @@ -102,6 +137,29 @@ public async Task LaunchAsync( } } + private static string? NormalizeExecutablePath(string? executablePath) + { + if (string.IsNullOrWhiteSpace(executablePath) || !Path.IsPathRooted(executablePath)) + return null; + + try + { + return Path.GetFullPath(executablePath); + } + catch (ArgumentException) + { + return null; + } + catch (NotSupportedException) + { + return null; + } + catch (PathTooLongException) + { + return null; + } + } + private string? ResolveLauncherPath(string workspacePath) { var configPath = _configuration["Mcp:LauncherPath"]; diff --git a/src/McpServer.Support.Mcp/Services/WindowsServiceDeploymentGuard.cs b/src/McpServer.Support.Mcp/Services/WindowsServiceDeploymentGuard.cs index 08d8456b..f3fda0a7 100644 --- a/src/McpServer.Support.Mcp/Services/WindowsServiceDeploymentGuard.cs +++ b/src/McpServer.Support.Mcp/Services/WindowsServiceDeploymentGuard.cs @@ -5,6 +5,12 @@ namespace McpServer.Support.Mcp.Services; internal static class WindowsServiceDeploymentGuard { + internal static bool HasDeploymentManifest(string baseDirectory) + { + ArgumentException.ThrowIfNullOrWhiteSpace(baseDirectory); + return File.Exists(Path.Combine(baseDirectory, ".mcpservice-deployment.json")); + } + internal static void EnsureApprovedDeployment(string baseDirectory, Action? logFailure = null) { ArgumentException.ThrowIfNullOrWhiteSpace(baseDirectory); diff --git a/src/McpServer.Support.Mcp/Web/PairingHtml.cs b/src/McpServer.Support.Mcp/Web/PairingHtml.cs index ab9bb098..0fdd04b4 100644 --- a/src/McpServer.Support.Mcp/Web/PairingHtml.cs +++ b/src/McpServer.Support.Mcp/Web/PairingHtml.cs @@ -1,3 +1,5 @@ +using System.Net; + namespace McpServer.Support.Mcp.Web; /// @@ -6,12 +8,12 @@ namespace McpServer.Support.Mcp.Web; /// internal static class PairingHtml { - /// Renders the login form. Shows an error banner when is true. - public static string LoginPage(bool error = false) + /// Renders the login form. Shows an error banner when is not empty. + public static string LoginPage(string? errorMessage = null) { - var errorBanner = error - ? "
Invalid username or password.
" - : ""; + var errorBanner = string.IsNullOrWhiteSpace(errorMessage) + ? string.Empty + : $"
{WebUtility.HtmlEncode(errorMessage)}
"; return $$""" diff --git a/src/McpServer.Support.Mcp/Web/PairingHtmlRenderer.cs b/src/McpServer.Support.Mcp/Web/PairingHtmlRenderer.cs index f853f7fb..4a17da86 100644 --- a/src/McpServer.Support.Mcp/Web/PairingHtmlRenderer.cs +++ b/src/McpServer.Support.Mcp/Web/PairingHtmlRenderer.cs @@ -1,3 +1,5 @@ +using System.Net; + namespace McpServer.Support.Mcp.Web; /// @@ -26,12 +28,12 @@ public PairingHtmlRenderer(Services.IPromptTemplateService templateService, ILog _logger = logger; } - /// Renders the login form page. Shows an error banner when is true. - public async Task RenderLoginPageAsync(bool error = false, CancellationToken cancellationToken = default) + /// Renders the login form page. Shows an error banner when is not empty. + public async Task RenderLoginPageAsync(string? errorMessage = null, CancellationToken cancellationToken = default) { - var errorBanner = error - ? "
Invalid username or password.
" - : ""; + var errorBanner = string.IsNullOrWhiteSpace(errorMessage) + ? string.Empty + : $"
{WebUtility.HtmlEncode(errorMessage)}
"; var template = await GetTemplateContentAsync(LoginPageId, cancellationToken).ConfigureAwait(false); if (template is not null) @@ -39,7 +41,7 @@ public async Task RenderLoginPageAsync(bool error = false, CancellationT return template.Replace("{errorBanner}", errorBanner, StringComparison.Ordinal); } - return PairingHtml.LoginPage(error); + return PairingHtml.LoginPage(errorMessage); } /// Renders the API key display page. diff --git a/src/McpServer.Support.Mcp/appsettings.yaml b/src/McpServer.Support.Mcp/appsettings.yaml index 9b2d2b27..be3f9617 100644 --- a/src/McpServer.Support.Mcp/appsettings.yaml +++ b/src/McpServer.Support.Mcp/appsettings.yaml @@ -47,6 +47,10 @@ Mcp: RepoAllowlist: - src/McpServer.Cqrs/**/*.cs - src/McpServer.Cqrs.Mvvm/**/*.cs + DesktopLaunch: + Enabled: false + AccessToken: '' + AllowedExecutables: [] GraphRag: Enabled: true EnhanceContextSearch: true @@ -109,8 +113,8 @@ Mcp: Url: http://localhost:8000 IngestUrl: http://localhost:8000/api/v1/ingest StreamName: mcp - Username: admin - Password: admin + Username: '' + Password: '' FallbackLogPath: logs/mcp-.log PairingUsers: [] Workspaces: diff --git a/tests/McpServer.Client.Tests/DesktopClientTests.cs b/tests/McpServer.Client.Tests/DesktopClientTests.cs index 7d554d5f..e6a60327 100644 --- a/tests/McpServer.Client.Tests/DesktopClientTests.cs +++ b/tests/McpServer.Client.Tests/DesktopClientTests.cs @@ -59,4 +59,31 @@ public async Task LaunchAsync_PostsStructuredDesktopLaunchRequest() Assert.Contains("\"windowStyle\":\"Hidden\"", handler.LastRequestBody!); Assert.Contains("\"waitForExit\":true", handler.LastRequestBody!); } + + /// + /// FR-MCP-047/TR-MCP-DESKTOP-001: Verifies that forwards the + /// optional privileged desktop-launch token as the dedicated HTTP header for the desktop + /// endpoint only. + /// The test uses the existing so the outbound headers can be + /// inspected without contacting a live MCP server. + /// + [Fact] + public async Task LaunchAsync_WhenDesktopLaunchTokenConfigured_AddsDesktopLaunchHeader() + { + var handler = new MockHttpHandler(HttpStatusCode.OK, """{"success":true,"processId":4242,"exitCode":0}"""); + using var http = new HttpClient(handler); + var client = new DesktopClient( + http, + new McpServerClientOptions + { + ApiKey = "test-key", + BaseUrl = new Uri("http://localhost:7147"), + DesktopLaunchToken = "desktop-secret" + }); + + await client.LaunchAsync(new DesktopLaunchRequest { ExecutablePath = @"C:\Windows\System32\cmd.exe" }); + + Assert.True(handler.LastRequest!.Headers.TryGetValues("X-Desktop-Launch-Token", out var values)); + Assert.Equal("desktop-secret", Assert.Single(values)); + } } diff --git a/tests/McpServer.Client.Tests/TodoClientTests.cs b/tests/McpServer.Client.Tests/TodoClientTests.cs index 3b4ac346..2f90676c 100644 --- a/tests/McpServer.Client.Tests/TodoClientTests.cs +++ b/tests/McpServer.Client.Tests/TodoClientTests.cs @@ -91,6 +91,69 @@ public async System.Threading.Tasks.Task GetAuditAsync_SendsCorrectUrl() Assert.Equal("Before", result.Entries[0].PreviousSnapshot?.Title); } + [Fact] + public async System.Threading.Tasks.Task GetProjectionStatusAsync_SendsCorrectUrl() + { + var handler = new MockHttpHandler( + HttpStatusCode.OK, + """ + { + "authoritativeStore": "sqlite", + "authoritativeDataSource": "E:\\todo.db", + "projectionTargetPath": "E:\\docs\\Project\\TODO.yaml", + "projectionTargetExists": true, + "projectionConsistent": true, + "repairRequired": false, + "verifiedAtUtc": "2026-03-21T00:00:00Z", + "lastProjectedToYamlUtc": "2026-03-21T00:00:00Z", + "message": "TODO.yaml matches authoritative SQLite state." + } + """); + using var http = new HttpClient(handler); + var client = new TodoClient(http, DefaultOptions); + + var result = await client.GetProjectionStatusAsync(); + + Assert.Equal("sqlite", result.AuthoritativeStore); + Assert.True(result.ProjectionConsistent); + Assert.False(result.RepairRequired); + Assert.Equal(HttpMethod.Get, handler.LastRequest!.Method); + Assert.Contains("/mcpserver/todo/projection/status", handler.LastRequest.RequestUri!.AbsolutePath); + } + + [Fact] + public async System.Threading.Tasks.Task RepairProjectionAsync_PostsCorrectUrl() + { + var handler = new MockHttpHandler( + HttpStatusCode.OK, + """ + { + "success": true, + "error": null, + "status": { + "authoritativeStore": "sqlite", + "authoritativeDataSource": "E:\\todo.db", + "projectionTargetPath": "E:\\docs\\Project\\TODO.yaml", + "projectionTargetExists": true, + "projectionConsistent": true, + "repairRequired": false, + "verifiedAtUtc": "2026-03-21T00:01:00Z", + "message": "TODO.yaml matches authoritative SQLite state." + } + } + """); + using var http = new HttpClient(handler); + var client = new TodoClient(http, DefaultOptions); + + var result = await client.RepairProjectionAsync(); + + Assert.True(result.Success); + Assert.NotNull(result.Status); + Assert.True(result.Status.ProjectionConsistent); + Assert.Equal(HttpMethod.Post, handler.LastRequest!.Method); + Assert.Contains("/mcpserver/todo/projection/repair", handler.LastRequest.RequestUri!.AbsolutePath); + } + [Fact] public async System.Threading.Tasks.Task CreateAsync_PostsJsonBody() { diff --git a/tests/McpServer.McpAgent.Tests/McpHostedAgentAdapterTests.cs b/tests/McpServer.McpAgent.Tests/McpHostedAgentAdapterTests.cs index 992f0c57..e52e64a3 100644 --- a/tests/McpServer.McpAgent.Tests/McpHostedAgentAdapterTests.cs +++ b/tests/McpServer.McpAgent.Tests/McpHostedAgentAdapterTests.cs @@ -277,7 +277,7 @@ public async Task Registration_Functions_RepoOperations_ReuseExistingRepoClient( [Fact] public async Task Registration_Functions_DesktopLaunch_ReusesExistingDesktopClient() { - var (hostedAgent, handler) = CreateHostedAgent(); + var (hostedAgent, handler) = CreateHostedAgent("desktop-secret"); var launchFunction = hostedAgent.Registration.Functions.Single(static function => function.Name == "mcp_desktop_launch"); @@ -303,6 +303,7 @@ public async Task Registration_Functions_DesktopLaunch_ReusesExistingDesktopClie Assert.Single(handler.Requests); Assert.Equal(HttpMethod.Post, handler.Requests[0].Method); Assert.Equal("/mcpserver/desktop/launch", handler.Requests[0].RequestUri.AbsolutePath); + Assert.Equal("desktop-secret", handler.Requests[0].DesktopLaunchToken); using var requestBody = JsonDocument.Parse(handler.Requests[0].Body!); Assert.Equal(@"C:\Windows\System32\cmd.exe", requestBody.RootElement.GetProperty("executablePath").GetString()); @@ -416,7 +417,7 @@ public async Task PowerShellSessions_ExecuteInteractiveCommand_PreservesHostLoca Assert.Empty(handler.Requests); } - private static (McpHostedAgent HostedAgent, RecordingMcpHttpMessageHandler Handler) CreateHostedAgent() + private static (McpHostedAgent HostedAgent, RecordingMcpHttpMessageHandler Handler) CreateHostedAgent(string? desktopLaunchToken = null) { var handler = new RecordingMcpHttpMessageHandler(); var httpClient = new HttpClient(handler); @@ -426,6 +427,7 @@ private static (McpHostedAgent HostedAgent, RecordingMcpHttpMessageHandler Handl { ApiKey = "test-key", BaseUrl = new Uri("http://localhost:7147"), + DesktopLaunchToken = desktopLaunchToken, WorkspacePath = @"E:\github\McpServer", }); var timeProvider = new FixedTimeProvider(new DateTimeOffset(2026, 03, 09, 15, 01, 05, TimeSpan.Zero)); @@ -434,6 +436,7 @@ private static (McpHostedAgent HostedAgent, RecordingMcpHttpMessageHandler Handl { ApiKey = "test-key", BaseUrl = new Uri("http://localhost:7147"), + DesktopLaunchToken = desktopLaunchToken, SourceType = "Codex", WorkspacePath = @"E:\github\McpServer", }); @@ -498,7 +501,11 @@ protected override async Task SendAsync(HttpRequestMessage ? null : await request.Content.ReadAsStringAsync(cancellationToken).ConfigureAwait(false); - Requests.Add(new RecordedRequest(request.Method, request.RequestUri!, body)); + var desktopLaunchToken = request.Headers.TryGetValues("X-Desktop-Launch-Token", out var tokenValues) + ? tokenValues.SingleOrDefault() + : null; + + Requests.Add(new RecordedRequest(request.Method, request.RequestUri!, body, desktopLaunchToken)); return request.RequestUri!.AbsolutePath switch { @@ -632,10 +639,12 @@ private static HttpResponseMessage CreateDesktopLaunchResponse() => CreateJsonRe /// The emitted HTTP method. /// The emitted request URI. /// The serialized request body, when present. + /// The privileged desktop-launch header, when present. private sealed record RecordedRequest( HttpMethod Method, Uri RequestUri, - string? Body); + string? Body, + string? DesktopLaunchToken); /// /// TEST-MCP-089: Provides a deterministic clock for the hosted-agent adapter tests. diff --git a/tests/McpServer.Support.Mcp.IntegrationTests/Controllers/ApiKeyEndpointTests.cs b/tests/McpServer.Support.Mcp.IntegrationTests/Controllers/ApiKeyEndpointTests.cs new file mode 100644 index 00000000..776506ab --- /dev/null +++ b/tests/McpServer.Support.Mcp.IntegrationTests/Controllers/ApiKeyEndpointTests.cs @@ -0,0 +1,33 @@ +using System.Net; +using Xunit; + +namespace McpServer.Support.Mcp.IntegrationTests.Controllers; + +/// TR-PLANNED-013: Integration tests for the default API-key issuance endpoint. +public sealed class ApiKeyEndpointTests : IClassFixture +{ + private readonly CustomWebApplicationFactory _factory; + + public ApiKeyEndpointTests(CustomWebApplicationFactory factory) + { + _factory = factory; + } + + /// GET /api-key returns 429 after the fixed window issuance limit is exhausted. + [Fact] + public async Task GetApiKey_AfterPermitLimit_ReturnsTooManyRequests() + { + using var client = _factory.CreateClient(); + + for (var i = 0; i < 30; i++) + { + using var response = await client.GetAsync(new Uri("/api-key", UriKind.Relative)).ConfigureAwait(true); + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + } + + using var throttled = await client.GetAsync(new Uri("/api-key", UriKind.Relative)).ConfigureAwait(true); + Assert.Equal(HttpStatusCode.TooManyRequests, throttled.StatusCode); + Assert.True(throttled.Headers.TryGetValues("Retry-After", out var values)); + Assert.NotEmpty(values); + } +} diff --git a/tests/McpServer.Support.Mcp.IntegrationTests/Controllers/DesktopControllerTests.cs b/tests/McpServer.Support.Mcp.IntegrationTests/Controllers/DesktopControllerTests.cs index 1d81de5f..8b5182be 100644 --- a/tests/McpServer.Support.Mcp.IntegrationTests/Controllers/DesktopControllerTests.cs +++ b/tests/McpServer.Support.Mcp.IntegrationTests/Controllers/DesktopControllerTests.cs @@ -19,6 +19,8 @@ namespace McpServer.Support.Mcp.IntegrationTests.Controllers; /// public sealed class DesktopControllerTests { + private const string DesktopLaunchToken = "desktop-launch-test-token"; + /// /// FR-MCP-047/TR-MCP-DESKTOP-001: Verifies that POST /mcpserver/desktop/launch /// returns the normalized launcher result and forwards the structured launch payload to the @@ -35,7 +37,9 @@ public async Task Launch_ReturnsOkAndNormalizedResult() .Returns(Task.FromResult(new ProcessRunResult(0, """{"success":true,"processId":4242,"exitCode":0}""", null))); var launcherPath = Environment.ProcessPath - ?? Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.System), "cmd.exe"); + ?? throw new InvalidOperationException("Expected the integration test host to expose an executable path."); + var workingDirectory = Path.GetDirectoryName(launcherPath) + ?? throw new InvalidOperationException("Expected a launcher working directory."); using var factory = new CustomWebApplicationFactory( services => @@ -45,19 +49,23 @@ public async Task Launch_ReturnsOkAndNormalizedResult() }, new Dictionary { - ["Mcp:LauncherPath"] = launcherPath + ["Mcp:LauncherPath"] = launcherPath, + ["Mcp:DesktopLaunch:Enabled"] = "true", + ["Mcp:DesktopLaunch:AccessToken"] = DesktopLaunchToken, + ["Mcp:DesktopLaunch:AllowedExecutables:0"] = $"**/{Path.GetFileName(launcherPath)}" }); var client = factory.CreateClient(); TestAuthHelper.AddAuthHeader(client, factory.Services); + client.DefaultRequestHeaders.Add("X-Desktop-Launch-Token", DesktopLaunchToken); var response = await client.PostAsJsonAsync( new Uri("/mcpserver/desktop/launch", UriKind.Relative), new DesktopLaunchRequest { - ExecutablePath = @"C:\Windows\System32\cmd.exe", + ExecutablePath = launcherPath, Arguments = "/c exit 0", - WorkingDirectory = @"C:\Windows\System32", + WorkingDirectory = workingDirectory, EnvironmentVariables = new Dictionary { ["TEST_ENV"] = "true" }, CreateNoWindow = true, WindowStyle = "Hidden", @@ -76,7 +84,7 @@ await processRunner.Received(1).RunAsync( launcherPath, Arg.Is(arguments => arguments != null - && arguments.Contains("cmd.exe", StringComparison.Ordinal) + && arguments.Contains(Path.GetFileName(launcherPath), StringComparison.Ordinal) && arguments.Contains("TEST_ENV", StringComparison.Ordinal) && arguments.Contains("Hidden", StringComparison.Ordinal)), Arg.Any()); @@ -99,4 +107,42 @@ public async Task Launch_WithoutBody_ReturnsBadRequest() Assert.Equal(HttpStatusCode.BadRequest, response.StatusCode); } + + /// + /// FR-MCP-047/TR-MCP-DESKTOP-001: Verifies that the HTTP desktop-launch endpoint rejects + /// authenticated workspace-key callers that do not also present the privileged desktop-launch + /// token header. + /// The test uses the real HTTP pipeline plus a substituted process runner so the stronger + /// authorization tier can be asserted without starting any local desktop program. + /// + [Fact] + public async Task Launch_WithoutDesktopLaunchToken_ReturnsForbidden() + { + var launcherPath = Environment.ProcessPath + ?? throw new InvalidOperationException("Expected the integration test host to expose an executable path."); + var processRunner = Substitute.For(); + using var factory = new CustomWebApplicationFactory( + services => + { + services.RemoveAll(); + services.AddSingleton(processRunner); + }, + new Dictionary + { + ["Mcp:LauncherPath"] = launcherPath, + ["Mcp:DesktopLaunch:Enabled"] = "true", + ["Mcp:DesktopLaunch:AccessToken"] = DesktopLaunchToken, + ["Mcp:DesktopLaunch:AllowedExecutables:0"] = $"**/{Path.GetFileName(launcherPath)}" + }); + + var client = factory.CreateClient(); + TestAuthHelper.AddAuthHeader(client, factory.Services); + + var response = await client.PostAsJsonAsync( + new Uri("/mcpserver/desktop/launch", UriKind.Relative), + new DesktopLaunchRequest { ExecutablePath = launcherPath }).ConfigureAwait(true); + + Assert.Equal(HttpStatusCode.Forbidden, response.StatusCode); + await processRunner.DidNotReceive().RunAsync(Arg.Any(), Arg.Any(), Arg.Any()); + } } diff --git a/tests/McpServer.Support.Mcp.IntegrationTests/Controllers/Http500ErrorContractTests.cs b/tests/McpServer.Support.Mcp.IntegrationTests/Controllers/Http500ErrorContractTests.cs index a8fec020..ef323682 100644 --- a/tests/McpServer.Support.Mcp.IntegrationTests/Controllers/Http500ErrorContractTests.cs +++ b/tests/McpServer.Support.Mcp.IntegrationTests/Controllers/Http500ErrorContractTests.cs @@ -240,6 +240,10 @@ public sealed class PassThroughTodoService : ITodoService public Task GetByIdAsync(string id, CancellationToken cancellationToken) => _inner.GetByIdAsync(id, cancellationToken); public Task GetAuditAsync(string id, int limit = 50, int offset = 0, CancellationToken cancellationToken = default) => _inner.GetAuditAsync(id, limit, offset, cancellationToken); + public Task GetProjectionStatusAsync(CancellationToken cancellationToken = default) + => _inner.GetProjectionStatusAsync(cancellationToken); + public Task RepairProjectionAsync(CancellationToken cancellationToken = default) + => _inner.RepairProjectionAsync(cancellationToken); public Task CreateAsync(TodoCreateRequest request, CancellationToken cancellationToken) => Task.FromResult(new TodoMutationResult(true, null, new TodoFlatItem { Id = request.Id, Title = request.Title, Section = request.Section, Priority = request.Priority, Done = false })); public Task UpdateAsync(string id, TodoUpdateRequest request, CancellationToken cancellationToken) => _inner.UpdateAsync(id, request, cancellationToken); public Task DeleteAsync(string id, CancellationToken cancellationToken) => _inner.DeleteAsync(id, cancellationToken); @@ -261,6 +265,10 @@ public Task QueryAsync(TodoQueryRequest request, CancellationTo public Task GetByIdAsync(string id, CancellationToken cancellationToken) => _inner.GetByIdAsync(id, cancellationToken); public Task GetAuditAsync(string id, int limit = 50, int offset = 0, CancellationToken cancellationToken = default) => _inner.GetAuditAsync(id, limit, offset, cancellationToken); + public Task GetProjectionStatusAsync(CancellationToken cancellationToken = default) + => _inner.GetProjectionStatusAsync(cancellationToken); + public Task RepairProjectionAsync(CancellationToken cancellationToken = default) + => _inner.RepairProjectionAsync(cancellationToken); public Task CreateAsync(TodoCreateRequest request, CancellationToken cancellationToken) => _inner.CreateAsync(request, cancellationToken); public Task UpdateAsync(string id, TodoUpdateRequest request, CancellationToken cancellationToken) => _inner.UpdateAsync(id, request, cancellationToken); public Task DeleteAsync(string id, CancellationToken cancellationToken) diff --git a/tests/McpServer.Support.Mcp.IntegrationTests/Controllers/MarkerRegenerationIntegrationTests.cs b/tests/McpServer.Support.Mcp.IntegrationTests/Controllers/MarkerRegenerationIntegrationTests.cs index 4de683e5..6a8bf98f 100644 --- a/tests/McpServer.Support.Mcp.IntegrationTests/Controllers/MarkerRegenerationIntegrationTests.cs +++ b/tests/McpServer.Support.Mcp.IntegrationTests/Controllers/MarkerRegenerationIntegrationTests.cs @@ -76,7 +76,7 @@ public ValueTask InitializeAsync() // FileSystemWatcher on appsettings.json — fires when config writes complete. _settingsWatcher = new FileSystemWatcher(_workspacePath, "appsettings.json") { - NotifyFilter = NotifyFilters.LastWrite | NotifyFilters.Size, + NotifyFilter = NotifyFilters.LastWrite | NotifyFilters.Size | NotifyFilters.CreationTime | NotifyFilters.FileName, EnableRaisingEvents = true, }; @@ -202,18 +202,28 @@ public async Task GlobalAndWorkspacePrompts_CombineInMarkerFile() /// /// Returns a that completes when appsettings.json is written. - /// The releases the latch on the first event. + /// The releases the latch on the first change/create/rename event so + /// atomic temp-file replace writes are observed deterministically. /// private Task WatchForSettingsChange() { var tcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); FileSystemEventHandler? handler = null; - handler = (_, _) => + RenamedEventHandler? renamedHandler = null; + + void Complete() { _settingsWatcher.Changed -= handler; + _settingsWatcher.Created -= handler; + _settingsWatcher.Renamed -= renamedHandler; tcs.TrySetResult(); - }; + } + + handler = (_, _) => Complete(); + renamedHandler = (_, _) => Complete(); _settingsWatcher.Changed += handler; + _settingsWatcher.Created += handler; + _settingsWatcher.Renamed += renamedHandler; // Guard against the write completing before the handler was attached. var cts = new CancellationTokenSource(TimeSpan.FromSeconds(10)); diff --git a/tests/McpServer.Support.Mcp.IntegrationTests/Controllers/McpTransportMultiTenantTests.cs b/tests/McpServer.Support.Mcp.IntegrationTests/Controllers/McpTransportMultiTenantTests.cs index 4ed98c52..9298bbc9 100644 --- a/tests/McpServer.Support.Mcp.IntegrationTests/Controllers/McpTransportMultiTenantTests.cs +++ b/tests/McpServer.Support.Mcp.IntegrationTests/Controllers/McpTransportMultiTenantTests.cs @@ -81,4 +81,34 @@ public async Task McpTransport_WithoutWorkspaceHeader_UsesDefaultWorkspace() var body = await response.Content.ReadAsStringAsync(); Assert.Contains("serverInfo", body, StringComparison.Ordinal); } + + [Fact] + public async Task McpTransport_BearerWithoutWorkspaceHeader_Returns404() + { + var initRequest = new + { + jsonrpc = "2.0", + id = 1, + method = "initialize", + @params = new + { + protocolVersion = "2025-03-26", + capabilities = new { }, + clientInfo = new { name = "test-client-bearer", version = "1.0.0" } + } + }; + + using var request = new HttpRequestMessage(HttpMethod.Post, "/mcp-transport"); + request.Content = new StringContent( + JsonSerializer.Serialize(initRequest), + Encoding.UTF8, + "application/json"); + request.Headers.Accept.Add(new System.Net.Http.Headers.MediaTypeWithQualityHeaderValue("application/json")); + request.Headers.Accept.Add(new System.Net.Http.Headers.MediaTypeWithQualityHeaderValue("text/event-stream")); + request.Headers.Authorization = new System.Net.Http.Headers.AuthenticationHeaderValue("Bearer", "synthetic-jwt"); + + var response = await _client.SendAsync(request); + + Assert.Equal(HttpStatusCode.NotFound, response.StatusCode); + } } diff --git a/tests/McpServer.Support.Mcp.IntegrationTests/Controllers/MultiTenantIntegrationTests.cs b/tests/McpServer.Support.Mcp.IntegrationTests/Controllers/MultiTenantIntegrationTests.cs index f11c4de7..700420b1 100644 --- a/tests/McpServer.Support.Mcp.IntegrationTests/Controllers/MultiTenantIntegrationTests.cs +++ b/tests/McpServer.Support.Mcp.IntegrationTests/Controllers/MultiTenantIntegrationTests.cs @@ -119,6 +119,17 @@ public async Task NoHeaders_NoApiKey_Returns401() } } + [Fact] + public async Task BearerWithoutWorkspaceHeader_OnTenantRoute_Returns404() + { + var client = _factory.CreateClient(); + client.DefaultRequestHeaders.Authorization = new System.Net.Http.Headers.AuthenticationHeaderValue("Bearer", "synthetic-jwt"); + + var response = await client.GetAsync(new Uri("/mcpserver/sessionlog/query", UriKind.Relative)); + + Assert.Equal(HttpStatusCode.NotFound, response.StatusCode); + } + [Fact] public async Task WorkspaceResolutionMiddleware_OnlyRunsForMcpRoutes() { diff --git a/tests/McpServer.Support.Mcp.IntegrationTests/Controllers/PairingEndpointTests.cs b/tests/McpServer.Support.Mcp.IntegrationTests/Controllers/PairingEndpointTests.cs index b8cc0801..2b460b3f 100644 --- a/tests/McpServer.Support.Mcp.IntegrationTests/Controllers/PairingEndpointTests.cs +++ b/tests/McpServer.Support.Mcp.IntegrationTests/Controllers/PairingEndpointTests.cs @@ -44,6 +44,43 @@ public async Task PairPost_WithBadCredentials_ShowsError() Assert.Contains("Invalid username or password", body, StringComparison.Ordinal); } + [Fact] + public async Task PairPost_AfterRepeatedFailures_ReturnsTooManyRequests() + { + await using var factory = new PairingWebApplicationFactory(); + using var client = factory.CreateClient(new Microsoft.AspNetCore.Mvc.Testing.WebApplicationFactoryClientOptions + { + AllowAutoRedirect = false, + }); + + for (var i = 0; i < 5; i++) + { + using var failedAttempt = new FormUrlEncodedContent( + [ + new KeyValuePair("username", "admin"), + new KeyValuePair("password", "wrong"), + ]); + + var failedResponse = await client.PostAsync("/pair", failedAttempt).ConfigureAwait(true); + Assert.Equal(HttpStatusCode.OK, failedResponse.StatusCode); + } + + using var lockedAttempt = new FormUrlEncodedContent( + [ + new KeyValuePair("username", "admin"), + new KeyValuePair("password", "testpass"), + ]); + + var response = await client.PostAsync("/pair", lockedAttempt).ConfigureAwait(true); + Assert.Equal(HttpStatusCode.TooManyRequests, response.StatusCode); + Assert.True(response.Headers.TryGetValues("Retry-After", out var retryAfterValues)); + Assert.NotEmpty(retryAfterValues); + + var body = await response.Content.ReadAsStringAsync().ConfigureAwait(true); + Assert.Contains("Too many failed sign-in attempts", body, StringComparison.Ordinal); + Assert.False(response.Headers.TryGetValues("Set-Cookie", out _)); + } + [Fact] public async Task PairPost_WithGoodCredentials_RedirectsToKey() { diff --git a/tests/McpServer.Support.Mcp.IntegrationTests/Controllers/TodoControllerTests.cs b/tests/McpServer.Support.Mcp.IntegrationTests/Controllers/TodoControllerTests.cs index f7674ff6..358650ec 100644 --- a/tests/McpServer.Support.Mcp.IntegrationTests/Controllers/TodoControllerTests.cs +++ b/tests/McpServer.Support.Mcp.IntegrationTests/Controllers/TodoControllerTests.cs @@ -157,6 +157,29 @@ public async Task Create_ThenGetById_ReturnsCreatedItem() Assert.Equal("Remaining from create", item.Remaining); } + /// POST /mcpserver/todo with the default /api-key token returns 403 Forbidden. + [Fact] + public async Task Create_WithDefaultApiKey_ReturnsForbidden() + { + using var client = _factory.CreateClient(); + var tokenService = _factory.Services.GetRequiredService(); + var config = _factory.Services.GetRequiredService(); + var defaultToken = tokenService.GetDefaultToken(config["Mcp:RepoRoot"]!) + ?? throw new InvalidOperationException("Workspace default API key was not generated for test host."); + client.DefaultRequestHeaders.TryAddWithoutValidation("X-Api-Key", defaultToken); + + var createRequest = new + { + id = "DEFAULT-TODO-001", + title = "Default key write should fail", + section = "mvp-app", + priority = "low" + }; + + var response = await client.PostAsJsonAsync(new Uri("/mcpserver/todo", UriKind.Relative), createRequest).ConfigureAwait(true); + Assert.Equal(HttpStatusCode.Forbidden, response.StatusCode); + } + /// POST /mcpserver/todo with duplicate id returns 400 Bad Request. [Fact] public async Task Create_DuplicateId_ReturnsConflict() @@ -268,12 +291,90 @@ public async Task AuditEndpoint_AfterCreateUpdateDelete_ReturnsOrderedHistory() Assert.Equal("Audit item", entry.PreviousSnapshot?.Title); }, entry => + { + Assert.Equal(3, entry.Version); + Assert.Equal("deleted", entry.Action); + Assert.Equal("Audit item updated", entry.Snapshot?.Title); + Assert.Equal("Audit item updated", entry.PreviousSnapshot?.Title); + }); + } + + /// + /// TR-MCP-TODO-006: Verifies that a real mutation-time projection failure remains visible through the + /// REST API while the SQLite-authoritative item is still committed and the repair endpoint can rebuild + /// TODO.yaml afterward. The fixture temporarily replaces the projected TODO.yaml file with a directory + /// so create fails deterministically without compromising the authoritative SQLite store. + /// + [Fact] + public async Task ProjectionEndpoints_AfterMutationProjectionFailure_ReportAndRepairAuthoritativeState() + { + var yamlPath = _factory.TodoYamlPath; + Assert.True(File.Exists(yamlPath)); + + File.Delete(yamlPath); + Directory.CreateDirectory(yamlPath); + + try + { + var createRequest = new { - Assert.Equal(3, entry.Version); - Assert.Equal("deleted", entry.Action); - Assert.Equal("Audit item updated", entry.Snapshot?.Title); - Assert.Equal("Audit item updated", entry.PreviousSnapshot?.Title); - }); + id = "INT-PROJ-001", + title = "Projection failure integration", + section = "mvp-app", + priority = "high" + }; + + var createResponse = await _client.PostAsJsonAsync(new Uri("/mcpserver/todo", UriKind.Relative), createRequest).ConfigureAwait(true); + Assert.Equal(HttpStatusCode.InternalServerError, createResponse.StatusCode); + + var createResult = await createResponse.Content.ReadFromJsonAsync().ConfigureAwait(true); + Assert.NotNull(createResult); + Assert.False(createResult.Success); + Assert.Equal("ProjectionFailed", createResult.FailureKind); + + var getResponse = await _client.GetAsync(new Uri("/mcpserver/todo/INT-PROJ-001", UriKind.Relative)).ConfigureAwait(true); + Assert.Equal(HttpStatusCode.OK, getResponse.StatusCode); + + var statusResponse = await _client.GetAsync(new Uri("/mcpserver/todo/projection/status", UriKind.Relative)).ConfigureAwait(true); + Assert.Equal(HttpStatusCode.OK, statusResponse.StatusCode); + + var status = await statusResponse.Content.ReadFromJsonAsync().ConfigureAwait(true); + Assert.NotNull(status); + Assert.True(status.RepairRequired); + Assert.False(status.ProjectionTargetExists); + Assert.False(status.ProjectionConsistent); + Assert.Contains("directory", status.Message ?? string.Empty, StringComparison.OrdinalIgnoreCase); + + Directory.Delete(yamlPath, recursive: true); + + var repairResponse = await _client.PostAsync(new Uri("/mcpserver/todo/projection/repair", UriKind.Relative), content: null).ConfigureAwait(true); + Assert.Equal(HttpStatusCode.OK, repairResponse.StatusCode); + + var repair = await repairResponse.Content.ReadFromJsonAsync().ConfigureAwait(true); + Assert.NotNull(repair); + Assert.True(repair.Success); + Assert.NotNull(repair.Status); + Assert.False(repair.Status.RepairRequired); + Assert.True(repair.Status.ProjectionTargetExists); + Assert.True(repair.Status.ProjectionConsistent); + Assert.True(File.Exists(yamlPath)); + + var repairedStatusResponse = await _client.GetAsync(new Uri("/mcpserver/todo/projection/status", UriKind.Relative)).ConfigureAwait(true); + Assert.Equal(HttpStatusCode.OK, repairedStatusResponse.StatusCode); + + var repairedStatus = await repairedStatusResponse.Content.ReadFromJsonAsync().ConfigureAwait(true); + Assert.NotNull(repairedStatus); + Assert.False(repairedStatus.RepairRequired); + Assert.True(repairedStatus.ProjectionConsistent); + } + finally + { + if (Directory.Exists(yamlPath)) + Directory.Delete(yamlPath, recursive: true); + + if (!File.Exists(yamlPath)) + await _client.PostAsync(new Uri("/mcpserver/todo/projection/repair", UriKind.Relative), content: null).ConfigureAwait(true); + } } /// DELETE /mcpserver/todo/{id} for missing item returns 404. @@ -480,7 +581,21 @@ public async Task Update_WithCircularDependency_ReturnsNotFound() Assert.Contains("Circular", result.Error ?? "", StringComparison.OrdinalIgnoreCase); } - private sealed record MutationResult(bool Success, string? Error); + private sealed record MutationResult(bool Success, string? Error, string? FailureKind = null); + private sealed record ProjectionStatusResult( + string AuthoritativeStore, + string AuthoritativeDataSource, + string ProjectionTargetPath, + bool ProjectionTargetExists, + bool ProjectionConsistent, + bool RepairRequired, + string VerifiedAtUtc, + string? LastImportedFromYamlUtc, + string? LastProjectedToYamlUtc, + string? LastProjectionFailureUtc, + string? LastProjectionFailure, + string? Message); + private sealed record ProjectionRepairResult(bool Success, string? Error, ProjectionStatusResult Status); #region Test DTOs (for deserialization) @@ -516,12 +631,14 @@ public sealed class TodoWebFactory : WebApplicationFactory, ID { private readonly string _tempDir = Path.Combine(Path.GetTempPath(), "mcp-todo-tests-" + Guid.NewGuid().ToString("N")[..8]); + public string TodoYamlPath => Path.Combine(_tempDir, "docs", "Project", "TODO.yaml"); + protected override void ConfigureWebHost(IWebHostBuilder builder) { // Create seed TODO.yaml var projectDir = Path.Combine(_tempDir, "docs", "Project"); Directory.CreateDirectory(projectDir); - File.WriteAllText(Path.Combine(projectDir, "TODO.yaml"), SeedYaml); + File.WriteAllText(TodoYamlPath, SeedYaml); builder.UseEnvironment("Test"); builder.UseContentRoot(CustomWebApplicationFactory.ResolveContentRoot()); diff --git a/tests/McpServer.Support.Mcp.Tests/Controllers/GitHubControllerTests.cs b/tests/McpServer.Support.Mcp.Tests/Controllers/GitHubControllerTests.cs new file mode 100644 index 00000000..e0a2abe8 --- /dev/null +++ b/tests/McpServer.Support.Mcp.Tests/Controllers/GitHubControllerTests.cs @@ -0,0 +1,89 @@ +using McpServer.Support.Mcp.Controllers; +using McpServer.Support.Mcp.Options; +using McpServer.Support.Mcp.Services; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Mvc; +using Microsoft.Extensions.Logging.Abstractions; +using Microsoft.Extensions.Options; +using NSubstitute; +using Xunit; + +namespace McpServer.Support.Mcp.Tests.Controllers; + +/// +/// TEST-MCP-GH-006: Validates GitHub controller boundary hardening for state and close-reason query +/// parameters so only canonical values reach and the gh CLI layer. +/// +public sealed class GitHubControllerTests +{ + /// + /// TEST-MCP-GH-006: Verifies that invalid close reasons are rejected at the controller boundary before + /// the GitHub CLI service is invoked, preventing query-string flag injection in issue-close requests. + /// + [Fact] + public async Task CloseIssueAsync_WithInvalidReason_ReturnsBadRequest() + { + var gitHubCliService = Substitute.For(); + var controller = CreateController(gitHubCliService); + + var result = await controller.CloseIssueAsync(42, "completed --repo other/repo", CancellationToken.None).ConfigureAwait(true); + + Assert.IsType(result.Result); + await gitHubCliService.DidNotReceive().CloseIssueAsync(Arg.Any(), Arg.Any(), Arg.Any()).ConfigureAwait(true); + } + + /// + /// TEST-MCP-GH-006: Verifies that list-state query parameters are normalized to canonical lowercase + /// values before forwarding them to the GitHub CLI service, preventing raw user input from reaching gh. + /// + [Fact] + public async Task ListIssuesAsync_WithMixedCaseState_NormalizesBeforeCallingService() + { + var gitHubCliService = Substitute.For(); + gitHubCliService.ListIssuesAsync("open", 30, Arg.Any()) + .Returns(new GitHubIssueListResult(true, null, Array.Empty())); + + var controller = CreateController(gitHubCliService); + var result = await controller.ListIssuesAsync(" Open ", 30, CancellationToken.None).ConfigureAwait(true); + + Assert.IsType(result.Result); + await gitHubCliService.Received(1).ListIssuesAsync("open", 30, Arg.Any()).ConfigureAwait(true); + } + + /// + /// TEST-MCP-GH-006: Verifies that invalid list-state query parameters are rejected before the GitHub CLI + /// service is invoked, blocking raw multi-token input from being incorporated into gh list commands. + /// + [Fact] + public async Task ListPullsAsync_WithInvalidState_ReturnsBadRequest() + { + var gitHubCliService = Substitute.For(); + var controller = CreateController(gitHubCliService); + + var result = await controller.ListPullsAsync("open --repo other/repo", 30, CancellationToken.None).ConfigureAwait(true); + + Assert.IsType(result.Result); + await gitHubCliService.DidNotReceive().ListPullsAsync(Arg.Any(), Arg.Any(), Arg.Any()).ConfigureAwait(true); + } + + private static GitHubController CreateController(IGitHubCliService gitHubCliService) + { + var tokenStore = Substitute.For(); + var gitHubOptions = Substitute.For>(); + gitHubOptions.CurrentValue.Returns(new GitHubIntegrationOptions()); + + return new GitHubController( + gitHubCliService, + tokenStore, + gitHubOptions, + syncService: null, + eventBus: null, + logger: NullLogger.Instance) + { + ControllerContext = new ControllerContext + { + HttpContext = new DefaultHttpContext() + } + }; + } +} diff --git a/tests/McpServer.Support.Mcp.Tests/Controllers/TodoControllerTests.cs b/tests/McpServer.Support.Mcp.Tests/Controllers/TodoControllerTests.cs index 1862cc88..6a489b1a 100644 --- a/tests/McpServer.Support.Mcp.Tests/Controllers/TodoControllerTests.cs +++ b/tests/McpServer.Support.Mcp.Tests/Controllers/TodoControllerTests.cs @@ -167,6 +167,90 @@ public async Task DeleteAsync_WhenProjectionFails_ReturnsInternalServerError() Assert.Equal(TodoMutationFailureKind.ProjectionFailed, mutation.FailureKind); } + /// + /// TR-MCP-TODO-006: Verifies that GET /mcpserver/todo/projection/status returns the service-provided + /// projection status payload when the active TODO provider supports SQLite projection diagnostics. + /// The fixture supplies a fully-populated status result so the controller's success shaping can be asserted. + /// + [Fact] + public async Task GetProjectionStatusAsync_WhenSupported_ReturnsOk() + { + var todoService = Substitute.For(); + todoService.GetProjectionStatusAsync(Arg.Any()) + .Returns(new TodoProjectionStatusResult( + "sqlite", + "E:\\todo.db", + "E:\\docs\\Project\\TODO.yaml", + true, + true, + false, + "2026-03-21T00:00:00.0000000Z", + LastProjectedToYamlUtc: "2026-03-21T00:00:00.0000000Z", + Message: "TODO.yaml matches authoritative SQLite state.")); + + var controller = CreateController(todoService); + var actionResult = await controller.GetProjectionStatusAsync(CancellationToken.None).ConfigureAwait(true); + + var ok = Assert.IsType(actionResult.Result); + var status = Assert.IsType(ok.Value); + Assert.False(status.RepairRequired); + Assert.True(status.ProjectionConsistent); + } + + /// + /// TR-MCP-TODO-006: Verifies that GET /mcpserver/todo/projection/status returns 501 when the active TODO + /// provider does not support SQLite projection diagnostics. The fixture uses a thrown + /// to exercise the controller's compatibility path. + /// + [Fact] + public async Task GetProjectionStatusAsync_WhenNotSupported_ReturnsNotImplemented() + { + var todoService = Substitute.For(); + todoService.GetProjectionStatusAsync(Arg.Any()) + .Returns(_ => Task.FromException( + new NotSupportedException("Projection status requires sqlite-backed TODO storage."))); + + var controller = CreateController(todoService); + var actionResult = await controller.GetProjectionStatusAsync(CancellationToken.None).ConfigureAwait(true); + + var objectResult = Assert.IsType(actionResult.Result); + Assert.Equal(StatusCodes.Status501NotImplemented, objectResult.StatusCode); + } + + /// + /// TR-MCP-TODO-006: Verifies that POST /mcpserver/todo/projection/repair returns HTTP 500 when the + /// service reports an unsuccessful repair attempt. The fixture returns a failed repair result so the + /// controller can preserve the service's operator-visible error details. + /// + [Fact] + public async Task RepairProjectionAsync_WhenRepairFails_ReturnsInternalServerError() + { + var todoService = Substitute.For(); + todoService.RepairProjectionAsync(Arg.Any()) + .Returns(new TodoProjectionRepairResult( + false, + "repair failed", + new TodoProjectionStatusResult( + "sqlite", + "E:\\todo.db", + "E:\\docs\\Project\\TODO.yaml", + false, + false, + true, + "2026-03-21T00:00:00.0000000Z", + LastProjectionFailure: "Directory exists at projection target.", + Message: "Projected TODO target 'E:\\docs\\Project\\TODO.yaml' is a directory instead of a file."))); + + var controller = CreateController(todoService); + var actionResult = await controller.RepairProjectionAsync(CancellationToken.None).ConfigureAwait(true); + + var objectResult = Assert.IsType(actionResult.Result); + Assert.Equal(StatusCodes.Status500InternalServerError, objectResult.StatusCode); + var repair = Assert.IsType(objectResult.Value); + Assert.True(repair.Status.RepairRequired); + Assert.Equal("repair failed", repair.Error); + } + private static TodoController CreateController( ITodoService todoService, IGitHubCliService? gitHubCliService = null, diff --git a/tests/McpServer.Support.Mcp.Tests/Middleware/InteractionLoggingMiddlewareTests.cs b/tests/McpServer.Support.Mcp.Tests/Middleware/InteractionLoggingMiddlewareTests.cs index 63a6c88f..78556ad6 100644 --- a/tests/McpServer.Support.Mcp.Tests/Middleware/InteractionLoggingMiddlewareTests.cs +++ b/tests/McpServer.Support.Mcp.Tests/Middleware/InteractionLoggingMiddlewareTests.cs @@ -2,6 +2,7 @@ using McpServer.Support.Mcp.Models; using McpServer.Support.Mcp.Options; using McpServer.Support.Mcp.Services; +using McpServer.Support.Mcp.Tests.TestSupport; using Microsoft.AspNetCore.Http; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; @@ -291,4 +292,32 @@ public async Task InvokeAsync_ResponseBody_StillWrittenToOriginalStream() var writtenContent = await reader.ReadToEndAsync().ConfigureAwait(true); Assert.Equal(responseJson, writtenContent); } + + /// InvokeAsync logs a warning when the submission queue rejects an interaction log entry because the buffer is full. + [Fact] + public async Task InvokeAsync_WhenQueueRejectsEntry_LogsWarning() + { + RequestDelegate next = _ => Task.CompletedTask; + var logger = new TestLogger(); + var options = Microsoft.Extensions.Options.Options.Create(new McpInteractionLoggingOptions + { + LoggingServiceUrl = "https://log.example.com/ingest", + IncludeRequestBody = false, + IncludeResponseBody = false + }); + var channel = Substitute.For(); + channel.TryEnqueue(Arg.Any()).Returns(false); + + var middleware = new McpServer.Support.Mcp.Middleware.InteractionLoggingMiddleware(next, logger, options, channel); + var context = CreateContext("POST", "/mcpserver/context/search"); + context.Response.StatusCode = 202; + + await middleware.InvokeAsync(context).ConfigureAwait(true); + + Assert.Contains( + logger.Entries, + entry => entry.Level == LogLevel.Warning && + entry.Message.Contains("Interaction log forwarding rejected request", StringComparison.Ordinal) && + entry.Message.Contains("/mcpserver/context/search", StringComparison.Ordinal)); + } } diff --git a/tests/McpServer.Support.Mcp.Tests/Middleware/WorkspaceAuthMiddlewareTests.cs b/tests/McpServer.Support.Mcp.Tests/Middleware/WorkspaceAuthMiddlewareTests.cs index 1e0b9e32..5352888a 100644 --- a/tests/McpServer.Support.Mcp.Tests/Middleware/WorkspaceAuthMiddlewareTests.cs +++ b/tests/McpServer.Support.Mcp.Tests/Middleware/WorkspaceAuthMiddlewareTests.cs @@ -7,7 +7,7 @@ namespace McpServer.Support.Mcp.Tests.Middleware; -/// Unit tests for default key behavior. +/// Unit tests for default-key behavior. public sealed class WorkspaceAuthMiddlewareTests { private const string WorkspacePath = @"C:\projects\test"; @@ -78,7 +78,7 @@ public async Task DefaultToken_AllowsReadOnNonTodoRoute() } [Fact] - public async Task DefaultToken_AllowsWriteOnTodoRoute() + public async Task DefaultToken_DeniesWriteOnTodoRoute() { var tokenService = CreateTokenService(); var defaultToken = tokenService.GetDefaultToken(WorkspacePath)!; @@ -88,7 +88,8 @@ public async Task DefaultToken_AllowsWriteOnTodoRoute() await middleware.InvokeAsync(ctx, tokenService, CreateConfig(), CreateWorkspaceContext()); - Assert.True(nextCalled); + Assert.False(nextCalled); + Assert.Equal(403, ctx.Response.StatusCode); } [Fact] @@ -122,7 +123,7 @@ public async Task DefaultToken_DeniesDeleteOnNonTodoRoute() } [Fact] - public async Task DefaultToken_AllowsDeleteOnTodoRoute() + public async Task DefaultToken_DeniesDeleteOnTodoRoute() { var tokenService = CreateTokenService(); var defaultToken = tokenService.GetDefaultToken(WorkspacePath)!; @@ -132,7 +133,8 @@ public async Task DefaultToken_AllowsDeleteOnTodoRoute() await middleware.InvokeAsync(ctx, tokenService, CreateConfig(), CreateWorkspaceContext()); - Assert.True(nextCalled); + Assert.False(nextCalled); + Assert.Equal(403, ctx.Response.StatusCode); } [Fact] @@ -184,9 +186,8 @@ public async Task ReadsWorkspaceFromContext_InsteadOfConfig() } [Fact] - public async Task EmptyApiKey_Config_PassesAll() + public async Task MissingWorkspaceToken_Returns503() { - // When Mcp:ApiKey is empty, auth is open mode — all requests pass. var tokenService = new WorkspaceTokenService(); var config = new ConfigurationBuilder() .AddInMemoryCollection(new Dictionary @@ -202,6 +203,25 @@ public async Task EmptyApiKey_Config_PassesAll() await middleware.InvokeAsync(ctx, tokenService, config, wsContext); - Assert.True(nextCalled); + Assert.False(nextCalled); + Assert.Equal(503, ctx.Response.StatusCode); + } + + [Fact] + public async Task EmptyWorkspaceContext_Returns503() + { + var tokenService = new WorkspaceTokenService(); + var config = new ConfigurationBuilder() + .AddInMemoryCollection(new Dictionary()) + .Build(); + var wsContext = new WorkspaceContext(); + var nextCalled = false; + var middleware = new WorkspaceAuthMiddleware(_ => { nextCalled = true; return Task.CompletedTask; }, NullLogger.Instance); + var ctx = CreateContext("GET", "/mcpserver/context/search", null); + + await middleware.InvokeAsync(ctx, tokenService, config, wsContext); + + Assert.False(nextCalled); + Assert.Equal(503, ctx.Response.StatusCode); } } diff --git a/tests/McpServer.Support.Mcp.Tests/Middleware/WorkspaceResolutionMiddlewareTests.cs b/tests/McpServer.Support.Mcp.Tests/Middleware/WorkspaceResolutionMiddlewareTests.cs index 933f6f57..21edcc79 100644 --- a/tests/McpServer.Support.Mcp.Tests/Middleware/WorkspaceResolutionMiddlewareTests.cs +++ b/tests/McpServer.Support.Mcp.Tests/Middleware/WorkspaceResolutionMiddlewareTests.cs @@ -46,7 +46,7 @@ private static IWorkspaceService CreateWorkspaceService(params WorkspaceDto[] wo return svc; } - private static DefaultHttpContext CreateContext(string path, string method = "GET", string? workspaceHeader = null, string? apiKey = null) + private static DefaultHttpContext CreateContext(string path, string method = "GET", string? workspaceHeader = null, string? apiKey = null, string? bearerToken = null) { var ctx = new DefaultHttpContext { @@ -57,6 +57,8 @@ private static DefaultHttpContext CreateContext(string path, string method = "GE ctx.Request.Headers[WorkspaceResolutionMiddleware.WorkspacePathHeader] = workspaceHeader; if (apiKey is not null) ctx.Request.Headers["X-Api-Key"] = apiKey; + if (bearerToken is not null) + ctx.Request.Headers.Authorization = $"Bearer {bearerToken}"; return ctx; } @@ -236,4 +238,39 @@ public async Task DefaultToken_SetsIsDefaultKey() Assert.True(wsContext.IsResolved); Assert.True(wsContext.IsDefaultKey); } + + [Fact] + public async Task BearerToken_WithoutWorkspaceHeader_RejectsTenantRoute() + { + var wsDto = MakeDto(WorkspaceA, isPrimary: true); + var workspaceService = CreateWorkspaceService(wsDto); + var tokenService = new WorkspaceTokenService(); + var wsContext = new WorkspaceContext(); + var nextCalled = false; + var mw = CreateMiddleware(_ => { nextCalled = true; return Task.CompletedTask; }); + + var ctx = CreateContext("/mcpserver/sessionlog/query", bearerToken: "jwt-token"); + await mw.InvokeAsync(ctx, wsContext, tokenService, workspaceService); + + Assert.False(nextCalled); + Assert.Equal(404, ctx.Response.StatusCode); + Assert.False(wsContext.IsResolved); + } + + [Fact] + public async Task BearerToken_WithoutWorkspaceHeader_AllowsWorkspaceRegistryRoute() + { + var wsDto = MakeDto(WorkspaceA, isPrimary: true); + var workspaceService = CreateWorkspaceService(wsDto); + var tokenService = new WorkspaceTokenService(); + var wsContext = new WorkspaceContext(); + var nextCalled = false; + var mw = CreateMiddleware(_ => { nextCalled = true; return Task.CompletedTask; }); + + var ctx = CreateContext("/mcpserver/workspace", bearerToken: "jwt-token"); + await mw.InvokeAsync(ctx, wsContext, tokenService, workspaceService); + + Assert.True(nextCalled); + Assert.False(wsContext.IsResolved); + } } diff --git a/tests/McpServer.Support.Mcp.Tests/Services/AppSettingsFileServiceTests.cs b/tests/McpServer.Support.Mcp.Tests/Services/AppSettingsFileServiceTests.cs index ba0ae3d9..ec929584 100644 --- a/tests/McpServer.Support.Mcp.Tests/Services/AppSettingsFileServiceTests.cs +++ b/tests/McpServer.Support.Mcp.Tests/Services/AppSettingsFileServiceTests.cs @@ -180,6 +180,127 @@ await File.WriteAllTextAsync( Assert.Contains("CopilotModel: should-not-change", contentRootYamlText, StringComparison.Ordinal); } + /// + /// TEST-MCP-091: Verifies that concurrent YAML patch requests are serialized across the full + /// read-modify-write cycle so both updates persist instead of one request overwriting the other. + /// The test blocks the first temp-file write after load to force a true overlap opportunity. + /// + [Fact] + public async Task PatchYamlConfigurationAsync_ConcurrentPatchesSerializeWholeMutation() + { + var yamlPath = Path.Combine(_tempDirectory, "appsettings.yaml"); + await File.WriteAllTextAsync( + yamlPath, + """ + VoiceConversation: + CopilotModel: gpt-5.3-codex + """).ConfigureAwait(true); + + var configuration = BuildConfiguration(yamlPath); + var firstWriteEntered = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var releaseFirstWrite = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var writeCount = 0; + var service = CreateService( + configuration, + _tempDirectory, + async (path, content, ct) => + { + var invocation = Interlocked.Increment(ref writeCount); + if (invocation == 1) + { + firstWriteEntered.SetResult(); + await releaseFirstWrite.Task.WaitAsync(ct).ConfigureAwait(false); + } + + await File.WriteAllTextAsync(path, content, ct).ConfigureAwait(false); + }); + + var firstPatch = service.PatchYamlConfigurationAsync( + new Dictionary { ["VoiceConversation:DefaultExecutionStrategy"] = "hosted-mcp-agent" }, + CancellationToken.None); + + await firstWriteEntered.Task.WaitAsync(TimeSpan.FromSeconds(5)).ConfigureAwait(true); + + var secondPatch = service.PatchYamlConfigurationAsync( + new Dictionary { ["VoiceConversation:ModelApiKeyEnvironmentVariableName"] = "OPENAI_API_KEY" }, + CancellationToken.None); + + releaseFirstWrite.SetResult(); + await Task.WhenAll(firstPatch, secondPatch).ConfigureAwait(true); + + var yamlText = await File.ReadAllTextAsync(yamlPath).ConfigureAwait(true); + Assert.Contains("DefaultExecutionStrategy: hosted-mcp-agent", yamlText, StringComparison.Ordinal); + Assert.Contains("ModelApiKeyEnvironmentVariableName: OPENAI_API_KEY", yamlText, StringComparison.Ordinal); + Assert.Equal("hosted-mcp-agent", configuration["VoiceConversation:DefaultExecutionStrategy"]); + Assert.Equal("OPENAI_API_KEY", configuration["VoiceConversation:ModelApiKeyEnvironmentVariableName"]); + } + + /// + /// TEST-MCP-091: Verifies that a failed temp-file write leaves the original YAML document untouched + /// and cleans up the temporary artifact so interrupted writes cannot strand partial config files. + /// + [Fact] + public async Task PatchYamlConfigurationAsync_WhenAtomicWriteFails_LeavesOriginalFileAndCleansTempFile() + { + var yamlPath = Path.Combine(_tempDirectory, "appsettings.yaml"); + await File.WriteAllTextAsync( + yamlPath, + """ + VoiceConversation: + CopilotModel: gpt-5.3-codex + """).ConfigureAwait(true); + + var configuration = BuildConfiguration(yamlPath); + string? tempPath = null; + var service = CreateService( + configuration, + _tempDirectory, + async (path, content, ct) => + { + tempPath = path; + await File.WriteAllTextAsync(path, "partial", ct).ConfigureAwait(false); + throw new IOException("Simulated temp-write failure."); + }); + + await Assert.ThrowsAsync(() => service.PatchYamlConfigurationAsync( + new Dictionary { ["VoiceConversation:CopilotModel"] = "gpt-5.4" }, + CancellationToken.None)).ConfigureAwait(true); + + var yamlText = await File.ReadAllTextAsync(yamlPath).ConfigureAwait(true); + Assert.Contains("CopilotModel: gpt-5.3-codex", yamlText, StringComparison.Ordinal); + Assert.Equal("gpt-5.3-codex", configuration["VoiceConversation:CopilotModel"]); + Assert.NotNull(tempPath); + Assert.False(File.Exists(tempPath)); + } + + /// + /// TEST-MCP-091: Verifies that global prompt updates use the same atomic write service when the active + /// configuration file is JSON-backed, preserving reload behavior and consistent formatting. + /// + [Fact] + public async Task UpdateGlobalPromptTemplateAsync_WhenJsonBacked_UpdatesJsonAndReloadsConfiguration() + { + var jsonPath = Path.Combine(_tempDirectory, "appsettings.json"); + await File.WriteAllTextAsync( + jsonPath, + """ + { + "Mcp": { + "MarkerPromptTemplate": "old-template" + } + } + """).ConfigureAwait(true); + + var configuration = BuildJsonConfiguration(jsonPath); + var service = CreateService(configuration); + + await service.UpdateGlobalPromptTemplateAsync("new-template", CancellationToken.None).ConfigureAwait(true); + + var jsonText = await File.ReadAllTextAsync(jsonPath).ConfigureAwait(true); + Assert.Equal("new-template", configuration["Mcp:MarkerPromptTemplate"]); + Assert.Contains("\"MarkerPromptTemplate\": \"new-template\"", jsonText, StringComparison.Ordinal); + } + /// public void Dispose() { @@ -192,11 +313,16 @@ private AppSettingsFileService CreateService(IConfiguration configuration) return CreateService(configuration, _tempDirectory); } - private static AppSettingsFileService CreateService(IConfiguration configuration, string contentRootPath) + private static AppSettingsFileService CreateService( + IConfiguration configuration, + string contentRootPath, + Func? writeTextAsync = null) { var environment = Substitute.For(); environment.ContentRootPath.Returns(contentRootPath); - return new AppSettingsFileService(configuration, environment); + return writeTextAsync is null + ? new AppSettingsFileService(configuration, environment) + : new AppSettingsFileService(configuration, environment, writeTextAsync); } private static IConfigurationRoot BuildConfiguration(string yamlPath) @@ -205,4 +331,11 @@ private static IConfigurationRoot BuildConfiguration(string yamlPath) .AddYamlFile(yamlPath, optional: false, reloadOnChange: false) .Build(); } + + private static IConfigurationRoot BuildJsonConfiguration(string jsonPath) + { + return new ConfigurationBuilder() + .AddJsonFile(jsonPath, optional: false, reloadOnChange: false) + .Build(); + } } diff --git a/tests/McpServer.Support.Mcp.Tests/Services/ChannelChangeEventBusTests.cs b/tests/McpServer.Support.Mcp.Tests/Services/ChannelChangeEventBusTests.cs index 2c89d6eb..666ea1f2 100644 --- a/tests/McpServer.Support.Mcp.Tests/Services/ChannelChangeEventBusTests.cs +++ b/tests/McpServer.Support.Mcp.Tests/Services/ChannelChangeEventBusTests.cs @@ -1,15 +1,17 @@ using McpServer.Support.Mcp.Notifications; +using McpServer.Support.Mcp.Tests.TestSupport; using Xunit; namespace McpServer.Support.Mcp.Tests.Services; -/// Unit tests for ChannelChangeEventBus pub/sub behavior. +/// TR-MCP-EVT-001: Unit tests for ChannelChangeEventBus pub/sub behavior. public sealed class ChannelChangeEventBusTests { + /// PublishAsync does not throw when no subscribers are registered for the event bus. [Fact] public async Task PublishAsync_NoSubscribers_DoesNotThrow() { - var sut = new ChannelChangeEventBus(); + var sut = new ChannelChangeEventBus(new TestLogger()); var evt = new ChangeEvent { Category = ChangeEventCategories.Todo, @@ -23,10 +25,11 @@ public async Task PublishAsync_NoSubscribers_DoesNotThrow() Assert.Null(ex); } + /// PublishAsync fans a change event out to all active subscribers. [Fact] public async Task PublishAsync_MultipleSubscribers_AllReceiveEvent() { - var sut = new ChannelChangeEventBus(); + var sut = new ChannelChangeEventBus(new TestLogger()); using var cts = new CancellationTokenSource(TimeSpan.FromSeconds(5)); var sub1 = sut.SubscribeAsync(cts.Token).GetAsyncEnumerator(cts.Token); @@ -49,10 +52,11 @@ public async Task PublishAsync_MultipleSubscribers_AllReceiveEvent() Assert.Equal("TEST-002", sub2.Current.EntityId); } + /// SubscribeAsync stops enumeration when the subscriber cancellation token is cancelled. [Fact] public async Task SubscribeAsync_Cancellation_StopsEnumeration() { - var sut = new ChannelChangeEventBus(); + var sut = new ChannelChangeEventBus(new TestLogger()); using var cts = new CancellationTokenSource(); var sub = sut.SubscribeAsync(cts.Token).GetAsyncEnumerator(cts.Token); @@ -63,4 +67,64 @@ await Assert.ThrowsAnyAsync(async () => await sub.MoveNextAsync().ConfigureAwait(true); }).ConfigureAwait(true); } + + /// PublishAsync logs overflow and preserves queued events when a subscriber buffer is full. + [Fact] + public async Task PublishAsync_WhenSubscriberBufferIsFull_LogsWarningAndRejectsNewEvent() + { + var logger = new TestLogger(); + var sut = new ChannelChangeEventBus(logger); + using var cts = new CancellationTokenSource(TimeSpan.FromSeconds(10)); + await using var subscriber = sut.SubscribeAsync(cts.Token).GetAsyncEnumerator(cts.Token); + + var firstReadTask = subscriber.MoveNextAsync().AsTask(); + await sut.PublishAsync( + new ChangeEvent + { + Category = ChangeEventCategories.Todo, + Action = ChangeEventActions.Updated, + EntityId = "ITEM-0000", + }, + cts.Token).ConfigureAwait(true); + + Assert.True(await firstReadTask.ConfigureAwait(true)); + Assert.Equal("ITEM-0000", subscriber.Current.EntityId); + + for (var index = 1; index <= 1000; index++) + { + await sut.PublishAsync( + new ChangeEvent + { + Category = ChangeEventCategories.Todo, + Action = ChangeEventActions.Updated, + EntityId = $"ITEM-{index:0000}", + }, + cts.Token).ConfigureAwait(true); + } + + await sut.PublishAsync( + new ChangeEvent + { + Category = ChangeEventCategories.Todo, + Action = ChangeEventActions.Updated, + EntityId = "ITEM-OVERFLOW", + }, + cts.Token).ConfigureAwait(true); + + var deliveredEntityIds = new List { "ITEM-0000" }; + for (var index = 1; index <= 1000; index++) + { + Assert.True(await subscriber.MoveNextAsync().ConfigureAwait(true)); + deliveredEntityIds.Add(subscriber.Current.EntityId!); + } + + Assert.Equal("ITEM-0000", deliveredEntityIds[0]); + Assert.Equal("ITEM-1000", deliveredEntityIds[^1]); + Assert.DoesNotContain("ITEM-OVERFLOW", deliveredEntityIds); + Assert.Contains( + logger.Entries, + entry => entry.Level == Microsoft.Extensions.Logging.LogLevel.Warning && + entry.Message.Contains("Change event delivery rejected 1 subscriber writes", StringComparison.Ordinal) && + entry.Message.Contains("ITEM-OVERFLOW", StringComparison.Ordinal)); + } } diff --git a/tests/McpServer.Support.Mcp.Tests/Services/DesktopLaunchServiceTests.cs b/tests/McpServer.Support.Mcp.Tests/Services/DesktopLaunchServiceTests.cs new file mode 100644 index 00000000..9ac7dbce --- /dev/null +++ b/tests/McpServer.Support.Mcp.Tests/Services/DesktopLaunchServiceTests.cs @@ -0,0 +1,137 @@ +using System.Collections.Generic; +using McpServer.Support.Mcp.Models; +using McpServer.Support.Mcp.Options; +using McpServer.Support.Mcp.Services; +using Microsoft.Extensions.Configuration; +using Microsoft.Extensions.Logging.Abstractions; +using Microsoft.Extensions.Options; +using NSubstitute; +using Xunit; + +namespace McpServer.Support.Mcp.Tests.Services; + +/// +/// FR-MCP-047/TR-MCP-DESKTOP-001: Verifies that enforces the +/// desktop-launch feature gate and executable allowlist before invoking the launcher process. +/// The tests use an in-memory configuration root plus a substituted +/// so privileged launch decisions can be asserted without starting real desktop programs. +/// +public sealed class DesktopLaunchServiceTests +{ + /// + /// FR-MCP-047/TR-MCP-DESKTOP-001: Verifies that the service fails closed when desktop launch + /// is disabled, even if the launcher path and request payload are otherwise valid. + /// The test uses the current process path as a deterministic absolute executable fixture and a + /// substituted runner so the denial path can prove no launcher invocation occurs. + /// + [Fact] + public async Task LaunchAsync_WhenDesktopLaunchDisabled_ReturnsFailureWithoutInvokingRunner() + { + var executablePath = GetExistingExecutablePath(); + var processRunner = Substitute.For(); + var service = CreateService( + processRunner, + new DesktopLaunchOptions + { + Enabled = false, + AllowedExecutables = { $"**/{Path.GetFileName(executablePath)}" } + }); + + var result = await service.LaunchAsync( + Path.GetDirectoryName(executablePath)!, + new DesktopLaunchRequest { ExecutablePath = executablePath }); + + Assert.False(result.Success); + Assert.Contains("disabled", result.ErrorMessage, StringComparison.OrdinalIgnoreCase); + await processRunner.DidNotReceive().RunAsync(Arg.Any(), Arg.Any(), Arg.Any()); + } + + /// + /// FR-MCP-047/TR-MCP-DESKTOP-001: Verifies that the service rejects executables that do not + /// match the configured allowlist, even when desktop launch is enabled. + /// The test uses the current process path plus a deliberately non-matching pattern so the + /// allowlist check proves the runner stays untouched on denied launches. + /// + [Fact] + public async Task LaunchAsync_WhenExecutableDoesNotMatchAllowlist_ReturnsFailureWithoutInvokingRunner() + { + var executablePath = GetExistingExecutablePath(); + var processRunner = Substitute.For(); + var service = CreateService( + processRunner, + new DesktopLaunchOptions + { + Enabled = true, + AllowedExecutables = { "**/not-allowed.exe" } + }); + + var result = await service.LaunchAsync( + Path.GetDirectoryName(executablePath)!, + new DesktopLaunchRequest { ExecutablePath = executablePath }); + + Assert.False(result.Success); + Assert.Contains("allowlist", result.ErrorMessage, StringComparison.OrdinalIgnoreCase); + await processRunner.DidNotReceive().RunAsync(Arg.Any(), Arg.Any(), Arg.Any()); + } + + /// + /// FR-MCP-047/TR-MCP-DESKTOP-001: Verifies that an enabled service forwards an allowlisted + /// executable to the launcher after normalizing the payload. + /// The test uses the current process path for both the launcher fixture and the executable + /// fixture so the runner can return a deterministic JSON success payload without external files. + /// + [Fact] + public async Task LaunchAsync_WhenExecutableMatchesAllowlist_InvokesRunner() + { + var executablePath = GetExistingExecutablePath(); + var processRunner = Substitute.For(); + processRunner + .RunAsync(Arg.Any(), Arg.Any(), Arg.Any()) + .Returns(Task.FromResult(new ProcessRunResult(0, """{"success":true,"processId":4242,"exitCode":0}""", null))); + + var service = CreateService( + processRunner, + new DesktopLaunchOptions + { + Enabled = true, + AllowedExecutables = { $"**/{Path.GetFileName(executablePath)}" } + }); + + var result = await service.LaunchAsync( + Path.GetDirectoryName(executablePath)!, + new DesktopLaunchRequest + { + ExecutablePath = executablePath, + CreateNoWindow = true, + WaitForExit = true + }); + + Assert.True(result.Success); + Assert.Equal(4242, result.ProcessId); + await processRunner.Received(1).RunAsync( + executablePath, + Arg.Is(arguments => arguments != null && arguments.Contains(Path.GetFileName(executablePath), StringComparison.Ordinal)), + Arg.Any()); + } + + private static DesktopLaunchService CreateService(IProcessRunner processRunner, DesktopLaunchOptions options) + { + var launcherPath = GetExistingExecutablePath(); + var configuration = new ConfigurationBuilder() + .AddInMemoryCollection( + new Dictionary + { + ["Mcp:LauncherPath"] = launcherPath + }) + .Build(); + return new DesktopLaunchService( + configuration, + Microsoft.Extensions.Options.Options.Create(options), + processRunner, + NullLogger.Instance); + } + + private static string GetExistingExecutablePath() + => Environment.ProcessPath + ?? throw new InvalidOperationException("Expected the test host to expose an executable path."); +} diff --git a/tests/McpServer.Support.Mcp.Tests/Services/FileGitHubWorkspaceTokenStoreTests.cs b/tests/McpServer.Support.Mcp.Tests/Services/FileGitHubWorkspaceTokenStoreTests.cs index 4c041c21..b9893918 100644 --- a/tests/McpServer.Support.Mcp.Tests/Services/FileGitHubWorkspaceTokenStoreTests.cs +++ b/tests/McpServer.Support.Mcp.Tests/Services/FileGitHubWorkspaceTokenStoreTests.cs @@ -4,6 +4,9 @@ using Microsoft.Extensions.Logging.Abstractions; using Microsoft.Extensions.Options; using NSubstitute; +using System.Runtime.Versioning; +using System.Security.AccessControl; +using System.Security.Principal; using Xunit; namespace McpServer.Support.Mcp.Tests.Services; @@ -70,6 +73,86 @@ public async Task DeleteAsync_RemovesTokenRecord() Assert.Null(record); } + [Fact] + public async Task UpsertAsync_WhenStoreLockIsHeld_WaitsForRelease() + { + var workspacePath = Path.Combine(_tempRoot, "workspace-locked"); + var lockPath = Path.Combine(_tempRoot, "github-token-store.json.lock"); + using var lockStream = new FileStream(lockPath, FileMode.OpenOrCreate, FileAccess.ReadWrite, FileShare.None); + + var upsertTask = _sut.UpsertAsync(workspacePath, "gho_wait_for_lock"); + + await Task.Delay(100).ConfigureAwait(true); + Assert.False(upsertTask.IsCompleted); + + lockStream.Dispose(); + await upsertTask.ConfigureAwait(true); + + var record = await _sut.GetAsync(workspacePath).ConfigureAwait(true); + Assert.NotNull(record); + Assert.Equal("gho_wait_for_lock", record.AccessToken); + } + + [Fact] + public async Task UpsertAsync_HardensStoreAndLockFilePermissions() + { + var workspacePath = Path.Combine(_tempRoot, "workspace-permissions"); + var storePath = Path.Combine(_tempRoot, "github-token-store.json"); + var lockPath = storePath + ".lock"; + + await _sut.UpsertAsync(workspacePath, "gho_permissions").ConfigureAwait(true); + + Assert.True(File.Exists(storePath)); + Assert.True(File.Exists(lockPath)); + + if (OperatingSystem.IsWindows()) + { + AssertRestrictedWindowsFile(storePath); + AssertRestrictedWindowsFile(lockPath); + return; + } + + if (OperatingSystem.IsLinux() || OperatingSystem.IsMacOS()) + { + var expected = UnixFileMode.UserRead | UnixFileMode.UserWrite; + Assert.Equal(expected, File.GetUnixFileMode(storePath)); + Assert.Equal(expected, File.GetUnixFileMode(lockPath)); + return; + } + + throw new PlatformNotSupportedException("The token-store permission test only supports Windows ACL or Unix file-mode assertions."); + } + + [SupportedOSPlatform("windows")] + private static void AssertRestrictedWindowsFile(string path) + { + var security = new FileInfo(path).GetAccessControl(); + Assert.True(security.AreAccessRulesProtected); + + var rules = security + .GetAccessRules(includeExplicit: true, includeInherited: true, targetType: typeof(SecurityIdentifier)) + .OfType() + .ToArray(); + + using var identity = WindowsIdentity.GetCurrent(); + var currentUser = identity.User + ?? throw new InvalidOperationException("The current Windows identity did not expose a user SID for ACL assertions."); + + AssertContainsAllowFullControl(rules, currentUser); + AssertContainsAllowFullControl(rules, new SecurityIdentifier(WellKnownSidType.BuiltinAdministratorsSid, null)); + AssertContainsAllowFullControl(rules, new SecurityIdentifier(WellKnownSidType.LocalSystemSid, null)); + } + + [SupportedOSPlatform("windows")] + private static void AssertContainsAllowFullControl(IEnumerable rules, SecurityIdentifier expectedSid) + { + Assert.Contains(rules, rule => + rule.AccessControlType == AccessControlType.Allow + && rule.IdentityReference is SecurityIdentifier actualSid + && string.Equals(actualSid.Value, expectedSid.Value, StringComparison.OrdinalIgnoreCase) + && rule.FileSystemRights.HasFlag(FileSystemRights.FullControl)); + } + public void Dispose() { _sut.Dispose(); diff --git a/tests/McpServer.Support.Mcp.Tests/Services/GitHubCliServiceTests.cs b/tests/McpServer.Support.Mcp.Tests/Services/GitHubCliServiceTests.cs index 3d580982..a5f34d4d 100644 --- a/tests/McpServer.Support.Mcp.Tests/Services/GitHubCliServiceTests.cs +++ b/tests/McpServer.Support.Mcp.Tests/Services/GitHubCliServiceTests.cs @@ -1,13 +1,13 @@ -using McpServer.Support.Mcp.Models; -using McpServer.Support.Mcp.Options; -using McpServer.Support.Mcp.Services; -using McpServer.Support.Mcp.Tests; -using Microsoft.AspNetCore.Http; -using Microsoft.Extensions.DependencyInjection; -using Microsoft.Extensions.Options; -using NSubstitute; -using Microsoft.Extensions.Logging.Abstractions; -using Xunit; +using McpServer.Support.Mcp.Models; +using McpServer.Support.Mcp.Options; +using McpServer.Support.Mcp.Services; +using McpServer.Support.Mcp.Tests; +using Microsoft.AspNetCore.Http; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Options; +using NSubstitute; +using Microsoft.Extensions.Logging.Abstractions; +using Xunit; namespace McpServer.Support.Mcp.Tests.Services; @@ -100,6 +100,24 @@ public async Task CommentOnIssueAsync_WhenGhSucceeds_ReturnsSuccess() Assert.True(result.Success); } + /// + /// TEST-MCP-GH-006: Verifies that issue comment targets are emitted after an explicit end-of-options + /// marker so a flag-shaped identifier cannot be reinterpreted as an injected gh CLI option. + /// + [Fact] + public async Task CommentOnIssueAsync_WithFlagLikeIdentifier_UsesEndOfOptionsMarker() + { + _processRunner.RunAsync("gh", Arg.Any(), Arg.Any()) + .Returns(new ProcessRunResult(0, "", null)); + + var result = await _sut.CommentOnIssueAsync("--repo", "test comment").ConfigureAwait(true); + + Assert.True(result.Success); + await _processRunner.Received(1).RunAsync("gh", + Arg.Is(a => a != null && a.Contains("issue comment --body \"test comment\" -- --repo", StringComparison.Ordinal)), + Arg.Any()).ConfigureAwait(true); + } + [Fact] public async Task CommentOnPullAsync_VerifiesGhArgs() { @@ -110,7 +128,7 @@ public async Task CommentOnPullAsync_VerifiesGhArgs() Assert.True(result.Success); await _processRunner.Received(1).RunAsync("gh", - Arg.Is(a => a != null && a.Contains("pr comment 42", StringComparison.Ordinal)), + Arg.Is(a => a != null && a.Contains("pr comment --body \"PR comment\" -- 42", StringComparison.Ordinal)), Arg.Any()).ConfigureAwait(true); } @@ -205,6 +223,20 @@ await _processRunner.Received(1).RunAsync("gh", Arg.Any()).ConfigureAwait(true); } + /// + /// TEST-MCP-GH-006: Verifies that invalid close reasons are rejected before launching the GitHub CLI so + /// attacker-controlled query strings cannot append extra flags through the close-issue reason parameter. + /// + [Fact] + public async Task CloseIssueAsync_WithInvalidReason_DoesNotInvokeGh() + { + var result = await _sut.CloseIssueAsync(42, "completed --repo other/repo").ConfigureAwait(true); + + Assert.False(result.Success); + Assert.Equal("Invalid close reason. Allowed values: completed, not_planned.", result.ErrorMessage); + await _processRunner.DidNotReceiveWithAnyArgs().RunAsync(default!, default!, default).ConfigureAwait(true); + } + [Fact] public async Task CloseIssueAsync_WithoutReason_NoReasonFlag() { @@ -234,6 +266,20 @@ await _processRunner.Received(1).RunAsync("gh", Arg.Any()).ConfigureAwait(true); } + /// + /// TEST-MCP-GH-006: Verifies that invalid list-state values are rejected before process launch so state + /// query parameters cannot smuggle additional GitHub CLI flags into the issue-list command line. + /// + [Fact] + public async Task ListIssuesAsync_WithInvalidState_DoesNotInvokeGh() + { + var result = await _sut.ListIssuesAsync("open --repo other/repo", 10).ConfigureAwait(true); + + Assert.False(result.Success); + Assert.Equal("Invalid state. Allowed values: open, closed, all.", result.Error); + await _processRunner.DidNotReceiveWithAnyArgs().RunAsync(default!, default!, default).ConfigureAwait(true); + } + [Fact] public async Task ListIssueLabelsAsync_WhenGhSucceeds_ReturnsLabels() { @@ -251,158 +297,158 @@ public async Task ListIssueLabelsAsync_WhenGhSucceeds_ReturnsLabels() } [Fact] - public async Task ListIssueLabelsAsync_WhenGhFails_ReturnsError() - { - _processRunner.RunAsync("gh", Arg.Any(), Arg.Any()) - .Returns(new ProcessRunResult(1, null, "not authenticated")); - - var result = await _sut.ListIssueLabelsAsync().ConfigureAwait(true); - - Assert.False(result.Success); - Assert.Equal("not authenticated", result.ErrorMessage); - } - - [Fact] - public async Task ListWorkflowRunsAsync_WhenGhSucceeds_ReturnsRuns() - { - var json = """[{"databaseId":101,"workflowName":"CI","displayTitle":"build","headBranch":"main","status":"completed","conclusion":"success","event":"push","url":"https://github.com/x/actions/runs/101","createdAt":"2026-03-01T00:00:00Z","updatedAt":"2026-03-01T00:05:00Z"}]"""; - _processRunner.RunAsync("gh", Arg.Any(), Arg.Any()) - .Returns(new ProcessRunResult(0, json, null)); - - var result = await _sut.ListWorkflowRunsAsync(new GitHubWorkflowRunQuery { Limit = 10 }).ConfigureAwait(true); - - Assert.True(result.Success); - Assert.Single(result.Runs); - Assert.Equal(101, result.Runs[0].RunId); - Assert.Equal("CI", result.Runs[0].WorkflowName); - } - - [Fact] - public async Task GetWorkflowRunAsync_WhenGhSucceeds_ReturnsRun() - { - var json = """ - { - "databaseId": 202, - "workflowName": "Deploy", - "displayTitle": "release", - "headBranch": "main", - "headSha": "abc123", - "status": "completed", - "conclusion": "success", - "event": "workflow_dispatch", - "url": "https://github.com/x/actions/runs/202", - "attempt": 1, - "createdAt": "2026-03-01T00:00:00Z", - "updatedAt": "2026-03-01T00:10:00Z", - "jobs": [ - { - "name": "build", - "status": "completed", - "conclusion": "success", - "startedAt": "2026-03-01T00:01:00Z", - "completedAt": "2026-03-01T00:09:00Z", - "url": "https://github.com/x/actions/runs/202/job/1", - "steps": [ - { "name": "checkout", "status": "completed", "conclusion": "success", "number": 1 } - ] - } - ] - } - """; - _processRunner.RunAsync("gh", Arg.Any(), Arg.Any()) - .Returns(new ProcessRunResult(0, json, null)); - - var result = await _sut.GetWorkflowRunAsync(202).ConfigureAwait(true); - - Assert.True(result.Success); - Assert.NotNull(result.Run); - Assert.Equal(202, result.Run.RunId); - Assert.Single(result.Run.Jobs); - Assert.Single(result.Run.Jobs[0].Steps); - Assert.Equal("checkout", result.Run.Jobs[0].Steps[0].Name); - } - - [Fact] - public async Task RerunWorkflowRunAsync_UsesRerunCommand() - { - _processRunner.RunAsync("gh", Arg.Any(), Arg.Any()) - .Returns(new ProcessRunResult(0, "", null)); - - var result = await _sut.RerunWorkflowRunAsync(303).ConfigureAwait(true); - - Assert.True(result.Success); - await _processRunner.Received(1).RunAsync("gh", - Arg.Is(a => a != null && a.Contains("run rerun 303", StringComparison.Ordinal)), - Arg.Any()).ConfigureAwait(true); - } - - [Fact] - public async Task CancelWorkflowRunAsync_UsesCancelCommand() - { - _processRunner.RunAsync("gh", Arg.Any(), Arg.Any()) - .Returns(new ProcessRunResult(0, "", null)); - - var result = await _sut.CancelWorkflowRunAsync(404).ConfigureAwait(true); - - Assert.True(result.Success); - await _processRunner.Received(1).RunAsync("gh", - Arg.Is(a => a != null && a.Contains("run cancel 404", StringComparison.Ordinal)), - Arg.Any()).ConfigureAwait(true); - } - - [Fact] - public async Task ListIssuesAsync_WithStoredWorkspaceToken_UsesProcessRunRequestOverride() - { - var tokenStore = Substitute.For(); - tokenStore.GetAsync(Arg.Any(), Arg.Any()) - .Returns(new GitHubWorkspaceTokenRecord("C:\\workspace", "gho_stored", DateTimeOffset.UtcNow, null)); - - var options = Substitute.For>(); - options.CurrentValue.Returns(new GitHubIntegrationOptions - { - PreferStoredToken = true, - AllowCliFallback = true, - }); - - var services = new ServiceCollection(); - services.AddScoped(_ => new WorkspaceContext { WorkspacePath = "C:\\workspace" }); - using var provider = services.BuildServiceProvider(); - using var scope = provider.CreateScope(); - var accessor = Substitute.For(); - accessor.HttpContext.Returns(new DefaultHttpContext { RequestServices = scope.ServiceProvider }); - - _processRunner.RunAsync(Arg.Any(), Arg.Any()) - .Returns(new ProcessRunResult(0, "[]", null)); - - var workspaceAccessor = TestWorkspaceAccessorHelper.Create(Substitute.For(), repoRoot: "C:\\workspace"); - var sut = new GitHubCliService(_processRunner, NullLogger.Instance, tokenStore, accessor, options, workspaceAccessor); - var result = await sut.ListIssuesAsync("open", 10).ConfigureAwait(true); - - Assert.True(result.Success); - await _processRunner.Received(1).RunAsync( - Arg.Is(r => r != null - && r.FileName == "gh" - && r.GitHubTokenOverride == "gho_stored" - && r.WorkingDirectory == Path.GetFullPath("C:\\workspace")), - Arg.Any()).ConfigureAwait(true); - } - - [Fact] - public async Task ListIssuesAsync_WithWorkspaceAccessor_UsesWorkspaceWorkingDirectory() - { - _processRunner.RunAsync(Arg.Any(), Arg.Any()) - .Returns(new ProcessRunResult(0, "[]", null)); - - var workspaceAccessor = TestWorkspaceAccessorHelper.Create(Substitute.For(), repoRoot: "C:\\repo\\workspace"); - var sut = new GitHubCliService(_processRunner, NullLogger.Instance, workspaceAccessor: workspaceAccessor); - - var result = await sut.ListIssuesAsync("open", 5).ConfigureAwait(true); - - Assert.True(result.Success); - await _processRunner.Received(1).RunAsync( - Arg.Is(request => request != null - && request.FileName == "gh" - && request.WorkingDirectory == Path.GetFullPath("C:\\repo\\workspace")), - Arg.Any()).ConfigureAwait(true); - } -} + public async Task ListIssueLabelsAsync_WhenGhFails_ReturnsError() + { + _processRunner.RunAsync("gh", Arg.Any(), Arg.Any()) + .Returns(new ProcessRunResult(1, null, "not authenticated")); + + var result = await _sut.ListIssueLabelsAsync().ConfigureAwait(true); + + Assert.False(result.Success); + Assert.Equal("not authenticated", result.ErrorMessage); + } + + [Fact] + public async Task ListWorkflowRunsAsync_WhenGhSucceeds_ReturnsRuns() + { + var json = """[{"databaseId":101,"workflowName":"CI","displayTitle":"build","headBranch":"main","status":"completed","conclusion":"success","event":"push","url":"https://github.com/x/actions/runs/101","createdAt":"2026-03-01T00:00:00Z","updatedAt":"2026-03-01T00:05:00Z"}]"""; + _processRunner.RunAsync("gh", Arg.Any(), Arg.Any()) + .Returns(new ProcessRunResult(0, json, null)); + + var result = await _sut.ListWorkflowRunsAsync(new GitHubWorkflowRunQuery { Limit = 10 }).ConfigureAwait(true); + + Assert.True(result.Success); + Assert.Single(result.Runs); + Assert.Equal(101, result.Runs[0].RunId); + Assert.Equal("CI", result.Runs[0].WorkflowName); + } + + [Fact] + public async Task GetWorkflowRunAsync_WhenGhSucceeds_ReturnsRun() + { + var json = """ + { + "databaseId": 202, + "workflowName": "Deploy", + "displayTitle": "release", + "headBranch": "main", + "headSha": "abc123", + "status": "completed", + "conclusion": "success", + "event": "workflow_dispatch", + "url": "https://github.com/x/actions/runs/202", + "attempt": 1, + "createdAt": "2026-03-01T00:00:00Z", + "updatedAt": "2026-03-01T00:10:00Z", + "jobs": [ + { + "name": "build", + "status": "completed", + "conclusion": "success", + "startedAt": "2026-03-01T00:01:00Z", + "completedAt": "2026-03-01T00:09:00Z", + "url": "https://github.com/x/actions/runs/202/job/1", + "steps": [ + { "name": "checkout", "status": "completed", "conclusion": "success", "number": 1 } + ] + } + ] + } + """; + _processRunner.RunAsync("gh", Arg.Any(), Arg.Any()) + .Returns(new ProcessRunResult(0, json, null)); + + var result = await _sut.GetWorkflowRunAsync(202).ConfigureAwait(true); + + Assert.True(result.Success); + Assert.NotNull(result.Run); + Assert.Equal(202, result.Run.RunId); + Assert.Single(result.Run.Jobs); + Assert.Single(result.Run.Jobs[0].Steps); + Assert.Equal("checkout", result.Run.Jobs[0].Steps[0].Name); + } + + [Fact] + public async Task RerunWorkflowRunAsync_UsesRerunCommand() + { + _processRunner.RunAsync("gh", Arg.Any(), Arg.Any()) + .Returns(new ProcessRunResult(0, "", null)); + + var result = await _sut.RerunWorkflowRunAsync(303).ConfigureAwait(true); + + Assert.True(result.Success); + await _processRunner.Received(1).RunAsync("gh", + Arg.Is(a => a != null && a.Contains("run rerun 303", StringComparison.Ordinal)), + Arg.Any()).ConfigureAwait(true); + } + + [Fact] + public async Task CancelWorkflowRunAsync_UsesCancelCommand() + { + _processRunner.RunAsync("gh", Arg.Any(), Arg.Any()) + .Returns(new ProcessRunResult(0, "", null)); + + var result = await _sut.CancelWorkflowRunAsync(404).ConfigureAwait(true); + + Assert.True(result.Success); + await _processRunner.Received(1).RunAsync("gh", + Arg.Is(a => a != null && a.Contains("run cancel 404", StringComparison.Ordinal)), + Arg.Any()).ConfigureAwait(true); + } + + [Fact] + public async Task ListIssuesAsync_WithStoredWorkspaceToken_UsesProcessRunRequestOverride() + { + var tokenStore = Substitute.For(); + tokenStore.GetAsync(Arg.Any(), Arg.Any()) + .Returns(new GitHubWorkspaceTokenRecord("C:\\workspace", "gho_stored", DateTimeOffset.UtcNow, null)); + + var options = Substitute.For>(); + options.CurrentValue.Returns(new GitHubIntegrationOptions + { + PreferStoredToken = true, + AllowCliFallback = true, + }); + + var services = new ServiceCollection(); + services.AddScoped(_ => new WorkspaceContext { WorkspacePath = "C:\\workspace" }); + using var provider = services.BuildServiceProvider(); + using var scope = provider.CreateScope(); + var accessor = Substitute.For(); + accessor.HttpContext.Returns(new DefaultHttpContext { RequestServices = scope.ServiceProvider }); + + _processRunner.RunAsync(Arg.Any(), Arg.Any()) + .Returns(new ProcessRunResult(0, "[]", null)); + + var workspaceAccessor = TestWorkspaceAccessorHelper.Create(Substitute.For(), repoRoot: "C:\\workspace"); + var sut = new GitHubCliService(_processRunner, NullLogger.Instance, tokenStore, accessor, options, workspaceAccessor); + var result = await sut.ListIssuesAsync("open", 10).ConfigureAwait(true); + + Assert.True(result.Success); + await _processRunner.Received(1).RunAsync( + Arg.Is(r => r != null + && r.FileName == "gh" + && r.GitHubTokenOverride == "gho_stored" + && r.WorkingDirectory == Path.GetFullPath("C:\\workspace")), + Arg.Any()).ConfigureAwait(true); + } + + [Fact] + public async Task ListIssuesAsync_WithWorkspaceAccessor_UsesWorkspaceWorkingDirectory() + { + _processRunner.RunAsync(Arg.Any(), Arg.Any()) + .Returns(new ProcessRunResult(0, "[]", null)); + + var workspaceAccessor = TestWorkspaceAccessorHelper.Create(Substitute.For(), repoRoot: "C:\\repo\\workspace"); + var sut = new GitHubCliService(_processRunner, NullLogger.Instance, workspaceAccessor: workspaceAccessor); + + var result = await sut.ListIssuesAsync("open", 5).ConfigureAwait(true); + + Assert.True(result.Success); + await _processRunner.Received(1).RunAsync( + Arg.Is(request => request != null + && request.FileName == "gh" + && request.WorkingDirectory == Path.GetFullPath("C:\\repo\\workspace")), + Arg.Any()).ConfigureAwait(true); + } +} diff --git a/tests/McpServer.Support.Mcp.Tests/Services/InteractionLogSubmissionChannelTests.cs b/tests/McpServer.Support.Mcp.Tests/Services/InteractionLogSubmissionChannelTests.cs index f01c981f..07f5bbc2 100644 --- a/tests/McpServer.Support.Mcp.Tests/Services/InteractionLogSubmissionChannelTests.cs +++ b/tests/McpServer.Support.Mcp.Tests/Services/InteractionLogSubmissionChannelTests.cs @@ -51,4 +51,40 @@ public async Task TryDequeueAsync_WhenEmpty_CanBeCancelled() Assert.False(success); Assert.Null(entry); } + + /// TryEnqueue rejects a new entry when the bounded queue is full and preserves the already queued entry. + [Fact] + public async Task TryEnqueue_WhenQueueFull_ReturnsFalseAndPreservesQueuedEntry() + { + var options = Microsoft.Extensions.Options.Options.Create(new McpInteractionLoggingOptions { QueueCapacity = 1 }); + var channel = new InteractionLogSubmissionChannel(options, NullLogger.Instance); + + var first = new InteractionLogEntry + { + TimestampUtc = DateTime.UtcNow, + Method = "GET", + Path = "/mcpserver/context/sources", + StatusCode = 200, + DurationMs = 1.5, + RequestId = "req-1" + }; + var second = new InteractionLogEntry + { + TimestampUtc = DateTime.UtcNow, + Method = "POST", + Path = "/mcpserver/context/search", + StatusCode = 202, + DurationMs = 3.0, + RequestId = "req-2" + }; + + Assert.True(channel.TryEnqueue(first)); + Assert.False(channel.TryEnqueue(second)); + + var (success, dequeued) = await channel.TryDequeueAsync().ConfigureAwait(true); + Assert.True(success); + Assert.NotNull(dequeued); + Assert.Equal("req-1", dequeued.RequestId); + Assert.Equal("/mcpserver/context/sources", dequeued.Path); + } } diff --git a/tests/McpServer.Support.Mcp.Tests/Services/PairingLoginAttemptGuardTests.cs b/tests/McpServer.Support.Mcp.Tests/Services/PairingLoginAttemptGuardTests.cs new file mode 100644 index 00000000..6816d526 --- /dev/null +++ b/tests/McpServer.Support.Mcp.Tests/Services/PairingLoginAttemptGuardTests.cs @@ -0,0 +1,81 @@ +using System.Net; +using McpServer.Support.Mcp.Services; +using Xunit; + +namespace McpServer.Support.Mcp.Tests.Services; + +/// +/// FR-MCP-014: Verifies pairing sign-in throttling and lockout behavior using a controllable clock so brute-force +/// resistance can be validated deterministically without sleeping in tests. +/// +public sealed class PairingLoginAttemptGuardTests +{ + /// + /// FR-MCP-014: Verifies that repeated failed attempts for the same username and remote IP produce a temporary + /// lockout and that the lockout clears after the reported retry interval elapses. + /// + [Fact] + public void TryAcquire_AfterThresholdFailures_BlocksUntilRetryAfterExpires() + { + var now = DateTimeOffset.Parse("2026-03-21T00:00:00Z"); + var guard = new PairingLoginAttemptGuard(() => now); + var remoteIp = IPAddress.Loopback; + + for (var i = 0; i < 5; i++) + guard.RecordFailure("admin", remoteIp); + + Assert.False(guard.TryAcquire("admin", remoteIp, out var retryAfter)); + Assert.True(retryAfter > TimeSpan.Zero); + + now = now.Add(retryAfter).Add(TimeSpan.FromSeconds(1)); + + Assert.True(guard.TryAcquire("admin", remoteIp, out var clearedRetryAfter)); + Assert.Equal(TimeSpan.Zero, clearedRetryAfter); + } + + /// + /// FR-MCP-014: Verifies that a successful authentication resets the principal-specific failure history so + /// earlier mistakes do not cause a later lockout for the same username and remote IP. + /// + [Fact] + public void RecordSuccess_ClearsPrincipalFailureHistory() + { + var now = DateTimeOffset.Parse("2026-03-21T00:00:00Z"); + var guard = new PairingLoginAttemptGuard(() => now); + var remoteIp = IPAddress.Loopback; + + for (var i = 0; i < 4; i++) + guard.RecordFailure("admin", remoteIp); + + guard.RecordSuccess("admin", remoteIp); + + for (var i = 0; i < 4; i++) + guard.RecordFailure("admin", remoteIp); + + Assert.True(guard.TryAcquire("admin", remoteIp, out var retryAfter)); + Assert.Equal(TimeSpan.Zero, retryAfter); + } + + /// + /// FR-MCP-014: Verifies that one remote IP cannot make unlimited failed pairing attempts across many usernames + /// because the IP-level failure window starts rejecting new attempts until the fixed window expires. + /// + [Fact] + public void TryAcquire_AfterIpFailureWindowExceeded_ReturnsFalseUntilWindowResets() + { + var now = DateTimeOffset.Parse("2026-03-21T00:00:00Z"); + var guard = new PairingLoginAttemptGuard(() => now); + var remoteIp = IPAddress.Parse("203.0.113.7"); + + for (var i = 0; i < 20; i++) + guard.RecordFailure($"user-{i}", remoteIp); + + Assert.False(guard.TryAcquire("another-user", remoteIp, out var retryAfter)); + Assert.True(retryAfter > TimeSpan.Zero); + + now = now.AddMinutes(5).AddSeconds(1); + + Assert.True(guard.TryAcquire("another-user", remoteIp, out var clearedRetryAfter)); + Assert.Equal(TimeSpan.Zero, clearedRetryAfter); + } +} diff --git a/tests/McpServer.Support.Mcp.Tests/Services/RepoFileServiceTests.cs b/tests/McpServer.Support.Mcp.Tests/Services/RepoFileServiceTests.cs index 82dbc349..39d2d06f 100644 --- a/tests/McpServer.Support.Mcp.Tests/Services/RepoFileServiceTests.cs +++ b/tests/McpServer.Support.Mcp.Tests/Services/RepoFileServiceTests.cs @@ -22,6 +22,9 @@ public RepoFileServiceTests() File.WriteAllText(Path.Combine(_tempDir, "readme.md"), "# Hello"); Directory.CreateDirectory(Path.Combine(_tempDir, "src")); File.WriteAllText(Path.Combine(_tempDir, "src", "code.cs"), "class Foo {}"); + File.WriteAllText(Path.Combine(_tempDir, "src", "notes.txt"), "notes"); + Directory.CreateDirectory(Path.Combine(_tempDir, "src", "nested")); + File.WriteAllText(Path.Combine(_tempDir, "src", "nested", "deep.cs"), "class Deep {}"); var options = Microsoft.Extensions.Options.Options.Create(new IngestionOptions { RepoRoot = _tempDir }); var workspaceContext = new WorkspaceContext(); @@ -115,4 +118,127 @@ public async Task WriteAsync_DisallowedPath_ReturnsFailure() Assert.False(result.Written); } + + [Fact] + public async Task ReadAsync_GlobAllowlist_RequiresActualPatternMatch() + { + var allowedRoot = Path.Combine(_tempDir, "src", "McpServer.Cqrs"); + Directory.CreateDirectory(allowedRoot); + File.WriteAllText(Path.Combine(allowedRoot, "inside.cs"), "class Inside {}"); + + var prefixLookalikeRoot = Path.Combine(_tempDir, "src", "McpServer.Cqrs.Bad"); + Directory.CreateDirectory(prefixLookalikeRoot); + File.WriteAllText(Path.Combine(prefixLookalikeRoot, "escape.cs"), "class Escape {}"); + + var options = Microsoft.Extensions.Options.Options.Create(new IngestionOptions + { + RepoRoot = _tempDir, + RepoAllowlist = new[] { "src/McpServer.Cqrs/**/*.cs" } + }); + var sut = new RepoFileService(options, new WorkspaceContext(), _auditLog, NullLogger.Instance); + + var allowed = await sut.ReadAsync("src/McpServer.Cqrs/inside.cs").ConfigureAwait(true); + var disallowed = await sut.ReadAsync("src/McpServer.Cqrs.Bad/escape.cs").ConfigureAwait(true); + + Assert.NotNull(allowed); + Assert.Null(disallowed); + } + + [Fact] + public async Task ReadAsync_GlobAllowlist_DeniesExtensionMismatchWithinAllowedTree() + { + var options = Microsoft.Extensions.Options.Options.Create(new IngestionOptions + { + RepoRoot = _tempDir, + RepoAllowlist = new[] { "src/**/*.cs" } + }); + var sut = new RepoFileService(options, new WorkspaceContext(), _auditLog, NullLogger.Instance); + + var allowed = await sut.ReadAsync("src/nested/deep.cs").ConfigureAwait(true); + var disallowed = await sut.ReadAsync("src/notes.txt").ConfigureAwait(true); + + Assert.NotNull(allowed); + Assert.Null(disallowed); + } + + [Fact] + public async Task ListAsync_GlobAllowlist_RootIncludesRelevantAncestorDirectoriesOnly() + { + var options = Microsoft.Extensions.Options.Options.Create(new IngestionOptions + { + RepoRoot = _tempDir, + RepoAllowlist = new[] { "src/**/*.cs" } + }); + var sut = new RepoFileService(options, new WorkspaceContext(), _auditLog, NullLogger.Instance); + + var root = await sut.ListAsync(".").ConfigureAwait(true); + var src = await sut.ListAsync("src").ConfigureAwait(true); + + Assert.Contains(root.Entries, entry => entry.Name == "src" && entry.IsDirectory); + Assert.DoesNotContain(root.Entries, entry => entry.Name == "readme.md"); + Assert.Contains(src.Entries, entry => entry.Name == "code.cs" && !entry.IsDirectory); + Assert.Contains(src.Entries, entry => entry.Name == "nested" && entry.IsDirectory); + Assert.DoesNotContain(src.Entries, entry => entry.Name == "notes.txt"); + } + + [Fact] + public async Task ReadAsync_PathThroughSymlinkOutsideRepo_ReturnsNull() + { + var outsideDir = Path.Combine(Path.GetTempPath(), $"repo_escape_{Guid.NewGuid():N}"); + Directory.CreateDirectory(outsideDir); + File.WriteAllText(Path.Combine(outsideDir, "outside.cs"), "class Outside {}"); + + var linkPath = Path.Combine(_tempDir, "linked-outside"); + try + { + if (!TryCreateDirectoryLink(linkPath, outsideDir)) + return; + + var options = Microsoft.Extensions.Options.Options.Create(new IngestionOptions + { + RepoRoot = _tempDir, + RepoAllowlist = new[] { "linked-outside/**/*.cs" } + }); + var sut = new RepoFileService(options, new WorkspaceContext(), _auditLog, NullLogger.Instance); + + var result = await sut.ReadAsync("linked-outside/outside.cs").ConfigureAwait(true); + + Assert.Null(result); + } + finally + { + try + { + if (Directory.Exists(linkPath)) + Directory.Delete(linkPath); + } + catch + { + } + + if (Directory.Exists(outsideDir)) + Directory.Delete(outsideDir, recursive: true); + } + } + + private static bool TryCreateDirectoryLink(string linkPath, string targetPath) + { + try + { + Directory.CreateSymbolicLink(linkPath, targetPath); + return true; + } + catch (IOException) + { + return false; + } + catch (UnauthorizedAccessException) + { + return false; + } + catch (PlatformNotSupportedException) + { + return false; + } + } } diff --git a/tests/McpServer.Support.Mcp.Tests/Services/SqliteTodoServiceTests.cs b/tests/McpServer.Support.Mcp.Tests/Services/SqliteTodoServiceTests.cs index c421a9b8..a4e05206 100644 --- a/tests/McpServer.Support.Mcp.Tests/Services/SqliteTodoServiceTests.cs +++ b/tests/McpServer.Support.Mcp.Tests/Services/SqliteTodoServiceTests.cs @@ -383,6 +383,114 @@ public async Task Create_WhenYamlProjectionFails_ReturnsProjectionFailureButKeep TryDelete(root); } + /// + /// TR-MCP-TODO-006: Verifies that projection status reports a healthy, consistent YAML projection after + /// a successful SQLite-backed mutation. The fixture uses the isolated temp DB/YAML pair created for this + /// test class so the status check can assert live consistency instead of repository-global state. + /// + [Fact] + public async Task GetProjectionStatusAsync_AfterSuccessfulProjection_ReturnsConsistentStatus() + { + var result = await _sut.CreateAsync(new TodoCreateRequest + { + Id = "SQL-STATUS-001", + Title = "Projection status", + Section = "mvp-app", + Priority = "high", + }).ConfigureAwait(true); + + Assert.True(result.Success); + + var status = await _sut.GetProjectionStatusAsync().ConfigureAwait(true); + + Assert.Equal("sqlite", status.AuthoritativeStore); + Assert.Equal(_tempDbPath, status.AuthoritativeDataSource); + Assert.Equal(_tempYamlPath, status.ProjectionTargetPath); + Assert.True(status.ProjectionTargetExists); + Assert.True(status.ProjectionConsistent); + Assert.False(status.RepairRequired); + Assert.Null(status.LastProjectionFailure); + Assert.Contains("matches authoritative SQLite state", status.Message ?? string.Empty, StringComparison.OrdinalIgnoreCase); + } + + /// + /// TR-MCP-TODO-006: Verifies that projection status requires repair when the projected TODO.yaml file is + /// missing even though SQLite state remains authoritative and healthy. The fixture creates a successful + /// projection first, then removes the YAML file to exercise the live consistency check without failure metadata. + /// + [Fact] + public async Task GetProjectionStatusAsync_WhenProjectedYamlIsMissing_RequiresRepair() + { + var result = await _sut.CreateAsync(new TodoCreateRequest + { + Id = "SQL-STATUS-002", + Title = "Missing projection", + Section = "mvp-app", + Priority = "medium", + }).ConfigureAwait(true); + + Assert.True(result.Success); + File.Delete(_tempYamlPath); + + var status = await _sut.GetProjectionStatusAsync().ConfigureAwait(true); + + Assert.False(status.ProjectionTargetExists); + Assert.False(status.ProjectionConsistent); + Assert.True(status.RepairRequired); + Assert.Null(status.LastProjectionFailure); + Assert.Contains("does not exist", status.Message ?? string.Empty, StringComparison.OrdinalIgnoreCase); + } + + /// + /// TR-MCP-TODO-006: Verifies that an operator-requested repair rewrites TODO.yaml from authoritative + /// SQLite state after a real projection failure and clears the repair-required status. The fixture first + /// forces projection failure by making the target path a directory, then restores the path and requests repair. + /// + [Fact] + public async Task RepairProjectionAsync_AfterProjectionFailure_RebuildsYamlAndClearsFailureStatus() + { + var root = Path.Combine(Path.GetTempPath(), $"todo_projection_repair_{Guid.NewGuid():N}"); + var dbPath = Path.Combine(root, "mcp.db"); + var yamlDirectoryPath = Path.Combine(root, "docs", "Project", "TODO.yaml"); + Directory.CreateDirectory(yamlDirectoryPath); + + var auditLog = Substitute.For(); + using var failingStore = new SqliteTodoService(dbPath, yamlDirectoryPath, auditLog, NullLogger.Instance); + + var createResult = await failingStore.CreateAsync(new TodoCreateRequest + { + Id = "SQL-REPAIR-001", + Title = "Repair target", + Section = "mvp-app", + Priority = "high", + }).ConfigureAwait(true); + + Assert.False(createResult.Success); + Assert.Equal(TodoMutationFailureKind.ProjectionFailed, createResult.FailureKind); + + var failedStatus = await failingStore.GetProjectionStatusAsync().ConfigureAwait(true); + Assert.True(failedStatus.RepairRequired); + Assert.NotNull(failedStatus.LastProjectionFailure); + + Directory.Delete(yamlDirectoryPath, recursive: true); + + var repairResult = await failingStore.RepairProjectionAsync().ConfigureAwait(true); + + Assert.True(repairResult.Success); + Assert.Null(repairResult.Error); + Assert.False(repairResult.Status.RepairRequired); + Assert.True(repairResult.Status.ProjectionTargetExists); + Assert.True(repairResult.Status.ProjectionConsistent); + Assert.Null(repairResult.Status.LastProjectionFailure); + Assert.True(File.Exists(yamlDirectoryPath)); + + var yaml = await File.ReadAllTextAsync(yamlDirectoryPath).ConfigureAwait(true); + Assert.Contains("SQL-REPAIR-001", yaml, StringComparison.Ordinal); + Assert.Contains("Repair target", yaml, StringComparison.Ordinal); + + TryDelete(root); + } + private static void TryDelete(string path) { try diff --git a/tests/McpServer.Support.Mcp.Tests/Services/ToolBucketServiceTests.cs b/tests/McpServer.Support.Mcp.Tests/Services/ToolBucketServiceTests.cs index c43cb0c1..039539f0 100644 --- a/tests/McpServer.Support.Mcp.Tests/Services/ToolBucketServiceTests.cs +++ b/tests/McpServer.Support.Mcp.Tests/Services/ToolBucketServiceTests.cs @@ -101,6 +101,30 @@ public async Task BrowseAsync_WhenGlobalBucketExistsOutsideWorkspaceScope_Return Assert.Equal("mcp-session-module", result.Tools[0].Name); } + /// + /// Verifies that an unscoped bucket query only returns global buckets instead of every tenant bucket, + /// which keeps global discovery working while preventing empty-workspace data visibility across tenants. + /// Validates FR-MCP-022 and TR-MCP-MT-003. + /// + [Fact] + public async Task ListBucketsAsync_WithoutWorkspaceScope_ReturnsOnlyGlobalBuckets() + { + using (var seed = CreateContext(null)) + { + seed.ToolBuckets.Add(CreateBucketEntity("official", string.Empty)); + seed.ToolBuckets.Add(CreateBucketEntity("workspace-only", @"E:\github\McpServer")); + seed.SaveChanges(); + } + + using var unscopedDb = CreateContext(null); + var sut = CreateSut(unscopedDb); + + var result = await sut.ListBucketsAsync().ConfigureAwait(true); + + Assert.Single(result.Buckets); + Assert.Equal("official", result.Buckets[0].Name); + } + /// /// Disposes the shared relational test database connection after each test class instance so temporary /// resources are released deterministically. diff --git a/tests/McpServer.Support.Mcp.Tests/Services/ToolRegistryScopeTests.cs b/tests/McpServer.Support.Mcp.Tests/Services/ToolRegistryScopeTests.cs index 3649fdbc..9cbf8435 100644 --- a/tests/McpServer.Support.Mcp.Tests/Services/ToolRegistryScopeTests.cs +++ b/tests/McpServer.Support.Mcp.Tests/Services/ToolRegistryScopeTests.cs @@ -112,6 +112,46 @@ public async Task InstallAsync_WithoutWorkspaceParameter_PersistsGlobalToolVisib } } + /// + /// Verifies that an unscoped registry query only returns global tools, preventing empty-workspace + /// visibility from surfacing workspace-specific tool definitions across tenants while keeping + /// intentionally global tools discoverable. + /// + [Fact] + public async Task ListAsync_WithoutWorkspaceParameter_ReturnsOnlyGlobalTools() + { + using (var seed = CreateContext(null)) + { + seed.ToolDefinitions.Add(new ToolDefinitionEntity + { + Name = "global-tool", + Description = "Global tool", + WorkspaceId = string.Empty, + WorkspacePath = null, + DateTimeCreated = DateTimeOffset.UtcNow, + DateTimeModified = DateTimeOffset.UtcNow, + }); + seed.ToolDefinitions.Add(new ToolDefinitionEntity + { + Name = "workspace-tool", + Description = "Workspace tool", + WorkspaceId = @"E:\github\McpServer", + WorkspacePath = @"E:\github\McpServer", + DateTimeCreated = DateTimeOffset.UtcNow, + DateTimeModified = DateTimeOffset.UtcNow, + }); + seed.SaveChanges(); + } + + using var unscopedDb = CreateContext(null); + var registry = CreateRegistry(unscopedDb); + + var result = await registry.ListAsync().ConfigureAwait(true); + + Assert.Single(result.Tools); + Assert.Equal("global-tool", result.Tools[0].Name); + } + /// /// Releases the shared relational test database connection after the test /// class completes so temporary resources do not leak across runs. diff --git a/tests/McpServer.Support.Mcp.Tests/Services/WindowsServiceDeploymentGuardTests.cs b/tests/McpServer.Support.Mcp.Tests/Services/WindowsServiceDeploymentGuardTests.cs index 8c3bdab6..940316ee 100644 --- a/tests/McpServer.Support.Mcp.Tests/Services/WindowsServiceDeploymentGuardTests.cs +++ b/tests/McpServer.Support.Mcp.Tests/Services/WindowsServiceDeploymentGuardTests.cs @@ -15,6 +15,20 @@ public WindowsServiceDeploymentGuardTests() Directory.CreateDirectory(_tempDirectory); } + [Fact] + public void HasDeploymentManifest_WhenManifestExists_ReturnsTrue() + { + WriteManifest(Array.Empty()); + + Assert.True(WindowsServiceDeploymentGuard.HasDeploymentManifest(_tempDirectory)); + } + + [Fact] + public void HasDeploymentManifest_WhenManifestMissing_ReturnsFalse() + { + Assert.False(WindowsServiceDeploymentGuard.HasDeploymentManifest(_tempDirectory)); + } + [Fact] public void EnsureApprovedDeployment_MissingManifest_ThrowsAndLogsClearMessage() { diff --git a/tests/McpServer.Support.Mcp.Tests/Storage/McpDbContextMultiTenantTests.cs b/tests/McpServer.Support.Mcp.Tests/Storage/McpDbContextMultiTenantTests.cs index f0b8721c..9278ca53 100644 --- a/tests/McpServer.Support.Mcp.Tests/Storage/McpDbContextMultiTenantTests.cs +++ b/tests/McpServer.Support.Mcp.Tests/Storage/McpDbContextMultiTenantTests.cs @@ -26,8 +26,52 @@ public McpDbContextMultiTenantTests() new SessionLogEntity { Id = 1, SessionId = "s1", SourceType = "cursor", WorkspaceId = @"C:\ws\alpha" }, new SessionLogEntity { Id = 2, SessionId = "s2", SourceType = "copilot", WorkspaceId = @"C:\ws\beta" }); ctx.ToolBuckets.AddRange( + new ToolBucketEntity { Id = 3, Name = "bucket-global", Owner = "org", Repo = "repo-global", WorkspaceId = string.Empty }, new ToolBucketEntity { Id = 1, Name = "bucket-a", Owner = "org", Repo = "repo-a", WorkspaceId = @"C:\ws\alpha" }, new ToolBucketEntity { Id = 2, Name = "bucket-b", Owner = "org", Repo = "repo-b", WorkspaceId = @"C:\ws\beta" }); + ctx.AgentDefinitions.AddRange( + new AgentDefinitionEntity + { + Id = "agent-global", + WorkspaceId = string.Empty, + DisplayName = "Global Agent", + DefaultLaunchCommand = "pwsh", + DefaultInstructionFile = "AGENTS-README-FIRST.yaml", + DefaultModelsJson = "[]", + DefaultBranchStrategy = "feature/global", + DefaultSeedPrompt = "", + IsBuiltIn = true, + CreatedAt = DateTime.UtcNow, + ModifiedAt = DateTime.UtcNow, + }, + new AgentDefinitionEntity + { + Id = "agent-alpha", + WorkspaceId = @"C:\ws\alpha", + DisplayName = "Alpha Agent", + DefaultLaunchCommand = "pwsh", + DefaultInstructionFile = "AGENTS-README-FIRST.yaml", + DefaultModelsJson = "[]", + DefaultBranchStrategy = "feature/alpha", + DefaultSeedPrompt = "", + IsBuiltIn = false, + CreatedAt = DateTime.UtcNow, + ModifiedAt = DateTime.UtcNow, + }, + new AgentDefinitionEntity + { + Id = "agent-beta", + WorkspaceId = @"C:\ws\beta", + DisplayName = "Beta Agent", + DefaultLaunchCommand = "pwsh", + DefaultInstructionFile = "AGENTS-README-FIRST.yaml", + DefaultModelsJson = "[]", + DefaultBranchStrategy = "feature/beta", + DefaultSeedPrompt = "", + IsBuiltIn = false, + CreatedAt = DateTime.UtcNow, + ModifiedAt = DateTime.UtcNow, + }); ctx.SaveChanges(); } @@ -71,8 +115,9 @@ public void QueryFilter_AppliesToToolBuckets() var buckets = ctx.ToolBuckets.ToList(); - Assert.Single(buckets); - Assert.Equal("bucket-a", buckets[0].Name); + Assert.Equal(2, buckets.Count); + Assert.Contains(buckets, bucket => bucket.Name == "bucket-a"); + Assert.Contains(buckets, bucket => bucket.Name == "bucket-global"); } [Fact] @@ -86,14 +131,46 @@ public void IgnoreQueryFilters_ReturnsAll() } [Fact] - public void EmptyWorkspaceId_ReturnsAll() + public void EmptyWorkspaceId_HidesTenantScopedDocuments() { - // When workspace context is empty, all rows are visible (backward compat) using var ctx = CreateContext(string.Empty); - var allDocs = ctx.Documents.ToList(); + var docs = ctx.Documents.ToList(); - Assert.Equal(2, allDocs.Count); + Assert.Empty(docs); + } + + [Fact] + public void EmptyWorkspaceId_ReturnsOnlyGlobalToolBuckets() + { + using var ctx = CreateContext(string.Empty); + + var buckets = ctx.ToolBuckets.ToList(); + + Assert.Single(buckets); + Assert.Equal("bucket-global", buckets[0].Name); + } + + [Fact] + public void EmptyWorkspaceId_ReturnsOnlyGlobalAgentDefinitions() + { + using var ctx = CreateContext(string.Empty); + + var agents = ctx.AgentDefinitions.OrderBy(a => a.Id).ToList(); + + Assert.Single(agents); + Assert.Equal("agent-global", agents[0].Id); + } + + [Fact] + public void WorkspaceFilter_IncludesGlobalAgentDefinitions() + { + using var ctx = CreateContext(@"C:\ws\alpha"); + + var agents = ctx.AgentDefinitions.OrderBy(a => a.Id).ToList(); + + Assert.Equal(2, agents.Count); + Assert.Equal(["agent-alpha", "agent-global"], agents.Select(a => a.Id).ToArray()); } [Fact] diff --git a/tests/McpServer.Support.Mcp.Tests/TestSupport/TestLogger.cs b/tests/McpServer.Support.Mcp.Tests/TestSupport/TestLogger.cs new file mode 100644 index 00000000..75f624dd --- /dev/null +++ b/tests/McpServer.Support.Mcp.Tests/TestSupport/TestLogger.cs @@ -0,0 +1,40 @@ +using Microsoft.Extensions.Logging; + +namespace McpServer.Support.Mcp.Tests.TestSupport; + +internal readonly record struct TestLogEntry(LogLevel Level, string Message, Exception? Exception); + +internal sealed class TestLogger : ILogger +{ + private readonly List _entries = new(); + + public IReadOnlyList Entries => _entries; + + public IDisposable BeginScope(TState state) + where TState : notnull + { + return NullScope.Instance; + } + + public bool IsEnabled(LogLevel logLevel) => true; + + public void Log( + LogLevel logLevel, + EventId eventId, + TState state, + Exception? exception, + Func formatter) + { + ArgumentNullException.ThrowIfNull(formatter); + _entries.Add(new TestLogEntry(logLevel, formatter(state, exception), exception)); + } + + private sealed class NullScope : IDisposable + { + public static NullScope Instance { get; } = new(); + + public void Dispose() + { + } + } +} diff --git a/tests/McpServer.Support.Mcp.Tests/Web/PairingHtmlRendererTests.cs b/tests/McpServer.Support.Mcp.Tests/Web/PairingHtmlRendererTests.cs index 81f42ab2..eb580d8d 100644 --- a/tests/McpServer.Support.Mcp.Tests/Web/PairingHtmlRendererTests.cs +++ b/tests/McpServer.Support.Mcp.Tests/Web/PairingHtmlRendererTests.cs @@ -22,7 +22,7 @@ public async Task RenderLoginPageAsync_TemplateInStore_SubstitutesErrorBanner() .Returns(new PromptTemplate { Id = PairingHtmlRenderer.LoginPageId, Title = "test", Category = "system", Content = template }); var renderer = new PairingHtmlRenderer(_templateService, _logger); - var result = await renderer.RenderLoginPageAsync(error: false); + var result = await renderer.RenderLoginPageAsync(); Assert.Equal("
", result); } @@ -35,7 +35,7 @@ public async Task RenderLoginPageAsync_WithError_InsertsErrorBanner() .Returns(new PromptTemplate { Id = PairingHtmlRenderer.LoginPageId, Title = "test", Category = "system", Content = template }); var renderer = new PairingHtmlRenderer(_templateService, _logger); - var result = await renderer.RenderLoginPageAsync(error: true); + var result = await renderer.RenderLoginPageAsync("Invalid username or password."); Assert.Contains("Invalid username or password", result); } diff --git a/tests/McpServer.Todo.Validation/TodoEndpointFixture.cs b/tests/McpServer.Todo.Validation/TodoEndpointFixture.cs index 07aa53a3..943454ad 100644 --- a/tests/McpServer.Todo.Validation/TodoEndpointFixture.cs +++ b/tests/McpServer.Todo.Validation/TodoEndpointFixture.cs @@ -32,7 +32,7 @@ public sealed class TodoEndpointFixture : IDisposable public TodoEndpointFixture() { Client = new HttpClient { BaseAddress = new Uri(BaseUrl) }; - var apiKey = GetDefaultApiKeyAsync(Client).GetAwaiter().GetResult(); + var apiKey = ResolvePreferredApiKeyAsync(Client).GetAwaiter().GetResult(); if (!string.IsNullOrWhiteSpace(apiKey)) { Client.DefaultRequestHeaders.Remove("X-Api-Key"); @@ -40,6 +40,68 @@ public TodoEndpointFixture() } } + private static async Task ResolvePreferredApiKeyAsync(HttpClient client) + { + var explicitKey = Environment.GetEnvironmentVariable("MCPSERVER_APIKEY"); + if (!string.IsNullOrWhiteSpace(explicitKey)) + return explicitKey; + + var fullKey = TryReadApiKeyFromSessionState() ?? TryReadApiKeyFromMarkerFile(); + if (!string.IsNullOrWhiteSpace(fullKey)) + return fullKey; + + return await GetDefaultApiKeyAsync(client).ConfigureAwait(false); + } + + private static string? TryReadApiKeyFromSessionState() + { + var sessionPath = FindFileUpwards(".mcpServer", "session.yaml"); + if (sessionPath is null) + return null; + + try + { + using var document = JsonDocument.Parse(File.ReadAllText(sessionPath)); + return document.RootElement.TryGetProperty("apiKey", out var apiKeyElement) + ? apiKeyElement.GetString() + : null; + } + catch + { + return null; + } + } + + private static string? TryReadApiKeyFromMarkerFile() + { + var markerPath = FindFileUpwards("AGENTS-README-FIRST.yaml"); + if (markerPath is null) + return null; + + foreach (var line in File.ReadLines(markerPath)) + { + if (line.StartsWith("apiKey:", StringComparison.OrdinalIgnoreCase)) + return line["apiKey:".Length..].Trim(); + } + + return null; + } + + private static string? FindFileUpwards(params string[] pathSegments) + { + var current = new DirectoryInfo(AppContext.BaseDirectory); + while (current is not null) + { + var candidate = Path.Combine([current.FullName, .. pathSegments]); + if (File.Exists(candidate)) + return candidate; + + current = current.Parent; + } + + return null; + } + private static async Task GetDefaultApiKeyAsync(HttpClient client) { using var response = await client.GetAsync("/api-key").ConfigureAwait(false); From 89f3e4bc15845361c2dbb8e58a96e31193bdca55 Mon Sep 17 00:00:00 2001 From: sharpninja Date: Sat, 21 Mar 2026 01:53:44 -0500 Subject: [PATCH 2/3] Fix workspace-scoped support test fixtures - set explicit workspace discriminators in session-log, ingestor, agent-runtime, and hybrid-search test fixtures - align full support-suite tests with multi-tenant query-filter behavior verified in CI Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../SessionLogIngestorImportTests.cs | 41 ++++++++++--------- .../Services/AgentServiceRuntimeTests.cs | 4 ++ .../Services/HybridSearchServiceTests.cs | 3 ++ .../SessionLogServiceAgentLinkTests.cs | 3 ++ .../Services/SessionLogServiceTests.cs | 3 ++ 5 files changed, 34 insertions(+), 20 deletions(-) diff --git a/tests/McpServer.Support.Mcp.Tests/Ingestion/SessionLogIngestorImportTests.cs b/tests/McpServer.Support.Mcp.Tests/Ingestion/SessionLogIngestorImportTests.cs index a8b245f6..c0a6dc8f 100644 --- a/tests/McpServer.Support.Mcp.Tests/Ingestion/SessionLogIngestorImportTests.cs +++ b/tests/McpServer.Support.Mcp.Tests/Ingestion/SessionLogIngestorImportTests.cs @@ -28,6 +28,7 @@ public SessionLogIngestorImportTests() _service = new SessionLogService(_db, NullLogger.Instance); _tempDir = Path.Combine(Path.GetTempPath(), $"fwh-ingestor-{Guid.NewGuid():N}"); Directory.CreateDirectory(Path.Combine(_tempDir, "docs", "sessions")); + _db.OverrideWorkspaceId(_tempDir); } public void Dispose() @@ -46,8 +47,8 @@ public async Task WhenImportingJsonSessionLogThenSessionIsPersisted() Title = "Imported Session", Model = "gpt-4", Started = "2026-02-12T10:00:00Z", - TurnCount = 1, - Turns = + TurnCount = 1, + Turns = [ new UnifiedRequestEntryDto { @@ -63,11 +64,11 @@ public async Task WhenImportingJsonSessionLogThenSessionIsPersisted() var result = await ingestor.ImportToSessionLogTablesAsync().ConfigureAwait(true); Assert.Equal(1, result.Imported); - var stored = await _db.SessionLogs.Include(s => s.Turns).FirstOrDefaultAsync(s => s.SessionId == "import-1").ConfigureAwait(true); + var stored = await _db.SessionLogs.Include(s => s.Turns).FirstOrDefaultAsync(s => s.SessionId == "import-1").ConfigureAwait(true); Assert.NotNull(stored); Assert.Equal("Copilot", stored!.SourceType); Assert.Equal("Imported Session", stored.Title); - Assert.Single(stored.Turns); + Assert.Single(stored.Turns); Assert.NotNull(stored.SourceFilePath); Assert.EndsWith("copilot-test.json", stored.SourceFilePath!); Assert.NotNull(stored.ContentHash); @@ -83,7 +84,7 @@ public async Task WhenImportingWithStringWorkspaceThenWorkspaceIsHandled() "sessionId": "ws-string", "title": "String Workspace", "workspace": "E:\\github\\FunWasHad", - "turnCount": 0 + "turnCount": 0 } """; File.WriteAllText(Path.Combine(_tempDir, "docs", "sessions", "cursor-ws.json"), json); @@ -110,7 +111,7 @@ public async Task WhenImportingWithObjectWorkspaceThenWorkspaceFieldsArePersiste "repository": "sharpninja/FunWasHad", "branch": "develop" }, - "turnCount": 0 + "turnCount": 0 } """; File.WriteAllText(Path.Combine(_tempDir, "docs", "sessions", "copilot-ws.json"), json); @@ -128,7 +129,7 @@ public async Task WhenImportingWithObjectWorkspaceThenWorkspaceFieldsArePersiste [Fact] public async Task WhenImportingMissingSourceTypeThenFileIsSkipped() { - var json = """{ "sessionId": "no-source", "turnCount": 0 }"""; + var json = """{ "sessionId": "no-source", "turnCount": 0 }"""; File.WriteAllText(Path.Combine(_tempDir, "docs", "sessions", "bad.json"), json); var ingestor = CreateIngestor(); @@ -148,7 +149,7 @@ public async Task WhenImportingMultipleFilesThenAllAreImported() SourceType = "Cursor", SessionId = $"multi-{i}", Title = $"Multi {i}", - TurnCount = 0 + TurnCount = 0 }); } @@ -168,7 +169,7 @@ public async Task WhenReimportingSameFileThenSessionIsUpserted() SourceType = "Copilot", SessionId = "upsert-1", Title = "Original", - TurnCount = 0 + TurnCount = 0 }); var ingestor = CreateIngestor(); @@ -180,7 +181,7 @@ public async Task WhenReimportingSameFileThenSessionIsUpserted() SourceType = "Copilot", SessionId = "upsert-1", Title = "Updated", - TurnCount = 0 + TurnCount = 0 }); var result = await ingestor.ImportToSessionLogTablesAsync().ConfigureAwait(true); @@ -199,7 +200,7 @@ public async Task WhenFileIsUnchangedThenImportSkipsIt() SourceType = "Cursor", SessionId = "unchanged-1", Title = "Stable", - TurnCount = 0 + TurnCount = 0 }); var ingestor = CreateIngestor(); @@ -222,7 +223,7 @@ public async Task WhenFileChangedThenImportUpdatesIt() SourceType = "Copilot", SessionId = "changing-1", Title = "V1", - TurnCount = 0 + TurnCount = 0 }); var ingestor = CreateIngestor(); @@ -234,7 +235,7 @@ public async Task WhenFileChangedThenImportUpdatesIt() SourceType = "Copilot", SessionId = "changing-1", Title = "V2", - TurnCount = 0 + TurnCount = 0 }); var result = await ingestor.ImportToSessionLogTablesAsync().ConfigureAwait(true); @@ -252,14 +253,14 @@ public async Task WhenMixOfChangedAndUnchangedThenOnlyChangedAreImported() SourceType = "Cursor", SessionId = "stable-1", Title = "Stable", - TurnCount = 0 + TurnCount = 0 }); WriteSessionFile("evolving.json", new UnifiedSessionLogDto { SourceType = "Cursor", SessionId = "evolving-1", Title = "V1", - TurnCount = 0 + TurnCount = 0 }); var ingestor = CreateIngestor(); @@ -272,7 +273,7 @@ public async Task WhenMixOfChangedAndUnchangedThenOnlyChangedAreImported() SourceType = "Cursor", SessionId = "evolving-1", Title = "V2", - TurnCount = 0 + TurnCount = 0 }); var second = await ingestor.ImportToSessionLogTablesAsync().ConfigureAwait(true); @@ -287,7 +288,7 @@ private SessionLogIngestor CreateIngestor() RepoRoot = _tempDir, SessionsPath = "docs/sessions" }); - return new SessionLogIngestor(new Chunker(), opts, new WorkspaceContext(), _service, NullLogger.Instance); + return new SessionLogIngestor(new Chunker(), opts, new WorkspaceContext(), _service, NullLogger.Instance); } private void WriteSessionFile(string filename, UnifiedSessionLogDto dto) @@ -327,12 +328,12 @@ Tested Markdown import pipeline. Assert.True(result.Imported >= 1); var stored = await _db.SessionLogs - .Include(s => s.Turns) + .Include(s => s.Turns) .FirstOrDefaultAsync(s => s.SourceType == "copilot") .ConfigureAwait(true); Assert.NotNull(stored); Assert.Contains("MD Import Test", stored!.Title, StringComparison.Ordinal); - Assert.NotEmpty(stored.Turns); + Assert.NotEmpty(stored.Turns); } [Fact] @@ -393,7 +394,7 @@ public async Task WhenImportingJsonAndMarkdownThenBothAreProcessed() SourceType = "Cursor", SessionId = "json-coexist-1", Title = "JSON Session", - TurnCount = 0 + TurnCount = 0 }); var md = """ diff --git a/tests/McpServer.Support.Mcp.Tests/Services/AgentServiceRuntimeTests.cs b/tests/McpServer.Support.Mcp.Tests/Services/AgentServiceRuntimeTests.cs index c6addc7b..1251ac14 100644 --- a/tests/McpServer.Support.Mcp.Tests/Services/AgentServiceRuntimeTests.cs +++ b/tests/McpServer.Support.Mcp.Tests/Services/AgentServiceRuntimeTests.cs @@ -61,6 +61,7 @@ public void Dispose() public async Task LaunchAgentAsync_EnabledConfig_DelegatesToProcessManager() { var workspacePath = Path.GetFullPath(Path.Combine(Path.GetTempPath(), "agent-service-launch")); + _db.OverrideWorkspaceId(workspacePath); SeedDefinitionAndWorkspace(workspacePath, enabled: true, banned: false, launchCommand: "agent --workspace {workspacePath} --id {agentId}"); _processManager.LaunchAsync(Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()) .Returns(Task.FromResult(new AgentProcessInfo @@ -86,6 +87,7 @@ await _processManager.Received(1).LaunchAsync( public async Task LaunchAgentAsync_BannedAgent_ThrowsInvalidOperationException() { var workspacePath = Path.GetFullPath(Path.Combine(Path.GetTempPath(), "agent-service-banned")); + _db.OverrideWorkspaceId(workspacePath); SeedDefinitionAndWorkspace(workspacePath, enabled: true, banned: true, launchCommand: "agent"); await Assert.ThrowsAsync(() => _sut.LaunchAgentAsync(workspacePath, "planner")).ConfigureAwait(true); @@ -95,6 +97,7 @@ public async Task LaunchAgentAsync_BannedAgent_ThrowsInvalidOperationException() [Fact] public async Task LaunchAgentAsync_MissingWorkspaceConfig_ThrowsInvalidOperationException() { + _db.OverrideWorkspaceId(Path.GetFullPath("C:/missing-ws")); _db.AgentDefinitions.Add(new AgentDefinitionEntity { Id = "planner", @@ -117,6 +120,7 @@ public async Task LaunchAgentAsync_MissingWorkspaceConfig_ThrowsInvalidOperation public async Task StopAgentAsync_RunningAgent_DelegatesToProcessManager() { var workspacePath = Path.GetFullPath(Path.Combine(Path.GetTempPath(), "agent-service-stop")); + _db.OverrideWorkspaceId(workspacePath); SeedDefinitionAndWorkspace(workspacePath, enabled: true, banned: false, launchCommand: "agent"); _processManager.StopAsync(workspacePath, "planner", Arg.Any()) .Returns(Task.FromResult(true)); diff --git a/tests/McpServer.Support.Mcp.Tests/Services/HybridSearchServiceTests.cs b/tests/McpServer.Support.Mcp.Tests/Services/HybridSearchServiceTests.cs index 45f76c66..88ec5647 100644 --- a/tests/McpServer.Support.Mcp.Tests/Services/HybridSearchServiceTests.cs +++ b/tests/McpServer.Support.Mcp.Tests/Services/HybridSearchServiceTests.cs @@ -13,6 +13,8 @@ namespace McpServer.Support.Mcp.Tests.Services; /// TR-PLANNED-013: Unit tests for HybridSearchService RRF blending and degradation. public sealed class HybridSearchServiceTests : IAsyncLifetime { + private const string WorkspacePath = @"E:\tests\hybrid-search"; + private McpDbContext _db = null!; private IContextSearchService _fts5 = null!; private IVectorIndexService _vectorIndex = null!; @@ -25,6 +27,7 @@ public async ValueTask InitializeAsync() .Options; _db = new McpDbContext(options); await _db.Database.EnsureCreatedAsync().ConfigureAwait(true); + _db.OverrideWorkspaceId(WorkspacePath); // Seed data var doc = new ContextDocumentEntity diff --git a/tests/McpServer.Support.Mcp.Tests/Services/SessionLogServiceAgentLinkTests.cs b/tests/McpServer.Support.Mcp.Tests/Services/SessionLogServiceAgentLinkTests.cs index 109fd4dd..e1c4ed0e 100644 --- a/tests/McpServer.Support.Mcp.Tests/Services/SessionLogServiceAgentLinkTests.cs +++ b/tests/McpServer.Support.Mcp.Tests/Services/SessionLogServiceAgentLinkTests.cs @@ -13,6 +13,8 @@ namespace McpServer.Support.Mcp.Tests.Services; /// public sealed class SessionLogServiceAgentLinkTests : IDisposable { + private const string WorkspacePath = @"E:\tests\sessionlog-agent-link"; + private readonly McpDbContext _db; private readonly SessionLogService _sut; @@ -23,6 +25,7 @@ public SessionLogServiceAgentLinkTests() .Options; _db = new McpDbContext(options); _db.Database.EnsureCreated(); + _db.OverrideWorkspaceId(WorkspacePath); _sut = new SessionLogService(_db, NullLogger.Instance); } diff --git a/tests/McpServer.Support.Mcp.Tests/Services/SessionLogServiceTests.cs b/tests/McpServer.Support.Mcp.Tests/Services/SessionLogServiceTests.cs index ab3e29bb..b2ea6607 100644 --- a/tests/McpServer.Support.Mcp.Tests/Services/SessionLogServiceTests.cs +++ b/tests/McpServer.Support.Mcp.Tests/Services/SessionLogServiceTests.cs @@ -13,6 +13,8 @@ namespace McpServer.Support.Mcp.Tests.Services; /// TR-PLANNED-013: Unit tests for SessionLogService submit and query (MVP-SUPPORT-011). public sealed class SessionLogServiceTests : IDisposable { + private const string WorkspacePath = @"E:\tests\sessionlog-service"; + private readonly McpDbContext _db; private readonly IChangeEventBus _eventBus; private readonly SessionLogService _sut; @@ -24,6 +26,7 @@ public SessionLogServiceTests() .Options; _db = new McpDbContext(options); _db.Database.EnsureCreated(); + _db.OverrideWorkspaceId(WorkspacePath); _eventBus = Substitute.For(); _sut = new SessionLogService(_db, NullLogger.Instance, _eventBus); } From 0b506e87829d6e128728b681089d56d86326d77d Mon Sep 17 00:00:00 2001 From: sharpninja Date: Sat, 21 Mar 2026 02:00:38 -0500 Subject: [PATCH 3/3] Update MCP-TODO-006 completion note - record that phases 0 through 9 are complete locally - capture PR #31 green status and release validation summary Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- docs/Project/TODO.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/Project/TODO.yaml b/docs/Project/TODO.yaml index 8fab4e1a..51321f57 100644 --- a/docs/Project/TODO.yaml +++ b/docs/Project/TODO.yaml @@ -1678,7 +1678,7 @@ infrastructure: - id: MCP-TODO-006 title: Remediate high-signal workspace code review findings estimate: L - note: 'Status 2026-03-21: phases 0 through 8 are now complete locally. Phase 5 removed silent interaction-log and change-event drops in favor of explicit rejection plus warning telemetry, Phase 6 added SQLite-authoritative TODO projection status/repair surfaces with focused service/client/integration coverage, Phase 7 hardened the pairing flow with brute-force throttling plus retry-after signaling, removed the blocking tunnel shutdown hook, replaced hosted-agent sync-over-async disposal bridges with best-effort synchronous cleanup, and extended managed Windows deployment validation to non-service launches when the deployment manifest is present, and Phase 8 moved configuration writes onto shared serialized temp-file-plus-atomic-replace semantics for YAML patching and global prompt updates with focused concurrency/interruption coverage. Phase 9 validation matrix is green locally: `dotnet build src\McpServer.Support.Mcp -c Debug`, `tests\McpServer.Support.Mcp.Tests` filtered remediation slice (62 passed), `tests\McpServer.Client.Tests` filtered `TodoClientTests` slice (16 passed), and `tests\McpServer.Support.Mcp.IntegrationTests` filtered remediation slice (40 passed). Immediate next step: decide how to handle the existing mixed dirty worktree before any commit, push, redeploy, or CI follow-through.' + note: 'Status 2026-03-21: phases 0 through 9 are complete locally and the remediation branch is ready for merge/release follow-through. The core remediation set was committed as `046c80d24115f832d57e0c718d9936984a28b16e`, the follow-up workspace-scope test-fixture fix was committed as `89f3e4bc15845361c2dbb8e58a96e31193bdca55`, and draft PR `#31` (`codex/mcp-todo-006-remediation` -> `develop`) is green on workflow run `#253`, including `build-test-publish` and `windows-msix`. Local validation is also green, including `dotnet test tests\McpServer.Support.Mcp.Tests -c Release` with 501 total, 495 passed, 6 skipped, and 0 failed. Remaining step, if desired, is merge/deploy follow-through outside this remediation implementation slice.' done: false description: - Execute a coordinated remediation program that hardens the workspace against security flaws, data-loss conditions, startup auth bypasses, and runtime/operational hazards uncovered by repository-wide review.