Repository navigation
Merged
Conversation
Values written into SQL break on quotes and let a crafted value change the query. query_database and /api/query now take an optional parameters object whose values are bound by the driver, never spliced into the text. Placeholders use each provider's syntax (@name, or :name on Oracle). Postgres strings are sent untyped so the server infers dates, uuids and numbers as it does for literals.
…kward values Found by probing all four databases: Oracle 1/3 failed the whole query, NaN/Infinity and Postgres inet threw while serializing, SQL Server geography failed the query, repeated or unnamed columns silently lost values, and a 20 MB value came back whole. An unknown Provider silently became SqlServer. - Values are normalized to plain JSON; one unreadable value becomes a note, not an error - Columns are made unique (column1, id_2) - App:MaxValueLength and App:MaxResultBytes cap values and result size - App:MaxConcurrentQueries bounds queries per database - Each call is logged with timing and row count, never parameter values - Config is validated at startup with every problem listed; a missing password variable disables only its database - Sample config uses read-only agent logins; its Oracle entry had an invalid attribute
…ck; readiness endpoint - GitHub Actions runs the full suite against Postgres, MySQL, SQL Server and Oracle service containers, fails if any test is skipped, builds with -warnaserror, fails on vulnerable packages, and smoke-tests the image - The image runs as the unprivileged app user and has a HEALTHCHECK via the new --healthcheck mode (the base image has no curl) - GET /health/ready connects to each database, runs the login audit and reports per-database status; 503 unless all are ready
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #6, which is stacked on #5. Merge those in order and this diff shrinks to its own commit. Each targets
masterdirectly, so none can land on a feature branch by mistake.CI (
.github/workflows/ci.yml), on pushes tomasterand on every PR:-warnaserrorand fails on vulnerable packages, direct or transitive./health/mcpwithout the token--healthcheckImage:
appuser (uid 1654) instead of root.HEALTHCHECKruns the newsql-mcp --healthcheck. That mode calls/healthon the configured port and exits 0 or 1, since the ASP.NET image has no curl or wget.GET /health/ready: connects to each database, runs the login audit and opens the read-only transaction. It returns 200 when every database is ready, else 503, with the status of each database. It needs the token when one is set, because it names the databases. These checks are kept out of the query log./healthis unchanged: open, liveness only.Verified locally:
podman healthcheck runpasses and the container reportshealthy/health/readyreturns 503 and names each missing password variableThe workflow itself runs for the first time on this PR.