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 is contained in:
@@ -22,6 +22,17 @@ the tag exists and is exercised; what is left is merging `next` to
|
|||||||
|
|
||||||
# Completed Steps
|
# 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
|
- 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
|
([issue #232](https://git.eeqj.de/sneak/vaultik/issues/232)). It was
|
||||||
loaded and defaulted but never passed to the S3 client, whose uploader
|
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 {
|
if statErr == nil {
|
||||||
mode := info.Mode().Perm()
|
mode := info.Mode().Perm()
|
||||||
if mode&0044 != 0 { // group or world readable
|
if mode&0044 != 0 { // group or world readable
|
||||||
log.Warn("Config file has insecure permissions (contains S3 credentials)",
|
log.Warn(cfg.readableByOthersWarning(),
|
||||||
"path", path,
|
"path", path,
|
||||||
"mode", fmt.Sprintf("%04o", mode),
|
"mode", fmt.Sprintf("%04o", mode),
|
||||||
"recommendation", "chmod 600 "+path)
|
"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.
|
// validateStorage validates storage configuration.
|
||||||
// If StorageURL is set, it takes precedence. S3 URLs require credentials.
|
// If StorageURL is set, it takes precedence. S3 URLs require credentials.
|
||||||
// File URLs don't require any S3 configuration.
|
// File URLs don't require any S3 configuration.
|
||||||
|
|||||||
@@ -8,6 +8,7 @@ import (
|
|||||||
"testing"
|
"testing"
|
||||||
|
|
||||||
"sneak.berlin/go/vaultik/internal/chunker"
|
"sneak.berlin/go/vaultik/internal/chunker"
|
||||||
|
"sneak.berlin/go/vaultik/internal/log"
|
||||||
)
|
)
|
||||||
|
|
||||||
const (
|
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"
|
"io/fs"
|
||||||
"os"
|
"os"
|
||||||
"path/filepath"
|
"path/filepath"
|
||||||
|
"strings"
|
||||||
"testing"
|
"testing"
|
||||||
|
|
||||||
"github.com/spf13/afero"
|
"github.com/spf13/afero"
|
||||||
@@ -153,3 +154,23 @@ func TestPruneKeepsLocalRecordsWhenDestinationMissing(t *testing.T) {
|
|||||||
require.NoError(t, err)
|
require.NoError(t, err)
|
||||||
assert.Len(t, snapshots, 1, "prune must delete no local snapshot record")
|
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).
|
// every local snapshot record (issue #160).
|
||||||
remoteKeys, err := v.listAllRemoteSnapshotKeys()
|
remoteKeys, err := v.listAllRemoteSnapshotKeys()
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return fmt.Errorf("listing remote snapshots: %w", err)
|
return err
|
||||||
}
|
}
|
||||||
|
|
||||||
remoteKeySet := make(map[string]bool, len(remoteKeys))
|
remoteKeySet := make(map[string]bool, len(remoteKeys))
|
||||||
|
|||||||
Reference in New Issue
Block a user