Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: Hide other sensitive cfg values #194

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: Hide other sensitive cfg values #194

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 2 additions & 4 deletions cmd/server/entrypoint.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,17 +73,15 @@ func StartProxySvr(cliCtx *cli.Context) error {
}

// TODO: we should probably just change EdaClientConfig struct definition in eigenda-client
// to have a `json:"-"` tag on the SignerPrivateKeyHex field, to prevent the privateKey from being marshaled at all
func prettyPrintConfig(cliCtx *cli.Context, log log.Logger) error {
// we read a new config which we modify to hide private info in order to log the rest
cfg := server.ReadCLIConfig(cliCtx)
cfg.EigenDAConfig.EdaClientConfig.SignerPrivateKeyHex = "HIDDEN"
cfg.EigenDAConfig.VerifierConfig.RPCURL = "HIDDEN"

configJSON, err := json.MarshalIndent(cfg, "", " ")
if err != nil {
return fmt.Errorf("failed to marshal config: %w", err)
}
cfg.EigenDAConfig.EdaClientConfig.SignerPrivateKeyHex = "" // marshaling defined in client config
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did you move this after the marshaling? Like this it doesn’t do anything

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it does actually - the SignerPrivateKey is stored as part of the EigenDAClientConfig which is defined within layr-labs/eigenda

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The log line below is

log.Info(fmt.Sprintf("Initializing EigenDA proxy server with config: %v", string(configJSON)))

which prints the already marshalled config. So changing the fields of cfg here above doesn't do anything.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤦‍♂️

cfg.EigenDAConfig.EdaClientConfig.RPC = "" // hiding as RPC providers typically use sensitive API keys within
log.Info(fmt.Sprintf("Initializing EigenDA proxy server with config: %v", string(configJSON)))
return nil
}
1 change: 0 additions & 1 deletion e2e/setup.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,6 @@ func createRedisConfig(eigendaCfg server.Config) server.CLIConfig {
Password: "",
DB: 0,
Eviction: 10 * time.Minute,
Profile: true,
}
return server.CLIConfig{
EigenDAConfig: eigendaCfg,
Expand Down
9 changes: 4 additions & 5 deletions store/precomputed_key/redis/redis.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,10 @@ import (

// Config ... user configurable
type Config struct {
Endpoint string
Password string
DB int
Eviction time.Duration
Profile bool
Endpoint string `json:"endpoint"`
Password string `json:"-"`
samlaf marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thought about this approach some more and not sure I like it, for a few reasons:

  1. if some future developer ever wants to marshal AND unmarshal the config for whatever reason he will run into a bug (can fix this by defining a custom marshalJSONHideSecrets function instead)
  2. this will print empty string, so reader won’t know if password field was hidden or just never set (we can fix this by using omitempty json tag)

Could look something like this (Claude generated so beware):

package main

import (
	"encoding/json"
	"fmt"
	"time"
)

type Config struct {
	Endpoint string        `json:"endpoint"`
	Password string        `json:"password,omitempty"`
	DB       int           `json:"database"`
	Eviction time.Duration `json:"eviction"`
}

// Custom MarshalJSON function to control what gets included in the JSON output
func (c Config) MarshalJSON() ([]byte, error) {
	type Alias Config // Use an alias to avoid recursion with MarshalJSON
	aux := struct {
		Alias
		Password string `json:"password,omitempty"`
	}{
		Alias:    (Alias)(c),
		Password: "",
	}

	// Conditionally include a masked password if it is set
	if c.Password != "" {
		aux.Password = "****" // Mask the password instead of excluding it
	}
	return json.Marshal(aux)
}

func main() {
	cfg := Config{
		Endpoint: "localhost:6379",
		Password: "supersecret",
		DB:       0,
		Eviction: time.Hour,
	}

	jsonData, _ := json.MarshalIndent(cfg, "", "  ")
	fmt.Println(string(jsonData))
}

Maybe also use a separate Marshal function to not change the default marshaling behavior?

DB int `json:"database"`
Eviction time.Duration `json:"eviction"`
}

// Store ... Redis storage backend implementation
Expand Down
14 changes: 7 additions & 7 deletions store/precomputed_key/s3/s3.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,13 +38,13 @@ var _ store.PrecomputedKeyStore = (*Store)(nil)

type CredentialType string
type Config struct {
CredentialType CredentialType
Endpoint string
EnableTLS bool
AccessKeyID string
AccessKeySecret string
Bucket string
Path string
CredentialType CredentialType `json:"credential_type"`
Endpoint string `json:"endpoint"`
EnableTLS bool `json:"enable_tls"`
AccessKeyID string `json:"access_key_id"`
AccessKeySecret string `json:"-"`
Bucket string `json:"bucket"`
Path string `json:"path"`
}

// Store ... S3 store
Expand Down
Loading