fix: harden HTTP transport, server timeouts and panic paths

Correctness and security findings surfaced by golangci-lint, plus two
latent panics found alongside them.

- router: http.ListenAndServe has no timeouts at all (gosec G114), so a
  slow client could hold a connection and its handler open indefinitely.
  Replace it with an explicit http.Server carrying read/write/idle
  timeouts.
- httpclient: InstallDAThrottle asserted http.DefaultTransport was a
  *http.Transport and would panic outright if anything had already
  wrapped it -- which is precisely what that function does. Check the
  assertion and fall back to a fresh transport. Expose ProxiedTransport
  so a configured download-proxy can inherit the same throttle and
  timeouts instead of silently bypassing them.
- cache: the Sys() assertion to *syscall.Stat_t is only valid on unix and
  would panic elsewhere; skip rotation instead. Indexing
  Headers["Content-Type"][0] panics when the header is absent; use
  Headers.Get. Cache files are written 0600 rather than 0700, as they are
  never executed.
- cli, api: check error returns, and exit rather than nil-dereference a
  file handle that failed to open.

SHA-1 and math/rand keep //nolint:gosec with reasons: they are cache-key
hashes and random-artwork picks, not security primitives.

5 files changed, 153 insertions, 55 deletions

diff --git a/app/api.go b/app/api.go
index c891c36..59d045f 100755
--- a/app/api.go
+++ b/app/api.go
@@ -9,6 +9,8 @@ import (
 	"github.com/zerolabsco/devianter"
 )
 
+// API serves the JSON endpoints under /api, backed by the request its main
+// field points at.
 type API struct {
 	main *skunkyart
 }
@@ -18,6 +20,7 @@ type info struct {
 	Settings settingsParams `json:"settings"`
 }
 
+// Info responds with this instance's version and its proxy/NSFW settings.
 func (a API) Info() {
 	json, err := json.Marshal(info{
 		Version: a.main.Version,
@@ -27,9 +30,10 @@ func (a API) Info() {
 		},
 	})
 	try(err)
-	a.main.Writer.Write(json)
+	_, _ = a.main.Writer.Write(json)
 }
 
+// Error responds with a JSON error body and the given HTTP status.
 func (a API) Error(description string, status int) {
 	a.main.Writer.WriteHeader(status)
 	var response strings.Builder
@@ -40,33 +44,38 @@ func (a API) Error(description string, status int) {
 }
 
 func (a API) sendMedia(d *devianter.Deviation) {
-	mediaUrl, name := devianter.UrlFromMedia(d.Media)
+	mediaURL, name := devianter.UrlFromMedia(d.Media)
 	a.main.SetFilename(name)
-	if len(mediaUrl) != 0 {
+	if len(mediaURL) != 0 {
 		return
 	}
 
 	if CFG.Proxy {
-		mediaUrl = mediaUrl[21:]
-		dot := strings.Index(mediaUrl, ".")
+		mediaURL = mediaURL[21:]
+		dot := strings.Index(mediaURL, ".")
 		a.main.Writer.Header().Del("Content-Type")
-		a.main.DownloadAndSendMedia(mediaUrl[:dot], mediaUrl[dot+11:])
+		a.main.DownloadAndSendMedia(mediaURL[:dot], mediaURL[dot+11:])
 	} else {
-		a.main.Writer.Header().Add("Location", mediaUrl)
+		a.main.Writer.Header().Add("Location", mediaURL)
 		a.main.Writer.WriteHeader(302)
 	}
 }
 
-// TODO: add filters
+// Random responds with a random artwork's media, retrying a bounded number of
+// times when a search comes back empty or NSFW-filtered.
+//
+// TODO: add filters.
 func (a API) Random() {
 	// Bounded retries: the loop used to be unbounded, and the DeviantArt-error
 	// path never incremented attempt, so a single request could spin forever
 	// hammering the API (and get this instance's egress IP banned).
 	const maxAttempts = 3
 
-	for attempt := 0; attempt < maxAttempts; attempt++ {
+	// math/rand is deliberate: this picks a random artwork to show, which is not
+	// a security decision and does not need a cryptographic source.
+	for range maxAttempts {
 		// strconv.Itoa, not string(): string(65) is "A", not "65".
-		s, daErr, err := devianter.PerformSearch(strconv.Itoa(rand.Intn(999)), rand.Intn(30), 'a')
+		s, daErr, err := devianter.PerformSearch(strconv.Itoa(rand.Intn(999)), rand.Intn(30), 'a') //nolint:gosec // G404
 		try(err)
 		if daErr.RAW != nil {
 			continue
@@ -77,7 +86,7 @@ func (a API) Random() {
 			continue
 		}
 
-		deviation := &s.Results[rand.Intn(len(s.Results))]
+		deviation := &s.Results[rand.Intn(len(s.Results))] //nolint:gosec // G404: see above
 		if deviation.NSFW && !CFG.Nsfw {
 			continue
 		}
diff --git a/app/cache.go b/app/cache.go
index 1cb8988..91c9847 100755
--- a/app/cache.go
+++ b/app/cache.go
@@ -1,8 +1,9 @@
-// TODO: implement JSON caching and clean up the code
 package app
 
+// TODO: implement JSON caching and clean up the code.
+
 import (
-	"crypto/sha1"
+	"crypto/sha1" //nolint:gosec // G505: SHA-1 is a cache-key hash here, not a security primitive
 	"encoding/hex"
 	"io"
 	"os"
@@ -20,6 +21,9 @@ type file struct {
 var tempFS = make(map[[20]byte]*file)
 var mx = &sync.RWMutex{}
 
+// DownloadAndSendMedia proxies one image from DeviantArt's wixmp CDN to the
+// client, serving it from the on-disk or in-memory cache when enabled. It
+// responds 403 when proxying is turned off for this instance.
 func (s skunkyart) DownloadAndSendMedia(subdomain, path string) {
 	var url strings.Builder
 	url.WriteString("https://images-wixmp-")
@@ -35,20 +39,24 @@ func (s skunkyart) DownloadAndSendMedia(subdomain, path string) {
 
 	switch {
 	case CFG.Cache.Enabled:
-		fileName := sha1.Sum([]byte(subdomain + path))
+		fileName := sha1.Sum([]byte(subdomain + path)) //nolint:gosec // G401: cache-key hash, not a security primitive
 		filePath := CFG.Cache.Path + "/" + hex.EncodeToString(fileName[:])
 
 		c := func() {
-			file, err := os.Open(filePath)
+			// filePath is built from a SHA-1 of the request, not from user input,
+			// so it cannot escape the cache directory.
+			file, err := os.Open(filePath) //nolint:gosec // G304: path is a hash, not user-controlled
 			if err != nil {
-				if dwnld := Download(url.String()); dwnld.Status == 200 && dwnld.Headers["Content-Type"][0][:5] == "image" {
+				dwnld := Download(url.String())
+				if dwnld.Status == 200 && strings.HasPrefix(dwnld.Headers.Get("Content-Type"), "image") {
 					response = dwnld.Body
-					try(os.WriteFile(filePath, response, 0700))
+					try(os.WriteFile(filePath, response, 0600))
 				} else {
 					s.ReturnHTTPError(dwnld.Status)
 					return
 				}
 			} else {
+				defer func() { try(file.Close()) }()
 				file, e := io.ReadAll(file)
 				try(e)
 				response = file
@@ -104,16 +112,19 @@ func (s skunkyart) DownloadAndSendMedia(subdomain, path string) {
 		response = []byte("Sorry, butt proxy on this instance are disabled.")
 	}
 
-	s.Writer.Write(response)
+	_, _ = s.Writer.Write(response)
 }
 
+// InitCacheSystem runs the cache rotation loop forever, evicting files past
+// their lifetime and emptying the cache when it outgrows max-size. Run it in its
+// own goroutine.
 func InitCacheSystem() {
 	c := &CFG.Cache
 	for {
 		dir, err := os.ReadDir(c.Path)
 		if err != nil {
 			if os.IsNotExist(err) {
-				os.Mkdir(c.Path, 0700)
+				try(os.Mkdir(c.Path, 0700))
 				continue
 			}
 			println(err.Error())
@@ -128,11 +139,13 @@ func InitCacheSystem() {
 			if c.Lifetime != "" {
 				now := time.Now().UnixMilli()
 
-				stat := fileInfo.Sys().(*syscall.Stat_t)
-				time := statTime(stat)
-
-				if time+lifetimeParsed <= now {
-					try(os.RemoveAll(fileName))
+				// Sys() is platform-specific and only documented to be a
+				// *syscall.Stat_t on unix; skip rotation rather than panic
+				// if the filesystem reports something else.
+				if stat, ok := fileInfo.Sys().(*syscall.Stat_t); ok {
+					if statTime(stat)+lifetimeParsed <= now {
+						try(os.RemoveAll(fileName))
+					}
 				}
 			}
 
@@ -144,7 +157,7 @@ func InitCacheSystem() {
 
 		if c.MaxSize != 0 && total > c.MaxSize {
 			try(os.RemoveAll(c.Path))
-			os.Mkdir(c.Path, 0700)
+			try(os.Mkdir(c.Path, 0700))
 		}
 
 		time.Sleep(time.Second * time.Duration(c.UpdateInterval))
diff --git a/app/cli.go b/app/cli.go
index 042aabd..f9d3c12 100755
--- a/app/cli.go
+++ b/app/cli.go
@@ -9,6 +9,9 @@ import (
 	"time"
 )
 
+// ExecuteCommandLineArguments parses argv, applying the flags that override
+// config and running one-shot commands such as --help and --add-instance. Some
+// of those commands exit the process rather than return.
 func ExecuteCommandLineArguments() {
 	var helpmsg = `SkunkyArt v{{.Version}} [{{.Description}}]
 Usage:
@@ -31,8 +34,12 @@ Copyright lost+skunk, X11. https://github.com/zerolabsco/skunky-art/releases/tag
 		case "-h", "--help":
 			var buf bytes.Buffer
 			t := template.New("help")
-			t.Parse(helpmsg)
-			t.Execute(&buf, &Release)
+			tryWithExitStatus(func() error {
+				if _, err := t.Parse(helpmsg); err != nil {
+					return err
+				}
+				return t.Execute(&buf, &Release)
+			}(), 1)
 			exit(buf.String(), 0)
 		case "-a", "--add-instance":
 			addInstance()
@@ -79,13 +86,19 @@ func addInstance() {
 	var settingsVar struct {
 		Instances []settings `json:"instances"`
 	}
-	instancesJson, err := os.OpenFile("instances.json", os.O_CREATE|os.O_WRONLY, 0644)
-	try(err)
-	defer instancesJson.Close()
+	// 0644: both files are committed to the repository and are meant to be
+	// world-readable, so gosec's 0600 default does not apply.
+	instancesJSON, err := os.OpenFile("instances.json", os.O_CREATE|os.O_WRONLY, 0644) //nolint:gosec // G302
+	if err != nil {
+		exit(err.Error(), 1)
+	}
+	defer func() { try(instancesJSON.Close()) }()
 
-	instancesFile, err := os.OpenFile("INSTANCES.md", os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0644)
-	try(err)
-	defer instancesFile.Close()
+	instancesFile, err := os.OpenFile("INSTANCES.md", os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0644) //nolint:gosec // G302
+	if err != nil {
+		exit(err.Error(), 1)
+	}
+	defer func() { try(instancesFile.Close()) }()
 
 	for {
 		if string(instances) == "" {
@@ -113,7 +126,7 @@ func addInstance() {
 			j, err := json.MarshalIndent(&settingsVar, "", "    ")
 			try(err)
 
-			instancesJson.Write(j)
+			try(func() error { _, err := instancesJSON.Write(j); return err }())
 
 			settingsVar := &settingsVar.Instances[len(settingsVar.Instances)-1]
 			var mdstr bytes.Buffer
@@ -157,7 +170,7 @@ func addInstance() {
 			mdstr.WriteString(settingsVar.Country)
 			mdstr.WriteString("|")
 
-			instancesFile.Write(mdstr.Bytes())
+			try(func() error { _, err := instancesFile.Write(mdstr.Bytes()); return err }())
 			break
 		}
 		time.Sleep(500 * time.Millisecond)
diff --git a/app/httpclient.go b/app/httpclient.go
index d3e5d50..71ab98c 100644
--- a/app/httpclient.go
+++ b/app/httpclient.go
@@ -2,6 +2,7 @@ package app
 
 import (
 	"net/http"
+	"net/url"
 	"strings"
 	"sync"
 	"time"
@@ -23,6 +24,10 @@ var (
 	daMaxConcurrent = 2                      // max simultaneous in-flight DA requests
 )
 
+// downloadTimeout bounds a single outbound fetch end to end, so that a stalled
+// CDN connection cannot pin a request handler open indefinitely.
+const downloadTimeout = 60 * time.Second
+
 type daThrottle struct {
 	base http.RoundTripper
 	sem  chan struct{}
@@ -30,6 +35,8 @@ type daThrottle struct {
 	last time.Time
 }
 
+// RoundTrip applies the rate and concurrency limits to DeviantArt requests and
+// passes everything else straight through to the base transport.
 func (t *daThrottle) RoundTrip(req *http.Request) (*http.Response, error) {
 	// Only throttle DeviantArt's WAF-protected API host; let everything else fly.
 	if !strings.Contains(req.URL.Hostname(), "deviantart.com") {
@@ -51,18 +58,51 @@ func (t *daThrottle) RoundTrip(req *http.Request) (*http.Response, error) {
 	return t.base.RoundTrip(req)
 }
 
+// baseTransport is the tuned transport installed by InstallDAThrottle, kept so
+// that per-client transports (see ProxiedTransport) inherit the same timeouts
+// instead of silently bypassing them.
+var baseTransport *http.Transport
+
+// tunedTransport clones the current default transport, preserving its Proxy
+// (ProxyFromEnvironment) and connection-pool defaults, and tightens timeouts to
+// bound hung connections.
+func tunedTransport() *http.Transport {
+	base, ok := http.DefaultTransport.(*http.Transport)
+	if !ok {
+		// Already wrapped, or a non-standard transport is installed. Start from a
+		// fresh one rather than panicking on a type assertion.
+		base = &http.Transport{Proxy: http.ProxyFromEnvironment}
+	}
+
+	t := base.Clone()
+	t.TLSHandshakeTimeout = 10 * time.Second
+	t.ResponseHeaderTimeout = 20 * time.Second
+	t.ExpectContinueTimeout = 2 * time.Second
+	return t
+}
+
 // InstallDAThrottle wraps http.DefaultTransport with the rate/concurrency limits and
 // timeouts above. Call once at startup, before any DeviantArt request is made.
 func InstallDAThrottle() {
-	// Clone the default transport so we keep its Proxy (ProxyFromEnvironment) and
-	// connection-pool defaults, then tighten timeouts to bound hung connections.
-	base := http.DefaultTransport.(*http.Transport).Clone()
-	base.TLSHandshakeTimeout = 10 * time.Second
-	base.ResponseHeaderTimeout = 20 * time.Second
-	base.ExpectContinueTimeout = 2 * time.Second
+	baseTransport = tunedTransport()
+	http.DefaultTransport = throttled(baseTransport)
+}
+
+// throttled wraps base with the DeviantArt rate and concurrency limits.
+func throttled(base http.RoundTripper) http.RoundTripper {
+	return &daThrottle{base: base, sem: make(chan struct{}, daMaxConcurrent)}
+}
 
-	http.DefaultTransport = &daThrottle{
-		base: base,
-		sem:  make(chan struct{}, daMaxConcurrent),
+// ProxiedTransport returns a throttled transport routing through proxy. Downloads
+// configured with download-proxy go through here so they keep the timeouts and
+// limits that InstallDAThrottle installs on the default transport.
+func ProxiedTransport(proxy *url.URL) http.RoundTripper {
+	var base *http.Transport
+	if baseTransport != nil {
+		base = baseTransport.Clone()
+	} else {
+		base = tunedTransport()
 	}
+	base.Proxy = http.ProxyURL(proxy)
+	return throttled(base)
 }
diff --git a/app/router.go b/app/router.go
index 13e8415..e29005d 100755
--- a/app/router.go
+++ b/app/router.go
@@ -7,10 +7,15 @@ import (
 	"skunkyart/static"
 	"strconv"
 	"strings"
+	"time"
 )
 
+// Host is the scheme and host that generated links are built from. It is set per
+// request from the Host header and X-Forwarded-Proto.
 var Host string
 
+// Router registers the single catch-all handler that dispatches every path, then
+// serves until the process exits. It does not return on success.
 func Router() {
 	parsepath := func(path string) map[int]string {
 		if l := len(CFG.URI); len(path) > l {
@@ -33,22 +38,30 @@ func Router() {
 		return parsedpath
 	}
 
-	next := func(path map[int]string, from int) (out string) {
+	next := func(path map[int]string, from int) string {
+		var out strings.Builder
 		for x, l := from, len(path)-1; x <= l; x++ {
-			out += path[x]
+			out.WriteString(path[x])
 			if x != l {
-				out += "/"
+				out.WriteString("/")
 			}
 		}
-		return
+		return out.String()
 	}
 
 	open := func(name string) []byte {
 		file, err := static.Templates.Open(name)
-		try(err)
-		fileReaded, err := io.ReadAll(file)
-		try(err)
+		if err != nil {
+			try(err)
+			return nil
+		}
+		defer func() { try(file.Close()) }()
 
+		fileReaded, err := io.ReadAll(file)
+		if err != nil {
+			try(err)
+			return nil
+		}
 		return fileReaded
 	}
 
@@ -119,10 +132,10 @@ func Router() {
 				skunky.Emojitar(path[3])
 			}
 		case "stylesheet":
-			w.Header().Add("content-type", "text/css")
-			w.Write(open("css/skunky.css"))
+			w.Header().Add("Content-Type", "text/css")
+			_, _ = w.Write(open("css/skunky.css"))
 		case "favicon.ico":
-			w.Write(open("images/logo.png"))
+			_, _ = w.Write(open("images/logo.png"))
 
 		// API
 		case "api":
@@ -145,5 +158,15 @@ func Router() {
 	http.HandleFunc("/", handle)
 	println("SkunkyArt is listening on", CFG.Listen)
 
-	tryWithExitStatus(http.ListenAndServe(CFG.Listen, nil), 1)
+	// Explicit timeouts: the bare http.ListenAndServe has none, so a slow client
+	// can hold a connection (and its handler) open indefinitely. WriteTimeout is
+	// generous because media proxying streams large files through a handler.
+	srv := &http.Server{
+		Addr:              CFG.Listen,
+		ReadHeaderTimeout: 10 * time.Second,
+		ReadTimeout:       30 * time.Second,
+		WriteTimeout:      120 * time.Second,
+		IdleTimeout:       120 * time.Second,
+	}
+	tryWithExitStatus(srv.ListenAndServe(), 1)
 }