:feat: add public API key handling in CloneRequestPresenter and ensur…

[kipsang01]

…e its creation in CloneRequestProvisioner

[qodo-code-review]

ⓘ You've reached your Qodo monthly free-tier limit. Reviews pause until next month — upgrade your plan to continue now, or link your paid account if you already have one.

[willymwai]

/agentic_review

[willymwai]

/agentic_describe

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (0)

1. Publishable token auth bypass 🐞 Bug ⛨ Security

Description

CloneRequestPresenter now returns public_api_key.token in the clone-store API response, but
CloneStoresController’s X-Spree-Api-Key path authenticates any Spree::ApiKey by token_digest
without validating key_type. If publishable keys share the same lookup mechanism, the exposed
publishable token can be replayed in X-Spree-Api-Key to access privileged clone endpoints without
OAuth.

Code

app/services/spree/olitt/clone_store/clone_request_presenter.rb[R50-56]

+          {
+            id: api_key.id,
+            name: api_key.name,
+            key_type: api_key.key_type,
+            token: api_key.token,
+            store_id: api_key.store_id
          }

Evidence

The presenter explicitly returns the API key token in meta.public_api_key, and the platform
controller uses X-Spree-Api-Key to bypass OAuth and authenticates by digest lookup without
checking key type. This creates a direct path for replaying the returned token as an authentication
credential.

app/services/spree/olitt/clone_store/clone_request_presenter.rb[34-56]
app/controllers/spree/api/v2/platform/clone_stores_controller.rb[29-37]
app/controllers/spree/api/v2/platform/clone_stores_controller.rb[62-69]
app/controllers/spree/olitt/clone_store/clone_store_controller.rb[158-163]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The API response now includes a raw API key token (`public_api_key.token`). The clone-store platform controller uses `X-Spree-Api-Key` to bypass OAuth and looks up keys by digest without checking that the key is a *secret* key (vs publishable), enabling possible privilege escalation if publishable keys are stored in the same model/table.

## Issue Context
- `CloneRequestPresenter` serializes and returns `api_key.token`.
- `CloneStoresController#authorize_clone_store_request!` chooses the secret-key auth path when the header is present.
- `CloneStoresController#authenticate_secret_key!` finds by token digest and only checks store ownership, not key type.

## Fix Focus Areas
- app/controllers/spree/api/v2/platform/clone_stores_controller.rb[29-73]
- app/services/spree/olitt/clone_store/clone_request_presenter.rb[46-56]

## Suggested fix
1. In `authenticate_secret_key!`, reject non-secret keys (e.g., `return unauthorized if @current_api_key.key_type == 'publishable'`), or enforce a dedicated “secret” type.
2. Optionally, if publishable tokens must be returned, ensure they can never satisfy the secret-key lookup (different digest column, different table, or separate auth header).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. API key errors unlogged 🐞 Bug ◔ Observability

Description

CloneRequestPresenter#public_api_key rescues StandardError and returns nil without logging, so
unexpected failures (query/scope/runtime) silently degrade responses to public_api_key: null. This
makes production debugging harder when clients rely on the key being present.

Code

app/services/spree/olitt/clone_store/clone_request_presenter.rb[R73-80]

+        def public_api_key
+          store = @clone_request.store
+          return nil if store.blank? || !store.respond_to?(:api_keys)
+
+          store.api_keys.active.publishable.order(created_at: :desc).first
+        rescue StandardError
+          nil
+        end

Evidence

The method catches all StandardError and returns nil, with no logging statements in the rescue
branch, so failures become silent nulls in the API response.

app/services/spree/olitt/clone_store/clone_request_presenter.rb[73-80]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`public_api_key` swallows all `StandardError` exceptions and returns `nil` without any log/telemetry. When `public_api_key` unexpectedly becomes `null`, there is no server-side breadcrumb to diagnose why.

## Issue Context
The method is used to populate `meta.public_api_key` in API responses.

## Fix Focus Areas
- app/services/spree/olitt/clone_store/clone_request_presenter.rb[73-80]

## Suggested fix
- Rescue more narrowly (e.g., `NoMethodError`, `ActiveRecord::StatementInvalid`) and log a `warn`/`error` including `clone_request_id`/`store_id` and the exception class/message.
- Keep returning `nil` for backward compatibility, but add observability so missing keys are diagnosable.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

[qodo-code-review]

Review Summary by Qodo

Add public API key handling to clone store provisioning

✨ Enhancement

Walkthroughs

Description

• Add public API key handling to CloneRequestPresenter metadata
• Implement automatic public API key creation in CloneRequestProvisioner
• Include API key details in clone request presentation payload
• Add comprehensive test coverage for new functionality

Diagram

flowchart LR
  A["CloneRequestProvisioner"] -->|"ensure_public_api_key!"| B["Store API Keys"]
  B -->|"create if missing"| C["Publishable API Key"]
  D["CloneRequestPresenter"] -->|"public_api_key_metadata"| C
  C -->|"included in metadata"| E["JSON Payload"]

Loading

File Changes

1. app/services/spree/olitt/clone_store/clone_request_presenter.rb ✨ Enhancement +24/-1

Add public API key metadata extraction

• Added public_api_key_metadata method to extract API key details
• Added public_api_key method to retrieve active publishable API key from store
• Included public_api_key in metadata hash returned by metadata method
• Implemented error handling with rescue clause for API key retrieval

app/services/spree/olitt/clone_store/clone_request_presenter.rb

---

2. app/services/spree/olitt/clone_store/clone_request_provisioner.rb ✨ Enhancement +8/-0

Ensure public API key creation during provisioning

• Added ensure_public_api_key! method to create or retrieve publishable API key
• Integrated API key creation into provisioning workflow after store save
• Implemented fallback logic to create default 'Storefront key' if none exists
• Added safety check for store API keys capability

app/services/spree/olitt/clone_store/clone_request_provisioner.rb

---

3. spec/services/spree/olitt/clone_store/clone_request_presenter_spec.rb 🧪 Tests +19/-1

Add public API key tests to presenter spec

• Added mock public_api_key instance with realistic attributes
• Updated clone_request mock to include store with api_keys relation
• Added message chain stub for active, publishable, order, first methods
• Extended test expectations to verify public_api_key in metadata payload

spec/services/spree/olitt/clone_store/clone_request_presenter_spec.rb

---

View more (1)

4. spec/services/spree/olitt/clone_store/clone_request_provisioner_spec.rb 🧪 Tests +1/-0

Add API key creation verification to provisioner spec

• Added assertion to verify at least one active publishable API key exists
• Validates that provisioning process creates or retrieves API key successfully

spec/services/spree/olitt/clone_store/clone_request_provisioner_spec.rb

---