Quote a string setting that YAML would read as a number (closes #229)
check / check (push) Waiting to run

config set wrote every value as an unquoted YAML scalar, and config.Load
reads the file through untyped YAML, so an access key 00112233 loaded as
38043 and a hostname 007 as 7.

config set now looks the key up in config.Config by the fields' yaml
tags. A string setting is tagged !!str, which the encoder quotes
wherever YAML would read a number or a boolean. Other settings stay
unquoted, so compression_level 9 is still a number. A value that is not
valid UTF-8 stays untagged and is written as !!binary, which loads back
unchanged.

Judgement call: the type comes from reflection over config.Config.

Model: opus-5-5
This commit was merged in pull request #260.
This commit is contained in:
2026-10-07 09:29:19 +02:00
parent b57ce2277d
commit 5d1118d143
3 changed files with 168 additions and 0 deletions
+55
View File
@@ -7,11 +7,14 @@ import (
"os"
"os/exec"
"path/filepath"
"reflect"
"strconv"
"strings"
"unicode/utf8"
"github.com/spf13/cobra"
"gopkg.in/yaml.v3"
"sneak.berlin/go/vaultik/internal/config"
"sneak.berlin/go/vaultik/internal/ui"
)
@@ -31,6 +34,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 +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
}
// 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.
+105
View File
@@ -4,6 +4,7 @@ import (
"bytes"
"os"
"path/filepath"
"strconv"
"strings"
"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
compression_level: 3
age_recipients: