Conversation
Gemfury registries (pypi.fury.io, npm.fury.io, ...) 302-redirect package downloads to pre-signed URLs on gemfury.s3-accelerate.dualstack.amazonaws.com. The redirect target was blocked, so updates failed after the authenticated registry request had succeeded. Add the bucket as an exact host under shared_registry_domains, like the JFrog-owned S3 buckets, since Gemfury serves several ecosystems.
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The global entry bypasses tenant isolation for a shared path-based Gemfury endpoint.
Review effort: Balanced
Findings: 1
Open (1)
What changed in this PR
Adds Gemfury’s S3 download bucket to the proxy egress allowlist.
Changes:
- Adds the exact Gemfury bucket hostname.
- Adds positive and hostname-boundary regression tests.
| File | Description |
|---|---|
internal/handlers/egress_allowlist_defaults.yaml |
Adds the Gemfury storage host. |
internal/handlers/egress_allowlist_test.go |
Tests access and exact-host matching. |
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| - jfrog-prod-use1-shared-virginia-main.s3.amazonaws.com | ||
| - jfrog-prod-usw2-shared-oregon-main.s3.amazonaws.com | ||
| - jfrog-prod-use1-dedicated-virginia-main.s3.amazonaws.com | ||
| - gemfury.s3-accelerate.dualstack.amazonaws.com |
5 tasks done
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

What are you trying to accomplish?
Since the egress allowlist started enforcing, every Dependabot update that downloads a package from a Gemfury registry fails. The authenticated registry request succeeds, but the download is blocked:
Every Gemfury registry endpoint 302-redirects package downloads to a short-lived pre-signed URL on one Gemfury-owned S3 bucket,
gemfury.s3-accelerate.dualstack.amazonaws.com. I checked this directly against bothpypi.fury.io(wheel) andnpm.fury.io(tarball): both redirect to that exact host. Older files are sometimes served inline with a 200, which is why the failure shows up per package rather than per registry.This PR adds the bucket as an exact host under
shared_registry_domains, next to the JFrog-owned S3 buckets. It goes there because Gemfury serves several ecosystems from the same backend.Anything you want to highlight for special attention from reviewers?
Matching form: the entry is an exact host, as the file header requires for shared storage domains:
gemfuryis already registered. S3 bucket names are globally unique, so no label in the entry is attacker-choosable.s3-accelerateparent all stay blocked. See the tests below.Multi-tenancy: the bucket holds all Gemfury accounts' files, with the account in the path. That's the same situation as the JFrog shared regional buckets. Reaching the host on its own gives access to nothing:
Ownership evidence:
pypi.fury.ioandnpm.fury.iothemselves issue the redirect to this host.CN=*.s3-accelerate.amazonaws.com, issuerAmazon RSA 2048 M01).Compared with #287: #287 adds the bucket per job instead, only for jobs with a
*.fury.iocredential, viaregistryRedirectHostsnext to the ECR starport bucket. That follows the rule inadd-egress-allowlist-domainfor multi-tenant hosts more strictly, at the cost of a little code. This PR is the smaller change and follows the JFrog precedent.How will you know you've accomplished your goal?
Tests in
internal/handlers/egress_allowlist_test.go:TestEgressAllowlist_GemfuryS3BucketAllowedButSharedS3Blocked:pipandnpm_and_yarnjobs.gemfuryx.), another tenant on the shared parent (attacker.s3-accelerate.dualstack.amazonaws.com), and path-style access to the bare accelerate endpoint.TestEgressAllowlist_NewEntriesDoNotWidenBeyondExactHostshas a new child probe,evil.gemfury.s3-accelerate.dualstack.amazonaws.com. I checked that it catches a regression: changing the entry to its leading-dot form makes the test fail.TestEgressDefaults_NoRedundantEntriespasses, and so doscript/test(-race -shuffle=on -count=2),go vetandgofmt -l.Checklist