You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@dolphinscheduler.apache.org by "BruceWong96 (via GitHub)" <gi...@apache.org> on 2024/12/25 07:37:35 UTC
[PR] [Fix-16918][Task] Wrong permissions configuration while executing shell [dolphinscheduler]
BruceWong96 opened a new pull request, #16923:
URL: https://github.com/apache/dolphinscheduler/pull/16923
<!--Thanks very much for contributing to Apache DolphinScheduler, we are happy that you want to help us improve DolphinScheduler! -->
## Purpose of the pull request
#16918
## Brief change log
<!--*(for example:)*
- *Add maven-checkstyle-plugin to root pom.xml*
-->
## Verify this pull request
<!--*(Please pick either of the following options)*-->
This pull request is code cleanup without any test coverage.
*(or)*
This pull request is already covered by existing tests, such as *(please describe tests)*.
(or)
This change added tests and can be verified as follows:
<!--*(example:)*
- *Added dolphinscheduler-dao tests for end-to-end.*
- *Added CronUtilsTest to verify the change.*
- *Manually verified the change by testing locally.* -->
(or)
## Pull Request Notice
[Pull Request Notice](https://github.com/apache/dolphinscheduler/blob/dev/docs/docs/en/contribute/join/pull-request.md)
If your pull request contain incompatible change, you should also add it to `docs/docs/en/guide/upgrede/incompatible.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-16918][Task] Fix wrong permissions configuration while executing shell [dolphinscheduler]
Posted by "github-advanced-security[bot] (via GitHub)" <gi...@apache.org>.
github-advanced-security[bot] commented on code in PR #16923:
URL: https://github.com/apache/dolphinscheduler/pull/16923#discussion_r1905130957
##########
dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/utils/FileUtils.java:
##########
@@ -289,6 +268,29 @@
}
}
+ public static void createDirectoryWithPermission(@NonNull Path path,
+ @NonNull Set<PosixFilePermission> permissions) throws IOException {
+ if (path.toFile().exists()) {
Review Comment:
## Uncontrolled data used in path expression
This path depends on a [user-provided value](1).
[Show more details](https://github.com/apache/dolphinscheduler/security/code-scanning/5132)
##########
dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/utils/FileUtils.java:
##########
@@ -289,6 +268,29 @@
}
}
+ public static void createDirectoryWithPermission(@NonNull Path path,
+ @NonNull Set<PosixFilePermission> permissions) throws IOException {
+ if (path.toFile().exists()) {
+ return;
+ }
+
+ if (OSUtils.isWindows()) {
+ Files.createDirectories(path);
+ } else {
+ Path parent = path.getParent();
+ if (parent != null && !parent.toFile().exists()) {
Review Comment:
## Uncontrolled data used in path expression
This path depends on a [user-provided value](1).
[Show more details](https://github.com/apache/dolphinscheduler/security/code-scanning/5133)
##########
dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/utils/FileUtils.java:
##########
@@ -289,6 +268,29 @@
}
}
+ public static void createDirectoryWithPermission(@NonNull Path path,
+ @NonNull Set<PosixFilePermission> permissions) throws IOException {
+ if (path.toFile().exists()) {
+ return;
+ }
+
+ if (OSUtils.isWindows()) {
+ Files.createDirectories(path);
+ } else {
+ Path parent = path.getParent();
+ if (parent != null && !parent.toFile().exists()) {
+ createDirectoryWithPermission(parent, permissions);
+ }
+
+ try {
+ Files.createDirectory(path);
Review Comment:
## Uncontrolled data used in path expression
This path depends on a [user-provided value](1).
[Show more details](https://github.com/apache/dolphinscheduler/security/code-scanning/5134)
##########
dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/utils/FileUtils.java:
##########
@@ -289,6 +268,29 @@
}
}
+ public static void createDirectoryWithPermission(@NonNull Path path,
+ @NonNull Set<PosixFilePermission> permissions) throws IOException {
+ if (path.toFile().exists()) {
+ return;
+ }
+
+ if (OSUtils.isWindows()) {
+ Files.createDirectories(path);
+ } else {
+ Path parent = path.getParent();
+ if (parent != null && !parent.toFile().exists()) {
+ createDirectoryWithPermission(parent, permissions);
+ }
+
+ try {
+ Files.createDirectory(path);
+ Files.setPosixFilePermissions(path, permissions);
+ } catch (FileAlreadyExistsException fileAlreadyExistsException) {
+ log.error("The directory: {} already exists", path);
Review Comment:
## Log Injection
This log entry depends on a [user-provided value](1).
[Show more details](https://github.com/apache/dolphinscheduler/security/code-scanning/5145)
--
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-16918][Task] Fix wrong permissions configuration while executing shell [dolphinscheduler]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #16923:
URL: https://github.com/apache/dolphinscheduler/pull/16923#issuecomment-2562120067
## [](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=16923) **Quality Gate failed**
Failed conditions
 [7.7% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=16923&metric=new_coverage&view=list) (required ≥ 60%)
 [48.1% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=16923&metric=new_duplicated_lines_density&view=list) (required ≤ 10%)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=16923)
--
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-16918][Task] Fix wrong permissions configuration while executing shell [dolphinscheduler]
Posted by "SbloodyS (via GitHub)" <gi...@apache.org>.
SbloodyS commented on code in PR #16923:
URL: https://github.com/apache/dolphinscheduler/pull/16923#discussion_r1897584880
##########
dolphinscheduler-worker/src/main/java/org/apache/dolphinscheduler/server/worker/utils/TaskExecutionContextUtils.java:
##########
@@ -51,6 +52,9 @@ public static void createTaskInstanceWorkingDirectory(TaskExecutionContext taskE
taskInstanceWorkingDirectory);
}
FileUtils.createDirectoryWith755(Paths.get(taskInstanceWorkingDirectory));
+ final Path taskInstanceWorkingDirectoryPath = Paths.get(taskInstanceWorkingDirectory);
Review Comment:
https://github.com/apache/dolphinscheduler/blob/dev/docs/docs/zh/guide/security/security.md
https://github.com/apache/dolphinscheduler/blob/dev/docs/docs/en/guide/security/security.md
The create tenant part.
--
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-16918][Task] Fix wrong permissions configuration while executing shell [dolphinscheduler]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #16923:
URL: https://github.com/apache/dolphinscheduler/pull/16923#issuecomment-2574800831
## [](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=16923) **Quality Gate passed**
Issues
 [2 New issues](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=16923&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=16923&issueStatuses=ACCEPTED)
Measures
 [1 Security Hotspot](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=16923&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [64.9% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=16923&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=16923&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=16923)
--
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-16918][Task] Fix wrong permissions configuration while executing shell [dolphinscheduler]
Posted by "SbloodyS (via GitHub)" <gi...@apache.org>.
SbloodyS commented on code in PR #16923:
URL: https://github.com/apache/dolphinscheduler/pull/16923#discussion_r1897751643
##########
docs/docs/zh/guide/security/security.md:
##########
@@ -18,7 +18,9 @@
- 租户编码：**租户编码是 Linux上 的用户，唯一，不能重复**
- 管理员进入安全中心->租户管理页面，点击“创建租户”按钮，创建租户。
-> 注意：目前仅有 admin 用户可以修改租户。
+> 注意：
+> 1. 目前仅有 admin 用户可以修改租户；
+> 2. 如果您在 Linux 中手动创建一个租户，则需要将手动创建的租户添加到 dolphinscheduler bootstrap 用户组，以便该租户拥有足够的工作目录权限。
Review Comment:
```suggestion
> 2. 如果您在 Linux 中手动创建一个租户，则需要将手动创建的租户添加到 dolphinscheduler 启动用户组，以便该租户拥有足够的工作目录权限。
```
--
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-16918][Task] Fix wrong permissions configuration while executing shell [dolphinscheduler]
Posted by "github-advanced-security[bot] (via GitHub)" <gi...@apache.org>.
github-advanced-security[bot] commented on code in PR #16923:
URL: https://github.com/apache/dolphinscheduler/pull/16923#discussion_r1897768995
##########
dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/utils/FileUtils.java:
##########
@@ -266,7 +268,7 @@
Files.setPosixFilePermissions(path, PERMISSION_755);
} catch (FileAlreadyExistsException fileAlreadyExistsException) {
// Catch the FileAlreadyExistsException here to avoid create the same parent directory in parallel
- log.debug("The directory: {} already exists", path);
+ log.error("The directory: {} already exists", path);
Review Comment:
## Log Injection
This log entry depends on a [user-provided value](1).
[Show more details](https://github.com/apache/dolphinscheduler/security/code-scanning/5123)
--
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-16918][Task] Fix wrong permissions configuration while executing shell [dolphinscheduler]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #16923:
URL: https://github.com/apache/dolphinscheduler/pull/16923#issuecomment-2562357185
## [](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=16923) **Quality Gate failed**
Failed conditions
 [7.4% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=16923&metric=new_coverage&view=list) (required ≥ 60%)
 [46.4% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=16923&metric=new_duplicated_lines_density&view=list) (required ≤ 10%)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=16923)
--
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-16918][Task] Fix wrong permissions configuration while executing shell [dolphinscheduler]
Posted by "SbloodyS (via GitHub)" <gi...@apache.org>.
SbloodyS commented on code in PR #16923:
URL: https://github.com/apache/dolphinscheduler/pull/16923#discussion_r1899428714
##########
dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/utils/FileUtils.java:
##########
@@ -289,6 +291,29 @@ public static void setFileTo755(File file) throws IOException {
}
}
+ public static void createDirectoryWith775(@NonNull Path path) throws IOException {
+ if (path.toFile().exists()) {
+ return;
+ }
+ if (OSUtils.isWindows()) {
+ Files.createDirectories(path);
+ } else {
Review Comment:
Okay.
--
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-16918][Task] Fix wrong permissions configuration while executing shell [dolphinscheduler]
Posted by "BruceWong96 (via GitHub)" <gi...@apache.org>.
BruceWong96 commented on code in PR #16923:
URL: https://github.com/apache/dolphinscheduler/pull/16923#discussion_r1897686159
##########
dolphinscheduler-worker/src/main/java/org/apache/dolphinscheduler/server/worker/utils/TaskExecutionContextUtils.java:
##########
@@ -51,6 +52,9 @@ public static void createTaskInstanceWorkingDirectory(TaskExecutionContext taskE
taskInstanceWorkingDirectory);
}
FileUtils.createDirectoryWith755(Paths.get(taskInstanceWorkingDirectory));
+ final Path taskInstanceWorkingDirectoryPath = Paths.get(taskInstanceWorkingDirectory);
Review Comment:
> https://github.com/apache/dolphinscheduler/blob/dev/docs/docs/zh/guide/security/security.md https://github.com/apache/dolphinscheduler/blob/dev/docs/docs/en/guide/security/security.md
>
> The create tenant part.
Finished.
--
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-16918][Task] Fix wrong permissions configuration while executing shell [dolphinscheduler]
Posted by "ruanwenjun (via GitHub)" <gi...@apache.org>.
ruanwenjun commented on code in PR #16923:
URL: https://github.com/apache/dolphinscheduler/pull/16923#discussion_r1898299918
##########
dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/utils/FileUtils.java:
##########
@@ -289,6 +291,29 @@ public static void setFileTo755(File file) throws IOException {
}
}
+ public static void createDirectoryWith775(@NonNull Path path) throws IOException {
+ if (path.toFile().exists()) {
+ return;
+ }
+ if (OSUtils.isWindows()) {
+ Files.createDirectories(path);
+ } else {
+ Path parent = path.getParent();
+ if (parent != null && !parent.toFile().exists()) {
+ createDirectoryWith755(parent);
Review Comment:
```suggestion
createDirectoryWith775(parent);
```
Can we unify the method `createDirectoryWith755` ? these two method has the same logic.
We can create a new method createDirectory(@NonNull Path path, Set<PosixFilePermission> permissions);
--
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-16918][Task] Fix wrong permissions configuration while executing shell [dolphinscheduler]
Posted by "SbloodyS (via GitHub)" <gi...@apache.org>.
SbloodyS commented on code in PR #16923:
URL: https://github.com/apache/dolphinscheduler/pull/16923#discussion_r1897584880
##########
dolphinscheduler-worker/src/main/java/org/apache/dolphinscheduler/server/worker/utils/TaskExecutionContextUtils.java:
##########
@@ -51,6 +52,9 @@ public static void createTaskInstanceWorkingDirectory(TaskExecutionContext taskE
taskInstanceWorkingDirectory);
}
FileUtils.createDirectoryWith755(Paths.get(taskInstanceWorkingDirectory));
+ final Path taskInstanceWorkingDirectoryPath = Paths.get(taskInstanceWorkingDirectory);
Review Comment:
https://github.com/apache/dolphinscheduler/blob/dev/docs/docs/zh/guide/security/security.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-16918][Task] Fix wrong permissions configuration while executing shell [dolphinscheduler]
Posted by "ruanwenjun (via GitHub)" <gi...@apache.org>.
ruanwenjun commented on code in PR #16923:
URL: https://github.com/apache/dolphinscheduler/pull/16923#discussion_r1899417153
##########
dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/utils/FileUtils.java:
##########
@@ -289,6 +291,29 @@ public static void setFileTo755(File file) throws IOException {
}
}
+ public static void createDirectoryWith775(@NonNull Path path) throws IOException {
+ if (path.toFile().exists()) {
+ return;
+ }
+ if (OSUtils.isWindows()) {
+ Files.createDirectories(path);
+ } else {
Review Comment:
Some task e.g. sql task can run on windows, user can use winfows to debug.
--
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-16918][Task] Fix wrong permissions configuration while executing shell [dolphinscheduler]
Posted by "BruceWong96 (via GitHub)" <gi...@apache.org>.
BruceWong96 commented on PR #16923:
URL: https://github.com/apache/dolphinscheduler/pull/16923#issuecomment-2562323548
> You should run `mvn spotless:apply` to format the code. @BruceWong96
Finished. Thanks.
--
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-16918][Task] Fix wrong permissions configuration while executing shell [dolphinscheduler]
Posted by "ruanwenjun (via GitHub)" <gi...@apache.org>.
ruanwenjun commented on code in PR #16923:
URL: https://github.com/apache/dolphinscheduler/pull/16923#discussion_r1897194978
##########
dolphinscheduler-worker/src/main/java/org/apache/dolphinscheduler/server/worker/utils/TaskExecutionContextUtils.java:
##########
@@ -51,6 +52,9 @@ public static void createTaskInstanceWorkingDirectory(TaskExecutionContext taskE
taskInstanceWorkingDirectory);
}
FileUtils.createDirectoryWith755(Paths.get(taskInstanceWorkingDirectory));
+ final Path taskInstanceWorkingDirectoryPath = Paths.get(taskInstanceWorkingDirectory);
Review Comment:
If we change to `775` and make bootstrap user has the same group with tenant, then tenant will have permission to create file at working directory.
--
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-16918][Task] Fix wrong permissions configuration while executing shell [dolphinscheduler]
Posted by "SbloodyS (via GitHub)" <gi...@apache.org>.
SbloodyS commented on code in PR #16923:
URL: https://github.com/apache/dolphinscheduler/pull/16923#discussion_r1897243474
##########
dolphinscheduler-worker/src/main/java/org/apache/dolphinscheduler/server/worker/utils/TaskExecutionContextUtils.java:
##########
@@ -51,6 +52,9 @@ public static void createTaskInstanceWorkingDirectory(TaskExecutionContext taskE
taskInstanceWorkingDirectory);
}
FileUtils.createDirectoryWith755(Paths.get(taskInstanceWorkingDirectory));
+ final Path taskInstanceWorkingDirectoryPath = Paths.get(taskInstanceWorkingDirectory);
Review Comment:
I'm ok with this. If we change the task instance working directory to 775, we should add it to the installation docs to tell users.
--
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-16918][Task] Fix wrong permissions configuration while executing shell [dolphinscheduler]
Posted by "BruceWong96 (via GitHub)" <gi...@apache.org>.
BruceWong96 commented on code in PR #16923:
URL: https://github.com/apache/dolphinscheduler/pull/16923#discussion_r1904869962
##########
dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/utils/FileUtils.java:
##########
@@ -289,6 +291,29 @@ public static void setFileTo755(File file) throws IOException {
}
}
+ public static void createDirectoryWith775(@NonNull Path path) throws IOException {
+ if (path.toFile().exists()) {
+ return;
+ }
+ if (OSUtils.isWindows()) {
+ Files.createDirectories(path);
+ } else {
+ Path parent = path.getParent();
+ if (parent != null && !parent.toFile().exists()) {
+ createDirectoryWith755(parent);
Review Comment:
Finished, thanks for your advice. PATL. @ruanwenjun
--
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-16918][Task] Fix wrong permissions configuration while executing shell [dolphinscheduler]
Posted by "ruanwenjun (via GitHub)" <gi...@apache.org>.
ruanwenjun commented on code in PR #16923:
URL: https://github.com/apache/dolphinscheduler/pull/16923#discussion_r1899417153
##########
dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/utils/FileUtils.java:
##########
@@ -289,6 +291,29 @@ public static void setFileTo755(File file) throws IOException {
}
}
+ public static void createDirectoryWith775(@NonNull Path path) throws IOException {
+ if (path.toFile().exists()) {
+ return;
+ }
+ if (OSUtils.isWindows()) {
+ Files.createDirectories(path);
+ } else {
Review Comment:
The only one use case is user can debug some tasks like sql task on windows.
--
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-16918][Task] Fix wrong permissions configuration while executing shell [dolphinscheduler]
Posted by "BruceWong96 (via GitHub)" <gi...@apache.org>.
BruceWong96 commented on code in PR #16923:
URL: https://github.com/apache/dolphinscheduler/pull/16923#discussion_r1897203084
##########
dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/utils/FileUtils.java:
##########
@@ -272,6 +271,12 @@ public static void createDirectoryWith755(@NonNull Path path) throws IOException
}
}
+ public static Path setOwner(@NonNull Path path, @NonNull String owner) throws IOException {
+ UserPrincipalLookupService lookupService = FileSystems.getDefault().getUserPrincipalLookupService();
+ UserPrincipal user = lookupService.lookupPrincipalByName(owner);
+ return Files.setOwner(path, user);
Review Comment:
OK
--
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-16918][Task] Fix wrong permissions configuration while executing shell [dolphinscheduler]
Posted by "SbloodyS (via GitHub)" <gi...@apache.org>.
SbloodyS commented on code in PR #16923:
URL: https://github.com/apache/dolphinscheduler/pull/16923#discussion_r1897243474
##########
dolphinscheduler-worker/src/main/java/org/apache/dolphinscheduler/server/worker/utils/TaskExecutionContextUtils.java:
##########
@@ -51,6 +52,9 @@ public static void createTaskInstanceWorkingDirectory(TaskExecutionContext taskE
taskInstanceWorkingDirectory);
}
FileUtils.createDirectoryWith755(Paths.get(taskInstanceWorkingDirectory));
+ final Path taskInstanceWorkingDirectoryPath = Paths.get(taskInstanceWorkingDirectory);
Review Comment:
I'm ok with this. If we change the task instance working directory to 775, we should add it to the installation docs to tell users add tenant to ds user's group.
--
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-16918][Task] Fix wrong permissions configuration while executing shell [dolphinscheduler]
Posted by "SbloodyS (via GitHub)" <gi...@apache.org>.
SbloodyS commented on code in PR #16923:
URL: https://github.com/apache/dolphinscheduler/pull/16923#discussion_r1898302732
##########
dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/utils/FileUtils.java:
##########
@@ -289,6 +291,29 @@ public static void setFileTo755(File file) throws IOException {
}
}
+ public static void createDirectoryWith775(@NonNull Path path) throws IOException {
+ if (path.toFile().exists()) {
+ return;
+ }
+ if (OSUtils.isWindows()) {
+ Files.createDirectories(path);
+ } else {
Review Comment:
I think we can remove this since we don't support running on windows.
--
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-16918][Task] Fix wrong permissions configuration while executing shell [dolphinscheduler]
Posted by "BruceWong96 (via GitHub)" <gi...@apache.org>.
BruceWong96 commented on code in PR #16923:
URL: https://github.com/apache/dolphinscheduler/pull/16923#discussion_r1897581994
##########
dolphinscheduler-worker/src/main/java/org/apache/dolphinscheduler/server/worker/utils/TaskExecutionContextUtils.java:
##########
@@ -51,6 +52,9 @@ public static void createTaskInstanceWorkingDirectory(TaskExecutionContext taskE
taskInstanceWorkingDirectory);
}
FileUtils.createDirectoryWith755(Paths.get(taskInstanceWorkingDirectory));
+ final Path taskInstanceWorkingDirectoryPath = Paths.get(taskInstanceWorkingDirectory);
Review Comment:
@SbloodyS
Which part of the document should I update?
Also, please review the code i just committed.
Thanks.
--
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-16918][Task] Fix wrong permissions configuration while executing shell [dolphinscheduler]
Posted by "BruceWong96 (via GitHub)" <gi...@apache.org>.
BruceWong96 commented on PR #16923:
URL: https://github.com/apache/dolphinscheduler/pull/16923#issuecomment-2562539742
@caishunfeng @EricGao888 PTAL.
--
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-16918][Task] Fix wrong permissions configuration while executing shell [dolphinscheduler]
Posted by "ruanwenjun (via GitHub)" <gi...@apache.org>.
ruanwenjun commented on code in PR #16923:
URL: https://github.com/apache/dolphinscheduler/pull/16923#discussion_r1899415997
##########
dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/utils/FileUtils.java:
##########
@@ -289,6 +291,29 @@ public static void setFileTo755(File file) throws IOException {
}
}
+ public static void createDirectoryWith775(@NonNull Path path) throws IOException {
+ if (path.toFile().exists()) {
+ return;
+ }
+ if (OSUtils.isWindows()) {
+ Files.createDirectories(path);
+ } else {
Review Comment:
+1
--
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-16918][Task] Fix wrong permissions configuration while executing shell [dolphinscheduler]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #16923:
URL: https://github.com/apache/dolphinscheduler/pull/16923#issuecomment-2573276200
## [](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=16923) **Quality Gate failed**
Failed conditions
 [7.4% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=16923&metric=new_coverage&view=list) (required ≥ 60%)
 [46.4% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=16923&metric=new_duplicated_lines_density&view=list) (required ≤ 10%)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=16923)
--
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-16918][Task] Fix wrong permissions configuration while executing shell [dolphinscheduler]
Posted by "ruanwenjun (via GitHub)" <gi...@apache.org>.
ruanwenjun commented on code in PR #16923:
URL: https://github.com/apache/dolphinscheduler/pull/16923#discussion_r1897217118
##########
dolphinscheduler-worker/src/main/java/org/apache/dolphinscheduler/server/worker/utils/TaskExecutionContextUtils.java:
##########
@@ -51,6 +52,9 @@ public static void createTaskInstanceWorkingDirectory(TaskExecutionContext taskE
taskInstanceWorkingDirectory);
}
FileUtils.createDirectoryWith755(Paths.get(taskInstanceWorkingDirectory));
+ final Path taskInstanceWorkingDirectoryPath = Paths.get(taskInstanceWorkingDirectory);
Review Comment:
I think we can. cc @caishunfeng, @SbloodyS
--
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-16918][Task] Fix wrong permissions configuration while executing shell [dolphinscheduler]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #16923:
URL: https://github.com/apache/dolphinscheduler/pull/16923#issuecomment-2562357470
Please retry analysis of this Pull-Request directly on SonarQube Cloud
--
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-16918][Task] Fix wrong permissions configuration while executing shell [dolphinscheduler]
Posted by "BruceWong96 (via GitHub)" <gi...@apache.org>.
BruceWong96 commented on code in PR #16923:
URL: https://github.com/apache/dolphinscheduler/pull/16923#discussion_r1897202879
##########
dolphinscheduler-worker/src/main/java/org/apache/dolphinscheduler/server/worker/utils/TaskExecutionContextUtils.java:
##########
@@ -51,6 +52,9 @@ public static void createTaskInstanceWorkingDirectory(TaskExecutionContext taskE
taskInstanceWorkingDirectory);
}
FileUtils.createDirectoryWith755(Paths.get(taskInstanceWorkingDirectory));
+ final Path taskInstanceWorkingDirectoryPath = Paths.get(taskInstanceWorkingDirectory);
Review Comment:
> If we change to `775` and make bootstrap user has the same group with tenant, then tenant will have permission to create file at working directory.
So, can i create a function createDirectoryWith775() and call it here ?
--
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-16918][Task] Fix wrong permissions configuration while executing shell [dolphinscheduler]
Posted by "ruanwenjun (via GitHub)" <gi...@apache.org>.
ruanwenjun commented on code in PR #16923:
URL: https://github.com/apache/dolphinscheduler/pull/16923#discussion_r1897194473
##########
dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/utils/FileUtils.java:
##########
@@ -272,6 +271,12 @@ public static void createDirectoryWith755(@NonNull Path path) throws IOException
}
}
+ public static Path setOwner(@NonNull Path path, @NonNull String owner) throws IOException {
+ UserPrincipalLookupService lookupService = FileSystems.getDefault().getUserPrincipalLookupService();
+ UserPrincipal user = lookupService.lookupPrincipalByName(owner);
+ return Files.setOwner(path, user);
Review Comment:
`Files.setOwner` can only be executed by root, this is not expected.
##########
dolphinscheduler-worker/src/main/java/org/apache/dolphinscheduler/server/worker/utils/TaskExecutionContextUtils.java:
##########
@@ -51,6 +52,9 @@ public static void createTaskInstanceWorkingDirectory(TaskExecutionContext taskE
taskInstanceWorkingDirectory);
}
FileUtils.createDirectoryWith755(Paths.get(taskInstanceWorkingDirectory));
+ final Path taskInstanceWorkingDirectoryPath = Paths.get(taskInstanceWorkingDirectory);
Review Comment:
If we change to `775` and make bootstrap user has the same group with tenant, then tenant will have permission to create file.
--
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-16918][Task] Fix wrong permissions configuration while executing shell [dolphinscheduler]
Posted by "SbloodyS (via GitHub)" <gi...@apache.org>.
SbloodyS commented on PR #16923:
URL: https://github.com/apache/dolphinscheduler/pull/16923#issuecomment-2562320169
You should run `mvn spotless:apply` to format the code. @BruceWong96
--
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-16918][Task] Fix wrong permissions configuration while executing shell [dolphinscheduler]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #16923:
URL: https://github.com/apache/dolphinscheduler/pull/16923#issuecomment-2562120290
## [](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=16923) **Quality Gate failed**
Failed conditions
 [7.7% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=16923&metric=new_coverage&view=list) (required ≥ 60%)
 [48.1% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=16923&metric=new_duplicated_lines_density&view=list) (required ≤ 10%)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=16923)
--
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-16918][Task] Fix wrong permissions configuration while executing shell [dolphinscheduler]
Posted by "SbloodyS (via GitHub)" <gi...@apache.org>.
SbloodyS commented on code in PR #16923:
URL: https://github.com/apache/dolphinscheduler/pull/16923#discussion_r1897729516
##########
docs/docs/en/guide/security/security.md:
##########
@@ -19,7 +19,9 @@ Administrator login, default username/password: admin/dolphinscheduler123
- Tenant Code: **The tenant code is the user on Linux, unique and cannot be repeated**
- The administrator enters the `Security Center->Tenant Management` page, and clicks the `Create Tenant` button to create a tenant.
-> Note: Currently, only admin users can modify tenant.
+> Note:
+> 1. Currently, only admin users can modify tenant.
+> 2. If you create a tenant manually in the environment, you need to add the current tenant to the configured group, otherwise the tenant does not have working directory permissions.
Review Comment:
```suggestion
> 2. If you create a tenant manually in the Linux, you need to add the manually created tenant to the dolphinscheduler bootstrap user's group, so that the tenant will have enough working directory permissions.
```
##########
dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/utils/FileUtils.java:
##########
@@ -289,6 +291,29 @@ public static void setFileTo755(File file) throws IOException {
}
}
+ public static void createDirectoryWith775(@NonNull Path path) throws IOException {
+ if (path.toFile().exists()) {
+ return;
+ }
+ if (OSUtils.isWindows()) {
+ Files.createDirectories(path);
+ } else {
+ Path parent = path.getParent();
+ if (parent != null && !parent.toFile().exists()) {
+ createDirectoryWith755(parent);
+ }
+
+ try {
+ Files.createDirectory(path);
+ Files.setPosixFilePermissions(path, PERMISSION_775);
+ } catch (FileAlreadyExistsException fileAlreadyExistsException) {
+ // Catch the FileAlreadyExistsException here to avoid create the same parent directory in parallel
+ log.debug("The directory: {} already exists", path);
Review Comment:
```suggestion
log.error("The directory: {} already exists", path);
```
##########
docs/docs/zh/guide/security/security.md:
##########
@@ -18,7 +18,9 @@
- 租户编码：**租户编码是 Linux上 的用户，唯一，不能重复**
- 管理员进入安全中心->租户管理页面，点击“创建租户”按钮，创建租户。
-> 注意：目前仅有 admin 用户可以修改租户。
+> 注意：
+> 1. 目前仅有 admin 用户可以修改租户；
+> 2. 如果用户手动在环境中创建租户，需要把当前租户添加到已配置的用户组下，否则该租户不具备工作目录的权限。
Review Comment:
Same as english.
--
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-16918][Task] Fix wrong permissions configuration while executing shell [dolphinscheduler]
Posted by "ruanwenjun (via GitHub)" <gi...@apache.org>.
ruanwenjun commented on code in PR #16923:
URL: https://github.com/apache/dolphinscheduler/pull/16923#discussion_r1899415997
##########
dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/utils/FileUtils.java:
##########
@@ -289,6 +291,29 @@ public static void setFileTo755(File file) throws IOException {
}
}
+ public static void createDirectoryWith775(@NonNull Path path) throws IOException {
+ if (path.toFile().exists()) {
+ return;
+ }
+ if (OSUtils.isWindows()) {
+ Files.createDirectories(path);
+ } else {
Review Comment:
+1
--
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-16918][Task] Fix wrong permissions configuration while executing shell [dolphinscheduler]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #16923:
URL: https://github.com/apache/dolphinscheduler/pull/16923#issuecomment-2574803918
## [](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=16923) **Quality Gate passed**
Issues
 [2 New issues](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=16923&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=16923&issueStatuses=ACCEPTED)
Measures
 [1 Security Hotspot](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=16923&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [64.9% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=16923&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=16923&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=16923)
--
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-16918][Task] Fix wrong permissions configuration while executing shell [dolphinscheduler]
Posted by "ruanwenjun (via GitHub)" <gi...@apache.org>.
ruanwenjun merged PR #16923:
URL: https://github.com/apache/dolphinscheduler/pull/16923
--
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-16918][Task] Fix wrong permissions configuration while executing shell [dolphinscheduler]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #16923:
URL: https://github.com/apache/dolphinscheduler/pull/16923#issuecomment-2573277954
## [](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=16923) **Quality Gate failed**
Failed conditions
 [7.4% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=16923&metric=new_coverage&view=list) (required ≥ 60%)
 [46.4% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=16923&metric=new_duplicated_lines_density&view=list) (required ≤ 10%)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=16923)
--
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