Harden auth secret handling and admin caching
- commit
- ec63ba92dcebcea95e4b53a2b16082461043c3b6
- author
- brice · · 2026-03-05 21:31 UTC
- parent
- 36dfce1
- signature
- unsigned
13 files · +162 -50
M internal/adapters/in/http/admin/middleware.go +9 -0
| @@ -106,6 +106,11 @@ func AuthMiddleware( | |||
| 106 | 106 | ctx = context.WithValue(ctx, domain.ContextKeySubject, claims.Subject) | |
| 107 | 107 | ctx = context.WithValue(ctx, domain.TokenClaimsKey, claims) | |
| 108 | 108 | ||
| 109 | + | // Prevent intermediaries from storing admin API responses, especially | |
| 110 | + | // when a rotated bearer token is returned in X-Gordon-Token. | |
| 111 | + | w.Header().Set("Cache-Control", "no-store") | |
| 112 | + | w.Header().Set("Pragma", "no-cache") | |
| 113 | + | ||
| 109 | 114 | // Attempt to slide token expiry. Non-fatal if it fails. | |
| 110 | 115 | // The new token is returned in X-Gordon-Token so the CLI can persist it atomically. | |
| 111 | 116 | if newToken, extErr := authSvc.ExtendToken(ctx, token); extErr != nil { | |
| @@ -163,6 +168,8 @@ func HasAccess(ctx context.Context, resource, action string) bool { | |||
| 163 | 168 | ||
| 164 | 169 | func sendUnauthorized(w http.ResponseWriter, message string) { | |
| 165 | 170 | w.Header().Set("Content-Type", "application/json") | |
| 171 | + | w.Header().Set("Cache-Control", "no-store") | |
| 172 | + | w.Header().Set("Pragma", "no-cache") | |
| 166 | 173 | w.Header().Set("WWW-Authenticate", `Bearer realm="gordon-admin"`) | |
| 167 | 174 | w.WriteHeader(http.StatusUnauthorized) | |
| 168 | 175 | _ = json.NewEncoder(w).Encode(dto.ErrorResponse{Error: message}) | |
| @@ -170,6 +177,8 @@ func sendUnauthorized(w http.ResponseWriter, message string) { | |||
| 170 | 177 | ||
| 171 | 178 | func sendForbidden(w http.ResponseWriter, message string) { | |
| 172 | 179 | w.Header().Set("Content-Type", "application/json") | |
| 180 | + | w.Header().Set("Cache-Control", "no-store") | |
| 181 | + | w.Header().Set("Pragma", "no-cache") | |
| 173 | 182 | w.WriteHeader(http.StatusForbidden) | |
| 174 | 183 | _ = json.NewEncoder(w).Encode(dto.ErrorResponse{Error: message}) | |
| 175 | 184 | } | |
| @@ -106,6 +106,11 @@ func AuthMiddleware( | |||||
| 106 | ctx = context.WithValue(ctx, domain.ContextKeySubject, claims.Subject) | 106 | ctx = context.WithValue(ctx, domain.ContextKeySubject, claims.Subject) | ||
| 107 | ctx = context.WithValue(ctx, domain.TokenClaimsKey, claims) | 107 | ctx = context.WithValue(ctx, domain.TokenClaimsKey, claims) | ||
| 108 | 108 | ||||
| 109 | + | // Prevent intermediaries from storing admin API responses, especially | |||
| 110 | + | // when a rotated bearer token is returned in X-Gordon-Token. | |||
| 111 | + | w.Header().Set("Cache-Control", "no-store") | |||
| 112 | + | w.Header().Set("Pragma", "no-cache") | |||
| 113 | + | ||||
| 109 | // Attempt to slide token expiry. Non-fatal if it fails. | 114 | // Attempt to slide token expiry. Non-fatal if it fails. | ||
| 110 | // The new token is returned in X-Gordon-Token so the CLI can persist it atomically. | 115 | // The new token is returned in X-Gordon-Token so the CLI can persist it atomically. | ||
| 111 | if newToken, extErr := authSvc.ExtendToken(ctx, token); extErr != nil { | 116 | if newToken, extErr := authSvc.ExtendToken(ctx, token); extErr != nil { | ||
| @@ -163,6 +168,8 @@ func HasAccess(ctx context.Context, resource, action string) bool { | |||||
| 163 | 168 | ||||
| 164 | func sendUnauthorized(w http.ResponseWriter, message string) { | 169 | func sendUnauthorized(w http.ResponseWriter, message string) { | ||
| 165 | w.Header().Set("Content-Type", "application/json") | 170 | w.Header().Set("Content-Type", "application/json") | ||
| 171 | + | w.Header().Set("Cache-Control", "no-store") | |||
| 172 | + | w.Header().Set("Pragma", "no-cache") | |||
| 166 | w.Header().Set("WWW-Authenticate", `Bearer realm="gordon-admin"`) | 173 | w.Header().Set("WWW-Authenticate", `Bearer realm="gordon-admin"`) | ||
| 167 | w.WriteHeader(http.StatusUnauthorized) | 174 | w.WriteHeader(http.StatusUnauthorized) | ||
| 168 | _ = json.NewEncoder(w).Encode(dto.ErrorResponse{Error: message}) | 175 | _ = json.NewEncoder(w).Encode(dto.ErrorResponse{Error: message}) | ||
| @@ -170,6 +177,8 @@ func sendUnauthorized(w http.ResponseWriter, message string) { | |||||
| 170 | 177 | ||||
| 171 | func sendForbidden(w http.ResponseWriter, message string) { | 178 | func sendForbidden(w http.ResponseWriter, message string) { | ||
| 172 | w.Header().Set("Content-Type", "application/json") | 179 | w.Header().Set("Content-Type", "application/json") | ||
| 180 | + | w.Header().Set("Cache-Control", "no-store") | |||
| 181 | + | w.Header().Set("Pragma", "no-cache") | |||
| 173 | w.WriteHeader(http.StatusForbidden) | 182 | w.WriteHeader(http.StatusForbidden) | ||
| 174 | _ = json.NewEncoder(w).Encode(dto.ErrorResponse{Error: message}) | 183 | _ = json.NewEncoder(w).Encode(dto.ErrorResponse{Error: message}) | ||
| 175 | } | 184 | } | ||
M internal/adapters/in/http/admin/middleware_test.go +6 -0
| @@ -181,6 +181,8 @@ func TestAuthMiddleware_MissingAuthHeader(t *testing.T) { | |||
| 181 | 181 | ||
| 182 | 182 | assert.Equal(t, http.StatusUnauthorized, rec.Code) | |
| 183 | 183 | assert.Contains(t, rec.Body.String(), "missing authorization header") | |
| 184 | + | assert.Equal(t, "no-store", rec.Header().Get("Cache-Control")) | |
| 185 | + | assert.Equal(t, "no-cache", rec.Header().Get("Pragma")) | |
| 184 | 186 | } | |
| 185 | 187 | ||
| 186 | 188 | func TestAuthMiddleware_InvalidToken(t *testing.T) { | |
| @@ -390,6 +392,8 @@ func TestAuthMiddleware_ExtendTokenFailureIsNonFatal(t *testing.T) { | |||
| 390 | 392 | ||
| 391 | 393 | assert.Equal(t, http.StatusOK, rec.Code) | |
| 392 | 394 | assert.Empty(t, rec.Header().Get("X-Gordon-Token")) | |
| 395 | + | assert.Equal(t, "no-store", rec.Header().Get("Cache-Control")) | |
| 396 | + | assert.Equal(t, "no-cache", rec.Header().Get("Pragma")) | |
| 393 | 397 | } | |
| 394 | 398 | ||
| 395 | 399 | func TestAuthMiddleware_ExtendTokenRotationHeader(t *testing.T) { | |
| @@ -417,6 +421,8 @@ func TestAuthMiddleware_ExtendTokenRotationHeader(t *testing.T) { | |||
| 417 | 421 | ||
| 418 | 422 | assert.Equal(t, http.StatusOK, rec.Code) | |
| 419 | 423 | assert.Equal(t, "rotated-token", rec.Header().Get("X-Gordon-Token")) | |
| 424 | + | assert.Equal(t, "no-store", rec.Header().Get("Cache-Control")) | |
| 425 | + | assert.Equal(t, "no-cache", rec.Header().Get("Pragma")) | |
| 420 | 426 | } | |
| 421 | 427 | ||
| 422 | 428 | func TestHasAccess(t *testing.T) { | |
| @@ -181,6 +181,8 @@ func TestAuthMiddleware_MissingAuthHeader(t *testing.T) { | |||||
| 181 | 181 | ||||
| 182 | assert.Equal(t, http.StatusUnauthorized, rec.Code) | 182 | assert.Equal(t, http.StatusUnauthorized, rec.Code) | ||
| 183 | assert.Contains(t, rec.Body.String(), "missing authorization header") | 183 | assert.Contains(t, rec.Body.String(), "missing authorization header") | ||
| 184 | + | assert.Equal(t, "no-store", rec.Header().Get("Cache-Control")) | |||
| 185 | + | assert.Equal(t, "no-cache", rec.Header().Get("Pragma")) | |||
| 184 | } | 186 | } | ||
| 185 | 187 | ||||
| 186 | func TestAuthMiddleware_InvalidToken(t *testing.T) { | 188 | func TestAuthMiddleware_InvalidToken(t *testing.T) { | ||
| @@ -390,6 +392,8 @@ func TestAuthMiddleware_ExtendTokenFailureIsNonFatal(t *testing.T) { | |||||
| 390 | 392 | ||||
| 391 | assert.Equal(t, http.StatusOK, rec.Code) | 393 | assert.Equal(t, http.StatusOK, rec.Code) | ||
| 392 | assert.Empty(t, rec.Header().Get("X-Gordon-Token")) | 394 | assert.Empty(t, rec.Header().Get("X-Gordon-Token")) | ||
| 395 | + | assert.Equal(t, "no-store", rec.Header().Get("Cache-Control")) | |||
| 396 | + | assert.Equal(t, "no-cache", rec.Header().Get("Pragma")) | |||
| 393 | } | 397 | } | ||
| 394 | 398 | ||||
| 395 | func TestAuthMiddleware_ExtendTokenRotationHeader(t *testing.T) { | 399 | func TestAuthMiddleware_ExtendTokenRotationHeader(t *testing.T) { | ||
| @@ -417,6 +421,8 @@ func TestAuthMiddleware_ExtendTokenRotationHeader(t *testing.T) { | |||||
| 417 | 421 | ||||
| 418 | assert.Equal(t, http.StatusOK, rec.Code) | 422 | assert.Equal(t, http.StatusOK, rec.Code) | ||
| 419 | assert.Equal(t, "rotated-token", rec.Header().Get("X-Gordon-Token")) | 423 | assert.Equal(t, "rotated-token", rec.Header().Get("X-Gordon-Token")) | ||
| 424 | + | assert.Equal(t, "no-store", rec.Header().Get("Cache-Control")) | |||
| 425 | + | assert.Equal(t, "no-cache", rec.Header().Get("Pragma")) | |||
| 420 | } | 426 | } | ||
| 421 | 427 | ||||
| 422 | func TestHasAccess(t *testing.T) { | 428 | func TestHasAccess(t *testing.T) { | ||
M internal/adapters/out/domainsecrets/store_test.go +23 -5
| @@ -101,7 +101,7 @@ func TestFileStore_PathContainment(t *testing.T) { | |||
| 101 | 101 | require.NoError(t, err) | |
| 102 | 102 | ||
| 103 | 103 | // Verify file exists inside tmpDir | |
| 104 | - | expectedFile := filepath.Join(tmpDir, "example_com.env") | |
| 104 | + | expectedFile := filepath.Join(tmpDir, "example__com.env") | |
| 105 | 105 | _, err = os.Stat(expectedFile) | |
| 106 | 106 | assert.NoError(t, err, "expected env file to exist at %s", expectedFile) | |
| 107 | 107 | ||
| @@ -116,6 +116,24 @@ func TestFileStore_PathContainment(t *testing.T) { | |||
| 116 | 116 | } | |
| 117 | 117 | } | |
| 118 | 118 | ||
| 119 | + | func TestFileStore_DistinctDomainsUseDistinctFiles(t *testing.T) { | |
| 120 | + | store, err := NewFileStore(t.TempDir(), testLogger()) | |
| 121 | + | require.NoError(t, err) | |
| 122 | + | ||
| 123 | + | dotPath, err := store.validateEnvFilePath("app.example.com") | |
| 124 | + | require.NoError(t, err) | |
| 125 | + | ||
| 126 | + | portPath, err := store.validateEnvFilePath("app:example:com") | |
| 127 | + | require.NoError(t, err) | |
| 128 | + | ||
| 129 | + | pathPath, err := store.validateEnvFilePath("app/example/com") | |
| 130 | + | require.NoError(t, err) | |
| 131 | + | ||
| 132 | + | assert.NotEqual(t, dotPath, portPath) | |
| 133 | + | assert.NotEqual(t, dotPath, pathPath) | |
| 134 | + | assert.NotEqual(t, portPath, pathPath) | |
| 135 | + | } | |
| 136 | + | ||
| 119 | 137 | func TestSanitizeDomainForContainer(t *testing.T) { | |
| 120 | 138 | tests := []struct { | |
| 121 | 139 | domain string | |
| @@ -389,10 +407,10 @@ func TestGetEnvFilePath(t *testing.T) { | |||
| 389 | 407 | wantEmpty bool | |
| 390 | 408 | wantFilename string | |
| 391 | 409 | }{ | |
| 392 | - | {"example.com", false, "example_com.env"}, | |
| 393 | - | {"sub.example.com", false, "sub_example_com.env"}, | |
| 394 | - | {"example.com:8080", false, "example_com_8080.env"}, | |
| 395 | - | {"example.com/path", false, "example_com_path.env"}, | |
| 410 | + | {"example.com", false, "example__com.env"}, | |
| 411 | + | {"sub.example.com", false, "sub__example__com.env"}, | |
| 412 | + | {"example.com:8080", false, "example__com-_8080.env"}, | |
| 413 | + | {"example.com/path", false, "example__com--path.env"}, | |
| 396 | 414 | {"../evil", true, ""}, | |
| 397 | 415 | {"", true, ""}, | |
| 398 | 416 | } | |
| @@ -101,7 +101,7 @@ func TestFileStore_PathContainment(t *testing.T) { | |||||
| 101 | require.NoError(t, err) | 101 | require.NoError(t, err) | ||
| 102 | 102 | ||||
| 103 | // Verify file exists inside tmpDir | 103 | // Verify file exists inside tmpDir | ||
| 104 | - | expectedFile := filepath.Join(tmpDir, "example_com.env") | 104 | + | expectedFile := filepath.Join(tmpDir, "example__com.env") |
| 105 | _, err = os.Stat(expectedFile) | 105 | _, err = os.Stat(expectedFile) | ||
| 106 | assert.NoError(t, err, "expected env file to exist at %s", expectedFile) | 106 | assert.NoError(t, err, "expected env file to exist at %s", expectedFile) | ||
| 107 | 107 | ||||
| @@ -116,6 +116,24 @@ func TestFileStore_PathContainment(t *testing.T) { | |||||
| 116 | } | 116 | } | ||
| 117 | } | 117 | } | ||
| 118 | 118 | ||||
| 119 | + | func TestFileStore_DistinctDomainsUseDistinctFiles(t *testing.T) { | |||
| 120 | + | store, err := NewFileStore(t.TempDir(), testLogger()) | |||
| 121 | + | require.NoError(t, err) | |||
| 122 | + | ||||
| 123 | + | dotPath, err := store.validateEnvFilePath("app.example.com") | |||
| 124 | + | require.NoError(t, err) | |||
| 125 | + | ||||
| 126 | + | portPath, err := store.validateEnvFilePath("app:example:com") | |||
| 127 | + | require.NoError(t, err) | |||
| 128 | + | ||||
| 129 | + | pathPath, err := store.validateEnvFilePath("app/example/com") | |||
| 130 | + | require.NoError(t, err) | |||
| 131 | + | ||||
| 132 | + | assert.NotEqual(t, dotPath, portPath) | |||
| 133 | + | assert.NotEqual(t, dotPath, pathPath) | |||
| 134 | + | assert.NotEqual(t, portPath, pathPath) | |||
| 135 | + | } | |||
| 136 | + | ||||
| 119 | func TestSanitizeDomainForContainer(t *testing.T) { | 137 | func TestSanitizeDomainForContainer(t *testing.T) { | ||
| 120 | tests := []struct { | 138 | tests := []struct { | ||
| 121 | domain string | 139 | domain string | ||
| @@ -389,10 +407,10 @@ func TestGetEnvFilePath(t *testing.T) { | |||||
| 389 | wantEmpty bool | 407 | wantEmpty bool | ||
| 390 | wantFilename string | 408 | wantFilename string | ||
| 391 | }{ | 409 | }{ | ||
| 392 | - | {"example.com", false, "example_com.env"}, | 410 | + | {"example.com", false, "example__com.env"}, |
| 393 | - | {"sub.example.com", false, "sub_example_com.env"}, | 411 | + | {"sub.example.com", false, "sub__example__com.env"}, |
| 394 | - | {"example.com:8080", false, "example_com_8080.env"}, | 412 | + | {"example.com:8080", false, "example__com-_8080.env"}, |
| 395 | - | {"example.com/path", false, "example_com_path.env"}, | 413 | + | {"example.com/path", false, "example__com--path.env"}, |
| 396 | {"../evil", true, ""}, | 414 | {"../evil", true, ""}, | ||
| 397 | {"", true, ""}, | 415 | {"", true, ""}, | ||
| 398 | } | 416 | } | ||
M internal/adapters/out/envloader/file.go +6 -5
| @@ -12,6 +12,7 @@ import ( | |||
| 12 | 12 | "github.com/bnema/zerowrap" | |
| 13 | 13 | ||
| 14 | 14 | "github.com/bnema/gordon/internal/boundaries/out" | |
| 15 | + | "github.com/bnema/gordon/internal/domain" | |
| 15 | 16 | ) | |
| 16 | 17 | ||
| 17 | 18 | // FileLoader implements the EnvLoader interface using filesystem-based env files. | |
| @@ -222,10 +223,10 @@ func (l *FileLoader) resolveSecrets(ctx context.Context, value string) (string, | |||
| 222 | 223 | return result, nil | |
| 223 | 224 | } | |
| 224 | 225 | ||
| 225 | - | func (l *FileLoader) getEnvFilePath(domain string) string { | |
| 226 | - | // Create domain-safe filename (replace dots and other chars with underscores) | |
| 227 | - | safeDomain := strings.ReplaceAll(domain, ".", "_") | |
| 228 | - | safeDomain = strings.ReplaceAll(safeDomain, ":", "_") | |
| 229 | - | safeDomain = strings.ReplaceAll(safeDomain, "/", "_") | |
| 226 | + | func (l *FileLoader) getEnvFilePath(domainName string) string { | |
| 227 | + | safeDomain, err := domain.SanitizeDomainForEnvFile(domainName) | |
| 228 | + | if err != nil { | |
| 229 | + | return filepath.Join(l.envDir, domainName+".env") | |
| 230 | + | } | |
| 230 | 231 | return filepath.Join(l.envDir, safeDomain+".env") | |
| 231 | 232 | } | |
| @@ -12,6 +12,7 @@ import ( | |||||
| 12 | "github.com/bnema/zerowrap" | 12 | "github.com/bnema/zerowrap" | ||
| 13 | 13 | ||||
| 14 | "github.com/bnema/gordon/internal/boundaries/out" | 14 | "github.com/bnema/gordon/internal/boundaries/out" | ||
| 15 | + | "github.com/bnema/gordon/internal/domain" | |||
| 15 | ) | 16 | ) | ||
| 16 | 17 | ||||
| 17 | // FileLoader implements the EnvLoader interface using filesystem-based env files. | 18 | // FileLoader implements the EnvLoader interface using filesystem-based env files. | ||
| @@ -222,10 +223,10 @@ func (l *FileLoader) resolveSecrets(ctx context.Context, value string) (string, | |||||
| 222 | return result, nil | 223 | return result, nil | ||
| 223 | } | 224 | } | ||
| 224 | 225 | ||||
| 225 | - | func (l *FileLoader) getEnvFilePath(domain string) string { | 226 | + | func (l *FileLoader) getEnvFilePath(domainName string) string { |
| 226 | - | // Create domain-safe filename (replace dots and other chars with underscores) | 227 | + | safeDomain, err := domain.SanitizeDomainForEnvFile(domainName) |
| 227 | - | safeDomain := strings.ReplaceAll(domain, ".", "_") | 228 | + | if err != nil { |
| 228 | - | safeDomain = strings.ReplaceAll(safeDomain, ":", "_") | 229 | + | return filepath.Join(l.envDir, domainName+".env") |
| 229 | - | safeDomain = strings.ReplaceAll(safeDomain, "/", "_") | 230 | + | } |
| 230 | return filepath.Join(l.envDir, safeDomain+".env") | 231 | return filepath.Join(l.envDir, safeDomain+".env") | ||
| 231 | } | 232 | } | ||
M internal/adapters/out/envloader/file_test.go +23 -7
| @@ -121,7 +121,7 @@ func TestFileLoader_LoadEnv(t *testing.T) { | |||
| 121 | 121 | NODE_ENV=production | |
| 122 | 122 | API_URL=https://api.example.com | |
| 123 | 123 | ` | |
| 124 | - | envFile := filepath.Join(tmpDir, "app_example_com.env") | |
| 124 | + | envFile := filepath.Join(tmpDir, "app__example__com.env") | |
| 125 | 125 | require.NoError(t, os.WriteFile(envFile, []byte(envContent), 0600)) | |
| 126 | 126 | ||
| 127 | 127 | envVars, err := loader.LoadEnv(ctx, "app.example.com") | |
| @@ -146,7 +146,7 @@ API_URL=https://api.example.com | |||
| 146 | 146 | envContent := `NODE_ENV=production | |
| 147 | 147 | API_KEY=${pass:app/api-key} | |
| 148 | 148 | ` | |
| 149 | - | envFile := filepath.Join(tmpDir, "app_example_com.env") | |
| 149 | + | envFile := filepath.Join(tmpDir, "app__example__com.env") | |
| 150 | 150 | require.NoError(t, os.WriteFile(envFile, []byte(envContent), 0600)) | |
| 151 | 151 | ||
| 152 | 152 | envVars, err := loader.LoadEnv(ctx, "app.example.com") | |
| @@ -164,7 +164,7 @@ API_KEY=${pass:app/api-key} | |||
| 164 | 164 | envContent := `DOUBLE_QUOTED="hello world" | |
| 165 | 165 | SINGLE_QUOTED='hello world' | |
| 166 | 166 | ` | |
| 167 | - | envFile := filepath.Join(tmpDir, "app_example_com.env") | |
| 167 | + | envFile := filepath.Join(tmpDir, "app__example__com.env") | |
| 168 | 168 | require.NoError(t, os.WriteFile(envFile, []byte(envContent), 0600)) | |
| 169 | 169 | ||
| 170 | 170 | envVars, err := loader.LoadEnv(ctx, "app.example.com") | |
| @@ -183,7 +183,7 @@ SINGLE_QUOTED='hello world' | |||
| 183 | 183 | invalid line without equals | |
| 184 | 184 | ALSO_VALID=another | |
| 185 | 185 | ` | |
| 186 | - | envFile := filepath.Join(tmpDir, "app_example_com.env") | |
| 186 | + | envFile := filepath.Join(tmpDir, "app__example__com.env") | |
| 187 | 187 | require.NoError(t, os.WriteFile(envFile, []byte(envContent), 0600)) | |
| 188 | 188 | ||
| 189 | 189 | envVars, err := loader.LoadEnv(ctx, "app.example.com") | |
| @@ -205,9 +205,9 @@ func TestFileLoader_GetEnvFilePath(t *testing.T) { | |||
| 205 | 205 | domain string | |
| 206 | 206 | expected string | |
| 207 | 207 | }{ | |
| 208 | - | {"app.example.com", "app_example_com.env"}, | |
| 209 | - | {"api.sub.example.com", "api_sub_example_com.env"}, | |
| 210 | - | {"localhost:8080", "localhost_8080.env"}, | |
| 208 | + | {"app.example.com", "app__example__com.env"}, | |
| 209 | + | {"api.sub.example.com", "api__sub__example__com.env"}, | |
| 210 | + | {"localhost:8080", "localhost-_8080.env"}, | |
| 211 | 211 | } | |
| 212 | 212 | ||
| 213 | 213 | for _, tt := range tests { | |
| @@ -217,3 +217,19 @@ func TestFileLoader_GetEnvFilePath(t *testing.T) { | |||
| 217 | 217 | }) | |
| 218 | 218 | } | |
| 219 | 219 | } | |
| 220 | + | ||
| 221 | + | func TestFileLoader_GetEnvFilePath_DistinguishesSeparators(t *testing.T) { | |
| 222 | + | tmpDir := t.TempDir() | |
| 223 | + | log := zerowrap.New(zerowrap.Config{Level: "fatal"}) | |
| 224 | + | ||
| 225 | + | loader, err := NewFileLoader(tmpDir, log) | |
| 226 | + | require.NoError(t, err) | |
| 227 | + | ||
| 228 | + | dotPath := loader.getEnvFilePath("app.example.com") | |
| 229 | + | portPath := loader.getEnvFilePath("app:example:com") | |
| 230 | + | slashPath := loader.getEnvFilePath("app/example/com") | |
| 231 | + | ||
| 232 | + | assert.NotEqual(t, dotPath, portPath) | |
| 233 | + | assert.NotEqual(t, dotPath, slashPath) | |
| 234 | + | assert.NotEqual(t, portPath, slashPath) | |
| 235 | + | } | |
| @@ -121,7 +121,7 @@ func TestFileLoader_LoadEnv(t *testing.T) { | |||||
| 121 | NODE_ENV=production | 121 | NODE_ENV=production | ||
| 122 | API_URL=https://api.example.com | 122 | API_URL=https://api.example.com | ||
| 123 | ` | 123 | ` | ||
| 124 | - | envFile := filepath.Join(tmpDir, "app_example_com.env") | 124 | + | envFile := filepath.Join(tmpDir, "app__example__com.env") |
| 125 | require.NoError(t, os.WriteFile(envFile, []byte(envContent), 0600)) | 125 | require.NoError(t, os.WriteFile(envFile, []byte(envContent), 0600)) | ||
| 126 | 126 | ||||
| 127 | envVars, err := loader.LoadEnv(ctx, "app.example.com") | 127 | envVars, err := loader.LoadEnv(ctx, "app.example.com") | ||
| @@ -146,7 +146,7 @@ API_URL=https://api.example.com | |||||
| 146 | envContent := `NODE_ENV=production | 146 | envContent := `NODE_ENV=production | ||
| 147 | API_KEY=${pass:app/api-key} | 147 | API_KEY=${pass:app/api-key} | ||
| 148 | ` | 148 | ` | ||
| 149 | - | envFile := filepath.Join(tmpDir, "app_example_com.env") | 149 | + | envFile := filepath.Join(tmpDir, "app__example__com.env") |
| 150 | require.NoError(t, os.WriteFile(envFile, []byte(envContent), 0600)) | 150 | require.NoError(t, os.WriteFile(envFile, []byte(envContent), 0600)) | ||
| 151 | 151 | ||||
| 152 | envVars, err := loader.LoadEnv(ctx, "app.example.com") | 152 | envVars, err := loader.LoadEnv(ctx, "app.example.com") | ||
| @@ -164,7 +164,7 @@ API_KEY=${pass:app/api-key} | |||||
| 164 | envContent := `DOUBLE_QUOTED="hello world" | 164 | envContent := `DOUBLE_QUOTED="hello world" | ||
| 165 | SINGLE_QUOTED='hello world' | 165 | SINGLE_QUOTED='hello world' | ||
| 166 | ` | 166 | ` | ||
| 167 | - | envFile := filepath.Join(tmpDir, "app_example_com.env") | 167 | + | envFile := filepath.Join(tmpDir, "app__example__com.env") |
| 168 | require.NoError(t, os.WriteFile(envFile, []byte(envContent), 0600)) | 168 | require.NoError(t, os.WriteFile(envFile, []byte(envContent), 0600)) | ||
| 169 | 169 | ||||
| 170 | envVars, err := loader.LoadEnv(ctx, "app.example.com") | 170 | envVars, err := loader.LoadEnv(ctx, "app.example.com") | ||
| @@ -183,7 +183,7 @@ SINGLE_QUOTED='hello world' | |||||
| 183 | invalid line without equals | 183 | invalid line without equals | ||
| 184 | ALSO_VALID=another | 184 | ALSO_VALID=another | ||
| 185 | ` | 185 | ` | ||
| 186 | - | envFile := filepath.Join(tmpDir, "app_example_com.env") | 186 | + | envFile := filepath.Join(tmpDir, "app__example__com.env") |
| 187 | require.NoError(t, os.WriteFile(envFile, []byte(envContent), 0600)) | 187 | require.NoError(t, os.WriteFile(envFile, []byte(envContent), 0600)) | ||
| 188 | 188 | ||||
| 189 | envVars, err := loader.LoadEnv(ctx, "app.example.com") | 189 | envVars, err := loader.LoadEnv(ctx, "app.example.com") | ||
| @@ -205,9 +205,9 @@ func TestFileLoader_GetEnvFilePath(t *testing.T) { | |||||
| 205 | domain string | 205 | domain string | ||
| 206 | expected string | 206 | expected string | ||
| 207 | }{ | 207 | }{ | ||
| 208 | - | {"app.example.com", "app_example_com.env"}, | 208 | + | {"app.example.com", "app__example__com.env"}, |
| 209 | - | {"api.sub.example.com", "api_sub_example_com.env"}, | 209 | + | {"api.sub.example.com", "api__sub__example__com.env"}, |
| 210 | - | {"localhost:8080", "localhost_8080.env"}, | 210 | + | {"localhost:8080", "localhost-_8080.env"}, |
| 211 | } | 211 | } | ||
| 212 | 212 | ||||
| 213 | for _, tt := range tests { | 213 | for _, tt := range tests { | ||
| @@ -217,3 +217,19 @@ func TestFileLoader_GetEnvFilePath(t *testing.T) { | |||||
| 217 | }) | 217 | }) | ||
| 218 | } | 218 | } | ||
| 219 | } | 219 | } | ||
| 220 | + | ||||
| 221 | + | func TestFileLoader_GetEnvFilePath_DistinguishesSeparators(t *testing.T) { | |||
| 222 | + | tmpDir := t.TempDir() | |||
| 223 | + | log := zerowrap.New(zerowrap.Config{Level: "fatal"}) | |||
| 224 | + | ||||
| 225 | + | loader, err := NewFileLoader(tmpDir, log) | |||
| 226 | + | require.NoError(t, err) | |||
| 227 | + | ||||
| 228 | + | dotPath := loader.getEnvFilePath("app.example.com") | |||
| 229 | + | portPath := loader.getEnvFilePath("app:example:com") | |||
| 230 | + | slashPath := loader.getEnvFilePath("app/example/com") | |||
| 231 | + | ||||
| 232 | + | assert.NotEqual(t, dotPath, portPath) | |||
| 233 | + | assert.NotEqual(t, dotPath, slashPath) | |||
| 234 | + | assert.NotEqual(t, portPath, slashPath) | |||
| 235 | + | } | |||
M internal/app/run.go +16 -8
| @@ -585,7 +585,10 @@ func setupInternalRegistryAuth(svc *services, log zerowrap.Logger) error { | |||
| 585 | 585 | ||
| 586 | 586 | func createDomainSecretStore(cfg Config, log zerowrap.Logger) (string, domain.SecretsBackend, *domainsecrets.PassStore, out.DomainSecretStore, error) { | |
| 587 | 587 | envDir := resolveEnvDir(cfg) | |
| 588 | - | backend := resolveSecretsBackend(cfg.Auth.SecretsBackend) | |
| 588 | + | backend, err := resolveSecretsBackend(cfg.Auth.SecretsBackend) | |
| 589 | + | if err != nil { | |
| 590 | + | return "", "", nil, nil, log.WrapErr(err, "failed to resolve secrets backend") | |
| 591 | + | } | |
| 589 | 592 | ||
| 590 | 593 | switch backend { | |
| 591 | 594 | case domain.SecretsBackendPass: | |
| @@ -896,7 +899,10 @@ func createAuthService(ctx context.Context, cfg Config, log zerowrap.Logger) (ou | |||
| 896 | 899 | } | |
| 897 | 900 | ||
| 898 | 901 | authType := resolveAuthType(cfg) | |
| 899 | - | backend := resolveSecretsBackend(cfg.Auth.SecretsBackend) | |
| 902 | + | backend, err := resolveSecretsBackend(cfg.Auth.SecretsBackend) | |
| 903 | + | if err != nil { | |
| 904 | + | return nil, nil, log.WrapErr(err, "failed to resolve secrets backend") | |
| 905 | + | } | |
| 900 | 906 | dataDir := resolveDataDir(cfg.Server.DataDir) | |
| 901 | 907 | ||
| 902 | 908 | store, err := createTokenStore(backend, dataDir, log) | |
| @@ -941,16 +947,18 @@ func resolveAuthType(cfg Config) domain.AuthType { | |||
| 941 | 947 | return domain.AuthTypeToken | |
| 942 | 948 | } | |
| 943 | 949 | ||
| 944 | - | func resolveSecretsBackend(backend string) domain.SecretsBackend { | |
| 950 | + | func resolveSecretsBackend(backend string) (domain.SecretsBackend, error) { | |
| 945 | 951 | switch backend { | |
| 946 | 952 | case "pass": | |
| 947 | - | return domain.SecretsBackendPass | |
| 953 | + | return domain.SecretsBackendPass, nil | |
| 948 | 954 | case "sops": | |
| 949 | - | return domain.SecretsBackendSops | |
| 950 | - | case "unsafe", "": | |
| 951 | - | return domain.SecretsBackendUnsafe | |
| 955 | + | return domain.SecretsBackendSops, nil | |
| 956 | + | case "unsafe": | |
| 957 | + | return domain.SecretsBackendUnsafe, nil | |
| 958 | + | case "": | |
| 959 | + | return "", fmt.Errorf("auth.secrets_backend is required") | |
| 952 | 960 | default: | |
| 953 | - | return domain.SecretsBackendUnsafe | |
| 961 | + | return "", fmt.Errorf("unsupported auth.secrets_backend %q", backend) | |
| 954 | 962 | } | |
| 955 | 963 | } | |
| 956 | 964 | ||
| @@ -585,7 +585,10 @@ func setupInternalRegistryAuth(svc *services, log zerowrap.Logger) error { | |||||
| 585 | 585 | ||||
| 586 | func createDomainSecretStore(cfg Config, log zerowrap.Logger) (string, domain.SecretsBackend, *domainsecrets.PassStore, out.DomainSecretStore, error) { | 586 | func createDomainSecretStore(cfg Config, log zerowrap.Logger) (string, domain.SecretsBackend, *domainsecrets.PassStore, out.DomainSecretStore, error) { | ||
| 587 | envDir := resolveEnvDir(cfg) | 587 | envDir := resolveEnvDir(cfg) | ||
| 588 | - | backend := resolveSecretsBackend(cfg.Auth.SecretsBackend) | 588 | + | backend, err := resolveSecretsBackend(cfg.Auth.SecretsBackend) |
| 589 | + | if err != nil { | |||
| 590 | + | return "", "", nil, nil, log.WrapErr(err, "failed to resolve secrets backend") | |||
| 591 | + | } | |||
| 589 | 592 | ||||
| 590 | switch backend { | 593 | switch backend { | ||
| 591 | case domain.SecretsBackendPass: | 594 | case domain.SecretsBackendPass: | ||
| @@ -896,7 +899,10 @@ func createAuthService(ctx context.Context, cfg Config, log zerowrap.Logger) (ou | |||||
| 896 | } | 899 | } | ||
| 897 | 900 | ||||
| 898 | authType := resolveAuthType(cfg) | 901 | authType := resolveAuthType(cfg) | ||
| 899 | - | backend := resolveSecretsBackend(cfg.Auth.SecretsBackend) | 902 | + | backend, err := resolveSecretsBackend(cfg.Auth.SecretsBackend) |
| 903 | + | if err != nil { | |||
| 904 | + | return nil, nil, log.WrapErr(err, "failed to resolve secrets backend") | |||
| 905 | + | } | |||
| 900 | dataDir := resolveDataDir(cfg.Server.DataDir) | 906 | dataDir := resolveDataDir(cfg.Server.DataDir) | ||
| 901 | 907 | ||||
| 902 | store, err := createTokenStore(backend, dataDir, log) | 908 | store, err := createTokenStore(backend, dataDir, log) | ||
| @@ -941,16 +947,18 @@ func resolveAuthType(cfg Config) domain.AuthType { | |||||
| 941 | return domain.AuthTypeToken | 947 | return domain.AuthTypeToken | ||
| 942 | } | 948 | } | ||
| 943 | 949 | ||||
| 944 | - | func resolveSecretsBackend(backend string) domain.SecretsBackend { | 950 | + | func resolveSecretsBackend(backend string) (domain.SecretsBackend, error) { |
| 945 | switch backend { | 951 | switch backend { | ||
| 946 | case "pass": | 952 | case "pass": | ||
| 947 | - | return domain.SecretsBackendPass | 953 | + | return domain.SecretsBackendPass, nil |
| 948 | case "sops": | 954 | case "sops": | ||
| 949 | - | return domain.SecretsBackendSops | 955 | + | return domain.SecretsBackendSops, nil |
| 950 | - | case "unsafe", "": | 956 | + | case "unsafe": |
| 951 | - | return domain.SecretsBackendUnsafe | 957 | + | return domain.SecretsBackendUnsafe, nil |
| 958 | + | case "": | |||
| 959 | + | return "", fmt.Errorf("auth.secrets_backend is required") | |||
| 952 | default: | 960 | default: | ||
| 953 | - | return domain.SecretsBackendUnsafe | 961 | + | return "", fmt.Errorf("unsupported auth.secrets_backend %q", backend) |
| 954 | } | 962 | } | ||
| 955 | } | 963 | } | ||
| 956 | 964 | ||||
M internal/app/run_auth_config_test.go +21 -0
| @@ -2,6 +2,7 @@ package app | |||
| 2 | 2 | ||
| 3 | 3 | import ( | |
| 4 | 4 | "context" | |
| 5 | + | "strings" | |
| 5 | 6 | "testing" | |
| 6 | 7 | ||
| 7 | 8 | "github.com/bnema/gordon/internal/domain" | |
| @@ -23,3 +24,23 @@ func TestBuildAuthConfig_UsesConfigEnabledFlag(t *testing.T) { | |||
| 23 | 24 | t.Fatalf("expected auth config enabled=false, got true") | |
| 24 | 25 | } | |
| 25 | 26 | } | |
| 27 | + | ||
| 28 | + | func TestResolveSecretsBackend_RejectsMissingBackend(t *testing.T) { | |
| 29 | + | _, err := resolveSecretsBackend("") | |
| 30 | + | if err == nil { | |
| 31 | + | t.Fatal("expected error for missing secrets backend") | |
| 32 | + | } | |
| 33 | + | if !strings.Contains(err.Error(), "auth.secrets_backend is required") { | |
| 34 | + | t.Fatalf("unexpected error: %v", err) | |
| 35 | + | } | |
| 36 | + | } | |
| 37 | + | ||
| 38 | + | func TestResolveSecretsBackend_RejectsUnknownBackend(t *testing.T) { | |
| 39 | + | _, err := resolveSecretsBackend("vault") | |
| 40 | + | if err == nil { | |
| 41 | + | t.Fatal("expected error for unsupported secrets backend") | |
| 42 | + | } | |
| 43 | + | if !strings.Contains(err.Error(), `unsupported auth.secrets_backend "vault"`) { | |
| 44 | + | t.Fatalf("unexpected error: %v", err) | |
| 45 | + | } | |
| 46 | + | } | |
| @@ -2,6 +2,7 @@ package app | |||||
| 2 | 2 | ||||
| 3 | import ( | 3 | import ( | ||
| 4 | "context" | 4 | "context" | ||
| 5 | + | "strings" | |||
| 5 | "testing" | 6 | "testing" | ||
| 6 | 7 | ||||
| 7 | "github.com/bnema/gordon/internal/domain" | 8 | "github.com/bnema/gordon/internal/domain" | ||
| @@ -23,3 +24,23 @@ func TestBuildAuthConfig_UsesConfigEnabledFlag(t *testing.T) { | |||||
| 23 | t.Fatalf("expected auth config enabled=false, got true") | 24 | t.Fatalf("expected auth config enabled=false, got true") | ||
| 24 | } | 25 | } | ||
| 25 | } | 26 | } | ||
| 27 | + | ||||
| 28 | + | func TestResolveSecretsBackend_RejectsMissingBackend(t *testing.T) { | |||
| 29 | + | _, err := resolveSecretsBackend("") | |||
| 30 | + | if err == nil { | |||
| 31 | + | t.Fatal("expected error for missing secrets backend") | |||
| 32 | + | } | |||
| 33 | + | if !strings.Contains(err.Error(), "auth.secrets_backend is required") { | |||
| 34 | + | t.Fatalf("unexpected error: %v", err) | |||
| 35 | + | } | |||
| 36 | + | } | |||
| 37 | + | ||||
| 38 | + | func TestResolveSecretsBackend_RejectsUnknownBackend(t *testing.T) { | |||
| 39 | + | _, err := resolveSecretsBackend("vault") | |||
| 40 | + | if err == nil { | |||
| 41 | + | t.Fatal("expected error for unsupported secrets backend") | |||
| 42 | + | } | |||
| 43 | + | if !strings.Contains(err.Error(), `unsupported auth.secrets_backend "vault"`) { | |||
| 44 | + | t.Fatalf("unexpected error: %v", err) | |||
| 45 | + | } | |||
| 46 | + | } | |||
M internal/domain/env.go +7 -5
| @@ -9,13 +9,15 @@ import ( | |||
| 9 | 9 | ||
| 10 | 10 | // envDomainRegex validates domain names used for env secrets. | |
| 11 | 11 | // Allows: alphanumeric, dots, hyphens, colons (for ports), and forward slashes (for paths). | |
| 12 | - | var envDomainRegex = regexp.MustCompile(`^[a-zA-Z0-9][a-zA-Z0-9._:/-]*[a-zA-Z0-9]$|^[a-zA-Z0-9]$`) | |
| 12 | + | // Underscores are intentionally disallowed because they would collide with the | |
| 13 | + | // on-disk filename encoding used for dots/ports/path separators. | |
| 14 | + | var envDomainRegex = regexp.MustCompile(`^[a-zA-Z0-9][a-zA-Z0-9.:/-]*[a-zA-Z0-9]$|^[a-zA-Z0-9]$`) | |
| 13 | 15 | var envKeyRegex = regexp.MustCompile(`^[A-Za-z_][A-Za-z0-9_]*$`) | |
| 14 | 16 | var containerNameRegex = regexp.MustCompile(`^[A-Za-z][A-Za-z0-9_-]*$`) | |
| 15 | 17 | ||
| 16 | 18 | // SanitizeDomainForEnvFile validates and sanitizes a domain name for env file storage. | |
| 17 | 19 | // Returns a safe domain string suitable for filenames. | |
| 18 | - | // This function is idempotent: calling it on already-sanitized input returns the same value. | |
| 20 | + | // Uses distinct replacements to avoid collisions between ".", ":", and "/". | |
| 19 | 21 | func SanitizeDomainForEnvFile(domainName string) (string, error) { | |
| 20 | 22 | if domainName == "" { | |
| 21 | 23 | return "", ErrPathTraversal | |
| @@ -29,9 +31,9 @@ func SanitizeDomainForEnvFile(domainName string) (string, error) { | |||
| 29 | 31 | return "", ErrPathTraversal | |
| 30 | 32 | } | |
| 31 | 33 | ||
| 32 | - | safeDomain := strings.ReplaceAll(domainName, ".", "_") | |
| 33 | - | safeDomain = strings.ReplaceAll(safeDomain, ":", "_") | |
| 34 | - | safeDomain = strings.ReplaceAll(safeDomain, "/", "_") | |
| 34 | + | safeDomain := strings.ReplaceAll(domainName, ".", "__") | |
| 35 | + | safeDomain = strings.ReplaceAll(safeDomain, ":", "-_") | |
| 36 | + | safeDomain = strings.ReplaceAll(safeDomain, "/", "--") | |
| 35 | 37 | return safeDomain, nil | |
| 36 | 38 | } | |
| 37 | 39 | ||
| @@ -9,13 +9,15 @@ import ( | |||||
| 9 | 9 | ||||
| 10 | // envDomainRegex validates domain names used for env secrets. | 10 | // envDomainRegex validates domain names used for env secrets. | ||
| 11 | // Allows: alphanumeric, dots, hyphens, colons (for ports), and forward slashes (for paths). | 11 | // Allows: alphanumeric, dots, hyphens, colons (for ports), and forward slashes (for paths). | ||
| 12 | - | var envDomainRegex = regexp.MustCompile(`^[a-zA-Z0-9][a-zA-Z0-9._:/-]*[a-zA-Z0-9]$|^[a-zA-Z0-9]$`) | 12 | + | // Underscores are intentionally disallowed because they would collide with the |
| 13 | + | // on-disk filename encoding used for dots/ports/path separators. | |||
| 14 | + | var envDomainRegex = regexp.MustCompile(`^[a-zA-Z0-9][a-zA-Z0-9.:/-]*[a-zA-Z0-9]$|^[a-zA-Z0-9]$`) | |||
| 13 | var envKeyRegex = regexp.MustCompile(`^[A-Za-z_][A-Za-z0-9_]*$`) | 15 | var envKeyRegex = regexp.MustCompile(`^[A-Za-z_][A-Za-z0-9_]*$`) | ||
| 14 | var containerNameRegex = regexp.MustCompile(`^[A-Za-z][A-Za-z0-9_-]*$`) | 16 | var containerNameRegex = regexp.MustCompile(`^[A-Za-z][A-Za-z0-9_-]*$`) | ||
| 15 | 17 | ||||
| 16 | // SanitizeDomainForEnvFile validates and sanitizes a domain name for env file storage. | 18 | // SanitizeDomainForEnvFile validates and sanitizes a domain name for env file storage. | ||
| 17 | // Returns a safe domain string suitable for filenames. | 19 | // Returns a safe domain string suitable for filenames. | ||
| 18 | - | // This function is idempotent: calling it on already-sanitized input returns the same value. | 20 | + | // Uses distinct replacements to avoid collisions between ".", ":", and "/". |
| 19 | func SanitizeDomainForEnvFile(domainName string) (string, error) { | 21 | func SanitizeDomainForEnvFile(domainName string) (string, error) { | ||
| 20 | if domainName == "" { | 22 | if domainName == "" { | ||
| 21 | return "", ErrPathTraversal | 23 | return "", ErrPathTraversal | ||
| @@ -29,9 +31,9 @@ func SanitizeDomainForEnvFile(domainName string) (string, error) { | |||||
| 29 | return "", ErrPathTraversal | 31 | return "", ErrPathTraversal | ||
| 30 | } | 32 | } | ||
| 31 | 33 | ||||
| 32 | - | safeDomain := strings.ReplaceAll(domainName, ".", "_") | 34 | + | safeDomain := strings.ReplaceAll(domainName, ".", "__") |
| 33 | - | safeDomain = strings.ReplaceAll(safeDomain, ":", "_") | 35 | + | safeDomain = strings.ReplaceAll(safeDomain, ":", "-_") |
| 34 | - | safeDomain = strings.ReplaceAll(safeDomain, "/", "_") | 36 | + | safeDomain = strings.ReplaceAll(safeDomain, "/", "--") |
| 35 | return safeDomain, nil | 37 | return safeDomain, nil | ||
| 36 | } | 38 | } | ||
| 37 | 39 | ||||
M internal/domain/env_test.go +21 -7
| @@ -15,12 +15,12 @@ func TestSanitizeDomainForEnvFile(t *testing.T) { | |||
| 15 | 15 | want string | |
| 16 | 16 | wantErr bool | |
| 17 | 17 | }{ | |
| 18 | - | {name: "simple domain", domain: "example.com", want: "example_com"}, | |
| 19 | - | {name: "subdomain", domain: "app.example.com", want: "app_example_com"}, | |
| 20 | - | {name: "domain with port", domain: "example.com:8080", want: "example_com_8080"}, | |
| 21 | - | {name: "domain with path", domain: "example.com/path", want: "example_com_path"}, | |
| 18 | + | {name: "simple domain", domain: "example.com", want: "example__com"}, | |
| 19 | + | {name: "subdomain", domain: "app.example.com", want: "app__example__com"}, | |
| 20 | + | {name: "domain with port", domain: "example.com:8080", want: "example__com-_8080"}, | |
| 21 | + | {name: "domain with path", domain: "example.com/path", want: "example__com--path"}, | |
| 22 | 22 | {name: "single char", domain: "a", want: "a"}, | |
| 23 | - | {name: "hyphenated", domain: "my-app.example.com", want: "my-app_example_com"}, | |
| 23 | + | {name: "hyphenated", domain: "my-app.example.com", want: "my-app__example__com"}, | |
| 24 | 24 | {name: "empty", domain: "", wantErr: true}, | |
| 25 | 25 | {name: "path traversal", domain: "../etc/passwd", wantErr: true}, | |
| 26 | 26 | {name: "double dots", domain: "foo..bar", wantErr: true}, | |
| @@ -28,8 +28,7 @@ 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 | + | {name: "underscore rejected to avoid filename collision", domain: "example_com", wantErr: true}, | |
| 33 | 32 | } | |
| 34 | 33 | ||
| 35 | 34 | for _, tt := range tests { | |
| @@ -45,6 +44,21 @@ func TestSanitizeDomainForEnvFile(t *testing.T) { | |||
| 45 | 44 | } | |
| 46 | 45 | } | |
| 47 | 46 | ||
| 47 | + | func TestSanitizeDomainForEnvFile_DistinguishesSeparators(t *testing.T) { | |
| 48 | + | gotDot, err := SanitizeDomainForEnvFile("app.example.com") | |
| 49 | + | require.NoError(t, err) | |
| 50 | + | ||
| 51 | + | gotPort, err := SanitizeDomainForEnvFile("app:example:com") | |
| 52 | + | require.NoError(t, err) | |
| 53 | + | ||
| 54 | + | gotPath, err := SanitizeDomainForEnvFile("app/example/com") | |
| 55 | + | require.NoError(t, err) | |
| 56 | + | ||
| 57 | + | assert.NotEqual(t, gotDot, gotPort) | |
| 58 | + | assert.NotEqual(t, gotDot, gotPath) | |
| 59 | + | assert.NotEqual(t, gotPort, gotPath) | |
| 60 | + | } | |
| 61 | + | ||
| 48 | 62 | func TestParseEnvData(t *testing.T) { | |
| 49 | 63 | tests := []struct { | |
| 50 | 64 | name string | |
| @@ -15,12 +15,12 @@ func TestSanitizeDomainForEnvFile(t *testing.T) { | |||||
| 15 | want string | 15 | want string | ||
| 16 | wantErr bool | 16 | wantErr bool | ||
| 17 | }{ | 17 | }{ | ||
| 18 | - | {name: "simple domain", domain: "example.com", want: "example_com"}, | 18 | + | {name: "simple domain", domain: "example.com", want: "example__com"}, |
| 19 | - | {name: "subdomain", domain: "app.example.com", want: "app_example_com"}, | 19 | + | {name: "subdomain", domain: "app.example.com", want: "app__example__com"}, |
| 20 | - | {name: "domain with port", domain: "example.com:8080", want: "example_com_8080"}, | 20 | + | {name: "domain with port", domain: "example.com:8080", want: "example__com-_8080"}, |
| 21 | - | {name: "domain with path", domain: "example.com/path", want: "example_com_path"}, | 21 | + | {name: "domain with path", domain: "example.com/path", want: "example__com--path"}, |
| 22 | {name: "single char", domain: "a", want: "a"}, | 22 | {name: "single char", domain: "a", want: "a"}, | ||
| 23 | - | {name: "hyphenated", domain: "my-app.example.com", want: "my-app_example_com"}, | 23 | + | {name: "hyphenated", domain: "my-app.example.com", want: "my-app__example__com"}, |
| 24 | {name: "empty", domain: "", wantErr: true}, | 24 | {name: "empty", domain: "", wantErr: true}, | ||
| 25 | {name: "path traversal", domain: "../etc/passwd", wantErr: true}, | 25 | {name: "path traversal", domain: "../etc/passwd", wantErr: true}, | ||
| 26 | {name: "double dots", domain: "foo..bar", wantErr: true}, | 26 | {name: "double dots", domain: "foo..bar", wantErr: true}, | ||
| @@ -28,8 +28,7 @@ func TestSanitizeDomainForEnvFile(t *testing.T) { | |||||
| 28 | {name: "ends with dot", domain: "trailing.", wantErr: true}, | 28 | {name: "ends with dot", domain: "trailing.", wantErr: true}, | ||
| 29 | {name: "space", domain: "has space", wantErr: true}, | 29 | {name: "space", domain: "has space", wantErr: true}, | ||
| 30 | {name: "special chars", domain: "bad$domain", wantErr: true}, | 30 | {name: "special chars", domain: "bad$domain", wantErr: true}, | ||
| 31 | - | {name: "idempotent simple", domain: "example_com", want: "example_com"}, | 31 | + | {name: "underscore rejected to avoid filename collision", domain: "example_com", wantErr: true}, |
| 32 | - | {name: "idempotent complex", domain: "my-app_example_com_8080", want: "my-app_example_com_8080"}, | |||
| 33 | } | 32 | } | ||
| 34 | 33 | ||||
| 35 | for _, tt := range tests { | 34 | for _, tt := range tests { | ||
| @@ -45,6 +44,21 @@ func TestSanitizeDomainForEnvFile(t *testing.T) { | |||||
| 45 | } | 44 | } | ||
| 46 | } | 45 | } | ||
| 47 | 46 | ||||
| 47 | + | func TestSanitizeDomainForEnvFile_DistinguishesSeparators(t *testing.T) { | |||
| 48 | + | gotDot, err := SanitizeDomainForEnvFile("app.example.com") | |||
| 49 | + | require.NoError(t, err) | |||
| 50 | + | ||||
| 51 | + | gotPort, err := SanitizeDomainForEnvFile("app:example:com") | |||
| 52 | + | require.NoError(t, err) | |||
| 53 | + | ||||
| 54 | + | gotPath, err := SanitizeDomainForEnvFile("app/example/com") | |||
| 55 | + | require.NoError(t, err) | |||
| 56 | + | ||||
| 57 | + | assert.NotEqual(t, gotDot, gotPort) | |||
| 58 | + | assert.NotEqual(t, gotDot, gotPath) | |||
| 59 | + | assert.NotEqual(t, gotPort, gotPath) | |||
| 60 | + | } | |||
| 61 | + | ||||
| 48 | func TestParseEnvData(t *testing.T) { | 62 | func TestParseEnvData(t *testing.T) { | ||
| 49 | tests := []struct { | 63 | tests := []struct { | ||
| 50 | name string | 64 | name string | ||
M internal/usecase/container/autoroute.go +4 -4
| @@ -468,10 +468,10 @@ func parseEnvFile(data []byte) (map[string]string, error) { | |||
| 468 | 468 | // e.g., "app.mydomain.com" -> "app_mydomain_com.env" | |
| 469 | 469 | // Must match the naming convention in envloader.FileLoader.getEnvFilePath | |
| 470 | 470 | func domainToEnvFileName(domainName string) string { | |
| 471 | - | // Replace special characters with underscores (matches envloader) | |
| 472 | - | name := strings.ReplaceAll(domainName, ".", "_") | |
| 473 | - | name = strings.ReplaceAll(name, ":", "_") | |
| 474 | - | name = strings.ReplaceAll(name, "/", "_") | |
| 471 | + | name, err := domain.SanitizeDomainForEnvFile(domainName) | |
| 472 | + | if err != nil { | |
| 473 | + | return domainName + ".env" | |
| 474 | + | } | |
| 475 | 475 | return name + ".env" | |
| 476 | 476 | } | |
| 477 | 477 | ||
| @@ -468,10 +468,10 @@ func parseEnvFile(data []byte) (map[string]string, error) { | |||||
| 468 | // e.g., "app.mydomain.com" -> "app_mydomain_com.env" | 468 | // e.g., "app.mydomain.com" -> "app_mydomain_com.env" | ||
| 469 | // Must match the naming convention in envloader.FileLoader.getEnvFilePath | 469 | // Must match the naming convention in envloader.FileLoader.getEnvFilePath | ||
| 470 | func domainToEnvFileName(domainName string) string { | 470 | func domainToEnvFileName(domainName string) string { | ||
| 471 | - | // Replace special characters with underscores (matches envloader) | 471 | + | name, err := domain.SanitizeDomainForEnvFile(domainName) |
| 472 | - | name := strings.ReplaceAll(domainName, ".", "_") | 472 | + | if err != nil { |
| 473 | - | name = strings.ReplaceAll(name, ":", "_") | 473 | + | return domainName + ".env" |
| 474 | - | name = strings.ReplaceAll(name, "/", "_") | 474 | + | } |
| 475 | return name + ".env" | 475 | return name + ".env" | ||
| 476 | } | 476 | } | ||
| 477 | 477 | ||||
M internal/usecase/container/autoroute_test.go +4 -4
| @@ -28,22 +28,22 @@ func TestDomainToEnvFileName(t *testing.T) { | |||
| 28 | 28 | { | |
| 29 | 29 | name: "simple domain", | |
| 30 | 30 | domain: "app.example.com", | |
| 31 | - | expected: "app_example_com.env", | |
| 31 | + | expected: "app__example__com.env", | |
| 32 | 32 | }, | |
| 33 | 33 | { | |
| 34 | 34 | name: "domain with port", | |
| 35 | 35 | domain: "app.example.com:8080", | |
| 36 | - | expected: "app_example_com_8080.env", | |
| 36 | + | expected: "app__example__com-_8080.env", | |
| 37 | 37 | }, | |
| 38 | 38 | { | |
| 39 | 39 | name: "domain with path", | |
| 40 | 40 | domain: "app.example.com/api", | |
| 41 | - | expected: "app_example_com_api.env", | |
| 41 | + | expected: "app__example__com--api.env", | |
| 42 | 42 | }, | |
| 43 | 43 | { | |
| 44 | 44 | name: "complex domain", | |
| 45 | 45 | domain: "sub.app.example.com:443/v1", | |
| 46 | - | expected: "sub_app_example_com_443_v1.env", | |
| 46 | + | expected: "sub__app__example__com-_443--v1.env", | |
| 47 | 47 | }, | |
| 48 | 48 | { | |
| 49 | 49 | name: "single word", | |
| @@ -28,22 +28,22 @@ func TestDomainToEnvFileName(t *testing.T) { | |||||
| 28 | { | 28 | { | ||
| 29 | name: "simple domain", | 29 | name: "simple domain", | ||
| 30 | domain: "app.example.com", | 30 | domain: "app.example.com", | ||
| 31 | - | expected: "app_example_com.env", | 31 | + | expected: "app__example__com.env", |
| 32 | }, | 32 | }, | ||
| 33 | { | 33 | { | ||
| 34 | name: "domain with port", | 34 | name: "domain with port", | ||
| 35 | domain: "app.example.com:8080", | 35 | domain: "app.example.com:8080", | ||
| 36 | - | expected: "app_example_com_8080.env", | 36 | + | expected: "app__example__com-_8080.env", |
| 37 | }, | 37 | }, | ||
| 38 | { | 38 | { | ||
| 39 | name: "domain with path", | 39 | name: "domain with path", | ||
| 40 | domain: "app.example.com/api", | 40 | domain: "app.example.com/api", | ||
| 41 | - | expected: "app_example_com_api.env", | 41 | + | expected: "app__example__com--api.env", |
| 42 | }, | 42 | }, | ||
| 43 | { | 43 | { | ||
| 44 | name: "complex domain", | 44 | name: "complex domain", | ||
| 45 | domain: "sub.app.example.com:443/v1", | 45 | domain: "sub.app.example.com:443/v1", | ||
| 46 | - | expected: "sub_app_example_com_443_v1.env", | 46 | + | expected: "sub__app__example__com-_443--v1.env", |
| 47 | }, | 47 | }, | ||
| 48 | { | 48 | { | ||
| 49 | name: "single word", | 49 | name: "single word", | ||
M internal/usecase/secrets/service.go +11 -5
| @@ -278,24 +278,30 @@ func resolveContainerName(domainName, service string) string { | |||
| 278 | 278 | ||
| 279 | 279 | // ValidateDomain validates that a domain is safe to use for secret storage. | |
| 280 | 280 | // This prevents path traversal attacks and ensures the domain is valid. | |
| 281 | - | func ValidateDomain(domain string) error { | |
| 281 | + | func ValidateDomain(domainName string) error { | |
| 282 | 282 | // Check for empty domain | |
| 283 | - | if domain == "" { | |
| 283 | + | if domainName == "" { | |
| 284 | 284 | return ErrDomainEmpty | |
| 285 | 285 | } | |
| 286 | 286 | ||
| 287 | 287 | // Check domain length (max DNS name length is 253) | |
| 288 | - | if len(domain) > 253 { | |
| 288 | + | if len(domainName) > 253 { | |
| 289 | 289 | return ErrDomainTooLong | |
| 290 | 290 | } | |
| 291 | 291 | ||
| 292 | 292 | // Check for path traversal attempts | |
| 293 | - | if strings.Contains(domain, "..") { | |
| 293 | + | if strings.Contains(domainName, "..") { | |
| 294 | 294 | return ErrDomainPathTraversal | |
| 295 | 295 | } | |
| 296 | 296 | ||
| 297 | 297 | // Check for null bytes (can cause issues in file paths) | |
| 298 | - | if strings.ContainsRune(domain, '\x00') { | |
| 298 | + | if strings.ContainsRune(domainName, '\x00') { | |
| 299 | + | return ErrDomainInvalidChars | |
| 300 | + | } | |
| 301 | + | ||
| 302 | + | // Reuse the env-file sanitizer so validation matches the on-disk naming | |
| 303 | + | // scheme and rejects ambiguous names that would collide after sanitization. | |
| 304 | + | if _, err := domain.SanitizeDomainForEnvFile(domainName); err != nil { | |
| 299 | 305 | return ErrDomainInvalidChars | |
| 300 | 306 | } | |
| 301 | 307 | ||
| @@ -278,24 +278,30 @@ func resolveContainerName(domainName, service string) string { | |||||
| 278 | 278 | ||||
| 279 | // ValidateDomain validates that a domain is safe to use for secret storage. | 279 | // ValidateDomain validates that a domain is safe to use for secret storage. | ||
| 280 | // This prevents path traversal attacks and ensures the domain is valid. | 280 | // This prevents path traversal attacks and ensures the domain is valid. | ||
| 281 | - | func ValidateDomain(domain string) error { | 281 | + | func ValidateDomain(domainName string) error { |
| 282 | // Check for empty domain | 282 | // Check for empty domain | ||
| 283 | - | if domain == "" { | 283 | + | if domainName == "" { |
| 284 | return ErrDomainEmpty | 284 | return ErrDomainEmpty | ||
| 285 | } | 285 | } | ||
| 286 | 286 | ||||
| 287 | // Check domain length (max DNS name length is 253) | 287 | // Check domain length (max DNS name length is 253) | ||
| 288 | - | if len(domain) > 253 { | 288 | + | if len(domainName) > 253 { |
| 289 | return ErrDomainTooLong | 289 | return ErrDomainTooLong | ||
| 290 | } | 290 | } | ||
| 291 | 291 | ||||
| 292 | // Check for path traversal attempts | 292 | // Check for path traversal attempts | ||
| 293 | - | if strings.Contains(domain, "..") { | 293 | + | if strings.Contains(domainName, "..") { |
| 294 | return ErrDomainPathTraversal | 294 | return ErrDomainPathTraversal | ||
| 295 | } | 295 | } | ||
| 296 | 296 | ||||
| 297 | // Check for null bytes (can cause issues in file paths) | 297 | // Check for null bytes (can cause issues in file paths) | ||
| 298 | - | if strings.ContainsRune(domain, '\x00') { | 298 | + | if strings.ContainsRune(domainName, '\x00') { |
| 299 | + | return ErrDomainInvalidChars | |||
| 300 | + | } | |||
| 301 | + | ||||
| 302 | + | // Reuse the env-file sanitizer so validation matches the on-disk naming | |||
| 303 | + | // scheme and rejects ambiguous names that would collide after sanitization. | |||
| 304 | + | if _, err := domain.SanitizeDomainForEnvFile(domainName); err != nil { | |||
| 299 | return ErrDomainInvalidChars | 305 | return ErrDomainInvalidChars | ||
| 300 | } | 306 | } | ||
| 301 | 307 | ||||
M internal/usecase/secrets/service_test.go +11 -0
| @@ -108,6 +108,11 @@ func TestValidateDomain(t *testing.T) { | |||
| 108 | 108 | domain: "\x00example.com", | |
| 109 | 109 | wantErr: ErrDomainInvalidChars, | |
| 110 | 110 | }, | |
| 111 | + | { | |
| 112 | + | name: "underscore rejected to avoid env filename collision", | |
| 113 | + | domain: "app_example.com", | |
| 114 | + | wantErr: ErrDomainInvalidChars, | |
| 115 | + | }, | |
| 111 | 116 | } | |
| 112 | 117 | ||
| 113 | 118 | for _, tt := range tests { | |
| @@ -122,6 +127,12 @@ func TestValidateDomain(t *testing.T) { | |||
| 122 | 127 | } | |
| 123 | 128 | } | |
| 124 | 129 | ||
| 130 | + | func TestValidateDomain_DistinguishesSeparatorsForEnvStorage(t *testing.T) { | |
| 131 | + | for _, domain := range []string{"app.example.com", "app:example:com", "app/example/com"} { | |
| 132 | + | assert.NoError(t, ValidateDomain(domain), "expected %q to remain valid", domain) | |
| 133 | + | } | |
| 134 | + | } | |
| 135 | + | ||
| 125 | 136 | func TestService_ListKeys_ValidationErrors(t *testing.T) { | |
| 126 | 137 | store := outmocks.NewMockDomainSecretStore(t) | |
| 127 | 138 | svc := NewService(store, testLogger()) | |
| @@ -108,6 +108,11 @@ func TestValidateDomain(t *testing.T) { | |||||
| 108 | domain: "\x00example.com", | 108 | domain: "\x00example.com", | ||
| 109 | wantErr: ErrDomainInvalidChars, | 109 | wantErr: ErrDomainInvalidChars, | ||
| 110 | }, | 110 | }, | ||
| 111 | + | { | |||
| 112 | + | name: "underscore rejected to avoid env filename collision", | |||
| 113 | + | domain: "app_example.com", | |||
| 114 | + | wantErr: ErrDomainInvalidChars, | |||
| 115 | + | }, | |||
| 111 | } | 116 | } | ||
| 112 | 117 | ||||
| 113 | for _, tt := range tests { | 118 | for _, tt := range tests { | ||
| @@ -122,6 +127,12 @@ func TestValidateDomain(t *testing.T) { | |||||
| 122 | } | 127 | } | ||
| 123 | } | 128 | } | ||
| 124 | 129 | ||||
| 130 | + | func TestValidateDomain_DistinguishesSeparatorsForEnvStorage(t *testing.T) { | |||
| 131 | + | for _, domain := range []string{"app.example.com", "app:example:com", "app/example/com"} { | |||
| 132 | + | assert.NoError(t, ValidateDomain(domain), "expected %q to remain valid", domain) | |||
| 133 | + | } | |||
| 134 | + | } | |||
| 135 | + | ||||
| 125 | func TestService_ListKeys_ValidationErrors(t *testing.T) { | 136 | func TestService_ListKeys_ValidationErrors(t *testing.T) { | ||
| 126 | store := outmocks.NewMockDomainSecretStore(t) | 137 | store := outmocks.NewMockDomainSecretStore(t) | ||
| 127 | svc := NewService(store, testLogger()) | 138 | svc := NewService(store, testLogger()) | ||