Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions agent/.env.example
Original file line number Diff line number Diff line change
Expand Up @@ -29,5 +29,10 @@ BRAVE_API_KEY=
GITHUB_TOKEN=
GITHUB_ORG=

# === Auth & CORS ===
VIBEDEPLOY_API_KEY=
VIBEDEPLOY_OPS_TOKEN=
VIBEDEPLOY_CORS_ORIGINS=http://localhost:3000

# === Database ===
DATABASE_URL=
152 changes: 152 additions & 0 deletions agent/auth.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,152 @@
"""Authentication and rate-limiting middleware for vibeDeploy gateway."""

import hmac
import logging
import os
import time
from collections import defaultdict

from fastapi import HTTPException, Request, Security
from fastapi.security import APIKeyHeader

logger = logging.getLogger(__name__)

_api_key_header = APIKeyHeader(name="X-API-Key", auto_error=False)

_PUBLIC_PATHS: frozenset[str] = frozenset(
{
"/",
"/health",
"/cost-estimate",
"/api/cost-estimate",
"/models",
"/api/models",
}
)

_PUBLIC_PREFIXES: tuple[str, ...] = ("/test/",)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The /test/ prefix is included in _PUBLIC_PREFIXES, which bypasses authentication for all matching routes. While these routes are also protected by the VIBEDEPLOY_ENABLE_TEST_API environment variable, it is a security best practice to require the API key even for test endpoints if one is configured. This prevents unauthorized access or state modification in environments where the test API might be accidentally enabled.



def _get_api_key() -> str:
for key in ("VIBEDEPLOY_API_KEY", "VIBEDEPLOY_OPS_TOKEN", "DASHBOARD_ADMIN_TOKEN"):
value = os.getenv(key, "").strip()
if value:
return value
return ""
Comment on lines +29 to +34
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

โš ๏ธ Potential issue | ๐ŸŸ  Major

VIBEDEPLOY_API_KEY๊ฐ€ ์—†์„ ๋•Œ auth๊ฐ€ ๊บผ์ง€์ง€ ์•Š์Šต๋‹ˆ๋‹ค.

PR ๋ชฉํ‘œ๋Œ€๋กœ๋ผ๋ฉด VIBEDEPLOY_API_KEY ๋ฏธ์„ค์ • ์‹œ ์ธ์ฆ์ด ๋น„ํ™œ์„ฑํ™”๋ผ์•ผ ํ•˜๋Š”๋ฐ, ์ง€๊ธˆ ๊ตฌํ˜„์€ VIBEDEPLOY_OPS_TOKEN์ด๋‚˜ DASHBOARD_ADMIN_TOKEN๋งŒ ๋‚จ์•„ ์žˆ์–ด๋„ ๊ณ„์† X-API-Key ๊ฒ€์ฆ์„ ๊ฐ•์ œํ•ฉ๋‹ˆ๋‹ค. ์ด๋ ‡๊ฒŒ ๋‘๋ฉด zero-downtime rollout ์ค‘ ๊ธฐ์กด ํ™˜๊ฒฝ์—์„œ ์˜ˆ์ƒ์น˜ ๋ชปํ•œ 401/403๊ฐ€ ๋‚  ์ˆ˜ ์žˆ๊ณ , ์„œ๋กœ ๋‹ค๋ฅธ ์šฉ๋„์˜ ์‹œํฌ๋ฆฟ์„ ๊ฐ™์€ ์ธ์ฆ ๊ฒฝ๊ณ„๋กœ ์„ž๊ฒŒ ๋ฉ๋‹ˆ๋‹ค.

์ˆ˜์ • ์˜ˆ์‹œ
 def _get_api_key() -> str:
-    for key in ("VIBEDEPLOY_API_KEY", "VIBEDEPLOY_OPS_TOKEN", "DASHBOARD_ADMIN_TOKEN"):
-        value = os.getenv(key, "").strip()
-        if value:
-            return value
-    return ""
+    return os.getenv("VIBEDEPLOY_API_KEY", "").strip()
๐Ÿค– Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@agent/auth.py` around lines 30 - 35, The current auth gating uses any of
VIBEDEPLOY_API_KEY, VIBEDEPLOY_OPS_TOKEN, or DASHBOARD_ADMIN_TOKEN from
_get_api_key(), which forces X-API-Key validation even when the primary
VIBEDEPLOY_API_KEY is unset; change the logic so authentication is only enabled
when VIBEDEPLOY_API_KEY is explicitly setโ€”either by updating _get_api_key() to
return a value only when os.getenv("VIBEDEPLOY_API_KEY") is present, or by
changing the auth check to specifically test for VIBEDEPLOY_API_KEY (keep
VIBEDEPLOY_OPS_TOKEN/DASHBOARD_ADMIN_TOKEN for other internal uses but do not
use them to enable X-API-Key validation); ensure references to _get_api_key and
the middleware/function that enforces X-API-Key are updated accordingly.



def _is_public_path(path: str) -> bool:
if path in _PUBLIC_PATHS:
return True
return any(path.startswith(prefix) for prefix in _PUBLIC_PREFIXES)


async def verify_api_key(
request: Request,
api_key: str | None = Security(_api_key_header),
) -> str | None:
path = request.url.path

if _is_public_path(path):
return None

expected = _get_api_key()

# Auth disabled when no key is configured (dev mode)
if not expected:
return None

# SSE endpoints: fall back to query param (EventSource can't send headers)
if not api_key and "/events" in path:
api_key = request.query_params.get("api_key")

if not api_key:
raise HTTPException(
status_code=401,
detail="missing_api_key",
headers={"WWW-Authenticate": "ApiKey"},
)

if not hmac.compare_digest(api_key, expected):
logger.warning("Invalid API key from %s for %s", request.client.host if request.client else "unknown", path)
raise HTTPException(status_code=403, detail="invalid_api_key")

return api_key


class _RateLimitBucket:
__slots__ = ("_requests",)

def __init__(self) -> None:
self._requests: list[float] = []

def hit(self, now: float, window_seconds: int, max_requests: int) -> bool:
cutoff = now - window_seconds
self._requests = [t for t in self._requests if t > cutoff]
if len(self._requests) >= max_requests:
return False
self._requests.append(now)
return True


_rate_buckets: defaultdict[str, _RateLimitBucket] = defaultdict(_RateLimitBucket)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The _rate_buckets dictionary is a global defaultdict that stores rate-limiting state in memory. There is currently no mechanism to prune or expire old entries. If the service is exposed to the internet and receives requests from many unique IP addresses, this dictionary will grow indefinitely, potentially leading to a memory leak and Out of Memory (OOM) errors.


_RATE_LIMITS: dict[str, tuple[int, int]] = {
"write": (10, 60),
"read": (120, 60),
"sse": (20, 60),
}

_WRITE_PATHS: frozenset[str] = frozenset(
{
"/run",
"/api/run",
"/resume",
"/api/resume",
"/brainstorm",
"/api/brainstorm",
"/zero-prompt/start",
"/api/zero-prompt/start",
"/zero-prompt/reset",
"/api/zero-prompt/reset",
}
)

_SSE_FRAGMENTS: tuple[str, ...] = ("/events", "/build/")


def _classify_rate_tier(path: str, method: str) -> str:
if path in _WRITE_PATHS:
return "write"
for fragment in _SSE_FRAGMENTS:
if fragment in path:
return "sse"
if method == "POST" and "/actions" in path:
return "write"
return "read"


async def rate_limit_check(request: Request) -> None:
path = request.url.path

if _is_public_path(path):
return

client_ip = request.client.host if request.client else "unknown"
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Using request.client.host retrieves the IP address of the immediate network peer. If the application is deployed behind a load balancer or reverse proxy (common in cloud environments like DigitalOcean App Platform), this will return the proxy's IP rather than the actual user's IP. This causes all users to share the same rate-limit bucket. It is recommended to check the X-Forwarded-For header.

Suggested change
client_ip = request.client.host if request.client else "unknown"
forwarded = request.headers.get("x-forwarded-for")
client_ip = forwarded.split(",")[0].strip() if forwarded else (request.client.host if request.client else "unknown")

method = request.method
tier = _classify_rate_tier(path, method)
max_requests, window_seconds = _RATE_LIMITS[tier]
bucket_key = f"{client_ip}:{tier}"
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

โš ๏ธ Potential issue | ๐ŸŸ  Major

๐Ÿงฉ Analysis chain

๐Ÿ Script executed:

# First, let's look at the auth.py file to understand the context
cat -n agent/auth.py | head -160 | tail -20

Repository: Two-Weeks-Team/vibeDeploy

Length of output: 765


๐Ÿ Script executed:

# Check if there's a .do/app.yaml file
find . -name "app.yaml" -o -name ".do" -type d

Repository: Two-Weeks-Team/vibeDeploy

Length of output: 89


๐Ÿ Script executed:

# Search for any Uvicorn configuration in the codebase
rg -i "uvicorn|proxy|trusted" --type py -A 3

Repository: Two-Weeks-Team/vibeDeploy

Length of output: 12541


๐Ÿ Script executed:

# Search for any usage of do-connecting-ip or similar headers
rg -i "do-connecting-ip|forwarded-allow" --type py -A 2

Repository: Two-Weeks-Team/vibeDeploy

Length of output: 51


๐Ÿ Script executed:

# Check pyproject.toml for Uvicorn or proxy-related settings
cat pyproject.toml | grep -i -A 5 -B 5 "uvicorn\|proxy\|trusted"

Repository: Two-Weeks-Team/vibeDeploy

Length of output: 115


๐Ÿ Script executed:

# Check the .do/app.yaml deployment configuration
cat ./.do/app.yaml

Repository: Two-Weeks-Team/vibeDeploy

Length of output: 3333


๐Ÿ Script executed:

# Check the Uvicorn startup code in run_server.py
cat -n agent/run_server.py

Repository: Two-Weeks-Team/vibeDeploy

Length of output: 962


๐Ÿ Script executed:

# Check the Uvicorn startup code in server.py
cat -n agent/server.py | head -50

Repository: Two-Weeks-Team/vibeDeploy

Length of output: 2007


๐Ÿ Script executed:

# Search for any middleware that might handle headers or proxy settings
rg -i "middleware|header|forwarded|proxy" agent/ --type py -B 2 -A 2

Repository: Two-Weeks-Team/vibeDeploy

Length of output: 43174


๐Ÿ Script executed:

# Search for any request.headers or custom header handling
rg "request\.headers|do-connecting|X-Forwarded" agent/ --type py -A 3

Repository: Two-Weeks-Team/vibeDeploy

Length of output: 51


๐ŸŒ Web query:

DigitalOcean App Platform client IP header do-connecting-ip documentation

๐Ÿ’ก Result:

DigitalOcean App Platform sets a request header named do-connecting-ip that contains the original client IP address. By contrast, x-forwarded-for on App Platform is used for the DigitalOcean ingress server IP that forwarded the request to your app. [1]

Sources:
[1] DigitalOcean Docs โ€” โ€œWhere can I find the client IP address of a request connecting to my app?โ€


DigitalOcean App Platform ํ™˜๊ฒฝ์—์„œ ์š”์ฒญ ํด๋ผ์ด์–ธํŠธ IP ์ž๋™ ๊ฐ์ง€๋ฅผ ์˜ฌ๋ฐ”๋ฅด๊ฒŒ ์ฒ˜๋ฆฌํ•˜์ง€ ์•Š์œผ๋ฉด ์‹ค์ œ ์‚ฌ์šฉ์ž๋ณ„์ด ์•„๋‹Œ ingress IP๋ณ„๋กœ rate limit์ด ์ ์šฉ๋ฉ๋‹ˆ๋‹ค.

ํ˜„์žฌ ์ฝ”๋“œ๋Š” request.client.host๋ฅผ ์‚ฌ์šฉํ•˜๊ณ  ์žˆ๋Š”๋ฐ, Uvicorn์˜ ๊ธฐ๋ณธ ์„ค์ •์—์„œ๋Š” trusted proxy๋ฅผ ๋ช…์‹œ์ ์œผ๋กœ ๊ตฌ์„ฑํ•˜์ง€ ์•Š์€ ์ƒํƒœ์ž…๋‹ˆ๋‹ค. DigitalOcean App Platform์—์„œ๋Š” ์‹ค์ œ ํด๋ผ์ด์–ธํŠธ IP๋ฅผ do-connecting-ip ํ—ค๋”์— ๋‹ด์•„ ์ „๋‹ฌํ•˜๋ฉฐ, X-Forwarded-For๋Š” ingress ์„œ๋ฒ„ IP๋ฅผ ํฌํ•จํ•ฉ๋‹ˆ๋‹ค. ๋”ฐ๋ผ์„œ ํ˜„์žฌ ์„ค์ •์—์„œ๋Š” request.client.host๊ฐ€ DigitalOcean์˜ ๊ณต์œ  ingress IP๋ฅผ ๋ฐ˜ํ™˜ํ•˜๊ฒŒ ๋˜๊ณ , ์ด๋กœ ์ธํ•ด ๋™์ผํ•œ ingress ๋’ค์— ์žˆ๋Š” ๋ชจ๋“  ์‚ฌ์šฉ์ž๊ฐ€ ํ•˜๋‚˜์˜ rate limit bucket์„ ๊ณต์œ ํ•˜๊ฒŒ ๋ฉ๋‹ˆ๋‹ค. ๊ฒฐ๊ณผ์ ์œผ๋กœ ํ•œ ์‚ฌ์šฉ์ž์˜ ์ •์ƒ ์š”์ฒญ์ด ๋‹ค๋ฅธ ์‚ฌ์šฉ์ž๋“ค์˜ ํŠธ๋ž˜ํ”ฝ์œผ๋กœ ์ธํ•ด ์˜๋„์น˜ ์•Š์€ 429 ์˜ค๋ฅ˜๋ฅผ ๋ฐ›์„ ์ˆ˜ ์žˆ์Šต๋‹ˆ๋‹ค.

DigitalOcean App Platform ์ „์šฉ ๋ฐฐํฌ์ธ ๋งŒํผ do-connecting-ip ํ—ค๋”๋ฅผ ์šฐ์„ ์ ์œผ๋กœ ์‚ฌ์šฉํ•˜๊ฑฐ๋‚˜, Uvicorn์˜ trusted proxy ์„ค์ •์„ ๋ช…์‹œ์ ์œผ๋กœ ๊ตฌ์„ฑํ•˜์—ฌ ์‹ค์ œ ํด๋ผ์ด์–ธํŠธ IP๋ฅผ ์ •ํ™•ํžˆ ๊ฐ์ง€ํ•˜๋Š” ๊ฒƒ์ด ํ•„์ˆ˜์ž…๋‹ˆ๋‹ค.

๐Ÿค– Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@agent/auth.py` around lines 150 - 154, The rate-limiting currently derives
client_ip from request.client.host (see client_ip and request.client.host) which
on DigitalOcean returns the ingress IP; update the logic in the handler that
builds bucket_key to first check the do-connecting-ip header (and then
X-Forwarded-For fallback), parse/normalize the header to extract the left-most
real client IP, and only fall back to request.client.host if neither header is
present; keep using _classify_rate_tier and _RATE_LIMITS but ensure bucket_key
uses the resolved client IP so per-user buckets are correct.

now = time.monotonic()

if not _rate_buckets[bucket_key].hit(now, window_seconds, max_requests):
logger.warning("Rate limit exceeded: %s %s from %s (tier=%s)", method, path, client_ip, tier)
raise HTTPException(
status_code=429,
detail="rate_limit_exceeded",
headers={
"Retry-After": str(window_seconds),
"X-RateLimit-Limit": str(max_requests),
"X-RateLimit-Window": str(window_seconds),
},
)
29 changes: 23 additions & 6 deletions agent/server.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
import httpx
import uvicorn
from dotenv import load_dotenv
from fastapi import FastAPI, Header, HTTPException, Request
from fastapi import Depends, FastAPI, Header, HTTPException, Request
from fastapi.middleware.cors import CORSMiddleware
from pydantic import BaseModel
from starlette.responses import JSONResponse, StreamingResponse
Expand Down Expand Up @@ -346,7 +346,7 @@ def _meeting_store_payload(meeting: dict) -> dict:


def _ops_token() -> str:
for key in ("VIBEDEPLOY_OPS_TOKEN", "DASHBOARD_ADMIN_TOKEN", "DIGITALOCEAN_INFERENCE_KEY"):
for key in ("VIBEDEPLOY_OPS_TOKEN", "DASHBOARD_ADMIN_TOKEN"):
value = os.getenv(key, "").strip()
if value:
return value
Expand Down Expand Up @@ -610,13 +610,30 @@ async def lifespan(app: FastAPI):
_store = None


app = FastAPI(title="vibeDeploy Agent (local)", lifespan=lifespan)
from .auth import rate_limit_check, verify_api_key

app = FastAPI(
title="vibeDeploy Agent (local)",
lifespan=lifespan,
dependencies=[Depends(verify_api_key), Depends(rate_limit_check)],
)

_ALLOWED_ORIGINS = [
origin.strip()
for origin in os.getenv(
"VIBEDEPLOY_CORS_ORIGINS",
"https://vibedeploy-7tgzk.ondigitalocean.app,http://localhost:3000,http://localhost:9001",
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The default value for VIBEDEPLOY_CORS_ORIGINS includes a specific DigitalOcean application URL. Hardcoding environment-specific infrastructure details as defaults in the source code is discouraged. It is better to use a generic default (like localhost) and rely on environment variables for production configuration.

Suggested change
"https://vibedeploy-7tgzk.ondigitalocean.app,http://localhost:3000,http://localhost:9001",
"http://localhost:3000,http://localhost:9001",

).split(",")
if origin.strip()
]

app.add_middleware(
CORSMiddleware,
allow_origins=["*"],
allow_methods=["*"],
allow_headers=["*"],
allow_origins=_ALLOWED_ORIGINS,
allow_methods=["GET", "POST", "PUT", "OPTIONS"],
allow_headers=["Content-Type", "X-API-Key", "X-Vibedeploy-Ops-Token"],
allow_credentials=False,
max_age=600,
)

_NODE_EVENTS = NODE_EVENTS
Expand Down
9 changes: 9 additions & 0 deletions agent/tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,18 @@
import pytest_asyncio
from httpx import ASGITransport, AsyncClient

from agent.auth import _rate_buckets
from agent.db.store import ResultStore


@pytest.fixture(autouse=True)
def _clear_rate_limits():
"""Clear rate limit buckets before each test to avoid cross-test pollution."""
_rate_buckets.clear()
yield
_rate_buckets.clear()


@pytest_asyncio.fixture
async def store() -> AsyncIterator[ResultStore]:
s = ResultStore(":memory:")
Expand Down
Loading
Loading