Skip to content

fix(Security): restrict draft action execution to editors#41614

Merged
subrata71 merged 2 commits intoreleasefrom
fix/app-15010-restrict-draft-execution-to-editors
Mar 13, 2026
Merged

fix(Security): restrict draft action execution to editors#41614
subrata71 merged 2 commits intoreleasefrom
fix/app-15010-restrict-draft-execution-to-editors

Conversation

@subrata71
Copy link
Collaborator

@subrata71 subrata71 commented Mar 12, 2026

Description

TL;DR: Authenticated viewers can bypass the editor/publish boundary by sending viewMode=false in the action execute request, causing the server to run unpublished draft actions. This fix requires MANAGE_ACTIONS (edit) permission for draft execution instead of EXECUTE_ACTIONS.

Root Cause

The action execution path (getValidActionForExecution, populateAndExecuteAction) only checks EXECUTE_ACTIONS permission regardless of viewMode. Since viewers inherit EXECUTE_ACTIONS via READ_APPLICATIONS → READ_PAGES → EXECUTE_ACTIONS, any authenticated viewer can set viewMode=false and execute unpublished draft actions. Only anonymous users were blocked.

Fix

Introduced getActionExecutionPermission() helper that returns:

  • MANAGE_ACTIONS (edit permission) when viewMode=false (draft execution)
  • EXECUTE_ACTIONS (execute permission) when viewMode=true (published execution)

Editors retain MANAGE_ACTIONS via the MANAGE_PAGES → MANAGE_ACTIONS hierarchy and continue working normally. Viewers only have EXECUTE_ACTIONS and are now blocked from draft execution.

Regression Tests

  • testViewerCannotExecuteUnpublishedAction — viewer with viewMode=false is rejected
  • testViewerCanExecutePublishedAction — viewer with viewMode=true succeeds
  • testEditorCanExecuteUnpublishedAction — editor with viewMode=false succeeds

Fixes https://linear.app/appsmith/issue/APP-15010/vulnerability-authenticated-viewers-can-execute-unpublished-draft

Automation

/ok-to-test tags="@tag.All"

🔍 Cypress test results

Tip

🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/23017716243
Commit: ed2b421
Cypress dashboard.
Tags: @tag.All
Spec:


Thu, 12 Mar 2026 19:57:26 UTC

Communication

Should the DevRel and Marketing teams inform users about this change?

  • Yes
  • No

Summary by CodeRabbit

  • Bug Fixes

    • Centralized permission decision for action execution so viewers can run published actions but cannot run unpublished ones; aligns draft (editor) vs published (viewer) behavior.
  • Tests

    • Added role- and publication-state tests covering viewer/editor execution paths, invitation/role setup, and ACL enforcement for published and unpublished actions.

@subrata71 subrata71 added the ok-to-test Required label for CI label Mar 12, 2026
@linear
Copy link

linear bot commented Mar 12, 2026

@cursor
Copy link

cursor bot commented Mar 12, 2026

You have run out of free Bugbot PR reviews for this billing cycle. This will reset on April 9.

To receive reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.

@subrata71 subrata71 self-assigned this Mar 12, 2026
@subrata71 subrata71 requested a review from sondermanish March 12, 2026 12:03
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 12, 2026

Walkthrough

Centralizes action execution permission selection into a new helper that chooses edit vs execute permission based on normalized viewMode and draft/published state, updates execution flows to use it, and adds tests covering viewer/editor execution for published and unpublished actions.

Changes

Cohort / File(s) Summary
Permission Enforcement Logic
app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/ActionExecutionSolutionCEImpl.java
Added protected AclPermission getActionExecutionPermission(ExecuteActionDTO, ExecuteActionMetaDTO); replaced direct permission retrievals in populateAndExecuteAction() and getValidActionForExecution() to centralize decision-making after viewMode/anonymous normalization and to align draft (edit) vs published (execute) flows.
Role-Based Execution Tests
app/server/appsmith-server/src/test/java/com/appsmith/server/solutions/ce/ActionExecutionSolutionCETest.java
Introduced tests for viewer/editor execution scenarios (published vs unpublished), added imports (PermissionGroup, InviteUsersDTO, UserAndAccessManagementService), injected UserAndAccessManagementService, and wired authentication/security context in tests to assert ACL behavior across roles.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

A helper decides the proper key,
View or edit, draft or deploy,
Tests arrive in orderly queue,
Viewer, editor — each one tried,
Permissions harmonized, flow renewed.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 22.22% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately captures the main security fix: restricting draft action execution to editors only, which aligns with the core change introduced in the PR.
Description check ✅ Passed The PR description comprehensively addresses the security vulnerability, root cause, fix strategy, and regression tests with all required template sections completed.

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

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/app-15010-restrict-draft-execution-to-editors
📝 Coding Plan
  • Generate coding plan for human review comments

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
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
`@app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/ActionExecutionSolutionCEImpl.java`:
- Around line 194-200: Update getActionExecutionPermission to treat a null
ExecuteActionDTO.getViewMode() the same as FALSE by checking
Boolean.TRUE.equals(executeActionDTO.getViewMode()) and returning
executePermission only in that case; otherwise return editPermission (use
actionPermission.getEditPermission()/getExecutePermission()). Also ensure
getValidActionForExecution computes permissions after anonymous-user
normalization (so normalize the user/anonymous state first, then call
getActionExecutionPermission) to avoid a missing viewMode bypass.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 47b453d1-fe11-4ba0-9d1d-d18ba18ab464

📥 Commits

Reviewing files that changed from the base of the PR and between 18c7cd3 and a2d3635.

📒 Files selected for processing (2)
  • app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/ActionExecutionSolutionCEImpl.java
  • app/server/appsmith-server/src/test/java/com/appsmith/server/solutions/ce/ActionExecutionSolutionCETest.java

Authenticated viewers could execute unpublished/draft actions by sending
viewMode=false in the action execute request. The server only blocked
anonymous users from draft execution; any authenticated user with
EXECUTE_ACTIONS permission could bypass the editor/publish boundary.

When viewMode=false (draft execution), require MANAGE_ACTIONS (edit)
permission instead of EXECUTE_ACTIONS. Editors retain MANAGE_ACTIONS via
the MANAGE_PAGES hierarchy; viewers only have EXECUTE_ACTIONS via
READ_PAGES and are now blocked from draft execution.

Adds regression tests for viewer blocked from draft, viewer allowed
published, and editor allowed draft execution scenarios.

Fixes APP-15010
@subrata71 subrata71 force-pushed the fix/app-15010-restrict-draft-execution-to-editors branch from a2d3635 to 3d3c58b Compare March 12, 2026 13:43
Copy link
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: 2

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

Inline comments:
In
`@app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/ActionExecutionSolutionCEImpl.java`:
- Around line 212-214: The execute path can NPE when
executeActionDTO.getViewMode() is null; normalize viewMode before downstream
calls by setting a local Boolean viewMode =
Boolean.TRUE.equals(executeActionDTO.getViewMode()) ? Boolean.TRUE :
Boolean.FALSE (or otherwise treat null as FALSE) and pass that normalized value
into getActionExecutionPermission and getTrueEnvironmentId (or alternatively
change getTrueEnvironmentId to use Boolean.TRUE.equals(viewMode)); update the
call sites around getActionExecutionPermission, getTrueEnvironmentId, and
newActionService.findById to consume the normalized viewMode so nulls never
reach branching logic.

In
`@app/server/appsmith-server/src/test/java/com/appsmith/server/solutions/ce/ActionExecutionSolutionCETest.java`:
- Around line 2073-2252: Add a regression test that calls the real execution
entrypoint instead of only getValidActionForExecution: invoke
ActionExecutionSolutionCEImpl.populateAndExecuteAction (via
actionExecutionSolution.populateAndExecuteAction) with a prepared
ExecuteActionDTO and ExecuteActionMetaDTO and the appropriate SecurityContext
for viewMode=false and/or viewMode=true, then verify the end-to-end execution
behavior (response/error) using StepVerifier; this ensures the multipart/public
execution path that routes through populateAndExecuteAction is exercised rather
than stopping at getValidActionForExecution.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: dd5d49ca-6c2a-4ff6-9e66-bd44c59a020e

📥 Commits

Reviewing files that changed from the base of the PR and between a2d3635 and 3d3c58b.

📒 Files selected for processing (2)
  • app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/ActionExecutionSolutionCEImpl.java
  • app/server/appsmith-server/src/test/java/com/appsmith/server/solutions/ce/ActionExecutionSolutionCETest.java

Replace raw Boolean auto-unboxing with TRUE.equals() in
getTrueEnvironmentId to prevent NPE when viewMode is null.
This is consistent with the null-safe pattern used throughout
the rest of the execution path.

Addresses CodeRabbit review feedback on PR #41614.
Copy link
Contributor

@sondermanish sondermanish left a comment

Choose a reason for hiding this comment

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

Looks good

@subrata71 subrata71 merged commit 3d46f68 into release Mar 13, 2026
84 checks passed
@subrata71 subrata71 deleted the fix/app-15010-restrict-draft-execution-to-editors branch March 13, 2026 08:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ok-to-test Required label for CI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants