d2f6768 fix(review): address all code review issues from PR · gordon

M internal/adapters/in/cli/auth.go +4 -1


@@ -240,9 +240,12 @@ Examples:
240	240
241	241		cmd.Flags().StringVarP(&remoteName, "remote", "r", "", "Remote to authenticate with (defaults to active remote)")
242	242		cmd.Flags().StringVarP(&username, "username", "u", "", "Username for authentication")
243		-	cmd.Flags().StringVarP(&password, "password", "p", "", "Password for authentication")
	243	+	cmd.Flags().StringVarP(&password, "password", "p", "", "Password (visible in process listings; prefer interactive prompt)")
244	244		cmd.Flags().StringVarP(&token, "token", "t", "", "Authentication token to store for the remote")
245	245
	246	+	cmd.MarkFlagsMutuallyExclusive("token", "password")
	247	+	cmd.MarkFlagsMutuallyExclusive("token", "username")
	248	+
246	249		return cmd
247	250		}
248	251

 	cmd.Flags().StringVarP(&remoteName, "remote", "r", "", "Remote to authenticate with (defaults to active remote)")
 	cmd.Flags().StringVarP(&username, "username", "u", "", "Username for authentication")
-	cmd.Flags().StringVarP(&password, "password", "p", "", "Password for authentication")
+	cmd.Flags().StringVarP(&password, "password", "p", "", "Password (visible in process listings; prefer interactive prompt)")
 	cmd.Flags().StringVarP(&token, "token", "t", "", "Authentication token to store for the remote")
+	cmd.MarkFlagsMutuallyExclusive("token", "password")
+	cmd.MarkFlagsMutuallyExclusive("token", "username")
 	return cmd
 }

A internal/adapters/out/domainsecrets/helpers_test.go +7 -0

@@ -0,0 +1,7 @@
	1	+	package domainsecrets
	2	+
	3	+	import "github.com/bnema/zerowrap"
	4	+
	5	+	func testLogger() zerowrap.Logger {
	6	+	return zerowrap.Default()
	7	+	}

@@ -0,0 +1,7 @@
	1	+	package domainsecrets
	2	+
	3	+	import "github.com/bnema/zerowrap"
	4	+
	5	+	func testLogger() zerowrap.Logger {
	6	+	return zerowrap.Default()
	7	+	}

M internal/adapters/out/domainsecrets/pass_store.go +14 -12


@@ -21,8 +21,8 @@ import (
21	21		var ansiRegex = regexp.MustCompile(`\x1b\[[0-9;]*m`)
22	22
23	23		const (
24		-	passDomainSecretsPath = "gordon/env" //nolint:gosec // Not a credential, this is a pass store path.
25		-	passAttachmentPath = "gordon/env/attachments" //nolint:gosec // Not a credential, this is a pass store path.
	24	+	PassDomainSecretsPath = "gordon/env" //nolint:gosec // Not a credential, this is a pass store path.
	25	+	PassAttachmentPath = "gordon/env/attachments" //nolint:gosec // Not a credential, this is a pass store path.
26	26		)
27	27
28	28		// PassStore implements the DomainSecretStore interface using the pass password manager.

@@ -190,14 +190,14 @@ func (s *PassStore) Delete(domainName, key string) error {
190	190		return err
191	191		}
192	192
193		-	ctx, cancel := context.WithTimeout(context.Background(), s.timeout)
194		-	defer cancel()
195		-
196	193		keyPath, err := s.keyPath(domainName, key)
197	194		if err != nil {
198	195		return err
199	196		}
200		-	if err := s.passRemove(ctx, keyPath); err != nil {
	197	+
	198	+	rmCtx, rmCancel := context.WithTimeout(context.Background(), s.timeout)
	199	+	defer rmCancel()
	200	+	if err := s.passRemove(rmCtx, keyPath); err != nil {
201	201		return err
202	202		}
203	203

@@ -220,7 +220,9 @@ func (s *PassStore) Delete(domainName, key string) error {
220	220		return err
221	221		}
222	222
223		-	if err := s.passInsert(ctx, manifestPath, strings.Join(updated, "\n")); err != nil {
	223	+	insertCtx, insertCancel := context.WithTimeout(context.Background(), s.timeout)
	224	+	defer insertCancel()
	225	+	if err := s.passInsert(insertCtx, manifestPath, strings.Join(updated, "\n")); err != nil {
224	226		return fmt.Errorf("failed to update manifest: %w", err)
225	227		}
226	228

@@ -236,7 +238,7 @@ func (s *PassStore) ListAttachmentKeys(domainName string) ([]out.AttachmentSecre
236	238		ctx, cancel := context.WithTimeout(context.Background(), s.timeout)
237	239		defer cancel()
238	240
239		-	containers, err := s.listTopLevelEntries(ctx, passAttachmentPath)
	241	+	containers, err := s.listTopLevelEntries(ctx, PassAttachmentPath)
240	242		if err != nil {
241	243		return nil, err
242	244		}

@@ -250,7 +252,7 @@ func (s *PassStore) ListAttachmentKeys(domainName string) ([]out.AttachmentSecre
250	252		continue
251	253		}
252	254
253		-	manifestPath := fmt.Sprintf("%s/%s/.keys", passAttachmentPath, containerName)
	255	+	manifestPath := fmt.Sprintf("%s/%s/.keys", PassAttachmentPath, containerName)
254	256		if err := secrets.ValidatePath(manifestPath); err != nil {
255	257		return nil, err
256	258		}

@@ -288,7 +290,7 @@ func (s *PassStore) domainPath(domainName string) (string, error) {
288	290		if err != nil {
289	291		return "", err
290	292		}
291		-	path := fmt.Sprintf("%s/%s", passDomainSecretsPath, safeDomain)
	293	+	path := fmt.Sprintf("%s/%s", PassDomainSecretsPath, safeDomain)
292	294		if err := secrets.ValidatePath(path); err != nil {
293	295		return "", err
294	296		}

@@ -431,6 +433,7 @@ func parsePassListOutput(basePath, output string) []passListEntry {
431	433		}
432	434
433	435		depth := 0
	436	+	prefixLoop:
434	437		for {
435	438		switch {
436	439		case strings.HasPrefix(line, "│ "):

@@ -443,10 +446,9 @@ func parsePassListOutput(basePath, output string) []passListEntry {
443	446		line = strings.TrimPrefix(line, " ")
444	447		depth++
445	448		default:
446		-	goto prefixDone
	449	+	break prefixLoop
447	450		}
448	451		}
449		-	prefixDone:
450	452
451	453		switch {
452	454		case strings.HasPrefix(line, "├── "):


@@ -21,8 +21,8 @@ import (
21		var ansiRegex = regexp.MustCompile(`\x1b\[[0-9;]*m`)	21		var ansiRegex = regexp.MustCompile(`\x1b\[[0-9;]*m`)
22			22
23		const (	23		const (
24	-	passDomainSecretsPath = "gordon/env" //nolint:gosec // Not a credential, this is a pass store path.	24	+	PassDomainSecretsPath = "gordon/env" //nolint:gosec // Not a credential, this is a pass store path.
25	-	passAttachmentPath = "gordon/env/attachments" //nolint:gosec // Not a credential, this is a pass store path.	25	+	PassAttachmentPath = "gordon/env/attachments" //nolint:gosec // Not a credential, this is a pass store path.
26		)	26		)
27			27
28		// PassStore implements the DomainSecretStore interface using the pass password manager.	28		// PassStore implements the DomainSecretStore interface using the pass password manager.

@@ -190,14 +190,14 @@ func (s *PassStore) Delete(domainName, key string) error {
190		return err	190		return err
191		}	191		}
192			192
193	-	ctx, cancel := context.WithTimeout(context.Background(), s.timeout)
194	-	defer cancel()
195	-
196		keyPath, err := s.keyPath(domainName, key)	193		keyPath, err := s.keyPath(domainName, key)
197		if err != nil {	194		if err != nil {
198		return err	195		return err
199		}	196		}
200	-	if err := s.passRemove(ctx, keyPath); err != nil {	197	+
			198	+	rmCtx, rmCancel := context.WithTimeout(context.Background(), s.timeout)
			199	+	defer rmCancel()
			200	+	if err := s.passRemove(rmCtx, keyPath); err != nil {
201		return err	201		return err
202		}	202		}
203			203

@@ -220,7 +220,9 @@ func (s *PassStore) Delete(domainName, key string) error {
220		return err	220		return err
221		}	221		}
222			222
223	-	if err := s.passInsert(ctx, manifestPath, strings.Join(updated, "\n")); err != nil {	223	+	insertCtx, insertCancel := context.WithTimeout(context.Background(), s.timeout)
			224	+	defer insertCancel()
			225	+	if err := s.passInsert(insertCtx, manifestPath, strings.Join(updated, "\n")); err != nil {
224		return fmt.Errorf("failed to update manifest: %w", err)	226		return fmt.Errorf("failed to update manifest: %w", err)
225		}	227		}
226			228

@@ -236,7 +238,7 @@ func (s *PassStore) ListAttachmentKeys(domainName string) ([]out.AttachmentSecre
236		ctx, cancel := context.WithTimeout(context.Background(), s.timeout)	238		ctx, cancel := context.WithTimeout(context.Background(), s.timeout)
237		defer cancel()	239		defer cancel()
238			240
239	-	containers, err := s.listTopLevelEntries(ctx, passAttachmentPath)	241	+	containers, err := s.listTopLevelEntries(ctx, PassAttachmentPath)
240		if err != nil {	242		if err != nil {
241		return nil, err	243		return nil, err
242		}	244		}

@@ -250,7 +252,7 @@ func (s *PassStore) ListAttachmentKeys(domainName string) ([]out.AttachmentSecre
250		continue	252		continue
251		}	253		}
252			254
253	-	manifestPath := fmt.Sprintf("%s/%s/.keys", passAttachmentPath, containerName)	255	+	manifestPath := fmt.Sprintf("%s/%s/.keys", PassAttachmentPath, containerName)
254		if err := secrets.ValidatePath(manifestPath); err != nil {	256		if err := secrets.ValidatePath(manifestPath); err != nil {
255		return nil, err	257		return nil, err
256		}	258		}

@@ -288,7 +290,7 @@ func (s *PassStore) domainPath(domainName string) (string, error) {
288		if err != nil {	290		if err != nil {
289		return "", err	291		return "", err
290		}	292		}
291	-	path := fmt.Sprintf("%s/%s", passDomainSecretsPath, safeDomain)	293	+	path := fmt.Sprintf("%s/%s", PassDomainSecretsPath, safeDomain)
292		if err := secrets.ValidatePath(path); err != nil {	294		if err := secrets.ValidatePath(path); err != nil {
293		return "", err	295		return "", err
294		}	296		}

@@ -431,6 +433,7 @@ func parsePassListOutput(basePath, output string) []passListEntry {
431		}	433		}
432			434
433		depth := 0	435		depth := 0
			436	+	prefixLoop:
434		for {	437		for {
435		switch {	438		switch {
436		case strings.HasPrefix(line, "│ "):	439		case strings.HasPrefix(line, "│ "):

@@ -443,10 +446,9 @@ func parsePassListOutput(basePath, output string) []passListEntry {
443		line = strings.TrimPrefix(line, " ")	446		line = strings.TrimPrefix(line, " ")
444		depth++	447		depth++
445		default:	448		default:
446	-	goto prefixDone	449	+	break prefixLoop
447		}	450		}
448		}	451		}
449	-	prefixDone:
450			452
451		switch {	453		switch {
452		case strings.HasPrefix(line, "├── "):	454		case strings.HasPrefix(line, "├── "):

M internal/adapters/out/domainsecrets/pass_store_test.go +43 -2


@@ -35,11 +35,11 @@ func cleanupPassDomain(_ *testing.T, domainName string, keys []string) {
35	35		}
36	36
37	37		for _, key := range keys {
38		-	path := fmt.Sprintf("%s/%s/%s", passDomainSecretsPath, safeDomain, key)
	38	+	path := fmt.Sprintf("%s/%s/%s", PassDomainSecretsPath, safeDomain, key)
39	39		_ = passCmd("rm", "-f", path)
40	40		}
41	41
42		-	manifestPath := fmt.Sprintf("%s/%s/.keys", passDomainSecretsPath, safeDomain)
	42	+	manifestPath := fmt.Sprintf("%s/%s/.keys", PassDomainSecretsPath, safeDomain)
43	43		_ = passCmd("rm", "-f", manifestPath)
44	44		}
45	45

@@ -78,3 +78,44 @@ func TestPassStore_SetGetDelete(t *testing.T) {
78	78		require.NoError(t, err)
79	79		assert.ElementsMatch(t, []string{"API_KEY"}, keysList)
80	80		}
	81	+
	82	+	func TestParsePassListOutput(t *testing.T) {
	83	+	tests := []struct {
	84	+	name string
	85	+	basePath string
	86	+	output string
	87	+	want []passListEntry
	88	+	}{
	89	+	{
	90	+	name: "simple tree",
	91	+	basePath: "domain",
	92	+	output: "domain\n├── key1\n└── key2",
	93	+	want: []passListEntry{{name: "key1", depth: 1}, {name: "key2", depth: 1}},
	94	+	},
	95	+	{
	96	+	name: "nested tree",
	97	+	basePath: "domain",
	98	+	output: "domain\n│ ├── subkey1\n│ └── subkey2\n└── key1",
	99	+	want: []passListEntry{{name: "subkey1", depth: 2}, {name: "subkey2", depth: 2}, {name: "key1", depth: 1}},
	100	+	},
	101	+	{
	102	+	name: "empty output",
	103	+	basePath: "domain",
	104	+	output: "",
	105	+	want: []passListEntry{},
	106	+	},
	107	+	{
	108	+	name: "ASCII fallback chars",
	109	+	basePath: "domain",
	110	+	output: "domain\n\| \|-- subkey1\n`-- key1",
	111	+	want: []passListEntry{{name: "subkey1", depth: 2}, {name: "key1", depth: 1}},
	112	+	},
	113	+	}
	114	+
	115	+	for _, tt := range tests {
	116	+	t.Run(tt.name, func(t *testing.T) {
	117	+	got := parsePassListOutput(tt.basePath, tt.output)
	118	+	assert.Equal(t, tt.want, got)
	119	+	})
	120	+	}
	121	+	}


@@ -35,11 +35,11 @@ func cleanupPassDomain(_ *testing.T, domainName string, keys []string) {
35		}	35		}
36			36
37		for _, key := range keys {	37		for _, key := range keys {
38	-	path := fmt.Sprintf("%s/%s/%s", passDomainSecretsPath, safeDomain, key)	38	+	path := fmt.Sprintf("%s/%s/%s", PassDomainSecretsPath, safeDomain, key)
39		_ = passCmd("rm", "-f", path)	39		_ = passCmd("rm", "-f", path)
40		}	40		}
41			41
42	-	manifestPath := fmt.Sprintf("%s/%s/.keys", passDomainSecretsPath, safeDomain)	42	+	manifestPath := fmt.Sprintf("%s/%s/.keys", PassDomainSecretsPath, safeDomain)
43		_ = passCmd("rm", "-f", manifestPath)	43		_ = passCmd("rm", "-f", manifestPath)
44		}	44		}
45			45

@@ -78,3 +78,44 @@ func TestPassStore_SetGetDelete(t *testing.T) {
78		require.NoError(t, err)	78		require.NoError(t, err)
79		assert.ElementsMatch(t, []string{"API_KEY"}, keysList)	79		assert.ElementsMatch(t, []string{"API_KEY"}, keysList)
80		}	80		}
			81	+
			82	+	func TestParsePassListOutput(t *testing.T) {
			83	+	tests := []struct {
			84	+	name string
			85	+	basePath string
			86	+	output string
			87	+	want []passListEntry
			88	+	}{
			89	+	{
			90	+	name: "simple tree",
			91	+	basePath: "domain",
			92	+	output: "domain\n├── key1\n└── key2",
			93	+	want: []passListEntry{{name: "key1", depth: 1}, {name: "key2", depth: 1}},
			94	+	},
			95	+	{
			96	+	name: "nested tree",
			97	+	basePath: "domain",
			98	+	output: "domain\n│ ├── subkey1\n│ └── subkey2\n└── key1",
			99	+	want: []passListEntry{{name: "subkey1", depth: 2}, {name: "subkey2", depth: 2}, {name: "key1", depth: 1}},
			100	+	},
			101	+	{
			102	+	name: "empty output",
			103	+	basePath: "domain",
			104	+	output: "",
			105	+	want: []passListEntry{},
			106	+	},
			107	+	{
			108	+	name: "ASCII fallback chars",
			109	+	basePath: "domain",
			110	+	output: "domain\n\| \|-- subkey1\n`-- key1",
			111	+	want: []passListEntry{{name: "subkey1", depth: 2}, {name: "key1", depth: 1}},
			112	+	},
			113	+	}
			114	+
			115	+	for _, tt := range tests {
			116	+	t.Run(tt.name, func(t *testing.T) {
			117	+	got := parsePassListOutput(tt.basePath, tt.output)
			118	+	assert.Equal(t, tt.want, got)
			119	+	})
			120	+	}
			121	+	}

M internal/adapters/out/domainsecrets/store_test.go +0 -5


@@ -6,17 +6,12 @@ import (
6	6		"path/filepath"
7	7		"testing"
8	8
9		-	"github.com/bnema/zerowrap"
10	9		"github.com/stretchr/testify/assert"
11	10		"github.com/stretchr/testify/require"
12	11
13	12		"github.com/bnema/gordon/internal/domain"
14	13		)
15	14
16		-	func testLogger() zerowrap.Logger {
17		-	return zerowrap.Default()
18		-	}
19		-
20	15		func TestFileStore_PathTraversal(t *testing.T) {
21	16		tmpDir, err := os.MkdirTemp("", "domainsecrets-test-*")
22	17		require.NoError(t, err)


@@ -6,17 +6,12 @@ import (
6		"path/filepath"	6	"path/filepath"
7		"testing"	7	"testing"
8			8
9	-	"github.com/bnema/zerowrap"
10		"github.com/stretchr/testify/assert"	9	"github.com/stretchr/testify/assert"
11		"github.com/stretchr/testify/require"	10	"github.com/stretchr/testify/require"
12			11
13		"github.com/bnema/gordon/internal/domain"	12	"github.com/bnema/gordon/internal/domain"
14		)	13	)
15			14
16	-	func testLogger() zerowrap.Logger {
17	-	return zerowrap.Default()
18	-	}
19	-
20		func TestFileStore_PathTraversal(t *testing.T) {	15	func TestFileStore_PathTraversal(t *testing.T) {
21		tmpDir, err := os.MkdirTemp("", "domainsecrets-test-*")	16	tmpDir, err := os.MkdirTemp("", "domainsecrets-test-*")
22		require.NoError(t, err)	17	require.NoError(t, err)

A internal/app/migrate_env_test.go +89 -0

@@ -0,0 +1,89 @@
	1	+	package app
	2	+
	3	+	import (
	4	+	"context"
	5	+	"fmt"
	6	+	"os"
	7	+	"os/exec"
	8	+	"path/filepath"
	9	+	"testing"
	10	+	"time"
	11	+
	12	+	"github.com/bnema/zerowrap"
	13	+	"github.com/stretchr/testify/assert"
	14	+	"github.com/stretchr/testify/require"
	15	+
	16	+	"github.com/bnema/gordon/internal/adapters/out/domainsecrets"
	17	+	"github.com/bnema/gordon/internal/domain"
	18	+	)
	19	+
	20	+	func passCmd(args ...string) error {
	21	+	ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second)
	22	+	defer cancel()
	23	+	return exec.CommandContext(ctx, "pass", args...).Run()
	24	+	}
	25	+
	26	+	func requirePass(t *testing.T) {
	27	+	if err := passCmd("version"); err != nil {
	28	+	t.Skip("pass not available")
	29	+	}
	30	+	if err := passCmd("ls"); err != nil {
	31	+	t.Skip("pass store not initialized")
	32	+	}
	33	+	}
	34	+
	35	+	func cleanupPassDomain(t *testing.T, domainName string, keys []string) {
	36	+	safeDomain, err := domain.SanitizeDomainForEnvFile(domainName)
	37	+	if err != nil {
	38	+	return
	39	+	}
	40	+
	41	+	for _, key := range keys {
	42	+	path := fmt.Sprintf("%s/%s/%s", domainsecrets.PassDomainSecretsPath, safeDomain, key)
	43	+	_ = passCmd("rm", "-f", path)
	44	+	}
	45	+
	46	+	manifestPath := fmt.Sprintf("%s/%s/.keys", domainsecrets.PassDomainSecretsPath, safeDomain)
	47	+	_ = passCmd("rm", "-f", manifestPath)
	48	+	}
	49	+
	50	+	func TestMigrateEnvFile(t *testing.T) {
	51	+	requirePass(t)
	52	+
	53	+	tmpDir, err := os.MkdirTemp("", "migrate-env-test-*")
	54	+	require.NoError(t, err)
	55	+	defer os.RemoveAll(tmpDir)
	56	+
	57	+	logger := zerowrap.Default()
	58	+	store, err := domainsecrets.NewPassStore(logger)
	59	+	require.NoError(t, err)
	60	+
	61	+	domainName := fmt.Sprintf("test.%d.example.com", time.Now().UnixNano())
	62	+	keys := []string{"API_KEY", "DB_PASSWORD"}
	63	+	defer cleanupPassDomain(t, domainName, keys)
	64	+
	65	+	envContent := "API_KEY=secret123\nDB_PASSWORD=pass456\n"
	66	+	envFileName := domainName + ".env"
	67	+	envFilePath := filepath.Join(tmpDir, envFileName)
	68	+	err = os.WriteFile(envFilePath, []byte(envContent), 0600)
	69	+	require.NoError(t, err)
	70	+
	71	+	err = migrateEnvFile(tmpDir, envFileName, store, logger)
	72	+	require.NoError(t, err)
	73	+
	74	+	migratedPath := envFilePath + ".migrated"
	75	+	_, err = os.Stat(envFilePath)
	76	+	assert.True(t, os.IsNotExist(err), "original .env file should be removed")
	77	+
	78	+	_, err = os.Stat(migratedPath)
	79	+	assert.NoError(t, err, ".env.migrated file should exist")
	80	+
	81	+	passKeys, err := store.ListKeys(domainName)
	82	+	require.NoError(t, err)
	83	+	assert.ElementsMatch(t, keys, passKeys)
	84	+
	85	+	values, err := store.GetAll(domainName)
	86	+	require.NoError(t, err)
	87	+	assert.Equal(t, "secret123", values["API_KEY"])
	88	+	assert.Equal(t, "pass456", values["DB_PASSWORD"])
	89	+	}

@@ -0,0 +1,89 @@
	1	+	package app
	2	+
	3	+	import (
	4	+	"context"
	5	+	"fmt"
	6	+	"os"
	7	+	"os/exec"
	8	+	"path/filepath"
	9	+	"testing"
	10	+	"time"
	11	+
	12	+	"github.com/bnema/zerowrap"
	13	+	"github.com/stretchr/testify/assert"
	14	+	"github.com/stretchr/testify/require"
	15	+
	16	+	"github.com/bnema/gordon/internal/adapters/out/domainsecrets"
	17	+	"github.com/bnema/gordon/internal/domain"
	18	+	)
	19	+
	20	+	func passCmd(args ...string) error {
	21	+	ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second)
	22	+	defer cancel()
	23	+	return exec.CommandContext(ctx, "pass", args...).Run()
	24	+	}
	25	+
	26	+	func requirePass(t *testing.T) {
	27	+	if err := passCmd("version"); err != nil {
	28	+	t.Skip("pass not available")
	29	+	}
	30	+	if err := passCmd("ls"); err != nil {
	31	+	t.Skip("pass store not initialized")
	32	+	}
	33	+	}
	34	+
	35	+	func cleanupPassDomain(t *testing.T, domainName string, keys []string) {
	36	+	safeDomain, err := domain.SanitizeDomainForEnvFile(domainName)
	37	+	if err != nil {
	38	+	return
	39	+	}
	40	+
	41	+	for _, key := range keys {
	42	+	path := fmt.Sprintf("%s/%s/%s", domainsecrets.PassDomainSecretsPath, safeDomain, key)
	43	+	_ = passCmd("rm", "-f", path)
	44	+	}
	45	+
	46	+	manifestPath := fmt.Sprintf("%s/%s/.keys", domainsecrets.PassDomainSecretsPath, safeDomain)
	47	+	_ = passCmd("rm", "-f", manifestPath)
	48	+	}
	49	+
	50	+	func TestMigrateEnvFile(t *testing.T) {
	51	+	requirePass(t)
	52	+
	53	+	tmpDir, err := os.MkdirTemp("", "migrate-env-test-*")
	54	+	require.NoError(t, err)
	55	+	defer os.RemoveAll(tmpDir)
	56	+
	57	+	logger := zerowrap.Default()
	58	+	store, err := domainsecrets.NewPassStore(logger)
	59	+	require.NoError(t, err)
	60	+
	61	+	domainName := fmt.Sprintf("test.%d.example.com", time.Now().UnixNano())
	62	+	keys := []string{"API_KEY", "DB_PASSWORD"}
	63	+	defer cleanupPassDomain(t, domainName, keys)
	64	+
	65	+	envContent := "API_KEY=secret123\nDB_PASSWORD=pass456\n"
	66	+	envFileName := domainName + ".env"
	67	+	envFilePath := filepath.Join(tmpDir, envFileName)
	68	+	err = os.WriteFile(envFilePath, []byte(envContent), 0600)
	69	+	require.NoError(t, err)
	70	+
	71	+	err = migrateEnvFile(tmpDir, envFileName, store, logger)
	72	+	require.NoError(t, err)
	73	+
	74	+	migratedPath := envFilePath + ".migrated"
	75	+	_, err = os.Stat(envFilePath)
	76	+	assert.True(t, os.IsNotExist(err), "original .env file should be removed")
	77	+
	78	+	_, err = os.Stat(migratedPath)
	79	+	assert.NoError(t, err, ".env.migrated file should exist")
	80	+
	81	+	passKeys, err := store.ListKeys(domainName)
	82	+	require.NoError(t, err)
	83	+	assert.ElementsMatch(t, keys, passKeys)
	84	+
	85	+	values, err := store.GetAll(domainName)
	86	+	require.NoError(t, err)
	87	+	assert.Equal(t, "secret123", values["API_KEY"])
	88	+	assert.Equal(t, "pass456", values["DB_PASSWORD"])
	89	+	}

M internal/app/run.go +6 -0


@@ -847,10 +847,16 @@ func createContainerService(ctx context.Context, v *viper.Viper, cfg Config, svc
847	847		if err != nil {
848	848		return nil, log.WrapErr(err, "failed to resolve service token expiry")
849	849		}
	850	+	// Note: Service tokens are not auto-refreshed. If the token expires during
	851	+	// container runtime, the container will need to be recreated to get a new token.
850	852		serviceToken, err := svc.authSvc.GenerateToken(ctx, serviceTokenSubject, []string{"pull"}, expiry)
851	853		if err != nil {
852	854		return nil, log.WrapErr(err, "failed to generate registry service token")
853	855		}
	856	+	log.Info().
	857	+	Str("subject", serviceTokenSubject).
	858	+	Str("expiry", expiry.String()).
	859	+	Msg("generated service token for container registry access")
854	860		containerConfig.ServiceTokenUsername = serviceTokenSubject
855	861		containerConfig.ServiceToken = serviceToken
856	862		}


@@ -847,10 +847,16 @@ func createContainerService(ctx context.Context, v *viper.Viper, cfg Config, svc
847	if err != nil {	847		if err != nil {
848	return nil, log.WrapErr(err, "failed to resolve service token expiry")	848		return nil, log.WrapErr(err, "failed to resolve service token expiry")
849	}	849		}
		850	+	// Note: Service tokens are not auto-refreshed. If the token expires during
		851	+	// container runtime, the container will need to be recreated to get a new token.
850	serviceToken, err := svc.authSvc.GenerateToken(ctx, serviceTokenSubject, []string{"pull"}, expiry)	852		serviceToken, err := svc.authSvc.GenerateToken(ctx, serviceTokenSubject, []string{"pull"}, expiry)
851	if err != nil {	853		if err != nil {
852	return nil, log.WrapErr(err, "failed to generate registry service token")	854		return nil, log.WrapErr(err, "failed to generate registry service token")
853	}	855		}
		856	+	log.Info().
		857	+	Str("subject", serviceTokenSubject).
		858	+	Str("expiry", expiry.String()).
		859	+	Msg("generated service token for container registry access")
854	containerConfig.ServiceTokenUsername = serviceTokenSubject	860		containerConfig.ServiceTokenUsername = serviceTokenSubject
855	containerConfig.ServiceToken = serviceToken	861		containerConfig.ServiceToken = serviceToken
856	}	862		}

M internal/domain/env.go +11 -7


@@ -14,6 +14,7 @@ var envKeyRegex = regexp.MustCompile(`^[A-Za-z_][A-Za-z0-9_]*$`)
14	14
15	15		// SanitizeDomainForEnvFile validates and sanitizes a domain name for env file storage.
16	16		// Returns a safe domain string suitable for filenames.
	17	+	// This function is idempotent: calling it on already-sanitized input returns the same value.
17	18		func SanitizeDomainForEnvFile(domainName string) (string, error) {
18	19		if domainName == "" {
19	20		return "", ErrPathTraversal

@@ -51,12 +52,15 @@ func ParseEnvData(data []byte) (map[string]string, error) {
51	52		}
52	53
53	54		key := strings.TrimSpace(parts[0])
54		-	value := strings.TrimSpace(parts[1])
	55	+	rawValue := parts[1]
55	56
56		-	if len(value) >= 2 {
57		-	if (strings.HasPrefix(value, "\"") && strings.HasSuffix(value, "\"")) \|\|
58		-	(strings.HasPrefix(value, "'") && strings.HasSuffix(value, "'")) {
59		-	value = value[1 : len(value)-1]
	57	+	trimmedValue := strings.TrimSpace(rawValue)
	58	+	value := trimmedValue
	59	+
	60	+	if len(trimmedValue) >= 2 {
	61	+	if (strings.HasPrefix(trimmedValue, "\"") && strings.HasSuffix(trimmedValue, "\"")) \|\|
	62	+	(strings.HasPrefix(trimmedValue, "'") && strings.HasSuffix(trimmedValue, "'")) {
	63	+	value = trimmedValue[1 : len(trimmedValue)-1]
60	64		}
61	65		}
62	66

@@ -75,13 +79,13 @@ func ParseEnvData(data []byte) (map[string]string, error) {
75	79		// ValidateEnvKey validates an env key for storage.
76	80		func ValidateEnvKey(key string) error {
77	81		if key == "" {
78		-	return ErrPathTraversal
	82	+	return ErrInvalidEnvKey
79	83		}
80	84		if strings.Contains(key, "..") \|\| strings.ContainsAny(key, "/\\") {
81	85		return ErrPathTraversal
82	86		}
83	87		if !envKeyRegex.MatchString(key) {
84		-	return ErrPathTraversal
	88	+	return ErrInvalidEnvKey
85	89		}
86	90		return nil
87	91		}


@@ -14,6 +14,7 @@ var envKeyRegex = regexp.MustCompile(`^[A-Za-z_][A-Za-z0-9_]*$`)
14			14
15		// SanitizeDomainForEnvFile validates and sanitizes a domain name for env file storage.	15		// SanitizeDomainForEnvFile validates and sanitizes a domain name for env file storage.
16		// Returns a safe domain string suitable for filenames.	16		// Returns a safe domain string suitable for filenames.
			17	+	// This function is idempotent: calling it on already-sanitized input returns the same value.
17		func SanitizeDomainForEnvFile(domainName string) (string, error) {	18		func SanitizeDomainForEnvFile(domainName string) (string, error) {
18		if domainName == "" {	19		if domainName == "" {
19		return "", ErrPathTraversal	20		return "", ErrPathTraversal

@@ -51,12 +52,15 @@ func ParseEnvData(data []byte) (map[string]string, error) {
51		}	52		}
52			53
53		key := strings.TrimSpace(parts[0])	54		key := strings.TrimSpace(parts[0])
54	-	value := strings.TrimSpace(parts[1])	55	+	rawValue := parts[1]
55			56
56	-	if len(value) >= 2 {	57	+	trimmedValue := strings.TrimSpace(rawValue)
57	-	if (strings.HasPrefix(value, "\"") && strings.HasSuffix(value, "\"")) \|\|	58	+	value := trimmedValue
58	-	(strings.HasPrefix(value, "'") && strings.HasSuffix(value, "'")) {	59	+
59	-	value = value[1 : len(value)-1]	60	+	if len(trimmedValue) >= 2 {
			61	+	if (strings.HasPrefix(trimmedValue, "\"") && strings.HasSuffix(trimmedValue, "\"")) \|\|
			62	+	(strings.HasPrefix(trimmedValue, "'") && strings.HasSuffix(trimmedValue, "'")) {
			63	+	value = trimmedValue[1 : len(trimmedValue)-1]
60		}	64		}
61		}	65		}
62			66

@@ -75,13 +79,13 @@ func ParseEnvData(data []byte) (map[string]string, error) {
75		// ValidateEnvKey validates an env key for storage.	79		// ValidateEnvKey validates an env key for storage.
76		func ValidateEnvKey(key string) error {	80		func ValidateEnvKey(key string) error {
77		if key == "" {	81		if key == "" {
78	-	return ErrPathTraversal	82	+	return ErrInvalidEnvKey
79		}	83		}
80		if strings.Contains(key, "..") \|\| strings.ContainsAny(key, "/\\") {	84		if strings.Contains(key, "..") \|\| strings.ContainsAny(key, "/\\") {
81		return ErrPathTraversal	85		return ErrPathTraversal
82		}	86		}
83		if !envKeyRegex.MatchString(key) {	87		if !envKeyRegex.MatchString(key) {
84	-	return ErrPathTraversal	88	+	return ErrInvalidEnvKey
85		}	89		}
86		return nil	90		return nil
87		}	91		}

M internal/domain/env_test.go +28 -12


@@ -28,6 +28,8 @@ func TestSanitizeDomainForEnvFile(t *testing.T) {
28	28		{name: "ends with dot", domain: "trailing.", wantErr: true},
29	29		{name: "space", domain: "has space", wantErr: true},
30	30		{name: "special chars", domain: "bad$domain", wantErr: true},
	31	+	{name: "idempotent simple", domain: "example_com", want: "example_com"},
	32	+	{name: "idempotent complex", domain: "my-app_example_com_8080", want: "my-app_example_com_8080"},
31	33		}
32	34
33	35		for _, tt := range tests {

@@ -100,6 +102,16 @@ func TestParseEnvData(t *testing.T) {
100	102		data: "BIG=" + strings.Repeat("x", 100000),
101	103		want: map[string]string{"BIG": strings.Repeat("x", 100000)},
102	104		},
	105	+	{
	106	+	name: "quoted value with inner spaces",
	107	+	data: `KEY=" hello "`,
	108	+	want: map[string]string{"KEY": " hello "},
	109	+	},
	110	+	{
	111	+	name: "unquoted value with outer spaces",
	112	+	data: "KEY= hello ",
	113	+	want: map[string]string{"KEY": "hello"},
	114	+	},
103	115		}
104	116
105	117		for _, tt := range tests {

@@ -117,30 +129,34 @@ func TestParseEnvData(t *testing.T) {
117	129
118	130		func TestValidateEnvKey(t *testing.T) {
119	131		tests := []struct {
120		-	name string
121		-	key string
122		-	wantErr bool
	132	+	name string
	133	+	key string
	134	+	wantErr bool
	135	+	wantErrType error
123	136		}{
124	137		{name: "simple", key: "FOO", wantErr: false},
125	138		{name: "with underscore", key: "FOO_BAR", wantErr: false},
126	139		{name: "starts with underscore", key: "_PRIVATE", wantErr: false},
127	140		{name: "lowercase", key: "foo", wantErr: false},
128	141		{name: "mixed", key: "myApp_v2", wantErr: false},
129		-	{name: "empty", key: "", wantErr: true},
130		-	{name: "starts with number", key: "1BAD", wantErr: true},
131		-	{name: "contains dot", key: "FOO.BAR", wantErr: true},
132		-	{name: "contains slash", key: "FOO/BAR", wantErr: true},
133		-	{name: "contains backslash", key: `FOO\BAR`, wantErr: true},
134		-	{name: "path traversal", key: "..", wantErr: true},
135		-	{name: "contains hyphen", key: "FOO-BAR", wantErr: true},
136		-	{name: "contains space", key: "FOO BAR", wantErr: true},
	142	+	{name: "empty", key: "", wantErr: true, wantErrType: ErrInvalidEnvKey},
	143	+	{name: "starts with number", key: "1BAD", wantErr: true, wantErrType: ErrInvalidEnvKey},
	144	+	{name: "contains dot", key: "FOO.BAR", wantErr: true, wantErrType: ErrInvalidEnvKey},
	145	+	{name: "contains slash", key: "FOO/BAR", wantErr: true, wantErrType: ErrPathTraversal},
	146	+	{name: "contains backslash", key: `FOO\BAR`, wantErr: true, wantErrType: ErrPathTraversal},
	147	+	{name: "path traversal", key: "..", wantErr: true, wantErrType: ErrPathTraversal},
	148	+	{name: "contains hyphen", key: "FOO-BAR", wantErr: true, wantErrType: ErrInvalidEnvKey},
	149	+	{name: "contains space", key: "FOO BAR", wantErr: true, wantErrType: ErrInvalidEnvKey},
137	150		}
138	151
139	152		for _, tt := range tests {
140	153		t.Run(tt.name, func(t *testing.T) {
141	154		err := ValidateEnvKey(tt.key)
142	155		if tt.wantErr {
143		-	assert.Error(t, err)
	156	+	require.Error(t, err)
	157	+	if tt.wantErrType != nil {
	158	+	assert.ErrorIs(t, err, tt.wantErrType)
	159	+	}
144	160		} else {
145	161		assert.NoError(t, err)
146	162		}

 		{name: "ends with dot", domain: "trailing.", wantErr: true},
 		{name: "space", domain: "has space", wantErr: true},
 		{name: "special chars", domain: "bad$domain", wantErr: true},
+		{name: "idempotent simple", domain: "example_com", want: "example_com"},
+		{name: "idempotent complex", domain: "my-app_example_com_8080", want: "my-app_example_com_8080"},
 	}
 	for _, tt := range tests {
 			data: "BIG=" + strings.Repeat("x", 100000),
 			want: map[string]string{"BIG": strings.Repeat("x", 100000)},
 		},
+		{
+			name: "quoted value with inner spaces",
+			data: `KEY="  hello  "`,
+			want: map[string]string{"KEY": "  hello  "},
+		},
+		{
+			name: "unquoted value with outer spaces",
+			data: "KEY=  hello  ",
+			want: map[string]string{"KEY": "hello"},
+		},
 	}
 	for _, tt := range tests {
 func TestValidateEnvKey(t *testing.T) {
 	tests := []struct {
-		name    string
+		name        string
-		key     string
+		key         string
-		wantErr bool
+		wantErr     bool
+		wantErrType error
 	}{
 		{name: "simple", key: "FOO", wantErr: false},
 		{name: "with underscore", key: "FOO_BAR", wantErr: false},
 		{name: "starts with underscore", key: "_PRIVATE", wantErr: false},
 		{name: "lowercase", key: "foo", wantErr: false},
 		{name: "mixed", key: "myApp_v2", wantErr: false},
-		{name: "empty", key: "", wantErr: true},
+		{name: "empty", key: "", wantErr: true, wantErrType: ErrInvalidEnvKey},
-		{name: "starts with number", key: "1BAD", wantErr: true},
+		{name: "starts with number", key: "1BAD", wantErr: true, wantErrType: ErrInvalidEnvKey},
-		{name: "contains dot", key: "FOO.BAR", wantErr: true},
+		{name: "contains dot", key: "FOO.BAR", wantErr: true, wantErrType: ErrInvalidEnvKey},
-		{name: "contains slash", key: "FOO/BAR", wantErr: true},
+		{name: "contains slash", key: "FOO/BAR", wantErr: true, wantErrType: ErrPathTraversal},
-		{name: "contains backslash", key: `FOO\BAR`, wantErr: true},
+		{name: "contains backslash", key: `FOO\BAR`, wantErr: true, wantErrType: ErrPathTraversal},
-		{name: "path traversal", key: "..", wantErr: true},
+		{name: "path traversal", key: "..", wantErr: true, wantErrType: ErrPathTraversal},
-		{name: "contains hyphen", key: "FOO-BAR", wantErr: true},
+		{name: "contains hyphen", key: "FOO-BAR", wantErr: true, wantErrType: ErrInvalidEnvKey},
-		{name: "contains space", key: "FOO BAR", wantErr: true},
+		{name: "contains space", key: "FOO BAR", wantErr: true, wantErrType: ErrInvalidEnvKey},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			err := ValidateEnvKey(tt.key)
 			if tt.wantErr {
-				assert.Error(t, err)
+				require.Error(t, err)
+				if tt.wantErrType != nil {
+					assert.ErrorIs(t, err, tt.wantErrType)
+				}
 			} else {
 				assert.NoError(t, err)
 			}

M internal/domain/errors.go +1 -0


@@ -63,6 +63,7 @@ var (
63	63		ErrInsufficientScope = errors.New("insufficient scope for operation")
64	64		ErrInvalidScope = errors.New("invalid scope format")
65	65		ErrSSRFBlocked = errors.New("request to internal/blocked network not allowed")
	66	+	ErrInvalidEnvKey = errors.New("invalid environment variable key")
66	67
67	68		// Attachment errors
68	69		ErrAttachmentNotFound = errors.New("attachment not found")


@@ -63,6 +63,7 @@ var (
63	ErrInsufficientScope = errors.New("insufficient scope for operation")	63		ErrInsufficientScope = errors.New("insufficient scope for operation")
64	ErrInvalidScope = errors.New("invalid scope format")	64		ErrInvalidScope = errors.New("invalid scope format")
65	ErrSSRFBlocked = errors.New("request to internal/blocked network not allowed")	65		ErrSSRFBlocked = errors.New("request to internal/blocked network not allowed")
		66	+	ErrInvalidEnvKey = errors.New("invalid environment variable key")
66		67
67	// Attachment errors	68		// Attachment errors
68	ErrAttachmentNotFound = errors.New("attachment not found")	69		ErrAttachmentNotFound = errors.New("attachment not found")