diff --git a/cmd/httpd/main.go b/cmd/httpd/main.go index 0d69c22..9503b57 100644 --- a/cmd/httpd/main.go +++ b/cmd/httpd/main.go @@ -105,16 +105,17 @@ func main() { Validate: preview.ValidateSettings, }) lib := library.NewWithCatalog(catalog.Write, catalog.Read, appSettings) - // Hold exclusive ownership of disposable Find staging for the service lifetime. - releaseIdenticalTemp, err := lib.PrepareIdenticalTemp(conf.Paths.Work) + // Configure one disposable root and keep age-based cleanup within the service lifetime. + if err := lib.PrepareTemporary(conf.Paths.Work); err != nil { + panic(err) + } + stopTemporaryCleanup, err := resource.StartTemporaryCleanup(tools.ShutdownContext, conf.Paths.Work, func(err error) { + logrus.WithError(err).Warn("clean stale temporary files failed") + }) if err != nil { panic(err) } - defer func() { - if err := releaseIdenticalTemp(); err != nil { - log.Printf("release identical result lock failed: %v", err) - } - }() + defer stopTemporaryCleanup() if err := lib.AutoMigrate(); err != nil { panic(err) } diff --git a/docs/architecture/library.md b/docs/architecture/library.md index d1995ef..f17b5fa 100644 --- a/docs/architecture/library.md +++ b/docs/architecture/library.md @@ -186,7 +186,7 @@ Location Trash remains browsable. User payloads can Move to ordinary directories The [identical tools](../../entity/files.proto) query recorded signatures only. Library includes ordinary regular Files, including unbacked Files, and excludes logical Trash. Each File contributes its current known original signature and every saved version signature. An unsigned original adds no matching edge, but its Location and path remain part of the fingerprint when saved versions connect its File to a Library group. Connected components include transitive matches; they do not imply that all current originals are identical. Locations groups use only current recorded signatures from explicitly selected whole Locations; repeated selections contribute once. No query starts Scan, computes hashes or publishes associations. -[Query staging](../../internal/library/identical_query.go) streams catalog facts into disposable SQLite tables through GORM. Signature minima settle small components; unresolved long chains use bounded disk-backed traversal without pairwise member expansion. One explicit Find retains the complete result temporarily. Dense positions index each retained order once. Size sorting orders whole groups by their largest visible recorded member size, with unknown groups last and component ID breaking ties; members stay in ascending path order in either size direction. File ID and name modes keep title-ordered groups and sort members within them. Paths and names compare case-insensitively, then by exact bytes and File ID for stable ties. Size comes from the recorded original, or the latest saved version when there is no original. When hidden files are excluded, each group uses its first visible member for the title. A bounded row read or position lookup reuses the requested order without rebuilding components. Group/member continuations retain their File ID order. Continuation cursors require the result ID; a missing or expired ID fails without collecting again. The service retains at most two results and expires each after 30 minutes idle. Find and targeted component SQLite files live in a private directory under configured `paths.work`; a lifetime lock excludes another service using that directory, and startup removes only its abandoned result directories. Clean shutdown closes retained results before releasing the lock. A restart invalidates result IDs, so an unavailable result requires another explicit Find. Older directories in shared system temporary storage are never swept automatically. The lock guards only disposable staging cleanup, not Library operations. No result enters Library backups or changes persistent data formats. Group fingerprints cover only their own membership, matching signatures and recorded originals. Keep and Merge independently validate the current connected component before changing it. Member evidence is a bounded sample of shared current/version signatures; only requested members receive live observations. +[Query staging](../../internal/library/identical_query.go) streams catalog facts into disposable SQLite tables through GORM. Signature minima settle small components; unresolved long chains use bounded disk-backed traversal without pairwise member expansion. One explicit Find retains the complete result temporarily. Dense positions index each retained order once. Size sorting orders whole groups by their largest visible recorded member size, with unknown groups last and component ID breaking ties; members stay in ascending path order in either size direction. File ID and name modes keep title-ordered groups and sort members within them. Paths and names compare case-insensitively, then by exact bytes and File ID for stable ties. Size comes from the recorded original, or the latest saved version when there is no original. When hidden files are excluded, each group uses its first visible member for the title. A bounded row read or position lookup reuses the requested order without rebuilding components. Group/member continuations retain their File ID order. Continuation cursors require the result ID; a missing or expired ID fails without collecting again. The service retains at most two results and expires each after 30 minutes idle. Find and targeted component SQLite files use the shared [temporary ownership](persistence.md#catalog-connections) under configured `paths.work/temporary`: closing a result removes its files, and startup/hourly cleanup removes abandoned directories older than seven days while retaining process-owned work. A restart invalidates result IDs, so an unavailable result requires another explicit Find. Older directories in shared system temporary storage are never swept automatically. No result enters Library backups or changes persistent data formats. Group fingerprints cover only their own membership, matching signatures and recorded originals. Keep and Merge independently validate the current connected component before changing it. Member evidence is a bounded sample of shared current/version signatures; only requested members receive live observations. **Merge into** selects one target and merges its complete authoritative Library group in a metadata transaction. The target keeps its name, logical path, identity and original. Sources retain their identities in separate folders under the existing Library Trash checkpoint, with original/tracking associations detached; physical files are untouched. The response counts source Files retired by the committed transaction; a dry run reports the number it would retire. Existing tag union and distinct-Note rules apply. Versions are combined by signature, preferring target metadata, then ascending source File/version IDs. Signature equality determines version identity; the preferred version keeps its content metadata unchanged. Merge does not validate or repair conflicting size/hash records. Saved-time observations are unioned and endpoints recomputed, without inventing a save at merge time. The write transaction protects the related metadata changes from partial database-write failure; it does not inspect unrelated Job bundles. diff --git a/docs/architecture/persistence.md b/docs/architecture/persistence.md index e96ac4d..7d33afa 100644 --- a/docs/architecture/persistence.md +++ b/docs/architecture/persistence.md @@ -121,3 +121,5 @@ Restore publishes Library associations before its Job checkpoint. If that checkp File operations update their original associations and return the request outcome without a persistent result table or automatic metadata retry. Their temporary manifest is removed at request end. [Physical-operation outcomes](library.md#physical-file-operations) define partial failure and explicit follow-up; no filesystem rollback is attempted. [Recovery](../../internal/executor/recovery.go) removes only literally empty incomplete Job directories. A missing bundle preserves its catalog entry and reports an error. Missing metadata/state files, invalid manifests/configuration, symlinks and nonempty orphan bundles are reported and preserved for inspection, not recursively deleted as unfinished creations. [Catalog updates](../../internal/executor/job.go) and [kind-specific runners](jobs.md) define the implemented checkpoints. [Offline migration](../operations/migration.md) owns temporary staging/backup tables; those tables are not part of the runtime schema. + +Disposable files use the shared Work temporary directory with timestamped names, explicit active ownership and seven-day expiry. Cleanup retains failed removals for retry; Restore name probes remain on their target filesystem and are tracked by that owner. diff --git a/internal/apis/catalog_test.go b/internal/apis/catalog_test.go index b2c13b6..c047af8 100644 --- a/internal/apis/catalog_test.go +++ b/internal/apis/catalog_test.go @@ -27,6 +27,7 @@ func TestVersionMetadataSurvivesCorruptPreview(t *testing.T) { db, err := resource.OpenSQLite(filepath.Join(root, "library.db")) require.NoError(t, err) lib := library.New(db) + require.NoError(t, lib.PrepareTemporary(root)) require.NoError(t, lib.AutoMigrate()) file := &library.File{Name: "photo.jpg", Kind: entity.FileKind_FILE_KIND_REGULAR} require.NoError(t, lib.SaveFile(ctx, file)) @@ -62,6 +63,7 @@ func TestImportArchiveDirectoryReportsBoundedResults(t *testing.T) { db, err := resource.OpenSQLite(filepath.Join(root, "library.db")) require.NoError(t, err) lib := library.New(db) + require.NoError(t, lib.PrepareTemporary(root)) require.NoError(t, lib.AutoMigrate()) media, err := lib.CreateMedia(ctx, &library.Media{Kind: entity.MediaKind_MEDIA_KIND_VOLUME, Identity: "b95355a8-e71d-4528-a49d-434724d56e41", Name: "Archive", @@ -106,6 +108,7 @@ func TestFileResponsesTolerateCatalogReplacement(t *testing.T) { db, err := resource.OpenSQLite(filepath.Join(root, "library.db")) require.NoError(t, err) lib := library.New(db) + require.NoError(t, lib.PrepareTemporary(root)) require.NoError(t, lib.AutoMigrate()) parent := &library.File{Name: "folder", Kind: entity.FileKind_FILE_KIND_DIRECTORY} require.NoError(t, lib.SaveFile(ctx, parent)) diff --git a/internal/apis/identical_keep.go b/internal/apis/identical_keep.go index 2011d1d..f1d3a50 100644 --- a/internal/apis/identical_keep.go +++ b/internal/apis/identical_keep.go @@ -63,7 +63,7 @@ func (s *filesService) keepAdmitted(req *entity.KeepIdenticalRequest, scope libr if group.Fingerprint != req.Fingerprint { return library.ErrLocationConflict } - stage, closeStage, err := openKeepStage(ctx) + stage, closeStage, err := s.openKeepStage(ctx) if err != nil { return err } @@ -179,9 +179,9 @@ func reportKeepPlan(ctx context.Context, stage *gorm.DB, summary *entity.FileOpe return stream.Send(&entity.FileOperationResult{Summary: summary}) } -func openKeepStage(ctx context.Context) (*gorm.DB, func() error, error) { +func (s *filesService) openKeepStage(ctx context.Context) (*gorm.DB, func() error, error) { // Request-owned staging is disposable and never part of Library backup or Job recovery. - stage, err := resource.OpenTemporaryDB("", "yatm-keep-") + stage, err := resource.OpenTemporaryDB(s.api.exe.Paths().Work, "yatm-keep-") if err != nil { return nil, nil, err } diff --git a/internal/apis/identical_rows_test.go b/internal/apis/identical_rows_test.go index 9927fa0..ed6aa25 100644 --- a/internal/apis/identical_rows_test.go +++ b/internal/apis/identical_rows_test.go @@ -11,6 +11,7 @@ import ( "github.com/samuelncui/yatm/entity" "github.com/samuelncui/yatm/internal/executor/fileops" + "github.com/samuelncui/yatm/internal/resource" "github.com/stretchr/testify/require" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" @@ -73,8 +74,7 @@ func TestIdenticalContinuationsRequireRetainedResult(t *testing.T) { cacheContentSignature(t, context.Background(), filename) } admittedPage(t, &locationService{api: s.api}, scope.Roots[0].LocationId) - temp := t.TempDir() - t.Setenv("TMPDIR", temp) + temp := resource.TemporaryRoot(s.api.exe.Paths().Work) // Both legacy first-page methods still work without a result ID. groups, err := s.ListIdenticalGroups(context.Background(), &entity.ListIdenticalGroupsRequest{Scope: scope, Limit: 1}) @@ -120,8 +120,7 @@ func TestIdenticalContinuationsRequireRetainedResult(t *testing.T) { func TestIdenticalServiceCloseRemovesRetainedSQLite(t *testing.T) { // Clean shutdown releases this process's temporary results after requests have drained. s, scope, _ := identicalFixture(t) - temp := t.TempDir() - t.Setenv("TMPDIR", temp) + temp := resource.TemporaryRoot(s.api.exe.Paths().Work) for range 2 { _, err := s.FindIdentical(context.Background(), &entity.FindIdenticalRequest{Scope: scope}) require.NoError(t, err) diff --git a/internal/apis/location_test.go b/internal/apis/location_test.go index e070f19..7fbb232 100644 --- a/internal/apis/location_test.go +++ b/internal/apis/location_test.go @@ -16,7 +16,7 @@ import ( ) func setupLocationAPI(t *testing.T) (*API, *locationService, string) { - // Use an isolated permitted namespace with explicit runtime resources below it. + // Use an isolated permitted namespace with separate runtime resources. t.Helper() root, err := filepath.EvalSymlinks(t.TempDir()) require.NoError(t, err) @@ -27,8 +27,10 @@ func setupLocationAPI(t *testing.T) (*API, *locationService, string) { Validate: preview.ValidateSettings, }) l := library.NewWithSettings(db, settings) + work := t.TempDir() + require.NoError(t, l.PrepareTemporary(work)) require.NoError(t, l.AutoMigrate()) - exe := executor.New(db, l, nil, executor.Paths{Access: []executor.AccessRange{{Root: root}}, Work: filepath.Join(root, "work")}, executor.Scripts{}, nil) + exe := executor.New(db, l, nil, executor.Paths{Access: []executor.AccessRange{{Root: root}}, Work: work}, executor.Scripts{}, nil) api := New(l, exe) return api, &locationService{api: api}, root } diff --git a/internal/apis/media_inspect_test.go b/internal/apis/media_inspect_test.go index 7e93711..6c5fcb6 100644 --- a/internal/apis/media_inspect_test.go +++ b/internal/apis/media_inspect_test.go @@ -28,7 +28,7 @@ func TestMediaInspectTapePrefersDeviceBarcodeAndReturnsLibraryStats(t *testing.T require.NoError(t, lib.AutoMigrate()) readInfo := filepath.Join(root, "read-info") require.NoError(t, os.WriteFile(readInfo, []byte("#!/bin/sh\nset -eu\nprintf '%s\\n' '{\"barcode\":\"ABC001\"}' > \"$OUT\"\n"), 0o755)) - exe := executor.New(executorDB, lib, []string{"/dev/nst0"}, executor.Paths{}, executor.Scripts{ReadInfo: readInfo}, nil) + exe := executor.New(executorDB, lib, []string{"/dev/nst0"}, executor.Paths{Work: filepath.Join(root, "work")}, executor.Scripts{ReadInfo: readInfo}, nil) require.NoError(t, exe.AutoMigrate()) api := apis.New(lib, exe) @@ -79,7 +79,7 @@ func TestMediaInspectTapeUsesManualBarcodeWhenDeviceHasNone(t *testing.T) { require.NoError(t, lib.AutoMigrate()) readInfo := filepath.Join(root, "read-info") require.NoError(t, os.WriteFile(readInfo, []byte("#!/bin/sh\nset -eu\nprintf '%s\\n' '{\"barcode\":\"\"}' > \"$OUT\"\n"), 0o755)) - exe := executor.New(executorDB, lib, []string{"/dev/nst0"}, executor.Paths{}, executor.Scripts{ReadInfo: readInfo}, nil) + exe := executor.New(executorDB, lib, []string{"/dev/nst0"}, executor.Paths{Work: filepath.Join(root, "work")}, executor.Scripts{ReadInfo: readInfo}, nil) require.NoError(t, exe.AutoMigrate()) api := apis.New(lib, exe) diff --git a/internal/demo/demo.go b/internal/demo/demo.go index 4412d09..01009fc 100644 --- a/internal/demo/demo.go +++ b/internal/demo/demo.go @@ -114,12 +114,15 @@ func Prepare(ctx context.Context, options Options) (returnErr error) { } }() - // Configure the same Library settings and Preview manager used by the small fixture. + // Bind Library preferences and temporary work before admitting fixture operations. appSettings := settingspkg.New(db, settingspkg.PreviewDefinition{ Default: func() (*entity.PreviewSettings, error) { return preview.SettingsFromConfig(preview.Config{}) }, Validate: preview.ValidateSettings, }) lib := library.NewWithSettings(db, appSettings) + if err := lib.PrepareTemporary(paths.Work); err != nil { + return fmt.Errorf("prepare Demo Library temporary storage failed, %w", err) + } if err := lib.AutoMigrate(); err != nil { return fmt.Errorf("migrate Demo Library failed, %w", err) } diff --git a/internal/demo/demo_test.go b/internal/demo/demo_test.go index e91b8ba..c70910a 100644 --- a/internal/demo/demo_test.go +++ b/internal/demo/demo_test.go @@ -75,6 +75,7 @@ func TestPrepareCreatesOperableReviewFixture(t *testing.T) { // Curated Files retain independent organization and annotations. lib := library.New(db) + require.NoError(t, lib.PrepareTemporary(filepath.Join(root, "work"))) budget, err := lib.GetByPath(context.Background(), library.Root.ID, "Projects/aurora/budget.csv") require.NoError(t, err) require.Equal(t, "Draft budget; finance review is still pending.", budget.Note) diff --git a/internal/demo/duplicates_test.go b/internal/demo/duplicates_test.go index 921b6c0..555fa19 100644 --- a/internal/demo/duplicates_test.go +++ b/internal/demo/duplicates_test.go @@ -167,6 +167,7 @@ func TestPrepareScaledIdenticalFixture(t *testing.T) { require.NoError(t, err) t.Cleanup(func() { require.NoError(t, sqlDB.Close()) }) lib := library.New(db) + require.NoError(t, lib.PrepareTemporary(filepath.Join(root, "work"))) // Scaling adds only regular Files; Locations, retained Jobs and curated annotations stay small. requireTableCount(t, db, library.ModelFile, 363+1001-207) diff --git a/internal/executor/executor.go b/internal/executor/executor.go index fc056d6..7ba3a07 100644 --- a/internal/executor/executor.go +++ b/internal/executor/executor.go @@ -273,7 +273,7 @@ func (e *Executor) createPreparedJob(ctx context.Context, kind entity.JobKind, p if rerr == nil { return } - e.cleanupFailedJob(ctx, job.ID) + rerr = errors.Join(rerr, e.cleanupFailedJob(ctx, job.ID)) }() // Index the primary target before exposing the completed bundle to catalog searches. diff --git a/internal/executor/fileops/operation.go b/internal/executor/fileops/operation.go index b10f646..cd1fdd7 100644 --- a/internal/executor/fileops/operation.go +++ b/internal/executor/fileops/operation.go @@ -3,7 +3,6 @@ package fileops import ( "context" - "github.com/google/uuid" "github.com/samuelncui/yatm/internal/executor" "github.com/samuelncui/yatm/internal/resource" ) @@ -14,9 +13,8 @@ type operation struct { } func newOperation(ctx context.Context, exe *executor.Executor) (*operation, error) { - // The shared resource owns cleanup; the operation keeps its existing temporary namespace. - prefix := ".yatm-fileops-" + uuid.NewString() + "-" - temporary, err := resource.OpenTemporaryDB("", prefix) + // The request owns its disposable manifest through result delivery. + temporary, err := resource.OpenTemporaryDB(exe.Paths().Work, "file-operation-") if err != nil { return nil, err } diff --git a/internal/executor/fileops/operation_test.go b/internal/executor/fileops/operation_test.go index 32f9270..25ec510 100644 --- a/internal/executor/fileops/operation_test.go +++ b/internal/executor/fileops/operation_test.go @@ -41,6 +41,7 @@ func setup(t *testing.T) *fixture { t.Cleanup(func() { require.NoError(t, sqlDB.Close()) }) } lib := library.New(libDB) + require.NoError(t, lib.PrepareTemporary(work)) require.NoError(t, lib.AutoMigrate()) exe := executor.New(executorDB, lib, nil, executor.Paths{Work: work, Access: []executor.AccessRange{{Root: root}}}, executor.Scripts{}, nil) require.NoError(t, exe.AutoMigrate()) @@ -53,12 +54,12 @@ func setup(t *testing.T) *fixture { var count int64 require.NoError(t, executorDB.Model(executor.ModelJob).Count(&count).Error) require.Zero(t, count, "ordinary file operations must never create catalog Jobs") - entries, err := os.ReadDir(work) + entries, err := os.ReadDir(resource.TemporaryRoot(work)) if os.IsNotExist(err) { return } require.NoError(t, err) - require.Empty(t, entries, "ordinary file operations must not create Job bundles") + require.Empty(t, entries, "ordinary file operations must immediately remove disposable manifests") }) return f } @@ -372,22 +373,15 @@ func TestDestinationReplacementBetweenItemsStaysOrdinaryConcurrency(t *testing.T require.FileExists(t, filepath.Join(f.root, "destination/b.txt")) } -func TestActiveOperationManifestIsHiddenByDefaultIgnore(t *testing.T) { - // The temporary database retains the operation namespace used by default Location Ignore. +func TestActiveOperationManifestUsesProtectedWork(t *testing.T) { + // The operation owns a disposable manifest inside protected Work storage. f := setup(t) op, err := newOperation(context.Background(), f.exe) require.NoError(t, err) defer op.Close() - directory, err := os.MkdirTemp(f.root, filepath.Base(op.Directory)+"-") - require.NoError(t, err) - defer os.RemoveAll(directory) - require.NoError(t, os.WriteFile(filepath.Join(directory, "content"), []byte("unfinished"), 0600)) + require.Equal(t, resource.TemporaryRoot(f.work), filepath.Dir(op.Directory)) - // The default Location Ignore rules hide the operation namespace from ordinary reads. - relative := filepath.Base(directory) + "/content" - require.True(t, f.location.Excluded(relative, false)) - - // Ignore is visibility, not permission: an explicit observation stays available. - _, err = f.exe.ObserveLocationEntry(context.Background(), f.location.ID, relative) - require.NoError(t, err) + // A runtime temporary directory cannot become an authorized original Location. + _, err = f.exe.LocationRoot(op.Directory) + require.Error(t, err) } diff --git a/internal/executor/job.go b/internal/executor/job.go index a775e4c..5f7146e 100644 --- a/internal/executor/job.go +++ b/internal/executor/job.go @@ -190,11 +190,15 @@ func (e *Executor) deleteJob(ctx context.Context, id int64) error { return err } - // Close cached resources before removing the bundle. - if runner := e.removeRunner(id); runner != nil { + // Retain cached resources until their owner confirms cleanup succeeded. + e.runnersLock.Lock() + runner := e.runners[id] + e.runnersLock.Unlock() + if runner != nil { if err := runner.Close(); err != nil { return fmt.Errorf("close job runner failed, id=%d, %w", id, err) } + e.removeRunner(id) } if err := os.RemoveAll(e.jobWorkPath(id)); err != nil { return fmt.Errorf("remove job bundle failed, id=%d, %w", id, err) @@ -466,13 +470,28 @@ func jobPageSize(limit *int64) (int, error) { return int(*limit), nil } -func (e *Executor) cleanupFailedJob(ctx context.Context, jobID int64) { - // Remove the unpublished bundle and its catalog search entries together during failed creation. +func (e *Executor) cleanupFailedJob(ctx context.Context, jobID int64) error { + // Keep the runner reachable until it has released every file owned by failed creation. ctx = context.WithoutCancel(ctx) - if runner := e.removeRunner(jobID); runner != nil { - _ = runner.Close() + e.runnersLock.Lock() + runner := e.runners[jobID] + e.runnersLock.Unlock() + if runner != nil { + if err := runner.Close(); err != nil { + return fmt.Errorf("close failed Job runner, id=%d, %w", jobID, err) + } + e.removeRunner(jobID) } - _ = os.RemoveAll(e.jobWorkPath(jobID)) - _ = e.db.WithContext(ctx).Where("job_id = ?", jobID).Delete(&JobProperty{}).Error - _ = e.db.WithContext(ctx).Unscoped().Delete(&jobCatalogRow{}, jobID).Error + + // Remove unpublished storage before deleting its catalog owner; report each failure. + if err := os.RemoveAll(e.jobWorkPath(jobID)); err != nil { + return fmt.Errorf("remove failed Job bundle, id=%d, %w", jobID, err) + } + if err := e.db.WithContext(ctx).Where("job_id = ?", jobID).Delete(&JobProperty{}).Error; err != nil { + return fmt.Errorf("remove failed Job properties, id=%d, %w", jobID, err) + } + if err := e.db.WithContext(ctx).Unscoped().Delete(&jobCatalogRow{}, jobID).Error; err != nil { + return fmt.Errorf("remove failed Job catalog entry, id=%d, %w", jobID, err) + } + return nil } diff --git a/internal/executor/media_read_test.go b/internal/executor/media_read_test.go index c4cbdb5..99dca27 100644 --- a/internal/executor/media_read_test.go +++ b/internal/executor/media_read_test.go @@ -57,7 +57,7 @@ func TestTapeReadRejectsWrongExpectedIdentityBeforeMount(t *testing.T) { mutationMarker := filepath.Join(root, "mutated") readInfo := writeExecutorTestScript(t, "identity", `printf '%s\n' '{"barcode":"ABC001"}' > "$OUT"`) mutation := writeExecutorTestScript(t, "mutation", fmt.Sprintf(": > %s", strconv.Quote(mutationMarker))) - exe := New(nil, lib, []string{"fixture-device"}, Paths{}, Scripts{ReadInfo: readInfo, Encrypt: mutation, Mount: mutation}, nil) + exe := New(nil, lib, []string{"fixture-device"}, Paths{Work: t.TempDir()}, Scripts{ReadInfo: readInfo, Encrypt: mutation, Mount: mutation}, nil) require.NoError(t, exe.beginAttempt(1, func(error) {})) t.Cleanup(func() { exe.endAttempt(1) }) target := (&entity.ReadTapeTarget{Device: "fixture-device"}).Pack() diff --git a/internal/executor/restore/output_names.go b/internal/executor/restore/output_names.go index dd826aa..6a15853 100644 --- a/internal/executor/restore/output_names.go +++ b/internal/executor/restore/output_names.go @@ -2,37 +2,33 @@ package restore import ( "context" - "errors" "fmt" - "io" "os" "path/filepath" - "strconv" "strings" "github.com/google/uuid" + "github.com/samuelncui/yatm/internal/resource" ) // outputNames is an attempt-local filesystem namespace, never an output file or durable recovery log. type outputNames struct { - runner *jobRestoreRunner - index string - name string - groups int + runner *jobRestoreRunner + temporary *resource.TemporaryDirectory + name string } func newOutputNames(ctx context.Context, runner *jobRestoreRunner) (*outputNames, error) { - // Keep cleanup records on the Job's protected storage, not in an unbounded in-memory directory map. - work, err := runner.exe.EnsureJobWorkPath(ctx, runner.job.ID) - if err != nil { + // A disposable allocation outlives Job deletion while probes stay on the destination filesystem. + if err := ctx.Err(); err != nil { return nil, err } - index, err := os.MkdirTemp(work, "output-names-") + temporary, err := resource.NewTemporaryDirectory(runner.exe.Paths().Work, "restore-output-names-") if err != nil { return nil, err } prefix := ".yatm-restore-" + uuid.NewString() - return &outputNames{runner: runner, index: index, name: prefix + "-\u00e9"}, nil + return &outputNames{runner: runner, temporary: temporary, name: prefix + "-\u00e9"}, nil } func (n *outputNames) rebuild(ctx context.Context) error { @@ -141,23 +137,21 @@ func (n *outputNames) claimRelative(shadow, suffix string) (bool, error) { func (n *outputNames) directory(parent string) (string, error) { // One random basename under each actual parent naturally shares a namespace across parent aliases. shadow := filepath.Join(parent, n.name) - err := os.Mkdir(shadow, 0700) - if os.IsExist(err) { - info, inspectErr := os.Lstat(shadow) - if inspectErr != nil { - return "", inspectErr - } + info, err := os.Lstat(shadow) + if err == nil { if !info.IsDir() || info.Mode()&os.ModeSymlink != 0 { return "", fmt.Errorf("Restore name probe is not a real directory") } return shadow, nil } - if err != nil { + if !os.IsNotExist(err) { return "", err } - n.groups++ - if err := os.WriteFile(filepath.Join(n.index, strconv.Itoa(n.groups)), []byte(shadow), 0600); err != nil { - return "", errors.Join(err, os.Remove(shadow)) + if err := n.temporary.RegisterRestoreProbe(shadow); err != nil { + return "", err + } + if err := os.Mkdir(shadow, 0700); err != nil { + return "", err } // Check inherited case/normalization behavior in the actual directories, including network mounts. @@ -203,38 +197,5 @@ func directoryNameRules(directory, name string) ([2]bool, error) { } func (n *outputNames) close() error { - // Clean only exact probe directories recorded by this attempt, in bounded pages. - index, err := os.Open(n.index) - if err != nil { - return err - } - defer index.Close() - var cleanupErr error - for { - entries, err := index.ReadDir(batchSize) - if err != nil && err != io.EOF { - cleanupErr = errors.Join(cleanupErr, err) - break - } - for _, entry := range entries { - value, readErr := os.ReadFile(filepath.Join(n.index, entry.Name())) - if readErr != nil { - cleanupErr = errors.Join(cleanupErr, readErr) - continue - } - target := string(value) - if !filepath.IsAbs(target) || filepath.Base(target) != n.name { - cleanupErr = errors.Join(cleanupErr, fmt.Errorf("invalid Restore probe cleanup record")) - continue - } - cleanupErr = errors.Join(cleanupErr, os.RemoveAll(target)) - } - if err == io.EOF { - break - } - } - if cleanupErr != nil { - return cleanupErr - } - return os.RemoveAll(n.index) + return n.temporary.Close() } diff --git a/internal/executor/restore/output_names_test.go b/internal/executor/restore/output_names_test.go index 3c35219..5952dd9 100644 --- a/internal/executor/restore/output_names_test.go +++ b/internal/executor/restore/output_names_test.go @@ -8,9 +8,11 @@ import ( "os" "path/filepath" "testing" + "time" "github.com/samuelncui/yatm/entity" "github.com/samuelncui/yatm/internal/library" + "github.com/samuelncui/yatm/internal/resource" "github.com/stretchr/testify/require" ) @@ -55,7 +57,7 @@ func TestOutputNameProbesAreTemporaryAndContainNoRegularFiles(t *testing.T) { require.ErrorContains(t, err, "file/directory") require.NoError(t, names.close()) require.NoDirExists(t, probe) - require.NoDirExists(t, names.index) + require.Empty(t, names.temporary.Directory) // The default Ignore rule is a name pattern; it stays in effect after the probe is gone. require.True(t, source.Excluded(names.name, true)) } @@ -213,3 +215,135 @@ func TestRestoreNameOwnerLookupPagesTargetAncestors(t *testing.T) { require.NoError(t, err) require.False(t, claimed, "an indexed owner on the second ancestor page keeps its name") } + +func TestOutputNameRegistrationFailureCreatesNoExternalProbe(t *testing.T) { + // Registration must succeed on protected storage before allocating a destination namespace. + root := t.TempDir() + index := filepath.Join(root, "index") + require.NoError(t, os.WriteFile(index, []byte("not a directory"), 0600)) + names := &outputNames{temporary: &resource.TemporaryDirectory{Directory: index}, + name: ".yatm-restore-00000000-0000-4000-8000-000000000001-é"} + parent := t.TempDir() + _, err := names.directory(parent) + require.Error(t, err) + entries, err := os.ReadDir(parent) + require.NoError(t, err) + require.Empty(t, entries) +} + +func TestOutputNameProbesUseEachActualParentAndNativeRules(t *testing.T) { + // Existing nested parents receive their own native namespaces rather than a Work-side approximation. + temporary, err := resource.NewTemporaryDirectory(t.TempDir(), "restore-output-names-") + require.NoError(t, err) + names := &outputNames{temporary: temporary, name: ".yatm-restore-00000000-0000-4000-8000-000000000007-é"} + t.Cleanup(func() { require.NoError(t, names.close()) }) + root := t.TempDir() + nested := filepath.Join(root, "existing", "nested") + require.NoError(t, os.MkdirAll(nested, 0700)) + for _, parent := range []string{root, nested} { + shadow, err := names.directory(parent) + require.NoError(t, err) + require.Equal(t, parent, filepath.Dir(shadow)) + rules, err := directoryNameRules(parent, names.name) + require.NoError(t, err) + claimed, err := names.claimRelative(shadow, "File.txt") + require.NoError(t, err) + require.True(t, claimed) + claimed, err = names.claimRelative(shadow, "file.txt") + require.NoError(t, err) + require.Equal(t, !rules[0], claimed) + } +} + +func TestOutputNameFailedRemovalPreservesAllocationForRetry(t *testing.T) { + // The shared owner retains the only exact record when destination cleanup is denied. + if os.Geteuid() == 0 { + t.Skip("permission fixture requires an unprivileged user") + } + temporary, err := resource.NewTemporaryDirectory(t.TempDir(), "restore-output-names-") + require.NoError(t, err) + names := &outputNames{temporary: temporary, name: ".yatm-restore-00000000-0000-4000-8000-000000000003-é"} + parent := t.TempDir() + shadow, err := names.directory(parent) + require.NoError(t, err) + require.NoError(t, os.Chmod(parent, 0500)) + t.Cleanup(func() { + require.NoError(t, os.Chmod(parent, 0700)) + require.NoError(t, names.close()) + }) + index := temporary.Directory + require.ErrorIs(t, names.close(), os.ErrPermission) + require.Equal(t, index, temporary.Directory) + require.DirExists(t, shadow) + + // Another close uses the same exact cleanup records after permissions recover. + require.NoError(t, os.Chmod(parent, 0700)) + require.NoError(t, names.close()) + require.NoDirExists(t, shadow) + require.NoDirExists(t, index) +} + +func TestDeletingRestoreJobPreservesSharedProbeCleanupRecords(t *testing.T) { + // Job deletion cannot remove the allocation that owns a failed external cleanup. + ctx := context.Background() + exe, lib := setupTestExecutor(t) + _, file := createMediaFile(t, lib, newTapeMedia("DELETEPROBE"), ".", "file.txt", []byte("saved"), nil) + job := createRestoreJob(t, exe, file.ID) + waitIndexed(t, exe, job.ID) + value, err := exe.GetJobRunner(ctx, job.ID) + require.NoError(t, err) + names, err := newOutputNames(ctx, value.(*jobRestoreRunner)) + require.NoError(t, err) + t.Cleanup(func() { require.NoError(t, names.close()) }) + parent := filepath.Join(exe.Paths().Target, "nested") + require.NoError(t, os.Mkdir(parent, 0700)) + shadow, err := names.directory(parent) + require.NoError(t, err) + index := names.temporary.Directory + require.Equal(t, resource.TemporaryRoot(exe.Paths().Work), filepath.Dir(index)) + + // An unavailable target releases the live hold but preserves its exact cleanup record. + parked := parent + "-offline" + require.NoError(t, os.Rename(parent, parked)) + require.Error(t, names.close()) + _, err = exe.DeleteJobs(ctx, false, job.ID) + require.NoError(t, err) + require.DirExists(t, index) + require.DirExists(t, filepath.Join(parked, names.name)) + require.Error(t, resource.CleanupTemporary(exe.Paths().Work, time.Now().Add(8*24*time.Hour))) + require.DirExists(t, index) + + // Once the same destination is available, ordinary expiry removes its directory-only probe. + require.NoError(t, os.Rename(parked, parent)) + require.NoError(t, resource.CleanupTemporary(exe.Paths().Work, time.Now().Add(8*24*time.Hour))) + require.NoDirExists(t, shadow) + require.NoDirExists(t, index) +} + +func TestClosingDuplicateRestoreRunnerPreservesActiveNameProbes(t *testing.T) { + // A factory instance that loses cache publication owns only its own handles. + ctx := context.Background() + exe, lib := setupTestExecutor(t) + _, file := createMediaFile(t, lib, newTapeMedia("DUPLICATE"), ".", "file.txt", []byte("saved"), nil) + job := createRestoreJob(t, exe, file.ID) + waitIndexed(t, exe, job.ID) + value, err := exe.GetJobRunner(ctx, job.ID) + require.NoError(t, err) + names, err := newOutputNames(ctx, value.(*jobRestoreRunner)) + require.NoError(t, err) + t.Cleanup(func() { require.NoError(t, names.close()) }) + shadow, err := names.directory(exe.Paths().Target) + require.NoError(t, err) + claimed, err := names.claimRelative(shadow, "reserved.txt") + require.NoError(t, err) + require.True(t, claimed) + + // GetJobRunner closes a redundant constructed instance; that must not erase the winner's index. + duplicate, err := newRunner(ctx, exe, job) + require.NoError(t, err) + require.NoError(t, duplicate.Close()) + require.DirExists(t, names.temporary.Directory) + claimed, err = names.claimRelative(shadow, "reserved.txt") + require.NoError(t, err) + require.False(t, claimed) +} diff --git a/internal/executor/tape.go b/internal/executor/tape.go index f7f046c..be6b4d6 100644 --- a/internal/executor/tape.go +++ b/internal/executor/tape.go @@ -17,7 +17,7 @@ func (e *Executor) ReadTapeBarcode(ctx context.Context, device string) (string, // Run the configured device probe and decode its stable JSON boundary. cmd := exec.CommandContext(ctx, e.scripts.ReadInfo) cmd.Env = append(cmd.Env, fmt.Sprintf("DEVICE=%s", device)) - data, err := tools.RunCmdWithReturn(logrus.StandardLogger(), cmd) + data, err := tools.RunCmdWithReturn(logrus.StandardLogger(), cmd, e.paths.Work) if err != nil { return "", fmt.Errorf("read tape info failed, %w", err) } diff --git a/internal/executor/tape_test.go b/internal/executor/tape_test.go index 3720c20..35b8075 100644 --- a/internal/executor/tape_test.go +++ b/internal/executor/tape_test.go @@ -43,7 +43,7 @@ func TestReadTapeBarcodeValidatesScriptResult(t *testing.T) { script := writeExecutorTestScript(t, "read-info", fmt.Sprintf( "printf '%%s\\n' %s > \"$OUT\"", strconv.Quote(string(data)), )) - exe := New(nil, nil, nil, Paths{}, Scripts{ReadInfo: script}, nil) + exe := New(nil, nil, nil, Paths{Work: t.TempDir()}, Scripts{ReadInfo: script}, nil) // Normalize only the explicitly reported identity; malformed reports stay errors. barcode, err := exe.ReadTapeBarcode(context.Background(), "/dev/nst0") @@ -72,7 +72,7 @@ func TestHistoricalReadInfoScriptWorksUnchanged(t *testing.T) { wrapper := writeExecutorTestScript(t, "legacy-probe", fmt.Sprintf( "cd %q\nexport PATH=%q:$PATH\nexec ./readinfo", root, root, )) - exe := New(nil, nil, nil, Paths{}, Scripts{ReadInfo: wrapper}, nil) + exe := New(nil, nil, nil, Paths{Work: t.TempDir()}, Scripts{ReadInfo: wrapper}, nil) barcode, err := exe.ReadTapeBarcode(context.Background(), "/dev/nst0") require.NoError(t, err) require.Equal(t, "ABC001", barcode) @@ -260,7 +260,7 @@ func TestTapeSessionsRejectUnverifiedIdentityBeforeMutation(t *testing.T) { readInfo = writeExecutorTestScript(t, "failed-read-info", "exit 1") } mutation := writeExecutorTestScript(t, "mutate", fmt.Sprintf(": > %s", strconv.Quote(mutationMarker))) - exe := New(nil, nil, []string{"/dev/nst0"}, Paths{}, Scripts{ + exe := New(nil, nil, []string{"/dev/nst0"}, Paths{Work: t.TempDir()}, Scripts{ ReadInfo: readInfo, Encrypt: mutation, Mkfs: mutation, Mount: mutation, }, nil) require.NoError(t, exe.beginAttempt(1, func(error) {})) diff --git a/internal/library/archive_import.go b/internal/library/archive_import.go index dc458f9..f4d2e57 100644 --- a/internal/library/archive_import.go +++ b/internal/library/archive_import.go @@ -70,9 +70,9 @@ func countImportedPath(tx, planned *gorm.DB, logicalPath string) (bool, int64, e return false, created, nil } -func openArchiveImportStage(ctx context.Context) (*gorm.DB, func() error, error) { +func (l *Library) openArchiveImportStage(ctx context.Context) (*gorm.DB, func() error, error) { // Reuse the existing File row schema in an isolated, request-owned naming workset. - stage, err := resource.OpenTemporaryDB("", "yatm-archive-import-") + stage, err := resource.OpenTemporaryDB(l.work, "yatm-archive-import-") if err != nil { return nil, nil, err } diff --git a/internal/library/catalog.go b/internal/library/catalog.go index 2059b9f..fb69a10 100644 --- a/internal/library/catalog.go +++ b/internal/library/catalog.go @@ -141,7 +141,7 @@ func (l *Library) ImportArchivePositionSelection(ctx context.Context, ids []int6 // Reports leave the catalog untouched; real admission rolls back on any failed candidate. if dryRun { - stage, closeStage, err := openArchiveImportStage(ctx) + stage, closeStage, err := l.openArchiveImportStage(ctx) if err != nil { return nil, err } diff --git a/internal/library/file_read_ownership_test.go b/internal/library/file_read_ownership_test.go index 5aa0510..1245ad2 100644 --- a/internal/library/file_read_ownership_test.go +++ b/internal/library/file_read_ownership_test.go @@ -20,6 +20,7 @@ func TestFileReadsUseCatalogReaderAndTransactionsKeepTheirOwnView(t *testing.T) require.NoError(t, err) t.Cleanup(func() { require.NoError(t, catalog.Close()) }) lib := NewWithCatalog(catalog.Write, catalog.Read, nil) + require.NoError(t, lib.PrepareTemporary(t.TempDir())) file := &File{Name: "committed"} require.NoError(t, lib.SaveFile(context.Background(), file)) tx := catalog.Write.Begin() diff --git a/internal/library/file_tree.go b/internal/library/file_tree.go index 11a66a2..06251ad 100644 --- a/internal/library/file_tree.go +++ b/internal/library/file_tree.go @@ -19,7 +19,7 @@ func (l *Library) FileTree() treeops.Store { return &fileTree{lib: l, db: l.read // FileTreeTransaction retains metadata-only rollback for a selected Library root. func (l *Library) FileTreeTransaction(ctx context.Context, use func(treeops.Store) error) error { - return l.db.WithContext(ctx).Transaction(func(tx *gorm.DB) error { return use(&fileTree{lib: &Library{db: tx}, db: tx}) }) + return l.db.WithContext(ctx).Transaction(func(tx *gorm.DB) error { return use(&fileTree{lib: &Library{db: tx, work: l.work}, db: tx}) }) } // moveTree keeps direct Library callers on the same organization engine as Files. @@ -46,7 +46,7 @@ func (l *Library) moveTree(ctx context.Context, file *File, name string) error { // organizeTree freezes the plan outside metadata transactions and runs shared tree rules. func (l *Library) organizeTree(ctx context.Context, request treeops.Request) (resultID int64, returnErr error) { // The request owns its temporary plan until execution and result delivery finish. - temporary, err := resource.OpenTemporaryDB("", "yatm-logical-operation-") + temporary, err := resource.OpenTemporaryDB(l.work, "yatm-logical-operation-") if err != nil { return 0, err } diff --git a/internal/library/identical_lifetime_test.go b/internal/library/identical_lifetime_test.go index 480eb05..135c3bb 100644 --- a/internal/library/identical_lifetime_test.go +++ b/internal/library/identical_lifetime_test.go @@ -8,6 +8,7 @@ import ( "testing" "github.com/samuelncui/yatm/entity" + "github.com/samuelncui/yatm/internal/resource" "github.com/stretchr/testify/require" "gorm.io/gorm" ) @@ -25,7 +26,7 @@ func TestIdenticalConstructionFailureCleansStaging(t *testing.T) { sqlDB, err := db.DB() require.NoError(t, err) t.Cleanup(func() { require.NoError(t, sqlDB.Close()) }) - lib.identicalTempRoot = t.TempDir() + require.NoError(t, lib.PrepareTemporary(t.TempDir())) ctx, cancel := context.WithCancel(context.Background()) defer cancel() failure := errors.New("catalog unavailable") @@ -38,10 +39,10 @@ func TestIdenticalConstructionFailureCleansStaging(t *testing.T) { if os.Geteuid() == 0 { t.Skip("permission fixture requires an unprivileged user") } - entries, err := os.ReadDir(lib.identicalTempRoot) + entries, err := os.ReadDir(resource.TemporaryRoot(lib.work)) require.NoError(t, err) require.Len(t, entries, 1) - directory := filepath.Join(lib.identicalTempRoot, entries[0].Name()) + directory := filepath.Join(resource.TemporaryRoot(lib.work), entries[0].Name()) t.Cleanup(func() { require.NoError(t, os.Chmod(directory, 0700)) require.NoError(t, os.RemoveAll(directory)) @@ -70,7 +71,7 @@ func TestIdenticalConstructionFailureCleansStaging(t *testing.T) { require.ErrorIs(t, err, os.ErrPermission) return } - entries, err := os.ReadDir(lib.identicalTempRoot) + entries, err := os.ReadDir(resource.TemporaryRoot(lib.work)) require.NoError(t, err) require.Empty(t, entries) }) @@ -84,7 +85,7 @@ func TestIdenticalResultsOutliveConstructionContext(t *testing.T) { sqlDB, err := db.DB() require.NoError(t, err) t.Cleanup(func() { require.NoError(t, sqlDB.Close()) }) - lib.identicalTempRoot = t.TempDir() + require.NoError(t, lib.PrepareTemporary(t.TempDir())) for _, name := range []string{"one", "two"} { file := &File{Name: name, Kind: entity.FileKind_FILE_KIND_REGULAR} createFileRows(t, db, file) diff --git a/internal/library/identical_query.go b/internal/library/identical_query.go index 63a40db..cc1b7f7 100644 --- a/internal/library/identical_query.go +++ b/internal/library/identical_query.go @@ -7,8 +7,6 @@ import ( "encoding/json" "errors" "fmt" - "os" - "path/filepath" "sort" "strconv" @@ -96,6 +94,7 @@ type identicalEdge struct { type IdenticalSnapshot struct { db *gorm.DB directory string + temporary *resource.TemporaryDB scope string Revision string AllRows int64 @@ -107,42 +106,32 @@ type IdenticalSnapshot struct { // Close releases SQLite before removing its disposable files. func (s *IdenticalSnapshot) Close() error { - // Partial construction still owns its directory; a close failure cannot skip file cleanup. - var result error - if s.db != nil { - db, err := s.db.DB() - if err == nil { - err = db.Close() - } - if err != nil { - result = fmt.Errorf("close identical staging database failed, %w", err) - } + // Keep the owner reachable when removal fails so a later Close can retry. + if s.temporary == nil { + return nil } - if err := os.RemoveAll(s.directory); err != nil { - result = errors.Join(result, fmt.Errorf("remove identical staging failed, %w", err)) + if err := s.temporary.Close(); err != nil { + return err } - return result + s.temporary = nil + return nil } func (l *Library) newIdenticalSnapshot( ctx context.Context, scope IdenticalScope, prefix string, ) (result *IdenticalSnapshot, returnErr error) { // Both builders transfer the same SQLite/directory owner without sharing their query semantics. - directory, err := l.identicalDirectory(prefix) + temporary, err := resource.OpenTemporaryDB(l.work, prefix) if err != nil { return nil, fmt.Errorf("create identical staging failed, %w", err) } - s := &IdenticalSnapshot{directory: directory} + s := &IdenticalSnapshot{directory: temporary.Directory, temporary: temporary} defer func() { if result == nil { returnErr = errors.Join(returnErr, s.Close()) } }() - db, err := resource.OpenSQLite(filepath.Join(directory, "groups.sqlite")) - if err != nil { - return nil, fmt.Errorf("open identical staging failed, %w", err) - } - s.db = db.WithContext(ctx) + s.db = temporary.DB.WithContext(ctx) encoded, _ := json.Marshal(scope) s.scope = string(encoded) return s, nil diff --git a/internal/library/identical_query_benchmark_test.go b/internal/library/identical_query_benchmark_test.go index 5e3bfec..a1c853a 100644 --- a/internal/library/identical_query_benchmark_test.go +++ b/internal/library/identical_query_benchmark_test.go @@ -142,9 +142,8 @@ func newIdenticalPerfCatalog(t testing.TB, count int) (*Library, []identicalPerf t.Cleanup(func() { require.NoError(t, sqlDB.Close()) }) lib := New(db) require.NoError(t, lib.AutoMigrate()) - release, err := lib.PrepareIdenticalTemp(filepath.Join(root, "work")) + err = lib.PrepareTemporary(filepath.Join(root, "work")) require.NoError(t, err) - t.Cleanup(func() { require.NoError(t, release()) }) locations := make([]Location, 4) for i := range locations { locations[i] = Location{ID: int64(i + 1), Name: fmt.Sprintf("Location %d", i), ExecutorID: "fixture", diff --git a/internal/library/identical_query_temp.go b/internal/library/identical_query_temp.go index 00a6fbe..636b570 100644 --- a/internal/library/identical_query_temp.go +++ b/internal/library/identical_query_temp.go @@ -1,76 +1,16 @@ package library import ( - "fmt" - "os" - "path/filepath" - "strings" - - "golang.org/x/sys/unix" + "github.com/samuelncui/yatm/internal/resource" ) -const identicalTempName = ".identical-results" - -// PrepareIdenticalTemp owns disposable Find and component SQLite files for one service lifetime. -// The returned release must run after requests and retained results have closed. -func (l *Library) PrepareIdenticalTemp(work string) (func() error, error) { - // Lock the configured Work directory before inspecting an earlier run's results. - if err := os.MkdirAll(work, 0700); err != nil { - return nil, fmt.Errorf("create identical work directory failed, %w", err) - } - lockPath := filepath.Join(work, identicalTempName+".lock") - fd, err := unix.Open(lockPath, unix.O_CREAT|unix.O_RDWR|unix.O_CLOEXEC|unix.O_NOFOLLOW, 0600) - if err != nil { - return nil, fmt.Errorf("open identical result lock failed, %w", err) - } - lock := os.NewFile(uintptr(fd), lockPath) - if err := unix.Flock(fd, unix.LOCK_EX|unix.LOCK_NB); err != nil { - _ = lock.Close() - return nil, fmt.Errorf("acquire identical result lock failed, %w", err) - } - - // Only this private directory can contain results eligible for startup cleanup. - root := filepath.Join(work, identicalTempName) - if err := prepareIdenticalTempRoot(root); err != nil { - _ = lock.Close() - return nil, err - } - l.identicalTempRoot = root - return lock.Close, nil -} - -func prepareIdenticalTempRoot(root string) error { - // Reject a redirected or shared directory before removing any prior-run entries. - info, err := os.Lstat(root) - if os.IsNotExist(err) { - if err := os.Mkdir(root, 0700); err != nil { - return fmt.Errorf("create identical result directory failed, %w", err) - } - return nil - } - if err != nil { - return fmt.Errorf("inspect identical result directory failed, %w", err) - } - if !info.IsDir() || info.Mode().Perm()&0o077 != 0 { - return fmt.Errorf("identical result directory must be a private directory: %s", root) - } - - // Prior runs may have left partially built snapshots; never inspect or remove other paths. - entries, err := os.ReadDir(root) - if err != nil { - return fmt.Errorf("list identical result directory failed, %w", err) - } - for _, entry := range entries { - if !strings.HasPrefix(entry.Name(), "yatm-identical-") { - continue - } - if err := os.RemoveAll(filepath.Join(root, entry.Name())); err != nil { - return fmt.Errorf("remove stale identical result %q failed, %w", entry.Name(), err) - } +// PrepareTemporary configures disposable Library work before requests are admitted. +// The service owns periodic cleanup; live operations retain their own directories. +func (l *Library) PrepareTemporary(work string) error { + // Validate the shared root before publishing the configured path to requests. + if err := resource.PrepareTemporary(work); err != nil { + return err } + l.work = work return nil } - -func (l *Library) identicalDirectory(prefix string) (string, error) { - return os.MkdirTemp(l.identicalTempRoot, prefix) -} diff --git a/internal/library/identical_query_temp_test.go b/internal/library/identical_query_temp_test.go index 6fc934f..e2407b7 100644 --- a/internal/library/identical_query_temp_test.go +++ b/internal/library/identical_query_temp_test.go @@ -1,126 +1,48 @@ package library import ( - "bufio" "context" - "fmt" - "io" "os" - "os/exec" "path/filepath" - "strings" "testing" + "time" "github.com/samuelncui/yatm/entity" + "github.com/samuelncui/yatm/internal/resource" "github.com/stretchr/testify/require" ) -func TestPrepareIdenticalTempCleansOnlyOwnedResults(t *testing.T) { - // Simulate an interrupted prior run and a legacy result in the shared system temp directory. - work := t.TempDir() - root := filepath.Join(work, identicalTempName) - require.NoError(t, os.Mkdir(root, 0700)) - for _, name := range []string{"yatm-identical-stale", "yatm-identical-component-stale"} { - path := filepath.Join(root, name) - require.NoError(t, os.Mkdir(path, 0700)) - require.NoError(t, os.WriteFile(filepath.Join(path, "groups.sqlite"), []byte("stale"), 0600)) - } - unrelated := filepath.Join(root, "unrelated") - require.NoError(t, os.Mkdir(unrelated, 0700)) - outside := t.TempDir() - marker := filepath.Join(outside, "keep") - require.NoError(t, os.WriteFile(marker, []byte("owned elsewhere"), 0600)) - link := filepath.Join(root, "yatm-identical-linked") - require.NoError(t, os.Symlink(outside, link)) - legacy, err := os.MkdirTemp("", "yatm-identical-") - require.NoError(t, err) - t.Cleanup(func() { require.NoError(t, os.RemoveAll(legacy)) }) - require.NoError(t, os.WriteFile(filepath.Join(legacy, "groups.sqlite"), []byte("legacy"), 0600)) - - // Startup removes only its old result names, including a link without following it. - lib := &Library{} - release, err := lib.PrepareIdenticalTemp(work) - require.NoError(t, err) - defer func() { require.NoError(t, release()) }() - for _, name := range []string{"yatm-identical-stale", "yatm-identical-component-stale"} { - require.NoDirExists(t, filepath.Join(root, name)) - } - require.NoFileExists(t, link) - require.FileExists(t, marker) - require.DirExists(t, unrelated) - require.FileExists(t, filepath.Join(legacy, "groups.sqlite")) -} - -func TestPrepareIdenticalTempLockProtectsLiveResults(t *testing.T) { - // A second process using the same Work directory cannot sweep a live result. - if work := os.Getenv("YATM_IDENTICAL_TEMP_TEST_WORK"); work != "" { - lib := &Library{} - release, err := lib.PrepareIdenticalTemp(work) - require.NoError(t, err) - defer release() - _, err = lib.identicalDirectory("yatm-identical-") - require.NoError(t, err) - fmt.Println("ready") - _, _ = io.Copy(io.Discard, os.Stdin) - return - } - - // Hold the lock in a subprocess so SIGKILL also tests automatic lock release. - work := t.TempDir() - cmd := exec.Command(os.Args[0], "-test.run=^TestPrepareIdenticalTempLockProtectsLiveResults$") - cmd.Env = append(os.Environ(), "YATM_IDENTICAL_TEMP_TEST_WORK="+work) - stdout, err := cmd.StdoutPipe() - require.NoError(t, err) - stdin, err := cmd.StdinPipe() - require.NoError(t, err) - require.NoError(t, cmd.Start()) - t.Cleanup(func() { - _ = cmd.Process.Kill() - _ = cmd.Wait() - _ = stdin.Close() - }) - ready, err := bufio.NewReader(stdout).ReadString('\n') - require.NoError(t, err) - require.Equal(t, "ready", strings.TrimSpace(ready)) - entries, err := os.ReadDir(filepath.Join(work, identicalTempName)) - require.NoError(t, err) - require.Len(t, entries, 1) - live := filepath.Join(work, identicalTempName, entries[0].Name()) - _, err = (&Library{}).PrepareIdenticalTemp(work) - require.Error(t, err) - require.DirExists(t, live) - - // The next startup removes the result left by the killed process. - require.NoError(t, cmd.Process.Kill()) - _ = cmd.Wait() - release, err := (&Library{}).PrepareIdenticalTemp(work) - require.NoError(t, err) - defer func() { require.NoError(t, release()) }() - require.NoDirExists(t, live) -} - -func TestPrepareIdenticalTempRejectsLinkedRoot(t *testing.T) { - // A linked scratch root must not redirect startup deletion outside Work. - work := t.TempDir() - outside := t.TempDir() - marker := filepath.Join(outside, "yatm-identical-foreign") - require.NoError(t, os.Mkdir(marker, 0700)) - require.NoError(t, os.Symlink(outside, filepath.Join(work, identicalTempName))) - _, err := (&Library{}).PrepareIdenticalTemp(work) - require.Error(t, err) - require.DirExists(t, marker) -} - -func TestIdenticalSnapshotsUsePreparedTemp(t *testing.T) { - // Both complete Find and targeted validation use the service-owned scratch root. +func TestPrepareTemporaryRetainsLiveIdenticalResults(t *testing.T) { + // Re-preparation in this process cannot remove a result still held by a reader. db, lib := newTestLibrary(t) file := &File{Name: "one", Kind: entity.FileKind_FILE_KIND_REGULAR} createFileRows(t, db, file) require.NoError(t, db.Create(&FileVersion{FileID: file.ID, Signature: []byte("signature")}).Error) - release, err := lib.PrepareIdenticalTemp(t.TempDir()) + snapshot, err := lib.OpenIdenticalSnapshot(context.Background(), IdenticalScope{Source: IdenticalLibrary}) require.NoError(t, err) - defer func() { require.NoError(t, release()) }() - root := lib.identicalTempRoot + t.Cleanup(func() { require.NoError(t, snapshot.Close()) }) + + // Even a sweep beyond the retention period must preserve the process-local hold. + require.NoError(t, resource.CleanupTemporary(lib.work, time.Now().Add(8*24*time.Hour))) + require.DirExists(t, snapshot.directory) + require.NoError(t, lib.PrepareTemporary(lib.work)) + require.DirExists(t, snapshot.directory) +} + +func TestPrepareTemporaryRejectsLinkedRoot(t *testing.T) { + // A linked temporary root cannot redirect cleanup outside Work. + work := t.TempDir() + outside := t.TempDir() + require.NoError(t, os.Symlink(outside, resource.TemporaryRoot(work))) + require.Error(t, (&Library{}).PrepareTemporary(work)) +} + +func TestIdenticalSnapshotsUsePreparedTemp(t *testing.T) { + // Both complete Find and targeted validation use the same Work temporary root. + db, lib := newTestLibrary(t) + file := &File{Name: "one", Kind: entity.FileKind_FILE_KIND_REGULAR} + createFileRows(t, db, file) + require.NoError(t, db.Create(&FileVersion{FileID: file.ID, Signature: []byte("signature")}).Error) scope := IdenticalScope{Source: IdenticalLibrary} for _, open := range []func() (*IdenticalSnapshot, error){ func() (*IdenticalSnapshot, error) { return lib.OpenIdenticalSnapshot(context.Background(), scope) }, @@ -130,7 +52,7 @@ func TestIdenticalSnapshotsUsePreparedTemp(t *testing.T) { } { snapshot, err := open() require.NoError(t, err) - require.Equal(t, root, filepath.Dir(snapshot.directory)) + require.Equal(t, resource.TemporaryRoot(lib.work), filepath.Dir(snapshot.directory)) require.DirExists(t, snapshot.directory) require.NoError(t, snapshot.Close()) require.NoDirExists(t, snapshot.directory) diff --git a/internal/library/json_legacy.go b/internal/library/json_legacy.go index 37162a0..abe5e70 100644 --- a/internal/library/json_legacy.go +++ b/internal/library/json_legacy.go @@ -60,7 +60,7 @@ type legacyLibraryDecoders struct { func (l *Library) importLegacyJSON(ctx context.Context, r io.Reader, dryRun bool) (rerr error) { // Resolve arbitrary legacy array order in a disposable indexed staging database, before replacement. - temporary, err := resource.OpenTemporaryDB("", "yatm-legacy-import-") + temporary, err := resource.OpenTemporaryDB(l.work, "yatm-legacy-import-") if err != nil { return err } diff --git a/internal/library/library.go b/internal/library/library.go index 9ccad61..d688f8d 100644 --- a/internal/library/library.go +++ b/internal/library/library.go @@ -15,10 +15,10 @@ const ( ) type Library struct { - db *gorm.DB - reader *gorm.DB - settings *settingspkg.Module - identicalTempRoot string + db *gorm.DB + reader *gorm.DB + settings *settingspkg.Module + work string } func New(db *gorm.DB) *Library { diff --git a/internal/library/testutil_test.go b/internal/library/testutil_test.go index e9606bb..8ea593b 100644 --- a/internal/library/testutil_test.go +++ b/internal/library/testutil_test.go @@ -20,6 +20,7 @@ func newTestLibrary(t *testing.T) (*gorm.DB, *Library) { t.Helper() db := openTestLibraryDB(t) lib := New(db) + require.NoError(t, lib.PrepareTemporary(t.TempDir())) require.NoError(t, lib.AutoMigrate()) return db, lib } diff --git a/internal/media/volume.go b/internal/media/volume.go index ab24f95..f45b132 100644 --- a/internal/media/volume.go +++ b/internal/media/volume.go @@ -41,7 +41,7 @@ type Volume struct { } // InitializeVolume creates a marker without mounting or ejecting the filesystem. -func InitializeVolume(root string, profile *entity.VolumeMediaProfile) (*Volume, error) { +func InitializeVolume(root string, profile *entity.VolumeMediaProfile) (result *Volume, returnErr error) { // Validate the immutable profile before writing an identity marker. if profile == nil { return nil, fmt.Errorf("initialize Volume failed, profile is nil") @@ -78,19 +78,26 @@ func InitializeVolume(root string, profile *entity.VolumeMediaProfile) (*Volume, return nil, fmt.Errorf("create temporary Volume marker failed, root=%q, %w", canonical, err) } temporaryPath := file.Name() - defer os.Remove(temporaryPath) + open := true + defer func() { + if open { + returnErr = errors.Join(returnErr, file.Close()) + } + returnErr = errors.Join(returnErr, removeTemporaryVolumeMarker(temporaryPath)) + if returnErr != nil { + result = nil + } + }() if err := file.Chmod(0o644); err != nil { - _ = file.Close() return nil, fmt.Errorf("set Volume marker mode failed, path=%q, %w", temporaryPath, err) } if _, err := file.Write(append(data, '\n')); err != nil { - _ = file.Close() return nil, fmt.Errorf("write Volume marker failed, path=%q, %w", temporaryPath, err) } if err := file.Sync(); err != nil { - _ = file.Close() return nil, fmt.Errorf("sync Volume marker failed, path=%q, %w", temporaryPath, err) } + open = false if err := file.Close(); err != nil { return nil, fmt.Errorf("close Volume marker failed, path=%q, %w", temporaryPath, err) } @@ -100,6 +107,13 @@ func InitializeVolume(root string, profile *entity.VolumeMediaProfile) (*Volume, return &Volume{Root: canonical, Marker: marker}, nil } +func removeTemporaryVolumeMarker(path string) error { + if err := os.Remove(path); err != nil && !errors.Is(err, os.ErrNotExist) { + return fmt.Errorf("remove temporary Volume marker failed, path=%q, %w", path, err) + } + return nil +} + // OpenVolume validates one marker at an already mounted root. func OpenVolume(root string) (*Volume, error) { // Resolve the mounted root before opening its authoritative marker. diff --git a/internal/media/volume_test.go b/internal/media/volume_test.go index 5fc536b..f456ae9 100644 --- a/internal/media/volume_test.go +++ b/internal/media/volume_test.go @@ -187,3 +187,31 @@ func TestListVolumeCandidatesKeepsEveryCandidateVisible(t *testing.T) { require.NoError(t, err) require.Empty(t, skipped) } + +func TestTemporaryVolumeMarkerCleanupFailureRemainsRetryable(t *testing.T) { + if os.Geteuid() == 0 { + t.Skip("permission fixture requires an unprivileged user") + } + root := t.TempDir() + path := filepath.Join(root, ".yatm-owned.tmp") + require.NoError(t, os.WriteFile(path, []byte("unpublished marker"), 0600)) + require.NoError(t, os.Chmod(root, 0500)) + t.Cleanup(func() { require.NoError(t, os.Chmod(root, 0700)) }) + err := removeTemporaryVolumeMarker(path) + require.ErrorIs(t, err, os.ErrPermission) + require.ErrorContains(t, err, path) + require.FileExists(t, path) + require.NoError(t, os.Chmod(root, 0700)) + require.NoError(t, removeTemporaryVolumeMarker(path)) + require.NoError(t, removeTemporaryVolumeMarker(path)) +} + +func TestInitializeVolumeLeavesOnlyPublishedMarker(t *testing.T) { + root := t.TempDir() + _, err := InitializeVolume(root, &entity.VolumeMediaProfile{Type: entity.VolumeType_VOLUME_TYPE_HDD}) + require.NoError(t, err) + entries, err := os.ReadDir(root) + require.NoError(t, err) + require.Len(t, entries, 1) + require.Equal(t, VolumeMarkerName, entries[0].Name()) +} diff --git a/internal/migrate/legacy/backup.go b/internal/migrate/legacy/backup.go index 02d3be2..b9237ea 100644 --- a/internal/migrate/legacy/backup.go +++ b/internal/migrate/legacy/backup.go @@ -31,55 +31,67 @@ type Backup struct { RestoreRoot string } -func (backup Backup) open() (*gorm.DB, func(), error) { +func (backup Backup) open(work string) (*gorm.DB, func() error, error) { // Work on a disposable database copy so SQLite cannot create or update backup sidecars. if backup.CatalogPath == "" || backup.WorkRoot == "" { return nil, nil, fmt.Errorf("a complete legacy backup is required") } - db, cleanup, err := openSnapshotCatalog(backup.CatalogPath) + db, cleanup, err := openSnapshotCatalog(work, backup.CatalogPath) if err != nil { return nil, nil, err } schema, err := DetectSchema(db) if err != nil { - cleanup() - return nil, nil, fmt.Errorf("inspect backup catalog failed, %w", err) + return nil, nil, errors.Join(fmt.Errorf("inspect backup catalog failed, %w", err), cleanup()) } if schema != SchemaLegacy { - cleanup() - return nil, nil, fmt.Errorf("backup must contain a legacy catalog, found %q", schema) + return nil, nil, errors.Join(fmt.Errorf("backup must contain a legacy catalog, found %q", schema), cleanup()) } return db, cleanup, nil } -func openSnapshotCatalog(filename string) (*gorm.DB, func(), error) { - // Copy the database and any retained WAL, never open the preserved source with SQLite. - dir, err := os.MkdirTemp("", "yatm-migration-check-") +func openSnapshotCatalog(work, filename string) (_ *gorm.DB, _ func() error, returnErr error) { + // Copy the database and retained WAL under the active Work root, never beside preserved evidence. + owner, err := resource.NewTemporaryDirectory(work, "migration-check-") if err != nil { return nil, nil, err } - remove := func() { _ = os.RemoveAll(dir) } - target := filepath.Join(dir, "state.db") + var db *gorm.DB + cleanup := func() error { + var result error + if db != nil { + sqlDB, err := db.DB() + if err != nil { + result = err + } else { + result = sqlDB.Close() + } + db = nil + } + return errors.Join(result, owner.Close()) + } + defer func() { + if returnErr != nil { + returnErr = errors.Join(returnErr, cleanup()) + } + }() + target := filepath.Join(owner.Directory, "state.db") for _, suffix := range []string{"", "-wal"} { if err := copyEvidence(filename+suffix, target+suffix); err != nil { if suffix != "" && errors.Is(err, fs.ErrNotExist) { continue } - remove() return nil, nil, fmt.Errorf("copy backup catalog failed, %w", err) } } - - // Opening only the copy also supports legacy transitional Job databases with WAL state. - db, err := resource.OpenSQLite(target) + db, err = resource.OpenSQLite(target) if err != nil { - remove() return nil, nil, err } - return db, func() { closeDB(db); remove() }, nil + return db, cleanup, nil } -func copyEvidence(sourcePath, targetPath string) error { +func copyEvidence(sourcePath, targetPath string) (returnErr error) { // Require a regular evidence file; links cannot redirect validation into the active tree. info, err := os.Lstat(sourcePath) if err != nil { @@ -92,7 +104,7 @@ func copyEvidence(sourcePath, targetPath string) error { if err != nil { return err } - defer source.Close() + defer func() { returnErr = errors.Join(returnErr, source.Close()) }() // The caller owns the temporary destination and its restrictive permissions. target, err := os.OpenFile(targetPath, os.O_CREATE|os.O_EXCL|os.O_WRONLY, 0o600) @@ -104,7 +116,13 @@ func copyEvidence(sourcePath, targetPath string) error { } // Validate compares migration results with independently rebuilt backup facts before activation cleanup. -func Validate(ctx context.Context, db *gorm.DB, workRoot string, backup Backup) (_ *Report, rerr error) { +func Validate(ctx context.Context, db *gorm.DB, workRoot string, backup Backup) (report *Report, rerr error) { + defer func() { + if rerr != nil && report != nil { + report.Success = false + report.Error = rerr.Error() + } + }() // Validate the active format before reading any backup or creating temporary output. if db == nil { return nil, fmt.Errorf("active migration catalog is missing") @@ -115,23 +133,18 @@ func Validate(ctx context.Context, db *gorm.DB, workRoot string, backup Backup) if err := dataformat.CheckBundles(db, workRoot); err != nil { return nil, err } - expected, cleanup, err := backup.open() + expected, cleanup, err := backup.open(workRoot) if err != nil { return nil, err } - defer cleanup() + defer func() { rerr = errors.Join(rerr, cleanup()) }() // Reuse the migration's content reconciliation against captured backup indexes. createdAtNS, err := dataformat.Nanoseconds(time.Now()) if err != nil { return nil, fmt.Errorf("record migration validation time failed, %w", err) } - report := &Report{CreatedAtNS: createdAtNS} - defer func() { - if rerr != nil { - report.Error = rerr.Error() - } - }() + report = &Report{CreatedAtNS: createdAtNS} if err := prepareLibrary(ctx, expected, backup.IndexRoot, report); err != nil { return report, fmt.Errorf("rebuild expected Library failed, %w", err) } @@ -152,11 +165,12 @@ func Validate(ctx context.Context, db *gorm.DB, workRoot string, backup Backup) } // Compare each complete historical manifest independently, including logs and frozen targets. - jobsRoot, err := os.MkdirTemp("", "yatm-migration-jobs-") + jobs, err := resource.NewTemporaryDirectory(workRoot, "migration-jobs-") if err != nil { return report, err } - defer os.RemoveAll(jobsRoot) + defer func() { rerr = errors.Join(rerr, jobs.Close()) }() + jobsRoot := jobs.Directory var after int64 for { var job legacyJob @@ -172,7 +186,7 @@ func Validate(ctx context.Context, db *gorm.DB, workRoot string, backup Backup) report.DeletedJobs = append(report.DeletedJobs, job.ID) continue } - counts, _, err := prepareJob(ctx, expected, "positions_staging", restorePositionsRequired, + counts, _, err := prepareJob(ctx, workRoot, expected, "positions_staging", restorePositionsRequired, jobsRoot, filepath.Join(backup.WorkRoot, "jobs"), backup.WorkRoot, &job, backup.RestoreRoot) if err != nil { return report, fmt.Errorf("rebuild expected Job %d failed, %w", job.ID, err) @@ -191,7 +205,7 @@ func Validate(ctx context.Context, db *gorm.DB, workRoot string, backup Backup) return report, nil } -func compareJob(ctx context.Context, db *gorm.DB, workRoot, expectedRoot string, job *legacyJob) error { +func compareJob(ctx context.Context, db *gorm.DB, workRoot, expectedRoot string, job *legacyJob) (returnErr error) { // Bind the historical row to the same catalog identity and authoritative Job status. var catalog catalogJob if err := db.WithContext(ctx).Table("jobs").First(&catalog, job.ID).Error; err != nil { @@ -199,16 +213,16 @@ func compareJob(ctx context.Context, db *gorm.DB, workRoot, expectedRoot string, } id := strconv.FormatInt(job.ID, 10) actualDir, expectedDir := filepath.Join(workRoot, "jobs", id), filepath.Join(expectedRoot, id) - actual, closeActual, err := openSnapshotCatalog(filepath.Join(actualDir, "state.db")) + actual, closeActual, err := openSnapshotCatalog(workRoot, filepath.Join(actualDir, "state.db")) if err != nil { return err } - defer closeActual() - expected, closeExpected, err := openSnapshotCatalog(filepath.Join(expectedDir, "state.db")) + defer func() { returnErr = errors.Join(returnErr, closeActual()) }() + expected, closeExpected, err := openSnapshotCatalog(workRoot, filepath.Join(expectedDir, "state.db")) if err != nil { return err } - defer closeExpected() + defer func() { returnErr = errors.Join(returnErr, closeExpected()) }() var record executor.JobRecord if err := expected.First(&record, 1).Error; err != nil { return err diff --git a/internal/migrate/legacy/backup_test.go b/internal/migrate/legacy/backup_test.go index 6395a5e..d073ac7 100644 --- a/internal/migrate/legacy/backup_test.go +++ b/internal/migrate/legacy/backup_test.go @@ -141,11 +141,11 @@ func TestBackupSnapshotIncludesWALWithoutWritingEvidence(t *testing.T) { hashes := backupHashes(t, backup.Root) // Snapshot reconstruction can write temporary SQLite state, but never source WAL or sidecars. - snapshot, cleanup, err := backup.open() + snapshot, cleanup, err := backup.open(t.TempDir()) require.NoError(t, err) var job legacyJob require.NoError(t, snapshot.First(&job, 42).Error) - cleanup() + require.NoError(t, cleanup()) require.Equal(t, hashes, backupHashes(t, backup.Root)) } @@ -170,3 +170,41 @@ func TestValidateDetectsChangedArchivedLog(t *testing.T) { require.Error(t, Cleanup(ctx, db, root, backup)) require.True(t, db.Migrator().HasTable("jobs_legacy")) } + +func TestSnapshotScratchUsesActiveWorkAndCleansFailedConstruction(t *testing.T) { + work := t.TempDir() + _, _, err := openSnapshotCatalog(work, filepath.Join(t.TempDir(), "missing.db")) + require.ErrorIs(t, err, os.ErrNotExist) + entries, err := os.ReadDir(resource.TemporaryRoot(work)) + require.NoError(t, err) + require.Empty(t, entries) +} + +func TestSnapshotCleanupFailureRetainsOwnedPathForRetry(t *testing.T) { + if os.Geteuid() == 0 { + t.Skip("permission fixture requires an unprivileged user") + } + db := newLegacyTestDB(t) + backup := preserveTestBackup(t, db, t.TempDir()) + hashes := backupHashes(t, backup.Root) + work := t.TempDir() + snapshot, cleanup, err := backup.open(work) + require.NoError(t, err) + var databases []struct{ File string } + require.NoError(t, snapshot.Raw("PRAGMA database_list").Scan(&databases).Error) + root := resource.TemporaryRoot(work) + directory := filepath.Dir(databases[0].File) + expectedRoot, err := os.Stat(root) + require.NoError(t, err) + actualRoot, err := os.Stat(filepath.Dir(directory)) + require.NoError(t, err) + require.True(t, os.SameFile(expectedRoot, actualRoot)) + require.NoError(t, os.Chmod(root, 0500)) + t.Cleanup(func() { require.NoError(t, os.Chmod(root, 0700)); require.NoError(t, cleanup()) }) + require.ErrorIs(t, cleanup(), os.ErrPermission) + require.DirExists(t, directory) + require.NoError(t, os.Chmod(root, 0700)) + require.NoError(t, cleanup()) + require.NoDirExists(t, directory) + require.Equal(t, hashes, backupHashes(t, backup.Root)) +} diff --git a/internal/migrate/legacy/migrate.go b/internal/migrate/legacy/migrate.go index 8a8f9b3..1cbb896 100644 --- a/internal/migrate/legacy/migrate.go +++ b/internal/migrate/legacy/migrate.go @@ -206,6 +206,7 @@ func PrepareWithLTFSIndex( } counts, warnings, err := prepareJob( ctx, + workRoot, db, "positions_staging", restorePositionsRequired, @@ -263,6 +264,7 @@ const ( func prepareJob( ctx context.Context, + workRoot string, libraryDB *gorm.DB, positionsTable string, positionPolicy restorePositionPolicy, @@ -271,7 +273,7 @@ func prepareJob( legacyWorkRoot string, job *legacyJob, restoreRoot ...string, -) (itemCounts, []string, error) { +) (_ itemCounts, _ []string, returnErr error) { // Reject an undecodable legacy row before creating staging artifacts. if job.State == nil { return itemCounts{}, nil, fmt.Errorf("JobState is missing") @@ -312,13 +314,13 @@ func prepareJob( var transitionalDB *gorm.DB var table string if state.Archive != nil && len(state.Archive.Sources) == 0 { - var cleanup func() - transitionalDB, table, cleanup, err = openTransitionalJobDB(legacyJobsRoot, job.ID, "files") + var cleanup func() error + transitionalDB, table, cleanup, err = openTransitionalJobDB(workRoot, legacyJobsRoot, job.ID, "files") if err != nil { return itemCounts{}, nil, err } if transitionalDB != nil { - defer cleanup() + defer func() { returnErr = errors.Join(returnErr, cleanup()) }() } } count, err := migrateArchive(ctx, jobDB, transitionalDB, table, job, state.Archive) @@ -345,13 +347,13 @@ func prepareJob( var transitionalDB *gorm.DB var table string if state.Restore != nil && len(state.Restore.Tapes) == 0 { - var cleanup func() - transitionalDB, table, cleanup, err = openTransitionalJobDB(legacyJobsRoot, job.ID, "files", "restore_files") + var cleanup func() error + transitionalDB, table, cleanup, err = openTransitionalJobDB(workRoot, legacyJobsRoot, job.ID, "files", "restore_files") if err != nil { return itemCounts{}, nil, err } if transitionalDB != nil { - defer cleanup() + defer func() { returnErr = errors.Join(returnErr, cleanup()) }() } } count, err := migrateRestore( @@ -385,12 +387,12 @@ func prepareJob( } } -func openTransitionalJobDB(root string, jobID int64, tables ...string) (*gorm.DB, string, func(), error) { +func openTransitionalJobDB(work, root string, jobID int64, tables ...string) (*gorm.DB, string, func() error, error) { filename := filepath.Join(root, fmt.Sprint(jobID), "state.db") if !exists(filename) { return nil, "", nil, nil } - db, cleanup, err := openSnapshotCatalog(filename) + db, cleanup, err := openSnapshotCatalog(work, filename) if err != nil { return nil, "", nil, fmt.Errorf("open transitional Job DB failed, %w", err) } @@ -399,8 +401,7 @@ func openTransitionalJobDB(root string, jobID int64, tables ...string) (*gorm.DB return db, table, cleanup, nil } } - cleanup() - return nil, "", nil, nil + return nil, "", nil, cleanup() } func migrateLegacyJobLog(workRoot, jobDir string, jobID int64) ([]int64, error) { @@ -1346,7 +1347,7 @@ func Commit(ctx context.Context, db *gorm.DB, workRoot string) error { return err } -func RepairJob(ctx context.Context, db *gorm.DB, workRoot string, jobID int64, backup Backup) error { +func RepairJob(ctx context.Context, db *gorm.DB, workRoot string, jobID int64, backup Backup) (returnErr error) { // Repair operates only on a recognized published installation. if _, err := dataformat.CheckCatalog(db); err != nil { return err @@ -1364,11 +1365,11 @@ func RepairJob(ctx context.Context, db *gorm.DB, workRoot string, jobID int64, b } // Read the complete legacy backup without relying on active migration tables. - source, cleanup, err := backup.open() + source, cleanup, err := backup.open(workRoot) if err != nil { return err } - defer cleanup() + defer func() { returnErr = errors.Join(returnErr, cleanup()) }() var job legacyJob result := source.WithContext(ctx).First(&job, jobID) if result.Error != nil { @@ -1419,6 +1420,7 @@ func RepairJob(ctx context.Context, db *gorm.DB, workRoot string, jobID int64, b } counts, _, err := prepareJob( ctx, + workRoot, source, "positions_staging", restorePositionsOptional, @@ -1451,11 +1453,11 @@ func RepairJob(ctx context.Context, db *gorm.DB, workRoot string, jobID int64, b } // Publish the repaired runner state through the same revision allocator used by offline activation. - stateDB, cleanupState, err := openSnapshotCatalog(filepath.Join(activeDir, "state.db")) + stateDB, cleanupState, err := openSnapshotCatalog(workRoot, filepath.Join(activeDir, "state.db")) if err != nil { return err } - defer cleanupState() + defer func() { returnErr = errors.Join(returnErr, cleanupState()) }() var record executor.JobRecord if err := stateDB.First(&record, 1).Error; err != nil { return err diff --git a/internal/preview/generator_test.go b/internal/preview/generator_test.go index 77b3b02..0628952 100644 --- a/internal/preview/generator_test.go +++ b/internal/preview/generator_test.go @@ -56,7 +56,7 @@ func TestGenerationUsesLiveRuntimeAndFrozenOutputSettings(t *testing.T) { } else { live.Generators[0].GetVideo().PosterWidth = 200 } - manager, err := New(config, "") + manager, err := New(config, t.TempDir()) require.NoError(t, err) var lock sync.Mutex manager.settings = func(context.Context) (*entity.PreviewSettings, error) { @@ -147,7 +147,7 @@ func TestGenerationRequiresFrozenAndLiveRoute(t *testing.T) { frozen := &entity.PreviewJobSettings{Generators: proto.Clone(settings).(*entity.PreviewSettings).Generators} live := proto.Clone(settings).(*entity.PreviewSettings) live.Generators[0].Extensions = []*entity.PreviewExtension{{Name: "png", Enabled: true}} - manager, err := NewWithSettings(context.Background(), filepath.Join(root, "previews"), "", func(context.Context) (*entity.PreviewSettings, error) { + manager, err := NewWithSettings(context.Background(), filepath.Join(root, "previews"), t.TempDir(), func(context.Context) (*entity.PreviewSettings, error) { return proto.Clone(live).(*entity.PreviewSettings), nil }) require.NoError(t, err) @@ -162,7 +162,7 @@ func TestGenerationRequiresFrozenAndLiveRoute(t *testing.T) { func TestNativeCapabilitiesExposeRAWByImageKind(t *testing.T) { // Probe the packaged executable through the real manager/public capability projection. - manager, err := New(Config{Root: t.TempDir()}, "") + manager, err := New(Config{Root: t.TempDir()}, t.TempDir()) require.NoError(t, err) helper := testPreviewHelper(t) settings, err := SettingsFromConfig(Config{}) @@ -198,7 +198,7 @@ func TestBuiltInGenerators(t *testing.T) { "command": helper, "format": format, "timeline_width": 45, "timeline_height": 81, "timeline_interval_seconds": 1, "timeline_max_frames": 2, }}, - }}, "") + }}, t.TempDir()) require.NoError(t, err) // Generate the image and validate that its manifest resource remains decodable in the stored bundle. @@ -261,7 +261,7 @@ func TestBuiltInGeneratorsAcceptRelativeStoreAndSourcePaths(t *testing.T) { require.False(t, filepath.IsAbs(relativeStore)) previews, err := New(Config{Root: relativeStore, Generators: []GeneratorConfig{ {Kind: kind, Extensions: []string{extension}, Options: map[string]any{"command": helper, "format": "png"}}, - }}, "") + }}, t.TempDir()) require.NoError(t, err) signature := generateFixturePreview(t, previews, relativeSource) diff --git a/internal/preview/preview.go b/internal/preview/preview.go index 5a094ba..bd13978 100644 --- a/internal/preview/preview.go +++ b/internal/preview/preview.go @@ -17,6 +17,7 @@ import ( "github.com/samuelncui/yatm/entity" "github.com/samuelncui/yatm/internal/library" "github.com/samuelncui/yatm/internal/previewprotocol" + "github.com/samuelncui/yatm/internal/resource" "google.golang.org/protobuf/proto" ) @@ -45,6 +46,7 @@ type Asset struct { // Manager owns Preview generation, storage addressing, and indexed bundle reads. type Manager struct { root string + work string generators map[string]configuredGenerator settings func(context.Context) (*entity.PreviewSettings, error) lock sync.Mutex @@ -66,7 +68,7 @@ func New(config Config, work string) (*Manager, error) { if err := os.MkdirAll(root, 0o755); err != nil { return nil, fmt.Errorf("create Preview root failed, path=%q, %w", root, err) } - return &Manager{root: root, generators: generators, pending: make(map[string]*generation), changed: make(chan struct{})}, nil + return &Manager{root: root, work: work, generators: generators, pending: make(map[string]*generation), changed: make(chan struct{})}, nil } func (m *Manager) Supports(sourcePath string, settings *entity.PreviewJobSettings) bool { @@ -209,12 +211,13 @@ func (m *Manager) Generate( } ctx, cancel := context.WithTimeout(ctx, time.Duration(current.TimeoutSeconds)*time.Second) defer cancel() - // Generate bounded derivative files beside the content-addressed store. - temporaryDir, err := os.MkdirTemp(m.root, ".generate-") + // Generate bounded derivatives under the shared Work temporary root. + temporary, err := resource.NewTemporaryDirectory(m.work, "preview-") if err != nil { return nil, fmt.Errorf("create Preview temporary directory failed, %w", err) } - defer os.RemoveAll(temporaryDir) + defer func() { resultErr = errors.Join(resultErr, temporary.Close()) }() + temporaryDir := temporary.Directory reportProgress(ctx, previewprotocol.Event{Type: "progress", Phase: "generate"}) assets, err := configured.generator.Generate(ctx, sourcePath, temporaryDir) if err != nil { diff --git a/internal/preview/preview_test.go b/internal/preview/preview_test.go index 661d36f..a2dfa7a 100644 --- a/internal/preview/preview_test.go +++ b/internal/preview/preview_test.go @@ -56,7 +56,7 @@ func TestGenerateStoresAndReusesContentAddressedBundle(t *testing.T) { Generators: []GeneratorConfig{{ Kind: "test-fixture", Extensions: []string{"fixture"}, Options: map[string]any{"ignored": true}, }}, - }, "") + }, t.TempDir()) require.NoError(t, err) content := []byte("source fixture") sourcePath := filepath.Join(t.TempDir(), "source.fixture") @@ -117,7 +117,7 @@ func TestGenerateForceReplacesExistingBundles(t *testing.T) { hash := sha256.Sum256(content) initial, err := New(Config{Root: root, Generators: []GeneratorConfig{{ Kind: "test-fixture", Extensions: []string{"fixture"}, Options: map[string]any{"marker": "initial"}, - }}}, "") + }}}, t.TempDir()) require.NoError(t, err) signature, err := initial.Generate( context.Background(), sourcePath, hash[:], info.Size(), info.ModTime().UnixNano(), false, nil, @@ -127,7 +127,7 @@ func TestGenerateForceReplacesExistingBundles(t *testing.T) { // Missing-only generation keeps an existing bundle even when generator settings differ. updated, err := New(Config{Root: root, Generators: []GeneratorConfig{{ Kind: "test-fixture", Extensions: []string{"fixture"}, Options: map[string]any{"marker": "updated"}, - }}}, "") + }}}, t.TempDir()) require.NoError(t, err) _, err = updated.Generate( context.Background(), sourcePath, hash[:], info.Size(), info.ModTime().UnixNano(), false, nil, @@ -156,7 +156,7 @@ func TestGenerateForceReplacesExistingBundles(t *testing.T) { // Replace the same content bundle when its configured generator changes. next, err := New(Config{Root: root, Generators: []GeneratorConfig{{ Kind: "test-fixture-next", Extensions: []string{"fixture"}, Options: map[string]any{"marker": "updated"}, - }}}, "") + }}}, t.TempDir()) require.NoError(t, err) _, err = next.Generate( context.Background(), sourcePath, hash[:], info.Size(), info.ModTime().UnixNano(), true, nil, @@ -179,7 +179,7 @@ func TestGenerateForceReplacesExistingBundles(t *testing.T) { } func TestManifestRejectsInvalidSignature(t *testing.T) { - manager, err := New(Config{Root: t.TempDir()}, "") + manager, err := New(Config{Root: t.TempDir()}, t.TempDir()) require.NoError(t, err) _, err = manager.Manifest([]byte{1}) require.ErrorContains(t, err, "invalid file signature length") diff --git a/internal/preview/settings_test.go b/internal/preview/settings_test.go index fd64458..70f7353 100644 --- a/internal/preview/settings_test.go +++ b/internal/preview/settings_test.go @@ -16,7 +16,7 @@ func TestNewWithSettingsUsesTypedDefaultsAndLiveGetter(t *testing.T) { Options: map[string]any{"command": "legacy-ffmpeg", "max_width": 240, "max_height": 200, "quality": 60}, }}}) require.NoError(t, err) - manager, err := NewWithSettings(context.Background(), t.TempDir(), "", func(context.Context) (*entity.PreviewSettings, error) { + manager, err := NewWithSettings(context.Background(), t.TempDir(), t.TempDir(), func(context.Context) (*entity.PreviewSettings, error) { return proto.Clone(stored).(*entity.PreviewSettings), nil }) require.NoError(t, err) diff --git a/internal/resource/temporary.go b/internal/resource/temporary.go index 3b6c1e1..2178ab6 100644 --- a/internal/resource/temporary.go +++ b/internal/resource/temporary.go @@ -3,7 +3,6 @@ package resource import ( "errors" "fmt" - "os" "path/filepath" "gorm.io/gorm" @@ -14,21 +13,22 @@ import ( type TemporaryDB struct { DB *gorm.DB Directory string + owner *TemporaryDirectory } -func OpenTemporaryDB(root, prefix string) (_ *TemporaryDB, rerr error) { +func OpenTemporaryDB(work, prefix string) (_ *TemporaryDB, rerr error) { // Retain ownership during construction so a failed open cannot leave a directory. - directory, err := os.MkdirTemp(root, prefix) + owner, err := NewTemporaryDirectory(work, prefix) if err != nil { return nil, fmt.Errorf("create temporary database directory failed, %w", err) } - stage := &TemporaryDB{Directory: directory} + stage := &TemporaryDB{Directory: owner.Directory, owner: owner} defer func() { if rerr != nil { rerr = errors.Join(rerr, stage.Close()) } }() - stage.DB, err = OpenSQLite(filepath.Join(directory, "state.sqlite")) + stage.DB, err = OpenSQLite(filepath.Join(owner.Directory, "state.sqlite")) if err != nil { return nil, err } @@ -42,9 +42,9 @@ func (s *TemporaryDB) Close() error { result = closeDB(s.DB) s.DB = nil } - if s.Directory != "" { - result = errors.Join(result, os.RemoveAll(s.Directory)) - s.Directory = "" + if s.owner != nil { + result = errors.Join(result, s.owner.Close()) + s.Directory = s.owner.Directory } return result } diff --git a/internal/resource/temporary_directory.go b/internal/resource/temporary_directory.go new file mode 100644 index 0000000..1b5a5c1 --- /dev/null +++ b/internal/resource/temporary_directory.go @@ -0,0 +1,189 @@ +package resource + +import ( + "context" + "errors" + "fmt" + "io" + "os" + "path/filepath" + "strings" + "sync" + "time" +) + +const ( + temporaryTimestamp = "20060102T150405.000000000Z" + temporaryRetention = 7 * 24 * time.Hour + temporarySweep = time.Hour + + // TemporaryKeyPrefix names transient Tape key allocations; cleanup removes them whenever unheld. + TemporaryKeyPrefix = "tape-key-" +) + +var temporaryOwners = struct { + sync.Mutex + paths map[string]struct{} +}{paths: make(map[string]struct{})} + +// TemporaryDirectory owns disposable files under Work. Close removes them immediately; +// failed removal retains Directory for a later Close or the age-based cleanup. +type TemporaryDirectory struct { + Directory string +} + +// TemporaryRoot returns the sole root for disposable runtime files. +func TemporaryRoot(work string) string { return filepath.Join(work, "temporary") } + +func prepareTemporaryRoot(work string) (string, error) { + // An explicit Work directory keeps disposable data out of shared system storage. + if work == "" { + return "", errors.New("temporary storage requires a configured Work directory") + } + root, err := filepath.Abs(TemporaryRoot(work)) + if err != nil { + return "", fmt.Errorf("resolve temporary root failed, %w", err) + } + if err := os.MkdirAll(root, 0700); err != nil { + return "", fmt.Errorf("create temporary root failed, %w", err) + } + + // Refuse a redirected root before any recursive deletion can use it. + info, err := os.Lstat(root) + if err != nil { + return "", fmt.Errorf("inspect temporary root failed, %w", err) + } + if !info.IsDir() { + return "", fmt.Errorf("temporary root is not a directory: %q", root) + } + return root, nil +} + +// NewTemporaryDirectory reserves one UTC-prefixed directory until Close finishes its use. +func NewTemporaryDirectory(work, prefix string) (*TemporaryDirectory, error) { + // Publish each unique directory with its hold before a sweep can inspect ownership. + temporaryOwners.Lock() + defer temporaryOwners.Unlock() + root, err := prepareTemporaryRoot(work) + if err != nil { + return nil, err + } + directory, err := os.MkdirTemp(root, time.Now().UTC().Format(temporaryTimestamp)+"-"+prefix) + if err != nil { + return nil, fmt.Errorf("create temporary directory failed, %w", err) + } + temporaryOwners.paths[directory] = struct{}{} + return &TemporaryDirectory{Directory: directory}, nil +} + +func (d *TemporaryDirectory) Close() error { + // Keep the live hold through removal, then make failed cleanup available to the age-based sweep. + if d.Directory == "" { + return nil + } + path := d.Directory + defer func() { + temporaryOwners.Lock() + delete(temporaryOwners.paths, path) + temporaryOwners.Unlock() + }() + if err := removeTemporaryDirectory(d.Directory); err != nil { + return fmt.Errorf("remove temporary directory %q failed, %w", d.Directory, err) + } + d.Directory = "" + return nil +} + +// PrepareTemporary validates and creates the disposable root without removing leftovers. +func PrepareTemporary(work string) error { + _, err := prepareTemporaryRoot(work) + return err +} + +// CleanupTemporary removes abandoned UTC-prefixed directories older than seven days and every +// unheld Tape key directory regardless of age. Durable Job bundles, recovery material and LTFS +// mount points live outside this root. +func CleanupTemporary(work string, now time.Time) (result error) { + // Page a private root without holding up unrelated temporary allocations. + root, err := prepareTemporaryRoot(work) + if err != nil { + return err + } + directory, err := os.Open(root) + if err != nil { + return fmt.Errorf("open temporary root failed, %w", err) + } + defer func() { result = errors.Join(result, directory.Close()) }() + for { + entries, readErr := directory.ReadDir(128) + if readErr != nil && !errors.Is(readErr, io.EOF) { + return errors.Join(result, fmt.Errorf("list temporary root failed, %w", readErr)) + } + + // The name records creation time; modified contents cannot renew retention. + // An unheld key directory is never in use, so its secret is not retained for the age limit. + for _, entry := range entries { + name := entry.Name() + if !entry.IsDir() || len(name) <= len(temporaryTimestamp) || name[len(temporaryTimestamp)] != '-' { + continue + } + created, err := time.Parse(temporaryTimestamp, name[:len(temporaryTimestamp)]) + if err != nil { + continue + } + key := strings.HasPrefix(name[len(temporaryTimestamp)+1:], TemporaryKeyPrefix) + if !key && !created.Before(now.Add(-temporaryRetention)) { + continue + } + path := filepath.Join(root, name) + temporaryOwners.Lock() + _, held := temporaryOwners.paths[path] + temporaryOwners.Unlock() + if held { + continue + } + + // Unique allocation never adopts an existing expired directory after this decision. + if err := removeTemporaryDirectory(path); err != nil { + result = errors.Join(result, fmt.Errorf("remove stale temporary directory %q failed, %w", path, err)) + } + } + if errors.Is(readErr, io.EOF) { + return result + } + } +} + +// StartTemporaryCleanup sweeps at startup and hourly until stopped. The caller stops it +// after request and Job owners have drained; background failures are reported for diagnosis. +func StartTemporaryCleanup(ctx context.Context, work string, report func(error)) (func(), error) { + return startTemporaryCleanup(ctx, work, temporarySweep, report) +} + +func startTemporaryCleanup(ctx context.Context, work string, interval time.Duration, report func(error)) (func(), error) { + // A usable root is required; an individual leftover must not block service startup. + if err := PrepareTemporary(work); err != nil { + return nil, err + } + if err := CleanupTemporary(work, time.Now()); err != nil { + report(err) + } + ctx, cancel := context.WithCancel(ctx) + done := make(chan struct{}) + go func() { + defer close(done) + ticker := time.NewTicker(interval) + defer ticker.Stop() + for { + select { + case <-ctx.Done(): + return + case now := <-ticker.C: + if err := CleanupTemporary(work, now); err != nil { + report(err) + } + } + } + }() + return func() { cancel(); <-done }, nil +} diff --git a/internal/resource/temporary_directory_other.go b/internal/resource/temporary_directory_other.go new file mode 100644 index 0000000..0fa5e1f --- /dev/null +++ b/internal/resource/temporary_directory_other.go @@ -0,0 +1,9 @@ +//go:build !linux && !darwin && !freebsd + +package resource + +import "fmt" + +func temporaryParentIdentity(path string) (uint64, uint64, error) { + return 0, 0, fmt.Errorf("Restore probe parent identity is unavailable on this platform: %q", path) +} diff --git a/internal/resource/temporary_directory_restore.go b/internal/resource/temporary_directory_restore.go new file mode 100644 index 0000000..2a2a198 --- /dev/null +++ b/internal/resource/temporary_directory_restore.go @@ -0,0 +1,153 @@ +package resource + +import ( + "encoding/json" + "errors" + "fmt" + "io" + "os" + "path/filepath" + "strings" + + "github.com/google/uuid" +) + +const restoreProbeRecords = ".restore-probes" + +type restoreProbeRecord struct { + Path string `json:"path"` + Device uint64 `json:"device"` + Inode uint64 `json:"inode"` +} + +// RegisterRestoreProbe records an exact destination-side name probe before the caller +// creates it. Both immediate and expired cleanup retain this record until removal succeeds. +func (d *TemporaryDirectory) RegisterRestoreProbe(path string) (result error) { + // Only Restore's private directory-only namespace can be registered for external cleanup. + if d.Directory == "" || !validRestoreProbe(path) { + return fmt.Errorf("invalid Restore probe cleanup path %q", path) + } + device, inode, err := temporaryParentIdentity(filepath.Dir(path)) + if err != nil { + return fmt.Errorf("inspect Restore probe parent failed, %w", err) + } + records := filepath.Join(d.Directory, restoreProbeRecords) + if err := os.MkdirAll(records, 0700); err != nil { + return fmt.Errorf("create Restore probe records failed, %w", err) + } + + // A partial record cannot authorize cleanup because its probe has not been created yet. + file, err := os.CreateTemp(records, "*.pending") + if err != nil { + return fmt.Errorf("create Restore probe record failed, %w", err) + } + pending := file.Name() + defer func() { + if err := os.Remove(pending); err != nil && !os.IsNotExist(err) { + result = errors.Join(result, err) + } + }() + writeErr := json.NewEncoder(file).Encode(restoreProbeRecord{Path: path, Device: device, Inode: inode}) + if err := errors.Join(writeErr, file.Close()); err != nil { + return fmt.Errorf("write Restore probe record failed, %w", err) + } + if err := os.Rename(pending, strings.TrimSuffix(pending, ".pending")); err != nil { + return fmt.Errorf("publish Restore probe record failed, %w", err) + } + return nil +} + +func validRestoreProbe(path string) bool { + name := filepath.Base(path) + encoded := strings.TrimSuffix(strings.TrimPrefix(name, ".yatm-restore-"), "-\u00e9") + _, err := uuid.Parse(encoded) + return filepath.IsAbs(path) && filepath.Clean(path) == path && err == nil && + name == ".yatm-restore-"+encoded+"-\u00e9" +} + +func removeTemporaryDirectory(path string) error { + // External probes must be settled before deleting the only records that identify them. + if err := removeRestoreProbes(filepath.Join(path, restoreProbeRecords)); err != nil { + return err + } + return os.RemoveAll(path) +} + +func removeRestoreProbes(path string) (result error) { + // Page records independently so one unavailable target cannot strand other destinations. + info, err := os.Lstat(path) + if os.IsNotExist(err) { + return nil + } + if err != nil { + return fmt.Errorf("inspect Restore probe records failed, %w", err) + } + if !info.IsDir() { + return fmt.Errorf("Restore probe records are not a real directory: %q", path) + } + directory, err := os.Open(path) + if err != nil { + return fmt.Errorf("open Restore probe records failed, %w", err) + } + defer func() { result = errors.Join(result, directory.Close()) }() + for { + entries, readErr := directory.ReadDir(128) + if readErr != nil && !errors.Is(readErr, io.EOF) { + return errors.Join(result, fmt.Errorf("list Restore probe records failed, %w", readErr)) + } + for _, entry := range entries { + record := filepath.Join(path, entry.Name()) + if !entry.Type().IsRegular() { + result = errors.Join(result, fmt.Errorf("Restore probe record is not a regular file: %q", record)) + continue + } + if strings.HasSuffix(entry.Name(), ".pending") { + result = errors.Join(result, os.Remove(record)) + continue + } + if err := removeRestoreProbe(record); err != nil { + result = errors.Join(result, err) + continue + } + result = errors.Join(result, os.Remove(record)) + } + if errors.Is(readErr, io.EOF) { + return result + } + } +} + +func removeRestoreProbe(path string) error { + // Bound disposable record reads; malformed records remain available for diagnosis. + file, err := os.Open(path) + if err != nil { + return fmt.Errorf("open Restore probe record failed, %w", err) + } + data, readErr := io.ReadAll(io.LimitReader(file, (64<<10)+1)) + if err := errors.Join(readErr, file.Close()); err != nil { + return fmt.Errorf("read Restore probe record failed, %w", err) + } + if len(data) > 64<<10 { + return fmt.Errorf("Restore probe record is too large: %q", path) + } + var record restoreProbeRecord + if err := json.Unmarshal(data, &record); err != nil { + return fmt.Errorf("decode Restore probe record failed, %w", err) + } + if !validRestoreProbe(record.Path) { + return fmt.Errorf("invalid Restore probe cleanup path %q", record.Path) + } + + // An absent or remounted destination is not evidence that its old probe is already gone. + device, inode, err := temporaryParentIdentity(filepath.Dir(record.Path)) + if err != nil { + return fmt.Errorf("inspect Restore probe parent failed, path=%q, %w", record.Path, err) + } + if device != record.Device || inode != record.Inode { + return fmt.Errorf("Restore probe parent is unavailable, path=%q", record.Path) + } + if err := os.RemoveAll(record.Path); err != nil { + return fmt.Errorf("remove Restore probe failed, path=%q, %w", record.Path, err) + } + return nil +} diff --git a/internal/resource/temporary_directory_restore_test.go b/internal/resource/temporary_directory_restore_test.go new file mode 100644 index 0000000..e0eaaa8 --- /dev/null +++ b/internal/resource/temporary_directory_restore_test.go @@ -0,0 +1,166 @@ +package resource + +import ( + "context" + "encoding/json" + "fmt" + "os" + "os/exec" + "path/filepath" + "testing" + "time" + + "github.com/stretchr/testify/require" +) + +func createRestoreProbe(t *testing.T, owner *TemporaryDirectory, parent string, id int) string { + t.Helper() + path := filepath.Join(parent, fmt.Sprintf(".yatm-restore-00000000-0000-4000-8000-%012d-é", id)) + require.NoError(t, owner.RegisterRestoreProbe(path)) + require.NoError(t, os.Mkdir(path, 0700)) + return path +} + +func TestTemporaryRestoreProbeClosePreservesUnregisteredDirectories(t *testing.T) { + // Registration does not create the native namespace; its exact record precedes that side effect. + owner, err := NewTemporaryDirectory(t.TempDir(), "restore-output-names-") + require.NoError(t, err) + t.Cleanup(func() { require.NoError(t, owner.Close()) }) + parent := t.TempDir() + path := filepath.Join(parent, ".yatm-restore-00000000-0000-4000-8000-000000000001-é") + require.NoError(t, owner.RegisterRestoreProbe(path)) + require.NoDirExists(t, path) + records, err := os.ReadDir(filepath.Join(owner.Directory, restoreProbeRecords)) + require.NoError(t, err) + require.Len(t, records, 1) + data, err := os.ReadFile(filepath.Join(owner.Directory, restoreProbeRecords, records[0].Name())) + require.NoError(t, err) + var record restoreProbeRecord + require.NoError(t, json.Unmarshal(data, &record)) + require.Equal(t, path, record.Path) + require.NoError(t, os.MkdirAll(filepath.Join(path, "nested", "file.txt"), 0700)) + + // Native probes are directory-only, and neighboring user directories are never searched or removed. + unrelated := filepath.Join(parent, ".yatm-restore-unregistered") + require.NoError(t, os.Mkdir(unrelated, 0700)) + require.Error(t, owner.RegisterRestoreProbe(unrelated)) + index := owner.Directory + require.NoError(t, owner.Close()) + require.NoDirExists(t, path) + require.NoDirExists(t, index) + require.DirExists(t, unrelated) +} + +func TestTemporaryRestoreProbeExpiryAfterProcessExit(t *testing.T) { + // A new service has no process-local hold for the interrupted allocation. + if work := os.Getenv("YATM_RESTORE_PROBE_EXIT_TEST_WORK"); work != "" { + owner, err := NewTemporaryDirectory(work, "restore-output-names-") + require.NoError(t, err) + parent := filepath.Join(work, "destination") + require.NoError(t, os.Mkdir(parent, 0700)) + createRestoreProbe(t, owner, parent, 2) + stale := filepath.Join(TemporaryRoot(work), time.Now().Add(-8*24*time.Hour).UTC().Format(temporaryTimestamp)+"-restore-output-names-interrupted") + require.NoError(t, os.Rename(owner.Directory, stale)) + os.Exit(0) + } + work := t.TempDir() + command := exec.Command(os.Args[0], "-test.run=^TestTemporaryRestoreProbeExpiryAfterProcessExit$") + command.Env = append(os.Environ(), "YATM_RESTORE_PROBE_EXIT_TEST_WORK="+work) + output, err := command.CombinedOutput() + require.NoError(t, err, "%s", output) + + // Startup uses the same external removal path as Close, without an existing Job or runner. + stop, err := StartTemporaryCleanup(context.Background(), work, func(err error) { t.Errorf("cleanup failed: %v", err) }) + require.NoError(t, err) + stop() + entries, err := os.ReadDir(TemporaryRoot(work)) + require.NoError(t, err) + require.Empty(t, entries) + entries, err = os.ReadDir(filepath.Join(work, "destination")) + require.NoError(t, err) + require.Empty(t, entries) +} + +func TestTemporaryRestoreProbeActiveOwnerSurvivesExpiry(t *testing.T) { + // Creation age never overrides a live operation's hold, including its external namespaces. + work := t.TempDir() + owner, err := NewTemporaryDirectory(work, "restore-output-names-") + require.NoError(t, err) + t.Cleanup(func() { require.NoError(t, owner.Close()) }) + path := createRestoreProbe(t, owner, t.TempDir(), 3) + require.NoError(t, CleanupTemporary(work, time.Now().Add(8*24*time.Hour))) + require.DirExists(t, owner.Directory) + require.DirExists(t, path) +} + +func TestTemporaryRestoreProbeUnavailableParentRetainsOnlyFailedRecords(t *testing.T) { + // An absent or replaced mount parent cannot turn a missing probe into confirmed cleanup. + work := t.TempDir() + owner, err := NewTemporaryDirectory(work, "restore-output-names-") + require.NoError(t, err) + t.Cleanup(func() { require.NoError(t, owner.Close()) }) + parent := filepath.Join(t.TempDir(), "destination") + require.NoError(t, os.Mkdir(parent, 0700)) + path := createRestoreProbe(t, owner, parent, 4) + other := createRestoreProbe(t, owner, t.TempDir(), 5) + parked := parent + "-offline" + require.NoError(t, os.Rename(parent, parked)) + index := owner.Directory + require.Error(t, owner.Close()) + require.NoDirExists(t, other) + records, err := os.ReadDir(filepath.Join(index, restoreProbeRecords)) + require.NoError(t, err) + require.Len(t, records, 1) + + // A different parent at the old spelling and its contents remain untouched on the next sweep. + require.NoError(t, os.Mkdir(parent, 0700)) + require.NoError(t, os.Mkdir(path, 0700)) + require.NoError(t, os.WriteFile(filepath.Join(path, "keep"), []byte("unrelated"), 0600)) + require.ErrorContains(t, CleanupTemporary(work, time.Now().Add(8*24*time.Hour)), "parent is unavailable") + require.FileExists(t, filepath.Join(path, "keep")) + require.DirExists(t, index) + + // Restoring the original parent makes the original exact namespace reclaimable. + require.NoError(t, os.RemoveAll(parent)) + require.NoError(t, os.Rename(parked, parent)) + require.NoError(t, CleanupTemporary(work, time.Now().Add(8*24*time.Hour))) + require.NoDirExists(t, path) + require.NoDirExists(t, index) +} + +func TestTemporaryRestoreProbeCleanupPagesRecords(t *testing.T) { + // A complete external-record pass is bounded by directory pages rather than a retained path list. + owner, err := NewTemporaryDirectory(t.TempDir(), "restore-output-names-") + require.NoError(t, err) + t.Cleanup(func() { require.NoError(t, owner.Close()) }) + parent := t.TempDir() + for i := range 257 { + createRestoreProbe(t, owner, parent, i) + } + require.NoError(t, owner.Close()) + entries, err := os.ReadDir(parent) + require.NoError(t, err) + require.Empty(t, entries) +} + +func TestTemporaryRestoreProbeCleanupRejectsMalformedAndLinkedRecords(t *testing.T) { + // Invalid or linked published records cannot authorize deletion outside the registered namespace. + owner, err := NewTemporaryDirectory(t.TempDir(), "restore-output-names-") + require.NoError(t, err) + parent := t.TempDir() + path := createRestoreProbe(t, owner, parent, 6) + records := filepath.Join(owner.Directory, restoreProbeRecords) + unrelated := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(unrelated, "keep"), []byte("original"), 0600)) + require.NoError(t, os.WriteFile(filepath.Join(records, "unpublished.pending"), []byte(unrelated), 0600)) + require.NoError(t, os.WriteFile(filepath.Join(records, "invalid"), []byte(`{"path":"`+unrelated+`"}`), 0600)) + require.NoError(t, os.Symlink(filepath.Join(unrelated, "keep"), filepath.Join(records, "linked"))) + require.Error(t, owner.Close()) + require.NoDirExists(t, path) + require.FileExists(t, filepath.Join(unrelated, "keep")) + require.NoFileExists(t, filepath.Join(records, "unpublished.pending")) + require.FileExists(t, filepath.Join(records, "invalid")) + require.NoError(t, os.Remove(filepath.Join(records, "invalid"))) + require.NoError(t, os.Remove(filepath.Join(records, "linked"))) + require.NoError(t, owner.Close()) +} diff --git a/internal/resource/temporary_directory_test.go b/internal/resource/temporary_directory_test.go new file mode 100644 index 0000000..e94183c --- /dev/null +++ b/internal/resource/temporary_directory_test.go @@ -0,0 +1,244 @@ +package resource + +import ( + "context" + "fmt" + "os" + "os/exec" + "path/filepath" + "sync" + "testing" + "time" + + "github.com/stretchr/testify/require" +) + +func TestTemporaryCleanupHonorsAgeAndLiveOwners(t *testing.T) { + // Disposable names carry UTC creation time; durable and unfamiliar entries are excluded. + work := t.TempDir() + live, err := NewTemporaryDirectory(work, "live-") + require.NoError(t, err) + t.Cleanup(func() { require.NoError(t, live.Close()) }) + name := filepath.Base(live.Directory) + require.Greater(t, len(name), len(temporaryTimestamp)) + created, err := time.Parse(temporaryTimestamp, name[:len(temporaryTimestamp)]) + require.NoError(t, err) + require.WithinDuration(t, time.Now(), created, time.Minute, "directory prefix must record UTC creation time") + now := time.Now().Add(8 * 24 * time.Hour) + root := TemporaryRoot(work) + old := filepath.Join(root, now.Add(-8*24*time.Hour).UTC().Format(temporaryTimestamp)+"-abandoned") + recent := filepath.Join(root, now.Add(-6*24*time.Hour).UTC().Format(temporaryTimestamp)+"-recent") + boundary := filepath.Join(root, now.Add(-7*24*time.Hour).UTC().Format(temporaryTimestamp)+"-boundary") + unknown := filepath.Join(root, "unknown") + durable := filepath.Join(work, "jobs", "1") + for _, path := range []string{old, recent, boundary, unknown, durable} { + require.NoError(t, os.MkdirAll(path, 0700)) + require.NoError(t, os.WriteFile(filepath.Join(path, "keep"), []byte("content"), 0600)) + } + outside := t.TempDir() + marker := filepath.Join(outside, "keep") + require.NoError(t, os.WriteFile(marker, []byte("outside"), 0600)) + link := filepath.Join(root, now.Add(-8*24*time.Hour).UTC().Format(temporaryTimestamp)+"-link") + require.NoError(t, os.Symlink(outside, link)) + + // An old live operation survives while abandoned content is removed recursively. + require.NoError(t, CleanupTemporary(work, now)) + require.NoDirExists(t, old) + for _, path := range []string{live.Directory, recent, boundary, unknown, durable} { + require.DirExists(t, path) + } + require.FileExists(t, marker) + require.FileExists(t, filepath.Join(link, "keep")) +} + +func TestTemporaryDBCloseRetainsFailedRemovalForRetry(t *testing.T) { + // Deny unlinking the owned directory without injecting an artificial filesystem implementation. + if os.Geteuid() == 0 { + t.Skip("permission fixture requires an unprivileged user") + } + work := t.TempDir() + stage, err := OpenTemporaryDB(work, "retry-") + require.NoError(t, err) + path := stage.Directory + root := TemporaryRoot(work) + t.Cleanup(func() { + require.NoError(t, os.Chmod(root, 0700)) + require.NoError(t, stage.Close()) + }) + require.NoError(t, os.Chmod(root, 0500)) + + // A failed removal retains the exact path and owner after the database is closed. + require.Error(t, stage.Close()) + require.Equal(t, path, stage.Directory) + require.Nil(t, stage.DB) + require.NoError(t, os.Chmod(root, 0700)) + require.NoError(t, stage.Close()) + require.Empty(t, stage.Directory) + require.NoDirExists(t, path) +} + +func TestTemporaryCleanupRejectsLinkedRoot(t *testing.T) { + // A root symlink cannot redirect a startup sweep into another owner's files. + work, outside := t.TempDir(), t.TempDir() + require.NoError(t, os.Symlink(outside, TemporaryRoot(work))) + require.Error(t, CleanupTemporary(work, time.Now())) + _, err := NewTemporaryDirectory(work, "test-") + require.Error(t, err) + _, err = NewTemporaryDirectory("", "test-") + require.Error(t, err) +} + +func TestTemporaryCleanupServiceStops(t *testing.T) { + // Startup creates the private root and cancellation drains its background owner. + work := t.TempDir() + stop, err := StartTemporaryCleanup(context.Background(), work, func(err error) { t.Errorf("cleanup failed: %v", err) }) + require.NoError(t, err) + require.DirExists(t, TemporaryRoot(work)) + stop() + stop() +} + +func TestTemporaryCleanupServiceSweepsPeriodically(t *testing.T) { + // Create the leftover after startup so only the periodic pass can remove it. + work := t.TempDir() + stop, err := startTemporaryCleanup(context.Background(), work, 10*time.Millisecond, func(err error) { t.Errorf("cleanup failed: %v", err) }) + require.NoError(t, err) + defer stop() + stale := filepath.Join(TemporaryRoot(work), time.Now().Add(-8*24*time.Hour).UTC().Format(temporaryTimestamp)+"-leftover") + require.NoError(t, os.Mkdir(stale, 0700)) + + // The recurring owner removes the abandoned directory without another explicit sweep. + require.Eventually(t, func() bool { _, err := os.Stat(stale); return os.IsNotExist(err) }, 5*time.Second, 10*time.Millisecond) +} + +func TestTemporaryCleanupConcurrentOwners(t *testing.T) { + // Allocation and sweeps overlap while each operation owns an independent directory. + work := t.TempDir() + var group sync.WaitGroup + for range 8 { + group.Go(func() { + for range 16 { + directory, err := NewTemporaryDirectory(work, "concurrent-") + if err != nil { + t.Error(err) + return + } + if err := CleanupTemporary(work, time.Now().Add(8*24*time.Hour)); err != nil { + t.Error(err) + } + if _, err := os.Stat(directory.Directory); err != nil { + t.Error(err) + } + if err := directory.Close(); err != nil { + t.Error(err) + } + } + }) + } + group.Wait() +} + +func TestTemporaryCleanupAfterProcessExit(t *testing.T) { + // The child exits without Close, leaving no process-local owner in the next service. + if work := os.Getenv("YATM_TEMPORARY_EXIT_TEST_WORK"); work != "" { + directory, err := NewTemporaryDirectory(work, "interrupted-") + require.NoError(t, err) + stale := filepath.Join(TemporaryRoot(work), time.Now().Add(-8*24*time.Hour).UTC().Format(temporaryTimestamp)+"-interrupted") + require.NoError(t, os.Rename(directory.Directory, stale)) + require.NoError(t, os.WriteFile(filepath.Join(stale, "state.sqlite"), []byte("unfinished disposable state"), 0600)) + os.Exit(0) + } + work := t.TempDir() + command := exec.Command(os.Args[0], "-test.run=^TestTemporaryCleanupAfterProcessExit$") + command.Env = append(os.Environ(), "YATM_TEMPORARY_EXIT_TEST_WORK="+work) + output, err := command.CombinedOutput() + require.NoError(t, err, "%s", output) + entries, err := os.ReadDir(TemporaryRoot(work)) + require.NoError(t, err) + require.Len(t, entries, 1) + + // A fresh service startup removes the expired leftover before admitting requests. + stop, err := StartTemporaryCleanup(context.Background(), work, func(err error) { t.Errorf("cleanup failed: %v", err) }) + require.NoError(t, err) + stop() + entries, err = os.ReadDir(TemporaryRoot(work)) + require.NoError(t, err) + require.Empty(t, entries) +} + +func TestTemporaryCleanupStartupWarnsAndRetriesUnremovableLeftover(t *testing.T) { + // Root preparation does not sweep; failed deletion of one old directory leaves the root usable. + if os.Geteuid() == 0 { + t.Skip("permission fixture requires an unprivileged user") + } + work := t.TempDir() + require.NoError(t, PrepareTemporary(work)) + stale := filepath.Join(TemporaryRoot(work), time.Now().Add(-8*24*time.Hour).UTC().Format(temporaryTimestamp)+"-blocked") + require.NoError(t, os.Mkdir(stale, 0700)) + require.NoError(t, os.WriteFile(filepath.Join(stale, "state.sqlite"), []byte("unfinished"), 0600)) + t.Cleanup(func() { _ = os.Chmod(stale, 0700) }) + require.NoError(t, os.Chmod(stale, 0500)) + require.NoError(t, PrepareTemporary(work)) + require.DirExists(t, stale) + + // Startup reports the stale deletion failure while admitting new temporary owners. + warnings := make(chan error, 1) + stop, err := startTemporaryCleanup(context.Background(), work, 10*time.Millisecond, func(err error) { + select { + case warnings <- err: + default: + } + }) + require.NoError(t, err) + defer stop() + select { + case warning := <-warnings: + require.ErrorIs(t, warning, os.ErrPermission) + default: + t.Fatal("startup must report the stale deletion failure") + } + current, err := NewTemporaryDirectory(work, "admitted-") + require.NoError(t, err) + require.NoError(t, current.Close()) + + // The existing cadence retries the same leftover after its filesystem permissions recover. + require.NoError(t, os.Chmod(stale, 0700)) + require.Eventually(t, func() bool { _, err := os.Stat(stale); return os.IsNotExist(err) }, 5*time.Second, 10*time.Millisecond) +} + +func TestTemporaryCleanupPagesEntries(t *testing.T) { + // More leftovers than one read page must all be visited while entries are removed. + work := t.TempDir() + require.NoError(t, PrepareTemporary(work)) + prefix := time.Now().Add(-8 * 24 * time.Hour).UTC().Format(temporaryTimestamp) + for i := range 300 { + require.NoError(t, os.Mkdir(filepath.Join(TemporaryRoot(work), fmt.Sprintf("%s-leftover-%d", prefix, i)), 0700)) + } + + // A bounded directory scan still completes the whole expiration pass. + require.NoError(t, CleanupTemporary(work, time.Now())) + entries, err := os.ReadDir(TemporaryRoot(work)) + require.NoError(t, err) + require.Empty(t, entries) +} + +func TestTemporaryCleanupRemovesUnheldKeysRegardlessOfAge(t *testing.T) { + // A fresh abandoned key is removed at once while live keys and fresh ordinary work remain. + work := t.TempDir() + live, err := NewTemporaryDirectory(work, TemporaryKeyPrefix) + require.NoError(t, err) + t.Cleanup(func() { require.NoError(t, live.Close()) }) + prefix := time.Now().UTC().Format(temporaryTimestamp) + abandoned := filepath.Join(TemporaryRoot(work), prefix+"-"+TemporaryKeyPrefix+"abandoned") + recent := filepath.Join(TemporaryRoot(work), prefix+"-script-output-recent") + for _, path := range []string{abandoned, recent} { + require.NoError(t, os.Mkdir(path, 0700)) + } + require.NoError(t, os.WriteFile(filepath.Join(abandoned, "key"), []byte("secret"), 0600)) + + // Only the unheld key directory leaves the root. + require.NoError(t, CleanupTemporary(work, time.Now())) + require.NoDirExists(t, abandoned) + require.DirExists(t, recent) + require.DirExists(t, live.Directory) +} diff --git a/internal/resource/temporary_directory_unix.go b/internal/resource/temporary_directory_unix.go new file mode 100644 index 0000000..863ad8e --- /dev/null +++ b/internal/resource/temporary_directory_unix.go @@ -0,0 +1,22 @@ +//go:build linux || darwin || freebsd + +package resource + +import ( + "fmt" + "os" + "syscall" +) + +func temporaryParentIdentity(path string) (uint64, uint64, error) { + // A real parent must still name the filesystem that held the registered probe. + info, err := os.Lstat(path) + if err != nil { + return 0, 0, err + } + stat, ok := info.Sys().(*syscall.Stat_t) + if !info.IsDir() || !ok { + return 0, 0, fmt.Errorf("Restore probe parent is not a real native directory: %q", path) + } + return uint64(stat.Dev), uint64(stat.Ino), nil +} diff --git a/internal/resource/temporary_test.go b/internal/resource/temporary_test.go index 618f953..414cdad 100644 --- a/internal/resource/temporary_test.go +++ b/internal/resource/temporary_test.go @@ -30,7 +30,7 @@ func TestTemporaryDBOwnsOnlyItsDirectory(t *testing.T) { _, err = os.Stat(path) require.ErrorIs(t, err, os.ErrNotExist) require.NoError(t, second.DB.Exec("CREATE TABLE item(id INTEGER PRIMARY KEY)").Error) - entries, err := os.ReadDir(root) + entries, err := os.ReadDir(TemporaryRoot(root)) require.NoError(t, err) require.Len(t, entries, 1) require.Equal(t, filepath.Base(second.Directory), entries[0].Name()) diff --git a/internal/tools/cmd.go b/internal/tools/cmd.go index 40d8eeb..10dd3a0 100644 --- a/internal/tools/cmd.go +++ b/internal/tools/cmd.go @@ -6,29 +6,36 @@ import ( "io/fs" "os" "os/exec" + "path/filepath" + "github.com/samuelncui/yatm/internal/resource" "github.com/sirupsen/logrus" ) -func RunCmdWithReturn(logger *logrus.Logger, cmd *exec.Cmd) ([]byte, error) { - out, err := os.CreateTemp("", "*.out") +func RunCmdWithReturn(logger *logrus.Logger, cmd *exec.Cmd, work string) (_ []byte, rerr error) { + // The script's OUT adapter remains a file, owned through command completion and readback. + temporary, err := resource.NewTemporaryDirectory(work, "script-output-") if err != nil { - return nil, fmt.Errorf("create cmd out fail, %w", err) + return nil, fmt.Errorf("create command output directory failed, %w", err) + } + defer func() { rerr = errors.Join(rerr, temporary.Close()) }() + path := filepath.Join(temporary.Directory, "output") + if err := os.WriteFile(path, nil, fs.ModePerm); err != nil { + return nil, fmt.Errorf("create command output failed, %w", err) + } + if err := os.Chmod(path, fs.ModePerm); err != nil { + return nil, fmt.Errorf("set command output permissions failed, %w", err) } - out.Chmod(fs.ModePerm) - out.Close() - defer os.Remove(out.Name()) - cmd.Env = append(cmd.Env, fmt.Sprintf("OUT=%s", out.Name())) + // Preserve the configured Tape script's output-file contract and execution context. + cmd.Env = append(cmd.Env, fmt.Sprintf("OUT=%s", path)) if err := RunCmd(logger, cmd); err != nil { return nil, err } - - buf, err := os.ReadFile(out.Name()) + buf, err := os.ReadFile(path) if err != nil { - return nil, fmt.Errorf("read cmd out fail, %w", err) + return nil, fmt.Errorf("read command output failed, %w", err) } - return buf, nil }