Merge pull request #40 from vinyl-linux/finalisers

Register finalisers

config.go

@@ -7,9 +7,13 @@ import (
 	"github.com/vinyl-linux/vin/config"
 )
 
+var (
+	cfg config.Config
+)
+
 type InstallationValues struct {
-	config.Config
 	*Manifest
+	config.Config
 }
 
 // Expand takes a string (containing an ostensible template) and
@@ -28,6 +32,7 @@ func (c InstallationValues) Expand(s string) (cmd string, err error) {
 		return
 	}
 
+	c.Config = cfg
 	err = tmpl.Execute(b, c)
 	if err != nil {
 		return
@@ -37,3 +42,9 @@ func (c InstallationValues) Expand(s string) (cmd string, err error) {
 
 	return
 }
+
+func loadConfig() (err error) {
+	cfg, err = config.Load(configFile)
+
+	return
+}

config/config.go

@@ -3,7 +3,7 @@ package config
 import (
 	"io/ioutil"
 
-	"github.com/pelletier/go-toml"
+	"github.com/pelletier/go-toml/v2"
 )
 
 // Config holds all of the configuration that vin needs,

config/config_test.go

@@ -1,6 +1,7 @@
 package config
 
 import (
+	"fmt"
 	"testing"
 )
 
@@ -23,18 +24,11 @@ func TestLoadConfig(t *testing.T) {
 	}
 }
 
-func TestConfig_String(t *testing.T) {
-	defer func() {
-		err := recover()
-		if err != nil {
-			t.Fatalf("unexpected error: %#v", err)
-		}
-	}()
-
-	c := Config{}.String()
-	expect := "CFLAGS = \"\"\nCXXFLAGS = \"\"\nMAKEOPTS = \"\"\nconfigure_flags = \"\"\n"
-
-	if expect != c {
-		t.Errorf("expected:\n%sreceived:\n%s", expect, c)
-	}
+func ExampleConfig_String() {
+	fmt.Println(Config{}.String())
+	// Output:
+	// configure_flags = ''
+	// MAKEOPTS = ''
+	// CFLAGS = ''
+	// CXXFLAGS = ''
 }

config_test.go

@@ -2,24 +2,23 @@ package main
 
 import (
 	"testing"
-
-	"github.com/vinyl-linux/vin/config"
 )
 
 func TestInstallationValues_Expand(t *testing.T) {
 	configFile = "testdata/test-config.toml"
 
-	c, err := config.Load(configFile)
+	err := loadConfig()
 	if err != nil {
 		t.Fatalf("unexpected error: %+v", err)
 	}
 
 	m := &Manifest{
 		ManifestDir: "/tmp/app/1.0.0/",
+		Commands: Commands{
+			installationValues: InstallationValues{Manifest: &Manifest{ManifestDir: "/tmp/app/1.0.0/"}},
+		},
 	}
 
-	ir := InstallationValues{c, m}
-
 	for _, test := range []struct {
 		name        string
 		in          string
@@ -33,7 +32,7 @@ func TestInstallationValues_Expand(t *testing.T) {
 		{"dodgy template", " {{ .HelloWorld }}", "", true},
 	} {
 		t.Run(test.name, func(t *testing.T) {
-			got, err := ir.Expand(test.in)
+			got, err := m.Commands.installationValues.Expand(test.in)
 
 			if err == nil && test.expectError {
 				t.Error("expected error, received none")

go.mod

@@ -11,7 +11,7 @@ require (
 	github.com/hashicorp/go-memdb v1.3.2
 	github.com/hashicorp/go-version v1.4.0
 	github.com/mitchellh/go-homedir v1.1.0
-	github.com/pelletier/go-toml v1.9.4
+	github.com/pelletier/go-toml/v2 v2.0.0-beta.6
 	github.com/spf13/cobra v1.1.1
 	github.com/spf13/viper v1.7.1
 	github.com/zeebo/blake3 v0.2.2
@@ -31,6 +31,7 @@ require (
 	github.com/mattn/go-colorable v0.1.12 // indirect
 	github.com/mattn/go-isatty v0.0.14 // indirect
 	github.com/mitchellh/mapstructure v1.1.2 // indirect
+	github.com/pelletier/go-toml v1.9.4 // indirect
 	github.com/spf13/afero v1.1.2 // indirect
 	github.com/spf13/cast v1.3.0 // indirect
 	github.com/spf13/jwalterweatherman v1.0.0 // indirect

go.sum

@@ -200,6 +200,8 @@ github.com/pascaldekloe/goe v0.0.0-20180627143212-57f6aae5913c/go.mod h1:lzWF7FI
 github.com/pelletier/go-toml v1.2.0/go.mod h1:5z9KED0ma1S8pY6P1sdut58dfprrGBbd/94hg7ilaic=
 github.com/pelletier/go-toml v1.9.4 h1:tjENF6MfZAg8e4ZmZTeWaWiT2vXtsoO6+iuOjFhECwM=
 github.com/pelletier/go-toml v1.9.4/go.mod h1:u1nR/EPcESfeI/szUZKdtJ0xRNbUoANCkoOuaOx1Y+c=
+github.com/pelletier/go-toml/v2 v2.0.0-beta.6 h1:JFNqj2afbbhCqTiyN16D7Tudc/aaDzE2FBDk+VlBQnE=
+github.com/pelletier/go-toml/v2 v2.0.0-beta.6/go.mod h1:ke6xncR3W76Ba8xnVxkrZG0js6Rd2BsQEAYrfgJ6eQA=
 github.com/pkg/errors v0.8.0/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=
 github.com/pkg/errors v0.8.1 h1:iURUrRGxPUNPdy5/HRSm+Yj6okJ6UtLINN0Q9M4+h3I=
 github.com/pkg/errors v0.8.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=
@@ -251,8 +253,9 @@ github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXf
 github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI=
 github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4=
 github.com/stretchr/testify v1.5.1/go.mod h1:5W2xD1RspED5o8YsWQXVCued0rvSQ+mT+I5cxcmMvtA=
-github.com/stretchr/testify v1.7.0 h1:nwc3DEeHmmLAfoZucVR881uASk0Mfjw8xYJ99tb5CcY=
 github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
+github.com/stretchr/testify v1.7.1-0.20210427113832-6241f9ab9942 h1:t0lM6y/M5IiUZyvbBTcngso8SZEZICH7is9B6g/obVU=
+github.com/stretchr/testify v1.7.1-0.20210427113832-6241f9ab9942/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
 github.com/subosito/gotenv v1.2.0 h1:Slr1R9HxAlEKefgq5jn9U+DnETlIUa6HfgEzj0g5d7s=
 github.com/subosito/gotenv v1.2.0/go.mod h1:N0PQaV/YGNqwC0u51sEeR/aUtSLEXKX9iv69rRypqCw=
 github.com/tmc/grpc-websocket-proxy v0.0.0-20190109142713-0ad062ec5ee5/go.mod h1:ncp9v5uamzpCO7NfCPTXjqaC+bZgJeR0sMTm6dMHP7U=

main.go

@@ -7,7 +7,6 @@ import (
 	"github.com/grpc-ecosystem/go-grpc-middleware"
 	"github.com/grpc-ecosystem/go-grpc-middleware/logging/zap"
 	"github.com/grpc-ecosystem/go-grpc-middleware/tags"
-	"github.com/vinyl-linux/vin/config"
 	"github.com/vinyl-linux/vin/server"
 	"go.uber.org/zap"
 	"google.golang.org/grpc"
@@ -53,7 +52,7 @@ func Setup() *grpc.Server {
 	sugar.Info("starting")
 	sugar.Info("loading config")
 
-	c, err := config.Load(configFile)
+	err := loadConfig()
 	if err != nil {
 		sugar.Panic(err)
 	}
@@ -77,7 +76,7 @@ func Setup() *grpc.Server {
 	sugar.Info("loaded")
 	sugar.Info("starting server")
 
-	s, err := NewServer(c, mdb, sdb)
+	s, err := NewServer(mdb, sdb)
 	if err != nil {
 		sugar.Panic(err)
 	}

manifest.go

@@ -1,16 +1,14 @@
 package main
 
 import (
-	_ "log"
-
 	"fmt"
 	"io/ioutil"
 	"os"
 	"path/filepath"
+	"strings"
 
 	"github.com/hashicorp/go-version"
-	"github.com/pelletier/go-toml"
-	"github.com/vinyl-linux/vin/config"
+	"github.com/pelletier/go-toml/v2"
 )
 
 const (
@@ -106,8 +104,6 @@ func (m Manifest) String() string { return fmt.Sprintf("%s %s", m.Provides, m.Ve
 // It handles things like downloading and verifying tarballs, and subsequently untarring
 func (m *Manifest) Prepare(output chan string) (err error) {
 	// This function will download the Manifest Tarball, checksum it, un-tar it, and so on.
-
-	m.dir = filepath.Join(cacheDir, m.Provides, m.VersionStr)
 	err = os.MkdirAll(m.dir, 0755)
 	if err != nil {
 		return
@@ -171,6 +167,33 @@ type Commands struct {
 	WorkingDir string
 	Patches    []string
 	Skipenv    bool
+	Finaliser  string
+
+	installationValues InstallationValues
+	absoluteWorkingDir string
+}
+
+func (c *Commands) Initialise(m Manifest) (err error) {
+	c.installationValues = InstallationValues{Manifest: &m}
+
+	// Do our best to ensure that workdirs are within the correct
+	// tree, free of symlinks, and resolvable.
+	//
+	// This wont solve dodgy packages with symlinks blatting things
+	// away, but it'll help stop install scripts breaking things;
+	// certainly accidentally.
+	//
+	// We're doing some pretty unsophisticated strings matching, purely
+	// because we control directory names ourselves, which means we're
+	// always in control of case sensitivity and so on
+	c.absoluteWorkingDir = filepath.Clean(filepath.Join(m.dir, c.WorkingDir))
+	if !strings.HasPrefix(c.absoluteWorkingDir, m.dir) {
+		return fmt.Errorf("working dir %s resolves to outside the cache dir %s to %s",
+			c.WorkingDir, m.dir, c.absoluteWorkingDir,
+		)
+	}
+
+	return
 }
 
 // Slice returns each command in an ordered slice
@@ -232,7 +255,7 @@ func (c Commands) Patch(wd string, output chan string) (err error) {
 		patchCmd := fmt.Sprintf("patch -p1 -i %s", p)
 
 		output <- fmt.Sprintf("patch %d/%d", i+1, patches)
-		err = execute(wd, patchCmd, c.Skipenv, output, config.Config{})
+		err = execute(wd, patchCmd, c.Skipenv, output)
 		if err != nil {
 			return
 		}
@@ -292,6 +315,13 @@ func readManifest(filename string) (m Manifest, err error) {
 func processManifest(m Manifest) (m1 Manifest, err error) {
 	m1 = m
 
+	m1.dir = filepath.Join(cacheDir, m.Provides, m.VersionStr)
+
+	err = m1.Commands.Initialise(m1)
+	if err != nil {
+		return
+	}
+
 	m1.Version, err = version.NewVersion(m.VersionStr)
 	if err != nil {
 		return

manifest_db.go

@@ -1,8 +1,6 @@
 package main
 
 import (
-	_ "log"
-
 	"fmt"
 	"sort"
 

manifest_test.go

@@ -22,7 +22,7 @@ func TestReadManifest(t *testing.T) {
 	}{
 		{"valid vin manifest", "vin", "0.0.0-rc0", Manifest{ID: "vin 0.0.0-rc0", Provides: "vin", VersionStr: "0.0.0-rc0", Licence: "BSD3", Tarball: "https://github.com/vinyl-linux/vin/archive/0.0.0-rc0.tar.gz", ManifestDir: "testdata/manifests/vin/0.0.0-rc0", Profiles: map[string]Profile{"default": {Deps: []Dep{{"go", ">= 1.12"}}}}, Commands: Commands{Configure: strP("true"), Compile: strP("make"), Install: strP("make install"), Patches: []string(nil)}}, false},
 		{"missing manifest", "unknown", "0", Manifest{}, true},
-		{"invalid manifest", "invalid", "0.1.0", Manifest{}, true},
+		{"invalid manifest", "invalid", "0.1.0", Manifest{Provides: "invalid"}, true},
 		{"invalid dep", "invalid", "0.1.1", Manifest{ID: "invalid 0.1.1", Provides: "invalid", VersionStr: "0.1.1", ManifestDir: "testdata/manifests/invalid/0.1.1", Profiles: map[string]Profile{"default": {Deps: []Dep{{"bash", "xxx"}}}}, Commands: Commands{Patches: []string(nil)}}, true},
 		{"manifest with patches", "patched", "0.0.1", Manifest{ID: "patched 0.0.1", Provides: "patched", VersionStr: "0.0.1", ManifestDir: "testdata/manifests/patched/0.0.1", Commands: Commands{Patches: []string{"testdata/manifests/patched/0.0.1/0.patch"}}}, false},
 	} {
@@ -31,16 +31,15 @@ func TestReadManifest(t *testing.T) {
 
 			if err == nil && test.expectError {
 				t.Error("expected error, received none")
-			} else if err != nil {
-				if !test.expectError {
-					t.Errorf("unexpected error: %+v", err)
-				} else {
-					t.Logf("received error: %+v", err)
-				}
+			} else if err != nil && !test.expectError {
+				t.Errorf("unexpected error: %+v", err)
 			}
 
-			// Remove Version pointer to make testing easier
+			// Remove anything with a pointer to aid testing
 			received.Version = nil
+			received.Commands.installationValues.Manifest = nil
+			received.Commands.absoluteWorkingDir = ""
+			received.dir = ""
 
 			if !reflect.DeepEqual(test.expect, received) {
 				t.Errorf("expected:\t\n%#v\nreceived:\t\n%#v", test.expect, received)
@@ -50,7 +49,12 @@ func TestReadManifest(t *testing.T) {
 }
 
 func TestManifest_Prepare(t *testing.T) {
-	cacheDir, _ = ioutil.TempDir("", "")
+	var err error
+
+	cacheDir, err = ioutil.TempDir("", "")
+	if err != nil {
+		t.Fatalf("unexpected error %#v", err)
+	}
 
 	man := Manifest{
 		Provides:   "test-package",
@@ -59,13 +63,18 @@ func TestManifest_Prepare(t *testing.T) {
 		VersionStr: "0.0.0-rc0",
 	}
 
+	man, err = processManifest(man)
+	if err != nil {
+		t.Fatalf("unexpected error: %#v", err)
+	}
+
 	output := make(chan string, 0)
 	go func() {
 		for _ = range output {
 		}
 	}()
 
-	err := man.Prepare(output)
+	err = man.Prepare(output)
 	if err != nil {
 		t.Errorf("unexpected error: %+v", err)
 	}
@@ -167,3 +176,39 @@ func TestManifests(t *testing.T) {
 		t.Errorf("expected %d services, received %d", expect, received)
 	}
 }
+
+func TestCommands_Initialise(t *testing.T) {
+	cacheDir, _ = ioutil.TempDir("", "")
+
+	for _, test := range []struct {
+		name        string
+		workingDir  string
+		expectError bool
+	}{
+		{"Named dir does not error", "foo-1.0.0-wd", false},
+		{"Empty does not error", "", false},
+		{"Single dot does not erorr", ".", false},
+
+		{"Trying to break out fails", "..", true},
+		{"Trying to go past root fails", "../../../../../../..", true},
+	} {
+		t.Run(test.name, func(t *testing.T) {
+			m := Manifest{
+				Provides:   "foo",
+				VersionStr: "1.0.0",
+				Commands: Commands{
+					WorkingDir: test.workingDir,
+				},
+			}
+
+			m, err := processManifest(m)
+			if err == nil && test.expectError {
+				t.Error("expected error, received none")
+			} else if err != nil && !test.expectError {
+				t.Errorf("unexpected error: %+v", err)
+			}
+
+			t.Log(m.Commands.absoluteWorkingDir)
+		})
+	}
+}