Skip to content

Commit e7c8dfd

Browse files
authored
fix: harden GetRequiredCredentialFields for Settings UI (#57)
- Filter non-credential JSON keys (URLs, flags, defaults) - Stable sort field names for deterministic forms - Extract credentialFieldsFromConfig for unit tests - Settings home save/load test under ~/.wisp Closes #56
1 parent 0f75ff8 commit e7c8dfd

3 files changed

Lines changed: 154 additions & 4 deletions

File tree

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
package connectors
2+
3+
import (
4+
"testing"
5+
6+
"github.com/wisp-trading/sdk/pkg/types/connector"
7+
)
8+
9+
// sampleConfig mirrors exchange configs: required keys have no omitempty;
10+
// optional URL/flags use omitempty so zero-value JSON omits them.
11+
type sampleConfig struct {
12+
APIKey string `json:"api_key"`
13+
APISecret string `json:"api_secret"`
14+
BaseURL string `json:"base_url,omitempty"`
15+
UseTest bool `json:"use_testnet,omitempty"`
16+
Slippage float64 `json:"default_slippage,omitempty"`
17+
}
18+
19+
func (sampleConfig) ExchangeName() connector.ExchangeName { return "sample" }
20+
func (sampleConfig) Validate() error { return nil }
21+
22+
func TestCredentialFieldsFromConfig_RequiredOnly(t *testing.T) {
23+
fields := credentialFieldsFromConfig(sampleConfig{})
24+
want := []string{"api_key", "api_secret"}
25+
if len(fields) != len(want) {
26+
t.Fatalf("fields=%v want %v", fields, want)
27+
}
28+
for i := range want {
29+
if fields[i] != want[i] {
30+
t.Fatalf("fields=%v want %v", fields, want)
31+
}
32+
}
33+
}
34+
35+
type noisyConfig struct {
36+
Key string `json:"private_key"`
37+
Network string `json:"network"`
38+
Base string `json:"base_url"`
39+
}
40+
41+
func (noisyConfig) ExchangeName() connector.ExchangeName { return "noisy" }
42+
func (noisyConfig) Validate() error { return nil }
43+
44+
func TestCredentialFieldsFromConfig_FiltersNonCredentials(t *testing.T) {
45+
got := credentialFieldsFromConfig(noisyConfig{})
46+
if len(got) != 1 || got[0] != "private_key" {
47+
t.Fatalf("got %v, want [private_key]", got)
48+
}
49+
}
50+
51+
type sortedConfig struct {
52+
B string `json:"z_last"`
53+
A string `json:"a_first"`
54+
}
55+
56+
func (sortedConfig) ExchangeName() connector.ExchangeName { return "sorted" }
57+
func (sortedConfig) Validate() error { return nil }
58+
59+
func TestCredentialFieldsFromConfig_Sorted(t *testing.T) {
60+
got := credentialFieldsFromConfig(sortedConfig{})
61+
if len(got) != 2 || got[0] != "a_first" || got[1] != "z_last" {
62+
t.Fatalf("unsorted or wrong: %v", got)
63+
}
64+
}

‎pkg/config/settings/connectors/service.go‎

Lines changed: 29 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"encoding/json"
55
"errors"
66
"fmt"
7+
"sort"
78
"strconv"
89

910
"github.com/wisp-trading/sdk/pkg/types/config"
@@ -228,7 +229,24 @@ func (c *connectorService) GetConnectorConfigsForStrategy(exchangeNames []string
228229
return connectorConfigs, nil
229230
}
230231

231-
// GetRequiredCredentialFields returns the credential field names required by an exchange
232+
// nonCredentialJSONKeys are present on some exchange configs but are not user
233+
// secrets for the Settings form (URLs, flags, defaults). Discovery still uses
234+
// NewConfig() JSON; we only filter noise from the form field list.
235+
var nonCredentialJSONKeys = map[string]struct{}{
236+
"network": {},
237+
"use_testnet": {},
238+
"is_testnet": {},
239+
"base_url": {},
240+
"websocket_url": {},
241+
"default_slippage": {},
242+
"signature_type": {},
243+
"starknet_rpc": {},
244+
}
245+
246+
// GetRequiredCredentialFields returns credential field names for an exchange.
247+
// Source of truth: connector.NewConfig() zero-value JSON keys (same path as MapToSDKConfig).
248+
// Fields with json omitempty that are zero are omitted — those are optional.
249+
// Stable sort for deterministic TUI forms.
232250
func (c *connectorService) GetRequiredCredentialFields(exchangeName string) []string {
233251
conn, exists := c.connectorRegistry.Connector(connector.ExchangeName(exchangeName))
234252
if !exists {
@@ -240,6 +258,12 @@ func (c *connectorService) GetRequiredCredentialFields(exchangeName string) []st
240258
return []string{}
241259
}
242260

261+
return credentialFieldsFromConfig(cfg)
262+
}
263+
264+
// credentialFieldsFromConfig extracts form field names from a connector config.
265+
// Used by GetRequiredCredentialFields; kept package-level for unit tests without a full registry.
266+
func credentialFieldsFromConfig(cfg connector.Config) []string {
243267
configBytes, err := json.Marshal(cfg)
244268
if err != nil {
245269
return []string{}
@@ -252,10 +276,11 @@ func (c *connectorService) GetRequiredCredentialFields(exchangeName string) []st
252276

253277
fields := make([]string, 0, len(fieldMap))
254278
for key := range fieldMap {
255-
if key != "network" && key != "use_testnet" {
256-
fields = append(fields, key)
279+
if _, skip := nonCredentialJSONKeys[key]; skip {
280+
continue
257281
}
282+
fields = append(fields, key)
258283
}
259-
284+
sort.Strings(fields)
260285
return fields
261286
}
Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
package settings
2+
3+
import (
4+
"os"
5+
"path/filepath"
6+
"testing"
7+
8+
"github.com/wisp-trading/sdk/pkg/types/config"
9+
)
10+
11+
func TestSaveAndLoadConnectorsUnderHome(t *testing.T) {
12+
home := t.TempDir()
13+
t.Setenv("HOME", home)
14+
t.Setenv(config.EnvSettingsPath, "")
15+
16+
path, err := config.DefaultConnectorsPath()
17+
if err != nil {
18+
t.Fatal(err)
19+
}
20+
// Ensure we write to the resolved home path
21+
cfg := NewConfiguration(ConfigOptions{SettingsPath: path}).(*settings)
22+
23+
err = cfg.AddConnector(config.Connector{
24+
Name: "hyperliquid",
25+
Enabled: true,
26+
Credentials: map[string]string{
27+
"private_key": "0xabc",
28+
"account_address": "0xdef",
29+
},
30+
})
31+
if err != nil {
32+
t.Fatalf("AddConnector: %v", err)
33+
}
34+
35+
// Reload from disk
36+
cfg2 := NewConfiguration(ConfigOptions{SettingsPath: path}).(*settings)
37+
list, err := cfg2.GetConnectors()
38+
if err != nil {
39+
t.Fatal(err)
40+
}
41+
if len(list) != 1 || list[0].Name != "hyperliquid" {
42+
t.Fatalf("list=%v", list)
43+
}
44+
if list[0].Credentials["private_key"] != "0xabc" {
45+
t.Fatalf("credentials not loaded")
46+
}
47+
48+
// File mode should be restrictive
49+
info, err := os.Stat(path)
50+
if err != nil {
51+
t.Fatal(err)
52+
}
53+
if info.Mode().Perm()&0o077 != 0 {
54+
t.Fatalf("expected 0600-ish perms, got %o", info.Mode().Perm())
55+
}
56+
57+
// Path lives under ~/.wisp
58+
if filepath.Base(filepath.Dir(path)) != config.WispHomeDirName {
59+
t.Fatalf("path not under .wisp: %s", path)
60+
}
61+
}

0 commit comments

Comments
 (0)