diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d45517e..cf1771d 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -54,7 +54,7 @@ jobs: with: persist-credentials: false - - uses: azure/setup-helm@1a275c3b69536ee54be43f2070a358922e12c8d4 # v4.3.1 + - uses: azure/setup-helm@9bc31f4ebc9c6b171d7bfbaa5d006ae7abdb4310 # v5.0.1 with: version: v3.18.6 diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml index bc064b6..46f2059 100644 --- a/.github/workflows/publish.yml +++ b/.github/workflows/publish.yml @@ -26,7 +26,7 @@ jobs: persist-credentials: false - name: Set up QEMU - uses: docker/setup-qemu-action@96fe6ef7f33517b61c61be40b68a1882f3264fb8 # v4.2.0 + uses: docker/setup-qemu-action@1f40c72289eff860ee54a304f1438e3cff362e0a # v4.3.0 with: platforms: linux/amd64,linux/arm64 @@ -112,7 +112,7 @@ jobs: persist-credentials: false ref: ${{ github.sha }} - - uses: azure/setup-helm@1a275c3b69536ee54be43f2070a358922e12c8d4 # v4.3.1 + - uses: azure/setup-helm@9bc31f4ebc9c6b171d7bfbaa5d006ae7abdb4310 # v5.0.1 with: version: v3.18.6 diff --git a/.github/workflows/zizmor.yml b/.github/workflows/zizmor.yml index ac587dd..b32ce36 100644 --- a/.github/workflows/zizmor.yml +++ b/.github/workflows/zizmor.yml @@ -26,4 +26,4 @@ jobs: persist-credentials: false - name: Run zizmor - uses: zizmorcore/zizmor-action@3dc1ecc9bcb9e94e9b2c709687979e1298497054 # v0.6.2 + uses: zizmorcore/zizmor-action@70fb788f84895a7701f5643d103d587e460b5c99 # v0.6.3 diff --git a/README.md b/README.md index 4355e51..7d374bd 100644 --- a/README.md +++ b/README.md @@ -68,7 +68,7 @@ The proxy never uploads artifact bytes to a scanner. Each scanner is notified wi | Arch | Arch Linux | | ✗ | | Chef | Chef | | ✗ | | Generic | Any | | ✓ | -| Helm | Kubernetes | | ✗ | +| Helm | Kubernetes | | ✓ | | Vagrant | Vagrant | | ✗ | Cooldown requires publish timestamps in metadata. Registries without a "Yes" in the cooldown column either don't expose timestamps or haven't been wired up yet. diff --git a/cmd/proxy/main.go b/cmd/proxy/main.go index c943a4d..cf28d8d 100644 --- a/cmd/proxy/main.go +++ b/cmd/proxy/main.go @@ -106,6 +106,7 @@ import ( "log/slog" "os" "os/signal" + "runtime/debug" "strings" "syscall" @@ -128,6 +129,15 @@ var ( Commit = "unknown" ) +func init() { + if Version != "dev" { + return + } + if bi, ok := debug.ReadBuildInfo(); ok && bi.Main.Version != "" && bi.Main.Version != "(devel)" { + Version = bi.Main.Version + } +} + func main() { if len(os.Args) > 1 { switch os.Args[1] { diff --git a/docs/configuration.md b/docs/configuration.md index 6d27d7f..40840e9 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -548,7 +548,7 @@ http_timeout: "30s" # default Or via environment variable: `PROXY_HTTP_TIMEOUT=2m`. -Set to `"0"` to disable the timeout entirely (requests then rely only on the server's write timeout). +Set to `"0"` to disable the timeout entirely (requests then rely only on the server's write timeout). Independently of this setting, the shared transport gives up on an upstream that has not sent response headers within 60 seconds. ## Mirror API diff --git a/go.mod b/go.mod index bcb9cd0..6de540f 100644 --- a/go.mod +++ b/go.mod @@ -5,8 +5,8 @@ go 1.26.7 require ( github.com/BurntSushi/toml v1.6.0 github.com/CycloneDX/cyclonedx-go v0.12.0 - github.com/aws/aws-sdk-go-v2/config v1.32.40 - github.com/aws/aws-sdk-go-v2/service/ecr v1.61.0 + github.com/aws/aws-sdk-go-v2/config v1.33.2 + github.com/aws/aws-sdk-go-v2/service/ecr v1.64.0 github.com/git-pkgs/archives v0.7.0 github.com/git-pkgs/artifacts v0.2.1 github.com/git-pkgs/cooldown v0.2.0 @@ -24,14 +24,14 @@ require ( github.com/lib/pq v1.12.3 github.com/opencontainers/go-digest v1.0.0 github.com/prometheus/client_golang v1.24.1 - github.com/prometheus/client_model v0.6.2 + github.com/prometheus/client_model v0.6.3 github.com/spdx/tools-golang v0.5.7 github.com/swaggo/swag v1.16.6 gocloud.dev v0.46.0 - golang.org/x/sync v0.22.0 + golang.org/x/sync v0.23.0 google.golang.org/protobuf v1.36.12 gopkg.in/yaml.v3 v3.0.1 - modernc.org/sqlite v1.57.0 + modernc.org/sqlite v1.58.0 ) require ( @@ -77,23 +77,23 @@ require ( github.com/apapsch/go-jsonmerge/v2 v2.0.0 // indirect github.com/ashanbrown/forbidigo/v2 v2.3.1 // indirect github.com/ashanbrown/makezero/v2 v2.2.1 // indirect - github.com/aws/aws-sdk-go-v2 v1.44.0 // indirect + github.com/aws/aws-sdk-go-v2 v1.46.0 // indirect github.com/aws/aws-sdk-go-v2/aws/protocol/eventstream v1.7.11 // indirect - github.com/aws/aws-sdk-go-v2/credentials v1.19.39 // indirect - github.com/aws/aws-sdk-go-v2/feature/ec2/imds v1.18.40 // indirect + github.com/aws/aws-sdk-go-v2/credentials v1.20.2 // indirect + github.com/aws/aws-sdk-go-v2/feature/ec2/imds v1.19.1 // indirect github.com/aws/aws-sdk-go-v2/feature/s3/transfermanager v0.2.3 // indirect - github.com/aws/aws-sdk-go-v2/internal/configsources v1.4.40 // indirect - github.com/aws/aws-sdk-go-v2/internal/endpoints/v2 v2.7.40 // indirect - github.com/aws/aws-sdk-go-v2/internal/v4a v1.4.41 // indirect + github.com/aws/aws-sdk-go-v2/internal/configsources v1.5.2 // indirect + github.com/aws/aws-sdk-go-v2/internal/endpoints/v2 v2.8.2 // indirect + github.com/aws/aws-sdk-go-v2/internal/v4a v1.5.1 // indirect github.com/aws/aws-sdk-go-v2/service/internal/accept-encoding v1.13.19 // indirect github.com/aws/aws-sdk-go-v2/service/internal/checksum v1.9.18 // indirect - github.com/aws/aws-sdk-go-v2/service/internal/presigned-url v1.13.40 // indirect + github.com/aws/aws-sdk-go-v2/service/internal/presigned-url v1.14.1 // indirect github.com/aws/aws-sdk-go-v2/service/internal/s3shared v1.19.25 // indirect github.com/aws/aws-sdk-go-v2/service/s3 v1.102.2 // indirect - github.com/aws/aws-sdk-go-v2/service/signin v1.6.0 // indirect - github.com/aws/aws-sdk-go-v2/service/sso v1.34.0 // indirect - github.com/aws/aws-sdk-go-v2/service/ssooidc v1.39.0 // indirect - github.com/aws/aws-sdk-go-v2/service/sts v1.46.0 // indirect + github.com/aws/aws-sdk-go-v2/service/signin v1.8.0 // indirect + github.com/aws/aws-sdk-go-v2/service/sso v1.36.0 // indirect + github.com/aws/aws-sdk-go-v2/service/ssooidc v1.41.0 // indirect + github.com/aws/aws-sdk-go-v2/service/sts v1.48.0 // indirect github.com/aws/smithy-go v1.28.1 // indirect github.com/beorn7/perks v1.0.1 // indirect github.com/bkielbasa/cyclop v1.2.3 // indirect @@ -315,13 +315,13 @@ require ( golang.org/x/xerrors v0.0.0-20240903120638-7835f813f4da // indirect google.golang.org/api v0.288.0 // indirect google.golang.org/genproto/googleapis/rpc v0.0.0-20260706201446-f0a921348800 // indirect - google.golang.org/grpc v1.83.1 // indirect + google.golang.org/grpc v1.83.2 // indirect gopkg.in/ini.v1 v1.67.0 // indirect gopkg.in/yaml.v2 v2.4.0 // indirect honnef.co/go/tools v0.8.0 // indirect - modernc.org/libc v1.74.4 // indirect + modernc.org/libc v1.75.6 // indirect modernc.org/mathutil v1.7.1 // indirect - modernc.org/memory v1.11.0 // indirect + modernc.org/memory v1.12.1 // indirect mvdan.cc/gofumpt v0.11.0 // indirect mvdan.cc/unparam v0.0.0-20260818115549-3f964bcb5673 // indirect sigs.k8s.io/yaml v1.6.0 // indirect diff --git a/go.sum b/go.sum index 5ef1906..fb92925 100644 --- a/go.sum +++ b/go.sum @@ -117,44 +117,44 @@ github.com/ashanbrown/forbidigo/v2 v2.3.1 h1:KAZijvQ7zeIBKbhikT4jCm0TLYXC4u78bTi github.com/ashanbrown/forbidigo/v2 v2.3.1/go.mod h1:2QDkLTzU6TV937eFROamXrW92M3paehdae4HCDCOZCM= github.com/ashanbrown/makezero/v2 v2.2.1 h1:A7uU8dgB1PA9aelTxHMfHIQ8Qev8AB3JLxJUBUsejqM= github.com/ashanbrown/makezero/v2 v2.2.1/go.mod h1:aEGT/9q3S8DHeE57C88z2a6xydvgx8J5hgXIGWgo0MY= -github.com/aws/aws-sdk-go-v2 v1.44.0 h1:4IbaHhtzy+4h37z4JQyO9a2QsiCml3CNYHtq5hIHigo= -github.com/aws/aws-sdk-go-v2 v1.44.0/go.mod h1:bttEH6JqnUL8LepvDVfdrds/fZ5bCIxzpe3abyUrhDU= +github.com/aws/aws-sdk-go-v2 v1.46.0 h1:1kt7m/EKcEHt5mlyyxx9cSlMddRPIKbjb6DIQsu4HPk= +github.com/aws/aws-sdk-go-v2 v1.46.0/go.mod h1:bttEH6JqnUL8LepvDVfdrds/fZ5bCIxzpe3abyUrhDU= github.com/aws/aws-sdk-go-v2/aws/protocol/eventstream v1.7.11 h1:h5+3VT69KUBK24grGuuA5saDJTj2IIjLb9au668Fo5I= github.com/aws/aws-sdk-go-v2/aws/protocol/eventstream v1.7.11/go.mod h1:dnakxebH6UwFvcvujL0LVggYQ8nEvBGjU4G/V79Nv94= -github.com/aws/aws-sdk-go-v2/config v1.32.40 h1:lAVC9gMmKusmqDRe32dPtgKl/BWvJmMJoWELKHCAObw= -github.com/aws/aws-sdk-go-v2/config v1.32.40/go.mod h1:8xOJLbe/hOj1g4PVsfJYV7O2byq+UGET1onDdUgbwqc= -github.com/aws/aws-sdk-go-v2/credentials v1.19.39 h1:XOg8LC3Kgnsa3WiPQjc7Bi8k5IBN92cPYfIV9XMFss0= -github.com/aws/aws-sdk-go-v2/credentials v1.19.39/go.mod h1:GonTDBQ+mTpCVNwaHjj0PagspfrYYMEqOx7FehoEP/I= -github.com/aws/aws-sdk-go-v2/feature/ec2/imds v1.18.40 h1:r5aGipEVgI9aT/tAGjdrPbDQvIAKdTrS3rUPQtG4Rmo= -github.com/aws/aws-sdk-go-v2/feature/ec2/imds v1.18.40/go.mod h1:vOD3CnPxAdkL6MWZeROkZsTlskklMFfgVFkHzx/oZpY= +github.com/aws/aws-sdk-go-v2/config v1.33.2 h1:Pj4+nF2kc4Z+1BJysVPnX9d5dMN7IYFXR4UJaWK2IpA= +github.com/aws/aws-sdk-go-v2/config v1.33.2/go.mod h1:Igw+HTwbR2tsTU/ydifAS9EHAFJ2s/FCgkwQWFnAdE4= +github.com/aws/aws-sdk-go-v2/credentials v1.20.2 h1:VQjZODPNfdikCX2ZZrltw4zNLkcwjyUFDUl2vT9yTwg= +github.com/aws/aws-sdk-go-v2/credentials v1.20.2/go.mod h1:OmeHCn28vZylsBvalLDf7t8fuJ2rHYQprJs+7WuxniI= +github.com/aws/aws-sdk-go-v2/feature/ec2/imds v1.19.1 h1:YIEBqcqRnpi4Pfv0YHImtgi6czGCwKHANC7SwmUAVD0= +github.com/aws/aws-sdk-go-v2/feature/ec2/imds v1.19.1/go.mod h1:imEf0oufgAo8KAkCHhrOdqGEC0YWx1PPBQH82shSxGw= github.com/aws/aws-sdk-go-v2/feature/s3/transfermanager v0.2.3 h1:w5OoDiMN6x53ROmiIImGzmVcxXv2q1GXY+aKV4WAJYM= github.com/aws/aws-sdk-go-v2/feature/s3/transfermanager v0.2.3/go.mod h1:dAhgYp776bX3LuWvnSCFwQEjNs6fuFg7YXIy5PXcP3Q= -github.com/aws/aws-sdk-go-v2/internal/configsources v1.4.40 h1:UIXlbijuB2XK1Kr57fo8iIxCuaSHJzwZ1uo+2tbEYIk= -github.com/aws/aws-sdk-go-v2/internal/configsources v1.4.40/go.mod h1:wcEsL6jscjZjVUinb0Q5qD/GXOG1yT3GNfmT9HuDwzU= -github.com/aws/aws-sdk-go-v2/internal/endpoints/v2 v2.7.40 h1:xLQVRDs2NddDmK9BEyh5KSlJ1Gpy5/GIJXrV6WcVGAE= -github.com/aws/aws-sdk-go-v2/internal/endpoints/v2 v2.7.40/go.mod h1:XRXnpFVFGLaEVK+olDdFIM1vNa04ETW452oFGEPUxAo= -github.com/aws/aws-sdk-go-v2/internal/v4a v1.4.41 h1:nv/ILuCY0yXACzMQwvtt/HbqDDjemZiI0AeDbxGQlnU= -github.com/aws/aws-sdk-go-v2/internal/v4a v1.4.41/go.mod h1:dzvOSpxaPqQ3j0xS6Lc1vyVuWW0RBj7s/QqYpzu3Q/0= -github.com/aws/aws-sdk-go-v2/service/ecr v1.61.0 h1:H+odOoYtvBGmUvwLgjq6MN8hmBYBny5R4FosoO+j8NU= -github.com/aws/aws-sdk-go-v2/service/ecr v1.61.0/go.mod h1:HrP4KYHFcZzSEjQNb6yDrWuEes3MeE62PYoDnaff+/0= +github.com/aws/aws-sdk-go-v2/internal/configsources v1.5.2 h1:q/PSLGuRWCChWg+dLnb9dWOnrCxJtnboXbBtFoqqRrI= +github.com/aws/aws-sdk-go-v2/internal/configsources v1.5.2/go.mod h1:TD1jvU2LvXkJexct5vBqcd8QlNXh5EmRUeL/Z32p0n4= +github.com/aws/aws-sdk-go-v2/internal/endpoints/v2 v2.8.2 h1:6fl86IPqKEXoySqiOWdfgbEp9OVbn44zTfEICNEBDhY= +github.com/aws/aws-sdk-go-v2/internal/endpoints/v2 v2.8.2/go.mod h1:63HDfhFkdzBpI8WGXTSKUHPKS6mqldj4u3LJW7RZtSU= +github.com/aws/aws-sdk-go-v2/internal/v4a v1.5.1 h1:yhw5KD1phVyP9vijxOUzDfEtJx+bt+L63k+VfuiYFAA= +github.com/aws/aws-sdk-go-v2/internal/v4a v1.5.1/go.mod h1:ZW2e0d7DYlRxlS9hEiMXE47gTdX5KRN4byUiNbUpG+Q= +github.com/aws/aws-sdk-go-v2/service/ecr v1.64.0 h1:iOYGE9bHGhMQYtbjEcgDJEobWIhKoUvE71m+Jm0vZgU= +github.com/aws/aws-sdk-go-v2/service/ecr v1.64.0/go.mod h1:5ccNgipT/aF9MWzTrKkyGJaCozPt+D6LOD4RFIdP22k= github.com/aws/aws-sdk-go-v2/service/internal/accept-encoding v1.13.19 h1:bAdDl/HkGCcGPoe25ToSHEw23VIxt6CT5fLcg111BKg= github.com/aws/aws-sdk-go-v2/service/internal/accept-encoding v1.13.19/go.mod h1:KaUzbLxv4CeSxh6ZCl9B4m7CuFenS8kUEaDs+f/DQr4= github.com/aws/aws-sdk-go-v2/service/internal/checksum v1.9.18 h1:W/EyPFl9A5rXrtoilfwHYEvzHER+K4SpBPtMXi24Mos= github.com/aws/aws-sdk-go-v2/service/internal/checksum v1.9.18/go.mod h1:UG50K+pvd/uy6xExbobg0rjqFBFZe6I3l75EPDZw4tg= -github.com/aws/aws-sdk-go-v2/service/internal/presigned-url v1.13.40 h1:gr3Fw1cxZXNCdeo/lQ7isHEHzvHVM7z75qb2zW9aMjw= -github.com/aws/aws-sdk-go-v2/service/internal/presigned-url v1.13.40/go.mod h1:8z/9CmfnQhiuXD7Ykbcg4a/whSWsniE0ODSx9uwVzfk= +github.com/aws/aws-sdk-go-v2/service/internal/presigned-url v1.14.1 h1:RmmWQPREQdk9U+PfqeHW3MqZaBaNK7TpV9W3RY+b+7g= +github.com/aws/aws-sdk-go-v2/service/internal/presigned-url v1.14.1/go.mod h1:0A3W4F+68ZnNk5XcNL/e9HFMwnP8RlEicFfy6eOEDyw= github.com/aws/aws-sdk-go-v2/service/internal/s3shared v1.19.25 h1:2pQEbwf+/6EDbiit/GcBE2K4IUpMZymaA0kOz3xK978= github.com/aws/aws-sdk-go-v2/service/internal/s3shared v1.19.25/go.mod h1:KvT6NCcQ0EZ+ZkVRrlBMt04Po3ok23YELEp7WimhLhM= github.com/aws/aws-sdk-go-v2/service/s3 v1.102.2 h1:ie4ElCmUKS26pzrZcIk/lmt4yWjAqLLcawstyQCh298= github.com/aws/aws-sdk-go-v2/service/s3 v1.102.2/go.mod h1:zjsomFeX5duj+4PlMB+o4JoWTIx+G0XMyzjYrUbQkN0= -github.com/aws/aws-sdk-go-v2/service/signin v1.6.0 h1:agcr0j8YeFEzdXNo17Rg9MbbjLRjrimabwNtji4e+lU= -github.com/aws/aws-sdk-go-v2/service/signin v1.6.0/go.mod h1:qU5PxgQ4JiUOOMotzfO3+5oUda5W+8JDVKyLQqlrJik= -github.com/aws/aws-sdk-go-v2/service/sso v1.34.0 h1:FxaN8/sn61DTXNI6Gt678tFJUY8iUsCchm6Y/F/RjaA= -github.com/aws/aws-sdk-go-v2/service/sso v1.34.0/go.mod h1:vu4OY6s8LJtT8BtYG2LD6BGSZMptkYn3o5hvCPB22jc= -github.com/aws/aws-sdk-go-v2/service/ssooidc v1.39.0 h1:crWKPeGYTBTuBxQ3p73kjfJvt4brUIsr+Fuypko8FxY= -github.com/aws/aws-sdk-go-v2/service/ssooidc v1.39.0/go.mod h1:HjjZVhaBz0JBR/kbWKThmNDhFKS7y6EURuk493tJk9Y= -github.com/aws/aws-sdk-go-v2/service/sts v1.46.0 h1:IZ63JdogSNNjex/jsODNv7jGDcO/xJYd9FsgyfCsp1g= -github.com/aws/aws-sdk-go-v2/service/sts v1.46.0/go.mod h1:I+rwAf3spG5dITBaAo3xXRowk8kiOhtU1kYxfvCTC44= +github.com/aws/aws-sdk-go-v2/service/signin v1.8.0 h1:bSvKIoLuRGFqGwASgeCQncCJDi9YKKBDEmCEZzOX1uU= +github.com/aws/aws-sdk-go-v2/service/signin v1.8.0/go.mod h1:9IqUlsJDbUPcg6cgx3WEzXdjrbWzLDQrak0aaSqlTcI= +github.com/aws/aws-sdk-go-v2/service/sso v1.36.0 h1:iivsh357VnfIc18IFWSuoyQEluf8frfWf4cL2Y0JUQw= +github.com/aws/aws-sdk-go-v2/service/sso v1.36.0/go.mod h1:tWuiVBUtPBr8/rgRiYS8Uf85sHcAN+G7XS3D3CEoUh8= +github.com/aws/aws-sdk-go-v2/service/ssooidc v1.41.0 h1:wVxM3QzSKIK8tSN6OGgezp9OK91lCLH2zhmRInN9rFM= +github.com/aws/aws-sdk-go-v2/service/ssooidc v1.41.0/go.mod h1:naFe83jSMuYkH+QjQPX8n1MLhBkeCFM5Lsnh5m5wz3c= +github.com/aws/aws-sdk-go-v2/service/sts v1.48.0 h1:RzZVCzYM19vhJCT5s6vO2wN8ie770Li/TmbAZ9B6N7E= +github.com/aws/aws-sdk-go-v2/service/sts v1.48.0/go.mod h1:mKo/CzaCz8qytGW70NG4vIIGAx1HXTlb5lHNkC5k3lk= github.com/aws/smithy-go v1.28.1 h1:R/nXH00c8qcfCzQVELtRw+eLQWtzv+VAIEFJ1/xxXlQ= github.com/aws/smithy-go v1.28.1/go.mod h1:YE2RhdIuDbA5E5bTdciG9KrW3+TiEONeUWCqxX9i1Fc= github.com/beorn7/perks v1.0.1 h1:VlbKKnNfV8bJzeqoa4cOKqO6bYr3WgKZxO8Z16+hsOM= @@ -569,8 +569,8 @@ github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 h1:Jamvg5psRI github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/prometheus/client_golang v1.24.1 h1:JnJkREXzWxUdCuPFpIWZiPispT9xVV59uiuyR2bPlnU= github.com/prometheus/client_golang v1.24.1/go.mod h1:F+oSRECHg4sse5ucfYpYDeIv/hu68Zo0uoHKetWnzcE= -github.com/prometheus/client_model v0.6.2 h1:oBsgwpGs7iVziMvrGhE53c/GrLUsZdHnqNwqPLxwZyk= -github.com/prometheus/client_model v0.6.2/go.mod h1:y3m2F6Gdpfy6Ut/GBsUqTWZqCUvMVzSfMLjcu6wAwpE= +github.com/prometheus/client_model v0.6.3 h1:O0jaTVAYNxTHYInEPFJt5I3+sN8zqBtVMPTB1qyxiEo= +github.com/prometheus/client_model v0.6.3/go.mod h1:gpN5P9S7Rr6Yr92PiQ+Ixvhf6JZEkF1dnxsYL2aPBEM= github.com/prometheus/common v0.70.1 h1:1HvjP4D5oL3t8RsPlwxA9onvvStjtIHYE5XuuwOi/PY= github.com/prometheus/common v0.70.1/go.mod h1:VdFUQDMZK3VLkurFUVhia6uys/0suUp86TJz5qbJRhc= github.com/prometheus/procfs v0.21.1 h1:GljZCt+zSTS+NZq88cyQ1LjZ+RCHp3uVuabBWA5+OJI= @@ -798,8 +798,8 @@ golang.org/x/sync v0.0.0-20201020160332-67f06af15bc9/go.mod h1:RxMgew5VJxzue5/jJ golang.org/x/sync v0.0.0-20210220032951-036812b2e83c/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20220722155255-886fb9371eb4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.1.0/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= -golang.org/x/sync v0.22.0 h1:SZjpbeLmrCk4xhRSZFNZW5gFUeCeFgjekvI/+gfScek= -golang.org/x/sync v0.22.0/go.mod h1:9xrNwdLfx4jkKbNva9FpL6vEN7evnE43NNNJQ2LF3+0= +golang.org/x/sync v0.23.0 h1:KameEIfc1IkluZyXWLn39Wd4tURc6GbCiISGiZm2bQk= +golang.org/x/sync v0.23.0/go.mod h1:sUUOizhqBxiL6pEWpqNLUiaJn1ShEbZ6BBqskPbjZm0= golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20200323222414-85ca7c5b95cd/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= @@ -861,8 +861,8 @@ google.golang.org/genproto/googleapis/api v0.0.0-20260630182238-925bb5da69e7 h1: google.golang.org/genproto/googleapis/api v0.0.0-20260630182238-925bb5da69e7/go.mod h1:KqHwBx2upmfa1XSi1WuRvC+2VGCLtooKkfmyvRbUmqA= google.golang.org/genproto/googleapis/rpc v0.0.0-20260706201446-f0a921348800 h1:qEHAMpSaUhtD0p3NbEEI83HwNGFxEwaSJ1G9PLnCBZE= google.golang.org/genproto/googleapis/rpc v0.0.0-20260706201446-f0a921348800/go.mod h1:4Hqkh8ycfw05ld/3BWL7rJOSfebL2Q+DVDeRgYgxUU8= -google.golang.org/grpc v1.83.1 h1:HIO0+BEtBP6soyqvqC8sNUjZ7bTs+0hFQuFF+RAy++Y= -google.golang.org/grpc v1.83.1/go.mod h1:kDyl6SKsiHKt0uylY5gtn5cEjkrIOhQOGDgIc4JGwzQ= +google.golang.org/grpc v1.83.2 h1:EManeRomTObA0BU7I8vXgg/78uE5MJ9M8B39EX2WscU= +google.golang.org/grpc v1.83.2/go.mod h1:YPI1hK3kDked6iHvgX3tR0y+nX/qpMFKhPgFsokw1S8= google.golang.org/protobuf v1.36.12 h1:pJOKDDOyeXErUroCihFAd5LQuwXBSpVnKGrj5o/fwxc= google.golang.org/protobuf v1.36.12/go.mod h1:HTf+CrKn2C3g5S8VImy6tdcUvCska2kB7j23XfzDpco= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= @@ -882,30 +882,30 @@ gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= honnef.co/go/tools v0.8.0 h1:UacpzPr7D6i5BAjTkA7sNVcx4kIbhAZcQ4zYtKiXx68= honnef.co/go/tools v0.8.0/go.mod h1:XA+OnlRA9EDh/ukGvXMNSZNKGwFQJ+5dER0ioUkOxks= -modernc.org/cc/v4 v4.29.1 h1:MKgdCV3WykTSPqpVrnxdEDS0HEd2FHpKZDzxzU5LyeI= -modernc.org/cc/v4 v4.29.1/go.mod h1:OnovgIhbbMXMu1aISnJ0wvVD1KnW+cAUJkIrAWh+kVI= -modernc.org/ccgo/v4 v4.34.6 h1:sBgfIwyN0TQ9C5hwIeuqyeAKyMWnbvj2fvpF4L11uzU= -modernc.org/ccgo/v4 v4.34.6/go.mod h1:SZ8YcN9NG7XVsQYdm6jYBvi8PQP1qi+kqB6OhjqI3Fk= +modernc.org/cc/v4 v4.29.2 h1:h6+9ciCnPKutf4I03CvheAvDLX7+IHlqR6Iy6J+cgd8= +modernc.org/cc/v4 v4.29.2/go.mod h1:OnovgIhbbMXMu1aISnJ0wvVD1KnW+cAUJkIrAWh+kVI= +modernc.org/ccgo/v4 v4.35.0 h1:F+TUsmw09QxLzmi3aeYYGxjAXarmZaKgj3mKQHNaA8w= +modernc.org/ccgo/v4 v4.35.0/go.mod h1:qrVGs9S3Sr2Ztcg9ve+kTAYMp5a3YvWjo+SoN06kJ5I= modernc.org/fileutil v1.4.0 h1:j6ZzNTftVS054gi281TyLjHPp6CPHr2KCxEXjEbD6SM= modernc.org/fileutil v1.4.0/go.mod h1:EqdKFDxiByqxLk8ozOxObDSfcVOv/54xDs/DUHdvCUU= modernc.org/gc/v2 v2.6.5 h1:nyqdV8q46KvTpZlsw66kWqwXRHdjIlJOhG6kxiV/9xI= modernc.org/gc/v2 v2.6.5/go.mod h1:YgIahr1ypgfe7chRuJi2gD7DBQiKSLMPgBQe9oIiito= -modernc.org/gc/v3 v3.1.4 h1:2g65LGVSmFQrXeITAw97x7hCRvZFcyE1uDP+7Vng7JI= -modernc.org/gc/v3 v3.1.4/go.mod h1:HFK/6AGESC7Ex+EZJhJ2Gni6cTaYpSMmU/cT9RmlfYY= +modernc.org/gc/v3 v3.1.5 h1:21ldfPfRYE31Tb7B3mwAK8gy1AxP4+dKjrOQPfqakoc= +modernc.org/gc/v3 v3.1.5/go.mod h1:HFK/6AGESC7Ex+EZJhJ2Gni6cTaYpSMmU/cT9RmlfYY= modernc.org/goabi0 v0.2.0 h1:HvEowk7LxcPd0eq6mVOAEMai46V+i7Jrj13t4AzuNks= modernc.org/goabi0 v0.2.0/go.mod h1:CEFRnnJhKvWT1c1JTI3Avm+tgOWbkOu5oPA8eH8LnMI= -modernc.org/libc v1.74.4 h1:fX1Omw4o2/1C2iRkkIsrQTasJQldLhRmuPreXLoWs9k= -modernc.org/libc v1.74.4/go.mod h1:eeQAS9W3sZeKYMFubydxJpII9ybHWshk+7or7bLG9co= +modernc.org/libc v1.75.6 h1:yKk8qo+Di4gkmvRboK8ocCqH22FiUCR6jRy2OwtCRus= +modernc.org/libc v1.75.6/go.mod h1:bO5o2ztHxBb2rjz0PgdHN0sSMw57CgxGFLZ3Qd/QpVQ= modernc.org/mathutil v1.7.1 h1:GCZVGXdaN8gTqB1Mf/usp1Y/hSqgI2vAGGP4jZMCxOU= modernc.org/mathutil v1.7.1/go.mod h1:4p5IwJITfppl0G4sUEDtCr4DthTaT47/N3aT6MhfgJg= -modernc.org/memory v1.11.0 h1:o4QC8aMQzmcwCK3t3Ux/ZHmwFPzE6hf2Y5LbkRs+hbI= -modernc.org/memory v1.11.0/go.mod h1:/JP4VbVC+K5sU2wZi9bHoq2MAkCnrt2r98UGeSK7Mjw= +modernc.org/memory v1.12.1 h1:nFMiWrpStgZczNl6XI9GnIk/rWhYIyHGUaR04pGbp9g= +modernc.org/memory v1.12.1/go.mod h1:/JP4VbVC+K5sU2wZi9bHoq2MAkCnrt2r98UGeSK7Mjw= modernc.org/opt v0.2.0 h1:tGyef5ApycA7FSEOMraay9SaTk5zmbx7Tu+cJs4QKZg= modernc.org/opt v0.2.0/go.mod h1:03fq9lsNfvkYSfxrfUhZCWPk1lm4cq4N+Bh//bEtgns= modernc.org/sortutil v1.2.1 h1:+xyoGf15mM3NMlPDnFqrteY07klSFxLElE2PVuWIJ7w= modernc.org/sortutil v1.2.1/go.mod h1:7ZI3a3REbai7gzCLcotuw9AC4VZVpYMjDzETGsSMqJE= -modernc.org/sqlite v1.57.0 h1:qNQP6xnx5M0ISNtlnxoOX0+cD5bJ0/gr9aMmndFczzg= -modernc.org/sqlite v1.57.0/go.mod h1:yCJ2cmAaIkHQ25oXWrF8H4O1lIfPYPR26yCEDj2P3pQ= +modernc.org/sqlite v1.58.0 h1:38u40/bwkfM7f0Myhosl+SEMltSDxnGdQf8o6Kjmys0= +modernc.org/sqlite v1.58.0/go.mod h1:rsD2CckafgObKC4DhBlGBf+RiHxkc3hINGt1Xw32tVY= modernc.org/strutil v1.2.1 h1:UneZBkQA+DX2Rp35KcM69cSsNES9ly8mQWD71HKlOA0= modernc.org/strutil v1.2.1/go.mod h1:EHkiggD70koQxjVdSBM3JKM7k6L0FbGE5eymy9i3B9A= modernc.org/token v1.1.0 h1:Xl7Ap9dKaEs5kLoOQeQmPWevfnk/DM5qcLcYlA8ys6Y= diff --git a/internal/config/config.go b/internal/config/config.go index 137d7f7..5cccd13 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -137,7 +137,9 @@ type Config struct { // HTTPTimeout is the timeout for individual upstream HTTP requests made // by protocol handlers (metadata fetches, pass-through file requests). // Uses Go duration syntax (e.g. "30s", "2m"). Default: "30s". - // Set to "0" to disable the timeout entirely. + // Set to "0" to disable the timeout entirely. Independently of this + // setting, the shared transport gives up on an upstream that has not sent + // response headers within 60 seconds. HTTPTimeout string `json:"http_timeout" yaml:"http_timeout"` // MirrorAPI enables the /api/mirror endpoints for starting mirror jobs via HTTP. diff --git a/internal/database/database.go b/internal/database/database.go index eded6d2..6b326ec 100644 --- a/internal/database/database.go +++ b/internal/database/database.go @@ -4,6 +4,7 @@ import ( "fmt" "os" "path/filepath" + "time" "github.com/jmoiron/sqlx" _ "github.com/lib/pq" @@ -14,6 +15,16 @@ const SchemaVersion = 1 const dirPermissions = 0755 +// Postgres connection pool limits. database/sql keeps only two idle +// connections by default, which opens a new Postgres session for almost every +// request under load. +const ( + postgresMaxOpenConns = 32 + postgresMaxIdleConns = 32 + postgresConnMaxIdleTime = 5 * time.Minute + postgresConnMaxLifetime = 30 * time.Minute +) + type Dialect string const ( @@ -94,6 +105,11 @@ func OpenPostgres(url string) (*DB, error) { return nil, fmt.Errorf("opening postgres database: %w", err) } + sqlDB.SetMaxOpenConns(postgresMaxOpenConns) + sqlDB.SetMaxIdleConns(postgresMaxIdleConns) + sqlDB.SetConnMaxIdleTime(postgresConnMaxIdleTime) + sqlDB.SetConnMaxLifetime(postgresConnMaxLifetime) + if err := sqlDB.Ping(); err != nil { _ = sqlDB.Close() return nil, fmt.Errorf("connecting to postgres: %w", err) diff --git a/internal/database/database_test.go b/internal/database/database_test.go index 0fd42fc..bb73bc8 100644 --- a/internal/database/database_test.go +++ b/internal/database/database_test.go @@ -610,8 +610,10 @@ func createTestPostgresDB(t *testing.T) *DB { t.Fatalf("OpenPostgres failed: %v", err) } - // Drop and recreate tables for clean test state - tables := []string{"artifacts", "versions", "packages", "schema_info"} + // Drop and recreate every table CreateSchema creates for clean test state; + // leftover migration records make the next CreateSchema fail on the + // migrations primary key. + tables := []string{"artifacts", "versions", "packages", "vulnerabilities", "metadata_cache", "migrations", "schema_info"} for _, table := range tables { _, _ = db.Exec("DROP TABLE IF EXISTS " + table + " CASCADE") } diff --git a/internal/database/postgres_pool_test.go b/internal/database/postgres_pool_test.go new file mode 100644 index 0000000..bd39bee --- /dev/null +++ b/internal/database/postgres_pool_test.go @@ -0,0 +1,57 @@ +package database + +import ( + "context" + "database/sql" + "os" + "testing" +) + +// TestOpenPostgresKeepsConnectionsIdle checks the connection-count limits +// OpenPostgres sets: the open cap admits a burst of postgresMaxIdleConns +// connections, and releasing them again leaves all of them idle in the pool. +// database/sql's default keeps only two, so the next burst would open a new +// Postgres session for almost every request. The idle-time and lifetime +// settings are not exercised here. +func TestOpenPostgresKeepsConnectionsIdle(t *testing.T) { + url := os.Getenv("PROXY_DATABASE_URL") + if url == "" { + t.Skip("PROXY_DATABASE_URL not set, skipping postgres pool test") + } + + db, err := OpenPostgres(url) + if err != nil { + t.Fatalf("OpenPostgres failed: %v", err) + } + defer func() { _ = db.Close() }() + + const burst = postgresMaxIdleConns + // database/sql keeps two idle connections by default; a burst that small + // could not tell the tuned pool from the default one. + if burst <= 2 { + t.Fatalf("postgresMaxIdleConns = %d, want more than database/sql's default of 2", burst) + } + if got := db.Stats().MaxOpenConnections; got <= 0 || got < burst { + t.Fatalf("MaxOpenConnections = %d, want a cap of at least %d", got, burst) + } + + conns := make([]*sql.Conn, 0, burst) + for range burst { + conn, err := db.Conn(context.Background()) + if err != nil { + t.Fatalf("taking connection %d: %v", len(conns)+1, err) + } + t.Cleanup(func() { _ = conn.Close() }) // release the session if an assertion below fails + conns = append(conns, conn) + } + if got := db.Stats().InUse; got != burst { + t.Fatalf("InUse = %d while holding %d connections", got, burst) + } + + for _, conn := range conns { + _ = conn.Close() + } + if got := db.Stats().Idle; got != burst { + t.Errorf("Idle = %d after releasing %d connections, want all of them kept", got, burst) + } +} diff --git a/internal/handler/coalesce_semantics_test.go b/internal/handler/coalesce_semantics_test.go new file mode 100644 index 0000000..c391d74 --- /dev/null +++ b/internal/handler/coalesce_semantics_test.go @@ -0,0 +1,615 @@ +package handler + +import ( + "context" + "errors" + "io" + "strings" + "sync" + "testing" + "time" + + "github.com/git-pkgs/artifacts" + "github.com/git-pkgs/registries/fetch" +) + +// runConcurrent runs fn in n goroutines released together and returns their errors. +func runConcurrent(n int, fn func(i int) error) []error { + errs := make([]error, n) + start := make(chan struct{}) + var wg sync.WaitGroup + for i := 0; i < n; i++ { + wg.Add(1) + go func(i int) { + defer wg.Done() + <-start + errs[i] = fn(i) + }(i) + } + close(start) + wg.Wait() + return errs +} + +// artifactBody builds a one-shot upstream artifact carrying the given bytes. +func artifactBody(content string) *fetch.Artifact { + return &fetch.Artifact{ + Body: io.NopCloser(strings.NewReader(content)), + ContentType: "application/gzip", + } +} + +// drain consumes and closes a CacheResult reader, if there is one. +func drain(res *CacheResult) { + if res != nil && res.Reader != nil { + _, _ = io.Copy(io.Discard, res.Reader) + _ = res.Reader.Close() + } +} + +// TestCoalesceKey_DifferentUpstreamHashDoesNotShare is the safety property that +// makes coalescing sound: callers expecting different bytes must never share a +// fetch, so a re-published version cannot serve stale bytes to a caller that +// asked for the new digest. +func TestCoalesceKey_DifferentUpstreamHashDoesNotShare(t *testing.T) { + const content = "artifact bytes" + proxy, _, _, _ := setupTestProxy(t) + fetcher := &countingFetcher{content: content, delay: fetchHoldTime} + proxy.Fetcher = fetcher + + // The digest must carry the "sha256:" prefix; without it the API treats the + // value as unverifiable and clears the hash, which would legitimately let + // the two callers share one fetch. + hashes := []string{ + "sha256:" + sha256Hex(content), + "sha256:" + sha256Hex("something else entirely"), + } + + _ = runConcurrent(2, func(i int) error { + res, err := proxy.GetOrFetchArtifactFromURLWithDigest(context.Background(), + "npm", "pkg", "1.0.0", "pkg-1.0.0.tgz", + "https://registry.npmjs.org/pkg/-/pkg-1.0.0.tgz", hashes[i]) + drain(res) + return err + }) + + if got := fetcher.calls.Load(); got != 2 { + t.Errorf("upstream fetches = %d, want 2: callers expecting different digests must not share a fetch", got) + } +} + +// TestCoalesceKey_HashCasingSharesOneFetch is the other half of that property. +// artifactHashMatches compares digests case-insensitively, so one digest in two +// casings describes one artifact and must not split into two fetches. +func TestCoalesceKey_HashCasingSharesOneFetch(t *testing.T) { + const content = "artifact bytes" + proxy, _, _, _ := setupTestProxy(t) + fetcher := &countingFetcher{content: content, delay: fetchHoldTime} + proxy.Fetcher = fetcher + + hex := sha256Hex(content) + digests := []string{"sha256:" + hex, "sha256:" + strings.ToUpper(hex)} + + for i, err := range runConcurrent(2, func(i int) error { + res, err := proxy.GetOrFetchArtifactFromURLWithDigest(context.Background(), + "npm", "pkg", "1.0.0", "pkg-1.0.0.tgz", + "https://registry.npmjs.org/pkg/-/pkg-1.0.0.tgz", digests[i]) + drain(res) + return err + }) { + if err != nil { + t.Fatalf("caller %d failed: %v", i, err) + } + } + + if got := fetcher.calls.Load(); got != 1 { + t.Errorf("upstream fetches = %d, want 1: one digest in two casings is one artifact", got) + } +} + +// TestCoalesceKey_DifferentDownloadURLDoesNotShare covers the other half of the +// key: same package, different upstream URL, must not collapse into one fetch. +func TestCoalesceKey_DifferentDownloadURLDoesNotShare(t *testing.T) { + proxy, _, _, _ := setupTestProxy(t) + fetcher := &countingFetcher{content: "artifact bytes", delay: fetchHoldTime} + proxy.Fetcher = fetcher + + urls := []string{ + "https://registry.npmjs.org/pkg/-/pkg-1.0.0.tgz", + "https://mirror.example.com/pkg/-/pkg-1.0.0.tgz", + } + + _ = runConcurrent(2, func(i int) error { + res, err := proxy.GetOrFetchArtifactFromURL(context.Background(), + "npm", "pkg", "1.0.0", "pkg-1.0.0.tgz", urls[i]) + drain(res) + return err + }) + + if got := fetcher.calls.Load(); got != 2 { + t.Errorf("upstream fetches = %d, want 2: different upstream URLs must not share a fetch", got) + } +} + +// TestCoalesceKey_DistinctArtifactsDoNotSerialize guards against an over-broad +// key: four packages fetched at once must still produce four fetches. +func TestCoalesceKey_DistinctArtifactsDoNotSerialize(t *testing.T) { + const n = 4 + proxy, _, _, _ := setupTestProxy(t) + fetcher := &countingFetcher{content: "artifact bytes", delay: fetchHoldTime} + proxy.Fetcher = fetcher + + names := []string{"alpha", "beta", "gamma", "delta"} + errs := runConcurrent(n, func(i int) error { + res, err := proxy.GetOrFetchArtifactFromURL(context.Background(), + "npm", names[i], "1.0.0", names[i]+"-1.0.0.tgz", + "https://registry.npmjs.org/"+names[i]+"/-/"+names[i]+"-1.0.0.tgz") + drain(res) + return err + }) + for i, err := range errs { + if err != nil { + t.Errorf("caller %d (%s): %v", i, names[i], err) + } + } + if got := fetcher.calls.Load(); got != n { + t.Errorf("upstream fetches = %d, want %d: distinct artifacts must not share a fetch", got, n) + } +} + +// TestCoalesce_FailedFetchReachesEveryCallerAndIsRetriable verifies both claims +// in coalesceFetch's doc comment: a failed fetch reaches every caller sharing +// it, and the key is released so a later request retries. +func TestCoalesce_FailedFetchReachesEveryCallerAndIsRetriable(t *testing.T) { + const callers = 8 + proxy, _, _, fetcher := setupTestProxy(t) + boom := errors.New("upstream unavailable") + fetcher.fetchErr = boom + + errs := runConcurrent(callers, func(int) error { + res, err := proxy.GetOrFetchArtifactFromURL(context.Background(), + "npm", "pkg", "1.0.0", "pkg-1.0.0.tgz", + "https://registry.npmjs.org/pkg/-/pkg-1.0.0.tgz") + drain(res) + return err + }) + for i, err := range errs { + if err == nil { + t.Errorf("caller %d: got nil error, want the shared fetch's failure", i) + } else if !errors.Is(err, boom) { + t.Errorf("caller %d: got %v, want it to wrap %v", i, err, boom) + } + } + + // The key must be released: a later request retries rather than inheriting + // the failure. + fetcher.fetchErr = nil + fetcher.artifact = artifactBody("recovered bytes") + res, err := proxy.GetOrFetchArtifactFromURL(context.Background(), + "npm", "pkg", "1.0.0", "pkg-1.0.0.tgz", + "https://registry.npmjs.org/pkg/-/pkg-1.0.0.tgz") + if err != nil { + t.Fatalf("retry after failed coalesced fetch: %v", err) + } + body, _ := io.ReadAll(res.Reader) + _ = res.Reader.Close() + if string(body) != "recovered bytes" { + t.Errorf("retry body = %q, want %q", body, "recovered bytes") + } +} + +// TestCoalesce_ResolverPath covers the other entry point: GetOrFetchArtifact +// resolves the URL itself, so it is keyed without one. +func TestCoalesce_ResolverPath(t *testing.T) { + const callers = 8 + proxy, _, _, _ := setupTestProxy(t) + fetcher := &countingFetcher{content: "resolved artifact bytes", delay: fetchHoldTime} + proxy.Fetcher = fetcher + + errs := runConcurrent(callers, func(int) error { + res, err := proxy.GetOrFetchArtifact(context.Background(), + "npm", "left-pad", "1.3.0", "left-pad-1.3.0.tgz") + drain(res) + return err + }) + for i, err := range errs { + if err != nil { + t.Errorf("caller %d: %v", i, err) + } + } + if got := fetcher.calls.Load(); got != 1 { + t.Errorf("upstream fetches = %d, want 1", got) + } +} + +// TestCoalesce_ResolverPathEmptyFilename exercises that path when the filename +// is left to be resolved, which the key cannot know up front. +func TestCoalesce_ResolverPathEmptyFilename(t *testing.T) { + const callers = 8 + proxy, _, _, _ := setupTestProxy(t) + fetcher := &countingFetcher{content: "resolved artifact bytes", delay: fetchHoldTime} + proxy.Fetcher = fetcher + + errs := runConcurrent(callers, func(int) error { + res, err := proxy.GetOrFetchArtifact(context.Background(), "npm", "left-pad", "1.3.0", "") + drain(res) + return err + }) + for i, err := range errs { + if err != nil { + t.Errorf("caller %d: %v", i, err) + } + } + if got := fetcher.calls.Load(); got != 1 { + t.Errorf("upstream fetches = %d, want 1", got) + } +} + +// TestCoalesce_SubsequentRequestIsACacheHit confirms the coalesced fetch was +// committed and is visible later, not just streamed to the waiting callers. +func TestCoalesce_SubsequentRequestIsACacheHit(t *testing.T) { + const callers = 8 + const url = "https://registry.npmjs.org/pkg/-/pkg-1.0.0.tgz" + proxy, _, _, _ := setupTestProxy(t) + fetcher := &countingFetcher{content: "artifact bytes", delay: fetchHoldTime} + proxy.Fetcher = fetcher + + _ = runConcurrent(callers, func(int) error { + res, err := proxy.GetOrFetchArtifactFromURL(context.Background(), + "npm", "pkg", "1.0.0", "pkg-1.0.0.tgz", url) + drain(res) + return err + }) + + res, err := proxy.GetOrFetchArtifactFromURL(context.Background(), + "npm", "pkg", "1.0.0", "pkg-1.0.0.tgz", url) + if err != nil { + t.Fatalf("follow-up request: %v", err) + } + defer func() { _ = res.Reader.Close() }() + if !res.Cached { + t.Error("follow-up request should be served from cache") + } + if got := fetcher.calls.Load(); got != 1 { + t.Errorf("upstream fetches = %d, want 1 after a follow-up cache hit", got) + } +} + +// TestCoalesce_ReadersAreIndependent guards openStoredArtifact: callers sharing +// a fetch each need their own reader, or one closing early breaks the rest. +func TestCoalesce_ReadersAreIndependent(t *testing.T) { + const callers = 8 + const content = "artifact bytes that every caller must receive intact" + proxy, _, _, _ := setupTestProxy(t) + fetcher := &countingFetcher{content: content, delay: fetchHoldTime} + proxy.Fetcher = fetcher + + results := make([]*CacheResult, callers) + errs := runConcurrent(callers, func(i int) error { + res, err := proxy.GetOrFetchArtifactFromURL(context.Background(), + "npm", "pkg", "1.0.0", "pkg-1.0.0.tgz", + "https://registry.npmjs.org/pkg/-/pkg-1.0.0.tgz") + results[i] = res + return err + }) + for i, err := range errs { + if err != nil { + t.Fatalf("caller %d: %v", i, err) + } + } + + // Close the first caller's reader before anyone else has read a byte. + _ = results[0].Reader.Close() + + for i := 1; i < callers; i++ { + body, err := io.ReadAll(results[i].Reader) + _ = results[i].Reader.Close() + if err != nil { + t.Errorf("caller %d read after another caller closed: %v", i, err) + continue + } + if string(body) != content { + t.Errorf("caller %d got %q, want %q", i, body, content) + } + } +} + +// TestCoalesce_CanceledWaiterDoesNotWaitForTheSharedFetch checks that joining a +// coalesced fetch does not cost a caller its own cancellation. Without the +// leader/waiter split a waiter is pinned until the shared fetch resolves, +// bounded only by the artifact client timeout, so clients that have already +// gone away keep handler goroutines alive for minutes. +func TestCoalesce_CanceledWaiterDoesNotWaitForTheSharedFetch(t *testing.T) { + const leaderFetch = 2 * time.Second + const url = "https://registry.npmjs.org/pkg/-/pkg-1.0.0.tgz" + + proxy, _, _, _ := setupTestProxy(t) + fetcher := &countingFetcher{content: "artifact bytes", delay: leaderFetch, entered: make(chan struct{})} + proxy.Fetcher = fetcher + + leaderDone := make(chan error, 1) + go func() { + res, err := proxy.GetOrFetchArtifactFromURL(context.Background(), + "npm", "pkg", "1.0.0", "pkg-1.0.0.tgz", url) + drain(res) + leaderDone <- err + }() + + select { + case <-fetcher.entered: // the leader holds the key and is inside its fetch + case <-time.After(5 * time.Second): + t.Fatal("leader never started its fetch") + } + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + start := time.Now() + _, err := proxy.GetOrFetchArtifactFromURL(ctx, "npm", "pkg", "1.0.0", "pkg-1.0.0.tgz", url) + blocked := time.Since(start) + + if !errors.Is(err, context.Canceled) { + t.Errorf("waiter error = %v, want context.Canceled", err) + } + if blocked > leaderFetch/4 { + t.Errorf("canceled waiter blocked %v, want well under %v: it is pinned to the shared fetch", + blocked, leaderFetch/4) + } + + // A waiter leaving must not disturb the fetch the others share. + if err := <-leaderDone; err != nil { + t.Fatalf("leader failed after a waiter canceled: %v", err) + } + res, err := proxy.GetOrFetchArtifactFromURL(context.Background(), + "npm", "pkg", "1.0.0", "pkg-1.0.0.tgz", url) + if err != nil { + t.Fatalf("follow-up after leader completed: %v", err) + } + defer func() { _ = res.Reader.Close() }() + if !res.Cached { + t.Error("leader's fetch should have been committed to the cache") + } + if got := fetcher.calls.Load(); got != 1 { + t.Errorf("upstream fetches = %d, want 1", got) + } +} + +// inFlightLen reports how many coalesced fetches are currently registered. +func inFlightLen(p *Proxy) int { + p.fetchMu.Lock() + defer p.fetchMu.Unlock() + return len(p.inFlight) +} + +// TestCoalesce_KeyIsReleasedAfterFetch guards the bug this hand-rolled map can +// have that singleflight could not: a key left behind means later callers join +// a finished entry, see its closed done channel, and are served that stale +// result forever, while the map grows without bound. +func TestCoalesce_KeyIsReleasedAfterFetch(t *testing.T) { + const url = "https://registry.npmjs.org/pkg/-/pkg-1.0.0.tgz" + proxy, _, _, _ := setupTestProxy(t) + fetcher := &countingFetcher{content: "artifact bytes", delay: fetchHoldTime} + proxy.Fetcher = fetcher + + _ = runConcurrent(8, func(int) error { + res, err := proxy.GetOrFetchArtifactFromURL(context.Background(), + "npm", "pkg", "1.0.0", "pkg-1.0.0.tgz", url) + drain(res) + return err + }) + if n := inFlightLen(proxy); n != 0 { + t.Errorf("in-flight entries after a successful fetch = %d, want 0", n) + } + + // A fresh miss for the same key must start a new fetch, not rejoin the old + // entry. Clearing the cache record forces the miss path again. + if err := proxy.ClearCachedArtifact(context.Background(), "npm", "pkg", "1.0.0", "pkg-1.0.0.tgz"); err != nil { + t.Fatalf("clear cached artifact: %v", err) + } + res, err := proxy.GetOrFetchArtifactFromURL(context.Background(), + "npm", "pkg", "1.0.0", "pkg-1.0.0.tgz", url) + if err != nil { + t.Fatalf("second miss for the same key: %v", err) + } + drain(res) + if got := fetcher.calls.Load(); got != 2 { + t.Errorf("upstream fetches = %d, want 2: the second miss must not reuse the finished entry", got) + } + if n := inFlightLen(proxy); n != 0 { + t.Errorf("in-flight entries at end = %d, want 0", n) + } +} + +// missingFromCache is a recheck that always reports a miss, so the shared fetch +// runs. +func missingFromCache() (artifacts.Artifact, string, bool) { + return artifacts.Artifact{}, "", false +} + +// TestCoalesce_LeaderRechecksCacheBeforeFetching covers the window between a +// caller's own cache lookup and it becoming the leader: a concurrent fetch can +// commit the artifact in that gap, and the leader must serve that rather than +// fetch it a second time. +func TestCoalesce_LeaderRechecksCacheBeforeFetching(t *testing.T) { + const content = "artifact bytes" + proxy, _, store, _ := setupTestProxy(t) + + const storagePath = "npm/pkg/1.0.0/pkg-1.0.0.tgz" + if _, _, err := store.Store(context.Background(), storagePath, strings.NewReader(content)); err != nil { + t.Fatalf("seeding storage: %v", err) + } + committed := artifacts.Artifact{ + PURL: "pkg:npm/pkg@1.0.0", + Filename: "pkg-1.0.0.tgz", + Size: int64(len(content)), + } + + res, err := proxy.coalesceFetch(context.Background(), "any-key", + func() (artifacts.Artifact, string, bool) { return committed, storagePath, true }, + func(context.Context) (artifacts.Artifact, string, error) { + t.Error("fetched an artifact that was already in the cache") + return artifacts.Artifact{}, "", errors.New("commit must not run") + }) + if err != nil { + t.Fatalf("coalesceFetch failed: %v", err) + } + defer drain(res) + got, err := io.ReadAll(res.Reader) + if err != nil { + t.Fatalf("reading result: %v", err) + } + if string(got) != content { + t.Errorf("got %q, want %q", got, content) + } + if n := inFlightLen(proxy); n != 0 { + t.Errorf("in-flight entries = %d, want 0", n) + } +} + +// TestCachedArtifactRecord covers the recheck itself: it must report the row a +// concurrent fetch committed, match its digest the way artifactHashMatches +// does, and report a miss for anything else. +func TestCachedArtifactRecord(t *testing.T) { + const ( + content = "artifact bytes" + pkgPURL = "pkg:npm/pkg" + versionPURL = "pkg:npm/pkg@1.0.0" + filename = "pkg-1.0.0.tgz" + storagePath = "npm/pkg/1.0.0/pkg-1.0.0.tgz" + ) + proxy, _, _, _ := setupTestProxy(t) + + hex := sha256Hex(content) + committed := testArtifact(content, versionPURL, filename, "application/gzip") + if err := proxy.updateCacheDB("npm", "pkg", pkgPURL, + "https://registry.npmjs.org/pkg/-/pkg-1.0.0.tgz", storagePath, committed); err != nil { + t.Fatalf("seeding cache record: %v", err) + } + + for _, tc := range []struct { + name, filename, hash string + want bool + }{ + {"no upstream hash", filename, "", true}, + {"matching hash", filename, hex, true}, + {"matching hash in upper case", filename, strings.ToUpper(hex), true}, + {"different hash", filename, sha256Hex("something else entirely"), false}, + {"unknown filename", "pkg-1.0.0.zip", hex, false}, + } { + t.Run(tc.name, func(t *testing.T) { + got, path, ok := proxy.cachedArtifactRecord(pkgPURL, versionPURL, tc.filename, tc.hash) + if ok != tc.want { + t.Fatalf("ok = %v, want %v", ok, tc.want) + } + if !ok { + return + } + if path != storagePath { + t.Errorf("storage path = %q, want %q", path, storagePath) + } + if got.Digest.Encoded() != hex { + t.Errorf("digest = %q, want %q", got.Digest.Encoded(), hex) + } + }) + } +} + +// TestCoalesce_LeaderFetchesWhenRecheckedBytesAreGone covers the other branch +// of the recheck: a record whose bytes no longer open is not served, and the +// shared fetch runs instead, the same recovery the cache lookup makes. +func TestCoalesce_LeaderFetchesWhenRecheckedBytesAreGone(t *testing.T) { + const content = "fetched bytes" + const storagePath = "npm/pkg/1.0.0/pkg-1.0.0.tgz" + proxy, _, store, _ := setupTestProxy(t) + + stale := artifacts.Artifact{PURL: "pkg:npm/pkg@1.0.0", Filename: "pkg-1.0.0.tgz"} + if _, err := store.Open(context.Background(), storagePath); err == nil { + t.Fatal("stale bytes were present, so the test proves nothing") + } + + // The leader runs commit on its own goroutine, so a plain counter is safe. + fetches := 0 + res, err := proxy.coalesceFetch(context.Background(), "any-key", + func() (artifacts.Artifact, string, bool) { return stale, storagePath, true }, + func(ctx context.Context) (artifacts.Artifact, string, error) { + fetches++ + if _, _, err := store.Store(ctx, storagePath, strings.NewReader(content)); err != nil { + return artifacts.Artifact{}, "", err + } + return testArtifact(content, stale.PURL, stale.Filename, "application/gzip"), storagePath, nil + }) + if err != nil { + t.Fatalf("coalesceFetch failed: %v", err) + } + defer drain(res) + if fetches != 1 { + t.Errorf("shared fetches = %d, want 1: a record without bytes must be refetched", fetches) + } + got, err := io.ReadAll(res.Reader) + if err != nil { + t.Fatalf("reading result: %v", err) + } + if string(got) != content { + t.Errorf("got %q, want %q", got, content) + } + if n := inFlightLen(proxy); n != 0 { + t.Errorf("in-flight entries = %d, want 0", n) + } +} + +// TestCoalesce_PanicInSharedFetchDoesNotStrandWaiters checks the failure mode +// that matters most: a caller parked on a shared fetch must never be left +// blocked forever when that fetch dies. +// +// This drives coalesceFetch directly and holds the shared entry itself, because +// whether a second caller has reached the wait is not observable from outside: +// it runs a cache lookup against the database first, so releasing the leader on +// a timer races that query. Losing the race made a second caller the leader +// instead of a waiter, and its panic was unrecovered, killing the test binary +// rather than failing the test. +func TestCoalesce_PanicInSharedFetchDoesNotStrandWaiters(t *testing.T) { + proxy, _, _, _ := setupTestProxy(t) + const key = "pkg:npm/pkg@1.0.0\x00pkg-1.0.0.tgz" + + inCommit := make(chan struct{}) + release := make(chan struct{}) + leaderPanicked := make(chan struct{}) + + go func() { + defer func() { + _ = recover() // the panic surfaces in the leader, as it would in a handler + close(leaderPanicked) + }() + _, _ = proxy.coalesceFetch(context.Background(), key, missingFromCache, + func(context.Context) (artifacts.Artifact, string, error) { + close(inCommit) + <-release + panic("upstream fetch exploded") + }) + }() + + <-inCommit // the leader holds the key and is inside the fetch + + // Take the entry a waiter would park on, while the leader is still held. + proxy.fetchMu.Lock() + shared := proxy.inFlight[key] + proxy.fetchMu.Unlock() + if shared == nil { + t.Fatal("no in-flight entry registered for a running fetch") + } + + close(release) + + select { + case <-shared.done: + case <-time.After(5 * time.Second): + t.Fatal("waiter stranded: a panicking shared fetch never released its waiters") + } + if !errors.Is(shared.err, errSharedFetchAbandoned) { + t.Errorf("waiter error = %v, want errSharedFetchAbandoned", shared.err) + } + + <-leaderPanicked + if n := inFlightLen(proxy); n != 0 { + t.Errorf("in-flight entries after a panic = %d, want 0", n) + } +} diff --git a/internal/handler/coalesce_test.go b/internal/handler/coalesce_test.go new file mode 100644 index 0000000..baf052d --- /dev/null +++ b/internal/handler/coalesce_test.go @@ -0,0 +1,185 @@ +package handler + +import ( + "bytes" + "context" + "io" + "log/slog" + "net/http" + "path/filepath" + "strings" + "sync" + "sync/atomic" + "testing" + "time" + + "github.com/git-pkgs/proxy/internal/database" + "github.com/git-pkgs/proxy/internal/storage" + "github.com/git-pkgs/registries/fetch" +) + +// fetchHoldTime holds each stub fetch open long enough that concurrent callers +// reliably overlap inside it. The exact value is not significant. +const fetchHoldTime = 50 * time.Millisecond + +// countingFetcher counts upstream fetches and holds each one open. +type countingFetcher struct { + calls atomic.Int64 + content string + delay time.Duration + + // entered, if set, is closed when the first fetch begins. A test can wait + // on it to know the leader holds the key, rather than guessing with a + // sleep. + entered chan struct{} + enterOnce sync.Once +} + +func (f *countingFetcher) Fetch(ctx context.Context, url string) (*fetch.Artifact, error) { + return f.FetchWithHeaders(ctx, url, nil) +} + +func (f *countingFetcher) FetchWithHeaders(_ context.Context, _ string, _ http.Header) (*fetch.Artifact, error) { + f.calls.Add(1) + if f.entered != nil { + f.enterOnce.Do(func() { close(f.entered) }) + } + time.Sleep(f.delay) + return &fetch.Artifact{ + Body: io.NopCloser(strings.NewReader(f.content)), + ContentType: "application/gzip", + }, nil +} + +func (f *countingFetcher) Head(context.Context, string) (int64, string, error) { + return 0, "", nil +} + +// TestGetOrFetchArtifactFromURL_ConcurrentMissesCoalesce asserts that N +// simultaneous misses for one artifact produce a single upstream fetch. That is +// the CI shape: parallel jobs installing overlapping dependencies cold. +func TestGetOrFetchArtifactFromURL_ConcurrentMissesCoalesce(t *testing.T) { + const goroutines = 8 + const content = "left-pad tarball bytes" + + proxy, _, _, _ := setupTestProxy(t) + fetcher := &countingFetcher{content: content, delay: fetchHoldTime} + proxy.Fetcher = fetcher + + start := make(chan struct{}) + var wg sync.WaitGroup + errs := make([]error, goroutines) + bodies := make([]string, goroutines) + + for i := 0; i < goroutines; i++ { + wg.Add(1) + go func(i int) { + defer wg.Done() + <-start + res, err := proxy.GetOrFetchArtifactFromURL(context.Background(), + "npm", "left-pad", "1.3.0", "left-pad-1.3.0.tgz", + "https://registry.npmjs.org/left-pad/-/left-pad-1.3.0.tgz") + if err != nil { + errs[i] = err + return + } + defer func() { _ = res.Reader.Close() }() + b, err := io.ReadAll(res.Reader) + errs[i] = err + bodies[i] = string(b) + }(i) + } + + close(start) + wg.Wait() + + for i, err := range errs { + if err != nil { + t.Errorf("goroutine %d: unexpected error: %v", i, err) + } + } + // Every caller must get its own intact copy of the bytes. + for i, b := range bodies { + if b != content { + t.Errorf("goroutine %d: body = %q, want %q", i, b, content) + } + } + if got := fetcher.calls.Load(); got != 1 { + t.Errorf("upstream fetches = %d, want 1 (%d concurrent callers stampeded the upstream)", got, goroutines) + } +} + +// TestGetOrFetchArtifactFromURL_ConcurrentMissesFileStorage runs the same +// scenario against the real file:// backend, the default in production. +// +// Uncoalesced this fails outright, not merely wastefully. Every caller stores +// to one key, and fileblob rewrites a ".attrs" sidecar per key with os.Create, +// truncating in place outside the rename that protects the blob. Decoding that +// sidecar mid-truncate gives "opening reader: EOF", served as a 502. +// +// Only the fetcher is stubbed, because the real one refuses loopback so an +// httptest upstream is unreachable. The storage, where this fails, is real. +func TestGetOrFetchArtifactFromURL_ConcurrentMissesFileStorage(t *testing.T) { + const goroutines = 16 + content := bytes.Repeat([]byte("tarball-bytes-"), 512) + + ctx := context.Background() + dir := t.TempDir() + + db, err := database.Create(filepath.Join(dir, "test.db")) + if err != nil { + t.Fatalf("create database: %v", err) + } + t.Cleanup(func() { _ = db.Close() }) + + store, err := storage.OpenBucket(ctx, "file://"+filepath.Join(dir, "cache")) + if err != nil { + t.Fatalf("open storage: %v", err) + } + t.Cleanup(func() { _ = store.Close() }) + + fetcher := &countingFetcher{content: string(content), delay: fetchHoldTime} + proxy := NewProxy(db, store, fetcher, fetch.NewResolver(), + slog.New(slog.NewTextHandler(io.Discard, nil))) + + start := make(chan struct{}) + var wg sync.WaitGroup + errs := make([]error, goroutines) + bodies := make([][]byte, goroutines) + + for i := 0; i < goroutines; i++ { + wg.Add(1) + go func(i int) { + defer wg.Done() + <-start + res, err := proxy.GetOrFetchArtifactFromURL(ctx, + "npm", "left-pad", "1.3.0", "left-pad-1.3.0.tgz", + "https://registry.npmjs.org/left-pad/-/left-pad-1.3.0.tgz") + if err != nil { + errs[i] = err + return + } + defer func() { _ = res.Reader.Close() }() + body, readErr := io.ReadAll(res.Reader) + errs[i] = readErr + bodies[i] = body + }(i) + } + + close(start) + wg.Wait() + + for i, err := range errs { + if err != nil { + t.Errorf("caller %d failed: %v", i, err) + } + } + for i, body := range bodies { + if !bytes.Equal(body, content) { + t.Errorf("caller %d got %d bytes, want %d", i, len(body), len(content)) + } + } + if got := fetcher.calls.Load(); got != 1 { + t.Errorf("upstream fetches = %d, want 1", got) + } +} diff --git a/internal/handler/handler.go b/internal/handler/handler.go index a78393d..137e0b1 100644 --- a/internal/handler/handler.go +++ b/internal/handler/handler.go @@ -184,6 +184,11 @@ type Proxy struct { // ScanFetchBaseURL is the address scanners use to reach this proxy to // pull staged artifacts. ScanFetchBaseURL string + + // inFlight coalesces concurrent cache misses for one artifact, so a single + // upstream fetch serves every waiting caller. Keyed by artifactCoalesceKey. + fetchMu sync.Mutex + inFlight map[string]*inflightFetch } // NewProxy creates a new Proxy with the given dependencies. @@ -225,7 +230,13 @@ func (p *Proxy) GetOrFetchArtifact(ctx context.Context, ecosystem, name, version } metrics.RecordCacheMiss(ecosystem) - return p.fetchAndCache(ctx, ecosystem, name, version, filename, pkgPURL, versionPURL) + key := artifactCoalesceKey(versionPURL, filename, "", "") + recheck := func() (artifacts.Artifact, string, bool) { + return p.cachedArtifactRecord(pkgPURL, versionPURL, filename, "") + } + return p.coalesceFetch(ctx, key, recheck, func(fetchCtx context.Context) (artifacts.Artifact, string, error) { + return p.fetchAndCache(fetchCtx, ecosystem, name, version, filename, pkgPURL, versionPURL) + }) } // GetCachedArtifact retrieves an artifact from cache without contacting an upstream. @@ -359,14 +370,14 @@ func (p *Proxy) rejectUnusableCacheRecord(artifact *database.CachedArtifact, ver } } -func (p *Proxy) fetchAndCache(ctx context.Context, ecosystem, name, version, filename, pkgPURL, versionPURL string) (*CacheResult, error) { +func (p *Proxy) fetchAndCache(ctx context.Context, ecosystem, name, version, filename, pkgPURL, versionPURL string) (artifacts.Artifact, string, error) { // Resolve download URL info, err := p.Resolver.Resolve(ctx, ecosystem, name, version) if err != nil { if errors.Is(err, fetch.ErrNotFound) { - return nil, ErrUpstreamNotFound + return artifacts.Artifact{}, "", ErrUpstreamNotFound } - return nil, fmt.Errorf("resolving download URL: %w", err) + return artifacts.Artifact{}, "", fmt.Errorf("resolving download URL: %w", err) } // Use resolved filename if provided filename is empty @@ -386,9 +397,9 @@ func (p *Proxy) fetchAndCache(ctx context.Context, ecosystem, name, version, fil metrics.RecordUpstreamFetch(ecosystem, fetchDuration) metrics.RecordUpstreamError(ecosystem, "fetch_failed") if errors.Is(err, fetch.ErrNotFound) { - return nil, ErrUpstreamNotFound + return artifacts.Artifact{}, "", ErrUpstreamNotFound } - return nil, fmt.Errorf("fetching from upstream: %w", err) + return artifacts.Artifact{}, "", fmt.Errorf("fetching from upstream: %w", err) } metrics.RecordUpstreamFetch(ecosystem, fetchDuration) @@ -405,7 +416,10 @@ func (p *Proxy) fetchAndCache(ctx context.Context, ecosystem, name, version, fil // verdict means a blocked artifact was never reachable by any client. On // block, the just-stored bytes are deleted and ErrArtifactBlocked is // returned; updateCacheDB is never called. -func (p *Proxy) storeArtifact(ctx context.Context, ecosystem, name, version, filename, pkgPURL, versionPURL, upstreamURL, upstreamHash string, artifact *fetch.Artifact) (*CacheResult, error) { +// +// It returns the artifact and its storage path, not a reader; callers get one +// from openStoredArtifact. +func (p *Proxy) storeArtifact(ctx context.Context, ecosystem, name, version, filename, pkgPURL, versionPURL, upstreamURL, upstreamHash string, artifact *fetch.Artifact) (artifacts.Artifact, string, error) { storagePath := storage.ArtifactPath(ecosystem, "", name, version, filename) storeStart := time.Now() @@ -414,14 +428,14 @@ func (p *Proxy) storeArtifact(ctx context.Context, ecosystem, name, version, fil metrics.RecordStorageOperation("write", time.Since(storeStart)) if err != nil { metrics.RecordStorageError("write") - return nil, fmt.Errorf("storing artifact: %w", err) + return artifacts.Artifact{}, "", fmt.Errorf("storing artifact: %w", err) } if !artifactHashMatches(hash, upstreamHash) { if delErr := p.Storage.Delete(ctx, storagePath); delErr != nil { p.Logger.Warn("failed to discard artifact with mismatched checksum", "path", storagePath, "error", delErr) } - return nil, fmt.Errorf("%w: upstream declared %s, got %s", ErrArtifactDigestMismatch, upstreamHash, hash) + return artifacts.Artifact{}, "", fmt.Errorf("%w: upstream declared %s, got %s", ErrArtifactDigestMismatch, upstreamHash, hash) } if p.Scanners != nil && p.Scanners.Enabled() { @@ -433,7 +447,7 @@ func (p *Proxy) storeArtifact(ctx context.Context, ecosystem, name, version, fil p.Logger.Warn("failed to delete blocked artifact from storage", "path", storagePath, "error", delErr) } - return nil, err + return artifacts.Artifact{}, "", err } } @@ -451,7 +465,14 @@ func (p *Proxy) storeArtifact(ctx context.Context, ecosystem, name, version, fil // Continue anyway - we have the file } - // Open the stored file to return + return sharedArtifact, storagePath, nil +} + +// openStoredArtifact gives one caller its own reader over just-committed +// bytes. Callers sharing a fetch cannot share a handle: it has one read +// position, so they would consume each other's bytes and the first Close would +// break the rest. +func (p *Proxy) openStoredArtifact(ctx context.Context, artifact artifacts.Artifact, storagePath string) (*CacheResult, error) { readStart := time.Now() reader, err := p.Storage.Open(ctx, storagePath) metrics.RecordStorageOperation("read", time.Since(readStart)) @@ -463,11 +484,138 @@ func (p *Proxy) storeArtifact(ctx context.Context, ecosystem, name, version, fil return &CacheResult{ Reader: reader, - Artifact: sharedArtifact, + Artifact: artifact, Cached: false, }, nil } +// artifactCoalesceKey identifies one artifact fetch. downloadURL and +// upstreamHash are included so callers expecting different bytes (multiple +// upstreams, or a re-published version) never share a fetch. The hash is +// lowercased because artifactHashMatches compares case-insensitively, so one +// digest in two casings describes one artifact and must not split the fetch. +func artifactCoalesceKey(versionPURL, filename, downloadURL, upstreamHash string) string { + return strings.Join([]string{versionPURL, filename, downloadURL, strings.ToLower(upstreamHash)}, "\x00") +} + +// cachedArtifactRecord reports an artifact already committed to the cache, +// without opening it. A caller checks the cache before it gets here, so a +// concurrent fetch can commit the same artifact in between; rechecking the +// record keeps that caller from fetching it a second time. A lookup error is +// reported as a miss, which costs a redundant fetch rather than a failure. +func (p *Proxy) cachedArtifactRecord(pkgPURL, versionPURL, filename, upstreamHash string) (artifacts.Artifact, string, bool) { + record, err := p.DB.GetCachedArtifact(pkgPURL, versionPURL, filename) + if err != nil || record == nil { + return artifacts.Artifact{}, "", false + } + if !artifactHashMatches(record.Artifact.Digest.Encoded(), upstreamHash) { + return artifacts.Artifact{}, "", false + } + return record.Artifact, record.StoragePath, true +} + +// errSharedFetchAbandoned is what waiters see if the caller running a shared +// fetch panicked out of it. +var errSharedFetchAbandoned = errors.New("shared upstream fetch did not complete") + +// inflightFetch is one upstream fetch that concurrent callers share. val and +// err are written before done closes and read only after, so the close is the +// handoff. +type inflightFetch struct { + done chan struct{} + val fetchedArtifact + err error +} + +// coalesceFetch runs commit at most once for concurrent callers sharing key, +// then gives each its own reader over the stored bytes. +// +// The first caller in runs the fetch and the rest wait on it. Roles are +// decided under fetchMu rather than inferred afterwards, because the two need +// different cancellation behaviour: a waiter may leave when its own client goes +// away, while the caller running the fetch must see it through so +// storeArtifact's scan-on-disconnect handling still decides the outcome. +// +// Before fetching, that caller rechecks the cache through recheck: its own +// lookup happened before it took the key, so a fetch that committed in +// between would otherwise be repeated. A hit fills the shared value as a +// fetch would. +// +// commit runs on that caller's context, so cancellation behaves as it did +// uncoalesced and mirroring still relies on it aborting the fetch. If that +// caller goes away, everyone sharing the fetch gets its error and the key is +// released for a later retry. +// +// Every sharing caller still records a cache miss, so the gap between +// proxy_cache_misses_total and upstream fetch observations is what coalescing +// saved. +func (p *Proxy) coalesceFetch(ctx context.Context, key string, recheck func() (artifacts.Artifact, string, bool), commit func(context.Context) (artifacts.Artifact, string, error)) (*CacheResult, error) { + p.fetchMu.Lock() + if p.inFlight == nil { + p.inFlight = make(map[string]*inflightFetch) + } + f, joined := p.inFlight[key] + if !joined { + f = &inflightFetch{done: make(chan struct{})} + p.inFlight[key] = f + } + p.fetchMu.Unlock() + + if !joined { + return p.runSharedFetch(ctx, key, f, recheck, commit) + } + + select { + case <-ctx.Done(): + // This caller gave up; the fetch continues for everyone else. + return nil, ctx.Err() + case <-f.done: + } + if f.err != nil { + return nil, f.err + } + return p.openStoredArtifact(ctx, f.val.artifact, f.val.storagePath) +} + +// runSharedFetch performs the fetch that joined callers are waiting on. It is +// never abandoned early, and always releases the key and wakes the waiters. +func (p *Proxy) runSharedFetch(ctx context.Context, key string, f *inflightFetch, recheck func() (artifacts.Artifact, string, bool), commit func(context.Context) (artifacts.Artifact, string, error)) (*CacheResult, error) { + // Set before running so a panicking commit leaves waiters with an error + // rather than a zero-valued artifact. + f.err = errSharedFetchAbandoned + defer func() { + p.fetchMu.Lock() + delete(p.inFlight, key) + p.fetchMu.Unlock() + close(f.done) + }() + + // A caller checks the cache before reaching here, so a fetch that finished + // in between would otherwise be repeated. Serve that record only if its + // bytes are still present: a record can outlive them, and refetching is + // the same recovery the cache lookup makes. + if stored, path, ok := recheck(); ok { + if res, err := p.openStoredArtifact(ctx, stored, path); err == nil { + f.val, f.err = fetchedArtifact{artifact: stored, storagePath: path}, nil + return res, nil + } + } + + stored, path, err := commit(ctx) + f.val, f.err = fetchedArtifact{artifact: stored, storagePath: path}, err + if err != nil { + return nil, err + } + return p.openStoredArtifact(ctx, stored, path) +} + +// fetchedArtifact is what a shared fetch hands its callers: metadata and a +// storage path, neither holding reader state. +type fetchedArtifact struct { + artifact artifacts.Artifact + storagePath string +} + // runScan generates a signed fetch URL for the just-staged artifact and // asks the configured scanners for a verdict. Returns a wrapped // ErrArtifactBlocked if any scanner blocks, or a scan-infrastructure error. @@ -697,17 +845,22 @@ func metadataStoragePath(ecosystem, cacheKey string) string { // cacheKey is typically the package name but can include subpath components. // Optional acceptHeaders specify the Accept header(s) to send; defaults to application/json. func (p *Proxy) FetchOrCacheMetadata(ctx context.Context, ecosystem, cacheKey, upstreamURL string, acceptHeaders ...string) ([]byte, string, error) { - return p.fetchOrCacheMetadata(ctx, ecosystem, cacheKey, upstreamURL, false, acceptHeaders...) + body, contentType, _, err := p.fetchOrCacheMetadata(ctx, ecosystem, cacheKey, upstreamURL, "", nil, acceptHeaders...) + return body, contentType, err } -// fetchOrCacheMetadata implements FetchOrCacheMetadata. When verbatim is true -// (the ProxyCached path, which serves upstream bytes through unchanged) the -// upstream is fetched with Accept-Encoding: identity so signed and hash-pinned -// index files are cached exactly as sent. Direct callers that parse or rewrite -// the body pass verbatim=false and keep transparent transfer compression. -func (p *Proxy) fetchOrCacheMetadata(ctx context.Context, ecosystem, cacheKey, upstreamURL string, verbatim bool, acceptHeaders ...string) ([]byte, string, error) { +// fetchOrCacheMetadata implements FetchOrCacheMetadata. acceptEncoding controls +// the upstream Accept-Encoding: an empty string leaves it unset so Go +// transparently decompresses (for direct callers that parse or rewrite the +// body); any non-empty value is sent verbatim, which disables Go's +// decompression so the wire bytes and their Content-Encoding are stored and +// replayed as sent. The ProxyCached path uses "identity" for signed indexes and +// "gzip" where both hops should stay compressed. +// validate, when supplied, runs before caching or serving a document. Validation +// failures follow the same stale-cache fallback path as upstream failures. +func (p *Proxy) fetchOrCacheMetadata(ctx context.Context, ecosystem, cacheKey, upstreamURL, acceptEncoding string, validate func([]byte) error, acceptHeaders ...string) ([]byte, string, string, error) { if containsPathTraversal(cacheKey) { - return nil, "", fmt.Errorf("invalid cache key: %q", cacheKey) + return nil, "", "", fmt.Errorf("invalid cache key: %q", cacheKey) } storagePath := metadataStoragePath(ecosystem, cacheKey) @@ -721,18 +874,14 @@ func (p *Proxy) fetchOrCacheMetadata(ctx context.Context, ecosystem, cacheKey, u // Serve from cache if within TTL (skip upstream entirely) if entry != nil && p.MetadataTTL > 0 && entry.FetchedAt.Valid { if time.Since(entry.FetchedAt.Time) < p.MetadataTTL { - cached, readErr := p.Storage.Open(ctx, entry.StoragePath) + data, ct, readErr := p.readCachedMetadata(ctx, entry, validate) if readErr == nil { - defer func() { _ = cached.Close() }() - data, readErr := p.ReadMetadata(cached) - if readErr == nil { - ct := contentTypeJSON - if entry.ContentType.Valid { - ct = entry.ContentType.String - } - metrics.RecordCacheHit(ecosystem) - return data, ct, nil - } + metrics.RecordCacheHit(ecosystem) + return data, ct, entry.ContentEncoding.String, nil + } + if validate != nil { + // Do not revalidate an unusable cached body with its ETag. + entry = nil } // Cache file missing/unreadable, fall through to upstream } @@ -745,43 +894,61 @@ func (p *Proxy) fetchOrCacheMetadata(ctx context.Context, ecosystem, cacheKey, u } // Try upstream - meta, err := p.fetchUpstreamMetadata(ctx, upstreamURL, entry, accept, verbatim) + meta, err := p.fetchUpstreamMetadata(ctx, upstreamURL, entry, accept, acceptEncoding) if errors.Is(err, errStale304) { // 304 but cached file is gone; retry without ETag - meta, err = p.fetchUpstreamMetadata(ctx, upstreamURL, nil, accept, verbatim) + meta, err = p.fetchUpstreamMetadata(ctx, upstreamURL, nil, accept, acceptEncoding) + } + if err == nil && validate != nil { + err = validate(meta.body) } if err == nil { if p.CacheMetadata { p.cacheMetadataBlob(ctx, ecosystem, cacheKey, storagePath, meta) } - return meta.body, meta.contentType, nil + return meta.body, meta.contentType, meta.contentEncoding, nil } // Upstream failed -- fall back to cache if available if !p.CacheMetadata || entry == nil { - return nil, "", fmt.Errorf("upstream failed and no cached metadata: %w", err) + return nil, "", "", fmt.Errorf("upstream failed and no cached metadata: %w", err) } p.Logger.Warn("upstream metadata fetch failed, checking cache", "ecosystem", ecosystem, "key", cacheKey, "error", err) - cached, readErr := p.Storage.Open(ctx, entry.StoragePath) + // Re-read the row so the encoding describes the blob as it is now: a + // concurrent refetch may have replaced both since entry was read above + // (an identity blob swapped for a gzip one during rollout). + entry = p.currentMetadataEntry(ecosystem, cacheKey, entry) + + data, ct, readErr := p.readCachedMetadata(ctx, entry, validate) if readErr != nil { - return nil, "", fmt.Errorf("upstream failed and cached file missing: %w", err) + return nil, "", "", fmt.Errorf("upstream failed and cached metadata unusable (%v): %w", readErr, err) + } + + p.Logger.Info("serving metadata from cache", + "ecosystem", ecosystem, "key", cacheKey) + return data, ct, entry.ContentEncoding.String, nil +} + +func (p *Proxy) readCachedMetadata(ctx context.Context, entry *database.MetadataCacheEntry, validate func([]byte) error) ([]byte, string, error) { + cached, err := p.Storage.Open(ctx, entry.StoragePath) + if err != nil { + return nil, "", err } defer func() { _ = cached.Close() }() - - data, readErr := p.ReadMetadata(cached) - if readErr != nil { - return nil, "", fmt.Errorf("upstream failed and cached read error: %w", err) + data, err := p.ReadMetadata(cached) + if err == nil && validate != nil { + err = validate(data) + } + if err != nil { + return nil, "", err } - ct := contentTypeJSON if entry.ContentType.Valid { ct = entry.ContentType.String } - p.Logger.Info("serving metadata from cache", - "ecosystem", ecosystem, "key", cacheKey) return data, ct, nil } @@ -801,20 +968,19 @@ type upstreamMetadata struct { } // fetchUpstreamMetadata fetches metadata from upstream, using ETag for conditional revalidation. -// It requests the identity encoding and never transparently decompresses, so the returned -// bytes are exactly what the upstream sent; any Content-Encoding the upstream applied -// anyway is reported alongside so callers can store and replay it. -func (p *Proxy) fetchUpstreamMetadata(ctx context.Context, upstreamURL string, entry *database.MetadataCacheEntry, accept string, verbatim bool) (*upstreamMetadata, error) { +// When acceptEncoding is non-empty it is sent as the Accept-Encoding header, which disables Go's +// transparent decompression (it only applies when the transport adds the header itself), so the +// returned bytes are exactly what the upstream sent and any Content-Encoding it applied is reported +// alongside for the caller to store and replay. An empty acceptEncoding leaves Go to negotiate and +// decompress transparently. +func (p *Proxy) fetchUpstreamMetadata(ctx context.Context, upstreamURL string, entry *database.MetadataCacheEntry, accept, acceptEncoding string) (*upstreamMetadata, error) { req, err := http.NewRequestWithContext(ctx, http.MethodGet, upstreamURL, nil) if err != nil { return nil, fmt.Errorf("creating request: %w", err) } req.Header.Set("Accept", accept) - if verbatim { - // Setting Accept-Encoding explicitly disables Go's transparent gzip - // decompression (it only applies when the transport adds the header - // itself), so signed index files are cached byte-for-byte as sent. - req.Header.Set(headerAcceptEncoding, "identity") + if acceptEncoding != "" { + req.Header.Set(headerAcceptEncoding, acceptEncoding) } p.applyUpstreamAuth(req) @@ -893,7 +1059,7 @@ func (p *Proxy) cacheMetadataBlob(ctx context.Context, ecosystem, cacheKey, stor return } - _ = p.DB.UpsertMetadataCache(&database.MetadataCacheEntry{ + err = p.DB.UpsertMetadataCache(&database.MetadataCacheEntry{ Ecosystem: ecosystem, Name: cacheKey, StoragePath: storagePath, @@ -904,14 +1070,34 @@ func (p *Proxy) cacheMetadataBlob(ctx context.Context, ecosystem, cacheKey, stor LastModified: sql.NullTime{Time: meta.lastModified, Valid: !meta.lastModified.IsZero()}, FetchedAt: sql.NullTime{Time: time.Now(), Valid: true}, }) + if err != nil { + // The blob is written but the row describing it is not, so a later + // TTL hit or stale fallback would serve these bytes with the previous + // row's encoding. Drop the blob so row and bytes can never disagree; + // the next request refetches instead. + p.Logger.Warn("failed to record cached metadata, discarding blob", "ecosystem", ecosystem, "key", cacheKey, "error", err) + if delErr := p.Storage.Delete(ctx, storagePath); delErr != nil { + p.Logger.Warn("failed to discard metadata blob", "ecosystem", ecosystem, "key", cacheKey, "error", delErr) + } + } +} + +// currentMetadataEntry re-reads the metadata cache row and returns it, or +// fallback when the row cannot be read. Used before serving a stored blob so +// its encoding comes from the row as it is now rather than from a snapshot +// taken before the upstream fetch. +func (p *Proxy) currentMetadataEntry(ecosystem, cacheKey string, fallback *database.MetadataCacheEntry) *database.MetadataCacheEntry { + if fresh, err := p.DB.GetMetadataCache(ecosystem, cacheKey); err == nil && fresh != nil { + return fresh + } + return fallback } // cachedMeta holds cache validators and freshness state from a metadata cache entry. type cachedMeta struct { - etag string - lastModified time.Time - contentEncoding string - stale bool + etag string + lastModified time.Time + stale bool } // lookupCachedMeta retrieves cache validators for a metadata entry. @@ -930,9 +1116,6 @@ func (p *Proxy) lookupCachedMeta(ecosystem, cacheKey string) cachedMeta { if entry.LastModified.Valid { cm.lastModified = entry.LastModified.Time } - if entry.ContentEncoding.Valid { - cm.contentEncoding = entry.ContentEncoding.String - } // If FetchedAt is older than TTL, upstream must have failed and // we served from stale cache (successful fetches update FetchedAt). if p.MetadataTTL > 0 && entry.FetchedAt.Valid && time.Since(entry.FetchedAt.Time) > p.MetadataTTL { @@ -946,13 +1129,22 @@ func (p *Proxy) lookupCachedMeta(ecosystem, cacheKey string) cachedMeta { // When metadata caching is disabled, the response is streamed directly to avoid buffering // large metadata responses (e.g. npm packages with many versions) in memory. func (p *Proxy) ProxyCached(w http.ResponseWriter, r *http.Request, upstreamURL, ecosystem, cacheKey string, acceptHeaders ...string) { + p.proxyCachedWithEncoding(w, r, upstreamURL, ecosystem, cacheKey, "identity", acceptHeaders...) +} + +// proxyCachedWithEncoding is ProxyCached with an explicit upstream Accept-Encoding. +// "identity" preserves signed index bytes (the default); "gzip" keeps both hops +// compressed for large, non-hash-pinned metadata whose clients decode gzip +// (Homebrew API). The stored bytes and Content-Encoding are replayed verbatim +// either way. +func (p *Proxy) proxyCachedWithEncoding(w http.ResponseWriter, r *http.Request, upstreamURL, ecosystem, cacheKey, acceptEncoding string, acceptHeaders ...string) { if !p.CacheMetadata { // Stream directly without buffering when caching is off. - p.proxyMetadataStream(w, r, upstreamURL, acceptHeaders...) + p.proxyMetadataStream(w, r, upstreamURL, acceptEncoding, acceptHeaders...) return } - body, contentType, err := p.fetchOrCacheMetadata(r.Context(), ecosystem, cacheKey, upstreamURL, true, acceptHeaders...) + body, contentType, contentEncoding, err := p.fetchOrCacheMetadata(r.Context(), ecosystem, cacheKey, upstreamURL, acceptEncoding, nil, acceptHeaders...) if err != nil { if errors.Is(err, ErrUpstreamNotFound) { http.Error(w, "not found", http.StatusNotFound) @@ -963,12 +1155,21 @@ func (p *Proxy) ProxyCached(w http.ResponseWriter, r *http.Request, upstreamURL, return } - p.writeMetadataCachedResponse(w, r, ecosystem, cacheKey, body, contentType) + p.writeMetadataCachedResponseWithEncoding(w, r, ecosystem, cacheKey, body, contentType, contentEncoding) } // writeMetadataCachedResponse writes a cached metadata response and handles // conditional request headers using metadata cache validators. func (p *Proxy) writeMetadataCachedResponse(w http.ResponseWriter, r *http.Request, ecosystem, cacheKey string, body []byte, contentType string) { + p.writeMetadataCachedResponseWithEncoding(w, r, ecosystem, cacheKey, body, contentType, "") +} + +// writeMetadataCachedResponseWithEncoding is writeMetadataCachedResponse with +// an explicit Content-Encoding. contentEncoding must describe the body being +// written; it is passed in rather than re-read from the cache row, which is +// missing or stale when the metadata cache write failed and would otherwise +// mislabel the bytes. +func (p *Proxy) writeMetadataCachedResponseWithEncoding(w http.ResponseWriter, r *http.Request, ecosystem, cacheKey string, body []byte, contentType, contentEncoding string) { cm := p.lookupCachedMeta(ecosystem, cacheKey) if cm.etag != "" { @@ -992,8 +1193,8 @@ func (p *Proxy) writeMetadataCachedResponse(w http.ResponseWriter, r *http.Reque w.Header().Set(headerContentType, contentType) w.Header().Set(headerContentLength, strconv.Itoa(len(body))) - if cm.contentEncoding != "" { - w.Header().Set(headerContentEncoding, cm.contentEncoding) + if contentEncoding != "" { + w.Header().Set(headerContentEncoding, contentEncoding) } if cm.stale { w.Header().Set("Warning", `110 - "Response is Stale"`) @@ -1006,7 +1207,7 @@ func (p *Proxy) writeMetadataCachedResponse(w http.ResponseWriter, r *http.Reque // proxyMetadataStream forwards an upstream metadata response by streaming it to the client // without buffering the full body in memory. -func (p *Proxy) proxyMetadataStream(w http.ResponseWriter, r *http.Request, upstreamURL string, acceptHeaders ...string) { +func (p *Proxy) proxyMetadataStream(w http.ResponseWriter, r *http.Request, upstreamURL, acceptEncoding string, acceptHeaders ...string) { req, err := http.NewRequestWithContext(r.Context(), r.Method, upstreamURL, nil) if err != nil { http.Error(w, "failed to create request", http.StatusInternalServerError) @@ -1018,10 +1219,14 @@ func (p *Proxy) proxyMetadataStream(w http.ResponseWriter, r *http.Request, upst accept = acceptHeaders[0] } req.Header.Set("Accept", accept) - // ProxyCached serves bytes through verbatim, so request identity to keep - // Go from transparently decompressing (and stripping the Content-Encoding - // of) signed index files, regardless of what the client negotiated. - req.Header.Set(headerAcceptEncoding, "identity") + // Set Accept-Encoding explicitly (identity, or gzip for compressible + // verbatim metadata) so Go does not transparently decompress and strip the + // Content-Encoding of the bytes we forward, regardless of what the client + // negotiated. An empty value leaves the header unset, as in + // fetchUpstreamMetadata. + if acceptEncoding != "" { + req.Header.Set(headerAcceptEncoding, acceptEncoding) + } p.applyUpstreamAuth(req) for _, header := range []string{"If-Modified-Since", "If-None-Match"} { @@ -1100,16 +1305,30 @@ func (p *Proxy) getOrFetchArtifactFromURLWithCachePURLs(ctx context.Context, eco return cached, nil } metrics.RecordCacheMiss(ecosystem) + return p.coalescedFetchFromURL(ctx, ecosystem, name, version, filename, pkgPURL, versionPURL, downloadURL, headers, upstreamHash) +} - return p.fetchAndCacheFromURL(ctx, ecosystem, name, version, filename, pkgPURL, versionPURL, downloadURL, headers, upstreamHash) +// coalescedFetchFromURL fetches an artifact the cache could not serve, sharing +// the fetch with concurrent callers. The caller running it discards a stale +// entry under the key, where it cannot delete a fetch that just replaced it. +func (p *Proxy) coalescedFetchFromURL(ctx context.Context, ecosystem, name, version, filename, pkgPURL, versionPURL, downloadURL string, headers http.Header, upstreamHash string) (*CacheResult, error) { + key := artifactCoalesceKey(versionPURL, filename, downloadURL, upstreamHash) + recheck := func() (artifacts.Artifact, string, bool) { + return p.cachedArtifactRecord(pkgPURL, versionPURL, filename, upstreamHash) + } + return p.coalesceFetch(ctx, key, recheck, func(fetchCtx context.Context) (artifacts.Artifact, string, error) { + p.discardStaleArtifact(fetchCtx, pkgPURL, versionPURL, filename, upstreamHash) + return p.fetchAndCacheFromURL(fetchCtx, ecosystem, name, version, filename, pkgPURL, versionPURL, downloadURL, headers, upstreamHash) + }) } // getCachedArtifactWithUpstreamHash returns a cached artifact whose recorded // content hash matches the checksum the upstream currently declares for it. // This detects an upstream re-publishing under the same version, which the // stream integrity check in checkCache cannot: that check only verifies the -// stored blob against the hash recorded when it was cached. On mismatch the -// stale entry is discarded and nil is returned so the caller re-fetches. +// stored blob against the hash recorded when it was cached. A stale entry is +// a miss and is left in place: the fetch that replaces it discards it under +// the coalescing key. func (p *Proxy) getCachedArtifactWithUpstreamHash(ctx context.Context, pkgPURL, versionPURL, filename, upstreamHash string) (*CacheResult, error) { cached, err := p.checkCache(ctx, pkgPURL, versionPURL, filename) if err != nil || cached == nil { @@ -1118,17 +1337,31 @@ func (p *Proxy) getCachedArtifactWithUpstreamHash(ctx context.Context, pkgPURL, if artifactHashMatches(cached.Artifact.Digest.Encoded(), upstreamHash) { return cached, nil } - if cached.Reader != nil { _ = cached.Reader.Close() } - p.Logger.Warn("cached artifact hash disagrees with upstream metadata, discarding", - "purl", versionPURL, "filename", filename, "cached", cached.Artifact.Digest.Encoded(), "upstream", upstreamHash) - p.discardCachedArtifact(ctx, versionPURL, filename, cached.storagePath) return nil, nil } -func (p *Proxy) fetchAndCacheFromURL(ctx context.Context, ecosystem, name, version, filename, pkgPURL, versionPURL, downloadURL string, headers http.Header, upstreamHash string) (*CacheResult, error) { +// discardStaleArtifact removes the cached entry when its digest disagrees +// with upstreamHash. It runs under the coalescing key, after the recheck, so +// an entry a previous fetch refreshed is kept. +func (p *Proxy) discardStaleArtifact(ctx context.Context, pkgPURL, versionPURL, filename, upstreamHash string) { + record, err := p.DB.GetCachedArtifact(pkgPURL, versionPURL, filename) + if err != nil { + p.Logger.Warn("failed to read cache record before refetch", + "purl", versionPURL, "filename", filename, "error", err) + return + } + if record == nil || artifactHashMatches(record.Artifact.Digest.Encoded(), upstreamHash) { + return + } + p.Logger.Warn("cached artifact hash disagrees with upstream metadata, discarding", + "purl", versionPURL, "filename", filename, "cached", record.Artifact.Digest.Encoded(), "upstream", upstreamHash) + p.discardCachedArtifact(ctx, versionPURL, filename, record.StoragePath) +} + +func (p *Proxy) fetchAndCacheFromURL(ctx context.Context, ecosystem, name, version, filename, pkgPURL, versionPURL, downloadURL string, headers http.Header, upstreamHash string) (artifacts.Artifact, string, error) { p.Logger.Info("fetching from upstream", "ecosystem", ecosystem, "name", name, "version", version, "url", downloadURL) @@ -1138,9 +1371,9 @@ func (p *Proxy) fetchAndCacheFromURL(ctx context.Context, ecosystem, name, versi if err != nil { metrics.RecordUpstreamError(ecosystem, "fetch_failed") if errors.Is(err, fetch.ErrNotFound) { - return nil, ErrUpstreamNotFound + return artifacts.Artifact{}, "", ErrUpstreamNotFound } - return nil, fmt.Errorf("fetching from upstream: %w", err) + return artifacts.Artifact{}, "", fmt.Errorf("fetching from upstream: %w", err) } return p.storeArtifact(ctx, ecosystem, name, version, filename, pkgPURL, versionPURL, downloadURL, upstreamHash, artifact) diff --git a/internal/handler/handler_test.go b/internal/handler/handler_test.go index 076b74b..b632294 100644 --- a/internal/handler/handler_test.go +++ b/internal/handler/handler_test.go @@ -11,6 +11,7 @@ import ( "net/http" "net/http/httptest" "strings" + "sync" "testing" "time" @@ -29,6 +30,7 @@ import ( // mockStorage implements storage.Storage for testing. type mockStorage struct { + mu sync.Mutex files map[string][]byte storeErr error openErr error @@ -41,6 +43,8 @@ func newMockStorage() *mockStorage { } func (s *mockStorage) Store(_ context.Context, path string, r io.Reader) (int64, string, error) { + s.mu.Lock() + defer s.mu.Unlock() if s.storeErr != nil { return 0, "", s.storeErr } @@ -53,6 +57,8 @@ func (s *mockStorage) Store(_ context.Context, path string, r io.Reader) (int64, } func (s *mockStorage) Open(_ context.Context, path string) (io.ReadCloser, error) { + s.mu.Lock() + defer s.mu.Unlock() if s.openErr != nil { return nil, s.openErr } @@ -64,6 +70,8 @@ func (s *mockStorage) Open(_ context.Context, path string) (io.ReadCloser, error } func (s *mockStorage) Exists(_ context.Context, path string) (bool, error) { + s.mu.Lock() + defer s.mu.Unlock() _, ok := s.files[path] return ok, nil } @@ -75,11 +83,15 @@ func (s *mockStorage) Delete(ctx context.Context, path string) error { if err := ctx.Err(); err != nil { return err } + s.mu.Lock() + defer s.mu.Unlock() delete(s.files, path) return nil } func (s *mockStorage) Size(_ context.Context, path string) (int64, error) { + s.mu.Lock() + defer s.mu.Unlock() data, ok := s.files[path] if !ok { return 0, storage.ErrNotFound @@ -88,6 +100,8 @@ func (s *mockStorage) Size(_ context.Context, path string) (int64, error) { } func (s *mockStorage) UsedSpace(_ context.Context) (int64, error) { + s.mu.Lock() + defer s.mu.Unlock() var total int64 for _, data := range s.files { total += int64(len(data)) @@ -109,11 +123,15 @@ func (s *mockStorage) URL() string { return "mem://" } func (s *mockStorage) Close() error { return nil } -// mockFetcher implements fetch.FetcherInterface for testing. +// mockFetcher implements fetch.FetcherInterface for testing. Recording is +// locked because coalescing tests call the handler from many goroutines; tests +// read the recorded fields only after those calls have returned. type mockFetcher struct { artifact *fetch.Artifact fetchErr error fetchErrByURL map[string]error + + mu sync.Mutex fetchCalled bool fetchedURL string fetchedHeader http.Header @@ -124,9 +142,11 @@ func (f *mockFetcher) Fetch(ctx context.Context, url string) (*fetch.Artifact, e } func (f *mockFetcher) FetchWithHeaders(_ context.Context, url string, headers http.Header) (*fetch.Artifact, error) { + f.mu.Lock() f.fetchCalled = true f.fetchedURL = url f.fetchedHeader = headers.Clone() + f.mu.Unlock() if f.fetchErrByURL != nil { if err, ok := f.fetchErrByURL[url]; ok { return nil, err diff --git a/internal/handler/homebrew.go b/internal/handler/homebrew.go index 0fd5b62..0af67af 100644 --- a/internal/handler/homebrew.go +++ b/internal/handler/homebrew.go @@ -55,7 +55,16 @@ func (h *HomebrewHandler) Routes() http.Handler { upstreamURL += "?" + r.URL.RawQuery } - h.proxy.ProxyCached(w, r, upstreamURL, homebrewMetadataEcosystem, homebrewMetadataCacheKey(requestPath, r.URL.RawQuery), "*/*") + // brew fetches every JSON API download with `curl --compressed` and + // decodes Content-Encoding itself, and formula.jws.json is ~33 MB plain + // versus ~5 MB gzip, so keep both hops compressed. The analytics + // endpoints are the one consumer brew fetches without --compressed; + // they stay identity. + acceptEncoding := "gzip" + if strings.HasPrefix(requestPath, "analytics/") { + acceptEncoding = "identity" + } + h.proxy.proxyCachedWithEncoding(w, r, upstreamURL, homebrewMetadataEcosystem, homebrewMetadataCacheKey(requestPath, r.URL.RawQuery), acceptEncoding, "*/*") }) } diff --git a/internal/handler/homebrew_test.go b/internal/handler/homebrew_test.go index e8931ad..e5d7e5c 100644 --- a/internal/handler/homebrew_test.go +++ b/internal/handler/homebrew_test.go @@ -1,11 +1,13 @@ package handler import ( + "bytes" "io" "net/http" "net/http/httptest" "strconv" "strings" + "sync/atomic" "testing" "time" @@ -361,3 +363,96 @@ func TestRegisterHomebrewArtifactsRejectsOtherHomebrewRoutes(t *testing.T) { t.Errorf("blocked Homebrew routes made %d upstream requests, want 0", upstreamRequests) } } + +// TestHomebrewHandler_RequestsGzipForAPIPaths covers #305's motivating case: +// the JSON API files are fetched, cached and served gzip-compressed with +// Content-Encoding: gzip (brew fetches them with --compressed), while the +// analytics endpoints, which brew fetches without --compressed, stay identity. +func TestHomebrewHandler_RequestsGzipForAPIPaths(t *testing.T) { + plain := []byte(`{"payload":"signed bytes","signatures":[]}`) + compressed := gzipPayload(t, plain) + + var available atomic.Bool + available.Store(true) + var requests atomic.Int32 + var sawAcceptEncoding atomic.Value // string + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + requests.Add(1) + sawAcceptEncoding.Store(r.Header.Get(headerAcceptEncoding)) + if !available.Load() { + http.Error(w, "unavailable", http.StatusServiceUnavailable) + return + } + w.Header().Set(headerContentType, "application/json") + if strings.Contains(r.Header.Get(headerAcceptEncoding), "gzip") { + w.Header().Set(headerContentEncoding, "gzip") + _, _ = w.Write(compressed) + return + } + _, _ = w.Write(plain) + })) + defer upstream.Close() + + proxy, _, _, _ := setupTestProxy(t) + proxy.CacheMetadata = true + proxy.MetadataTTL = time.Hour + proxy.HTTPClient = upstream.Client() + h := NewHomebrewHandler(proxy, upstream.URL+"/api").Routes() + + get := func(path string) *httptest.ResponseRecorder { + w := httptest.NewRecorder() + h.ServeHTTP(w, httptest.NewRequest(http.MethodGet, path, nil)) + return w + } + lastAE := func() string { + s, _ := sawAcceptEncoding.Load().(string) + return s + } + + first := get("/formula.jws.json") + if first.Code != http.StatusOK { + t.Fatalf("formula.jws.json: status = %d, want 200: %s", first.Code, first.Body.String()) + } + if got := lastAE(); got != "gzip" { + t.Errorf("formula.jws.json: upstream Accept-Encoding = %q, want %q", got, "gzip") + } + if !bytes.Equal(first.Body.Bytes(), compressed) { + t.Errorf("formula.jws.json: body is not the compressed bytes (got %d, want %d)", first.Body.Len(), len(compressed)) + } + if got := first.Header().Get(headerContentEncoding); got != "gzip" { + t.Errorf("formula.jws.json: Content-Encoding = %q, want %q", got, "gzip") + } + if got := first.Header().Get(headerContentLength); got != strconv.Itoa(len(compressed)) { + t.Errorf("formula.jws.json: Content-Length = %q, want %d", got, len(compressed)) + } + + // Replay from cache with the upstream down: same bytes and header, no refetch. + before := requests.Load() + available.Store(false) + cached := get("/formula.jws.json") + if cached.Code != http.StatusOK { + t.Fatalf("cached formula.jws.json: status = %d, want 200: %s", cached.Code, cached.Body.String()) + } + if !bytes.Equal(cached.Body.Bytes(), compressed) || cached.Header().Get(headerContentEncoding) != "gzip" { + t.Errorf("cached formula.jws.json: body/header not replayed verbatim") + } + if requests.Load() != before { + t.Errorf("cached formula.jws.json hit upstream: requests %d -> %d", before, requests.Load()) + } + available.Store(true) + + // Analytics is fetched by brew without --compressed: stays identity, no header. + analytics := get("/analytics/install/30d.json") + if analytics.Code != http.StatusOK { + t.Fatalf("analytics: status = %d, want 200: %s", analytics.Code, analytics.Body.String()) + } + if got := lastAE(); got != "identity" { + t.Errorf("analytics: upstream Accept-Encoding = %q, want %q", got, "identity") + } + if !bytes.Equal(analytics.Body.Bytes(), plain) { + t.Errorf("analytics: body = %q, want plain %q", analytics.Body.Bytes(), plain) + } + if got := analytics.Header().Get(headerContentEncoding); got != "" { + t.Errorf("analytics: Content-Encoding = %q, want empty", got) + } +} diff --git a/internal/handler/nuget.go b/internal/handler/nuget.go index 7df6240..3216f23 100644 --- a/internal/handler/nuget.go +++ b/internal/handler/nuget.go @@ -7,7 +7,6 @@ import ( "io" "net/http" "strings" - "time" ) const ( @@ -51,10 +50,12 @@ func (h *NuGetHandler) Routes() http.Handler { // Package content (downloads) mux.HandleFunc("GET /v3-flatcontainer/{id}/{version}/{filename}", h.handleDownload) - mux.HandleFunc("GET /v3-flatcontainer/{id}/index.json", h.proxyUpstream) + mux.HandleFunc("GET /v3-flatcontainer/{id}/index.json", h.handleVersionList) // Registration (package metadata) - use prefix matching since {version}.json isn't allowed - mux.HandleFunc("GET /v3/registration5-gz-semver2/", h.handleRegistration) + for _, prefix := range nugetRegistrationPrefixes { + mux.HandleFunc("GET "+prefix, h.handleRegistration) + } // Search mux.HandleFunc("GET /query", h.proxyUpstream) @@ -84,6 +85,10 @@ func (h *NuGetHandler) handleServiceIndex(w http.ResponseWriter, r *http.Request rewritten, err := h.rewriteServiceIndex(body) if err != nil { + if h.cooldownEnabled() { + h.nugetMetadataError(w, err) + return + } h.proxy.Logger.Warn("failed to rewrite service index, proxying original", "error", err) w.Header().Set(headerContentType, "application/json") _, _ = w.Write(body) @@ -131,6 +136,10 @@ func (h *NuGetHandler) rewriteNuGetURL(origURL, serviceType string) string { switch serviceType { case "PackageBaseAddress/3.0.0": return h.proxyURL + "/nuget/v3-flatcontainer/" + case "RegistrationsBaseUrl", "RegistrationsBaseUrl/3.0.0-beta", "RegistrationsBaseUrl/3.0.0-rc": + return h.proxyURL + "/nuget/v3/registration5-semver1/" + case "RegistrationsBaseUrl/3.4.0": + return h.proxyURL + "/nuget/v3/registration5-gz-semver1/" case "RegistrationsBaseUrl/3.6.0", "RegistrationsBaseUrl/Versioned": return h.proxyURL + "/nuget/v3/registration5-gz-semver2/" case "SearchQueryService", "SearchQueryService/3.0.0-rc", "SearchQueryService/3.5.0": @@ -142,140 +151,6 @@ func (h *NuGetHandler) rewriteNuGetURL(origURL, serviceType string) string { } } -// handleRegistration proxies NuGet registration pages, applying cooldown filtering. -func (h *NuGetHandler) handleRegistration(w http.ResponseWriter, r *http.Request) { - if h.proxy.Cooldown == nil || !h.proxy.Cooldown.Enabled() { - h.proxyUpstream(w, r) - return - } - - upstreamURL := h.buildUpstreamURL(r) - - h.proxy.Logger.Debug("fetching registration for cooldown filtering", "url", upstreamURL) - - req, err := http.NewRequestWithContext(r.Context(), http.MethodGet, upstreamURL, nil) - if err != nil { - http.Error(w, "failed to create request", http.StatusInternalServerError) - return - } - req.Header.Set(headerAcceptEncoding, "gzip") - - resp, err := h.proxy.HTTPClient.Do(req) - if err != nil { - h.proxy.Logger.Error("upstream request failed", "error", err) - http.Error(w, "upstream request failed", http.StatusBadGateway) - return - } - defer func() { _ = resp.Body.Close() }() - - if resp.StatusCode != http.StatusOK { - for k, vv := range resp.Header { - for _, v := range vv { - w.Header().Add(k, v) - } - } - w.WriteHeader(resp.StatusCode) - _, _ = io.Copy(w, resp.Body) - return - } - - body, err := h.proxy.ReadMetadata(resp.Body) - if err != nil { - http.Error(w, "failed to read response", http.StatusInternalServerError) - return - } - - filtered, err := h.applyCooldownFiltering(body) - if err != nil { - h.proxy.Logger.Warn("failed to filter registration, proxying original", "error", err) - w.Header().Set(headerContentType, "application/json") - _, _ = w.Write(body) - return - } - - w.Header().Set(headerContentType, "application/json") - _, _ = w.Write(filtered) -} - -// applyCooldownFiltering filters versions from NuGet registration pages -// that are too recently published. -func (h *NuGetHandler) applyCooldownFiltering(body []byte) ([]byte, error) { - if h.proxy.Cooldown == nil || !h.proxy.Cooldown.Enabled() { - return body, nil - } - - var registration map[string]any - if err := json.Unmarshal(body, ®istration); err != nil { - return nil, err - } - - pages, ok := registration["items"].([]any) - if !ok { - return body, nil - } - - for _, page := range pages { - pageMap, ok := page.(map[string]any) - if !ok { - continue - } - - items, ok := pageMap["items"].([]any) - if !ok { - continue - } - - filtered := items[:0] - for _, item := range items { - itemMap, ok := item.(map[string]any) - if !ok { - continue - } - - catalogEntry, ok := itemMap["catalogEntry"].(map[string]any) - if !ok { - filtered = append(filtered, item) - continue - } - - version, _ := catalogEntry["version"].(string) - id, _ := catalogEntry["id"].(string) - publishedStr, _ := catalogEntry["published"].(string) - - if publishedStr == "" { - filtered = append(filtered, item) - continue - } - - publishedAt, err := time.Parse(time.RFC3339, publishedStr) - if err != nil { - // NuGet uses a slightly non-standard format, try parsing with fractional seconds - publishedAt, err = time.Parse("2006-01-02T15:04:05.999-07:00", publishedStr) - if err != nil { - filtered = append(filtered, item) - continue - } - } - - packagePURL := canonicalPackagePURL("nuget", strings.ToLower(id)) - - if !h.proxy.Cooldown.IsAllowed("nuget", packagePURL, publishedAt) { - h.proxy.Logger.Info("cooldown: filtering nuget version", - "package", id, "version", version, - "published", publishedStr) - continue - } - - filtered = append(filtered, item) - } - - pageMap["items"] = filtered - pageMap["count"] = len(filtered) - } - - return json.Marshal(registration) -} - // handleDownload serves a package file, fetching and caching from upstream if needed. func (h *NuGetHandler) handleDownload(w http.ResponseWriter, r *http.Request) { id := r.PathValue("id") @@ -287,6 +162,18 @@ func (h *NuGetHandler) handleDownload(w http.ResponseWriter, r *http.Request) { return } + if h.cooldownEnabled() { + allowed, err := h.nugetDownloadAllowed(r.Context(), id, version) + if err != nil { + h.nugetMetadataError(w, err) + return + } + if !allowed { + JSONError(w, http.StatusNotFound, "version not found") + return + } + } + // Only cache .nupkg files if !strings.HasSuffix(filename, ".nupkg") { h.proxyUpstream(w, r) diff --git a/internal/handler/nuget_cooldown.go b/internal/handler/nuget_cooldown.go new file mode 100644 index 0000000..6b95ae9 --- /dev/null +++ b/internal/handler/nuget_cooldown.go @@ -0,0 +1,374 @@ +package handler + +import ( + "bytes" + "compress/gzip" + "context" + "crypto/sha256" + "encoding/json" + "errors" + "fmt" + "net/http" + "net/url" + "slices" + "strings" + "time" +) + +var nugetRegistrationPrefixes = []string{ + "/v3/registration5-semver1/", + "/v3/registration5-gz-semver1/", + "/v3/registration5-gz-semver2/", +} + +var nugetArtifactPrefixes = append([]string{"/v3-flatcontainer/"}, nugetRegistrationPrefixes...) + +const nugetRegistrationPath = "/v3/registration5-gz-semver2/" + +func (h *NuGetHandler) cooldownEnabled() bool { + return h.proxy.Cooldown != nil && h.proxy.Cooldown.Enabled() +} + +func (h *NuGetHandler) nugetCooldownApplies(id string) bool { + return h.cooldownEnabled() && h.proxy.Cooldown.For("nuget", canonicalPackagePURL("nuget", strings.ToLower(id))) > 0 +} + +// Cache upstream documents, not filtered results, so policy changes and elapsed +// time take effect even while metadata is fresh. Include the upstream in the key. +func (h *NuGetHandler) nugetMetadata(ctx context.Context, path string) (map[string]any, error) { + target := h.upstreamURL + path + key := fmt.Sprintf("_cooldown/%x", sha256.Sum256([]byte(target))) + var document map[string]any + validate := func(body []byte) error { + var err error + document, err = h.decodeNuGetMetadata(body) + return err + } + _, _, _, err := h.proxy.fetchOrCacheMetadata(ctx, "nuget", key, target, "", validate) + if err != nil { + return nil, err + } + return document, nil +} + +func (h *NuGetHandler) decodeNuGetMetadata(body []byte) (map[string]any, error) { + // Normally the HTTP transport decodes gzip. Also support compressed cached + // bytes and clients with transparent decompression disabled, with the same + // metadata limit applied to the decompressed document. + if bytes.HasPrefix(body, []byte{0x1f, 0x8b}) { + reader, err := gzip.NewReader(bytes.NewReader(body)) + if err != nil { + return nil, err + } + defer func() { _ = reader.Close() }() + body, err = h.proxy.ReadMetadata(reader) + if err != nil { + return nil, err + } + } + var document map[string]any + if err := json.Unmarshal(body, &document); err != nil { + return nil, fmt.Errorf("parsing NuGet metadata: %w", err) + } + if document == nil { + return nil, fmt.Errorf("empty NuGet metadata") + } + return document, nil +} + +// Prefer semver2, but a configured source may advertise only an older hive. +// Retry only advertised aliases on 404; transport/validation errors must not +// silently switch to a hive with less complete metadata. Keep requests on the +// configured upstream, consistent with the service-index route rewriting. +func (h *NuGetHandler) nugetRegistrationMetadata(ctx context.Context, suffix string) (map[string]any, string, error) { + path := nugetRegistrationPath + suffix + document, err := h.nugetMetadata(ctx, path) + if !errors.Is(err, ErrUpstreamNotFound) { + return document, path, err + } + index, indexErr := h.nugetMetadata(ctx, "/v3/index.json") + if indexErr != nil { + return nil, path, indexErr + } + resources, _ := index["resources"].([]any) + seen := map[string]bool{nugetRegistrationPath: true} + for _, resource := range resources { + entry, _ := resource.(map[string]any) + service, _ := entry["@type"].(string) + id, _ := entry["@id"].(string) + if id == "" || !strings.HasPrefix(service, "RegistrationsBaseUrl") { + continue + } + prefix := strings.TrimPrefix(h.rewriteNuGetURL(id, service), h.proxyURL+"/nuget") + if !slices.Contains(nugetRegistrationPrefixes, prefix) || seen[prefix] { + continue + } + seen[prefix] = true + path = prefix + suffix + document, err = h.nugetMetadata(ctx, path) + if !errors.Is(err, ErrUpstreamNotFound) { + return document, path, err + } + } + return nil, path, err +} + +func (h *NuGetHandler) nugetMetadataError(w http.ResponseWriter, err error) { + if errors.Is(err, ErrUpstreamNotFound) { + JSONError(w, http.StatusNotFound, "package metadata not found") + return + } + h.proxy.Logger.Warn("failed to process NuGet metadata", "error", err) + JSONError(w, http.StatusBadGateway, "failed to process package metadata") +} + +func (h *NuGetHandler) handleVersionList(w http.ResponseWriter, r *http.Request) { + if !h.cooldownEnabled() { + h.proxyUpstream(w, r) + return + } + id := strings.ToLower(r.PathValue("id")) + document, err := h.nugetMetadata(r.Context(), "/v3-flatcontainer/"+url.PathEscape(id)+"/index.json") + if err != nil { + h.nugetMetadataError(w, err) + return + } + blocked := make(map[string]bool) + // A globally enabled policy may still exempt this package or ecosystem. + // Keep metadata caching, but do not require publication data in that case. + if h.nugetCooldownApplies(id) { + registration, registrationPath, err := h.nugetRegistrationMetadata(r.Context(), url.PathEscape(id)+"/index.json") + if err == nil { + err = h.expandNuGetPages(r.Context(), registration, registrationPath) + } + if err != nil { + h.nugetMetadataError(w, err) + return + } + h.collectNuGetBlockedVersions(registration, id, blocked) + } + versions, ok := document["versions"].([]any) + if !ok { + h.nugetMetadataError(w, fmt.Errorf("missing NuGet versions")) + return + } + filtered := make([]any, 0, len(versions)) + for _, value := range versions { + version, ok := value.(string) + if !ok { + h.nugetMetadataError(w, fmt.Errorf("invalid NuGet version")) + return + } + if !blocked[nugetVersionKey(version)] { + filtered = append(filtered, value) + } + } + document["versions"] = filtered + w.Header().Set(headerContentType, contentTypeJSON) + _ = json.NewEncoder(w).Encode(document) +} + +func nugetVersionKey(version string) string { + version, _, _ = strings.Cut(version, "+") + return strings.ToLower(version) +} + +func (h *NuGetHandler) nugetDownloadAllowed(ctx context.Context, id, version string) (bool, error) { + if !h.nugetCooldownApplies(id) { + return true, nil + } + suffix := url.PathEscape(strings.ToLower(id)) + "/" + url.PathEscape(nugetVersionKey(version)) + ".json" + leaf, _, err := h.nugetRegistrationMetadata(ctx, suffix) + if err != nil { + return false, err + } + return h.nugetLeafAllowed(leaf, id), nil +} + +// A standalone leaf has published at its root; leaves embedded in pages carry +// it in catalogEntry. Missing/invalid timestamps retain the existing permissive +// behavior, but fetch and JSON errors must not bypass the policy. +func (h *NuGetHandler) nugetLeafAllowed(leaf map[string]any, id string) bool { + if !h.cooldownEnabled() { + return true + } + entry := nugetCatalogEntry(leaf) + if id == "" { + id, _ = entry["id"].(string) + } + published, _ := entry["published"].(string) + when, err := time.Parse(time.RFC3339, published) + if err != nil { + return true + } + return h.proxy.Cooldown.IsAllowed("nuget", canonicalPackagePURL("nuget", strings.ToLower(id)), when) +} + +func nugetCatalogEntry(leaf map[string]any) map[string]any { + if entry, ok := leaf["catalogEntry"].(map[string]any); ok { + return entry + } + return leaf +} + +func (h *NuGetHandler) collectNuGetBlockedVersions(document map[string]any, id string, blocked map[string]bool) { + entry := nugetCatalogEntry(document) + if version, ok := entry["version"].(string); ok && !h.nugetLeafAllowed(document, id) { + blocked[nugetVersionKey(version)] = true + } + items, _ := document["items"].([]any) + for _, item := range items { + if child, ok := item.(map[string]any); ok { + h.collectNuGetBlockedVersions(child, id, blocked) + } + } +} + +func (h *NuGetHandler) handleRegistration(w http.ResponseWriter, r *http.Request) { + if !h.cooldownEnabled() { + h.proxyUpstream(w, r) + return + } + id := nugetRegistrationID(r.URL.Path) + applyCooldown := h.nugetCooldownApplies(id) + document, err := h.nugetMetadata(r.Context(), r.URL.Path) + if err == nil && applyCooldown { + err = h.expandNuGetPages(r.Context(), document, r.URL.Path) + } + if err != nil { + h.nugetMetadataError(w, err) + return + } + _, hasItems := document["items"] + if applyCooldown && !h.filterNuGetRegistration(document, id) && !hasItems { + JSONError(w, http.StatusNotFound, "version not found") + return + } + h.rewriteNuGetRegistrationLinks(document) + w.Header().Set(headerContentType, contentTypeJSON) + _ = json.NewEncoder(w).Encode(document) +} + +func nugetRegistrationID(path string) string { + for _, prefix := range nugetRegistrationPrefixes { + if rest, ok := strings.CutPrefix(path, prefix); ok { + id, _, _ := strings.Cut(rest, "/") + return id + } + } + return "" +} + +// Only expand index pages, never recursively follow arbitrary upstream links. +// Pin requests to this configured upstream and the current package's page path. +func (h *NuGetHandler) expandNuGetPages(ctx context.Context, document map[string]any, path string) error { + if !strings.HasSuffix(path, "/index.json") { + return nil + } + items, ok := document["items"].([]any) + if !ok { + return fmt.Errorf("missing registration pages") + } + base, err := url.Parse(h.upstreamURL + path) + if err != nil { + return err + } + pagePrefix := strings.TrimSuffix(base.Path, "index.json") + "page/" + for _, item := range items { + page, ok := item.(map[string]any) + if !ok { + return fmt.Errorf("invalid registration page") + } + if _, ok := page["items"].([]any); ok { + continue + } + link, _ := page["@id"].(string) + target, err := base.Parse(link) + if err != nil || target.Scheme != base.Scheme || target.Host != base.Host || + !strings.HasPrefix(target.Path, pagePrefix) || containsPathTraversal(target.Path) || target.RawQuery != "" || target.Fragment != "" { + return fmt.Errorf("invalid registration page URL: %q", link) + } + upstream, _ := url.Parse(h.upstreamURL) + pageDocument, err := h.nugetMetadata(ctx, strings.TrimPrefix(target.Path, upstream.Path)) + if err != nil { + return err + } + leaves, ok := pageDocument["items"].([]any) + if !ok { + return fmt.Errorf("missing registration leaves") + } + page["items"] = leaves + } + return nil +} + +func (h *NuGetHandler) filterNuGetRegistration(document map[string]any, id string) bool { + items, ok := document["items"].([]any) + if !ok { + return h.nugetLeafAllowed(document, id) + } + filtered := make([]any, 0, len(items)) + for _, item := range items { + child, ok := item.(map[string]any) + if ok && h.filterNuGetRegistration(child, id) { + filtered = append(filtered, child) + } + } + document["items"] = filtered + document["count"] = len(filtered) + // Page bounds describe the retained leaves, not versions hidden by cooldown. + if _, isPage := document["lower"]; isPage && len(filtered) > 0 { + first, _ := filtered[0].(map[string]any) + last, _ := filtered[len(filtered)-1].(map[string]any) + document["lower"] = nugetCatalogEntry(first)["version"] + document["upper"] = nugetCatalogEntry(last)["version"] + } + return len(filtered) > 0 +} + +func (h *NuGetHandler) rewriteNuGetRegistrationLinks(value any) { + switch node := value.(type) { + case map[string]any: + for key, child := range node { + if link, ok := child.(string); ok { + switch key { + case "@id", "parent", "registration", "packageContent": + node[key] = h.nugetProxyLink(link) + } + } else { + h.rewriteNuGetRegistrationLinks(child) + } + } + case []any: + for _, child := range node { + h.rewriteNuGetRegistrationLinks(child) + } + } +} + +func (h *NuGetHandler) nugetProxyLink(link string) string { + u, err := url.Parse(link) + if err != nil { + return link + } + upstream, err := url.Parse(h.upstreamURL) + if err != nil { + return link + } + path := u.Path + if u.Host == upstream.Host { + path = strings.TrimPrefix(path, upstream.Path) + } + for _, prefix := range nugetArtifactPrefixes { + if strings.HasPrefix(path, prefix) { + proxy, err := url.Parse(h.proxyURL + "/nuget" + path) + if err != nil { + return link + } + proxy.RawQuery = u.RawQuery + proxy.Fragment = u.Fragment + return proxy.String() + } + } + return link +} diff --git a/internal/handler/nuget_cooldown_test.go b/internal/handler/nuget_cooldown_test.go new file mode 100644 index 0000000..74258da --- /dev/null +++ b/internal/handler/nuget_cooldown_test.go @@ -0,0 +1,501 @@ +package handler + +import ( + "bytes" + "compress/gzip" + "encoding/json" + "errors" + "io" + "net/http" + "net/http/httptest" + "strings" + "sync/atomic" + "testing" + "time" + + "github.com/git-pkgs/cooldown" + "github.com/git-pkgs/registries/fetch" +) + +func TestNuGetCooldownRoutes(t *testing.T) { + for _, disableCompression := range []bool{false, true} { + t.Run(map[bool]string{false: "transport gzip", true: "explicit gzip"}[disableCompression], func(t *testing.T) { + proxy, db, store, fetcher := setupTestProxy(t) + proxy.Cooldown = &cooldown.Config{Default: "14d"} + proxy.CacheMetadata = true + proxy.MetadataTTL = time.Hour + seedPackage(t, db, store, "nuget", "testpkg", "2.0.0", "testpkg.2.0.0.nupkg", "cached package") + seedPackage(t, db, store, "nuget", "testpkg", "1.0.0", "testpkg.1.0.0.nupkg", "old package") + metadataRequests := 0 + upstream := newNuGetCooldownUpstream(t, &metadataRequests) + defer upstream.Close() + transport := http.DefaultTransport.(*http.Transport).Clone() + transport.DisableCompression = disableCompression + defer transport.CloseIdleConnections() + proxy.HTTPClient = &http.Client{Transport: transport} + h := NewNuGetHandlerWithUpstreams(proxy, "http://proxy.test", upstream.URL, upstream.URL) + routes := http.StripPrefix("/nuget", h.Routes()) + get := func(path string, status int) *httptest.ResponseRecorder { + t.Helper() + return nugetGet(t, routes, path, status) + } + list := get("/nuget/v3-flatcontainer/testpkg/index.json", http.StatusOK) + if got := strings.TrimSpace(list.Body.String()); got != `{"versions":["1.0.0"]}` { + t.Fatalf("filtered list = %s", got) + } + index := get("/nuget"+nugetRegistrationPath+"testpkg/index.json", http.StatusOK) + if strings.Contains(index.Body.String(), `"version":"2.0.0"`) || strings.Contains(index.Body.String(), upstream.URL) { + t.Fatalf("registration leaks blocked leaf or upstream link: %s", index.Body.String()) + } + if index.Header().Get("Content-Encoding") != "" || !json.Valid(index.Body.Bytes()) { + t.Fatal("registration must be decoded JSON") + } + var doc struct { + Items []struct { + ID string `json:"@id"` + Count int + Lower, Upper string + Items []struct { + ID string `json:"@id"` + PackageContent string + } + } + } + if err := json.Unmarshal(index.Body.Bytes(), &doc); err != nil { + t.Fatal(err) + } + if len(doc.Items) != 1 || doc.Items[0].Count != 1 || doc.Items[0].Upper != "1.0.0" { + t.Fatalf("incorrect page: %+v", doc) + } + get(doc.Items[0].ID, http.StatusOK) + get(doc.Items[0].Items[0].ID, http.StatusOK) + get(doc.Items[0].Items[0].PackageContent, http.StatusOK) + get("/nuget"+nugetRegistrationPath+"testpkg/2.0.0.json", http.StatusNotFound) + get("/nuget/v3-flatcontainer/TestPkg/2.0.0/testpkg.2.0.0.nupkg", http.StatusNotFound) + get("/nuget/v3-flatcontainer/testpkg/2.0.0/testpkg.nuspec", http.StatusNotFound) + if fetcher.fetchCalled { + t.Fatal("blocked or cached downloads must not fetch artifacts") + } + + // Reevaluate fresh, unfiltered metadata under a changed package policy. + requestsBefore := metadataRequests + proxy.Cooldown = &cooldown.Config{Default: "14d", Packages: map[string]string{"pkg:nuget/testpkg": "1d"}} + list = get("/nuget/v3-flatcontainer/testpkg/index.json", http.StatusOK) + if !strings.Contains(list.Body.String(), "2.0.0") { + t.Fatal("fresh metadata retained the previous policy") + } + get("/nuget/v3-flatcontainer/testpkg/2.0.0/testpkg.2.0.0.nupkg", http.StatusOK) + if metadataRequests != requestsBefore { + t.Fatal("fresh metadata should be reused") + } + }) + } +} + +func nugetGet(t *testing.T, routes http.Handler, path string, status int) *httptest.ResponseRecorder { + t.Helper() + w := httptest.NewRecorder() + routes.ServeHTTP(w, httptest.NewRequest(http.MethodGet, path, nil)) + if w.Code != status { + t.Fatalf("GET %s: status %d, want %d: %s", path, w.Code, status, w.Body.String()) + } + return w +} + +func TestNuGetMetadataWithoutEffectiveCooldown(t *testing.T) { + for _, tt := range []struct { + name string + policy *cooldown.Config + }{ + {"package exemption", &cooldown.Config{Default: "14d", Packages: map[string]string{"pkg:nuget/testpkg": "0"}}}, + {"ecosystem exemption", &cooldown.Config{Default: "14d", Ecosystems: map[string]string{"nuget": "0"}}}, + {"other ecosystem only", &cooldown.Config{Ecosystems: map[string]string{"npm": "14d"}}}, + {"other package only", &cooldown.Config{Packages: map[string]string{"pkg:nuget/other": "14d"}}}, + } { + t.Run(tt.name, func(t *testing.T) { + const body = `{"versions":["1.0.0","2.0.0"]}` + const pagePath = nugetRegistrationPath + "testpkg/page/1.0.0/2.0.0.json" + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/v3-flatcontainer/testpkg/index.json": + _, _ = io.WriteString(w, body) + case nugetRegistrationPath + "testpkg/index.json": + _, _ = io.WriteString(w, `{"count":1,"items":[{"@id":"`+pagePath+`","count":2,"lower":"1.0.0","upper":"2.0.0"}]}`) + default: + t.Errorf("unnecessary registration request: %s", r.URL.Path) + http.Error(w, "registration unavailable", http.StatusServiceUnavailable) + } + })) + defer upstream.Close() + p := nugetTestProxy() + p.Cooldown = tt.policy + h := NewNuGetHandlerWithUpstreams(p, "http://proxy.test", upstream.URL, upstream.URL) + w := nugetGet(t, h.Routes(), "/v3-flatcontainer/TestPkg/index.json", http.StatusOK) + if got := strings.TrimSpace(w.Body.String()); got != body { + t.Fatalf("version list = %s, want %s", got, body) + } + w = nugetGet(t, h.Routes(), nugetRegistrationPath+"testpkg/index.json", http.StatusOK) + if !strings.Contains(w.Body.String(), `"@id":"http://proxy.test/nuget`+pagePath+`"`) { + t.Fatalf("registration page link was not rewritten: %s", w.Body.String()) + } + }) + } +} + +func TestNuGetCooldownColdDownload(t *testing.T) { + for _, tt := range []struct { + name, published string + policy *cooldown.Config + want int + }{ + {"recent", time.Now().Add(-time.Hour).Format(time.RFC3339), &cooldown.Config{Default: "14d"}, http.StatusNotFound}, + {"missing timestamp", "", &cooldown.Config{Default: "14d"}, http.StatusOK}, + {"package exemption", time.Now().Add(-time.Hour).Format(time.RFC3339), &cooldown.Config{Default: "14d", Packages: map[string]string{"pkg:nuget/testpkg": "0"}}, http.StatusOK}, + {"ecosystem override", time.Now().Add(-time.Hour).Format(time.RFC3339), &cooldown.Config{Ecosystems: map[string]string{"nuget": "14d"}}, http.StatusNotFound}, + } { + t.Run(tt.name, func(t *testing.T) { + p, _, _, fetcher := setupTestProxy(t) + p.Cooldown = tt.policy + fetcher.artifact = &fetch.Artifact{Body: io.NopCloser(strings.NewReader("package")), ContentType: "application/octet-stream"} + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + _ = json.NewEncoder(w).Encode(map[string]string{"published": tt.published}) + })) + defer upstream.Close() + h := NewNuGetHandlerWithUpstreams(p, "http://proxy.test", upstream.URL, upstream.URL) + nugetGet(t, h.Routes(), "/v3-flatcontainer/testpkg/2.0.0/testpkg.2.0.0.nupkg", tt.want) + if fetcher.fetchCalled != (tt.want == http.StatusOK) { + t.Errorf("artifact fetch called = %v", fetcher.fetchCalled) + } + }) + } +} + +func TestNuGetRegistrationServiceAliases(t *testing.T) { + h := NewNuGetHandler(nugetTestProxy(), "http://proxy.test") + for _, tt := range []struct{ service, path string }{ + {"RegistrationsBaseUrl", "/v3/registration5-semver1/"}, + {"RegistrationsBaseUrl/3.0.0-beta", "/v3/registration5-semver1/"}, + {"RegistrationsBaseUrl/3.0.0-rc", "/v3/registration5-semver1/"}, + {"RegistrationsBaseUrl/3.4.0", "/v3/registration5-gz-semver1/"}, + {"RegistrationsBaseUrl/3.6.0", nugetRegistrationPath}, + {"RegistrationsBaseUrl/Versioned", nugetRegistrationPath}, + } { + t.Run(tt.service, func(t *testing.T) { + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != tt.path+"testpkg/index.json" { + t.Errorf("wrong hive: %s", r.URL.Path) + } + _, _ = io.WriteString(w, `{"count":0,"items":[]}`) + })) + defer upstream.Close() + h.upstreamURL = upstream.URL + h.proxy.Cooldown = &cooldown.Config{Default: "14d"} + body := []byte(`{"resources":[{"@id":"` + upstream.URL + tt.path + `","@type":"` + tt.service + `"}]}`) + out, err := h.rewriteServiceIndex(body) + if err != nil { + t.Fatal(err) + } + var doc struct { + Resources []struct { + ID string `json:"@id"` + } + } + if err := json.Unmarshal(out, &doc); err != nil { + t.Fatal(err) + } + w := httptest.NewRecorder() + http.StripPrefix("/nuget", h.Routes()).ServeHTTP(w, httptest.NewRequest(http.MethodGet, doc.Resources[0].ID+"testpkg/index.json", nil)) + if w.Code != http.StatusOK { + t.Fatalf("alias route status = %d: %s", w.Code, w.Body.String()) + } + }) + } +} + +func TestNuGetCooldownMetadataErrors(t *testing.T) { + for _, tt := range []struct { + name, body string + status int + }{ + {"upstream failure", "unavailable", http.StatusServiceUnavailable}, + {"invalid JSON", "broken JSON", http.StatusOK}, + {"null", "null", http.StatusOK}, + } { + t.Run(tt.name, func(t *testing.T) { + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(tt.status) + _, _ = io.WriteString(w, tt.body) + })) + defer upstream.Close() + p := nugetTestProxy() + p.Cooldown = &cooldown.Config{Default: "14d"} + h := NewNuGetHandlerWithUpstreams(p, "http://proxy.test", upstream.URL, upstream.URL) + for _, path := range []string{"/v3-flatcontainer/testpkg/index.json", "/v3-flatcontainer/testpkg/2.0.0/testpkg.2.0.0.nupkg", nugetRegistrationPath + "testpkg/index.json"} { + w := httptest.NewRecorder() + h.Routes().ServeHTTP(w, httptest.NewRequest(http.MethodGet, path, nil)) + if w.Code != http.StatusBadGateway { + t.Errorf("GET %s: %d, want 502", path, w.Code) + } + } + }) + } +} + +func TestNuGetCooldownRejectsUnsafePageLinks(t *testing.T) { + for _, link := range []string{"https://other.example/page.json", "/v3/registration5-gz-semver2/other/page/1/2.json", "page/../index.json", "index.json"} { + t.Run(link, func(t *testing.T) { + requests := 0 + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + requests++ + _ = json.NewEncoder(w).Encode(map[string]any{"items": []any{map[string]any{"@id": link}}}) + })) + defer upstream.Close() + p := nugetTestProxy() + p.Cooldown = &cooldown.Config{Default: "14d"} + h := NewNuGetHandlerWithUpstreams(p, "http://proxy.test", upstream.URL, upstream.URL) + nugetGet(t, h.Routes(), nugetRegistrationPath+"testpkg/index.json", http.StatusBadGateway) + if requests != 1 { + t.Fatalf("unsafe page link was followed (%d requests)", requests) + } + }) + } +} + +func TestNuGetCooldownDecompressedMetadataLimit(t *testing.T) { + var compressed bytes.Buffer + gz := gzip.NewWriter(&compressed) + _, _ = io.WriteString(gz, `{"padding":"`+strings.Repeat("x", 2048)+`"}`) + _ = gz.Close() + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Encoding", "gzip") + _, _ = w.Write(compressed.Bytes()) + })) + defer upstream.Close() + p := nugetTestProxy() + p.Cooldown = &cooldown.Config{Default: "14d"} + p.MetadataMaxSize = 1024 + transport := http.DefaultTransport.(*http.Transport).Clone() + transport.DisableCompression = true + defer transport.CloseIdleConnections() + p.HTTPClient = &http.Client{Transport: transport} + h := NewNuGetHandlerWithUpstreams(p, "http://proxy.test", upstream.URL, upstream.URL) + nugetGet(t, h.Routes(), nugetRegistrationPath+"testpkg/index.json", http.StatusBadGateway) +} + +func newNuGetCooldownUpstream(t *testing.T, metadataRequests *int) *httptest.Server { + t.Helper() + var upstream *httptest.Server + upstream = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + (*metadataRequests)++ + base := upstream.URL + nugetRegistrationPath + "testpkg/" + leaf := func(version string, age time.Duration) map[string]any { + return map[string]any{ + "@id": base + version + ".json", + "packageContent": upstream.URL + "/v3-flatcontainer/testpkg/" + version + "/testpkg." + version + ".nupkg", + "catalogEntry": map[string]any{"id": "TestPkg", "version": version, "published": time.Now().Add(-age).Format(time.RFC3339)}, + } + } + page := map[string]any{"@id": base + "page/1.0.0/2.0.0.json", "lower": "1.0.0", "upper": "2.0.0", "count": 2, + "parent": base + "index.json", "items": []any{leaf("1.0.0", 30*24*time.Hour), leaf("2.0.0", 2*24*time.Hour)}} + var body any + switch r.URL.Path { + case "/v3-flatcontainer/testpkg/index.json": + body = map[string]any{"versions": []string{"1.0.0", "2.0.0"}} + case nugetRegistrationPath + "testpkg/index.json": + // This index deliberately does not inline its leaves. + body = map[string]any{"count": 1, "items": []any{map[string]any{ + "@id": page["@id"], "count": 2, "lower": "1.0.0", "upper": "2.0.0", + }}} + case nugetRegistrationPath + "testpkg/page/1.0.0/2.0.0.json": + body = page + case nugetRegistrationPath + "testpkg/1.0.0.json": + body = map[string]any{"published": time.Now().Add(-30 * 24 * time.Hour).Format(time.RFC3339)} + case nugetRegistrationPath + "testpkg/2.0.0.json": + body = map[string]any{"published": time.Now().Add(-2 * 24 * time.Hour).Format(time.RFC3339)} + default: + t.Errorf("unexpected metadata request: %s", r.URL.Path) + http.NotFound(w, r) + return + } + w.Header().Set("Content-Type", "application/json") + w.Header().Set("Content-Encoding", "gzip") + gz := gzip.NewWriter(w) + _ = json.NewEncoder(gz).Encode(body) + _ = gz.Close() + })) + + return upstream +} + +func TestNuGetCooldownLegacyRegistration(t *testing.T) { + for _, service := range []string{"RegistrationsBaseUrl", "RegistrationsBaseUrl/3.0.0-beta", "RegistrationsBaseUrl/3.0.0-rc", "RegistrationsBaseUrl/3.4.0"} { + t.Run(service, func(t *testing.T) { + prefix := "/v3/registration5-semver1/" + if service == "RegistrationsBaseUrl/3.4.0" { + prefix = "/v3/registration5-gz-semver1/" + } + upstream := newNuGetLegacyUpstream(t, service, prefix) + defer upstream.Close() + p, db, store, fetcher := setupTestProxy(t) + p.Cooldown = &cooldown.Config{Default: "14d"} + seedPackage(t, db, store, "nuget", "testpkg", "1.0.0", "testpkg.1.0.0.nupkg", "cached old package") + seedPackage(t, db, store, "nuget", "testpkg", "2.0.0", "testpkg.2.0.0.nupkg", "cached recent package") + h := NewNuGetHandlerWithUpstreams(p, "http://proxy.test", upstream.URL+"/feed", upstream.URL) + list := nugetGet(t, h.Routes(), "/v3-flatcontainer/testpkg/index.json", http.StatusOK) + if strings.TrimSpace(list.Body.String()) != `{"versions":["1.0.0"]}` { + t.Fatalf("incorrect version list: %s", list.Body.String()) + } + nugetGet(t, h.Routes(), "/v3-flatcontainer/testpkg/1.0.0/testpkg.1.0.0.nupkg", http.StatusOK) + nugetGet(t, h.Routes(), "/v3-flatcontainer/testpkg/2.0.0/testpkg.2.0.0.nupkg", http.StatusNotFound) + if fetcher.fetchCalled { + t.Fatal("cached or blocked package must not be fetched") + } + }) + } +} + +func newNuGetLegacyUpstream(t *testing.T, service, prefix string) *httptest.Server { + t.Helper() + var upstream *httptest.Server + upstream = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + path := strings.TrimPrefix(r.URL.Path, "/feed") + base := upstream.URL + "/feed" + prefix + "testpkg/" + published := func(age time.Duration) string { return time.Now().Add(-age).Format(time.RFC3339) } + var body any + switch path { + case "/v3/index.json": + body = map[string]any{"resources": []any{map[string]string{"@id": upstream.URL + "/feed" + prefix, "@type": service}}} + case "/v3-flatcontainer/testpkg/index.json": + body = map[string]any{"versions": []string{"1.0.0", "2.0.0"}} + case prefix + "testpkg/index.json": + body = map[string]any{"items": []any{map[string]any{"@id": base + "page/1.0.0/2.0.0.json"}}} + case prefix + "testpkg/page/1.0.0/2.0.0.json": + body = map[string]any{"items": []any{ + map[string]any{"catalogEntry": map[string]string{"id": "testpkg", "version": "1.0.0", "published": published(30 * 24 * time.Hour)}}, + map[string]any{"catalogEntry": map[string]string{"id": "testpkg", "version": "2.0.0", "published": published(time.Hour)}}, + }} + case prefix + "testpkg/1.0.0.json": + body = map[string]string{"published": published(30 * 24 * time.Hour)} + case prefix + "testpkg/2.0.0.json": + body = map[string]string{"published": published(time.Hour)} + default: + if !strings.HasPrefix(path, nugetRegistrationPath) { + t.Errorf("unexpected request: %s", r.URL.Path) + } + http.NotFound(w, r) + return + } + _ = json.NewEncoder(w).Encode(body) + })) + return upstream +} + +func TestNuGetRegistrationDoesNotFallbackOnFailure(t *testing.T) { + for _, status := range []int{http.StatusServiceUnavailable, http.StatusUnauthorized, http.StatusOK} { + t.Run(http.StatusText(status), func(t *testing.T) { + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != nugetRegistrationPath+"testpkg/index.json" { + t.Errorf("must not switch registration hive on failure: %s", r.URL.Path) + } + w.WriteHeader(status) + _, _ = io.WriteString(w, "invalid metadata") + })) + defer upstream.Close() + h := NewNuGetHandlerWithUpstreams(nugetTestProxy(), "http://proxy.test", upstream.URL, upstream.URL) + if _, _, err := h.nugetRegistrationMetadata(t.Context(), "testpkg/index.json"); err == nil { + t.Fatal("expected metadata error") + } + }) + } +} + +func TestNuGetMetadataPreservesValidCache(t *testing.T) { + for _, invalid := range []string{"broken JSON", "null", "[]", string([]byte{0x1f, 0x8b, 0x00})} { + t.Run(invalid, func(t *testing.T) { + const good = `{"published":"2020-01-01T00:00:00Z"}` + var response atomic.Value + response.Store(good) + var requests atomic.Int32 + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if requests.Add(1) > 1 && r.Header.Get("If-None-Match") != `"good"` { + t.Errorf("cached ETag was replaced: %s", r.Header.Get("If-None-Match")) + } + body := response.Load().(string) + etag := `"good"` + if body == invalid { + etag = `"bad"` + } + w.Header().Set("ETag", etag) + _, _ = io.WriteString(w, body) + })) + defer upstream.Close() + p, _, _, _ := setupTestProxy(t) + p.CacheMetadata = true + h := NewNuGetHandlerWithUpstreams(p, "http://proxy.test", upstream.URL, upstream.URL) + check := func(want string) { + t.Helper() + doc, err := h.nugetMetadata(t.Context(), nugetRegistrationPath+"testpkg/1.0.0.json") + if err != nil || doc["published"] != want { + t.Fatalf("metadata = %v, err = %v, want publication %s", doc, err, want) + } + } + check("2020-01-01T00:00:00Z") + response.Store(invalid) + check("2020-01-01T00:00:00Z") // Bad 200 must fall back without overwriting. + p.MetadataTTL = time.Hour + check("2020-01-01T00:00:00Z") // The on-disk cache must still be usable. + if requests.Load() != 2 { + t.Fatalf("requests = %d, want 2", requests.Load()) + } + p.MetadataTTL = 0 + response.Store(`{"published":"2021-01-01T00:00:00Z"}`) + check("2021-01-01T00:00:00Z") // A later valid response replaces the cache. + }) + } +} + +func TestNuGetMetadataInvalidResponseNotCached(t *testing.T) { + var requests atomic.Int32 + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if requests.Add(1) == 1 { + _, _ = io.WriteString(w, "invalid JSON") + return + } + _, _ = io.WriteString(w, `{"published":"2020-01-01T00:00:00Z"}`) + })) + defer upstream.Close() + p, _, _, _ := setupTestProxy(t) + p.CacheMetadata = true + p.MetadataTTL = time.Hour + h := NewNuGetHandlerWithUpstreams(p, "http://proxy.test", upstream.URL, upstream.URL) + path := nugetRegistrationPath + "testpkg/1.0.0.json" + if _, err := h.nugetMetadata(t.Context(), path); err == nil { + t.Fatal("invalid response without a usable cache must fail") + } + if _, err := h.nugetMetadata(t.Context(), path); err != nil { + t.Fatalf("invalid response was cached: %v", err) + } + if requests.Load() != 2 { + t.Fatalf("requests = %d, want 2", requests.Load()) + } +} + +func TestNuGetRegistrationDoesNotGuessUnadvertisedAliases(t *testing.T) { + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case nugetRegistrationPath + "testpkg/index.json": + http.NotFound(w, r) + case "/v3/index.json": + _, _ = io.WriteString(w, `{"resources":[{"@type":"UnrelatedService","@id":"https://other.example/"}]}`) + default: + t.Errorf("unadvertised endpoint requested: %s", r.URL.Path) + http.NotFound(w, r) + } + })) + defer upstream.Close() + h := NewNuGetHandlerWithUpstreams(nugetTestProxy(), "http://proxy.test", upstream.URL, upstream.URL) + _, _, err := h.nugetRegistrationMetadata(t.Context(), "testpkg/index.json") + if !errors.Is(err, ErrUpstreamNotFound) { + t.Fatalf("error = %v, want metadata not found", err) + } +} diff --git a/internal/handler/nuget_test.go b/internal/handler/nuget_test.go index 09efe95..710b8ea 100644 --- a/internal/handler/nuget_test.go +++ b/internal/handler/nuget_test.go @@ -812,11 +812,6 @@ func TestNuGetCooldownFiltering(t *testing.T) { }, } - body, err := json.Marshal(registration) - if err != nil { - t.Fatal(err) - } - proxy := testProxy() proxy.Cooldown = &cooldown.Config{ Default: "3d", @@ -827,17 +822,11 @@ func TestNuGetCooldownFiltering(t *testing.T) { proxyURL: "http://localhost:8080", } - filtered, err := h.applyCooldownFiltering(body) - if err != nil { - t.Fatal(err) + if !h.filterNuGetRegistration(registration, "") { + t.Fatal("expected registration items to be retained") } - var result map[string]any - if err := json.Unmarshal(filtered, &result); err != nil { - t.Fatal(err) - } - - pages := result["items"].([]any) + pages := registration["items"].([]any) page := pages[0].(map[string]any) items := page["items"].([]any) @@ -851,7 +840,7 @@ func TestNuGetCooldownFiltering(t *testing.T) { } count := page["count"] - if count != float64(1) { + if count != 1 { t.Errorf("expected page count to be 1, got %v", count) } } @@ -877,11 +866,6 @@ func TestNuGetCooldownFilteringWithPackageOverride(t *testing.T) { }, } - body, err := json.Marshal(registration) - if err != nil { - t.Fatal(err) - } - proxy := testProxy() proxy.Cooldown = &cooldown.Config{ Default: "3d", @@ -893,17 +877,11 @@ func TestNuGetCooldownFilteringWithPackageOverride(t *testing.T) { proxyURL: "http://localhost:8080", } - filtered, err := h.applyCooldownFiltering(body) - if err != nil { - t.Fatal(err) + if !h.filterNuGetRegistration(registration, "") { + t.Fatal("expected registration items to be retained") } - var result map[string]any - if err := json.Unmarshal(filtered, &result); err != nil { - t.Fatal(err) - } - - pages := result["items"].([]any) + pages := registration["items"].([]any) page := pages[0].(map[string]any) items := page["items"].([]any) @@ -930,36 +908,20 @@ func TestNuGetCooldownNoCooldownConfig(t *testing.T) { }, } - body, err := json.Marshal(registration) - if err != nil { - t.Fatal(err) - } - - // No cooldown - applyCooldownFiltering still works, just doesn't filter + // No cooldown: all registration items are retained. h := &NuGetHandler{ proxy: testProxy(), proxyURL: "http://localhost:8080", } - filtered, err := h.applyCooldownFiltering(body) - if err != nil { - t.Fatal(err) + if !h.filterNuGetRegistration(registration, "") { + t.Fatal("expected registration items to be retained") } - var result map[string]any - if err := json.Unmarshal(filtered, &result); err != nil { - t.Fatal(err) - } - - pages := result["items"].([]any) + pages := registration["items"].([]any) page := pages[0].(map[string]any) items := page["items"].([]any) - // Without cooldown config on the handler, applyCooldownFiltering - // is called but proxy.Cooldown is nil, so IsAllowed is never called - // Actually, applyCooldownFiltering always runs the filter logic - - // but the caller (handleRegistration) short-circuits when cooldown is disabled. - // The function itself should still work fine with a nil Cooldown. if len(items) != 1 { t.Fatalf("expected 1 item, got %d", len(items)) } @@ -988,11 +950,6 @@ func TestNuGetCooldownFilteringNuGetTimestamp(t *testing.T) { }, } - body, err := json.Marshal(registration) - if err != nil { - t.Fatal(err) - } - proxy := testProxy() proxy.Cooldown = &cooldown.Config{ Default: "3d", @@ -1003,17 +960,11 @@ func TestNuGetCooldownFilteringNuGetTimestamp(t *testing.T) { proxyURL: "http://localhost:8080", } - filtered, err := h.applyCooldownFiltering(body) - if err != nil { - t.Fatal(err) + if !h.filterNuGetRegistration(registration, "") { + t.Fatal("expected registration items to be retained") } - var result map[string]any - if err := json.Unmarshal(filtered, &result); err != nil { - t.Fatal(err) - } - - pages := result["items"].([]any) + pages := registration["items"].([]any) page := pages[0].(map[string]any) items := page["items"].([]any) diff --git a/internal/handler/proxy_cached_encoding_test.go b/internal/handler/proxy_cached_encoding_test.go new file mode 100644 index 0000000..35c8793 --- /dev/null +++ b/internal/handler/proxy_cached_encoding_test.go @@ -0,0 +1,260 @@ +package handler + +import ( + "bytes" + "context" + "errors" + "net/http" + "net/http/httptest" + "strconv" + "strings" + "sync/atomic" + "testing" + "time" +) + +// gzipWhenAskedUpstream serves compressed bytes with Content-Encoding: gzip +// when the request advertises gzip, plain bytes otherwise, like a CDN that +// compresses on the fly. It records the last Accept-Encoding it saw and counts +// every request before the availability gate so a cache-miss refetch during a +// simulated outage is observable. +type gzipWhenAskedUpstream struct { + *httptest.Server + available atomic.Bool + requests atomic.Int32 + acceptEncoding atomic.Value // string +} + +func newGzipWhenAskedUpstream(plain, compressed []byte) *gzipWhenAskedUpstream { + u := &gzipWhenAskedUpstream{} + u.available.Store(true) + u.Server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + u.requests.Add(1) + u.acceptEncoding.Store(r.Header.Get(headerAcceptEncoding)) + if !u.available.Load() { + http.Error(w, "unavailable", http.StatusServiceUnavailable) + return + } + w.Header().Set(headerContentType, contentTypeJSON) + if strings.Contains(r.Header.Get(headerAcceptEncoding), "gzip") { + w.Header().Set(headerContentEncoding, "gzip") + _, _ = w.Write(compressed) + return + } + _, _ = w.Write(plain) + })) + return u +} + +func (u *gzipWhenAskedUpstream) sawAcceptEncoding() string { + s, _ := u.acceptEncoding.Load().(string) + return s +} + +// serveGzip issues one request through proxyCachedWithEncoding asking the +// upstream for gzip. +func serveGzip(proxy *Proxy, upstreamURL string) *httptest.ResponseRecorder { + w := httptest.NewRecorder() + r := httptest.NewRequest(http.MethodGet, "/index.json", nil) + proxy.proxyCachedWithEncoding(w, r, upstreamURL, "gzip-test", "index", "gzip", "*/*") + return w +} + +func assertGzipResponse(t *testing.T, label string, w *httptest.ResponseRecorder, compressed []byte) { + t.Helper() + if w.Code != http.StatusOK { + t.Fatalf("%s: status = %d, want 200: %s", label, w.Code, w.Body.String()) + } + if !bytes.Equal(w.Body.Bytes(), compressed) { + t.Errorf("%s: body is not the compressed bytes (got %d, want %d)", label, w.Body.Len(), len(compressed)) + } + if got := w.Header().Get(headerContentEncoding); got != "gzip" { + t.Errorf("%s: Content-Encoding = %q, want %q", label, got, "gzip") + } + if got := w.Header().Get(headerContentLength); got != strconv.Itoa(len(compressed)) { + t.Errorf("%s: Content-Length = %q, want %d", label, got, len(compressed)) + } +} + +// TestProxyCachedWithEncoding_GzipCachesAndReplays covers the cached path: +// requesting gzip upstream stores the compressed bytes plus Content-Encoding +// and replays both from cache without contacting the upstream again. +func TestProxyCachedWithEncoding_GzipCachesAndReplays(t *testing.T) { + plain := []byte(`{"packages":{}}`) + compressed := gzipPayload(t, plain) + upstream := newGzipWhenAskedUpstream(plain, compressed) + defer upstream.Close() + + proxy, _, _, _ := setupTestProxy(t) + proxy.CacheMetadata = true + proxy.MetadataTTL = time.Hour + proxy.HTTPClient = upstream.Client() + + first := serveGzip(proxy, upstream.URL+"/index.json") + assertGzipResponse(t, "first", first, compressed) + if got := upstream.sawAcceptEncoding(); got != "gzip" { + t.Errorf("upstream Accept-Encoding = %q, want %q", got, "gzip") + } + + before := upstream.requests.Load() + upstream.available.Store(false) + cached := serveGzip(proxy, upstream.URL+"/index.json") + assertGzipResponse(t, "cached", cached, compressed) + if upstream.requests.Load() != before { + t.Errorf("cached replay hit upstream: requests %d -> %d", before, upstream.requests.Load()) + } +} + +// TestProxyCachedWithEncoding_GzipStreamPath covers the cache_metadata=false +// branch: the streaming path must request gzip and forward Content-Encoding. +func TestProxyCachedWithEncoding_GzipStreamPath(t *testing.T) { + plain := []byte(`{"packages":{}}`) + compressed := gzipPayload(t, plain) + upstream := newGzipWhenAskedUpstream(plain, compressed) + defer upstream.Close() + + proxy, _, _, _ := setupTestProxy(t) + proxy.CacheMetadata = false + proxy.HTTPClient = upstream.Client() + + w := serveGzip(proxy, upstream.URL+"/index.json") + assertGzipResponse(t, "stream", w, compressed) + if got := upstream.sawAcceptEncoding(); got != "gzip" { + t.Errorf("stream path upstream Accept-Encoding = %q, want %q", got, "gzip") + } +} + +// TestProxyCachedWithEncoding_GzipSurvivesCacheWriteFailure covers the failure +// the gzip mode makes reachable: when the metadata cache write fails the +// freshly fetched body is still served, so its Content-Encoding must come from +// the fetch and not from the (unwritten) cache row -- otherwise gzip bytes go +// out labelled application/json with no Content-Encoding. +func TestProxyCachedWithEncoding_GzipSurvivesCacheWriteFailure(t *testing.T) { + plain := []byte(`{"packages":{}}`) + compressed := gzipPayload(t, plain) + upstream := newGzipWhenAskedUpstream(plain, compressed) + defer upstream.Close() + + proxy, _, store, _ := setupTestProxy(t) + proxy.CacheMetadata = true + proxy.MetadataTTL = time.Hour + proxy.HTTPClient = upstream.Client() + store.storeErr = errors.New("disk full") + + w := serveGzip(proxy, upstream.URL+"/index.json") + assertGzipResponse(t, "store-failure", w, compressed) +} + +// TestProxyCachedWithEncoding_GzipStaleFallbackKeepsEncoding pins the +// stale-fallback return: when the upstream fails after the entry has expired, +// the stored gzip blob is served with its Content-Encoding taken from the +// cache row. +func TestProxyCachedWithEncoding_GzipStaleFallbackKeepsEncoding(t *testing.T) { + plain := []byte(`{"packages":{}}`) + compressed := gzipPayload(t, plain) + upstream := newGzipWhenAskedUpstream(plain, compressed) + defer upstream.Close() + + proxy, _, _, _ := setupTestProxy(t) + proxy.CacheMetadata = true + proxy.MetadataTTL = 0 // every request revalidates; an upstream failure falls back to the stale row + proxy.HTTPClient = upstream.Client() + + first := serveGzip(proxy, upstream.URL+"/index.json") + assertGzipResponse(t, "first", first, compressed) + + upstream.available.Store(false) + stale := serveGzip(proxy, upstream.URL+"/index.json") + assertGzipResponse(t, "stale", stale, compressed) +} + +// TestProxyCachedWithEncoding_UpsertFailureDiscardsBlob covers the row-write +// failure: when the gzip blob is stored but the cache row cannot be updated, +// the blob must be discarded so a later stale fallback cannot serve gzip +// bytes with the previous row's encoding. The fresh response is still +// correct because its encoding comes from the fetch. +func TestProxyCachedWithEncoding_UpsertFailureDiscardsBlob(t *testing.T) { + plain := []byte(`{"packages":{}}`) + compressed := gzipPayload(t, plain) + upstream := newGzipWhenAskedUpstream(plain, compressed) + defer upstream.Close() + + proxy, db, store, _ := setupTestProxy(t) + proxy.CacheMetadata = true + proxy.MetadataTTL = 0 // every request revalidates + proxy.HTTPClient = upstream.Client() + + // Seed an identity row + plain blob, as every key has before the gzip rollout. + w := httptest.NewRecorder() + proxy.proxyCachedWithEncoding(w, httptest.NewRequest(http.MethodGet, "/index.json", nil), + upstream.URL+"/index.json", "gzip-test", "index", "identity", "*/*") + if w.Code != http.StatusOK { + t.Fatalf("seed status = %d, want 200", w.Code) + } + + // Now DB writes fail while reads keep working. + db.SetMaxOpenConns(1) + if _, err := db.Exec("PRAGMA query_only=1"); err != nil { + t.Fatalf("PRAGMA query_only=1: %v", err) + } + fresh := serveGzip(proxy, upstream.URL+"/index.json") + assertGzipResponse(t, "fresh with failed row write", fresh, compressed) + + storagePath := metadataStoragePath("gzip-test", "index") + if exists, _ := store.Exists(context.Background(), storagePath); exists { + t.Fatalf("blob %s still present after the row write failed", storagePath) + } + + // Upstream down: the stale fallback must not serve the orphaned gzip + // blob under the old identity row. + if _, err := db.Exec("PRAGMA query_only=0"); err != nil { + t.Fatalf("PRAGMA query_only=0: %v", err) + } + upstream.available.Store(false) + stale := serveGzip(proxy, upstream.URL+"/index.json") + if stale.Code == http.StatusOK { + t.Fatalf("stale fallback served status 200 (Content-Encoding=%q, %d bytes) from an orphaned blob; want an error", + stale.Header().Get(headerContentEncoding), stale.Body.Len()) + } +} + +// TestProxyCachedWithEncoding_StaleFallbackRereadsRow covers the rollout race: +// a request that read the identity row, lost the upstream race to a request +// that stored the gzip blob, and then failed upstream must label the blob +// with the row as it is now, not with the row it read at the start. +func TestProxyCachedWithEncoding_StaleFallbackRereadsRow(t *testing.T) { + plain := []byte(`{"packages":{}}`) + compressed := gzipPayload(t, plain) + + var proxy *Proxy + var requests atomic.Int32 + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if requests.Add(1) == 1 { + w.Header().Set(headerContentType, contentTypeJSON) + _, _ = w.Write(plain) // seed request: identity + return + } + // Second request has already read the identity row. Simulate a + // concurrent request finishing first: store the gzip blob and row, + // then fail this request so it takes the stale fallback. + proxy.cacheMetadataBlob(r.Context(), "gzip-test", "index", metadataStoragePath("gzip-test", "index"), + &upstreamMetadata{body: compressed, contentType: contentTypeJSON, contentEncoding: "gzip"}) + http.Error(w, "unavailable", http.StatusServiceUnavailable) + })) + defer upstream.Close() + + proxy, _, _, _ = setupTestProxy(t) + proxy.CacheMetadata = true + proxy.MetadataTTL = 0 + proxy.HTTPClient = upstream.Client() + + w := httptest.NewRecorder() + proxy.proxyCachedWithEncoding(w, httptest.NewRequest(http.MethodGet, "/index.json", nil), + upstream.URL+"/index.json", "gzip-test", "index", "identity", "*/*") + if w.Code != http.StatusOK { + t.Fatalf("seed status = %d, want 200", w.Code) + } + + raced := serveGzip(proxy, upstream.URL+"/index.json") + assertGzipResponse(t, "stale fallback after concurrent gzip store", raced, compressed) +} diff --git a/internal/handler/stale_cache_test.go b/internal/handler/stale_cache_test.go new file mode 100644 index 0000000..3d520a4 --- /dev/null +++ b/internal/handler/stale_cache_test.go @@ -0,0 +1,160 @@ +package handler + +import ( + "context" + "errors" + "io" + "strings" + "testing" +) + +// These tests cover an upstream re-publishing a version: the cache holds one +// artifact and upstream now declares another digest for it. + +const ( + stalePkgPURL = "pkg:npm/pkg" + staleVersionPURL = "pkg:npm/pkg@1.0.0" + staleFilename = "pkg-1.0.0.tgz" + staleStoragePath = "npm/pkg/1.0.0/pkg-1.0.0.tgz" + staleURL = "https://registry.npmjs.org/pkg/-/pkg-1.0.0.tgz" +) + +// seedCachedArtifact commits content the way a fetch does. +func seedCachedArtifact(t *testing.T, proxy *Proxy, store *mockStorage, content string) { + t.Helper() + ctx := context.Background() + if _, _, err := store.Store(ctx, staleStoragePath, strings.NewReader(content)); err != nil { + t.Fatalf("seeding storage: %v", err) + } + artifact := testArtifact(content, staleVersionPURL, staleFilename, "application/gzip") + if err := proxy.updateCacheDB("npm", "pkg", stalePkgPURL, staleURL, staleStoragePath, artifact); err != nil { + t.Fatalf("seeding cache record: %v", err) + } +} + +// cachedDigest reports the digest the cache record holds, or "" without one. +func cachedDigest(t *testing.T, proxy *Proxy) string { + t.Helper() + record, err := proxy.DB.GetCachedArtifact(stalePkgPURL, staleVersionPURL, staleFilename) + if err != nil { + t.Fatalf("reading cache record: %v", err) + } + if record == nil { + return "" + } + return record.Artifact.Digest.Encoded() +} + +func bytesPresent(store *mockStorage) bool { + r, err := store.Open(context.Background(), staleStoragePath) + if err != nil { + return false + } + _ = r.Close() + return true +} + +func TestStaleCacheCheckHasNoSideEffects(t *testing.T) { + proxy, _, store, _ := setupTestProxy(t) + seedCachedArtifact(t, proxy, store, "old bytes") + + res, err := proxy.getCachedArtifactWithUpstreamHash(context.Background(), + stalePkgPURL, staleVersionPURL, staleFilename, sha256Hex("new bytes")) + if err != nil { + t.Fatalf("cache check failed: %v", err) + } + if res != nil { + drain(res) + t.Fatal("stale entry was served") + } + if got := cachedDigest(t, proxy); got != sha256Hex("old bytes") { + t.Errorf("record digest = %q, want the stale one kept: the check must not discard", got) + } + if !bytesPresent(store) { + t.Error("stale bytes were deleted by the check") + } +} + +func TestStaleCacheIsDiscardedBeforeTheFetch(t *testing.T) { + proxy, _, store, fetcher := setupTestProxy(t) + seedCachedArtifact(t, proxy, store, "old bytes") + boom := errors.New("upstream unavailable") + fetcher.fetchErr = boom + + _, err := proxy.GetOrFetchArtifactFromURLWithDigest(context.Background(), + "npm", "pkg", "1.0.0", staleFilename, staleURL, "sha256:"+sha256Hex("new bytes")) + if !errors.Is(err, boom) { + t.Fatalf("got %v, want the fetch failure", err) + } + if got := cachedDigest(t, proxy); got != "" { + t.Errorf("stale record survived a failed refresh, digest = %q", got) + } + if bytesPresent(store) { + t.Error("stale bytes survived a failed refresh") + } +} + +func TestStaleCacheIsReplacedByTheFetch(t *testing.T) { + proxy, _, store, fetcher := setupTestProxy(t) + seedCachedArtifact(t, proxy, store, "old bytes") + fetcher.artifact = artifactBody("new bytes") + upstream := sha256Hex("new bytes") + + res, err := proxy.GetOrFetchArtifactFromURLWithDigest(context.Background(), + "npm", "pkg", "1.0.0", staleFilename, staleURL, "sha256:"+upstream) + if err != nil { + t.Fatalf("refresh failed: %v", err) + } + got, err := io.ReadAll(res.Reader) + _ = res.Reader.Close() + if err != nil || string(got) != "new bytes" { + t.Fatalf("got %q (err %v), want the refreshed bytes", got, err) + } + if !fetcher.fetchCalled { + t.Error("stale entry was served without a fetch") + } + if d := cachedDigest(t, proxy); d != upstream { + t.Errorf("record digest = %q, want %q", d, upstream) + } + + fetcher.fetchCalled = false + res, err = proxy.GetOrFetchArtifactFromURLWithDigest(context.Background(), + "npm", "pkg", "1.0.0", staleFilename, staleURL, "sha256:"+upstream) + if err != nil { + t.Fatalf("request after refresh failed: %v", err) + } + drain(res) + if fetcher.fetchCalled || !res.Cached { + t.Errorf("request after refresh: fetched=%v cached=%v, want served from cache", fetcher.fetchCalled, res.Cached) + } +} + +// TestLateLeaderKeepsRefreshedEntry is the race, at the point it would happen: +// a caller whose cache check saw a stale entry reaches the coalescing step +// after another caller's fetch replaced it. It must serve the replacement. +func TestLateLeaderKeepsRefreshedEntry(t *testing.T) { + proxy, _, store, fetcher := setupTestProxy(t) + seedCachedArtifact(t, proxy, store, "new bytes") + fetcher.fetchErr = errors.New("must not fetch") + upstream := sha256Hex("new bytes") + + res, err := proxy.coalescedFetchFromURL(context.Background(), + "npm", "pkg", "1.0.0", staleFilename, stalePkgPURL, staleVersionPURL, staleURL, nil, upstream) + if err != nil { + t.Fatalf("late leader failed: %v", err) + } + got, err := io.ReadAll(res.Reader) + _ = res.Reader.Close() + if err != nil || string(got) != "new bytes" { + t.Fatalf("got %q (err %v), want the refreshed bytes", got, err) + } + if fetcher.fetchCalled { + t.Error("refreshed entry was fetched again") + } + if d := cachedDigest(t, proxy); d != upstream { + t.Errorf("refreshed record was discarded, digest = %q", d) + } + if !bytesPresent(store) { + t.Error("refreshed bytes were deleted") + } +} diff --git a/internal/handler/swift_test.go b/internal/handler/swift_test.go index 88ed30e..c9df35a 100644 --- a/internal/handler/swift_test.go +++ b/internal/handler/swift_test.go @@ -332,7 +332,7 @@ func TestSwiftSourceArchiveCanonicalizesPackageIdentity(t *testing.T) { } } -func TestSwiftSourceArchiveHeadDiscardsCachedChecksumMismatch(t *testing.T) { +func TestSwiftSourceArchiveHeadLeavesStaleCacheForTheFetch(t *testing.T) { archive := []byte("cached archive") upstreamChecksum := sha256.Sum256([]byte("upstream archive")) @@ -379,11 +379,27 @@ func TestSwiftSourceArchiveHeadDiscardsCachedChecksumMismatch(t *testing.T) { if got := w.Header().Get("Content-Length"); got != "456" { t.Errorf("Content-Length = %q, want 456 from upstream probe", got) } - if len(store.files) != 0 { - t.Errorf("mismatched cached archive remained in storage: %v", store.files) + // HEAD leaves the stale entry alone; the next GET replaces it under the + // coalescing key. + if len(store.files) != 1 { + t.Errorf("HEAD must leave the stale archive in storage, got %v", store.files) } - if rec, _ := db.GetCachedArtifact(packagePURL, versionPURL, "example-1.2.3.zip"); rec != nil { - t.Error("mismatched cache record was not cleared") + if rec, _ := db.GetCachedArtifact(packagePURL, versionPURL, "example-1.2.3.zip"); rec == nil { + t.Error("HEAD must leave the stale cache record in place") + } + + fetcher.artifact = artifactBody("upstream archive") + w = httptest.NewRecorder() + handler.ServeHTTP(w, httptest.NewRequest(http.MethodGet, "/apple/example/1.2.3.zip", nil)) + if w.Code != http.StatusOK { + t.Fatalf("GET status = %d, want 200; body: %s", w.Code, w.Body.String()) + } + if w.Body.String() != "upstream archive" { + t.Errorf("GET body = %q, want the refreshed archive", w.Body.String()) + } + rec, _ := db.GetCachedArtifact(packagePURL, versionPURL, "example-1.2.3.zip") + if rec == nil || rec.Artifact.Digest.Encoded() != hex.EncodeToString(upstreamChecksum[:]) { + t.Errorf("cache record after GET = %+v, want the upstream checksum", rec) } } diff --git a/internal/server/dashboard.go b/internal/server/dashboard.go index ec5eb7f..a3ac9eb 100644 --- a/internal/server/dashboard.go +++ b/internal/server/dashboard.go @@ -207,6 +207,7 @@ func getRegistryConfigs(baseURL string) []RegistryConfig { if strings.HasPrefix(strings.ToLower(baseURL), "http://") { swiftInsecureFlag = "--allow-insecure-http " } + dockerHost := strings.TrimPrefix(strings.TrimPrefix(baseURL, "https://"), "http://") return []RegistryConfig{ { @@ -433,7 +434,7 @@ using Pkg; Pkg.update()`), sudo systemctl restart docker # Or pull directly -docker pull ` + baseURL[8:] + `/library/nginx:latest`), +docker pull ` + dockerHost + `/library/nginx:latest`), }, { ID: "deb", diff --git a/internal/server/server.go b/internal/server/server.go index 7613f52..e82b626 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -89,6 +89,13 @@ const ( serverIdleTimeout = 60 * time.Second dashboardTopN = 10 hoursPerDay = 24 + + // Upstream transport defaults, matching what fetch.NewFetcher would use + // if we did not hand it our own client. Go's default transport keeps only + // two idle connections per host and never times out waiting for response + // headers. + upstreamMaxIdleConnsPerHost = 10 + upstreamResponseHeaderTimeout = 60 * time.Second ) // Server is the main proxy server. @@ -205,7 +212,7 @@ func (s *Server) Start(listeners ...net.Listener) error { func (s *Server) serve(listener net.Listener) error { // Use one authentication-aware transport for metadata and artifacts so // configured credentials and cached OCI challenges apply consistently. - safeClient := safehttp.New(nil, upstreamSafeHTTPOptions(s.cfg.Upstream)) + safeClient := newUpstreamClient(s.cfg.Upstream) baseTransport := safeClient.Transport if s.accessLog != nil { baseTransport = upstreamhttp.NewAccessLogTransport(baseTransport, s.accessLog, s.logger) @@ -454,6 +461,19 @@ func configureScanning(proxy *handler.Proxy, cfg config.ScanningConfig, baseURL return scanGroup, nil } +// newUpstreamClient builds the shared upstream client: a safehttp client whose +// transport keeps upstreamMaxIdleConnsPerHost idle connections per host and +// gives up after upstreamResponseHeaderTimeout when an upstream accepts a +// request but stalls before sending headers. +func newUpstreamClient(upstream config.UpstreamConfig) *http.Client { + client := safehttp.New(nil, upstreamSafeHTTPOptions(upstream)) + if transport, ok := client.Transport.(*http.Transport); ok { + transport.MaxIdleConnsPerHost = upstreamMaxIdleConnsPerHost + transport.ResponseHeaderTimeout = upstreamResponseHeaderTimeout + } + return client +} + func upstreamSafeHTTPOptions(upstream config.UpstreamConfig) safehttp.Options { return safehttp.Options{ AllowLoopback: upstream.AllowLoopback, diff --git a/internal/server/server_test.go b/internal/server/server_test.go index a52a5b5..d6bd20f 100644 --- a/internal/server/server_test.go +++ b/internal/server/server_test.go @@ -237,7 +237,7 @@ func testStartUsesConfiguredLoopbackUpstreams(t *testing.T) { } }() - client := &http.Client{Timeout: 250 * time.Millisecond} + probeClient := &http.Client{Timeout: 250 * time.Millisecond} deadline := time.Now().Add(5 * time.Second) for { req, err := http.NewRequest(http.MethodGet, cfg.BaseURL+"/pypi/simple/ruff/", nil) @@ -245,7 +245,7 @@ func testStartUsesConfiguredLoopbackUpstreams(t *testing.T) { t.Fatalf("creating request: %v", err) } req.Header.Set("Accept", "application/vnd.pypi.simple.v1+json") - resp, requestErr := client.Do(req) + resp, requestErr := probeClient.Do(req) if requestErr == nil { body, readErr := io.ReadAll(resp.Body) _ = resp.Body.Close() @@ -266,6 +266,9 @@ func testStartUsesConfiguredLoopbackUpstreams(t *testing.T) { time.Sleep(10 * time.Millisecond) } + // This checks upstream routing, not latency. Allow time for fetching and + // cache I/O under -race on slower CI workers. + client := &http.Client{Timeout: 5 * time.Second} resp, err := client.Get(cfg.BaseURL + "/v2/library/demo/manifests/latest") if err != nil { t.Fatalf("OCI request failed: %v", err) diff --git a/internal/server/templates/pages/dashboard.html b/internal/server/templates/pages/dashboard.html index 97c3082..9b9a9e2 100644 --- a/internal/server/templates/pages/dashboard.html +++ b/internal/server/templates/pages/dashboard.html @@ -54,7 +54,7 @@ {{end}} -
+
@@ -74,9 +74,9 @@ {{if .VulnCount}}{{.VulnCount}} vulns{{end}}
-
- {{.Hits}} cache hits - {{.Size}} +
+ {{.Hits}} cache hits + {{.Size}}
{{end}} @@ -97,12 +97,12 @@
{{if .RecentPackages}} {{range .RecentPackages}} -
+
-
+
{{template "ecosystem_badge" .Ecosystem}} - {{.Name}} - @{{.Version}} + {{.Name}} + @{{.Version}}
{{if .License}}{{.License}}{{end}} @@ -110,9 +110,9 @@ {{if .VulnCount}}{{.VulnCount}} vulns{{end}}
-
- {{.CachedAt}} - {{.Size}} +
+ {{.CachedAt}} + {{.Size}}
{{end}} diff --git a/internal/server/templates_test.go b/internal/server/templates_test.go index 11e4b19..a2d3b7d 100644 --- a/internal/server/templates_test.go +++ b/internal/server/templates_test.go @@ -519,6 +519,21 @@ func TestEcosystemBadgeLabel(t *testing.T) { } } +func TestOCIRegistryInstructionsDockerPull(t *testing.T) { + registries := getRegistryConfigs("http://package-proxy:8080") + for _, registry := range registries { + if registry.ID != "oci" { + continue + } + want := "docker pull package-proxy:8080/library/nginx:latest" + if !strings.Contains(string(registry.Instructions), want) { + t.Errorf("OCI instructions = %q, want substring %q", registry.Instructions, want) + } + return + } + t.Fatal("OCI registry instructions not found") +} + func TestSwiftRegistryInstructionsAllowLocalHTTP(t *testing.T) { registries := getRegistryConfigs("http://localhost:8080") for _, registry := range registries { diff --git a/internal/server/transport_test.go b/internal/server/transport_test.go new file mode 100644 index 0000000..7464282 --- /dev/null +++ b/internal/server/transport_test.go @@ -0,0 +1,190 @@ +package server + +import ( + "crypto/tls" + "io" + "net" + "net/http" + "net/http/httptest" + "strings" + "sync" + "testing" + "time" + + "github.com/git-pkgs/proxy/internal/config" + "github.com/git-pkgs/registries/safehttp" +) + +// tlsUpstream starts a TLS test server that counts accepted connections. +func tlsUpstream(t *testing.T, handler http.HandlerFunc) (*httptest.Server, func() int) { + t.Helper() + var mu sync.Mutex + accepted := 0 + srv := httptest.NewUnstartedServer(handler) + srv.Config.ConnState = func(_ net.Conn, state http.ConnState) { + if state == http.StateNew { + mu.Lock() + accepted++ + mu.Unlock() + } + } + srv.StartTLS() + t.Cleanup(srv.Close) + return srv, func() int { + mu.Lock() + defer mu.Unlock() + return accepted + } +} + +// trustUpstream makes transport trust srv's certificate and pins HTTP/1.1 so +// every in-flight request needs its own connection. +func trustUpstream(t *testing.T, transport *http.Transport, srv *httptest.Server) { + t.Helper() + transport.TLSClientConfig = &tls.Config{ + RootCAs: srv.Client().Transport.(*http.Transport).TLSClientConfig.RootCAs, + NextProtos: []string{"http/1.1"}, + MinVersion: tls.VersionTLS12, + } + transport.ForceAttemptHTTP2 = false + t.Cleanup(transport.CloseIdleConnections) +} + +// burst issues n concurrent GETs and drains every body. The transport hands a +// connection back to the idle pool before the body's final Read returns, so +// the pool is settled when burst returns. +func burst(t *testing.T, client *http.Client, url string, n int) { + t.Helper() + var wg sync.WaitGroup + errs := make(chan error, n) + for range n { + wg.Add(1) + go func() { + defer wg.Done() + resp, err := client.Get(url) + if err != nil { + errs <- err + return + } + _, _ = io.Copy(io.Discard, resp.Body) + _ = resp.Body.Close() + }() + } + wg.Wait() + close(errs) + for err := range errs { + t.Errorf("burst request: %v", err) + } +} + +// TestUpstreamClientReusesConnectionsAcrossBursts measures how many +// connections a second burst of concurrent requests reuses. With Go's default +// of two idle connections per host most of them are re-dialled; with the +// tuned transport the second burst reuses all of them. +func TestUpstreamClientReusesConnectionsAcrossBursts(t *testing.T) { + const burstSize = 8 + + // holdBurst returns a handler that answers a request only once burstSize + // of them are waiting at the same time. With HTTP/1.1 pinned that puts + // every burst on burstSize distinct connections, whatever the scheduling. + holdBurst := func() http.HandlerFunc { + var mu sync.Mutex + waiting := 0 + release := make(chan struct{}) + return func(w http.ResponseWriter, r *http.Request) { + mu.Lock() + gate := release + waiting++ + if waiting == burstSize { + close(gate) + waiting = 0 + release = make(chan struct{}) + } + mu.Unlock() + select { + case <-gate: + case <-r.Context().Done(): + } + _, _ = w.Write([]byte("ok")) + } + } + + tests := []struct { + name string + client *http.Client + // Bounds on how many connections the second burst has to dial. + minNew, maxNew int + }{ + { + name: "go default keeps two idle connections", + client: safehttp.New(nil, safehttp.Options{AllowLoopback: true}), + minNew: burstSize - 2, + maxNew: burstSize, + }, + { + name: "tuned transport reuses the whole burst", + client: newUpstreamClient(config.UpstreamConfig{AllowLoopback: true}), + minNew: 0, + maxNew: 0, + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + srv, accepted := tlsUpstream(t, holdBurst()) + transport := tc.client.Transport.(*http.Transport) + trustUpstream(t, transport, srv) + + burst(t, tc.client, srv.URL, burstSize) + afterFirst := accepted() + if afterFirst < burstSize { + t.Fatalf("first burst opened %d connections, want at least %d", afterFirst, burstSize) + } + + burst(t, tc.client, srv.URL, burstSize) + newInSecond := accepted() - afterFirst + t.Logf("second burst: %d new connections, %d reused", newInSecond, burstSize-newInSecond) + + if newInSecond < tc.minNew || newInSecond > tc.maxNew { + t.Errorf("second burst opened %d new connections, want between %d and %d", newInSecond, tc.minNew, tc.maxNew) + } + }) + } +} + +// TestUpstreamClientBoundsStallBeforeHeaders pins the production transport +// values, then lowers the header timeout so it can show within milliseconds +// that this is what cuts off an upstream which accepts a request but never +// sends headers. +func TestUpstreamClientBoundsStallBeforeHeaders(t *testing.T) { + client := newUpstreamClient(config.UpstreamConfig{AllowLoopback: true}) + transport := client.Transport.(*http.Transport) + if transport.MaxIdleConnsPerHost != upstreamMaxIdleConnsPerHost { + t.Fatalf("MaxIdleConnsPerHost = %d, want %d", transport.MaxIdleConnsPerHost, upstreamMaxIdleConnsPerHost) + } + if transport.ResponseHeaderTimeout != upstreamResponseHeaderTimeout { + t.Fatalf("ResponseHeaderTimeout = %v, want %v", transport.ResponseHeaderTimeout, upstreamResponseHeaderTimeout) + } + + stall := make(chan struct{}) + srv, _ := tlsUpstream(t, func(_ http.ResponseWriter, r *http.Request) { + select { + case <-stall: + case <-r.Context().Done(): + } + }) + t.Cleanup(func() { close(stall) }) + trustUpstream(t, transport, srv) + + // Far below the client's overall timeout, so the header timeout ends the + // request; the error text tells the two timeouts apart. + transport.ResponseHeaderTimeout = 200 * time.Millisecond + + resp, err := client.Get(srv.URL) + if err == nil { + _ = resp.Body.Close() + t.Fatal("request to a stalled upstream succeeded, want a timeout") + } + if !strings.Contains(err.Error(), "timeout awaiting response headers") { + t.Fatalf("error = %v, want a response-header timeout", err) + } +} diff --git a/internal/storage/blob.go b/internal/storage/blob.go index 97e50f3..cdc7aa0 100644 --- a/internal/storage/blob.go +++ b/internal/storage/blob.go @@ -22,11 +22,19 @@ import ( const osWindows = "windows" +// attrsExt is fileblob's sidecar suffix, kept only to clear sidecars an +// earlier version wrote. +const attrsExt = ".attrs" + // Blob implements Storage using gocloud.dev/blob. // Supports local filesystem (file://) and S3 (s3://) URLs. type Blob struct { bucket *blob.Bucket url string + + // fileRoot is the directory backing a file:// bucket, empty for cloud + // backends. Used only to clear sidecars an earlier version wrote. + fileRoot string } // OpenBucket opens a blob bucket from a URL. @@ -47,6 +55,8 @@ func OpenBucket(ctx context.Context, urlStr string) (Storage, error) { return OpenGCS(ctx, urlStr) } + var fileRoot string + // Handle file:// URLs specially to create the directory if strings.HasPrefix(urlStr, "file://") { path := strings.TrimPrefix(urlStr, "file://") @@ -74,6 +84,8 @@ func OpenBucket(ctx context.Context, urlStr string) (Storage, error) { return nil, fmt.Errorf("resolving path: %w", err) } + fileRoot = absPath + // Convert back to URL format with forward slashes urlPath := filepath.ToSlash(absPath) if runtime.GOOS == osWindows { @@ -87,7 +99,14 @@ func OpenBucket(ctx context.Context, urlStr string) (Storage, error) { // This avoids "invalid cross-device link" errors from os.Rename when // the bucket directory and os.TempDir are on different filesystems // (e.g. Docker volume mounts). - urlStr += "?no_tmp_dir=true" + // + // Do not write fileblob's ".attrs" sidecar. It is rewritten with + // os.Create, truncating in place outside the atomic rename that + // protects the blob, so a read overlapping a write can decode a + // partial file; a missing one defaults cleanly, a truncated one does + // not. Nothing in the proxy needs it: Store sets no ContentType, and + // Size reads os.Stat via Attributes. + urlStr += "?no_tmp_dir=true&metadata=skip" } bucket, err := blob.OpenBucket(ctx, urlStr) @@ -95,10 +114,87 @@ func OpenBucket(ctx context.Context, urlStr string) (Storage, error) { return nil, fmt.Errorf("opening bucket: %w", err) } - return &Blob{bucket: bucket, url: urlStr}, nil + return &Blob{bucket: bucket, url: urlStr, fileRoot: fileRoot}, nil +} + +// legacySidecarPath gives the ".attrs" path an earlier version wrote for key, +// or "" when that path would not be a file inside fileRoot. +// +// The key is escaped the way fileblob escapes it on the way to disk, so the +// sidecar is looked for where fileblob wrote it. filepath.Localize then +// validates the escaped form: it rejects an empty, absolute or ".." path, and +// "." would name fileRoot itself. What it declines are keys the proxy never +// produces. +func (b *Blob) legacySidecarPath(key string) string { + if b.fileRoot == "" { + return "" + } + rel, err := filepath.Localize(escapeKey(key)) + if err != nil || rel == "." { + return "" + } + return filepath.Join(b.fileRoot, rel) + attrsExt +} + +// escapeKey mirrors fileblob's unexported escapeKey, which hex-escapes a rune +// as "__0x__". Slashes stay as "/" for filepath.Localize to convert. +func escapeKey(key string) string { + runes := []rune(key) + var out strings.Builder + for i, r := range runes { + if escapeRune(runes, i) { + fmt.Fprintf(&out, "__%#x__", r) + } else { + out.WriteRune(r) + } + } + return out.String() +} + +// escapeRune is fileblob's rule for which runes of a key to escape: control +// characters, a raw path separator, a slash that would form "../", "//" or +// end the key, and on Windows the characters its filesystem reserves. +func escapeRune(r []rune, i int) bool { + c := r[i] + switch { + case c < ' ': + return true + case os.PathSeparator != '/' && c == os.PathSeparator: + return true + case i > 1 && c == '/' && r[i-1] == '.' && r[i-2] == '.': + return true + case i > 0 && c == '/' && r[i-1] == '/': + return true + case c == '/' && i == len(r)-1: + return true + case os.PathSeparator == '\\' && strings.ContainsRune(`<>:"|?*`, c): + return true + } + return false +} + +// clearLegacySidecar removes the ".attrs" file an earlier version wrote for +// key. Nothing rewrites one now, so a sidecar left partial by an interrupted +// write would fail every read of that key for good. Removing is atomic where +// the rewrite was not, so a concurrent reader gets the whole old file or +// nothing. +// +// Failure is deliberately not fatal. Usually the key never had a sidecar and +// os.Remove reports not-exist. A real failure leaves exactly the state this +// change inherited, while failing the write would turn a cleanup miss into a +// failed request. Windows makes that concrete: Go opens files without +// FILE_SHARE_DELETE, so a reader holding the sidecar open blocks deletion, and +// that reader is the very workload this change protects. The next store of the +// key retries. +func (b *Blob) clearLegacySidecar(key string) { + if sidecar := b.legacySidecarPath(key); sidecar != "" { + _ = os.Remove(sidecar) + } } func (b *Blob) Store(ctx context.Context, path string, r io.Reader) (int64, string, error) { + b.clearLegacySidecar(path) + // Compute hash while writing h := sha256.New() tee := io.TeeReader(r, h) diff --git a/internal/storage/blob_test.go b/internal/storage/blob_test.go index 3e5bf65..3a97704 100644 --- a/internal/storage/blob_test.go +++ b/internal/storage/blob_test.go @@ -6,11 +6,17 @@ import ( "encoding/hex" "errors" "io" + "io/fs" + "os" "path/filepath" "runtime" "strings" + "sync" + "sync/atomic" "testing" "time" + + "gocloud.dev/blob" ) func TestOpenBucket(t *testing.T) { @@ -293,3 +299,317 @@ func fileURLFromPath(path string) string { } return "file://" + path } + +func TestOpenBucketWritesNoAttrsSidecar(t *testing.T) { + dir := t.TempDir() + ctx := context.Background() + + b, err := OpenBucket(ctx, fileURLFromPath(dir)) + if err != nil { + t.Fatalf("OpenBucket failed: %v", err) + } + defer func() { _ = b.Close() }() + + if _, _, err := b.Store(ctx, "pkg/thing-1.0.0.tgz", strings.NewReader("content")); err != nil { + t.Fatalf("Store failed: %v", err) + } + + sidecars, err := filepath.Glob(filepath.Join(dir, "*", "*.attrs")) + if err != nil { + t.Fatalf("Glob failed: %v", err) + } + if len(sidecars) != 0 { + t.Errorf("got sidecar files %v, want none: a truncated sidecar fails reads that overlap a write", sidecars) + } +} + +// A read overlapping a write to the same key must not fail. fileblob rewrote +// its ".attrs" sidecar in place, so a reader decoding it mid-write saw a +// partial file, which the proxy served as a 502 on an artifact it held. +func TestConcurrentReadsSurviveWritesToSameKey(t *testing.T) { + if runtime.GOOS == "windows" { + // Go opens files without FILE_SHARE_DELETE, so a writer cannot replace + // a file a reader holds open: its rename fails with access denied + // instead of contending. The other platforms exercise this race. + t.Skip("Windows refuses to replace a file readers hold open") + } + const ( + key = "pkg/thing-1.0.0.tgz" + readers = 4 + readsPerRead = 500 + ) + dir := t.TempDir() + ctx := context.Background() + + b, err := OpenBucket(ctx, fileURLFromPath(dir)) + if err != nil { + t.Fatalf("OpenBucket failed: %v", err) + } + defer func() { _ = b.Close() }() + + payload := strings.Repeat("x", 4096) + if _, _, err := b.Store(ctx, key, strings.NewReader(payload)); err != nil { + t.Fatalf("seeding Store failed: %v", err) + } + + // The writer reports how it ended: a Store failure would otherwise stop + // the writes silently and let zero read failures pass for a test that + // never contended anything. + done := make(chan struct{}) + var writers sync.WaitGroup + var writes int + var writeErr error + writers.Add(1) + go func() { + defer writers.Done() + for { + select { + case <-done: + return + default: + } + if _, _, err := b.Store(ctx, key, strings.NewReader(payload)); err != nil { + writeErr = err + return + } + writes++ + } + }() + + var failures atomic.Int64 + var reading sync.WaitGroup + for range readers { + reading.Add(1) + go func() { + defer reading.Done() + for range readsPerRead { + r, err := b.Open(ctx, key) + if err != nil { + failures.Add(1) + continue + } + if _, err := io.Copy(io.Discard, r); err != nil { + failures.Add(1) + } + _ = r.Close() + } + }() + } + reading.Wait() + close(done) + writers.Wait() + + if writeErr != nil { + t.Fatalf("writer stopped early: %v", writeErr) + } + if writes == 0 { + t.Fatal("no write completed, so the reads were never contended") + } + if got := failures.Load(); got != 0 { + t.Errorf("%d of %d reads failed while one writer rewrote the same key, want 0", got, readers*readsPerRead) + } +} + +// seedLegacySidecar stores key through a bucket that still writes sidecars, as +// an earlier version did, and returns the path fileblob actually used. It is +// discovered rather than assumed, so callers test the real mapping. +func seedLegacySidecar(t *testing.T, dir, key, payload string) string { + t.Helper() + ctx := context.Background() + + legacy, err := blob.OpenBucket(ctx, fileURLFromPath(dir)+"?no_tmp_dir=true") + if err != nil { + t.Fatalf("opening legacy bucket: %v", err) + } + if err := legacy.WriteAll(ctx, key, []byte(payload), nil); err != nil { + t.Fatalf("legacy WriteAll: %v", err) + } + if err := legacy.Close(); err != nil { + t.Fatalf("closing legacy bucket: %v", err) + } + + var found []string + walkErr := filepath.WalkDir(dir, func(path string, d fs.DirEntry, err error) error { + if err != nil { + return err + } + if !d.IsDir() && strings.HasSuffix(path, ".attrs") { + found = append(found, path) + } + return nil + }) + if walkErr != nil { + t.Fatalf("walking %s: %v", dir, walkErr) + } + if len(found) != 1 { + t.Fatalf("got sidecars %v, want exactly one", found) + } + return found[0] +} + +// An interrupted setAttrs leaves a partial sidecar that fails every read of the +// key, and nothing rewrites one now, so a store has to clear it. +// +// One key per storage path the proxy builds: ArtifactPath across ecosystems, +// metadata blobs, and the Gradle build cache. Scoped npm names, Go's "!" case +// escaping and the ":" in OCI digests and Debian epochs are the characters +// most likely to part fileblob's mapping from a plain path join. +func TestStoreClearsLegacyAttrsSidecar(t *testing.T) { + keys := []string{ + "npm/@babel/core/7.24.0/core-7.24.0.tgz", + "maven/org.apache.commons/commons-lang3/3.14.0/commons-lang3-3.14.0.jar", + "golang/github.com/!burnt!sushi/toml/v1.3.2/v1.3.2.zip", + "oci/library/nginx/sha256:abc123def456/manifest", + "debian/tzdata/1:2024a-1/tzdata_2024a-1_all.deb", + "pypi/requests/2.31.0/requests-2.31.0-py3-none-any.whl", + "cargo/serde/1.0.197/serde-1.0.197.crate", + "julia/Example/a1b2c3/a1b2c3.tar.gz", + "conda/numpy/1.26.4/numpy-1.26.4-py311.conda", + "_metadata/npm/@babel/core/metadata", + "_gradle/http-build-cache/0a1b2c3d4e5f", + // Keys fileblob escapes on every platform. + "npm/pkg//1.0.0/x.tgz", + "npm/pkg/../1.0.0/x.tgz", + } + + for _, key := range keys { + t.Run(key, func(t *testing.T) { + assertStoreClearsSidecar(t, key) + }) + } +} + +func assertStoreClearsSidecar(t *testing.T, key string) { + t.Helper() + const payload = "payload" + ctx := context.Background() + dir := t.TempDir() + + sidecar := seedLegacySidecar(t, dir, key, payload) + if err := os.WriteFile(sidecar, []byte(`{"user.content_type":"appl`), 0o600); err != nil { + t.Fatalf("corrupting sidecar: %v", err) + } + + b := openFileBlob(t, dir) + if _, err := b.Open(ctx, key); err == nil { + t.Fatal("corrupt sidecar did not fail the read, so it is not the file fileblob reads for this key") + } + + derived := b.legacySidecarPath(key) + if _, _, err := b.Store(ctx, key, strings.NewReader(payload)); err != nil { + t.Fatalf("Store failed: %v", err) + } + + if derived == "" { + t.Fatalf("legacySidecarPath declined %q, but fileblob wrote %q", key, sidecar) + } + if derived != sidecar { + t.Fatalf("derived %q, but fileblob wrote %q", derived, sidecar) + } + if _, err := os.Stat(sidecar); !os.IsNotExist(err) { + t.Errorf("sidecar still present after Store, stat err = %v", err) + } + assertReadsBack(t, b, key, payload) +} + +func assertReadsBack(t *testing.T, b *Blob, key, want string) { + t.Helper() + r, err := b.Open(context.Background(), key) + if err != nil { + t.Fatalf("read still failing after Store cleared the sidecar: %v", err) + } + defer func() { _ = r.Close() }() + got, err := io.ReadAll(r) + if err != nil { + t.Fatalf("ReadAll failed: %v", err) + } + if string(got) != want { + t.Errorf("got %q, want %q", got, want) + } +} + +func openFileBlob(t *testing.T, dir string) *Blob { + t.Helper() + s, err := OpenBucket(context.Background(), fileURLFromPath(dir)) + if err != nil { + t.Fatalf("OpenBucket failed: %v", err) + } + t.Cleanup(func() { _ = s.Close() }) + b, ok := s.(*Blob) + if !ok { + t.Fatalf("got %T, want *Blob", s) + } + return b +} + +// fileblob escapes a non-local key in a way this cannot reproduce, and one +// holding ".." resolves outside the cache directory. Removal declines both +// rather than delete the wrong file. +func TestLegacySidecarPathEscapesLikeFileblob(t *testing.T) { + root := filepath.FromSlash("/var/cache/proxy") + b := &Blob{fileRoot: root} + windows := runtime.GOOS == osWindows + + for _, tc := range []struct{ key, unix, windows string }{ + {"npm/pkg/1.0.0/x.tgz", "npm/pkg/1.0.0/x.tgz", "npm/pkg/1.0.0/x.tgz"}, + {"npm/pkg//1.0.0/x.tgz", "npm/pkg/__0x2f__1.0.0/x.tgz", "npm/pkg/__0x2f__1.0.0/x.tgz"}, + {"npm/pkg/../../etc/passwd", "npm/pkg/..__0x2f__..__0x2f__etc/passwd", "npm/pkg/..__0x2f__..__0x2f__etc/passwd"}, + {"npm/pkg/1.0.0/", "npm/pkg/1.0.0__0x2f__", "npm/pkg/1.0.0__0x2f__"}, + {"npm/a\x01b", "npm/a__0x1__b", "npm/a__0x1__b"}, + {"oci/nginx/sha256:abc/manifest", "oci/nginx/sha256:abc/manifest", "oci/nginx/sha256__0x3a__abc/manifest"}, + {"debian/tzdata/1:2024a-1/x.deb", "debian/tzdata/1:2024a-1/x.deb", "debian/tzdata/1__0x3a__2024a-1/x.deb"}, + {`npm/a\b`, `npm/a\b`, "npm/a__0x5c__b"}, + } { + want := tc.unix + if windows { + want = tc.windows + } + want = filepath.Join(root, filepath.FromSlash(want)) + attrsExt + if got := b.legacySidecarPath(tc.key); got != want { + t.Errorf("legacySidecarPath(%q) = %q, want %q", tc.key, got, want) + } + } +} + +func TestLegacySidecarPathDeclinesNonLocalKeys(t *testing.T) { + b := &Blob{fileRoot: filepath.FromSlash("/var/cache/proxy")} + + for _, key := range []string{"", ".", "..", "/etc/passwd"} { + if got := b.legacySidecarPath(key); got != "" { + t.Errorf("legacySidecarPath(%q) = %q, want \"\"", key, got) + } + } +} + +// Cloud backends have no local directory, so nothing is removed for them. +func TestLegacySidecarPathEmptyForCloudBackends(t *testing.T) { + b := &Blob{} + if got := b.legacySidecarPath("npm/pkg/1.0.0/x.tgz"); got != "" { + t.Errorf("legacySidecarPath = %q, want \"\" when there is no file root", got) + } +} + +// Cleanup that cannot complete must not fail the write. A non-empty directory +// at the sidecar path makes os.Remove fail with something other than not-exist +// on every platform, which is what a Windows sharing violation would look like +// here. +func TestStoreSucceedsWhenSidecarCannotBeRemoved(t *testing.T) { + const key = "npm/pkg/1.0.0/pkg-1.0.0.tgz" + const payload = "payload" + dir := t.TempDir() + ctx := context.Background() + + b := openFileBlob(t, dir) + + sidecar := filepath.Join(dir, filepath.FromSlash(key)) + ".attrs" + if err := os.MkdirAll(filepath.Join(sidecar, "blocker"), 0o750); err != nil { + t.Fatalf("seeding an unremovable sidecar: %v", err) + } + if err := os.Remove(sidecar); err == nil { + t.Fatal("sidecar path was removable, so the test proves nothing") + } + + if _, _, err := b.Store(ctx, key, strings.NewReader(payload)); err != nil { + t.Errorf("Store failed because cleanup could not complete: %v", err) + } +}