From d6dbea2fb22b811ba0aa02f0a29c3961d05f803b Mon Sep 17 00:00:00 2001 From: Lloyd Watkin Date: Mon, 21 Sep 2026 09:26:59 +0100 Subject: [PATCH] Cover update and a withheld attribute for block-form permit_params Follow-up to #19, which fixed the four findings in #18 but left two gaps in the e2e coverage of the block-form `permit_params` shape. `resolve_permitted` gates `update` as well as `create`, and only `create` had an example. Reverting the fix leaves the new update example failing, so the second call site is now covered by something that can see it. Both branches of `Assignment`'s block permitted only columns the fixture already wrote, so no example could tell a block that filters from one whose result is ignored. `secret_note` is a column neither branch permits, and an example now asserts a write never reaches it. Mutating the block to permit it fails that example and no other. The README gains the two behaviours #19 changed but did not describe: an association's fields are reported under `nested` whether declared with `has_many` or `inputs for:`, and a form block declaring no inputs of its own is described from the permitted params. Co-Authored-By: Claude Opus 5 (1M context) --- README.md | 15 ++++++-- spec/e2e/fixture_app/app/admin/assignments.rb | 5 +++ ...01000007_add_secret_note_to_assignments.rb | 5 +++ spec/e2e/fixture_app/db/seeds.rb | 16 +++++++++ spec/e2e/mcp_tools_spec.rb | 34 +++++++++++++++++++ 5 files changed, 72 insertions(+), 3 deletions(-) create mode 100644 spec/e2e/fixture_app/db/migrate/20260101000007_add_secret_note_to_assignments.rb diff --git a/README.md b/README.md index 8cc4c23..a7c0403 100644 --- a/README.md +++ b/README.md @@ -123,7 +123,10 @@ action, so a write from MCP is the same write the admin UI makes: so a `slug` sent to a `Post` permitting only `title` and `body` never reaches the record. A resource that declares no `permit_params` at all (like `Tag`) is refused outright, with a message saying so — ActiveAdmin cannot - write such a resource through its own forms either. + write such a resource through its own forms either. A block-form + `permit_params` is evaluated in controller context as the authenticated MCP + user, so a block varying the writable set by user gets the same answer it + would give that user in admin. - **Your callbacks run** — ActiveAdmin's `before_build`, `before_create`, `before_save`, `after_update` and friends all fire, because the controller action is what fires them. A `before_create` that fills in a `slug` the form @@ -154,8 +157,14 @@ at render time, so there is nothing to read. The response's `source` says which of the two you are looking at. Either way every field is annotated from the model with the column type it is -stored in and whether the model validates its presence. `has_many` blocks are -reported under `nested` rather than flattened in with the record's own fields. +stored in and whether the model validates its presence. An associated record's +fields — declared with `has_many`, or with `inputs for: :author` — are reported +under `nested` rather than flattened in with the record's own, because a write +against the parent would drop them. + +A form block that declares no inputs of its own is described from +`permit_params` too. A bare `f.inputs` is legal, and Formtastic only expands it +against the model at render time, so there is nothing in the block to read. `action:` selects the gate, not the shape — ActiveAdmin uses one form block for both. `"new"` (the default) requires the resource to register `create` and pass diff --git a/spec/e2e/fixture_app/app/admin/assignments.rb b/spec/e2e/fixture_app/app/admin/assignments.rb index 7b7be33..ab4ba11 100644 --- a/spec/e2e/fixture_app/app/admin/assignments.rb +++ b/spec/e2e/fixture_app/app/admin/assignments.rb @@ -8,6 +8,11 @@ # expand a bare `f.inputs` at render time, as ActiveAdmin's own default # form does — so there is nothing in the block to read and the description # has to come from the permitted params instead. +# +# `secret_note` is a column neither branch of the block permits, so the suite +# has something the block withholds as well as something it grants: an example +# that only ever writes a permitted attribute cannot tell a block that filters +# from one whose result is ignored. ActiveAdmin.register Assignment do permit_params do current_admin_user ? %i[name notes] : %i[name] diff --git a/spec/e2e/fixture_app/db/migrate/20260101000007_add_secret_note_to_assignments.rb b/spec/e2e/fixture_app/db/migrate/20260101000007_add_secret_note_to_assignments.rb new file mode 100644 index 0000000..29fde5e --- /dev/null +++ b/spec/e2e/fixture_app/db/migrate/20260101000007_add_secret_note_to_assignments.rb @@ -0,0 +1,5 @@ +class AddSecretNoteToAssignments < ActiveRecord::Migration[7.2] + def change + add_column :assignments, :secret_note, :string + end +end diff --git a/spec/e2e/fixture_app/db/seeds.rb b/spec/e2e/fixture_app/db/seeds.rb index d942e22..48d43db 100644 --- a/spec/e2e/fixture_app/db/seeds.rb +++ b/spec/e2e/fixture_app/db/seeds.rb @@ -41,3 +41,19 @@ # something to refuse. Nothing mutates it: the examples that name it assert it # is unchanged. Tag.find_or_initialize_by(name: "fantasy").save! + +# Written through the MCP tools by the examples covering a block-form +# `permit_params`, so seeded restoratively on a name no example rewrites. Two +# rows, so an example rewriting one can assert the other was left alone. +# `secret_note` is permitted by neither branch of that block, so an example +# can prove a write never reaches it. +{ + "Saturday sort" => "Two volunteers", + "Sunday sort" => "One volunteer", +}.each do |name, notes| + Assignment.find_or_initialize_by(name: name).tap do |assignment| + assignment.notes = notes + assignment.secret_note = "Not for the rota" + assignment.save! + end +end diff --git a/spec/e2e/mcp_tools_spec.rb b/spec/e2e/mcp_tools_spec.rb index eeb3360..e163f46 100644 --- a/spec/e2e/mcp_tools_spec.rb +++ b/spec/e2e/mcp_tools_spec.rb @@ -267,6 +267,40 @@ def posts expect(reread["title"]).to eq("Small Gods") end + # `resolve_permitted` gates update as well as create, and the block it + # resolves is the resource's only statement of what may be written. + context "for a resource whose permit_params is a block reading controller state" do + def assignment(name) + client.call_tool("query", resource: "Assignment", q: { name_eq: name })["records"].first + end + + it "updates the named record, rather than refusing it as a resource that permits nothing" do + result = client.call_tool( + "update", + resource: "Assignment", + id: assignment("Saturday sort")["id"], + attributes: { notes: "Three volunteers" } + ) + + expect(result["error"]).to be_nil + expect(result["updated"]).to eq(["notes"]) + expect(assignment("Saturday sort")["notes"]).to eq("Three volunteers") + expect(assignment("Sunday sort")["notes"]).to eq("One volunteer") + end + + it "drops an attribute neither branch of the block permits" do + result = client.call_tool( + "update", + resource: "Assignment", + id: assignment("Saturday sort")["id"], + attributes: { notes: "Three volunteers", secret_note: "tampered" } + ) + + expect(result["updated"]).to eq(["notes"]) + expect(assignment("Saturday sort")["secret_note"]).to eq("Not for the rota") + end + end + it "refuses to update a resource that declares no permitted params" do tag_id = client.call_tool("query", resource: "Tag")["records"].first["id"]