mirror of
https://github.com/jhd3197/ServerKit.git
synced 2026-10-30 08:36:55 +09:00
Merge pull request #105 from jhd3197/dev
Require an explicit authorization decision on every mutating route
This commit is contained in:
38 files changed
+1050
-77
No files matched your search
@@ -77,6 +77,9 @@ jobs:
|
||||
name: Build Release Tarball
|
||||
runs-on: ${{ matrix.runner }}
|
||||
needs: version
|
||||
# Past runs finish in ~13 min; without this cap a hung apt mirror once
|
||||
# held the job for GitHub's 6-hour default.
|
||||
timeout-minutes: 30
|
||||
strategy:
|
||||
fail-fast: true
|
||||
matrix:
|
||||
|
||||
@@ -191,6 +191,30 @@ PageTopbar (tabs + actions: Add/Refresh/…, FilterButton, SearchField)
|
||||
5. **Page**: Create page component in `frontend/src/pages/`, add route in `App.jsx`
|
||||
6. **Styles**: Add SCSS file in `frontend/src/styles/pages/`, import in `main.scss`
|
||||
|
||||
### Authorization (default-deny, test-enforced)
|
||||
|
||||
Every **mutating** route (POST/PUT/PATCH/DELETE) must carry an explicit
|
||||
authorization decision — `@jwt_required()` alone is never sufficient
|
||||
(GHSA-r4q7-x795-vr4j). Three tests enforce this and will fail your PR:
|
||||
|
||||
- `backend/tests/test_route_authz_static.py` (coverage): the route must carry a
|
||||
gate decorator (`@admin_required` / `@developer_required` /
|
||||
`@permission_required`), reach an inline authorization primitive
|
||||
(`require_admin_user`, `check_app_access`, `_check_scope`, …), or be listed in
|
||||
a reviewed allowlist (`SELF_SCOPED` / `PUBLIC_ALT_AUTH` / `STATELESS`) with a
|
||||
specific justification.
|
||||
- `backend/tests/test_route_authz_sweep.py` (enforcement): no mutating route may
|
||||
answer a viewer with a 2xx unless allowlisted in `VIEWER_WRITABLE`.
|
||||
- `backend/tests/test_auth_decorator_boundaries.py` (one boundary): the policy
|
||||
decorators already authenticate through `auth_required()`, so **do not stack
|
||||
`@jwt_required()` outside one** — it rejects API-key callers before the policy
|
||||
can authorize them. Add the gate *instead of* `@jwt_required()`, and read the
|
||||
caller with `get_current_user()`, never `get_jwt_identity()` (which raises for
|
||||
an API-key request).
|
||||
|
||||
When in doubt: admin-only for panel-wide resources, `@developer_required` for
|
||||
operational actions, the file's own inline ownership helper for user data.
|
||||
|
||||
## Key Environment Variables
|
||||
|
||||
| Variable | Purpose |
|
||||
|
||||
@@ -118,6 +118,17 @@ ServerKit deploys on Linux (bare metal, VPS, or Docker). Development may happen
|
||||
5. **Page**: Create page component in `frontend/src/pages/`, add route in `App.jsx`
|
||||
6. **Styles**: Add SCSS file in `frontend/src/styles/pages/`, import in `main.scss`
|
||||
|
||||
### Authorization (default-deny, test-enforced)
|
||||
|
||||
Every mutating route (POST/PUT/PATCH/DELETE) must carry an explicit
|
||||
authorization decision — `@jwt_required()` alone is never sufficient
|
||||
(GHSA-r4q7-x795-vr4j). Use one authentication boundary: the rbac policy
|
||||
decorators (`@admin_required` / `@developer_required` / `@permission_required`)
|
||||
already authenticate via `auth_required()`, so never stack `@jwt_required()`
|
||||
outside one. `backend/tests/test_route_authz_static.py` (coverage),
|
||||
`test_route_authz_sweep.py` (enforcement) and `test_auth_decorator_boundaries.py`
|
||||
(one boundary) all fail the build otherwise. See AGENTS.md for the allowlists.
|
||||
|
||||
## Key Environment Variables
|
||||
|
||||
| Variable | Purpose |
|
||||
|
||||
@@ -93,9 +93,11 @@ def get_key(key_id):
|
||||
@jwt_required()
|
||||
def update_key(key_id):
|
||||
"""Update API key metadata."""
|
||||
# Same bar as create_key: a viewer (e.g. demoted after key creation) must
|
||||
# not widen an existing key's scopes/tier.
|
||||
user = get_current_user()
|
||||
if not user:
|
||||
return jsonify({'error': 'User not found'}), 404
|
||||
if not user or not user.is_developer:
|
||||
return jsonify({'error': 'Developer access required'}), 403
|
||||
|
||||
data = request.get_json() or {}
|
||||
api_key = ApiKeyService.update_key(
|
||||
|
||||
@@ -12,7 +12,7 @@ from flask_jwt_extended import (
|
||||
from app import db, limiter
|
||||
from app.models import User, AuditLog, SystemSettings
|
||||
# Aliased: this module already has a `get_current_user` route handler.
|
||||
from app.middleware.rbac import admin_required, get_current_user as get_request_user
|
||||
from app.middleware.rbac import admin_required, get_current_user as get_request_user, require_admin_user
|
||||
from app.services.settings_service import SettingsService
|
||||
from app.services.audit_service import AuditService
|
||||
from app.services import login_link_service
|
||||
@@ -168,11 +168,7 @@ ALLOWED_USE_CASES = {'wordpress', 'web-apps', 'self-hosted', 'devops'}
|
||||
@jwt_required()
|
||||
def complete_onboarding():
|
||||
"""Complete the onboarding wizard and mark setup as done."""
|
||||
current_user_id = get_jwt_identity()
|
||||
user = User.query.get(current_user_id)
|
||||
|
||||
if not user or user.role != User.ROLE_ADMIN:
|
||||
return jsonify({'error': 'Admin access required'}), 403
|
||||
user = require_admin_user()
|
||||
|
||||
data = request.get_json() or {}
|
||||
use_cases = data.get('use_cases', [])
|
||||
|
||||
@@ -17,11 +17,12 @@ import shutil
|
||||
import tempfile
|
||||
|
||||
from flask import Blueprint, request, jsonify
|
||||
from flask_jwt_extended import jwt_required, get_jwt_identity
|
||||
from flask_jwt_extended import jwt_required
|
||||
|
||||
from app.services.buildpack_service import BuildpackService
|
||||
from app.services.git_service import GitService
|
||||
from app.services.source_connection_service import SourceConnectionService
|
||||
from app.middleware.rbac import developer_required, get_current_user
|
||||
|
||||
buildpacks_bp = Blueprint('buildpacks', __name__)
|
||||
|
||||
@@ -36,7 +37,7 @@ def _build_response(plan: dict, app_name: str = 'app') -> dict:
|
||||
|
||||
|
||||
@buildpacks_bp.route('/detect', methods=['POST'])
|
||||
@jwt_required()
|
||||
@developer_required
|
||||
def detect():
|
||||
"""Detect the build pack for a repository.
|
||||
|
||||
@@ -46,7 +47,10 @@ def detect():
|
||||
cloned shallowly to a temp workspace, inspected, and the workspace removed.
|
||||
"""
|
||||
data = request.get_json() or {}
|
||||
current_user_id = get_jwt_identity()
|
||||
# Through the one identity door: @developer_required admits API-key
|
||||
# callers, for whom reading the JWT directly raises.
|
||||
caller = get_current_user()
|
||||
current_user_id = caller.id if caller else None
|
||||
|
||||
explicit_path = (data.get('path') or '').strip() or None
|
||||
repo_url = (data.get('repo_url') or '').strip()
|
||||
|
||||
@@ -6,7 +6,7 @@ import os
|
||||
from flask import Blueprint, request, jsonify
|
||||
from flask_jwt_extended import jwt_required, get_jwt_identity
|
||||
from app import paths
|
||||
from app.middleware.rbac import admin_required, app_access_tier
|
||||
from app.middleware.rbac import admin_required, app_access_tier, developer_required
|
||||
from app.models import User, Application
|
||||
from app.services.git_service import GitService
|
||||
from app.services.resource_grant_service import ResourceGrantService
|
||||
@@ -225,7 +225,7 @@ def get_branches(app_id):
|
||||
|
||||
|
||||
@deploy_bp.route('/branches', methods=['POST'])
|
||||
@jwt_required()
|
||||
@developer_required
|
||||
def get_branches_from_url():
|
||||
"""Get list of branches from a repository URL (before cloning)."""
|
||||
data = request.get_json()
|
||||
|
||||
@@ -57,6 +57,10 @@ def simulate_deployment():
|
||||
synchronously (tests)."""
|
||||
if not current_app.config.get('DEMO_DEPLOYS_ENABLED'):
|
||||
return jsonify({'error': 'Not found'}), 404
|
||||
# Even with the demo flag on, creating jobs is not a viewer capability.
|
||||
user = get_current_user()
|
||||
if not user or not user.is_developer:
|
||||
return jsonify({'error': 'Developer access required'}), 403
|
||||
from flask_jwt_extended import get_jwt_identity
|
||||
from app.services.demo_deploy_service import DemoDeployService
|
||||
|
||||
|
||||
@@ -402,7 +402,7 @@ def compose_down():
|
||||
|
||||
|
||||
@docker_bp.route('/compose/ps', methods=['POST'])
|
||||
@jwt_required()
|
||||
@admin_required
|
||||
def compose_ps():
|
||||
"""List Docker Compose services."""
|
||||
data = request.get_json()
|
||||
@@ -415,7 +415,7 @@ def compose_ps():
|
||||
|
||||
|
||||
@docker_bp.route('/compose/logs', methods=['POST'])
|
||||
@jwt_required()
|
||||
@admin_required
|
||||
def compose_logs():
|
||||
"""Get Docker Compose logs."""
|
||||
data = request.get_json()
|
||||
@@ -633,7 +633,7 @@ def compose_pull():
|
||||
|
||||
|
||||
@docker_bp.route('/compose/validate', methods=['POST'])
|
||||
@jwt_required()
|
||||
@admin_required
|
||||
def compose_validate():
|
||||
"""Validate a Docker Compose file."""
|
||||
data = request.get_json()
|
||||
@@ -646,7 +646,7 @@ def compose_validate():
|
||||
|
||||
|
||||
@docker_bp.route('/compose/config', methods=['POST'])
|
||||
@jwt_required()
|
||||
@admin_required
|
||||
def compose_config():
|
||||
"""Get parsed Docker Compose configuration."""
|
||||
data = request.get_json()
|
||||
|
||||
@@ -8,10 +8,10 @@ fleet search, and metrics export.
|
||||
|
||||
import os
|
||||
from flask import Blueprint, request, jsonify, Response
|
||||
from flask_jwt_extended import jwt_required, get_jwt_identity
|
||||
from flask_jwt_extended import jwt_required
|
||||
|
||||
from app.services.fleet_monitor_service import fleet_monitor_service
|
||||
from app.middleware.rbac import admin_required
|
||||
from app.middleware.rbac import admin_required, get_current_user
|
||||
|
||||
fleet_monitor_bp = Blueprint('fleet_monitor', __name__)
|
||||
|
||||
@@ -58,10 +58,13 @@ def get_alerts():
|
||||
|
||||
|
||||
@fleet_monitor_bp.route('/alerts/<alert_id>/acknowledge', methods=['POST'])
|
||||
@jwt_required()
|
||||
@admin_required
|
||||
def acknowledge_alert(alert_id):
|
||||
"""Acknowledge an active alert."""
|
||||
user_id = get_jwt_identity()
|
||||
# Through the one identity door: @admin_required admits API-key callers,
|
||||
# for whom reading the JWT directly raises.
|
||||
caller = get_current_user()
|
||||
user_id = caller.id if caller else None
|
||||
success = fleet_monitor_service.acknowledge_alert(alert_id, user_id)
|
||||
if not success:
|
||||
return jsonify({'error': 'Cannot acknowledge alert'}), 400
|
||||
@@ -69,7 +72,7 @@ def acknowledge_alert(alert_id):
|
||||
|
||||
|
||||
@fleet_monitor_bp.route('/alerts/<alert_id>/resolve', methods=['POST'])
|
||||
@jwt_required()
|
||||
@admin_required
|
||||
def resolve_alert(alert_id):
|
||||
"""Resolve an alert."""
|
||||
success = fleet_monitor_service.resolve_alert(alert_id)
|
||||
|
||||
@@ -1,9 +1,8 @@
|
||||
from flask import Blueprint, jsonify, current_app
|
||||
from flask_jwt_extended import jwt_required
|
||||
|
||||
from app.models import User
|
||||
from app.services.migration_service import MigrationService
|
||||
from app.middleware.rbac import get_current_user
|
||||
from app.middleware.rbac import require_admin_user
|
||||
|
||||
migrations_bp = Blueprint('migrations', __name__)
|
||||
|
||||
@@ -19,9 +18,7 @@ def get_migration_status():
|
||||
@jwt_required()
|
||||
def create_backup():
|
||||
"""Create a database backup before applying migrations. Admin only."""
|
||||
user = get_current_user()
|
||||
if not user or user.role != User.ROLE_ADMIN:
|
||||
return jsonify({'error': 'Admin access required'}), 403
|
||||
require_admin_user()
|
||||
|
||||
result = MigrationService.create_backup(current_app)
|
||||
if result['success']:
|
||||
@@ -33,9 +30,7 @@ def create_backup():
|
||||
@jwt_required()
|
||||
def apply_migrations():
|
||||
"""Apply all pending migrations. Admin only."""
|
||||
user = get_current_user()
|
||||
if not user or user.role != User.ROLE_ADMIN:
|
||||
return jsonify({'error': 'Admin access required'}), 403
|
||||
require_admin_user()
|
||||
|
||||
result = MigrationService.apply_migrations(current_app)
|
||||
if result['success']:
|
||||
@@ -47,9 +42,7 @@ def apply_migrations():
|
||||
@jwt_required()
|
||||
def get_migration_history():
|
||||
"""Return all migration revisions. Admin only."""
|
||||
user = get_current_user()
|
||||
if not user or user.role != User.ROLE_ADMIN:
|
||||
return jsonify({'error': 'Admin access required'}), 403
|
||||
require_admin_user()
|
||||
|
||||
history = MigrationService.get_migration_history(current_app)
|
||||
return jsonify({'revisions': history}), 200
|
||||
@@ -10,6 +10,7 @@ from flask import Blueprint, request, jsonify
|
||||
from flask_jwt_extended import jwt_required
|
||||
|
||||
from app.exceptions import NotFoundError
|
||||
from app.middleware.rbac import permission_required
|
||||
from app.services.monitor_service import MonitorService
|
||||
|
||||
monitors_bp = Blueprint('monitors', __name__)
|
||||
@@ -49,7 +50,7 @@ def get_stats():
|
||||
|
||||
@monitors_bp.route('', methods=['POST'])
|
||||
@monitors_bp.route('/', methods=['POST'])
|
||||
@jwt_required()
|
||||
@permission_required('monitoring', 'write')
|
||||
def create_monitor():
|
||||
data = request.get_json() or {}
|
||||
monitor = MonitorService.create(data)
|
||||
@@ -66,7 +67,7 @@ def get_monitor(monitor_id):
|
||||
|
||||
|
||||
@monitors_bp.route('/<int:monitor_id>', methods=['PATCH', 'PUT'])
|
||||
@jwt_required()
|
||||
@permission_required('monitoring', 'write')
|
||||
def update_monitor(monitor_id):
|
||||
data = request.get_json() or {}
|
||||
monitor = MonitorService.update(monitor_id, data)
|
||||
@@ -76,7 +77,7 @@ def update_monitor(monitor_id):
|
||||
|
||||
|
||||
@monitors_bp.route('/<int:monitor_id>', methods=['DELETE'])
|
||||
@jwt_required()
|
||||
@permission_required('monitoring', 'write')
|
||||
def delete_monitor(monitor_id):
|
||||
if not MonitorService.delete(monitor_id):
|
||||
return {'error': 'Monitor not found'}, 404
|
||||
@@ -84,7 +85,7 @@ def delete_monitor(monitor_id):
|
||||
|
||||
|
||||
@monitors_bp.route('/<int:monitor_id>/check', methods=['POST'])
|
||||
@jwt_required()
|
||||
@permission_required('monitoring', 'write')
|
||||
def run_check(monitor_id):
|
||||
"""Probe now, out of band from the scheduler."""
|
||||
hc = MonitorService.run_check(monitor_id)
|
||||
@@ -95,7 +96,7 @@ def run_check(monitor_id):
|
||||
|
||||
|
||||
@monitors_bp.route('/<int:monitor_id>/pause', methods=['POST'])
|
||||
@jwt_required()
|
||||
@permission_required('monitoring', 'write')
|
||||
def set_paused(monitor_id):
|
||||
data = request.get_json() or {}
|
||||
monitor = MonitorService.set_paused(monitor_id, data.get('paused', True))
|
||||
@@ -148,7 +149,7 @@ def list_incidents():
|
||||
|
||||
|
||||
@monitors_bp.route('/incidents', methods=['POST'])
|
||||
@jwt_required()
|
||||
@permission_required('monitoring', 'write')
|
||||
def create_incident():
|
||||
data = request.get_json() or {}
|
||||
if not data.get('title'):
|
||||
@@ -158,7 +159,7 @@ def create_incident():
|
||||
|
||||
|
||||
@monitors_bp.route('/incidents/<int:incident_id>', methods=['PATCH', 'PUT'])
|
||||
@jwt_required()
|
||||
@permission_required('monitoring', 'write')
|
||||
def update_incident(incident_id):
|
||||
incident = MonitorService.update_incident(incident_id, request.get_json() or {})
|
||||
if not incident:
|
||||
@@ -167,7 +168,7 @@ def update_incident(incident_id):
|
||||
|
||||
|
||||
@monitors_bp.route('/incidents/<int:incident_id>', methods=['DELETE'])
|
||||
@jwt_required()
|
||||
@permission_required('monitoring', 'write')
|
||||
def delete_incident(incident_id):
|
||||
if not MonitorService.delete_incident(incident_id):
|
||||
return {'error': 'Incident not found'}, 404
|
||||
|
||||
@@ -33,6 +33,9 @@ def create_proxy():
|
||||
@nginx_advanced_bp.route('/test', methods=['POST'])
|
||||
@jwt_required()
|
||||
def test_config():
|
||||
user = get_current_user()
|
||||
if not user or not user.is_admin:
|
||||
return jsonify({'error': 'Admin access required'}), 403
|
||||
result = NginxAdvancedService.test_config()
|
||||
return jsonify(result)
|
||||
|
||||
@@ -50,12 +53,19 @@ def reload():
|
||||
@nginx_advanced_bp.route('/diff', methods=['POST'])
|
||||
@jwt_required()
|
||||
def preview_diff():
|
||||
# Admin-only: the diff includes the CURRENT vhost file's contents, so this
|
||||
# is a file read as much as a preview.
|
||||
user = get_current_user()
|
||||
if not user or not user.is_admin:
|
||||
return jsonify({'error': 'Admin access required'}), 403
|
||||
data = request.get_json() or {}
|
||||
domain = data.get('domain')
|
||||
new_config = data.get('config', '')
|
||||
if not domain:
|
||||
return jsonify({'error': 'domain required'}), 400
|
||||
result = NginxAdvancedService.preview_diff(domain, new_config)
|
||||
if 'error' in result:
|
||||
return jsonify(result), 400
|
||||
return jsonify(result)
|
||||
|
||||
|
||||
|
||||
@@ -134,6 +134,7 @@ def get_group(group_slug):
|
||||
def update_group(group_slug):
|
||||
try:
|
||||
_ensure_group_mutable(group_slug)
|
||||
_ensure_group_accessible(group_slug)
|
||||
data = request.get_json() or {}
|
||||
group = QueueBusService.update_group(
|
||||
slug=group_slug,
|
||||
@@ -151,6 +152,7 @@ def update_group(group_slug):
|
||||
def delete_group(group_slug):
|
||||
try:
|
||||
_ensure_group_mutable(group_slug)
|
||||
_ensure_group_accessible(group_slug)
|
||||
result = QueueBusService.delete_group(group_slug)
|
||||
return jsonify(result), 200
|
||||
except Exception as e:
|
||||
@@ -178,6 +180,7 @@ def list_queues(group_slug):
|
||||
def create_queue(group_slug):
|
||||
try:
|
||||
_ensure_group_mutable(group_slug)
|
||||
_ensure_group_accessible(group_slug)
|
||||
data = request.get_json() or {}
|
||||
slug = data.get('slug')
|
||||
name = data.get('name')
|
||||
@@ -216,6 +219,7 @@ def get_queue(group_slug, queue_slug):
|
||||
def update_queue(group_slug, queue_slug):
|
||||
try:
|
||||
_ensure_group_mutable(group_slug)
|
||||
_ensure_group_accessible(group_slug)
|
||||
data = request.get_json() or {}
|
||||
queue = QueueBusService.update_queue(
|
||||
group_slug=group_slug,
|
||||
@@ -234,6 +238,7 @@ def update_queue(group_slug, queue_slug):
|
||||
def delete_queue(group_slug, queue_slug):
|
||||
try:
|
||||
_ensure_group_mutable(group_slug)
|
||||
_ensure_group_accessible(group_slug)
|
||||
result = QueueBusService.delete_queue(group_slug, queue_slug)
|
||||
return jsonify(result), 200
|
||||
except Exception as e:
|
||||
@@ -269,6 +274,7 @@ def list_messages(group_slug, queue_slug):
|
||||
def send_message(group_slug, queue_slug):
|
||||
try:
|
||||
_ensure_group_mutable(group_slug)
|
||||
_ensure_group_accessible(group_slug)
|
||||
data = request.get_json() or {}
|
||||
payload = data.get('payload')
|
||||
priority = data.get('priority', 0)
|
||||
@@ -358,6 +364,7 @@ def fail_message(group_slug, queue_slug, message_id):
|
||||
def requeue_message(group_slug, queue_slug, message_id):
|
||||
try:
|
||||
_ensure_group_mutable(group_slug)
|
||||
_ensure_group_accessible(group_slug)
|
||||
message = QueueBusService.requeue(group_slug, queue_slug, message_id)
|
||||
return jsonify({'message': message}), 200
|
||||
except Exception as e:
|
||||
@@ -369,6 +376,7 @@ def requeue_message(group_slug, queue_slug, message_id):
|
||||
def delete_message(group_slug, queue_slug, message_id):
|
||||
try:
|
||||
_ensure_group_mutable(group_slug)
|
||||
_ensure_group_accessible(group_slug)
|
||||
result = QueueBusService.delete_message(group_slug, queue_slug, message_id)
|
||||
return jsonify(result), 200
|
||||
except Exception as e:
|
||||
|
||||
@@ -31,7 +31,7 @@ def list_deleted():
|
||||
|
||||
|
||||
@recycle_bin_bp.route('/<kind>/<int:record_id>/restore', methods=['POST'])
|
||||
@jwt_required()
|
||||
@admin_required
|
||||
def restore(kind, record_id):
|
||||
try:
|
||||
item, err = recycle_bin_service.restore(kind, record_id)
|
||||
|
||||
@@ -9,7 +9,7 @@ from flask import Blueprint, request, jsonify
|
||||
from flask_jwt_extended import jwt_required
|
||||
|
||||
from app.services.registrar_service import RegistrarService
|
||||
from app.middleware.rbac import get_current_user
|
||||
from app.middleware.rbac import admin_required, get_current_user
|
||||
|
||||
registrars_bp = Blueprint('registrars', __name__)
|
||||
|
||||
@@ -54,7 +54,7 @@ def delete_connection(cid):
|
||||
|
||||
|
||||
@registrars_bp.route('/connections/<int:cid>/test', methods=['POST'])
|
||||
@jwt_required()
|
||||
@admin_required
|
||||
def test_connection(cid):
|
||||
conn = RegistrarService.get_connection(cid)
|
||||
if not conn:
|
||||
@@ -71,7 +71,7 @@ def list_domains():
|
||||
|
||||
|
||||
@registrars_bp.route('/sync', methods=['POST'])
|
||||
@jwt_required()
|
||||
@admin_required
|
||||
def sync():
|
||||
"""Force-refresh the portfolio (and stamp last_synced_at)."""
|
||||
return jsonify({'domains': RegistrarService.sync_now()})
|
||||
@@ -3,7 +3,7 @@ from flask_jwt_extended import jwt_required
|
||||
from app.services.server_template_service import ServerTemplateService
|
||||
from app.services.audit_service import AuditService
|
||||
from app.models.audit_log import AuditLog
|
||||
from app.middleware.rbac import get_current_user
|
||||
from app.middleware.rbac import admin_required, get_current_user
|
||||
|
||||
server_templates_bp = Blueprint('server_templates', __name__)
|
||||
|
||||
@@ -150,7 +150,7 @@ def unassign(assignment_id):
|
||||
|
||||
|
||||
@server_templates_bp.route('/assignments/<int:assignment_id>/check', methods=['POST'])
|
||||
@jwt_required()
|
||||
@admin_required
|
||||
def check_drift(assignment_id):
|
||||
assignment = ServerTemplateService.check_drift(assignment_id)
|
||||
if not assignment:
|
||||
|
||||
@@ -795,7 +795,7 @@ def get_server_status(server_id):
|
||||
|
||||
|
||||
@servers_bp.route('/<server_id>/ping', methods=['POST'])
|
||||
@jwt_required()
|
||||
@developer_required
|
||||
def ping_server(server_id):
|
||||
"""Force a ping/health check on a server"""
|
||||
server = Server.query.get(server_id)
|
||||
@@ -1521,7 +1521,7 @@ def list_remote_compose_projects(server_id):
|
||||
|
||||
|
||||
@servers_bp.route('/<server_id>/docker/compose/ps', methods=['POST'])
|
||||
@jwt_required()
|
||||
@developer_required
|
||||
def remote_compose_ps(server_id):
|
||||
"""List containers for a compose project"""
|
||||
user_id = get_jwt_identity()
|
||||
@@ -1591,7 +1591,7 @@ def remote_compose_down(server_id):
|
||||
|
||||
|
||||
@servers_bp.route('/<server_id>/docker/compose/logs', methods=['POST'])
|
||||
@jwt_required()
|
||||
@developer_required
|
||||
def remote_compose_logs(server_id):
|
||||
"""Get logs from a compose project"""
|
||||
user_id = get_jwt_identity()
|
||||
@@ -2686,7 +2686,7 @@ def remote_cloudflared_delete(server_id, tunnel_ref):
|
||||
# Capabilities message so all panel listeners stay in sync.
|
||||
|
||||
@servers_bp.route('/<server_id>/refresh-capabilities', methods=['POST'])
|
||||
@jwt_required()
|
||||
@developer_required
|
||||
def remote_refresh_capabilities(server_id):
|
||||
user_id = get_jwt_identity()
|
||||
result = dispatch_agent_command(
|
||||
|
||||
@@ -208,6 +208,9 @@ def add_tag():
|
||||
missing = _require(data, 'resource_type', 'resource_id', 'tag')
|
||||
if missing:
|
||||
return _bad(f'{missing} is required')
|
||||
denied = _check_resource_write(data['resource_type'], data['resource_id'])
|
||||
if denied:
|
||||
return denied
|
||||
try:
|
||||
row = SharedResourceService.add_tag(
|
||||
data['resource_type'], data['resource_id'], data['tag']
|
||||
@@ -231,6 +234,10 @@ def remove_tag():
|
||||
if not resource_type or resource_id in (None, '') or not tag:
|
||||
return _bad('resource_type, resource_id and tag are required')
|
||||
|
||||
denied = _check_resource_write(resource_type, resource_id)
|
||||
if denied:
|
||||
return denied
|
||||
|
||||
removed = SharedResourceService.remove_tag(resource_type, resource_id, tag)
|
||||
return jsonify({'removed': removed})
|
||||
|
||||
|
||||
@@ -12,7 +12,7 @@ Provides REST endpoints for:
|
||||
|
||||
from flask import Blueprint, current_app, request, jsonify
|
||||
from flask_jwt_extended import jwt_required, get_jwt_identity
|
||||
from app.middleware.rbac import admin_required
|
||||
from app.middleware.rbac import admin_required, developer_required
|
||||
from app.models import User, Application
|
||||
from app.services.deployment_job_service import DeploymentJobService
|
||||
from app.services.template_service import TemplateService
|
||||
@@ -451,7 +451,7 @@ def validate_installation():
|
||||
|
||||
|
||||
@templates_bp.route('/test-db-connection', methods=['POST'])
|
||||
@jwt_required()
|
||||
@developer_required
|
||||
def test_db_connection():
|
||||
"""Test database connection before template installation.
|
||||
|
||||
|
||||
@@ -8,6 +8,7 @@ from flask import Blueprint, request, jsonify
|
||||
from flask_jwt_extended import jwt_required
|
||||
|
||||
from app.services.uptime_service import UptimeService
|
||||
from app.middleware.rbac import admin_required
|
||||
|
||||
uptime_bp = Blueprint('uptime', __name__)
|
||||
|
||||
@@ -66,7 +67,7 @@ def get_uptime_history():
|
||||
|
||||
|
||||
@uptime_bp.route('/tracking/start', methods=['POST'])
|
||||
@jwt_required()
|
||||
@admin_required
|
||||
def start_tracking():
|
||||
"""Start uptime tracking."""
|
||||
result = UptimeService.start_tracking()
|
||||
@@ -76,7 +77,7 @@ def start_tracking():
|
||||
|
||||
|
||||
@uptime_bp.route('/tracking/stop', methods=['POST'])
|
||||
@jwt_required()
|
||||
@admin_required
|
||||
def stop_tracking():
|
||||
"""Stop uptime tracking."""
|
||||
result = UptimeService.stop_tracking()
|
||||
|
||||
@@ -5,7 +5,9 @@ from app.services.audit_service import AuditService
|
||||
from app.models.audit_log import AuditLog
|
||||
# The workspace guards now live in middleware/rbac.py so any resource blueprint
|
||||
# can reuse them (plan 19 Decision 2). Re-exported here for existing callers.
|
||||
from app.middleware.rbac import get_current_user, require_workspace_access, require_workspace_role
|
||||
from app.middleware.rbac import (
|
||||
admin_required, get_current_user, require_workspace_access, require_workspace_role,
|
||||
)
|
||||
|
||||
workspaces_bp = Blueprint('workspaces', __name__)
|
||||
|
||||
@@ -49,7 +51,7 @@ def get_workspace(workspace_id):
|
||||
|
||||
|
||||
@workspaces_bp.route('/', methods=['POST'])
|
||||
@jwt_required()
|
||||
@admin_required
|
||||
def create_workspace():
|
||||
user = get_current_user()
|
||||
data = request.get_json()
|
||||
|
||||
@@ -62,6 +62,10 @@ def require_role(*allowed_roles):
|
||||
if user.role not in allowed_roles:
|
||||
return jsonify({'error': 'Insufficient permissions'}), 403
|
||||
return fn(*args, **kwargs)
|
||||
# Marks this view as carrying an explicit authorization decision, so the
|
||||
# default-deny sweep test (tests/test_route_authz_sweep.py) recognizes it
|
||||
# as gated rather than flagging it as an unguarded mutating route.
|
||||
wrapper._sk_authz = ('require_role', allowed_roles)
|
||||
return wrapper
|
||||
return decorator
|
||||
|
||||
@@ -86,6 +90,7 @@ def admin_required(fn):
|
||||
if not user.is_admin:
|
||||
return jsonify({'error': 'Admin access required'}), 403
|
||||
return fn(*args, **kwargs)
|
||||
wrapper._sk_authz = ('admin_required',)
|
||||
return wrapper
|
||||
|
||||
|
||||
@@ -136,6 +141,7 @@ def developer_required(fn):
|
||||
if not user.is_developer:
|
||||
return jsonify({'error': 'Developer access required'}), 403
|
||||
return fn(*args, **kwargs)
|
||||
wrapper._sk_authz = ('developer_required',)
|
||||
return wrapper
|
||||
|
||||
|
||||
@@ -162,6 +168,7 @@ def permission_required(feature, level='read'):
|
||||
'error': f'Permission denied: {level} access to {feature} required'
|
||||
}), 403
|
||||
return fn(*args, **kwargs)
|
||||
wrapper._sk_authz = ('permission_required', feature, level)
|
||||
return wrapper
|
||||
return decorator
|
||||
|
||||
@@ -278,5 +285,6 @@ def require_app_member(min_role='viewer', arg='app_id'):
|
||||
return jsonify({'error': 'Access denied'}), 403
|
||||
g.current_application = application
|
||||
return fn(*args, **kwargs)
|
||||
wrapper._sk_authz = ('require_app_member', min_role)
|
||||
return wrapper
|
||||
return decorator
|
||||
@@ -153,6 +153,11 @@ class NginxAdvancedService:
|
||||
@staticmethod
|
||||
def preview_diff(domain, new_config):
|
||||
"""Preview config changes as a diff."""
|
||||
# `domain` is joined onto SITES_AVAILABLE below — refuse anything that
|
||||
# is not a plain filename so ../ can't turn this into an arbitrary
|
||||
# file read (the API layer gates admins, but never trust one layer).
|
||||
if not domain or os.path.basename(domain) != domain or domain in ('.', '..'):
|
||||
return {'error': 'invalid domain'}
|
||||
conf_path = os.path.join(NginxAdvancedService.SITES_AVAILABLE, domain)
|
||||
old_config = ''
|
||||
if os.path.isfile(conf_path):
|
||||
|
||||
@@ -164,12 +164,15 @@ class WebhookGatewayService:
|
||||
db.session.add(delivery)
|
||||
db.session.commit()
|
||||
|
||||
if not signature_valid and signature_header:
|
||||
# Reject unconditionally: every endpoint has a secret, so a request with
|
||||
# a missing OR wrong signature is equally unauthenticated. The old
|
||||
# `and signature_header` clause let unsigned requests through.
|
||||
if not signature_valid:
|
||||
delivery.status = 'failed'
|
||||
delivery.error_message = 'Invalid signature'
|
||||
delivery.error_message = 'Invalid signature' if signature_header else 'Missing signature'
|
||||
delivery.completed_at = datetime.utcnow()
|
||||
db.session.commit()
|
||||
return {'success': False, 'error': 'Invalid signature', 'event_id': event_id}, 401
|
||||
return {'success': False, 'error': delivery.error_message, 'event_id': event_id}, 401
|
||||
|
||||
try:
|
||||
payload_json = json.loads(payload.decode('utf-8', errors='replace')) if payload else {}
|
||||
|
||||
@@ -1 +1 @@
|
||||
4309
|
||||
4417
|
||||
@@ -1 +1 @@
|
||||
605
|
||||
577
|
||||
@@ -3,7 +3,7 @@
|
||||
"policy": "Exact reviewed inventory of direct persistence, subprocess, and filesystem mutation calls in Flask API controllers. New calls are forbidden; remove entries as controllers delegate to services.",
|
||||
"counts": {
|
||||
"filesystem": 11,
|
||||
"persistence": 499
|
||||
"persistence": 498
|
||||
},
|
||||
"owners": {
|
||||
"api/admin.py": {
|
||||
@@ -369,7 +369,6 @@
|
||||
"api/apps.py::upload_app_archive::persistence::db.session.commit()::1",
|
||||
"api/apps.py::upload_app_archive::persistence::db.session.commit()::2",
|
||||
"api/apps.py::wake_app::persistence::Application.query_active().filter_by(id=app_id).first()::1",
|
||||
"api/auth.py::complete_onboarding::persistence::User.query.get(current_user_id)::1",
|
||||
"api/auth.py::create_login_link::persistence::User.query.get(target_id)::1",
|
||||
"api/auth.py::create_login_link::persistence::db.session.commit()::1",
|
||||
"api/auth.py::demo_info::persistence::User.query.filter_by(username='demo').first()::1",
|
||||
|
||||
@@ -0,0 +1,127 @@
|
||||
"""Proving tests for the inline guards added in the round-3 authz audit.
|
||||
|
||||
The two sweep tests can only see so much: ``test_route_authz_static.py`` proves
|
||||
a route *reaches* an authorization primitive, and
|
||||
``test_route_authz_sweep.py`` proves a viewer does not get a 2xx. Neither can
|
||||
tell whether the guard admits the right callers, and neither looks below the
|
||||
API layer at all. These are the behavioural checks for the three fixes in that
|
||||
round whose shape the sweeps cannot express:
|
||||
|
||||
* ``api_keys.update_key`` — the *only* key route with no role bar, so a viewer
|
||||
demoted after minting a key could still widen its scopes to ``['*']``.
|
||||
Invisible to the sweep because the dummy key id 404s first.
|
||||
* ``deployment_jobs.simulate_deployment`` — gated only by the demo flag, and
|
||||
``DEMO_DEPLOYS_ENABLED`` is on in testing, so a viewer could create real
|
||||
``DeploymentJob`` rows.
|
||||
* ``nginx_advanced_service.preview_diff`` — a body-supplied ``domain`` joined
|
||||
straight onto ``SITES_AVAILABLE``. The API layer now admits only admins, but
|
||||
the traversal guard belongs in the service, where every future caller
|
||||
inherits it.
|
||||
"""
|
||||
import pytest
|
||||
|
||||
from factories import make_user, headers_for
|
||||
|
||||
|
||||
# --------------------------------------------------------- API-key metadata
|
||||
|
||||
def _key_for(db, user):
|
||||
from app.services.api_key_service import ApiKeyService
|
||||
api_key, _raw = ApiKeyService.create_key(user.id, name='round3', scopes=['read'])
|
||||
return api_key
|
||||
|
||||
|
||||
def test_viewer_cannot_widen_the_scopes_of_their_own_api_key(client, db_session):
|
||||
"""Ownership is not the question — capability is.
|
||||
|
||||
``ApiKeyService.update_key`` already filters by ``user_id``, so this was
|
||||
never an IDOR; the hole was that a viewer could take a key they legally own
|
||||
and hand it ``scopes=['*']``, which every API-key-authenticated route then
|
||||
honours.
|
||||
"""
|
||||
viewer = make_user(db_session, role='viewer')
|
||||
key = _key_for(db_session, viewer)
|
||||
|
||||
resp = client.put(f'/api/v1/api-keys/{key.id}',
|
||||
json={'scopes': ['*'], 'tier': 'unlimited'},
|
||||
headers=headers_for(viewer))
|
||||
|
||||
assert resp.status_code == 403
|
||||
db_session.session.refresh(key)
|
||||
assert key.get_scopes() == ['read'], 'the viewer widened their key anyway'
|
||||
|
||||
|
||||
def test_developer_can_still_update_their_own_api_key(client, db_session):
|
||||
"""Non-vacuity: the gate is a role bar, not a freeze on the route."""
|
||||
dev = make_user(db_session, role='developer')
|
||||
key = _key_for(db_session, dev)
|
||||
|
||||
resp = client.put(f'/api/v1/api-keys/{key.id}',
|
||||
json={'name': 'renamed'},
|
||||
headers=headers_for(dev))
|
||||
|
||||
assert resp.status_code == 200
|
||||
assert resp.get_json()['name'] == 'renamed'
|
||||
|
||||
|
||||
# ------------------------------------------------------ demo deployment jobs
|
||||
|
||||
def test_viewer_cannot_start_a_simulated_deployment(app, client, db_session):
|
||||
"""``DEMO_DEPLOYS_ENABLED`` is a *visibility* flag, not an authorization
|
||||
one — with it on, job creation still has to be a developer capability."""
|
||||
from app.models.deployment_job import DeploymentJob
|
||||
|
||||
app.config['DEMO_DEPLOYS_ENABLED'] = True
|
||||
before = DeploymentJob.query.count()
|
||||
|
||||
resp = client.post('/api/v1/deployment-jobs/simulate?wait=true',
|
||||
json={'scenario': 'success', 'speed': 'instant'},
|
||||
headers=headers_for(make_user(db_session, role='viewer')))
|
||||
|
||||
assert resp.status_code == 403
|
||||
assert DeploymentJob.query.count() == before, 'a viewer created a job row'
|
||||
|
||||
|
||||
# --------------------------------------------- nginx vhost path containment
|
||||
|
||||
@pytest.mark.parametrize('domain', [
|
||||
'../../../../etc/passwd',
|
||||
'..',
|
||||
'.',
|
||||
'sub/dir/example.com',
|
||||
'',
|
||||
None,
|
||||
])
|
||||
def test_preview_diff_refuses_anything_that_is_not_a_plain_filename(app, domain):
|
||||
"""``domain`` is joined onto ``/etc/nginx/sites-available``; a traversing
|
||||
value turns a config preview into an arbitrary file read, since the diff
|
||||
body carries the current file's contents verbatim."""
|
||||
from app.services.nginx_advanced_service import NginxAdvancedService
|
||||
|
||||
result = NginxAdvancedService.preview_diff(domain, 'new config')
|
||||
|
||||
assert result == {'error': 'invalid domain'}
|
||||
|
||||
|
||||
def test_preview_diff_still_diffs_a_plain_domain(app, tmp_path, monkeypatch):
|
||||
"""Non-vacuity: a normal vhost name is accepted and really is diffed."""
|
||||
from app.services.nginx_advanced_service import NginxAdvancedService
|
||||
|
||||
monkeypatch.setattr(NginxAdvancedService, 'SITES_AVAILABLE', str(tmp_path))
|
||||
(tmp_path / 'example.com').write_text('server { listen 80; }\n', encoding='utf-8')
|
||||
|
||||
result = NginxAdvancedService.preview_diff('example.com', 'server { listen 443; }\n')
|
||||
|
||||
assert 'error' not in result
|
||||
assert result['has_changes'] is True
|
||||
assert 'listen 443' in result['diff']
|
||||
|
||||
|
||||
def test_preview_diff_route_reports_the_refusal_as_a_400(client, db_session):
|
||||
"""The API must not hand the service's error dict back as a 200 success."""
|
||||
resp = client.post('/api/v1/nginx/advanced/diff',
|
||||
json={'domain': '../../etc/passwd', 'config': 'x'},
|
||||
headers=headers_for(make_user(db_session, role='admin')))
|
||||
|
||||
assert resp.status_code == 400
|
||||
assert resp.get_json() == {'error': 'invalid domain'}
|
||||
@@ -95,3 +95,31 @@ def test_no_revision_at_all_is_not_treated_as_orphaned(migration_state):
|
||||
status = migration_state(None, '080_dashboard_boards')
|
||||
assert status['orphaned_revision'] is False
|
||||
assert status['needs_migration'] is False
|
||||
|
||||
|
||||
# --- authorization ---------------------------------------------------------
|
||||
# `/migrations/history` is the one admin-gated route on this blueprint with no
|
||||
# side effect, which made it the one whose gate nothing exercised: when the
|
||||
# module moved to `require_admin_user()` it kept an inline `User.ROLE_ADMIN`
|
||||
# check whose imports had already been deleted, so every call — admin included
|
||||
# — raised NameError. The gate has to be fired, not just read.
|
||||
|
||||
def test_history_is_admin_only(client, db_session):
|
||||
from factories import make_user, headers_for
|
||||
|
||||
for role in ('viewer', 'developer'):
|
||||
response = client.get('/api/v1/migrations/history',
|
||||
headers=headers_for(make_user(db_session, role=role)))
|
||||
assert response.status_code == 403, (
|
||||
f'a {role} read the migration history (got {response.status_code})'
|
||||
)
|
||||
|
||||
|
||||
def test_history_answers_an_admin(client, db_session):
|
||||
"""Non-vacuity for the test above: the same route succeeds for an admin."""
|
||||
from factories import make_user, headers_for
|
||||
|
||||
response = client.get('/api/v1/migrations/history',
|
||||
headers=headers_for(make_user(db_session, role='admin')))
|
||||
assert response.status_code == 200
|
||||
assert isinstance(response.get_json()['revisions'], list)
|
||||
@@ -0,0 +1,335 @@
|
||||
"""Static authorization-coverage test — every mutating route must carry an
|
||||
authorization *decision*, proven from the source, not from firing requests.
|
||||
|
||||
Companion to ``test_route_authz_sweep.py``. The two are deliberately
|
||||
complementary:
|
||||
|
||||
* The sweep (live-fire) proves **enforcement**: a real viewer JWT is refused.
|
||||
It cannot prove coverage — routes that 400/404 on a dummy request before the
|
||||
gate runs are invisible to it (GHSA-r4q7-x795-vr4j's ``POST /workspaces/``
|
||||
400s on an empty body, so the net would never have caught it).
|
||||
* This test proves **coverage**: every mutating route in ``app.url_map`` either
|
||||
carries a recognized gate, reaches an authorization primitive in its module's
|
||||
call graph, or sits in a small reviewed allowlist with a written
|
||||
justification. It cannot prove a gate actually rejects — that stays with the
|
||||
sweep.
|
||||
|
||||
How a route passes (first match wins):
|
||||
|
||||
1. ``_sk_authz`` marker on the view function — set by the rbac decorators
|
||||
(``@admin_required``, ``@developer_required``, ``@permission_required``,
|
||||
``@require_role``, ``@require_app_member``). ``functools.wraps`` propagates
|
||||
the marker outward through ``@jwt_required``.
|
||||
2. AST reachability: the view's module-level call graph reaches a name in
|
||||
``AUTHORIZATION_PRIMITIVES`` (inline helpers like ``require_admin_user``,
|
||||
``_check_scope``, ``can_edit_app``…) or inspects ``.is_admin`` /
|
||||
``.is_developer``.
|
||||
3. ``SELF_SCOPED[endpoint]`` — a *reviewed* route that acts only on the
|
||||
caller's own rows. Listing is explicit (never inferred) because calling
|
||||
``get_current_user()`` is necessary but NOT sufficient: the advisory's
|
||||
``create_workspace`` used the caller id as the *owner of a new global row*,
|
||||
which is not self-scoping. The route must still reach an identity primitive.
|
||||
4. ``PUBLIC_ALT_AUTH[endpoint]`` — public by design; the value must name the
|
||||
alternative mechanism (webhook signature, pairing code, enrollment secret).
|
||||
5. ``STATELESS[endpoint]`` — pure computation: no persistence, no outbound
|
||||
connections, no command execution on attacker-controlled input.
|
||||
|
||||
Anything else fails with instructions. Scope note: routes served from
|
||||
``builtin-extensions/`` (mounted under ``/api/v1/<ext>``) are excluded — they
|
||||
use the ``auth_required`` API-key model, a different surface that deserves its
|
||||
own pass.
|
||||
"""
|
||||
import ast
|
||||
import inspect
|
||||
|
||||
import pytest
|
||||
|
||||
|
||||
# --- authorization vocabulary ----------------------------------------------
|
||||
|
||||
# Inline primitives: calling any of these (directly or through a module-local
|
||||
# helper) counts as an authorization decision. Names are matched against both
|
||||
# plain calls (``require_admin_user()``) and method calls
|
||||
# (``WorkspaceService.get_user_role(...)``).
|
||||
AUTHORIZATION_PRIMITIVES = frozenset({
|
||||
# app/middleware/rbac.py
|
||||
'require_admin_user',
|
||||
'require_workspace_access',
|
||||
'require_workspace_role',
|
||||
# workspace / app / server access checks
|
||||
'get_user_role',
|
||||
'check_app_access',
|
||||
'can_access_app',
|
||||
'can_edit_app',
|
||||
'can_operate_app',
|
||||
'can_write_in_workspace',
|
||||
'verify_workspace_access',
|
||||
'verify_server_access',
|
||||
'check_server_access',
|
||||
'can_access_server',
|
||||
'require_app_access',
|
||||
# per-file gate helpers established by the audit
|
||||
'_check_scope',
|
||||
'_check_resource_write',
|
||||
'_endpoint_gate',
|
||||
'_vault_gate',
|
||||
'_ensure_group_accessible',
|
||||
'_load_app_for',
|
||||
'_require_admin',
|
||||
})
|
||||
|
||||
# Inspecting these attributes on the current user is an inline role check.
|
||||
ATTRIBUTE_GATES = frozenset({'is_admin', 'is_developer'})
|
||||
|
||||
# Identity resolution — required for SELF_SCOPED, never sufficient alone.
|
||||
IDENTITY_PRIMITIVES = frozenset({
|
||||
'get_current_user', 'get_jwt_identity', 'get_jwt',
|
||||
})
|
||||
|
||||
|
||||
# --- reviewed allowlists (endpoint -> justification) -------------------------
|
||||
# Every entry was reviewed against the code (audit of 2026-08-19; evidence
|
||||
# file:line recorded in the review). The justification is load-bearing: it is
|
||||
# the reason this route may legitimately lack a role gate. Keep it specific.
|
||||
|
||||
SELF_SCOPED = {
|
||||
# Acts only on the caller's own rows, ownership enforced in the service.
|
||||
'ai.chat': 'conversation ownership via _load_or_create/_owned_conversation',
|
||||
'ai.chat_cancel': 'ownership checked before cancel',
|
||||
'ai.chat_confirm': 'conversation ownership + pending action belongs to caller',
|
||||
'ai.chat_stream': 'conversation ownership via _load_or_create',
|
||||
'ai.create_conversation': 'row stamped with caller user_id',
|
||||
'ai.delete_conversation': '_owned_conversation gate',
|
||||
'ai.rename_conversation': '_owned_conversation gate',
|
||||
'api_keys.revoke_key': 'service query filtered by caller user_id',
|
||||
'api_keys.rotate_key': 'service query filtered by caller user_id',
|
||||
'auth.passkey_delete': 'credential lookup filtered by id + caller user_id',
|
||||
'auth.passkey_register': 'registers a passkey for the JWT user only',
|
||||
'auth.passkey_register_options': 'challenge issued for the JWT user only',
|
||||
'auth.refresh': 'mints an access token only for the refresh-token identity',
|
||||
'auth.update_current_user': 'mutates only the caller\'s own user row',
|
||||
'dashboards.create_board': 'board created with caller user_id',
|
||||
'dashboards.delete_board': 'get_board_for_user scopes id + user_id',
|
||||
'dashboards.reset_board': 'get_board_for_user scopes id + user_id',
|
||||
'dashboards.update_board': 'get_board_for_user scopes id + user_id',
|
||||
'event_subscriptions.delete_subscription': 'service get_for_user enforces admin-or-owner',
|
||||
'event_subscriptions.retry_delivery': 'routes through get_for_user (owner check)',
|
||||
'event_subscriptions.test_subscription': 'routes through get_for_user (owner check)',
|
||||
'event_subscriptions.update_subscription': 'routes through get_for_user (owner check)',
|
||||
'mobile.register_push': 'writes the caller\'s own push_subscriptions_json',
|
||||
'notifications.mark_inbox_all_read': 'mark_all_read scoped to caller user_id',
|
||||
'notifications.mark_inbox_read': 'inbox query scoped to caller user_id',
|
||||
'notifications.test_user_notification': 'sends only to the caller\'s own channels',
|
||||
'notifications.unmute_own_email': 'unmutes the caller\'s own email only',
|
||||
'notifications.update_user_preferences': 'preferences row keyed to caller',
|
||||
'queue_bus.create_group': 'non-admin owner_type/owner_id forced to the caller',
|
||||
'secrets_webhooks.create_webhook_endpoint': 'workspace resolved via membership-checked resolve_workspace_id; endpoint stamped with caller user_id',
|
||||
'source_connections.bitbucket_callback': 'completes OAuth for the caller\'s user_id',
|
||||
'source_connections.disconnect_bitbucket': 'disconnect scoped to caller user_id',
|
||||
'source_connections.disconnect_github': 'disconnect scoped to caller user_id',
|
||||
'source_connections.disconnect_gitlab': 'disconnect scoped to caller user_id',
|
||||
'source_connections.github_callback': 'completes OAuth for the caller\'s user_id',
|
||||
'source_connections.gitlab_callback': 'completes OAuth for the caller\'s user_id',
|
||||
'sso.link_provider': 'links identity to the JWT user with cross-account conflict check',
|
||||
'sso.unlink_provider': 'unlink_identity scoped to caller user_id',
|
||||
'two_factor.confirm_2fa_setup': 'own user + valid TOTP required',
|
||||
'two_factor.disable_2fa': 'own user, requires TOTP/backup code',
|
||||
'two_factor.initiate_2fa_setup': 'own user 2FA setup only',
|
||||
'two_factor.regenerate_backup_codes': 'own user, TOTP required',
|
||||
'views.create_view': 'view created with caller user_id',
|
||||
'views.delete_view': 'query filtered by id + caller user_id',
|
||||
'views.update_view': 'query filtered by id + caller user_id',
|
||||
}
|
||||
|
||||
PUBLIC_ALT_AUTH = {
|
||||
# Public by design; the value names the mechanism that authenticates the call.
|
||||
'agent_poll.connect': 'agent HMAC handshake + per-IP throttle',
|
||||
'agent_poll.disconnect': 'X-Session-Token lookup',
|
||||
'agent_poll.poll': 'X-Session-Token resolves the ConnectedAgent',
|
||||
'agent_poll.result': 'X-Session-Token resolves the ConnectedAgent',
|
||||
'auth.login': 'password credential + per-IP/per-user throttles + lockout',
|
||||
'auth.passkey_authenticate': 'WebAuthn assertion verification, 5/min',
|
||||
'auth.redeem_login_link': 'single-use login-link token + per-IP throttle',
|
||||
'auth.register': 'invite token / first-user / registration-enabled flag, 3/min',
|
||||
'ddns.update': 'per-host token is the credential (DynDNS protocol)',
|
||||
'deploy.webhook': 'URL token + provider signature verified before handling',
|
||||
'error_logs.client_error_log': 'unauthenticated client error intake: per-IP rate limit + strict field validation',
|
||||
'git.receive_webhook': 'webhook token lookup + HMAC signature before processing',
|
||||
'notifications.inbound_email_webhook': 'HMAC-SHA256 over raw body; 404 when secret unset',
|
||||
'pairing.code_freeze': 'enrollment_id + enrollment_secret',
|
||||
'pairing.code_refresh': 'enrollment_id + enrollment_secret',
|
||||
'pairing.enroll': 'pubkey+passphrase enrollment; produces a PENDING enrollment needing developer-gated claim',
|
||||
'pairing.poll': 'enrollment creds + optional Ed25519 proof-of-possession',
|
||||
'preview_webhooks.receive_pull_request': 'webhook token lookup + HMAC signature before any action',
|
||||
'secrets_webhooks.receive_webhook': 'HMAC signature mandatory since the 2026-08 fix (missing or wrong signature -> 401)',
|
||||
'servers.register_agent': 'single-use hashed registration token, 5/min',
|
||||
'sso.callback': 'OAuth authorization-code + state exchange with the provider',
|
||||
'sso.saml_callback': 'SAML assertion validated by the SSO service',
|
||||
'two_factor.verify_2fa_code': 'short-lived 2fa_pending token + TOTP/backup code + throttle',
|
||||
}
|
||||
|
||||
STATELESS = {
|
||||
# Persists nothing, dials nothing attacker-chosen, executes nothing
|
||||
# attacker-controlled.
|
||||
'agent_plugins.validate_manifest': 'pure manifest schema validation',
|
||||
'auth.passkey_auth_options': 'challenge issuance only, no mutation',
|
||||
'buildpacks.generate': 'pure plan -> Dockerfile/compose templating',
|
||||
'cron.preview_schedule': 'pure cron-expression computation',
|
||||
'dns_cutover.ttl_guidance': 'pure computation over supplied records',
|
||||
'dns_cutover.verify': 'read-only DNS lookups against public resolvers',
|
||||
'docker.get_containers_stats': 'read-via-POST of container stats; viewer holds docker:read',
|
||||
'firewall.rule_removal_preflight': 'pure analysis against current rules',
|
||||
'htaccess_tools.htaccess_convert': 'pure text transform, 256KB cap',
|
||||
'mobile.execute_quick_action': 'stub: canned no-op responses / live metrics read only',
|
||||
'mobile.unregister_push': 'stub: returns success without touching the DB',
|
||||
'servers.check_agent_version': 'agent-facing version check; outbound only to a fixed GitHub releases URL, cached',
|
||||
'templates.validate_installation': 'validation only (name-clash read + variable checks)',
|
||||
}
|
||||
|
||||
|
||||
# --- AST machinery -----------------------------------------------------------
|
||||
|
||||
_MUTATING = {'POST', 'PUT', 'PATCH', 'DELETE'}
|
||||
|
||||
|
||||
def _mutating_rules(app):
|
||||
"""Mutating rules served from backend/app/api (the core surface).
|
||||
|
||||
Yields ``(rule, view, unwrapped)``: ``view`` is the registered (possibly
|
||||
decorator-wrapped) endpoint function — where ``_sk_authz`` markers live —
|
||||
and ``unwrapped`` is the original view reached through ``__wrapped__``
|
||||
chains, which is what source-file attribution and AST analysis must use
|
||||
(``inspect.getsourcefile`` on a wrapped view returns the *decorator's*
|
||||
module, e.g. flask_jwt_extended, not the route's file).
|
||||
"""
|
||||
for rule in app.url_map.iter_rules():
|
||||
if not (rule.methods & _MUTATING):
|
||||
continue
|
||||
view = app.view_functions.get(rule.endpoint)
|
||||
if view is None:
|
||||
continue
|
||||
try:
|
||||
unwrapped = inspect.unwrap(view)
|
||||
except ValueError: # wrapper cycle
|
||||
unwrapped = view
|
||||
src = inspect.getsourcefile(unwrapped)
|
||||
if src is None or '\\app\\api\\' not in src.replace('/', '\\'):
|
||||
continue # builtin-extensions and flask internals: separate surface
|
||||
yield rule, view, unwrapped
|
||||
|
||||
|
||||
def _module_functions(path):
|
||||
"""name -> FunctionDef for every function defined in the module."""
|
||||
with open(path, encoding='utf-8') as fh:
|
||||
tree = ast.parse(fh.read())
|
||||
return {n.name: n for n in ast.walk(tree)
|
||||
if isinstance(n, (ast.FunctionDef, ast.AsyncFunctionDef))}
|
||||
|
||||
|
||||
def _called_names(fn):
|
||||
"""Every called name plus gated-attribute lookups in one function body."""
|
||||
names = set()
|
||||
for node in ast.walk(fn):
|
||||
if isinstance(node, ast.Call):
|
||||
func = node.func
|
||||
if isinstance(func, ast.Name):
|
||||
names.add(func.id)
|
||||
elif isinstance(func, ast.Attribute):
|
||||
names.add(func.attr)
|
||||
elif isinstance(node, ast.Attribute) and node.attr in ATTRIBUTE_GATES:
|
||||
names.add(node.attr)
|
||||
return names
|
||||
|
||||
|
||||
def _reachable_names(funcs, start):
|
||||
"""Transitive closure of names callable from `start` within the module."""
|
||||
calls = {name: _called_names(f) for name, f in funcs.items()}
|
||||
seen, stack = set(), [start]
|
||||
while stack:
|
||||
for callee in calls.get(stack.pop(), ()):
|
||||
if callee not in seen:
|
||||
seen.add(callee)
|
||||
stack.append(callee)
|
||||
return seen
|
||||
|
||||
|
||||
# --- the test ----------------------------------------------------------------
|
||||
|
||||
def test_every_mutating_route_carries_an_authorization_decision(app):
|
||||
# Parse each api module at most once.
|
||||
module_funcs = {}
|
||||
|
||||
def reachable(unwrapped):
|
||||
path = inspect.getsourcefile(unwrapped)
|
||||
if path not in module_funcs:
|
||||
module_funcs[path] = _module_functions(path)
|
||||
return _reachable_names(module_funcs[path], unwrapped.__name__)
|
||||
|
||||
ungated = []
|
||||
for rule, view, unwrapped in _mutating_rules(app):
|
||||
endpoint = rule.endpoint
|
||||
|
||||
# 1. Decorator gate (marker propagates through functools.wraps).
|
||||
if getattr(view, '_sk_authz', None):
|
||||
continue
|
||||
|
||||
names = reachable(unwrapped)
|
||||
|
||||
# 2. Inline primitive or attribute role check.
|
||||
if names & AUTHORIZATION_PRIMITIVES or names & ATTRIBUTE_GATES:
|
||||
continue
|
||||
|
||||
# 3. Reviewed self-scoped route — must resolve the caller's identity.
|
||||
if endpoint in SELF_SCOPED and names & IDENTITY_PRIMITIVES:
|
||||
continue
|
||||
|
||||
# 4/5. Reviewed public-with-alt-auth / stateless routes.
|
||||
if endpoint in PUBLIC_ALT_AUTH or endpoint in STATELESS:
|
||||
continue
|
||||
|
||||
methods = sorted(rule.methods & _MUTATING)
|
||||
ungated.append(f'{methods} {rule.rule} ({endpoint})')
|
||||
|
||||
assert not ungated, (
|
||||
'Mutating route(s) with no authorization decision found:\n '
|
||||
+ '\n '.join(sorted(ungated))
|
||||
+ '\n\nFix one of:'
|
||||
'\n - add a gate decorator (@admin_required / @developer_required /'
|
||||
' @permission_required), or'
|
||||
'\n - call an inline authorization primitive (require_admin_user,'
|
||||
' check_app_access, _check_scope, ...), or'
|
||||
'\n - if the route is genuinely self-scoped / public-with-alt-auth /'
|
||||
' stateless, add it to the matching allowlist in this file with a'
|
||||
' specific justification.'
|
||||
)
|
||||
|
||||
|
||||
def test_static_allowlists_are_not_stale(app):
|
||||
"""Every allowlisted endpoint must still exist and still need its listing.
|
||||
|
||||
A stale entry is a hiding place: if the route is renamed or gains a real
|
||||
gate, the entry must leave so a future route cannot reuse the slot.
|
||||
"""
|
||||
live = {rule.endpoint for rule, _, _ in _mutating_rules(app)}
|
||||
listed = set(SELF_SCOPED) | set(PUBLIC_ALT_AUTH) | set(STATELESS)
|
||||
stale = listed - live
|
||||
assert not stale, f'Allowlist entries for endpoints that no longer exist: {sorted(stale)}'
|
||||
|
||||
|
||||
def test_self_scoped_entries_actually_resolve_identity(app):
|
||||
"""A SELF_SCOPED entry whose route no longer resolves the caller's identity
|
||||
is not self-scoped by any definition — drop the entry or fix the route."""
|
||||
module_funcs = {}
|
||||
bogus = []
|
||||
for rule, _, unwrapped in _mutating_rules(app):
|
||||
if rule.endpoint not in SELF_SCOPED:
|
||||
continue
|
||||
path = inspect.getsourcefile(unwrapped)
|
||||
if path not in module_funcs:
|
||||
module_funcs[path] = _module_functions(path)
|
||||
names = _reachable_names(module_funcs[path], unwrapped.__name__)
|
||||
if not (names & IDENTITY_PRIMITIVES):
|
||||
bogus.append(rule.endpoint)
|
||||
assert not bogus, (
|
||||
f'SELF_SCOPED entries whose view never resolves the caller identity: {sorted(bogus)}'
|
||||
)
|
||||
@@ -0,0 +1,292 @@
|
||||
"""Authorization sweep — the default-deny backstop for mutating routes.
|
||||
|
||||
Motivation
|
||||
----------
|
||||
GHSA-r4q7-x795-vr4j: ``POST /api/v1/workspaces/`` shipped gated only by
|
||||
``@jwt_required()``, so a read-only *viewer* could create and administer
|
||||
workspaces. It was one forgotten authorization check, and it was not the only
|
||||
one — an audit of every mutating route turned up the same class in the docker
|
||||
compose read routes (arbitrary-path log/config disclosure) and the whole
|
||||
monitors surface (viewer could create/edit/delete monitors and incidents).
|
||||
|
||||
The root cause is architectural: ServerKit has **no enforced default-deny**.
|
||||
Authorization is opt-in per route, via a decorator *or* an inline helper
|
||||
(``_can_edit_app``, ``require_workspace_role``, ``_check_scope``, ...). Nothing
|
||||
fails closed when an author forgets, so each new feature file is one missing
|
||||
line away from re-introducing the bug.
|
||||
|
||||
This module is the CI backstop for that class. It has two parts:
|
||||
|
||||
* ``test_fixed_routes_reject_non_admins`` — a precise, hermetic proof that the
|
||||
specific routes fixed alongside the advisory (docker compose + monitors)
|
||||
reject a viewer *and* a developer. These use ``@permission_required`` /
|
||||
``@admin_required``, which run before the view body, so a dummy id still
|
||||
exercises the gate (403, not a 404 for the missing row).
|
||||
|
||||
* ``test_no_mutating_route_succeeds_for_a_viewer`` — the regression net. It
|
||||
fires every mutating route as a viewer and asserts none returns a 2xx unless
|
||||
its endpoint is in ``VIEWER_WRITABLE`` (a reviewed allowlist). A newly-added
|
||||
route that forgets to gate and happily answers a viewer with ``201`` trips
|
||||
this test until the author either gates it or consciously allowlists it.
|
||||
|
||||
Known limitation: a route with a path parameter that resolves its resource
|
||||
(and 404s) *before* checking authorization is invisible to the live-fire net
|
||||
— the 404 masks a missing gate. The net therefore catches the clearest
|
||||
regressions (creates and global toggles with no path param); resource-scoped
|
||||
routes need their own per-feature tests. See ``VIEWER_WRITABLE`` notes for the
|
||||
handful of endpoints that remain viewer-writable and are tracked as follow-ups.
|
||||
"""
|
||||
import pytest
|
||||
|
||||
from factories import make_user, headers_for
|
||||
|
||||
|
||||
# --- Part A: the routes fixed alongside the advisory -----------------------
|
||||
|
||||
# (endpoint path, method) — every mutating route on the docker compose read
|
||||
# surface and the monitors surface. A viewer and a developer must both be
|
||||
# refused; only an admin gets through.
|
||||
_MONITORS_WRITE = [
|
||||
('/api/v1/monitors/', 'POST'),
|
||||
('/api/v1/monitors/1', 'PUT'),
|
||||
('/api/v1/monitors/1', 'DELETE'),
|
||||
('/api/v1/monitors/1/check', 'POST'),
|
||||
('/api/v1/monitors/1/pause', 'POST'),
|
||||
('/api/v1/monitors/incidents', 'POST'),
|
||||
('/api/v1/monitors/incidents/1', 'PUT'),
|
||||
('/api/v1/monitors/incidents/1', 'DELETE'),
|
||||
]
|
||||
|
||||
_DOCKER_COMPOSE_READ = [
|
||||
('/api/v1/docker/compose/ps', 'POST'),
|
||||
('/api/v1/docker/compose/logs', 'POST'),
|
||||
('/api/v1/docker/compose/validate', 'POST'),
|
||||
('/api/v1/docker/compose/config', 'POST'),
|
||||
]
|
||||
|
||||
# The advisory fix itself plus the viewer-write gaps the same audit surfaced:
|
||||
# workspace creation, the global uptime on/off toggles, registrar sync/test
|
||||
# (their add/delete siblings were already admin-only), and template drift checks.
|
||||
_ADMIN_GATED_ROUND2 = [
|
||||
('/api/v1/workspaces/', 'POST'),
|
||||
('/api/v1/uptime/tracking/start', 'POST'),
|
||||
('/api/v1/uptime/tracking/stop', 'POST'),
|
||||
('/api/v1/registrars/sync', 'POST'),
|
||||
('/api/v1/registrars/connections/1/test', 'POST'),
|
||||
('/api/v1/server-templates/assignments/1/check', 'POST'),
|
||||
]
|
||||
|
||||
_FIXED_ROUTES = _MONITORS_WRITE + _DOCKER_COMPOSE_READ + _ADMIN_GATED_ROUND2
|
||||
|
||||
# Round 3 — gaps surfaced by the static-coverage audit (see
|
||||
# test_route_authz_static.py, seeded 2026-08-19). Admin-gated: both a viewer
|
||||
# and a developer must be refused.
|
||||
_ADMIN_GATED_ROUND3 = [
|
||||
('/api/v1/fleet-monitor/alerts/a1/acknowledge', 'POST'),
|
||||
('/api/v1/fleet-monitor/alerts/a1/resolve', 'POST'),
|
||||
('/api/v1/recycle-bin/apps/1/restore', 'POST'),
|
||||
('/api/v1/nginx/advanced/test', 'POST'),
|
||||
('/api/v1/nginx/advanced/diff', 'POST'),
|
||||
]
|
||||
|
||||
# Developer-gated: a viewer is refused; a developer is admitted (the body may
|
||||
# still 400/404/500 on the dummy payload — only the gate must not fire).
|
||||
_DEVELOPER_GATED_ROUND3 = [
|
||||
('/api/v1/servers/s1/docker/compose/ps', 'POST'),
|
||||
('/api/v1/servers/s1/docker/compose/logs', 'POST'),
|
||||
('/api/v1/servers/s1/refresh-capabilities', 'POST'),
|
||||
('/api/v1/servers/s1/ping', 'POST'),
|
||||
('/api/v1/templates/test-db-connection', 'POST'),
|
||||
('/api/v1/deploy/branches', 'POST'),
|
||||
('/api/v1/buildpacks/detect', 'POST'),
|
||||
]
|
||||
|
||||
_FIXED_ROUTES += _ADMIN_GATED_ROUND3
|
||||
|
||||
|
||||
@pytest.mark.parametrize('path,method', _FIXED_ROUTES)
|
||||
@pytest.mark.parametrize('role', ['viewer', 'developer'])
|
||||
def test_fixed_routes_reject_non_admins(client, db_session, path, method, role):
|
||||
"""A viewer and a developer are refused (403) on every fixed route.
|
||||
|
||||
The gate is a decorator, so it fires before the view body — the dummy id
|
||||
``1`` never matters; we never reach the row lookup.
|
||||
"""
|
||||
headers = headers_for(make_user(db_session, role=role))
|
||||
resp = client.open(path, method=method, headers=headers, json={})
|
||||
assert resp.status_code == 403, (
|
||||
f'{method} {path} let a {role} through with {resp.status_code}; '
|
||||
f'expected 403 from the authorization gate'
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.parametrize('path,method', _FIXED_ROUTES)
|
||||
def test_fixed_routes_admit_admin(client, db_session, path, method):
|
||||
"""Sanity: an admin is NOT stopped by the authorization gate.
|
||||
|
||||
The body may still 400/404/500 on the dummy payload — we only assert the
|
||||
gate itself did not reject the admin (never 401/403).
|
||||
"""
|
||||
if path == '/api/v1/uptime/tracking/start':
|
||||
# Side-effecting for an admin (spawns the tracking daemon thread);
|
||||
# the viewer/developer 403 above is the security assertion here.
|
||||
return
|
||||
headers = headers_for(make_user(db_session, role='admin'))
|
||||
resp = client.open(path, method=method, headers=headers, json={})
|
||||
assert resp.status_code not in (401, 403), (
|
||||
f'{method} {path} wrongly blocked an admin with {resp.status_code}'
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.parametrize('path,method', _DEVELOPER_GATED_ROUND3)
|
||||
def test_developer_gated_routes_reject_viewers(client, db_session, path, method):
|
||||
"""A viewer is refused (403) on every developer-gated round-3 route."""
|
||||
headers = headers_for(make_user(db_session, role='viewer'))
|
||||
resp = client.open(path, method=method, headers=headers, json={})
|
||||
assert resp.status_code == 403, (
|
||||
f'{method} {path} let a viewer through with {resp.status_code}; '
|
||||
f'expected 403 from @developer_required'
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.parametrize('path,method', _DEVELOPER_GATED_ROUND3)
|
||||
def test_developer_gated_routes_admit_developers(client, db_session, path, method):
|
||||
"""Sanity: a developer is NOT stopped by the gate (body may 400/404/500)."""
|
||||
headers = headers_for(make_user(db_session, role='developer'))
|
||||
resp = client.open(path, method=method, headers=headers, json={})
|
||||
assert resp.status_code not in (401, 403), (
|
||||
f'{method} {path} wrongly blocked a developer with {resp.status_code}'
|
||||
)
|
||||
|
||||
|
||||
def test_queue_group_mutations_are_owner_scoped(app, client, db_session):
|
||||
"""queue_bus cross-user fix: `_ensure_group_mutable` alone only blocked
|
||||
*system* groups — any authenticated user could rename/delete another
|
||||
user's groups, queues and messages. Every mutating route now also calls
|
||||
`_ensure_group_accessible` (owner-or-admin)."""
|
||||
from app.queue_bus.service import QueueBusService
|
||||
|
||||
owner = make_user(db_session, role='developer')
|
||||
intruder = make_user(db_session, role='developer')
|
||||
group = QueueBusService.create_group(
|
||||
slug='owned-group', name='Owned', owner_type='user', owner_id=str(owner.id),
|
||||
)
|
||||
slug = group['slug'] if isinstance(group, dict) else group.slug
|
||||
|
||||
headers = headers_for(intruder)
|
||||
attempts = [
|
||||
('PATCH', f'/api/v1/queue/groups/{slug}'),
|
||||
('DELETE', f'/api/v1/queue/groups/{slug}'),
|
||||
('POST', f'/api/v1/queue/groups/{slug}/queues'),
|
||||
('POST', f'/api/v1/queue/groups/{slug}/queues/q1/messages'),
|
||||
]
|
||||
for method, url in attempts:
|
||||
resp = client.open(url, method=method, headers=headers,
|
||||
json={'name': 'x', 'payload': {}})
|
||||
assert resp.status_code == 403, (
|
||||
f'{method} {url} let a non-owner through with {resp.status_code}'
|
||||
)
|
||||
|
||||
# The owner themselves is NOT blocked by the accessibility guard.
|
||||
resp = client.open(f'/api/v1/queue/groups/{slug}', method='PATCH',
|
||||
headers=headers_for(owner), json={'description': 'mine'})
|
||||
assert resp.status_code != 403
|
||||
|
||||
|
||||
# --- Part B: the default-deny regression net -------------------------------
|
||||
|
||||
# Endpoints where a viewer legitimately receives a 2xx today. Each is reviewed
|
||||
# and categorized. Adding a NEW mutating route that answers a viewer with a 2xx
|
||||
# will fail the sweep below until it is gated or deliberately listed here.
|
||||
VIEWER_WRITABLE = {
|
||||
# SELF — the viewer acts only on their own account/data.
|
||||
'ai.create_conversation',
|
||||
'auth.update_current_user',
|
||||
'notifications.mark_inbox_all_read',
|
||||
'notifications.test_user_notification',
|
||||
'notifications.unmute_own_email',
|
||||
'two_factor.initiate_2fa_setup',
|
||||
'mobile.execute_quick_action',
|
||||
# STATELESS — parse/validate/dry-run helpers; no persisted mutation.
|
||||
'cron.preview_schedule',
|
||||
'dns_cutover.ttl_guidance',
|
||||
'firewall.rule_removal_preflight',
|
||||
'nginx_advanced.test_config',
|
||||
# READ-VIA-POST — a read the permission model already grants a viewer
|
||||
# (viewer has docker:read). POST only because it takes a body of ids.
|
||||
'docker.get_containers_stats',
|
||||
# PUBLIC transport — agent long-poll fallback; user JWT is ignored here.
|
||||
'agent_poll.disconnect',
|
||||
}
|
||||
|
||||
# Endpoints skipped by the live-fire net because firing them reaches out to the
|
||||
# network / external providers (slow, flaky in CI). Their gating is asserted by
|
||||
# dedicated per-feature tests, not here.
|
||||
_SKIP_ENDPOINTS = {
|
||||
'servers.check_agent_version',
|
||||
}
|
||||
|
||||
|
||||
def _mutating_rules(app):
|
||||
for rule in app.url_map.iter_rules():
|
||||
methods = rule.methods - {'GET', 'HEAD', 'OPTIONS'}
|
||||
if not methods:
|
||||
continue
|
||||
if rule.endpoint in _SKIP_ENDPOINTS:
|
||||
continue
|
||||
method = 'POST' if 'POST' in methods else sorted(methods)[0]
|
||||
yield rule, method
|
||||
|
||||
|
||||
def _fill(rule):
|
||||
"""Concrete URL for a rule, dummy values for path params."""
|
||||
url = rule.rule
|
||||
for arg in rule.arguments:
|
||||
conv = type(rule._converters.get(arg)).__name__
|
||||
val = '1' if 'Integer' in conv or 'Float' in conv else 'x'
|
||||
for token in (f'<{arg}>', f'<int:{arg}>', f'<string:{arg}>',
|
||||
f'<path:{arg}>', f'<float:{arg}>', f'<default:{arg}>'):
|
||||
url = url.replace(token, val)
|
||||
return url
|
||||
|
||||
|
||||
def test_no_mutating_route_succeeds_for_a_viewer(app, client, db_session):
|
||||
"""No mutating route may answer a viewer with a 2xx unless allowlisted.
|
||||
|
||||
This is the default-deny backstop. A 2xx here means the authorization gate
|
||||
definitively did NOT stop a read-only user — the exact shape of the
|
||||
workspace advisory. Non-2xx (401/403/400/404/5xx) is fine: either the gate
|
||||
refused, or the body rejected the dummy payload before mutating anything.
|
||||
"""
|
||||
headers = headers_for(make_user(db_session, role='viewer'))
|
||||
offenders = []
|
||||
for rule, method in _mutating_rules(app):
|
||||
url = _fill(rule)
|
||||
try:
|
||||
resp = client.open(url, method=method, headers=headers, json={})
|
||||
code = resp.status_code
|
||||
except Exception:
|
||||
# A crash in the view body is not a viewer succeeding — the gate
|
||||
# question is answered "not 2xx". Real gating bugs surface as 2xx.
|
||||
continue
|
||||
if 200 <= code < 300 and rule.endpoint not in VIEWER_WRITABLE:
|
||||
offenders.append(f'{method} {url} ({rule.endpoint}) -> {code}')
|
||||
|
||||
assert not offenders, (
|
||||
'A read-only viewer received a success response on mutating '
|
||||
'route(s) with no authorization gate:\n ' + '\n '.join(sorted(offenders))
|
||||
+ '\n\nGate the route (a decorator or an inline role/ownership check), '
|
||||
'or — if a viewer is genuinely allowed — add the endpoint to '
|
||||
'VIEWER_WRITABLE with a category comment.'
|
||||
)
|
||||
|
||||
|
||||
def test_viewer_writable_allowlist_is_not_stale(app):
|
||||
"""Every allowlisted endpoint must still exist — stale entries hide gaps.
|
||||
|
||||
If a route is renamed/removed and its VIEWER_WRITABLE entry is left behind,
|
||||
a future viewer-writable route could reuse a nearby name and slip through.
|
||||
"""
|
||||
live = {r.endpoint for r in app.url_map.iter_rules()}
|
||||
stale = (VIEWER_WRITABLE | _SKIP_ENDPOINTS) - live
|
||||
assert not stale, f'Allowlist references endpoints that no longer exist: {sorted(stale)}'
|
||||
@@ -246,3 +246,58 @@ def test_delivery_replay_requires_owner_role(client, vault_rbac, monkeypatch):
|
||||
assert client.post(url, headers=vault_rbac.ws_owner).status_code == 200
|
||||
assert client.post('/api/v1/webhooks/deliveries/999999/replay',
|
||||
headers=s.admin).status_code == 404
|
||||
|
||||
|
||||
# ------------------- inbound receiver (public, signature-authenticated) ----
|
||||
|
||||
def _sign(secret, payload):
|
||||
import hashlib
|
||||
import hmac
|
||||
return 'sha256=' + hmac.new(secret.encode(), payload, hashlib.sha256).hexdigest()
|
||||
|
||||
|
||||
def test_inbound_webhook_without_a_signature_is_rejected(client, vault_rbac, monkeypatch):
|
||||
"""The receiver is unauthenticated by design — the HMAC signature IS the
|
||||
credential — so an *absent* signature has to fail exactly like a wrong one.
|
||||
|
||||
It did not: the reject branch read ``if not signature_valid and
|
||||
signature_header``, which only fires for a request that bothered to sign.
|
||||
Sending no ``X-Hub-Signature-256`` at all skipped the branch entirely and
|
||||
the payload was logged, filtered and forwarded to ``forward_url``.
|
||||
"""
|
||||
from app.services.webhook_gateway_service import WebhookGatewayService
|
||||
forwarded = []
|
||||
monkeypatch.setattr(WebhookGatewayService, '_forward',
|
||||
classmethod(lambda cls, e, d, p, h: forwarded.append(p) or
|
||||
{'success': True, 'status': 200, 'body': ''}))
|
||||
|
||||
resp = client.post('/api/v1/webhooks/receive/scope-endpoint',
|
||||
data=b'{"a": 1}', content_type='application/json')
|
||||
assert resp.status_code == 401
|
||||
assert resp.get_json()['error'] == 'Missing signature'
|
||||
assert forwarded == [], 'an unsigned payload was forwarded upstream'
|
||||
|
||||
|
||||
def test_inbound_webhook_with_a_wrong_signature_is_rejected(client, vault_rbac):
|
||||
resp = client.post('/api/v1/webhooks/receive/scope-endpoint',
|
||||
data=b'{"a": 1}', content_type='application/json',
|
||||
headers={'X-Hub-Signature-256': _sign('not-the-secret', b'{"a": 1}')})
|
||||
assert resp.status_code == 401
|
||||
assert resp.get_json()['error'] == 'Invalid signature'
|
||||
|
||||
|
||||
def test_inbound_webhook_with_a_valid_signature_is_accepted(client, vault_rbac, monkeypatch):
|
||||
"""Non-vacuity for the two rejections: a correctly signed delivery still
|
||||
reaches the forwarder."""
|
||||
from app.services.webhook_gateway_service import WebhookGatewayService
|
||||
forwarded = []
|
||||
monkeypatch.setattr(WebhookGatewayService, '_forward',
|
||||
classmethod(lambda cls, e, d, p, h: forwarded.append(p) or
|
||||
{'success': True, 'status': 200, 'body': ''}))
|
||||
|
||||
payload = b'{"a": 1}'
|
||||
resp = client.post('/api/v1/webhooks/receive/scope-endpoint',
|
||||
data=payload, content_type='application/json',
|
||||
headers={'X-Hub-Signature-256': _sign('whsec', payload)})
|
||||
assert resp.status_code == 200
|
||||
assert forwarded == [payload]
|
||||
@@ -413,3 +413,28 @@ def test_audit_rows_written_on_mutations(app):
|
||||
assert {'tag.add', 'tag.remove', 'group.create', 'variable.create',
|
||||
'group.attach', 'group.detach', 'group.delete'} <= ops
|
||||
assert AuditLog.query.count() > before
|
||||
|
||||
|
||||
# ------------------------------------------------- tag write authorization
|
||||
|
||||
def test_api_tag_write_is_authorized_not_just_authenticated(app, client, db_session):
|
||||
"""Tagging is a mutation on someone else's resource, so it needs the same
|
||||
``_check_resource_write`` gate as attach/detach.
|
||||
|
||||
``POST``/``DELETE /shared/tags`` shipped behind ``@jwt_required()`` alone:
|
||||
any authenticated account could label any resource of any type. ``service``
|
||||
is the sharpest case — it has no ownership seam, so the gate resolves it to
|
||||
admin-only and a developer must be refused.
|
||||
"""
|
||||
from factories import make_user, headers_for
|
||||
|
||||
dev = headers_for(make_user(db_session, role='developer'))
|
||||
admin = headers_for(make_user(db_session, role='admin'))
|
||||
body = {'resource_type': 'service', 'resource_id': 'web-1', 'tag': 'prod'}
|
||||
|
||||
assert client.post('/api/v1/shared/tags', json=body, headers=dev).status_code == 403
|
||||
assert client.delete('/api/v1/shared/tags', json=body, headers=dev).status_code == 403
|
||||
|
||||
# Non-vacuity: the same calls succeed for a caller the gate admits.
|
||||
assert client.post('/api/v1/shared/tags', json=body, headers=admin).status_code == 201
|
||||
assert client.delete('/api/v1/shared/tags', json=body, headers=admin).status_code == 200
|
||||
@@ -509,3 +509,22 @@ def test_list_workspaces_surfaces_effective_role(app, client):
|
||||
rows = {w['name']: w for w in r.get_json()['workspaces']}
|
||||
assert rows['LW']['my_role'] == 'viewer'
|
||||
assert rows['LW']['my_effective_role'] == 'viewer'
|
||||
|
||||
|
||||
def test_create_workspace_requires_admin(app, client):
|
||||
"""GHSA-r4q7-x795-vr4j: POST /api/v1/workspaces/ was gated only by
|
||||
@jwt_required(), so a read-only viewer could create a workspace, become its
|
||||
owner, then add arbitrary members and mint workspace API keys. It now
|
||||
carries @admin_required like every other panel-wide create."""
|
||||
from app import db
|
||||
|
||||
for role in ('viewer', 'developer'):
|
||||
u = _mk_user(db, f'cw_{role}', role)
|
||||
r = client.post('/api/v1/workspaces/', headers=_token(u.id),
|
||||
json={'name': f'cw-{role}', 'slug': f'cw-{role}'})
|
||||
assert r.status_code == 403, f'{role} reached workspace creation: {r.status_code}'
|
||||
|
||||
admin = _mk_user(db, 'cw_admin', 'admin')
|
||||
r = client.post('/api/v1/workspaces/', headers=_token(admin.id),
|
||||
json={'name': 'cw-ok', 'slug': 'cw-ok'})
|
||||
assert r.status_code == 201
|
||||
@@ -13,12 +13,12 @@ debt. Regenerate with `python scripts/generate-migration-inventory.py`.
|
||||
| Concern | Door | Remaining | Ceiling | Policy |
|
||||
|---|---|---:|---:|---|
|
||||
| Identity lookups outside rbac.get_current_user() | rbac.get_current_user() | 69 | 69 | JWT-only routes; the API-key-capable population is held at 0 by a companion assertion |
|
||||
| Routes on bare @jwt_required() | auth_required() / role decorators | 605 | 605 | REGISTERED EXCEPTION: JWT-only is the deliberate default; conversion grants API-key access and happens per route, on decision |
|
||||
| Routes on bare @jwt_required() | auth_required() / role decorators | 577 | 577 | REGISTERED EXCEPTION: JWT-only is the deliberate default; conversion grants API-key access and happens per route, on decision |
|
||||
| HTTP statuses chosen by sniffing error text | typed errors from app.exceptions | 0 | 0 | INVARIANT at 0 - migration completed 2026-08-19 |
|
||||
| API crashes swallowed without recording | app.error_reporting | 0 | 0 | INVARIANT at 0 |
|
||||
| Hand-shaped {'error': ...} bodies in app/api | typed errors + the global handler | 1159 | 1159 | migrate when touched; new endpoints raise |
|
||||
| Hand-shaped {'error': ...} bodies in app/api | typed errors + the global handler | 1158 | 1159 | migrate when touched; new endpoints raise |
|
||||
| Raw subprocess calls outside the runners | app/utils/system.py runners | 24 | 24 | migrate when touched |
|
||||
| Controller-boundary violations (routes doing service work) | service layer extraction | 510 | 510 | migrate when touched (first-wave ratchet) |
|
||||
| Controller-boundary violations (routes doing service work) | service layer extraction | 509 | 509 | migrate when touched (first-wave ratchet) |
|
||||
| raw api.* calls in pages/ | E1: useServerQuery/useServerMutation | 460 | 460 | migrate when touched |
|
||||
| per-page toast.error extractions in pages/ | E1: query-layer error presentation | 264 | 264 | migrate when touched |
|
||||
| hand-rolled form-group blocks | F2: FormField/useForm | 343 | 343 | migrate when touched |
|
||||
|
||||
@@ -12,6 +12,7 @@ import { Button } from '@/components/ui/button';
|
||||
import { Input } from '@/components/ui/input';
|
||||
import { Textarea } from '@/components/ui/textarea';
|
||||
import { useWorkspace } from '../contexts/WorkspaceContext';
|
||||
import { useAuth } from '../contexts/AuthContext';
|
||||
import { useServerMutation, useServerQuery } from '../hooks/useServerQuery';
|
||||
|
||||
// Preset views. `servers` and `users` are quota CEILINGS, not usage, so there
|
||||
@@ -84,11 +85,14 @@ const Workspaces = () => {
|
||||
const toast = useToast();
|
||||
const navigate = useNavigate();
|
||||
const { activeWorkspaceId } = useWorkspace();
|
||||
// Workspace creation is admin-only server-side (@admin_required); hide the
|
||||
// entry points for everyone else instead of letting them hit a 403.
|
||||
const { isAdmin } = useAuth();
|
||||
const [search, setSearch] = useState('');
|
||||
const [selectedIds, setSelectedIds] = useState(new Set());
|
||||
const [showCreateModal, setShowCreateModal] = useState(false);
|
||||
// Quick-create deep link: /workspaces?focus=create:workspace opens the modal.
|
||||
useFocusParam('create', () => setShowCreateModal(true));
|
||||
useFocusParam('create', () => { if (isAdmin) setShowCreateModal(true); });
|
||||
const [form, setForm] = useState({ name: '', description: '', max_servers: 0, max_users: 0, primary_color: '#6d7cff' });
|
||||
|
||||
const loadWorkspaces = useCallback(
|
||||
@@ -109,17 +113,19 @@ const Workspaces = () => {
|
||||
|
||||
useTopbarActions(() => (
|
||||
<>
|
||||
<Button size="sm" onClick={() => setShowCreateModal(true)}>
|
||||
<Plus size={16} />
|
||||
New Workspace
|
||||
</Button>
|
||||
{isAdmin && (
|
||||
<Button size="sm" onClick={() => setShowCreateModal(true)}>
|
||||
<Plus size={16} />
|
||||
New Workspace
|
||||
</Button>
|
||||
)}
|
||||
<SearchField
|
||||
value={search}
|
||||
onSearch={setSearch}
|
||||
placeholder="Search workspaces…"
|
||||
/>
|
||||
</>
|
||||
), [search]);
|
||||
), [search, isAdmin]);
|
||||
|
||||
const handleCreate = async () => {
|
||||
try {
|
||||
@@ -239,12 +245,14 @@ const Workspaces = () => {
|
||||
onClearSelection={() => setSelectedIds(new Set())}
|
||||
emptyIcon={LayoutGrid}
|
||||
emptyTitle="No workspaces yet"
|
||||
emptyDescription="Create one to isolate servers by team or project."
|
||||
emptyAction={
|
||||
emptyDescription={isAdmin
|
||||
? 'Create one to isolate servers by team or project.'
|
||||
: 'Workspaces isolate servers by team or project. Ask an admin to create one.'}
|
||||
emptyAction={isAdmin ? (
|
||||
<Button onClick={() => setShowCreateModal(true)}>
|
||||
New Workspace
|
||||
</Button>
|
||||
}
|
||||
) : null}
|
||||
filteredEmptyIcon={LayoutGrid}
|
||||
filteredEmptyTitle="No workspaces found"
|
||||
filteredEmptyDescription="Try adjusting your search or filter."
|
||||
|
||||
Reference in new issue
Block a user