Skip to content

feat: add downloadBaseURL input to support custom download mirrors#260

Open
acouvreur wants to merge 1 commit into
Azure:mainfrom
acouvreur:feature/download-base-url
Open

feat: add downloadBaseURL input to support custom download mirrors#260
acouvreur wants to merge 1 commit into
Azure:mainfrom
acouvreur:feature/download-base-url

Conversation

@acouvreur
Copy link
Copy Markdown

@acouvreur acouvreur commented May 13, 2026

Summary

Closes #206

Adds a new optional downloadBaseURL action input that allows users to specify a custom or private mirror for kubectl downloads, useful in air-gapped or enterprise environments.

Changes

  • action.yml — new optional input downloadBaseURL (default: https://dl.k8s.io)
  • src/helpers.tsgetkubectlDownloadURL() accepts a baseURL parameter (trailing slashes are stripped)
  • src/run.tsdownloadKubectl() accepts and forwards downloadBaseURL; run() reads the input and passes it through
  • src/run.test.ts — 7 new tests covering custom base URL and trailing-slash normalization

Usage

- uses: Azure/setup-kubectl@v4
  with:
    version: 'v1.30.0'
    downloadBaseURL: 'https://my-mirror.example.com'

Tests

All 37 tests pass.

@acouvreur acouvreur requested a review from a team as a code owner May 13, 2026 19:02
@acouvreur
Copy link
Copy Markdown
Author

@microsoft-github-policy-service agree

Copy link
Copy Markdown
Member

@Tatsinnit Tatsinnit left a comment

Choose a reason for hiding this comment

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

❤️ 💡 Thank you so much for this @acouvreur, also for the contribution for #261 , I really appreciate this, gentle Fyi to regular contributors like @davidgamero or @bosesuneha

Here are some key notes and worth to get thoughts and eye collectively here, but worth a clear share in details:

TL;DR

This PR is a definitely small, reasonable feature with an outsized security surface. The diff itself is clean, but it ships an arbitrary-URL fetch-and-exec primitive with no integrity verification, no scheme validation, no SSRF guardrails, and an incomplete realization of its own use case (latest still hits dl.k8s.io). The PR is a much stronger merge candidate with input validation + a checksum story baked in, even as a follow-up tracked in the PR description.

Behavior changes vs. current

  1. New optional input downloadBaseURL (default https://dl.k8s.io) — backward compatible when omitted. Trailing slash is stripped via baseURL.replace(/\/$/, '').

  2. getkubectlDownloadURL(version, arch, baseURL) — third param added with default. Default-arg keeps callers happy; no breaking signature change at runtime, but TypeScript consumers importing the helper get a wider signature.

  3. downloadKubectl(version, downloadBaseURL) — same pattern. Default 'https://dl.k8s.io' is duplicated in three places (action.yml, getkubectlDownloadURL, downloadKubectl). Drift risk — should be one exported constant.

  4. run() reads downloadBaseURL and forwards it. Note: core.getInput returns '' (empty string) when not set, not undefined. The current code passes that empty string straight into downloadKubectl(version, ''), which then passes '' into getkubectlDownloadURL(version, arch, ''). Because the default-parameter only kicks in for undefined, the action.yml default https://dl.k8s.io is what actually saves it — the function defaults are dead code in the GH Actions path. If a non-Actions caller forgets the param they get the helper default; if it's the Actions path the action.yml default wins. Inconsistent but not broken. Worth either: (a) coalescing || 'https://dl.k8s.io' in run.ts, or (b) removing the action.yml default and relying on the function defaults.

  5. getStableKubectlVersion() still hardcodes https://dl.k8s.io/release/stable.txt. So in an air-gapped scenario where version: latest is set, the mirror is bypassed and the call to dl.k8s.io fails. The feature is incomplete for its stated air-gapped use case — stableVersionUrl should also derive from downloadBaseURL.

  6. No checksum/signature verification anywhere in the action (this is pre-existing, but #260 makes the gap much worse — see security).

Security aspects: this is where it gets interesting

1. SSRF / arbitrary binary execution (the big one)

The whole point of this input is "fetch a binary from a URL of the caller's choosing, mark it executable, and add it to PATH". That is a introduced unrestricted-URL fetch + exec sink, and it lives in a GitHub Action that runs in workflows.

Threat model matters a lot here:

  • Repo-controlled workflow with hardcoded downloadBaseURL: low risk. Workflow author already controls what runs.
  • Reusable workflow / composite action that forwards inputs: medium risk. A downstream consumer can supply a malicious mirror.
  • pull_request_target or matrix from untrusted source / Dependabot-like flows that pass values through: high risk. An attacker who can influence the input value gets RCE on the runner with whatever secrets the job has.
  • Workflows that interpolate ${{ github.event.* }} into action inputs (anti-pattern but common): direct path to attacker-controlled URL.

Mitigations the PR could/should adopt:

  • Scheme allow-list: reject anything but https:// (and maybe http:// only when explicitly allowed). The current code accepts , ftp://, data:, javascript: (toolCache may filter some, but defense in depth).
  • URL parsing via new URL(baseURL) to validate structure, then reconstruct — don't just string-concatenate.
  • Optional allow-list input (downloadBaseURLAllowlist or doc guidance) so org admins can pin via org-level required workflows.
  • Warn loudly (core.warning or core.notice) when a non-default base URL is used — gives audit visibility in logs.

2. No integrity verification

With the official https://dl.k8s.io host you at least get TLS + a host the k8s community controls. With a custom mirror you've delegated trust to whoever runs that mirror, and the action still:

  • Doesn't verify a SHA256 against https://dl.k8s.io/release/<v>/bin/<os>/<arch>/kubectl.sha256 (which exists upstream).
  • Doesn't verify cosign / sigstore signatures (kubectl is signed since v1.26).

Recommendation worth raising on the PR: when downloadBaseURL != default, require or at least strongly encourage a checksum input. The combination "custom mirror + no checksum" is the actually-dangerous configuration. The PR would be much more defensible if it shipped with checksum verification in the same change (or as an immediate follow-up tracked in the PR description).

3. URL construction — string concat with user input

const base = baseURL.replace(/\/$/, '')
return `${base}/release/${version}/bin/linux/${arch}/kubectl`

Issues:

  • version here is also user-controlled (same sink as #261's concern). Combined with custom base URL, an attacker controlling both could craft baseURL=https://evil.tld + version=../../whatever to bend the path. Tool cache key (toolCache.find('kubectl', version)) also picks this up as a directory name. Validate version against a regex (the suggestion from #261 applies here too — even more so now).
  • No validation that baseURL doesn't already include /release/.... User puts downloadBaseURL: https://mirror/k8s/release and you get …/release/release/v… — silent 404. Cosmetic, but a sharp edge.
  • Userinfo in URL (https://user:pass@host) is accepted. That can leak credentials into logs via core.info of the download URL (if anything logs it).
  • IDN / unicode hosts: new URL('https://раураl.com') parses fine. Probably out of scope, but doc the assumption.

4. Open redirect / 302 to another origin

toolCache.downloadTool follows redirects. A mirror that 302s to passwd or http://169.254.169.254/... (cloud metadata) is a classic SSRF pivot. Mitigation: pass { allowRedirects: false } or validate the final resolved URL stays on the same origin as baseURL. At minimum: when not the default base URL, disable HTTP fallback (only allow HTTPS end-to-end, including redirects).

5. Cloud metadata / SSRF to link-local

http://169.254.169.254/ (AWS/Azure IMDS), http://[fd00:ec2::254], http://localhost:..., http://10.x.x.x. On hosted runners these may be partly mitigated (IMDSv2 etc.), but on self-hosted runners this is real. Worth blocking RFC1918 / link-local / loopback by default and gating behind an explicit "allow private" flag.

6. Test coverage gaps

The PR's 7 tests cover happy-path + trailing-slash. They do not assert:

  • Empty-string input is handled.
  • Invalid URLs are rejected.
  • The getStableKubectlVersion path is unaffected (it still hits dl.k8s.io, which contradicts the air-gap goal — should be a test or an acknowledged limitation).
  • Behavior when downloadBaseURL includes a path (https://host/k8s) — does it concatenate correctly? Looks like yes, but no test.

7. Supply-chain framing

GitHub's own actions/setup-* actions have moved toward signature/checksum verification specifically because "let users point at a mirror" is a known footgun (actions/setup-node, setup-go discussions). Worth aligning with that direction before merging.

A plan for moving forward PR as a concrete review asks for #260

  1. Validate the input: new URL() parse, require https:, reject userinfo, optionally reject private/link-local hosts unless an opt-in flag is set.
  2. Single source of truth for the default: export const DEFAULT_KUBECTL_BASE_URL = 'https://dl.k8s.io' from helpers.ts and import it in action.yml docs and run.ts. Remove duplication.
  3. Fix the getStableKubectlVersion gap so stableVersionUrl is built from downloadBaseURL — otherwise version: latest + custom mirror is broken (the headline use case is air-gapped envs).
  4. Validate version with a regex (same fix that #261 needs).
  5. Add checksum verification, or at least an optional checksum / checksumURL input, and document that custom mirrors should always set it. Without this the feature meaningfully degrades the action's security posture.
  6. Disable or constrain redirects in toolCache.downloadTool; or post-validate that the final URL's origin matches baseURL's origin.
  7. core.notice when a non-default base URL is in use — visible in the Actions summary, helps audit.
  8. Tests: empty string, invalid URL, redirect to disallowed scheme, IMDS address, base URL containing a path, base URL with userinfo.
  9. Docs: warn loudly in README that the input bypasses upstream and the user is on the hook for integrity; recommend pairing with checksum.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds an optional downloadBaseURL action input so users in air-gapped/enterprise environments can fetch kubectl from a custom mirror instead of https://dl.k8s.io. The new value is threaded from run() through downloadKubectl() into getkubectlDownloadURL(), with trailing-slash normalization, and is covered by new unit tests.

Changes:

  • New optional downloadBaseURL input in action.yml defaulting to https://dl.k8s.io.
  • getkubectlDownloadURL and downloadKubectl accept a base URL (default preserved, trailing / stripped).
  • New tests cover custom base URL, trailing-slash normalization, the default URL, and that core.getInput is called for the new input.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

File Description
action.yml Declares the new optional downloadBaseURL input with default https://dl.k8s.io.
src/helpers.ts getkubectlDownloadURL gains a baseURL parameter and strips a trailing slash before building per-OS URLs.
src/run.ts run() reads downloadBaseURL via core.getInput and forwards it; downloadKubectl accepts and propagates it with a default.
src/run.test.ts Adds tests for custom base URL, trailing-slash stripping, default-URL assertion, and new getInput call.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/run.ts
Comment on lines +52 to +55
export async function downloadKubectl(
version: string,
downloadBaseURL: string = 'https://dl.k8s.io'
): Promise<string> {
Comment thread src/run.ts
version = await resolveKubectlVersion(version)
}
const cachedPath = await downloadKubectl(version)
const downloadBaseURL = core.getInput('downloadBaseURL', {required: false})
Comment thread src/run.ts
Comment on lines +24 to +25
const downloadBaseURL = core.getInput('downloadBaseURL', {required: false})
const cachedPath = await downloadKubectl(version, downloadBaseURL)
Comment thread action.yml
required: true
default: 'latest'
downloadBaseURL:
description: 'Set the download base URL'
@Tatsinnit Tatsinnit requested a review from a team May 15, 2026 00:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Feature Request: Add downloadBaseURL input

3 participants