fix: harden APT overlay precedence

2026-07-17 03:38:42 +08:00
parent 43b7537955
commit b36a585b3e
2 changed files with 122 additions and 23 deletions
@@ -272,12 +272,9 @@ func translate(input profile.Profile) (translation, error) {
 			return translation{}, fmt.Errorf("APT cannot translate %s value %q: proxy scheme %q is incompatible", candidate.field, profile.ObscureURL(candidate.value), parsed.Scheme)
 		}
 	}
-	settings := make(map[string]string)
-	if httpValue != "" {
-		settings["Acquire::http::Proxy"] = httpValue
-	}
-	if httpsValue != "" {
-		settings["Acquire::https::Proxy"] = httpsValue
+	settings := map[string]string{
+		"Acquire::http::Proxy":  valueOrDirect(httpValue),
+		"Acquire::https::Proxy": valueOrDirect(httpsValue),
 	}
 	for _, bypass := range p.Bypass {
 		if strings.HasPrefix(bypass, ".") || strings.Contains(bypass, "/") || strings.Contains(bypass, ":") {
@@ -292,6 +289,13 @@ func translate(input profile.Profile) (translation, error) {
 	return translation{settings: settings, rendered: render(settings)}, nil
 }

+func valueOrDirect(value string) string {
+	if value == "" {
+		return "DIRECT"
+	}
+	return value
+}
+
 var settingOrder = []string{"Acquire::http::Proxy", "Acquire::https::Proxy"}

 func render(settings map[string]string) []byte {
@@ -336,7 +340,8 @@ func parseConfiguration(data []byte) map[string]string {
 		if !ok {
 			key, rest, ok = strings.Cut(line, "\t")
 		}
-		if !ok || !recognizedKey(key) {
+		key, recognized := canonicalKey(key)
+		if !ok || !recognized {
 			continue
 		}
 		rest = strings.TrimSpace(rest)
@@ -352,9 +357,22 @@ func parseConfiguration(data []byte) map[string]string {
 	return settings
 }

-func recognizedKey(key string) bool {
-	return key == "Acquire::http::Proxy" || key == "Acquire::https::Proxy" ||
-		strings.HasPrefix(key, "Acquire::http::Proxy::") || strings.HasPrefix(key, "Acquire::https::Proxy::")
+func canonicalKey(key string) (string, bool) {
+	parts := strings.Split(key, "::")
+	if len(parts) < 3 || !strings.EqualFold(parts[0], "Acquire") ||
+		(!strings.EqualFold(parts[1], "http") && !strings.EqualFold(parts[1], "https")) ||
+		!strings.EqualFold(parts[2], "Proxy") {
+		return "", false
+	}
+	canonical := "Acquire::" + strings.ToLower(parts[1]) + "::Proxy"
+	if len(parts) == 3 {
+		return canonical, true
+	}
+	host := strings.Join(parts[3:], "::")
+	if host == "" {
+		return "", false
+	}
+	return canonical + "::" + strings.ToLower(host), true
 }

 func recognizedProxy(settings map[string]string) (string, bool) {
@@ -16,14 +16,15 @@ import (

 func TestAvailabilityRequiresDebianMarkerAPTToolAndConfigurationDirectory(t *testing.T) {
 	for _, tc := range []struct {
-		name              string
-		marker, tool, dir bool
-		want              bool
+		name                         string
+		marker, tool, dir, dumpWorks bool
+		want                         bool
 	}{
-		{"detected", true, true, true, true},
-		{"not Debian", false, true, true, false},
-		{"apt-config absent", true, false, true, false},
-		{"configuration absent", true, true, false, false},
+		{"detected", true, true, true, true, true},
+		{"not Debian", false, true, true, true, false},
+		{"apt-config absent", true, false, true, true, false},
+		{"configuration absent", true, true, false, true, false},
+		{"apt-config inspection fails", true, true, true, false, false},
 	} {
 		t.Run(tc.name, func(t *testing.T) {
 			root := t.TempDir()
@@ -35,7 +36,7 @@ func TestAvailabilityRequiresDebianMarkerAPTToolAndConfigurationDirectory(t *tes
 				mustMkdir(t, filepath.Dir(filepath.Join(root, "etc", "debian_version")))
 				mustWrite(t, filepath.Join(root, "etc", "debian_version"), "12\n")
 			}
-			runner := &fakeRunner{available: tc.tool}
+			runner := &fakeRunner{available: tc.tool, outputError: !tc.dumpWorks}
 			a := apt.New(apt.Configuration{DebianVersionFile: filepath.Join(root, "etc", "debian_version"), ConfigDirectory: configDir, OverlayFile: filepath.Join(configDir, "99proxyctl"), TrustedRoot: root}, nil, runner)
 			got := a.Availability(context.Background())
 			if got.Available != tc.want || got.Evidence == "" {
@@ -92,6 +93,62 @@ func TestAPTAdapterRunsReusableTargetContract(t *testing.T) {
 	})
 }

+func TestAPTConfigurationKeysAndHostOverridesAreCaseInsensitive(t *testing.T) {
+	a, runner := newAdapter(t, nil)
+	runner.external = "aCqUiRe::HtTp::pRoXy \"http://prior.example:3128\";\n" +
+		"ACQUIRE::HTTP::PROXY::SPECIAL.EXAMPLE \"http://special.example:3128\";"
+	if got := a.Inspect(context.Background(), compatibleProfile()); got.State != target.External {
+		t.Fatalf("mixed-case external inspection = %#v", got)
+	}
+	if got := a.SetDesired(context.Background(), true, compatibleProfile()); got.Err != nil || got.Inspection.State != target.Enabled {
+		t.Fatalf("enable = %#v", got)
+	}
+	managed := readFile(t, a.OverlayPath())
+	if !strings.Contains(managed, `Acquire::http::Proxy::special.example "http://alice:secret@proxy.example:8080";`) {
+		t.Fatalf("mixed-case host override was not masked case-insensitively:\n%s", managed)
+	}
+
+	mustWrite(t, a.OverlayPath(), strings.ReplaceAll(managed, "Acquire::http::Proxy", "ACQUIRE::HTTP::PROXY"))
+	if got := a.Inspect(context.Background(), compatibleProfile()); got.State != target.Enabled {
+		t.Fatalf("mixed-case managed inspection = %#v", got)
+	}
+}
+
+func TestMissingProtocolWritesDIRECTAndDisableRestoresUnderlyingConfiguration(t *testing.T) {
+	for _, tc := range []struct {
+		name, configuredKey, configuredURL, missingKey, priorURL string
+	}{
+		{"missing HTTPS", "Acquire::http::Proxy", "http://proxy.example:8080", "Acquire::https::Proxy", "https://prior.example:8443"},
+		{"missing HTTP", "Acquire::https::Proxy", "https://proxy.example:8443", "Acquire::http::Proxy", "http://prior.example:8080"},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			a, runner := newAdapter(t, nil)
+			runner.external = tc.missingKey + ` "` + tc.priorURL + `";`
+			p := &profile.Profile{Name: "partial", Mode: profile.AdvancedMode}
+			if strings.Contains(tc.configuredKey, "http::") {
+				p.HTTPURL = tc.configuredURL
+			} else {
+				p.HTTPSURL = tc.configuredURL
+			}
+
+			if got := a.SetDesired(context.Background(), true, p); got.Err != nil || got.Inspection.State != target.Enabled {
+				t.Fatalf("enable = %#v", got)
+			}
+			managed := readFile(t, a.OverlayPath())
+			if !strings.Contains(managed, tc.configuredKey+` "`+tc.configuredURL+`";`) ||
+				!strings.Contains(managed, tc.missingKey+` "DIRECT";`) {
+				t.Fatalf("overlay does not explicitly mask missing protocol:\n%s", managed)
+			}
+			if got := a.SetDesired(context.Background(), false, nil); got.Err != nil || got.Inspection.State != target.External {
+				t.Fatalf("disable = %#v", got)
+			}
+			if !strings.Contains(runner.external, tc.priorURL) {
+				t.Fatalf("underlying configuration changed: %q", runner.external)
+			}
+		})
+	}
+}
+
 func TestOwnedOverlayWinsAndDisableRevealsPriorEffectiveConfiguration(t *testing.T) {
 	a, runner := newAdapter(t, nil)
 	runner.external = "Acquire::http::Proxy \"http://prior.example:3128\";\n" +
@@ -144,6 +201,22 @@ func TestOverlayNameCollisionNeverRewritesOrRemovesUnownedConfiguration(t *testi
 	}
 }

+func TestDisablePermissionFailureReportsActualManagedState(t *testing.T) {
+	files := &failingFS{}
+	a, _ := newFailingAdapter(t, files)
+	if got := a.SetDesired(context.Background(), true, compatibleProfile()); got.Err != nil {
+		t.Fatalf("enable = %#v", got)
+	}
+	files.removePermission = true
+	got := a.SetDesired(context.Background(), false, nil)
+	if got.Err == nil || !errors.Is(got.Err, os.ErrPermission) || got.Inspection.State != target.Drifted {
+		t.Fatalf("permission-denied disable = %#v", got)
+	}
+	if _, err := os.Stat(a.OverlayPath()); err != nil {
+		t.Fatalf("managed overlay should remain after denied removal: %v", err)
+	}
+}
+
 func TestEnableReturnsFailureWhenManagedValuesDoNotBecomeEffective(t *testing.T) {
 	a, runner := newAdapter(t, nil)
 	runner.override = `Acquire::http::Proxy "http://later.example:8080";`
@@ -274,7 +347,7 @@ func newFailingAdapter(t *testing.T, files *failingFS) (*apt.Adapter, *fakeRunne
 }

 type fakeRunner struct {
-	available                                       bool
+	available, outputError                          bool
 	overlay, external, override, failDumpAfterWrite string
 }

@@ -285,6 +358,9 @@ func (r *fakeRunner) LookPath(string) (string, error) {
 	return "/usr/bin/apt-config", nil
 }
 func (r *fakeRunner) Output(context.Context, string, ...string) ([]byte, error) {
+	if r.outputError {
+		return nil, errors.New("apt-config dump failed")
+	}
 	if r.failDumpAfterWrite != "" {
 		if _, err := os.Stat(r.failDumpAfterWrite); err == nil {
 			return nil, errors.New("apt-config dump failed")
@@ -302,9 +378,9 @@ func (r *fakeRunner) Output(context.Context, string, ...string) ([]byte, error)
 }

 type failingFS struct {
-	delegate             apt.FileSystem
-	permission, failOnce bool
-	writeError           error
+	delegate                               apt.FileSystem
+	permission, failOnce, removePermission bool
+	writeError                             error
 }

 func (f *failingFS) ReadFile(path string) ([]byte, error) { return f.delegate.ReadFile(path) }
@@ -321,7 +397,12 @@ func (f *failingFS) AtomicWrite(path string, data []byte, mode os.FileMode) erro
 	}
 	return f.delegate.AtomicWrite(path, data, mode)
 }
-func (f *failingFS) Remove(path string) error { return f.delegate.Remove(path) }
+func (f *failingFS) Remove(path string) error {
+	if f.removePermission {
+		return os.ErrPermission
+	}
+	return f.delegate.Remove(path)
+}
 func mustMkdir(t *testing.T, path string) {
 	t.Helper()
 	if err := os.MkdirAll(path, 0o755); err != nil {