diff --git a/.jules/sentinel.md b/.jules/sentinel.md index 1c145b3a..1fc9dce5 100644 --- a/.jules/sentinel.md +++ b/.jules/sentinel.md @@ -2,3 +2,7 @@ **Vulnerability:** User-provided string fields (like project and connection names) lacked strict validation against control characters, only relying on length constraints. **Learning:** This could potentially lead to Log Injection (CRLF injection), Null Byte Injection, or terminal escape injection if these strings are subsequently logged or rendered directly. **Prevention:** Use explicit regex validation `pattern=r'^[^\x00-\x1F\x7F]+$'` on Pydantic string fields to strictly reject control characters. +## 2026-08-01 - Hardening Unprotected Pydantic String Fields Against Control Characters +**Vulnerability:** Several Pydantic string fields (`DiagramViewCreateIn.name`, `TableAnnotationUpsertIn.schema_name`, `TableAnnotationUpsertIn.relation_name`, `ApiKeyCreateIn.key_name`) were missing the `pattern` parameter to restrict ASCII control characters. +**Learning:** Even when some fields have validation, developers often miss applying the same rigorous rules to new schemas. If a field lacks strict regex filtering (e.g., `pattern=r"^[^\x00-\x1F\x7F]+$"`), attackers can inject newlines, carriage returns, or terminal escapes which can lead to log forging or unexpected parsing errors down the line. +**Prevention:** Systematically apply `pattern=r"^[^\x00-\x1F\x7F]+$"` to all new identifier or name-based string fields in `backend/app/schemas.py`. diff --git a/backend/app/schemas.py b/backend/app/schemas.py index d7c6de77..84bc3bb3 100644 --- a/backend/app/schemas.py +++ b/backend/app/schemas.py @@ -190,7 +190,7 @@ class IndexRedundancyOut(BaseModel): class DiagramViewCreateIn(BaseModel): """Request body for saving an ERD canvas view.""" - name: str = Field(min_length=1, max_length=200) + name: str = Field(min_length=1, max_length=200, pattern=r"^[^\x00-\x1F\x7F]+$") # Opaque client layout (node positions, hidden tables, viewport). The API # bounds the serialized size in the endpoint to prevent abuse. layout_json: dict @@ -214,8 +214,12 @@ class DiagramViewDetailOut(DiagramViewOut): class TableAnnotationUpsertIn(BaseModel): """Request body for creating/updating a table annotation.""" - schema_name: str = Field(min_length=1, max_length=255) - relation_name: str = Field(min_length=1, max_length=255) + schema_name: str = Field( + min_length=1, max_length=255, pattern=r"^[^\x00-\x1F\x7F]+$" + ) + relation_name: str = Field( + min_length=1, max_length=255, pattern=r"^[^\x00-\x1F\x7F]+$" + ) body: str = Field(min_length=1, max_length=10_000) @@ -302,7 +306,7 @@ class DbmlConvertOut(BaseModel): class ApiKeyCreateIn(BaseModel): """Request body for creating an API key.""" - key_name: str = Field(min_length=1, max_length=128) + key_name: str = Field(min_length=1, max_length=128, pattern=r"^[^\x00-\x1F\x7F]+$") class ApiKeyOut(BaseModel): diff --git a/backend/tests/test_schema_validation.py b/backend/tests/test_schema_validation.py index 317292b8..a50134d9 100644 --- a/backend/tests/test_schema_validation.py +++ b/backend/tests/test_schema_validation.py @@ -3,7 +3,14 @@ import pytest from pydantic import ValidationError -from app.schemas import ConnectionCreateIn, ProjectCreateIn, ProjectMemberAddIn +from app.schemas import ( + ApiKeyCreateIn, + ConnectionCreateIn, + DiagramViewCreateIn, + ProjectCreateIn, + ProjectMemberAddIn, + TableAnnotationUpsertIn, +) def test_project_name_length_is_bounded() -> None: @@ -37,3 +44,61 @@ 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") + + +@pytest.mark.parametrize( + "valid_input", + [ + "Valid Name", + "한글 이름", + "データベース", + "🚀 Project", + "name_with_underscores", + "name-with-dashes", + ], +) +def test_hardened_pydantic_strings_accept_valid_input(valid_input: str) -> None: + """Accept realistic printable names in every hardened request schema.""" + DiagramViewCreateIn(name=valid_input, layout_json={}) + TableAnnotationUpsertIn( + schema_name=valid_input, relation_name=valid_input, body="body" + ) + ApiKeyCreateIn(key_name=valid_input) + + +@pytest.mark.parametrize( + "control_char", + [chr(i) for i in range(32)] + [chr(127)], +) +@pytest.mark.parametrize("position_fmt", ["{}suffix", "pre{}post", "prefix{}"]) +def test_hardened_pydantic_strings_reject_control_characters( + control_char: str, position_fmt: str +) -> None: + """Reject ASCII controls and DEL at every position in hardened names.""" + test_str = position_fmt.format(control_char) + + with pytest.raises(ValidationError): + DiagramViewCreateIn(name=test_str, layout_json={}) + + with pytest.raises(ValidationError): + TableAnnotationUpsertIn( + schema_name=test_str, relation_name="valid", body="body" + ) + + with pytest.raises(ValidationError): + TableAnnotationUpsertIn( + schema_name="valid", relation_name=test_str, body="body" + ) + + with pytest.raises(ValidationError): + ApiKeyCreateIn(key_name=test_str) + + +def test_table_annotation_body_allows_multiline() -> None: + """Ensure the body field is untouched by the strict validation.""" + multiline_body = "Line 1\nLine 2\r\nLine 3\t(with tab)" + TableAnnotationUpsertIn( + schema_name="public", + relation_name="users", + body=multiline_body, + )