docs: identifier policy (match the sink)

[devopam]

Summary

• Adds docs/identifier-policy.md
• Links it from docs/index.md

Documents match the sink naming: SQL/pg_dump paths accept delimited identifiers (hyphen, space, case, $, unicode, reserved words, …) via encoding; ORM/AGE/shell/config sinks stay plain-identifier-only with clear agent-facing wording.

Supersedes experiments on docs/identifier-policy / v2 (do not use those branches).

Optional follow-up (not in this PR)

README Why-MCPg bullet still cites the old global regex — 3-line fix linking this page; then ORM tool descriptions + exporter errors.

Test plan

• Docs render
• Index link works

Summary by Sourcery

Document identifier handling according to the constraints of each destination sink.

Enhancements:

• Document the sink-specific identifier policy, distinguishing delimited PostgreSQL names from plain-identifier-only ORM, AGE, shell, literal, and configuration sinks.
• Provide agent-facing guidance for handling rejected names and selecting SQL-oriented tools.

Documentation:

• Add the canonical identifier policy documentation and link it from the documentation index.

[sourcery-ai]

Hey - I've found 3 issues

Prompt for AI Agents

Please address the comments from this code review:

## Individual Comments

### Comment 1
<location path="docs/identifier-policy.md" line_range="29-31" />
<code_context>
+| `order` / `user` | Reserved words — must be quoted as identifiers |
+| `weird"name` | Embedded `"` is doubled to `""` inside quotes |
+
+**Always rejected** (not valid addressable identifiers): empty string, embedded
+NUL (`\x00`), names longer than **63 bytes** (PostgreSQL's `NAMEDATALEN - 1`;
+the server would otherwise **silently truncate**).
+
+Callers pass the **bare** name (e.g. `adm-pgbench`). Do not add SQL quotes
</code_context>
<issue_to_address>
**issue:** The policy says names longer than 63 bytes are always rejected, but the `pg_dump` pattern encoder accepts them and `dump_database` passes them to `pg_dump` without length validation. As a result, dump callers receive a subprocess result rather than the documented upfront identifier rejection.

**Triggers:** When a caller supplies an over-63-byte name to a pg_dump-backed tool.

**Suggested fix:** Apply the same UTF-8 byte-length check used by `ensure_identifier` to dump-pattern inputs, or state that the 63-byte rejection applies only to SQL-identifier paths.

```suggestion
**Always rejected on SQL-identifier paths** (not valid addressable identifiers): empty string, embedded
NUL (`\x00`), names longer than **63 bytes** (PostgreSQL's `NAMEDATALEN - 1`;
the server would otherwise **silently truncate**).
```
</issue_to_address>

### Comment 2
<location path="docs/identifier-policy.md" line_range="12-16" />
<code_context>
+
+## SQL and `pg_dump` / `pg_restore` paths
+
+Tools that address real PostgreSQL objects (query, export, import, dump,
+replication, roles used in `SET ROLE`, and similar) accept any **addressable**
+PostgreSQL identifier. Values are encoded with `quote_identifier` (or dump
+pattern encoding for `pg_dump`), so injection is prevented by **escaping**,
+not by forbidding hyphens.
+
+| Example | Notes |
</code_context>
<issue_to_address>
**issue (broader_impact):** The broad claim that query tools accept any addressable PostgreSQL identifier is false for the NL-to-SQL path: `_validate_schema_name` still rejects delimited names such as `adm-pgbench` with the plain-identifier regex. Agents following this page are directed to use a query-oriented tool for names that the query path actually rejects.

**Triggers:** When an agent uses the NL-to-SQL query tool with a schema requiring delimited quoting.

**Suggested fix:** Either exempt NL-to-SQL from the SQL sink claim and document its plain-identifier restriction, or migrate that path and its prompt-injection safeguards before claiming that query tools accept all addressable identifiers.

```suggestion
Tools that address real PostgreSQL objects (SQL query tools other than NL-to-SQL,
export, import, dump, replication, roles used in `SET ROLE`, and similar) accept any
**addressable** PostgreSQL identifier. Values are encoded with `quote_identifier`
(or dump pattern encoding for `pg_dump`), so injection is prevented by **escaping**,
not by forbidding hyphens. The NL-to-SQL path is an exception: it accepts only plain
identifiers matching `[A-Za-z_][A-Za-z0-9_]*`, so names such as `adm-pgbench` are not
supported there.
```
</issue_to_address>

### Comment 3
<location path="docs/identifier-policy.md" line_range="50" />
<code_context>
+
+| Sink | Examples | Why |
+|------|----------|-----|
+| Generated source | Prisma, Drizzle, Diesel, Ecto, Ent, jOOQ, sqlc, SQLAlchemy export | Name becomes an identifier in TypeScript / Go / Rust / Elixir / Python |
+| Graph labels | Apache AGE / graph projection labels | Extension label rules, not PG delimited identifiers |
+| Shell / host command | `schedule_logical_backup` paths, some `COPY TO PROGRAM` args | Shell metacharacters are a real injection surface |
+| Some SQL *literals* | `regconfig` in text-search helpers | Embedded as a string literal, not as a delimited identifier |
+| Config keys | `MCPG_SECONDARY_DATABASE_URLS` names | MCPg's own ids (`[a-z0-9_]+`), not PG object names |
+
</code_context>
<issue_to_address>
**nitpick:** The shell-sink guidance presents these tools as plain-identifier-only and gives `[A-Za-z_][A-Za-z0-9_]*` as the typical allowlist, but `schedule_logical_backup` deliberately accepts hyphenated database names and allows hyphens in its path and executable-path inputs. Agents are therefore told to reject valid inputs such as `app-prod` even though the implementation accepts them safely under its shell-specific allowlist.

**Triggers:** When an agent schedules a backup for a database whose name contains a hyphen.

**Suggested fix:** Document the actual per-argument shell-safe patterns for `schedule_logical_backup` instead of classifying all shell/host command inputs as plain identifiers.

```suggestion
| Shell / host command | `schedule_logical_backup`: `destination` = `/[A-Za-z0-9_./-]+`; `pg_dump_path` = `[A-Za-z0-9_./-]+`; `database` = `[A-Za-z0-9_][A-Za-z0-9_-]*` | Shell metacharacters are a real injection surface |
```
</issue_to_address>

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

[sourcery-ai]

issue: The policy says names longer than 63 bytes are always rejected, but the pg_dump pattern encoder accepts them and dump_database passes them to pg_dump without length validation. As a result, dump callers receive a subprocess result rather than the documented upfront identifier rejection.

Triggers: When a caller supplies an over-63-byte name to a pg_dump-backed tool.

Suggested fix: Apply the same UTF-8 byte-length check used by ensure_identifier to dump-pattern inputs, or state that the 63-byte rejection applies only to SQL-identifier paths.

Suggested change

	**Always rejected** (not valid addressable identifiers): empty string, embedded
	NUL (`\x00`), names longer than **63 bytes** (PostgreSQL's `NAMEDATALEN - 1`;
	the server would otherwise **silently truncate**).
	**Always rejected on SQL-identifier paths** (not valid addressable identifiers): empty string, embedded
	NUL (`\x00`), names longer than **63 bytes** (PostgreSQL's `NAMEDATALEN - 1`;
	the server would otherwise **silently truncate**).

[sourcery-ai]

issue (broader_impact): The broad claim that query tools accept any addressable PostgreSQL identifier is false for the NL-to-SQL path: _validate_schema_name still rejects delimited names such as adm-pgbench with the plain-identifier regex. Agents following this page are directed to use a query-oriented tool for names that the query path actually rejects.

Triggers: When an agent uses the NL-to-SQL query tool with a schema requiring delimited quoting.

Suggested fix: Either exempt NL-to-SQL from the SQL sink claim and document its plain-identifier restriction, or migrate that path and its prompt-injection safeguards before claiming that query tools accept all addressable identifiers.

Suggested change

	Tools that address real PostgreSQL objects (query, export, import, dump,
	replication, roles used in `SET ROLE`, and similar) accept any **addressable**
	PostgreSQL identifier. Values are encoded with `quote_identifier` (or dump
	pattern encoding for `pg_dump`), so injection is prevented by **escaping**,
	not by forbidding hyphens.
	Tools that address real PostgreSQL objects (SQL query tools other than NL-to-SQL,
	export, import, dump, replication, roles used in `SET ROLE`, and similar) accept any
	**addressable** PostgreSQL identifier. Values are encoded with `quote_identifier`
	(or dump pattern encoding for `pg_dump`), so injection is prevented by **escaping**,
	not by forbidding hyphens. The NL-to-SQL path is an exception: it accepts only plain
	identifiers matching `[A-Za-z_][A-Za-z0-9_]*`, so names such as `adm-pgbench` are not
	supported there.

[sourcery-ai]

nitpick: The shell-sink guidance presents these tools as plain-identifier-only and gives [A-Za-z_][A-Za-z0-9_]* as the typical allowlist, but schedule_logical_backup deliberately accepts hyphenated database names and allows hyphens in its path and executable-path inputs. Agents are therefore told to reject valid inputs such as app-prod even though the implementation accepts them safely under its shell-specific allowlist.

Triggers: When an agent schedules a backup for a database whose name contains a hyphen.

Suggested fix: Document the actual per-argument shell-safe patterns for schedule_logical_backup instead of classifying all shell/host command inputs as plain identifiers.

Suggested change

	| Shell / host command | `schedule_logical_backup` paths, some `COPY TO PROGRAM` args | Shell metacharacters are a real injection surface |
	| Shell / host command | `schedule_logical_backup`: `destination` = `/[A-Za-z0-9_./-]+`; `pg_dump_path` = `[A-Za-z0-9_./-]+`; `database` = `[A-Za-z0-9_][A-Za-z0-9_-]*` | Shell metacharacters are a real injection surface |