Skip to content

Conversation

@ddworken
Copy link
Contributor

@ddworken ddworken commented May 22, 2025

This mitigates the trick of injecting COMMIT and END to bypass the read-only restrictions. I believe this should fully fix the issue, but I added a warning to encourage people to also enforce this through postgres permissions.

Note: This is based on the approach used by @smola here, so thank you Santiago for the idea of using a parameterized query to accomplish this.

@ddworken
Copy link
Contributor Author

@dsp-ant

…ites or other dangerous actions

Co-authored-by: Santiago Mola <santiago.mola@datadoghq.com>
Copy link
Member

@pcarleton pcarleton left a comment

Choose a reason for hiding this comment

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

LGTM

A test would be awesome to verify, but given there aren't tests already, i'm good w/ merging this as is.

@ddworken
Copy link
Contributor Author

Yeah, I had experimented with adding a test for this, but ended up deciding it took a bit too much setup (i.e. setting up docker to run postgres) to be worth it here.

@pcarleton pcarleton merged commit 1f70567 into modelcontextprotocol:main May 29, 2025
25 checks passed
PazerOP referenced this pull request in PazerOP/mcp-template Jul 15, 2025
…ites or other dangerous actions (#1889)

Co-authored-by: Santiago Mola <santiago.mola@datadoghq.com>
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