Files
OwnCord/Server/api/avatar_handler_test.go
T
Claude 7a4e5dc357 refactor: rename the Go module to github.com/J3vb/OwnCord/Server (RL-13)
`Server/go.mod` declared `github.com/owncord/server` while the public
repository is `github.com/J3vb/OwnCord`. Nothing resolves that path — there is
no `owncord` GitHub org and no vanity-import host serving go-import metadata
for it — so every import line in the tree named a location that does not
exist. It compiles because a main module's own path is never fetched, which is
exactly why it went unnoticed.

The obvious fix — an AST-aware import rewriter (`gomvpkg`, `go mod edit`) —
is wrong here, and provably so. Six of the 722 occurrences are not imports at
all: `api/main_test.go:20` (a goleak `IgnoreTopFunction` pattern),
`telemetry/metrics.go:17-19` (three OTel instrumentation-scope names),
`invariants/syncutil_locks.go:73` (a diagnostic message), and
`invariants/syncutil_locks_test.go:56` (an import line inside a raw-string Go
fixture). An import rewriter touches none of them, and the compiler cannot
see any of them either.

Done as one scripted substitution over `git ls-files`, anchored on the full
`github.com/owncord/server` string. The anchor matters: `owncord-server` is a
different identifier — the OTel `service.name` (`config/config.go`,
`telemetry/telemetry_otel.go`) and the GHCR image name
(`.github/workflows/release.yml`, `docker-compose.yml`) — and a looser pattern
would have moved it. It is untouched: 10 occurrences across 9 files, before
and after.

350 files, 728 insertions, 728 deletions. 722 occurrences in 344 Go files,
plus `go.mod:1`, the `sed` at `Makefile:67`, `Server/CLAUDE.md:3`,
`docs/architecture/server.md:5`, and the ledger pair
(`findings-ledger.json:3758` plus a `render-ledger.mjs` re-render of
`FINDINGS.md`). Zero in any workflow, zero in the Dockerfile, zero in
`Server/.golangci.yml` (no `local-prefixes`, `gci`, `importas` or `depguard`
rule keys on the module path, so import grouping is not configured anywhere).

The plan's blast-radius estimate missed one thing, and it is the one that
would have gone red: **gofmt**. `J` (0x4A) sorts before every lowercase
letter, so in the 36 files where a module-local import shares a contiguous
group with a third-party one, the module's imports must move above
`github.com/go-chi/...`. `gofmt -l` was clean before the substitution and
listed exactly 36 files after it; `gofmt -w` on those 36 restores it to
clean. `gofmt` is an enforced gate — the `formatters` block in
`Server/.golangci.yml`, which is S-05 — so a substitution-only commit fails
Lint.

Verified: both directions, and the line accounting is exact. Every added line
in this diff contains the new module path (728) and every removed line
contains the old one (728); the count of changed lines containing neither is
**zero**, so the gofmt re-sort moved module-path lines only and touched no
third-party import. The residual check
(`git ls-files -z | xargs -0 grep -n 'github\.com/owncord/server'`) returns
exactly two hits, both deliberately out of scope: the RL-13 row in
`docs/audit-2026-08-23-repository-layout.md` and the measurement row in this
phase's own plan. The compiler-invisible half was proven by reverting *only*
`api/main_test.go:20` to the old path on the otherwise-renamed tree:
`go build ./...` and `go vet ./api/` both still pass — they see nothing wrong
— while `go test ./api/` FAILS, because the runtime function name now carries
the new path and goleak stops ignoring `ws.(*Hub).Run.func1`. Restoring the
line makes it pass. `go.sum` is byte-identical (no `go mod tidy` was run and
none was needed). All four build-tag variants compile; `go vet ./...`,
`go vet -tags otel,wazero ./...` and `go vet -tags deadlock ./...` pass;
`go test -race ./...` is 16/16 packages green; `go test -tags deadlock ./...`
passes; the tag-gated `./plugin/...` (wazero) and `./telemetry/...` (otel)
runs pass. `golangci-lint` v2.11.3 — the pinned CI version, rebuilt locally
against Go 1.26 because the packaged binary cannot load a 1.26 config —
reports **0 issues**. `go run ./cmd/genprotocol` leaves
`git diff --exit-code ws/message_types.go ../Client/src/lib/protocolTypes.ts`
clean, so the rename does not reach the generated protocol constants.
`npx prettier --check .` and `node .superpowers/render-ledger.mjs --check`
pass.

Not included: `docs/audit-2026-08-23-repository-layout.md` and
`docs/plans/b1-repository-foundation-2026-08-25.md` keep the old path — they
are the audit row and the measurement that motivated this change, and
rewriting them would erase the record of what was measured. They are why the
residual check needs a two-path allowance rather than being empty; that
allowance is stated above rather than hidden in a pathspec.
`telemetry/metrics.go:19` declares `scopeVoice` for a `Server/voice` package
that does not exist; the substitution carried the dead path forward verbatim
as `github.com/J3vb/OwnCord/Server/voice` rather than fixing it, because
correcting a real observability bug inside a mechanical rename would hide it
in a 350-file diff. It needs its own item. No `go.work`, no second module,
and no vanity-import host was set up — the new path resolves against the real
repository, but nothing imports this module as a library, so `go get`
reachability was not exercised either way.

Refs RL-13, L-12
2026-08-26 20:23:49 +00:00

372 lines
14 KiB
Go

package api_test
import (
"bytes"
"context"
"encoding/json"
"net/http"
"net/http/httptest"
"os"
"strings"
"testing"
"github.com/J3vb/OwnCord/Server/api"
"github.com/J3vb/OwnCord/Server/auth"
"github.com/J3vb/OwnCord/Server/db"
"github.com/J3vb/OwnCord/Server/service"
"github.com/J3vb/OwnCord/Server/storage"
"github.com/J3vb/OwnCord/Server/ws"
"github.com/go-chi/chi/v5"
)
// Avatar upload. The interesting properties are the ones a URL-only avatar
// field never had to answer: what the server accepts as an image, where the
// bytes end up, and — the one that would otherwise be a privacy bug — who is
// allowed to fetch them back.
// buildAvatarRouter mounts the profile routes (with storage, so the upload
// route exists) and the upload routes (so the file can be fetched back)
// against one database.
func buildAvatarRouter(database *db.DB, store *storage.Storage) http.Handler {
r := chi.NewRouter()
limiter := auth.NewRateLimiter()
svc := service.New(database, limiter)
api.MountProfileRoutes(r, database, svc, store, limiter, nil, nil)
api.MountUploadRoutes(r, database, store, limiter, nil, svc.Permissions)
return r
}
func doAvatarUpload(t *testing.T, router http.Handler, token, filename string, content []byte) *httptest.ResponseRecorder {
t.Helper()
body, contentType := makeMultipartFile(t, "file", filename, content)
req := httptest.NewRequest(http.MethodPost, "/api/v1/users/me/avatar", body)
req.Header.Set("Content-Type", contentType)
if token != "" {
req.Header.Set("Authorization", "Bearer "+token)
}
req.RemoteAddr = "127.0.0.1:9999"
rr := httptest.NewRecorder()
router.ServeHTTP(rr, req)
return rr
}
func TestUploadAvatar_StoresAndSetsAvatarURL(t *testing.T) {
database := newUploadTestDB(t)
store := newUploadTestStorage(t)
router := buildAvatarRouter(database, store)
token := uploadCreateToken(t, database, "pfp_owner", 4)
rr := doAvatarUpload(t, router, token, "me.png", makePNGBytes(t, 64, 64))
if rr.Code != http.StatusCreated {
t.Fatalf("status = %d, want 201; body = %s", rr.Code, rr.Body.String())
}
var resp map[string]any
if err := json.NewDecoder(rr.Body).Decode(&resp); err != nil {
t.Fatalf("decode: %v", err)
}
id, _ := resp["id"].(string)
if id == "" {
t.Fatal("response carried no file id")
}
if resp["url"] != service.AvatarFileURL(id) {
t.Errorf("url = %v, want %q", resp["url"], service.AvatarFileURL(id))
}
if resp["mime"] != "image/png" {
t.Errorf("mime = %v, want image/png", resp["mime"])
}
// The column must now point at the served file — that is what makes the
// avatar both renderable and readable.
user, err := database.GetUserByUsername(context.Background(), "pfp_owner")
if err != nil || user == nil {
t.Fatalf("GetUserByUsername: %v", err)
}
if user.Avatar == nil || *user.Avatar != service.AvatarFileURL(id) {
t.Fatalf("stored avatar = %v, want %q", user.Avatar, service.AvatarFileURL(id))
}
}
func TestUploadAvatar_IsReadableByOtherUsersWhileInUse(t *testing.T) {
database := newUploadTestDB(t)
store := newUploadTestStorage(t)
router := buildAvatarRouter(database, store)
ownerToken := uploadCreateToken(t, database, "avatar_owner", 4)
otherToken := uploadCreateToken(t, database, "avatar_peer", 4)
rr := doAvatarUpload(t, router, ownerToken, "me.png", makePNGBytes(t, 32, 32))
if rr.Code != http.StatusCreated {
t.Fatalf("upload status = %d; body = %s", rr.Code, rr.Body.String())
}
var resp map[string]any
_ = json.NewDecoder(rr.Body).Decode(&resp)
id, _ := resp["id"].(string)
// An unlinked attachment is normally uploader-only. An avatar has to be
// visible to the people who see the messages it sits beside.
if got := doServeFile(t, router, id, otherToken, nil); got.Code != http.StatusOK {
t.Fatalf("peer fetch status = %d, want 200; body = %s", got.Code, got.Body.String())
}
// Replacing the avatar revokes that: the old file goes back to being a
// private unlinked attachment.
rr2 := doAvatarUpload(t, router, ownerToken, "me2.png", makePNGBytes(t, 33, 33))
if rr2.Code != http.StatusCreated {
t.Fatalf("second upload status = %d; body = %s", rr2.Code, rr2.Body.String())
}
if got := doServeFile(t, router, id, otherToken, nil); got.Code != http.StatusForbidden {
t.Errorf("peer fetch of replaced avatar = %d, want 403", got.Code)
}
// The uploader can still reach their own old file.
if got := doServeFile(t, router, id, ownerToken, nil); got.Code != http.StatusOK {
t.Errorf("uploader fetch of replaced avatar = %d, want 200", got.Code)
}
}
func TestUploadAvatar_RejectsNonImageAndOversizedDimensions(t *testing.T) {
database := newUploadTestDB(t)
store := newUploadTestStorage(t)
router := buildAvatarRouter(database, store)
token := uploadCreateToken(t, database, "avatar_bad", 4)
// Sniffed from the bytes, never from the filename or the client's header.
if rr := doAvatarUpload(t, router, token, "me.png", []byte("this is plain text, not a PNG")); rr.Code != http.StatusBadRequest {
t.Errorf("text-as-png status = %d, want 400", rr.Code)
}
// GIF is a real image and still refused: an animated avatar in every
// message row is a distraction the renderer cannot opt out of.
gif := []byte("GIF89a")
if rr := doAvatarUpload(t, router, token, "me.gif", gif); rr.Code != http.StatusBadRequest {
t.Errorf("gif status = %d, want 400", rr.Code)
}
// Too many pixels for any surface that renders it.
if rr := doAvatarUpload(t, router, token, "huge.png", makePNGBytes(t, 2000, 100)); rr.Code != http.StatusBadRequest {
t.Errorf("oversized status = %d, want 400", rr.Code)
}
// None of the rejections may have moved the column.
user, _ := database.GetUserByUsername(context.Background(), "avatar_bad")
if user != nil && user.Avatar != nil && *user.Avatar != "" {
t.Errorf("a rejected upload set the avatar to %q", *user.Avatar)
}
}
func TestUploadAvatar_RequiresAuthAndAFile(t *testing.T) {
database := newUploadTestDB(t)
store := newUploadTestStorage(t)
router := buildAvatarRouter(database, store)
token := uploadCreateToken(t, database, "avatar_auth", 4)
if rr := doAvatarUpload(t, router, "", "me.png", makePNGBytes(t, 8, 8)); rr.Code != http.StatusUnauthorized {
t.Errorf("anonymous status = %d, want 401", rr.Code)
}
// Right form, wrong field name.
body, contentType := makeMultipartFile(t, "avatar", "me.png", makePNGBytes(t, 8, 8))
req := httptest.NewRequest(http.MethodPost, "/api/v1/users/me/avatar", body)
req.Header.Set("Content-Type", contentType)
req.Header.Set("Authorization", "Bearer "+token)
rr := httptest.NewRecorder()
router.ServeHTTP(rr, req)
if rr.Code != http.StatusBadRequest {
t.Errorf("wrong field status = %d, want 400", rr.Code)
}
}
// TestUploadAvatar_StorageErrorDoesNotLeakPath pins the same contract
// upload_handler.go's safeStorageErrorMessage enforces for the plain-file
// upload route: a storage.Save failure (disk full, permission change,
// read-only mount) must never hand the client the server's absolute
// storage path. handleUploadAvatar currently forwards saveErr verbatim.
func TestUploadAvatar_StorageErrorDoesNotLeakPath(t *testing.T) {
database := newUploadTestDB(t)
dir := t.TempDir()
store, err := storage.New(dir, 10)
if err != nil {
t.Fatalf("storage.New: %v", err)
}
router := buildAvatarRouter(database, store)
token := uploadCreateToken(t, database, "avatar_leakuser", 4)
// Remove the storage directory out from under the already-constructed
// Storage so Save's os.Create fails — this is what a disk-full,
// permission-change, or read-only-mount failure looks like from the
// handler's point of view: a storage-layer error surfaces at Save time.
if err := os.RemoveAll(dir); err != nil {
t.Fatalf("RemoveAll: %v", err)
}
rr := doAvatarUpload(t, router, token, "me.png", makePNGBytes(t, 32, 32))
// Server-side filesystem failures are 507 (storage.ErrIO) so they are
// distinguishable from bad uploads; the no-leak contract is unchanged.
if rr.Code != http.StatusInsufficientStorage {
t.Fatalf("status = %d, want 507; body: %s", rr.Code, rr.Body.String())
}
var resp map[string]any
if err := json.NewDecoder(rr.Body).Decode(&resp); err != nil {
t.Fatalf("decode: %v", err)
}
message, _ := resp["message"].(string)
if strings.Contains(message, dir) {
t.Fatalf("response message leaks the absolute storage path: %q", message)
}
if strings.ContainsAny(message, `/\`) {
t.Fatalf("response message looks like it contains a filesystem path: %q", message)
}
}
func TestUploadAvatar_NotMountedWithoutStorage(t *testing.T) {
database := newUploadTestDB(t)
r := chi.NewRouter()
limiter := auth.NewRateLimiter()
api.MountProfileRoutes(r, database, service.New(database, limiter), nil, limiter, nil, nil)
token := uploadCreateToken(t, database, "no_storage", 4)
if rr := doAvatarUpload(t, r, token, "me.png", makePNGBytes(t, 8, 8)); rr.Code == http.StatusCreated {
t.Error("avatar upload must not be served when there is no storage backend")
}
}
// ─── PATCH /users/me: display name and about ─────────────────────────────────
func TestUpdateProfile_SetsDisplayNameAndAbout(t *testing.T) {
database := newAuthTestDB(t)
router := buildProfileRouter(database)
token := profileCreateToken(t, database, "bio_user", 4)
rr := patchJSON(t, router, "/api/v1/users/me", token, map[string]any{
"username": "bio_user",
"display_name": "Bio User",
"about": "writes tests",
})
if rr.Code != http.StatusOK {
t.Fatalf("status = %d, want 200; body = %s", rr.Code, rr.Body.String())
}
var resp map[string]any
_ = json.NewDecoder(rr.Body).Decode(&resp)
if resp["display_name"] != "Bio User" {
t.Errorf("display_name = %v", resp["display_name"])
}
if resp["about"] != "writes tests" {
t.Errorf("about = %v", resp["about"])
}
// Omitting them leaves them alone.
rr = patchJSON(t, router, "/api/v1/users/me", token, map[string]any{"username": "bio_user"})
if rr.Code != http.StatusOK {
t.Fatalf("status = %d, want 200", rr.Code)
}
resp = map[string]any{}
_ = json.NewDecoder(rr.Body).Decode(&resp)
if resp["display_name"] != "Bio User" || resp["about"] != "writes tests" {
t.Errorf("a username-only PATCH cleared fields: %v / %v", resp["display_name"], resp["about"])
}
// An explicit empty string clears them.
rr = patchJSON(t, router, "/api/v1/users/me", token, map[string]any{
"username": "bio_user", "display_name": "", "about": "",
})
resp = map[string]any{}
_ = json.NewDecoder(rr.Body).Decode(&resp)
if resp["display_name"] != nil || resp["about"] != nil {
t.Errorf("expected cleared, got %v / %v", resp["display_name"], resp["about"])
}
}
func TestUpdateProfile_RejectsBadDisplayName(t *testing.T) {
database := newAuthTestDB(t)
router := buildProfileRouter(database)
token := profileCreateToken(t, database, "dn_user", 4)
// A right-to-left override makes a name render as something other than
// what it says — the same spoof auth.ValidateUsername rejects.
rr := patchJSON(t, router, "/api/v1/users/me", token, map[string]any{
"username": "dn_user", "display_name": "ada\u202egnp.exe",
})
if rr.Code != http.StatusBadRequest {
t.Errorf("bidi-override display_name status = %d, want 400", rr.Code)
}
rr = patchJSON(t, router, "/api/v1/users/me", token, map[string]any{
"username": "dn_user", "display_name": strings.Repeat("a", 33),
})
if rr.Code != http.StatusBadRequest {
t.Errorf("overlong display_name status = %d, want 400", rr.Code)
}
rr = patchJSON(t, router, "/api/v1/users/me", token, map[string]any{
"username": "dn_user", "about": strings.Repeat("b", 301),
})
if rr.Code != http.StatusBadRequest {
t.Errorf("overlong about status = %d, want 400", rr.Code)
}
}
func TestUpdateProfile_BroadcastCarriesEveryProfileField(t *testing.T) {
database := newAuthTestDB(t)
r := chi.NewRouter()
limiter := auth.NewRateLimiter()
spy := &userUpdateSpy{}
api.MountProfileRoutes(r, database, service.New(database, limiter), nil, limiter, nil, spy)
token := profileCreateToken(t, database, "bc_user", 4)
rr := patchJSON(t, r, "/api/v1/users/me", token, map[string]any{
"username": "bc_user", "display_name": "Broadcaster", "about": "hi",
})
if rr.Code != http.StatusOK {
t.Fatalf("status = %d; body = %s", rr.Code, rr.Body.String())
}
if len(spy.got) != 1 {
t.Fatalf("broadcasts = %d, want 1", len(spy.got))
}
u := spy.got[0]
// user_update replaces the client's copy wholesale, so a broadcast that
// omits a field would silently blank it everywhere.
if u.DisplayName == nil || *u.DisplayName != "Broadcaster" {
t.Errorf("broadcast display_name = %v", u.DisplayName)
}
if u.About == nil || *u.About != "hi" {
t.Errorf("broadcast about = %v", u.About)
}
if u.Username != "bc_user" {
t.Errorf("broadcast username = %q", u.Username)
}
}
func TestLogout_ClearsCustomStatus(t *testing.T) {
database := newAuthTestDB(t)
router := buildAuthRouter(database, auth.NewRateLimiter())
token := profileCreateToken(t, database, "logout_status", 4)
user, err := database.GetUserByUsername(context.Background(), "logout_status")
if err != nil || user == nil {
t.Fatalf("GetUserByUsername: %v", err)
}
text := "in a meeting"
if err := database.UpdateUserCustomStatus(context.Background(), user.ID, &text); err != nil {
t.Fatalf("UpdateUserCustomStatus: %v", err)
}
req := httptest.NewRequest(http.MethodPost, "/api/v1/auth/logout", bytes.NewReader(nil))
req.Header.Set("Authorization", "Bearer "+token)
req.RemoteAddr = "127.0.0.1:9999"
rr := httptest.NewRecorder()
router.ServeHTTP(rr, req)
if rr.Code != http.StatusNoContent {
t.Fatalf("logout status = %d, want 204; body = %s", rr.Code, rr.Body.String())
}
after, _ := database.GetUserByID(context.Background(), user.ID)
if after.CustomStatus != nil {
t.Errorf("custom_status = %q, want cleared on logout", *after.CustomStatus)
}
}
// userUpdateSpy captures the user_update broadcasts the profile routes emit.
type userUpdateSpy struct {
got []ws.UserUpdate
}
func (s *userUpdateSpy) BroadcastUserUpdate(u ws.UserUpdate) {
s.got = append(s.got, u)
}