detached 9cc9012

fix(security): prevent command injection in pass provider

commit
9cc901258b3da5369909b5e39dc59b4e3471b91c
author
bnema · · 2026-01-17 20:55 UTC
parent
fe25b4f
signature
GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email
2 files · +248 -0
Unified
UnifiedSplit
M internal/adapters/out/secrets/pass.go +45 -0
@@ -5,12 +5,20 @@ import (
55 "context"
66 "fmt"
77 "os/exec"
8+ "regexp"
89 "strings"
910 "time"
1011
1112 "github.com/bnema/zerowrap"
13+
14+ "gordon/internal/domain"
1215 )
1316
17+// passPathRegex validates pass paths.
18+// Allows alphanumeric characters, forward slashes, underscores, dots, and hyphens.
19+// This prevents command injection via shell metacharacters.
20+var passPathRegex = regexp.MustCompile(`^[a-zA-Z0-9/_.-]+$`)
21+
1422 // PassProvider implements the SecretProvider interface using the pass password manager.
1523 type PassProvider struct {
1624 timeout time.Duration
@@ -30,8 +38,45 @@ func (p *PassProvider) Name() string {
3038 return "pass"
3139 }
3240
41+// ValidatePath validates a pass path to prevent command injection and path traversal.
42+// Returns an error if the path is invalid.
43+func ValidatePath(path string) error {
44+ if path == "" {
45+ return fmt.Errorf("path cannot be empty")
46+ }
47+
48+ // SECURITY: Validate path format to prevent command injection
49+ if !passPathRegex.MatchString(path) {
50+ return fmt.Errorf("invalid path: must contain only alphanumeric characters, /, _, ., -")
51+ }
52+
53+ // SECURITY: Prevent path traversal
54+ if strings.Contains(path, "..") {
55+ return domain.ErrPathTraversal
56+ }
57+
58+ // SECURITY: Reject absolute paths
59+ if strings.HasPrefix(path, "/") {
60+ return fmt.Errorf("invalid path: absolute paths not allowed")
61+ }
62+
63+ return nil
64+}
65+
3366 // GetSecret retrieves a secret from pass by path.
3467 func (p *PassProvider) GetSecret(ctx context.Context, path string) (string, error) {
68+ // SECURITY: Validate path before executing command
69+ if err := ValidatePath(path); err != nil {
70+ p.log.Warn().
71+ Str(zerowrap.FieldLayer, "adapter").
72+ Str(zerowrap.FieldAdapter, "secrets").
73+ Str("provider", "pass").
74+ Str("path", path).
75+ Err(err).
76+ Msg("rejected invalid pass path")
77+ return "", err
78+ }
79+
3580 ctx, cancel := context.WithTimeout(ctx, p.timeout)
3681 defer cancel()
3782
@@ -5,12 +5,20 @@ import (
5 "context"5 "context"
6 "fmt"6 "fmt"
7 "os/exec"7 "os/exec"
8+ "regexp"
8 "strings"9 "strings"
9 "time"10 "time"
10 11
11 "github.com/bnema/zerowrap"12 "github.com/bnema/zerowrap"
13+
14+ "gordon/internal/domain"
12 )15 )
13 16
17+// passPathRegex validates pass paths.
18+// Allows alphanumeric characters, forward slashes, underscores, dots, and hyphens.
19+// This prevents command injection via shell metacharacters.
20+var passPathRegex = regexp.MustCompile(`^[a-zA-Z0-9/_.-]+$`)
21+
14 // PassProvider implements the SecretProvider interface using the pass password manager.22 // PassProvider implements the SecretProvider interface using the pass password manager.
15 type PassProvider struct {23 type PassProvider struct {
16 timeout time.Duration24 timeout time.Duration
@@ -30,8 +38,45 @@ func (p *PassProvider) Name() string {
30 return "pass"38 return "pass"
31 }39 }
32 40
41+// ValidatePath validates a pass path to prevent command injection and path traversal.
42+// Returns an error if the path is invalid.
43+func ValidatePath(path string) error {
44+ if path == "" {
45+ return fmt.Errorf("path cannot be empty")
46+ }
47+
48+ // SECURITY: Validate path format to prevent command injection
49+ if !passPathRegex.MatchString(path) {
50+ return fmt.Errorf("invalid path: must contain only alphanumeric characters, /, _, ., -")
51+ }
52+
53+ // SECURITY: Prevent path traversal
54+ if strings.Contains(path, "..") {
55+ return domain.ErrPathTraversal
56+ }
57+
58+ // SECURITY: Reject absolute paths
59+ if strings.HasPrefix(path, "/") {
60+ return fmt.Errorf("invalid path: absolute paths not allowed")
61+ }
62+
63+ return nil
64+}
65+
33 // GetSecret retrieves a secret from pass by path.66 // GetSecret retrieves a secret from pass by path.
34 func (p *PassProvider) GetSecret(ctx context.Context, path string) (string, error) {67 func (p *PassProvider) GetSecret(ctx context.Context, path string) (string, error) {
68+ // SECURITY: Validate path before executing command
69+ if err := ValidatePath(path); err != nil {
70+ p.log.Warn().
71+ Str(zerowrap.FieldLayer, "adapter").
72+ Str(zerowrap.FieldAdapter, "secrets").
73+ Str("provider", "pass").
74+ Str("path", path).
75+ Err(err).
76+ Msg("rejected invalid pass path")
77+ return "", err
78+ }
79+
35 ctx, cancel := context.WithTimeout(ctx, p.timeout)80 ctx, cancel := context.WithTimeout(ctx, p.timeout)
36 defer cancel()81 defer cancel()
37 82
A internal/adapters/out/secrets/pass_test.go +203 -0
@@ -0,0 +1,203 @@
1+package secrets
2+
3+import (
4+ "context"
5+ "errors"
6+ "testing"
7+
8+ "github.com/bnema/zerowrap"
9+ "github.com/stretchr/testify/assert"
10+ "github.com/stretchr/testify/require"
11+
12+ "gordon/internal/domain"
13+)
14+
15+func TestValidatePath(t *testing.T) {
16+ tests := []struct {
17+ name string
18+ path string
19+ wantErr bool
20+ errType error
21+ }{
22+ // Valid paths
23+ {
24+ name: "simple path",
25+ path: "gordon/registry/secret",
26+ wantErr: false,
27+ },
28+ {
29+ name: "path with dots in name",
30+ path: "my.secret.key",
31+ wantErr: false,
32+ },
33+ {
34+ name: "path with hyphens",
35+ path: "my-app/api-key",
36+ wantErr: false,
37+ },
38+ {
39+ name: "path with underscores",
40+ path: "my_app/api_key",
41+ wantErr: false,
42+ },
43+ {
44+ name: "nested path",
45+ path: "org/team/app/secret",
46+ wantErr: false,
47+ },
48+
49+ // Invalid paths - path traversal
50+ {
51+ name: "path traversal with double dots",
52+ path: "../../../etc/passwd",
53+ wantErr: true,
54+ errType: domain.ErrPathTraversal,
55+ },
56+ {
57+ name: "path traversal in middle",
58+ path: "foo/../../../etc/passwd",
59+ wantErr: true,
60+ errType: domain.ErrPathTraversal,
61+ },
62+ {
63+ name: "path traversal at end",
64+ path: "foo/bar/..",
65+ wantErr: true,
66+ errType: domain.ErrPathTraversal,
67+ },
68+ {
69+ name: "encoded path traversal",
70+ path: "..%2f..%2fetc/passwd",
71+ wantErr: true, // % is not allowed by regex
72+ },
73+
74+ // Invalid paths - absolute paths
75+ {
76+ name: "absolute path",
77+ path: "/etc/passwd",
78+ wantErr: true,
79+ },
80+ {
81+ name: "absolute path with valid chars",
82+ path: "/gordon/secret",
83+ wantErr: true,
84+ },
85+
86+ // Invalid paths - special characters (command injection)
87+ {
88+ name: "semicolon injection",
89+ path: "secret;rm -rf /",
90+ wantErr: true,
91+ },
92+ {
93+ name: "pipe injection",
94+ path: "secret|cat /etc/passwd",
95+ wantErr: true,
96+ },
97+ {
98+ name: "backtick injection",
99+ path: "secret`whoami`",
100+ wantErr: true,
101+ },
102+ {
103+ name: "dollar injection",
104+ path: "secret$(whoami)",
105+ wantErr: true,
106+ },
107+ {
108+ name: "ampersand injection",
109+ path: "secret&rm -rf /",
110+ wantErr: true,
111+ },
112+ {
113+ name: "newline injection",
114+ path: "secret\nrm -rf /",
115+ wantErr: true,
116+ },
117+ {
118+ name: "space in path",
119+ path: "secret with space",
120+ wantErr: true,
121+ },
122+ {
123+ name: "null byte injection",
124+ path: "secret\x00evil",
125+ wantErr: true,
126+ },
127+
128+ // Edge cases
129+ {
130+ name: "empty path",
131+ path: "",
132+ wantErr: true,
133+ },
134+ {
135+ name: "single dot",
136+ path: ".",
137+ wantErr: false, // single dot is valid, not traversal
138+ },
139+ }
140+
141+ for _, tt := range tests {
142+ t.Run(tt.name, func(t *testing.T) {
143+ err := ValidatePath(tt.path)
144+
145+ if tt.wantErr {
146+ require.Error(t, err, "expected error for path: %q", tt.path)
147+ if tt.errType != nil {
148+ assert.True(t, errors.Is(err, tt.errType), "expected error type %v, got %v", tt.errType, err)
149+ }
150+ } else {
151+ assert.NoError(t, err, "unexpected error for path: %q", tt.path)
152+ }
153+ })
154+ }
155+}
156+
157+func testLogger() zerowrap.Logger {
158+ return zerowrap.Default()
159+}
160+
161+func TestPassProvider_GetSecret_PathValidation(t *testing.T) {
162+ provider := NewPassProvider(testLogger())
163+
164+ tests := []struct {
165+ name string
166+ path string
167+ wantErr bool
168+ }{
169+ {
170+ name: "valid path rejected due to pass not installed",
171+ path: "gordon/test/secret",
172+ wantErr: true, // Will fail because pass command doesn't exist in test, but path is valid
173+ },
174+ {
175+ name: "path traversal rejected before command execution",
176+ path: "../../../etc/passwd",
177+ wantErr: true,
178+ },
179+ {
180+ name: "command injection rejected before command execution",
181+ path: "secret;whoami",
182+ wantErr: true,
183+ },
184+ }
185+
186+ for _, tt := range tests {
187+ t.Run(tt.name, func(t *testing.T) {
188+ _, err := provider.GetSecret(context.Background(), tt.path)
189+
190+ if tt.wantErr {
191+ require.Error(t, err)
192+ }
193+
194+ // For security-related rejections, verify the error is from validation, not command execution
195+ if tt.path == "../../../etc/passwd" {
196+ assert.True(t, errors.Is(err, domain.ErrPathTraversal))
197+ }
198+ if tt.path == "secret;whoami" {
199+ assert.Contains(t, err.Error(), "invalid path")
200+ }
201+ })
202+ }
203+}
@@ -0,0 +1,203 @@
1+package secrets
2+
3+import (
4+ "context"
5+ "errors"
6+ "testing"
7+
8+ "github.com/bnema/zerowrap"
9+ "github.com/stretchr/testify/assert"
10+ "github.com/stretchr/testify/require"
11+
12+ "gordon/internal/domain"
13+)
14+
15+func TestValidatePath(t *testing.T) {
16+ tests := []struct {
17+ name string
18+ path string
19+ wantErr bool
20+ errType error
21+ }{
22+ // Valid paths
23+ {
24+ name: "simple path",
25+ path: "gordon/registry/secret",
26+ wantErr: false,
27+ },
28+ {
29+ name: "path with dots in name",
30+ path: "my.secret.key",
31+ wantErr: false,
32+ },
33+ {
34+ name: "path with hyphens",
35+ path: "my-app/api-key",
36+ wantErr: false,
37+ },
38+ {
39+ name: "path with underscores",
40+ path: "my_app/api_key",
41+ wantErr: false,
42+ },
43+ {
44+ name: "nested path",
45+ path: "org/team/app/secret",
46+ wantErr: false,
47+ },
48+
49+ // Invalid paths - path traversal
50+ {
51+ name: "path traversal with double dots",
52+ path: "../../../etc/passwd",
53+ wantErr: true,
54+ errType: domain.ErrPathTraversal,
55+ },
56+ {
57+ name: "path traversal in middle",
58+ path: "foo/../../../etc/passwd",
59+ wantErr: true,
60+ errType: domain.ErrPathTraversal,
61+ },
62+ {
63+ name: "path traversal at end",
64+ path: "foo/bar/..",
65+ wantErr: true,
66+ errType: domain.ErrPathTraversal,
67+ },
68+ {
69+ name: "encoded path traversal",
70+ path: "..%2f..%2fetc/passwd",
71+ wantErr: true, // % is not allowed by regex
72+ },
73+
74+ // Invalid paths - absolute paths
75+ {
76+ name: "absolute path",
77+ path: "/etc/passwd",
78+ wantErr: true,
79+ },
80+ {
81+ name: "absolute path with valid chars",
82+ path: "/gordon/secret",
83+ wantErr: true,
84+ },
85+
86+ // Invalid paths - special characters (command injection)
87+ {
88+ name: "semicolon injection",
89+ path: "secret;rm -rf /",
90+ wantErr: true,
91+ },
92+ {
93+ name: "pipe injection",
94+ path: "secret|cat /etc/passwd",
95+ wantErr: true,
96+ },
97+ {
98+ name: "backtick injection",
99+ path: "secret`whoami`",
100+ wantErr: true,
101+ },
102+ {
103+ name: "dollar injection",
104+ path: "secret$(whoami)",
105+ wantErr: true,
106+ },
107+ {
108+ name: "ampersand injection",
109+ path: "secret&rm -rf /",
110+ wantErr: true,
111+ },
112+ {
113+ name: "newline injection",
114+ path: "secret\nrm -rf /",
115+ wantErr: true,
116+ },
117+ {
118+ name: "space in path",
119+ path: "secret with space",
120+ wantErr: true,
121+ },
122+ {
123+ name: "null byte injection",
124+ path: "secret\x00evil",
125+ wantErr: true,
126+ },
127+
128+ // Edge cases
129+ {
130+ name: "empty path",
131+ path: "",
132+ wantErr: true,
133+ },
134+ {
135+ name: "single dot",
136+ path: ".",
137+ wantErr: false, // single dot is valid, not traversal
138+ },
139+ }
140+
141+ for _, tt := range tests {
142+ t.Run(tt.name, func(t *testing.T) {
143+ err := ValidatePath(tt.path)
144+
145+ if tt.wantErr {
146+ require.Error(t, err, "expected error for path: %q", tt.path)
147+ if tt.errType != nil {
148+ assert.True(t, errors.Is(err, tt.errType), "expected error type %v, got %v", tt.errType, err)
149+ }
150+ } else {
151+ assert.NoError(t, err, "unexpected error for path: %q", tt.path)
152+ }
153+ })
154+ }
155+}
156+
157+func testLogger() zerowrap.Logger {
158+ return zerowrap.Default()
159+}
160+
161+func TestPassProvider_GetSecret_PathValidation(t *testing.T) {
162+ provider := NewPassProvider(testLogger())
163+
164+ tests := []struct {
165+ name string
166+ path string
167+ wantErr bool
168+ }{
169+ {
170+ name: "valid path rejected due to pass not installed",
171+ path: "gordon/test/secret",
172+ wantErr: true, // Will fail because pass command doesn't exist in test, but path is valid
173+ },
174+ {
175+ name: "path traversal rejected before command execution",
176+ path: "../../../etc/passwd",
177+ wantErr: true,
178+ },
179+ {
180+ name: "command injection rejected before command execution",
181+ path: "secret;whoami",
182+ wantErr: true,
183+ },
184+ }
185+
186+ for _, tt := range tests {
187+ t.Run(tt.name, func(t *testing.T) {
188+ _, err := provider.GetSecret(context.Background(), tt.path)
189+
190+ if tt.wantErr {
191+ require.Error(t, err)
192+ }
193+
194+ // For security-related rejections, verify the error is from validation, not command execution
195+ if tt.path == "../../../etc/passwd" {
196+ assert.True(t, errors.Is(err, domain.ErrPathTraversal))
197+ }
198+ if tt.path == "secret;whoami" {
199+ assert.Contains(t, err.Error(), "invalid path")
200+ }
201+ })
202+ }
203+}

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