From 6150e1e1189f344e9d06ab61f553d205103d37fb Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Fri, 1 May 2026 16:49:28 -0600 Subject: [PATCH 01/10] feat(api): expose sync status --- app/controllers/api/v1/syncs_controller.rb | 92 ++++++++ app/views/api/v1/syncs/_sync.json.jbuilder | 24 ++ app/views/api/v1/syncs/index.json.jbuilder | 12 + app/views/api/v1/syncs/show.json.jbuilder | 9 + config/routes.rb | 3 + docs/api/openapi.yaml | 206 ++++++++++++++++++ spec/requests/api/v1/syncs_spec.rb | 106 +++++++++ spec/swagger_helper.rb | 69 ++++++ .../api/v1/syncs_controller_test.rb | 120 ++++++++++ 9 files changed, 641 insertions(+) create mode 100644 app/controllers/api/v1/syncs_controller.rb create mode 100644 app/views/api/v1/syncs/_sync.json.jbuilder create mode 100644 app/views/api/v1/syncs/index.json.jbuilder create mode 100644 app/views/api/v1/syncs/show.json.jbuilder create mode 100644 spec/requests/api/v1/syncs_spec.rb create mode 100644 test/controllers/api/v1/syncs_controller_test.rb diff --git a/app/controllers/api/v1/syncs_controller.rb b/app/controllers/api/v1/syncs_controller.rb new file mode 100644 index 000000000..4c68afea6 --- /dev/null +++ b/app/controllers/api/v1/syncs_controller.rb @@ -0,0 +1,92 @@ +# frozen_string_literal: true + +class Api::V1::SyncsController < Api::V1::BaseController + include Pagy::Backend + + SYNCABLE_ASSOCIATIONS = { + "Account" => :accounts, + "PlaidItem" => :plaid_items, + "SimplefinItem" => :simplefin_items, + "LunchflowItem" => :lunchflow_items, + "EnableBankingItem" => :enable_banking_items, + "CoinbaseItem" => :coinbase_items, + "BinanceItem" => :binance_items, + "CoinstatsItem" => :coinstats_items, + "SnaptradeItem" => :snaptrade_items, + "MercuryItem" => :mercury_items, + "SophtronItem" => :sophtron_items, + "IndexaCapitalItem" => :indexa_capital_items + }.freeze + + before_action :ensure_read_scope + before_action :set_sync, only: [ :show ] + + helper_method :sync_error_payload + + def index + @per_page = safe_per_page_param + @pagy, @syncs = pagy( + family_syncs_query.preload(:syncable, :children).ordered, + page: safe_page_param, + limit: @per_page + ) + + render :index + end + + def latest + @sync = family_syncs_query.preload(:syncable, :children).ordered.first + render :show + end + + def show + render :show + end + + private + + def set_sync + @sync = family_syncs_query.preload(:syncable, :children).find(params[:id]) + end + + def ensure_read_scope + authorize_scope!(:read) + end + + def family_syncs_query + family = current_resource_owner.family + query = Sync.where(syncable_type: "Family", syncable_id: family.id) + + SYNCABLE_ASSOCIATIONS.each do |syncable_type, association_name| + ids = family.public_send(association_name).select(:id) + query = query.or(Sync.where(syncable_type: syncable_type, syncable_id: ids)) + end + + query + end + + def sync_error_payload(sync) + return unless sync.failed? || sync.stale? + + { + present: sync.error.present?, + message: sync.stale? ? "Sync became stale before completion" : "Sync failed" + } + end + + def safe_page_param + page = params[:page].to_i + page > 0 ? page : 1 + end + + def safe_per_page_param + per_page = params[:per_page].to_i + + case per_page + when 1..100 + per_page + else + 25 + end + end +end diff --git a/app/views/api/v1/syncs/_sync.json.jbuilder b/app/views/api/v1/syncs/_sync.json.jbuilder new file mode 100644 index 000000000..1760f8154 --- /dev/null +++ b/app/views/api/v1/syncs/_sync.json.jbuilder @@ -0,0 +1,24 @@ +# frozen_string_literal: true + +syncable = sync.syncable + +json.id sync.id +json.status sync.status +json.in_progress sync.pending? || sync.syncing? +json.terminal sync.completed? || sync.failed? || sync.stale? +json.syncable do + json.type sync.syncable_type + json.id sync.syncable_id + json.name syncable.respond_to?(:name) ? syncable.name : nil +end +json.parent_id sync.parent_id +json.children_count sync.children.size +json.window_start_date sync.window_start_date +json.window_end_date sync.window_end_date +json.pending_at sync.pending_at +json.syncing_at sync.syncing_at +json.completed_at sync.completed_at +json.failed_at sync.failed_at +json.error sync_error_payload(sync) +json.created_at sync.created_at +json.updated_at sync.updated_at diff --git a/app/views/api/v1/syncs/index.json.jbuilder b/app/views/api/v1/syncs/index.json.jbuilder new file mode 100644 index 000000000..70496f7ff --- /dev/null +++ b/app/views/api/v1/syncs/index.json.jbuilder @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +json.data do + json.array! @syncs, partial: "api/v1/syncs/sync", as: :sync +end + +json.meta do + json.page @pagy.page + json.per_page @per_page + json.total_count @pagy.count + json.total_pages @pagy.pages +end diff --git a/app/views/api/v1/syncs/show.json.jbuilder b/app/views/api/v1/syncs/show.json.jbuilder new file mode 100644 index 000000000..621b5638a --- /dev/null +++ b/app/views/api/v1/syncs/show.json.jbuilder @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +json.data do + if @sync + json.partial! "api/v1/syncs/sync", sync: @sync + else + json.nil! + end +end diff --git a/config/routes.rb b/config/routes.rb index ce780cc49..ba0f3669d 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -444,6 +444,9 @@ resource :balance_sheet, only: [ :show ], controller: :balance_sheet resource :family_settings, only: [ :show ], controller: :family_settings post :sync, to: "sync#create" + resources :syncs, only: [ :index, :show ], as: :sync_records do + get :latest, on: :collection + end resources :chats, only: [ :index, :show, :create, :update, :destroy ] do resources :messages, only: [ :create ] do diff --git a/docs/api/openapi.yaml b/docs/api/openapi.yaml index 5fbd6aac9..bb6227649 100644 --- a/docs/api/openapi.yaml +++ b/docs/api/openapi.yaml @@ -1318,6 +1318,123 @@ components: properties: data: "$ref": "#/components/schemas/ImportDetail" + SyncableSummary: + type: object + required: + - type + - id + properties: + type: + type: string + id: + type: string + format: uuid + name: + type: string + nullable: true + SyncErrorSummary: + type: object + required: + - present + - message + properties: + present: + type: boolean + message: + type: string + SyncResource: + type: object + required: + - id + - status + - in_progress + - terminal + - syncable + - children_count + - created_at + - updated_at + properties: + id: + type: string + format: uuid + status: + type: string + enum: + - pending + - syncing + - completed + - failed + - stale + in_progress: + type: boolean + terminal: + type: boolean + syncable: + "$ref": "#/components/schemas/SyncableSummary" + parent_id: + type: string + format: uuid + nullable: true + children_count: + type: integer + minimum: 0 + window_start_date: + type: string + format: date + nullable: true + window_end_date: + type: string + format: date + nullable: true + pending_at: + type: string + format: date-time + nullable: true + syncing_at: + type: string + format: date-time + nullable: true + completed_at: + type: string + format: date-time + nullable: true + failed_at: + type: string + format: date-time + nullable: true + error: + oneOf: + - "$ref": "#/components/schemas/SyncErrorSummary" + - type: 'null' + nullable: true + created_at: + type: string + format: date-time + updated_at: + type: string + format: date-time + SyncResponse: + type: object + required: + - data + properties: + data: + oneOf: + - "$ref": "#/components/schemas/SyncResource" + - type: 'null' + nullable: true + SyncCollection: + type: object + required: + - data + - meta + properties: + data: + type: array + items: + "$ref": "#/components/schemas/SyncResource" + meta: + "$ref": "#/components/schemas/Pagination" Trade: type: object required: @@ -3683,6 +3800,95 @@ paths: application/json: schema: "$ref": "#/components/schemas/ErrorResponse" + "/api/v1/syncs": + get: + summary: Lists sync history + description: List sanitized sync status history for the authenticated user's + family, accounts, and provider connections. + tags: + - Syncs + security: + - apiKeyAuth: [] + parameters: + - name: page + in: query + required: false + schema: + type: integer + - name: per_page + in: query + required: false + schema: + type: integer + responses: + '200': + description: syncs listed + content: + application/json: + schema: + "$ref": "#/components/schemas/SyncCollection" + '401': + description: unauthorized + content: + application/json: + schema: + "$ref": "#/components/schemas/ErrorResponse" + "/api/v1/syncs/latest": + get: + summary: Shows the latest sync + description: Return the most recently created sanitized sync status for the + authenticated user's family. + tags: + - Syncs + security: + - apiKeyAuth: [] + responses: + '200': + description: latest sync shown + content: + application/json: + schema: + "$ref": "#/components/schemas/SyncResponse" + '401': + description: unauthorized + content: + application/json: + schema: + "$ref": "#/components/schemas/ErrorResponse" + "/api/v1/syncs/{id}": + parameters: + - name: id + in: path + format: uuid + required: true + schema: + type: string + get: + summary: Shows a sync + description: Return sanitized status metadata for a single family-scoped sync. + tags: + - Syncs + security: + - apiKeyAuth: [] + responses: + '200': + description: sync shown + content: + application/json: + schema: + "$ref": "#/components/schemas/SyncResponse" + '401': + description: unauthorized + content: + application/json: + schema: + "$ref": "#/components/schemas/ErrorResponse" + '404': + description: not found + content: + application/json: + schema: + "$ref": "#/components/schemas/ErrorResponse" "/api/v1/tags": get: summary: List tags diff --git a/spec/requests/api/v1/syncs_spec.rb b/spec/requests/api/v1/syncs_spec.rb new file mode 100644 index 000000000..e97546d80 --- /dev/null +++ b/spec/requests/api/v1/syncs_spec.rb @@ -0,0 +1,106 @@ +# frozen_string_literal: true + +require "swagger_helper" + +RSpec.describe "Api::V1::Syncs", type: :request do + let(:family) do + Family.create!( + name: "API Family", + currency: "USD", + locale: "en", + date_format: "%m-%d-%Y" + ) + end + + let(:user) do + family.users.create!( + email: "sync-api-user@example.com", + password: "password123", + password_confirmation: "password123" + ) + end + + let(:api_key) do + key = ApiKey.generate_secure_key + ApiKey.create!( + user: user, + name: "API Docs Key", + key: key, + scopes: %w[read_write], + source: "web" + ) + end + let(:'X-Api-Key') { api_key.plain_key } + let!(:sync) { Sync.create!(syncable: family, status: "completed", completed_at: 1.minute.ago) } + let(:id) { sync.id } + + path "/api/v1/syncs" do + get "Lists sync history" do + description "List sanitized sync status history for the authenticated user's family, accounts, and provider connections." + tags "Syncs" + security [ { apiKeyAuth: [] } ] + produces "application/json" + parameter name: :page, in: :query, type: :integer, required: false + parameter name: :per_page, in: :query, type: :integer, required: false + + response "200", "syncs listed" do + schema "$ref" => "#/components/schemas/SyncCollection" + run_test! + end + + response "401", "unauthorized" do + let(:'X-Api-Key') { nil } + schema "$ref" => "#/components/schemas/ErrorResponse" + run_test! + end + end + end + + path "/api/v1/syncs/latest" do + get "Shows the latest sync" do + description "Return the most recently created sanitized sync status for the authenticated user's family." + tags "Syncs" + security [ { apiKeyAuth: [] } ] + produces "application/json" + + response "200", "latest sync shown" do + schema "$ref" => "#/components/schemas/SyncResponse" + run_test! + end + + response "401", "unauthorized" do + let(:'X-Api-Key') { nil } + schema "$ref" => "#/components/schemas/ErrorResponse" + run_test! + end + end + end + + path "/api/v1/syncs/{id}" do + parameter name: :id, in: :path, type: :string, format: :uuid, required: true + + get "Shows a sync" do + description "Return sanitized status metadata for a single family-scoped sync." + tags "Syncs" + security [ { apiKeyAuth: [] } ] + produces "application/json" + + response "200", "sync shown" do + schema "$ref" => "#/components/schemas/SyncResponse" + run_test! + end + + response "401", "unauthorized" do + let(:'X-Api-Key') { nil } + schema "$ref" => "#/components/schemas/ErrorResponse" + run_test! + end + + response "404", "not found" do + let(:id) { SecureRandom.uuid } + schema "$ref" => "#/components/schemas/ErrorResponse" + run_test! + end + end + end +end diff --git a/spec/swagger_helper.rb b/spec/swagger_helper.rb index cf86c73e0..f2883ae26 100644 --- a/spec/swagger_helper.rb +++ b/spec/swagger_helper.rb @@ -743,6 +743,75 @@ data: { '$ref' => '#/components/schemas/ImportDetail' } } }, + SyncableSummary: { + type: :object, + required: %w[type id], + properties: { + type: { type: :string }, + id: { type: :string, format: :uuid }, + name: { type: :string, nullable: true } + } + }, + SyncErrorSummary: { + type: :object, + required: %w[present message], + properties: { + present: { type: :boolean }, + message: { type: :string } + } + }, + SyncResource: { + type: :object, + required: %w[id status in_progress terminal syncable children_count created_at updated_at], + properties: { + id: { type: :string, format: :uuid }, + status: { type: :string, enum: %w[pending syncing completed failed stale] }, + in_progress: { type: :boolean }, + terminal: { type: :boolean }, + syncable: { '$ref' => '#/components/schemas/SyncableSummary' }, + parent_id: { type: :string, format: :uuid, nullable: true }, + children_count: { type: :integer, minimum: 0 }, + window_start_date: { type: :string, format: :date, nullable: true }, + window_end_date: { type: :string, format: :date, nullable: true }, + pending_at: { type: :string, format: :'date-time', nullable: true }, + syncing_at: { type: :string, format: :'date-time', nullable: true }, + completed_at: { type: :string, format: :'date-time', nullable: true }, + failed_at: { type: :string, format: :'date-time', nullable: true }, + error: { + oneOf: [ + { '$ref' => '#/components/schemas/SyncErrorSummary' }, + { type: :null } + ], + nullable: true + }, + created_at: { type: :string, format: :'date-time' }, + updated_at: { type: :string, format: :'date-time' } + } + }, + SyncResponse: { + type: :object, + required: %w[data], + properties: { + data: { + oneOf: [ + { '$ref' => '#/components/schemas/SyncResource' }, + { type: :null } + ], + nullable: true + } + } + }, + SyncCollection: { + type: :object, + required: %w[data meta], + properties: { + data: { + type: :array, + items: { '$ref' => '#/components/schemas/SyncResource' } + }, + meta: { '$ref' => '#/components/schemas/Pagination' } + } + }, Trade: { type: :object, required: %w[id date amount currency name qty price account created_at updated_at], diff --git a/test/controllers/api/v1/syncs_controller_test.rb b/test/controllers/api/v1/syncs_controller_test.rb new file mode 100644 index 000000000..878007aec --- /dev/null +++ b/test/controllers/api/v1/syncs_controller_test.rb @@ -0,0 +1,120 @@ +# frozen_string_literal: true + +require "test_helper" + +class Api::V1::SyncsControllerTest < ActionDispatch::IntegrationTest + setup do + @user = users(:family_admin) + @family = @user.family + @account = @family.accounts.first + + Sync.destroy_all + + @user.api_keys.active.destroy_all + + @api_key = ApiKey.create!( + user: @user, + name: "Test Read-Write Key", + scopes: [ "read_write" ], + display_key: "test_rw_#{SecureRandom.hex(8)}", + source: "web" + ) + + @read_only_api_key = ApiKey.create!( + user: @user, + name: "Test Read Key", + scopes: [ "read" ], + display_key: "test_ro_#{SecureRandom.hex(8)}", + source: "mobile" + ) + + Redis.new.del("api_rate_limit:#{@api_key.id}") + Redis.new.del("api_rate_limit:#{@read_only_api_key.id}") + end + + test "lists family scoped syncs" do + family_sync = Sync.create!(syncable: @family, status: "completed", completed_at: 1.hour.ago) + account_sync = Sync.create!(syncable: @account, status: "syncing", syncing_at: Time.current) + other_sync = Sync.create!(syncable: families(:empty), status: "completed", completed_at: 1.hour.ago) + + get api_v1_sync_records_url, headers: api_headers(@read_only_api_key) + assert_response :success + + json_response = JSON.parse(response.body) + sync_ids = json_response["data"].map { |sync| sync["id"] } + + assert_includes sync_ids, family_sync.id + assert_includes sync_ids, account_sync.id + assert_not_includes sync_ids, other_sync.id + assert_equal 2, json_response["meta"]["total_count"] + end + + test "shows a sync" do + sync = Sync.create!( + syncable: @family, + status: "completed", + completed_at: 1.hour.ago, + window_start_date: Date.current - 7.days, + window_end_date: Date.current + ) + + get api_v1_sync_record_url(sync), headers: api_headers(@read_only_api_key) + assert_response :success + + data = JSON.parse(response.body)["data"] + assert_equal sync.id, data["id"] + assert_equal "completed", data["status"] + assert_equal false, data["in_progress"] + assert_equal true, data["terminal"] + assert_equal "Family", data["syncable"]["type"] + assert_equal @family.id, data["syncable"]["id"] + assert_nil data["error"] + end + + test "returns latest sync" do + Sync.create!(syncable: @family, status: "completed", created_at: 2.hours.ago, completed_at: 2.hours.ago) + latest_sync = Sync.create!(syncable: @account, status: "pending", created_at: 1.minute.ago) + + get latest_api_v1_sync_records_url, headers: api_headers(@read_only_api_key) + assert_response :success + + assert_equal latest_sync.id, JSON.parse(response.body)["data"]["id"] + end + + test "does not expose raw sync errors" do + sync = Sync.create!( + syncable: @family, + status: "failed", + failed_at: Time.current, + error: "provider token secret leaked" + ) + + get api_v1_sync_record_url(sync), headers: api_headers(@read_only_api_key) + assert_response :success + + data = JSON.parse(response.body)["data"] + assert_equal true, data["error"]["present"] + assert_equal "Sync failed", data["error"]["message"] + refute_includes response.body, "provider token secret leaked" + end + + test "returns not found for another family sync" do + sync = Sync.create!(syncable: families(:empty), status: "completed") + + get api_v1_sync_record_url(sync), headers: api_headers(@read_only_api_key) + assert_response :not_found + + assert_equal "record_not_found", JSON.parse(response.body)["error"] + end + + test "requires authentication" do + get api_v1_sync_records_url + assert_response :unauthorized + end + + private + + def api_headers(api_key) + { "X-Api-Key" => api_key.display_key } + end +end From 8a4c6ce28a31df85ac9f951a7348ba0db3bb65d2 Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Sat, 2 May 2026 01:01:36 -0600 Subject: [PATCH 02/10] fix(api): harden sync status review paths --- app/controllers/api/v1/syncs_controller.rb | 17 +++++- docs/api/openapi.yaml | 8 +-- spec/requests/api/v1/syncs_spec.rb | 4 +- spec/swagger_helper.rb | 16 +---- .../api/v1/syncs_controller_test.rb | 61 +++++++++++++++++-- 5 files changed, 80 insertions(+), 26 deletions(-) diff --git a/app/controllers/api/v1/syncs_controller.rb b/app/controllers/api/v1/syncs_controller.rb index 4c68afea6..b63fd89b9 100644 --- a/app/controllers/api/v1/syncs_controller.rb +++ b/app/controllers/api/v1/syncs_controller.rb @@ -3,6 +3,9 @@ class Api::V1::SyncsController < Api::V1::BaseController include Pagy::Backend + UUID_PATTERN = /\A[0-9a-f]{8}-[0-9a-f]{4}-[1-5][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}\z/i + private_constant :UUID_PATTERN + SYNCABLE_ASSOCIATIONS = { "Account" => :accounts, "PlaidItem" => :plaid_items, @@ -46,6 +49,8 @@ def show private def set_sync + raise ActiveRecord::RecordNotFound unless valid_uuid?(params[:id]) + @sync = family_syncs_query.preload(:syncable, :children).find(params[:id]) end @@ -58,13 +63,23 @@ def family_syncs_query query = Sync.where(syncable_type: "Family", syncable_id: family.id) SYNCABLE_ASSOCIATIONS.each do |syncable_type, association_name| - ids = family.public_send(association_name).select(:id) + ids = syncable_ids_for(family, syncable_type, association_name) query = query.or(Sync.where(syncable_type: syncable_type, syncable_id: ids)) end query end + def syncable_ids_for(family, syncable_type, association_name) + return current_resource_owner.accessible_accounts.select(:id) if syncable_type == "Account" + + family.public_send(association_name).select(:id) + end + + def valid_uuid?(value) + value.to_s.match?(UUID_PATTERN) + end + def sync_error_payload(sync) return unless sync.failed? || sync.stale? diff --git a/docs/api/openapi.yaml b/docs/api/openapi.yaml index bb6227649..e76c65556 100644 --- a/docs/api/openapi.yaml +++ b/docs/api/openapi.yaml @@ -1403,9 +1403,7 @@ components: format: date-time nullable: true error: - oneOf: - - "$ref": "#/components/schemas/SyncErrorSummary" - - type: 'null' + "$ref": "#/components/schemas/SyncErrorSummary" nullable: true created_at: type: string @@ -1419,9 +1417,7 @@ components: - data properties: data: - oneOf: - - "$ref": "#/components/schemas/SyncResource" - - type: 'null' + "$ref": "#/components/schemas/SyncResource" nullable: true SyncCollection: type: object diff --git a/spec/requests/api/v1/syncs_spec.rb b/spec/requests/api/v1/syncs_spec.rb index e97546d80..3e26e207e 100644 --- a/spec/requests/api/v1/syncs_spec.rb +++ b/spec/requests/api/v1/syncs_spec.rb @@ -31,7 +31,7 @@ ) end let(:'X-Api-Key') { api_key.plain_key } - let!(:sync) { Sync.create!(syncable: family, status: "completed", completed_at: 1.minute.ago) } + let(:sync) { Sync.create!(syncable: family, status: "completed", completed_at: 1.minute.ago) } let(:id) { sync.id } path "/api/v1/syncs" do @@ -45,6 +45,7 @@ response "200", "syncs listed" do schema "$ref" => "#/components/schemas/SyncCollection" + before { sync } run_test! end @@ -65,6 +66,7 @@ response "200", "latest sync shown" do schema "$ref" => "#/components/schemas/SyncResponse" + before { sync } run_test! end diff --git a/spec/swagger_helper.rb b/spec/swagger_helper.rb index f2883ae26..b5a4916c5 100644 --- a/spec/swagger_helper.rb +++ b/spec/swagger_helper.rb @@ -777,13 +777,7 @@ syncing_at: { type: :string, format: :'date-time', nullable: true }, completed_at: { type: :string, format: :'date-time', nullable: true }, failed_at: { type: :string, format: :'date-time', nullable: true }, - error: { - oneOf: [ - { '$ref' => '#/components/schemas/SyncErrorSummary' }, - { type: :null } - ], - nullable: true - }, + error: { '$ref' => '#/components/schemas/SyncErrorSummary', nullable: true }, created_at: { type: :string, format: :'date-time' }, updated_at: { type: :string, format: :'date-time' } } @@ -792,13 +786,7 @@ type: :object, required: %w[data], properties: { - data: { - oneOf: [ - { '$ref' => '#/components/schemas/SyncResource' }, - { type: :null } - ], - nullable: true - } + data: { '$ref' => '#/components/schemas/SyncResource', nullable: true } } }, SyncCollection: { diff --git a/test/controllers/api/v1/syncs_controller_test.rb b/test/controllers/api/v1/syncs_controller_test.rb index 878007aec..d41fdc36e 100644 --- a/test/controllers/api/v1/syncs_controller_test.rb +++ b/test/controllers/api/v1/syncs_controller_test.rb @@ -28,8 +28,18 @@ class Api::V1::SyncsControllerTest < ActionDispatch::IntegrationTest source: "mobile" ) - Redis.new.del("api_rate_limit:#{@api_key.id}") - Redis.new.del("api_rate_limit:#{@read_only_api_key.id}") + @member_api_key = ApiKey.create!( + user: users(:family_member), + name: "Member Read Key", + scopes: [ "read" ], + display_key: "test_member_#{SecureRandom.hex(8)}", + source: "web" + ) + + redis = Redis.new + redis.del("api_rate_limit:#{@api_key.id}") + redis.del("api_rate_limit:#{@read_only_api_key.id}") + redis.del("api_rate_limit:#{@member_api_key.id}") end test "lists family scoped syncs" do @@ -49,6 +59,23 @@ class Api::V1::SyncsControllerTest < ActionDispatch::IntegrationTest assert_equal 2, json_response["meta"]["total_count"] end + test "does not list account syncs outside caller account access" do + private_account = @family.accounts.create!( + owner: @user, + name: "Private Sync Account", + balance: 0, + currency: "USD", + accountable: Depository.new + ) + inaccessible_sync = Sync.create!(syncable: private_account, status: "completed", completed_at: 1.hour.ago) + + get api_v1_sync_records_url, headers: api_headers(@member_api_key) + assert_response :success + + sync_ids = JSON.parse(response.body)["data"].map { |sync| sync["id"] } + assert_not_includes sync_ids, inaccessible_sync.id + end + test "shows a sync" do sync = Sync.create!( syncable: @family, @@ -81,6 +108,13 @@ class Api::V1::SyncsControllerTest < ActionDispatch::IntegrationTest assert_equal latest_sync.id, JSON.parse(response.body)["data"]["id"] end + test "latest returns null data when no sync exists" do + get latest_api_v1_sync_records_url, headers: api_headers(@read_only_api_key) + assert_response :success + + assert_nil JSON.parse(response.body)["data"] + end + test "does not expose raw sync errors" do sync = Sync.create!( syncable: @family, @@ -107,14 +141,33 @@ class Api::V1::SyncsControllerTest < ActionDispatch::IntegrationTest assert_equal "record_not_found", JSON.parse(response.body)["error"] end - test "requires authentication" do + test "returns not found for malformed sync id" do + get api_v1_sync_record_url("not-a-uuid"), headers: api_headers(@read_only_api_key) + assert_response :not_found + + assert_equal "record_not_found", JSON.parse(response.body)["error"] + end + + test "index requires authentication" do get api_v1_sync_records_url assert_response :unauthorized end + test "latest requires authentication" do + get latest_api_v1_sync_records_url + assert_response :unauthorized + end + + test "show requires authentication" do + sync = Sync.create!(syncable: @family, status: "completed", completed_at: 1.hour.ago) + + get api_v1_sync_record_url(sync) + assert_response :unauthorized + end + private def api_headers(api_key) - { "X-Api-Key" => api_key.display_key } + { "X-Api-Key" => api_key.plain_key } end end From e85a43f7ba568d50373a9370e72dac9d62ee86fe Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Sat, 2 May 2026 18:33:14 -0700 Subject: [PATCH 03/10] fix(api): address sync status review --- app/controllers/api/v1/syncs_controller.rb | 42 +------------ app/models/sync.rb | 59 +++++++++++++++++++ app/views/api/v1/syncs/_sync.json.jbuilder | 8 +-- config/routes.rb | 1 + docs/api/openapi.yaml | 25 +++++++- spec/requests/api/v1/syncs_spec.rb | 27 ++++++++- spec/swagger_helper.rb | 1 + .../api/v1/syncs_controller_test.rb | 3 +- 8 files changed, 115 insertions(+), 51 deletions(-) diff --git a/app/controllers/api/v1/syncs_controller.rb b/app/controllers/api/v1/syncs_controller.rb index b63fd89b9..87ea00b28 100644 --- a/app/controllers/api/v1/syncs_controller.rb +++ b/app/controllers/api/v1/syncs_controller.rb @@ -6,26 +6,9 @@ class Api::V1::SyncsController < Api::V1::BaseController UUID_PATTERN = /\A[0-9a-f]{8}-[0-9a-f]{4}-[1-5][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}\z/i private_constant :UUID_PATTERN - SYNCABLE_ASSOCIATIONS = { - "Account" => :accounts, - "PlaidItem" => :plaid_items, - "SimplefinItem" => :simplefin_items, - "LunchflowItem" => :lunchflow_items, - "EnableBankingItem" => :enable_banking_items, - "CoinbaseItem" => :coinbase_items, - "BinanceItem" => :binance_items, - "CoinstatsItem" => :coinstats_items, - "SnaptradeItem" => :snaptrade_items, - "MercuryItem" => :mercury_items, - "SophtronItem" => :sophtron_items, - "IndexaCapitalItem" => :indexa_capital_items - }.freeze - before_action :ensure_read_scope before_action :set_sync, only: [ :show ] - helper_method :sync_error_payload - def index @per_page = safe_per_page_param @pagy, @syncs = pagy( @@ -59,36 +42,13 @@ def ensure_read_scope end def family_syncs_query - family = current_resource_owner.family - query = Sync.where(syncable_type: "Family", syncable_id: family.id) - - SYNCABLE_ASSOCIATIONS.each do |syncable_type, association_name| - ids = syncable_ids_for(family, syncable_type, association_name) - query = query.or(Sync.where(syncable_type: syncable_type, syncable_id: ids)) - end - - query - end - - def syncable_ids_for(family, syncable_type, association_name) - return current_resource_owner.accessible_accounts.select(:id) if syncable_type == "Account" - - family.public_send(association_name).select(:id) + Sync.for_family(current_resource_owner.family, resource_owner: current_resource_owner) end def valid_uuid?(value) value.to_s.match?(UUID_PATTERN) end - def sync_error_payload(sync) - return unless sync.failed? || sync.stale? - - { - present: sync.error.present?, - message: sync.stale? ? "Sync became stale before completion" : "Sync failed" - } - end - def safe_page_param page = params[:page].to_i page > 0 ? page : 1 diff --git a/app/models/sync.rb b/app/models/sync.rb index d1ba07a26..30855da1f 100644 --- a/app/models/sync.rb +++ b/app/models/sync.rb @@ -19,6 +19,22 @@ class Sync < ApplicationRecord scope :incomplete, -> { where("syncs.status IN (?)", %w[pending syncing]) } scope :visible, -> { incomplete.where("syncs.created_at > ?", VISIBLE_FOR.ago) } + SYNCABLE_ASSOCIATIONS = { + "Account" => :accounts, + "PlaidItem" => :plaid_items, + "SimplefinItem" => :simplefin_items, + "LunchflowItem" => :lunchflow_items, + "EnableBankingItem" => :enable_banking_items, + "CoinbaseItem" => :coinbase_items, + "BinanceItem" => :binance_items, + "CoinstatsItem" => :coinstats_items, + "SnaptradeItem" => :snaptrade_items, + "MercuryItem" => :mercury_items, + "SophtronItem" => :sophtron_items, + "IndexaCapitalItem" => :indexa_capital_items + }.freeze + private_constant :SYNCABLE_ASSOCIATIONS + after_commit :update_family_sync_timestamp serialize :sync_stats, coder: JSON @@ -57,6 +73,49 @@ class << self def clean incomplete.where("syncs.created_at < ?", STALE_AFTER.ago).find_each(&:mark_stale!) end + + def for_family(family, resource_owner: nil) + query = where(syncable_type: "Family", syncable_id: family.id) + + SYNCABLE_ASSOCIATIONS.each do |syncable_type, association_name| + ids = syncable_ids_for_family(family, syncable_type, association_name, resource_owner) + next unless ids + + query = query.or(where(syncable_type: syncable_type, syncable_id: ids)) + end + + query + end + + private + def syncable_ids_for_family(family, syncable_type, association_name, resource_owner) + if syncable_type == "Account" + return (resource_owner ? resource_owner.accessible_accounts : family.accounts) + .where(family_id: family.id) + .select(:id) + end + + return unless family.respond_to?(association_name) + + family.public_send(association_name).select(:id) + end + end + + def in_progress? + pending? || syncing? + end + + def terminal? + completed? || failed? || stale? + end + + def api_error_payload + return unless failed? || stale? + + { + present: error.present?, + message: stale? ? "Sync became stale before completion" : "Sync failed" + } end def perform diff --git a/app/views/api/v1/syncs/_sync.json.jbuilder b/app/views/api/v1/syncs/_sync.json.jbuilder index 1760f8154..9550aeca1 100644 --- a/app/views/api/v1/syncs/_sync.json.jbuilder +++ b/app/views/api/v1/syncs/_sync.json.jbuilder @@ -4,12 +4,12 @@ syncable = sync.syncable json.id sync.id json.status sync.status -json.in_progress sync.pending? || sync.syncing? -json.terminal sync.completed? || sync.failed? || sync.stale? +json.in_progress sync.in_progress? +json.terminal sync.terminal? json.syncable do json.type sync.syncable_type json.id sync.syncable_id - json.name syncable.respond_to?(:name) ? syncable.name : nil + json.name syncable&.try(:name) end json.parent_id sync.parent_id json.children_count sync.children.size @@ -19,6 +19,6 @@ json.pending_at sync.pending_at json.syncing_at sync.syncing_at json.completed_at sync.completed_at json.failed_at sync.failed_at -json.error sync_error_payload(sync) +json.error sync.api_error_payload json.created_at sync.created_at json.updated_at sync.updated_at diff --git a/config/routes.rb b/config/routes.rb index ba0f3669d..da83a7337 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -444,6 +444,7 @@ resource :balance_sheet, only: [ :show ], controller: :balance_sheet resource :family_settings, only: [ :show ], controller: :family_settings post :sync, to: "sync#create" + # Alias route helpers to avoid colliding with the singular POST /api/v1/sync helper. resources :syncs, only: [ :index, :show ], as: :sync_records do get :latest, on: :collection end diff --git a/docs/api/openapi.yaml b/docs/api/openapi.yaml index e76c65556..370167d28 100644 --- a/docs/api/openapi.yaml +++ b/docs/api/openapi.yaml @@ -1427,6 +1427,7 @@ components: properties: data: type: array + maxItems: 100 items: "$ref": "#/components/schemas/SyncResource" meta: @@ -3809,11 +3810,13 @@ paths: - name: page in: query required: false + description: 'Page number (default: 1)' schema: type: integer - name: per_page in: query required: false + description: 'Items per page (default: 25, max: 100)' schema: type: integer responses: @@ -3829,11 +3832,17 @@ paths: application/json: schema: "$ref": "#/components/schemas/ErrorResponse" + '403': + description: forbidden + content: + application/json: + schema: + "$ref": "#/components/schemas/ErrorResponse" "/api/v1/syncs/latest": get: summary: Shows the latest sync - description: Return the most recently created sanitized sync status for the - authenticated user's family. + description: 'Return the most recently created sanitized sync status for the + authenticated user''s family, or data: null when no sync exists.' tags: - Syncs security: @@ -3851,6 +3860,12 @@ paths: application/json: schema: "$ref": "#/components/schemas/ErrorResponse" + '403': + description: forbidden + content: + application/json: + schema: + "$ref": "#/components/schemas/ErrorResponse" "/api/v1/syncs/{id}": parameters: - name: id @@ -3879,6 +3894,12 @@ paths: application/json: schema: "$ref": "#/components/schemas/ErrorResponse" + '403': + description: forbidden + content: + application/json: + schema: + "$ref": "#/components/schemas/ErrorResponse" '404': description: not found content: diff --git a/spec/requests/api/v1/syncs_spec.rb b/spec/requests/api/v1/syncs_spec.rb index 3e26e207e..d48f59027 100644 --- a/spec/requests/api/v1/syncs_spec.rb +++ b/spec/requests/api/v1/syncs_spec.rb @@ -40,8 +40,8 @@ tags "Syncs" security [ { apiKeyAuth: [] } ] produces "application/json" - parameter name: :page, in: :query, type: :integer, required: false - parameter name: :per_page, in: :query, type: :integer, required: false + parameter name: :page, in: :query, type: :integer, required: false, description: "Page number (default: 1)" + parameter name: :per_page, in: :query, type: :integer, required: false, description: "Items per page (default: 25, max: 100)" response "200", "syncs listed" do schema "$ref" => "#/components/schemas/SyncCollection" @@ -54,12 +54,19 @@ schema "$ref" => "#/components/schemas/ErrorResponse" run_test! end + + response "403", "forbidden" do + before { api_key.update_column(:scopes, [ "write" ]) } + + schema "$ref" => "#/components/schemas/ErrorResponse" + run_test! + end end end path "/api/v1/syncs/latest" do get "Shows the latest sync" do - description "Return the most recently created sanitized sync status for the authenticated user's family." + description "Return the most recently created sanitized sync status for the authenticated user's family, or data: null when no sync exists." tags "Syncs" security [ { apiKeyAuth: [] } ] produces "application/json" @@ -75,6 +82,13 @@ schema "$ref" => "#/components/schemas/ErrorResponse" run_test! end + + response "403", "forbidden" do + before { api_key.update_column(:scopes, [ "write" ]) } + + schema "$ref" => "#/components/schemas/ErrorResponse" + run_test! + end end end @@ -98,6 +112,13 @@ run_test! end + response "403", "forbidden" do + before { api_key.update_column(:scopes, [ "write" ]) } + + schema "$ref" => "#/components/schemas/ErrorResponse" + run_test! + end + response "404", "not found" do let(:id) { SecureRandom.uuid } schema "$ref" => "#/components/schemas/ErrorResponse" diff --git a/spec/swagger_helper.rb b/spec/swagger_helper.rb index b5a4916c5..7a93e34a8 100644 --- a/spec/swagger_helper.rb +++ b/spec/swagger_helper.rb @@ -795,6 +795,7 @@ properties: { data: { type: :array, + maxItems: 100, items: { '$ref' => '#/components/schemas/SyncResource' } }, meta: { '$ref' => '#/components/schemas/Pagination' } diff --git a/test/controllers/api/v1/syncs_controller_test.rb b/test/controllers/api/v1/syncs_controller_test.rb index d41fdc36e..c6b03ea75 100644 --- a/test/controllers/api/v1/syncs_controller_test.rb +++ b/test/controllers/api/v1/syncs_controller_test.rb @@ -8,7 +8,7 @@ class Api::V1::SyncsControllerTest < ActionDispatch::IntegrationTest @family = @user.family @account = @family.accounts.first - Sync.destroy_all + Sync.for_family(@family).destroy_all @user.api_keys.active.destroy_all @@ -40,6 +40,7 @@ class Api::V1::SyncsControllerTest < ActionDispatch::IntegrationTest redis.del("api_rate_limit:#{@api_key.id}") redis.del("api_rate_limit:#{@read_only_api_key.id}") redis.del("api_rate_limit:#{@member_api_key.id}") + redis.close end test "lists family scoped syncs" do From afcec79671133a975f2ae5a30bd0331ca5036369 Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Sat, 2 May 2026 22:28:39 -0700 Subject: [PATCH 04/10] fix(api): tighten sync status review fixes --- app/controllers/api/v1/syncs_controller.rb | 11 ++----- .../api/v1/syncs_controller_test.rb | 29 ++++++++++++------- 2 files changed, 22 insertions(+), 18 deletions(-) diff --git a/app/controllers/api/v1/syncs_controller.rb b/app/controllers/api/v1/syncs_controller.rb index 87ea00b28..f928db165 100644 --- a/app/controllers/api/v1/syncs_controller.rb +++ b/app/controllers/api/v1/syncs_controller.rb @@ -3,9 +3,6 @@ class Api::V1::SyncsController < Api::V1::BaseController include Pagy::Backend - UUID_PATTERN = /\A[0-9a-f]{8}-[0-9a-f]{4}-[1-5][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}\z/i - private_constant :UUID_PATTERN - before_action :ensure_read_scope before_action :set_sync, only: [ :show ] @@ -22,6 +19,8 @@ def index def latest @sync = family_syncs_query.preload(:syncable, :children).ordered.first + return render json: { data: nil } unless @sync + render :show end @@ -42,11 +41,7 @@ def ensure_read_scope end def family_syncs_query - Sync.for_family(current_resource_owner.family, resource_owner: current_resource_owner) - end - - def valid_uuid?(value) - value.to_s.match?(UUID_PATTERN) + Sync.for_family(Current.family, resource_owner: Current.user) end def safe_page_param diff --git a/test/controllers/api/v1/syncs_controller_test.rb b/test/controllers/api/v1/syncs_controller_test.rb index c6b03ea75..93bb8dbe6 100644 --- a/test/controllers/api/v1/syncs_controller_test.rb +++ b/test/controllers/api/v1/syncs_controller_test.rb @@ -28,18 +28,9 @@ class Api::V1::SyncsControllerTest < ActionDispatch::IntegrationTest source: "mobile" ) - @member_api_key = ApiKey.create!( - user: users(:family_member), - name: "Member Read Key", - scopes: [ "read" ], - display_key: "test_member_#{SecureRandom.hex(8)}", - source: "web" - ) - redis = Redis.new redis.del("api_rate_limit:#{@api_key.id}") redis.del("api_rate_limit:#{@read_only_api_key.id}") - redis.del("api_rate_limit:#{@member_api_key.id}") redis.close end @@ -70,7 +61,9 @@ class Api::V1::SyncsControllerTest < ActionDispatch::IntegrationTest ) inaccessible_sync = Sync.create!(syncable: private_account, status: "completed", completed_at: 1.hour.ago) - get api_v1_sync_records_url, headers: api_headers(@member_api_key) + member_api_key = create_api_key(users(:family_member), name: "Member Read Key", scopes: [ "read" ]) + + get api_v1_sync_records_url, headers: api_headers(member_api_key) assert_response :success sync_ids = JSON.parse(response.body)["data"].map { |sync| sync["id"] } @@ -171,4 +164,20 @@ class Api::V1::SyncsControllerTest < ActionDispatch::IntegrationTest def api_headers(api_key) { "X-Api-Key" => api_key.plain_key } end + + def create_api_key(user, name:, scopes:) + api_key = ApiKey.create!( + user: user, + name: name, + scopes: scopes, + display_key: "test_#{SecureRandom.hex(8)}", + source: "web" + ) + + redis = Redis.new + redis.del("api_rate_limit:#{api_key.id}") + redis.close + + api_key + end end From b938e381eabf5fcf8ce14e53b6af2325ca1b2e92 Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Sat, 2 May 2026 22:46:08 -0700 Subject: [PATCH 05/10] fix(api): address sync status review --- app/controllers/api/v1/syncs_controller.rb | 16 ---------- app/models/sync.rb | 3 +- .../api/v1/syncs_controller_test.rb | 29 +++++++++++++++++++ test/models/sync_test.rb | 14 +++++++++ 4 files changed, 45 insertions(+), 17 deletions(-) diff --git a/app/controllers/api/v1/syncs_controller.rb b/app/controllers/api/v1/syncs_controller.rb index f928db165..401362392 100644 --- a/app/controllers/api/v1/syncs_controller.rb +++ b/app/controllers/api/v1/syncs_controller.rb @@ -43,20 +43,4 @@ def ensure_read_scope def family_syncs_query Sync.for_family(Current.family, resource_owner: Current.user) end - - def safe_page_param - page = params[:page].to_i - page > 0 ? page : 1 - end - - def safe_per_page_param - per_page = params[:per_page].to_i - - case per_page - when 1..100 - per_page - else - 25 - end - end end diff --git a/app/models/sync.rb b/app/models/sync.rb index 30855da1f..9a6eed445 100644 --- a/app/models/sync.rb +++ b/app/models/sync.rb @@ -15,7 +15,7 @@ class Sync < ApplicationRecord belongs_to :parent, class_name: "Sync", optional: true has_many :children, class_name: "Sync", foreign_key: :parent_id, dependent: :destroy - scope :ordered, -> { order(created_at: :desc) } + scope :ordered, -> { order(created_at: :desc, id: :desc) } scope :incomplete, -> { where("syncs.status IN (?)", %w[pending syncing]) } scope :visible, -> { incomplete.where("syncs.created_at > ?", VISIBLE_FOR.ago) } @@ -111,6 +111,7 @@ def terminal? def api_error_payload return unless failed? || stale? + return if stale? && error.blank? { present: error.present?, diff --git a/test/controllers/api/v1/syncs_controller_test.rb b/test/controllers/api/v1/syncs_controller_test.rb index 93bb8dbe6..f8183c656 100644 --- a/test/controllers/api/v1/syncs_controller_test.rb +++ b/test/controllers/api/v1/syncs_controller_test.rb @@ -126,6 +126,18 @@ class Api::V1::SyncsControllerTest < ActionDispatch::IntegrationTest refute_includes response.body, "provider token secret leaked" end + test "omits stale sync error payload when no error is present" do + sync = Sync.create!( + syncable: @family, + status: "stale" + ) + + get api_v1_sync_record_url(sync), headers: api_headers(@read_only_api_key) + assert_response :success + + assert_nil JSON.parse(response.body).dig("data", "error") + end + test "returns not found for another family sync" do sync = Sync.create!(syncable: families(:empty), status: "completed") @@ -159,6 +171,23 @@ class Api::V1::SyncsControllerTest < ActionDispatch::IntegrationTest assert_response :unauthorized end + test "index requires read scope" do + api_key_without_read = ApiKey.new( + user: @user, + name: "No Read Key", + scopes: [], + source: "monitoring", + display_key: "no_read_#{SecureRandom.hex(8)}" + ) + api_key_without_read.save!(validate: false) + + get api_v1_sync_records_url, headers: api_headers(api_key_without_read) + + assert_response :forbidden + ensure + api_key_without_read&.destroy + end + private def api_headers(api_key) diff --git a/test/models/sync_test.rb b/test/models/sync_test.rb index d1182fd51..1b5f671b2 100644 --- a/test/models/sync_test.rb +++ b/test/models/sync_test.rb @@ -200,6 +200,20 @@ class SyncTest < ActiveSupport::TestCase assert_equal "stale", stale_syncing.reload.status end + test "ordered uses id as deterministic tie breaker" do + timestamp = Time.current.change(usec: 0) + older_id = SecureRandom.uuid + newer_id = SecureRandom.uuid + older_id, newer_id = [ older_id, newer_id ].sort + + older_sync = Sync.create!(id: older_id, syncable: accounts(:depository), status: :completed, created_at: timestamp) + newer_sync = Sync.create!(id: newer_id, syncable: accounts(:connected), status: :completed, created_at: timestamp) + + ordered_ids = Sync.where(id: [ older_sync.id, newer_sync.id ]).ordered.pluck(:id) + + assert_equal [ newer_sync.id, older_sync.id ], ordered_ids + end + test "expand_window_if_needed widens start and end dates on a pending sync" do initial_start = 1.day.ago.to_date initial_end = 1.day.ago.to_date From 979dad35a32ae89b9e3434df369fbb082e938531 Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Sat, 2 May 2026 22:57:29 -0700 Subject: [PATCH 06/10] test(api): avoid secret-like sync fixture key --- test/controllers/api/v1/syncs_controller_test.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/controllers/api/v1/syncs_controller_test.rb b/test/controllers/api/v1/syncs_controller_test.rb index f8183c656..7e3483a7b 100644 --- a/test/controllers/api/v1/syncs_controller_test.rb +++ b/test/controllers/api/v1/syncs_controller_test.rb @@ -199,7 +199,7 @@ def create_api_key(user, name:, scopes:) user: user, name: name, scopes: scopes, - display_key: "test_#{SecureRandom.hex(8)}", + display_key: "syncs-test-key-#{user.id}-#{name.parameterize}", source: "web" ) From e05c8f8b3d6629156a4bb5dea33cc3c05d79ca3d Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Sat, 2 May 2026 23:01:11 -0700 Subject: [PATCH 07/10] test(api): reuse sync status fixture key --- .../api/v1/syncs_controller_test.rb | 20 ++----------------- 1 file changed, 2 insertions(+), 18 deletions(-) diff --git a/test/controllers/api/v1/syncs_controller_test.rb b/test/controllers/api/v1/syncs_controller_test.rb index 7e3483a7b..0cdfdd502 100644 --- a/test/controllers/api/v1/syncs_controller_test.rb +++ b/test/controllers/api/v1/syncs_controller_test.rb @@ -61,9 +61,9 @@ class Api::V1::SyncsControllerTest < ActionDispatch::IntegrationTest ) inaccessible_sync = Sync.create!(syncable: private_account, status: "completed", completed_at: 1.hour.ago) - member_api_key = create_api_key(users(:family_member), name: "Member Read Key", scopes: [ "read" ]) + @read_only_api_key.update_column(:user_id, users(:family_member).id) - get api_v1_sync_records_url, headers: api_headers(member_api_key) + get api_v1_sync_records_url, headers: api_headers(@read_only_api_key) assert_response :success sync_ids = JSON.parse(response.body)["data"].map { |sync| sync["id"] } @@ -193,20 +193,4 @@ class Api::V1::SyncsControllerTest < ActionDispatch::IntegrationTest def api_headers(api_key) { "X-Api-Key" => api_key.plain_key } end - - def create_api_key(user, name:, scopes:) - api_key = ApiKey.create!( - user: user, - name: name, - scopes: scopes, - display_key: "syncs-test-key-#{user.id}-#{name.parameterize}", - source: "web" - ) - - redis = Redis.new - redis.del("api_rate_limit:#{api_key.id}") - redis.close - - api_key - end end From 82aca49d606d2800923199650850fc040ef2a0ea Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Sat, 2 May 2026 23:05:55 -0700 Subject: [PATCH 08/10] fix(api): align sync route helpers --- config/routes.rb | 5 ++-- .../api/v1/sync_controller_test.rb | 8 +++--- .../api/v1/syncs_controller_test.rb | 26 +++++++++---------- 3 files changed, 19 insertions(+), 20 deletions(-) diff --git a/config/routes.rb b/config/routes.rb index da83a7337..3f10de7d6 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -443,9 +443,8 @@ resource :usage, only: [ :show ], controller: :usage resource :balance_sheet, only: [ :show ], controller: :balance_sheet resource :family_settings, only: [ :show ], controller: :family_settings - post :sync, to: "sync#create" - # Alias route helpers to avoid colliding with the singular POST /api/v1/sync helper. - resources :syncs, only: [ :index, :show ], as: :sync_records do + post :sync, to: "sync#create", as: :sync_job + resources :syncs, only: [ :index, :show ] do get :latest, on: :collection end diff --git a/test/controllers/api/v1/sync_controller_test.rb b/test/controllers/api/v1/sync_controller_test.rb index 3a920a672..fd6e18856 100644 --- a/test/controllers/api/v1/sync_controller_test.rb +++ b/test/controllers/api/v1/sync_controller_test.rb @@ -33,7 +33,7 @@ class Api::V1::SyncControllerTest < ActionDispatch::IntegrationTest test "should trigger sync with valid write API key" do assert_enqueued_with(job: SyncJob) do - post api_v1_sync_url, headers: api_headers(@api_key) + post api_v1_sync_job_url, headers: api_headers(@api_key) end assert_response :accepted @@ -48,7 +48,7 @@ class Api::V1::SyncControllerTest < ActionDispatch::IntegrationTest end test "should reject sync with read-only API key" do - post api_v1_sync_url, headers: api_headers(@read_only_api_key) + post api_v1_sync_job_url, headers: api_headers(@read_only_api_key) assert_response :forbidden response_data = JSON.parse(response.body) @@ -56,7 +56,7 @@ class Api::V1::SyncControllerTest < ActionDispatch::IntegrationTest end test "should reject sync without API key" do - post api_v1_sync_url + post api_v1_sync_job_url assert_response :unauthorized response_data = JSON.parse(response.body) @@ -64,7 +64,7 @@ class Api::V1::SyncControllerTest < ActionDispatch::IntegrationTest end test "should return proper sync details in response" do - post api_v1_sync_url, headers: api_headers(@api_key) + post api_v1_sync_job_url, headers: api_headers(@api_key) assert_response :accepted response_data = JSON.parse(response.body) diff --git a/test/controllers/api/v1/syncs_controller_test.rb b/test/controllers/api/v1/syncs_controller_test.rb index 0cdfdd502..5c219118d 100644 --- a/test/controllers/api/v1/syncs_controller_test.rb +++ b/test/controllers/api/v1/syncs_controller_test.rb @@ -39,7 +39,7 @@ class Api::V1::SyncsControllerTest < ActionDispatch::IntegrationTest account_sync = Sync.create!(syncable: @account, status: "syncing", syncing_at: Time.current) other_sync = Sync.create!(syncable: families(:empty), status: "completed", completed_at: 1.hour.ago) - get api_v1_sync_records_url, headers: api_headers(@read_only_api_key) + get api_v1_syncs_url, headers: api_headers(@read_only_api_key) assert_response :success json_response = JSON.parse(response.body) @@ -63,7 +63,7 @@ class Api::V1::SyncsControllerTest < ActionDispatch::IntegrationTest @read_only_api_key.update_column(:user_id, users(:family_member).id) - get api_v1_sync_records_url, headers: api_headers(@read_only_api_key) + get api_v1_syncs_url, headers: api_headers(@read_only_api_key) assert_response :success sync_ids = JSON.parse(response.body)["data"].map { |sync| sync["id"] } @@ -79,7 +79,7 @@ class Api::V1::SyncsControllerTest < ActionDispatch::IntegrationTest window_end_date: Date.current ) - get api_v1_sync_record_url(sync), headers: api_headers(@read_only_api_key) + get api_v1_sync_url(sync), headers: api_headers(@read_only_api_key) assert_response :success data = JSON.parse(response.body)["data"] @@ -96,14 +96,14 @@ class Api::V1::SyncsControllerTest < ActionDispatch::IntegrationTest Sync.create!(syncable: @family, status: "completed", created_at: 2.hours.ago, completed_at: 2.hours.ago) latest_sync = Sync.create!(syncable: @account, status: "pending", created_at: 1.minute.ago) - get latest_api_v1_sync_records_url, headers: api_headers(@read_only_api_key) + get latest_api_v1_syncs_url, headers: api_headers(@read_only_api_key) assert_response :success assert_equal latest_sync.id, JSON.parse(response.body)["data"]["id"] end test "latest returns null data when no sync exists" do - get latest_api_v1_sync_records_url, headers: api_headers(@read_only_api_key) + get latest_api_v1_syncs_url, headers: api_headers(@read_only_api_key) assert_response :success assert_nil JSON.parse(response.body)["data"] @@ -117,7 +117,7 @@ class Api::V1::SyncsControllerTest < ActionDispatch::IntegrationTest error: "provider token secret leaked" ) - get api_v1_sync_record_url(sync), headers: api_headers(@read_only_api_key) + get api_v1_sync_url(sync), headers: api_headers(@read_only_api_key) assert_response :success data = JSON.parse(response.body)["data"] @@ -132,7 +132,7 @@ class Api::V1::SyncsControllerTest < ActionDispatch::IntegrationTest status: "stale" ) - get api_v1_sync_record_url(sync), headers: api_headers(@read_only_api_key) + get api_v1_sync_url(sync), headers: api_headers(@read_only_api_key) assert_response :success assert_nil JSON.parse(response.body).dig("data", "error") @@ -141,33 +141,33 @@ class Api::V1::SyncsControllerTest < ActionDispatch::IntegrationTest test "returns not found for another family sync" do sync = Sync.create!(syncable: families(:empty), status: "completed") - get api_v1_sync_record_url(sync), headers: api_headers(@read_only_api_key) + get api_v1_sync_url(sync), headers: api_headers(@read_only_api_key) assert_response :not_found assert_equal "record_not_found", JSON.parse(response.body)["error"] end test "returns not found for malformed sync id" do - get api_v1_sync_record_url("not-a-uuid"), headers: api_headers(@read_only_api_key) + get api_v1_sync_url("not-a-uuid"), headers: api_headers(@read_only_api_key) assert_response :not_found assert_equal "record_not_found", JSON.parse(response.body)["error"] end test "index requires authentication" do - get api_v1_sync_records_url + get api_v1_syncs_url assert_response :unauthorized end test "latest requires authentication" do - get latest_api_v1_sync_records_url + get latest_api_v1_syncs_url assert_response :unauthorized end test "show requires authentication" do sync = Sync.create!(syncable: @family, status: "completed", completed_at: 1.hour.ago) - get api_v1_sync_record_url(sync) + get api_v1_sync_url(sync) assert_response :unauthorized end @@ -181,7 +181,7 @@ class Api::V1::SyncsControllerTest < ActionDispatch::IntegrationTest ) api_key_without_read.save!(validate: false) - get api_v1_sync_records_url, headers: api_headers(api_key_without_read) + get api_v1_syncs_url, headers: api_headers(api_key_without_read) assert_response :forbidden ensure From 3fa7df392ca89cc891bc146ec4c769226c4b8ef5 Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Sun, 3 May 2026 21:30:57 -0700 Subject: [PATCH 09/10] fix(api): tighten sync status scoping --- app/models/sync.rb | 49 +++++++------------ .../api/v1/syncs_controller_test.rb | 15 ++++++ test/models/sync_test.rb | 26 ++++++++++ 3 files changed, 59 insertions(+), 31 deletions(-) diff --git a/app/models/sync.rb b/app/models/sync.rb index 9a6eed445..b613ce720 100644 --- a/app/models/sync.rb +++ b/app/models/sync.rb @@ -19,22 +19,6 @@ class Sync < ApplicationRecord scope :incomplete, -> { where("syncs.status IN (?)", %w[pending syncing]) } scope :visible, -> { incomplete.where("syncs.created_at > ?", VISIBLE_FOR.ago) } - SYNCABLE_ASSOCIATIONS = { - "Account" => :accounts, - "PlaidItem" => :plaid_items, - "SimplefinItem" => :simplefin_items, - "LunchflowItem" => :lunchflow_items, - "EnableBankingItem" => :enable_banking_items, - "CoinbaseItem" => :coinbase_items, - "BinanceItem" => :binance_items, - "CoinstatsItem" => :coinstats_items, - "SnaptradeItem" => :snaptrade_items, - "MercuryItem" => :mercury_items, - "SophtronItem" => :sophtron_items, - "IndexaCapitalItem" => :indexa_capital_items - }.freeze - private_constant :SYNCABLE_ASSOCIATIONS - after_commit :update_family_sync_timestamp serialize :sync_stats, coder: JSON @@ -76,28 +60,31 @@ def clean def for_family(family, resource_owner: nil) query = where(syncable_type: "Family", syncable_id: family.id) + query = query.or(where(syncable_type: "Account", syncable_id: account_syncable_ids(family, resource_owner))) - SYNCABLE_ASSOCIATIONS.each do |syncable_type, association_name| - ids = syncable_ids_for_family(family, syncable_type, association_name, resource_owner) - next unless ids - - query = query.or(where(syncable_type: syncable_type, syncable_id: ids)) + family_syncable_associations.each do |association| + query = query.or( + where(syncable_type: association.klass.name, syncable_id: family.public_send(association.name).select(:id)) + ) end query end private - def syncable_ids_for_family(family, syncable_type, association_name, resource_owner) - if syncable_type == "Account" - return (resource_owner ? resource_owner.accessible_accounts : family.accounts) - .where(family_id: family.id) - .select(:id) - end - - return unless family.respond_to?(association_name) + def account_syncable_ids(family, resource_owner) + (resource_owner ? resource_owner.accessible_accounts : family.accounts) + .where(family_id: family.id) + .select(:id) + end - family.public_send(association_name).select(:id) + def family_syncable_associations + Family.reflect_on_all_associations(:has_many).select do |association| + association.name.to_s.end_with?("_items") && + association.klass.included_modules.include?(Syncable) + rescue NameError + false + end end end @@ -114,7 +101,7 @@ def api_error_payload return if stale? && error.blank? { - present: error.present?, + present: true, message: stale? ? "Sync became stale before completion" : "Sync failed" } end diff --git a/test/controllers/api/v1/syncs_controller_test.rb b/test/controllers/api/v1/syncs_controller_test.rb index 5c219118d..775d43884 100644 --- a/test/controllers/api/v1/syncs_controller_test.rb +++ b/test/controllers/api/v1/syncs_controller_test.rb @@ -126,6 +126,21 @@ class Api::V1::SyncsControllerTest < ActionDispatch::IntegrationTest refute_includes response.body, "provider token secret leaked" end + test "reports failed sync errors as present without raw error text" do + sync = Sync.create!( + syncable: @family, + status: "failed", + failed_at: Time.current, + error: nil + ) + + get api_v1_sync_url(sync), headers: api_headers(@read_only_api_key) + assert_response :success + + assert_equal true, JSON.parse(response.body).dig("data", "error", "present") + assert_equal "Sync failed", JSON.parse(response.body).dig("data", "error", "message") + end + test "omits stale sync error payload when no error is present" do sync = Sync.create!( syncable: @family, diff --git a/test/models/sync_test.rb b/test/models/sync_test.rb index 1b5f671b2..f77564ca4 100644 --- a/test/models/sync_test.rb +++ b/test/models/sync_test.rb @@ -214,6 +214,32 @@ class SyncTest < ActiveSupport::TestCase assert_equal [ newer_sync.id, older_sync.id ], ordered_ids end + test "for_family includes syncable provider item associations from family reflections" do + family = families(:dylan_family) + syncable_item_associations = Family.reflect_on_all_associations(:has_many).select do |association| + association.name.to_s.end_with?("_items") && + association.klass.included_modules.include?(Syncable) + rescue NameError + false + end + + syncs = syncable_item_associations.filter_map do |association| + syncable = family.public_send(association.name).first + next unless syncable + + Sync.create!(syncable: syncable, status: :completed) + end + + assert syncs.any?, "Expected syncable provider item fixtures for this family" + assert_equal syncs.map(&:id).sort, Sync.for_family(family).where(id: syncs.map(&:id)).pluck(:id).sort + end + + test "api error payload is present for failed syncs without raw error text" do + sync = Sync.create!(syncable: accounts(:depository), status: :failed) + + assert_equal({ present: true, message: "Sync failed" }, sync.api_error_payload) + end + test "expand_window_if_needed widens start and end dates on a pending sync" do initial_start = 1.day.ago.to_date initial_end = 1.day.ago.to_date From 9bc4c7fe925aeec6ff6c28cd56f55e11aee849c5 Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Tue, 5 May 2026 13:46:44 -0700 Subject: [PATCH 10/10] fix(api): make sync status schema nullable-compliant --- app/models/sync.rb | 1 - docs/api/openapi.yaml | 9 ++++----- spec/requests/api/v1/syncs_spec.rb | 18 +++++++++++++++--- spec/swagger_helper.rb | 7 +++---- .../api/v1/syncs_controller_test.rb | 4 ++-- test/models/sync_test.rb | 2 +- 6 files changed, 25 insertions(+), 16 deletions(-) diff --git a/app/models/sync.rb b/app/models/sync.rb index b613ce720..cc09b9ffd 100644 --- a/app/models/sync.rb +++ b/app/models/sync.rb @@ -101,7 +101,6 @@ def api_error_payload return if stale? && error.blank? { - present: true, message: stale? ? "Sync became stale before completion" : "Sync failed" } end diff --git a/docs/api/openapi.yaml b/docs/api/openapi.yaml index 198ca84a8..e638122b1 100644 --- a/docs/api/openapi.yaml +++ b/docs/api/openapi.yaml @@ -1634,11 +1634,8 @@ components: SyncErrorSummary: type: object required: - - present - message properties: - present: - type: boolean message: type: string SyncResource: @@ -1702,8 +1699,9 @@ components: format: date-time nullable: true error: - "$ref": "#/components/schemas/SyncErrorSummary" nullable: true + allOf: + - "$ref": "#/components/schemas/SyncErrorSummary" created_at: type: string format: date-time @@ -1716,8 +1714,9 @@ components: - data properties: data: - "$ref": "#/components/schemas/SyncResource" nullable: true + allOf: + - "$ref": "#/components/schemas/SyncResource" SyncCollection: type: object required: diff --git a/spec/requests/api/v1/syncs_spec.rb b/spec/requests/api/v1/syncs_spec.rb index d48f59027..90c2dccca 100644 --- a/spec/requests/api/v1/syncs_spec.rb +++ b/spec/requests/api/v1/syncs_spec.rb @@ -26,10 +26,22 @@ user: user, name: "API Docs Key", key: key, + display_key: key, scopes: %w[read_write], source: "web" ) end + let(:api_key_without_read_scope) do + key = ApiKey.generate_secure_key + ApiKey.new( + user: user, + name: "No Read Docs Key", + key: key, + display_key: key, + scopes: %w[write], + source: "web" + ).tap { |api_key| api_key.save!(validate: false) } + end let(:'X-Api-Key') { api_key.plain_key } let(:sync) { Sync.create!(syncable: family, status: "completed", completed_at: 1.minute.ago) } let(:id) { sync.id } @@ -56,7 +68,7 @@ end response "403", "forbidden" do - before { api_key.update_column(:scopes, [ "write" ]) } + let(:'X-Api-Key') { api_key_without_read_scope.plain_key } schema "$ref" => "#/components/schemas/ErrorResponse" run_test! @@ -84,7 +96,7 @@ end response "403", "forbidden" do - before { api_key.update_column(:scopes, [ "write" ]) } + let(:'X-Api-Key') { api_key_without_read_scope.plain_key } schema "$ref" => "#/components/schemas/ErrorResponse" run_test! @@ -113,7 +125,7 @@ end response "403", "forbidden" do - before { api_key.update_column(:scopes, [ "write" ]) } + let(:'X-Api-Key') { api_key_without_read_scope.plain_key } schema "$ref" => "#/components/schemas/ErrorResponse" run_test! diff --git a/spec/swagger_helper.rb b/spec/swagger_helper.rb index b993c233d..b98e48c80 100644 --- a/spec/swagger_helper.rb +++ b/spec/swagger_helper.rb @@ -906,9 +906,8 @@ }, SyncErrorSummary: { type: :object, - required: %w[present message], + required: %w[message], properties: { - present: { type: :boolean }, message: { type: :string } } }, @@ -929,7 +928,7 @@ syncing_at: { type: :string, format: :'date-time', nullable: true }, completed_at: { type: :string, format: :'date-time', nullable: true }, failed_at: { type: :string, format: :'date-time', nullable: true }, - error: { '$ref' => '#/components/schemas/SyncErrorSummary', nullable: true }, + error: { nullable: true, allOf: [ { '$ref' => '#/components/schemas/SyncErrorSummary' } ] }, created_at: { type: :string, format: :'date-time' }, updated_at: { type: :string, format: :'date-time' } } @@ -938,7 +937,7 @@ type: :object, required: %w[data], properties: { - data: { '$ref' => '#/components/schemas/SyncResource', nullable: true } + data: { nullable: true, allOf: [ { '$ref' => '#/components/schemas/SyncResource' } ] } } }, SyncCollection: { diff --git a/test/controllers/api/v1/syncs_controller_test.rb b/test/controllers/api/v1/syncs_controller_test.rb index 775d43884..ff0943fb9 100644 --- a/test/controllers/api/v1/syncs_controller_test.rb +++ b/test/controllers/api/v1/syncs_controller_test.rb @@ -121,7 +121,7 @@ class Api::V1::SyncsControllerTest < ActionDispatch::IntegrationTest assert_response :success data = JSON.parse(response.body)["data"] - assert_equal true, data["error"]["present"] + assert data["error"].present? assert_equal "Sync failed", data["error"]["message"] refute_includes response.body, "provider token secret leaked" end @@ -137,7 +137,7 @@ class Api::V1::SyncsControllerTest < ActionDispatch::IntegrationTest get api_v1_sync_url(sync), headers: api_headers(@read_only_api_key) assert_response :success - assert_equal true, JSON.parse(response.body).dig("data", "error", "present") + assert JSON.parse(response.body).dig("data", "error").present? assert_equal "Sync failed", JSON.parse(response.body).dig("data", "error", "message") end diff --git a/test/models/sync_test.rb b/test/models/sync_test.rb index f77564ca4..6fa8be3c9 100644 --- a/test/models/sync_test.rb +++ b/test/models/sync_test.rb @@ -237,7 +237,7 @@ class SyncTest < ActiveSupport::TestCase test "api error payload is present for failed syncs without raw error text" do sync = Sync.create!(syncable: accounts(:depository), status: :failed) - assert_equal({ present: true, message: "Sync failed" }, sync.api_error_payload) + assert_equal({ message: "Sync failed" }, sync.api_error_payload) end test "expand_window_if_needed widens start and end dates on a pending sync" do