Revocable API Tokens

[matmanna]

This PR will be landed prior to but alongside a corresponding one for the revoker.

This PR adds:

• additionally functionality to the /api/internal/revoke endpoint to distinguish between admin and normal user keys in order to roll exposed user tokens
• properly documents HKA_REVOCATION_KEY in .env.example
• new revocations controller tests for regular keys

[greptile-apps]

Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.

[matmanna]

hmm is it possible for me to get a @greptileai review

[greptile-apps]

Greptile Summary

This PR extends the internal revocation endpoint to support revoking regular user ApiKeys (via token-rolling) in addition to the existing AdminApiKey revocation (via revoked_at timestamp). A regex-based dispatcher distinguishes key types, error handling has been hardened with a non-raising update + rescue ActiveRecord::ActiveRecordError, and a TODO comment pre-emptively flags the missing active scope on ApiKey. The previously undocumented HKA_REVOCATION_KEY env var is now documented in .env.example.

• Regex constants (ADMIN_KEY_REGEX, REGULAR_KEY_REGEX) cleanly separate the two key formats with no overlap risk.
• revoke_key! correctly uses update (non-bang) for ApiKey and wraps AdminApiKey#revoke! (which uses update!) in a shared rescue clause — all previously flagged concerns about unhandled exceptions are addressed.
• A new test file provides thorough coverage: happy-path token rolling, non-existent token, unrecognised format, simulated DB collision failure (via a SecureRandom.uuid_v4 stub), and an admin-key regression test.
• Minor: the key_name field in the success response returns the post-mutation name (e.g., "Desktop_revoked_abc12345") rather than the original name; this is consistent with the pre-existing admin-key behaviour but may affect how the revoker service formats user-facing notifications.

Confidence Score: 4/5

• This PR is safe to merge; the new code paths are well-tested and error-handling is solid.
• All three previously flagged concerns (missing active-scope guard, missing tests, unhandled update! exception) have been successfully addressed in the code. The TODO comment appropriately flags the future need for an active scope on ApiKey, the test file provides comprehensive coverage of both happy paths and edge cases, and the error handler properly rescues ActiveRecord::ActiveRecordError. The only remaining finding is a design consideration regarding the post-mutation key_name in the response, which is consistent with pre-existing behavior but may merit discussion with the revoker service maintainers.
• No files require special attention; app/controllers/api/internal/revocations_controller.rb is the most critical but is well-implemented.

Sequence Diagram

sequenceDiagram
    participant Revoker as Revoker Service
    participant RC as RevocationsController
    participant DB as Database

    Revoker->>RC: POST /api/internal/revoke (Bearer auth)
    RC->>RC: authenticate! via secure_compare

    alt Matches ADMIN_KEY_REGEX
        RC->>DB: AdminApiKey.active.find_by(key)
        DB-->>RC: admin_key or nil
        RC->>DB: admin_key.revoke! sets revoked_at and renames
        DB-->>RC: ok or raises
    else Matches REGULAR_KEY_REGEX
        RC->>DB: ApiKey.find_by(key)
        DB-->>RC: api_key or nil
        RC->>DB: api_key.update rolls uuid and renames
        DB-->>RC: true or false
    else No regex match
        RC-->>Revoker: success false
    end

    alt Not found or failed
        RC-->>Revoker: success false
    else Succeeded
        RC-->>Revoker: success true plus owner_email and key_name
    end

Loading

_{Last reviewed commit: 2fb2aff}

[matmanna]

@greptileai re-review pls?

[greptile-apps]

key_name reflects post-mutation name

After revoke_key! runs, key is already updated in memory — for regular ApiKeys the name becomes "Desktop_revoked_abc12345", and for AdminApiKeys the same applies via revoke!. So the key_name field in the success response always carries the mangled, post-revocation name rather than the human-readable original.

This behaviour was already present for the admin-key path before this PR, so it's consistent, but any downstream consumer (e.g. the revoker service notification) will see "Desktop_revoked_abc12345" instead of "Desktop". If the revoker uses key_name to build a user-facing message, consider capturing the original name before calling revoke_key!:

original_key_name = key.name

return render json: { success: false } unless revoke_key!(key)

render json: {
  success: true,
  owner_email: user.email_addresses.first&.email,
  key_name: original_key_name
}.compact_blank

Prompt To Fix With AI

This is a comment left during a code review.
Path: app/controllers/api/internal/revocations_controller.rb
Line: 17-21

Comment:
`key_name` reflects post-mutation name

After `revoke_key!` runs, `key` is already updated in memory — for regular `ApiKey`s the name becomes `"Desktop_revoked_abc12345"`, and for `AdminApiKey`s the same applies via `revoke!`. So the `key_name` field in the success response always carries the mangled, post-revocation name rather than the human-readable original.

This behaviour was already present for the admin-key path before this PR, so it's consistent, but any downstream consumer (e.g. the revoker service notification) will see `"Desktop_revoked_abc12345"` instead of `"Desktop"`. If the revoker uses `key_name` to build a user-facing message, consider capturing the original name before calling `revoke_key!`:

```ruby
original_key_name = key.name

return render json: { success: false } unless revoke_key!(key)

render json: {
  success: true,
  owner_email: user.email_addresses.first&.email,
  key_name: original_key_name
}.compact_blank
```

How can I resolve this? If you propose a fix, please make it concise.

[matmanna]

@skyfallwastaken what do you think the best behavior would be in regards to this?