Skip to content

feat: When creating files/folders, use the parent directory's user an… #8419

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
f2c-ci-robot merged 1 commit into from
Apr 18, 2025
Merged

feat: When creating files/folders, use the parent directory's user an… #8419

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 13 additions & 3 deletions agent/app/service/file.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -204,15 +204,25 @@ func (f *FileService) Create(op request.FileCreate) error {
}
}
if op.IsDir {
return fo.CreateDirWithMode(op.Path, fs.FileMode(mode))
if err := fo.CreateDirWithMode(op.Path, fs.FileMode(mode)); err != nil {
return err
}
handleDefaultOwn(op.Path)
return nil
}
if op.IsLink {
if !fo.Stat(op.LinkPath) {
return buserr.New("ErrLinkPathNotFound")
}
return fo.LinkFile(op.LinkPath, op.Path, op.IsSymlink)
if err := fo.LinkFile(op.LinkPath, op.Path, op.IsSymlink); err != nil {
return err
}
}
if err := fo.CreateFileWithMode(op.Path, fs.FileMode(mode)); err != nil {
return err
}
return fo.CreateFileWithMode(op.Path, fs.FileMode(mode))
handleDefaultOwn(op.Path)
return nil
}

func (f *FileService) Delete(op request.FileDelete) error {
Copy link
Copy Markdown
Member

@wanghe-fit2cloud wanghe-fit2cloud Apr 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are several areas where the code could improve:

  1. Return Type Consistency: The function Create returns an error type directly if an operation fails, but when it reaches the end of the method without returning an error, its return type does not match other methods. This inconsistency might be due to an oversight.

  2. Error Handling Clarity: When handling errors returned by certain operations within a conditional block, you're using early return statements (return). However, for better clarity and consistency, consider whether you should have used if err != nil { // handle error } else { continue; }.

  3. Duplication: There is repetition in some parts of the Create method regarding how errors are handled after creating directories or files with specific modes. Consider refactoring this duplication into helper functions or variables for clarity.

Here is a slightly improved version of the Create method considering these points:

func (f *FileService) Create(op request.FileCreate) error {
	if op.IsDir {
		return fo.CreateDirWithMode(op.Path, fs.FileMode(mode)) ||
			 handleError(fo.DirExists(op.Path), fmt.Sprintf("%s already exists", op.Path))
	}

	if op.IsLink && !fo.Exists(op.LinkPath) {
		return buserr.New("ErrLinkPathNotFound") ||
			 handleError(fo.LinkFile(op.LinkPath, op.Path, op.IsSymlink),
					fmt.Sprintf("unable to link %v -> %v", op.LinkPath, op.Path)),
	}

	return createFileIfNeeded(op.Path, mode)

handleDefaultOwn := func(path string) {}
handleError := func(err error, message string) error {
    log.Errorf(message)
    return err
}

Summary:

  • Ensure that all paths leading to successful completion of Create must call return. If they do not, ensure consistent return types.
  • Improve readability and maintainability by reducing redundancy and making errors more granularly manageable.

Expand Down
15 changes: 15 additions & 0 deletions agent/app/service/website_utils.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"path/filepath"
"strconv"
"strings"
"syscall"
"time"

"github.com/1Panel-dev/1Panel/agent/app/repo"
Expand Down Expand Up @@ -1357,3 +1358,17 @@ func ConfigAIProxy(website model.Website) error {
}
return nil
}

func handleDefaultOwn(dir string) {
parentDir := path.Dir(dir)
info, err := os.Stat(parentDir)
if err != nil {
return
}
stat, ok := info.Sys().(*syscall.Stat_t)
uid, gid := -1, -1
if ok {
uid, gid = int(stat.Uid), int(stat.Gid)
}
_ = os.Chown(dir, uid, gid)
}
Copy link
Copy Markdown
Member

@wanghe-fit2cloud wanghe-fit2cloud Apr 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The provided code changes mostly focus on modifying how files are created during the handleDefaultOwn function to change their ownership based on parent directory properties using os.Chown. No other significant code modifications exist. This could be considered an improvement in organizational clarity as it centralizes this behavior within a single function, rather than having similar operations repeated elsewhere.

A minor suggestion would be to include a return statement after calling _ = os.Chown(dir, uid, gid) in case there might be cases where setting ownership fails but no action is required (e.g., permissions errors).

6 changes: 3 additions & 3 deletions core/app/api/v2/backup.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ func (b *BaseApi) RefreshToken(c *gin.Context) {
helper.InternalServer(c, err)
return
}
helper.SuccessWithOutData(c)
helper.Success(c)
}

// @Tags Backup Account
Expand Down Expand Up @@ -112,7 +112,7 @@ func (b *BaseApi) DeleteBackup(c *gin.Context) {
helper.InternalServer(c, err)
return
}
helper.SuccessWithOutData(c)
helper.Success(c)
}

// @Tags Backup Account
Expand All @@ -134,5 +134,5 @@ func (b *BaseApi) UpdateBackup(c *gin.Context) {
helper.InternalServer(c, err)
return
}
helper.SuccessWithOutData(c)
helper.Success(c)
}
6 changes: 3 additions & 3 deletions core/app/api/v2/command.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ func (b *BaseApi) CreateCommand(c *gin.Context) {
helper.InternalServer(c, err)
return
}
helper.SuccessWithOutData(c)
helper.Success(c)
}

// @Tags Command
Expand Down Expand Up @@ -119,7 +119,7 @@ func (b *BaseApi) DeleteCommand(c *gin.Context) {
helper.InternalServer(c, err)
return
}
helper.SuccessWithOutData(c)
helper.Success(c)
}

// @Tags Command
Expand All @@ -141,5 +141,5 @@ func (b *BaseApi) UpdateCommand(c *gin.Context) {
helper.InternalServer(c, err)
return
}
helper.SuccessWithOutData(c)
helper.Success(c)
}
6 changes: 3 additions & 3 deletions core/app/api/v2/group.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ func (b *BaseApi) CreateGroup(c *gin.Context) {
helper.InternalServer(c, err)
return
}
helper.SuccessWithOutData(c)
helper.Success(c)
}

// @Tags System Group
Expand All @@ -47,7 +47,7 @@ func (b *BaseApi) DeleteGroup(c *gin.Context) {
helper.InternalServer(c, err)
return
}
helper.SuccessWithOutData(c)
helper.Success(c)
}

// @Tags System Group
Expand All @@ -69,7 +69,7 @@ func (b *BaseApi) UpdateGroup(c *gin.Context) {
helper.InternalServer(c, err)
return
}
helper.SuccessWithOutData(c)
helper.Success(c)
}

// @Tags System Group
Expand Down
2 changes: 1 addition & 1 deletion core/app/api/v2/helper/helper.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ func SuccessWithData(ctx *gin.Context, data interface{}) {
ctx.Abort()
}

func SuccessWithOutData(ctx *gin.Context) {
func Success(ctx *gin.Context) {
res := dto.Response{
Code: http.StatusOK,
Message: "success",
Expand Down
4 changes: 2 additions & 2 deletions core/app/api/v2/host.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ func (b *BaseApi) DeleteHost(c *gin.Context) {
helper.InternalServer(c, err)
return
}
helper.SuccessWithOutData(c)
helper.Success(c)
}

// @Tags Host
Expand Down Expand Up @@ -244,7 +244,7 @@ func (b *BaseApi) UpdateHostGroup(c *gin.Context) {
helper.InternalServer(c, err)
return
}
helper.SuccessWithOutData(c)
helper.Success(c)
}

// @Tags Host
Expand Down
2 changes: 1 addition & 1 deletion core/app/api/v2/logs.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,5 +78,5 @@ func (b *BaseApi) CleanLogs(c *gin.Context) {
return
}

helper.SuccessWithOutData(c)
helper.Success(c)
}
8 changes: 4 additions & 4 deletions core/app/api/v2/script_library.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ func (b *BaseApi) CreateScript(c *gin.Context) {
helper.InternalServer(c, err)
return
}
helper.SuccessWithOutData(c)
helper.Success(c)
}

// @Tags ScriptLibrary
Expand Down Expand Up @@ -84,7 +84,7 @@ func (b *BaseApi) DeleteScript(c *gin.Context) {
helper.InternalServer(c, err)
return
}
helper.SuccessWithOutData(c)
helper.Success(c)
}

// @Tags ScriptLibrary
Expand All @@ -99,7 +99,7 @@ func (b *BaseApi) SyncScript(c *gin.Context) {
helper.InternalServer(c, err)
return
}
helper.SuccessWithOutData(c)
helper.Success(c)
}

// @Tags ScriptLibrary
Expand All @@ -121,7 +121,7 @@ func (b *BaseApi) UpdateScript(c *gin.Context) {
helper.InternalServer(c, err)
return
}
helper.SuccessWithOutData(c)
helper.Success(c)
}

func (b *BaseApi) RunScript(c *gin.Context) {
Expand Down
26 changes: 13 additions & 13 deletions core/app/api/v2/setting.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ func (b *BaseApi) GetTerminalSettingInfo(c *gin.Context) {
// @Security Timestamp
// @Router /core/settings/search/available [get]
func (b *BaseApi) GetSystemAvailable(c *gin.Context) {
helper.SuccessWithOutData(c)
helper.Success(c)
}

// @Tags System Setting
Expand Down Expand Up @@ -86,7 +86,7 @@ func (b *BaseApi) UpdateSetting(c *gin.Context) {
entranceValue := base64.StdEncoding.EncodeToString([]byte(req.Value))
c.SetCookie("SecurityEntrance", entranceValue, 0, "", "", false, true)
}
helper.SuccessWithOutData(c)
helper.Success(c)
}

// @Tags System Setting
Expand All @@ -108,7 +108,7 @@ func (b *BaseApi) UpdateTerminalSetting(c *gin.Context) {
helper.InternalServer(c, err)
return
}
helper.SuccessWithOutData(c)
helper.Success(c)
}

// @Tags System Setting
Expand Down Expand Up @@ -139,7 +139,7 @@ func (b *BaseApi) UpdateProxy(c *gin.Context) {
helper.InternalServer(c, err)
return
}
helper.SuccessWithOutData(c)
helper.Success(c)
}

// @Tags System Setting
Expand All @@ -161,7 +161,7 @@ func (b *BaseApi) UpdateMenu(c *gin.Context) {
helper.InternalServer(c, err)
return
}
helper.SuccessWithOutData(c)
helper.Success(c)
}

// @Tags System Setting
Expand All @@ -183,7 +183,7 @@ func (b *BaseApi) UpdatePassword(c *gin.Context) {
helper.InternalServer(c, err)
return
}
helper.SuccessWithOutData(c)
helper.Success(c)
}

// @Tags System Setting
Expand All @@ -205,7 +205,7 @@ func (b *BaseApi) UpdateSSL(c *gin.Context) {
helper.InternalServer(c, err)
return
}
helper.SuccessWithOutData(c)
helper.Success(c)
}

// @Tags System Setting
Expand Down Expand Up @@ -274,7 +274,7 @@ func (b *BaseApi) UpdateBindInfo(c *gin.Context) {
helper.InternalServer(c, err)
return
}
helper.SuccessWithOutData(c)
helper.Success(c)
}

// @Tags System Setting
Expand All @@ -296,7 +296,7 @@ func (b *BaseApi) UpdatePort(c *gin.Context) {
helper.InternalServer(c, err)
return
}
helper.SuccessWithOutData(c)
helper.Success(c)
}

// @Tags System Setting
Expand All @@ -318,7 +318,7 @@ func (b *BaseApi) HandlePasswordExpired(c *gin.Context) {
helper.InternalServer(c, err)
return
}
helper.SuccessWithOutData(c)
helper.Success(c)
}

// @Tags System Setting
Expand Down Expand Up @@ -380,7 +380,7 @@ func (b *BaseApi) MFABind(c *gin.Context) {
return
}

helper.SuccessWithOutData(c)
helper.Success(c)
}

func (b *BaseApi) ReloadSSL(c *gin.Context) {
Expand All @@ -393,7 +393,7 @@ func (b *BaseApi) ReloadSSL(c *gin.Context) {
helper.InternalServer(c, err)
return
}
helper.SuccessWithOutData(c)
helper.Success(c)
}

// @Tags System Setting
Expand Down Expand Up @@ -442,7 +442,7 @@ func (b *BaseApi) UpdateApiConfig(c *gin.Context) {
helper.InternalServer(c, err)
return
}
helper.SuccessWithOutData(c)
helper.Success(c)
}

func checkEntrancePattern(val string) bool {
Copy link
Copy Markdown
Member

@wanghe-fit2cloud wanghe-fit2cloud Apr 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The provided Go code changes seem generally well-structured, but there are a few minor issues and optimizations that could be addressed:

Issues:

  1. Duplicate Comments: The comments above func (b *BaseApi) UpdateProxy(...) and other similar functions have identical content: // @Tags System Setting. This redundancy can be removed.

  2. Code Length: Some of the function definitions extend over multiple lines without proper indentation. It's helpful to keep line lengths under 100 characters for readability.

  3. Comments Formatting: Use consistent formatting for inline comments within function blocks.

  4. Helper Usage: Ensure that helper.Success is used correctly throughout the file. If it takes additional parameters, ensure they are passed appropriately.

Optimizations:

  1. Simplify Cookie Handling: The cookie handling in UpdateTerminalSetting seems redundant. You can simplify it or remove it if not needed.

Here’s an updated version with these improvements:

package main

import (
	"encoding/base64"
)

type BaseApi struct{}

func (b *BaseApi) GetTerminalSettingInfo(c *gin.Context) {
	helper.SuccessWithOutData(c)
}

// @Security Timestamp
// @Router /core/settings/search/available [get]
func (b *BaseApi) GetSystemAvailable(c *gin.Context) {
	helper.Success(c)
}

// Update setting information
func (b *BaseApi) UpdateSetting(c *gin.Context) error {
	req := &updateReq{} // Assuming updateReq is defined elsewhere
	if req.Value != "" {
		valueBytes := base64.StdEncoding.EncodeToString([]byte(req.Value))
		c.SetCookie("SecurityEntrance", valueBytes, 0, "", "", false, true)
	}
	return helper.Success(c)
}

// Update terminal setting
func (b *BaseApi) UpdateTerminalSetting(c *gin.Context) error {
	var err error
	// Your logic here...
	return handlerSuccess(err)
}

// Update proxy settings
func (b *BaseApi) UpdateProxy(c *gin.Context) error {
	err := handleInternalServer()
	return handlerSuccess(err)
}

// Other handlers remain unchanged...

func handlerSuccess(err error) error {
	if err != nil {
		return helper.InternalServer(e)
	}
	return nil
}

// Check entrance pattern
func checkEntrancePattern(val string) bool {
	// Implement your validation logic here
	return false
}

Explanation:

  • Improved Structure: Removed duplicate comment headers and simplified the comments for clarity.
  • Consistent Naming Convention: Ensured handlerSuccess is consistently used instead of repeated calls to helper.Success.
  • Error Handling: Simplified error handling using err == nil.

These changes should improve the maintainability and readability of the code. Make sure to review any dependent functions or modules as they may require adjustments as well.

Expand Down
4 changes: 2 additions & 2 deletions core/app/api/v2/upgrade.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ func (b *BaseApi) Upgrade(c *gin.Context) {
helper.InternalServer(c, err)
return
}
helper.SuccessWithOutData(c)
helper.Success(c)
}

// @Tags System Setting
Expand All @@ -84,5 +84,5 @@ func (b *BaseApi) Rollback(c *gin.Context) {
helper.InternalServer(c, err)
return
}
helper.SuccessWithOutData(c)
helper.Success(c)
}
5 changes: 3 additions & 2 deletions core/init/router/proxy.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,9 @@ func Proxy() gin.HandlerFunc {
return
}
var currentNode string
if c.Query("operateNode") != "" {
currentNode = c.Query("operateNode")
queryNode := c.Query("operateNode")
if queryNode != "" && queryNode != "undefined" {
currentNode = queryNode
} else {
currentNode = c.Request.Header.Get("CurrentNode")
}
Expand Down
1 change: 1 addition & 0 deletions frontend/src/lang/modules/en.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3116,6 +3116,7 @@ const message = {
websites: 'Website List',
trend: 'Trend Statistics',
reqCount: 'Request Count',
uriHelper: 'You can use /test/* or /*/index.php to exclude Uri',
},
tamper: {
tamper: 'Website Tamper Protection',
Expand Down
1 change: 1 addition & 0 deletions frontend/src/lang/modules/ja.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2974,6 +2974,7 @@ const message = {
websites: 'ウェブサイトリスト',
trend: 'トレンド統計',
reqCount: 'リクエスト数',
uriHelper: '/test/* や /*/index.php を使用して Uri を除外できます',
},
tamper: {
tamper: 'ウェブサイトの改ざん防止',
Expand Down
1 change: 1 addition & 0 deletions frontend/src/lang/modules/ko.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2930,6 +2930,7 @@ const message = {
websites: '웹사이트 목록',
trend: '추세 통계',
reqCount: '요청 수',
uriHelper: '/test/* 또는 /*/index.php를 사용하여 Uri를 제외할 수 있습니다',
},
tamper: {
tamper: '웹사이트 변조 방지',
Expand Down
1 change: 1 addition & 0 deletions frontend/src/lang/modules/ms.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3041,6 +3041,7 @@ const message = {
websites: 'Senarai Laman Web',
trend: 'Statistik Trend',
reqCount: 'Jumlah Permintaan',
uriHelper: 'Anda boleh menggunakan /test/* atau /*/index.php untuk mengecualikan Uri',
},
tamper: {
tamper: 'Perlindungan daripada peng篡改 laman web',
Expand Down
1 change: 1 addition & 0 deletions frontend/src/lang/modules/pt-br.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3043,6 +3043,7 @@ const message = {
websites: 'Lista de Sites',
trend: 'Estatísticas de Tendência',
reqCount: 'Contagem de Solicitações',
uriHelper: 'Você pode usar /test/* ou /*/index.php para excluir Uri',
},
tamper: {
tamper: 'Proteção contra adulteração do site',
Expand Down
1 change: 1 addition & 0 deletions frontend/src/lang/modules/ru.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3034,6 +3034,7 @@ const message = {
websites: 'Список веб-сайтов',
trend: 'Статистика тренда',
reqCount: 'Количество запросов',
uriHelper: 'Вы можете использовать /test/* или /*/index.php для исключения Uri',
},
tamper: {
tamper: 'Защита от подделки сайта',
Expand Down
Loading
Loading