feat: add Excel export via execute_sql_xlsx tool

[ccclucky]

Summary

• Add execute_sql_xlsx MCP tool that executes a SQL query and exports results to an Excel (.xlsx) file
• Create formatter.py with format_to_excel() function (openpyxl-based, auto-adjusts column widths)
• Dynamic tool registration based on access mode: destructiveHint=True (unrestricted) vs readOnlyHint=True (restricted), consistent with execute_sql pattern
• Add max_rows parameter (default 10000) with truncation warning to prevent excessive output
• Fix empty result handling: informational response instead of error response
• Add comprehensive unit tests (12 test cases covering formatter and tool logic)
• Update README tools table

Changes

File	Change
pyproject.toml	Add openpyxl dependency
src/postgres_mcp/formatter.py	New file: Excel formatting logic
src/postgres_mcp/server.py	New execute_sql_xlsx tool + dynamic registration
tests/unit/test_excel_export.py	New file: 12 unit tests
README.md	Document new tool in tools table

Test plan

• pytest tests/unit/test_excel_export.py -v — 12/12 passed
• ruff check — no errors

🤖 Generated with Claude Code

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9be2590b51

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Enforce max_rows during query execution

execute_sql_xlsx fetches the full result set before truncating (rows[:max_rows]), so max_rows does not actually protect the server from large queries. In this codebase, SqlDriver._execute_with_connection materializes all rows with cursor.fetchall() (src/postgres_mcp/sql/sql_driver.py), so a query returning millions of rows can still consume large memory/CPU and fail even when max_rows is small. Apply the row cap while executing (e.g., limit/stream/fetchmany) instead of after full fetch.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Validate max_rows as a positive integer

max_rows currently allows 0 or negative values, and those inputs make row_dicts empty while the next line indexes row_dicts[0], which raises an exception and returns a generic error response. This makes the tool fail for a validly-typed but invalid semantic input; add a ge=1 constraint (or equivalent guard) and return a clear validation error.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Generate unique export filenames

The Excel path is derived from a timestamp with second-level precision, so concurrent/rapid exports in the same second will target the same filename and overwrite each other. This can return a stale/incorrect file to one caller and lose prior output; use a uniqueness source (UUID, monotonic suffix, or tempfile APIs) when constructing the filename.

Useful? React with 👍 / 👎.

[jssmith]

Review — concise

Verdict: Request changes. Good feature, but has a SQL injection vector and design issues to address.

Issues to address

1. LIMIT injection is bypassable via subqueries (blocking)
The regex re.search(r"\bLIMIT\b", sql, re.IGNORECASE) checks if LIMIT appears anywhere in the SQL. A query like SELECT * FROM (SELECT * FROM huge_table LIMIT 1) t would match and skip the outer LIMIT injection, but the outer query is uncapped. Conversely, the presence of LIMIT in a comment or string would also skip injection. Consider using pglast to parse and inject LIMIT at the AST level, or wrap the query as a subquery: SELECT * FROM ({sql}) AS sub LIMIT {max_rows}.

2. import re inside the function (non-blocking)
Move import re to the top of server.py with other imports. Local imports are unnecessary here and inconsistent with the codebase style.

3. RESTRICTED mode bypasses SafeSqlDriver validation (blocking)
In RESTRICTED mode, get_sql_driver() returns a SafeSqlDriver that validates queries. But execute_sql_xlsx calls sql_driver.execute_query() — verify that SafeSqlDriver.execute_query() enforces read-only validation. If it delegates to the base SqlDriver.execute_query() without validation, this tool could execute writes in RESTRICTED mode. The execute_sql tool uses SafeSqlDriver validation; this new tool should too. The readOnlyHint annotation is cosmetic if the driver doesn't enforce it.

4. No file path cleanup mechanism (non-blocking)
Files are written to tempfile.gettempdir()/postgres-mcp-results/ with no cleanup. Long-running servers will accumulate files. Consider adding a TTL or max-file-count cleanup, or documenting that the user is responsible.

5. Files written to server-side temp dir (non-blocking, design)
The Excel file is written to the server's filesystem, not the client's. The agent receives only a text path. Unless the MCP client has filesystem access to the server's temp dir, the file is unreachable. Consider returning the file as a base64-encoded resource, or document this limitation.

Tests

Test coverage is thorough for the formatter and tool handler: file creation, custom/default dirs, column width capping, None values, empty rows, complex type serialization, unique filenames, LIMIT injection/preservation, error handling, max_rows validation.

This review was created by an AI agent (OpenHands) on behalf of @jssmith.

[ccclucky]

Thanks for the review, @jssmith. I pushed b4a1e2b with the requested changes:

1. Replaced the regex-based LIMIT detection with an unconditional outer query wrapper:

   SELECT * FROM (...) AS _postgres_mcp_export LIMIT {max_rows}

   This enforces the cap when the input already contains a top-level or nested LIMIT, and when LIMIT appears in a comment or string literal. Regression tests cover each case.

2. Removed the function-local re import—the regex is no longer needed—and moved the formatter imports to module scope.

3. Verified the restricted-mode path: get_sql_driver() returns SafeSqlDriver, whose execute_query() calls _validate() and always delegates with force_readonly=True. I added a regression test using a data-modifying CTE and verified that the underlying driver is never invoked.

4. Documented that export files are not automatically deleted.

5. Documented, both in the README and the tool response, that the returned path is server-local and requires a shared filesystem for remote or containerized clients.

I also updated uv.lock for the new dependency and resolved the type-checking issues in the new files.

Validation:

• ruff format --check .
• ruff check .
• pyright — 0 errors
• pytest -q — 187 passed, 48 skipped, 1 xfailed

The GitHub Actions run is currently marked action_required and appears to require maintainer approval for the forked workflow.

[jssmith]

Review — concise

Verdict: Approve. Commit b4a1e2b resolves both blocking issues from the previous review.

Blocking issues — resolved

1. LIMIT injection (was blocking) — The regex-based re.search(r"\bLIMIT\b") check is gone. Replaced with an outer query cap: SELECT * FROM ({sql}) AS _postgres_mcp_export LIMIT {max_rows}. This is the correct fix — the outer LIMIT always applies regardless of inner LIMITs, comments, or string literals. Tests cover all bypass attempts: nested subqueries, existing LIMIT, LIMIT in strings, LIMIT in comments.

2. Restricted mode validation (was blocking) — execute_sql_xlsx calls get_sql_driver() which returns SafeSqlDriver in restricted mode. SafeSqlDriver.execute_query() calls self._validate() before execution, so the wrapped query is AST-validated. Test test_execute_sql_xlsx_restricted_mode_uses_safe_driver confirms a CTE-wrapped DELETE is rejected and the base driver is never called.

Design issues — documented

• Server-local file path: README and tool response both document that the file is server-local and not auto-deleted. Reasonable for an opt-in export tool.
• openpyxl as required dependency: Adds ~250KB to all installs. Acceptable for a database tool where Excel export is a core feature.

Verification

• 20/20 Excel export tests pass
• Full unit suite: 190 passed, 1 xfailed — no regressions
• ruff check, ruff format --check, pyright — all clean

Minor (non-blocking)

1. No truncation indicator. If the query returns exactly max_rows rows, the response shows Rows exported: {n} with no indication that results may have been truncated. Consider adding a note when len(rows) == max_rows.

2. Multi-statement input. rstrip(";") only removes trailing semicolons. SELECT 1; DROP TABLE x would produce a syntax error inside the subquery wrapper — safe but unhelpful error message. In restricted mode, SafeSqlDriver._validate() would also reject it. Non-exploitable.

---

This review was created by an AI agent (OpenHands) on behalf of @jssmith.

[ccclucky]

Hi @jssmith, thanks again for the approval. The PR still appears to be waiting for maintainer approval to run the forked CI workflow (action_required). Could you please approve the workflow, or let me know if anything else is needed before merging? Happy to update the branch if you prefer.