2026-01-19 20:51:00 -05:00
|
|
|
package plugins
|
|
|
|
|
|
|
|
|
|
import (
|
fix(plugins): reject plugin IDs that are unusable as directory names (#5886)
The plugin ID is derived from the package filename and used verbatim as a
directory name under DataFolder/plugins by the kvstore and taskqueue host
services. A package installed as '..ndp' yields the ID '.', whose data
directory resolves to the parent of every other plugin's directory, so it
overlaps their private data. On Windows, trailing dots and spaces are dropped
during path normalization, so 'foo..ndp' and 'foo.ndp' yield distinct IDs that
resolve to the same directory and would share the same SQLite files.
Discovery and the file watcher now derive the ID through pluginIDFromPath,
which rejects '.', '..', empty names, separators, trailing dots or spaces, and
anything filepath.IsLocal refuses (Windows reserved names, drive-relative
paths). The loader repeats the check, since a sync failure is non-fatal and
could otherwise leave a stale row reaching the host services.
2026-08-03 13:09:53 -04:00
|
|
|
"github.com/navidrome/navidrome/model"
|
2026-01-19 20:51:00 -05:00
|
|
|
. "github.com/onsi/ginkgo/v2"
|
|
|
|
|
. "github.com/onsi/gomega"
|
|
|
|
|
)
|
|
|
|
|
|
fix(plugins): confine plugin filesystem mounts to their root (#5881)
* fix(plugins): confine plugin filesystem mounts to their root
A plugin granted read-write filesystem access could escape its mount by
creating a relative symlink inside it and then writing through that link,
reaching any path the server process can write, including navidrome.db.
wazero resolves guest paths by concatenating them onto the host root. Its
WASI layer validates every path argument except the symlink target, which
path_symlink forwards unvalidated by design, and fs.ValidPath splits on
"/" only, so on Windows a "..\" path escapes the mount as well.
Mounts now go through a jailedFS wrapper that denies symlink creation and
rejects any path that is not filepath.IsLocal. That requires bypassing
extism's AllowedPaths, which discards any FSConfig passed alongside it, so
the mounts are built directly and applied per instance instead. Following
symlinks that already exist in a mount is unchanged: music libraries rely
on it, and read-only mounts already reject creating new ones.
* test(plugins): guard against setting extism AllowedPaths
Extracts the extism manifest construction so a test can assert AllowedPaths
is never set. Setting it makes extism build its own FSConfig and discard the
jailed mounts, silently restoring the symlink escape.
Verified by simulating the regression: with AllowedPaths populated for plugins
holding the filesystem permission, the new spec fails, as do two of the
end-to-end sandbox specs.
2026-08-02 12:27:08 -04:00
|
|
|
var _ = Describe("buildExtismManifest", func() {
|
|
|
|
|
var pkg *ndpPackage
|
|
|
|
|
|
|
|
|
|
BeforeEach(func() {
|
|
|
|
|
pkg = &ndpPackage{
|
|
|
|
|
WasmBytes: []byte("wasm"),
|
|
|
|
|
Manifest: &Manifest{Permissions: &Permissions{
|
|
|
|
|
Library: &LibraryPermission{Reason: new("test"), Filesystem: true},
|
|
|
|
|
Http: &HTTPPermission{Reason: new("test"), RequiredHosts: []string{"example.com"}},
|
|
|
|
|
}},
|
|
|
|
|
}
|
|
|
|
|
})
|
|
|
|
|
|
|
|
|
|
It("never sets AllowedPaths, even with filesystem permission", func() {
|
|
|
|
|
Expect(buildExtismManifest(pkg, nil).AllowedPaths).To(BeEmpty())
|
|
|
|
|
})
|
|
|
|
|
|
Merge commit from fork
* fix(share): always assign the authenticated user as share owner
A share's UserID was taken from the request body and only defaulted when
empty, so any authenticated user could create a share attributed to
another user. For playlist shares the contents are resolved in the
owner's library-access context, turning the spoofed owner into an
access-escalation vector in multi-library setups.
Force the owner from the request context at both the service boundary
and the persistence layer, ignoring any client-supplied UserID.
* fix(plugins): block SSRF to private IPs resolved from hostnames
The HTTP host client only checked the literal host string, so a symbolic
hostname (or a trailing-dot "localhost.") resolving to a private/loopback
address bypassed the SSRF guard when a plugin declared no requiredHosts.
Enforce the check at dial time via net.Dialer.Control on the resolved IP,
which also covers redirect hops and DNS rebinding. When an explicit
requiredHosts allowlist is set, defer to it as the operator's trust decision.
* fix(plugins): gate private IPs on explicit IP/CIDR allowlist entries
Following review feedback: an allowlisted hostname authorizes the external
service, not whatever private IP it may resolve or rebind to. Enforce the
resolved-IP guard even when requiredHosts is set, permitting a private
address only when a literal IP or CIDR entry explicitly covers it. This
keeps "reach this external API" and "reach my internal network" as two
separate, explicit operator decisions.
* fix(plugins): treat unspecified addresses as private in the SSRF guard
Dialing 0.0.0.0 or :: reaches the local host, so they bypassed the
private/loopback check.
* fix(plugins): let a bare "*" allowlist reach private addresses
Plugins such as AudioMuse-AI declare requiredHosts ["*"] to reach a
user-configured service on the LAN, whose address the manifest cannot
know. Requiring a literal IP/CIDR entry broke them. Named hosts and
subdomain wildcards still cannot resolve to private addresses.
* refactor(plugins): simplify the SSRF-guarded HTTP client and release its pool
Build the client directly around the guarded transport instead of
replacing a throwaway one, fail closed on an unparseable dial address,
and close the per-plugin transport's idle connections when the plugin
unloads. Trim stale comments.
* fix(plugins): stop enabling extism's unguarded http_request host function
Passing requiredHosts as the extism manifest's AllowedHosts enabled
extism's own http_request (pdk.NewHTTPRequest), which only glob-matches
the hostname and follows redirects without re-checking, bypassing the
resolved-IP SSRF guard. Plugins must use host.HTTPSend.
* fix(plugins): move bundled Rust examples to the host HTTP service
Extism's built-in http_request is now disabled, so the webhook and
Discord examples switch to nd_pdk::host::http::send. Update the README
to say host.HTTPSend is the only supported way to make HTTP requests.
* fix(plugins): move the Python example to the host HTTP service
coverartarchive-py used extism's built-in Http.request, which is now
disabled. Call Navidrome's http_send host function instead. The plugin
can no longer run under the standalone extism CLI, so drop the CLI test
targets and instructions.
2026-09-12 13:38:08 -04:00
|
|
|
It("never sets AllowedHosts, so plugin HTTP can't bypass the host service's SSRF guard", func() {
|
|
|
|
|
Expect(buildExtismManifest(pkg, nil).AllowedHosts).To(BeEmpty())
|
fix(plugins): confine plugin filesystem mounts to their root (#5881)
* fix(plugins): confine plugin filesystem mounts to their root
A plugin granted read-write filesystem access could escape its mount by
creating a relative symlink inside it and then writing through that link,
reaching any path the server process can write, including navidrome.db.
wazero resolves guest paths by concatenating them onto the host root. Its
WASI layer validates every path argument except the symlink target, which
path_symlink forwards unvalidated by design, and fs.ValidPath splits on
"/" only, so on Windows a "..\" path escapes the mount as well.
Mounts now go through a jailedFS wrapper that denies symlink creation and
rejects any path that is not filepath.IsLocal. That requires bypassing
extism's AllowedPaths, which discards any FSConfig passed alongside it, so
the mounts are built directly and applied per instance instead. Following
symlinks that already exist in a mount is unchanged: music libraries rely
on it, and read-only mounts already reject creating new ones.
* test(plugins): guard against setting extism AllowedPaths
Extracts the extism manifest construction so a test can assert AllowedPaths
is never set. Setting it makes extism build its own FSConfig and discard the
jailed mounts, silently restoring the symlink escape.
Verified by simulating the regression: with AllowedPaths populated for plugins
holding the filesystem permission, the new spec fails, as do two of the
end-to-end sandbox specs.
2026-08-02 12:27:08 -04:00
|
|
|
})
|
|
|
|
|
})
|
|
|
|
|
|
fix(plugins): reject plugin IDs that are unusable as directory names (#5886)
The plugin ID is derived from the package filename and used verbatim as a
directory name under DataFolder/plugins by the kvstore and taskqueue host
services. A package installed as '..ndp' yields the ID '.', whose data
directory resolves to the parent of every other plugin's directory, so it
overlaps their private data. On Windows, trailing dots and spaces are dropped
during path normalization, so 'foo..ndp' and 'foo.ndp' yield distinct IDs that
resolve to the same directory and would share the same SQLite files.
Discovery and the file watcher now derive the ID through pluginIDFromPath,
which rejects '.', '..', empty names, separators, trailing dots or spaces, and
anything filepath.IsLocal refuses (Windows reserved names, drive-relative
paths). The loader repeats the check, since a sync failure is non-fatal and
could otherwise leave a stale row reaching the host services.
2026-08-03 13:09:53 -04:00
|
|
|
var _ = Describe("loadPluginWithConfig", func() {
|
|
|
|
|
// Discovery already rejects these, but a row predating that check, or one
|
|
|
|
|
// left behind by a failed sync, must not reach the mount setup
|
|
|
|
|
It("refuses a plugin whose ID is not usable as a directory name", func() {
|
|
|
|
|
m := &Manager{plugins: make(map[string]*plugin)}
|
|
|
|
|
|
|
|
|
|
err := m.loadPluginWithConfig(&model.Plugin{ID: "..", Path: "/does/not/matter.ndp"})
|
|
|
|
|
|
|
|
|
|
Expect(err).To(MatchError(ContainSubstring("invalid plugin ID")))
|
|
|
|
|
})
|
|
|
|
|
})
|
|
|
|
|
|
2026-01-19 20:51:00 -05:00
|
|
|
var _ = Describe("parsePluginConfig", func() {
|
|
|
|
|
It("returns nil for empty string", func() {
|
|
|
|
|
result, err := parsePluginConfig("")
|
|
|
|
|
Expect(err).ToNot(HaveOccurred())
|
|
|
|
|
Expect(result).To(BeNil())
|
|
|
|
|
})
|
|
|
|
|
|
|
|
|
|
It("serializes object values as JSON strings", func() {
|
|
|
|
|
result, err := parsePluginConfig(`{"settings": {"enabled": true, "count": 5}}`)
|
|
|
|
|
Expect(err).ToNot(HaveOccurred())
|
|
|
|
|
Expect(result).To(HaveLen(1))
|
|
|
|
|
Expect(result["settings"]).To(Equal(`{"count":5,"enabled":true}`))
|
|
|
|
|
})
|
|
|
|
|
|
|
|
|
|
It("handles mixed value types", func() {
|
|
|
|
|
result, err := parsePluginConfig(`{"api_key": "secret", "timeout": 30, "rate": 1.5, "enabled": true, "tags": ["a", "b"]}`)
|
|
|
|
|
Expect(err).ToNot(HaveOccurred())
|
|
|
|
|
Expect(result).To(HaveLen(5))
|
|
|
|
|
Expect(result["api_key"]).To(Equal("secret"))
|
|
|
|
|
Expect(result["timeout"]).To(Equal("30"))
|
|
|
|
|
Expect(result["rate"]).To(Equal("1.5"))
|
|
|
|
|
Expect(result["enabled"]).To(Equal("true"))
|
|
|
|
|
Expect(result["tags"]).To(Equal(`["a","b"]`))
|
|
|
|
|
})
|
|
|
|
|
|
|
|
|
|
It("returns error for invalid JSON", func() {
|
|
|
|
|
_, err := parsePluginConfig(`{invalid json}`)
|
|
|
|
|
Expect(err).To(HaveOccurred())
|
|
|
|
|
Expect(err.Error()).To(ContainSubstring("parsing plugin config"))
|
|
|
|
|
})
|
|
|
|
|
|
|
|
|
|
It("returns error for non-object JSON", func() {
|
|
|
|
|
_, err := parsePluginConfig(`["array", "not", "object"]`)
|
|
|
|
|
Expect(err).To(HaveOccurred())
|
|
|
|
|
Expect(err.Error()).To(ContainSubstring("parsing plugin config"))
|
|
|
|
|
})
|
|
|
|
|
|
|
|
|
|
It("handles null values", func() {
|
|
|
|
|
result, err := parsePluginConfig(`{"key": null}`)
|
|
|
|
|
Expect(err).ToNot(HaveOccurred())
|
|
|
|
|
Expect(result).To(HaveLen(1))
|
|
|
|
|
Expect(result["key"]).To(Equal("null"))
|
|
|
|
|
})
|
|
|
|
|
|
|
|
|
|
It("handles empty object", func() {
|
|
|
|
|
result, err := parsePluginConfig(`{}`)
|
|
|
|
|
Expect(err).ToNot(HaveOccurred())
|
|
|
|
|
Expect(result).To(HaveLen(0))
|
|
|
|
|
Expect(result).ToNot(BeNil())
|
|
|
|
|
})
|
|
|
|
|
})
|