Skip to content

Allow Gemfury's path-style S3 downloads for Gemfury jobs - #312

Open
v-robaiken wants to merge 1 commit into
mainfrom
robaiken/gemfury-path-style-s3
Open

v-robaiken wants to merge 1 commit into
mainfrom
robaiken/gemfury-path-style-s3

Conversation

@v-robaiken

Copy link
Copy Markdown
Contributor

Fixes #308

Problem

#288 lets jobs with a *.fury.io credential reach Gemfury's bucket through its Transfer Acceleration hosts. Some Gemfury accounts are redirected to the same bucket as a path-style URL on the shared S3 endpoint instead:

302 https://pypi.fury.io/***/<account>/-/ver_xxx/<pkg>.whl
GET https://s3.amazonaws.com/gemfury/gems/<id>/<file>?X-Amz-...
* egress not allowlisted s3.amazonaws.com

Allowing s3.amazonaws.com as a host is not an option. With path-style addressing, every bucket in S3 lives on that host, so the job could send data to any bucket.

Fix

Add an optional derived_paths list to registry_redirect_derivations. Each entry is an exact host plus a literal path prefix. It is allowed only when:

  • the job holds a credential for the matching registry, as with derived;
  • the host matches exactly, with no subdomains, regional endpoints or globs;
  • the method is GET or HEAD, since these are pre-signed downloads;
  • both the escaped and decoded paths start with the prefix, so %2F and %2e tricks can't move the request into another bucket;
  • the path has no . or .. segments and no backslashes.

At startup, prefixes must be a literal, normalized /segment/ ending in a slash, so /gemfury can't also match /gemfury-attacker.

Gemfury now uses this rule:

  - credential_host: .fury.io
    derived: [...]
    derived_paths:
      - host: s3.amazonaws.com
        path_prefix: /gemfury/

The existing host-only matching is unchanged; this adds one more way for a request to be allowed. I've updated the allowlist skill doc with when to use derived_paths.

Tests

  • Allowed: Gemfury path-style downloads with a port, a trailing dot, an encoded +, and HEAD.
  • Blocked:
    • the bare host;
    • other buckets and look-alike buckets (gemfuryx, gemfury-attacker, GEMFURY);
    • dot segments, including encoded %2e%2e;
    • encoded slashes (%2F) and backslashes (%5C);
    • subdomains, regional endpoints and suffix look-alikes of the host;
    • PUT, POST and DELETE;
    • jobs without a Gemfury credential.
  • Startup validation rejects bad hosts and prefixes.
  • I checked that the tests catch problems by removing the escaped-path check and the dot-segment check in turn; each removal makes the tests fail.

Gemfury redirects some downloads to its bucket as a path-style URL on
the shared S3 endpoint (s3.amazonaws.com/gemfury/...), which #288's
accelerate hosts don't cover. Allowing s3.amazonaws.com as a host would
expose every bucket, so add path-scoped redirect derivations: an exact
host plus a literal path prefix, allowed for GET/HEAD only, and only
for jobs holding a matching registry credential.

Fixes #308

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 817c1703-3814-47a6-a9ea-41092ac242af
@v-robaiken
v-robaiken requested a review from a team as a code owner October 12, 2026 10:59
Copilot AI balanced review requested due to automatic review settings October 12, 2026 10:59
@v-robaiken v-robaiken added the allowlist Changes the default egress allowlist label Oct 12, 2026

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Startup validation currently accepts host and path-prefix configurations that can never match requests.

2 open findings
What changed in this PR

Adds credential-derived, path-scoped egress rules for secure Gemfury downloads through shared S3 endpoints.

Changes:

  • Adds exact host/path-prefix matching for GET and HEAD requests.
  • Configures /gemfury/ access on s3.amazonaws.com.
  • Adds security, validation, and regression tests.
File Description
internal/​handlers/​egress_dynamic_hosts.go Implements credential-derived path rules.
internal/​handlers/​egress_dynamic_hosts_test.go Tests matching and security boundaries.
internal/​handlers/​egress_allowlist.go Integrates path-scoped request authorization.
internal/​handlers/​egress_allowlist_defaults.yaml Adds the Gemfury S3 path rule.
internal/​handlers/​egress_allowlist_defaults.go Validates path derivation configuration.
.github/​skills/​add-egress-allowlist-domain/​SKILL.md Documents path-scoped derivations.

🧠 Review effort: Balanced


💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +111 to +112
if p.Host == "" || isGlobPattern(p.Host) || strings.HasPrefix(p.Host, ".") {
return fmt.Errorf("derived_paths host %q (credential_host %q) must be an exact host", p.Host, d.CredentialHost)
Comment on lines +129 to +130
if strings.ContainsAny(prefix, "%\\?#*[") || strings.Contains(prefix, "//") {
return fmt.Errorf("must be a literal, normalized path")
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

allowlist Changes the default egress allowlist

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Gemfury downloads still blocked after #288 (redirect goes to s3.amazonaws.com/gemfury)

3 participants