Skip to content

feat: 添加用户屏蔽检查功能,防止被组织屏蔽的用户评论#149

Open
Enter-tainer wants to merge 1 commit intomasterfrom
mgt/org-block
Open

feat: 添加用户屏蔽检查功能,防止被组织屏蔽的用户评论#149
Enter-tainer wants to merge 1 commit intomasterfrom
mgt/org-block

Conversation

@Enter-tainer
Copy link
Member

No description provided.

@Enter-tainer Enter-tainer requested review from Copilot and shaokeyibb and removed request for Copilot September 22, 2025 09:15
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Adds a GitHub organization block check to prevent users blocked by the org from posting comments.

  • Adds an OAuth utility to query GitHub’s org block API
  • Integrates the check into the comment POST route
  • Updates documentation with development/testing details

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File Description
cloudflare-workers/src/oauth.ts Introduces isUserBlockedByOrg to check org block status via GitHub API
cloudflare-workers/src/index.ts Calls isUserBlockedByOrg before creating a comment to block disallowed users
CLAUDE.md Expands dev/test setup and schema info; general docs improvements
Comments suppressed due to low confidence (1)

CLAUDE.md:1

  • Document how the backend obtains the token used for the org block check (GitHub App installation access token or a PAT with admin:org) and its required permissions/scopes. Clarify that GITHUB_APP_CLIENT_SECRET is not used as an API Bearer token, and add any additional variables needed to mint or supply the correct access token.
# CLAUDE.md

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

if (token.provider === 'github') {
const [org] = env.GITHUB_ORG_ADMINISTRATOR_TEAM.split('/');
try {
const isBlocked = await isUserBlockedByOrg(env.GITHUB_APP_CLIENT_SECRET, org, token.name);
Copy link

Copilot AI Sep 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

env.GITHUB_APP_CLIENT_SECRET is a client secret, not an access token, and should not be sent as a Bearer token to the GitHub API. Use a GitHub App installation access token (or a PAT with admin:org scope) to call GET /orgs/{org}/blocks/{username}. For example, obtain an installation token and pass it here; do not use the client secret in Authorization.

Suggested change
const isBlocked = await isUserBlockedByOrg(env.GITHUB_APP_CLIENT_SECRET, org, token.name);
const isBlocked = await isUserBlockedByOrg(env.GITHUB_APP_INSTALLATION_TOKEN, org, token.name);

Copilot uses AI. Check for mistakes.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里不太确定说的对不对

if (token.provider === 'github') {
const [org] = env.GITHUB_ORG_ADMINISTRATOR_TEAM.split('/');
try {
const isBlocked = await isUserBlockedByOrg(env.GITHUB_APP_CLIENT_SECRET, org, token.name);
Copy link

Copilot AI Sep 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The endpoint requires the GitHub login (username), not the user's display name. token.name is typically a display name; use the login (e.g., token.login or token.username) to avoid false negatives (404).

Suggested change
const isBlocked = await isUserBlockedByOrg(env.GITHUB_APP_CLIENT_SECRET, org, token.name);
const isBlocked = await isUserBlockedByOrg(env.GITHUB_APP_CLIENT_SECRET, org, token.login);

Copilot uses AI. Check for mistakes.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

token.name 存储的确实是用户的 displayName,这里需要的应该是 username。之前因为考虑到用户的 username 可能更改所以 token 目前存储是内部 id(integer),看了下 /orgs/{org}/blocks/{username} 似乎不支持通过 id 来查,需要看看这里是用 id 再查一次 username(或者说,login);还是说让 token 修改/增加存储 username(login)。
基于非冗余的设计,我个人可能偏向现场查,然后加个缓存避免每次都查

});

// 204 = 用户被屏蔽, 404 = 用户未被屏蔽
return response.status === 204;
Copy link

Copilot AI Sep 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Non-204/404 responses (e.g., 401/403/5xx) are treated as 'not blocked', and the caller’s try/catch will not run because fetch does not throw on HTTP errors. If you intend to observe failures in the caller, throw on unexpected statuses, e.g., return true/false only for 204/404 and throw otherwise.

Suggested change
return response.status === 204;
if (response.status === 204) {
return true;
}
if (response.status === 404) {
return false;
}
throw new Error(`Unexpected status code from GitHub API: ${response.status} - ${await response.text()}`);

Copilot uses AI. Check for mistakes.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

我觉得 Copilot 说得对

});

// 204 = 用户被屏蔽, 404 = 用户未被屏蔽
return response.status === 204;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

我觉得 Copilot 说得对

if (token.provider === 'github') {
const [org] = env.GITHUB_ORG_ADMINISTRATOR_TEAM.split('/');
try {
const isBlocked = await isUserBlockedByOrg(env.GITHUB_APP_CLIENT_SECRET, org, token.name);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

token.name 存储的确实是用户的 displayName,这里需要的应该是 username。之前因为考虑到用户的 username 可能更改所以 token 目前存储是内部 id(integer),看了下 /orgs/{org}/blocks/{username} 似乎不支持通过 id 来查,需要看看这里是用 id 再查一次 username(或者说,login);还是说让 token 修改/增加存储 username(login)。
基于非冗余的设计,我个人可能偏向现场查,然后加个缓存避免每次都查

if (token.provider === 'github') {
const [org] = env.GITHUB_ORG_ADMINISTRATOR_TEAM.split('/');
try {
const isBlocked = await isUserBlockedByOrg(env.GITHUB_APP_CLIENT_SECRET, org, token.name);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里不太确定说的对不对

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants