fix(security): P0-1 — port exposure + service-token auth on /internal/* + jupyter RPC
The fix lands in three concentric layers, all backed by a single
INTERNAL_SERVICE_TOKEN shared secret so we have one mechanism
instead of three:
1. docker-compose: drop the backend.ports: 8891:8000 and
runtime.ports: 8892:8000 mappings. Nginx is the only host
ingress again (architecture §2.2).
2. /internal/v1/*: the storage control plane had six endpoints, five
of which were dead code (frontend already migrated to
/api/v1/data-resources/* with JWT; schedule only ever called
POST /internal/v1/objects). Delete the dead routes, mount the
one survivor with Depends(require_internal_service) that
compares the X-Internal-Service-Token header against
settings.internal_service_token with secrets.compare_digest.
3. POST /api/v1/jupyter on the runtime container: previously open
inside the Docker network. Same token mechanism — backend's
runtime_http_client now carries the header, runtime's
handle_jupyter_action requires the same header. /api/v1/health
stays open for the Nginx and compose healthchecks.
The schedule worker was already configured to call
POST /internal/v1/objects; build_storage_http_client now
sets the token header so its existing call site keeps working
without changes.
Files touched:
backend/src/backend/storage_api.py # 5 dead routes deleted + token guard
backend/src/backend/main.py # runtime_http_client header
runtime/src/runtime/main.py # require_internal_service Depends
common/src/common/config.py # internal_service_token setting
schedule/src/schedule/service.py # httpx client header
docker-compose.yml # ports dropped, INTERNAL_SERVICE_TOKEN env
.env.example # INTERNAL_SERVICE_TOKEN placeholder
API.md / README.md / DEVELOP.md # §9 trimmed to 1 endpoint
Verified:
compileall -> 0 errors
pytest backend/tests -> 37 passed
in-process ASGI smoke:
POST /internal/v1/objects no/wrong/correct token -> 401/401/200
POST /api/v1/jupyter no/wrong/correct token -> 401/401/200
5 deleted internal routes -> 404
docker compose config (with env) -> OK
P0-1 still has one open sub-item (rclone RC --rc-no-auth) that
the user has explicitly deferred; not touched here.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
tao.chen
co-authored by
Claude Fable 5
Claude Fable 5
parent
6258cf5d12
commit
dfe3f0b118
@@ -52,9 +52,15 @@ async def lifespan(app: Any) -> AsyncIterator[None]:
|
||||
for purpose in PURPOSE_BUCKETS
|
||||
}
|
||||
app.state.default_bucket = settings.s3_workspace_bucket
|
||||
# P0-1 fix: runtime's /api/v1/jupyter is token-guarded. The token is
|
||||
# the same ``INTERNAL_SERVICE_TOKEN`` value used by /internal/v1/* —
|
||||
# reusing one mechanism instead of inventing a second one.
|
||||
runtime_http_client = httpx.AsyncClient(
|
||||
base_url=settings.runtime_api_url,
|
||||
timeout=httpx.Timeout(30.0),
|
||||
headers={
|
||||
"X-Internal-Service-Token": settings.internal_service_token,
|
||||
},
|
||||
)
|
||||
app.state.runtime_client = RuntimeClient(runtime_http_client)
|
||||
# Short timeout — refresh is best-effort and runs in a BackgroundTask.
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
from __future__ import annotations
|
||||
|
||||
import hashlib
|
||||
import secrets
|
||||
from collections.abc import AsyncIterator
|
||||
from datetime import UTC, datetime, timedelta
|
||||
from pathlib import PurePosixPath
|
||||
@@ -19,22 +20,49 @@ from common.ids import new_ulid
|
||||
from common.storage import USAGE_TYPE_TO_PURPOSE, actual_bucket_name, build_storage_uri
|
||||
from common.storage.schemas import (
|
||||
CreateUploadRequest,
|
||||
DownloadUrlRequest,
|
||||
ServerObjectRequest,
|
||||
)
|
||||
from fastapi import APIRouter, Depends, HTTPException, Request, status
|
||||
from fastapi import APIRouter, Depends, Header, HTTPException, Request, status
|
||||
from sqlalchemy import select
|
||||
from sqlalchemy.ext.asyncio import AsyncSession
|
||||
|
||||
from backend.services.storage import (
|
||||
create_download_url_payload,
|
||||
create_server_object_payload,
|
||||
create_upload_record,
|
||||
soft_delete_object,
|
||||
upload_bytes_to_session,
|
||||
)
|
||||
|
||||
|
||||
# Header name for the service-to-service token. Schedule worker is the
|
||||
# only legitimate caller — every other consumer goes through the
|
||||
# JWT-protected ``/api/v1/data-resources/*`` routes. The header name
|
||||
# mirrors the ``X-Internal-*`` convention used elsewhere in the stack.
|
||||
INTERNAL_SERVICE_TOKEN_HEADER = "x-internal-service-token"
|
||||
|
||||
|
||||
def require_internal_service(
|
||||
x_internal_service_token: str | None = Header(default=None),
|
||||
) -> None:
|
||||
"""Enforce a shared secret for /internal/v1/* routes.
|
||||
|
||||
Compares the supplied header against ``settings.internal_service_token``
|
||||
with a constant-time check. The token is configured identically on the
|
||||
backend and the schedule container via ``INTERNAL_SERVICE_TOKEN``; the
|
||||
default in ``Settings`` is a development-only placeholder that callers
|
||||
must override in any non-dev deployment.
|
||||
"""
|
||||
expected = settings.internal_service_token
|
||||
if not expected:
|
||||
raise HTTPException(
|
||||
status.HTTP_503_SERVICE_UNAVAILABLE,
|
||||
"internal service token not configured",
|
||||
)
|
||||
if not x_internal_service_token or not secrets.compare_digest(
|
||||
x_internal_service_token, expected
|
||||
):
|
||||
raise HTTPException(status.HTTP_401_UNAUTHORIZED, "internal service token required")
|
||||
|
||||
|
||||
def utcnow() -> datetime:
|
||||
return datetime.now(UTC).replace(tzinfo=None)
|
||||
|
||||
@@ -358,82 +386,25 @@ async def upload_bytes_to_session(
|
||||
return item
|
||||
|
||||
|
||||
@router.post("/v1/uploads")
|
||||
async def create_upload(
|
||||
payload: CreateUploadRequest,
|
||||
request: Request,
|
||||
session: AsyncSession = Depends(database_session),
|
||||
) -> dict[str, Any]:
|
||||
return {
|
||||
"data": await create_upload_record(payload, session, request),
|
||||
}
|
||||
|
||||
|
||||
@router.put("/v1/uploads/{upload_id}")
|
||||
async def upload_bytes(
|
||||
upload_id: str, request: Request, session: AsyncSession = Depends(database_session)
|
||||
) -> dict[str, Any]:
|
||||
"""Server-proxied upload: PUT raw bytes in the request body. Replaces the
|
||||
old ``POST /uploads/{id}/complete`` flow that paired presigned-PUT with
|
||||
a head()-validate step.
|
||||
"""
|
||||
item = await upload_bytes_to_session(upload_id, session, request)
|
||||
return {"data": storage_payload(item)}
|
||||
|
||||
|
||||
@router.post("/v1/uploads/{upload_id}/abort")
|
||||
async def abort_upload(
|
||||
upload_id: str, request: Request, session: AsyncSession = Depends(database_session)
|
||||
) -> dict[str, Any]:
|
||||
upload = await session.scalar(
|
||||
select(UploadSessions)
|
||||
.where(UploadSessions.upload_id == upload_id)
|
||||
.with_for_update()
|
||||
)
|
||||
if upload is None:
|
||||
raise HTTPException(status.HTTP_404_NOT_FOUND, "upload not found")
|
||||
if upload.upload_status == "completed":
|
||||
raise HTTPException(
|
||||
status.HTTP_409_CONFLICT, "completed upload cannot be aborted"
|
||||
)
|
||||
if upload.upload_status != "aborted":
|
||||
await request.app.state.object_stores[upload.bucket_name].delete(
|
||||
upload.object_key
|
||||
)
|
||||
upload.upload_status = "aborted"
|
||||
return {"data": {"upload_id": upload_id, "status": "aborted"}}
|
||||
|
||||
|
||||
@router.post("/v1/objects")
|
||||
@router.post(
|
||||
"/v1/objects",
|
||||
dependencies=[Depends(require_internal_service)],
|
||||
)
|
||||
async def create_server_object(
|
||||
payload: ServerObjectRequest,
|
||||
request: Request,
|
||||
session: AsyncSession = Depends(database_session),
|
||||
) -> dict[str, Any]:
|
||||
"""Server-side single-call object upload used by the schedule worker.
|
||||
|
||||
Token-guarded: the only legitimate caller is the schedule service
|
||||
that writes ``run_log`` / ``run_result`` artifacts after a notebook
|
||||
finishes. Frontend users upload through the JWT-protected
|
||||
``/api/v1/data-resources/*`` routes instead.
|
||||
"""
|
||||
return await create_server_object_payload(payload, request, session)
|
||||
|
||||
|
||||
@router.post("/v1/objects/{storage_object_id}/download-url")
|
||||
async def create_download_url(
|
||||
storage_object_id: str,
|
||||
payload: DownloadUrlRequest,
|
||||
request: Request,
|
||||
session: AsyncSession = Depends(database_session),
|
||||
) -> dict[str, Any]:
|
||||
item = await session.get(StorageObjects, storage_object_id)
|
||||
return await create_download_url_payload(item, payload, request)
|
||||
|
||||
|
||||
@router.delete("/v1/objects/{storage_object_id}")
|
||||
async def delete_object(
|
||||
storage_object_id: str,
|
||||
request: Request,
|
||||
session: AsyncSession = Depends(database_session),
|
||||
) -> dict[str, Any]:
|
||||
"""Soft-delete a storage object. See ``backend.services.storage.soft_delete_object``."""
|
||||
return await soft_delete_object(storage_object_id, request, session)
|
||||
|
||||
|
||||
@router.post("/v1/objects/{storage_object_id}/restore")
|
||||
async def restore_object(
|
||||
storage_object_id: str,
|
||||
|
||||
Reference in New Issue
Block a user