diff --git a/pkg/git/installer.go b/pkg/git/installer.go index 015ca4dec..aded2ac65 100644 --- a/pkg/git/installer.go +++ b/pkg/git/installer.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "os" "github.com/devsy-org/devsy/pkg/command" "github.com/devsy-org/devsy/pkg/log" @@ -106,7 +107,13 @@ type pkgManagerStrategy struct { func (s *pkgManagerStrategy) name() string { return s.manager } -func (s *pkgManagerStrategy) usable() bool { return command.Exists(s.manager) } +var isRoot = func() bool { return os.Geteuid() == 0 } + +// usable requires root: package installs write to system-owned locations +// (e.g. apt's lock files, dpkg's database). +func (s *pkgManagerStrategy) usable() bool { + return command.Exists(s.manager) && isRoot() +} func (s *pkgManagerStrategy) install(ctx context.Context, t tool) error { log.Infof("installing %s with %s", t.pkg, s.manager) diff --git a/pkg/git/installer_release.go b/pkg/git/installer_release.go index f0e331b3e..f1bbab1a2 100644 --- a/pkg/git/installer_release.go +++ b/pkg/git/installer_release.go @@ -84,6 +84,10 @@ func (s *releaseSource) install(ctx context.Context, binary string) error { } } + if err := ensureDirWritable(installDir); err != nil { + return fmt.Errorf("install dir %q is not writable: %w", installDir, err) + } + req := fetchRequest{ binary: binary, url: s.downloadURL(s.version, asset), @@ -96,9 +100,6 @@ func (s *releaseSource) install(ctx context.Context, binary string) error { } defer cleanup() - if err := os.MkdirAll(installDir, 0o750); err != nil { - return fmt.Errorf("create install dir %q: %w", installDir, err) - } dst := filepath.Join(installDir, req.execName) if err := moveExecutable(src, dst); err != nil { return fmt.Errorf("install %s to %q: %w", binary, dst, err) @@ -206,3 +207,19 @@ func moveExecutable(src, dst string) error { // #nosec G306,G703 -- dst is internally constructed; an executable must be world-executable return os.WriteFile(dst, data, 0o755) } + +func ensureDirWritable(dir string) error { + if err := os.MkdirAll(dir, 0o750); err != nil { + return err + } + probe, err := os.CreateTemp(dir, ".devsy-write-test-*") + if err != nil { + return err + } + name := probe.Name() + if err := probe.Close(); err != nil { + _ = os.Remove(name) + return err + } + return os.Remove(name) +} diff --git a/pkg/git/installer_test.go b/pkg/git/installer_test.go index 6ea52a548..390afe8ba 100644 --- a/pkg/git/installer_test.go +++ b/pkg/git/installer_test.go @@ -3,6 +3,8 @@ package git import ( "context" "fmt" + "os" + "path/filepath" "strings" "testing" @@ -48,6 +50,40 @@ func TestReleaseStrategyRequiresReleaseSource(t *testing.T) { assert.Assert(t, err != nil) } +func TestPkgManagerStrategyUsableRequiresRoot(t *testing.T) { + s := &pkgManagerStrategy{manager: "sh"} // "sh" always exists in test environments + + original := isRoot + t.Cleanup(func() { isRoot = original }) + + isRoot = func() bool { return true } + assert.Assert(t, s.usable()) + + isRoot = func() bool { return false } + assert.Assert(t, !s.usable()) +} + +func TestEnsureDirWritable(t *testing.T) { + assert.NilError(t, ensureDirWritable(t.TempDir())) +} + +func TestEnsureDirWritableCreatesMissingDir(t *testing.T) { + dir := filepath.Join(t.TempDir(), "does", "not", "exist", "yet") + assert.NilError(t, ensureDirWritable(dir)) +} + +func TestEnsureDirWritableRejectsReadOnlyDir(t *testing.T) { + if os.Geteuid() == 0 { + t.Skip("permission test not meaningful when running as root") + } + + dir := t.TempDir() + // #nosec G302 -- intentional: testing restrictive perms + assert.NilError(t, os.Chmod(dir, 0o500)) + + assert.Assert(t, ensureDirWritable(dir) != nil) +} + // fakeStrategy is a test installStrategy with configurable behavior. type fakeStrategy struct { label string