From 08b5759a1b07820e4fc2f0d956a756d871d13db4 Mon Sep 17 00:00:00 2001 From: Akshay2191 Date: Fri, 25 Sep 2026 11:30:53 +0100 Subject: [PATCH 1/2] adding a filetype check --- internal/file/file_operator.go | 9 +++++ internal/file/file_operator_test.go | 17 +++++++++ internal/file/file_service_operator.go | 6 +++ internal/file/file_service_operator_test.go | 38 +++++++++++++++++++ internal/watcher/file/file_watcher_service.go | 11 ++++++ .../watcher/file/file_watcher_service_test.go | 20 ++++++++++ 6 files changed, 101 insertions(+) diff --git a/internal/file/file_operator.go b/internal/file/file_operator.go index b28b0807b9..c886b9494a 100644 --- a/internal/file/file_operator.go +++ b/internal/file/file_operator.go @@ -211,6 +211,15 @@ func (fo *FileOperator) WriteManifestFile( } func (fo *FileOperator) MoveFile(ctx context.Context, sourcePath, destPath string) error { + info, statErr := os.Stat(sourcePath) + if statErr != nil { + return fmt.Errorf("failed to stat source file %s: %w", sourcePath, statErr) + } + if !info.Mode().IsRegular() { + return fmt.Errorf("cannot backup non-regular file %s (type: %s) — resolve file manually on the data plane", + sourcePath, info.Mode().Type()) + } + inputFile, openErr := os.Open(sourcePath) if openErr != nil { return fmt.Errorf("failed to open source file %s: %w", sourcePath, openErr) diff --git a/internal/file/file_operator_test.go b/internal/file/file_operator_test.go index a16c9140f9..d91a2fce6c 100644 --- a/internal/file/file_operator_test.go +++ b/internal/file/file_operator_test.go @@ -12,6 +12,7 @@ import ( "path" "path/filepath" "sync" + "syscall" "testing" "github.com/nginx/agent/v3/pkg/files" @@ -179,3 +180,19 @@ func TestFileOperator_MoveFile_destFileDoesNotExist(t *testing.T) { assert.FileExists(t, tempFile) assert.NoFileExists(t, newFile) } + +func TestFileOperator_MoveFile_sourceIsNonRegularFile(t *testing.T) { + t.Parallel() + + tempDir := t.TempDir() + fifoPath := filepath.Join(tempDir, "hang.conf") + + err := syscall.Mkfifo(fifoPath, 0o644) + require.NoError(t, err) + + fileOperator := NewFileOperator(&sync.RWMutex{}) + err = fileOperator.MoveFile(t.Context(), fifoPath, filepath.Join(tempDir, "hang.conf.bak")) + + require.Error(t, err) + assert.Contains(t, err.Error(), "non-regular file") +} diff --git a/internal/file/file_service_operator.go b/internal/file/file_service_operator.go index 1ae0ef9886..9117d926e1 100644 --- a/internal/file/file_service_operator.go +++ b/internal/file/file_service_operator.go @@ -312,6 +312,12 @@ func (fso *FileServiceOperator) RenameFile( ) error { slog.InfoContext(ctx, fmt.Sprintf("Renaming file %s to %s", source, desination)) + if info, err := os.Stat(desination); err == nil && !info.Mode().IsRegular() { + return fmt.Errorf("cannot write to non-regular destination file %s (type: %s) - "+ + "resolve file manually on the data plane", + desination, info.Mode().Type()) + } + // Create parent directories for the target file if they don't exist if err := os.MkdirAll(filepath.Dir(desination), dirPerm); err != nil { return fmt.Errorf("failed to create directories for %s: %w", desination, err) diff --git a/internal/file/file_service_operator_test.go b/internal/file/file_service_operator_test.go index e86cbb9ea4..5e2601b4a8 100644 --- a/internal/file/file_service_operator_test.go +++ b/internal/file/file_service_operator_test.go @@ -11,6 +11,7 @@ import ( "path/filepath" "sync" "sync/atomic" + "syscall" "testing" "time" @@ -195,3 +196,40 @@ func TestFileManagerService_UpdateFile_LargeFile(t *testing.T) { helpers.RemoveFileWithErrorCheck(t, testFile.Name()) } + +func TestFileServiceOperator_RenameFile_success(t *testing.T) { + t.Parallel() + + tempDir := t.TempDir() + src := filepath.Join(tempDir, "source.conf") + dest := filepath.Join(tempDir, "dest.conf") + + err := os.WriteFile(src, []byte("nginx config"), 0o600) + require.NoError(t, err) + + fso := NewFileServiceOperator(types.AgentConfig(), nil, &sync.RWMutex{}) + err = fso.RenameFile(t.Context(), src, dest) + + require.NoError(t, err) + assert.FileExists(t, dest) +} + +func TestFileServiceOperator_RenameFile_destIsNonRegularFile(t *testing.T) { + t.Parallel() + + tempDir := t.TempDir() + src := filepath.Join(tempDir, "source.conf") + dest := filepath.Join(tempDir, "dest.conf") + + err := os.WriteFile(src, []byte("nginx config"), 0o600) + require.NoError(t, err) + + err = syscall.Mkfifo(dest, 0o644) + require.NoError(t, err) + + fso := NewFileServiceOperator(types.AgentConfig(), nil, &sync.RWMutex{}) + err = fso.RenameFile(t.Context(), src, dest) + + require.Error(t, err) + assert.Contains(t, err.Error(), "non-regular") +} diff --git a/internal/watcher/file/file_watcher_service.go b/internal/watcher/file/file_watcher_service.go index 4355c3d295..c2ed718ade 100644 --- a/internal/watcher/file/file_watcher_service.go +++ b/internal/watcher/file/file_watcher_service.go @@ -205,6 +205,17 @@ func (fws *FileWatcherService) handleEvent(ctx context.Context, event fsnotify.E return } + if event.Has(fsnotify.Create) || event.Has(fsnotify.Rename) { + if info, err := os.Stat(event.Name); err == nil && !info.Mode().IsRegular() { + slog.ErrorContext(ctx, + "Managed config file changed to un-supported type — "+ + "config apply will fail until file is reverted to a supported type manually on the data plane", + "path", event.Name, + "type", info.Mode().Type(), + ) + } + } + slog.DebugContext(ctx, "Processing FSNotify event", "event", event) fws.filesChanged.Store(true) } diff --git a/internal/watcher/file/file_watcher_service_test.go b/internal/watcher/file/file_watcher_service_test.go index 5802018eca..d886d41410 100644 --- a/internal/watcher/file/file_watcher_service_test.go +++ b/internal/watcher/file/file_watcher_service_test.go @@ -11,6 +11,7 @@ import ( "os" "path" "path/filepath" + "syscall" "testing" "time" @@ -320,3 +321,22 @@ func TestFileWatcherService_checkForUpdates(t *testing.T) { t.Fatalf("Expected file update event") } } + +func TestFileWatcherService_handleEvent_nonRegularFile(t *testing.T) { + t.Parallel() + + tempDir := t.TempDir() + fifoPath := filepath.Join(tempDir, "hang.conf") + + err := syscall.Mkfifo(fifoPath, 0o644) + require.NoError(t, err) + + fws := NewFileWatcherService(types.AgentConfig()) + + fws.handleEvent(t.Context(), fsnotify.Event{ + Name: fifoPath, + Op: fsnotify.Create, + }) + + assert.True(t, fws.filesChanged.Load()) +} From 3cb2cb56c721a8d5f0cff2a8a62b9760d0345d96 Mon Sep 17 00:00:00 2001 From: Akshay2191 Date: Wed, 30 Sep 2026 15:13:25 +0100 Subject: [PATCH 2/2] fixing a typo --- internal/file/file_service_operator.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/internal/file/file_service_operator.go b/internal/file/file_service_operator.go index 9117d926e1..7dd8103fca 100644 --- a/internal/file/file_service_operator.go +++ b/internal/file/file_service_operator.go @@ -308,22 +308,22 @@ func (fso *FileServiceOperator) UpdateFile( // renameFile, renames (moves) file from tempDir to new location to update file. func (fso *FileServiceOperator) RenameFile( - ctx context.Context, source, desination string, + ctx context.Context, source, destination string, ) error { - slog.InfoContext(ctx, fmt.Sprintf("Renaming file %s to %s", source, desination)) + slog.InfoContext(ctx, fmt.Sprintf("Renaming file %s to %s", source, destination)) - if info, err := os.Stat(desination); err == nil && !info.Mode().IsRegular() { + if info, err := os.Stat(destination); err == nil && !info.Mode().IsRegular() { return fmt.Errorf("cannot write to non-regular destination file %s (type: %s) - "+ "resolve file manually on the data plane", - desination, info.Mode().Type()) + destination, info.Mode().Type()) } // Create parent directories for the target file if they don't exist - if err := os.MkdirAll(filepath.Dir(desination), dirPerm); err != nil { - return fmt.Errorf("failed to create directories for %s: %w", desination, err) + if err := os.MkdirAll(filepath.Dir(destination), dirPerm); err != nil { + return fmt.Errorf("failed to create directories for %s: %w", destination, err) } - moveErr := os.Rename(source, desination) + moveErr := os.Rename(source, destination) if moveErr != nil { return fmt.Errorf("failed to rename file: %w", moveErr) }