Harden nuget per-request index rewrite #149

Merged

benvin merged 10 commits from benvin/nuget-followups into master 2026-10-17 21:39:42 +11:00

Conversation 2 Commits 10 Files Changed 10

+279 -37

unkin-agent commented 2026-10-17 12:43:15 +11:00

Member

Copy Link

Copy Source

NuGet index rewrites embed the client Host and X-Forwarded-Proto into per-request JSON, so a shared cache could store one host's URLs, a forged header could inject content, HEAD skipped the rewrite, and immutable flat-container blobs were re-validated as mutable.

• set no-store, Vary: Host, X-Forwarded-Proto and the rewritten Content-Length on GET and HEAD
• share v1.Scheme() (first X-Forwarded-Proto value, http/https only) across nuget, cargo, virtual redirects and the terraform registry
• reject non host[:port] Host headers and JSON-escape the injected base URL
• classify .snupkg, readme and icon flat-container blobs as immutable

requires unkin/artifactapi#148 merged first

NuGet index rewrites embed the client Host and X-Forwarded-Proto into per-request JSON, so a shared cache could store one host's URLs, a forged header could inject content, HEAD skipped the rewrite, and immutable flat-container blobs were re-validated as mutable. - set `no-store`, `Vary: Host, X-Forwarded-Proto` and the rewritten Content-Length on GET and HEAD - share `v1.Scheme()` (first X-Forwarded-Proto value, `http`/`https` only) across nuget, cargo, virtual redirects and the terraform registry - reject non host[:port] Host headers and JSON-escape the injected base URL - classify `.snupkg`, `readme` and `icon` flat-container blobs as immutable requires unkin/artifactapi#148 merged first

unkin-agent added 2 commits 2026-10-17 12:43:16 +11:00

Harden nuget per-request rewrite and fix HEAD length ... 3b69b58ba3

- set Cache-Control no-store and Vary on rewritten responses
- accept only http/https X-Forwarded-Proto
- reject invalid Host headers and JSON-escape the injected base URL
- route remote HEAD and serve rewritten HEAD via the GET path
- classify snupkg, readme and icon as immutable

Leave remote HEAD routing to the generic HEAD fix a7aa37982a

unkin-agent commented 2026-10-17 12:47:05 +11:00

Author

Member

Copy Link

Copy Source

• internal/api/v1/proxy.go:131 — HEAD is not covered: no HEAD change in this diff and master has no /remote HEAD route; once benvin/remote-head-route lands, HEAD on a rewritten mutable path reports the raw cached Content-Length, not the rewritten size GET now sends, and skips the host check/no-store/Vary → either land after remote-head-route and route HEAD through rewriteServed/the same size logic, or drop "HEAD" from the scope. The two branches both rewrite handleProxy (no textual conflict today, semantic gap).
• internal/api/v1/proxy.go:258 — shared scheme() now maps a multi-hop X-Forwarded-Proto: https, http (or HTTPS) to http, silently downgrading cargo, github_rpm and virtual URLs behind chained proxies; PR body does not mention these callers → split the scheme() change into its own PR with tests for those handlers, or take the first token and lowercase it (strings.EqualFold).
• internal/api/terraform/registry.go:297 — sibling X-Forwarded-Proto handling still trusts any value → reuse the validated helper or note it as follow-up.
• internal/api/v1/proxy.go:285 — validHost accepts host: (empty port) because SplitHostPort returns port "", producing http://host:/api/... in the index → reject empty port after a colon.
• nit: internal/api/v1/proxy.go:285 — IPv6 with zone ([fe80::1%eth0]) is rejected with 400; unlikely from clients but ParseIP-based check cannot accept it.
• Scope/atomicity — PR mixes three concerns; suggested boundaries:
  • PR A: nuget rewrite hardening (rewriteServed, no-store/Vary, validHost, JSON escaping, tests)
  • PR B: shared scheme() strictness (affects cargo/github_rpm/virtual)
  • PR C: nuget classification of .snupkg/readme/icon (nuget.go Classify/ContentType/isPackage)

- internal/api/v1/proxy.go:131 — HEAD is not covered: no HEAD change in this diff and master has no `/remote` HEAD route; once `benvin/remote-head-route` lands, HEAD on a rewritten mutable path reports the raw cached Content-Length, not the rewritten size GET now sends, and skips the host check/no-store/Vary → either land after remote-head-route and route HEAD through `rewriteServed`/the same size logic, or drop "HEAD" from the scope. The two branches both rewrite handleProxy (no textual conflict today, semantic gap). - internal/api/v1/proxy.go:258 — shared `scheme()` now maps a multi-hop `X-Forwarded-Proto: https, http` (or `HTTPS`) to `http`, silently downgrading cargo, github_rpm and virtual URLs behind chained proxies; PR body does not mention these callers → split the `scheme()` change into its own PR with tests for those handlers, or take the first token and lowercase it (`strings.EqualFold`). - internal/api/terraform/registry.go:297 — sibling `X-Forwarded-Proto` handling still trusts any value → reuse the validated helper or note it as follow-up. - internal/api/v1/proxy.go:285 — `validHost` accepts `host:` (empty port) because `SplitHostPort` returns port "", producing `http://host:/api/...` in the index → reject empty port after a colon. - nit: internal/api/v1/proxy.go:285 — IPv6 with zone (`[fe80::1%eth0]`) is rejected with 400; unlikely from clients but ParseIP-based check cannot accept it. - Scope/atomicity — PR mixes three concerns; suggested boundaries: - PR A: nuget rewrite hardening (`rewriteServed`, no-store/Vary, validHost, JSON escaping, tests) - PR B: shared `scheme()` strictness (affects cargo/github_rpm/virtual) - PR C: nuget classification of `.snupkg`/readme/icon (`nuget.go` Classify/ContentType/isPackage)

unkin-agent added 3 commits 2026-10-17 12:56:33 +11:00

Add HEAD route for remotes matching GET headers 3dd4acd233

Merge remote-tracking branch 'origin/benvin/remote-head-route' into benvin/nuget-followups d0c30e145e

Share strict X-Forwarded-Proto parsing and reject empty Host ports fe10a50d27

unkin-agent added 2 commits 2026-10-17 13:01:43 +11:00

Fetch rewritten mutable responses on HEAD to match GET length 7769902530

Merge remote-tracking branch 'origin/benvin/remote-head-route' into benvin/nuget-followups c3862fbd5f

unkin-agent added 2 commits 2026-10-17 17:12:49 +11:00

Merge remote-tracking branch 'origin/master' into benvin/nuget-followups ... f4e0a6a7de

# Conflicts:
#	internal/provider/provider.go

Document new nuget immutable blob classes 29f335c364

unkin-agent commented 2026-10-17 17:27:09 +11:00

Author

Member

Copy Link

Copy Source

No findings.

No findings.

unkin-agent added 1 commit 2026-10-17 21:25:29 +11:00

Merge remote-tracking branch 'origin/master' into benvin/nuget-followups ... d92f7cca5a

# Conflicts:
#	e2e/head_test.go
#	internal/api/v1/proxy.go

benvin merged commit bf8228e9f2 into master 2026-10-17 21:39:42 +11:00

benvin deleted branch benvin/nuget-followups 2026-10-17 21:39:42 +11:00

Sign in to join this conversation.

Reviewers

No Reviewers

Labels

No items

No Label

Milestone

No items

No Milestone

Projects

Clear projects

No project

Assignees

Clear assignees

benvin (BenVincent) unkin-agent (Unkin Agent) unkinben (Ben Vincent)

No Assignees

1 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: unkin/artifactapi#149