The warning for a config file that others can read always said the
file contained S3 credentials, so a file:// config with none got a
false claim. When s3.access_key_id or s3.secret_access_key is set it
now says the file may contain them, because Load sees the values only
after smartconfig has replaced any ${...} reference, so a set
credential need not be in the file. Otherwise it says the file is
readable by others.
snapshot purge wrapped the listing error, which already starts with
"listing remote snapshots:", in that prefix a second time.
syncWithRemote now returns it unwrapped, as CleanupLocalSnapshots
does.
Model: opus-5-5
This commit was merged in pull request #263.
This commit is contained in:
@@ -22,6 +22,17 @@ the tag exists and is exercised; what is left is merging `next` to
|
||||
|
||||
# Completed Steps
|
||||
|
||||
- 2026-10-07: Made two messages say only what is true
|
||||
([issue #240](https://git.eeqj.de/sneak/vaultik/issues/240)). A config
|
||||
file that others can read was warned about as containing S3
|
||||
credentials even when it set none, as a `file://` config does. The
|
||||
warning now says the file may contain S3 credentials only when
|
||||
`s3.access_key_id` or `s3.secret_access_key` is set, since either may
|
||||
come from a `${...}` reference rather than the file, and otherwise
|
||||
says the file is readable by others. `snapshot purge` against a
|
||||
destination store it could not list gave an error with
|
||||
`listing remote snapshots:` in it twice; the prefix now appears once.
|
||||
|
||||
- 2026-10-07: Made `s3.part_size` set the multipart upload part size
|
||||
([issue #232](https://git.eeqj.de/sneak/vaultik/issues/232)). It was
|
||||
loaded and defaulted but never passed to the S3 client, whose uploader
|
||||
|
||||
@@ -305,7 +305,7 @@ func Load(path string) (*Config, error) {
|
||||
if statErr == nil {
|
||||
mode := info.Mode().Perm()
|
||||
if mode&0044 != 0 { // group or world readable
|
||||
log.Warn("Config file has insecure permissions (contains S3 credentials)",
|
||||
log.Warn(cfg.readableByOthersWarning(),
|
||||
"path", path,
|
||||
"mode", fmt.Sprintf("%04o", mode),
|
||||
"recommendation", "chmod 600 "+path)
|
||||
@@ -418,6 +418,18 @@ func (c *Config) setAgeSecretKey() {
|
||||
}
|
||||
}
|
||||
|
||||
// readableByOthersWarning is the warning Load logs when others can read
|
||||
// the config file. It says "may contain" because the S3 credentials are
|
||||
// seen only after smartconfig has replaced any ${...} reference in the
|
||||
// file with its value, so a set credential need not be in the file.
|
||||
func (c *Config) readableByOthersWarning() string {
|
||||
if c.S3.AccessKeyID != "" || c.S3.SecretAccessKey != "" {
|
||||
return "Config file is readable by others and may contain S3 credentials"
|
||||
}
|
||||
|
||||
return "Config file is readable by others"
|
||||
}
|
||||
|
||||
// validateStorage validates storage configuration.
|
||||
// If StorageURL is set, it takes precedence. S3 URLs require credentials.
|
||||
// File URLs don't require any S3 configuration.
|
||||
|
||||
@@ -8,6 +8,7 @@ import (
|
||||
"testing"
|
||||
|
||||
"sneak.berlin/go/vaultik/internal/chunker"
|
||||
"sneak.berlin/go/vaultik/internal/log"
|
||||
)
|
||||
|
||||
const (
|
||||
@@ -459,3 +460,125 @@ func TestAgeSecretKeySourceName(t *testing.T) {
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// loadReadableConfig writes configYAML to a file that others can read,
|
||||
// loads it, and returns what the logger wrote to stderr meanwhile. The
|
||||
// logger writes to the os.Stderr it finds when it is initialized, so
|
||||
// os.Stderr is pointed at a file first. Not parallel-safe: os.Stderr and
|
||||
// the logger are process-global.
|
||||
func loadReadableConfig(t *testing.T, configYAML string) string {
|
||||
t.Helper()
|
||||
|
||||
dir := t.TempDir()
|
||||
configPath := filepath.Join(dir, "config.yml")
|
||||
stderrPath := filepath.Join(dir, "stderr")
|
||||
|
||||
err := os.WriteFile(configPath, []byte(configYAML), 0o600)
|
||||
if err != nil {
|
||||
t.Fatalf("writing config: %v", err)
|
||||
}
|
||||
|
||||
//nolint:gosec // G302: the test needs a config file others can read
|
||||
err = os.Chmod(configPath, 0o644)
|
||||
if err != nil {
|
||||
t.Fatalf("chmod config: %v", err)
|
||||
}
|
||||
|
||||
stderrFile, err := os.Create(stderrPath) //nolint:gosec // G304: test temp path
|
||||
if err != nil {
|
||||
t.Fatalf("creating stderr file: %v", err)
|
||||
}
|
||||
|
||||
previous := os.Stderr
|
||||
os.Stderr = stderrFile
|
||||
|
||||
log.Initialize(log.Config{})
|
||||
|
||||
_, loadErr := Load(configPath)
|
||||
|
||||
os.Stderr = previous
|
||||
|
||||
log.Initialize(log.Config{})
|
||||
|
||||
_ = stderrFile.Close()
|
||||
|
||||
if loadErr != nil {
|
||||
t.Fatalf("Load() error = %v", loadErr)
|
||||
}
|
||||
|
||||
captured, err := os.ReadFile(stderrPath) //nolint:gosec // G304: test temp path
|
||||
if err != nil {
|
||||
t.Fatalf("reading stderr file: %v", err)
|
||||
}
|
||||
|
||||
return string(captured)
|
||||
}
|
||||
|
||||
// TestLoadWarnsReadableConfigWithoutS3Credentials checks that a config
|
||||
// file others can read, holding no S3 credentials, is warned about
|
||||
// without a claim that it holds them.
|
||||
//
|
||||
//nolint:paralleltest // loadReadableConfig replaces os.Stderr
|
||||
func TestLoadWarnsReadableConfigWithoutS3Credentials(t *testing.T) {
|
||||
stderr := loadReadableConfig(t, `
|
||||
storage_url: file:///var/backups/vaultik
|
||||
snapshots:
|
||||
home:
|
||||
paths:
|
||||
- /home
|
||||
`)
|
||||
|
||||
if !strings.Contains(stderr, "Config file is readable by others") {
|
||||
t.Errorf("expected a warning that the file is readable by others, got %q",
|
||||
stderr)
|
||||
}
|
||||
|
||||
if strings.Contains(stderr, "S3 credentials") {
|
||||
t.Errorf("warning names S3 credentials the file does not set: %q", stderr)
|
||||
}
|
||||
}
|
||||
|
||||
// TestLoadWarnsReadableConfigWithS3Credentials checks that a config file
|
||||
// others can read and that sets S3 credentials, as values or as ${ENV:...}
|
||||
// references, is warned about as one that may contain them.
|
||||
//
|
||||
//nolint:paralleltest // loadReadableConfig replaces os.Stderr
|
||||
func TestLoadWarnsReadableConfigWithS3Credentials(t *testing.T) {
|
||||
t.Setenv("VAULTIK_TEST_ACCESS_KEY_ID", "test-access-key")
|
||||
t.Setenv("VAULTIK_TEST_SECRET_ACCESS_KEY", "test-secret-key")
|
||||
|
||||
configs := map[string]string{
|
||||
"values": `
|
||||
storage_url: s3://bucket/prefix?endpoint=s3.example.com
|
||||
s3:
|
||||
access_key_id: test-access-key
|
||||
secret_access_key: test-secret-key
|
||||
snapshots:
|
||||
home:
|
||||
paths:
|
||||
- /home
|
||||
`,
|
||||
"references": `
|
||||
storage_url: s3://bucket/prefix?endpoint=s3.example.com
|
||||
s3:
|
||||
access_key_id: ${ENV:VAULTIK_TEST_ACCESS_KEY_ID}
|
||||
secret_access_key: ${ENV:VAULTIK_TEST_SECRET_ACCESS_KEY}
|
||||
snapshots:
|
||||
home:
|
||||
paths:
|
||||
- /home
|
||||
`,
|
||||
}
|
||||
|
||||
for name, configYAML := range configs {
|
||||
t.Run(name, func(t *testing.T) {
|
||||
stderr := loadReadableConfig(t, configYAML)
|
||||
|
||||
if !strings.Contains(stderr,
|
||||
"Config file is readable by others and may contain S3 credentials") {
|
||||
t.Errorf("expected a warning naming the S3 credentials, got %q",
|
||||
stderr)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
@@ -6,6 +6,7 @@ import (
|
||||
"io/fs"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"github.com/spf13/afero"
|
||||
@@ -153,3 +154,23 @@ func TestPruneKeepsLocalRecordsWhenDestinationMissing(t *testing.T) {
|
||||
require.NoError(t, err)
|
||||
assert.Len(t, snapshots, 1, "prune must delete no local snapshot record")
|
||||
}
|
||||
|
||||
// TestPurgeSaysListingFailedOnceWhenDestinationMissing checks that
|
||||
// snapshot purge fails on a destination it cannot list, with an error
|
||||
// that says "listing remote snapshots" once.
|
||||
//
|
||||
//nolint:paralleltest // installs the global logger via log.Initialize
|
||||
func TestPurgeSaysListingFailedOnceWhenDestinationMissing(t *testing.T) {
|
||||
log.Initialize(log.Config{})
|
||||
|
||||
ctx := context.Background()
|
||||
v, _, _ := backUpThenUnplug(ctx, t)
|
||||
|
||||
err := v.PurgeSnapshotsWithOptions(&vaultik.SnapshotPurgeOptions{
|
||||
KeepLatest: true,
|
||||
Force: true,
|
||||
})
|
||||
require.ErrorIs(t, err, fs.ErrNotExist)
|
||||
assert.Equal(t, 1, strings.Count(err.Error(), "listing remote snapshots"),
|
||||
err.Error())
|
||||
}
|
||||
|
||||
@@ -1052,7 +1052,7 @@ func (v *Vaultik) syncWithRemote() error {
|
||||
// every local snapshot record (issue #160).
|
||||
remoteKeys, err := v.listAllRemoteSnapshotKeys()
|
||||
if err != nil {
|
||||
return fmt.Errorf("listing remote snapshots: %w", err)
|
||||
return err
|
||||
}
|
||||
|
||||
remoteKeySet := make(map[string]bool, len(remoteKeys))
|
||||
|
||||
Reference in New Issue
Block a user