-
Notifications
You must be signed in to change notification settings - Fork 93
fix: route himalaya chat commands to device backend #818
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,69 @@ | ||
| # SPDX-FileCopyrightText: 2025 Weibo, Inc. | ||
| # | ||
| # SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| """Internal device APIs for service-to-service communication.""" | ||
|
|
||
| from fastapi import APIRouter, Depends, HTTPException, status | ||
| from pydantic import BaseModel, Field | ||
| from sqlalchemy.orm import Session | ||
|
|
||
| from app.api.dependencies import get_db | ||
| from app.api.endpoints.devices import DeviceSandboxExecResponse | ||
|
|
||
| router = APIRouter(prefix="/devices", tags=["internal-devices"]) | ||
|
|
||
|
|
||
| class InternalDeviceSandboxExecRequest(BaseModel): | ||
| """Internal request model for device-backed command execution.""" | ||
|
|
||
| user_id: int = Field(..., ge=1, description="Owner user ID") | ||
| command: str = Field(..., min_length=1, description="Command to execute") | ||
| working_dir: str = Field( | ||
| default="/home/user", | ||
| description="Working directory for command execution", | ||
| ) | ||
| timeout_seconds: int = Field( | ||
| default=300, | ||
| ge=1, | ||
| le=1800, | ||
| description="Command timeout in seconds", | ||
| ) | ||
| required_capability: str | None = Field( | ||
| default=None, | ||
| description="Optional device capability required for routing", | ||
| ) | ||
| device_id: str | None = Field( | ||
| default=None, | ||
| description="Optional explicit device ID override", | ||
| ) | ||
|
|
||
|
|
||
| @router.post("/sandbox/exec", response_model=DeviceSandboxExecResponse) | ||
| async def execute_device_sandbox_command_internal( | ||
| request: InternalDeviceSandboxExecRequest, | ||
| db: Session = Depends(get_db), | ||
| ) -> DeviceSandboxExecResponse: | ||
| """Execute a command on a user's device for internal trusted services.""" | ||
|
Comment on lines
+155
to
+159
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion | 🟠 Major Add OpenTelemetry tracing on the async endpoint handler. Line 43 defines an async API handler without the required trace decorator/span metadata, which leaves this execution path under-instrumented. As per coding guidelines, "Use 🧰 Tools🪛 Ruff (0.15.6)[warning] 45-45: Do not perform function call (B008) 🤖 Prompt for AI Agents |
||
| from app.services.device_sandbox_service import ( | ||
| DeviceSandboxError, | ||
| device_sandbox_service, | ||
| ) | ||
|
|
||
| try: | ||
| result = await device_sandbox_service.execute_command( | ||
| db=db, | ||
| user_id=request.user_id, | ||
| command=request.command, | ||
| working_dir=request.working_dir, | ||
| timeout_seconds=request.timeout_seconds, | ||
| required_capability=request.required_capability, | ||
| device_id=request.device_id, | ||
| ) | ||
| except DeviceSandboxError as exc: | ||
| raise HTTPException( | ||
| status_code=status.HTTP_400_BAD_REQUEST, | ||
| detail=str(exc), | ||
| ) from exc | ||
|
|
||
| return DeviceSandboxExecResponse(**result) | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Protect this internal exec endpoint with explicit internal authz/authn.
Line 42 exposes remote command execution with caller-controlled
request.user_id. Combined withbackend/app/api/api.py(Lines 178-181) including this router without route-level dependencies, this is a privilege-escalation path if/internalis reachable. Add a strict internal-service auth dependency and enforce impersonation policy before executing.🔐 Example hardening sketch
🧰 Tools
🪛 Ruff (0.15.6)
[warning] 45-45: Do not perform function call
Dependsin argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable(B008)
🤖 Prompt for AI Agents