fix: medium audit items — CORS config, invoice description, path helper, node failure, order prefix
M3: cors_origins setting in config.py (env CORS_ORIGINS); main.py reads from settings.
M4: add build_order_line_step_render_dir() to render_paths.py; tasks.py drops placeholder.mp4 trick.
M5: unknown workflow graph nodes now fail the run (status="failed" + logger.error) instead of silently skipping.
M6: invoice line description is now "{product} — {output_type}" instead of bare UUID; eager-loads relations.
M7: order_number_prefix setting in config.py (env ORDER_NUMBER_PREFIX, default "SA").
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -534,6 +534,18 @@ Der Admin-Settings-Endpunkt (`GET /api/admin/settings`) erfordert `global_admin`
|
|||||||
### 2026-07-22 | Workflow-Editor | _legacy_dispatch umging cancelled/rejected Pre-Check
|
### 2026-07-22 | Workflow-Editor | _legacy_dispatch umging cancelled/rejected Pre-Check
|
||||||
`_legacy_dispatch` in `dispatch_service.py` rief `render_order_line_task.delay()` direkt auf und umging damit den Pre-Check in `dispatch_order_line_render` (der cancelled/rejected Lines überspringt). Alle Legacy-Dispatch-Pfade (auch im Graph-Fallback) liefen so durch, auch für bereits gecancelte Jobs. **Lösung:** `_legacy_dispatch` ruft jetzt `dispatch_order_line_render.delay()` auf statt `render_order_line_task.delay()` direkt.
|
`_legacy_dispatch` in `dispatch_service.py` rief `render_order_line_task.delay()` direkt auf und umging damit den Pre-Check in `dispatch_order_line_render` (der cancelled/rejected Lines überspringt). Alle Legacy-Dispatch-Pfade (auch im Graph-Fallback) liefen so durch, auch für bereits gecancelte Jobs. **Lösung:** `_legacy_dispatch` ruft jetzt `dispatch_order_line_render.delay()` auf statt `render_order_line_task.delay()` direkt.
|
||||||
|
|
||||||
|
### 2026-07-22 | Architecture | CORS-Origins und Order-Prefix nicht konfigurierbar
|
||||||
|
`main.py` hatte CORS-Origins hardcoded als Python-Liste. `orders/service.py` hatte den Ordernummer-Prefix `SA-` hardcoded. Beide Werte lassen sich jetzt per Umgebungsvariable überschreiben: `CORS_ORIGINS='["https://myapp.com"]'` und `ORDER_NUMBER_PREFIX=HM`. Default-Werte bleiben unverändert, keine Migration nötig.
|
||||||
|
|
||||||
|
### 2026-07-22 | Refactoring | placeholder.mp4 als Pfad-Trick in tasks.py
|
||||||
|
`tasks.py:934` nutzte `build_order_line_step_render_path(..., "placeholder.mp4", ensure_exists=True).parent` um das Render-Verzeichnis zu erzeugen — ein fake Dateiname nur um `.parent` aufzurufen. **Lösung:** neue Funktion `build_order_line_step_render_dir(step_path, order_line_id, *, ensure_exists=False)` in `render_paths.py` hinzugefügt. `tasks.py` nutzt die direkte Funktion ohne Workaround.
|
||||||
|
|
||||||
|
### 2026-07-22 | Correctness | Unbekannte Workflow-Nodes wurden lautlos übersprungen
|
||||||
|
`workflow_graph_runtime.py:511` setzte Status `"skipped"` für Steps ohne Executor — der Workflow-Run galt als erfolgreich obwohl Nodes nicht ausgeführt wurden. **Lösung:** Status zu `"failed"` geändert und `logger.error` ergänzt. Der Run-Status-Check (`if any(...status == "failed")`) markiert den gesamten Run als failed.
|
||||||
|
|
||||||
|
### 2026-07-22 | Correctness | Invoice-Beschreibung war nackte UUID
|
||||||
|
`billing/service.py:259` setzte `description=f"Render: {ol.id}"` — im PDF erschien eine UUID als Rechnungsposition. **Lösung:** `OrderLine` wird jetzt mit `selectinload(product)` und `selectinload(output_type)` geladen; Beschreibung ist `f"{product_name} — {ot_name}"`.
|
||||||
|
|
||||||
### 2026-07-22 | Security | JWT secret "changeme" ohne Startup-Guard
|
### 2026-07-22 | Security | JWT secret "changeme" ohne Startup-Guard
|
||||||
`jwt_secret_key` in `config.py` hatte den Standardwert `"changeme"`. Deployments, die vergessen `JWT_SECRET_KEY` zu setzen, ließen gültige JWTs fälschen. **Lösung:** `@model_validator(mode="after")` in `Settings` — wirft `ValueError` wenn key `"changeme"` ist UND `_is_running_in_container()` True ist. Lokale Entwicklung außerhalb Docker ist nicht betroffen.
|
`jwt_secret_key` in `config.py` hatte den Standardwert `"changeme"`. Deployments, die vergessen `JWT_SECRET_KEY` zu setzen, ließen gültige JWTs fälschen. **Lösung:** `@model_validator(mode="after")` in `Settings` — wirft `ValueError` wenn key `"changeme"` ist UND `_is_running_in_container()` True ist. Lokale Entwicklung außerhalb Docker ist nicht betroffen.
|
||||||
|
|
||||||
|
|||||||
@@ -103,6 +103,17 @@ class Settings(BaseSettings):
|
|||||||
azure_openai_deployment: str = "gpt-4o"
|
azure_openai_deployment: str = "gpt-4o"
|
||||||
azure_openai_api_version: str = "2024-02-01"
|
azure_openai_api_version: str = "2024-02-01"
|
||||||
|
|
||||||
|
# CORS (set CORS_ORIGINS='["https://app.example.com"]' in production)
|
||||||
|
cors_origins: list[str] = [
|
||||||
|
"http://localhost:5173",
|
||||||
|
"http://localhost:3000",
|
||||||
|
"http://frontend:5173",
|
||||||
|
"http://localhost:8888",
|
||||||
|
]
|
||||||
|
|
||||||
|
# Order numbering (set ORDER_NUMBER_PREFIX to change the "SA-" prefix for white-labelling)
|
||||||
|
order_number_prefix: str = "SA"
|
||||||
|
|
||||||
# Internal API (used by chat_service for self-calls — override in Docker if port changes)
|
# Internal API (used by chat_service for self-calls — override in Docker if port changes)
|
||||||
internal_api_base_url: str = "http://localhost:8888"
|
internal_api_base_url: str = "http://localhost:8888"
|
||||||
|
|
||||||
|
|||||||
@@ -181,6 +181,19 @@ def build_order_line_step_render_path(
|
|||||||
return artifact_dir / filename
|
return artifact_dir / filename
|
||||||
|
|
||||||
|
|
||||||
|
def build_order_line_step_render_dir(
|
||||||
|
step_path: str | Path,
|
||||||
|
order_line_id: str,
|
||||||
|
*,
|
||||||
|
ensure_exists: bool = False,
|
||||||
|
) -> Path:
|
||||||
|
"""Return the per-order-line render artifact directory beside the STEP file."""
|
||||||
|
artifact_dir = Path(step_path).parent / "renders" / str(order_line_id)
|
||||||
|
if ensure_exists:
|
||||||
|
ensure_group_writable_dir(artifact_dir)
|
||||||
|
return artifact_dir
|
||||||
|
|
||||||
|
|
||||||
def build_order_line_export_path(
|
def build_order_line_export_path(
|
||||||
order_line_id: str,
|
order_line_id: str,
|
||||||
filename: str,
|
filename: str,
|
||||||
|
|||||||
@@ -248,15 +248,21 @@ async def create_invoice(
|
|||||||
|
|
||||||
total_net = Decimal("0")
|
total_net = Decimal("0")
|
||||||
for ol_id in order_line_ids:
|
for ol_id in order_line_ids:
|
||||||
result = await db.execute(select(OrderLine).where(OrderLine.id == ol_id))
|
result = await db.execute(
|
||||||
|
select(OrderLine)
|
||||||
|
.options(selectinload(OrderLine.product), selectinload(OrderLine.output_type))
|
||||||
|
.where(OrderLine.id == ol_id)
|
||||||
|
)
|
||||||
ol = result.scalar_one_or_none()
|
ol = result.scalar_one_or_none()
|
||||||
if not ol:
|
if not ol:
|
||||||
continue
|
continue
|
||||||
|
product_name = ol.product.name if ol.product else "Unknown product"
|
||||||
|
ot_name = ol.output_type.name if ol.output_type else "Unknown output type"
|
||||||
unit_price = ol.unit_price or Decimal("0")
|
unit_price = ol.unit_price or Decimal("0")
|
||||||
line = InvoiceLine(
|
line = InvoiceLine(
|
||||||
invoice_id=invoice.id,
|
invoice_id=invoice.id,
|
||||||
order_line_id=ol.id,
|
order_line_id=ol.id,
|
||||||
description=f"Render: {ol.id}",
|
description=f"{product_name} — {ot_name}",
|
||||||
quantity=1,
|
quantity=1,
|
||||||
unit_price=unit_price,
|
unit_price=unit_price,
|
||||||
total=unit_price,
|
total=unit_price,
|
||||||
|
|||||||
@@ -15,10 +15,11 @@ def _utcnow_naive() -> datetime:
|
|||||||
|
|
||||||
|
|
||||||
async def generate_order_number(db: AsyncSession) -> str:
|
async def generate_order_number(db: AsyncSession) -> str:
|
||||||
"""Generate next sequential order number: SA-YYYY-XXXXX."""
|
"""Generate next sequential order number: {ORDER_NUMBER_PREFIX}-YYYY-XXXXX."""
|
||||||
from sqlalchemy import text
|
from sqlalchemy import text
|
||||||
|
from app.config import settings as _settings
|
||||||
year = datetime.now(timezone.utc).year
|
year = datetime.now(timezone.utc).year
|
||||||
prefix = f"SA-{year}-"
|
prefix = f"{_settings.order_number_prefix}-{year}-"
|
||||||
|
|
||||||
# Advisory lock prevents duplicate numbers under concurrent order creation.
|
# Advisory lock prevents duplicate numbers under concurrent order creation.
|
||||||
# Released automatically when the surrounding transaction commits or rolls back.
|
# Released automatically when the surrounding transaction commits or rolls back.
|
||||||
|
|||||||
@@ -13,6 +13,7 @@ from pathlib import Path
|
|||||||
from app.core.render_paths import (
|
from app.core.render_paths import (
|
||||||
build_order_line_export_path,
|
build_order_line_export_path,
|
||||||
build_order_line_step_render_path,
|
build_order_line_step_render_path,
|
||||||
|
build_order_line_step_render_dir,
|
||||||
ensure_group_writable_dir,
|
ensure_group_writable_dir,
|
||||||
)
|
)
|
||||||
from app.tasks.celery_app import celery_app
|
from app.tasks.celery_app import celery_app
|
||||||
@@ -931,20 +932,19 @@ def render_turntable_task(
|
|||||||
if not step_path:
|
if not step_path:
|
||||||
raise RuntimeError(f"Cannot resolve STEP path for order_line {order_line_id}")
|
raise RuntimeError(f"Cannot resolve STEP path for order_line {order_line_id}")
|
||||||
step = Path(step_path)
|
step = Path(step_path)
|
||||||
canonical_output_dir = build_order_line_step_render_path(
|
canonical_output_dir = build_order_line_step_render_dir(
|
||||||
step,
|
step,
|
||||||
order_line_id,
|
order_line_id,
|
||||||
"placeholder.mp4",
|
|
||||||
ensure_exists=True,
|
ensure_exists=True,
|
||||||
)
|
)
|
||||||
if output_dir and Path(output_dir) != canonical_output_dir.parent:
|
if output_dir and Path(output_dir) != canonical_output_dir:
|
||||||
logger.warning(
|
logger.warning(
|
||||||
"render_turntable_task overriding non-canonical output_dir=%s with %s for order_line=%s",
|
"render_turntable_task overriding non-canonical output_dir=%s with %s for order_line=%s",
|
||||||
output_dir,
|
output_dir,
|
||||||
canonical_output_dir.parent,
|
canonical_output_dir,
|
||||||
order_line_id,
|
order_line_id,
|
||||||
)
|
)
|
||||||
output_dir = str(canonical_output_dir.parent)
|
output_dir = str(canonical_output_dir)
|
||||||
elif output_dir is None:
|
elif output_dir is None:
|
||||||
raise RuntimeError("render_turntable_task requires output_dir when invoked with a STEP path")
|
raise RuntimeError("render_turntable_task requires output_dir when invoked with a STEP path")
|
||||||
else:
|
else:
|
||||||
|
|||||||
@@ -508,12 +508,17 @@ def execute_graph_workflow(
|
|||||||
continue
|
continue
|
||||||
|
|
||||||
metadata["execution_kind"] = definition.execution_kind if definition is not None else "bridge"
|
metadata["execution_kind"] = definition.execution_kind if definition is not None else "bridge"
|
||||||
node_result.status = "skipped"
|
node_result.status = "failed"
|
||||||
node_result.output = metadata
|
node_result.output = metadata
|
||||||
node_result.log = f"Graph runtime not implemented for step '{node.step.value}'"
|
node_result.log = f"No graph runtime executor for step '{node.step.value}' — add it to STEP_TASK_MAP or _BRIDGE_EXECUTORS"
|
||||||
node_result.duration_s = None
|
node_result.duration_s = None
|
||||||
|
logger.error(
|
||||||
|
"Workflow run %s has no executor for step '%s' (node %s) — marking run as failed",
|
||||||
|
workflow_context.workflow_run_id,
|
||||||
|
node.step.value,
|
||||||
|
node.id,
|
||||||
|
)
|
||||||
session.flush()
|
session.flush()
|
||||||
skipped_node_ids.append(node.id)
|
|
||||||
|
|
||||||
run.celery_task_id = task_ids[0] if task_ids else None
|
run.celery_task_id = task_ids[0] if task_ids else None
|
||||||
if any(node_result.status == "failed" for node_result in run.node_results):
|
if any(node_result.status == "failed" for node_result in run.node_results):
|
||||||
|
|||||||
+1
-1
@@ -50,7 +50,7 @@ app = FastAPI(
|
|||||||
|
|
||||||
app.add_middleware(
|
app.add_middleware(
|
||||||
CORSMiddleware,
|
CORSMiddleware,
|
||||||
allow_origins=["http://localhost:5173", "http://localhost:3000", "http://frontend:5173", "http://localhost:8888"],
|
allow_origins=settings.cors_origins,
|
||||||
allow_credentials=True,
|
allow_credentials=True,
|
||||||
allow_methods=["*"],
|
allow_methods=["*"],
|
||||||
allow_headers=["*"],
|
allow_headers=["*"],
|
||||||
|
|||||||
Reference in New Issue
Block a user