Fix access control check for repository name #90

2025-12-29T11:48:08+08:00

s0urcelab commented

2025-12-29 11:48:08 +08:00

(Migrated from github.com)

should fix #89

前缀匹配应该对比仓库名称（reponame）而不是完整名称（fullname）

should fix #89 前缀匹配应该对比仓库名称（reponame）而不是完整名称（fullname）

coderabbitai[bot] commented

2025-12-29 11:48:17 +08:00

(Migrated from github.com)

📝 Walkthrough

Walkthrough

The checkList function in access control logic has been updated to fix wildcard prefix matching. When a list item ends with "*", the function now compares only the repository name against the prefix, rather than the full "username/repo" string.

Changes

Cohort / File(s)	Summary
Wildcard prefix matching fix `src/utils/access_control.go`	Modified wildcard comparison in `checkList` to match against repository name only instead of full repository string (username/repo), narrowing the scope of prefix matching.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 A wildcard once danced with confusion,
Matching repos through a foggy delusion,
But now with a snip of the username away,
The prefix finds home—no longer astray! ✨

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and specifically describes the main change: fixing the access control check for repository name pattern matching.
Linked Issues check	✅ Passed	The change directly addresses issue #89 by fixing wildcard pattern matching to compare only the repository name instead of the full username/repo string.
Out of Scope Changes check	✅ Passed	All changes are focused on fixing the repository pattern matching logic in the access control check, directly addressing the linked issue.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

<details> <summary>📝 Walkthrough</summary> ## Walkthrough The `checkList` function in access control logic has been updated to fix wildcard prefix matching. When a list item ends with "*", the function now compares only the repository name against the prefix, rather than the full "username/repo" string. ## Changes | Cohort / File(s) | Summary | |---|---| | **Wildcard prefix matching fix** <br> `src/utils/access_control.go` | Modified wildcard comparison in `checkList` to match against repository name only instead of full repository string (username/repo), narrowing the scope of prefix matching. | ## Estimated code review effort 🎯 2 (Simple) | ⏱️ ~10 minutes ## Poem > 🐰 A wildcard once danced with confusion, > Matching repos through a foggy delusion, > But now with a snip of the username away, > The prefix finds home—no longer astray! ✨ </details>   ## Pre-merge checks and finishing touches <details> <summary>✅ Passed checks (5 passed)</summary> | Check name | Status | Explanation | | :------------------------: | :------- | :-------------------------------------------------------------------------------------------------------------------------------------------------------- | | Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. | | Title check | ✅ Passed | The title clearly and specifically describes the main change: fixing the access control check for repository name pattern matching. | | Linked Issues check | ✅ Passed | The change directly addresses issue #89 by fixing wildcard pattern matching to compare only the repository name instead of the full username/repo string. | | Out of Scope Changes check | ✅ Passed | All changes are focused on fixing the repository pattern matching logic in the access control check, directly addressing the linked issue. | | Docstring Coverage | ✅ Passed | No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. | </details>   <details> <summary>✨ Finishing touches</summary> - [ ]  📝 Generate docstrings <details> <summary>🧪 Generate unit tests (beta)</summary> - [ ]  Create PR with unit tests - [ ]  Post copyable unit tests in a comment </details> </details>   --- Thanks for using [CodeRabbit](https://coderabbit.ai?utm_source=oss&utm_medium=github&utm_campaign=sky22333/hubproxy&utm_content=90)! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. <details> <summary>❤️ Share</summary> - [X](https://twitter.com/intent/tweet?text=I%20just%20used%20%40coderabbitai%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20the%20proprietary%20code.%20Check%20it%20out%3A&url=https%3A//coderabbit.ai) - [Mastodon](https://mastodon.social/share?text=I%20just%20used%20%40coderabbitai%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20the%20proprietary%20code.%20Check%20it%20out%3A%20https%3A%2F%2Fcoderabbit.ai) - [Reddit](https://www.reddit.com/submit?title=Great%20tool%20for%20code%20review%20-%20CodeRabbit&text=I%20just%20used%20CodeRabbit%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20proprietary%20code.%20Check%20it%20out%3A%20https%3A//coderabbit.ai) - [LinkedIn](https://www.linkedin.com/sharing/share-offsite/?url=https%3A%2F%2Fcoderabbit.ai&mini=true&title=Great%20tool%20for%20code%20review%20-%20CodeRabbit&summary=I%20just%20used%20CodeRabbit%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20proprietary%20code) </details> <sub>Comment `@coderabbitai help` to get the list of available commands and usage tips.</sub>

coderabbitai[bot] (Migrated from github.com) reviewed 2025-12-29 11:52:54 +08:00

coderabbitai[bot] (Migrated from github.com) left a comment

Actionable comments posted: 0

🧹 Nitpick comments (1)

src/utils/access_control.go (1)

193-198: Fix correctly handles simple wildcard patterns; edge case with slash-containing patterns is valid but appears unused.

The change correctly fixes wildcard patterns like "test*" by comparing against the repository name instead of the full path. Based on the documented examples in config.toml ("baduser/malicious-repo", "/malicious-repo", "baduser/"), this covers the intended use cases.

However, patterns containing a slash (e.g., "user/repo*") would not work as expected:

Pattern "user/repo*" → prefix = "user/repo"

Check becomes strings.HasPrefix(repoName, "user/repo") where repoName is just "repo"

This would never match

No examples in the codebase suggest such patterns are used. The explicit pattern handling for username+"/*" at line 189 already covers the namespace-level blocking case.

If robustness against slash-containing wildcards is desired, consider checking whether the prefix contains "/" and matching against fullRepo in that case.

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 31f62fde35 and 6a1ddd8261.

📒 Files selected for processing (1)

src/utils/access_control.go

**Actionable comments posted: 0** <details> <summary>🧹 Nitpick comments (1)</summary><blockquote> <details> <summary>src/utils/access_control.go (1)</summary><blockquote> `193-198`: **Fix correctly handles simple wildcard patterns; edge case with slash-containing patterns is valid but appears unused.** The change correctly fixes wildcard patterns like "test*" by comparing against the repository name instead of the full path. Based on the documented examples in `config.toml` ("baduser/malicious-repo", "*/malicious-repo", "baduser/*"), this covers the intended use cases. However, patterns containing a slash (e.g., "user/repo*") would not work as expected: - Pattern "user/repo*" → prefix = "user/repo" - Check becomes `strings.HasPrefix(repoName, "user/repo")` where `repoName` is just "repo" - This would never match No examples in the codebase suggest such patterns are used. The explicit pattern handling for `username+"/*"` at line 189 already covers the namespace-level blocking case. If robustness against slash-containing wildcards is desired, consider checking whether the prefix contains "/" and matching against `fullRepo` in that case. </blockquote></details> </blockquote></details> <details> <summary>📜 Review details</summary> **Configuration used**: defaults **Review profile**: CHILL **Plan**: Pro <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 31f62fde352e3ac259225b60aed15fe0627ab07c and 6a1ddd82614af48f2d1b36ba6b9692e74673051b. </details> <details> <summary>📒 Files selected for processing (1)</summary> * `src/utils/access_control.go` </details> </details>

sky22333 commented

2025-12-29 14:17:52 +08:00

(Migrated from github.com)

@s0urcelab 有些用户需要匹配仓库的用户名，建议fork修改自己的版本

s0urcelab commented

2025-12-30 10:23:59 +08:00

(Migrated from github.com)

@s0urcelab 有些用户需要匹配仓库的用户名，建议fork修改自己的版本

建议修改一下文档里的说明，描述为“用户名匹配”而不是“前缀匹配”比较好

> @s0urcelab 有些用户需要匹配仓库的用户名，建议fork修改自己的版本建议修改一下文档里的说明，描述为“用户名匹配”而不是“前缀匹配”比较好

sky22333 commented

2025-12-30 10:33:03 +08:00

(Migrated from github.com)

@s0urcelab 有些用户需要匹配仓库的用户名，建议fork修改自己的版本

建议修改一下文档里的说明，描述为“用户名匹配”而不是“前缀匹配”比较好

OK，有空我修复一下让用户名也支持通配符

> > @s0urcelab 有些用户需要匹配仓库的用户名，建议fork修改自己的版本 > > 建议修改一下文档里的说明，描述为“用户名匹配”而不是“前缀匹配”比较好 OK，有空我修复一下让用户名也支持通配符

Pull request closed

This pull request cannot be reopened because the branch was deleted.

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: 3344/hubproxy#90