feat: Initial code for web dashboard #11
Conversation
WalkthroughThis pull request introduces a web interface to the existing bot application, adding a Fiber-based HTTP server with API handlers for Real-Debrid operations, API key authentication middleware, configuration extensions, and a responsive frontend dashboard for managing torrents and downloads. Changes
Sequence Diagram(s)sequenceDiagram
participant Bot as Bot Process
participant DB as Database
participant Web as Fiber Server
participant FE as Frontend Browser
Bot->>DB: Initialize with DSN
Bot->>Web: Start server with Dependencies
Web->>Web: Register middleware (APIKeyAuth)
Web->>Web: Mount API routes
Web->>Web: Serve static files
FE->>FE: Page load
FE->>FE: Prompt for API key
FE->>Web: GET /api/status (X-API-Key)
Web->>Web: Verify API key via middleware
Web->>DB: Query user/torrents/downloads
Web->>FE: JSON response {success, data}
FE->>FE: Render dashboard
FE->>Web: POST /api/torrents (magnet link + API key)
Web->>Web: Validate key, parse input
Web->>DB: Insert torrent, select files
Web->>FE: {success, data} with torrent info
FE->>FE: Update torrents list
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes The diff introduces substantial new functionality across multiple cohorts: backend handlers with business logic (torrent/download operations, status formatting, pagination), authentication middleware, configuration changes, and a complete frontend implementation. While changes follow consistent patterns within each cohort, the variety of concerns—API design, auth flow, database integration, frontend state management—and the interdependencies between backend and frontend require careful review of correctness, security (API key handling), and integration points. Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
go.mod (1)
3-3: Invalid go directive: use major.minor only
godirective must not include a patch version. Replace withgo 1.25and, if you want to pin, add atoolchaindirective.-go 1.25.3 +go 1.25 +toolchain go1.25.3cmd/rdctl-bot/main.go (1)
170-172: Sanitize proxy URL in logs; avoid leaking credentials
cfg.RealDebrid.Proxymay containuser:pass@. Logging it raw can leak secrets.- if cfg.RealDebrid.Proxy != "" { - log.Printf("Using proxy: %s", cfg.RealDebrid.Proxy) - } + if cfg.RealDebrid.Proxy != "" { + if u, err := url.Parse(cfg.RealDebrid.Proxy); err == nil { + // Log scheme and host only, drop userinfo and query + log.Printf("Using proxy: %s://%s", u.Scheme, u.Host) + } else { + log.Printf("Using proxy is enabled") + } + }Add:
import "net/url"
🧹 Nitpick comments (8)
internal/config/config.go (2)
18-25: Add a Web.Enabled flag to allow disabling the dashboard without breaking validationRight now, the web layer is mandatory because APIKey is required. Consider introducing an
Enabledflag and gating validation/defaults on it.type Config struct { Telegram TelegramConfig `mapstructure:"telegram"` RealDebrid RealDebridConfig `mapstructure:"realdebrid"` App AppConfig `mapstructure:"app"` Database DatabaseConfig `mapstructure:"database"` - Web WebConfig `mapstructure:"web"` + Web WebConfig `mapstructure:"web"` } // WebConfig holds all web server configuration type WebConfig struct { + Enabled bool `mapstructure:"enabled"` ListenAddr string `mapstructure:"listen_addr"` APIKey string `mapstructure:"api_key"` }And in Validate:
- if c.Web.ListenAddr == "" { + if c.Web.Enabled && c.Web.ListenAddr == "" { c.Web.ListenAddr = ":8080" } - if c.Web.APIKey == "" { + if c.Web.Enabled && c.Web.APIKey == "" { return fmt.Errorf("web api_key is required for dashboard access") }
199-205: Strengthen API key validation and document env overrideEnforce a minimal length to avoid trivial keys, and note env override (
TGRD_WEB_API_KEY) in error text.- if c.Web.APIKey == "" { - return fmt.Errorf("web api_key is required for dashboard access") - } + if c.Web.APIKey == "" || len(c.Web.APIKey) < 16 { + return fmt.Errorf("web api_key is required (set via config or TGRD_WEB_API_KEY) and must be at least 16 characters") + }example-config.yaml (1)
44-46: Avoid weak/sample API key values; prefer envUse a strong random key and document env override to keep secrets out of files.
web: listen_addr: ":8082" - api_key: "random_key" + # Prefer setting via environment: TGRD_WEB_API_KEY + # Example to generate: openssl rand -hex 32 + api_key: "CHANGE_ME_WITH_A_LONG_RANDOM_VALUE"web/static/index.html (2)
1-8: Consider adding Content Security Policy (CSP) meta tag.For enhanced XSS protection, consider adding a CSP meta tag to restrict resource loading and inline script execution.
Add this meta tag in the
<head>section:<meta name="viewport" content="width=device-width, initial-scale=1.0" /> + <meta http-equiv="Content-Security-Policy" content="default-src 'self'; script-src 'self'; style-src 'self'; img-src 'self' data:;"> <title>rdctl-bot Dashboard</title>Note: Adjust the CSP directives based on your specific requirements and ensure they don't break functionality.
52-52: Refactor inline onclick handlers to addEventListener.Inline event handlers make CSP implementation more difficult and are harder to maintain. Consider moving these to the script.js file using
addEventListener.Remove the onclick attributes:
- <button onclick="fetchTorrents()">Refresh Torrents</button> + <button id="refresh-torrents-btn">Refresh Torrents</button>- <button onclick="fetchDownloads()">Refresh Downloads</button> + <button id="refresh-downloads-btn">Refresh Downloads</button>Then add event listeners in script.js (in the DOMContentLoaded block):
document.getElementById("refresh-torrents-btn").addEventListener("click", fetchTorrents); document.getElementById("refresh-downloads-btn").addEventListener("click", fetchDownloads);Also applies to: 58-58
internal/web/server.go (1)
48-51: Add graceful shutdown support.The server currently blocks on
Listenwithout graceful shutdown handling. This can cause in-flight requests to be terminated abruptly when the application stops.Consider adding graceful shutdown with signal handling:
+ // Start server in a goroutine + go func() { + log.Printf("Starting web server on %s", deps.Config.Web.ListenAddr) + if err := app.Listen(deps.Config.Web.ListenAddr); err != nil { + log.Printf("Web server error: %v", err) + } + }() + + // Wait for interrupt signal + quit := make(chan os.Signal, 1) + signal.Notify(quit, os.Interrupt, syscall.SIGTERM) + <-quit + - log.Printf("Starting web server on %s", deps.Config.Web.ListenAddr) - if err := app.Listen(deps.Config.Web.ListenAddr); err != nil { - log.Printf("Web server shut down or failed: %v", err) + log.Println("Shutting down web server...") + if err := app.Shutdown(); err != nil { + log.Printf("Web server shutdown error: %v", err) } + log.Println("Web server stopped")Note: Add
"os","os/signal", and"syscall"to imports.internal/web/handlers.go (2)
50-74: Consider validating magnet link format.The handler only checks for empty strings but doesn't validate the magnet link format. Invalid links will be caught by the RDClient, but early validation provides better UX.
Add basic magnet link validation:
if body.Magnet == "" { return c.Status(fiber.StatusBadRequest).JSON(fiber.Map{"success": false, "error": "Magnet link is required"}) } + if !strings.HasPrefix(body.Magnet, "magnet:?") { + return c.Status(fiber.StatusBadRequest).JSON(fiber.Map{"success": false, "error": "Invalid magnet link format"}) + } resp, err := d.RDClient.AddMagnet(body.Magnet)Note: Add
"strings"to imports.
98-116: Consider validating URL format.The handler only checks for empty strings. Adding URL format validation can prevent invalid requests and provide better error messages.
Add basic URL validation:
if body.Link == "" { return c.Status(fiber.StatusBadRequest).JSON(fiber.Map{"success": false, "error": "Link is required"}) } + if _, err := url.Parse(body.Link); err != nil || (!strings.HasPrefix(body.Link, "http://") && !strings.HasPrefix(body.Link, "https://")) { + return c.Status(fiber.StatusBadRequest).JSON(fiber.Map{"success": false, "error": "Invalid URL format"}) + } unrestricted, err := d.RDClient.UnrestrictLink(body.Link)Note: Add
"net/url"and"strings"to imports if not already present.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (10)
cmd/rdctl-bot/main.go(3 hunks)example-config.yaml(1 hunks)go.mod(2 hunks)internal/config/config.go(2 hunks)internal/web/handlers.go(1 hunks)internal/web/middleware.go(1 hunks)internal/web/server.go(1 hunks)web/static/index.html(1 hunks)web/static/script.js(1 hunks)web/static/style.css(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
cmd/rdctl-bot/main.go (5)
internal/db/db.go (1)
Init(25-66)internal/web/server.go (2)
Dependencies(14-22)Start(25-52)internal/realdebrid/client.go (1)
NewClient(36-57)internal/db/repository.go (5)
NewUserRepository(18-20)NewActivityRepository(70-72)NewTorrentRepository(103-105)NewDownloadRepository(167-169)NewCommandRepository(207-209)internal/config/config.go (1)
Config(13-19)
internal/web/server.go (4)
internal/realdebrid/client.go (1)
Client(15-19)internal/db/repository.go (5)
UserRepository(13-15)ActivityRepository(65-67)TorrentRepository(98-100)DownloadRepository(160-162)CommandRepository(202-204)internal/config/config.go (2)
Config(13-19)Get(210-212)internal/web/middleware.go (1)
APIKeyAuth(8-23)
internal/web/handlers.go (2)
internal/web/server.go (1)
Dependencies(14-22)internal/realdebrid/torrents.go (1)
FormatStatus(182-206)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build (windows-latest)
🔇 Additional comments (15)
go.mod (1)
7-7: Fiber version looks goodPinning Fiber to v2.52.9 brings BodyParser bounds and MIME fixes. Keep it. Based on learnings.
cmd/rdctl-bot/main.go (2)
14-17: Imports for db/realdebrid/web look consistentNo issues spotted with the new imports.
158-163: DB init placement OK; ensure coordinated shutdown with webDB is initialized before starting web/bot. When fixing web shutdown (see below), stop web before closing DB to avoid handlers using a closed pool.
web/static/style.css (1)
1-152: LGTM! Well-structured CSS with modern patterns.The stylesheet follows modern CSS best practices with custom properties for theming, responsive grid layouts, and a cohesive dark theme. The color tokens and layout rules provide a solid foundation for the dashboard UI.
internal/web/server.go (2)
13-22: LGTM! Clean dependency injection pattern.The Dependencies struct provides a clean way to pass required dependencies to handlers, following good Go practices for dependency injection.
34-46: LGTM! API routes properly secured.The API group is correctly protected by the APIKeyAuth middleware, ensuring all endpoints require authentication. Route definitions are clean and follow RESTful conventions.
web/static/script.js (4)
23-35: LGTM! Clean API fetch wrapper.The
apiFetchfunction provides a clean abstraction for API calls with proper error handling and consistent header injection. The structure is well-designed for the dashboard's needs.
37-45: LGTM! Good user feedback mechanism.The message display function provides clear user feedback with appropriate auto-hide behavior.
125-177: LGTM! Well-structured action functions.The action functions properly handle form submissions, include confirmation dialogs for destructive actions, and provide appropriate user feedback. Error handling is consistent throughout.
181-188: LGTM! Correct byte formatting implementation.The utility function correctly handles byte size formatting with proper edge case handling and standard binary units.
internal/web/handlers.go (5)
12-18: LGTM! Simple and correct status handler.The handler correctly fetches and returns user status with appropriate error handling.
39-47: LGTM! Torrent info handler is correctly implemented.The handler appropriately delegates ID validation to the RDClient layer, which should return appropriate errors for invalid IDs.
77-83: LGTM! Delete handler is correctly implemented.The handler properly delegates deletion to the RDClient with appropriate error handling.
119-125: LGTM! Delete handler is correctly implemented.The handler properly delegates deletion with appropriate error handling.
128-139: LGTM! User stats handler with proper validation.The handler correctly parses and validates the user ID parameter with appropriate error handling and HTTP status codes.
| go func() { | ||
| // Create dependencies for web handlers | ||
| deps := web.Dependencies{ | ||
| RDClient: realdebrid.NewClient(cfg.RealDebrid.BaseURL, cfg.RealDebrid.APIToken, cfg.RealDebrid.Proxy, time.Duration(cfg.RealDebrid.Timeout)*time.Second), | ||
| UserRepo: db.NewUserRepository(database), | ||
| ActivityRepo: db.NewActivityRepository(database), | ||
| TorrentRepo: db.NewTorrentRepository(database), | ||
| DownloadRepo: db.NewDownloadRepository(database), | ||
| CommandRepo: db.NewCommandRepository(database), | ||
| Config: cfg, | ||
| } | ||
| web.Start(deps) | ||
| }() | ||
|
|
There was a problem hiding this comment.
Web server lacks graceful shutdown and error propagation; integrate with context
web.Start(deps) blocks in a goroutine and never stops on SIGTERM, and its failures aren’t observed. Also, DB may be closed while web still serves requests.
Suggested approach:
- Make
web.Start(ctx context.Context, deps Dependencies) errorlisten forctx.Done()and callapp.Shutdown(). - Start it with the same
ctxyou use for the bot, and select on its error channel.
Apply this diff here (paired with changes in internal/web/server.go):
- go func() {
- // Create dependencies for web handlers
- deps := web.Dependencies{
+ // Create dependencies for web handlers
+ deps := web.Dependencies{
RDClient: realdebrid.NewClient(cfg.RealDebrid.BaseURL, cfg.RealDebrid.APIToken, cfg.RealDebrid.Proxy, time.Duration(cfg.RealDebrid.Timeout)*time.Second),
UserRepo: db.NewUserRepository(database),
ActivityRepo: db.NewActivityRepository(database),
TorrentRepo: db.NewTorrentRepository(database),
DownloadRepo: db.NewDownloadRepository(database),
CommandRepo: db.NewCommandRepository(database),
Config: cfg,
- }
- web.Start(deps)
- }()
+ }
+ webErrCh := make(chan error, 1)
+ // Create shutdown context before launching web
+ // (Move the signal.NotifyContext block above this call.)
+ go func() { webErrCh <- web.Start(ctx, deps) }()And extend the wait loop:
select {
case <-ctx.Done():
...
-case err := <-errCh:
+case err := <-errCh:
...
+case err := <-webErrCh:
+ log.Printf("Web server encountered an error: %v", err)
+ log.Println("Initiating shutdown due to web error...")
}In internal/web/server.go, implement:
func Start(ctx context.Context, deps Dependencies) error {
app := fiber.New()
// ...
go func() {
<-ctx.Done()
_ = app.Shutdown()
}()
return app.Listen(deps.Config.Web.ListenAddr)
}🤖 Prompt for AI Agents
In cmd/rdctl-bot/main.go around lines 181-194, the web server is started in a
goroutine with web.Start(deps) that blocks and neither receives the parent
context nor returns errors, risking unobserved failures and DB closure while
handlers are running; change web.Start to accept ctx (web.Start(ctx, deps)
error), call it in a dedicated goroutine that sends its returned error over a
channel, pass the same ctx you use for the bot when calling Start, select on
ctx.Done() and the web error channel in your main wait logic to propagate
startup/runtime errors and only close the database after the web server has shut
down (or after its error is handled) so shutdown ordering is correct.
| RDClient: realdebrid.NewClient(cfg.RealDebrid.BaseURL, cfg.RealDebrid.APIToken, cfg.RealDebrid.Proxy, time.Duration(cfg.RealDebrid.Timeout)*time.Second), | ||
| UserRepo: db.NewUserRepository(database), |
There was a problem hiding this comment.
🧩 Analysis chain
Avoid os.Exit in NewClient; return an error instead
internal/realdebrid/client.go: NewClient calls os.Exit(1) on invalid proxy URL (per snippet), which can terminate the whole process from here. Change it to return (*Client, error) and handle the error at the call site.
Would you like a patch for internal/realdebrid/client.go and the call sites?
🏁 Script executed:
#!/bin/bash
# Find and examine the NewClient function in internal/realdebrid/client.go
fd -t f "client.go" internal/realdebrid/ && cat internal/realdebrid/client.goLength of output: 5714
🏁 Script executed:
#!/bin/bash
# Search for all calls to NewClient in the codebase
rg -n "NewClient" --type go -B2 -A2Length of output: 1090
🏁 Script executed:
#!/bin/bash
# Search for os.Exit calls in internal/realdebrid/client.go
rg -n "os\.Exit" internal/realdebrid/Length of output: 46
🏁 Script executed:
#!/bin/bash
# Examine lines 180-190 in cmd/rdctl-bot/main.go for context
cat cmd/rdctl-bot/main.go | sed -n '180,190p'Length of output: 525
🏁 Script executed:
#!/bin/bash
# Check for other os.Exit calls in the codebase that might indicate pattern
rg -n "os\.Exit" --type go -B3 -A1Length of output: 635
🏁 Script executed:
#!/bin/bash
# Examine internal/bot/bot.go:58 for context and error handling
cat internal/bot/bot.go | sed -n '55,65p'Length of output: 212
Return error from NewClient instead of calling os.Exit
The NewClient function in internal/realdebrid/client.go calls os.Exit(1) when proxy URL parsing fails, terminating the entire process. This is problematic because:
- The caller has no way to handle the error (function returns only
*Client, noterror) - One call site is inside a goroutine (
cmd/rdctl-bot/main.go:184), causing unexpected process termination from a background task - The code comment contradicts the behavior ("proceed without proxy" vs. immediate exit)
Change NewClient signature to func NewClient(...) (*Client, error) and return the error. Update call sites in internal/bot/bot.go:58 and cmd/rdctl-bot/main.go:184 to handle the returned error.
🤖 Prompt for AI Agents
In cmd/rdctl-bot/main.go around lines 184-185 and internal/realdebrid/client.go,
NewClient currently calls os.Exit on proxy parse errors and returns only
*Client; change its signature to func NewClient(...)(*Client, error), remove any
os.Exit usage and return the parse error (or nil, err) so callers can handle it;
update all call sites (at least cmd/rdctl-bot/main.go:184 and
internal/bot/bot.go:58) to check the error, propagate or log/handle it
appropriately (e.g., log and stop initialization or fallback to no-proxy
behavior) instead of letting the client terminate the process.
| github.com/valyala/bytebufferpool v1.0.0 // indirect | ||
| github.com/valyala/fasthttp v1.51.0 // indirect | ||
| github.com/valyala/tcplisten v1.0.0 // indirect | ||
| go.yaml.in/yaml/v3 v3.0.4 // indirect |
There was a problem hiding this comment.
Broken indirect module path: gopkg.in/yaml.v3, not go.yaml.in/yaml/v3
This will break go mod tidy/build. Fix the path.
-go.yaml.in/yaml/v3 v3.0.4 // indirect
+gopkg.in/yaml.v3 v3.0.4 // indirectAfter the change, run:
go mod tidy🤖 Prompt for AI Agents
In go.mod around line 43, the indirect module path is incorrect
("go.yaml.in/yaml/v3 v3.0.4"); replace it with the correct module path
"gopkg.in/yaml.v3 v3.0.4" so the indirect dependency resolves properly, then
save and run "go mod tidy" to update the go.sum and verify the build.
| func (d *Dependencies) GetTorrents(c *fiber.Ctx) error { | ||
| limit, _ := strconv.Atoi(c.Query("limit", "50")) | ||
| offset, _ := strconv.Atoi(c.Query("offset", "0")) | ||
|
|
||
| torrents, err := d.RDClient.GetTorrents(limit, offset) | ||
| if err != nil { | ||
| return c.Status(fiber.StatusInternalServerError).JSON(fiber.Map{"success": false, "error": err.Error()}) | ||
| } | ||
|
|
||
| // Format status and size for frontend convenience | ||
| for i := range torrents { | ||
| torrents[i].Status = realdebrid.FormatStatus(torrents[i].Status) | ||
| } | ||
|
|
||
| return c.JSON(fiber.Map{"success": true, "data": torrents}) | ||
| } |
There was a problem hiding this comment.
Add validation for pagination parameters.
The handler silently ignores parsing errors and doesn't validate limit/offset ranges, which could lead to resource exhaustion or unexpected behavior.
Add proper validation:
func (d *Dependencies) GetTorrents(c *fiber.Ctx) error {
- limit, _ := strconv.Atoi(c.Query("limit", "50"))
- offset, _ := strconv.Atoi(c.Query("offset", "0"))
+ limit, err := strconv.Atoi(c.Query("limit", "50"))
+ if err != nil || limit < 1 || limit > 100 {
+ limit = 50
+ }
+ offset, err := strconv.Atoi(c.Query("offset", "0"))
+ if err != nil || offset < 0 {
+ offset = 0
+ }
torrents, err := d.RDClient.GetTorrents(limit, offset)This prevents:
- Negative offset values
- Excessively large limit values (DoS)
- Invalid non-numeric inputs falling through silently
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| func (d *Dependencies) GetTorrents(c *fiber.Ctx) error { | |
| limit, _ := strconv.Atoi(c.Query("limit", "50")) | |
| offset, _ := strconv.Atoi(c.Query("offset", "0")) | |
| torrents, err := d.RDClient.GetTorrents(limit, offset) | |
| if err != nil { | |
| return c.Status(fiber.StatusInternalServerError).JSON(fiber.Map{"success": false, "error": err.Error()}) | |
| } | |
| // Format status and size for frontend convenience | |
| for i := range torrents { | |
| torrents[i].Status = realdebrid.FormatStatus(torrents[i].Status) | |
| } | |
| return c.JSON(fiber.Map{"success": true, "data": torrents}) | |
| } | |
| func (d *Dependencies) GetTorrents(c *fiber.Ctx) error { | |
| limit, err := strconv.Atoi(c.Query("limit", "50")) | |
| if err != nil || limit < 1 || limit > 100 { | |
| limit = 50 | |
| } | |
| offset, err := strconv.Atoi(c.Query("offset", "0")) | |
| if err != nil || offset < 0 { | |
| offset = 0 | |
| } | |
| torrents, err := d.RDClient.GetTorrents(limit, offset) | |
| if err != nil { | |
| return c.Status(fiber.StatusInternalServerError).JSON(fiber.Map{"success": false, "error": err.Error()}) | |
| } | |
| // Format status and size for frontend convenience | |
| for i := range torrents { | |
| torrents[i].Status = realdebrid.FormatStatus(torrents[i].Status) | |
| } | |
| return c.JSON(fiber.Map{"success": true, "data": torrents}) | |
| } |
🤖 Prompt for AI Agents
In internal/web/handlers.go around lines 21 to 36, the handler currently ignores
strconv.Atoi errors and does not validate pagination ranges; update parsing to
check errors and return a 400 with a clear message when limit/offset are
non-numeric, enforce offset >= 0, clamp or reject limit (e.g., maxLimit = 100)
and reject limit <= 0, and return a 400 on violations; if values are omitted
keep sensible defaults, and only call d.RDClient.GetTorrents when validated to
prevent negative offsets or excessively large limits causing resource
exhaustion.
| func (d *Dependencies) GetDownloads(c *fiber.Ctx) error { | ||
| limit, _ := strconv.Atoi(c.Query("limit", "50")) | ||
| offset, _ := strconv.Atoi(c.Query("offset", "0")) | ||
|
|
||
| downloads, err := d.RDClient.GetDownloads(limit, offset) | ||
| if err != nil { | ||
| return c.Status(fiber.StatusInternalServerError).JSON(fiber.Map{"success": false, "error": err.Error()}) | ||
| } | ||
| return c.JSON(fiber.Map{"success": true, "data": downloads}) | ||
| } |
There was a problem hiding this comment.
Same pagination validation issue as GetTorrents.
This handler has the same lack of input validation for limit and offset parameters.
Apply the same validation fix as recommended for GetTorrents (lines 21-36).
🤖 Prompt for AI Agents
In internal/web/handlers.go around lines 86 to 95, the GetDownloads handler does
not validate query parameters; mirror the GetTorrents fix by parsing limit and
offset with error handling (return a 400 with a clear message on parse failure),
enforce acceptable ranges (e.g., default limit 50, min 1, cap limit to a sane
max like 100; ensure offset is >= 0), and use the validated/clamped values when
calling d.RDClient.GetDownloads; update responses to return a bad request on
invalid input rather than silently using defaults.
| func Start(deps Dependencies) { | ||
| app := fiber.New() | ||
|
|
||
| // Middleware | ||
| app.Use(logger.New()) | ||
|
|
||
| // Static files for the dashboard | ||
| app.Static("/", "./web/static") |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Harden Fiber configuration and static file serving.
Several security and operational considerations:
- Static file path traversal: The static file serving uses a relative path which could behave unexpectedly depending on the working directory.
- Missing security headers: No security headers middleware configured.
- No server timeouts: Default Fiber config lacks read/write timeouts.
Apply these improvements:
func Start(deps Dependencies) {
- app := fiber.New()
+ app := fiber.New(fiber.Config{
+ ReadTimeout: time.Second * 30,
+ WriteTimeout: time.Second * 30,
+ IdleTimeout: time.Second * 120,
+ ServerHeader: "rdctl-bot",
+ AppName: "rdctl-bot",
+ })
// Middleware
app.Use(logger.New())
+ app.Use(func(c *fiber.Ctx) error {
+ c.Set("X-Content-Type-Options", "nosniff")
+ c.Set("X-Frame-Options", "DENY")
+ c.Set("X-XSS-Protection", "1; mode=block")
+ return c.Next()
+ })
// Static files for the dashboard
- app.Static("/", "./web/static")
+ app.Static("/", "./web/static", fiber.Static{
+ Browse: false,
+ Index: "index.html",
+ })Note: Add "time" to imports.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In internal/web/server.go around lines 25 to 32, harden the Fiber setup: replace
fiber.New() with fiber.New(fiber.Config{ReadTimeout: 10 * time.Second,
WriteTimeout: 10 * time.Second, IdleTimeout: 30 * time.Second}) (add "time" to
imports), serve static files using an absolute path (e.g., resolve with
filepath.Abs or use path.Join with a configured web root) instead of a raw
relative "./web/static" to avoid path-traversal/working-directory issues, and
register security middleware before routes (e.g., helmet-equivalent or manually
set common security headers like Content-Security-Policy, X-Frame-Options,
X-Content-Type-Options, Referrer-Policy) so headers are applied globally.
| document.addEventListener("DOMContentLoaded", () => { | ||
| const apiKey = prompt("Please enter your API Key:"); | ||
| if (!apiKey) { | ||
| alert("API Key is required to use the dashboard."); | ||
| return; | ||
| } | ||
| window.apiKey = apiKey; | ||
|
|
||
| fetchStatus(); | ||
| fetchTorrents(); | ||
| fetchDownloads(); | ||
|
|
||
| document | ||
| .getElementById("add-torrent-form") | ||
| .addEventListener("submit", addTorrent); | ||
| document | ||
| .getElementById("unrestrict-link-form") | ||
| .addEventListener("submit", unrestrictLink); | ||
| }); |
There was a problem hiding this comment.
Critical: Insecure API key storage and handling.
The current implementation stores the API key in the global window.apiKey, making it vulnerable to XSS attacks. Additionally, using prompt() for API key input provides poor UX and no validation.
Issues:
- API key in global scope can be accessed by any malicious script
- No validation of API key format before usage
prompt()is not user-friendly and doesn't mask input- API key stored in plain text in browser memory
Recommended approach:
document.addEventListener("DOMContentLoaded", () => {
- const apiKey = prompt("Please enter your API Key:");
- if (!apiKey) {
- alert("API Key is required to use the dashboard.");
- return;
- }
- window.apiKey = apiKey;
+ // Store API key in sessionStorage with some basic validation
+ let apiKey = sessionStorage.getItem("rdctl_api_key");
+ if (!apiKey) {
+ apiKey = prompt("Please enter your API Key:");
+ if (!apiKey || apiKey.trim().length < 10) {
+ alert("Valid API Key is required to use the dashboard.");
+ return;
+ }
+ sessionStorage.setItem("rdctl_api_key", apiKey.trim());
+ }
+
+ // Use closure to keep API key private
+ const getApiKey = () => sessionStorage.getItem("rdctl_api_key");
+ window.getApiKey = getApiKey;
fetchStatus();
fetchTorrents();Then update apiFetch:
async function apiFetch(url, options = {}) {
options.headers = {
"Content-Type": "application/json",
- "X-API-Key": window.apiKey,
+ "X-API-Key": window.getApiKey(),
...options.headers,
};Note: For production, consider implementing a proper login flow with session cookies instead of API key prompts.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| document.addEventListener("DOMContentLoaded", () => { | |
| const apiKey = prompt("Please enter your API Key:"); | |
| if (!apiKey) { | |
| alert("API Key is required to use the dashboard."); | |
| return; | |
| } | |
| window.apiKey = apiKey; | |
| fetchStatus(); | |
| fetchTorrents(); | |
| fetchDownloads(); | |
| document | |
| .getElementById("add-torrent-form") | |
| .addEventListener("submit", addTorrent); | |
| document | |
| .getElementById("unrestrict-link-form") | |
| .addEventListener("submit", unrestrictLink); | |
| }); | |
| document.addEventListener("DOMContentLoaded", () => { | |
| // Store API key in sessionStorage with some basic validation | |
| let apiKey = sessionStorage.getItem("rdctl_api_key"); | |
| if (!apiKey) { | |
| apiKey = prompt("Please enter your API Key:"); | |
| if (!apiKey || apiKey.trim().length < 10) { | |
| alert("Valid API Key is required to use the dashboard."); | |
| return; | |
| } | |
| sessionStorage.setItem("rdctl_api_key", apiKey.trim()); | |
| } | |
| // Use closure to keep API key private | |
| const getApiKey = () => sessionStorage.getItem("rdctl_api_key"); | |
| window.getApiKey = getApiKey; | |
| fetchStatus(); | |
| fetchTorrents(); | |
| fetchDownloads(); | |
| document | |
| .getElementById("add-torrent-form") | |
| .addEventListener("submit", addTorrent); | |
| document | |
| .getElementById("unrestrict-link-form") | |
| .addEventListener("submit", unrestrictLink); | |
| }); |
| async function fetchStatus() { | ||
| try { | ||
| const result = await apiFetch(`${API_BASE_URL}/status`); | ||
| const user = result.data; | ||
| const container = document.getElementById("status-container"); | ||
| const expirationDate = new Date(user.expiration).toLocaleDateString(); | ||
| container.innerHTML = ` | ||
| <p><strong>User:</strong> ${user.username} | <strong>Type:</strong> ${user.type} | <strong>Expires:</strong> ${expirationDate} | <strong>Points:</strong> ${user.points}</p> | ||
| `; | ||
| } catch (error) { | ||
| console.error("Error fetching status:", error); | ||
| } | ||
| } |
There was a problem hiding this comment.
XSS vulnerability in innerHTML usage.
The function uses innerHTML to inject user data without sanitization. If the API returns malicious content (e.g., compromised backend or MITM attack), it could lead to XSS.
Use textContent for user-controlled data or create elements safely:
async function fetchStatus() {
try {
const result = await apiFetch(`${API_BASE_URL}/status`);
const user = result.data;
const container = document.getElementById("status-container");
const expirationDate = new Date(user.expiration).toLocaleDateString();
- container.innerHTML = `
- <p><strong>User:</strong> ${user.username} | <strong>Type:</strong> ${user.type} | <strong>Expires:</strong> ${expirationDate} | <strong>Points:</strong> ${user.points}</p>
- `;
+ const p = document.createElement("p");
+ p.innerHTML = `<strong>User:</strong> ${escapeHtml(user.username)} | <strong>Type:</strong> ${escapeHtml(user.type)} | <strong>Expires:</strong> ${expirationDate} | <strong>Points:</strong> ${user.points}`;
+ container.replaceChildren(p);
} catch (error) {
console.error("Error fetching status:", error);
}
}Add a helper function:
function escapeHtml(unsafe) {
return unsafe
.replace(/&/g, "&")
.replace(/</g, "<")
.replace(/>/g, ">")
.replace(/"/g, """)
.replace(/'/g, "'");
}🤖 Prompt for AI Agents
In web/static/script.js around lines 49 to 61, the code uses innerHTML to render
user-controlled fields which risks XSS; replace the innerHTML usage with safe
DOM creation or textContent assignment: create the paragraph and child text
nodes or elements for username, type, expiration and points, setting their
textContent (or run values through an escapeHtml helper) rather than injecting a
formatted HTML string, then append the nodes to the status-container; ensure no
user value is ever inserted via innerHTML.
| async function fetchTorrents() { | ||
| try { | ||
| const result = await apiFetch(`${API_BASE_URL}/torrents`); | ||
| const torrents = result.data || []; | ||
| const list = document.getElementById("torrents-list"); | ||
| list.innerHTML = ""; | ||
| if (torrents.length === 0) { | ||
| list.innerHTML = "<p>No active torrents found.</p>"; | ||
| return; | ||
| } | ||
| torrents.forEach((t) => { | ||
| const card = document.createElement("div"); | ||
| card.className = "item-card"; | ||
| card.innerHTML = ` | ||
| <h3>${t.filename}</h3> | ||
| <p class="id">ID: ${t.id}</p> | ||
| <p>Status: <strong>${t.status}</strong></p> | ||
| <p>Size: ${formatBytes(t.bytes)}</p> | ||
| <p>Progress: ${t.progress.toFixed(1)}%</p> | ||
| <div class="progress-bar"> | ||
| <div class="progress-bar-inner" style="width: ${t.progress}%;"></div> | ||
| </div> | ||
| <p>Speed: ${formatBytes(t.speed)}/s | Seeders: ${t.seeders || 0}</p> | ||
| <button onclick="deleteTorrent('${t.id}')">Delete</button> | ||
| `; | ||
| list.appendChild(card); | ||
| }); | ||
| } catch (error) { | ||
| showMessage(`Error fetching torrents: ${error.message}`, true); | ||
| } | ||
| } |
There was a problem hiding this comment.
XSS vulnerability and inline event handlers in dynamic content.
Multiple security and maintainability issues:
- XSS risk: Using
innerHTMLwith unsanitized torrent data (filename, id, status) - Inline onclick: Creates CSP violations and security risks
- String interpolation: IDs are interpolated directly into onclick handlers
Refactor to use safe DOM manipulation:
torrents.forEach((t) => {
const card = document.createElement("div");
card.className = "item-card";
- card.innerHTML = `
- <h3>${t.filename}</h3>
- <p class="id">ID: ${t.id}</p>
- <p>Status: <strong>${t.status}</strong></p>
- <p>Size: ${formatBytes(t.bytes)}</p>
- <p>Progress: ${t.progress.toFixed(1)}%</p>
- <div class="progress-bar">
- <div class="progress-bar-inner" style="width: ${t.progress}%;"></div>
- </div>
- <p>Speed: ${formatBytes(t.speed)}/s | Seeders: ${t.seeders || 0}</p>
- <button onclick="deleteTorrent('${t.id}')">Delete</button>
- `;
+
+ const h3 = document.createElement("h3");
+ h3.textContent = t.filename;
+ card.appendChild(h3);
+
+ const idP = document.createElement("p");
+ idP.className = "id";
+ idP.textContent = `ID: ${t.id}`;
+ card.appendChild(idP);
+
+ // ... create other elements similarly ...
+
+ const deleteBtn = document.createElement("button");
+ deleteBtn.textContent = "Delete";
+ deleteBtn.addEventListener("click", () => deleteTorrent(t.id));
+ card.appendChild(deleteBtn);
+
list.appendChild(card);
});Alternatively, if you trust your backend completely, use the escapeHtml helper from the previous comment.
Committable suggestion skipped: line range outside the PR's diff.
| async function fetchDownloads() { | ||
| try { | ||
| const result = await apiFetch(`${API_BASE_URL}/downloads`); | ||
| const downloads = result.data || []; | ||
| const list = document.getElementById("downloads-list"); | ||
| list.innerHTML = ""; | ||
| if (downloads.length === 0) { | ||
| list.innerHTML = "<p>No recent downloads found.</p>"; | ||
| return; | ||
| } | ||
| downloads.forEach((d) => { | ||
| const card = document.createElement("div"); | ||
| card.className = "item-card"; | ||
| card.innerHTML = ` | ||
| <h3>${d.filename}</h3> | ||
| <p class="id">ID: ${d.id}</p> | ||
| <p>Size: ${formatBytes(d.filesize)}</p> | ||
| <p>Host: ${d.host}</p> | ||
| <p>Generated: ${new Date(d.generated).toLocaleString()}</p> | ||
| <button onclick="deleteDownload('${d.id}')">Delete</button> | ||
| `; | ||
| list.appendChild(card); | ||
| }); | ||
| } catch (error) { | ||
| showMessage(`Error fetching downloads: ${error.message}`, true); | ||
| } | ||
| } |
There was a problem hiding this comment.
Same XSS and inline onclick issues as fetchTorrents.
This function has identical security concerns to fetchTorrents: unsanitized innerHTML usage and inline event handlers.
Apply the same refactoring approach as recommended for fetchTorrents (use createElement, textContent, and addEventListener instead of innerHTML with inline onclick).
Summary by CodeRabbit