Skip to content

Commit

Permalink
fixed tests & added abort conditions for auto-repo-file
Browse files Browse the repository at this point in the history
also uses cast.ToBool to be on-par with vipers bool parsing from string
  • Loading branch information
jkellerer committed Mar 17, 2024
1 parent 2c0d464 commit 48f391b
Show file tree
Hide file tree
Showing 5 changed files with 131 additions and 28 deletions.
57 changes: 57 additions & 0 deletions config/confidential_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"bytes"
"encoding/json"
"fmt"
"github.com/creativeprojects/resticprofile/platform"
"github.com/creativeprojects/resticprofile/util/templates"
"github.com/stretchr/testify/require"
"os"
Expand Down Expand Up @@ -284,6 +285,9 @@ func TestGetNonConfidentialArgs(t *testing.T) {
}

func TestGetAutoRepositoryFile(t *testing.T) {
require.NoError(t, os.Unsetenv("RESTIC_REPOSITORY"))
require.NoError(t, os.Unsetenv("RESTIC_REPOSITORY_FILE"))

tests := map[string]bool{
"/path/to/file": false,
"file:/path/to/file": false,
Expand All @@ -302,6 +306,10 @@ func TestGetAutoRepositoryFile(t *testing.T) {
require.NoError(t, err)
args := profile.GetCommandFlags(constants.CommandBackup)

if usesFile && platform.IsWindows() {
usesFile = false // Windows has no support for repository file right now (as temp files can't be forced to private with standard go API)
}

if usesFile {
file := templates.TempFile("my-profile-repo.txt")
expected := shell.NewArg(file, shell.ArgConfigEscape).String()
Expand All @@ -318,6 +326,55 @@ func TestGetAutoRepositoryFile(t *testing.T) {
}
}

func TestGetAutoRepositoryFileDisabledWithEnv(t *testing.T) {
if platform.IsWindows() {
t.Skip()
}

tests := []string{
"RESTIC_REPOSITORY",
"RESTIC_REPOSITORY_FILE",
}

for _, envKey := range tests {
t.Run(envKey, func(t *testing.T) {
config := fmt.Sprintf(`
[my-profile]
repository = %q
`, defaultHttpUrl)

defer os.Unsetenv(envKey)
profile, err := getResolvedProfile("toml", config, "my-profile")
require.NoError(t, err)

hasRepoFlag := func() (found bool) {
_, found = profile.GetCommandFlags(constants.CommandBackup).Get("repo")
return
}

t.Run("no-env", func(t *testing.T) {
require.NoError(t, os.Unsetenv(envKey))
profile.Environment = nil
assert.False(t, hasRepoFlag())
})

t.Run("profile-env", func(t *testing.T) {
require.NoError(t, os.Unsetenv(envKey))
profile.Environment = map[string]ConfidentialValue{
envKey: NewConfidentialValue("1"),
}
assert.True(t, hasRepoFlag())
})

t.Run("os-env", func(t *testing.T) {
require.NoError(t, os.Setenv(envKey, "1"))
profile.Environment = nil
assert.True(t, hasRepoFlag())
})
})
}
}

func TestConfidentialToJSON(t *testing.T) {
t.Run("marshal", func(t *testing.T) {
value := NewConfidentialValue("plain")
Expand Down
3 changes: 2 additions & 1 deletion config/global.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"time"

"github.com/creativeprojects/resticprofile/constants"
"github.com/creativeprojects/resticprofile/util/maybe"
)

// Global holds the configuration from the global section
Expand All @@ -15,7 +16,7 @@ type Global struct {
Priority string `mapstructure:"priority" default:"normal" enum:"idle;background;low;normal;high;highest" description:"Sets process priority class for resticprofile and child processes (on any OS)"`
DefaultCommand string `mapstructure:"default-command" default:"snapshots" description:"The restic or resticprofile command to use when no command was specified"`
Initialize bool `mapstructure:"initialize" default:"false" description:"Initialize a repository if missing"`
NoAutoRepositoryFile bool `mapstructure:"prevent-auto-repository-file" default:"false" description:"Prevents using a repository file for repository definitions containing a password"`
NoAutoRepositoryFile maybe.Bool `mapstructure:"prevent-auto-repository-file" default:"false" description:"Prevents using a repository file for repository definitions containing a password"`
ResticBinary string `mapstructure:"restic-binary" description:"Full path of the restic executable (detected if not set)"`
ResticVersion string // not configurable at the moment. To be set after ResticBinary is known.
FilterResticFlags bool `mapstructure:"restic-arguments-filter" default:"true" description:"Remove unknown flags instead of passing all configured flags to restic"`
Expand Down
39 changes: 27 additions & 12 deletions config/profile.go
Original file line number Diff line number Diff line change
Expand Up @@ -752,24 +752,39 @@ func (p *Profile) replaceWithRepositoryFile(repository *ConfidentialValue, repos
*repositoryFile = origFile
}

if !p.hasConfig() || p.config.mustGetGlobalSection().NoAutoRepositoryFile {
// abort if repo is not confidential or a file is in use already
if !repository.IsConfidential() || len(origFile) > 0 {
return
}

if repository.IsConfidential() && len(*repositoryFile) == 0 {
file, err := templates.PrivateTempFile(fmt.Sprintf("%s%s-repo.txt", p.Name, suffix))
if err != nil {
clog.Debugf(`private file %s not supported: %s`, file, err.Error())
// abort by global config or environment
var global *Global
if p.hasConfig() {
global = p.config.mustGetGlobalSection()
}
if global == nil || global.NoAutoRepositoryFile.IsTrue() {
return
} else if global.NoAutoRepositoryFile.IsUndefined() {
env := p.GetEnvironment(true)
if len(env.Get("RESTIC_REPOSITORY")) > 0 || len(env.Get("RESTIC_REPOSITORY_FILE")) > 0 {
clog.Debug("restic repository is set using environment variables, not replacing plain \"repository\" argument")
return
}

Check warning on line 772 in config/profile.go

View check run for this annotation

Codecov / codecov/patch

config/profile.go#L772

Added line #L772 was not covered by tests
}

if err = os.WriteFile(file, []byte(origRepo.Value()), 0600); err == nil {
clog.Debugf(`replaced plain "repository" argument with "repository-file" (%s) to avoid password leak`, file)
*repository = NewConfidentialValue("")
*repositoryFile = file
} else {
clog.Debugf(`failed writing %s: %s`, file, err.Error())
}
// apply temporary change
file, err := templates.PrivateTempFile(fmt.Sprintf("%s%s-repo.txt", p.Name, suffix))
if err != nil {
clog.Debugf(`private file %s not supported: %s`, file, err.Error())
return
}

if err = os.WriteFile(file, []byte(origRepo.Value()), 0600); err == nil {
clog.Debugf(`replaced plain "repository" argument with "repository-file" (%s) to avoid password leak`, file)
*repository = NewConfidentialValue("")
*repositoryFile = file
} else {
clog.Debugf(`failed writing %s: %s`, file, err.Error())
}

Check warning on line 788 in config/profile.go

View check run for this annotation

Codecov / codecov/patch

config/profile.go#L787-L788

Added lines #L787 - L788 were not covered by tests
return
}
Expand Down
29 changes: 20 additions & 9 deletions util/maybe/bool.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
package maybe

import (
"errors"
"github.com/spf13/cast"
"reflect"
"strconv"
)

type Bool struct {
Expand Down Expand Up @@ -45,17 +48,25 @@ func BoolFromNilable(value *bool) Bool {

// BoolDecoder implements config parsing for maybe.Bool
func BoolDecoder() func(from reflect.Type, to reflect.Type, data interface{}) (interface{}, error) {
boolValueType := reflect.TypeOf(Bool{})
fromType := reflect.TypeOf(true)
valueType := reflect.TypeOf(Bool{})

return func(from reflect.Type, to reflect.Type, data interface{}) (interface{}, error) {
if from != reflect.TypeOf(true) || to != boolValueType {
return data, nil
return func(from reflect.Type, to reflect.Type, data interface{}) (result interface{}, err error) {
result = data
if to != valueType {
return
}
boolValue, ok := data.(bool)
if !ok {
// it should never happen
return data, nil

if value, e := cast.ToBoolE(data); e == nil {
from = fromType
data = value
} else if errors.Is(e, new(strconv.NumError)) {
err = e
}

Check warning on line 65 in util/maybe/bool.go

View check run for this annotation

Codecov / codecov/patch

util/maybe/bool.go#L64-L65

Added lines #L64 - L65 were not covered by tests

if err == nil && from == fromType {
result = SetBool(data.(bool))
}
return Bool{Set(boolValue)}, nil
return
}
}
31 changes: 25 additions & 6 deletions util/maybe/bool_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package maybe_test

import (
"fmt"
"reflect"
"testing"

Expand Down Expand Up @@ -77,10 +78,10 @@ func TestBoolDecoder(t *testing.T) {
expected any
}{
{
from: reflect.TypeOf(""),
from: reflect.TypeOf(struct{}{}),
to: reflect.TypeOf(maybe.Bool{}),
source: true,
expected: true, // same value returned as the "from" type in unexpected
source: struct{}{},
expected: struct{}{}, // same value returned as the "from" type in unexpected
},
{
from: reflect.TypeOf(true),
Expand All @@ -89,11 +90,29 @@ func TestBoolDecoder(t *testing.T) {
expected: false, // same value returned as the "to" type in unexpected
},
{
from: reflect.TypeOf(true),
from: reflect.TypeOf(""),
to: reflect.TypeOf(maybe.Bool{}),
source: "",
expected: "", // same value returned as the original value in unexpected
},
{
from: reflect.TypeOf(""),
to: reflect.TypeOf(maybe.Bool{}),
source: "true",
expected: maybe.True(),
},
{
from: reflect.TypeOf(""),
to: reflect.TypeOf(maybe.Bool{}),
source: "t",
expected: maybe.True(),
},
{
from: reflect.TypeOf(""),
to: reflect.TypeOf(maybe.Bool{}),
source: "f",
expected: maybe.False(),
},
{
from: reflect.TypeOf(true),
to: reflect.TypeOf(maybe.Bool{}),
Expand All @@ -107,8 +126,8 @@ func TestBoolDecoder(t *testing.T) {
expected: maybe.False(),
},
}
for _, fixture := range fixtures {
t.Run("", func(t *testing.T) {
for i, fixture := range fixtures {
t.Run(fmt.Sprintf("%d", i), func(t *testing.T) {
decoder := maybe.BoolDecoder()
decoded, err := decoder(fixture.from, fixture.to, fixture.source)
assert.NoError(t, err)
Expand Down

0 comments on commit 48f391b

Please sign in to comment.