feat: add CDN redirect resolution to cgw strategy by sampras343 · Pull Request #91 · securesign/sigstore-e2e

sampras343 · 2026-07-24T15:27:54Z

Summary

When the cgw strategy downloads a CLI binary from the Developer Portal file/ URL, the portal returns a 302 redirect to an HTML interstitial page — not the binary. This PR adds automatic CDN redirect resolution: if the direct download fails, ResolveCDNLink extracts the actual CDN download URL from the redirect's tcDownloadURL query parameter and retries.

How it works

1. Try direct download from CGW_URL/<binary>.tar.gz
   → Fails (HTML, not tar.gz)
2. ResolveCDNLink: no-follow request → extract tcDownloadURL from 302 Location
3. Download from CDN URL (access.cdn.redhat.com/...)
   → Success (actual binary)

Why

With the cli-server being removed from the operator (securesign/secure-sign-operator#2143), the GitHub Actions Kind CI can no longer use CLI_STRATEGY=cli_server. The cgw strategy with CDN resolution enables downloading CLI binaries from the publicly accessible Developer Portal.

Verified

CGW_URL=https://developers.redhat.com/content-gateway/file/RHTAS/1.4.2
cosign   → PASS
gitsign  → PASS
rekor-cli → PASS

All go test ./pkg/... pass.

When a direct download from the content gateway returns HTML (the Developer Portal interstitial page) instead of a tar.gz, resolve the actual CDN download URL via ResolveCDNLink and retry. This enables CGW_URL to point to the Developer Portal file URL (e.g., developers.redhat.com/content-gateway/file/RHTAS/<version>) which is publicly accessible from any environment including Kind clusters in CI. Signed-off-by: Sachin Sampras M <sampras343@gmail.com>

qodo-for-securesign · 2026-07-24T15:28:55Z

PR Summary by Qodo

Add CDN redirect resolution fallback to CGW CLI downloads

✨ Enhancement 🐞 Bug fix 🕐 10-20 Minutes

AI Description

• Retry CGW CLI downloads via resolved CDN URL when direct downloads return HTML.
• Add temp-dir cleanup and logging around fallback download attempts.

Diagram

graph TD
  A["cgw strategy"] --> B["Build CGW URL"] --> C["Download+untar (direct)"] --> D{"Direct OK?"}
  D -->|yes| E["FindBinary"]
  D -->|no| F["ResolveCDNLink"] --> G["Download+untar (CDN)"] --> E

High-Level Assessment

The following are alternative approaches to this PR:

1. Proactively resolve CDN URL first

➕ Avoids an extra failed download attempt (faster, less network churn).
➕ Makes behavior consistent regardless of whether the portal serves the tarball directly.
➖ Adds an extra request even when direct links would have worked.
➖ Requires deciding when to resolve (always vs heuristics), potentially reducing simplicity.

2. Move fallback into DownloadAndUntarArchive

➕ Centralizes redirect/HTML detection and retry logic for all strategies.
➕ Reduces duplicated retry patterns across call sites.
➖ Broadens behavioral change scope; higher regression risk for other consumers.
➖ May complicate a utility that currently has a single responsibility.

Recommendation: Current approach (fallback only after a failed direct download) is a good balance of minimal behavior change and compatibility with Developer Portal redirects. If additional strategies hit the same redirect behavior, consider centralizing the resolution logic inside the shared download helper to keep call sites simpler.

Files changed (1) +15 / -1

Enhancement (1) +15 / -1

cgw.goAdd CDN-resolution retry when direct CGW archive download fails +15/-1

Add CDN-resolution retry when direct CGW archive download fails

• On direct download/extract failure, the strategy now cleans up the temp directory, resolves the CDN URL via support.ResolveCDNLink, and retries the download into a fresh temp directory. It also adds log messages for the fallback attempt and the resolved CDN link.

pkg/strategy/cgw/cgw.go

qodo-for-securesign · 2026-07-24T15:31:48Z

Code Review by Qodo

🐞 Bugs (3) 📘 Rule violations (0) 📜 Skill insights (0)

1. Lost direct error 🐞 Bug ◔ Observability

Description

In cgw.download(), if the direct download fails and CDN resolution fails (or the CDN download
fails), the returned error drops the original direct-download error, making failures much harder to
debug. This is caused by returning only cdnErr (or the second download err) without preserving the
initial err.

Code

pkg/strategy/cgw/cgw.go[R40-55]

	if err = support.DownloadAndUntarArchive(ctx, link, tmp); err != nil {
-		return "", err
+		_ = os.RemoveAll(tmp)
+		logrus.Infof("Direct download failed, resolving CDN link for: %s", link)
+		cdnLink, cdnErr := support.ResolveCDNLink(ctx, link)
+		if cdnErr != nil {
+			return "", fmt.Errorf("download failed and CDN resolution failed: %w", cdnErr)
+		}
+		logrus.Infof("Resolved CDN link: %s", cdnLink)
+		tmp, err = os.MkdirTemp("", cliName)
+		if err != nil {
+			return "", err
+		}
+		if err = support.DownloadAndUntarArchive(ctx, cdnLink, tmp); err != nil {
+			_ = os.RemoveAll(tmp)
+			return "", err
+		}

Relevance

⭐⭐⭐ High
Team recently accepted improving error propagation in download paths; wrapping original direct error
aids debugging.
PR-#76

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The new fallback logic removes the tmp dir and attempts ResolveCDNLink, but on failure returns
only cdnErr and on second-download failure returns only that error; the original direct-download
error is not included in the returned error value.

pkg/strategy/cgw/cgw.go[40-55]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`pkg/strategy/cgw/cgw.go` falls back to CDN resolution on direct download failure, but the final error returned does not include the original (direct) failure in either the “CDN resolution failed” branch or the “CDN download failed” branch.

### Issue Context
- The initial `DownloadAndUntarArchive(ctx, link, tmp)` error is overwritten/ignored when attempting CDN fallback.
- This removes key context (e.g., original HTTP status / tar parsing error) needed to diagnose why the fallback was triggered.

### Fix Focus Areas
- pkg/strategy/cgw/cgw.go[40-55]

### Suggested change
- Capture the initial error (e.g., `directErr := err`) before CDN resolution.
- When CDN resolution fails, return an error that includes **both** `directErr` and `cdnErr` (use `errors.Join(directErr, cdnErr)` if available in your Go version, or include one as `%w` and the other as `%v`).
- When CDN download fails, wrap/join `directErr` with the CDN download error so the caller can see both attempts.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

2. Unvalidated CDN target 🐞 Bug ⛨ Security

Description

The new cgw fallback downloads and extracts from whatever URL ResolveCDNLink returns, but
ResolveCDNLink returns the tcDownloadURL query value verbatim without validating scheme/host.
This means a misconfigured or compromised redirect source could cause fetching and extracting from
an unexpected/unapproved host.

Code

pkg/strategy/cgw/cgw.go[R43-53]

+		cdnLink, cdnErr := support.ResolveCDNLink(ctx, link)
+		if cdnErr != nil {
+			return "", fmt.Errorf("download failed and CDN resolution failed: %w", cdnErr)
+		}
+		logrus.Infof("Resolved CDN link: %s", cdnLink)
+		tmp, err = os.MkdirTemp("", cliName)
+		if err != nil {
+			return "", err
+		}
+		if err = support.DownloadAndUntarArchive(ctx, cdnLink, tmp); err != nil {
+			_ = os.RemoveAll(tmp)

Relevance

⭐⭐ Medium
Repo has accepted some download/extract hardening, but URL allowlisting/redirect validation is more
policy/architecture-specific.
PR-#76

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The cgw strategy now calls ResolveCDNLink and then passes the returned cdnLink into
DownloadAndUntarArchive. ResolveCDNLink returns the tcDownloadURL query parameter without
validation, and Download constructs an HTTP request from that URL string.

pkg/strategy/cgw/cgw.go[40-55]
pkg/support/testSupport.go[130-160]
pkg/support/testSupport.go[88-128]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`ResolveCDNLink` extracts `tcDownloadURL` from a redirect `Location` header and returns it without validating that it is a safe/expected URL. The cgw strategy then downloads and untars from that returned URL.

### Issue Context
- `ResolveCDNLink` returns `parsed.Query().Get("tcDownloadURL")` as-is.
- The download path (`DownloadAndUntarArchive` → `Download`) will perform an HTTP request to whatever URL string is returned.

### Fix Focus Areas
- pkg/strategy/cgw/cgw.go[40-55]
- pkg/support/testSupport.go[130-160]
- pkg/support/testSupport.go[88-128]

### Suggested change
- In `ResolveCDNLink`, parse `tcDownloadURL` with `url.Parse` and **reject**:
 - empty/invalid URLs,
 - non-http(s) schemes,
 - unexpected hosts (e.g., enforce an allowlist like `access.cdn.redhat.com` / `developers.redhat.com` as appropriate).
- Optionally, in cgw strategy, only attempt CDN resolution when the original `link` host matches the expected Developer Portal host(s), so this logic is not applied to arbitrary endpoints.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

3. Sensitive URL logging 🐞 Bug ⛨ Security

Description

cgw.download() logs the fully resolved CDN URL at Info level, including any query string present in
the resolved value. Since the resolved link is taken from a redirect query parameter
(tcDownloadURL), this can leak sensitive URL parameters into logs if present.

Code

pkg/strategy/cgw/cgw.go[R42-48]

+		logrus.Infof("Direct download failed, resolving CDN link for: %s", link)
+		cdnLink, cdnErr := support.ResolveCDNLink(ctx, link)
+		if cdnErr != nil {
+			return "", fmt.Errorf("download failed and CDN resolution failed: %w", cdnErr)
+		}
+		logrus.Infof("Resolved CDN link: %s", cdnLink)
+		tmp, err = os.MkdirTemp("", cliName)

Relevance

⭐⭐ Medium
Security concern, but no close repo precedent about redacting logged URLs/query strings; may be
debated.
PR-#76

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The PR adds an Info-level log line that prints cdnLink, and cdnLink is sourced from the
tcDownloadURL query parameter in ResolveCDNLink. That means the logged value can include
embedded query parameters from the resolved CDN URL.

pkg/strategy/cgw/cgw.go[40-49]
pkg/support/testSupport.go[150-160]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The new log line prints the entire resolved CDN URL (`cdnLink`) at Info level. URLs commonly include sensitive query parameters; logging them can expose those parameters to anyone with log access.

### Issue Context
- `cdnLink` is derived from `tcDownloadURL` inside the redirect `Location` header.

### Fix Focus Areas
- pkg/strategy/cgw/cgw.go[42-48]
- pkg/support/testSupport.go[150-160]

### Suggested change
- Sanitize before logging:
 - parse `cdnLink` as a URL and log only `scheme://host/path`, dropping `RawQuery` and fragments; or
 - log only the hostname (and maybe a short hash of the full URL) for correlation.
- Consider lowering verbosity (Debug) if full link is required for troubleshooting.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

osmman approved these changes Jul 24, 2026

View reviewed changes

sampras343 merged commit 92c7730 into main Jul 24, 2026
6 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat: add CDN redirect resolution to cgw strategy#91

feat: add CDN redirect resolution to cgw strategy#91
sampras343 merged 1 commit into
mainfrom
cgw-cdn-resolve

sampras343 commented Jul 24, 2026

Uh oh!

qodo-for-securesign Bot commented Jul 24, 2026

Uh oh!

qodo-for-securesign Bot commented Jul 24, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

sampras343 commented Jul 24, 2026

Summary

How it works

Why

Verified

Uh oh!

qodo-for-securesign Bot commented Jul 24, 2026

PR Summary by Qodo

Uh oh!

qodo-for-securesign Bot commented Jul 24, 2026

Code Review by Qodo

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants