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
13 changes: 9 additions & 4 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,15 @@ jobs:
- name: Test
run: go test -v -race -coverprofile=coverage.out ./...
# Standard consumer build configuration (working-with-secrets.md
# §1.10): credstore must stay green with the keyring opt-out tags
# the CLIs ship with. keyring_nofile / keyring_nopass are NOT in
# the set — credstore exposes those backends in cgo builds.
- name: Build and test with keyring opt-out tags
# §1.10): 1Password support is enabled, Passage is disabled.
- name: Build and test with standard keyring tags
run: |
go build -tags keyring_nopassage ./...
go test -race -tags keyring_nopassage ./...
# Optional compliance/minimal-dependency build configuration:
# 1Password support is compiled out but exposed file/pass backends
# remain enabled.
- name: Build and test with no-1Password opt-out tags
run: |
go build -tags keyring_no1password,keyring_nopassage ./...
go test -race -tags keyring_no1password,keyring_nopassage ./...
Expand Down
35 changes: 33 additions & 2 deletions credstore/flag_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,9 @@ func TestAllBackends_MatchesConstants(t *testing.T) {
BackendSecretService,
BackendFile,
BackendPass,
BackendOP,
BackendOPConnect,
BackendOPDesktop,
BackendMemory,
}
if len(allBackends) != len(expected) {
Expand Down Expand Up @@ -91,8 +94,9 @@ func TestValidBackendNames_MatchesAllBackends(t *testing.T) {

func TestBackendFlagUsage_NamesEveryBackend(t *testing.T) {
usage := BackendFlagUsage()
names := backendFlagUsageNames(t, usage)
for _, b := range allBackends {
if !strings.Contains(usage, string(b)) {
if !names[string(b)] {
t.Errorf("BackendFlagUsage() missing %q: %q", b, usage)
}
}
Expand All @@ -104,6 +108,23 @@ func TestBackendFlagUsage_NamesEveryBackend(t *testing.T) {
}
}

func backendFlagUsageNames(t *testing.T, usage string) map[string]bool {
t.Helper()
_, after, ok := strings.Cut(usage, "one of: ")
if !ok {
t.Fatalf("BackendFlagUsage() missing backend list: %q", usage)
}
list, _, ok := strings.Cut(after, ". Precedence:")
if !ok {
t.Fatalf("BackendFlagUsage() missing precedence suffix: %q", usage)
}
names := map[string]bool{}
for _, name := range strings.Split(list, ", ") {
names[name] = true
}
return names
}

func TestBackendEnvVar(t *testing.T) {
cases := []struct {
service string
Expand Down Expand Up @@ -218,7 +239,17 @@ func TestBindBackendFlag_NilOpts(t *testing.T) {
// downstream go.mod bump.
func TestBackendFlagUsage_IncludesPass(t *testing.T) {
usage := BackendFlagUsage()
if !strings.Contains(usage, "pass") {
if !backendFlagUsageNames(t, usage)[string(BackendPass)] {
t.Errorf("BackendFlagUsage() should mention pass; got %q", usage)
}
}

func TestBackendFlagUsage_IncludesOnePasswordBackends(t *testing.T) {
usage := BackendFlagUsage()
names := backendFlagUsageNames(t, usage)
for _, want := range []Backend{BackendOP, BackendOPConnect, BackendOPDesktop} {
if !names[string(want)] {
t.Errorf("BackendFlagUsage() should mention %q; got %q", want, usage)
}
}
}
11 changes: 11 additions & 0 deletions credstore/osbackend_1password_build_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
package credstore

import "testing"

func TestUnsupportedOnePasswordBackendInBuild_NonOnePasswordBackendsPass(t *testing.T) {
for _, kind := range []Backend{BackendKeychain, BackendWinCred, BackendSecretService, BackendFile, BackendPass, BackendMemory} {
if err := unsupportedOnePasswordBackendInBuild(kind); err != nil {
t.Fatalf("unsupportedOnePasswordBackendInBuild(%q) = %v, want nil", kind, err)
}
}
}
14 changes: 14 additions & 0 deletions credstore/osbackend_1password_disabled.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
//go:build keyring_no1password

package credstore

import "fmt"

func unsupportedOnePasswordBackendInBuild(kind Backend) error {
switch kind {
case BackendOP, BackendOPConnect, BackendOPDesktop:
return fmt.Errorf("%w: backend %q was compiled out of this build via keyring_no1password", ErrBackendNotImplemented, kind)
default:
return nil
}
}
28 changes: 28 additions & 0 deletions credstore/osbackend_1password_disabled_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
//go:build keyring_no1password

package credstore

import (
"errors"
"testing"
)

func TestUnsupportedOnePasswordBackendInBuild_DisabledBuildRejectsOnePasswordBackends(t *testing.T) {
for _, kind := range []Backend{BackendOP, BackendOPConnect, BackendOPDesktop} {
err := unsupportedOnePasswordBackendInBuild(kind)
if !errors.Is(err, ErrBackendNotImplemented) {
t.Fatalf("unsupportedOnePasswordBackendInBuild(%q) = %v, want ErrBackendNotImplemented", kind, err)
}
}
}

func TestOpen_OnePasswordDisabledBuildRejectsOnePasswordBackends(t *testing.T) {
for _, kind := range []Backend{BackendOP, BackendOPConnect, BackendOPDesktop} {
t.Run(string(kind), func(t *testing.T) {
_, err := Open("credstore-optest", &Options{Backend: kind})
if !errors.Is(err, ErrBackendNotImplemented) {
t.Fatalf("Open(%q) = %v, want ErrBackendNotImplemented", kind, err)
}
})
}
}
7 changes: 7 additions & 0 deletions credstore/osbackend_1password_enabled.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
//go:build !keyring_no1password

package credstore

func unsupportedOnePasswordBackendInBuild(Backend) error {
return nil
}
25 changes: 25 additions & 0 deletions credstore/osbackend_1password_enabled_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
//go:build !keyring_no1password && !freebsd && (cgo || windows)

package credstore

import (
"errors"
"testing"
)

func TestOpenOSBackend_OnePasswordEnabledBuildReachesBackendValidation(t *testing.T) {
t.Setenv("OP_VAULT_ID", "")
t.Setenv("OP_CONNECT_HOST", "")
t.Setenv("OP_DESKTOP_ACCOUNT_ID", "")
for _, kind := range []Backend{BackendOP, BackendOPConnect, BackendOPDesktop} {
t.Run(string(kind), func(t *testing.T) {
_, err := openOSBackend(kind, "credstore-optest", &Options{}, func(string) string { return "" })
if err == nil {
t.Fatal("openOSBackend unexpectedly succeeded without required 1Password configuration")
}
if errors.Is(err, ErrBackendNotImplemented) {
t.Fatalf("openOSBackend(%q) = %v, want enabled backend validation error, not ErrBackendNotImplemented", kind, err)
}
})
}
}
24 changes: 19 additions & 5 deletions credstore/osbackend_byteness.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,15 @@ import (
)

func openKeyringBackend(kind Backend, cfg backendConfig) (keyringBackend, error) {
kcfg := keyringConfigFromBackendConfig(cfg)
kr, err := keyring.Open(kcfg)
if err != nil {
return nil, fmt.Errorf("keyring open %s: %w", kind, err)
}
return bytenessBackend{kr: kr}, nil
}

func keyringConfigFromBackendConfig(cfg backendConfig) keyring.Config {
kcfg := keyring.Config{
ServiceName: cfg.serviceName,
AllowedBackends: []keyring.BackendType{keyring.BackendType(cfg.allowedBackend)},
Expand All @@ -18,15 +27,20 @@ func openKeyringBackend(kind Backend, cfg backendConfig) (keyringBackend, error)
PassDir: cfg.passDir,
PassCmd: cfg.passCmd,
PassPrefix: cfg.passPrefix,
OPTimeout: cfg.opTimeout,
OPVaultID: cfg.opVaultID,
OPItemTitlePrefix: cfg.opItemTitlePrefix,
OPItemTag: cfg.opItemTag,
OPItemFieldTitle: cfg.opItemFieldTitle,
OPConnectHost: cfg.opConnectHost,
OPConnectTokenEnv: cfg.opConnectTokenEnv,
OPTokenEnv: cfg.opTokenEnv,
OPDesktopAccountID: cfg.opDesktopAccountID,
}
if cfg.filePasswordFunc != nil {
kcfg.FilePasswordFunc = keyring.PromptFunc(cfg.filePasswordFunc)
}
kr, err := keyring.Open(kcfg)
if err != nil {
return nil, fmt.Errorf("keyring open %s: %w", kind, err)
}
return bytenessBackend{kr: kr}, nil
return kcfg
}

type bytenessBackend struct {
Expand Down
37 changes: 37 additions & 0 deletions credstore/osbackend_byteness_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ package credstore

import (
"testing"
"time"

"github.com/byteness/keyring"
)
Expand Down Expand Up @@ -61,3 +62,39 @@ func TestBytenessBackendSetPassesThroughMetadata(t *testing.T) {
t.Fatal("KeychainNotSynchronizable = false, want true")
}
}

func TestKeyringConfigFromBackendConfigForwardsOnePasswordOptions(t *testing.T) {
// #nosec G101 -- test fixture values are non-secret placeholders
cfg := keyringConfigFromBackendConfig(backendConfig{
serviceName: "codereview",
allowedBackend: BackendOPDesktop,
opTimeout: 5 * time.Second,
opVaultID: "vault-123",
opItemTitlePrefix: "cr",
opItemTag: "codereview",
opItemFieldTitle: "credential",
opConnectHost: "https://connect.example",
opConnectTokenEnv: "CUSTOM_OP_CONNECT_TOKEN",
opTokenEnv: "CUSTOM_OP_TOKEN",
opDesktopAccountID: "desktop-account",
})

if cfg.ServiceName != "codereview" {
t.Fatalf("ServiceName = %q, want %q", cfg.ServiceName, "codereview")
}
if len(cfg.AllowedBackends) != 1 || cfg.AllowedBackends[0] != keyring.OPDesktopBackend {
t.Fatalf("AllowedBackends = %v, want [%v]", cfg.AllowedBackends, keyring.OPDesktopBackend)
}
if cfg.OPTimeout != 5*time.Second || cfg.OPVaultID != "vault-123" {
t.Fatalf("unexpected timeout/vault forwarding: %+v", cfg)
}
if cfg.OPItemTitlePrefix != "cr" || cfg.OPItemTag != "codereview" || cfg.OPItemFieldTitle != "credential" {
t.Fatalf("unexpected item metadata forwarding: %+v", cfg)
}
if cfg.OPConnectHost != "https://connect.example" || cfg.OPConnectTokenEnv != "CUSTOM_OP_CONNECT_TOKEN" {
t.Fatalf("unexpected connect forwarding: %+v", cfg)
}
if cfg.OPTokenEnv != "CUSTOM_OP_TOKEN" || cfg.OPDesktopAccountID != "desktop-account" {
t.Fatalf("unexpected token/account forwarding: %+v", cfg)
}
}
40 changes: 39 additions & 1 deletion credstore/osbackend_core.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"os/exec"
"path/filepath"
"runtime"
"time"
)

var errKeyringItemNotFound = errors.New("credstore: keyring item not found")
Expand Down Expand Up @@ -44,6 +45,15 @@ type backendConfig struct {
passDir string
passCmd string
passPrefix string
opTimeout time.Duration
opVaultID string
opItemTitlePrefix string
opItemTag string
opItemFieldTitle string
opConnectHost string
opConnectTokenEnv string
opTokenEnv string
opDesktopAccountID string
}

// osKeyringBackend adapts a platform keyring implementation to the
Expand All @@ -59,6 +69,9 @@ type osKeyringBackend struct {
// build-tagged so static Unix builds do not import ByteNess keyring and
// therefore do not compile its unused 1Password backends.
func openOSBackend(kind Backend, service string, opts *Options, getenv func(string) string) (backend, error) {
if err := unsupportedOnePasswordBackendInBuild(kind); err != nil {
return nil, fmt.Errorf("credstore: opening %s backend for service %q: %w", kind, service, err)
}
if err := preflightOSBackend(kind); err != nil {
return nil, fmt.Errorf("credstore: opening %s backend for service %q: %w", kind, service, err)
}
Expand Down Expand Up @@ -88,7 +101,7 @@ func preflightOSBackend(kind Backend) error {
if _, err := exec.LookPath("pass"); err != nil {
return fmt.Errorf("the pass(1) CLI is not on $PATH; install it (e.g. `apt install pass` / `brew install pass`) and run `pass init <gpg-key-id>` before selecting --backend pass")
}
case BackendKeychain, BackendWinCred, BackendSecretService, BackendFile, BackendMemory:
case BackendKeychain, BackendWinCred, BackendSecretService, BackendFile, BackendOP, BackendOPConnect, BackendOPDesktop, BackendMemory:
// No preflight check — the openers (or store.Open's memory
// short-circuit) already return actionable errors.
}
Expand Down Expand Up @@ -130,6 +143,31 @@ func buildKeyringConfig(kind Backend, service string, opts *Options, getenv func
if kind == BackendKeychain {
cfg.keychainTrustApplication = true
}
case BackendOP, BackendOPConnect, BackendOPDesktop:
cfg.opItemTitlePrefix = service
cfg.opItemTag = service
// ByteNess requires a non-zero OPTimeout for OP and OPDesktop.
// OPConnect does not consult OPTimeout during construction.
if kind == BackendOP || kind == BackendOPDesktop {
Comment thread
monit-reviewer marked this conversation as resolved.
cfg.opTimeout = DefaultOnePasswordTimeout
}
if opts != nil && opts.OnePassword != nil {
if opts.OnePassword.Timeout != 0 {
cfg.opTimeout = opts.OnePassword.Timeout
}
cfg.opVaultID = opts.OnePassword.VaultID
if opts.OnePassword.ItemTitlePrefix != "" {
cfg.opItemTitlePrefix = opts.OnePassword.ItemTitlePrefix
}
if opts.OnePassword.ItemTag != "" {
cfg.opItemTag = opts.OnePassword.ItemTag
}
cfg.opItemFieldTitle = opts.OnePassword.ItemFieldTitle
cfg.opConnectHost = opts.OnePassword.ConnectHost
cfg.opConnectTokenEnv = opts.OnePassword.ConnectTokenEnv
cfg.opTokenEnv = opts.OnePassword.ServiceTokenEnv
cfg.opDesktopAccountID = opts.OnePassword.DesktopAccountID
}
case BackendMemory:
// BackendMemory short-circuits in store.Open before reaching
// openOSBackend; this arm exists only to keep the exhaustive
Expand Down
Loading
Loading