From a2de28e8046f16698c651de60ba25153ff91372e Mon Sep 17 00:00:00 2001 From: Andrew Nesbitt Date: Fri, 30 Oct 2026 13:34:07 +0000 Subject: [PATCH] Contain lockfile paths during verification --- internal/cli/verify_paths_test.go | 168 ++++++++++++++++++++++++++++++ verify.go | 91 ++++++++++------ 2 files changed, 228 insertions(+), 31 deletions(-) create mode 100644 internal/cli/verify_paths_test.go diff --git a/internal/cli/verify_paths_test.go b/internal/cli/verify_paths_test.go new file mode 100644 index 0000000..e574289 --- /dev/null +++ b/internal/cli/verify_paths_test.go @@ -0,0 +1,168 @@ +package cli + +import ( + "crypto/sha512" + "encoding/base64" + "errors" + "os" + "path/filepath" + "runtime" + "strings" + "testing" + + pin "github.com/git-pkgs/pin" + "github.com/git-pkgs/pin/lock" +) + +func writeVerifyLock(t *testing.T, dir, outDir string, paths ...string) { + t.Helper() + digest := sha512.Sum384([]byte("contents")) + l := &lock.Lock{OutDir: outDir} + for _, path := range paths { + l.Assets = append(l.Assets, lock.Asset{ + Name: "demo", Version: "1.0.0", Path: "x.js", Out: path, + Integrity: "sha384-" + base64.StdEncoding.EncodeToString(digest[:]), + }) + } + encoded, err := pin.EncodeLock(l) + if err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(dir, "pin.lock"), encoded, 0o644); err != nil { + t.Fatal(err) + } +} + +func TestCLI_VerifyRejectsInvalidPaths(t *testing.T) { + for _, tc := range []struct { + name string + outDir string + paths []string + }{ + {name: "asset-parent", outDir: "v", paths: []string{"../secret.txt"}}, + {name: "asset-absolute", outDir: "v", paths: []string{"/secret.txt"}}, + {name: "asset-empty", outDir: "v", paths: []string{""}}, + {name: "asset-dot", outDir: "v", paths: []string{"."}}, + {name: "output-parent", outDir: "../outside", paths: []string{"secret.txt"}}, + {name: "output-absolute", outDir: "/outside", paths: []string{"secret.txt"}}, + {name: "empty-assets", outDir: "../outside"}, + {name: "validate-all-first", outDir: "v", paths: []string{"directory", "../secret.txt"}}, + } { + t.Run(tc.name, func(t *testing.T) { + base := t.TempDir() + dir := filepath.Join(base, "project") + for _, path := range []string{"project/v/directory", "outside"} { + if err := os.MkdirAll(filepath.Join(base, path), 0o755); err != nil { + t.Fatal(err) + } + } + for _, path := range []string{"project/secret.txt", "outside/secret.txt"} { + if err := os.WriteFile(filepath.Join(base, path), []byte("contents"), 0o644); err != nil { + t.Fatal(err) + } + } + writeVerifyLock(t, dir, tc.outDir, tc.paths...) + out, err := runCmd(t, "verify", "-C", dir, "--json") + if !errors.Is(err, pin.ErrPathEscape) { + t.Fatalf("verify error = %v, want ErrPathEscape", err) + } + if out != "" { + t.Errorf("invalid lockfile produced verification output: %s", out) + } + }) + } +} + +func TestCLI_VerifySymlinkContainment(t *testing.T) { + for _, tc := range []struct { + name string + link string + target string + out string + allow bool + empty bool + }{ + {name: "file-outside-project", link: "v/link", target: "../../outside/secret.txt", out: "link"}, + {name: "asset-outside-project", link: "v/link", target: "../../outside", out: "link/secret.txt"}, + {name: "asset-outside-output", link: "v/link", target: "../private", out: "link/secret.txt"}, + {name: "output-outside-project", link: "v", target: "../outside", out: "secret.txt"}, + {name: "output-empty-assets", link: "v", target: "../outside", empty: true}, + {name: "internal-output-link", link: "v", target: "private", out: "secret.txt", allow: true}, + {name: "internal-link", link: "v/link", target: "files", out: "link/secret.txt", allow: true}, + } { + t.Run(tc.name, func(t *testing.T) { + dir := setupVerifySymlink(t, tc.link, tc.target) + paths := []string{tc.out} + if tc.empty { + paths = nil + } + writeVerifyLock(t, dir, "v", paths...) + out, err := runCmd(t, "verify", "-C", dir, "--json") + if tc.allow { + if err != nil || !strings.Contains(out, tc.out) { + t.Fatalf("internal symlink: error = %v, output = %s", err, out) + } + } else if err == nil || out != "" { + t.Fatalf("escaping symlink: error = %v, output = %s", err, out) + } + }) + } +} + +func setupVerifySymlink(t *testing.T, link, target string) string { + t.Helper() + base := t.TempDir() + dir := filepath.Join(base, "project") + for _, path := range []string{"outside", "project/private", "project/v/files"} { + if link == "v" && path == "project/v/files" { + continue + } + full := filepath.Join(base, path) + if err := os.MkdirAll(full, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(full, "secret.txt"), []byte("contents"), 0o644); err != nil { + t.Fatal(err) + } + } + if err := os.Symlink(filepath.FromSlash(target), filepath.Join(dir, link)); err != nil { + if runtime.GOOS == "windows" { + t.Skipf("symlinks unavailable: %v", err) + } + t.Fatal(err) + } + return dir +} + +func TestCLI_VerifyMissingOutputDirectory(t *testing.T) { + dir := t.TempDir() + writeVerifyLock(t, dir, "v", "demo/x.js") + out, err := runCmd(t, "verify", "-C", dir) + var exit *exitError + if !errors.As(err, &exit) || exit.ExitCode() != exitVerifyFailed { + t.Fatalf("verify error = %v, want missing-file exit code", err) + } + if !strings.Contains(out, "missing: demo/x.js") { + t.Errorf("missing asset not reported: %s", out) + } +} + +func TestCLI_VerifyRelativeOutputDirectory(t *testing.T) { + for _, outDir := range []string{"", ".", "./v/", "nested/vendor"} { + t.Run(outDir, func(t *testing.T) { + dir := t.TempDir() + output := filepath.Join(dir, outDir) + if err := os.MkdirAll(output, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(output, "x.js"), []byte("contents"), 0o644); err != nil { + t.Fatal(err) + } + writeVerifyLock(t, dir, outDir, "x.js") + out, err := runCmd(t, "verify", "-C", dir) + if err != nil || !strings.HasPrefix(out, "1 ok") { + t.Fatalf("verify error = %v, output = %s", err, out) + } + }) + } +} diff --git a/verify.go b/verify.go index 043fce2..b93063c 100644 --- a/verify.go +++ b/verify.go @@ -8,6 +8,7 @@ import ( "io" "io/fs" "os" + "path" "path/filepath" "sort" "strings" @@ -68,13 +69,60 @@ func (c *Client) Verify(opts VerifyOptions) (*VerifyResult, error) { return nil, fmt.Errorf("%w at %s", ErrNoLockfile, filepath.Join(opts.Dir, opts.Lock)) } + res, err := verifyLocalFiles(opts.Dir, l) + if err != nil { + return nil, err + } + + if opts.Strict { + drifts, err := c.verifyStrictNPM(l) + if err != nil { + return nil, err + } + res.Drifted = append(res.Drifted, drifts...) + } + + sort.Strings(res.OK) + sort.Strings(res.Missing) + sort.Strings(res.Extra) + return res, nil +} + +func verifyLocalFiles(dir string, l *lock.Lock) (*VerifyResult, error) { + outDir, err := filepath.Localize(path.Clean(l.OutDir)) + if err != nil { + return nil, fmt.Errorf("%w: invalid output directory %q", ErrPathEscape, l.OutDir) + } + for _, a := range l.Assets { + if _, err := filepath.Localize(a.Out); err != nil || a.Out == "." { + return nil, fmt.Errorf("%w: invalid asset path %q", ErrPathEscape, a.Out) + } + } + + project, err := os.OpenRoot(filepath.Clean(dir)) + if err != nil { + return nil, err + } + defer func() { _ = project.Close() }() + res := &VerifyResult{} - known := map[string]bool{} + root, err := project.OpenRoot(outDir) + if errors.Is(err, fs.ErrNotExist) { + for _, a := range l.Assets { + res.Missing = append(res.Missing, a.Out) + } + return res, nil + } + if err != nil { + return nil, fmt.Errorf("open output directory %q: %w", l.OutDir, err) + } + defer func() { _ = root.Close() }() + files := root.FS() + known := map[string]bool{} for _, a := range l.Assets { known[a.Out] = true - p := filepath.Join(opts.Dir, l.OutDir, filepath.FromSlash(a.Out)) - got, matches, err := verifyFile(p, a.Integrity) + got, matches, err := verifyFile(files, a.Out, a.Integrity) if errors.Is(err, fs.ErrNotExist) { res.Missing = append(res.Missing, a.Out) continue @@ -88,25 +136,8 @@ func (c *Client) Verify(opts VerifyOptions) (*VerifyResult, error) { } res.OK = append(res.OK, a.Out) } - - extra, err := findExtras(filepath.Join(opts.Dir, l.OutDir), known) - if err != nil { - return nil, err - } - res.Extra = extra - - if opts.Strict { - drifts, err := c.verifyStrictNPM(l) - if err != nil { - return nil, err - } - res.Drifted = append(res.Drifted, drifts...) - } - - sort.Strings(res.OK) - sort.Strings(res.Missing) - sort.Strings(res.Extra) - return res, nil + res.Extra, err = findExtras(files, known) + return res, err } // verifyStrictNPM re-fetches each npm tarball, re-extracts the @@ -167,8 +198,8 @@ func (c *Client) verifyStrictNPM(l *lock.Lock) ([]Drift, error) { return drifts, nil } -func verifyFile(path, expected string) (string, bool, error) { - f, err := os.Open(path) +func verifyFile(files fs.FS, path, expected string) (string, bool, error) { + f, err := files.Open(path) if err != nil { return "", false, err } @@ -204,19 +235,17 @@ func verifyIntegrity(source io.Reader, expectedValue string) (string, bool, erro return actual, matches, nil } -func findExtras(root string, known map[string]bool) ([]string, error) { +func findExtras(files fs.FS, known map[string]bool) ([]string, error) { var extras []string - err := filepath.WalkDir(root, func(p string, d fs.DirEntry, err error) error { + err := fs.WalkDir(files, ".", func(p string, d fs.DirEntry, err error) error { if err != nil || d.IsDir() { return err } - rel, _ := filepath.Rel(root, p) - rel = filepath.ToSlash(rel) - if strings.HasSuffix(rel, ".tmp") { + if strings.HasSuffix(p, ".tmp") { return nil } - if !known[rel] { - extras = append(extras, rel) + if !known[p] { + extras = append(extras, p) } return nil })