Reject directories in read_file and apply the requested mode on write_file overwrite

server/cmd/api/api/fs.go

@@ -75,6 +75,11 @@ func (s *ApiService) ReadFile(ctx context.Context, req oapi.ReadFileRequestObjec
 		log.Error("failed to stat file", "err", err, "path", path)
 		return oapi.ReadFile500JSONResponse{InternalErrorJSONResponse: oapi.InternalErrorJSONResponse{Message: "unable to stat file"}}, nil
 	}
+	// A directory opens fine but fails on the first read, after the 200 headers are sent.
+	if stat.IsDir() {
+		f.Close()
+		return oapi.ReadFile400JSONResponse{BadRequestErrorJSONResponse: oapi.BadRequestErrorJSONResponse{Message: "path is a directory"}}, nil
+	}
 
 	return oapi.ReadFile200ApplicationoctetStreamResponse{
 		Body:          f,
@@ -118,6 +123,15 @@ func (s *ApiService) WriteFile(ctx context.Context, req oapi.WriteFileRequestObj
 	}
 	defer f.Close()
 
+	// OpenFile applies perm only when it creates the file, and through the umask,
+	// so set an explicitly requested mode on the file itself.
+	if req.Params.Mode != nil {
+		if err := f.Chmod(perm); err != nil {
+			log.Error("failed to set file mode", "err", err, "path", path)
+			return oapi.WriteFile500JSONResponse{InternalErrorJSONResponse: oapi.InternalErrorJSONResponse{Message: "unable to set file mode"}}, nil
+		}
+	}
+
 	if _, err := io.Copy(f, req.Body); err != nil {
 		log.Error("failed to write file", "err", err, "path", path)
 		return oapi.WriteFile500JSONResponse{InternalErrorJSONResponse: oapi.InternalErrorJSONResponse{Message: "failed to write data"}}, nil

server/cmd/api/api/fs_test.go

@@ -74,6 +74,77 @@ func TestWriteReadFile(t *testing.T) {
 	}
 }
 
+// TestReadFileDirectory verifies that reading a directory is rejected up front
+// instead of returning 200 with a body that fails on the first read.
+func TestReadFileDirectory(t *testing.T) {
+	t.Parallel()
+
+	svc := &ApiService{defaultRecorderID: "default"}
+	resp, err := svc.ReadFile(context.Background(), oapi.ReadFileRequestObject{Params: oapi.ReadFileParams{Path: t.TempDir()}})
+	if err != nil {
+		t.Fatalf("ReadFile returned error: %v", err)
+	}
+	if _, ok := resp.(oapi.ReadFile400JSONResponse); !ok {
+		t.Fatalf("expected 400 response for a directory, got %T", resp)
+	}
+}
+
+// TestWriteFileModeOnOverwrite verifies that an explicit mode is applied when
+// overwriting an existing file and is not narrowed by the umask.
+func TestWriteFileModeOnOverwrite(t *testing.T) {
+	t.Parallel()
+
+	ctx := context.Background()
+	svc := &ApiService{defaultRecorderID: "default"}
+	filePath := filepath.Join(t.TempDir(), "script.sh")
+
+	write := func(mode *string) {
+		t.Helper()
+		resp, err := svc.WriteFile(ctx, oapi.WriteFileRequestObject{
+			Params: oapi.WriteFileParams{Path: filePath, Mode: mode},
+			Body:   strings.NewReader("#!/bin/sh\n"),
+		})
+		if err != nil {
+			t.Fatalf("WriteFile returned error: %v", err)
+		}
+		if _, ok := resp.(oapi.WriteFile201Response); !ok {
+			t.Fatalf("unexpected response type from WriteFile: %T", resp)
+		}
+	}
+	modeOf := func() os.FileMode {
+		t.Helper()
+		info, err := os.Stat(filePath)
+		if err != nil {
+			t.Fatalf("stat: %v", err)
+		}
+		return info.Mode().Perm()
+	}
+
+	write(nil)
+	if got := modeOf(); got != 0o644 {
+		t.Fatalf("default mode: got %o want 644", got)
+	}
+
+	mode := "755"
+	write(&mode)
+	if got := modeOf(); got != 0o755 {
+		t.Fatalf("mode after overwrite with 755: got %o want 755", got)
+	}
+
+	// The umask (typically 022) would turn 0777 into 0755 on create.
+	mode = "777"
+	write(&mode)
+	if got := modeOf(); got != 0o777 {
+		t.Fatalf("mode after overwrite with 777: got %o want 777", got)
+	}
+
+	// Without a mode, an overwrite keeps the file's current mode.
+	write(nil)
+	if got := modeOf(); got != 0o777 {
+		t.Fatalf("mode after overwrite without mode: got %o want 777", got)
+	}
+}
+
 // TestWriteFileAndWatch verifies WriteFile operation and filesystem watch event generation.
 func TestWriteFileAndWatch(t *testing.T) {
 	t.Parallel()