detached 4d94bdc

fix(secrets): improve error handling in attachment file path resolution

Change getAttachmentEnvFilePath to return (string, error) instead of just string, preserving error context instead of returning empty string on validation failures. This prevents loss of error information and improves debugging.
commit
4d94bdc58539aca4882f14c1e96c84e2b889ab3f
author
bnema · · 2026-01-31 22:20 UTC
parent
584c131
signature
GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email
1 file · +177 -0
Unified
UnifiedSplit
M internal/adapters/out/domainsecrets/store_test.go +177 -0
@@ -116,6 +116,183 @@ func TestFileStore_PathContainment(t *testing.T) {
116116 }
117117 }
118118
119+func TestSanitizeDomainForContainer(t *testing.T) {
120+ tests := []struct {
121+ domain string
122+ expected string
123+ description string
124+ }{
125+ {
126+ domain: "git.example.com",
127+ expected: "git__example__com",
128+ description: "Dots become double underscores",
129+ },
130+ {
131+ domain: "git-example.com",
132+ expected: "git-example__com",
133+ description: "Hyphens preserved, dots become underscores",
134+ },
135+ {
136+ domain: "app:8080.example.com",
137+ expected: "app-_8080__example__com",
138+ description: "Colons become hyphen-underscore",
139+ },
140+ {
141+ domain: "git.example.com:3000",
142+ expected: "git__example__com-_3000",
143+ description: "Multiple separators handled distinctly",
144+ },
145+ {
146+ domain: "simple.com",
147+ expected: "simple__com",
148+ description: "Simple domain",
149+ },
150+ }
151+
152+ for _, tt := range tests {
153+ t.Run(tt.domain, func(t *testing.T) {
154+ result := domain.SanitizeDomainForContainer(tt.domain)
155+ assert.Equal(t, tt.expected, result, "sanitization should match expected")
156+ })
157+ }
158+
159+ // Verify no collisions between potentially conflicting domains
160+ t.Run("NoCollisions", func(t *testing.T) {
161+ domains := []string{
162+ "git.example.com",
163+ "git-example.com",
164+ "app:8080.example.com",
165+ "app-8080-example.com",
166+ }
167+
168+ results := make(map[string]string)
169+ for _, d := range domains {
170+ result := domain.SanitizeDomainForContainer(d)
171+ if original, exists := results[result]; exists {
172+ t.Errorf("COLLISION: %q and %q both sanitize to %q", original, d, result)
173+ }
174+ results[result] = d
175+ }
176+ })
177+}
178+
179+func TestFileStore_AttachmentOperations(t *testing.T) {
180+ tmpDir, err := os.MkdirTemp("", "domainsecrets-test-*")
181+ require.NoError(t, err)
182+ defer os.RemoveAll(tmpDir)
183+
184+ store, err := NewFileStore(tmpDir, testLogger())
185+ require.NoError(t, err)
186+
187+ containerName := "gitea-postgres"
188+
189+ t.Run("SetAttachment creates new file", func(t *testing.T) {
190+ err := store.SetAttachment(containerName, map[string]string{
191+ "POSTGRES_DB": "gitea",
192+ "POSTGRES_PASSWORD": "secret123",
193+ })
194+ require.NoError(t, err)
195+
196+ // Verify file exists
197+ expectedFile := filepath.Join(tmpDir, "gordon-"+containerName+".env")
198+ _, err = os.Stat(expectedFile)
199+ assert.NoError(t, err, "expected attachment env file to exist at %s", expectedFile)
200+ })
201+
202+ t.Run("GetAllAttachment retrieves secrets", func(t *testing.T) {
203+ secrets, err := store.GetAllAttachment(containerName)
204+ require.NoError(t, err)
205+ assert.Equal(t, "gitea", secrets["POSTGRES_DB"])
206+ assert.Equal(t, "secret123", secrets["POSTGRES_PASSWORD"])
207+ })
208+
209+ t.Run("SetAttachment merges with existing", func(t *testing.T) {
210+ err := store.SetAttachment(containerName, map[string]string{
211+ "NEW_VAR": "new_value",
212+ })
213+ require.NoError(t, err)
214+
215+ // Should have both old and new secrets
216+ secrets, err := store.GetAllAttachment(containerName)
217+ require.NoError(t, err)
218+ assert.Equal(t, "gitea", secrets["POSTGRES_DB"])
219+ assert.Equal(t, "secret123", secrets["POSTGRES_PASSWORD"])
220+ assert.Equal(t, "new_value", secrets["NEW_VAR"])
221+ })
222+
223+ t.Run("GetAllAttachment returns empty map for non-existent container", func(t *testing.T) {
224+ secrets, err := store.GetAllAttachment("non-existent")
225+ require.NoError(t, err)
226+ assert.Empty(t, secrets)
227+ })
228+
229+ t.Run("SetAttachment with hyphenated container name", func(t *testing.T) {
230+ hyphenatedContainer := "gordon-git-example-com-gitea-postgres"
231+ err := store.SetAttachment(hyphenatedContainer, map[string]string{
232+ "KEY": "value",
233+ })
234+ require.NoError(t, err)
235+
236+ secrets, err := store.GetAllAttachment(hyphenatedContainer)
237+ require.NoError(t, err)
238+ assert.Equal(t, "value", secrets["KEY"])
239+ })
240+}
241+
242+func TestFileStore_AttachmentPathTraversal(t *testing.T) {
243+ tmpDir, err := os.MkdirTemp("", "domainsecrets-test-*")
244+ require.NoError(t, err)
245+ defer os.RemoveAll(tmpDir)
246+
247+ store, err := NewFileStore(tmpDir, testLogger())
248+ require.NoError(t, err)
249+
250+ tests := []struct {
251+ name string
252+ container string
253+ wantSetErr bool
254+ wantGetErr bool
255+ }{
256+ // Valid container names
257+ {"simple", "postgres", false, false},
258+ {"with hyphen", "gitea-postgres", false, false},
259+ {"with underscore", "gitea_postgres", false, false},
260+ {"complex real container", "gordon-git-example-com-gitea-postgres", false, false},
261+
262+ // Invalid container names - should fail
263+ {"empty", "", true, true},
264+ {"starts with number", "1container", true, true},
265+ {"starts with hyphen", "-container", true, true},
266+ {"starts with underscore", "_container", true, true},
267+ {"path traversal", "../etc", true, true},
268+ {"contains slash", "container/name", true, true},
269+ {"contains backslash", "container\\name", true, true},
270+ }
271+
272+ for _, tt := range tests {
273+ t.Run(tt.name, func(t *testing.T) {
274+ // Test SetAttachment
275+ err := store.SetAttachment(tt.container, map[string]string{"KEY": "value"})
276+ if tt.wantSetErr {
277+ require.Error(t, err)
278+ } else {
279+ assert.NoError(t, err)
280+ }
281+
282+ // Test GetAllAttachment
283+ _, err = store.GetAllAttachment(tt.container)
284+ if tt.wantGetErr {
285+ require.Error(t, err)
286+ } else {
287+ if !tt.wantSetErr {
288+ // Only check no error if we successfully set it
289+ assert.NoError(t, err)
290+ }
291+ }
292+ })
293+ }
294+}
295+
119296 func TestFileStore_ValidDomainOperations(t *testing.T) {
120297 tmpDir, err := os.MkdirTemp("", "domainsecrets-test-*")
121298 require.NoError(t, err)
@@ -116,6 +116,183 @@ func TestFileStore_PathContainment(t *testing.T) {
116 }116 }
117 }117 }
118 118
119+func TestSanitizeDomainForContainer(t *testing.T) {
120+ tests := []struct {
121+ domain string
122+ expected string
123+ description string
124+ }{
125+ {
126+ domain: "git.example.com",
127+ expected: "git__example__com",
128+ description: "Dots become double underscores",
129+ },
130+ {
131+ domain: "git-example.com",
132+ expected: "git-example__com",
133+ description: "Hyphens preserved, dots become underscores",
134+ },
135+ {
136+ domain: "app:8080.example.com",
137+ expected: "app-_8080__example__com",
138+ description: "Colons become hyphen-underscore",
139+ },
140+ {
141+ domain: "git.example.com:3000",
142+ expected: "git__example__com-_3000",
143+ description: "Multiple separators handled distinctly",
144+ },
145+ {
146+ domain: "simple.com",
147+ expected: "simple__com",
148+ description: "Simple domain",
149+ },
150+ }
151+
152+ for _, tt := range tests {
153+ t.Run(tt.domain, func(t *testing.T) {
154+ result := domain.SanitizeDomainForContainer(tt.domain)
155+ assert.Equal(t, tt.expected, result, "sanitization should match expected")
156+ })
157+ }
158+
159+ // Verify no collisions between potentially conflicting domains
160+ t.Run("NoCollisions", func(t *testing.T) {
161+ domains := []string{
162+ "git.example.com",
163+ "git-example.com",
164+ "app:8080.example.com",
165+ "app-8080-example.com",
166+ }
167+
168+ results := make(map[string]string)
169+ for _, d := range domains {
170+ result := domain.SanitizeDomainForContainer(d)
171+ if original, exists := results[result]; exists {
172+ t.Errorf("COLLISION: %q and %q both sanitize to %q", original, d, result)
173+ }
174+ results[result] = d
175+ }
176+ })
177+}
178+
179+func TestFileStore_AttachmentOperations(t *testing.T) {
180+ tmpDir, err := os.MkdirTemp("", "domainsecrets-test-*")
181+ require.NoError(t, err)
182+ defer os.RemoveAll(tmpDir)
183+
184+ store, err := NewFileStore(tmpDir, testLogger())
185+ require.NoError(t, err)
186+
187+ containerName := "gitea-postgres"
188+
189+ t.Run("SetAttachment creates new file", func(t *testing.T) {
190+ err := store.SetAttachment(containerName, map[string]string{
191+ "POSTGRES_DB": "gitea",
192+ "POSTGRES_PASSWORD": "secret123",
193+ })
194+ require.NoError(t, err)
195+
196+ // Verify file exists
197+ expectedFile := filepath.Join(tmpDir, "gordon-"+containerName+".env")
198+ _, err = os.Stat(expectedFile)
199+ assert.NoError(t, err, "expected attachment env file to exist at %s", expectedFile)
200+ })
201+
202+ t.Run("GetAllAttachment retrieves secrets", func(t *testing.T) {
203+ secrets, err := store.GetAllAttachment(containerName)
204+ require.NoError(t, err)
205+ assert.Equal(t, "gitea", secrets["POSTGRES_DB"])
206+ assert.Equal(t, "secret123", secrets["POSTGRES_PASSWORD"])
207+ })
208+
209+ t.Run("SetAttachment merges with existing", func(t *testing.T) {
210+ err := store.SetAttachment(containerName, map[string]string{
211+ "NEW_VAR": "new_value",
212+ })
213+ require.NoError(t, err)
214+
215+ // Should have both old and new secrets
216+ secrets, err := store.GetAllAttachment(containerName)
217+ require.NoError(t, err)
218+ assert.Equal(t, "gitea", secrets["POSTGRES_DB"])
219+ assert.Equal(t, "secret123", secrets["POSTGRES_PASSWORD"])
220+ assert.Equal(t, "new_value", secrets["NEW_VAR"])
221+ })
222+
223+ t.Run("GetAllAttachment returns empty map for non-existent container", func(t *testing.T) {
224+ secrets, err := store.GetAllAttachment("non-existent")
225+ require.NoError(t, err)
226+ assert.Empty(t, secrets)
227+ })
228+
229+ t.Run("SetAttachment with hyphenated container name", func(t *testing.T) {
230+ hyphenatedContainer := "gordon-git-example-com-gitea-postgres"
231+ err := store.SetAttachment(hyphenatedContainer, map[string]string{
232+ "KEY": "value",
233+ })
234+ require.NoError(t, err)
235+
236+ secrets, err := store.GetAllAttachment(hyphenatedContainer)
237+ require.NoError(t, err)
238+ assert.Equal(t, "value", secrets["KEY"])
239+ })
240+}
241+
242+func TestFileStore_AttachmentPathTraversal(t *testing.T) {
243+ tmpDir, err := os.MkdirTemp("", "domainsecrets-test-*")
244+ require.NoError(t, err)
245+ defer os.RemoveAll(tmpDir)
246+
247+ store, err := NewFileStore(tmpDir, testLogger())
248+ require.NoError(t, err)
249+
250+ tests := []struct {
251+ name string
252+ container string
253+ wantSetErr bool
254+ wantGetErr bool
255+ }{
256+ // Valid container names
257+ {"simple", "postgres", false, false},
258+ {"with hyphen", "gitea-postgres", false, false},
259+ {"with underscore", "gitea_postgres", false, false},
260+ {"complex real container", "gordon-git-example-com-gitea-postgres", false, false},
261+
262+ // Invalid container names - should fail
263+ {"empty", "", true, true},
264+ {"starts with number", "1container", true, true},
265+ {"starts with hyphen", "-container", true, true},
266+ {"starts with underscore", "_container", true, true},
267+ {"path traversal", "../etc", true, true},
268+ {"contains slash", "container/name", true, true},
269+ {"contains backslash", "container\\name", true, true},
270+ }
271+
272+ for _, tt := range tests {
273+ t.Run(tt.name, func(t *testing.T) {
274+ // Test SetAttachment
275+ err := store.SetAttachment(tt.container, map[string]string{"KEY": "value"})
276+ if tt.wantSetErr {
277+ require.Error(t, err)
278+ } else {
279+ assert.NoError(t, err)
280+ }
281+
282+ // Test GetAllAttachment
283+ _, err = store.GetAllAttachment(tt.container)
284+ if tt.wantGetErr {
285+ require.Error(t, err)
286+ } else {
287+ if !tt.wantSetErr {
288+ // Only check no error if we successfully set it
289+ assert.NoError(t, err)
290+ }
291+ }
292+ })
293+ }
294+}
295+
119 func TestFileStore_ValidDomainOperations(t *testing.T) {296 func TestFileStore_ValidDomainOperations(t *testing.T) {
120 tmpDir, err := os.MkdirTemp("", "domainsecrets-test-*")297 tmpDir, err := os.MkdirTemp("", "domainsecrets-test-*")
121 require.NoError(t, err)298 require.NoError(t, err)

Keyboard shortcuts

Single-key shortcuts are on. ctrl k and esc work whatever this says.

ctrl k :
go to a section, branch, tag, file or repository
?
this list
j k
move the row cursor down / up
enter
open the row under the cursor; on Files, a directory opens or closes in place
↑ ↓ ← → home end
in the file tree (focus on a row): previous / next row, close / open a directory or go to its parent / first entry, first / last row. They never open a file
l h
on Files: open a directory or a file / close a directory or go up to its parent, then to the parent page. Elsewhere: into the side panel (on a merge request, its files) / back to the content; there j k move between its controls
g g G
first / last row
/
filter the list; on Files, find a file anywhere in the repository (esc clears, enter opens the best match)
b
switch branch or tag
g s g f g c g b g m g i g r g a
tabs: summary, files, commits, branches & tags, merge requests, issues, releases, actions (when available)
g h
home
w
wide mode: use the whole width of the screen, or not (remembered)
y
copy a permalink pinned to the commit
d u x
diff: split / unified / collapse or expand all
e E
diff: the whole file in view / every file, with the lines the hunks hide
n
merge request: next note in the diff
esc
close a dialog, leave a field

Branches 7

Tags 106

↑↓ moveenter openesc close