release: cut MCPg v0.8.2 - #330
Merged
Merged
Conversation
dump_database rejected any schema name that isn't a plain [A-Za-z_][A-Za-z0-9_]* identifier, so a valid PostgreSQL schema such as `adm-pgbench` failed with "invalid schema name" before pg_dump ever ran (issue #329). pg_dump --schema takes a *pattern* (psql \d rules), not a literal name: unquoted `*`/`?`/`[` are wildcards and bare letters fold to lowercase. Encode each schema name as a double-quoted literal pattern instead of validating it against the plain-identifier allowlist — every character, metacharacters included, then matches literally and case-sensitively, so the actual schema is dumped and no wildcard can accidentally expand. This is a dump-specific encoding (_encode_schema_pattern), deliberately separate from the shared identifier validator the in-process SQL paths rely on. Only an empty name or an embedded NUL is rejected up-front. Regression tests cover hyphens, spaces, mixed case, embedded double quotes, pattern metacharacters, and semicolons. Fixes #329 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011u7aiWq1HqxmGKvFgLMHYC
Bump __version__ to 0.8.2 (single source in src/mcpg/__init__.py) and sync the MCPB bundle (manifest.json + packaging pyproject) and server.json pins via the sync_* scripts. Roll CHANGELOG [Unreleased] into a dated [0.8.2] section, add docs/release-notes-0.8.2.md, and link it from docs/index.md. A focused bug-fix release: dump_database now accepts quoted PostgreSQL schema names (issue #329). No tool-surface change (254/256 tools), no contract-snapshot drift. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011u7aiWq1HqxmGKvFgLMHYC
Introduce mcpg.identifiers.quote_identifier — a single escape-based SQL identifier quoter (doubles embedded ", rejects only empty / NUL / >63 bytes) with adversarial tests. This replaces the per-module validate-then-naively-quote pattern, where the strict [A-Za-z_][A-Za-z0-9_]* regex was doing double duty as the injection defense AND (accidentally) rejecting every name PostgreSQL supports only through delimited identifiers. Migrate data_movement's in-process paths (export_table, import_csv, import_json, import_vectors) to quote_identifier, and encode copy_table_between_databases' pg_dump --table like dump_database's --schema (each half a literal quoted pattern). Hyphens, spaces, mixed case, and embedded quotes now work and are safely escaped; only empty / NUL / overlong names are rejected. Tests updated to assert safe escaping (no break-out) rather than blanket rejection. Part of the issue #329 identifier-validation sweep. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011u7aiWq1HqxmGKvFgLMHYC
Route vector_tuning, vector_ops, rag_efficiency, composite, and textsearch
through mcpg.identifiers so schema/table/column names needing delimited
quoting (hyphens, spaces, mixed case, embedded quotes) work and are safely
escaped instead of rejected.
textsearch keeps its strict allowlist for the one remaining string-literal
use — a regconfig name embedded in to_tsvector('<config>', …), where the
plain-identifier rule (which forbids the quote that could close the
literal) is the safety boundary — and gains a lenient identifier pre-check
for the identifier paths. rag_efficiency already double-quoted safely, so
only its validator was over-strict.
Tests that asserted blanket rejection of quotable names (and of
leading-digit names, which are valid delimited identifiers) now assert safe
escaping / accept those names, with empty and NUL kept as the rejected
cases. Escaping itself is covered adversarially in test_identifiers.py.
Part of the issue #329 identifier-validation sweep.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011u7aiWq1HqxmGKvFgLMHYC
pg_search, logical_replication, and turboquant already double-quote via
their own _pg_quote_ident, so only their over-strict validators needed
relaxing to mcpg.identifiers.ensure_identifier. rls and tenancy spliced
role/schema/table names with an unescaped f'"{name}"', so those splices now
go through quote_identifier (embedded quotes doubled) alongside the relaxed
validator — a role/schema/table needing delimited quoting works and can't
break out of SET LOCAL ROLE / the qualified relation. config's role
validators align with tenancy (blank-string rejection is unchanged).
Delimited role/schema/table/publication names (hyphens, spaces, mixed case,
embedded quotes) are now accepted and escaped; empty / NUL / overlong names
stay rejected. A logical_replication test now asserts a quoted table name is
safely doubled in the generated SQL rather than rejected.
Part of the issue #329 identifier-validation sweep.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011u7aiWq1HqxmGKvFgLMHYC
…uoting Route the remaining SQL-object-name gates through mcpg.identifiers: - migrations: relax target/reference-schema (and catalog table/column) validation; every user/catalog splice now quotes via quote_identifier. The generated shadow-schema name is already sanitized to a plain identifier, so its splices are unchanged. - test_data / test_row_factory: relaxed validators + quoted INSERT identifiers (value literals unchanged). - redis_fdw: relaxed validator + quoted CREATE SERVER / USER MAPPING / FOREIGN TABLE identifiers and the advisor stub. - listen: LISTEN/UNLISTEN channel names quoted (LISTEN "my-chan" works). - timescaledb: create_hypertable / add_*_policy take the relation as a regclass *text* arg, so the double-quoted relation is wrapped in a single-quoted literal with both layers escaped (_regclass_literal); the time-column NAME literal is single-quote-escaped too. Delimited names now work everywhere here; empty / NUL / overlong names stay rejected. Tests repointed accordingly. Completes the FIX half of the issue #329 identifier-validation sweep. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011u7aiWq1HqxmGKvFgLMHYC
Update the HTTP auth middleware tests to reflect that role names from an OIDC claim / X-MCPG-Role header may now be any delimited identifier (quoted safely at the SET LOCAL ROLE splice); empty / NUL / overlong roles are still rejected. Regenerate the architecture module map for the new mcpg.identifiers module, and expand CHANGELOG + release notes to cover the full sweep and the deliberately-strict exceptions (Apache AGE, ORM codegen, SQL/PGQ, regconfig). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011u7aiWq1HqxmGKvFgLMHYC
CI's `ruff format --check .` (whole repo) flagged one unformatted line in the new adversarial test file. No behavior change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011u7aiWq1HqxmGKvFgLMHYC
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="docs/release-notes-0.8.2.md" line_range="55-62" />
<code_context>
+argv (never a shell — see `mcpg.shell`), so quoting the pattern is both
+necessary and sufficient; there is no shell-injection surface to widen.
+
+### Scope, deliberately narrow
+
+- **Dump-specific.** The new `_encode_schema_pattern` helper lives in
+ `data_movement.py` and is used *only* by `dump_database`. The
+ plain-identifier validator that the in-process SQL paths rely on
+ (`export_table`, `import_csv` / `import_json` / `import_vectors`,
+ `copy_table_between_databases`) is unchanged — those splice names into
+ SQL text, a different contract, so relaxing them was explicitly avoided.
+- **Access mode unchanged.** `dump_database` still requires `unrestricted`
+ mode **and** `MCPG_ALLOW_SHELL=true`.
</code_context>
<issue_to_address>
**nitpick:** The release notes claim that the plain-identifier validator for `copy_table_between_databases` is unchanged and that the dump-specific helper is used only by `dump_database`, but the diff removes `_check_identifier_for_copy` and routes `copy_table_between_databases` through `_encode_schema_pattern`. The release documentation gives materially false scope and security-boundary information for this fix.
**Triggers:** When users or maintainers use the release notes to understand which tools accept delimited names and how their names are encoded.
**Suggested fix:** Update the release notes to say that `copy_table_between_databases` also uses literal pg_dump pattern encoding, as stated elsewhere in the same release.
</issue_to_address>
…abases Sourcery review on PR #330 caught it: the "Scope, deliberately narrow" section said copy_table_between_databases still used the old plain-identifier validator, unchanged. The code (and the CHANGELOG's own "Migrated" list) says otherwise -- it uses _encode_schema_pattern for both --schema and --table, same as dump_database. Corrected to state both pg_dump/pg_restore-argv tools share that helper, while the in-process SQL tools are migrated separately to quote_identifier. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XrLEbkf9BAxrqFTBfcU48z
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
2 participants
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.
Summary
Bug-fix release resolving the project's first user-reported issue, #329:
dump_databaserejected valid PostgreSQL schema names (e.g.adm-pgbench)that need delimited-identifier quoting. On inspection, ~20 other tools
shared the same over-strict
[A-Za-z_][A-Za-z0-9_]*allowlist.mcpg.identifiersmodule:quote_identifier/ensure_identifier,an escape-based (doubled-quote) SQL identifier quoter replacing the
copy-pasted plain-identifier regex. Adversarial test suite added
(
tests/unit/test_identifiers.py).dump_database,export_table,import_csv/import_json/import_vectors,copy_table_between_databases), the pgvector suites,textsearch,composite, RLS/tenancy/config roles, logical replication, migrations,test-data generators,
redis_fdw, LISTEN/NOTIFY, TimescaleDB.Deliberately left strict where the name isn't a PG identifier (AGE
labels, ORM code generators, SQL/PGQ, the
regconfigliteral).__version__→ 0.8.2 (single source), MCPB bundle +server.jsonsynced,
CHANGELOG.md+docs/release-notes-0.8.2.mdadded.drift, no breaking changes, no configuration changes.
Verification (re-run locally on this branch before opening the PR)
ruff check/ruff format --check: cleanmypy src/mcpg: clean (110 source files)bandit -r src/mcpg --skip B101,B608,B110 -ll: cleanpip-audit --strict --no-deps: no known vulnerabilitiestests/contract/test_doc_tables.py(doc-drift guard): 3 passedpython -m build && twine check dist/*: both artifacts PASSEDorigin/main🤖 Generated with Claude Code
Summary by Sourcery
Release MCPg 0.8.2 with safe PostgreSQL identifier quoting and broader support for valid delimited object names without changing the tool surface or contracts.
New Features:
Bug Fixes:
Enhancements:
Build:
Documentation:
Tests:
Chores: