fix: address coderabbit security review
- commit
- 225cbb6d3f4602e614fad380e2f3666b5f534264
- author
- brice · · 2026-04-25 16:13 UTC
- parent
- e73e6fc
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
23 files · +348 -139
M .github/actions/setup-gordon/action.yml +2 -1
| @@ -83,7 +83,8 @@ runs: | |||
| 83 | 83 | curl -fsSL -o "${dest}" "${url}" | |
| 84 | 84 | ||
| 85 | 85 | # Verify checksum before extraction | |
| 86 | - | checksum_lines="$(grep -E "^[0-9a-fA-F]{64}[[:space:]]+\\*?${archive}$" "${checksums}" || true)" | |
| 86 | + | archive_escaped="$(printf '%s\n' "${archive}" | sed 's/[.[\\*^$()+?{|]/\\\\&/g')" | |
| 87 | + | checksum_lines="$(grep -E "^[0-9a-fA-F]{64}[[:space:]]+\\*?${archive_escaped}$" "${checksums}" || true)" | |
| 87 | 88 | if [ "$(printf '%s\n' "${checksum_lines}" | sed '/^$/d' | wc -l | tr -d ' ')" != "1" ]; then | |
| 88 | 89 | echo "Expected exactly one checksum for ${archive} in checksums.txt" >&2 | |
| 89 | 90 | exit 1 | |
| @@ -83,7 +83,8 @@ runs: | |||||
| 83 | curl -fsSL -o "${dest}" "${url}" | 83 | curl -fsSL -o "${dest}" "${url}" | ||
| 84 | 84 | ||||
| 85 | # Verify checksum before extraction | 85 | # Verify checksum before extraction | ||
| 86 | - | checksum_lines="$(grep -E "^[0-9a-fA-F]{64}[[:space:]]+\\*?${archive}$" "${checksums}" || true)" | 86 | + | archive_escaped="$(printf '%s\n' "${archive}" | sed 's/[.[\\*^$()+?{|]/\\\\&/g')" |
| 87 | + | checksum_lines="$(grep -E "^[0-9a-fA-F]{64}[[:space:]]+\\*?${archive_escaped}$" "${checksums}" || true)" | |||
| 87 | if [ "$(printf '%s\n' "${checksum_lines}" | sed '/^$/d' | wc -l | tr -d ' ')" != "1" ]; then | 88 | if [ "$(printf '%s\n' "${checksum_lines}" | sed '/^$/d' | wc -l | tr -d ' ')" != "1" ]; then | ||
| 88 | echo "Expected exactly one checksum for ${archive} in checksums.txt" >&2 | 89 | echo "Expected exactly one checksum for ${archive} in checksums.txt" >&2 | ||
| 89 | exit 1 | 90 | exit 1 | ||
M docs/config/images.md +1 -1
| @@ -30,7 +30,7 @@ keep_last = 3 | |||
| 30 | 30 | ||
| 31 | 31 | | Key | Type | Default | Description | | |
| 32 | 32 | |-----|------|---------|-------------| | |
| 33 | - | | `images.allowed_registries` | array | `[]` | Allowlist for explicit external image registries. Empty means no explicit external registries are accepted. Gordon always allows its configured registry and rejects localhost/private/link-local registries. Include ports when needed, e.g. `"registry.example.com:5000"`. | | |
| 33 | + | | `images.allowed_registries` | array | `[]` | Allowlist for external image registries. Empty means no external registries are accepted. Unqualified image names such as `nginx:latest` are treated as Docker Hub (`docker.io`) for policy checks. Gordon always allows its configured registry and rejects localhost/private/link-local registries. Include ports when needed, e.g. `"registry.example.com:5000"`. | | |
| 34 | 34 | | `images.require_digest` | bool | `false` | Require allowlisted external image references to use a valid `@sha256:<64 hex chars>` digest. Gordon registry images are exempt. | | |
| 35 | 35 | | `images.prune.enabled` | bool | `false` | Enables scheduled image cleanup | | |
| 36 | 36 | | `images.prune.schedule` | string | `"daily"` | Schedule preset: `hourly`, `daily`, `weekly`, `monthly` | | |
| @@ -30,7 +30,7 @@ keep_last = 3 | |||||
| 30 | 30 | ||||
| 31 | | Key | Type | Default | Description | | 31 | | Key | Type | Default | Description | | ||
| 32 | |-----|------|---------|-------------| | 32 | |-----|------|---------|-------------| | ||
| 33 | - | | `images.allowed_registries` | array | `[]` | Allowlist for explicit external image registries. Empty means no explicit external registries are accepted. Gordon always allows its configured registry and rejects localhost/private/link-local registries. Include ports when needed, e.g. `"registry.example.com:5000"`. | | 33 | + | | `images.allowed_registries` | array | `[]` | Allowlist for external image registries. Empty means no external registries are accepted. Unqualified image names such as `nginx:latest` are treated as Docker Hub (`docker.io`) for policy checks. Gordon always allows its configured registry and rejects localhost/private/link-local registries. Include ports when needed, e.g. `"registry.example.com:5000"`. | |
| 34 | | `images.require_digest` | bool | `false` | Require allowlisted external image references to use a valid `@sha256:<64 hex chars>` digest. Gordon registry images are exempt. | | 34 | | `images.require_digest` | bool | `false` | Require allowlisted external image references to use a valid `@sha256:<64 hex chars>` digest. Gordon registry images are exempt. | | ||
| 35 | | `images.prune.enabled` | bool | `false` | Enables scheduled image cleanup | | 35 | | `images.prune.enabled` | bool | `false` | Enables scheduled image cleanup | | ||
| 36 | | `images.prune.schedule` | string | `"daily"` | Schedule preset: `hourly`, `daily`, `weekly`, `monthly` | | 36 | | `images.prune.schedule` | string | `"daily"` | Schedule preset: `hourly`, `daily`, `weekly`, `monthly` | | ||
M install.sh +2 -1
| @@ -101,7 +101,8 @@ fi | |||
| 101 | 101 | ||
| 102 | 102 | # Verify checksum | |
| 103 | 103 | echo "Verifying checksum..." | |
| 104 | - | CHECKSUM_LINES=$(grep -E "^[0-9a-fA-F]{64}[[:space:]]+\*?${TARBALL}\$" "$TMP_DIR/checksums.txt" || true) | |
| 104 | + | TARBALL_ESCAPED=$(printf '%s\n' "$TARBALL" | sed 's/[.[\*^$()+?{|]/\\&/g') | |
| 105 | + | CHECKSUM_LINES=$(grep -E "^[0-9a-fA-F]{64}[[:space:]]+\*?${TARBALL_ESCAPED}\$" "$TMP_DIR/checksums.txt" || true) | |
| 105 | 106 | CHECKSUM_COUNT=$(printf '%s\n' "$CHECKSUM_LINES" | sed '/^$/d' | wc -l | tr -d ' ') | |
| 106 | 107 | if [ "$CHECKSUM_COUNT" != "1" ]; then | |
| 107 | 108 | echo "Error: Expected exactly one checksum for ${TARBALL} in checksums.txt" | |
| @@ -101,7 +101,8 @@ fi | |||||
| 101 | 101 | ||||
| 102 | # Verify checksum | 102 | # Verify checksum | ||
| 103 | echo "Verifying checksum..." | 103 | echo "Verifying checksum..." | ||
| 104 | - | CHECKSUM_LINES=$(grep -E "^[0-9a-fA-F]{64}[[:space:]]+\*?${TARBALL}\$" "$TMP_DIR/checksums.txt" || true) | 104 | + | TARBALL_ESCAPED=$(printf '%s\n' "$TARBALL" | sed 's/[.[\*^$()+?{|]/\\&/g') |
| 105 | + | CHECKSUM_LINES=$(grep -E "^[0-9a-fA-F]{64}[[:space:]]+\*?${TARBALL_ESCAPED}\$" "$TMP_DIR/checksums.txt" || true) | |||
| 105 | CHECKSUM_COUNT=$(printf '%s\n' "$CHECKSUM_LINES" | sed '/^$/d' | wc -l | tr -d ' ') | 106 | CHECKSUM_COUNT=$(printf '%s\n' "$CHECKSUM_LINES" | sed '/^$/d' | wc -l | tr -d ' ') | ||
| 106 | if [ "$CHECKSUM_COUNT" != "1" ]; then | 107 | if [ "$CHECKSUM_COUNT" != "1" ]; then | ||
| 107 | echo "Error: Expected exactly one checksum for ${TARBALL} in checksums.txt" | 108 | echo "Error: Expected exactly one checksum for ${TARBALL} in checksums.txt" | ||
M internal/adapters/in/cli/auth.go +3 -1
| @@ -84,7 +84,7 @@ Registry scopes (for Docker push/pull): | |||
| 84 | 84 | Admin scopes (for remote CLI access): | |
| 85 | 85 | Format: admin:<resource>:<actions> | |
| 86 | 86 | ||
| 87 | - | Resources: routes, secrets, config, status, logs, * (all) | |
| 87 | + | Resources: routes, secrets, config, status, logs, volumes, * (all) | |
| 88 | 88 | Actions: read, write, * (all) | |
| 89 | 89 | ||
| 90 | 90 | Examples: | |
| @@ -92,6 +92,8 @@ Admin scopes (for remote CLI access): | |||
| 92 | 92 | admin:routes:read Read-only routes access | |
| 93 | 93 | admin:status:read Read-only status access | |
| 94 | 94 | admin:logs:read Read-only log access | |
| 95 | + | admin:volumes:read Read-only volume access | |
| 96 | + | admin:volumes:write Volume management access | |
| 95 | 97 | ||
| 96 | 98 | Repository scoping: | |
| 97 | 99 | --repo myapp Scope to specific repository | |
| @@ -84,7 +84,7 @@ Registry scopes (for Docker push/pull): | |||||
| 84 | Admin scopes (for remote CLI access): | 84 | Admin scopes (for remote CLI access): | ||
| 85 | Format: admin:<resource>:<actions> | 85 | Format: admin:<resource>:<actions> | ||
| 86 | 86 | ||||
| 87 | - | Resources: routes, secrets, config, status, logs, * (all) | 87 | + | Resources: routes, secrets, config, status, logs, volumes, * (all) |
| 88 | Actions: read, write, * (all) | 88 | Actions: read, write, * (all) | ||
| 89 | 89 | ||||
| 90 | Examples: | 90 | Examples: | ||
| @@ -92,6 +92,8 @@ Admin scopes (for remote CLI access): | |||||
| 92 | admin:routes:read Read-only routes access | 92 | admin:routes:read Read-only routes access | ||
| 93 | admin:status:read Read-only status access | 93 | admin:status:read Read-only status access | ||
| 94 | admin:logs:read Read-only log access | 94 | admin:logs:read Read-only log access | ||
| 95 | + | admin:volumes:read Read-only volume access | |||
| 96 | + | admin:volumes:write Volume management access | |||
| 95 | 97 | ||||
| 96 | Repository scoping: | 98 | Repository scoping: | ||
| 97 | --repo myapp Scope to specific repository | 99 | --repo myapp Scope to specific repository | ||
M internal/adapters/in/cli/controlplane_local.go +4 -0
| @@ -4,6 +4,7 @@ import ( | |||
| 4 | 4 | "context" | |
| 5 | 5 | "errors" | |
| 6 | 6 | "fmt" | |
| 7 | + | "sort" | |
| 7 | 8 | "time" | |
| 8 | 9 | ||
| 9 | 10 | "github.com/bnema/gordon/internal/adapters/dto" | |
| @@ -377,6 +378,9 @@ func (l *localControlPlane) GetConfig(ctx context.Context) (*remote.Config, erro | |||
| 377 | 378 | for domainName := range externalRoutes { | |
| 378 | 379 | externalResponses = append(externalResponses, remote.ExternalRoute{Domain: domainName}) | |
| 379 | 380 | } | |
| 381 | + | sort.Slice(externalResponses, func(i, j int) bool { | |
| 382 | + | return externalResponses[i].Domain < externalResponses[j].Domain | |
| 383 | + | }) | |
| 380 | 384 | cfg := &remote.Config{ | |
| 381 | 385 | Routes: l.configSvc.GetRoutes(ctx), | |
| 382 | 386 | ExternalRoutes: externalResponses, | |
| @@ -4,6 +4,7 @@ import ( | |||||
| 4 | "context" | 4 | "context" | ||
| 5 | "errors" | 5 | "errors" | ||
| 6 | "fmt" | 6 | "fmt" | ||
| 7 | + | "sort" | |||
| 7 | "time" | 8 | "time" | ||
| 8 | 9 | ||||
| 9 | "github.com/bnema/gordon/internal/adapters/dto" | 10 | "github.com/bnema/gordon/internal/adapters/dto" | ||
| @@ -377,6 +378,9 @@ func (l *localControlPlane) GetConfig(ctx context.Context) (*remote.Config, erro | |||||
| 377 | for domainName := range externalRoutes { | 378 | for domainName := range externalRoutes { | ||
| 378 | externalResponses = append(externalResponses, remote.ExternalRoute{Domain: domainName}) | 379 | externalResponses = append(externalResponses, remote.ExternalRoute{Domain: domainName}) | ||
| 379 | } | 380 | } | ||
| 381 | + | sort.Slice(externalResponses, func(i, j int) bool { | |||
| 382 | + | return externalResponses[i].Domain < externalResponses[j].Domain | |||
| 383 | + | }) | |||
| 380 | cfg := &remote.Config{ | 384 | cfg := &remote.Config{ | ||
| 381 | Routes: l.configSvc.GetRoutes(ctx), | 385 | Routes: l.configSvc.GetRoutes(ctx), | ||
| 382 | ExternalRoutes: externalResponses, | 386 | ExternalRoutes: externalResponses, | ||
M internal/adapters/in/http/admin/handler.go +6 -0
| @@ -389,6 +389,10 @@ func (h *Handler) handleBootstrap(w http.ResponseWriter, r *http.Request) { | |||
| 389 | 389 | addStep("route", "failed") | |
| 390 | 390 | h.sendError(w, http.StatusBadRequest, err.Error()) | |
| 391 | 391 | return | |
| 392 | + | case errors.Is(err, domain.ErrRouteConflict): | |
| 393 | + | addStep("route", "failed") | |
| 394 | + | h.sendError(w, http.StatusConflict, err.Error()) | |
| 395 | + | return | |
| 392 | 396 | default: | |
| 393 | 397 | addStep("route", "failed") | |
| 394 | 398 | log.Error().Err(err).Str("domain", req.Domain).Str("image", req.Image).Msg("failed to bootstrap route") | |
| @@ -594,6 +598,8 @@ func (h *Handler) handleRoutesPost(w http.ResponseWriter, r *http.Request) { | |||
| 594 | 598 | switch { | |
| 595 | 599 | case errors.Is(err, domain.ErrRouteDomainEmpty), errors.Is(err, domain.ErrRouteDomainInvalid), errors.Is(err, domain.ErrRouteImageEmpty): | |
| 596 | 600 | h.sendError(w, http.StatusBadRequest, err.Error()) | |
| 601 | + | case errors.Is(err, domain.ErrRouteConflict): | |
| 602 | + | h.sendError(w, http.StatusConflict, err.Error()) | |
| 597 | 603 | default: | |
| 598 | 604 | h.sendError(w, http.StatusInternalServerError, "failed to add route") | |
| 599 | 605 | } | |
| @@ -389,6 +389,10 @@ func (h *Handler) handleBootstrap(w http.ResponseWriter, r *http.Request) { | |||||
| 389 | addStep("route", "failed") | 389 | addStep("route", "failed") | ||
| 390 | h.sendError(w, http.StatusBadRequest, err.Error()) | 390 | h.sendError(w, http.StatusBadRequest, err.Error()) | ||
| 391 | return | 391 | return | ||
| 392 | + | case errors.Is(err, domain.ErrRouteConflict): | |||
| 393 | + | addStep("route", "failed") | |||
| 394 | + | h.sendError(w, http.StatusConflict, err.Error()) | |||
| 395 | + | return | |||
| 392 | default: | 396 | default: | ||
| 393 | addStep("route", "failed") | 397 | addStep("route", "failed") | ||
| 394 | log.Error().Err(err).Str("domain", req.Domain).Str("image", req.Image).Msg("failed to bootstrap route") | 398 | log.Error().Err(err).Str("domain", req.Domain).Str("image", req.Image).Msg("failed to bootstrap route") | ||
| @@ -594,6 +598,8 @@ func (h *Handler) handleRoutesPost(w http.ResponseWriter, r *http.Request) { | |||||
| 594 | switch { | 598 | switch { | ||
| 595 | case errors.Is(err, domain.ErrRouteDomainEmpty), errors.Is(err, domain.ErrRouteDomainInvalid), errors.Is(err, domain.ErrRouteImageEmpty): | 599 | case errors.Is(err, domain.ErrRouteDomainEmpty), errors.Is(err, domain.ErrRouteDomainInvalid), errors.Is(err, domain.ErrRouteImageEmpty): | ||
| 596 | h.sendError(w, http.StatusBadRequest, err.Error()) | 600 | h.sendError(w, http.StatusBadRequest, err.Error()) | ||
| 601 | + | case errors.Is(err, domain.ErrRouteConflict): | |||
| 602 | + | h.sendError(w, http.StatusConflict, err.Error()) | |||
| 597 | default: | 603 | default: | ||
| 598 | h.sendError(w, http.StatusInternalServerError, "failed to add route") | 604 | h.sendError(w, http.StatusInternalServerError, "failed to add route") | ||
| 599 | } | 605 | } | ||
M internal/adapters/in/http/admin/handler_test.go +52 -46
| @@ -118,11 +118,11 @@ func TestHandler_VolumesGet_RequiresVolumesReadScope(t *testing.T) { | |||
| 118 | 118 | if tt.wantStatus == http.StatusOK { | |
| 119 | 119 | volumeSvc.EXPECT().ListVolumes(mock.Anything).Return([]*domain.VolumeInfo{}, nil).Once() | |
| 120 | 120 | } | |
| 121 | - | req := httptest.NewRequest(http.MethodGet, "/admin/volumes", nil) | |
| 122 | - | req = req.WithContext(ctxWithScopes(tt.scopes...)) | |
| 123 | - | rec := httptest.NewRecorder() | |
| 124 | - | handler.ServeHTTP(rec, req) | |
| 125 | - | assert.Equal(t, tt.wantStatus, rec.Code) | |
| 121 | + | server := newScopedTestServer(t, handler, tt.scopes...) | |
| 122 | + | resp, err := http.Get(server.URL + "/admin/volumes") | |
| 123 | + | require.NoError(t, err) | |
| 124 | + | defer resp.Body.Close() | |
| 125 | + | assert.Equal(t, tt.wantStatus, resp.StatusCode) | |
| 126 | 126 | }) | |
| 127 | 127 | } | |
| 128 | 128 | } | |
| @@ -147,11 +147,11 @@ func TestHandler_VolumesPrune_RequiresVolumesWriteScope(t *testing.T) { | |||
| 147 | 147 | if tt.wantStatus == http.StatusOK { | |
| 148 | 148 | volumeSvc.EXPECT().PruneVolumes(mock.Anything, true).Return(&domain.VolumePruneReport{}, []*domain.VolumeInfo{}, nil).Once() | |
| 149 | 149 | } | |
| 150 | - | req := httptest.NewRequest(http.MethodPost, "/admin/volumes/prune", bytes.NewBufferString(`{"dry_run":true}`)) | |
| 151 | - | req = req.WithContext(ctxWithScopes(tt.scopes...)) | |
| 152 | - | rec := httptest.NewRecorder() | |
| 153 | - | handler.ServeHTTP(rec, req) | |
| 154 | - | assert.Equal(t, tt.wantStatus, rec.Code) | |
| 150 | + | server := newScopedTestServer(t, handler, tt.scopes...) | |
| 151 | + | resp, err := http.Post(server.URL+"/admin/volumes/prune", "application/json", bytes.NewBufferString(`{"dry_run":true}`)) | |
| 152 | + | require.NoError(t, err) | |
| 153 | + | defer resp.Body.Close() | |
| 154 | + | assert.Equal(t, tt.wantStatus, resp.StatusCode) | |
| 155 | 155 | }) | |
| 156 | 156 | } | |
| 157 | 157 | } | |
| @@ -1135,18 +1135,19 @@ func TestHandler_Config_HidesSensitiveInventoryByDefault(t *testing.T) { | |||
| 1135 | 1135 | configSvc.EXPECT().GetRoutes(mock.Anything).Return([]domain.Route{}).Once() | |
| 1136 | 1136 | configSvc.EXPECT().GetExternalRoutes().Return(map[string]string{"db.example.com": "http://10.0.0.5:5432"}).Once() | |
| 1137 | 1137 | ||
| 1138 | - | req := httptest.NewRequest("GET", "/admin/config", nil) | |
| 1139 | - | req = req.WithContext(ctxWithScopes("admin:config:read")) | |
| 1140 | - | rec := httptest.NewRecorder() | |
| 1141 | - | ||
| 1142 | - | handler.ServeHTTP(rec, req) | |
| 1138 | + | server := newScopedTestServer(t, handler, "admin:config:read") | |
| 1139 | + | resp, err := http.Get(server.URL + "/admin/config") | |
| 1140 | + | require.NoError(t, err) | |
| 1141 | + | defer resp.Body.Close() | |
| 1142 | + | bodyBytes, err := io.ReadAll(resp.Body) | |
| 1143 | + | require.NoError(t, err) | |
| 1143 | 1144 | ||
| 1144 | - | require.Equal(t, http.StatusOK, rec.Code) | |
| 1145 | - | body := rec.Body.String() | |
| 1145 | + | require.Equal(t, http.StatusOK, resp.StatusCode) | |
| 1146 | + | body := string(bodyBytes) | |
| 1146 | 1147 | var raw map[string]any | |
| 1147 | 1148 | require.NoError(t, json.Unmarshal([]byte(body), &raw)) | |
| 1148 | - | server := raw["server"].(map[string]any) | |
| 1149 | - | assert.NotContains(t, server, "data_dir") | |
| 1149 | + | serverConfig := raw["server"].(map[string]any) | |
| 1150 | + | assert.NotContains(t, serverConfig, "data_dir") | |
| 1150 | 1151 | assert.NotContains(t, body, "/var/lib/gordon") | |
| 1151 | 1152 | assert.NotContains(t, body, "10.0.0.5") | |
| 1152 | 1153 | } | |
| @@ -1650,16 +1651,18 @@ func TestHandler_Deploy_ConfigWriteOnlyOmitsDeployFailureLogs(t *testing.T) { | |||
| 1650 | 1651 | configSvc.EXPECT().GetRoute(mock.Anything, "app.example.com").Return(route, nil).Once() | |
| 1651 | 1652 | containerSvc.EXPECT().Deploy(mock.Anything, *route).Return(nil, deployErr).Once() | |
| 1652 | 1653 | ||
| 1653 | - | req := httptest.NewRequest(http.MethodPost, "/admin/deploy/app.example.com", nil) | |
| 1654 | - | req = req.WithContext(ctxWithScopes("admin:config:write")) | |
| 1655 | - | rec := httptest.NewRecorder() | |
| 1656 | - | ||
| 1657 | - | handler.ServeHTTP(rec, req) | |
| 1654 | + | server := newScopedTestServer(t, handler, "admin:config:write") | |
| 1655 | + | resp, err := http.Post(server.URL+"/admin/deploy/app.example.com", "application/json", nil) | |
| 1656 | + | require.NoError(t, err) | |
| 1657 | + | defer resp.Body.Close() | |
| 1658 | + | bodyBytes, err := io.ReadAll(resp.Body) | |
| 1659 | + | require.NoError(t, err) | |
| 1660 | + | body := string(bodyBytes) | |
| 1658 | 1661 | ||
| 1659 | - | assert.Equal(t, http.StatusInternalServerError, rec.Code) | |
| 1660 | - | assert.JSONEq(t, `{"error":"failed to deploy","cause":"image pull failed","hint":"push the image before retrying"}`, rec.Body.String()) | |
| 1661 | - | assert.NotContains(t, rec.Body.String(), "hunter2") | |
| 1662 | - | assert.NotContains(t, rec.Body.String(), "abc123") | |
| 1662 | + | assert.Equal(t, http.StatusInternalServerError, resp.StatusCode) | |
| 1663 | + | assert.JSONEq(t, `{"error":"failed to deploy","cause":"image pull failed","hint":"push the image before retrying"}`, body) | |
| 1664 | + | assert.NotContains(t, body, "hunter2") | |
| 1665 | + | assert.NotContains(t, body, "abc123") | |
| 1663 | 1666 | } | |
| 1664 | 1667 | ||
| 1665 | 1668 | func TestHandler_Deploy_StructuredDeployFailure(t *testing.T) { | |
| @@ -2174,30 +2177,33 @@ func TestHandler_LogsRequireLogsReadAndRedact(t *testing.T) { | |||
| 2174 | 2177 | }) | |
| 2175 | 2178 | ||
| 2176 | 2179 | t.Run("status read denied", func(t *testing.T) { | |
| 2177 | - | req := httptest.NewRequest(http.MethodGet, "/admin/logs", nil) | |
| 2178 | - | req = req.WithContext(ctxWithScopes("admin:status:read")) | |
| 2179 | - | rec := httptest.NewRecorder() | |
| 2180 | - | ||
| 2181 | - | handler.ServeHTTP(rec, req) | |
| 2180 | + | server := newScopedTestServer(t, handler, "admin:status:read") | |
| 2181 | + | resp, err := http.Get(server.URL + "/admin/logs") | |
| 2182 | + | require.NoError(t, err) | |
| 2183 | + | defer resp.Body.Close() | |
| 2184 | + | bodyBytes, err := io.ReadAll(resp.Body) | |
| 2185 | + | require.NoError(t, err) | |
| 2182 | 2186 | ||
| 2183 | - | assert.Equal(t, http.StatusForbidden, rec.Code) | |
| 2184 | - | assert.Contains(t, rec.Body.String(), "insufficient permissions for logs:read") | |
| 2187 | + | assert.Equal(t, http.StatusForbidden, resp.StatusCode) | |
| 2188 | + | assert.Contains(t, string(bodyBytes), "insufficient permissions for logs:read") | |
| 2185 | 2189 | }) | |
| 2186 | 2190 | ||
| 2187 | 2191 | t.Run("logs read allowed and redacted", func(t *testing.T) { | |
| 2188 | 2192 | logSvc.EXPECT().GetProcessLogs(mock.Anything, 50).Return([]string{"PASSWORD=hunter2", `{"API_KEY":"abc123"}`}, nil).Once() | |
| 2189 | 2193 | ||
| 2190 | - | req := httptest.NewRequest(http.MethodGet, "/admin/logs", nil) | |
| 2191 | - | req = req.WithContext(ctxWithScopes("admin:logs:read")) | |
| 2192 | - | rec := httptest.NewRecorder() | |
| 2193 | - | ||
| 2194 | - | handler.ServeHTTP(rec, req) | |
| 2195 | - | ||
| 2196 | - | assert.Equal(t, http.StatusOK, rec.Code) | |
| 2197 | - | assert.Contains(t, rec.Body.String(), "PASSWORD=[REDACTED]") | |
| 2198 | - | assert.Contains(t, rec.Body.String(), `\"API_KEY\":\"[REDACTED]\"`) | |
| 2199 | - | assert.NotContains(t, rec.Body.String(), "hunter2") | |
| 2200 | - | assert.NotContains(t, rec.Body.String(), "abc123") | |
| 2194 | + | server := newScopedTestServer(t, handler, "admin:logs:read") | |
| 2195 | + | resp, err := http.Get(server.URL + "/admin/logs") | |
| 2196 | + | require.NoError(t, err) | |
| 2197 | + | defer resp.Body.Close() | |
| 2198 | + | bodyBytes, err := io.ReadAll(resp.Body) | |
| 2199 | + | require.NoError(t, err) | |
| 2200 | + | body := string(bodyBytes) | |
| 2201 | + | ||
| 2202 | + | assert.Equal(t, http.StatusOK, resp.StatusCode) | |
| 2203 | + | assert.Contains(t, body, "PASSWORD=[REDACTED]") | |
| 2204 | + | assert.Contains(t, body, `\"API_KEY\":\"[REDACTED]\"`) | |
| 2205 | + | assert.NotContains(t, body, "hunter2") | |
| 2206 | + | assert.NotContains(t, body, "abc123") | |
| 2201 | 2207 | }) | |
| 2202 | 2208 | } | |
| 2203 | 2209 | ||
| @@ -118,11 +118,11 @@ func TestHandler_VolumesGet_RequiresVolumesReadScope(t *testing.T) { | |||||
| 118 | if tt.wantStatus == http.StatusOK { | 118 | if tt.wantStatus == http.StatusOK { | ||
| 119 | volumeSvc.EXPECT().ListVolumes(mock.Anything).Return([]*domain.VolumeInfo{}, nil).Once() | 119 | volumeSvc.EXPECT().ListVolumes(mock.Anything).Return([]*domain.VolumeInfo{}, nil).Once() | ||
| 120 | } | 120 | } | ||
| 121 | - | req := httptest.NewRequest(http.MethodGet, "/admin/volumes", nil) | 121 | + | server := newScopedTestServer(t, handler, tt.scopes...) |
| 122 | - | req = req.WithContext(ctxWithScopes(tt.scopes...)) | 122 | + | resp, err := http.Get(server.URL + "/admin/volumes") |
| 123 | - | rec := httptest.NewRecorder() | 123 | + | require.NoError(t, err) |
| 124 | - | handler.ServeHTTP(rec, req) | 124 | + | defer resp.Body.Close() |
| 125 | - | assert.Equal(t, tt.wantStatus, rec.Code) | 125 | + | assert.Equal(t, tt.wantStatus, resp.StatusCode) |
| 126 | }) | 126 | }) | ||
| 127 | } | 127 | } | ||
| 128 | } | 128 | } | ||
| @@ -147,11 +147,11 @@ func TestHandler_VolumesPrune_RequiresVolumesWriteScope(t *testing.T) { | |||||
| 147 | if tt.wantStatus == http.StatusOK { | 147 | if tt.wantStatus == http.StatusOK { | ||
| 148 | volumeSvc.EXPECT().PruneVolumes(mock.Anything, true).Return(&domain.VolumePruneReport{}, []*domain.VolumeInfo{}, nil).Once() | 148 | volumeSvc.EXPECT().PruneVolumes(mock.Anything, true).Return(&domain.VolumePruneReport{}, []*domain.VolumeInfo{}, nil).Once() | ||
| 149 | } | 149 | } | ||
| 150 | - | req := httptest.NewRequest(http.MethodPost, "/admin/volumes/prune", bytes.NewBufferString(`{"dry_run":true}`)) | 150 | + | server := newScopedTestServer(t, handler, tt.scopes...) |
| 151 | - | req = req.WithContext(ctxWithScopes(tt.scopes...)) | 151 | + | resp, err := http.Post(server.URL+"/admin/volumes/prune", "application/json", bytes.NewBufferString(`{"dry_run":true}`)) |
| 152 | - | rec := httptest.NewRecorder() | 152 | + | require.NoError(t, err) |
| 153 | - | handler.ServeHTTP(rec, req) | 153 | + | defer resp.Body.Close() |
| 154 | - | assert.Equal(t, tt.wantStatus, rec.Code) | 154 | + | assert.Equal(t, tt.wantStatus, resp.StatusCode) |
| 155 | }) | 155 | }) | ||
| 156 | } | 156 | } | ||
| 157 | } | 157 | } | ||
| @@ -1135,18 +1135,19 @@ func TestHandler_Config_HidesSensitiveInventoryByDefault(t *testing.T) { | |||||
| 1135 | configSvc.EXPECT().GetRoutes(mock.Anything).Return([]domain.Route{}).Once() | 1135 | configSvc.EXPECT().GetRoutes(mock.Anything).Return([]domain.Route{}).Once() | ||
| 1136 | configSvc.EXPECT().GetExternalRoutes().Return(map[string]string{"db.example.com": "http://10.0.0.5:5432"}).Once() | 1136 | configSvc.EXPECT().GetExternalRoutes().Return(map[string]string{"db.example.com": "http://10.0.0.5:5432"}).Once() | ||
| 1137 | 1137 | ||||
| 1138 | - | req := httptest.NewRequest("GET", "/admin/config", nil) | 1138 | + | server := newScopedTestServer(t, handler, "admin:config:read") |
| 1139 | - | req = req.WithContext(ctxWithScopes("admin:config:read")) | 1139 | + | resp, err := http.Get(server.URL + "/admin/config") |
| 1140 | - | rec := httptest.NewRecorder() | 1140 | + | require.NoError(t, err) |
| 1141 | - | 1141 | + | defer resp.Body.Close() | |
| 1142 | - | handler.ServeHTTP(rec, req) | 1142 | + | bodyBytes, err := io.ReadAll(resp.Body) |
| 1143 | + | require.NoError(t, err) | |||
| 1143 | 1144 | ||||
| 1144 | - | require.Equal(t, http.StatusOK, rec.Code) | 1145 | + | require.Equal(t, http.StatusOK, resp.StatusCode) |
| 1145 | - | body := rec.Body.String() | 1146 | + | body := string(bodyBytes) |
| 1146 | var raw map[string]any | 1147 | var raw map[string]any | ||
| 1147 | require.NoError(t, json.Unmarshal([]byte(body), &raw)) | 1148 | require.NoError(t, json.Unmarshal([]byte(body), &raw)) | ||
| 1148 | - | server := raw["server"].(map[string]any) | 1149 | + | serverConfig := raw["server"].(map[string]any) |
| 1149 | - | assert.NotContains(t, server, "data_dir") | 1150 | + | assert.NotContains(t, serverConfig, "data_dir") |
| 1150 | assert.NotContains(t, body, "/var/lib/gordon") | 1151 | assert.NotContains(t, body, "/var/lib/gordon") | ||
| 1151 | assert.NotContains(t, body, "10.0.0.5") | 1152 | assert.NotContains(t, body, "10.0.0.5") | ||
| 1152 | } | 1153 | } | ||
| @@ -1650,16 +1651,18 @@ func TestHandler_Deploy_ConfigWriteOnlyOmitsDeployFailureLogs(t *testing.T) { | |||||
| 1650 | configSvc.EXPECT().GetRoute(mock.Anything, "app.example.com").Return(route, nil).Once() | 1651 | configSvc.EXPECT().GetRoute(mock.Anything, "app.example.com").Return(route, nil).Once() | ||
| 1651 | containerSvc.EXPECT().Deploy(mock.Anything, *route).Return(nil, deployErr).Once() | 1652 | containerSvc.EXPECT().Deploy(mock.Anything, *route).Return(nil, deployErr).Once() | ||
| 1652 | 1653 | ||||
| 1653 | - | req := httptest.NewRequest(http.MethodPost, "/admin/deploy/app.example.com", nil) | 1654 | + | server := newScopedTestServer(t, handler, "admin:config:write") |
| 1654 | - | req = req.WithContext(ctxWithScopes("admin:config:write")) | 1655 | + | resp, err := http.Post(server.URL+"/admin/deploy/app.example.com", "application/json", nil) |
| 1655 | - | rec := httptest.NewRecorder() | 1656 | + | require.NoError(t, err) |
| 1656 | - | 1657 | + | defer resp.Body.Close() | |
| 1657 | - | handler.ServeHTTP(rec, req) | 1658 | + | bodyBytes, err := io.ReadAll(resp.Body) |
| 1659 | + | require.NoError(t, err) | |||
| 1660 | + | body := string(bodyBytes) | |||
| 1658 | 1661 | ||||
| 1659 | - | assert.Equal(t, http.StatusInternalServerError, rec.Code) | 1662 | + | assert.Equal(t, http.StatusInternalServerError, resp.StatusCode) |
| 1660 | - | assert.JSONEq(t, `{"error":"failed to deploy","cause":"image pull failed","hint":"push the image before retrying"}`, rec.Body.String()) | 1663 | + | assert.JSONEq(t, `{"error":"failed to deploy","cause":"image pull failed","hint":"push the image before retrying"}`, body) |
| 1661 | - | assert.NotContains(t, rec.Body.String(), "hunter2") | 1664 | + | assert.NotContains(t, body, "hunter2") |
| 1662 | - | assert.NotContains(t, rec.Body.String(), "abc123") | 1665 | + | assert.NotContains(t, body, "abc123") |
| 1663 | } | 1666 | } | ||
| 1664 | 1667 | ||||
| 1665 | func TestHandler_Deploy_StructuredDeployFailure(t *testing.T) { | 1668 | func TestHandler_Deploy_StructuredDeployFailure(t *testing.T) { | ||
| @@ -2174,30 +2177,33 @@ func TestHandler_LogsRequireLogsReadAndRedact(t *testing.T) { | |||||
| 2174 | }) | 2177 | }) | ||
| 2175 | 2178 | ||||
| 2176 | t.Run("status read denied", func(t *testing.T) { | 2179 | t.Run("status read denied", func(t *testing.T) { | ||
| 2177 | - | req := httptest.NewRequest(http.MethodGet, "/admin/logs", nil) | 2180 | + | server := newScopedTestServer(t, handler, "admin:status:read") |
| 2178 | - | req = req.WithContext(ctxWithScopes("admin:status:read")) | 2181 | + | resp, err := http.Get(server.URL + "/admin/logs") |
| 2179 | - | rec := httptest.NewRecorder() | 2182 | + | require.NoError(t, err) |
| 2180 | - | 2183 | + | defer resp.Body.Close() | |
| 2181 | - | handler.ServeHTTP(rec, req) | 2184 | + | bodyBytes, err := io.ReadAll(resp.Body) |
| 2185 | + | require.NoError(t, err) | |||
| 2182 | 2186 | ||||
| 2183 | - | assert.Equal(t, http.StatusForbidden, rec.Code) | 2187 | + | assert.Equal(t, http.StatusForbidden, resp.StatusCode) |
| 2184 | - | assert.Contains(t, rec.Body.String(), "insufficient permissions for logs:read") | 2188 | + | assert.Contains(t, string(bodyBytes), "insufficient permissions for logs:read") |
| 2185 | }) | 2189 | }) | ||
| 2186 | 2190 | ||||
| 2187 | t.Run("logs read allowed and redacted", func(t *testing.T) { | 2191 | t.Run("logs read allowed and redacted", func(t *testing.T) { | ||
| 2188 | logSvc.EXPECT().GetProcessLogs(mock.Anything, 50).Return([]string{"PASSWORD=hunter2", `{"API_KEY":"abc123"}`}, nil).Once() | 2192 | logSvc.EXPECT().GetProcessLogs(mock.Anything, 50).Return([]string{"PASSWORD=hunter2", `{"API_KEY":"abc123"}`}, nil).Once() | ||
| 2189 | 2193 | ||||
| 2190 | - | req := httptest.NewRequest(http.MethodGet, "/admin/logs", nil) | 2194 | + | server := newScopedTestServer(t, handler, "admin:logs:read") |
| 2191 | - | req = req.WithContext(ctxWithScopes("admin:logs:read")) | 2195 | + | resp, err := http.Get(server.URL + "/admin/logs") |
| 2192 | - | rec := httptest.NewRecorder() | 2196 | + | require.NoError(t, err) |
| 2193 | - | 2197 | + | defer resp.Body.Close() | |
| 2194 | - | handler.ServeHTTP(rec, req) | 2198 | + | bodyBytes, err := io.ReadAll(resp.Body) |
| 2195 | - | 2199 | + | require.NoError(t, err) | |
| 2196 | - | assert.Equal(t, http.StatusOK, rec.Code) | 2200 | + | body := string(bodyBytes) |
| 2197 | - | assert.Contains(t, rec.Body.String(), "PASSWORD=[REDACTED]") | 2201 | + | |
| 2198 | - | assert.Contains(t, rec.Body.String(), `\"API_KEY\":\"[REDACTED]\"`) | 2202 | + | assert.Equal(t, http.StatusOK, resp.StatusCode) |
| 2199 | - | assert.NotContains(t, rec.Body.String(), "hunter2") | 2203 | + | assert.Contains(t, body, "PASSWORD=[REDACTED]") |
| 2200 | - | assert.NotContains(t, rec.Body.String(), "abc123") | 2204 | + | assert.Contains(t, body, `\"API_KEY\":\"[REDACTED]\"`) |
| 2205 | + | assert.NotContains(t, body, "hunter2") | |||
| 2206 | + | assert.NotContains(t, body, "abc123") | |||
| 2201 | }) | 2207 | }) | ||
| 2202 | } | 2208 | } | ||
| 2203 | 2209 | ||||
M internal/adapters/in/http/middleware/cidr_test.go +21 -13
| @@ -187,15 +187,20 @@ func TestHTTPSRedirect_TrailingDotHostRedirectsToCanonicalHost(t *testing.T) { | |||
| 187 | 187 | }) | |
| 188 | 188 | handler := HTTPSRedirect(nil, 8088, 8443, true, log, func(host string) bool { return host == "o2.bnema.dev" })(ok) | |
| 189 | 189 | ||
| 190 | - | req := httptest.NewRequest(http.MethodGet, "http://example.test/path", nil) | |
| 191 | - | req.RequestURI = "/path" | |
| 192 | - | req.Host = "O2.Bnema.Dev.:8088" | |
| 193 | - | rec := httptest.NewRecorder() | |
| 190 | + | srv := httptest.NewServer(handler) | |
| 191 | + | defer srv.Close() | |
| 192 | + | client := srv.Client() | |
| 193 | + | client.CheckRedirect = func(*http.Request, []*http.Request) error { return http.ErrUseLastResponse } | |
| 194 | 194 | ||
| 195 | - | handler.ServeHTTP(rec, req) | |
| 195 | + | req, err := http.NewRequest(http.MethodGet, srv.URL+"/path", nil) | |
| 196 | + | require.NoError(t, err) | |
| 197 | + | req.Host = "O2.Bnema.Dev.:8088" | |
| 198 | + | resp, err := client.Do(req) | |
| 199 | + | require.NoError(t, err) | |
| 200 | + | defer resp.Body.Close() | |
| 196 | 201 | ||
| 197 | - | assert.Equal(t, http.StatusPermanentRedirect, rec.Code) | |
| 198 | - | assert.Equal(t, "https://o2.bnema.dev:8443/path", rec.Header().Get("Location")) | |
| 202 | + | assert.Equal(t, http.StatusPermanentRedirect, resp.StatusCode) | |
| 203 | + | assert.Equal(t, "https://o2.bnema.dev:8443/path", resp.Header.Get("Location")) | |
| 199 | 204 | } | |
| 200 | 205 | ||
| 201 | 206 | func TestHTTPSRedirect_RejectsInvalidHost(t *testing.T) { | |
| @@ -218,14 +223,17 @@ func TestHTTPSRedirect_RejectsInvalidHost(t *testing.T) { | |||
| 218 | 223 | ||
| 219 | 224 | for _, tt := range tests { | |
| 220 | 225 | t.Run(tt.name, func(t *testing.T) { | |
| 221 | - | req := httptest.NewRequest(http.MethodGet, "http://example.test/", nil) | |
| 226 | + | srv := httptest.NewServer(handler) | |
| 227 | + | defer srv.Close() | |
| 228 | + | req, err := http.NewRequest(http.MethodGet, srv.URL+"/", nil) | |
| 229 | + | require.NoError(t, err) | |
| 222 | 230 | req.Host = tt.host | |
| 223 | - | rec := httptest.NewRecorder() | |
| 224 | - | ||
| 225 | - | handler.ServeHTTP(rec, req) | |
| 231 | + | resp, err := srv.Client().Do(req) | |
| 232 | + | require.NoError(t, err) | |
| 233 | + | defer resp.Body.Close() | |
| 226 | 234 | ||
| 227 | - | assert.Equal(t, http.StatusBadRequest, rec.Code) | |
| 228 | - | assert.Empty(t, rec.Header().Get("Location")) | |
| 235 | + | assert.Equal(t, http.StatusBadRequest, resp.StatusCode) | |
| 236 | + | assert.Empty(t, resp.Header.Get("Location")) | |
| 229 | 237 | }) | |
| 230 | 238 | } | |
| 231 | 239 | } | |
| @@ -187,15 +187,20 @@ func TestHTTPSRedirect_TrailingDotHostRedirectsToCanonicalHost(t *testing.T) { | |||||
| 187 | }) | 187 | }) | ||
| 188 | handler := HTTPSRedirect(nil, 8088, 8443, true, log, func(host string) bool { return host == "o2.bnema.dev" })(ok) | 188 | handler := HTTPSRedirect(nil, 8088, 8443, true, log, func(host string) bool { return host == "o2.bnema.dev" })(ok) | ||
| 189 | 189 | ||||
| 190 | - | req := httptest.NewRequest(http.MethodGet, "http://example.test/path", nil) | 190 | + | srv := httptest.NewServer(handler) |
| 191 | - | req.RequestURI = "/path" | 191 | + | defer srv.Close() |
| 192 | - | req.Host = "O2.Bnema.Dev.:8088" | 192 | + | client := srv.Client() |
| 193 | - | rec := httptest.NewRecorder() | 193 | + | client.CheckRedirect = func(*http.Request, []*http.Request) error { return http.ErrUseLastResponse } |
| 194 | 194 | ||||
| 195 | - | handler.ServeHTTP(rec, req) | 195 | + | req, err := http.NewRequest(http.MethodGet, srv.URL+"/path", nil) |
| 196 | + | require.NoError(t, err) | |||
| 197 | + | req.Host = "O2.Bnema.Dev.:8088" | |||
| 198 | + | resp, err := client.Do(req) | |||
| 199 | + | require.NoError(t, err) | |||
| 200 | + | defer resp.Body.Close() | |||
| 196 | 201 | ||||
| 197 | - | assert.Equal(t, http.StatusPermanentRedirect, rec.Code) | 202 | + | assert.Equal(t, http.StatusPermanentRedirect, resp.StatusCode) |
| 198 | - | assert.Equal(t, "https://o2.bnema.dev:8443/path", rec.Header().Get("Location")) | 203 | + | assert.Equal(t, "https://o2.bnema.dev:8443/path", resp.Header.Get("Location")) |
| 199 | } | 204 | } | ||
| 200 | 205 | ||||
| 201 | func TestHTTPSRedirect_RejectsInvalidHost(t *testing.T) { | 206 | func TestHTTPSRedirect_RejectsInvalidHost(t *testing.T) { | ||
| @@ -218,14 +223,17 @@ func TestHTTPSRedirect_RejectsInvalidHost(t *testing.T) { | |||||
| 218 | 223 | ||||
| 219 | for _, tt := range tests { | 224 | for _, tt := range tests { | ||
| 220 | t.Run(tt.name, func(t *testing.T) { | 225 | t.Run(tt.name, func(t *testing.T) { | ||
| 221 | - | req := httptest.NewRequest(http.MethodGet, "http://example.test/", nil) | 226 | + | srv := httptest.NewServer(handler) |
| 227 | + | defer srv.Close() | |||
| 228 | + | req, err := http.NewRequest(http.MethodGet, srv.URL+"/", nil) | |||
| 229 | + | require.NoError(t, err) | |||
| 222 | req.Host = tt.host | 230 | req.Host = tt.host | ||
| 223 | - | rec := httptest.NewRecorder() | 231 | + | resp, err := srv.Client().Do(req) |
| 224 | - | 232 | + | require.NoError(t, err) | |
| 225 | - | handler.ServeHTTP(rec, req) | 233 | + | defer resp.Body.Close() |
| 226 | 234 | ||||
| 227 | - | assert.Equal(t, http.StatusBadRequest, rec.Code) | 235 | + | assert.Equal(t, http.StatusBadRequest, resp.StatusCode) |
| 228 | - | assert.Empty(t, rec.Header().Get("Location")) | 236 | + | assert.Empty(t, resp.Header.Get("Location")) |
| 229 | }) | 237 | }) | ||
| 230 | } | 238 | } | ||
| 231 | } | 239 | } | ||
M internal/adapters/in/http/proxy/handler.go +6 -0
| @@ -111,6 +111,12 @@ func normalizeRequestHost(host string) string { | |||
| 111 | 111 | return "" | |
| 112 | 112 | } | |
| 113 | 113 | host = h | |
| 114 | + | } else if strings.HasPrefix(host, "[") && strings.HasSuffix(host, "]") { | |
| 115 | + | inner := strings.TrimSuffix(strings.TrimPrefix(host, "["), "]") | |
| 116 | + | if ip := net.ParseIP(inner); ip == nil || ip.To4() != nil { | |
| 117 | + | return "" | |
| 118 | + | } | |
| 119 | + | host = inner | |
| 114 | 120 | } else if strings.Contains(host, ":") { | |
| 115 | 121 | return "" | |
| 116 | 122 | } | |
| @@ -111,6 +111,12 @@ func normalizeRequestHost(host string) string { | |||||
| 111 | return "" | 111 | return "" | ||
| 112 | } | 112 | } | ||
| 113 | host = h | 113 | host = h | ||
| 114 | + | } else if strings.HasPrefix(host, "[") && strings.HasSuffix(host, "]") { | |||
| 115 | + | inner := strings.TrimSuffix(strings.TrimPrefix(host, "["), "]") | |||
| 116 | + | if ip := net.ParseIP(inner); ip == nil || ip.To4() != nil { | |||
| 117 | + | return "" | |||
| 118 | + | } | |||
| 119 | + | host = inner | |||
| 114 | } else if strings.Contains(host, ":") { | 120 | } else if strings.Contains(host, ":") { | ||
| 115 | return "" | 121 | return "" | ||
| 116 | } | 122 | } | ||
M internal/adapters/in/http/proxy/handler_test.go +17 -10
| @@ -11,6 +11,7 @@ import ( | |||
| 11 | 11 | "github.com/bnema/zerowrap" | |
| 12 | 12 | "github.com/stretchr/testify/assert" | |
| 13 | 13 | "github.com/stretchr/testify/mock" | |
| 14 | + | "github.com/stretchr/testify/require" | |
| 14 | 15 | ||
| 15 | 16 | "github.com/bnema/gordon/internal/boundaries/in" | |
| 16 | 17 | inmocks "github.com/bnema/gordon/internal/boundaries/in/mocks" | |
| @@ -80,13 +81,16 @@ func TestHandler_NormalizesRequestHostForLookup(t *testing.T) { | |||
| 80 | 81 | proxySvc.EXPECT().GetTarget(mock.Anything, tt.wantHost).Return(nil, domain.ErrNoTargetAvailable) | |
| 81 | 82 | ||
| 82 | 83 | handler := NewHandler(proxySvc, nil, testLogger()) | |
| 83 | - | req := httptest.NewRequest(http.MethodGet, "http://example.test/", nil) | |
| 84 | + | srv := httptest.NewServer(handler) | |
| 85 | + | defer srv.Close() | |
| 86 | + | req, err := http.NewRequest(http.MethodGet, srv.URL+"/", nil) | |
| 87 | + | require.NoError(t, err) | |
| 84 | 88 | req.Host = tt.host | |
| 85 | - | w := httptest.NewRecorder() | |
| 89 | + | resp, err := srv.Client().Do(req) | |
| 90 | + | require.NoError(t, err) | |
| 91 | + | defer resp.Body.Close() | |
| 86 | 92 | ||
| 87 | - | handler.ServeHTTP(w, req) | |
| 88 | - | ||
| 89 | - | assert.Equal(t, http.StatusNotFound, w.Code) | |
| 93 | + | assert.Equal(t, http.StatusNotFound, resp.StatusCode) | |
| 90 | 94 | }) | |
| 91 | 95 | } | |
| 92 | 96 | } | |
| @@ -96,14 +100,17 @@ func TestHandler_RejectsInvalidRequestHost(t *testing.T) { | |||
| 96 | 100 | proxySvc.EXPECT().ProxyConfig().Return(in.ProxyServiceConfig{}) | |
| 97 | 101 | ||
| 98 | 102 | handler := NewHandler(proxySvc, nil, testLogger()) | |
| 103 | + | srv := httptest.NewServer(handler) | |
| 104 | + | defer srv.Close() | |
| 99 | 105 | for _, host := range []string{"app.example.com:bad", "app.example.com:0", "app.example.com:99999"} { | |
| 100 | - | req := httptest.NewRequest(http.MethodGet, "http://example.test/", nil) | |
| 106 | + | req, err := http.NewRequest(http.MethodGet, srv.URL+"/", nil) | |
| 107 | + | require.NoError(t, err) | |
| 101 | 108 | req.Host = host | |
| 102 | - | w := httptest.NewRecorder() | |
| 103 | - | ||
| 104 | - | handler.ServeHTTP(w, req) | |
| 109 | + | resp, err := srv.Client().Do(req) | |
| 110 | + | require.NoError(t, err) | |
| 111 | + | defer resp.Body.Close() | |
| 105 | 112 | ||
| 106 | - | assert.Equal(t, http.StatusBadRequest, w.Code) | |
| 113 | + | assert.Equal(t, http.StatusBadRequest, resp.StatusCode) | |
| 107 | 114 | } | |
| 108 | 115 | } | |
| 109 | 116 | ||
| @@ -11,6 +11,7 @@ import ( | |||||
| 11 | "github.com/bnema/zerowrap" | 11 | "github.com/bnema/zerowrap" | ||
| 12 | "github.com/stretchr/testify/assert" | 12 | "github.com/stretchr/testify/assert" | ||
| 13 | "github.com/stretchr/testify/mock" | 13 | "github.com/stretchr/testify/mock" | ||
| 14 | + | "github.com/stretchr/testify/require" | |||
| 14 | 15 | ||||
| 15 | "github.com/bnema/gordon/internal/boundaries/in" | 16 | "github.com/bnema/gordon/internal/boundaries/in" | ||
| 16 | inmocks "github.com/bnema/gordon/internal/boundaries/in/mocks" | 17 | inmocks "github.com/bnema/gordon/internal/boundaries/in/mocks" | ||
| @@ -80,13 +81,16 @@ func TestHandler_NormalizesRequestHostForLookup(t *testing.T) { | |||||
| 80 | proxySvc.EXPECT().GetTarget(mock.Anything, tt.wantHost).Return(nil, domain.ErrNoTargetAvailable) | 81 | proxySvc.EXPECT().GetTarget(mock.Anything, tt.wantHost).Return(nil, domain.ErrNoTargetAvailable) | ||
| 81 | 82 | ||||
| 82 | handler := NewHandler(proxySvc, nil, testLogger()) | 83 | handler := NewHandler(proxySvc, nil, testLogger()) | ||
| 83 | - | req := httptest.NewRequest(http.MethodGet, "http://example.test/", nil) | 84 | + | srv := httptest.NewServer(handler) |
| 85 | + | defer srv.Close() | |||
| 86 | + | req, err := http.NewRequest(http.MethodGet, srv.URL+"/", nil) | |||
| 87 | + | require.NoError(t, err) | |||
| 84 | req.Host = tt.host | 88 | req.Host = tt.host | ||
| 85 | - | w := httptest.NewRecorder() | 89 | + | resp, err := srv.Client().Do(req) |
| 90 | + | require.NoError(t, err) | |||
| 91 | + | defer resp.Body.Close() | |||
| 86 | 92 | ||||
| 87 | - | handler.ServeHTTP(w, req) | 93 | + | assert.Equal(t, http.StatusNotFound, resp.StatusCode) |
| 88 | - | ||||
| 89 | - | assert.Equal(t, http.StatusNotFound, w.Code) | |||
| 90 | }) | 94 | }) | ||
| 91 | } | 95 | } | ||
| 92 | } | 96 | } | ||
| @@ -96,14 +100,17 @@ func TestHandler_RejectsInvalidRequestHost(t *testing.T) { | |||||
| 96 | proxySvc.EXPECT().ProxyConfig().Return(in.ProxyServiceConfig{}) | 100 | proxySvc.EXPECT().ProxyConfig().Return(in.ProxyServiceConfig{}) | ||
| 97 | 101 | ||||
| 98 | handler := NewHandler(proxySvc, nil, testLogger()) | 102 | handler := NewHandler(proxySvc, nil, testLogger()) | ||
| 103 | + | srv := httptest.NewServer(handler) | |||
| 104 | + | defer srv.Close() | |||
| 99 | for _, host := range []string{"app.example.com:bad", "app.example.com:0", "app.example.com:99999"} { | 105 | for _, host := range []string{"app.example.com:bad", "app.example.com:0", "app.example.com:99999"} { | ||
| 100 | - | req := httptest.NewRequest(http.MethodGet, "http://example.test/", nil) | 106 | + | req, err := http.NewRequest(http.MethodGet, srv.URL+"/", nil) |
| 107 | + | require.NoError(t, err) | |||
| 101 | req.Host = host | 108 | req.Host = host | ||
| 102 | - | w := httptest.NewRecorder() | 109 | + | resp, err := srv.Client().Do(req) |
| 103 | - | 110 | + | require.NoError(t, err) | |
| 104 | - | handler.ServeHTTP(w, req) | 111 | + | defer resp.Body.Close() |
| 105 | 112 | ||||
| 106 | - | assert.Equal(t, http.StatusBadRequest, w.Code) | 113 | + | assert.Equal(t, http.StatusBadRequest, resp.StatusCode) |
| 107 | } | 114 | } | ||
| 108 | } | 115 | } | ||
| 109 | 116 | ||||
M internal/adapters/in/http/registry/handler.go +47 -14
| @@ -2,6 +2,7 @@ | |||
| 2 | 2 | package registry | |
| 3 | 3 | ||
| 4 | 4 | import ( | |
| 5 | + | "context" | |
| 5 | 6 | "encoding/json" | |
| 6 | 7 | "errors" | |
| 7 | 8 | "fmt" | |
| @@ -9,6 +10,7 @@ import ( | |||
| 9 | 10 | "net/http" | |
| 10 | 11 | "strconv" | |
| 11 | 12 | "strings" | |
| 13 | + | "sync/atomic" | |
| 12 | 14 | ||
| 13 | 15 | "github.com/bnema/zerowrap" | |
| 14 | 16 | ||
| @@ -34,8 +36,8 @@ const ( | |||
| 34 | 36 | type Handler struct { | |
| 35 | 37 | registrySvc in.RegistryService | |
| 36 | 38 | log zerowrap.Logger | |
| 37 | - | maxBlobChunkSize int64 | |
| 38 | - | maxBlobSize int64 | |
| 39 | + | maxBlobChunkSize atomic.Int64 | |
| 40 | + | maxBlobSize atomic.Int64 | |
| 39 | 41 | } | |
| 40 | 42 | ||
| 41 | 43 | // NewHandler creates a new registry HTTP handler. | |
| @@ -53,12 +55,24 @@ func NewHandler( | |||
| 53 | 55 | blobSizeLimit = maxBlobSize[0] | |
| 54 | 56 | } | |
| 55 | 57 | ||
| 56 | - | return &Handler{ | |
| 57 | - | registrySvc: registrySvc, | |
| 58 | - | log: log, | |
| 59 | - | maxBlobChunkSize: maxBlobChunkSize, | |
| 60 | - | maxBlobSize: blobSizeLimit, | |
| 58 | + | h := &Handler{ | |
| 59 | + | registrySvc: registrySvc, | |
| 60 | + | log: log, | |
| 61 | 61 | } | |
| 62 | + | h.UpdateBlobLimits(maxBlobChunkSize, blobSizeLimit) | |
| 63 | + | return h | |
| 64 | + | } | |
| 65 | + | ||
| 66 | + | // UpdateBlobLimits updates request-size limits used by live upload handlers. | |
| 67 | + | func (h *Handler) UpdateBlobLimits(maxBlobChunkSize, maxBlobSize int64) { | |
| 68 | + | if maxBlobChunkSize <= 0 { | |
| 69 | + | maxBlobChunkSize = DefaultMaxBlobChunkSize | |
| 70 | + | } | |
| 71 | + | if maxBlobSize <= 0 { | |
| 72 | + | maxBlobSize = DefaultMaxBlobSize | |
| 73 | + | } | |
| 74 | + | h.maxBlobChunkSize.Store(maxBlobChunkSize) | |
| 75 | + | h.maxBlobSize.Store(maxBlobSize) | |
| 62 | 76 | } | |
| 63 | 77 | ||
| 64 | 78 | // RegisterRoutes registers the registry routes on the given mux. | |
| @@ -416,24 +430,31 @@ func (h *Handler) handleBlobUpload(w http.ResponseWriter, r *http.Request) { | |||
| 416 | 430 | } | |
| 417 | 431 | } | |
| 418 | 432 | ||
| 433 | + | maxBlobChunkSize := h.maxBlobChunkSize.Load() | |
| 434 | + | maxBlobSize := h.maxBlobSize.Load() | |
| 435 | + | ||
| 419 | 436 | // Limit blob chunk size to prevent excessive uploads. | |
| 420 | 437 | // MaxBytesReader wraps the body so the downstream io.Copy stops at the limit. | |
| 421 | - | r.Body = http.MaxBytesReader(w, r.Body, h.maxBlobChunkSize) | |
| 438 | + | r.Body = http.MaxBytesReader(w, r.Body, maxBlobChunkSize) | |
| 422 | 439 | ||
| 423 | 440 | // Stream the body directly to storage — memory usage is bounded to a small | |
| 424 | 441 | // copy buffer (~32KB) regardless of chunk size. | |
| 425 | - | length, err := h.registrySvc.AppendBlobChunk(ctx, name, uuid, r.Body, r.ContentLength, h.maxBlobSize) | |
| 442 | + | length, err := h.registrySvc.AppendBlobChunk(ctx, name, uuid, r.Body, r.ContentLength, maxBlobSize) | |
| 426 | 443 | if err != nil { | |
| 427 | 444 | var maxBytesErr *http.MaxBytesError | |
| 428 | 445 | if errors.As(err, &maxBytesErr) { | |
| 429 | - | log.Warn().Int64("max_size", h.maxBlobChunkSize).Msg("blob chunk too large") | |
| 430 | - | _ = h.registrySvc.CancelUpload(ctx, uuid) | |
| 446 | + | log.Warn().Int64("max_size", maxBlobChunkSize).Msg("blob chunk too large") | |
| 447 | + | if !h.cancelUploadAfterError(w, ctx, uuid, "failed to cancel oversized blob upload") { | |
| 448 | + | return | |
| 449 | + | } | |
| 431 | 450 | h.sendRegistryError(w, http.StatusRequestEntityTooLarge, "SIZE_INVALID", "blob chunk exceeds maximum size") | |
| 432 | 451 | return | |
| 433 | 452 | } | |
| 434 | 453 | if errors.Is(err, domain.ErrBlobSizeExceeded) { | |
| 435 | - | log.Warn().Int64("max_size", h.maxBlobSize).Msg("blob upload too large") | |
| 436 | - | _ = h.registrySvc.CancelUpload(ctx, uuid) | |
| 454 | + | log.Warn().Int64("max_size", maxBlobSize).Msg("blob upload too large") | |
| 455 | + | if !h.cancelUploadAfterError(w, ctx, uuid, "failed to cancel oversized blob upload") { | |
| 456 | + | return | |
| 457 | + | } | |
| 437 | 458 | h.sendRegistryError(w, http.StatusRequestEntityTooLarge, "SIZE_INVALID", "blob exceeds maximum size") | |
| 438 | 459 | return | |
| 439 | 460 | } | |
| @@ -446,7 +467,9 @@ func (h *Handler) handleBlobUpload(w http.ResponseWriter, r *http.Request) { | |||
| 446 | 467 | if r.Method == "PUT" && digest != "" { | |
| 447 | 468 | if err := h.registrySvc.FinishUpload(ctx, uuid, digest); err != nil { | |
| 448 | 469 | log.Error().Err(err).Str("digest", digest).Msg("failed to finalize blob upload") | |
| 449 | - | _ = h.registrySvc.CancelUpload(ctx, uuid) | |
| 470 | + | if !h.cancelUploadAfterError(w, ctx, uuid, "failed to cancel invalid blob upload") { | |
| 471 | + | return | |
| 472 | + | } | |
| 450 | 473 | h.sendRegistryError(w, http.StatusBadRequest, "DIGEST_INVALID", "digest mismatch") | |
| 451 | 474 | return | |
| 452 | 475 | } | |
| @@ -466,6 +489,16 @@ func (h *Handler) handleBlobUpload(w http.ResponseWriter, r *http.Request) { | |||
| 466 | 489 | w.WriteHeader(http.StatusAccepted) | |
| 467 | 490 | } | |
| 468 | 491 | ||
| 492 | + | func (h *Handler) cancelUploadAfterError(w http.ResponseWriter, ctx context.Context, uuid, message string) bool { | |
| 493 | + | if err := h.registrySvc.CancelUpload(ctx, uuid); err != nil { | |
| 494 | + | log := zerowrap.FromCtx(ctx) | |
| 495 | + | log.Error().Err(err).Str("uuid", uuid).Msg(message) | |
| 496 | + | h.sendRegistryError(w, http.StatusInternalServerError, "UNKNOWN", "failed to cancel upload") | |
| 497 | + | return false | |
| 498 | + | } | |
| 499 | + | return true | |
| 500 | + | } | |
| 501 | + | ||
| 469 | 502 | func (h *Handler) handleListTags(w http.ResponseWriter, r *http.Request) { | |
| 470 | 503 | ctx := r.Context() | |
| 471 | 504 | log := zerowrap.FromCtx(ctx) | |
| @@ -2,6 +2,7 @@ | |||||
| 2 | package registry | 2 | package registry | ||
| 3 | 3 | ||||
| 4 | import ( | 4 | import ( | ||
| 5 | + | "context" | |||
| 5 | "encoding/json" | 6 | "encoding/json" | ||
| 6 | "errors" | 7 | "errors" | ||
| 7 | "fmt" | 8 | "fmt" | ||
| @@ -9,6 +10,7 @@ import ( | |||||
| 9 | "net/http" | 10 | "net/http" | ||
| 10 | "strconv" | 11 | "strconv" | ||
| 11 | "strings" | 12 | "strings" | ||
| 13 | + | "sync/atomic" | |||
| 12 | 14 | ||||
| 13 | "github.com/bnema/zerowrap" | 15 | "github.com/bnema/zerowrap" | ||
| 14 | 16 | ||||
| @@ -34,8 +36,8 @@ const ( | |||||
| 34 | type Handler struct { | 36 | type Handler struct { | ||
| 35 | registrySvc in.RegistryService | 37 | registrySvc in.RegistryService | ||
| 36 | log zerowrap.Logger | 38 | log zerowrap.Logger | ||
| 37 | - | maxBlobChunkSize int64 | 39 | + | maxBlobChunkSize atomic.Int64 |
| 38 | - | maxBlobSize int64 | 40 | + | maxBlobSize atomic.Int64 |
| 39 | } | 41 | } | ||
| 40 | 42 | ||||
| 41 | // NewHandler creates a new registry HTTP handler. | 43 | // NewHandler creates a new registry HTTP handler. | ||
| @@ -53,12 +55,24 @@ func NewHandler( | |||||
| 53 | blobSizeLimit = maxBlobSize[0] | 55 | blobSizeLimit = maxBlobSize[0] | ||
| 54 | } | 56 | } | ||
| 55 | 57 | ||||
| 56 | - | return &Handler{ | 58 | + | h := &Handler{ |
| 57 | - | registrySvc: registrySvc, | 59 | + | registrySvc: registrySvc, |
| 58 | - | log: log, | 60 | + | log: log, |
| 59 | - | maxBlobChunkSize: maxBlobChunkSize, | |||
| 60 | - | maxBlobSize: blobSizeLimit, | |||
| 61 | } | 61 | } | ||
| 62 | + | h.UpdateBlobLimits(maxBlobChunkSize, blobSizeLimit) | |||
| 63 | + | return h | |||
| 64 | + | } | |||
| 65 | + | ||||
| 66 | + | // UpdateBlobLimits updates request-size limits used by live upload handlers. | |||
| 67 | + | func (h *Handler) UpdateBlobLimits(maxBlobChunkSize, maxBlobSize int64) { | |||
| 68 | + | if maxBlobChunkSize <= 0 { | |||
| 69 | + | maxBlobChunkSize = DefaultMaxBlobChunkSize | |||
| 70 | + | } | |||
| 71 | + | if maxBlobSize <= 0 { | |||
| 72 | + | maxBlobSize = DefaultMaxBlobSize | |||
| 73 | + | } | |||
| 74 | + | h.maxBlobChunkSize.Store(maxBlobChunkSize) | |||
| 75 | + | h.maxBlobSize.Store(maxBlobSize) | |||
| 62 | } | 76 | } | ||
| 63 | 77 | ||||
| 64 | // RegisterRoutes registers the registry routes on the given mux. | 78 | // RegisterRoutes registers the registry routes on the given mux. | ||
| @@ -416,24 +430,31 @@ func (h *Handler) handleBlobUpload(w http.ResponseWriter, r *http.Request) { | |||||
| 416 | } | 430 | } | ||
| 417 | } | 431 | } | ||
| 418 | 432 | ||||
| 433 | + | maxBlobChunkSize := h.maxBlobChunkSize.Load() | |||
| 434 | + | maxBlobSize := h.maxBlobSize.Load() | |||
| 435 | + | ||||
| 419 | // Limit blob chunk size to prevent excessive uploads. | 436 | // Limit blob chunk size to prevent excessive uploads. | ||
| 420 | // MaxBytesReader wraps the body so the downstream io.Copy stops at the limit. | 437 | // MaxBytesReader wraps the body so the downstream io.Copy stops at the limit. | ||
| 421 | - | r.Body = http.MaxBytesReader(w, r.Body, h.maxBlobChunkSize) | 438 | + | r.Body = http.MaxBytesReader(w, r.Body, maxBlobChunkSize) |
| 422 | 439 | ||||
| 423 | // Stream the body directly to storage — memory usage is bounded to a small | 440 | // Stream the body directly to storage — memory usage is bounded to a small | ||
| 424 | // copy buffer (~32KB) regardless of chunk size. | 441 | // copy buffer (~32KB) regardless of chunk size. | ||
| 425 | - | length, err := h.registrySvc.AppendBlobChunk(ctx, name, uuid, r.Body, r.ContentLength, h.maxBlobSize) | 442 | + | length, err := h.registrySvc.AppendBlobChunk(ctx, name, uuid, r.Body, r.ContentLength, maxBlobSize) |
| 426 | if err != nil { | 443 | if err != nil { | ||
| 427 | var maxBytesErr *http.MaxBytesError | 444 | var maxBytesErr *http.MaxBytesError | ||
| 428 | if errors.As(err, &maxBytesErr) { | 445 | if errors.As(err, &maxBytesErr) { | ||
| 429 | - | log.Warn().Int64("max_size", h.maxBlobChunkSize).Msg("blob chunk too large") | 446 | + | log.Warn().Int64("max_size", maxBlobChunkSize).Msg("blob chunk too large") |
| 430 | - | _ = h.registrySvc.CancelUpload(ctx, uuid) | 447 | + | if !h.cancelUploadAfterError(w, ctx, uuid, "failed to cancel oversized blob upload") { |
| 448 | + | return | |||
| 449 | + | } | |||
| 431 | h.sendRegistryError(w, http.StatusRequestEntityTooLarge, "SIZE_INVALID", "blob chunk exceeds maximum size") | 450 | h.sendRegistryError(w, http.StatusRequestEntityTooLarge, "SIZE_INVALID", "blob chunk exceeds maximum size") | ||
| 432 | return | 451 | return | ||
| 433 | } | 452 | } | ||
| 434 | if errors.Is(err, domain.ErrBlobSizeExceeded) { | 453 | if errors.Is(err, domain.ErrBlobSizeExceeded) { | ||
| 435 | - | log.Warn().Int64("max_size", h.maxBlobSize).Msg("blob upload too large") | 454 | + | log.Warn().Int64("max_size", maxBlobSize).Msg("blob upload too large") |
| 436 | - | _ = h.registrySvc.CancelUpload(ctx, uuid) | 455 | + | if !h.cancelUploadAfterError(w, ctx, uuid, "failed to cancel oversized blob upload") { |
| 456 | + | return | |||
| 457 | + | } | |||
| 437 | h.sendRegistryError(w, http.StatusRequestEntityTooLarge, "SIZE_INVALID", "blob exceeds maximum size") | 458 | h.sendRegistryError(w, http.StatusRequestEntityTooLarge, "SIZE_INVALID", "blob exceeds maximum size") | ||
| 438 | return | 459 | return | ||
| 439 | } | 460 | } | ||
| @@ -446,7 +467,9 @@ func (h *Handler) handleBlobUpload(w http.ResponseWriter, r *http.Request) { | |||||
| 446 | if r.Method == "PUT" && digest != "" { | 467 | if r.Method == "PUT" && digest != "" { | ||
| 447 | if err := h.registrySvc.FinishUpload(ctx, uuid, digest); err != nil { | 468 | if err := h.registrySvc.FinishUpload(ctx, uuid, digest); err != nil { | ||
| 448 | log.Error().Err(err).Str("digest", digest).Msg("failed to finalize blob upload") | 469 | log.Error().Err(err).Str("digest", digest).Msg("failed to finalize blob upload") | ||
| 449 | - | _ = h.registrySvc.CancelUpload(ctx, uuid) | 470 | + | if !h.cancelUploadAfterError(w, ctx, uuid, "failed to cancel invalid blob upload") { |
| 471 | + | return | |||
| 472 | + | } | |||
| 450 | h.sendRegistryError(w, http.StatusBadRequest, "DIGEST_INVALID", "digest mismatch") | 473 | h.sendRegistryError(w, http.StatusBadRequest, "DIGEST_INVALID", "digest mismatch") | ||
| 451 | return | 474 | return | ||
| 452 | } | 475 | } | ||
| @@ -466,6 +489,16 @@ func (h *Handler) handleBlobUpload(w http.ResponseWriter, r *http.Request) { | |||||
| 466 | w.WriteHeader(http.StatusAccepted) | 489 | w.WriteHeader(http.StatusAccepted) | ||
| 467 | } | 490 | } | ||
| 468 | 491 | ||||
| 492 | + | func (h *Handler) cancelUploadAfterError(w http.ResponseWriter, ctx context.Context, uuid, message string) bool { | |||
| 493 | + | if err := h.registrySvc.CancelUpload(ctx, uuid); err != nil { | |||
| 494 | + | log := zerowrap.FromCtx(ctx) | |||
| 495 | + | log.Error().Err(err).Str("uuid", uuid).Msg(message) | |||
| 496 | + | h.sendRegistryError(w, http.StatusInternalServerError, "UNKNOWN", "failed to cancel upload") | |||
| 497 | + | return false | |||
| 498 | + | } | |||
| 499 | + | return true | |||
| 500 | + | } | |||
| 501 | + | ||||
| 469 | func (h *Handler) handleListTags(w http.ResponseWriter, r *http.Request) { | 502 | func (h *Handler) handleListTags(w http.ResponseWriter, r *http.Request) { | ||
| 470 | ctx := r.Context() | 503 | ctx := r.Context() | ||
| 471 | log := zerowrap.FromCtx(ctx) | 504 | log := zerowrap.FromCtx(ctx) | ||
M internal/adapters/out/docker/runtime.go +2 -1
| @@ -1269,10 +1269,11 @@ func (r *Runtime) CreateNetwork(ctx context.Context, name string, config domain. | |||
| 1269 | 1269 | if driver == "" { | |
| 1270 | 1270 | driver = "bridge" | |
| 1271 | 1271 | } | |
| 1272 | - | labels := map[string]string{domain.LabelManaged: "true"} | |
| 1272 | + | labels := make(map[string]string, len(config.Labels)+1) | |
| 1273 | 1273 | for key, value := range config.Labels { | |
| 1274 | 1274 | labels[key] = value | |
| 1275 | 1275 | } | |
| 1276 | + | labels[domain.LabelManaged] = "true" | |
| 1276 | 1277 | ||
| 1277 | 1278 | createOptions := network.CreateOptions{ | |
| 1278 | 1279 | Driver: driver, | |
| @@ -1269,10 +1269,11 @@ func (r *Runtime) CreateNetwork(ctx context.Context, name string, config domain. | |||||
| 1269 | if driver == "" { | 1269 | if driver == "" { | ||
| 1270 | driver = "bridge" | 1270 | driver = "bridge" | ||
| 1271 | } | 1271 | } | ||
| 1272 | - | labels := map[string]string{domain.LabelManaged: "true"} | 1272 | + | labels := make(map[string]string, len(config.Labels)+1) |
| 1273 | for key, value := range config.Labels { | 1273 | for key, value := range config.Labels { | ||
| 1274 | labels[key] = value | 1274 | labels[key] = value | ||
| 1275 | } | 1275 | } | ||
| 1276 | + | labels[domain.LabelManaged] = "true" | |||
| 1276 | 1277 | ||||
| 1277 | createOptions := network.CreateOptions{ | 1278 | createOptions := network.CreateOptions{ | ||
| 1278 | Driver: driver, | 1279 | Driver: driver, | ||
M internal/app/migrate_env.go +3 -5
| @@ -167,18 +167,16 @@ func migrateAttachmentEnvFile(envDir, name string, passStore *domainsecrets.Pass | |||
| 167 | 167 | if err := passStore.SetAttachment(containerName, secrets); err != nil { | |
| 168 | 168 | return fmt.Errorf("failed to migrate attachment secrets for %s: %w", containerName, err) | |
| 169 | 169 | } | |
| 170 | - | log.Info(). | |
| 171 | - | Int(zerowrap.FieldCount, len(secrets)). | |
| 172 | - | Str("container", containerName). | |
| 173 | - | Msg("migrated secrets for attachment container from plain text to pass") | |
| 174 | 170 | ||
| 175 | 171 | if err := os.Remove(filePath); err != nil { | |
| 176 | 172 | return fmt.Errorf("failed to remove migrated attachment env file %s: %w", filePath, err) | |
| 177 | 173 | } | |
| 178 | 174 | ||
| 179 | 175 | log.Info(). | |
| 176 | + | Int(zerowrap.FieldCount, len(secrets)). | |
| 177 | + | Str("container", containerName). | |
| 180 | 178 | Str("file", filePath). | |
| 181 | - | Msg("removed original attachment env file after migration to pass") | |
| 179 | + | Msg("migrated attachment secrets to pass and removed original env file") | |
| 182 | 180 | ||
| 183 | 181 | return nil | |
| 184 | 182 | } | |
| @@ -167,18 +167,16 @@ func migrateAttachmentEnvFile(envDir, name string, passStore *domainsecrets.Pass | |||||
| 167 | if err := passStore.SetAttachment(containerName, secrets); err != nil { | 167 | if err := passStore.SetAttachment(containerName, secrets); err != nil { | ||
| 168 | return fmt.Errorf("failed to migrate attachment secrets for %s: %w", containerName, err) | 168 | return fmt.Errorf("failed to migrate attachment secrets for %s: %w", containerName, err) | ||
| 169 | } | 169 | } | ||
| 170 | - | log.Info(). | |||
| 171 | - | Int(zerowrap.FieldCount, len(secrets)). | |||
| 172 | - | Str("container", containerName). | |||
| 173 | - | Msg("migrated secrets for attachment container from plain text to pass") | |||
| 174 | 170 | ||||
| 175 | if err := os.Remove(filePath); err != nil { | 171 | if err := os.Remove(filePath); err != nil { | ||
| 176 | return fmt.Errorf("failed to remove migrated attachment env file %s: %w", filePath, err) | 172 | return fmt.Errorf("failed to remove migrated attachment env file %s: %w", filePath, err) | ||
| 177 | } | 173 | } | ||
| 178 | 174 | ||||
| 179 | log.Info(). | 175 | log.Info(). | ||
| 176 | + | Int(zerowrap.FieldCount, len(secrets)). | |||
| 177 | + | Str("container", containerName). | |||
| 180 | Str("file", filePath). | 178 | Str("file", filePath). | ||
| 181 | - | Msg("removed original attachment env file after migration to pass") | 179 | + | Msg("migrated attachment secrets to pass and removed original env file") |
| 182 | 180 | ||||
| 183 | return nil | 181 | return nil | ||
| 184 | } | 182 | } | ||
M internal/app/run.go +45 -14
| @@ -224,6 +224,9 @@ type services struct { | |||
| 224 | 224 | caAdapter *pkiadapter.CA | |
| 225 | 225 | pkiSvc *pkiusecase.Service | |
| 226 | 226 | reloadCoordinator *reloadCoordinator | |
| 227 | + | registryHandler interface { | |
| 228 | + | UpdateBlobLimits(maxBlobChunkSize, maxBlobSize int64) | |
| 229 | + | } | |
| 227 | 230 | } | |
| 228 | 231 | ||
| 229 | 232 | // Run initializes and starts the Gordon application. | |
| @@ -466,7 +469,7 @@ func createServices(ctx context.Context, v *viper.Viper, cfg Config, log zerowra | |||
| 466 | 469 | return nil, err | |
| 467 | 470 | } | |
| 468 | 471 | ||
| 469 | - | si.svc.reloadCoordinator = newReloadCoordinator(v, si.svc.configSvc, si.svc.proxySvc, si.svc.eventBus, log) | |
| 472 | + | si.svc.reloadCoordinator = newReloadCoordinator(v, si.svc.configSvc, si.svc.proxySvc, nil, si.svc.eventBus, log) | |
| 470 | 473 | ||
| 471 | 474 | si.initHandlers() | |
| 472 | 475 | ||
| @@ -1369,11 +1372,18 @@ func readFileBeneath(root, cleanedRelPath string) ([]byte, error) { | |||
| 1369 | 1372 | } | |
| 1370 | 1373 | last := i == len(parts)-1 | |
| 1371 | 1374 | if last { | |
| 1372 | - | fd, err := unix.Openat(dirFD, part, unix.O_RDONLY|unix.O_CLOEXEC|unix.O_NOFOLLOW, 0) | |
| 1375 | + | fd, err := unix.Openat(dirFD, part, unix.O_RDONLY|unix.O_NONBLOCK|unix.O_CLOEXEC|unix.O_NOFOLLOW, 0) | |
| 1373 | 1376 | if err != nil { | |
| 1374 | 1377 | return nil, fmt.Errorf("failed to read secret file: %w", err) | |
| 1375 | 1378 | } | |
| 1376 | 1379 | defer unix.Close(fd) | |
| 1380 | + | var st unix.Stat_t | |
| 1381 | + | if err := unix.Fstat(fd, &st); err != nil { | |
| 1382 | + | return nil, fmt.Errorf("failed to stat secret file: %w", err) | |
| 1383 | + | } | |
| 1384 | + | if st.Mode&unix.S_IFMT != unix.S_IFREG { | |
| 1385 | + | return nil, fmt.Errorf("invalid secret path: secret must be a regular file") | |
| 1386 | + | } | |
| 1377 | 1387 | return os.ReadFile(fmt.Sprintf("/proc/self/fd/%d", fd)) | |
| 1378 | 1388 | } | |
| 1379 | 1389 | ||
| @@ -1437,24 +1447,38 @@ type reloadCoordinator struct { | |||
| 1437 | 1447 | lastRun time.Time | |
| 1438 | 1448 | debounce time.Duration | |
| 1439 | 1449 | ||
| 1440 | - | configSvc configReloader | |
| 1441 | - | v *viper.Viper | |
| 1442 | - | proxySvc proxyConfigUpdater | |
| 1443 | - | eventBus out.EventPublisher | |
| 1444 | - | log zerowrap.Logger | |
| 1450 | + | configSvc configReloader | |
| 1451 | + | v *viper.Viper | |
| 1452 | + | proxySvc proxyConfigUpdater | |
| 1453 | + | registryLimits interface { | |
| 1454 | + | UpdateBlobLimits(maxBlobChunkSize, maxBlobSize int64) | |
| 1455 | + | } | |
| 1456 | + | eventBus out.EventPublisher | |
| 1457 | + | log zerowrap.Logger | |
| 1445 | 1458 | } | |
| 1446 | 1459 | ||
| 1447 | - | func newReloadCoordinator(v *viper.Viper, configSvc configReloader, proxySvc proxyConfigUpdater, eventBus out.EventPublisher, log zerowrap.Logger) *reloadCoordinator { | |
| 1460 | + | func newReloadCoordinator(v *viper.Viper, configSvc configReloader, proxySvc proxyConfigUpdater, registryLimits interface { | |
| 1461 | + | UpdateBlobLimits(maxBlobChunkSize, maxBlobSize int64) | |
| 1462 | + | }, eventBus out.EventPublisher, log zerowrap.Logger) *reloadCoordinator { | |
| 1448 | 1463 | return &reloadCoordinator{ | |
| 1449 | - | debounce: 500 * time.Millisecond, | |
| 1450 | - | configSvc: configSvc, | |
| 1451 | - | v: v, | |
| 1452 | - | proxySvc: proxySvc, | |
| 1453 | - | eventBus: eventBus, | |
| 1454 | - | log: log, | |
| 1464 | + | debounce: 500 * time.Millisecond, | |
| 1465 | + | configSvc: configSvc, | |
| 1466 | + | v: v, | |
| 1467 | + | proxySvc: proxySvc, | |
| 1468 | + | registryLimits: registryLimits, | |
| 1469 | + | eventBus: eventBus, | |
| 1470 | + | log: log, | |
| 1455 | 1471 | } | |
| 1456 | 1472 | } | |
| 1457 | 1473 | ||
| 1474 | + | func (c *reloadCoordinator) SetRegistryLimits(limits interface { | |
| 1475 | + | UpdateBlobLimits(maxBlobChunkSize, maxBlobSize int64) | |
| 1476 | + | }) { | |
| 1477 | + | c.mu.Lock() | |
| 1478 | + | defer c.mu.Unlock() | |
| 1479 | + | c.registryLimits = limits | |
| 1480 | + | } | |
| 1481 | + | ||
| 1458 | 1482 | func (c *reloadCoordinator) Trigger(ctx context.Context) error { | |
| 1459 | 1483 | c.mu.Lock() | |
| 1460 | 1484 | defer c.mu.Unlock() | |
| @@ -1504,6 +1528,9 @@ func (c *reloadCoordinator) applyLoadedConfig(now time.Time) error { | |||
| 1504 | 1528 | } | |
| 1505 | 1529 | ||
| 1506 | 1530 | c.proxySvc.UpdateConfig(reloadedProxy.proxyConfig) | |
| 1531 | + | if c.registryLimits != nil { | |
| 1532 | + | c.registryLimits.UpdateBlobLimits(reloadedProxy.maxBlobChunkSize, reloadedProxy.maxBlobSize) | |
| 1533 | + | } | |
| 1507 | 1534 | ||
| 1508 | 1535 | if c.eventBus != nil { | |
| 1509 | 1536 | if err := c.eventBus.Publish(domain.EventConfigReload, nil); err != nil { | |
| @@ -1835,6 +1862,10 @@ func createHTTPHandlers(svc *services, cfg Config, log zerowrap.Logger, accessWr | |||
| 1835 | 1862 | ||
| 1836 | 1863 | // Registry handler | |
| 1837 | 1864 | registryHandler := registry.NewHandler(svc.registrySvc, log, svc.maxBlobChunkSize, svc.maxBlobSize) | |
| 1865 | + | svc.registryHandler = registryHandler | |
| 1866 | + | if svc.reloadCoordinator != nil { | |
| 1867 | + | svc.reloadCoordinator.SetRegistryLimits(registryHandler) | |
| 1868 | + | } | |
| 1838 | 1869 | registryWithMiddleware, cidrAllowlistMiddleware, rateLimitMiddleware := buildRegistryHandlerWithMiddleware( | |
| 1839 | 1870 | svc, | |
| 1840 | 1871 | cfg, | |
| @@ -224,6 +224,9 @@ type services struct { | |||||
| 224 | caAdapter *pkiadapter.CA | 224 | caAdapter *pkiadapter.CA | ||
| 225 | pkiSvc *pkiusecase.Service | 225 | pkiSvc *pkiusecase.Service | ||
| 226 | reloadCoordinator *reloadCoordinator | 226 | reloadCoordinator *reloadCoordinator | ||
| 227 | + | registryHandler interface { | |||
| 228 | + | UpdateBlobLimits(maxBlobChunkSize, maxBlobSize int64) | |||
| 229 | + | } | |||
| 227 | } | 230 | } | ||
| 228 | 231 | ||||
| 229 | // Run initializes and starts the Gordon application. | 232 | // Run initializes and starts the Gordon application. | ||
| @@ -466,7 +469,7 @@ func createServices(ctx context.Context, v *viper.Viper, cfg Config, log zerowra | |||||
| 466 | return nil, err | 469 | return nil, err | ||
| 467 | } | 470 | } | ||
| 468 | 471 | ||||
| 469 | - | si.svc.reloadCoordinator = newReloadCoordinator(v, si.svc.configSvc, si.svc.proxySvc, si.svc.eventBus, log) | 472 | + | si.svc.reloadCoordinator = newReloadCoordinator(v, si.svc.configSvc, si.svc.proxySvc, nil, si.svc.eventBus, log) |
| 470 | 473 | ||||
| 471 | si.initHandlers() | 474 | si.initHandlers() | ||
| 472 | 475 | ||||
| @@ -1369,11 +1372,18 @@ func readFileBeneath(root, cleanedRelPath string) ([]byte, error) { | |||||
| 1369 | } | 1372 | } | ||
| 1370 | last := i == len(parts)-1 | 1373 | last := i == len(parts)-1 | ||
| 1371 | if last { | 1374 | if last { | ||
| 1372 | - | fd, err := unix.Openat(dirFD, part, unix.O_RDONLY|unix.O_CLOEXEC|unix.O_NOFOLLOW, 0) | 1375 | + | fd, err := unix.Openat(dirFD, part, unix.O_RDONLY|unix.O_NONBLOCK|unix.O_CLOEXEC|unix.O_NOFOLLOW, 0) |
| 1373 | if err != nil { | 1376 | if err != nil { | ||
| 1374 | return nil, fmt.Errorf("failed to read secret file: %w", err) | 1377 | return nil, fmt.Errorf("failed to read secret file: %w", err) | ||
| 1375 | } | 1378 | } | ||
| 1376 | defer unix.Close(fd) | 1379 | defer unix.Close(fd) | ||
| 1380 | + | var st unix.Stat_t | |||
| 1381 | + | if err := unix.Fstat(fd, &st); err != nil { | |||
| 1382 | + | return nil, fmt.Errorf("failed to stat secret file: %w", err) | |||
| 1383 | + | } | |||
| 1384 | + | if st.Mode&unix.S_IFMT != unix.S_IFREG { | |||
| 1385 | + | return nil, fmt.Errorf("invalid secret path: secret must be a regular file") | |||
| 1386 | + | } | |||
| 1377 | return os.ReadFile(fmt.Sprintf("/proc/self/fd/%d", fd)) | 1387 | return os.ReadFile(fmt.Sprintf("/proc/self/fd/%d", fd)) | ||
| 1378 | } | 1388 | } | ||
| 1379 | 1389 | ||||
| @@ -1437,24 +1447,38 @@ type reloadCoordinator struct { | |||||
| 1437 | lastRun time.Time | 1447 | lastRun time.Time | ||
| 1438 | debounce time.Duration | 1448 | debounce time.Duration | ||
| 1439 | 1449 | ||||
| 1440 | - | configSvc configReloader | 1450 | + | configSvc configReloader |
| 1441 | - | v *viper.Viper | 1451 | + | v *viper.Viper |
| 1442 | - | proxySvc proxyConfigUpdater | 1452 | + | proxySvc proxyConfigUpdater |
| 1443 | - | eventBus out.EventPublisher | 1453 | + | registryLimits interface { |
| 1444 | - | log zerowrap.Logger | 1454 | + | UpdateBlobLimits(maxBlobChunkSize, maxBlobSize int64) |
| 1455 | + | } | |||
| 1456 | + | eventBus out.EventPublisher | |||
| 1457 | + | log zerowrap.Logger | |||
| 1445 | } | 1458 | } | ||
| 1446 | 1459 | ||||
| 1447 | - | func newReloadCoordinator(v *viper.Viper, configSvc configReloader, proxySvc proxyConfigUpdater, eventBus out.EventPublisher, log zerowrap.Logger) *reloadCoordinator { | 1460 | + | func newReloadCoordinator(v *viper.Viper, configSvc configReloader, proxySvc proxyConfigUpdater, registryLimits interface { |
| 1461 | + | UpdateBlobLimits(maxBlobChunkSize, maxBlobSize int64) | |||
| 1462 | + | }, eventBus out.EventPublisher, log zerowrap.Logger) *reloadCoordinator { | |||
| 1448 | return &reloadCoordinator{ | 1463 | return &reloadCoordinator{ | ||
| 1449 | - | debounce: 500 * time.Millisecond, | 1464 | + | debounce: 500 * time.Millisecond, |
| 1450 | - | configSvc: configSvc, | 1465 | + | configSvc: configSvc, |
| 1451 | - | v: v, | 1466 | + | v: v, |
| 1452 | - | proxySvc: proxySvc, | 1467 | + | proxySvc: proxySvc, |
| 1453 | - | eventBus: eventBus, | 1468 | + | registryLimits: registryLimits, |
| 1454 | - | log: log, | 1469 | + | eventBus: eventBus, |
| 1470 | + | log: log, | |||
| 1455 | } | 1471 | } | ||
| 1456 | } | 1472 | } | ||
| 1457 | 1473 | ||||
| 1474 | + | func (c *reloadCoordinator) SetRegistryLimits(limits interface { | |||
| 1475 | + | UpdateBlobLimits(maxBlobChunkSize, maxBlobSize int64) | |||
| 1476 | + | }) { | |||
| 1477 | + | c.mu.Lock() | |||
| 1478 | + | defer c.mu.Unlock() | |||
| 1479 | + | c.registryLimits = limits | |||
| 1480 | + | } | |||
| 1481 | + | ||||
| 1458 | func (c *reloadCoordinator) Trigger(ctx context.Context) error { | 1482 | func (c *reloadCoordinator) Trigger(ctx context.Context) error { | ||
| 1459 | c.mu.Lock() | 1483 | c.mu.Lock() | ||
| 1460 | defer c.mu.Unlock() | 1484 | defer c.mu.Unlock() | ||
| @@ -1504,6 +1528,9 @@ func (c *reloadCoordinator) applyLoadedConfig(now time.Time) error { | |||||
| 1504 | } | 1528 | } | ||
| 1505 | 1529 | ||||
| 1506 | c.proxySvc.UpdateConfig(reloadedProxy.proxyConfig) | 1530 | c.proxySvc.UpdateConfig(reloadedProxy.proxyConfig) | ||
| 1531 | + | if c.registryLimits != nil { | |||
| 1532 | + | c.registryLimits.UpdateBlobLimits(reloadedProxy.maxBlobChunkSize, reloadedProxy.maxBlobSize) | |||
| 1533 | + | } | |||
| 1507 | 1534 | ||||
| 1508 | if c.eventBus != nil { | 1535 | if c.eventBus != nil { | ||
| 1509 | if err := c.eventBus.Publish(domain.EventConfigReload, nil); err != nil { | 1536 | if err := c.eventBus.Publish(domain.EventConfigReload, nil); err != nil { | ||
| @@ -1835,6 +1862,10 @@ func createHTTPHandlers(svc *services, cfg Config, log zerowrap.Logger, accessWr | |||||
| 1835 | 1862 | ||||
| 1836 | // Registry handler | 1863 | // Registry handler | ||
| 1837 | registryHandler := registry.NewHandler(svc.registrySvc, log, svc.maxBlobChunkSize, svc.maxBlobSize) | 1864 | registryHandler := registry.NewHandler(svc.registrySvc, log, svc.maxBlobChunkSize, svc.maxBlobSize) | ||
| 1865 | + | svc.registryHandler = registryHandler | |||
| 1866 | + | if svc.reloadCoordinator != nil { | |||
| 1867 | + | svc.reloadCoordinator.SetRegistryLimits(registryHandler) | |||
| 1868 | + | } | |||
| 1838 | registryWithMiddleware, cidrAllowlistMiddleware, rateLimitMiddleware := buildRegistryHandlerWithMiddleware( | 1869 | registryWithMiddleware, cidrAllowlistMiddleware, rateLimitMiddleware := buildRegistryHandlerWithMiddleware( | ||
| 1839 | svc, | 1870 | svc, | ||
| 1840 | cfg, | 1871 | cfg, | ||
M internal/app/run_auth_config_test.go +16 -0
| @@ -4,6 +4,7 @@ import ( | |||
| 4 | 4 | "context" | |
| 5 | 5 | "os" | |
| 6 | 6 | "path/filepath" | |
| 7 | + | "runtime" | |
| 7 | 8 | "testing" | |
| 8 | 9 | ||
| 9 | 10 | "github.com/bnema/gordon/internal/domain" | |
| @@ -11,6 +12,7 @@ import ( | |||
| 11 | 12 | "github.com/spf13/viper" | |
| 12 | 13 | "github.com/stretchr/testify/assert" | |
| 13 | 14 | "github.com/stretchr/testify/require" | |
| 15 | + | "golang.org/x/sys/unix" | |
| 14 | 16 | ) | |
| 15 | 17 | ||
| 16 | 18 | func TestBuildAuthConfig_UsesConfigEnabledFlag(t *testing.T) { | |
| @@ -120,6 +122,20 @@ func TestLoadSecretUnsafeRejectsIntermediateSymlinkEscape(t *testing.T) { | |||
| 120 | 122 | assert.Contains(t, err.Error(), "failed to open secret path component") | |
| 121 | 123 | } | |
| 122 | 124 | ||
| 125 | + | func TestLoadSecretUnsafeRejectsNonRegularFile(t *testing.T) { | |
| 126 | + | if runtime.GOOS == "windows" { | |
| 127 | + | t.Skip("unix mkfifo is not available on windows") | |
| 128 | + | } | |
| 129 | + | dataDir := t.TempDir() | |
| 130 | + | secretsDir := filepath.Join(dataDir, "secrets") | |
| 131 | + | require.NoError(t, os.MkdirAll(secretsDir, 0700)) | |
| 132 | + | require.NoError(t, unix.Mkfifo(filepath.Join(secretsDir, "token"), 0600)) | |
| 133 | + | ||
| 134 | + | _, err := loadSecret(context.Background(), domain.SecretsBackendUnsafe, "token", dataDir, zerowrap.Default()) | |
| 135 | + | require.Error(t, err) | |
| 136 | + | assert.Contains(t, err.Error(), "secret must be a regular file") | |
| 137 | + | } | |
| 138 | + | ||
| 123 | 139 | func TestLoadSecretUnsafeRejectsSymlinkEscape(t *testing.T) { | |
| 124 | 140 | dataDir := t.TempDir() | |
| 125 | 141 | secretsDir := filepath.Join(dataDir, "secrets") | |
| @@ -4,6 +4,7 @@ import ( | |||||
| 4 | "context" | 4 | "context" | ||
| 5 | "os" | 5 | "os" | ||
| 6 | "path/filepath" | 6 | "path/filepath" | ||
| 7 | + | "runtime" | |||
| 7 | "testing" | 8 | "testing" | ||
| 8 | 9 | ||||
| 9 | "github.com/bnema/gordon/internal/domain" | 10 | "github.com/bnema/gordon/internal/domain" | ||
| @@ -11,6 +12,7 @@ import ( | |||||
| 11 | "github.com/spf13/viper" | 12 | "github.com/spf13/viper" | ||
| 12 | "github.com/stretchr/testify/assert" | 13 | "github.com/stretchr/testify/assert" | ||
| 13 | "github.com/stretchr/testify/require" | 14 | "github.com/stretchr/testify/require" | ||
| 15 | + | "golang.org/x/sys/unix" | |||
| 14 | ) | 16 | ) | ||
| 15 | 17 | ||||
| 16 | func TestBuildAuthConfig_UsesConfigEnabledFlag(t *testing.T) { | 18 | func TestBuildAuthConfig_UsesConfigEnabledFlag(t *testing.T) { | ||
| @@ -120,6 +122,20 @@ func TestLoadSecretUnsafeRejectsIntermediateSymlinkEscape(t *testing.T) { | |||||
| 120 | assert.Contains(t, err.Error(), "failed to open secret path component") | 122 | assert.Contains(t, err.Error(), "failed to open secret path component") | ||
| 121 | } | 123 | } | ||
| 122 | 124 | ||||
| 125 | + | func TestLoadSecretUnsafeRejectsNonRegularFile(t *testing.T) { | |||
| 126 | + | if runtime.GOOS == "windows" { | |||
| 127 | + | t.Skip("unix mkfifo is not available on windows") | |||
| 128 | + | } | |||
| 129 | + | dataDir := t.TempDir() | |||
| 130 | + | secretsDir := filepath.Join(dataDir, "secrets") | |||
| 131 | + | require.NoError(t, os.MkdirAll(secretsDir, 0700)) | |||
| 132 | + | require.NoError(t, unix.Mkfifo(filepath.Join(secretsDir, "token"), 0600)) | |||
| 133 | + | ||||
| 134 | + | _, err := loadSecret(context.Background(), domain.SecretsBackendUnsafe, "token", dataDir, zerowrap.Default()) | |||
| 135 | + | require.Error(t, err) | |||
| 136 | + | assert.Contains(t, err.Error(), "secret must be a regular file") | |||
| 137 | + | } | |||
| 138 | + | ||||
| 123 | func TestLoadSecretUnsafeRejectsSymlinkEscape(t *testing.T) { | 139 | func TestLoadSecretUnsafeRejectsSymlinkEscape(t *testing.T) { | ||
| 124 | dataDir := t.TempDir() | 140 | dataDir := t.TempDir() | ||
| 125 | secretsDir := filepath.Join(dataDir, "secrets") | 141 | secretsDir := filepath.Join(dataDir, "secrets") | ||
M internal/app/run_reload_test.go +27 -5
| @@ -100,6 +100,18 @@ func (p *proxyRecorder) UpdateConfig(config proxy.Config) { | |||
| 100 | 100 | p.config = config | |
| 101 | 101 | } | |
| 102 | 102 | ||
| 103 | + | type registryLimitsRecorder struct { | |
| 104 | + | calls int | |
| 105 | + | maxBlobChunkSize int64 | |
| 106 | + | maxBlobSize int64 | |
| 107 | + | } | |
| 108 | + | ||
| 109 | + | func (r *registryLimitsRecorder) UpdateBlobLimits(maxBlobChunkSize, maxBlobSize int64) { | |
| 110 | + | r.calls++ | |
| 111 | + | r.maxBlobChunkSize = maxBlobChunkSize | |
| 112 | + | r.maxBlobSize = maxBlobSize | |
| 113 | + | } | |
| 114 | + | ||
| 103 | 115 | type eventBusRecorder struct { | |
| 104 | 116 | mu sync.Mutex | |
| 105 | 117 | calls int | |
| @@ -164,6 +176,7 @@ func TestReloadCoordinator_ApplyLoadedConfig_RebuildsProxyConfigAndPublishesEven | |||
| 164 | 176 | v.Set("server.registry_port", 5000) | |
| 165 | 177 | v.Set("server.max_proxy_body_size", "5MB") | |
| 166 | 178 | v.Set("server.max_blob_chunk_size", "6MB") | |
| 179 | + | v.Set("server.max_blob_size", "8MB") | |
| 167 | 180 | v.Set("server.max_proxy_response_size", "7MB") | |
| 168 | 181 | v.Set("server.max_concurrent_connections", 99) | |
| 169 | 182 | ||
| @@ -171,12 +184,16 @@ func TestReloadCoordinator_ApplyLoadedConfig_RebuildsProxyConfigAndPublishesEven | |||
| 171 | 184 | proxySvc := &proxyRecorder{} | |
| 172 | 185 | events := &eventBusRecorder{} | |
| 173 | 186 | ||
| 174 | - | coord := newReloadCoordinator(v, reloadSvc, proxySvc, events, zerowrap.Default()) | |
| 187 | + | registryLimits := ®istryLimitsRecorder{} | |
| 188 | + | coord := newReloadCoordinator(v, reloadSvc, proxySvc, registryLimits, events, zerowrap.Default()) | |
| 175 | 189 | require.NoError(t, coord.ApplyLoadedConfig(ctx)) | |
| 176 | 190 | ||
| 177 | 191 | require.Equal(t, 0, reloadSvc.Calls()) | |
| 178 | 192 | require.Equal(t, 1, proxySvc.calls) | |
| 179 | 193 | require.Equal(t, 1, events.Calls()) | |
| 194 | + | require.Equal(t, 1, registryLimits.calls) | |
| 195 | + | require.Equal(t, int64(6<<20), registryLimits.maxBlobChunkSize) | |
| 196 | + | require.Equal(t, int64(8<<20), registryLimits.maxBlobSize) | |
| 180 | 197 | require.Equal(t, domain.EventConfigReload, events.eventType) | |
| 181 | 198 | require.Equal(t, proxy.Config{ | |
| 182 | 199 | RegistryDomain: "reload.example.com", | |
| @@ -194,6 +211,7 @@ func TestReloadCoordinator_DebouncesRepeatedWatchCallbacks(t *testing.T) { | |||
| 194 | 211 | v.Set("server.registry_port", 5000) | |
| 195 | 212 | v.Set("server.max_proxy_body_size", "5MB") | |
| 196 | 213 | v.Set("server.max_blob_chunk_size", "6MB") | |
| 214 | + | v.Set("server.max_blob_size", "8MB") | |
| 197 | 215 | v.Set("server.max_proxy_response_size", "7MB") | |
| 198 | 216 | v.Set("server.max_concurrent_connections", 99) | |
| 199 | 217 | ||
| @@ -201,13 +219,17 @@ func TestReloadCoordinator_DebouncesRepeatedWatchCallbacks(t *testing.T) { | |||
| 201 | 219 | proxySvc := &proxyRecorder{} | |
| 202 | 220 | events := &eventBusRecorder{} | |
| 203 | 221 | ||
| 204 | - | coord := newReloadCoordinator(v, reloadSvc, proxySvc, events, zerowrap.Default()) | |
| 222 | + | registryLimits := ®istryLimitsRecorder{} | |
| 223 | + | coord := newReloadCoordinator(v, reloadSvc, proxySvc, registryLimits, events, zerowrap.Default()) | |
| 205 | 224 | require.NoError(t, coord.Trigger(ctx)) | |
| 206 | 225 | require.NoError(t, coord.Trigger(ctx)) | |
| 207 | 226 | ||
| 208 | 227 | require.Equal(t, 1, reloadSvc.Calls()) | |
| 209 | 228 | require.Equal(t, 1, proxySvc.calls) | |
| 210 | 229 | require.Equal(t, 1, events.Calls()) | |
| 230 | + | require.Equal(t, 1, registryLimits.calls) | |
| 231 | + | require.Equal(t, int64(6<<20), registryLimits.maxBlobChunkSize) | |
| 232 | + | require.Equal(t, int64(8<<20), registryLimits.maxBlobSize) | |
| 211 | 233 | require.Equal(t, domain.EventConfigReload, events.eventType) | |
| 212 | 234 | require.Equal(t, proxy.Config{ | |
| 213 | 235 | RegistryDomain: "new.example.com", | |
| @@ -231,7 +253,7 @@ func TestReloadCoordinator_RetriesImmediatelyAfterFailedReload(t *testing.T) { | |||
| 231 | 253 | reloadErr := errors.New("reload failed") | |
| 232 | 254 | reloadSvc := &reloadRecorder{err: reloadErr} | |
| 233 | 255 | proxySvc := &proxyRecorder{} | |
| 234 | - | coord := newReloadCoordinator(v, reloadSvc, proxySvc, nil, zerowrap.Default()) | |
| 256 | + | coord := newReloadCoordinator(v, reloadSvc, proxySvc, nil, nil, zerowrap.Default()) | |
| 235 | 257 | ||
| 236 | 258 | err := coord.Trigger(ctx) | |
| 237 | 259 | require.ErrorIs(t, err, reloadErr) | |
| @@ -260,7 +282,7 @@ func TestReloadCoordinator_PublishErrorDoesNotAdvanceDebounceState(t *testing.T) | |||
| 260 | 282 | proxySvc := &proxyRecorder{} | |
| 261 | 283 | events := &eventBusRecorder{err: publishErr} | |
| 262 | 284 | ||
| 263 | - | coord := newReloadCoordinator(v, reloadSvc, proxySvc, events, zerowrap.Default()) | |
| 285 | + | coord := newReloadCoordinator(v, reloadSvc, proxySvc, nil, events, zerowrap.Default()) | |
| 264 | 286 | ||
| 265 | 287 | err := coord.Trigger(ctx) | |
| 266 | 288 | require.ErrorIs(t, err, publishErr) | |
| @@ -292,7 +314,7 @@ func TestReloadCoordinator_SerializesOverlappingReloadRequests(t *testing.T) { | |||
| 292 | 314 | <-release | |
| 293 | 315 | }} | |
| 294 | 316 | proxySvc := &proxyRecorder{} | |
| 295 | - | coord := newReloadCoordinator(v, reloadSvc, proxySvc, nil, zerowrap.Default()) | |
| 317 | + | coord := newReloadCoordinator(v, reloadSvc, proxySvc, nil, nil, zerowrap.Default()) | |
| 296 | 318 | ||
| 297 | 319 | firstDone := make(chan struct{}) | |
| 298 | 320 | go func() { | |
| @@ -100,6 +100,18 @@ func (p *proxyRecorder) UpdateConfig(config proxy.Config) { | |||||
| 100 | p.config = config | 100 | p.config = config | ||
| 101 | } | 101 | } | ||
| 102 | 102 | ||||
| 103 | + | type registryLimitsRecorder struct { | |||
| 104 | + | calls int | |||
| 105 | + | maxBlobChunkSize int64 | |||
| 106 | + | maxBlobSize int64 | |||
| 107 | + | } | |||
| 108 | + | ||||
| 109 | + | func (r *registryLimitsRecorder) UpdateBlobLimits(maxBlobChunkSize, maxBlobSize int64) { | |||
| 110 | + | r.calls++ | |||
| 111 | + | r.maxBlobChunkSize = maxBlobChunkSize | |||
| 112 | + | r.maxBlobSize = maxBlobSize | |||
| 113 | + | } | |||
| 114 | + | ||||
| 103 | type eventBusRecorder struct { | 115 | type eventBusRecorder struct { | ||
| 104 | mu sync.Mutex | 116 | mu sync.Mutex | ||
| 105 | calls int | 117 | calls int | ||
| @@ -164,6 +176,7 @@ func TestReloadCoordinator_ApplyLoadedConfig_RebuildsProxyConfigAndPublishesEven | |||||
| 164 | v.Set("server.registry_port", 5000) | 176 | v.Set("server.registry_port", 5000) | ||
| 165 | v.Set("server.max_proxy_body_size", "5MB") | 177 | v.Set("server.max_proxy_body_size", "5MB") | ||
| 166 | v.Set("server.max_blob_chunk_size", "6MB") | 178 | v.Set("server.max_blob_chunk_size", "6MB") | ||
| 179 | + | v.Set("server.max_blob_size", "8MB") | |||
| 167 | v.Set("server.max_proxy_response_size", "7MB") | 180 | v.Set("server.max_proxy_response_size", "7MB") | ||
| 168 | v.Set("server.max_concurrent_connections", 99) | 181 | v.Set("server.max_concurrent_connections", 99) | ||
| 169 | 182 | ||||
| @@ -171,12 +184,16 @@ func TestReloadCoordinator_ApplyLoadedConfig_RebuildsProxyConfigAndPublishesEven | |||||
| 171 | proxySvc := &proxyRecorder{} | 184 | proxySvc := &proxyRecorder{} | ||
| 172 | events := &eventBusRecorder{} | 185 | events := &eventBusRecorder{} | ||
| 173 | 186 | ||||
| 174 | - | coord := newReloadCoordinator(v, reloadSvc, proxySvc, events, zerowrap.Default()) | 187 | + | registryLimits := ®istryLimitsRecorder{} |
| 188 | + | coord := newReloadCoordinator(v, reloadSvc, proxySvc, registryLimits, events, zerowrap.Default()) | |||
| 175 | require.NoError(t, coord.ApplyLoadedConfig(ctx)) | 189 | require.NoError(t, coord.ApplyLoadedConfig(ctx)) | ||
| 176 | 190 | ||||
| 177 | require.Equal(t, 0, reloadSvc.Calls()) | 191 | require.Equal(t, 0, reloadSvc.Calls()) | ||
| 178 | require.Equal(t, 1, proxySvc.calls) | 192 | require.Equal(t, 1, proxySvc.calls) | ||
| 179 | require.Equal(t, 1, events.Calls()) | 193 | require.Equal(t, 1, events.Calls()) | ||
| 194 | + | require.Equal(t, 1, registryLimits.calls) | |||
| 195 | + | require.Equal(t, int64(6<<20), registryLimits.maxBlobChunkSize) | |||
| 196 | + | require.Equal(t, int64(8<<20), registryLimits.maxBlobSize) | |||
| 180 | require.Equal(t, domain.EventConfigReload, events.eventType) | 197 | require.Equal(t, domain.EventConfigReload, events.eventType) | ||
| 181 | require.Equal(t, proxy.Config{ | 198 | require.Equal(t, proxy.Config{ | ||
| 182 | RegistryDomain: "reload.example.com", | 199 | RegistryDomain: "reload.example.com", | ||
| @@ -194,6 +211,7 @@ func TestReloadCoordinator_DebouncesRepeatedWatchCallbacks(t *testing.T) { | |||||
| 194 | v.Set("server.registry_port", 5000) | 211 | v.Set("server.registry_port", 5000) | ||
| 195 | v.Set("server.max_proxy_body_size", "5MB") | 212 | v.Set("server.max_proxy_body_size", "5MB") | ||
| 196 | v.Set("server.max_blob_chunk_size", "6MB") | 213 | v.Set("server.max_blob_chunk_size", "6MB") | ||
| 214 | + | v.Set("server.max_blob_size", "8MB") | |||
| 197 | v.Set("server.max_proxy_response_size", "7MB") | 215 | v.Set("server.max_proxy_response_size", "7MB") | ||
| 198 | v.Set("server.max_concurrent_connections", 99) | 216 | v.Set("server.max_concurrent_connections", 99) | ||
| 199 | 217 | ||||
| @@ -201,13 +219,17 @@ func TestReloadCoordinator_DebouncesRepeatedWatchCallbacks(t *testing.T) { | |||||
| 201 | proxySvc := &proxyRecorder{} | 219 | proxySvc := &proxyRecorder{} | ||
| 202 | events := &eventBusRecorder{} | 220 | events := &eventBusRecorder{} | ||
| 203 | 221 | ||||
| 204 | - | coord := newReloadCoordinator(v, reloadSvc, proxySvc, events, zerowrap.Default()) | 222 | + | registryLimits := ®istryLimitsRecorder{} |
| 223 | + | coord := newReloadCoordinator(v, reloadSvc, proxySvc, registryLimits, events, zerowrap.Default()) | |||
| 205 | require.NoError(t, coord.Trigger(ctx)) | 224 | require.NoError(t, coord.Trigger(ctx)) | ||
| 206 | require.NoError(t, coord.Trigger(ctx)) | 225 | require.NoError(t, coord.Trigger(ctx)) | ||
| 207 | 226 | ||||
| 208 | require.Equal(t, 1, reloadSvc.Calls()) | 227 | require.Equal(t, 1, reloadSvc.Calls()) | ||
| 209 | require.Equal(t, 1, proxySvc.calls) | 228 | require.Equal(t, 1, proxySvc.calls) | ||
| 210 | require.Equal(t, 1, events.Calls()) | 229 | require.Equal(t, 1, events.Calls()) | ||
| 230 | + | require.Equal(t, 1, registryLimits.calls) | |||
| 231 | + | require.Equal(t, int64(6<<20), registryLimits.maxBlobChunkSize) | |||
| 232 | + | require.Equal(t, int64(8<<20), registryLimits.maxBlobSize) | |||
| 211 | require.Equal(t, domain.EventConfigReload, events.eventType) | 233 | require.Equal(t, domain.EventConfigReload, events.eventType) | ||
| 212 | require.Equal(t, proxy.Config{ | 234 | require.Equal(t, proxy.Config{ | ||
| 213 | RegistryDomain: "new.example.com", | 235 | RegistryDomain: "new.example.com", | ||
| @@ -231,7 +253,7 @@ func TestReloadCoordinator_RetriesImmediatelyAfterFailedReload(t *testing.T) { | |||||
| 231 | reloadErr := errors.New("reload failed") | 253 | reloadErr := errors.New("reload failed") | ||
| 232 | reloadSvc := &reloadRecorder{err: reloadErr} | 254 | reloadSvc := &reloadRecorder{err: reloadErr} | ||
| 233 | proxySvc := &proxyRecorder{} | 255 | proxySvc := &proxyRecorder{} | ||
| 234 | - | coord := newReloadCoordinator(v, reloadSvc, proxySvc, nil, zerowrap.Default()) | 256 | + | coord := newReloadCoordinator(v, reloadSvc, proxySvc, nil, nil, zerowrap.Default()) |
| 235 | 257 | ||||
| 236 | err := coord.Trigger(ctx) | 258 | err := coord.Trigger(ctx) | ||
| 237 | require.ErrorIs(t, err, reloadErr) | 259 | require.ErrorIs(t, err, reloadErr) | ||
| @@ -260,7 +282,7 @@ func TestReloadCoordinator_PublishErrorDoesNotAdvanceDebounceState(t *testing.T) | |||||
| 260 | proxySvc := &proxyRecorder{} | 282 | proxySvc := &proxyRecorder{} | ||
| 261 | events := &eventBusRecorder{err: publishErr} | 283 | events := &eventBusRecorder{err: publishErr} | ||
| 262 | 284 | ||||
| 263 | - | coord := newReloadCoordinator(v, reloadSvc, proxySvc, events, zerowrap.Default()) | 285 | + | coord := newReloadCoordinator(v, reloadSvc, proxySvc, nil, events, zerowrap.Default()) |
| 264 | 286 | ||||
| 265 | err := coord.Trigger(ctx) | 287 | err := coord.Trigger(ctx) | ||
| 266 | require.ErrorIs(t, err, publishErr) | 288 | require.ErrorIs(t, err, publishErr) | ||
| @@ -292,7 +314,7 @@ func TestReloadCoordinator_SerializesOverlappingReloadRequests(t *testing.T) { | |||||
| 292 | <-release | 314 | <-release | ||
| 293 | }} | 315 | }} | ||
| 294 | proxySvc := &proxyRecorder{} | 316 | proxySvc := &proxyRecorder{} | ||
| 295 | - | coord := newReloadCoordinator(v, reloadSvc, proxySvc, nil, zerowrap.Default()) | 317 | + | coord := newReloadCoordinator(v, reloadSvc, proxySvc, nil, nil, zerowrap.Default()) |
| 296 | 318 | ||||
| 297 | firstDone := make(chan struct{}) | 319 | firstDone := make(chan struct{}) | ||
| 298 | go func() { | 320 | go func() { | ||
M internal/domain/errors.go +1 -0
| @@ -52,6 +52,7 @@ var ( | |||
| 52 | 52 | ErrInvalidConfig = errors.New("invalid configuration") | |
| 53 | 53 | ErrConfigLoadFailed = errors.New("failed to load configuration") | |
| 54 | 54 | ErrInvalidDomainPattern = errors.New("invalid domain pattern") | |
| 55 | + | ErrRouteConflict = errors.New("route conflicts with existing configuration") | |
| 55 | 56 | ||
| 56 | 57 | // Environment errors | |
| 57 | 58 | ErrEnvFileNotFound = errors.New("environment file not found") | |
| @@ -52,6 +52,7 @@ var ( | |||||
| 52 | ErrInvalidConfig = errors.New("invalid configuration") | 52 | ErrInvalidConfig = errors.New("invalid configuration") | ||
| 53 | ErrConfigLoadFailed = errors.New("failed to load configuration") | 53 | ErrConfigLoadFailed = errors.New("failed to load configuration") | ||
| 54 | ErrInvalidDomainPattern = errors.New("invalid domain pattern") | 54 | ErrInvalidDomainPattern = errors.New("invalid domain pattern") | ||
| 55 | + | ErrRouteConflict = errors.New("route conflicts with existing configuration") | |||
| 55 | 56 | ||||
| 56 | // Environment errors | 57 | // Environment errors | ||
| 57 | ErrEnvFileNotFound = errors.New("environment file not found") | 58 | ErrEnvFileNotFound = errors.New("environment file not found") | ||
M internal/domain/redact.go +2 -2
| @@ -11,7 +11,7 @@ const secretKeyPattern = `[A-Z0-9_]*(?:PASSWORD|TOKEN|SECRET|API[_-]?KEY|DATABAS | |||
| 11 | 11 | ||
| 12 | 12 | var secretValuePatterns = []secretRedactionPattern{ | |
| 13 | 13 | { | |
| 14 | - | re: regexp.MustCompile(`(?i)(\b` + secretKeyPattern + `\b\s*=\s*)([^\s]+)`), | |
| 14 | + | re: regexp.MustCompile(`(?i)(\b` + secretKeyPattern + `\b\s*=\s*)(?:"[^"]*"|'[^']*'|[^\s]+)`), | |
| 15 | 15 | repl: `${1}[REDACTED]`, | |
| 16 | 16 | }, | |
| 17 | 17 | { | |
| @@ -35,7 +35,7 @@ func RedactSecrets(line string) string { | |||
| 35 | 35 | ||
| 36 | 36 | // RedactSecretLines redacts common secret values from log or diagnostic lines. | |
| 37 | 37 | func RedactSecretLines(lines []string) []string { | |
| 38 | - | if len(lines) == 0 { | |
| 38 | + | if lines == nil { | |
| 39 | 39 | return nil | |
| 40 | 40 | } | |
| 41 | 41 | redacted := make([]string, len(lines)) | |
| @@ -11,7 +11,7 @@ const secretKeyPattern = `[A-Z0-9_]*(?:PASSWORD|TOKEN|SECRET|API[_-]?KEY|DATABAS | |||||
| 11 | 11 | ||||
| 12 | var secretValuePatterns = []secretRedactionPattern{ | 12 | var secretValuePatterns = []secretRedactionPattern{ | ||
| 13 | { | 13 | { | ||
| 14 | - | re: regexp.MustCompile(`(?i)(\b` + secretKeyPattern + `\b\s*=\s*)([^\s]+)`), | 14 | + | re: regexp.MustCompile(`(?i)(\b` + secretKeyPattern + `\b\s*=\s*)(?:"[^"]*"|'[^']*'|[^\s]+)`), |
| 15 | repl: `${1}[REDACTED]`, | 15 | repl: `${1}[REDACTED]`, | ||
| 16 | }, | 16 | }, | ||
| 17 | { | 17 | { | ||
| @@ -35,7 +35,7 @@ func RedactSecrets(line string) string { | |||||
| 35 | 35 | ||||
| 36 | // RedactSecretLines redacts common secret values from log or diagnostic lines. | 36 | // RedactSecretLines redacts common secret values from log or diagnostic lines. | ||
| 37 | func RedactSecretLines(lines []string) []string { | 37 | func RedactSecretLines(lines []string) []string { | ||
| 38 | - | if len(lines) == 0 { | 38 | + | if lines == nil { |
| 39 | return nil | 39 | return nil | ||
| 40 | } | 40 | } | ||
| 41 | redacted := make([]string, len(lines)) | 41 | redacted := make([]string, len(lines)) | ||
M internal/domain/redact_test.go +8 -0
| @@ -17,6 +17,8 @@ func TestRedactSecrets(t *testing.T) { | |||
| 17 | 17 | {"secret env", "SECRET=shh", "SECRET=[REDACTED]"}, | |
| 18 | 18 | {"api key env", "API_KEY=key-value", "API_KEY=[REDACTED]"}, | |
| 19 | 19 | {"database url env", "DATABASE_URL=postgres://user:pass@db/app", "DATABASE_URL=[REDACTED]"}, | |
| 20 | + | {"quoted password env with spaces", `PASSWORD="hunter 2"`, "PASSWORD=[REDACTED]"}, | |
| 21 | + | {"single quoted token env with spaces", `TOKEN='abc 123'`, "TOKEN=[REDACTED]"}, | |
| 20 | 22 | {"json password field", `{"PASSWORD":"hunter2","ok":"value"}`, `{"PASSWORD":"[REDACTED]","ok":"value"}`}, | |
| 21 | 23 | {"json token field with spaces", `{"TOKEN": "abc123"}`, `{"TOKEN": "[REDACTED]"}`}, | |
| 22 | 24 | {"case insensitive", "database_url=postgres://user:pass@db/app", "database_url=[REDACTED]"}, | |
| @@ -35,3 +37,9 @@ func TestRedactSecrets(t *testing.T) { | |||
| 35 | 37 | }) | |
| 36 | 38 | } | |
| 37 | 39 | } | |
| 40 | + | ||
| 41 | + | func TestRedactSecretLinesPreservesEmptySlice(t *testing.T) { | |
| 42 | + | assert.Nil(t, RedactSecretLines(nil)) | |
| 43 | + | assert.Empty(t, RedactSecretLines([]string{})) | |
| 44 | + | assert.NotNil(t, RedactSecretLines([]string{})) | |
| 45 | + | } | |
| @@ -17,6 +17,8 @@ func TestRedactSecrets(t *testing.T) { | |||||
| 17 | {"secret env", "SECRET=shh", "SECRET=[REDACTED]"}, | 17 | {"secret env", "SECRET=shh", "SECRET=[REDACTED]"}, | ||
| 18 | {"api key env", "API_KEY=key-value", "API_KEY=[REDACTED]"}, | 18 | {"api key env", "API_KEY=key-value", "API_KEY=[REDACTED]"}, | ||
| 19 | {"database url env", "DATABASE_URL=postgres://user:pass@db/app", "DATABASE_URL=[REDACTED]"}, | 19 | {"database url env", "DATABASE_URL=postgres://user:pass@db/app", "DATABASE_URL=[REDACTED]"}, | ||
| 20 | + | {"quoted password env with spaces", `PASSWORD="hunter 2"`, "PASSWORD=[REDACTED]"}, | |||
| 21 | + | {"single quoted token env with spaces", `TOKEN='abc 123'`, "TOKEN=[REDACTED]"}, | |||
| 20 | {"json password field", `{"PASSWORD":"hunter2","ok":"value"}`, `{"PASSWORD":"[REDACTED]","ok":"value"}`}, | 22 | {"json password field", `{"PASSWORD":"hunter2","ok":"value"}`, `{"PASSWORD":"[REDACTED]","ok":"value"}`}, | ||
| 21 | {"json token field with spaces", `{"TOKEN": "abc123"}`, `{"TOKEN": "[REDACTED]"}`}, | 23 | {"json token field with spaces", `{"TOKEN": "abc123"}`, `{"TOKEN": "[REDACTED]"}`}, | ||
| 22 | {"case insensitive", "database_url=postgres://user:pass@db/app", "database_url=[REDACTED]"}, | 24 | {"case insensitive", "database_url=postgres://user:pass@db/app", "database_url=[REDACTED]"}, | ||
| @@ -35,3 +37,9 @@ func TestRedactSecrets(t *testing.T) { | |||||
| 35 | }) | 37 | }) | ||
| 36 | } | 38 | } | ||
| 37 | } | 39 | } | ||
| 40 | + | ||||
| 41 | + | func TestRedactSecretLinesPreservesEmptySlice(t *testing.T) { | |||
| 42 | + | assert.Nil(t, RedactSecretLines(nil)) | |||
| 43 | + | assert.Empty(t, RedactSecretLines([]string{})) | |||
| 44 | + | assert.NotNil(t, RedactSecretLines([]string{})) | |||
| 45 | + | } | |||
M internal/usecase/config/service.go +1 -1
| @@ -563,7 +563,7 @@ func (s *Service) AddRoute(ctx context.Context, route domain.Route) error { | |||
| 563 | 563 | } | |
| 564 | 564 | if _, exists := currentConfig.ExternalRoutes[route.Domain]; exists { | |
| 565 | 565 | s.mu.Unlock() | |
| 566 | - | return fmt.Errorf("route %q conflicts with external route", route.Domain) | |
| 566 | + | return fmt.Errorf("%w: route %q conflicts with external route", domain.ErrRouteConflict, route.Domain) | |
| 567 | 567 | } | |
| 568 | 568 | previousCanonicalRoute, canonicalExisted := currentConfig.Routes[route.Domain] | |
| 569 | 569 | legacyKey := legacyRouteStorageKey(route.Domain) | |
| @@ -563,7 +563,7 @@ func (s *Service) AddRoute(ctx context.Context, route domain.Route) error { | |||||
| 563 | } | 563 | } | ||
| 564 | if _, exists := currentConfig.ExternalRoutes[route.Domain]; exists { | 564 | if _, exists := currentConfig.ExternalRoutes[route.Domain]; exists { | ||
| 565 | s.mu.Unlock() | 565 | s.mu.Unlock() | ||
| 566 | - | return fmt.Errorf("route %q conflicts with external route", route.Domain) | 566 | + | return fmt.Errorf("%w: route %q conflicts with external route", domain.ErrRouteConflict, route.Domain) |
| 567 | } | 567 | } | ||
| 568 | previousCanonicalRoute, canonicalExisted := currentConfig.Routes[route.Domain] | 568 | previousCanonicalRoute, canonicalExisted := currentConfig.Routes[route.Domain] | ||
| 569 | legacyKey := legacyRouteStorageKey(route.Domain) | 569 | legacyKey := legacyRouteStorageKey(route.Domain) | ||
M internal/usecase/config/service_test.go +1 -0
| @@ -518,6 +518,7 @@ func TestService_AddRoute_RejectsExternalRouteConflict(t *testing.T) { | |||
| 518 | 518 | err := svc.AddRoute(ctx, domain.Route{Domain: "App.Example.com", Image: "app:latest"}) | |
| 519 | 519 | ||
| 520 | 520 | require.Error(t, err) | |
| 521 | + | assert.ErrorIs(t, err, domain.ErrRouteConflict) | |
| 521 | 522 | assert.Contains(t, err.Error(), "conflicts with external route") | |
| 522 | 523 | } | |
| 523 | 524 | ||
| @@ -518,6 +518,7 @@ func TestService_AddRoute_RejectsExternalRouteConflict(t *testing.T) { | |||||
| 518 | err := svc.AddRoute(ctx, domain.Route{Domain: "App.Example.com", Image: "app:latest"}) | 518 | err := svc.AddRoute(ctx, domain.Route{Domain: "App.Example.com", Image: "app:latest"}) | ||
| 519 | 519 | ||||
| 520 | require.Error(t, err) | 520 | require.Error(t, err) | ||
| 521 | + | assert.ErrorIs(t, err, domain.ErrRouteConflict) | |||
| 521 | assert.Contains(t, err.Error(), "conflicts with external route") | 522 | assert.Contains(t, err.Error(), "conflicts with external route") | ||
| 522 | } | 523 | } | ||
| 523 | 524 | ||||
M internal/usecase/container/service.go +40 -16
| @@ -1763,26 +1763,33 @@ func hasExplicitRegistry(image string) bool { | |||
| 1763 | 1763 | } | |
| 1764 | 1764 | ||
| 1765 | 1765 | func (s *Service) validateImageRef(imageRef string) error { | |
| 1766 | - | s.mu.RLock() | |
| 1767 | - | cfg := s.config | |
| 1768 | - | s.mu.RUnlock() | |
| 1769 | - | ||
| 1770 | 1766 | if !hasExplicitRegistry(imageRef) { | |
| 1771 | - | if cfg.RequireImageDigest && !isValidSha256DigestRef(imageRef) { | |
| 1772 | - | return fmt.Errorf("image %q rejected: digest reference required", imageRef) | |
| 1773 | - | } | |
| 1774 | 1767 | return nil | |
| 1775 | 1768 | } | |
| 1769 | + | return s.validateExternalImageRef(imageRef, imageRegistry(imageRef)) | |
| 1770 | + | } | |
| 1771 | + | ||
| 1772 | + | func (s *Service) validateImagePullRef(imageRef string) error { | |
| 1773 | + | return s.validateExternalImageRef(imageRef, imageRegistryForPolicy(imageRef)) | |
| 1774 | + | } | |
| 1775 | + | ||
| 1776 | + | func (s *Service) validateExternalImageRef(imageRef, registry string) error { | |
| 1777 | + | s.mu.RLock() | |
| 1778 | + | cfg := s.config | |
| 1779 | + | s.mu.RUnlock() | |
| 1776 | 1780 | ||
| 1777 | - | registry := imageRegistry(imageRef) | |
| 1778 | 1781 | if registry == "" { | |
| 1779 | - | return nil | |
| 1782 | + | return fmt.Errorf("image %q rejected: invalid image reference", imageRef) | |
| 1780 | 1783 | } | |
| 1781 | 1784 | ||
| 1782 | 1785 | if sameRegistry(registry, cfg.RegistryDomain) { | |
| 1783 | 1786 | return nil | |
| 1784 | 1787 | } | |
| 1785 | - | if isDangerousRegistryHost(imageRegistryHost(registry)) { | |
| 1788 | + | dangerous, err := isDangerousRegistryHost(imageRegistryHost(registry)) | |
| 1789 | + | if err != nil { | |
| 1790 | + | return fmt.Errorf("image %q rejected: registry %q could not be verified: %w", imageRef, registry, err) | |
| 1791 | + | } | |
| 1792 | + | if dangerous { | |
| 1786 | 1793 | return fmt.Errorf("image %q rejected: registry %q is not allowed", imageRef, registry) | |
| 1787 | 1794 | } | |
| 1788 | 1795 | if len(cfg.AllowedRegistries) == 0 { | |
| @@ -1807,6 +1814,13 @@ func imageRegistry(imageRef string) string { | |||
| 1807 | 1814 | return normalizeRegistry(imageRef[:slashIdx]) | |
| 1808 | 1815 | } | |
| 1809 | 1816 | ||
| 1817 | + | func imageRegistryForPolicy(imageRef string) string { | |
| 1818 | + | if hasExplicitRegistry(imageRef) { | |
| 1819 | + | return imageRegistry(imageRef) | |
| 1820 | + | } | |
| 1821 | + | return "docker.io" | |
| 1822 | + | } | |
| 1823 | + | ||
| 1810 | 1824 | func normalizeRegistry(registry string) string { | |
| 1811 | 1825 | registry = strings.ToLower(strings.TrimSuffix(strings.TrimSpace(registry), ".")) | |
| 1812 | 1826 | if strings.HasPrefix(registry, "[") { | |
| @@ -1839,21 +1853,28 @@ func sameRegistry(a, b string) bool { | |||
| 1839 | 1853 | return normalizeRegistry(a) == normalizeRegistry(b) | |
| 1840 | 1854 | } | |
| 1841 | 1855 | ||
| 1842 | - | func isDangerousRegistryHost(host string) bool { | |
| 1856 | + | func isDangerousRegistryHost(host string) (bool, error) { | |
| 1843 | 1857 | host = strings.ToLower(strings.TrimSuffix(host, ".")) | |
| 1844 | 1858 | if host == "localhost" || host == "metadata.google.internal" { | |
| 1845 | - | return true | |
| 1859 | + | return true, nil | |
| 1860 | + | } | |
| 1861 | + | if ip := net.ParseIP(host); ip != nil { | |
| 1862 | + | return isDangerousRegistryIP(ip), nil | |
| 1846 | 1863 | } | |
| 1847 | 1864 | ips, err := net.LookupIP(host) | |
| 1848 | 1865 | if err != nil { | |
| 1849 | - | return false | |
| 1866 | + | return false, err | |
| 1850 | 1867 | } | |
| 1851 | 1868 | for _, ip := range ips { | |
| 1852 | - | if ip.IsLoopback() || ip.IsPrivate() || ip.IsLinkLocalUnicast() || ip.IsLinkLocalMulticast() || ip.IsUnspecified() { | |
| 1853 | - | return true | |
| 1869 | + | if isDangerousRegistryIP(ip) { | |
| 1870 | + | return true, nil | |
| 1854 | 1871 | } | |
| 1855 | 1872 | } | |
| 1856 | - | return false | |
| 1873 | + | return false, nil | |
| 1874 | + | } | |
| 1875 | + | ||
| 1876 | + | func isDangerousRegistryIP(ip net.IP) bool { | |
| 1877 | + | return ip.IsLoopback() || ip.IsPrivate() || ip.IsLinkLocalUnicast() || ip.IsLinkLocalMulticast() || ip.IsUnspecified() | |
| 1857 | 1878 | } | |
| 1858 | 1879 | ||
| 1859 | 1880 | func applySecurityProfile(config *domain.ContainerConfig, cfg Config) { | |
| @@ -1924,6 +1945,9 @@ func (s *Service) ensureImage(ctx context.Context, imageRef string) (string, err | |||
| 1924 | 1945 | ||
| 1925 | 1946 | // Determine if this is an internal deploy and what reference to use for pulls. | |
| 1926 | 1947 | pullRef, isInternal := s.pullRefForDeploy(ctx, imageRef) | |
| 1948 | + | if err := s.validateImagePullRef(imageRef); err != nil { | |
| 1949 | + | return "", err | |
| 1950 | + | } | |
| 1927 | 1951 | if pullRef != imageRef { | |
| 1928 | 1952 | log.Info(). | |
| 1929 | 1953 | Str("original_ref", imageRef). | |
| @@ -1763,26 +1763,33 @@ func hasExplicitRegistry(image string) bool { | |||||
| 1763 | } | 1763 | } | ||
| 1764 | 1764 | ||||
| 1765 | func (s *Service) validateImageRef(imageRef string) error { | 1765 | func (s *Service) validateImageRef(imageRef string) error { | ||
| 1766 | - | s.mu.RLock() | |||
| 1767 | - | cfg := s.config | |||
| 1768 | - | s.mu.RUnlock() | |||
| 1769 | - | ||||
| 1770 | if !hasExplicitRegistry(imageRef) { | 1766 | if !hasExplicitRegistry(imageRef) { | ||
| 1771 | - | if cfg.RequireImageDigest && !isValidSha256DigestRef(imageRef) { | |||
| 1772 | - | return fmt.Errorf("image %q rejected: digest reference required", imageRef) | |||
| 1773 | - | } | |||
| 1774 | return nil | 1767 | return nil | ||
| 1775 | } | 1768 | } | ||
| 1769 | + | return s.validateExternalImageRef(imageRef, imageRegistry(imageRef)) | |||
| 1770 | + | } | |||
| 1771 | + | ||||
| 1772 | + | func (s *Service) validateImagePullRef(imageRef string) error { | |||
| 1773 | + | return s.validateExternalImageRef(imageRef, imageRegistryForPolicy(imageRef)) | |||
| 1774 | + | } | |||
| 1775 | + | ||||
| 1776 | + | func (s *Service) validateExternalImageRef(imageRef, registry string) error { | |||
| 1777 | + | s.mu.RLock() | |||
| 1778 | + | cfg := s.config | |||
| 1779 | + | s.mu.RUnlock() | |||
| 1776 | 1780 | ||||
| 1777 | - | registry := imageRegistry(imageRef) | |||
| 1778 | if registry == "" { | 1781 | if registry == "" { | ||
| 1779 | - | return nil | 1782 | + | return fmt.Errorf("image %q rejected: invalid image reference", imageRef) |
| 1780 | } | 1783 | } | ||
| 1781 | 1784 | ||||
| 1782 | if sameRegistry(registry, cfg.RegistryDomain) { | 1785 | if sameRegistry(registry, cfg.RegistryDomain) { | ||
| 1783 | return nil | 1786 | return nil | ||
| 1784 | } | 1787 | } | ||
| 1785 | - | if isDangerousRegistryHost(imageRegistryHost(registry)) { | 1788 | + | dangerous, err := isDangerousRegistryHost(imageRegistryHost(registry)) |
| 1789 | + | if err != nil { | |||
| 1790 | + | return fmt.Errorf("image %q rejected: registry %q could not be verified: %w", imageRef, registry, err) | |||
| 1791 | + | } | |||
| 1792 | + | if dangerous { | |||
| 1786 | return fmt.Errorf("image %q rejected: registry %q is not allowed", imageRef, registry) | 1793 | return fmt.Errorf("image %q rejected: registry %q is not allowed", imageRef, registry) | ||
| 1787 | } | 1794 | } | ||
| 1788 | if len(cfg.AllowedRegistries) == 0 { | 1795 | if len(cfg.AllowedRegistries) == 0 { | ||
| @@ -1807,6 +1814,13 @@ func imageRegistry(imageRef string) string { | |||||
| 1807 | return normalizeRegistry(imageRef[:slashIdx]) | 1814 | return normalizeRegistry(imageRef[:slashIdx]) | ||
| 1808 | } | 1815 | } | ||
| 1809 | 1816 | ||||
| 1817 | + | func imageRegistryForPolicy(imageRef string) string { | |||
| 1818 | + | if hasExplicitRegistry(imageRef) { | |||
| 1819 | + | return imageRegistry(imageRef) | |||
| 1820 | + | } | |||
| 1821 | + | return "docker.io" | |||
| 1822 | + | } | |||
| 1823 | + | ||||
| 1810 | func normalizeRegistry(registry string) string { | 1824 | func normalizeRegistry(registry string) string { | ||
| 1811 | registry = strings.ToLower(strings.TrimSuffix(strings.TrimSpace(registry), ".")) | 1825 | registry = strings.ToLower(strings.TrimSuffix(strings.TrimSpace(registry), ".")) | ||
| 1812 | if strings.HasPrefix(registry, "[") { | 1826 | if strings.HasPrefix(registry, "[") { | ||
| @@ -1839,21 +1853,28 @@ func sameRegistry(a, b string) bool { | |||||
| 1839 | return normalizeRegistry(a) == normalizeRegistry(b) | 1853 | return normalizeRegistry(a) == normalizeRegistry(b) | ||
| 1840 | } | 1854 | } | ||
| 1841 | 1855 | ||||
| 1842 | - | func isDangerousRegistryHost(host string) bool { | 1856 | + | func isDangerousRegistryHost(host string) (bool, error) { |
| 1843 | host = strings.ToLower(strings.TrimSuffix(host, ".")) | 1857 | host = strings.ToLower(strings.TrimSuffix(host, ".")) | ||
| 1844 | if host == "localhost" || host == "metadata.google.internal" { | 1858 | if host == "localhost" || host == "metadata.google.internal" { | ||
| 1845 | - | return true | 1859 | + | return true, nil |
| 1860 | + | } | |||
| 1861 | + | if ip := net.ParseIP(host); ip != nil { | |||
| 1862 | + | return isDangerousRegistryIP(ip), nil | |||
| 1846 | } | 1863 | } | ||
| 1847 | ips, err := net.LookupIP(host) | 1864 | ips, err := net.LookupIP(host) | ||
| 1848 | if err != nil { | 1865 | if err != nil { | ||
| 1849 | - | return false | 1866 | + | return false, err |
| 1850 | } | 1867 | } | ||
| 1851 | for _, ip := range ips { | 1868 | for _, ip := range ips { | ||
| 1852 | - | if ip.IsLoopback() || ip.IsPrivate() || ip.IsLinkLocalUnicast() || ip.IsLinkLocalMulticast() || ip.IsUnspecified() { | 1869 | + | if isDangerousRegistryIP(ip) { |
| 1853 | - | return true | 1870 | + | return true, nil |
| 1854 | } | 1871 | } | ||
| 1855 | } | 1872 | } | ||
| 1856 | - | return false | 1873 | + | return false, nil |
| 1874 | + | } | |||
| 1875 | + | ||||
| 1876 | + | func isDangerousRegistryIP(ip net.IP) bool { | |||
| 1877 | + | return ip.IsLoopback() || ip.IsPrivate() || ip.IsLinkLocalUnicast() || ip.IsLinkLocalMulticast() || ip.IsUnspecified() | |||
| 1857 | } | 1878 | } | ||
| 1858 | 1879 | ||||
| 1859 | func applySecurityProfile(config *domain.ContainerConfig, cfg Config) { | 1880 | func applySecurityProfile(config *domain.ContainerConfig, cfg Config) { | ||
| @@ -1924,6 +1945,9 @@ func (s *Service) ensureImage(ctx context.Context, imageRef string) (string, err | |||||
| 1924 | 1945 | ||||
| 1925 | // Determine if this is an internal deploy and what reference to use for pulls. | 1946 | // Determine if this is an internal deploy and what reference to use for pulls. | ||
| 1926 | pullRef, isInternal := s.pullRefForDeploy(ctx, imageRef) | 1947 | pullRef, isInternal := s.pullRefForDeploy(ctx, imageRef) | ||
| 1948 | + | if err := s.validateImagePullRef(imageRef); err != nil { | |||
| 1949 | + | return "", err | |||
| 1950 | + | } | |||
| 1927 | if pullRef != imageRef { | 1951 | if pullRef != imageRef { | ||
| 1928 | log.Info(). | 1952 | log.Info(). | ||
| 1929 | Str("original_ref", imageRef). | 1953 | Str("original_ref", imageRef). | ||
M internal/usecase/container/service_test.go +41 -8
| @@ -49,6 +49,7 @@ func dockerLogFrames(stream byte, lines ...string) io.ReadCloser { | |||
| 49 | 49 | // suitable for unit tests that don't want to wait for real timeouts. | |
| 50 | 50 | func testMinDelayConfig() Config { | |
| 51 | 51 | return Config{ | |
| 52 | + | AllowedRegistries: []string{"docker.io"}, | |
| 52 | 53 | ReadinessDelay: time.Millisecond, | |
| 53 | 54 | DrainDelay: time.Millisecond, | |
| 54 | 55 | DrainDelayConfigured: true, | |
| @@ -326,6 +327,7 @@ func TestService_Deploy_Success(t *testing.T) { | |||
| 326 | 327 | eventBus := mocks.NewMockEventPublisher(t) | |
| 327 | 328 | ||
| 328 | 329 | config := Config{ | |
| 330 | + | AllowedRegistries: []string{"docker.io"}, | |
| 329 | 331 | NetworkIsolation: false, | |
| 330 | 332 | VolumeAutoCreate: false, | |
| 331 | 333 | ReadinessDelay: time.Millisecond, // Minimal delay for tests | |
| @@ -408,6 +410,7 @@ func TestService_Deploy_ReadinessRecoveryWindow_AllowsTransientFlap(t *testing.T | |||
| 408 | 410 | eventBus := mocks.NewMockEventPublisher(t) | |
| 409 | 411 | ||
| 410 | 412 | config := Config{ | |
| 413 | + | AllowedRegistries: []string{"docker.io"}, | |
| 411 | 414 | NetworkIsolation: false, | |
| 412 | 415 | VolumeAutoCreate: false, | |
| 413 | 416 | ReadinessDelay: time.Millisecond, | |
| @@ -472,7 +475,7 @@ func TestService_Deploy_ImagePullFailure(t *testing.T) { | |||
| 472 | 475 | envLoader := mocks.NewMockEnvLoader(t) | |
| 473 | 476 | eventBus := mocks.NewMockEventPublisher(t) | |
| 474 | 477 | ||
| 475 | - | config := Config{} | |
| 478 | + | config := Config{AllowedRegistries: []string{"docker.io"}} | |
| 476 | 479 | svc := NewService(runtime, envLoader, eventBus, nil, config, nil) | |
| 477 | 480 | ctx := testContext() | |
| 478 | 481 | ||
| @@ -514,7 +517,7 @@ func TestService_Deploy_DoesNotMisclassifyUnauthorizedEnvLoadFailure(t *testing. | |||
| 514 | 517 | envLoader := mocks.NewMockEnvLoader(t) | |
| 515 | 518 | eventBus := mocks.NewMockEventPublisher(t) | |
| 516 | 519 | ||
| 517 | - | svc := NewService(runtime, envLoader, eventBus, nil, Config{}, nil) | |
| 520 | + | svc := NewService(runtime, envLoader, eventBus, nil, Config{AllowedRegistries: []string{"docker.io"}}, nil) | |
| 518 | 521 | ctx := testContext() | |
| 519 | 522 | ||
| 520 | 523 | route := domain.Route{ | |
| @@ -555,6 +558,7 @@ func TestService_Deploy_ReadinessFailureReturnsDeployFailureWithLogs(t *testing. | |||
| 555 | 558 | eventBus := mocks.NewMockEventPublisher(t) | |
| 556 | 559 | ||
| 557 | 560 | config := Config{ | |
| 561 | + | AllowedRegistries: []string{"docker.io"}, | |
| 558 | 562 | ReadinessMode: "docker-health", | |
| 559 | 563 | HealthTimeout: 10 * time.Millisecond, | |
| 560 | 564 | ReadinessDelay: time.Millisecond, | |
| @@ -613,6 +617,7 @@ func TestService_Deploy_StrictHealthModeWithoutHealthcheckReturnsHint(t *testing | |||
| 613 | 617 | eventBus := mocks.NewMockEventPublisher(t) | |
| 614 | 618 | ||
| 615 | 619 | config := Config{ | |
| 620 | + | AllowedRegistries: []string{"docker.io"}, | |
| 616 | 621 | ReadinessMode: "docker-health", | |
| 617 | 622 | ReadinessDelay: time.Millisecond, | |
| 618 | 623 | DrainDelay: time.Millisecond, | |
| @@ -659,6 +664,7 @@ func TestService_PullImage_UsesServiceTokenForExternalPull(t *testing.T) { | |||
| 659 | 664 | runtime := mocks.NewMockContainerRuntime(t) | |
| 660 | 665 | ||
| 661 | 666 | config := Config{ | |
| 667 | + | AllowedRegistries: []string{"docker.io"}, | |
| 662 | 668 | RegistryAuthEnabled: true, | |
| 663 | 669 | ServiceTokenUsername: "gordon-service", | |
| 664 | 670 | ServiceToken: "service-token", | |
| @@ -678,6 +684,7 @@ func TestService_PullImage_RequiresServiceTokenForExternalPull(t *testing.T) { | |||
| 678 | 684 | runtime := mocks.NewMockContainerRuntime(t) | |
| 679 | 685 | ||
| 680 | 686 | config := Config{ | |
| 687 | + | AllowedRegistries: []string{"docker.io"}, | |
| 681 | 688 | RegistryAuthEnabled: true, | |
| 682 | 689 | } | |
| 683 | 690 | ||
| @@ -694,6 +701,7 @@ func TestService_PullImage_InternalRetriesOnConnectionRefused(t *testing.T) { | |||
| 694 | 701 | runtime := mocks.NewMockContainerRuntime(t) | |
| 695 | 702 | ||
| 696 | 703 | config := Config{ | |
| 704 | + | AllowedRegistries: []string{"docker.io"}, | |
| 697 | 705 | RegistryAuthEnabled: true, | |
| 698 | 706 | InternalRegistryUsername: "internal", | |
| 699 | 707 | InternalRegistryPassword: "secret", | |
| @@ -721,6 +729,7 @@ func TestService_Deploy_ReplacesExistingContainer(t *testing.T) { | |||
| 721 | 729 | cacheInvalidator := mocks.NewMockProxyCacheInvalidator(t) | |
| 722 | 730 | ||
| 723 | 731 | config := Config{ | |
| 732 | + | AllowedRegistries: []string{"docker.io"}, | |
| 724 | 733 | ReadinessDelay: time.Millisecond, // Minimal delay for tests | |
| 725 | 734 | DrainDelay: time.Millisecond, | |
| 726 | 735 | DrainDelayConfigured: true, | |
| @@ -963,6 +972,7 @@ func TestService_Deploy_WithNetworkIsolation(t *testing.T) { | |||
| 963 | 972 | eventBus := mocks.NewMockEventPublisher(t) | |
| 964 | 973 | ||
| 965 | 974 | config := Config{ | |
| 975 | + | AllowedRegistries: []string{"docker.io"}, | |
| 966 | 976 | NetworkIsolation: true, | |
| 967 | 977 | NetworkPrefix: "gordon", | |
| 968 | 978 | ReadinessDelay: time.Millisecond, // Minimal delay for tests | |
| @@ -1020,6 +1030,7 @@ func TestService_Deploy_WithVolumeAutoCreate(t *testing.T) { | |||
| 1020 | 1030 | eventBus := mocks.NewMockEventPublisher(t) | |
| 1021 | 1031 | ||
| 1022 | 1032 | config := Config{ | |
| 1033 | + | AllowedRegistries: []string{"docker.io"}, | |
| 1023 | 1034 | VolumeAutoCreate: true, | |
| 1024 | 1035 | VolumePrefix: "gordon", | |
| 1025 | 1036 | ReadinessDelay: time.Millisecond, // Minimal delay for tests | |
| @@ -1135,8 +1146,9 @@ func TestService_Remove_WithNetworkCleanup(t *testing.T) { | |||
| 1135 | 1146 | eventBus := mocks.NewMockEventPublisher(t) | |
| 1136 | 1147 | ||
| 1137 | 1148 | config := Config{ | |
| 1138 | - | NetworkIsolation: true, | |
| 1139 | - | NetworkPrefix: "gordon", | |
| 1149 | + | AllowedRegistries: []string{"docker.io"}, | |
| 1150 | + | NetworkIsolation: true, | |
| 1151 | + | NetworkPrefix: "gordon", | |
| 1140 | 1152 | } | |
| 1141 | 1153 | svc := NewService(runtime, envLoader, eventBus, nil, config, nil) | |
| 1142 | 1154 | ctx := testContext() | |
| @@ -1932,6 +1944,7 @@ func TestService_Deploy_InternalDeployForcesPull(t *testing.T) { | |||
| 1932 | 1944 | eventBus := mocks.NewMockEventPublisher(t) | |
| 1933 | 1945 | ||
| 1934 | 1946 | config := Config{ | |
| 1947 | + | AllowedRegistries: []string{"docker.io"}, | |
| 1935 | 1948 | RegistryAuthEnabled: true, | |
| 1936 | 1949 | RegistryDomain: "registry.example.com", | |
| 1937 | 1950 | RegistryPort: 5000, | |
| @@ -2011,6 +2024,7 @@ func TestService_AutoStart_StartsNewContainers(t *testing.T) { | |||
| 2011 | 2024 | eventBus := mocks.NewMockEventPublisher(t) | |
| 2012 | 2025 | ||
| 2013 | 2026 | config := Config{ | |
| 2027 | + | AllowedRegistries: []string{"docker.io"}, | |
| 2014 | 2028 | ReadinessDelay: time.Millisecond, | |
| 2015 | 2029 | DrainDelay: time.Millisecond, | |
| 2016 | 2030 | DrainDelayConfigured: true, | |
| @@ -2059,6 +2073,7 @@ func TestService_AutoStart_SkipsExistingContainers(t *testing.T) { | |||
| 2059 | 2073 | eventBus := mocks.NewMockEventPublisher(t) | |
| 2060 | 2074 | ||
| 2061 | 2075 | config := Config{ | |
| 2076 | + | AllowedRegistries: []string{"docker.io"}, | |
| 2062 | 2077 | ReadinessDelay: time.Millisecond, | |
| 2063 | 2078 | DrainDelay: time.Millisecond, | |
| 2064 | 2079 | DrainDelayConfigured: true, | |
| @@ -2106,7 +2121,7 @@ func TestService_AutoStart_HandlesDeployErrors(t *testing.T) { | |||
| 2106 | 2121 | envLoader := mocks.NewMockEnvLoader(t) | |
| 2107 | 2122 | eventBus := mocks.NewMockEventPublisher(t) | |
| 2108 | 2123 | ||
| 2109 | - | config := Config{} | |
| 2124 | + | config := Config{AllowedRegistries: []string{"docker.io"}} | |
| 2110 | 2125 | svc := NewService(runtime, envLoader, eventBus, nil, config, nil) | |
| 2111 | 2126 | ctx := testContext() | |
| 2112 | 2127 | ||
| @@ -2188,6 +2203,7 @@ func TestService_Deploy_OrphanCleanupSkipsTrackedContainer(t *testing.T) { | |||
| 2188 | 2203 | cacheInvalidator := mocks.NewMockProxyCacheInvalidator(t) | |
| 2189 | 2204 | ||
| 2190 | 2205 | config := Config{ | |
| 2206 | + | AllowedRegistries: []string{"docker.io"}, | |
| 2191 | 2207 | ReadinessDelay: time.Millisecond, // Minimal delay for tests | |
| 2192 | 2208 | DrainDelay: time.Millisecond, | |
| 2193 | 2209 | DrainDelayConfigured: true, | |
| @@ -2282,6 +2298,7 @@ func TestService_Deploy_OrphanCleanupRemovesTrueOrphans(t *testing.T) { | |||
| 2282 | 2298 | eventBus := mocks.NewMockEventPublisher(t) | |
| 2283 | 2299 | ||
| 2284 | 2300 | config := Config{ | |
| 2301 | + | AllowedRegistries: []string{"docker.io"}, | |
| 2285 | 2302 | ReadinessDelay: time.Millisecond, // Minimal delay for tests | |
| 2286 | 2303 | DrainDelay: time.Millisecond, | |
| 2287 | 2304 | DrainDelayConfigured: true, | |
| @@ -2732,6 +2749,7 @@ func TestService_Deploy_ContextCancellation(t *testing.T) { | |||
| 2732 | 2749 | eventBus := mocks.NewMockEventPublisher(t) | |
| 2733 | 2750 | ||
| 2734 | 2751 | config := Config{ | |
| 2752 | + | AllowedRegistries: []string{"docker.io"}, | |
| 2735 | 2753 | ReadinessDelay: time.Millisecond, | |
| 2736 | 2754 | DrainDelay: time.Millisecond, | |
| 2737 | 2755 | DrainDelayConfigured: true, | |
| @@ -2890,6 +2908,7 @@ func TestService_Deploy_SkipsRedundantDeploy(t *testing.T) { | |||
| 2890 | 2908 | eventBus := mocks.NewMockEventPublisher(t) | |
| 2891 | 2909 | ||
| 2892 | 2910 | config := Config{ | |
| 2911 | + | AllowedRegistries: []string{"docker.io"}, | |
| 2893 | 2912 | ReadinessDelay: time.Millisecond, | |
| 2894 | 2913 | DrainDelay: time.Millisecond, | |
| 2895 | 2914 | DrainDelayConfigured: true, | |
| @@ -2949,6 +2968,7 @@ func TestService_Deploy_SkipsRedundantDeploy_WhenTrackedImageIDMissing(t *testin | |||
| 2949 | 2968 | eventBus := mocks.NewMockEventPublisher(t) | |
| 2950 | 2969 | ||
| 2951 | 2970 | config := Config{ | |
| 2971 | + | AllowedRegistries: []string{"docker.io"}, | |
| 2952 | 2972 | ReadinessDelay: time.Millisecond, | |
| 2953 | 2973 | DrainDelay: time.Millisecond, | |
| 2954 | 2974 | DrainDelayConfigured: true, | |
| @@ -3455,6 +3475,7 @@ func TestService_Deploy_RollbackOnPostSwitchCrash(t *testing.T) { | |||
| 3455 | 3475 | cacheInvalidator := mocks.NewMockProxyCacheInvalidator(t) | |
| 3456 | 3476 | ||
| 3457 | 3477 | config := Config{ | |
| 3478 | + | AllowedRegistries: []string{"docker.io"}, | |
| 3458 | 3479 | ReadinessDelay: time.Millisecond, | |
| 3459 | 3480 | DrainDelay: time.Millisecond, | |
| 3460 | 3481 | DrainDelayConfigured: true, | |
| @@ -3548,6 +3569,7 @@ func TestService_Deploy_StabilizationSuccess(t *testing.T) { | |||
| 3548 | 3569 | cacheInvalidator := mocks.NewMockProxyCacheInvalidator(t) | |
| 3549 | 3570 | ||
| 3550 | 3571 | config := Config{ | |
| 3572 | + | AllowedRegistries: []string{"docker.io"}, | |
| 3551 | 3573 | ReadinessDelay: time.Millisecond, | |
| 3552 | 3574 | DrainDelay: time.Millisecond, | |
| 3553 | 3575 | DrainDelayConfigured: true, | |
| @@ -3635,6 +3657,7 @@ func TestService_Deploy_ZeroDowntime_OldNeverStoppedBeforeNewReady(t *testing.T) | |||
| 3635 | 3657 | cacheInvalidator := mocks.NewMockProxyCacheInvalidator(t) | |
| 3636 | 3658 | ||
| 3637 | 3659 | config := Config{ | |
| 3660 | + | AllowedRegistries: []string{"docker.io"}, | |
| 3638 | 3661 | ReadinessDelay: time.Millisecond, | |
| 3639 | 3662 | DrainDelay: time.Millisecond, | |
| 3640 | 3663 | DrainDelayConfigured: true, | |
| @@ -3729,6 +3752,7 @@ func TestService_Deploy_ZeroDowntime_ReadinessFailure_OldUntouched(t *testing.T) | |||
| 3729 | 3752 | eventBus := mocks.NewMockEventPublisher(t) | |
| 3730 | 3753 | ||
| 3731 | 3754 | config := Config{ | |
| 3755 | + | AllowedRegistries: []string{"docker.io"}, | |
| 3732 | 3756 | ReadinessDelay: time.Millisecond, | |
| 3733 | 3757 | DrainDelay: time.Millisecond, | |
| 3734 | 3758 | DrainDelayConfigured: true, | |
| @@ -3818,6 +3842,7 @@ func TestService_Deploy_ZeroDowntime_NoExisting_FirstDeploy(t *testing.T) { | |||
| 3818 | 3842 | eventBus := mocks.NewMockEventPublisher(t) | |
| 3819 | 3843 | ||
| 3820 | 3844 | config := Config{ | |
| 3845 | + | AllowedRegistries: []string{"docker.io"}, | |
| 3821 | 3846 | ReadinessDelay: time.Millisecond, | |
| 3822 | 3847 | DrainDelay: time.Millisecond, | |
| 3823 | 3848 | DrainDelayConfigured: true, | |
| @@ -3893,6 +3918,7 @@ func TestService_Deploy_PropagatesImageLabelsToContainerConfig(t *testing.T) { | |||
| 3893 | 3918 | eventBus := mocks.NewMockEventPublisher(t) | |
| 3894 | 3919 | ||
| 3895 | 3920 | config := Config{ | |
| 3921 | + | AllowedRegistries: []string{"docker.io"}, | |
| 3896 | 3922 | NetworkIsolation: false, | |
| 3897 | 3923 | VolumeAutoCreate: false, | |
| 3898 | 3924 | ReadinessDelay: time.Millisecond, | |
| @@ -4208,6 +4234,10 @@ func TestService_BuildValidatedImageRef_RejectsExternalRegistryWhenAllowlistEmpt | |||
| 4208 | 4234 | _, err := svc.buildValidatedImageRef("ghcr.io/acme/app:latest") | |
| 4209 | 4235 | require.Error(t, err) | |
| 4210 | 4236 | assert.Contains(t, err.Error(), "images.allowed_registries") | |
| 4237 | + | ||
| 4238 | + | err = svc.validateImagePullRef("nginx:latest") | |
| 4239 | + | require.Error(t, err) | |
| 4240 | + | assert.Contains(t, err.Error(), "docker.io") | |
| 4211 | 4241 | } | |
| 4212 | 4242 | ||
| 4213 | 4243 | func TestService_BuildValidatedImageRef_RequireDigestOnlyForExternalAllowedRegistries(t *testing.T) { | |
| @@ -4221,6 +4251,9 @@ func TestService_BuildValidatedImageRef_RequireDigestOnlyForExternalAllowedRegis | |||
| 4221 | 4251 | _, err := svc.buildValidatedImageRef("docker.io/library/nginx:latest") | |
| 4222 | 4252 | require.Error(t, err) | |
| 4223 | 4253 | ||
| 4254 | + | err = svc.validateImagePullRef("nginx:latest") | |
| 4255 | + | require.Error(t, err) | |
| 4256 | + | ||
| 4224 | 4257 | _, err = svc.buildValidatedImageRef("docker.io/library/nginx@sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa") | |
| 4225 | 4258 | require.NoError(t, err) | |
| 4226 | 4259 | ||
| @@ -4236,12 +4269,12 @@ func TestService_BuildValidatedImageRef_RequireDigestOnlyForExternalAllowedRegis | |||
| 4236 | 4269 | } | |
| 4237 | 4270 | ||
| 4238 | 4271 | func TestService_BuildValidatedImageRef_AllowedRegistryMatchesPort(t *testing.T) { | |
| 4239 | - | svc := NewService(nil, nil, nil, nil, Config{AllowedRegistries: []string{"registry.example.com:5000"}}, nil) | |
| 4272 | + | svc := NewService(nil, nil, nil, nil, Config{AllowedRegistries: []string{"docker.io:5000"}}, nil) | |
| 4240 | 4273 | ||
| 4241 | - | _, err := svc.buildValidatedImageRef("registry.example.com:5000/app:latest") | |
| 4274 | + | _, err := svc.buildValidatedImageRef("docker.io:5000/app:latest") | |
| 4242 | 4275 | require.NoError(t, err) | |
| 4243 | 4276 | ||
| 4244 | - | _, err = svc.buildValidatedImageRef("registry.example.com:5001/app:latest") | |
| 4277 | + | _, err = svc.buildValidatedImageRef("docker.io:5001/app:latest") | |
| 4245 | 4278 | require.Error(t, err) | |
| 4246 | 4279 | } | |
| 4247 | 4280 | ||
| @@ -49,6 +49,7 @@ func dockerLogFrames(stream byte, lines ...string) io.ReadCloser { | |||||
| 49 | // suitable for unit tests that don't want to wait for real timeouts. | 49 | // suitable for unit tests that don't want to wait for real timeouts. | ||
| 50 | func testMinDelayConfig() Config { | 50 | func testMinDelayConfig() Config { | ||
| 51 | return Config{ | 51 | return Config{ | ||
| 52 | + | AllowedRegistries: []string{"docker.io"}, | |||
| 52 | ReadinessDelay: time.Millisecond, | 53 | ReadinessDelay: time.Millisecond, | ||
| 53 | DrainDelay: time.Millisecond, | 54 | DrainDelay: time.Millisecond, | ||
| 54 | DrainDelayConfigured: true, | 55 | DrainDelayConfigured: true, | ||
| @@ -326,6 +327,7 @@ func TestService_Deploy_Success(t *testing.T) { | |||||
| 326 | eventBus := mocks.NewMockEventPublisher(t) | 327 | eventBus := mocks.NewMockEventPublisher(t) | ||
| 327 | 328 | ||||
| 328 | config := Config{ | 329 | config := Config{ | ||
| 330 | + | AllowedRegistries: []string{"docker.io"}, | |||
| 329 | NetworkIsolation: false, | 331 | NetworkIsolation: false, | ||
| 330 | VolumeAutoCreate: false, | 332 | VolumeAutoCreate: false, | ||
| 331 | ReadinessDelay: time.Millisecond, // Minimal delay for tests | 333 | ReadinessDelay: time.Millisecond, // Minimal delay for tests | ||
| @@ -408,6 +410,7 @@ func TestService_Deploy_ReadinessRecoveryWindow_AllowsTransientFlap(t *testing.T | |||||
| 408 | eventBus := mocks.NewMockEventPublisher(t) | 410 | eventBus := mocks.NewMockEventPublisher(t) | ||
| 409 | 411 | ||||
| 410 | config := Config{ | 412 | config := Config{ | ||
| 413 | + | AllowedRegistries: []string{"docker.io"}, | |||
| 411 | NetworkIsolation: false, | 414 | NetworkIsolation: false, | ||
| 412 | VolumeAutoCreate: false, | 415 | VolumeAutoCreate: false, | ||
| 413 | ReadinessDelay: time.Millisecond, | 416 | ReadinessDelay: time.Millisecond, | ||
| @@ -472,7 +475,7 @@ func TestService_Deploy_ImagePullFailure(t *testing.T) { | |||||
| 472 | envLoader := mocks.NewMockEnvLoader(t) | 475 | envLoader := mocks.NewMockEnvLoader(t) | ||
| 473 | eventBus := mocks.NewMockEventPublisher(t) | 476 | eventBus := mocks.NewMockEventPublisher(t) | ||
| 474 | 477 | ||||
| 475 | - | config := Config{} | 478 | + | config := Config{AllowedRegistries: []string{"docker.io"}} |
| 476 | svc := NewService(runtime, envLoader, eventBus, nil, config, nil) | 479 | svc := NewService(runtime, envLoader, eventBus, nil, config, nil) | ||
| 477 | ctx := testContext() | 480 | ctx := testContext() | ||
| 478 | 481 | ||||
| @@ -514,7 +517,7 @@ func TestService_Deploy_DoesNotMisclassifyUnauthorizedEnvLoadFailure(t *testing. | |||||
| 514 | envLoader := mocks.NewMockEnvLoader(t) | 517 | envLoader := mocks.NewMockEnvLoader(t) | ||
| 515 | eventBus := mocks.NewMockEventPublisher(t) | 518 | eventBus := mocks.NewMockEventPublisher(t) | ||
| 516 | 519 | ||||
| 517 | - | svc := NewService(runtime, envLoader, eventBus, nil, Config{}, nil) | 520 | + | svc := NewService(runtime, envLoader, eventBus, nil, Config{AllowedRegistries: []string{"docker.io"}}, nil) |
| 518 | ctx := testContext() | 521 | ctx := testContext() | ||
| 519 | 522 | ||||
| 520 | route := domain.Route{ | 523 | route := domain.Route{ | ||
| @@ -555,6 +558,7 @@ func TestService_Deploy_ReadinessFailureReturnsDeployFailureWithLogs(t *testing. | |||||
| 555 | eventBus := mocks.NewMockEventPublisher(t) | 558 | eventBus := mocks.NewMockEventPublisher(t) | ||
| 556 | 559 | ||||
| 557 | config := Config{ | 560 | config := Config{ | ||
| 561 | + | AllowedRegistries: []string{"docker.io"}, | |||
| 558 | ReadinessMode: "docker-health", | 562 | ReadinessMode: "docker-health", | ||
| 559 | HealthTimeout: 10 * time.Millisecond, | 563 | HealthTimeout: 10 * time.Millisecond, | ||
| 560 | ReadinessDelay: time.Millisecond, | 564 | ReadinessDelay: time.Millisecond, | ||
| @@ -613,6 +617,7 @@ func TestService_Deploy_StrictHealthModeWithoutHealthcheckReturnsHint(t *testing | |||||
| 613 | eventBus := mocks.NewMockEventPublisher(t) | 617 | eventBus := mocks.NewMockEventPublisher(t) | ||
| 614 | 618 | ||||
| 615 | config := Config{ | 619 | config := Config{ | ||
| 620 | + | AllowedRegistries: []string{"docker.io"}, | |||
| 616 | ReadinessMode: "docker-health", | 621 | ReadinessMode: "docker-health", | ||
| 617 | ReadinessDelay: time.Millisecond, | 622 | ReadinessDelay: time.Millisecond, | ||
| 618 | DrainDelay: time.Millisecond, | 623 | DrainDelay: time.Millisecond, | ||
| @@ -659,6 +664,7 @@ func TestService_PullImage_UsesServiceTokenForExternalPull(t *testing.T) { | |||||
| 659 | runtime := mocks.NewMockContainerRuntime(t) | 664 | runtime := mocks.NewMockContainerRuntime(t) | ||
| 660 | 665 | ||||
| 661 | config := Config{ | 666 | config := Config{ | ||
| 667 | + | AllowedRegistries: []string{"docker.io"}, | |||
| 662 | RegistryAuthEnabled: true, | 668 | RegistryAuthEnabled: true, | ||
| 663 | ServiceTokenUsername: "gordon-service", | 669 | ServiceTokenUsername: "gordon-service", | ||
| 664 | ServiceToken: "service-token", | 670 | ServiceToken: "service-token", | ||
| @@ -678,6 +684,7 @@ func TestService_PullImage_RequiresServiceTokenForExternalPull(t *testing.T) { | |||||
| 678 | runtime := mocks.NewMockContainerRuntime(t) | 684 | runtime := mocks.NewMockContainerRuntime(t) | ||
| 679 | 685 | ||||
| 680 | config := Config{ | 686 | config := Config{ | ||
| 687 | + | AllowedRegistries: []string{"docker.io"}, | |||
| 681 | RegistryAuthEnabled: true, | 688 | RegistryAuthEnabled: true, | ||
| 682 | } | 689 | } | ||
| 683 | 690 | ||||
| @@ -694,6 +701,7 @@ func TestService_PullImage_InternalRetriesOnConnectionRefused(t *testing.T) { | |||||
| 694 | runtime := mocks.NewMockContainerRuntime(t) | 701 | runtime := mocks.NewMockContainerRuntime(t) | ||
| 695 | 702 | ||||
| 696 | config := Config{ | 703 | config := Config{ | ||
| 704 | + | AllowedRegistries: []string{"docker.io"}, | |||
| 697 | RegistryAuthEnabled: true, | 705 | RegistryAuthEnabled: true, | ||
| 698 | InternalRegistryUsername: "internal", | 706 | InternalRegistryUsername: "internal", | ||
| 699 | InternalRegistryPassword: "secret", | 707 | InternalRegistryPassword: "secret", | ||
| @@ -721,6 +729,7 @@ func TestService_Deploy_ReplacesExistingContainer(t *testing.T) { | |||||
| 721 | cacheInvalidator := mocks.NewMockProxyCacheInvalidator(t) | 729 | cacheInvalidator := mocks.NewMockProxyCacheInvalidator(t) | ||
| 722 | 730 | ||||
| 723 | config := Config{ | 731 | config := Config{ | ||
| 732 | + | AllowedRegistries: []string{"docker.io"}, | |||
| 724 | ReadinessDelay: time.Millisecond, // Minimal delay for tests | 733 | ReadinessDelay: time.Millisecond, // Minimal delay for tests | ||
| 725 | DrainDelay: time.Millisecond, | 734 | DrainDelay: time.Millisecond, | ||
| 726 | DrainDelayConfigured: true, | 735 | DrainDelayConfigured: true, | ||
| @@ -963,6 +972,7 @@ func TestService_Deploy_WithNetworkIsolation(t *testing.T) { | |||||
| 963 | eventBus := mocks.NewMockEventPublisher(t) | 972 | eventBus := mocks.NewMockEventPublisher(t) | ||
| 964 | 973 | ||||
| 965 | config := Config{ | 974 | config := Config{ | ||
| 975 | + | AllowedRegistries: []string{"docker.io"}, | |||
| 966 | NetworkIsolation: true, | 976 | NetworkIsolation: true, | ||
| 967 | NetworkPrefix: "gordon", | 977 | NetworkPrefix: "gordon", | ||
| 968 | ReadinessDelay: time.Millisecond, // Minimal delay for tests | 978 | ReadinessDelay: time.Millisecond, // Minimal delay for tests | ||
| @@ -1020,6 +1030,7 @@ func TestService_Deploy_WithVolumeAutoCreate(t *testing.T) { | |||||
| 1020 | eventBus := mocks.NewMockEventPublisher(t) | 1030 | eventBus := mocks.NewMockEventPublisher(t) | ||
| 1021 | 1031 | ||||
| 1022 | config := Config{ | 1032 | config := Config{ | ||
| 1033 | + | AllowedRegistries: []string{"docker.io"}, | |||
| 1023 | VolumeAutoCreate: true, | 1034 | VolumeAutoCreate: true, | ||
| 1024 | VolumePrefix: "gordon", | 1035 | VolumePrefix: "gordon", | ||
| 1025 | ReadinessDelay: time.Millisecond, // Minimal delay for tests | 1036 | ReadinessDelay: time.Millisecond, // Minimal delay for tests | ||
| @@ -1135,8 +1146,9 @@ func TestService_Remove_WithNetworkCleanup(t *testing.T) { | |||||
| 1135 | eventBus := mocks.NewMockEventPublisher(t) | 1146 | eventBus := mocks.NewMockEventPublisher(t) | ||
| 1136 | 1147 | ||||
| 1137 | config := Config{ | 1148 | config := Config{ | ||
| 1138 | - | NetworkIsolation: true, | 1149 | + | AllowedRegistries: []string{"docker.io"}, |
| 1139 | - | NetworkPrefix: "gordon", | 1150 | + | NetworkIsolation: true, |
| 1151 | + | NetworkPrefix: "gordon", | |||
| 1140 | } | 1152 | } | ||
| 1141 | svc := NewService(runtime, envLoader, eventBus, nil, config, nil) | 1153 | svc := NewService(runtime, envLoader, eventBus, nil, config, nil) | ||
| 1142 | ctx := testContext() | 1154 | ctx := testContext() | ||
| @@ -1932,6 +1944,7 @@ func TestService_Deploy_InternalDeployForcesPull(t *testing.T) { | |||||
| 1932 | eventBus := mocks.NewMockEventPublisher(t) | 1944 | eventBus := mocks.NewMockEventPublisher(t) | ||
| 1933 | 1945 | ||||
| 1934 | config := Config{ | 1946 | config := Config{ | ||
| 1947 | + | AllowedRegistries: []string{"docker.io"}, | |||
| 1935 | RegistryAuthEnabled: true, | 1948 | RegistryAuthEnabled: true, | ||
| 1936 | RegistryDomain: "registry.example.com", | 1949 | RegistryDomain: "registry.example.com", | ||
| 1937 | RegistryPort: 5000, | 1950 | RegistryPort: 5000, | ||
| @@ -2011,6 +2024,7 @@ func TestService_AutoStart_StartsNewContainers(t *testing.T) { | |||||
| 2011 | eventBus := mocks.NewMockEventPublisher(t) | 2024 | eventBus := mocks.NewMockEventPublisher(t) | ||
| 2012 | 2025 | ||||
| 2013 | config := Config{ | 2026 | config := Config{ | ||
| 2027 | + | AllowedRegistries: []string{"docker.io"}, | |||
| 2014 | ReadinessDelay: time.Millisecond, | 2028 | ReadinessDelay: time.Millisecond, | ||
| 2015 | DrainDelay: time.Millisecond, | 2029 | DrainDelay: time.Millisecond, | ||
| 2016 | DrainDelayConfigured: true, | 2030 | DrainDelayConfigured: true, | ||
| @@ -2059,6 +2073,7 @@ func TestService_AutoStart_SkipsExistingContainers(t *testing.T) { | |||||
| 2059 | eventBus := mocks.NewMockEventPublisher(t) | 2073 | eventBus := mocks.NewMockEventPublisher(t) | ||
| 2060 | 2074 | ||||
| 2061 | config := Config{ | 2075 | config := Config{ | ||
| 2076 | + | AllowedRegistries: []string{"docker.io"}, | |||
| 2062 | ReadinessDelay: time.Millisecond, | 2077 | ReadinessDelay: time.Millisecond, | ||
| 2063 | DrainDelay: time.Millisecond, | 2078 | DrainDelay: time.Millisecond, | ||
| 2064 | DrainDelayConfigured: true, | 2079 | DrainDelayConfigured: true, | ||
| @@ -2106,7 +2121,7 @@ func TestService_AutoStart_HandlesDeployErrors(t *testing.T) { | |||||
| 2106 | envLoader := mocks.NewMockEnvLoader(t) | 2121 | envLoader := mocks.NewMockEnvLoader(t) | ||
| 2107 | eventBus := mocks.NewMockEventPublisher(t) | 2122 | eventBus := mocks.NewMockEventPublisher(t) | ||
| 2108 | 2123 | ||||
| 2109 | - | config := Config{} | 2124 | + | config := Config{AllowedRegistries: []string{"docker.io"}} |
| 2110 | svc := NewService(runtime, envLoader, eventBus, nil, config, nil) | 2125 | svc := NewService(runtime, envLoader, eventBus, nil, config, nil) | ||
| 2111 | ctx := testContext() | 2126 | ctx := testContext() | ||
| 2112 | 2127 | ||||
| @@ -2188,6 +2203,7 @@ func TestService_Deploy_OrphanCleanupSkipsTrackedContainer(t *testing.T) { | |||||
| 2188 | cacheInvalidator := mocks.NewMockProxyCacheInvalidator(t) | 2203 | cacheInvalidator := mocks.NewMockProxyCacheInvalidator(t) | ||
| 2189 | 2204 | ||||
| 2190 | config := Config{ | 2205 | config := Config{ | ||
| 2206 | + | AllowedRegistries: []string{"docker.io"}, | |||
| 2191 | ReadinessDelay: time.Millisecond, // Minimal delay for tests | 2207 | ReadinessDelay: time.Millisecond, // Minimal delay for tests | ||
| 2192 | DrainDelay: time.Millisecond, | 2208 | DrainDelay: time.Millisecond, | ||
| 2193 | DrainDelayConfigured: true, | 2209 | DrainDelayConfigured: true, | ||
| @@ -2282,6 +2298,7 @@ func TestService_Deploy_OrphanCleanupRemovesTrueOrphans(t *testing.T) { | |||||
| 2282 | eventBus := mocks.NewMockEventPublisher(t) | 2298 | eventBus := mocks.NewMockEventPublisher(t) | ||
| 2283 | 2299 | ||||
| 2284 | config := Config{ | 2300 | config := Config{ | ||
| 2301 | + | AllowedRegistries: []string{"docker.io"}, | |||
| 2285 | ReadinessDelay: time.Millisecond, // Minimal delay for tests | 2302 | ReadinessDelay: time.Millisecond, // Minimal delay for tests | ||
| 2286 | DrainDelay: time.Millisecond, | 2303 | DrainDelay: time.Millisecond, | ||
| 2287 | DrainDelayConfigured: true, | 2304 | DrainDelayConfigured: true, | ||
| @@ -2732,6 +2749,7 @@ func TestService_Deploy_ContextCancellation(t *testing.T) { | |||||
| 2732 | eventBus := mocks.NewMockEventPublisher(t) | 2749 | eventBus := mocks.NewMockEventPublisher(t) | ||
| 2733 | 2750 | ||||
| 2734 | config := Config{ | 2751 | config := Config{ | ||
| 2752 | + | AllowedRegistries: []string{"docker.io"}, | |||
| 2735 | ReadinessDelay: time.Millisecond, | 2753 | ReadinessDelay: time.Millisecond, | ||
| 2736 | DrainDelay: time.Millisecond, | 2754 | DrainDelay: time.Millisecond, | ||
| 2737 | DrainDelayConfigured: true, | 2755 | DrainDelayConfigured: true, | ||
| @@ -2890,6 +2908,7 @@ func TestService_Deploy_SkipsRedundantDeploy(t *testing.T) { | |||||
| 2890 | eventBus := mocks.NewMockEventPublisher(t) | 2908 | eventBus := mocks.NewMockEventPublisher(t) | ||
| 2891 | 2909 | ||||
| 2892 | config := Config{ | 2910 | config := Config{ | ||
| 2911 | + | AllowedRegistries: []string{"docker.io"}, | |||
| 2893 | ReadinessDelay: time.Millisecond, | 2912 | ReadinessDelay: time.Millisecond, | ||
| 2894 | DrainDelay: time.Millisecond, | 2913 | DrainDelay: time.Millisecond, | ||
| 2895 | DrainDelayConfigured: true, | 2914 | DrainDelayConfigured: true, | ||
| @@ -2949,6 +2968,7 @@ func TestService_Deploy_SkipsRedundantDeploy_WhenTrackedImageIDMissing(t *testin | |||||
| 2949 | eventBus := mocks.NewMockEventPublisher(t) | 2968 | eventBus := mocks.NewMockEventPublisher(t) | ||
| 2950 | 2969 | ||||
| 2951 | config := Config{ | 2970 | config := Config{ | ||
| 2971 | + | AllowedRegistries: []string{"docker.io"}, | |||
| 2952 | ReadinessDelay: time.Millisecond, | 2972 | ReadinessDelay: time.Millisecond, | ||
| 2953 | DrainDelay: time.Millisecond, | 2973 | DrainDelay: time.Millisecond, | ||
| 2954 | DrainDelayConfigured: true, | 2974 | DrainDelayConfigured: true, | ||
| @@ -3455,6 +3475,7 @@ func TestService_Deploy_RollbackOnPostSwitchCrash(t *testing.T) { | |||||
| 3455 | cacheInvalidator := mocks.NewMockProxyCacheInvalidator(t) | 3475 | cacheInvalidator := mocks.NewMockProxyCacheInvalidator(t) | ||
| 3456 | 3476 | ||||
| 3457 | config := Config{ | 3477 | config := Config{ | ||
| 3478 | + | AllowedRegistries: []string{"docker.io"}, | |||
| 3458 | ReadinessDelay: time.Millisecond, | 3479 | ReadinessDelay: time.Millisecond, | ||
| 3459 | DrainDelay: time.Millisecond, | 3480 | DrainDelay: time.Millisecond, | ||
| 3460 | DrainDelayConfigured: true, | 3481 | DrainDelayConfigured: true, | ||
| @@ -3548,6 +3569,7 @@ func TestService_Deploy_StabilizationSuccess(t *testing.T) { | |||||
| 3548 | cacheInvalidator := mocks.NewMockProxyCacheInvalidator(t) | 3569 | cacheInvalidator := mocks.NewMockProxyCacheInvalidator(t) | ||
| 3549 | 3570 | ||||
| 3550 | config := Config{ | 3571 | config := Config{ | ||
| 3572 | + | AllowedRegistries: []string{"docker.io"}, | |||
| 3551 | ReadinessDelay: time.Millisecond, | 3573 | ReadinessDelay: time.Millisecond, | ||
| 3552 | DrainDelay: time.Millisecond, | 3574 | DrainDelay: time.Millisecond, | ||
| 3553 | DrainDelayConfigured: true, | 3575 | DrainDelayConfigured: true, | ||
| @@ -3635,6 +3657,7 @@ func TestService_Deploy_ZeroDowntime_OldNeverStoppedBeforeNewReady(t *testing.T) | |||||
| 3635 | cacheInvalidator := mocks.NewMockProxyCacheInvalidator(t) | 3657 | cacheInvalidator := mocks.NewMockProxyCacheInvalidator(t) | ||
| 3636 | 3658 | ||||
| 3637 | config := Config{ | 3659 | config := Config{ | ||
| 3660 | + | AllowedRegistries: []string{"docker.io"}, | |||
| 3638 | ReadinessDelay: time.Millisecond, | 3661 | ReadinessDelay: time.Millisecond, | ||
| 3639 | DrainDelay: time.Millisecond, | 3662 | DrainDelay: time.Millisecond, | ||
| 3640 | DrainDelayConfigured: true, | 3663 | DrainDelayConfigured: true, | ||
| @@ -3729,6 +3752,7 @@ func TestService_Deploy_ZeroDowntime_ReadinessFailure_OldUntouched(t *testing.T) | |||||
| 3729 | eventBus := mocks.NewMockEventPublisher(t) | 3752 | eventBus := mocks.NewMockEventPublisher(t) | ||
| 3730 | 3753 | ||||
| 3731 | config := Config{ | 3754 | config := Config{ | ||
| 3755 | + | AllowedRegistries: []string{"docker.io"}, | |||
| 3732 | ReadinessDelay: time.Millisecond, | 3756 | ReadinessDelay: time.Millisecond, | ||
| 3733 | DrainDelay: time.Millisecond, | 3757 | DrainDelay: time.Millisecond, | ||
| 3734 | DrainDelayConfigured: true, | 3758 | DrainDelayConfigured: true, | ||
| @@ -3818,6 +3842,7 @@ func TestService_Deploy_ZeroDowntime_NoExisting_FirstDeploy(t *testing.T) { | |||||
| 3818 | eventBus := mocks.NewMockEventPublisher(t) | 3842 | eventBus := mocks.NewMockEventPublisher(t) | ||
| 3819 | 3843 | ||||
| 3820 | config := Config{ | 3844 | config := Config{ | ||
| 3845 | + | AllowedRegistries: []string{"docker.io"}, | |||
| 3821 | ReadinessDelay: time.Millisecond, | 3846 | ReadinessDelay: time.Millisecond, | ||
| 3822 | DrainDelay: time.Millisecond, | 3847 | DrainDelay: time.Millisecond, | ||
| 3823 | DrainDelayConfigured: true, | 3848 | DrainDelayConfigured: true, | ||
| @@ -3893,6 +3918,7 @@ func TestService_Deploy_PropagatesImageLabelsToContainerConfig(t *testing.T) { | |||||
| 3893 | eventBus := mocks.NewMockEventPublisher(t) | 3918 | eventBus := mocks.NewMockEventPublisher(t) | ||
| 3894 | 3919 | ||||
| 3895 | config := Config{ | 3920 | config := Config{ | ||
| 3921 | + | AllowedRegistries: []string{"docker.io"}, | |||
| 3896 | NetworkIsolation: false, | 3922 | NetworkIsolation: false, | ||
| 3897 | VolumeAutoCreate: false, | 3923 | VolumeAutoCreate: false, | ||
| 3898 | ReadinessDelay: time.Millisecond, | 3924 | ReadinessDelay: time.Millisecond, | ||
| @@ -4208,6 +4234,10 @@ func TestService_BuildValidatedImageRef_RejectsExternalRegistryWhenAllowlistEmpt | |||||
| 4208 | _, err := svc.buildValidatedImageRef("ghcr.io/acme/app:latest") | 4234 | _, err := svc.buildValidatedImageRef("ghcr.io/acme/app:latest") | ||
| 4209 | require.Error(t, err) | 4235 | require.Error(t, err) | ||
| 4210 | assert.Contains(t, err.Error(), "images.allowed_registries") | 4236 | assert.Contains(t, err.Error(), "images.allowed_registries") | ||
| 4237 | + | ||||
| 4238 | + | err = svc.validateImagePullRef("nginx:latest") | |||
| 4239 | + | require.Error(t, err) | |||
| 4240 | + | assert.Contains(t, err.Error(), "docker.io") | |||
| 4211 | } | 4241 | } | ||
| 4212 | 4242 | ||||
| 4213 | func TestService_BuildValidatedImageRef_RequireDigestOnlyForExternalAllowedRegistries(t *testing.T) { | 4243 | func TestService_BuildValidatedImageRef_RequireDigestOnlyForExternalAllowedRegistries(t *testing.T) { | ||
| @@ -4221,6 +4251,9 @@ func TestService_BuildValidatedImageRef_RequireDigestOnlyForExternalAllowedRegis | |||||
| 4221 | _, err := svc.buildValidatedImageRef("docker.io/library/nginx:latest") | 4251 | _, err := svc.buildValidatedImageRef("docker.io/library/nginx:latest") | ||
| 4222 | require.Error(t, err) | 4252 | require.Error(t, err) | ||
| 4223 | 4253 | ||||
| 4254 | + | err = svc.validateImagePullRef("nginx:latest") | |||
| 4255 | + | require.Error(t, err) | |||
| 4256 | + | ||||
| 4224 | _, err = svc.buildValidatedImageRef("docker.io/library/nginx@sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa") | 4257 | _, err = svc.buildValidatedImageRef("docker.io/library/nginx@sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa") | ||
| 4225 | require.NoError(t, err) | 4258 | require.NoError(t, err) | ||
| 4226 | 4259 | ||||
| @@ -4236,12 +4269,12 @@ func TestService_BuildValidatedImageRef_RequireDigestOnlyForExternalAllowedRegis | |||||
| 4236 | } | 4269 | } | ||
| 4237 | 4270 | ||||
| 4238 | func TestService_BuildValidatedImageRef_AllowedRegistryMatchesPort(t *testing.T) { | 4271 | func TestService_BuildValidatedImageRef_AllowedRegistryMatchesPort(t *testing.T) { | ||
| 4239 | - | svc := NewService(nil, nil, nil, nil, Config{AllowedRegistries: []string{"registry.example.com:5000"}}, nil) | 4272 | + | svc := NewService(nil, nil, nil, nil, Config{AllowedRegistries: []string{"docker.io:5000"}}, nil) |
| 4240 | 4273 | ||||
| 4241 | - | _, err := svc.buildValidatedImageRef("registry.example.com:5000/app:latest") | 4274 | + | _, err := svc.buildValidatedImageRef("docker.io:5000/app:latest") |
| 4242 | require.NoError(t, err) | 4275 | require.NoError(t, err) | ||
| 4243 | 4276 | ||||
| 4244 | - | _, err = svc.buildValidatedImageRef("registry.example.com:5001/app:latest") | 4277 | + | _, err = svc.buildValidatedImageRef("docker.io:5001/app:latest") |
| 4245 | require.Error(t, err) | 4278 | require.Error(t, err) | ||
| 4246 | } | 4279 | } | ||
| 4247 | 4280 | ||||