← back to Cli Printing Press
fix(cli): reject external symlinks during copy (#53)
5ee774c80c6cb06f535bce2eddfbbb83a131e545 · 2026-03-29 12:49:30 -0700 · Trevin Chow
Prevent published and archived artifacts from preserving symlinks that
escape the source tree while still avoiding traversal into directory
symlinks. Add tests covering internal symlink preservation and external
symlink rejection.
Files touched
M internal/pipeline/publish.goA internal/pipeline/publish_test.go
Diff
commit 5ee774c80c6cb06f535bce2eddfbbb83a131e545
Author: Trevin Chow <trevin@trevinchow.com>
Date: Sun Mar 29 12:49:30 2026 -0700
fix(cli): reject external symlinks during copy (#53)
Prevent published and archived artifacts from preserving symlinks that
escape the source tree while still avoiding traversal into directory
symlinks. Add tests covering internal symlink preservation and external
symlink rejection.
---
internal/pipeline/publish.go | 62 ++++++++++++++++----
internal/pipeline/publish_test.go | 115 ++++++++++++++++++++++++++++++++++++++
2 files changed, 166 insertions(+), 11 deletions(-)
diff --git a/internal/pipeline/publish.go b/internal/pipeline/publish.go
index 35f6a0bc..2673d985 100644
--- a/internal/pipeline/publish.go
+++ b/internal/pipeline/publish.go
@@ -4,6 +4,7 @@ import (
"encoding/json"
"fmt"
"io"
+ "io/fs"
"os"
"path/filepath"
"strings"
@@ -223,7 +224,15 @@ func CopyDir(src, dst string) error {
return err
}
- return filepath.Walk(src, func(path string, _ os.FileInfo, walkErr error) error {
+ srcRoot, err := filepath.Abs(src)
+ if err != nil {
+ return fmt.Errorf("resolving source dir: %w", err)
+ }
+
+ // WalkDir (unlike Walk) does not follow directory symlinks, so the
+ // callback sees them as symlink entries and we can validate them
+ // without descending into potentially huge or circular targets.
+ return filepath.WalkDir(src, func(path string, d fs.DirEntry, walkErr error) error {
if walkErr != nil {
return walkErr
}
@@ -237,27 +246,58 @@ func CopyDir(src, dst string) error {
}
target := filepath.Join(dst, rel)
- info, err := os.Lstat(path)
- if err != nil {
- return err
- }
-
- if info.IsDir() {
- return os.MkdirAll(target, info.Mode())
- }
-
- if info.Mode()&os.ModeSymlink != 0 {
+ // d.Type() returns mode bits from Lstat, so symlinks (including
+ // directory symlinks) are detected before any descent.
+ if d.Type()&os.ModeSymlink != 0 {
link, err := os.Readlink(path)
if err != nil {
return err
}
+ ok, err := symlinkTargetWithinRoot(srcRoot, path, link)
+ if err != nil {
+ return err
+ }
+ if !ok {
+ return fmt.Errorf("symlink %s points outside source tree", path)
+ }
return os.Symlink(link, target)
}
+ if d.IsDir() {
+ info, err := d.Info()
+ if err != nil {
+ return err
+ }
+ return os.MkdirAll(target, info.Mode())
+ }
+
+ info, err := d.Info()
+ if err != nil {
+ return err
+ }
return copyFile(path, target, info.Mode())
})
}
+func symlinkTargetWithinRoot(root, path, link string) (bool, error) {
+ resolved := link
+ if !filepath.IsAbs(resolved) {
+ resolved = filepath.Join(filepath.Dir(path), resolved)
+ }
+
+ absResolved, err := filepath.Abs(resolved)
+ if err != nil {
+ return false, fmt.Errorf("resolving symlink target for %s: %w", path, err)
+ }
+
+ rel, err := filepath.Rel(root, absResolved)
+ if err != nil {
+ return false, fmt.Errorf("checking symlink target for %s: %w", path, err)
+ }
+
+ return rel == "." || (rel != ".." && !strings.HasPrefix(rel, ".."+string(os.PathSeparator))), nil
+}
+
func copyFile(src, dst string, mode os.FileMode) error {
if err := os.MkdirAll(filepath.Dir(dst), 0o755); err != nil {
return err
diff --git a/internal/pipeline/publish_test.go b/internal/pipeline/publish_test.go
new file mode 100644
index 00000000..2f59d9b7
--- /dev/null
+++ b/internal/pipeline/publish_test.go
@@ -0,0 +1,115 @@
+package pipeline
+
+import (
+ "os"
+ "path/filepath"
+ "testing"
+
+ "github.com/stretchr/testify/assert"
+ "github.com/stretchr/testify/require"
+)
+
+func TestCopyDir(t *testing.T) {
+ tests := []struct {
+ name string
+ setup func(t *testing.T, src string)
+ check func(t *testing.T, src, dst string)
+ }{
+ {
+ name: "regular files and directories",
+ setup: func(t *testing.T, src string) {
+ require.NoError(t, os.MkdirAll(filepath.Join(src, "sub"), 0o755))
+ require.NoError(t, os.WriteFile(filepath.Join(src, "root.txt"), []byte("root"), 0o644))
+ require.NoError(t, os.WriteFile(filepath.Join(src, "sub", "nested.txt"), []byte("nested"), 0o644))
+ },
+ check: func(t *testing.T, _, dst string) {
+ assert.FileExists(t, filepath.Join(dst, "root.txt"))
+ assert.FileExists(t, filepath.Join(dst, "sub", "nested.txt"))
+ data, err := os.ReadFile(filepath.Join(dst, "root.txt"))
+ require.NoError(t, err)
+ assert.Equal(t, "root", string(data))
+ },
+ },
+ {
+ name: "internal file symlink preserved as symlink",
+ setup: func(t *testing.T, src string) {
+ target := filepath.Join(src, "target.txt")
+ require.NoError(t, os.WriteFile(target, []byte("target"), 0o644))
+ require.NoError(t, os.Symlink("target.txt", filepath.Join(src, "link.txt")))
+ },
+ check: func(t *testing.T, _, dst string) {
+ linkPath := filepath.Join(dst, "link.txt")
+ info, err := os.Lstat(linkPath)
+ require.NoError(t, err)
+ assert.NotZero(t, info.Mode()&os.ModeSymlink, "expected symlink, got regular file")
+ },
+ },
+ {
+ name: "internal directory symlink preserved as symlink",
+ setup: func(t *testing.T, src string) {
+ targetDir := filepath.Join(src, "target-dir")
+ require.NoError(t, os.MkdirAll(targetDir, 0o755))
+ require.NoError(t, os.WriteFile(filepath.Join(targetDir, "big.bin"), []byte("data"), 0o644))
+ require.NoError(t, os.Symlink("target-dir", filepath.Join(src, "linked-dir")))
+ },
+ check: func(t *testing.T, _, dst string) {
+ linkPath := filepath.Join(dst, "linked-dir")
+ info, err := os.Lstat(linkPath)
+ require.NoError(t, err)
+ assert.NotZero(t, info.Mode()&os.ModeSymlink,
+ "directory symlink should be preserved as a symlink, not followed")
+ targetData, err := os.ReadFile(filepath.Join(dst, "target-dir", "big.bin"))
+ require.NoError(t, err)
+ assert.Equal(t, "data", string(targetData))
+ },
+ },
+ }
+
+ for _, tt := range tests {
+ t.Run(tt.name, func(t *testing.T) {
+ src := filepath.Join(t.TempDir(), "src")
+ dst := filepath.Join(t.TempDir(), "dst")
+ require.NoError(t, os.MkdirAll(src, 0o755))
+
+ tt.setup(t, src)
+ require.NoError(t, CopyDir(src, dst))
+ tt.check(t, src, dst)
+ })
+ }
+}
+
+func TestCopyDirRejectsExternalSymlinks(t *testing.T) {
+ tests := []struct {
+ name string
+ linkName string
+ linkTarget string
+ }{
+ {
+ name: "absolute external target",
+ linkName: "external.txt",
+ linkTarget: filepath.Join(t.TempDir(), "outside.txt"),
+ },
+ {
+ name: "relative target escaping root",
+ linkName: "escape.txt",
+ linkTarget: filepath.Join("..", "outside.txt"),
+ },
+ }
+
+ for _, tt := range tests {
+ t.Run(tt.name, func(t *testing.T) {
+ root := t.TempDir()
+ src := filepath.Join(root, "src")
+ dst := filepath.Join(root, "dst")
+ require.NoError(t, os.MkdirAll(src, 0o755))
+
+ outside := filepath.Join(root, "outside.txt")
+ require.NoError(t, os.WriteFile(outside, []byte("outside"), 0o644))
+ require.NoError(t, os.Symlink(tt.linkTarget, filepath.Join(src, tt.linkName)))
+
+ err := CopyDir(src, dst)
+ require.Error(t, err)
+ assert.ErrorContains(t, err, "points outside source tree")
+ })
+ }
+}
← cd61dd88 fix(skills): recommend installing both capture tools in snif
·
back to Cli Printing Press
·
fix(skills): make sniff gate reliable with CLI-driven browsi ccf7b2ee →