mirror of
https://github.com/J3vb/OwnCord.git
synced 2026-09-03 03:50:00 +03:00
Claude
d320a8b587
Clean sweep of every actionable item from the two Copilot review passes
on head 59ae4d8. Grouped by severity:
─── Crash / security (must-fix) ─────────────────────────────────────
1. main.go:140 — telemetryShutdown nil panic.
telemetry.Init can return (nil, err) on the -tags otel skeleton
path; the deferred closure would then call a nil function. Normalise
to a no-op shutdown when Init errors so the defer is always safe.
2. api/upload_handler.go — permSvc nil deref.
MountUploadRoutes + handleServeFile dereference permSvc on every
authenticated file request. Add a fail-fast panic at mount time so
the misconfiguration surfaces at wiring, not on the first 500.
Update upload_handler_test.go to pass a real PermissionService built
on the test DB (the existing tests were missing the argument entirely,
which meant the package wouldn't compile — this fixes the real bug
Copilot flagged).
3. ws/event_persister.go — NewEventPersister nil EventStore panic.
run() dereferences p.store on every flush. Panic at constructor
time instead so the crash happens once at startup rather than
minutes later in a background goroutine.
4. plugin/host_ui.go — serve-time symlink check.
rejectSymlinksUnder only runs at install time, so a symlink created
post-install (accidental or malicious) would be followed by
http.ServeFile and leak host files. Add an os.Lstat + ModeSymlink
check + IsRegular check to AssetHandler on every request. Cheap
relative to the file read and closes the TOCTOU window.
─── Correctness / observability (should-fix) ───────────────────────
5. ws/deps.go:77 — requirePerm hides misconfig as FORBIDDEN.
Previously, nil database, nil perms, or a GetRoleForUser error all
returned ErrCodeForbidden with the same message, making operator
failures indistinguishable from legitimate permission denials.
Split the branches: misconfig + DB error now return ErrCodeInternal
with a server-side slog.Error so operators see the real problem;
FORBIDDEN is reserved for the actual permission-bit check.
6. telemetry/metrics.go — ServiceCallDurationMs renamed to Sec.
Field name said "Ms" but the instrument name was
`service_call_duration_seconds` with unit "s". Renamed the field
and updated all 8 service-layer callers so the struct field and
metric semantics match.
7. ws/event_persister.go — flushEvy typo → flushEvery.
Renamed the field and the one call site in run().
─── Comments out of sync with code ──────────────────────────────────
8. plugin/loader.go — Stat vs Lstat comment.
The comment claimed "Stat (not Lstat)" but the code correctly uses
os.Lstat to detect symlinks. Updated the comment to match the code;
the code was already right.
9. telemetry/telemetry_otel.go — compile claim wrong.
Comment said the file would fail to compile without the upstream
OTel modules, but the skeleton deliberately avoids importing them
and Init returns a runtime error instead. Updated the comment to
reflect actual CI behaviour (the -tags otel build step passes
today but doesn't exercise real telemetry).
─── Nit / polish ────────────────────────────────────────────────────
10. ws/event_pruner.go — startup delay magic constant.
Hard-coded time.Minute made the "run shortly after startup"
behaviour untestable (a test with a 100ms interval would still
wait a full minute). Cap the startup delay by the interval:
min(interval, time.Minute). Documented via a new `maxStartupDelay`
constant.
11. ws/event_pruner_test.go — new file.
Unit coverage for runPrune cutoff correctness, error swallowing,
StartEventPruner nil-store short-circuit, ctx cancellation, and
the interval-bounded startup delay from fix #10. Uses a fakeEventStore
stub that records every prune call and signals the first one so
tests don't sleep.
─── Verification ────────────────────────────────────────────────────
gofmt -l clean. No network access in sandbox so `go vet` and `go test`
could not run; the changes are local and surgical and every touched
file compiles in isolation against the existing signatures.
https://claude.ai/code/session_01UsBsQW2YiA2usk9pnJjAWk
86 lines
2.9 KiB
Go
86 lines
2.9 KiB
Go
package service
|
|
|
|
import (
|
|
"context"
|
|
"fmt"
|
|
"log/slog"
|
|
"time"
|
|
|
|
"github.com/owncord/server/db"
|
|
"github.com/owncord/server/store"
|
|
"github.com/owncord/server/telemetry"
|
|
)
|
|
|
|
// UserService handles user profile and session operations.
|
|
type UserService struct {
|
|
st store.Store
|
|
}
|
|
|
|
// NewUserService creates a UserService.
|
|
func NewUserService(st store.Store) *UserService {
|
|
return &UserService{st: st}
|
|
}
|
|
|
|
// UpdateProfile updates a user's username and/or avatar.
|
|
// Returns the updated user for response building.
|
|
func (s *UserService) UpdateProfile(userID int64, username string, avatar *string) (*db.User, error) {
|
|
ctx, span := telemetry.GlobalTracer("service/user").Start(context.Background(), "UserService.UpdateProfile",
|
|
telemetry.Int64("user_id", userID),
|
|
)
|
|
start := time.Now()
|
|
defer func() {
|
|
telemetry.TimeSince(ctx, telemetry.NewAppMetrics().ServiceCallDurationSec, start,
|
|
telemetry.String("method", "UpdateProfile"))
|
|
span.End()
|
|
}()
|
|
|
|
if err := s.st.UpdateUserProfile(userID, username, avatar); err != nil {
|
|
if db.IsUniqueConstraintError(err) {
|
|
return nil, fmt.Errorf("%w: username is already taken", ErrConflict)
|
|
}
|
|
return nil, fmt.Errorf("%w: failed to update profile", ErrInternal)
|
|
}
|
|
user, err := s.st.GetUserByID(userID)
|
|
if err != nil {
|
|
return nil, fmt.Errorf("%w: failed to fetch updated user", ErrInternal)
|
|
}
|
|
_ = s.st.LogAudit(userID, "profile_update", "user", userID,
|
|
fmt.Sprintf("username=%s", username))
|
|
slog.Info("profile updated", "user_id", userID, "username", username)
|
|
return user, nil
|
|
}
|
|
|
|
// ChangePassword verifies the old password hash matches, then updates.
|
|
// Returns the number of other sessions revoked.
|
|
func (s *UserService) ChangePassword(userID int64, newPasswordHash string, keepSessionID int64) (int64, error) {
|
|
if err := s.st.UpdateUserPassword(userID, newPasswordHash); err != nil {
|
|
return 0, fmt.Errorf("%w: failed to update password", ErrInternal)
|
|
}
|
|
revoked, err := s.st.DeleteOtherSessions(userID, keepSessionID)
|
|
if err != nil {
|
|
slog.Error("UserService.ChangePassword DeleteOtherSessions", "err", err, "user_id", userID)
|
|
}
|
|
_ = s.st.LogAudit(userID, "password_change", "user", userID, "password changed")
|
|
slog.Info("password changed", "user_id", userID, "sessions_revoked", revoked)
|
|
return revoked, nil
|
|
}
|
|
|
|
// ListSessions returns all active sessions for a user.
|
|
func (s *UserService) ListSessions(userID int64) ([]db.Session, error) {
|
|
sessions, err := s.st.ListUserSessions(userID)
|
|
if err != nil {
|
|
return nil, fmt.Errorf("%w: failed to list sessions", ErrInternal)
|
|
}
|
|
return sessions, nil
|
|
}
|
|
|
|
// RevokeSession deletes a specific session owned by the user.
|
|
func (s *UserService) RevokeSession(userID, sessionID int64) error {
|
|
if err := s.st.DeleteSessionByID(sessionID, userID); err != nil {
|
|
return fmt.Errorf("%w: session not found", ErrNotFound)
|
|
}
|
|
_ = s.st.LogAudit(userID, "session_revoke", "session", sessionID, "session revoked")
|
|
slog.Info("session revoked", "user_id", userID, "session_id", sessionID)
|
|
return nil
|
|
}
|