From 92b7df3da7721343d8e7c3698239111b85bc5cc6 Mon Sep 17 00:00:00 2001
From: Christoph Heiss <christoph@c8h4.io>
Date: Sun, 25 May 2025 19:16:18 +0200
Subject: [PATCH] fix: artifacts: format IP:port pair using net.JoinHostPort()

This ensures that brackets are added for IPv6 addresses.
Without this, This could result in addresses like "2001:db8::1:3456",
which - obviously - would break further down and prevent the server from
starting.

Signed-off-by: Christoph Heiss <christoph@c8h4.io>
---
 act/artifactcache/handler.go      |  6 +++---
 act/artifactcache/handler_test.go | 35 +++++++++++++++++++++++++++++++
 act/artifacts/server.go           |  5 +++--
 3 files changed, 41 insertions(+), 5 deletions(-)

diff --git a/act/artifactcache/handler.go b/act/artifactcache/handler.go
index 28bacec5..55bb3308 100644
--- a/act/artifactcache/handler.go
+++ b/act/artifactcache/handler.go
@@ -115,10 +115,10 @@ func StartHandler(dir, outboundIP string, port uint16, secret string, logger log
 }
 
 func (h *Handler) ExternalURL() string {
+	port := strconv.Itoa(h.listener.Addr().(*net.TCPAddr).Port)
+
 	// TODO: make the external url configurable if necessary
-	return fmt.Sprintf("http://%s:%d",
-		h.outboundIP,
-		h.listener.Addr().(*net.TCPAddr).Port)
+	return fmt.Sprintf("http://%s", net.JoinHostPort(h.outboundIP, port))
 }
 
 func (h *Handler) Close() error {
diff --git a/act/artifactcache/handler_test.go b/act/artifactcache/handler_test.go
index ebfdad77..fdb807ad 100644
--- a/act/artifactcache/handler_test.go
+++ b/act/artifactcache/handler_test.go
@@ -746,3 +746,38 @@ func TestHandler_gcCache(t *testing.T) {
 	}
 	require.NoError(t, db.Close())
 }
+
+func TestHandler_ExternalURL(t *testing.T) {
+	t.Run("reports correct URL on IPv4", func(t *testing.T) {
+		dir := filepath.Join(t.TempDir(), "artifactcache")
+		handler, err := StartHandler(dir, "127.0.0.1", 34567, "secret", nil)
+		require.NoError(t, err)
+
+		assert.Equal(t, handler.ExternalURL(), "http://127.0.0.1:34567")
+		require.NoError(t, handler.Close())
+		assert.Nil(t, handler.server)
+		assert.Nil(t, handler.listener)
+	})
+
+	t.Run("reports correct URL on IPv6 zero host", func(t *testing.T) {
+		dir := filepath.Join(t.TempDir(), "artifactcache")
+		handler, err := StartHandler(dir, "2001:db8::", 34567, "secret", nil)
+		require.NoError(t, err)
+
+		assert.Equal(t, handler.ExternalURL(), "http://[2001:db8::]:34567")
+		require.NoError(t, handler.Close())
+		assert.Nil(t, handler.server)
+		assert.Nil(t, handler.listener)
+	})
+
+	t.Run("reports correct URL on IPv6", func(t *testing.T) {
+		dir := filepath.Join(t.TempDir(), "artifactcache")
+		handler, err := StartHandler(dir, "2001:db8::1:2:3:4", 34567, "secret", nil)
+		require.NoError(t, err)
+
+		assert.Equal(t, handler.ExternalURL(), "http://[2001:db8::1:2:3:4]:34567")
+		require.NoError(t, handler.Close())
+		assert.Nil(t, handler.server)
+		assert.Nil(t, handler.listener)
+	})
+}
diff --git a/act/artifacts/server.go b/act/artifacts/server.go
index 160dd4db..4d5bbf14 100644
--- a/act/artifacts/server.go
+++ b/act/artifacts/server.go
@@ -7,6 +7,7 @@ import (
 	"fmt"
 	"io"
 	"io/fs"
+	"net"
 	"net/http"
 	"os"
 	"path/filepath"
@@ -291,14 +292,14 @@ func Serve(ctx context.Context, artifactPath string, addr string, port string) c
 	downloads(router, artifactPath, fsys)
 
 	server := &http.Server{
-		Addr:              fmt.Sprintf("%s:%s", addr, port),
+		Addr:              net.JoinHostPort(addr, port),
 		ReadHeaderTimeout: 2 * time.Second,
 		Handler:           router,
 	}
 
 	// run server
 	go func() {
-		logger.Infof("Start server on http://%s:%s", addr, port)
+		logger.Infof("Start server on http://%s", server.Addr)
 		if err := server.ListenAndServe(); err != nil && err != http.ErrServerClosed {
 			logger.Fatalf("http listener: %v", err)
 		}