From 0baf0062165461850d6771690d968ff57236cc91 Mon Sep 17 00:00:00 2001 From: Brian Fox Date: Wed, 21 Oct 2026 18:01:20 +0200 Subject: [PATCH 1/3] fix(agent): refuse a single-record action given several records [PRD-1704] A single-record action triggered on several ids ran on the first record only and answered success, since ActionContextSingle#get_record reads the first record. An API client, an MCP call or an approval request could send such a selection, and every record past the first was silently skipped. The action route now answers 422 for a single-record action whose selection names more than one id or is a select-all, before the permission check: no approval is asked for, approving a stored request runs nothing, and the form hooks refuse it too. Same fix as the Node agent (ForestAdmin/agent-nodejs#2028). Refs: PRD-1704 Co-Authored-By: Claude Opus 5.5 --- .../routes/action/actions.rb | 9 +++ .../routes/action/actions_spec.rb | 61 +++++++++++++++++++ 2 files changed, 70 insertions(+) diff --git a/packages/forest_admin_agent/lib/forest_admin_agent/routes/action/actions.rb b/packages/forest_admin_agent/lib/forest_admin_agent/routes/action/actions.rb index 3a57509b2..abaca24cd 100644 --- a/packages/forest_admin_agent/lib/forest_admin_agent/routes/action/actions.rb +++ b/packages/forest_admin_agent/lib/forest_admin_agent/routes/action/actions.rb @@ -127,6 +127,14 @@ def ensure_served_action(context) "The collection #{context.collection.name} does not have this smart action" end + # A single action only ever runs on the first record: refuse rather than skip the others. + def ensure_single_record(context, selection_ids) + return unless context.collection.schema[:actions][@action_name].scope == Types::ActionScope::SINGLE + return unless selection_ids[:are_excluded] || selection_ids[:ids].size > 1 + + raise Http::Exceptions::UnprocessableError, 'This action can only be triggered on one record at a time.' + end + # Recorded as pending before the action runs and confirmed after, so an action that takes the process # down with it still leaves evidence that it started. A failed run is worth recording too — "who tried # to run this" is usually the interesting part — and an action answering with an Error result failed @@ -229,6 +237,7 @@ def get_record_selection(args, context) # Restrict the filter to the selected records for single or bulk actions if context.collection.schema[:actions][@action_name].scope != Types::ActionScope::GLOBAL selection_ids = Utils::Id.parse_selection_ids(context.collection, args[:params]) + ensure_single_record(context, selection_ids) selected_ids = ConditionTreeFactory.match_ids(context.collection, selection_ids[:ids]) selected_ids = selected_ids.inverse if selection_ids[:are_excluded] filter = filter.override( diff --git a/packages/forest_admin_agent/spec/lib/forest_admin_agent/routes/action/actions_spec.rb b/packages/forest_admin_agent/spec/lib/forest_admin_agent/routes/action/actions_spec.rb index 2a42a043c..a2d2151b2 100644 --- a/packages/forest_admin_agent/spec/lib/forest_admin_agent/routes/action/actions_spec.rb +++ b/packages/forest_admin_agent/spec/lib/forest_admin_agent/routes/action/actions_spec.rb @@ -159,6 +159,67 @@ def execute(result: { type: 'Success', message: 'done' }, &raising) expect(execute).to eq({ type: 'Success', message: 'done' }) end end + + # A single action context only ever reads the first record, so a selection naming more than one + # would run on that record alone and still answer success. + describe 'a single action given more than one record' do + let(:action_scope) { ForestAdminDatasourceCustomizer::Decorators::Action::Types::ActionScope::SINGLE } + let(:permissions) { instance_spy(ForestAdminAgent::Services::Permissions) } + let(:context) do + double('context', collection: collection, caller: build_caller, permissions: permissions, + datasource: collection.datasource) + end + let(:message) { 'This action can only be triggered on one record at a time.' } + + def request(attributes) + { params: { data: { attributes: { values: {} }.merge(attributes) } } } + end + + before do + allow(route).to receive_messages(build: context, build_context_filter: filter) + allow(collection).to receive_messages(get_form: [], execute: { type: 'Success', message: 'done' }) + end + + it 'refuses two ids with a 422 and runs nothing' do + expect { route.handle_request(request(ids: %w[4 7])) } + .to raise_error(Http::Exceptions::UnprocessableError, message) + expect(collection).not_to have_received(:execute) + end + + it 'refuses two ids before checking the permissions, so no approval is asked for' do + expect { route.handle_request(request(ids: %w[4 7])) } + .to raise_error(Http::Exceptions::UnprocessableError, message) + expect(permissions).not_to have_received(:can_smart_action?) + end + + it 'refuses the approval of a request naming two ids and runs nothing' do + allow(route).to receive(:decode_signed_approval_request).and_return({ data: { attributes: {} } }) + + expect { route.handle_request(request(ids: %w[4 7], signed_approval_request: 'signed')) } + .to raise_error(Http::Exceptions::UnprocessableError, message) + expect(collection).not_to have_received(:execute) + end + + it 'refuses a select-all selection with a 422 and runs nothing' do + expect { route.handle_request(request(ids: [], all_records: true, all_records_ids_excluded: [])) } + .to raise_error(Http::Exceptions::UnprocessableError, message) + expect(collection).not_to have_received(:execute) + end + + it 'refuses loading the form with two ids and computes no form' do + expect { route.handle_hook_request(request(ids: %w[4 7])) } + .to raise_error(Http::Exceptions::UnprocessableError, message) + expect(collection).not_to have_received(:get_form) + end + + it 'still runs on exactly one id' do + route.handle_request(request(ids: %w[4])) + + expect(collection).to have_received(:execute).with( + context.caller, 'Refund', {}, having_attributes(condition_tree: having_attributes(field: 'id', value: 4)) + ) + end + end end end end From 13185c00125d5baf1a0ddddf9e5dd517563cf75b Mon Sep 17 00:00:00 2001 From: Brian Fox Date: Wed, 21 Oct 2026 18:32:40 +0200 Subject: [PATCH 2/3] test(agent): sign a real approval request instead of stubbing the private decoder [PRD-1704] Refs: PRD-1704 Co-Authored-By: Claude Opus 5.5 --- .../spec/lib/forest_admin_agent/routes/action/actions_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/forest_admin_agent/spec/lib/forest_admin_agent/routes/action/actions_spec.rb b/packages/forest_admin_agent/spec/lib/forest_admin_agent/routes/action/actions_spec.rb index a2d2151b2..a66a0e7e1 100644 --- a/packages/forest_admin_agent/spec/lib/forest_admin_agent/routes/action/actions_spec.rb +++ b/packages/forest_admin_agent/spec/lib/forest_admin_agent/routes/action/actions_spec.rb @@ -193,9 +193,9 @@ def request(attributes) end it 'refuses the approval of a request naming two ids and runs nothing' do - allow(route).to receive(:decode_signed_approval_request).and_return({ data: { attributes: {} } }) + signed = JWT.encode({ data: { attributes: { ids: %w[4 7] } } }, Facades::Container.cache(:env_secret), 'HS256') - expect { route.handle_request(request(ids: %w[4 7], signed_approval_request: 'signed')) } + expect { route.handle_request(request(ids: %w[4 7], signed_approval_request: signed)) } .to raise_error(Http::Exceptions::UnprocessableError, message) expect(collection).not_to have_received(:execute) end From e92ccd1eb91b48abf60593e8e4ac50d84fb54b51 Mon Sep 17 00:00:00 2001 From: Matt Date: Thu, 22 Oct 2026 17:12:37 +0200 Subject: [PATCH 3/3] fix(agent): refuse a single-record action given no record id [PRD-1704] The route refused a single action on several ids or a select-all but let an empty selection through: the action then ran on a match_none filter and answered success, or raised on a nil record. A single action now runs only on exactly one id, as the Node and Python agents do. Co-Authored-By: Claude Fable 5.1 --- .../lib/forest_admin_agent/routes/action/actions.rb | 4 ++-- .../lib/forest_admin_agent/routes/action/actions_spec.rb | 8 +++++++- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/packages/forest_admin_agent/lib/forest_admin_agent/routes/action/actions.rb b/packages/forest_admin_agent/lib/forest_admin_agent/routes/action/actions.rb index abaca24cd..6b9689fa7 100644 --- a/packages/forest_admin_agent/lib/forest_admin_agent/routes/action/actions.rb +++ b/packages/forest_admin_agent/lib/forest_admin_agent/routes/action/actions.rb @@ -127,10 +127,10 @@ def ensure_served_action(context) "The collection #{context.collection.name} does not have this smart action" end - # A single action only ever runs on the first record: refuse rather than skip the others. + # A single action runs on its first record, or on nothing: refuse any selection but one id. def ensure_single_record(context, selection_ids) return unless context.collection.schema[:actions][@action_name].scope == Types::ActionScope::SINGLE - return unless selection_ids[:are_excluded] || selection_ids[:ids].size > 1 + return unless selection_ids[:are_excluded] || selection_ids[:ids].size != 1 raise Http::Exceptions::UnprocessableError, 'This action can only be triggered on one record at a time.' end diff --git a/packages/forest_admin_agent/spec/lib/forest_admin_agent/routes/action/actions_spec.rb b/packages/forest_admin_agent/spec/lib/forest_admin_agent/routes/action/actions_spec.rb index a66a0e7e1..7a0be720a 100644 --- a/packages/forest_admin_agent/spec/lib/forest_admin_agent/routes/action/actions_spec.rb +++ b/packages/forest_admin_agent/spec/lib/forest_admin_agent/routes/action/actions_spec.rb @@ -162,7 +162,7 @@ def execute(result: { type: 'Success', message: 'done' }, &raising) # A single action context only ever reads the first record, so a selection naming more than one # would run on that record alone and still answer success. - describe 'a single action given more than one record' do + describe 'a single action given anything but one record' do let(:action_scope) { ForestAdminDatasourceCustomizer::Decorators::Action::Types::ActionScope::SINGLE } let(:permissions) { instance_spy(ForestAdminAgent::Services::Permissions) } let(:context) do @@ -200,6 +200,12 @@ def request(attributes) expect(collection).not_to have_received(:execute) end + it 'refuses an empty selection with a 422 and runs nothing' do + expect { route.handle_request(request(ids: [])) } + .to raise_error(Http::Exceptions::UnprocessableError, message) + expect(collection).not_to have_received(:execute) + end + it 'refuses a select-all selection with a 422 and runs nothing' do expect { route.handle_request(request(ids: [], all_records: true, all_records_ids_excluded: [])) } .to raise_error(Http::Exceptions::UnprocessableError, message)