Validate configured hosts before use

Read complete environment credentials without touching the config
directory. Reject non-HTTP hosts from environment variables or
stored profiles before request setup.

Author: myabc

components/configuration/hardening_test.go

@@ -99,3 +99,66 @@ func TestReadConfig_rejectsInvalidHost(t *testing.T) {
 		t.Errorf("error should mention invalid host, got: %v", err)
 	}
 }
+
+func TestReadConfig_EnvironmentDoesNotRequireConfigDirectory(t *testing.T) {
+	configRoot := filepath.Join(t.TempDir(), "not-a-directory")
+	if err := os.WriteFile(configRoot, []byte("occupied"), 0600); err != nil {
+		t.Fatal(err)
+	}
+	t.Setenv("XDG_CONFIG_HOME", configRoot)
+	t.Setenv("OP_CLI_HOST", "https://env.example.com")
+	t.Setenv("OP_CLI_TOKEN", "env-token")
+
+	host, token, err := configuration.ReadConfig("default")
+	if err != nil {
+		t.Fatalf("ReadConfig with environment credentials: %v", err)
+	}
+	if host != "https://env.example.com" || token != "env-token" {
+		t.Errorf("got host %q token %q", host, token)
+	}
+}
+
+func TestReadConfig_rejectsUnsupportedEnvironmentScheme(t *testing.T) {
+	setupTempConfig(t)
+	t.Setenv("OP_CLI_HOST", "ftp://env.example.com")
+	t.Setenv("OP_CLI_TOKEN", "env-token")
+
+	_, _, err := configuration.ReadConfig("default")
+	if err == nil {
+		t.Fatal("expected error for unsupported environment host scheme")
+	}
+	if !strings.Contains(err.Error(), "only http and https") {
+		t.Errorf("error should describe supported schemes, got: %v", err)
+	}
+	if !strings.Contains(err.Error(), "OP_CLI_HOST") {
+		t.Errorf("error should name OP_CLI_HOST as the source, got: %v", err)
+	}
+}
+
+// Scheme comparison must be case-insensitive: URL schemes are defined as
+// case-insensitive, and hosts pasted from other tools may be upper-cased.
+func TestReadConfig_acceptsMixedCaseScheme(t *testing.T) {
+	setupTempConfig(t)
+	writeRaw(t, "[default]\nhost = HTTPS://file.example.com\ntoken = file-token\n")
+
+	host, token, err := configuration.ReadConfig("default")
+	if err != nil {
+		t.Fatalf("ReadConfig with mixed-case scheme: %v", err)
+	}
+	if host != "HTTPS://file.example.com" || token != "file-token" {
+		t.Errorf("got host %q token %q", host, token)
+	}
+}
+
+func TestReadConfig_rejectsUnsupportedProfileScheme(t *testing.T) {
+	setupTempConfig(t)
+	writeRaw(t, "[default]\nhost = ftp://file.example.com\ntoken = file-token\n")
+
+	_, _, err := configuration.ReadConfig("default")
+	if err == nil {
+		t.Fatal("expected error for unsupported profile host scheme")
+	}
+	if !strings.Contains(err.Error(), "only http and https") {
+		t.Errorf("error should describe supported schemes, got: %v", err)
+	}
+}

components/configuration/profiles.go

@@ -58,15 +58,21 @@ func ValidateProfileName(name string) error {
 	return nil
 }
 
-// ReadConfig returns host and token for profile.
-// OP_CLI_HOST and OP_CLI_TOKEN always take precedence over the file.
+// ReadConfig returns the host and token for profile. Environment credentials
+// take precedence and do not require the config directory to be accessible.
 func ReadConfig(profile string) (host, token string, err error) {
-	if err = ensureConfigDir(); err != nil {
-		return "", "", err
-	}
 	if ok, h, t := readEnvironment(); ok {
+		if err := validateHost(h); err != nil {
+			return "", "", errors.Custom(fmt.Sprintf(
+				"environment variable OP_CLI_HOST has an invalid host %q: %v",
+				h, err,
+			))
+		}
 		return h, t, nil
 	}
+	if err = ensureConfigDir(); err != nil {
+		return "", "", err
+	}
 	return readConfigForProfile(profile)
 }
 
@@ -245,12 +251,26 @@ func (f *iniFile) marshal() []byte {
 	return []byte(sb.String())
 }
 
-// looksLikeHost reports whether s parses as an absolute URL with a scheme and
-// host. Used to distinguish a genuine old-format "host token" line from a
-// corrupt file, so garbage is not silently migrated into bogus credentials.
+// looksLikeHost reports whether s is an absolute HTTP(S) URL. It distinguishes
+// the legacy "host token" format from corrupt configuration content.
 func looksLikeHost(s string) bool {
+	return validateHost(s) == nil
+}
+
+// validateHost rejects relative URLs and schemes the HTTP client cannot use.
+func validateHost(s string) error {
 	u, err := url.Parse(s)
-	return err == nil && u.Scheme != "" && u.Host != ""
+	if err != nil {
+		return fmt.Errorf("invalid host URL: %w", err)
+	}
+	if u.Scheme == "" || u.Host == "" {
+		return fmt.Errorf("expected an absolute URL like https://example.openproject.com")
+	}
+	scheme := strings.ToLower(u.Scheme)
+	if scheme != "http" && scheme != "https" {
+		return fmt.Errorf("unsupported protocol scheme %q: only http and https are allowed", u.Scheme)
+	}
+	return nil
 }
 
 // readOrMigrate reads the config file and migrates it from the old
@@ -360,14 +380,13 @@ func readConfigForProfile(profile string) (host, token string, err error) {
 	if f.hasSection(profile) && (host == "" || token == "") {
 		return "", "", invalidConfigError()
 	}
-	// Reject junk hosts here with a clear message; url.Parse alone accepts
-	// almost anything as a relative URL, which would surface later as a
-	// confusing request failure.
-	if host != "" && !looksLikeHost(host) {
-		return "", "", errors.Custom(fmt.Sprintf(
-			"profile %q has an invalid host %q: expected an absolute URL like https://example.openproject.com",
-			profile, host,
-		))
+	if host != "" {
+		if err := validateHost(host); err != nil {
+			return "", "", errors.Custom(fmt.Sprintf(
+				"profile %q has an invalid host %q: %v",
+				profile, host, err,
+			))
+		}
 	}
 	return host, token, nil
 }