mirror of https://github.com/coder/coder.git
feat: disable directory listings for static files (#12229)
* feat: disable directory listings for static files Static file server handles serving static asset files (js, css, etc). The default file server would also list all files in a directory. This has been changed to only serve files.
This commit is contained in:
parent
2dac34276a
commit
07cccf9033
|
@ -1067,6 +1067,14 @@ func New(options *Options) *API {
|
||||||
// See globalHTTPSwaggerHandler comment as to why we use a package
|
// See globalHTTPSwaggerHandler comment as to why we use a package
|
||||||
// global variable here.
|
// global variable here.
|
||||||
r.Get("/swagger/*", globalHTTPSwaggerHandler)
|
r.Get("/swagger/*", globalHTTPSwaggerHandler)
|
||||||
|
} else {
|
||||||
|
swaggerDisabled := http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
|
||||||
|
httpapi.Write(context.Background(), rw, http.StatusNotFound, codersdk.Response{
|
||||||
|
Message: "Swagger documentation is disabled.",
|
||||||
|
})
|
||||||
|
})
|
||||||
|
r.Get("/swagger", swaggerDisabled)
|
||||||
|
r.Get("/swagger/*", swaggerDisabled)
|
||||||
}
|
}
|
||||||
|
|
||||||
// Add CSP headers to all static assets and pages. CSP headers only affect
|
// Add CSP headers to all static assets and pages. CSP headers only affect
|
||||||
|
|
|
@ -312,12 +312,9 @@ func TestSwagger(t *testing.T) {
|
||||||
|
|
||||||
resp, err := requestWithRetries(ctx, t, client, http.MethodGet, swaggerEndpoint, nil)
|
resp, err := requestWithRetries(ctx, t, client, http.MethodGet, swaggerEndpoint, nil)
|
||||||
require.NoError(t, err)
|
require.NoError(t, err)
|
||||||
|
|
||||||
body, err := io.ReadAll(resp.Body)
|
|
||||||
require.NoError(t, err)
|
|
||||||
defer resp.Body.Close()
|
defer resp.Body.Close()
|
||||||
|
|
||||||
require.Equal(t, "<pre>\n</pre>\n", string(body))
|
require.Equal(t, http.StatusNotFound, resp.StatusCode)
|
||||||
})
|
})
|
||||||
t.Run("doc.json disabled by default", func(t *testing.T) {
|
t.Run("doc.json disabled by default", func(t *testing.T) {
|
||||||
t.Parallel()
|
t.Parallel()
|
||||||
|
@ -329,12 +326,9 @@ func TestSwagger(t *testing.T) {
|
||||||
|
|
||||||
resp, err := requestWithRetries(ctx, t, client, http.MethodGet, swaggerEndpoint+"/doc.json", nil)
|
resp, err := requestWithRetries(ctx, t, client, http.MethodGet, swaggerEndpoint+"/doc.json", nil)
|
||||||
require.NoError(t, err)
|
require.NoError(t, err)
|
||||||
|
|
||||||
body, err := io.ReadAll(resp.Body)
|
|
||||||
require.NoError(t, err)
|
|
||||||
defer resp.Body.Close()
|
defer resp.Body.Close()
|
||||||
|
|
||||||
require.Equal(t, "<pre>\n</pre>\n", string(body))
|
require.Equal(t, http.StatusNotFound, resp.StatusCode)
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
45
site/site.go
45
site/site.go
|
@ -102,7 +102,8 @@ func New(opts *Options) *Handler {
|
||||||
// Set ETag header to the SHA1 hash of the file contents.
|
// Set ETag header to the SHA1 hash of the file contents.
|
||||||
name := filePath(r.URL.Path)
|
name := filePath(r.URL.Path)
|
||||||
if name == "" || name == "/" {
|
if name == "" || name == "/" {
|
||||||
// Serve the directory listing.
|
// Serve the directory listing. This intentionally allows directory listings to
|
||||||
|
// be served. This file system should not contain anything sensitive.
|
||||||
http.FileServer(opts.BinFS).ServeHTTP(rw, r)
|
http.FileServer(opts.BinFS).ServeHTTP(rw, r)
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
@ -129,7 +130,15 @@ func New(opts *Options) *Handler {
|
||||||
// If-Match and If-None-Match headers on the request properly.
|
// If-Match and If-None-Match headers on the request properly.
|
||||||
http.FileServer(opts.BinFS).ServeHTTP(rw, r)
|
http.FileServer(opts.BinFS).ServeHTTP(rw, r)
|
||||||
})))
|
})))
|
||||||
mux.Handle("/", http.FileServer(http.FS(opts.SiteFS)))
|
mux.Handle("/", http.FileServer(
|
||||||
|
http.FS(
|
||||||
|
// OnlyFiles is a wrapper around the file system that prevents directory
|
||||||
|
// listings. Directory listings are not required for the site file system, so we
|
||||||
|
// exclude it as a security measure. In practice, this file system comes from our
|
||||||
|
// open source code base, but this is considered a best practice for serving
|
||||||
|
// static files.
|
||||||
|
OnlyFiles(opts.SiteFS))),
|
||||||
|
)
|
||||||
|
|
||||||
buildInfo := codersdk.BuildInfoResponse{
|
buildInfo := codersdk.BuildInfoResponse{
|
||||||
ExternalURL: buildinfo.ExternalURL(),
|
ExternalURL: buildinfo.ExternalURL(),
|
||||||
|
@ -873,3 +882,35 @@ func applicationNameOrDefault(cfg codersdk.AppearanceConfig) string {
|
||||||
}
|
}
|
||||||
return "Coder"
|
return "Coder"
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// OnlyFiles returns a new fs.FS that only contains files. If a directory is
|
||||||
|
// requested, os.ErrNotExist is returned. This prevents directory listings from
|
||||||
|
// being served.
|
||||||
|
func OnlyFiles(files fs.FS) fs.FS {
|
||||||
|
return justFilesSystem{FS: files}
|
||||||
|
}
|
||||||
|
|
||||||
|
type justFilesSystem struct {
|
||||||
|
FS fs.FS
|
||||||
|
}
|
||||||
|
|
||||||
|
func (jfs justFilesSystem) Open(name string) (fs.File, error) {
|
||||||
|
f, err := jfs.FS.Open(name)
|
||||||
|
if err != nil {
|
||||||
|
return nil, err
|
||||||
|
}
|
||||||
|
|
||||||
|
stat, err := f.Stat()
|
||||||
|
if err != nil {
|
||||||
|
return nil, err
|
||||||
|
}
|
||||||
|
|
||||||
|
// Returning a 404 here does prevent the http.FileServer from serving
|
||||||
|
// index.* files automatically. Coder handles this above as all index pages
|
||||||
|
// are considered template files. So we never relied on this behavior.
|
||||||
|
if stat.IsDir() {
|
||||||
|
return nil, os.ErrNotExist
|
||||||
|
}
|
||||||
|
|
||||||
|
return f, nil
|
||||||
|
}
|
||||||
|
|
|
@ -4,12 +4,19 @@
|
||||||
package site
|
package site
|
||||||
|
|
||||||
import (
|
import (
|
||||||
"embed"
|
|
||||||
"io/fs"
|
"io/fs"
|
||||||
|
"testing/fstest"
|
||||||
|
"time"
|
||||||
)
|
)
|
||||||
|
|
||||||
var slim embed.FS
|
|
||||||
|
|
||||||
func FS() fs.FS {
|
func FS() fs.FS {
|
||||||
return slim
|
// This is required to contain an index.html file for unit tests.
|
||||||
|
// Our unit tests frequently just hit `/` and expect to get a 200.
|
||||||
|
// So a valid index.html file should be expected to be served.
|
||||||
|
return fstest.MapFS{
|
||||||
|
"index.html": &fstest.MapFile{
|
||||||
|
Data: []byte("Slim build of Coder, does not contain the frontend static files."),
|
||||||
|
ModTime: time.Now(),
|
||||||
|
},
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
|
@ -18,6 +18,7 @@ import (
|
||||||
"testing/fstest"
|
"testing/fstest"
|
||||||
"time"
|
"time"
|
||||||
|
|
||||||
|
"github.com/go-chi/chi/v5"
|
||||||
"github.com/google/uuid"
|
"github.com/google/uuid"
|
||||||
"github.com/stretchr/testify/assert"
|
"github.com/stretchr/testify/assert"
|
||||||
"github.com/stretchr/testify/require"
|
"github.com/stretchr/testify/require"
|
||||||
|
@ -659,3 +660,29 @@ func TestRenderStaticErrorPageNoStatus(t *testing.T) {
|
||||||
require.Contains(t, bodyStr, "Retry")
|
require.Contains(t, bodyStr, "Retry")
|
||||||
require.Contains(t, bodyStr, d.DashboardURL)
|
require.Contains(t, bodyStr, d.DashboardURL)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestJustFilesSystem(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
tfs := fstest.MapFS{
|
||||||
|
"dir/foo.txt": &fstest.MapFile{
|
||||||
|
Data: []byte("hello world"),
|
||||||
|
},
|
||||||
|
"dir/bar.txt": &fstest.MapFile{
|
||||||
|
Data: []byte("hello world"),
|
||||||
|
},
|
||||||
|
}
|
||||||
|
|
||||||
|
mux := chi.NewRouter()
|
||||||
|
mux.Mount("/onlyfiles/", http.StripPrefix("/onlyfiles", http.FileServer(http.FS(site.OnlyFiles(tfs)))))
|
||||||
|
mux.Mount("/all/", http.StripPrefix("/all", http.FileServer(http.FS(tfs))))
|
||||||
|
|
||||||
|
// The /all/ endpoint should serve the directory listing.
|
||||||
|
resp := httptest.NewRecorder()
|
||||||
|
mux.ServeHTTP(resp, httptest.NewRequest("GET", "/all/dir/", nil))
|
||||||
|
require.Equal(t, http.StatusOK, resp.Code, "all serves the directory")
|
||||||
|
|
||||||
|
resp = httptest.NewRecorder()
|
||||||
|
mux.ServeHTTP(resp, httptest.NewRequest("GET", "/onlyfiles/dir/", nil))
|
||||||
|
require.Equal(t, http.StatusNotFound, resp.Code, "onlyfiles does not serve the directory")
|
||||||
|
}
|
||||||
|
|
Loading…
Reference in New Issue