This is an automated email from the ASF dual-hosted git repository. github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-7605-976499472ac4d406894206931bc53f8a7886a835 in repository https://gitbox.apache.org/repos/asf/texera.git
commit 289189741dad32c7d613108ecd9fe75a8eca420d Author: Xinyuan Lin <[email protected]> AuthorDate: Thu Aug 13 05:01:17 2026 +0000 fix(amber): guard cloneWorkflow with a read-access check (#7605) ### What changes were proposed in this PR? `cloneWorkflow` fetched the source workflow by `wid` and copied its content into a workflow owned by the caller, with no access check on the way in. Any authenticated REGULAR user could `POST /workflow/clone/<wid>` for a wid they hold no privilege on and receive a full copy of a private workflow's content — operator configurations, file paths and all. **Root cause.** Every sibling on this path guards; this one endpoint did not. | Endpoint | Guard | |---|---| | `retrieveWorkflow` | `hasReadAccess` directly | | `duplicateWorkflow` | `hasReadAccess` directly | | `cloneVersion` (`/version/clone/{vid}`) | inherits it via `retrieveWorkflowVersion` | | **`cloneWorkflow`** | **none** | That reads as an oversight rather than a decision. SECURITY.md states that REGULAR users "cannot access other users' private resources without granted permissions", so the endpoint contradicted the project's own declared model. **Before → after** ``` caller with no privilege on wid | v v POST /workflow/clone/{wid} POST /workflow/clone/{wid} | | | (no check) +-- hasReadAccess(wid, uid)? --> no --> 403 v | fetchOneByWid(wid) v yes (owner / READ grant / public) | fetchOneByWid(wid) v | full content copied to caller v full content copied to caller ``` The fix adds the same three lines the siblings use. `hasReadAccess` already returns true for public workflows, so the hub's clone button — the only caller, and always acting on a published workflow — is unaffected. A caller holding an explicit READ grant can still clone. ### Any related issues, documentation, discussions? Found while reviewing the clone-endpoint test in #7592, now merged; the new cases here build on the spec helpers that landed with it. Not filed as an issue, because SECURITY.md asks that security bugs not be reported through public issues. The `release/v1.2` backport preflight comes back grey: the guard itself applies, but the two new test cases depend on helpers that arrived with #7592, which was not backported. The backport needs those cases rewritten self-contained, so it will have to be resolved by hand rather than pushed straight through. ### How was this PR tested? Two cases added to the existing `WorkflowResourceSpec`, and the pre-existing success case in #7592 now publishes its source first so it exercises the public path. | Test | Pins | |---|---| | `clone a private workflow the caller has been granted read access to` | the guard does not over-block a legitimate READ grant | | `reject a caller with no access to the source workflow` | 403, no copy reaches the caller, and no `WORKFLOW_USER_CLONES` row is written for the rejected attempt | Red before the guard, green after — with the guard reverted the rejection case fails and the other 69 pass, so it is the guard the test is pinning and not a fixture. ``` sbt "WorkflowExecutionService/testOnly org.apache.texera.web.resource.dashboard.file.WorkflowResourceSpec" ``` ``` [info] Total number of tests run: 70 [info] Tests: succeeded 70, failed 0, canceled 0, ignored 0, pending 0 ``` `scalafmtCheck` and `scalafix --check` pass for both `Compile` and `Test`. ### Was this PR authored or co-authored using generative AI tooling? Generated-by: Claude Code (Opus 5) --- .../dashboard/user/workflow/WorkflowResource.scala | 3 +++ .../dashboard/file/WorkflowResourceSpec.scala | 27 ++++++++++++++++++---- 2 files changed, 26 insertions(+), 4 deletions(-) diff --git a/amber/src/main/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowResource.scala b/amber/src/main/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowResource.scala index eaf4460b29..2d438e8dc7 100644 --- a/amber/src/main/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowResource.scala +++ b/amber/src/main/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowResource.scala @@ -549,6 +549,9 @@ class WorkflowResource extends LazyLogging { @Auth sessionUser: SessionUser, @Context request: HttpServletRequest ): Integer = { + if (!WorkflowAccessResource.hasReadAccess(wid, sessionUser.getUid)) { + throw new ForbiddenException("No sufficient access privilege.") + } val oldWorkflow: Workflow = workflowDao.fetchOneByWid(wid) val newWorkflow: DashboardWorkflow = createWorkflow( new Workflow( diff --git a/amber/src/test/scala/org/apache/texera/web/resource/dashboard/file/WorkflowResourceSpec.scala b/amber/src/test/scala/org/apache/texera/web/resource/dashboard/file/WorkflowResourceSpec.scala index 089b5234cd..266992afa6 100644 --- a/amber/src/test/scala/org/apache/texera/web/resource/dashboard/file/WorkflowResourceSpec.scala +++ b/amber/src/test/scala/org/apache/texera/web/resource/dashboard/file/WorkflowResourceSpec.scala @@ -1197,10 +1197,7 @@ class WorkflowResourceSpec "WorkflowResource.cloneWorkflow" should "copy the workflow to the caller and record the clone" in { // The source is made public because that is the flow this endpoint serves: the hub's clone - // button, on someone else's published workflow. Cloning a *private* workflow the caller has - // no access to also succeeds today -- cloneWorkflow fetches by wid with no `hasReadAccess` - // guard, unlike retrieveWorkflow and duplicateWorkflow -- but that is a gap to fix in the - // resource, not a contract to pin here, so this test does not assert it either way. + // button, on someone else's published workflow. val wid = seedWorkflow(sessionUser1, "clone-src", "d", contentWithOperator).workflow.getWid workflowResource.makePublic(wid, sessionUser1) @@ -1223,6 +1220,28 @@ class WorkflowResourceSpec ) } + it should "clone a private workflow the caller has been granted read access to" in { + val wid = seedWorkflow(sessionUser1, "clone-shared", "d", contentWithOperator).workflow.getWid + grantAccess(wid, testUser2, PrivilegeEnum.READ) + + val newWid = workflowResource.cloneWorkflow(wid, sessionUser2, cloneRequest) + + assert(workflowResource.retrieveWorkflow(newWid, sessionUser2).name == "clone-shared_clone") + } + + it should "reject a caller with no access to the source workflow" in { + val wid = + seedWorkflow(sessionUser1, "clone-forbidden", "d", contentWithOperator).workflow.getWid + + assertThrows[ForbiddenException]( + workflowResource.cloneWorkflow(wid, sessionUser2, cloneRequest) + ) + + // no copy reached the caller, and the rejected attempt was not recorded as a clone + assert(workflowNamesOf(sessionUser2).isEmpty) + assert(getDSLContext.fetchCount(WORKFLOW_USER_CLONES, WORKFLOW_USER_CLONES.WID.eq(wid)) == 0) + } + "WorkflowResource.duplicateWorkflow" should "add the copy, not the original, to the project" in { val wid = seedWorkflow(sessionUser1, "dup-into-project", "d", contentWithOperator).workflow.getWid
