feat: Support specifying multiple apiKeys #144
Conversation
WalkthroughAdds API key authentication: new middleware extracts keys from Authorization Bearer and Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Server
participant Auth as API Key Auth Middleware
participant Handler as Route Handler
Client->>Server: HTTP Request
Server->>Auth: Invoke middleware
alt state.apiKeys configured
Auth->>Auth: Extract key (Authorization Bearer / x-api-key)
alt key valid
Auth->>Handler: forward request
Handler->>Client: 200 OK
else invalid/missing
Auth->>Client: 401 Unauthorized
end
else no apiKeys configured
Auth->>Handler: skip auth
Handler->>Client: 200 OK
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Comment |
![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 (1)
src/lib/api-key-auth.ts (1)
37-65: Consider constant-time comparison for API key validation.The middleware logic is well-structured and correctly handles missing and invalid keys. However, line 55 uses
Array.includes()for key validation, which is not constant-time and could theoretically be vulnerable to timing attacks.For enhanced security, consider using a constant-time comparison:
// Helper function for constant-time string comparison function constantTimeEqual(a: string, b: string): boolean { if (a.length !== b.length) { return false } let result = 0 for (let i = 0; i < a.length; i++) { result |= a.charCodeAt(i) ^ b.charCodeAt(i) } return result === 0 } // Then in the middleware: const isValidKey = state.apiKeys.some(key => constantTimeEqual(key, providedKey))Note: For typical API keys (UUIDs or long random strings), the timing attack risk is low, so this is an optional enhancement rather than a critical fix.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
README.md(3 hunks)src/lib/api-key-auth.ts(1 hunks)src/lib/state.ts(1 hunks)src/server.ts(2 hunks)src/start.ts(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
src/lib/api-key-auth.ts (1)
src/lib/state.ts (1)
state(23-28)
src/server.ts (1)
src/lib/api-key-auth.ts (1)
apiKeyAuthMiddleware(37-65)
src/start.ts (1)
src/lib/state.ts (1)
state(23-28)
🪛 LanguageTool
README.md
[style] ~166-~166: To form a complete sentence, be sure to include a subject.
Context: ...key | API keys for authentication. Can be specified multiple times ...
(MISSING_IT_THERE)
🔇 Additional comments (7)
src/lib/state.ts (1)
18-20: LGTM!The addition of the optional
apiKeysproperty to theStateinterface is clean and maintains backward compatibility when no API keys are configured.src/server.ts (1)
5-26: LGTM! Middleware integration is correct.The
apiKeyAuthMiddlewareis properly applied to all protected endpoints. The root endpoint remains public as intended, and the middleware coverage correctly includes both standard and v1-prefixed routes.README.md (1)
277-281: LGTM! Clear usage examples.The examples effectively demonstrate how to use the
--api-keyflag with both single and multiple keys.src/start.ts (3)
28-28: LGTM! Options interface updated correctly.The addition of the optional
apiKeysproperty toRunServerOptionsis consistent with the backward-compatible design.
50-56: LGTM! Clear initialization and logging.The state initialization and informational logging provide good user feedback about the authentication status.
195-211: LGTM! Robust CLI argument parsing.The implementation correctly handles citty's behavior of passing either a string (single value) or array (multiple values), ensuring the
--api-keyflag works as documented.src/lib/api-key-auth.ts (1)
11-31: Verified: Query parameter API key support is an intentional trade-off to enable browser-based Usage Viewer access.The security concern is legitimate. The
/usageendpoint is protected byapiKeyAuthMiddleware(src/server.ts line 21), which intentionally supports query parameters (src/lib/api-key-auth.ts comment: "for extra compatibility of/usageor/tokenroute"). This design enables the browser-based Usage Viewer documented in the README to fetch usage data from localhost, since external browser viewers cannot set custom headers due to CORS restrictions.However, this creates the security risks you identified:
- API keys appear in server logs and browser history
- Keys leak via Referer headers when navigating externally
- URL bar exposes credentials
Before committing to a mitigation approach, verify:
- Is the Usage Viewer expected to work with API key authentication enabled?
- Are there existing security controls (firewall, network isolation) limiting access to
/usage?- Is this endpoint intended to be public or accessed only from specific networks?
If the browser viewer must support API key auth, your three suggestions remain valid. If not, removing query parameter support eliminates the security gap.
| ## API Key Authentication | ||
|
|
||
| The proxy supports API key authentication to restrict access to the endpoints. When API keys are configured, all API endpoints require authentication. | ||
|
|
||
| ### Authentication Methods | ||
|
|
||
| The proxy supports both OpenAI and Anthropic authentication formats: | ||
|
|
||
| - **OpenAI format**: Include the API key in the `Authorization` header with `Bearer` prefix: | ||
| ```bash | ||
| curl -H "Authorization: Bearer your_api_key_here" http://localhost:4141/v1/models | ||
| ``` | ||
|
|
||
| - **Anthropic format**: Include the API key in the `x-api-key` header: | ||
| ```bash | ||
| curl -H "x-api-key: your_api_key_here" http://localhost:4141/v1/messages | ||
| ``` | ||
|
|
||
| ### Configuration | ||
|
|
||
| Use the `--api-key` flag to enable API key authentication. You can specify multiple keys for different clients: | ||
|
|
||
| ```bash | ||
| # Single API key | ||
| npx copilot-api@latest start --api-key your_secret_key | ||
|
|
||
| # Multiple API keys | ||
| npx copilot-api@latest start --api-key key1 --api-key key2 --api-key key3 | ||
| ``` | ||
|
|
||
| When API keys are configured: | ||
| - All API endpoints (`/v1/chat/completions`, `/v1/models`, `/v1/embeddings`, `/v1/messages`, `/usage`, `/token`) require authentication | ||
| - Requests without valid API keys will receive a 401 Unauthorized response | ||
| - The root endpoint `/` remains accessible without authentication |
There was a problem hiding this comment.
Document usage viewer behavior with API key authentication.
The API Key Authentication section is well-written and comprehensive. However, there's a documentation gap: the Usage Viewer section (lines 302-322) doesn't explain how to access the dashboard when API key authentication is enabled. Users may be confused about whether they need to provide an API key in the URL or if the endpoint remains accessible.
Consider adding a note in either the API Key Authentication section or the Usage Viewer section explaining how authentication works with the browser-based dashboard (e.g., whether the /usage endpoint requires authentication when accessed directly vs. through the dashboard).
🤖 Prompt for AI Agents
In README.md around lines 213-246, add a short note clarifying how the Usage
Viewer/dashboard is affected when API key authentication is enabled: inspect the
server implementation to determine whether the /usage endpoint and the browser
dashboard require the API key, and then document the exact way to authenticate
when using the dashboard (e.g., which header/query param/cookie or UI login flow
the dashboard accepts), including an example of how to provide the key from a
browser (or mention browser extensions/tools if direct header setting is not
supported).
|
@coderabbitai generate docstrings |
✅ Actions performedInitiated docstring generation; will generate only if new commits exist. |
|
Note Docstrings generation - SUCCESS |
Docstrings generation was requested by @ZiuChen. * #144 (comment) The following files were modified: * `src/lib/api-key-auth.ts` * `src/start.ts`
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/lib/api-key-auth.ts(1 hunks)src/lib/utils.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/lib/api-key-auth.ts (2)
src/lib/state.ts (1)
state(23-28)src/lib/utils.ts (1)
constantTimeEqual(28-37)
🔇 Additional comments (2)
src/lib/utils.ts (1)
27-37: LGTM! Solid constant-time comparison implementation.The function correctly prevents timing attacks by using XOR accumulation instead of short-circuit evaluation. The early return on length mismatch is acceptable since length is not secret information. Using
codePointAtproperly handles Unicode characters.src/lib/api-key-auth.ts (1)
38-68: LGTM! Middleware implementation is secure and well-structured.The middleware correctly:
- Maintains backward compatibility when no keys are configured
- Uses
constantTimeEqualfor timing-attack-resistant validation- Provides clear error messages
- Validates against all configured keys efficiently with
.some()
…ify error messages
1 participant
Add API Key Authentication Support
🎯 Overview
This pull request adds comprehensive API key authentication support to the Copilot API proxy, allowing administrators to secure their proxy endpoints with configurable API keys.
✨ Features
🔐 Flexible Authentication
Authorization: Bearer <token>) and Anthropic (x-api-key: <token>) authentication formats🛠️ Configuration
--api-keyflag supports multiple values for easy configuration🔒 Security Features
/v1/chat/completions,/v1/models,/v1/embeddings,/v1/messages,/usage,/token) are protected when API keys are configured📝 Changes
New Files
Modified Files
apiKeysfield to application state interface🚀 Usage Examples
Single API Key
Multiple API Keys
Authentication Requests
💡 Implementation Notes
Summary by CodeRabbit
New Features
Documentation