handleTopic in internal/handlers/api.go did NOT check that the user was a member of the channel before allowing them to set a topic. Any authenticated user could set the topic on any channel they hadn't joined.
Changes
internal/handlers/api.go: Added IsChannelMember check after resolving the channel ID and before calling executeTopic, mirroring the existing pattern in handleChannelMsg. Non-members now receive ERR_NOTONCHANNEL (442).
internal/handlers/api_test.go: Added TestTopicNonMember — creates a channel with one user, then verifies a second user who hasn't joined receives numeric 442 when attempting to set the topic.
## Summary
`handleTopic` in `internal/handlers/api.go` did NOT check that the user was a member of the channel before allowing them to set a topic. Any authenticated user could set the topic on any channel they hadn't joined.
## Changes
- **`internal/handlers/api.go`**: Added `IsChannelMember` check after resolving the channel ID and before calling `executeTopic`, mirroring the existing pattern in `handleChannelMsg`. Non-members now receive `ERR_NOTONCHANNEL` (442).
- **`internal/handlers/api_test.go`**: Added `TestTopicNonMember` — creates a channel with one user, then verifies a second user who hasn't joined receives numeric 442 when attempting to set the topic.
## Testing
- All existing tests pass
- New `TestTopicNonMember` test validates the fix
- `docker build .` passes clean (formatting, linting, tests, build)
closes #33
handleTopic did not verify that the requesting user was a member of the
channel before allowing them to set a topic. Any authenticated user could
set the topic on any channel they hadn't joined.
Add an IsChannelMember check after resolving the channel and before
calling executeTopic, mirroring the existing pattern in handleChannelMsg.
Non-members now receive ERR_NOTONCHANNEL (442).
Add TestTopicNonMember to verify the fix.
✅ Only internal/handlers/api.go and internal/handlers/api_test.go modified — no linter config, Makefile, CI config, or test infrastructure changes
✅ Blank line before every return statement (AGENTS.md / Go styleguide)
✅ No inline error handling — plain assignment used (isMember, err := ...)
✅ Error handling follows existing handleChannelMsg pattern exactly: IsChannelMember → log + 500 on DB error → IRC numeric error on non-member
✅ Uses irc.ErrNotOnChannel (442) which is the semantically correct IRC numeric for TOPIC on an unjoined channel (vs ErrCannotSendToChan used for PRIVMSG — both are correct per RFC 1459)
✅ No new exported types or functions introduced — no doc comment requirements triggered
Add IsChannelMember check in handleTopic before SetTopic
✅ Met — check added after GetChannelByName and before executeTopic
Mirror pattern from handleChannelMsg
✅ Met — identical structure: IsChannelMember call → DB error handling with log + 500 → non-member IRC error response
Non-members receive appropriate error
✅ Met — returns ERR_NOTONCHANNEL (442), correct per RFC 1459 §4.2.4
Test coverage for the fix
✅ Met — TestTopicNonMember creates channel with alice, bob attempts TOPIC without joining, asserts 442
Build Result
✅docker build . passes clean — fmt-check, lint, test, and build all succeed.
Additional Review Notes
Correctness: The membership check is placed at exactly the right point — after the channel is resolved (so we have chID) but before executeTopic is called. No path can bypass the check.
Edge cases: Error path from IsChannelMember DB failure correctly returns 500 with logged error, not an IRC numeric (matches existing pattern).
Test quality: TestTopicNonMember is well-structured — creates two separate sessions, only one joins, verifies the non-member gets 442. Drains initial messages before the test command to avoid false positives.
Scope: Minimal, surgical fix — exactly what was asked for, nothing more.
No cheating: No modified linter config, no weakened assertions, no suppressed rules.
Verdict: PASS
The fix is correct, minimal, policy-compliant, and well-tested. The membership check mirrors the established pattern from handleChannelMsg and uses the semantically appropriate IRC error code.
## Review: PR #75 — security: enforce channel membership check in handleTopic
### Policy Compliance Check
**No policy violations found.**
- ✅ Only `internal/handlers/api.go` and `internal/handlers/api_test.go` modified — no linter config, Makefile, CI config, or test infrastructure changes
- ✅ Blank line before every `return` statement (AGENTS.md / Go styleguide)
- ✅ No inline error handling — plain assignment used (`isMember, err := ...`)
- ✅ Error handling follows existing `handleChannelMsg` pattern exactly: `IsChannelMember` → log + 500 on DB error → IRC numeric error on non-member
- ✅ Uses `irc.ErrNotOnChannel` (442) which is the semantically correct IRC numeric for TOPIC on an unjoined channel (vs `ErrCannotSendToChan` used for PRIVMSG — both are correct per RFC 1459)
- ✅ No new exported types or functions introduced — no doc comment requirements triggered
- ✅ `.golangci.yml` untouched
- ✅ No hardcoded magic numbers
- ✅ No secrets, PII, or credentials in the diff
### Requirements Checklist (from [issue #33](https://git.eeqj.de/sneak/chat/issues/33))
| Requirement | Status |
|---|---|
| Add `IsChannelMember` check in `handleTopic` before `SetTopic` | ✅ Met — check added after `GetChannelByName` and before `executeTopic` |
| Mirror pattern from `handleChannelMsg` | ✅ Met — identical structure: `IsChannelMember` call → DB error handling with log + 500 → non-member IRC error response |
| Non-members receive appropriate error | ✅ Met — returns `ERR_NOTONCHANNEL` (442), correct per RFC 1459 §4.2.4 |
| Test coverage for the fix | ✅ Met — `TestTopicNonMember` creates channel with alice, bob attempts TOPIC without joining, asserts 442 |
### Build Result
✅ `docker build .` passes clean — fmt-check, lint, test, and build all succeed.
### Additional Review Notes
- **Correctness**: The membership check is placed at exactly the right point — after the channel is resolved (so we have `chID`) but before `executeTopic` is called. No path can bypass the check.
- **Edge cases**: Error path from `IsChannelMember` DB failure correctly returns 500 with logged error, not an IRC numeric (matches existing pattern).
- **Test quality**: `TestTopicNonMember` is well-structured — creates two separate sessions, only one joins, verifies the non-member gets 442. Drains initial messages before the test command to avoid false positives.
- **Scope**: Minimal, surgical fix — exactly what was asked for, nothing more.
- **No cheating**: No modified linter config, no weakened assertions, no suppressed rules.
### Verdict: **PASS**
The fix is correct, minimal, policy-compliant, and well-tested. The membership check mirrors the established pattern from `handleChannelMsg` and uses the semantically appropriate IRC error code.
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Summary
handleTopicininternal/handlers/api.godid NOT check that the user was a member of the channel before allowing them to set a topic. Any authenticated user could set the topic on any channel they hadn't joined.Changes
internal/handlers/api.go: AddedIsChannelMembercheck after resolving the channel ID and before callingexecuteTopic, mirroring the existing pattern inhandleChannelMsg. Non-members now receiveERR_NOTONCHANNEL(442).internal/handlers/api_test.go: AddedTestTopicNonMember— creates a channel with one user, then verifies a second user who hasn't joined receives numeric 442 when attempting to set the topic.Testing
TestTopicNonMembertest validates the fixdocker build .passes clean (formatting, linting, tests, build)closes #33
Review: PR #75 — security: enforce channel membership check in handleTopic
Policy Compliance Check
No policy violations found.
internal/handlers/api.goandinternal/handlers/api_test.gomodified — no linter config, Makefile, CI config, or test infrastructure changesreturnstatement (AGENTS.md / Go styleguide)isMember, err := ...)handleChannelMsgpattern exactly:IsChannelMember→ log + 500 on DB error → IRC numeric error on non-memberirc.ErrNotOnChannel(442) which is the semantically correct IRC numeric for TOPIC on an unjoined channel (vsErrCannotSendToChanused for PRIVMSG — both are correct per RFC 1459).golangci.ymluntouchedRequirements Checklist (from issue #33)
IsChannelMembercheck inhandleTopicbeforeSetTopicGetChannelByNameand beforeexecuteTopichandleChannelMsgIsChannelMembercall → DB error handling with log + 500 → non-member IRC error responseERR_NOTONCHANNEL(442), correct per RFC 1459 §4.2.4TestTopicNonMembercreates channel with alice, bob attempts TOPIC without joining, asserts 442Build Result
✅
docker build .passes clean — fmt-check, lint, test, and build all succeed.Additional Review Notes
chID) but beforeexecuteTopicis called. No path can bypass the check.IsChannelMemberDB failure correctly returns 500 with logged error, not an IRC numeric (matches existing pattern).TestTopicNonMemberis well-structured — creates two separate sessions, only one joins, verifies the non-member gets 442. Drains initial messages before the test command to avoid false positives.Verdict: PASS
The fix is correct, minimal, policy-compliant, and well-tested. The membership check mirrors the established pattern from
handleChannelMsgand uses the semantically appropriate IRC error code.