Conversation
…ting - Create agent/auth.py with X-API-Key header verification and per-IP sliding-window rate limiter (write 10/min, read 120/min, SSE 20/min) - Fix CORS from allow_origins=["*"] to env-configurable origin list - Remove DIGITALOCEAN_INFERENCE_KEY fallback from ops token chain - Add FastAPI app-level Depends for auth and rate limiting - Create web/src/lib/fetch-with-auth.ts wrapper for authenticated requests - Update all frontend API calls (api.ts, zero-prompt-api.ts, dashboard-api.ts, sse-client.ts) to use authenticatedFetch/authHeaders - Add SSE query param fallback for EventSource (can't send headers) - Auth disabled when VIBEDEPLOY_API_KEY not set (dev mode, zero-downtime deploy) - 32 new auth unit tests, all existing 1376 tests pass Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Walkthrough에이전트 백엔드에 API 키 기반 인증과 IP/티어별 인메모리 레이트 제한을 추가하고 CORS를 제한적으로 재설정했으며, 웹 프론트엔드의 모든 백엔드 호출을 인증된 fetch/헤더 쪽으로 통합했습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as 클라이언트
participant Frontend as Frontend 모듈
participant FetchAuth as fetch-with-auth
participant BrowserFetch as 브라우저 fetch
participant Server as FastAPI 서버
participant AuthModule as 인증 모듈
participant RateLimiter as 레이트 리미터
Client->>Frontend: API 호출 (e.g., getResult)
Frontend->>FetchAuth: authenticatedFetch(url, init)
FetchAuth->>FetchAuth: authHeaders() 생성 (X-API-Key 조건부)
FetchAuth->>FetchAuth: appendApiKey() (SSE URL인 경우)
FetchAuth->>BrowserFetch: fetch(url, mergedInit)
BrowserFetch->>Server: HTTP 요청 (헤더 또는 쿼리의 api_key)
Server->>AuthModule: verify_api_key(request)
AuthModule-->>Server: 인증결과 또는 HTTPException(401/403)
Server->>RateLimiter: rate_limit_check(request)
RateLimiter-->>Server: 허용 또는 HTTPException(429)
Server->>Server: 라우트 핸들러 실행
Server-->>BrowserFetch: 응답
BrowserFetch-->>Frontend: Response 반환
sequenceDiagram
participant Client as 이벤트 클라이언트
participant Frontend as ZeroPrompt Hook
participant FetchAuth as fetch-with-auth
participant BrowserFetch as 브라우저 fetch
participant Server as FastAPI 서버
participant AuthModule as 인증 모듈
Client->>Frontend: SSE 연결 요청
Frontend->>FetchAuth: appendApiKey(baseUrl)
FetchAuth->>FrontEnd: eventsUrl (api_key 쿼리 포함)
Frontend->>BrowserFetch: fetch(eventsUrl, { method: "GET", headers: authHeaders() })
BrowserFetch->>Server: SSE 연결 (쿼리/헤더에 키)
Server->>AuthModule: verify_api_key (SSE용 쿼리 허용)
AuthModule-->>Server: 인증 성공/실패
Server-->>BrowserFetch: 이벤트 스트리밍 응답
BrowserFetch-->>Frontend: 스트림 수신
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements authentication and rate-limiting middleware for the vibeDeploy gateway, including API key verification and tiered rate limits for read, write, and SSE operations. The frontend has been updated to use a new authenticatedFetch utility to include necessary credentials. Review feedback identifies a significant issue where non-public environment variables will be evaluated as empty strings in the browser, breaking client-side requests. Other identified improvements include addressing a potential memory leak in the rate-limiting state, correctly identifying client IPs behind proxies using the X-Forwarded-For header, and refining header merging logic in the frontend fetch wrapper.
| * This key must NEVER be exposed to the browser (no NEXT_PUBLIC_ prefix). | ||
| */ | ||
|
|
||
| const API_KEY = process.env.VIBEDEPLOY_API_KEY ?? ""; |
There was a problem hiding this comment.
There is a contradiction between the security goal and the implementation. The comment states the API key must never be exposed to the browser, and thus it uses an environment variable without the NEXT_PUBLIC_ prefix. However, this file is imported by Client Components and hooks (e.g., usePipelineMonitor).
In Next.js, non-public environment variables are evaluated as empty strings in the browser. This means all client-side requests will be sent with an empty X-API-Key header, causing authentication to fail on the backend. If the client must call the backend directly, the key must be available to it. If the key must remain secret, the application should use a proxy (BFF pattern) via Next.js API routes or Server Actions.
| } | ||
| ) | ||
|
|
||
| _PUBLIC_PREFIXES: tuple[str, ...] = ("/test/",) |
There was a problem hiding this comment.
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.
agent/auth.py
Outdated
| return True | ||
|
|
||
|
|
||
| _rate_buckets: defaultdict[str, _RateLimitBucket] = defaultdict(_RateLimitBucket) |
There was a problem hiding this comment.
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.
| if _is_public_path(path): | ||
| return | ||
|
|
||
| client_ip = request.client.host if request.client else "unknown" |
There was a problem hiding this comment.
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.
| 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") |
| origin.strip() | ||
| for origin in os.getenv( | ||
| "VIBEDEPLOY_CORS_ORIGINS", | ||
| "https://vibedeploy-7tgzk.ondigitalocean.app,http://localhost:3000,http://localhost:9001", |
There was a problem hiding this comment.
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.
| "https://vibedeploy-7tgzk.ondigitalocean.app,http://localhost:3000,http://localhost:9001", | |
| "http://localhost:3000,http://localhost:9001", |
| export async function authenticatedFetch(url: string, init?: RequestInit): Promise<Response> { | ||
| const merged: RequestInit = { | ||
| ...init, | ||
| headers: { | ||
| ...authHeaders(), | ||
| ...(init?.headers as Record<string, string> | undefined), | ||
| }, | ||
| }; | ||
| return fetch(url, merged); | ||
| } |
There was a problem hiding this comment.
The authenticatedFetch function does not correctly merge headers if init.headers is provided as a Headers object or an array of entries, as the spread operator is only effective for plain objects. Additionally, the type cast to Record<string, string> is unsafe. Using the Headers constructor is a more robust way to merge headers.
export async function authenticatedFetch(url: string, init?: RequestInit): Promise<Response> {
const headers = new Headers(authHeaders());
if (init?.headers) {
const incoming = new Headers(init.headers);
incoming.forEach((value, key) => headers.set(key, value));
}
return fetch(url, { ...init, headers });
}Review fixes for Phase 1 PR #219: - Add VIBEDEPLOY_API_KEY to both api and web services in .do/app.yaml - Add VIBEDEPLOY_CORS_ORIGINS to api service - Fix rate_buckets unbounded growth: periodic cleanup of stale entries every 300s, explicit bucket creation instead of defaultdict Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
agent/tests/test_auth.py (1)
187-201:_rate_buckets.clear()호출이 중복됨.Line 190의
_rate_buckets.clear()는conftest.py의autouse=Truefixture가 이미 각 테스트 전에 버킷을 정리하므로 불필요합니다. 제거해도 무방하지만, 테스트 의도를 명확히 하기 위해 남겨둘 수도 있습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@agent/tests/test_auth.py` around lines 187 - 201, The test test_blocks_after_limit contains a redundant call to _rate_buckets.clear() because the autouse fixture in conftest.py already clears buckets before each test; remove the duplicate _rate_buckets.clear() invocation in test_blocks_after_limit (or if you prefer to keep it for explicitness add a comment explaining it's redundant) so the test relies on the fixture and avoids unnecessary duplication.web/src/lib/fetch-with-auth.ts (1)
21-30:Headers인스턴스 처리 시 타입 캐스팅 주의.
init?.headers가Headers인스턴스일 경우,Record<string, string>으로 타입 캐스팅하면 실제로 spread가 제대로 동작하지 않습니다.Headers객체는 iterable이지만 plain object spread로는 프로퍼티가 복사되지 않습니다.현재 프로젝트에서
Headers인스턴스를 직접 전달하는 경우가 없다면 문제없지만, 방어적 코딩을 원한다면 다음과 같이 수정할 수 있습니다:♻️ 개선 제안
export async function authenticatedFetch(url: string, init?: RequestInit): Promise<Response> { + const incomingHeaders = init?.headers instanceof Headers + ? Object.fromEntries(init.headers.entries()) + : (init?.headers as Record<string, string> | undefined); const merged: RequestInit = { ...init, headers: { ...authHeaders(), - ...(init?.headers as Record<string, string> | undefined), + ...incomingHeaders, }, }; return fetch(url, merged); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/lib/fetch-with-auth.ts` around lines 21 - 30, authenticatedFetch currently casts init?.headers to Record<string,string>, which fails when init.headers is a Headers instance; update authenticatedFetch to robustly merge headers by creating a Headers container (e.g., new Headers(authHeaders())), then if init?.headers is a Headers iterate and set each entry, else if it's an array or plain object add those entries, and finally pass that Headers instance in the merged RequestInit; reference the authenticatedFetch function and the authHeaders() call when implementing the defensive merging logic.agent/auth.py (1)
92-98: 인메모리 rate limiting의 한계점 인지 필요.
_rate_buckets는 인메모리defaultdict로 구현되어 있어 다음과 같은 제약이 있습니다:
- 서버가 수평 확장(multiple instances)될 경우 각 인스턴스가 독립적인 버킷을 가지므로 rate limiting이 제대로 동작하지 않음
- 시간이 지나면서 고유 IP:tier 조합이 누적되어 메모리 사용량이 증가할 수 있음
현재 단일 인스턴스 배포에서는 문제없지만, 향후 확장 시 Redis 기반 rate limiter로 전환을 고려해 주세요.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@agent/auth.py` around lines 92 - 98, The in-memory _rate_buckets defaultdict using _RateLimitBucket and _RATE_LIMITS won't work when scaling or over long runtimes; replace the in-memory bucket store with a Redis-backed implementation (e.g., create a RedisRateLimitBucket class that implements the same interface as _RateLimitBucket and uses atomic Redis ops/INCR+EXPIRE or Lua scripts to enforce counts and windows) and swap use of _rate_buckets to a factory that returns RedisRateLimitBucket instances keyed by IP:tier; ensure keys have a TTL to avoid unbounded memory growth and that the implementation is safe for multi-instance enforcement.web/src/hooks/use-zero-prompt.ts (1)
375-383: URL 쿼리 파라미터와 헤더에 API 키가 중복 적용됨.
appendApiKey(baseUrl)로 URL에api_key를 추가하고,authHeaders()로 헤더에X-API-Key를 추가하여 동일한 API 키가 두 곳에 전달됩니다.agent/auth.py가 둘 다 허용하므로 기능상 문제는 없지만, 하나만 사용하면 더 명확합니다.GET 기반 SSE이므로
appendApiKey만 사용해도 충분합니다.♻️ 선택적 개선: 중복 인증 제거
const baseUrl = eventSessionId ? `${DASHBOARD_API_URL}/zero-prompt/events?session_id=${encodeURIComponent(eventSessionId)}` : `${DASHBOARD_API_URL}/zero-prompt/events`; const eventsUrl = appendApiKey(baseUrl); const res = await fetch(eventsUrl, { signal: controller.signal, - headers: authHeaders(), });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/hooks/use-zero-prompt.ts` around lines 375 - 383, The fetch call is sending the API key twice (both in the URL via appendApiKey and in headers via authHeaders); for this SSE GET request keep only URL-based auth: remove the headers: authHeaders() usage from the fetch options and rely on eventsUrl produced by appendApiKey (refer to appendApiKey, authHeaders, eventsUrl, fetch(..., { signal: controller.signal, ... }) and the eventSessionId/DASHBOARD_API_URL construction to locate the code).web/src/lib/__tests__/fetch-with-auth.test.ts (1)
67-85:authenticatedFetch테스트에서X-API-Key헤더 검증 누락.
VIBEDEPLOY_API_KEY가 설정된 상태에서 테스트하지만,X-API-Key헤더가 실제로 전달되는지 검증하지 않습니다.Content-Type만 확인하고 있어 인증 헤더 주입의 핵심 기능이 테스트되지 않습니다.💚 X-API-Key 검증 추가
expect(mockFetch).toHaveBeenCalledTimes(1); const [url, init] = mockFetch.mock.calls[0]; expect(url).toBe("http://example.com/api/run"); expect(init.method).toBe("POST"); expect(init.headers["Content-Type"]).toBe("application/json"); + expect(init.headers["X-API-Key"]).toBe(MOCK_API_KEY); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/lib/__tests__/fetch-with-auth.test.ts` around lines 67 - 85, The test for authenticatedFetch is missing an assertion that the X-API-Key header is sent; update the test in the describe("authenticatedFetch") block to assert that the mockFetch call's init.headers includes "X-API-Key" equal to MOCK_API_KEY (or the env value set via vi.stubEnv("VIBEDEPLOY_API_KEY")), while keeping the existing checks for url, method and Content-Type; locate the authenticatedFetch import and mockFetch usage in the test to add this header assertion.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@agent/server.py`:
- Around line 613-614: The import of rate_limit_check and verify_api_key is
placed after function definitions, causing an E402 lint error; move the from
.auth import rate_limit_check, verify_api_key statement up into the module
import block with the other top-level imports (near the existing imports at the
top of the file) and remove the late import line so the symbols are imported
only once at module load time.
---
Nitpick comments:
In `@agent/auth.py`:
- Around line 92-98: The in-memory _rate_buckets defaultdict using
_RateLimitBucket and _RATE_LIMITS won't work when scaling or over long runtimes;
replace the in-memory bucket store with a Redis-backed implementation (e.g.,
create a RedisRateLimitBucket class that implements the same interface as
_RateLimitBucket and uses atomic Redis ops/INCR+EXPIRE or Lua scripts to enforce
counts and windows) and swap use of _rate_buckets to a factory that returns
RedisRateLimitBucket instances keyed by IP:tier; ensure keys have a TTL to avoid
unbounded memory growth and that the implementation is safe for multi-instance
enforcement.
In `@agent/tests/test_auth.py`:
- Around line 187-201: The test test_blocks_after_limit contains a redundant
call to _rate_buckets.clear() because the autouse fixture in conftest.py already
clears buckets before each test; remove the duplicate _rate_buckets.clear()
invocation in test_blocks_after_limit (or if you prefer to keep it for
explicitness add a comment explaining it's redundant) so the test relies on the
fixture and avoids unnecessary duplication.
In `@web/src/hooks/use-zero-prompt.ts`:
- Around line 375-383: The fetch call is sending the API key twice (both in the
URL via appendApiKey and in headers via authHeaders); for this SSE GET request
keep only URL-based auth: remove the headers: authHeaders() usage from the fetch
options and rely on eventsUrl produced by appendApiKey (refer to appendApiKey,
authHeaders, eventsUrl, fetch(..., { signal: controller.signal, ... }) and the
eventSessionId/DASHBOARD_API_URL construction to locate the code).
In `@web/src/lib/__tests__/fetch-with-auth.test.ts`:
- Around line 67-85: The test for authenticatedFetch is missing an assertion
that the X-API-Key header is sent; update the test in the
describe("authenticatedFetch") block to assert that the mockFetch call's
init.headers includes "X-API-Key" equal to MOCK_API_KEY (or the env value set
via vi.stubEnv("VIBEDEPLOY_API_KEY")), while keeping the existing checks for
url, method and Content-Type; locate the authenticatedFetch import and mockFetch
usage in the test to add this header assertion.
In `@web/src/lib/fetch-with-auth.ts`:
- Around line 21-30: authenticatedFetch currently casts init?.headers to
Record<string,string>, which fails when init.headers is a Headers instance;
update authenticatedFetch to robustly merge headers by creating a Headers
container (e.g., new Headers(authHeaders())), then if init?.headers is a Headers
iterate and set each entry, else if it's an array or plain object add those
entries, and finally pass that Headers instance in the merged RequestInit;
reference the authenticatedFetch function and the authHeaders() call when
implementing the defensive merging logic.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: adc20a84-12c8-441b-8411-fad1f9bcc1ee
📒 Files selected for processing (16)
agent/.env.exampleagent/auth.pyagent/server.pyagent/tests/conftest.pyagent/tests/test_auth.pyagent/tests/test_dashboard_snapshot.pyweb/src/app/zero-prompt/page.tsxweb/src/hooks/use-pipeline-monitor.tsweb/src/hooks/use-zero-prompt.tsweb/src/lib/__tests__/api.test.tsweb/src/lib/__tests__/fetch-with-auth.test.tsweb/src/lib/api.tsweb/src/lib/dashboard-api.tsweb/src/lib/fetch-with-auth.tsweb/src/lib/sse-client.tsweb/src/lib/zero-prompt-api.ts
| from .auth import rate_limit_check, verify_api_key | ||
|
|
There was a problem hiding this comment.
E402 린트 오류: 모듈 레벨 import가 파일 상단에 위치하지 않음.
파이프라인 실패 로그에 따르면, line 613의 import 문이 파일 상단이 아닌 함수 정의 이후에 위치하여 ruff check가 실패합니다. Import 문을 파일 상단의 다른 import 문들과 함께 이동해야 합니다.
🔧 수정 제안
Line 30 근처의 다른 import 문들 사이에 추가:
from fastapi import Depends, FastAPI, Header, HTTPException, Request
from fastapi.middleware.cors import CORSMiddleware
+from .auth import rate_limit_check, verify_api_key
from pydantic import BaseModel그리고 line 613-614를 삭제:
-from .auth import rate_limit_check, verify_api_key
-
app = FastAPI(📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| from .auth import rate_limit_check, verify_api_key | |
| from fastapi import Depends, FastAPI, Header, HTTPException, Request | |
| from fastapi.middleware.cors import CORSMiddleware | |
| from .auth import rate_limit_check, verify_api_key | |
| from pydantic import BaseModel | |
| # ... other imports ... |
| from .auth import rate_limit_check, verify_api_key | |
| app = FastAPI( |
🧰 Tools
🪛 GitHub Actions: CI
[error] 613-613: ruff check failed: E402 Module level import not at top of file. Import shown at server.py:613.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@agent/server.py` around lines 613 - 614, The import of rate_limit_check and
verify_api_key is placed after function definitions, causing an E402 lint error;
move the from .auth import rate_limit_check, verify_api_key statement up into
the module import block with the other top-level imports (near the existing
imports at the top of the file) and remove the late import line so the symbols
are imported only once at module load time.
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.do/app.yaml:
- Around line 129-132: Client hooks use-pipeline-monitor.ts and
use-zero-prompt.ts import authenticatedFetch()/appendApiKey() from
fetch-with-auth.ts but rely on a runtime secret VIBEDEPLOY_API_KEY that is not
exposed to the browser, causing undefined API key and 401s; update these client
hooks to call a server-side proxy/API route instead of using
appendApiKey()/authHeaders() on the client: create or reuse a Next.js API route
(or server action) that reads process.env.VIBEDEPLOY_API_KEY and performs the
authenticated fetch server-side, then have use-pipeline-monitor.ts and
use-zero-prompt.ts call that API route (or a server wrapper) rather than
importing authenticatedFetch()/appendApiKey(); you can reference
getInitialSession() in zero-prompt/page.tsx as an example of server-side usage.
In `@agent/auth.py`:
- Around line 134-141: _cleanup_stale_buckets currently removes buckets based
only on b.is_empty(), which never becomes true for IPs that stop sending
requests; update the cleanup to use the current now timestamp to prune/expire
buckets: either add/maintain a last_seen timestamp on each bucket (update in
hit()) and remove any bucket whose last_seen is older than the rate window, or
call a new/prune method on the bucket (e.g., b.prune(now) or
b.expire(oldest_allowed)) before checking is_empty(); ensure you still honor
_BUCKET_CLEANUP_INTERVAL and update _last_bucket_cleanup when cleanup runs.
- Around line 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.
- Line 7: Remove the unused import "defaultdict" from the top of agent/auth.py
by deleting or changing the import line "from collections import defaultdict" so
the file no longer imports an unused symbol (this will resolve the ruff F401
lint error); after editing, run ruff check and ruff format --check on
agent/**/*.py to verify no remaining unused-import violations.
- Around line 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.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 86da1179-d8bb-47f5-8b91-06b40ee39c0e
📒 Files selected for processing (2)
.do/app.yamlagent/auth.py
| - key: VIBEDEPLOY_API_KEY | ||
| scope: RUN_TIME | ||
| type: SECRET | ||
| value: REPLACE_WITH_API_KEY |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
git ls-files | head -20Repository: Two-Weeks-Team/vibeDeploy
Length of output: 471
🏁 Script executed:
fd "app.yaml" --type fRepository: Two-Weeks-Team/vibeDeploy
Length of output: 51
🏁 Script executed:
fd "fetch-with-auth" --type fRepository: Two-Weeks-Team/vibeDeploy
Length of output: 145
🏁 Script executed:
find . -name ".do" -type dRepository: Two-Weeks-Team/vibeDeploy
Length of output: 74
🏁 Script executed:
cat -n .do/app.yamlRepository: Two-Weeks-Team/vibeDeploy
Length of output: 4257
🏁 Script executed:
cat -n web/src/lib/fetch-with-auth.tsRepository: Two-Weeks-Team/vibeDeploy
Length of output: 1361
🏁 Script executed:
cat -n web/src/lib/__tests__/fetch-with-auth.test.tsRepository: Two-Weeks-Team/vibeDeploy
Length of output: 3924
🏁 Script executed:
rg "fetch-with-auth" --type ts --type tsx -A 2 -B 2Repository: Two-Weeks-Team/vibeDeploy
Length of output: 96
🏁 Script executed:
rg "authHeaders|authenticatedFetch|appendApiKey" --type ts --type tsx -A 2 -B 2 | head -100Repository: Two-Weeks-Team/vibeDeploy
Length of output: 96
🏁 Script executed:
fd --type f -name "*.ts" -o -name "*.tsx" | xargs grep -l "fetch-with-auth" 2>/dev/nullRepository: Two-Weeks-Team/vibeDeploy
Length of output: 301
🏁 Script executed:
rg "from.*fetch-with-auth|import.*fetch-with-auth" --type tsRepository: Two-Weeks-Team/vibeDeploy
Length of output: 1743
🏁 Script executed:
rg "import.*authHeaders|import.*authenticatedFetch" --type tsRepository: Two-Weeks-Team/vibeDeploy
Length of output: 422
🏁 Script executed:
fd "\.tsx?" -path "*/web/*" | head -30Repository: Two-Weeks-Team/vibeDeploy
Length of output: 237
🏁 Script executed:
find web -name "*.ts" -o -name "*.tsx" | xargs grep -l "fetch-with-auth" 2>/dev/nullRepository: Two-Weeks-Team/vibeDeploy
Length of output: 323
🏁 Script executed:
cat -n web/src/hooks/use-pipeline-monitor.tsRepository: Two-Weeks-Team/vibeDeploy
Length of output: 11593
🏁 Script executed:
cat -n web/src/hooks/use-zero-prompt.tsRepository: Two-Weeks-Team/vibeDeploy
Length of output: 27261
🏁 Script executed:
head -20 web/src/app/zero-prompt/page.tsxRepository: Two-Weeks-Team/vibeDeploy
Length of output: 912
🏁 Script executed:
cat -n web/src/lib/api.tsRepository: Two-Weeks-Team/vibeDeploy
Length of output: 4379
클라이언트 훅에서 인증되지 않은 API 호출이 발생합니다.
web/src/hooks/use-pipeline-monitor.ts와 web/src/hooks/use-zero-prompt.ts는 "use client" 마크를 가진 클라이언트 컴포넌트인데, 두 파일 모두 fetch-with-auth.ts에서 authenticatedFetch()와 appendApiKey()를 임포트하여 사용하고 있습니다. 하지만 .do/app.yaml 라인 129-132의 VIBEDEPLOY_API_KEY는 NEXT_PUBLIC_ 접두사 없이 RUN_TIME SECRET으로만 설정되어 있으므로, 브라우저 코드에서 process.env.VIBEDEPLOY_API_KEY는 항상 undefined가 되어 authHeaders()가 빈 헤더를, appendApiKey()가 수정되지 않은 URL을 반환합니다. 결과적으로 대부분의 클라이언트 요청이 401로 실패합니다.
올바른 해결책은 클라이언트 측 훅을 수정하여 인증을 위해 Next.js 서버 경유 프록시나 API 라우트 핸들러를 거쳐야 합니다. 서버 컴포넌트(예: zero-prompt/page.tsx의 getInitialSession())는 이미 올바르게 설정되어 있습니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.do/app.yaml around lines 129 - 132, Client hooks use-pipeline-monitor.ts
and use-zero-prompt.ts import authenticatedFetch()/appendApiKey() from
fetch-with-auth.ts but rely on a runtime secret VIBEDEPLOY_API_KEY that is not
exposed to the browser, causing undefined API key and 401s; update these client
hooks to call a server-side proxy/API route instead of using
appendApiKey()/authHeaders() on the client: create or reuse a Next.js API route
(or server action) that reads process.env.VIBEDEPLOY_API_KEY and performs the
authenticated fetch server-side, then have use-pipeline-monitor.ts and
use-zero-prompt.ts call that API route (or a server wrapper) rather than
importing authenticatedFetch()/appendApiKey(); you can reference
getInitialSession() in zero-prompt/page.tsx as an example of server-side usage.
| import logging | ||
| import os | ||
| import time | ||
| from collections import defaultdict |
There was a problem hiding this comment.
사용하지 않는 defaultdict import를 제거하세요.
현재 CI가 ruff F401로 깨지고 있습니다.
As per coding guidelines, agent/**/*.py: For Python linting in agent, use ruff check and ruff format --check
🧰 Tools
🪛 GitHub Actions: CI
[error] 7-7: ruff check: F401 collections.defaultdict imported but unused. Remove unused import: collections.defaultdict.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@agent/auth.py` at line 7, Remove the unused import "defaultdict" from the top
of agent/auth.py by deleting or changing the import line "from collections
import defaultdict" so the file no longer imports an unused symbol (this will
resolve the ruff F401 lint error); after editing, run ruff check and ruff format
--check on agent/**/*.py to verify no remaining unused-import violations.
| 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 "" |
There was a problem hiding this comment.
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 _cleanup_stale_buckets(now: float) -> None: | ||
| global _last_bucket_cleanup | ||
| if now - _last_bucket_cleanup < _BUCKET_CLEANUP_INTERVAL: | ||
| return | ||
| _last_bucket_cleanup = now | ||
| stale = [k for k, b in _rate_buckets.items() if b.is_empty()] | ||
| for k in stale: | ||
| _rate_buckets.pop(k, None) |
There was a problem hiding this comment.
stale bucket 정리가 실제로 동작하지 않습니다.
_cleanup_stale_buckets()는 is_empty()만 보고 삭제하는데, 오래된 타임스탬프는 해당 bucket이 다시 hit()될 때만 정리됩니다. 그래서 더 이상 요청이 오지 않는 IP bucket은 영원히 non-empty로 남고, 이번 PR이 의도한 unbounded growth 방지가 되지 않습니다. cleanup 시점에 now 기준으로 직접 prune하거나 last_seen을 저장해서 window 밖 bucket을 제거해야 합니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@agent/auth.py` around lines 134 - 141, _cleanup_stale_buckets currently
removes buckets based only on b.is_empty(), which never becomes true for IPs
that stop sending requests; update the cleanup to use the current now timestamp
to prune/expire buckets: either add/maintain a last_seen timestamp on each
bucket (update in hit()) and remove any bucket whose last_seen is older than the
rate window, or call a new/prune method on the bucket (e.g., b.prune(now) or
b.expire(oldest_allowed)) before checking is_empty(); ensure you still honor
_BUCKET_CLEANUP_INTERVAL and update _last_bucket_cleanup when cleanup runs.
| client_ip = 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}" |
There was a problem hiding this comment.
🧩 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 -20Repository: 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 dRepository: 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 3Repository: 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 2Repository: 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.yamlRepository: Two-Weeks-Team/vibeDeploy
Length of output: 3333
🏁 Script executed:
# Check the Uvicorn startup code in run_server.py
cat -n agent/run_server.pyRepository: Two-Weeks-Team/vibeDeploy
Length of output: 962
🏁 Script executed:
# Check the Uvicorn startup code in server.py
cat -n agent/server.py | head -50Repository: 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 2Repository: 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 3Repository: 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.
1 participant
Summary
agent/auth.pywith API Key authentication (X-API-Keyheader) and per-IP sliding-window rate limiterallow_origins=["*"]to env-configurable origin listDIGITALOCEAN_INFERENCE_KEYfallback from ops token chainweb/src/lib/fetch-with-auth.tswrapper for all frontend API callsChanges
agent/auth.py,web/src/lib/fetch-with-auth.ts, 2 test filesVIBEDEPLOY_API_KEYnot set (dev mode, zero-downtime deploy)Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit
새로운 기능
Chores
테스트