Fixes #29467: expose Trigger in ingestionPipeline descriptor and require EditAll to kill#29530
Merged
Merged
Conversation
…quire EditAll to kill
Contributor
✅ PR checks passedThe linked issue has a description and all required Shipping project fields set. Thanks! |
github-actions
Bot
added
backend
safe to test
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
…d make editor case orchestrator-independent
|
Contributor
🟡 Playwright Results — all passed (26 flaky)✅ 4446 passed · ❌ 0 failed · 🟡 26 flaky · ⏭️ 38 skipped
🟡 26 flaky test(s) (passed on retry)
How to debug locally# Download playwright-test-results-<shard> artifact and unzip
npx playwright show-trace path/to/trace.zip # view trace |
harshach
approved these changes
Jun 26, 2026
Contributor
|
Changes have been cherry-picked to the 1.13 branch. |
github-actions Bot
pushed a commit
that referenced
this pull request
Jun 29, 2026
…ire EditAll to kill (#29530) * fix(ingestion-pipeline): expose Trigger in resource descriptor and require EditAll to kill * test(ingestion-pipeline): assert ForbiddenException for kill authz and make editor case orchestrator-independent * test(ingestion-pipeline): trim kill-test comments to the essentials (cherry picked from commit f7dd54f)
Contributor
|
Changes have been cherry-picked to the 1.12.13 branch. |
github-actions Bot
pushed a commit
that referenced
this pull request
Jun 29, 2026
…ire EditAll to kill (#29530) * fix(ingestion-pipeline): expose Trigger in resource descriptor and require EditAll to kill * test(ingestion-pipeline): assert ForbiddenException for kill authz and make editor case orchestrator-independent * test(ingestion-pipeline): trim kill-test comments to the essentials (cherry picked from commit f7dd54f)
Code Review ✅ ApprovedExposes Trigger in the ingestionPipeline descriptor and enforces EditAll for kill operations, closing existing RBAC gaps. Verified with new integration tests for permission scoping and authorization enforcement. OptionsDisplay: compact → Showing less information. Comment with these commands to change:
Was this helpful? React with 👍 / 👎 | Gitar |
mohityadav766
added a commit
that referenced
this pull request
Jun 29, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #29467
Describe your changes
Backend half of the ingestion-pipeline permission alignment. Two fixes in
IngestionPipelineResource:1. Expose
Triggerin theingestionPipelineresource descriptor. Since #28109 (1.12.9),POST /services/ingestionPipelines/trigger/{id}authorizesingestionPipeline.Trigger, butTriggerwas never added to the resource descriptor — so it could only be granted under All Resources (which over-grants, since the same op governs Apps, AI Automations, and Dynamic Agents).getEntitySpecificOperations()now includesMetadataOperation.TRIGGER, so the policy editor lists it under Ingestion Pipeline and admins can grant it scoped.2. Require
EditAllto kill a pipeline.POST /services/ingestionPipelines/kill/{id}ran through the standard read path (getInternal→getViewOperations), so it only requiredingestionPipeline.ViewAll— a read-level permission for a disruptive action, and a mismatch with the UI which gates Kill onEditAll.killIngestionnow authorizesMetadataOperation.EDIT_ALLup front. This adds the edit gate on top of the pre-existing view read (it does not relax anything);EditAlldoes not subsume view ops, which is why the read path'sViewAllis unchanged.Type of change
Tests
Added to
IngestionPipelineOwnerInheritanceIT(the IT introduced by #28109):test_ingestionPipelineDescriptorExposesTrigger—GET /v1/policies/resourcesreturns theingestionPipelinedescriptor withTriggerin its operations.test_killRequiresEditPermission— aViewAll-only user can read a pipeline but is forbidden from killing it (fails without fix 2); aViewAll+EditAlluser can kill.mvn spotless:applyapplied;openmetadata-servicecompiles andopenmetadata-integration-teststest-compiles clean.Related
EditAllinstead ofTrigger): Gate ingestion pipeline Run action on Operation.Trigger instead of EditAll (UI/backend permission mismatch since 1.12.9) #29468Greptile Summary
This PR closes two RBAC gaps in
IngestionPipelineResource: it addsMetadataOperation.TRIGGERto the entity's descriptor so the permission can be granted scoped to Ingestion Pipeline in the policy editor, and it inserts an explicitEditAllauthorization check at the entry ofkillIngestionto stop view-only users from calling the kill endpoint.getEntitySpecificOperations()now returnsTRIGGERalongside the existing entity-specific ops, making it visible in the policy editor without requiring an over-broad "All Resources" grant.killIngestionnow callsauthorizer.authorize(…, EDIT_ALL, …)beforegetInternal(), so aViewAll-only principal receives a 403 rather than reaching the orchestrator client.IngestionPipelineOwnerInheritanceIT— one asserting the descriptor containsTRIGGER, one verifying theViewAll/EditAllboundary usingForbiddenException-aware assertions.Confidence Score: 5/5
Safe to merge — both changes are minimal additive authorization checks with no risk of relaxing existing access controls.
The backend change is a two-line addition to the descriptor and a three-line guard at the top of killIngestion. Neither path removes or weakens an existing check; the kill endpoint only gains a stricter gate. The integration tests directly exercise both scenarios (view-only to 403, edit to pass) and address the concerns raised in the previous review round.
No files require special attention.
Important Files Changed
Sequence Diagram
%%{init: {'theme': 'neutral'}}%% sequenceDiagram participant Client participant KillEndpoint as POST /kill/{id} participant Authorizer participant DB as Entity Store participant PSC as PipelineServiceClient Client->>KillEndpoint: "POST kill/{id}" Note over KillEndpoint: NEW: explicit EditAll check KillEndpoint->>Authorizer: authorize(EDIT_ALL, getResourceContextById(id)) Authorizer-->>KillEndpoint: 403 if not EditAll Note over KillEndpoint: Pre-existing: getInternal ViewAll check KillEndpoint->>DB: getInternal (fetches + ViewAll auth) DB-->>KillEndpoint: IngestionPipeline KillEndpoint->>KillEndpoint: decryptOrNullify(pipeline) alt "pipelineServiceClient == null" KillEndpoint-->>Client: 200 Pipeline Client Disabled else KillEndpoint->>PSC: killIngestion(pipeline) PSC-->>KillEndpoint: PipelineServiceClientResponse KillEndpoint-->>Client: response end%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%% sequenceDiagram participant Client participant KillEndpoint as POST /kill/{id} participant Authorizer participant DB as Entity Store participant PSC as PipelineServiceClient Client->>KillEndpoint: "POST kill/{id}" Note over KillEndpoint: NEW: explicit EditAll check KillEndpoint->>Authorizer: authorize(EDIT_ALL, getResourceContextById(id)) Authorizer-->>KillEndpoint: 403 if not EditAll Note over KillEndpoint: Pre-existing: getInternal ViewAll check KillEndpoint->>DB: getInternal (fetches + ViewAll auth) DB-->>KillEndpoint: IngestionPipeline KillEndpoint->>KillEndpoint: decryptOrNullify(pipeline) alt "pipelineServiceClient == null" KillEndpoint-->>Client: 200 Pipeline Client Disabled else KillEndpoint->>PSC: killIngestion(pipeline) PSC-->>KillEndpoint: PipelineServiceClientResponse KillEndpoint-->>Client: response endReviews (3): Last reviewed commit: "test(ingestion-pipeline): trim kill-test..." | Re-trigger Greptile