unkin/artifactapi
3
0
Fork 0
Code Issues 5 Pull Requests 10 Actions Packages Projects Releases 1 Wiki Activity

Forward allowlisted query params for remote providers #146

Open
unkin-agent wants to merge 2 commits from benvin/remote-query-forwarder into master
pull from: benvin/remote-query-forwarder
merge into: unkin:master
Conversation 1 Commits 2 Files Changed 7
+151 -3
unkin-agent commented 2026-10-17 12:31:50 +11:00
Member
Copy Link
Copy Source

Remote proxying drops the query string, so paginated JSON APIs (e.g. Galaxy versions/?limit=&offset=) cannot be served. Forwarding the raw query would let arbitrary params mint unbounded cache entries and bypass path-anchored blocklists.

  • add provider.QueryForwarder, mapping each forwarded param for a path to a validator that canonicalises or rejects its value
  • forward only validated params upstream and into the cache key, single-valued and in sorted order
  • classify on the bare path so blocklist/allowlist/mutability patterns ignore the query
  • evict a path together with its path?... query variants
  • leave providers without QueryForwarder unchanged
Remote proxying drops the query string, so paginated JSON APIs (e.g. Galaxy `versions/?limit=&offset=`) cannot be served. Forwarding the raw query would let arbitrary params mint unbounded cache entries and bypass path-anchored blocklists. - add `provider.QueryForwarder`, mapping each forwarded param for a path to a validator that canonicalises or rejects its value - forward only validated params upstream and into the cache key, single-valued and in sorted order - classify on the bare path so blocklist/allowlist/mutability patterns ignore the query - evict a path together with its `path?...` query variants - leave providers without `QueryForwarder` unchanged
unkin-agent added 1 commit 2026-10-17 12:31:50 +11:00
Forward allowlisted query params for opt-in remote providers
ci/woodpecker/pr/pre-commit Pipeline was successful
Details
ci/woodpecker/pr/test Pipeline was successful
Details
ci/woodpecker/pr/build Pipeline was successful
Details
586b07bab6
unkin-agent commented 2026-10-17 12:48:50 +11:00
Author
Member
Copy Link
Copy Source
  • internal/api/v1/proxy.go:97 — the query is appended to path before engine.Fetch, so Classifier.Classify runs Blocklist/Patterns/ImmutablePatterns/MutablePatterns regexes against path?limit=10. $-anchored patterns stop matching: a blocklist like \.exe$ is bypassed with ?limit=1, and a Patterns allowlist denies paginated requests → classify and apply patterns on the bare path, key cache/upstream on the canonical path+query.
  • internal/api/v1/proxy.go:276 — values are only length-limited (<=32), so limit=10, 010, +10, 1e1 and every distinct offset=N become separate cache keys, each causing an upstream fetch, an S3 object and a DB row, with no auth on remote reads → let the forwarder own value validation (e.g. ForwardedQuery returns name→validator, digits-only for limit/offset) and drop non-conforming values.
  • nit: internal/proxy/engine.go:220 — non-wildcard Evict("a/versions/") no longer drops a/versions/?limit=10 variants → document that a/versions/* is needed, or evict the path?* variants too.
- internal/api/v1/proxy.go:97 — the query is appended to `path` before `engine.Fetch`, so `Classifier.Classify` runs `Blocklist`/`Patterns`/`ImmutablePatterns`/`MutablePatterns` regexes against `path?limit=10`. `$`-anchored patterns stop matching: a blocklist like `\.exe$` is bypassed with `?limit=1`, and a `Patterns` allowlist denies paginated requests → classify and apply patterns on the bare path, key cache/upstream on the canonical path+query. - internal/api/v1/proxy.go:276 — values are only length-limited (<=32), so `limit=10`, `010`, `+10`, `1e1` and every distinct `offset=N` become separate cache keys, each causing an upstream fetch, an S3 object and a DB row, with no auth on remote reads → let the forwarder own value validation (e.g. `ForwardedQuery` returns name→validator, digits-only for limit/offset) and drop non-conforming values. - nit: internal/proxy/engine.go:220 — non-wildcard `Evict("a/versions/")` no longer drops `a/versions/?limit=10` variants → document that `a/versions/*` is needed, or evict the `path?*` variants too.
unkin-agent added 1 commit 2026-10-17 12:53:05 +11:00
Classify bare paths, validate forwarded query values, evict query variants
ci/woodpecker/pr/pre-commit Pipeline was successful
Details
ci/woodpecker/pr/build Pipeline was successful
Details
ci/woodpecker/pr/test Pipeline was successful
Details
769d673a4a
All checks were successful
ci/woodpecker/pr/pre-commit Pipeline was successful
Required
Details
ci/woodpecker/pr/build Pipeline was successful
Required
Details
ci/woodpecker/pr/test Pipeline was successful
Required
Details
This pull request has changes conflicting with the target branch.
  • internal/provider/provider.go
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin benvin/remote-query-forwarder:benvin/remote-query-forwarder
git checkout benvin/remote-query-forwarder
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
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: unkin/artifactapi#146
Delete Branch "benvin/remote-query-forwarder"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?