[christophebrun-forest]

Closes PRD-1026. Same situation as #423 (PRD-1025).

Context

PRD-1026 reports that the action routes apply the caller's filters without checking that the caller can read the collections those filters reach. The serious case is executing an action that has a conditional trigger or approval rule. SmartActionChecker counts the records that match the filter, and the request then ends three different ways: the action runs, it needs approval, or it is forbidden. Which one depends on the caller's filter, so the caller learns one bit per request about a column they cannot read, and the action never runs.

That is no longer true on main. #414 (PRD-1667) routed get_record_selection through build_context_filter, which calls assert_can_read_query_fields. Both entry points build the selection before anything uses it:

• handle_request builds it before can_smart_action?, so a filter on an unreadable column gets a 403 before any count;
• handle_hook_request builds it before get_form, so the filter never reaches the customer's form hooks.

The guard covers search too, which the ticket assumed could not reach these routes. The out-of-date comment the ticket mentioned ("Match user filter + search + scope? + segment") is already gone.

The spec stubbed the guard to nil. The only test that checked it was called was on the hook route, with search alone. Nothing tested filters, or that a refusal stops the request before the count.

Not covered here: the related-data parent

get_record_selection also narrows the selection with a parent the caller names (parent_collection_name, parent_collection_id, parent_association_name), with no read or scope check on that parent collection. That feeds the same count, so it is the same one-bit oracle by another input. It is not about the caller's filters, so it is tracked separately in PRD-1929 and stays open after this PR.

Changes

Specs only, no application code changed.

• Fixture: book gets a many-to-one holder relation to a new holder collection with a national_id column, so the caller's filter targets a related collection, as in the ticket.
• Select-all contexts: the searched/segmented case and the new caller-filter case share one parent context, with the subset query as a let.
• handle_request, with an And filter on holder:national_id and title:
  • the guard receives the parsed condition tree, and every condition it checked is in the filter can_smart_action? counts with (compared by object identity, whatever the tree's shape);
  • when the guard refuses that field, can_smart_action?, get_form and execute are not called.
• handle_hook_request:
  • the filter get_form receives carries the very conditions the guard checked;
  • when the guard refuses, get_form is not called.
• Or filter: the guarded Or branch reaches the count as is, so a route that kept the conditions but rebuilt the branch as an And would fail.
• Other filter sources: a filter sent in params[:filters] or params[:filter], outside all_records_subset_query, is refused the same way.
• Every captured argument: besides the filter, the specs assert the collection, action_name: and subset query handed to can_smart_action?, that the guard gets no search besides the condition tree, and the caller, action name, data, filter attributes and meta handed to get_form.

The refusal stub only raises when the checked tree contains holder:national_id, so the assertions depend on the field being filtered rather than on a fixed message. get_form and execute are stubbed and asserted on the published collection, which is the one the route calls. The real guard's handling of relation paths is unit-tested in related_read_permissions_spec.rb.

Four checks, all reverted after running:

• making query_handler.rb pass condition_tree: nil to the guard fails all 7 new examples;
• making it rewrite an Or caller tree as an And of the same conditions fails only the Or example;
• passing @route_collection instead of context.collection to can_smart_action? fails 2 examples;
• passing action_name: 'bar' fails 3.

The published collection is compared with equal? and a short message: on a mismatch, eq / be print both decorated collections, and that output never finishes.

Definition of Done

General

• Write an explicit title for the Pull Request, following Conventional Commits specification
• Test manually the implemented changes (n/a: specs only)
• Validate the code quality (indentation, syntax, style, simplicity, readability)

Security

• Consider the security impact of the changes made

🤖 Generated with Claude Code

Note

Add specs locking the read-field guard behavior on action record selections

• Adds regression specs for bulk-action select-all and form hooks when caller filters reference relation fields such as holder:national_id
• Verifies the guarded condition tree is the one used for action authorization and passed to form hooks, that a rejected relation field blocks authorization counting and execution, and that Or conditions keep the guarded branch
• Covers filters passed through either params[:filters] or params[:filter]
• Extends the Actions spec fixture in actions_spec.rb with a book-to-holder relation, a holder collection, and the needed filter operators
• Makes the bulk-action subset query configurable and checks the form filter and search-field read permission for searched, segmented lists

Changes since #424 opened

• Tightened the guarded_tree helper expectation on permissions.assert_can_read_query_fields in the Actions spec. [31287b8]
• Added a counted_filter helper and switched the 'checks the very condition tree the authorization then counts with' and 'counts with the guarded branch as is' examples to use it. [31287b8]
• Strengthened the get_form call expectations in the 'hands the form hooks only a filter the guard checked' example. [31287b8]
• Added a bearer_caller matcher and replaced the generic caller expectation with it in the get_form expectations [4b1b6a5]
• Added an assertion on the meta argument in the 'when the list is searched and segmented' example [4b1b6a5]
• Added helper lets and tightened filter and condition tree assertions in the ForestAdminAgent::Routes::Action::Actions spec. [5858c5f]
• Added single-invocation quantifiers to call expectations in the ForestAdminAgent::Routes::Action::Actions spec. [5858c5f]

^{Macroscope summarized 1c115e4.}

[linear-code]

PRD-1026

[qltysh]

Coverage Impact

This PR will not change total coverage.

🚦 See full report on Qlty Cloud »

🛟 Help

• Diff Coverage: Coverage for added or modified lines of code (excludes deleted files). Learn more.

• Total Coverage: Coverage for the whole repository, calculated as the sum of all File Coverage. Learn more.

• File Coverage: Covered Lines divided by Covered Lines plus Missed Lines. (Excludes non-executable lines including blank lines and comments.)

  • Indirect Changes: Changes to File Coverage for files that were not modified in this PR. Learn more.

[hercemer42]

Spec (PRD-1026): conforms.

[hercemer42]

Forest Validator · Verify fixes: ✅ 1 closed

When (UTC)	Commit	Step	Result
05/11 14:37	5858c5f	Verify fixes	✅ 1 closed 1 finding ✅ closed · Violates conventions: a route counting the guarded filter with the wrong collection or action name … · run 424-20261105143628-hercemer42-verify

1 earlier step

When (UTC)	Commit	Step	Result
03/11 11:32	1c115e4	Review	findings posted · run 424-20261103112700-hercemer42

[hercemer42]

Approved after /verify-fixes run 424-20261105143628-hercemer42-verify: 1 findings of 424-20261103112700-hercemer42 closed or accepted, none reopened.