mirror of
https://github.com/J3vb/OwnCord.git
synced 2026-09-03 03:50:00 +03:00
main
5
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 J3vbandClaude Opus 5
|
ea0430c5b0 |
fix: batch of correctness fixes across server and client (#1375)
* fix(client): 1 defect(s) (OC-0201)
* fix(service): 1 defect(s) (OC-0202)
HandleTyping built the per-user-per-channel rate-limit key before resolving the channel or checking read permission, so forged channel ids could pin unbounded dead entries in the shared process-wide RateLimiter.
* fix(client): 2 defect(s) (OC-0203, OC-0224)
* fix(server): 1 defect(s) (OC-0204)
* fix(ws): 2 defect(s) (OC-0205, OC-0211)
* fix(admin): 2 defect(s) (OC-0209, OC-0212)
* fix(client): 1 defect(s) (OC-0210)
* fix(db): 1 defect(s) (OC-0213)
* fix(ws): 1 defect(s) (OC-0214)
Route handler-driven PresenceEvent through BroadcastToAll instead of BroadcastToAllLow so every source of a user's presence shares one ordered per-client FIFO.
* fix(admin): 1 defect(s) (OC-0215)
PATCH /users/{id} combining banned + role_id committed and broadcast the ban before authorizing the role change, so a refused role change returned an error while leaving the target banned. Authorize the role change up front via the new ModerationService.AuthorizeRoleChange.
* fix(db): 1 defect(s) (OC-0216)
LinkAttachmentsToMessage no longer claims an attachment that is a user's live avatar (users.avatar points at it). Once message_id is set, handleServeFile's avatar branch (gated on ChannelID == nil) is unreachable and the file falls under the message's channel ACL / soft-delete state, permanently disagreeing with users.avatar about who may read it.
* fix(emoji): 1 defect(s) (OC-0217)
* fix(client): 1 defect(s) (OC-0218)
The data-copy phase of an HTTP proxy tunnel was unbounded. Steps 1-2 of
handle_connection (header read, TCP connect, TLS handshake) each run under
a 10s guard, but step 3 called io::copy_bidirectional with no deadline. A
remote that completes the TLS handshake and then neither responds nor
closes parks the spawned connection task, the loopback socket and the
remote TLS session indefinitely: copy_bidirectional only resolves once
BOTH directions finish, so closing the local side alone does not free it.
Wrap the copy in copy_with_deadline, a generic helper bounded by
DATA_PHASE_TIMEOUT (600s). The bound is deliberately far looser than the
10s setup guards because this phase carries the REST body, including
attachment and avatar uploads, so it must reclaim only genuinely stuck
connections rather than merely slow ones. The helper is generic over the
stream types so it can be exercised without a live TLS connection.
Regression test drives two in-memory duplex pairs whose far ends stay
alive, so neither half ever observes EOF and raw copy_bidirectional would
block forever; the test asserts the call resolves on its own deadline with
ErrorKind::TimedOut.
Claude-Session: https://claude.ai/code/session_01ENMDTh8gDLiHCaRFdMYRiL
* fix(ws): 1 defect(s) (OC-0219)
* fix(client): 1 defect(s) (OC-0221)
UpdateNotifier scheduled its deferred update check with a setTimeout whose
handle was never retained, so destroy() could not cancel it. A component torn
down inside the 3s window (page swap / logout) still fired performCheck() and
issued a network update check against the old server URL. Retain the timer
handle and clear it in destroy().
* fix(dm): 1 defect(s) (OC-0222)
* fix(client): 1 defect(s) (OC-0223)
* fix(voice): 1 defect(s) (OC-0225)
The Grant-Microphone retry's .finally hardcoded grantMicBtn.disabled = false, undoing updateFrozen()'s socket-down freeze when the WS socket dropped while the mic permission request was in flight. Delegate the state back to render().
* fix(admin): 1 defect(s) (OC-0226)
handleApplyUpdate broadcasts a 'restarting in 5s' notice before the on-disk
swap. Every failure path in the swap returned silently, leaving clients
counting down to a restart that never happened. Extract the swap into
applyStagedUpdate and send a corrective 'update_aborted' broadcast from a
deferred guard on every path that does not reach the respawn.
* fix(admin): 1 defect(s) (OC-0227)
PATCH /channels/{id} accepted a blank or whitespace-only name, leaving the
channel unidentifiable in clients. updateChannelRequest.validate() now
rejects it the way handleCreateChannel already did.
* fix(identity): 1 defect(s) (OC-0228)
* fix(admin): run deferred cleanup before the update restart exits
The fix batch left three golangci-lint findings and two prettier findings
that CI gates on.
applyStagedUpdate called os.Exit(0) in the same function that defers both
staged.Close() and the corrective "update_aborted" broadcast, so neither
ran (gocritic exitAfterDefer). Return a bool instead and let the caller
exit once those defers have run — on Windows, releasing the staged binary's
file handle is the reason the restart exists at all, so this is a real fix
rather than a lint appeasement. The exported test hook calls the function as
a statement, so the added result does not affect it.
Also modernize a bulk-insert loop to range-over-int, compare backup bytes
with bytes.Equal, and reflow two test files to prettier's output.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ENMDTh8gDLiHCaRFdMYRiL
* test(ws): pin the live presence path against the invisible custom-status leak
OC-0207 and OC-0211 are the same defect at two emitters: hub_broadcast.go's
BroadcastPresence (connect/reconnect) and event.go's presenceEvents (live
presence_update). The fix for OC-0211 closed both sites in one change, but
only the hub_broadcast side got a regression test.
This pins the event.go sibling: an invisible user's real custom status must
be blanked on the PresenceOthersEvent frame while the owner's own
PresenceSelfEvent still carries it. Without it, a later change could reopen
the live path while the committed test kept passing.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ENMDTh8gDLiHCaRFdMYRiL
* fix(ws): 1 defect(s) (OC-0206)
* test(ws): silence a contextcheck false positive in the reconnect race test
RefreshChannelVisibility takes no context by design — it is reached through
the admin HubBroadcaster interface, which carries none, so it builds its own
internally. contextcheck flags the call only because the test closure around
it holds a ctx for its override write, so there is nothing to propagate.
Suppress at the call site rather than widen a production interface (and its
mocks) to satisfy a lint in a test.
golangci-lint v2.11.3 (the version ci.yml pins) now reports 0 issues.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ENMDTh8gDLiHCaRFdMYRiL
---------
Co-authored-by: Claude <noreply@anthropic.com>
|
||
|
J3vbandClaude Fable 5
|
6afa9e974c |
refactor(server): thread context.Context through the db layer and all callers
Fixes all 109 golangci-lint findings (106 contextcheck, 1 gocritic,
2 gosec) that accumulated after D2 wired dbgen (whose queries take ctx)
under ctx-less db.DB wrappers while CI lint was quota-dead. No nolint
comments added; every finding fixed by genuinely threading context.
- db: all 138 hand-written db.DB methods take ctx first; the dbCtx()
Background shim is deleted; raw Query/QueryRow/Exec/Begin use their
Context variants; the four redundant ctx-less passthroughs removed.
db.Auditor/WriteAudit gain ctx.
- Seams: permissions.Checker (DB iface, HasChannelPerm,
RequireChannelAccess) and the service.Store interface mirror the new
signatures (ws.EventStore and plugin.PluginStore already did).
- Callers: api/admin handlers use r.Context(); ws per-message paths use
the connection ctx via DispatchV2; hub loops and startup wiring use
context.Background(); service methods thread ctx where they have one
and Background where no ctx exists. Public service surface reached by
ctx-holding chains (PermissionService.HasChannelPerm/GetRoleForUser/
RequireChannelAccess, message/dm/block/invite/profile methods) is now
ctx-first.
- Detached (context.WithoutCancel) where cancellation would break an
invariant, found by a 3-lens adversarial review of the diff:
* voice-leave background retries (a dead webhook/connection ctx killed
retry 2 before it ran, leaving ghost capacity-holding voice rows)
* rollbackVoiceJoin's compensating delete (its trigger IS the cancel)
* post-2FA-change DeleteOtherSessions and logout DeleteSession (the
security tail of a committed change must not die with the request)
* all api/ws audit writes (a banned user could suppress their own
login_blocked_banned row by aborting the request mid-bcrypt)
* admin backup VACUUM INTO (an interrupt left a truncated .db that
the backup list presented as restorable)
* post-commit message/edit refetches (a committed message must still
fan out when the sender disconnects)
* hub settings-cache refresh (one dead connection could pin stale
values for the 30s TTL)
- gocritic rangeValCopy fixed (index iteration); gosec G306 excluded in
config with justification (generated source must stay world-readable)
instead of flipping genprotocol output to 0o600.
Verified: gofmt/vet, all four build-tag variants, full suite, deadlock
pass, full -race pass, golangci-lint 0 issues uncapped.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
||
|
J3vbandClaude Fable 5
|
95f85e213f |
test(service): cover atomic attachment-ownership link semantics (W1-3)
Mechanical signature updates for LinkAttachmentsToMessage callsites, plus: db-level OwnershipGuard test (owned links, foreign never links, legacy NULL-uploader claimable, nonexistent skipped) and a service-level SendMessage test proving skip semantics end-to-end including the already-linked retry path. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> |
||
|
jevb
|
65a8403a92 |
fix: resolve 1 critical and 5 high security/reliability issues from go-review
- CRIT-2: Prevent HTTP header injection in Content-Disposition via mime.FormatMediaType - HIGH-1: Call hub.GracefulStop() on server shutdown to clean up WebSocket/voice state - HIGH-2: Remove double send on serveErr channel that caused goroutine leak - HIGH-3: Handle GetUserByID error after registration to prevent nil panic - HIGH-4: Return nil,nil on sql.ErrNoRows in GetAttachmentByID (matching codebase convention) - HIGH-5: Add voiceDone channel to bound RTP goroutine lifetime on voice teardown - Fix handleVoiceICE to return VOICE_ERROR when no PC (consistent with other voice handlers) - Update tests to match corrected behavior |
||
|
jevb
|
fca1b0cc0c |
test: boost server test coverage to 80%+ across all packages
Add comprehensive tests for db, storage, updater, and ws packages covering edge cases, error paths, and voice handler functions. New test files for attachment queries and WebSocket coverage boost. |