Merge pull request #69 from 99designs/percent-encode-slash

Percent-encode filenames with slash

Author: mtibben

file.go

@@ -11,6 +11,7 @@ import (
 
 	jose "github.com/dvsekhvalnov/jose2go"
 	homedir "github.com/mitchellh/go-homedir"
+	"github.com/mtibben/percent"
 )
 
 func init() {
@@ -22,6 +23,11 @@ func init() {
 	})
 }
 
+var filenameEscape = func(s string) string {
+	return percent.Encode(s, "/")
+}
+var filenameUnescape = percent.Decode
+
 type fileKeyring struct {
 	dir          string
 	passwordFunc PromptFunc
@@ -73,12 +79,12 @@ func (k *fileKeyring) unlock() error {
 }
 
 func (k *fileKeyring) Get(key string) (Item, error) {
-	dir, err := k.resolveDir()
+	filename, err := k.filename(key)
 	if err != nil {
 		return Item{}, err
 	}
 
-	bytes, err := ioutil.ReadFile(filepath.Join(dir, key))
+	bytes, err := ioutil.ReadFile(filename)
 	if os.IsNotExist(err) {
 		return Item{}, ErrKeyNotFound
 	} else if err != nil {
@@ -101,12 +107,12 @@ func (k *fileKeyring) Get(key string) (Item, error) {
 }
 
 func (k *fileKeyring) GetMetadata(key string) (Metadata, error) {
-	dir, err := k.resolveDir()
+	filename, err := k.filename(key)
 	if err != nil {
 		return Metadata{}, err
 	}
 
-	stat, err := os.Stat(filepath.Join(dir, key))
+	stat, err := os.Stat(filename)
 	if os.IsNotExist(err) {
 		return Metadata{}, ErrKeyNotFound
 	} else if err != nil {
@@ -131,11 +137,6 @@ func (k *fileKeyring) Set(i Item) error {
 		return err
 	}
 
-	dir, err := k.resolveDir()
-	if err != nil {
-		return err
-	}
-
 	if err = k.unlock(); err != nil {
 		return err
 	}
@@ -148,16 +149,29 @@ func (k *fileKeyring) Set(i Item) error {
 		return err
 	}
 
-	return ioutil.WriteFile(filepath.Join(dir, i.Key), []byte(token), 0600)
+	filename, err := k.filename(i.Key)
+	if err != nil {
+		return err
+	}
+	return ioutil.WriteFile(filename, []byte(token), 0600)
 }
 
-func (k *fileKeyring) Remove(key string) error {
+func (k *fileKeyring) filename(key string) (string, error) {
 	dir, err := k.resolveDir()
+	if err != nil {
+		return "", err
+	}
+
+	return filepath.Join(dir, filenameEscape(key)), nil
+}
+
+func (k *fileKeyring) Remove(key string) error {
+	filename, err := k.filename(key)
 	if err != nil {
 		return err
 	}
 
-	return os.Remove(filepath.Join(dir, key))
+	return os.Remove(filename)
 }
 
 func (k *fileKeyring) Keys() ([]string, error) {
@@ -169,7 +183,7 @@ func (k *fileKeyring) Keys() ([]string, error) {
 	var keys = []string{}
 	files, _ := ioutil.ReadDir(dir)
 	for _, f := range files {
-		keys = append(keys, f.Name())
+		keys = append(keys, filenameUnescape(f.Name()))
 	}
 
 	return keys, nil

file_test.go

@@ -29,3 +29,33 @@ func TestFileKeyringSetWhenEmpty(t *testing.T) {
 		t.Fatalf("Key wasn't persisted: %q", foundItem.Key)
 	}
 }
+
+func TestFileKeyringGetWithSlashes(t *testing.T) {
+	k := &fileKeyring{
+		dir:          os.TempDir(),
+		passwordFunc: fixedStringPrompt("no more secrets"),
+	}
+
+	item := Item{Key: "https://aws-sso-portal.awsapps.com/start", Data: []byte("https://aws-sso-portal.awsapps.com/start")}
+
+	if err := k.Set(item); err != nil {
+		t.Fatal(err)
+	}
+
+	if err := k.Remove(item.Key); err != nil {
+		t.Fatal(err)
+	}
+}
+
+func TestFilenameWithBadChars(t *testing.T) {
+	a := `abc/.././123`
+	e := filenameEscape(a)
+	if e != `abc%2F..%2F.%2F123` {
+		t.Fatalf("Unexpected result from filenameEscape: %s", e)
+	}
+
+	b := filenameUnescape(e)
+	if b != a {
+		t.Fatal("Unexpected filenameEscape")
+	}
+}

go.mod

@@ -10,6 +10,7 @@ require (
 	github.com/keybase/go-keychain v0.0.0-20190712205309-48d3d31d256d
 	github.com/kr/pretty v0.1.0 // indirect
 	github.com/mitchellh/go-homedir v1.1.0
+	github.com/mtibben/percent v0.2.1
 	github.com/stretchr/objx v0.2.0 // indirect
 	golang.org/x/crypto v0.0.0-20190701094942-4def268fd1a4
 	golang.org/x/sys v0.0.0-20190712062909-fae7ac547cb7 // indirect

go.sum

@@ -1,3 +1,5 @@
+github.com/99designs/go-keychain v0.0.0-20191008050251-8e49817e8af4 h1:/vQbFIOMbk2FiG/kXiLl8BRyzTWDw7gX/Hz7Dd5eDMs=
+github.com/99designs/go-keychain v0.0.0-20191008050251-8e49817e8af4/go.mod h1:hN7oaIRCjzsZ2dE+yG5k+rsdt3qcwykqK6HVGcKwsw4=
 github.com/danieljoos/wincred v1.0.2 h1:zf4bhty2iLuwgjgpraD2E9UbvO+fe54XXGJbOwe23fU=
 github.com/danieljoos/wincred v1.0.2/go.mod h1:SnuYRW9lp1oJrZX/dXJqr0cPK5gYXqx3EJbmjhLdK9U=
 github.com/davecgh/go-spew v1.1.0 h1:ZDRjVQ15GmhC3fiQ8ni8+OwkZQO4DARzQgrnXU1Liz8=
@@ -19,6 +21,10 @@ github.com/kr/text v0.1.0 h1:45sCR5RtlFHMR4UwH9sdQ5TC8v0qDQCHnXt+kaKSTVE=
 github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI=
 github.com/mitchellh/go-homedir v1.1.0 h1:lukF9ziXFxDFPkA1vsr5zpc1XuPDn/wFntq5mG+4E0Y=
 github.com/mitchellh/go-homedir v1.1.0/go.mod h1:SfyaCUpYCn1Vlf4IUYiD9fPX4A5wJrkLzIz1N1q0pr0=
+github.com/mtibben/percent v0.2.0 h1:3A04NiaCmcLX6jkb4zCYIABXWJydnjhE+tzGlzNjdss=
+github.com/mtibben/percent v0.2.0/go.mod h1:KG9uO+SZkUp+VkRHsCdYQV3XSZrrSpR3O9ibNBTZrns=
+github.com/mtibben/percent v0.2.1 h1:5gssi8Nqo8QU/r2pynCm+hBQHpkB/uNK7BJCFogWdzs=
+github.com/mtibben/percent v0.2.1/go.mod h1:KG9uO+SZkUp+VkRHsCdYQV3XSZrrSpR3O9ibNBTZrns=
 github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
 github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
 github.com/stretchr/objx v0.1.0 h1:4G4v2dO3VZwixGIRoQ5Lfboy6nUhCyYzaqnIAPPhYs4=