Add plugin functionality and dummy plugin for testing by peopleig · Pull Request #431 · sdslabs/beast

peopleig · 2025-10-14T06:21:55Z

Added Dummy Plugin for testing.
Getting error in email-verification plugin, need to ask @v1bh475u about the errors
Want to add plugin loading/choosing functionality in config.toml
Attempted to add a method to somehow use ctfD's existing plugins (written in python) in Beast, but couldn't figure out.

peopleig · 2025-10-19T08:53:29Z

Added email verify plugin. Defines allowed domains in config.toml.

Copilot

Pull request overview

This PR introduces a plugin system to Beast, enabling modular functionality extensions. The implementation includes a plugin interface, registration mechanism, and two example plugins for testing and email domain verification.

Key Changes:

Added plugin infrastructure with a registration and initialization system
Implemented DummyPlugin as a testing example
Implemented EmailVerifyPlugin to restrict user registration to specific email domains

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 13 comments.

Show a summary per file

File	Description
plugins/base.go	Defines the Plugin interface and provides registration/initialization functions for the plugin system
plugins/dummy/plugin.go	Simple test plugin that adds a dummy endpoint to verify the plugin system works
plugins/email_verify/plugin.go	Plugin that intercepts registration requests to enforce email domain restrictions
plugins/email_verify/test.sh	Bash script to test the email verification plugin functionality
core/config/config.go	Adds configuration structures for plugin management and email verification settings
api/router.go	Integrates plugin initialization into the router setup
api/main.go	Imports the plugins to trigger their init() functions for registration
_examples/example.config.toml	Documents plugin configuration options with examples

Critical Issues Found:

Plugin initialization timing issue: Plugins are initialized before routes are registered, causing middleware to intercept requests to non-existent routes
Significant code duplication: The EmailVerifyPlugin duplicates the entire registration handler logic (~86 lines) and response types from existing code, creating a maintainability burden
API design concern: The EmailVerifyPlugin completely replaces the original registration handler rather than extending it, making the architecture inflexible

Additional Issues:

Email validation logic is too simplistic and doesn't properly validate email format
Missing leading slash in DummyPlugin route path
Test script is not idempotent (fails on repeated runs)
Several minor spelling errors and formatting inconsistencies

Comments suppressed due to low confidence (1)

api/router.go:40

Plugin initialization occurs before route registration, which causes the EmailVerifyPlugin middleware to intercept requests to /auth/register even though the route hasn't been registered yet. This means the plugin's middleware will always execute instead of allowing the route to be registered normally.

Move the plugin initialization (line 35) to after all route registrations are complete, just before the NoRoute handler (before line 141). This ensures that plugin middleware can properly interact with registered routes.

	// Initialize all plugins
	plugins.InitPlugins(router)

	// Authorization routes group
	authGroup := router.Group("/auth")
	{
		authGroup.POST("/register", register)

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

Copilot · 2025-12-22T17:21:01Z

plugins/email_verify/plugin.go

+func (p *EmailVerifyPlugin) verifyEmailRegister(c *gin.Context, checkFlag bool) {
+	name := c.PostForm("name")
+	username := c.PostForm("username")
+	password := c.PostForm("password")
+	email := c.PostForm("email")
+	sshKey := c.PostForm("ssh-key")
+
+	name = strings.TrimSpace(name)
+	username = strings.TrimSpace(strings.ToLower(username))
+	password = strings.TrimSpace(password)
+	email = strings.TrimSpace(strings.ToLower(email))
+	sshKey = strings.TrimSpace(sshKey)
+
+	if username == "" || password == "" || email == "" {
+
+		c.JSON(http.StatusBadRequest, HTTPPlainResp{
+			Message: "Username, password and email can not be empty",
+		})
+		return
+	}
+
+	if len(username) > 12 {
+		c.JSON(http.StatusBadRequest, HTTPErrorResp{
+			Error: "Username cannot be greater than 12 characters",
+		})
+		return
+	}
+	if checkFlag {
+		if !p.isAllowedEmail(email) {
+			c.JSON(http.StatusForbidden, gin.H{
+				"error": "Registration restricted to organization emails only",
+			})
+			return
+		}
+	}
+
+	userEntry := database.User{
+		Name:      name,
+		AuthModel: auth.CreateModel(username, password, core.USER_ROLES["contestant"]),
+		Email:     email,
+		SshKey:    sshKey,
+	}
+
+	if !config.SkipAuthorization {
+		smtpHost := config.Cfg.MailConfig.SMTPHost
+		smtpPort := config.Cfg.MailConfig.SMTPPort
+
+		if smtpHost == "" || smtpPort == "" {
+			log.Errorf("WARNING: SMTP not configured")
+		} else {
+			otpEntry, err := database.QueryOTPEntry(email)
+			if err != nil {
+				if errors.Is(err, gorm.ErrRecordNotFound) {
+					c.JSON(http.StatusUnauthorized, HTTPErrorResp{
+						Error: "OTP not found, email not verified",
+					})
+					return
+				} else {
+					log.Println("Failed to query OTP:", err)
+					c.JSON(http.StatusInternalServerError, HTTPErrorResp{
+						Error: "Failed to send OTP",
+					})
+					return
+				}
+			}
+			if !otpEntry.Verified {
+				c.JSON(http.StatusNotAcceptable, HTTPErrorResp{
+					Error: "Email not verified, cannot register user",
+				})
+				return
+			}
+		}
+	}
+
+	err := database.CreateUserEntry(&userEntry)
+	if err != nil {
+		c.JSON(http.StatusNotAcceptable, HTTPErrorResp{
+			Error: err.Error(),
+		})
+		return
+	}
+
+	c.JSON(http.StatusOK, HTTPPlainResp{
+		Message: "User created successfully",
+	})
+}


The entire registration handler logic (lines 60-145) is duplicated from api/auth.go. This creates a significant maintainability problem because any bug fixes or updates to the registration logic must be made in two places. Additionally, the response types (HTTPPlainResp, HTTPPlainMapResp, HTTPErrorResp) are redefined on lines 19-29, duplicating the types already defined in api/response.go.

Consider refactoring the original register function in api/auth.go to accept an optional email validation function as a parameter, then call that function from the plugin. This would allow the plugin to inject its domain validation logic without duplicating the entire registration flow.

Copilot · 2025-12-22T17:21:01Z

plugins/dummy/plugin.go

+}
+
+func (p *DummyPlugin) Init(router *gin.Engine) error {
+	router.GET("api/plugins/dummy", func(context *gin.Context) {


The route path is missing a leading slash. It should be "/api/plugins/dummy" instead of "api/plugins/dummy". Without the leading slash, this route may not work as intended or could conflict with route groups.

Suggested change

router.GET("api/plugins/dummy", func(context *gin.Context) {

router.GET("/api/plugins/dummy", func(context *gin.Context) {

Copilot · 2025-12-22T17:21:02Z

plugins/email_verify/plugin.go

+}
+
+type HTTPErrorResp struct {
+	Error string `json:"error" example:"Error occured while veifying the challenge."`


Typo in the example string: "veifying" should be "verifying".

Suggested change

Error string `json:"error" example:"Error occured while veifying the challenge."`

Error string `json:"error" example:"Error occured while verifying the challenge."`

Copilot · 2025-12-22T17:21:02Z

plugins/base.go

+//All plugins will be initialized with the gin engine, at startup
+//Logging twice, once when we register and once when we initialize


The comment lacks proper spacing and formatting. Comments should start with a space after the "//" and use proper punctuation. This comment should be formatted as:

// All plugins will be initialized with the gin engine at startup.
// Logging twice: once when we register and once when we initialize.

Suggested change

//All plugins will be initialized with the gin engine, at startup

//Logging twice, once when we register and once when we initialize

// All plugins will be initialized with the gin engine at startup.

// Logging twice: once when we register and once when we initialize.

Copilot · 2025-12-22T17:21:02Z

plugins/email_verify/plugin.go

+	router.Use(func(c *gin.Context) {
+		if c.Request.URL.Path == "/auth/register" && c.Request.Method == "POST" {
+			p.verifyEmailRegister(c, checkFlag)
+			c.Abort()
+		} else {
+			c.Next()
+		}
+	})


The middleware approach unconditionally intercepts and aborts all POST requests to /auth/register when the plugin is enabled, completely replacing the original registration handler. This is problematic because:

It bypasses the normal route registration system

Makes the original /auth/register route (line 40 in api/router.go) unreachable when the plugin is enabled

Creates an all-or-nothing scenario where the original handler cannot execute

A better approach would be to add the email domain validation as a check within the middleware but then call c.Next() to allow the original handler to process the request, or use a pre-handler hook pattern. Alternatively, refactor the registration logic to be composable so plugins can inject validation steps without duplicating the entire handler.

Copilot · 2025-12-22T17:21:03Z

plugins/email_verify/plugin.go

+)
+
+type HTTPPlainResp struct {
+	Message string `json:"message" example:"Messsage in response to your request"`


Typo in the example string: "Messsage" should be "Message" (contains an extra 's').

Suggested change

Message string `json:"message" example:"Messsage in response to your request"`

Message string `json:"message" example:"Message in response to your request"`

Copilot · 2025-12-22T17:21:04Z

plugins/email_verify/plugin.go

+		smtpPort := config.Cfg.MailConfig.SMTPPort
+
+		if smtpHost == "" || smtpPort == "" {
+			log.Errorf("WARNING: SMTP not configured")


Using log.Errorf with "WARNING:" prefix is inconsistent. Since this is a warning, use log.Warnf instead to maintain proper log level semantics and consistency with the existing codebase (line 246 in api/auth.go uses log.Printf for the same warning).

Suggested change

log.Errorf("WARNING: SMTP not configured")

log.Warnf("SMTP not configured")

Copilot · 2025-12-22T17:21:04Z

plugins/base.go

+			log.Warnf("%s is not enabled, skipping initialization", p.Name())
+			continue
+		}
+		log.Infof("Intializing plugin: %s", p.Name())


Typo in comment: "Intializing" should be "Initializing".

Suggested change

log.Infof("Intializing plugin: %s", p.Name())

log.Infof("Initializing plugin: %s", p.Name())

Copilot · 2025-12-22T17:21:04Z

_examples/example.config.toml

+# For reference, Beast currently supports: "DummyPlugin", "EmailVerifyPlugin"
+enabled_plugins = ["DummyPlugin", "EmailVerifyPlugin"]
+
+#Allowed email ids for the Email Verify plugin


Missing space in comment. Should be "# Allowed email ids for the Email Verify plugin" (space after "#").

Suggested change

#Allowed email ids for the Email Verify plugin

# Allowed email ids for the Email Verify plugin

Copilot · 2025-12-22T17:21:05Z

plugins/email_verify/test.sh

+  echo "✅ Test Passed-Allowed Domain"
+else
+  echo "❌ Test Failed-Allowed Domain (Got HTTP $response_allowed)"


Inconsistent spacing in test output message. Line 13 has "Test Passed-Allowed" (no space before hyphen) while line 27 has "Test Passed - Disallowed" (spaces around hyphen). For consistency, both should use the same format.

Suggested change

echo "✅ Test Passed-Allowed Domain"

else

echo "❌ Test Failed-Allowed Domain (Got HTTP $response_allowed)"

echo "✅ Test Passed - Allowed Domain"

else

echo "❌ Test Failed - Allowed Domain (Got HTTP $response_allowed)"

peopleig added 8 commits October 14, 2025 11:47

Add plugin functionality and dummy plugin for testing

4b94b96

Add allowed mail domains to config.toml

343fc14

Add email_verify plugin

4ae61cd

Add check flag to email_verify plugin

fd2e11c

Update Email Verify Plugin Logic

4ce2585

Update order of initializing routes and plugins

aa39819

Update logging of email verify plugin

8cf4896

Update logging of email verify plugin

bee98b7

peopleig added 3 commits October 19, 2025 20:51

Add test for verifying plugin functionality

10b02bc

Update example.config.toml to allow plugin initialization choice

e70ed01

Add choice functionality for plugins

eaacea0

v1bh475u requested a review from Copilot December 22, 2025 17:16

Copilot started reviewing on behalf of v1bh475u December 22, 2025 17:16 View session

Copilot AI reviewed Dec 22, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add plugin functionality and dummy plugin for testing#431

Add plugin functionality and dummy plugin for testing#431
peopleig wants to merge 11 commits intobl4ze/devfrom
neptune/plugins

peopleig commented Oct 14, 2025

Uh oh!

peopleig commented Oct 19, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Dec 22, 2025

Uh oh!

Copilot AI Dec 22, 2025

Uh oh!

Copilot AI Dec 22, 2025

Uh oh!

Copilot AI Dec 22, 2025

Uh oh!

Copilot AI Dec 22, 2025

Uh oh!

Copilot AI Dec 22, 2025

Uh oh!

Copilot AI Dec 22, 2025

Uh oh!

Copilot AI Dec 22, 2025

Uh oh!

Copilot AI Dec 22, 2025

Uh oh!

Copilot AI Dec 22, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

	router.GET("api/plugins/dummy", func(context *gin.Context) {
	router.GET("/api/plugins/dummy", func(context *gin.Context) {

	Error string `json:"error" example:"Error occured while veifying the challenge."`
	Error string `json:"error" example:"Error occured while verifying the challenge."`

		//All plugins will be initialized with the gin engine, at startup
		//Logging twice, once when we register and once when we initialize

	Message string `json:"message" example:"Messsage in response to your request"`
	Message string `json:"message" example:"Message in response to your request"`

	log.Errorf("WARNING: SMTP not configured")
	log.Warnf("SMTP not configured")

	log.Infof("Intializing plugin: %s", p.Name())
	log.Infof("Initializing plugin: %s", p.Name())

	#Allowed email ids for the Email Verify plugin
	# Allowed email ids for the Email Verify plugin

Conversation

peopleig commented Oct 14, 2025

Uh oh!

peopleig commented Oct 19, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Dec 22, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 22, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 22, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 22, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 22, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 22, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 22, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 22, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 22, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 22, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant