Skip to content

Conversation

nirinchev
Copy link
Collaborator

Proposed changes

Part of MCP-197 - this adds the untrusted data wrapper around the logs response and updates the tests to reflect the new format.

@nirinchev nirinchev requested a review from a team as a code owner September 11, 2025 13:05
@Copilot Copilot AI review requested due to automatic review settings September 11, 2025 13:05
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR updates the MongoDB logs tool to wrap log response data in an untrusted data wrapper as part of security improvements (MCP-197). The change ensures that log data retrieved from MongoDB is properly marked as untrusted content when returned to users.

  • Replaced direct content array with formatUntrustedData wrapper for log responses
  • Updated test assertions to handle the new untrusted data format
  • Added helper function import for extracting data from untrusted content

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
src/tools/mongodb/metadata/logs.ts Wraps log data in untrusted data format and simplifies response structure
tests/integration/tools/mongodb/metadata/logs.test.ts Updates test expectations to handle untrusted data wrapper format

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

expect(elements).toHaveLength(2);
expect(elements[1]?.text).toContain("<untrusted-user-data-");

const logs = getDataFromUntrustedContent(elements[1]?.text ?? "").split("\n");
Copy link

Copilot AI Sep 11, 2025

Choose a reason for hiding this comment

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

The split('\n') operation may produce empty strings at the end if the logs data ends with newlines. This could cause the length checks and JSON parsing to fail. Consider filtering out empty strings: getDataFromUntrustedContent(elements[1]?.text ?? "").split("\n").filter(line => line.trim())

Suggested change
const logs = getDataFromUntrustedContent(elements[1]?.text ?? "").split("\n");
const logs = getDataFromUntrustedContent(elements[1]?.text ?? "")
.split("\n")
.filter(line => line.trim());

Copilot uses AI. Check for mistakes.

expect(elements).toHaveLength(2);
expect(elements[1]?.text).toContain("<untrusted-user-data-");

const logs = getDataFromUntrustedContent(elements[1]?.text ?? "").split("\n");
Copy link

Copilot AI Sep 11, 2025

Choose a reason for hiding this comment

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

Same issue as above - the split('\n') operation may produce empty strings that could break JSON parsing. Consider filtering out empty strings: getDataFromUntrustedContent(elements[1]?.text ?? "").split("\n").filter(line => line.trim())

Suggested change
const logs = getDataFromUntrustedContent(elements[1]?.text ?? "").split("\n");
const logs = getDataFromUntrustedContent(elements[1]?.text ?? "")
.split("\n")
.filter(line => line.trim());

Copilot uses AI. Check for mistakes.

Copy link
Collaborator

@himanshusinghs himanshusinghs left a comment

Choose a reason for hiding this comment

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

Looks good. Do you have something in mind for export tool? I think we can wrap the data there but we need to check if LLM is then able to follow up on that data for further work.

@nirinchev
Copy link
Collaborator Author

Haven't started on the export yet, will check if the LLM gets confused by the wrapper.

@nirinchev nirinchev merged commit d022c5b into main Sep 11, 2025
23 of 24 checks passed
@nirinchev nirinchev deleted the ni/untrusted-logs branch September 11, 2025 14:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants