Repository navigation
feat: add trusted local PostgreSQL snapshot CLI #724
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Draft
seonghobae
wants to merge
22
commits into
main
Choose a base branch
from
feat/trusted-local-snapshot-cli
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Changes from 1 commit
Commits
Show all changes
22 commits
Select commit
Hold shift + click to select a range
73fef74
feat: add trusted local snapshot CLI
seonghobae a40ebd1
fix: harden local snapshot CLI validation
seonghobae b7e1619
test: prove shared snapshot collector coverage
seonghobae d3449b8
test: cover snapshot CLI success output
seonghobae d695e22
test: include snapshot paths in coverage evidence
seonghobae 4782cee
test: enforce snapshot public docstrings
seonghobae 8f7fdef
ci: enforce backend coverage evidence
seonghobae a785e7e
docs: record trusted local snapshot workflow
seonghobae 3c89c2e
Merge branch 'main' into feat/trusted-local-snapshot-cli
seonghobae 97f1500
Merge branch 'main' into feat/trusted-local-snapshot-cli
opencode-agent[bot] 1c8dfce
Merge branch 'main' into feat/trusted-local-snapshot-cli
opencode-agent[bot] 44e44fb
test(cli): document trusted snapshot behavior
seonghobae 9dbd302
test(snapshot): document Citus collection states
seonghobae 0c17794
test(cli): prevent PGPASSWORD inheritance
seonghobae 1bae174
fix(cli): disable ambient credential fallback
seonghobae b3d4ce7
refactor(cli): make empty credential policy explicit
seonghobae f8e2292
Merge main into PR 724
seonghobae 98db179
Merge branch 'main' into feat/trusted-local-snapshot-cli
opencode-agent[bot] dd2d279
Merge branch 'main' into feat/trusted-local-snapshot-cli
seonghobae 946b130
Merge remote-tracking branch 'refs/remotes/origin/main' into HEAD
seonghobae 1d714a1
docs(introspection): complete guarded connection docstrings
seonghobae dbc3158
fix(cli): handle connection timeouts safely
seonghobae File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,129 @@ | ||
| """Trusted-local PostgreSQL snapshot CLI. | ||
|
|
||
| The web API intentionally rejects loopback and private database targets. This | ||
| separate operator CLI accepts only an absolute Unix-domain socket directory, so | ||
| developers can reverse a local migration database without weakening the remote | ||
| API's SSRF boundary or putting a password-bearing DSN in the process list. | ||
| """ | ||
|
|
||
| from __future__ import annotations | ||
|
|
||
| import argparse | ||
| import asyncio | ||
| import json | ||
| import os | ||
| import re | ||
| import sys | ||
| from pathlib import Path | ||
| from typing import Sequence | ||
|
|
||
| import asyncpg | ||
|
|
||
| from app.pg_introspect.snapshot_collect import collect_postgres_snapshot | ||
|
|
||
| _SCHEMA_RE = re.compile(r"^[A-Za-z_][A-Za-z0-9_$]{0,62}$") | ||
|
|
||
|
|
||
| def _socket_directory(value: str) -> str: | ||
| path = Path(value) | ||
| if not path.is_absolute(): | ||
| raise argparse.ArgumentTypeError( | ||
| "--host must be an absolute PostgreSQL Unix socket directory" | ||
| ) | ||
| if not path.is_dir(): | ||
| raise argparse.ArgumentTypeError("--host Unix socket directory does not exist") | ||
| return str(path) | ||
|
|
||
|
|
||
| def _schema_name(value: str) -> str: | ||
| if not _SCHEMA_RE.fullmatch(value): | ||
| raise argparse.ArgumentTypeError("--schema is not a valid PostgreSQL identifier") | ||
| return value | ||
|
|
||
|
|
||
| def _port(value: str) -> int: | ||
| try: | ||
| port = int(value) | ||
| except ValueError as exc: | ||
| raise argparse.ArgumentTypeError("--port must be an integer") from exc | ||
| if not 1 <= port <= 65535: | ||
| raise argparse.ArgumentTypeError("--port must be between 1 and 65535") | ||
| return port | ||
|
|
||
|
|
||
| def build_parser() -> argparse.ArgumentParser: | ||
| parser = argparse.ArgumentParser( | ||
| prog="pg-erd-snapshot", | ||
| description=( | ||
| "Write a pg-erd-cloud snapshot for a trusted local PostgreSQL Unix socket " | ||
| "to stdout." | ||
| ), | ||
| ) | ||
| parser.add_argument( | ||
| "--database", | ||
| default=os.environ.get("PGDATABASE"), | ||
| required=os.environ.get("PGDATABASE") is None, | ||
| help="database name (defaults to PGDATABASE)", | ||
| ) | ||
| parser.add_argument( | ||
| "--host", | ||
| type=_socket_directory, | ||
| default=os.environ.get("PGHOST", "/tmp"), | ||
| help="absolute Unix socket directory (defaults to PGHOST or /tmp)", | ||
| ) | ||
| parser.add_argument( | ||
| "--port", | ||
| type=_port, | ||
| default=os.environ.get("PGPORT", "5432"), | ||
| metavar="1..65535", | ||
| ) | ||
| parser.add_argument( | ||
| "--user", | ||
| default=os.environ.get("PGUSER"), | ||
| help="database user (defaults to PGUSER or the operating-system user)", | ||
| ) | ||
| parser.add_argument("--schema", type=_schema_name, default=None) | ||
| parser.add_argument("--pretty", action="store_true") | ||
| return parser | ||
|
|
||
|
|
||
| async def capture_local_snapshot(args: argparse.Namespace) -> dict: | ||
| connect_args: dict[str, object] = { | ||
| "database": args.database, | ||
| "host": args.host, | ||
| "port": args.port, | ||
| "timeout": 10, | ||
| } | ||
| if args.user: | ||
| connect_args["user"] = args.user | ||
| conn = await asyncpg.connect(**connect_args) | ||
| try: | ||
| return await collect_postgres_snapshot(conn, args.schema) | ||
| finally: | ||
| await conn.close() | ||
|
|
||
|
|
||
| def main(argv: Sequence[str] | None = None) -> int: | ||
| args = build_parser().parse_args(argv) | ||
| try: | ||
| snapshot = asyncio.run(capture_local_snapshot(args)) | ||
| except (OSError, asyncpg.PostgresError) as exc: | ||
| print( | ||
| f"pg-erd-snapshot failed: {type(exc).__name__}", | ||
| file=sys.stderr, | ||
| ) | ||
| return 1 | ||
| json.dump( | ||
| snapshot, | ||
| sys.stdout, | ||
| ensure_ascii=False, | ||
| indent=2 if args.pretty else None, | ||
| sort_keys=True, | ||
| separators=None if args.pretty else (",", ":"), | ||
| ) | ||
| sys.stdout.write("\n") | ||
| return 0 | ||
|
|
||
|
|
||
| if __name__ == "__main__": # pragma: no cover | ||
| raise SystemExit(main()) | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,68 @@ | ||
| """Shared PostgreSQL catalog snapshot collector. | ||
|
|
||
| This module deliberately has no application-settings import. Network trust and | ||
| credential policy are established before callers hand it an open connection. | ||
| """ | ||
|
|
||
| from __future__ import annotations | ||
|
|
||
| import datetime as dt | ||
|
|
||
| import asyncpg | ||
|
|
||
| from app.pg_introspect import queries | ||
| from app.pg_introspect.column_examples import add_column_examples | ||
| from app.sanitize import sanitize_for_storage | ||
|
|
||
|
|
||
| async def collect_postgres_snapshot( | ||
| conn: asyncpg.Connection, schema_filter: str | None | ||
| ) -> dict: | ||
| """Collect the canonical snapshot from an authorized PostgreSQL connection.""" | ||
|
|
||
| version = await conn.fetchval("SHOW server_version") | ||
| schema_name = schema_filter | ||
| include_system = False | ||
|
|
||
| schemas = await conn.fetch(queries.SCHEMAS_SQL, schema_name, include_system) | ||
| relations = await conn.fetch(queries.RELATIONS_SQL, schema_name, include_system) | ||
| columns = await conn.fetch(queries.COLUMNS_SQL, schema_name, include_system) | ||
| constraints = await conn.fetch( | ||
| queries.CONSTRAINTS_SQL, schema_name, include_system | ||
| ) | ||
| indexes = await conn.fetch(queries.INDEXES_SQL, schema_name, include_system) | ||
| pk_columns = await conn.fetch( | ||
| queries.PK_COLUMNS_SQL, schema_name, include_system | ||
| ) | ||
| fk_edges = await conn.fetch(queries.FK_EDGES_SQL, schema_name, include_system) | ||
| citus_distributed_tables = [] | ||
| has_citus = await conn.fetchval( | ||
| "SELECT EXISTS (SELECT 1 FROM pg_catalog.pg_extension WHERE extname = 'citus')" | ||
| ) | ||
| if has_citus: | ||
| try: | ||
| citus_distributed_tables = await conn.fetch( | ||
| queries.CITUS_DISTRIBUTED_TABLES_SQL, | ||
| schema_name, | ||
| include_system, | ||
| ) | ||
| except asyncpg.UndefinedTableError: | ||
| citus_distributed_tables = [] | ||
|
|
||
| snapshot = { | ||
| "captured_at": dt.datetime.now(dt.timezone.utc).isoformat(), | ||
| "server_version": str(version), | ||
| "schema_filter": schema_filter, | ||
| "schemas": [dict(row) for row in schemas], | ||
| "relations": [dict(row) for row in relations], | ||
| "columns": add_column_examples([dict(row) for row in columns]), | ||
| "constraints": [dict(row) for row in constraints], | ||
| "indexes": [dict(row) for row in indexes], | ||
| "pk_columns": [dict(row) for row in pk_columns], | ||
| "fk_edges": [dict(row) for row in fk_edges], | ||
| "citus_distributed_tables": [ | ||
| dict(row) for row in citus_distributed_tables | ||
| ], | ||
| } | ||
|
|
||
| return sanitize_for_storage(snapshot) # type: ignore[return-value] |
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,97 @@ | ||
| from __future__ import annotations | ||
|
|
||
| import argparse | ||
| from pathlib import Path | ||
| from typing import Any | ||
|
|
||
| import pytest | ||
|
|
||
| from app import local_snapshot_cli | ||
|
|
||
|
|
||
| class FakeConnection: | ||
| def __init__(self) -> None: | ||
| self.closed = False | ||
|
|
||
| async def close(self) -> None: | ||
| self.closed = True | ||
|
|
||
|
|
||
| def test_socket_directory_requires_existing_absolute_directory(tmp_path: Path) -> None: | ||
| assert local_snapshot_cli._socket_directory(str(tmp_path)) == str(tmp_path) | ||
| with pytest.raises(argparse.ArgumentTypeError): | ||
| local_snapshot_cli._socket_directory("relative/socket") | ||
| with pytest.raises(argparse.ArgumentTypeError): | ||
| local_snapshot_cli._socket_directory(str(tmp_path / "missing")) | ||
|
|
||
|
|
||
| def test_environment_host_default_cannot_bypass_socket_boundary( | ||
| monkeypatch: pytest.MonkeyPatch, | ||
| capsys: pytest.CaptureFixture[str], | ||
| ) -> None: | ||
| monkeypatch.setenv("PGDATABASE", "catalog") | ||
| monkeypatch.setenv("PGHOST", "db.example.com") | ||
|
|
||
| with pytest.raises(SystemExit) as exc_info: | ||
| local_snapshot_cli.build_parser().parse_args([]) | ||
|
|
||
| assert exc_info.value.code == 2 | ||
| assert "absolute PostgreSQL Unix socket directory" in capsys.readouterr().err | ||
|
|
||
|
|
||
| @pytest.mark.parametrize("value", ["not-a-port", "0", "65536"]) | ||
| def test_port_rejects_invalid_values(value: str) -> None: | ||
| with pytest.raises(argparse.ArgumentTypeError): | ||
| local_snapshot_cli._port(value) | ||
|
|
||
|
|
||
| def test_schema_name_rejects_sql_fragments() -> None: | ||
| assert local_snapshot_cli._schema_name("catalog_v2") == "catalog_v2" | ||
| with pytest.raises(argparse.ArgumentTypeError): | ||
| local_snapshot_cli._schema_name("public; DROP SCHEMA public") | ||
|
|
||
|
|
||
| @pytest.mark.asyncio | ||
| async def test_capture_uses_local_connection_without_password_or_dsn( | ||
| monkeypatch: pytest.MonkeyPatch, | ||
| tmp_path: Path, | ||
| ) -> None: | ||
| captured: dict[str, Any] = {} | ||
| connection = FakeConnection() | ||
|
|
||
| async def fake_connect(**kwargs: object) -> FakeConnection: | ||
| captured.update(kwargs) | ||
| return connection | ||
|
|
||
| async def fake_collect(conn: FakeConnection, schema: str | None) -> dict: | ||
| assert conn is connection | ||
| return {"schema_filter": schema, "relations": []} | ||
|
|
||
| monkeypatch.setattr(local_snapshot_cli.asyncpg, "connect", fake_connect) | ||
| monkeypatch.setattr( | ||
| local_snapshot_cli, | ||
| "collect_postgres_snapshot", | ||
| fake_collect, | ||
| ) | ||
| args = argparse.Namespace( | ||
| database="catalog", | ||
| host=str(tmp_path), | ||
| port=5432, | ||
| user="operator", | ||
| schema="public", | ||
| pretty=False, | ||
| ) | ||
|
|
||
| snapshot = await local_snapshot_cli.capture_local_snapshot(args) | ||
|
|
||
| assert snapshot == {"schema_filter": "public", "relations": []} | ||
| assert captured == { | ||
| "database": "catalog", | ||
| "host": str(tmp_path), | ||
| "port": 5432, | ||
| "timeout": 10, | ||
| "user": "operator", | ||
| } | ||
| assert "dsn" not in captured | ||
| assert "password" not in captured | ||
| assert connection.closed is True |
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.