diff --git a/TODO.md b/TODO.md index 3383609..1e1fdaa 100644 --- a/TODO.md +++ b/TODO.md @@ -22,6 +22,14 @@ the tag exists and is exercised; what is left is merging `next` to # Completed Steps +- 2026-10-07: Made `config set` keep a string that looks like a number + ([issue #229](https://git.eeqj.de/sneak/vaultik/issues/229)). It wrote + every value unquoted, and `config.Load` reads the file through untyped + YAML, so an access key `00112233` loaded as `38043` and a hostname `007` + as `7`. A value for a string setting in `config.Config` is now tagged as + a YAML string, which the file quotes wherever YAML would read a number or + a boolean; other settings are still written unquoted. + - 2026-10-07: Made a backup notice a file rewritten with its size unchanged and a new mtime in the same second as the one in the index ([issue #226](https://git.eeqj.de/sneak/vaultik/issues/226)). The diff --git a/internal/cli/config.go b/internal/cli/config.go index a7444f8..3f047b2 100644 --- a/internal/cli/config.go +++ b/internal/cli/config.go @@ -7,11 +7,13 @@ import ( "os" "os/exec" "path/filepath" + "reflect" "strconv" "strings" "github.com/spf13/cobra" "gopkg.in/yaml.v3" + "sneak.berlin/go/vaultik/internal/config" "sneak.berlin/go/vaultik/internal/ui" ) @@ -31,6 +33,9 @@ const configDirMode = 0o755 // yaml.Marshal's 4-space default. const configYAMLIndent = 2 +// yamlStringTag is YAML's tag for a string scalar. +const yamlStringTag = "!!str" + var ( errConfigExists = errors.New("config file already exists") errEmptyConfig = errors.New("empty config file") @@ -583,9 +588,55 @@ func yamlPathSet(root *yaml.Node, keys []string, value string) error { } } + // config.Load reads the file through untyped YAML, which turns an + // unquoted 00112233 into the number 38043 and 1e5 into 100000. Tagging + // a string setting as a string makes the encoder quote such a value. + // Other settings stay unquoted, so compression_level 9 is a number. + if configKeyIsString(keys) { + node.Tag = yamlStringTag + } + return nil } +// configKeyIsString reports whether the dotted key names a string in +// config.Config, following the fields' yaml tags, as s3.access_key_id and +// snapshots.home.exclude.0 do. +func configKeyIsString(keys []string) bool { + typ := reflect.TypeFor[config.Config]() + + for _, key := range keys { + switch { + case typ.Kind() == reflect.Map || typ.Kind() == reflect.Slice: + // The key is a snapshot name or a list index. + typ = typ.Elem() + case typ.Kind() == reflect.Struct: + field, ok := yamlField(typ, key) + if !ok { + return false + } + + typ = field.Type + default: + return false + } + } + + return typ.Kind() == reflect.String +} + +// yamlField returns the field of struct type typ whose yaml tag names key. +func yamlField(typ reflect.Type, key string) (reflect.StructField, bool) { + for field := range typ.Fields() { + name, _, _ := strings.Cut(field.Tag.Get("yaml"), ",") + if name == key { + return field, true + } + } + + return reflect.StructField{}, false +} + // yamlSetInMapping resolves (creating if needed) the value node for key // within a mapping node, setting it to value when it is the final path // element, and returns the node to descend into. diff --git a/internal/cli/config_test.go b/internal/cli/config_test.go index 4e3f7e2..3275f59 100644 --- a/internal/cli/config_test.go +++ b/internal/cli/config_test.go @@ -4,6 +4,7 @@ import ( "bytes" "os" "path/filepath" + "strconv" "strings" "testing" @@ -94,6 +95,92 @@ func TestConfigSetRecipientOnFreshConfig(t *testing.T) { } } +// TestConfigSetStringLooksLikeNumber sets string settings to values that +// YAML reads as numbers or booleans when they are unquoted, and checks that +// config.Load returns each one unchanged. +func TestConfigSetStringLooksLikeNumber(t *testing.T) { + t.Parallel() + + tests := []struct { + key string + value string + field func(cfg *config.Config) string + }{ + {"s3.access_key_id", "00112233", + func(cfg *config.Config) string { return cfg.S3.AccessKeyID }}, + {"s3.secret_access_key", "12345678901234567890123456789012", + func(cfg *config.Config) string { return cfg.S3.SecretAccessKey }}, + {"hostname", "007", + func(cfg *config.Config) string { return cfg.Hostname }}, + {"s3.prefix", "1e5", + func(cfg *config.Config) string { return cfg.S3.Prefix }}, + {"s3.bucket", "true", + func(cfg *config.Config) string { return cfg.S3.Bucket }}, + {"snapshots.home.exclude.0", "1.10", + func(cfg *config.Config) string { return cfg.Snapshots["home"].Exclude[0] }}, + } + + for _, tt := range tests { + t.Run(tt.key+"="+tt.value, func(t *testing.T) { + t.Parallel() + + cfg := loadAfterConfigSet(t, tt.key, tt.value) + + got := tt.field(cfg) + if got != tt.value { + t.Errorf("%s = %q after config set %q", tt.key, got, tt.value) + } + }) + } +} + +// TestConfigSetNumberStaysNumber checks that a number set for an integer +// setting is still read as a number, not as a quoted string. +func TestConfigSetNumberStaysNumber(t *testing.T) { + t.Parallel() + + const level = 9 + + cfg := loadAfterConfigSet(t, "compression_level", strconv.Itoa(level)) + + if cfg.CompressionLevel != level { + t.Errorf("compression_level = %d, want %d", cfg.CompressionLevel, level) + } +} + +// loadAfterConfigSet writes the file `config init` writes, sets storage_url +// to a local directory so that the file passes validation, applies +// `config set key value` and returns what config.Load reads back. +func loadAfterConfigSet(t *testing.T, key, value string) *config.Config { + t.Helper() + + path := filepath.Join(t.TempDir(), "config.yml") + + err := os.WriteFile(path, []byte(defaultConfigTemplate), configFileMode) + if err != nil { + t.Fatalf("write config: %v", err) + } + + out := ui.NewWithColor(&bytes.Buffer{}, false) + + err = writeConfigSet(out, path, "storage_url", "file:///mnt/backups") + if err != nil { + t.Fatalf("config set storage_url: %v", err) + } + + err = writeConfigSet(out, path, key, value) + if err != nil { + t.Fatalf("config set %s: %v", key, err) + } + + cfg, err := config.Load(path) + if err != nil { + t.Fatalf("config.Load: %v", err) + } + + return cfg +} + const testYAML = `# top comment compression_level: 3 age_recipients: