Give the local index and its export copy an explicit 0600 mode (closes #168)
check / check (pull_request) Successful in 2m41s
check / check (pull_request) Successful in 2m41s
The local index lists every backed-up path and chunk hash, but its file mode was left to the SQLite driver and the umask, so under a typical 022 umask a fresh index (and its -wal/-shm side files) landed world-readable. The snapshot export copied the index to snapshot.db with a permissive create as well. provideDatabase now calls ensureIndexFileMode before opening the driver: it creates the index 0600 if missing and chmods an existing one to 0600. Doing this before the driver opens the file matters because SQLite gives its -wal and -shm files the mode of the main database file. The export copy is now created 0600 instead of via the umask-dependent default. Tests under umask 022 cover a fresh index, an existing 0644 index in a 0755 directory, and the export copy, asserting each ends up 0600. Model: opus-4-8
This commit is contained in:
@@ -2,6 +2,7 @@ package database
|
|||||||
|
|
||||||
import (
|
import (
|
||||||
"context"
|
"context"
|
||||||
|
"errors"
|
||||||
"fmt"
|
"fmt"
|
||||||
"os"
|
"os"
|
||||||
"path/filepath"
|
"path/filepath"
|
||||||
@@ -15,6 +16,44 @@ import (
|
|||||||
// the index describes the backed-up file tree and must stay private.
|
// the index describes the backed-up file tree and must stay private.
|
||||||
const indexDirPerm = 0o700
|
const indexDirPerm = 0o700
|
||||||
|
|
||||||
|
// indexFilePerm restricts the index file to the owning user; it lists every
|
||||||
|
// backed-up path and chunk hash and must stay private.
|
||||||
|
const indexFilePerm = 0o600
|
||||||
|
|
||||||
|
// ensureIndexFileMode makes the index file owner-only before the SQLite
|
||||||
|
// driver opens it: it creates the file 0600 if absent, or chmods an existing
|
||||||
|
// one to 0600. Doing this first matters because SQLite creates its -wal and
|
||||||
|
// -shm side files with the mode of the main database file, so a private main
|
||||||
|
// file yields private side files. The driver treats a zero-byte file as an
|
||||||
|
// empty database, so pre-creating it here is safe.
|
||||||
|
func ensureIndexFileMode(path string) error {
|
||||||
|
info, err := os.Stat(path)
|
||||||
|
|
||||||
|
switch {
|
||||||
|
case err == nil:
|
||||||
|
if info.Mode().Perm() == indexFilePerm {
|
||||||
|
return nil
|
||||||
|
}
|
||||||
|
|
||||||
|
err = os.Chmod(path, indexFilePerm)
|
||||||
|
if err != nil {
|
||||||
|
return fmt.Errorf("restricting index file permissions: %w", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
return nil
|
||||||
|
case errors.Is(err, os.ErrNotExist):
|
||||||
|
//nolint:gosec // G304: the index path is operator-configured by design
|
||||||
|
f, err := os.OpenFile(path, os.O_CREATE|os.O_WRONLY, indexFilePerm)
|
||||||
|
if err != nil {
|
||||||
|
return fmt.Errorf("creating index file: %w", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
return f.Close()
|
||||||
|
default:
|
||||||
|
return fmt.Errorf("checking index file: %w", err)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// Module provides database dependencies
|
// Module provides database dependencies
|
||||||
//
|
//
|
||||||
//nolint:gochecknoglobals // fx module definitions are package globals by convention
|
//nolint:gochecknoglobals // fx module definitions are package globals by convention
|
||||||
@@ -34,6 +73,11 @@ func provideDatabase(lc fx.Lifecycle, cfg *config.Config) (*DB, error) {
|
|||||||
return nil, fmt.Errorf("creating index directory: %w", err)
|
return nil, fmt.Errorf("creating index directory: %w", err)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
err = ensureIndexFileMode(cfg.IndexPath)
|
||||||
|
if err != nil {
|
||||||
|
return nil, err
|
||||||
|
}
|
||||||
|
|
||||||
db, err := New(context.Background(), cfg.IndexPath)
|
db, err := New(context.Background(), cfg.IndexPath)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return nil, fmt.Errorf("opening database: %w", err)
|
return nil, fmt.Errorf("opening database: %w", err)
|
||||||
|
|||||||
@@ -0,0 +1,100 @@
|
|||||||
|
package database
|
||||||
|
|
||||||
|
import (
|
||||||
|
"os"
|
||||||
|
"path/filepath"
|
||||||
|
"syscall"
|
||||||
|
"testing"
|
||||||
|
|
||||||
|
"go.uber.org/fx/fxtest"
|
||||||
|
"sneak.berlin/go/vaultik/internal/config"
|
||||||
|
)
|
||||||
|
|
||||||
|
// TestProvideDatabaseFreshIndexMode verifies that provideDatabase creates a
|
||||||
|
// missing index file owner-only (0600), even under a lenient 022 umask that
|
||||||
|
// would otherwise leave a freshly created file world-readable.
|
||||||
|
//
|
||||||
|
//nolint:paralleltest // syscall.Umask is process-global; parallel tests would clash
|
||||||
|
func TestProvideDatabaseFreshIndexMode(t *testing.T) {
|
||||||
|
restore := syscall.Umask(0o022)
|
||||||
|
defer syscall.Umask(restore)
|
||||||
|
|
||||||
|
indexPath := filepath.Join(t.TempDir(), "index.sqlite")
|
||||||
|
|
||||||
|
openIndex(t, indexPath)
|
||||||
|
assertPerm(t, indexPath, 0o600)
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestProvideDatabaseExistingIndexMode verifies that provideDatabase tightens
|
||||||
|
// an existing world-readable index (0644) in a group/other-readable directory
|
||||||
|
// down to owner-only (0600).
|
||||||
|
//
|
||||||
|
//nolint:paralleltest // syscall.Umask is process-global; parallel tests would clash
|
||||||
|
func TestProvideDatabaseExistingIndexMode(t *testing.T) {
|
||||||
|
restore := syscall.Umask(0o022)
|
||||||
|
defer syscall.Umask(restore)
|
||||||
|
|
||||||
|
dir := filepath.Join(t.TempDir(), "data")
|
||||||
|
|
||||||
|
//nolint:gosec // G301: the test intentionally uses a 0755 directory
|
||||||
|
err := os.MkdirAll(dir, 0o755)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("creating index directory: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
//nolint:gosec // G302: the test intentionally uses a 0755 directory
|
||||||
|
err = os.Chmod(dir, 0o755)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("relaxing index directory permissions: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
indexPath := filepath.Join(dir, "index.sqlite")
|
||||||
|
|
||||||
|
//nolint:gosec // G306: the test intentionally starts from a 0644 index
|
||||||
|
err = os.WriteFile(indexPath, nil, 0o644)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("creating pre-existing index: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
//nolint:gosec // G302: the test intentionally starts from a 0644 index
|
||||||
|
err = os.Chmod(indexPath, 0o644)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("relaxing pre-existing index permissions: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
openIndex(t, indexPath)
|
||||||
|
assertPerm(t, indexPath, 0o600)
|
||||||
|
}
|
||||||
|
|
||||||
|
// openIndex runs provideDatabase against indexPath and closes the resulting
|
||||||
|
// database before returning.
|
||||||
|
func openIndex(t *testing.T, indexPath string) {
|
||||||
|
t.Helper()
|
||||||
|
|
||||||
|
cfg := &config.Config{IndexPath: indexPath}
|
||||||
|
|
||||||
|
db, err := provideDatabase(fxtest.NewLifecycle(t), cfg)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("provideDatabase: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
err = db.Close()
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("closing database: %v", err)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// assertPerm fails the test unless path has exactly the given permission bits.
|
||||||
|
func assertPerm(t *testing.T, path string, want os.FileMode) {
|
||||||
|
t.Helper()
|
||||||
|
|
||||||
|
info, err := os.Stat(path)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("stat %s: %v", path, err)
|
||||||
|
}
|
||||||
|
|
||||||
|
got := info.Mode().Perm()
|
||||||
|
if got != want {
|
||||||
|
t.Fatalf("permissions of %s = %#o, want %#o", path, got, want)
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -0,0 +1,49 @@
|
|||||||
|
//nolint:testpackage // exercises the unexported copyFile helper
|
||||||
|
package snapshot
|
||||||
|
|
||||||
|
import (
|
||||||
|
"os"
|
||||||
|
"path/filepath"
|
||||||
|
"syscall"
|
||||||
|
"testing"
|
||||||
|
|
||||||
|
"github.com/spf13/afero"
|
||||||
|
)
|
||||||
|
|
||||||
|
// TestCopyFileExportCopyMode verifies that the exported snapshot database
|
||||||
|
// copy is created owner-only (0600), even under a lenient 022 umask that
|
||||||
|
// would otherwise leave a fresh file world-readable.
|
||||||
|
//
|
||||||
|
//nolint:paralleltest // syscall.Umask is process-global; parallel tests would clash
|
||||||
|
func TestCopyFileExportCopyMode(t *testing.T) {
|
||||||
|
restore := syscall.Umask(0o022)
|
||||||
|
defer syscall.Umask(restore)
|
||||||
|
|
||||||
|
dir := t.TempDir()
|
||||||
|
|
||||||
|
src := filepath.Join(dir, "index.sqlite")
|
||||||
|
|
||||||
|
err := os.WriteFile(src, []byte("index data"), 0o600)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("creating source index: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
dst := filepath.Join(dir, "snapshot.db")
|
||||||
|
|
||||||
|
sm := &SnapshotManager{fs: afero.NewOsFs()}
|
||||||
|
|
||||||
|
err = sm.copyFile(src, dst)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("copyFile: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
info, err := os.Stat(dst)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("stat export copy: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
got := info.Mode().Perm()
|
||||||
|
if got != 0o600 {
|
||||||
|
t.Fatalf("export copy permissions = %#o, want %#o", got, 0o600)
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -44,6 +44,7 @@ import (
|
|||||||
"errors"
|
"errors"
|
||||||
"fmt"
|
"fmt"
|
||||||
"io"
|
"io"
|
||||||
|
"os"
|
||||||
"path/filepath"
|
"path/filepath"
|
||||||
"strings"
|
"strings"
|
||||||
"time"
|
"time"
|
||||||
@@ -763,7 +764,13 @@ func (sm *SnapshotManager) compressFile(inputPath, outputPath string) error {
|
|||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
|
|
||||||
// copyFile copies a file from src to dst
|
// exportCopyPerm restricts the exported snapshot database copy to the owning
|
||||||
|
// user; it holds the same private index data as the local index file.
|
||||||
|
const exportCopyPerm = 0o600
|
||||||
|
|
||||||
|
// copyFile copies a file from src to dst. The destination is the exported
|
||||||
|
// snapshot database, so it is created owner-only rather than with the
|
||||||
|
// umask-dependent default.
|
||||||
func (sm *SnapshotManager) copyFile(src, dst string) error {
|
func (sm *SnapshotManager) copyFile(src, dst string) error {
|
||||||
log.Debug("Opening source file for copy", "path", src)
|
log.Debug("Opening source file for copy", "path", src)
|
||||||
|
|
||||||
@@ -783,7 +790,9 @@ func (sm *SnapshotManager) copyFile(src, dst string) error {
|
|||||||
|
|
||||||
log.Debug("Creating destination file", "path", dst)
|
log.Debug("Creating destination file", "path", dst)
|
||||||
|
|
||||||
destFile, err := sm.fs.Create(dst)
|
destFile, err := sm.fs.OpenFile(
|
||||||
|
dst, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, exportCopyPerm,
|
||||||
|
)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return err
|
return err
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user