[Bug] Fix can modify file which is not under resource path #15652
Conversation
ruanwenjun
requested review from
Amy0104,
SbloodyS,
caishunfeng and
songjianet
as code owners
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| 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), |
Check failure
Code scanning / CodeQL
Log Injection
| 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())); |
Check failure
Code scanning / CodeQL
Log Injection
| putMsg(result, Status.SUCCESS); | ||
| } catch (Exception e) { | ||
| log.error(MessageFormat.format(" copy {0} -> {1} fail", originFullName, destHdfsFileName), e); | ||
| putMsg(result, Status.HDFS_COPY_FAIL); |
Check failure
Code scanning / CodeQL
Log Injection
| } catch (Exception e) { | ||
| log.error(MessageFormat.format(" copy {0} -> {1} fail", originFullName, destHdfsFileName), e); | ||
| putMsg(result, Status.HDFS_COPY_FAIL); | ||
| throw new ServiceException(MessageFormat.format( |
Check failure
Code scanning / CodeQL
Log Injection
|
|
||
| /** | ||
| * verify resource by full name or pid and type | ||
| * |
Check failure
Code scanning / CodeQL
Log Injection
| storageOperate.delete(fullName, false); | ||
| } | ||
|
|
||
| storageOperate.upload(tenantCode, localFilename, fullName, true, true); |
Check failure
Code scanning / CodeQL
Log Injection
| storageOperate.delete(fullName, false); | ||
| } | ||
|
|
||
| storageOperate.upload(tenantCode, localFilename, fullName, true, true); |
Check failure
Code scanning / CodeQL
Log Injection
| putMsg(result, Status.SUCCESS); | ||
| return result; | ||
| } | ||
|
|
Check failure
Code scanning / CodeQL
Log Injection
| putMsg(result, Status.SUCCESS); | ||
| return result; | ||
| } | ||
|
|
Check failure
Code scanning / CodeQL
Log Injection
| public org.springframework.core.io.Resource downloadResource(User loginUser, | ||
| String fullName) { | ||
| public org.springframework.core.io.Resource downloadResource(User loginUser, String fullName) { | ||
| if (fullName.endsWith("/")) { |
Check failure
Code scanning / CodeQL
Log Injection
songjianet
left a comment
songjianet
left a comment
There was a problem hiding this comment.
Please remove referenced but unused variables.
ruanwenjun
force-pushed
the
dev_wenjun_fixResourceAPI
branch
from
2168846 to
6a1d24c
Compare
ruanwenjun
force-pushed
the
dev_wenjun_fixResourceAPI
branch
from
6a1d24c to
6ac4720
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev #15652 +/- ##
============================================
- Coverage 38.57% 38.54% -0.03%
+ Complexity 4792 4783 -9
============================================
Files 1316 1316
Lines 45043 44968 -75
Branches 4823 4816 -7
============================================
- Hits 17374 17333 -41
+ Misses 25779 25755 -24
+ Partials 1890 1880 -10 ☔ View full report in Codecov by Sentry. |
|
| String name, | ||
| ResourceType type, | ||
| MultipartFile file, | ||
| public Result<Object> uploadResource(User loginUser, String name, ResourceType type, MultipartFile file, |
There was a problem hiding this comment.
I am not sure how the user can only modify the resource file under resource path. Could you please give some brief description
There was a problem hiding this 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.
6 participants
Purpose of the pull request
Right now, user can use resource API to modify all files in the machine, this is not accepted.
This pr will fix this, user can only modify the resource file under resource path.
Brief change log
Verify this pull request
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:
(or)
If your pull request contain incompatible change, you should also add it to
docs/docs/en/guide/upgrede/incompatible.md