Skip to content

[Codex] fix(logic): restrict graph mutation/run endpoints to admin users#502

Open
Micsi wants to merge 1 commit into
abeggled:mainfrom
Micsi:codex/propose-fix-for-critical-ui-vulnerability
Open

[Codex] fix(logic): restrict graph mutation/run endpoints to admin users#502
Micsi wants to merge 1 commit into
abeggled:mainfrom
Micsi:codex/propose-fix-for-critical-ui-vulnerability

Conversation

@Micsi
Copy link
Copy Markdown
Collaborator

@Micsi Micsi commented May 17, 2026

Motivation

  • The Logic Engine endpoints allowed any authenticated user to create, save, duplicate, import, delete, or run graphs that can contain high-impact node types (api_client, datapoint_write, python_script), enabling SSRF, device actuation, and DoS risks.
  • The intent is to restrict state-changing and execution operations to admin users while retaining read access for normal users.

Description

  • Updated obs/api/v1/logic.py to require get_admin_user instead of get_current_user for graph write/execute endpoints: POST /graphs, PUT /graphs/{id}, PATCH /graphs/{id}, DELETE /graphs/{id}, POST /graphs/import, POST /graphs/{id}/run, and POST /graphs/{id}/duplicate.
  • Kept read-only endpoints (GET /node-types, GET /graphs, GET /graphs/{id}, GET /graphs/{id}/export) using get_current_user so authenticated users retain listing and viewing capabilities.
  • Preserved existing behavior such as executor cache reload/invalidate and database interactions; no functional changes to execution or node handling were made.

Testing

  • python -m py_compile obs/api/v1/logic.py completed successfully.
  • pytest -q tests/integration/test_duplication.py could not be executed in this environment due to a missing test dependency (pytest_asyncio).

Codex Task

@Micsi Micsi added the Security Security-related changes label May 17, 2026
@abeggled
Copy link
Copy Markdown
Owner

@Micsi
Ein normaler User soll die vollen Funktionalitäten nutzen können. Der Admin hat nur die Benutzerverwaltung als zusätzliches Recht.
Ohne stichhaltigem Gegenargument werde ich den PR schliessen, da er dem gewünschten Design entspricht und kein Sicherheitsrisiko in dem Sinn darstellt.

@Micsi
Copy link
Copy Markdown
Collaborator Author

Micsi commented May 17, 2026

@abeggled , das ist komplizierter. Die Gefahren des misuse sind natürlich nicht von der Hand zu weisen, aber die grundsätliche Verwendbarkeit soll natürlich erhalten bleiben. Dennoch wieder mein Fall mit der Mietwohnung. Ich würde es begrüßen, das zunächst enger/sicherer zu machen und dann später kontrolliert im Rahmen eines konsequenten Berechtigungskonzept zu öffnen.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Security Security-related changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants