MM-68375: Verify channel membership before posting from createissue nd createissuecomment handlers

[nang2049]

Summary

fix reported via Bugcrowd. Both /api/v1/createissue and /api/v1/createissuecomment accepted a post_id loaded the referenced post and created a reply in that post's channel without checking that the calling user could post there. An authenticated user could supply a post_id from a private channel they were not a member of and have the plugin reply on their behalf in that channel.

Ticket Link

https://mattermost.atlassian.net/browse/MM-68375

QA with Curl

Create two users:
User A is member of a private channel
User B is not a member

Connect User B via /github connect. User B needs at least one repo they can create issues on.
As User A post any message in private-test. Right-click → Copy ID.
Get User B's session token: in their browser DevTools → Application → Cookies → MMAUTHTOKEN.

Verified:

1. createissuecomment now return 403

curl -i -X POST https://<your-mm-url>/plugins/github/api/v1/createissuecomment \
  -H "Cookie: MMAUTHTOKEN=<USER_B_TOKEN>" \
  -H "Content-Type: application/json" \
  -d '{
    "post_id": "<POST_ID>",
    "owner": "<USER_B_GITHUB_OWNER>",
    "repo": "<USER_B_TEST_REPO>",
    "number": 1,
    "comment": "qa test"
  }'
  
  Body: {"id":"","message":"not authorized to post in this channel","status_code":403}

Same for createissue with post_id.

[coderabbitai]

📝 Walkthrough

Walkthrough

Permission checks added to GitHub write actions: createIssueComment and createIssue now validate PermissionCreatePost before processing, returning 403 errors for unauthorized users.

Changes

GitHub Write Action Permission Enforcement

Layer / File(s)	Summary
Channel permission validation for write actions server/plugin/api.go	createIssueComment validates the user has PermissionCreatePost for the post's channel. createIssue enforces the same permission when using channel_id directly or after loading the post when using post_id. Unauthorized requests return a 403 error in all cases.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Poem

🐰 A rabbit hops through channels with care,
Checking permissions before posting there—
Three gates now guarded, three handlers in place,
No sneaking past GitHub's authorization space!

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title describes the core security fix—verifying channel membership/permission before posting—but contains a typo ('createissue nd createissuecomment' should be 'createissue and createissuecomment'), making it slightly unclear.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch MM-68375-channel-membership-check

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (1)

server/plugin/api.go (1)

1030-1033: ⚡ Quick win

Add regression tests for all three authorization branches.

This closes a security-sensitive gap in three separate paths, but there’s no accompanying handler coverage here for the forbidden case. Please add API tests for /createissuecomment with an inaccessible post_id, plus /createissue for both the channel_id and post_id modes, so this private-channel posting bug doesn’t regress silently.

Also applies to: 1734-1737, 1754-1757

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@server/plugin/api.go` around lines 1030 - 1033, Add regression API tests
covering the three authorization branches that return forbidden when the user
lacks channel post permissions: write a test for the /createissuecomment handler
that calls with an inaccessible post_id and asserts the response is HTTP 403 and
the API error message matches the path in api.go where
p.client.User.HasPermissionToChannel(...) triggers the forbidden branch; also
add two tests for /createissue covering both modes (channel_id mode and post_id
mode) that simulate a user without permission on the target channel/post and
assert HTTP 403 and the same error response. Ensure the tests exercise the exact
handlers that call p.client.User.HasPermissionToChannel and the code paths
referenced around lines 1030 and the similar checks at 1734-1737 and 1754-1757.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@server/plugin/api.go`:
- Around line 1030-1033: Add regression API tests covering the three
authorization branches that return forbidden when the user lacks channel post
permissions: write a test for the /createissuecomment handler that calls with an
inaccessible post_id and asserts the response is HTTP 403 and the API error
message matches the path in api.go where
p.client.User.HasPermissionToChannel(...) triggers the forbidden branch; also
add two tests for /createissue covering both modes (channel_id mode and post_id
mode) that simulate a user without permission on the target channel/post and
assert HTTP 403 and the same error response. Ensure the tests exercise the exact
handlers that call p.client.User.HasPermissionToChannel and the code paths
referenced around lines 1030 and the similar checks at 1734-1737 and 1754-1757.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 79894bc5-1ca9-418e-97fb-50004dffcc23

📥 Commits

Reviewing files that changed from the base of the PR and between 5e32dfe and 3723469.

📒 Files selected for processing (1)

• server/plugin/api.go

[avasconcelos114]

LGTM!