Skip to content

Commit

Permalink
Enable explicitly setting target file permissions (#365)
Browse files Browse the repository at this point in the history
  • Loading branch information
ethanjli authored Jan 27, 2025
1 parent 3c7e68b commit 103c173
Show file tree
Hide file tree
Showing 4 changed files with 44 additions and 13 deletions.
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,11 @@ All notable changes to this project will be documented in this file.
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

## Unreleased
## 0.8.0-alpha.6 - 2024-01-27

### Added

- (spec) Added an optional `permissions` field to file exports in the packaging spec. If specified, this field enables overriding the source file's Unix permissions with an explicitly-provided permissions value for creating a regular file at the target path. The permissions should be specified as an octal value (e.g. `0644`).
- (spec) Now the bundle manifest file's `exports` section lists information about the Docker Compose apps created by the bundle's package deployments.
- (spec) Now the bundle manifest file's `downloads` section lists OCI images to be cached for Docker Compose apps; this is enabled by a breaking change in the layout of that section, described below.
- (cli) Now `stage show-bun` prints information about required pallets in the "Includes" section.
Expand Down
10 changes: 10 additions & 0 deletions docs/specs/00-package.md
Original file line number Diff line number Diff line change
Expand Up @@ -1360,6 +1360,16 @@ url: ghcr.io/planktoscope/machine-name:0.1.3
target: overlays/etc/dnsmasq.d/dhcp-and-dns.conf
```

`permissions` is the octal Unix permission bits which should be attached to the exported file.

- This field is optional: it defaults to the permissions of the source file. For `local`-type source files, this is likely to be `0644` (corresponding to `rw-r--r--`) due to how Git handles file permissions.

- Example:

```yaml
permissions: 0777
```

`tags` is an array of strings which describe the file export. These tags are ignored in determining whether file exports conflict with each other, since they are not part of the file export's location(s).

- This field is optional.
Expand Down
40 changes: 28 additions & 12 deletions internal/app/forklift/bundles.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,11 +125,11 @@ func CopyFS(fsys core.PathedFS, dest string) error {
}
return os.MkdirAll(filepath.FromSlash(path.Join(dest, filePath)), fileInfo.Mode())
}
return copyFSFile(fsys, filePath, path.Join(dest, filePath))
return copyFSFile(fsys, filePath, path.Join(dest, filePath), 0)
})
}

func copyFSFile(fsys core.PathedFS, sourcePath, destPath string) error {
func copyFSFile(fsys core.PathedFS, sourcePath, destPath string, destPerms fs.FileMode) error {
if readLinkFS, ok := fsys.(ReadLinkFS); ok {
sourceInfo, err := readLinkFS.StatLink(sourcePath)
if err != nil {
Expand Down Expand Up @@ -167,8 +167,11 @@ func copyFSFile(fsys core.PathedFS, sourcePath, destPath string) error {
return CopyFS(fsys, destPath)
}

if destPerms == 0 {
destPerms = sourceInfo.Mode().Perm()
}
destFile, err := os.OpenFile( //nolint:gosec // dest path is set by config files which we trust
destPath, os.O_CREATE|os.O_TRUNC|os.O_WRONLY, sourceInfo.Mode().Perm(),
destPath, os.O_CREATE|os.O_TRUNC|os.O_WRONLY, destPerms,
)
if err != nil {
return errors.Wrapf(err, "couldn't open dest file %s for copying", destPath)
Expand Down Expand Up @@ -503,6 +506,7 @@ func (b *FSBundle) WriteFileExports(dlCache *FSDownloadCache) error {
func exportLocalFile(resolved *ResolvedDepl, export core.FileExportRes, exportPath string) error {
if err := copyFSFile(
resolved.Pkg.FS, strings.TrimPrefix(export.Source, "/"), filepath.FromSlash(exportPath),
export.Permissions,
); err != nil {
return errors.Wrapf(err, "couldn't export file from %s to %s", export.Source, exportPath)
}
Expand All @@ -517,6 +521,7 @@ func exportHTTPFile(export core.FileExportRes, exportPath string, dlCache *FSDow
if err := copyFSFile(
dlCache.FS, strings.TrimPrefix(strings.TrimPrefix(sourcePath, dlCache.FS.Path()), "/"),
filepath.FromSlash(exportPath),
export.Permissions,
); err != nil {
return errors.Wrapf(err, "couldn't export file from %s to %s", sourcePath, exportPath)
}
Expand Down Expand Up @@ -572,7 +577,9 @@ func exportArchiveFile(
"unrecognized archive file type: %s (.%s)", kind.MIME.Value, kind.Extension,
)
}
if err = extractFromArchive(archiveReader, export.Source, exportPath); err != nil {
if err = extractFromArchive(
archiveReader, export.Source, exportPath, export.Permissions,
); err != nil {
return errors.Wrapf(
err, "couldn't extract %s from cached download archive %s to %s",
export.Source, export.URL, exportPath,
Expand Down Expand Up @@ -612,7 +619,9 @@ func determineFileType(
return filetype.MatchReader(archiveFile)
}

func extractFromArchive(tarReader *tar.Reader, sourcePath, exportPath string) error {
func extractFromArchive(
tarReader *tar.Reader, sourcePath, exportPath string, destPerms fs.FileMode,
) error {
if sourcePath == "/" || sourcePath == "." {
sourcePath = ""
}
Expand All @@ -629,15 +638,16 @@ func extractFromArchive(tarReader *tar.Reader, sourcePath, exportPath string) er
continue
}

if err = extractFile(header, tarReader, sourcePath, exportPath); err != nil {
if err = extractFile(header, tarReader, sourcePath, exportPath, destPerms); err != nil {
return err
}
}
return nil
}

func extractFile(
header *tar.Header, tarReader *tar.Reader, sourcePath, exportPath string,
// FIXME: also handle destPerms for directories and symlinks!
header *tar.Header, tarReader *tar.Reader, sourcePath, exportPath string, destPerms fs.FileMode,
) error {
targetPath := path.Join(exportPath, strings.TrimPrefix(header.Name, sourcePath))
switch header.Typeflag {
Expand All @@ -650,7 +660,7 @@ func extractFile(
)
}
case tar.TypeReg:
if err := extractRegularFile(header, tarReader, sourcePath, targetPath); err != nil {
if err := extractRegularFile(header, tarReader, sourcePath, targetPath, destPerms); err != nil {
return errors.Wrapf(
err, "couldn't export regular file %s from archive to %s", header.Name, targetPath,
)
Expand All @@ -677,11 +687,17 @@ func extractFile(
}

func extractRegularFile(
header *tar.Header, tarReader *tar.Reader, sourcePath, targetPath string,
header *tar.Header, tarReader *tar.Reader, sourcePath, targetPath string, destPerms fs.FileMode,
) error {
targetFile, err := os.OpenFile(
filepath.FromSlash(targetPath), os.O_RDWR|os.O_CREATE|os.O_TRUNC,
fs.FileMode(header.Mode)&fs.ModePerm, //nolint:gosec // tar's Mode won't(?) overflow fs.FileMode
if destPerms == 0 {
destPerms = fs.FileMode( //nolint:gosec // (G115) tar's Mode won't(?) overflow fs.FileMode
header.Mode,
) & fs.ModePerm
}
// FIXME: we suppress gosec G304 below, but for security we should check targetPath to ensure it's
// a valid path (i.e. within the Forklift workspace)!
targetFile, err := os.OpenFile( //nolint:gosec // (G304) the point is to write to arbitrary paths
filepath.Clean(filepath.FromSlash(targetPath)), os.O_RDWR|os.O_CREATE|os.O_TRUNC, destPerms,
)
if err != nil {
return errors.Wrapf(err, "couldn't create export file at %s", targetPath)
Expand Down
4 changes: 4 additions & 0 deletions pkg/core/packages-models.go
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package core

import "io/fs"

// A FSPkg is a Forklift package stored at the root of a [fs.FS] filesystem.
type FSPkg struct {
// Pkg is the Forklift package at the root of the filesystem.
Expand Down Expand Up @@ -197,6 +199,8 @@ type FileExportRes struct {
Source string `yaml:"source,omitempty"`
// URL is the URL of the file to be downloaded for export, for a `http` source.
URL string `yaml:"url,omitempty"`
// Permissions is the Unix permission bits to attach to the exported file.
Permissions fs.FileMode `yaml:"permissions,omitempty"`
// Target is the path where the file will be exported to, relative to an export directory.
Target string `yaml:"target"`
}
Expand Down

0 comments on commit 103c173

Please sign in to comment.