From 45a532e6a6d6dcf4d1695331cfce7e1ff3811b18 Mon Sep 17 00:00:00 2001 From: Kyd Cao Date: Tue, 11 Aug 2026 02:10:40 -0500 Subject: [PATCH] feat: add configurable file permissions for new objects Replace the hardcoded 0644 defaultFilePerm with a NewFilePerm option on the posix and scoutfs backends, exposed as the --file-perms flag and VGW_FILE_PERMS env var alongside the existing dir-perms option. The mode passed to open() is masked by the process umask, so the O_TMPFILE path now chmods explicitly to match the CreateTemp fallback path and give new objects the configured mode regardless of umask. --- backend/posix/posix.go | 42 +++++++++++++- backend/posix/with_otmpfile.go | 91 +++++++++++++++++-------------- backend/posix/without_otmpfile.go | 49 ++++++++--------- backend/scoutfs/scoutfs.go | 22 +++++++- backend/scoutfs/scoutfs_compat.go | 13 ++++- cmd/internal/gwcli/posix.go | 21 ++++++- cmd/internal/gwcli/scoutfs.go | 15 ++++- extra/example.conf | 14 +++++ 8 files changed, 191 insertions(+), 76 deletions(-) diff --git a/backend/posix/posix.go b/backend/posix/posix.go index 62fd7ebf..ef2216b7 100644 --- a/backend/posix/posix.go +++ b/backend/posix/posix.go @@ -78,6 +78,9 @@ type Posix struct { // newDirPerm is the permission to set on newly created directories newDirPerm fs.FileMode + // newFilePerm is the permission to set on newly created object files + newFilePerm fs.FileMode + // forceNoTmpFile is a flag to disable the use of O_TMPFILE even // if the filesystem supports it. This is needed for cases where // there are different filesystems mounted below the bucket level. @@ -130,9 +133,22 @@ type Posix struct { dataIntegrityEtag bool } +func (o *PosixOpts) SetNewDirPerm(perm fs.FileMode) { + o.NewDirPerm = perm + o.newDirPermSet = true +} + +func (o *PosixOpts) SetNewFilePerm(perm fs.FileMode) { + o.NewFilePerm = perm + o.newFilePermSet = true +} + var _ backend.Backend = &Posix{} const ( + defaultNewDirPerm fs.FileMode = 0755 + defaultNewFilePerm fs.FileMode = 0644 + MetaTmpDir = ".sgwtmp" MetaTmpMultipartDir = MetaTmpDir + "/multipart" onameAttr = "objname" @@ -190,8 +206,18 @@ type PosixOpts struct { BucketLinks bool //VersioningDir sets the version directory to enable object versioning VersioningDir string - // NewDirPerm specifies the permission to set on newly created directories - NewDirPerm fs.FileMode + // NewDirPerm specifies the permission to set on newly created directories. + // When unset, the default is 0755. An explicit zero value (0000) is valid + // and is preserved when SetNewDirPerm(0000) is used. + NewDirPerm fs.FileMode + newDirPermSet bool + // NewFilePerm specifies the permission to set on newly created object + // files. Defaults to 0644 when unset. Unlike NewDirPerm, this mode is + // applied explicitly and is therefore not reduced by the process umask. + // An explicit zero value (0000) is valid and is preserved when + // SetNewFilePerm(0000) is used. + NewFilePerm fs.FileMode + newFilePermSet bool // SideCarDir sets the directory to store sidecar metadata SideCarDir string // ForceNoTmpFile disables the use of O_TMPFILE even if the filesystem @@ -284,6 +310,15 @@ func New(rootdir string, meta meta.MetadataStorer, opts PosixOpts) (*Posix, erro fmt.Println("Using sidecar directory for metadata:", sidecardirAbs) } + newDirPerm := defaultNewDirPerm + if opts.newDirPermSet { + newDirPerm = opts.NewDirPerm.Perm() + } + newFilePerm := defaultNewFilePerm + if opts.newFilePermSet { + newFilePerm = opts.NewFilePerm.Perm() + } + return &Posix{ meta: meta, rootfd: f, @@ -294,7 +329,8 @@ func New(rootdir string, meta meta.MetadataStorer, opts PosixOpts) (*Posix, erro chowngid: opts.ChownGID, bucketlinks: opts.BucketLinks, versioningDir: versioningdirAbs, - newDirPerm: opts.NewDirPerm, + newDirPerm: newDirPerm, + newFilePerm: newFilePerm, forceNoTmpFile: opts.ForceNoTmpFile, forceNoCopyFileRange: opts.ForceNoCopyFileRange, enableODirect: opts.EnableODirect, diff --git a/backend/posix/with_otmpfile.go b/backend/posix/with_otmpfile.go index 497abeb1..a1e73e48 100644 --- a/backend/posix/with_otmpfile.go +++ b/backend/posix/with_otmpfile.go @@ -38,24 +38,20 @@ import ( const procfddir = "/proc/self/fd" type tmpfile struct { - f *os.File - bucket string - objname string - isOTmp bool - procFDName string - useODirect bool - size int64 - doChown bool - uid int - gid int - newDirPerm fs.FileMode + f *os.File + bucket string + objname string + isOTmp bool + procFDName string + useODirect bool + size int64 + doChown bool + uid int + gid int + newDirPerm fs.FileMode + newFilePerm fs.FileMode } -var ( - // TODO: make this configurable - defaultFilePerm uint32 = 0644 -) - func (p *Posix) openTmpFile(dir, bucket, obj string, size int64, acct auth.Account, dofalloc bool, forceNoTmpFile bool, allowODirect odirectPolicy) (*tmpfile, error) { uid, gid, doChown := p.getChownIDs(acct) @@ -76,7 +72,9 @@ func (p *Posix) openTmpFile(dir, bucket, obj string, size int64, acct auth.Accou useODirect = true } - fd, err := unix.Open(dir, openFlags, defaultFilePerm) + filePerm := uint32(p.newFilePerm.Perm()) + + fd, err := unix.Open(dir, openFlags, filePerm) if err != nil { if errors.Is(err, syscall.EROFS) { return nil, s3err.GetAPIError(s3err.ErrMethodNotAllowed) @@ -85,7 +83,7 @@ func (p *Posix) openTmpFile(dir, bucket, obj string, size int64, acct auth.Accou if p.enableODirect && bool(allowODirect) && isODirectUnsupportedOpenErr(err) { warnODirectUnsupportedOnce("openTmpFile", err) - fd, err = unix.Open(dir, unix.O_RDWR|unix.O_TMPFILE|unix.O_CLOEXEC, defaultFilePerm) + fd, err = unix.Open(dir, unix.O_RDWR|unix.O_TMPFILE|unix.O_CLOEXEC, filePerm) if err == nil { useODirect = false } else if errors.Is(err, syscall.EROFS) { @@ -103,18 +101,29 @@ func (p *Posix) openTmpFile(dir, bucket, obj string, size int64, acct auth.Accou // later to link file into namespace f := os.NewFile(uintptr(fd), filepath.Join(procfddir, strconv.Itoa(fd))) + // The mode passed to open() is masked by the process umask. Set the + // configured mode explicitly so new objects get the same permissions + // regardless of umask, and regardless of whether this or the CreateTemp + // fallback path (which also chmods) created the file. + err = f.Chmod(p.newFilePerm) + if err != nil { + f.Close() + return nil, fmt.Errorf("set temp file mode: %w", err) + } + tmp := &tmpfile{ - f: f, - bucket: bucket, - objname: obj, - isOTmp: true, - procFDName: strconv.Itoa(fd), - useODirect: useODirect, - size: size, - doChown: doChown, - uid: uid, - gid: gid, - newDirPerm: p.newDirPerm, + f: f, + bucket: bucket, + objname: obj, + isOTmp: true, + procFDName: strconv.Itoa(fd), + useODirect: useODirect, + size: size, + doChown: doChown, + uid: uid, + gid: gid, + newDirPerm: p.newDirPerm, + newFilePerm: p.newFilePerm, } // falloc is best effort, its fine if this fails @@ -158,7 +167,7 @@ func (p *Posix) openMkTemp(dir, bucket, obj string, size int64, dofalloc bool, u return nil, fmt.Errorf("close temp file before O_DIRECT reopen: %w", err) } - fd, err := unix.Open(name, unix.O_RDWR|unix.O_CLOEXEC|unix.O_DIRECT, defaultFilePerm) + fd, err := unix.Open(name, unix.O_RDWR|unix.O_CLOEXEC|unix.O_DIRECT, uint32(p.newFilePerm.Perm())) if err == nil { f = os.NewFile(uintptr(fd), name) useODirect = true @@ -176,14 +185,16 @@ func (p *Posix) openMkTemp(dir, bucket, obj string, size int64, dofalloc bool, u } tmp := &tmpfile{ - f: f, - bucket: bucket, - objname: obj, - useODirect: useODirect, - size: size, - doChown: doChown, - uid: uid, - gid: gid, + f: f, + bucket: bucket, + objname: obj, + useODirect: useODirect, + size: size, + doChown: doChown, + uid: uid, + gid: gid, + newDirPerm: p.newDirPerm, + newFilePerm: p.newFilePerm, } // falloc is best effort, its fine if this fails if size > 0 && dofalloc { @@ -326,7 +337,7 @@ func (tmp *tmpfile) fallbackLink() error { tempname := tmp.f.Name() // reset default file mode because CreateTemp uses 0600 - tmp.f.Chmod(fs.FileMode(defaultFilePerm)) + tmp.f.Chmod(tmp.newFilePerm) err := tmp.f.Close() if err != nil { @@ -359,7 +370,7 @@ func (tmp *tmpfile) fallbackLink() error { // if this fails fallback to copy backoffMs := initialBackoffMs for range maxDirRecreateRetries { - err = backend.MoveFile(tempname, objPath, fs.FileMode(defaultFilePerm)) + err = backend.MoveFile(tempname, objPath, tmp.newFilePerm) if !errors.Is(err, syscall.ENOENT) { break } diff --git a/backend/posix/without_otmpfile.go b/backend/posix/without_otmpfile.go index 05748b22..8c03bf09 100644 --- a/backend/posix/without_otmpfile.go +++ b/backend/posix/without_otmpfile.go @@ -40,14 +40,15 @@ type tmpfile struct { bucket string objname string // Retained for compatibility with shared tmpfile methods in otmpfile_common. - isOTmp bool - procFDName string - useODirect bool - size int64 - newDirPerm fs.FileMode - uid int - gid int - doChown bool + isOTmp bool + procFDName string + useODirect bool + size int64 + newDirPerm fs.FileMode + newFilePerm fs.FileMode + uid int + gid int + doChown bool } func (p *Posix) openTmpFile(dir, bucket, obj string, size int64, acct auth.Account, _ bool, _ bool, allowODirect odirectPolicy) (*tmpfile, error) { @@ -85,32 +86,28 @@ func (p *Posix) openTmpFile(dir, bucket, obj string, size int64, acct auth.Accou } return &tmpfile{ - f: f, - bucket: bucket, - objname: obj, - isOTmp: false, - procFDName: "", - useODirect: false, - size: size, - newDirPerm: p.newDirPerm, - uid: uid, - gid: gid, - doChown: doChown, + f: f, + bucket: bucket, + objname: obj, + isOTmp: false, + procFDName: "", + useODirect: false, + size: size, + newDirPerm: p.newDirPerm, + newFilePerm: p.newFilePerm, + uid: uid, + gid: gid, + doChown: doChown, }, nil } -var ( - // TODO: make this configurable - defaultFilePerm fs.FileMode = 0644 -) - func (tmp *tmpfile) link() error { tempname := tmp.f.Name() objPath := filepath.Join(tmp.bucket, tmp.objname) // reset default file mode because CreateTemp uses 0600 - tmp.f.Chmod(defaultFilePerm) + tmp.f.Chmod(tmp.newFilePerm) err := tmp.f.Close() if err != nil { @@ -119,7 +116,7 @@ func (tmp *tmpfile) link() error { backoffMs := initialBackoffMs for { - err = backend.MoveFile(tempname, objPath, defaultFilePerm) + err = backend.MoveFile(tempname, objPath, tmp.newFilePerm) if !os.IsNotExist(err) { break } diff --git a/backend/scoutfs/scoutfs.go b/backend/scoutfs/scoutfs.go index a0d94318..71fa547f 100644 --- a/backend/scoutfs/scoutfs.go +++ b/backend/scoutfs/scoutfs.go @@ -32,8 +32,16 @@ type ScoutfsOpts struct { BucketLinks bool //VersioningDir sets the version directory to enable object versioning VersioningDir string - // NewDirPerm specifies the permission to set on newly created directories - NewDirPerm fs.FileMode + // NewDirPerm specifies the permission to set on newly created directories. + // An explicit zero value (0000) is valid and is preserved when set via + // SetNewDirPerm. + NewDirPerm fs.FileMode + newDirPermSet bool + // NewFilePerm specifies the permission to set on newly created object + // files. Defaults to 0644 when unset; an explicit zero value (0000) is + // valid and is preserved when set via SetNewFilePerm. + NewFilePerm fs.FileMode + newFilePermSet bool // GlacierMode enables glacier emulation for offline files GlacierMode bool // DisableNoArchive prevents setting noarchive on temporary files @@ -61,4 +69,14 @@ type ScoutfsOpts struct { DataIntegrityEtag bool } +func (o *ScoutfsOpts) SetNewDirPerm(perm fs.FileMode) { + o.NewDirPerm = perm + o.newDirPermSet = true +} + +func (o *ScoutfsOpts) SetNewFilePerm(perm fs.FileMode) { + o.NewFilePerm = perm + o.newFilePermSet = true +} + var _ backend.Backend = &ScoutFS{} diff --git a/backend/scoutfs/scoutfs_compat.go b/backend/scoutfs/scoutfs_compat.go index e4461c0c..34b70b40 100644 --- a/backend/scoutfs/scoutfs_compat.go +++ b/backend/scoutfs/scoutfs_compat.go @@ -74,18 +74,25 @@ type ScoutFS struct { func New(rootdir string, opts ScoutfsOpts) (*ScoutFS, error) { metastore := meta.XattrMeta{} - p, err := posix.New(rootdir, metastore, posix.PosixOpts{ + posixOpts := posix.PosixOpts{ ChownUID: opts.ChownUID, ChownGID: opts.ChownGID, BucketLinks: opts.BucketLinks, - NewDirPerm: opts.NewDirPerm, VersioningDir: opts.VersioningDir, ValidateBucketNames: opts.ValidateBucketNames, Concurrency: opts.Concurrency, CopyObjectThreshold: opts.CopyObjectThreshold, DefaultEtag: opts.DefaultEtag, DataIntegrityEtag: opts.DataIntegrityEtag, - }) + } + if opts.newDirPermSet { + posixOpts.SetNewDirPerm(opts.NewDirPerm) + } + if opts.newFilePermSet { + posixOpts.SetNewFilePerm(opts.NewFilePerm) + } + + p, err := posix.New(rootdir, metastore, posixOpts) if err != nil { return nil, err } diff --git a/cmd/internal/gwcli/posix.go b/cmd/internal/gwcli/posix.go index 714a9b5e..782180cf 100644 --- a/cmd/internal/gwcli/posix.go +++ b/cmd/internal/gwcli/posix.go @@ -24,11 +24,17 @@ import ( "github.com/versity/versitygw/backend/posix" ) +// maxFilePerms is the highest accepted value for the file-perms option. The +// posix backend only applies permission bits to new objects, so anything +// above this would be silently dropped. +const maxFilePerms = 0777 + var ( chownuid, chowngid bool bucketlinks bool versioningDir string dirPerms uint + filePerms uint sidecar string nometa bool forceNoTmpFile bool @@ -90,6 +96,14 @@ will be translated into the file /mnt/fs/gwroot/mybucket/a/b/c/myobject`, DefaultText: "0755", Value: 0755, }, + &cli.UintFlag{ + Name: "file-perms", + Usage: "default file permissions for new objects", + EnvVars: []string{"VGW_FILE_PERMS"}, + Destination: &filePerms, + DefaultText: "0644", + Value: 0644, + }, &cli.StringFlag{ Name: "sidecar", Usage: "use provided sidecar directory to store metadata", @@ -161,6 +175,10 @@ func runPosix(ctx *cli.Context) error { return fmt.Errorf("invalid directory permissions: %d", dirPerms) } + if filePerms > maxFilePerms { + return fmt.Errorf("invalid file permissions: %o, must be within 0000-0777", filePerms) + } + if nometa && sidecar != "" { return fmt.Errorf("cannot use both nometa and sidecar metadata") } @@ -174,7 +192,6 @@ func runPosix(ctx *cli.Context) error { ChownGID: chowngid, BucketLinks: bucketlinks, VersioningDir: versioningDir, - NewDirPerm: fs.FileMode(dirPerms), ForceNoTmpFile: forceNoTmpFile, ForceNoCopyFileRange: forceNoCopyFileRange, EnableODirect: enableODirect, @@ -185,6 +202,8 @@ func runPosix(ctx *cli.Context) error { DefaultEtag: defaultEtag, DataIntegrityEtag: dataIntegrityEtag, } + opts.SetNewDirPerm(fs.FileMode(dirPerms)) + opts.SetNewFilePerm(fs.FileMode(filePerms)) var ms meta.MetadataStorer switch { diff --git a/cmd/internal/gwcli/scoutfs.go b/cmd/internal/gwcli/scoutfs.go index f48d4fc7..bfe883c1 100644 --- a/cmd/internal/gwcli/scoutfs.go +++ b/cmd/internal/gwcli/scoutfs.go @@ -95,6 +95,14 @@ move interfaces as well as support for tiered filesystems.`, DefaultText: "0755", Value: 0755, }, + &cli.UintFlag{ + Name: "file-perms", + Usage: "default file permissions for new objects", + EnvVars: []string{"VGW_FILE_PERMS"}, + Destination: &filePerms, + DefaultText: "0644", + Value: 0644, + }, &cli.BoolFlag{ Name: "disable-noarchive", Usage: "disable setting noarchive for multipart part uploads", @@ -133,6 +141,10 @@ func runScoutfs(ctx *cli.Context) error { return fmt.Errorf("invalid directory permissions: %d", dirPerms) } + if filePerms > maxFilePerms { + return fmt.Errorf("invalid file permissions: %o, must be within 0000-0777", filePerms) + } + if actionsConcurrency <= 0 { return fmt.Errorf("concurrency must be positive, got %d", actionsConcurrency) } @@ -142,7 +154,6 @@ func runScoutfs(ctx *cli.Context) error { opts.ChownUID = chownuid opts.ChownGID = chowngid opts.BucketLinks = bucketlinks - opts.NewDirPerm = fs.FileMode(dirPerms) opts.DisableNoArchive = disableNoArchive opts.VersioningDir = versioningDir opts.ValidateBucketNames = DisableStrictBucketNames @@ -151,6 +162,8 @@ func runScoutfs(ctx *cli.Context) error { opts.CopyObjectThreshold = CopyObjectThreshold opts.DefaultEtag = defaultEtag opts.DataIntegrityEtag = dataIntegrityEtag + opts.SetNewDirPerm(fs.FileMode(dirPerms)) + opts.SetNewFilePerm(fs.FileMode(filePerms)) be, err := scoutfs.New(ctx.Args().Get(0), opts) if err != nil { diff --git a/extra/example.conf b/extra/example.conf index cce8538c..93cea2b3 100644 --- a/extra/example.conf +++ b/extra/example.conf @@ -596,6 +596,13 @@ ROOT_SECRET_ACCESS_KEY= # as any parent directories automatically created with object uploads. #VGW_DIR_PERMS=0755 +# The default permissions mode when creating new object files is 0644. Use +# VGW_FILE_PERMS option to set a different mode for any new file that the +# gateway creates for an uploaded object. Only permission bits (0000-0777) are +# accepted. Unlike VGW_DIR_PERMS, this mode is applied explicitly and is not +# reduced by the process umask. +#VGW_FILE_PERMS=0644 + # To enable object versions, the VGW_VERSIONING_DIR option must be set to the # directory that will be used to store the object versions. The version # directory must NOT be a subdirectory of the VGW_BACKEND_ARG directory. @@ -722,6 +729,13 @@ ROOT_SECRET_ACCESS_KEY= # as any parent directories automatically created with object uploads. #VGW_DIR_PERMS=0755 +# The default permissions mode when creating new object files is 0644. Use +# VGW_FILE_PERMS option to set a different mode for any new file that the +# gateway creates for an uploaded object. Only permission bits (0000-0777) are +# accepted. Unlike VGW_DIR_PERMS, this mode is applied explicitly and is not +# reduced by the process umask. +#VGW_FILE_PERMS=0644 + # To enable object versions, the VGW_VERSIONING_DIR option must be set to the # directory that will be used to store the object versions. The version # directory must NOT be a subdirectory of the VGW_BACKEND_ARG directory.