Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion pkg/git/installer.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"errors"
"fmt"
"os"

"github.com/devsy-org/devsy/pkg/command"
"github.com/devsy-org/devsy/pkg/log"
Expand Down Expand Up @@ -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)
Expand Down
23 changes: 20 additions & 3 deletions pkg/git/installer_release.go
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand All @@ -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)
Expand Down Expand Up @@ -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)
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}
36 changes: 36 additions & 0 deletions pkg/git/installer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ package git
import (
"context"
"fmt"
"os"
"path/filepath"
"strings"
"testing"

Expand Down Expand Up @@ -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
Expand Down
Loading