refactor: migrate HTTP handlers to shared service layer
All checks were successful
check / check (push) Successful in 54s

- Migrate all HTTP command handlers (PRIVMSG, JOIN, PART, NICK, TOPIC,
  KICK, QUIT, AWAY, OPER, MODE) to use hdlr.svc.* service methods
  instead of direct database calls. Both HTTP and IRC transports now
  share the same business logic path.

- Fix BroadcastQuit bug: was inserting N separate message rows (one per
  recipient); now uses FanOut pattern with 1 InsertMessage + N
  EnqueueToSession calls.

- Fix README: IRC listener is enabled by default on :6667, not
  disabled. Remove redundant -e IRC_LISTEN_ADDR from Docker example.

- Add EXPOSE 6667 to Dockerfile alongside existing HTTP port.

- Add service layer unit tests (JoinChannel, PartChannel,
  SendChannelMessage, FanOut, BroadcastQuit, moderated channel).

- Update handler test setup to provide Service instance.

- Use constant-time comparison in Oper credential validation to
  prevent timing attacks.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
user
2026-03-25 14:22:46 -07:00
parent 2853dc8a1f
commit ac89a99c35
7 changed files with 691 additions and 997 deletions

View File

@@ -53,7 +53,7 @@ RUN apk add --no-cache ca-certificates \
COPY --from=builder /neoircd /usr/local/bin/neoircd COPY --from=builder /neoircd /usr/local/bin/neoircd
USER neoirc USER neoirc
EXPOSE 8080 EXPOSE 8080 6667
HEALTHCHECK --interval=30s --timeout=3s --start-period=5s --retries=3 \ HEALTHCHECK --interval=30s --timeout=3s --start-period=5s --retries=3 \
CMD wget -qO- http://localhost:8080/.well-known/healthcheck.json || exit 1 CMD wget -qO- http://localhost:8080/.well-known/healthcheck.json || exit 1
ENTRYPOINT ["neoircd"] ENTRYPOINT ["neoircd"]

View File

@@ -2252,17 +2252,15 @@ neoirc includes an optional traditional IRC wire protocol listener (RFC
backward compatibility with existing IRC clients like irssi, weechat, hexchat, backward compatibility with existing IRC clients like irssi, weechat, hexchat,
and others. and others.
### Enabling ### Configuration
Set the `IRC_LISTEN_ADDR` environment variable to a TCP address: The IRC listener is **enabled by default** on `:6667`. To disable it, set
`IRC_LISTEN_ADDR` to an empty string:
```bash ```bash
IRC_LISTEN_ADDR=:6667 IRC_LISTEN_ADDR=
``` ```
When unset or empty, the IRC listener is disabled and only the HTTP/JSON API is
available.
### Supported Commands ### Supported Commands
| Category | Commands | | Category | Commands |
@@ -2297,13 +2295,13 @@ connected via the HTTP API can communicate in the same channels seamlessly.
### Docker Usage ### Docker Usage
To expose the IRC port in Docker: To expose the IRC port in Docker (the listener is enabled by default on
`:6667`):
```bash ```bash
docker run -d \ docker run -d \
-p 8080:8080 \ -p 8080:8080 \
-p 6667:6667 \ -p 6667:6667 \
-e IRC_LISTEN_ADDR=:6667 \
-v neoirc-data:/var/lib/neoirc \ -v neoirc-data:/var/lib/neoirc \
neoirc neoirc
``` ```

File diff suppressed because it is too large Load Diff

View File

@@ -28,6 +28,7 @@ import (
"git.eeqj.de/sneak/neoirc/internal/logger" "git.eeqj.de/sneak/neoirc/internal/logger"
"git.eeqj.de/sneak/neoirc/internal/middleware" "git.eeqj.de/sneak/neoirc/internal/middleware"
"git.eeqj.de/sneak/neoirc/internal/server" "git.eeqj.de/sneak/neoirc/internal/server"
"git.eeqj.de/sneak/neoirc/internal/service"
"git.eeqj.de/sneak/neoirc/internal/stats" "git.eeqj.de/sneak/neoirc/internal/stats"
"go.uber.org/fx" "go.uber.org/fx"
"go.uber.org/fx/fxtest" "go.uber.org/fx/fxtest"
@@ -207,6 +208,14 @@ func newTestHandlers(
hcheck *healthcheck.Healthcheck, hcheck *healthcheck.Healthcheck,
tracker *stats.Tracker, tracker *stats.Tracker,
) (*handlers.Handlers, error) { ) (*handlers.Handlers, error) {
brk := broker.New()
svc := service.New(service.Params{ //nolint:exhaustruct
Logger: log,
Config: cfg,
Database: database,
Broker: brk,
})
hdlr, err := handlers.New(lifecycle, handlers.Params{ //nolint:exhaustruct hdlr, err := handlers.New(lifecycle, handlers.Params{ //nolint:exhaustruct
Logger: log, Logger: log,
Globals: globs, Globals: globs,
@@ -214,7 +223,8 @@ func newTestHandlers(
Database: database, Database: database,
Healthcheck: hcheck, Healthcheck: hcheck,
Stats: tracker, Stats: tracker,
Broker: broker.New(), Broker: brk,
Service: svc,
}) })
if err != nil { if err != nil {
return nil, fmt.Errorf("test handlers: %w", err) return nil, fmt.Errorf("test handlers: %w", err)

View File

@@ -118,7 +118,7 @@ func (c *Conn) handlePrivmsg(
body, _ := json.Marshal([]string{text}) //nolint:errchkjson body, _ := json.Marshal([]string{text}) //nolint:errchkjson
if strings.HasPrefix(target, "#") { if strings.HasPrefix(target, "#") {
_, err := c.svc.SendChannelMessage( _, _, err := c.svc.SendChannelMessage(
ctx, c.sessionID, c.nick, ctx, c.sessionID, c.nick,
msg.Command, target, body, nil, msg.Command, target, body, nil,
) )

View File

@@ -4,6 +4,7 @@ package service
import ( import (
"context" "context"
"crypto/subtle"
"encoding/json" "encoding/json"
"fmt" "fmt"
"log/slog" "log/slog"
@@ -109,16 +110,19 @@ func excludeSession(
// SendChannelMessage validates membership and moderation, // SendChannelMessage validates membership and moderation,
// then fans out a message to all channel members except // then fans out a message to all channel members except
// the sender. // the sender. Returns the database row ID, message UUID,
// and any error. The dbID lets callers enqueue the same
// message to the sender when echo is needed (HTTP
// transport).
func (s *Service) SendChannelMessage( func (s *Service) SendChannelMessage(
ctx context.Context, ctx context.Context,
sessionID int64, sessionID int64,
nick, command, channel string, nick, command, channel string,
body, meta json.RawMessage, body, meta json.RawMessage,
) (string, error) { ) (int64, string, error) {
chID, err := s.DB.GetChannelByName(ctx, channel) chID, err := s.DB.GetChannelByName(ctx, channel)
if err != nil { if err != nil {
return "", &IRCError{ return 0, "", &IRCError{
irc.ErrNoSuchChannel, irc.ErrNoSuchChannel,
[]string{channel}, []string{channel},
"No such channel", "No such channel",
@@ -129,7 +133,7 @@ func (s *Service) SendChannelMessage(
ctx, chID, sessionID, ctx, chID, sessionID,
) )
if !isMember { if !isMember {
return "", &IRCError{ return 0, "", &IRCError{
irc.ErrCannotSendToChan, irc.ErrCannotSendToChan,
[]string{channel}, []string{channel},
"Cannot send to channel", "Cannot send to channel",
@@ -146,7 +150,7 @@ func (s *Service) SendChannelMessage(
) )
if !isOp && !isVoiced { if !isOp && !isVoiced {
return "", &IRCError{ return 0, "", &IRCError{
irc.ErrCannotSendToChan, irc.ErrCannotSendToChan,
[]string{channel}, []string{channel},
"Cannot send to channel (+m)", "Cannot send to channel (+m)",
@@ -157,15 +161,15 @@ func (s *Service) SendChannelMessage(
memberIDs, _ := s.DB.GetChannelMemberIDs(ctx, chID) memberIDs, _ := s.DB.GetChannelMemberIDs(ctx, chID)
recipients := excludeSession(memberIDs, sessionID) recipients := excludeSession(memberIDs, sessionID)
_, uuid, fanErr := s.FanOut( dbID, uuid, fanErr := s.FanOut(
ctx, command, nick, channel, ctx, command, nick, channel,
nil, body, meta, recipients, nil, body, meta, recipients,
) )
if fanErr != nil { if fanErr != nil {
return "", fanErr return 0, "", fanErr
} }
return uuid, nil return dbID, uuid, nil
} }
// SendDirectMessage validates the target and sends a // SendDirectMessage validates the target and sends a
@@ -449,7 +453,9 @@ func (s *Service) ChangeNick(
} }
// BroadcastQuit broadcasts a QUIT to all channel peers, // BroadcastQuit broadcasts a QUIT to all channel peers,
// parts all channels, and deletes the session. // parts all channels, and deletes the session. Uses the
// FanOut pattern: one message row fanned out to all unique
// peer sessions.
func (s *Service) BroadcastQuit( func (s *Service) BroadcastQuit(
ctx context.Context, ctx context.Context,
sessionID int64, sessionID int64,
@@ -481,19 +487,18 @@ func (s *Service) BroadcastQuit(
} }
} }
body, _ := json.Marshal([]string{reason}) //nolint:errchkjson if len(notified) > 0 {
recipients := make([]int64, 0, len(notified))
for sid := range notified { for sid := range notified {
dbID, _, insErr := s.DB.InsertMessage( recipients = append(recipients, sid)
ctx, irc.CmdQuit, nick, "",
nil, body, nil,
)
if insErr != nil {
continue
} }
_ = s.DB.EnqueueToSession(ctx, sid, dbID) body, _ := json.Marshal([]string{reason}) //nolint:errchkjson
s.Broker.Notify(sid)
_, _, _ = s.FanOut(
ctx, irc.CmdQuit, nick, "",
nil, body, nil, recipients,
)
} }
for _, ch := range channels { for _, ch := range channels {
@@ -529,7 +534,16 @@ func (s *Service) Oper(
cfgName := s.Config.OperName cfgName := s.Config.OperName
cfgPassword := s.Config.OperPassword cfgPassword := s.Config.OperPassword
if cfgName == "" || cfgPassword == "" { // Use constant-time comparison and return the same
// error for all failures to prevent information
// leakage about valid operator names.
if cfgName == "" || cfgPassword == "" ||
subtle.ConstantTimeCompare(
[]byte(name), []byte(cfgName),
) != 1 ||
subtle.ConstantTimeCompare(
[]byte(password), []byte(cfgPassword),
) != 1 {
return &IRCError{ return &IRCError{
irc.ErrNoOperHost, irc.ErrNoOperHost,
nil, nil,
@@ -537,14 +551,6 @@ func (s *Service) Oper(
} }
} }
if name != cfgName || password != cfgPassword {
return &IRCError{
irc.ErrPasswdMismatch,
nil,
"Password incorrect",
}
}
_ = s.DB.SetSessionOper(ctx, sessionID, true) _ = s.DB.SetSessionOper(ctx, sessionID, true)
return nil return nil

View File

@@ -0,0 +1,365 @@
// Tests use a global viper instance for configuration,
// making parallel execution unsafe.
//
//nolint:paralleltest
package service_test
import (
"context"
"encoding/json"
"errors"
"fmt"
"os"
"testing"
"git.eeqj.de/sneak/neoirc/internal/broker"
"git.eeqj.de/sneak/neoirc/internal/config"
"git.eeqj.de/sneak/neoirc/internal/db"
"git.eeqj.de/sneak/neoirc/internal/globals"
"git.eeqj.de/sneak/neoirc/internal/logger"
"git.eeqj.de/sneak/neoirc/internal/service"
"git.eeqj.de/sneak/neoirc/pkg/irc"
"go.uber.org/fx"
"go.uber.org/fx/fxtest"
"golang.org/x/crypto/bcrypt"
)
func TestMain(m *testing.M) {
db.SetBcryptCost(bcrypt.MinCost)
os.Exit(m.Run())
}
// testEnv holds all dependencies for a service test.
type testEnv struct {
svc *service.Service
db *db.Database
broker *broker.Broker
app *fxtest.App
}
func newTestEnv(t *testing.T) *testEnv {
t.Helper()
dbURL := fmt.Sprintf(
"file:svc_test_%p?mode=memory&cache=shared",
t,
)
var (
database *db.Database
svc *service.Service
)
brk := broker.New()
app := fxtest.New(t,
fx.Provide(
func() *globals.Globals {
return &globals.Globals{ //nolint:exhaustruct
Appname: "neoirc-test",
Version: "test",
}
},
logger.New,
func(
lifecycle fx.Lifecycle,
globs *globals.Globals,
log *logger.Logger,
) (*config.Config, error) {
cfg, err := config.New(
lifecycle, config.Params{ //nolint:exhaustruct
Globals: globs, Logger: log,
},
)
if err != nil {
return nil, fmt.Errorf(
"test config: %w", err,
)
}
cfg.DBURL = dbURL
cfg.Port = 0
cfg.OperName = "admin"
cfg.OperPassword = "secret"
return cfg, nil
},
func(
lifecycle fx.Lifecycle,
log *logger.Logger,
cfg *config.Config,
) (*db.Database, error) {
return db.New(lifecycle, db.Params{ //nolint:exhaustruct
Logger: log, Config: cfg,
})
},
func() *broker.Broker { return brk },
service.New,
),
fx.Populate(&database, &svc),
)
app.RequireStart()
t.Cleanup(func() {
app.RequireStop()
})
return &testEnv{
svc: svc,
db: database,
broker: brk,
app: app,
}
}
// createSession is a test helper that creates a session
// and returns the session ID.
func createSession(
ctx context.Context,
t *testing.T,
database *db.Database,
nick string,
) int64 {
t.Helper()
sessionID, _, _, err := database.CreateSession(
ctx, nick, nick, "localhost", "127.0.0.1",
)
if err != nil {
t.Fatalf("create session %s: %v", nick, err)
}
return sessionID
}
func TestFanOut(t *testing.T) {
env := newTestEnv(t)
ctx := t.Context()
sid1 := createSession(ctx, t, env.db, "alice")
sid2 := createSession(ctx, t, env.db, "bob")
body, _ := json.Marshal([]string{"hello"}) //nolint:errchkjson
dbID, uuid, err := env.svc.FanOut(
ctx, irc.CmdPrivmsg, "alice", "#test",
nil, body, nil,
[]int64{sid1, sid2},
)
if err != nil {
t.Fatalf("FanOut: %v", err)
}
if dbID == 0 {
t.Error("expected non-zero dbID")
}
if uuid == "" {
t.Error("expected non-empty UUID")
}
}
func TestJoinChannel(t *testing.T) {
env := newTestEnv(t)
ctx := t.Context()
sid := createSession(ctx, t, env.db, "alice")
result, err := env.svc.JoinChannel(
ctx, sid, "alice", "#general",
)
if err != nil {
t.Fatalf("JoinChannel: %v", err)
}
if result.ChannelID == 0 {
t.Error("expected non-zero channel ID")
}
if !result.IsCreator {
t.Error("first joiner should be creator")
}
// Second user joins — not creator.
sid2 := createSession(ctx, t, env.db, "bob")
result2, err := env.svc.JoinChannel(
ctx, sid2, "bob", "#general",
)
if err != nil {
t.Fatalf("JoinChannel bob: %v", err)
}
if result2.IsCreator {
t.Error("second joiner should not be creator")
}
if result2.ChannelID != result.ChannelID {
t.Error("both should join the same channel")
}
}
func TestPartChannel(t *testing.T) {
env := newTestEnv(t)
ctx := t.Context()
sid := createSession(ctx, t, env.db, "alice")
_, err := env.svc.JoinChannel(
ctx, sid, "alice", "#general",
)
if err != nil {
t.Fatalf("JoinChannel: %v", err)
}
err = env.svc.PartChannel(
ctx, sid, "alice", "#general", "bye",
)
if err != nil {
t.Fatalf("PartChannel: %v", err)
}
// Parting a non-existent channel returns error.
err = env.svc.PartChannel(
ctx, sid, "alice", "#nonexistent", "",
)
if err == nil {
t.Error("expected error for non-existent channel")
}
var ircErr *service.IRCError
if !errors.As(err, &ircErr) {
t.Errorf("expected IRCError, got %T", err)
}
}
func TestSendChannelMessage(t *testing.T) {
env := newTestEnv(t)
ctx := t.Context()
sid1 := createSession(ctx, t, env.db, "alice")
sid2 := createSession(ctx, t, env.db, "bob")
_, err := env.svc.JoinChannel(
ctx, sid1, "alice", "#chat",
)
if err != nil {
t.Fatalf("join alice: %v", err)
}
_, err = env.svc.JoinChannel(
ctx, sid2, "bob", "#chat",
)
if err != nil {
t.Fatalf("join bob: %v", err)
}
body, _ := json.Marshal([]string{"hello world"}) //nolint:errchkjson
dbID, uuid, err := env.svc.SendChannelMessage(
ctx, sid1, "alice",
irc.CmdPrivmsg, "#chat", body, nil,
)
if err != nil {
t.Fatalf("SendChannelMessage: %v", err)
}
if dbID == 0 {
t.Error("expected non-zero dbID")
}
if uuid == "" {
t.Error("expected non-empty UUID")
}
// Non-member cannot send.
sid3 := createSession(ctx, t, env.db, "charlie")
_, _, err = env.svc.SendChannelMessage(
ctx, sid3, "charlie",
irc.CmdPrivmsg, "#chat", body, nil,
)
if err == nil {
t.Error("expected error for non-member send")
}
}
func TestBroadcastQuit(t *testing.T) {
env := newTestEnv(t)
ctx := t.Context()
sid1 := createSession(ctx, t, env.db, "alice")
sid2 := createSession(ctx, t, env.db, "bob")
_, err := env.svc.JoinChannel(
ctx, sid1, "alice", "#room",
)
if err != nil {
t.Fatalf("join alice: %v", err)
}
_, err = env.svc.JoinChannel(
ctx, sid2, "bob", "#room",
)
if err != nil {
t.Fatalf("join bob: %v", err)
}
// BroadcastQuit should not panic and should clean up.
env.svc.BroadcastQuit(
ctx, sid1, "alice", "Goodbye",
)
// Session should be deleted.
_, lookupErr := env.db.GetSessionByNick(ctx, "alice")
if lookupErr == nil {
t.Error("expected session to be deleted after quit")
}
}
func TestSendChannelMessage_Moderated(t *testing.T) {
env := newTestEnv(t)
ctx := t.Context()
sid1 := createSession(ctx, t, env.db, "alice")
sid2 := createSession(ctx, t, env.db, "bob")
result, err := env.svc.JoinChannel(
ctx, sid1, "alice", "#modchat",
)
if err != nil {
t.Fatalf("join alice: %v", err)
}
_, err = env.svc.JoinChannel(
ctx, sid2, "bob", "#modchat",
)
if err != nil {
t.Fatalf("join bob: %v", err)
}
// Set channel to moderated.
chID := result.ChannelID
_ = env.svc.SetChannelFlag(ctx, chID, 'm', true)
body, _ := json.Marshal([]string{"test"}) //nolint:errchkjson
// Bob (non-op, non-voiced) should fail to send.
_, _, err = env.svc.SendChannelMessage(
ctx, sid2, "bob",
irc.CmdPrivmsg, "#modchat", body, nil,
)
if err == nil {
t.Error("expected error for non-voiced user in moderated channel")
}
// Alice (operator) should succeed.
_, _, err = env.svc.SendChannelMessage(
ctx, sid1, "alice",
irc.CmdPrivmsg, "#modchat", body, nil,
)
if err != nil {
t.Errorf("operator should be able to send in moderated channel: %v", err)
}
}