From 859d578ef4d863b51f5bb3b8661570ed3de23071 Mon Sep 17 00:00:00 2001 From: Michael Tibben Date: Mon, 7 Mar 2022 21:00:13 +1100 Subject: [PATCH] Add golangci-lint and fix linting issues --- .gitattributes | 1 + .github/workflows/lint.yml | 19 ++++++++ .github/workflows/{ci.yml => test.yml} | 20 ++------ .golangci.yml | 39 ++++++++++++++++ array.go | 12 ++--- cmd/keyring/main.go | 1 + config.go | 2 +- docker-compose.yml | 7 +++ go.mod | 3 +- go.sum | 8 ---- keychain_test.go | 24 +++++----- keyctl.go | 65 +++++++++++++------------- keyctl_test.go | 4 +- keyring.go | 22 ++++----- keyring_test.go | 3 ++ kwallet.go | 3 +- pass.go | 30 ++++-------- pass_test.go | 14 ++++-- prompt.go | 6 +-- secretservice.go | 16 ++----- secretservice_test.go | 3 +- tilde.go | 3 +- 22 files changed, 165 insertions(+), 140 deletions(-) create mode 100644 .gitattributes create mode 100644 .github/workflows/lint.yml rename .github/workflows/{ci.yml => test.yml} (54%) create mode 100644 .golangci.yml create mode 100644 docker-compose.yml diff --git a/.gitattributes b/.gitattributes new file mode 100644 index 0000000..d207b18 --- /dev/null +++ b/.gitattributes @@ -0,0 +1 @@ +*.go text eol=lf diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml new file mode 100644 index 0000000..f8ee58a --- /dev/null +++ b/.github/workflows/lint.yml @@ -0,0 +1,19 @@ +name: golangci-lint +on: push +jobs: + golangci: + strategy: + matrix: + go-version: [1.15.x] + os: [macos-latest, windows-latest, ubuntu-latest] + name: lint + runs-on: ${{ matrix.os }} + steps: + - uses: actions/checkout@v2 + - uses: actions/setup-go@v2 + with: + go-version: 1.17 + - name: golangci-lint + uses: golangci/golangci-lint-action@v2 + with: + version: v1.44.2 diff --git a/.github/workflows/ci.yml b/.github/workflows/test.yml similarity index 54% rename from .github/workflows/ci.yml rename to .github/workflows/test.yml index 440ff04..1e81147 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/test.yml @@ -1,8 +1,6 @@ name: Continuous Integration on: push - jobs: - linux: runs-on: ubuntu-latest steps: @@ -10,26 +8,14 @@ jobs: with: go-version: 1.17 - run: sudo apt-get install pass gnome-keyring dbus-x11 - - uses: actions/checkout@v1 + - uses: actions/checkout@v2 - run: go test -race ./... - - run: go vet ./... - mac: - runs-on: macOS-latest + runs-on: macos-latest steps: - uses: actions/setup-go@v2 with: go-version: 1.17 - run: brew install pass gnupg - - uses: actions/checkout@v1 + - uses: actions/checkout@v2 - run: go test -race ./... - - run: go vet ./... - - lint: - runs-on: ubuntu-latest - steps: - - uses: actions/setup-go@v2 - with: - go-version: 1.17 - - uses: actions/checkout@v1 - - run: diff -u <(echo -n) <(gofmt -s -d .) diff --git a/.golangci.yml b/.golangci.yml new file mode 100644 index 0000000..d5b9097 --- /dev/null +++ b/.golangci.yml @@ -0,0 +1,39 @@ +linters: + enable: + - bodyclose + - contextcheck + - deadcode + - depguard + - durationcheck + - dupl + - errcheck + - errchkjson + - errname + - exhaustive + - exportloopref + - gocritic + - gofmt + - goimports + - gosimple + - govet + - ineffassign + - makezero + - misspell + - nakedret + - nilerr + - nilnil + - noctx + - prealloc + - revive + - rowserrcheck + - staticcheck + - structcheck + - thelper + - tparallel + - typecheck + - unconvert + - unparam + - unused + - varcheck + - wastedassign + - whitespace diff --git a/array.go b/array.go index fca61b8..3179cb5 100644 --- a/array.go +++ b/array.go @@ -2,13 +2,13 @@ package keyring // ArrayKeyring is a mock/non-secure backend that meets the Keyring interface. // It is intended to be used to aid unit testing of code that relies on the package. -// NOTE: Do not use in production code +// NOTE: Do not use in production code. type ArrayKeyring struct { items map[string]Item } // NewArrayKeyring returns an ArrayKeyring, optionally constructed with an initial slice -// of items +// of items. func NewArrayKeyring(initial []Item) *ArrayKeyring { kr := &ArrayKeyring{} for _, i := range initial { @@ -17,7 +17,7 @@ func NewArrayKeyring(initial []Item) *ArrayKeyring { return kr } -// Get returns an Item matching Key +// Get returns an Item matching Key. func (k *ArrayKeyring) Get(key string) (Item, error) { if i, ok := k.items[key]; ok { return i, nil @@ -25,7 +25,7 @@ func (k *ArrayKeyring) Get(key string) (Item, error) { return Item{}, ErrKeyNotFound } -// Set will store an item on the mock Keyring +// Set will store an item on the mock Keyring. func (k *ArrayKeyring) Set(i Item) error { if k.items == nil { k.items = map[string]Item{} @@ -34,13 +34,13 @@ func (k *ArrayKeyring) Set(i Item) error { return nil } -// Remove will delete an Item from the Keyring +// Remove will delete an Item from the Keyring. func (k *ArrayKeyring) Remove(key string) error { delete(k.items, key) return nil } -// Keys provides a slice of all Item keys on the Keyring +// Keys provides a slice of all Item keys on the Keyring. func (k *ArrayKeyring) Keys() ([]string, error) { var keys = []string{} for key := range k.items { diff --git a/cmd/keyring/main.go b/cmd/keyring/main.go index 25f3c04..7cf88c7 100644 --- a/cmd/keyring/main.go +++ b/cmd/keyring/main.go @@ -104,5 +104,6 @@ func hasBackend(key string) bool { return true } } + return false } diff --git a/config.go b/config.go index c942409..590af7c 100644 --- a/config.go +++ b/config.go @@ -1,6 +1,6 @@ package keyring -// Config contains configuration for keyring +// Config contains configuration for keyring. type Config struct { // AllowedBackends is a whitelist of backend providers that can be used. Nil means all available. AllowedBackends []BackendType diff --git a/docker-compose.yml b/docker-compose.yml new file mode 100644 index 0000000..8319281 --- /dev/null +++ b/docker-compose.yml @@ -0,0 +1,7 @@ +version: "3.9" +services: + keyring: + image: golang:1.17 + volumes: + - .:/usr/local/src/keyring + working_dir: /usr/local/src/keyring diff --git a/go.mod b/go.mod index 65874cc..c421d71 100644 --- a/go.mod +++ b/go.mod @@ -10,14 +10,13 @@ require ( github.com/gsterjov/go-libsecret v0.0.0-20161001094733-a6f4afe4910c github.com/mtibben/percent v0.2.1 github.com/stretchr/testify v1.7.0 - golang.org/x/crypto v0.0.0-20220131195533-30dcbda58838 golang.org/x/sys v0.0.0-20220204135822-1c1b9b1eba6a + golang.org/x/term v0.0.0-20210927222741-03fcf44c2211 ) require ( github.com/davecgh/go-spew v1.1.1 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect github.com/stretchr/objx v0.3.0 // indirect - golang.org/x/term v0.0.0-20210927222741-03fcf44c2211 // indirect gopkg.in/yaml.v3 v3.0.0-20210107192922-496545a6307b // indirect ) diff --git a/go.sum b/go.sum index dc1fd90..09fbcb2 100644 --- a/go.sum +++ b/go.sum @@ -26,20 +26,12 @@ github.com/stretchr/objx v0.3.0/go.mod h1:qt09Ya8vawLte6SNmTgCsAVtYtaKzEcn8ATUoH github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI= github.com/stretchr/testify v1.7.0 h1:nwc3DEeHmmLAfoZucVR881uASk0Mfjw8xYJ99tb5CcY= github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= -golang.org/x/crypto v0.0.0-20220131195533-30dcbda58838 h1:71vQrMauZZhcTVK6KdYM+rklehEEwb3E+ZhaE5jrPrE= -golang.org/x/crypto v0.0.0-20220131195533-30dcbda58838/go.mod h1:IxCIyHEi3zRg3s0A5j5BB6A9Jmi73HwBIUl50j+osU4= -golang.org/x/net v0.0.0-20211112202133-69e39bad7dc2/go.mod h1:9nx3DQGgdP8bBQD5qxJ1jj9UTztislL4KSBs9R2vV5Y= -golang.org/x/sys v0.0.0-20201119102817-f84b799fce68/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= -golang.org/x/sys v0.0.0-20210423082822-04245dca01da/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20210615035016-665e8c7367d1/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20210819135213-f52c844e1c1c/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20220204135822-1c1b9b1eba6a h1:ppl5mZgokTT8uPkmYOyEUmPTr3ypaKkg5eFOGrAmxxE= golang.org/x/sys v0.0.0-20220204135822-1c1b9b1eba6a/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= golang.org/x/term v0.0.0-20210927222741-03fcf44c2211 h1:JGgROgKl9N8DuW20oFS5gxc+lE67/N3FcwmBPMe7ArY= golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8= -golang.org/x/text v0.3.6/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= -golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/check.v1 v1.0.0-20200902074654-038fdea0a05b h1:QRR6H1YWRnHb4Y/HeNFCTJLFVxaq6wH4YuVdsUOr75U= gopkg.in/check.v1 v1.0.0-20200902074654-038fdea0a05b/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= diff --git a/keychain_test.go b/keychain_test.go index 887db7d..cf0a25f 100644 --- a/keychain_test.go +++ b/keychain_test.go @@ -14,7 +14,7 @@ import ( func TestOSXKeychainKeyringSet(t *testing.T) { path := tempPath() - defer deleteKeychain(path, t) + defer deleteKeychain(t, path) k := &keychain{ path: path, @@ -43,18 +43,18 @@ func TestOSXKeychainKeyringSet(t *testing.T) { t.Fatalf("Data stored was not the data retrieved: %q vs %q", v.Data, item.Data) } - if string(v.Key) != item.Key { + if v.Key != item.Key { t.Fatalf("Key stored was not the data retrieved: %q vs %q", v.Key, item.Key) } - if string(v.Description) != item.Description { + if v.Description != item.Description { t.Fatalf("Description stored was not the data retrieved: %q vs %q", v.Description, item.Description) } } func TestOSXKeychainKeyringOverwrite(t *testing.T) { path := tempPath() - defer deleteKeychain(path, t) + defer deleteKeychain(t, path) k := &keychain{ path: path, @@ -106,7 +106,7 @@ func TestOSXKeychainKeyringOverwrite(t *testing.T) { func TestOSXKeychainKeyringListKeysWhenEmpty(t *testing.T) { path := tempPath() - defer deleteKeychain(path, t) + defer deleteKeychain(t, path) k := &keychain{ path: path, @@ -126,7 +126,7 @@ func TestOSXKeychainKeyringListKeysWhenEmpty(t *testing.T) { func TestOSXKeychainKeyringListKeysWhenNotEmpty(t *testing.T) { path := tempPath() - defer deleteKeychain(path, t) + defer deleteKeychain(t, path) k := &keychain{ path: path, @@ -158,7 +158,9 @@ func TestOSXKeychainKeyringListKeysWhenNotEmpty(t *testing.T) { } } -func deleteKeychain(path string, t *testing.T) { +func deleteKeychain(t *testing.T, path string) { + t.Helper() + if _, err := os.Stat(path); os.IsExist(err) { _ = os.Remove(path) } @@ -172,7 +174,7 @@ func deleteKeychain(path string, t *testing.T) { func TestOSXKeychainGetKeyWhenEmpty(t *testing.T) { path := tempPath() - defer deleteKeychain(path, t) + defer deleteKeychain(t, path) k := &keychain{ path: path, @@ -189,7 +191,7 @@ func TestOSXKeychainGetKeyWhenEmpty(t *testing.T) { func TestOSXKeychainGetKeyWhenNotEmpty(t *testing.T) { path := tempPath() - defer deleteKeychain(path, t) + defer deleteKeychain(t, path) k := &keychain{ path: path, @@ -219,7 +221,7 @@ func TestOSXKeychainGetKeyWhenNotEmpty(t *testing.T) { func TestOSXKeychainRemoveKeyWhenEmpty(t *testing.T) { path := tempPath() - defer deleteKeychain(path, t) + defer deleteKeychain(t, path) k := &keychain{ path: path, @@ -236,7 +238,7 @@ func TestOSXKeychainRemoveKeyWhenEmpty(t *testing.T) { func TestOSXKeychainRemoveKeyWhenNotEmpty(t *testing.T) { path := tempPath() - defer deleteKeychain(path, t) + defer deleteKeychain(t, path) k := &keychain{ path: path, diff --git a/keyctl.go b/keyctl.go index 22ee4f0..bd8018a 100644 --- a/keyctl.go +++ b/keyctl.go @@ -13,6 +13,7 @@ import ( "golang.org/x/sys/unix" ) +//nolint:revive const ( KEYCTL_PERM_VIEW = uint32(1 << 0) KEYCTL_PERM_READ = uint32(1 << 1) @@ -28,7 +29,7 @@ const ( KEYCTL_PERM_PROCESS = 24 ) -// GetPermissions constructs the permission mask from the elements +// GetPermissions constructs the permission mask from the elements. func GetPermissions(process, user, group, others uint32) uint32 { perm := others << KEYCTL_PERM_OTHERS perm |= group << KEYCTL_PERM_GROUP @@ -38,7 +39,7 @@ func GetPermissions(process, user, group, others uint32) uint32 { return perm } -// GetKeyringIDForScope get the keyring ID for a given scope +// GetKeyringIDForScope get the keyring ID for a given scope. func GetKeyringIDForScope(scope string) (int32, error) { ringRef, err := getKeyringForScope(scope) if err != nil { @@ -68,7 +69,7 @@ func init() { // Check for named keyrings keyring.keyring = parent if cfg.ServiceName != "" { - namedKeyring, err := keyctl_search(parent, "keyring", cfg.ServiceName) + namedKeyring, err := keyctlSearch(parent, "keyring", cfg.ServiceName) if err != nil { if !errors.Is(err, syscall.ENOKEY) { return nil, fmt.Errorf("opening named %q keyring failed: %v", cfg.KeyCtlScope, err) @@ -88,7 +89,7 @@ func init() { } func (k *keyctlKeyring) Get(name string) (Item, error) { - key, err := keyctl_search(k.keyring, "user", name) + key, err := keyctlSearch(k.keyring, "user", name) if err != nil { if errors.Is(err, syscall.ENOKEY) { return Item{}, ErrKeyNotFound @@ -96,7 +97,7 @@ func (k *keyctlKeyring) Get(name string) (Item, error) { return Item{}, err } // data, err := key.Get() - data, err := keyctl_read(key) + data, err := keyctlRead(key) if err != nil { return Item{}, err } @@ -118,7 +119,7 @@ func (k *keyctlKeyring) GetMetadata(_ string) (Metadata, error) { func (k *keyctlKeyring) Set(item Item) error { if k.perm == 0 { // Keep the default permissions (alswrv-----v------------) - _, err := keyctl_add(k.keyring, "user", item.Key, item.Data) + _, err := keyctlAdd(k.keyring, "user", item.Key, item.Data) return err } @@ -127,20 +128,20 @@ func (k *keyctlKeyring) Set(item Item) error { // cannot change the permissions without possessing the key. Therefore, create the // key in the session keyring, change permissions and then link to the target // keyring and unlink from the intermediate keyring again. - key, err := keyctl_add(unix.KEY_SPEC_SESSION_KEYRING, "user", item.Key, item.Data) + key, err := keyctlAdd(unix.KEY_SPEC_SESSION_KEYRING, "user", item.Key, item.Data) if err != nil { return fmt.Errorf("adding key to session failed: %v", err) } - if err := keyctl_setperm(key, k.perm); err != nil { + if err := keyctlSetperm(key, k.perm); err != nil { return fmt.Errorf("setting permission 0x%x failed: %v", k.perm, err) } - if err := keyctl_link(k.keyring, key); err != nil { + if err := keyctlLink(k.keyring, key); err != nil { return fmt.Errorf("linking key to keyring failed: %v", err) } - if err := keyctl_unlink(unix.KEY_SPEC_SESSION_KEYRING, key); err != nil { + if err := keyctlUnlink(unix.KEY_SPEC_SESSION_KEYRING, key); err != nil { return fmt.Errorf("unlinking key from session failed: %v", err) } @@ -148,28 +149,28 @@ func (k *keyctlKeyring) Set(item Item) error { } func (k *keyctlKeyring) Remove(name string) error { - key, err := keyctl_search(k.keyring, "user", name) + key, err := keyctlSearch(k.keyring, "user", name) if err != nil { return ErrKeyNotFound } - return keyctl_unlink(k.keyring, key) + return keyctlUnlink(k.keyring, key) } func (k *keyctlKeyring) Keys() ([]string, error) { results := []string{} - data, err := keyctl_read(k.keyring) + data, err := keyctlRead(k.keyring) if err != nil { return nil, fmt.Errorf("reading keyring failed: %v", err) } - ids, err := keyctl_convertKeyBuffer(data) + ids, err := keyctlConvertKeyBuffer(data) if err != nil { return nil, fmt.Errorf("converting raw keylist failed: %v", err) } for _, id := range ids { - info, err := keyctl_describe(id) + info, err := keyctlDescribe(id) if err != nil { return nil, err } @@ -181,11 +182,10 @@ func (k *keyctlKeyring) Keys() ([]string, error) { return results, nil } -// INTERNAL FUNCTIONS func (k *keyctlKeyring) createNamedKeyring(parent int32, name string) (int32, error) { if k.perm == 0 { // Keep the default permissions (alswrv-----v------------) - return keyctl_add(parent, "keyring", name, nil) + return keyctlAdd(parent, "keyring", name, nil) } // By default we loose possession of the keyring in anything above the session keyring. @@ -193,20 +193,20 @@ func (k *keyctlKeyring) createNamedKeyring(parent int32, name string) (int32, er // cannot change the permissions without possessing the keyring. Therefore, create the // keyring linked to the session keyring, change permissions and then link to the target // keyring and unlink from the intermediate keyring again. - keyring, err := keyctl_add(unix.KEY_SPEC_SESSION_KEYRING, "keyring", name, nil) + keyring, err := keyctlAdd(unix.KEY_SPEC_SESSION_KEYRING, "keyring", name, nil) if err != nil { return 0, fmt.Errorf("creating keyring failed: %v", err) } - if err := keyctl_setperm(keyring, k.perm); err != nil { + if err := keyctlSetperm(keyring, k.perm); err != nil { return 0, fmt.Errorf("setting permission 0x%x failed: %v", k.perm, err) } - if err := keyctl_link(k.keyring, keyring); err != nil { + if err := keyctlLink(k.keyring, keyring); err != nil { return 0, fmt.Errorf("linking keyring failed: %v", err) } - if err := keyctl_unlink(unix.KEY_SPEC_SESSION_KEYRING, keyring); err != nil { + if err := keyctlUnlink(unix.KEY_SPEC_SESSION_KEYRING, keyring); err != nil { return 0, fmt.Errorf("unlinking keyring from session failed: %v", err) } @@ -233,8 +233,7 @@ func getKeyringForScope(scope string) (int32, error) { return 0, fmt.Errorf("unknown scope %q", scope) } -// INTERNAL KEYCTL ABSTRACTION FUNCTIONS -func keyctl_add(parent int32, keytype, key string, data []byte) (int32, error) { +func keyctlAdd(parent int32, keytype, key string, data []byte) (int32, error) { id, err := unix.AddKey(keytype, key, data, int(parent)) if err != nil { return 0, err @@ -242,7 +241,7 @@ func keyctl_add(parent int32, keytype, key string, data []byte) (int32, error) { return int32(id), nil } -func keyctl_search(id int32, idtype, name string) (int32, error) { +func keyctlSearch(id int32, idtype, name string) (int32, error) { key, err := unix.KeyctlSearch(int(id), idtype, name, 0) if err != nil { return 0, err @@ -250,7 +249,7 @@ func keyctl_search(id int32, idtype, name string) (int32, error) { return int32(key), nil } -func keyctl_read(id int32) ([]byte, error) { +func keyctlRead(id int32) ([]byte, error) { var buffer []byte for { @@ -269,7 +268,7 @@ func keyctl_read(id int32) ([]byte, error) { } } -func keyctl_describe(id int32) (map[string]string, error) { +func keyctlDescribe(id int32) (map[string]string, error) { description, err := unix.KeyctlString(unix.KEYCTL_DESCRIBE, int(id)) if err != nil { return nil, err @@ -280,8 +279,8 @@ func keyctl_describe(id int32) (map[string]string, error) { } data := make(map[string]string) - names := []string{"type", "uid", "gid", "perm"} // according to keyctl_describe(3) new fields are added at the end - data["description"] = fields[len(fields)-1] // according to keyctl_describe(3) description is always last + names := []string{"type", "uid", "gid", "perm"} // according to keyctlDescribe(3) new fields are added at the end + data["description"] = fields[len(fields)-1] // according to keyctlDescribe(3) description is always last for i, f := range fields[:len(fields)-1] { if i >= len(names) { // Do not stumble upon unknown fields @@ -293,7 +292,7 @@ func keyctl_describe(id int32) (map[string]string, error) { return data, nil } -func keyctl_link(parent, child int32) error { +func keyctlLink(parent, child int32) error { _, _, errno := syscall.Syscall(syscall.SYS_KEYCTL, uintptr(unix.KEYCTL_LINK), uintptr(child), uintptr(parent)) if errno != 0 { return errno @@ -301,7 +300,7 @@ func keyctl_link(parent, child int32) error { return nil } -func keyctl_unlink(parent, child int32) error { +func keyctlUnlink(parent, child int32) error { _, _, errno := syscall.Syscall(syscall.SYS_KEYCTL, uintptr(unix.KEYCTL_UNLINK), uintptr(child), uintptr(parent)) if errno != 0 { return errno @@ -309,11 +308,11 @@ func keyctl_unlink(parent, child int32) error { return nil } -func keyctl_setperm(id int32, perm uint32) error { +func keyctlSetperm(id int32, perm uint32) error { return unix.KeyctlSetperm(int(id), perm) } -func keyctl_convertKeyBuffer(buffer []byte) ([]int32, error) { +func keyctlConvertKeyBuffer(buffer []byte) ([]int32, error) { if len(buffer)%4 != 0 { return nil, fmt.Errorf("buffer size %d not a multiple of 4", len(buffer)) } @@ -322,7 +321,7 @@ func keyctl_convertKeyBuffer(buffer []byte) ([]int32, error) { for i := 0; i < len(buffer); i += 4 { // We need to case in host-native endianess here as this is what we get from the kernel. r := *((*int32)(unsafe.Pointer(&buffer[i]))) - results = append(results, int32(r)) + results = append(results, r) } return results, nil } diff --git a/keyctl_test.go b/keyctl_test.go index 4036204..6725dc6 100644 --- a/keyctl_test.go +++ b/keyctl_test.go @@ -35,7 +35,7 @@ func getRandomKeyringName(length int) string { func doesNamedKeyringExist() (bool, error) { ringparentID, err := keyring.GetKeyringIDForScope(ringparent) if err != nil { - return false, nil + return false, nil //nolint:nilerr } _, err = unix.KeyctlSearch(int(ringparentID), "keyring", ringname, 0) @@ -55,7 +55,7 @@ func cleanupNamedKeyring() { if err != nil { return } - _, _, err = syscall.Syscall(syscall.SYS_KEYCTL, uintptr(unix.KEYCTL_UNLINK), uintptr(named), uintptr(ringparentID)) + _, _, _ = syscall.Syscall(syscall.SYS_KEYCTL, uintptr(unix.KEYCTL_UNLINK), uintptr(named), uintptr(ringparentID)) } func TestKeyCtlIsAvailable(t *testing.T) { diff --git a/keyring.go b/keyring.go index 7f892d7..12161b7 100644 --- a/keyring.go +++ b/keyring.go @@ -1,6 +1,4 @@ -// Package keyring provides a uniform API over a range of desktop credential storage engines -// -// See project homepage at https://github.com/99designs/keyring for more background +// Package keyring provides a uniform API over a range of desktop credential storage engines. package keyring import ( @@ -9,10 +7,10 @@ import ( "time" ) -// A BackendType is an identifier for a credential storage service +// BackendType is an identifier for a credential storage service. type BackendType string -// All currently supported secure storage backends +// All currently supported secure storage backends. const ( InvalidBackend BackendType = "" SecretServiceBackend BackendType = "secret-service" @@ -42,7 +40,7 @@ var backendOrder = []BackendType{ var supportedBackends = map[BackendType]opener{} -// AvailableBackends provides a slice of all available backend keys on the current OS +// AvailableBackends provides a slice of all available backend keys on the current OS. func AvailableBackends() []BackendType { b := []BackendType{} for _, k := range backendOrder { @@ -56,7 +54,7 @@ func AvailableBackends() []BackendType { type opener func(cfg Config) (Keyring, error) -// Open will open a specific keyring backend +// Open will open a specific keyring backend. func Open(cfg Config) (Keyring, error) { if cfg.AllowedBackends == nil { cfg.AllowedBackends = AvailableBackends() @@ -75,7 +73,7 @@ func Open(cfg Config) (Keyring, error) { return nil, ErrNoAvailImpl } -// Item is a thing stored on the keyring +// Item is a thing stored on the keyring. type Item struct { Key string Data []byte @@ -97,7 +95,7 @@ type Metadata struct { ModificationTime time.Time } -// Keyring provides the uniform interface over the underlying backends +// Keyring provides the uniform interface over the underlying backends. type Keyring interface { // Returns an Item matching the key or ErrKeyNotFound Get(key string) (Item, error) @@ -111,10 +109,10 @@ type Keyring interface { Keys() ([]string, error) } -// ErrNoAvailImpl is returned by Open when a backend cannot be found +// ErrNoAvailImpl is returned by Open when a backend cannot be found. var ErrNoAvailImpl = errors.New("Specified keyring backend not available") -// ErrKeyNotFound is returned by Keyring Get when the item is not on the keyring +// ErrKeyNotFound is returned by Keyring Get when the item is not on the keyring. var ErrKeyNotFound = errors.New("The specified item could not be found in the keyring") // ErrMetadataNeedsCredentials is returned when Metadata is called against a @@ -125,7 +123,7 @@ var ErrMetadataNeedsCredentials = errors.New("The keyring backend requires crede var ErrMetadataNotSupported = errors.New("The keyring backend does not support metadata access") var ( - // Debug specifies whether to print debugging output + // Debug specifies whether to print debugging output. Debug bool ) diff --git a/keyring_test.go b/keyring_test.go index 1b8bb43..e68c48d 100644 --- a/keyring_test.go +++ b/keyring_test.go @@ -11,6 +11,9 @@ func ExampleOpen() { kr, err := keyring.Open(keyring.Config{ ServiceName: "my-service", }) + if err != nil { + log.Fatal(err) + } v, err := kr.Get("llamas") if err != nil { diff --git a/kwallet.go b/kwallet.go index 08ca473..771baba 100644 --- a/kwallet.go +++ b/kwallet.go @@ -16,7 +16,6 @@ const ( ) func init() { - if os.Getenv("DISABLE_KWALLET") == "1" { return } @@ -172,7 +171,7 @@ func newKwallet() (*kwalletBinding, error) { }, nil } -// Dumb Dbus bindings for kwallet bindings with types +// Dumb Dbus bindings for kwallet bindings with types. type kwalletBinding struct { dbus dbus.BusObject } diff --git a/pass.go b/pass.go index 823666f..5ca0bf0 100644 --- a/pass.go +++ b/pass.go @@ -60,14 +60,14 @@ type passKeyring struct { prefix string } -func (k *passKeyring) pass(args ...string) (*exec.Cmd, error) { +func (k *passKeyring) pass(args ...string) *exec.Cmd { cmd := exec.Command(k.passcmd, args...) if k.dir != "" { cmd.Env = append(os.Environ(), fmt.Sprintf("PASSWORD_STORE_DIR=%s", k.dir)) } cmd.Stderr = os.Stderr - return cmd, nil + return cmd } func (k *passKeyring) Get(key string) (Item, error) { @@ -76,11 +76,7 @@ func (k *passKeyring) Get(key string) (Item, error) { } name := filepath.Join(k.prefix, key) - cmd, err := k.pass("show", name) - if err != nil { - return Item{}, err - } - + cmd := k.pass("show", name) output, err := cmd.Output() if err != nil { return Item{}, err @@ -103,11 +99,7 @@ func (k *passKeyring) Set(i Item) error { } name := filepath.Join(k.prefix, i.Key) - cmd, err := k.pass("insert", "-m", "-f", name) - if err != nil { - return err - } - + cmd := k.pass("insert", "-m", "-f", name) cmd.Stdin = strings.NewReader(string(bytes)) err = cmd.Run() @@ -124,12 +116,8 @@ func (k *passKeyring) Remove(key string) error { } name := filepath.Join(k.prefix, key) - cmd, err := k.pass("rm", "-f", name) - if err != nil { - return err - } - - err = cmd.Run() + cmd := k.pass("rm", "-f", name) + err := cmd.Run() if err != nil { return err } @@ -140,10 +128,8 @@ func (k *passKeyring) Remove(key string) error { func (k *passKeyring) itemExists(key string) bool { var path = filepath.Join(k.dir, k.prefix, key+".gpg") _, err := os.Stat(path) - if err != nil { - return false - } - return true + + return err == nil } func (k *passKeyring) Keys() ([]string, error) { diff --git a/pass_test.go b/pass_test.go index 90db138..267c1b5 100644 --- a/pass_test.go +++ b/pass_test.go @@ -15,6 +15,7 @@ import ( ) func runCmd(t *testing.T, cmds ...string) { + t.Helper() cmd := exec.Command(cmds[0], cmds[1:]...) out, err := cmd.CombinedOutput() if err != nil { @@ -25,6 +26,8 @@ func runCmd(t *testing.T, cmds ...string) { } func setup(t *testing.T) (*passKeyring, func(t *testing.T)) { + t.Helper() + pwd, err := os.Getwd() if err != nil { t.Fatal(err) @@ -38,7 +41,10 @@ func setup(t *testing.T) (*passKeyring, func(t *testing.T)) { // Initialise a blank GPG homedir; import & trust the test key gnupghome := filepath.Join(tmpdir, ".gnupg") - os.Mkdir(gnupghome, os.FileMode(int(0700))) + err = os.Mkdir(gnupghome, os.FileMode(int(0700))) + if err != nil { + t.Fatal(err) + } os.Setenv("GNUPGHOME", gnupghome) os.Unsetenv("GPG_AGENT_INFO") os.Unsetenv("GPG_TTY") @@ -52,10 +58,7 @@ func setup(t *testing.T) (*passKeyring, func(t *testing.T)) { prefix: "keyring", } - cmd, err := k.pass("init", "test@example.com") - if err != nil { - t.Fatal(err) - } + cmd := k.pass("init", "test@example.com") cmd.Stderr = os.Stderr err = cmd.Run() if err != nil { @@ -63,6 +66,7 @@ func setup(t *testing.T) (*passKeyring, func(t *testing.T)) { } return k, func(t *testing.T) { + t.Helper() os.RemoveAll(tmpdir) } } diff --git a/prompt.go b/prompt.go index feb0e7f..59ad81c 100644 --- a/prompt.go +++ b/prompt.go @@ -4,15 +4,15 @@ import ( "fmt" "os" - "golang.org/x/crypto/ssh/terminal" + "golang.org/x/term" ) -// PromptFunc is a function used to prompt the user for a password +// PromptFunc is a function used to prompt the user for a password. type PromptFunc func(string) (string, error) func TerminalPrompt(prompt string) (string, error) { fmt.Printf("%s: ", prompt) - b, err := terminal.ReadPassword(int(os.Stdin.Fd())) + b, err := term.ReadPassword(int(os.Stdin.Fd())) if err != nil { return "", err } diff --git a/secretservice.go b/secretservice.go index 6780a94..2a1f9d9 100644 --- a/secretservice.go +++ b/secretservice.go @@ -50,18 +50,9 @@ type secretsKeyring struct { session *libsecret.Session } -type secretsError struct { - message string -} - -func (e *secretsError) Error() string { - return e.message -} - var errCollectionNotFound = errors.New("The collection does not exist. Please add a key first") func decodeKeyringString(src string) string { - var dst strings.Builder for i := 0; i < len(src); i++ { if src[i] != '_' { @@ -99,7 +90,8 @@ func (k *secretsKeyring) openSecrets() error { for _, collection := range collections { if decodeKeyringString(string(collection.Path())) == path { - k.collection = &collection + c := collection // fix variable into the local variable to ensure it's referenced correctly, see https://github.com/kyoh86/exportloopref + k.collection = &c return nil } } @@ -272,11 +264,9 @@ func (k *secretsKeyring) Keys() ([]string, error) { } keys := []string{} for _, item := range items { - label, err := item.Label() + label, err := item.Label() // FIXME: err is being silently ignored if err == nil { keys = append(keys, label) - } else { - // err is being silently ignored here, not sure if that's good or bad } } return keys, nil diff --git a/secretservice_test.go b/secretservice_test.go index 6c2fe6e..deeb458 100644 --- a/secretservice_test.go +++ b/secretservice_test.go @@ -23,6 +23,7 @@ import ( // and provides the Prompt interface using the go-libsecret library. func libSecretSetup(t *testing.T) (Keyring, func(t *testing.T)) { + t.Helper() if os.Getenv("GITHUB_ACTIONS") != "" { t.Skip("Skipping testing in CI environment") } @@ -36,6 +37,7 @@ func libSecretSetup(t *testing.T) (Keyring, func(t *testing.T)) { service: service, } return kr, func(t *testing.T) { + t.Helper() if err := kr.deleteCollection(); err != nil { t.Fatal(err) } @@ -144,7 +146,6 @@ func TestLibSecretRemoveWhenNotEmpty(t *testing.T) { } func TestLibSpecialCharacters(t *testing.T) { - decoded := decodeKeyringString("keyring_2dtest") if decoded != "keyring-test" { t.Fatal("incorrect decodeKeyringString") diff --git a/tilde.go b/tilde.go index d3b9d54..c1847a6 100644 --- a/tilde.go +++ b/tilde.go @@ -6,10 +6,9 @@ import ( "strings" ) -// ~/ or ~\ depending on OS var tildePrefix = string([]rune{'~', filepath.Separator}) -// expand tilde for home directory +// ExpandTilde will expand tilde (~/ or ~\ depending on OS) for the user home directory. func ExpandTilde(dir string) (string, error) { if strings.HasPrefix(dir, tildePrefix) { homeDir, err := os.UserHomeDir()