Quote a string setting that YAML would read as a number #260

Merged
clawbot merged 1 commits from fix-config-set-numeric-strings into next 2026-10-07 09:29:21 +02:00
3 changed files with 168 additions and 0 deletions
+8
View File
@@ -22,6 +22,14 @@ the tag exists and is exercised; what is left is merging `next` to
# Completed Steps # 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 - 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 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 ([issue #226](https://git.eeqj.de/sneak/vaultik/issues/226)). The
+55
View File
@@ -7,11 +7,14 @@ import (
"os" "os"
"os/exec" "os/exec"
"path/filepath" "path/filepath"
"reflect"
"strconv" "strconv"
"strings" "strings"
"unicode/utf8"
"github.com/spf13/cobra" "github.com/spf13/cobra"
"gopkg.in/yaml.v3" "gopkg.in/yaml.v3"
"sneak.berlin/go/vaultik/internal/config"
"sneak.berlin/go/vaultik/internal/ui" "sneak.berlin/go/vaultik/internal/ui"
) )
@@ -31,6 +34,9 @@ const configDirMode = 0o755
// yaml.Marshal's 4-space default. // yaml.Marshal's 4-space default.
const configYAMLIndent = 2 const configYAMLIndent = 2
// yamlStringTag is YAML's tag for a string scalar.
const yamlStringTag = "!!str"
var ( var (
errConfigExists = errors.New("config file already exists") errConfigExists = errors.New("config file already exists")
errEmptyConfig = errors.New("empty config file") errEmptyConfig = errors.New("empty config file")
@@ -583,9 +589,58 @@ 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.
// The encoder refuses to write a value that is not valid UTF-8 as a
// string. Left untagged, such a value is written as base64 !!binary and
// loads back unchanged.
if configKeyIsString(keys) && utf8.ValidString(value) {
node.Tag = yamlStringTag
}
return nil 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 // yamlSetInMapping resolves (creating if needed) the value node for key
// within a mapping node, setting it to value when it is the final path // within a mapping node, setting it to value when it is the final path
// element, and returns the node to descend into. // element, and returns the node to descend into.
+105
View File
@@ -4,6 +4,7 @@ import (
"bytes" "bytes"
"os" "os"
"path/filepath" "path/filepath"
"strconv"
"strings" "strings"
"testing" "testing"
@@ -94,6 +95,110 @@ 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 }},
{"s3.region", "FALSE",
func(cfg *config.Config) string { return cfg.S3.Region }},
{"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)
}
})
}
}
// TestConfigSetNonUTF8Path checks that config set still accepts a value that
// is not valid UTF-8, such as a path with a Latin-1 file name, and that
// config.Load returns it unchanged.
func TestConfigSetNonUTF8Path(t *testing.T) {
t.Parallel()
const dir = "/srv/caf\xe9"
cfg := loadAfterConfigSet(t, "snapshots.home.paths.0", dir)
got := cfg.Snapshots["home"].Paths[0]
if got != dir {
t.Errorf("snapshots.home.paths.0 = %q, want %q", got, dir)
}
}
// 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 const testYAML = `# top comment
compression_level: 3 compression_level: 3
age_recipients: age_recipients: