fix: validate OpenAI hook credentials (#14068)

# Pull Request Template

## Description

- Validates openai key while configuring hooks
- added backfill logic

Fixes # (issue)

## Type of change

- [x] New feature (non-breaking change which adds functionality)


## How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide
instructions so we can reproduce. Please also list any relevant details
for your test configuration.
locally

<img width="1710" height="1234" alt="CleanShot 2026-04-15 at 16 15
02@2x"
src="https://github.com/user-attachments/assets/3d319fe0-19f9-4fd0-9308-74987daac2e1"
/>

<img width="2884" height="1136" alt="CleanShot 2026-05-11 at 19 22
53@2x"
src="https://github.com/user-attachments/assets/5eae8650-985b-4c4a-af42-35f7175ff52d"
/>



## Checklist:

- [x] My code follows the style guidelines of this project
- [x] I have performed a self-review of my code
- [x] I have commented on my code, particularly in hard-to-understand
areas
- [ ] I have made corresponding changes to the documentation
- [x] My changes generate no new warnings
- [x] I have added tests that prove my fix is effective or that my
feature works
- [x] New and existing unit tests pass locally with my changes
- [x] Any dependent changes have been merged and published in downstream
modules

---------

Co-authored-by: Vishnu Narayanan <iamwishnu@gmail.com>
This commit is contained in:
Aakash Bakhle
2026-05-18 14:08:57 +05:30
committed by GitHub
co-authored by Vishnu Narayanan
parent 059d840272
commit 3253e863ed
21 changed files with 299 additions and 4 deletions
@@ -3,6 +3,8 @@ require 'rails_helper'
RSpec.describe 'Integration Apps API', type: :request do
let(:account) { create(:account) }
before { allow(Integrations::Openai::KeyValidator).to receive(:valid?).and_return(true) }
describe 'GET /api/v1/integrations/apps' do
context 'when it is an unauthenticated user' do
it 'returns unauthorized' do
@@ -15,6 +15,7 @@ RSpec.describe Captain::ConversationCompletionService do
allow(mock_chat).to receive(:with_schema).and_return(mock_chat)
allow(account).to receive(:feature_enabled?).and_call_original
allow(account).to receive(:feature_enabled?).with('captain_tasks').and_return(true)
allow(Integrations::Openai::KeyValidator).to receive(:valid?).and_return(true)
end
describe '#perform' do
@@ -0,0 +1,58 @@
require 'rails_helper'
RSpec.describe Migration::ValidateOpenaiHooksJob do
let(:integrations_mailer) { instance_double(AdministratorNotifications::IntegrationsNotificationMailer) }
let(:mailer_response) { instance_double(ActionMailer::MessageDelivery, deliver_later: true) }
before do
allow(Integrations::Openai::KeyValidator).to receive(:valid?).and_return(true)
allow(AdministratorNotifications::IntegrationsNotificationMailer).to receive(:with).and_return(integrations_mailer)
allow(integrations_mailer).to receive(:openai_disconnect).and_return(mailer_response)
end
def create_openai_hook(account:, api_key: 'sk-good')
create(:integrations_hook, :openai, account: account, settings: { 'api_key' => api_key })
end
it 'destroys invalid hooks, preserves valid ones, sends disconnect email, and reports stats' do
account_a = create(:account)
account_b = create(:account)
valid_hook = create_openai_hook(account: account_a, api_key: 'sk-good')
invalid_hook = create_openai_hook(account: account_b, api_key: 'sk-bad')
allow(Integrations::Openai::KeyValidator).to receive(:valid?).with('sk-bad').and_return(false)
result = described_class.perform_now
expect(valid_hook.reload).to be_enabled
expect { invalid_hook.reload }.to raise_error(ActiveRecord::RecordNotFound)
expect(AdministratorNotifications::IntegrationsNotificationMailer).to have_received(:with).with(account: account_b)
expect(result).to eq(checked: 2, destroyed: 1)
end
it 'scopes to a specific account when provided' do
account_a = create(:account)
account_b = create(:account)
hook_a = create_openai_hook(account: account_a, api_key: 'sk-bad')
hook_b = create_openai_hook(account: account_b, api_key: 'sk-bad')
allow(Integrations::Openai::KeyValidator).to receive(:valid?).with('sk-bad').and_return(false)
described_class.perform_now(account: account_a)
expect { hook_a.reload }.to raise_error(ActiveRecord::RecordNotFound)
expect(hook_b.reload).to be_enabled
end
it 'only checks enabled OpenAI hooks' do
account = create(:account)
slack_hook = create(:integrations_hook, account: account, app_id: 'slack')
disabled_hook = create_openai_hook(account: account)
disabled_hook.disable
allow(Integrations::Openai::KeyValidator).to receive(:valid?).and_return(false)
described_class.perform_now
expect(slack_hook.reload).to be_enabled
expect(disabled_hook.reload).to be_disabled # still disabled, not re-checked
end
end
@@ -26,6 +26,7 @@ RSpec.describe Captain::BaseTaskService do
# without enterprise module interference
allow(account).to receive(:feature_enabled?).and_call_original
allow(account).to receive(:feature_enabled?).with('captain_tasks').and_return(true)
allow(Integrations::Openai::KeyValidator).to receive(:valid?).and_return(true)
end
describe '#perform' do
@@ -10,6 +10,8 @@ RSpec.describe Integrations::LlmBaseService do
let(:error) { StandardError.new('API Error') }
let(:body) { { model: 'gpt-4', messages: [{ role: 'user', content: 'Hello' }] }.to_json }
before { allow(Integrations::Openai::KeyValidator).to receive(:valid?).and_return(true) }
describe '#make_api_call' do
before do
allow(service).to receive(:instrument_llm_call).and_yield
@@ -0,0 +1,41 @@
require 'rails_helper'
RSpec.describe Integrations::Openai::KeyValidator do
let(:api_key) { 'sk-test-valid-key-123456789' }
let(:probe_url) { 'https://api.openai.com/v1/models' }
it 'accepts keys that OpenAI recognizes' do
stub_request(:get, probe_url).to_return(status: 200)
expect(described_class.valid?(api_key)).to be true
end
it 'rejects keys that OpenAI does not recognize' do
stub_request(:get, probe_url).to_return(status: 401)
expect(described_class.valid?(api_key)).to be false
end
it 'rejects blank keys without making a network call' do
expect(described_class.valid?(nil)).to be false
expect(described_class.valid?('')).to be false
end
it 'treats transient failures as valid to avoid blocking saves' do
stub_request(:get, probe_url).to_return(status: 500)
expect(described_class.valid?(api_key)).to be true
stub_request(:get, probe_url).to_timeout
expect(described_class.valid?(api_key)).to be true
end
it 'routes the probe through the configured endpoint' do
custom_url = 'https://proxy.example.com/v1/models'
allow(InstallationConfig).to receive(:find_by).with(name: 'CAPTAIN_OPEN_AI_ENDPOINT')
.and_return(instance_double(InstallationConfig, value: 'https://proxy.example.com/'))
stub_request(:get, custom_url).to_return(status: 200)
described_class.valid?(api_key)
expect(WebMock).to have_requested(:get, custom_url)
expect(WebMock).not_to have_requested(:get, probe_url)
end
end
@@ -39,4 +39,20 @@ RSpec.describe AdministratorNotifications::IntegrationsNotificationMailer do
expect(mail.to).to contain_exactly(administrator.email, another_administrator.email)
end
end
describe 'openai_disconnect' do
let(:mail) { described_class.with(account: account).openai_disconnect.deliver_now }
it 'renders the subject' do
expect(mail.subject).to eq('Your OpenAI integration was disconnected')
end
it 'renders the content' do
expect(mail.body.encoded).to include('the configured API key is invalid or revoked')
end
it 'renders the receiver email' do
expect(mail.to).to contain_exactly(administrator.email, another_administrator.email)
end
end
end
+2
View File
@@ -5,6 +5,8 @@ RSpec.describe Integrations::App do
let(:app) { apps.find(id: app_name) }
let(:account) { create(:account) }
before { allow(Integrations::Openai::KeyValidator).to receive(:valid?).and_return(true) }
describe '#name' do
let(:app_name) { 'slack' }
+66
View File
@@ -111,4 +111,70 @@ RSpec.describe Integrations::Hook do
end
end
end
describe 'openai api key validation' do
let(:account) { create(:account) }
it 'prevents saving an openai hook with an invalid key' do
allow(Integrations::Openai::KeyValidator).to receive(:valid?).and_return(false)
hook = build(:integrations_hook, :openai, account: account, settings: { 'api_key' => 'sk-bad' })
expect(hook).not_to be_valid
expect(hook.errors[:base]).to include(I18n.t('errors.openai.invalid_api_key'))
end
it 'prevents saving an openai hook with a blank key' do
hook = build(:integrations_hook, :openai, account: account, settings: { 'api_key' => '' })
expect(hook).not_to be_valid
end
it 'allows saving an openai hook with a valid key' do
allow(Integrations::Openai::KeyValidator).to receive(:valid?).and_return(true)
hook = build(:integrations_hook, :openai, account: account, settings: { 'api_key' => 'sk-good' })
expect(hook).to be_valid
end
it 'skips validation when an enabled openai hook is saved without changing the api key' do
allow(Integrations::Openai::KeyValidator).to receive(:valid?).and_return(true)
hook = create(:integrations_hook, :openai, account: account, settings: { 'api_key' => 'sk-good', 'label_suggestion' => false })
allow(Integrations::Openai::KeyValidator).to receive(:valid?).and_return(false)
hook.settings['label_suggestion'] = true
expect(hook.save).to be true
end
it 'validates when a disabled openai hook is re-enabled' do
allow(Integrations::Openai::KeyValidator).to receive(:valid?).and_return(true)
hook = create(:integrations_hook, :openai, account: account, settings: { 'api_key' => 'sk-bad' })
hook.update!(status: :disabled)
allow(Integrations::Openai::KeyValidator).to receive(:valid?).with('sk-bad').and_return(false)
expect(hook.update(status: :enabled)).to be false
expect(hook.errors[:base]).to include(I18n.t('errors.openai.invalid_api_key'))
end
it 'skips validation for disabled hooks' do
allow(Integrations::Openai::KeyValidator).to receive(:valid?).and_return(true)
hook = create(:integrations_hook, :openai, account: account, settings: { 'api_key' => 'sk-good' })
# Even with validator returning false, disable succeeds because disabled hooks skip validation
allow(Integrations::Openai::KeyValidator).to receive(:valid?).and_return(false)
hook.disable
expect(hook.reload).to be_disabled
end
it 'does not validate keys for non-openai hooks' do
allow(Integrations::Openai::KeyValidator).to receive(:valid?).and_return(false)
hook = build(:integrations_hook, account: account, app_id: 'slack')
expect(hook).to be_valid
end
end
end