Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion app/components/DS/menu_item.rb
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,8 @@ def merged_opts
data = merged_opts.delete(:data) || {}

if confirm.present?
data = data.merge(turbo_confirm: confirm.to_data_attribute)
confirm_value = confirm.respond_to?(:to_data_attribute) ? confirm.to_data_attribute : confirm
data = data.merge(turbo_confirm: confirm_value)
end

if frame.present?
Expand Down
5 changes: 5 additions & 0 deletions app/controllers/rules_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,11 @@ def destroy_all
redirect_to rules_path, notice: "All rules deleted"
end

def clear_ai_cache
ClearAiCacheJob.perform_later
redirect_to rules_path, notice: "AI cache is being cleared. This may take a few moments."
end
Comment on lines +81 to +84
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

LGTM! Consider verifying authorization.

The action correctly enqueues the background job and provides clear user feedback. Given the PR description warns this is an "aggressive feature" with potential cost implications, verify that appropriate authorization checks are in place (either at the application controller level or through the UI confirmation).

Run the following script to check if ApplicationController has authorization checks:


🏁 Script executed:

#!/bin/bash
# Description: Verify authorization setup in ApplicationController

# Check ApplicationController for authentication/authorization
rg -n -A10 'class ApplicationController' app/controllers/application_controller.rb

# Check if RulesController has additional authorization
rg -n 'before_action|authorize|authenticate' app/controllers/rules_controller.rb

Length of output: 501



🏁 Script executed:

#!/bin/bash
# Check for clear_ai_cache authorization and method definition
rg -n -B5 -A10 'def clear_ai_cache' app/controllers/rules_controller.rb

# Check for authorization patterns in RulesController (policy, authorize, etc.)
rg -n 'authorize|policy|pundit|cancan|skip_before_action' app/controllers/rules_controller.rb

# Check Authentication module to understand what it provides
fd -t f 'authentication' app/concerns/ --exec cat {}

Length of output: 531


🏁 Script executed:

#!/bin/bash
# Check Authentication module implementation
fd -t f 'authentication.rb' app/concerns/ --exec cat {}

# Check if clear_ai_cache has explicit before_action or authorization
rg -n -B2 'clear_ai_cache' app/controllers/rules_controller.rb

# Check FeatureGuardable for clear_ai_cache patterns
rg -n 'FeatureGuardable|feature_guard|feature_flag' app/concerns/ app/controllers/

Length of output: 347


🏁 Script executed:

#!/bin/bash
# Find and display Authentication concern
fd -type f -name '*authentication*' app/concerns/ --exec cat {}

# Check full context around clear_ai_cache definition
rg -n -B5 -A15 'def clear_ai_cache' app/controllers/rules_controller.rb

# Check if routes file has any special handling for clear_ai_cache
rg -n 'clear_ai_cache' config/routes.rb

# Check FeatureGuardable concern
fd -type f -name '*feature_guardable*' app/concerns/ --exec cat {}

Length of output: 1098



🏁 Script executed:

#!/bin/bash
# Display FeatureGuardable concern
cat app/controllers/concerns/feature_guardable.rb

# Find and display Authentication concern (likely in app/controllers/concerns/)
fd 'authentication' app/controllers/concerns/ -x cat {}

# Check for any skip_before_action or explicit authorization in full RulesController
rg -n 'skip_before_action|authorize|private.*def' app/controllers/rules_controller.rb | head -20

# Check ClearAiCacheJob for authorization
fd 'clear_ai_cache_job' -x cat {}

Length of output: 2572


🏁 Script executed:

#!/bin/bash
# Display FeatureGuardable concern
cat app/controllers/concerns/feature_guardable.rb

# Find and display Authentication concern
fd 'authentication' app/controllers/concerns/ -x cat {}

# Check for skip_before_action patterns in RulesController
rg -n 'skip_before_action' app/controllers/rules_controller.rb

# Check ClearAiCacheJob for any authorization
fd 'clear_ai_cache_job' -x cat {}

# Check if there's a policy file for RulesController (Pundit pattern)
fd 'rules_policy|rule_policy' -x cat {}

Length of output: 2572


Add authorization check to prevent unauthorized cache clearing.

The action is only protected by authentication (via the Authentication module), meaning any authenticated user can trigger a global cache clear affecting all users. For an "aggressive feature" with cost implications, this needs explicit authorization:

  • ClearAiCacheJob clears cache globally for all transactions and entries (not scoped to Current.family like other actions)
  • No role check, feature flag guard, or permission control
  • Unlike destroy_all (line 76-79, also risky), this has no visibility that it's system-wide

Add authorize :rule, :clear_ai_cache? (if using Pundit) or implement a before_action to verify admin/appropriate permissions before clearing the global cache.

🤖 Prompt for AI Agents
In app/controllers/rules_controller.rb around lines 81 to 84, the clear_ai_cache
action lacks an authorization check allowing any authenticated user to clear the
global AI cache; add an explicit authorization before allowing this destructive,
system-wide operation—either call authorize :rule, :clear_ai_cache? (Pundit) at
the start of the action or add a before_action (e.g., ensure_admin or
authorize_clear_ai_cache) that verifies the current user has admin/permission to
clear global caches, and only then invoke ClearAiCacheJob and redirect; ensure
the authorization uses the same policy/role checks used elsewhere in the
controller for consistency.


private
def set_rule
@rule = Current.family.rules.find(params[:id])
Expand Down
15 changes: 15 additions & 0 deletions app/jobs/clear_ai_cache_job.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
class ClearAiCacheJob < ApplicationJob
queue_as :low_priority

def perform
Rails.logger.info("Clearing AI cache for all transactions and entries")

# Clear AI cache for all transactions
Transaction.clear_ai_cache
Rails.logger.info("Cleared AI cache for transactions")

# Clear AI cache for all entries
Entry.clear_ai_cache
Rails.logger.info("Cleared AI cache for entries")
end
end
52 changes: 52 additions & 0 deletions app/models/concerns/enrichable.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,46 @@ module Enrichable
InvalidAttributeError = Class.new(StandardError)

included do
has_many :data_enrichments, as: :enrichable, dependent: :destroy

scope :enrichable, ->(attrs) {
attrs = Array(attrs).map(&:to_s)
json_condition = attrs.each_with_object({}) { |attr, hash| hash[attr] = true }
where.not(Arel.sql("#{table_name}.locked_attributes ?| array[:keys]"), keys: attrs)
}
end

class_methods do
# Clear AI-sourced locked attributes for all records
def clear_ai_cache
transaction do
# Find all AI enrichments for this model
ai_enrichments = DataEnrichment.where(enrichable_type: name, source: "ai")

# Get all enrichable_ids and load records in one query
enrichable_ids = ai_enrichments.distinct.pluck(:enrichable_id)
records = where(id: enrichable_ids).index_by(&:id)
enrichments_by_id = ai_enrichments.group_by(&:enrichable_id)

enrichments_by_id.each do |enrichable_id, enrichments|
record = records[enrichable_id]
next unless record

# Unlock all AI-locked attributes
new_locked_attributes = record.locked_attributes.dup
enrichments.each do |enrichment|
new_locked_attributes.delete(enrichment.attribute_name)
end

record.update_column(:locked_attributes, new_locked_attributes)
end

# Delete all AI enrichment records
ai_enrichments.delete_all
end
end
end

# Convenience method for a single attribute
def enrich_attribute(attr, value, source:, metadata: {})
enrich_attributes({ attr => value }, source:, metadata:)
Expand Down Expand Up @@ -72,6 +105,25 @@ def lock_saved_attributes!
end
end

# Clear AI-sourced locked attributes for this record
def clear_ai_cache
self.class.transaction do
# Find all AI enrichments for this record
ai_enrichments = data_enrichments.where(source: "ai")

# Unlock all AI-locked attributes
new_locked_attributes = locked_attributes.dup
ai_enrichments.each do |enrichment|
new_locked_attributes.delete(enrichment.attribute_name)
end

update_column(:locked_attributes, new_locked_attributes)

# Delete all AI enrichment records
ai_enrichments.delete_all
end
end

private
def log_enrichment(attribute_name:, attribute_value:, source:, metadata: {})
de = DataEnrichment.find_or_create_by(
Expand Down
3 changes: 1 addition & 2 deletions app/models/family/auto_categorizer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,8 @@ def auto_categorize
category_id,
source: "ai"
)
transaction.lock_attr!(:category_id)
end

transaction.lock_attr!(:category_id)
end
end

Expand Down
6 changes: 2 additions & 4 deletions app/models/family/auto_merchant_detector.rb
Original file line number Diff line number Diff line change
Expand Up @@ -50,11 +50,9 @@ def auto_detect
merchant_id,
source: "ai"
)

# We lock the attribute so that this Rule doesn't try to run again
transaction.lock_attr!(:merchant_id)
end

# We lock the attribute so that this Rule doesn't try to run again
transaction.lock_attr!(:merchant_id)
end
end

Expand Down
12 changes: 12 additions & 0 deletions app/models/setting.rb
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ def self.validate_onboarding_state!(state)
class << self
alias_method :raw_onboarding_state, :onboarding_state
alias_method :raw_onboarding_state=, :onboarding_state=
alias_method :raw_openai_model=, :openai_model=

def onboarding_state
value = raw_onboarding_state
Expand All @@ -49,6 +50,17 @@ def onboarding_state=(state)
self.raw_onboarding_state = state
end

def openai_model=(value)
old_value = openai_model
self.raw_openai_model = value

# Clear AI cache when model changes
if old_value != value && old_value.present?
Rails.logger.info("OpenAI model changed from #{old_value} to #{value}, clearing AI cache")
ClearAiCacheJob.perform_later
end
end

# Support dynamic field access via bracket notation
# First checks if it's a declared field, then falls back to dynamic_fields hash
def [](key)
Expand Down
11 changes: 11 additions & 0 deletions app/views/rules/index.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,17 @@
<div class="flex items-center gap-2">
<% if @rules.any? %>
<%= render DS::Menu.new do |menu| %>
<% menu.with_item(
variant: "button",
text: "Reset AI cache",
href: clear_ai_cache_rules_path,
icon: "refresh-cw",
method: :post,
confirm: CustomConfirm.new(
title: "Reset AI cache?",
body: "Are you sure you want to reset the AI cache? This will allow AI rules to re-process all transactions.",
btn_text: "Reset Cache"
)) %>
<% menu.with_item(
variant: "button",
text: "Delete all rules",
Expand Down
1 change: 1 addition & 0 deletions config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,7 @@

collection do
delete :destroy_all
post :clear_ai_cache
end
end

Expand Down
5 changes: 3 additions & 2 deletions test/models/family/auto_categorizer_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,9 @@ class Family::AutoCategorizerTest < ActiveSupport::TestCase
assert_equal test_category, txn2.reload.category
assert_nil txn3.reload.category

# After auto-categorization, all transactions are locked and no longer enrichable
assert_equal 0, @account.transactions.reload.enrichable(:category_id).count
# After auto-categorization, only successfully categorized transactions are locked
# txn3 should still be enrichable since it wasn't categorized
assert_equal 1, @account.transactions.reload.enrichable(:category_id).count
end

private
Expand Down
5 changes: 3 additions & 2 deletions test/models/family/auto_merchant_detector_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,9 @@ class Family::AutoMerchantDetectorTest < ActiveSupport::TestCase
assert_equal "https://cdn.brandfetch.io/chipotle.com/icon/fallback/lettermark/w/40/h/40?c=123", txn2.reload.merchant.logo_url
assert_nil txn3.reload.merchant

# After auto-detection, all transactions are locked and no longer enrichable
assert_equal 0, @account.transactions.reload.enrichable(:merchant_id).count
# After auto-detection, only successfully detected merchants are locked
# txn3 should still be enrichable since no merchant was detected
assert_equal 1, @account.transactions.reload.enrichable(:merchant_id).count
end

private
Expand Down