fix: Issue #9364 JSON config validation

config/config.go

@@ -7,6 +7,7 @@
 	"fmt"
 	"os"
 	"path/filepath"
+	"reflect"
 	"strings"
 
 	"github.com/ipfs/kubo/misc/fsutil"
@@ -137,6 +138,29 @@
 	return m, nil
 }
 
+// Convert config to a map, without using encoding/json.
+func ReflectToMap(conf interface{}) map[string]interface{} {
+	confmap := make(map[string]interface{})
+	rv := reflect.ValueOf(conf)
+	if rv.Kind() == reflect.Ptr {
+		rv = rv.Elem()
+	}
+
+	for i := 0; i < rv.NumField(); i++ {
+		field := rv.Field(i)
+		if field.CanInterface() {
+			if field.Kind() == reflect.Struct {
+				confmap[rv.Type().Field(i).Name] = ReflectToMap(field.Interface())
+			} else if field.Kind() == reflect.Map {
+				confmap[rv.Type().Field(i).Name] = "map"
+			} else {
+				confmap[rv.Type().Field(i).Name] = field.Interface()
+			}
+		}
+	}
+	return confmap
+}
+
 // Clone copies the config. Use when updating.
 func (c *Config) Clone() (*Config, error) {
 	var newConfig Config
@@ -152,3 +176,42 @@
 
 	return &newConfig, nil
 }
+
+// Check if the provided key is present in the structure.
+func CheckKey(key string) error {
+	conf := Config{}
+
+	// Convert an empty config to a map without JSON.
+	confmap := ReflectToMap(&conf)
+
+	// Parse the key and verify it's presence in the map.
+	var ok bool
+	var mcursor map[string]interface{}
+	var cursor interface{} = confmap
+
+	parts := strings.Split(key, ".")
+	for i, part := range parts {
+		sofar := strings.Join(parts[:i], ".")
+
+		mcursor, ok = cursor.(map[string]interface{})
+		if !ok {
+			s, ok := cursor.(string)
+			if ok && s == "map" {
+				return nil
+			}
+			return fmt.Errorf("%s key is not a map", sofar)
+		}
+
+		cursor, ok = mcursor[part]
+		if !ok {
+			// Construct the current path traversed to print a nice error message
+			var path string
+			if len(sofar) > 0 {
+				path += sofar + "."
+			}
+			path += part
+			return fmt.Errorf("%s not found", path)
+		}
+	}
+	return nil
+}

docs/changelogs/v0.34.md

@@ -1,19 +1,23 @@
 # Kubo changelog v0.34
 
-- [v0.34.0](#v0310)
+- [v0.34.0](#v0340)
 
 ## v0.34.0
 
 - [Overview](#overview)
 - [🔦 Highlights](#-highlights)
+  - [JSON config validation](#json-config-validation)
 - [📝 Changelog](#-changelog)
 - [👨‍👩‍👧‍👦 Contributors](#-contributors)
 
 ### Overview
 
 ### 🔦 Highlights
 
+#### JSON config validation
+
+`ipfs config` is now validating json fields ([#10679](https://github.com/ipfs/kubo/pull/10679)).
+
 ### 📝 Changelog
 
 ### 👨‍👩‍👧‍👦 Contributors
-

repo/fsrepo/fsrepo.go

@@ -676,6 +676,12 @@
 		return errors.New("repo is closed")
 	}
 
+	// Validate the key's presence in the config structure.
+	err := config.CheckKey(key)
+	if err != nil {
+		return err
+	}
+
 	// Load into a map so we don't end up writing any additional defaults to the config file.
 	var mapconf map[string]interface{}
 	if err := serialize.ReadConfigFile(r.configFilePath, &mapconf); err != nil {

test/sharness/t0002-docker-image.sh

@@ -36,8 +36,8 @@ test_expect_success "docker image build succeeds" '
 '
 
 test_expect_success "write init scripts" '
-  echo "ipfs config Foo Bar" > 001.sh &&
-  echo "ipfs config Baz Qux" > 002.sh &&
+  echo "ipfs config Provider.Strategy Bar" > 001.sh &&
+  echo "ipfs config Datastore.Path Qux" > 002.sh &&
   chmod +x 002.sh
 '
 
@@ -65,10 +65,10 @@ test_expect_success "check that init scripts were run correctly and in the corre
 
 test_expect_success "check that init script configs were applied" '
   echo Bar > expected &&
-  docker exec "$DOC_ID" ipfs config Foo > actual &&
+  docker exec "$DOC_ID" ipfs config Provider.Strategy > actual &&
   test_cmp actual expected &&
   echo Qux > expected &&
-  docker exec "$DOC_ID" ipfs config Baz > actual &&
+  docker exec "$DOC_ID" ipfs config Datastore.Path > actual &&
   test_cmp actual expected
 '
 

test/sharness/t0021-config.sh

@@ -13,41 +13,17 @@ test_config_cmd_set() {
 
   cfg_key=$1
   cfg_val=$2
-  test_expect_success "ipfs config succeeds" '
-    ipfs config $cfg_flags "$cfg_key" "$cfg_val"
-  '
-
-  test_expect_success "ipfs config output looks good" '
-    echo "$cfg_val" >expected &&
-    ipfs config "$cfg_key" >actual &&
-    test_cmp expected actual
-  '
+  test_expect_success "ipfs config succeeds" "
+    ipfs config $cfg_flags \"$cfg_key\" \"$cfg_val\"
+  "
 
-  # also test our lib function. it should work too.
-  cfg_key="Lib.$cfg_key"
-  test_expect_success "test_config_set succeeds" '
-    test_config_set $cfg_flags "$cfg_key" "$cfg_val"
-  '
-
-  test_expect_success "test_config_set value looks good" '
-    echo "$cfg_val" >expected &&
-    ipfs config "$cfg_key" >actual &&
+  test_expect_success "ipfs config output looks good" "
+    echo \"$cfg_val\" >expected &&
+    ipfs config \"$cfg_key\" >actual &&
     test_cmp expected actual
-  '
+  "
 }
 
-# this is a bit brittle. the problem is we need to test
-# with something that will be forced to unmarshal as a struct.
-# (i.e. just setting 'ipfs config --json foo "[1, 2, 3]"') may
-# set it as astring instead of proper json. We leverage the
-# unmarshalling that has to happen.
-CONFIG_SET_JSON_TEST='{
-  "MDNS": {
-    "Enabled": true,
-    "Interval": 10
-  }
-}'
-
 test_profile_apply_revert() {
   profile=$1
   inverse_profile=$2
@@ -87,27 +63,26 @@ test_profile_apply_dry_run_not_alter() {
 }
 
 test_config_cmd() {
-  test_config_cmd_set "beep" "boop"
-  test_config_cmd_set "beep1" "boop2"
-  test_config_cmd_set "beep1" "boop2"
-  test_config_cmd_set "--bool" "beep2" "true"
-  test_config_cmd_set "--bool" "beep2" "false"
-  test_config_cmd_set "--json" "beep3" "true"
-  test_config_cmd_set "--json" "beep3" "false"
-  test_config_cmd_set "--json" "Discovery" "$CONFIG_SET_JSON_TEST"
-  test_config_cmd_set "--json" "deep-not-defined.prop" "true"
-  test_config_cmd_set "--json" "deep-null" "null"
-  test_config_cmd_set "--json" "deep-null.prop" "true"
+  test_config_cmd_set "Addresses.API" "foo"
+  test_config_cmd_set "Addresses.Gateway" "bar"
+  test_config_cmd_set "Datastore.GCPeriod" "baz"
+  test_config_cmd_set "--bool" "Discovery.MDNS.Enabled" "true"
+  test_config_cmd_set "--bool" "Discovery.MDNS.Enabled" "false"
+  test_config_cmd_set "--json" "Datastore.HashOnRead" "true"
+  test_config_cmd_set "--json" "Datastore.HashOnRead" "false"
+  test_config_cmd_set "--json" "Experimental.FilestoreEnabled" "true"
+  test_config_cmd_set "--json" "Import.BatchMaxSize" "null"
+  test_config_cmd_set "--json" "Import.UnixFSRawLeaves" "true"
 
   test_expect_success "'ipfs config show' works" '
     ipfs config show >actual
   '
 
   test_expect_success "'ipfs config show' output looks good" '
-    grep "\"beep\": \"boop\"," actual &&
-    grep "\"beep1\": \"boop2\"," actual &&
-    grep "\"beep2\": false," actual &&
-    grep "\"beep3\": false," actual
+    grep "\"API\": \"foo\"," actual &&
+    grep "\"Gateway\": \"bar\"" actual &&
+    grep "\"Enabled\": false" actual &&
+    grep "\"HashOnRead\": false" actual
   '
 
   test_expect_success "'ipfs config show --config-file' works" '

test/sharness/t0070-user-config.sh

@@ -11,10 +11,12 @@ test_description="Test user-provided config values"
 test_init_ipfs
 
 test_expect_success "bootstrap doesn't overwrite user-provided config keys (top-level)" '
-  ipfs config Foo.Bar baz &&
+  ipfs config Provider.Strategy >previous &&
+  ipfs config Provider.Strategy foo &&
   ipfs bootstrap rm --all &&
-  echo "baz" >expected &&
-  ipfs config Foo.Bar >actual &&
+  echo "foo" >expected &&
+  ipfs config Provider.Strategy >actual &&
+  ipfs config Provider.Strategy $(cat previous) &&
   test_cmp expected actual
 '