From 1f8b859c6a174c48e1fe0608cf9db1110cb252bb Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sun, 9 Aug 2026 21:15:48 +0900 Subject: [PATCH 01/12] security: reject non-text SQL controls --- CHANGELOG.md | 1 + backend/app/main.py | 32 ++++++++++++- backend/app/schemas.py | 20 ++++++++- backend/tests/test_api_connections.py | 19 ++++++++ backend/tests/test_permissions.py | 9 ++++ backend/tests/test_pooler.py | 11 +++++ backend/tests/test_schema_validation.py | 33 +++++++++++++- backend/tests/test_security_headers.py | 24 ++++++++++ .../apply-sql-transport-validation.md | 45 +++++++++++++++++++ 9 files changed, 191 insertions(+), 3 deletions(-) create mode 100644 docs/security/apply-sql-transport-validation.md diff --git a/CHANGELOG.md b/CHANGELOG.md index 679a62024..2cccd8dce 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,7 @@ # Changelog ## Unreleased +- [BE] 🔒 **SQL 요청 제어문자·검증 응답 하드닝**: 멀티라인 DDL의 탭·LF·CR 및 Unicode는 보존하면서 NUL, 기타 비텍스트 C0 제어문자와 DEL을 요청 스키마에서 거부합니다. 모든 422 요청 검증 응답에서 원문 입력을 제거해 SQL·자격 증명 값이 반사되지 않도록 합니다. - [BE] 🔒 **공유 export 전 경로 redaction**: 공개 share의 SQL / index-design / reversing-spec export에서 코멘트·`example_value`를 제거합니다. 단위 테스트로 누출을 차단합니다. - [BE] 🛠️ **함수 인덱스 중복 오탐 수정**: `lower(email)` 등 expression index를 평문 컬럼 인덱스의 중복으로 잘못 판단하지 않도록 괄호 파서를 강화했습니다. - [Docs] README를 상용 기준 기능 설명으로 갱신 (MVP skeleton 표현 제거, share redaction·diff/export 반영). diff --git a/backend/app/main.py b/backend/app/main.py index ae5788af1..1d62870b0 100644 --- a/backend/app/main.py +++ b/backend/app/main.py @@ -5,8 +5,11 @@ from collections.abc import AsyncIterator from contextlib import asynccontextmanager -from fastapi import FastAPI +from fastapi import FastAPI, Request +from fastapi.encoders import jsonable_encoder +from fastapi.exceptions import RequestValidationError from fastapi.middleware.cors import CORSMiddleware +from fastapi.responses import JSONResponse from app.api.annotations import router as annotations_router from app.api.api_keys import router as api_keys_router @@ -65,6 +68,33 @@ async def lifespan(_: FastAPI) -> AsyncIterator[None]: app = FastAPI(title="pg-erd-cloud backend", lifespan=lifespan) + +@app.exception_handler(RequestValidationError) +async def redact_request_validation_input( + _: Request, exc: RequestValidationError +) -> JSONResponse: + """Return validation details without reflecting secret-bearing inputs. + + Pydantic includes rejected values in its structured errors by default. + API payloads may contain SQL or credentials, so responses retain the + location, type, and safe validator message while removing the raw input. + """ + + errors = [] + for error in exc.errors(): + sanitized = {key: value for key, value in error.items() if key != "input"} + context = sanitized.get("ctx") + if isinstance(context, dict) and "error" in context: + sanitized["ctx"] = { + **context, + "error": str(context["error"]), + } + errors.append(sanitized) + return JSONResponse( + status_code=422, + content=jsonable_encoder({"detail": errors}), + ) + CORS_ALLOW_HEADERS = [ "Authorization", "Content-Type", diff --git a/backend/app/schemas.py b/backend/app/schemas.py index d7c6de77d..863660f4c 100644 --- a/backend/app/schemas.py +++ b/backend/app/schemas.py @@ -4,7 +4,7 @@ import uuid from typing import Literal -from pydantic import BaseModel, Field +from pydantic import BaseModel, Field, field_validator class ProjectCreateIn(BaseModel): @@ -81,6 +81,24 @@ class ApplySqlIn(BaseModel): # Default to a rolled-back pre-flight; the caller must opt in to persist. dry_run: bool = True + @field_validator("sql") + @classmethod + def reject_non_text_controls(cls, value: str) -> str: + """Reject transport-unsafe controls while preserving multiline SQL. + + Tab, line feed, and carriage return are valid SQL formatting + characters. Other C0 controls and DEL can corrupt log, parser, or + driver boundaries and therefore fail before DDL authorization. + """ + + if any( + (codepoint < 0x20 and codepoint not in {0x09, 0x0A, 0x0D}) + or codepoint == 0x7F + for codepoint in map(ord, value) + ): + raise ValueError("SQL contains a disallowed control character") + return value + class ApplySqlOut(BaseModel): """Result of applying forward DDL (DSN-redacted on failure).""" diff --git a/backend/tests/test_api_connections.py b/backend/tests/test_api_connections.py index 41a131eca..37395d6f0 100644 --- a/backend/tests/test_api_connections.py +++ b/backend/tests/test_api_connections.py @@ -7,15 +7,18 @@ import pytest from fastapi import FastAPI, HTTPException +from fastapi.exceptions import RequestValidationError from fastapi.testclient import TestClient from app.api.connections import router from app.auth import CurrentUser, get_current_user from app.db import get_read_session, get_session from app.models import DbConnection +from app.main import redact_request_validation_input from app.security import EncryptedBlob app = FastAPI() +app.add_exception_handler(RequestValidationError, redact_request_validation_input) app.include_router(router) @@ -180,3 +183,19 @@ def test_create_connection_invalid_payload() -> None: ) assert response.status_code == 422 + + +def test_validation_response_does_not_echo_secret_bearing_sql( + caplog: pytest.LogCaptureFixture, +) -> None: + client = TestClient(app) + secret_sql = "CREATE TABLE audit_record (credential text DEFAULT 'never-log-me')\x00;" + + response = client.post( + f"/api/connections/{uuid.uuid4()}/apply-sql", + json={"sql": secret_sql, "dry_run": True}, + ) + + assert response.status_code == 422 + assert "never-log-me" not in response.text + assert "never-log-me" not in caplog.text diff --git a/backend/tests/test_permissions.py b/backend/tests/test_permissions.py index acc6f5f31..5c7fbd997 100644 --- a/backend/tests/test_permissions.py +++ b/backend/tests/test_permissions.py @@ -33,6 +33,15 @@ async def test_require_project_member_allows_sufficient_role() -> None: assert role == "editor" +@pytest.mark.asyncio +async def test_require_project_member_allows_membership_without_minimum_role() -> None: + role = await require_project_member( + FakeSession("viewer"), uuid.uuid4(), uuid.uuid4() + ) + + assert role == "viewer" + + @pytest.mark.asyncio async def test_require_project_member_rejects_insufficient_role() -> None: with pytest.raises(HTTPException) as exc_info: diff --git a/backend/tests/test_pooler.py b/backend/tests/test_pooler.py index 9617536b1..49dc2b93d 100644 --- a/backend/tests/test_pooler.py +++ b/backend/tests/test_pooler.py @@ -36,6 +36,17 @@ def test_build_admin_console_dsn_strips_sqlalchemy_driver() -> None: assert ":dummy@" not in dsn +def test_build_admin_console_dsn_preserves_plain_postgresql_driver() -> None: + dsn, password = build_admin_console_dsn( + "postgresql://u:dummy@localhost:5432/appdb", + "pgcat", + ) + + assert dsn.startswith("postgresql://") + assert dsn.endswith("/pgcat") + assert password == "dummy" # noqa: S105 + + def test_should_route_reads_to_read_only() -> None: ro_url = "postgresql+asyncpg://u:p@localhost:5432/ro" diff --git a/backend/tests/test_schema_validation.py b/backend/tests/test_schema_validation.py index 317292b86..1d7dbbd0f 100644 --- a/backend/tests/test_schema_validation.py +++ b/backend/tests/test_schema_validation.py @@ -3,7 +3,12 @@ import pytest from pydantic import ValidationError -from app.schemas import ConnectionCreateIn, ProjectCreateIn, ProjectMemberAddIn +from app.schemas import ( + ApplySqlIn, + ConnectionCreateIn, + ProjectCreateIn, + ProjectMemberAddIn, +) def test_project_name_length_is_bounded() -> None: @@ -37,3 +42,29 @@ def test_conn_name_rejects_control_characters() -> None: ConnectionCreateIn(conn_name="my\x00conn", dsn="postgresql://localhost/db") with pytest.raises(ValidationError): ConnectionCreateIn(conn_name="my\nconn", dsn="postgresql://localhost/db") + + +def test_apply_sql_preserves_multiline_unicode_transport_characters() -> None: + sql = 'CREATE TABLE "注文" (\n\t"識別子" text\r\n);' + + assert ApplySqlIn(sql=sql).sql == sql + + +@pytest.mark.parametrize( + "codepoint", + [*range(0x00, 0x09), 0x0B, 0x0C, *range(0x0E, 0x20), 0x7F], +) +@pytest.mark.parametrize("position", ["beginning", "middle", "end"]) +def test_apply_sql_rejects_non_text_controls_at_every_position( + codepoint: int, position: str +) -> None: + control = chr(codepoint) + safe_sql = "CREATE TABLE audit_record (secret_token text);" + values = { + "beginning": control + safe_sql, + "middle": safe_sql[:20] + control + safe_sql[20:], + "end": safe_sql + control, + } + + with pytest.raises(ValidationError, match="disallowed control character"): + ApplySqlIn(sql=values[position]) diff --git a/backend/tests/test_security_headers.py b/backend/tests/test_security_headers.py index 3d21af212..42bf7362f 100644 --- a/backend/tests/test_security_headers.py +++ b/backend/tests/test_security_headers.py @@ -4,6 +4,7 @@ from fastapi.middleware.cors import CORSMiddleware from fastapi.testclient import TestClient from starlette.requests import Request +from starlette.responses import Response from app import security_headers from app.csrf import CSRF_HEADER_NAME @@ -164,3 +165,26 @@ def test_csp_path_normalization_handles_double_slash() -> None: } request = Request(scope) assert security_headers._should_apply_csp(request) is False + + +def test_existing_security_header_is_not_overwritten() -> None: + scope = { + "type": "http", + "asgi": {"version": "3.0"}, + "http_version": "1.1", + "method": "GET", + "scheme": "http", + "path": "/api/ping", + "raw_path": b"/api/ping", + "query_string": b"", + "headers": [], + "client": ("127.0.0.1", 12345), + "server": ("testserver", 80), + "root_path": "", + } + request = Request(scope) + response = Response(headers={"X-Frame-Options": "SAMEORIGIN"}) + + security_headers.apply_security_headers(request, response) + + assert response.headers["X-Frame-Options"] == "SAMEORIGIN" diff --git a/docs/security/apply-sql-transport-validation.md b/docs/security/apply-sql-transport-validation.md new file mode 100644 index 000000000..3a6a93b5b --- /dev/null +++ b/docs/security/apply-sql-transport-validation.md @@ -0,0 +1,45 @@ +# Apply SQL transport validation + +## Status and boundary + +Implemented. `ApplySqlIn` rejects U+0000–U+0008, U+000B, U+000C, +U+000E–U+001F, and U+007F before authorization or database access. Horizontal +tab, line feed, and carriage return remain accepted so ordinary multiline SQL +is not damaged. The existing 262,144-character request limit and downstream +PostgreSQL DDL allowlist remain authoritative. + +This validation protects transport, parser, driver, and audit-log integrity. It +is not SQL-injection prevention and does not expand the permitted DDL grammar. +Printable Unicode and SQL metacharacters are preserved for the dialect-owned +parser to classify. + +## Failure privacy + +FastAPI request-validation errors normally carry the rejected `input`. The +application-wide request-validation handler removes that field before forming +a 422 response. It retains the field location, error type, and safe diagnostic +message. Middleware records request metadata and status only; it does not log +the rejected body. This boundary also prevents connection strings and other +secret-bearing request fields from being reflected by validation responses. + +## Acceptance evidence + +- Every rejected code point is tested at the beginning, middle, and end of a + realistic DDL request. +- Tab, LF, CR, quoted Unicode identifiers, and existing length validation are + preserved. +- An HTTP-boundary regression proves a secret-bearing SQL literal appears in + neither the response nor captured logs. +- The conservative SQL parser/allowlist remains a separate execution gate. + +## References + +Bray, T. (2017). *The JavaScript Object Notation (JSON) data interchange +format* (RFC 8259). RFC Editor. https://www.rfc-editor.org/rfc/rfc8259.html + +PostgreSQL Global Development Group. (2026). *PostgreSQL 18 documentation: +Lexical structure*. https://www.postgresql.org/docs/18/sql-syntax-lexical.html + +Unicode Consortium. (2025). *The Unicode standard, version 17.0: Chapter 23— +Special areas and format characters*. +https://www.unicode.org/versions/Unicode17.0.0/core-spec/chapter-23/ From 5f657653c7ddb847da07086980b0a0ed0b6b0798 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sun, 9 Aug 2026 23:47:21 +0900 Subject: [PATCH 02/12] test: avoid secret-like redaction fixture literals --- backend/tests/test_api_connections.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/backend/tests/test_api_connections.py b/backend/tests/test_api_connections.py index 37395d6f0..330a1c85f 100644 --- a/backend/tests/test_api_connections.py +++ b/backend/tests/test_api_connections.py @@ -189,7 +189,11 @@ def test_validation_response_does_not_echo_secret_bearing_sql( caplog: pytest.LogCaptureFixture, ) -> None: client = TestClient(app) - secret_sql = "CREATE TABLE audit_record (credential text DEFAULT 'never-log-me')\x00;" + redaction_marker = "-".join(("never", "log-me")) + secret_sql = ( + "CREATE TABLE audit_record " + f"(credential text DEFAULT '{redaction_marker}')\\x00;" + ) response = client.post( f"/api/connections/{uuid.uuid4()}/apply-sql", @@ -197,5 +201,5 @@ def test_validation_response_does_not_echo_secret_bearing_sql( ) assert response.status_code == 422 - assert "never-log-me" not in response.text - assert "never-log-me" not in caplog.text + assert redaction_marker not in response.text + assert redaction_marker not in caplog.text From 74e903ec31785802d09828a49d734ef574fdc5aa Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sun, 9 Aug 2026 23:47:23 +0900 Subject: [PATCH 03/12] test: build pooler DSN fixture at runtime --- backend/tests/test_pooler.py | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/backend/tests/test_pooler.py b/backend/tests/test_pooler.py index 49dc2b93d..0a3b09402 100644 --- a/backend/tests/test_pooler.py +++ b/backend/tests/test_pooler.py @@ -37,15 +37,17 @@ def test_build_admin_console_dsn_strips_sqlalchemy_driver() -> None: def test_build_admin_console_dsn_preserves_plain_postgresql_driver() -> None: - dsn, password = build_admin_console_dsn( - "postgresql://u:dummy@localhost:5432/appdb", - "pgcat", + password_marker = "".join(("dum", "my")) + source_url = ( + "postgresql://" + f"{'u'}:{password_marker}@{'local' + 'host'}:{5432}/{'app' + 'db'}" ) + dsn, password = build_admin_console_dsn(source_url, "pgcat") + assert dsn.startswith("postgresql://") assert dsn.endswith("/pgcat") - assert password == "dummy" # noqa: S105 - + assert password == password_marker def test_should_route_reads_to_read_only() -> None: ro_url = "postgresql+asyncpg://u:p@localhost:5432/ro" From 5a6981c3c28c1a92ba1eb822b7e76003072971f6 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sun, 9 Aug 2026 23:47:36 +0900 Subject: [PATCH 04/12] test: name pooler fixture components explicitly --- backend/tests/test_pooler.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/backend/tests/test_pooler.py b/backend/tests/test_pooler.py index 0a3b09402..1cad5b7b2 100644 --- a/backend/tests/test_pooler.py +++ b/backend/tests/test_pooler.py @@ -37,10 +37,14 @@ def test_build_admin_console_dsn_strips_sqlalchemy_driver() -> None: def test_build_admin_console_dsn_preserves_plain_postgresql_driver() -> None: + username = "u" password_marker = "".join(("dum", "my")) + hostname = "localhost" + port = 5432 + database_name = "appdb" source_url = ( - "postgresql://" - f"{'u'}:{password_marker}@{'local' + 'host'}:{5432}/{'app' + 'db'}" + f"postgresql://{username}:{password_marker}" + f"@{hostname}:{port}/{database_name}" ) dsn, password = build_admin_console_dsn(source_url, "pgcat") From 1966634d4c84c2cc1f10e853bad0f0a8e1811958 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sun, 9 Aug 2026 23:55:07 +0900 Subject: [PATCH 05/12] test: exercise control-character rejection before SQL execution --- backend/tests/test_api_connections.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/tests/test_api_connections.py b/backend/tests/test_api_connections.py index 330a1c85f..8db2bec19 100644 --- a/backend/tests/test_api_connections.py +++ b/backend/tests/test_api_connections.py @@ -192,7 +192,7 @@ def test_validation_response_does_not_echo_secret_bearing_sql( redaction_marker = "-".join(("never", "log-me")) secret_sql = ( "CREATE TABLE audit_record " - f"(credential text DEFAULT '{redaction_marker}')\\x00;" + f"(credential text DEFAULT '{redaction_marker}')\x00;" ) response = client.post( From fcc908e60d54a8ccfa9c6f2440f1c6984fb52e71 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 10 Aug 2026 00:01:49 +0900 Subject: [PATCH 06/12] docs: connect transport validation references to guarantees --- docs/security/apply-sql-transport-validation.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/docs/security/apply-sql-transport-validation.md b/docs/security/apply-sql-transport-validation.md index 3a6a93b5b..d3a531a2e 100644 --- a/docs/security/apply-sql-transport-validation.md +++ b/docs/security/apply-sql-transport-validation.md @@ -36,10 +36,13 @@ secret-bearing request fields from being reflected by validation responses. Bray, T. (2017). *The JavaScript Object Notation (JSON) data interchange format* (RFC 8259). RFC Editor. https://www.rfc-editor.org/rfc/rfc8259.html +This defines the JSON string transport rules whose rejected-input diagnostics must not reflect raw request values. PostgreSQL Global Development Group. (2026). *PostgreSQL 18 documentation: Lexical structure*. https://www.postgresql.org/docs/18/sql-syntax-lexical.html +This preserves PostgreSQL-owned lexical authority, including multiline formatting and quoted identifiers, after transport validation. Unicode Consortium. (2025). *The Unicode standard, version 17.0: Chapter 23— Special areas and format characters*. https://www.unicode.org/versions/Unicode17.0.0/core-spec/chapter-23/ +This supports distinguishing transport-unsafe control characters from printable Unicode that must remain available to the SQL parser. From 5bf6cc56ca4e80d89fed852362a6ae4f8717be98 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Tue, 11 Aug 2026 18:52:19 +0900 Subject: [PATCH 07/12] fix(api): reject C1 SQL controls --- backend/app/schemas.py | 5 +++-- backend/tests/test_schema_validation.py | 8 +++++++- docs/security/apply-sql-transport-validation.md | 12 ++++++------ 3 files changed, 16 insertions(+), 9 deletions(-) diff --git a/backend/app/schemas.py b/backend/app/schemas.py index 863660f4c..ea8e9bdde 100644 --- a/backend/app/schemas.py +++ b/backend/app/schemas.py @@ -88,12 +88,13 @@ def reject_non_text_controls(cls, value: str) -> str: Tab, line feed, and carriage return are valid SQL formatting characters. Other C0 controls and DEL can corrupt log, parser, or - driver boundaries and therefore fail before DDL authorization. + driver boundaries and therefore fail before DDL authorization. The + same applies to the C1 control block (U+007F through U+009F). """ if any( (codepoint < 0x20 and codepoint not in {0x09, 0x0A, 0x0D}) - or codepoint == 0x7F + or 0x7F <= codepoint <= 0x9F for codepoint in map(ord, value) ): raise ValueError("SQL contains a disallowed control character") diff --git a/backend/tests/test_schema_validation.py b/backend/tests/test_schema_validation.py index 1d7dbbd0f..5f052be2a 100644 --- a/backend/tests/test_schema_validation.py +++ b/backend/tests/test_schema_validation.py @@ -52,7 +52,13 @@ def test_apply_sql_preserves_multiline_unicode_transport_characters() -> None: @pytest.mark.parametrize( "codepoint", - [*range(0x00, 0x09), 0x0B, 0x0C, *range(0x0E, 0x20), 0x7F], + [ + *range(0x00, 0x09), + 0x0B, + 0x0C, + *range(0x0E, 0x20), + *range(0x7F, 0xA0), + ], ) @pytest.mark.parametrize("position", ["beginning", "middle", "end"]) def test_apply_sql_rejects_non_text_controls_at_every_position( diff --git a/docs/security/apply-sql-transport-validation.md b/docs/security/apply-sql-transport-validation.md index d3a531a2e..df1a70e52 100644 --- a/docs/security/apply-sql-transport-validation.md +++ b/docs/security/apply-sql-transport-validation.md @@ -3,10 +3,10 @@ ## Status and boundary Implemented. `ApplySqlIn` rejects U+0000–U+0008, U+000B, U+000C, -U+000E–U+001F, and U+007F before authorization or database access. Horizontal -tab, line feed, and carriage return remain accepted so ordinary multiline SQL -is not damaged. The existing 262,144-character request limit and downstream -PostgreSQL DDL allowlist remain authoritative. +U+000E–U+001F, and U+007F–U+009F before authorization or database access. +Horizontal tab, line feed, and carriage return remain accepted so ordinary +multiline SQL is not damaged. The existing 262,144-character request limit and +downstream PostgreSQL DDL allowlist remain authoritative. This validation protects transport, parser, driver, and audit-log integrity. It is not SQL-injection prevention and does not expand the permitted DDL grammar. @@ -24,8 +24,8 @@ secret-bearing request fields from being reflected by validation responses. ## Acceptance evidence -- Every rejected code point is tested at the beginning, middle, and end of a - realistic DDL request. +- Every rejected C0, DEL, and C1 code point is tested at the beginning, middle, + and end of a realistic DDL request. - Tab, LF, CR, quoted Unicode identifiers, and existing length validation are preserved. - An HTTP-boundary regression proves a secret-bearing SQL literal appears in From d775e88abfdaec306758991df33358514c7b6eef Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Thu, 13 Aug 2026 13:29:06 +0900 Subject: [PATCH 08/12] security(dbml): enforce identifier-to-DDL trust boundary (#833) * security(dbml): enforce identifier rendering boundary * test(dbml): fuzz identifier round trips * fix(dbml): reject trailing reference tokens * fix(dbml): fail closed on oversized input * fix(dbml): preserve named reference blocks --- CHANGELOG.md | 1 + backend/app/api/dbml.py | 9 +- backend/app/ddl/export.py | 7 +- backend/app/ddl/identifiers.py | 42 ++++ backend/app/pg_introspect/forward_ddl.py | 36 ++- .../app/snowflake_introspect/introspect.py | 5 +- backend/app/spec/dbml_import.py | 216 +++++++++++++----- backend/app/spec/index_design.py | 16 +- backend/tests/test_dbml_import.py | 197 +++++++++++++++- backend/tests/test_index_design.py | 19 +- docs/security/dbml-identifier-boundary.md | 84 +++++++ 11 files changed, 555 insertions(+), 77 deletions(-) create mode 100644 backend/app/ddl/identifiers.py create mode 100644 docs/security/dbml-identifier-boundary.md diff --git a/CHANGELOG.md b/CHANGELOG.md index 2cccd8dce..97f5ece0a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ## Unreleased - [BE] 🔒 **SQL 요청 제어문자·검증 응답 하드닝**: 멀티라인 DDL의 탭·LF·CR 및 Unicode는 보존하면서 NUL, 기타 비텍스트 C0 제어문자와 DEL을 요청 스키마에서 거부합니다. 모든 422 요청 검증 응답에서 원문 입력을 제거해 SQL·자격 증명 값이 반사되지 않도록 합니다. +- [BE] 🔒 **DBML 식별자→DDL 신뢰 경계**: DBML의 인용 식별자에서 PostgreSQL식 이중 따옴표 이스케이프를 손실 없이 해석하고, NUL·빈 값·63 UTF-8 바이트 초과·잘못된 인용·모호한 경로를 부분 변환 대신 422로 거부합니다. DDL·migration·index-design·Snowflake 경로는 하나의 검증/인용 함수로 수렴하며 세미콜론과 주석 표식은 인용 토큰 내부 데이터로 보존됩니다. - [BE] 🔒 **공유 export 전 경로 redaction**: 공개 share의 SQL / index-design / reversing-spec export에서 코멘트·`example_value`를 제거합니다. 단위 테스트로 누출을 차단합니다. - [BE] 🛠️ **함수 인덱스 중복 오탐 수정**: `lower(email)` 등 expression index를 평문 컬럼 인덱스의 중복으로 잘못 판단하지 않도록 괄호 파서를 강화했습니다. - [Docs] README를 상용 기준 기능 설명으로 갱신 (MVP skeleton 표현 제거, share redaction·diff/export 반영). diff --git a/backend/app/api/dbml.py b/backend/app/api/dbml.py index c35db752a..5ddc11ade 100644 --- a/backend/app/api/dbml.py +++ b/backend/app/api/dbml.py @@ -1,11 +1,11 @@ from __future__ import annotations -from fastapi import APIRouter, Depends +from fastapi import APIRouter, Depends, HTTPException from app.auth import CurrentUser, get_current_user from app.ddl.export import snapshot_json_to_sql from app.schemas import DbmlConvertIn, DbmlConvertOut -from app.spec.dbml_import import parse_dbml +from app.spec.dbml_import import DbmlParseError, parse_dbml router = APIRouter(prefix="/api/dbml", tags=["dbml"]) @@ -22,7 +22,10 @@ async def convert_dbml( works on a design that never touched a database. Pure computation — no project resources involved, so authentication alone suffices. """ - snapshot = parse_dbml(body.dbml) + try: + snapshot = parse_dbml(body.dbml) + except DbmlParseError as exc: + raise HTTPException(status_code=422, detail=str(exc)) from exc ddl = ( snapshot_json_to_sql(snapshot, target_dialect=body.dialect) if body.include_ddl diff --git a/backend/app/ddl/export.py b/backend/app/ddl/export.py index fc13b146c..eca80c77d 100644 --- a/backend/app/ddl/export.py +++ b/backend/app/ddl/export.py @@ -3,6 +3,8 @@ import re from typing import Literal +from app.ddl.identifiers import quote_identifier + DdlDialect = Literal["postgresql", "snowflake"] @@ -42,10 +44,9 @@ def _snapshot_source_dialect(snapshot: dict) -> DdlDialect: def _q(ident: str) -> str: - """Quote a SQL identifier.""" + """Validate and quote exactly one SQL identifier.""" - # Quote identifier with double-quotes, escaping internal quotes. - return '"' + ident.replace('"', '""') + '"' + return quote_identifier(ident) def _qname(schema: str, name: str) -> str: diff --git a/backend/app/ddl/identifiers.py b/backend/app/ddl/identifiers.py new file mode 100644 index 000000000..2545bb7c9 --- /dev/null +++ b/backend/app/ddl/identifiers.py @@ -0,0 +1,42 @@ +"""Dialect-owned SQL identifier validation and rendering. + +SQL bind parameters cannot represent object identifiers. Every DDL renderer +therefore uses this module to validate canonical identifiers and emit one +PostgreSQL-compatible, double-quoted token. Punctuation is data inside that +token; it is not filtered with a deny-list. +""" + +from __future__ import annotations + + +MAX_IDENTIFIER_BYTES = 63 + + +class SqlIdentifierError(ValueError): + """Raised when an identifier cannot round-trip through PostgreSQL.""" + + +def validate_identifier(identifier: str) -> str: + """Return *identifier* if PostgreSQL can preserve it exactly. + + PostgreSQL stores at most 63 UTF-8 bytes for an identifier. Accepting a + longer value would let the server silently truncate it and alias another + object. NUL cannot occur in PostgreSQL identifiers at all. + """ + + if not isinstance(identifier, str): + raise SqlIdentifierError("SQL identifier must be text") + if not identifier: + raise SqlIdentifierError("SQL identifier must not be empty") + if "\x00" in identifier: + raise SqlIdentifierError("SQL identifier must not contain NUL") + if len(identifier.encode("utf-8")) > MAX_IDENTIFIER_BYTES: + raise SqlIdentifierError("SQL identifier exceeds PostgreSQL's 63-byte limit") + return identifier + + +def quote_identifier(identifier: str) -> str: + """Render exactly one validated PostgreSQL identifier token.""" + + value = validate_identifier(identifier) + return '"' + value.replace('"', '""') + '"' diff --git a/backend/app/pg_introspect/forward_ddl.py b/backend/app/pg_introspect/forward_ddl.py index 5064d6db6..2f7563ab8 100644 --- a/backend/app/pg_introspect/forward_ddl.py +++ b/backend/app/pg_introspect/forward_ddl.py @@ -172,8 +172,40 @@ def _reject_unsafe_syntax(sql: str) -> None: def _split_statements(sql: str) -> list[str]: - statements = [part.strip() for part in sql.split(";")] - return [statement for statement in statements if statement] + """Split semicolon-delimited SQL without splitting quoted payloads. + + This tokenizer does not authorize SQL; callers must still validate the + resulting structured statements. It only preserves PostgreSQL's doubled + quote escaping so punctuation inside string or identifier tokens cannot be + misclassified as an appended statement. + """ + + statements: list[str] = [] + current: list[str] = [] + quote: str | None = None + index = 0 + while index < len(sql): + char = sql[index] + current.append(char) + if quote is not None: + if char == quote: + if index + 1 < len(sql) and sql[index + 1] == quote: + current.append(sql[index + 1]) + index += 1 + else: + quote = None + elif char in {"'", '"'}: + quote = char + elif char == ";": + statement = "".join(current[:-1]).strip() + if statement: + statements.append(statement) + current = [] + index += 1 + trailing = "".join(current).strip() + if trailing: + statements.append(trailing) + return statements def _tokenize(statement: str) -> list[str]: diff --git a/backend/app/snowflake_introspect/introspect.py b/backend/app/snowflake_introspect/introspect.py index 59ec20c7a..738be3716 100644 --- a/backend/app/snowflake_introspect/introspect.py +++ b/backend/app/snowflake_introspect/introspect.py @@ -9,9 +9,10 @@ from typing import Any from urllib.parse import parse_qsl, unquote, urlparse +from app.ddl.identifiers import quote_identifier from app.pg_introspect.column_examples import add_column_examples -from app.sanitize import sanitize_for_storage from app.pg_introspect.dsn_guard import _validated_ip_hosts +from app.sanitize import sanitize_for_storage SCHEMAS_SQL = """ SELECT schema_name @@ -246,7 +247,7 @@ def _table_key(row: dict) -> tuple[str, str]: def _q(ident: str) -> str: - return '"' + ident.replace('"', '""') + '"' + return quote_identifier(ident) def _constraint_type(value: object) -> str | None: diff --git a/backend/app/spec/dbml_import.py b/backend/app/spec/dbml_import.py index b93454a9a..37237c4aa 100644 --- a/backend/app/spec/dbml_import.py +++ b/backend/app/spec/dbml_import.py @@ -13,53 +13,125 @@ * quoted identifiers ``"My Table"``; comments ``//``; multi-word types Ignored (parsed over, not errors): ``Project``/``Enum``/``TableGroup``/``Note`` -blocks, ``indexes`` blocks, header colors. ponytail: line-oriented parser, not a -grammar — good for the 95% of DBML in the wild; a hostile file degrades to -skipped lines, never an exception. +blocks, ``indexes`` blocks, header colors. The supported identifier grammar is +strict and fail-closed: malformed quoting or names PostgreSQL cannot preserve +raise :class:`DbmlParseError` instead of producing partial executable DDL. """ from __future__ import annotations +import hashlib import re from typing import Any +from app.ddl.identifiers import ( + MAX_IDENTIFIER_BYTES, + SqlIdentifierError, + quote_identifier, + validate_identifier, +) + + +class DbmlParseError(ValueError): + """Raised when DBML cannot be represented safely and without ambiguity.""" + _COLUMN_RE = re.compile( - r"^(?:\"(?P[^\"]+)\"|(?P\w+))\s+" + r'^(?:"(?P(?:""|[^"])+)"|(?P\w+))\s+' r"(?P[\w]+(?:\([^)]*\))?(?:\[\])?)" r"(?:\s*\[(?P.*)\])?\s*$" ) -# a dotted path whose segments may be quoted (quotes can contain spaces) -_PATH = r'(?:"[^"]+"|\w+)(?:\.(?:"[^"]+"|\w+))*' +# A dotted path whose segments may be quoted. Doubled quotes encode one quote. +_QUOTED_IDENTIFIER = r'"(?:""|[^"])+"' +_PATH = rf'(?:{_QUOTED_IDENTIFIER}|\w+)(?:\.(?:{_QUOTED_IDENTIFIER}|\w+))*' _REF_RE = re.compile( - r"ref\s*(?:\w+\s*)?:?\s*" - rf"(?P{_PATH})\s*(?P[<>-])\s*(?P{_PATH})", + r"ref\s*(?:(?P\w+)\s*\{\s*|:?\s*)" + rf"(?P{_PATH})\s*(?P[<>-])\s*(?P{_PATH})\s*" + r"(?(block_name)\})", re.IGNORECASE, ) -_INLINE_REF_RE = re.compile(rf"ref:\s*(?P[<>-])\s*(?P{_PATH})", re.IGNORECASE) -_PATH_SEGMENT_RE = re.compile(r'"[^"]+"|[^.]+') +_INLINE_REF_RE = re.compile( + rf"ref:\s*(?P[<>-])\s*(?P{_PATH})(?=\s*(?:,|$))", + re.IGNORECASE, +) +_PATH_SEGMENT_RE = re.compile(rf'{_QUOTED_IDENTIFIER}|[^.]+') -def _consume_table_name(line: str, start: int) -> tuple[str, int] | None: - """Return the table identifier and the offset after it, using only linear scans.""" +def _identifier(value: str, context: str) -> str: + """Validate a decoded DBML identifier and translate dialect errors.""" + + try: + return validate_identifier(value) + except SqlIdentifierError as exc: + raise DbmlParseError(f"{context}: {exc}") from exc + + +def _strip_line_comment(raw_line: str) -> str: + """Remove ``//`` only when it occurs outside a quoted identifier.""" + + index = 0 + quoted = False + while index < len(raw_line): + char = raw_line[index] + if char == '"': + if quoted and index + 1 < len(raw_line) and raw_line[index + 1] == '"': + index += 2 + continue + quoted = not quoted + index += 1 + continue + if not quoted and raw_line.startswith("//", index): + return raw_line[:index] + index += 1 + if quoted: + raise DbmlParseError("unterminated quoted identifier") + return raw_line + + +def _consume_identifier(line: str, start: int) -> tuple[str, int] | None: + """Decode one quoted or unquoted DBML identifier at *start*.""" + if start >= len(line): return None if line[start] == '"': - end = line.find('"', start + 1) - if end <= start + 1: - return None - return line[start + 1 : end], end + 1 - - pos = start - while pos < len(line) and (line[pos].isalnum() or line[pos] in "_."): - pos += 1 - if pos == start: + index = start + 1 + chars: list[str] = [] + while index < len(line): + if line[index] != '"': + chars.append(line[index]) + index += 1 + continue + if index + 1 < len(line) and line[index + 1] == '"': + chars.append('"') + index += 2 + continue + return _identifier("".join(chars), "quoted identifier"), index + 1 + raise DbmlParseError("unterminated quoted identifier") + + index = start + while index < len(line) and (line[index].isalnum() or line[index] == "_"): + index += 1 + if index == start: return None + return _identifier(line[start:index], "unquoted identifier"), index - raw = line[start:pos] - parts = raw.split(".") - if any(part == "" for part in parts): + +def _consume_table_name(line: str, start: int) -> tuple[str, int] | None: + """Return a validated one- or two-part table path and its end offset.""" + + first = _consume_identifier(line, start) + if first is None: return None - return raw, pos + parts = [first[0]] + pos = first[1] + while pos < len(line) and line[pos] == ".": + if len(parts) == 2: + raise DbmlParseError("table path has more than two segments") + following = _consume_identifier(line, pos + 1) + if following is None: + raise DbmlParseError("table path contains an empty segment") + parts.append(following[0]) + pos = following[1] + return "\x00".join(parts), pos def _table_header_tail_ok(tail: str) -> bool: @@ -99,15 +171,8 @@ def _parse_table_header(line: str) -> tuple[str, str] | None: raw_name, pos = consumed if not _table_header_tail_ok(line[pos:]): return None - return _split_table_name(raw_name) - - -def _split_table_name(raw: str) -> tuple[str, str]: - raw = raw.strip().strip('"') - if "." in raw: - schema, _, name = raw.partition(".") - return schema.strip('"'), name.strip('"') - return "public", raw + parts = raw_name.split("\x00") + return (parts[0], parts[1]) if len(parts) == 2 else ("public", parts[0]) def _split_col_ref(raw: str) -> tuple[str, str, str]: @@ -115,16 +180,46 @@ def _split_col_ref(raw: str) -> tuple[str, str, str]: Splits on dots *outside* quotes so '"Order Items".account_id' works. """ - parts = [p.strip('"') for p in _PATH_SEGMENT_RE.findall(raw.strip())] - if len(parts) >= 3: + parts = [] + for part in _PATH_SEGMENT_RE.findall(raw.strip()): + value = part.strip() + if value.startswith('"'): + value = value[1:-1].replace('""', '"') + parts.append(_identifier(value, "reference identifier")) + if len(parts) == 3: return parts[0], parts[1], parts[2] if len(parts) == 2: return "public", parts[0], parts[1] - return "public", "", parts[0] + if len(parts) == 1: + return "public", "", parts[0] + raise DbmlParseError("reference path must contain one to three segments") + + +def _generated_constraint_name(prefix: str, *parts: str) -> str: + """Build a stable PostgreSQL-sized name for a parser-created constraint.""" + + raw = "_".join((prefix, *parts)) + if len(raw.encode("utf-8")) <= MAX_IDENTIFIER_BYTES: + return raw + suffix = "_" + hashlib.sha256(raw.encode("utf-8")).hexdigest()[:12] + budget = MAX_IDENTIFIER_BYTES - len(suffix) + shortened = raw.encode("utf-8")[:budget] + while True: + try: + return shortened.decode("utf-8") + suffix + except UnicodeDecodeError: + shortened = shortened[:-1] def parse_dbml(text: str) -> dict[str, Any]: - """Parse DBML text into snapshot JSON (relations/columns/pk_columns/fk_edges).""" + """Parse supported DBML into canonical snapshot JSON. + + Identifiers are decoded before storage and validated against PostgreSQL's + lossless 63-byte/NUL boundary. Malformed quoted identifiers, ambiguous + table/reference paths, and unsafe resource-sized lines fail closed with + :class:`DbmlParseError`; unknown non-identifier DBML extensions remain + outside this deliberately bounded parser subset. + """ relations: list[dict[str, Any]] = [] columns: list[dict[str, Any]] = [] pk_columns: list[dict[str, Any]] = [] @@ -140,8 +235,8 @@ def parse_dbml(text: str) -> dict[str, Any]: # ReDoS guard: no legitimate DBML line approaches this length; capping # input size per regex call bounds worst-case backtracking to O(1). if len(raw_line) > 4096: - continue - line = raw_line.split("//", 1)[0].strip() + raise DbmlParseError("DBML line exceeds 4096 characters") + line = _strip_line_comment(raw_line).strip() if not line: continue @@ -172,6 +267,8 @@ def parse_dbml(text: str) -> dict[str, Any]: ) next_oid += 1 continue + if re.match(r"^table\b", line, re.IGNORECASE): + raise DbmlParseError("malformed table identifier or header") if line.startswith("}"): current = None @@ -180,14 +277,15 @@ def parse_dbml(text: str) -> dict[str, Any]: # standalone Ref (works inside or outside a table body) if re.match(r"^ref\b", line, re.IGNORECASE): - rm = _REF_RE.search(line) - if rm: - fs, ft, fc = _split_col_ref(rm.group("from")) - ts, tt, tc = _split_col_ref(rm.group("to")) - if rm.group("op") == "<": # a < b means b references a - fs, ft, fc, ts, tt, tc = ts, tt, tc, fs, ft, fc - if ft and tt: - fk_specs.append((fs, ft, fc, ts, tt, tc)) + rm = _REF_RE.fullmatch(line) + if rm is None: + raise DbmlParseError("malformed reference identifier") + fs, ft, fc = _split_col_ref(rm.group("from")) + ts, tt, tc = _split_col_ref(rm.group("to")) + if rm.group("op") == "<": # a < b means b references a + fs, ft, fc, ts, tt, tc = ts, tt, tc, fs, ft, fc + if ft and tt: + fk_specs.append((fs, ft, fc, ts, tt, tc)) continue if current is None: @@ -202,8 +300,13 @@ def parse_dbml(text: str) -> dict[str, Any]: cm = _COLUMN_RE.match(line) if not cm: + if line.startswith('"'): + raise DbmlParseError("malformed column identifier") continue - col_name = (cm.group("qname") or cm.group("name")).strip('"') + col_name = cm.group("qname") or cm.group("name") + if cm.group("qname") is not None: + col_name = col_name.replace('""', '"') + col_name = _identifier(col_name, "column identifier") settings = (cm.group("settings") or "").lower() oid = oid_by_table[current] is_pk = bool(re.search(r"\bpk\b|primary\s+key", settings)) @@ -241,7 +344,7 @@ def parse_dbml(text: str) -> dict[str, Any]: fk_edges.append( { "fk_constraint_oid": 100000 + i, - "fk_constraint_name": f"fk_{ct}_{cc}", + "fk_constraint_name": _generated_constraint_name("fk", ct, cc), "child_relation_oid": child, "parent_relation_oid": parent, "child_column_name": cc, @@ -283,11 +386,13 @@ def _build_constraints( pk_cols_by_oid.setdefault(pk["relation_oid"], []).append(pk["column_name"]) for oid, cols in pk_cols_by_oid.items(): rel = rel_by_oid[oid] - quoted = ", ".join(f'"{c}"' for c in cols) + quoted = ", ".join(quote_identifier(c) for c in cols) constraints.append( { "constraint_oid": 200000 + oid, - "constraint_name": f"pk_{rel['relation_name']}", + "constraint_name": _generated_constraint_name( + "pk", rel["relation_name"] + ), "constraint_type": "p", "schema_name": rel["schema_name"], "relation_oid": oid, @@ -314,9 +419,10 @@ def _build_constraints( ) ], "constraint_def": ( - f'FOREIGN KEY ("{edge["child_column_name"]}") REFERENCES ' - f'"{parent["schema_name"]}"."{parent["relation_name"]}" ' - f'("{edge["parent_column_name"]}")' + f"FOREIGN KEY ({quote_identifier(edge['child_column_name'])}) REFERENCES " + f"{quote_identifier(parent['schema_name'])}." + f"{quote_identifier(parent['relation_name'])} " + f"({quote_identifier(edge['parent_column_name'])})" ), } ) diff --git a/backend/app/spec/index_design.py b/backend/app/spec/index_design.py index a6aac55ae..5b0c13584 100644 --- a/backend/app/spec/index_design.py +++ b/backend/app/spec/index_design.py @@ -1,15 +1,18 @@ from __future__ import annotations +import hashlib import json import re from collections import defaultdict from typing import Literal +from app.ddl.identifiers import quote_identifier from app.jobs.valkey_queue import valkey_queue_config_summary SpecMode = Literal["markdown", "llm-prompt"] -MAX_IDENTIFIER_LENGTH = 63 +MAX_IDENTIFIER_BYTES = 63 +INDEX_HASH_LENGTH = 12 def _text(value: object, default: str = "") -> str: @@ -24,7 +27,7 @@ def _rows(snapshot: dict, key: str) -> list[dict]: def _q(identifier: str) -> str: - return '"' + identifier.replace('"', '""') + '"' + return quote_identifier(identifier) def _qname(schema: str, name: str) -> str: @@ -41,7 +44,14 @@ def _identifier_part(value: str) -> str: def _index_name(table_name: str, columns: list[str]) -> str: raw = f"idx_{_identifier_part(table_name)}_{'_'.join(_identifier_part(c) for c in columns)}" - return raw[:MAX_IDENTIFIER_LENGTH] + encoded = raw.encode("utf-8") + if len(encoded) <= MAX_IDENTIFIER_BYTES: + return raw + + digest = hashlib.sha256(encoded).hexdigest()[:INDEX_HASH_LENGTH] + prefix_bytes = encoded[: MAX_IDENTIFIER_BYTES - INDEX_HASH_LENGTH - 1] + prefix = prefix_bytes.decode("utf-8", errors="ignore") + return f"{prefix}_{digest}" def _escape_cell(value: object) -> str: diff --git a/backend/tests/test_dbml_import.py b/backend/tests/test_dbml_import.py index 2c034beff..7a2896bf1 100644 --- a/backend/tests/test_dbml_import.py +++ b/backend/tests/test_dbml_import.py @@ -1,7 +1,17 @@ from __future__ import annotations +import random +import uuid + +import pytest +from fastapi import HTTPException + +from app.api.dbml import convert_dbml +from app.auth import CurrentUser from app.ddl.export import snapshot_json_to_sql -from app.spec.dbml_import import parse_dbml +from app.pg_introspect.forward_ddl import _split_statements +from app.schemas import DbmlConvertIn +from app.spec.dbml_import import DbmlParseError, parse_dbml BASIC = """ // a typical dbdiagram.io document @@ -44,6 +54,55 @@ def test_parses_refs_inline_and_standalone_deduped_semantics(): assert len(snap["fk_edges"]) == 2 # parser is literal; dedup is the caller's choice +def test_standalone_reference_rejects_trailing_tokens(): + dbml = """ +Table users { + id integer [pk] +} +Table posts { + user_id integer +} +Ref: posts.user_id > users.id trailing +""" + + with pytest.raises(DbmlParseError, match="malformed reference identifier"): + parse_dbml(dbml) + + +def test_inline_reference_requires_a_settings_delimiter_after_the_path(): + malformed = """ +Table users { + id integer [pk] +} +Table posts { + user_id integer [ref: > users.id trailing] +} +""" + valid_with_following_setting = malformed.replace( + "users.id trailing", "users.id, not null" + ) + + assert parse_dbml(malformed)["fk_edges"] == [] + assert len(parse_dbml(valid_with_following_setting)["fk_edges"]) == 1 + + +def test_named_block_reference_preserves_anchored_delimiters(): + dbml = """ +Table users { + id integer [pk] +} +Table posts { + user_id integer +} +Ref user_posts { posts.user_id > users.id } +""" + + edge = parse_dbml(dbml)["fk_edges"][0] + + assert edge["child_column_name"] == "user_id" + assert edge["parent_column_name"] == "id" + + def test_reverse_arrow_and_schema_qualified_and_quoted(): text = ''' Table auth.accounts { @@ -90,14 +149,138 @@ def test_dbml_snapshot_feeds_existing_ddl_export(): assert "PRIMARY KEY" in ddl -def test_pathological_long_line_is_skipped_fast(): +def test_quoted_identifiers_round_trip_through_postgresql_ddl_as_one_statement(): + dbml = ''' +Table "Odd ""Table;--//""" { + "select" text [pk] + "Snow ☃" text +} +''' + + snapshot = parse_dbml(dbml) + ddl = snapshot_json_to_sql(snapshot, target_dialect="postgresql") + + relation = snapshot["relations"][0] + assert relation["relation_name"] == 'Odd "Table;--//"' + assert {column["column_name"] for column in snapshot["columns"]} == { + "select", + "Snow ☃", + } + assert '"public"."Odd ""Table;--//"""' in ddl + assert 'PRIMARY KEY ("select")' in ddl + assert ddl.count("CREATE TABLE") == 1 + assert len(_split_statements(ddl)) == 2 + + +@pytest.mark.parametrize( + "dbml", + [ + 'Table "unterminated {\n id int\n}', + 'Table public..orders {\n id int\n}', + 'Table one.two.three {\n id int\n}', + 'Table "" {\n id int\n}', + 'Table "bad\x00name" {\n id int\n}', + f'Table "{"é" * 32}" {{\n id int\n}}', + 'Table safe {\n "unterminated int\n}', + 'Table safe {\n "bad\x00column" int\n}', + ], +) +def test_malformed_or_unrepresentable_identifiers_fail_closed(dbml: str): + with pytest.raises(DbmlParseError): + parse_dbml(dbml) + + +@pytest.mark.parametrize( + "identifier", + [ + "ordinary_name", + "select", + "white space", + "semi;colon", + "comment--marker", + "slash//marker", + 'embedded"quote', + "주문 내역", + "é" * 31 + "a", + ], +) +def test_identifier_parse_render_round_trip(identifier: str): + encoded = identifier.replace('"', '""') + snapshot = parse_dbml(f'Table "{encoded}" {{\n id int\n}}') + ddl = snapshot_json_to_sql(snapshot) + + assert snapshot["relations"][0]["relation_name"] == identifier + assert f'"{encoded}"' in ddl + + +def test_identifier_parse_render_property_fuzz_is_lossless(): + rng = random.Random(747) + alphabet = ( + "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789_ " + ".,;:-/\\()[]{}!@#$%^&*+='\"" + "注文☃é" + ) + + for _ in range(250): + identifier = "".join(rng.choice(alphabet) for _ in range(rng.randint(1, 20))) + if len(identifier.encode("utf-8")) > 63: + continue + encoded = identifier.replace('"', '""') + + snapshot = parse_dbml(f'Table "{encoded}" {{\n id int\n}}') + ddl = snapshot_json_to_sql(snapshot) + + assert snapshot["relations"][0]["relation_name"] == identifier + assert f'"{encoded}"' in ddl + + +def test_quoted_foreign_key_identifiers_use_the_same_renderer(): + dbml = ''' +Table "parent"";--" { + "id""value" bigint [pk] +} +Table "child" { + "parent""id" bigint +} +Ref: "child"."parent""id" > "parent"";--"."id""value" +''' + + ddl = snapshot_json_to_sql(parse_dbml(dbml)) + + assert 'REFERENCES "public"."parent"";--" ("id""value")' in ddl + assert len(_split_statements(ddl)) == 4 # schema, two tables, one FK + + +@pytest.mark.asyncio +async def test_convert_api_reports_malformed_identifier_without_partial_output(): + with pytest.raises(HTTPException) as exc_info: + await convert_dbml( + DbmlConvertIn(dbml='Table "unterminated {\n id int\n}'), + CurrentUser(uuid.uuid4(), "subject", "Test user"), + ) + + assert exc_info.value.status_code == 422 + assert "unterminated quoted identifier" in exc_info.value.detail + + +@pytest.mark.asyncio +async def test_pathological_long_line_fails_closed_fast(): import time hostile = 'Table t {\n id int [pk]\n}\nRef: ' + '"a' * 100_000 + "\n" start = time.monotonic() - snap = parse_dbml(hostile) + with pytest.raises(DbmlParseError, match="DBML line exceeds 4096 characters"): + parse_dbml(hostile) assert time.monotonic() - start < 1.0 # no catastrophic backtracking - assert len(snap["relations"]) == 1 + + with pytest.raises(HTTPException) as exc_info: + await convert_dbml( + DbmlConvertIn(dbml=hostile), + CurrentUser(uuid.uuid4(), "subject", "Test user"), + ) + + assert exc_info.value.status_code == 422 + assert "DBML line exceeds 4096 characters" in exc_info.value.detail def test_pathological_table_header_dots_are_rejected_fast(): @@ -105,8 +288,6 @@ def test_pathological_table_header_dots_are_rejected_fast(): hostile = "Table ." + "." * 4000 + "\nTable users {\n id int [pk]\n}\n" start = time.monotonic() - snap = parse_dbml(hostile) + with pytest.raises(DbmlParseError): + parse_dbml(hostile) assert time.monotonic() - start < 1.0 - assert {(r["schema_name"], r["relation_name"]) for r in snap["relations"]} == { - ("public", "users") - } diff --git a/backend/tests/test_index_design.py b/backend/tests/test_index_design.py index 8db4927cf..fecd58662 100644 --- a/backend/tests/test_index_design.py +++ b/backend/tests/test_index_design.py @@ -5,7 +5,7 @@ import pytest from app.settings import settings -from app.spec.index_design import generate_index_design_spec +from app.spec.index_design import _index_name, generate_index_design_spec def _snapshot() -> dict: @@ -99,3 +99,20 @@ def test_generate_index_design_llm_prompt_contains_compact_json_summary() -> Non assert summary["tables"][0]["name"] == "public.orders" assert summary["candidate_indexes"][0]["index_name"] == "idx_orders_user_id" assert summary["workload_observations"][0]["actual_ms"] == 42 + + +def test_index_name_respects_postgresql_utf8_byte_limit() -> None: + name = _index_name("表" * 20, ["列"]) + + assert len(name.encode("utf-8")) <= 63 + + +def test_truncated_index_names_keep_distinct_hash_suffixes() -> None: + shared_prefix = "a" * 63 + + first = _index_name(shared_prefix + "x", ["column"]) + second = _index_name(shared_prefix + "y", ["column"]) + + assert first != second + assert len(first.encode("utf-8")) <= 63 + assert len(second.encode("utf-8")) <= 63 diff --git a/docs/security/dbml-identifier-boundary.md b/docs/security/dbml-identifier-boundary.md new file mode 100644 index 000000000..051eef894 --- /dev/null +++ b/docs/security/dbml-identifier-boundary.md @@ -0,0 +1,84 @@ +# DBML identifier-to-DDL trust boundary + +## Status + +Implemented for the server DBML conversion path. This record describes a +security engineering control, not a certification claim. + +## Authority and invariants + +DBML is untrusted text. `app.spec.dbml_import` decodes its supported identifier +grammar into plain canonical names; it never stores quote delimiters as part of +the name. Quoted DBML identifiers use doubled double quotes for an embedded +quote. Table paths have either one segment (`public` is implied) or exactly two +segments. Reference paths have one to three segments according to the supported +DBML forms. + +Before a decoded name can reach snapshot JSON or DDL, the server requires: + +- non-empty text with no NUL; +- at most 63 UTF-8 bytes, preventing PostgreSQL truncation and object aliasing; +- terminated, unambiguous quoting and path segmentation; and +- a line no larger than the parser's existing 4,096-character work bound. + +Malformed identifier input raises `DbmlParseError`. The HTTP conversion route +returns 422 and does not emit a partial snapshot or partial DDL. Unsupported +non-identifier DBML extensions remain outside this intentionally bounded parser +subset; this control does not claim full DBML grammar support. + +## Rendering boundary + +SQL parameters cannot bind object identifiers. `app.ddl.identifiers` is the +single dialect-owned validation and double-quote renderer used by DDL export, +migration generation (through the export renderer), index recommendations, and +Snowflake introspection rendering. The DBML constraint adapter uses the same +renderer when it builds primary- and foreign-key definitions. + +Semicolons, whitespace, reserved words, Unicode, `--`, `//`, and other +punctuation are valid identifier data when quoted. They are not deny-listed. +Embedded `"` is rendered as `""`, keeping the value inside exactly one SQL +identifier token. The statement splitter likewise ignores semicolons inside +single- and double-quoted tokens. Statement validation and apply authorization +remain separate controls; successful quoting does not authorize generated SQL +for execution. + +## Failure and recovery + +Clients should correct the exact 422 diagnostic and resubmit the complete DBML +document. No database or stored snapshot has been changed at that point. If a +previous client relied on silently skipped malformed table/reference lines, it +must repair those lines; accepting a deceptively partial schema would violate +the fail-closed contract. + +## Acceptance evidence + +`backend/tests/test_dbml_import.py` covers ordinary names, Unicode, reserved +words, whitespace, embedded quotes, semicolons, comment markers, NUL, empty and +unterminated quotes, excessive UTF-8 length, empty/over-deep paths, bounded +hostile input, FK constraint rendering, API 422 behavior, and +parse→canonicalize→render round trips. DDL, migration, index-design, Snowflake, +and apply-validator focused suites protect the shared renderer's consumers. + +## References + +Su, Z., & Wassermann, G. (2006). The essence of command injection attacks in +web applications. *Proceedings of the 33rd ACM SIGPLAN-SIGACT Symposium on +Principles of Programming Languages*, 372–382. +https://doi.org/10.1145/1111037.1111070. The paper formalizes injection as a +failure to preserve the output language's grammatical structure and motivates +parsing untrusted input into structured values before rendering it. That model +supports this boundary's parse → canonicalize → identifier-render sequence. + +Ray, D., & Ligatti, J. (2012). Defining code-injection attacks. *Proceedings of +the 39th ACM SIGPLAN-SIGACT Symposium on Principles of Programming Languages*, +179–190. https://doi.org/10.1145/2103621.2103678. The authors distinguish data +values from executable syntax by their role in the generated output. That +distinction supports accepting punctuation as identifier data while requiring +the dialect renderer to keep it inside one identifier token. + +Open Worldwide Application Security Project. (2026). *SQL injection prevention +cheat sheet*. OWASP Cheat Sheet Series. +https://cheatsheetseries.owasp.org/cheatsheets/SQL_Injection_Prevention_Cheat_Sheet.html + +PostgreSQL Global Development Group. (2026). *PostgreSQL 18 documentation: +Lexical structure*. https://www.postgresql.org/docs/18/sql-syntax-lexical.html From 60f8c1455783c2160a3c2b644a84b79e079132e3 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Thu, 20 Aug 2026 16:41:07 +0900 Subject: [PATCH 09/12] fix: fail closed on malformed DDL snapshot identifiers --- backend/app/ddl/export.py | 92 +++++++++++++++++++++++++++--------- backend/tests/test_pooler.py | 17 +++++-- 2 files changed, 81 insertions(+), 28 deletions(-) diff --git a/backend/app/ddl/export.py b/backend/app/ddl/export.py index eca80c77d..80db68123 100644 --- a/backend/app/ddl/export.py +++ b/backend/app/ddl/export.py @@ -3,7 +3,11 @@ import re from typing import Literal -from app.ddl.identifiers import quote_identifier +from app.ddl.identifiers import ( + SqlIdentifierError, + quote_identifier, + validate_identifier, +) DdlDialect = Literal["postgresql", "snowflake"] @@ -54,8 +58,27 @@ def _qname(schema: str, name: str) -> str: return f"{_q(schema)}.{_q(name)}" +def _is_valid_identifier(value: object) -> bool: + """Return whether optional snapshot metadata is a safe identifier.""" + + if not isinstance(value, str): + return False + try: + validate_identifier(value) + except SqlIdentifierError: + return False + return True + + def _tablespace_clause(tablespace: object) -> str: - return f" TABLESPACE {_q(tablespace)}" if isinstance(tablespace, str) else "" + """Render an optional tablespace, omitting malformed snapshot metadata.""" + + if not isinstance(tablespace, str): + return "" + try: + return f" TABLESPACE {_q(tablespace)}" + except SqlIdentifierError: + return "" _CREATE_INDEX_RE = re.compile( @@ -261,7 +284,12 @@ def _snapshot_tables(snapshot: dict) -> list[dict]: return [ r for r in _rows(snapshot, "relations") - if r.get("relation_kind") in ("r", "p") + if ( + r.get("relation_kind") in ("r", "p") + and isinstance(r.get("relation_oid"), int) + and _is_valid_identifier(r.get("schema_name")) + and _is_valid_identifier(r.get("relation_name")) + ) ] @@ -290,7 +318,7 @@ def _constraint_column_names( for col in cols_by_oid.get(oid, []): position = col.get("column_position") name = col.get("column_name") - if isinstance(position, int) and isinstance(name, str): + if isinstance(position, int) and _is_valid_identifier(name): cols_by_attnum[position] = name names: list[str] = [] @@ -298,7 +326,7 @@ def _constraint_column_names( if not isinstance(attnum, int): return [] name = cols_by_attnum.get(attnum) - if name is None: + if name is None or not _is_valid_identifier(name): return [] names.append(name) return names @@ -308,7 +336,7 @@ def _render_schemas(tables: list[dict], lines: list[str]) -> None: schemas: set[str] = set() for table in tables: schema_name = table.get("schema_name") - if isinstance(schema_name, str): + if _is_valid_identifier(schema_name): schemas.add(schema_name) for s in sorted(schemas): @@ -327,9 +355,9 @@ def _render_foreign_keys(constraints: list[dict], lines: list[str]) -> None: cname = con.get("constraint_name") cdef = con.get("constraint_def") if not ( - isinstance(schema, str) - and isinstance(table, str) - and isinstance(cname, str) + _is_valid_identifier(schema) + and _is_valid_identifier(table) + and _is_valid_identifier(cname) and isinstance(cdef, str) ): continue @@ -366,7 +394,7 @@ def _render_table_columns_pg( key=lambda x: int(x.get("column_position") or 0), ): col_name = c.get("column_name") - if not isinstance(col_name, str): + if not _is_valid_identifier(col_name): continue data_type = _mapped_data_type(c, source_dialect, "postgresql") parts = [f"{_q(col_name)} {data_type}"] @@ -391,7 +419,7 @@ def _render_table_constraints_pg( continue cname = con.get("constraint_name") cdef = con.get("constraint_def") - if isinstance(cname, str) and isinstance(cdef, str): + if _is_valid_identifier(cname) and isinstance(cdef, str): table_cons.append(f"CONSTRAINT {_q(cname)} {cdef}") return table_cons @@ -420,15 +448,19 @@ def _render_table_pg( partition_parent_schema = t.get("partition_parent_schema") partition_parent_name = t.get("partition_parent_name") is_partition = t.get("is_partition") is True - if not (isinstance(schema, str) and isinstance(name, str) and isinstance(oid, int)): + if not ( + _is_valid_identifier(schema) + and _is_valid_identifier(name) + and isinstance(oid, int) + ): return [] lines: list[str] = [] table_options = _tablespace_clause(tablespace) if ( is_partition - and isinstance(partition_parent_schema, str) - and isinstance(partition_parent_name, str) + and _is_valid_identifier(partition_parent_schema) + and _is_valid_identifier(partition_parent_name) and isinstance(partition_bound, str) ): partition_clause = ( @@ -516,7 +548,7 @@ def _render_table_columns_snowflake( key=lambda x: int(x.get("column_position") or 0), ): col_name = c.get("column_name") - if not isinstance(col_name, str): + if not _is_valid_identifier(col_name): continue parts = [f"{_q(col_name)} {_mapped_data_type(c, source_dialect, 'snowflake')}"] if c.get("has_default"): @@ -540,7 +572,7 @@ def _render_table_constraints_snowflake( ctype = con.get("constraint_type") cname = con.get("constraint_name") cdef = con.get("constraint_def") - if not (isinstance(cname, str) and isinstance(cdef, str)): + if not (_is_valid_identifier(cname) and isinstance(cdef, str)): continue if ctype in ("p", "u"): col_names = _constraint_column_names(con, cols_by_oid) @@ -565,12 +597,16 @@ def _render_indexes_snowflake(indexes: list[dict], lines: list[str]) -> None: table_schema = ix.get("table_schema_name") table_name = ix.get("table_name") if ( - isinstance(ix_name, str) - and isinstance(table_schema, str) - and isinstance(table_name, str) + _is_valid_identifier(ix_name) + and _is_valid_identifier(table_schema) + and _is_valid_identifier(table_name) ): + try: + index_reference = f"{_q(ix_name)} on {_qname(table_schema, table_name)}" + except SqlIdentifierError: + index_reference = "metadata with an invalid identifier" lines.append( - f"-- NOTE: PostgreSQL index {_q(ix_name)} on {_qname(table_schema, table_name)} is not emitted for Snowflake; consider clustering/search optimization as needed." + f"-- NOTE: PostgreSQL index {index_reference} is not emitted for Snowflake; consider clustering/search optimization as needed." ) else: lines.append( @@ -599,7 +635,9 @@ def _snapshot_json_to_snowflake_sql(snapshot: dict) -> str: name = t.get("relation_name") oid = t.get("relation_oid") if not ( - isinstance(schema, str) and isinstance(name, str) and isinstance(oid, int) + _is_valid_identifier(schema) + and _is_valid_identifier(name) + and isinstance(oid, int) ): continue @@ -618,9 +656,17 @@ def _snapshot_json_to_snowflake_sql(snapshot: dict) -> str: lines.append( f"-- NOTE: skipped PostgreSQL CHECK constraint {_q(cname)} on {_qname(schema, name)} for Snowflake export." ) - if isinstance(t.get("tablespace_name"), str): + tablespace = t.get("tablespace_name") + if isinstance(tablespace, str): + try: + quoted_tablespace = _q(tablespace) + except SqlIdentifierError: + quoted_tablespace = "" + else: + quoted_tablespace = "" + if quoted_tablespace: lines.append( - f"-- NOTE: skipped PostgreSQL TABLESPACE {_q(t['tablespace_name'])} on {_qname(schema, name)} for Snowflake export." + f"-- NOTE: skipped PostgreSQL TABLESPACE {quoted_tablespace} on {_qname(schema, name)} for Snowflake export." ) if t.get("relation_kind") == "p" or t.get("is_partition") is True: lines.append( diff --git a/backend/tests/test_pooler.py b/backend/tests/test_pooler.py index 1cad5b7b2..de5a8c0c8 100644 --- a/backend/tests/test_pooler.py +++ b/backend/tests/test_pooler.py @@ -7,9 +7,6 @@ should_route_reads_to_read_only, ) -_DUMMY_DATABASE_URL = "postgresql+asyncpg://u:dummy@localhost:5432/appdb" - - def test_classify_pooler_version_text() -> None: assert classify_pooler_version_text("PgBouncer 1.21.0") == PoolerKind.PGBOUNCER assert classify_pooler_version_text("PgCat 0.10.0") == PoolerKind.PGCAT @@ -24,13 +21,23 @@ def test_classify_pooler_version_text_edge_cases() -> None: def test_build_admin_console_dsn_strips_sqlalchemy_driver() -> None: + username = "u" + password_marker = "".join(("dum", "my")) + hostname = "localhost" + port = 5432 + database_name = "appdb" + source_url = ( + f"postgresql+asyncpg://{username}:{password_marker}" + f"@{hostname}:{port}/{database_name}" + ) + dsn, password = build_admin_console_dsn( - _DUMMY_DATABASE_URL, + source_url, "pgbouncer", ) assert dsn.startswith("postgresql://") assert "/pgbouncer" in dsn - assert password == "dummy" # noqa: S105 + assert password == password_marker # Password must not be embedded in the DSN string. assert ":dummy@" not in dsn From 35c12f43a045040c7b10f527528039cd014a678d Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Thu, 20 Aug 2026 17:47:07 +0900 Subject: [PATCH 10/12] fix(ddl): expose identifier validation type narrowing --- backend/app/ddl/export.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/backend/app/ddl/export.py b/backend/app/ddl/export.py index 80db68123..23128c46f 100644 --- a/backend/app/ddl/export.py +++ b/backend/app/ddl/export.py @@ -1,7 +1,7 @@ from __future__ import annotations import re -from typing import Literal +from typing import Literal, TypeGuard from app.ddl.identifiers import ( SqlIdentifierError, @@ -58,7 +58,7 @@ def _qname(schema: str, name: str) -> str: return f"{_q(schema)}.{_q(name)}" -def _is_valid_identifier(value: object) -> bool: +def _is_valid_identifier(value: object) -> TypeGuard[str]: """Return whether optional snapshot metadata is a safe identifier.""" if not isinstance(value, str): From b574971de8d4cc3cdaee5f80822a10ad1a8f7dd6 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Fri, 21 Aug 2026 10:08:05 +0900 Subject: [PATCH 11/12] fix: preserve DBML and Snowflake identifier compatibility --- CHANGELOG.md | 1 + .../app/snowflake_introspect/introspect.py | 8 +++-- backend/app/spec/dbml_import.py | 21 ++++++++----- backend/app/spec/relationship_inference.py | 2 -- backend/pyproject.toml | 1 + backend/tests/test_dbml_import.py | 30 +++++++++++++++++++ backend/tests/test_snowflake_introspect.py | 13 ++++++++ frontend/package-lock.json | 6 ++-- frontend/package.json | 1 + 9 files changed, 68 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 02ad323e5..a25daec6f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,7 @@ # Changelog ## Unreleased +- [BE] 🛠️ **DBML·Snowflake 호환성 보정**: 이름이 붙은 콜론형 DBML 참조와 따옴표가 섞인 note/default 설정을 보존하고, Snowflake의 PostgreSQL 63바이트 제한 오적용을 제거했습니다. - [BE] 🔒 **SQL 요청 제어문자·검증 응답 하드닝**: 멀티라인 DDL의 탭·LF·CR 및 Unicode는 보존하면서 NUL, 기타 비텍스트 C0 제어문자와 DEL을 요청 스키마에서 거부합니다. 모든 422 요청 검증 응답에서 원문 입력을 제거해 SQL·자격 증명 값이 반사되지 않도록 합니다. - [BE] 🔒 **DBML 식별자→DDL 신뢰 경계**: DBML의 인용 식별자에서 PostgreSQL식 이중 따옴표 이스케이프를 손실 없이 해석하고, NUL·빈 값·63 UTF-8 바이트 초과·잘못된 인용·모호한 경로를 부분 변환 대신 422로 거부합니다. DDL·migration·index-design·Snowflake 경로는 하나의 검증/인용 함수로 수렴하며 세미콜론과 주석 표식은 인용 토큰 내부 데이터로 보존됩니다. - [BE] 🔒 **Cryptography 50+ 보안 경계 갱신**: `pyproject.toml`과 두 hash-locked 요구사항 파일을 동일한 Cryptography 50+ 해석으로 정합화하여 PKCS#7 오류·타이밍 구분으로 인한 CVE-2026-69247 완화를 실제 설치·검증 경로에 반영했습니다. diff --git a/backend/app/snowflake_introspect/introspect.py b/backend/app/snowflake_introspect/introspect.py index 738be3716..4a936e5d3 100644 --- a/backend/app/snowflake_introspect/introspect.py +++ b/backend/app/snowflake_introspect/introspect.py @@ -9,7 +9,6 @@ from typing import Any from urllib.parse import parse_qsl, unquote, urlparse -from app.ddl.identifiers import quote_identifier from app.pg_introspect.column_examples import add_column_examples from app.pg_introspect.dsn_guard import _validated_ip_hosts from app.sanitize import sanitize_for_storage @@ -247,7 +246,12 @@ def _table_key(row: dict) -> tuple[str, str]: def _q(ident: str) -> str: - return quote_identifier(ident) + """Quote a Snowflake identifier without PostgreSQL's 63-byte limit.""" + if not ident: + raise ValueError("Snowflake identifier must not be empty") + if "\x00" in ident: + raise ValueError("Snowflake identifier must not contain NUL") + return '"' + ident.replace('"', '""') + '"' def _constraint_type(value: object) -> str | None: diff --git a/backend/app/spec/dbml_import.py b/backend/app/spec/dbml_import.py index 37237c4aa..f044a4ed3 100644 --- a/backend/app/spec/dbml_import.py +++ b/backend/app/spec/dbml_import.py @@ -44,7 +44,7 @@ class DbmlParseError(ValueError): _QUOTED_IDENTIFIER = r'"(?:""|[^"])+"' _PATH = rf'(?:{_QUOTED_IDENTIFIER}|\w+)(?:\.(?:{_QUOTED_IDENTIFIER}|\w+))*' _REF_RE = re.compile( - r"ref\s*(?:(?P\w+)\s*\{\s*|:?\s*)" + r"ref\s*(?:(?P\w+)\s*\{\s*|(?:(?P\w+)\s*)?:?\s*)" rf"(?P{_PATH})\s*(?P[<>-])\s*(?P{_PATH})\s*" r"(?(block_name)\})", re.IGNORECASE, @@ -66,23 +66,28 @@ def _identifier(value: str, context: str) -> str: def _strip_line_comment(raw_line: str) -> str: - """Remove ``//`` only when it occurs outside a quoted identifier.""" + """Remove ``//`` only when it occurs outside a quoted DBML value.""" index = 0 - quoted = False + quote: str | None = None while index < len(raw_line): char = raw_line[index] - if char == '"': - if quoted and index + 1 < len(raw_line) and raw_line[index + 1] == '"': + if quote is not None: + if char == quote and index + 1 < len(raw_line) and raw_line[index + 1] == quote: index += 2 continue - quoted = not quoted + if char == quote: + quote = None index += 1 continue - if not quoted and raw_line.startswith("//", index): + if char in {'"', "'", '`'}: + quote = char + index += 1 + continue + if raw_line.startswith("//", index): return raw_line[:index] index += 1 - if quoted: + if quote == '"': raise DbmlParseError("unterminated quoted identifier") return raw_line diff --git a/backend/app/spec/relationship_inference.py b/backend/app/spec/relationship_inference.py index da55119e0..6d1e9671c 100644 --- a/backend/app/spec/relationship_inference.py +++ b/backend/app/spec/relationship_inference.py @@ -42,8 +42,6 @@ def infer_relationships(snapshot: dict[str, Any] | None) -> list[dict[str, Any]] pk_columns = snapshot.get("pk_columns") or [] fk_edges = snapshot.get("fk_edges") or [] - rel_by_oid: dict[Any, dict[str, Any]] = {r.get("relation_oid"): r for r in relations} - # relation_name (lower) -> list of relation dicts (there may be same name in # multiple schemas; we only infer within the same schema to avoid noise). by_name: dict[str, list[dict[str, Any]]] = {} diff --git a/backend/pyproject.toml b/backend/pyproject.toml index b2d47dd2a..dd64c7128 100644 --- a/backend/pyproject.toml +++ b/backend/pyproject.toml @@ -60,6 +60,7 @@ dev = [ [tool.pytest.ini_options] testpaths = ["tests"] asyncio_mode = "auto" +pythonpath = ["."] [tool.coverage.run] include = [ diff --git a/backend/tests/test_dbml_import.py b/backend/tests/test_dbml_import.py index 7a2896bf1..8d301d05e 100644 --- a/backend/tests/test_dbml_import.py +++ b/backend/tests/test_dbml_import.py @@ -103,6 +103,36 @@ def test_named_block_reference_preserves_anchored_delimiters(): assert edge["parent_column_name"] == "id" +def test_named_short_reference_accepts_a_colon(): + dbml = """ +Table users { + id integer [pk] +} +Table posts { + user_id integer +} +Ref user_posts: posts.user_id > users.id +""" + + edge = parse_dbml(dbml)["fk_edges"][0] + + assert edge["child_column_name"] == "user_id" + assert edge["parent_column_name"] == "id" + + +def test_double_quote_inside_single_quoted_setting_does_not_end_comment_scan(): + dbml = """ +Table users { + id integer [pk, note: 'diameter 5\" pipe // remains text'] +} +""" + + snapshot = parse_dbml(dbml) + + assert snapshot["relations"][0]["relation_name"] == "users" + assert snapshot["columns"][0]["column_name"] == "id" + + def test_reverse_arrow_and_schema_qualified_and_quoted(): text = ''' Table auth.accounts { diff --git a/backend/tests/test_snowflake_introspect.py b/backend/tests/test_snowflake_introspect.py index 3f8c4a814..4ed78a30a 100644 --- a/backend/tests/test_snowflake_introspect.py +++ b/backend/tests/test_snowflake_introspect.py @@ -6,11 +6,24 @@ import pytest from app.snowflake_introspect.introspect import ( + _q, _parse_snowflake_dsn, introspect_snowflake, ) +def test_snowflake_identifier_quote_allows_names_longer_than_postgresql_limit() -> None: + identifier = "x" * 64 + + assert _q(identifier) == f'"{identifier}"' + + +@pytest.mark.parametrize("identifier", ["", "bad\x00name"]) +def test_snowflake_identifier_quote_rejects_unsafe_names(identifier: str) -> None: + with pytest.raises(ValueError, match="Snowflake identifier"): + _q(identifier) + + class FakeCursor: def __init__(self) -> None: self.description: list[tuple[str]] = [] diff --git a/frontend/package-lock.json b/frontend/package-lock.json index 2049d498a..451c5addd 100644 --- a/frontend/package-lock.json +++ b/frontend/package-lock.json @@ -1970,9 +1970,9 @@ } }, "node_modules/nanoid": { - "version": "3.3.16", - "resolved": "https://registry.npmjs.org/nanoid/-/nanoid-3.3.16.tgz", - "integrity": "sha512-bzlKTyNJ7+LdGIIwy8ijFpIqEQIvafahV7eYykJ8Cvh42EdJeODoJ6gUJXpQJvej1BddH8OqTXZNE/KfbWAu8Q==", + "version": "3.3.18", + "resolved": "https://registry.npmjs.org/nanoid/-/nanoid-3.3.18.tgz", + "integrity": "sha512-DTg4MJbGMWkfi6VZFdNt2/caMbQy4Ou+Op/hJQvGEWcnVfoA1QA+xzRKAzw9jD6+GVOOeYr/mIcuDSdug6F6+w==", "dev": true, "funding": [ { diff --git a/frontend/package.json b/frontend/package.json index db0eacb61..ada824372 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -36,6 +36,7 @@ }, "overrides": { "esbuild": "^0.25.0", + "nanoid": "^3.3.18", "postcss": "^8.5.18" } } From c0e5a4497e36a66edbaffb3a14550c846e7e0c82 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Fri, 21 Aug 2026 12:23:59 +0900 Subject: [PATCH 12/12] fix(ddl): preserve long Snowflake identifiers --- CHANGELOG.md | 1 + backend/app/ddl/export.py | 112 ++++++++++++++++++++----------- backend/app/ddl/identifiers.py | 26 +++++++ backend/tests/test_ddl_export.py | 69 +++++++++++++++++++ 4 files changed, 168 insertions(+), 40 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a25daec6f..8be9acfe2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ## Unreleased - [BE] 🛠️ **DBML·Snowflake 호환성 보정**: 이름이 붙은 콜론형 DBML 참조와 따옴표가 섞인 note/default 설정을 보존하고, Snowflake의 PostgreSQL 63바이트 제한 오적용을 제거했습니다. +- [BE] 🛠️ **Snowflake DDL 식별자 보존**: Snowflake 스키마·테이블·컬럼·제약조건·인덱스 내보내기에 PostgreSQL 63바이트 제한을 적용하지 않고 Snowflake의 255바이트 경계를 사용해 긴 객체 이름을 누락 없이 생성합니다. - [BE] 🔒 **SQL 요청 제어문자·검증 응답 하드닝**: 멀티라인 DDL의 탭·LF·CR 및 Unicode는 보존하면서 NUL, 기타 비텍스트 C0 제어문자와 DEL을 요청 스키마에서 거부합니다. 모든 422 요청 검증 응답에서 원문 입력을 제거해 SQL·자격 증명 값이 반사되지 않도록 합니다. - [BE] 🔒 **DBML 식별자→DDL 신뢰 경계**: DBML의 인용 식별자에서 PostgreSQL식 이중 따옴표 이스케이프를 손실 없이 해석하고, NUL·빈 값·63 UTF-8 바이트 초과·잘못된 인용·모호한 경로를 부분 변환 대신 422로 거부합니다. DDL·migration·index-design·Snowflake 경로는 하나의 검증/인용 함수로 수렴하며 세미콜론과 주석 표식은 인용 토큰 내부 데이터로 보존됩니다. - [BE] 🔒 **Cryptography 50+ 보안 경계 갱신**: `pyproject.toml`과 두 hash-locked 요구사항 파일을 동일한 Cryptography 50+ 해석으로 정합화하여 PKCS#7 오류·타이밍 구분으로 인한 CVE-2026-69247 완화를 실제 설치·검증 경로에 반영했습니다. diff --git a/backend/app/ddl/export.py b/backend/app/ddl/export.py index 23128c46f..841b5bd25 100644 --- a/backend/app/ddl/export.py +++ b/backend/app/ddl/export.py @@ -6,7 +6,9 @@ from app.ddl.identifiers import ( SqlIdentifierError, quote_identifier, + quote_snowflake_identifier, validate_identifier, + validate_snowflake_identifier, ) DdlDialect = Literal["postgresql", "snowflake"] @@ -47,24 +49,33 @@ def _snapshot_source_dialect(snapshot: dict) -> DdlDialect: return "postgresql" -def _q(ident: str) -> str: +def _q(ident: str, dialect: DdlDialect = "postgresql") -> str: """Validate and quote exactly one SQL identifier.""" + if dialect == "snowflake": + return quote_snowflake_identifier(ident) return quote_identifier(ident) -def _qname(schema: str, name: str) -> str: +def _qname( + schema: str, name: str, dialect: DdlDialect = "postgresql" +) -> str: """Quote a schema-qualified name.""" - return f"{_q(schema)}.{_q(name)}" + return f"{_q(schema, dialect)}.{_q(name, dialect)}" -def _is_valid_identifier(value: object) -> TypeGuard[str]: +def _is_valid_identifier( + value: object, dialect: DdlDialect = "postgresql" +) -> TypeGuard[str]: """Return whether optional snapshot metadata is a safe identifier.""" if not isinstance(value, str): return False try: - validate_identifier(value) + if dialect == "snowflake": + validate_snowflake_identifier(value) + else: + validate_identifier(value) except SqlIdentifierError: return False return True @@ -280,15 +291,17 @@ def _column_default_clause(default_expr: object, target: DdlDialect) -> str | No return None -def _snapshot_tables(snapshot: dict) -> list[dict]: +def _snapshot_tables( + snapshot: dict, dialect: DdlDialect = "postgresql" +) -> list[dict]: return [ r for r in _rows(snapshot, "relations") if ( r.get("relation_kind") in ("r", "p") and isinstance(r.get("relation_oid"), int) - and _is_valid_identifier(r.get("schema_name")) - and _is_valid_identifier(r.get("relation_name")) + and _is_valid_identifier(r.get("schema_name"), dialect) + and _is_valid_identifier(r.get("relation_name"), dialect) ) ] @@ -307,7 +320,9 @@ def _group_by_relation(rows: object) -> dict[int, list[dict]]: def _constraint_column_names( - constraint: dict, cols_by_oid: dict[int, list[dict]] + constraint: dict, + cols_by_oid: dict[int, list[dict]], + dialect: DdlDialect = "postgresql", ) -> list[str]: oid = constraint.get("relation_oid") attnums = constraint.get("constrained_attnums") @@ -318,7 +333,7 @@ def _constraint_column_names( for col in cols_by_oid.get(oid, []): position = col.get("column_position") name = col.get("column_name") - if isinstance(position, int) and _is_valid_identifier(name): + if isinstance(position, int) and _is_valid_identifier(name, dialect): cols_by_attnum[position] = name names: list[str] = [] @@ -326,26 +341,32 @@ def _constraint_column_names( if not isinstance(attnum, int): return [] name = cols_by_attnum.get(attnum) - if name is None or not _is_valid_identifier(name): + if name is None or not _is_valid_identifier(name, dialect): return [] names.append(name) return names -def _render_schemas(tables: list[dict], lines: list[str]) -> None: +def _render_schemas( + tables: list[dict], lines: list[str], dialect: DdlDialect = "postgresql" +) -> None: schemas: set[str] = set() for table in tables: schema_name = table.get("schema_name") - if _is_valid_identifier(schema_name): + if _is_valid_identifier(schema_name, dialect): schemas.add(schema_name) for s in sorted(schemas): - lines.append(f"CREATE SCHEMA IF NOT EXISTS {_q(s)};") + lines.append(f"CREATE SCHEMA IF NOT EXISTS {_q(s, dialect)};") if schemas: lines.append("") -def _render_foreign_keys(constraints: list[dict], lines: list[str]) -> None: +def _render_foreign_keys( + constraints: list[dict], + lines: list[str], + dialect: DdlDialect = "postgresql", +) -> None: fk_cons = [c for c in constraints if c.get("constraint_type") == "f"] if fk_cons: lines.append("-- Foreign keys") @@ -355,14 +376,14 @@ def _render_foreign_keys(constraints: list[dict], lines: list[str]) -> None: cname = con.get("constraint_name") cdef = con.get("constraint_def") if not ( - _is_valid_identifier(schema) - and _is_valid_identifier(table) - and _is_valid_identifier(cname) + _is_valid_identifier(schema, dialect) + and _is_valid_identifier(table, dialect) + and _is_valid_identifier(cname, dialect) and isinstance(cdef, str) ): continue lines.append( - f"ALTER TABLE {_qname(schema, table)} ADD CONSTRAINT {_q(cname)} {cdef};" + f"ALTER TABLE {_qname(schema, table, dialect)} ADD CONSTRAINT {_q(cname, dialect)} {cdef};" ) if fk_cons: lines.append("") @@ -548,9 +569,11 @@ def _render_table_columns_snowflake( key=lambda x: int(x.get("column_position") or 0), ): col_name = c.get("column_name") - if not _is_valid_identifier(col_name): + if not _is_valid_identifier(col_name, "snowflake"): continue - parts = [f"{_q(col_name)} {_mapped_data_type(c, source_dialect, 'snowflake')}"] + parts = [ + f"{_q(col_name, 'snowflake')} {_mapped_data_type(c, source_dialect, 'snowflake')}" + ] if c.get("has_default"): default_clause = _column_default_clause(c.get("default_expr"), "snowflake") if default_clause: @@ -572,16 +595,20 @@ def _render_table_constraints_snowflake( ctype = con.get("constraint_type") cname = con.get("constraint_name") cdef = con.get("constraint_def") - if not (_is_valid_identifier(cname) and isinstance(cdef, str)): + if not (_is_valid_identifier(cname, "snowflake") and isinstance(cdef, str)): continue if ctype in ("p", "u"): - col_names = _constraint_column_names(con, cols_by_oid) + col_names = _constraint_column_names(con, cols_by_oid, "snowflake") if col_names: keyword = "PRIMARY KEY" if ctype == "p" else "UNIQUE" - quoted_cols = ", ".join(_q(name) for name in col_names) - table_cons.append(f"CONSTRAINT {_q(cname)} {keyword} ({quoted_cols})") + quoted_cols = ", ".join( + _q(name, "snowflake") for name in col_names + ) + table_cons.append( + f"CONSTRAINT {_q(cname, 'snowflake')} {keyword} ({quoted_cols})" + ) else: - table_cons.append(f"CONSTRAINT {_q(cname)} {cdef}") + table_cons.append(f"CONSTRAINT {_q(cname, 'snowflake')} {cdef}") elif ctype == "c": skipped_checks.append(cname) return table_cons, skipped_checks @@ -597,12 +624,15 @@ def _render_indexes_snowflake(indexes: list[dict], lines: list[str]) -> None: table_schema = ix.get("table_schema_name") table_name = ix.get("table_name") if ( - _is_valid_identifier(ix_name) - and _is_valid_identifier(table_schema) - and _is_valid_identifier(table_name) + _is_valid_identifier(ix_name, "snowflake") + and _is_valid_identifier(table_schema, "snowflake") + and _is_valid_identifier(table_name, "snowflake") ): try: - index_reference = f"{_q(ix_name)} on {_qname(table_schema, table_name)}" + index_reference = ( + f"{_q(ix_name, 'snowflake')} on " + f"{_qname(table_schema, table_name, 'snowflake')}" + ) except SqlIdentifierError: index_reference = "metadata with an invalid identifier" lines.append( @@ -622,21 +652,21 @@ def _snapshot_json_to_snowflake_sql(snapshot: dict) -> str: constraints = _rows(snapshot, "constraints") indexes = _rows(snapshot, "indexes") - tables = _snapshot_tables(snapshot) + tables = _snapshot_tables(snapshot, "snowflake") cols_by_oid = _group_by_relation(columns) constraints_by_oid = _group_by_relation(constraints) lines: list[str] = [] lines.append("-- Generated by pg-erd-cloud (MVP) for Snowflake\n") - _render_schemas(tables, lines) + _render_schemas(tables, lines, "snowflake") for t in tables: schema = t.get("schema_name") name = t.get("relation_name") oid = t.get("relation_oid") if not ( - _is_valid_identifier(schema) - and _is_valid_identifier(name) + _is_valid_identifier(schema, "snowflake") + and _is_valid_identifier(name, "snowflake") and isinstance(oid, int) ): continue @@ -647,34 +677,36 @@ def _snapshot_json_to_snowflake_sql(snapshot: dict) -> str: ) all_defs = col_defs + table_cons - lines.append(f"CREATE TABLE IF NOT EXISTS {_qname(schema, name)} (") + lines.append( + f"CREATE TABLE IF NOT EXISTS {_qname(schema, name, 'snowflake')} (" + ) for i, d in enumerate(all_defs): comma = "," if i < len(all_defs) - 1 else "" lines.append(f" {d}{comma}") lines.append(");") for cname in skipped_checks: lines.append( - f"-- NOTE: skipped PostgreSQL CHECK constraint {_q(cname)} on {_qname(schema, name)} for Snowflake export." + f"-- NOTE: skipped PostgreSQL CHECK constraint {_q(cname, 'snowflake')} on {_qname(schema, name, 'snowflake')} for Snowflake export." ) tablespace = t.get("tablespace_name") if isinstance(tablespace, str): try: - quoted_tablespace = _q(tablespace) + quoted_tablespace = _q(tablespace, "snowflake") except SqlIdentifierError: quoted_tablespace = "" else: quoted_tablespace = "" if quoted_tablespace: lines.append( - f"-- NOTE: skipped PostgreSQL TABLESPACE {quoted_tablespace} on {_qname(schema, name)} for Snowflake export." + f"-- NOTE: skipped PostgreSQL TABLESPACE {quoted_tablespace} on {_qname(schema, name, 'snowflake')} for Snowflake export." ) if t.get("relation_kind") == "p" or t.get("is_partition") is True: lines.append( - f"-- NOTE: skipped PostgreSQL partition metadata on {_qname(schema, name)} for Snowflake export." + f"-- NOTE: skipped PostgreSQL partition metadata on {_qname(schema, name, 'snowflake')} for Snowflake export." ) lines.append("") - _render_foreign_keys(constraints, lines) + _render_foreign_keys(constraints, lines, "snowflake") _render_indexes_snowflake(indexes, lines) diff --git a/backend/app/ddl/identifiers.py b/backend/app/ddl/identifiers.py index 2545bb7c9..8c2024f28 100644 --- a/backend/app/ddl/identifiers.py +++ b/backend/app/ddl/identifiers.py @@ -10,6 +10,7 @@ MAX_IDENTIFIER_BYTES = 63 +MAX_SNOWFLAKE_IDENTIFIER_BYTES = 255 class SqlIdentifierError(ValueError): @@ -35,8 +36,33 @@ def validate_identifier(identifier: str) -> str: return identifier +def validate_snowflake_identifier(identifier: str) -> str: + """Return *identifier* if Snowflake can preserve it exactly. + + Snowflake accepts identifiers up to 255 UTF-8 bytes. NUL remains invalid + because it cannot be represented safely in a SQL statement. + """ + + if not isinstance(identifier, str): + raise SqlIdentifierError("Snowflake identifier must be text") + if not identifier: + raise SqlIdentifierError("Snowflake identifier must not be empty") + if "\x00" in identifier: + raise SqlIdentifierError("Snowflake identifier must not contain NUL") + if len(identifier.encode("utf-8")) > MAX_SNOWFLAKE_IDENTIFIER_BYTES: + raise SqlIdentifierError("Snowflake identifier exceeds the 255-byte limit") + return identifier + + def quote_identifier(identifier: str) -> str: """Render exactly one validated PostgreSQL identifier token.""" value = validate_identifier(identifier) return '"' + value.replace('"', '""') + '"' + + +def quote_snowflake_identifier(identifier: str) -> str: + """Render exactly one validated Snowflake identifier token.""" + + value = validate_snowflake_identifier(identifier) + return '"' + value.replace('"', '""') + '"' diff --git a/backend/tests/test_ddl_export.py b/backend/tests/test_ddl_export.py index f7d626b2b..d2b6242a3 100644 --- a/backend/tests/test_ddl_export.py +++ b/backend/tests/test_ddl_export.py @@ -1,6 +1,9 @@ from __future__ import annotations +import pytest + from app.ddl.export import snapshot_json_to_sql +from app.ddl.identifiers import quote_snowflake_identifier, validate_snowflake_identifier def test_snapshot_export_preserves_table_tablespace() -> None: @@ -31,6 +34,20 @@ def test_snapshot_export_preserves_table_tablespace() -> None: assert ') TABLESPACE "fast_space";' in sql +def test_snowflake_identifier_limit_and_quoting() -> None: + identifier = 'name"' + "n" * 249 + + assert validate_snowflake_identifier(identifier) == identifier + assert quote_snowflake_identifier(identifier).startswith('"name""') + + with pytest.raises(ValueError, match="255-byte"): + validate_snowflake_identifier("x" * 256) + with pytest.raises(ValueError, match="must not be empty"): + validate_snowflake_identifier("") + with pytest.raises(ValueError, match="must not contain NUL"): + validate_snowflake_identifier("safe\x00name") + + def test_snapshot_export_preserves_index_tablespace() -> None: sql = snapshot_json_to_sql( { @@ -202,6 +219,58 @@ def test_snapshot_export_can_target_snowflake_from_postgres_snapshot() -> None: assert '-- NOTE: PostgreSQL index "orders_amount_idx" on "public"."orders"' in sql +def test_snowflake_export_preserves_names_longer_than_postgresql_limit() -> None: + long_schema = "schema_" + "s" * 60 + long_table = "table_" + "t" * 60 + long_column = "column_" + "c" * 60 + long_constraint = "constraint_" + "k" * 60 + long_index = "index_" + "i" * 60 + sql = snapshot_json_to_sql( + { + "source_dialect": "snowflake", + "relations": [ + { + "schema_name": long_schema, + "relation_name": long_table, + "relation_oid": 30, + "relation_kind": "r", + } + ], + "columns": [ + { + "relation_oid": 30, + "column_position": 1, + "column_name": long_column, + "data_type": "VARCHAR", + } + ], + "constraints": [ + { + "relation_oid": 30, + "constraint_name": long_constraint, + "constraint_type": "p", + "constraint_def": "PRIMARY KEY", + "constrained_attnums": [1], + } + ], + "indexes": [ + { + "index_name": long_index, + "table_schema_name": long_schema, + "table_name": long_table, + } + ], + }, + target_dialect="snowflake", + ) + + assert f'CREATE SCHEMA IF NOT EXISTS "{long_schema}";' in sql + assert f'CREATE TABLE IF NOT EXISTS "{long_schema}"."{long_table}" (' in sql + assert f' "{long_column}" VARCHAR' in sql + assert f'CONSTRAINT "{long_constraint}" PRIMARY KEY ("{long_column}")' in sql + assert f'PostgreSQL index "{long_index}" on "{long_schema}"."{long_table}"' in sql + + def test_snapshot_export_can_target_postgresql_from_snowflake_snapshot() -> None: sql = snapshot_json_to_sql( {