From 85fceaa686038787773256f9888071f78fb21dc8 Mon Sep 17 00:00:00 2001 From: mrleo1nid <43583385+mrleo1nid@users.noreply.github.com> Date: Tue, 20 Oct 2026 19:22:49 +0300 Subject: [PATCH] fix(storage-agent): write nothing but the music files to the mounted storage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Imported tracks are created directly under their final name with O_EXCL (never overwriting; " (2)" on conflict), fsync'ed and checked against SHA-256 — no hidden .pipu-tmp file next to them any more. - On any failure the agent removes its own partial file and the empty directories it created for it. - The write check uses access(W_OK) and the read-only mount flag instead of creating and deleting .pipu-write-test in the storage root. - Tests assert the storage tree holds only audio files after successful and failed writes and after the write check. Co-Authored-By: Claude Opus 5.5 --- docs/library-and-storage.md | 10 +- services/api/go.mod | 6 +- .../api/internal/storageagent/access_other.go | 20 ++++ .../api/internal/storageagent/access_unix.go | 18 ++++ services/api/internal/storageagent/files.go | 96 ++++++++++--------- .../api/internal/storageagent/files_test.go | 22 ++++- 6 files changed, 116 insertions(+), 56 deletions(-) create mode 100644 services/api/internal/storageagent/access_other.go create mode 100644 services/api/internal/storageagent/access_unix.go diff --git a/docs/library-and-storage.md b/docs/library-and-storage.md index ade5b52..9377a0b 100644 --- a/docs/library-and-storage.md +++ b/docs/library-and-storage.md @@ -35,8 +35,7 @@ `fsync`, сверка SHA-256, атомарный `rename`. Перезапись существующих файлов и удаление запрещены на уровне кода агента. - Права на NFS-сервере: экспорт должен разрешать запись для UID/GID агента (`PIPU_FILE_UID` / `PIPU_FILE_GID`, задаются в `.env` под владельца музыки на файловом сервере); новые файлы — `0644`, каталоги — `0755`. -- `POST /admin/sources/{id}/test` для такого источника дополнительно проверяет запись: создаёт и удаляет - `.pipu-write-test` в корне. +- `POST /admin/sources/{id}/test` для такого источника дополнительно проверяет право записи — без создания файлов. - У источника задаётся `write_template` — шаблон пути для новых файлов, с автоподбором по существующей структуре и проверкой обратимости через регэкспы источника (подробно — [uploads.md](uploads.md#7-раскладка-в-хранилище-как-принято-на-сервере)). @@ -44,8 +43,11 @@ - Агент: `POST /internal/files` `{source_id, local_path?, rel_path, staging_path, sha256}` → `{rel_path, size, renamed}`; `POST /internal/files/test` — проверка записи. NFS с `accepts_uploads` монтируется rw в `/library-rw/` (каталог внутри контейнера агента), а в `/library/` — bind этой точки с `remount,ro`. Local-источники пишутся через - `/library-host-rw` (тот же `LIBRARY_HOST`, на запись — только у агента). Итоговое имя создаётся жёсткой ссылкой - на временный файл (`link` не перезаписывает, в отличие от `rename`); занято — ` (2)`, ` (3)`… + `/library-host-rw` (тот же `LIBRARY_HOST`, на запись — только у агента). На музыкальный диск не пишется ничего, + кроме самих треков: файл создаётся сразу под итоговым именем (`O_EXCL` — существующий не перезаписывается; + занято — ` (2)`, ` (3)`…), `fsync`, сверка SHA-256; при ошибке агент удаляет свой недописанный файл и созданные + для него пустые каталоги. Проверка записи — `access(W_OK)` и флаг read-only файловой системы, без пробного файла. + Остальные сервисы видят музыку только на чтение (`read_only` в compose). Владелец новых файлов и каталогов — `PIPU_FILE_UID` / `PIPU_FILE_GID` (`-1` — не менять). - Шаблон: `meta.ParseTemplate` / `Render` (очистка имён, пределы длины), `meta.SuggestTemplate` (регэксп → шаблон через `regexp/syntax`: именованные группы → поля, `(?:(?P\d)-)?` → `{disc:1-}`, необязательная группа → `[ ]`, diff --git a/services/api/go.mod b/services/api/go.mod index 9506bfc..c8581ca 100644 --- a/services/api/go.mod +++ b/services/api/go.mod @@ -6,6 +6,7 @@ tool github.com/oapi-codegen/oapi-codegen/v2/cmd/oapi-codegen require ( github.com/caarlos0/env/v11 v11.3.1 + github.com/cespare/xxhash/v2 v2.3.0 github.com/getkin/kin-openapi v0.127.0 github.com/go-chi/chi/v5 v5.2.2 github.com/golang-jwt/jwt/v5 v5.2.2 @@ -16,11 +17,12 @@ require ( github.com/redis/go-redis/v9 v9.10.0 github.com/stretchr/testify v1.10.0 golang.org/x/crypto v0.38.0 + golang.org/x/sys v0.33.0 + golang.org/x/text v0.25.0 ) require ( github.com/apapsch/go-jsonmerge/v2 v2.0.0 // indirect - github.com/cespare/xxhash/v2 v2.3.0 // indirect github.com/davecgh/go-spew v1.1.1 // indirect github.com/dgryski/go-rendezvous v0.0.0-20200823014737-9f7001d12a5f // indirect github.com/dprotaso/go-yit v0.0.0-20220510233725-9ba8df137936 // indirect @@ -43,8 +45,6 @@ require ( go.uber.org/multierr v1.11.0 // indirect golang.org/x/mod v0.17.0 // indirect golang.org/x/sync v0.14.0 // indirect - golang.org/x/sys v0.33.0 // indirect - golang.org/x/text v0.25.0 // indirect golang.org/x/tools v0.21.1-0.20240508182429-e35e4ccd0d2d // indirect gopkg.in/yaml.v2 v2.4.0 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect diff --git a/services/api/internal/storageagent/access_other.go b/services/api/internal/storageagent/access_other.go new file mode 100644 index 0000000..d8d91cd --- /dev/null +++ b/services/api/internal/storageagent/access_other.go @@ -0,0 +1,20 @@ +//go:build !unix + +package storageagent + +import ( + "errors" + "os" +) + +// canWrite — запасная проверка для не-Unix (только разработка): бит записи у каталога. +func canWrite(dir string) error { + info, err := os.Stat(dir) + if err != nil { + return err + } + if info.Mode().Perm()&0o200 == 0 { + return errors.New("directory is not writable") + } + return nil +} diff --git a/services/api/internal/storageagent/access_unix.go b/services/api/internal/storageagent/access_unix.go new file mode 100644 index 0000000..70a3937 --- /dev/null +++ b/services/api/internal/storageagent/access_unix.go @@ -0,0 +1,18 @@ +//go:build unix + +package storageagent + +import ( + "errors" + + "golang.org/x/sys/unix" +) + +// canWrite — можно ли создавать файлы в каталоге (без пробной записи); read-only монтирование — ошибка. +func canWrite(dir string) error { + var st unix.Statfs_t + if err := unix.Statfs(dir, &st); err == nil && st.Flags&unix.ST_RDONLY != 0 { + return errors.New("filesystem is mounted read-only") + } + return unix.Access(dir, unix.W_OK|unix.X_OK) +} diff --git a/services/api/internal/storageagent/files.go b/services/api/internal/storageagent/files.go index 9239fc0..cd145f1 100644 --- a/services/api/internal/storageagent/files.go +++ b/services/api/internal/storageagent/files.go @@ -142,27 +142,19 @@ func (a *agent) writeFile(w http.ResponseWriter, r *http.Request) { writeJSON(w, http.StatusOK, res) } -// write копирует staging во временный файл рядом с целью, fsync, сверяет SHA-256 и создаёт итоговое имя -// жёсткой ссылкой (не перезаписывает существующий файл, в отличие от rename), затем убирает временный. +// write копирует staging сразу под итоговым именем (O_EXCL — существующий файл не перезаписывается, при +// конфликте — « (2)»), fsync, сверяет SHA-256 копии. На музыкальный диск не попадает ничего, кроме самого трека: +// ни временных файлов, ни служебных пометок. При любой ошибке недописанный файл и созданные для него пустые +// каталоги удаляются — агент убирает только то, что создал сам. func (a *agent) write(root, rel, staging string, want []byte) (WriteResult, error) { - target := filepath.Join(root, filepath.FromSlash(rel)) - dir := filepath.Dir(target) - if err := a.mkdirs(root, dir); err != nil { - return WriteResult{}, err - } - tmp := filepath.Join(dir, "."+filepath.Base(target)+".pipu-tmp") - size, err := a.copyTemp(staging, tmp) - if err != nil { - _ = os.Remove(tmp) - return WriteResult{}, err - } - defer func() { _ = os.Remove(tmp) }() - got, err := fileSHA256(tmp) + dir := filepath.Dir(filepath.Join(root, filepath.FromSlash(rel))) + created, err := a.mkdirs(root, dir) if err != nil { return WriteResult{}, err } - if !bytes.Equal(got, want) { - return WriteResult{}, fmt.Errorf("checksum mismatch: copy %x, expected %x", got, want) + fail := func(err error) (WriteResult, error) { + removeEmpty(created) + return WriteResult{}, err } ext := path.Ext(rel) @@ -172,24 +164,39 @@ func (a *agent) write(root, rel, staging string, want []byte) (WriteResult, erro if n > 1 { name = fmt.Sprintf("%s (%d)%s", base, n, ext) } - err := os.Link(tmp, filepath.Join(root, filepath.FromSlash(name))) + target := filepath.Join(root, filepath.FromSlash(name)) + dst, err := os.OpenFile(target, os.O_CREATE|os.O_EXCL|os.O_WRONLY, 0o644) + if os.IsExist(err) { + continue + } + if err != nil { + return fail(fmt.Errorf("create: %w", err)) + } + size, err := copyInto(dst, staging) if err == nil { - syncDir(dir) - return WriteResult{RelPath: name, Size: size, Renamed: n > 1}, nil + var got []byte + if got, err = fileSHA256(target); err == nil && !bytes.Equal(got, want) { + err = fmt.Errorf("checksum mismatch: copy %x, expected %x", got, want) + } } - if !os.IsExist(err) { - return WriteResult{}, fmt.Errorf("link: %w", err) + if err != nil { + _ = os.Remove(target) // наш собственный недописанный / испорченный файл + return fail(err) } + a.chown(target) + syncDir(dir) + return WriteResult{RelPath: name, Size: size, Renamed: n > 1}, nil } - return WriteResult{}, errors.New("too many files with the same name") + return fail(errors.New("too many files with the same name")) } -// mkdirs создаёт недостающие каталоги от root до dir (0755, владелец — FileUID/FileGID). -func (a *agent) mkdirs(root, dir string) error { +// mkdirs создаёт недостающие каталоги от root до dir (0755, владелец — FileUID/FileGID) и возвращает созданные. +func (a *agent) mkdirs(root, dir string) ([]string, error) { rel, err := filepath.Rel(root, dir) if err != nil || strings.HasPrefix(rel, "..") { - return errBadPath + return nil, errBadPath } + var created []string cur := root for _, seg := range strings.Split(rel, string(filepath.Separator)) { if seg == "." || seg == "" { @@ -200,24 +207,30 @@ func (a *agent) mkdirs(root, dir string) error { if os.IsExist(err) { continue } - return fmt.Errorf("mkdir: %w", err) + removeEmpty(created) + return nil, fmt.Errorf("mkdir: %w", err) } + created = append(created, cur) a.chown(cur) } - return nil + return created, nil } -func (a *agent) copyTemp(staging, tmp string) (int64, error) { +// removeEmpty удаляет созданные каталоги от глубоких к верхним; os.Remove не трогает непустые. +func removeEmpty(dirs []string) { + for i := len(dirs) - 1; i >= 0; i-- { + _ = os.Remove(dirs[i]) + } +} + +// copyInto копирует staging в открытый файл и делает fsync; файл закрывается. +func copyInto(dst *os.File, staging string) (int64, error) { src, err := os.Open(staging) if err != nil { + _ = dst.Close() return 0, fmt.Errorf("staging: %w", err) } defer func() { _ = src.Close() }() - _ = os.Remove(tmp) // остаток прерванной попытки - dst, err := os.OpenFile(tmp, os.O_CREATE|os.O_EXCL|os.O_WRONLY, 0o644) - if err != nil { - return 0, fmt.Errorf("create temp: %w", err) - } n, err := io.Copy(dst, src) if err == nil { err = dst.Sync() @@ -228,7 +241,6 @@ func (a *agent) copyTemp(staging, tmp string) (int64, error) { if err != nil { return 0, fmt.Errorf("copy: %w", err) } - a.chown(tmp) return n, nil } @@ -240,7 +252,8 @@ func (a *agent) chown(p string) { } } -// testWrite создаёт и удаляет .pipu-write-test в корне источника на запись. +// testWrite проверяет право записи в корень источника, ничего не создавая на диске (access(W_OK); +// на NFS ответ даёт сам сервер с учётом root_squash и прав экспорта). func (a *agent) testWrite(w http.ResponseWriter, r *http.Request) { var req WriteTestRequest if !a.decode(w, r, &req) { @@ -251,16 +264,7 @@ func (a *agent) testWrite(w http.ResponseWriter, r *http.Request) { writeJSON(w, http.StatusOK, WriteTestResult{Message: err.Error()}) return } - p := filepath.Join(root, ".pipu-write-test") - f, err := os.OpenFile(p, os.O_CREATE|os.O_TRUNC|os.O_WRONLY, 0o644) - if err == nil { - _, err = f.WriteString("pipu") - if cerr := f.Close(); err == nil { - err = cerr - } - } - _ = os.Remove(p) - if err != nil { + if err := canWrite(root); err != nil { writeJSON(w, http.StatusOK, WriteTestResult{Message: "cannot write: " + err.Error()}) return } diff --git a/services/api/internal/storageagent/files_test.go b/services/api/internal/storageagent/files_test.go index 830d093..163648c 100644 --- a/services/api/internal/storageagent/files_test.go +++ b/services/api/internal/storageagent/files_test.go @@ -9,6 +9,7 @@ import ( "net/http/httptest" "os" "path/filepath" + "strings" "testing" "github.com/stretchr/testify/assert" @@ -74,6 +75,7 @@ func TestWriteFile(t *testing.T) { assert.True(t, res.Renamed) entries, _ := os.ReadDir(dir) assert.Len(t, entries, 2) + assertOnlyMusic(t, filepath.Join(e.paths.LibraryRW, srcID)) // Local-источник пишется в LIBRARY_HOST. local := "music" @@ -103,8 +105,21 @@ func TestWriteFileRejects(t *testing.T) { other := sha256.Sum256([]byte("other")) rec := e.post("/internal/files", WriteRequest{SourceID: srcID, RelPath: "A/x.flac", StagingPath: "u1/f1", SHA256: hex.EncodeToString(other[:])}) assert.Equal(t, http.StatusInternalServerError, rec.Code) - entries, _ := os.ReadDir(filepath.Join(e.paths.LibraryRW, srcID, "A")) - assert.Empty(t, entries) + // Созданная под файл папка тоже убрана: на диске не остаётся следов неудачной записи. + assert.NoDirExists(t, filepath.Join(e.paths.LibraryRW, srcID, "A")) + assertOnlyMusic(t, filepath.Join(e.paths.LibraryRW, srcID)) +} + +// assertOnlyMusic — в дереве хранилища только аудиофайлы и каталоги: ни временных, ни служебных файлов. +func assertOnlyMusic(t *testing.T, root string) { + t.Helper() + require.NoError(t, filepath.WalkDir(root, func(p string, d os.DirEntry, err error) error { + if err != nil || d.IsDir() { + return err + } + assert.True(t, audioExt[strings.ToLower(filepath.Ext(p))] && !strings.HasPrefix(d.Name(), "."), "unexpected file %s", p) + return nil + })) } func TestWritableMount(t *testing.T) { @@ -120,7 +135,8 @@ func TestWritableMount(t *testing.T) { var res WriteTestResult require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &res)) assert.True(t, res.OK, res.Message) - assert.NoFileExists(t, filepath.Join(rw, ".pipu-write-test")) + entries, _ := os.ReadDir(rw) + assert.Empty(t, entries, "write check creates nothing") // Без accepts_uploads rw-точка снимается. e.post("/internal/mounts", MountRequest{SourceID: srcID, NFSSpec: NFSSpec{Server: "nas", Export: "/music"}})