You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@dolphinscheduler.apache.org by "ruanwenjun (via GitHub)" <gi...@apache.org> on 2026/08/27 07:47:41 UTC
[PR] [Fix-18596][API] Validate sub-workflow references [dolphinscheduler]
ruanwenjun opened a new pull request, #18597:
URL: https://github.com/apache/dolphinscheduler/pull/18597
## Was this PR generated or assisted by AI?
YES. The implementation and regression tests were assisted by AI.
## Purpose of the pull request
Validate sub-workflow references consistently before task definitions are persisted or activated.
close #18596
## Brief change log
- Add a checker for referenced sub-workflows and their target projects.
- Apply the check to workflow creation and updates, workflow-instance updates, version switches, and online or release operations.
- Return the existing generic unavailable-resource result for missing or unavailable references.
- Add unit and service-level regression tests.
## Verify this pull request
This change added tests and can be verified as follows:
- `./mvnw verify -B -pl dolphinscheduler-api -DskipE2E=true -DskipITs -Dmaven.test.skip=false -DskipUT=false -Danalyze.skip=true`
- API module result: 734 tests run, 0 failures, 0 errors, 10 skipped.
## Pull Request Notice
[Pull Request Notice](https://github.com/apache/dolphinscheduler/blob/dev/docs/docs/en/contribute/join/pull-request.md)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Fix-18596][API] Enforce permission checks for sub-workflow references [dolphinscheduler]
Posted by "SbloodyS (via GitHub)" <gi...@apache.org>.
SbloodyS merged PR #18597:
URL: https://github.com/apache/dolphinscheduler/pull/18597
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Fix-18596][API] Enforce permission checks for sub-workflow references [dolphinscheduler]
Posted by "ruanwenjun (via GitHub)" <gi...@apache.org>.
ruanwenjun commented on PR #18597:
URL: https://github.com/apache/dolphinscheduler/pull/18597#issuecomment-5450040116
@SbloodyS Thanks for the comment. After reconsidering the scope, I removed the execution-time completeness check from this PR.
This PR is limited to permission enforcement when a sub-workflow reference is created, changed, or activated. The user must have write permission for the project that owns the referenced sub-workflow. Once the reference has been validly
bound, execution does not recheck the executor's direct permission on the referenced project.
If the referenced sub-workflow later becomes unavailable, it remains a runtime dependency failure, consistent with other missing task dependencies such as data sources, rather than an authorization failure. Therefore, the existing execution behavior is intentionally unchanged.
I have updated the PR/issue title.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Fix-18596][API] Validate sub-workflow references [dolphinscheduler]
Posted by "ruanwenjun (via GitHub)" <gi...@apache.org>.
ruanwenjun commented on PR #18597:
URL: https://github.com/apache/dolphinscheduler/pull/18597#issuecomment-5447951115
> #### Validate missing sub-workflows at execution time as well
> The new checker protects workflow creation, updates, version switches, and online operations, but the execution path still accepts missing sub-workflow definitions.
>
> `ExecutorServiceImpl#checkSubWorkflowDefinitionValid()` collects the referenced workflow codes and calls `workflowDefinitionDao.queryByCodes()`. It then only checks whether any returned definition is offline. If one or all referenced definitions are missing, they are absent from the query result, so the method still returns `true`.
>
> This affects:
>
> * invalid workflows that were already online before this patch;
> * an online workflow whose referenced sub-workflow later becomes unavailable;
> * already-online workflows for which `onlineWorkflowDefinition()` returns early without running the new checker.
Thanks for catching this. I updated `checkSubWorkflowDefinitionValid()` to return false when `queryByCodes()` returns null or does not contain every referenced sub-workflow code, before checking the release state.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Fix-18596][API] Validate sub-workflow references [dolphinscheduler]
Posted by "github-advanced-security[bot] (via GitHub)" <gi...@apache.org>.
github-advanced-security[bot] commented on code in PR #18597:
URL: https://github.com/apache/dolphinscheduler/pull/18597#discussion_r3869909009
##########
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/permission/TaskSubWorkflowPermissionChecker.java:
##########
@@ -0,0 +1,97 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.dolphinscheduler.api.permission;
+
+import org.apache.dolphinscheduler.api.enums.Status;
+import org.apache.dolphinscheduler.api.exceptions.ServiceException;
+import org.apache.dolphinscheduler.api.service.ProjectService;
+import org.apache.dolphinscheduler.common.utils.JSONUtils;
+import org.apache.dolphinscheduler.dao.entity.TaskDefinition;
+import org.apache.dolphinscheduler.dao.entity.User;
+import org.apache.dolphinscheduler.dao.entity.WorkflowDefinition;
+import org.apache.dolphinscheduler.dao.repository.WorkflowDefinitionDao;
+import org.apache.dolphinscheduler.plugin.task.api.parameters.SubWorkflowParameters;
+import org.apache.dolphinscheduler.plugin.task.api.utils.TaskTypeUtils;
+
+import java.util.Collection;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
+import java.util.stream.Collectors;
+
+import lombok.extern.slf4j.Slf4j;
+
+import org.springframework.stereotype.Component;
+
+@Slf4j
+@Component
+public class TaskSubWorkflowPermissionChecker {
+
+ private final WorkflowDefinitionDao workflowDefinitionDao;
+ private final ProjectService projectService;
+
+ public TaskSubWorkflowPermissionChecker(WorkflowDefinitionDao workflowDefinitionDao,
+ ProjectService projectService) {
+ this.workflowDefinitionDao = workflowDefinitionDao;
+ this.projectService = projectService;
+ }
+
+ public void checkPermission(User loginUser, Collection<? extends TaskDefinition> taskDefinitions) {
+ if (taskDefinitions == null || taskDefinitions.isEmpty()) {
+ return;
+ }
+
+ Set<Long> subWorkflowDefinitionCodes = new HashSet<>();
+ for (TaskDefinition taskDefinition : taskDefinitions) {
+ if (!TaskTypeUtils.isSubWorkflowTask(taskDefinition.getTaskType())) {
+ continue;
+ }
+ SubWorkflowParameters subWorkflowParameters =
+ JSONUtils.parseObject(taskDefinition.getTaskParams(), SubWorkflowParameters.class);
+ if (subWorkflowParameters.getWorkflowDefinitionCode() > 0) {
+ subWorkflowDefinitionCodes.add(subWorkflowParameters.getWorkflowDefinitionCode());
+ }
+ }
+
+ if (subWorkflowDefinitionCodes.isEmpty()) {
+ return;
+ }
+
+ List<WorkflowDefinition> subWorkflowDefinitions =
+ workflowDefinitionDao.queryByCodes(subWorkflowDefinitionCodes);
+ Set<Long> existingSubWorkflowDefinitionCodes = subWorkflowDefinitions == null
+ ? new HashSet<>()
+ : subWorkflowDefinitions.stream().map(WorkflowDefinition::getCode).collect(Collectors.toSet());
+ if (!existingSubWorkflowDefinitionCodes.containsAll(subWorkflowDefinitionCodes)) {
+ log.warn("Referenced sub workflow is unavailable, userId:{}.", loginUser.getId());
+ throw new ServiceException(Status.RESOURCE_NOT_EXIST_OR_NO_PERMISSION);
+ }
+
+ Set<Long> subWorkflowProjectCodes = subWorkflowDefinitions.stream()
Review Comment:
## CodeQL / Dereferenced variable may be null
Variable [subWorkflowDefinitions](1) may be null at this access as suggested by [this](2) null guard.
[Show more details](https://github.com/apache/dolphinscheduler/security/code-scanning/5858)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org