Close all 14 findings from the codebase audit (1 high, 5 medium, 8 low): - Locking: replace the global API and workspace mutexes with per-instance and per-user keyed locks (new internal/api/locks.go) so a slow restore, upgrade or transfer no longer blocks unrelated users or instances; move Docker I/O out of the build queue and invalidate concurrent image-index observations with a generation counter; serialize instance deletion behind in-flight creation. - Security: equalize login timing for unknown usernames, periodically purge expired sessions/refresh tokens/lock rows, URL-encode orphan IDs in the frontend, upgrade vitest 3.x->5.x to clear 3 npm audit vulnerabilities (2 critical RCE). - Deployment: add a -healthcheck mode probing /healthz (used by Dockerfile HEALTHCHECK and compose), make ADMIN_PASSWORD optional in compose with removal guidance after first start, exclude test scripts from the image, and cap panel resources (cpus: 2, pids_limit: 512). - Reliability: check rows.Err() on admin listing loops, surface intent polling errors without sticking, disable close during in-flight create. Verified: go build/vet, full go test -race -count=1 ./..., frontend tsc build + 32 tests, npm audit 0, compose config, real-container healthcheck.
299 lines
8.7 KiB
Go
299 lines
8.7 KiB
Go
package api
|
|
|
|
import (
|
|
"errors"
|
|
"fmt"
|
|
"io"
|
|
"net/http"
|
|
"net/http/httptest"
|
|
"os"
|
|
"path/filepath"
|
|
"strings"
|
|
"sync"
|
|
"testing"
|
|
"time"
|
|
|
|
"git.misaka.ren/m1saka/dsh-web/internal/store"
|
|
)
|
|
|
|
type pausedDownload struct {
|
|
*httptest.ResponseRecorder
|
|
started chan struct{}
|
|
release chan struct{}
|
|
once sync.Once
|
|
}
|
|
|
|
func (w *pausedDownload) Write(p []byte) (int, error) {
|
|
w.once.Do(func() { close(w.started) })
|
|
<-w.release
|
|
return w.ResponseRecorder.Write(p)
|
|
}
|
|
|
|
func TestSlowDownloadDoesNotHoldLifecycleLock(t *testing.T) {
|
|
a, sid := runtimeTestAPI(t, func(w http.ResponseWriter, r *http.Request) { t.Error("unexpected Docker call") })
|
|
for _, id := range []string{"one", "two"} {
|
|
if err := a.s.AddInstance(store.Instance{ID: id, UID: "owner"}); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
dir := filepath.Join(a.root, "workspaces/owner/instances", id, "home")
|
|
if err := os.MkdirAll(dir, 0750); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
if err := os.WriteFile(filepath.Join(dir, "file.txt"), []byte("content"), 0600); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
}
|
|
w := &pausedDownload{ResponseRecorder: httptest.NewRecorder(), started: make(chan struct{}), release: make(chan struct{})}
|
|
done := make(chan struct{})
|
|
go func() {
|
|
defer close(done)
|
|
a.workspace(w, runtimeRequest("GET", "/api/ws/download?instance=one&path=file.txt", "", sid))
|
|
}()
|
|
defer func() { close(w.release); <-done }()
|
|
select {
|
|
case <-w.started:
|
|
case <-time.After(3 * time.Second):
|
|
t.Fatal("download not started")
|
|
}
|
|
if a.instanceLocks.held() {
|
|
t.Fatal("slow network holds instance lifecycle lock")
|
|
}
|
|
a.readersMu.Lock()
|
|
active := a.workspaceReaders["one"]
|
|
a.readersMu.Unlock()
|
|
if active != 1 {
|
|
t.Fatalf("lease count %d", active)
|
|
}
|
|
other := httptest.NewRecorder()
|
|
otherDone := make(chan struct{})
|
|
go func() {
|
|
defer close(otherDone)
|
|
a.workspace(other, runtimeRequest("GET", "/api/ws/tree?instance=two", "", sid))
|
|
}()
|
|
select {
|
|
case <-otherDone:
|
|
case <-time.After(3 * time.Second):
|
|
t.Fatal("other instance blocked")
|
|
}
|
|
if other.Code != 200 {
|
|
t.Fatalf("other instance: %d", other.Code)
|
|
}
|
|
proxyDone := make(chan struct{})
|
|
proxyResponse := httptest.NewRecorder()
|
|
go func() {
|
|
defer close(proxyDone)
|
|
a.proxy(proxyResponse, runtimeRequest("GET", "/dsh/missing/", "", sid))
|
|
}()
|
|
select {
|
|
case <-proxyDone:
|
|
case <-time.After(3 * time.Second):
|
|
t.Fatal("proxy entry blocked by download")
|
|
}
|
|
if proxyResponse.Code != 404 {
|
|
t.Fatalf("proxy status %d", proxyResponse.Code)
|
|
}
|
|
for _, action := range []string{"restore", "upgrade", "stop", ""} {
|
|
method := "POST"
|
|
if action == "" {
|
|
method = "DELETE"
|
|
}
|
|
response := httptest.NewRecorder()
|
|
url := "/api/instances/one"
|
|
if action != "" {
|
|
url += "/" + action
|
|
}
|
|
body := `{}`
|
|
if action == "upgrade" {
|
|
body = `{"image_id":"sha256:` + strings.Repeat("a", 64) + `"}`
|
|
}
|
|
a.instance(response, runtimeRequest(method, url, body, sid))
|
|
if response.Code != 409 {
|
|
t.Errorf("%s during download: %d %s", action, response.Code, response.Body.String())
|
|
}
|
|
}
|
|
}
|
|
|
|
func TestDirectoryEntryLimit(t *testing.T) {
|
|
dir := t.TempDir()
|
|
for i := 0; i <= maxDirectoryEntries; i++ {
|
|
f, err := os.Create(filepath.Join(dir, fmt.Sprint(i)))
|
|
if err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
f.Close()
|
|
}
|
|
root, err := os.OpenRoot(dir)
|
|
if err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
defer root.Close()
|
|
if _, err = readRootDir(root, "."); !errors.Is(err, errDirectoryLimit) {
|
|
t.Fatalf("limit: %v", err)
|
|
}
|
|
if err = os.Remove(filepath.Join(dir, fmt.Sprint(maxDirectoryEntries))); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
entries, err := readRootDir(root, ".")
|
|
if err != nil || len(entries) != maxDirectoryEntries {
|
|
t.Fatalf("boundary: %d %v", len(entries), err)
|
|
}
|
|
}
|
|
|
|
type pausedUpload struct {
|
|
started chan struct{}
|
|
release chan struct{}
|
|
once sync.Once
|
|
}
|
|
|
|
func (r *pausedUpload) Read(p []byte) (int, error) {
|
|
r.once.Do(func() { close(r.started) })
|
|
<-r.release
|
|
return 0, io.EOF
|
|
}
|
|
func (r *pausedUpload) Close() error { return nil }
|
|
|
|
func TestSlowUploadDoesNotHoldLifecycleLock(t *testing.T) {
|
|
a, sid := runtimeTestAPI(t, func(w http.ResponseWriter, r *http.Request) { t.Error("unexpected Docker call") })
|
|
if err := a.s.AddInstance(store.Instance{ID: "one", UID: "owner"}); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
if _, err := a.s.DB.Exec("UPDATE users SET quota=? WHERE id='owner'", 1<<30); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
if err := os.MkdirAll(filepath.Join(a.root, "workspaces/owner/instances/one/home"), 0750); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
body := &pausedUpload{started: make(chan struct{}), release: make(chan struct{})}
|
|
r := runtimeRequest("POST", "/api/ws/upload?instance=one", "", sid)
|
|
r.Body = body
|
|
r.Header.Set("Content-Type", "multipart/form-data; boundary=test")
|
|
done := make(chan struct{})
|
|
go func() { defer close(done); a.workspace(httptest.NewRecorder(), r) }()
|
|
defer func() { close(body.release); <-done }()
|
|
select {
|
|
case <-body.started:
|
|
case <-time.After(3 * time.Second):
|
|
t.Fatal("upload not reading body")
|
|
}
|
|
if a.instanceLocks.held() {
|
|
t.Fatal("slow upload holds instance lock")
|
|
}
|
|
if a.userLocks.held() {
|
|
t.Fatal("slow upload holds user write lock")
|
|
}
|
|
}
|
|
|
|
func TestWorkspaceErrorResponseReleasesLocks(t *testing.T) {
|
|
a, sid := runtimeTestAPI(t, func(w http.ResponseWriter, r *http.Request) { t.Error("unexpected Docker call") })
|
|
if err := a.s.AddInstance(store.Instance{ID: "one", UID: "owner"}); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
if err := os.MkdirAll(filepath.Join(a.root, "workspaces/owner/instances/one/home"), 0750); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
w := &pausedDownload{ResponseRecorder: httptest.NewRecorder(), started: make(chan struct{}), release: make(chan struct{})}
|
|
done := make(chan struct{})
|
|
go func() {
|
|
defer close(done)
|
|
a.workspace(w, runtimeRequest("GET", "/api/ws/tree?instance=one&path=missing", "", sid))
|
|
}()
|
|
defer func() { close(w.release); <-done }()
|
|
select {
|
|
case <-w.started:
|
|
case <-time.After(3 * time.Second):
|
|
t.Fatal("response not started")
|
|
}
|
|
if a.userLocks.held() {
|
|
t.Fatal("error response holds user write lock")
|
|
}
|
|
if a.instanceLocks.held() {
|
|
t.Fatal("error response holds instance lock")
|
|
}
|
|
a.readersMu.Lock()
|
|
leases := len(a.workspaceReaders)
|
|
a.readersMu.Unlock()
|
|
if leases != 0 {
|
|
t.Fatal("response retains lease")
|
|
}
|
|
}
|
|
|
|
// A restore or upgrade holds its instance and user locks for minutes; that
|
|
// must never block file operations or lifecycle actions of other users.
|
|
func TestLongMutationDoesNotBlockOtherUsers(t *testing.T) {
|
|
a, _ := runtimeTestAPI(t, func(w http.ResponseWriter, r *http.Request) {
|
|
switch {
|
|
case r.Method == "POST" && strings.HasSuffix(r.URL.Path, "/stop"):
|
|
w.WriteHeader(http.StatusNoContent)
|
|
case r.Method == "GET" && strings.HasSuffix(r.URL.Path, "/json"):
|
|
writeJSON(w, map[string]any{"Id": "cid-two", "State": map[string]any{"Status": "exited", "Running": false}})
|
|
default:
|
|
t.Errorf("unexpected Docker call %s %s", r.Method, r.URL.Path)
|
|
}
|
|
})
|
|
if err := a.s.CreateUser(store.User{ID: "other", Username: "other"}, "unused"); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
otherSid, _, err := a.s.Session("other")
|
|
if err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
registerTestAccess(t, a, otherSid)
|
|
if err := a.s.AddInstance(store.Instance{ID: "one", UID: "owner"}); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
if err := a.s.AddInstance(store.Instance{ID: "two", UID: "other", ContainerID: "cid-two", Status: "running", Created: time.Now()}); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
for _, dir := range []string{"workspaces/owner/instances/one/home", "workspaces/other/instances/two/home"} {
|
|
if err := os.MkdirAll(filepath.Join(a.root, dir), 0750); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
}
|
|
// Simulate a mutation paused mid-flight (restore/upgrade state).
|
|
inst := a.instanceLocks.Acquire("one")
|
|
defer inst.Release()
|
|
user := a.userLocks.Acquire("owner")
|
|
defer user.Release()
|
|
|
|
treeDone := make(chan *httptest.ResponseRecorder, 1)
|
|
go func() {
|
|
w := httptest.NewRecorder()
|
|
a.workspace(w, runtimeRequest("GET", "/api/ws/tree?instance=two", "", otherSid))
|
|
treeDone <- w
|
|
}()
|
|
select {
|
|
case w := <-treeDone:
|
|
if w.Code != 200 {
|
|
t.Fatalf("other user tree: %d %s", w.Code, w.Body.String())
|
|
}
|
|
case <-time.After(3 * time.Second):
|
|
t.Fatal("other user file operation blocked by long mutation")
|
|
}
|
|
stopDone := make(chan *httptest.ResponseRecorder, 1)
|
|
go func() {
|
|
w := httptest.NewRecorder()
|
|
a.instance(w, runtimeRequest("POST", "/api/instances/two/stop", "", otherSid))
|
|
stopDone <- w
|
|
}()
|
|
select {
|
|
case w := <-stopDone:
|
|
if w.Code != 200 {
|
|
t.Fatalf("other instance stop: %d %s", w.Code, w.Body.String())
|
|
}
|
|
case <-time.After(3 * time.Second):
|
|
t.Fatal("other instance lifecycle blocked by long mutation")
|
|
}
|
|
// The same user's mutation is still serialized by the user lock.
|
|
blocked := make(chan struct{})
|
|
go func() {
|
|
user := a.userLocks.Acquire("owner")
|
|
user.Release()
|
|
close(blocked)
|
|
}()
|
|
select {
|
|
case <-blocked:
|
|
t.Fatal("owner lock not held during mutation")
|
|
case <-time.After(300 * time.Millisecond):
|
|
}
|
|
}
|