Skip to content

fix(pipeline): gate card writes on pipeline_items.update, not pipelines.update - #272

Merged
gomessguii merged 5 commits into
developfrom
feat/pipeline-items-update-gate
Aug 18, 2026
Merged

fix(pipeline): gate card writes on pipeline_items.update, not pipelines.update#272
gomessguii merged 5 commits into
developfrom
feat/pipeline-items-update-gate

Conversation

@pastoriniMatheus

@pastoriniMatheus pastoriniMatheus commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

Problem

PipelineItemsController authorized every card write (create, move_to_stage, edit) against the parent Pipeline's #update? — i.e. pipelines.update, the manager-level permission. Consequences: the default agent could delete a whole stage (pipeline_stages.delete) but not move a card, and the only way to unblock cards — granting pipelines.update — also unlocked archive / set_as_default / pipeline_service_definitions / products (over-grant).

Fix

  • New PipelinePolicy#update_items? = pipeline_items.update + the same accessible_record? scope.
  • PipelineItemsController#ensure_authorized_user authorizes WRITE_ACTIONS against :update_items?; reads stay :view?.
  • Add pipeline_items.update to the catalog mirror.

Tests

  • spec/requests/api/v1/pipeline_items_write_authz_spec.rb — updated to the card-write policy level (update_items?).
  • spec/requests/api/v1/pipeline_items_permission_rbac_spec.rb (new) — proves the split: denied without the key, denied for a holder of pipelines.update alone, allowed with pipeline_items.update, and the key exists in the catalog.

Run via spec/requests/api/v1/*_rbac_spec.rb + the pipeline authz spec.

Deploy together

With the evolution-foundation/evo-auth-service-community PR that adds pipeline_items.update to the catalog + agent seed.

🤖 Generated with Claude Code

Summary by Sourcery

Gate pipeline card mutations with the dedicated pipeline_items.update permission while keeping destructive operations manager-authorized.

New Features:

  • Introduce the dedicated pipeline_items.update permission for routine pipeline card writes while retaining manager-level authorization for card deletion.
  • Authorize cross-pipeline moves against both the target and source pipelines to prevent inaccessible cards from being relocated.

Bug Fixes:

  • Correct the conversation update action so the routed endpoint executes and is subject to card-write authorization.

Enhancements:

  • Separate pipeline card-write authorization from manager-level pipeline updates while preserving pipeline visibility checks.

Tests:

  • Update pipeline item authorization coverage and add RBAC tests for permission separation, deletion boundaries, conversation updates, cross-pipeline moves, and catalog presence.

Chores:

  • Register pipeline_items.update in the permission catalog mirror and update authorization guard expectations.

…es.update

PipelineItemsController authorized every card write (create, move_to_stage,
edit) against the parent Pipeline's #update? — i.e. pipelines.update, the
manager-level permission. So the default agent could delete a whole stage
(pipeline_stages.delete) but not move a card, and unblocking cards by granting
pipelines.update also unlocked archive/set_as_default/service_definitions
(over-grant).

Introduce PipelinePolicy#update_items? (pipeline_items.update + the same
accessible_record? scope) and authorize card writes against it. Reads stay at
#view?. Adds pipeline_items.update to the catalog mirror.

Specs: pipeline_items_write_authz_spec now stubs update_items?; new
pipeline_items_permission_rbac_spec proves the split (denied without the key,
denied for a holder of pipelines.update alone, allowed with pipeline_items.update,
and the key exists in the catalog).

Deploy together with the evo-auth-service-community seed/catalog change.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@sourcery-ai

sourcery-ai Bot commented Aug 17, 2026

Copy link
Copy Markdown

Reviewer's Guide

Introduces a new card-level pipeline_items.update authorization path in PipelinePolicy and PipelineItemsController so card mutations are gated by a dedicated permission instead of the broader pipelines.update, and adds corresponding RBAC catalog entry and tests to verify behavior and catalog presence.

Sequence diagram for pipeline_items.write authorization using update_items?

sequenceDiagram
  actor User
  participant PipelineItemsController
  participant PipelinePolicy

  User->>PipelineItemsController: create/move/update pipeline_item
  PipelineItemsController->>PipelineItemsController: WRITE_ACTIONS.include?(action_name)
  alt write action
    PipelineItemsController->>PipelinePolicy: authorize(@pipeline, :update_items?)
    PipelinePolicy->>PipelinePolicy: update_items?
    PipelinePolicy->>PipelinePolicy: permitted_item_write?
    PipelinePolicy->>PipelinePolicy: accessible_record?
  else read action
    PipelineItemsController->>PipelinePolicy: authorize(@pipeline, :view?)
  end
Loading

File-Level Changes

Change Details Files
Add a dedicated card-write policy predicate that gates pipeline item mutations on pipeline_items.update while preserving existing accessibility checks.
  • Introduce PipelinePolicy#update_items? that requires permitted_item_write? and accessible_record?
  • Define PipelinePolicy#permitted_item_write? to allow admins or users with pipeline_items.update
  • Document behavior and relationship to existing update? and visibility logic in policy comments
app/policies/pipeline_policy.rb
Switch PipelineItemsController to authorize write actions via the new update_items? policy instead of update?.
  • Update ensure_authorized_user to call authorize @pipeline, :update_items? for WRITE_ACTIONS while leaving reads on :view?
  • Clarify controller comments to describe the new card-write vs manager-level permission split and preserved accessibility behavior
app/controllers/api/v1/pipeline_items_controller.rb
Align existing request specs with the new update_items? gate for pipeline item writes.
  • Update pipeline_items_write_authz_spec to stub PipelinePolicy#update_items? instead of #update? when simulating denied card writes
  • Refresh spec documentation/comments to describe card-write vs view-level split and defer permission-key proofs to a dedicated RBAC spec
spec/requests/api/v1/pipeline_items_write_authz_spec.rb
Add a dedicated RBAC request spec to prove that pipeline_items.update, not pipelines.update, gates card writes and that the permission exists in the catalog mirror.
  • Introduce pipeline_items_permission_rbac_spec to exercise card creation under different permission combinations (none, pipelines.update only, pipeline_items.update)
  • Stub EvoAuthService#check_user_permission via a helper to simulate per-test permission sets
  • Use a public pipeline to isolate permission behavior from visibility constraints
  • Verify that pipeline_items.update is present in spec/fixtures/rbac/permission_catalog.yml
spec/requests/api/v1/pipeline_items_permission_rbac_spec.rb
spec/fixtures/rbac/permission_catalog.yml

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@pastoriniMatheus

Copy link
Copy Markdown
Contributor Author

🔗 Par de deploy com #85 (auth). Este é o gate; a chave pipeline_items.update vem no auth #85. Subir os dois juntos — gate sem a chave 403a; chave sem o gate é no-op.

Ordem geral da leva (desmembramento do antigo PR #84):

Os 4 PRs do auth tocam os mesmos arquivos (db/seeds/rbac.rb, rbac_spec.rb, versão do db/schema.rb) → mergeiam em ordem, com rebase: #85 (178) → #86 (181) → #87 (182) → #88 (190). schema.rb resolve para a versão maior.

Par de deploy: #85 (auth, a chave pipeline_items.update) ⟷ #272 (CRM, o gate) — subir juntos.

Independentes (sem ordem): #281 (CRM-181 comentário), #282 (CRM-182 spec), #283 (CRM-190 spec), #310 (CRM-178 front).

🤖 Generated with Claude Code

Matheus Pastorini and others added 4 commits August 18, 2026 15:21
…M-178 review)

Addresses the two CRM-178 review blockers on this gate:

1. destroy no longer grants the agent delete-card. `destroy` was in WRITE_ACTIONS,
   so it authorized against pipeline_items.update (which the agent seed grants) —
   letting an agent delete a card and cascade-delete its stage_movements/tasks/
   products. It now authorizes against :update? (pipelines.update, manager),
   mirroring CRM-182 keeping deletes off the agent. Card create/move/edit stay on
   :update_items? (pipeline_items.update); reads on :view?.

2. CI regression fixed. pipeline_items_archived_spec stubbed PipelinePolicy#update?,
   the predicate this PR retired for card writes — the create/move_conversation
   examples fell through to the real #update_items? and 401'd. The spec now also
   stubs #update_items? so it stays focused on the archived-pipeline guard.

The RBAC spec grows the proof: move_to_stage authz (agent key), and destroy is
DENIED for a holder of pipeline_items.update (card survives) but ALLOWED for
pipelines.update. 14 examples, 0 failures.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…d) (CRM-178 review)

Closes review finding 7: the card-write positive asserted only not-401. It now
POSTs a real contact card and asserts the card is created (2xx + PipelineItem
count +1) with pipeline_items.update, so the split is proven by an actual create,
not just an open gate. move_to_stage keeps the gate-level assertion (its
conversation-first lookup is out of scope for an authz spec). Verified: 14
examples, 0 failures; and the destroy pair is RED against the pre-fix controller
(agent got 200 on delete), so the guard catches the leak, it does not lock it in.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- M6: pipeline_items_write_authz_spec.rb -> *_write_authz_rbac_spec.rb so it
  matches the community-parity `*_rbac_spec.rb` glob and actually runs in CI
  (its update? -> update_items? change was never executed before).
- L8: products_controller comment was false ("mirrors ...: mutations on :update?").
  Product-catalog writes are manager-level (pipelines.update via require_permissions);
  only pipeline CARD writes moved to pipeline_items.update. Comment corrected.
- L9: register `pipeline_items.update` via EvoPermissionConcern.register_permission_key
  so the catalog-conformance guard covers it, even though the gate is Pundit
  (accessible_record? needs the resolved pipeline) not the named check_*_permission!.
- L10: `def update_notesconversation` was a typo — no route reached it while the
  route/WRITE_ACTIONS/set_pipeline_item all referenced `update_conversation`,
  so PATCH .../update_conversation hit a dead action. Renamed to update_conversation.

QA: pipeline_items_write_authz_rbac + permission_rbac + archived + catalog
conformance specs — 18 examples, 0 failures (1 pending: auth SSOT cross-check
skips when the sibling repo is absent).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… move

move_conversation relocates a card OUT of whatever pipeline currently holds it,
but ensure_authorized_user only authorized the pipeline named in the URL. With
card writes now agent-level, an agent could pull a card out of a private funnel
it cannot see by naming one it can — the opposite of what update_items? +
accessible_record? promise. Proven: without the guard the request answers 200
and the card leaves the private pipeline.

Also covers update_conversation, which this PR brought back from a dead route
(the action was defined as the typo update_notesconversation, so no request ever
reached it): a gate example plus one proving the move and the note actually
land. Both live in the *_rbac_spec that CI runs.

Fixes the two stale comments the split left behind (PipelinePolicy#accessible_record?
and the pipeline_items entry in mutating_actions_gate_guard_spec).
@gomessguii
gomessguii merged commit f8b354d into develop Aug 18, 2026
12 checks passed
@gomessguii
gomessguii deleted the feat/pipeline-items-update-gate branch August 18, 2026 20:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants