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/18 14:09:51 UTC
[PR] [Fix-18565][API] Validate datasource access for task definitions [dolphinscheduler]
ruanwenjun opened a new pull request, #18566:
URL: https://github.com/apache/dolphinscheduler/pull/18566
## Was this PR generated or assisted by AI?
YES. The implementation and tests were assisted by AI and reviewed locally.
## Purpose of the pull request
Validate datasource access consistently before task definitions are saved or activated.
Closes #18565.
## Brief change log
- Add a shared checker for datasource resources declared by task plugins.
- Apply the check to task and workflow save, version switch, online, and instance update paths.
- Add regression tests for the checker and affected API paths.
## Verify this pull request
This change added tests and can be verified as follows:
- Run `./mvnw -pl dolphinscheduler-api -DskipE2E=true -DskipITs -Dspotless.check.skip=true -Dcheckstyle.skip=true -Dtest=TaskDatasourcePermissionCheckerTest,TaskDefinitionServiceImplTest,WorkflowDefinitionServiceTest,WorkflowInstanceServiceTest test`.
- 68 tests pass.
- `./mvnw -pl dolphinscheduler-api -DskipTests spotless:check` passes.
--
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-18565][API] Validate datasource access for task definitions [dolphinscheduler]
Posted by "det101 (via GitHub)" <gi...@apache.org>.
det101 commented on code in PR #18566:
URL: https://github.com/apache/dolphinscheduler/pull/18566#discussion_r3809890646
##########
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/TaskDefinitionServiceImpl.java:
##########
@@ -362,6 +367,7 @@ public Long updateTaskWithUpstream(User loginUser, long projectCode, long taskCo
notExistTaskCodes);
throw new ServiceException(Status.TASK_DEFINE_NOT_EXIST, notExistTaskCodes);
}
+ taskDatasourcePermissionChecker.checkPermission(loginUser, upstreamTaskDefinitionList);
Review Comment:
This check looks too broad for `updateTaskWithUpstream`.
This list is the **upstream/pre-task** definitions used only to rewrite DAG edges (`preTaskCode` / version). Changing a dependency does not mean the current user will access those upstream tasks' datasources.
The task being saved is already validated in `updateTask(...)`. Checking `upstreamTaskDefinitionList` here can false-reject a legitimate edge update when the editor has project write permission but not the upstream SQL/RemoteShell datasource.
Suggest dropping this line:
```java
taskDatasourcePermissionChecker.checkPermission(loginUser, upstreamTaskDefinitionList);
```
--
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-18565][API] Validate datasource access for task definitions [dolphinscheduler]
Posted by "SbloodyS (via GitHub)" <gi...@apache.org>.
SbloodyS merged PR #18566:
URL: https://github.com/apache/dolphinscheduler/pull/18566
--
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-18565][API] Validate datasource access for task definitions [dolphinscheduler]
Posted by "det101 (via GitHub)" <gi...@apache.org>.
det101 commented on code in PR #18566:
URL: https://github.com/apache/dolphinscheduler/pull/18566#discussion_r3809890646
##########
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/TaskDefinitionServiceImpl.java:
##########
@@ -362,6 +367,7 @@ public Long updateTaskWithUpstream(User loginUser, long projectCode, long taskCo
notExistTaskCodes);
throw new ServiceException(Status.TASK_DEFINE_NOT_EXIST, notExistTaskCodes);
}
+ taskDatasourcePermissionChecker.checkPermission(loginUser, upstreamTaskDefinitionList);
Review Comment:
Thanks for adding the shared datasource check — this looks like a good fix overall.
One small question on this line: `upstreamTaskDefinitionList` here is used to rewrite DAG edges (`preTaskCode` / version), not to connect those upstream datasources. The current task is already validated in `updateTask(...)`.
If I understand correctly, this might false-reject a user who can edit the workflow but does not own the upstream SQL/RemoteShell datasource. Would it make sense to drop this check, or is there a case I am missing?
--
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-18565][API] Validate datasource access for task definitions [dolphinscheduler]
Posted by "ruanwenjun (via GitHub)" <gi...@apache.org>.
ruanwenjun commented on code in PR #18566:
URL: https://github.com/apache/dolphinscheduler/pull/18566#discussion_r3809998138
##########
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/TaskDefinitionServiceImpl.java:
##########
@@ -362,6 +367,7 @@ public Long updateTaskWithUpstream(User loginUser, long projectCode, long taskCo
notExistTaskCodes);
throw new ServiceException(Status.TASK_DEFINE_NOT_EXIST, notExistTaskCodes);
}
+ taskDatasourcePermissionChecker.checkPermission(loginUser, upstreamTaskDefinitionList);
Review Comment:
Good catch. `upstreamCodes` contains the complete relationship set, so unchanged upstream tasks should not be revalidated here. I removed this check and kept validation for the submitted task in `updateTask(...)`. The obsolete endpoint will be removed separately in #18568.
--
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