aboutsummaryrefslogtreecommitdiff
path: root/internal/admin
diff options
context:
space:
mode:
Diffstat (limited to 'internal/admin')
-rw-r--r--internal/admin/api_entity.go4
-rw-r--r--internal/admin/api_files.go30
-rw-r--r--internal/admin/api_helpers.go7
-rw-r--r--internal/admin/api_room_courses.go25
-rw-r--r--internal/admin/correct_audit_fixes_test.go31
-rw-r--r--internal/admin/live_owner_test.go1
6 files changed, 78 insertions, 20 deletions
diff --git a/internal/admin/api_entity.go b/internal/admin/api_entity.go
index 204c59e..04b9dca 100644
--- a/internal/admin/api_entity.go
+++ b/internal/admin/api_entity.go
@@ -230,6 +230,10 @@ func (s *AdminServer) entityCreate(w http.ResponseWriter, r *http.Request, c ent
c.fail(w, http.StatusBadRequest, c.invalidID())
return
}
+ // Editor-only keys never belong in the file (the update path drops them
+ // too); without this a GET->POST round trip poisons the YAML.
+ delete(m, "_path")
+ delete(m, "_raw")
if _, _, err := c.find(s, id); err == nil {
c.fail(w, http.StatusConflict, c.noun+" already exists")
return
diff --git a/internal/admin/api_files.go b/internal/admin/api_files.go
index a19cb3b..b1b3fff 100644
--- a/internal/admin/api_files.go
+++ b/internal/admin/api_files.go
@@ -234,8 +234,13 @@ func (s *AdminServer) handleFiles(w http.ResponseWriter, r *http.Request) {
return
}
// A redacted account hash is restored from the file on disk before
- // writing, so the raw editor can never disclose or overwrite it.
- body = restoreAccountSecret(relPath, fullPath, body)
+ // writing, so the raw editor can never disclose or overwrite it. A
+ // non-mapping body is refused rather than blanking the hash.
+ body, err = restoreAccountSecret(relPath, fullPath, body)
+ if err != nil {
+ failJSON(w, err.Error())
+ return
+ }
oldContent, existed, err := snapshotBefore(fullPath)
if err != nil {
@@ -375,31 +380,34 @@ func redactAccountContent(relPath string, data []byte) []byte {
// restoreAccountSecret copies the on-disk password hash back into a submitted
// account file, so a redacted value (or any client-supplied value) can never
-// change or blank an existing hash through the raw file editor.
-func restoreAccountSecret(relPath, fullPath string, body []byte) []byte {
+// change or blank an existing hash through the raw file editor. It returns an
+// error when the existing hash cannot be preserved (for example a non-mapping
+// body), so the caller refuses the write instead of blanking the hash.
+func restoreAccountSecret(relPath, fullPath string, body []byte) ([]byte, error) {
if !isAccountFilePath(relPath) {
- return body
+ return body, nil
}
existing, err := os.ReadFile(fullPath)
if err != nil {
- return body
+ // A missing account file is a create; nothing to preserve.
+ return body, nil
}
em, err := yamlToMap(existing)
if err != nil {
- return body
+ return nil, fmt.Errorf("existing account file is not a YAML mapping: %w", err)
}
hash, ok := em["password_hash"]
if !ok {
- return body
+ return body, nil
}
bm, err := yamlToMap(body)
if err != nil {
- return body
+ return nil, fmt.Errorf("submitted account file is not a YAML mapping; refusing to blank the password hash")
}
bm["password_hash"] = hash
out, err := marshalMapYAML(bm)
if err != nil {
- return body
+ return nil, fmt.Errorf("marshal account file: %w", err)
}
- return out
+ return out, nil
}
diff --git a/internal/admin/api_helpers.go b/internal/admin/api_helpers.go
index 37cc4ae..d5e23c3 100644
--- a/internal/admin/api_helpers.go
+++ b/internal/admin/api_helpers.go
@@ -17,12 +17,17 @@ import (
// JSON bodies, so an unchecked join could otherwise escape the data
// directory ("../") or touch dot files.
func validEntityID(id string) bool {
- if id == "" || id == "." || id == ".." {
+ if id == "" || id == "." || id == ".." || strings.HasPrefix(id, ".") {
return false
}
if strings.ContainsAny(id, `/\`) || strings.ContainsRune(id, 0) {
return false
}
+ for _, r := range id {
+ if r < 32 || r == 127 {
+ return false
+ }
+ }
return true
}
diff --git a/internal/admin/api_room_courses.go b/internal/admin/api_room_courses.go
index db74adf..099ebef 100644
--- a/internal/admin/api_room_courses.go
+++ b/internal/admin/api_room_courses.go
@@ -420,8 +420,9 @@ func (s *AdminServer) handleRoomAttachCourse(w http.ResponseWriter, r *http.Requ
// Detach a room from its course; delete the course file if it becomes empty
-// detachRoomFromCourse removes roomID from the course it belongs to, if any.
-// Returns the course ID that was modified, or "" if none match.
+// detachRoomFromCourse removes roomID from every course it belongs to, if any.
+// Returns the first affected course ID (or "" if none match) and whether any
+// course was deleted.
func (s *AdminServer) detachRoomFromCourse(roomID int) (courseID string, deleted bool) {
ids, err := s.listCourseIDs()
if err != nil {
@@ -448,15 +449,23 @@ func (s *AdminServer) detachRoomFromCourse(roomID int) (courseID string, deleted
remaining = append(remaining, obstacles[idx+1:]...)
if len(remaining) == 0 {
- os.Remove(path)
+ // A failed unlink must not be reported as a detach: the course
+ // would keep listing a deleted room.
+ if err := os.Remove(path); err != nil {
+ continue
+ }
s.undoStack.Push(ChangeDesc{
Description: fmt.Sprintf("delete course %s (last obstacle removed)", id),
FilePath: path,
OldContent: oldContent,
IsDelete: true,
})
+ if courseID == "" {
+ courseID = id
+ }
+ deleted = true
s.reloadCoursesIfWired()
- return id, true
+ continue
}
anyList := make([]any, len(remaining))
@@ -466,7 +475,7 @@ func (s *AdminServer) detachRoomFromCourse(roomID int) (courseID string, deleted
m["obstacles"] = anyList
newContent, err := writeMapAsYAML(path, m)
if err != nil {
- return "", false
+ continue
}
s.undoStack.Push(ChangeDesc{
Description: fmt.Sprintf("detach room %d from course %s", roomID, id),
@@ -474,10 +483,12 @@ func (s *AdminServer) detachRoomFromCourse(roomID int) (courseID string, deleted
OldContent: oldContent,
NewContent: newContent,
})
+ if courseID == "" {
+ courseID = id
+ }
s.reloadCoursesIfWired()
- return id, false
}
- return "", false
+ return courseID, deleted
}
func (s *AdminServer) handleRoomDetachCourse(w http.ResponseWriter, r *http.Request, roomID int) {
diff --git a/internal/admin/correct_audit_fixes_test.go b/internal/admin/correct_audit_fixes_test.go
index 381fb5c..60073f7 100644
--- a/internal/admin/correct_audit_fixes_test.go
+++ b/internal/admin/correct_audit_fixes_test.go
@@ -1,6 +1,11 @@
package admin
-import "testing"
+import (
+ "os"
+ "path/filepath"
+ "strings"
+ "testing"
+)
// Regression: the raw file API must canonicalize repeated "/" and "data/"
// prefixes, or "//players/..." reaches the real file while the classifiers
@@ -20,3 +25,27 @@ func TestCanonicalDataRelPathStripsRepeatedPrefixes(t *testing.T) {
}
}
}
+
+// Regression: a valid-but-non-mapping body used to pass through unchanged and
+// blank password_hash. It must now be refused, and a mapping body must keep
+// the on-disk hash.
+func TestRestoreAccountSecretRefusesNonMappingBody(t *testing.T) {
+ dir := t.TempDir()
+ full := filepath.Join(dir, "players", "accounts", "alice.yaml")
+ if err := os.MkdirAll(filepath.Dir(full), 0755); err != nil {
+ t.Fatal(err)
+ }
+ if err := os.WriteFile(full, []byte("name: alice\npassword_hash: secret\n"), 0600); err != nil {
+ t.Fatal(err)
+ }
+ if _, err := restoreAccountSecret("players/accounts/alice.yaml", full, []byte("# comment only\n")); err == nil {
+ t.Fatal("non-mapping body accepted; the password hash would be blanked")
+ }
+ out, err := restoreAccountSecret("players/accounts/alice.yaml", full, []byte("name: alice\npassword_hash: \"[redacted]\"\n"))
+ if err != nil {
+ t.Fatal(err)
+ }
+ if !strings.Contains(string(out), "secret") {
+ t.Fatalf("password hash not preserved: %s", out)
+ }
+}
diff --git a/internal/admin/live_owner_test.go b/internal/admin/live_owner_test.go
index ae53606..de3ade8 100644
--- a/internal/admin/live_owner_test.go
+++ b/internal/admin/live_owner_test.go
@@ -178,6 +178,7 @@ func (discardConn) ReadMessage() (string, error) { return "", io.EOF }
func (discardConn) Write(p []byte) (int, error) { return len(p), nil }
func (discardConn) Close() error { return nil }
func (discardConn) SetWriteDeadline(time.Time) error { return nil }
+func (discardConn) SetReadDeadline(time.Time) error { return nil }
func (discardConn) SetEcho(bool) error { return nil }
func newEditorHarness(t *testing.T) (*editorHarness, *gamenet.Session) {
generated by cgit v1.2.3 (git 2.25.1) at 2026-10-24 13:01:58 +0000