feat: add client (CLI/IDE) restrictions for user management#341
feat: add client (CLI/IDE) restrictions for user management#341ding113 merged 4 commits intoding113:devfrom
Conversation
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Add functionality to restrict which API clients (CLI tools/IDEs) can use a user's API keys based on User-Agent header matching. Features: - New `allowedClients` field on users table (JSONB array) - ProxyClientGuard validates User-Agent in proxy pipeline (after auth) - Admin UI with preset client checkboxes + custom pattern input - Display allowed clients info visible to both admin and user Preset client patterns: - claude-cli: Claude Code CLI - gemini-cli: Gemini CLI - factory-cli: Droid CLI (Factory AI) - codex-cli: Codex CLI Behavior: - Empty array = no restrictions (all clients allowed) - Non-empty array = only listed patterns allowed (case-insensitive) - Missing/empty User-Agent with restrictions → 400 error - Non-matching User-Agent with restrictions → 400 error 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Summary of ChangesHello @miraserver, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances security by introducing a client restriction mechanism for user API keys. It allows administrators to specify which CLI tools or IDEs are permitted to use a user's API keys by validating the User-Agent header of incoming requests. This feature provides granular control over API access, preventing unauthorized usage from unapproved client applications. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
github-actions
bot
added
enhancement
![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 introduces a valuable feature for restricting API client access based on the User-Agent. The implementation is comprehensive, covering the database schema, backend logic, and a well-designed UI for configuration. The code is generally well-structured and follows good practices. I've included a few specific comments with suggestions to enhance maintainability, readability, and performance in the new components. Overall, this is a solid and well-executed feature addition.
| {/* Allowed Clients (CLI/IDE restrictions) */} | ||
| <div className="space-y-3"> | ||
| <div className="space-y-0.5"> | ||
| <Label className="text-sm font-medium">{tForm("allowedClients.label")}</Label> | ||
| <p className="text-xs text-muted-foreground">{tForm("allowedClients.description")}</p> | ||
| </div> | ||
|
|
||
| {/* Preset client checkboxes */} | ||
| <div className="grid grid-cols-2 gap-2"> | ||
| {PRESET_CLIENTS.map((client) => { | ||
| const isChecked = (form.values.allowedClients || []).includes(client.value); | ||
| return ( | ||
| <div key={client.value} className="flex items-center space-x-2"> | ||
| <Checkbox | ||
| id={`client-${client.value}`} | ||
| checked={isChecked} | ||
| onCheckedChange={(checked) => { | ||
| const currentClients = form.values.allowedClients || []; | ||
| if (checked) { | ||
| form.setValue("allowedClients", [...currentClients, client.value]); | ||
| } else { | ||
| form.setValue( | ||
| "allowedClients", | ||
| currentClients.filter((c: string) => c !== client.value) | ||
| ); | ||
| } | ||
| }} | ||
| /> | ||
| <Label | ||
| htmlFor={`client-${client.value}`} | ||
| className="text-sm font-normal cursor-pointer" | ||
| > | ||
| {client.label} | ||
| </Label> | ||
| </div> | ||
| ); | ||
| })} | ||
| </div> | ||
|
|
||
| {/* Custom client patterns */} | ||
| <ArrayTagInputField | ||
| label={tForm("allowedClients.customLabel")} | ||
| maxTagLength={64} | ||
| maxTags={50} | ||
| placeholder={tForm("allowedClients.customPlaceholder")} | ||
| onInvalidTag={(_tag, reason) => { | ||
| const messages: Record<string, string> = { | ||
| empty: tUI("emptyTag"), | ||
| duplicate: tUI("duplicateTag"), | ||
| too_long: tUI("tooLong", { max: 64 }), | ||
| invalid_format: tUI("invalidFormat"), | ||
| max_tags: tUI("maxTags"), | ||
| }; | ||
| toast.error(messages[reason] || reason); | ||
| }} | ||
| value={(form.values.allowedClients || []).filter( | ||
| (c: string) => !PRESET_CLIENTS.some((p) => p.value === c) | ||
| )} | ||
| onChange={(customClients: string[]) => { | ||
| // Merge preset clients with custom clients | ||
| const presetClients = (form.values.allowedClients || []).filter((c: string) => | ||
| PRESET_CLIENTS.some((p) => p.value === c) | ||
| ); | ||
| form.setValue("allowedClients", [...presetClients, ...customClients]); | ||
| }} | ||
| /> | ||
| </div> |
There was a problem hiding this comment.
The current implementation for managing preset and custom clients is functional but a bit complex, with filtering logic repeated in multiple places. This can be simplified for better readability and maintainability.
I suggest two improvements:
- Use
useMemoto derivepresetClientsandcustomClientsfromform.values.allowedClientsonce per render. This avoids repeated calculations in your JSX. You can add this hook before the component'sreturnstatement:const { presetClients, customClients } = useMemo(() => { const allClients = form.values.allowedClients || []; const presetClientValues = new Set(PRESET_CLIENTS.map((p) => p.value)); const presetClients = allClients.filter(client => presetClientValues.has(client)); const customClients = allClients.filter(client => !presetClientValues.has(client)); return { presetClients, customClients }; }, [form.values.allowedClients]);
- Use a
Setin the checkboxonCheckedChangehandler to make adding and removing clients more efficient and the code more declarative.
With these changes, the ArrayTagInputField's value would become customClients and its onChange would be (newCustomClients) => form.setValue("allowedClients", [...presetClients, ...newCustomClients]). The checkbox handler would also be simplified.
| {/* Allowed Clients Display - on separate line, visible to both admin and user */} | ||
| {activeUser && ( | ||
| <div className="mt-2 px-2 py-1 text-xs text-muted-foreground border border-muted-foreground/30 rounded-md w-fit"> | ||
| <span> | ||
| {activeUser.allowedClients?.length | ||
| ? `${t("allowedClients.label")} [${activeUser.allowedClients.length}]:` | ||
| : t("allowedClients.noRestrictions")} | ||
| </span> | ||
| {activeUser.allowedClients && activeUser.allowedClients.length > 0 && ( | ||
| <span className="text-foreground ml-1"> | ||
| {activeUser.allowedClients.join(", ")} | ||
| </span> | ||
| )} | ||
| </div> | ||
| )} |
There was a problem hiding this comment.
The JSX for displaying the allowed clients can be made more concise and readable. The current implementation has a slightly redundant check and separates the label from the list. Refactoring this into a single conditional block improves clarity.
| {/* Allowed Clients Display - on separate line, visible to both admin and user */} | |
| {activeUser && ( | |
| <div className="mt-2 px-2 py-1 text-xs text-muted-foreground border border-muted-foreground/30 rounded-md w-fit"> | |
| <span> | |
| {activeUser.allowedClients?.length | |
| ? `${t("allowedClients.label")} [${activeUser.allowedClients.length}]:` | |
| : t("allowedClients.noRestrictions")} | |
| </span> | |
| {activeUser.allowedClients && activeUser.allowedClients.length > 0 && ( | |
| <span className="text-foreground ml-1"> | |
| {activeUser.allowedClients.join(", ")} | |
| </span> | |
| )} | |
| </div> | |
| )} | |
| {/* Allowed Clients Display - on separate line, visible to both admin and user */} | |
| {activeUser && ( | |
| <div className="mt-2 px-2 py-1 text-xs text-muted-foreground border border-muted-foreground/30 rounded-md w-fit"> | |
| {activeUser.allowedClients?.length > 0 ? ( | |
| <> | |
| {`${t("allowedClients.label")} [${activeUser.allowedClients.length}]: `} | |
| <span className="text-foreground"> | |
| {activeUser.allowedClients.join(", ")} | |
| </span> | |
| </> | |
| ) : ( | |
| t("allowedClients.noRestrictions") | |
| )} | |
| </div> | |
| )} |
| const userAgentLower = userAgent.toLowerCase(); | ||
| const isAllowed = allowedClients.some((pattern) => | ||
| userAgentLower.includes(pattern.toLowerCase()) | ||
| ); |
There was a problem hiding this comment.
To improve performance slightly, you can avoid calling pattern.toLowerCase() inside the some loop. Since the allowedClients array can have up to 50 items, it's better to map them to lowercase once before the loop.
| const userAgentLower = userAgent.toLowerCase(); | |
| const isAllowed = allowedClients.some((pattern) => | |
| userAgentLower.includes(pattern.toLowerCase()) | |
| ); | |
| const userAgentLower = userAgent.toLowerCase(); | |
| const allowedClientsLower = allowedClients.map((p) => p.toLowerCase()); | |
| const isAllowed = allowedClientsLower.some((pattern) => | |
| userAgentLower.includes(pattern) | |
| ); |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review Summary
No significant issues identified in this PR. The implementation adds a well-designed client restriction feature that follows existing patterns in the codebase.
PR Size: S
- Lines changed: 294 (293 additions, 1 deletion)
- Files changed: 18 (primarily i18n translations + core implementation)
Implementation Highlights
- ProxyClientGuard: Clean implementation with proper early-exit when no restrictions configured
- Guard Pipeline Integration: Correctly placed after auth, ensuring user context is available
- Type Safety: Proper TypeScript types added across User, CreateUserData, UpdateUserData, and UserDisplay interfaces
- Validation: Zod schema properly limits patterns (max 50 patterns, 64 chars each)
- Database: Simple migration adding JSONB column with sensible default
Review Coverage
- Logic and correctness - Clean
- Security (OWASP Top 10) - Clean (case-insensitive substring matching is intentional and documented)
- Error handling - Clean (proper 400 responses with descriptive messages)
- Type safety - Clean
- Documentation accuracy - Clean
- Test coverage - No new tests added, but feature is low-risk with simple logic
- Code clarity - Good
Notes
- The substring matching approach (e.g.,
claude-climatchesclaude-cli/2.0.50) is appropriate for User-Agent validation - Empty
allowedClientsarray correctly bypasses all checks (fail-open for unconfigured users) - The guard runs immediately after auth, which is the correct position in the pipeline
Automated review by Claude AI
There was a problem hiding this comment.
Code Review Summary
No significant issues identified in this PR. The implementation adds a well-designed client restriction feature that follows existing patterns in the codebase.
PR Size: S
- Lines changed: 294 (293 additions, 1 deletion)
- Files changed: 18 (primarily i18n translations + core implementation)
Implementation Highlights
- ProxyClientGuard: Clean implementation with proper early-exit when no restrictions configured
- Guard Pipeline Integration: Correctly placed after auth, ensuring user context is available
- Type Safety: Proper TypeScript types added across User, CreateUserData, UpdateUserData, and UserDisplay interfaces
- Validation: Zod schema properly limits patterns (max 50 patterns, 64 chars each)
- Database: Simple migration adding JSONB column with sensible default
Review Coverage
- Logic and correctness - Clean
- Security (OWASP Top 10) - Clean (case-insensitive substring matching is intentional and documented)
- Error handling - Clean (proper 400 responses with descriptive messages)
- Type safety - Clean
- Documentation accuracy - Clean
- Test coverage - No new tests added, but feature is low-risk with simple logic
- Code clarity - Good
Notes
- The substring matching approach (e.g., `claude-cli` matches `claude-cli/2.0.50`) is appropriate for User-Agent validation
- Empty `allowedClients` array correctly bypasses all checks (fail-open for unconfigured users)
- The guard runs immediately after auth, which is the correct position in the pipeline
Automated review by Claude AI
|
It looks like this PR did not use the Please resolve this issue before merging. |
not sure what i understand correctly, here is drizzle/0035_add_allowed_clients.sql migration file. Need some other? |
After correctly running the command, a snapshot file and a journal file will be automatically generated. This is necessary for subsequent development. |
miraserver
force-pushed
the
feat/allowed-clients
branch
from
f380b8f to
f8d13fc
Compare
Migration FixFixed the database migration files by using proper Drizzle Kit workflow. What was done:
Validation:
Commit: @ding113 The migration structure is now correct as requested. All three file types (SQL, snapshot, journal) are properly generated. |
github-project-automation
bot
moved this from Backlog
to Done
in Claude Code Hub Roadmap
2 participants
Summary
Add functionality to restrict which API clients (CLI tools/IDEs) can use a user's API keys based on User-Agent header matching.
Related PRs:
/api/key-statsendpoint that was in the previous PR)Features
allowedClientsfield on users table (JSONB array)Preset Client Patterns
claude-cligemini-clifactory-clicodex-cliBehavior
Files Changed (18 files)
0035_add_allowed_clients.sqlTest Plan
🤖 Generated with Claude Code
Description enhanced by Claude AI