From 3e3b669d69f8fe39a3a0044db192fe2a91d20098 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andri=20=C3=93skarsson?= Date: Sat, 28 Feb 2026 09:55:31 +0100 Subject: [PATCH] Move pkg/ to internal/, extract ca and mobileconfig packages, remove redundant tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Rename pkg/ to internal/ — this is an application, not a library - Extract ca.go + cert.go into internal/ca/ (CA generation, cert caching) - Extract mobileconfig.go into internal/mobileconfig/ (Apple profiles) - Remove 18 redundant tests (~320 lines): strict subsets, HTTPS transport duplicates consolidated into TestHTTPSHTMLRewriting, unit/integration overlaps, and low-value tests --- DECISIONS.md | 3 + api.go | 4 +- api_auth.go | 2 +- api_rules.go | 2 +- api_subscriptions.go | 2 +- api_test.go | 4 +- connect.go | 2 +- elemhide_inject.go | 2 +- http.go | 2 +- inject_test.go | 31 +- {pkg => internal}/blocklist/blocklist.go | 0 {pkg => internal}/blocklist/elemhide.go | 0 {pkg => internal}/blocklist/elemhide_test.go | 37 +- {pkg => internal}/blocklist/pattern.go | 0 {pkg => internal}/blocklist/pattern_test.go | 2 +- {pkg => internal}/blocklist/resource_type.go | 0 .../blocklist/resource_type_test.go | 11 +- {pkg => internal}/blocklist/ruleset.go | 0 {pkg => internal}/blocklist/ruleset_test.go | 55 +-- ca.go => internal/ca/ca.go | 26 +- cert.go => internal/ca/cache.go | 27 +- .../mobileconfig/mobileconfig.go | 14 +- {pkg => internal}/store/cache.go | 0 {pkg => internal}/store/credentials.go | 0 {pkg => internal}/store/rules.go | 0 {pkg => internal}/store/sessions.go | 0 {pkg => internal}/store/store.go | 0 {pkg => internal}/store/store_test.go | 0 {pkg => internal}/store/subscriptions.go | 0 {pkg => internal}/webauthn/json.go | 0 {pkg => internal}/webauthn/webauthn.go | 0 {pkg => internal}/webauthn/webauthn_test.go | 13 - main.go | 11 +- portal.go | 6 +- portal_https.go | 6 +- proxy.go | 9 +- proxy_test.go | 354 ++---------------- 37 files changed, 112 insertions(+), 513 deletions(-) rename {pkg => internal}/blocklist/blocklist.go (100%) rename {pkg => internal}/blocklist/elemhide.go (100%) rename {pkg => internal}/blocklist/elemhide_test.go (75%) rename {pkg => internal}/blocklist/pattern.go (100%) rename {pkg => internal}/blocklist/pattern_test.go (99%) rename {pkg => internal}/blocklist/resource_type.go (100%) rename {pkg => internal}/blocklist/resource_type_test.go (92%) rename {pkg => internal}/blocklist/ruleset.go (100%) rename {pkg => internal}/blocklist/ruleset_test.go (89%) rename ca.go => internal/ca/ca.go (82%) rename cert.go => internal/ca/cache.go (85%) rename mobileconfig.go => internal/mobileconfig/mobileconfig.go (88%) rename {pkg => internal}/store/cache.go (100%) rename {pkg => internal}/store/credentials.go (100%) rename {pkg => internal}/store/rules.go (100%) rename {pkg => internal}/store/sessions.go (100%) rename {pkg => internal}/store/store.go (100%) rename {pkg => internal}/store/store_test.go (100%) rename {pkg => internal}/store/subscriptions.go (100%) rename {pkg => internal}/webauthn/json.go (100%) rename {pkg => internal}/webauthn/webauthn.go (100%) rename {pkg => internal}/webauthn/webauthn_test.go (96%) diff --git a/DECISIONS.md b/DECISIONS.md index c2ec60e..94bfad3 100644 --- a/DECISIONS.md +++ b/DECISIONS.md @@ -61,4 +61,7 @@ - 2026-02-27 m+git@andri.dk — Static files served from `/static/*` using an embedded file map in `portal.go`. Files include `shared.css` and `shared.js`. 5-minute cache with `Cache-Control: public, max-age=300`. - 2026-02-27 m+git@andri.dk — Mobile device support (iOS/Android). iOS and Android do not support the `HTTPS` PAC proxy type, so the HTTP port (8080) now also accepts CONNECT tunnels and HTTP forwarding for mobile clients. A separate `/mobile.pac` file returns `PROXY host:port` (plain HTTP) instead of `HTTPS host:port`. The bootstrap script (element picker + session token) is not injected on plain HTTP connections to prevent leaking the session token over unencrypted traffic — detection uses `r.TLS == nil`. CSS element hiding and resource stripping still apply normally. Added `github.com/skip2/go-qrcode` (pure Go, zero transitive runtime deps) for a QR code on the setup page linking to the HTTP setup URL. - 2026-02-28 m+git@andri.dk — PAC files bypass captive portal / connectivity check domains (`captive.apple.com`, `connectivitycheck.gstatic.com`, `connectivitycheck.android.com`, `clients3.google.com`, `www.msftconnecttest.com`, `dns.msftncsi.com`, `detectportal.firefox.com`). These domains return `DIRECT` so the OS can verify internet connectivity after wifi connects. Without this, the proxy (or its blocklists) could interfere with the expected responses, causing the OS to show a captive portal page or report "no internet". +- 2026-02-28 m+git@andri.dk — Removed 18 redundant tests (~320 lines net). Lines of code are a liability. Four strict subsets (e.g. `TestIframeStrippingWithScriptStripping` subsumed by `TestAllBlockableElementsTogether`), five HTTPS transport duplicates consolidated into one `TestHTTPSHTMLRewriting` (the rewriting logic is transport-independent), five unit/integration overlaps where both layers tested identical behavior, and four low-value tests (CSS on void elements, trivial byte comparison, etc.). +- 2026-02-28 m+git@andri.dk — Moved `pkg/` to `internal/`. This is an application, not a library — `internal/` enforces that `blocklist`, `store`, and `webauthn` packages can't be imported by external code. +- 2026-02-28 m+git@andri.dk — Extracted `ca.go` + `cert.go` into `internal/ca/` and `mobileconfig.go` into `internal/mobileconfig/`. These had zero dependencies on any `main`-package type — genuinely independent domain concepts (CA management, cert caching, Apple profile generation). Decided against extracting API code (`api*.go`) into `internal/api/` — the API handlers are deeply interleaved with the proxy (sessions, activity log, rule reload callbacks) and would require plumbing-only packages to break import cycles. File-level separation (`api_*.go` prefix) is sufficient for a single-binary proxy. - 2026-02-28 m+git@andri.dk — GitHub Actions CI/CD. `ci.yml` runs `go test ./...` on pull requests. `release.yml` runs on every push to main: tests, cross-compiles binaries for linux/amd64, linux/arm64, darwin/amd64, darwin/arm64, creates a GitHub Release (prerelease for commits, full release for `v*` tags), and builds+pushes multi-arch Docker images to `ghcr.io`. Version embedded via `-ldflags "-X main.version=..."` — tag name for tagged builds, short SHA for trunk builds. `ublproxy --version` now works. diff --git a/api.go b/api.go index cc4079a..9e0bf6c 100644 --- a/api.go +++ b/api.go @@ -7,8 +7,8 @@ import ( "sync" "time" - "ublproxy/pkg/store" - "ublproxy/pkg/webauthn" + "ublproxy/internal/store" + "ublproxy/internal/webauthn" ) const ( diff --git a/api_auth.go b/api_auth.go index 20a4eee..18a7c5a 100644 --- a/api_auth.go +++ b/api_auth.go @@ -4,7 +4,7 @@ import ( "encoding/base64" "net/http" - "ublproxy/pkg/webauthn" + "ublproxy/internal/webauthn" ) func (a *apiHandler) routeAuth(w http.ResponseWriter, r *http.Request, path string) { diff --git a/api_rules.go b/api_rules.go index 565da4e..c55f1c7 100644 --- a/api_rules.go +++ b/api_rules.go @@ -7,7 +7,7 @@ import ( "strconv" "strings" - "ublproxy/pkg/store" + "ublproxy/internal/store" ) func (a *apiHandler) routeRules(w http.ResponseWriter, r *http.Request, path string, sess *store.Session) { diff --git a/api_subscriptions.go b/api_subscriptions.go index 34349f0..48790e3 100644 --- a/api_subscriptions.go +++ b/api_subscriptions.go @@ -5,7 +5,7 @@ import ( "strconv" "strings" - "ublproxy/pkg/store" + "ublproxy/internal/store" ) func (a *apiHandler) routeSubscriptions(w http.ResponseWriter, r *http.Request, path string, sess *store.Session) { diff --git a/api_test.go b/api_test.go index 1712da6..f91312a 100644 --- a/api_test.go +++ b/api_test.go @@ -17,8 +17,8 @@ import ( "github.com/fxamacker/cbor/v2" - "ublproxy/pkg/store" - "ublproxy/pkg/webauthn" + "ublproxy/internal/store" + "ublproxy/internal/webauthn" ) var testAPIConfig = webauthn.Config{ diff --git a/connect.go b/connect.go index 3b6e4f5..7a1d3bc 100644 --- a/connect.go +++ b/connect.go @@ -49,7 +49,7 @@ func (p *proxyHandler) handleConnect(w http.ResponseWriter, r *http.Request) { } defer clientConn.Close() - tlsCert, err := p.certs.getCert(host) + tlsCert, err := p.certs.GetCert(host) if err != nil { logError("connect/cert", err) return diff --git a/elemhide_inject.go b/elemhide_inject.go index 93e0712..d710985 100644 --- a/elemhide_inject.go +++ b/elemhide_inject.go @@ -12,7 +12,7 @@ import ( "github.com/andybalholm/brotli" "golang.org/x/net/html" - "ublproxy/pkg/blocklist" + "ublproxy/internal/blocklist" ) // styleCloseRe matches +var tmpl = template.Must(template.New("mobileconfig").Parse(` @@ -71,7 +71,7 @@ var mobileconfigTmpl = template.Must(template.New("mobileconfig").Parse(` `)) -type mobileconfigData struct { +type data struct { CACertDER string // base64-encoded DER certificate PACURL string ProfileUUID string @@ -91,8 +91,8 @@ func deterministicUUID(caCert *x509.Certificate, namespace string) string { sum[0:4], sum[4:6], sum[6:8], sum[8:10], sum[10:16]) } -func serveMobileconfig(w http.ResponseWriter, caCert *x509.Certificate, pacURL string) { - data := mobileconfigData{ +func Serve(w http.ResponseWriter, caCert *x509.Certificate, pacURL string) { + d := data{ CACertDER: base64.StdEncoding.EncodeToString(caCert.Raw), PACURL: pacURL, ProfileUUID: deterministicUUID(caCert, "profile"), @@ -103,5 +103,5 @@ func serveMobileconfig(w http.ResponseWriter, caCert *x509.Certificate, pacURL s w.Header().Set("Content-Type", "application/x-apple-asix-config") w.Header().Set("Content-Disposition", `attachment; filename="ublproxy.mobileconfig"`) w.WriteHeader(http.StatusOK) - mobileconfigTmpl.Execute(w, data) + tmpl.Execute(w, d) } diff --git a/pkg/store/cache.go b/internal/store/cache.go similarity index 100% rename from pkg/store/cache.go rename to internal/store/cache.go diff --git a/pkg/store/credentials.go b/internal/store/credentials.go similarity index 100% rename from pkg/store/credentials.go rename to internal/store/credentials.go diff --git a/pkg/store/rules.go b/internal/store/rules.go similarity index 100% rename from pkg/store/rules.go rename to internal/store/rules.go diff --git a/pkg/store/sessions.go b/internal/store/sessions.go similarity index 100% rename from pkg/store/sessions.go rename to internal/store/sessions.go diff --git a/pkg/store/store.go b/internal/store/store.go similarity index 100% rename from pkg/store/store.go rename to internal/store/store.go diff --git a/pkg/store/store_test.go b/internal/store/store_test.go similarity index 100% rename from pkg/store/store_test.go rename to internal/store/store_test.go diff --git a/pkg/store/subscriptions.go b/internal/store/subscriptions.go similarity index 100% rename from pkg/store/subscriptions.go rename to internal/store/subscriptions.go diff --git a/pkg/webauthn/json.go b/internal/webauthn/json.go similarity index 100% rename from pkg/webauthn/json.go rename to internal/webauthn/json.go diff --git a/pkg/webauthn/webauthn.go b/internal/webauthn/webauthn.go similarity index 100% rename from pkg/webauthn/webauthn.go rename to internal/webauthn/webauthn.go diff --git a/pkg/webauthn/webauthn_test.go b/internal/webauthn/webauthn_test.go similarity index 96% rename from pkg/webauthn/webauthn_test.go rename to internal/webauthn/webauthn_test.go index 429b8ab..8cc8830 100644 --- a/pkg/webauthn/webauthn_test.go +++ b/internal/webauthn/webauthn_test.go @@ -341,18 +341,5 @@ func TestCredentialIDBase64(t *testing.T) { } } -// Ensure bytesEqual works correctly. -func TestBytesEqual(t *testing.T) { - if !bytesEqual([]byte{1, 2, 3}, []byte{1, 2, 3}) { - t.Error("equal slices should be equal") - } - if bytesEqual([]byte{1, 2, 3}, []byte{1, 2, 4}) { - t.Error("different slices should not be equal") - } - if bytesEqual([]byte{1, 2}, []byte{1, 2, 3}) { - t.Error("different length slices should not be equal") - } -} - // Suppress unused import warning for big package. var _ = new(big.Int) diff --git a/main.go b/main.go index 18e1456..b97cf00 100644 --- a/main.go +++ b/main.go @@ -10,8 +10,9 @@ import ( "github.com/urfave/cli/v3" - "ublproxy/pkg/store" - "ublproxy/pkg/webauthn" + "ublproxy/internal/ca" + "ublproxy/internal/store" + "ublproxy/internal/webauthn" ) // version is set at build time via -ldflags "-X main.version=..." @@ -91,13 +92,13 @@ func run(_ context.Context, cmd *cli.Command) error { dbPath := cmd.String("db") blocklistSources := cmd.StringSlice("blocklist") - caCert, caKey, err := loadOrGenerateCA(caDir) + caCert, caKey, err := ca.LoadOrGenerate(caDir) if err != nil { return fmt.Errorf("CA setup failed: %w", err) } - certs := newCertCache(caCert, caKey) - caCertPEM := encodeCertPEM(caCert) + certs := ca.NewCache(caCert, caKey) + caCertPEM := ca.EncodeCertPEM(caCert) handler := newProxyHandler(certs, caCertPEM) activityLog := NewActivityLog(1000) handler.activityLog = activityLog diff --git a/portal.go b/portal.go index 30ffe47..777f10c 100644 --- a/portal.go +++ b/portal.go @@ -9,6 +9,8 @@ import ( "net/url" qrcode "github.com/skip2/go-qrcode" + + "ublproxy/internal/mobileconfig" ) //go:embed static/portal.html @@ -89,7 +91,7 @@ func (s *setupHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { } if r.URL.Path == "/ublproxy.mobileconfig" { pacURL := s.httpOrigin + "/mobile.pac" - serveMobileconfig(w, s.caCert, pacURL) + mobileconfig.Serve(w, s.caCert, pacURL) return } if r.URL.Path == "/proxy.pac" { @@ -318,7 +320,7 @@ func (p *proxyHandler) handleSetup(w http.ResponseWriter, r *http.Request) { func (p *proxyHandler) handleMobileconfig(w http.ResponseWriter, r *http.Request) { pacURL := p.httpOrigin + "/mobile.pac" - serveMobileconfig(w, p.certs.caCert, pacURL) + mobileconfig.Serve(w, p.certs.CACert, pacURL) } func (p *proxyHandler) handlePortalCACert(w http.ResponseWriter, r *http.Request) { diff --git a/portal_https.go b/portal_https.go index 95fbb19..b5aaedf 100644 --- a/portal_https.go +++ b/portal_https.go @@ -6,6 +6,8 @@ import ( "net" "net/http" "os" + + "ublproxy/internal/ca" ) // portalHandler routes requests on the HTTPS port. It serves the proxy @@ -85,8 +87,8 @@ func (h *portalHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { // startPortalHTTPS starts the HTTPS server that handles both proxy // traffic and the management portal. HTTP/2 is disabled because // CONNECT tunnels require Hijack which only works with HTTP/1.1. -func startPortalHTTPS(listenAddr string, host string, extraIPs []net.IP, certs *certCache, handler *portalHandler) { - cert, err := certs.portalCert(host, extraIPs...) +func startPortalHTTPS(listenAddr string, host string, extraIPs []net.IP, certs *ca.Cache, handler *portalHandler) { + cert, err := certs.PortalCert(host, extraIPs...) if err != nil { fmt.Fprintf(os.Stderr, "portal: failed to generate TLS cert: %v\n", err) os.Exit(1) diff --git a/proxy.go b/proxy.go index 9ca5b52..f144378 100644 --- a/proxy.go +++ b/proxy.go @@ -11,12 +11,13 @@ import ( "sync/atomic" "time" - "ublproxy/pkg/blocklist" - "ublproxy/pkg/store" + "ublproxy/internal/blocklist" + "ublproxy/internal/ca" + "ublproxy/internal/store" ) type proxyHandler struct { - certs *certCache + certs *ca.Cache caCertPEM []byte transport *http.Transport store *store.Store @@ -47,7 +48,7 @@ type proxyHandler struct { reloadMu sync.Mutex } -func newProxyHandler(certs *certCache, caCertPEM []byte) *proxyHandler { +func newProxyHandler(certs *ca.Cache, caCertPEM []byte) *proxyHandler { return &proxyHandler{ certs: certs, caCertPEM: caCertPEM, diff --git a/proxy_test.go b/proxy_test.go index cf9357e..c760026 100644 --- a/proxy_test.go +++ b/proxy_test.go @@ -25,8 +25,9 @@ import ( "github.com/andybalholm/brotli" - "ublproxy/pkg/blocklist" - "ublproxy/pkg/store" + "ublproxy/internal/blocklist" + "ublproxy/internal/ca" + "ublproxy/internal/store" ) // testEnv holds everything needed to run a proxy e2e test. @@ -50,13 +51,13 @@ func startTestEnv(t *testing.T, upstreamHandler http.Handler, rules *blocklist.R httpsServer := httptest.NewTLSServer(upstreamHandler) // In-memory CA - caCert, caKey, err := generateCA() + caCert, caKey, err := ca.Generate() if err != nil { - t.Fatalf("generateCA: %v", err) + t.Fatalf("ca.Generate: %v", err) } - certs := newCertCache(caCert, caKey) - caCertPEM := encodeCertPEM(caCert) + certs := ca.NewCache(caCert, caKey) + caCertPEM := ca.EncodeCertPEM(caCert) handler := newProxyHandler(certs, caCertPEM) if rules != nil { handler.baselineRules.Store(rules) @@ -229,35 +230,45 @@ func TestHTTPSProxy(t *testing.T) { } } -func TestHTTPSProxyHeaders(t *testing.T) { - var receivedHeaders http.Header +func TestHTTPSHTMLRewriting(t *testing.T) { + rs := blocklist.NewRuleSet() + rs.AddLine("||ads.tracker.com^") + rs.AddLine("##.ad-banner") - env := startTestEnv(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - receivedHeaders = r.Header.Clone() - w.Header().Set("X-Upstream-Secure", "yes") + upstream := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "text/html; charset=utf-8") w.WriteHeader(http.StatusOK) - }), nil) + w.Write([]byte(`` + + `` + + `` + + `
Ad
` + + `

Content

` + + ``)) + }) + env := startTestEnv(t, upstream, rs) client := env.httpClient(t) - req, err := http.NewRequest("GET", env.httpsURL+"/secure-headers", nil) - if err != nil { - t.Fatalf("new request: %v", err) - } - req.Header.Set("X-Custom-Secure", "secure-value") - - resp, err := client.Do(req) + resp, err := client.Get(env.httpsURL + "/page.html") if err != nil { t.Fatalf("GET: %v", err) } defer resp.Body.Close() - if got := receivedHeaders.Get("X-Custom-Secure"); got != "secure-value" { - t.Errorf("upstream X-Custom-Secure = %q, want %q", got, "secure-value") - } + body, _ := io.ReadAll(resp.Body) + bodyStr := string(body) - if got := resp.Header.Get("X-Upstream-Secure"); got != "yes" { - t.Errorf("response X-Upstream-Secure = %q, want %q", got, "yes") + if !strings.Contains(bodyStr, "