diff --git a/CLAUDE.md b/CLAUDE.md index fa90f629..cc1d60cc 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -77,3 +77,8 @@ For local artifact runs, install GoReleaser, Go 1.25, Node 20+, PNPM 10, and Per ## Repository Structure - Backend: Go server using BoltDB for storage - Frontend: Preact/Redux-based UI with iframe embedding +- `/web` is served from two sources, in lookup order: the frontend build output + (`frontend/apps/remark42/public`, embedded at `backend/app/cmd/web` or read from `--web-root`), + then `backend/app/webassets/assets`, embedded in the binary. A plain page or image the bundler + does not process belongs in `webassets`; anything needing templating or the widget's CSS/JS goes + through webpack. A name present in both is served from the frontend build. diff --git a/backend/app/rest/api/rest.go b/backend/app/rest/api/rest.go index 26b2132c..cac95b4a 100644 --- a/backend/app/rest/api/rest.go +++ b/backend/app/rest/api/rest.go @@ -3,7 +3,6 @@ package api import ( "bytes" "context" - "embed" "encoding/json" "fmt" "io/fs" @@ -27,6 +26,7 @@ import ( "github.com/umputun/remark42/backend/app/store" "github.com/umputun/remark42/backend/app/store/image" "github.com/umputun/remark42/backend/app/store/service" + "github.com/umputun/remark42/backend/app/webassets" ) // Rest is a rest access server @@ -45,7 +45,7 @@ type Rest struct { AnonVote bool WebRoot string - WebFS embed.FS + WebFS fs.FS RemarkURL string ReadOnlyAge int SharedSecret string @@ -385,8 +385,16 @@ func (s *Rest) routes() http.Handler { rroot.HandleFunc("POST /email/unsubscribe.html", s.privRest.emailUnsubscribeCtrl) }) - // file server for static content from s.WebRoot on path /web - addFileServer(router, s.WebFS, s.WebRoot, s.Version) + // file server for /web: the frontend build first, then the assets embedded in the binary. + // the build is embedded under web/ by app/cmd, so that prefix is stripped here. fs.Sub only + // fails for an fs.SubFS that refuses, and a nil result would panic on the first request, so + // serve nothing from the frontend rather than serving it at the wrong paths + embeddedFrontend, err := fs.Sub(s.WebFS, "web") + if err != nil { + log.Printf("[WARN] no embedded frontend, serving built-in assets only: %v", err) + embeddedFrontend = emptyFS{} + } + addFileServer(router, embeddedFrontend, s.WebRoot, s.Version) return router } @@ -499,20 +507,20 @@ func (s *Rest) configCtrl(w http.ResponseWriter, r *http.Request) { R.RenderJSON(w, cnf) } -// serves static files from the webRoot directory or files embedded into the compiled binary if that directory is absent -func addFileServer(r *routegroup.Bundle, embedFS embed.FS, webRoot, version string) { - var webFS http.Handler +// serves /web from the frontend build, falling back to the assets embedded in the binary for +// names the build does not produce. the frontend build is read from webRoot on disk, or from the +// copy embedded at app/cmd/web when that directory is absent. +func addFileServer(r *routegroup.Bundle, embeddedFrontend fs.FS, webRoot, version string) { + frontendFS := embeddedFrontend if _, err := os.Stat(webRoot); err == nil { log.Printf("[INFO] run file server from %s from the disk", webRoot) - webFS = http.FileServer(http.Dir(webRoot)) + frontendFS = os.DirFS(webRoot) } else { log.Printf("[INFO] run file server, embedded") - var contentFS, _ = fs.Sub(embedFS, "web") - webFS = http.FileServer(http.FS(contentFS)) } - webFS = http.StripPrefix("/web", webFS) + webFS := http.StripPrefix("/web", http.FileServer(http.FS(webFiles{frontend: frontendFS, embedded: webassets.FS}))) r.HandleFunc("GET /web", http.RedirectHandler("/web/", http.StatusMovedPermanently).ServeHTTP) r.With(rateLimiter(20), diff --git a/backend/app/rest/api/rest_test.go b/backend/app/rest/api/rest_test.go index 3807df73..2d2a6032 100644 --- a/backend/app/rest/api/rest_test.go +++ b/backend/app/rest/api/rest_test.go @@ -4,16 +4,20 @@ import ( "bytes" "crypto/tls" "encoding/json" + "errors" "fmt" "io" + "io/fs" "math/rand" "net" "net/http" "net/http/httptest" "os" + "path/filepath" "strconv" "strings" "testing" + "testing/fstest" "time" "github.com/go-pkgz/auth/v2" @@ -22,6 +26,7 @@ import ( "github.com/go-pkgz/auth/v2/token" cache "github.com/go-pkgz/lcw/v2" R "github.com/go-pkgz/rest" + "github.com/go-pkgz/routegroup" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" bolt "go.etcd.io/bbolt" @@ -36,6 +41,7 @@ import ( "github.com/umputun/remark42/backend/app/store/engine" "github.com/umputun/remark42/backend/app/store/image" "github.com/umputun/remark42/backend/app/store/service" + "github.com/umputun/remark42/backend/app/webassets" ) // To generate a token, enter one of the tokens here into https://jwt.io, change the secret to one you're using in your test @@ -118,6 +124,182 @@ func TestRest_FileServerStaticAssets(t *testing.T) { }) } +// TestRest_FileServerBackendAssets covers the assets embedded in the binary and the rule that a +// name the frontend build provides is served from there instead. WebRoot is a fresh empty +// directory so the frontend side is known, rather than the shared temp dir startupT defaults to. +func TestRest_FileServerBackendAssets(t *testing.T) { + ts, srv, teardown := startupT(t, func(srv *Rest) { srv.WebRoot = t.TempDir() }) + defer teardown() + + t.Run("serves every embedded asset byte for byte", func(t *testing.T) { + for _, name := range []string{"privacy.html", "markdown-help.html", "400x400.jpeg"} { + t.Run(name, func(t *testing.T) { + want, err := fs.ReadFile(webassets.FS, name) + require.NoError(t, err) + + body, code := get(t, ts.URL+"/web/"+name) + assert.Equal(t, http.StatusOK, code) + assert.Equal(t, string(want), body, "the bytes must come from the embedded assets") + }) + } + }) + + t.Run("serves the image with its own content type", func(t *testing.T) { + resp, err := http.Get(ts.URL + "/web/400x400.jpeg") + require.NoError(t, err) + defer resp.Body.Close() + assert.Equal(t, http.StatusOK, resp.StatusCode) + assert.Equal(t, "image/jpeg", resp.Header.Get("Content-Type")) + }) + + t.Run("head is served", func(t *testing.T) { + resp, err := http.Head(ts.URL + "/web/privacy.html") + require.NoError(t, err) + defer resp.Body.Close() + assert.Equal(t, http.StatusOK, resp.StatusCode) + }) + + t.Run("frontend output wins over the embedded copy", func(t *testing.T) { + require.NoError(t, os.WriteFile(srv.WebRoot+"/privacy.html", []byte("operator's own policy"), 0o600)) + t.Cleanup(func() { _ = os.Remove(srv.WebRoot + "/privacy.html") }) + + body, code := get(t, ts.URL+"/web/privacy.html") + assert.Equal(t, http.StatusOK, code) + assert.Equal(t, "operator's own policy", body) + }) + + t.Run("traversal out of the asset root is refused", func(t *testing.T) { + for _, p := range []string{"/web/../../etc/passwd", "/web/..%2f..%2fetc%2fpasswd", "/web/%2e%2e/%2e%2e/etc/passwd"} { + t.Run(p, func(t *testing.T) { + body, code := get(t, ts.URL+p) + assert.NotContains(t, body, "root:", "must never serve a file outside the served roots") + assert.NotEqual(t, http.StatusInternalServerError, code, "a rejected name must not surface as 500") + }) + } + }) + + t.Run("missing in both still returns 404", func(t *testing.T) { + _, code := get(t, ts.URL+"/web/neither-source-has-this.html") + assert.Equal(t, http.StatusNotFound, code) + }) +} + +// TestRest_FileServerEmbeddedFrontend covers the branch taken when no web root exists on disk, +// which is how the released binary runs. The frontend stands in for the copy embedded at +// app/cmd/web, so a name it provides and a name only the assets provide are both exercised. +func TestRest_FileServerEmbeddedFrontend(t *testing.T) { + frontend := fstest.MapFS{"index.html": {Data: []byte("embedded frontend index")}} + router := routegroup.New(http.NewServeMux()) + addFileServer(router, frontend, filepath.Join(t.TempDir(), "absent"), "test-version") + + ts := httptest.NewServer(router) + defer ts.Close() + + t.Run("serves the embedded frontend", func(t *testing.T) { + body, code := get(t, ts.URL+"/web/index.html") + assert.Equal(t, http.StatusOK, code) + assert.Equal(t, "embedded frontend index", body) + }) + + for _, name := range []string{"privacy.html", "markdown-help.html", "400x400.jpeg"} { + t.Run("falls back to "+name, func(t *testing.T) { + want, err := fs.ReadFile(webassets.FS, name) + require.NoError(t, err) + + body, code := get(t, ts.URL+"/web/"+name) + assert.Equal(t, http.StatusOK, code) + assert.Equal(t, string(want), body) + }) + } + + t.Run("a name neither source has is missing", func(t *testing.T) { + _, code := get(t, ts.URL+"/web/nothing-here.html") + assert.Equal(t, http.StatusNotFound, code) + }) + + t.Run("a name the operating system rejects is missing, not an error", func(t *testing.T) { + _, code := get(t, ts.URL+"/web/a%00b.html") + assert.Equal(t, http.StatusNotFound, code) + }) +} + +// TestRest_FileServerRoutesEmbedded drives the whole router the released binary runs: no web root +// on disk, and the frontend read from WebFS. It is what pins the web/ prefix routes() strips, which +// a test calling addFileServer directly cannot see. +func TestRest_FileServerRoutesEmbedded(t *testing.T) { + frontend := fstest.MapFS{ + "web/index.html": {Data: []byte("embedded index")}, + "web/iframe.html": {Data: []byte("embedded iframe")}, + "web/remark.mjs": {Data: []byte("embedded bundle")}, + } + ts, _, teardown := startupT(t, func(srv *Rest) { + srv.WebRoot = filepath.Join(t.TempDir(), "absent") + srv.WebFS = frontend + }) + defer teardown() + + t.Run("serves the frontend from under the web prefix", func(t *testing.T) { + for name, want := range map[string]string{ + "index.html": "embedded index", + "iframe.html": "embedded iframe", + "remark.mjs": "embedded bundle", + } { + t.Run(name, func(t *testing.T) { + body, code := get(t, ts.URL+"/web/"+name) + assert.Equal(t, http.StatusOK, code) + assert.Equal(t, want, body) + }) + } + }) + + t.Run("the prefix is stripped rather than exposed", func(t *testing.T) { + _, code := get(t, ts.URL+"/web/web/index.html") + assert.Equal(t, http.StatusNotFound, code, "the web/ prefix must not be reachable as a path") + }) + + t.Run("the embedded assets still answer alongside it", func(t *testing.T) { + want, err := fs.ReadFile(webassets.FS, "privacy.html") + require.NoError(t, err) + + body, code := get(t, ts.URL+"/web/privacy.html") + assert.Equal(t, http.StatusOK, code) + assert.Equal(t, string(want), body) + }) +} + +// refusingSubFS is an fs.FS whose Sub refuses, which is the only way fs.Sub returns a nil +// filesystem. routes() has to survive it, since a nil frontend would panic on the first request. +type refusingSubFS struct{} + +func (refusingSubFS) Open(name string) (fs.File, error) { + return nil, &fs.PathError{Op: "open", Path: name, Err: fs.ErrNotExist} +} +func (refusingSubFS) Sub(string) (fs.FS, error) { return nil, errors.New("refused") } + +// TestRest_FileServerFrontendSourceRefused covers the branch where the frontend source cannot be +// sub-rooted: /web must keep serving the embedded assets rather than panicking. +func TestRest_FileServerFrontendSourceRefused(t *testing.T) { + ts, _, teardown := startupT(t, func(srv *Rest) { + srv.WebRoot = filepath.Join(t.TempDir(), "absent") + srv.WebFS = refusingSubFS{} + }) + defer teardown() + + t.Run("the embedded assets still serve", func(t *testing.T) { + want, err := fs.ReadFile(webassets.FS, "privacy.html") + require.NoError(t, err) + + body, code := get(t, ts.URL+"/web/privacy.html") + assert.Equal(t, http.StatusOK, code) + assert.Equal(t, string(want), body) + }) + + t.Run("a frontend name is missing rather than fatal", func(t *testing.T) { + _, code := get(t, ts.URL+"/web/iframe.html") + assert.Equal(t, http.StatusNotFound, code) + }) +} + // TestRest_RejectHeadOnDestructiveGET verifies that HEAD is blocked on the state-mutating // GET routes (which stdlib http.ServeMux would otherwise route to the GET handler) while // still being served for safe, read-only routes. diff --git a/backend/app/rest/api/webfiles.go b/backend/app/rest/api/webfiles.go new file mode 100644 index 00000000..d82d67f8 --- /dev/null +++ b/backend/app/rest/api/webfiles.go @@ -0,0 +1,46 @@ +package api + +import ( + "errors" + "io/fs" + "path/filepath" +) + +// webFiles serves /web from two sources: a name present in the frontend build is served from there, +// and any other name from the assets embedded in the binary. +type webFiles struct { + frontend fs.FS + embedded fs.FS +} + +// Open looks the name up in the frontend build first. Only a missing file falls through to the +// embedded assets; every other error is returned so an unreadable file keeps reporting as one +// rather than being replaced by the embedded copy or reported as missing. +func (w webFiles) Open(name string) (fs.File, error) { + // fs.ValidPath alone is not enough: it accepts names the operating system rejects, NUL among + // them, and os.DirFS turns those into fs.ErrInvalid, which renders as 500 rather than 404 + if _, err := filepath.Localize(name); err != nil || !fs.ValidPath(name) { + return nil, &fs.PathError{Op: "open", Path: name, Err: fs.ErrNotExist} + } + f, err := w.frontend.Open(name) + if err == nil { + return f, nil + } + if !errors.Is(err, fs.ErrNotExist) { + return nil, err + } + if name == "." { + // the embedded set is a flat list of files; only the frontend build answers for the + // directory itself, so a missing web root reports as missing rather than listing them + return nil, err + } + return w.embedded.Open(name) +} + +// emptyFS stands in for a frontend source that could not be opened, so a misconfigured one serves +// nothing instead of panicking or serving the build at paths it does not belong at +type emptyFS struct{} + +func (emptyFS) Open(name string) (fs.File, error) { + return nil, &fs.PathError{Op: "open", Path: name, Err: fs.ErrNotExist} +} diff --git a/backend/app/rest/api/webfiles_test.go b/backend/app/rest/api/webfiles_test.go new file mode 100644 index 00000000..f82647e7 --- /dev/null +++ b/backend/app/rest/api/webfiles_test.go @@ -0,0 +1,147 @@ +package api + +import ( + "io" + "io/fs" + "os" + "path/filepath" + "testing" + "testing/fstest" + + "github.com/umputun/remark42/backend/app/webassets" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestWebFiles_Open(t *testing.T) { + frontend := fstest.MapFS{ + "both.html": {Data: []byte("from the frontend build")}, + "only-frontend.html": {Data: []byte("frontend only")}, + } + embedded := fstest.MapFS{ + "both.html": {Data: []byte("from the embedded assets")}, + "only-embedded.html": {Data: []byte("embedded only")}, + } + w := webFiles{frontend: frontend, embedded: embedded} + + tbl := []struct { + name string + lookup string + want string + wantErr error + }{ + {name: "present in both is served from the frontend build", lookup: "both.html", want: "from the frontend build"}, + {name: "frontend only", lookup: "only-frontend.html", want: "frontend only"}, + {name: "embedded only", lookup: "only-embedded.html", want: "embedded only"}, + {name: "missing in both", lookup: "neither.html", wantErr: fs.ErrNotExist}, + } + + for _, tt := range tbl { + t.Run(tt.name, func(t *testing.T) { + f, err := w.Open(tt.lookup) + if tt.wantErr != nil { + require.Error(t, err) + assert.ErrorIs(t, err, tt.wantErr) + return + } + require.NoError(t, err) + defer f.Close() + b, err := io.ReadAll(f) + require.NoError(t, err) + assert.Equal(t, tt.want, string(b)) + }) + } +} + +// TestEmptyFS_ServesNothing pins the stand-in used when the frontend source cannot be opened: +// every name must report as missing rather than panicking, since it backs a nil-free fallback. +func TestEmptyFS_ServesNothing(t *testing.T) { + for _, name := range []string{".", "index.html", "web/index.html"} { + t.Run(name, func(t *testing.T) { + f, err := emptyFS{}.Open(name) + require.Error(t, err) + assert.ErrorIs(t, err, fs.ErrNotExist) + assert.Nil(t, f) + }) + } +} + +// TestWebFiles_EmptyFrontendFallsThrough covers the shape routes() builds when fs.Sub refuses: +// the embedded assets must still answer even though the frontend source serves nothing. +func TestWebFiles_EmptyFrontendFallsThrough(t *testing.T) { + w := webFiles{frontend: emptyFS{}, embedded: webassets.FS} + + want, err := fs.ReadFile(webassets.FS, "privacy.html") + require.NoError(t, err) + + f, err := w.Open("privacy.html") + require.NoError(t, err) + defer f.Close() + got, err := io.ReadAll(f) + require.NoError(t, err) + assert.Equal(t, string(want), string(got)) +} + +// TestWebFiles_OpenRootIsNotListed keeps the embedded assets from being browsable: they answer +// for their own names only, so a web root that has gone missing reports as missing. +func TestWebFiles_OpenRootIsNotListed(t *testing.T) { + w := webFiles{frontend: os.DirFS(filepath.Join(t.TempDir(), "absent")), embedded: webassets.FS} + + f, err := w.Open(".") + require.Error(t, err, "the embedded assets must not answer for the directory itself") + assert.ErrorIs(t, err, fs.ErrNotExist) + if err == nil { + _ = f.Close() + } + + // the assets themselves still serve + f, err = w.Open("privacy.html") + require.NoError(t, err) + require.NoError(t, f.Close()) +} + +// TestWebFiles_OpenInvalidName pins that a name fs rejects reports as missing rather than invalid. +// os.DirFS returns fs.ErrInvalid for these, which http.FileServer renders as 500, so the check has +// to happen before the lookup. A memory filesystem cannot show this: it reports missing either way. +func TestWebFiles_OpenInvalidName(t *testing.T) { + dir := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(dir, "privacy.html"), []byte("frontend"), 0o600)) + w := webFiles{frontend: os.DirFS(dir), embedded: webassets.FS} + + for _, name := range []string{"../escape.html", "/etc/passwd", "a\x00b.html", "./privacy.html"} { + t.Run(name, func(t *testing.T) { + f, err := w.Open(name) + require.Error(t, err) + assert.ErrorIs(t, err, fs.ErrNotExist) + assert.NotErrorIs(t, err, fs.ErrInvalid, "an invalid name must not surface as 500") + if err == nil { + _ = f.Close() + } + }) + } +} + +// TestWebFiles_OpenUnreadableFrontendFile pins the rule that only a missing file falls through: +// a frontend file that cannot be read must report that, not be masked by the embedded copy. +func TestWebFiles_OpenUnreadableFrontendFile(t *testing.T) { + if os.Geteuid() == 0 { + t.Skip("root ignores file permissions") + } + + dir := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(dir, "privacy.html"), []byte("operator's own"), 0o000)) + + w := webFiles{ + frontend: os.DirFS(dir), + embedded: fstest.MapFS{"privacy.html": {Data: []byte("built in")}}, + } + + f, err := w.Open("privacy.html") + require.Error(t, err, "an unreadable frontend file must not be replaced by the embedded copy") + assert.NotErrorIs(t, err, fs.ErrNotExist, "the error must stay a permission error so it does not render as 404") + assert.ErrorIs(t, err, fs.ErrPermission) + if err == nil { + _ = f.Close() + } +} diff --git a/frontend/apps/remark42/templates/400x400.jpeg b/backend/app/webassets/assets/400x400.jpeg similarity index 100% rename from frontend/apps/remark42/templates/400x400.jpeg rename to backend/app/webassets/assets/400x400.jpeg diff --git a/frontend/apps/remark42/templates/markdown-help.html b/backend/app/webassets/assets/markdown-help.html similarity index 99% rename from frontend/apps/remark42/templates/markdown-help.html rename to backend/app/webassets/assets/markdown-help.html index 0cd1053d..8672f06e 100644 --- a/frontend/apps/remark42/templates/markdown-help.html +++ b/backend/app/webassets/assets/markdown-help.html @@ -7,7 +7,6 @@