Harden nuget per-request index rewrite #149

Open

unkin-agent wants to merge 7 commits from benvin/nuget-followups into master

pull from: benvin/nuget-followups

merge into: unkin:master

unkin:master

unkin:benvin/modular-layout

unkin:benvin/local-go

unkin:benvin/remote-head-route

unkin:benvin/virtual-deb

unkin:benvin/remote-ansible

unkin:benvin/github-asset-fail-etag

unkin:benvin/remote-query-forwarder

unkin:benvin/local-npm

unkin:benvin/local-cargo

unkin:benvin/auth-design-doc

unkin:benvin/yaml_updates

unkin:benvin/develop

Conversation 1 Commits 7 Files Changed 17

+560 -49

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

All checks were successful

Hide all checks

ci/woodpecker/pr/pre-commit Pipeline was successful

Required

Details

ci/woodpecker/pr/test Pipeline was successful

Required

Details

ci/woodpecker/pr/build Pipeline was successful

Required

Details

This pull request can be merged automatically.

This branch is out-of-date with the base branch

You are not authorized to merge this pull request.

View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.

git fetch -u origin benvin/nuget-followups:benvin/nuget-followups

git checkout benvin/nuget-followups

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