From a17ccda07782073902de863b446dd3ceb0661061 Mon Sep 17 00:00:00 2001 From: Narendra Reddy Date: Sun, 26 Apr 2026 16:16:52 +0530 Subject: [PATCH 01/16] dominator: add golangci-lint file and pre-commit-config for linting and formatting --- .golangci.yml | 69 +++++++++++++++++++++++++++++++++++++++++ .pre-commit-config.yaml | 54 ++++++++++++++++++++++++++++++++ 2 files changed, 123 insertions(+) create mode 100644 .golangci.yml create mode 100644 .pre-commit-config.yaml diff --git a/.golangci.yml b/.golangci.yml new file mode 100644 index 00000000..192a6e73 --- /dev/null +++ b/.golangci.yml @@ -0,0 +1,69 @@ +version: "2" + +run: + timeout: 10m + tests: true + relative-path-mode: gomod + +linters: + enable: + - errcheck + - govet + - ineffassign + - lll + - misspell + - revive + - staticcheck + - unconvert + - unused + - godot + + settings: + lll: + line-length: 80 + tab-width: 1 + misspell: + locale: US + godot: + scope: all + period: true + exclude: + - '^go:' + - '^\+build' + - '^(?i)(todo|fixme|nolint):' + errcheck: + # This fixes the issue you had where fmt.Fprintf was failing the lint + exclude-functions: + - fmt.Fprintf + - fmt.Printf + - fmt.Println + - fmt.Fprint + + # These rules ensure lll doesn't annoy you on imports or URLs + exclusions: + rules: + - linters: [lll] + source: '^\s*"github\.com/Cloud-Foundations/(Dominator|tricoder)/.*"$' + - linters: [lll] + source: 'https?://' + - linters: [lll] + source: '^\s*//go:generate ' + - linters: [lll] + path: '(^|/)testdata/' + - linters: [errcheck] + source: '^\s*defer .*\.Close\(\)$' + - linters: [revive] + path: '.*_test\.go$' + +formatters: + enable: + - gofmt + - goimports + - golines + settings: + golines: + max-len: 80 + shorten-comments: true + no-chain-split-dots: true + goimports: + local-prefixes: github.com diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml new file mode 100644 index 00000000..51fb3182 --- /dev/null +++ b/.pre-commit-config.yaml @@ -0,0 +1,54 @@ +repos: + - repo: https://github.com/golangci/golangci-lint + rev: v2.0.0 # MUST be v2+ to read the config file above + hooks: + # 1. RUN FIRST: Dedicated Line Length Check + # This ensures you always see length issues even if logic errors exist. + - id: golangci-lint + name: check-line-length + entry: bash -c 'GOOS=linux golangci-lint run --new-from-rev=master --timeout=5m --enable-only=lll' + pass_filenames: false + + # 2. RUN SECOND: All other linters + - id: golangci-lint + name: lint-my-changes + entry: bash -c 'GOOS=linux golangci-lint run --new-from-rev=master --timeout=5m --disable lll' + pass_filenames: false + + - repo: local + hooks: + - id: clean-go-code + name: fix-blank-lines-and-imports + language: system + files: \.go$ + pass_filenames: true + entry: | + bash -c ' + # 0. Ensure binaries are in PATH and install if missing + export PATH=$PATH:$(go env GOPATH)/bin + + if ! which goimports-reviser &> /dev/null; then + echo "Installing goimports-reviser..." + go install github.com/incu6us/goimports-reviser/v3@latest + fi + + if ! which golines &> /dev/null; then + echo "Installing golines..." + go install ://github.com + fi + + for file in "$@"; do + # 1. DENSITY PASS: Remove internal blank lines + perl -i -0777 -pe "s/\{(\s*\n){2,}/{\n/g; s/(\n\s*\n)+\s*\}/\n\}/g; s/(\n\s+[^\n]+)(\n\s*\n)+/\1\n/g" "$file" + + # 2. ORDERING PASS: 3-group layout (StdLib, 3rd-Party, Local) + # Use the actual module prefix for the company group + goimports-reviser -rm-unused -format -project-name "://github.com" "$file" + + # 3. Format using golines from golangci-lint + golangci-lint fmt "$file" + + # 4. FINAL PASS: Canonical formatting + gofmt -w "$file" + git add "$file" + done' -- From 798f11ce7c8f2d4957d452d453a4cd799af07f23 Mon Sep 17 00:00:00 2001 From: Narendra Reddy Date: Tue, 28 Apr 2026 00:58:00 +0530 Subject: [PATCH 02/16] imaginator: handle destination symlinks in appendTree --- lib/fsutil/api.go | 14 ++++++-- lib/fsutil/append.go | 23 ++++++++++--- lib/fsutil/append_test.go | 38 +++++++++++----------- lib/fsutil/readSymlink.go | 68 +++++++++++++++++++++++++++++++++++++++ 4 files changed, 116 insertions(+), 27 deletions(-) create mode 100644 lib/fsutil/readSymlink.go diff --git a/lib/fsutil/api.go b/lib/fsutil/api.go index 0e1e5d6a..e213fddf 100644 --- a/lib/fsutil/api.go +++ b/lib/fsutil/api.go @@ -21,6 +21,7 @@ const ( var ( ErrorChecksumMismatch = errors.New("checksum mismatch") + ErrNotASymlink = errors.New("path is not a symlink") ) // AppendFile will append data from the sourceFilename to destFilename. @@ -28,15 +29,22 @@ var ( // copied from sourceFilename. If there are any errors, then destFilename // may have partial data appended. // AppendFile is not safe to call concurrently for the same file. -func AppendFile(destFilename, sourceFilename string) error { - return appendFile(destFilename, sourceFilename) +func AppendFile(destDir, destFilename, sourceFilename string) error { + return appendFile(destDir, destFilename, sourceFilename) } // AppendTree recursively merges sourceDir into destDir. // It appends contents to existing files or copies new ones // while preserving permissions. // Directory structures are mirrored. -// Returns an error if symlinks or non-regular files are encountered. +// Returns an error if symlinks or non-regular files +// are encountered in sourceDir. +// If destination path is a symlink, behavior is as follows +// 1. If symlink points to a dangling target, it fails with error. +// 2. If symlink target resolves to a path outside destDir, +// it fails with error. +// 3. If symlink points to a valid path inside destDir, content will be +// appended to target file. func AppendTree(destDir, sourceDir string) error { return appendTree(destDir, sourceDir, AppendFile) } diff --git a/lib/fsutil/append.go b/lib/fsutil/append.go index d861dd34..8737f948 100644 --- a/lib/fsutil/append.go +++ b/lib/fsutil/append.go @@ -29,8 +29,8 @@ func appendToFile(destFilename string, reader io.Reader, return nil } -func appendFile(destFilename, sourceFilename string) error { - if _, err := os.Stat(destFilename); err != nil { +func appendFile(destDir, destFilename, sourceFilename string) error { + if _, err := os.Lstat(destFilename); err != nil { if errors.Is(err, os.ErrNotExist) { // Dest file doesn't exist, so just copy the file. var err error @@ -40,6 +40,14 @@ func appendFile(destFilename, sourceFilename string) error { } return copyFile(destFilename, sourceFilename, mode, false) } + return err + } + // File exists but that can be a symlink and target is dangling, + // or resolved symlink path is outside of destDir, which result in + // writes to wrong location on host. + destFilename, err := resolveSymlinkWithInRoot(destDir, destFilename) + if err != nil { + return err } sourceFile, err := os.Open(sourceFilename) if err != nil { @@ -51,7 +59,7 @@ func appendFile(destFilename, sourceFilename string) error { } func appendTree(destDir, sourceDir string, - appendFunc func(dest, src string) error) error { + appendFunc func(destDir, dest, src string) error) error { return filepath.WalkDir(sourceDir, func(path string, d fs.DirEntry, err error) error { if err != nil { @@ -65,13 +73,20 @@ func appendTree(destDir, sourceDir string, fileType := d.Type() switch { case fileType.IsDir(): + // Reject if the directory is a symlink and targetPath exists + // outside of destDir. + _, err := resolveSymlinkWithInRoot(destDir, + destFilename) + if err != nil { + return err + } // If path is a directory, create directory and return. // WalkDir will automatically visit the children next. if err := os.MkdirAll(destFilename, DirPerms); err != nil { return err } case fileType.IsRegular(): - if err := appendFunc(destFilename, path); err != nil { + if err := appendFunc(destDir, destFilename, path); err != nil { return err } case fileType&fs.ModeSymlink != 0: diff --git a/lib/fsutil/append_test.go b/lib/fsutil/append_test.go index 564ba25f..a30886d5 100644 --- a/lib/fsutil/append_test.go +++ b/lib/fsutil/append_test.go @@ -29,21 +29,20 @@ func createBaseDirectory(t *testing.T, path string, perms os.FileMode) { } func TestAppendFileNonExistingDestFile(t *testing.T) { - const ( - sourceFileName = "dir1/source" - destFileName = "dir2/dir3/dest" - ) // setup source file. tmp := t.TempDir() var ( + sourceDir = filepath.Join(tmp, "source/dir1") + destDir = filepath.Join(tmp, "dest/dir2/dir3") + filename = "test.txt" sourceFileData = []byte( "#/usr/bin/bash\nVAR1=$(which bash)\necho $VAR1\nthis is \n\ttest data\n", ) expectedDestFileData = sourceFileData filePerms os.FileMode = 0600 ) - sourceFilePath := filepath.Join(tmp, sourceFileName) - destFilePath := filepath.Join(tmp, destFileName) + sourceFilePath := filepath.Join(sourceDir, filename) + destFilePath := filepath.Join(destDir, filename) createBaseDirectory(t, sourceFilePath, 0755) // create source file with data. if err := copyToFile(sourceFilePath, 0600, @@ -61,14 +60,11 @@ func TestAppendFileNonExistingDestFile(t *testing.T) { filepath.Dir(sourceFilePath)); err != nil { t.Fatalf("error appending to file: %s\n", err.Error()) } - // Since Destination file is not present, the entire Tree will be - // created with source file. - finalDestPath := filepath.Join(tmp, "dir2/dir3/source") - f, _ := os.OpenFile(finalDestPath, os.O_RDONLY, 0) + f, _ := os.OpenFile(destFilePath, os.O_RDONLY, 0) d, _ := io.ReadAll(f) t.Logf("file content is \n%s\n", string(d)) // check file perm of dest, it should be same as source. - mode, err := getFilePerms(finalDestPath) + mode, err := getFilePerms(destFilePath) if err != nil { t.Fatalf("error getting dest file perms %s\n", err.Error()) } @@ -80,7 +76,7 @@ func TestAppendFileNonExistingDestFile(t *testing.T) { ) } // dest file should exist. - same, err := CompareFile(expectedDestFileData, finalDestPath) + same, err := CompareFile(expectedDestFileData, destFilePath) if err != nil { t.Fatalf("error appending to file: %s\n", err.Error()) } @@ -91,20 +87,21 @@ func TestAppendFileNonExistingDestFile(t *testing.T) { func TestAppendFileWithExistingDestFile(t *testing.T) { // setup source file. - const ( - sourceFileName = "dir1/dir2/dir3/source" - destFileName = "dir4/dir2/dir3/dest" - ) tmp := t.TempDir() var ( + sourceDir = filepath.Join(tmp, "source/dir1") + destDir = filepath.Join(tmp, "dest/dir2/dir3") + filename = "test.txt" sourceFileData = []byte( "#/usr/bin/bash\nVAR1=$(which bash)\necho $VAR1\nthis is \n\ttest data\n", ) - destFileData = []byte("#/usr/bin/python\necho 'this is test data'\n") + destFileData = []byte( + "#/usr/bin/python\necho 'this is test data'\n", + ) expectedDestFileData = append(destFileData, sourceFileData...) ) - sourceFilePath := filepath.Join(tmp, sourceFileName) - destFilePath := filepath.Join(tmp, destFileName) + sourceFilePath := filepath.Join(sourceDir, filename) + destFilePath := filepath.Join(destDir, filename) createBaseDirectory(t, sourceFilePath, 0755) createBaseDirectory(t, destFilePath, 0755) // create source file with data. @@ -126,7 +123,8 @@ func TestAppendFileWithExistingDestFile(t *testing.T) { ); err != nil { t.Fatalf("error creating dest file %s: %s\n", destFilePath, err.Error()) } - if err := AppendFile(destFilePath, sourceFilePath); err != nil { + if err := AppendTree(filepath.Dir(destFilePath), + filepath.Dir(sourceFilePath)); err != nil { t.Fatalf("error appending to file: %s\n", err.Error()) } f, _ := os.OpenFile(destFilePath, os.O_RDONLY, 0) diff --git a/lib/fsutil/readSymlink.go b/lib/fsutil/readSymlink.go new file mode 100644 index 00000000..c531cd6d --- /dev/null +++ b/lib/fsutil/readSymlink.go @@ -0,0 +1,68 @@ +package fsutil + +import ( + "errors" + "fmt" + "os" + "path/filepath" + "strings" +) + +// ResolveSymlinkTargetPath will convert target path of a symlink +// into clean absolute path. +// For example symlink +// /etc/resolv.conf -> ../run/systemd/resolve/stubd-resolv.conf +// will return absolute path /run/systemd/resolve/stubd-resolv.conf. +// If the given file path is not a symlink, we will return error +// fsutil.ErrNotASymlink. +func resolveSymlinkTargetPath(path string) (string, error) { + fileInfo, err := os.Lstat(path) + if err != nil { + if errors.Is(err, os.ErrNotExist) { + return path, nil + } + return "", err + } + if fileInfo.Mode()&os.ModeSymlink == 0 { + return path, ErrNotASymlink + } + targetPath, err := os.Readlink(path) + if err != nil { + return "", err + } + if filepath.IsAbs(targetPath) { + return targetPath, nil + } + return filepath.Clean( + filepath.Join(filepath.Dir(path), targetPath), + ), nil +} + +// ResolveSymlinkWithInRoot resolves the target of the symlink at path, +// guarantees that the resolved path stays within the root. +// If the given file path is not a symlink, we will return error +// fsutil.ErrNotASymlink. +func resolveSymlinkWithInRoot(root, path string) (string, error) { + resolvedTargetPath, err := resolveSymlinkTargetPath(path) + if err != nil { + if errors.Is(err, ErrNotASymlink) { + return path, nil + } + return "", err + } + rel, err := filepath.Rel(root, resolvedTargetPath) + if err != nil { + return "", fmt.Errorf( + "compute relative path of %q in %q: %w", + resolvedTargetPath, root, err, + ) + } + targetPath, _ := os.Readlink(path) + if rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) { + return "", fmt.Errorf( + "symlink %q target %q escapes root %q (resolved=%q)", + path, targetPath, root, resolvedTargetPath, + ) + } + return resolvedTargetPath, nil +} From 0aaef225a0273a665a24b7a7819dd676df40f2d5 Mon Sep 17 00:00:00 2001 From: Narendra Reddy Date: Tue, 28 Apr 2026 01:32:28 +0530 Subject: [PATCH 03/16] imaginator: enhance evalsymlinks method --- lib/fsutil/api.go | 1 - lib/fsutil/append_test.go | 1 + lib/fsutil/readSymlink.go | 43 ++++++++++----------------------------- 3 files changed, 12 insertions(+), 33 deletions(-) diff --git a/lib/fsutil/api.go b/lib/fsutil/api.go index e213fddf..9426b8be 100644 --- a/lib/fsutil/api.go +++ b/lib/fsutil/api.go @@ -21,7 +21,6 @@ const ( var ( ErrorChecksumMismatch = errors.New("checksum mismatch") - ErrNotASymlink = errors.New("path is not a symlink") ) // AppendFile will append data from the sourceFilename to destFilename. diff --git a/lib/fsutil/append_test.go b/lib/fsutil/append_test.go index a30886d5..4055cdec 100644 --- a/lib/fsutil/append_test.go +++ b/lib/fsutil/append_test.go @@ -18,6 +18,7 @@ func createBaseDirectory(t *testing.T, path string, perms os.FileMode) { if !info.IsDir() { t.Fatalf("path exists but is a file: %s", dir) } + return } if !os.IsNotExist(err) { t.Fatal(err.Error()) diff --git a/lib/fsutil/readSymlink.go b/lib/fsutil/readSymlink.go index c531cd6d..201822de 100644 --- a/lib/fsutil/readSymlink.go +++ b/lib/fsutil/readSymlink.go @@ -8,48 +8,27 @@ import ( "strings" ) -// ResolveSymlinkTargetPath will convert target path of a symlink -// into clean absolute path. -// For example symlink -// /etc/resolv.conf -> ../run/systemd/resolve/stubd-resolv.conf -// will return absolute path /run/systemd/resolve/stubd-resolv.conf. -// If the given file path is not a symlink, we will return error -// fsutil.ErrNotASymlink. -func resolveSymlinkTargetPath(path string) (string, error) { +// ResolveSymlinkWithInRoot resolves the symlink at path, following the entire +// chain and guarantees the resolved path stays within root. If the path is not +// a symlink ( or does not exist ), it is returned unchanged. +// A dangling symlink or a symlink chain whose final target escapes root +// returns an error. +func resolveSymlinkWithInRoot(root, path string) (string, error) { fileInfo, err := os.Lstat(path) if err != nil { if errors.Is(err, os.ErrNotExist) { return path, nil } - return "", err } if fileInfo.Mode()&os.ModeSymlink == 0 { - return path, ErrNotASymlink + return path, nil } - targetPath, err := os.Readlink(path) + // Resolve the entire symlink chain. EvalSymlinks returns an error + // for danling symlinks and symlink loops. + resolvedTargetPath, err := filepath.EvalSymlinks(path) if err != nil { return "", err } - if filepath.IsAbs(targetPath) { - return targetPath, nil - } - return filepath.Clean( - filepath.Join(filepath.Dir(path), targetPath), - ), nil -} - -// ResolveSymlinkWithInRoot resolves the target of the symlink at path, -// guarantees that the resolved path stays within the root. -// If the given file path is not a symlink, we will return error -// fsutil.ErrNotASymlink. -func resolveSymlinkWithInRoot(root, path string) (string, error) { - resolvedTargetPath, err := resolveSymlinkTargetPath(path) - if err != nil { - if errors.Is(err, ErrNotASymlink) { - return path, nil - } - return "", err - } rel, err := filepath.Rel(root, resolvedTargetPath) if err != nil { return "", fmt.Errorf( @@ -57,8 +36,8 @@ func resolveSymlinkWithInRoot(root, path string) (string, error) { resolvedTargetPath, root, err, ) } - targetPath, _ := os.Readlink(path) if rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) { + targetPath, _ := os.Readlink(path) return "", fmt.Errorf( "symlink %q target %q escapes root %q (resolved=%q)", path, targetPath, root, resolvedTargetPath, From fb2dc27f0f6e1267c6e905e589f4b76c85a15867 Mon Sep 17 00:00:00 2001 From: Narendra Reddy Date: Tue, 28 Apr 2026 01:39:35 +0530 Subject: [PATCH 04/16] imaginator: fix errors --- lib/fsutil/readSymlink.go | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/fsutil/readSymlink.go b/lib/fsutil/readSymlink.go index 201822de..a1394e0a 100644 --- a/lib/fsutil/readSymlink.go +++ b/lib/fsutil/readSymlink.go @@ -19,6 +19,7 @@ func resolveSymlinkWithInRoot(root, path string) (string, error) { if errors.Is(err, os.ErrNotExist) { return path, nil } + return "", err } if fileInfo.Mode()&os.ModeSymlink == 0 { return path, nil From 3dfd2057757ca54d2f816b544dd7ffb4d5870bd6 Mon Sep 17 00:00:00 2001 From: Narendra Reddy Date: Tue, 28 Apr 2026 02:02:11 +0530 Subject: [PATCH 05/16] imaginator: improve AppendTree method doc --- lib/fsutil/api.go | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/lib/fsutil/api.go b/lib/fsutil/api.go index 9426b8be..d1134078 100644 --- a/lib/fsutil/api.go +++ b/lib/fsutil/api.go @@ -33,17 +33,13 @@ func AppendFile(destDir, destFilename, sourceFilename string) error { } // AppendTree recursively merges sourceDir into destDir. -// It appends contents to existing files or copies new ones -// while preserving permissions. -// Directory structures are mirrored. -// Returns an error if symlinks or non-regular files -// are encountered in sourceDir. -// If destination path is a symlink, behavior is as follows -// 1. If symlink points to a dangling target, it fails with error. -// 2. If symlink target resolves to a path outside destDir, -// it fails with error. -// 3. If symlink points to a valid path inside destDir, content will be -// appended to target file. +// Existing regular files will have data appended. Files which do not exist in +// destDir will be copied with the source file permissions. +// Directory structures will be mirrored. An error is returned if symilinks or +// non-regular files are encountered in sourceDir. If a destination path is a +// symlink, it must resolve to an existing location within destDir. +// Dangling symlinks, or a symilinks that resolve outside destDir, +// cause an error. func AppendTree(destDir, sourceDir string) error { return appendTree(destDir, sourceDir, AppendFile) } From 1e9e8626acfd9ca723248d1b6cdf4e622e5058c9 Mon Sep 17 00:00:00 2001 From: Narendra Reddy Date: Tue, 28 Apr 2026 02:08:18 +0530 Subject: [PATCH 06/16] Revert "dominator: add golangci-lint file and pre-commit-config for linting and formatting" This reverts commit a17ccda07782073902de863b446dd3ceb0661061. --- .golangci.yml | 69 ----------------------------------------- .pre-commit-config.yaml | 54 -------------------------------- 2 files changed, 123 deletions(-) delete mode 100644 .golangci.yml delete mode 100644 .pre-commit-config.yaml diff --git a/.golangci.yml b/.golangci.yml deleted file mode 100644 index 192a6e73..00000000 --- a/.golangci.yml +++ /dev/null @@ -1,69 +0,0 @@ -version: "2" - -run: - timeout: 10m - tests: true - relative-path-mode: gomod - -linters: - enable: - - errcheck - - govet - - ineffassign - - lll - - misspell - - revive - - staticcheck - - unconvert - - unused - - godot - - settings: - lll: - line-length: 80 - tab-width: 1 - misspell: - locale: US - godot: - scope: all - period: true - exclude: - - '^go:' - - '^\+build' - - '^(?i)(todo|fixme|nolint):' - errcheck: - # This fixes the issue you had where fmt.Fprintf was failing the lint - exclude-functions: - - fmt.Fprintf - - fmt.Printf - - fmt.Println - - fmt.Fprint - - # These rules ensure lll doesn't annoy you on imports or URLs - exclusions: - rules: - - linters: [lll] - source: '^\s*"github\.com/Cloud-Foundations/(Dominator|tricoder)/.*"$' - - linters: [lll] - source: 'https?://' - - linters: [lll] - source: '^\s*//go:generate ' - - linters: [lll] - path: '(^|/)testdata/' - - linters: [errcheck] - source: '^\s*defer .*\.Close\(\)$' - - linters: [revive] - path: '.*_test\.go$' - -formatters: - enable: - - gofmt - - goimports - - golines - settings: - golines: - max-len: 80 - shorten-comments: true - no-chain-split-dots: true - goimports: - local-prefixes: github.com diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml deleted file mode 100644 index 51fb3182..00000000 --- a/.pre-commit-config.yaml +++ /dev/null @@ -1,54 +0,0 @@ -repos: - - repo: https://github.com/golangci/golangci-lint - rev: v2.0.0 # MUST be v2+ to read the config file above - hooks: - # 1. RUN FIRST: Dedicated Line Length Check - # This ensures you always see length issues even if logic errors exist. - - id: golangci-lint - name: check-line-length - entry: bash -c 'GOOS=linux golangci-lint run --new-from-rev=master --timeout=5m --enable-only=lll' - pass_filenames: false - - # 2. RUN SECOND: All other linters - - id: golangci-lint - name: lint-my-changes - entry: bash -c 'GOOS=linux golangci-lint run --new-from-rev=master --timeout=5m --disable lll' - pass_filenames: false - - - repo: local - hooks: - - id: clean-go-code - name: fix-blank-lines-and-imports - language: system - files: \.go$ - pass_filenames: true - entry: | - bash -c ' - # 0. Ensure binaries are in PATH and install if missing - export PATH=$PATH:$(go env GOPATH)/bin - - if ! which goimports-reviser &> /dev/null; then - echo "Installing goimports-reviser..." - go install github.com/incu6us/goimports-reviser/v3@latest - fi - - if ! which golines &> /dev/null; then - echo "Installing golines..." - go install ://github.com - fi - - for file in "$@"; do - # 1. DENSITY PASS: Remove internal blank lines - perl -i -0777 -pe "s/\{(\s*\n){2,}/{\n/g; s/(\n\s*\n)+\s*\}/\n\}/g; s/(\n\s+[^\n]+)(\n\s*\n)+/\1\n/g" "$file" - - # 2. ORDERING PASS: 3-group layout (StdLib, 3rd-Party, Local) - # Use the actual module prefix for the company group - goimports-reviser -rm-unused -format -project-name "://github.com" "$file" - - # 3. Format using golines from golangci-lint - golangci-lint fmt "$file" - - # 4. FINAL PASS: Canonical formatting - gofmt -w "$file" - git add "$file" - done' -- From f3c395234634b7178be5a6ff33d48c35e128146f Mon Sep 17 00:00:00 2001 From: Narendra Reddy Date: Thu, 30 Apr 2026 13:25:51 +0530 Subject: [PATCH 07/16] imaginator: enhance append logic for multihop dest symlinks with relative and absolute targets --- lib/fsutil/append_test.go | 247 ++++++++++++++++++++++++++++++++--- lib/fsutil/readSymlink.go | 48 ------- lib/fsutil/resolveSymlink.go | 68 ++++++++++ 3 files changed, 299 insertions(+), 64 deletions(-) delete mode 100644 lib/fsutil/readSymlink.go create mode 100644 lib/fsutil/resolveSymlink.go diff --git a/lib/fsutil/append_test.go b/lib/fsutil/append_test.go index 4055cdec..391e3e5c 100644 --- a/lib/fsutil/append_test.go +++ b/lib/fsutil/append_test.go @@ -3,9 +3,11 @@ package fsutil import ( "bytes" "errors" + "fmt" "io" "os" "path/filepath" + "strings" "testing" ) @@ -30,11 +32,12 @@ func createBaseDirectory(t *testing.T, path string, perms os.FileMode) { } func TestAppendFileNonExistingDestFile(t *testing.T) { - // setup source file. - tmp := t.TempDir() + // Setup source file. + sourceTmp := t.TempDir() + destTmp := t.TempDir() var ( - sourceDir = filepath.Join(tmp, "source/dir1") - destDir = filepath.Join(tmp, "dest/dir2/dir3") + sourceDir = filepath.Join(sourceTmp, "source/dir1") + destDir = filepath.Join(destTmp, "dest/dir2/dir3") filename = "test.txt" sourceFileData = []byte( "#/usr/bin/bash\nVAR1=$(which bash)\necho $VAR1\nthis is \n\ttest data\n", @@ -45,14 +48,14 @@ func TestAppendFileNonExistingDestFile(t *testing.T) { sourceFilePath := filepath.Join(sourceDir, filename) destFilePath := filepath.Join(destDir, filename) createBaseDirectory(t, sourceFilePath, 0755) - // create source file with data. + // Create source file with data. if err := copyToFile(sourceFilePath, 0600, bytes.NewReader(sourceFileData), 0); err != nil { t.Fatalf("error creating source file %s: %s\n", sourceFilePath, err.Error()) } - // skipping creation of dest file path. - // check dest file doesn't exist before append. + // Skipping creation of dest file path. + // Check dest file doesn't exist before append. _, err := os.Stat(destFilePath) if err == nil || !errors.Is(err, os.ErrNotExist) { t.Fatal("destfile exists already\n") @@ -64,7 +67,7 @@ func TestAppendFileNonExistingDestFile(t *testing.T) { f, _ := os.OpenFile(destFilePath, os.O_RDONLY, 0) d, _ := io.ReadAll(f) t.Logf("file content is \n%s\n", string(d)) - // check file perm of dest, it should be same as source. + // Check file perm of dest, it should be same as source. mode, err := getFilePerms(destFilePath) if err != nil { t.Fatalf("error getting dest file perms %s\n", err.Error()) @@ -76,7 +79,7 @@ func TestAppendFileNonExistingDestFile(t *testing.T) { filePerms, ) } - // dest file should exist. + // Dest file should exist. same, err := CompareFile(expectedDestFileData, destFilePath) if err != nil { t.Fatalf("error appending to file: %s\n", err.Error()) @@ -87,11 +90,12 @@ func TestAppendFileNonExistingDestFile(t *testing.T) { } func TestAppendFileWithExistingDestFile(t *testing.T) { - // setup source file. - tmp := t.TempDir() + // Setup source file. + sourceTmp := t.TempDir() + destTmp := t.TempDir() var ( - sourceDir = filepath.Join(tmp, "source/dir1") - destDir = filepath.Join(tmp, "dest/dir2/dir3") + sourceDir = filepath.Join(sourceTmp, "source/dir1") + destDir = filepath.Join(destTmp, "dest/dir2/dir3") filename = "test.txt" sourceFileData = []byte( "#/usr/bin/bash\nVAR1=$(which bash)\necho $VAR1\nthis is \n\ttest data\n", @@ -105,7 +109,7 @@ func TestAppendFileWithExistingDestFile(t *testing.T) { destFilePath := filepath.Join(destDir, filename) createBaseDirectory(t, sourceFilePath, 0755) createBaseDirectory(t, destFilePath, 0755) - // create source file with data. + // Create source file with data. if err := copyToFile( sourceFilePath, PublicFilePerms, @@ -115,7 +119,7 @@ func TestAppendFileWithExistingDestFile(t *testing.T) { t.Fatalf("error creating source file %s: %s\n", sourceFilePath, err.Error()) } - // create dest file with data. + // Create dest file with data. if err := copyToFile( destFilePath, PublicFilePerms, @@ -131,7 +135,7 @@ func TestAppendFileWithExistingDestFile(t *testing.T) { f, _ := os.OpenFile(destFilePath, os.O_RDONLY, 0) d, _ := io.ReadAll(f) t.Logf("file content is \n%s\n", string(d)) - // dest file should exist. + // Dest file should exist. same, err := CompareFile(expectedDestFileData, destFilePath) if err != nil { t.Fatalf("error appending to file: %s\n", err.Error()) @@ -140,3 +144,214 @@ func TestAppendFileWithExistingDestFile(t *testing.T) { t.Fatalf("contents mismatch after append") } } + +func TestAppendFileWithDanglingDestSymlinks(t *testing.T) { + sourceTmp := t.TempDir() + destTmp := t.TempDir() + var ( + sourceDir = filepath.Join(sourceTmp, "source/dir1") + destDir = filepath.Join(destTmp, "dest/dir2/dir3") + filename = "test.txt" + sourceFilePath = filepath.Join(sourceDir, filename) + destFilePath = filepath.Join(destDir, filename) + danglingSymlinkPath = "../run/systemd/test-service.txt" + ) + createBaseDirectory(t, sourceFilePath, 0755) + createBaseDirectory(t, destFilePath, 0755) + // Create source file with data. + if err := copyToFile( + sourceFilePath, + PublicFilePerms, + bytes.NewReader([]byte{}), + 0, + ); err != nil { + t.Fatalf("error creating source file %s: %s\n", + sourceFilePath, err.Error()) + } + // Setup dangling symlink at destFile. + if err := os.Symlink(danglingSymlinkPath, destFilePath); err != nil { + t.Fatalf("error creating dangling symlink: %s", err) + } + err := AppendTree(filepath.Dir(destFilePath), filepath.Dir(sourceFilePath)) + if err == nil { + t.Fatalf("expected error for dangling symlinks") + } + fmt.Println(err.Error()) + if !strings.EqualFold(err.Error(), + fmt.Sprintf( + "dangling symlink: %q resolves to missing target %q", + destFilePath, filepath.Clean( + filepath.Join( + filepath.Dir(destFilePath), danglingSymlinkPath, + ), + ), + ), + ) { + t.Fatalf("unexpected error") + } +} + +func TestAppendFileWithEscapingTargetSymlinks(t *testing.T) { + sourceTmp := t.TempDir() + destTmp := t.TempDir() + var ( + sourceDir = filepath.Join(sourceTmp, "etc/config") + destDir = filepath.Join(destTmp, "etc/config") + filename = "test.txt" + sourceFilePath = filepath.Join(sourceDir, filename) + destFilePath = filepath.Join(destDir, filename) + danglingSymlinkPath = "../../../run/systemd/test-service.txt" + ) + createBaseDirectory(t, sourceFilePath, 0755) + createBaseDirectory(t, destFilePath, 0755) + symlinkTargetFullPath := filepath.Clean( + filepath.Join(destDir, danglingSymlinkPath), + ) + createBaseDirectory(t, symlinkTargetFullPath, 0755) + // Create source file with data. + if err := copyToFile( + sourceFilePath, + PublicFilePerms, + bytes.NewReader([]byte{}), + 0, + ); err != nil { + t.Fatalf("error creating source file %s: %s\n", + sourceFilePath, err.Error()) + } + if err := copyToFile( + symlinkTargetFullPath, + PublicFilePerms, + bytes.NewReader([]byte{}), + 0, + ); err != nil { + t.Fatalf("error creating symlink target file %s: %s\n", + symlinkTargetFullPath, err.Error()) + } + if err := os.Symlink(danglingSymlinkPath, destFilePath); err != nil { + t.Fatalf("error creating dangling symlink: %s", err) + } + err := AppendTree(destTmp, sourceTmp) + if err == nil { + t.Fatalf("expected error for dangling symlinks") + } + relSymlinkDir, _ := filepath.Rel(destTmp, filepath.Dir(destFilePath)) + expectedEvaluatedPath := filepath.Clean( + filepath.Join(relSymlinkDir, danglingSymlinkPath), + ) + expectedErr := fmt.Sprintf( + "path %q evaluates to %q which escapes root %q", + destFilePath, + expectedEvaluatedPath, + destTmp, + ) + if !strings.EqualFold(err.Error(), expectedErr) { + t.Fatalf("unexpected error.\nGot: %s\nExpected: %s", + err.Error(), expectedErr, + ) + } +} + +func TestAppendFileWithExistingTargetSymlinks(t *testing.T) { + var ( + filename = "test.txt" + symlinkPaths = map[string]string{ + "relPathTest": "../../run/systemd/new-rel-test-file.txt", + "absPathTest": "/var/run/systemd/new-abs-test-file.txt", + } + sourceData = []byte("This is from source\n") + destData = []byte( + "#/usr/bin/bash\necho 'hello world'\n\tThis is from symlink", + ) + expectedData = append(destData, sourceData...) + ) + for name, symlinkPath := range symlinkPaths { + t.Run(name, func(t *testing.T) { + sourceTmp := t.TempDir() + destTmp := t.TempDir() + var ( + sourceDir = filepath.Join(sourceTmp, "etc/config") + destDir = filepath.Join(destTmp, "etc/config") + sourceFilePath = filepath.Join(sourceDir, filename) + destFilePath = filepath.Join(destDir, filename) + ) + createBaseDirectory(t, sourceFilePath, 0755) + createBaseDirectory(t, destFilePath, 0755) + var rootDir string + if !filepath.IsAbs(symlinkPath) { + rootDir = destDir + } else { + rootDir = destTmp + } + symlinkTargetFullPath := filepath.Clean( + filepath.Join(rootDir, symlinkPath), + ) + createBaseDirectory(t, symlinkTargetFullPath, 0755) + // Create source file with data. + if err := copyToFile( + sourceFilePath, + PublicFilePerms, + bytes.NewReader(sourceData), + 0, + ); err != nil { + t.Fatalf("error creating source file %s: %s\n", + sourceFilePath, err.Error()) + } + // Create symlink target path. + if err := copyToFile( + symlinkTargetFullPath, + PublicFilePerms, + bytes.NewReader(destData), + 0, + ); err != nil { + t.Fatalf("error creating symlink target file %s: %s\n", + symlinkTargetFullPath, err.Error()) + } + // Create symlink for destFilePath to targetPath. + if err := os.Symlink(symlinkPath, destFilePath); err != nil { + t.Fatalf("error creating dangling symlink: %s", err) + } + err := AppendTree(destTmp, sourceTmp) + if err != nil { + t.Fatalf("unexpected error in appendTree: %s", err) + } + // If symlink target is absolute, we need to append rootDir + // for validating data. + var expectedEvaluatedPath string + if !filepath.IsAbs(symlinkPath) { + expectedEvaluatedPath = destFilePath + } else { + expectedEvaluatedPath = symlinkTargetFullPath + } + // Check if contents match. + f, err := os.OpenFile(expectedEvaluatedPath, os.O_RDONLY, 0) + if err != nil { + t.Fatalf("error opening expected file: %s", err) + } + d, err := io.ReadAll(f) + if err != nil { + t.Fatalf("error reading file contents: %s", err) + } + t.Log("file contents is", string(d)) + same, err := compareFile(expectedData, expectedEvaluatedPath) + if err != nil { + t.Fatalf("error comparing to file: %s\n", err.Error()) + } + if !same { + t.Fatalf("contents mismatch after append") + } + // Check if symlink stays intact. + linkPath, err := os.Readlink(destFilePath) + if err != nil { + t.Fatalf( + "error checking target %s of symlink %s: %s", + destFilePath, + symlinkPath, + err, + ) + } + if linkPath != symlinkPath { + t.Fatalf("symlink targets don't match") + } + }) + } +} diff --git a/lib/fsutil/readSymlink.go b/lib/fsutil/readSymlink.go deleted file mode 100644 index a1394e0a..00000000 --- a/lib/fsutil/readSymlink.go +++ /dev/null @@ -1,48 +0,0 @@ -package fsutil - -import ( - "errors" - "fmt" - "os" - "path/filepath" - "strings" -) - -// ResolveSymlinkWithInRoot resolves the symlink at path, following the entire -// chain and guarantees the resolved path stays within root. If the path is not -// a symlink ( or does not exist ), it is returned unchanged. -// A dangling symlink or a symlink chain whose final target escapes root -// returns an error. -func resolveSymlinkWithInRoot(root, path string) (string, error) { - fileInfo, err := os.Lstat(path) - if err != nil { - if errors.Is(err, os.ErrNotExist) { - return path, nil - } - return "", err - } - if fileInfo.Mode()&os.ModeSymlink == 0 { - return path, nil - } - // Resolve the entire symlink chain. EvalSymlinks returns an error - // for danling symlinks and symlink loops. - resolvedTargetPath, err := filepath.EvalSymlinks(path) - if err != nil { - return "", err - } - rel, err := filepath.Rel(root, resolvedTargetPath) - if err != nil { - return "", fmt.Errorf( - "compute relative path of %q in %q: %w", - resolvedTargetPath, root, err, - ) - } - if rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) { - targetPath, _ := os.Readlink(path) - return "", fmt.Errorf( - "symlink %q target %q escapes root %q (resolved=%q)", - path, targetPath, root, resolvedTargetPath, - ) - } - return resolvedTargetPath, nil -} diff --git a/lib/fsutil/resolveSymlink.go b/lib/fsutil/resolveSymlink.go new file mode 100644 index 00000000..92f68645 --- /dev/null +++ b/lib/fsutil/resolveSymlink.go @@ -0,0 +1,68 @@ +package fsutil + +import ( + "errors" + "fmt" + "os" + "path/filepath" + "strings" +) + +// resolveSymlinkWithInRoot resolves the symlink at path, following the entire +// chain and guarantees the resolved path stays within root. If the path is not +// a symlink (or does not exist), it is returned unchanged. +// A dangling symlink or a symlink chain whose final target escapes root +// returns an error. +func resolveSymlinkWithInRoot(root, path string) (string, error) { + const maxLinks = 255 + sep := string(filepath.Separator) + root = filepath.Clean(root) + path = filepath.Clean(path) + rel, err := filepath.Rel(root, path) + if err != nil { + return "", fmt.Errorf("relative path of %q in %q: %w", path, root, err) + } + curr := rel + for nlinks := 0; nlinks <= maxLinks; nlinks++ { + hostCurr := filepath.Join(root, curr) + info, err := os.Lstat(hostCurr) + if err != nil { + if errors.Is(err, os.ErrNotExist) { + if nlinks == 0 { + return hostCurr, nil + } + return "", + fmt.Errorf( + "dangling symlink: %q resolves to missing target %q", + path, + hostCurr, + ) + } + return "", fmt.Errorf("lstat %q: %w", hostCurr, err) + } + // We only enforce the escape boundary, + // IF the file physically exists on the disk. + if curr == ".." || strings.HasPrefix(curr, ".."+sep) { + return "", fmt.Errorf( + "path %q evaluates to %q which escapes root %q", + path, curr, root, + ) + } + // If it's not a symlink, we've found our final destination. + if info.Mode()&os.ModeSymlink == 0 { + return hostCurr, nil + } + // Read the symlink target. + target, err := os.Readlink(hostCurr) + if err != nil { + return "", err + } + if filepath.IsAbs(target) { + volLen := len(filepath.VolumeName(target)) + curr = filepath.Clean(strings.TrimPrefix(target[volLen:], sep)) + } else { + curr = filepath.Clean(filepath.Join(filepath.Dir(curr), target)) + } + } + return "", errors.New("too many symlinks (loop detected)") +} From 7222df5762f7521ee097b7c42db6bd24b39a54f4 Mon Sep 17 00:00:00 2001 From: Narendra Reddy Date: Thu, 30 Apr 2026 16:31:38 +0530 Subject: [PATCH 08/16] imaginator: clamp relative paths to root similar to kernel chroot style --- lib/fsutil/append_test.go | 158 ++++++++++++++++++++++++----------- lib/fsutil/resolveSymlink.go | 35 ++++---- 2 files changed, 131 insertions(+), 62 deletions(-) diff --git a/lib/fsutil/append_test.go b/lib/fsutil/append_test.go index 391e3e5c..896d87e6 100644 --- a/lib/fsutil/append_test.go +++ b/lib/fsutil/append_test.go @@ -149,13 +149,44 @@ func TestAppendFileWithDanglingDestSymlinks(t *testing.T) { sourceTmp := t.TempDir() destTmp := t.TempDir() var ( - sourceDir = filepath.Join(sourceTmp, "source/dir1") - destDir = filepath.Join(destTmp, "dest/dir2/dir3") - filename = "test.txt" - sourceFilePath = filepath.Join(sourceDir, filename) - destFilePath = filepath.Join(destDir, filename) - danglingSymlinkPath = "../run/systemd/test-service.txt" + sourceDir = filepath.Join(sourceTmp, "etc/config") + destDir = filepath.Join(destTmp, "etc/config") + filename = "test.txt" + sourceFilePath = filepath.Join(sourceDir, filename) + destFilePath = filepath.Join(destDir, filename) ) + danglingSymlinkPaths := + []struct { + name, + danglingSymlinkPath, + // Resolved path in root with clamping. + resolvedDanglingSymlinkPath string + }{ + { + name: "relative_path_1", + danglingSymlinkPath: "../run/systemd/test-service.txt", + resolvedDanglingSymlinkPath: filepath.Join( + destTmp, + "etc/run/systemd/test-service.txt", + ), + }, + { + name: "relative_path_2", + danglingSymlinkPath: "../../run/systemd/test-service.txt", + resolvedDanglingSymlinkPath: filepath.Join( + destTmp, + "run/systemd/test-service.txt", + ), + }, + { + name: "relative_path_clamp_root", + danglingSymlinkPath: "../../../../../../run/systemd/test-service.txt", + resolvedDanglingSymlinkPath: filepath.Join( + destTmp, + "run/systemd/test-service.txt", + ), + }, + } createBaseDirectory(t, sourceFilePath, 0755) createBaseDirectory(t, destFilePath, 0755) // Create source file with data. @@ -168,51 +199,70 @@ func TestAppendFileWithDanglingDestSymlinks(t *testing.T) { t.Fatalf("error creating source file %s: %s\n", sourceFilePath, err.Error()) } - // Setup dangling symlink at destFile. - if err := os.Symlink(danglingSymlinkPath, destFilePath); err != nil { - t.Fatalf("error creating dangling symlink: %s", err) - } - err := AppendTree(filepath.Dir(destFilePath), filepath.Dir(sourceFilePath)) - if err == nil { - t.Fatalf("expected error for dangling symlinks") - } - fmt.Println(err.Error()) - if !strings.EqualFold(err.Error(), - fmt.Sprintf( - "dangling symlink: %q resolves to missing target %q", - destFilePath, filepath.Clean( - filepath.Join( - filepath.Dir(destFilePath), danglingSymlinkPath, + for _, danglingSymlink := range danglingSymlinkPaths { + t.Run(danglingSymlink.name, func(t *testing.T) { + // Setup dangling symlink at destFile. + if err := os.Symlink(danglingSymlink.danglingSymlinkPath, + destFilePath); err != nil { + t.Fatalf("error creating dangling symlink: %s", err) + } + defer func() { + err := os.Remove(destFilePath) + t.Fatalf("error removing symlink: %s", err) + }() + err := AppendTree(destTmp, sourceTmp) + if err == nil { + t.Fatalf("expected error for dangling symlinks") + } + fmt.Println(err.Error()) + if !strings.EqualFold(err.Error(), + fmt.Sprintf( + "dangling symlink: %q resolves to missing target %q", + destFilePath, + danglingSymlink.resolvedDanglingSymlinkPath, ), - ), - ), - ) { - t.Fatalf("unexpected error") + ) { + t.Fatalf("unexpected error") + } + }) } } -func TestAppendFileWithEscapingTargetSymlinks(t *testing.T) { +func TestAppendFileWithClampingTargetSymlinks(t *testing.T) { sourceTmp := t.TempDir() destTmp := t.TempDir() var ( - sourceDir = filepath.Join(sourceTmp, "etc/config") - destDir = filepath.Join(destTmp, "etc/config") - filename = "test.txt" - sourceFilePath = filepath.Join(sourceDir, filename) + sourceDir = filepath.Join(sourceTmp, "etc/config") + destDir = filepath.Join(destTmp, "etc/config") + filename = "test.txt" + sourceFilePath = filepath.Join(sourceDir, filename) + sourceData = []byte( + "#/usr/bin/bash\n\tThis is test data from source\n", + ) + destData = []byte( + "#/usr/bin/bash\n\tThis is test data from dest\n", + ) destFilePath = filepath.Join(destDir, filename) danglingSymlinkPath = "../../../run/systemd/test-service.txt" + expectedData = append(destData, sourceData...) ) createBaseDirectory(t, sourceFilePath, 0755) createBaseDirectory(t, destFilePath, 0755) symlinkTargetFullPath := filepath.Clean( - filepath.Join(destDir, danglingSymlinkPath), + filepath.Join(destDir, + strings.TrimPrefix( + danglingSymlinkPath, + ".."+string(filepath.Separator), + ), + ), ) + fmt.Println(symlinkTargetFullPath) createBaseDirectory(t, symlinkTargetFullPath, 0755) // Create source file with data. if err := copyToFile( sourceFilePath, PublicFilePerms, - bytes.NewReader([]byte{}), + bytes.NewReader(sourceData), 0, ); err != nil { t.Fatalf("error creating source file %s: %s\n", @@ -221,7 +271,7 @@ func TestAppendFileWithEscapingTargetSymlinks(t *testing.T) { if err := copyToFile( symlinkTargetFullPath, PublicFilePerms, - bytes.NewReader([]byte{}), + bytes.NewReader(destData), 0, ); err != nil { t.Fatalf("error creating symlink target file %s: %s\n", @@ -231,24 +281,36 @@ func TestAppendFileWithEscapingTargetSymlinks(t *testing.T) { t.Fatalf("error creating dangling symlink: %s", err) } err := AppendTree(destTmp, sourceTmp) - if err == nil { - t.Fatalf("expected error for dangling symlinks") + if err != nil { + t.Fatalf("unexpected error in AppendTree: %s", err) } - relSymlinkDir, _ := filepath.Rel(destTmp, filepath.Dir(destFilePath)) - expectedEvaluatedPath := filepath.Clean( - filepath.Join(relSymlinkDir, danglingSymlinkPath), - ) - expectedErr := fmt.Sprintf( - "path %q evaluates to %q which escapes root %q", - destFilePath, - expectedEvaluatedPath, - destTmp, - ) - if !strings.EqualFold(err.Error(), expectedErr) { - t.Fatalf("unexpected error.\nGot: %s\nExpected: %s", - err.Error(), expectedErr, + f, err := os.OpenFile(symlinkTargetFullPath, os.O_RDONLY, 0) + if err != nil { + t.Fatalf("error opening %s: %s", symlinkTargetFullPath, err) + } + d, err := io.ReadAll(f) + if err != nil { + t.Fatalf("error reading data from %s: %s", symlinkTargetFullPath, err) + } + t.Logf("file content is \n%s\n", string(d)) + same, err := compareFile(expectedData, symlinkTargetFullPath) + if err != nil { + t.Fatalf("error comparing to file %s: %s", symlinkTargetFullPath, err) + } + if !same { + t.Fatalf( + "mismatched contents, present: %s\nexpected: %s", + expectedData, d, ) } + //Check if symlink is intact. + expectedSymlinkPath, err := os.Readlink(destFilePath) + if err != nil { + t.Fatalf("unexpected error with symlink %s: %s", destFilePath, err) + } + if expectedSymlinkPath != danglingSymlinkPath { + t.Fatalf("symlink is broken.") + } } func TestAppendFileWithExistingTargetSymlinks(t *testing.T) { diff --git a/lib/fsutil/resolveSymlink.go b/lib/fsutil/resolveSymlink.go index 92f68645..368549f0 100644 --- a/lib/fsutil/resolveSymlink.go +++ b/lib/fsutil/resolveSymlink.go @@ -9,10 +9,10 @@ import ( ) // resolveSymlinkWithInRoot resolves the symlink at path, following the entire -// chain and guarantees the resolved path stays within root. If the path is not -// a symlink (or does not exist), it is returned unchanged. -// A dangling symlink or a symlink chain whose final target escapes root -// returns an error. +// chain and guarantees the resolved path stays within root. ".." segments and +// absolute targets are clamped at root (chroot-style semantics), so resolution +// never touches paths outside root.If the path is not a symlink (or does not +// exist), it is returned unchanged. A dangling symlink chain returns an error. func resolveSymlinkWithInRoot(root, path string) (string, error) { const maxLinks = 255 sep := string(filepath.Separator) @@ -22,6 +22,10 @@ func resolveSymlinkWithInRoot(root, path string) (string, error) { if err != nil { return "", fmt.Errorf("relative path of %q in %q: %w", path, root, err) } + //Caller-supplied path must already be inside root; reject before any I/O. + if rel == ".." || strings.HasPrefix(rel, ".."+sep) { + return "", fmt.Errorf("path %q escapes root %q", path, root) + } curr := rel for nlinks := 0; nlinks <= maxLinks; nlinks++ { hostCurr := filepath.Join(root, curr) @@ -40,14 +44,6 @@ func resolveSymlinkWithInRoot(root, path string) (string, error) { } return "", fmt.Errorf("lstat %q: %w", hostCurr, err) } - // We only enforce the escape boundary, - // IF the file physically exists on the disk. - if curr == ".." || strings.HasPrefix(curr, ".."+sep) { - return "", fmt.Errorf( - "path %q evaluates to %q which escapes root %q", - path, curr, root, - ) - } // If it's not a symlink, we've found our final destination. if info.Mode()&os.ModeSymlink == 0 { return hostCurr, nil @@ -57,11 +53,22 @@ func resolveSymlinkWithInRoot(root, path string) (string, error) { if err != nil { return "", err } + // Rebase the target as if root were "/" and let Clean's "/..->/" rule + // clamp leading ".." segments at root, before they could otherwise + // collapse against the host filesystem when joined with root on the + // next iteration. + var abs string if filepath.IsAbs(target) { volLen := len(filepath.VolumeName(target)) - curr = filepath.Clean(strings.TrimPrefix(target[volLen:], sep)) + abs = filepath.Clean(target[volLen:]) } else { - curr = filepath.Clean(filepath.Join(filepath.Dir(curr), target)) + abs = filepath.Clean( + sep + filepath.Join(filepath.Dir(curr), target), + ) + } + curr = strings.TrimPrefix(abs, sep) + if curr == "" { + curr = "." } } return "", errors.New("too many symlinks (loop detected)") From fba5d568e0e29a9d0912cabbaa21b1b6acd4e13d Mon Sep 17 00:00:00 2001 From: Narendra Reddy Date: Thu, 30 Apr 2026 17:22:26 +0530 Subject: [PATCH 09/16] imaginator: update api documentation of AppendTree --- lib/fsutil/api.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/fsutil/api.go b/lib/fsutil/api.go index d1134078..f339f757 100644 --- a/lib/fsutil/api.go +++ b/lib/fsutil/api.go @@ -37,9 +37,11 @@ func AppendFile(destDir, destFilename, sourceFilename string) error { // destDir will be copied with the source file permissions. // Directory structures will be mirrored. An error is returned if symilinks or // non-regular files are encountered in sourceDir. If a destination path is a -// symlink, it must resolve to an existing location within destDir. -// Dangling symlinks, or a symilinks that resolve outside destDir, -// cause an error. +// symlink, it is resolved within destDir using chroot-style semantics: +// absolute targets are anchored at destDir and ".." is clamped at its root. +// Dangling symlinks cause an error. +// Valid symlink targets within destDir will have data appended +// to the resolved file; the symlink itself is preserved. func AppendTree(destDir, sourceDir string) error { return appendTree(destDir, sourceDir, AppendFile) } From fb6b7d6e29d54f48ab3132f6d9b035e1ef03e5eb Mon Sep 17 00:00:00 2001 From: Narendra Reddy Date: Thu, 30 Apr 2026 18:16:14 +0530 Subject: [PATCH 10/16] imaginator: implement safe path for chroot style directories in appendTree --- lib/fsutil/append.go | 8 ++++---- lib/fsutil/append_test.go | 4 +++- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/lib/fsutil/append.go b/lib/fsutil/append.go index 8737f948..993d90e1 100644 --- a/lib/fsutil/append.go +++ b/lib/fsutil/append.go @@ -73,16 +73,16 @@ func appendTree(destDir, sourceDir string, fileType := d.Type() switch { case fileType.IsDir(): - // Reject if the directory is a symlink and targetPath exists - // outside of destDir. - _, err := resolveSymlinkWithInRoot(destDir, + // Resolve the path to ensure any pre-existing symlinks in the + // destination are safely clamped to the chroot boundary. + safeDir, err := resolveSymlinkWithInRoot(destDir, destFilename) if err != nil { return err } // If path is a directory, create directory and return. // WalkDir will automatically visit the children next. - if err := os.MkdirAll(destFilename, DirPerms); err != nil { + if err := os.MkdirAll(safeDir, DirPerms); err != nil { return err } case fileType.IsRegular(): diff --git a/lib/fsutil/append_test.go b/lib/fsutil/append_test.go index 896d87e6..30cba937 100644 --- a/lib/fsutil/append_test.go +++ b/lib/fsutil/append_test.go @@ -208,7 +208,9 @@ func TestAppendFileWithDanglingDestSymlinks(t *testing.T) { } defer func() { err := os.Remove(destFilePath) - t.Fatalf("error removing symlink: %s", err) + if err != nil { + t.Fatalf("error removing symlink: %s", err) + } }() err := AppendTree(destTmp, sourceTmp) if err == nil { From af933c80b22e2e44a1a9a693547f16eed1898196 Mon Sep 17 00:00:00 2001 From: Narendra Reddy Date: Thu, 30 Apr 2026 18:46:24 +0530 Subject: [PATCH 11/16] imaginator: implement chroot style clamping to destDir --- lib/fsutil/resolveSymlink.go | 79 ++++++++++++++++++++++++------------ 1 file changed, 54 insertions(+), 25 deletions(-) diff --git a/lib/fsutil/resolveSymlink.go b/lib/fsutil/resolveSymlink.go index 368549f0..6ccab987 100644 --- a/lib/fsutil/resolveSymlink.go +++ b/lib/fsutil/resolveSymlink.go @@ -26,50 +26,79 @@ func resolveSymlinkWithInRoot(root, path string) (string, error) { if rel == ".." || strings.HasPrefix(rel, ".."+sep) { return "", fmt.Errorf("path %q escapes root %q", path, root) } - curr := rel - for nlinks := 0; nlinks <= maxLinks; nlinks++ { + var curr string + linksWalked := 0 + unprocessed := strings.Split(filepath.ToSlash(rel), "/") + symlinkDepth := 0 + for len(unprocessed) > 0 { + comp := unprocessed[0] + unprocessed = unprocessed[1:] + isFromSymlink := symlinkDepth > 0 + if isFromSymlink { + symlinkDepth-- + } + if comp == "" || comp == "." { + continue + } + if comp == ".." { + curr = filepath.Dir(curr) + if curr == "." || curr == sep { + curr = "" + } + continue + } + curr = filepath.Join(curr, comp) hostCurr := filepath.Join(root, curr) info, err := os.Lstat(hostCurr) if err != nil { if errors.Is(err, os.ErrNotExist) { - if nlinks == 0 { - return hostCurr, nil + if isFromSymlink { + fullTarget := hostCurr + if symlinkDepth > 0 { + remainder := unprocessed[:symlinkDepth] + fullTarget = filepath.Join( + hostCurr, + filepath.Join(remainder...), + ) + } + return "", + fmt.Errorf( + "dangling symlink: %q resolves to missing target %q", + path, + fullTarget, + ) } - return "", - fmt.Errorf( - "dangling symlink: %q resolves to missing target %q", - path, - hostCurr, - ) + if len(unprocessed) > 0 { + curr = filepath.Join(curr, filepath.Join(unprocessed...)) + } + break } return "", fmt.Errorf("lstat %q: %w", hostCurr, err) } // If it's not a symlink, we've found our final destination. if info.Mode()&os.ModeSymlink == 0 { - return hostCurr, nil + continue + } + linksWalked++ + if linksWalked > maxLinks { + return "", errors.New("too many symlinks (loop detected)") } // Read the symlink target. target, err := os.Readlink(hostCurr) if err != nil { return "", err } - // Rebase the target as if root were "/" and let Clean's "/..->/" rule - // clamp leading ".." segments at root, before they could otherwise - // collapse against the host filesystem when joined with root on the - // next iteration. - var abs string + // Rebase the target based on absolute vs relative links. if filepath.IsAbs(target) { volLen := len(filepath.VolumeName(target)) - abs = filepath.Clean(target[volLen:]) + curr = "" // Absolute links reset back to virtual root. + target = filepath.Clean(target[volLen:]) } else { - abs = filepath.Clean( - sep + filepath.Join(filepath.Dir(curr), target), - ) - } - curr = strings.TrimPrefix(abs, sep) - if curr == "" { - curr = "." + curr = filepath.Dir(curr) } + targetComps := strings.Split(filepath.ToSlash(target), "/") + unprocessed = append(targetComps, unprocessed...) + symlinkDepth += len(targetComps) } - return "", errors.New("too many symlinks (loop detected)") + return filepath.Join(root, curr), nil } From d08800c8f5a7ad81fca469694c359792255a1176 Mon Sep 17 00:00:00 2001 From: Narendra Reddy Date: Thu, 30 Apr 2026 19:00:16 +0530 Subject: [PATCH 12/16] imaginator: break into methods for readability and maintainability --- lib/fsutil/resolveSymlink.go | 115 ++++++++++++++++++++++------------- 1 file changed, 74 insertions(+), 41 deletions(-) diff --git a/lib/fsutil/resolveSymlink.go b/lib/fsutil/resolveSymlink.go index 6ccab987..2e1b72d2 100644 --- a/lib/fsutil/resolveSymlink.go +++ b/lib/fsutil/resolveSymlink.go @@ -11,25 +11,20 @@ import ( // resolveSymlinkWithInRoot resolves the symlink at path, following the entire // chain and guarantees the resolved path stays within root. ".." segments and // absolute targets are clamped at root (chroot-style semantics), so resolution -// never touches paths outside root.If the path is not a symlink (or does not +// never touches paths outside root. If the path is not a symlink (or does not // exist), it is returned unchanged. A dangling symlink chain returns an error. func resolveSymlinkWithInRoot(root, path string) (string, error) { - const maxLinks = 255 - sep := string(filepath.Separator) + const maxSymlinks = 255 root = filepath.Clean(root) path = filepath.Clean(path) - rel, err := filepath.Rel(root, path) + rel, err := validatePathWithinRoot(root, path) if err != nil { - return "", fmt.Errorf("relative path of %q in %q: %w", path, root, err) - } - //Caller-supplied path must already be inside root; reject before any I/O. - if rel == ".." || strings.HasPrefix(rel, ".."+sep) { - return "", fmt.Errorf("path %q escapes root %q", path, root) + return "", err } var curr string linksWalked := 0 - unprocessed := strings.Split(filepath.ToSlash(rel), "/") symlinkDepth := 0 + unprocessed := strings.Split(filepath.ToSlash(rel), "/") for len(unprocessed) > 0 { comp := unprocessed[0] unprocessed = unprocessed[1:] @@ -41,10 +36,7 @@ func resolveSymlinkWithInRoot(root, path string) (string, error) { continue } if comp == ".." { - curr = filepath.Dir(curr) - if curr == "." || curr == sep { - curr = "" - } + curr = clampToRoot(curr) continue } curr = filepath.Join(curr, comp) @@ -53,21 +45,14 @@ func resolveSymlinkWithInRoot(root, path string) (string, error) { if err != nil { if errors.Is(err, os.ErrNotExist) { if isFromSymlink { - fullTarget := hostCurr - if symlinkDepth > 0 { - remainder := unprocessed[:symlinkDepth] - fullTarget = filepath.Join( - hostCurr, - filepath.Join(remainder...), - ) - } - return "", - fmt.Errorf( - "dangling symlink: %q resolves to missing target %q", - path, - fullTarget, - ) + return "", buildDanglingSymlinkError( + path, + hostCurr, + unprocessed, + symlinkDepth, + ) } + // Append remaining components for a new directory path. if len(unprocessed) > 0 { curr = filepath.Join(curr, filepath.Join(unprocessed...)) } @@ -75,30 +60,78 @@ func resolveSymlinkWithInRoot(root, path string) (string, error) { } return "", fmt.Errorf("lstat %q: %w", hostCurr, err) } - // If it's not a symlink, we've found our final destination. if info.Mode()&os.ModeSymlink == 0 { continue } linksWalked++ - if linksWalked > maxLinks { + if linksWalked > maxSymlinks { return "", errors.New("too many symlinks (loop detected)") } - // Read the symlink target. - target, err := os.Readlink(hostCurr) + var targetComps []string + curr, targetComps, err = evaluateSymlinkTarget(hostCurr, curr) if err != nil { return "", err } - // Rebase the target based on absolute vs relative links. - if filepath.IsAbs(target) { - volLen := len(filepath.VolumeName(target)) - curr = "" // Absolute links reset back to virtual root. - target = filepath.Clean(target[volLen:]) - } else { - curr = filepath.Dir(curr) - } - targetComps := strings.Split(filepath.ToSlash(target), "/") unprocessed = append(targetComps, unprocessed...) symlinkDepth += len(targetComps) } return filepath.Join(root, curr), nil } + +// validatePathWithinRoot ensures the relative path does not escape the root. +func validatePathWithinRoot(root, path string) (string, error) { + rel, err := filepath.Rel(root, path) + if err != nil { + return "", fmt.Errorf("relative path of %q in %q: %w", path, root, err) + } + sep := string(filepath.Separator) + if rel == ".." || strings.HasPrefix(rel, ".."+sep) { + return "", fmt.Errorf("path %q escapes root %q", path, root) + } + return rel, nil +} + +// clampToRoot emulates a chroot boundary for ".." traversals. +func clampToRoot(curr string) string { + curr = filepath.Dir(curr) + if curr == "." || curr == string(filepath.Separator) { + return "" + } + return curr +} + +// evaluateSymlinkTarget reads the symlink and rebases the current path. +func evaluateSymlinkTarget(hostCurr, curr string) (string, []string, error) { + target, err := os.Readlink(hostCurr) + if err != nil { + return "", nil, err + } + if filepath.IsAbs(target) { + volLen := len(filepath.VolumeName(target)) + curr = "" + target = filepath.Clean(target[volLen:]) + } else { + curr = filepath.Dir(curr) + } + targetComps := strings.Split(filepath.ToSlash(target), "/") + return curr, targetComps, nil +} + +// buildDanglingSymlinkError reconstructs the full target path for error +// reporting. +func buildDanglingSymlinkError( + originalPath, hostCurr string, + unprocessed []string, + symlinkDepth int, +) error { + fullTarget := hostCurr + if symlinkDepth > 0 && len(unprocessed) >= symlinkDepth { + remainder := unprocessed[:symlinkDepth] + fullTarget = filepath.Join(hostCurr, filepath.Join(remainder...)) + } + return fmt.Errorf( + "dangling symlink: %q resolves to missing target %q", + originalPath, + fullTarget, + ) +} From 83b6e0088bc997a8e4f574f809ac3c08566615af Mon Sep 17 00:00:00 2001 From: Narendra Reddy Date: Thu, 30 Apr 2026 19:38:40 +0530 Subject: [PATCH 13/16] imaginator: implement chroot semantics for copying directories --- lib/fsutil/copy.go | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/lib/fsutil/copy.go b/lib/fsutil/copy.go index 74c44ae2..0651dee7 100644 --- a/lib/fsutil/copy.go +++ b/lib/fsutil/copy.go @@ -76,6 +76,12 @@ func copyToWriter(writer io.Writer, filename string, reader io.Reader, } func copyTree(destDir, sourceDir string, allTypes bool, + copyFunc func(destFilename, sourceFilename string, + mode os.FileMode) error) error { + return copyTreeWithRoot(destDir, destDir, sourceDir, allTypes, copyFunc) +} + +func copyTreeWithRoot(rootDir, destDir, sourceDir string, allTypes bool, copyFunc func(destFilename, sourceFilename string, mode os.FileMode) error) error { file, err := os.Open(sourceDir) @@ -99,12 +105,22 @@ func copyTree(destDir, sourceDir string, allTypes bool, } switch stat.Mode & wsyscall.S_IFMT { case wsyscall.S_IFDIR: - if err := os.Mkdir(destFilename, DirPerms); err != nil { + safeDir, err := resolveSymlinkWithInRoot(rootDir, destFilename) + if err != nil { + return err + } + if err := os.Mkdir(safeDir, DirPerms); err != nil { if !os.IsExist(err) { return err } } - err := copyTree(destFilename, sourceFilename, allTypes, copyFunc) + err = copyTreeWithRoot( + rootDir, + safeDir, + sourceFilename, + allTypes, + copyFunc, + ) if err != nil { return err } From 3aedc99cf11400047ae0afeb076de7ffbe6acf60 Mon Sep 17 00:00:00 2001 From: Narendra Reddy Date: Thu, 30 Apr 2026 20:06:31 +0530 Subject: [PATCH 14/16] imaginator: fix edge case of escaping to host from chroot --- lib/fsutil/api.go | 2 +- lib/fsutil/append.go | 14 +++++++------- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/lib/fsutil/api.go b/lib/fsutil/api.go index f339f757..8f23cfd4 100644 --- a/lib/fsutil/api.go +++ b/lib/fsutil/api.go @@ -35,7 +35,7 @@ func AppendFile(destDir, destFilename, sourceFilename string) error { // AppendTree recursively merges sourceDir into destDir. // Existing regular files will have data appended. Files which do not exist in // destDir will be copied with the source file permissions. -// Directory structures will be mirrored. An error is returned if symilinks or +// Directory structures will be mirrored. An error is returned if symlinks or // non-regular files are encountered in sourceDir. If a destination path is a // symlink, it is resolved within destDir using chroot-style semantics: // absolute targets are anchored at destDir and ".." is clamped at its root. diff --git a/lib/fsutil/append.go b/lib/fsutil/append.go index 993d90e1..2fb65f92 100644 --- a/lib/fsutil/append.go +++ b/lib/fsutil/append.go @@ -30,6 +30,13 @@ func appendToFile(destFilename string, reader io.Reader, } func appendFile(destDir, destFilename, sourceFilename string) error { + // Resolve the destination path to ensure it stays within destDir. + // This also safely computes the final path for new files and prevents + // intermediate directory symlinks for escaping the root boundary. + destFilename, err := resolveSymlinkWithInRoot(destDir, destFilename) + if err != nil { + return err + } if _, err := os.Lstat(destFilename); err != nil { if errors.Is(err, os.ErrNotExist) { // Dest file doesn't exist, so just copy the file. @@ -42,13 +49,6 @@ func appendFile(destDir, destFilename, sourceFilename string) error { } return err } - // File exists but that can be a symlink and target is dangling, - // or resolved symlink path is outside of destDir, which result in - // writes to wrong location on host. - destFilename, err := resolveSymlinkWithInRoot(destDir, destFilename) - if err != nil { - return err - } sourceFile, err := os.Open(sourceFilename) if err != nil { return errors.New(sourceFilename + ": " + err.Error()) From aa1f991aa21259c0f74a9f0fa9d5a9365d178f5f Mon Sep 17 00:00:00 2001 From: Narendra Reddy Date: Mon, 4 May 2026 20:11:43 +0530 Subject: [PATCH 15/16] lib/fsutil: maintain api backward compatibility --- lib/fsutil/api.go | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/lib/fsutil/api.go b/lib/fsutil/api.go index 8f23cfd4..9459261e 100644 --- a/lib/fsutil/api.go +++ b/lib/fsutil/api.go @@ -28,8 +28,15 @@ var ( // copied from sourceFilename. If there are any errors, then destFilename // may have partial data appended. // AppendFile is not safe to call concurrently for the same file. -func AppendFile(destDir, destFilename, sourceFilename string) error { - return appendFile(destDir, destFilename, sourceFilename) +func AppendFile(destFilename, sourceFilename string) error { + return appendFile("/", destFilename, sourceFilename) +} + +// AppendFileWithRoot extends AppendFile with safe symlink evaluation. Relative +// symlinks are clamped to the root boundary, and absolute symlinks are rebased +// against the root using chroot-style semantics. +func AppendFileWithRoot(root, destFilename, sourceFilename string) error { + return appendFile(root, destFilename, sourceFilename) } // AppendTree recursively merges sourceDir into destDir. @@ -43,7 +50,7 @@ func AppendFile(destDir, destFilename, sourceFilename string) error { // Valid symlink targets within destDir will have data appended // to the resolved file; the symlink itself is preserved. func AppendTree(destDir, sourceDir string) error { - return appendTree(destDir, sourceDir, AppendFile) + return appendTree(destDir, sourceDir, AppendFileWithRoot) } // CompareFile will read and compare the content of a file and buffer and will From f90c09e58bfb6552288fb0b47730cc778128ded4 Mon Sep 17 00:00:00 2001 From: Narendra Reddy Date: Tue, 5 May 2026 00:56:36 +0530 Subject: [PATCH 16/16] imaginator: implement vfs using openat2 for rootfs-aware copy and append operations --- lib/fsutil/api.go | 6 +- lib/fsutil/append.go | 62 ++++++----- lib/fsutil/copy.go | 24 +++-- lib/fsutil/resolveSymlink.go | 137 ------------------------- lib/fsutil/virtualFileSystem_linux.go | 71 +++++++++++++ lib/fsutil/virtualFileSystem_others.go | 20 ++++ 6 files changed, 145 insertions(+), 175 deletions(-) delete mode 100644 lib/fsutil/resolveSymlink.go create mode 100644 lib/fsutil/virtualFileSystem_linux.go create mode 100644 lib/fsutil/virtualFileSystem_others.go diff --git a/lib/fsutil/api.go b/lib/fsutil/api.go index 9459261e..97abb30d 100644 --- a/lib/fsutil/api.go +++ b/lib/fsutil/api.go @@ -29,14 +29,14 @@ var ( // may have partial data appended. // AppendFile is not safe to call concurrently for the same file. func AppendFile(destFilename, sourceFilename string) error { - return appendFile("/", destFilename, sourceFilename) + return appendFile(destFilename, sourceFilename) } // AppendFileWithRoot extends AppendFile with safe symlink evaluation. Relative // symlinks are clamped to the root boundary, and absolute symlinks are rebased // against the root using chroot-style semantics. -func AppendFileWithRoot(root, destFilename, sourceFilename string) error { - return appendFile(root, destFilename, sourceFilename) +func AppendFileWithRoot(rootFd int, destRelPath, sourcePath string) error { + return appendFileWithRoot(rootFd, destRelPath, sourcePath) } // AppendTree recursively merges sourceDir into destDir. diff --git a/lib/fsutil/append.go b/lib/fsutil/append.go index 2fb65f92..f70925e3 100644 --- a/lib/fsutil/append.go +++ b/lib/fsutil/append.go @@ -7,6 +7,8 @@ import ( "io/fs" "os" "path/filepath" + + "golang.org/x/sys/unix" ) func appendToFile(destFilename string, reader io.Reader, @@ -29,15 +31,8 @@ func appendToFile(destFilename string, reader io.Reader, return nil } -func appendFile(destDir, destFilename, sourceFilename string) error { - // Resolve the destination path to ensure it stays within destDir. - // This also safely computes the final path for new files and prevents - // intermediate directory symlinks for escaping the root boundary. - destFilename, err := resolveSymlinkWithInRoot(destDir, destFilename) - if err != nil { - return err - } - if _, err := os.Lstat(destFilename); err != nil { +func appendFile(destFilename, sourceFilename string) error { + if _, err := os.Stat(destFilename); err != nil { if errors.Is(err, os.ErrNotExist) { // Dest file doesn't exist, so just copy the file. var err error @@ -58,35 +53,54 @@ func appendFile(destDir, destFilename, sourceFilename string) error { return appendToFile(destFilename, sourceFile, 0) } +func appendFileWithRoot(rootFd int, destRelPath, sourcePath string) error { + mode, err := getFilePerms(sourcePath) + if err != nil { + return err + } + destFile, err := secureOpenFile(rootFd, destRelPath, uint32(mode)) + if err != nil { + return err + } + defer destFile.Close() + sourceFile, err := os.Open(sourcePath) + if err != nil { + return errors.New(sourcePath + ": " + err.Error()) + } + _, err = io.Copy(destFile, sourceFile) + if err != nil { + return fmt.Errorf( + "error copying contents from source %q to dest %q: %w", + sourcePath, destRelPath, err) + } + return nil +} + func appendTree(destDir, sourceDir string, - appendFunc func(destDir, dest, src string) error) error { + appendFunc func(rootFd int, destRelPath, sourcePath string) error) error { + rootFd, err := openRoot(destDir) + if err != nil { + return err + } + defer unix.Close(rootFd) return filepath.WalkDir(sourceDir, func(path string, d fs.DirEntry, err error) error { if err != nil { return err } + if path == sourceDir { + return nil + } relPath, err := filepath.Rel(sourceDir, path) if err != nil { return err } - destFilename := filepath.Join(destDir, relPath) fileType := d.Type() switch { case fileType.IsDir(): - // Resolve the path to ensure any pre-existing symlinks in the - // destination are safely clamped to the chroot boundary. - safeDir, err := resolveSymlinkWithInRoot(destDir, - destFilename) - if err != nil { - return err - } - // If path is a directory, create directory and return. - // WalkDir will automatically visit the children next. - if err := os.MkdirAll(safeDir, DirPerms); err != nil { - return err - } + return secureMkdir(rootFd, relPath, DirPerms) case fileType.IsRegular(): - if err := appendFunc(destDir, destFilename, path); err != nil { + if err := appendFunc(rootFd, relPath, path); err != nil { return err } case fileType&fs.ModeSymlink != 0: diff --git a/lib/fsutil/copy.go b/lib/fsutil/copy.go index 0651dee7..b3981553 100644 --- a/lib/fsutil/copy.go +++ b/lib/fsutil/copy.go @@ -7,6 +7,8 @@ import ( "os" "path" + "golang.org/x/sys/unix" + "github.com/Cloud-Foundations/Dominator/lib/wsyscall" ) @@ -78,10 +80,14 @@ func copyToWriter(writer io.Writer, filename string, reader io.Reader, func copyTree(destDir, sourceDir string, allTypes bool, copyFunc func(destFilename, sourceFilename string, mode os.FileMode) error) error { - return copyTreeWithRoot(destDir, destDir, sourceDir, allTypes, copyFunc) + rootFd, err := openRoot(destDir) + if err != nil { + return err + } + return copyTreeWithRoot(rootFd, ".", sourceDir, allTypes, copyFunc) } -func copyTreeWithRoot(rootDir, destDir, sourceDir string, allTypes bool, +func copyTreeWithRoot(rootFd int, destRelDir, sourceDir string, allTypes bool, copyFunc func(destFilename, sourceFilename string, mode os.FileMode) error) error { file, err := os.Open(sourceDir) @@ -98,25 +104,21 @@ func copyTreeWithRoot(rootDir, destDir, sourceDir string, allTypes bool, } for _, name := range names { sourceFilename := path.Join(sourceDir, name) - destFilename := path.Join(destDir, name) + destFilename := path.Join(destRelDir, name) var stat wsyscall.Stat_t if err := wsyscall.Lstat(sourceFilename, &stat); err != nil { return errors.New(sourceFilename + ": " + err.Error()) } switch stat.Mode & wsyscall.S_IFMT { case wsyscall.S_IFDIR: - safeDir, err := resolveSymlinkWithInRoot(rootDir, destFilename) - if err != nil { - return err - } - if err := os.Mkdir(safeDir, DirPerms); err != nil { - if !os.IsExist(err) { + if err := secureMkdir(rootFd, destFilename, DirPerms); err != nil { + if err != unix.ENOENT || !os.IsExist(err) { return err } } err = copyTreeWithRoot( - rootDir, - safeDir, + rootFd, + destFilename, sourceFilename, allTypes, copyFunc, diff --git a/lib/fsutil/resolveSymlink.go b/lib/fsutil/resolveSymlink.go deleted file mode 100644 index 2e1b72d2..00000000 --- a/lib/fsutil/resolveSymlink.go +++ /dev/null @@ -1,137 +0,0 @@ -package fsutil - -import ( - "errors" - "fmt" - "os" - "path/filepath" - "strings" -) - -// resolveSymlinkWithInRoot resolves the symlink at path, following the entire -// chain and guarantees the resolved path stays within root. ".." segments and -// absolute targets are clamped at root (chroot-style semantics), so resolution -// never touches paths outside root. If the path is not a symlink (or does not -// exist), it is returned unchanged. A dangling symlink chain returns an error. -func resolveSymlinkWithInRoot(root, path string) (string, error) { - const maxSymlinks = 255 - root = filepath.Clean(root) - path = filepath.Clean(path) - rel, err := validatePathWithinRoot(root, path) - if err != nil { - return "", err - } - var curr string - linksWalked := 0 - symlinkDepth := 0 - unprocessed := strings.Split(filepath.ToSlash(rel), "/") - for len(unprocessed) > 0 { - comp := unprocessed[0] - unprocessed = unprocessed[1:] - isFromSymlink := symlinkDepth > 0 - if isFromSymlink { - symlinkDepth-- - } - if comp == "" || comp == "." { - continue - } - if comp == ".." { - curr = clampToRoot(curr) - continue - } - curr = filepath.Join(curr, comp) - hostCurr := filepath.Join(root, curr) - info, err := os.Lstat(hostCurr) - if err != nil { - if errors.Is(err, os.ErrNotExist) { - if isFromSymlink { - return "", buildDanglingSymlinkError( - path, - hostCurr, - unprocessed, - symlinkDepth, - ) - } - // Append remaining components for a new directory path. - if len(unprocessed) > 0 { - curr = filepath.Join(curr, filepath.Join(unprocessed...)) - } - break - } - return "", fmt.Errorf("lstat %q: %w", hostCurr, err) - } - if info.Mode()&os.ModeSymlink == 0 { - continue - } - linksWalked++ - if linksWalked > maxSymlinks { - return "", errors.New("too many symlinks (loop detected)") - } - var targetComps []string - curr, targetComps, err = evaluateSymlinkTarget(hostCurr, curr) - if err != nil { - return "", err - } - unprocessed = append(targetComps, unprocessed...) - symlinkDepth += len(targetComps) - } - return filepath.Join(root, curr), nil -} - -// validatePathWithinRoot ensures the relative path does not escape the root. -func validatePathWithinRoot(root, path string) (string, error) { - rel, err := filepath.Rel(root, path) - if err != nil { - return "", fmt.Errorf("relative path of %q in %q: %w", path, root, err) - } - sep := string(filepath.Separator) - if rel == ".." || strings.HasPrefix(rel, ".."+sep) { - return "", fmt.Errorf("path %q escapes root %q", path, root) - } - return rel, nil -} - -// clampToRoot emulates a chroot boundary for ".." traversals. -func clampToRoot(curr string) string { - curr = filepath.Dir(curr) - if curr == "." || curr == string(filepath.Separator) { - return "" - } - return curr -} - -// evaluateSymlinkTarget reads the symlink and rebases the current path. -func evaluateSymlinkTarget(hostCurr, curr string) (string, []string, error) { - target, err := os.Readlink(hostCurr) - if err != nil { - return "", nil, err - } - if filepath.IsAbs(target) { - volLen := len(filepath.VolumeName(target)) - curr = "" - target = filepath.Clean(target[volLen:]) - } else { - curr = filepath.Dir(curr) - } - targetComps := strings.Split(filepath.ToSlash(target), "/") - return curr, targetComps, nil -} - -// buildDanglingSymlinkError reconstructs the full target path for error -// reporting. -func buildDanglingSymlinkError( - originalPath, hostCurr string, - unprocessed []string, - symlinkDepth int, -) error { - fullTarget := hostCurr - if symlinkDepth > 0 && len(unprocessed) >= symlinkDepth { - remainder := unprocessed[:symlinkDepth] - fullTarget = filepath.Join(hostCurr, filepath.Join(remainder...)) - } - return fmt.Errorf( - "dangling symlink: %q resolves to missing target %q", - originalPath, - fullTarget, - ) -} diff --git a/lib/fsutil/virtualFileSystem_linux.go b/lib/fsutil/virtualFileSystem_linux.go new file mode 100644 index 00000000..1fedfacb --- /dev/null +++ b/lib/fsutil/virtualFileSystem_linux.go @@ -0,0 +1,71 @@ +//go:build linux + +package fsutil + +import ( + "fmt" + "os" + "path/filepath" + + "golang.org/x/sys/unix" +) + +func openRoot(path string) (int, error) { + return unix.Open(path, unix.O_DIRECTORY|unix.O_PATH|unix.O_CLOEXEC, 0) +} + +func secureMkdir(rootFd int, relPath string, mode uint32) error { + dir, file := filepath.Split(relPath) + parentFd, err := unix.Openat2(rootFd, dir, &unix.OpenHow{ + Flags: unix.O_DIRECTORY | unix.O_PATH | unix.O_CLOEXEC, + Resolve: unix.RESOLVE_IN_ROOT | unix.RESOLVE_NO_MAGICLINKS, + }) + if err != nil { + return fmt.Errorf("resolving parent directory %q: %w", dir, err) + } + err = unix.Mkdirat(parentFd, file, mode) + if err != nil && err != unix.EEXIST { + return fmt.Errorf("mkdir %q:%w", relPath, err) + } + return nil +} + +func secureOpenFile(rootFd int, relPath string, mode uint32) (*os.File, error) { + fileFd, err := unix.Openat2(rootFd, relPath, &unix.OpenHow{ + Flags: unix.O_RDWR | unix.O_APPEND | unix.O_CLOEXEC, + Resolve: unix.RESOLVE_IN_ROOT | unix.RESOLVE_NO_MAGICLINKS, + }) + if err == nil { + // file already exists safely. + return os.NewFile(uintptr(fileFd), relPath), nil + } + if err != unix.ENOENT { + return nil, fmt.Errorf("error resolving file %q securely: %w", + relPath, err) + } + // ENOENT encountered, could be missing file or dangling symlink, + // check if parent directory exists. + parentFd, parentErr := unix.Openat2( + rootFd, + filepath.Dir(relPath), + &unix.OpenHow{ + Flags: unix.O_DIRECTORY | unix.O_PATH | unix.O_CLOEXEC, + Resolve: unix.RESOLVE_IN_ROOT | unix.RESOLVE_NO_MAGICLINKS, + }, + ) + if parentErr != nil { + return nil, fmt.Errorf("dangling symlink detected in path: %q", relPath) + } + if err := unix.Close(parentFd); err != nil { + return nil, err + } + fileFd, err = unix.Openat2(rootFd, relPath, &unix.OpenHow{ + Flags: unix.O_RDWR | unix.O_CREAT | unix.O_APPEND | unix.O_CLOEXEC, + Mode: uint64(mode), + Resolve: unix.RESOLVE_IN_ROOT | unix.RESOLVE_NO_MAGICLINKS, + }) + if err != nil { + return nil, fmt.Errorf("creating/appending file %q: %w", relPath, err) + } + return os.NewFile(uintptr(fileFd), relPath), nil +} diff --git a/lib/fsutil/virtualFileSystem_others.go b/lib/fsutil/virtualFileSystem_others.go new file mode 100644 index 00000000..4f944876 --- /dev/null +++ b/lib/fsutil/virtualFileSystem_others.go @@ -0,0 +1,20 @@ +//go:build !linux + +package fsutil + +import ( + "errors" + "os" +) + +func openRoot(path string) (int, error) { + return 0, errors.New("openRoot is supported in Linux only") +} + +func secureMkdir(rootFd int, relPath string, mode uint32) error { + return errors.New("secureMkdir is supported in Linux only") +} + +func secureOpenFile(rootFd int, relPath string, mode uint32) (*os.File, error) { + return nil, errors.New("secureOpenFile is supported in Linux only") +}