Enforce HTTPS for wrapper remote repositories

[jamesfredley]

The Problem

The Grails Wrapper defaults to HTTPS repositories, but the repository list can be overridden via GRAILS_REPO_URL or grails.repo.url. Two gaps let wrapper downloads happen over an untrusted channel:

• A remote repository entry using http:// was accepted and used for Maven metadata and wrapper artifact downloads.
• HttpURLConnection was allowed to follow redirects automatically, so an HTTPS repository could redirect to an http:// URL and the wrapper would continue the download without re-checking the final scheme.

This is a hardening issue - it requires an explicitly non-HTTPS remote repository, a hostile redirect, or network control over a non-HTTPS path - but when it happens the wrapper can fetch metadata or CLI artifacts over a tamperable channel.

The Fix

Remote wrapper repository downloads now fail closed unless the URL is HTTPS, at every hop.

• Validate the remote metadata URL is HTTPS before opening the connection.
• Disable automatic redirect following on HttpURLConnection.
• Follow redirects manually, resolving and re-validating each Location target before use.
• Reject redirects with a missing Location header and redirects that downgrade HTTPS -> HTTP.
• Cap the redirect chain to avoid unbounded redirect handling.
• Local filesystem repositories are unchanged - they continue through the existing local repository path.

Files

• grails-wrapper/.../GrailsUpdater.java - HTTPS validation, manual redirect handling, downgrade/loop rejection.
• GrailsUpdaterSpec.groovy - new no-network boundary tests for accepted and rejected redirect chains.
• INSTALL - note on HTTPS enforcement.

Review feedback

Copilot raised three points, all addressed: added the missing java.net.URI import; reworked redirect counting so MAX_REDIRECTS is a hard cap while a final non-redirect response after the allowed redirects still succeeds; and generalized redirect error wording to "wrapper remote resource" since the helper serves both metadata and artifact downloads.

Testing

• :grails-wrapper:test --tests "grails.init.GrailsUpdaterSpec" and :grails-wrapper:check pass.

[Copilot]

Pull request overview

This PR hardens the Grails wrapper updater by enforcing HTTPS for remote repository downloads (including redirect targets), aligning wrapper-fetch behavior with audit finding requirements while keeping local filesystem repositories usable via GRAILS_REPO_URL.

Changes:

• Enforce HTTPS-only for remote wrapper repository URLs and validate redirect targets to prevent HTTP downgrade.
• Add redirect handling with an explicit redirect limit (manual redirect resolution rather than implicit follow-redirects).
• Document the HTTPS requirement for remote GRAILS_REPO_URL entries while clarifying local filesystem support.

Reviewed changes

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

File	Description
INSTALL	Documents that remote GRAILS_REPO_URL repositories must be HTTPS while local filesystem repos remain supported.
grails-wrapper/src/test/groovy/grails/init/GrailsUpdaterSpec.groovy	Adds tests for HTTPS enforcement and redirect validation behavior.
grails-wrapper/src/main/java/grails/init/GrailsUpdater.java	Implements HTTPS validation and manual redirect handling with a redirect cap.

---

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

[codecov]

Codecov Report

❌ Patch coverage is 77.77778% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 49.5119%. Comparing base (b86be15) to head (fecd117).
⚠️ Report is 2 commits behind head on 8.0.x.

Files with missing lines	Patch %	Lines
...apper/src/main/java/grails/init/GrailsUpdater.java	77.7778%	6 Missing and 2 partials ⚠️

Additional details and impacted files

@@                Coverage Diff                 @@
##                8.0.x     #15809        +/-   ##
==================================================
+ Coverage     49.4820%   49.5119%   +0.0299%     
- Complexity      16698      16711        +13     
==================================================
  Files            1947       1947                
  Lines           92474      92505        +31     
  Branches        16152      16154         +2     
==================================================
+ Hits            45758      45801        +43     
+ Misses          39610      39592        -18     
- Partials         7106       7112         +6     

Files with missing lines	Coverage Δ
...apper/src/main/java/grails/init/GrailsUpdater.java	14.5729% <77.7778%> (+14.5729%)	⬆️

... and 4 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[jamesfredley]

Addressed the Copilot feedback in follow-up commit fecd117. The missing URI import is fixed, redirect counting now has explicit boundary coverage for the allowed and rejected cases, the shared download error wording no longer says only artifact, all three review threads have replies and are marked resolved, and :grails-wrapper:check passed locally.

[testlens-app]

✅ All tests passed ✅

🏷️ Commit: fecd117
▶️ Tests: 40506 executed
⚪️ Checks: 44/44 completed

---

Learn more about TestLens at testlens.app.

[jamesfredley]

Pre-release review pass (2026-07-02, dual-reviewer: GPT-5.5 oracle + Codex CLI): APPROVE - no blocking findings.

Reviewed the full diff plus local grails-wrapper context: HTTPS validation for all remote repo access, implicit redirect following disabled with manual re-validation of each hop, redirect cap (MAX_REDIRECTS = 5), missing Location treated as failure, local filesystem repos preserved, and the HTTPS requirement documented in INSTALL. GrailsUpdaterSpec exercises success and failure paths including redirect loops. :grails-wrapper:test passed locally and PR checks were green.

[jdaugherty]

To add this documentation, this change is incomplete - we use the GRAILS_REPO_URL in both profiles & forge. Such limits are not added there.

[jdaugherty]

1. we need to make GRAILS_REPO_URL consistent (not fully implemented)
2. we need to mock the server ourself or avoid external url dependencies.

[jdaugherty]

This shouldn't be infinite. I get that MAX_REDIRECTS below is the max, but it should be used in the loop condition. I asked AI to clean this up and it came up with:

    @FunctionalInterface
    interface HttpURLConnectionFactory {
        HttpURLConnection open(URL url) throws IOException;
    }

    static HttpURLConnection createHttpURLConnection(
            String mavenMetadataFileUrl,
            HttpURLConnectionFactory openConnection) throws IOException {

        URI uri = createSecureRemoteUri(mavenMetadataFileUrl);

        for (int redirectCount = 0; ; redirectCount++) {
            URL url = uri.toURL();
            HttpURLConnection conn = openConnection.open(url);

            conn.setRequestProperty("User-Agent", "Apache-Maven/3.9.6");
            conn.setInstanceFollowRedirects(false);

            int responseCode = conn.getResponseCode();

            if (!isRedirect(responseCode)) {
                return conn; // caller owns this connection and must close/disconnect it
            }

            if (redirectCount >= MAX_REDIRECTS) {
                closeQuietly(conn);
                throw new IOException("Too many redirects while downloading Grails wrapper remote resource: "
                        + mavenMetadataFileUrl);
            }

            String location = conn.getHeaderField("Location");
            closeQuietly(conn);

            if (location == null || location.isBlank()) {
                throw new IOException("Redirect response from " + uri + " did not include a Location header");
            }

            uri = resolveSecureRedirectUrl(uri, location);
        }
    }

    private static void closeQuietly(HttpURLConnection conn) {
        try {
            InputStream errorStream = conn.getErrorStream();
            if (errorStream != null) {
                errorStream.close();
            }
        } catch (IOException ignored) {
            // ignore cleanup failure
        } finally {
            conn.disconnect();
        }
    }

[jdaugherty]

We should not be using external urls for the build. Use the ersatz server to set these up and test them correctly.