Skip to content

fix: key logging#3671

Merged
Flo4604 merged 5 commits intomainfrom
fix/key-logging
Jul 28, 2025
Merged

fix: key logging#3671
Flo4604 merged 5 commits intomainfrom
fix/key-logging

Conversation

@Flo4604
Copy link
Member

@Flo4604 Flo4604 commented Jul 28, 2025

What does this PR do?

Fixes #3548
Fixes #3546

If there is not an issue for this, please create one first. This is used to tracking purposes and also helps use understand why this PR exists

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • Chore (refactoring code, technical debt, workflow improvements)
  • Enhancement (small improvements)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How should this be tested?

  • Test A
  • Test B

Checklist

Required

  • Filled out the "How to test" section in this PR
  • Read Contributing Guide
  • Self-reviewed my own code
  • Commented on my code in hard-to-understand areas
  • Ran pnpm build
  • Ran pnpm fmt
  • Checked for warnings, there are none
  • Removed all console.logs
  • Merged the latest changes from main onto my branch with git pull origin main
  • My changes don't cause any responsiveness issues

Appreciated

  • If a UI change was made: Added a screen recording or screenshots to this PR
  • Updated the Unkey Docs if changes were necessary

Summary by CodeRabbit

  • Refactor

    • Improved authentication and permission verification across API endpoints by introducing a deferred cleanup step and switching to a more specific root key verification method.
    • Enhanced resource management and observability during key operations.
    • Adjusted error handling for rate limiter failures to log issues without blocking requests.
  • Bug Fixes

    • Prevented rate limiter errors from blocking requests by logging failures instead of returning errors.
  • Chores

    • Removed unused API ID verification logic and related options.
    • Updated internal interfaces and method signatures for consistency and maintainability.

@changeset-bot
Copy link

changeset-bot bot commented Jul 28, 2025

⚠️ No Changeset found

Latest commit: c514484

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@vercel
Copy link

vercel bot commented Jul 28, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

2 Skipped Deployments
Name Status Preview Comments Updated (UTC)
dashboard ⬜️ Ignored (Inspect) Visit Preview Jul 28, 2025 7:42pm
engineering ⬜️ Ignored (Inspect) Visit Preview Jul 28, 2025 7:42pm

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jul 28, 2025

📝 Walkthrough

Walkthrough

This change refactors the authentication and authorization flow for root keys and regular keys throughout the API routes and the internal key service. It introduces a deferred logging/cleanup callback (emit) returned by key retrieval methods, adds a dedicated VerifyRootKey method for root key permission checks, removes API ID-based verification, and updates method signatures and interfaces accordingly.

Changes

Cohort / File(s) Change Summary
API Route Handlers: Root Key Auth Refactor
go/apps/api/routes/v2_apis_create_api/handler.go, go/apps/api/routes/v2_apis_delete_api/handler.go, go/apps/api/routes/v2_apis_get_api/handler.go, go/apps/api/routes/v2_apis_list_keys/handler.go, go/apps/api/routes/v2_identities_create_identity/handler.go, go/apps/api/routes/v2_identities_delete_identity/handler.go, go/apps/api/routes/v2_identities_get_identity/handler.go, go/apps/api/routes/v2_identities_list_identities/handler.go, go/apps/api/routes/v2_identities_update_identity/handler.go, go/apps/api/routes/v2_keys_add_permissions/handler.go, go/apps/api/routes/v2_keys_add_roles/handler.go, go/apps/api/routes/v2_keys_create_key/handler.go, go/apps/api/routes/v2_keys_delete_key/handler.go, go/apps/api/routes/v2_keys_get_key/handler.go, go/apps/api/routes/v2_keys_remove_permissions/handler.go, go/apps/api/routes/v2_keys_remove_roles/handler.go, go/apps/api/routes/v2_keys_set_permissions/handler.go, go/apps/api/routes/v2_keys_set_roles/handler.go, go/apps/api/routes/v2_keys_update_credits/handler.go, go/apps/api/routes/v2_keys_update_key/handler.go, go/apps/api/routes/v2_keys_verify_key/handler.go, go/apps/api/routes/v2_keys_whoami/handler.go, go/apps/api/routes/v2_permissions_create_permission/handler.go, go/apps/api/routes/v2_permissions_create_role/handler.go, go/apps/api/routes/v2_permissions_delete_permission/handler.go, go/apps/api/routes/v2_permissions_delete_role/handler.go, go/apps/api/routes/v2_permissions_get_permission/handler.go, go/apps/api/routes/v2_permissions_get_role/handler.go, go/apps/api/routes/v2_permissions_list_permissions/handler.go, go/apps/api/routes/v2_permissions_list_roles/handler.go, go/apps/api/routes/v2_ratelimit_delete_override/handler.go, go/apps/api/routes/v2_ratelimit_get_override/handler.go, go/apps/api/routes/v2_ratelimit_limit/handler.go, go/apps/api/routes/v2_ratelimit_list_overrides/handler.go, go/apps/api/routes/v2_ratelimit_set_override/handler.go
Refactored root key authentication to use a new emit deferred callback from GetRootKey, switched permission checks to use VerifyRootKey instead of Verify, and updated method signatures and usage accordingly.
Key Service Implementation
go/internal/services/keys/get.go
Updated Get and GetRootKey to return an additional cleanup/logging function; moved root key-specific validation from Get to GetRootKey; updated error handling and session assignment logic.
Key Service Interface
go/internal/services/keys/interface.go
Changed KeyService interface for Get and GetRootKey to return an additional func() for cleanup/logging.
Key Verification Options
go/internal/services/keys/options.go, go/internal/services/keys/validation.go
Removed API ID field and related option/method; updated rate limiter error handling to log and continue instead of returning errors.
Key Verifier Logic
go/internal/services/keys/verifier.go
Added tags field to KeyVerifier, introduced VerifyRootKey method, updated Verify method logic and tag handling, removed API ID logic, and improved logging integration.

Sequence Diagram(s)

sequenceDiagram
    participant Handler
    participant KeyService
    participant KeyVerifier

    Handler->>KeyService: GetRootKey(ctx, session)
    KeyService-->>Handler: KeyVerifier, emit, error
    Handler->>Handler: defer emit()
    Handler->>KeyVerifier: VerifyRootKey(ctx, permissions)
    KeyVerifier-->>Handler: error (if any)
    Handler->>Handler: Continue with request handling
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Assessment against linked issues

Objective Addressed Explanation
Move root key-specific validation logic from Get to GetRootKey for separation of concerns (#3548)
Ensure disabled regular keys are logged during key verification (#3546) The PR does not introduce or refactor logging logic for disabled regular keys during verification; logging changes focus on the new deferred emit for root keys.

Assessment against linked issues: Out-of-scope changes

Code Change Explanation
Removal of API ID verification logic and related option/method (WithApiID) (go/internal/services/keys/options.go, go/internal/services/keys/validation.go, go/internal/services/keys/verifier.go`) The linked issues do not mention API ID-based verification; removal of this feature is unrelated to the objectives of root key logic separation or logging for disabled keys.

Possibly related PRs

  • fix: remove apiId from verifyKey request #3666: Also modifies key verification logic and request parameter handling, particularly around permission checks and response codes, which overlaps with the root key verification refactor in this PR.

Suggested labels

Core Team

Suggested reviewers

  • chronark
  • mcstepp
  • perkinsjr
✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/key-logging

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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai generate unit tests to generate unit tests for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@github-actions
Copy link
Contributor

github-actions bot commented Jul 28, 2025

Thank you for following the naming conventions for pull request titles! 🙏

@graphite-app
Copy link

graphite-app bot commented Jul 28, 2025

Video gif. An elderly man in suspenders smiles as he holds up two thumbs as if saying good luck.  (Added via Giphy)

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8eab750 and c69c582.

📒 Files selected for processing (41)
  • go/apps/api/routes/v2_apis_create_api/handler.go (2 hunks)
  • go/apps/api/routes/v2_apis_delete_api/handler.go (2 hunks)
  • go/apps/api/routes/v2_apis_get_api/handler.go (2 hunks)
  • go/apps/api/routes/v2_apis_list_keys/handler.go (3 hunks)
  • go/apps/api/routes/v2_identities_create_identity/handler.go (2 hunks)
  • go/apps/api/routes/v2_identities_delete_identity/handler.go (2 hunks)
  • go/apps/api/routes/v2_identities_get_identity/handler.go (2 hunks)
  • go/apps/api/routes/v2_identities_list_identities/handler.go (2 hunks)
  • go/apps/api/routes/v2_identities_update_identity/handler.go (2 hunks)
  • go/apps/api/routes/v2_keys_add_permissions/handler.go (2 hunks)
  • go/apps/api/routes/v2_keys_add_roles/handler.go (2 hunks)
  • go/apps/api/routes/v2_keys_create_key/handler.go (3 hunks)
  • go/apps/api/routes/v2_keys_delete_key/handler.go (2 hunks)
  • go/apps/api/routes/v2_keys_get_key/handler.go (3 hunks)
  • go/apps/api/routes/v2_keys_remove_permissions/handler.go (2 hunks)
  • go/apps/api/routes/v2_keys_remove_roles/handler.go (2 hunks)
  • go/apps/api/routes/v2_keys_set_permissions/handler.go (2 hunks)
  • go/apps/api/routes/v2_keys_set_roles/handler.go (2 hunks)
  • go/apps/api/routes/v2_keys_update_credits/handler.go (2 hunks)
  • go/apps/api/routes/v2_keys_update_key/handler.go (2 hunks)
  • go/apps/api/routes/v2_keys_verify_key/200_test.go (1 hunks)
  • go/apps/api/routes/v2_keys_verify_key/403_test.go (0 hunks)
  • go/apps/api/routes/v2_keys_verify_key/handler.go (4 hunks)
  • go/apps/api/routes/v2_permissions_create_permission/handler.go (2 hunks)
  • go/apps/api/routes/v2_permissions_create_role/handler.go (2 hunks)
  • go/apps/api/routes/v2_permissions_delete_permission/handler.go (2 hunks)
  • go/apps/api/routes/v2_permissions_delete_role/handler.go (2 hunks)
  • go/apps/api/routes/v2_permissions_get_permission/handler.go (2 hunks)
  • go/apps/api/routes/v2_permissions_get_role/handler.go (2 hunks)
  • go/apps/api/routes/v2_permissions_list_permissions/handler.go (2 hunks)
  • go/apps/api/routes/v2_permissions_list_roles/handler.go (2 hunks)
  • go/apps/api/routes/v2_ratelimit_delete_override/handler.go (2 hunks)
  • go/apps/api/routes/v2_ratelimit_get_override/handler.go (2 hunks)
  • go/apps/api/routes/v2_ratelimit_limit/handler.go (2 hunks)
  • go/apps/api/routes/v2_ratelimit_list_overrides/handler.go (2 hunks)
  • go/apps/api/routes/v2_ratelimit_set_override/handler.go (2 hunks)
  • go/internal/services/keys/get.go (6 hunks)
  • go/internal/services/keys/interface.go (1 hunks)
  • go/internal/services/keys/options.go (1 hunks)
  • go/internal/services/keys/validation.go (1 hunks)
  • go/internal/services/keys/verifier.go (4 hunks)
💤 Files with no reviewable changes (1)
  • go/apps/api/routes/v2_keys_verify_key/403_test.go
🧰 Additional context used
🧠 Learnings (23)
📓 Common learnings
Learnt from: MichaelUnkey
PR: unkeyed/unkey#3103
File: apps/dashboard/app/(app)/settings/general/page.tsx:36-36
Timestamp: 2025-04-14T13:39:22.635Z
Learning: The Unkey team uses Linear for issue tracking rather than GitHub Issues.
Learnt from: chronark
PR: unkeyed/unkey#2693
File: apps/api/src/routes/v1_keys_updateKey.ts:350-368
Timestamp: 2024-11-29T15:15:47.308Z
Learning: In `apps/api/src/routes/v1_keys_updateKey.ts`, the code intentionally handles `externalId` and `ownerId` separately for clarity. The `ownerId` field will be removed in the future, simplifying the code.
Learnt from: MichaelUnkey
PR: unkeyed/unkey#2114
File: apps/api/src/routes/v1_keys_updateKey.error.test.ts:0-0
Timestamp: 2024-09-27T15:20:05.475Z
Learning: In the `v1/keys.updateKey` endpoint, the server validates the refill configuration before checking if the key exists. Therefore, tests can assert validation errors without needing to create the key first.
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#3661
File: go/apps/api/routes/v2_identities_update_identity/handler.go:115-119
Timestamp: 2025-07-28T11:47:43.134Z
Learning: The v2 update identity endpoint (go/apps/api/routes/v2_identities_update_identity/handler.go) intentionally uses `ExternalId` field instead of the unified `Identity` field used in other v2 identity endpoints. This is because the update endpoint needs to both find by externalId and potentially update the externalId value, making the specific field name more appropriate than the generic `Identity` field.
go/apps/api/routes/v2_permissions_delete_role/handler.go (3)

Learnt from: chronark
PR: #3560
File: go/apps/api/routes/v2_keys_create_key/handler.go:468-581
Timestamp: 2025-07-15T14:47:20.490Z
Learning: In the Unkey codebase, role and permission names are validated at the OpenAPI schema layer with strict regex patterns: role names must match "^[a-zA-Z][a-zA-Z0-9_-]*$" (start with letter, followed by letters/numbers/underscores/hyphens) and permission names must match "^[a-zA-Z0-9_]+$" (letters, numbers, underscores only). This validation occurs during zen.BindBody call before handlers run, preventing malicious or improperly formatted names from reaching auto-creation logic.

Learnt from: AkshayBandi027
PR: #2215
File: apps/dashboard/app/(app)/@breadcrumb/authorization/roles/[roleId]/page.tsx:28-29
Timestamp: 2024-10-08T15:33:04.290Z
Learning: In authorization/roles/[roleId]/update-role.tsx, the tag role-${role.id} is revalidated after updating a role to ensure that the caching mechanism is properly handled for roles.

Learnt from: ogzhanolguncu
PR: #3324
File: apps/dashboard/app/(app)/authorization/roles/components/table/components/actions/keys-table-action.popover.constants.tsx:17-18
Timestamp: 2025-06-19T11:48:05.070Z
Learning: In the authorization roles refactor, the RoleBasic type uses roleId as the property name for the role identifier, not id. This is consistent throughout the codebase in apps/dashboard/lib/trpc/routers/authorization/roles/query.ts.

go/apps/api/routes/v2_keys_remove_permissions/handler.go (1)

Learnt from: chronark
PR: #3560
File: go/apps/api/routes/v2_keys_create_key/handler.go:468-581
Timestamp: 2025-07-15T14:47:20.490Z
Learning: In the Unkey codebase, role and permission names are validated at the OpenAPI schema layer with strict regex patterns: role names must match "^[a-zA-Z][a-zA-Z0-9_-]*$" (start with letter, followed by letters/numbers/underscores/hyphens) and permission names must match "^[a-zA-Z0-9_]+$" (letters, numbers, underscores only). This validation occurs during zen.BindBody call before handlers run, preventing malicious or improperly formatted names from reaching auto-creation logic.

go/apps/api/routes/v2_identities_create_identity/handler.go (2)

Learnt from: ogzhanolguncu
PR: #3661
File: go/apps/api/routes/v2_identities_update_identity/handler.go:115-119
Timestamp: 2025-07-28T11:47:43.134Z
Learning: The v2 update identity endpoint (go/apps/api/routes/v2_identities_update_identity/handler.go) intentionally uses ExternalId field instead of the unified Identity field used in other v2 identity endpoints. This is because the update endpoint needs to both find by externalId and potentially update the externalId value, making the specific field name more appropriate than the generic Identity field.

Learnt from: Flo4604
PR: #2955
File: go/apps/api/routes/v2_identities_create_identity/handler.go:162-202
Timestamp: 2025-03-19T09:25:59.751Z
Learning: In the Unkey codebase, input validation for API endpoints is primarily handled through OpenAPI schema validation, which occurs before requests reach the handler code. For example, in the identities.createIdentity endpoint, minimum values for ratelimit duration and limit are defined in the OpenAPI schema rather than duplicating these checks in the handler.

go/apps/api/routes/v2_permissions_list_roles/handler.go (1)

Learnt from: chronark
PR: #3560
File: go/apps/api/routes/v2_keys_create_key/handler.go:468-581
Timestamp: 2025-07-15T14:47:20.490Z
Learning: In the Unkey codebase, role and permission names are validated at the OpenAPI schema layer with strict regex patterns: role names must match "^[a-zA-Z][a-zA-Z0-9_-]*$" (start with letter, followed by letters/numbers/underscores/hyphens) and permission names must match "^[a-zA-Z0-9_]+$" (letters, numbers, underscores only). This validation occurs during zen.BindBody call before handlers run, preventing malicious or improperly formatted names from reaching auto-creation logic.

go/apps/api/routes/v2_keys_delete_key/handler.go (1)

Learnt from: chronark
PR: #3560
File: go/apps/api/routes/v2_keys_create_key/handler.go:468-581
Timestamp: 2025-07-15T14:47:20.490Z
Learning: In the Unkey codebase, role and permission names are validated at the OpenAPI schema layer with strict regex patterns: role names must match "^[a-zA-Z][a-zA-Z0-9_-]*$" (start with letter, followed by letters/numbers/underscores/hyphens) and permission names must match "^[a-zA-Z0-9_]+$" (letters, numbers, underscores only). This validation occurs during zen.BindBody call before handlers run, preventing malicious or improperly formatted names from reaching auto-creation logic.

go/apps/api/routes/v2_keys_verify_key/200_test.go (3)

Learnt from: MichaelUnkey
PR: #2114
File: apps/api/src/routes/v1_keys_updateKey.error.test.ts:0-0
Timestamp: 2024-09-27T15:20:05.475Z
Learning: In the v1/keys.updateKey endpoint, the server validates the refill configuration before checking if the key exists. Therefore, tests can assert validation errors without needing to create the key first.

Learnt from: chronark
PR: #3617
File: go/apps/api/openapi/openapi.yaml:3309-3312
Timestamp: 2025-07-16T17:51:57.297Z
Learning: In the Unkey API OpenAPI schema, the permissions query regex for the verifyKey endpoint intentionally allows all whitespace characters (including tabs and newlines) via \s. Do not flag this as an error in future reviews.

Learnt from: ogzhanolguncu
PR: #3661
File: go/apps/api/routes/v2_identities_update_identity/handler.go:115-119
Timestamp: 2025-07-28T11:47:43.134Z
Learning: The v2 update identity endpoint (go/apps/api/routes/v2_identities_update_identity/handler.go) intentionally uses ExternalId field instead of the unified Identity field used in other v2 identity endpoints. This is because the update endpoint needs to both find by externalId and potentially update the externalId value, making the specific field name more appropriate than the generic Identity field.

go/apps/api/routes/v2_identities_get_identity/handler.go (1)

Learnt from: ogzhanolguncu
PR: #3661
File: go/apps/api/routes/v2_identities_update_identity/handler.go:115-119
Timestamp: 2025-07-28T11:47:43.134Z
Learning: The v2 update identity endpoint (go/apps/api/routes/v2_identities_update_identity/handler.go) intentionally uses ExternalId field instead of the unified Identity field used in other v2 identity endpoints. This is because the update endpoint needs to both find by externalId and potentially update the externalId value, making the specific field name more appropriate than the generic Identity field.

go/apps/api/routes/v2_keys_get_key/handler.go (1)

Learnt from: chronark
PR: #3560
File: go/apps/api/routes/v2_keys_create_key/handler.go:468-581
Timestamp: 2025-07-15T14:47:20.490Z
Learning: In the Unkey codebase, role and permission names are validated at the OpenAPI schema layer with strict regex patterns: role names must match "^[a-zA-Z][a-zA-Z0-9_-]*$" (start with letter, followed by letters/numbers/underscores/hyphens) and permission names must match "^[a-zA-Z0-9_]+$" (letters, numbers, underscores only). This validation occurs during zen.BindBody call before handlers run, preventing malicious or improperly formatted names from reaching auto-creation logic.

go/apps/api/routes/v2_keys_set_roles/handler.go (1)

Learnt from: chronark
PR: #3560
File: go/apps/api/routes/v2_keys_create_key/handler.go:468-581
Timestamp: 2025-07-15T14:47:20.490Z
Learning: In the Unkey codebase, role and permission names are validated at the OpenAPI schema layer with strict regex patterns: role names must match "^[a-zA-Z][a-zA-Z0-9_-]*$" (start with letter, followed by letters/numbers/underscores/hyphens) and permission names must match "^[a-zA-Z0-9_]+$" (letters, numbers, underscores only). This validation occurs during zen.BindBody call before handlers run, preventing malicious or improperly formatted names from reaching auto-creation logic.

go/apps/api/routes/v2_permissions_get_role/handler.go (1)

Learnt from: chronark
PR: #3560
File: go/apps/api/routes/v2_keys_create_key/handler.go:468-581
Timestamp: 2025-07-15T14:47:20.490Z
Learning: In the Unkey codebase, role and permission names are validated at the OpenAPI schema layer with strict regex patterns: role names must match "^[a-zA-Z][a-zA-Z0-9_-]*$" (start with letter, followed by letters/numbers/underscores/hyphens) and permission names must match "^[a-zA-Z0-9_]+$" (letters, numbers, underscores only). This validation occurs during zen.BindBody call before handlers run, preventing malicious or improperly formatted names from reaching auto-creation logic.

go/apps/api/routes/v2_keys_add_permissions/handler.go (1)

Learnt from: chronark
PR: #3560
File: go/apps/api/routes/v2_keys_create_key/handler.go:468-581
Timestamp: 2025-07-15T14:47:20.490Z
Learning: In the Unkey codebase, role and permission names are validated at the OpenAPI schema layer with strict regex patterns: role names must match "^[a-zA-Z][a-zA-Z0-9_-]*$" (start with letter, followed by letters/numbers/underscores/hyphens) and permission names must match "^[a-zA-Z0-9_]+$" (letters, numbers, underscores only). This validation occurs during zen.BindBody call before handlers run, preventing malicious or improperly formatted names from reaching auto-creation logic.

go/apps/api/routes/v2_keys_set_permissions/handler.go (1)

Learnt from: chronark
PR: #3560
File: go/apps/api/routes/v2_keys_create_key/handler.go:468-581
Timestamp: 2025-07-15T14:47:20.490Z
Learning: In the Unkey codebase, role and permission names are validated at the OpenAPI schema layer with strict regex patterns: role names must match "^[a-zA-Z][a-zA-Z0-9_-]*$" (start with letter, followed by letters/numbers/underscores/hyphens) and permission names must match "^[a-zA-Z0-9_]+$" (letters, numbers, underscores only). This validation occurs during zen.BindBody call before handlers run, preventing malicious or improperly formatted names from reaching auto-creation logic.

go/apps/api/routes/v2_identities_delete_identity/handler.go (2)

Learnt from: ogzhanolguncu
PR: #3661
File: go/apps/api/routes/v2_identities_update_identity/handler.go:115-119
Timestamp: 2025-07-28T11:47:43.134Z
Learning: The v2 update identity endpoint (go/apps/api/routes/v2_identities_update_identity/handler.go) intentionally uses ExternalId field instead of the unified Identity field used in other v2 identity endpoints. This is because the update endpoint needs to both find by externalId and potentially update the externalId value, making the specific field name more appropriate than the generic Identity field.

Learnt from: Flo4604
PR: #3151
File: go/apps/api/openapi/gen.go:221-233
Timestamp: 2025-04-18T20:01:33.812Z
Learning: For identity deletion operations in the Unkey API, identityId takes precedence over externalId when both are provided in the request body.

go/apps/api/routes/v2_identities_list_identities/handler.go (1)

Learnt from: ogzhanolguncu
PR: #3661
File: go/apps/api/routes/v2_identities_update_identity/handler.go:115-119
Timestamp: 2025-07-28T11:47:43.134Z
Learning: The v2 update identity endpoint (go/apps/api/routes/v2_identities_update_identity/handler.go) intentionally uses ExternalId field instead of the unified Identity field used in other v2 identity endpoints. This is because the update endpoint needs to both find by externalId and potentially update the externalId value, making the specific field name more appropriate than the generic Identity field.

go/apps/api/routes/v2_keys_update_credits/handler.go (1)

Learnt from: chronark
PR: #3560
File: go/apps/api/routes/v2_keys_create_key/handler.go:468-581
Timestamp: 2025-07-15T14:47:20.490Z
Learning: In the Unkey codebase, role and permission names are validated at the OpenAPI schema layer with strict regex patterns: role names must match "^[a-zA-Z][a-zA-Z0-9_-]*$" (start with letter, followed by letters/numbers/underscores/hyphens) and permission names must match "^[a-zA-Z0-9_]+$" (letters, numbers, underscores only). This validation occurs during zen.BindBody call before handlers run, preventing malicious or improperly formatted names from reaching auto-creation logic.

go/apps/api/routes/v2_identities_update_identity/handler.go (1)

Learnt from: ogzhanolguncu
PR: #3661
File: go/apps/api/routes/v2_identities_update_identity/handler.go:115-119
Timestamp: 2025-07-28T11:47:43.134Z
Learning: The v2 update identity endpoint (go/apps/api/routes/v2_identities_update_identity/handler.go) intentionally uses ExternalId field instead of the unified Identity field used in other v2 identity endpoints. This is because the update endpoint needs to both find by externalId and potentially update the externalId value, making the specific field name more appropriate than the generic Identity field.

go/apps/api/routes/v2_keys_add_roles/handler.go (1)

Learnt from: chronark
PR: #3560
File: go/apps/api/routes/v2_keys_create_key/handler.go:468-581
Timestamp: 2025-07-15T14:47:20.490Z
Learning: In the Unkey codebase, role and permission names are validated at the OpenAPI schema layer with strict regex patterns: role names must match "^[a-zA-Z][a-zA-Z0-9_-]*$" (start with letter, followed by letters/numbers/underscores/hyphens) and permission names must match "^[a-zA-Z0-9_]+$" (letters, numbers, underscores only). This validation occurs during zen.BindBody call before handlers run, preventing malicious or improperly formatted names from reaching auto-creation logic.

go/apps/api/routes/v2_keys_verify_key/handler.go (1)

Learnt from: chronark
PR: #3560
File: go/apps/api/routes/v2_keys_create_key/handler.go:468-581
Timestamp: 2025-07-15T14:47:20.490Z
Learning: In the Unkey codebase, role and permission names are validated at the OpenAPI schema layer with strict regex patterns: role names must match "^[a-zA-Z][a-zA-Z0-9_-]*$" (start with letter, followed by letters/numbers/underscores/hyphens) and permission names must match "^[a-zA-Z0-9_]+$" (letters, numbers, underscores only). This validation occurs during zen.BindBody call before handlers run, preventing malicious or improperly formatted names from reaching auto-creation logic.

go/apps/api/routes/v2_permissions_create_role/handler.go (1)

Learnt from: chronark
PR: #3560
File: go/apps/api/routes/v2_keys_create_key/handler.go:468-581
Timestamp: 2025-07-15T14:47:20.490Z
Learning: In the Unkey codebase, role and permission names are validated at the OpenAPI schema layer with strict regex patterns: role names must match "^[a-zA-Z][a-zA-Z0-9_-]*$" (start with letter, followed by letters/numbers/underscores/hyphens) and permission names must match "^[a-zA-Z0-9_]+$" (letters, numbers, underscores only). This validation occurs during zen.BindBody call before handlers run, preventing malicious or improperly formatted names from reaching auto-creation logic.

go/apps/api/routes/v2_keys_create_key/handler.go (1)

Learnt from: chronark
PR: #3560
File: go/apps/api/routes/v2_keys_create_key/handler.go:468-581
Timestamp: 2025-07-15T14:47:20.490Z
Learning: In the Unkey codebase, role and permission names are validated at the OpenAPI schema layer with strict regex patterns: role names must match "^[a-zA-Z][a-zA-Z0-9_-]*$" (start with letter, followed by letters/numbers/underscores/hyphens) and permission names must match "^[a-zA-Z0-9_]+$" (letters, numbers, underscores only). This validation occurs during zen.BindBody call before handlers run, preventing malicious or improperly formatted names from reaching auto-creation logic.

go/apps/api/routes/v2_keys_update_key/handler.go (1)

Learnt from: MichaelUnkey
PR: #2114
File: apps/api/src/routes/v1_keys_updateKey.error.test.ts:0-0
Timestamp: 2024-09-27T15:20:05.475Z
Learning: In the v1/keys.updateKey endpoint, the server validates the refill configuration before checking if the key exists. Therefore, tests can assert validation errors without needing to create the key first.

go/internal/services/keys/get.go (2)

Learnt from: ogzhanolguncu
PR: #2872
File: apps/dashboard/lib/trpc/routers/ratelimit/createNamespace.ts:36-39
Timestamp: 2025-04-08T09:34:24.576Z
Learning: In the Unkey dashboard, when making database queries involving workspaces, use ctx.workspace.id directly instead of fetching the workspace separately for better performance and security.

Learnt from: ogzhanolguncu
PR: #2872
File: apps/dashboard/lib/trpc/routers/ratelimit/createNamespace.ts:36-39
Timestamp: 2025-04-08T09:34:24.576Z
Learning: When querying or updating namespaces in the Unkey dashboard, always scope the operations to the current workspace using eq(table.workspaceId, ctx.workspace.id) to prevent cross-workspace access.

🧬 Code Graph Analysis (22)
go/apps/api/routes/v2_permissions_delete_permission/handler.go (2)
go/internal/services/keys/options.go (1)
  • WithPermissions (47-52)
go/pkg/rbac/query.go (1)
  • Or (64-70)
go/apps/api/routes/v2_identities_create_identity/handler.go (2)
go/internal/services/keys/options.go (1)
  • WithPermissions (47-52)
go/pkg/rbac/query.go (1)
  • Or (64-70)
go/apps/api/routes/v2_apis_list_keys/handler.go (2)
go/internal/services/keys/options.go (1)
  • WithPermissions (47-52)
go/pkg/rbac/query.go (1)
  • Or (64-70)
go/apps/api/routes/v2_permissions_list_roles/handler.go (2)
go/internal/services/keys/options.go (1)
  • WithPermissions (47-52)
go/pkg/rbac/query.go (1)
  • Or (64-70)
go/apps/api/routes/v2_keys_delete_key/handler.go (2)
go/internal/services/keys/options.go (1)
  • WithPermissions (47-52)
go/pkg/rbac/query.go (1)
  • Or (64-70)
go/apps/api/routes/v2_identities_get_identity/handler.go (2)
go/internal/services/keys/options.go (1)
  • WithPermissions (47-52)
go/pkg/rbac/query.go (1)
  • Or (64-70)
go/apps/api/routes/v2_keys_get_key/handler.go (2)
go/internal/services/keys/options.go (1)
  • WithPermissions (47-52)
go/pkg/rbac/query.go (1)
  • Or (64-70)
go/apps/api/routes/v2_keys_set_roles/handler.go (2)
go/internal/services/keys/options.go (1)
  • WithPermissions (47-52)
go/pkg/rbac/query.go (1)
  • Or (64-70)
go/apps/api/routes/v2_permissions_get_role/handler.go (2)
go/internal/services/keys/options.go (1)
  • WithPermissions (47-52)
go/pkg/rbac/query.go (1)
  • Or (64-70)
go/apps/api/routes/v2_keys_add_permissions/handler.go (2)
go/internal/services/keys/options.go (1)
  • WithPermissions (47-52)
go/pkg/rbac/query.go (1)
  • Or (64-70)
go/apps/api/routes/v2_identities_list_identities/handler.go (1)
go/internal/services/keys/options.go (1)
  • WithPermissions (47-52)
go/apps/api/routes/v2_keys_update_credits/handler.go (2)
go/internal/services/keys/options.go (1)
  • WithPermissions (47-52)
go/pkg/rbac/query.go (1)
  • Or (64-70)
go/apps/api/routes/v2_identities_update_identity/handler.go (2)
go/internal/services/keys/options.go (1)
  • WithPermissions (47-52)
go/pkg/rbac/query.go (1)
  • Or (64-70)
go/apps/api/routes/v2_keys_verify_key/handler.go (3)
go/internal/services/keys/options.go (4)
  • WithPermissions (47-52)
  • VerifyOption (12-12)
  • WithTags (64-69)
  • WithIPWhitelist (38-43)
go/pkg/rbac/query.go (1)
  • Or (64-70)
go/pkg/ptr/deref.go (1)
  • SafeDeref (35-44)
go/apps/api/routes/v2_apis_create_api/handler.go (2)
go/internal/services/keys/options.go (1)
  • WithPermissions (47-52)
go/pkg/rbac/query.go (1)
  • Or (64-70)
go/apps/api/routes/v2_ratelimit_get_override/handler.go (2)
go/internal/services/keys/options.go (1)
  • WithPermissions (47-52)
go/pkg/rbac/query.go (1)
  • Or (64-70)
go/apps/api/routes/v2_permissions_create_role/handler.go (2)
go/internal/services/keys/options.go (1)
  • WithPermissions (47-52)
go/pkg/rbac/query.go (1)
  • Or (64-70)
go/apps/api/routes/v2_keys_create_key/handler.go (2)
go/internal/services/keys/options.go (1)
  • WithPermissions (47-52)
go/pkg/rbac/query.go (1)
  • Or (64-70)
go/apps/api/routes/v2_permissions_create_permission/handler.go (3)
go/internal/services/keys/options.go (1)
  • WithPermissions (47-52)
go/pkg/rbac/query.go (1)
  • T (84-90)
go/pkg/rbac/permissions.go (1)
  • Tuple (175-184)
go/internal/services/keys/interface.go (2)
go/pkg/zen/session.go (1)
  • Session (28-42)
go/internal/services/keys/verifier.go (1)
  • KeyVerifier (31-48)
go/internal/services/keys/verifier.go (1)
go/internal/services/keys/options.go (1)
  • VerifyOption (12-12)
go/internal/services/keys/get.go (5)
go/pkg/zen/session.go (1)
  • Session (28-42)
go/internal/services/keys/verifier.go (1)
  • KeyVerifier (31-48)
go/pkg/otel/tracing/trace.go (1)
  • Start (59-62)
go/pkg/zen/auth.go (1)
  • Bearer (23-44)
go/pkg/fault/wrap.go (3)
  • Wrap (25-67)
  • Internal (75-89)
  • Public (97-111)
🪛 golangci-lint (2.2.2)
go/internal/services/keys/get.go

118-118: Error return value is not checked

(errcheck)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
  • GitHub Check: Test Go API Local / Test
  • GitHub Check: Build / Build
  • GitHub Check: Test API / API Test Local
  • GitHub Check: Test Agent Local / test_agent_local
  • GitHub Check: Test Packages / Test
  • GitHub Check: autofix
  • GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (82)
go/apps/api/routes/v2_keys_verify_key/200_test.go (1)

220-223: LGTM: Key association correctly updated for IP whitelist test.

The change correctly associates the test key with the IP whitelisted API (ipWhitelistApi.KeyAuthID.String) instead of the default API. This ensures the test properly validates IP whitelist functionality by using a key that belongs to the same API being verified against.

go/apps/api/routes/v2_permissions_delete_role/handler.go (2)

46-47: LGTM: Authentication updated with deferred emit function.

The authentication flow correctly captures the additional emit function from GetRootKey and defers its execution. This ensures proper cleanup/logging occurs when the handler completes, aligning with the broader authentication refactor.


59-59: LGTM: Root key verification method updated.

The switch from auth.Verify to auth.VerifyRootKey provides more specific verification handling for root keys while maintaining the same permission check logic.

go/apps/api/routes/v2_keys_set_permissions/handler.go (2)

52-53: LGTM: Authentication updated with deferred emit function.

The authentication flow correctly captures and defers the emit function returned by GetRootKey, ensuring proper cleanup/logging occurs when the handler completes. This follows the consistent pattern being applied across API handlers.


65-65: LGTM: Root key verification method updated.

The switch to auth.VerifyRootKey provides specialized verification handling for root keys while preserving the existing permission validation logic for rbac.UpdateKey on API resources.

go/apps/api/routes/v2_identities_create_identity/handler.go (2)

54-55: LGTM: Authentication updated with deferred emit function.

The authentication flow correctly implements the refactored pattern by capturing the emit function from GetRootKey and deferring its execution for proper cleanup/logging when the handler returns.


65-65: LGTM: Root key verification method updated.

The transition to auth.VerifyRootKey provides specialized root key verification while maintaining the appropriate permission check for rbac.CreateIdentity on identity resources.

go/apps/api/routes/v2_permissions_delete_permission/handler.go (2)

44-45: LGTM: Authentication updated with deferred emit function.

The authentication flow correctly implements the refactored pattern by capturing and deferring the emit function returned by GetRootKey, ensuring proper cleanup/logging occurs when the handler completes.


57-57: LGTM: Root key verification method updated.

The switch to auth.VerifyRootKey provides specialized verification for root keys while maintaining the appropriate permission check for rbac.DeletePermission on RBAC resources.

go/apps/api/routes/v2_keys_remove_roles/handler.go (2)

49-50: LGTM: Correctly implements the new GetRootKey interface.

The change properly handles the additional emit function returned by GetRootKey and defers it immediately to ensure cleanup/logging occurs when the handler completes.


77-77: LGTM: Correctly switches to specialized root key verification.

The change from Verify to VerifyRootKey properly uses the new specialized method for root key authorization while maintaining the same permission checks.

go/apps/api/routes/v2_keys_remove_permissions/handler.go (2)

50-51: LGTM: Consistent implementation of new GetRootKey interface.

The changes correctly handle the additional emit function and defer it appropriately for cleanup/logging.


63-63: LGTM: Proper use of specialized root key verification method.

The switch to VerifyRootKey maintains the same authorization logic while using the root key-specific verification method.

go/apps/api/routes/v2_permissions_list_roles/handler.go (2)

44-45: LGTM: Consistent with the new authentication pattern.

The implementation correctly handles the additional emit function from GetRootKey and defers it properly.


60-60: LGTM: Correctly uses specialized root key verification.

The change to VerifyRootKey properly implements the new root key-specific authorization method.

go/apps/api/routes/v2_identities_delete_identity/handler.go (2)

45-46: LGTM: Proper implementation of the updated GetRootKey interface.

The changes correctly handle the new emit function return value and defer it appropriately for cleanup/logging.


57-57: LGTM: Consistent use of specialized root key verification.

The switch to VerifyRootKey properly implements the root key-specific authorization method while maintaining the same permission checks.

go/internal/services/keys/options.go (1)

30-30: LGTM: Minor formatting improvement enhances readability.

The added blank line appropriately separates input validation from configuration assignment in the WithCredits function.

go/apps/api/routes/v2_permissions_create_role/handler.go (2)

50-51: LGTM: Consistent deferred logging pattern implementation

The addition of the emit function return value and its immediate deferral follows the established pattern for this refactoring. The placement ensures proper cleanup/logging execution regardless of how the handler exits.


63-63: LGTM: Specialized root key verification

The switch from Verify to VerifyRootKey provides more specific handling for root key authorization while maintaining the same permission check logic.

go/apps/api/routes/v2_identities_update_identity/handler.go (2)

53-54: LGTM: Consistent implementation of deferred emission pattern

The authentication flow correctly implements the new signature with proper deferral of the emit function.


65-65: LGTM: Consistent root key verification update

The transition to VerifyRootKey maintains the existing permission logic while providing specialized root key handling.

go/apps/api/routes/v2_keys_add_permissions/handler.go (2)

46-47: LGTM: Proper deferred emission implementation

The authentication pattern is consistently applied with correct placement of the defer statement.


59-59: LGTM: Specialized verification method adoption

The use of VerifyRootKey aligns with the refactoring pattern while preserving the permission validation logic.

go/apps/api/routes/v2_permissions_get_role/handler.go (2)

43-44: LGTM: Consistent deferred logging pattern

The authentication flow correctly implements the updated GetRootKey signature with proper emission deferral.


56-56: LGTM: Root key verification consistency

The switch to VerifyRootKey maintains the permission check semantics while providing specialized root key handling.

go/apps/api/routes/v2_permissions_list_permissions/handler.go (2)

44-45: LGTM: Consistent authentication pattern implementation

The handler correctly adopts the new GetRootKey signature with proper defer placement for the emit function.


60-60: LGTM: Completes consistent root key verification refactoring

The final handler correctly adopts VerifyRootKey while preserving the permission list access control logic. This change completes a consistent pattern across all reviewed handlers.

go/apps/api/routes/v2_permissions_get_permission/handler.go (2)

43-44: LGTM: Properly implements deferred emission pattern.

The addition of the emit function capture and immediate deferral follows the expected pattern for the key logging fix. This ensures cleanup/logging actions are executed when the handler completes.


56-56: LGTM: Correct switch to root key-specific verification.

The change from auth.Verify to auth.VerifyRootKey is appropriate for root key verification and aligns with the broader refactor to use more specific verification methods.

go/apps/api/routes/v2_keys_delete_key/handler.go (2)

50-51: LGTM: Consistent implementation of deferred emission pattern.

The changes properly capture and defer the emit function, ensuring cleanup/logging occurs when the handler exits. This matches the pattern established across other handlers.


96-96: LGTM: Appropriate use of root key verification method.

The switch to auth.VerifyRootKey maintains the same permission logic while using the more specific verification method for root keys.

go/apps/api/routes/v2_keys_add_roles/handler.go (2)

48-49: LGTM: Proper deferred emission implementation.

The emit function is correctly captured and deferred, ensuring cleanup/logging actions execute when the handler completes. This implementation is consistent with the refactor pattern.


61-61: LGTM: Correct root key verification usage.

The change to auth.VerifyRootKey appropriately uses the root key-specific verification method while preserving the permission logic for the UpdateKey action.

go/apps/api/routes/v2_keys_update_credits/handler.go (2)

50-51: LGTM: Consistent deferred emission pattern.

The emit function is properly captured and deferred immediately after GetRootKey, ensuring cleanup/logging actions are executed when the handler returns. This aligns with the refactor pattern across all handlers.


105-105: LGTM: Appropriate root key verification method.

The switch to auth.VerifyRootKey correctly uses the root key-specific verification while maintaining the same permission logic for the UpdateKey action.

go/apps/api/routes/v2_keys_set_roles/handler.go (2)

50-51: LGTM: Proper implementation of deferred emission.

The emit function is correctly captured and deferred, ensuring cleanup/logging occurs when the handler exits. This maintains consistency with the refactor pattern applied across all handlers.


63-63: LGTM: Correct use of root key verification.

The change to auth.VerifyRootKey appropriately uses the root key-specific verification method while preserving the permission logic for the UpdateKey action.

go/apps/api/routes/v2_ratelimit_get_override/handler.go (2)

47-48: LGTM! Proper implementation of deferred cleanup/logging.

The addition of the emit function from GetRootKey and its immediate deferral ensures proper cleanup or logging regardless of how the method exits. This follows the expected pattern for resource management.


104-115: LGTM! Appropriate switch to root key-specific verification.

The change from auth.Verify to auth.VerifyRootKey aligns with the specialized verification logic for root keys while maintaining the same permission checks. The RBAC logic correctly uses OR conditions for both specific namespace and wildcard permissions.

go/apps/api/routes/v2_identities_get_identity/handler.go (2)

44-45: LGTM! Consistent implementation of the emit pattern.

The updated GetRootKey signature with the deferred emit function is correctly implemented, ensuring proper cleanup/logging functionality.


104-115: LGTM! Correct root key verification with proper RBAC logic.

The switch to auth.VerifyRootKey is appropriate, and the permission logic correctly allows access with either wildcard (*) or specific identity ID permissions for reading identities.

go/apps/api/routes/v2_ratelimit_limit/handler.go (2)

57-58: LGTM! Proper resource management pattern.

The deferred emit function from the updated GetRootKey signature is correctly implemented for cleanup/logging purposes.


144-155: LGTM! Appropriate verification and permissions for rate limiting.

The change to auth.VerifyRootKey is correct, and the RBAC permissions properly allow rate limiting with either namespace-specific or wildcard (*) access to the Limit action.

go/apps/api/routes/v2_ratelimit_list_overrides/handler.go (2)

40-41: LGTM! Consistent application of the emit pattern.

The updated GetRootKey call with the deferred emit function maintains the established pattern for proper resource cleanup/logging.


70-81: LGTM! Correct root key verification for override access.

The switch to auth.VerifyRootKey is appropriate, and the RBAC logic correctly grants access to rate limit overrides with either namespace-specific or wildcard permissions.

go/apps/api/routes/v2_apis_list_keys/handler.go (2)

48-49: LGTM: Proper implementation of deferred emit function

The changes correctly handle the new emit function returned by GetRootKey, deferring it immediately to ensure cleanup/logging occurs when the method completes.


58-85: LGTM: Consistent migration to VerifyRootKey

The authorization verification calls have been properly updated from auth.Verify to auth.VerifyRootKey, maintaining the same permission logic while using the specialized root key verification method.

Also applies to: 151-162

go/apps/api/routes/v2_keys_get_key/handler.go (2)

52-53: LGTM: Proper deferred cleanup implementation

The new emit function is correctly deferred to ensure cleanup/logging execution at method completion.


115-129: LGTM: Consistent root key verification

The migration from auth.Verify to auth.VerifyRootKey is implemented correctly while preserving the existing permission validation logic.

Also applies to: 156-170

go/apps/api/routes/v2_apis_delete_api/handler.go (2)

48-49: LGTM: Proper implementation of emit function

The deferred emit() call is correctly placed to ensure cleanup/logging occurs when the handler completes.


58-72: LGTM: Root key verification update

The authorization check has been properly updated to use auth.VerifyRootKey while maintaining the same permission logic for API deletion.

go/apps/api/routes/v2_identities_list_identities/handler.go (2)

43-44: LGTM: Proper deferred cleanup

The emit function is correctly deferred to ensure cleanup/logging occurs at method completion.


97-97: LGTM: Consistent verification method update

The permission verification within the identity loop has been properly updated to use auth.VerifyRootKey while maintaining the same permission logic.

go/apps/api/routes/v2_keys_update_key/handler.go (2)

49-50: LGTM: Proper emit function handling

The deferred emit() call is correctly implemented to ensure cleanup/logging occurs when the handler completes.


100-114: LGTM: Root key verification migration

The authorization check has been properly updated to use auth.VerifyRootKey while preserving the existing permission validation logic for key updates.

go/apps/api/routes/v2_apis_create_api/handler.go (2)

42-43: LGTM: Correct implementation of deferred emission pattern.

The addition of the emit function from GetRootKey and its immediate deferral follows the expected pattern for the key logging enhancement.


53-62: LGTM: Proper migration to root key-specific verification.

The change from auth.Verify to auth.VerifyRootKey correctly implements the specialized verification method for root keys while maintaining the same permission logic.

go/apps/api/routes/v2_keys_verify_key/handler.go (4)

52-53: LGTM: Correct implementation of deferred emission for root key.

The GetRootKey method changes are properly implemented with the emit function being deferred immediately after retrieval.


64-65: LGTM: Consistent deferred emission pattern for key retrieval.

The Get method also follows the same deferred emission pattern, ensuring consistent logging/cleanup behavior across key operations.


99-113: LGTM: Proper root key verification with maintained permission logic.

The migration to auth.VerifyRootKey is correctly implemented while preserving the existing RBAC permission checks for key verification.


115-118: LGTM: Improved verification options ordering.

The reordering of verification options (tags before IP whitelist) and removal of the WithApiID option aligns with the service layer changes mentioned in the AI summary.

go/apps/api/routes/v2_permissions_create_permission/handler.go (2)

47-48: LGTM: Correct deferred emission implementation.

The GetRootKey method changes are properly handled with immediate deferral of the emit function.


58-65: LGTM: Proper root key verification for permission creation.

The change to auth.VerifyRootKey correctly implements the specialized verification method while maintaining the RBAC permission check for CreatePermission action.

go/apps/api/routes/v2_apis_get_api/handler.go (2)

41-42: LGTM: Correct deferred emission pattern implementation.

The GetRootKey method changes are properly implemented with the emit function being deferred immediately.


52-66: LGTM: Proper root key verification with maintained RBAC logic.

The migration to auth.VerifyRootKey is correctly implemented while preserving the existing OR-based permission logic for API read access (both wildcard and specific API ID).

go/apps/api/routes/v2_ratelimit_set_override/handler.go (2)

49-50: LGTM: Correct implementation of deferred emission pattern.

The GetRootKey method changes are properly handled with immediate deferral of the emit function.


81-98: LGTM: Proper root key verification with maintained permission logic.

The change to auth.VerifyRootKey correctly implements the specialized verification method while preserving the existing RBAC permission checks for rate limit override operations.

go/apps/api/routes/v2_ratelimit_delete_override/handler.go (2)

47-48: LGTM! Proper handling of the deferred logging function.

The code correctly captures and defers the emit function returned by GetRootKey, ensuring cleanup/logging occurs when the handler completes.


78-78: LGTM! Consistent use of root key-specific verification.

The change from auth.Verify to auth.VerifyRootKey aligns with the broader refactor to use a dedicated verification method for root keys.

go/internal/services/keys/interface.go (1)

13-13: LGTM! Interface updated to support deferred logging/cleanup.

The addition of func() return values to both Get and GetRootKey methods enables proper resource management and logging through deferred execution.

Also applies to: 15-15

go/apps/api/routes/v2_keys_create_key/handler.go (2)

55-56: LGTM! Consistent handling of deferred logging.

The code properly defers the emit function returned by GetRootKey.


68-68: LGTM! Consistent root key verification.

Both permission checks now use auth.VerifyRootKey instead of auth.Verify, maintaining consistency with the new root key verification pattern.

Also applies to: 141-141

go/internal/services/keys/verifier.go (5)

47-47: LGTM! Added tags field for enhanced logging.

The addition of the tags field enables better categorization and tracking of key usage.


55-62: LGTM! Dedicated root key verification method.

The VerifyRootKey method provides a clean separation between root key and regular key verification, returning fault errors specifically for root keys.


81-83: LGTM! Tags are properly assigned before verification checks.

Tags are set early in the verification flow, ensuring they're available for all subsequent operations.


105-110: Verify the order change: credits now applied after rate limits.

The credits verification has been moved to occur after rate limit checks. This changes the order of validation and could affect behavior when both credits and rate limits are configured.

Was this order change intentional? If a key has both rate limits and credits configured, rate limits will now be checked first, potentially rejecting requests before credit checks.


115-138: LGTM! Improved logging method with tags support.

The log method now properly uses the instance's tags field and no longer returns errors, aligning with its role as a deferred cleanup function.

go/internal/services/keys/get.go (4)

22-51: LGTM! GetRootKey properly returns and handles the log function.

The method correctly returns the log function as the second return value and properly sets the workspace ID on both the KeyVerifier and the session.


54-54: LGTM! Empty log function for early returns.

The emptyLog variable provides a safe no-op function for cases where the KeyVerifier isn't fully initialized.


59-186: LGTM! Consistent log function handling in Get method.

The Get method properly returns either emptyLog for early error cases or kv.log for initialized KeyVerifier instances, ensuring a valid log function is always returned.


118-121: Static analysis false positive - error is properly checked.

The static analysis warning about unchecked error at line 118 is incorrect. The error from json.Unmarshal is properly checked at lines 119-121.

@graphite-app
Copy link

graphite-app bot commented Jul 28, 2025

Graphite Automations

"Post a GIF when PR approved" took an action on this PR • (07/28/25)

1 gif was posted to this PR based on Andreas Thomas's automation.

"Notify author when CI fails" took an action on this PR • (07/28/25)

1 teammate was notified to this PR based on Andreas Thomas's automation.

@graphite-app
Copy link

graphite-app bot commented Jul 28, 2025

Merge activity

  • Jul 28, 12:17 PM UTC: This pull request can not be added to the Graphite merge queue. Please try rebasing and resubmitting to merge when ready.
  • Jul 28, 12:17 PM UTC: Graphite disabled "merge when ready" on this PR due to: a merge conflict with the target branch; resolve the conflict and try again..

@Flo4604 Flo4604 enabled auto-merge July 28, 2025 19:41
@Flo4604 Flo4604 added this pull request to the merge queue Jul 28, 2025
Merged via the queue into main with commit c4508fc Jul 28, 2025
19 of 24 checks passed
@Flo4604 Flo4604 deleted the fix/key-logging branch July 28, 2025 20:49
@coderabbitai coderabbitai bot mentioned this pull request Aug 11, 2025
18 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Refactor: Separate root key auth logic from regular key auth in keys service Ensure disabled regular keys are logged during key verification

2 participants