Skip to content

Conversation

@ngvanthanggit
Copy link
Member

Description

In this PR, I fixed the problem of being unable to upload the plugin due to the restriction of the plugin name, author, and version.

Related Issue

Fixes #2162

Changes Made

  • Added '.' character to the safe list

Checklist

Please ensure the following before submitting your PR:

  • I have reviewed the project's contribution guidelines.
  • I have written unit tests for the changes (if applicable).
  • I have updated the documentation (if applicable).
  • I have tested the changes locally and ensured they work as expected.
  • My code follows the project's coding standards.

Screenshots or Logs (if applicable)

Screen.Recording.2025-11-02.at.20.52.21.mov

Additional Notes

Copilot AI review requested due to automatic review settings November 2, 2025 12:54
@kubestellar-prow kubestellar-prow bot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Nov 2, 2025
@github-actions github-actions bot added backend Issues or PRs related to the low-level API machinery for KubeStellar and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Nov 2, 2025
@ngvanthanggit
Copy link
Member Author

@MAVRICK-1 please review!

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR fixes a typo in the sanitize function name and enhances input sanitization and error logging for plugin handling.

  • Fixed spelling of santinize to sanitize throughout the codebase
  • Updated sanitize function to trim whitespace and convert to lowercase
  • Added error logging for plugin handler execution failures
  • Extended the regex pattern to allow dots in plugin names, authors, and versions

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

var (
// sanitize input to allow only alphanumeric, underscore, hyphen
safePattern = regexp.MustCompile(`^[a-zA-Z0-9_-]+$`)
safePattern = regexp.MustCompile(`^[a-zA-Z0-9_.-]+$`)
Copy link

Copilot AI Nov 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding dot (.) to the allowed characters creates a security risk. The dot has special meaning in regular expressions and file paths. In file paths, dots can be used for directory traversal (e.g., ..). The BuildPluginKey function constructs file paths (see pluginDir := filepath.Join(pluginDirLoad, pluginKey) at line 433 of api/plugins.go), making this vulnerable to path traversal attacks. Consider escaping dots or only allowing them in specific positions (e.g., for semantic versioning like 1.0.0).

Suggested change
safePattern = regexp.MustCompile(`^[a-zA-Z0-9_.-]+$`)
safePattern = regexp.MustCompile(`^[a-zA-Z0-9_-]+$`)

Copilot uses AI. Check for mistakes.
@MAVRICK-1
Copy link
Member

/approve /lgtm

@kubestellar-prow kubestellar-prow bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 2, 2025
@ngvanthanggit
Copy link
Member Author

ngvanthanggit commented Nov 2, 2025

@MAVRICK-1 @gaurab-khanal, Currently, it's pluginName~authorName~version. I suggested this and implemented this in our code. However, it's up to you how you want to make the naming convention. I did consider and tried to find the best way to deliver it.

If you think it's not a good implementation, um, please give some suggestions!

The PR I made this convention: PR #2034

@vedansh-5
Copy link

/ok-to-test

@kubestellar-prow kubestellar-prow bot added the ok-to-test Indicates a non-member PR verified by an org member that is safe to test. label Nov 3, 2025
@ghanshyam2005singh
Copy link

/approve

@github-project-automation github-project-automation bot moved this from In Progress to Ready to Merge in KubeStellar UI Project Nov 5, 2025
@kubestellar-prow kubestellar-prow bot added the lgtm Indicates that a PR is ready to be merged. label Nov 5, 2025
@kubestellar-prow
Copy link

LGTM label has been added.

DetailsGit tree hash: 7eee7b51b6bbc0e2d80a5217457cd5c5ee72d327

@btwshivam
Copy link
Contributor

/approve

@kubestellar-prow
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: btwshivam, ghanshyam2005singh, MAVRICK-1

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

The pull request process is described here

Details Needs approval from an approver in each of these files:
  • OWNERS [MAVRICK-1,btwshivam]

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. backend Issues or PRs related to the low-level API machinery for KubeStellar lgtm Indicates that a PR is ready to be merged. ok-to-test Indicates a non-member PR verified by an org member that is safe to test.

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

[Bug]: Unable to upload UI Plugin

5 participants