anthropics / anthropics/claude-code-action
isAllowedBot is duplicated byte-for-byte across checkHumanActor and checkWritePermissions
- 主要言語
- TypeScript
- スター
- 8.9k
- フォーク
- 2.1k
- PR マージ指標
- PR 指標を取得中
説明
**Type:** maintainability / drift risk in a security control
**Severity:** medium
**Area:** `src/github/validation/actor.ts`, `src/github/validation/permissions.ts`
**Effort:** trivial
## Summary
The `allowed_bots` allow-list matcher exists twice, as two identical private
copies in two different files. Both are part of the action's authorization path.
Verified identical:
```
$ diff <(sed -n 8,26p src/github/validation/permissions.ts) \
<(sed -n 11,29p src/github/validation/actor.ts)
# (no output)
```
## Affected code
- `src/github/validation/permissions.ts:8-25` - used by `checkActorWritePermissions`
when the collaborator-permission API reports the actor "is not a user"
- `src/github/validation/actor.ts:11-28` - used by `checkHumanActor` for
non-`User` account types and for actors the Users API cannot resolve
```ts
function isAllowedBot(actor: string, allowedBots: string): boolean {
const trimmed = allowedBots.trim();
if (trimmed === "*") return true;
if (!trimmed) return false;
const allowedList = trimmed
.split(",")
.map((bot) => bot.trim().toLowerCase().replace(/\[bot\]$/, ""))
.filter((bot) => bot.length > 0);
const normalizedActor = actor.toLowerCase().replace(/\[bot\]$/, "");
return allowedList.includes(normalizedActor);
}
```
## Impact
Both call sites gate whether an automated actor may trigger Claude. A fix or
hardening applied to one copy - handling whitespace inside entries, supporting a
`*[bot]` wildcard the way `actorMatchesPattern` does, normalising a different
suffix - silently does not apply to the other. The two checks then disagree about
whether a given bot is allowed, which is the worst outcome for an authorization
predicate: one layer permits what the other denies, and which one wins depends on
which code path the event happens to take.
There is no test asserting the two stay in agreement.
## Suggested fix
Move the function to the existing actor-filter utility and import it in both
validators:
```ts
// src/github/utils/actor-filter.ts
export function isAllowedBot(actor: string, allowedBots: string): boolean { ... }
```
`src/github/utils/actor-filter.ts` already owns the neighbouring concerns
(`parseActorFilter`, `resolveActorName`, `actorMatchesPattern`,
`shouldIncludeCommentByActor`) and already has coverage in
`test/actor-filter.test.ts`, so the shared copy inherits a test home.
Worth noting while consolidating: `isAllowedBot` does exact matching after
stripping a `[bot]` suffix, while `actorMatchesPattern` in the same area supports
a `*[bot]` wildcard. `allowed_bots` documents `'*'` for "all bots" so the
behaviours are defensible, but having them in one file makes the difference
visible instead of accidental.
コントリビューションガイド
評価
この issue はまだ評価されていません。