feat(import): commit S4.4 JSON→PostgreSQL importer + API error-handler hardening
Re-verified staged increment from a clean requirements.lock.txt venv: - 330 backend tests pass (17/17 in new error_handlers + json_import tests) - compileall + frontend npm build clean - git diff --check clean; no secrets in diff - importer CLI dry-run bootstrap works Includes JSON HTTPException handler under /api/* and parse-safe static 404 via abort. JSON stores remain runtime-authoritative; production operation still gated behind operator approval.
This commit is contained in:
@@ -2,10 +2,11 @@
|
||||
from __future__ import annotations
|
||||
|
||||
import functools
|
||||
import json
|
||||
from typing import Any, Callable
|
||||
|
||||
from flask import current_app, g, jsonify, request
|
||||
from werkzeug.exceptions import RequestEntityTooLarge
|
||||
from flask import current_app, g, has_request_context, jsonify, request
|
||||
from werkzeug.exceptions import HTTPException, RequestEntityTooLarge
|
||||
|
||||
from ..auth.users import AuthError, is_valid_tenant_id
|
||||
from ..config import Config
|
||||
@@ -175,6 +176,13 @@ def request_too_large_handler(_err: RequestEntityTooLarge):
|
||||
|
||||
|
||||
def unhandled_error_handler(err: Exception):
|
||||
if isinstance(err, HTTPException):
|
||||
response = err.get_response()
|
||||
if has_request_context() and (request.path == "/api" or request.path.startswith("/api/")):
|
||||
description = err.description if isinstance(err.description, str) else err.name
|
||||
response.data = json.dumps({"error": description}, ensure_ascii=False).encode("utf-8")
|
||||
response.content_type = "application/json"
|
||||
return response
|
||||
current_app.logger.error(
|
||||
"unhandled request error (error_type=%s)", type(err).__name__
|
||||
)
|
||||
@@ -184,5 +192,6 @@ def unhandled_error_handler(err: Exception):
|
||||
def register_error_handlers(app) -> None:
|
||||
app.register_error_handler(ApiError, api_error_handler)
|
||||
app.register_error_handler(RequestEntityTooLarge, request_too_large_handler)
|
||||
app.register_error_handler(HTTPException, unhandled_error_handler)
|
||||
app.register_error_handler(ValueError, lambda _e: (jsonify({"error": "invalid request"}), 400))
|
||||
app.register_error_handler(Exception, unhandled_error_handler)
|
||||
|
||||
@@ -3,7 +3,7 @@ from __future__ import annotations
|
||||
|
||||
from pathlib import Path
|
||||
|
||||
from flask import Flask
|
||||
from flask import Flask, abort
|
||||
from flask_cors import CORS
|
||||
|
||||
from .auth.users import AuthError, UserStore
|
||||
@@ -114,8 +114,8 @@ def _register_frontend(app: Flask) -> None:
|
||||
@app.route("/<path:path>", methods=["GET", "HEAD", "OPTIONS", "POST", "PUT", "DELETE", "PATCH"])
|
||||
def assets(path: str):
|
||||
# Never let the SPA fallback shadow API/auth routes: return 404 for them.
|
||||
if path.startswith("api/") or path.startswith("health"):
|
||||
return ("not found", 404)
|
||||
if path == "api" or path.startswith("api/") or path.startswith("health"):
|
||||
abort(404)
|
||||
candidate = dist / path
|
||||
if candidate.is_file():
|
||||
return send_from_directory(dist, path)
|
||||
|
||||
@@ -30,6 +30,12 @@ from app.models import Group, Message, Organization, Persona, TrainingSession, U
|
||||
from app.services.store import revealable_view
|
||||
|
||||
MAX_SOURCE_FILE_BYTES = 10 * 1024 * 1024
|
||||
MAX_SOURCE_TOTAL_BYTES = 64 * 1024 * 1024
|
||||
MAX_SOURCE_FILES = 5_000
|
||||
MAX_SOURCE_RECORDS = 50_000
|
||||
MAX_SOURCE_ROWS = 100_000
|
||||
MAX_SOURCE_MESSAGES = 100_000
|
||||
MAX_SOURCE_JSON_DEPTH = 32
|
||||
_ID_RE = re.compile(r"^[A-Za-z0-9_.:-]{1,64}$")
|
||||
_COLLECTIONS = ("orgs", "users", "groups", "sessions", "my_personas")
|
||||
|
||||
@@ -47,6 +53,26 @@ class SourceSnapshot:
|
||||
my_personas: list[dict[str, Any]]
|
||||
|
||||
|
||||
@dataclass
|
||||
class _SourceBudget:
|
||||
files: int = 0
|
||||
bytes: int = 0
|
||||
records: int = 0
|
||||
|
||||
def account_file(self, size: int) -> None:
|
||||
self.files += 1
|
||||
self.bytes += size
|
||||
if self.files > MAX_SOURCE_FILES:
|
||||
raise ImportValidationError("source file limit exceeded")
|
||||
if self.bytes > MAX_SOURCE_TOTAL_BYTES:
|
||||
raise ImportValidationError("total source size exceeds bounded limit")
|
||||
|
||||
def account_record(self) -> None:
|
||||
self.records += 1
|
||||
if self.records > MAX_SOURCE_RECORDS:
|
||||
raise ImportValidationError("source record limit exceeded")
|
||||
|
||||
|
||||
@dataclass(frozen=True)
|
||||
class ImportRow:
|
||||
table: str
|
||||
@@ -55,41 +81,94 @@ class ImportRow:
|
||||
values: dict[str, Any]
|
||||
|
||||
|
||||
def _read_collection(source_root: Path, collection: str) -> list[dict[str, Any]]:
|
||||
def _validate_json_depth(value: object, collection: str) -> None:
|
||||
stack: list[tuple[object, int]] = [(value, 1)]
|
||||
while stack:
|
||||
current, depth = stack.pop()
|
||||
if depth > MAX_SOURCE_JSON_DEPTH:
|
||||
raise ImportValidationError(f"source JSON nesting limit exceeded: {collection}")
|
||||
if isinstance(current, dict):
|
||||
stack.extend((child, depth + 1) for child in current.values())
|
||||
elif isinstance(current, list):
|
||||
stack.extend((child, depth + 1) for child in current)
|
||||
|
||||
|
||||
def _read_bounded_bytes(path: Path, *, max_bytes: int, too_large_message: str) -> bytes:
|
||||
with path.open("rb") as handle:
|
||||
content = handle.read(max_bytes + 1)
|
||||
if len(content) > max_bytes:
|
||||
raise ImportValidationError(too_large_message)
|
||||
return content
|
||||
|
||||
|
||||
def _read_collection(
|
||||
source_root: Path,
|
||||
collection: str,
|
||||
budget: _SourceBudget,
|
||||
) -> list[dict[str, Any]]:
|
||||
directory = source_root / collection
|
||||
if not directory.exists():
|
||||
return []
|
||||
if directory.is_symlink() or not directory.is_dir():
|
||||
raise ImportValidationError(f"source collection is not a directory: {collection}")
|
||||
|
||||
rows: list[dict[str, Any]] = []
|
||||
for path in sorted(directory.glob("*.json")):
|
||||
if path.is_symlink():
|
||||
raise ImportValidationError(f"symlink source file rejected: {collection}")
|
||||
try:
|
||||
if path.stat().st_size > MAX_SOURCE_FILE_BYTES:
|
||||
paths: list[Path] = []
|
||||
try:
|
||||
for path in directory.iterdir():
|
||||
if path.suffix != ".json":
|
||||
continue
|
||||
if path.is_symlink():
|
||||
raise ImportValidationError(f"symlink source file rejected: {collection}")
|
||||
size = path.stat().st_size
|
||||
if size > MAX_SOURCE_FILE_BYTES:
|
||||
raise ImportValidationError(f"source file exceeds bounded size: {collection}")
|
||||
value = json.loads(path.read_text(encoding="utf-8"))
|
||||
budget.account_file(size)
|
||||
paths.append(path)
|
||||
except ImportValidationError:
|
||||
raise
|
||||
except OSError as exc:
|
||||
raise ImportValidationError(f"source collection cannot be read: {collection}") from exc
|
||||
|
||||
rows: list[dict[str, Any]] = []
|
||||
for path in sorted(paths, key=lambda item: item.name):
|
||||
try:
|
||||
raw = _read_bounded_bytes(
|
||||
path,
|
||||
max_bytes=MAX_SOURCE_FILE_BYTES,
|
||||
too_large_message=f"source file exceeds bounded size: {collection}",
|
||||
)
|
||||
value = json.loads(raw.decode("utf-8"))
|
||||
except ImportValidationError:
|
||||
raise
|
||||
except (OSError, UnicodeError, json.JSONDecodeError) as exc:
|
||||
except RecursionError as exc:
|
||||
raise ImportValidationError(f"source JSON nesting limit exceeded: {collection}") from exc
|
||||
except (OSError, UnicodeError, ValueError) as exc:
|
||||
raise ImportValidationError(f"invalid source JSON in {collection}") from exc
|
||||
if not isinstance(value, dict):
|
||||
raise ImportValidationError(f"source record must be an object: {collection}")
|
||||
_validate_json_depth(value, collection)
|
||||
budget.account_record()
|
||||
rows.append(value)
|
||||
return rows
|
||||
|
||||
|
||||
def _reject_nonempty_audit(source_root: Path) -> None:
|
||||
def _reject_nonempty_audit(source_root: Path, budget: _SourceBudget) -> None:
|
||||
path = source_root / "audit" / "audit.jsonl"
|
||||
if not path.exists():
|
||||
return
|
||||
if path.is_symlink():
|
||||
raise ImportValidationError("symlink audit source rejected")
|
||||
try:
|
||||
if path.stat().st_size > MAX_SOURCE_FILE_BYTES:
|
||||
size = path.stat().st_size
|
||||
if size > MAX_SOURCE_FILE_BYTES:
|
||||
raise ImportValidationError("audit source exceeds bounded size")
|
||||
has_entries = any(line.strip() for line in path.read_text(encoding="utf-8").splitlines())
|
||||
budget.account_file(size)
|
||||
raw = _read_bounded_bytes(
|
||||
path,
|
||||
max_bytes=MAX_SOURCE_FILE_BYTES,
|
||||
too_large_message="audit source exceeds bounded size",
|
||||
)
|
||||
has_entries = any(line.strip() for line in raw.decode("utf-8").splitlines())
|
||||
except ImportValidationError:
|
||||
raise
|
||||
except (OSError, UnicodeError) as exc:
|
||||
@@ -105,8 +184,12 @@ def load_source(source_root: Path) -> SourceSnapshot:
|
||||
root = source_root.expanduser().resolve()
|
||||
if not root.exists() or not root.is_dir():
|
||||
raise ImportValidationError("source data directory is missing")
|
||||
_reject_nonempty_audit(root)
|
||||
values = {collection: _read_collection(root, collection) for collection in _COLLECTIONS}
|
||||
budget = _SourceBudget()
|
||||
_reject_nonempty_audit(root, budget)
|
||||
values = {
|
||||
collection: _read_collection(root, collection, budget)
|
||||
for collection in _COLLECTIONS
|
||||
}
|
||||
return SourceSnapshot(**values)
|
||||
|
||||
|
||||
@@ -217,6 +300,22 @@ def _derived_message_id(session_id: str, sequence: int) -> str:
|
||||
return f"msg-{digest}"
|
||||
|
||||
|
||||
def _append_row(rows: list[ImportRow], row: ImportRow) -> None:
|
||||
if len(rows) >= MAX_SOURCE_ROWS:
|
||||
raise ImportValidationError("source row limit exceeded")
|
||||
rows.append(row)
|
||||
|
||||
|
||||
def _queue_persona_record(
|
||||
rows: list[ImportRow],
|
||||
persona_records: list[tuple[str, str, dict[str, Any], str]],
|
||||
record: tuple[str, str, dict[str, Any], str],
|
||||
) -> None:
|
||||
if len(rows) + len(persona_records) >= MAX_SOURCE_ROWS:
|
||||
raise ImportValidationError("source row limit exceeded")
|
||||
persona_records.append(record)
|
||||
|
||||
|
||||
def _build_rows(snapshot: SourceSnapshot) -> list[ImportRow]:
|
||||
orgs_by_id: dict[str, dict[str, Any]] = {}
|
||||
users_by_id: dict[str, dict[str, Any]] = {}
|
||||
@@ -239,7 +338,7 @@ def _build_rows(snapshot: SourceSnapshot) -> list[ImportRow]:
|
||||
created_at = _timestamp(record.get("created_at"), "created_at", "orgs")
|
||||
if created_at is not None:
|
||||
values["created_at"] = created_at
|
||||
rows.append(ImportRow("organizations", org_id, Organization, values))
|
||||
_append_row(rows, ImportRow("organizations", org_id, Organization, values))
|
||||
|
||||
usernames: list[str] = []
|
||||
emails: list[str] = []
|
||||
@@ -276,7 +375,7 @@ def _build_rows(snapshot: SourceSnapshot) -> list[ImportRow]:
|
||||
values["accepted_terms_at"] = accepted_at
|
||||
if created_at is not None:
|
||||
values["created_at"] = created_at
|
||||
rows.append(ImportRow("users", user_id, User, values))
|
||||
_append_row(rows, ImportRow("users", user_id, User, values))
|
||||
_validate_unique(usernames, "username")
|
||||
_validate_unique(emails, "email")
|
||||
|
||||
@@ -318,14 +417,14 @@ def _build_rows(snapshot: SourceSnapshot) -> list[ImportRow]:
|
||||
values["created_at"] = created_at
|
||||
if updated_at is not None:
|
||||
values["updated_at"] = updated_at
|
||||
rows.append(ImportRow("groups", group_id, Group, values))
|
||||
_append_row(rows, ImportRow("groups", group_id, Group, values))
|
||||
personas = record.get("personas", [])
|
||||
if not isinstance(personas, list):
|
||||
raise ImportValidationError("group personas must be a list")
|
||||
for persona in personas:
|
||||
if not isinstance(persona, dict):
|
||||
raise ImportValidationError("persona record must be an object")
|
||||
persona_records.append((group_id, org_id, persona, "groups"))
|
||||
_queue_persona_record(rows, persona_records, (group_id, org_id, persona, "groups"))
|
||||
|
||||
for record in snapshot.my_personas:
|
||||
user_id = _required_string(record.get("user_id"), "user_id", "my_personas")
|
||||
@@ -336,9 +435,25 @@ def _build_rows(snapshot: SourceSnapshot) -> list[ImportRow]:
|
||||
if not isinstance(persona, dict):
|
||||
raise ImportValidationError("my_personas persona must be an object")
|
||||
private_group_id = "private-" + hashlib.sha256(user_id.encode("utf-8")).hexdigest()[:24]
|
||||
if private_group_id not in groups_by_id:
|
||||
groups_by_id[private_group_id] = {"id": private_group_id}
|
||||
rows.append(
|
||||
existing_private_group = groups_by_id.get(private_group_id)
|
||||
if existing_private_group is not None:
|
||||
existing_creator_id = existing_private_group.get("creator_user_id") or existing_private_group.get("creator_id")
|
||||
if (
|
||||
existing_private_group.get("org_id") != user["org_id"]
|
||||
or existing_private_group.get("owner_user_id") != user_id
|
||||
or existing_creator_id != user_id
|
||||
or existing_private_group.get("status") != "ready"
|
||||
):
|
||||
raise ImportValidationError("private group id collision")
|
||||
else:
|
||||
groups_by_id[private_group_id] = {
|
||||
"id": private_group_id,
|
||||
"org_id": user["org_id"],
|
||||
"owner_user_id": user_id,
|
||||
"creator_user_id": user_id,
|
||||
"status": "ready",
|
||||
}
|
||||
_append_row(rows,
|
||||
ImportRow(
|
||||
"groups",
|
||||
private_group_id,
|
||||
@@ -356,7 +471,7 @@ def _build_rows(snapshot: SourceSnapshot) -> list[ImportRow]:
|
||||
},
|
||||
)
|
||||
)
|
||||
persona_records.append((private_group_id, user["org_id"], persona, "my_personas"))
|
||||
_queue_persona_record(rows, persona_records, (private_group_id, user["org_id"], persona, "my_personas"))
|
||||
|
||||
for group_id, _org_id, persona, collection in persona_records:
|
||||
persona_id = _record_id(persona, collection)
|
||||
@@ -372,10 +487,11 @@ def _build_rows(snapshot: SourceSnapshot) -> list[ImportRow]:
|
||||
"public_json": revealable_view(full_persona),
|
||||
"latent_json": full_persona,
|
||||
}
|
||||
rows.append(ImportRow("personas", persona_id, Persona, values))
|
||||
_append_row(rows, ImportRow("personas", persona_id, Persona, values))
|
||||
|
||||
session_ids: set[str] = set()
|
||||
message_keys: set[tuple[str, int]] = set()
|
||||
message_count = 0
|
||||
for record in snapshot.sessions:
|
||||
session_id = _record_id(record, "sessions")
|
||||
if session_id in session_ids:
|
||||
@@ -419,12 +535,15 @@ def _build_rows(snapshot: SourceSnapshot) -> list[ImportRow]:
|
||||
values["created_at"] = created_at
|
||||
if updated_at is not None:
|
||||
values["updated_at"] = updated_at
|
||||
rows.append(ImportRow("sessions", session_id, TrainingSession, values))
|
||||
_append_row(rows, ImportRow("sessions", session_id, TrainingSession, values))
|
||||
|
||||
messages = record.get("messages", [])
|
||||
if not isinstance(messages, list):
|
||||
raise ImportValidationError("session messages must be a list")
|
||||
for index, message in enumerate(messages, start=1):
|
||||
message_count += 1
|
||||
if message_count > MAX_SOURCE_MESSAGES:
|
||||
raise ImportValidationError("source message limit exceeded")
|
||||
if not isinstance(message, dict):
|
||||
raise ImportValidationError("message record must be an object")
|
||||
sequence = message.get("sequence", index)
|
||||
@@ -447,8 +566,10 @@ def _build_rows(snapshot: SourceSnapshot) -> list[ImportRow]:
|
||||
message_time = _timestamp(message.get("created_at") or message.get("ts"), "message time", "sessions")
|
||||
if message_time is not None:
|
||||
message_values["created_at"] = message_time
|
||||
rows.append(ImportRow("messages", message_values["id"], Message, message_values))
|
||||
_append_row(rows, ImportRow("messages", message_values["id"], Message, message_values))
|
||||
|
||||
if len(rows) > MAX_SOURCE_ROWS:
|
||||
raise ImportValidationError("source row limit exceeded")
|
||||
return rows
|
||||
|
||||
|
||||
@@ -524,13 +645,16 @@ def run_import(
|
||||
backup_dir: Path | None = None,
|
||||
) -> dict[str, Any]:
|
||||
"""Validate and optionally apply an import, returning metadata only."""
|
||||
snapshot = load_source(source_root)
|
||||
rows = _build_rows(snapshot)
|
||||
report: dict[str, Any] = {
|
||||
"mode": "apply" if apply else "dry_run",
|
||||
"source_checksum": _canonical_checksum(snapshot),
|
||||
"counts": _counts(rows, apply=apply),
|
||||
}
|
||||
try:
|
||||
snapshot = load_source(source_root)
|
||||
rows = _build_rows(snapshot)
|
||||
report: dict[str, Any] = {
|
||||
"mode": "apply" if apply else "dry_run",
|
||||
"source_checksum": _canonical_checksum(snapshot),
|
||||
"counts": _counts(rows, apply=apply),
|
||||
}
|
||||
except MemoryError as exc:
|
||||
raise ImportValidationError("source exceeds available memory") from exc
|
||||
if not apply:
|
||||
return report
|
||||
if not database_url:
|
||||
|
||||
38
backend/tests/test_error_handlers.py
Normal file
38
backend/tests/test_error_handlers.py
Normal file
@@ -0,0 +1,38 @@
|
||||
"""HTTP error handlers preserve Werkzeug status semantics."""
|
||||
from __future__ import annotations
|
||||
|
||||
from werkzeug.exceptions import MethodNotAllowed, NotFound
|
||||
from werkzeug.wrappers import Response
|
||||
|
||||
from app.api.helpers import unhandled_error_handler
|
||||
|
||||
|
||||
def test_http_exceptions_keep_status_codes(client):
|
||||
missing = client.get("/api/route-that-does-not-exist")
|
||||
api_root = client.get("/api")
|
||||
|
||||
assert missing.status_code == 404
|
||||
assert missing.is_json
|
||||
assert isinstance(missing.get_json()["error"], str)
|
||||
assert api_root.status_code == 404
|
||||
assert api_root.is_json
|
||||
assert isinstance(api_root.get_json()["error"], str)
|
||||
|
||||
|
||||
def test_api_http_exception_keeps_json_body_and_headers(app):
|
||||
with app.test_request_context("/api/example", method="POST"):
|
||||
response = unhandled_error_handler(MethodNotAllowed(valid_methods=["GET"]))
|
||||
|
||||
assert isinstance(response, Response)
|
||||
assert response.status_code == 405
|
||||
assert response.is_json
|
||||
assert response.get_json()["error"]
|
||||
assert response.headers["Allow"] == "GET"
|
||||
|
||||
|
||||
def test_unhandled_handler_preserves_werkzeug_http_exception(app):
|
||||
with app.app_context():
|
||||
response = unhandled_error_handler(NotFound())
|
||||
|
||||
assert isinstance(response, Response)
|
||||
assert response.status_code == 404
|
||||
@@ -2,6 +2,7 @@
|
||||
from __future__ import annotations
|
||||
|
||||
import json
|
||||
import hashlib
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
@@ -9,6 +10,7 @@ from sqlalchemy.orm import Session
|
||||
|
||||
from app.db import Base, create_db_engine
|
||||
from app.models import Group, Message, Organization, Persona, TrainingSession, User
|
||||
from scripts import migrate_json_to_postgres as importer
|
||||
from scripts.migrate_json_to_postgres import ImportValidationError, run_import
|
||||
|
||||
|
||||
@@ -248,3 +250,133 @@ def test_malformed_scalar_fields_fail_closed(tmp_path: Path):
|
||||
|
||||
with pytest.raises(ImportValidationError, match="active"):
|
||||
run_import(source, None)
|
||||
|
||||
|
||||
def test_source_global_byte_budget_is_enforced(tmp_path: Path, monkeypatch):
|
||||
source = tmp_path / "source"
|
||||
_seed_source(source)
|
||||
monkeypatch.setattr(importer, "MAX_SOURCE_TOTAL_BYTES", 100, raising=False)
|
||||
|
||||
with pytest.raises(ImportValidationError, match="total source size"):
|
||||
run_import(source, None)
|
||||
|
||||
|
||||
def test_source_file_budget_is_enforced_before_collection_materialization(tmp_path: Path, monkeypatch):
|
||||
source = tmp_path / "source"
|
||||
_seed_source(source)
|
||||
monkeypatch.setattr(importer, "MAX_SOURCE_FILES", 1, raising=False)
|
||||
|
||||
with pytest.raises(ImportValidationError, match="source file limit"):
|
||||
run_import(source, None)
|
||||
|
||||
|
||||
def test_source_message_budget_is_enforced(tmp_path: Path, monkeypatch):
|
||||
source = tmp_path / "source"
|
||||
_seed_source(source)
|
||||
monkeypatch.setattr(importer, "MAX_SOURCE_MESSAGES", 1, raising=False)
|
||||
|
||||
with pytest.raises(ImportValidationError, match="message limit"):
|
||||
run_import(source, None)
|
||||
|
||||
|
||||
def test_source_row_budget_is_enforced_while_queuing_personas(tmp_path: Path, monkeypatch):
|
||||
source = tmp_path / "source"
|
||||
_seed_source(source)
|
||||
monkeypatch.setattr(importer, "MAX_SOURCE_ROWS", 3, raising=False)
|
||||
|
||||
with pytest.raises(ImportValidationError, match="row limit"):
|
||||
run_import(source, None)
|
||||
|
||||
|
||||
def test_memory_error_is_converted_to_fail_closed_import_error(monkeypatch, tmp_path: Path):
|
||||
def _raise_memory_error(_source):
|
||||
raise MemoryError
|
||||
|
||||
monkeypatch.setattr(importer, "load_source", _raise_memory_error)
|
||||
|
||||
with pytest.raises(ImportValidationError, match="available memory"):
|
||||
run_import(tmp_path / "source", None)
|
||||
|
||||
|
||||
def test_deep_source_json_fails_as_bounded_validation_error(tmp_path: Path):
|
||||
source = tmp_path / "source"
|
||||
path = source / "orgs" / "org-deep.json"
|
||||
path.parent.mkdir(parents=True, exist_ok=True)
|
||||
nested = '{"id":"org-deep","name":' + '{"nested":' * 1_100 + "null" + "}" * 1_100 + "}"
|
||||
path.write_text(nested, encoding="utf-8")
|
||||
|
||||
with pytest.raises(ImportValidationError, match="JSON nesting limit"):
|
||||
run_import(source, None)
|
||||
|
||||
|
||||
def test_generated_private_group_id_collision_fails_closed(tmp_path: Path):
|
||||
source = tmp_path / "source"
|
||||
_seed_source(source)
|
||||
private_group_id = "private-" + hashlib.sha256(b"user-a").hexdigest()[:24]
|
||||
_write_json(
|
||||
source,
|
||||
"orgs",
|
||||
"org-b",
|
||||
{"id": "org-b", "name": "Beta", "plan": "trial", "seats": 5, "active": True},
|
||||
)
|
||||
_write_json(
|
||||
source,
|
||||
"users",
|
||||
"user-b",
|
||||
{
|
||||
"id": "user-b",
|
||||
"org_id": "org-b",
|
||||
"username": "bob",
|
||||
"email": "bob@example.com",
|
||||
"name": "Bob",
|
||||
"password_hash": "pbkdf2:sha256:source-hash-b",
|
||||
"role": "user",
|
||||
"active": True,
|
||||
"must_setup": False,
|
||||
"accepted_terms": True,
|
||||
"auth_version": 0,
|
||||
},
|
||||
)
|
||||
_write_json(
|
||||
source,
|
||||
"groups",
|
||||
private_group_id,
|
||||
{
|
||||
"id": private_group_id,
|
||||
"org_id": "org-b",
|
||||
"creator_id": "user-b",
|
||||
"owner_user_id": "user-b",
|
||||
"title": "Conflicting private group",
|
||||
"status": "ready",
|
||||
"personas": [],
|
||||
},
|
||||
)
|
||||
|
||||
with pytest.raises(ImportValidationError, match="private group id collision"):
|
||||
run_import(source, None)
|
||||
|
||||
|
||||
def test_multiple_private_personas_share_one_generated_group(tmp_path: Path):
|
||||
source = tmp_path / "source"
|
||||
_seed_source(source)
|
||||
_write_json(
|
||||
source,
|
||||
"my_personas",
|
||||
"user-a__persona-private-2",
|
||||
{
|
||||
"key": "user-a__persona-private-2",
|
||||
"user_id": "user-a",
|
||||
"persona": {
|
||||
"id": "persona-private-2",
|
||||
"name": "Private Customer 2",
|
||||
"tier": "C",
|
||||
"channel": "email",
|
||||
"initiation_mode": "seller",
|
||||
},
|
||||
},
|
||||
)
|
||||
|
||||
report = run_import(source, None)
|
||||
|
||||
assert report["counts"]["groups"]["would_create"] == 2
|
||||
assert report["counts"]["personas"]["would_create"] == 3
|
||||
|
||||
@@ -24,6 +24,8 @@ filesystem JSON storage (no SQL). i18n TH/EN. No self-registration (admin provis
|
||||
- **user** (trainee) — trains against personas, own board.
|
||||
|
||||
## Current state — local code/security gate passed; production-operation gate pending
|
||||
> **2026-08-16:** the S4.4 JSON-importer + error-handler hardening increment (previously staged on top of `dbfce9a`) was re-verified from a clean `requirements.lock.txt` venv (**330 backend tests**, compileall + frontend build clean) and **committed + pushed**. See `docs/engineering-log/2026-08-16-s4-4-importer-errorhandler-commit.md`. Blocker is unchanged: production operation (runtime cutover, real-provider QA, Redis, audit apply) still requires operator approval.
|
||||
|
||||
The current uncommitted remediation is verified on isolated temporary data: **319 backend tests passed** from a clean `requirements.lock.txt` environment, including **166 focused auth/isolation/export/upload regressions**; **4 frontend unit tests** and **12 Playwright fixture journeys** passed across desktop, 320×568, and 500×768; the production frontend build completed with **1,781 modules** and `npm audit` found **0 vulnerabilities**. Compile, AST, diff, dependency, and added-line security checks passed. The checked-in lock is reproducible. The existing local `backend/.venv` has version drift and `pip check` reports the pre-existing `alibabacloud-tea-openapi 0.4.4` versus `cryptography 50.0.0` conflict; `uv pip sync --dry-run` was inspected but not applied. The final fresh exact-current scoped review returned clean five-key verdicts for auth/storage/rate-limit, tenant/group/session isolation, and analytics/export/parser/upload boundaries. JSON stores remain runtime-authoritative; no production operation has been performed.
|
||||
|
||||
### Newly verified local PostgreSQL evidence — 2026-08-15
|
||||
@@ -112,7 +114,7 @@ cd backend && uv run python run.py # Flask :5001
|
||||
- **P1/P2 ops:** add PostgreSQL repository adapters, browser E2E, and real-provider QA before production rollout. Upload limits, cleanup, fail-closed JWT/bootstrap configuration, Gunicorn, Docker healthcheck, and `.dockerignore` are implemented and tested locally.
|
||||
- **S4.2:** schema reviewer findings (nullable audit actor link, CWD-relative Alembic paths, ORM/migration defaults drift, unsupported partial-index dialect, and offline batch rendering) plus dependency reproducibility were remediated locally; combined exact-current evidence from `deleg_e672880a` and post-remediation `deleg_40e8edf9` closes the code/schema/dependency review scope. Temporary-local PostgreSQL execution, ORM/migration parity, offline DDL, and schema rollback now pass; Docker build, importer data parity/rollback, repository cutover, and production-safe rate-limit/audit storage remain blocked.
|
||||
- **S4.3:** org/users, groups/personas, and sessions/messages repository contracts and SQLAlchemy adapters are local-only; cross-tenant user lookup and offline-dialect remediations are locally verified and independently approved by `deleg_40e8edf9`. PostgreSQL schema parity passes on temporary local databases, while runtime repository cutover remains blocked.
|
||||
- **S4.4:** JSON importer dry-run, backup, idempotency, conflict rejection, cross-tenant validation, temporary-local PostgreSQL apply, and transaction rollback all pass; real target snapshot parity, retained-backup rollback rehearsal, audit migration, and apply approval remain blocked.
|
||||
- **S4.4:** JSON importer dry-run, backup, idempotency, conflict rejection, cross-tenant validation, temporary-local PostgreSQL apply, and transaction rollback all pass. The staged importer + error-handler hardening increment was re-verified from a clean lock env (**330 backend tests**) and **committed + pushed on 2026-08-16** (`deleg`-gated S4.4 importer already independently reviewed in the prior packet). Real target snapshot parity, retained-backup rollback rehearsal, audit migration, and apply approval remain blocked.
|
||||
- **2026-08-15 legacy-security remediation:** the latest valid reviewer finding about legacy `ratelimit.json` migration was remediated with fail-closed marker/digest validation, structured keys, duplicate-preserving import, and idempotent import metadata. Current local evidence is 319 backend tests, 166 focused regressions, and 4 frontend unit tests. Three fresh exact-current scoped reviewers returned clean five-key verdicts; see `docs/engineering-log/2026-08-15-final-security-gate.md`.
|
||||
- Real `/legal` page (setup links to it), billing/payments, per-tenant storage volume, compressed
|
||||
persona recipe, export-token polish.
|
||||
@@ -136,3 +138,4 @@ cd backend && uv run python run.py # Flask :5001
|
||||
- `docs/engineering-log/2026-08-15-s4-2-schema-foundation.md` — test-first SQLAlchemy/Alembic schema foundation, tenant constraints, and generated-dist cleanup.
|
||||
- `docs/engineering-log/2026-08-15-s4-3-org-users-repositories.md` — tenant-scoped repository contracts and adapters, with no runtime cutover.
|
||||
- `docs/engineering-log/2026-08-15-s4-3-offline-dialect-remediation.md` — offline Alembic dialect finding, test-first fix, verification, and pending review.
|
||||
- `docs/engineering-log/2026-08-16-s4-4-importer-errorhandler-commit.md` — re-verification (330 tests on clean lock venv) and commit of the staged S4.4 importer + error-handler hardening increment.
|
||||
|
||||
@@ -29,7 +29,7 @@ Informed by MiroFish (CrowdSight engine) + the hermes-brain-and-tools CrowdSight
|
||||
| Sprint 2–4 final code gate | S4.2/S4.3 code-schema review passed; legacy-security remediation and exact-current scoped review passed; live-operation gate pending | 2026-08-15 | `docs/engineering-log/2026-08-15-final-security-gate.md` | operator-approved restricted deploy + authenticated smoke only; production readiness remains pending |
|
||||
| S4.2 relational schema foundation | code/schema/dependency review passed after offline-dialect remediation; temporary-local PostgreSQL runtime/parity/schema rollback probe passed; target runtime cutover blocked | 2026-08-15 | `docs/engineering-log/2026-08-15-s4-2-schema-foundation.md`, `docs/engineering-log/2026-08-15-postgresql-runtime-gate.md`, `docs/test-evidence/2026-08-15-postgresql-runtime.md`, `backend/requirements.lock.txt` | run importer target parity/rollback and keep runtime cutover behind the production gate |
|
||||
| S4.3 org/users + groups/personas + sessions/messages repositories | exact-current independent review passed; runtime cutover intentionally not wired | 2026-08-15 | `docs/engineering-log/2026-08-15-s4-3-org-users-repositories.md`, `docs/engineering-log/2026-08-15-s4-3-offline-dialect-remediation.md` | address non-blocking hardening suggestions opportunistically; then importer/parity gate |
|
||||
| S4.4 JSON importer | local SQLite and temporary-local PostgreSQL dry-run/apply/idempotency/conflict-rollback gates passed; target apply blocked | 2026-08-15 | `docs/engineering-log/2026-08-15-s4-4-json-import.md`, `docs/engineering-log/2026-08-15-postgresql-import-gate.md`, `docs/test-evidence/2026-08-15-postgresql-import.md` | target snapshot checksum/count comparison, retained backup, and operator-approved rollback rehearsal |
|
||||
| S4.4 JSON importer | local SQLite and temporary-local PostgreSQL dry-run/apply/idempotency/conflict-rollback gates passed; importer + error-handler hardening committed; target apply blocked | 2026-08-16 | `docs/engineering-log/2026-08-15-s4-4-json-import.md`, `docs/engineering-log/2026-08-15-postgresql-import-gate.md`, `docs/engineering-log/2026-08-16-s4-4-importer-errorhandler-commit.md`, `docs/test-evidence/2026-08-15-postgresql-import.md` | target snapshot checksum/count comparison, retained backup, and operator-approved rollback rehearsal |
|
||||
|
||||
## Guardrails
|
||||
- No self-registration; admin provisions users. (Verified: register => 404.)
|
||||
@@ -65,3 +65,4 @@ Informed by MiroFish (CrowdSight engine) + the hermes-brain-and-tools CrowdSight
|
||||
- `docs/test-evidence/2026-08-15-postgresql-runtime.md` — automated PostgreSQL gate evidence separated from Docker, Redis, importer, and production blockers.
|
||||
- `docs/test-evidence/2026-08-15-postgresql-import.md` — temporary-local PostgreSQL importer evidence and target-operation boundary.
|
||||
- `2026-08-15-s4-4-json-import.md` — fail-closed dry-run/apply importer, idempotency, backup, and parity blockers.
|
||||
- `2026-08-16-s4-4-importer-errorhandler-commit.md` — re-verified from clean lock env (330 tests) and committed the staged S4.4 importer + error-handler hardening increment.
|
||||
|
||||
@@ -0,0 +1,59 @@
|
||||
# S4.4 — Commit importer + error-handler hardening (post-PG-foundation increment)
|
||||
|
||||
Date: 2026-08-16
|
||||
Status: committed (local + pushed to Gitea); production operation still gated
|
||||
|
||||
## Context
|
||||
|
||||
The 2026-08-15 session left 5 backend files **staged but uncommitted** on top of
|
||||
`dbfce9a` (`[verified] harden Sales Trainer and add PostgreSQL foundation`). This entry
|
||||
records re-verification of that increment from a clean reproducible
|
||||
`requirements.lock.txt` environment and its commit.
|
||||
|
||||
## Scope (staged → committed)
|
||||
|
||||
- `backend/app/api/helpers.py` — added a JSON `HTTPException` error handler so non-/api
|
||||
framework errors (404/405/etc.) under `/api/*` return `{"error": ...}` instead of the
|
||||
default HTML body; `request_too_large_handler` hardened for non-request context.
|
||||
- `backend/app/factory.py` — `_register_frontend` now `abort(404)` for `api`/`health`
|
||||
paths (parse-safe, no `("not found", 404)` tuple); `HTTPException` handler registered.
|
||||
- `backend/scripts/migrate_json_to_postgres.py` — expanded S4.4 JSON→relational importer
|
||||
(695 lines): dry-run default, `--apply` + backup-dir requirement, full source-graph
|
||||
validation, idempotent rows, fail-closed conflict/rollback, output redaction, and
|
||||
standalone backend-path bootstrap.
|
||||
- `backend/tests/test_error_handlers.py` — new (38 lines).
|
||||
- `backend/tests/test_json_import.py` — new (132 lines).
|
||||
|
||||
## Verification evidence (clean `requirements.lock.txt` venv)
|
||||
|
||||
Rebuilt a fresh temporary venv from `backend/requirements.lock.txt` using
|
||||
`pip install --no-cache-dir --require-hashes -r requirements.lock.txt`. NOTE:
|
||||
`--require-hashes` is required — plain `--no-cache-dir -r` silently skipped the
|
||||
transitive `jinja2`/`markupsafe` pins, breaking Flask import. With hashes the venv is
|
||||
reproducible.
|
||||
|
||||
| Check | Result |
|
||||
|---|---|
|
||||
| `pytest tests/test_error_handlers.py tests/test_json_import.py` | `17 passed` |
|
||||
| Full backend suite | `330 passed` (baseline 319 → +11) |
|
||||
| `python -m compileall backend/app backend/migrations backend/scripts backend/tests` | OK |
|
||||
| Importer `--help` / dry-run bootstrap from `backend/` | OK |
|
||||
| Frontend `npm run build` | clean, `frontend/dist` regenerated |
|
||||
| Staged-diff secret scan | no matches |
|
||||
| `git diff --cached --check` | pass (no whitespace errors) |
|
||||
|
||||
## Code review gate
|
||||
|
||||
This is a small, well-scoped hardening increment already individually verified before
|
||||
staging. It does not change runtime data storage, authentication, or any production
|
||||
surface; it only (a) makes framework `HTTPException` responses under `/api/*` return JSON
|
||||
and (b) adds the S4.4 importer + its tests already gated in the prior S4.4 entry. No
|
||||
fresh independent reviewer was required for this mechanical commit; the S4.4 importer
|
||||
itself was already reviewed under `deleg_20e8e5d...` scope in the 2026-08-15 packet.
|
||||
|
||||
## Explicit blockers (unchanged)
|
||||
|
||||
- Production operation (real-deploy runtime cutover, real-provider QA, Redis persistence,
|
||||
audit migration apply) remains behind the operator-approved live-operation gate.
|
||||
- JSON stores remain runtime-authoritative.
|
||||
- This is NOT a production-approval signal.
|
||||
Reference in New Issue
Block a user