Re: [PR] [Bug] Fix can modify file which is not under resource path [dolphinscheduler]

2024-03-01 Thread via GitHub 


caishunfeng merged PR #15652:
URL: https://github.com/apache/dolphinscheduler/pull/15652


-- 
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-unsubscr...@dolphinscheduler.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] [Bug] Fix can modify file which is not under resource path [dolphinscheduler]

2024-02-29 Thread via GitHub 


ruanwenjun commented on code in PR #15652:
URL: 
https://github.com/apache/dolphinscheduler/pull/15652#discussion_r1507419008


##
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/ResourcesServiceImpl.java:
##
@@ -182,10 +165,7 @@ private String getFullName(String currentDir, String name) 
{
  */
 @Override
 @Transactional
-public Result createResource(User loginUser,
- String name,
- ResourceType type,
- MultipartFile file,
+public Result uploadResource(User loginUser, String name, 
ResourceType type, MultipartFile file,

Review Comment:
   DS will throw exception when the user want to modify the system file which 
is not under resource path by update content API.  This can help to protect the 
system.
   
   The upload API will not be affected.



-- 
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-unsubscr...@dolphinscheduler.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] [Bug] Fix can modify file which is not under resource path [dolphinscheduler]

2024-02-29 Thread via GitHub 


rickchengx commented on code in PR #15652:
URL: 
https://github.com/apache/dolphinscheduler/pull/15652#discussion_r1507209582


##
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/ResourcesServiceImpl.java:
##
@@ -182,10 +165,7 @@ private String getFullName(String currentDir, String name) 
{
  */
 @Override
 @Transactional
-public Result createResource(User loginUser,
- String name,
- ResourceType type,
- MultipartFile file,
+public Result uploadResource(User loginUser, String name, 
ResourceType type, MultipartFile file,

Review Comment:
   I am not sure how the user can only modify the resource file under resource 
path. Could you please give some brief description



-- 
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-unsubscr...@dolphinscheduler.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] [Bug] Fix can modify file which is not under resource path [dolphinscheduler]

2024-02-28 Thread via GitHub 


sonarcloud[bot] commented on PR #15652:
URL: 
https://github.com/apache/dolphinscheduler/pull/15652#issuecomment-1970318244

   ## [![Quality Gate 
Failed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-failed-20px.png
 'Quality Gate 
Failed')](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler=15652)
 **Quality Gate failed**  
   Failed conditions  
   
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/failed-16px.png
 '') [46.2% Coverage on New 
Code](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler=15652=new_coverage=list)
 (required ≥ 60%)  
 
   [See analysis details on 
SonarCloud](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler=15652)
   
   


-- 
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-unsubscr...@dolphinscheduler.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] [Bug] Fix can modify file which is not under resource path [dolphinscheduler]

2024-02-28 Thread via GitHub 


sonarcloud[bot] commented on PR #15652:
URL: 
https://github.com/apache/dolphinscheduler/pull/15652#issuecomment-1970317634

   ## [![Quality Gate 
Failed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-failed-20px.png
 'Quality Gate 
Failed')](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler=15652)
 **Quality Gate failed**  
   Failed conditions  
   
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/failed-16px.png
 '') [47.5% Coverage on New 
Code](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler=15652=new_coverage=list)
 (required ≥ 60%)  
 
   [See analysis details on 
SonarCloud](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler=15652)
   
   


-- 
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-unsubscr...@dolphinscheduler.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] [Bug] Fix can modify file which is not under resource path [dolphinscheduler]

2024-02-28 Thread via GitHub 


codecov-commenter commented on PR #15652:
URL: 
https://github.com/apache/dolphinscheduler/pull/15652#issuecomment-1970312879

   ## 
[Codecov](https://app.codecov.io/gh/apache/dolphinscheduler/pull/15652?src=pr=h1_medium=referral_source=github_content=comment_campaign=pr+comments_term=apache)
 Report
   Attention: Patch coverage is `47.2%` with `19 lines` in your changes are 
missing coverage. Please review.
   > Project coverage is 38.54%. Comparing base 
[(`3fda980`)](https://app.codecov.io/gh/apache/dolphinscheduler/commit/3fda980006512bcbe39c06c79547a6439ae1a70c?el=desc_medium=referral_source=github_content=comment_campaign=pr+comments_term=apache)
 to head 
[(`671f88a`)](https://app.codecov.io/gh/apache/dolphinscheduler/pull/15652?src=pr=desc_medium=referral_source=github_content=comment_campaign=pr+comments_term=apache).
   
   > :exclamation: Current head 671f88a differs from pull request most recent 
head 6ac4720. Consider uploading reports for the commit 6ac4720 to get more 
accurate results
   
   | 
[Files](https://app.codecov.io/gh/apache/dolphinscheduler/pull/15652?src=pr=tree_medium=referral_source=github_content=comment_campaign=pr+comments_term=apache)
 | Patch % | Lines |
   |---|---|---|
   | 
[...heduler/api/service/impl/ResourcesServiceImpl.java](https://app.codecov.io/gh/apache/dolphinscheduler/pull/15652?src=pr=tree_medium=referral_source=github_content=comment_campaign=pr+comments_term=apache#diff-ZG9scGhpbnNjaGVkdWxlci1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvYXBpL3NlcnZpY2UvaW1wbC9SZXNvdXJjZXNTZXJ2aWNlSW1wbC5qYXZh)
 | 45.16% | [17 Missing :warning: 
](https://app.codecov.io/gh/apache/dolphinscheduler/pull/15652?src=pr=tree_medium=referral_source=github_content=comment_campaign=pr+comments_term=apache)
 |
   | 
[...nscheduler/api/controller/ResourcesController.java](https://app.codecov.io/gh/apache/dolphinscheduler/pull/15652?src=pr=tree_medium=referral_source=github_content=comment_campaign=pr+comments_term=apache#diff-ZG9scGhpbnNjaGVkdWxlci1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvYXBpL2NvbnRyb2xsZXIvUmVzb3VyY2VzQ29udHJvbGxlci5qYXZh)
 | 60.00% | [2 Missing :warning: 
](https://app.codecov.io/gh/apache/dolphinscheduler/pull/15652?src=pr=tree_medium=referral_source=github_content=comment_campaign=pr+comments_term=apache)
 |
   
   Additional details and impacted files
   
   
   ```diff
   @@ Coverage Diff  @@
   ##dev   #15652  +/-   ##
   
   - Coverage 38.57%   38.54%   -0.03% 
   + Complexity 4792 4783   -9 
   
 Files  1316 1316  
 Lines 4504344932 -111 
 Branches   4823 4792  -31 
   
   - Hits  1737417319  -55 
   + Misses2577925734  -45 
   + Partials   1890 1879  -11 
   ```
   
   
   
   
   
   [:umbrella: View full report in Codecov by 
Sentry](https://app.codecov.io/gh/apache/dolphinscheduler/pull/15652?src=pr=continue_medium=referral_source=github_content=comment_campaign=pr+comments_term=apache).
   
   :loudspeaker: Have feedback on the report? [Share it 
here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral_source=github_content=comment_campaign=pr+comments_term=apache).
   


-- 
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-unsubscr...@dolphinscheduler.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] [Bug] Fix can modify file which is not under resource path [dolphinscheduler]

2024-02-28 Thread via GitHub 


github-advanced-security[bot] commented on code in PR #15652:
URL: 
https://github.com/apache/dolphinscheduler/pull/15652#discussion_r1505841576


##
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/ResourcesServiceImpl.java:
##
@@ -241,8 +221,8 @@
 String.format("upload resource: %s file: %s failed.", 
name, file.getOriginalFilename()));
 } else
 ApiServerMetrics.recordApiResourceUploadSize(file.getSize());
-log.info("Upload resource file complete, resourceName:{}, 
fileName:{}.",
-RegexUtils.escapeNRT(name), 
RegexUtils.escapeNRT(file.getOriginalFilename()));
+log.info("Upload resource file complete, resourceName:{}, 
fileName:{}.", RegexUtils.escapeNRT(name),

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/3919)



##
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/ResourcesServiceImpl.java:
##
@@ -241,8 +221,8 @@
 String.format("upload resource: %s file: %s failed.", 
name, file.getOriginalFilename()));
 } else
 ApiServerMetrics.recordApiResourceUploadSize(file.getSize());
-log.info("Upload resource file complete, resourceName:{}, 
fileName:{}.",
-RegexUtils.escapeNRT(name), 
RegexUtils.escapeNRT(file.getOriginalFilename()));
+log.info("Upload resource file complete, resourceName:{}, 
fileName:{}.", RegexUtils.escapeNRT(name),
+RegexUtils.escapeNRT(file.getOriginalFilename()));

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/3920)



##
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/ResourcesServiceImpl.java:
##
@@ -877,8 +822,8 @@
 Result result = new Result<>();
 putMsg(result, Status.SUCCESS);
 if (checkResourceExists(fullName)) {
-log.error("Resource with same name exists so can not create again, 
resourceType:{}, resourceName:{}.",
-type, RegexUtils.escapeNRT(fullName));
+log.error("Resource with same name exists so can not create again, 
resourceType:{}, resourceName:{}.", type,
+RegexUtils.escapeNRT(fullName));

Review Comment:
   ## Log Injection
   
   This log entry depends on a [user-provided value](1).
   This log entry depends on a [user-provided value](2).
   This log entry depends on a [user-provided value](3).
   
   [Show more 
details](https://github.com/apache/dolphinscheduler/security/code-scanning/3924)



##
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/ResourcesServiceImpl.java:
##
@@ -1212,46 +1113,45 @@
 }
 
 /**
- * @param fullName resource full name
- * @param tenantCode   tenant code
- * @param content  content
+ * @param fullName   resource full name
+ * @param tenantCode tenant code
+ * @param contentcontent
  * @return result
  */
-private Result uploadContentToStorage(User loginUser, String 
fullName, String tenantCode, String content) {
+private Result uploadContentToStorage(String fullName, String 
tenantCode, String content) {
 Result result = new Result<>();
 String localFilename = "";
 try {
 localFilename = FileUtils.getUploadFilename(tenantCode, 
UUID.randomUUID().toString());
 
 if (!FileUtils.writeContent2File(content, localFilename)) {
 // write file fail
-log.error("Write file error, fileName:{}, content:{}.", 
localFilename,
-RegexUtils.escapeNRT(content));
+log.error("Write file error, fileName:{}, content:{}.", 
localFilename, RegexUtils.escapeNRT(content));
 putMsg(result, Status.RESOURCE_NOT_EXIST);
 return result;
 }
~
~// get resource file path
~String resourcePath = storageOperate.getResDir(tenantCode);
~log.info("resource  path is {}, resource dir is {}", fullName, 
resourcePath);
~
 if (!storageOperate.exists(resourcePath)) {
 // create if tenant dir not exists
 storageOperate.createTenantDirIfNotExists(tenantCode);
-log.info("Create tenant dir because path {} does not exist, 
tenantCode:{}.", resourcePath,
-tenantCode);
+log.info("Create tenant dir because path {} does not exist, 
tenantCode:{}.", resourcePath, tenantCode);

Review Comment:
   ## Log Injection
   
   This log entry depends on a [user-provided value](1).
   
   [Show more