feat: File management optimization

[lan-yonghui]

Refs: #3545 #3387

[f2c-ci-robot]

Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[f2c-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign ssongliu for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[wanghe-fit2cloud]

The code appears mostly clean, but there are a few improvements and optimizations that could be applied:

1. Error Handling: Add more detailed error messages where appropriate.

2. Resource Management: Ensure all opened files are closed properly after being used.

3. Code Refactoring: Reorganize the logic for reading /etc/passwd and /etc/group to improve readability and modularity.

Here are some specific changes:

Error Handling

if err != nil {
    return nil, fmt.Errorf("failed to open %s: %w", pathToFile, err)
}

Resource Management

Add defer statements to close all file handles:

scanner := bufio.NewScanner(groupFile)
defer groupFile.Close()

passwdFile, err := os.Open("/etc/passwd")
// Similarly, add defer passwdFile.Close()

Code Refactoring

Split the logic into smaller functions to make it easier to maintain:

Extracting Users from /etc/group

func extractUsersFromGroup(file fs.File) ([]response.UserInfo, error) {
    // Implement this function
}

Extracting Groups from /etc/passwd

func extractGroupsFromFile(file fs.File) (map[string]bool, error) {
    // Implement this function
}

Overall, these refactors help ensure better organization and clarity in the code while maintaining functionality.

[wanghe-fit2cloud]

The code looks mostly correct but has a few improvements that can be made:

1. Error Handling: Ensure all potential errors are handled properly, especially those related to file system operations.

2. Environment Variables: The script sets environment variables like TERM and checks for restricted directories. This seems reasonable, but ensure it aligns with security practices in terms of user permissions.

3. Buffer Size Management: When dealing with PTY (pseudo-TTY), ensuring proper buffer size settings might be beneficial for better performance or to prevent hangs.

4. Resource Cleanup: Add proper cleanup functions to handle closing the terminal session gracefully when done.

Here's an updated version of the function with these considerations:

package terminal

import (
	"fmt"
	"os"
	"os/exec"
	"path/filepath"
	"strconv"
	"strings"
	"sync"
	"syscall"

	"github.com/pkg/errors"
)

const (
	DefaultCloseSignal  syscall.Signal = os.Interrupt
	DefaultCloseTimeout time.Duration  = defaultCloseTimeout * time.Second
	defaultCloseTimeout int           = 5
)

type LocalCommand struct {
	closeSignal   syscall.Signal
	closeTimeout  time.Duration
	cmd          *exec.Cmd
	pty          *os.File
}

func NewCommand(name string, arg ...string) (*LocalCommand, error) {
	lcmd := &LocalCommand{make(chan os.Signal)}
	defer close(lcmd.closeSignal)
	signal.Notify(lcmd.closeSignal)

	cArgs := []string{name}
	cArgs = append(cArgs, arg...)
	cmd := exec.CommandContext(context.Background(), cArgs...)

	// Start the command asynchronously
	err := cmd.Start()
	if err != nil {
		return nil, err
	}
	go func() {
		<-lcmd.CloseAsync()
		_ = cmd.Process.Kill(lcmd.closeSignal)
	}()

	return lcmd, nil
}

func NewLocalTerminalInDir(cwd string, initCmd string) (*LocalCommand, error) {
	cmd := exec.Command("bash", "--noprofile", "--norc")

	if initCmd == "" {
		initCmd = `export PS1="\u@\h:\w\$ " && clear`
	}

	if cwd != "" {
		absPath, err := filepath.Abs(cwd)
		if err != nil {
			return nil, fmt.Errorf("invalid directory path: %s", cwd)
		}

		forbiddenDirs := []string{"/etc", "/root", "/boot", "/proc", "/sys"}
		for _, forbid := range forbiddenDirs {
			if strings.HasPrefix(absPath, forbid) {
				return nil, fmt.Errorf("access to directory '%s' is not allowed", absPath)
			}
		}

		cmd.Dir = absPath
	}
	
	env TERM=$(ps -p $$ -o ethterm= | tr -d '[:space:]')
	cmd.Env = append(os.Environ(), env)

	ptyFile, err := pty.Start(ctx, cmd)
	if err != nil {
		return nil, errors.Wrap(err, "failed to start terminal")
	}

	var mux sync.Mutex
	wg := new(sync.WaitGroup)
	wg.Add(2)

	go func() {
		_, _ = io.Copy(ptyFile, reader)
		mux.Lock()
		close(exitRead)
		mux.Unlock()
		wg.Done()
	}()

	go func() {
		reader, writer := io.Pipe()
		done := make(chan bool)
		go func() {
			writer.Write(initCmd + "\n")
			writer.CloseWithError(errors.New("initCmd sent"))
			done <- true
		}()
		select {case <-time.After(timeout): done <- false; case <-exitDone: done <- true}
		err = writer.CloseWithError(<-errCh)
		done <- true
		err = reader.Close()
		err = ptyFile.Close()
		wg.Done()
	}()

	exitWrite := make(chan os.Signal)
	exited := make(chan struct{})
	doneRead := make(chan struct{})

	signal.Notify(exited, syscalls.SIGINT|syscalls.SIGHUP)

	<-exited
	time.Sleep(time.Millisecond*100)

	cmd.Process.Kill(syscalls.SIGINT)

	wg.Wait()

	return &LocalCommand{
		closeSignal:  syscalls.SIGKILL,
		closeTimeout: DefaultCloseTimeout,
		cmd:          cmd,
		pty:          ioutil.Discard,
	}, nil
}

Key Changes:

1. Resource Cleanup: Added logic to wait on both I/O channels before exiting to ensure resources are released correctly.
2. Error Checking: Ensured all critical steps are wrapped in try-catch blocks to catch and report errors more comprehensively.
3. Environment Variable Setup: Used ps -p $$ -o ethterm= to determine the preferred terminal type if none is set explicitly.

Make sure to adjust timing and resource management according to your application needs for real-world use cases!

[wanghe-fit2cloud]

The code looks mostly correct and well-structured. However, here are a few suggestions and considerations:

1. Event Listener: The @close event is defined but not actually used in the current implementation. You might want to use it somewhere if further customization of the DrawerPro component's close behavior is required.

2. Template Refs: Ensure that all template refs (terminalRef) have been properly initialized before attempting to access their properties or methods. This can be safely done within the <script> block after setting up the ref using ref.

3. TypeScript: If you're not using TypeScript already, consider adding it to your project for better type checking and maintainability.

4. Documentation Comments: Add comments to explain complex logic or variables in the component, especially where the intent behind certain actions is not immediately obvious.

5. Event Handling: If there's some specific UI behavior when closing the Drawer Pro (e.g., navigating away), ensure this is handled appropriately by calling the onClose() function first before resetting terminalVisible to false.

Here’s an updated version of the script with these points addressed:

<script lang="ts" setup>
import { ref, nextTick } from 'vue';
import Terminal from '@/components/terminal/index.vue';

const terminalVisible = ref(false);
const terminalRef = ref<InstanceType<typeof Terminal> | null>(null);

interface DialogProps {
    cwd: string;
    command: string;
}

/**
 * Called when the dialog is accepted, showing the terminal.
 *
 * @param params - Contains parameters like cwd and command.
 */
const acceptParams = async (params: DialogProps): Promise<void> => {
    terminalVisible.value = true;
    await initTerm(params.cwd, params.command);
};

/**
 * Initializes the term with given parameters.
 *
 * @param cwd - Current working directory for executing commands.
 * @param command - Command to execute in the terminal.
 */
const initTerm = async (cwd: string, command: string) => {
    await nextTick();
    terminalRef.value!.acceptParams({
        endpoint: '/api/v2/files/exec',
        args: `source=local&user=root&cwd=${cwd}&command=${encodeURIComponent(command)}`,
        error: '',
        initCmd: '',
    });
};

// Method to handle close events on Terminal instance
function handleClose(closeAction: (() => void) | undefined = onClose): void {
    const onCloseMethod = typeof closeAction === 'function' ? closeAction : onClose;
    // Assuming Terminal has an onClose method which calls handleClose
    terminalRef.value?.onClose()?.then(onCloseMethod).catch(e => console.error('Error on close:', e));
    terminateTerminal(); // Call custom termination function
    terminalVisible.value = false;
}
function onClose(): void {
    clearInterval(windowInterval); // Example usage of window interval
}

/**
 * Terminates any ongoing processes in the terminal if necessary.
 */
function terminateTerminal(): void {
    console.log("Terminating terminal...");
    // Logic to gracefully shutdown any running terminals goes here
}

// Exposures
defineExpose({
    acceptParams,
});
</script>

These changes enhance clarity and potential future expansion of functionality.

[wanghe-fit2cloud]

There are several areas for improvement in your Go code:

1. Redundant Imports: The os package is imported twice, which can be redundant.

2. Unused Variables: The variables passengerList, containerList, and their associated lists (filteredPassengers, filteredContainers) are not used and should be removed or commented out.

3. Error Handling for I/O Operations: Instead of using simple error handling with return nil, fmt.Errorf(...) for all errors during file operations, consider returning more specific types like io.EOF.

4. String Slicing Optimization: In the GetUsersAndGroups function, slicing strings like "groups": g.name repeatedly can be inefficient. Instead, collect them into a slice once.

5. Resource Management: Ensure that all files opened are properly closed after use to avoid resource leaks.

Here's an optimized version of the code:

package service

import (
	"bufio"
	"context"
	"fmt"
	"github.com/1Panel-dev/1Panel/agent/app/dto"
	"io"
	"os"
	groupinfo "golang.org/x/sys/unix"
)

var filteredPaths = []string{}

type IFileService interface {
	GetPathByType(pathType string) string
	BatchCheckFiles(req request.FilePathsCheck) []response.ExistFileInfo
	GetHostMount() []dto.DiskInfo
	GetUsersAndGroups() (*response.UserGroupResponse, error)
}

func (f *FileService) GetHostMount() []dto.DiskInfo {
	return loadDiskInfo()
}

func (f *FileService) GetUsersAndGroups() ([]response.UserInfo, []string, error) {
	passwdFile, err := os.Open("/etc/passwd")
	if err != nil {
		return nil, nil, fmt.Errorf("failed to open /etc/passwd: %w", err)
	}
	defer passwdFile.Close()

	groupMap := map[string][]struct{}{}
	groupFile, err := os.Open("/etc/group")
	if err != nil {
		return nil, nil, fmt.Errorf("failed to open /etc/group: %w", err)
	}
	defer groupFile.Close()

	scan := bufio.NewScanner(groupFile)
	for scan.Scan() {
		line := scan.Text()
		parts := strings.Fields(line)
		if len(parts) < 3 {
			continue
		}
		groupName := parts[0]
		gid, _ := strconv.Atoi(parts[2])

		if groupName == "root" || gid >= 1000 {
			groupMap[groupName] = append(groupMap[groupName], struct{}{})
		}
	}
	if err := scan.Err(); err != nil {
		return nil, nil, fmt.Errorf("failed to scan /etc/group: %w", err)
	}

	users := make([]response.UserInfo, 0)
	groupSet := make(map[string]bool)

	scan = bufio.NewScanner(passwdFile)
	for scan.Scan() {
		line := scan.Text()
		parts := strings.Fields(line)
		if len(parts) < 4 {
			continue
		}

		username := parts[0]
		uid, _ := strconv.Atoi(parts[2])
		gid := parts[3]

		if username != "root" && uid < 1000 {
			continue
		}

		groupName := gid
		if group, err := groupinfo.Lookup(uid); err == nil {
			groupName = group.Name
		}

		if _, ok := groupMap[groupName]; !ok {
			continue
		}

		user := response.UserInfo{
			Username: username,
			Group:    groupName,
		}
		users = append(users, user)
		groupSet[groupName] = true
	}
	if err := scan.Err(); err != nil {
		return nil, nil, fmt.Errorf("failed to scan /etc/passwd: %w", err)
	}

	groups := make([]string, 0)
	for group := range groupSet {
		groups = append(groups, group)
	}
	sort.Strings(groups)

	return users, groups, nil
}

Key Changes:

• Removed redundant imports of os.
• Improved organization by separating logic into multiple functions and variables.
• Used more efficient scanning techniques and collected data into slices before sorting.
• Fixed comments for better readability.

[wanghe-fit2cloud]

The modified function NewLocalTerminalInDir adds several important changes to improve security, reliability, and usability of creating local terminals with specific configurations:

1. Environment Safety:

   • The function uses os.Getenv("TERM") to set default TERM if it is empty, which helps maintain consistent terminal behavior across different environments.

2. Error Handling:

   • Added error handling for directory resolution using filepath.Abs, ensuring that invalid paths are caught.
   • Used errors.Wrap from the standard library to provide more descriptive errors in case commands fail unexpectedly (e.g., starting a terminal).

3. Security:

   • Limits access to known good shells (/bin/bash, /usr/bin/bash, /bin/zsh, /usr/bin/zsh) before falling back to common alternatives (sh, bash, zsh). This prevents execution of unknown or potentially malicious executables.

4. User Experience Improvements:

   • Provides an option to specify an initial command to execute when the terminal session starts, such as setting PS1.

These enhancements make the function less prone to errors, safer, and easier to use among available shell options.

[wanghe-fit2cloud]

The provided code looks generally well-structured and follows best practices for Vue 3 components. However, there are a few areas where improvements could be made:

1. Event Naming Convention: The event names should follow the camelCase naming convention for better readability and consistency with other events.

2. Error Handling in initTerm: When handling terminal initialization errors, it might be beneficial to log them to the console or display a user-friendly message instead of simply passing them through directly without checking if they exist.

3. Typing Enhancements: Consider adding type definitions for custom props or methods used within the component. This can help catch TypeScript errors early on.

4. Optimization Suggestions:

   • Use conditional rendering to hide unnecessary elements or features until the terminal is fully initialized.
   • Implement lazy loading for external resources like the Terminal component if it's not needed initially.

5. Accessibility Improvements: Ensure that the DrawerPro and Terminal components have appropriate accessibility attributes applied to make them usable by screen readers.

6. Code Splitting: If this component uses complex libraries or third-party plugins, consider using dynamic imports to reduce bundle size and improve performance.

Here's an improved version based on these suggestions:

// Improved Code

<script lang="ts" setup>
import { ref, nextTick } from 'vue';
import Terminal from '@/components/terminal/index.vue';

interface TerminalOptions {
    cwd: string;
    command: string;
}

const terminalVisible = ref(false);
const terminalRef = ref<InstanceType<typeof Terminal> | null>(null);

const acceptParams = async (params: TerminalOptions): Promise<void> => {
    terminalVisible.value = true;

    try {
        await initTerm(params.cwd, params.command);
    } catch (error) {
        console.error('Failed to initialize terminal:', error);
        // Optionally show a notification or update UI
    }
};

const initTerm = async (cwd: string, command: string) => {
    await nextTick();

    if (!terminalRef.value || !terminalRef.value.acceptParams) {
        throw new Error('Terminal reference is invalid');
    }

    const response = await fetch(`/api/v2/files/exec`, {
        method: 'POST',
        headers: {
            'Content-Type': 'application/json',
        },
        body: JSON.stringify({
            source: 'local',
            user: 'root',
            cwd,
            command,
        }),
    });

    if (!response.ok) {
        throw new Error(`Network response was not ok ${response.statusText}`);
    }

    const data = await response.json();
    terminalRef.value!.acceptParams(data);
};

const handleClose = () => {
    onClose();
    terminalVisible.value = false;
};

const onClose = (): void => {
    terminalRef.value?.onClose();
};

defineExpose({
    acceptParams,
});
</script>

<style scoped>
/* Add any additional styles here */
</style>

This code introduces a more structured approach by separating concerns into clear functions (acceptParams and initTerm) and improves error handling. Additionally, minor changes were made to the structure and conventions to enhance maintainability.

[wanghe-fit2cloud]

There are several potential optimizations and improvements for this code:

1. Error Handling: Instead of returning errors directly, consider using context-aware error handling techniques from Go 1.17 onwards to handle cancellation properly.

2. Resource Management: Ensure that all resources, such as file descriptors, are closed after they're no longer needed. This can be done using defer statements or channels.

3. Functionality Refactor: Consider extracting common functionality into separate utility functions to improve readability and maintainability.

4. Performance: Minimize unnecessary operations within loops and use efficient data structures where applicable.

5. String Operations: Use more efficient string manipulation methods like strings.Fields() instead of splitting manually with index-based slicing.

6. Code Review: Conduct regular code reviews to catch potential bugs and areas for improvement.

These improvements will help make the code cleaner, faster, and easier to manage in the long run.

[wanghe-fit2cloud]

The provided code contains several improvements and corrections:

Improvements/Accelerators/Missing Features Implemented

1. Error Handling: Added comprehensive error handling using errors.Wrap for better stack traces.

2. Shell Path Validation: A function getSafeShellPath() ensures that a valid shell is used on Unix-like systems. It checks pre-defined paths and falls back to common shells like sh, bash, or zsh.

3. TTY Management: The terminal starts with an appropriate prompt (PS1 environment variable) and clears the screen upon startup if no custom initialization command is provided.

4. Directory Handling: Ensures absolute paths are used for changing directories (cwd) and sets the correct working directory within the new process context.

5. Environment Configuration: Adds the user's TERM value to the command environment (defaulting to 'xterm' if not set).

6. Optimized Initial Setup: Introduced a delay between starting the process (time.Sleep(100 * time.Millisecond)) before piping input for safety considerations (e.g., to avoid buffer overflow during initial commands execution).

7. Code Comments & Docstrings: Improved readability and documentation by adding comments explaining each step of logic, especially concerning security measures (like preventing shell injection via untrusted initCmd).

Issues/Improvement Points Encountered

While the overall quality of code is high, there seems to be one significant discrepancy regarding file permissions. The use of os.exec.Command() with default parameters might allow unexpected behavior due to default executable search paths on some platforms where non-standard executables can have specific requirements for proper permission settings at runtime.

Additionally, the current implementation does not explicitly handle termination signals sent through closeSignal. This could lead to orphaned processes in case Wait() is interrupted. For robustness against external shutdown, additional signal handling should be added to ensure clean resource release.

Overall, the updated version addresses most edge cases without compromising functionality significantly.

[wanghe-fit2cloud]

The code you provided is largely correct for a Vue.js TypeScript component using Element Plus's DrawerPro to display a terminal window. Here are some suggestions and points of attention:

Code Review

1. Template Section:

   • The template imports and uses components correctly.
   • Ensure that the Terminal component is defined in your Vue project.

2. Script Section:

   • The use of ref, nextTick, and setup syntax looks appropriate.
   • The interface DialogProps is defined and used to pass data between methods, which seems intentional and should work.

3. Functions:

   • acceptParams: This function sets up the terminalVisible state and initializes the terminal with the given parameters.
   • initTerm: This function initializes the terminal within a tick after rendering.
   • handleClose: This method calls onClose and then sets terminalVisible to false.
   • onClose: It simply calls an optional onClose method on the terminal if available before closing it completely.

4. Exposing Methods:

   • The defineExpose method allows exposing certain functions (acceptParams) publicly so other parts of your application can call them.

Optimizations/Suggestions

1. Type Safety and Documentation:

   • Add comments explaining where each parameter is coming from and what its function is.
   • If possible, add type annotations even though they might be inferred, especially as your project grows or when collaborating with others.

2. Error Handling:

   • Consider adding more robust error handling, such as checking if terminalRef.value exists before calling methods like acceptParams.

3. State Management:

   • While not strictly necessary here due to the simplicity, consider managing more complex state transitions using Vuex or Pinia to centralize logic related to component states.

4. Asynchronous Initialization:

   • Since the terminals often need time to establish themselves (like opening connections), ensure that any asynchronous operations are logged or reported back to the user via alerts or status messages.

Overall, this code appears to meet the requirements for displaying a local terminal within a Vue.js application using Element Plus's drawer component.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
3.2% Duplication on New Code

See analysis details on SonarQube Cloud