Skip to content

fix(rbac): add ActionPlaybookCancel to AllActions#1871

Closed
adityathebe wants to merge 1 commit intomainfrom
fix/policy-playbook-cancel
Closed

fix(rbac): add ActionPlaybookCancel to AllActions#1871
adityathebe wants to merge 1 commit intomainfrom
fix/policy-playbook-cancel

Conversation

@adityathebe
Copy link
Copy Markdown
Member

@adityathebe adityathebe commented Apr 9, 2026

Summary by CodeRabbit

  • New Features
    • Playbook cancellation action is now available in access control policies.

@adityathebe adityathebe enabled auto-merge (squash) April 9, 2026 04:43
@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 9, 2026

Benchstat (Other)

Base: bb96b503bf454c47c82d8eeae46248e873c3e11e
Head: 0cfa0de77708881d938584445b43644c7af8adf7

📊 1 minor regression(s) (all within 5% threshold)

Benchmark Base Head Change p-value
ResourceSelectorConfigs/name-4 180.4µ 184.5µ +2.27% 0.041
Full benchstat output
goos: linux
goarch: amd64
pkg: github.com/flanksource/duty/bench
cpu: AMD EPYC 9V74 80-Core Processor                
                                                       │ bench-base.txt │           bench-head.txt            │
                                                       │     sec/op     │    sec/op      vs base              │
InsertionForRowsWithAliases/external_users.aliases-4      515.7µ ±  20%   518.6µ ±   8%       ~ (p=0.937 n=6)
InsertionForRowsWithAliases/config_items.external_id-4    1.045m ±   8%   1.023m ±  11%       ~ (p=1.000 n=6)
ResourceSelectorConfigs/name-4                            180.4µ ±   3%   184.5µ ±   2%  +2.27% (p=0.041 n=6)
ResourceSelectorConfigs/name_and_type-4                   197.2µ ±   6%   198.1µ ±   4%       ~ (p=0.485 n=6)
ResourceSelectorConfigs/tags-4                            33.26m ± 163%   33.05m ± 179%       ~ (p=1.000 n=6)
ResourceSelectorQueryBuild/name-4                         42.70µ ±   2%   42.73µ ±   1%       ~ (p=1.000 n=6)
ResourceSelectorQueryBuild/name_and_type-4                61.56µ ±   3%   62.33µ ±   1%       ~ (p=0.485 n=6)
ResourceSelectorQueryBuild/tags-4                         16.69µ ±   3%   17.21µ ±   1%       ~ (p=0.065 n=6)
geomean                                                   269.7µ          271.3µ         +0.61%

@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 9, 2026

Benchstat (RLS)

Base: bb96b503bf454c47c82d8eeae46248e873c3e11e
Head: 0cfa0de77708881d938584445b43644c7af8adf7

📊 4 minor regression(s) (all within 5% threshold)

Benchmark Base Head Change p-value
RLS/Sample-15000/analyzer_types/With_RLS-4 3.755m 3.851m +2.55% 0.041
RLS/Sample-15000/catalog_changes/Without_RLS-4 5.274m 5.385m +2.10% 0.002
RLS/Sample-15000/config_changes/Without_RLS-4 5.263m 5.363m +1.90% 0.004
RLS/Sample-15000/config_summary/Without_RLS-4 59.52m 60.54m +1.71% 0.004
✅ 3 improvement(s)
Benchmark Base Head Change p-value
RLS/Sample-15000/analysis_types/With_RLS-4 4.333m 3.917m -9.61% 0.002
RLS/Sample-15000/change_types/With_RLS-4 5.432m 5.243m -3.48% 0.004
RLS/Sample-15000/config_detail/With_RLS-4 126.0m 124.8m -0.98% 0.002
Full benchstat output
goos: linux
goarch: amd64
pkg: github.com/flanksource/duty/bench
cpu: AMD EPYC 7763 64-Core Processor                
                                               │ bench-base.txt │          bench-head.txt           │
                                               │     sec/op     │   sec/op     vs base              │
RLS/Sample-15000/catalog_changes/Without_RLS-4      5.274m ± 1%   5.385m ± 1%  +2.10% (p=0.002 n=6)
RLS/Sample-15000/catalog_changes/With_RLS-4         128.8m ± 0%   128.8m ± 1%       ~ (p=1.000 n=6)
RLS/Sample-15000/config_changes/Without_RLS-4       5.263m ± 1%   5.363m ± 1%  +1.90% (p=0.004 n=6)
RLS/Sample-15000/config_changes/With_RLS-4          128.5m ± 3%   129.3m ± 1%       ~ (p=0.180 n=6)
RLS/Sample-15000/config_detail/Without_RLS-4        3.956m ± 2%   3.982m ± 1%       ~ (p=0.589 n=6)
RLS/Sample-15000/config_detail/With_RLS-4           126.0m ± 1%   124.8m ± 0%  -0.98% (p=0.002 n=6)
RLS/Sample-15000/config_names/Without_RLS-4         12.72m ± 2%   12.67m ± 0%       ~ (p=0.394 n=6)
RLS/Sample-15000/config_names/With_RLS-4            126.1m ± 4%   126.2m ± 1%       ~ (p=0.818 n=6)
RLS/Sample-15000/config_summary/Without_RLS-4       59.52m ± 1%   60.54m ± 2%  +1.71% (p=0.004 n=6)
RLS/Sample-15000/config_summary/With_RLS-4          745.9m ± 1%   748.0m ± 1%       ~ (p=0.589 n=6)
RLS/Sample-15000/configs/Without_RLS-4              7.088m ± 2%   7.088m ± 0%       ~ (p=1.000 n=6)
RLS/Sample-15000/configs/With_RLS-4                 124.9m ± 2%   126.3m ± 1%       ~ (p=0.065 n=6)
RLS/Sample-15000/analysis_types/Without_RLS-4       3.916m ± 1%   3.894m ± 3%       ~ (p=0.394 n=6)
RLS/Sample-15000/analysis_types/With_RLS-4          4.333m ± 3%   3.917m ± 4%  -9.61% (p=0.002 n=6)
RLS/Sample-15000/analyzer_types/Without_RLS-4       3.739m ± 3%   3.732m ± 0%       ~ (p=0.818 n=6)
RLS/Sample-15000/analyzer_types/With_RLS-4          3.755m ± 5%   3.851m ± 3%  +2.55% (p=0.041 n=6)
RLS/Sample-15000/change_types/Without_RLS-4         5.278m ± 4%   5.299m ± 2%       ~ (p=0.818 n=6)
RLS/Sample-15000/change_types/With_RLS-4            5.432m ± 3%   5.243m ± 3%  -3.48% (p=0.004 n=6)
RLS/Sample-15000/config_classes/Without_RLS-4       3.289m ± 1%   3.276m ± 1%       ~ (p=0.240 n=6)
RLS/Sample-15000/config_classes/With_RLS-4          124.5m ± 1%   124.4m ± 1%       ~ (p=0.485 n=6)
RLS/Sample-15000/config_types/Without_RLS-4         3.932m ± 0%   3.942m ± 1%       ~ (p=0.589 n=6)
RLS/Sample-15000/config_types/With_RLS-4            124.5m ± 2%   124.8m ± 0%       ~ (p=0.485 n=6)
geomean                                             19.33m        19.29m       -0.20%

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Apr 9, 2026

Walkthrough

The AllActions action-list variable in the RBAC policy module was updated to include the ActionPlaybookCancel action and reordered ActionDelete to appear after ActionUpdate.

Changes

Cohort / File(s) Summary
RBAC Policy Variable Update
rbac/policy/policy.go
Added ActionPlaybookCancel to the exported AllActions list and reordered ActionDelete to appear after ActionUpdate.
🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: adding ActionPlaybookCancel to the AllActions variable in the RBAC policy module.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/policy-playbook-cancel
✨ Simplify code
  • Create PR with simplified code
  • Commit simplified code in branch fix/policy-playbook-cancel

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@rbac/policy/policy.go`:
- Around line 272-279: AllActions includes ActionPlaybookCancel but
ABACObjectSelector still only treats run/approve as playbook wildcard actions,
causing RBAC/ABAC mismatch; update the ABACObjectSelector implementation (the
code that checks playbook wildcard actions and the logic that maps playbook
actions to wildcard selectors) to include ActionPlaybookCancel alongside
ActionPlaybookRun and ActionPlaybookApprove so cancel permissions are recognized
by ABAC matching the RBAC list (locate the ABACObjectSelector function and the
branch handling playbook action wildcards and add ActionPlaybookCancel to that
set).
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: bcee0c01-d6c4-436c-9178-36157e4425e1

📥 Commits

Reviewing files that changed from the base of the PR and between bb96b50 and 0cfa0de.

📒 Files selected for processing (1)
  • rbac/policy/policy.go

Comment thread rbac/policy/policy.go
Comment on lines 272 to 279
var AllActions = []string{
ActionCreate,
ActionDelete,
ActionRead,
ActionUpdate,
ActionDelete,
ActionPlaybookApprove,
ActionPlaybookCancel,
ActionPlaybookRun,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Add ABAC companion support for ActionPlaybookCancel to avoid auth mismatch.

Line 278 adds ActionPlaybookCancel to AllActions, but ABACObjectSelector still only treats run/approve as playbook wildcard actions (Line 321). This can cause cancel permissions to be recognized in RBAC lists but fail ABAC-based permission matching.

Proposed fix
diff --git a/rbac/policy/policy.go b/rbac/policy/policy.go
@@
-		if lo.Contains([]string{ActionPlaybookRun, ActionPlaybookApprove}, action) {
+		if lo.Contains([]string{ActionPlaybookRun, ActionPlaybookApprove, ActionPlaybookCancel}, action) {
 			return []byte(`{"playbooks": [{"name":"*"}]}`)
 		}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@rbac/policy/policy.go` around lines 272 - 279, AllActions includes
ActionPlaybookCancel but ABACObjectSelector still only treats run/approve as
playbook wildcard actions, causing RBAC/ABAC mismatch; update the
ABACObjectSelector implementation (the code that checks playbook wildcard
actions and the logic that maps playbook actions to wildcard selectors) to
include ActionPlaybookCancel alongside ActionPlaybookRun and
ActionPlaybookApprove so cancel permissions are recognized by ABAC matching the
RBAC list (locate the ABACObjectSelector function and the branch handling
playbook action wildcards and add ActionPlaybookCancel to that set).

@adityathebe adityathebe closed this Apr 9, 2026
auto-merge was automatically disabled April 9, 2026 05:34

Pull request was closed

@adityathebe adityathebe deleted the fix/policy-playbook-cancel branch April 9, 2026 05:34
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.

1 participant