diff --git a/backend/src/Taskdeck.Api/Mcp/BoardResources.cs b/backend/src/Taskdeck.Api/Mcp/BoardResources.cs index f63221ef7..7131b6d4d 100644 --- a/backend/src/Taskdeck.Api/Mcp/BoardResources.cs +++ b/backend/src/Taskdeck.Api/Mcp/BoardResources.cs @@ -3,6 +3,7 @@ using ModelContextProtocol.Server; using Taskdeck.Application.Interfaces; using Taskdeck.Application.Services; +using Taskdeck.Domain.Common; namespace Taskdeck.Api.Mcp; @@ -59,7 +60,7 @@ public async Task ListBoards() // List board summaries scoped to this user. var listResult = await _boardService.ListBoardsAsync(userId, searchText: null, includeArchived: false); if (!listResult.IsSuccess) - throw new InvalidOperationException($"MCP: failed to list boards: {listResult.ErrorMessage}"); + throw new InvalidOperationException($"MCP: failed to list boards: {PublicFailureMessage(listResult)}"); var boardSummaries = new List(); foreach (var boardDto in listResult.Value) @@ -108,7 +109,7 @@ public async Task GetBoardDetail(string boardId) var detailResult = await _boardService.GetBoardDetailAsync(boardGuid, userId); if (!detailResult.IsSuccess) - throw new InvalidOperationException($"MCP: failed to get board detail: {detailResult.ErrorMessage}"); + throw new InvalidOperationException($"MCP: failed to get board detail: {PublicFailureMessage(detailResult)}"); var detail = detailResult.Value; @@ -159,7 +160,7 @@ public async Task GetColumnCards(string boardId, string columnId) // Verify user can access this board var boardResult = await _boardService.GetBoardDetailAsync(boardGuid, userId); if (!boardResult.IsSuccess) - throw new InvalidOperationException($"MCP: failed to access board: {boardResult.ErrorMessage}"); + throw new InvalidOperationException($"MCP: failed to access board: {PublicFailureMessage(boardResult)}"); var column = boardResult.Value.Columns.FirstOrDefault(c => c.Id == columnGuid); if (column == null) @@ -167,7 +168,7 @@ public async Task GetColumnCards(string boardId, string columnId) var cardsResult = await _cardService.SearchCardsAsync(boardGuid, columnId: columnGuid); if (!cardsResult.IsSuccess) - throw new InvalidOperationException($"MCP: failed to get cards: {cardsResult.ErrorMessage}"); + throw new InvalidOperationException($"MCP: failed to get cards: {PublicFailureMessage(cardsResult)}"); var cards = cardsResult.Value.OrderBy(c => c.Position).Select(c => new { @@ -208,11 +209,11 @@ public async Task GetCardDetail(string boardId, string cardId) // Verify user can access this board var boardResult = await _boardService.GetBoardDetailAsync(boardGuid, userId); if (!boardResult.IsSuccess) - throw new InvalidOperationException($"MCP: failed to access board: {boardResult.ErrorMessage}"); + throw new InvalidOperationException($"MCP: failed to access board: {PublicFailureMessage(boardResult)}"); var cardsResult = await _cardService.SearchCardsAsync(boardGuid); if (!cardsResult.IsSuccess) - throw new InvalidOperationException($"MCP: failed to search cards: {cardsResult.ErrorMessage}"); + throw new InvalidOperationException($"MCP: failed to search cards: {PublicFailureMessage(cardsResult)}"); var card = cardsResult.Value.FirstOrDefault(c => c.Id == cardGuid); if (card == null) @@ -258,11 +259,11 @@ public async Task GetBoardLabels(string boardId) // Verify user can access this board var boardResult = await _boardService.GetBoardDetailAsync(boardGuid, userId); if (!boardResult.IsSuccess) - throw new InvalidOperationException($"MCP: failed to access board: {boardResult.ErrorMessage}"); + throw new InvalidOperationException($"MCP: failed to access board: {PublicFailureMessage(boardResult)}"); var labelsResult = await _labelService.GetLabelsByBoardIdAsync(boardGuid); if (!labelsResult.IsSuccess) - throw new InvalidOperationException($"MCP: failed to get labels: {labelsResult.ErrorMessage}"); + throw new InvalidOperationException($"MCP: failed to get labels: {PublicFailureMessage(labelsResult)}"); var labels = labelsResult.Value.Select(l => new { @@ -279,4 +280,7 @@ public async Task GetBoardLabels(string boardId) totalCount = labelsResult.Value.Count() }, SerializerOptions); } + + private static string PublicFailureMessage(Result result) => + SensitiveDataRedactor.SanitizeLlmFailureMessage(result.ErrorCode, result.ErrorMessage); } diff --git a/backend/src/Taskdeck.Api/Mcp/CaptureResources.cs b/backend/src/Taskdeck.Api/Mcp/CaptureResources.cs index f4d48bd13..9883feb93 100644 --- a/backend/src/Taskdeck.Api/Mcp/CaptureResources.cs +++ b/backend/src/Taskdeck.Api/Mcp/CaptureResources.cs @@ -3,6 +3,7 @@ using Taskdeck.Application.DTOs; using Taskdeck.Application.Interfaces; using Taskdeck.Application.Services; +using Taskdeck.Domain.Common; namespace Taskdeck.Api.Mcp; @@ -38,7 +39,7 @@ public async Task ListCaptures() var result = await _captureService.ListAsync(userId, new CaptureListFilterDto()); if (!result.IsSuccess) - throw new InvalidOperationException($"MCP: failed to list captures: {result.ErrorMessage}"); + throw new InvalidOperationException($"MCP: failed to list captures: {PublicFailureMessage(result)}"); var captures = result.Value.Select(c => new { @@ -75,7 +76,7 @@ public async Task GetCaptureDetail(string captureId) var result = await _captureService.GetByIdAsync(userId, captureGuid); if (!result.IsSuccess) - throw new InvalidOperationException($"MCP: failed to get capture: {result.ErrorMessage}"); + throw new InvalidOperationException($"MCP: failed to get capture: {PublicFailureMessage(result)}"); var c = result.Value; @@ -94,4 +95,7 @@ public async Task GetCaptureDetail(string captureId) errorMessage = c.ErrorMessage }, BoardResources.SerializerOptions); } + + private static string PublicFailureMessage(Result result) => + SensitiveDataRedactor.SanitizeLlmFailureMessage(result.ErrorCode, result.ErrorMessage); } diff --git a/backend/tests/Taskdeck.Api.Tests/BoardResourcesErrorSafetyTests.cs b/backend/tests/Taskdeck.Api.Tests/BoardResourcesErrorSafetyTests.cs new file mode 100644 index 000000000..8dd9315dd --- /dev/null +++ b/backend/tests/Taskdeck.Api.Tests/BoardResourcesErrorSafetyTests.cs @@ -0,0 +1,132 @@ +using FluentAssertions; +using Moq; +using Taskdeck.Api.Mcp; +using Taskdeck.Application.Interfaces; +using Taskdeck.Application.Services; +using Taskdeck.Domain.Common; +using Taskdeck.Domain.Enums; +using Taskdeck.Domain.Exceptions; +using Xunit; + +namespace Taskdeck.Api.Tests; + +public class BoardResourcesErrorSafetyTests +{ + private const string HostileError = + "Bearer tdsk_test_secret C:\\Users\\alice\\taskdeck.db " + + "SQLite UNIQUE constraint failed: Boards.Name https://provider.example/v1/internal"; + + [Fact] + public async Task ListBoards_UnexpectedAuthorizationFailure_UsesGenericMessage() + { + var userId = Guid.NewGuid(); + var boardId = Guid.NewGuid(); + var (resources, unitOfWork, authorization) = CreateResources(userId); + var boardRepository = new Mock(MockBehavior.Strict); + unitOfWork + .SetupGet(value => value.Boards) + .Returns(boardRepository.Object); + boardRepository + .Setup(repository => repository.SearchIdsAsync(null, false, It.IsAny())) + .ReturnsAsync(new[] { boardId }); + authorization + .Setup(service => service.GetReadableBoardIdsAsync( + userId, + It.IsAny>(), + It.IsAny())) + .ReturnsAsync(Result.Failure>(ErrorCodes.UnexpectedError, HostileError)); + + var action = () => resources.ListBoards(); + + var exception = (await action.Should().ThrowAsync()).Which; + exception.Message.Should().Be( + $"MCP: failed to list boards: {SensitiveDataRedactor.GenericUnexpectedFailureMessage}"); + exception.Message.Should().NotContain(HostileError); + authorization.VerifyAll(); + boardRepository.VerifyAll(); + } + + [Theory] + [InlineData("detail")] + [InlineData("column")] + [InlineData("card")] + [InlineData("labels")] + public async Task BoardAccess_UnexpectedFailure_UsesGenericMessage(string resource) + { + var userId = Guid.NewGuid(); + var boardId = Guid.NewGuid(); + var (resources, _, authorization) = CreateResources(userId); + authorization + .Setup(service => service.CanReadBoardAsync(userId, boardId)) + .ReturnsAsync(Result.Failure(ErrorCodes.UnexpectedError, HostileError)); + + Func> action = resource switch + { + "detail" => () => resources.GetBoardDetail(boardId.ToString()), + "column" => () => resources.GetColumnCards(boardId.ToString(), Guid.NewGuid().ToString()), + "card" => () => resources.GetCardDetail(boardId.ToString(), Guid.NewGuid().ToString()), + "labels" => () => resources.GetBoardLabels(boardId.ToString()), + _ => throw new ArgumentOutOfRangeException(nameof(resource), resource, null) + }; + + var exception = (await action.Should().ThrowAsync()).Which; + var prefix = resource switch + { + "detail" => "MCP: failed to get board detail: ", + "column" => "MCP: failed to access board: ", + "card" => "MCP: failed to access board: ", + "labels" => "MCP: failed to access board: ", + _ => throw new ArgumentOutOfRangeException(nameof(resource), resource, null) + }; + exception.Message.Should().Be(prefix + SensitiveDataRedactor.GenericUnexpectedFailureMessage); + authorization.VerifyAll(); + } + + [Fact] + public async Task GetBoardDetail_KnownDomainFailure_PreservesStableMessage() + { + const string stableMessage = "You do not have access to this board."; + var userId = Guid.NewGuid(); + var boardId = Guid.NewGuid(); + var (resources, _, authorization) = CreateResources(userId); + authorization + .Setup(service => service.CanReadBoardAsync(userId, boardId)) + .ReturnsAsync(Result.Failure(ErrorCodes.Forbidden, stableMessage)); + + var action = () => resources.GetBoardDetail(boardId.ToString()); + + var exception = (await action.Should().ThrowAsync()).Which; + exception.Message.Should().Be($"MCP: failed to get board detail: {stableMessage}"); + authorization.VerifyAll(); + } + + private static ( + BoardResources Resources, + Mock UnitOfWork, + Mock Authorization) CreateResources(Guid userId) + { + var unitOfWork = new Mock(MockBehavior.Strict); + var authorization = new Mock(MockBehavior.Strict); + var boardService = new BoardService(unitOfWork.Object, authorization.Object); + var resources = new BoardResources( + boardService, + new ColumnService(unitOfWork.Object), + new CardService(unitOfWork.Object), + new LabelService(unitOfWork.Object), + new FixedUserContextProvider(userId)); + + return (resources, unitOfWork, authorization); + } + + private sealed class FixedUserContextProvider(Guid userId) : IUserContextProvider + { + public Task GetCurrentContextAsync(CancellationToken cancellationToken = default) => + Task.FromResult(new McpUserContext(userId, ApiKeyScope.Full)); + + public Task GetCurrentUserIdAsync(CancellationToken cancellationToken = default) => + Task.FromResult(userId); + + public Task GetUserIdAsync(CancellationToken cancellationToken = default) => + Task.FromResult(userId); + } +} diff --git a/backend/tests/Taskdeck.Api.Tests/CaptureResourcesErrorSafetyTests.cs b/backend/tests/Taskdeck.Api.Tests/CaptureResourcesErrorSafetyTests.cs new file mode 100644 index 000000000..d5a586afa --- /dev/null +++ b/backend/tests/Taskdeck.Api.Tests/CaptureResourcesErrorSafetyTests.cs @@ -0,0 +1,98 @@ +using FluentAssertions; +using Moq; +using Taskdeck.Api.Mcp; +using Taskdeck.Application.DTOs; +using Taskdeck.Application.Interfaces; +using Taskdeck.Application.Services; +using Taskdeck.Domain.Common; +using Taskdeck.Domain.Enums; +using Taskdeck.Domain.Exceptions; +using Xunit; + +namespace Taskdeck.Api.Tests; + +public class CaptureResourcesErrorSafetyTests +{ + private const string HostileError = + "Bearer tdsk_test_secret C:\\Users\\alice\\taskdeck.db " + + "SQLite UNIQUE constraint failed: Users.Email https://provider.example/v1/internal"; + + [Fact] + public async Task ListCaptures_UnexpectedFailure_UsesGenericMessage() + { + var userId = Guid.NewGuid(); + var captureService = new Mock(MockBehavior.Strict); + captureService + .Setup(service => service.ListAsync( + userId, + It.IsAny(), + It.IsAny())) + .ReturnsAsync(Result.Failure>( + ErrorCodes.UnexpectedError, + HostileError)); + + var action = () => CreateResources(captureService.Object, userId).ListCaptures(); + + var exception = (await action.Should().ThrowAsync()).Which; + exception.Message.Should().Be( + $"MCP: failed to list captures: {SensitiveDataRedactor.GenericUnexpectedFailureMessage}"); + exception.Message.Should().NotContain(HostileError); + captureService.VerifyAll(); + } + + [Fact] + public async Task GetCaptureDetail_UnexpectedFailure_UsesGenericMessage() + { + var userId = Guid.NewGuid(); + var captureId = Guid.NewGuid(); + var captureService = new Mock(MockBehavior.Strict); + captureService + .Setup(service => service.GetByIdAsync(userId, captureId, It.IsAny())) + .ReturnsAsync(Result.Failure(ErrorCodes.UnexpectedError, HostileError)); + + var action = () => CreateResources(captureService.Object, userId) + .GetCaptureDetail(captureId.ToString()); + + var exception = (await action.Should().ThrowAsync()).Which; + exception.Message.Should().Be( + $"MCP: failed to get capture: {SensitiveDataRedactor.GenericUnexpectedFailureMessage}"); + exception.Message.Should().NotContain(HostileError); + captureService.VerifyAll(); + } + + [Fact] + public async Task GetCaptureDetail_KnownDomainFailure_PreservesStableMessage() + { + const string stableMessage = "Capture item not found."; + var userId = Guid.NewGuid(); + var captureId = Guid.NewGuid(); + var captureService = new Mock(MockBehavior.Strict); + captureService + .Setup(service => service.GetByIdAsync(userId, captureId, It.IsAny())) + .ReturnsAsync(Result.Failure(ErrorCodes.NotFound, stableMessage)); + + var action = () => CreateResources(captureService.Object, userId) + .GetCaptureDetail(captureId.ToString()); + + var exception = (await action.Should().ThrowAsync()).Which; + exception.Message.Should().Be($"MCP: failed to get capture: {stableMessage}"); + captureService.VerifyAll(); + } + + private static CaptureResources CreateResources( + ICaptureService captureService, + Guid userId) => + new(captureService, new FixedUserContextProvider(userId)); + + private sealed class FixedUserContextProvider(Guid userId) : IUserContextProvider + { + public Task GetCurrentContextAsync(CancellationToken cancellationToken = default) => + Task.FromResult(new McpUserContext(userId, ApiKeyScope.Full)); + + public Task GetCurrentUserIdAsync(CancellationToken cancellationToken = default) => + Task.FromResult(userId); + + public Task GetUserIdAsync(CancellationToken cancellationToken = default) => + Task.FromResult(userId); + } +} diff --git a/docs/security/SECURITY_LOGGING_REDACTION.md b/docs/security/SECURITY_LOGGING_REDACTION.md index bd0e198f1..378f1c07f 100644 --- a/docs/security/SECURITY_LOGGING_REDACTION.md +++ b/docs/security/SECURITY_LOGGING_REDACTION.md @@ -39,6 +39,10 @@ It applies to API middleware, SignalR transport request logging, queue/worker lo `UnexpectedError` with the same stable generic failure message before returning tool output or throwing a resource error. Known domain result messages stay specific, and diagnostic logging remains owned by the service and MCP operation boundaries. +- MCP capture and board resources apply the same boundary rule to every interpolated failed + `Result`: `UnexpectedError` becomes the stable generic failure message, while known domain + messages remain specific. Persisted `CaptureItemDto.ErrorMessage` values, invalid caller IDs, and + arbitrary thrown exceptions are separate contracts and are not rewritten by this rule. - Capture-source validation errors use generic wording (`Invalid capture source value`) instead of reflecting the untrusted source string. - Opt-in Sentry keeps server-side exception tracking and the existing event/breadcrumb scrubbing, but does not decorate the registered OpenAI, OpenAICompatible, Ollama, or outbound-webhook clients. - The web host enforces `Warning` as the minimum for