Conversation
…nality - Refactor plugin introspection and store utilities to use absolute paths - Add handler for fetching and saving plugin store URLs - Update plugin list synchronization to handle multiple sources and default URLs - Enhance UI for managing plugin store repositories with real-time updates
- Introduce `PluginDataDir` for managing plugin data storage locations. - Ensure directories for persistent plugin data are created with appropriate permissions. - Update `PluginManager` to handle `PluginDataDir` initialization. - Modify lifecycle and plugin code to utilize the new data directory configuration.
AnthonyMichaelTDM
left a comment
There was a problem hiding this comment.
Other than that comment, looks good to me
| // Prefer source entrypoints for plugin repositories that ship source code. | ||
| // This avoids stale checked-in binaries shadowing newer API/UI changes. | ||
| if _, err := os.Stat(goModPath); err == nil { | ||
| if runtime.GOOS == "windows" { | ||
| if _, err := os.Stat(startBatchPath); err == nil { | ||
| return startBatchPath, nil | ||
| } | ||
| } else { | ||
| if _, err := os.Stat(startScriptPath); err == nil { | ||
| return startScriptPath, nil | ||
| } | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
I'm not sure about this. Typically plugins should ship the executable, and if you're developing a plugin you can just recompile it to get the latest version.
Also, not every plugin is implemented in Go (ex: rust api) so this isn't a reliable detection mechanism
There was a problem hiding this comment.
You're right — this is specific to Go, so checking for go.mod here isn't necessary. We'll still get support for Windows and other operating systems.
There was a problem hiding this comment.
Pull request overview
This PR enhances Zoraxy’s plugin manager by adding persistent, user-configurable plugin repository URLs and improving plugin store syncing/introspection behavior.
Changes:
- Added an “Advanced Settings” UI for editing plugin store repository URLs and persisting them via a new API endpoint.
- Refactored plugin store syncing to support multiple sources, default URL fallback, and relative→absolute asset URL resolution.
- Introduced a per-plugin persistent data directory passed to plugins at startup, and made plugin introspection run from an absolute entrypoint path with a stable working directory.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/web/snippet/pluginstore.html | Adds UI for managing plugin store repository URLs and triggers save/resync flows. |
| src/start.go | Initializes plugin manager with a plugin data dir, default store URLs, and loads persisted store URLs on startup. |
| src/mod/plugins/zoraxy_plugin/zoraxy_plugin.go | Extends plugin configure spec with a data_dir field for plugin-owned persistent storage. |
| src/mod/plugins/utils.go | Prefers start.sh/start.bat for source-based plugins (detected via go.mod) before falling back to binaries. |
| src/mod/plugins/typdef.go | Adds PluginDataDir to plugin manager options for persistent plugin storage. |
| src/mod/plugins/store.go | Adds default/persisted repo URL handling, multi-source sync + dedupe, relative asset URL resolution, and a new URLs API handler. |
| src/mod/plugins/lifecycle.go | Creates per-plugin data directories and passes them into plugin configuration at start time. |
| src/mod/plugins/introspect.go | Uses absolute entrypoint paths and sets cmd.Dir for plugin introspection execution. |
| src/api.go | Registers the new /api/plugins/store/urls endpoint. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if strings.TrimSpace(m.Options.PluginDataDir) != "" { | ||
| pluginConfiguration.DataDir = filepath.Join(m.Options.PluginDataDir, thisPlugin.Spec.ID) | ||
| if err := os.MkdirAll(pluginConfiguration.DataDir, 0o775); err != nil { | ||
| return err | ||
| } |
There was a problem hiding this comment.
thisPlugin.Spec.ID is used as a path element when creating pluginConfiguration.DataDir. Since plugin IDs ultimately come from plugin introspection, they should be treated as untrusted: values containing path separators (e.g. ../) could escape PluginDataDir and create/write directories elsewhere. Consider sanitizing/validating the plugin ID for filesystem use (e.g., allowlist [A-Za-z0-9._-] only, or use filepath.Base/reject if Clean changes it) before calling filepath.Join/MkdirAll.
| return nil, fmt.Errorf("failed to unmarshal plugin list from %s: %w", url, err) | ||
| } | ||
|
|
||
| baseURL, _ := urlpkg.Parse(url) | ||
|
|
||
| // Filter out if IconPath is empty string, set it to "/img/plugin_icon.png" | ||
| for _, plugin := range pluginList { |
There was a problem hiding this comment.
In getPluginListFromURL, repository fetches are done via http.Get (default client) without any timeout. Now that sync iterates over multiple sources, a single slow/hung repository can block resync/startup sync for an unbounded time. Consider switching to an http.Client{Timeout: ...} and/or http.NewRequestWithContext with a deadline so each source has a bounded cost.
There was a problem hiding this comment.
This is a good point, maybe it'd be better to pull each list in parallel with a timeout (say, 20 seconds or something) and collect results over a channel?
| func (m *Manager) SavePluginStoreURLs(urls []string) error { | ||
| normalized := normalizePluginStoreURLs(urls) | ||
| m.Options.PluginStoreURLs = normalized | ||
| if m.Options.Database == nil { | ||
| return nil | ||
| } | ||
| return m.Options.Database.Write("plugins", "store_urls", normalized) | ||
| } |
There was a problem hiding this comment.
SavePluginStoreURLs always normalizes an empty list to the current defaults and then persists that default slice to the database. That means “reset to defaults” will pin the user to today’s default list and they won’t automatically pick up future default repository additions/removals. Consider treating an empty input as “unset override” (e.g., Database.Delete("plugins","store_urls")) while keeping m.Options.PluginStoreURLs in-memory set to GetDefaultPluginStoreURLs().
| rawURLs, err := utils.PostPara(r, "urls") | ||
| if err != nil && !strings.Contains(err.Error(), "invalid urls") { | ||
| utils.SendErrorResponse(w, "urls not found") | ||
| return | ||
| } |
There was a problem hiding this comment.
HandlePluginStoreURLs relies on substring-matching the error message from utils.PostPara (strings.Contains(err.Error(), "invalid urls")) to decide whether to treat a missing/empty value as a reset. This is brittle (it couples behavior to an error string format) and can misbehave if PostPara changes. Consider handling this explicitly (e.g., accept missing/empty urls as reset without checking the error string, but still fail on ParseForm errors).
| rawURLs, err := utils.PostPara(r, "urls") | |
| if err != nil && !strings.Contains(err.Error(), "invalid urls") { | |
| utils.SendErrorResponse(w, "urls not found") | |
| return | |
| } | |
| if err := r.ParseForm(); err != nil { | |
| utils.SendErrorResponse(w, "invalid form data: "+err.Error()) | |
| return | |
| } | |
| rawURLs := r.FormValue("urls") |
|
|
||
| // See https://github.com/aroz-online/zoraxy-official-plugins/blob/main/directories/index2.json for the standard format | ||
|
|
||
| var defaultPluginStoreURLs = []string{ | ||
| "https://raw.githubusercontent.com/aroz-online/zoraxy-official-plugins/refs/heads/main/directories/index2.json", | ||
| } | ||
|
|
||
| type DownloadablePlugin struct { | ||
| IconPath string `json:"IconPath"` //Icon path or URL for the plugin | ||
| PluginIntroSpect zoraxy_plugin.IntroSpect `json:"PluginIntroSpect"` //Plugin introspect information | ||
| DownloadURLs map[string]string `json:"DownloadURLs"` //Download URLs for different platforms | ||
| } | ||
|
|
||
| func GetDefaultPluginStoreURLs() []string { | ||
| return append([]string{}, defaultPluginStoreURLs...) | ||
| } | ||
|
|
||
| func normalizePluginStoreURLs(rawURLs []string) []string { | ||
| seen := map[string]bool{} | ||
| normalized := []string{} | ||
| for _, rawURL := range rawURLs { | ||
| trimmed := strings.TrimSpace(rawURL) | ||
| if trimmed == "" || seen[trimmed] { | ||
| continue | ||
| } | ||
| seen[trimmed] = true | ||
| normalized = append(normalized, trimmed) | ||
| } | ||
| if len(normalized) == 0 { | ||
| return GetDefaultPluginStoreURLs() | ||
| } | ||
| return normalized | ||
| } | ||
|
|
||
| func (m *Manager) GetPluginStoreURLs() []string { | ||
| return normalizePluginStoreURLs(m.Options.PluginStoreURLs) | ||
| } | ||
|
|
||
| func (m *Manager) LoadPluginStoreURLs() error { | ||
| if m.Options.Database == nil { | ||
| m.Options.PluginStoreURLs = normalizePluginStoreURLs(m.Options.PluginStoreURLs) | ||
| return nil | ||
| } | ||
|
|
||
| loaded := []string{} | ||
| if err := m.Options.Database.Read("plugins", "store_urls", &loaded); err != nil { | ||
| m.Options.PluginStoreURLs = normalizePluginStoreURLs(m.Options.PluginStoreURLs) | ||
| return nil | ||
| } | ||
|
|
||
| m.Options.PluginStoreURLs = normalizePluginStoreURLs(loaded) | ||
| return nil | ||
| } | ||
|
|
||
| func (m *Manager) SavePluginStoreURLs(urls []string) error { | ||
| normalized := normalizePluginStoreURLs(urls) | ||
| m.Options.PluginStoreURLs = normalized | ||
| if m.Options.Database == nil { | ||
| return nil | ||
| } | ||
| return m.Options.Database.Write("plugins", "store_urls", normalized) | ||
| } |
There was a problem hiding this comment.
New persistence/normalization logic for repository URLs and the new /api/plugins/store/urls handler aren’t covered by existing store tests (which currently cover list sync and fetching). Adding unit tests for URL normalization/dedup/default fallback, Save/Load round-trips (including empty-reset semantics), and handler GET/POST behavior would help prevent regressions.
| } else { | ||
| parent.msgbox("URLs saved successfully", true); | ||
| parent.msgbox("Plugin store repositories saved", true); | ||
| initPluginStoreRepositories(); |
There was a problem hiding this comment.
After a successful save, this calls both initPluginStoreRepositories() and then forceResyncPlugins(), but forceResyncPlugins() already calls initPluginStoreRepositories() on success. This causes an extra GET request and can make the UI do redundant work; consider removing the standalone initPluginStoreRepositories() here and relying on the resync callback (or vice versa).
| initPluginStoreRepositories(); |
There was a problem hiding this comment.
Can this commit (0ca6d68) be in a separate PR? Or is it necessary for these plugin store improvements?
This PR improves the plugin manager:
Changes: